fix(security): centralize owner-only tool gating and scope maps

This commit is contained in:
Peter Steinberger
2026-02-19 15:27:45 +01:00
parent 9130fd2b06
commit 3d7ad1cfca
16 changed files with 372 additions and 251 deletions

View File

@@ -17,23 +17,15 @@ vi.mock("./tools/gateway.js", () => ({
}));
describe("gateway tool", () => {
it("rejects non-owner callers explicitly", async () => {
const { callGatewayTool } = await import("./tools/gateway.js");
it("marks gateway as owner-only", async () => {
const tool = createOpenClawTools({
senderIsOwner: false,
config: { commands: { restart: true } },
}).find((candidate) => candidate.name === "gateway");
expect(tool).toBeDefined();
if (!tool) {
throw new Error("missing gateway tool");
}
await expect(
tool.execute("call-owner-check", {
action: "config.get",
}),
).rejects.toThrow("Tool restricted to owner senders.");
expect(callGatewayTool).not.toHaveBeenCalled();
expect(tool.ownerOnly).toBe(true);
});
it("schedules SIGUSR1 restart", async () => {

View File

@@ -114,7 +114,6 @@ export function createOpenClawTools(options?: {
}),
createCronTool({
agentSessionKey: options?.agentSessionKey,
senderIsOwner: options?.senderIsOwner,
}),
...(messageTool ? [messageTool] : []),
createTtsTool({
@@ -124,7 +123,6 @@ export function createOpenClawTools(options?: {
createGatewayTool({
agentSessionKey: options?.agentSessionKey,
config: options?.config,
senderIsOwner: options?.senderIsOwner,
}),
createAgentsListTool({
agentSessionKey: options?.agentSessionKey,

View File

@@ -22,11 +22,13 @@ function createOwnerPolicyTools() {
},
{
name: "cron",
ownerOnly: true,
// oxlint-disable-next-line typescript/no-explicit-any
execute: async () => ({ content: [], details: {} }) as any,
},
{
name: "gateway",
ownerOnly: true,
// oxlint-disable-next-line typescript/no-explicit-any
execute: async () => ({ content: [], details: {} }) as any,
},
@@ -89,6 +91,19 @@ describe("tool-policy", () => {
const filtered = applyOwnerOnlyToolPolicy(tools, true);
expect(filtered.map((t) => t.name)).toEqual(["read", "cron", "gateway", "whatsapp_login"]);
});
it("honors ownerOnly metadata for custom tool names", async () => {
const tools = [
{
name: "custom_admin_tool",
ownerOnly: true,
// oxlint-disable-next-line typescript/no-explicit-any
execute: async () => ({ content: [], details: {} }) as any,
},
] as unknown as AnyAgentTool[];
expect(applyOwnerOnlyToolPolicy(tools, false)).toEqual([]);
expect(applyOwnerOnlyToolPolicy(tools, true)).toHaveLength(1);
});
});
describe("TOOL_POLICY_CONFORMANCE", () => {

View File

@@ -1,4 +1,4 @@
import { OWNER_ONLY_TOOL_ERROR, type AnyAgentTool } from "./tools/common.js";
import { type AnyAgentTool, wrapOwnerOnlyToolExecution } from "./tools/common.js";
export type ToolProfileId = "minimal" | "coding" | "messaging" | "full";
@@ -60,7 +60,7 @@ export const TOOL_GROUPS: Record<string, string[]> = {
],
};
const OWNER_ONLY_TOOL_NAMES = new Set<string>(["whatsapp_login", "cron", "gateway"]);
const OWNER_ONLY_TOOL_NAME_FALLBACKS = new Set<string>(["whatsapp_login", "cron", "gateway"]);
const TOOL_PROFILES: Record<ToolProfileId, ToolProfilePolicy> = {
minimal: {
@@ -87,28 +87,24 @@ export function normalizeToolName(name: string) {
}
export function isOwnerOnlyToolName(name: string) {
return OWNER_ONLY_TOOL_NAMES.has(normalizeToolName(name));
return OWNER_ONLY_TOOL_NAME_FALLBACKS.has(normalizeToolName(name));
}
function isOwnerOnlyTool(tool: AnyAgentTool) {
return tool.ownerOnly === true || isOwnerOnlyToolName(tool.name);
}
export function applyOwnerOnlyToolPolicy(tools: AnyAgentTool[], senderIsOwner: boolean) {
const withGuard = tools.map((tool) => {
if (!isOwnerOnlyToolName(tool.name)) {
if (!isOwnerOnlyTool(tool)) {
return tool;
}
if (senderIsOwner || !tool.execute) {
return tool;
}
return {
...tool,
execute: async () => {
throw new Error(OWNER_ONLY_TOOL_ERROR);
},
};
return wrapOwnerOnlyToolExecution(tool, senderIsOwner);
});
if (senderIsOwner) {
return withGuard;
}
return withGuard.filter((tool) => !isOwnerOnlyToolName(tool.name));
return withGuard.filter((tool) => !isOwnerOnlyTool(tool));
}
export function normalizeToolList(list?: string[]) {

View File

@@ -5,7 +5,9 @@ import type { ImageSanitizationLimits } from "../image-sanitization.js";
import { sanitizeToolResultImages } from "../tool-images.js";
// oxlint-disable-next-line typescript/no-explicit-any
export type AnyAgentTool = AgentTool<any, unknown>;
export type AnyAgentTool = AgentTool<any, unknown> & {
ownerOnly?: boolean;
};
export type StringParamOptions = {
required?: boolean;
@@ -210,10 +212,19 @@ export function jsonResult(payload: unknown): AgentToolResult<unknown> {
};
}
export function assertOwnerSender(senderIsOwner?: boolean): void {
if (senderIsOwner === false) {
throw new Error(OWNER_ONLY_TOOL_ERROR);
export function wrapOwnerOnlyToolExecution(
tool: AnyAgentTool,
senderIsOwner: boolean,
): AnyAgentTool {
if (tool.ownerOnly !== true || senderIsOwner || !tool.execute) {
return tool;
}
return {
...tool,
execute: async () => {
throw new Error(OWNER_ONLY_TOOL_ERROR);
},
};
}
export async function imageResult(params: {

View File

@@ -30,14 +30,9 @@ describe("cron tool", () => {
callGatewayMock.mockResolvedValue({ ok: true });
});
it("rejects non-owner callers explicitly", async () => {
const tool = createCronTool({ senderIsOwner: false });
await expect(
tool.execute("call-owner-check", {
action: "status",
}),
).rejects.toThrow("Tool restricted to owner senders.");
expect(callGatewayMock).not.toHaveBeenCalled();
it("marks cron as owner-only", async () => {
const tool = createCronTool();
expect(tool.ownerOnly).toBe(true);
});
it.each([

View File

@@ -8,7 +8,7 @@ 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 { assertOwnerSender, type AnyAgentTool, jsonResult, readStringParam } from "./common.js";
import { type AnyAgentTool, jsonResult, readStringParam } from "./common.js";
import { callGatewayTool, readGatewayCallOptions, type GatewayCallOptions } from "./gateway.js";
import { resolveInternalSessionKey, resolveMainSessionAlias } from "./sessions-helpers.js";
@@ -48,7 +48,6 @@ const CronToolSchema = Type.Object({
type CronToolOptions = {
agentSessionKey?: string;
senderIsOwner?: boolean;
};
type ChatMessage = {
@@ -202,6 +201,7 @@ export function createCronTool(opts?: CronToolOptions): AnyAgentTool {
return {
label: "Cron",
name: "cron",
ownerOnly: true,
description: `Manage Gateway cron jobs (status/list/add/update/remove/run/runs) and send wake events.
ACTIONS:
@@ -260,7 +260,6 @@ 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) => {
assertOwnerSender(opts?.senderIsOwner);
const params = args as Record<string, unknown>;
const action = readStringParam(params, "action", { required: true });
const gatewayOpts: GatewayCallOptions = {

View File

@@ -10,7 +10,7 @@ import {
} from "../../infra/restart-sentinel.js";
import { scheduleGatewaySigusr1Restart } from "../../infra/restart.js";
import { stringEnum } from "../schema/typebox.js";
import { assertOwnerSender, type AnyAgentTool, jsonResult, readStringParam } from "./common.js";
import { type AnyAgentTool, jsonResult, readStringParam } from "./common.js";
import { callGatewayTool, readGatewayCallOptions } from "./gateway.js";
const DEFAULT_UPDATE_TIMEOUT_MS = 20 * 60_000;
@@ -65,16 +65,15 @@ const GatewayToolSchema = Type.Object({
export function createGatewayTool(opts?: {
agentSessionKey?: string;
config?: OpenClawConfig;
senderIsOwner?: boolean;
}): AnyAgentTool {
return {
label: "Gateway",
name: "gateway",
ownerOnly: true,
description:
"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) => {
assertOwnerSender(opts?.senderIsOwner);
const params = args as Record<string, unknown>;
const action = readStringParam(params, "action", { required: true });
if (action === "restart") {