refactor(security): unify command gating and blocked-key guards

This commit is contained in:
Peter Steinberger
2026-02-21 13:04:31 +01:00
parent 356d61aacf
commit 08e020881d
10 changed files with 111 additions and 58 deletions

View File

@@ -9,6 +9,7 @@ import { logVerbose } from "../../globals.js";
import { clampInt } from "../../utils.js";
import type { MsgContext } from "../templating.js";
import type { ReplyPayload } from "../types.js";
import { buildDisabledCommandReply } from "./command-gates.js";
import { formatElevatedUnavailableMessage } from "./elevated-unavailable.js";
import { stripMentions, stripStructuralPrefixes } from "./mentions.js";
@@ -188,9 +189,11 @@ export async function handleBashChatCommand(params: {
};
}): Promise<ReplyPayload> {
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",
};
return buildDisabledCommandReply({
label: "bash",
configKey: "bash",
docsUrl: "https://docs.openclaw.ai/tools/slash-commands#config",
});
}
const agentId =

View File

@@ -0,0 +1,49 @@
import type { CommandFlagKey } from "../../config/commands.js";
import { isCommandFlagEnabled } from "../../config/commands.js";
import { logVerbose } from "../../globals.js";
import type { ReplyPayload } from "../types.js";
import type { CommandHandlerResult, HandleCommandsParams } from "./commands-types.js";
export function rejectUnauthorizedCommand(
params: HandleCommandsParams,
commandLabel: string,
): CommandHandlerResult | null {
if (params.command.isAuthorizedSender) {
return null;
}
logVerbose(
`Ignoring ${commandLabel} from unauthorized sender: ${params.command.senderId || "<unknown>"}`,
);
return { shouldContinue: false };
}
export function buildDisabledCommandReply(params: {
label: string;
configKey: CommandFlagKey;
disabledVerb?: "is" | "are";
docsUrl?: string;
}): ReplyPayload {
const disabledVerb = params.disabledVerb ?? "is";
const docsSuffix = params.docsUrl ? ` Docs: ${params.docsUrl}` : "";
return {
text: `⚠️ ${params.label} ${disabledVerb} disabled. Set commands.${params.configKey}=true to enable.${docsSuffix}`,
};
}
export function requireCommandFlagEnabled(
cfg: { commands?: unknown } | undefined,
params: {
label: string;
configKey: CommandFlagKey;
disabledVerb?: "is" | "are";
docsUrl?: string;
},
): CommandHandlerResult | null {
if (isCommandFlagEnabled(cfg, params.configKey)) {
return null;
}
return {
shouldContinue: false,
reply: buildDisabledCommandReply(params),
};
}

View File

@@ -3,7 +3,6 @@ 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,
@@ -12,7 +11,6 @@ import {
} from "../../config/config.js";
import { resolveDiscordAccount } from "../../discord/accounts.js";
import { resolveDiscordUserAllowlist } from "../../discord/resolve-users.js";
import { logVerbose } from "../../globals.js";
import { resolveIMessageAccount } from "../../imessage/accounts.js";
import {
addChannelAllowFromStoreEntry,
@@ -25,6 +23,7 @@ import { resolveSlackAccount } from "../../slack/accounts.js";
import { resolveSlackUserAllowlist } from "../../slack/resolve-users.js";
import { resolveTelegramAccount } from "../../telegram/accounts.js";
import { resolveWhatsAppAccount } from "../../web/accounts.js";
import { rejectUnauthorizedCommand, requireCommandFlagEnabled } from "./command-gates.js";
import type { CommandHandler } from "./commands-types.js";
type AllowlistScope = "dm" | "group" | "all";
@@ -331,11 +330,9 @@ export const handleAllowlistCommand: CommandHandler = async (params, allowTextCo
if (parsed.action === "error") {
return { shouldContinue: false, reply: { text: `⚠️ ${parsed.message}` } };
}
if (!params.command.isAuthorizedSender) {
logVerbose(
`Ignoring /allowlist from unauthorized sender: ${params.command.senderId || "<unknown>"}`,
);
return { shouldContinue: false };
const unauthorized = rejectUnauthorizedCommand(params, "/allowlist");
if (unauthorized) {
return unauthorized;
}
const channelId =
@@ -520,11 +517,13 @@ export const handleAllowlistCommand: CommandHandler = async (params, allowTextCo
return { shouldContinue: false, reply: { text: lines.join("\n") } };
}
if (!isCommandFlagEnabled(params.cfg, "config")) {
return {
shouldContinue: false,
reply: { text: "⚠️ /allowlist edits are disabled. Set commands.config=true to enable." },
};
const disabled = requireCommandFlagEnabled(params.cfg, {
label: "/allowlist edits",
configKey: "config",
disabledVerb: "are",
});
if (disabled) {
return disabled;
}
const shouldUpdateConfig = parsed.target !== "store";

View File

@@ -1,5 +1,5 @@
import { logVerbose } from "../../globals.js";
import { handleBashChatCommand } from "./bash-command.js";
import { rejectUnauthorizedCommand } from "./command-gates.js";
import type { CommandHandler } from "./commands-types.js";
export const handleBashCommand: CommandHandler = async (params, allowTextCommands) => {
@@ -13,9 +13,9 @@ export const handleBashCommand: CommandHandler = async (params, allowTextCommand
if (!bashSlashRequested && !(bashBangRequested && command.isAuthorizedSender)) {
return null;
}
if (!command.isAuthorizedSender) {
logVerbose(`Ignoring /bash from unauthorized sender: ${command.senderId || "<unknown>"}`);
return { shouldContinue: false };
const unauthorized = rejectUnauthorizedCommand(params, "/bash");
if (unauthorized) {
return unauthorized;
}
const reply = await handleBashChatCommand({
ctx: params.ctx,

View File

@@ -1,6 +1,5 @@
import { resolveChannelConfigWrites } from "../../channels/plugins/config-writes.js";
import { normalizeChannelId } from "../../channels/registry.js";
import { isCommandFlagEnabled } from "../../config/commands.js";
import {
getConfigValueAtPath,
parseConfigPath,
@@ -18,7 +17,7 @@ import {
setConfigOverride,
unsetConfigOverride,
} from "../../config/runtime-overrides.js";
import { logVerbose } from "../../globals.js";
import { rejectUnauthorizedCommand, requireCommandFlagEnabled } from "./command-gates.js";
import type { CommandHandler } from "./commands-types.js";
import { parseConfigCommand } from "./config-commands.js";
import { parseDebugCommand } from "./debug-commands.js";
@@ -31,19 +30,16 @@ export const handleConfigCommand: CommandHandler = async (params, allowTextComma
if (!configCommand) {
return null;
}
if (!params.command.isAuthorizedSender) {
logVerbose(
`Ignoring /config from unauthorized sender: ${params.command.senderId || "<unknown>"}`,
);
return { shouldContinue: false };
const unauthorized = rejectUnauthorizedCommand(params, "/config");
if (unauthorized) {
return unauthorized;
}
if (!isCommandFlagEnabled(params.cfg, "config")) {
return {
shouldContinue: false,
reply: {
text: "⚠️ /config is disabled. Set commands.config=true to enable.",
},
};
const disabled = requireCommandFlagEnabled(params.cfg, {
label: "/config",
configKey: "config",
});
if (disabled) {
return disabled;
}
if (configCommand.action === "error") {
return {
@@ -185,19 +181,16 @@ export const handleDebugCommand: CommandHandler = async (params, allowTextComman
if (!debugCommand) {
return null;
}
if (!params.command.isAuthorizedSender) {
logVerbose(
`Ignoring /debug from unauthorized sender: ${params.command.senderId || "<unknown>"}`,
);
return { shouldContinue: false };
const unauthorized = rejectUnauthorizedCommand(params, "/debug");
if (unauthorized) {
return unauthorized;
}
if (!isCommandFlagEnabled(params.cfg, "debug")) {
return {
shouldContinue: false,
reply: {
text: "⚠️ /debug is disabled. Set commands.debug=true to enable.",
},
};
const disabled = requireCommandFlagEnabled(params.cfg, {
label: "/debug",
configKey: "debug",
});
if (disabled) {
return disabled;
}
if (debugCommand.action === "error") {
return {

View File

@@ -1,7 +1,11 @@
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";
import type { CommandsConfig, NativeCommandsSetting } from "./types.js";
export type CommandFlagKey = {
[K in keyof CommandsConfig]-?: Exclude<CommandsConfig[K], undefined> extends boolean ? K : never;
}[keyof CommandsConfig];
function resolveAutoDefault(providerId?: ChannelId): boolean {
const id = normalizeChannelId(providerId);
@@ -63,7 +67,10 @@ export function isNativeCommandsExplicitlyDisabled(params: {
return false;
}
function getOwnCommandFlagValue(config: { commands?: unknown } | undefined, key: string): unknown {
function getOwnCommandFlagValue(
config: { commands?: unknown } | undefined,
key: CommandFlagKey,
): unknown {
const { commands } = config ?? {};
if (!isPlainObject(commands) || !Object.hasOwn(commands, key)) {
return undefined;
@@ -73,7 +80,7 @@ function getOwnCommandFlagValue(config: { commands?: unknown } | undefined, key:
export function isCommandFlagEnabled(
config: { commands?: unknown } | undefined,
key: string,
key: CommandFlagKey,
): boolean {
return getOwnCommandFlagValue(config, key) === true;
}

View File

@@ -1,9 +1,8 @@
import { isPlainObject } from "../utils.js";
import { isBlockedObjectKey } from "./prototype-keys.js";
type PathNode = Record<string, unknown>;
const BLOCKED_KEYS = new Set(["__proto__", "prototype", "constructor"]);
export function parseConfigPath(raw: string): {
ok: boolean;
path?: string[];
@@ -23,7 +22,7 @@ export function parseConfigPath(raw: string): {
error: "Invalid path. Use dot notation (e.g. foo.bar).",
};
}
if (parts.some((part) => BLOCKED_KEYS.has(part))) {
if (parts.some((part) => isBlockedObjectKey(part))) {
return { ok: false, error: "Invalid path segment." };
}
return { ok: true, path: parts };

View File

@@ -15,6 +15,7 @@ import path from "node:path";
import JSON5 from "json5";
import { isPathInside } from "../security/scan-paths.js";
import { isPlainObject } from "../utils.js";
import { isBlockedObjectKey } from "./prototype-keys.js";
export const INCLUDE_KEY = "$include";
export const MAX_INCLUDE_DEPTH = 10;
@@ -54,8 +55,6 @@ export class CircularIncludeError extends ConfigIncludeError {
// Utilities
// ============================================================================
const BLOCKED_MERGE_KEYS = new Set(["__proto__", "prototype", "constructor"]);
/** Deep merge: arrays concatenate, objects merge recursively, primitives: source wins */
export function deepMerge(target: unknown, source: unknown): unknown {
if (Array.isArray(target) && Array.isArray(source)) {
@@ -64,7 +63,7 @@ export function deepMerge(target: unknown, source: unknown): unknown {
if (isPlainObject(target) && isPlainObject(source)) {
const result: Record<string, unknown> = { ...target };
for (const key of Object.keys(source)) {
if (BLOCKED_MERGE_KEYS.has(key)) {
if (isBlockedObjectKey(key)) {
continue;
}
result[key] = key in result ? deepMerge(result[key], source[key]) : source[key];

View File

@@ -0,0 +1,5 @@
const BLOCKED_OBJECT_KEYS = new Set(["__proto__", "prototype", "constructor"]);
export function isBlockedObjectKey(key: string): boolean {
return BLOCKED_OBJECT_KEYS.has(key);
}

View File

@@ -1,11 +1,10 @@
import { isPlainObject } from "../utils.js";
import { parseConfigPath, setConfigValueAtPath, unsetConfigValueAtPath } from "./config-paths.js";
import { isBlockedObjectKey } from "./prototype-keys.js";
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 {
@@ -21,7 +20,7 @@ function sanitizeOverrideValue(value: unknown, seen = new WeakSet<object>()): un
seen.add(value);
const sanitized: OverrideTree = {};
for (const [key, entry] of Object.entries(value)) {
if (entry === undefined || BLOCKED_MERGE_KEYS.has(key)) {
if (entry === undefined || isBlockedObjectKey(key)) {
continue;
}
sanitized[key] = sanitizeOverrideValue(entry, seen);
@@ -36,7 +35,7 @@ function mergeOverrides(base: unknown, override: unknown): unknown {
}
const next: OverrideTree = { ...base };
for (const [key, value] of Object.entries(override)) {
if (value === undefined || BLOCKED_MERGE_KEYS.has(key)) {
if (value === undefined || isBlockedObjectKey(key)) {
continue;
}
next[key] = mergeOverrides((base as OverrideTree)[key], value);