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

* 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>
This commit is contained in:
Michael Appel
2026-04-24 15:16:45 -04:00
committed by GitHub
parent 8cae2ed645
commit 8b76392e3e
5 changed files with 345 additions and 5 deletions

View File

@@ -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

View File

@@ -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,

View File

@@ -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,
};

View File

@@ -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();

View File

@@ -1,5 +1,9 @@
import crypto from "node:crypto";
import { createServer as createHttpServer } from "node:http";
import {
createServer as createHttpServer,
type IncomingMessage,
type ServerResponse,
} from "node:http";
import { loadConfig } from "../config/config.js";
import { isTruthyEnvValue } from "../infra/env.js";
import { formatErrorMessage } from "../infra/errors.js";
@@ -51,6 +55,37 @@ function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === "object" && value !== null && !Array.isArray(value);
}
function createRequestAbortSignal(req: IncomingMessage, res: ServerResponse) {
const controller = new AbortController();
const abort = () => {
if (!controller.signal.aborted) {
controller.abort();
}
};
const abortIfRequestIncomplete = () => {
if (!req.complete) {
abort();
}
};
const abortIfResponseStillOpen = () => {
if (!res.writableEnded) {
abort();
}
};
req.once("close", abortIfRequestIncomplete);
res.once("close", abortIfResponseStillOpen);
if (req.destroyed && !req.complete) {
abort();
}
return {
signal: controller.signal,
cleanup: () => {
req.off("close", abortIfRequestIncomplete);
res.off("close", abortIfResponseStillOpen);
},
};
}
export async function startMcpLoopbackServer(port = 0): Promise<{
port: number;
close: () => Promise<void>;
@@ -65,6 +100,7 @@ export async function startMcpLoopbackServer(port = 0): Promise<{
return;
}
const requestAbort = createRequestAbortSignal(req, res);
void (async () => {
try {
const body = await readMcpHttpBody(req);
@@ -94,6 +130,11 @@ export async function startMcpLoopbackServer(port = 0): Promise<{
message,
tools: scopedTools.tools,
toolSchema: scopedTools.toolSchema,
hookContext: {
agentId: scopedTools.agentId,
sessionKey: requestContext.sessionKey,
},
signal: requestAbort.signal,
});
if (response !== null) {
const toolName =
@@ -131,6 +172,8 @@ export async function startMcpLoopbackServer(port = 0): Promise<{
res.writeHead(400, { "Content-Type": "application/json" });
res.end(JSON.stringify(jsonRpcError(null, -32700, "Parse error")));
}
} finally {
requestAbort.cleanup();
}
})();
});