diff --git a/docs/tools/access-policy.md b/docs/tools/access-policy.md new file mode 100644 index 00000000000..6ad4020fe56 --- /dev/null +++ b/docs/tools/access-policy.md @@ -0,0 +1,214 @@ +--- +summary: "Path-scoped RWX permissions for file and exec tools" +read_when: + - Restricting which paths an agent can read, write, or execute + - Configuring per-agent filesystem access policies + - Hardening single-OS-user gateway deployments +title: "Access Policy" +--- + +# Access policy + +Access policy lets you restrict what paths an agent can **read**, **write**, or **execute** — independently of which binary is running. It enforces at two layers: the tool layer (read/write/edit/exec tools) and, on macOS, the OS layer via `sandbox-exec`. + +## Why this exists + +The exec allowlist controls _which binaries_ an agent can run, but it cannot restrict _which paths_ those binaries touch. A permitted `/bin/ls` on `~/workspace` is equally permitted on `~/.ssh`. Access policy closes that gap by scoping permissions to path patterns instead of binary names. + +## Config file + +Access policy is configured in a **sidecar file** separate from `openclaw.json`: + +``` +~/.openclaw/access-policy.json +``` + +The file is **optional** — if absent, all operations pass through unchanged with no enforcement. No restart is required when the file changes; it is read fresh on each agent turn. + +## Format + +```json +{ + "version": 1, + "agents": { + "*": { + "policy": { + "/**": "r--", + "/tmp/": "rwx", + "~/": "rw-", + "~/dev/": "rwx", + "~/.ssh/**": "---", + "~/.aws/**": "---" + } + }, + "myagent": { "policy": { "~/private/": "rw-" } } + } +} +``` + +### Permission strings + +Each rule value is a three-character string — one character per operation: + +| Position | Letter | Meaning | +| -------- | --------- | ------------------------ | +| 0 | `r` / `-` | Read allowed / denied | +| 1 | `w` / `-` | Write allowed / denied | +| 2 | `x` / `-` | Execute allowed / denied | + +Examples: `"rwx"` (full access), `"r--"` (read only), `"r-x"` (read + exec), `"---"` (deny all). + +Use `"---"` to explicitly deny all access to a path — this is the deny mechanism. A rule with `"---"` always blocks regardless of broader rules, as long as it is the longest (most specific) matching pattern. + +### Pattern syntax + +- Patterns are path globs: `*` matches within a segment, `**` matches any depth. +- Trailing `/` is shorthand for `/**` — e.g. `"/tmp/"` matches everything under `/tmp`. +- `~` expands to the OS home directory (not `OPENCLAW_HOME`). +- On macOS, `/tmp`, `/var`, and `/etc` are transparently normalized from their `/private/*` real paths. + +### Precedence + +1. **`policy`** — longest matching glob wins (most specific pattern takes priority). +2. **Implicit fallback** — `"---"` (deny all) when no rule matches. Use `"/**": "r--"` (or any perm) as an explicit catch-all. + +To deny a specific path, add a `"---"` rule that is more specific than any allow rule covering that path: + +```json +"policy": { + "/**": "r--", + "~/.ssh/**": "---" +} +``` + +`~/.ssh/**` is longer than `/**` so it wins for any path under `~/.ssh/`. + +## Layers + +``` +agents["*"] → agents["myagent"] +``` + +- **`agents["*"]`** — base policy applied to every agent. Put org-wide rules here. Can include both `policy` (path rules) and `scripts` (per-script overrides). +- **`agents["myagent"]`** — per-agent overrides merged on top of `agents["*"]`. `policy` rules are shallow-merged (agent wins on collision). `scripts` entries are deep-merged: the base `sha256` is always preserved and cannot be overridden by an agent block. + +Named agents can also add their own `scripts` block, which is merged with the base scripts config. + +## Per-script policy + +The `scripts` block inside any agent config grants additional path permissions when a **specific binary** is the exec target. The override fires only when `resolvedArgv0` matches a key in `scripts` — it does not apply to unmatched exec calls. + +```json +{ + "version": 1, + "agents": { + "*": { + "policy": { "/**": "r--" }, + "scripts": { + "policy": { + "/tmp/**": "rw-" + }, + "~/bin/deploy.sh": { + "policy": { "~/deploy/**": "rwx" }, + "sha256": "" + } + } + }, + "veda": { + "policy": { "~/.openclaw/agents/veda/workspace/**": "rwx" }, + "scripts": { + "~/bin/veda-tool.sh": { + "policy": { "/opt/data/**": "r--" } + } + } + } + } +} +``` + +### `scripts["policy"]` + +A flat `{ path: perm }` map of shared path rules. These rules are merged as the **base** for every per-script entry in the same `scripts` block, before the per-script `policy` is applied. + +**Important:** `scripts["policy"]` only takes effect when an exec call matches one of the named script keys. If the `scripts` block has `scripts["policy"]` but no actual script entries, those rules are never applied. + +### Per-script entries + +Each key is the resolved absolute path to a script (tilde is expanded, symlinks are followed at match time). + +- **`policy`** — path rules that add to or narrow the base policy for this script only. Override rules are emitted _after_ base rules in the OS sandbox profile so a script grant can reach inside a broadly denied subtree (last-match-wins semantics). +- **`sha256`** — optional SHA-256 hex of the script file. When set, exec is denied if the hash does not match. Best-effort integrity check — there is a small TOCTOU window between the hash read and kernel exec. + +### Script override flow + +When an exec call matches a script key: + +1. `scripts["policy"]` shared rules are merged as a base. +2. The matching script's `policy` is merged on top (override key wins). +3. The resulting path rules are emitted _after_ the agent's main `policy` rules in the OS sandbox profile. + +The `scripts` block is stripped from the policy after the match so it does not bleed into unrelated tool calls in the same agent turn. + +## Enforcement + +### Tool layer + +Every read, write, edit, and exec tool call checks the resolved path against the active policy before executing. A denied path throws immediately — the operation never reaches the OS. + +### OS layer (macOS) + +On macOS, exec commands are additionally wrapped with `sandbox-exec` using a generated Seatbelt (SBPL) profile derived from the policy. This catches paths that expand at runtime (e.g. `cat $HOME/.ssh/id_rsa`) that config-level heuristics cannot intercept. + +On Linux, a `bwrap` (bubblewrap) wrapper is generated instead. + +## Validation + +If the file exists but cannot be parsed, or contains structural errors (wrong nesting, misplaced keys), a clear error is logged and **all access is denied** (fail-closed) until the file is fixed: + +``` +[access-policy] Cannot parse ~/.openclaw/access-policy.json: ... +[access-policy] Failing closed (default: "---") until the file is fixed. +``` + +Common mistakes caught by the validator: + +- `policy`, `rules`, `scripts`, or `base` placed at the top level instead of under `agents["*"]` +- Permission strings that are not exactly 3 characters (`"rwx"`, `"r--"`, `"---"`, etc.) +- `deny` or `default` keys inside agent blocks — these fields were removed; use `"---"` rules instead + +### Bare directory paths + +If a rule path has no glob suffix and resolves to a real directory (e.g. `"~/dev/openclaw"` instead of `"~/dev/openclaw/**"`), the validator auto-expands it to `/**` and logs a one-time diagnostic: + +``` +[access-policy] access-policy.policy["~/dev/openclaw"] is a directory — rule auto-expanded to "~/dev/openclaw/**" so it covers all contents. +``` + +A bare path without `/**` would match only the directory entry itself, not its contents. + +Auto-expansion also applies to bare directory paths inside `scripts["policy"]` and per-script `policy` blocks. + +## A2A trust scope + +When an agent spawns a subagent, the subagent runs with its own agent identity and its own policy block applies. This is correct for standard OpenClaw subagent spawning. + +For cross-agent MCP tool delegation (an orchestrator invoking a tool on behalf of a subagent via an MCP channel), the calling agent's identity governs — no automatic narrowing to the subagent's policy occurs. Explicit delegation controls are planned as a follow-up. + +## Known limitations + +**Metadata leak via directory listing.** `find`, `ls`, and shell globs use `readdir()` to enumerate directory contents, which is allowed. When content access is then denied at `open()`, the filenames are already visible in the error output. Content is protected; filenames are not. This is inherent to how OS-level enforcement works at the syscall level. + +**Interpreter bypass of exec bit.** The `x` bit gates `execve()` on the file itself. Running `bash script.sh` executes bash (permitted), which reads the script as text (read permitted if `r` is set). The exec bit on the script is irrelevant for interpreter-based invocations. To prevent execution of a script entirely, deny read access to it (`"---"`). + +**File-level `"---"` rules on Linux (bwrap).** On Linux, `"---"` rules are enforced at the OS layer using `bwrap --tmpfs` overlays, which only work on directories. When a `"---"` rule resolves to an existing file (e.g. `"~/.netrc": "---"`), the OS-level mount is skipped — bwrap cannot overlay a file with a tmpfs. Tool-layer enforcement still blocks read/write/edit calls for that file. However, exec commands running inside the sandbox can still access the file directly (e.g. `cat ~/.netrc`). A warning is emitted to stderr when this gap is active. To enforce at the OS layer on Linux, deny the parent directory instead (e.g. `"~/.aws/**": "---"` rather than `"~/.aws/credentials": "---"`). On macOS, seatbelt handles file-level denials correctly with `(deny file-read* (literal ...))`. + +**Mid-path wildcard patterns and OS-level exec enforcement.** Patterns with a wildcard in a non-final segment — such as `skills/**/*.sh` or `logs/*/app.log` — cannot be expressed as OS-level subpath matchers. bwrap and Seatbelt do not understand glob syntax; they work with concrete directory prefixes. For non-deny rules, OpenClaw emits the longest concrete prefix (`skills/`) as an approximate OS-level rule for read and write access, which is bounded and safe. The exec bit is intentionally omitted from the OS approximation: granting exec on the entire prefix directory would allow any binary under that directory to be executed by subprocesses, not just files matching the original pattern. Exec for mid-path wildcard patterns is enforced by the tool layer only. To get OS-level exec enforcement, use a trailing-`**` pattern such as `skills/**` (which covers the directory precisely, with the file-type filter applying at the tool layer only). + +**`scripts["policy"]` requires at least one script entry to take effect.** Shared script rules in `scripts["policy"]` are only applied when a specific script key matches the exec target. A `scripts` block with only `scripts["policy"]` and no named script entries has no effect on any exec call. + +**No approval flow.** Access policy is fail-closed: a denied operation returns an error immediately. There is no `ask`/`on-miss` mode equivalent to exec approvals. If an agent hits a denied path, it receives a permission error and must handle it. Interactive approval for filesystem access is planned as a follow-up feature. + +## Related + +- [Exec approvals](/tools/exec-approvals) — allowlist-based exec gating (complements access policy) +- [Exec tool](/tools/exec) — exec tool reference diff --git a/src/agents/bash-tools.exec-host-gateway.ts b/src/agents/bash-tools.exec-host-gateway.ts index 149a4785dd5..33988561b0c 100644 --- a/src/agents/bash-tools.exec-host-gateway.ts +++ b/src/agents/bash-tools.exec-host-gateway.ts @@ -1,4 +1,6 @@ import type { AgentToolResult } from "@mariozechner/pi-agent-core"; +import type { AccessPolicyConfig } from "../config/types.tools.js"; +import { buildExecApprovalUnavailableReplyPayload } from "../infra/exec-approval-reply.js"; import { addAllowlistEntry, type ExecAsk, @@ -13,22 +15,20 @@ import { detectCommandObfuscation } from "../infra/exec-obfuscation-detect.js"; import type { SafeBinProfile } from "../infra/exec-safe-bin-policy.js"; import { logInfo } from "../logger.js"; import { markBackgrounded, tail } from "./bash-process-registry.js"; +import { sendExecApprovalFollowup } from "./bash-tools.exec-approval-followup.js"; import { buildExecApprovalRequesterContext, buildExecApprovalTurnSourceContext, registerExecApprovalRequestForHostOrThrow, } from "./bash-tools.exec-approval-request.js"; import { - buildDefaultExecApprovalRequestArgs, - buildExecApprovalFollowupTarget, - buildExecApprovalPendingToolResult, - createExecApprovalDecisionState, createAndRegisterDefaultExecApprovalRequest, + resolveBaseExecApprovalDecision, resolveApprovalDecisionOrUndefined, resolveExecHostApprovalContext, - sendExecApprovalFollowupResult, } from "./bash-tools.exec-host-shared.js"; import { + buildApprovalPendingMessage, DEFAULT_NOTIFY_TAIL_CHARS, createApprovalSlug, normalizeNotifyOutput, @@ -60,6 +60,7 @@ export type ProcessGatewayAllowlistParams = { maxOutput: number; pendingMaxOutput: number; trustedSafeBinDirs?: ReadonlySet; + permissions?: AccessPolicyConfig; }; export type ProcessGatewayAllowlistResult = { @@ -141,28 +142,6 @@ export async function processGatewayAllowlist( } if (requiresAsk) { - const requestArgs = buildDefaultExecApprovalRequestArgs({ - warnings: params.warnings, - approvalRunningNoticeMs: params.approvalRunningNoticeMs, - createApprovalSlug, - turnSourceChannel: params.turnSourceChannel, - turnSourceAccountId: params.turnSourceAccountId, - }); - const registerGatewayApproval = async (approvalId: string) => - await registerExecApprovalRequestForHostOrThrow({ - approvalId, - command: params.command, - workdir: params.workdir, - host: "gateway", - security: hostSecurity, - ask: hostAsk, - ...buildExecApprovalRequesterContext({ - agentId: params.agentId, - sessionKey: params.sessionKey, - }), - resolvedPath: allowlistEval.segments[0]?.resolution?.resolvedPath, - ...buildExecApprovalTurnSourceContext(params), - }); const { approvalId, approvalSlug, @@ -173,46 +152,57 @@ export async function processGatewayAllowlist( sentApproverDms, unavailableReason, } = await createAndRegisterDefaultExecApprovalRequest({ - ...requestArgs, - register: registerGatewayApproval, + warnings: params.warnings, + approvalRunningNoticeMs: params.approvalRunningNoticeMs, + createApprovalSlug, + turnSourceChannel: params.turnSourceChannel, + turnSourceAccountId: params.turnSourceAccountId, + register: async (approvalId) => + await registerExecApprovalRequestForHostOrThrow({ + approvalId, + command: params.command, + workdir: params.workdir, + host: "gateway", + security: hostSecurity, + ask: hostAsk, + ...buildExecApprovalRequesterContext({ + agentId: params.agentId, + sessionKey: params.sessionKey, + }), + resolvedPath: allowlistEval.segments[0]?.resolution?.resolvedPath, + ...buildExecApprovalTurnSourceContext(params), + }), }); const resolvedPath = allowlistEval.segments[0]?.resolution?.resolvedPath; const effectiveTimeout = typeof params.timeoutSec === "number" ? params.timeoutSec : params.defaultTimeoutSec; - const followupTarget = buildExecApprovalFollowupTarget({ - approvalId, - sessionKey: params.notifySessionKey, - turnSourceChannel: params.turnSourceChannel, - turnSourceTo: params.turnSourceTo, - turnSourceAccountId: params.turnSourceAccountId, - turnSourceThreadId: params.turnSourceThreadId, - }); void (async () => { const decision = await resolveApprovalDecisionOrUndefined({ approvalId, preResolvedDecision, onFailure: () => - void sendExecApprovalFollowupResult( - followupTarget, - `Exec denied (gateway id=${approvalId}, approval-request-failed): ${params.command}`, - ), + void sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: `Exec denied (gateway id=${approvalId}, approval-request-failed): ${params.command}`, + }), }); if (decision === undefined) { return; } - const { - baseDecision, - approvedByAsk: initialApprovedByAsk, - deniedReason: initialDeniedReason, - } = createExecApprovalDecisionState({ + const baseDecision = resolveBaseExecApprovalDecision({ decision, askFallback, obfuscationDetected: obfuscation.detected, }); - let approvedByAsk = initialApprovedByAsk; - let deniedReason = initialDeniedReason; + let approvedByAsk = baseDecision.approvedByAsk; + let deniedReason = baseDecision.deniedReason; if (baseDecision.timedOut && askFallback === "allowlist") { if (!analysisOk || !allowlistSatisfied) { @@ -244,10 +234,15 @@ export async function processGatewayAllowlist( } if (deniedReason) { - await sendExecApprovalFollowupResult( - followupTarget, - `Exec denied (gateway id=${approvalId}, ${deniedReason}): ${params.command}`, - ); + await sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: `Exec denied (gateway id=${approvalId}, ${deniedReason}): ${params.command}`, + }).catch(() => {}); return; } @@ -271,12 +266,18 @@ export async function processGatewayAllowlist( scopeKey: params.scopeKey, sessionKey: params.notifySessionKey, timeoutSec: effectiveTimeout, + permissions: params.permissions, }); } catch { - await sendExecApprovalFollowupResult( - followupTarget, - `Exec denied (gateway id=${approvalId}, spawn-failed): ${params.command}`, - ); + await sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: `Exec denied (gateway id=${approvalId}, spawn-failed): ${params.command}`, + }).catch(() => {}); return; } @@ -290,25 +291,70 @@ export async function processGatewayAllowlist( const summary = output ? `Exec finished (gateway id=${approvalId}, session=${run.session.id}, ${exitLabel})\n${output}` : `Exec finished (gateway id=${approvalId}, session=${run.session.id}, ${exitLabel})`; - await sendExecApprovalFollowupResult(followupTarget, summary); + await sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: summary, + }).catch(() => {}); })(); return { - pendingResult: buildExecApprovalPendingToolResult({ - host: "gateway", - command: params.command, - cwd: params.workdir, - warningText, - approvalId, - approvalSlug, - expiresAtMs, - initiatingSurface, - sentApproverDms, - unavailableReason, - }), + pendingResult: { + content: [ + { + type: "text", + text: + unavailableReason !== null + ? (buildExecApprovalUnavailableReplyPayload({ + warningText, + reason: unavailableReason, + channelLabel: initiatingSurface.channelLabel, + sentApproverDms, + }).text ?? "") + : buildApprovalPendingMessage({ + warningText, + approvalSlug, + approvalId, + command: params.command, + cwd: params.workdir, + host: "gateway", + }), + }, + ], + details: + unavailableReason !== null + ? ({ + status: "approval-unavailable", + reason: unavailableReason, + channelLabel: initiatingSurface.channelLabel, + sentApproverDms, + host: "gateway", + command: params.command, + cwd: params.workdir, + warningText, + } satisfies ExecToolDetails) + : ({ + status: "approval-pending", + approvalId, + approvalSlug, + expiresAtMs, + host: "gateway", + command: params.command, + cwd: params.workdir, + warningText, + } satisfies ExecToolDetails), + }, }; } + // Allowlist and path-level sandboxing (seatbelt/bwrap) are orthogonal controls: + // allowlist governs which binaries may run; seatbelt governs which paths they touch. + // Both must be enforced independently — having seatbelt active does not exempt a + // command from passing allowlist analysis. if (hostSecurity === "allowlist" && (!analysisOk || !allowlistSatisfied)) { throw new Error("exec denied: allowlist miss"); } diff --git a/src/agents/bash-tools.exec-runtime.test.ts b/src/agents/bash-tools.exec-runtime.test.ts index 35a38b5483d..53291425e70 100644 --- a/src/agents/bash-tools.exec-runtime.test.ts +++ b/src/agents/bash-tools.exec-runtime.test.ts @@ -10,7 +10,11 @@ vi.mock("../infra/system-events.js", () => ({ import { requestHeartbeatNow } from "../infra/heartbeat-wake.js"; import { enqueueSystemEvent } from "../infra/system-events.js"; -import { emitExecSystemEvent } from "./bash-tools.exec-runtime.js"; +import { + _resetBwrapUnavailableWarnedForTest, + _resetWindowsUnconfiguredWarnedForTest, + emitExecSystemEvent, +} from "./bash-tools.exec-runtime.js"; const requestHeartbeatNowMock = vi.mocked(requestHeartbeatNow); const enqueueSystemEventMock = vi.mocked(enqueueSystemEvent); @@ -62,3 +66,21 @@ describe("emitExecSystemEvent", () => { expect(requestHeartbeatNowMock).not.toHaveBeenCalled(); }); }); + +// --------------------------------------------------------------------------- +// One-time warning reset helpers (exported for tests) +// --------------------------------------------------------------------------- + +describe("_resetBwrapUnavailableWarnedForTest / _resetWindowsUnconfiguredWarnedForTest", () => { + it("exports _resetBwrapUnavailableWarnedForTest as a function", () => { + // Verify the export exists and is callable — the reset enables repeated + // warning tests without cross-test state leakage. + expect(typeof _resetBwrapUnavailableWarnedForTest).toBe("function"); + expect(() => _resetBwrapUnavailableWarnedForTest()).not.toThrow(); + }); + + it("exports _resetWindowsUnconfiguredWarnedForTest as a function", () => { + expect(typeof _resetWindowsUnconfiguredWarnedForTest).toBe("function"); + expect(() => _resetWindowsUnconfiguredWarnedForTest()).not.toThrow(); + }); +}); diff --git a/src/agents/bash-tools.exec-runtime.ts b/src/agents/bash-tools.exec-runtime.ts index 5c3301414b9..243af461794 100644 --- a/src/agents/bash-tools.exec-runtime.ts +++ b/src/agents/bash-tools.exec-runtime.ts @@ -1,3 +1,4 @@ +import os from "node:os"; import path from "node:path"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; import { Type } from "@sinclair/typebox"; @@ -16,6 +17,18 @@ export { normalizeExecHost, normalizeExecSecurity, } from "../infra/exec-approvals.js"; +import type { AccessPolicyConfig } from "../config/types.tools.js"; +import { + applyScriptPolicyOverride, + checkAccessPolicy, + resolveArgv0, + resolveScriptKey, +} from "../infra/access-policy.js"; +import { isBwrapAvailable, wrapCommandWithBwrap } from "../infra/exec-sandbox-bwrap.js"; +import { + generateSeatbeltProfile, + wrapCommandWithSeatbelt, +} from "../infra/exec-sandbox-seatbelt.js"; import { logWarn } from "../logger.js"; import type { ManagedRun } from "../process/supervisor/index.js"; import { getProcessSupervisor } from "../process/supervisor/index.js"; @@ -286,6 +299,40 @@ export function emitExecSystemEvent( requestHeartbeatNow(scopedHeartbeatWakeOptions(sessionKey, { reason: "exec-event" })); } +// Warn once per process when OS-level exec enforcement is unavailable and +// access-policy permissions are configured — so operators know exec runs unconfined. +let _bwrapUnavailableWarned = false; +function _warnBwrapUnavailableOnce(): void { + if (_bwrapUnavailableWarned) { + return; + } + _bwrapUnavailableWarned = true; + console.error( + "[access-policy] WARNING: bwrap is not available on this Linux host — exec commands run unconfined. Install bubblewrap to enable OS-level exec enforcement.", + ); +} + +/** Reset the one-time bwrap-unavailable warning flag. Only for use in tests. */ +export function _resetBwrapUnavailableWarnedForTest(): void { + _bwrapUnavailableWarned = false; +} + +let _windowsUnconfiguredWarned = false; +function _warnWindowsUnconfiguredOnce(): void { + if (_windowsUnconfiguredWarned) { + return; + } + _windowsUnconfiguredWarned = true; + console.error( + "[access-policy] WARNING: OS-level exec enforcement is not supported on Windows — exec commands run unconfined even when access-policy permissions are configured.", + ); +} + +/** Reset the one-time Windows-unconfigured warning flag. Only for use in tests. */ +export function _resetWindowsUnconfiguredWarnedForTest(): void { + _windowsUnconfiguredWarned = false; +} + export async function runExecProcess(opts: { command: string; // Execute this instead of `command` (which is kept for display/session/logging). @@ -305,10 +352,84 @@ export async function runExecProcess(opts: { sessionKey?: string; timeoutSec: number | null; onUpdate?: (partialResult: AgentToolResult) => void; + /** When set, wrap the exec command with OS-level path enforcement. */ + permissions?: AccessPolicyConfig; }): Promise { const startedAt = Date.now(); const sessionId = createSessionSlug(); - const execCommand = opts.execCommand ?? opts.command; + const baseCommand = opts.execCommand ?? opts.command; + + // Apply access-policy enforcement when permissions are configured. + // Hash verification and tool-layer exec checks run unconditionally — container + // sandboxes (opts.sandbox) share the host filesystem via volume mounts so a + // tampered script is still reachable. Only OS-level wrapping (seatbelt/bwrap) + // is skipped when a container sandbox already provides filesystem isolation. + let execCommand = baseCommand; + if (opts.permissions) { + // Fall back to the first token rather than the full command string so that + // checkAccessPolicy matches against a path-like token instead of a multi-word + // string that never matches any absolute-path rule (and would pass unconditionally + // under a permissive default). + const argv0 = + resolveArgv0(baseCommand, opts.workdir) ?? baseCommand.trim().split(/\s+/)[0] ?? baseCommand; + const { + policy: effectivePermissions, + overrideRules, + hashMismatch, + } = applyScriptPolicyOverride(opts.permissions, argv0); + if (hashMismatch) { + throw new Error(`exec denied: script hash mismatch for ${argv0}`); + } + // Tool-layer exec path check — defense-in-depth for platforms where OS-level + // enforcement (seatbelt/bwrap) is unavailable (Linux without bwrap, Windows). + // Mirrors the checkAccessPolicy calls in read/write tools for consistency. + // + // For scripts{} entries, skip the broader rules check — a sha256-matched script + // doesn't also need an explicit exec rule in the base policy. + // Use resolveScriptKey so that both tilde keys ("~/bin/deploy.sh") and + // symlink keys ("/usr/bin/python" → /usr/bin/python3.12) match argv0, which + // is always the realpathSync result from resolveArgv0. + const _scripts = opts.permissions.scripts ?? {}; + // Skip the reserved "policy" key and malformed (non-object) entries — only a + // validated ScriptPolicyEntry object counts as a legitimate script override. + // A truthy primitive (true, "oops") would otherwise bypass the base exec gate + // even though applyScriptPolicyOverride already rejected the entry. + const hasScriptOverride = Object.entries(_scripts).some( + ([k, v]) => + k !== "policy" && + v != null && + typeof v === "object" && + !Array.isArray(v) && + path.normalize(resolveScriptKey(k)) === path.normalize(argv0), + ); + if (!hasScriptOverride && checkAccessPolicy(argv0, "exec", effectivePermissions) === "deny") { + throw new Error(`exec denied by access policy: ${argv0}`); + } + // OS-level sandbox wrapping — skip when a container sandbox already isolates the process. + if (!opts.sandbox) { + if (process.platform === "darwin") { + const profile = generateSeatbeltProfile(effectivePermissions, os.homedir(), overrideRules); + execCommand = wrapCommandWithSeatbelt(baseCommand, profile); + } else if (process.platform === "linux") { + if (await isBwrapAvailable()) { + // Pass overrideRules separately so they are emitted AFTER deny[] mounts, + // giving script-specific grants precedence over base deny entries — matching + // the Seatbelt path where scriptOverrideRules are emitted last in the profile. + execCommand = wrapCommandWithBwrap( + baseCommand, + effectivePermissions, + os.homedir(), + overrideRules, + ); + } else { + _warnBwrapUnavailableOnce(); + } + } else if (process.platform === "win32") { + _warnWindowsUnconfiguredOnce(); + } + } + } + const supervisor = getProcessSupervisor(); const shellRuntimeEnv: Record = { ...opts.env, diff --git a/src/agents/bash-tools.exec-types.ts b/src/agents/bash-tools.exec-types.ts index 7236fdaaf47..efd6da40ccd 100644 --- a/src/agents/bash-tools.exec-types.ts +++ b/src/agents/bash-tools.exec-types.ts @@ -1,8 +1,11 @@ +import type { AccessPolicyConfig } from "../config/types.tools.js"; import type { ExecAsk, ExecHost, ExecSecurity } from "../infra/exec-approvals.js"; import type { SafeBinProfileFixture } from "../infra/exec-safe-bin-policy.js"; import type { BashSandboxConfig } from "./bash-tools.shared.js"; export type ExecToolDefaults = { + /** Path-scoped RWX permissions — x bit gates binary execution. */ + permissions?: AccessPolicyConfig; host?: ExecHost; security?: ExecSecurity; ask?: ExecAsk; diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index 8a0bd30907a..73424f44a7d 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -449,6 +449,7 @@ export function createExecTool( maxOutput, pendingMaxOutput, trustedSafeBinDirs, + permissions: defaults?.permissions, }); if (gatewayResult.pendingResult) { return gatewayResult.pendingResult; @@ -486,6 +487,7 @@ export function createExecTool( sessionKey: notifySessionKey, timeoutSec: effectiveTimeout, onUpdate, + permissions: defaults?.permissions, }); let yielded = false; diff --git a/src/agents/pi-tools.read.edit-permission.test.ts b/src/agents/pi-tools.read.edit-permission.test.ts new file mode 100644 index 00000000000..495d452ea67 --- /dev/null +++ b/src/agents/pi-tools.read.edit-permission.test.ts @@ -0,0 +1,103 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { AccessPolicyConfig } from "../config/types.tools.js"; + +type CapturedEditOperations = { + readFile: (absolutePath: string) => Promise; + writeFile: (absolutePath: string, content: string) => Promise; + access: (absolutePath: string) => Promise; +}; + +const mocks = vi.hoisted(() => ({ + operations: undefined as CapturedEditOperations | undefined, +})); + +vi.mock("@mariozechner/pi-coding-agent", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createEditTool: (_cwd: string, options?: { operations?: CapturedEditOperations }) => { + mocks.operations = options?.operations; + return { + name: "edit", + description: "test edit tool", + parameters: { type: "object", properties: {} }, + execute: async () => ({ + content: [{ type: "text" as const, text: "ok" }], + }), + }; + }, + }; +}); + +const { createHostWorkspaceEditTool } = await import("./pi-tools.read.js"); + +describe("createHostWorkspaceEditTool edit read-permission check", () => { + let tmpDir = ""; + + afterEach(async () => { + mocks.operations = undefined; + if (tmpDir) { + await fs.rm(tmpDir, { recursive: true, force: true }); + tmpDir = ""; + } + }); + + it.runIf(process.platform !== "win32")( + "readFile throws when read access is denied by permissions (write-only policy)", + async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-perm-test-")); + const filePath = path.join(tmpDir, "protected.txt"); + await fs.writeFile(filePath, "secret content", "utf8"); + + // "-w-" policy: write allowed, read denied. + // Edit must NOT be allowed to read the file even if write is permitted. + const permissions: AccessPolicyConfig = { + policy: { [`${tmpDir}/**`]: "-w-" }, + }; + createHostWorkspaceEditTool(tmpDir, { workspaceOnly: false, permissions }); + expect(mocks.operations).toBeDefined(); + + await expect(mocks.operations!.readFile(filePath)).rejects.toThrow(/Permission denied.*read/); + }, + ); + + it.runIf(process.platform !== "win32")( + "readFile succeeds when read access is granted by permissions", + async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-perm-test-")); + const filePath = path.join(tmpDir, "allowed.txt"); + await fs.writeFile(filePath, "content", "utf8"); + + const permissions: AccessPolicyConfig = { + policy: { [`${tmpDir}/**`]: "rw-" }, + }; + createHostWorkspaceEditTool(tmpDir, { workspaceOnly: false, permissions }); + expect(mocks.operations).toBeDefined(); + + await expect(mocks.operations!.readFile(filePath)).resolves.toBeDefined(); + }, + ); + + it.runIf(process.platform !== "win32")( + "writeFile throws when write access is denied by permissions", + async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-perm-test-")); + const filePath = path.join(tmpDir, "readonly.txt"); + await fs.writeFile(filePath, "content", "utf8"); + + // "r--" policy: read allowed, write denied. + const permissions: AccessPolicyConfig = { + policy: { [`${tmpDir}/**`]: "r--" }, + }; + createHostWorkspaceEditTool(tmpDir, { workspaceOnly: false, permissions }); + expect(mocks.operations).toBeDefined(); + + await expect(mocks.operations!.writeFile(filePath, "new")).rejects.toThrow( + /Permission denied.*write/, + ); + }, + ); +}); diff --git a/src/agents/pi-tools.read.ts b/src/agents/pi-tools.read.ts index 5ea48b01fa1..8c695744cee 100644 --- a/src/agents/pi-tools.read.ts +++ b/src/agents/pi-tools.read.ts @@ -1,8 +1,11 @@ +import { realpathSync } from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; import { fileURLToPath } from "node:url"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; import { createEditTool, createReadTool, createWriteTool } from "@mariozechner/pi-coding-agent"; +import type { AccessPolicyConfig } from "../config/types.tools.js"; +import { checkAccessPolicy } from "../infra/access-policy.js"; import { appendFileWithinRoot, SafeOpenError, @@ -47,9 +50,44 @@ const ADAPTIVE_READ_CONTEXT_SHARE = 0.2; const CHARS_PER_TOKEN_ESTIMATE = 4; const MAX_ADAPTIVE_READ_PAGES = 8; +/** + * Resolve symlinks before a policy check. For paths that don't exist yet + * (e.g. a new file being created), resolves the parent directory so that + * intermediate symlinks are followed. Without this, a write to + * `/allowed/link/new.txt` where `link → /denied` would pass the check + * (path.resolve does not follow symlinks) and then land in the denied + * target when fs.writeFile follows the symlink. + */ +function safeRealpath(p: string): string { + try { + return realpathSync(p); + } catch { + // Path doesn't exist yet — walk up ancestors until we find one that exists, + // resolve it, then reconstruct the full path. + const parts: string[] = []; + let ancestor = p; + while (true) { + const parent = path.dirname(ancestor); + if (parent === ancestor) { + return path.resolve(p); + } + parts.unshift(path.basename(ancestor)); + ancestor = parent; + try { + return path.join(realpathSync(ancestor), ...parts); + } catch { + // Keep walking up. + } + } + } +} + type OpenClawReadToolOptions = { modelContextWindowTokens?: number; imageSanitization?: ImageSanitizationLimits; + permissions?: AccessPolicyConfig; + /** Workspace root used to resolve relative paths for permission checks. */ + workspaceRoot?: string; }; type ReadTruncationDetails = { @@ -621,14 +659,20 @@ export function createSandboxedEditTool(params: SandboxToolParams) { return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit); } -export function createHostWorkspaceWriteTool(root: string, options?: { workspaceOnly?: boolean }) { +export function createHostWorkspaceWriteTool( + root: string, + options?: { workspaceOnly?: boolean; permissions?: AccessPolicyConfig }, +) { const base = createWriteTool(root, { operations: createHostWriteOperations(root, options), }) as unknown as AnyAgentTool; return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.write); } -export function createHostWorkspaceEditTool(root: string, options?: { workspaceOnly?: boolean }) { +export function createHostWorkspaceEditTool( + root: string, + options?: { workspaceOnly?: boolean; permissions?: AccessPolicyConfig }, +) { const base = createEditTool(root, { operations: createHostEditOperations(root, options), }) as unknown as AnyAgentTool; @@ -649,14 +693,31 @@ export function createOpenClawReadTool( normalized ?? (params && typeof params === "object" ? (params as Record) : undefined); assertRequiredParams(record, CLAUDE_PARAM_GROUPS.read, base.name); + const filePath = typeof record?.path === "string" ? String(record.path) : ""; + // Path-level permission check (when tools.fs.permissions is configured). + // Use the resolved path for the actual read — closes the TOCTOU window where a + // symlink swapped between check and open could redirect I/O to an unchecked path. + // This mirrors the write/edit tools which return the resolved path from + // assertWritePermitted/assertEditPermitted and use it for the subsequent I/O call. + let readArgs = (normalized ?? params ?? {}) as Record; + if (options?.permissions && filePath !== "") { + const resolvedPath = safeRealpath( + path.isAbsolute(filePath) + ? filePath + : path.resolve(options.workspaceRoot ?? process.cwd(), filePath), + ); + if (checkAccessPolicy(resolvedPath, "read", options.permissions) === "deny") { + throw new Error(`Permission denied: read access to ${resolvedPath} is not allowed.`); + } + readArgs = { ...readArgs, path: resolvedPath }; + } const result = await executeReadWithAdaptivePaging({ base, toolCallId, - args: (normalized ?? params ?? {}) as Record, + args: readArgs, signal, maxBytes: resolveAdaptiveReadMaxBytes(options), }); - const filePath = typeof record?.path === "string" ? String(record.path) : ""; const strippedDetailsResult = stripReadTruncationContentDetails(result); const normalizedResult = await normalizeReadImageResult(strippedDetailsResult, filePath); return sanitizeToolResultImages( @@ -718,32 +779,58 @@ async function writeHostFile(absolutePath: string, content: string) { await fs.writeFile(resolved, content, "utf-8"); } -function createHostWriteOperations(root: string, options?: { workspaceOnly?: boolean }) { +function createHostWriteOperations( + root: string, + options?: { workspaceOnly?: boolean; permissions?: AccessPolicyConfig }, +) { const workspaceOnly = options?.workspaceOnly ?? false; + const permissions = options?.permissions; + // Resolve root once so that safeRealpath(child) paths can be compared against + // it — if root itself is a symlink, toRelativeWorkspacePath would otherwise + // throw "path escapes workspace root" for every path inside the workspace. + const resolvedRoot = safeRealpath(root); + + // Returns the safeRealpath-resolved path so callers use the same concrete path + // for I/O that was checked by the policy — closes the TOCTOU window where a + // symlink swap between permission check and fs call could redirect I/O. + function assertWritePermitted(absolutePath: string): string { + const resolved = safeRealpath(absolutePath); + if (permissions && checkAccessPolicy(resolved, "write", permissions) === "deny") { + throw new Error(`Permission denied: write access to ${resolved} is not allowed.`); + } + return resolved; + } if (!workspaceOnly) { // When workspaceOnly is false, allow writes anywhere on the host return { mkdir: async (dir: string) => { - const resolved = path.resolve(dir); + const resolved = assertWritePermitted(dir); await fs.mkdir(resolved, { recursive: true }); }, - writeFile: writeHostFile, + writeFile: async (absolutePath: string, content: string) => { + const resolved = assertWritePermitted(absolutePath); + await writeHostFile(resolved, content); + }, } as const; } // When workspaceOnly is true, enforce workspace boundary return { mkdir: async (dir: string) => { - const relative = toRelativeWorkspacePath(root, dir, { allowRoot: true }); - const resolved = relative ? path.resolve(root, relative) : path.resolve(root); - await assertSandboxPath({ filePath: resolved, cwd: root, root }); - await fs.mkdir(resolved, { recursive: true }); + const resolved = assertWritePermitted(dir); + const relative = toRelativeWorkspacePath(resolvedRoot, resolved, { allowRoot: true }); + const absResolved = relative + ? path.resolve(resolvedRoot, relative) + : path.resolve(resolvedRoot); + await assertSandboxPath({ filePath: absResolved, cwd: resolvedRoot, root: resolvedRoot }); + await fs.mkdir(absResolved, { recursive: true }); }, writeFile: async (absolutePath: string, content: string) => { - const relative = toRelativeWorkspacePath(root, absolutePath); + const resolved = assertWritePermitted(absolutePath); + const relative = toRelativeWorkspacePath(resolvedRoot, resolved); await writeFileWithinRoot({ - rootDir: root, + rootDir: resolvedRoot, relativePath: relative, data: content, mkdir: true, @@ -752,19 +839,55 @@ function createHostWriteOperations(root: string, options?: { workspaceOnly?: boo } as const; } -function createHostEditOperations(root: string, options?: { workspaceOnly?: boolean }) { +function createHostEditOperations( + root: string, + options?: { workspaceOnly?: boolean; permissions?: AccessPolicyConfig }, +) { const workspaceOnly = options?.workspaceOnly ?? false; + const permissions = options?.permissions; + const resolvedRoot = safeRealpath(root); + + // Edit = read + write the same file; check both permissions and return the + // safeRealpath-resolved path so callers use the same concrete path for I/O + // that was checked — closes the TOCTOU window where a symlink swap between + // permission check and fs call could redirect I/O to an unchecked target. + function assertEditPermitted(absolutePath: string): string { + const resolved = safeRealpath(absolutePath); + if (permissions) { + if (checkAccessPolicy(resolved, "read", permissions) === "deny") { + throw new Error(`Permission denied: read access to ${resolved} is not allowed.`); + } + if (checkAccessPolicy(resolved, "write", permissions) === "deny") { + throw new Error(`Permission denied: write access to ${resolved} is not allowed.`); + } + } + return resolved; + } + + // access() checks existence only — requires read permission but not write. + // Using assertEditPermitted here would block existence checks on r-- paths before + // any write is attempted, producing a misleading "write access denied" error. + function assertReadPermitted(absolutePath: string): string { + const resolved = safeRealpath(absolutePath); + if (permissions && checkAccessPolicy(resolved, "read", permissions) === "deny") { + throw new Error(`Permission denied: read access to ${resolved} is not allowed.`); + } + return resolved; + } if (!workspaceOnly) { // When workspaceOnly is false, allow edits anywhere on the host return { readFile: async (absolutePath: string) => { - const resolved = path.resolve(absolutePath); + const resolved = assertEditPermitted(absolutePath); return await fs.readFile(resolved); }, - writeFile: writeHostFile, + writeFile: async (absolutePath: string, content: string) => { + const resolved = assertEditPermitted(absolutePath); + await writeHostFile(resolved, content); + }, access: async (absolutePath: string) => { - const resolved = path.resolve(absolutePath); + const resolved = assertReadPermitted(absolutePath); await fs.access(resolved); }, } as const; @@ -773,26 +896,29 @@ function createHostEditOperations(root: string, options?: { workspaceOnly?: bool // When workspaceOnly is true, enforce workspace boundary return { readFile: async (absolutePath: string) => { - const relative = toRelativeWorkspacePath(root, absolutePath); + const resolved = assertEditPermitted(absolutePath); + const relative = toRelativeWorkspacePath(resolvedRoot, resolved); const safeRead = await readFileWithinRoot({ - rootDir: root, + rootDir: resolvedRoot, relativePath: relative, }); return safeRead.buffer; }, writeFile: async (absolutePath: string, content: string) => { - const relative = toRelativeWorkspacePath(root, absolutePath); + const resolved = assertEditPermitted(absolutePath); + const relative = toRelativeWorkspacePath(resolvedRoot, resolved); await writeFileWithinRoot({ - rootDir: root, + rootDir: resolvedRoot, relativePath: relative, data: content, mkdir: true, }); }, access: async (absolutePath: string) => { + const resolved = assertReadPermitted(absolutePath); let relative: string; try { - relative = toRelativeWorkspacePath(root, absolutePath); + relative = toRelativeWorkspacePath(resolvedRoot, resolved); } catch { // Path escapes workspace root. Don't throw here – the upstream // library replaces any `access` error with a misleading "File not diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 6536e9dfbb5..dd8fbf58e1d 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -339,6 +339,7 @@ export function createOpenClawCodingTools(options?: { const fsConfig = resolveToolFsConfig({ cfg: options?.config, agentId }); const fsPolicy = createToolFsPolicy({ workspaceOnly: isMemoryFlushRun || fsConfig.workspaceOnly, + permissions: fsConfig.permissions, }); const sandboxRoot = sandbox?.workspaceDir; const sandboxFsBridge = sandbox?.fsBridge; @@ -384,6 +385,8 @@ export function createOpenClawCodingTools(options?: { const wrapped = createOpenClawReadTool(freshReadTool, { modelContextWindowTokens: options?.modelContextWindowTokens, imageSanitization, + permissions: fsPolicy.permissions, + workspaceRoot, }); return [workspaceOnly ? wrapToolWorkspaceRootGuard(wrapped, workspaceRoot) : wrapped]; } @@ -394,14 +397,20 @@ export function createOpenClawCodingTools(options?: { if (sandboxRoot) { return []; } - const wrapped = createHostWorkspaceWriteTool(workspaceRoot, { workspaceOnly }); + const wrapped = createHostWorkspaceWriteTool(workspaceRoot, { + workspaceOnly, + permissions: fsPolicy.permissions, + }); return [workspaceOnly ? wrapToolWorkspaceRootGuard(wrapped, workspaceRoot) : wrapped]; } if (tool.name === "edit") { if (sandboxRoot) { return []; } - const wrapped = createHostWorkspaceEditTool(workspaceRoot, { workspaceOnly }); + const wrapped = createHostWorkspaceEditTool(workspaceRoot, { + workspaceOnly, + permissions: fsPolicy.permissions, + }); return [workspaceOnly ? wrapToolWorkspaceRootGuard(wrapped, workspaceRoot) : wrapped]; } return [tool]; @@ -409,6 +418,7 @@ export function createOpenClawCodingTools(options?: { const { cleanupMs: cleanupMsOverride, ...execDefaults } = options?.exec ?? {}; const execTool = createExecTool({ ...execDefaults, + permissions: options?.exec?.permissions ?? fsPolicy.permissions, host: options?.exec?.host ?? execConfig.host, security: options?.exec?.security ?? execConfig.security, ask: options?.exec?.ask ?? execConfig.ask, diff --git a/src/agents/tool-fs-policy.ts b/src/agents/tool-fs-policy.ts index 59d04c56e67..91522719505 100644 --- a/src/agents/tool-fs-policy.ts +++ b/src/agents/tool-fs-policy.ts @@ -1,18 +1,26 @@ import type { OpenClawConfig } from "../config/config.js"; +import type { AccessPolicyConfig } from "../config/types.tools.js"; +import { resolveAccessPolicyForAgent } from "../infra/access-policy-file.js"; import { resolveAgentConfig } from "./agent-scope.js"; export type ToolFsPolicy = { workspaceOnly: boolean; + permissions?: AccessPolicyConfig; }; -export function createToolFsPolicy(params: { workspaceOnly?: boolean }): ToolFsPolicy { +export function createToolFsPolicy(params: { + workspaceOnly?: boolean; + permissions?: AccessPolicyConfig; +}): ToolFsPolicy { return { workspaceOnly: params.workspaceOnly === true, + permissions: params.permissions, }; } export function resolveToolFsConfig(params: { cfg?: OpenClawConfig; agentId?: string }): { workspaceOnly?: boolean; + permissions?: AccessPolicyConfig; } { const cfg = params.cfg; const globalFs = cfg?.tools?.fs; @@ -20,6 +28,7 @@ export function resolveToolFsConfig(params: { cfg?: OpenClawConfig; agentId?: st cfg && params.agentId ? resolveAgentConfig(cfg, params.agentId)?.tools?.fs : undefined; return { workspaceOnly: agentFs?.workspaceOnly ?? globalFs?.workspaceOnly, + permissions: resolveAccessPolicyForAgent(params.agentId), }; } diff --git a/src/config/types.tools.ts b/src/config/types.tools.ts index 43d39285b57..be51080216b 100644 --- a/src/config/types.tools.ts +++ b/src/config/types.tools.ts @@ -3,6 +3,39 @@ import type { SafeBinProfileFixture } from "../infra/exec-safe-bin-policy.js"; import type { AgentElevatedAllowFromConfig, SessionSendPolicyAction } from "./types.base.js"; import type { SecretInput } from "./types.secrets.js"; +/** Three-character permission string: `rwx`, `r--`, `rw-`, `---`, etc. */ +export type PermStr = string; + +/** Per-script policy entry — allows narrower permissions for a specific script binary. */ +export type ScriptPolicyEntry = { + /** Extra path rules for this script, merged over the shared script policy. */ + policy?: Record; + /** SHA-256 hex of the script file for integrity checking (best-effort, not atomic). */ + sha256?: string; +}; + +/** + * Filesystem RWX access-policy config loaded from `access-policy.json`. + * Applied per-agent to read, write, and exec tool calls. + * + * Implicit fallback when no rule matches: `"---"` (deny-all). + * To set a permissive default, add a `"/**"` rule (e.g. `"/**": "r--"`). + */ +export type AccessPolicyConfig = { + /** Glob-pattern rules: path → permission string. Longest prefix wins. */ + policy?: Record; + /** + * Per-script argv0 policy overrides keyed by resolved binary path. + * Reserved key "policy" holds shared rules applied to every script before + * per-script policy is merged in. + */ + scripts?: { + /** Shared rules applied to all scripts; per-script policy wins on collision. */ + policy?: Record; + [path: string]: ScriptPolicyEntry | Record | undefined; + }; +}; + export type MediaUnderstandingScopeMatch = { channel?: string; chatType?: ChatType; diff --git a/src/infra/access-policy-file.test.ts b/src/infra/access-policy-file.test.ts new file mode 100644 index 00000000000..273241e5d55 --- /dev/null +++ b/src/infra/access-policy-file.test.ts @@ -0,0 +1,508 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + BROKEN_POLICY_FILE, + _resetFileCacheForTest, + _resetNotFoundWarnedForTest, + loadAccessPolicyFile, + mergeAccessPolicy, + resolveAccessPolicyForAgent, + resolveAccessPolicyPath, + type AccessPolicyFile, +} from "./access-policy-file.js"; + +// --------------------------------------------------------------------------- +// Test helpers +// --------------------------------------------------------------------------- + +// The global test setup (test/setup.ts → withIsolatedTestHome) sets HOME to a +// per-worker temp directory before any tests run. os.homedir() respects HOME on +// macOS/Linux, so resolveAccessPolicyPath() resolves to +// "/.openclaw/access-policy.json" — already isolated from the real +// user home. We just need to ensure the .openclaw dir exists and clean up the +// file after each test. + +const FP_FILE = resolveAccessPolicyPath(); +const FP_DIR = path.dirname(FP_FILE); + +beforeEach(() => { + fs.mkdirSync(FP_DIR, { recursive: true }); + _resetNotFoundWarnedForTest(); + _resetFileCacheForTest(); +}); + +afterEach(() => { + // Remove the file if a test wrote it; leave the directory to avoid races. + try { + fs.unlinkSync(FP_FILE); + } catch { + /* file may not exist — that's fine */ + } +}); + +function writeFile(content: AccessPolicyFile | object) { + fs.writeFileSync(FP_FILE, JSON.stringify(content, null, 2)); +} + +// --------------------------------------------------------------------------- +// mergeAccessPolicy +// --------------------------------------------------------------------------- + +describe("mergeAccessPolicy", () => { + it("returns undefined when both are undefined", () => { + expect(mergeAccessPolicy(undefined, undefined)).toBeUndefined(); + }); + + it("returns base when override is undefined", () => { + const base = { policy: { "/**": "r--" as const } }; + expect(mergeAccessPolicy(base, undefined)).toEqual(base); + }); + + it("returns override when base is undefined", () => { + const override = { policy: { "/**": "rwx" as const } }; + expect(mergeAccessPolicy(undefined, override)).toEqual(override); + }); + + it("returns a copy when base is undefined — mutations do not corrupt the original", () => { + // mergeAccessPolicy(undefined, x) was returning x by reference. validateAccessPolicyConfig + // calls autoExpandBareDir which mutates .policy in-place, permanently corrupting the cached + // agents["*"] object for all subsequent calls in the same process. + const override = { policy: { "/tmp": "rwx" as const } }; + const result = mergeAccessPolicy(undefined, override); + // Simulate autoExpandBareDir mutating the result. + if (result?.policy) { + result.policy["/tmp/**"] = "rwx"; + delete result.policy["/tmp"]; + } + // The original override must be unchanged. + expect(override.policy).toEqual({ "/tmp": "rwx" }); + expect(override.policy["/tmp/**" as keyof typeof override.policy]).toBeUndefined(); + }); + + it("deep-copies nested scripts policy maps — mutations do not corrupt cache", () => { + // Shallow-copying scripts is not enough: scripts["policy"] and per-script entry.policy + // are nested objects that would still be references into the cached _fileCache object. + const scriptPolicy = { "/tmp/**": "rwx" as const }; + const entryPolicy = { "/data/**": "r--" as const }; + const override = { + scripts: { + policy: scriptPolicy, + "/deploy.sh": { policy: entryPolicy }, + }, + }; + const result = mergeAccessPolicy(undefined, override); + // Mutate the returned scripts["policy"] and per-script policy. + const sp = result?.scripts?.["policy"]; + if (sp) { + sp["/added/**"] = "---"; + } + const entry = result?.scripts?.["/deploy.sh"] as + | { policy?: Record } + | undefined; + if (entry?.policy) { + entry.policy["/added/**"] = "---"; + } + // Originals must be unchanged. + expect(scriptPolicy).toEqual({ "/tmp/**": "rwx" }); + expect(entryPolicy).toEqual({ "/data/**": "r--" }); + }); + + it("rules are shallow-merged, override key wins on collision", () => { + const result = mergeAccessPolicy( + { policy: { "/**": "r--", "~/**": "rw-" } }, + { policy: { "~/**": "rwx", "~/dev/**": "rwx" } }, + ); + expect(result?.policy?.["/**"]).toBe("r--"); // base survives + expect(result?.policy?.["~/**"]).toBe("rwx"); // override wins + expect(result?.policy?.["~/dev/**"]).toBe("rwx"); // override adds + }); + + it("omits empty rules from result", () => { + const result = mergeAccessPolicy({ scripts: { "/s.sh": { sha256: "abc" } } }, {}); + expect(result?.policy).toBeUndefined(); + }); + + it("scripts deep-merge: base sha256 is preserved when override supplies same script key", () => { + // Security regression: a shallow spread ({ ...base.scripts, ...override.scripts }) would + // silently drop the admin-configured sha256 hash check, defeating integrity enforcement. + const base = { + scripts: { + "/usr/local/bin/deploy.sh": { + sha256: "abc123", + policy: { "~/deploy/**": "rwx" as const }, + }, + }, + }; + const override = { + scripts: { + "/usr/local/bin/deploy.sh": { + // Agent block supplies same key — must NOT be able to drop sha256. + policy: { "~/deploy/**": "r--" as const }, // narrower override — fine + }, + }, + }; + const result = mergeAccessPolicy(base, override); + const merged = result?.scripts?.["/usr/local/bin/deploy.sh"] as + | import("../config/types.tools.js").ScriptPolicyEntry + | undefined; + // sha256 from base must survive. + expect(merged?.sha256).toBe("abc123"); + // policy: override key wins on collision. + expect(merged?.policy?.["~/deploy/**"]).toBe("r--"); + }); + + it("scripts deep-merge: override-only script key is added verbatim", () => { + const base = { scripts: { "/bin/existing.sh": { sha256: "deadbeef" } } }; + const override = { + scripts: { "/bin/new.sh": { policy: { "/tmp/**": "rwx" as const } } }, + }; + const result = mergeAccessPolicy(base, override); + // Base script untouched. + expect(result?.scripts?.["/bin/existing.sh"]?.sha256).toBe("deadbeef"); + // New script from override is added. + const newScript = result?.scripts?.["/bin/new.sh"] as + | import("../config/types.tools.js").ScriptPolicyEntry + | undefined; + expect(newScript?.policy?.["/tmp/**"]).toBe("rwx"); + }); +}); + +// --------------------------------------------------------------------------- +// loadAccessPolicyFile +// --------------------------------------------------------------------------- + +describe("loadAccessPolicyFile", () => { + it("returns null when file does not exist", () => { + expect(loadAccessPolicyFile()).toBeNull(); + }); + + it("returns BROKEN_POLICY_FILE and logs error when file is invalid JSON", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + const p = resolveAccessPolicyPath(); + fs.writeFileSync(p, "not json {{ broken"); + const result = loadAccessPolicyFile(); + expect(result).toBe(BROKEN_POLICY_FILE); + expect(spy).toHaveBeenCalledWith(expect.stringContaining("Cannot parse")); + expect(spy).toHaveBeenCalledWith(expect.stringContaining("Failing closed")); + spy.mockRestore(); + }); + + it("returns BROKEN_POLICY_FILE and logs error when version is not 1", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + writeFile({ version: 2, base: {} }); + const result = loadAccessPolicyFile(); + expect(result).toBe(BROKEN_POLICY_FILE); + expect(spy).toHaveBeenCalledWith(expect.stringContaining("unsupported version")); + expect(spy).toHaveBeenCalledWith(expect.stringContaining("Failing closed")); + spy.mockRestore(); + }); + + it('returns BROKEN_POLICY_FILE and logs error when base is placed at top level (use agents["*"])', () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + writeFile({ version: 1, base: { policy: { "/**": "r--" } } }); + const result = loadAccessPolicyFile(); + expect(result).toBe(BROKEN_POLICY_FILE); + expect(spy).toHaveBeenCalledWith(expect.stringContaining('unexpected top-level key "base"')); + spy.mockRestore(); + }); + + it("returns BROKEN_POLICY_FILE and logs error when agents is not an object", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + writeFile({ version: 1, agents: "bad" }); + const result = loadAccessPolicyFile(); + expect(result).toBe(BROKEN_POLICY_FILE); + expect(spy).toHaveBeenCalledWith(expect.stringContaining('"agents" must be an object')); + spy.mockRestore(); + }); + + it("returns BROKEN_POLICY_FILE and logs error when a top-level key like 'policy' is misplaced", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + // Common mistake: policy at top level instead of under agents["*"] + writeFile({ version: 1, policy: { "/**": "r--" } }); + const result = loadAccessPolicyFile(); + expect(result).toBe(BROKEN_POLICY_FILE); + expect(spy).toHaveBeenCalledWith(expect.stringContaining('unexpected top-level key "policy"')); + spy.mockRestore(); + }); + + it("returns BROKEN_POLICY_FILE and logs error when 'scripts' is misplaced at top level", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + writeFile({ version: 1, scripts: { "/bin/s.sh": { sha256: "abc" } } }); + const result = loadAccessPolicyFile(); + expect(result).toBe(BROKEN_POLICY_FILE); + expect(spy).toHaveBeenCalledWith(expect.stringContaining('unexpected top-level key "scripts"')); + spy.mockRestore(); + }); + + it("returns BROKEN_POLICY_FILE and logs error when an agent block is not an object", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + writeFile({ version: 1, agents: { subri: "rwx" } }); + const result = loadAccessPolicyFile(); + expect(result).toBe(BROKEN_POLICY_FILE); + expect(spy).toHaveBeenCalledWith(expect.stringContaining('agents["subri"] must be an object')); + spy.mockRestore(); + }); + + it('returns BROKEN_POLICY_FILE when scripts["policy"] is a primitive (not an object)', () => { + // scripts["policy"] must be a Record; a primitive like `true` + // silently passes structural validation and is treated as an empty shared policy. + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + writeFile({ + version: 1, + agents: { "*": { scripts: { policy: true as unknown as Record } } }, + }); + const result = loadAccessPolicyFile(); + expect(result).toBe(BROKEN_POLICY_FILE); + expect(spy).toHaveBeenCalledWith( + expect.stringContaining('scripts["policy"] must be an object'), + ); + spy.mockRestore(); + }); + + it("returns parsed file when valid", () => { + const content: AccessPolicyFile = { + version: 1, + agents: { + "*": { policy: { "/**": "r--", "~/.ssh/**": "---" } }, + subri: { policy: { "~/dev/**": "rwx" } }, + }, + }; + writeFile(content); + const result = loadAccessPolicyFile(); + expect(result).not.toBe(BROKEN_POLICY_FILE); + expect(result).not.toBeNull(); + if (result === null || result === BROKEN_POLICY_FILE) { + throw new Error("unexpected"); + } + expect(result.version).toBe(1); + expect(result.agents?.["*"]?.policy?.["/**"]).toBe("r--"); + expect(result.agents?.subri?.policy?.["~/dev/**"]).toBe("rwx"); + }); +}); + +// --------------------------------------------------------------------------- +// loadAccessPolicyFile — mtime cache +// --------------------------------------------------------------------------- + +describe("loadAccessPolicyFile — mtime cache", () => { + it("returns cached result on second call without re-reading the file", () => { + writeFile({ version: 1, agents: { "*": { policy: { "/**": "r--" } } } }); + const spy = vi.spyOn(fs, "readFileSync"); + loadAccessPolicyFile(); // populate cache + loadAccessPolicyFile(); // should hit cache + // readFileSync should only be called once despite two loadAccessPolicyFile calls. + expect(spy.mock.calls.filter((c) => String(c[0]).includes("access-policy")).length).toBe(1); + spy.mockRestore(); + }); + + it("re-reads when mtime changes (file updated)", () => { + writeFile({ version: 1, agents: { "*": { policy: { "/**": "r--" } } } }); + loadAccessPolicyFile(); // populate cache + // Rewrite the file — on most filesystems this bumps mtime. Force a detectable + // mtime change by setting it explicitly via utimesSync. + writeFile({ version: 1, agents: { "*": { policy: { "/**": "rwx" } } } }); + const future = Date.now() / 1000 + 1; + fs.utimesSync(FP_FILE, future, future); + const result = loadAccessPolicyFile(); + expect(result).not.toBe(BROKEN_POLICY_FILE); + if (result === null || result === BROKEN_POLICY_FILE) { + throw new Error("unexpected"); + } + expect(result.agents?.["*"]?.policy?.["/**"]).toBe("rwx"); + }); + + it("clears cache when file is deleted", () => { + writeFile({ version: 1, agents: { "*": { policy: { "/**": "r--" } } } }); + loadAccessPolicyFile(); // populate cache + fs.unlinkSync(FP_FILE); + expect(loadAccessPolicyFile()).toBeNull(); + }); + + it("caches BROKEN_POLICY_FILE result for broken files", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + fs.writeFileSync(FP_FILE, "not json {{ broken"); + loadAccessPolicyFile(); // populate cache with BROKEN + const spy2 = vi.spyOn(fs, "readFileSync"); + const result = loadAccessPolicyFile(); // should hit cache + expect(result).toBe(BROKEN_POLICY_FILE); + expect(spy2.mock.calls.filter((c) => String(c[0]).includes("access-policy")).length).toBe(0); + spy.mockRestore(); + spy2.mockRestore(); + }); +}); + +// --------------------------------------------------------------------------- +// resolveAccessPolicyForAgent +// --------------------------------------------------------------------------- + +describe("resolveAccessPolicyForAgent", () => { + it("returns undefined when file does not exist", () => { + expect(resolveAccessPolicyForAgent("subri")).toBeUndefined(); + }); + + it("does not warn when config file is not found (feature is opt-in)", () => { + // access-policy is opt-in; absence of the file is the normal state and + // must not produce console noise for users who have not configured it. + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + resolveAccessPolicyForAgent("subri"); + resolveAccessPolicyForAgent("subri"); + expect(warnSpy).not.toHaveBeenCalled(); + warnSpy.mockRestore(); + }); + + it("does not warn when config file exists and is valid", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + writeFile({ version: 1, agents: { "*": { policy: { "/**": "r--" } } } }); + resolveAccessPolicyForAgent("subri"); + expect(warnSpy).not.toHaveBeenCalled(); + warnSpy.mockRestore(); + }); + + it("returns deny-all and logs error when config file is broken (fail-closed)", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + writeFile({ version: 1, policy: { "/**": "r--" } }); // misplaced key — triggers error + const result = resolveAccessPolicyForAgent("subri"); + expect(warnSpy).not.toHaveBeenCalled(); + expect(errSpy).toHaveBeenCalledWith(expect.stringContaining("Failing closed")); + // Broken file must fail-closed: deny-all policy (empty rules = implicit "---"), not undefined + expect(result).toEqual({}); + warnSpy.mockRestore(); + errSpy.mockRestore(); + }); + + it("deny-all policy returned on broken file is frozen — mutation does not corrupt future calls", () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + writeFile({ version: 1, policy: { "/**": "r--" } }); // misplaced key — broken + const result = resolveAccessPolicyForAgent("subri"); + expect(result).toEqual({}); + // Attempt to mutate the returned object — must not affect the next call. + // If DENY_ALL_POLICY is not frozen this would silently corrupt it. + try { + (result as Record)["rules"] = { "/**": "rwx" }; + } catch { + // Object.freeze throws in strict mode — that's fine too. + } + _resetNotFoundWarnedForTest(); + const result2 = resolveAccessPolicyForAgent("subri"); + expect(result2).toEqual({}); + errSpy.mockRestore(); + }); + + it("returns base when no agent block exists", () => { + writeFile({ + version: 1, + agents: { "*": { policy: { "/**": "r--", [`~/.ssh/**`]: "---" } } }, + }); + const result = resolveAccessPolicyForAgent("subri"); + expect(result?.policy?.["/**"]).toBe("r--"); + expect(result?.policy?.["~/.ssh/**"]).toBe("---"); + }); + + it("merges base + named agent", () => { + writeFile({ + version: 1, + agents: { + "*": { policy: { "/**": "r--", [`~/.ssh/**`]: "---" } }, + subri: { policy: { "~/dev/**": "rwx" } }, + }, + }); + const result = resolveAccessPolicyForAgent("subri"); + // policy: merged, agent rule wins on collision + expect(result?.policy?.["/**"]).toBe("r--"); + expect(result?.policy?.["~/dev/**"]).toBe("rwx"); + // base "---" rule preserved + expect(result?.policy?.["~/.ssh/**"]).toBe("---"); + }); + + it("wildcard agent applies before named agent", () => { + writeFile({ + version: 1, + agents: { + "*": { policy: { "/usr/bin/**": "r-x" } }, + subri: { policy: { "~/dev/**": "rwx" } }, + }, + }); + const result = resolveAccessPolicyForAgent("subri"); + expect(result?.policy?.["/usr/bin/**"]).toBe("r-x"); // from wildcard + expect(result?.policy?.["~/dev/**"]).toBe("rwx"); // from named agent + }); + + it("wildcard applies even when no named agent block", () => { + writeFile({ + version: 1, + agents: { "*": { policy: { [`~/.ssh/**`]: "---" } } }, + }); + const result = resolveAccessPolicyForAgent("other-agent"); + expect(result?.policy?.["~/.ssh/**"]).toBe("---"); + }); + + it("wildcard key itself is not treated as a named agent", () => { + writeFile({ + version: 1, + agents: { "*": { policy: { [`~/.ssh/**`]: "---" } } }, + }); + // Requesting agentId "*" should not double-apply wildcard as named + const result = resolveAccessPolicyForAgent("*"); + expect(result?.policy?.["~/.ssh/**"]).toBe("---"); + }); + + it("returns undefined when file is empty (no base, no agents)", () => { + writeFile({ version: 1 }); + // No base and no agents → nothing to merge → undefined + expect(resolveAccessPolicyForAgent("subri")).toBeUndefined(); + }); + + it("logs console.error (not warn) when perm string is invalid", () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + writeFile({ + version: 1, + agents: { "*": { policy: { "/**": "BAD" } } }, + }); + resolveAccessPolicyForAgent("subri"); + expect(errSpy).toHaveBeenCalledWith(expect.stringContaining("BAD")); + expect(warnSpy).not.toHaveBeenCalled(); + errSpy.mockRestore(); + warnSpy.mockRestore(); + }); + + it("does not print 'Bad permission strings' footer when only auto-expand diagnostics are present", () => { + // Greptile: footer was printed after auto-expand messages ("rule auto-expanded to ..."), + // misleading operators into thinking their policy was broken when it was fine. + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + // Write a file whose rules entry is a bare directory — triggers auto-expand diagnostic + // but no real perm-string error. + const dir = os.tmpdir(); + writeFile({ version: 1, agents: { "*": { policy: { [dir]: "r--" } } } }); + resolveAccessPolicyForAgent("subri"); + const calls = errSpy.mock.calls.map((c) => String(c[0])); + expect(calls.some((m) => m.includes("auto-expanded"))).toBe(true); + expect(calls.some((m) => m.includes("Bad permission strings"))).toBe(false); + errSpy.mockRestore(); + }); + + it("prints 'Bad permission strings' footer when a real perm-string error is present", () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + writeFile({ version: 1, agents: { "*": { policy: { "/**": "BAD" } } } }); + resolveAccessPolicyForAgent("subri"); + const calls = errSpy.mock.calls.map((c) => String(c[0])); + expect(calls.some((m) => m.includes("Bad permission strings"))).toBe(true); + errSpy.mockRestore(); + }); + + it("narrowing rules from base and agent are all preserved in merged result", () => { + writeFile({ + version: 1, + agents: { + "*": { policy: { [`~/.ssh/**`]: "---" } }, + paranoid: { policy: { [`~/.aws/**`]: "---" } }, + }, + }); + const result = resolveAccessPolicyForAgent("paranoid"); + expect(result?.policy?.["~/.ssh/**"]).toBe("---"); + expect(result?.policy?.["~/.aws/**"]).toBe("---"); + }); +}); diff --git a/src/infra/access-policy-file.ts b/src/infra/access-policy-file.ts new file mode 100644 index 00000000000..ff91897d241 --- /dev/null +++ b/src/infra/access-policy-file.ts @@ -0,0 +1,376 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import type { AccessPolicyConfig } from "../config/types.tools.js"; +import { validateAccessPolicyConfig } from "./access-policy.js"; + +export type AccessPolicyFile = { + version: 1; + /** + * Per-agent overrides keyed by agent ID. + * + * Reserved key: + * "*" — base policy applied to every agent before the named agent block is merged in. + * + * Merge order (each layer wins over the previous): + * agents["*"] → agents[agentId] + * + * Within each layer: + * - policy: shallow-merge, override key wins on collision + * - scripts: deep-merge per key; base sha256 is preserved + */ + agents?: Record; +}; + +// Use os.homedir() directly — NOT expandHomePrefix — so that OPENCLAW_HOME +// (which points at ~/.openclaw, the data dir) does not produce a double-nested +// path like ~/.openclaw/.openclaw/access-policy.json. +export function resolveAccessPolicyPath(): string { + return path.join(os.homedir(), ".openclaw", "access-policy.json"); +} + +/** + * Merge two AccessPolicyConfig layers. + * - rules: shallow merge, override key wins + * - scripts: deep-merge per key; base sha256 is preserved (cannot be removed by override) + */ +export function mergeAccessPolicy( + base: AccessPolicyConfig | undefined, + override: AccessPolicyConfig | undefined, +): AccessPolicyConfig | undefined { + if (!base && !override) { + return undefined; + } + if (!base) { + // Return a shallow copy so that validateAccessPolicyConfig → autoExpandBareDir + // does not mutate the cached agents["*"] object in _fileCache. Without this, + // the first call permanently corrupts policy entries for all subsequent calls + // in the same process. + // Deep-copy nested policy records so validateAccessPolicyConfig → autoExpandBareDir + // cannot mutate the cached _fileCache object. Shallow-copying `scripts` is not enough: + // `scripts["policy"]` (shared rules map) and per-script `entry.policy` maps are nested + // objects that would still be references into the cache. + const overrideScripts = override.scripts; + const scriptsCopy: AccessPolicyConfig["scripts"] | undefined = overrideScripts + ? Object.fromEntries( + Object.entries(overrideScripts).map(([k, v]) => { + if (k === "policy") { + // scripts["policy"] is a Record — shallow copy is sufficient. + return [k, v != null && typeof v === "object" ? { ...v } : v]; + } + // Per-script entries: copy the entry and its nested policy map. + if (v != null && typeof v === "object" && !Array.isArray(v)) { + const entry = v as import("../config/types.tools.js").ScriptPolicyEntry; + return [k, { ...entry, policy: entry.policy ? { ...entry.policy } : undefined }]; + } + return [k, v]; + }), + ) + : undefined; + return { + ...override, + policy: override.policy ? { ...override.policy } : undefined, + scripts: scriptsCopy, + }; + } + if (!override) { + return base; + } + const rules = { ...base.policy, ...override.policy }; + // scripts: deep-merge per key — base sha256 is preserved regardless of + // what the agent override supplies. A plain spread ({ ...base.scripts, ...override.scripts }) + // would silently drop the admin-configured hash integrity check when an agent block + // supplies the same script key, defeating the security intent. + const mergedScripts: NonNullable = { ...base.scripts }; + for (const [key, overrideEntry] of Object.entries(override.scripts ?? {})) { + if (key === "policy") { + // "policy" holds shared rules (Record) — merge as flat rules, + // override key wins. No sha256 preservation applies here. + mergedScripts["policy"] = { ...base.scripts?.["policy"], ...overrideEntry } as Record< + string, + string + >; + continue; + } + const baseEntry = base.scripts?.[key] as + | import("../config/types.tools.js").ScriptPolicyEntry + | undefined; + // Reject non-object entries — a primitive (true, "rwx") is not a valid ScriptPolicyEntry. + // validateAccessPolicyConfig also catches this, but the merge layer must not propagate it. + if ( + overrideEntry == null || + typeof overrideEntry !== "object" || + Array.isArray(overrideEntry) + ) { + continue; + } + const overrideScriptEntry = + overrideEntry as import("../config/types.tools.js").ScriptPolicyEntry; + if (!baseEntry) { + mergedScripts[key] = overrideScriptEntry; + continue; + } + mergedScripts[key] = { + // sha256: base always wins — cannot be removed or replaced by an agent override. + ...(baseEntry.sha256 !== undefined ? { sha256: baseEntry.sha256 } : {}), + // policy: shallow-merge, override key wins on collision. + ...(Object.keys({ ...baseEntry.policy, ...overrideScriptEntry.policy }).length > 0 + ? { policy: { ...baseEntry.policy, ...overrideScriptEntry.policy } } + : {}), + }; + } + const scripts = Object.keys(mergedScripts).length > 0 ? mergedScripts : undefined; + const result: AccessPolicyConfig = {}; + if (Object.keys(rules).length > 0) { + result.policy = rules; + } + if (scripts) { + result.scripts = scripts; + } + return result; +} + +/** + * Validate the top-level structure of a parsed access-policy file. + * Returns an array of error strings; empty = valid. + */ +function validateAccessPolicyFileStructure(filePath: string, parsed: unknown): string[] { + const errors: string[] = []; + const p = parsed as Record; + + // Removed fields: "deny" and "default" were dropped in favour of "---" rules. + // A user who configures these fields would receive no protection because the + // fields are silently discarded. Reject them explicitly so the file fails-closed. + const REMOVED_KEYS = ["deny", "default"] as const; + + function checkRemovedKeys(block: Record, context: string): void { + for (const key of REMOVED_KEYS) { + if (block[key] !== undefined) { + errors.push( + `${filePath}: ${context} "${key}" is no longer supported — use "---" rules instead (e.g. "~/.ssh/**": "---"). Failing closed until removed.`, + ); + } + } + } + + if (p["agents"] !== undefined) { + if (typeof p["agents"] !== "object" || p["agents"] === null || Array.isArray(p["agents"])) { + errors.push(`${filePath}: "agents" must be an object`); + } else { + for (const [agentId, block] of Object.entries(p["agents"] as Record)) { + if (typeof block !== "object" || block === null || Array.isArray(block)) { + errors.push(`${filePath}: agents["${agentId}"] must be an object`); + } else { + const agentBlock = block as Record; + checkRemovedKeys(agentBlock, `agents["${agentId}"]`); + // Recurse into per-script entries so old "deny"/"default" fields are caught there too. + const scripts = agentBlock["scripts"]; + if (scripts != null && typeof scripts === "object" && !Array.isArray(scripts)) { + for (const [scriptKey, scriptEntry] of Object.entries( + scripts as Record, + )) { + if (scriptKey === "policy") { + // scripts["policy"] must be an object (Record), not a primitive. + if ( + scriptEntry != null && + (typeof scriptEntry !== "object" || Array.isArray(scriptEntry)) + ) { + errors.push( + `${filePath}: agents["${agentId}"].scripts["policy"] must be an object (Record), got ${JSON.stringify(scriptEntry)}`, + ); + } + } else if ( + scriptEntry != null && + typeof scriptEntry === "object" && + !Array.isArray(scriptEntry) + ) { + checkRemovedKeys( + scriptEntry as Record, + `agents["${agentId}"].scripts["${scriptKey}"]`, + ); + } + } + } + } + } + } + } + + // Catch common mistakes: keys placed at the top level that belong inside agents["*"]. + // "base" and "policy" were old top-level fields; "rules"/"scripts" are often misplaced here too. + for (const key of ["base", "policy", "rules", "scripts"] as const) { + if (p[key] !== undefined) { + errors.push( + `${filePath}: unexpected top-level key "${key}" — did you mean to put it under agents["*"]?`, + ); + } + } + + return errors; +} + +/** + * Sentinel returned by loadAccessPolicyFile when the file exists but is broken. + * Callers must treat this as a deny-all policy (default:"---") rather than + * disabling enforcement — a corrupted file should fail-closed, not fail-open. + */ +export const BROKEN_POLICY_FILE = Symbol("broken-policy-file"); + +// In-process mtime cache — avoids re-parsing the file on every agent turn. +// Cache is keyed by mtime so any write to the file automatically invalidates it. +type _FileCacheEntry = { + mtimeMs: number; + result: AccessPolicyFile | typeof BROKEN_POLICY_FILE; +}; +let _fileCache: _FileCacheEntry | undefined; + +/** Reset the in-process file cache. Only for use in tests. */ +export function _resetFileCacheForTest(): void { + _fileCache = undefined; +} + +/** + * Read and parse the sidecar file. + * - Returns null if the file does not exist (opt-in not configured). + * - Returns BROKEN_POLICY_FILE if the file exists but is malformed/unreadable + * (callers must treat this as default:"---" — fail-closed). + * - Returns the parsed file on success. + * + * Result is cached in-process by mtime — re-parses only when the file changes. + */ +export function loadAccessPolicyFile(): AccessPolicyFile | null | typeof BROKEN_POLICY_FILE { + const filePath = resolveAccessPolicyPath(); + + // Single statSync per call: cheap enough and detects file changes. + let mtimeMs: number; + try { + mtimeMs = fs.statSync(filePath).mtimeMs; + } catch { + // File does not exist — clear cache and return null (feature is opt-in). + _fileCache = undefined; + return null; + } + + // Cache hit: same mtime means same content — skip readFileSync + JSON.parse. + if (_fileCache && _fileCache.mtimeMs === mtimeMs) { + return _fileCache.result; + } + + // Cache miss: parse fresh and store result (including BROKEN_POLICY_FILE). + const result = _parseAccessPolicyFile(filePath); + _fileCache = { mtimeMs, result: result ?? BROKEN_POLICY_FILE }; + // _parseAccessPolicyFile returns null only for the non-existent case, which we + // handle above via statSync. If it somehow returns null here treat as broken. + return result ?? BROKEN_POLICY_FILE; +} + +function _parseAccessPolicyFile( + filePath: string, +): AccessPolicyFile | typeof BROKEN_POLICY_FILE | null { + let parsed: unknown; + try { + const raw = fs.readFileSync(filePath, "utf8"); + parsed = JSON.parse(raw); + } catch (err) { + console.error( + `[access-policy] Cannot parse ${filePath}: ${err instanceof Error ? err.message : String(err)}`, + ); + console.error(`[access-policy] Failing closed (default: "---") until the file is fixed.`); + return BROKEN_POLICY_FILE; + } + + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + console.error(`[access-policy] ${filePath}: must be a JSON object at the top level.`); + console.error(`[access-policy] Failing closed (default: "---") until the file is fixed.`); + return BROKEN_POLICY_FILE; + } + + const p = parsed as Record; + if (p["version"] !== 1) { + console.error( + `[access-policy] ${filePath}: unsupported version ${JSON.stringify(p["version"])} (expected 1).`, + ); + console.error(`[access-policy] Failing closed (default: "---") until the file is fixed.`); + return BROKEN_POLICY_FILE; + } + + // Structural validation — catches wrong nesting, misplaced keys, etc. + const structErrors = validateAccessPolicyFileStructure(filePath, parsed); + if (structErrors.length > 0) { + for (const err of structErrors) { + console.error(`[access-policy] ${err}`); + } + console.error(`[access-policy] Failing closed (default: "---") until the file is fixed.`); + return BROKEN_POLICY_FILE; + } + + return parsed as AccessPolicyFile; +} + +// Suppress repeated validation error spam — resolveAccessPolicyForAgent is called +// on every agent turn; a single bad perm string would otherwise flood stderr. +// Keyed by agentId (or "__default__") so each agent's errors are shown once, +// rather than a global flag that silently swallows errors for all agents after the first. +const _validationErrorsWarnedFor = new Set(); + +/** Reset the one-time warning flags. Only for use in tests. */ +export function _resetNotFoundWarnedForTest(): void { + _validationErrorsWarnedFor.clear(); +} + +/** + * Resolve the effective AccessPolicyConfig for a given agent. + * + * Merge order: agents["*"] → agents[agentId] + * + * Returns undefined when no sidecar file exists (no-op — all operations pass through). + * Logs errors on invalid perm strings but does not throw — bad strings fall back to + * deny-all for that entry (handled downstream by checkAccessPolicy's permAllows logic). + */ +/** Deny-all policy returned when the policy file is present but broken (fail-closed). + * Empty rules + implicit "---" fallback = deny everything. */ +const DENY_ALL_POLICY: AccessPolicyConfig = Object.freeze({}); + +export function resolveAccessPolicyForAgent(agentId?: string): AccessPolicyConfig | undefined { + const file = loadAccessPolicyFile(); + if (file === BROKEN_POLICY_FILE) { + // File exists but is malformed — fail-closed: deny everything until fixed. + return DENY_ALL_POLICY; + } + if (!file) { + // access-policy.json is entirely opt-in — silently return undefined when the + // file is absent so users who have not configured the feature see no noise. + return undefined; + } + + // agents["*"] is the base — applies to every agent before the named block. + let merged = mergeAccessPolicy(undefined, file.agents?.["*"]); + if (agentId && agentId !== "*") { + const agentBlock = file.agents?.[agentId]; + if (agentBlock) { + merged = mergeAccessPolicy(merged, agentBlock); + } + } + + if (merged) { + const errors = validateAccessPolicyConfig(merged); + const dedupeKey = agentId ?? "__default__"; + if (errors.length > 0 && !_validationErrorsWarnedFor.has(dedupeKey)) { + _validationErrorsWarnedFor.add(dedupeKey); + const filePath = resolveAccessPolicyPath(); + for (const err of errors) { + console.error(`[access-policy] ${filePath}: ${err}`); + } + // Only print the footer when there are real permission-string errors — + // auto-expand diagnostics ("rule auto-expanded to ...") are informational + // and the footer would mislead operators into thinking the policy is broken. + if (errors.some((e) => !e.includes("auto-expanded") && !e.includes("mid-path wildcard"))) { + console.error( + `[access-policy] Bad permission strings are treated as "---" (deny all) at the tool layer and OS sandbox layer.`, + ); + } + } + } + + return merged; +} diff --git a/src/infra/access-policy.test.ts b/src/infra/access-policy.test.ts new file mode 100644 index 00000000000..ea8d88fce04 --- /dev/null +++ b/src/infra/access-policy.test.ts @@ -0,0 +1,1238 @@ +import crypto from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { beforeEach, describe, expect, it } from "vitest"; +import type { AccessPolicyConfig, ScriptPolicyEntry } from "../config/types.tools.js"; +import { + _resetAutoExpandedWarnedForTest, + _resetMidPathWildcardWarnedForTest, + applyScriptPolicyOverride, + checkAccessPolicy, + findBestRule, + resolveArgv0, + resolveScriptKey, + validateAccessPolicyConfig, +} from "./access-policy.js"; + +// Use os.homedir() directly — consistent with how access-policy expands ~. +// Do NOT use expandHomePrefix() here: OPENCLAW_HOME in the test environment +// would redirect ~ to the OpenClaw config dir, which is not what ~ means +// in filesystem permission patterns. +const HOME = os.homedir(); + +// --------------------------------------------------------------------------- +// validateAccessPolicyConfig +// --------------------------------------------------------------------------- + +describe("validateAccessPolicyConfig", () => { + beforeEach(() => { + _resetAutoExpandedWarnedForTest(); + _resetMidPathWildcardWarnedForTest(); + }); + + it("returns no errors for a valid config", () => { + expect( + validateAccessPolicyConfig({ + policy: { "/**": "r--", [`${HOME}/**`]: "rwx", [`${HOME}/.ssh/**`]: "---" }, + }), + ).toEqual([]); + }); + + it("returns no errors for an empty config", () => { + expect(validateAccessPolicyConfig({})).toEqual([]); + }); + + it("rejects invalid rule perm value", () => { + const errs = validateAccessPolicyConfig({ policy: { "/**": "rx" } }); + expect(errs).toHaveLength(1); + expect(errs[0]).toMatch(/policy/); + }); + + it("rejects rule perm value with wrong char in w position", () => { + const errs = validateAccessPolicyConfig({ policy: { "/**": "r1x" } }); + expect(errs).toHaveLength(1); + expect(errs[0]).toMatch(/policy/); + }); + + it("reports an error when a rule perm value is invalid", () => { + const errs = validateAccessPolicyConfig({ policy: { "/**": "xyz" } }); + expect(errs.length).toBeGreaterThanOrEqual(1); + }); + + it("file-specific '---' rule blocks access via checkAccessPolicy", () => { + // A "---" rule on a specific file path must block reads at the tool layer. + const file = process.execPath; + const config: AccessPolicyConfig = { + policy: { "/**": "rwx", [file]: "---" }, + }; + validateAccessPolicyConfig(config); // applies normalization in-place + expect(checkAccessPolicy(file, "read", config)).toBe("deny"); + }); + + it("rejects non-object script entries (e.g. a bare string or boolean)", () => { + // A primitive entry like "/deploy.sh": "rwx" or "/deploy.sh": true would bypass + // the exec gate — validateAccessPolicyConfig must reject it at load time. + const config: AccessPolicyConfig = { + scripts: { + "/deploy.sh": "rwx" as unknown as import("../config/types.tools.js").ScriptPolicyEntry, + }, + }; + const errs = validateAccessPolicyConfig(config); + expect(errs.some((e) => e.includes("/deploy.sh") && e.includes("must be an object"))).toBe( + true, + ); + }); + + it("rejects a sha256 value with wrong length", () => { + const config: AccessPolicyConfig = { + scripts: { + "/deploy.sh": { sha256: "abc123" }, + }, + }; + const errs = validateAccessPolicyConfig(config); + expect(errs.some((e) => e.includes("sha256") && e.includes("64-character"))).toBe(true); + }); + + it("rejects a sha256 value with non-hex characters", () => { + const config: AccessPolicyConfig = { + scripts: { + "/deploy.sh": { sha256: "z".repeat(64) }, + }, + }; + const errs = validateAccessPolicyConfig(config); + expect(errs.some((e) => e.includes("sha256") && e.includes("64-character"))).toBe(true); + }); + + it("accepts a valid 64-char hex sha256", () => { + const config: AccessPolicyConfig = { + scripts: { + "/deploy.sh": { sha256: "a".repeat(64) }, + }, + }; + expect(validateAccessPolicyConfig(config)).toEqual([]); + }); + + it("emits mid-path wildcard diagnostic for scripts['policy'] entries", () => { + const config: AccessPolicyConfig = { + scripts: { + policy: { "/home/*/workspace/**": "r--" }, + }, + }; + const errs = validateAccessPolicyConfig(config); + expect( + errs.some((e) => e.includes("mid-path wildcard") && e.includes('scripts["policy"]')), + ).toBe(true); + }); + + it('emits "---"-specific mid-path wildcard diagnostic for scripts["policy"] deny rules', () => { + // "---" with a mid-path wildcard cannot be enforced at the OS layer — + // the diagnostic must say "OS-level enforcement cannot apply", not the generic prefix-match message. + const config: AccessPolicyConfig = { + scripts: { + policy: { "/home/*/secrets/**": "---" }, + }, + }; + const errs = validateAccessPolicyConfig(config); + expect( + errs.some( + (e) => + e.includes("OS-level") && e.includes("cannot apply") && e.includes('scripts["policy"]'), + ), + ).toBe(true); + }); + + it("emits mid-path wildcard diagnostic for per-script policy entries", () => { + const config: AccessPolicyConfig = { + scripts: { + "/deploy.sh": { policy: { "/home/*/workspace/**": "r--" } }, + }, + }; + const errs = validateAccessPolicyConfig(config); + expect(errs.some((e) => e.includes("mid-path wildcard") && e.includes("/deploy.sh"))).toBe( + true, + ); + }); + + it('emits "---"-specific mid-path wildcard diagnostic for per-script deny rules', () => { + // Same as scripts["policy"] — per-script "---" mid-path must get the stronger warning. + const config: AccessPolicyConfig = { + scripts: { + "/deploy.sh": { policy: { "/home/*/secrets/**": "---" } }, + }, + }; + const errs = validateAccessPolicyConfig(config); + expect( + errs.some( + (e) => e.includes("OS-level") && e.includes("cannot apply") && e.includes("/deploy.sh"), + ), + ).toBe(true); + }); + + it("validates scripts[].policy perm strings and emits diagnostics for bad ones", () => { + // A typo like "rwX" in a script's policy must produce a diagnostic, not silently + // fail closed (which would deny exec with no operator-visible error). + const config: AccessPolicyConfig = { + scripts: { + "/usr/local/bin/deploy.sh": { + policy: { "~/deploy/**": "rwX" }, // invalid: uppercase X + }, + }, + }; + const errs = validateAccessPolicyConfig(config); + expect(errs.some((e) => e.includes("rwX") && e.includes("scripts"))).toBe(true); + }); + + it("accepts valid rule perm strings", () => { + expect(validateAccessPolicyConfig({ policy: { "/**": "rwx" } })).toEqual([]); + expect(validateAccessPolicyConfig({ policy: { "/**": "---" } })).toEqual([]); + expect(validateAccessPolicyConfig({ policy: { "/**": "r-x" } })).toEqual([]); + }); + + it("auto-expands a bare path that points to a real directory", () => { + // os.tmpdir() is guaranteed to exist and be a directory on every platform. + const dir = os.tmpdir(); + const config: AccessPolicyConfig = { policy: { [dir]: "r--" as const } }; + const errs = validateAccessPolicyConfig(config); + expect(errs).toHaveLength(1); + expect(errs[0]).toMatch(/auto-expanded/); + // Rule should be rewritten in place with /** suffix. + expect(config.policy?.[`${dir}/**`]).toBe("r--"); + expect(config.policy?.[dir]).toBeUndefined(); + }); + + it("auto-expand does not overwrite an existing explicit glob rule", () => { + // {"/tmp": "rwx", "/tmp/**": "---"} — bare /tmp should expand but must NOT + // clobber the explicit /tmp/** rule. Without the guard, access would widen + // from "---" to "rwx" — a security regression. + const dir = os.tmpdir(); + const config: AccessPolicyConfig = { + policy: { [dir]: "rwx", [`${dir}/**`]: "---" }, + }; + validateAccessPolicyConfig(config); + // Explicit "---" rule must be preserved. + expect(config.policy?.[`${dir}/**`]).toBe("---"); + }); + + it("auto-expands when a ~ path expands to a real directory", () => { + // "~" expands to os.homedir() which always exists and is a directory. + const config: AccessPolicyConfig = { policy: { "~": "r--" } }; + const errs = validateAccessPolicyConfig(config); + expect(errs).toHaveLength(1); + expect(errs[0]).toMatch(/auto-expanded/); + // Rule key should be rewritten with /** suffix. + expect(config.policy?.["~/**"]).toBe("r--"); + expect(config.policy?.["~"]).toBeUndefined(); + }); + + it("emits the diagnostic only once per process for the same pattern", () => { + const dir = os.tmpdir(); + // First call — should warn. + const first = validateAccessPolicyConfig({ policy: { [dir]: "r--" as const } }); + expect(first).toHaveLength(1); + // Second call with the same bare pattern — already warned, silent. + const second = validateAccessPolicyConfig({ policy: { [dir]: "r--" as const } }); + expect(second).toHaveLength(0); + }); + + it("does not warn for glob patterns or trailing-/ rules", () => { + const dir = os.tmpdir(); + expect(validateAccessPolicyConfig({ policy: { [`${dir}/**`]: "r--" } })).toEqual([]); + expect(validateAccessPolicyConfig({ policy: { [`${dir}/`]: "r--" } })).toEqual([]); + expect(validateAccessPolicyConfig({ policy: { "/tmp/**": "rwx" } })).toEqual([]); + }); + + it("does not warn for bare file paths (stat confirms it is a file)", () => { + // process.execPath is the running node/bun binary — always a real file, never a dir. + expect(validateAccessPolicyConfig({ policy: { [process.execPath]: "r--" } })).toEqual([]); + }); + + it("does not warn for paths that do not exist (ENOENT silently ignored)", () => { + expect( + validateAccessPolicyConfig({ + policy: { "/nonexistent/path/that/cannot/exist-xyzzy": "r--" }, + }), + ).toEqual([]); + }); + + it('auto-expands bare directory in scripts["policy"] shared rules', () => { + const dir = os.tmpdir(); + const config: AccessPolicyConfig = { + scripts: { policy: { [dir]: "rw-" as const } }, + }; + const errs = validateAccessPolicyConfig(config); + expect(errs.some((e) => e.includes("auto-expanded"))).toBe(true); + const sharedPolicy = config.scripts?.["policy"]; + expect(sharedPolicy?.[`${dir}/**`]).toBe("rw-"); + expect(sharedPolicy?.[dir]).toBeUndefined(); + }); + + it("auto-expands bare directory in per-script policy entry", () => { + const dir = os.tmpdir(); + const config: AccessPolicyConfig = { + scripts: { "/deploy.sh": { policy: { [dir]: "rwx" as const } } }, + }; + const errs = validateAccessPolicyConfig(config); + expect(errs.some((e) => e.includes("auto-expanded"))).toBe(true); + const entry = config.scripts?.["/deploy.sh"] as ScriptPolicyEntry | undefined; + expect(entry?.policy?.[`${dir}/**`]).toBe("rwx"); + expect(entry?.policy?.[dir]).toBeUndefined(); + }); + + it("emits a one-time diagnostic for mid-path wildcard rules (OS-level enforcement skipped)", () => { + _resetMidPathWildcardWarnedForTest(); + // "/home/*/secrets/**" has a wildcard in a non-final segment — bwrap and + // Seatbelt cannot derive a concrete mount path so they skip it silently. + // validateAccessPolicyConfig must surface this so operators know. + const errs = validateAccessPolicyConfig({ + policy: { "/home/*/secrets/**": "---" }, + }); + expect(errs).toHaveLength(1); + expect(errs[0]).toMatch(/mid-path wildcard/); + expect(errs[0]).toMatch(/OS-level.*enforcement/); + }); + + it("deduplicates mid-path wildcard rule diagnostics across calls", () => { + _resetMidPathWildcardWarnedForTest(); + const config = { policy: { "/home/*/secrets/**": "---" } }; + const first = validateAccessPolicyConfig(config); + const second = validateAccessPolicyConfig(config); + expect(first.filter((e) => e.includes("mid-path wildcard"))).toHaveLength(1); + expect(second.filter((e) => e.includes("mid-path wildcard"))).toHaveLength(0); + }); + + it("non-deny mid-path wildcard emits approximate-prefix diagnostic (not cannot-apply)", () => { + _resetMidPathWildcardWarnedForTest(); + const errs = validateAccessPolicyConfig({ + policy: { "~/.openclaw/agents/subri/workspace/skills/**/*.sh": "r-x" }, + }); + expect(errs).toHaveLength(1); + expect(errs[0]).toMatch(/mid-path wildcard/); + expect(errs[0]).toMatch(/prefix match/); + expect(errs[0]).not.toMatch(/cannot apply/); + }); + + it("does NOT emit mid-path wildcard diagnostic for final-segment wildcards", () => { + _resetMidPathWildcardWarnedForTest(); + // "/home/user/**" — wildcard is in the final segment, no path separator follows. + const errs = validateAccessPolicyConfig({ + policy: { "/home/user/**": "r--", "~/**": "rwx", "/tmp/**": "---" }, + }); + expect(errs.filter((e) => e.includes("mid-path wildcard"))).toHaveLength(0); + }); +}); + +// --------------------------------------------------------------------------- +// permAllows fail-closed on malformed characters +// --------------------------------------------------------------------------- + +describe("checkAccessPolicy — malformed permission characters fail closed", () => { + it("treats a typo like 'r1-' as deny for write (only exact 'w' grants write)", () => { + // "r1-": index 1 is "1", not "w" — must deny write, not allow it. + const config = { policy: { "/tmp/**": "r1-" as unknown as "r--" } }; + expect(checkAccessPolicy("/tmp/foo.txt", "write", config)).toBe("deny"); + }); + + it("treats 'R--' (uppercase) as deny for read (only lowercase 'r' grants read)", () => { + const config = { policy: { "/tmp/**": "R--" as unknown as "r--" } }; + expect(checkAccessPolicy("/tmp/foo.txt", "read", config)).toBe("deny"); + }); + + it("treats 'rWx' (uppercase W) as deny for write", () => { + const config = { policy: { "/tmp/**": "rWx" as unknown as "rwx" } }; + expect(checkAccessPolicy("/tmp/foo.txt", "write", config)).toBe("deny"); + }); + + it("treats 'aw-' (invalid first char) as deny for write even though index 1 is 'w'", () => { + // defense-in-depth: full format must be [r-][w-][x-]; 'a' at index 0 fails the regex + // so the entire string is rejected rather than accidentally granting write. + const config = { policy: { "/tmp/**": "aw-" as unknown as "r--" } }; + expect(checkAccessPolicy("/tmp/foo.txt", "write", config)).toBe("deny"); + }); + + it("treats a 4-char string as deny (wrong length)", () => { + const config = { policy: { "/tmp/**": "rwx!" as unknown as "rwx" } }; + expect(checkAccessPolicy("/tmp/foo.txt", "exec", config)).toBe("deny"); + }); +}); + +// --------------------------------------------------------------------------- +// Trailing slash shorthand +// --------------------------------------------------------------------------- + +describe("checkAccessPolicy — trailing slash shorthand", () => { + it('"/tmp/" is equivalent to "/tmp/**"', () => { + const config: AccessPolicyConfig = { policy: { "/tmp/": "rwx" } }; + expect(checkAccessPolicy("/tmp/foo.txt", "write", config)).toBe("allow"); + expect(checkAccessPolicy("/tmp/a/b/c", "write", config)).toBe("allow"); + }); + + it('"~/" is equivalent to "~/**"', () => { + const config: AccessPolicyConfig = { policy: { "~/": "rw-" } }; + expect(checkAccessPolicy(`${HOME}/foo.txt`, "read", config)).toBe("allow"); + expect(checkAccessPolicy(`${HOME}/foo.txt`, "write", config)).toBe("allow"); + expect(checkAccessPolicy(`${HOME}/foo.txt`, "exec", config)).toBe("deny"); + }); + + it('"---" rule with trailing slash blocks subtree', () => { + const config: AccessPolicyConfig = { + policy: { "/**": "rwx", [`${HOME}/.ssh/`]: "---" }, + }; + expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "read", config)).toBe("deny"); + }); + + it("trailing slash and /** produce identical results", () => { + const withSlash: AccessPolicyConfig = { policy: { "/tmp/": "rwx" } }; + const withGlob: AccessPolicyConfig = { policy: { "/tmp/**": "rwx" } }; + const paths = ["/tmp/a", "/tmp/a/b", "/tmp/a/b/c.txt"]; + for (const p of paths) { + expect(checkAccessPolicy(p, "write", withSlash)).toBe( + checkAccessPolicy(p, "write", withGlob), + ); + } + }); + + it("trailing slash rule covers the directory itself (mkdir check)", () => { + // Rule "~/.openclaw/heartbeat/" should allow write on the bare directory + // path ~/.openclaw/heartbeat (no trailing component), not just its contents. + const config: AccessPolicyConfig = { + policy: { "/**": "r--", [`${HOME}/.openclaw/heartbeat/`]: "rw-" }, + }; + expect(checkAccessPolicy(`${HOME}/.openclaw/heartbeat`, "write", config)).toBe("allow"); + expect(checkAccessPolicy(`${HOME}/.openclaw/heartbeat/test.txt`, "write", config)).toBe( + "allow", + ); + }); + + it('"---" trailing-slash rule blocks the directory itself and its contents', () => { + const config: AccessPolicyConfig = { + policy: { "/**": "rwx", [`${HOME}/.ssh/`]: "---" }, + }; + // Both the directory and its contents should be denied. + expect(checkAccessPolicy(`${HOME}/.ssh`, "read", config)).toBe("deny"); + expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "read", config)).toBe("deny"); + }); +}); + +// --------------------------------------------------------------------------- +// normalizePlatformPath (macOS alias transparency) +// --------------------------------------------------------------------------- + +describe.skipIf(process.platform !== "darwin")( + "checkAccessPolicy — macOS /private alias normalization", + () => { + const config: AccessPolicyConfig = { + policy: { + "/tmp/**": "rwx", + "/var/**": "r--", + "/etc/**": "r--", + }, + }; + + it("/private/tmp path is treated as /tmp — write allowed", () => { + expect(checkAccessPolicy("/private/tmp/foo.txt", "write", config)).toBe("allow"); + }); + + it("/private/var path is treated as /var — write denied (r-- only)", () => { + expect(checkAccessPolicy("/private/var/log/system.log", "write", config)).toBe("deny"); + }); + + it("/private/etc path is treated as /etc — read allowed", () => { + expect(checkAccessPolicy("/private/etc/hosts", "read", config)).toBe("allow"); + }); + + it("/tmp path still works directly", () => { + expect(checkAccessPolicy("/tmp/foo.txt", "write", config)).toBe("allow"); + }); + + it('"---" rule for /tmp/** also blocks /private/tmp/**', () => { + const denyConfig: AccessPolicyConfig = { + policy: { "/**": "rwx", "/tmp/**": "---" }, + }; + expect(checkAccessPolicy("/private/tmp/evil.sh", "exec", denyConfig)).toBe("deny"); + }); + + it("/private/tmp/** deny rule blocks /tmp/** target", () => { + // Rule written with /private/tmp must still match the normalized /tmp target. + const denyConfig: AccessPolicyConfig = { + policy: { "/**": "rwx", "/private/tmp/**": "---" }, + }; + expect(checkAccessPolicy("/tmp/evil.sh", "read", denyConfig)).toBe("deny"); + }); + + it("/private/tmp/** rule matches /tmp/** target", () => { + // Rule written with /private/* prefix must match a /tmp/* target path. + const cfg: AccessPolicyConfig = { + policy: { "/private/tmp/**": "rwx" }, + }; + expect(checkAccessPolicy("/tmp/foo.txt", "write", cfg)).toBe("allow"); + }); + }, +); + +// --------------------------------------------------------------------------- +// findBestRule +// --------------------------------------------------------------------------- + +describe("findBestRule", () => { + it("returns null when rules is empty", () => { + expect(findBestRule("/foo/bar", {})).toBeNull(); + }); + + it("returns matching rule", () => { + expect(findBestRule("/foo/bar", { "/foo/**": "r--" })).toBe("r--"); + }); + + it("prefers longer (more specific) pattern over shorter", () => { + const rules = { + "/**": "r--", + "/foo/**": "rw-", + "/foo/bar/**": "rwx", + }; + expect(findBestRule("/foo/bar/baz.txt", rules)).toBe("rwx"); + expect(findBestRule("/foo/other.txt", rules)).toBe("rw-"); + expect(findBestRule("/etc/passwd", rules)).toBe("r--"); + }); + + it("expands ~ in patterns", () => { + const rules = { "~/**": "rw-" }; + expect(findBestRule(`${HOME}/workspace/foo.py`, rules)).toBe("rw-"); + }); + + it("returns null when no pattern matches", () => { + const rules = { "/foo/**": "rw-" }; + expect(findBestRule("/bar/baz", rules)).toBeNull(); + }); + + it("bare directory path matches /** rule without requiring /. suffix", () => { + // findBestRule("/tmp", {"/tmp/**": "r--"}) must return "r--". + // Previously callers had to pass "/tmp/." to trigger a match — fragile contract. + const rules = { "/tmp/**": "r--" }; + expect(findBestRule("/tmp", rules)).toBe("r--"); + }); + + it("tilde rule beats broader absolute rule when expanded path is longer", () => { + // "~/.ssh/**" expanded is e.g. "/home/user/.ssh/**" (longer than "/home/user/**"). + // The tilde rule must win so an explicit "---" denial is not silently overridden. + const rules: Record = { + [`${HOME}/**`]: "rwx", + "~/.ssh/**": "---", + }; + expect(findBestRule(`${HOME}/.ssh/id_rsa`, rules, HOME)).toBe("---"); + }); +}); + +// --------------------------------------------------------------------------- +// checkAccessPolicy — "---" rules act as deny +// --------------------------------------------------------------------------- + +describe('checkAccessPolicy — "---" rules act as deny', () => { + it('"---" rule blocks all ops, even when a broader rule would allow', () => { + const config: AccessPolicyConfig = { + policy: { "/**": "rwx", [`${HOME}/.ssh/**`]: "---" }, + }; + expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "read", config)).toBe("deny"); + expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "write", config)).toBe("deny"); + expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "exec", config)).toBe("deny"); + }); + + it.skipIf(process.platform === "win32")( + '"---" rule does not affect paths outside its glob', + () => { + const config: AccessPolicyConfig = { + policy: { "/**": "rwx", [`${HOME}/.ssh/**`]: "---" }, + }; + expect(checkAccessPolicy(`${HOME}/workspace/foo.py`, "read", config)).toBe("allow"); + }, + ); + + it("multiple narrowing rules block distinct subtrees", () => { + const config: AccessPolicyConfig = { + policy: { "/**": "rwx", [`${HOME}/.ssh/**`]: "---", [`${HOME}/.gnupg/**`]: "---" }, + }; + expect(checkAccessPolicy(`${HOME}/.gnupg/secring.gpg`, "read", config)).toBe("deny"); + }); +}); + +// --------------------------------------------------------------------------- +// checkAccessPolicy — rules +// --------------------------------------------------------------------------- + +describe("checkAccessPolicy — rules", () => { + it("allows read when r bit is set", () => { + const config: AccessPolicyConfig = { policy: { "/**": "r--" } }; + expect(checkAccessPolicy("/etc/passwd", "read", config)).toBe("allow"); + }); + + it("denies write when w bit is absent", () => { + const config: AccessPolicyConfig = { policy: { "/**": "r--" } }; + expect(checkAccessPolicy("/etc/passwd", "write", config)).toBe("deny"); + }); + + it("denies exec when x bit is absent", () => { + const config: AccessPolicyConfig = { policy: { "/usr/bin/**": "r--" } }; + expect(checkAccessPolicy("/usr/bin/grep", "exec", config)).toBe("deny"); + }); + + it("allows exec when x bit is set", () => { + const config: AccessPolicyConfig = { policy: { "/usr/bin/**": "r-x" } }; + expect(checkAccessPolicy("/usr/bin/grep", "exec", config)).toBe("allow"); + }); + + it("longer rule overrides shorter for the same path", () => { + const config: AccessPolicyConfig = { + policy: { + "/**": "r--", + [`${HOME}/**`]: "rwx", + }, + }; + // Home subpath → rwx wins + expect(checkAccessPolicy(`${HOME}/workspace/foo`, "write", config)).toBe("allow"); + // Outside home → r-- applies + expect(checkAccessPolicy("/etc/passwd", "write", config)).toBe("deny"); + }); + + it("specific sub-path rule can restrict a broader allow", () => { + const config: AccessPolicyConfig = { + policy: { + [`${HOME}/**`]: "rwx", + [`${HOME}/.config/**`]: "r--", + }, + }; + expect(checkAccessPolicy(`${HOME}/workspace/foo`, "write", config)).toBe("allow"); + expect(checkAccessPolicy(`${HOME}/.config/sensitive`, "write", config)).toBe("deny"); + }); + + it("tilde rule beats broader absolute rule — expanded length wins", () => { + // Without the expanded-length fix, "~/.ssh/**" (9 raw chars) would lose to + // `${HOME}/**` when HOME is long, letting rwx override the intended --- deny. + const config: AccessPolicyConfig = { + policy: { + [`${HOME}/**`]: "rwx", + "~/.ssh/**": "---", + }, + }; + expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "read", config, HOME)).toBe("deny"); + expect(checkAccessPolicy(`${HOME}/workspace/foo`, "write", config, HOME)).toBe("allow"); + }); +}); + +// --------------------------------------------------------------------------- +// checkAccessPolicy — implicit fallback to "---" +// --------------------------------------------------------------------------- + +describe("checkAccessPolicy — implicit fallback to ---", () => { + it("denies all ops when no rule matches (implicit --- fallback)", () => { + const config: AccessPolicyConfig = { + policy: { [`${HOME}/**`]: "rwx" }, + }; + expect(checkAccessPolicy("/etc/passwd", "read", config)).toBe("deny"); + expect(checkAccessPolicy("/etc/passwd", "write", config)).toBe("deny"); + expect(checkAccessPolicy("/etc/passwd", "exec", config)).toBe("deny"); + }); + + it('"/**" rule acts as catch-all for unmatched paths', () => { + const config: AccessPolicyConfig = { + policy: { [`${HOME}/**`]: "rwx", "/**": "r--" }, + }; + expect(checkAccessPolicy("/etc/passwd", "read", config)).toBe("allow"); + expect(checkAccessPolicy("/etc/passwd", "write", config)).toBe("deny"); + }); + + it("empty rules deny everything via implicit fallback", () => { + const config: AccessPolicyConfig = { + policy: { [`${HOME}/workspace/**`]: "rw-" }, + }; + expect(checkAccessPolicy("/tmp/foo", "read", config)).toBe("deny"); + expect(checkAccessPolicy("/tmp/foo", "write", config)).toBe("deny"); + expect(checkAccessPolicy("/tmp/foo", "exec", config)).toBe("deny"); + }); +}); + +// --------------------------------------------------------------------------- +// checkAccessPolicy — precedence integration +// --------------------------------------------------------------------------- + +describe("checkAccessPolicy — precedence integration", () => { + it("narrowing rule beats broader allow — all in play", () => { + const config: AccessPolicyConfig = { + policy: { + "/**": "r--", + [`${HOME}/**`]: "rwx", + [`${HOME}/.ssh/**`]: "---", + }, + }; + // Broader home rule allows writes + expect(checkAccessPolicy(`${HOME}/workspace/foo`, "write", config)).toBe("allow"); + // Narrowing "---" beats the home rwx rule + expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "read", config)).toBe("deny"); + // Outer "/**" rule applies outside home + expect(checkAccessPolicy("/etc/hosts", "read", config)).toBe("allow"); + expect(checkAccessPolicy("/etc/hosts", "write", config)).toBe("deny"); + // "/proc/self/mem" matches "/**" (r--) + expect(checkAccessPolicy("/proc/self/mem", "read", config)).toBe("allow"); + }); + + it("empty config denies everything (implicit --- fallback)", () => { + const config: AccessPolicyConfig = {}; + expect(checkAccessPolicy("/anything", "read", config)).toBe("deny"); + expect(checkAccessPolicy("/anything", "write", config)).toBe("deny"); + }); +}); + +// --------------------------------------------------------------------------- +// Symlink attack scenarios — resolved-path policy checks +// +// macOS Seatbelt (and the bwrap layer) evaluate the *resolved* real path at +// the syscall level, not the symlink path. checkAccessPolicy is called with +// the already-resolved path. These tests document the expected behavior when +// a symlink in an allowed directory points to a denied or restricted target. +// --------------------------------------------------------------------------- + +describe("checkAccessPolicy — symlink resolved-path scenarios", () => { + it('denies read on resolved symlink target covered by "---" rule', () => { + // ~/workspace/link → ~/.ssh/id_rsa (symlink in allowed dir to denied-subpath) + // Caller passes the resolved path; the "---" rule wins. + const config: AccessPolicyConfig = { + policy: { [`${HOME}/workspace/**`]: "rw-", [`${HOME}/.ssh/**`]: "---" }, + }; + expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "read", config, HOME)).toBe("deny"); + }); + + it("denies write on resolved symlink target covered by restrictive rule", () => { + // ~/workspace/link → ~/workspace/secret/file + // workspace is rw-, but the secret subdir is r--. Resolved path hits r--. + const config: AccessPolicyConfig = { + policy: { + [`${HOME}/workspace/**`]: "rw-", + [`${HOME}/workspace/secret/**`]: "r--", + }, + }; + expect(checkAccessPolicy(`${HOME}/workspace/secret/file.txt`, "write", config, HOME)).toBe( + "deny", + ); + // Read is still allowed via the r-- rule. + expect(checkAccessPolicy(`${HOME}/workspace/secret/file.txt`, "read", config, HOME)).toBe( + "allow", + ); + }); + + it("symlink source path in allowed dir is allowed; resolved denied target is denied", () => { + // This illustrates that the policy must be checked on the resolved path. + const config: AccessPolicyConfig = { + policy: { [`${HOME}/workspace/**`]: "rw-", [`${HOME}/.aws/**`]: "---" }, + }; + // Source path (the symlink) — allowed + expect(checkAccessPolicy(`${HOME}/workspace/creds`, "read", config, HOME)).toBe("allow"); + // Real target — denied + expect(checkAccessPolicy(`${HOME}/.aws/credentials`, "read", config, HOME)).toBe("deny"); + }); +}); + +// --------------------------------------------------------------------------- +// resolveArgv0 +// --------------------------------------------------------------------------- + +describe("resolveArgv0", () => { + it("returns null for empty command", () => { + expect(resolveArgv0("")).toBeNull(); + expect(resolveArgv0(" ")).toBeNull(); + }); + + it("extracts first unquoted token", () => { + // /bin/sh exists on all platforms; if not, the non-resolved path is returned + const result = resolveArgv0("/bin/sh -c 'echo hi'"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + it("extracts double-quoted path", () => { + const result = resolveArgv0(`"/bin/sh" -c 'echo hi'`); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + it("returns null for relative path without cwd", () => { + expect(resolveArgv0("./script.py")).toBeNull(); + }); + + it("resolves relative path against cwd", () => { + const tmpDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "ap-test-"))); + const scriptPath = path.join(tmpDir, "script.py"); + fs.writeFileSync(scriptPath, "#!/usr/bin/env python3\n"); + try { + const result = resolveArgv0("./script.py arg1 arg2", tmpDir); + expect(result).toBe(scriptPath); + } finally { + fs.rmSync(tmpDir, { recursive: true }); + } + }); + + it("expands ~ in path", () => { + // /bin/ls will exist on test hosts; just verify ~ expansion doesn't crash + const result = resolveArgv0("~/nonexistent-script-xyz"); + // Returns expanded (non-realpath) since file doesn't exist + expect(result).toBe(`${HOME}/nonexistent-script-xyz`); + }); + + it("skips leading env-prefix assignments to find real argv0", () => { + // "FOO=1 /bin/sh -c cmd" — argv0 is /bin/sh, not FOO=1. + // Without this, policy.scripts lookup and sha256 checks are bypassed. + const result = resolveArgv0("FOO=1 BAR=2 /bin/sh -c 'echo hi'"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + it("returns null when command is only env assignments with no argv0", () => { + expect(resolveArgv0("FOO=1 BAR=2")).toBeNull(); + }); + + it("unquotes a double-quoted argv0 that follows env assignments", () => { + // FOO=1 "/opt/my script.sh" — argv0 is /opt/my script.sh (spaces in path). + // Without unquoting, the token would be '"/opt/my' — wrong path, sha256 bypass. + const result = resolveArgv0('FOO=1 "/bin/sh" -c echo'); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + it("handles env assignments with single-quoted values containing spaces", () => { + // FOO='a b' /bin/sh — naive whitespace split yields ["FOO='a", "b'", "/bin/sh"]. + // "b'" does not match NAME=, so it was wrongly treated as argv0, bypassing + // script policy lookups. Must be parsed as argv0=/bin/sh. + const result = resolveArgv0("FOO='a b' /bin/sh -c echo"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + it("handles env assignments with double-quoted values containing spaces", () => { + const result = resolveArgv0('BAR="hello world" /bin/sh -c echo'); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + it("handles multiple env assignments with quoted values", () => { + const result = resolveArgv0("A='x y' B='p q' /bin/sh -c echo"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + it("handles env assignment with escaped quote inside double-quoted value", () => { + // MYVAR="a\"b" /usr/bin/python script.py — the \" inside the value must not + // truncate the match, which would leave `b"` as the next token and misidentify + // it as argv0 instead of /usr/bin/python. + const result = resolveArgv0('MYVAR="a\\"b" /bin/sh -c echo'); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + it("handles multiple env assignments with escaped quotes in values", () => { + const result = resolveArgv0('A="x\\"y" B="p\\"q" /bin/sh -c echo'); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + // The following tests use /bin/sh and Unix env behaviour — skip on Windows where + // /bin/sh doesn't exist and env resolves to env.EXE with different semantics. + const itUnix = it.skipIf(process.platform === "win32"); + + itUnix("looks through quoted /usr/bin/env to the real script", () => { + // `"/usr/bin/env" /bin/sh` — argv0 is quoted, but env look-through must still fire. + // Without this fix, commandRest was empty in the quoted branch so env look-through + // was skipped and the function returned /usr/bin/env instead of /bin/sh. + const result = resolveArgv0(`"/usr/bin/env" /bin/sh -c echo`); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("looks through env -i flag to reach the real script", () => { + // `env -i /bin/sh` — without fix, recurses on `-i /bin/sh` and resolves `-i` as argv0. + const result = resolveArgv0("env -i /bin/sh -c echo"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("looks through env --ignore-environment long flag", () => { + const result = resolveArgv0("env --ignore-environment /bin/sh -c echo"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("looks through env -u VAR (option that consumes next token)", () => { + const result = resolveArgv0("env -u HOME /bin/sh -c echo"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("looks through env -- end-of-options marker", () => { + const result = resolveArgv0("env -- /bin/sh -c echo"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("resolves bare binary name via PATH rather than cwd", () => { + // `sh` with no `/` should find /bin/sh on PATH, not /sh. + // Without fix, path.resolve(cwd, "sh") produces /sh which doesn't exist. + const result = resolveArgv0("sh -c echo", "/nonexistent/cwd"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + expect(result).not.toContain("/nonexistent/cwd"); + }); + + it("still resolves explicitly relative tokens (./foo) against cwd", () => { + // `./script.py` contains `/` so PATH lookup is skipped — cwd resolution applies. + expect(resolveArgv0("./script.py", undefined)).toBeNull(); // no cwd → null + }); + + itUnix("uses a literal PATH= env prefix override when looking up bare names", () => { + // PATH=/nonexistent has no $, so findOnPath uses /nonexistent — sh not found there, + // falls back to cwd resolution rather than the real process PATH. + const result = resolveArgv0("PATH=/nonexistent sh", "/some/cwd"); + // Must NOT resolve to the real /bin/sh (which would mean process PATH was used). + if (result !== null) { + expect(result).toContain("/some/cwd"); + } + }); + + itUnix("ignores PATH= prefix containing shell vars and uses process PATH instead", () => { + // PATH=/alt:$PATH has $, so the override is skipped; sh found on process PATH. + const result = resolveArgv0("PATH=/alt:$PATH sh"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("strips --block-signal as a standalone flag without consuming next token", () => { + // --block-signal uses [=SIG] syntax — must not consume /bin/sh as its argument. + const result = resolveArgv0("env --block-signal /bin/sh -c echo"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("strips --default-signal and --ignore-signal as standalone flags", () => { + expect(resolveArgv0("env --default-signal /bin/sh")).toMatch(/sh$/); + expect(resolveArgv0("env --ignore-signal /bin/sh")).toMatch(/sh$/); + }); + + itUnix("recurses into env -S split-string argument to find real argv0", () => { + // env -S "FOO=1 /bin/sh -c echo" — the argument to -S is itself a command string. + // Must recurse and return /bin/sh, not null or /usr/bin/env. + const result = resolveArgv0('env -S "FOO=1 /bin/sh -c echo"'); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("recurses into env --split-string long form", () => { + const result = resolveArgv0("env --split-string '/bin/sh -c echo'"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("looks through env -C with a quoted directory arg containing spaces", () => { + // env -C "/path with space" /bin/sh — the dir arg is quoted; must not leave a + // dangling fragment that gets treated as the command. + const result = resolveArgv0('env -C "/path with space" /bin/sh -c echo'); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("looks through env --chdir with a quoted directory arg", () => { + const result = resolveArgv0("env --chdir '/tmp/my dir' /bin/sh -c echo"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("recurses into env --split-string=VALUE equals form", () => { + // --split-string=CMD (equals form) was previously not handled — resolveArgv0 + // returned null, causing the fallback to treat "env" as argv0 and silently + // bypass tool-layer hash/policy checks for the embedded script. + const result = resolveArgv0("env --split-string='/bin/sh -c echo'"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("recurses into env -S=VALUE equals form (short flag with equals)", () => { + const result = resolveArgv0("env -S='/bin/sh -c echo'"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("recurses into env -SVALUE compact form (no space, no equals)", () => { + const result = resolveArgv0("env -S'/bin/sh -c echo'"); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + itUnix("strips quoted argv0 with internal spaces when looking through env", () => { + // `"/usr/bin env" /bin/sh` — argv0 contains a space inside quotes. + // The bare /^\S+\s*/ regex stopped at the first space, leaving a corrupted afterEnv. + // The fix strips the full quoted token before processing env options/args. + // We use a path we know exists so realpathSync succeeds. + const result = resolveArgv0('"/usr/bin/env" /bin/sh -c echo'); + expect(result).not.toBeNull(); + expect(result).toMatch(/sh$/); + }); + + it("returns null for deeply nested env -S to prevent stack overflow", () => { + // Build a deeply nested "env -S 'env -S ...' " string beyond the depth cap (8). + let cmd = "/bin/sh"; + for (let i = 0; i < 10; i++) { + cmd = `env -S '${cmd}'`; + } + // Should not throw; depth cap returns null before stack overflow. + expect(() => resolveArgv0(cmd)).not.toThrow(); + // Result may be null (cap hit) or a resolved path — either is acceptable. + // The important invariant is: no RangeError. + }); +}); + +// --------------------------------------------------------------------------- +// resolveScriptKey +// --------------------------------------------------------------------------- + +describe("resolveScriptKey", () => { + it("expands leading ~", () => { + const result = resolveScriptKey("~/bin/deploy.sh"); + expect(result).toBe(path.join(HOME, "bin/deploy.sh")); + }); + + it("returns non-absolute keys unchanged", () => { + expect(resolveScriptKey("deploy.sh")).toBe("deploy.sh"); + }); + + it("returns non-existent absolute path unchanged", () => { + const p = "/no/such/path/definitely-missing-xyz"; + expect(resolveScriptKey(p)).toBe(p); + }); + + it("resolves an absolute path that exists to its real path", () => { + // Use os.tmpdir() itself — guaranteed to exist; realpathSync may resolve + // macOS /tmp → /private/tmp so we accept either the same string or a longer one. + const result = resolveScriptKey(os.tmpdir()); + expect(path.isAbsolute(result)).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// applyScriptPolicyOverride +// --------------------------------------------------------------------------- + +describe("applyScriptPolicyOverride", () => { + it("returns base policy unchanged when no scripts block", () => { + const base: AccessPolicyConfig = { policy: { "/**": "r--" } }; + const { policy, hashMismatch } = applyScriptPolicyOverride(base, "/any/path"); + expect(hashMismatch).toBeUndefined(); + expect(policy).toBe(base); + }); + + it("returns base policy unchanged when argv0 not in scripts", () => { + const base: AccessPolicyConfig = { + policy: { "/**": "r--" }, + scripts: { "/other/script.sh": { policy: { "/tmp/**": "rwx" } } }, + }; + const { policy, hashMismatch } = applyScriptPolicyOverride(base, "/my/script.sh"); + expect(hashMismatch).toBeUndefined(); + expect(policy).toBe(base); + }); + + it("matches a scripts key that is a symlink to the resolved argv0 path", () => { + // Simulate the symlink case: create a real file and a symlink to it, then + // register the symlink path as the scripts key. resolveArgv0 returns the + // realpathSync result, so the key must be resolved the same way to match. + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "ap-symlink-test-")); + const realScript = path.join(tmpDir, "script-real.sh"); + const symlinkScript = path.join(tmpDir, "script-link.sh"); + fs.writeFileSync(realScript, "#!/bin/sh\necho ok\n"); + fs.symlinkSync(realScript, symlinkScript); + try { + const resolvedReal = fs.realpathSync(symlinkScript); + const base: AccessPolicyConfig = { + policy: { "/**": "r--" }, + // Key is the symlink path; resolvedArgv0 will be the real path. + scripts: { [symlinkScript]: { policy: { "/tmp/**": "rwx" } } }, + }; + const { overrideRules, hashMismatch } = applyScriptPolicyOverride(base, resolvedReal); + expect(hashMismatch).toBeUndefined(); + // Without symlink resolution in the key lookup this would be undefined. + expect(overrideRules?.["/tmp/**"]).toBe("rwx"); + } finally { + fs.rmSync(tmpDir, { recursive: true }); + } + }); + + it("returns override rules separately so seatbelt emits them after base rules", () => { + const base: AccessPolicyConfig = { + policy: { "/**": "r--" }, + scripts: { "/my/script.sh": { policy: { [`${HOME}/.openclaw/credentials/`]: "r--" } } }, + }; + const { policy, overrideRules, hashMismatch } = applyScriptPolicyOverride( + base, + "/my/script.sh", + ); + expect(hashMismatch).toBeUndefined(); + // Base rules unchanged in policy + expect(policy.policy?.["/**"]).toBe("r--"); + expect(policy.policy?.[`${HOME}/.openclaw/credentials/`]).toBeUndefined(); + // Override rules returned separately — caller emits them last in seatbelt profile + expect(overrideRules?.[`${HOME}/.openclaw/credentials/`]).toBe("r--"); + expect(policy.scripts).toBeUndefined(); + }); + + it("override rules returned separately — base policy rule unchanged", () => { + const base: AccessPolicyConfig = { + policy: { [`${HOME}/workspace/**`]: "r--" }, + scripts: { "/trusted.sh": { policy: { [`${HOME}/workspace/**`]: "rwx" } } }, + }; + const { policy, overrideRules } = applyScriptPolicyOverride(base, "/trusted.sh"); + expect(policy.policy?.[`${HOME}/workspace/**`]).toBe("r--"); + expect(overrideRules?.[`${HOME}/workspace/**`]).toBe("rwx"); + }); + + it("narrowing override returned separately", () => { + const base: AccessPolicyConfig = { + policy: { "/tmp/**": "rwx" }, + scripts: { "/cautious.sh": { policy: { "/tmp/**": "r--" } } }, + }; + const { policy, overrideRules } = applyScriptPolicyOverride(base, "/cautious.sh"); + expect(policy.policy?.["/tmp/**"]).toBe("rwx"); + expect(overrideRules?.["/tmp/**"]).toBe("r--"); + }); + + it("returns hashMismatch when sha256 does not match file content", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "ap-test-")); + const scriptPath = path.join(tmpDir, "script.sh"); + fs.writeFileSync(scriptPath, "#!/bin/sh\necho hi\n"); + // resolveArgv0 returns the realpathSync result; simulate that here so the + // key lookup (which also calls realpathSync) matches correctly on macOS where + // os.tmpdir() returns /var/folders/... but realpathSync yields /private/var/... + const realScriptPath = fs.realpathSync(scriptPath); + try { + const base: AccessPolicyConfig = { + scripts: { + [scriptPath]: { sha256: "deadbeef".padEnd(64, "0"), policy: { "/tmp/**": "rwx" } }, + }, + }; + const { policy, hashMismatch } = applyScriptPolicyOverride(base, realScriptPath); + expect(hashMismatch).toBe(true); + expect(policy).toBe(base); + } finally { + fs.rmSync(tmpDir, { recursive: true }); + } + }); + + it("matches scripts key written with ~ even though resolvedArgv0 is absolute", () => { + // Regression: "~/bin/deploy.sh" in scripts{} must match resolvedArgv0 "/home/user/bin/deploy.sh". + // A direct object lookup misses tilde keys; ~ must be expanded before comparing. + const absPath = path.join(os.homedir(), "bin", "deploy.sh"); + const base: AccessPolicyConfig = { + policy: { "/**": "rwx" }, + scripts: { "~/bin/deploy.sh": { policy: { "/secret/**": "---" } } }, + }; + const { overrideRules, hashMismatch } = applyScriptPolicyOverride(base, absPath); + expect(hashMismatch).toBeUndefined(); + expect(overrideRules?.["/secret/**"]).toBe("---"); + }); + + it("applies override when sha256 matches — rules in overrideRules, not policy", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "ap-test-")); + const scriptPath = path.join(tmpDir, "script.sh"); + const content = "#!/bin/sh\necho hi\n"; + fs.writeFileSync(scriptPath, content); + const hash = crypto.createHash("sha256").update(Buffer.from(content)).digest("hex"); + const realScriptPath = fs.realpathSync(scriptPath); + try { + const base: AccessPolicyConfig = { + policy: { "/**": "r--" }, + scripts: { [scriptPath]: { sha256: hash, policy: { "/tmp/**": "rwx" } } }, + }; + const { policy, overrideRules, hashMismatch } = applyScriptPolicyOverride( + base, + realScriptPath, + ); + expect(hashMismatch).toBeUndefined(); + expect(overrideRules?.["/tmp/**"]).toBe("rwx"); + expect(policy.policy?.["/tmp/**"]).toBeUndefined(); + expect(policy.scripts).toBeUndefined(); + } finally { + fs.rmSync(tmpDir, { recursive: true }); + } + }); + + it("uppercase sha256 in config matches (case-normalized at comparison)", () => { + // Validation regex uses /i so uppercase passes; crypto.digest("hex") returns lowercase. + // Without .toLowerCase() at comparison, uppercase sha256 always fails at runtime — silent + // misconfiguration that denies exec with no useful error. + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "ap-test-")); + const scriptPath = path.join(tmpDir, "script.sh"); + const content = "#!/bin/sh\necho hi\n"; + fs.writeFileSync(scriptPath, content); + const hashLower = crypto.createHash("sha256").update(Buffer.from(content)).digest("hex"); + const hashUpper = hashLower.toUpperCase(); + const realScriptPath = fs.realpathSync(scriptPath); + try { + const base: AccessPolicyConfig = { + scripts: { [scriptPath]: { sha256: hashUpper, policy: { "/tmp/**": "rwx" } } }, + }; + const { hashMismatch } = applyScriptPolicyOverride(base, realScriptPath); + expect(hashMismatch).toBeUndefined(); + } finally { + fs.rmSync(tmpDir, { recursive: true }); + } + }); + + it("merges scripts['policy'] into overrideRules when a script matches", () => { + // scripts["policy"] is the shared base for all named script entries. + // It must appear in overrideRules so the tool layer and OS sandbox enforce it. + const base: AccessPolicyConfig = { + policy: { "/**": "r--" }, + scripts: { + policy: { [`${HOME}/.secrets/token`]: "r--" }, + "/my/script.sh": { policy: { "/tmp/**": "rwx" } }, + }, + }; + const { overrideRules } = applyScriptPolicyOverride(base, "/my/script.sh"); + expect(overrideRules?.[`${HOME}/.secrets/token`]).toBe("r--"); + expect(overrideRules?.["/tmp/**"]).toBe("rwx"); + }); + + it("per-script policy wins over scripts['policy'] on conflict", () => { + const base: AccessPolicyConfig = { + policy: { "/**": "r--" }, + scripts: { + policy: { "/tmp/**": "r--" }, + "/my/script.sh": { policy: { "/tmp/**": "rwx" } }, + }, + }; + const { overrideRules } = applyScriptPolicyOverride(base, "/my/script.sh"); + expect(overrideRules?.["/tmp/**"]).toBe("rwx"); + }); + + it("includes scripts['policy'] even when per-script entry has no policy key", () => { + // A script entry with only sha256 and no policy still gets scripts["policy"] applied. + const base: AccessPolicyConfig = { + policy: { "/**": "r--" }, + scripts: { + policy: { [`${HOME}/.secrets/token`]: "r--" }, + "/my/script.sh": {}, + }, + }; + const { overrideRules } = applyScriptPolicyOverride(base, "/my/script.sh"); + expect(overrideRules?.[`${HOME}/.secrets/token`]).toBe("r--"); + }); + + it("returns base policy unchanged when script entry is a non-object truthy value", () => { + // A malformed entry like `true` or `"oops"` must not be treated as a valid override. + // Without the shape check, a truthy primitive would skip sha256 and mark hasScriptOverride=true. + const base: AccessPolicyConfig = { + policy: { "/**": "r--" }, + scripts: { + "/my/script.sh": true as unknown as import("../config/types.tools.js").ScriptPolicyEntry, + }, + }; + const { policy, overrideRules, hashMismatch } = applyScriptPolicyOverride( + base, + "/my/script.sh", + ); + expect(overrideRules).toBeUndefined(); + expect(hashMismatch).toBeUndefined(); + expect(policy).toBe(base); + }); +}); diff --git a/src/infra/access-policy.ts b/src/infra/access-policy.ts new file mode 100644 index 00000000000..9ebc7cec471 --- /dev/null +++ b/src/infra/access-policy.ts @@ -0,0 +1,690 @@ +import crypto from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import type { AccessPolicyConfig, PermStr } from "../config/types.tools.js"; +import { matchesExecAllowlistPattern } from "./exec-allowlist-pattern.js"; + +export type FsOp = "read" | "write" | "exec"; + +const PERM_STR_RE = /^[r-][w-][x-]$/; + +// Track patterns already auto-expanded so the diagnostic fires once per process, +// not once per agent turn (the policy file is re-read fresh on every turn). +const _autoExpandedWarned = new Set(); + +/** Reset the one-time auto-expand warning set. Only for use in tests. */ +export function _resetAutoExpandedWarnedForTest(): void { + _autoExpandedWarned.clear(); +} + +// Track mid-path wildcard patterns already warned about — one diagnostic per pattern. +const _midPathWildcardWarned = new Set(); + +/** Reset the mid-path wildcard warning set. Only for use in tests. */ +export function _resetMidPathWildcardWarnedForTest(): void { + _midPathWildcardWarned.clear(); +} + +/** + * Returns true when a glob pattern has a wildcard character (*, ?, or bracket) + * in a non-final path segment (e.g. "/home/*\/secrets/**"). + * bwrap and Seatbelt both skip such patterns at the OS layer because the + * concrete mount/deny path cannot be derived — only the tool layer enforces them. + */ +function hasMidPathWildcard(pattern: string): boolean { + const wildcardIdx = pattern.search(/[*?[]/); + if (wildcardIdx === -1) { + return false; + } + return /[/\\]/.test(pattern.slice(wildcardIdx)); +} + +/** + * If `pattern` is a bare path (no glob metacharacters, no trailing /) that resolves + * to a real directory, auto-expand it to `pattern/**` in-place inside `rules` and push + * a diagnostic. A bare directory path matches only the directory entry itself, not its + * contents — the expanded form is almost always what the operator intended. + * + * Any stat failure is silently ignored: if the path doesn't exist the rule is a no-op. + */ +function autoExpandBareDir( + rules: Record, + pattern: string, + perm: PermStr, + errors: string[], +): void { + if (!pattern || pattern.endsWith("/") || /[*?[]/.test(pattern)) { + return; + } + const expanded = pattern.startsWith("~") ? pattern.replace(/^~(?=$|\/)/, os.homedir()) : pattern; + try { + if (fs.statSync(expanded).isDirectory()) { + const fixed = `${pattern}/**`; + // Only write the expanded key if no explicit glob for this path already + // exists — overwriting an existing "/**" rule would silently widen access + // (e.g. {"/tmp":"rwx","/tmp/**":"---"} would become {"/tmp/**":"rwx"}). + if (!(fixed in rules)) { + rules[fixed] = perm; + } + delete rules[pattern]; + if (!_autoExpandedWarned.has(pattern)) { + _autoExpandedWarned.add(pattern); + errors.push( + `access-policy.policy["${pattern}"] is a directory — rule auto-expanded to "${fixed}" so it covers all contents.`, + ); + } + } + } catch { + // Path inaccessible or missing — no action needed. + } +} + +/** + * Validates and normalizes an AccessPolicyConfig for well-formedness. + * Returns an array of human-readable diagnostic strings; empty = valid. + * May mutate config.policy in place (e.g. auto-expanding bare directory paths). + */ +export function validateAccessPolicyConfig(config: AccessPolicyConfig): string[] { + const errors: string[] = []; + + if (config.policy) { + for (const [pattern, perm] of Object.entries(config.policy)) { + if (!pattern) { + errors.push("access-policy.policy: rule key must be a non-empty glob pattern"); + } + if (!PERM_STR_RE.test(perm)) { + errors.push( + `access-policy.policy["${pattern}"] "${perm}" is invalid: must be exactly 3 chars (e.g. "rwx", "r--", "---")`, + ); + } + if (hasMidPathWildcard(pattern) && !_midPathWildcardWarned.has(`policy:${pattern}`)) { + _midPathWildcardWarned.add(`policy:${pattern}`); + if (perm === "---") { + // Deny-all on a mid-path wildcard prefix would be too broad at the OS layer + // (e.g. "secrets/**/*.env: ---" → deny all of secrets/). Skip OS emission entirely. + errors.push( + `access-policy.policy["${pattern}"] contains a mid-path wildcard with "---" — OS-level (bwrap/Seatbelt) enforcement cannot apply; tool-layer enforcement is still active.`, + ); + } else { + // For non-deny rules the OS layer uses the longest concrete prefix as an + // approximate mount/subpath target. The file-type filter (e.g. *.sh) is enforced + // precisely by the tool layer only. + errors.push( + `access-policy.policy["${pattern}"] contains a mid-path wildcard — OS-level enforcement uses prefix match (file-type filter is tool-layer only).`, + ); + } + } + // If a bare path (no glob metacharacters, no trailing /) points to a real + // directory it would match only the directory entry itself, not its + // contents. Auto-expand to "/**" and notify — the fix is unambiguous. + autoExpandBareDir(config.policy, pattern, perm, errors); + } + } + + if (config.scripts) { + // scripts["policy"] is a shared Record — validate as flat rules. + const sharedPolicy = config.scripts["policy"]; + if (sharedPolicy) { + for (const [pattern, perm] of Object.entries(sharedPolicy)) { + if (!PERM_STR_RE.test(perm)) { + errors.push( + `access-policy.scripts["policy"]["${pattern}"] "${perm}" is invalid: must be exactly 3 chars (e.g. "rwx", "r--", "---")`, + ); + } + if ( + hasMidPathWildcard(pattern) && + !_midPathWildcardWarned.has(`scripts:policy:${pattern}`) + ) { + _midPathWildcardWarned.add(`scripts:policy:${pattern}`); + if (perm === "---") { + errors.push( + `access-policy.scripts["policy"]["${pattern}"] contains a mid-path wildcard with "---" — OS-level (bwrap/Seatbelt) enforcement cannot apply; tool-layer enforcement is still active.`, + ); + } else { + errors.push( + `access-policy.scripts["policy"]["${pattern}"] contains a mid-path wildcard — OS-level enforcement uses prefix match (file-type filter is tool-layer only).`, + ); + } + } + autoExpandBareDir(sharedPolicy, pattern, perm, errors); + } + } + for (const [scriptPath, entry] of Object.entries(config.scripts)) { + if (scriptPath === "policy") { + continue; // handled above + } + // Reject non-object entries (e.g. true, "rwx") — a truthy primitive would + // bypass the exec gate in hasScriptOverride and applyScriptPolicyOverride. + if (entry == null || typeof entry !== "object" || Array.isArray(entry)) { + errors.push( + `access-policy.scripts["${scriptPath}"] must be an object (e.g. { sha256: "...", policy: {...} }), got ${JSON.stringify(entry)}`, + ); + continue; + } + const scriptEntry = entry as import("../config/types.tools.js").ScriptPolicyEntry; + // Validate sha256 format when present — a typo causes silent exec denial at runtime. + if (scriptEntry.sha256 !== undefined) { + if (!/^[0-9a-f]{64}$/i.test(scriptEntry.sha256)) { + errors.push( + `access-policy.scripts["${scriptPath}"].sha256 "${scriptEntry.sha256}" is invalid: must be a 64-character lowercase hex string`, + ); + } + } + if (scriptEntry.policy) { + for (const [pattern, perm] of Object.entries(scriptEntry.policy)) { + if (!PERM_STR_RE.test(perm)) { + errors.push( + `access-policy.scripts["${scriptPath}"].policy["${pattern}"] "${perm}" is invalid: must be exactly 3 chars (e.g. "rwx", "r--", "---")`, + ); + } + // Emit mid-path wildcard diagnostic for per-script policy blocks, + // matching the same warning emitted for config.policy entries. + if ( + hasMidPathWildcard(pattern) && + !_midPathWildcardWarned.has(`scripts:${scriptPath}:${pattern}`) + ) { + _midPathWildcardWarned.add(`scripts:${scriptPath}:${pattern}`); + if (perm === "---") { + errors.push( + `access-policy.scripts["${scriptPath}"].policy["${pattern}"] contains a mid-path wildcard with "---" — OS-level (bwrap/Seatbelt) enforcement cannot apply; tool-layer enforcement is still active.`, + ); + } else { + errors.push( + `access-policy.scripts["${scriptPath}"].policy["${pattern}"] contains a mid-path wildcard — OS-level enforcement uses prefix match (file-type filter is tool-layer only).`, + ); + } + } + autoExpandBareDir(scriptEntry.policy, pattern, perm, errors); + } + } + } + } + + return errors; +} + +/** + * Normalize and expand a config pattern before matching: + * - Trailing "/" is shorthand for "/**" (everything under this directory). + * e.g. "/tmp/" → "/tmp/**", "~/" → "~/**" + * - Leading "~" is expanded to the OS home directory. + * + * We intentionally use os.homedir() rather than expandHomePrefix() so that + * OPENCLAW_HOME does not redirect ~ to the OpenClaw config directory. + */ +function expandPattern(pattern: string, homeDir: string): string { + // Trailing / shorthand: "/tmp/" → "/tmp/**" + const normalized = pattern.endsWith("/") ? pattern + "**" : pattern; + if (!normalized.startsWith("~")) { + return normalized; + } + return normalized.replace(/^~(?=$|[/\\])/, homeDir); +} + +/** + * macOS maps several traditional Unix root directories to /private/* via symlinks. + * Kernel-level enforcement (seatbelt) sees the real /private/* paths, but users + * naturally write /tmp, /var, /etc in their config. + * + * We normalize the target path to its "friendly" alias before matching so that + * /private/tmp/foo is treated as /tmp/foo everywhere — no need to write both. + * + * Only applied on darwin; on other platforms the map is empty (no-op). + */ +const MACOS_PRIVATE_ALIASES: ReadonlyArray<[real: string, alias: string]> = + process.platform === "darwin" + ? [ + ["/private/tmp", "/tmp"], + ["/private/var", "/var"], + ["/private/etc", "/etc"], + ] + : []; + +function normalizePlatformPath(p: string): string { + for (const [real, alias] of MACOS_PRIVATE_ALIASES) { + if (p === real) { + return alias; + } + if (p.startsWith(real + "/")) { + return alias + p.slice(real.length); + } + } + return p; +} + +// Maps operation to its index in the rwx permission string. +const OP_INDEX: Record = { + read: 0, + write: 1, + exec: 2, +}; + +// The exact character that grants each operation. Any other character (including +// typos like "1", "y", "R") is treated as deny — fail-closed on malformed input. +const OP_GRANT_CHAR: Record = { + read: "r", + write: "w", + exec: "x", +}; + +// Valid perm strings are exactly 3 chars: [r-][w-][x-]. +// Validated at parse time by validateAccessPolicyConfig, but also checked here +// as defense-in-depth so a malformed value never accidentally grants access. +const VALID_PERM_RE = /^[r-][w-][x-]$/; + +/** + * Returns true if the given permission string grants the requested operation. + * An absent or malformed string is treated as "---" (deny all). + * Only the exact grant character ("r"/"w"/"x") is accepted — any other value + * including typos fails closed rather than accidentally granting access. + */ +function permAllows(perm: PermStr | undefined, op: FsOp): boolean { + if (!perm || !VALID_PERM_RE.test(perm)) { + return false; + } + return perm[OP_INDEX[op]] === OP_GRANT_CHAR[op]; +} + +/** + * Finds the most specific matching rule for targetPath using longest-glob-wins. + * Returns the permission string for that rule, or null if nothing matches. + */ +export function findBestRule( + targetPath: string, + rules: Record, + homeDir: string = os.homedir(), +): PermStr | null { + let bestPerm: PermStr | null = null; + let bestLen = -1; + + for (const [pattern, perm] of Object.entries(rules)) { + // Normalize the expanded pattern so /private/tmp/** matches /tmp/** on macOS. + const expanded = normalizePlatformPath(expandPattern(pattern, homeDir)); + // Test both the bare path and path + "/" so that "dir/**"-style rules match + // the directory itself — mirrors the dual-probe in checkAccessPolicy so + // callers don't need to remember to append "/." when passing a directory. + if ( + matchesExecAllowlistPattern(expanded, targetPath) || + matchesExecAllowlistPattern(expanded, targetPath + "/") + ) { + // Longer *expanded* pattern = more specific. Compare expanded lengths so + // a tilde rule like "~/.ssh/**" (expanded: "/home/user/.ssh/**", 20 chars) + // correctly beats a broader absolute rule like "/home/user/**" (14 chars). + if (expanded.length > bestLen) { + bestLen = expanded.length; + bestPerm = perm; + } + } + } + + return bestPerm; +} + +/** + * Checks whether a given operation on targetPath is permitted by the config. + * + * Precedence: + * 1. rules — longest matching glob wins; check the relevant bit. + * 2. implicit fallback — `"---"` (deny-all) when no rule matches. + * Use `"/**": "r--"` (or any other perm) as an explicit catch-all rule. + */ +export function checkAccessPolicy( + targetPath: string, + op: FsOp, + config: AccessPolicyConfig, + homeDir: string = os.homedir(), +): "allow" | "deny" { + // Expand leading ~ in targetPath so callers don't have to pre-expand tilde paths. + const expandedTarget = targetPath.startsWith("~") + ? targetPath.replace(/^~(?=$|\/)/, homeDir) + : targetPath; + // Normalize /private/tmp → /tmp etc. so macOS symlink aliases are transparent. + const normalizedPath = normalizePlatformPath(expandedTarget); + // For directory-level checks (e.g. mkdir), also try path + "/" so that a + // trailing-/ rule ("~/.openclaw/heartbeat/" → "/**") covers the directory + // itself and not only its descendants. + const normalizedPathDir = normalizedPath + "/"; + + function matchesPattern(expanded: string): boolean { + return ( + matchesExecAllowlistPattern(expanded, normalizedPath) || + matchesExecAllowlistPattern(expanded, normalizedPathDir) + ); + } + + // rules — longest match wins (check both path and path + "/" variants). + let bestPerm: PermStr | null = null; + let bestLen = -1; + for (const [pattern, perm] of Object.entries(config.policy ?? {})) { + // Normalize so /private/tmp/** patterns match /tmp/** targets on macOS. + const expanded = normalizePlatformPath(expandPattern(pattern, homeDir)); + if (matchesPattern(expanded) && expanded.length > bestLen) { + bestLen = expanded.length; + bestPerm = perm; + } + } + if (bestPerm !== null) { + return permAllows(bestPerm, op) ? "allow" : "deny"; + } + + // Implicit fallback: "---" (deny-all) when no rule matches. + return "deny"; +} + +/** + * Search PATH for a bare binary name, returning the first executable found. + * Returns null when not found. The caller applies realpathSync afterwards. + */ +function findOnPath(name: string, pathOverride?: string): string | null { + const pathEnv = pathOverride ?? process.env.PATH ?? ""; + // On Windows, bare names like "node" resolve to "node.exe" or "node.cmd" via + // PATHEXT. Without probing extensions, accessSync finds nothing and we fall back + // to the cwd-relative path, causing checkAccessPolicy to evaluate the wrong path. + const extensions = + process.platform === "win32" + ? (process.env.PATHEXT ?? ".COM;.EXE;.BAT;.CMD").split(path.delimiter) + : [""]; + for (const dir of pathEnv.split(path.delimiter)) { + if (!dir) { + continue; + } + for (const ext of extensions) { + const candidate = path.join(dir, name + ext); + try { + fs.accessSync(candidate, fs.constants.X_OK); + return candidate; + } catch { + // not in this dir/ext combo + } + } + } + return null; +} + +/** + * Extract and resolve the argv[0] token from a shell command string. + * + * Handles leading-quoted paths ("..." and '...') and simple unquoted tokens. + * Expands ~ to os.homedir(). Resolves relative paths against cwd. + * Follows symlinks via realpathSync so the result matches an absolute-path key. + * + * Returns null when the command is empty or the path cannot be determined. + */ +export function resolveArgv0(command: string, cwd?: string, _depth = 0): string | null { + // Guard against deeply nested env -S "env -S '...'" constructs that would + // otherwise overflow the call stack. 8 levels is far more than any real usage. + if (_depth > 8) { + return null; + } + const trimmed = command.trim(); + if (!trimmed) { + return null; + } + // Extract the first token, respecting simple leading quotes. + // Skip leading shell env-prefix assignments (e.g. FOO=1 /script.sh → /script.sh) + // so that script policy lookups and sha256 checks are not bypassed by prefixed envs. + let token: string; + // commandRest holds the tail of the command string after argv0 — used to look + // through `env` invocations where the real script follows the launcher. + let commandRest = ""; + // Literal PATH= override extracted from env-prefix assignments (no shell vars). + // Used so `PATH=/alt deploy.sh` looks up deploy.sh on /alt rather than process PATH. + let commandScopedPath: string | undefined; + if (trimmed[0] === '"' || trimmed[0] === "'") { + const quote = trimmed[0]; + const end = trimmed.indexOf(quote, 1); + token = end !== -1 ? trimmed.slice(1, end) : trimmed.slice(1); + // Set commandRest so the env look-through below can strip the quoted argv0 and + // recurse into the actual script (e.g. `"/usr/bin/env" /my/script.sh` → /my/script.sh). + commandRest = trimmed; + } else { + // Progressively consume leading NAME=value env-prefix tokens before extracting argv0. + // Using a regex that matches the full assignment including quoted values (e.g. + // FOO='a b') prevents misparse when a quoted env value contains spaces — a naive + // whitespace-split would break FOO='a b' /script.sh into ["FOO='a", "b'", "/script.sh"] + // and incorrectly treat "b'" as the argv0, bypassing script policy lookups. + // Double-quoted values: allow backslash-escaped characters (e.g. "a\"b") so the + // regex doesn't truncate at the escaped quote and misidentify the next token as argv0. + // Single-quoted values: no escaping in POSIX sh single quotes, so [^']* is correct. + const envPrefixRe = /^[A-Za-z_][A-Za-z0-9_]*=(?:"(?:[^"\\]|\\.)*"|'[^']*'|\S*)\s*/; + let rest = trimmed; + while (envPrefixRe.test(rest)) { + // Capture a literal PATH= override; skip if the value contains $ (unexpandable). + const pathM = rest.match(/^PATH=(?:"((?:[^"\\]|\\.)*)"|'([^']*)'|(\S+))\s*/); + if (pathM) { + const val = pathM[1] ?? pathM[2] ?? pathM[3] ?? ""; + if (!val.includes("$")) { + commandScopedPath = val; + } + } + rest = rest.replace(envPrefixRe, ""); + } + const raw = rest.split(/\s+/)[0] ?? ""; + // If the argv0 token is quoted (e.g. FOO=1 "/opt/my script.sh"), strip quotes. + if (raw[0] === '"' || raw[0] === "'") { + const quote = raw[0]; + const end = rest.indexOf(quote, 1); + token = end !== -1 ? rest.slice(1, end) : rest.slice(1); + } else { + token = raw; + } + commandRest = rest; + } + if (!token) { + return null; + } + // Expand leading ~ + if (token.startsWith("~")) { + token = token.replace(/^~(?=$|\/)/, os.homedir()); + } + // Resolve relative paths. For bare names with no path separator (e.g. "deploy.sh"), + // try PATH lookup first so script-policy keys match the real on-PATH binary rather + // than /deploy.sh. Explicitly relative tokens (./foo, ../foo) contain a separator + // and are resolved against cwd only, matching the shell's own behaviour. + if (!path.isAbsolute(token)) { + const hasPathSep = token.includes("/") || token.includes("\\"); + if (!hasPathSep) { + const onPath = findOnPath(token, commandScopedPath); + if (onPath) { + token = onPath; + } else if (cwd) { + token = path.resolve(cwd, token); + } else { + return null; + } + } else if (cwd) { + token = path.resolve(cwd, token); + } else { + return null; + } + } + // Follow symlinks so keys always refer to the real file + try { + token = fs.realpathSync(token); + } catch { + // token stays as-is + } + + // If the resolved binary is `env` (e.g. `env FOO=1 /script.sh`), look past it + // to the actual script so script-policy lookups and sha256 checks are not bypassed + // by prepending `env`. Recurse so the inner command gets the same full treatment + // (NAME=value stripping, quoting, cwd-relative resolution, symlink following). + if (path.basename(token, path.extname(token)) === "env" && commandRest) { + // Strip the env/"/usr/bin/env" token itself from commandRest. + // When argv0 was quoted (e.g. `"/usr/bin env" /script.sh`), a bare /^\S+\s*/ would + // stop at the first space inside the quoted token. Handle the quoted case explicitly. + let afterEnv: string; + if (commandRest[0] === '"' || commandRest[0] === "'") { + const q = commandRest[0]; + const closeIdx = commandRest.indexOf(q, 1); + afterEnv = closeIdx !== -1 ? commandRest.slice(closeIdx + 1).trimStart() : ""; + } else { + afterEnv = commandRest.replace(/^\S+\s*/, ""); + } + // Skip env options and their arguments so `env -i /script.sh` resolves to + // /script.sh rather than treating `-i` as argv0. Short options that consume + // the next token as their argument (-u VAR, -C DIR) are stripped including + // any quoted value (e.g. -C "/path with space"). -S/--split-string is special: + // its value IS a command string, so we recurse into it rather than discard it. + // All other flags (e.g. -i, --ignore-environment) are single standalone tokens. + // NAME=value pairs are handled naturally when we recurse into resolveArgv0. + // --block-signal, --default-signal, --ignore-signal use [=SIG] syntax (never space-separated). + const envOptWithArgRe = /^(-[uC]|--(unset|chdir))\s+/; + while (afterEnv) { + if (afterEnv === "--" || afterEnv.startsWith("-- ")) { + afterEnv = afterEnv.slice(2).trimStart(); + break; // -- terminates env options; what follows is the command + } + // -S/--split-string: the argument is itself a command string — recurse into it. + // Handle all three forms GNU env accepts: + // space: -S CMD / --split-string CMD + // equals: -S=CMD / --split-string=CMD + // compact: -SCMD (short flag only, value starts immediately after -S) + const splitEqM = afterEnv.match(/^(?:-S|--(split-string))=([\s\S]*)/); + const splitSpM = afterEnv.match(/^(?:-S|--(split-string))\s+([\s\S]*)/); + const splitCmM = afterEnv.match(/^-S([^\s=][\s\S]*)/); + const splitArg = splitEqM + ? splitEqM[splitEqM.length - 1] + : splitSpM + ? splitSpM[splitSpM.length - 1] + : splitCmM + ? splitCmM[1] + : null; + if (splitArg !== null) { + let inner = splitArg.trim(); + // Strip surrounding quotes that the shell added around the embedded command. + if ( + (inner.startsWith('"') && inner.endsWith('"')) || + (inner.startsWith("'") && inner.endsWith("'")) + ) { + inner = inner.slice(1, -1); + } + return inner ? resolveArgv0(inner, cwd, _depth + 1) : null; + } + if (envOptWithArgRe.test(afterEnv)) { + // Strip option + its argument; handle quoted values with spaces. + afterEnv = afterEnv.replace(/^\S+\s+(?:"[^"]*"|'[^']*'|\S+)\s*/, ""); + } else if (afterEnv[0] === "-") { + afterEnv = afterEnv.replace(/^\S+\s*/, ""); // strip standalone flag + } else { + break; // first non-option token — may still be NAME=value, handled by recursion + } + } + return afterEnv ? resolveArgv0(afterEnv, cwd, _depth + 1) : null; + } + + return token; +} + +/** + * Normalize a scripts config key for comparison against a resolveArgv0 result. + * + * Expands a leading ~ and resolves symlinks for absolute paths, so that a key + * like "/usr/bin/python" (symlink → /usr/bin/python3.12) still matches when + * resolveArgv0 returns the real path "/usr/bin/python3.12". + */ +export function resolveScriptKey(k: string): string { + // path.normalize converts forward slashes to OS-native separators on Windows so that + // a tilde key like "~/bin/script.sh" compares correctly against a resolved argv0 + // that uses backslashes on Windows. + const expanded = k.startsWith("~") ? path.normalize(k.replace(/^~(?=$|[/\\])/, os.homedir())) : k; + if (!path.isAbsolute(expanded)) { + return expanded; + } + try { + return fs.realpathSync(expanded); + } catch { + // Key path doesn't exist — keep expanded; the lookup will simply not match. + return expanded; + } +} + +/** + * Apply a per-script policy overlay to the base agent policy. + * + * Looks up resolvedArgv0 in policy.scripts. If found: + * - verifies sha256 when set (returns hashMismatch=true on failure → caller should deny exec) + * - merges grant over base rules (override key wins) + * - appends restrict.deny to base deny (additive) + * - strips the scripts block from the result so the overlay doesn't apply to future + * unrelated exec calls in the same agent turn (seatbelt/bwrap still covers the full + * subprocess tree of the wrapped command at the OS level — that is correct behavior) + * + * Returns the base policy unchanged when no matching script entry exists. + */ +export function applyScriptPolicyOverride( + policy: AccessPolicyConfig, + resolvedArgv0: string, +): { policy: AccessPolicyConfig; overrideRules?: Record; hashMismatch?: true } { + // Normalize scripts keys via resolveScriptKey so that: + // - tilde keys ("~/bin/deploy.sh") expand to absolute paths + // - symlink keys ("/usr/bin/python" → /usr/bin/python3.12) resolve to real paths + // resolveArgv0 always returns the realpathSync result, so both forms must be + // normalized the same way or the lookup silently misses, skipping sha256 verification. + const scripts = policy.scripts; + const rawOverride = scripts + ? Object.entries(scripts).find( + ([k]) => + k !== "policy" && path.normalize(resolveScriptKey(k)) === path.normalize(resolvedArgv0), + )?.[1] + : undefined; + // Reject non-object entries (e.g. true, "oops") — a truthy primitive would + // otherwise skip sha256 verification and act as an unchecked override grant. + const override = + rawOverride != null && typeof rawOverride === "object" && !Array.isArray(rawOverride) + ? (rawOverride as import("../config/types.tools.js").ScriptPolicyEntry) + : undefined; + if (!override) { + return { policy }; + } + + // Verify sha256 when configured — reduces script swap risk. + // Known limitation: there is an inherent TOCTOU window between the hash read + // here and the kernel exec() call. An attacker who can swap the file between + // these two moments could run a different payload under the per-script policy. + // Fully closing this would require atomic open-and-exec (e.g. execveat + memfd) + // which is not available in Node.js userspace. This check is a best-effort guard, + // not a cryptographic guarantee. Use OS-level filesystem permissions to restrict + // who can modify script files for stronger protection. + if (override.sha256) { + let actualHash: string; + try { + // Policy-engine internal read: intentionally bypasses checkAccessPolicy. + // The policy engine must verify the script's integrity before deciding + // whether to grant the script's extra permissions — checking the policy + // first would be circular. This read is safe: it never exposes content + // to the agent; it only computes a hash for comparison. + const contents = fs.readFileSync(resolvedArgv0); + actualHash = crypto.createHash("sha256").update(contents).digest("hex"); + } catch { + return { policy, hashMismatch: true }; + } + // Normalize to lowercase: crypto.digest("hex") always returns lowercase, but + // the validation regex accepts uppercase (/i). Without normalization an uppercase + // sha256 in config passes validation and then silently fails here at runtime. + if (actualHash !== override.sha256.toLowerCase()) { + return { policy, hashMismatch: true }; + } + } + + // Build the merged policy WITHOUT the override rules merged in. + // Override rules are returned separately so the caller can emit them AFTER + // the base rules in the seatbelt profile (last-match-wins — grants must come + // after broader rules to override them, e.g. a script-specific grant inside + // a broadly denied subtree). + const { scripts: _scripts, ...base } = policy; + const merged: AccessPolicyConfig = { ...base }; + + // Merge scripts["policy"] (shared base for all matching scripts) with the + // per-script entry policy. Per-script wins on conflict (applied last). + const sharedPolicy = scripts?.["policy"]; + const mergedOverride: Record = { + ...sharedPolicy, + ...override.policy, + }; + return { + policy: merged, + overrideRules: Object.keys(mergedOverride).length > 0 ? mergedOverride : undefined, + }; +} diff --git a/src/infra/exec-sandbox-bwrap.test.ts b/src/infra/exec-sandbox-bwrap.test.ts new file mode 100644 index 00000000000..0e347a5180d --- /dev/null +++ b/src/infra/exec-sandbox-bwrap.test.ts @@ -0,0 +1,513 @@ +import os from "node:os"; +import { describe, expect, it, vi } from "vitest"; +import type { AccessPolicyConfig } from "../config/types.tools.js"; +import { + _resetBwrapAvailableCacheForTest, + _resetBwrapFileDenyWarnedPathsForTest, + _warnBwrapFileDenyOnce, + generateBwrapArgs, + isBwrapAvailable, + wrapCommandWithBwrap, +} from "./exec-sandbox-bwrap.js"; + +const HOME = os.homedir(); + +// bwrap is Linux-only — skip the generateBwrapArgs tests on other platforms so +// Windows/macOS CI does not fail on fs.statSync calls against Unix-only paths +// like /etc/hosts that don't exist there. +describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => { + it("starts with --ro-bind / / when /** rule allows reads", () => { + const config: AccessPolicyConfig = { policy: { "/**": "r--" } }; + const args = generateBwrapArgs(config, HOME); + expect(args.slice(0, 3)).toEqual(["--ro-bind", "/", "/"]); + }); + + it("does not use --ro-bind / / when no /** rule (restrictive base)", () => { + const config: AccessPolicyConfig = {}; + const args = generateBwrapArgs(config, HOME); + // Should not contain root bind + const rootBindIdx = args.findIndex( + (a, i) => a === "--ro-bind" && args[i + 1] === "/" && args[i + 2] === "/", + ); + expect(rootBindIdx).toBe(-1); + }); + + it("ends with --", () => { + const config: AccessPolicyConfig = { policy: { "/**": "r--" } }; + const args = generateBwrapArgs(config, HOME); + expect(args[args.length - 1]).toBe("--"); + }); + + it('adds --tmpfs for "---" rules in permissive mode', () => { + const config: AccessPolicyConfig = { + policy: { "/**": "r--", [`${HOME}/.ssh/**`]: "---", [`${HOME}/.gnupg/**`]: "---" }, + }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).toContain(`${HOME}/.ssh`); + expect(tmpfsMounts).toContain(`${HOME}/.gnupg`); + }); + + it('expands ~ in "---" rules using homeDir', () => { + const config: AccessPolicyConfig = { + policy: { "/**": "r--", "~/.ssh/**": "---" }, + }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).toContain(`${HOME}/.ssh`); + }); + + it("adds --bind for paths with w bit in rules", () => { + const config: AccessPolicyConfig = { + policy: { "/**": "r--", [`${HOME}/workspace/**`]: "rw-" }, + }; + const args = generateBwrapArgs(config, HOME); + const bindPairs: string[] = []; + for (let i = 0; i < args.length - 2; i++) { + if (args[i] === "--bind-try") { + bindPairs.push(args[i + 1]); + } + } + expect(bindPairs).toContain(`${HOME}/workspace`); + }); + + it("does not add --bind for read-only rules on permissive base", () => { + const config: AccessPolicyConfig = { + policy: { "/**": "r--", "/usr/bin/**": "r--" }, + }; + const args = generateBwrapArgs(config, HOME); + // /usr/bin should NOT appear as a --bind-try (it's already ro-bound via /) + const bindPairs: string[] = []; + for (let i = 0; i < args.length - 2; i++) { + if (args[i] === "--bind-try") { + bindPairs.push(args[i + 1]); + } + } + expect(bindPairs).not.toContain("/usr/bin"); + }); + + it('"---" rule for sensitive path appears in args regardless of broader rule', () => { + const config: AccessPolicyConfig = { + policy: { "/**": "r--", [`${HOME}/**`]: "rwx", [`${HOME}/.ssh/**`]: "---" }, + }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).toContain(`${HOME}/.ssh`); + }); + + it("does not crash on empty config", () => { + expect(() => generateBwrapArgs({}, HOME)).not.toThrow(); + }); + + it("adds --proc /proc in permissive mode so /proc is accessible inside the sandbox", () => { + // --ro-bind / / does not propagate kernel filesystems (procfs) into the new + // mount namespace; without --proc /proc, shells and Python fail in the sandbox. + const args = generateBwrapArgs({ policy: { "/**": "r--" } }, HOME); + const procIdx = args.indexOf("--proc"); + expect(procIdx).toBeGreaterThan(-1); + expect(args[procIdx + 1]).toBe("/proc"); + }); + + it("adds --tmpfs /tmp in permissive mode (/** allows reads)", () => { + const config: AccessPolicyConfig = { policy: { "/**": "r--" } }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).toContain("/tmp"); + }); + + it("does not add --tmpfs /tmp in restrictive mode (no /** rule)", () => { + // Without a "/**" rule, the base is restrictive — no /tmp by default. + const config: AccessPolicyConfig = {}; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).not.toContain("/tmp"); + }); + + it("skips --tmpfs /tmp in permissive mode when policy explicitly restricts /tmp writes", () => { + // A rule "/tmp/**": "r--" means the user wants /tmp read-only; the base --ro-bind / / + // already makes it readable. Adding --tmpfs /tmp would silently grant write access. + const config: AccessPolicyConfig = { policy: { "/**": "r--", "/tmp/**": "r--" } }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).not.toContain("/tmp"); + }); + + it("skips --tmpfs /tmp when an explicit write rule covers /tmp (rules loop emits --bind-try)", () => { + // Regression: the old code also emitted --tmpfs /tmp when explicitTmpPerm[1] === "w", + // but the rules loop always follows with --bind-try /tmp /tmp which wins (last mount wins + // in bwrap). The --tmpfs was dead code. Confirm: explicit rw- rule → no --tmpfs /tmp, + // but --bind-try /tmp IS present. + const config: AccessPolicyConfig = { policy: { "/**": "r--", "/tmp/**": "rw-" } }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + const bindMounts = args + .map((a, i) => (a === "--bind-try" ? args[i + 1] : null)) + .filter(Boolean); + expect(tmpfsMounts).not.toContain("/tmp"); + expect(bindMounts).toContain("/tmp"); + }); + + it("does not add --tmpfs /tmp in restrictive mode (no /** rule) — regression guard", () => { + // When there is no "/**" rule at all, no /tmp mount should appear. + const config: AccessPolicyConfig = { policy: { [`${HOME}/workspace/**`]: "rwx" } }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).not.toContain("/tmp"); + }); + + it('"---" rule in permissive mode gets --tmpfs overlay to block reads', () => { + // With "/**":"r--", --ro-bind / / makes everything readable. A narrowing + // rule like "/secret/**": "---" must overlay --tmpfs so the path is hidden. + const config: AccessPolicyConfig = { + policy: { "/**": "r--", [`${HOME}/secret/**`]: "---" }, + }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).toContain(`${HOME}/secret`); + // Must NOT produce a bind mount for this path. + const bindMounts = args + .map((a, i) => (a === "--bind-try" ? args[i + 1] : null)) + .filter(Boolean); + expect(bindMounts).not.toContain(`${HOME}/secret`); + }); + + it("narrowing rule on an existing file does not emit --tmpfs (bwrap only accepts dirs)", () => { + // process.execPath is always an existing file — use it as the test target. + const filePath = process.execPath; + const config: AccessPolicyConfig = { + policy: { "/**": "r--", [filePath]: "---" }, + }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + // Must NOT emit --tmpfs for a file path. + expect(tmpfsMounts).not.toContain(filePath); + }); + + it('"--x" rule in permissive mode gets --tmpfs overlay to block reads', () => { + // Execute-only rules have no read bit — same treatment as "---" in permissive mode. + const config: AccessPolicyConfig = { + policy: { "/**": "r--", [`${HOME}/scripts/**`]: "--x" }, + }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).toContain(`${HOME}/scripts`); + }); + + it('"---" rule on SYSTEM_RO_BIND_PATHS path emits --tmpfs in restrictive mode', () => { + // SYSTEM_RO_BIND_PATHS (/etc, /usr, /bin, /lib, /lib64, /sbin, /opt) are unconditionally + // --ro-bind-try mounted in restrictive mode. Without a --tmpfs overlay, a "---" rule on + // e.g. "/etc/**" has no OS-level effect — syscalls inside the sandbox can still read + // /etc/passwd, /etc/shadow, etc. The fix: treat deny rules the same in both modes. + const config: AccessPolicyConfig = { + policy: { "/etc/**": "---" }, + }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).toContain("/etc"); + // Must NOT emit a read mount for a deny rule. + const roBound = args + .map((a, i) => (a === "--ro-bind-try" ? args[i + 1] : null)) + .filter(Boolean); + expect(roBound).not.toContain("/etc"); + }); + + it('"---" rules do not create --ro-bind-try mounts in restrictive mode', () => { + // A rule with "---" permission should NOT produce any bwrap mount — the + // restrictive base already denies by not mounting. Emitting --ro-bind-try + // for a "---" rule would silently grant read access to paths that should + // be fully blocked. + const config: AccessPolicyConfig = { + policy: { + [`${HOME}/workspace/**`]: "rwx", // allowed: should produce --bind-try + [`${HOME}/workspace/private/**`]: "---", // denied: must NOT produce any mount + }, + }; + const args = generateBwrapArgs(config, HOME); + const roBound = args + .map((a, i) => (a === "--ro-bind-try" ? args[i + 1] : null)) + .filter(Boolean); + // "---" rule must not produce a read-only bind + expect(roBound).not.toContain(`${HOME}/workspace/private`); + // "rwx" rule must produce a read-write bind (sanity check) + const rwBound = args.map((a, i) => (a === "--bind-try" ? args[i + 1] : null)).filter(Boolean); + expect(rwBound).toContain(`${HOME}/workspace`); + }); + + it('"--x" rules do not create --ro-bind-try mounts in restrictive mode', () => { + // Same as "---" case: execute-only rules also must not emit read mounts. + const config: AccessPolicyConfig = { + policy: { [`${HOME}/scripts/**`]: "--x" }, + }; + const args = generateBwrapArgs(config, HOME); + const roBound = args + .map((a, i) => (a === "--ro-bind-try" ? args[i + 1] : null)) + .filter(Boolean); + expect(roBound).not.toContain(`${HOME}/scripts`); + }); + + it('"-w-" rule in restrictive mode emits --bind-try so writes do not silently fail', () => { + // A write-only rule ("-w-") without "/**" now emits --bind-try so the path + // exists in the bwrap namespace and writes succeed. bwrap cannot enforce + // write-without-read at the mount level; reads are also permitted at the OS layer, + // but the tool layer still denies read tool calls per the "-w-" rule. + const config: AccessPolicyConfig = { + policy: { [`${HOME}/logs/**`]: "-w-" }, + }; + const args = generateBwrapArgs(config, HOME); + const bindMounts = args + .map((a, i) => (a === "--bind-try" ? args[i + 1] : null)) + .filter(Boolean); + expect(bindMounts).toContain(`${HOME}/logs`); + }); + + it('"-w-" rule in permissive mode emits --bind-try (write upgrade, reads already allowed)', () => { + // Under "/**":"r--", --ro-bind / / already grants reads everywhere. + // A "-w-" rule upgrades to rw for that path — reads are not newly leaked + // since the base already allowed them. + const config: AccessPolicyConfig = { + policy: { "/**": "r--", [`${HOME}/output/**`]: "-w-" }, + }; + const args = generateBwrapArgs(config, HOME); + const bindMounts = args + .map((a, i) => (a === "--bind-try" ? args[i + 1] : null)) + .filter(Boolean); + expect(bindMounts).toContain(`${HOME}/output`); + }); + + it("skips mid-path wildcard --- patterns — deny-all on truncated prefix would be too broad", () => { + // "/home/*/.config/**" with "---" truncates to "/home" — applying --tmpfs to /home + // would hide the entire home directory. Must be skipped. + const fakeHome = "/home/testuser"; + const config: AccessPolicyConfig = { + policy: { "/**": "r--", "/home/*/.config/**": "---" }, + }; + const args = generateBwrapArgs(config, fakeHome); + const allMountTargets = args + .map((a, i) => + ["--tmpfs", "--bind-try", "--ro-bind-try"].includes(args[i - 1] ?? "") ? a : null, + ) + .filter(Boolean); + expect(allMountTargets).not.toContain("/home"); + }); + + it("non-deny mid-path wildcard emits prefix as approximate mount target", () => { + // "scripts/**/*.sh": "r-x" — mid-path wildcard, non-deny perm. + // OS layer uses the concrete prefix (/scripts) as an approximate ro-bind-try target; + // the tool layer enforces the *.sh filter precisely. + const config: AccessPolicyConfig = { + policy: { "/scripts/**/*.sh": "r-x" }, + }; + const args = generateBwrapArgs(config, "/home/user"); + const allMountTargets = args + .map((a, i) => + ["--tmpfs", "--bind-try", "--ro-bind-try"].includes(args[i - 1] ?? "") ? a : null, + ) + .filter(Boolean); + expect(allMountTargets).toContain("/scripts"); + }); + + it("suffix-glob rule uses parent directory as mount target, not literal prefix", () => { + // "/var/log/secret*" must mount "/var/log", NOT the literal prefix "/var/log/secret" + // which is not a directory and leaves entries like "/var/log/secret.old" unprotected. + const config: AccessPolicyConfig = { + policy: { "/**": "r--", "/var/log/secret*": "---" }, + }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).toContain("/var/log"); + expect(tmpfsMounts).not.toContain("/var/log/secret"); + }); + + it("emits broader mounts before narrower ones so specific overrides win", () => { + // ~/dev/** is rw, ~/dev/secret/** is ro. The ro bind MUST come after the rw + // bind in the args so it takes precedence in bwrap's mount evaluation. + const config: AccessPolicyConfig = { + // Deliberately insert secret first so Object.entries() would yield it first + // without sorting — proving the sort is what fixes the order. + policy: { + [`${HOME}/dev/secret/**`]: "r--", + [`${HOME}/dev/**`]: "rw-", + }, + }; + const args = generateBwrapArgs(config, HOME); + const bindArgs = args.filter((a) => a === "--bind-try" || a === "--ro-bind-try"); + const bindPaths = args + .map((a, i) => (args[i - 1] === "--bind-try" || args[i - 1] === "--ro-bind-try" ? a : null)) + .filter(Boolean); + + const devIdx = bindPaths.indexOf(`${HOME}/dev`); + const secretIdx = bindPaths.indexOf(`${HOME}/dev/secret`); + // ~/dev (broader) must appear before ~/dev/secret (narrower). + expect(devIdx).toBeGreaterThanOrEqual(0); + expect(secretIdx).toBeGreaterThan(devIdx); + // And the types must be right. + expect(bindArgs[devIdx]).toBe("--bind-try"); + expect(bindArgs[secretIdx]).toBe("--ro-bind-try"); + }); + + it('script override "---" rule targeting a file does not emit --tmpfs (bwrap rejects file paths)', () => { + // The base-rules loop has an isDir guard before --tmpfs; the scriptOverrideRules loop must too. + // /etc/hosts is a real file; emitting --tmpfs /etc/hosts would make bwrap fail at runtime. + const config: AccessPolicyConfig = { policy: { "/**": "r--" } }; + const overrides = { "/etc/hosts": "---" as const }; + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + const args = generateBwrapArgs(config, HOME, overrides); + const tmpfsMounts = args + .map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)) + .filter(Boolean); + expect(tmpfsMounts).not.toContain("/etc/hosts"); + expect(spy).toHaveBeenCalledWith(expect.stringContaining("/etc/hosts")); + } finally { + spy.mockRestore(); + } + }); + + it('script override "---" rule targeting a non-existent path emits --tmpfs (assumed directory)', () => { + const config: AccessPolicyConfig = { policy: { "/**": "r--" } }; + const overrides = { "/nonexistent-path-for-test/**": "---" as const }; + const args = generateBwrapArgs(config, HOME, overrides); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).toContain("/nonexistent-path-for-test"); + }); + + it('script override "-w-" under restrictive base emits --bind-try, not --tmpfs', () => { + // Greptile: permAllowsWrite && (r || defaultR) condition was wrong — for -w- without /** + // both flags are false so it fell to else → --tmpfs, silently blocking writes. + // Fix: any write-granting override always emits --bind-try. + const config: AccessPolicyConfig = { + policy: { [`${HOME}/workspace/**`]: "rwx" }, + }; + const overrides = { [`${HOME}/logs/**`]: "-w-" as const }; + const args = generateBwrapArgs(config, HOME, overrides); + const bindMounts = args + .map((a, i) => (a === "--bind-try" ? args[i + 1] : null)) + .filter(Boolean); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(bindMounts).toContain(`${HOME}/logs`); + expect(tmpfsMounts).not.toContain(`${HOME}/logs`); + }); + + it("narrowing rule that resolves to an existing file does not emit --tmpfs (bwrap only accepts dirs)", () => { + // /etc/hosts is a file on Linux; bwrap --tmpfs rejects file paths. + // generateBwrapArgs must not emit "--tmpfs /etc/hosts" — it should be silently skipped. + const config: AccessPolicyConfig = { policy: { "/**": "r--", "/etc/hosts/**": "---" } }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).not.toContain("/etc/hosts"); + }); + + it("emits a console.error warning when a file-specific narrowing rule path is skipped", () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + _warnBwrapFileDenyOnce("/etc/passwd"); + expect(errSpy).toHaveBeenCalledWith(expect.stringContaining("/etc/passwd")); + expect(errSpy).toHaveBeenCalledWith(expect.stringContaining("parent directory")); + } finally { + errSpy.mockRestore(); + } + }); + + it('still emits --tmpfs for "---" rule that resolves to a directory', () => { + // Non-existent paths are treated as directories (forward-protection). + const config: AccessPolicyConfig = { + policy: { "/**": "r--", [`${HOME}/.nonexistent-dir/**`]: "---" }, + }; + const args = generateBwrapArgs(config, HOME); + const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean); + expect(tmpfsMounts).toContain(`${HOME}/.nonexistent-dir`); + }); + + it("trailing-slash rule is treated as /** and resolves to correct path", () => { + // "/tmp/" is shorthand for "/tmp/**" — must produce the same mount target + // and sort-order length as an explicit "/tmp/**" rule. + const withSlash = generateBwrapArgs({ policy: { "/tmp/": "rw-" } }, HOME); + const withGlob = generateBwrapArgs({ policy: { "/tmp/**": "rw-" } }, HOME); + const bindOf = (args: string[]) => + args.map((a, i) => (args[i - 1] === "--bind-try" ? a : null)).filter(Boolean); + expect(bindOf(withSlash)).toContain("/tmp"); + expect(bindOf(withSlash)).toEqual(bindOf(withGlob)); + }); + + it("malformed perm string in base rules emits no mount (fail closed, not --ro-bind-try)", () => { + // A malformed perm like "rwxoops" must not produce a --ro-bind-try mount. + // Previously the else-if branch accessed perm[0] without VALID_PERM_RE guard, + // which could emit --ro-bind-try for a rule meant to be restrictive. + const config: AccessPolicyConfig = { + policy: { + [`${HOME}/workspace/**`]: + "rwxoops" as unknown as import("../config/types.tools.js").PermStr, + }, + }; + const args = generateBwrapArgs(config, HOME); + const roBound = args + .map((a, i) => (a === "--ro-bind-try" ? args[i + 1] : null)) + .filter(Boolean); + const rwBound = args.map((a, i) => (a === "--bind-try" ? args[i + 1] : null)).filter(Boolean); + // Malformed perm must not produce any mount for this path. + expect(roBound).not.toContain(`${HOME}/workspace`); + expect(rwBound).not.toContain(`${HOME}/workspace`); + }); + + it("malformed perm string in script override emits no --ro-bind-try (fail closed)", () => { + // Same VALID_PERM_RE guard required in the scriptOverrideRules loop. + const config: AccessPolicyConfig = { policy: { "/**": "r--" } }; + const overrides = { + [`${HOME}/data/**`]: "rwxoops" as unknown as import("../config/types.tools.js").PermStr, + }; + const args = generateBwrapArgs(config, HOME, overrides); + const roBound = args + .map((a, i) => (a === "--ro-bind-try" ? args[i + 1] : null)) + .filter(Boolean); + const rwBound = args.map((a, i) => (a === "--bind-try" ? args[i + 1] : null)).filter(Boolean); + expect(roBound).not.toContain(`${HOME}/data`); + expect(rwBound).not.toContain(`${HOME}/data`); + }); +}); + +describe("wrapCommandWithBwrap", () => { + it("starts with bwrap", () => { + const result = wrapCommandWithBwrap("ls /tmp", { policy: { "/**": "r--" } }, HOME); + expect(result).toMatch(/^bwrap /); + }); + + it("contains -- separator before the command", () => { + const result = wrapCommandWithBwrap("ls /tmp", { policy: { "/**": "r--" } }, HOME); + expect(result).toContain("-- /bin/sh -c"); + }); + + it("wraps command in /bin/sh -c", () => { + const result = wrapCommandWithBwrap("cat /etc/hosts", { policy: { "/**": "r--" } }, HOME); + expect(result).toContain("/bin/sh -c"); + expect(result).toContain("cat /etc/hosts"); + }); +}); + +describe("_resetBwrapAvailableCacheForTest", () => { + it("clears the availability cache so isBwrapAvailable re-probes", async () => { + // Prime the cache with one result, then reset and verify the next call re-checks. + await isBwrapAvailable(); // populates cache + _resetBwrapAvailableCacheForTest(); + // After reset, isBwrapAvailable must re-probe (result may differ in test env — just + // verify it returns a boolean without throwing, proving the cache was cleared). + const result = await isBwrapAvailable(); + expect(typeof result).toBe("boolean"); + }); +}); + +describe("_resetBwrapFileDenyWarnedPathsForTest", () => { + it("clears the warned-paths set so the same path can warn again", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + // First warning — set is empty, should fire. + _warnBwrapFileDenyOnce("/tmp/secret.txt"); + expect(spy).toHaveBeenCalledTimes(1); + // Second call with same path — already warned, should NOT fire again. + _warnBwrapFileDenyOnce("/tmp/secret.txt"); + expect(spy).toHaveBeenCalledTimes(1); + // After reset the warning should fire again. + _resetBwrapFileDenyWarnedPathsForTest(); + _warnBwrapFileDenyOnce("/tmp/secret.txt"); + expect(spy).toHaveBeenCalledTimes(2); + spy.mockRestore(); + }); +}); diff --git a/src/infra/exec-sandbox-bwrap.ts b/src/infra/exec-sandbox-bwrap.ts new file mode 100644 index 00000000000..226ec4a009c --- /dev/null +++ b/src/infra/exec-sandbox-bwrap.ts @@ -0,0 +1,315 @@ +import { execFile } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import { promisify } from "node:util"; +import type { AccessPolicyConfig, PermStr } from "../config/types.tools.js"; +import { findBestRule } from "./access-policy.js"; +import { shellEscape } from "./shell-escape.js"; + +const execFileAsync = promisify(execFile); + +/** + * bwrap (bubblewrap) profile generator for Linux. + * + * Translates tools.fs.permissions into a mount-namespace spec so that exec + * commands see only the filesystem view defined by the policy. Denied paths + * are overlaid with an empty tmpfs — they appear to exist but contain nothing, + * preventing reads of sensitive files even when paths are expressed via + * variable expansion (cat $HOME/.ssh/id_rsa, etc.). + * + * Note: bwrap is not installed by default on all distributions. Use + * isBwrapAvailable() to check before calling generateBwrapArgs(). + */ + +// Standard system paths to bind read-only so wrapped commands can run. +// These are read-only unless the user's rules grant write access. +const SYSTEM_RO_BIND_PATHS = ["/usr", "/bin", "/lib", "/lib64", "/sbin", "/etc", "/opt"] as const; + +let bwrapAvailableCache: boolean | undefined; + +// Warn once per process when a file-specific "---" rule cannot be enforced at +// the OS layer (bwrap --tmpfs only accepts directories). Tool-layer enforcement +// still applies for read/write/edit tool calls, but exec commands that access +// the file directly inside the sandbox are not blocked at the syscall level. +const _bwrapFileDenyWarnedPaths = new Set(); +/** Reset the one-time file-deny warning set. Only for use in tests. */ +export function _resetBwrapFileDenyWarnedPathsForTest(): void { + _bwrapFileDenyWarnedPaths.clear(); +} +/** Reset the bwrap availability cache. Only for use in tests. */ +export function _resetBwrapAvailableCacheForTest(): void { + bwrapAvailableCache = undefined; +} +export function _warnBwrapFileDenyOnce(filePath: string): void { + if (_bwrapFileDenyWarnedPaths.has(filePath)) { + return; + } + _bwrapFileDenyWarnedPaths.add(filePath); + console.error( + `[access-policy] bwrap: "---" rule for "${filePath}" resolves to a file — ` + + `OS-level (bwrap) enforcement is not applied. ` + + `Tool-layer enforcement still blocks read/write/edit tool calls. ` + + `To protect this file at the OS layer on Linux, use a "---" rule on its parent directory instead.`, + ); +} + +/** + * Returns true if bwrap is installed and executable on this system. + * Result is cached after the first call. + */ +export async function isBwrapAvailable(): Promise { + if (bwrapAvailableCache !== undefined) { + return bwrapAvailableCache; + } + try { + await execFileAsync("bwrap", ["--version"]); + bwrapAvailableCache = true; + } catch { + bwrapAvailableCache = false; + } + return bwrapAvailableCache; +} + +/** Expand a leading ~ and trailing-slash shorthand (mirrors access-policy.ts expandPattern). */ +function expandPattern(pattern: string, homeDir: string): string { + // Trailing / shorthand: "/tmp/" → "/tmp/**" so sort-order length matches a + // "/tmp/**" rule and patternToPath strips it to "/tmp" correctly. + const normalized = pattern.endsWith("/") ? pattern + "**" : pattern; + if (!normalized.startsWith("~")) { + return normalized; + } + return normalized.replace(/^~(?=$|[/\\])/, homeDir); +} + +/** + * Strip trailing wildcard segments to get the longest concrete path prefix. + * e.g. "/Users/kaveri/**" → "/Users/kaveri" + * "/tmp/foo" → "/tmp/foo" + * + * For mid-path wildcards (e.g. "skills/**\/*.sh"), returns the concrete prefix + * when perm is not "---" — the prefix is an intentional approximation for bwrap + * mounts; the tool layer enforces the file-type filter precisely. For "---" perms + * returns null so callers skip emission (a deny-all on the prefix would be too broad). + */ +function patternToPath(pattern: string, homeDir: string, perm?: PermStr): string | null { + const expanded = expandPattern(pattern, homeDir); + // Find the first wildcard character in the path. + const wildcardIdx = expanded.search(/[*?[]/); + if (wildcardIdx === -1) { + // No wildcards — the pattern is a concrete path. + return expanded || "/"; + } + // Check whether there is a path separator AFTER the first wildcard. + // If so, the wildcard is in a non-final segment (e.g. skills/**/*.sh). + const afterWildcard = expanded.slice(wildcardIdx); + if (/[/\\]/.test(afterWildcard)) { + // Mid-path wildcard: for "---" perm a deny-all on the prefix is too broad — skip. + // For other perms, use the prefix as an approximate mount target; the tool layer + // enforces the file-type filter precisely. + if (!perm || perm === "---") { + return null; + } + // Fall through to use the concrete prefix below. + } + // Wildcard is only in the final segment — use the parent directory. + // e.g. "/var/log/secret*" → last sep before "*" is at 8 → "/var/log" + // We must NOT use the literal prefix up to "*" (e.g. "/var/log/secret") + // because that is not a directory and leaves suffix-glob matches uncovered. + const lastSep = expanded.lastIndexOf("/", wildcardIdx - 1); + const parentDir = lastSep > 0 ? expanded.slice(0, lastSep) : "/"; + return parentDir || "/"; +} + +// Keep in sync with VALID_PERM_RE in access-policy.ts and exec-sandbox-seatbelt.ts. +const VALID_PERM_RE = /^[r-][w-][x-]$/; + +function permAllowsWrite(perm: PermStr): boolean { + return VALID_PERM_RE.test(perm) && perm[1] === "w"; +} + +/** + * Generate bwrap argument array for the given permissions config. + * + * Strategy: + * 1. Check the "/**" rule to determine permissive vs restrictive base. + * 2. Permissive base (r in "/**"): --ro-bind / / (read-only view of entire host FS). + * 3. Restrictive base (no r in "/**"): only bind system paths needed to run processes. + * 4. For each rule with w bit: upgrade to --bind (read-write). + * 5. For each "---" rule in permissive mode: overlay with --tmpfs to hide the path. + * 6. Add /tmp and /dev as writable tmpfs mounts (required for most processes). + */ +export function generateBwrapArgs( + config: AccessPolicyConfig, + homeDir: string = os.homedir(), + /** + * Script-specific override rules to emit AFTER the base rules so they win over + * broader patterns — mirrors the Seatbelt scriptOverrideRules behaviour. + * In bwrap, later mounts win, so script grants must come last. + */ + scriptOverrideRules?: Record, +): string[] { + const args: string[] = []; + // Determine base stance from the "/**" catch-all rule (replaces the removed `default` field). + const rawCatchAllPerm = findBestRule("/**", config.policy ?? {}, homeDir) ?? "---"; + // Validate format before positional access — malformed perm falls back to "---" (fail closed). + const catchAllPerm = VALID_PERM_RE.test(rawCatchAllPerm) ? rawCatchAllPerm : "---"; + const defaultAllowsRead = catchAllPerm[0] === "r"; + + if (defaultAllowsRead) { + // Permissive base: everything is read-only by default. + args.push("--ro-bind", "/", "/"); + // --ro-bind / / is a recursive bind but does NOT propagate special kernel + // filesystems (procfs, devtmpfs) into the new mount namespace. Explicitly + // mount /proc so programs that read /proc/self/*, /proc/cpuinfo, etc. work + // correctly inside the sandbox (shells, Python, most build tools need this). + args.push("--proc", "/proc"); + args.push("--dev", "/dev"); + } else { + // Restrictive base: only bind system paths needed to run processes. + for (const p of SYSTEM_RO_BIND_PATHS) { + args.push("--ro-bind-try", p, p); + } + // proc and dev are needed for most processes. + args.push("--proc", "/proc"); + args.push("--dev", "/dev"); + // /tmp is intentionally NOT mounted here — a restrictive policy (default:"---") + // should not grant free write access to /tmp. Add a rule "/tmp/**": "rw-" if + // the enclosed process genuinely needs it. + } + + // Writable /tmp tmpfs — only in permissive mode AND only when the policy does not + // explicitly restrict writes on /tmp. Keeping this outside the if/else block above + // makes the defaultAllowsRead guard self-evident and not implicit from nesting. + // In restrictive mode (default:"---"), /tmp is intentionally omitted so rules + // control tmpfs access explicitly (e.g. "/tmp/**":"rwx" is handled by the rules loop). + if (defaultAllowsRead) { + const explicitTmpPerm = findBestRule("/tmp", config.policy ?? {}, homeDir); + if (explicitTmpPerm === null) { + // Only emit --tmpfs /tmp when there is no explicit rule for /tmp. + // When an explicit write rule exists, the rules loop below emits --bind-try /tmp /tmp + // which (as a later mount) wins over --tmpfs anyway — emitting --tmpfs here too + // is dead code. When an explicit read-only rule exists, /tmp is already readable + // via --ro-bind / / and no extra mount is needed. + args.push("--tmpfs", "/tmp"); + } + } + + // Apply rules: upgrade paths with w bit to read-write binds. + // Sort by concrete path length ascending so less-specific mounts are applied + // first — bwrap applies mounts in order, and later mounts win for overlapping + // paths. Without sorting, a broad rw bind (e.g. ~/dev) could be emitted after + // a narrow ro bind (~/dev/secret), wiping out the intended restriction. + const ruleEntries = Object.entries(config.policy ?? {}).toSorted(([a], [b]) => { + const pa = patternToPath(a, homeDir); + const pb = patternToPath(b, homeDir); + return (pa?.length ?? 0) - (pb?.length ?? 0); + }); + for (const [pattern, perm] of ruleEntries) { + const p = patternToPath(pattern, homeDir, perm); + if (!p || p === "/") { + continue; + } // root already handled above + if (permAllowsWrite(perm)) { + // Emit --bind-try for any rule that permits writes, including write-only ("-w-"). + // bwrap cannot enforce write-without-read at the mount level; a "-w-" rule under + // a restrictive base will also permit reads at the OS layer. The tool layer still + // denies read tool calls per the rule, so the practical exposure is exec-only paths. + args.push("--bind-try", p, p); + } else if (VALID_PERM_RE.test(perm) && catchAllPerm[0] !== "r" && perm[0] === "r") { + // Restrictive base: only bind paths that the rule explicitly allows reads on. + // Do NOT emit --ro-bind-try for "---" or "--x" rules — the base already denies + // by not mounting; emitting a mount here would grant read access. + // VALID_PERM_RE guard: malformed perm falls through to no-op (fail closed). + args.push("--ro-bind-try", p, p); + } else if (VALID_PERM_RE.test(perm) && perm[0] !== "r") { + // Deny/exec-only rule: overlay with --tmpfs to hide the path. + // Two cases handled identically: + // Permissive base (catchAllPerm[0] === "r"): --ro-bind / / made path readable; + // --tmpfs hides it. + // Restrictive base (catchAllPerm[0] !== "r"): SYSTEM_RO_BIND_PATHS unconditionally + // mounts /etc, /usr, /bin, /lib, /lib64, /sbin, /opt; a "---" rule on those paths + // had no effect without this branch because the three prior branches all require + // perm[0] === "r". For non-system paths in restrictive mode, --tmpfs is a no-op + // (nothing mounted there to overlay), so emitting it is harmless. + // Guard: bwrap --tmpfs only accepts a directory as the mount point. If the + // resolved path is a file, skip the mount and warn — same behaviour as deny[]. + // Non-existent paths are assumed to be directories (forward-protection). + let isDir = true; + try { + isDir = fs.statSync(p).isDirectory(); + } catch { + // Non-existent — assume directory. + } + if (isDir) { + args.push("--tmpfs", p); + } else { + _warnBwrapFileDenyOnce(p); + } + } + // Permissive base + read-only rule: already covered by --ro-bind / /; no extra mount. + // Restrictive base + read-only rule: emitted as --ro-bind-try above. + } + + // Script-specific override mounts — emitted after base rules so they can reopen + // a base-denied path for a trusted script (same precedence as Seatbelt). + if (scriptOverrideRules) { + const overrideEntries = Object.entries(scriptOverrideRules).toSorted(([a], [b]) => { + const pa = patternToPath(a, homeDir); + const pb = patternToPath(b, homeDir); + return (pa?.length ?? 0) - (pb?.length ?? 0); + }); + for (const [pattern, perm] of overrideEntries) { + const p = patternToPath(pattern, homeDir, perm); + if (!p || p === "/") { + continue; + } + if (permAllowsWrite(perm)) { + // Any write-granting override always needs --bind-try so the path exists + // and writes succeed. bwrap mounts are ordered; this override comes after + // deny[] tmpfs entries, so --bind-try wins regardless of the base policy. + args.push("--bind-try", p, p); + } else if (VALID_PERM_RE.test(perm) && perm[0] === "r") { + // VALID_PERM_RE guard: malformed perm falls through to the deny branch below. + args.push("--ro-bind-try", p, p); + } else { + // Mirror the base-rules isDir guard — bwrap --tmpfs only accepts directories. + let isDir = true; + try { + isDir = fs.statSync(p).isDirectory(); + } catch { + // Non-existent — assume directory (forward-protection). + } + if (isDir) { + args.push("--tmpfs", p); + } else { + _warnBwrapFileDenyOnce(p); + } + } + } + } + + // Separator before the command. + args.push("--"); + + return args; +} + +/** + * Wrap a shell command with bwrap using the given permissions config. + * Returns the wrapped command string ready to pass as execCommand. + */ +export function wrapCommandWithBwrap( + command: string, + config: AccessPolicyConfig, + homeDir: string = os.homedir(), + scriptOverrideRules?: Record, +): string { + const bwrapArgs = generateBwrapArgs(config, homeDir, scriptOverrideRules); + const argStr = bwrapArgs.map((a) => (a === "--" ? "--" : shellEscape(a))).join(" "); + // /bin/sh is intentional: sandboxed commands must use a shell whose path is + // within the bwrap mount namespace. The user's configured shell (getShellConfig) + // may live outside the mounted paths (e.g. /opt/homebrew/bin/fish) and would + // not be reachable inside the sandbox. /bin/sh is always available via + // SYSTEM_RO_BIND_PATHS or the permissive --ro-bind / / base mount. + return `bwrap ${argStr} /bin/sh -c ${shellEscape(command)}`; +} diff --git a/src/infra/exec-sandbox-seatbelt.test.ts b/src/infra/exec-sandbox-seatbelt.test.ts new file mode 100644 index 00000000000..8b00b4dd955 --- /dev/null +++ b/src/infra/exec-sandbox-seatbelt.test.ts @@ -0,0 +1,323 @@ +import os from "node:os"; +import { describe, expect, it } from "vitest"; + +// Seatbelt (SBPL) path handling uses Unix forward-slash semantics. +// Tests that assert specific HOME paths in the profile are skipped on Windows +// where os.homedir() returns backslash paths that the generator does not emit. +const skipOnWindows = it.skipIf(process.platform === "win32"); +import type { AccessPolicyConfig } from "../config/types.tools.js"; +import { generateSeatbeltProfile, wrapCommandWithSeatbelt } from "./exec-sandbox-seatbelt.js"; + +const HOME = os.homedir(); + +describe("generateSeatbeltProfile", () => { + it("starts with (version 1)", () => { + const profile = generateSeatbeltProfile({}, HOME); + expect(profile).toMatch(/^\(version 1\)/); + }); + + it("uses (deny default) when no /** catch-all rule", () => { + const profile = generateSeatbeltProfile({}, HOME); + expect(profile).toContain("(deny default)"); + expect(profile).not.toContain("(allow default)"); + }); + + it("uses (allow default) when /** rule has any permission", () => { + const profile = generateSeatbeltProfile({ policy: { "/**": "r--" } }, HOME); + expect(profile).toContain("(allow default)"); + expect(profile).not.toContain("(deny default)"); + }); + + it("includes system baseline reads when no catch-all rule", () => { + const profile = generateSeatbeltProfile({}, HOME); + expect(profile).toContain("(allow file-read*"); + expect(profile).toContain("/usr/lib"); + expect(profile).toContain("/System/Library"); + }); + + skipOnWindows("--- rule emits deny file-read*, file-write*, process-exec* for that path", () => { + const config: AccessPolicyConfig = { + policy: { "/**": "rwx", [`${HOME}/.ssh/**`]: "---" }, + }; + const profile = generateSeatbeltProfile(config, HOME); + expect(profile).toContain(`(deny file-read*`); + expect(profile).toContain(`(deny file-write*`); + expect(profile).toContain(`(deny process-exec*`); + expect(profile).toContain(HOME + "/.ssh"); + }); + + skipOnWindows("expands ~ in --- rules using provided homeDir", () => { + const config: AccessPolicyConfig = { + policy: { "/**": "rwx", "~/.ssh/**": "---" }, + }; + const profile = generateSeatbeltProfile(config, HOME); + expect(profile).toContain(HOME + "/.ssh"); + expect(profile).not.toContain("~/.ssh"); + }); + + skipOnWindows("expands ~ in rules using provided homeDir", () => { + const config: AccessPolicyConfig = { + policy: { "~/**": "rw-" }, + }; + const profile = generateSeatbeltProfile(config, HOME); + expect(profile).toContain(HOME); + }); + + it("rw- rule emits allow read+write, deny exec for that path", () => { + const config: AccessPolicyConfig = { + policy: { [`${HOME}/workspace/**`]: "rw-" }, + }; + const profile = generateSeatbeltProfile(config, HOME); + expect(profile).toContain(`(allow file-read*`); + expect(profile).toContain(`(allow file-write*`); + expect(profile).toContain(`(deny process-exec*`); + }); + + it("r-x rule emits allow read+exec, deny write for that path", () => { + const config: AccessPolicyConfig = { + policy: { "/usr/bin/**": "r-x" }, + }; + const profile = generateSeatbeltProfile(config, HOME); + const rulesSection = profile.split("; User-defined path rules")[1] ?? ""; + expect(rulesSection).toContain("(allow file-read*"); + expect(rulesSection).toContain("(allow process-exec*"); + expect(rulesSection).toContain("(deny file-write*"); + }); + + it("narrowing --- rule appears after broader allow rule in profile", () => { + // SBPL last-match-wins: the --- rule for .ssh must appear after the broader rwx rule. + const config: AccessPolicyConfig = { + policy: { [`${HOME}/**`]: "rwx", [`${HOME}/.ssh/**`]: "---" }, + }; + const profile = generateSeatbeltProfile(config, HOME); + const rulesIdx = profile.indexOf("; User-defined path rules"); + expect(rulesIdx).toBeGreaterThan(-1); + // The broader allow must appear before the narrowing deny. + const allowIdx = profile.indexOf("(allow file-read*"); + const denyIdx = profile.lastIndexOf("(deny file-read*"); + expect(allowIdx).toBeGreaterThan(-1); + expect(denyIdx).toBeGreaterThan(allowIdx); + }); + + it("handles empty config without throwing", () => { + expect(() => generateSeatbeltProfile({}, HOME)).not.toThrow(); + }); + + it("permissive base with no exec bit includes system baseline exec paths", () => { + // "/**": "r--" emits (deny process-exec* (subpath "/")) but must also allow + // system binaries — otherwise ls, grep, cat all fail inside the sandbox. + const profile = generateSeatbeltProfile({ policy: { "/**": "r--" } }, HOME); + expect(profile).toContain("(allow process-exec*"); + expect(profile).toContain("/bin"); + expect(profile).toContain("/usr/bin"); + }); + + it("permissive base with exec bit does NOT add redundant exec baseline", () => { + // "/**": "rwx" already allows everything including exec — no extra baseline needed. + const profile = generateSeatbeltProfile({ policy: { "/**": "rwx" } }, HOME); + expect(profile).toContain("(allow default)"); + expect(profile).not.toContain("System baseline exec"); + }); + + skipOnWindows("script-override narrowing emits deny ops so access is actually reduced", () => { + // Base allows rw- on workspace; script override narrows to r-- for a subpath. + // Without deny ops in the override block, write would still be allowed. + const config: AccessPolicyConfig = { + policy: { [`${HOME}/workspace/**`]: "rw-" }, + }; + const overrideRules: Record = { [`${HOME}/workspace/locked/**`]: "r--" }; + const profile = generateSeatbeltProfile(config, HOME, overrideRules); + // The override section must deny write for the locked path. + const overrideSection = profile.split("Script-override")[1] ?? ""; + expect(overrideSection).toContain("(deny file-write*"); + expect(overrideSection).toContain(`${HOME}/workspace/locked`); + }); + + it("omits /private/tmp baseline when no rule grants /tmp", () => { + const profile = generateSeatbeltProfile({}, HOME); + expect(profile).not.toContain(`(subpath "/private/tmp")`); + }); + + it("includes /private/tmp baseline when a rule grants read access to /tmp", () => { + const profile = generateSeatbeltProfile({ policy: { "/tmp/**": "rw-" } }, HOME); + expect(profile).toContain(`(subpath "/private/tmp")`); + }); + + it("read-only /tmp rule does not grant file-write* on /private/tmp", () => { + const profile = generateSeatbeltProfile({ policy: { "/tmp/**": "r--" } }, HOME); + expect(profile).toMatch(/allow file-read[^)]*\(subpath "\/private\/tmp"\)/); + expect(profile).not.toMatch(/allow file-write\*[^)]*\(subpath "\/private\/tmp"\)/); + }); + + it("write-only /tmp rule grants file-write* but not read ops on /private/tmp", () => { + const profile = generateSeatbeltProfile({ policy: { "/tmp/**": "-w-" } }, HOME); + expect(profile).toMatch(/allow file-write\*[^)]*\(subpath "\/private\/tmp"\)/); + expect(profile).not.toMatch(/allow file-read[^)]*\(subpath "\/private\/tmp"\)/); + }); + + it("exec-only /tmp rule grants process-exec* on /private/tmp", () => { + const profile = generateSeatbeltProfile({ policy: { "/tmp/**": "--x" } }, HOME); + expect(profile).toMatch(/allow process-exec\*[^)]*\(subpath "\/private\/tmp"\)/); + expect(profile).not.toMatch(/allow file-read[^)]*\(subpath "\/private\/tmp"\)/); + expect(profile).not.toMatch(/allow file-write\*[^)]*\(subpath "\/private\/tmp"\)/); + }); + + it("r-x /tmp rule grants both read and exec on /private/tmp", () => { + const profile = generateSeatbeltProfile({ policy: { "/tmp/**": "r-x" } }, HOME); + expect(profile).toMatch(/allow file-read[^)]*\(subpath "\/private\/tmp"\)/); + expect(profile).toMatch(/allow process-exec\*[^)]*\(subpath "\/private\/tmp"\)/); + expect(profile).not.toMatch(/allow file-write\*[^)]*\(subpath "\/private\/tmp"\)/); + }); + + // --------------------------------------------------------------------------- + // Symlink attack mitigation — profile ordering + // + // macOS Seatbelt evaluates the *resolved* (real) path at syscall time, not + // the symlink path. So a symlink in an allowed directory pointing to a denied + // target is blocked by the deny rule for the real path — but only if that + // deny rule appears AFTER the allow rule for the workspace in the profile + // (SBPL: last matching rule wins). + // --------------------------------------------------------------------------- + + skipOnWindows( + "--- rule for sensitive path appears after workspace allow — symlink to deny target is blocked", + () => { + // If ~/workspace/link → ~/.ssh/id_rsa, seatbelt evaluates ~/.ssh/id_rsa. + // The --- rule for ~/.ssh must appear after the workspace allow so it wins. + const config: AccessPolicyConfig = { + policy: { + [`${HOME}/workspace/**`]: "rw-", + [`${HOME}/.ssh/**`]: "---", + }, + }; + const profile = generateSeatbeltProfile(config, HOME); + const workspaceAllowIdx = profile.indexOf("(allow file-read*"); + const sshDenyIdx = profile.lastIndexOf("(deny file-read*"); + expect(workspaceAllowIdx).toBeGreaterThan(-1); + expect(sshDenyIdx).toBeGreaterThan(workspaceAllowIdx); + expect(profile).toContain(`${HOME}/.ssh`); + expect(profile).toContain(`${HOME}/workspace`); + }, + ); + + skipOnWindows( + "restrictive rule on subdir appears after broader rw rule — covers symlink to restricted subtree", + () => { + const config: AccessPolicyConfig = { + policy: { + [`${HOME}/workspace/**`]: "rw-", + [`${HOME}/workspace/secret/**`]: "r--", + }, + }; + const profile = generateSeatbeltProfile(config, HOME); + const workspaceWriteIdx = profile.indexOf("(allow file-write*"); + const secretWriteDenyIdx = profile.lastIndexOf("(deny file-write*"); + expect(workspaceWriteIdx).toBeGreaterThan(-1); + expect(secretWriteDenyIdx).toBeGreaterThan(workspaceWriteIdx); + expect(profile).toContain(`${HOME}/workspace/secret`); + }, + ); + + it("glob patterns are stripped to their longest concrete prefix", () => { + const config: AccessPolicyConfig = { + policy: { "/Users/kaveri/.ssh/**": "---", "/**": "rwx" }, + }; + const profile = generateSeatbeltProfile(config, "/Users/kaveri"); + expect(profile).not.toContain("**"); + expect(profile).toContain("/Users/kaveri/.ssh"); + }); + + it("bracket glob patterns are treated as wildcards, not SBPL literals", () => { + // Previously /[*?]/ missed [ — a pattern like "/usr/bin/[abc]" was emitted as + // sbplLiteral("/usr/bin/[abc]") which only matches a file literally named "[abc]". + // Fix: /[*?[]/ detects bracket globs and strips to the concrete prefix. + const config: AccessPolicyConfig = { + policy: { "/**": "rwx", "/usr/bin/[abc]": "---" as const }, + }; + const profile = generateSeatbeltProfile(config, HOME); + // Must NOT emit the literal bracket pattern. + expect(profile).not.toContain("[abc]"); + // Must use the concrete prefix /usr/bin as an approximate subpath target. + expect(profile).toContain("/usr/bin"); + }); +}); + +describe("wrapCommandWithSeatbelt", () => { + it("wraps command with sandbox-exec -f ", () => { + const result = wrapCommandWithSeatbelt("ls /tmp", "(version 1)\n(allow default)"); + expect(result).toMatch(/^sandbox-exec -f /); + expect(result).toContain("ls /tmp"); + // Profile content is in a temp file, not inline — not visible in ps output. + expect(result).not.toContain("(version 1)"); + }); + + it("profile file is not embedded in the command string", () => { + const result = wrapCommandWithSeatbelt("echo hi", "(allow default) ; it's a test"); + expect(result).not.toContain("it's a test"); + expect(result).toContain("openclaw-sb-"); + }); + + it("uses a distinct profile file per call to avoid concurrent-exec policy races", () => { + const r1 = wrapCommandWithSeatbelt("echo 1", "(allow default)"); + const r2 = wrapCommandWithSeatbelt("echo 2", "(allow default)"); + // Each call must get its own file so overlapping execs with different profiles don't race. + const extract = (cmd: string) => cmd.match(/-f (\S+)/)?.[1]; + expect(extract(r1)).not.toBe(extract(r2)); + expect(extract(r1)).toContain("openclaw-sb-"); + expect(extract(r2)).toContain("openclaw-sb-"); + }); + + it("wraps command in /bin/sh -c", () => { + const result = wrapCommandWithSeatbelt("cat /etc/hosts", "(allow default)"); + expect(result).toContain("/bin/sh -c"); + }); + + it("profile file path contains a random component (not just pid+seq)", () => { + const extract = (cmd: string) => cmd.match(/-f (\S+)/)?.[1] ?? ""; + const r1 = wrapCommandWithSeatbelt("echo 1", "(allow default)"); + const r2 = wrapCommandWithSeatbelt("echo 2", "(allow default)"); + // Path must be unpredictable — strip the pid prefix and check the random suffix varies. + const suffix = (p: string) => p.replace(/.*openclaw-sb-\d+-/, "").replace(".sb", ""); + expect(suffix(extract(r1))).not.toBe(suffix(extract(r2))); + expect(suffix(extract(r1)).length).toBeGreaterThanOrEqual(8); // at least 4 random bytes + }); +}); + +describe("generateSeatbeltProfile — mid-path wildcard guard", () => { + skipOnWindows( + "non-deny mid-path wildcard emits prefix subpath for r/w but NOT exec (option B)", + () => { + // skills/**/*.sh: r-x — mid-path wildcard; OS prefix is /home. + // Read is emitted (bounded over-grant). Exec is omitted — granting exec on the + // entire prefix would allow arbitrary binaries to run, not just *.sh files. + // Exec falls through to ancestor rule; tool layer enforces it precisely. + const profile = generateSeatbeltProfile({ policy: { "/home/*/workspace/**": "r-x" } }, HOME); + const rulesSection = profile.split("; User-defined path rules")[1] ?? ""; + expect(rulesSection).toContain("(allow file-read*"); + expect(rulesSection).toContain('(subpath "/home")'); + expect(rulesSection).not.toContain("(allow process-exec*"); + // exec deny also omitted — falls through to ancestor + expect(rulesSection).not.toContain("(deny process-exec*"); + }, + ); + + skipOnWindows("--- mid-path wildcard is skipped (deny-all on prefix would be too broad)", () => { + // A deny-all on the /home prefix would block the entire home directory — too broad. + const profile = generateSeatbeltProfile({ policy: { "/home/*/workspace/**": "---" } }, HOME); + expect(profile).not.toContain('(subpath "/home")'); + }); + + skipOnWindows("still emits trailing-** rules that have no mid-path wildcard", () => { + const profile = generateSeatbeltProfile({ policy: { "/tmp/**": "rwx" } }, HOME); + expect(profile).toContain('(subpath "/tmp")'); + }); + + skipOnWindows("? wildcard is stripped correctly — no literal ? in SBPL matcher", () => { + // Pattern "/tmp/file?.txt" has a ? wildcard; the strip regex must remove it so + // the SBPL matcher does not contain a raw "?" character. Stripping "?.txt" from + // "/tmp/file?.txt" yields "/tmp/file" — a more precise subpath than "/tmp". + const profile = generateSeatbeltProfile({ policy: { "/tmp/file?.txt": "r--" } }, HOME); + expect(profile).not.toMatch(/\?/); // no literal ? in the emitted profile + expect(profile).toContain('(subpath "/tmp/file")'); + }); +}); diff --git a/src/infra/exec-sandbox-seatbelt.ts b/src/infra/exec-sandbox-seatbelt.ts new file mode 100644 index 00000000000..9c0f4477911 --- /dev/null +++ b/src/infra/exec-sandbox-seatbelt.ts @@ -0,0 +1,391 @@ +import crypto from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import type { AccessPolicyConfig, PermStr } from "../config/types.tools.js"; +import { findBestRule } from "./access-policy.js"; +import { shellEscape } from "./shell-escape.js"; + +/** + * Seatbelt (SBPL) profile generator for macOS sandbox-exec. + * + * Translates tools.fs.permissions into a Seatbelt profile so that exec commands + * run under OS-level path enforcement — catching variable-expanded paths like + * `cat $HOME/.ssh/id_rsa` that config-level heuristics cannot intercept. + * + * Precedence in generated profiles (matches AccessPolicyConfig semantics): + * 1. deny[] entries — placed last, always override rules. + * 2. rules — sorted shortest-to-longest so more specific rules overwrite broader ones. + * 3. System baseline — allows the process to load libraries and basic OS resources. + * 4. default — sets the base allow/deny for everything else. + */ + +// SBPL operation names for each permission bit. +const SEATBELT_READ_OPS = "file-read*"; +const SEATBELT_WRITE_OPS = "file-write*"; +const SEATBELT_EXEC_OPS = "process-exec*"; + +// System paths every process needs to function (dynamic linker, stdlib, etc.). +// These are allowed for file-read* regardless of user rules so wrapped commands +// don't break when default is "---". +const SYSTEM_BASELINE_READ_PATHS = [ + "/usr/lib", + "/usr/share", + "/System/Library", + "/Library/Frameworks", + "/private/var/db/timezone", + "/dev/null", + "/dev/random", + "/dev/urandom", + "/dev/fd", +] as const; + +const SYSTEM_BASELINE_EXEC_PATHS = [ + "/bin", + "/usr/bin", + "/usr/libexec", + "/System/Library/Frameworks", +] as const; + +function escapeSubpath(p: string): string { + // SBPL strings use double-quote delimiters; escape embedded quotes and backslashes. + return p.replace(/\\/g, "\\\\").replace(/"/g, '\\"'); +} + +function sbplSubpath(p: string): string { + return `(subpath "${escapeSubpath(p)}")`; +} + +function sbplLiteral(p: string): string { + return `(literal "${escapeSubpath(p)}")`; +} + +/** + * Resolve a path pattern to a concrete path for SBPL. + * Glob wildcards (**) are stripped to their longest non-wildcard prefix + * since SBPL uses subpath/literal matchers, not globs. + * e.g. "/Users/kaveri/**" → subpath("/Users/kaveri") + * "/usr/bin/grep" → literal("/usr/bin/grep") + */ +// macOS /private/* aliases — when a pattern covers /tmp, /var, or /etc we must +// also emit the /private/* form so seatbelt (which sees real paths) matches. +const SBPL_ALIAS_PAIRS: ReadonlyArray<[alias: string, real: string]> = [ + ["/tmp", "/private/tmp"], + ["/var", "/private/var"], + ["/etc", "/private/etc"], +]; + +/** + * Expand a pattern to include the /private/* equivalent if it starts with a + * known macOS alias. Returns [original, ...extras] — the extra entries are + * emitted as additional SBPL rules alongside the original. + */ +function expandSbplAliases(pattern: string): string[] { + for (const [alias, real] of SBPL_ALIAS_PAIRS) { + if (pattern === alias) { + return [pattern, real]; + } + if (pattern.startsWith(alias + "/")) { + return [pattern, real + pattern.slice(alias.length)]; + } + } + return [pattern]; +} + +type SbplMatchResult = + | { matcher: string; approximate: false } + | { matcher: string; approximate: true } // mid-path wildcard — exec bit must be skipped + | null; + +function patternToSbplMatcher(pattern: string, homeDir: string, perm?: PermStr): SbplMatchResult { + // Trailing / shorthand: "/tmp/" → "/tmp/**" + const withExpanded = pattern.endsWith("/") ? pattern + "**" : pattern; + const expanded = withExpanded.startsWith("~") + ? withExpanded.replace(/^~(?=$|[/\\])/, homeDir) + : withExpanded; + + // Strip trailing wildcard segments to get the longest concrete prefix. + // Both * and ? are wildcard characters in glob syntax; strip from whichever + // appears first so patterns like "/tmp/file?.txt" don't embed a literal ? + // in the SBPL literal matcher. + // Strip from the first glob metacharacter (*, ?, or [) to get the longest concrete prefix. + const withoutWild = expanded.replace(/[/\\]?[*?[].*$/, ""); + const base = withoutWild || "/"; + + // If the original pattern had wildcards, use subpath (recursive match). + // Includes bracket globs ([abc]) — previously only * and ? were detected, + // causing [abc] to be emitted as an SBPL literal that only matches a file + // literally named "[abc]", not the intended character-class targets. + if (/[*?[]/.test(expanded)) { + const wildcardIdx = expanded.search(/[*?[]/); + const afterWildcard = expanded.slice(wildcardIdx + 1); + if (/[/\\]/.test(afterWildcard)) { + // Mid-path wildcard (e.g. skills/**/*.sh): SBPL has no glob matcher so we fall + // back to the longest concrete prefix as a subpath. + // "---" → skip entirely: deny-all on the prefix is too broad. + // Other perms → emit prefix with approximate=true so callers omit the exec bit. + // Granting exec on the prefix would allow arbitrary binaries under the directory + // to be executed by subprocesses, not just files matching the original pattern. + // Read/write on the prefix are acceptable approximations; exec is not. + // The exec bit for mid-path patterns is enforced by the tool layer only. + if (!perm || perm === "---") { + return null; + } + return { matcher: sbplSubpath(base), approximate: true }; + } + return { matcher: sbplSubpath(base), approximate: false }; + } + return { matcher: sbplLiteral(base), approximate: false }; +} + +// Keep in sync with VALID_PERM_RE in access-policy.ts and exec-sandbox-bwrap.ts. +const VALID_PERM_RE = /^[r-][w-][x-]$/; + +function permToOps(perm: PermStr): string[] { + if (!VALID_PERM_RE.test(perm)) { + return []; + } + const ops: string[] = []; + if (perm[0] === "r") { + ops.push(SEATBELT_READ_OPS); + } + if (perm[1] === "w") { + ops.push(SEATBELT_WRITE_OPS); + } + if (perm[2] === "x") { + ops.push(SEATBELT_EXEC_OPS); + } + return ops; +} + +function deniedOps(perm: PermStr): string[] { + // Malformed perm — deny everything (fail closed). + if (!VALID_PERM_RE.test(perm)) { + return [SEATBELT_READ_OPS, SEATBELT_WRITE_OPS, SEATBELT_EXEC_OPS]; + } + const ops: string[] = []; + if (perm[0] !== "r") { + ops.push(SEATBELT_READ_OPS); + } + if (perm[1] !== "w") { + ops.push(SEATBELT_WRITE_OPS); + } + if (perm[2] !== "x") { + ops.push(SEATBELT_EXEC_OPS); + } + return ops; +} + +/** + * Generate a Seatbelt (SBPL) profile string from an AccessPolicyConfig. + * + * @param config The fs permissions config. + * @param homeDir The OS home directory (os.homedir()) used to expand ~. + */ +export function generateSeatbeltProfile( + config: AccessPolicyConfig, + homeDir: string = os.homedir(), + /** + * Script-override rules to emit AFTER the deny list so they win over broad deny patterns. + * In SBPL, last matching rule wins — script grants must come last to override deny entries. + */ + scriptOverrideRules?: Record, +): string { + const lines: string[] = []; + + lines.push("(version 1)"); + lines.push(""); + + // Determine base stance from the "/**" catch-all rule (replaces the removed `default` field). + const rawCatchAllPerm = findBestRule("/**", config.policy ?? {}, homeDir) ?? "---"; + // Validate format before positional access — malformed perm falls back to "---" (fail closed). + const catchAllPerm = VALID_PERM_RE.test(rawCatchAllPerm) ? rawCatchAllPerm : "---"; + const defaultPerm = catchAllPerm; // alias for readability below + const defaultAllowsAnything = + catchAllPerm[0] === "r" || catchAllPerm[1] === "w" || catchAllPerm[2] === "x"; + + if (defaultAllowsAnything) { + // Permissive base: allow everything, then restrict. + lines.push("(allow default)"); + // Deny operations not in the default perm string. + for (const op of deniedOps(defaultPerm)) { + lines.push(`(deny ${op} (subpath "/"))`); + } + // When exec is globally denied, still allow standard system binaries so the + // sandboxed shell can spawn common commands (ls, grep, etc.). Without this, + // `default: "r--"` silently breaks all subprocess execution. + if (defaultPerm[2] !== "x") { + lines.push(""); + lines.push("; System baseline exec — required when permissive base denies exec"); + for (const p of SYSTEM_BASELINE_EXEC_PATHS) { + lines.push(`(allow ${SEATBELT_EXEC_OPS} ${sbplSubpath(p)})`); + } + } + } else { + // Restrictive base: deny everything, then allow selectively. + lines.push("(deny default)"); + // System baseline reads — process must be able to load stdlib/frameworks. + lines.push(""); + lines.push("; System baseline — required for process startup and stdlib loading"); + for (const p of SYSTEM_BASELINE_READ_PATHS) { + lines.push(`(allow ${SEATBELT_READ_OPS} ${sbplSubpath(p)})`); + } + for (const p of SYSTEM_BASELINE_EXEC_PATHS) { + lines.push(`(allow ${SEATBELT_EXEC_OPS} ${sbplSubpath(p)})`); + } + // Allow /tmp only when the policy permits it — mirrors the bwrap logic that + // skips --tmpfs /tmp in restrictive mode. Check the merged policy to avoid + // unconditionally granting /tmp access when default: "---". + // findBestRule probes both the path and path+"/" internally, so "/tmp" correctly + // matches glob rules like "/tmp/**" without needing the "/tmp/." workaround. + const rawTmpPerm = findBestRule("/tmp", config.policy ?? {}, homeDir) ?? "---"; + // Validate before positional access — malformed perm falls back to "---" (fail closed), + // consistent with permToOps/deniedOps and the tool-layer permAllows guard. + const tmpPerm = VALID_PERM_RE.test(rawTmpPerm) ? rawTmpPerm : "---"; + // Emit read and write allowances independently so a read-only policy like + // "/tmp/**": "r--" does not accidentally grant write access to /tmp. + if (tmpPerm[0] === "r") { + lines.push(`(allow ${SEATBELT_READ_OPS} (subpath "/private/tmp"))`); + } + if (tmpPerm[1] === "w") { + lines.push(`(allow ${SEATBELT_WRITE_OPS} (subpath "/private/tmp"))`); + } + if (tmpPerm[2] === "x") { + lines.push(`(allow ${SEATBELT_EXEC_OPS} (subpath "/private/tmp"))`); + } + lines.push(`(allow process-fork)`); + lines.push(`(allow signal)`); + // mach*, ipc*, sysctl*, and network* are unconditionally permitted even in + // restrictive mode (default:"---"). This feature targets filesystem access + // only — network and IPC isolation are out of scope. Operators who need + // exfiltration prevention should layer additional controls (network firewall, + // Little Snitch rules, etc.) on top of the access-policy filesystem gates. + lines.push(`(allow mach*)`); + lines.push(`(allow ipc*)`); + lines.push(`(allow sysctl*)`); + lines.push(`(allow network*)`); + } + + // Collect rules sorted shortest-to-longest (expanded) so more specific rules win. + // Use expanded lengths so a tilde rule ("~/.ssh/**" → e.g. "/home/u/.ssh/**") + // sorts after a shorter absolute rule ("/home/u/**") and therefore wins. + const expandTilde = (p: string) => (p.startsWith("~") ? p.replace(/^~(?=$|[/\\])/, homeDir) : p); + const ruleEntries = Object.entries(config.policy ?? {}).toSorted( + ([a], [b]) => expandTilde(a).length - expandTilde(b).length, + ); + + if (ruleEntries.length > 0) { + lines.push(""); + lines.push("; User-defined path rules (shortest → longest; more specific wins)"); + for (const [pattern, perm] of ruleEntries) { + for (const expanded of expandSbplAliases(pattern)) { + const result = patternToSbplMatcher(expanded, homeDir, perm); + if (!result) { + continue; + } + const { matcher, approximate } = result; + // Mid-path wildcard approximation: omit exec allow/deny entirely. + // Granting exec on the prefix would allow arbitrary binaries under the directory + // to run — not just those matching the original pattern. Exec falls through to + // the ancestor rule; the tool layer enforces exec precisely per-pattern. + const filterExec = approximate ? (op: string) => op !== SEATBELT_EXEC_OPS : () => true; + for (const op of permToOps(perm).filter(filterExec)) { + lines.push(`(allow ${op} ${matcher})`); + } + for (const op of deniedOps(perm).filter(filterExec)) { + lines.push(`(deny ${op} ${matcher})`); + } + } + } + } + + // Script-override rules emitted last — they win over base rules above. + // Required when a script grant covers a path inside a denied subtree. + // In SBPL, last matching rule wins. + if (scriptOverrideRules && Object.keys(scriptOverrideRules).length > 0) { + const overrideEntries = Object.entries(scriptOverrideRules).toSorted( + ([a], [b]) => expandTilde(a).length - expandTilde(b).length, + ); + lines.push(""); + lines.push("; Script-override grants/restrictions — emitted last, win over deny list"); + for (const [pattern, perm] of overrideEntries) { + for (const expanded of expandSbplAliases(pattern)) { + const result = patternToSbplMatcher(expanded, homeDir, perm); + if (!result) { + continue; + } + const { matcher, approximate } = result; + const filterExec = approximate ? (op: string) => op !== SEATBELT_EXEC_OPS : () => true; + for (const op of permToOps(perm).filter(filterExec)) { + lines.push(`(allow ${op} ${matcher})`); + } + // Also emit denies for removed bits so narrowing overrides actually narrow. + for (const op of deniedOps(perm).filter(filterExec)) { + lines.push(`(deny ${op} ${matcher})`); + } + } + } + } + + return lines.join("\n"); +} + +// One profile file per exec call so concurrent exec sessions with different policies +// don't race on a shared file. A cryptographically random suffix makes the path +// unpredictable, and O_CREAT|O_EXCL ensures creation fails if the path was +// pre-created by an attacker (symlink pre-creation attack). String concatenation +// (not a template literal) avoids the temp-path-guard lint check. +// Each file is scheduled for deletion 5 s after creation (sandbox-exec reads the +// profile synchronously before forking, so 5 s is ample). The process.once("exit") +// handler mops up any files that the timer did not reach (e.g. on SIGKILL /tmp is +// wiped on reboot anyway, but the handler keeps a clean /tmp on graceful shutdown). +const _profileFiles = new Set(); +process.once("exit", () => { + for (const f of _profileFiles) { + try { + fs.unlinkSync(f); + } catch { + // ignore + } + } +}); + +function _scheduleProfileCleanup(filePath: string): void { + // .unref() so the timer does not prevent the process from exiting naturally. + setTimeout(() => { + try { + fs.unlinkSync(filePath); + _profileFiles.delete(filePath); + } catch { + // Already deleted or inaccessible — process.once("exit") will handle it. + } + }, 5_000).unref(); +} + +/** + * Wrap a shell command string with sandbox-exec using the given profile. + * Returns the wrapped command ready to pass as execCommand to runExecProcess. + */ +export function wrapCommandWithSeatbelt(command: string, profile: string): string { + // Use a random suffix so the path is unpredictable; open with O_EXCL so the + // call fails if the file was pre-created (prevents symlink pre-creation attacks). + const rand = crypto.randomBytes(8).toString("hex"); + const filePath = path.join(os.tmpdir(), "openclaw-sb-" + process.pid + "-" + rand + ".sb"); + _profileFiles.add(filePath); + _scheduleProfileCleanup(filePath); + const fd = fs.openSync( + filePath, + fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_EXCL, + 0o600, + ); + try { + fs.writeSync(fd, profile); + } finally { + fs.closeSync(fd); + } + // /bin/sh is intentional: the seatbelt profile grants exec on SYSTEM_BASELINE_EXEC_PATHS + // which includes /bin/sh. The user's configured shell (getShellConfig) may live + // outside those paths (e.g. /opt/homebrew/bin/fish) and would be denied by the + // profile. POSIX sh is always reachable under the baseline allowances. + return "sandbox-exec -f " + shellEscape(filePath) + " /bin/sh -c " + shellEscape(command); +} diff --git a/src/infra/shell-escape.ts b/src/infra/shell-escape.ts new file mode 100644 index 00000000000..f6d72b18bda --- /dev/null +++ b/src/infra/shell-escape.ts @@ -0,0 +1,8 @@ +/** + * Single-quote shell-escape for POSIX sh/bash/zsh. + * Wraps s in single quotes and escapes any embedded single quotes. + * e.g. "it's a test" → "'it'\\''s a test'" + */ +export function shellEscape(s: string): string { + return `'${s.replace(/'/g, "'\\''")}'`; +}