feat(local-mcp): enforce relay tool approval on the desktop

richardsolomou · web-flow · commit 555b643c6481 · 2026-07-16T11:25:48.000+03:00
A relayed tools/call executes on the user's machine with their privileges, so
CloudTaskService now refuses to execute one until the user approves it —
enforced at the desktop, the real trust boundary, rather than trusting the
sandbox's own prompt (a compromised sandbox could emit mcp_request events
without asking) and adapter-neutrally (codex has no per-MCP-call approval
hook, so a harness-side gate would leave GPT runs unguarded).

The claude adapter still prompts via its always-ask path; to avoid a double
prompt, the user's answer to that sandbox prompt is routed through sendCommand
and converted into a consume-once pass (or run-scoped always-allow) for the
matching relayed call. Denials return a JSON-RPC error with the user's
feedback instead of executing. Approval state is evicted on terminal status.
diff --git a/docs/cloud-mcp-relay.md b/docs/cloud-mcp-relay.md
@@ -180,12 +180,21 @@ const mcpResponseParamsSchema = z.object({
 A relayed tool call is the cloud agent executing something **on the user's
 machine or network with the user's local privileges**. Decisions:
 
-1. **Always-ask by default.** Relayed servers' tools default to
-   `needs_approval` through the existing MCP tool-approval mechanism
-   (`McpToolApprovals`), regardless of the run's permission mode — an `auto`
-   or `allow-all` cloud run still prompts (via the existing
-   permission-request relay) before each relayed tool call. Users can
-   allowlist individual tools, which persists like other tool approvals.
+1. **Always-ask, enforced on the desktop.** The desktop refuses to execute a
+   relayed `tools/call` until the user has approved it, in
+   `CloudTaskService.handleMcpRelayRequest`. This is the real trust boundary:
+   the sandbox's own prompt can't be relied on (a compromised sandbox could
+   emit `mcp_request` events without ever asking), and it is adapter-neutral —
+   codex has no per-MCP-call approval hook, so a harness-side gate would leave
+   GPT runs unguarded. The claude adapter *also* prompts (its always-ask
+   `McpToolApprovals` path); to avoid a double prompt, the user's answer to
+   that sandbox prompt is routed through `sendCommand`, where an allow is
+   converted into a consume-once pass (or an always-allow, per run + server +
+   tool) for the matching relayed call — so a harness that asks and one that
+   doesn't both end at exactly one prompt. "Always allow" is scoped to the run
+   and dropped when the run reaches a terminal status; a plain allow covers a
+   single call with identical arguments. A denial (with optional feedback) is
+   returned to the sandbox as a JSON-RPC error rather than executed.
 2. **No configuration crosses the wire.** The sandbox only ever names a
    server; what "grafana" means (command line, env, URL, headers) is resolved
    from local config on the desktop. A compromised sandbox cannot make the
diff --git a/packages/core/src/cloud-task/cloud-task.test.ts b/packages/core/src/cloud-task/cloud-task.test.ts
@@ -3139,19 +3139,74 @@ describe("CloudTaskService MCP relay", () => {
       requestId: string;
       server: string;
       expiresAt: string;
+      payload: Record<string, unknown>;
     }> = {},
   ): string {
     const event = {
       type: "mcp_request",
       requestId: overrides.requestId ?? "req-1",
       server: overrides.server ?? "slack",
-      payload: { jsonrpc: "2.0", id: 1, method: "initialize" },
+      payload: overrides.payload ?? {
+        jsonrpc: "2.0",
+        id: 1,
+        method: "initialize",
+      },
       expiresAt:
         overrides.expiresAt ?? new Date(Date.now() + 60_000).toISOString(),
     };
     return `data: ${JSON.stringify(event)}\n\n`;
   }
 
+  function toolsCallPayload(
+    args: Record<string, unknown> = { channel: "#general" },
+  ): Record<string, unknown> {
+    return {
+      jsonrpc: "2.0",
+      id: 2,
+      method: "tools/call",
+      params: { name: "send_message", arguments: args },
+    };
+  }
+
+  function createControllableSseResponse(): {
+    response: Response;
+    push: (chunk: string) => void;
+  } {
+    const encoder = new TextEncoder();
+    let streamController: ReadableStreamDefaultController<Uint8Array>;
+    const stream = new ReadableStream<Uint8Array>({
+      start(controller) {
+        streamController = controller;
+      },
+    });
+    return {
+      response: new Response(stream, {
+        status: 200,
+        headers: { "Content-Type": "text/event-stream" },
+      }),
+      push: (chunk: string) => streamController.enqueue(encoder.encode(chunk)),
+    };
+  }
+
+  function lastPermissionRequestUpdate(
+    updates: unknown[],
+  ): { requestId: string } | undefined {
+    return [...updates]
+      .reverse()
+      .find(
+        (u): u is { kind: string; requestId: string } =>
+          typeof u === "object" &&
+          u !== null &&
+          (u as { kind?: string }).kind === "permission_request",
+      );
+  }
+
+  function commandPosts(): unknown[] {
+    return mockNetFetch.mock.calls
+      .filter(([url]) => (url as string).includes("/command/"))
+      .map(([, init]) => JSON.parse((init as RequestInit).body as string));
+  }
+
   function watchRun(runId: string): void {
     mockNetFetch.mockResolvedValueOnce(
       createJsonResponse({
@@ -3328,4 +3383,241 @@ describe("CloudTaskService MCP relay", () => {
 
     expect(mcpRelayExecutor.closeRun).toHaveBeenCalledWith("run-1");
   });
+
+  describe("relayed tools/call approval", () => {
+    it("prompts on the desktop and executes after the user allows", async () => {
+      const updates: unknown[] = [];
+      relayService.on(CloudTaskEvent.Update, (payload) => {
+        updates.push(payload);
+      });
+      mockNetFetch.mockResolvedValue(createJsonResponse({ result: {} }));
+      mockStreamFetch.mockResolvedValueOnce(
+        createOpenSseResponse(
+          mcpRequestSseLine({ payload: toolsCallPayload() }),
+        ),
+      );
+      relayService.designateRelayedMcpServers("run-1", ["slack"]);
+      watchRun("run-1");
+
+      await waitFor(() => lastPermissionRequestUpdate(updates) !== undefined);
+      expect(mcpRelayExecutor.execute).not.toHaveBeenCalled();
+
+      const prompt = lastPermissionRequestUpdate(updates);
+      await relayService.sendCommand({
+        taskId: "task-1",
+        runId: "run-1",
+        apiHost: "https://app.example.com",
+        teamId: 2,
+        method: "permission_response",
+        params: { requestId: prompt?.requestId, optionId: "allow" },
+      });
+
+      await waitFor(() => mcpRelayExecutor.execute.mock.calls.length > 0);
+      expect(mcpRelayExecutor.execute).toHaveBeenCalledWith(
+        "run-1",
+        "slack",
+        expect.objectContaining({ method: "tools/call" }),
+      );
+      await waitFor(() =>
+        commandPosts().some(
+          (body) => (body as { method?: string }).method === "mcp_response",
+        ),
+      );
+    });
+
+    it("answers a denial to the sandbox without executing, carrying the user's feedback", async () => {
+      const updates: unknown[] = [];
+      relayService.on(CloudTaskEvent.Update, (payload) => {
+        updates.push(payload);
+      });
+      mockNetFetch.mockResolvedValue(createJsonResponse({ result: {} }));
+      mockStreamFetch.mockResolvedValueOnce(
+        createOpenSseResponse(
+          mcpRequestSseLine({ payload: toolsCallPayload() }),
+        ),
+      );
+      relayService.designateRelayedMcpServers("run-1", ["slack"]);
+      watchRun("run-1");
+
+      await waitFor(() => lastPermissionRequestUpdate(updates) !== undefined);
+      const prompt = lastPermissionRequestUpdate(updates);
+      await relayService.sendCommand({
+        taskId: "task-1",
+        runId: "run-1",
+        apiHost: "https://app.example.com",
+        teamId: 2,
+        method: "permission_response",
+        params: {
+          requestId: prompt?.requestId,
+          optionId: "reject",
+          customInput: "use the announcements channel instead",
+        },
+      });
+
+      await waitFor(() =>
+        commandPosts().some(
+          (body) => (body as { method?: string }).method === "mcp_response",
+        ),
+      );
+      expect(mcpRelayExecutor.execute).not.toHaveBeenCalled();
+      const response = commandPosts().find(
+        (body) => (body as { method?: string }).method === "mcp_response",
+      ) as { params: { error?: { message?: string } } };
+      expect(response.params.error?.message).toContain(
+        "use the announcements channel instead",
+      );
+    });
+
+    it("drops an unanswered prompt at the request's expiry without executing", async () => {
+      const updates: unknown[] = [];
+      relayService.on(CloudTaskEvent.Update, (payload) => {
+        updates.push(payload);
+      });
+      mockNetFetch.mockResolvedValue(createJsonResponse({ result: {} }));
+      mockStreamFetch.mockResolvedValueOnce(
+        createOpenSseResponse(
+          mcpRequestSseLine({
+            payload: toolsCallPayload(),
+            expiresAt: new Date(Date.now() + 150).toISOString(),
+          }),
+        ),
+      );
+      relayService.designateRelayedMcpServers("run-1", ["slack"]);
+      watchRun("run-1");
+
+      await waitFor(() => lastPermissionRequestUpdate(updates) !== undefined);
+      await new Promise((resolve) => setTimeout(resolve, 300));
+
+      expect(mcpRelayExecutor.execute).not.toHaveBeenCalled();
+      expect(
+        commandPosts().some(
+          (body) => (body as { method?: string }).method === "mcp_response",
+        ),
+      ).toBe(false);
+    });
+
+    it("consumes a harness prompt approval instead of prompting a second time", async () => {
+      const updates: unknown[] = [];
+      relayService.on(CloudTaskEvent.Update, (payload) => {
+        updates.push(payload);
+      });
+      mockNetFetch.mockResolvedValue(createJsonResponse({ result: {} }));
+      const { response, push } = createControllableSseResponse();
+      mockStreamFetch.mockResolvedValueOnce(response);
+      relayService.designateRelayedMcpServers("run-1", ["slack"]);
+      watchRun("run-1");
+      await waitFor(() => mockStreamFetch.mock.calls.length > 0);
+
+      // The harness (claude always-ask) prompt for the same call arrives first.
+      push(
+        `data: ${JSON.stringify({
+          type: "permission_request",
+          requestId: "harness-req-1",
+          toolCall: {
+            toolCallId: "tc-1",
+            title: "The agent wants to call send_message (slack)",
+            kind: "other",
+            rawInput: {
+              channel: "#general",
+              toolName: "mcp__slack__send_message",
+            },
+          },
+          options: [
+            { kind: "allow_once", name: "Yes", optionId: "allow" },
+            {
+              kind: "allow_always",
+              name: "Yes, always allow",
+              optionId: "allow_always",
+            },
+            { kind: "reject_once", name: "No", optionId: "reject" },
+          ],
+        })}\n\n`,
+      );
+      await waitFor(() => lastPermissionRequestUpdate(updates) !== undefined);
+
+      // The user answers it in the task view; the response routes through
+      // sendCommand, granting a consume-once pass for the identical call.
+      await relayService.sendCommand({
+        taskId: "task-1",
+        runId: "run-1",
+        apiHost: "https://app.example.com",
+        teamId: 2,
+        method: "permission_response",
+        params: { requestId: "harness-req-1", optionId: "allow" },
+      });
+
+      const updatesBeforeCall = updates.length;
+      push(mcpRequestSseLine({ payload: toolsCallPayload() }));
+      await waitFor(() => mcpRelayExecutor.execute.mock.calls.length > 0);
+
+      // No desktop prompt was raised for the relayed call itself.
+      expect(
+        updates
+          .slice(updatesBeforeCall)
+          .filter(
+            (u) => (u as { kind?: string }).kind === "permission_request",
+          ),
+      ).toEqual([]);
+
+      // The pass was consume-once: an identical unattested call prompts again.
+      push(
+        mcpRequestSseLine({
+          requestId: "req-2",
+          payload: toolsCallPayload(),
+        }),
+      );
+      await waitFor(
+        () =>
+          updates
+            .slice(updatesBeforeCall)
+            .filter(
+              (u) => (u as { kind?: string }).kind === "permission_request",
+            ).length > 0,
+      );
+      expect(mcpRelayExecutor.execute).toHaveBeenCalledOnce();
+    });
+
+    it("an always-allow answer covers subsequent calls to the same tool", async () => {
+      const updates: unknown[] = [];
+      relayService.on(CloudTaskEvent.Update, (payload) => {
+        updates.push(payload);
+      });
+      mockNetFetch.mockResolvedValue(createJsonResponse({ result: {} }));
+      const { response, push } = createControllableSseResponse();
+      mockStreamFetch.mockResolvedValueOnce(response);
+      relayService.designateRelayedMcpServers("run-1", ["slack"]);
+      watchRun("run-1");
+      await waitFor(() => mockStreamFetch.mock.calls.length > 0);
+
+      push(mcpRequestSseLine({ payload: toolsCallPayload() }));
+      await waitFor(() => lastPermissionRequestUpdate(updates) !== undefined);
+      const prompt = lastPermissionRequestUpdate(updates);
+      await relayService.sendCommand({
+        taskId: "task-1",
+        runId: "run-1",
+        apiHost: "https://app.example.com",
+        teamId: 2,
+        method: "permission_response",
+        params: { requestId: prompt?.requestId, optionId: "allow_always" },
+      });
+      await waitFor(() => mcpRelayExecutor.execute.mock.calls.length === 1);
+
+      const updatesAfterFirst = updates.length;
+      // Different arguments — always-allow is per tool, not per exact call.
+      push(
+        mcpRequestSseLine({
+          requestId: "req-2",
+          payload: toolsCallPayload({ channel: "#random" }),
+        }),
+      );
+      await waitFor(() => mcpRelayExecutor.execute.mock.calls.length === 2);
+      expect(
+        updates
+          .slice(updatesAfterFirst)
+          .filter(
+            (u) => (u as { kind?: string }).kind === "permission_request",
+          ),
+      ).toEqual([]);
+    });
+  });
 });
diff --git a/packages/core/src/cloud-task/cloud-task.ts b/packages/core/src/cloud-task/cloud-task.ts
