Fix Claude plugin auth compatibility across custom OAuth and keychain setups#331
Fix Claude plugin auth compatibility across custom OAuth and keychain setups#331robinebers wants to merge 2 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 👍 / 👎.
Summary
ccusage, and include theuser:file_uploadscopeTesting
Note
Medium Risk
Touches OAuth credential discovery/refresh and macOS keychain read/write behavior, which can break authentication or usage reporting across environments if misconfigured. Adds broader env passthrough and more logging/redaction logic, increasing configuration surface area but with regression tests to reduce risk.
Overview
Improves the Claude plugin’s auth compatibility by supporting
CLAUDE_CONFIG_DIRfor credential file lookup andccusagehome overrides, env-injected access tokens viaCLAUDE_CODE_OAUTH_TOKEN(treated as inference-only and skips live/api/oauth/usage), and expanded refresh scopes to includeuser:file_upload.Adds flexible OAuth routing for local/staging/custom endpoints and client ID overrides, and namespaces keychain service names with an OAuth suffix to avoid collisions. On the Rust side, expands the host env allowlist for these Claude variables, fixes macOS keychain access to always target the current account (and logs hits/misses), and strengthens redaction to cover additional IDs and filesystem paths (including redacting
app_data_dirandccusagerunner paths).Written by Cursor Bugbot for commit cc1d180. This will update automatically on new commits. Configure here.
Summary by cubic
Fixes Claude plugin auth across custom config dirs and custom/staging/local OAuth, with account-scoped keychain and broader redaction. Adds env token support, includes the
user:file_uploadscope, and skips live usage when tokens lackuser:profile.New Features
CLAUDE_CONFIG_DIR; pass and expand home path to ccusage.CLAUDE_CODE_OAUTH_TOKENfor inference-only sessions; skip live usage when missinguser:profile.user:file_uploadin OAuth refresh scope.Bug Fixes
CLAUDE_CONFIG_DIR,USE_LOCAL_OAUTH,CLAUDE_LOCAL_OAUTH_API_BASE,CLAUDE_CODE_OAUTH_CLIENT_ID); sensitive values redacted.app_data_dirand ccusage paths masked as [PATH].Written for commit cc1d180. Summary will update on new commits.