From 3cb1a56bfc9579a0f2336f9cfa12a8a744332a19 Mon Sep 17 00:00:00 2001 From: Devin Robison Date: Tue, 21 Apr 2026 14:39:48 -0600 Subject: [PATCH] fix(gateway): derive loopback owner context from token (#69796) * fix(gateway): derive loopback owner context from token * docs(changelog): note loopback owner token hardening * refactor(gateway): clarify loopback runtime cleanup * fix(gateway): compare both loopback bearer classes --- CHANGELOG.md | 1 + src/agents/cli-runner/bundle-mcp.test.ts | 7 +-- src/agents/cli-runner/prepare.ts | 6 ++- .../gateway-cli-backend.live-helpers.ts | 4 +- src/gateway/mcp-http.loopback-runtime.ts | 15 ++++-- src/gateway/mcp-http.request.ts | 40 +++++++------- src/gateway/mcp-http.runtime.ts | 4 +- src/gateway/mcp-http.test.ts | 54 ++++++++++++++----- src/gateway/mcp-http.ts | 15 +++--- 9 files changed, 91 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39a40c92ed4..ca8e4e5a24e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Docs: https://docs.openclaw.ai - Synology Chat: validate outbound webhook `file_url` values against the shared SSRF policy before forwarding to the NAS, rejecting malformed URLs, non-`http(s)` schemes, and private/blocked network targets so the NAS cannot be used as a confused deputy to fetch internal addresses. (#69784) Thanks @eleqtrizit. - Gateway/Control UI: require gateway auth on the Control UI avatar route (`GET /avatar/` and `?meta=1` metadata) when auth is configured, matching the sibling assistant-media route, and propagate the existing gateway token through the UI avatar fetch (bearer header + authenticated blob URL) so authenticated dashboards still load local avatars. (#69775) - Exec/allowlist: reject POSIX parameter expansion forms such as `$VAR`, `$?`, `$$`, `$1`, and `$@` inside unquoted heredocs during shell approval analysis, so these heredocs no longer pass allowlist review as plain text. (#69795) Thanks @drobison00. +- Gateway/MCP loopback: derive owner-only tool visibility from distinct authenticated owner vs non-owner loopback bearers instead of the caller-controlled owner header, so non-owner MCP child processes cannot recover owner access by spoofing request metadata. (#69796) ## 2026.4.20 diff --git a/src/agents/cli-runner/bundle-mcp.test.ts b/src/agents/cli-runner/bundle-mcp.test.ts index 25547055ed3..20b62a9c66b 100644 --- a/src/agents/cli-runner/bundle-mcp.test.ts +++ b/src/agents/cli-runner/bundle-mcp.test.ts @@ -204,14 +204,12 @@ describe("prepareCliBundleMcpConfig", () => { env: { OPENCLAW_MCP_TOKEN: "loopback-token-123", OPENCLAW_MCP_SESSION_KEY: "agent:main:telegram:group:chat123", - OPENCLAW_MCP_SENDER_IS_OWNER: "false", }, }); expect(prepared.env).toEqual({ OPENCLAW_MCP_TOKEN: "loopback-token-123", OPENCLAW_MCP_SESSION_KEY: "agent:main:telegram:group:chat123", - OPENCLAW_MCP_SENDER_IS_OWNER: "false", }); await prepared.cleanup?.(); @@ -250,7 +248,6 @@ describe("prepareCliBundleMcpConfig", () => { headers: { Authorization: "Bearer ${OPENCLAW_MCP_TOKEN}", "x-session-key": "${OPENCLAW_MCP_SESSION_KEY}", - "x-openclaw-sender-is-owner": "${OPENCLAW_MCP_SENDER_IS_OWNER}", }, }, }, @@ -261,14 +258,14 @@ describe("prepareCliBundleMcpConfig", () => { "exec", "--json", "-c", - 'mcp_servers={ openclaw = { url = "http://127.0.0.1:23119/mcp", bearer_token_env_var = "OPENCLAW_MCP_TOKEN", env_http_headers = { x-session-key = "OPENCLAW_MCP_SESSION_KEY", x-openclaw-sender-is-owner = "OPENCLAW_MCP_SENDER_IS_OWNER" } } }', + 'mcp_servers={ openclaw = { url = "http://127.0.0.1:23119/mcp", bearer_token_env_var = "OPENCLAW_MCP_TOKEN", env_http_headers = { x-session-key = "OPENCLAW_MCP_SESSION_KEY" } } }', ]); expect(prepared.backend.resumeArgs).toEqual([ "exec", "resume", "{sessionId}", "-c", - 'mcp_servers={ openclaw = { url = "http://127.0.0.1:23119/mcp", bearer_token_env_var = "OPENCLAW_MCP_TOKEN", env_http_headers = { x-session-key = "OPENCLAW_MCP_SESSION_KEY", x-openclaw-sender-is-owner = "OPENCLAW_MCP_SENDER_IS_OWNER" } } }', + 'mcp_servers={ openclaw = { url = "http://127.0.0.1:23119/mcp", bearer_token_env_var = "OPENCLAW_MCP_TOKEN", env_http_headers = { x-session-key = "OPENCLAW_MCP_SESSION_KEY" } } }', ]); expect(prepared.cleanup).toBeUndefined(); }); diff --git a/src/agents/cli-runner/prepare.ts b/src/agents/cli-runner/prepare.ts index 28b17ad5010..4edbe220fd3 100644 --- a/src/agents/cli-runner/prepare.ts +++ b/src/agents/cli-runner/prepare.ts @@ -168,12 +168,14 @@ export async function prepareCliRunContext( : undefined, env: mcpLoopbackRuntime ? { - OPENCLAW_MCP_TOKEN: mcpLoopbackRuntime.token, + OPENCLAW_MCP_TOKEN: + params.senderIsOwner === true + ? mcpLoopbackRuntime.ownerToken + : mcpLoopbackRuntime.nonOwnerToken, OPENCLAW_MCP_AGENT_ID: sessionAgentId ?? "", OPENCLAW_MCP_ACCOUNT_ID: params.agentAccountId ?? "", OPENCLAW_MCP_SESSION_KEY: params.sessionKey ?? "", OPENCLAW_MCP_MESSAGE_CHANNEL: params.messageProvider ?? "", - OPENCLAW_MCP_SENDER_IS_OWNER: params.senderIsOwner === true ? "true" : "false", } : undefined, warn: (message) => cliBackendLog.warn(message), diff --git a/src/gateway/gateway-cli-backend.live-helpers.ts b/src/gateway/gateway-cli-backend.live-helpers.ts index 296d2f162f1..e61122259b7 100644 --- a/src/gateway/gateway-cli-backend.live-helpers.ts +++ b/src/gateway/gateway-cli-backend.live-helpers.ts @@ -28,6 +28,7 @@ import { } from "./live-agent-probes.js"; import { renderCatFacePngBase64 } from "./live-image-probe.js"; import { getActiveMcpLoopbackRuntime } from "./mcp-http.js"; +import { resolveMcpLoopbackBearerToken } from "./mcp-http.loopback-runtime.js"; import { extractPayloadText } from "./test-helpers.agent-results.js"; // Aggregate docker live runs can contend on startup enough that the gateway @@ -261,10 +262,9 @@ async function callLoopbackJsonRpc(params: { throw new Error("mcp loopback runtime is not active"); } const headers: Record = { - Authorization: `Bearer ${runtime.token}`, + Authorization: `Bearer ${resolveMcpLoopbackBearerToken(runtime, params.senderIsOwner)}`, "Content-Type": "application/json", "x-session-key": params.sessionKey, - "x-openclaw-sender-is-owner": params.senderIsOwner ? "true" : "false", }; if (params.messageProvider) { headers["x-openclaw-message-channel"] = params.messageProvider; diff --git a/src/gateway/mcp-http.loopback-runtime.ts b/src/gateway/mcp-http.loopback-runtime.ts index 3d274a6a073..7fff9071f13 100644 --- a/src/gateway/mcp-http.loopback-runtime.ts +++ b/src/gateway/mcp-http.loopback-runtime.ts @@ -1,6 +1,7 @@ export type McpLoopbackRuntime = { port: number; - token: string; + ownerToken: string; + nonOwnerToken: string; }; let activeRuntime: McpLoopbackRuntime | undefined; @@ -13,8 +14,15 @@ export function setActiveMcpLoopbackRuntime(runtime: McpLoopbackRuntime): void { activeRuntime = { ...runtime }; } -export function clearActiveMcpLoopbackRuntime(token: string): void { - if (activeRuntime?.token === token) { +export function resolveMcpLoopbackBearerToken( + runtime: McpLoopbackRuntime, + senderIsOwner: boolean, +): string { + return senderIsOwner ? runtime.ownerToken : runtime.nonOwnerToken; +} + +export function clearActiveMcpLoopbackRuntimeByOwnerToken(ownerToken: string): void { + if (activeRuntime?.ownerToken === ownerToken) { activeRuntime = undefined; } } @@ -31,7 +39,6 @@ export function createMcpLoopbackServerConfig(port: number) { "x-openclaw-agent-id": "${OPENCLAW_MCP_AGENT_ID}", "x-openclaw-account-id": "${OPENCLAW_MCP_ACCOUNT_ID}", "x-openclaw-message-channel": "${OPENCLAW_MCP_MESSAGE_CHANNEL}", - "x-openclaw-sender-is-owner": "${OPENCLAW_MCP_SENDER_IS_OWNER}", }, }, }, diff --git a/src/gateway/mcp-http.request.ts b/src/gateway/mcp-http.request.ts index 412297eb23f..f97c726c668 100644 --- a/src/gateway/mcp-http.request.ts +++ b/src/gateway/mcp-http.request.ts @@ -3,10 +3,7 @@ import { resolveMainSessionKey } from "../config/sessions.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { isTruthyEnvValue } from "../infra/env.js"; import { safeEqualSecret } from "../security/secret-equal.js"; -import { - normalizeOptionalLowercaseString, - normalizeOptionalString, -} from "../shared/string-coerce.js"; +import { normalizeOptionalString } from "../shared/string-coerce.js"; import { normalizeMessageChannel } from "../utils/message-channel.js"; import { getHeader } from "./http-utils.js"; import { isLoopbackAddress } from "./net.js"; @@ -32,7 +29,7 @@ export type McpRequestContext = { sessionKey: string; messageProvider: string | undefined; accountId: string | undefined; - senderIsOwner: boolean | undefined; + senderIsOwner: boolean; }; function resolveScopedSessionKey(cfg: OpenClawConfig, rawSessionKey: string | undefined): string { @@ -66,8 +63,9 @@ function rejectsBrowserLoopbackRequest(req: IncomingMessage): boolean { export function validateMcpLoopbackRequest(params: { req: IncomingMessage; res: ServerResponse; - token: string; -}): boolean { + ownerToken: string; + nonOwnerToken: string; +}): { senderIsOwner: boolean } | null { let url: URL; try { url = new URL(params.req.url ?? "/", `http://${params.req.headers.host ?? "localhost"}`); @@ -75,13 +73,13 @@ export function validateMcpLoopbackRequest(params: { logMcpLoopbackHttp("reject", { reason: "bad_request_url", method: params.req.method ?? "" }); params.res.writeHead(400, { "Content-Type": "application/json" }); params.res.end(JSON.stringify({ error: "bad_request" })); - return false; + return null; } if (params.req.method === "GET" && url.pathname.startsWith("/.well-known/")) { params.res.writeHead(404); params.res.end(); - return false; + return null; } if (url.pathname !== "/mcp") { @@ -92,7 +90,7 @@ export function validateMcpLoopbackRequest(params: { }); params.res.writeHead(404, { "Content-Type": "application/json" }); params.res.end(JSON.stringify({ error: "not_found" })); - return false; + return null; } if (params.req.method !== "POST") { @@ -103,7 +101,7 @@ export function validateMcpLoopbackRequest(params: { }); params.res.writeHead(405, { Allow: "POST" }); params.res.end(); - return false; + return null; } if (rejectsBrowserLoopbackRequest(params.req)) { @@ -114,11 +112,14 @@ export function validateMcpLoopbackRequest(params: { }); params.res.writeHead(403, { "Content-Type": "application/json" }); params.res.end(JSON.stringify({ error: "forbidden" })); - return false; + return null; } const authHeader = getHeader(params.req, "authorization") ?? ""; - if (!safeEqualSecret(authHeader, `Bearer ${params.token}`)) { + const ownerTokenMatched = safeEqualSecret(authHeader, `Bearer ${params.ownerToken}`); + const nonOwnerTokenMatched = safeEqualSecret(authHeader, `Bearer ${params.nonOwnerToken}`); + const senderIsOwner = ownerTokenMatched ? true : nonOwnerTokenMatched ? false : null; + if (senderIsOwner === null) { logMcpLoopbackHttp("reject", { reason: "unauthorized", method: params.req.method ?? "", @@ -126,7 +127,7 @@ export function validateMcpLoopbackRequest(params: { }); params.res.writeHead(401, { "Content-Type": "application/json" }); params.res.end(JSON.stringify({ error: "unauthorized" })); - return false; + return null; } const contentType = getHeader(params.req, "content-type") ?? ""; @@ -138,10 +139,10 @@ export function validateMcpLoopbackRequest(params: { }); params.res.writeHead(415, { "Content-Type": "application/json" }); params.res.end(JSON.stringify({ error: "unsupported_media_type" })); - return false; + return null; } - return true; + return { senderIsOwner }; } export async function readMcpHttpBody(req: IncomingMessage): Promise { @@ -165,16 +166,13 @@ export async function readMcpHttpBody(req: IncomingMessage): Promise { export function resolveMcpRequestContext( req: IncomingMessage, cfg: OpenClawConfig, + auth: { senderIsOwner: boolean }, ): McpRequestContext { - const senderIsOwnerRaw = normalizeOptionalLowercaseString( - getHeader(req, "x-openclaw-sender-is-owner"), - ); return { sessionKey: resolveScopedSessionKey(cfg, getHeader(req, "x-session-key")), messageProvider: normalizeMessageChannel(getHeader(req, "x-openclaw-message-channel")) ?? undefined, accountId: normalizeOptionalString(getHeader(req, "x-openclaw-account-id")), - senderIsOwner: - senderIsOwnerRaw === "true" ? true : senderIsOwnerRaw === "false" ? false : undefined, + senderIsOwner: auth.senderIsOwner, }; } diff --git a/src/gateway/mcp-http.runtime.ts b/src/gateway/mcp-http.runtime.ts index 23e8d006608..fadf34b43f5 100644 --- a/src/gateway/mcp-http.runtime.ts +++ b/src/gateway/mcp-http.runtime.ts @@ -1,6 +1,6 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import { - clearActiveMcpLoopbackRuntime, + clearActiveMcpLoopbackRuntimeByOwnerToken, createMcpLoopbackServerConfig, getActiveMcpLoopbackRuntime, setActiveMcpLoopbackRuntime, @@ -70,7 +70,7 @@ export class McpLoopbackToolCache { } export { - clearActiveMcpLoopbackRuntime, + clearActiveMcpLoopbackRuntimeByOwnerToken, createMcpLoopbackServerConfig, getActiveMcpLoopbackRuntime, setActiveMcpLoopbackRuntime, diff --git a/src/gateway/mcp-http.test.ts b/src/gateway/mcp-http.test.ts index 6f8b4e2a47e..5670bb74bc0 100644 --- a/src/gateway/mcp-http.test.ts +++ b/src/gateway/mcp-http.test.ts @@ -34,6 +34,7 @@ import { createMcpLoopbackServerConfig, closeMcpLoopbackServer, getActiveMcpLoopbackRuntime, + resolveMcpLoopbackBearerToken, ensureMcpLoopbackServer, startMcpLoopbackServer, } from "./mcp-http.js"; @@ -89,7 +90,7 @@ describe("mcp loopback server", () => { const response = await sendRaw({ port: server.port, - token: runtime?.token, + token: runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined, headers: { "content-type": "application/json", "x-session-key": "agent:main:telegram:group:chat123", @@ -105,13 +106,13 @@ describe("mcp loopback server", () => { sessionKey: "agent:main:telegram:group:chat123", accountId: "work", messageProvider: "telegram", - senderIsOwner: undefined, + senderIsOwner: false, surface: "loopback", }), ); }); - it("threads senderIsOwner through loopback request context and cache separation", async () => { + it("derives senderIsOwner from the loopback bearer token", async () => { server = await startMcpLoopbackServer(0); const activeServer = server; const runtime = getActiveMcpLoopbackRuntime(); @@ -119,12 +120,13 @@ describe("mcp loopback server", () => { const sendToolsList = async (senderIsOwner: "true" | "false") => await sendRaw({ port: activeServer.port, - token: runtime?.token, + token: runtime + ? resolveMcpLoopbackBearerToken(runtime, senderIsOwner === "true") + : undefined, headers: { "content-type": "application/json", "x-session-key": "agent:main:matrix:dm:test", "x-openclaw-message-channel": "matrix", - "x-openclaw-sender-is-owner": senderIsOwner, }, body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list" }), }); @@ -153,11 +155,39 @@ describe("mcp loopback server", () => { ); }); + it("ignores spoofed owner headers when the bearer token is non-owner scoped", async () => { + 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:matrix:dm:test", + "x-openclaw-message-channel": "matrix", + "x-openclaw-sender-is-owner": "true", + }, + body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list" }), + }); + + expect(response.status).toBe(200); + expect(resolveGatewayScopedToolsMock).toHaveBeenCalledWith( + expect.objectContaining({ + sessionKey: "agent:main:matrix:dm:test", + messageProvider: "matrix", + senderIsOwner: false, + surface: "loopback", + }), + ); + }); + it("tracks the active runtime only while the server is running", async () => { server = await startMcpLoopbackServer(0); const active = getActiveMcpLoopbackRuntime(); expect(active?.port).toBe(server.port); - expect(active?.token).toMatch(/^[0-9a-f]{64}$/); + expect(active?.ownerToken).toMatch(/^[0-9a-f]{64}$/); + expect(active?.nonOwnerToken).toMatch(/^[0-9a-f]{64}$/); await server.close(); server = undefined; @@ -192,7 +222,7 @@ describe("mcp loopback server", () => { const runtime = getActiveMcpLoopbackRuntime(); const response = await sendRaw({ port: server.port, - token: runtime?.token, + token: runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined, headers: { "content-type": "text/plain" }, body: "{}", }); @@ -233,7 +263,7 @@ describe("mcp loopback server", () => { const runtime = getActiveMcpLoopbackRuntime(); const response = await sendRaw({ port: server.port, - token: runtime?.token, + token: runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined, headers: { "content-type": "application/json", origin: "http://127.0.0.1:43123", @@ -249,7 +279,7 @@ describe("mcp loopback server", () => { const runtime = getActiveMcpLoopbackRuntime(); const response = await sendRaw({ port: server.port, - token: runtime?.token, + token: runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined, headers: { "content-type": "application/json", origin: `http://127.0.0.1:${server.port}`, @@ -272,7 +302,7 @@ describe("mcp loopback server", () => { const runtime = getActiveMcpLoopbackRuntime(); const response = await sendRaw({ port: server.port, - token: runtime?.token, + token: runtime ? resolveMcpLoopbackBearerToken(runtime, false) : undefined, headers: { "content-type": "application/json", origin: "http://localhost:43123", @@ -297,8 +327,6 @@ describe("createMcpLoopbackServerConfig", () => { expect(config.mcpServers?.openclaw?.headers?.["x-openclaw-message-channel"]).toBe( "${OPENCLAW_MCP_MESSAGE_CHANNEL}", ); - expect(config.mcpServers?.openclaw?.headers?.["x-openclaw-sender-is-owner"]).toBe( - "${OPENCLAW_MCP_SENDER_IS_OWNER}", - ); + expect(config.mcpServers?.openclaw?.headers?.["x-openclaw-sender-is-owner"]).toBeUndefined(); }); }); diff --git a/src/gateway/mcp-http.ts b/src/gateway/mcp-http.ts index 2f8c38de05f..2e0fd915ffa 100644 --- a/src/gateway/mcp-http.ts +++ b/src/gateway/mcp-http.ts @@ -6,7 +6,7 @@ import { formatErrorMessage } from "../infra/errors.js"; import { logDebug, logWarn } from "../logger.js"; import { handleMcpJsonRpc } from "./mcp-http.handlers.js"; import { - clearActiveMcpLoopbackRuntime, + clearActiveMcpLoopbackRuntimeByOwnerToken, createMcpLoopbackServerConfig, getActiveMcpLoopbackRuntime, setActiveMcpLoopbackRuntime, @@ -22,6 +22,7 @@ import { McpLoopbackToolCache } from "./mcp-http.runtime.js"; export { createMcpLoopbackServerConfig, getActiveMcpLoopbackRuntime, + resolveMcpLoopbackBearerToken, } from "./mcp-http.loopback-runtime.js"; type McpLoopbackServer = { @@ -54,11 +55,13 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ port: number; close: () => Promise; }> { - const token = crypto.randomBytes(32).toString("hex"); + const ownerToken = crypto.randomBytes(32).toString("hex"); + const nonOwnerToken = crypto.randomBytes(32).toString("hex"); const toolCache = new McpLoopbackToolCache(); const httpServer = createHttpServer((req, res) => { - if (!validateMcpLoopbackRequest({ req, res, token })) { + const auth = validateMcpLoopbackRequest({ req, res, ownerToken, nonOwnerToken }); + if (!auth) { return; } @@ -67,7 +70,7 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ const body = await readMcpHttpBody(req); const parsed: JsonRpcRequest | JsonRpcRequest[] = JSON.parse(body); const cfg = loadConfig(); - const requestContext = resolveMcpRequestContext(req, cfg); + const requestContext = resolveMcpRequestContext(req, cfg, auth); const scopedTools = toolCache.resolve({ cfg, sessionKey: requestContext.sessionKey, @@ -144,7 +147,7 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ if (!address || typeof address === "string") { throw new Error("mcp loopback did not bind to a TCP port"); } - setActiveMcpLoopbackRuntime({ port: address.port, token }); + setActiveMcpLoopbackRuntime({ port: address.port, ownerToken, nonOwnerToken }); logDebug(`mcp loopback listening on 127.0.0.1:${address.port}`); const server: McpLoopbackServer = { @@ -153,7 +156,7 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ new Promise((resolve, reject) => { httpServer.close((error) => { if (!error) { - clearActiveMcpLoopbackRuntime(token); + clearActiveMcpLoopbackRuntimeByOwnerToken(ownerToken); if (activeMcpLoopbackServer === server) { activeMcpLoopbackServer = undefined; }