fix(access-policy): remove dotdir looksLikeFile gap, drop dead bwrap tmpfs branch

This commit is contained in:
subrih 2026-03-13 22:27:30 -07:00
parent 8e0e02d7ac
commit 6b9828182c
4 changed files with 41 additions and 16 deletions

View File

@ -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
}
});

View File

@ -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}/**`;

View File

@ -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);

View File

@ -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");
}
}