fix(cli): identity-only auth-epoch hashing for token credentials (#74312)#74493
Conversation
|
Codex review: needs changes before merge. Summary Reproducibility: yes. source-level. Current main can parse a partial Claude OAuth credential as token-shaped, hash rotating token material into the auth epoch, and reset same-version reusable sessions when that hash changes. Real behavior proof Next step before merge Security Review findings
Review detailsBest possible solution: Land a narrowed auth-epoch fix that keeps Claude token-shaped local reads stable across OAuth rotation, bumps the epoch version, and preserves generic static-token auth-profile invalidation unless a stable identity boundary is explicit. Do we have a high-confidence way to reproduce the issue? Yes, source-level. Current main can parse a partial Claude OAuth credential as token-shaped, hash rotating token material into the auth epoch, and reset same-version reusable sessions when that hash changes. Is this the best way to solve the issue? No, not as currently proposed. Identity-only hashing for Claude token-shaped local credentials is the right direction, but applying it to every token auth profile is broader than the documented static-token contract. Full review comments:
Overall correctness: patch is incorrect Security concerns:
Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against ba8a6499f02f. |
|
Thanks for picking this up. The One defense-in-depth observation while you're in this code, scoped separately from parser hardening:
A null-safe fallback closes that path: case "claude-cli": {
const credential = cliAuthEpochDeps.readClaudeCliCredentialsCached({
ttlMs: 5e3,
allowKeychainPrompt: false
});
return hashCliAuthEpochPart(encodeClaudeCredential(credential ?? { type: "oauth", provider: "anthropic" }));
}I observed this empirically on macOS — once-per-rotation pattern post-v1-patch (3 sessions resets at 08:01 EDT today, before I added the null-safe fallback). After both patches, zero further race-induced resets across 5h runtime; only the one expected migration reset. Happy to file as a separate follow-up PR if you'd prefer it scoped that way — not blocking this fix. Re-review progress:
|
f54fad9 to
2c9c06b
Compare
|
thanks for the empirical confirmation and the null-safe fallback catch — that shape-flip was a real gap, the encoder-only fix wasn't sufficient. just rebased onto current main (resolves the dirty state clawsweeper flagged) and rolled the null-safe fallback into the same commit since it's the same root cause and your validation makes it ready-to-land:
scoped to claude-cli per your repro; codex-cli and google-gemini-cli have the same structural shape but no reported empirical evidence yet, so leaving those for a separate follow-up if anyone hits the same pattern there. 13 cli-auth-epoch tests pass, 16 cli-credentials tests pass, tsgo:core + core:test clean. CI rerun pending after force-push. |
c484f66 to
9f05be3
Compare
c2fe876 to
cf125cc
Compare
cf125cc to
57502b2
Compare
57502b2 to
c9e64f2
Compare
c9e64f2 to
96d3857
Compare
|
Rebased onto current main and resolved the Local proof:
|
96d3857 to
a795060
Compare
a795060 to
78a9d6b
Compare
78a9d6b to
047feda
Compare
|
Fixed the ClawSweeper auth-state blocker. I removed the synthetic null Claude credential fallback, so identity-only hashing now applies only when a Claude credential is actually read. A true absent credential returns Validation:
@clawsweeper re-review Re-review progress:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
9e059ce to
38e0aea
Compare
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Heads up: this PR needs to be updated against current |
…nclaw#74312) Extend Ayaan's OAuth identity-only hashing pattern (1ff461f, 3eb6edc) and Chunyue's Gemini extension (openclaw#71076) to the static-token credential branch. Token rotation and transient partial keychain reads no longer flip the claude-cli auth-epoch. Before: encodeClaudeCredential token branch hashed credential.token, and encodeAuthProfileCredential token case did the same. Either rotation or a partial keychain read (where parseClaudeCliOauthCredential sees accessToken without refreshToken and falls into the token branch) flipped the auth-epoch and forced cli session reset: reason=auth-epoch on every active claude-cli conversation. macOS keychain-only setups hit the auth-profile path as the dominant trigger. After: encodeClaudeCredential routes both branches through encodeOAuthIdentity, collapsing partial reads onto the same provider-keyed identity hash. encodeAuthProfileCredential token case drops credential.token from the hash; identity fields (provider, tokenRef, email, displayName) are the discriminator. CLI_AUTH_EPOCH_VERSION bumped 4 to 5. Test updates flip the existing token-flips assertion to stability, add a regression case proving Claude CLI OAuth and token epochs match (closing the partial-read race), and add auth-profile token tests for both rotation stability and identity-change invalidation.
38e0aea to
c2b786f
Compare
|
Verification before merge for
Known proof gap: no live macOS Claude keychain rotation timing test was run; the proof exercises the auth-epoch layer that made transient token-shaped Claude CLI reads reset reusable sessions. |
Summary
cli session reset: provider=claude-cli reason=auth-epochclusters that correlate with claude-cli's OAuth token rotation. Reporter (claude-cli auth-epoch flips on token rotation, forcing session resets mid-conversation #74312) observed ~16 resets in 24h on one host, accelerating to 2 resets in 3 minutes near token expiry. The user's live conversation context is dropped on every rotation.accessTokenwithoutrefreshToken, causingparseClaudeCliOauthCredentialto returntype: "token"instead oftype: "oauth". The previousencodeClaudeCredentialtoken branch hashedcredential.tokendirectly, so that transient race flipped the auth-epoch and reset every reusable claude-cli session. On macOS keychain-only setups (no~/.claude/.credentials.jsonfallback),getLocalCliCredentialFingerprintreturnsundefinedand the auth-profile branch is the dominant trigger;encodeAuthProfileCredentialtoken case had the same hash-rotates-with-token defect.1ff461fe7b(fix(cli): stabilize oauth session auth epochs) and3eb6edc67c(fix(cli): key oauth session epochs on identity), and which Chunyue extended to Gemini in fix(cli): key gemini cli auth epoch on google account identity #71076, to the static-token credential branch.encodeClaudeCredentialroutes both OAuth and token claude-cli credentials throughencodeOAuthIdentity, collapsing partial keychain reads onto the same provider-keyed identity hash.encodeAuthProfileCredentialtoken case dropscredential.tokenfrom the hash; identity fields (provider, tokenRef, email, displayName) are the discriminator.CLI_AUTH_EPOCH_VERSIONbumped 4 → 5.parseClaudeCliOauthCredential(the parser, not hardened here per scope discipline; identity-only hashing makes the partial-read race harmless without parser changes), the OAuth branches (already identity-only since3eb6edc67c), Gemini and Codex encoders (already identity-only),resolveCliAuthEpochorchestration, or any consumer code (prepareExecutionreads the same string).Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
1ff461fe7b,3eb6edc67c, fix(cli): key gemini cli auth epoch on google account identity #71076 (the OAuth + Gemini precedent)Root Cause
encodeClaudeCredentialandencodeAuthProfileCredentialretainedcredential.tokenin the auth-epoch hash for the static-token branch, even though Ayaan's recent commits had moved OAuth credentials and Chunyue's fix(cli): key gemini cli auth epoch on google account identity #71076 had moved Gemini OAuth onto identity-only hashing. The remaining token branches hashed rotating material, so any transienttype: "token"parse (which the macOS keychain rewrite race triggers reliably) flipped the epoch.expect(second).not.toBe(first)— i.e., locked in the buggy behavior. There was no regression case proving OAuth and token claude-cli epochs match (which the partial-read race requires).parseClaudeCliOauthCredentialfalls through totype: "token"whenrefreshTokenis empty/missing. This is correct for credentials that were never OAuth, but produces falsetype: "token"parses during keychain rotation. The hash semantics needed to be stable across that fallthrough.Regression Test Plan
src/agents/cli-auth-epoch.test.tskeeps claude cli token epochs stable across token rotation(replaces the previouschanges ... when the static token changesassertion)matches claude cli token and oauth epochs so partial keychain reads do not flip(new regression case for the partial-read race)keeps token auth-profile epochs stable across credential.token rotation(auth-profile branch — the dominant trigger on keychain-only macOS setups)changes token auth-profile epochs when the email identity changes(preserves real account-switch invalidation)User-visible / Behavior Changes
cli session reset: reason=auth-epochlog lines no longer correlate with token rotation; they now only fire on real identity changes (provider, email, tokenRef, displayName).CLI_AUTH_EPOCH_VERSIONbumped 4 → 5; existing stored auth-epochs invalidate once on first read after upgrade (one expected reset per host on the upgrade boundary).Security Impact
NoNo(the hash is over identity fields; tokens are still read and used for auth requests, just no longer included in the epoch fingerprint)NoNoNoRepro + Verification
Environment
Steps (per reporter)
gateway.outforcli session reset: provider=claude-cli reason=auth-epochlines.Expected (after this PR)
No
auth-epochresets during normal token rotation. Conversation context preserved across rotations.Actual (before this PR)
Resets on every rotation. Each reset drops mid-conversation context. Reporter measured ~16 resets/24h, peaking at 2 resets in 3 minutes near token expiry.
Actual (after this PR)
Local test verification:
pnpm test src/agents/cli-auth-epoch.test.ts→ 16 passed. The new and rebased cases prove (a) token rotation no longer flips, (b) OAuth and token epochs match, (c) auth-profile token rotation no longer flips, (d) OAuth profile aliases for the same account stay stable, and (e) real email identity changes still flip the epoch.Real behavior proof
57502b29fb0c8354124ae65d5861af75f855e25e, macOS host, Node 24.15.0, pnpm 10.33.2. The proof exercised the productionresolveCliAuthEpochpath with controlled Claude CLI and auth-profile credential providers.pnpm exec tsx -eproof script that resolved an epoch for a Claude token credential, rotated the token, resolved again, then repeated the same rotation check for an auth-profile token credential and finally changed the auth-profile email identity.Evidence
pnpm test src/agents/cli-auth-epoch.test.ts→ 16 passed.pnpm test src/agents/cli-credentials.test.ts→ 16 passed (no regressions).pnpm test src/agents/cli-session.test.ts→ 12 passed (auth-epoch consumer).pnpm tsgo:coreandpnpm tsgo:core:testclean.oxlint0 warnings, 0 errors. Format check clean.Followed clawsweeper acceptance criteria from the issue:
pnpm test src/agents/cli-auth-epoch.test.ts src/agents/cli-credentials.test.ts✓pnpm exec oxfmt --check --threads=1 src/agents/cli-auth-epoch.ts src/agents/cli-auth-epoch.test.ts✓Human Verification
expect(second).toBe(first)mismatch.keeps identity-less claude cli oauth epochs stable across token changes,keeps oauth auth-profile epochs stable across token refreshes,changes oauth auth-profile epochs when the account identity changes) all still pass with their original assertions.cli-session.test.ts(12 tests) andcli-credentials.test.ts(16 tests) both pass unchanged.prepareExecutionatsrc/agents/cli-runner/prepare.ts:216only reads the resolved string, not the encoding internals.Review Conversations
Compatibility / Migration
Yesfor runtime behavior. TheCLI_AUTH_EPOCH_VERSIONbump (4 → 5) means cached epochs from prior versions invalidate once on first read; users see one expectedauth-epochreset on the upgrade boundary, then stable epochs afterward.NoNoRisks and Mitigations
type: "token"credential (e.g. PAT) and switching to a different token for the SAME identity will no longer get a session reset on the swap. Mitigation: This is the intended behavior per Ayaan's OAuth pattern — token replacement is an authorized refresh, not an identity change. Real identity changes (different email/provider/tokenRef) still invalidate.parseClaudeCliOauthCredentialpartial-read fallthrough still produces a "token" credential type that gets persisted to auth-profile, even though the hash is now stable. The semantic is misleading (claude-cli OAuth users may see a "token" auth-profile entry). Mitigation: Out of scope for this PR per the reporter's preferred fix shape ("identity-only hash for both types"). If parser hardening is desired as defense-in-depth, that's a clean follow-up — the current fix is sufficient for the reported symptom.CLI_AUTH_EPOCH_VERSIONbump invalidates cached epochs once. Mitigation: Documented behavior; one-time upgrade-boundary reset is acceptable cost vs ongoing rotation-induced resets.