Fix identity resolution for ServiceAccount tokens behind OAuth proxy#1401
Draft
guzalv wants to merge 1 commit intoambient-code:mainfrom
Draft
Fix identity resolution for ServiceAccount tokens behind OAuth proxy#1401guzalv wants to merge 1 commit intoambient-code:mainfrom
guzalv wants to merge 1 commit intoambient-code:mainfrom
Conversation
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
When a request uses a K8s ServiceAccount token (API key), the OAuth proxy sets X-Forwarded-User to the SA subject (e.g. "system:serviceaccount:ns:sa-name"). The middleware sanitized this into a synthetic userID, but the fallback that reads the SA's created-by-user-id annotation was guarded by `userID == ""` — which was never true because the proxy already set it. This caused credential RBAC failures for API key-authenticated requests (mobile apps, CLI tools) because the synthetic SA identity didn't match the session owner's identity, so the runner couldn't fetch GitHub/Jira/Google credentials. Fix: also trigger the annotation lookup when X-Forwarded-User is a ServiceAccount subject, overriding the synthetic ID with the real creating user's identity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
f70ab1f to
1002f2b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
forwardedIdentityMiddlewareto resolve the creating user's identity whenX-Forwarded-Useris a ServiceAccount subjectProblem
When a mobile app or CLI tool uses a K8s ServiceAccount JWT (created via "Access Keys"), the OAuth proxy sets
X-Forwarded-Userto the SA subject:system:serviceaccount:stackrox-1:ambient-key-test-write-2b3908aa.The middleware sanitizes this into
system-serviceaccount-stackrox-1-ambient-key-test-write-2b3908aaand sets it asuserID. The SA annotation fallback (created-by-user-id) was guarded byuserID == "", which was never true — so the creating user's identity was never resolved.This caused a cascade of failures:
X-Current-User-IDto the runnerX-Runner-Current-Userheader)PermissionError→RUN_ERROR→ agent run abortedThe web UI doesn't hit this because the OAuth proxy sets
X-Forwarded-Userto the human username (e.g.userid), not a SA subject.Fix
When
X-Forwarded-Userstarts withsystem:serviceaccount:, also check the SA'sambient-code.io/created-by-user-idannotation and use the real human identity. This is the same annotation already set by the access key creation endpoint (permissions.go:420).Test plan
X-Forwarded-Uservalues are unchanged)🤖 Generated with Claude Code