diff --git a/src/infra/access-policy.test.ts b/src/infra/access-policy.test.ts index f9f16559190..0fb47be1b90 100644 --- a/src/infra/access-policy.test.ts +++ b/src/infra/access-policy.test.ts @@ -128,16 +128,18 @@ describe("validateAccessPolicyConfig", () => { expect(config.deny?.[0]).toBe(fileLikePath); // must NOT be expanded to /** }); - it("does NOT auto-expand dotfile-style non-existent deny[] paths (.env, .netrc)", () => { - // Leading-dot names like ".env" or ".netrc" are files, not directories. Expanding - // "~/.env" to "~/.env/**" would protect only non-existent children, leaving the - // file itself unprotected once created. The heuristic treats leading-dot names - // (no further dot/slash) as file-like and skips auto-expansion. + it("auto-expands non-existent bare dotnames (.ssh, .aws, .env) to /** — treats as future directory", () => { + // Bare dotnames without a secondary extension (.ssh, .aws, .env, .gnupg) cannot be + // reliably identified as file vs. directory before they exist. The safe choice is to + // expand to /** so the subtree is protected if a directory is created there. + // When the path later exists as a file, statSync confirms it and the bare pattern is kept. const base = os.tmpdir(); - for (const name of [".env", ".netrc", ".htpasswd", ".npmrc"]) { - const config: AccessPolicyConfig = { deny: [path.join(base, name)] }; + for (const name of [".ssh", ".aws", ".env", ".netrc", ".gnupg", ".config"]) { + _resetAutoExpandedWarnedForTest(); + const p = path.join(base, name); + const config: AccessPolicyConfig = { deny: [p] }; validateAccessPolicyConfig(config); - expect(config.deny?.[0]).toBe(path.join(base, name)); // must NOT be expanded to /** + expect(config.deny?.[0]).toBe(`${p}/**`); // expanded to protect subtree } }); diff --git a/src/infra/access-policy.ts b/src/infra/access-policy.ts index 860d6d0505f..888244bafa1 100644 --- a/src/infra/access-policy.ts +++ b/src/infra/access-policy.ts @@ -167,13 +167,16 @@ export function validateAccessPolicyConfig(config: AccessPolicyConfig): string[] .replace(/[/\\]$/, "") .split(/[/\\]/) .pop() ?? ""; - // Looks like a file when either: - // a) Pure dotfile name: leading dot, no further dot/slash — covers ".env", - // ".netrc", ".htpasswd". These are typically files, not directories. - // b) Non-leading dot with a letter-containing extension — covers "secrets.key", - // "config.json". Digit-only extensions (v1.0, app-2.3) are treated as - // versioned directory names and excluded. - looksLikeFile = /^\.[^./\\]+$/.test(lastName) || /[^.]\.[a-zA-Z][^/\\]*$/.test(lastName); + // Looks like a file when the last segment has a non-leading dot followed by a + // letter-containing extension — covers "secrets.key", "config.json", ".npmrc". + // Digit-only suffixes (v1.0, app-2.3) are treated as versioned directory names. + // Bare dotnames without a secondary extension (.ssh, .aws, .env, .gnupg) are + // NOT treated as file-like: they are expanded to /** so the subtree is protected + // when the path does not yet exist. For .env-style plain files the expansion is + // conservative but safe — once the file exists, statSync confirms it and the bare + // path is kept. The leading-dot heuristic was removed because it also matched + // common directory names (.ssh, .aws, .config) and silently skipped expansion. + looksLikeFile = /[^.]\.[a-zA-Z][^/\\]*$/.test(lastName); } if (!isExistingFile && !looksLikeFile) { const fixed = `${pattern}/**`; diff --git a/src/infra/exec-sandbox-bwrap.test.ts b/src/infra/exec-sandbox-bwrap.test.ts index 0c1e181e012..bbd37064ee3 100644 --- a/src/infra/exec-sandbox-bwrap.test.ts +++ b/src/infra/exec-sandbox-bwrap.test.ts @@ -138,6 +138,21 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => { 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 = { default: "r--", rules: { "/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 (default: ---)", () => { const config: AccessPolicyConfig = { default: "---" }; const args = generateBwrapArgs(config, HOME); diff --git a/src/infra/exec-sandbox-bwrap.ts b/src/infra/exec-sandbox-bwrap.ts index 17f3162f0a6..279193de2aa 100644 --- a/src/infra/exec-sandbox-bwrap.ts +++ b/src/infra/exec-sandbox-bwrap.ts @@ -164,7 +164,12 @@ export function generateBwrapArgs( // control tmpfs access explicitly (e.g. "/tmp/**":"rwx" is handled by the rules loop). if (defaultAllowsRead) { const explicitTmpPerm = findBestRule("/tmp/.", config.rules ?? {}, homeDir); - if (explicitTmpPerm === null || explicitTmpPerm[1] === "w") { + 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"); } }