Skip to content

fix(test): stub macOS Keychain in TestReadClaudeCodeCredentials#16498

Closed
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/anthropic-credentials-keychain-bypass-tests
Closed

fix(test): stub macOS Keychain in TestReadClaudeCodeCredentials#16498
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/anthropic-credentials-keychain-bypass-tests

Conversation

@briandevans

@briandevans briandevans commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • read_claude_code_credentials() checks the macOS Keychain before falling back to the JSON credentials file
  • On any developer machine with Claude Code installed, the Keychain holds a live sk-ant-oat01-* token
  • All 5 TestReadClaudeCodeCredentials tests fail on such machines: the 4 tests expecting None get the real credential, and the positive test reads the Keychain token instead of its fixture value
  • Fix: add an autouse fixture to the class that stubs _read_claude_code_credentials_from_keychain to return None, keeping all tests isolated from the host environment

Root cause

def read_claude_code_credentials():
    kc_creds = _read_claude_code_credentials_from_keychain()  # ← returns real creds on macOS
    if kc_creds:
        return kc_creds  # ← never reaches Path.home() patch
    ...

The tests only patched agent.anthropic_adapter.Path.home; they did not account for the Keychain path that short-circuits first.

Test plan

  • pytest tests/agent/test_anthropic_adapter.py::TestReadClaudeCodeCredentials — all 5 pass (was 5 failures on macOS with Claude Code installed)
  • Confirmed failures on unpatched main then green after fix
  • No production code changed — test isolation fix only

🤖 Generated with Claude Code

@briandevans @claude
fix(test): stub macOS Keychain in TestReadClaudeCodeCredentials
0b647f1 
read_claude_code_credentials() checks the macOS Keychain before falling
back to the JSON file. On machines with Claude Code installed the
Keychain lookup returns a real token, causing all 5 tests to fail:
3 that expect None get a live credential, and the one that checks the
file path gets the Keychain token instead of the fixture value.

Add an autouse fixture that stubs _read_claude_code_credentials_from_keychain
to return None, keeping tests isolated from the host environment.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 27, 2026 10:37
Copilot AI reviewed Apr 27, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 improves test isolation for read_claude_code_credentials() by preventing TestReadClaudeCodeCredentials from reading live macOS Keychain credentials on developer machines, ensuring the tests reliably exercise the JSON credentials-file code path.

Changes:

  • Add an autouse pytest fixture to stub _read_claude_code_credentials_from_keychain() to None for all tests in TestReadClaudeCodeCredentials.

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

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/agent Core agent loop, run_agent.py, prompt builder area/auth Authentication, OAuth, credential pools provider/anthropic Anthropic native Messages API labels Apr 27, 2026
@briandevans @claude
fix(test): extend keychain stub to TestResolveAnthropicToken and Test…
1d0f7fc 
…RunOauthSetupToken

The same macOS Keychain bypass that broke TestReadClaudeCodeCredentials
also affects resolve_anthropic_token() and run_oauth_setup_token() which
both call read_claude_code_credentials() internally. Add the same
autouse _no_keychain fixture to both classes so all 10 affected tests
are isolated from the host environment.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@briandevans

briandevans commented Apr 27, 2026

Copy link
Copy Markdown
Contributor Author

Extended the fix in commit 1d0f7fcd: same macOS Keychain bypass also affects TestResolveAnthropicToken and TestRunOauthSetupToken (both call read_claude_code_credentials() transitively). Added identical _no_keychain autouse fixtures to both classes. All 137 tests in the file now pass.

@alt-glitch

alt-glitch commented Apr 27, 2026

Copy link
Copy Markdown
Collaborator

Likely duplicate of #15958 — same fix: stub _read_claude_code_credentials_from_keychain in TestReadClaudeCodeCredentials. #15958 also covers botocore skip tests.

@briandevans

briandevans commented Apr 27, 2026

Copy link
Copy Markdown
Contributor Author

Closing — superseded by our own #15958, which is a strictly more comprehensive fix:

This PR (#16498) #15958
TestReadClaudeCodeCredentials stub ✅ ✅
TestResolveAnthropicToken stub ✅ ✅
TestRunOauthSetupToken stub ✅ ✅
botocore soft-dep test isolation ❌ ✅

#15958 covers all three keychain stubs plus the botocore fixture. No reason to keep both open — closing this one in favour of the more complete PR.

@nodejun nodejun mentioned this pull request May 7, 2026
Open
2 tasks
@alt-glitch alt-glitch mentioned this pull request May 15, 2026
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

area/auth Authentication, OAuth, credential pools comp/agent Core agent loop, run_agent.py, prompt builder P3 Low — cosmetic, nice to have provider/anthropic Anthropic native Messages API type/test Test coverage or test infrastructure

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@briandevans @alt-glitch