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)))