From 29cb1e3c7edd54a3d060419ffa153eecbd19c133 Mon Sep 17 00:00:00 2001 From: Jacob Tomlinson Date: Mon, 30 Mar 2026 08:59:40 -0700 Subject: [PATCH] Gateway: tighten HTTP tool invoke authorization (#57773) * Gateway: harden HTTP tool invoke access * Gateway: strengthen HTTP tools invoke regression coverage * Gateway: keep owner-only tools off HTTP --- .../tools-invoke-http.cron-regression.test.ts | 30 +++-- src/gateway/tools-invoke-http.test.ts | 109 ++++++++++++++++-- src/gateway/tools-invoke-http.ts | 25 +++- src/security/dangerous-tools.ts | 16 +++ 4 files changed, 163 insertions(+), 17 deletions(-) diff --git a/src/gateway/tools-invoke-http.cron-regression.test.ts b/src/gateway/tools-invoke-http.cron-regression.test.ts index dfee9be2c20..aae048c5788 100644 --- a/src/gateway/tools-invoke-http.cron-regression.test.ts +++ b/src/gateway/tools-invoke-http.cron-regression.test.ts @@ -3,6 +3,11 @@ import type { AddressInfo } from "node:net"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; const TEST_GATEWAY_TOKEN = "test-gateway-token-1234567890"; +const resolveToolLoopDetectionConfig = () => ({ warnAt: 3 }); +const runBeforeToolCallHook = async (args: { params: unknown }) => ({ + blocked: false as const, + params: args.params, +}); let cfg: Record = {}; const alwaysAuthorized = async () => ({ ok: true as const }); @@ -26,6 +31,14 @@ vi.mock("../logger.js", () => ({ logWarn: noWarnLog, })); +vi.mock("../agents/pi-tools.js", () => ({ + resolveToolLoopDetectionConfig, +})); + +vi.mock("../agents/pi-tools.before-tool-call.js", () => ({ + runBeforeToolCallHook, +})); + vi.mock("../plugins/config-state.js", () => ({ isTestDefaultMemorySlotDisabled: disableDefaultMemorySlot, })); @@ -91,12 +104,13 @@ beforeEach(() => { cfg = {}; }); -async function invoke(tool: string) { +async function invoke(tool: string, scopes = "operator.write") { return await fetch(`http://127.0.0.1:${port}/tools/invoke`, { method: "POST", headers: { "content-type": "application/json", authorization: `Bearer ${TEST_GATEWAY_TOKEN}`, + "x-openclaw-scopes": scopes, }, body: JSON.stringify({ tool, action: "status", args: {}, sessionKey: "main" }), }); @@ -105,13 +119,13 @@ async function invoke(tool: string) { describe("tools invoke HTTP denylist", () => { it("blocks cron and gateway by default", async () => { const gatewayRes = await invoke("gateway"); - const cronRes = await invoke("cron"); + const cronRes = await invoke("cron", "operator.admin"); expect(gatewayRes.status).toBe(404); expect(cronRes.status).toBe(404); }); - it("allows cron only when explicitly enabled in gateway.tools.allow", async () => { + it("keeps cron hidden even when explicitly enabled in gateway.tools.allow", async () => { cfg = { gateway: { tools: { @@ -120,12 +134,12 @@ describe("tools invoke HTTP denylist", () => { }, }; - const cronRes = await invoke("cron"); + const cronRes = await invoke("cron", "operator.admin"); - expect(cronRes.status).toBe(200); + expect(cronRes.status).toBe(404); }); - it("keeps cron available under coding profile without exposing gateway", async () => { + it("keeps cron and gateway hidden under the coding profile", async () => { cfg = { tools: { profile: "coding", @@ -137,10 +151,10 @@ describe("tools invoke HTTP denylist", () => { }, }; - const cronRes = await invoke("cron"); + const cronRes = await invoke("cron", "operator.admin"); const gatewayRes = await invoke("gateway"); - expect(cronRes.status).toBe(200); + expect(cronRes.status).toBe(404); expect(gatewayRes.status).toBe(404); }); }); diff --git a/src/gateway/tools-invoke-http.test.ts b/src/gateway/tools-invoke-http.test.ts index 96ede78ef00..cca11c2c6e8 100644 --- a/src/gateway/tools-invoke-http.test.ts +++ b/src/gateway/tools-invoke-http.test.ts @@ -114,6 +114,28 @@ vi.mock("../agents/openclaw-tools.js", () => { throw toolInputError("invalid args"); }, }, + { + name: "exec", + parameters: { type: "object", properties: {} }, + execute: async () => ({ ok: true, result: "exec" }), + }, + { + name: "apply_patch", + parameters: { type: "object", properties: {} }, + execute: async () => ({ ok: true, result: "apply_patch" }), + }, + { + name: "nodes", + ownerOnly: true, + parameters: { type: "object", properties: {} }, + execute: async () => ({ ok: true, result: "nodes" }), + }, + { + name: "owner_only_test", + ownerOnly: true, + parameters: { type: "object", properties: {} }, + execute: async () => ({ ok: true, result: "owner-only" }), + }, { name: "tools_invoke_test", parameters: { @@ -241,7 +263,14 @@ beforeEach(() => { }); const resolveGatewayToken = (): string => TEST_GATEWAY_TOKEN; -const gatewayAuthHeaders = () => ({ authorization: `Bearer ${resolveGatewayToken()}` }); +const gatewayAuthHeaders = () => ({ + authorization: `Bearer ${resolveGatewayToken()}`, + "x-openclaw-scopes": "operator.write", +}); +const gatewayAdminHeaders = () => ({ + authorization: `Bearer ${resolveGatewayToken()}`, + "x-openclaw-scopes": "operator.admin", +}); const allowAgentsListForMain = () => { cfg = { @@ -573,16 +602,14 @@ describe("POST /tools/invoke", () => { it("allows gateway tool via HTTP when explicitly enabled in gateway.tools.allow", async () => { setMainAllowedTools({ allow: ["gateway"], gatewayAllow: ["gateway"] }); - const res = await invokeToolAuthed({ + const res = await invokeTool({ + port: sharedPort, + headers: gatewayAdminHeaders(), tool: "gateway", sessionKey: "main", }); - // Ensure we didn't hit the HTTP deny list (404). Invalid args should map to 400. - expect(res.status).toBe(400); - const body = await res.json(); - expect(body.ok).toBe(false); - expect(body.error?.type).toBe("tool_error"); + expect(res.status).toBe(404); }); it("treats gateway.tools.deny as higher priority than gateway.tools.allow", async () => { @@ -685,4 +712,72 @@ describe("POST /tools/invoke", () => { expect(body.result?.observedFormat).toBe("pdf"); expect(body.result?.observedFileFormat).toBeUndefined(); }); + + it("requires operator.write scope for HTTP tool invocation", async () => { + allowAgentsListForMain(); + + const res = await invokeTool({ + port: sharedPort, + headers: { + authorization: `Bearer ${resolveGatewayToken()}`, + }, + tool: "agents_list", + sessionKey: "main", + }); + + expect(res.status).toBe(403); + await expect(res.json()).resolves.toMatchObject({ + ok: false, + error: { + type: "forbidden", + message: "missing scope: operator.write", + }, + }); + }); + + it("applies owner-only tool policy on the HTTP path", async () => { + setMainAllowedTools({ allow: ["owner_only_test"] }); + + const deniedRes = await invokeToolAuthed({ + tool: "owner_only_test", + sessionKey: "main", + }); + expect(deniedRes.status).toBe(404); + + const allowedRes = await invokeTool({ + port: sharedPort, + headers: gatewayAdminHeaders(), + tool: "owner_only_test", + sessionKey: "main", + }); + expect(allowedRes.status).toBe(404); + }); + + it("extends the HTTP deny list to high-risk execution and file tools", async () => { + setMainAllowedTools({ allow: ["exec", "apply_patch", "nodes"] }); + + const execRes = await invokeToolAuthed({ + tool: "exec", + sessionKey: "main", + }); + const patchRes = await invokeToolAuthed({ + tool: "apply_patch", + sessionKey: "main", + }); + const nodesRes = await invokeToolAuthed({ + tool: "nodes", + sessionKey: "main", + }); + const nodesAdminRes = await invokeTool({ + port: sharedPort, + headers: gatewayAdminHeaders(), + tool: "nodes", + sessionKey: "main", + }); + + expect(execRes.status).toBe(404); + expect(patchRes.status).toBe(404); + expect(nodesRes.status).toBe(404); + expect(nodesAdminRes.status).toBe(404); + }); }); diff --git a/src/gateway/tools-invoke-http.ts b/src/gateway/tools-invoke-http.ts index f9f97c5c9ee..43d08b28e1c 100644 --- a/src/gateway/tools-invoke-http.ts +++ b/src/gateway/tools-invoke-http.ts @@ -13,6 +13,7 @@ import { buildDefaultToolPolicyPipelineSteps, } from "../agents/tool-policy-pipeline.js"; import { + applyOwnerOnlyToolPolicy, collectExplicitAllowlist, mergeAlsoAllowPolicy, resolveToolProfilePolicy, @@ -28,7 +29,10 @@ import { DEFAULT_GATEWAY_HTTP_TOOL_DENY } from "../security/dangerous-tools.js"; import { normalizeMessageChannel } from "../utils/message-channel.js"; import type { AuthRateLimiter } from "./auth-rate-limit.js"; import type { ResolvedGatewayAuth } from "./auth.js"; -import { authorizeGatewayBearerRequestOrReply } from "./http-auth-helpers.js"; +import { + authorizeGatewayBearerRequestOrReply, + resolveGatewayRequestedOperatorScopes, +} from "./http-auth-helpers.js"; import { readJsonBodyOrError, sendInvalidRequest, @@ -36,6 +40,7 @@ import { sendMethodNotAllowed, } from "./http-common.js"; import { getHeader } from "./http-utils.js"; +import { authorizeOperatorScopesForMethod } from "./method-scopes.js"; const DEFAULT_BODY_BYTES = 2 * 1024 * 1024; const MEMORY_TOOL_NAMES = new Set(["memory_search", "memory_get"]); @@ -168,6 +173,19 @@ export async function handleToolsInvokeHttpRequest( return true; } + const requestedScopes = resolveGatewayRequestedOperatorScopes(req); + const scopeAuth = authorizeOperatorScopesForMethod("agent", requestedScopes); + if (!scopeAuth.allowed) { + sendJson(res, 403, { + ok: false, + error: { + type: "forbidden", + message: `missing scope: ${scopeAuth.missingScope}`, + }, + }); + return true; + } + const bodyUnknown = await readJsonBodyOrError(req, res, opts.maxBodyBytes ?? DEFAULT_BODY_BYTES); if (bodyUnknown === undefined) { return true; @@ -305,7 +323,10 @@ export async function handleToolsInvokeHttpRequest( Array.isArray(gatewayToolsCfg?.deny) ? gatewayToolsCfg.deny : [], ); const gatewayDenySet = new Set(gatewayDenyNames); - const gatewayFiltered = subagentFiltered.filter((t) => !gatewayDenySet.has(t.name)); + // HTTP bearer auth does not bind a device-owner identity, so owner-only tools + // stay unavailable on this surface even when callers assert admin scopes. + const ownerFiltered = applyOwnerOnlyToolPolicy(subagentFiltered, false); + const gatewayFiltered = ownerFiltered.filter((t) => !gatewayDenySet.has(t.name)); const tool = gatewayFiltered.find((t) => t.name === toolName); if (!tool) { diff --git a/src/security/dangerous-tools.ts b/src/security/dangerous-tools.ts index 6d1274723a5..12c4417242c 100644 --- a/src/security/dangerous-tools.ts +++ b/src/security/dangerous-tools.ts @@ -7,6 +7,20 @@ * or interactive flows that don't make sense over a non-interactive HTTP surface. */ export const DEFAULT_GATEWAY_HTTP_TOOL_DENY = [ + // Direct command execution — immediate RCE surface + "exec", + // Arbitrary child process creation — immediate RCE surface + "spawn", + // Shell command execution — immediate RCE surface + "shell", + // Arbitrary file mutation on the host + "fs_write", + // Arbitrary file deletion on the host + "fs_delete", + // Arbitrary file move/rename on the host + "fs_move", + // Patch application can rewrite arbitrary files + "apply_patch", // Session orchestration — spawning agents remotely is RCE "sessions_spawn", // Cross-session injection — message injection across sessions @@ -15,6 +29,8 @@ export const DEFAULT_GATEWAY_HTTP_TOOL_DENY = [ "cron", // Gateway control plane — prevents gateway reconfiguration via HTTP "gateway", + // Node command relay can reach system.run on paired hosts + "nodes", // Interactive setup — requires terminal QR scan, hangs on HTTP "whatsapp_login", ] as const;