diff --git a/docs/tools/access-policy.md b/docs/tools/access-policy.md index 121f2fd6372..fb1504f0f54 100644 --- a/docs/tools/access-policy.md +++ b/docs/tools/access-policy.md @@ -130,6 +130,8 @@ For cross-agent MCP tool delegation (an orchestrator invoking a tool on behalf o **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, place it in the deny list (no read access). +**File-specific `deny[]` entries on Linux (bwrap).** On Linux, `deny[]` entries are enforced at the OS layer using `bwrap --tmpfs` overlays, which only work on directories. When a `deny[]` entry resolves to an existing file (e.g. `deny: ["~/.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. `deny: ["~/.aws/"]` rather than `deny: ["~/.aws/credentials"]`). On macOS, seatbelt handles file-level denials correctly with `(deny file-read* (literal ...))`. + **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 diff --git a/src/infra/access-policy.test.ts b/src/infra/access-policy.test.ts index 4950a996afb..b4b6b8ffdbf 100644 --- a/src/infra/access-policy.test.ts +++ b/src/infra/access-policy.test.ts @@ -865,6 +865,19 @@ describe("applyScriptPolicyOverride", () => { } }); + 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 = { + default: "rwx", + scripts: { "~/bin/deploy.sh": { rules: { "/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"); diff --git a/src/infra/access-policy.ts b/src/infra/access-policy.ts index 821a8b2249b..c86aa2c2cd1 100644 --- a/src/infra/access-policy.ts +++ b/src/infra/access-policy.ts @@ -456,7 +456,19 @@ export function applyScriptPolicyOverride( policy: AccessPolicyConfig, resolvedArgv0: string, ): { policy: AccessPolicyConfig; overrideRules?: Record; hashMismatch?: true } { - const override = policy.scripts?.[resolvedArgv0]; + // Normalise ~ in scripts keys so "~/bin/deploy.sh" matches the resolved absolute + // path "/home/user/bin/deploy.sh" that resolveArgv0 returns. A direct lookup + // would always miss tilde-keyed entries, silently skipping sha256 verification. + const scripts = policy.scripts; + const override = scripts + ? (scripts[resolvedArgv0] ?? + Object.entries(scripts).find(([k]) => { + if (!k.startsWith("~")) { + return false; + } + return k.replace(/^~(?=$|[/\\])/, os.homedir()) === resolvedArgv0; + })?.[1]) + : undefined; if (!override) { return { policy }; } diff --git a/src/infra/exec-sandbox-bwrap.test.ts b/src/infra/exec-sandbox-bwrap.test.ts index 7d1eb0d858c..df771b76f10 100644 --- a/src/infra/exec-sandbox-bwrap.test.ts +++ b/src/infra/exec-sandbox-bwrap.test.ts @@ -1,7 +1,11 @@ import os from "node:os"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import type { AccessPolicyConfig } from "../config/types.tools.js"; -import { generateBwrapArgs, wrapCommandWithBwrap } from "./exec-sandbox-bwrap.js"; +import { + _warnBwrapFileDenyOnce, + generateBwrapArgs, + wrapCommandWithBwrap, +} from "./exec-sandbox-bwrap.js"; const HOME = os.homedir(); @@ -302,6 +306,19 @@ describe("generateBwrapArgs", () => { expect(tmpfsMounts).not.toContain("/etc/hosts"); }); + it("emits a console.error warning when a file-specific deny[] entry is skipped", () => { + // Use /etc/passwd (always a file) rather than /etc/hosts which is already in + // _bwrapFileDenyWarnedPaths from the generateBwrapArgs test above. + 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 deny[] entry that resolves to a directory", () => { // Non-existent paths are treated as directories (forward-protection). const config: AccessPolicyConfig = { default: "r--", deny: [`${HOME}/.nonexistent-dir/**`] }; diff --git a/src/infra/exec-sandbox-bwrap.ts b/src/infra/exec-sandbox-bwrap.ts index 7790ae90a15..0c570255597 100644 --- a/src/infra/exec-sandbox-bwrap.ts +++ b/src/infra/exec-sandbox-bwrap.ts @@ -26,6 +26,25 @@ const SYSTEM_RO_BIND_PATHS = ["/usr", "/bin", "/lib", "/lib64", "/sbin", "/etc", let bwrapAvailableCache: boolean | undefined; +// Warn once per process when a file-specific deny[] entry 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. +// See docs/tools/access-policy.md — "File-specific deny[] entries on Linux". +const _bwrapFileDenyWarnedPaths = new Set(); +export function _warnBwrapFileDenyOnce(filePath: string): void { + if (_bwrapFileDenyWarnedPaths.has(filePath)) { + return; + } + _bwrapFileDenyWarnedPaths.add(filePath); + console.error( + `[access-policy] bwrap: deny[] entry "${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, deny its parent directory instead.`, + ); +} + /** * Returns true if bwrap is installed and executable on this system. * Result is cached after the first call. @@ -195,9 +214,12 @@ export function generateBwrapArgs( } if (isDir) { args.push("--tmpfs", p); + } else { + // File-specific entry: tool-layer checkAccessPolicy still denies read/write/edit + // tool calls, but exec commands inside the sandbox can still access the file + // directly. Warn operators so they know to deny the parent directory instead. + _warnBwrapFileDenyOnce(p); } - // File-specific entry: tool-layer checkAccessPolicy already denies reads/writes; - // bwrap cannot mount tmpfs over a file so skip the OS-layer mount silently. } // Script-specific override mounts — emitted after deny[] so they can reopen