feat(synthesizer): propagate cloak config from OAuth JSON to auth attributes#1939
feat(synthesizer): propagate cloak config from OAuth JSON to auth attributes#1939lichengzhe wants to merge 2 commits intorouter-for-me:mainfrom
Conversation
… auth attributes Currently, cloak configuration (mode, strict-mode, sensitive-words, cache-user-id) can only be set via `claude-api-key` entries in config.yaml. This works for static API keys but not for OAuth accounts whose access_token rotates every ~15 minutes, making key-based matching impossible. This change reads cloak_* fields from OAuth JSON metadata and copies them into auth.Attributes, enabling the existing getCloakConfigFromAuth fallback path to pick them up. Usage — add to OAuth JSON file: "cloak_cache_user_id": "true" "cloak_mode": "full" "cloak_strict_mode": "true" "cloak_sensitive_words": "API,proxy,openai" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the system's authentication mechanism by allowing users leveraging OAuth tokens to configure cloaking settings directly within their OAuth JSON metadata. Previously, such configurations were limited to static API keys, making it impossible for OAuth accounts with frequently rotating access tokens to utilize these features. The change bridges this gap, providing greater flexibility and control over user-specific cloaking behavior for OAuth users. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively enables per-account cloak configuration for OAuth tokens by propagating cloak_* fields from the OAuth JSON metadata to auth.Attributes. The implementation is clear and the new tests provide solid validation for the changes. I have a couple of suggestions to improve maintainability and test code clarity.
internal/watcher/synthesizer/file.go
Outdated
| for _, cloakKey := range []string{ | ||
| "cloak_mode", "cloak_strict_mode", | ||
| "cloak_sensitive_words", "cloak_cache_user_id", | ||
| } { |
There was a problem hiding this comment.
For better performance and maintainability, this slice of cloak keys should be defined as a package-level variable instead of being re-created on every call to synthesizeFileAuths. This avoids repeated memory allocations.
For example, you can define this at the package level:
var cloakAuthKeys = []string{
"cloak_mode", "cloak_strict_mode",
"cloak_sensitive_words", "cloak_cache_user_id",
}And then use cloakAuthKeys in the loop here.
| cloakKeys := []string{ | ||
| "cloak_mode", "cloak_strict_mode", | ||
| "cloak_sensitive_words", "cloak_cache_user_id", | ||
| } | ||
| for _, key := range cloakKeys { | ||
| got, gotOK := auths[0].Attributes[key] | ||
| want, wantOK := tt.wantAttrs[key] | ||
| if wantOK { | ||
| if !gotOK { | ||
| t.Errorf("expected attribute %q=%q, but not set", key, want) | ||
| } else if got != want { | ||
| t.Errorf("attribute %q: expected %q, got %q", key, want, got) | ||
| } | ||
| } else if gotOK { | ||
| t.Errorf("attribute %q should not be set, got %q", key, got) | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for asserting cloak attributes is correct but could be simplified by using reflect.DeepEqual, which is common practice in Go tests for comparing maps or structs. This would make the test easier to read and maintain. You will need to import the reflect package.
Also, the cloakKeys slice is duplicated from file.go. If you define it as a package-level variable as suggested in the other comment, you could reuse it here.
gotCloakAttrs := make(map[string]string)
// This slice is also used in synthesizeFileAuths and could be a shared package-level variable.
cloakKeys := []string{
"cloak_mode", "cloak_strict_mode",
"cloak_sensitive_words", "cloak_cache_user_id",
}
for _, key := range cloakKeys {
if val, ok := auths[0].Attributes[key]; ok {
gotCloakAttrs[key] = val
}
}
if !reflect.DeepEqual(tt.wantAttrs, gotCloakAttrs) {
t.Errorf("cloak attributes mismatch:\n got: %v\n want: %v", gotCloakAttrs, tt.wantAttrs)
}…t assertions Address Gemini Code Assist review feedback: - Move cloak key slice to package-level var to avoid per-call allocation - Simplify test assertions using reflect.DeepEqual - Reuse cloakAuthKeys in tests instead of duplicating the slice Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
luispater
left a comment
luispater
left a comment
There was a problem hiding this comment.
Summary: Thanks — the approach is nicely scoped and reusing the existing getCloakConfigFromAuth path makes sense. I found one correctness gap before this is safe to merge.
Key findings:
- Blocking: the new propagation logic only accepts string-valued cloak_* fields from OAuth JSON. Native JSON forms like "cloak_cache_user_id": true, "cloak_strict_mode": true, or "cloak_sensitive_words": ["API","proxy"] are silently ignored, even though the canonical CloakConfig model uses bool / []string types. Because this feature exposes cloak settings through JSON, I think the synthesizer should normalize those native JSON shapes into attribute strings (or reject them explicitly instead of silently no-op'ing).
- Non-blocking: the added tests verify attribute synthesis, but there is still no executor-level regression proving that an OAuth auth with propagated cloak_* attributes actually changes the cloaking path.
Test plan:
- Reviewed the diff plus the existing cloak fallback and executor tests.
- PR CI is green (build, ensure-no-translator-changes).
2 participants
Summary
cloak_*fields from OAuth JSON metadata toauth.Attributes, enabling per-account cloak configuration for OAuth tokensclaude-api-keyentries in config.yaml, which requires matching by static API key — impossible for OAuth accounts whoseaccess_tokenrotates every ~15 minutesgetCloakConfigFromAuthfallback pathMotivation
Users who exclusively use OAuth tokens (Claude Max subscriptions) cannot configure
cache-user-id,sensitive-words, orstrict-modebecause:CloakConfigis only attached toClaudeKeyin config.yamlresolveClaudeKeyCloakConfig()matches by API key, but OAuthaccess_tokenrotates every 15 mingetCloakConfigFromAuth()reads fromauth.Attributes, but the file synthesizer never populated cloak fields thereUsage
Add cloak fields to the OAuth JSON file:
{ "type": "claude", "email": "user@example.com", "cloak_cache_user_id": "true", "cloak_mode": "full", "cloak_strict_mode": "true", "cloak_sensitive_words": "API,proxy,openai" }All fields are optional. Empty or whitespace-only values are ignored.
Test plan
TestFileSynthesizer_Synthesize_CloakAttributeswith 5 sub-tests:cache-user-id🤖 Generated with Claude Code