From ca41021a5190dc0253bbabc4cc7d0cb24930c43f Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Sun, 31 May 2026 05:01:19 -0700 Subject: [PATCH] fix(menubar): treat the CLI credential store as the source of truth The menubar kept its own copy of each provider's OAuth grant and refreshed it on a timer, racing the CLI. Both Claude and Codex use single-use refresh tokens that rotate on every refresh, so the menubar's self-rotation could invalidate the user's own CLI login and surfaced as "disconnected" after a long idle period. Codex: read ~/.codex/auth.json fresh each cycle; only self-refresh when last_refresh is older than 8 days; on 401 re-read the source before spending our token; write rotated tokens back to auth.json (atomic, preserving other keys); recover from reuse/invalid_grant by re-reading instead of going terminal. Claude: never HTTP-refresh the CLI-owned token. On expiry/401 re-read the keychain with a no-UI query (LAContext.interactionNotAllowed) to adopt a token the CLI already rotated; when none is available yet, report a transient sourceTokenStale rather than a terminal disconnect. --- .../Data/ClaudeCredentialStore.swift | 186 ++++++------------ .../Data/CodexCredentialStore.swift | 91 ++++++++- .../Data/CodexSubscriptionService.swift | 2 +- 3 files changed, 150 insertions(+), 129 deletions(-) diff --git a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift index 56e1603d..df5b1a38 100644 --- a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift @@ -1,37 +1,30 @@ import Foundation +import LocalAuthentication import Security -/// Owns the lifecycle of Claude OAuth credentials end-to-end. Replaces -/// SubscriptionClient + SubscriptionRefreshGate with a model that mirrors -/// CodexBar's proven pattern: +/// Owns the lifecycle of Claude OAuth credentials, mirroring CodexBar's pattern: /// /// 1. **Bootstrap is user-initiated.** The first read of Claude's keychain /// entry — which triggers a macOS keychain prompt — only happens when /// the user clicks "Connect" in the Plan tab. The menubar does not /// touch Claude's keychain on launch. /// -/// 2. **We persist refreshed tokens.** When Anthropic returns a new access -/// token (or a rotated refresh token) we write it back to our own keychain -/// item. The next fetch uses it directly — one API call per cycle, not -/// three. This was the root cause of "connect once, never updates": the -/// previous code refreshed on every tick because the new token was -/// thrown away. +/// 2. **The Claude CLI owns the grant; we never refresh it ourselves.** +/// Claude's refresh token is single-use and rotates on every refresh, and +/// the CLI is refreshing the same grant. If the menubar spent that token +/// it would invalidate the CLI's own login. So on expiry/401 we re-read +/// the CLI's store for a token it has already rotated rather than calling +/// the refresh endpoint. If the CLI hasn't rotated yet we report a +/// transient staleness (`sourceTokenStale`) and recover on its next use. /// -/// 3. **Our own keychain item, not Claude's.** We bootstrap from Claude's -/// entry once, then maintain `com.codeburn.menubar.claude.oauth.v1` in -/// the user's keychain. Subsequent reads do not prompt because we own -/// that item's ACL. -/// -/// 4. **In-memory cache (5 min)** so back-to-back reads in the same refresh -/// cycle don't even hit the keychain. +/// 3. **In-memory + file cache** so back-to-back reads in the same refresh +/// cycle don't re-hit the source, and we keep serving the last good token +/// across launches. enum ClaudeCredentialStore { private static let bootstrapCompletedKey = "codeburn.claude.bootstrapCompleted" private static let inMemoryTTL: TimeInterval = 5 * 60 private static let proactiveRefreshMargin: TimeInterval = 5 * 60 - private static let oauthClientID = "9d1c250a-e61b-44d9-88ed-5944d1962f5e" - private static let refreshURL = URL(string: "https://platform.claude.com/v1/oauth/token")! - private static let claudeKeychainService = "Claude Code-credentials" private static let credentialsRelativePath = ".claude/.credentials.json" private static let maxCredentialBytes = 64 * 1024 @@ -61,10 +54,8 @@ enum ClaudeCredentialStore { case bootstrapDecodeFailed case keychainWriteFailed(OSStatus) case keychainReadFailed(OSStatus) - case refreshHTTPError(Int, String?) - case refreshNetworkError(Error) - case refreshDecodeFailed case noRefreshToken + case sourceTokenStale // CLI hasn't rotated yet; transient, not a re-auth var errorDescription: String? { switch self { @@ -76,28 +67,18 @@ enum ClaudeCredentialStore { return "Could not write to keychain (status \(status))." case let .keychainReadFailed(status): return "Could not read from keychain (status \(status))." - case let .refreshHTTPError(code, body): - return "Token refresh failed (HTTP \(code))\(body.map { ": \($0)" } ?? "")" - case let .refreshNetworkError(err): - return "Token refresh network error: \(err.localizedDescription)" - case .refreshDecodeFailed: - return "Token refresh response was malformed." case .noRefreshToken: return "No refresh token available; reconnect required." + case .sourceTokenStale: + return "Waiting for the Claude CLI to refresh its token." } } /// True when the failure means the user must re-authenticate (re-run /// `claude` or click Reconnect). Used by the UI to distinguish between - /// "try again later" and "you must act". + /// "try again later" and "you must act". `sourceTokenStale` is the CLI + /// not having rotated yet — transient, recovers on its next use. var isTerminal: Bool { - if case let .refreshHTTPError(code, body) = self, code >= 400, code < 500 { - let lower = body?.lowercased() ?? "" - if lower.contains("invalid_grant") || lower.contains("invalid_client") || lower.contains("invalid_token") { - return true - } - return true // 4xx other than rate-limiting is terminal too - } if case .noRefreshToken = self { return true } return false } @@ -155,23 +136,46 @@ enum ClaudeCredentialStore { return nil } - /// Returns a token guaranteed to be either fresh or just-refreshed. If the - /// current token expires within `proactiveRefreshMargin`, refreshes ahead - /// of time and persists the new token. + /// Returns the current token, adopting a fresher one from the CLI's store if + /// ours is near expiry. Never spends the refresh token — see the type doc. static func freshAccessToken() async throws -> String? { guard let record = try currentRecord() else { return nil } if let expiresAt = record.expiresAt, expiresAt.timeIntervalSinceNow < proactiveRefreshMargin { - let updated = try await refreshAndPersist(record: record) - return updated.accessToken + if let live = adoptFresherSource(than: record) { + return live.accessToken + } } return record.accessToken } - /// Called after an explicit 401. Refreshes, persists, returns the new token. + /// Called after an explicit 401. Delegates to the CLI: re-reads its store + /// (silently, no prompt) for a token it has already rotated. If none is + /// available yet, throws the transient `sourceTokenStale` rather than + /// spending the shared refresh token, which would break the CLI's login. static func refreshAfter401() async throws -> String { guard let record = try currentRecord() else { throw StoreError.noRefreshToken } - let updated = try await refreshAndPersist(record: record) - return updated.accessToken + if let live = adoptFresherSource(than: record) { + return live.accessToken + } + throw StoreError.sourceTokenStale + } + + /// Re-reads Claude's own store (file, then keychain with a no-UI query) and + /// adopts it when it holds a different access token than `record` — i.e. the + /// CLI rotated since we last read. Returns nil when nothing fresher exists. + private static func adoptFresherSource(than record: CredentialRecord) -> CredentialRecord? { + guard let live = readClaudeSourceSilently(), live.accessToken != record.accessToken else { + return nil + } + cacheInMemory(live) + try? writeOurCache(record: live) + return live + } + + private static func readClaudeSourceSilently() -> CredentialRecord? { + if let fromFile = try? readClaudeFile() { return fromFile } + if let fromKeychain = try? readClaudeKeychain(allowUI: false) { return fromKeychain } + return nil } static func subscriptionTier() throws -> String? { @@ -182,7 +186,7 @@ enum ClaudeCredentialStore { private static func readClaudeSource() throws -> CredentialRecord { if let fromFile = try? readClaudeFile() { return fromFile } - if let fromKeychain = try readClaudeKeychain() { return fromKeychain } + if let fromKeychain = try readClaudeKeychain(allowUI: true) { return fromKeychain } throw StoreError.bootstrapNoSource } @@ -201,14 +205,14 @@ enum ClaudeCredentialStore { /// often returns the older stale one. We try the user-keyed entry first /// (the modern format), then fall back to the unscoped query for older /// installations. - private static func readClaudeKeychain() throws -> CredentialRecord? { - if let record = try readClaudeKeychain(account: NSUserName()) { + private static func readClaudeKeychain(allowUI: Bool) throws -> CredentialRecord? { + if let record = try readClaudeKeychain(account: NSUserName(), allowUI: allowUI) { return record } - return try readClaudeKeychain(account: nil) + return try readClaudeKeychain(account: nil, allowUI: allowUI) } - private static func readClaudeKeychain(account: String?) throws -> CredentialRecord? { + private static func readClaudeKeychain(account: String?, allowUI: Bool) throws -> CredentialRecord? { var query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: claudeKeychainService, @@ -216,9 +220,20 @@ enum ClaudeCredentialStore { kSecReturnData as String: true, ] if let account { query[kSecAttrAccount as String] = account } + if !allowUI { + // Background refresh cycles must never raise a keychain prompt. Fail + // the read instead. Relies on the user having granted "Always Allow" + // on the one-time bootstrap prompt. + let context = LAContext() + context.interactionNotAllowed = true + query[kSecUseAuthenticationContext as String] = context + } var result: CFTypeRef? let status = SecItemCopyMatching(query as CFDictionary, &result) if status == errSecItemNotFound { return nil } + // Silent read that would need interaction: treat as "no fresher token + // available", not an error. The caller falls back to the cached token. + if !allowUI, status == errSecInteractionNotAllowed { return nil } guard status == errSecSuccess, let data = result as? Data else { throw StoreError.keychainReadFailed(status) } @@ -301,79 +316,6 @@ enum ClaudeCredentialStore { private static func cacheInMemory(_ record: CredentialRecord) { lock.withLock { memoryCache = CachedRecord(record: record, cachedAt: Date()) } } - - // MARK: - Refresh - - private static func refreshAndPersist(record: CredentialRecord) async throws -> CredentialRecord { - guard let refreshToken = record.refreshToken, !refreshToken.isEmpty else { - throw StoreError.noRefreshToken - } - - var request = URLRequest(url: refreshURL) - request.httpMethod = "POST" - request.timeoutInterval = 30 - request.setValue("application/x-www-form-urlencoded", forHTTPHeaderField: "Content-Type") - request.setValue("application/json", forHTTPHeaderField: "Accept") - var components = URLComponents() - components.queryItems = [ - URLQueryItem(name: "grant_type", value: "refresh_token"), - URLQueryItem(name: "refresh_token", value: refreshToken), - URLQueryItem(name: "client_id", value: oauthClientID), - ] - request.httpBody = (components.percentEncodedQuery ?? "").data(using: .utf8) - - let data: Data - let response: URLResponse - do { - (data, response) = try await URLSession.shared.data(for: request) - } catch { - throw StoreError.refreshNetworkError(error) - } - guard let http = response as? HTTPURLResponse else { - throw StoreError.refreshHTTPError(-1, nil) - } - guard http.statusCode == 200 else { - let body = String(data: data, encoding: .utf8) - throw StoreError.refreshHTTPError(http.statusCode, body) - } - - struct RefreshResponse: Decodable { - let accessToken: String - let refreshToken: String? - let expiresIn: Int? - enum CodingKeys: String, CodingKey { - case accessToken = "access_token" - case refreshToken = "refresh_token" - case expiresIn = "expires_in" - } - } - guard let decoded = try? JSONDecoder().decode(RefreshResponse.self, from: data) else { - throw StoreError.refreshDecodeFailed - } - - // Anthropic may rotate the refresh token. If it did, the OLD one is - // already invalid server-side — discarding the new one would lock - // the user out permanently. So we cache the new record in memory - // BEFORE attempting the keychain write, and if the write fails we - // still return the new record (memory cache will serve subsequent - // calls inside the 5-min TTL while we keep retrying the persist). - let updated = CredentialRecord( - accessToken: decoded.accessToken, - refreshToken: decoded.refreshToken ?? record.refreshToken, - expiresAt: decoded.expiresIn.map { Date().addingTimeInterval(TimeInterval($0)) } ?? record.expiresAt, - rateLimitTier: record.rateLimitTier - ) - cacheInMemory(updated) - do { - try writeOurCache(record: updated) - } catch { - // Best effort — surface to logs but do not abandon the rotated - // token. Next refresh will retry persistence; UI will continue - // working from the in-memory cache. - NSLog("CodeBurn: cache write failed during refresh rotation: %@", String(describing: error)) - } - return updated - } } private extension NSLock { diff --git a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift index ba9a4c14..9f7ae18f 100644 --- a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift @@ -10,7 +10,11 @@ import Security enum CodexCredentialStore { private static let bootstrapCompletedKey = "codeburn.codex.bootstrapCompleted" private static let inMemoryTTL: TimeInterval = 5 * 60 - private static let proactiveRefreshMargin: TimeInterval = 5 * 60 + // Codex refresh tokens are single-use and rotate on every refresh. The CLI + // owns the grant via ~/.codex/auth.json; we only refresh ourselves when its + // last_refresh is older than this, mirroring the Codex CLI's own cadence. + // Refreshing more eagerly races the CLI and burns its rotating token. + private static let staleRefreshInterval: TimeInterval = 8 * 24 * 60 * 60 private static let oauthClientID = "app_EMoamEEZ73f0CkXaXp7hrann" private static let refreshURL = URL(string: "https://auth.openai.com/oauth/token")! @@ -35,6 +39,7 @@ enum CodexCredentialStore { let idToken: String? let accountId: String? let expiresAt: Date? + let lastRefresh: Date? } enum StoreError: Error, LocalizedError { @@ -115,6 +120,15 @@ enum CodexCredentialStore { static func currentRecord() throws -> CredentialRecord? { guard isBootstrapCompleted else { return nil } + // The Codex CLI's auth.json is the source of truth. Read it fresh each + // call so we always serve the CLI's current token rather than racing it + // with a stale private copy. Our cache is only a fallback for when the + // file is briefly unreadable. + if let live = try? readCodexAuth() { + cacheInMemory(live) + try? writeOurCache(record: live) + return live + } if let cached = lock.withLock({ memoryCache }), cached.isFresh { return cached.record } @@ -122,25 +136,37 @@ enum CodexCredentialStore { cacheInMemory(stored) return stored } - isBootstrapCompleted = false return nil } static func freshAccessToken() async throws -> String? { guard let record = try currentRecord() else { return nil } - if let expiresAt = record.expiresAt, expiresAt.timeIntervalSinceNow < proactiveRefreshMargin { + if needsRefresh(record) { let updated = try await refreshAndPersist(record: record) return updated.accessToken } return record.accessToken } - static func refreshAfter401() async throws -> String { + static func refreshAfter401(failedToken: String) async throws -> String { + // Source of truth first: the CLI may have already rotated the token out + // from under us. Re-read auth.json before spending our single-use refresh + // token, which would race the CLI and can invalidate its login. + if let live = try? readCodexAuth(), live.accessToken != failedToken { + cacheInMemory(live) + try? writeOurCache(record: live) + return live.accessToken + } guard let record = try currentRecord() else { throw StoreError.noRefreshToken } let updated = try await refreshAndPersist(record: record) return updated.accessToken } + private static func needsRefresh(_ record: CredentialRecord) -> Bool { + guard let last = record.lastRefresh else { return true } + return Date().timeIntervalSince(last) > staleRefreshInterval + } + // MARK: - Bootstrap source: ~/.codex/auth.json private static func readCodexAuth() throws -> CredentialRecord { @@ -152,6 +178,7 @@ enum CodexCredentialStore { struct Root: Decodable { let auth_mode: String? let tokens: Tokens? + let last_refresh: String? } struct Tokens: Decodable { let access_token: String? @@ -179,7 +206,8 @@ enum CodexCredentialStore { refreshToken: refresh, idToken: tokens.id_token, accountId: tokens.account_id, - expiresAt: nil // Codex CLI does not record expiresAt in auth.json + expiresAt: nil, // Codex CLI does not record expiresAt in auth.json + lastRefresh: root.last_refresh.flatMap(parseISO8601) ) } catch let err as StoreError { throw err @@ -188,6 +216,43 @@ enum CodexCredentialStore { } } + private static func parseISO8601(_ s: String) -> Date? { + // auth.json records fractional seconds (e.g. ...:12.010758Z); the plain + // ISO8601 formatter rejects those, so try the fractional variant first. + let withFraction = ISO8601DateFormatter() + withFraction.formatOptions = [.withInternetDateTime, .withFractionalSeconds] + if let d = withFraction.date(from: s) { return d } + return ISO8601DateFormatter().date(from: s) + } + + // MARK: - Write rotated tokens back to ~/.codex/auth.json + + /// Atomic read-modify-write of auth.json that preserves every other top-level + /// key (OPENAI_API_KEY, auth_mode, ...) and only rewrites the tokens dict and + /// last_refresh. Keeps the CLI and the menubar on the same rotated grant. + private static func writeBackToCodexAuth(record: CredentialRecord) { + let url = FileManager.default.homeDirectoryForCurrentUser.appendingPathComponent(codexAuthPath) + var json: [String: Any] = [:] + if let data = try? SafeFile.read(from: url.path, maxBytes: maxCredentialBytes), + let existing = try? JSONSerialization.jsonObject(with: data) as? [String: Any] { + json = existing + } + var tokens: [String: Any] = [ + "access_token": record.accessToken, + "refresh_token": record.refreshToken, + ] + if let idToken = record.idToken { tokens["id_token"] = idToken } + if let accountId = record.accountId { tokens["account_id"] = accountId } + json["tokens"] = tokens + let stamp = ISO8601DateFormatter() + stamp.formatOptions = [.withInternetDateTime, .withFractionalSeconds] + json["last_refresh"] = stamp.string(from: record.lastRefresh ?? Date()) + guard let out = try? JSONSerialization.data(withJSONObject: json, options: [.prettyPrinted, .sortedKeys]) else { + return + } + try? out.write(to: url, options: .atomic) + } + // MARK: - Local cache file private static func cacheFileURL() -> URL { @@ -253,6 +318,16 @@ enum CodexCredentialStore { throw StoreError.refreshHTTPError(-1, nil) } guard http.statusCode == 200 else { + // A 4xx here usually means the CLI already rotated the shared grant + // out from under us (single-use refresh token). Re-read the source: + // if it now holds a different token, the CLI healed it and we adopt + // that instead of surfacing a terminal "disconnected". + if http.statusCode >= 400, http.statusCode < 500, + let live = try? readCodexAuth(), live.refreshToken != record.refreshToken { + cacheInMemory(live) + try? writeOurCache(record: live) + return live + } let body = String(data: data, encoding: .utf8) throw StoreError.refreshHTTPError(http.statusCode, body) } @@ -272,9 +347,13 @@ enum CodexCredentialStore { refreshToken: decoded.refresh_token ?? record.refreshToken, idToken: decoded.id_token ?? record.idToken, accountId: record.accountId, - expiresAt: decoded.expires_in.map { Date().addingTimeInterval(TimeInterval($0)) } ?? record.expiresAt + expiresAt: decoded.expires_in.map { Date().addingTimeInterval(TimeInterval($0)) } ?? record.expiresAt, + lastRefresh: Date() ) cacheInMemory(updated) + // Write the rotated grant back to the CLI's store first so the CLI keeps + // working, then mirror it into our fallback cache. + writeBackToCodexAuth(record: updated) do { try writeOurCache(record: updated) } catch { diff --git a/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift b/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift index ac3bd940..e83bd32e 100644 --- a/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift +++ b/mac/Sources/CodeBurnMenubar/Data/CodexSubscriptionService.swift @@ -122,7 +122,7 @@ enum CodexSubscriptionService { } case 401: if allowOne401Recovery { - let newToken = try await CodexCredentialStore.refreshAfter401() + let newToken = try await CodexCredentialStore.refreshAfter401(failedToken: token) return try await fetchWithToken(newToken, allowOne401Recovery: false) } throw FetchError.usageHTTPError(401, String(data: data, encoding: .utf8))