Skip to content

fix(server): drop caller-sent _auth_* arguments before stamping the activity identity - #1789

Open
rudycelekli wants to merge 1 commit into
smart-mcp-proxy:mainfrom
rudycelekli:fix/activity-forged-auth-args-20261025
Open

rudycelekli wants to merge 1 commit into
smart-mcp-proxy:mainfrom
rudycelekli:fix/activity-forged-auth-args-20261025

Conversation

@rudycelekli

Copy link
Copy Markdown
Contributor

Bug

injectAuthMetadata (internal/server/mcp.go) builds the argument map that gets stored on the activity record. It copies the caller's arguments, then adds the _auth_* identity keys that the auth context has. It adds only the keys that exist for that caller:

  • an agent token adds auth_type, agent_name and token_prefix, but not user_id or user_email;
  • an admin caller adds only auth_type.

So when the caller sends one of the missing keys itself, it survives. Those keys are what the activity log trusts for identity:

  • internal/runtime/activity_service.go copies _auth_user_id / _auth_user_email onto ActivityRecord.UserID / UserEmail. That is the field the server-edition per-user activity view filters on.
  • _auth_agent_name is what the agent_name filter matches and what EffectiveTokenName falls back to when the record has no token name, which is always the case for an admin caller.

activityArgsFromRequest already strips caller-sent _auth_* keys for the two management tools, for exactly this reason (see the comment there). The normal call_tool_*, direct-mode, retrieve_tools and describe_tool paths go through injectAuthMetadata and did not.

Trigger

Any connected MCP client, or a model steered by a tool result, calls a tool with extra arguments such as:

{"query": "x", "_auth_user_id": "<another user's id>", "_auth_agent_name": "ci-bot"}

The activity row is then attributed to that user, or to the ci-bot agent token, instead of the real caller.

Fix

Run security.StripInternalArgs on the copy before adding the real identity keys. The caller's own args map, which is what gets forwarded upstream, is not changed. Activity views already strip _auth_* from the displayed arguments, so nothing visible is lost.

Test

TestInjectAuthMetadata_DropsCallerSuppliedAuthKeys: an agent context with forged _auth_user_id, _auth_user_email, _auth_agent_name and _auth_token_prefix arguments. The result must carry the token's real agent name and prefix, and no user id or email. With no auth context, the forged keys must also be gone. The original map must be unchanged.

Commands run

  • CGO_ENABLED=0 go test -count=1 -run 'TestInjectAuthMetadata|TestGetAuthMetadata|Activity' ./internal/server/: ok
  • On current main, and again with the one-line fix reverted, the new test fails: the forged _auth_user_id and _auth_user_email are present, and the no-auth result still has all four forged keys.
  • go vet ./internal/server/: ok

I did not run the full ./internal/server package locally.

…ctivity identity

injectAuthMetadata copied the caller's arguments and then added only the
identity keys the auth context had. An agent token carries no user_id, and
an admin caller carries no agent_name or token_prefix, so a caller that sent
those keys itself kept them. internal/runtime copies _auth_user_id and
_auth_user_email onto ActivityRecord.UserID/UserEmail, and the agent filter
and token name read _auth_agent_name, so the activity log recorded the
forged identity.

Strip the _auth_* keys from the copy before adding the real ones, the same
way activityArgsFromRequest already does for the management tools.
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants