diff --git a/CHANGELOG.md b/CHANGELOG.md index e909131af..c29a34c9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ Docs: https://docs.openclaw.ai - Security/Shell env: validate login-shell executable paths for shell-env fallback (`/etc/shells` + trusted prefixes) and block `SHELL` in dangerous env override policy paths so untrusted shell-path injection falls back safely to `/bin/sh`. Thanks @athuljayaram for reporting. - Security/Config: make parsed chat allowlist checks fail closed when `allowFrom` is empty, restoring expected DM/pairing gating. - Security/Exec: in non-default setups that manually add `sort` to `tools.exec.safeBins`, block `sort --compress-program` so allowlist-mode safe-bin checks cannot bypass approval. Thanks @tdjackey for reporting. +- Security/Exec approvals: when users choose `allow-always` for shell-wrapper commands (for example `/bin/zsh -lc ...`), persist allowlist patterns for the inner executable(s) instead of the wrapper shell binary, preventing accidental broad shell allowlisting in moderate mode. (#23276) Thanks @xrom2863. - Security/macOS app beta: enforce path-only `system.run` allowlist matching (drop basename matches like `echo`), migrate legacy basename entries to last resolved paths when available, and harden shell-chain handling to fail closed on unsafe parse/control syntax (including quoted command substitution/backticks). This is an optional allowlist-mode feature; default installs remain deny-by-default. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/SSRF: expand IPv4 fetch guard blocking to include RFC special-use/non-global ranges (including benchmarking, TEST-NET, multicast, and reserved/broadcast blocks), and centralize range checks into a single CIDR policy table to reduce classifier drift. - Security/Archive: block zip symlink escapes during archive extraction. diff --git a/src/agents/bash-tools.exec-host-gateway.ts b/src/agents/bash-tools.exec-host-gateway.ts index d3cc26c46..7e0698169 100644 --- a/src/agents/bash-tools.exec-host-gateway.ts +++ b/src/agents/bash-tools.exec-host-gateway.ts @@ -11,6 +11,7 @@ import { minSecurity, recordAllowlistUse, requiresExecApproval, + resolveAllowAlwaysPatterns, resolveExecApprovals, } from "../infra/exec-approvals.js"; import { markBackgrounded, tail } from "./bash-process-registry.js"; @@ -153,8 +154,13 @@ export async function processGatewayAllowlist( } else if (decision === "allow-always") { approvedByAsk = true; if (hostSecurity === "allowlist") { - for (const segment of allowlistEval.segments) { - const pattern = segment.resolution?.resolvedPath ?? ""; + const patterns = resolveAllowAlwaysPatterns({ + segments: allowlistEval.segments, + cwd: params.workdir, + env: params.env, + platform: process.platform, + }); + for (const pattern of patterns) { if (pattern) { addAllowlistEntry(approvals.file, params.agentId, pattern); } diff --git a/src/infra/exec-approvals-allowlist.ts b/src/infra/exec-approvals-allowlist.ts index a1d7a2a92..147905522 100644 --- a/src/infra/exec-approvals-allowlist.ts +++ b/src/infra/exec-approvals-allowlist.ts @@ -205,6 +205,148 @@ export type ExecAllowlistAnalysis = { segmentSatisfiedBy: ExecSegmentSatisfiedBy[]; }; +const SHELL_WRAPPER_EXECUTABLES = new Set([ + "ash", + "bash", + "cmd", + "cmd.exe", + "dash", + "fish", + "ksh", + "powershell", + "powershell.exe", + "pwsh", + "pwsh.exe", + "sh", + "zsh", +]); + +function normalizeExecutableName(name: string | undefined): string { + return (name ?? "").trim().toLowerCase(); +} + +function isShellWrapperSegment(segment: ExecCommandSegment): boolean { + const candidates = [ + normalizeExecutableName(segment.resolution?.executableName), + normalizeExecutableName(segment.resolution?.rawExecutable), + ]; + for (const candidate of candidates) { + if (!candidate) { + continue; + } + if (SHELL_WRAPPER_EXECUTABLES.has(candidate)) { + return true; + } + const base = candidate.split(/[\\/]/).pop(); + if (base && SHELL_WRAPPER_EXECUTABLES.has(base)) { + return true; + } + } + return false; +} + +function extractShellInlineCommand(argv: string[]): string | null { + for (let i = 1; i < argv.length; i += 1) { + const token = argv[i]; + if (!token) { + continue; + } + const lower = token.toLowerCase(); + if (lower === "--") { + break; + } + if ( + lower === "-c" || + lower === "--command" || + lower === "-command" || + lower === "/c" || + lower === "/k" + ) { + const next = argv[i + 1]?.trim(); + return next ? next : null; + } + if (/^-[^-]*c[^-]*$/i.test(token)) { + const commandIndex = lower.indexOf("c"); + const inline = token.slice(commandIndex + 1).trim(); + if (inline) { + return inline; + } + const next = argv[i + 1]?.trim(); + return next ? next : null; + } + } + return null; +} + +function collectAllowAlwaysPatterns(params: { + segment: ExecCommandSegment; + cwd?: string; + env?: NodeJS.ProcessEnv; + platform?: string | null; + depth: number; + out: Set; +}) { + const candidatePath = resolveAllowlistCandidatePath(params.segment.resolution, params.cwd); + if (!candidatePath) { + return; + } + if (!isShellWrapperSegment(params.segment)) { + params.out.add(candidatePath); + return; + } + if (params.depth >= 3) { + return; + } + const inlineCommand = extractShellInlineCommand(params.segment.argv); + if (!inlineCommand) { + return; + } + const nested = analyzeShellCommand({ + command: inlineCommand, + cwd: params.cwd, + env: params.env, + platform: params.platform, + }); + if (!nested.ok) { + return; + } + for (const nestedSegment of nested.segments) { + collectAllowAlwaysPatterns({ + segment: nestedSegment, + cwd: params.cwd, + env: params.env, + platform: params.platform, + depth: params.depth + 1, + out: params.out, + }); + } +} + +/** + * Derive persisted allowlist patterns for an "allow always" decision. + * When a command is wrapped in a shell (for example `zsh -lc ""`), + * persist the inner executable(s) rather than the shell binary. + */ +export function resolveAllowAlwaysPatterns(params: { + segments: ExecCommandSegment[]; + cwd?: string; + env?: NodeJS.ProcessEnv; + platform?: string | null; +}): string[] { + const patterns = new Set(); + for (const segment of params.segments) { + collectAllowAlwaysPatterns({ + segment, + cwd: params.cwd, + env: params.env, + platform: params.platform, + depth: 0, + out: patterns, + }); + } + return Array.from(patterns); +} + /** * Evaluates allowlist for shell commands (including &&, ||, ;) and returns analysis metadata. */ diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index 2d34ba468..993c43e2a 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -18,6 +18,7 @@ import { normalizeSafeBins, requiresExecApproval, resolveCommandResolution, + resolveAllowAlwaysPatterns, resolveExecApprovals, resolveExecApprovalsFromFile, resolveExecApprovalsPath, @@ -1214,3 +1215,122 @@ describe("normalizeExecApprovals handles string allowlist entries (#9790)", () = } }); }); + +describe("resolveAllowAlwaysPatterns", () => { + function makeExecutable(dir: string, name: string): string { + const fileName = process.platform === "win32" ? `${name}.exe` : name; + const exe = path.join(dir, fileName); + fs.writeFileSync(exe, ""); + fs.chmodSync(exe, 0o755); + return exe; + } + + it("returns direct executable paths for non-shell segments", () => { + const exe = path.join("/tmp", "openclaw-tool"); + const patterns = resolveAllowAlwaysPatterns({ + segments: [ + { + raw: exe, + argv: [exe], + resolution: { rawExecutable: exe, resolvedPath: exe, executableName: "openclaw-tool" }, + }, + ], + }); + expect(patterns).toEqual([exe]); + }); + + it("unwraps shell wrappers and persists the inner executable instead", () => { + if (process.platform === "win32") { + return; + } + const dir = makeTempDir(); + const whoami = makeExecutable(dir, "whoami"); + const patterns = resolveAllowAlwaysPatterns({ + segments: [ + { + raw: "/bin/zsh -lc 'whoami'", + argv: ["/bin/zsh", "-lc", "whoami"], + resolution: { + rawExecutable: "/bin/zsh", + resolvedPath: "/bin/zsh", + executableName: "zsh", + }, + }, + ], + cwd: dir, + env: makePathEnv(dir), + platform: process.platform, + }); + expect(patterns).toEqual([whoami]); + expect(patterns).not.toContain("/bin/zsh"); + }); + + it("extracts all inner binaries from shell chains and deduplicates", () => { + if (process.platform === "win32") { + return; + } + const dir = makeTempDir(); + const whoami = makeExecutable(dir, "whoami"); + const ls = makeExecutable(dir, "ls"); + const patterns = resolveAllowAlwaysPatterns({ + segments: [ + { + raw: "/bin/zsh -lc 'whoami && ls && whoami'", + argv: ["/bin/zsh", "-lc", "whoami && ls && whoami"], + resolution: { + rawExecutable: "/bin/zsh", + resolvedPath: "/bin/zsh", + executableName: "zsh", + }, + }, + ], + cwd: dir, + env: makePathEnv(dir), + platform: process.platform, + }); + expect(new Set(patterns)).toEqual(new Set([whoami, ls])); + }); + + it("does not persist broad shell binaries when no inner command can be derived", () => { + const patterns = resolveAllowAlwaysPatterns({ + segments: [ + { + raw: "/bin/zsh -s", + argv: ["/bin/zsh", "-s"], + resolution: { + rawExecutable: "/bin/zsh", + resolvedPath: "/bin/zsh", + executableName: "zsh", + }, + }, + ], + platform: process.platform, + }); + expect(patterns).toEqual([]); + }); + + it("detects shell wrappers even when unresolved executableName is a full path", () => { + if (process.platform === "win32") { + return; + } + const dir = makeTempDir(); + const whoami = makeExecutable(dir, "whoami"); + const patterns = resolveAllowAlwaysPatterns({ + segments: [ + { + raw: "/usr/local/bin/zsh -lc whoami", + argv: ["/usr/local/bin/zsh", "-lc", "whoami"], + resolution: { + rawExecutable: "/usr/local/bin/zsh", + resolvedPath: undefined, + executableName: "/usr/local/bin/zsh", + }, + }, + ], + cwd: dir, + env: makePathEnv(dir), + platform: process.platform, + }); + expect(patterns).toEqual([whoami]); + }); +}); diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index fc1a2fee3..9a190b58c 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -9,6 +9,7 @@ import { evaluateShellAllowlist, recordAllowlistUse, requiresExecApproval, + resolveAllowAlwaysPatterns, resolveExecApprovals, resolveSafeBins, type ExecAllowlistEntry, @@ -314,8 +315,13 @@ export async function handleSystemRunInvoke(opts: { } if (approvalDecision === "allow-always" && security === "allowlist") { if (analysisOk) { - for (const segment of segments) { - const pattern = segment.resolution?.resolvedPath ?? ""; + const patterns = resolveAllowAlwaysPatterns({ + segments, + cwd: opts.params.cwd ?? undefined, + env, + platform: process.platform, + }); + for (const pattern of patterns) { if (pattern) { addAllowlistEntry(approvals.file, agentId, pattern); }