mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 20:48:26 +00:00
fix: harden agent gateway authorization scopes
This commit is contained in:
@@ -16,6 +16,25 @@ vi.mock("./tools/gateway.js", () => ({
|
||||
}));
|
||||
|
||||
describe("gateway tool", () => {
|
||||
it("rejects non-owner callers explicitly", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
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();
|
||||
});
|
||||
|
||||
it("schedules SIGUSR1 restart", async () => {
|
||||
vi.useFakeTimers();
|
||||
const kill = vi.spyOn(process, "kill").mockImplementation(() => true);
|
||||
|
||||
@@ -61,6 +61,8 @@ export function createOpenClawTools(options?: {
|
||||
requireExplicitMessageTarget?: boolean;
|
||||
/** If true, omit the message tool from the tool list. */
|
||||
disableMessageTool?: boolean;
|
||||
/** Whether the requesting sender is an owner. */
|
||||
senderIsOwner?: boolean;
|
||||
}): AnyAgentTool[] {
|
||||
const workspaceDir = resolveWorkspaceRoot(options?.workspaceDir);
|
||||
const imageTool = options?.agentDir?.trim()
|
||||
@@ -109,6 +111,7 @@ export function createOpenClawTools(options?: {
|
||||
}),
|
||||
createCronTool({
|
||||
agentSessionKey: options?.agentSessionKey,
|
||||
senderIsOwner: options?.senderIsOwner,
|
||||
}),
|
||||
...(messageTool ? [messageTool] : []),
|
||||
createTtsTool({
|
||||
@@ -118,6 +121,7 @@ export function createOpenClawTools(options?: {
|
||||
createGatewayTool({
|
||||
agentSessionKey: options?.agentSessionKey,
|
||||
config: options?.config,
|
||||
senderIsOwner: options?.senderIsOwner,
|
||||
}),
|
||||
createAgentsListTool({
|
||||
agentSessionKey: options?.agentSessionKey,
|
||||
|
||||
@@ -455,6 +455,7 @@ export function createOpenClawCodingTools(options?: {
|
||||
requireExplicitMessageTarget: options?.requireExplicitMessageTarget,
|
||||
disableMessageTool: options?.disableMessageTool,
|
||||
requesterAgentIdOverride: agentId,
|
||||
senderIsOwner: options?.senderIsOwner,
|
||||
}),
|
||||
];
|
||||
// Security: treat unknown/undefined as unauthorized (opt-in, not opt-out)
|
||||
|
||||
@@ -14,22 +14,28 @@ vi.mock("./channel-tools.js", () => {
|
||||
};
|
||||
});
|
||||
|
||||
describe("whatsapp_login tool gating", () => {
|
||||
it("removes whatsapp_login for unauthorized senders", () => {
|
||||
describe("owner-only tool gating", () => {
|
||||
it("removes owner-only tools for unauthorized senders", () => {
|
||||
const tools = createOpenClawCodingTools({ senderIsOwner: false });
|
||||
const toolNames = tools.map((tool) => tool.name);
|
||||
expect(toolNames).not.toContain("whatsapp_login");
|
||||
expect(toolNames).not.toContain("cron");
|
||||
expect(toolNames).not.toContain("gateway");
|
||||
});
|
||||
|
||||
it("keeps whatsapp_login for authorized senders", () => {
|
||||
it("keeps owner-only tools for authorized senders", () => {
|
||||
const tools = createOpenClawCodingTools({ senderIsOwner: true });
|
||||
const toolNames = tools.map((tool) => tool.name);
|
||||
expect(toolNames).toContain("whatsapp_login");
|
||||
expect(toolNames).toContain("cron");
|
||||
expect(toolNames).toContain("gateway");
|
||||
});
|
||||
|
||||
it("defaults to removing whatsapp_login when owner status is unknown", () => {
|
||||
it("defaults to removing owner-only tools when owner status is unknown", () => {
|
||||
const tools = createOpenClawCodingTools();
|
||||
const toolNames = tools.map((tool) => tool.name);
|
||||
expect(toolNames).not.toContain("whatsapp_login");
|
||||
expect(toolNames).not.toContain("cron");
|
||||
expect(toolNames).not.toContain("gateway");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -20,6 +20,16 @@ function createOwnerPolicyTools() {
|
||||
// oxlint-disable-next-line typescript/no-explicit-any
|
||||
execute: async () => ({ content: [], details: {} }) as any,
|
||||
},
|
||||
{
|
||||
name: "cron",
|
||||
// oxlint-disable-next-line typescript/no-explicit-any
|
||||
execute: async () => ({ content: [], details: {} }) as any,
|
||||
},
|
||||
{
|
||||
name: "gateway",
|
||||
// oxlint-disable-next-line typescript/no-explicit-any
|
||||
execute: async () => ({ content: [], details: {} }) as any,
|
||||
},
|
||||
{
|
||||
name: "whatsapp_login",
|
||||
// oxlint-disable-next-line typescript/no-explicit-any
|
||||
@@ -63,6 +73,8 @@ describe("tool-policy", () => {
|
||||
|
||||
it("identifies owner-only tools", () => {
|
||||
expect(isOwnerOnlyToolName("whatsapp_login")).toBe(true);
|
||||
expect(isOwnerOnlyToolName("cron")).toBe(true);
|
||||
expect(isOwnerOnlyToolName("gateway")).toBe(true);
|
||||
expect(isOwnerOnlyToolName("read")).toBe(false);
|
||||
});
|
||||
|
||||
@@ -75,7 +87,7 @@ describe("tool-policy", () => {
|
||||
it("keeps owner-only tools for the owner sender", async () => {
|
||||
const tools = createOwnerPolicyTools();
|
||||
const filtered = applyOwnerOnlyToolPolicy(tools, true);
|
||||
expect(filtered.map((t) => t.name)).toEqual(["read", "whatsapp_login"]);
|
||||
expect(filtered.map((t) => t.name)).toEqual(["read", "cron", "gateway", "whatsapp_login"]);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -60,7 +60,7 @@ export const TOOL_GROUPS: Record<string, string[]> = {
|
||||
],
|
||||
};
|
||||
|
||||
const OWNER_ONLY_TOOL_NAMES = new Set<string>(["whatsapp_login"]);
|
||||
const OWNER_ONLY_TOOL_NAMES = new Set<string>(["whatsapp_login", "cron", "gateway"]);
|
||||
|
||||
const TOOL_PROFILES: Record<ToolProfileId, ToolProfilePolicy> = {
|
||||
minimal: {
|
||||
|
||||
@@ -39,6 +39,16 @@ 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.each([
|
||||
[
|
||||
"update",
|
||||
|
||||
@@ -48,6 +48,7 @@ const CronToolSchema = Type.Object({
|
||||
|
||||
type CronToolOptions = {
|
||||
agentSessionKey?: string;
|
||||
senderIsOwner?: boolean;
|
||||
};
|
||||
|
||||
type ChatMessage = {
|
||||
@@ -259,6 +260,9 @@ 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.");
|
||||
}
|
||||
const params = args as Record<string, unknown>;
|
||||
const action = readStringParam(params, "action", { required: true });
|
||||
const gatewayOpts: GatewayCallOptions = {
|
||||
|
||||
@@ -65,6 +65,7 @@ const GatewayToolSchema = Type.Object({
|
||||
export function createGatewayTool(opts?: {
|
||||
agentSessionKey?: string;
|
||||
config?: OpenClawConfig;
|
||||
senderIsOwner?: boolean;
|
||||
}): AnyAgentTool {
|
||||
return {
|
||||
label: "Gateway",
|
||||
@@ -73,6 +74,9 @@ 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.");
|
||||
}
|
||||
const params = args as Record<string, unknown>;
|
||||
const action = readStringParam(params, "action", { required: true });
|
||||
if (action === "restart") {
|
||||
|
||||
@@ -32,6 +32,40 @@ describe("gateway tool defaults", () => {
|
||||
url: "ws://127.0.0.1:18789",
|
||||
token: "t",
|
||||
timeoutMs: 5000,
|
||||
scopes: ["operator.read"],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses least-privilege write scope for write methods", async () => {
|
||||
callGatewayMock.mockResolvedValueOnce({ ok: true });
|
||||
await callGatewayTool("wake", {}, { mode: "now", text: "hi" });
|
||||
expect(callGatewayMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
method: "wake",
|
||||
scopes: ["operator.write"],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses admin scope only for admin methods", async () => {
|
||||
callGatewayMock.mockResolvedValueOnce({ ok: true });
|
||||
await callGatewayTool("cron.add", {}, { id: "job-1" });
|
||||
expect(callGatewayMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
method: "cron.add",
|
||||
scopes: ["operator.admin"],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("default-denies unknown methods by sending no scopes", async () => {
|
||||
callGatewayMock.mockResolvedValueOnce({ ok: true });
|
||||
await callGatewayTool("nonexistent.method", {}, {});
|
||||
expect(callGatewayMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
method: "nonexistent.method",
|
||||
scopes: [],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { loadConfig, resolveGatewayPort } from "../../config/config.js";
|
||||
import { callGateway } from "../../gateway/call.js";
|
||||
import { resolveLeastPrivilegeOperatorScopesForMethod } from "../../gateway/method-scopes.js";
|
||||
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../../utils/message-channel.js";
|
||||
import { readStringParam } from "./common.js";
|
||||
|
||||
@@ -109,6 +110,7 @@ export async function callGatewayTool<T = Record<string, unknown>>(
|
||||
extra?: { expectFinal?: boolean },
|
||||
) {
|
||||
const gateway = resolveGatewayOptions(opts);
|
||||
const scopes = resolveLeastPrivilegeOperatorScopesForMethod(method);
|
||||
return await callGateway<T>({
|
||||
url: gateway.url,
|
||||
token: gateway.token,
|
||||
@@ -119,5 +121,6 @@ export async function callGatewayTool<T = Record<string, unknown>>(
|
||||
clientName: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT,
|
||||
clientDisplayName: "agent",
|
||||
mode: GATEWAY_CLIENT_MODES.BACKEND,
|
||||
scopes,
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user