[Security 4/9] MCP tools require confirmation unless positively known-safe

backend/migrations/20260802_01_mcp_pending_tool_calls.sql

@@ -0,0 +1,40 @@
+-- Pending-approval ledger for MCP tool calls.
+--
+-- When the model proposes calling an MCP tool that is not positively trusted,
+-- the EXACT proposed call (tool + arguments) is stored here and shown to the
+-- user; the tool executes only after the user approves THAT row. Approval is
+-- bound to (user, pending call id, stored payload), short-lived (expires_at)
+-- and single-use: the status column is a one-way state machine
+--   pending -> approved -> executing -> executed
+--   pending -> denied
+--   pending -> expired
+-- enforced by conditional UPDATEs in the backend (status must match the
+-- expected prior state), so a decision or execution can never happen twice.
+--
+-- RLS is enabled with no browser policies, matching the other MCP tables:
+-- only the service-role backend reads or writes rows, and it always scopes
+-- queries by user_id.
+
+CREATE TABLE IF NOT EXISTS public.user_mcp_pending_tool_calls (
+  id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
+  user_id uuid NOT NULL REFERENCES auth.users(id) ON DELETE CASCADE,
+  connector_id uuid NOT NULL REFERENCES public.user_mcp_connectors(id) ON DELETE CASCADE,
+  tool_id uuid REFERENCES public.user_mcp_connector_tools(id) ON DELETE SET NULL,
+  tool_name text NOT NULL,
+  openai_tool_name text NOT NULL,
+  arguments jsonb NOT NULL DEFAULT '{}'::jsonb,
+  status text NOT NULL DEFAULT 'pending'
+    CHECK (status IN ('pending', 'approved', 'denied', 'executing', 'executed', 'expired')),
+  created_at timestamptz NOT NULL DEFAULT now(),
+  expires_at timestamptz NOT NULL,
+  decided_at timestamptz,
+  executed_at timestamptz
+);
+
+CREATE INDEX IF NOT EXISTS idx_user_mcp_pending_tool_calls_user
+  ON public.user_mcp_pending_tool_calls(user_id);
+
+CREATE INDEX IF NOT EXISTS idx_user_mcp_pending_tool_calls_status
+  ON public.user_mcp_pending_tool_calls(status, expires_at);
+
+ALTER TABLE public.user_mcp_pending_tool_calls ENABLE ROW LEVEL SECURITY;

backend/src/lib/chat/tools/toolDispatcher.ts

@@ -597,6 +597,32 @@ export async function runToolCalls(
         tc.function.name,
         args,
         db,
+        {
+          // Approval-gated call: surface the exact stored payload so the user
+          // can approve or decline it inline, then report how it resolved.
+          onApprovalRequired: (pending) => {
+            write(
+              `data: ${JSON.stringify({
+                type: "mcp_confirmation_required",
+                id: pending.id,
+                name: tc.function.name,
+                connector_name: pending.connector_name,
+                tool_name: pending.tool_name,
+                arguments_json: pending.arguments_json,
+                expires_at: pending.expires_at,
+              })}\n\n`,
+            );
+          },
+          onApprovalResolved: (pendingId, decision) => {
+            write(
+              `data: ${JSON.stringify({
+                type: "mcp_confirmation_resolved",
+                id: pendingId,
+                decision,
+              })}\n\n`,
+            );
+          },
+        },
       );
       toolResults.push({
         role: "tool",

backend/src/lib/mcp/__tests__/approvals.test.ts

@@ -0,0 +1,214 @@
+import { describe, expect, it } from "vitest";
+import {
+    claimApprovedMcpToolCall,
+    createPendingMcpToolCall,
+    decideMcpPendingToolCall,
+    waitForMcpApprovalDecision,
+} from "../approvals";
+import type { ConnectorRow, Db, ToolCacheRow } from "../types";
+
+// The approval ledger's promises — bound to a user, short-lived, single-use,
+// executes only the stored payload — are all enforced by conditional UPDATEs
+// against the pending row's state. This suite drives the real module against
+// an in-memory stand-in for the Supabase query builder to prove each
+// transition admits exactly one winner.
+
+type Row = Record<string, unknown>;
+
+function createFakeDb(initial: Row[] = []) {
+    const rows: Row[] = initial.map((row) => ({ ...row }));
+    let nextId = rows.length + 1;
+
+    function from(_table: string) {
+        const state = {
+            op: "select" as "select" | "insert" | "update",
+            values: {} as Row,
+            filters: [] as Array<(row: Row) => boolean>,
+            single: false,
+        };
+        const api = {
+            insert(values: Row) {
+                state.op = "insert";
+                state.values = values;
+                return api;
+            },
+            update(values: Row) {
+                state.op = "update";
+                state.values = values;
+                return api;
+            },
+            select(_columns?: string) {
+                return api;
+            },
+            eq(key: string, value: unknown) {
+                state.filters.push((row) => row[key] === value);
+                return api;
+            },
+            gt(key: string, value: unknown) {
+                // ISO timestamps compare correctly as strings.
+                state.filters.push((row) => String(row[key]) > String(value));
+                return api;
+            },
+            single() {
+                state.single = true;
+                return api;
+            },
+            then(
+                resolve: (value: { data: unknown; error: unknown }) => void,
+            ) {
+                let data: unknown;
+                if (state.op === "insert") {
+                    const row: Row = {
+                        id: `pending-${nextId++}`,
+                        created_at: new Date().toISOString(),
+                        decided_at: null,
+                        executed_at: null,
+                        ...state.values,
+                    };
+                    rows.push(row);
+                    data = state.single ? { ...row } : [{ ...row }];
+                } else {
+                    const matched = rows.filter((row) =>
+                        state.filters.every((filter) => filter(row)),
+                    );
+                    if (state.op === "update") {
+                        for (const row of matched)
+                            Object.assign(row, state.values);
+                    }
+                    data = state.single
+                        ? matched[0]
+                            ? { ...matched[0] }
+                            : null
+                        : matched.map((row) => ({ ...row }));
+                }
+                const error =
+                    state.single && !data ? { message: "not found" } : null;
+                resolve({ data, error });
+            },
+        };
+        return api;
+    }
+
+    return { db: { from } as unknown as Db, rows };
+}
+
+const connector = { id: "conn-1", name: "Test Server" } as ConnectorRow;
+const tool = {
+    id: "tool-1",
+    tool_name: "delete_case",
+    openai_tool_name: "mcp_test_delete_case_abc",
+} as ToolCacheRow;
+
+async function seedPending(db: Db) {
+    return createPendingMcpToolCall(
+        "user-1",
+        connector,
+        tool,
+        { case_id: 42 },
+        db,
+    );
+}
+
+describe("pending MCP tool call lifecycle", () => {
+    it("stores the exact proposed payload with an expiry", async () => {
+        const { db, rows } = createFakeDb();
+        const pending = await seedPending(db);
+        expect(pending.status).toBe("pending");
+        expect(pending.arguments).toEqual({ case_id: 42 });
+        expect(new Date(pending.expires_at).getTime()).toBeGreaterThan(
+            Date.now(),
+        );
+        expect(rows).toHaveLength(1);
+    });
+
+    it("approves only for the owning user; a stranger's decision changes nothing", async () => {
+        const { db, rows } = createFakeDb();
+        const pending = await seedPending(db);
+
+        const stranger = await decideMcpPendingToolCall(
+            "attacker",
+            pending.id,
+            "approve",
+            db,
+        );
+        expect(stranger).toBe("not_found");
+        expect(rows[0].status).toBe("pending");
+
+        const owner = await decideMcpPendingToolCall(
+            "user-1",
+            pending.id,
+            "approve",
+            db,
+        );
+        expect(owner).toBe("approved");
+        expect(rows[0].status).toBe("approved");
+    });
+
+    it("a decision is single-use: the second decision cannot flip the first", async () => {
+        const { db, rows } = createFakeDb();
+        const pending = await seedPending(db);
+
+        expect(
+            await decideMcpPendingToolCall("user-1", pending.id, "deny", db),
+        ).toBe("denied");
+        expect(
+            await decideMcpPendingToolCall("user-1", pending.id, "approve", db),
+        ).toBe("not_found");
+        expect(rows[0].status).toBe("denied");
+    });
+
+    it("an expired pending call can no longer be approved", async () => {
+        const { db, rows } = createFakeDb();
+        const pending = await seedPending(db);
+        rows[0].expires_at = new Date(Date.now() - 1000).toISOString();
+
+        expect(
+            await decideMcpPendingToolCall("user-1", pending.id, "approve", db),
+        ).toBe("expired");
+        expect(rows[0].status).toBe("pending");
+    });
+
+    it("execution claim is single-use: exactly one winner per approval", async () => {
+        const { db, rows } = createFakeDb();
+        const pending = await seedPending(db);
+        await decideMcpPendingToolCall("user-1", pending.id, "approve", db);
+
+        const first = await claimApprovedMcpToolCall(pending.id, db);
+        expect(first?.arguments).toEqual({ case_id: 42 });
+        expect(rows[0].status).toBe("executing");
+
+        const second = await claimApprovedMcpToolCall(pending.id, db);
+        expect(second).toBeNull();
+    });
+
+    it("a denied or never-approved call can never be claimed for execution", async () => {
+        const { db } = createFakeDb();
+        const pending = await seedPending(db);
+        expect(await claimApprovedMcpToolCall(pending.id, db)).toBeNull();
+
+        await decideMcpPendingToolCall("user-1", pending.id, "deny", db);
+        expect(await claimApprovedMcpToolCall(pending.id, db)).toBeNull();
+    });
+
+    it("waitForMcpApprovalDecision returns an existing decision immediately", async () => {
+        const { db } = createFakeDb();
+        const pending = await seedPending(db);
+        await decideMcpPendingToolCall("user-1", pending.id, "approve", db);
+        expect(await waitForMcpApprovalDecision(pending.id, db, 0)).toBe(
+            "approved",
+        );
+    });
+
+    it("waitForMcpApprovalDecision retires an undecided call on timeout so a late click is inert", async () => {
+        const { db, rows } = createFakeDb();
+        const pending = await seedPending(db);
+
+        expect(await waitForMcpApprovalDecision(pending.id, db, 0)).toBe(
+            "expired",
+        );
+        expect(rows[0].status).toBe("expired");
+        expect(
+            await decideMcpPendingToolCall("user-1", pending.id, "approve", db),
+        ).toBe("expired");
+    });
+});