Skip to content

fix: increase max buffer for reading keychain dump#201

Merged
griffinmartin merged 2 commits intogriffinmartin:mainfrom
robbash:increase-keychain-dump-buffer-size
Apr 30, 2026
Merged

fix: increase max buffer for reading keychain dump#201
griffinmartin merged 2 commits intogriffinmartin:mainfrom
robbash:increase-keychain-dump-buffer-size

Conversation

@robbash
Copy link
Copy Markdown
Contributor

@robbash robbash commented Apr 15, 2026

Summary

In a multi-account scenario, listing the keychain services failed silently. The account selection was not available. Increasing the max buffer on that command fixes the issue.

Related issue

none

Testing

  • set up multiple Claude Code accounts
  • built the plugin locally and copied it to ~/.config/opencode/plugins
  • ran opencode auth login to verify it's working

Checklist

  • PR title follows Conventional Commits (feat:, fix:, docs:, chore:, etc.)
  • make all passes locally (runs lint, build, and test)
  • Tests added or updated where applicable
  • README or docs updated where applicable

Copy link
Copy Markdown

@bvironn bvironn left a comment

Choose a reason for hiding this comment

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

Buffer bump is correct — execSync defaults to a 1MB stdout buffer, which a populated keychain on a long-lived dev machine can exceed. 10MB is a sensible ceiling.

One nit: typo in the new log entry — "Failed to list keychein services" should be "keychain" (src/keychain.ts:168).

Otherwise LGTM. 213/213 tests pass.

Address review feedback from @bvironn:

- Fix typo: 'keychein' -> 'keychain' in the catch block log entry.
- Bind the caught error and include its message in the log payload so
  the failure is no longer just a count - matches the diagnostic style
  used elsewhere in this file (readKeychainEntry).
@griffinmartin
Copy link
Copy Markdown
Owner

Pushed b754910 to address @bvironn's feedback:

  • Fixed typo: keycheinkeychain in the catch-block log entry.
  • Bound the caught error and included err.message in the log payload, matching the diagnostic style used by readKeychainEntry earlier in the same file. Now when the buffer or another security failure mode occurs, we'll have the actual error to look at instead of just a counter.

Verified locally: make all clean (lint + build + 213/213 tests pass).

@griffinmartin griffinmartin merged commit 2f97161 into griffinmartin:main Apr 30, 2026
4 checks passed
griffinmartin pushed a commit that referenced this pull request Apr 30, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.5.3](v1.5.2...v1.5.3)
(2026-04-30)


### Bug Fixes

* increase max buffer for reading keychain dump
([#201](#201))
([2f97161](2f97161))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
griffinmartin added a commit to jstruv-bot/opencode-claude-auth that referenced this pull request Apr 30, 2026
Resolve conflicts and apply two follow-up fixes from the regression
analysis on Windows code paths.

Conflicts resolved:

- README.md: env-var override table — both branches added a row.
  Kept the new OPENCODE_CLAUDE_AUTH_MAX_RETRY_MS row from main and
  re-added the CLAUDE_CODE_OAUTH_TOKEN row from this PR after it,
  rewidened to the wider column shape main introduced.
- src/keychain.ts: auto-merged. PR griffinmartin#201 (in main) edited the catch
  block of listClaudeKeychainServices to capture the error; this
  PR added readEnvToken further down in the file. Both changes
  preserved cleanly.

Follow-up fixes for issues found in the Windows regression analysis:

1. src/index.ts (authorize): the sourceDescription used a 2-arm
   conditional that bucketed every non-'file' source as 'macOS
   Keychain'. With this PR's new 'env' source, a Windows user
   authenticated via Claude Desktop's CLAUDE_CODE_OAUTH_TOKEN would
   see 'credentials loaded from macOS Keychain' which is wrong on
   every axis. Added an 'env' arm that says
   'CLAUDE_CODE_OAUTH_TOKEN environment variable'.

2. src/credentials.ts (syncAuthJson): the sync loop re-threw on the
   first per-path failure, aborting subsequent paths and tearing
   down the unhandled callers (plugin init at index.ts, account
   switch authorize). With this PR adding a third Windows path,
   the failure surface is 50% larger; an antivirus-locked or
   network-share-borked Roaming auth.json could now break plugin
   startup. Each path is independent — failures are now logged
   per-path and the loop continues.

3. src/credentials.test.ts: regression test for fix griffinmartin#2.
   Pre-creates a directory at the XDG target path so writeFileSync
   fails with EISDIR, asserts syncAuthJson does not throw.
@robbash
Copy link
Copy Markdown
Contributor Author

robbash commented Apr 30, 2026

Sorry I missed your feedback. Thanks for fixing...

@robbash robbash deleted the increase-keychain-dump-buffer-size branch April 30, 2026 04:03
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