diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d8f2c87af7..be92e53993b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,7 @@ Docs: https://docs.openclaw.ai - Security/Exec: for the next npm release, harden safe-bin stdin-only enforcement by blocking output/recursive flags (`sort -o/--output`, grep recursion) and tightening default safe bins to remove `sort`/`grep`, preventing safe-bin allowlist bypass for file writes/recursive reads. Thanks @nedlir for reporting. - Cron/Webhooks: protect cron webhook POST delivery with SSRF-guarded outbound fetch (`fetchWithSsrFGuard`) to block private/metadata destinations before request dispatch. Thanks @Adam55A-code. - Security/Gateway/Agents: remove implicit admin scopes from agent tool gateway calls by classifying methods to least-privilege operator scopes, and restrict `cron`/`gateway` tools to owner senders (with explicit runtime owner checks) to prevent non-owner DM privilege escalation. Ships in the next npm release. Thanks @Adam55A-code for reporting. +- Security/Gateway: centralize gateway method-scope authorization and default non-CLI gateway callers to least-privilege method scopes, with explicit CLI scope handling and regression coverage to prevent scope drift. - Security/Net: block SSRF bypass via NAT64 (`64:ff9b::/96`, `64:ff9b:1::/48`), 6to4 (`2002::/16`), and Teredo (`2001:0000::/32`) IPv6 transition addresses, and fail closed on IPv6 parse errors. Thanks @jackhax. ## 2026.2.17 diff --git a/src/agents/bash-tools.exec.approval-id.e2e.test.ts b/src/agents/bash-tools.exec.approval-id.e2e.test.ts index 527e45fa5e1..3d90797b22a 100644 --- a/src/agents/bash-tools.exec.approval-id.e2e.test.ts +++ b/src/agents/bash-tools.exec.approval-id.e2e.test.ts @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; vi.mock("./tools/gateway.js", () => ({ callGatewayTool: vi.fn(), + readGatewayCallOptions: vi.fn(() => ({})), })); vi.mock("./tools/nodes-utils.js", () => ({ diff --git a/src/agents/openclaw-gateway-tool.e2e.test.ts b/src/agents/openclaw-gateway-tool.e2e.test.ts index bf800e4b1fd..04137f7dc1a 100644 --- a/src/agents/openclaw-gateway-tool.e2e.test.ts +++ b/src/agents/openclaw-gateway-tool.e2e.test.ts @@ -13,6 +13,7 @@ vi.mock("./tools/gateway.js", () => ({ } return { ok: true }; }), + readGatewayCallOptions: vi.fn(() => ({})), })); describe("gateway tool", () => { diff --git a/src/agents/tool-policy.ts b/src/agents/tool-policy.ts index ab5faf3a491..0b3edfbb399 100644 --- a/src/agents/tool-policy.ts +++ b/src/agents/tool-policy.ts @@ -1,4 +1,4 @@ -import type { AnyAgentTool } from "./tools/common.js"; +import { OWNER_ONLY_TOOL_ERROR, type AnyAgentTool } from "./tools/common.js"; export type ToolProfileId = "minimal" | "coding" | "messaging" | "full"; @@ -101,7 +101,7 @@ export function applyOwnerOnlyToolPolicy(tools: AnyAgentTool[], senderIsOwner: b return { ...tool, execute: async () => { - throw new Error("Tool restricted to owner senders."); + throw new Error(OWNER_ONLY_TOOL_ERROR); }, }; }); diff --git a/src/agents/tools/common.ts b/src/agents/tools/common.ts index bca56ceada3..27f22be1d05 100644 --- a/src/agents/tools/common.ts +++ b/src/agents/tools/common.ts @@ -19,6 +19,8 @@ export type ActionGate> = ( defaultValue?: boolean, ) => boolean; +export const OWNER_ONLY_TOOL_ERROR = "Tool restricted to owner senders."; + export class ToolInputError extends Error { readonly status = 400; @@ -208,6 +210,12 @@ export function jsonResult(payload: unknown): AgentToolResult { }; } +export function assertOwnerSender(senderIsOwner?: boolean): void { + if (senderIsOwner === false) { + throw new Error(OWNER_ONLY_TOOL_ERROR); + } +} + export async function imageResult(params: { label: string; path: string; diff --git a/src/agents/tools/cron-tool.ts b/src/agents/tools/cron-tool.ts index b692fdd7911..8a5b51519fa 100644 --- a/src/agents/tools/cron-tool.ts +++ b/src/agents/tools/cron-tool.ts @@ -8,8 +8,8 @@ import { extractTextFromChatContent } from "../../shared/chat-content.js"; import { isRecord, truncateUtf16Safe } from "../../utils.js"; import { resolveSessionAgentId } from "../agent-scope.js"; import { optionalStringEnum, stringEnum } from "../schema/typebox.js"; -import { type AnyAgentTool, jsonResult, readStringParam } from "./common.js"; -import { callGatewayTool, type GatewayCallOptions } from "./gateway.js"; +import { assertOwnerSender, type AnyAgentTool, jsonResult, readStringParam } from "./common.js"; +import { callGatewayTool, readGatewayCallOptions, type GatewayCallOptions } from "./gateway.js"; import { resolveInternalSessionKey, resolveMainSessionAlias } from "./sessions-helpers.js"; // NOTE: We use Type.Object({}, { additionalProperties: true }) for job/patch @@ -260,15 +260,15 @@ WAKE MODES (for wake action): Use jobId as the canonical identifier; id is accepted for compatibility. Use contextMessages (0-10) to add previous messages as context to the job text.`, parameters: CronToolSchema, execute: async (_toolCallId, args) => { - if (opts?.senderIsOwner === false) { - throw new Error("Tool restricted to owner senders."); - } + assertOwnerSender(opts?.senderIsOwner); const params = args as Record; const action = readStringParam(params, "action", { required: true }); const gatewayOpts: GatewayCallOptions = { - gatewayUrl: readStringParam(params, "gatewayUrl", { trim: false }), - gatewayToken: readStringParam(params, "gatewayToken", { trim: false }), - timeoutMs: typeof params.timeoutMs === "number" ? params.timeoutMs : 60_000, + ...readGatewayCallOptions(params), + timeoutMs: + typeof params.timeoutMs === "number" && Number.isFinite(params.timeoutMs) + ? params.timeoutMs + : 60_000, }; switch (action) { diff --git a/src/agents/tools/gateway-tool.ts b/src/agents/tools/gateway-tool.ts index 1e2f2cf7f4a..ee360082455 100644 --- a/src/agents/tools/gateway-tool.ts +++ b/src/agents/tools/gateway-tool.ts @@ -10,8 +10,8 @@ import { } from "../../infra/restart-sentinel.js"; import { scheduleGatewaySigusr1Restart } from "../../infra/restart.js"; import { stringEnum } from "../schema/typebox.js"; -import { type AnyAgentTool, jsonResult, readStringParam } from "./common.js"; -import { callGatewayTool } from "./gateway.js"; +import { assertOwnerSender, type AnyAgentTool, jsonResult, readStringParam } from "./common.js"; +import { callGatewayTool, readGatewayCallOptions } from "./gateway.js"; const DEFAULT_UPDATE_TIMEOUT_MS = 20 * 60_000; @@ -74,9 +74,7 @@ export function createGatewayTool(opts?: { "Restart, apply config, or update the gateway in-place (SIGUSR1). Use config.patch for safe partial config updates (merges with existing). Use config.apply only when replacing entire config. Both trigger restart after writing. Always pass a human-readable completion message via the `note` parameter so the system can deliver it to the user after restart.", parameters: GatewayToolSchema, execute: async (_toolCallId, args) => { - if (opts?.senderIsOwner === false) { - throw new Error("Tool restricted to owner senders."); - } + assertOwnerSender(opts?.senderIsOwner); const params = args as Record; const action = readStringParam(params, "action", { required: true }); if (action === "restart") { @@ -129,19 +127,7 @@ export function createGatewayTool(opts?: { return jsonResult(scheduled); } - const gatewayUrl = - typeof params.gatewayUrl === "string" && params.gatewayUrl.trim() - ? params.gatewayUrl.trim() - : undefined; - const gatewayToken = - typeof params.gatewayToken === "string" && params.gatewayToken.trim() - ? params.gatewayToken.trim() - : undefined; - const timeoutMs = - typeof params.timeoutMs === "number" && Number.isFinite(params.timeoutMs) - ? Math.max(1, Math.floor(params.timeoutMs)) - : undefined; - const gatewayOpts = { gatewayUrl, gatewayToken, timeoutMs }; + const gatewayOpts = readGatewayCallOptions(params); const resolveGatewayWriteMeta = (): { sessionKey: string | undefined; @@ -214,15 +200,16 @@ export function createGatewayTool(opts?: { } if (action === "update.run") { const { sessionKey, note, restartDelayMs } = resolveGatewayWriteMeta(); + const updateTimeoutMs = gatewayOpts.timeoutMs ?? DEFAULT_UPDATE_TIMEOUT_MS; const updateGatewayOpts = { ...gatewayOpts, - timeoutMs: timeoutMs ?? DEFAULT_UPDATE_TIMEOUT_MS, + timeoutMs: updateTimeoutMs, }; const result = await callGatewayTool("update.run", updateGatewayOpts, { sessionKey, note, restartDelayMs, - timeoutMs: timeoutMs ?? DEFAULT_UPDATE_TIMEOUT_MS, + timeoutMs: updateTimeoutMs, }); return jsonResult({ ok: true, result }); } diff --git a/src/gateway/call.test.ts b/src/gateway/call.test.ts index 554c5f9b312..0435ed705d3 100644 --- a/src/gateway/call.test.ts +++ b/src/gateway/call.test.ts @@ -75,7 +75,8 @@ vi.mock("./client.js", () => ({ }, })); -const { buildGatewayConnectionDetails, callGateway } = await import("./call.js"); +const { buildGatewayConnectionDetails, callGateway, callGatewayCli, callGatewayScoped } = + await import("./call.js"); function resetGatewayCallMocks() { loadConfig.mockReset(); @@ -198,13 +199,23 @@ describe("callGateway url resolution", () => { expect(lastClientOptions?.token).toBe("explicit-token"); }); - it("keeps legacy admin scopes when call scopes are omitted", async () => { + it("uses least-privilege scopes by default for non-CLI callers", async () => { loadConfig.mockReturnValue({ gateway: { mode: "local", bind: "loopback" } }); resolveGatewayPort.mockReturnValue(18789); pickPrimaryTailnetIPv4.mockReturnValue(undefined); await callGateway({ method: "health" }); + expect(lastClientOptions?.scopes).toEqual(["operator.read"]); + }); + + it("keeps legacy admin scopes for explicit CLI callers", async () => { + loadConfig.mockReturnValue({ gateway: { mode: "local", bind: "loopback" } }); + resolveGatewayPort.mockReturnValue(18789); + pickPrimaryTailnetIPv4.mockReturnValue(undefined); + + await callGatewayCli({ method: "health" }); + expect(lastClientOptions?.scopes).toEqual([ "operator.admin", "operator.approvals", @@ -217,10 +228,10 @@ describe("callGateway url resolution", () => { resolveGatewayPort.mockReturnValue(18789); pickPrimaryTailnetIPv4.mockReturnValue(undefined); - await callGateway({ method: "health", scopes: ["operator.read"] }); + await callGatewayScoped({ method: "health", scopes: ["operator.read"] }); expect(lastClientOptions?.scopes).toEqual(["operator.read"]); - await callGateway({ method: "health", scopes: [] }); + await callGatewayScoped({ method: "health", scopes: [] }); expect(lastClientOptions?.scopes).toEqual([]); }); }); diff --git a/src/gateway/call.ts b/src/gateway/call.ts index 85660662861..09845b7e1cf 100644 --- a/src/gateway/call.ts +++ b/src/gateway/call.ts @@ -16,11 +16,15 @@ import { type GatewayClientName, } from "../utils/message-channel.js"; import { GatewayClient } from "./client.js"; -import type { OperatorScope } from "./method-scopes.js"; +import { + CLI_DEFAULT_OPERATOR_SCOPES, + resolveLeastPrivilegeOperatorScopesForMethod, + type OperatorScope, +} from "./method-scopes.js"; import { isSecureWebSocketUrl, pickPrimaryLanIPv4 } from "./net.js"; import { PROTOCOL_VERSION } from "./protocol/index.js"; -export type CallGatewayOptions = { +type CallGatewayBaseOptions = { url?: string; token?: string; password?: string; @@ -38,7 +42,6 @@ export type CallGatewayOptions = { instanceId?: string; minProtocol?: number; maxProtocol?: number; - scopes?: OperatorScope[]; /** * Overrides the config path shown in connection error details. * Does not affect config loading; callers still control auth via opts.token/password/env/config. @@ -46,6 +49,18 @@ export type CallGatewayOptions = { configPath?: string; }; +export type CallGatewayScopedOptions = CallGatewayBaseOptions & { + scopes: OperatorScope[]; +}; + +export type CallGatewayCliOptions = CallGatewayBaseOptions & { + scopes?: OperatorScope[]; +}; + +export type CallGatewayOptions = CallGatewayBaseOptions & { + scopes?: OperatorScope[]; +}; + export type GatewayConnectionDetails = { url: string; urlSource: string; @@ -171,8 +186,9 @@ export function buildGatewayConnectionDetails( }; } -export async function callGateway>( - opts: CallGatewayOptions, +async function callGatewayWithScopes>( + opts: CallGatewayBaseOptions, + scopes: OperatorScope[], ): Promise { const timeoutMs = typeof opts.timeoutMs === "number" && Number.isFinite(opts.timeoutMs) ? opts.timeoutMs : 10_000; @@ -259,9 +275,6 @@ export async function callGateway>( }; const formatTimeoutError = () => `gateway timeout after ${timeoutMs}ms\n${connectionDetails.message}`; - const scopes = Array.isArray(opts.scopes) - ? opts.scopes - : ["operator.admin", "operator.approvals", "operator.pairing"]; return await new Promise((resolve, reject) => { let settled = false; let ignoreClose = false; @@ -328,6 +341,44 @@ export async function callGateway>( }); } +export async function callGatewayScoped>( + opts: CallGatewayScopedOptions, +): Promise { + return await callGatewayWithScopes(opts, opts.scopes); +} + +export async function callGatewayCli>( + opts: CallGatewayCliOptions, +): Promise { + const scopes = Array.isArray(opts.scopes) ? opts.scopes : CLI_DEFAULT_OPERATOR_SCOPES; + return await callGatewayWithScopes(opts, scopes); +} + +export async function callGatewayLeastPrivilege>( + opts: CallGatewayBaseOptions, +): Promise { + const scopes = resolveLeastPrivilegeOperatorScopesForMethod(opts.method); + return await callGatewayWithScopes(opts, scopes); +} + +export async function callGateway>( + opts: CallGatewayOptions, +): Promise { + if (Array.isArray(opts.scopes)) { + return await callGatewayWithScopes(opts, opts.scopes); + } + const callerMode = opts.mode ?? GATEWAY_CLIENT_MODES.BACKEND; + const callerName = opts.clientName ?? GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT; + if (callerMode === GATEWAY_CLIENT_MODES.CLI || callerName === GATEWAY_CLIENT_NAMES.CLI) { + return await callGatewayCli(opts); + } + return await callGatewayLeastPrivilege({ + ...opts, + mode: callerMode, + clientName: callerName, + }); +} + export function randomIdempotencyKey() { return randomUUID(); } diff --git a/src/gateway/method-scopes.test.ts b/src/gateway/method-scopes.test.ts new file mode 100644 index 00000000000..61a80bfeaae --- /dev/null +++ b/src/gateway/method-scopes.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vitest"; +import { + authorizeOperatorScopesForMethod, + resolveLeastPrivilegeOperatorScopesForMethod, +} from "./method-scopes.js"; + +describe("method scope resolution", () => { + it("classifies sessions.resolve as read and poll as write", () => { + expect(resolveLeastPrivilegeOperatorScopesForMethod("sessions.resolve")).toEqual([ + "operator.read", + ]); + expect(resolveLeastPrivilegeOperatorScopesForMethod("poll")).toEqual(["operator.write"]); + }); + + it("returns empty scopes for unknown methods", () => { + expect(resolveLeastPrivilegeOperatorScopesForMethod("totally.unknown.method")).toEqual([]); + }); +}); + +describe("operator scope authorization", () => { + it("allows read methods with operator.read or operator.write", () => { + expect(authorizeOperatorScopesForMethod("health", ["operator.read"])).toEqual({ + allowed: true, + }); + expect(authorizeOperatorScopesForMethod("health", ["operator.write"])).toEqual({ + allowed: true, + }); + }); + + it("requires operator.write for write methods", () => { + expect(authorizeOperatorScopesForMethod("send", ["operator.read"])).toEqual({ + allowed: false, + missingScope: "operator.write", + }); + }); + + it("requires approvals scope for approval methods", () => { + expect(authorizeOperatorScopesForMethod("exec.approval.resolve", ["operator.write"])).toEqual({ + allowed: false, + missingScope: "operator.approvals", + }); + }); + + it("requires admin for unknown methods", () => { + expect(authorizeOperatorScopesForMethod("unknown.method", ["operator.read"])).toEqual({ + allowed: false, + missingScope: "operator.admin", + }); + }); +}); diff --git a/src/gateway/method-scopes.ts b/src/gateway/method-scopes.ts index 699f9691e2e..c270a4495ab 100644 --- a/src/gateway/method-scopes.ts +++ b/src/gateway/method-scopes.ts @@ -11,6 +11,12 @@ export type OperatorScope = | typeof APPROVALS_SCOPE | typeof PAIRING_SCOPE; +export const CLI_DEFAULT_OPERATOR_SCOPES: OperatorScope[] = [ + ADMIN_SCOPE, + APPROVALS_SCOPE, + PAIRING_SCOPE, +]; + const APPROVAL_METHODS = new Set([ "exec.approval.request", "exec.approval.waitDecision", @@ -52,6 +58,7 @@ const READ_METHODS = new Set([ "voicewake.get", "sessions.list", "sessions.preview", + "sessions.resolve", "cron.list", "cron.status", "cron.runs", @@ -66,6 +73,7 @@ const READ_METHODS = new Set([ const WRITE_METHODS = new Set([ "send", + "poll", "agent", "agent.wait", "wake", @@ -133,22 +141,50 @@ export function isAdminOnlyMethod(method: string): boolean { return ADMIN_METHODS.has(method); } -export function resolveLeastPrivilegeOperatorScopesForMethod(method: string): OperatorScope[] { +export function resolveRequiredOperatorScopeForMethod(method: string): OperatorScope | undefined { if (isApprovalMethod(method)) { - return [APPROVALS_SCOPE]; + return APPROVALS_SCOPE; } if (isPairingMethod(method)) { - return [PAIRING_SCOPE]; + return PAIRING_SCOPE; } if (isReadMethod(method)) { - return [READ_SCOPE]; + return READ_SCOPE; } if (isWriteMethod(method)) { - return [WRITE_SCOPE]; + return WRITE_SCOPE; } if (isAdminOnlyMethod(method)) { - return [ADMIN_SCOPE]; + return ADMIN_SCOPE; + } + return undefined; +} + +export function resolveLeastPrivilegeOperatorScopesForMethod(method: string): OperatorScope[] { + const requiredScope = resolveRequiredOperatorScopeForMethod(method); + if (requiredScope) { + return [requiredScope]; } // Default-deny for unclassified methods. return []; } + +export function authorizeOperatorScopesForMethod( + method: string, + scopes: readonly string[], +): { allowed: true } | { allowed: false; missingScope: OperatorScope } { + if (scopes.includes(ADMIN_SCOPE)) { + return { allowed: true }; + } + const requiredScope = resolveRequiredOperatorScopeForMethod(method) ?? ADMIN_SCOPE; + if (requiredScope === READ_SCOPE) { + if (scopes.includes(READ_SCOPE) || scopes.includes(WRITE_SCOPE)) { + return { allowed: true }; + } + return { allowed: false, missingScope: READ_SCOPE }; + } + if (scopes.includes(requiredScope)) { + return { allowed: true }; + } + return { allowed: false, missingScope: requiredScope }; +} diff --git a/src/gateway/server-methods.ts b/src/gateway/server-methods.ts index c772bc1c367..0ac6ba1ece2 100644 --- a/src/gateway/server-methods.ts +++ b/src/gateway/server-methods.ts @@ -2,16 +2,8 @@ import { formatControlPlaneActor, resolveControlPlaneActor } from "./control-pla import { consumeControlPlaneWriteBudget } from "./control-plane-rate-limit.js"; import { ADMIN_SCOPE, - APPROVALS_SCOPE, - isAdminOnlyMethod, - isApprovalMethod, + authorizeOperatorScopesForMethod, isNodeRoleMethod, - isPairingMethod, - isReadMethod, - isWriteMethod, - PAIRING_SCOPE, - READ_SCOPE, - WRITE_SCOPE, } from "./method-scopes.js"; import { ErrorCodes, errorShape } from "./protocol/index.js"; import { agentHandlers } from "./server-methods/agent.js"; @@ -64,34 +56,11 @@ function authorizeGatewayMethod(method: string, client: GatewayRequestOptions["c if (scopes.includes(ADMIN_SCOPE)) { return null; } - if (isApprovalMethod(method) && !scopes.includes(APPROVALS_SCOPE)) { - return errorShape(ErrorCodes.INVALID_REQUEST, "missing scope: operator.approvals"); + const scopeAuth = authorizeOperatorScopesForMethod(method, scopes); + if (!scopeAuth.allowed) { + return errorShape(ErrorCodes.INVALID_REQUEST, `missing scope: ${scopeAuth.missingScope}`); } - if (isPairingMethod(method) && !scopes.includes(PAIRING_SCOPE)) { - return errorShape(ErrorCodes.INVALID_REQUEST, "missing scope: operator.pairing"); - } - if (isReadMethod(method) && !(scopes.includes(READ_SCOPE) || scopes.includes(WRITE_SCOPE))) { - return errorShape(ErrorCodes.INVALID_REQUEST, "missing scope: operator.read"); - } - if (isWriteMethod(method) && !scopes.includes(WRITE_SCOPE)) { - return errorShape(ErrorCodes.INVALID_REQUEST, "missing scope: operator.write"); - } - if (isApprovalMethod(method)) { - return null; - } - if (isPairingMethod(method)) { - return null; - } - if (isReadMethod(method)) { - return null; - } - if (isWriteMethod(method)) { - return null; - } - if (isAdminOnlyMethod(method)) { - return errorShape(ErrorCodes.INVALID_REQUEST, "missing scope: operator.admin"); - } - return errorShape(ErrorCodes.INVALID_REQUEST, "missing scope: operator.admin"); + return null; } export const coreGatewayHandlers: GatewayRequestHandlers = { diff --git a/src/infra/outbound/message.e2e.test.ts b/src/infra/outbound/message.e2e.test.ts index cd2212928ac..fda22fc19e9 100644 --- a/src/infra/outbound/message.e2e.test.ts +++ b/src/infra/outbound/message.e2e.test.ts @@ -13,6 +13,7 @@ const setRegistry = (registry: ReturnType) => { const callGatewayMock = vi.fn(); vi.mock("../../gateway/call.js", () => ({ callGateway: (...args: unknown[]) => callGatewayMock(...args), + callGatewayLeastPrivilege: (...args: unknown[]) => callGatewayMock(...args), randomIdempotencyKey: () => "idem-1", })); diff --git a/src/infra/outbound/message.ts b/src/infra/outbound/message.ts index e7e0a0a93c0..71b36eca6b1 100644 --- a/src/infra/outbound/message.ts +++ b/src/infra/outbound/message.ts @@ -1,7 +1,7 @@ import { getChannelPlugin, normalizeChannelId } from "../../channels/plugins/index.js"; import type { OpenClawConfig } from "../../config/config.js"; import { loadConfig } from "../../config/config.js"; -import { callGateway, randomIdempotencyKey } from "../../gateway/call.js"; +import { callGatewayLeastPrivilege, randomIdempotencyKey } from "../../gateway/call.js"; import type { PollInput } from "../../polls.js"; import { normalizePollInput } from "../../polls.js"; import { @@ -151,7 +151,7 @@ async function callMessageGateway(params: { params: Record; }): Promise { const gateway = resolveGatewayOptions(params.gateway); - return await callGateway({ + return await callGatewayLeastPrivilege({ url: gateway.url, token: gateway.token, method: params.method,