mirror of https://github.com/openclaw/openclaw.git
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
This commit is contained in:
parent
ae703ab0e7
commit
29cb1e3c7e
|
|
@ -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<string, unknown> = {};
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in New Issue