fix: View as Agent must enforce the target user's data policy - #71
Merged
Conversation
Keep the admin JWT subject for logout/refresh/control-plane, but stamp impUid on the access token so overlay (and Agent Bearer fallback) evaluate as the viewed-as user. Never provision the admin session JWT into an Agent profile while impersonating, and forward the effective username to the Agent API instead of hardcoding admin. Co-authored-by: Venkat SF <venkatesh.sakamuri@stayflexi.com>
The previous change dropped the function declaration when inserting clearAgentRemoteUser, which broke the frontend parse. Co-authored-by: Venkat SF <venkatesh.sakamuri@stayflexi.com>
Walk the full SQL tree, deny unparseable or unhandled statements, and take the MCP/Editor actor from SecurityContext. Persist allowed schemas, scope RAG/brain retrieval, block protected-column prompt mentions, and refuse public shares on connections with an active policy. Co-authored-by: Venkat SF <venkatesh.sakamuri@stayflexi.com>
Hermes keeps one DeepSQL MCP stdio server from the first loaded profile (usually u-admin). Profile switch and a new chat thread do not respawn it, so View as marts-editor still executed SQL as admin and skipped policy. The provisioner now writes the minted token to every deepsql.token the already-running MCP client re-reads, not only the target profile's file. Co-authored-by: Venkat SF <venkatesh.sakamuri@stayflexi.com>
venkateshsakamuri-lab
marked this pull request as ready for review
August 20, 2026 14:43
venkateshsakamuri-lab
requested review from
a team and
geekypunk
as code owners
August 20, 2026 14:43
Keep fail-closed parse/actor rules and recursive SELECT inspection from this branch, and take main's whole-statement schema allowlist plus assertProtectedTablesAreInspectable. Blank outer-column provenance from a derived table is no longer treated as unresolved so a qualified protection does not catch the same table name in another schema. Co-authored-by: Venkat SF <venkatesh.sakamuri@stayflexi.com>
venkateshsakamuri-lab
pushed a commit
that referenced
this pull request
Aug 23, 2026
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>
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.
What was going wrong
A new Agent chat thread while Viewing as
marts-editorstill returnedcrm.customersrows (includingamount). SQL Editor already blockedcrm. Direct MCP API calls with a marts-editor token also blockedcrm. The Agent path did not.End-to-end logs for the failing turn (
session=fca723ca8171, 14:14 UTC):/api/agent/sessionminted MCP token 257 formarts-editorand provisionedu-marts-editor.POST /api/profile/switchreturned 200 three times; the session JSON is tagged"profile": "u-marts-editor".mcp__deepsql__get_brain_contextandmcp__deepsql__execute_sqlauthenticated as admin token 253.EDITOR_QUERY_EXECUTED/clientType=mcp/user_id=1(admin@demo.local) /SELECT … FROM crm.customers/rowCount=2.last_used_atis only the SpringprobeMcpAuthright after mint — Hermes never sent it.u-marts-editorhas nomcp-stderr.log;u-adminMCP was started on Aug 18 and reused.Hermes keeps one DeepSQL MCP stdio process, started from the first loaded profile (
u-admin).POST /api/profile/switchis explicitlyprocess_wide=False(cookie / thread-local only). A new chat does not respawn MCP.resolveEffectivePolicy(..., actorIsAdmin=true)returnsnone(), so schema and column policy never run.probeMcpAuthonly proves the minted token works against Spring, not that the live MCP process will use it.Fix
scripts/local-agent-provisioner.py(the Agent container's/provisionhandler) now mirrors the minted token onto$HERMES_HOME/deepsql.tokenand everyprofiles/*/deepsql.token. The long-lived MCP client re-readsDEEPSQL_TOKEN_FILEper request (mtime cache), so the nextexecute_sqlauthenticates as the viewed-as user and policy applies.Chat-path policy is also fail-closed for unparseable/unhandled SQL and missing actors, and walks CTEs, UNIONs, and nested
FROM/WHEREselects.Merge with main
Resolved conflicts with
main(#70whole-statement schema allowlist + nested protected-table inspectability,#72brain endpoint authz).Kept this branch's recursive SELECT walker and fail-closed parse/actor rules. Took main's
assertProtectedTablesAreInspectable/namesMatchso a qualified protection (public.customer_profiles) does not catch the same table name in another schema, while an unqualified nested reference still fails closed.UserDataAccessPolicyServiceTest(24) andBrainControllerAuthorizationSafetyTest(2) pass after the merge.How to verify
marts-editor, Agent tab, New chat.show me all customer information from crm tables.security_event.user_idfor that SQL must be the editor, not admin.Rebuild/restart
deepsql-agentso the container picks up the provisioner change, or copyscripts/local-agent-provisioner.pyonto/opt/deepsql-agent/provisioner.pyand restart the provisioner process, then reopen the Agent tab (triggers/provision).Re-test after the fix
Same prompt in a new Agent thread while Viewing as marts-editor. Agent refused CRM; audit
user_id=2/clientAgent=marts-editor.Agent refuses CRM while viewing as marts-editor
agent_view_as_crm_policy_block.mp4
To show artifacts inline, enable in settings.