-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix Claude background Keychain prompts #2191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
86c3f18
47120d4
1bc27d6
1a6d6c6
e9a6242
d686b4b
0fbdf6e
03b47f8
2ac078d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ public enum ClaudeOAuthDelegatedRefreshCoordinator { | |
|
|
||
| public enum Outcome: Sendable, Equatable { | ||
| case skippedByCooldown | ||
| case skippedByPromptPolicy | ||
| case cliUnavailable | ||
| case attemptedSucceeded | ||
| case attemptedFailed(String) | ||
|
|
@@ -59,7 +60,7 @@ public enum ClaudeOAuthDelegatedRefreshCoordinator { | |
| let outcome = await task.value | ||
| self.clearInFlightTaskIfStillCurrent(id: id, state: state) | ||
| switch outcome { | ||
| case .attemptedFailed, .skippedByCooldown, .cliUnavailable: | ||
| case .attemptedFailed, .skippedByCooldown, .skippedByPromptPolicy, .cliUnavailable: | ||
| return await self.attempt(now: now, timeout: timeout, environment: environment) | ||
| case .attemptedSucceeded: | ||
| return outcome | ||
|
|
@@ -81,6 +82,7 @@ public enum ClaudeOAuthDelegatedRefreshCoordinator { | |
| let environment: [String: String] | ||
| let interaction: ProviderInteraction | ||
| let readStrategy: ClaudeOAuthKeychainReadStrategy | ||
| let promptMode: ClaudeOAuthKeychainPromptMode | ||
| let keychainAccessDisabled: Bool | ||
| #if DEBUG | ||
| let cliAvailableOverride: Bool? | ||
|
|
@@ -113,20 +115,28 @@ public enum ClaudeOAuthDelegatedRefreshCoordinator { | |
| let attemptID = state.nextAttemptID | ||
| // Detached to avoid inheriting the caller's executor context (e.g. MainActor) and cancellation state. | ||
| #if DEBUG | ||
| let readStrategy = ClaudeOAuthKeychainReadStrategyPreference.current() | ||
| let configuration = AttemptConfiguration( | ||
| environment: environment, | ||
| interaction: interaction, | ||
| readStrategy: ClaudeOAuthKeychainReadStrategyPreference.current(), | ||
| readStrategy: readStrategy, | ||
| // The delegated Claude process is an opaque Keychain boundary. Its policy must come | ||
| // from the user's stored preference, not the strategy-adjusted mode used by our own reads. | ||
| promptMode: ClaudeOAuthKeychainPromptPreference.storedMode(), | ||
| keychainAccessDisabled: KeychainAccessGate.isDisabled, | ||
| cliAvailableOverride: self.cliAvailableOverrideForTesting, | ||
| touchAuthPathOverride: self.touchAuthPathOverrideForTesting, | ||
| keychainFingerprintOverride: self.keychainFingerprintOverrideForTesting) | ||
| let securityCLIReadOverride = ClaudeOAuthCredentialsStore.currentSecurityCLIReadOverrideForTesting() | ||
| #else | ||
| let readStrategy = ClaudeOAuthKeychainReadStrategyPreference.current() | ||
| let configuration = AttemptConfiguration( | ||
| environment: environment, | ||
| interaction: interaction, | ||
| readStrategy: ClaudeOAuthKeychainReadStrategyPreference.current(), | ||
| readStrategy: readStrategy, | ||
| // The delegated Claude process is an opaque Keychain boundary. Its policy must come | ||
| // from the user's stored preference, not the strategy-adjusted mode used by our own reads. | ||
| promptMode: ClaudeOAuthKeychainPromptPreference.storedMode(), | ||
| keychainAccessDisabled: KeychainAccessGate.isDisabled) | ||
| #endif | ||
| let task = Task.detached(priority: .utility) { | ||
|
|
@@ -158,6 +168,16 @@ public enum ClaudeOAuthDelegatedRefreshCoordinator { | |
| configuration: AttemptConfiguration, | ||
| state: AttemptStateStorage) async -> Outcome | ||
| { | ||
| // `/status` is an opaque Claude CLI invocation and may launch `/usr/bin/security` outside | ||
| // CodexBar's own no-UI query controls. Background work may not cross that boundary unless | ||
| // the user explicitly opted into always allowing Keychain access. | ||
| if configuration.interaction == .background, | ||
| configuration.keychainAccessDisabled || configuration.promptMode != .always | ||
|
Comment on lines
+174
to
+175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If Useful? React with 👍 / 👎. |
||
| { | ||
| self.log.info("Claude OAuth delegated refresh skipped by Keychain prompt policy") | ||
| return .skippedByPromptPolicy | ||
| } | ||
|
|
||
| guard self.isClaudeCLIAvailable(environment: configuration.environment, configuration: configuration) else { | ||
| self.log.info("Claude OAuth delegated refresh skipped: claude CLI unavailable") | ||
| return .cliUnavailable | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -328,6 +328,14 @@ struct ClaudeOAuthFetchStrategy: ProviderFetchStrategy { | |
| guard sourceMode == .auto else { return true } | ||
| guard claudeCLIAvailable else { return false } | ||
| guard ProviderInteractionContext.current == .background else { return true } | ||
| // An expired Claude CLI credential requires the delegated Claude CLI refresh path. | ||
| // That child process can access Keychain outside CodexBar's no-UI controls, so do | ||
| // not plan it during background Auto refresh without an explicit opt-in. | ||
| guard !KeychainAccessGate.isDisabled, | ||
| ClaudeOAuthKeychainPromptPreference.current() == .always | ||
| else { | ||
| return false | ||
| } | ||
| return !Self.hasMcpOAuthOnlyClaudeKeychainPayload(environment: environment) | ||
| case .environment: | ||
| return sourceMode != .auto | ||
|
|
@@ -641,12 +649,23 @@ struct ClaudeCLIFetchStrategy: ProviderFetchStrategy { | |
| let hasWebFallback: Bool | ||
|
|
||
| func isAvailable(_ context: ProviderFetchContext) async -> Bool { | ||
| // The interactive Claude REPL can open browser OAuth when it starts logged out. CLI runtime and background | ||
| // app Auto refreshes must establish authentication through the non-interactive status command first. | ||
| let requiresAuthPreflight = context.runtime == .cli || ( | ||
| context.runtime == .app && | ||
| context.sourceMode == .auto && | ||
| ProviderInteractionContext.current == .background) | ||
| // Claude's "auth status" command is a child process that may invoke /usr/bin/security itself. A no-prompt | ||
| // policy in CodexBar cannot constrain that child process, so background Auto refresh must not launch it | ||
| // unless the user explicitly opted into Keychain access for background work. | ||
| let isBackgroundAutoRefresh = context.runtime == .app | ||
| && context.sourceMode == .auto | ||
| && ProviderInteractionContext.current == .background | ||
| if isBackgroundAutoRefresh { | ||
| guard !KeychainAccessGate.isDisabled, | ||
| ClaudeOAuthKeychainPromptPreference.current() == .always | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In background Auto refresh with the experimental Useful? React with 👍 / 👎. |
||
| else { | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| // The interactive Claude REPL can open browser OAuth when it starts logged out. CLI runtime and the | ||
| // explicitly opted-in background Auto path establish authentication through the status command first. | ||
| let requiresAuthPreflight = context.runtime == .cli || isBackgroundAutoRefresh | ||
| guard requiresAuthPreflight else { return true } | ||
| guard let binary = ClaudeCLIResolver.resolvedBinaryPath(environment: context.env) else { return false } | ||
| return await ClaudeCLIAuthStatusProbe.isLoggedIn(binary: binary, environment: context.env) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the experimental security CLI reader is selected,
effectiveMode(readStrategy:)maps the stored.onlyOnUserActionor.neverpreference to.always, so this MCP-only check still launches/usr/bin/securityduring background expired-credential handling before delegated refresh is suppressed. This leaves a background Keychain prompt path open for users onsecurityCLIExperimental; use the stored prompt mode for this child-process boundary as well.Useful? React with 👍 / 👎.