fix(security): harden runtime command override gating
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
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 = [
|
||||
{
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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<ReplyPayload> {
|
||||
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",
|
||||
};
|
||||
|
||||
@@ -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." },
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
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", () => {
|
||||
|
||||
@@ -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 <level>", "/model <id>", "/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");
|
||||
|
||||
@@ -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<string, unknown>,
|
||||
}),
|
||||
).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<string, unknown>,
|
||||
},
|
||||
"bash",
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,15 +4,39 @@ import type { OpenClawConfig } from "./types.js";
|
||||
|
||||
type OverrideTree = Record<string, unknown>;
|
||||
|
||||
const BLOCKED_MERGE_KEYS = new Set(["__proto__", "prototype", "constructor"]);
|
||||
|
||||
let overrides: OverrideTree = {};
|
||||
|
||||
function sanitizeOverrideValue(value: unknown, seen = new WeakSet<object>()): 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 };
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user