fix(gateway): enforce owner-only tool policy and before-tool-call hook on MCP loopback surface (#71159)

mmaps · drobison00 · web-flow · commit 8b76392e3e79 · 2026-04-24T13:16:45.000-06:00
* fix: address issue * fix: address review feedback * fix: address PR review feedback * changelog: PR #71159 MCP loopback owner-only policy + before-tool-call hook --------- Co-authored-by: Devin Robison <drobison@nvidia.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -149,6 +149,7 @@ Docs: https://docs.openclaw.ai
 - Webhooks/security: re-resolve `SecretRef`-backed webhook route secrets on each request so `openclaw secrets reload` revokes the previous secret immediately instead of waiting for a gateway restart. (#70727) Thanks @drobison00.
 - Memory/dreaming: decouple the managed dreaming cron from heartbeat by running it as an isolated lightweight agent turn, so dreaming runs even when heartbeat is disabled for the default agent and is no longer skipped by `heartbeat.activeHours`. `openclaw doctor --fix` migrates stale main-session dreaming jobs in persisted cron configs to the new shape. Fixes #69811, #67397, #68972. (#70737) Thanks @jalehman.
 - Agents/CLI: keep `--agent` plus `--session-id` lookup scoped to the requested agent store, so explicit agent resumes cannot select another agent's session. (#70985) Thanks @frankekn.
+- Gateway/MCP loopback: apply owner-only tool policy and run before-tool-call hooks on `127.0.0.1/mcp` `tools/list` and `tools/call`, so non-owner bearer callers can no longer see or invoke owner-only tools such as `cron`, `gateway`, and `nodes`, matching the existing HTTP `/tools/invoke` and embedded-agent paths. (#71159) Thanks @mmaps.
 
 ## 2026.4.22
 
diff --git a/src/gateway/mcp-http.handlers.ts b/src/gateway/mcp-http.handlers.ts
@@ -1,4 +1,5 @@
 import crypto from "node:crypto";
+import { runBeforeToolCallHook, type HookContext } from "../agents/pi-tools.before-tool-call.js";
 import { formatErrorMessage } from "../infra/errors.js";
 import {
   MCP_LOOPBACK_SERVER_NAME,
@@ -35,6 +36,8 @@ export async function handleMcpJsonRpc(params: {
   message: JsonRpcRequest;
   tools: McpLoopbackTool[];
   toolSchema: McpToolSchemaEntry[];
+  hookContext?: HookContext;
+  signal?: AbortSignal;
 }): Promise<object | null> {
   const { id, method, params: methodParams } = params.message;
 
@@ -70,7 +73,20 @@ export async function handleMcpJsonRpc(params: {
       }
       const toolCallId = `mcp-${crypto.randomUUID()}`;
       try {
-        const result = await tool.execute(toolCallId, toolArgs);
+        const hookResult = await runBeforeToolCallHook({
+          toolName,
+          params: toolArgs,
+          toolCallId,
+          ctx: params.hookContext,
+          signal: params.signal,
+        });
+        if (hookResult.blocked) {
+          return jsonRpcResult(id, {
+            content: [{ type: "text", text: hookResult.reason }],
+            isError: true,
+          });
+        }
+        const result = await tool.execute(toolCallId, hookResult.params, params.signal);
         return jsonRpcResult(id, {
           content: normalizeToolCallContent(result),
           isError: false,
diff --git a/src/gateway/mcp-http.runtime.ts b/src/gateway/mcp-http.runtime.ts
@@ -1,3 +1,4 @@
+import { applyOwnerOnlyToolPolicy } from "../agents/tool-policy.js";
 import type { OpenClawConfig } from "../config/types.openclaw.js";
 import {
   clearActiveMcpLoopbackRuntimeByOwnerToken,
@@ -16,6 +17,7 @@ const TOOL_CACHE_TTL_MS = 30_000;
 const NATIVE_TOOL_EXCLUDE = new Set(["read", "write", "edit", "apply_patch", "exec", "process"]);
 
 type CachedScopedTools = {
+  agentId: string | undefined;
   tools: McpLoopbackTool[];
   toolSchema: McpToolSchemaEntry[];
   configRef: OpenClawConfig;
@@ -36,7 +38,7 @@ export class McpLoopbackToolCache {
       params.sessionKey,
       params.messageProvider ?? "",
       params.accountId ?? "",
-      params.senderIsOwner === true ? "owner" : params.senderIsOwner === false ? "non-owner" : "",
+      params.senderIsOwner === true ? "owner" : "non-owner",
     ].join("\u0000");
     const now = Date.now();
     const cached = this.#entries.get(cacheKey);
@@ -53,9 +55,11 @@ export class McpLoopbackToolCache {
       surface: "loopback",
       excludeToolNames: NATIVE_TOOL_EXCLUDE,
     });
+    const tools = applyOwnerOnlyToolPolicy(next.tools, params.senderIsOwner === true);
     const nextEntry: CachedScopedTools = {
-      tools: next.tools,
-      toolSchema: buildMcpToolSchema(next.tools),
+      agentId: next.agentId,
+      tools,
+      toolSchema: buildMcpToolSchema(tools),
       configRef: params.cfg,
       time: now,
     };
diff --git a/src/gateway/mcp-http.test.ts b/src/gateway/mcp-http.test.ts
@@ -5,6 +5,7 @@ type MockGatewayTool = {
   name: string;
   description: string;
   parameters: Record<string, unknown>;
+  ownerOnly?: boolean;
   execute: (...args: unknown[]) => Promise<{ content: Array<{ type: string; text: string }> }>;
 };
 
@@ -13,6 +14,19 @@ type MockGatewayScopedTools = {
   tools: MockGatewayTool[];
 };
 
+type MockBeforeToolCallHookResult =
+  | { blocked: true; reason: string }
+  | { blocked: false; params: unknown };
+
+const runBeforeToolCallHookMock = vi.hoisted(() =>
+  vi.fn(
+    async (args: { params: unknown }): Promise<MockBeforeToolCallHookResult> => ({
+      blocked: false,
+      params: args.params,
+    }),
+  ),
+);
+
 const resolveGatewayScopedToolsMock = vi.hoisted(() =>
   vi.fn<(...args: unknown[]) => MockGatewayScopedTools>(() => ({
     agentId: "main",
@@ -37,6 +51,11 @@ vi.mock("../config/sessions.js", () => ({
   resolveMainSessionKey: () => "agent:main:main",
 }));
 
+vi.mock("../agents/pi-tools.before-tool-call.js", () => ({
+  runBeforeToolCallHook: (...args: Parameters<typeof runBeforeToolCallHookMock>) =>
+    runBeforeToolCallHookMock(...args),
+}));
+
 vi.mock("./tool-resolution.js", () => ({
   resolveGatewayScopedTools: (...args: Parameters<typeof resolveGatewayScopedToolsMock>) =>
     resolveGatewayScopedToolsMock(...args),
@@ -71,6 +90,13 @@ async function sendRaw(params: {
 
 beforeEach(() => {
   resolveGatewayScopedToolsMock.mockClear();
+  runBeforeToolCallHookMock.mockClear();
+  runBeforeToolCallHookMock.mockImplementation(
+    async (args: { params: unknown }): Promise<MockBeforeToolCallHookResult> => ({
+      blocked: false,
+      params: args.params,
+    }),
+  );
   resolveGatewayScopedToolsMock.mockReturnValue({
     agentId: "main",
     tools: [
@@ -231,6 +257,256 @@ describe("mcp loopback server", () => {
     );
   });
 
+  it("filters owner-only tools from non-owner tool lists", async () => {
+    resolveGatewayScopedToolsMock.mockReturnValue({
+      agentId: "main",
+      tools: [
+        {
+          name: "message",
+          description: "send a message",
+          parameters: { type: "object", properties: {} },
+          execute: async () => ({
+            content: [{ type: "text", text: "ok" }],
+          }),
+        },
+        {
+          name: "cron",
+          description: "manage schedules",
+          parameters: { type: "object", properties: {} },
+          execute: async () => ({
+            content: [{ type: "text", text: "cron" }],
+          }),
+        },
+        {
+          name: "owner_probe",
+          description: "owner-only by flag",
+          parameters: { type: "object", properties: {} },
+          ownerOnly: true,
+          execute: async () => ({
+            content: [{ type: "text", text: "owner" }],
+          }),
+        },
+      ],
+    });
+    server = await startMcpLoopbackServer(0);
+    const runtime = getActiveMcpLoopbackRuntime();
+
+    const response = await sendRaw({
+      port: server.port,
+      token: runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined,
+      headers: {
+        "content-type": "application/json",
+        "x-session-key": "agent:main:main",
+      },
+      body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list" }),
+    });
+    const payload = (await response.json()) as {
+      result?: { tools?: Array<{ name: string }> };
+    };
+    const names = (payload.result?.tools ?? []).map((tool) => tool.name);
+
+    expect(response.status).toBe(200);
+    expect(names).toContain("message");
+    expect(names).not.toContain("cron");
+    expect(names).not.toContain("owner_probe");
+  });
+
+  it("keeps owner-only tools available to owner loopback callers", async () => {
+    resolveGatewayScopedToolsMock.mockReturnValue({
+      agentId: "main",
+      tools: [
+        {
+          name: "message",
+          description: "send a message",
+          parameters: { type: "object", properties: {} },
+          execute: async () => ({
+            content: [{ type: "text", text: "ok" }],
+          }),
+        },
+        {
+          name: "cron",
+          description: "manage schedules",
+          parameters: { type: "object", properties: {} },
+          execute: async () => ({
+            content: [{ type: "text", text: "cron" }],
+          }),
+        },
+      ],
+    });
+    server = await startMcpLoopbackServer(0);
+    const runtime = getActiveMcpLoopbackRuntime();
+
+    const response = await sendRaw({
+      port: server.port,
+      token: runtime ? resolveMcpLoopbackBearerToken(runtime, true) : undefined,
+      headers: {
+        "content-type": "application/json",
+        "x-session-key": "agent:main:main",
+      },
+      body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list" }),
+    });
+    const payload = (await response.json()) as {
+      result?: { tools?: Array<{ name: string }> };
+    };
+    const names = (payload.result?.tools ?? []).map((tool) => tool.name);
+
+    expect(response.status).toBe(200);
+    expect(names).toContain("message");
+    expect(names).toContain("cron");
+  });
+
+  it("does not execute owner-only tools for non-owner callers", async () => {
+    const cronExecute = vi.fn(async () => ({
+      content: [{ type: "text", text: "CRON_EXECUTED" }],
+    }));
+    resolveGatewayScopedToolsMock.mockReturnValue({
+      agentId: "main",
+      tools: [
+        {
+          name: "message",
+          description: "send a message",
+          parameters: { type: "object", properties: {} },
+          execute: async () => ({
+            content: [{ type: "text", text: "ok" }],
+          }),
+        },
+        {
+          name: "cron",
+          description: "manage schedules",
+          parameters: { type: "object", properties: {} },
+          execute: cronExecute,
+        },
+      ],
+    });
+    server = await startMcpLoopbackServer(0);
+    const runtime = getActiveMcpLoopbackRuntime();
+
+    const response = await sendRaw({
+      port: server.port,
+      token: runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined,
+      headers: {
+        "content-type": "application/json",
+        "x-session-key": "agent:main:main",
+      },
+      body: JSON.stringify({
+        jsonrpc: "2.0",
+        id: 1,
+        method: "tools/call",
+        params: { name: "cron", arguments: {} },
+      }),
+    });
+    const payload = (await response.json()) as {
+      result?: { content?: Array<{ text?: string }>; isError?: boolean };
+    };
+
+    expect(response.status).toBe(200);
+    expect(cronExecute).not.toHaveBeenCalled();
+    expect(payload.result?.isError).toBe(true);
+    expect(payload.result?.content?.[0]?.text).toBe("Tool not available: cron");
+  });
+
+  it("honors before-tool-call hook blocks before loopback tool execution", async () => {
+    const execute = vi.fn(async () => ({
+      content: [{ type: "text", text: "EXECUTED" }],
+    }));
+    runBeforeToolCallHookMock.mockResolvedValueOnce({
+      blocked: true,
+      reason: "blocked by hook",
+    });
+    resolveGatewayScopedToolsMock.mockReturnValue({
+      agentId: "main",
+      tools: [
+        {
+          name: "message",
+          description: "send a message",
+          parameters: { type: "object", properties: {} },
+          execute,
+        },
+      ],
+    });
+    server = await startMcpLoopbackServer(0);
+    const runtime = getActiveMcpLoopbackRuntime();
+
+    const response = await sendRaw({
+      port: server.port,
+      token: runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined,
+      headers: {
+        "content-type": "application/json",
+        "x-session-key": "agent:main:main",
+      },
+      body: JSON.stringify({
+        jsonrpc: "2.0",
+        id: 1,
+        method: "tools/call",
+        params: { name: "message", arguments: { body: "hello" } },
+      }),
+    });
+    const payload = (await response.json()) as {
+      result?: { content?: Array<{ text?: string }>; isError?: boolean };
+    };
+
+    expect(response.status).toBe(200);
+    expect(runBeforeToolCallHookMock).toHaveBeenCalledWith(
+      expect.objectContaining({
+        toolName: "message",
+        params: { body: "hello" },
+        ctx: expect.objectContaining({
+          agentId: "main",
+          sessionKey: "agent:main:main",
+        }),
+        signal: expect.any(AbortSignal),
+      }),
+    );
+    expect(execute).not.toHaveBeenCalled();
+    expect(payload.result?.isError).toBe(true);
+    expect(payload.result?.content?.[0]?.text).toBe("blocked by hook");
+  });
+
+  it("forwards the request abort signal to loopback tool execution", async () => {
+    const execute = vi.fn(async () => ({
+      content: [{ type: "text", text: "EXECUTED" }],
+    }));
+    resolveGatewayScopedToolsMock.mockReturnValue({
+      agentId: "main",
+      tools: [
+        {
+          name: "message",
+          description: "send a message",
+          parameters: { type: "object", properties: {} },
+          execute,
+        },
+      ],
+    });
+    server = await startMcpLoopbackServer(0);
+    const runtime = getActiveMcpLoopbackRuntime();
+
+    const response = await sendRaw({
+      port: server.port,
+      token: runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined,
+      headers: {
+        "content-type": "application/json",
+        "x-session-key": "agent:main:main",
+      },
+      body: JSON.stringify({
+        jsonrpc: "2.0",
+        id: 1,
+        method: "tools/call",
+        params: { name: "message", arguments: { body: "hello" } },
+      }),
+    });
+    const payload = (await response.json()) as {
+      result?: { isError?: boolean };
+    };
+
+    expect(response.status).toBe(200);
+    expect(payload.result?.isError).toBe(false);
+    expect(execute).toHaveBeenCalledWith(
+      expect.stringMatching(/^mcp-/),
+      { body: "hello" },
+      expect.any(AbortSignal),
+    );
+  });
+
   it("tracks the active runtime only while the server is running", async () => {
     server = await startMcpLoopbackServer(0);
     const active = getActiveMcpLoopbackRuntime();
diff --git a/src/gateway/mcp-http.ts b/src/gateway/mcp-http.ts
