mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 15:40:44 +00:00
fix(gateway): tighten gateway config mutation guard (#69377)
* fix(gateway): tighten gateway config mutation guard Co-authored-by: zsx <git@zsxsoft.com> * fix(gateway): cover unkeyed guard entries Co-authored-by: zsx <git@zsxsoft.com> * fix(gateway): preserve array entry order in guard Co-authored-by: zsx <git@zsxsoft.com> * fix(gateway): close remaining review gaps * fix(gateway): stabilize dangerous flag ids * fix(gateway): log comment resolution * fix(gateway): block id removal stripping protected overrides * fix(gateway): drop review worklog * docs(changelog): note gateway tool config mutation guard expansion (#69377) --------- Co-authored-by: zsx <git@zsxsoft.com> Co-authored-by: Devin Robison <drobison@nvidia.com>
This commit is contained in:
@@ -20,6 +20,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Agents/compaction: rename embedded Pi compaction lifecycle events to `compaction_start` / `compaction_end` so OpenClaw stays aligned with `pi-coding-agent` 0.66.1 event naming. (#67713) Thanks @mpz4life.
|
||||
- Security/dotenv: block all `OPENCLAW_*` keys from untrusted workspace `.env` files so workspace-local env loading fails closed for new runtime-control variables instead of silently inheriting them. (#473)
|
||||
- Gateway/device pairing: restrict non-admin paired-device sessions (device-token auth) to their own pairing list, approve, and reject actions so a paired device cannot enumerate other devices or approve/reject pairing requests authored by another device. Admin and shared-secret operator sessions retain full visibility. (#69375) Thanks @eleqtrizit.
|
||||
- Agents/gateway tool: extend the agent-facing `gateway` tool's config mutation guard so model-driven `config.patch` and `config.apply` cannot rewrite operator-trusted paths (sandbox, plugin trust, gateway auth/TLS, hook routing and tokens, SSRF policy, MCP servers, workspace filesystem hardening) and cannot bypass the guard by editing per-agent sandbox, tools, or embedded-Pi overrides in place under `agents.list[]`. (#69377) Thanks @eleqtrizit.
|
||||
|
||||
## 2026.4.20
|
||||
|
||||
|
||||
455
src/agents/tools/gateway-tool-guard-coverage.test.ts
Normal file
455
src/agents/tools/gateway-tool-guard-coverage.test.ts
Normal file
@@ -0,0 +1,455 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
assertGatewayConfigMutationAllowedForTest,
|
||||
PROTECTED_GATEWAY_CONFIG_PATHS_FOR_TEST,
|
||||
} from "./gateway-tool.js";
|
||||
|
||||
function expectBlocked(
|
||||
currentConfig: Record<string, unknown>,
|
||||
patch: Record<string, unknown>,
|
||||
): void {
|
||||
expect(() =>
|
||||
assertGatewayConfigMutationAllowedForTest({
|
||||
action: "config.patch",
|
||||
currentConfig,
|
||||
raw: JSON.stringify(patch),
|
||||
}),
|
||||
).toThrow(/cannot (?:change protected|enable dangerous)/);
|
||||
}
|
||||
|
||||
function expectAllowed(
|
||||
currentConfig: Record<string, unknown>,
|
||||
patch: Record<string, unknown>,
|
||||
): void {
|
||||
expect(() =>
|
||||
assertGatewayConfigMutationAllowedForTest({
|
||||
action: "config.patch",
|
||||
currentConfig,
|
||||
raw: JSON.stringify(patch),
|
||||
}),
|
||||
).not.toThrow();
|
||||
}
|
||||
|
||||
function expectBlockedApply(
|
||||
currentConfig: Record<string, unknown>,
|
||||
nextConfig: Record<string, unknown>,
|
||||
): void {
|
||||
expect(() =>
|
||||
assertGatewayConfigMutationAllowedForTest({
|
||||
action: "config.apply",
|
||||
currentConfig,
|
||||
raw: JSON.stringify(nextConfig),
|
||||
}),
|
||||
).toThrow(/cannot (?:change protected|enable dangerous)/);
|
||||
}
|
||||
|
||||
function expectAllowedApply(
|
||||
currentConfig: Record<string, unknown>,
|
||||
nextConfig: Record<string, unknown>,
|
||||
): void {
|
||||
expect(() =>
|
||||
assertGatewayConfigMutationAllowedForTest({
|
||||
action: "config.apply",
|
||||
currentConfig,
|
||||
raw: JSON.stringify(nextConfig),
|
||||
}),
|
||||
).not.toThrow();
|
||||
}
|
||||
|
||||
describe("gateway config mutation guard coverage", () => {
|
||||
it("keeps advisory-critical protected path coverage in the production denylist", () => {
|
||||
expect(PROTECTED_GATEWAY_CONFIG_PATHS_FOR_TEST).toEqual(
|
||||
expect.arrayContaining([
|
||||
"agents.defaults.sandbox",
|
||||
"agents.list[].sandbox",
|
||||
"agents.list[].tools",
|
||||
"agents.list[].embeddedPi",
|
||||
"tools.fs",
|
||||
"plugins.allow",
|
||||
"plugins.entries",
|
||||
"hooks.token",
|
||||
"hooks.allowRequestSessionKey",
|
||||
"browser.ssrfPolicy",
|
||||
"mcp.servers",
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks disabling sandbox mode via config.patch", () => {
|
||||
expectBlocked(
|
||||
{ agents: { defaults: { sandbox: { mode: "all" } } } },
|
||||
{ agents: { defaults: { sandbox: { mode: "off" } } } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks enabling an installed-but-disabled plugin via config.patch", () => {
|
||||
expectBlocked(
|
||||
{ plugins: { entries: { malicious: { enabled: false } } } },
|
||||
{ plugins: { entries: { malicious: { enabled: true } } } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks clearing tools.fs.workspaceOnly hardening via config.patch", () => {
|
||||
expectBlocked(
|
||||
{ tools: { fs: { workspaceOnly: true } } },
|
||||
{ tools: { fs: { workspaceOnly: false } } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks enabling sandbox dangerouslyAllowContainerNamespaceJoin via config.patch", () => {
|
||||
expectBlocked(
|
||||
{
|
||||
agents: {
|
||||
defaults: {
|
||||
sandbox: {
|
||||
docker: { dangerouslyAllowContainerNamespaceJoin: false },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
defaults: {
|
||||
sandbox: {
|
||||
docker: { dangerouslyAllowContainerNamespaceJoin: true },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks unlocking exec/shell/spawn on /tools/invoke via gateway.tools.allow", () => {
|
||||
expectBlocked(
|
||||
{ gateway: { tools: { allow: [] as string[] } } },
|
||||
{ gateway: { tools: { allow: ["exec", "shell", "spawn"] } } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks in-place hooks.mappings sessionKey rewrite via mergeObjectArraysById", () => {
|
||||
expectBlocked(
|
||||
{
|
||||
hooks: {
|
||||
mappings: [{ id: "gmail", sessionKey: "hook:gmail:{{messages[0].id}}" }],
|
||||
},
|
||||
},
|
||||
{
|
||||
hooks: {
|
||||
mappings: [{ id: "gmail", sessionKey: "hook:{{payload.session}}" }],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks per-agent sandbox override under agents.list[]", () => {
|
||||
expectBlocked(
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "worker", sandbox: { mode: "all" } }],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "worker", sandbox: { mode: "off" } }],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks id-less per-agent sandbox injection under agents.list[]", () => {
|
||||
expectBlocked(
|
||||
{ agents: { list: [] as Array<Record<string, unknown>> } },
|
||||
{
|
||||
agents: {
|
||||
list: [{ sandbox: { mode: "off" } }],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks per-agent tools.allow override under agents.list[]", () => {
|
||||
expectBlocked(
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "worker", tools: { allow: [] as string[] } }],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "worker", tools: { allow: ["exec", "shell", "spawn"] } }],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks per-agent embeddedPi override under agents.list[]", () => {
|
||||
expectBlocked(
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "worker", embeddedPi: { executionContract: "strict-agentic" } }],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "worker", embeddedPi: { executionContract: "none" } }],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks subagent tool deny-list override via tools.subagents", () => {
|
||||
expectBlocked(
|
||||
{ tools: { subagents: { tools: { allow: [] as string[] } } } },
|
||||
{ tools: { subagents: { tools: { allow: ["gateway", "cron", "sessions_send"] } } } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks gateway.auth.token rewrite via config.patch", () => {
|
||||
expectBlocked(
|
||||
{ gateway: { auth: { mode: "token", token: "operator-secret" } } },
|
||||
{ gateway: { auth: { token: "attacker-known-token" } } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks gateway.tls.certPath redirect via config.patch", () => {
|
||||
expectBlocked(
|
||||
{ gateway: { tls: { enabled: true, certPath: "/etc/openclaw/cert.pem" } } },
|
||||
{ gateway: { tls: { certPath: "/tmp/attacker/cert.pem" } } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks plugins.load.paths injection via config.patch", () => {
|
||||
expectBlocked(
|
||||
{ plugins: { load: { paths: [] as string[] } } },
|
||||
{ plugins: { load: { paths: ["/tmp/malicious-plugin"] } } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks plugins.slots memory swap via config.patch", () => {
|
||||
expectBlocked(
|
||||
{ plugins: { slots: { memory: "official-memory" } } },
|
||||
{ plugins: { slots: { memory: "attacker-memory" } } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks root sandbox override via config.patch", () => {
|
||||
expectBlocked({ sandbox: { mode: "all" } }, { sandbox: { mode: "off" } });
|
||||
});
|
||||
|
||||
it("blocks plugins.allow edits via config.patch", () => {
|
||||
expectBlocked(
|
||||
{ plugins: { allow: ["trusted-plugin"] } },
|
||||
{ plugins: { allow: ["trusted-plugin", "evil-plugin"] } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks hooks.token rewrites via config.patch", () => {
|
||||
expectBlocked({ hooks: { token: "operator-secret" } }, { hooks: { token: "attacker-secret" } });
|
||||
});
|
||||
|
||||
it("blocks hooks.allowRequestSessionKey via config.patch", () => {
|
||||
expectBlocked(
|
||||
{ hooks: { allowRequestSessionKey: false } },
|
||||
{ hooks: { allowRequestSessionKey: true } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks browser.ssrfPolicy rewrites via config.patch", () => {
|
||||
expectBlocked(
|
||||
{ browser: { ssrfPolicy: { dangerouslyAllowPrivateNetwork: false } } },
|
||||
{ browser: { ssrfPolicy: { dangerouslyAllowPrivateNetwork: true } } },
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks mcp.servers rewrites via config.patch", () => {
|
||||
expectBlocked(
|
||||
{ mcp: { servers: {} } },
|
||||
{ mcp: { servers: { evil: { command: "nc", args: ["-e", "/bin/sh"] } } } },
|
||||
);
|
||||
});
|
||||
|
||||
it("allows adding a new agent without protected subfields via config.patch", () => {
|
||||
expectAllowed(
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "worker", sandbox: { mode: "all" } }],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "helper", model: "sonnet-4.6" }],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("allows removing an agent without protected subfields via config.apply", () => {
|
||||
expectAllowedApply(
|
||||
{
|
||||
agents: {
|
||||
list: [
|
||||
{ id: "worker", model: "sonnet-4.6" },
|
||||
{ id: "helper", sandbox: { mode: "all" } },
|
||||
],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "helper", sandbox: { mode: "all" } }],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks removing an agent that carries a protected sandbox override via config.apply", () => {
|
||||
expectBlockedApply(
|
||||
{
|
||||
agents: {
|
||||
list: [
|
||||
{ id: "worker", sandbox: { mode: "all" } },
|
||||
{ id: "helper", model: "sonnet-4.6" },
|
||||
],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "helper", model: "sonnet-4.6" }],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("allows reordering agents without protected changes via config.apply", () => {
|
||||
expectAllowedApply(
|
||||
{
|
||||
agents: {
|
||||
list: [
|
||||
{ id: "worker", sandbox: { mode: "all" } },
|
||||
{ id: "helper", sandbox: { mode: "all" } },
|
||||
],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
list: [
|
||||
{ id: "helper", sandbox: { mode: "all" } },
|
||||
{ id: "worker", sandbox: { mode: "all" } },
|
||||
],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("allows reordering agents when a dangerous per-agent sandbox flag is already enabled", () => {
|
||||
expectAllowedApply(
|
||||
{
|
||||
agents: {
|
||||
list: [
|
||||
{
|
||||
id: "worker",
|
||||
sandbox: {
|
||||
docker: { dangerouslyAllowContainerNamespaceJoin: true },
|
||||
},
|
||||
},
|
||||
{ id: "helper" },
|
||||
],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
list: [
|
||||
{ id: "helper" },
|
||||
{
|
||||
id: "worker",
|
||||
sandbox: {
|
||||
docker: { dangerouslyAllowContainerNamespaceJoin: true },
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks adding a new agent with a protected sandbox override via config.patch", () => {
|
||||
expectBlocked(
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "worker", sandbox: { mode: "all" } }],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "helper", sandbox: { mode: "off" } }],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("still allows benign agent-driven tweaks", () => {
|
||||
expectAllowed(
|
||||
{
|
||||
agents: {
|
||||
defaults: { prompt: "You are a helpful assistant." },
|
||||
list: [{ id: "worker", model: "sonnet-4" }],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
defaults: { prompt: "You are a terse assistant." },
|
||||
list: [{ id: "worker", model: "opus-4.6" }],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks config.apply replacing the config with protected changes", () => {
|
||||
expectBlockedApply(
|
||||
{
|
||||
agents: {
|
||||
defaults: { sandbox: { mode: "all" }, prompt: "You are a helpful assistant." },
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
defaults: { sandbox: { mode: "off" }, prompt: "You are a terse assistant." },
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks config.apply duplicate-id protected rewrites", () => {
|
||||
expectBlockedApply(
|
||||
{
|
||||
agents: {
|
||||
list: [{ id: "worker", sandbox: { mode: "all" } }],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
list: [
|
||||
{ id: "worker", sandbox: { mode: "off" } },
|
||||
{ id: "worker", sandbox: { mode: "all" } },
|
||||
],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("still allows benign config.apply replacements", () => {
|
||||
expectAllowedApply(
|
||||
{
|
||||
agents: {
|
||||
defaults: { prompt: "You are a helpful assistant." },
|
||||
list: [{ id: "worker", model: "sonnet-4" }],
|
||||
},
|
||||
},
|
||||
{
|
||||
agents: {
|
||||
defaults: { prompt: "You are a terse assistant." },
|
||||
list: [{ id: "worker", model: "opus-4.6" }],
|
||||
},
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -22,15 +22,69 @@ import { isOpenClawOwnerOnlyCoreToolName } from "./owner-only-tools.js";
|
||||
const log = createSubsystemLogger("gateway-tool");
|
||||
|
||||
const DEFAULT_UPDATE_TIMEOUT_MS = 20 * 60_000;
|
||||
// Security: the agent-facing `gateway` tool is owner-only, but per SECURITY.md the model/agent
|
||||
// itself is not a trusted principal. `assertGatewayConfigMutationAllowed` is the explicit
|
||||
// model -> operator trust-boundary control on `config.apply`/`config.patch`. Any operator-trusted
|
||||
// path listed here must not be changed by agent-driven mutations, including descendant keys
|
||||
// reached via deep merge or `mergeObjectArraysById` in-place edits.
|
||||
const PROTECTED_GATEWAY_CONFIG_PATHS = [
|
||||
// Exec consent / allowlist.
|
||||
"tools.exec.ask",
|
||||
"tools.exec.security",
|
||||
"tools.exec.safeBins",
|
||||
"tools.exec.safeBinProfiles",
|
||||
"tools.exec.safeBinTrustedDirs",
|
||||
"tools.exec.strictInlineEval",
|
||||
// Filesystem boundary.
|
||||
"tools.fs",
|
||||
// Sandbox isolation and per-agent sandbox overrides.
|
||||
"agents.defaults.sandbox",
|
||||
"agents.sandbox",
|
||||
"sandbox",
|
||||
"agents.list[].sandbox",
|
||||
// Per-agent tool/runtime execution policy.
|
||||
"agents.list[].tools",
|
||||
"agents.list[].embeddedPi",
|
||||
"tools.subagents",
|
||||
// Plugin trust boundary.
|
||||
"plugins.enabled",
|
||||
"plugins.allow",
|
||||
"plugins.deny",
|
||||
"plugins.entries",
|
||||
"plugins.installs",
|
||||
"plugins.load",
|
||||
"plugins.slots",
|
||||
// Gateway auth / TLS / HTTP tool exposure.
|
||||
"gateway.auth",
|
||||
"gateway.tls",
|
||||
"gateway.tools.allow",
|
||||
"gateway.tools.deny",
|
||||
// Hook auth/routing and extra trusted code loading.
|
||||
"hooks.token",
|
||||
"hooks.allowRequestSessionKey",
|
||||
"hooks.defaultSessionKey",
|
||||
"hooks.allowedSessionKeyPrefixes",
|
||||
"hooks.internal.load.extraDirs",
|
||||
"hooks.transformsDir",
|
||||
"hooks.mappings",
|
||||
// SSRF and MCP transport reach.
|
||||
"browser.ssrfPolicy",
|
||||
"tools.web.fetch.ssrfPolicy",
|
||||
"mcp.servers",
|
||||
] as const;
|
||||
|
||||
/** @internal Exposed for regression tests only; do not import from runtime code. */
|
||||
export const PROTECTED_GATEWAY_CONFIG_PATHS_FOR_TEST = PROTECTED_GATEWAY_CONFIG_PATHS;
|
||||
|
||||
/** @internal Exposed for regression tests only; do not import from runtime code. */
|
||||
export function assertGatewayConfigMutationAllowedForTest(params: {
|
||||
action: "config.apply" | "config.patch";
|
||||
currentConfig: Record<string, unknown>;
|
||||
raw: string;
|
||||
}): void {
|
||||
assertGatewayConfigMutationAllowed(params);
|
||||
}
|
||||
|
||||
function resolveBaseHashFromSnapshot(snapshot: unknown): string | undefined {
|
||||
if (!snapshot || typeof snapshot !== "object") {
|
||||
return undefined;
|
||||
@@ -95,6 +149,94 @@ function getValueAtPath(config: Record<string, unknown>, path: string): unknown
|
||||
return getValueAtCanonicalPath(config, path.replace(/^tools\.exec\./, "tools.bash."));
|
||||
}
|
||||
|
||||
function isProtectedPathEqual(
|
||||
currentConfig: Record<string, unknown>,
|
||||
nextConfig: Record<string, unknown>,
|
||||
path: string,
|
||||
): boolean {
|
||||
const bracketIdx = path.indexOf("[]");
|
||||
if (bracketIdx === -1) {
|
||||
return isDeepStrictEqual(getValueAtPath(currentConfig, path), getValueAtPath(nextConfig, path));
|
||||
}
|
||||
|
||||
const arrayPath = path.slice(0, bracketIdx);
|
||||
const subPath = path.slice(bracketIdx + "[]".length).replace(/^\./, "");
|
||||
const currentList = getValueAtCanonicalPath(currentConfig, arrayPath);
|
||||
const nextList = getValueAtCanonicalPath(nextConfig, arrayPath);
|
||||
if (!Array.isArray(currentList) && !Array.isArray(nextList)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const readProjectedEntries = (
|
||||
list: unknown,
|
||||
): {
|
||||
duplicateIds: boolean;
|
||||
hasUnkeyedProtectedValue: boolean;
|
||||
keyedValues: Map<string, unknown>;
|
||||
} => {
|
||||
if (!Array.isArray(list)) {
|
||||
return {
|
||||
duplicateIds: false,
|
||||
hasUnkeyedProtectedValue: false,
|
||||
keyedValues: new Map<string, unknown>(),
|
||||
};
|
||||
}
|
||||
let duplicateIds = false;
|
||||
let hasUnkeyedProtectedValue = false;
|
||||
const keyedValues = new Map<string, unknown>();
|
||||
for (const entry of list) {
|
||||
const id =
|
||||
entry &&
|
||||
typeof entry === "object" &&
|
||||
!Array.isArray(entry) &&
|
||||
typeof (entry as { id?: unknown }).id === "string" &&
|
||||
(entry as { id: string }).id.length > 0
|
||||
? (entry as { id: string }).id
|
||||
: undefined;
|
||||
const value =
|
||||
!subPath || !entry || typeof entry !== "object" || Array.isArray(entry)
|
||||
? entry
|
||||
: getValueAtCanonicalPath(entry as Record<string, unknown>, subPath);
|
||||
if (!id) {
|
||||
hasUnkeyedProtectedValue ||= value !== undefined;
|
||||
continue;
|
||||
}
|
||||
if (keyedValues.has(id)) {
|
||||
duplicateIds = true;
|
||||
continue;
|
||||
}
|
||||
keyedValues.set(id, value);
|
||||
}
|
||||
return { duplicateIds, hasUnkeyedProtectedValue, keyedValues };
|
||||
};
|
||||
|
||||
const currentProjected = readProjectedEntries(currentList);
|
||||
const nextProjected = readProjectedEntries(nextList);
|
||||
if (nextProjected.duplicateIds || nextProjected.hasUnkeyedProtectedValue) {
|
||||
return false;
|
||||
}
|
||||
for (const [id, currentValue] of currentProjected.keyedValues) {
|
||||
if (!nextProjected.keyedValues.has(id)) {
|
||||
// Dropping an entry that currently carries an operator-set protected
|
||||
// subfield value strips that operator state — treat as a protected
|
||||
// change so per-agent overrides cannot be removed via config.apply.
|
||||
if (currentValue !== undefined) {
|
||||
return false;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (!isDeepStrictEqual(currentValue, nextProjected.keyedValues.get(id))) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
for (const [id, nextValue] of nextProjected.keyedValues) {
|
||||
if (!currentProjected.keyedValues.has(id) && nextValue !== undefined) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function assertGatewayConfigMutationAllowed(params: {
|
||||
action: "config.apply" | "config.patch";
|
||||
currentConfig: Record<string, unknown>;
|
||||
@@ -108,11 +250,7 @@ function assertGatewayConfigMutationAllowed(params: {
|
||||
mergeObjectArraysById: true,
|
||||
}) as Record<string, unknown>);
|
||||
const changedProtectedPaths = PROTECTED_GATEWAY_CONFIG_PATHS.filter(
|
||||
(path) =>
|
||||
!isDeepStrictEqual(
|
||||
getValueAtPath(params.currentConfig, path),
|
||||
getValueAtPath(nextConfig, path),
|
||||
),
|
||||
(path) => !isProtectedPathEqual(params.currentConfig, nextConfig, path),
|
||||
);
|
||||
if (changedProtectedPaths.length > 0) {
|
||||
throw new Error(
|
||||
|
||||
@@ -78,4 +78,105 @@ describe("collectEnabledInsecureOrDangerousFlags", () => {
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("collects dangerous sandbox, hook, browser, and fs flags", () => {
|
||||
expect(
|
||||
collectEnabledInsecureOrDangerousFlags(
|
||||
asConfig({
|
||||
agents: {
|
||||
defaults: {
|
||||
sandbox: {
|
||||
docker: {
|
||||
dangerouslyAllowReservedContainerTargets: true,
|
||||
dangerouslyAllowContainerNamespaceJoin: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
list: [
|
||||
{
|
||||
id: "worker",
|
||||
sandbox: {
|
||||
docker: {
|
||||
dangerouslyAllowExternalBindSources: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
hooks: {
|
||||
allowRequestSessionKey: true,
|
||||
},
|
||||
browser: {
|
||||
ssrfPolicy: {
|
||||
dangerouslyAllowPrivateNetwork: true,
|
||||
},
|
||||
},
|
||||
tools: {
|
||||
fs: {
|
||||
workspaceOnly: false,
|
||||
},
|
||||
},
|
||||
}),
|
||||
),
|
||||
).toEqual(
|
||||
expect.arrayContaining([
|
||||
"agents.defaults.sandbox.docker.dangerouslyAllowReservedContainerTargets=true",
|
||||
"agents.defaults.sandbox.docker.dangerouslyAllowContainerNamespaceJoin=true",
|
||||
'agents.list[id="worker"].sandbox.docker.dangerouslyAllowExternalBindSources=true',
|
||||
"hooks.allowRequestSessionKey=true",
|
||||
"browser.ssrfPolicy.dangerouslyAllowPrivateNetwork=true",
|
||||
"tools.fs.workspaceOnly=false",
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses stable agent ids for per-agent dangerous sandbox flags", () => {
|
||||
expect(
|
||||
collectEnabledInsecureOrDangerousFlags(
|
||||
asConfig({
|
||||
agents: {
|
||||
list: [
|
||||
{
|
||||
id: "worker",
|
||||
sandbox: {
|
||||
docker: {
|
||||
dangerouslyAllowContainerNamespaceJoin: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
id: "helper",
|
||||
},
|
||||
],
|
||||
},
|
||||
}),
|
||||
),
|
||||
).toContain(
|
||||
'agents.list[id="worker"].sandbox.docker.dangerouslyAllowContainerNamespaceJoin=true',
|
||||
);
|
||||
|
||||
expect(
|
||||
collectEnabledInsecureOrDangerousFlags(
|
||||
asConfig({
|
||||
agents: {
|
||||
list: [
|
||||
{
|
||||
id: "helper",
|
||||
},
|
||||
{
|
||||
id: "worker",
|
||||
sandbox: {
|
||||
docker: {
|
||||
dangerouslyAllowContainerNamespaceJoin: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
}),
|
||||
),
|
||||
).toContain(
|
||||
'agents.list[id="worker"].sandbox.docker.dangerouslyAllowContainerNamespaceJoin=true',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../agents/agent-scope.js";
|
||||
import { DANGEROUS_SANDBOX_DOCKER_BOOLEAN_KEYS } from "../agents/sandbox/config.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import {
|
||||
collectPluginConfigContractMatches,
|
||||
@@ -10,8 +11,35 @@ function formatDangerousConfigFlagValue(value: string | number | boolean | null)
|
||||
return value === null ? "null" : String(value);
|
||||
}
|
||||
|
||||
function getAgentDangerousFlagPathSegment(agent: unknown, index: number): string {
|
||||
const id =
|
||||
agent &&
|
||||
typeof agent === "object" &&
|
||||
!Array.isArray(agent) &&
|
||||
typeof (agent as { id?: unknown }).id === "string" &&
|
||||
(agent as { id: string }).id.length > 0
|
||||
? (agent as { id: string }).id
|
||||
: undefined;
|
||||
return id ? `agents.list[id=${JSON.stringify(id)}]` : `agents.list[${index}]`;
|
||||
}
|
||||
|
||||
export function collectEnabledInsecureOrDangerousFlags(cfg: OpenClawConfig): string[] {
|
||||
const enabledFlags: string[] = [];
|
||||
|
||||
const collectSandboxDockerDangerousFlags = (
|
||||
docker: Record<string, unknown> | undefined,
|
||||
pathPrefix: string,
|
||||
): void => {
|
||||
if (!isRecord(docker)) {
|
||||
return;
|
||||
}
|
||||
for (const key of DANGEROUS_SANDBOX_DOCKER_BOOLEAN_KEYS) {
|
||||
if (docker[key] === true) {
|
||||
enabledFlags.push(`${pathPrefix}.${key}=true`);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
if (cfg.gateway?.controlUi?.allowInsecureAuth === true) {
|
||||
enabledFlags.push("gateway.controlUi.allowInsecureAuth=true");
|
||||
}
|
||||
@@ -31,9 +59,32 @@ export function collectEnabledInsecureOrDangerousFlags(cfg: OpenClawConfig): str
|
||||
}
|
||||
}
|
||||
}
|
||||
if (cfg.hooks?.allowRequestSessionKey === true) {
|
||||
enabledFlags.push("hooks.allowRequestSessionKey=true");
|
||||
}
|
||||
if (cfg.browser?.ssrfPolicy?.dangerouslyAllowPrivateNetwork === true) {
|
||||
enabledFlags.push("browser.ssrfPolicy.dangerouslyAllowPrivateNetwork=true");
|
||||
}
|
||||
if (cfg.tools?.exec?.applyPatch?.workspaceOnly === false) {
|
||||
enabledFlags.push("tools.exec.applyPatch.workspaceOnly=false");
|
||||
}
|
||||
if (cfg.tools?.fs?.workspaceOnly === false) {
|
||||
enabledFlags.push("tools.fs.workspaceOnly=false");
|
||||
}
|
||||
collectSandboxDockerDangerousFlags(
|
||||
isRecord(cfg.agents?.defaults?.sandbox?.docker)
|
||||
? cfg.agents?.defaults?.sandbox?.docker
|
||||
: undefined,
|
||||
"agents.defaults.sandbox.docker",
|
||||
);
|
||||
if (Array.isArray(cfg.agents?.list)) {
|
||||
for (const [index, agent] of cfg.agents.list.entries()) {
|
||||
collectSandboxDockerDangerousFlags(
|
||||
isRecord(agent?.sandbox?.docker) ? agent.sandbox.docker : undefined,
|
||||
`${getAgentDangerousFlagPathSegment(agent, index)}.sandbox.docker`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const pluginEntries = cfg.plugins?.entries;
|
||||
if (!isRecord(pluginEntries)) {
|
||||
|
||||
Reference in New Issue
Block a user