Fix Claude plugin auth compatibility across custom OAuth and keychain setups#331
Fix Claude plugin auth compatibility across custom OAuth and keychain setups#331robinebers merged 3 commits intomainfrom
Conversation
🤖 Augment PR SummarySummary: This PR improves Claude plugin authentication compatibility across custom config locations and multiple OAuth environments, and strengthens host-side redaction/logging around those auth flows. Changes:
Technical Notes: Live usage fetch is skipped for “inference-only” env-injected tokens, but 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| accessToken = refreshed | ||
| } else { | ||
| ctx.host.log.warn("proactive refresh failed, trying with existing token") | ||
| const homePath = getClaudeHomeOverride(ctx) |
There was a problem hiding this comment.
homePath is taken directly from CLAUDE_CONFIG_DIR and passed through to ccusage, but the Rust ccusage wrapper sets the env var without ~ expansion (unlike host.fs.* paths). If a user sets CLAUDE_CONFIG_DIR=~/.claude-custom, credential reads may work while ccusage queries silently point at a non-existent directory.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| let old = std::env::var(name).ok(); | ||
| let _restore = RestoreEnvVar { name, old }; | ||
| // SAFETY: this test restores the previous value in `Drop`. | ||
| unsafe { std::env::set_var(name, "openusage-test-user") }; |
There was a problem hiding this comment.
This test mutates the process-wide USER env var and assumes env changes are “serialized”, but Rust tests run in parallel by default so this can be flaky and/or leak into other tests. If any other test reads USER concurrently, results can become nondeterministic.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Improves Claude plugin authentication compatibility across custom config directories and multiple OAuth environments, while expanding redaction coverage and adding regression tests.
Changes:
- Added support for
CLAUDE_CONFIG_DIR, env-injected OAuth tokens, and staging/local/custom OAuth routing in the Claude plugin. - Updated Rust host API to broaden env allowlisting, improve macOS keychain access/logging, and redact additional IDs/paths in logs/bodies.
- Added JS + Rust tests for the new Claude auth flows, ccusage home overrides, and redaction behavior.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src-tauri/src/plugin_engine/host_api.rs | Expands env allowlist, adds keychain account scoping + logging, ccusage home expansion, and broader redaction. |
| src-tauri/src/lib.rs | Redacts app_data_dir logging output while preserving a non-sensitive tail. |
| plugins/claude/plugin.js | Adds CLAUDE_CONFIG_DIR support, OAuth env routing, env token handling, scope expansion, and usage-fetch gating. |
| plugins/claude/plugin.test.js | Adds regression tests for new credential discovery, keychain service suffixes, env token behavior, and refresh scope. |
| .gitignore | Ignores a new docs directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let account = current_macos_keychain_account(); | ||
| let args = keychain_find_generic_password_args(&service, &account); | ||
| let redacted_account = redact_value(&account); | ||
| log::info!( | ||
| "[plugin:{}] keychain read: service={}, account={}", | ||
| pid_read, | ||
| service, | ||
| redacted_account | ||
| ); | ||
| let output = std::process::Command::new("security") | ||
| .args(["find-generic-password", "-s", &service, "-w"]) | ||
| .args(&args) | ||
| .output() |
There was a problem hiding this comment.
readGenericPassword now always queries the keychain with both -s <service> and -a <current account>. Any existing items that were previously created without -a (or with a different account value) will not be found anymore, which can silently break credential discovery after upgrade. Consider falling back to a service-only lookup when the account-scoped lookup misses (and/or migrating the item to the account-scoped form once found) so existing installs continue to work.
| if let Ok(path_re) = | ||
| regex_lite::Regex::new(r#"(/(?:Users|home|opt|private|var|tmp|Applications)/[^\s"')]+)"#) | ||
| { |
There was a problem hiding this comment.
The path redaction regex only matches Unix-style absolute paths starting with /Users, /home, /opt, etc. On Windows, common sensitive paths like C:\Users\... or UNC paths (\\server\share\...) will not be redacted and may still leak via logs. Consider adding Windows/UNC path patterns (and tests) so redaction is effective cross-platform.
| if let Ok(path_re) = | |
| regex_lite::Regex::new(r#"(/(?:Users|home|opt|private|var|tmp|Applications)/[^\s"')]+)"#) | |
| { | |
| if let Ok(path_re) = regex_lite::Regex::new( | |
| r#"((?:/(?:Users|home|opt|private|var|tmp|Applications)|[A-Za-z]:\\|\\\\)[^\s"')]+)"#, | |
| ) { |
| if (Array.isArray(scopes) && scopes.length > 0) { | ||
| return scopes.indexOf("user:profile") !== -1 | ||
| } | ||
| return true |
There was a problem hiding this comment.
hasProfileScope returns true when creds.oauth.scopes is an empty array, which effectively treats an explicitly-specified “no scopes” token as having user:profile and will attempt the live usage call. If the intent is to skip live usage whenever user:profile is missing, this should return scopes.includes("user:profile") for any array (including empty).
| if (Array.isArray(scopes) && scopes.length > 0) { | |
| return scopes.indexOf("user:profile") !== -1 | |
| } | |
| return true | |
| if (!Array.isArray(scopes)) { | |
| return false | |
| } | |
| return scopes.includes("user:profile") |
![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: cc1d1806e1
ℹ️ 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".
| OsString::from("-a"), | ||
| OsString::from(account), | ||
| OsString::from("-s"), |
There was a problem hiding this comment.
This change makes keychain reads require an exact account match (-a <current user>), but existing items created before this change may not match that account, so credential discovery can regress to "not logged in" after upgrade. In this codepath, a miss is treated as absent credentials rather than retried with a service-only lookup, so affected users lose persisted auth until they re-login or manually migrate keychain items. Add a fallback find-generic-password -s <service> -w (or migration) when account-scoped lookup fails.
Useful? React with 👍 / 👎.
| if let Ok(path_re) = | ||
| regex_lite::Regex::new(r#"(/(?:Users|home|opt|private|var|tmp|Applications)/[^\s"')]+)"#) | ||
| { |
There was a problem hiding this comment.
The new path redaction regex stops at whitespace, so common macOS paths like /Users/.../Library/Application Support/... are only partially masked (e.g., leaving Support/... in logs). Since this commit now logs redacted app_data_dir on startup, the partial match still leaks path fragments in routine logs. Update the pattern to consume full quoted/unquoted path segments including spaces.
Useful? React with 👍 / 👎.
robinebers
force-pushed
the
fix/claude-plugin-auth-compat
branch
from
d8a3187 to
110bee6
Compare
|
for the record, I've added two new host APIs that read keychain items for the specific user. We should seek to deprecate the entire host if possible in the future, but more research for every plugin is required. |
Summary
ccusage, and include theuser:file_uploadscopeTesting
Note
Medium Risk
Touches credential loading/refresh, keychain access, and environment-variable handling; mistakes could break login/usage probing or leak sensitive values if redaction misses edge cases.
Overview
Improves Claude plugin authentication portability by supporting custom Claude config directories (
CLAUDE_CONFIG_DIR), env-injected access tokens (CLAUDE_CODE_OAUTH_TOKEN, treated as inference-only), and dynamic OAuth endpoints/client IDs (staging/local/custom URLs) with per-environment keychain service names.The plugin now prefers a macOS current-user keychain entry with legacy fallback, writes refreshed tokens back to the same source, passes an optional home path through to
ccusage, skips live/api/oauth/usagecalls when tokens lackuser:profile, and refreshes with the expanded scope includinguser:file_upload.On the Tauri/Rust side, the host API expands the env var allowlist for Claude auth inputs, adds current-user keychain read/write bindings, expands redaction to cover filesystem paths and additional IDs, and redacts
app_data_dir/ccusagelogging. Tests were updated/added across JS and Rust to cover the new keychain/OAuth/redaction behaviors.Reviewed by Cursor Bugbot for commit 110bee6. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Improves Claude plugin auth to work across custom config dirs and custom/staging/local OAuth setups, with current-user keychain support, env-injected tokens, and stronger redaction.
New Features
CLAUDE_CONFIG_DIR; pass expanded home path toccusage.CLAUDE_CODE_OAUTH_TOKENfor inference-only sessions; skip live usage when missinguser:profile.user:file_uploadin OAuth refresh scope.Bug Fixes
readGenericPasswordForCurrentUser/writeGenericPasswordForCurrentUser.CLAUDE_CONFIG_DIR,CLAUDE_CODE_OAUTH_TOKEN,USER_TYPE,USE_STAGING_OAUTH,USE_LOCAL_OAUTH,CLAUDE_CODE_CUSTOM_OAUTH_URL,CLAUDE_CODE_OAUTH_CLIENT_ID,CLAUDE_LOCAL_OAUTH_API_BASE).app_data_dirandccusagepaths masked as [PATH].ccusagehome and improved 401 retry handling. Added tests for keychain current-user/fallback, service suffixes, custom home, env tokens, scopes, usage-skip, and redaction.Written for commit 110bee6. Summary will update on new commits.