fix(agentengine): bind session ops to authenticated caller identity#962
Open
XananasX7 wants to merge 1 commit into
Open
fix(agentengine): bind session ops to authenticated caller identity#962XananasX7 wants to merge 1 commit into
XananasX7 wants to merge 1 commit into
Conversation
AgentEngine session method handlers (create, get, list, delete, stream_query) were trusting caller-supplied user_id and session_id values with no binding to an authenticated principal, enabling BOLA/IDOR-style cross-user session access. Additionally, create_session accepted raw app: and user: prefixed state keys from external callers, allowing callers to write into shared app-wide and per-user state stores visible to other users. This change introduces two targeted fixes: 1. resolveUserID() - when authentication middleware has stored a verified identity in the request context via WithAuthenticatedUserID, that identity takes precedence over the caller-supplied user_id. Deployments without authentication middleware are unaffected (the caller-supplied value is used as before, preserving backward compatibility with single-tenant and local-dev use). 2. sanitizeExternalState() - strips app: and user: prefixed keys from initial session state supplied by external callers at create time, preventing untrusted callers from poisoning shared state stores. Reported via Google OSS VRP.
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
1 participant
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
AgentEngine session method handlers (
create_session,get_session,list_sessions,delete_session,stream_query) were trusting caller-supplieduser_idandsession_idvalues with no binding to an authenticated principal. This enabled BOLA/IDOR-style cross-user session access: any caller who can reach the endpoint can read, list, and delete another user's sessions by supplying that user'suser_id.Additionally,
create_sessionaccepted rawapp:anduser:prefixed keys in the initial state payload. Because the session service interprets those prefixes as writes into shared app-wide and per-user state stores, an external caller could poison state visible to other users or persist malicious context across a victim's future sessions.Changes
server/agentengine/controllers/method/authz.go(new file)WithAuthenticatedUserID/authenticatedUserID— context helpers for middleware to inject a verified principal.resolveUserID— returns the authenticated identity when present, falls back to the caller-supplied value otherwise (backward-compatible with single-tenant / local-dev deployments).sanitizeExternalState— stripsapp:anduser:prefixed keys from state maps supplied by external callers.Handler patches
All five AgentEngine method handlers now call
resolveUserIDbefore constructing their session service request.create_sessionadditionally passes the state throughsanitizeExternalState.Backward compatibility
Deployments that do not configure authentication middleware are unaffected:
resolveUserIDfalls back to the caller-supplieduser_idexactly as before. The fix only takes effect when a middleware stores a verified identity viaWithAuthenticatedUserID.Testing
Builds cleanly with no new errors or warnings.
Reported via Google OSS VRP.