diff --git a/CHANGELOG.md b/CHANGELOG.md index 2dd40fbd5..b5271e2ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Docs: https://docs.clawd.bot - CLI: default exec approvals to the local host, add gateway/node targeting flags, and show target details in allowlist output. - CLI: exec approvals mutations render tables instead of raw JSON. - Exec approvals: support wildcard agent allowlists (`*`) across all agents. +- Exec approvals: allowlist matches resolved binary paths only, add safe stdin-only bins, and tighten allowlist shell parsing. - Nodes: expose node PATH in status/describe and bootstrap PATH for node-host execution. - CLI: flatten node service commands under `clawdbot node` and remove `service node` docs. - CLI: move gateway service commands under `clawdbot gateway` and add `gateway probe` for reachability. diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index c6dccaab5..b96273664 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -87,6 +87,7 @@ If a prompt is required but no UI is reachable, fallback decides: Allowlists are **per agent**. If multiple agents exist, switch which agent you’re editing in the macOS app. Patterns are **case-insensitive glob matches**. +Patterns should resolve to **binary paths** (basename-only entries are ignored). Examples: - `~/Projects/**/bin/bird` @@ -104,6 +105,15 @@ When **Auto-allow skill CLIs** is enabled, executables referenced by known skill are treated as allowlisted on nodes (macOS node or headless node host). This uses the Bridge RPC to ask the gateway for the skill bin list. Disable this if you want strict manual allowlists. +## Safe bins (stdin-only) + +`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. +Shell chaining and redirections are not auto-allowed in allowlist mode. + +Default safe bins: `jq`, `grep`, `cut`, `sort`, `uniq`, `head`, `tail`, `tr`, `wc`. + ## Control UI editing Use the **Control UI → Nodes → Exec approvals** card to edit defaults, per‑agent diff --git a/docs/tools/exec.md b/docs/tools/exec.md index 759f0fe30..3a4f8297f 100644 --- a/docs/tools/exec.md +++ b/docs/tools/exec.md @@ -43,6 +43,7 @@ Notes: - `tools.exec.ask` (default: `on-miss`) - `tools.exec.node` (default: unset) - `tools.exec.pathPrepend`: list of directories to prepend to `PATH` for exec runs. +- `tools.exec.safeBins`: stdin-only safe binaries that can run without explicit allowlist entries. Example: ```json5 @@ -64,7 +65,8 @@ Example: - `host=sandbox`: runs `sh -lc` (login shell) inside the container, so `/etc/profile` may reset `PATH`. Clawdbot prepends `env.PATH` after profile sourcing; `tools.exec.pathPrepend` applies here too. - `host=node`: only env overrides you pass are sent to the node. `tools.exec.pathPrepend` only applies - if the exec call already sets `env.PATH`. + if the exec call already sets `env.PATH`. Node PATH overrides are accepted only when they prepend + the node host PATH (no replacement). Per-agent node binding (use the agent list index in config): @@ -90,6 +92,13 @@ Example: Sandboxed agents can require per-request approval before `exec` runs on the gateway or node host. See [Exec approvals](/tools/exec-approvals) for the policy, allowlist, and UI flow. +## Allowlist + safe bins + +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. + ## Examples Foreground: diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index 7f5091d8c..b13003d8b 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -8,12 +8,15 @@ import { type ExecAsk, type ExecHost, type ExecSecurity, + type ExecAllowlistEntry, addAllowlistEntry, + analyzeShellCommand, + isSafeBinUsage, matchAllowlist, maxAsk, minSecurity, + resolveSafeBins, recordAllowlistUse, - resolveCommandResolution, resolveExecApprovals, } from "../infra/exec-approvals.js"; import { requestHeartbeatNow } from "../infra/heartbeat-wake.js"; @@ -94,6 +97,7 @@ export type ExecToolDefaults = { ask?: ExecAsk; node?: string; pathPrepend?: string[]; + safeBins?: string[]; agentId?: string; backgroundMs?: number; timeoutSec?: number; @@ -298,6 +302,7 @@ export function createExecTool( ? defaults.timeoutSec : 1800; const defaultPathPrepend = normalizePathPrepend(defaults?.pathPrepend); + const safeBins = resolveSafeBins(defaults?.safeBins); const notifyOnExit = defaults?.notifyOnExit !== false; const notifySessionKey = defaults?.sessionKey?.trim() || undefined; @@ -593,13 +598,27 @@ export function createExecTool( if (hostSecurity === "deny") { throw new Error("exec denied: host=gateway security=deny"); } - - const resolution = resolveCommandResolution(params.command, workdir, env); - const allowlistMatch = - hostSecurity === "allowlist" ? matchAllowlist(approvals.allowlist, resolution) : null; + const analysis = analyzeShellCommand({ command: params.command, cwd: workdir, env }); + const allowlistMatches: ExecAllowlistEntry[] = []; + let allowlistSatisfied = false; + if (hostSecurity === "allowlist" && analysis.ok && analysis.segments.length > 0) { + allowlistSatisfied = analysis.segments.every((segment) => { + const match = matchAllowlist(approvals.allowlist, segment.resolution); + if (match) allowlistMatches.push(match); + const safe = isSafeBinUsage({ + argv: segment.argv, + resolution: segment.resolution, + safeBins, + cwd: workdir, + }); + return Boolean(match || safe); + }); + } const requiresAsk = hostAsk === "always" || - (hostAsk === "on-miss" && hostSecurity === "allowlist" && !allowlistMatch); + (hostAsk === "on-miss" && + hostSecurity === "allowlist" && + (!analysis.ok || !allowlistSatisfied)); let approvedByAsk = false; if (requiresAsk) { @@ -613,7 +632,7 @@ export function createExecTool( security: hostSecurity, ask: hostAsk, agentId: defaults?.agentId, - resolvedPath: resolution?.resolvedPath ?? null, + resolvedPath: analysis.segments[0]?.resolution?.resolvedPath ?? null, sessionKey: defaults?.sessionKey ?? null, timeoutMs: 120_000, }, @@ -630,7 +649,7 @@ export function createExecTool( if (askFallback === "full") { approvedByAsk = true; } else if (askFallback === "allowlist") { - if (!allowlistMatch) { + if (!analysis.ok || !allowlistSatisfied) { throw new Error("exec denied: approval required (approval UI not available)"); } approvedByAsk = true; @@ -644,30 +663,37 @@ export function createExecTool( if (decision === "allow-always") { approvedByAsk = true; if (hostSecurity === "allowlist") { - const pattern = - resolution?.resolvedPath ?? - resolution?.rawExecutable ?? - params.command.split(/\s+/).shift() ?? - ""; - if (pattern) { - addAllowlistEntry(approvals.file, defaults?.agentId, pattern); + for (const segment of analysis.segments) { + const pattern = segment.resolution?.resolvedPath ?? ""; + if (pattern) { + addAllowlistEntry(approvals.file, defaults?.agentId, pattern); + } } } } } - if (hostSecurity === "allowlist" && !allowlistMatch && !approvedByAsk) { + if ( + hostSecurity === "allowlist" && + (!analysis.ok || !allowlistSatisfied) && + !approvedByAsk + ) { throw new Error("exec denied: allowlist miss"); } - if (allowlistMatch) { - recordAllowlistUse( - approvals.file, - defaults?.agentId, - allowlistMatch, - params.command, - resolution?.resolvedPath, - ); + if (allowlistMatches.length > 0) { + const seen = new Set(); + for (const match of allowlistMatches) { + if (seen.has(match.pattern)) continue; + seen.add(match.pattern); + recordAllowlistUse( + approvals.file, + defaults?.agentId, + match, + params.command, + analysis.segments[0]?.resolution?.resolvedPath, + ); + } } } diff --git a/src/cli/nodes-cli.coverage.test.ts b/src/cli/nodes-cli.coverage.test.ts index 7e6e482dd..13fc36caf 100644 --- a/src/cli/nodes-cli.coverage.test.ts +++ b/src/cli/nodes-cli.coverage.test.ts @@ -27,7 +27,7 @@ const callGateway = vi.fn(async (opts: { method?: string }) => { }, }; } - if (opts.method === "exec.approvals.get") { + if (opts.method === "exec.approvals.node.get") { return { path: "/tmp/exec-approvals.json", exists: true, diff --git a/src/cli/nodes-cli/register.invoke.ts b/src/cli/nodes-cli/register.invoke.ts index 02c013cee..4cad89315 100644 --- a/src/cli/nodes-cli/register.invoke.ts +++ b/src/cli/nodes-cli/register.invoke.ts @@ -38,6 +38,7 @@ type ExecDefaults = { ask?: ExecAsk; node?: string; pathPrepend?: string[]; + safeBins?: string[]; }; function normalizeExecSecurity(value?: string | null): ExecSecurity | null { @@ -95,6 +96,7 @@ function resolveExecDefaults( ask: globalExec.ask, node: globalExec.node, pathPrepend: globalExec.pathPrepend, + safeBins: globalExec.safeBins, } : undefined; } @@ -104,6 +106,7 @@ function resolveExecDefaults( ask: agentExec?.ask ?? globalExec?.ask, node: agentExec?.node ?? globalExec?.node, pathPrepend: agentExec?.pathPrepend ?? globalExec?.pathPrepend, + safeBins: agentExec?.safeBins ?? globalExec?.safeBins, }; } @@ -230,7 +233,9 @@ export function registerNodesInvokeCommands(nodes: Command) { const security = minSecurity(configuredSecurity, requestedSecurity ?? configuredSecurity); const ask = maxAsk(configuredAsk, requestedAsk ?? configuredAsk); - const approvalsSnapshot = (await callGatewayCli("exec.approvals.get", opts, {})) as { + const approvalsSnapshot = (await callGatewayCli("exec.approvals.node.get", opts, { + nodeId, + })) as { file?: unknown; } | null; const approvalsFile = diff --git a/src/config/schema.ts b/src/config/schema.ts index 1ba527439..bf25f0adb 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -166,6 +166,7 @@ const FIELD_LABELS: Record = { "tools.exec.ask": "Exec Ask", "tools.exec.node": "Exec Node Binding", "tools.exec.pathPrepend": "Exec PATH Prepend", + "tools.exec.safeBins": "Exec Safe Bins", "tools.message.allowCrossContextSend": "Allow Cross-Context Messaging", "tools.message.crossContext.allowWithinProvider": "Allow Cross-Context (Same Provider)", "tools.message.crossContext.allowAcrossProviders": "Allow Cross-Context (Across Providers)", @@ -367,6 +368,8 @@ const FIELD_HELP: Record = { "tools.exec.notifyOnExit": "When true (default), backgrounded exec sessions enqueue a system event and request a heartbeat on exit.", "tools.exec.pathPrepend": "Directories to prepend to PATH for exec runs (gateway/sandbox).", + "tools.exec.safeBins": + "Allow stdin-only safe binaries to run without explicit allowlist entries.", "tools.message.allowCrossContextSend": "Legacy override: allow cross-context sends across all providers.", "tools.message.crossContext.allowWithinProvider": diff --git a/src/config/types.tools.ts b/src/config/types.tools.ts index b68d9f5dd..7ce292935 100644 --- a/src/config/types.tools.ts +++ b/src/config/types.tools.ts @@ -131,6 +131,8 @@ export type ExecToolConfig = { node?: string; /** Directories to prepend to PATH when running exec (gateway/sandbox). */ pathPrepend?: string[]; + /** Safe stdin-only binaries that can run without allowlist entries. */ + safeBins?: string[]; /** Default time (ms) before an exec command auto-backgrounds. */ backgroundMs?: number; /** Default timeout (seconds) before auto-killing exec commands. */ diff --git a/src/config/zod-schema.agent-runtime.ts b/src/config/zod-schema.agent-runtime.ts index 293475506..ba6eb1db9 100644 --- a/src/config/zod-schema.agent-runtime.ts +++ b/src/config/zod-schema.agent-runtime.ts @@ -257,6 +257,7 @@ export const AgentToolsSchema = z ask: z.enum(["off", "on-miss", "always"]).optional(), node: z.string().optional(), pathPrepend: z.array(z.string()).optional(), + safeBins: z.array(z.string()).optional(), backgroundMs: z.number().int().positive().optional(), timeoutSec: z.number().int().positive().optional(), cleanupMs: z.number().int().positive().optional(), @@ -485,6 +486,7 @@ export const ToolsSchema = z ask: z.enum(["off", "on-miss", "always"]).optional(), node: z.string().optional(), pathPrepend: z.array(z.string()).optional(), + safeBins: z.array(z.string()).optional(), backgroundMs: z.number().int().positive().optional(), timeoutSec: z.number().int().positive().optional(), cleanupMs: z.number().int().positive().optional(), diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index bfd29ab9e..e0ff6a33b 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -5,9 +5,13 @@ import path from "node:path"; import { describe, expect, it, vi } from "vitest"; import { + analyzeArgvCommand, + analyzeShellCommand, + isSafeBinUsage, matchAllowlist, maxAsk, minSecurity, + normalizeSafeBins, resolveCommandResolution, resolveExecApprovals, type ExecAllowlistEntry, @@ -18,7 +22,7 @@ function makeTempDir() { } describe("exec approvals allowlist matching", () => { - it("matches by executable name (case-insensitive)", () => { + it("ignores basename-only patterns", () => { const resolution = { rawExecutable: "rg", resolvedPath: "/opt/homebrew/bin/rg", @@ -26,7 +30,7 @@ describe("exec approvals allowlist matching", () => { }; const entries: ExecAllowlistEntry[] = [{ pattern: "RG" }]; const match = matchAllowlist(entries, resolution); - expect(match?.pattern).toBe("RG"); + expect(match).toBeNull(); }); it("matches by resolved path with **", () => { @@ -51,7 +55,7 @@ describe("exec approvals allowlist matching", () => { expect(match).toBeNull(); }); - it("falls back to raw executable when no resolved path", () => { + it("requires a resolved path", () => { const resolution = { rawExecutable: "bin/rg", resolvedPath: undefined, @@ -59,7 +63,7 @@ describe("exec approvals allowlist matching", () => { }; const entries: ExecAllowlistEntry[] = [{ pattern: "bin/rg" }]; const match = matchAllowlist(entries, resolution); - expect(match?.pattern).toBe("bin/rg"); + expect(match).toBeNull(); }); }); @@ -70,6 +74,7 @@ describe("exec approvals command resolution", () => { fs.mkdirSync(binDir, { recursive: true }); const exe = path.join(binDir, "rg"); fs.writeFileSync(exe, ""); + fs.chmodSync(exe, 0o755); const res = resolveCommandResolution("rg -n foo", undefined, { PATH: binDir }); expect(res?.resolvedPath).toBe(exe); expect(res?.executableName).toBe("rg"); @@ -81,6 +86,7 @@ describe("exec approvals command resolution", () => { const script = path.join(cwd, "scripts", "run.sh"); fs.mkdirSync(path.dirname(script), { recursive: true }); fs.writeFileSync(script, ""); + fs.chmodSync(script, 0o755); const res = resolveCommandResolution("./scripts/run.sh --flag", cwd, undefined); expect(res?.resolvedPath).toBe(script); }); @@ -91,11 +97,81 @@ describe("exec approvals command resolution", () => { const script = path.join(cwd, "bin", "tool"); fs.mkdirSync(path.dirname(script), { recursive: true }); fs.writeFileSync(script, ""); + fs.chmodSync(script, 0o755); const res = resolveCommandResolution('"./bin/tool" --version', cwd, undefined); expect(res?.resolvedPath).toBe(script); }); }); +describe("exec approvals shell parsing", () => { + it("parses simple pipelines", () => { + const res = analyzeShellCommand({ command: "echo ok | jq .foo" }); + expect(res.ok).toBe(true); + expect(res.segments.map((seg) => seg.argv[0])).toEqual(["echo", "jq"]); + }); + + it("rejects chained commands", () => { + const res = analyzeShellCommand({ command: "ls && rm -rf /" }); + expect(res.ok).toBe(false); + }); + + it("parses argv commands", () => { + const res = analyzeArgvCommand({ argv: ["/bin/echo", "ok"] }); + expect(res.ok).toBe(true); + expect(res.segments[0]?.argv).toEqual(["/bin/echo", "ok"]); + }); +}); + +describe("exec approvals safe bins", () => { + it("allows safe bins with non-path args", () => { + const dir = makeTempDir(); + const binDir = path.join(dir, "bin"); + fs.mkdirSync(binDir, { recursive: true }); + const exe = path.join(binDir, "jq"); + fs.writeFileSync(exe, ""); + fs.chmodSync(exe, 0o755); + const res = analyzeShellCommand({ + command: "jq .foo", + cwd: dir, + env: { PATH: binDir }, + }); + expect(res.ok).toBe(true); + const segment = res.segments[0]; + const ok = isSafeBinUsage({ + argv: segment.argv, + resolution: segment.resolution, + safeBins: normalizeSafeBins(["jq"]), + cwd: dir, + }); + expect(ok).toBe(true); + }); + + it("blocks safe bins with file args", () => { + const dir = makeTempDir(); + const binDir = path.join(dir, "bin"); + fs.mkdirSync(binDir, { recursive: true }); + const exe = path.join(binDir, "jq"); + fs.writeFileSync(exe, ""); + fs.chmodSync(exe, 0o755); + const file = path.join(dir, "secret.json"); + fs.writeFileSync(file, "{}"); + const res = analyzeShellCommand({ + command: "jq .foo secret.json", + cwd: dir, + env: { PATH: binDir }, + }); + expect(res.ok).toBe(true); + const segment = res.segments[0]; + const ok = isSafeBinUsage({ + argv: segment.argv, + resolution: segment.resolution, + safeBins: normalizeSafeBins(["jq"]), + cwd: dir, + }); + expect(ok).toBe(false); + }); +}); + describe("exec approvals policy helpers", () => { it("minSecurity returns the more restrictive value", () => { expect(minSecurity("deny", "full")).toBe("deny"); diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index ad4846e9c..06d58b0d5 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -60,6 +60,7 @@ const DEFAULT_ASK_FALLBACK: ExecSecurity = "deny"; const DEFAULT_AUTO_ALLOW_SKILLS = false; const DEFAULT_SOCKET = "~/.clawdbot/exec-approvals.sock"; const DEFAULT_FILE = "~/.clawdbot/exec-approvals.json"; +export const DEFAULT_SAFE_BINS = ["jq", "grep", "cut", "sort", "uniq", "head", "tail", "tr", "wc"]; function hashExecApprovalsRaw(raw: string | null): string { return crypto @@ -283,6 +284,19 @@ type CommandResolution = { executableName: string; }; +function isExecutableFile(filePath: string): boolean { + try { + const stat = fs.statSync(filePath); + if (!stat.isFile()) return false; + if (process.platform !== "win32") { + fs.accessSync(filePath, fs.constants.X_OK); + } + return true; + } catch { + return false; + } +} + function parseFirstToken(command: string): string | null { const trimmed = command.trim(); if (!trimmed) return null; @@ -299,15 +313,26 @@ function parseFirstToken(command: string): string | null { function resolveExecutablePath(rawExecutable: string, cwd?: string, env?: NodeJS.ProcessEnv) { const expanded = rawExecutable.startsWith("~") ? expandHome(rawExecutable) : rawExecutable; if (expanded.includes("/") || expanded.includes("\\")) { - if (path.isAbsolute(expanded)) return expanded; + if (path.isAbsolute(expanded)) { + return isExecutableFile(expanded) ? expanded : undefined; + } const base = cwd && cwd.trim() ? cwd.trim() : process.cwd(); - return path.resolve(base, expanded); + const candidate = path.resolve(base, expanded); + return isExecutableFile(candidate) ? candidate : undefined; } const envPath = env?.PATH ?? process.env.PATH ?? ""; const entries = envPath.split(path.delimiter).filter(Boolean); + const extensions = + process.platform === "win32" + ? (env?.PATHEXT ?? process.env.PATHEXT ?? ".EXE;.CMD;.BAT;.COM") + .split(";") + .map((ext) => ext.toLowerCase()) + : [""]; for (const entry of entries) { - const candidate = path.join(entry, expanded); - if (fs.existsSync(candidate)) return candidate; + for (const ext of extensions) { + const candidate = path.join(entry, expanded + ext); + if (isExecutableFile(candidate)) return candidate; + } } return undefined; } @@ -324,6 +349,18 @@ export function resolveCommandResolution( return { rawExecutable, resolvedPath, executableName }; } +export function resolveCommandResolutionFromArgv( + argv: string[], + cwd?: string, + env?: NodeJS.ProcessEnv, +): CommandResolution | null { + const rawExecutable = argv[0]?.trim(); + if (!rawExecutable) return null; + const resolvedPath = resolveExecutablePath(rawExecutable, cwd, env); + const executableName = resolvedPath ? path.basename(resolvedPath) : rawExecutable; + return { rawExecutable, resolvedPath, executableName }; +} + function normalizeMatchTarget(value: string): string { return value.replace(/\\\\/g, "/").toLowerCase(); } @@ -370,24 +407,284 @@ export function matchAllowlist( entries: ExecAllowlistEntry[], resolution: CommandResolution | null, ): ExecAllowlistEntry | null { - if (!entries.length || !resolution) return null; - const rawExecutable = resolution.rawExecutable; + if (!entries.length || !resolution?.resolvedPath) return null; const resolvedPath = resolution.resolvedPath; - const executableName = resolution.executableName; for (const entry of entries) { const pattern = entry.pattern?.trim(); if (!pattern) continue; const hasPath = pattern.includes("/") || pattern.includes("\\") || pattern.includes("~"); - if (hasPath) { - const target = resolvedPath ?? rawExecutable; - if (target && matchesPattern(pattern, target)) return entry; - continue; - } - if (executableName && matchesPattern(pattern, executableName)) return entry; + if (!hasPath) continue; + if (matchesPattern(pattern, resolvedPath)) return entry; } return null; } +export type ExecCommandSegment = { + raw: string; + argv: string[]; + resolution: CommandResolution | null; +}; + +export type ExecCommandAnalysis = { + ok: boolean; + reason?: string; + segments: ExecCommandSegment[]; +}; + +const DISALLOWED_TOKENS = new Set([";", "&", ">", "<", "`", "\n", "\r", "(", ")"]); + +function splitShellPipeline(command: string): { ok: boolean; reason?: string; segments: string[] } { + const segments: string[] = []; + let buf = ""; + let inSingle = false; + let inDouble = false; + let escaped = false; + + const pushSegment = () => { + const trimmed = buf.trim(); + if (!trimmed) { + return false; + } + segments.push(trimmed); + buf = ""; + return true; + }; + + for (let i = 0; i < command.length; i += 1) { + const ch = command[i]; + if (escaped) { + buf += ch; + escaped = false; + continue; + } + if (!inSingle && ch === "\\") { + escaped = true; + buf += ch; + continue; + } + if (inSingle) { + if (ch === "'") inSingle = false; + buf += ch; + continue; + } + if (inDouble) { + if (ch === '"') inDouble = false; + buf += ch; + continue; + } + if (ch === "'") { + inSingle = true; + buf += ch; + continue; + } + if (ch === '"') { + inDouble = true; + buf += ch; + continue; + } + if (ch === "|" && command[i + 1] === "|") { + return { ok: false, reason: "unsupported shell token: ||", segments: [] }; + } + if (ch === "|" && command[i + 1] === "&") { + return { ok: false, reason: "unsupported shell token: |&", segments: [] }; + } + if (ch === "|") { + if (!pushSegment()) { + return { ok: false, reason: "empty pipeline segment", segments: [] }; + } + continue; + } + if (DISALLOWED_TOKENS.has(ch)) { + return { ok: false, reason: `unsupported shell token: ${ch}`, segments: [] }; + } + if (ch === "$" && command[i + 1] === "(") { + return { ok: false, reason: "unsupported shell token: $()", segments: [] }; + } + buf += ch; + } + + if (escaped || inSingle || inDouble) { + return { ok: false, reason: "unterminated shell quote/escape", segments: [] }; + } + if (!pushSegment()) { + return { ok: false, reason: "empty command", segments: [] }; + } + return { ok: true, segments }; +} + +function tokenizeShellSegment(segment: string): string[] | null { + const tokens: string[] = []; + let buf = ""; + let inSingle = false; + let inDouble = false; + let escaped = false; + + const pushToken = () => { + if (buf.length > 0) { + tokens.push(buf); + buf = ""; + } + }; + + for (let i = 0; i < segment.length; i += 1) { + const ch = segment[i]; + if (escaped) { + buf += ch; + escaped = false; + continue; + } + if (!inSingle && ch === "\\") { + escaped = true; + continue; + } + if (inSingle) { + if (ch === "'") { + inSingle = false; + } else { + buf += ch; + } + continue; + } + if (inDouble) { + if (ch === '"') { + inDouble = false; + } else { + buf += ch; + } + continue; + } + if (ch === "'") { + inSingle = true; + continue; + } + if (ch === '"') { + inDouble = true; + continue; + } + if (/\s/.test(ch)) { + pushToken(); + continue; + } + buf += ch; + } + + if (escaped || inSingle || inDouble) { + return null; + } + pushToken(); + return tokens; +} + +export function analyzeShellCommand(params: { + command: string; + cwd?: string; + env?: NodeJS.ProcessEnv; +}): ExecCommandAnalysis { + const split = splitShellPipeline(params.command); + if (!split.ok) { + return { ok: false, reason: split.reason, segments: [] }; + } + const segments: ExecCommandSegment[] = []; + for (const raw of split.segments) { + const argv = tokenizeShellSegment(raw); + if (!argv || argv.length === 0) { + return { ok: false, reason: "unable to parse shell segment", segments: [] }; + } + segments.push({ + raw, + argv, + resolution: resolveCommandResolutionFromArgv(argv, params.cwd, params.env), + }); + } + return { ok: true, segments }; +} + +export function analyzeArgvCommand(params: { + argv: string[]; + cwd?: string; + env?: NodeJS.ProcessEnv; +}): ExecCommandAnalysis { + const argv = params.argv.filter((entry) => entry.trim().length > 0); + if (argv.length === 0) { + return { ok: false, reason: "empty argv", segments: [] }; + } + return { + ok: true, + segments: [ + { + raw: argv.join(" "), + argv, + resolution: resolveCommandResolutionFromArgv(argv, params.cwd, params.env), + }, + ], + }; +} + +function isPathLikeToken(value: string): boolean { + const trimmed = value.trim(); + if (!trimmed) return false; + if (trimmed === "-") return false; + if (trimmed.startsWith("./") || trimmed.startsWith("../") || trimmed.startsWith("~")) return true; + if (trimmed.startsWith("/")) return true; + return /^[A-Za-z]:[\\/]/.test(trimmed); +} + +function defaultFileExists(filePath: string): boolean { + try { + return fs.existsSync(filePath); + } catch { + return false; + } +} + +export function normalizeSafeBins(entries?: string[]): Set { + if (!Array.isArray(entries)) return new Set(); + const normalized = entries + .map((entry) => entry.trim().toLowerCase()) + .filter((entry) => entry.length > 0); + return new Set(normalized); +} + +export function resolveSafeBins(entries?: string[] | null): Set { + if (entries === undefined) return normalizeSafeBins(DEFAULT_SAFE_BINS); + return normalizeSafeBins(entries ?? []); +} + +export function isSafeBinUsage(params: { + argv: string[]; + resolution: CommandResolution | null; + safeBins: Set; + cwd?: string; + fileExists?: (filePath: string) => boolean; +}): boolean { + if (params.safeBins.size === 0) return false; + const resolution = params.resolution; + const execName = resolution?.executableName?.toLowerCase(); + if (!execName || !params.safeBins.has(execName)) return false; + if (!resolution?.resolvedPath) return false; + const cwd = params.cwd ?? process.cwd(); + const exists = params.fileExists ?? defaultFileExists; + const argv = params.argv.slice(1); + for (let i = 0; i < argv.length; i += 1) { + const token = argv[i]; + if (!token) continue; + if (token === "-") continue; + if (token.startsWith("-")) { + const eqIndex = token.indexOf("="); + if (eqIndex > 0) { + const value = token.slice(eqIndex + 1); + if (value && (isPathLikeToken(value) || exists(path.resolve(cwd, value)))) { + return false; + } + } + continue; + } + if (isPathLikeToken(token)) return false; + if (exists(path.resolve(cwd, token))) return false; + } + return true; +} + export function recordAllowlistUse( approvals: ExecApprovalsFile, agentId: string | undefined, diff --git a/src/node-host/runner.ts b/src/node-host/runner.ts index 8d56881dc..ea320fb21 100644 --- a/src/node-host/runner.ts +++ b/src/node-host/runner.ts @@ -4,12 +4,16 @@ import fs from "node:fs"; import path from "node:path"; import { + type ExecAllowlistEntry, addAllowlistEntry, + analyzeArgvCommand, + analyzeShellCommand, + isSafeBinUsage, matchAllowlist, normalizeExecApprovals, recordAllowlistUse, - resolveCommandResolution, resolveExecApprovals, + resolveSafeBins, ensureExecApprovals, readExecApprovalsSnapshot, resolveExecApprovalsSocketPath, @@ -25,6 +29,7 @@ import { import { getMachineDisplayName } from "../infra/machine-name.js"; import { loadOrCreateDeviceIdentity } from "../infra/device-identity.js"; import { loadConfig } from "../config/config.js"; +import { resolveAgentConfig } from "../agents/agent-scope.js"; import { ensureClawdbotCliOnPath } from "../infra/path-env.js"; import { VERSION } from "../version.js"; import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js"; @@ -110,7 +115,6 @@ const execHostFallbackAllowed = process.env.CLAWDBOT_NODE_EXEC_FALLBACK?.trim().toLowerCase() !== "0"; const blockedEnvKeys = new Set([ - "PATH", "NODE_OPTIONS", "PYTHONHOME", "PYTHONPATH", @@ -156,10 +160,24 @@ function sanitizeEnv( ): Record | undefined { if (!overrides) return undefined; const merged = { ...process.env } as Record; + const basePath = process.env.PATH ?? DEFAULT_NODE_PATH; for (const [rawKey, value] of Object.entries(overrides)) { const key = rawKey.trim(); if (!key) continue; const upper = key.toUpperCase(); + if (upper === "PATH") { + const trimmed = value.trim(); + if (!trimmed) continue; + if (!basePath || trimmed === basePath) { + merged[key] = trimmed; + continue; + } + const suffix = `${path.delimiter}${basePath}`; + if (trimmed.endsWith(suffix)) { + merged[key] = trimmed; + } + continue; + } if (blockedEnvKeys.has(upper)) continue; if (blockedEnvPrefixes.some((prefix) => upper.startsWith(prefix))) continue; merged[key] = value; @@ -567,12 +585,32 @@ async function handleInvoke( const sessionKey = params.sessionKey?.trim() || "node"; const runId = crypto.randomUUID(); const env = sanitizeEnv(params.env ?? undefined); - const resolution = resolveCommandResolution(cmdText, params.cwd ?? undefined, env); - const allowlistMatch = - security === "allowlist" ? matchAllowlist(approvals.allowlist, resolution) : null; + const analysis = rawCommand + ? analyzeShellCommand({ command: rawCommand, cwd: params.cwd ?? undefined, env }) + : analyzeArgvCommand({ argv, cwd: params.cwd ?? undefined, env }); + const cfg = loadConfig(); + const agentExec = resolveAgentConfig(cfg, agentId)?.tools?.exec; + const safeBins = resolveSafeBins(agentExec?.safeBins ?? cfg.tools?.exec?.safeBins); + const allowlistMatches: ExecAllowlistEntry[] = []; const bins = autoAllowSkills ? await skillBins.current() : new Set(); - const skillAllow = - autoAllowSkills && resolution?.executableName ? bins.has(resolution.executableName) : false; + const allowlistSatisfied = + security === "allowlist" && analysis.ok && analysis.segments.length > 0 + ? analysis.segments.every((segment) => { + const match = matchAllowlist(approvals.allowlist, segment.resolution); + if (match) allowlistMatches.push(match); + const safe = isSafeBinUsage({ + argv: segment.argv, + resolution: segment.resolution, + safeBins, + cwd: params.cwd ?? undefined, + }); + const skillAllow = + autoAllowSkills && segment.resolution?.executableName + ? bins.has(segment.resolution.executableName) + : false; + return Boolean(match || safe || skillAllow); + }) + : false; const useMacAppExec = process.platform === "darwin"; if (useMacAppExec) { @@ -678,7 +716,7 @@ async function handleInvoke( const requiresAsk = ask === "always" || - (ask === "on-miss" && security === "allowlist" && !allowlistMatch && !skillAllow); + (ask === "on-miss" && security === "allowlist" && (!analysis.ok || !allowlistSatisfied)); const approvalDecision = params.approvalDecision === "allow-once" || params.approvalDecision === "allow-always" @@ -704,11 +742,15 @@ async function handleInvoke( return; } if (approvalDecision === "allow-always" && security === "allowlist") { - const pattern = resolution?.resolvedPath ?? resolution?.rawExecutable ?? argv[0] ?? ""; - if (pattern) addAllowlistEntry(approvals.file, agentId, pattern); + if (analysis.ok) { + for (const segment of analysis.segments) { + const pattern = segment.resolution?.resolvedPath ?? ""; + if (pattern) addAllowlistEntry(approvals.file, agentId, pattern); + } + } } - if (security === "allowlist" && !allowlistMatch && !skillAllow && !approvedByAsk) { + if (security === "allowlist" && (!analysis.ok || !allowlistSatisfied) && !approvedByAsk) { await sendNodeEvent( client, "exec.denied", @@ -727,8 +769,19 @@ async function handleInvoke( return; } - if (allowlistMatch) { - recordAllowlistUse(approvals.file, agentId, allowlistMatch, cmdText, resolution?.resolvedPath); + if (allowlistMatches.length > 0) { + const seen = new Set(); + for (const match of allowlistMatches) { + if (!match?.pattern || seen.has(match.pattern)) continue; + seen.add(match.pattern); + recordAllowlistUse( + approvals.file, + agentId, + match, + cmdText, + analysis.segments[0]?.resolution?.resolvedPath, + ); + } } if (params.needsScreenRecording === true) {