Skip to content

fix: respect CLAUDE_CONFIG_DIR for credential lookup#59

Open
that-lucas wants to merge 5 commits intogriffinmartin:mainfrom
that-lucas:fix/claude-config-dir-auth
Open

fix: respect CLAUDE_CONFIG_DIR for credential lookup#59
that-lucas wants to merge 5 commits intogriffinmartin:mainfrom
that-lucas:fix/claude-config-dir-auth

Conversation

@that-lucas
Copy link
Copy Markdown

@that-lucas that-lucas commented Mar 21, 2026

Summary

  • derive the Claude credential file path from CLAUDE_CONFIG_DIR instead of always assuming ~/.claude
  • derive the macOS keychain service name from the configured Claude directory so hashed service names are found correctly
  • keep the fix scoped to credential lookup so authenticated Claude installs with custom config dirs work in OpenCode

Issue

The plugin currently assumes Claude Code always stores credentials in the legacy default locations:

  • macOS keychain service: Claude Code-credentials
  • file fallback: ~/.claude/.credentials.json
    That breaks when Claude is configured with a custom CLAUDE_CONFIG_DIR.
    In that setup, Claude derives both credential locations from the configured directory instead:
  • the file fallback becomes <CLAUDE_CONFIG_DIR>/.credentials.json
  • the macOS keychain service becomes Claude Code-credentials-<sha256(configDir).slice(0,8)>
    Because the plugin only checks the old defaults, it can fail to find valid credentials even when claude itself is fully authenticated. In practice, this shows up as:
  • security: SecKeychainSearchCopyNext: The specified item could not be found in the keychain.
  • opencode-claude-auth: No Claude Code credentials found. Plugin disabled.

Solution

Update credential lookup to follow Claude Code's actual storage rules:

  • resolve the credentials file from process.env.CLAUDE_CONFIG_DIR ?? ~/.claude
  • on macOS, resolve the keychain service name using the same hashing scheme Claude uses when CLAUDE_CONFIG_DIR is set
  • preserve existing behavior for default installs that still use ~/.claude and the un-hashed Claude Code-credentials service
    This keeps the change narrowly scoped while making the plugin work for both default and relocated Claude config directories.

Verification

  • npm test
  • npm run build
  • node --input-type=module --experimental-strip-types -e "import { readClaudeCredentials } from './src/keychain.ts'; const creds = readClaudeCredentials(); console.log(JSON.stringify({ found: Boolean(creds), expiresAtType: creds ? typeof creds.expiresAt : null }))"

Copilot AI review requested due to automatic review settings March 21, 2026 03:14
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates Claude credential discovery so OpenCode can locate credentials when Claude is configured to use a non-default configuration directory via CLAUDE_CONFIG_DIR, including macOS Keychain service name derivation.

Changes:

  • Derive the credential file path from CLAUDE_CONFIG_DIR instead of always using ~/.claude.
  • Derive the macOS Keychain service name suffix from the configured Claude directory (hash-based) to match Claude’s hashed service naming.
  • Factor credential path/service-name derivation into helper functions for reuse.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@that-lucas that-lucas marked this pull request as draft March 21, 2026 03:27
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@that-lucas that-lucas marked this pull request as ready for review March 21, 2026 04:02
@griffinmartin
Copy link
Copy Markdown
Owner

I can take a look at this in the morning. TY for the contribution.

Copy link
Copy Markdown
Owner

@griffinmartin griffinmartin left a comment

Choose a reason for hiding this comment

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

Review

Two items worth addressing before merge:

1. Hash computation may silently diverge from Claude Code's scheme

keychain.ts:33-36 — The implementation hashes the NFC-normalized config dir string, but doesn't resolve the path (trailing slashes, symlinks, .. segments). If a user sets CLAUDE_CONFIG_DIR=/foo/bar/ (trailing slash) and Claude Code strips it before hashing, the service names won't match and keychain lookup silently fails — which is exactly the class of bug this PR is fixing.

Worth verifying against Claude Code's source whether it does any path resolution (e.g., path.resolve(), realpath(), trailing-slash stripping) before hashing.

2. Tests don't actually verify NFC normalization

keychain.test.ts:100 — The hash assertion test computes its expected hash from dir (raw, un-normalized), while the implementation hashes getClaudeConfigDir() (NFC-normalized). For ASCII test paths these are identical, so the test is tautological — it would still pass if NFC normalization were accidentally removed. A test with a path containing combining characters (e.g., "café" as \u0063\u0061\u0066\u0065\u0301 vs NFC \u0063\u0061\u0066\u00e9) would actually exercise that code path.

@that-lucas
Copy link
Copy Markdown
Author

Review

Two items worth addressing before merge:

1. Hash computation may silently diverge from Claude Code's scheme

keychain.ts:33-36 — The implementation hashes the NFC-normalized config dir string, but doesn't resolve the path (trailing slashes, symlinks, .. segments). If a user sets CLAUDE_CONFIG_DIR=/foo/bar/ (trailing slash) and Claude Code strips it before hashing, the service names won't match and keychain lookup silently fails — which is exactly the class of bug this PR is fixing.

Worth verifying against Claude Code's source whether it does any path resolution (e.g., path.resolve(), realpath(), trailing-slash stripping) before hashing.

2. Tests don't actually verify NFC normalization

keychain.test.ts:100 — The hash assertion test computes its expected hash from dir (raw, un-normalized), while the implementation hashes getClaudeConfigDir() (NFC-normalized). For ASCII test paths these are identical, so the test is tautological — it would still pass if NFC normalization were accidentally removed. A test with a path containing combining characters (e.g., "café" as \u0063\u0061\u0066\u0065\u0301 vs NFC \u0063\u0061\u0066\u00e9) would actually exercise that code path.

Addressed in 547754c.

For the first point, I investigated against local Claude Code 2.1.80 and the current keychain entry format. The installed binary appears to derive the config dir as (process.env.CLAUDE_CONFIG_DIR ?? ~/.claude).normalize("NFC") and then hash that string directly for the Claude Code-credentials-<hash> service name. I also verified my local keychain entry matches sha256("/Users/lucas/.agents")[:8], so I’m leaving src/keychain.ts unchanged because it already matches Claude Code’s current behavior.

For the second point, I added a regression test in src/keychain.test.ts that uses a decomposed Unicode path (cafe\u0301) and asserts the service hash is derived from the NFC-normalized form, so the normalization path is now actually covered.

Let me know if that's enough.

@griffinmartin
Copy link
Copy Markdown
Owner

@that-lucas is this still relevant with the new keychain support for multiple accounts?

@that-lucas
Copy link
Copy Markdown
Author

@that-lucas is this still relevant with the new keychain support for multiple accounts?

@griffinmartin, it's unrelated. This is to get your plugin working for people that use the CLAUDE_CONFIG_DIR env var to change their Claude home dir from the default ~/.claude to something else, e.g., ~/.agents.

A simple use of that is to get all your tools (think Codex, OpenCode, Claude Code) to use the same home dir to share skills across all tools.

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.

3 participants