fix(agent): stop cross-user MCP credential leak in the Agent tab - #78
Merged
Merged
Conversation
The agent provisioner mirrored each freshly minted MCP token onto `$HERMES_HOME/deepsql.token` AND every `profiles/*/deepsql.token`, which made the agent credential globally last-writer-wins. Any user opening the Agent tab overwrote every other user's token, so their agent then authenticated as the newcomer. Two concurrent users was the whole trigger; no impersonation required. Verified end to end before the fix (analyst = DEVELOPER, admin = ADMIN, `QA Vault Copy` admin-only): analyst opens tab -> analyst file: dsql_mcp_v2nA7... admin opens tab -> analyst file: dsql_mcp_p4nIo... (admin's token) /api/auth/me with analyst's file -> username=admin, role=ADMIN analyst session -> QA Vault Copy -> 403 analyst agent -> QA Vault Copy -> 200, 133 tables SELECT via that token -> logged as user_id=8 (admin) So one user read another's database and the audit row named the wrong person. The schema read produced no audit event at all. The mirror was introduced to fix a real View-as bug (#71): Hermes keeps ONE MCP subprocess, `profile/switch` is `process_wide=False`, so the live process kept whichever credential loaded first. That diagnosis stands — only the remedy was too broad. Fix, in two layers: 1. The provisioner writes only `$HERMES_HOME/deepsql.token`, the one shared path the live process may have started from. Per-profile tokens are no longer touched, so one user's open cannot destroy another's credential. 2. That root file is still shared, so the durable guard is server-side: `McpTokenAuthenticationFilter` refuses an MCP token whose owner differs from the request's `DEEPSQL_MCP_USER_ID` claim (already sent as `X-DeepSQL-Client-Agent`), answering 401 `mcp_identity_mismatch`. The claim is only ever used to REFUSE, never to grant, so forging it cannot widen access. A claim that is not a real DeepSQL username is ignored, keeping editor/CLI MCP installs (`cursor`, `claude-desktop`, any `--caller-agent`) working. Also: - The provisioner self-test asserted the fan-out was CORRECT — it modelled only the View-as case, where overwriting is desired — so a green suite guarded the bug and would have failed on this fix. It now asserts the opposite: provisioning B leaves A's token intact. - `probeMcpAuth` sends the same identity header so the boot health check exercises the binding instead of bypassing it. Without that it would report a token healthy that the live MCP process cannot use. - `AgentChatPanel` re-bootstraps on an SSE error to surface the real cause; EventSource exposes no status code, so session expiry, an agent restart and a refused credential all arrived as "the agent run ended early". Verified after the fix: analyst keeps its own token when admin opens the tab; the leaked-token request is 401 instead of 200/133-tables; and every legitimate path still works (own agent 200, editor MCP 200, no-header curl 200, analyst -> admin-only DB still 403). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
notSumit25
requested review from
a team,
geekypunk and
venkateshsakamuri-lab
as code owners
August 23, 2026 07:19
venkateshsakamuri-lab
approved these changes
Aug 23, 2026
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.
The agent provisioner mirrored each freshly minted MCP token onto
$HERMES_HOME/deepsql.tokenAND everyprofiles/*/deepsql.token, which made the agent credential globally last-writer-wins. Any user opening the Agent tab overwrote every other user's token, so their agent then authenticated as the newcomer. Two concurrent users was the whole trigger; no impersonation required.Verified end to end before the fix (analyst = DEVELOPER, admin = ADMIN,
QA Vault Copyadmin-only):analyst opens tab -> analyst file: dsql_mcp_v2nA7...
admin opens tab -> analyst file: dsql_mcp_p4nIo... (admin's token)
/api/auth/me with analyst's file -> username=admin, role=ADMIN
analyst session -> QA Vault Copy -> 403
analyst agent -> QA Vault Copy -> 200, 133 tables
SELECT via that token -> logged as user_id=8 (admin)
So one user read another's database and the audit row named the wrong person. The schema read produced no audit event at all.
The mirror was introduced to fix a real View-as bug (#71): Hermes keeps ONE MCP subprocess,
profile/switchisprocess_wide=False, so the live process kept whichever credential loaded first. That diagnosis stands — only the remedy was too broad.Fix, in two layers:
The provisioner writes only
$HERMES_HOME/deepsql.token, the one shared path the live process may have started from. Per-profile tokens are no longer touched, so one user's open cannot destroy another's credential.That root file is still shared, so the durable guard is server-side:
McpTokenAuthenticationFilterrefuses an MCP token whose owner differs from the request'sDEEPSQL_MCP_USER_IDclaim (already sent asX-DeepSQL-Client-Agent), answering 401mcp_identity_mismatch. The claim is only ever used to REFUSE, never to grant, so forging it cannot widen access. A claim that is not a real DeepSQL username is ignored, keeping editor/CLI MCP installs (cursor,claude-desktop, any--caller-agent) working.Also:
probeMcpAuthsends the same identity header so the boot health check exercises the binding instead of bypassing it. Without that it would report a token healthy that the live MCP process cannot use.AgentChatPanelre-bootstraps on an SSE error to surface the real cause; EventSource exposes no status code, so session expiry, an agent restart and a refused credential all arrived as "the agent run ended early".Verified after the fix: analyst keeps its own token when admin opens the tab; the leaked-token request is 401 instead of 200/133-tables; and every legitimate path still works (own agent 200, editor MCP 200, no-header curl 200, analyst -> admin-only DB still 403).