fix(models): reflect Claude CLI auth status#71332
Conversation
Greptile SummaryThis PR fixes a false expiry/expiring warning in Confidence Score: 5/5Safe to merge — targeted, well-tested fix with no credential-persistence risk. All changes are minimal and directly address the stated bug. The No files require special attention. Reviews (1): Last reviewed commit: "fix(models): reflect Claude CLI auth sta..." | Re-trigger Greptile |
|
Codex review: do not land this until the runtime-only credential boundary is fixed. I checked the current auth-profile code path and Aisle's persistence concern looks real. The broad store overlay is also too much for a status command: this PR switches Suggested safer shape: keep |
|
Addressed the AI review findings in commit dfda286d51. What changed:
Review findings:
Security/runtime controls unchanged:
Tests run:
|
|
I will pause work on this given Peter's comment above about runtime-only credential boundary |
🔒 Aisle Security AnalysisWe found 2 potential security issue(s) in this PR:
1. 🟠 Synthetic auth credential (API key/token) included in `models status --json` output
DescriptionThe Although the human-readable output only prints Impact:
Vulnerable code paths:
Vulnerable code: syntheticAuthByProvider.set(normalized, {
value: "plugin-owned",
source: resolved?.source ?? "plugin synthetic auth",
credential: resolved?.apiKey,
mode: resolved?.mode,
expiresAt: resolved?.expiresAt,
});
...
resolveProviderAuthOverview({
...,
syntheticAuth: syntheticAuthByProvider.get(provider),
});
...
writeRuntimeJson(runtime, { auth: { providers: providerAuth, ... } });RecommendationDo not serialize raw credentials in Options:
Example fix (strip at call site): const synthetic = syntheticAuthByProvider.get(provider);
resolveProviderAuthOverview({
provider,
cfg,
store,
modelsPath,
syntheticAuth: synthetic
? { value: synthetic.value, source: synthetic.source }
: undefined,
});If you need to indicate presence of a credential, output a boolean or a masked value (e.g., via existing 2. 🟡 Untrusted plugin synthetic auth can spoof OAuth/token health status in models status output
Description
This creates a trust-boundary issue:
Vulnerable flow excerpts:
const resolved = resolveProviderSyntheticAuthWithPlugin({ /* plugin-controlled */ });
...
const runtimeCredentialsByProvider = new Map(
Array.from(syntheticAuthByProvider.entries())
.map(([provider, auth]) => [provider, syntheticAuthCredential(provider, auth)] as const)
.filter((entry): entry is readonly [string, AuthProfileCredential] => Boolean(entry[1])),
);
const healthCredential = runtimeCredential ?? credential;Because plugins are not necessarily trusted to provide truthful metadata, the status view can be made to show healthy auth when canonical stored auth is unhealthy. RecommendationTreat plugin/runtime synthetic auth as separate from stored profile health, or clearly label it. Options:
// buildProfileHealth: always compute from stored credential
const provider = normalizeProviderId(credential.provider);
...
// separately attach runtime health if present
const runtime = runtimeCredential ? computeHealth(runtimeCredential) : undefined;
return { ...storedHealth, runtime };
Analyzed PR: #71332 at commit Last updated on: 2026-04-26T03:51:01Z |
7b3f707 to
f805450
Compare
obviyus
left a comment
There was a problem hiding this comment.
Verified the models status/Claude CLI mismatch: status now reads runtime synthetic-auth expiry instead of hardcoding Claude credential reads into core auth health.
Maintainer follow-up: moved the changelog entry into the active top Fixes block and kept the provider-owned synthetic auth seam.
Local gate: focused auth/status/Anthropic tests passed; check:changed passed type/lint/import-cycle/guards, with one later Vitest worker-pool infrastructure failure in changed tests after the changelog-only commit.
|
Landed on main. Thanks @neeravmakwana. |
Summary
openclaw models statuscould reportanthropic:claude-clias expired or expiring from a stale stored profile while Claude CLI native credentials were fresh enough for runtime use.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
models statusbuilt OAuth health from stale stored profile expiry data, while Claude CLI runtime auth was resolved separately through the native Claude CLI credential path.anthropic:claude-clihealth reflects fresh native Claude CLI credentials when the stored profile has drifted stale.anthropic:claude-cliprofiles can drift from local Claude CLI credentials after the external CLI refreshes independently.Regression Test Plan (if applicable)
src/agents/auth-health.test.ts,src/commands/models/list.status.test.tsanthropic:claude-clihealth uses fresh native Claude CLI OAuth expiry for status while the status command keeps the no-external-profile store path.User-visible / Behavior Changes
openclaw models statusnow reflects fresh local Claude CLI OAuth credentials when the storedanthropic:claude-cliprofile is stale.Diagram (if applicable)
Security Impact (required)
Yes, explain risk + mitigation: N/ASecurity/runtime controls unchanged: the fix reuses the existing Claude CLI credential reader only inside auth-health status evaluation and does not add an external auth profile, plugin contract, store overlay, persistence path, refresh path, network call, or command execution surface.
Repro + Verification
Environment
anthropic:claude-clistored profile plus fresh native Claude CLI OAuth credentialsSteps
anthropic:claude-cliprofile.openclaw models status.Expected
Actual
Evidence
Human Verification (required)
What I personally verified, and how:
anthropic:claude-clistored profile when native Claude CLI OAuth credentials are fresh;models statuskeeps using the no-external-profile store path.AuthProfileStoreand cannot be persisted by external-profile save filtering.Exact tests run:
pnpm test src/agents/auth-health.test.ts src/commands/models/list.status.test.ts extensions/anthropic/index.test.tspnpm check:changedReview Conversations
AI-assisted: yes.
Compatibility / Migration
Risks and Mitigations
anthropic:claude-cliprofile.