Skip to content

fix: scope the Keychain lookup to the current account - #30

Closed
mike-ildan wants to merge 1 commit into
dotCipher:mainfrom
mike-ildan:fix/keychain-account-scope
Closed

mike-ildan wants to merge 1 commit into
dotCipher:mainfrom
mike-ildan:fix/keychain-account-scope

Conversation

@mike-ildan

Copy link
Copy Markdown
Contributor

Fixes #29.

readClaudeCredentials() looks the Keychain item up by service name with no account, and security returns the first match. Attaching an MCP server writes a second item under the same service with acct="unknown" holding only mcpOAuth, so the bridge can read that one, find no claudeAiOauth, and fall through to opencode's own OAuth entry. When that entry is stale the result is invalid_grant on every new session while the user's real credential is fine. Re-authenticating does not fix it, which makes it painful to diagnose.

Three changes:

  • Look up scoped to userInfo().username first, then fall back to the existing unscoped lookup, so nobody whose item lives under a different account name regresses.
  • Select on content rather than match order: an item with no claudeAiOauth is not the credential. Pure exported function, so it is testable without a Keychain.
  • When matches exist but none carry claudeAiOauth, fall through to ~/.claude/.credentials.json. The old code returned the contentless item, which made that fallback unreachable. This is the one behavior change, and it can only turn a previously-null result into a working one.

Also switches to execFileSync with an argument array, since the account name is user-controlled and should not be interpolated into a shell command line.

Verification

  • Full suite: 114 pass, 0 fail. Adds four tests for the selection logic; there was no keychain coverage before.
  • npx tsc --noEmit clean.
  • On a machine carrying both items, readClaudeCredentials() returns keys ["mcpOAuth"] with claudeAiOauth: false before this change, and ["claudeAiOauth","mcpOAuth"] with claudeAiOauth: true after.

Independent of #27 / #28; the two touch different files and can land in either order.

readClaudeCredentials() runs, with no account specified:

    security find-generic-password -s "Claude Code-credentials" -w

More than one Keychain item can share that service name. Attaching an MCP
server in an OpenCode session writes a second item under it, with
acct="unknown", holding only mcpOAuth. `security` returns the first match, so
the bridge can read that item, find no claudeAiOauth, and have getClaudeTokens
return null.

It then falls through to opencode's own OAuth entry, and when that entry is
stale the user sees "Token refresh failed (400): invalid_grant" on every new
session while their actual Claude credential is valid the entire time. Neither
re-authenticating nor `claude login` fixes it, because neither touches the item
being read.

Observed on a machine carrying both items:

    unscoped lookup          -> acct="unknown", keys ["mcpOAuth"]
    scoped to the account    -> keys ["claudeAiOauth", "mcpOAuth"]

Three changes:

- Look the item up scoped to userInfo().username first, then fall back to the
  historical unscoped lookup, so anyone whose item lives under a different
  account name keeps working exactly as before.
- Select on content rather than on match order. A returned item that carries no
  claudeAiOauth is not the credential, whichever account it belongs to. The
  selection is a pure exported function so it can be tested without a Keychain.
- When the Keychain produced matches but none carried claudeAiOauth, fall
  through to ~/.claude/.credentials.json instead of returning that item. The
  old code returned it, which made the file fallback below it unreachable.

Also switches to execFileSync with an argument array. The account name is
user-controlled and should not be interpolated into a shell command line, and
the array form removes the need for the embedded 2>/dev/null redirection.

Adds four unit tests for the selection logic; there was no keychain coverage.

Verified: full suite 114 pass / 0 fail. On a machine with both items,
readClaudeCredentials() returns the mcpOAuth-only item before this change and
the real credential after it.

Signed-off-by: Mike Hiltz <mike@ildan.ai>

@dotCipher dotCipher left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and independently verified the Keychain account-scoping fix. removes shell interpolation, the selection helper prevents MCP-only credentials from shadowing Claude OAuth credentials, and the credential-file fallback remains available. I ran
added 3 packages, and audited 4 packages in 381ms

found 0 vulnerabilities,

opencode-claude-bridge@1.10.11 typecheck
tsc --noEmit,
opencode-claude-bridge@1.10.11 test
npm run build && node --test dist/index.test.js

opencode-claude-bridge@1.10.11 build
tsc

▶ thinking injection
✔ does NOT inject thinking for claude-sonnet-4-5 (0.466541ms)
✔ does NOT inject thinking for claude-haiku-4-5-20251001 (0.0575ms)
✔ does NOT inject thinking for an unknown model (0.158625ms)
✔ injects adaptive thinking for claude-sonnet-4-6 (0.35925ms)
✔ injects adaptive thinking for claude-opus-4-5 (0.066875ms)
✔ injects adaptive thinking for claude-opus-4-6 (0.043959ms)
✔ does not overwrite an existing thinking block (0.042583ms)
✔ thinking injection (1.734458ms)
▶ temperature coercion
✔ forces temperature to 1 when adaptive thinking is injected (0.442708ms)
✔ forces temperature to 1 when thinking:enabled is already set (0.070792ms)
✔ does not touch temperature when thinking is not injected (sonnet-4-5) (0.071916ms)
✔ does not touch temperature when it is already 1 (0.045833ms)
✔ does not touch temperature when it is absent (0.048209ms)
✔ temperature coercion (0.805375ms)
▶ OAuth error parsing
✔ extracts nested error messages from token responses (0.061833ms)
✔ falls back to raw response bodies when parsing fails (0.044208ms)
✔ OAuth error parsing (0.139708ms)
▶ Claude assistant prefill stripping
✔ strips the synthetic continue prefill so Anthropic requests end with a user message (0.069458ms)
✔ keeps non-synthetic assistant messages intact (0.029917ms)
✔ Claude assistant prefill stripping (0.13125ms)
▶ system cache control
✔ strips cache_control from system blocks (0.051083ms)
✔ system cache control (0.070167ms)
▶ tool schema selection
✔ uses Claude schemas for native Anthropic requests (0.069833ms)
✔ uses Claude schemas for Claude-family models on OpenRouter (0.022042ms)
✔ keeps default schemas for non-Claude models on OpenRouter (0.021209ms)
✔ keeps default schemas for non-Claude targets (0.020834ms)
✔ injects Claude tool schemas only when active tools are present (0.032334ms)
✔ does not inject Claude tool schemas for tool-less summary or compaction requests (0.027541ms)
✔ filters Claude tool schemas to tools active in the OpenCode request (5.832625ms)
✔ does not advertise WebSearch when only a custom websearch_cited tool is active (0.066375ms)
✔ preserves active MCP and custom OpenCode tools (0.089083ms)
✔ preserves MCP tools that have no description (0.051708ms)
✔ preserves MCP tools whose names look like prefixed core tools (0.048709ms)
✔ preserves custom tools whose names collide with Claude core tool names (0.041875ms)
✔ maps inbound Claude core names only for active OpenCode core tools (0.045834ms)
✔ maps outbound OpenCode core history but preserves MCP-prefixed names (0.024541ms)
✔ does not advertise AskUserQuestion when OpenCode did not enable question (0.041292ms)
✔ tool schema selection (6.5555ms)
▶ translateToolArgsJsonString
✔ renames file_path → filePath for Read (0.138625ms)
✔ renames all Edit params (0.030167ms)
✔ renames glob → include for Grep and preserves other keys (0.034417ms)
✔ renames skill → name for Skill and strips args (OpenCode skill has no args param) (0.042375ms)
✔ translates activeForm → priority INSIDE TodoWrite todos[] (0.040792ms)
✔ does NOT rewrite activeForm when it appears only inside a string value (Linus case) (0.040209ms)
✔ does NOT rewrite file_path when it appears inside a Bash command string (0.028041ms)
✔ maps Agent subagent_type values (0.035875ms)
✔ leaves an unknown Agent subagent_type untouched (0.019958ms)
✔ strips prompt param for WebFetch and injects default format (0.030167ms)
✔ respects explicit WebFetch format if already set (0.025084ms)
✔ returns input unchanged for malformed JSON (0.032875ms)
✔ returns input unchanged for non-object JSON (array or primitive) (0.020625ms)
✔ passes through unknown tool names without modification (0.027041ms)
✔ strips Claude-only Agent fields (model, run_in_background, isolation) (0.028167ms)
✔ defaults Agent subagent_type to 'general' when missing (OpenCode requires it) (0.02325ms)
✔ strips Claude-only Bash fields (run_in_background, dangerouslyDisableSandbox) (0.023ms)
✔ strips Claude-only Read field (pages) (0.026375ms)
✔ strips Claude-only Grep fields (0.0445ms)
✔ translates AskUserQuestion multiSelect → multiple per question (0.040542ms)
✔ parses stringified AskUserQuestion questions and translates multiSelect (0.049ms)
✔ translateToolArgsJsonString (0.922791ms)
▶ translateArgsOpencodeToClaude
✔ renames camelCase keys to snake_case for Edit (0.104916ms)
✔ maps OpenCode subagent_type values back to Claude (0.022917ms)
✔ strips Agent OpenCode-only fields (task_id, command) (0.024ms)
✔ translates AskUserQuestion multiple → multiSelect per question (0.033875ms)
✔ synthesizes a WebFetch prompt from format (markdown) (0.021709ms)
✔ synthesizes a WebFetch prompt from format (text) (0.018541ms)
✔ synthesizes a WebFetch prompt from format (html) (0.0265ms)
✔ strips WebFetch timeout (OpenCode-only) (0.020041ms)
✔ renames priority → activeForm in TodoWrite and collapses cancelled → completed (0.028542ms)
✔ maps plan_enter → EnterPlanMode with empty args (outbound) (0.022125ms)
✔ maps plan_exit → ExitPlanMode preserving allowedPrompts (outbound) (0.022375ms)
✔ is the inverse of translateToolArgsJsonString for the Edit round trip (0.033208ms)
✔ translateArgsOpencodeToClaude (0.478542ms)
▶ Agent type maps
✔ CLAUDE_TO_OPENCODE covers Claude's subagent_type values (0.02875ms)
✔ OPENCODE_TO_CLAUDE covers OpenCode's built-in agents (0.019ms)
✔ Agent type maps (0.070708ms)
▶ SSE processor: tool name mapping
✔ maps Bash → bash on content_block_start (0.350625ms)
✔ maps Read → read on content_block_start (0.055333ms)
✔ maps Glob → glob on content_block_start (0.034834ms)
✔ maps Grep → grep on content_block_start (0.040166ms)
✔ maps Edit → edit on content_block_start (0.04825ms)
✔ maps Write → write on content_block_start (0.036458ms)
✔ maps Agent → task on content_block_start (0.037625ms)
✔ maps WebFetch → webfetch on content_block_start (0.040167ms)
✔ maps TodoWrite → todowrite on content_block_start (0.026917ms)
✔ maps Skill → skill on content_block_start (0.027333ms)
✔ maps AskUserQuestion → question on content_block_start (0.024417ms)
✔ maps EnterPlanMode → plan_enter on content_block_start (0.022709ms)
✔ maps ExitPlanMode → plan_exit on content_block_start (0.021708ms)
✔ passes through an unknown tool name without modification (0.038ms)
✔ SSE processor: tool name mapping (0.91675ms)
▶ SSE processor: argument translation
✔ translates file_path → filePath for Read (0.074417ms)
✔ translates all Edit params (0.035458ms)
✔ translates glob → include for Grep, preserves pattern (0.037708ms)
✔ translates activeForm → priority per todo item in TodoWrite (0.045583ms)
✔ translates Agent subagent_type values (0.040125ms)
✔ strips prompt and injects default format for WebFetch (0.037042ms)
✔ translates skill → name and strips args for Skill (0.041167ms)
✔ leaves Bash args untouched (0.029875ms)
✔ maps EnterPlanMode → plan_enter with empty args (0.039334ms)
✔ maps ExitPlanMode → plan_exit preserving allowedPrompts (0.038708ms)
✔ SSE processor: argument translation (0.505292ms)
▶ SSE processor: chunk boundary handling
✔ handles args split across many small fragments (0.067917ms)
✔ handles multiple SSE events concatenated into one chunk (0.071916ms)
✔ handles an SSE event split across two chunks (0.034708ms)
✔ passes through text deltas unchanged (0.027958ms)
✔ does NOT translate tool args inside a text_delta (0.025125ms)
✔ SSE processor: chunk boundary handling (0.2675ms)
▶ SSE processor: interleaved and concurrent tool_use blocks
✔ keeps per-block state isolated across interleaved deltas (0.070333ms)
✔ SSE processor: interleaved and concurrent tool_use blocks (0.084167ms)
▶ SSE processor: error handling
✔ calls debug callback on malformed JSON in an SSE data frame (0.066ms)
✔ calls debug callback when translateToolArgs throws (0.072292ms)
✔ SSE processor: error handling (0.15875ms)
▶ SSE processor: flush
✔ flush returns any trailing buffered bytes (0.461416ms)
✔ logs a debug warning when a tool_use block is abandoned mid-stream (0.053709ms)
✔ SSE processor: flush (0.534083ms)
▶ SSE processor: pass-through optimization
✔ emits the exact input bytes for ping events (no reserialize) (0.040083ms)
✔ emits the exact input bytes for message_start events (0.024792ms)
✔ emits the exact input bytes for text_delta events (0.028417ms)
✔ still transforms tool_use content_block_start (optimization does not skip interesting events) (0.032667ms)
✔ SSE processor: pass-through optimization (0.159ms)
▶ parseSseEvent / buildSseEvent round trip
✔ round-trips a simple event (0.032291ms)
✔ returns null for a frame with no data line (0.016666ms)
✔ handles a data-only frame (no event line) (0.017041ms)
✔ parseSseEvent / buildSseEvent round trip (0.090167ms)
▶ deriveModelDisplayName
✔ parses claude-opus-4-7 → Opus 4.7 (0.159791ms)
✔ parses claude-sonnet-4-6 → Sonnet 4.6 (0.045167ms)
✔ ignores a trailing date suffix (claude-haiku-4-5-20251001 → Haiku 4.5) (0.018958ms)
✔ falls back to the raw id when the convention doesn't match (0.026ms)
✔ deriveModelDisplayName (0.289541ms)
▶ rewriteSystemBlocksForModel
✔ rewrites the identity line to match the requested model (0.10975ms)
✔ leaves blocks untouched when no model id is provided (0.020333ms)
✔ handles a dated variant without re-including the date suffix in the display name (0.025708ms)
✔ preserves non-text blocks and non-matching text (0.025583ms)
✔ rewriteSystemBlocksForModel (0.211375ms)
ℹ tests 116
ℹ suites 19
ℹ pass 116
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 162.109084 (114 passing), and ; all passed.

@dotCipher dotCipher left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and independently verified the Keychain account-scoping fix. execFileSync removes shell interpolation, the selection helper prevents MCP-only credentials from shadowing Claude OAuth credentials, and the credential-file fallback remains available. I ran npm ci, npm run typecheck, npm test (114 passing), and git diff --check; all passed.

@dotCipher

Copy link
Copy Markdown
Owner

Closing as superseded by merged PR #31. #31 carries this reviewed Keychain credential-selection fix after the necessary rebase onto #19.

@dotCipher dotCipher closed this Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keychain lookup is unscoped, so an MCP OAuth item can shadow the real credential

2 participants