feat: add service account id to auth cache#210
Merged
alvinkam2001 merged 3 commits intomainfrom Apr 30, 2026
Merged
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
declan-scale
approved these changes
Apr 27, 2026
Collaborator
declan-scale
left a comment
There was a problem hiding this comment.
Just update the readme and should be good
| ```yaml | ||
| auth: | ||
| principal: | ||
| user_id: "my-dev-cluster-user-id" # SGP user UUID |
Collaborator
There was a problem hiding this comment.
Let's use this comment from before (in the open-source case don't use SGP): # Unique identifier for the user who is deploying the agent
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
Adds
service_account_idto the principal-identity field list in_create_authorization_cache_key.Without this, two SA principals in the same account on the same resource would hash to the same authz cache key — one SA's cached permission decision could be served to another SA's request. With per-principal grants supported in
agentex_permissions, that's a real correctness risk for SA flows.Also documents the new
user_idvsservice_account_idmutual exclusion inconfiguration.md.Related PRs (ship together)
scaleapi/scale-agentex#210Test Plan
AuthenticationCache— adding one for this single line would establish a new convention for what's effectively a 1-character change.ruff check+ruff format --checkclean.Greptile Summary
This PR fixes a correctness bug in
_create_authorization_cache_keywhere two service-account principals sharing the sameaccount_id,agent_id, and other fields would produce an identical cache key, causing one SA's cached authorization decision to be served to a different SA's request. The fix addsservice_account_idto the field-extraction list, making per-SA grants correctly isolated. The accompanyingconfiguration.mdupdate clearly documents theuser_id/service_account_idmutual-exclusion contract.Confidence Score: 5/5
Safe to merge — the change is a minimal, clearly correct addition with no regressions.
The code change is a single field added to an existing field-extraction list; logic is straightforward and directly addresses the described correctness bug. No new control flow, no data-loss risk, no security issues. All remaining observations are P2 at most.
No files require special attention.
Important Files Changed
service_account_idto the principal field-extraction list in_create_authorization_cache_key; correctly fixes cache-key collision between distinct SA principals.service_account_idfield and its mutual exclusion withuser_id; adds a YAML example and updated best-practice bullets.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Authorization check request\nresource_type, resource_selector, operation, principal_context] --> B[_create_authorization_cache_key] B --> C{principal_context\npresent?} C -- No --> D[empty principal_key_data] C -- Yes --> E{has __dict__?} E -- Yes --> F[use __dict__] E -- No --> G{is dict?} G -- Yes --> H[use as-is] G -- No --> I[wrap as value string] F & H & I --> J[Extract fields:\nuser_id\nservice_account_id NEW\naccount_id\nagent_id\nid / sub / email] J --> K{any fields\nfound?} K -- Yes --> L[principal_key_data = extracted fields] K -- No --> M[principal_key_data = hash of full context] L & M & D --> N[cache_data dict\nresource_type + resource_selector\n+ operation + principal] N --> O[SHA-256 hash → authz: key] O --> P{cache hit?} P -- Yes --> Q[Return cached allow/deny] P -- No --> R[Run authorization check\nStore result in cache]Reviews (3): Last reviewed commit: "Merge branch 'main' into akam/agentex-SA..." | Re-trigger Greptile