mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-28 18:33:37 +00:00
Agents: block protected gateway config writes (#55682)
* Agents: block protected gateway config writes * Agents: tighten gateway config guard coverage * Agents: guard migrated exec config aliases
This commit is contained in:
@@ -9,7 +9,17 @@ function createGatewayToolModuleMocks() {
|
||||
return {
|
||||
callGatewayTool: vi.fn(async (method: string) => {
|
||||
if (method === "config.get") {
|
||||
return { hash: "hash-1" };
|
||||
return {
|
||||
hash: "hash-1",
|
||||
config: {
|
||||
tools: {
|
||||
exec: {
|
||||
ask: "on-miss",
|
||||
security: "allowlist",
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
if (method === "config.schema.lookup") {
|
||||
return {
|
||||
@@ -141,7 +151,8 @@ describe("gateway tool", () => {
|
||||
const sessionKey = "agent:main:whatsapp:dm:+15555550123";
|
||||
const tool = requireGatewayTool(sessionKey);
|
||||
|
||||
const raw = '{\n agents: { defaults: { workspace: "~/openclaw" } }\n}\n';
|
||||
const raw =
|
||||
'{\n agents: { defaults: { workspace: "~/openclaw" } },\n tools: { exec: { ask: "on-miss", security: "allowlist" } }\n}\n';
|
||||
await tool.execute("call2", {
|
||||
action: "config.apply",
|
||||
raw,
|
||||
@@ -174,6 +185,90 @@ describe("gateway tool", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects config.patch when it changes exec approval settings", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const tool = requireGatewayTool();
|
||||
|
||||
await expect(
|
||||
tool.execute("call-protected-patch", {
|
||||
action: "config.patch",
|
||||
raw: '{ tools: { exec: { ask: "off" } } }',
|
||||
}),
|
||||
).rejects.toThrow("gateway config.patch cannot change protected config paths: tools.exec.ask");
|
||||
expect(callGatewayTool).toHaveBeenCalledWith("config.get", expect.any(Object), {});
|
||||
expect(callGatewayTool).not.toHaveBeenCalledWith(
|
||||
"config.patch",
|
||||
expect.any(Object),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects config.patch when a legacy tools.bash alias changes exec security", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
vi.mocked(callGatewayTool).mockImplementationOnce(async (method: string) => {
|
||||
if (method === "config.get") {
|
||||
return { hash: "hash-1", config: {} };
|
||||
}
|
||||
return { ok: true };
|
||||
});
|
||||
const tool = requireGatewayTool();
|
||||
|
||||
await expect(
|
||||
tool.execute("call-legacy-protected-patch", {
|
||||
action: "config.patch",
|
||||
raw: '{ tools: { bash: { security: "full" } } }',
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
"gateway config.patch cannot change protected config paths: tools.exec.security",
|
||||
);
|
||||
expect(callGatewayTool).toHaveBeenCalledWith("config.get", expect.any(Object), {});
|
||||
expect(callGatewayTool).not.toHaveBeenCalledWith(
|
||||
"config.patch",
|
||||
expect.any(Object),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects config.apply when it changes exec security settings", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const tool = requireGatewayTool();
|
||||
|
||||
await expect(
|
||||
tool.execute("call-protected-apply", {
|
||||
action: "config.apply",
|
||||
raw: '{ tools: { exec: { ask: "on-miss", security: "full" } } }',
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
"gateway config.apply cannot change protected config paths: tools.exec.security",
|
||||
);
|
||||
expect(callGatewayTool).toHaveBeenCalledWith("config.get", expect.any(Object), {});
|
||||
expect(callGatewayTool).not.toHaveBeenCalledWith(
|
||||
"config.apply",
|
||||
expect.any(Object),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects config.apply when protected exec settings are omitted", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const tool = requireGatewayTool();
|
||||
|
||||
await expect(
|
||||
tool.execute("call-missing-protected", {
|
||||
action: "config.apply",
|
||||
raw: '{ agents: { defaults: { workspace: "~/openclaw" } } }',
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
"gateway config.apply cannot change protected config paths: tools.exec.ask, tools.exec.security",
|
||||
);
|
||||
expect(callGatewayTool).toHaveBeenCalledWith("config.get", expect.any(Object), {});
|
||||
expect(callGatewayTool).not.toHaveBeenCalledWith(
|
||||
"config.apply",
|
||||
expect.any(Object),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it("passes update.run through gateway call", async () => {
|
||||
const { callGatewayTool } = await import("./tools/gateway.js");
|
||||
const sessionKey = "agent:main:whatsapp:dm:+15555550123";
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import { Type } from "@sinclair/typebox";
|
||||
import { isRestartEnabled } from "../../config/commands.js";
|
||||
import type { OpenClawConfig } from "../../config/config.js";
|
||||
import { resolveConfigSnapshotHash } from "../../config/io.js";
|
||||
import { parseConfigJson5, resolveConfigSnapshotHash } from "../../config/io.js";
|
||||
import { applyLegacyMigrations } from "../../config/legacy.js";
|
||||
import { applyMergePatch } from "../../config/merge-patch.js";
|
||||
import { extractDeliveryInfo } from "../../config/sessions.js";
|
||||
import {
|
||||
formatDoctorNonInteractiveHint,
|
||||
@@ -17,6 +19,7 @@ import { callGatewayTool, readGatewayCallOptions } from "./gateway.js";
|
||||
const log = createSubsystemLogger("gateway-tool");
|
||||
|
||||
const DEFAULT_UPDATE_TIMEOUT_MS = 20 * 60_000;
|
||||
const PROTECTED_GATEWAY_CONFIG_PATHS = ["tools.exec.ask", "tools.exec.security"] as const;
|
||||
|
||||
function resolveBaseHashFromSnapshot(snapshot: unknown): string | undefined {
|
||||
if (!snapshot || typeof snapshot !== "object") {
|
||||
@@ -31,6 +34,71 @@ function resolveBaseHashFromSnapshot(snapshot: unknown): string | undefined {
|
||||
return hash ?? undefined;
|
||||
}
|
||||
|
||||
function getSnapshotConfig(snapshot: unknown): Record<string, unknown> {
|
||||
if (!snapshot || typeof snapshot !== "object") {
|
||||
throw new Error("config.get response is not an object.");
|
||||
}
|
||||
const config = (snapshot as { config?: unknown }).config;
|
||||
if (!config || typeof config !== "object" || Array.isArray(config)) {
|
||||
throw new Error("config.get response is missing a config object.");
|
||||
}
|
||||
return config as Record<string, unknown>;
|
||||
}
|
||||
|
||||
function parseGatewayConfigMutationRaw(
|
||||
raw: string,
|
||||
action: "config.apply" | "config.patch",
|
||||
): unknown {
|
||||
const parsedRes = parseConfigJson5(raw);
|
||||
if (!parsedRes.ok) {
|
||||
throw new Error(parsedRes.error);
|
||||
}
|
||||
if (
|
||||
!parsedRes.parsed ||
|
||||
typeof parsedRes.parsed !== "object" ||
|
||||
Array.isArray(parsedRes.parsed)
|
||||
) {
|
||||
throw new Error(`${action} raw must be an object.`);
|
||||
}
|
||||
return parsedRes.parsed;
|
||||
}
|
||||
|
||||
function getValueAtPath(config: Record<string, unknown>, path: string): unknown {
|
||||
let current: unknown = config;
|
||||
for (const part of path.split(".")) {
|
||||
if (!current || typeof current !== "object" || Array.isArray(current)) {
|
||||
return undefined;
|
||||
}
|
||||
current = (current as Record<string, unknown>)[part];
|
||||
}
|
||||
return current;
|
||||
}
|
||||
|
||||
function assertGatewayConfigMutationAllowed(params: {
|
||||
action: "config.apply" | "config.patch";
|
||||
currentConfig: Record<string, unknown>;
|
||||
raw: string;
|
||||
}): void {
|
||||
const parsed = parseGatewayConfigMutationRaw(params.raw, params.action);
|
||||
const nextConfig =
|
||||
params.action === "config.apply"
|
||||
? (parsed as Record<string, unknown>)
|
||||
: (applyMergePatch(params.currentConfig, parsed, {
|
||||
mergeObjectArraysById: true,
|
||||
}) as Record<string, unknown>);
|
||||
const migratedNextConfig = applyLegacyMigrations(nextConfig).next ?? nextConfig;
|
||||
const changedProtectedPaths = PROTECTED_GATEWAY_CONFIG_PATHS.filter(
|
||||
(path) =>
|
||||
getValueAtPath(params.currentConfig, path) !== getValueAtPath(migratedNextConfig, path),
|
||||
);
|
||||
if (changedProtectedPaths.length === 0) {
|
||||
return;
|
||||
}
|
||||
throw new Error(
|
||||
`gateway ${params.action} cannot change protected config paths: ${changedProtectedPaths.join(", ")}`,
|
||||
);
|
||||
}
|
||||
|
||||
const GATEWAY_ACTIONS = [
|
||||
"restart",
|
||||
"config.get",
|
||||
@@ -154,20 +222,24 @@ export function createGatewayTool(opts?: {
|
||||
const resolveConfigWriteParams = async (): Promise<{
|
||||
raw: string;
|
||||
baseHash: string;
|
||||
snapshotConfig: Record<string, unknown>;
|
||||
sessionKey: string | undefined;
|
||||
note: string | undefined;
|
||||
restartDelayMs: number | undefined;
|
||||
}> => {
|
||||
const raw = readStringParam(params, "raw", { required: true });
|
||||
const snapshot = await callGatewayTool("config.get", gatewayOpts, {});
|
||||
// Always fetch config.get so we can compare protected exec settings
|
||||
// against the current snapshot before forwarding any write RPC.
|
||||
const snapshotConfig = getSnapshotConfig(snapshot);
|
||||
let baseHash = readStringParam(params, "baseHash");
|
||||
if (!baseHash) {
|
||||
const snapshot = await callGatewayTool("config.get", gatewayOpts, {});
|
||||
baseHash = resolveBaseHashFromSnapshot(snapshot);
|
||||
}
|
||||
if (!baseHash) {
|
||||
throw new Error("Missing baseHash from config snapshot.");
|
||||
}
|
||||
return { raw, baseHash, ...resolveGatewayWriteMeta() };
|
||||
return { raw, baseHash, snapshotConfig, ...resolveGatewayWriteMeta() };
|
||||
};
|
||||
|
||||
if (action === "config.get") {
|
||||
@@ -183,8 +255,13 @@ export function createGatewayTool(opts?: {
|
||||
return jsonResult({ ok: true, result });
|
||||
}
|
||||
if (action === "config.apply") {
|
||||
const { raw, baseHash, sessionKey, note, restartDelayMs } =
|
||||
const { raw, baseHash, snapshotConfig, sessionKey, note, restartDelayMs } =
|
||||
await resolveConfigWriteParams();
|
||||
assertGatewayConfigMutationAllowed({
|
||||
action: "config.apply",
|
||||
currentConfig: snapshotConfig,
|
||||
raw,
|
||||
});
|
||||
const result = await callGatewayTool("config.apply", gatewayOpts, {
|
||||
raw,
|
||||
baseHash,
|
||||
@@ -195,8 +272,13 @@ export function createGatewayTool(opts?: {
|
||||
return jsonResult({ ok: true, result });
|
||||
}
|
||||
if (action === "config.patch") {
|
||||
const { raw, baseHash, sessionKey, note, restartDelayMs } =
|
||||
const { raw, baseHash, snapshotConfig, sessionKey, note, restartDelayMs } =
|
||||
await resolveConfigWriteParams();
|
||||
assertGatewayConfigMutationAllowed({
|
||||
action: "config.patch",
|
||||
currentConfig: snapshotConfig,
|
||||
raw,
|
||||
});
|
||||
const result = await callGatewayTool("config.patch", gatewayOpts, {
|
||||
raw,
|
||||
baseHash,
|
||||
|
||||
Reference in New Issue
Block a user