mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-08 06:54:24 +00:00
fix(security): block safeBins shell expansion
This commit is contained in:
@@ -16,6 +16,7 @@ Docs: https://docs.openclaw.ai
|
||||
- BlueBubbles: include sender identity in group chat envelopes and pass clean message text to the agent prompt, aligning with iMessage/Signal formatting. (#16210) Thanks @zerone0x.
|
||||
- WhatsApp: honor per-account `dmPolicy` overrides (account-level settings now take precedence over channel defaults for inbound DMs). (#10082) Thanks @mcaxtr.
|
||||
- Security/Node Host: enforce `system.run` rawCommand/argv consistency to prevent allowlist/approval bypass. Thanks @christos-eth.
|
||||
- Security/Exec approvals: prevent safeBins allowlist bypass via shell expansion (host exec allowlist mode only; not enabled by default). Thanks @christos-eth.
|
||||
- Security/Gateway: block `system.execApprovals.*` via `node.invoke` (use `exec.approvals.node.*` instead). Thanks @christos-eth.
|
||||
- CLI: fix lazy core command registration so top-level maintenance commands (`doctor`, `dashboard`, `reset`, `uninstall`) resolve correctly instead of exposing a non-functional `maintenance` placeholder command.
|
||||
- Security/Agents: scope CLI process cleanup to owned child PIDs to avoid killing unrelated processes on shared hosts. Thanks @aether-ai-agent.
|
||||
|
||||
@@ -124,6 +124,8 @@ are treated as allowlisted on nodes (macOS node or headless node host). This use
|
||||
`tools.exec.safeBins` defines a small list of **stdin-only** binaries (for example `jq`)
|
||||
that can run in allowlist mode **without** explicit allowlist entries. Safe bins reject
|
||||
positional file args and path-like tokens, so they can only operate on the incoming stream.
|
||||
Safe bins also force argv tokens to be treated as **literal text** at execution time (no globbing
|
||||
and no `$VARS` expansion), so patterns like `*` or `$HOME/...` cannot be used to smuggle file reads.
|
||||
Shell chaining and redirections are not auto-allowed in allowlist mode.
|
||||
|
||||
Shell chaining (`&&`, `||`, `;`) is allowed when every top-level segment satisfies the allowlist
|
||||
|
||||
@@ -120,7 +120,8 @@ running after `tools.exec.approvalRunningNoticeMs`, a single `Exec running` noti
|
||||
Allowlist enforcement matches **resolved binary paths only** (no basename matches). When
|
||||
`security=allowlist`, shell commands are auto-allowed only if every pipeline segment is
|
||||
allowlisted or a safe bin. Chaining (`;`, `&&`, `||`) and redirections are rejected in
|
||||
allowlist mode.
|
||||
allowlist mode unless every top-level segment satisfies the allowlist (including safe bins).
|
||||
Redirections remain unsupported.
|
||||
|
||||
## Examples
|
||||
|
||||
|
||||
@@ -338,6 +338,9 @@ export function emitExecSystemEvent(
|
||||
|
||||
export async function runExecProcess(opts: {
|
||||
command: string;
|
||||
// Execute this instead of `command` (which is kept for display/session/logging).
|
||||
// Used to sanitize safeBins execution while preserving the original user input.
|
||||
execCommand?: string;
|
||||
workdir: string;
|
||||
env: Record<string, string>;
|
||||
sandbox?: BashSandboxConfig;
|
||||
@@ -357,6 +360,7 @@ export async function runExecProcess(opts: {
|
||||
let child: ChildProcessWithoutNullStreams | null = null;
|
||||
let pty: PtyHandle | null = null;
|
||||
let stdin: SessionStdin | undefined;
|
||||
const execCommand = opts.execCommand ?? opts.command;
|
||||
|
||||
if (opts.sandbox) {
|
||||
const { child: spawned } = await spawnWithFallback({
|
||||
@@ -364,7 +368,7 @@ export async function runExecProcess(opts: {
|
||||
"docker",
|
||||
...buildDockerExecArgs({
|
||||
containerName: opts.sandbox.containerName,
|
||||
command: opts.command,
|
||||
command: execCommand,
|
||||
workdir: opts.containerWorkdir ?? opts.sandbox.containerWorkdir,
|
||||
env: opts.env,
|
||||
tty: opts.usePty,
|
||||
@@ -403,7 +407,7 @@ export async function runExecProcess(opts: {
|
||||
if (!spawnPty) {
|
||||
throw new Error("PTY support is unavailable (node-pty spawn not found).");
|
||||
}
|
||||
pty = spawnPty(shell, [...shellArgs, opts.command], {
|
||||
pty = spawnPty(shell, [...shellArgs, execCommand], {
|
||||
cwd: opts.workdir,
|
||||
env: opts.env,
|
||||
name: process.env.TERM ?? "xterm-256color",
|
||||
@@ -435,7 +439,7 @@ export async function runExecProcess(opts: {
|
||||
logWarn(`exec: PTY spawn failed (${errText}); retrying without PTY for "${opts.command}".`);
|
||||
opts.warnings.push(warning);
|
||||
const { child: spawned } = await spawnWithFallback({
|
||||
argv: [shell, ...shellArgs, opts.command],
|
||||
argv: [shell, ...shellArgs, execCommand],
|
||||
options: {
|
||||
cwd: opts.workdir,
|
||||
env: opts.env,
|
||||
@@ -462,7 +466,7 @@ export async function runExecProcess(opts: {
|
||||
} else {
|
||||
const { shell, args: shellArgs } = getShellConfig();
|
||||
const { child: spawned } = await spawnWithFallback({
|
||||
argv: [shell, ...shellArgs, opts.command],
|
||||
argv: [shell, ...shellArgs, execCommand],
|
||||
options: {
|
||||
cwd: opts.workdir,
|
||||
env: opts.env,
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
recordAllowlistUse,
|
||||
resolveExecApprovals,
|
||||
resolveExecApprovalsFromFile,
|
||||
buildSafeShellCommand,
|
||||
} from "../infra/exec-approvals.js";
|
||||
import { buildNodeShellCommand } from "../infra/node-shell.js";
|
||||
import {
|
||||
@@ -170,6 +171,7 @@ export function createExecTool(
|
||||
const maxOutput = DEFAULT_MAX_OUTPUT;
|
||||
const pendingMaxOutput = DEFAULT_PENDING_MAX_OUTPUT;
|
||||
const warnings: string[] = [];
|
||||
let execCommandOverride: string | undefined;
|
||||
const backgroundRequested = params.background === true;
|
||||
const yieldRequested = typeof params.yieldMs === "number";
|
||||
if (!allowBackground && (backgroundRequested || yieldRequested)) {
|
||||
@@ -804,6 +806,25 @@ export function createExecTool(
|
||||
throw new Error("exec denied: allowlist miss");
|
||||
}
|
||||
|
||||
// If allowlist is satisfied only via safeBins (no explicit allowlist match),
|
||||
// run a sanitized `shell -c` command that disables glob/var expansion by
|
||||
// forcing every argv token to be literal via single-quoting.
|
||||
if (
|
||||
hostSecurity === "allowlist" &&
|
||||
analysisOk &&
|
||||
allowlistSatisfied &&
|
||||
allowlistMatches.length === 0
|
||||
) {
|
||||
const safe = buildSafeShellCommand({
|
||||
command: params.command,
|
||||
platform: process.platform,
|
||||
});
|
||||
if (!safe.ok || !safe.command) {
|
||||
throw new Error(`exec denied: safeBins sanitize failed (${safe.reason ?? "unknown"})`);
|
||||
}
|
||||
execCommandOverride = safe.command;
|
||||
}
|
||||
|
||||
if (allowlistMatches.length > 0) {
|
||||
const seen = new Set<string>();
|
||||
for (const match of allowlistMatches) {
|
||||
@@ -828,6 +849,7 @@ export function createExecTool(
|
||||
const usePty = params.pty === true && !sandbox;
|
||||
const run = await runExecProcess({
|
||||
command: params.command,
|
||||
execCommand: execCommandOverride,
|
||||
workdir,
|
||||
env,
|
||||
sandbox,
|
||||
|
||||
@@ -130,4 +130,46 @@ describe("createOpenClawCodingTools safeBins", () => {
|
||||
expect(result.details.status).toBe("completed");
|
||||
expect(text).toContain(marker);
|
||||
});
|
||||
|
||||
it("does not allow env var expansion to smuggle file args via safeBins", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
|
||||
const { createOpenClawCodingTools } = await import("./pi-tools.js");
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-safe-bins-expand-"));
|
||||
|
||||
const secret = `TOP_SECRET_${Date.now()}`;
|
||||
fs.writeFileSync(path.join(tmpDir, "secret.txt"), `${secret}\n`, "utf8");
|
||||
|
||||
const cfg: OpenClawConfig = {
|
||||
tools: {
|
||||
exec: {
|
||||
host: "gateway",
|
||||
security: "allowlist",
|
||||
ask: "off",
|
||||
safeBins: ["head", "wc"],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const tools = createOpenClawCodingTools({
|
||||
config: cfg,
|
||||
sessionKey: "agent:main:main",
|
||||
workspaceDir: tmpDir,
|
||||
agentDir: path.join(tmpDir, "agent"),
|
||||
});
|
||||
const execTool = tools.find((tool) => tool.name === "exec");
|
||||
expect(execTool).toBeDefined();
|
||||
|
||||
const result = await execTool!.execute("call1", {
|
||||
command: "head $FOO ; wc -l",
|
||||
workdir: tmpDir,
|
||||
env: { FOO: "secret.txt" },
|
||||
});
|
||||
const text = result.content.find((content) => content.type === "text")?.text ?? "";
|
||||
|
||||
expect(result.details.status).toBe("completed");
|
||||
expect(text).not.toContain(secret);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -248,6 +248,13 @@ export type ExecCommandAnalysis = {
|
||||
chains?: ExecCommandSegment[][]; // Segments grouped by chain operator (&&, ||, ;)
|
||||
};
|
||||
|
||||
export type ShellChainOperator = "&&" | "||" | ";";
|
||||
|
||||
export type ShellChainPart = {
|
||||
part: string;
|
||||
opToNext: ShellChainOperator | null;
|
||||
};
|
||||
|
||||
const DISALLOWED_PIPELINE_TOKENS = new Set([">", "<", "`", "\n", "\r", "(", ")"]);
|
||||
const DOUBLE_QUOTE_ESCAPES = new Set(["\\", '"', "$", "`", "\n", "\r"]);
|
||||
const WINDOWS_UNSUPPORTED_TOKENS = new Set([
|
||||
@@ -603,6 +610,168 @@ function parseSegmentsFromParts(
|
||||
return segments;
|
||||
}
|
||||
|
||||
/**
|
||||
* Splits a command string by chain operators (&&, ||, ;) while preserving the operators.
|
||||
* Returns null when no chain is present or when the chain is malformed.
|
||||
*/
|
||||
export function splitCommandChainWithOperators(command: string): ShellChainPart[] | null {
|
||||
const parts: ShellChainPart[] = [];
|
||||
let buf = "";
|
||||
let inSingle = false;
|
||||
let inDouble = false;
|
||||
let escaped = false;
|
||||
let foundChain = false;
|
||||
let invalidChain = false;
|
||||
|
||||
const pushPart = (opToNext: ShellChainOperator | null) => {
|
||||
const trimmed = buf.trim();
|
||||
buf = "";
|
||||
if (!trimmed) {
|
||||
return false;
|
||||
}
|
||||
parts.push({ part: trimmed, opToNext });
|
||||
return true;
|
||||
};
|
||||
|
||||
for (let i = 0; i < command.length; i += 1) {
|
||||
const ch = command[i];
|
||||
const next = command[i + 1];
|
||||
if (escaped) {
|
||||
buf += ch;
|
||||
escaped = false;
|
||||
continue;
|
||||
}
|
||||
if (!inSingle && !inDouble && ch === "\\") {
|
||||
escaped = true;
|
||||
buf += ch;
|
||||
continue;
|
||||
}
|
||||
if (inSingle) {
|
||||
if (ch === "'") {
|
||||
inSingle = false;
|
||||
}
|
||||
buf += ch;
|
||||
continue;
|
||||
}
|
||||
if (inDouble) {
|
||||
if (ch === "\\" && isDoubleQuoteEscape(next)) {
|
||||
buf += ch;
|
||||
buf += next;
|
||||
i += 1;
|
||||
continue;
|
||||
}
|
||||
if (ch === '"') {
|
||||
inDouble = false;
|
||||
}
|
||||
buf += ch;
|
||||
continue;
|
||||
}
|
||||
if (ch === "'") {
|
||||
inSingle = true;
|
||||
buf += ch;
|
||||
continue;
|
||||
}
|
||||
if (ch === '"') {
|
||||
inDouble = true;
|
||||
buf += ch;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (ch === "&" && next === "&") {
|
||||
if (!pushPart("&&")) {
|
||||
invalidChain = true;
|
||||
}
|
||||
i += 1;
|
||||
foundChain = true;
|
||||
continue;
|
||||
}
|
||||
if (ch === "|" && next === "|") {
|
||||
if (!pushPart("||")) {
|
||||
invalidChain = true;
|
||||
}
|
||||
i += 1;
|
||||
foundChain = true;
|
||||
continue;
|
||||
}
|
||||
if (ch === ";") {
|
||||
if (!pushPart(";")) {
|
||||
invalidChain = true;
|
||||
}
|
||||
foundChain = true;
|
||||
continue;
|
||||
}
|
||||
|
||||
buf += ch;
|
||||
}
|
||||
|
||||
if (!foundChain) {
|
||||
return null;
|
||||
}
|
||||
const trimmed = buf.trim();
|
||||
if (!trimmed) {
|
||||
return null;
|
||||
}
|
||||
parts.push({ part: trimmed, opToNext: null });
|
||||
if (invalidChain || parts.length === 0) {
|
||||
return null;
|
||||
}
|
||||
return parts;
|
||||
}
|
||||
|
||||
function shellEscapeSingleArg(value: string): string {
|
||||
// Shell-safe across sh/bash/zsh: single-quote everything, escape embedded single quotes.
|
||||
// Example: foo'bar -> 'foo'"'"'bar'
|
||||
const singleQuoteEscape = `'"'"'`;
|
||||
return `'${value.replace(/'/g, singleQuoteEscape)}'`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds a shell command string that preserves pipes/chaining, but forces *arguments* to be
|
||||
* literal (no globbing, no env-var expansion) by single-quoting every argv token.
|
||||
*
|
||||
* Used to make "safe bins" actually stdin-only even though execution happens via `shell -c`.
|
||||
*/
|
||||
export function buildSafeShellCommand(params: { command: string; platform?: string | null }): {
|
||||
ok: boolean;
|
||||
command?: string;
|
||||
reason?: string;
|
||||
} {
|
||||
const platform = params.platform ?? null;
|
||||
if (isWindowsPlatform(platform)) {
|
||||
return { ok: false, reason: "unsupported platform" };
|
||||
}
|
||||
const source = params.command.trim();
|
||||
if (!source) {
|
||||
return { ok: false, reason: "empty command" };
|
||||
}
|
||||
|
||||
const chain = splitCommandChainWithOperators(source);
|
||||
const chainParts = chain ?? [{ part: source, opToNext: null }];
|
||||
let out = "";
|
||||
|
||||
for (let i = 0; i < chainParts.length; i += 1) {
|
||||
const part = chainParts[i];
|
||||
const pipelineSplit = splitShellPipeline(part.part);
|
||||
if (!pipelineSplit.ok) {
|
||||
return { ok: false, reason: pipelineSplit.reason ?? "unable to parse pipeline" };
|
||||
}
|
||||
const renderedSegments: string[] = [];
|
||||
for (const segmentRaw of pipelineSplit.segments) {
|
||||
const argv = splitShellArgs(segmentRaw);
|
||||
if (!argv || argv.length === 0) {
|
||||
return { ok: false, reason: "unable to parse shell segment" };
|
||||
}
|
||||
renderedSegments.push(argv.map((token) => shellEscapeSingleArg(token)).join(" "));
|
||||
}
|
||||
out += renderedSegments.join(" | ");
|
||||
if (part.opToNext) {
|
||||
out += ` ${part.opToNext} `;
|
||||
}
|
||||
}
|
||||
|
||||
return { ok: true, command: out };
|
||||
}
|
||||
|
||||
/**
|
||||
* Splits a command string by chain operators (&&, ||, ;) while respecting quotes.
|
||||
* Returns null when no chain is present or when the chain is malformed.
|
||||
|
||||
@@ -5,6 +5,7 @@ import { describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
analyzeArgvCommand,
|
||||
analyzeShellCommand,
|
||||
buildSafeShellCommand,
|
||||
evaluateExecAllowlist,
|
||||
evaluateShellAllowlist,
|
||||
isSafeBinUsage,
|
||||
@@ -78,6 +79,25 @@ describe("exec approvals allowlist matching", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("exec approvals safe shell command builder", () => {
|
||||
it("single-quotes argv tokens while preserving pipes/chaining", () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const res = buildSafeShellCommand({
|
||||
command: 'head $FOO | grep * && echo "a\'b" ; wc -l',
|
||||
platform: process.platform,
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
expect(res.command).toContain("'$FOO'");
|
||||
expect(res.command).toContain("'*'");
|
||||
expect(res.command).toContain("&&");
|
||||
expect(res.command).toContain(";");
|
||||
expect(res.command).toContain("|");
|
||||
expect(res.command).toContain("'a'\"'\"'b'");
|
||||
});
|
||||
});
|
||||
|
||||
describe("exec approvals command resolution", () => {
|
||||
it("resolves PATH executables", () => {
|
||||
const dir = makeTempDir();
|
||||
|
||||
Reference in New Issue
Block a user