Fix Claude background Keychain prompts#2191
Conversation
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 10:20 AM ET / 14:20 UTC. Summary Reproducibility: yes. from source inspection: the experimental reader can produce an effective Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Use the stored Claude prompt preference at every opaque child-process gate, preserve explicit-user and stored- Do we have a high-confidence way to reproduce the issue? Yes from source inspection: the experimental reader can produce an effective Is this the best way to solve the issue? No, not at the current head; the fail-closed boundary is maintainable, but both child-process checks must use the stored policy and the end-to-end recovery path still needs real proof. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 6d71af30b84d. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fbdf6e89c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| guard !KeychainAccessGate.isDisabled, | ||
| ClaudeOAuthKeychainPromptPreference.current() == .always |
There was a problem hiding this comment.
In background Auto refresh with the experimental securityCLIExperimental reader selected, ClaudeOAuthKeychainPromptPreference.current() returns the effective mode .always even when the stored user preference is still the default .onlyOnUserAction. That lets this guard pass and then runs ClaudeCLIAuthStatusProbe.isLoggedIn (claude auth status --json) in the background, reintroducing the opaque Claude CLI Keychain path this change is trying to block unless the user explicitly opted into always allowing prompts. Use the stored prompt mode for this child-process boundary, as the delegated refresh path now does.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: Fix Claude background Keychain prompts This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ac078d6a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if configuration.interaction == .background, | ||
| configuration.keychainAccessDisabled || configuration.promptMode != .always |
There was a problem hiding this comment.
If KeychainAccessGate.isDisabled is true but the expired OAuth repair is triggered by a user action, this guard does not return because it is scoped to background interactions only. The coordinator then continues to touchOAuthAuthPath and launches claude /status, which can invoke /usr/bin/security, so the global “Disable Keychain access” setting is bypassed for explicit refresh/OAuth-source repairs.
Useful? React with 👍 / 👎.
| return false | ||
| } | ||
| guard ClaudeOAuthKeychainPromptPreference.effectiveMode(readStrategy: readStrategy) != .never else { | ||
| let promptMode = ClaudeOAuthKeychainPromptPreference.effectiveMode(readStrategy: readStrategy) |
There was a problem hiding this comment.
When the experimental security CLI reader is selected, effectiveMode(readStrategy:) maps the stored .onlyOnUserAction or .never preference to .always, so this MCP-only check still launches /usr/bin/security during background expired-credential handling before delegated refresh is suppressed. This leaves a background Keychain prompt path open for users on securityCLIExperimental; use the stored prompt mode for this child-process boundary as well.
Useful? React with 👍 / 👎.
|
Maintainer verification after merge:
|
2 participants
Summary
Always allow Keychain promptspolicy:claude auth status --jsonpreflight/statusrefresh/usr/bin/securityaccessRoot cause
CodexBar had two background Claude CLI paths: the
claude auth status --jsonpreflight and the delegated OAuth-expiry/statusrefresh. Both can invoke/usr/bin/securityoutside CodexBar's in-process Keychain prompt policy. That allowed recurringClaude Code-credentialsdialogs during background refreshes even when prompts were disabled. Separately, a Security.framework MCP-only discovery path could read the same item from background work despite the user-action-only policy.This patch treats child-process Keychain behavior as opaque: background Auto refresh does not start it unless the user explicitly opts into background Keychain access. Explicit refresh and explicit CLI use remain available.
Fixes #2115.
Live affected-Mac proof (redacted)
Tested on a real macOS account with Claude Code 2.1.207 installed, a live OAuth login, and the default restrictive prompt policy (
onlyOnUserAction). The opt-in proof exercises production code, not a process stub:The background assertion returns
.skippedByPromptPolicybefore either opaque Claude CLI//usr/bin/securityboundary can launch. The explicit-user assertion invokes the real installedclaude auth status --jsonpath and correctly reports the existing OAuth login. No email, token, Keychain item, or account identifier is collected or printed. CI leaves this live suite disabled unlessLIVE_CLAUDE_KEYCHAIN_PROOF=1is explicitly set.Validation
LIVE_CLAUDE_KEYCHAIN_PROOF=1 swift test --disable-sandbox --scratch-path /private/tmp/codexbar-pr2191-proof --filter ClaudeKeychainLiveProofTests— 2 real macOS tests passedswift test --filter 'ClaudeBaselineCharacterizationTests|ClaudeOAuthCredentialsStoreMCPOnlyGuardTests|ClaudeCLIAuthStatusProbeTests|ClaudeOAuthDelegatedRefreshCoordinatorTests|ClaudeOAuthFetchStrategyAvailabilityTests'— 48 tests passedgit diff --check— cleanmake check— repository checks and formatting complete; SwiftLint reported then I fixed one new line-length warning. The check's final result is otherwise blocked by its existing sandbox-generated plist write behavior.