From d328539c5bb4fac9d9a8c9682c81189471edb092 Mon Sep 17 00:00:00 2001 From: Jay Wedgeworth <12656028+jaywedgeworth22@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:53:00 -0500 Subject: [PATCH] fix(claude): re-read the Keychain once the remembered token expires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Claude row could sit on "login idle" indefinitely while Claude Code was signed in and working. PR #156 fixed the consent loop by remembering the Keychain payload in process memory, but `resolveSilently` returned that memory whenever it was non-nil, so the item was never re-read for the life of the process. Claude Code rewrites that same item every time it renews its own login, so the copy captured at launch went stale within the hour and the frozen, expired token kept answering instead. That also killed the self-healing path: after `renewClaudeLogin` ran and Claude Code refreshed its item, the reader asked for the credential again and got the same stale bytes back, so the renewal could never take effect. Memory is a permission fact, not a freshness fact. It records that macOS already granted `/usr/bin/security` this item, which is what stops the consent row re-arming on every launch. So keep serving memory while its own access token is unexpired, and re-read once it is not. The re-read is the same silent child, so nothing new prompts, and a denied or slow read still reports what we already hold rather than `.unauthorized`, which is the consent loop #156 closed. Verified against the live Keychain on this Mac: the item holds a valid `max` token expiring 2026-10-06T07:44Z, and with this change the same silent read resolves to `.connected` instead of `.idle`. The unified log had shown only "silent attributes status 0" every five minutes since launch, with zero "silent security CLI" lines, which is the short circuit itself. 331 tests pass, including a regression test that walks the real sequence — stale payload remembered, Claude Code renews underneath it, next refresh must return the new bytes. --- .../QuotaCore/ClaudeCredentialSource.swift | 63 ++++++++++++------- Tests/QuotaCoreTests/ClaudeConsentTests.swift | 53 +++++++++++++++- 2 files changed, 91 insertions(+), 25 deletions(-) diff --git a/Sources/QuotaCore/ClaudeCredentialSource.swift b/Sources/QuotaCore/ClaudeCredentialSource.swift index db3d852..1d2eb8e 100644 --- a/Sources/QuotaCore/ClaudeCredentialSource.swift +++ b/Sources/QuotaCore/ClaudeCredentialSource.swift @@ -296,40 +296,59 @@ public enum ClaudeCredentialSource { case .unknown: return .temporarilyUnavailable case .present: - if let data = rememberedCredential() { - // The item was authorized and read from Keychain into memory. - // Even if the token inside is expired, macOS Keychain access was - // already granted. Return .authorized(data) so LocalQuotaReader - // and ClaudeLoginState report it as signedOut/idle, not - // needsPermission. + // Memory is a permission fact, not a freshness fact. It records + // that macOS already granted `/usr/bin/security` this item, which + // is what stops the consent row from re-arming on every launch. + // It does NOT record the current token: Claude Code rewrites this + // same item every time it renews its own login, so a payload + // captured at launch goes stale within the hour. Answering from it + // forever (which is what shipped in #156) pinned the row to + // "login idle" while Claude Code sat right there signed in — the + // Keychain item held a fresh token the app simply never re-read. + // So: serve memory only while its own access token is unexpired. + let remembered = rememberedCredential() + if let data = remembered, rememberedGrantIsFresh(data) { return .authorized(data) } - // Nothing in memory, but the item is here. Before this was a bare - // `.unauthorized`, which re-armed the "needs permission" row on - // every single launch and made the owner re-click Allow Access - // forever — the grant was process-memory only, so it never - // survived a relaunch. - // - // Try the one read that can still succeed quietly. Once the owner - // has answered Always Allow, `/usr/bin/security` is on the item's - // access list and this returns the bytes without a panel, so the - // app reads itself for the rest of the session with no further - // prompting. The panel only reappears if the ACL is revoked, which - // is precisely when the owner should be asked again. - // - // A refusal here is not a failure: it is the first run on a fresh - // install that has never been granted, and the caller's - // `.unauthorized` is the correct answer for that case. + // Past that point, or with nothing remembered at all, read the + // bytes again. This is the one read that can succeed quietly: + // once the owner has answered Always Allow, `/usr/bin/security` is + // on the item's access list, so it returns without a panel and the + // app refreshes itself with no prompting. The panel only reappears + // if the ACL is revoked, which is precisely when the owner should + // be asked again. It is also what lets a renewal that Claude Code + // performed on our behalf actually be seen. let outcome = probe.readViaSecurityCLI(attemptDeadline, { isAbandoned() }) log.notice("claude keychain silent security CLI: \(outcome.logLabel, privacy: .public)") if case .found(let data) = outcome { remember(data) return .authorized(data) } + // The read produced nothing. With a grant already remembered that + // is not a permission problem — it is a slow or refused read on a + // loaded Mac, and reporting `.unauthorized` here is exactly the + // consent loop #156 closed. Serve what we already hold and let the + // row report signedOut/idle, which is the honest state. + if let remembered { return .authorized(remembered) } + // Nothing remembered and nothing readable: a fresh install that has + // never been granted, so `.unauthorized` is the correct answer. return .unauthorized } } + /// Whether the remembered payload still carries an access token that has + /// not expired. Only then may the refresh loop answer from memory. + /// + /// This is deliberately stricter than `rememberedGrantStillUsable`, which + /// also accepts an expired-but-renewable record. That looser rule is right + /// for deciding whether to keep trusting the *grant*, and wrong for + /// deciding whether to keep sending the *bytes*: an expired token answers + /// 401 no matter how renewable it is. + static func rememberedGrantIsFresh(_ data: Data, now: Date = Date()) -> Bool { + guard let root = ClaudeOAuthParser.parse(data) else { return false } + return ClaudeOAuthParser.validOAuth(in: root, now: now) != nil + } + /// A remembered payload is usable while its access token is still /// unexpired, or while it holds an expired access token that can be renewed /// via its refresh token. Anything else, including a rotated or unreadable diff --git a/Tests/QuotaCoreTests/ClaudeConsentTests.swift b/Tests/QuotaCoreTests/ClaudeConsentTests.swift index b397c92..513309f 100644 --- a/Tests/QuotaCoreTests/ClaudeConsentTests.swift +++ b/Tests/QuotaCoreTests/ClaudeConsentTests.swift @@ -353,18 +353,65 @@ final class ClaudeConsentTests: XCTestCase { XCTAssertNil(ClaudeCredentialSource.rememberedCredential()) } - func testExpiredRememberedGrantRemainsAuthorizedWithoutAskingForConsent() { + func testExpiredRememberedGrantIsReReadButStillNeverAsksForConsent() { + // An expired remembered payload is no longer served as the answer: it + // is stale, and Claude Code rewrites the item when it renews its own + // login. So the loop reads the bytes again. That read must NOT turn a + // read failure into `.unauthorized` — a grant already remembered proves + // macOS allows this item, and re-arming consent is the loop #156 closed. let expired = Data(#"{"claudeAiOauth":{"accessToken":"fixture","expiresAt":1}}"#.utf8) ClaudeCredentialSource.remember(expired) let log = ProbeLog() let access = ClaudeCredentialSource.resolveSilently( - probe: makeProbe(log: log, cli: [.found(expired)], presence: .present), + probe: makeProbe(log: log, cli: [.denied], presence: .present), isAbandoned: { false }) XCTAssertEqual(access, .authorized(expired)) - XCTAssertEqual(log.calls, ["lookup"]) + XCTAssertEqual(log.calls, ["lookup", "cli"], "an expired payload must be re-read") + XCTAssertEqual(ClaudeLoginState.resolve(hasUsableCredential: false, access: access), + .signedOut, "and it must not become a consent prompt") XCTAssertFalse(ClaudeCredentialSource.rememberedGrantStillUsable(expired, now: Date(timeIntervalSince1970: 10))) } + func testExpiredRememberedGrantPicksUpTheTokenClaudeCodeRenewedUnderIt() { + // The regression, end to end through the silent read. Launch reads an + // expired payload and remembers it; Claude Code then renews and rewrites + // the same Keychain item; the next refresh must return the NEW bytes. + // Serving the frozen copy forever is what pinned the row to "login idle" + // while Claude Code sat right there signed in (board dd5f6702). + let stale = Data(#"{"claudeAiOauth":{"accessToken":"old","expiresAt":1}}"#.utf8) + let renewed = Data(#"{"claudeAiOauth":{"accessToken":"new","expiresAt":4102444800000}}"#.utf8) + ClaudeCredentialSource.remember(stale) + let log = ProbeLog() + let access = ClaudeCredentialSource.resolveSilently( + probe: makeProbe(log: log, cli: [.found(renewed)], presence: .present)) + XCTAssertEqual(access, .authorized(renewed)) + XCTAssertEqual(log.calls, ["lookup", "cli"]) + XCTAssertEqual(ClaudeCredentialSource.rememberedCredential(), renewed) + // And the renewed token is now what the reader sends, so the row is + // connected rather than idle. + XCTAssertEqual(ClaudeLoginState.resolve(hasUsableCredential: true, access: access), .connected) + // Being fresh, it is served from memory again on the next cycle. + let next = ProbeLog() + XCTAssertEqual(ClaudeCredentialSource.resolveSilently( + probe: makeProbe(log: next, cli: [.denied], presence: .present)), + .authorized(renewed)) + XCTAssertEqual(next.calls, ["lookup"], "a fresh grant is still served without a child") + ClaudeCredentialSource.resetRememberedCredential() + } + + func testRememberedGrantIsFreshOnlyWhileItsAccessTokenIsUnexpired() { + let fresh = Data(#"{"claudeAiOauth":{"accessToken":"fixture","expiresAt":4102444800000}}"#.utf8) + let expired = Data(#"{"claudeAiOauth":{"accessToken":"fixture","expiresAt":1}}"#.utf8) + let now = Date(timeIntervalSince1970: 1_700_000_000) + XCTAssertTrue(ClaudeCredentialSource.rememberedGrantIsFresh(fresh, now: now)) + XCTAssertFalse(ClaudeCredentialSource.rememberedGrantIsFresh(expired, now: now)) + // Stricter than the grant test on purpose: renewable-but-expired is + // still an expired token, and an expired token answers 401. + let renewableExpired = Data(#"{"claudeAiOauth":{"accessToken":"old","refreshToken":"r","expiresAt":1}}"#.utf8) + XCTAssertTrue(ClaudeCredentialSource.rememberedGrantStillUsable(renewableExpired, now: now)) + XCTAssertFalse(ClaudeCredentialSource.rememberedGrantIsFresh(renewableExpired, now: now)) + } + func testUnexpiredRememberedGrantStaysUsable() { let payload = Data(#"{"claudeAiOauth":{"accessToken":"fixture","expiresAt":4102444800000}}"#.utf8) XCTAssertTrue(ClaudeCredentialSource.rememberedGrantStillUsable(payload, now: Date(timeIntervalSince1970: 1_700_000_000)))