From fbb79d4013000552d6a2c23b9613d8b3cb92f6b6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 21 Feb 2026 12:49:45 +0100 Subject: [PATCH] fix(security): harden runtime command override gating --- CHANGELOG.md | 1 + src/auto-reply/commands-registry.test.ts | 14 +++++++++++ src/auto-reply/commands-registry.ts | 7 +++--- src/auto-reply/reply/bash-command.ts | 3 ++- src/auto-reply/reply/commands-allowlist.ts | 3 ++- src/auto-reply/reply/commands-config.ts | 5 ++-- src/auto-reply/reply/commands.test.ts | 24 +++++++++++++++++++ src/auto-reply/status.ts | 5 ++-- src/config/commands.test.ts | 24 +++++++++++++++++++ src/config/commands.ts | 20 ++++++++++++++-- src/config/runtime-overrides.test.ts | 28 ++++++++++++++++++++++ src/config/runtime-overrides.ts | 28 ++++++++++++++++++++-- 12 files changed, 149 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cafd5a179..e625e71b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Docs: https://docs.openclaw.ai - Gateway/Security: require secure context and paired-device checks for Control UI auth even when `gateway.controlUi.allowInsecureAuth` is set, and align audit messaging with the hardened behavior. (#20684) thanks @coygeek. - Docker/Security: run E2E and install-sh test images as non-root by adding appuser directives. Thanks @thewilloftheshadow. - Skills/Security: sanitize skill env overrides to block unsafe runtime injection variables and only allow sensitive keys when declared in skill metadata, with warnings for suspicious values. Thanks @thewilloftheshadow. +- Security/Commands: block prototype-key injection in runtime `/debug` overrides and require own-property checks for gated command flags (`bash`, `config`, `debug`) so inherited prototype values cannot enable privileged commands. Thanks @tdjackey for reporting. - Security/Browser: block non-network browser navigation protocols (including `file:`, `data:`, and `javascript:`) while preserving `about:blank`, preventing local file reads via browser tool navigation. This ships in the next npm release. Thanks @q1uf3ng for reporting. - Security/Exec: block shell startup-file env injection (`BASH_ENV`, `ENV`, `BASH_FUNC_*`, `LD_*`, `DYLD_*`) across config env ingestion, node-host inherited environment sanitization, and macOS exec host runtime to prevent pre-command execution from attacker-controlled environment variables. This ships in the next npm release. Thanks @tdjackey. - Security/Exec (Windows): canonicalize `cmd.exe /c` command text across validation, approval binding, and audit/event rendering to prevent trailing-argument approval mismatches in `system.run`. This ships in the next npm release. Thanks @tdjackey for reporting. diff --git a/src/auto-reply/commands-registry.test.ts b/src/auto-reply/commands-registry.test.ts index a38bb5a10..b05e5ea83 100644 --- a/src/auto-reply/commands-registry.test.ts +++ b/src/auto-reply/commands-registry.test.ts @@ -62,6 +62,20 @@ describe("commands registry", () => { expect(nativeDisabled.find((spec) => spec.name === "debug")).toBeFalsy(); }); + it("does not enable restricted commands from inherited flags", () => { + const inheritedCommands = Object.create({ + config: true, + debug: true, + bash: true, + }) as Record; + const commands = listChatCommandsForConfig({ + commands: inheritedCommands as never, + }); + expect(commands.find((spec) => spec.key === "config")).toBeFalsy(); + expect(commands.find((spec) => spec.key === "debug")).toBeFalsy(); + expect(commands.find((spec) => spec.key === "bash")).toBeFalsy(); + }); + it("appends skill commands when provided", () => { const skillCommands = [ { diff --git a/src/auto-reply/commands-registry.ts b/src/auto-reply/commands-registry.ts index dbfc789e7..34ca31492 100644 --- a/src/auto-reply/commands-registry.ts +++ b/src/auto-reply/commands-registry.ts @@ -1,6 +1,7 @@ import { DEFAULT_MODEL, DEFAULT_PROVIDER } from "../agents/defaults.js"; import { resolveConfiguredModelRef } from "../agents/model-selection.js"; import type { SkillCommandSpec } from "../agents/skills.js"; +import { isCommandFlagEnabled } from "../config/commands.js"; import type { OpenClawConfig } from "../config/types.js"; import { escapeRegExp } from "../utils.js"; import { getChatCommands, getNativeCommandSurfaces } from "./commands-registry.data.js"; @@ -96,13 +97,13 @@ export function listChatCommands(params?: { export function isCommandEnabled(cfg: OpenClawConfig, commandKey: string): boolean { if (commandKey === "config") { - return cfg.commands?.config === true; + return isCommandFlagEnabled(cfg, "config"); } if (commandKey === "debug") { - return cfg.commands?.debug === true; + return isCommandFlagEnabled(cfg, "debug"); } if (commandKey === "bash") { - return cfg.commands?.bash === true; + return isCommandFlagEnabled(cfg, "bash"); } return true; } diff --git a/src/auto-reply/reply/bash-command.ts b/src/auto-reply/reply/bash-command.ts index 49a1c4df1..c442dbe1b 100644 --- a/src/auto-reply/reply/bash-command.ts +++ b/src/auto-reply/reply/bash-command.ts @@ -3,6 +3,7 @@ import { getFinishedSession, getSession, markExited } from "../../agents/bash-pr import { createExecTool } from "../../agents/bash-tools.js"; import { resolveSandboxRuntimeStatus } from "../../agents/sandbox.js"; import { killProcessTree } from "../../agents/shell-utils.js"; +import { isCommandFlagEnabled } from "../../config/commands.js"; import type { OpenClawConfig } from "../../config/config.js"; import { logVerbose } from "../../globals.js"; import { clampInt } from "../../utils.js"; @@ -186,7 +187,7 @@ export async function handleBashChatCommand(params: { failures: Array<{ gate: string; key: string }>; }; }): Promise { - if (params.cfg.commands?.bash !== true) { + if (!isCommandFlagEnabled(params.cfg, "bash")) { return { text: "⚠️ bash is disabled. Set commands.bash=true to enable. Docs: https://docs.openclaw.ai/tools/slash-commands#config", }; diff --git a/src/auto-reply/reply/commands-allowlist.ts b/src/auto-reply/reply/commands-allowlist.ts index 3b1b88776..79d040b97 100644 --- a/src/auto-reply/reply/commands-allowlist.ts +++ b/src/auto-reply/reply/commands-allowlist.ts @@ -3,6 +3,7 @@ import { resolveChannelConfigWrites } from "../../channels/plugins/config-writes import { listPairingChannels } from "../../channels/plugins/pairing.js"; import type { ChannelId } from "../../channels/plugins/types.js"; import { normalizeChannelId } from "../../channels/registry.js"; +import { isCommandFlagEnabled } from "../../config/commands.js"; import type { OpenClawConfig } from "../../config/config.js"; import { readConfigFileSnapshot, @@ -519,7 +520,7 @@ export const handleAllowlistCommand: CommandHandler = async (params, allowTextCo return { shouldContinue: false, reply: { text: lines.join("\n") } }; } - if (params.cfg.commands?.config !== true) { + if (!isCommandFlagEnabled(params.cfg, "config")) { return { shouldContinue: false, reply: { text: "⚠️ /allowlist edits are disabled. Set commands.config=true to enable." }, diff --git a/src/auto-reply/reply/commands-config.ts b/src/auto-reply/reply/commands-config.ts index 87aa8732f..c6578d7d4 100644 --- a/src/auto-reply/reply/commands-config.ts +++ b/src/auto-reply/reply/commands-config.ts @@ -1,5 +1,6 @@ import { resolveChannelConfigWrites } from "../../channels/plugins/config-writes.js"; import { normalizeChannelId } from "../../channels/registry.js"; +import { isCommandFlagEnabled } from "../../config/commands.js"; import { getConfigValueAtPath, parseConfigPath, @@ -36,7 +37,7 @@ export const handleConfigCommand: CommandHandler = async (params, allowTextComma ); return { shouldContinue: false }; } - if (params.cfg.commands?.config !== true) { + if (!isCommandFlagEnabled(params.cfg, "config")) { return { shouldContinue: false, reply: { @@ -190,7 +191,7 @@ export const handleDebugCommand: CommandHandler = async (params, allowTextComman ); return { shouldContinue: false }; } - if (params.cfg.commands?.debug !== true) { + if (!isCommandFlagEnabled(params.cfg, "debug")) { return { shouldContinue: false, reply: { diff --git a/src/auto-reply/reply/commands.test.ts b/src/auto-reply/reply/commands.test.ts index d0a80beb0..842aaa3ff 100644 --- a/src/auto-reply/reply/commands.test.ts +++ b/src/auto-reply/reply/commands.test.ts @@ -186,6 +186,30 @@ describe("handleCommands gating", () => { expect(result.shouldContinue).toBe(false); expect(result.reply?.text).toContain("/debug is disabled"); }); + + it("does not enable gated commands from inherited command flags", async () => { + const inheritedCommands = Object.create({ + bash: true, + config: true, + debug: true, + }) as Record; + const cfg = { + commands: inheritedCommands as never, + channels: { whatsapp: { allowFrom: ["*"] } }, + } as OpenClawConfig; + + const bashResult = await handleCommands(buildParams("/bash echo hi", cfg)); + expect(bashResult.shouldContinue).toBe(false); + expect(bashResult.reply?.text).toContain("bash is disabled"); + + const configResult = await handleCommands(buildParams("/config show", cfg)); + expect(configResult.shouldContinue).toBe(false); + expect(configResult.reply?.text).toContain("/config is disabled"); + + const debugResult = await handleCommands(buildParams("/debug show", cfg)); + expect(debugResult.shouldContinue).toBe(false); + expect(debugResult.reply?.text).toContain("/debug is disabled"); + }); }); describe("/approve command", () => { diff --git a/src/auto-reply/status.ts b/src/auto-reply/status.ts index 8148cb12e..399997ea2 100644 --- a/src/auto-reply/status.ts +++ b/src/auto-reply/status.ts @@ -11,6 +11,7 @@ import { resolveSandboxRuntimeStatus } from "../agents/sandbox.js"; import type { SkillCommandSpec } from "../agents/skills.js"; import { derivePromptTokens, normalizeUsage, type UsageLike } from "../agents/usage.js"; import { resolveChannelModelOverride } from "../channels/model-overrides.js"; +import { isCommandFlagEnabled } from "../config/commands.js"; import type { OpenClawConfig } from "../config/config.js"; import { resolveMainSessionKey, @@ -688,10 +689,10 @@ export function buildHelpMessage(cfg?: OpenClawConfig): string { lines.push(""); const optionParts = ["/think ", "/model ", "/verbose on|off"]; - if (cfg?.commands?.config === true) { + if (isCommandFlagEnabled(cfg, "config")) { optionParts.push("/config"); } - if (cfg?.commands?.debug === true) { + if (isCommandFlagEnabled(cfg, "debug")) { optionParts.push("/debug"); } lines.push("Options"); diff --git a/src/config/commands.test.ts b/src/config/commands.test.ts index eb9060767..c96c8122a 100644 --- a/src/config/commands.test.ts +++ b/src/config/commands.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + isCommandFlagEnabled, isRestartEnabled, isNativeCommandsExplicitlyDisabled, resolveNativeCommandsEnabled, @@ -107,4 +108,27 @@ describe("isRestartEnabled", () => { expect(isRestartEnabled({ commands: { restart: true } })).toBe(true); expect(isRestartEnabled({ commands: { restart: false } })).toBe(false); }); + + it("ignores inherited restart flags", () => { + expect( + isRestartEnabled({ + commands: Object.create({ restart: false }) as Record, + }), + ).toBe(true); + }); +}); + +describe("isCommandFlagEnabled", () => { + it("requires own boolean true", () => { + expect(isCommandFlagEnabled({ commands: { bash: true } }, "bash")).toBe(true); + expect(isCommandFlagEnabled({ commands: { bash: false } }, "bash")).toBe(false); + expect( + isCommandFlagEnabled( + { + commands: Object.create({ bash: true }) as Record, + }, + "bash", + ), + ).toBe(false); + }); }); diff --git a/src/config/commands.ts b/src/config/commands.ts index c5b145d76..1be69e970 100644 --- a/src/config/commands.ts +++ b/src/config/commands.ts @@ -1,5 +1,6 @@ import { normalizeChannelId } from "../channels/plugins/index.js"; import type { ChannelId } from "../channels/plugins/types.js"; +import { isPlainObject } from "../infra/plain-object.js"; import type { NativeCommandsSetting } from "./types.js"; function resolveAutoDefault(providerId?: ChannelId): boolean { @@ -62,6 +63,21 @@ export function isNativeCommandsExplicitlyDisabled(params: { return false; } -export function isRestartEnabled(config?: { commands?: { restart?: boolean } }): boolean { - return config?.commands?.restart !== false; +function getOwnCommandFlagValue(config: { commands?: unknown } | undefined, key: string): unknown { + const { commands } = config ?? {}; + if (!isPlainObject(commands) || !Object.hasOwn(commands, key)) { + return undefined; + } + return commands[key]; +} + +export function isCommandFlagEnabled( + config: { commands?: unknown } | undefined, + key: string, +): boolean { + return getOwnCommandFlagValue(config, key) === true; +} + +export function isRestartEnabled(config?: { commands?: unknown }): boolean { + return getOwnCommandFlagValue(config, "restart") !== false; } diff --git a/src/config/runtime-overrides.test.ts b/src/config/runtime-overrides.test.ts index 2f859e10b..1dee97805 100644 --- a/src/config/runtime-overrides.test.ts +++ b/src/config/runtime-overrides.test.ts @@ -48,4 +48,32 @@ describe("runtime overrides", () => { expect(Object.keys(getConfigOverrides()).length).toBe(0); } }); + + it("blocks __proto__ keys inside override object values", () => { + const cfg = { commands: {} } as OpenClawConfig; + setConfigOverride("commands", JSON.parse('{"__proto__":{"bash":true}}')); + + const next = applyConfigOverrides(cfg); + expect(next.commands?.bash).toBeUndefined(); + expect(Object.prototype.hasOwnProperty.call(next.commands ?? {}, "bash")).toBe(false); + }); + + it("blocks constructor/prototype keys inside override object values", () => { + const cfg = { commands: {} } as OpenClawConfig; + setConfigOverride("commands", JSON.parse('{"constructor":{"prototype":{"bash":true}}}')); + + const next = applyConfigOverrides(cfg); + expect(next.commands?.bash).toBeUndefined(); + expect(Object.prototype.hasOwnProperty.call(next.commands ?? {}, "bash")).toBe(false); + }); + + it("sanitizes blocked object keys when writing overrides", () => { + setConfigOverride("commands", JSON.parse('{"__proto__":{"bash":true},"debug":true}')); + + expect(getConfigOverrides()).toEqual({ + commands: { + debug: true, + }, + }); + }); }); diff --git a/src/config/runtime-overrides.ts b/src/config/runtime-overrides.ts index 14de08304..da3a2ec25 100644 --- a/src/config/runtime-overrides.ts +++ b/src/config/runtime-overrides.ts @@ -4,15 +4,39 @@ import type { OpenClawConfig } from "./types.js"; type OverrideTree = Record; +const BLOCKED_MERGE_KEYS = new Set(["__proto__", "prototype", "constructor"]); + let overrides: OverrideTree = {}; +function sanitizeOverrideValue(value: unknown, seen = new WeakSet()): unknown { + if (Array.isArray(value)) { + return value.map((entry) => sanitizeOverrideValue(entry, seen)); + } + if (!isPlainObject(value)) { + return value; + } + if (seen.has(value)) { + return {}; + } + seen.add(value); + const sanitized: OverrideTree = {}; + for (const [key, entry] of Object.entries(value)) { + if (entry === undefined || BLOCKED_MERGE_KEYS.has(key)) { + continue; + } + sanitized[key] = sanitizeOverrideValue(entry, seen); + } + seen.delete(value); + return sanitized; +} + function mergeOverrides(base: unknown, override: unknown): unknown { if (!isPlainObject(base) || !isPlainObject(override)) { return override; } const next: OverrideTree = { ...base }; for (const [key, value] of Object.entries(override)) { - if (value === undefined) { + if (value === undefined || BLOCKED_MERGE_KEYS.has(key)) { continue; } next[key] = mergeOverrides((base as OverrideTree)[key], value); @@ -39,7 +63,7 @@ export function setConfigOverride( if (!parsed.ok || !parsed.path) { return { ok: false, error: parsed.error ?? "Invalid path." }; } - setConfigValueAtPath(overrides, parsed.path, value); + setConfigValueAtPath(overrides, parsed.path, sanitizeOverrideValue(value)); return { ok: true }; }