fix(agent): stop cross-user MCP credential leak in the Agent tab (#78)

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>

Author: notSumit25

CLAUDE.md

@@ -251,9 +251,30 @@ The Agent tab must not inherit the admin MCP token. `/api/agent/session` mints a
    on purpose (per-browser cookie), so a View as / new-thread Agent chat tagged
    `profile: u-marts-editor` still sends the admin MCP bearer. Chat-access policy
    then `resolveEffectivePolicy(..., actorIsAdmin=true)` → `none()`. The
-   provisioner mirrors the target user's token onto every `deepsql.token` the
-   live process might re-read (`DEEPSQL_TOKEN_FILE` mtime cache in
-   `mcp/deepsql-phase1-lib.js`).
+   provisioner writes the target user's token to `$HERMES_HOME/deepsql.token`,
+   the one shared path the live process may have started from
+   (`DEEPSQL_TOKEN_FILE` mtime cache in `mcp/deepsql-phase1-lib.js`).
+
+   **Never fan that write out across `profiles/*/deepsql.token`.** It did once,
+   and made the agent credential globally last-writer-wins: any user opening the
+   Agent tab overwrote every other user's token, so their agent authenticated as
+   the newcomer. Verified end to end — `analyst`'s agent read an admin-only
+   connection (403 on their own session, 200 with the agent token, 133 vault
+   tables) and the `EDITOR_QUERY_EXECUTED` row named **admin**, not analyst. Two
+   concurrent users was the whole trigger; no impersonation needed. The
+   provisioner self-test asserted the fan-out as *correct* (it modelled only the
+   View-as case, where overwriting is desired), so a green suite guarded the bug —
+   it now asserts the opposite, that provisioning B leaves A's token intact.
+
+   Because that root file is still shared, the real guard is server-side:
+   `McpTokenAuthenticationFilter` refuses an MCP token whose owner differs from
+   the request's `DEEPSQL_MCP_USER_ID` claim (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 isn't a real
+   DeepSQL username is ignored, which is what keeps editor/CLI MCP installs
+   (`cursor`, `claude-desktop`, any `--caller-agent`) working. `probeMcpAuth`
+   sends the same header so the boot health check exercises the binding instead
+   of bypassing it.
 6. **The agent image build clones two third-party repos over the public internet,
    unauthenticated.** `agent/Dockerfile` fetches `NousResearch/hermes-agent` and
    `nesquena/hermes-webui` at build time. GitHub rate-limits unauthenticated

backend/src/main/java/com/dbaagent/controller/AgentBridgeController.java

@@ -59,7 +59,7 @@ public ResponseEntity<Map<String, Object>> session(
         Map<String, Object> response = new HashMap<>();
         response.put("profile", bootstrap.profile());
         response.put("username", username);
-        boolean mcpAuthOk = agentBridgeService.probeMcpAuth(bootstrap.token());
+        boolean mcpAuthOk = agentBridgeService.probeMcpAuth(bootstrap.token(), username);
         response.put("mcpAuthOk", mcpAuthOk);
         if (!mcpAuthOk) {
             response.put("mcpAuthError", "The DeepSQL Agent could not authenticate against this API with its "

backend/src/main/java/com/dbaagent/security/McpTokenAuthenticationFilter.java

@@ -1,5 +1,7 @@
 package com.dbaagent.security;
 
+import com.dbaagent.repository.UserRepository;
+import com.dbaagent.service.ClientContext;
 import com.dbaagent.service.McpTokenService;
 import jakarta.servlet.FilterChain;
 import jakarta.servlet.ServletException;
@@ -8,6 +10,8 @@
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Value;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.core.userdetails.UserDetails;
@@ -27,6 +31,7 @@ public class McpTokenAuthenticationFilter extends OncePerRequestFilter {
 
     private final McpTokenService mcpTokenService;
     private final CustomUserDetailsService userDetailsService;
+    private final UserRepository userRepository;
 
     @Override
     protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
@@ -49,7 +54,26 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
             return;
         }
 
-        mcpTokenService.authenticate(token, request.getRemoteAddr()).ifPresent(authenticatedToken -> {
+        var authenticated = mcpTokenService.authenticate(token, request.getRemoteAddr());
+        if (authenticated.isPresent() && !declaredUserMatches(request, authenticated.get().username())) {
+            // The agent runtime keeps ONE MCP subprocess for every profile, and
+            // the provisioner rotates its credential on disk. A token belonging
+            // to a different user than the one this MCP process was started for
+            // means the credential was overwritten by someone else's Agent-tab
+            // open — authenticating it here would run this user's tools as that
+            // other user (cross-user read + falsified audit attribution).
+            // Fail closed: the caller must re-provision, not silently proceed.
+            log.warn("Rejecting MCP token for {} — request declares user {}",
+                authenticated.get().username(), declaredUser(request));
+            response.setStatus(HttpStatus.UNAUTHORIZED.value());
+            response.setContentType(MediaType.APPLICATION_JSON_VALUE);
+            response.getWriter().write("{\"error\":\"mcp_identity_mismatch\",\"message\":"
+                + "\"This Agent session's credential belongs to a different user. "
+                + "Reopen the Agent tab to continue.\"}");
+            return;
+        }
+
+        authenticated.ifPresent(authenticatedToken -> {
             UserDetails userDetails = userDetailsService.loadUserByUsername(authenticatedToken.username());
             UsernamePasswordAuthenticationToken authentication = new UsernamePasswordAuthenticationToken(
                 userDetails,
@@ -64,4 +88,49 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
 
         chain.doFilter(request, response);
     }
+
+    /**
+     * The user this MCP process was provisioned for, as declared by the caller.
+     *
+     * <p>The agent provisioner writes {@code DEEPSQL_MCP_USER_ID=<username>} into
+     * each profile's MCP server env, and the MCP shim forwards it as
+     * {@link ClientContext#HEADER_AGENT}. It is a *claim*, not a credential — it
+     * is only ever used to REFUSE a mismatched token, never to grant access, so
+     * a forged value cannot widen access beyond what the token already allows.
+     */
+    private static String declaredUser(HttpServletRequest request) {
+        String declared = request.getHeader(ClientContext.HEADER_AGENT);
+        return declared == null || declared.isBlank() ? null : declared.trim();
+    }
+
+    /**
+     * True when the request carries no user claim (editor/CLI installs, curl,
+     * every pre-existing caller) or the claim matches the token's owner.
+     *
+     * <p>Absent claim stays permissive on purpose: {@code DEEPSQL_MCP_USER_ID}
+     * defaults to non-username values for editor installs ("cursor",
+     * "claude-code", "mcp-phase1"), and those tokens are not agent-provisioned,
+     * so there is no shared-credential hazard to guard against. Only a claim
+     * that looks like a DeepSQL agent profile identity is enforced.
+     */
+    private boolean declaredUserMatches(HttpServletRequest request, String tokenOwner) {
+        String declared = declaredUser(request);
+        if (declared == null || tokenOwner == null) {
+            return true;
+        }
+        if (declared.equalsIgnoreCase(tokenOwner)) {
+            return true;
+        }
+        // The claim differs from the token owner. Enforce only when the claim
+        // names a real DeepSQL user — that is the agent-provisioner case, where
+        // DEEPSQL_MCP_USER_ID is a username and a mismatch means the shared
+        // token file was overwritten by another user's Agent-tab open.
+        //
+        // Editor/CLI clients put a *tool* name here ("cursor", "claude-desktop",
+        // "terminal", "mcp-phase1", or any --caller-agent value), which never
+        // resolves to a user, so those callers are unaffected. Matching against
+        // the user table rather than a denylist of sentinels keeps free-form
+        // --caller-agent values working without a list to maintain.
+        return userRepository.findByUsernameIgnoreCase(declared).isEmpty();
+    }
 }

backend/src/main/java/com/dbaagent/service/AgentBridgeService.java

@@ -312,16 +312,30 @@ private void callProvisionerRevoke(String username) {
      * bearer credential against the local backend (loopback — this call never
      * leaves the box, so no external base-URL config is needed).
      *
+     * <p>Sends the same {@code X-DeepSQL-Client-Agent: <username>} claim the
+     * provisioned MCP process will send, so the probe passes only if the token
+     * and the declared identity agree. A probe without that header would call a
+     * token healthy that the live MCP process cannot actually use.
+     *
+     * @param token    the credential to verify
+     * @param username the identity the MCP process will declare for this profile
      * @return true if the API accepted the token (2xx), false on any
      *         non-2xx/auth failure or network error.
      */
-    public boolean probeMcpAuth(String token) {
+    public boolean probeMcpAuth(String token, String username) {
         if (token == null || token.isBlank()) {
             return false;
         }
         try {
             HttpRequest req = HttpRequest.newBuilder(URI.create(localApiBaseUrl + "/connections"))
                 .header("Authorization", "Bearer " + token)
+                // Declare the same identity the provisioned MCP process will send
+                // (DEEPSQL_MCP_USER_ID -> X-DeepSQL-Client-Agent), so the probe
+                // exercises the identity-binding check in
+                // McpTokenAuthenticationFilter rather than bypassing it. Probing
+                // without this header would report a token as healthy even when
+                // the live MCP process's own calls will be refused.
+                .header(ClientContext.HEADER_AGENT, username)
                 .timeout(Duration.ofSeconds(5))
                 .GET()
                 .build();

backend/src/test/java/com/dbaagent/security/McpTokenAuthenticationFilterTest.java

@@ -1,5 +1,6 @@
 package com.dbaagent.security;
 
+import com.dbaagent.repository.UserRepository;
 import com.dbaagent.service.McpTokenService;
 import jakarta.servlet.ServletException;
 import org.junit.jupiter.api.AfterEach;
@@ -22,6 +23,8 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.when;
 
 @ExtendWith(MockitoExtension.class)
@@ -33,6 +36,9 @@ class McpTokenAuthenticationFilterTest {
     @Mock
     private CustomUserDetailsService userDetailsService;
 
+    @Mock
+    private UserRepository userRepository;
+
     @InjectMocks
     private McpTokenAuthenticationFilter filter;
 
@@ -67,4 +73,100 @@ void authenticateUsesResolvedUsernameWithoutTouchingLazyEntity() throws ServletE
         assertNotNull(SecurityContextHolder.getContext().getAuthentication());
         assertEquals("alice", SecurityContextHolder.getContext().getAuthentication().getName());
     }
+
+    /**
+     * The cross-user token leak, at the layer that stops it.
+     *
+     * <p>The agent runtime keeps ONE MCP subprocess and the provisioner rotates
+     * its credential on disk, so another user's Agent-tab open could leave
+     * analyst's MCP process holding admin's token. Authenticating that token
+     * would run analyst's tools as admin — reading connections analyst has no
+     * grant for, and writing admin into the audit row. The request's own
+     * DEEPSQL_MCP_USER_ID claim ("analyst") contradicts the token owner
+     * ("admin"), which is the signal to refuse.
+     */
+    @Test
+    void rejectsTokenWhoseOwnerDiffersFromTheDeclaredMcpUser() throws ServletException, IOException {
+        ReflectionTestUtils.setField(filter, "authEnabled", true);
+
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/connections");
+        request.addHeader("Authorization", "Bearer dsql_mcp_public.secret");
+        // This MCP process was provisioned for analyst...
+        request.addHeader("X-DeepSQL-Client-Agent", "analyst");
+        request.setRemoteAddr("127.0.0.1");
+
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        MockFilterChain chain = new MockFilterChain();
+
+        when(mcpTokenService.looksLikeMcpToken("dsql_mcp_public.secret")).thenReturn(true);
+        // ...but the token file was overwritten with admin's credential.
+        when(mcpTokenService.authenticate("dsql_mcp_public.secret", "127.0.0.1"))
+            .thenReturn(Optional.of(new McpTokenService.AuthenticatedMcpToken(9L, "admin")));
+        when(userRepository.findByUsernameIgnoreCase("analyst"))
+            .thenReturn(Optional.of(new com.dbaagent.model.User()));
+
+        filter.doFilter(request, response, chain);
+
+        assertNull(SecurityContextHolder.getContext().getAuthentication(),
+            "a mismatched MCP credential must not authenticate anyone");
+        assertEquals(401, response.getStatus());
+        assertTrue(response.getContentAsString().contains("mcp_identity_mismatch"));
+        assertNull(chain.getRequest(), "the request must not reach downstream handlers");
+    }
+
+    /** The normal agent case: the claim matches the token owner. */
+    @Test
+    void allowsTokenWhenDeclaredMcpUserMatchesOwner() throws ServletException, IOException {
+        ReflectionTestUtils.setField(filter, "authEnabled", true);
+
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/connections");
+        request.addHeader("Authorization", "Bearer dsql_mcp_public.secret");
+        request.addHeader("X-DeepSQL-Client-Agent", "analyst");
+        request.setRemoteAddr("127.0.0.1");
+
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        MockFilterChain chain = new MockFilterChain();
+
+        when(mcpTokenService.looksLikeMcpToken("dsql_mcp_public.secret")).thenReturn(true);
+        when(mcpTokenService.authenticate("dsql_mcp_public.secret", "127.0.0.1"))
+            .thenReturn(Optional.of(new McpTokenService.AuthenticatedMcpToken(9L, "analyst")));
+        when(userDetailsService.loadUserByUsername("analyst"))
+            .thenReturn(new User("analyst", "ignored",
+                List.of(new SimpleGrantedAuthority("ROLE_DEVELOPER"))));
+
+        filter.doFilter(request, response, chain);
+
+        assertEquals("analyst", SecurityContextHolder.getContext().getAuthentication().getName());
+    }
+
+    /**
+     * Editor/CLI installs put a *tool* name in this header ("cursor",
+     * "claude-desktop", any --caller-agent value). Those tokens are not
+     * agent-provisioned, so the claim must not be compared against a username —
+     * otherwise every editor MCP install would 401.
+     */
+    @Test
+    void allowsEditorClientAgentThatIsNotADeepSqlUsername() throws ServletException, IOException {
+        ReflectionTestUtils.setField(filter, "authEnabled", true);
+
+        MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/connections");
+        request.addHeader("Authorization", "Bearer dsql_mcp_public.secret");
+        request.addHeader("X-DeepSQL-Client-Agent", "cursor");
+        request.setRemoteAddr("127.0.0.1");
+
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        MockFilterChain chain = new MockFilterChain();
+
+        when(mcpTokenService.looksLikeMcpToken("dsql_mcp_public.secret")).thenReturn(true);
+        when(mcpTokenService.authenticate("dsql_mcp_public.secret", "127.0.0.1"))
+            .thenReturn(Optional.of(new McpTokenService.AuthenticatedMcpToken(11L, "bob")));
+        when(userRepository.findByUsernameIgnoreCase("cursor")).thenReturn(Optional.empty());
+        when(userDetailsService.loadUserByUsername("bob"))
+            .thenReturn(new User("bob", "ignored",
+                List.of(new SimpleGrantedAuthority("ROLE_DEVELOPER"))));
+
+        filter.doFilter(request, response, chain);
+
+        assertEquals("bob", SecurityContextHolder.getContext().getAuthentication().getName());
+    }
 }