nyrn
/
openclaw
mirror of https://github.com/openclaw/openclaw.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

refactor(access-policy): drop deny[] and default — rules only, --- implies deny

This commit is contained in:
subrih 2026-03-14 05:44:16 -07:00
parent 4f4d1af887
commit cd2d9c3b8d
11 changed files with 246 additions and 684 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/agents/bash-tools.exec-runtime.ts
View File

@ -384,30 +384,15 @@ export async function runExecProcess(opts: {
// enforcement (seatbelt/bwrap) is unavailable (Linux without bwrap, Windows).
// Mirrors the checkAccessPolicy calls in read/write tools for consistency.
//
// deny[] always wins — checked unconditionally even for scripts{} entries.
// The hash check authorises the policy overlay (effectivePermissions), not exec
// itself. A script in both deny[] and scripts{} is a config error; the deny wins.
// For scripts{} entries not in deny[], we skip the broader rules/default check
// so a sha256-matched script doesn't also need an explicit exec rule in the base policy.
// 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. Without symlink
// resolution, a symlink-keyed script would silently bypass the override gate,
// running under the base policy with no integrity or deny-narrowing checks.
// is always the realpathSync result from resolveArgv0.
const _scripts = opts.permissions.scripts ?? {};
const hasScriptOverride = Object.keys(_scripts).some(
(k) => path.normalize(resolveScriptKey(k)) === path.normalize(argv0),
);
// Use default:"rwx" so only actual deny-pattern hits produce "deny".
// Without a default, permAllows(undefined, "exec") → false → every path
// not matched by a deny pattern would be incorrectly blocked.
const denyVerdict = checkAccessPolicy(argv0, "exec", {
deny: opts.permissions.deny,
default: "rwx",
});
if (denyVerdict === "deny") {
throw new Error(`exec denied by access policy: ${argv0}`);
}
if (!hasScriptOverride && checkAccessPolicy(argv0, "exec", effectivePermissions) === "deny") {
throw new Error(`exec denied by access policy: ${argv0}`);
}

3
src/agents/pi-tools.read.edit-permission.test.ts
View File

@ -55,7 +55,6 @@ describe("createHostWorkspaceEditTool edit read-permission check", () => {
// "-w-" policy: write allowed, read denied.
// Edit must NOT be allowed to read the file even if write is permitted.
const permissions: AccessPolicyConfig = {
default: "---",
rules: { [`${tmpDir}/**`]: "-w-" },
};
createHostWorkspaceEditTool(tmpDir, { workspaceOnly: false, permissions });
@ -73,7 +72,6 @@ describe("createHostWorkspaceEditTool edit read-permission check", () => {
await fs.writeFile(filePath, "content", "utf8");
const permissions: AccessPolicyConfig = {
default: "---",
rules: { [`${tmpDir}/**`]: "rw-" },
};
createHostWorkspaceEditTool(tmpDir, { workspaceOnly: false, permissions });
@ -92,7 +90,6 @@ describe("createHostWorkspaceEditTool edit read-permission check", () => {
// "r--" policy: read allowed, write denied.
const permissions: AccessPolicyConfig = {
default: "---",
rules: { [`${tmpDir}/**`]: "r--" },
};
createHostWorkspaceEditTool(tmpDir, { workspaceOnly: false, permissions });

9
src/config/types.tools.ts
View File

@ -10,8 +10,6 @@ export type PermStr = string;
export type ScriptPolicyEntry = {
/** Restrict/expand rules for this script. Merged over the base policy rules. */
rules?: Record<string, PermStr>;
/** Additional deny patterns added when this script runs (additive). */
deny?: string[];
/** SHA-256 hex of the script file for integrity checking (best-effort, not atomic). */
sha256?: string;
};
@ -19,14 +17,13 @@ export type ScriptPolicyEntry = {
/**
* 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 = {
/** Fallback permission when no rule matches. Defaults to `"---"` (deny-all) when absent. */
default?: PermStr;
/** Glob-pattern rules: path → permission string. Longest prefix wins. */
rules?: Record<string, PermStr>;
/** Patterns that are always denied regardless of rules (additive across merges). */
deny?: string[];
/** Per-script argv0 policy overrides keyed by resolved binary path. */
scripts?: Record<string, ScriptPolicyEntry>;
};

122
src/infra/access-policy-file.test.ts
View File

@ -56,39 +56,15 @@ describe("mergeAccessPolicy", () => {
});
it("returns base when override is undefined", () => {
const base = { default: "r--" };
const base = { rules: { "/**": "r--" as const } };
expect(mergeAccessPolicy(base, undefined)).toEqual(base);
});
it("returns override when base is undefined", () => {
const override = { default: "rwx" };
const override = { rules: { "/**": "rwx" as const } };
expect(mergeAccessPolicy(undefined, override)).toEqual(override);
});
it("override default wins", () => {
const result = mergeAccessPolicy({ default: "r--" }, { default: "rwx" });
expect(result?.default).toBe("rwx");
});
it("base default survives when override has no default", () => {
const result = mergeAccessPolicy({ default: "r--" }, { rules: { "/**": "r-x" } });
expect(result?.default).toBe("r--");
});
it("deny arrays are concatenated — base denies cannot be removed", () => {
const result = mergeAccessPolicy(
{ deny: ["~/.ssh/**", "~/.aws/**"] },
{ deny: ["~/.gnupg/**"] },
);
expect(result?.deny).toEqual(["~/.ssh/**", "~/.aws/**", "~/.gnupg/**"]);
});
it("override deny extends base deny", () => {
const result = mergeAccessPolicy({ deny: ["~/.ssh/**"] }, { deny: ["~/.env"] });
expect(result?.deny).toContain("~/.ssh/**");
expect(result?.deny).toContain("~/.env");
});
it("rules are shallow-merged, override key wins on collision", () => {
const result = mergeAccessPolicy(
{ rules: { "/**": "r--", "~/**": "rw-" } },
@ -99,9 +75,8 @@ describe("mergeAccessPolicy", () => {
expect(result?.rules?.["~/dev/**"]).toBe("rwx"); // override adds
});
it("omits empty deny/rules from result", () => {
const result = mergeAccessPolicy({ default: "r--" }, { default: "rwx" });
expect(result?.deny).toBeUndefined();
it("omits empty rules from result", () => {
const result = mergeAccessPolicy({ scripts: { "/s.sh": { sha256: "abc" } } }, {});
expect(result?.rules).toBeUndefined();
});
@ -113,16 +88,14 @@ describe("mergeAccessPolicy", () => {
"/usr/local/bin/deploy.sh": {
sha256: "abc123",
rules: { "~/deploy/**": "rwx" as const },
deny: ["~/.ssh/**"],
},
},
};
const override = {
scripts: {
"/usr/local/bin/deploy.sh": {
// Agent block supplies same key — must NOT be able to drop sha256 or deny[].
// Agent block supplies same key — must NOT be able to drop sha256.
rules: { "~/deploy/**": "r--" as const }, // narrower override — fine
deny: ["~/extra-deny/**"],
},
},
};
@ -130,9 +103,6 @@ describe("mergeAccessPolicy", () => {
const merged = result?.scripts?.["/usr/local/bin/deploy.sh"];
// sha256 from base must survive.
expect(merged?.sha256).toBe("abc123");
// deny[] must be additive — base deny cannot be removed.
expect(merged?.deny).toContain("~/.ssh/**");
expect(merged?.deny).toContain("~/extra-deny/**");
// rules: override key wins on collision.
expect(merged?.rules?.["~/deploy/**"]).toBe("r--");
});
@ -148,17 +118,6 @@ describe("mergeAccessPolicy", () => {
// New script from override is added.
expect(result?.scripts?.["/bin/new.sh"]?.rules?.["/tmp/**"]).toBe("rwx");
});
it("scripts deep-merge: base deny[] cannot be removed by override supplying empty deny[]", () => {
const base = {
scripts: { "/bin/s.sh": { deny: ["~/.secrets/**"] } },
};
const override = {
scripts: { "/bin/s.sh": { deny: [] } }, // empty override deny — base must survive
};
const result = mergeAccessPolicy(base, override);
expect(result?.scripts?.["/bin/s.sh"]?.deny).toContain("~/.secrets/**");
});
});
// ---------------------------------------------------------------------------
@ -219,12 +178,12 @@ describe("loadAccessPolicyFile", () => {
spy.mockRestore();
});
it("returns BROKEN_POLICY_FILE and logs error when 'deny' is misplaced at top level", () => {
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, deny: ["~/.ssh/**"] });
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 "deny"'));
expect(spy).toHaveBeenCalledWith(expect.stringContaining('unexpected top-level key "scripts"'));
spy.mockRestore();
});
@ -240,7 +199,7 @@ describe("loadAccessPolicyFile", () => {
it("returns parsed file when valid", () => {
const content: AccessPolicyFile = {
version: 1,
base: { default: "r--", deny: ["~/.ssh/**"] },
base: { rules: { "/**": "r--", "~/.ssh/**": "---" } },
agents: { subri: { rules: { "~/dev/**": "rwx" } } },
};
writeFile(content);
@ -251,7 +210,7 @@ describe("loadAccessPolicyFile", () => {
throw new Error("unexpected");
}
expect(result.version).toBe(1);
expect(result.base?.default).toBe("r--");
expect(result.base?.rules?.["/**"]).toBe("r--");
expect(result.agents?.subri?.rules?.["~/dev/**"]).toBe("rwx");
});
});
@ -262,7 +221,7 @@ describe("loadAccessPolicyFile", () => {
describe("loadAccessPolicyFile — mtime cache", () => {
it("returns cached result on second call without re-reading the file", () => {
writeFile({ version: 1, base: { default: "r--" } });
writeFile({ version: 1, base: { rules: { "/**": "r--" } } });
const spy = vi.spyOn(fs, "readFileSync");
loadAccessPolicyFile(); // populate cache
loadAccessPolicyFile(); // should hit cache
@ -272,11 +231,11 @@ describe("loadAccessPolicyFile — mtime cache", () => {
});
it("re-reads when mtime changes (file updated)", () => {
writeFile({ version: 1, base: { default: "r--" } });
writeFile({ version: 1, base: { rules: { "/**": "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, base: { default: "rwx" } });
writeFile({ version: 1, base: { rules: { "/**": "rwx" } } });
const future = Date.now() / 1000 + 1;
fs.utimesSync(FP_FILE, future, future);
const result = loadAccessPolicyFile();
@ -284,7 +243,7 @@ describe("loadAccessPolicyFile — mtime cache", () => {
if (result === null || result === BROKEN_POLICY_FILE) {
throw new Error("unexpected");
}
expect(result.base?.default).toBe("rwx");
expect(result.base?.rules?.["/**"]).toBe("rwx");
});
it("clears cache when file is deleted", () => {
@ -328,7 +287,7 @@ describe("resolveAccessPolicyForAgent", () => {
it("does not warn when config file exists and is valid", () => {
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
writeFile({ version: 1, base: { default: "r--" } });
writeFile({ version: 1, base: { rules: { "/**": "r--" } } });
resolveAccessPolicyForAgent("subri");
expect(warnSpy).not.toHaveBeenCalled();
warnSpy.mockRestore();
@ -341,8 +300,8 @@ describe("resolveAccessPolicyForAgent", () => {
const result = resolveAccessPolicyForAgent("subri");
expect(warnSpy).not.toHaveBeenCalled();
expect(errSpy).toHaveBeenCalledWith(expect.stringContaining("Failing closed"));
// Broken file must fail-closed: deny-all policy, not undefined
expect(result).toEqual({ default: "---" });
// Broken file must fail-closed: deny-all policy (empty rules = implicit "---"), not undefined
expect(result).toEqual({});
warnSpy.mockRestore();
errSpy.mockRestore();
});
@ -351,50 +310,48 @@ describe("resolveAccessPolicyForAgent", () => {
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
writeFile({ version: 1, rules: { "/**": "r--" } }); // misplaced key — broken
const result = resolveAccessPolicyForAgent("subri");
expect(result).toEqual({ default: "---" });
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<string, unknown>)["default"] = "rwx";
(result as Record<string, unknown>)["rules"] = { "/**": "rwx" };
} catch {
// Object.freeze throws in strict mode — that's fine too.
}
_resetNotFoundWarnedForTest();
const result2 = resolveAccessPolicyForAgent("subri");
expect(result2).toEqual({ default: "---" });
expect(result2).toEqual({});
errSpy.mockRestore();
});
it("returns base when no agent block exists", () => {
writeFile({
version: 1,
base: { default: "r--", deny: ["~/.ssh/**"] },
base: { rules: { "/**": "r--", [`~/.ssh/**`]: "---" } },
});
const result = resolveAccessPolicyForAgent("subri");
expect(result?.default).toBe("r--");
expect(result?.deny).toContain("~/.ssh/**");
expect(result?.rules?.["/**"]).toBe("r--");
expect(result?.rules?.["~/.ssh/**"]).toBe("---");
});
it("merges base + named agent", () => {
writeFile({
version: 1,
base: { default: "---", deny: ["~/.ssh/**"], rules: { "/**": "r--" } },
agents: { subri: { rules: { "~/dev/**": "rwx" }, default: "r--" } },
base: { rules: { "/**": "r--", [`~/.ssh/**`]: "---" } },
agents: { subri: { rules: { "~/dev/**": "rwx" } } },
});
const result = resolveAccessPolicyForAgent("subri");
// default: agent wins
expect(result?.default).toBe("r--");
// deny: additive
expect(result?.deny).toContain("~/.ssh/**");
// rules: merged
// rules: merged, agent rule wins on collision
expect(result?.rules?.["/**"]).toBe("r--");
expect(result?.rules?.["~/dev/**"]).toBe("rwx");
// base "---" rule preserved
expect(result?.rules?.["~/.ssh/**"]).toBe("---");
});
it("wildcard agent applies before named agent", () => {
writeFile({
version: 1,
base: { default: "---" },
base: {},
agents: {
"*": { rules: { "/usr/bin/**": "r-x" } },
subri: { rules: { "~/dev/**": "rwx" } },
@ -403,27 +360,26 @@ describe("resolveAccessPolicyForAgent", () => {
const result = resolveAccessPolicyForAgent("subri");
expect(result?.rules?.["/usr/bin/**"]).toBe("r-x"); // from wildcard
expect(result?.rules?.["~/dev/**"]).toBe("rwx"); // from named agent
expect(result?.default).toBe("---"); // from base
});
it("wildcard applies even when no named agent block", () => {
writeFile({
version: 1,
base: { default: "---" },
agents: { "*": { deny: ["~/.ssh/**"] } },
base: {},
agents: { "*": { rules: { [`~/.ssh/**`]: "---" } } },
});
const result = resolveAccessPolicyForAgent("other-agent");
expect(result?.deny).toContain("~/.ssh/**");
expect(result?.rules?.["~/.ssh/**"]).toBe("---");
});
it("wildcard key itself is not treated as a named agent", () => {
writeFile({
version: 1,
agents: { "*": { deny: ["~/.ssh/**"] } },
agents: { "*": { rules: { [`~/.ssh/**`]: "---" } } },
});
// Requesting agentId "*" should not double-apply wildcard as named
const result = resolveAccessPolicyForAgent("*");
expect(result?.deny).toEqual(["~/.ssh/**"]);
expect(result?.rules?.["~/.ssh/**"]).toBe("---");
});
it("returns undefined when file is empty (no base, no agents)", () => {
@ -470,14 +426,14 @@ describe("resolveAccessPolicyForAgent", () => {
errSpy.mockRestore();
});
it("named agent deny extends global deny — global deny cannot be removed", () => {
it("narrowing rules from base and agent are all preserved in merged result", () => {
writeFile({
version: 1,
base: { deny: ["~/.ssh/**"] },
agents: { paranoid: { deny: ["~/.aws/**"] } },
base: { rules: { [`~/.ssh/**`]: "---" } },
agents: { paranoid: { rules: { [`~/.aws/**`]: "---" } } },
});
const result = resolveAccessPolicyForAgent("paranoid");
expect(result?.deny).toContain("~/.ssh/**");
expect(result?.deny).toContain("~/.aws/**");
expect(result?.rules?.["~/.ssh/**"]).toBe("---");
expect(result?.rules?.["~/.aws/**"]).toBe("---");
});
});

33
src/infra/access-policy-file.ts
View File

@ -15,9 +15,8 @@ export type AccessPolicyFile = {
* base → agents["*"] → agents[agentId]
*
* Within each layer:
* - deny: additive (concat) — a base deny entry can never be removed by an override
* - rules: shallow-merge, override key wins on collision
* - default: override wins if set
* - scripts: deep-merge per key; base sha256 is preserved
*/
agents?: Record<string, AccessPolicyConfig>;
};
@ -31,9 +30,8 @@ export function resolveAccessPolicyPath(): string {
/**
* Merge two AccessPolicyConfig layers.
* - deny: additive (cannot remove a base deny)
* - rules: shallow merge, override key wins
* - default: override wins if set
* - scripts: deep-merge per key; base sha256 is preserved (cannot be removed by override)
*/
export function mergeAccessPolicy(
base: AccessPolicyConfig | undefined,
@ -48,12 +46,11 @@ export function mergeAccessPolicy(
if (!override) {
return base;
}
const deny = [...(base.deny ?? []), ...(override.deny ?? [])];
const rules = { ...base.rules, ...override.rules };
// scripts: deep-merge per key — base sha256 and deny[] are preserved regardless of
// 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 and per-script deny list
// when an agent block supplies the same script key, defeating the security intent.
// 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<AccessPolicyConfig["scripts"]> = { ...base.scripts };
for (const [key, overrideEntry] of Object.entries(override.scripts ?? {})) {
const baseEntry = base.scripts?.[key];
@ -61,7 +58,6 @@ export function mergeAccessPolicy(
mergedScripts[key] = overrideEntry;
continue;
}
const entryDeny = [...(baseEntry.deny ?? []), ...(overrideEntry.deny ?? [])];
mergedScripts[key] = {
// sha256: base always wins — cannot be removed or replaced by an agent override.
...(baseEntry.sha256 !== undefined ? { sha256: baseEntry.sha256 } : {}),
@ -69,26 +65,16 @@ export function mergeAccessPolicy(
...(Object.keys({ ...baseEntry.rules, ...overrideEntry.rules }).length > 0
? { rules: { ...baseEntry.rules, ...overrideEntry.rules } }
: {}),
// deny: additive — base per-script deny cannot be removed.
...(entryDeny.length > 0 ? { deny: entryDeny } : {}),
};
}
const scripts = Object.keys(mergedScripts).length > 0 ? mergedScripts : undefined;
const result: AccessPolicyConfig = {};
if (deny.length > 0) {
result.deny = deny;
}
if (Object.keys(rules).length > 0) {
result.rules = rules;
}
if (scripts) {
result.scripts = scripts;
}
if (override.default !== undefined) {
result.default = override.default;
} else if (base.default !== undefined) {
result.default = base.default;
}
return result;
}
@ -119,8 +105,8 @@ function validateAccessPolicyFileStructure(filePath: string, parsed: unknown): s
}
// Catch common mistake: AccessPolicyConfig fields accidentally at top level
// (e.g. user puts "rules" or "deny" directly instead of under "base").
for (const key of ["rules", "deny", "default", "scripts"] as const) {
// (e.g. user puts "rules" or "scripts" directly instead of under "base").
for (const key of ["rules", "scripts"] as const) {
if (p[key] !== undefined) {
errors.push(
`${filePath}: unexpected top-level key "${key}" — did you mean to put it under "base"?`,
@ -249,8 +235,9 @@ export function _resetNotFoundWarnedForTest(): void {
* 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). */
const DENY_ALL_POLICY: AccessPolicyConfig = Object.freeze({ default: "---" });
/** 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();

285
src/infra/access-policy.test.ts
View File

@ -34,9 +34,7 @@ describe("validateAccessPolicyConfig", () => {
it("returns no errors for a valid config", () => {
expect(
validateAccessPolicyConfig({
rules: { "/**": "r--", [`${HOME}/**`]: "rwx" },
deny: [`${HOME}/.ssh/**`],
default: "---",
rules: { "/**": "r--", [`${HOME}/**`]: "rwx", [`${HOME}/.ssh/**`]: "---" },
}),
).toEqual([]);
});
@ -45,24 +43,6 @@ describe("validateAccessPolicyConfig", () => {
expect(validateAccessPolicyConfig({})).toEqual([]);
});
it("rejects invalid default perm string — too short", () => {
const errs = validateAccessPolicyConfig({ default: "rw" });
expect(errs).toHaveLength(1);
expect(errs[0]).toMatch(/default/);
});
it("rejects invalid default perm string — too long", () => {
const errs = validateAccessPolicyConfig({ default: "rwxr" });
expect(errs).toHaveLength(1);
expect(errs[0]).toMatch(/default/);
});
it("rejects invalid default perm string — wrong chars", () => {
const errs = validateAccessPolicyConfig({ default: "rq-" });
expect(errs).toHaveLength(1);
expect(errs[0]).toMatch(/default/);
});
it("rejects invalid rule perm value", () => {
const errs = validateAccessPolicyConfig({ rules: { "/**": "rx" } });
expect(errs).toHaveLength(1);
@ -75,90 +55,17 @@ describe("validateAccessPolicyConfig", () => {
expect(errs[0]).toMatch(/rules/);
});
it("reports multiple errors when both default and a rule are invalid", () => {
const errs = validateAccessPolicyConfig({
default: "bad",
rules: { "/**": "xyz" },
});
expect(errs.length).toBeGreaterThanOrEqual(2);
it("reports an error when a rule perm value is invalid", () => {
const errs = validateAccessPolicyConfig({ rules: { "/**": "xyz" } });
expect(errs.length).toBeGreaterThanOrEqual(1);
});
it("rejects empty deny entry", () => {
const errs = validateAccessPolicyConfig({ deny: [""] });
expect(errs).toHaveLength(1);
expect(errs[0]).toMatch(/deny/);
});
it("auto-expands a bare directory path in deny[] to /**", () => {
const dir = os.tmpdir();
const config: AccessPolicyConfig = { deny: [dir] };
const errs = validateAccessPolicyConfig(config);
expect(errs).toHaveLength(1);
expect(errs[0]).toMatch(/auto-expanded/);
expect(config.deny?.[0]).toBe(`${dir}/**`);
});
it("auto-expands bare deny[] entry even when the directory does not yet exist", () => {
// Non-existent paths are treated as future directories and expanded to /**.
const nonExistent = path.join(os.tmpdir(), "openclaw-test-nonexistent-" + Date.now());
const config: AccessPolicyConfig = { deny: [nonExistent] };
const errs = validateAccessPolicyConfig(config);
expect(config.deny?.[0]).toBe(`${nonExistent}/**`);
expect(errs[0]).toMatch(/auto-expanded/);
});
it("auto-expands non-existent versioned directory names (v1.0, app-2.3) in deny[]", () => {
// Versioned names like "v1.0" or "pkg-1.0.0" look like files via naive dot-detection
// but are almost always directories. The tightened heuristic requires the extension
// to contain at least one letter — digits-only extensions (like ".0") are treated as
// directory-like and expanded to /**.
const base = os.tmpdir();
const versionedDir = path.join(base, `openclaw-test-v1.0-${Date.now()}`);
const config: AccessPolicyConfig = { deny: [versionedDir] };
validateAccessPolicyConfig(config);
expect(config.deny?.[0]).toBe(`${versionedDir}/**`); // must be expanded
});
it("does NOT auto-expand a non-existent deny[] path that looks like a file (has extension)", () => {
// "~/future-secrets.key" doesn't exist yet but the extension heuristic should
// prevent expansion to "~/future-secrets.key/**" — the user intends to protect
// the file itself, not a subtree of non-existent children.
const fileLikePath = path.join(os.tmpdir(), `openclaw-test-${Date.now()}.key`);
const config: AccessPolicyConfig = { deny: [fileLikePath] };
validateAccessPolicyConfig(config);
expect(config.deny?.[0]).toBe(fileLikePath); // must NOT be expanded to /**
});
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 [".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(`${p}/**`); // expanded to protect subtree
}
});
it("does NOT auto-expand a bare deny[] entry that is an existing file", () => {
// A specific file like "~/.ssh/id_rsa" must stay as an exact-match pattern.
// Expanding it to "~/.ssh/id_rsa/**" would only match non-existent children,
// leaving the file itself unprotected at the tool layer and in bwrap.
// process.execPath is the running node/bun binary — always a real file.
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 = { deny: [file] };
validateAccessPolicyConfig(config);
expect(config.deny?.[0]).toBe(file); // kept as bare path, not expanded
});
it("file-specific deny[] entry blocks access to the file via checkAccessPolicy", () => {
// Regression: bare file path in deny[] must block reads at the tool layer.
const file = process.execPath;
const config: AccessPolicyConfig = { default: "rwx", deny: [file] };
const config: AccessPolicyConfig = {
rules: { "/**": "rwx", [file]: "---" },
};
validateAccessPolicyConfig(config); // applies normalization in-place
expect(checkAccessPolicy(file, "read", config)).toBe("deny");
});
@ -177,22 +84,10 @@ describe("validateAccessPolicyConfig", () => {
expect(errs.some((e) => e.includes("rwX") && e.includes("scripts"))).toBe(true);
});
it("validates scripts[].deny entries and emits diagnostics for empty patterns", () => {
const config: AccessPolicyConfig = {
scripts: {
"/usr/local/bin/deploy.sh": {
deny: ["", "~/.secrets/**"], // first entry is invalid empty string
},
},
};
const errs = validateAccessPolicyConfig(config);
expect(errs.some((e) => e.includes("scripts") && e.includes("deny"))).toBe(true);
});
it("accepts valid 'rwx' and '---' perm strings", () => {
expect(validateAccessPolicyConfig({ default: "rwx" })).toEqual([]);
expect(validateAccessPolicyConfig({ default: "---" })).toEqual([]);
expect(validateAccessPolicyConfig({ default: "r-x" })).toEqual([]);
it("accepts valid rule perm strings", () => {
expect(validateAccessPolicyConfig({ rules: { "/**": "rwx" } })).toEqual([]);
expect(validateAccessPolicyConfig({ rules: { "/**": "---" } })).toEqual([]);
expect(validateAccessPolicyConfig({ rules: { "/**": "r-x" } })).toEqual([]);
});
it("auto-expands a bare path that points to a real directory", () => {
@ -283,22 +178,11 @@ describe("validateAccessPolicyConfig", () => {
expect(second.filter((e) => e.includes("mid-path wildcard"))).toHaveLength(0);
});
it("emits a one-time diagnostic for mid-path wildcard deny[] entries", () => {
_resetMidPathWildcardWarnedForTest();
const errs = validateAccessPolicyConfig({
deny: ["/tmp/*/sensitive/**"],
});
expect(errs).toHaveLength(1);
expect(errs[0]).toMatch(/mid-path wildcard/);
expect(errs[0]).toMatch(/OS-level.*enforcement/);
});
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({
rules: { "/home/user/**": "r--", "~/**": "rwx" },
deny: ["/tmp/**"],
rules: { "/home/user/**": "r--", "~/**": "rwx", "/tmp/**": "---" },
});
expect(errs.filter((e) => e.includes("mid-path wildcard"))).toHaveLength(0);
});
@ -311,17 +195,17 @@ describe("validateAccessPolicyConfig", () => {
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 = { rules: { "/tmp/**": "r1-" as unknown as "r--" }, default: "---" };
const config = { rules: { "/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 = { rules: { "/tmp/**": "R--" as unknown as "r--" }, default: "---" };
const config = { rules: { "/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 = { rules: { "/tmp/**": "rWx" as unknown as "rwx" }, default: "---" };
const config = { rules: { "/tmp/**": "rWx" as unknown as "rwx" } };
expect(checkAccessPolicy("/tmp/foo.txt", "write", config)).toBe("deny");
});
});
@ -332,29 +216,28 @@ describe("checkAccessPolicy — malformed permission characters fail closed", ()
describe("checkAccessPolicy — trailing slash shorthand", () => {
it('"/tmp/" is equivalent to "/tmp/**"', () => {
const config: AccessPolicyConfig = { rules: { "/tmp/": "rwx" }, default: "---" };
const config: AccessPolicyConfig = { rules: { "/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 = { rules: { "~/": "rw-" }, default: "---" };
const config: AccessPolicyConfig = { rules: { "~/": "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("trailing slash in deny list blocks subtree", () => {
it('"---" rule with trailing slash blocks subtree', () => {
const config: AccessPolicyConfig = {
rules: { "/**": "rwx" },
deny: [`${HOME}/.ssh/`],
rules: { "/**": "rwx", [`${HOME}/.ssh/`]: "---" },
};
expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "read", config)).toBe("deny");
});
it("trailing slash and /** produce identical results", () => {
const withSlash: AccessPolicyConfig = { rules: { "/tmp/": "rwx" }, default: "---" };
const withGlob: AccessPolicyConfig = { rules: { "/tmp/**": "rwx" }, default: "---" };
const withSlash: AccessPolicyConfig = { rules: { "/tmp/": "rwx" } };
const withGlob: AccessPolicyConfig = { rules: { "/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(
@ -368,7 +251,6 @@ describe("checkAccessPolicy — trailing slash shorthand", () => {
// path ~/.openclaw/heartbeat (no trailing component), not just its contents.
const config: AccessPolicyConfig = {
rules: { "/**": "r--", [`${HOME}/.openclaw/heartbeat/`]: "rw-" },
default: "---",
};
expect(checkAccessPolicy(`${HOME}/.openclaw/heartbeat`, "write", config)).toBe("allow");
expect(checkAccessPolicy(`${HOME}/.openclaw/heartbeat/test.txt`, "write", config)).toBe(
@ -376,10 +258,9 @@ describe("checkAccessPolicy — trailing slash shorthand", () => {
);
});
it("trailing slash in deny list blocks the directory itself", () => {
it('"---" trailing-slash rule blocks the directory itself and its contents', () => {
const config: AccessPolicyConfig = {
rules: { "/**": "rwx" },
deny: [`${HOME}/.ssh/`],
rules: { "/**": "rwx", [`${HOME}/.ssh/`]: "---" },
};
// Both the directory and its contents should be denied.
expect(checkAccessPolicy(`${HOME}/.ssh`, "read", config)).toBe("deny");
@ -400,7 +281,6 @@ describe.skipIf(process.platform !== "darwin")(
"/var/**": "r--",
"/etc/**": "r--",
},
default: "---",
};
it("/private/tmp path is treated as /tmp — write allowed", () => {
@ -419,19 +299,17 @@ describe.skipIf(process.platform !== "darwin")(
expect(checkAccessPolicy("/tmp/foo.txt", "write", config)).toBe("allow");
});
it("deny list entry /tmp/** also blocks /private/tmp/**", () => {
it('"---" rule for /tmp/** also blocks /private/tmp/**', () => {
const denyConfig: AccessPolicyConfig = {
deny: ["/tmp/**"],
rules: { "/**": "rwx" },
rules: { "/**": "rwx", "/tmp/**": "---" },
};
expect(checkAccessPolicy("/private/tmp/evil.sh", "exec", denyConfig)).toBe("deny");
});
it("/private/tmp/** pattern in deny list blocks /tmp/** target", () => {
// Pattern written with /private/tmp must still match the normalized /tmp target.
it("/private/tmp/** deny rule blocks /tmp/** target", () => {
// Rule written with /private/tmp must still match the normalized /tmp target.
const denyConfig: AccessPolicyConfig = {
deny: ["/private/tmp/**"],
rules: { "/**": "rwx" },
rules: { "/**": "rwx", "/private/tmp/**": "---" },
};
expect(checkAccessPolicy("/tmp/evil.sh", "read", denyConfig)).toBe("deny");
});
@ -439,7 +317,6 @@ describe.skipIf(process.platform !== "darwin")(
it("/private/tmp/** rule matches /tmp/** target", () => {
// Rule written with /private/* prefix must match a /tmp/* target path.
const cfg: AccessPolicyConfig = {
default: "---",
rules: { "/private/tmp/**": "rwx" },
};
expect(checkAccessPolicy("/tmp/foo.txt", "write", cfg)).toBe("allow");
@ -500,15 +377,13 @@ describe("findBestRule", () => {
});
// ---------------------------------------------------------------------------
// checkAccessPolicy — deny list
// checkAccessPolicy — "---" rules act as deny
// ---------------------------------------------------------------------------
describe("checkAccessPolicy — deny list", () => {
it("deny always blocks, even when a rule would allow", () => {
describe('checkAccessPolicy — "---" rules act as deny', () => {
it('"---" rule blocks all ops, even when a broader rule would allow', () => {
const config: AccessPolicyConfig = {
rules: { "/**": "rwx" },
deny: [`${HOME}/.ssh/**`],
default: "rwx",
rules: { "/**": "rwx", [`${HOME}/.ssh/**`]: "---" },
};
expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "read", config)).toBe("deny");
expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "write", config)).toBe("deny");
@ -516,20 +391,18 @@ describe("checkAccessPolicy — deny list", () => {
});
it.skipIf(process.platform === "win32")(
"deny does not affect paths outside the deny glob",
'"---" rule does not affect paths outside its glob',
() => {
const config: AccessPolicyConfig = {
rules: { "/**": "rwx" },
deny: [`${HOME}/.ssh/**`],
rules: { "/**": "rwx", [`${HOME}/.ssh/**`]: "---" },
};
expect(checkAccessPolicy(`${HOME}/workspace/foo.py`, "read", config)).toBe("allow");
},
);
it("multiple deny entries — first match blocks", () => {
it("multiple narrowing rules block distinct subtrees", () => {
const config: AccessPolicyConfig = {
rules: { "/**": "rwx" },
deny: [`${HOME}/.ssh/**`, `${HOME}/.gnupg/**`],
rules: { "/**": "rwx", [`${HOME}/.ssh/**`]: "---", [`${HOME}/.gnupg/**`]: "---" },
};
expect(checkAccessPolicy(`${HOME}/.gnupg/secring.gpg`, "read", config)).toBe("deny");
});
@ -599,30 +472,30 @@ describe("checkAccessPolicy — rules", () => {
});
// ---------------------------------------------------------------------------
// checkAccessPolicy — default
// checkAccessPolicy — implicit fallback to "---"
// ---------------------------------------------------------------------------
describe("checkAccessPolicy — default", () => {
it("uses default when no rule matches", () => {
describe("checkAccessPolicy — implicit fallback to ---", () => {
it("denies all ops when no rule matches (implicit --- fallback)", () => {
const config: AccessPolicyConfig = {
rules: { [`${HOME}/**`]: "rwx" },
default: "r--",
};
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 = {
rules: { [`${HOME}/**`]: "rwx", "/**": "r--" },
};
expect(checkAccessPolicy("/etc/passwd", "read", config)).toBe("allow");
expect(checkAccessPolicy("/etc/passwd", "write", config)).toBe("deny");
});
it("absent default is treated as --- (deny all)", () => {
const config: AccessPolicyConfig = {
rules: { [`${HOME}/**`]: "rwx" },
};
expect(checkAccessPolicy("/etc/passwd", "read", config)).toBe("deny");
});
it("default --- denies all ops on unmatched paths", () => {
it("empty rules deny everything via implicit fallback", () => {
const config: AccessPolicyConfig = {
rules: { [`${HOME}/workspace/**`]: "rw-" },
default: "---",
};
expect(checkAccessPolicy("/tmp/foo", "read", config)).toBe("deny");
expect(checkAccessPolicy("/tmp/foo", "write", config)).toBe("deny");
@ -635,27 +508,26 @@ describe("checkAccessPolicy — default", () => {
// ---------------------------------------------------------------------------
describe("checkAccessPolicy — precedence integration", () => {
it("deny beats rules beats default — all three in play", () => {
it("narrowing rule beats broader allow — all in play", () => {
const config: AccessPolicyConfig = {
rules: {
"/**": "r--",
[`${HOME}/**`]: "rwx",
[`${HOME}/.ssh/**`]: "---",
},
deny: [`${HOME}/.ssh/**`],
default: "---",
};
// Rule allows home paths
// Broader home rule allows writes
expect(checkAccessPolicy(`${HOME}/workspace/foo`, "write", config)).toBe("allow");
// Deny beats the home rule
// Narrowing "---" beats the home rwx rule
expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "read", config)).toBe("deny");
// Outer rule applies outside home
// Outer "/**" rule applies outside home
expect(checkAccessPolicy("/etc/hosts", "read", config)).toBe("allow");
expect(checkAccessPolicy("/etc/hosts", "write", config)).toBe("deny");
// Nothing matches /proc → default ---
expect(checkAccessPolicy("/proc/self/mem", "read", config)).toBe("allow"); // matches /**
// "/proc/self/mem" matches "/**" (r--)
expect(checkAccessPolicy("/proc/self/mem", "read", config)).toBe("allow");
});
it("empty config denies everything (no rules, no default)", () => {
it("empty config denies everything (implicit --- fallback)", () => {
const config: AccessPolicyConfig = {};
expect(checkAccessPolicy("/anything", "read", config)).toBe("deny");
expect(checkAccessPolicy("/anything", "write", config)).toBe("deny");
@ -672,13 +544,11 @@ describe("checkAccessPolicy — precedence integration", () => {
// ---------------------------------------------------------------------------
describe("checkAccessPolicy — symlink resolved-path scenarios", () => {
it("denies read on resolved symlink target that falls under deny list", () => {
// ~/workspace/link → ~/.ssh/id_rsa (symlink in allowed dir to denied file)
// Caller passes the resolved path; deny wins.
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 = {
rules: { [`${HOME}/workspace/**`]: "rw-" },
deny: [`${HOME}/.ssh/**`],
default: "---",
rules: { [`${HOME}/workspace/**`]: "rw-", [`${HOME}/.ssh/**`]: "---" },
};
expect(checkAccessPolicy(`${HOME}/.ssh/id_rsa`, "read", config, HOME)).toBe("deny");
});
@ -691,7 +561,6 @@ describe("checkAccessPolicy — symlink resolved-path scenarios", () => {
[`${HOME}/workspace/**`]: "rw-",
[`${HOME}/workspace/secret/**`]: "r--",
},
default: "---",
};
expect(checkAccessPolicy(`${HOME}/workspace/secret/file.txt`, "write", config, HOME)).toBe(
"deny",
@ -702,13 +571,10 @@ describe("checkAccessPolicy — symlink resolved-path scenarios", () => {
);
});
it("symlink source path in allowed dir would be allowed; resolved denied target is denied", () => {
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.
// The symlink path itself looks allowed; the real target does not.
const config: AccessPolicyConfig = {
rules: { [`${HOME}/workspace/**`]: "rw-" },
deny: [`${HOME}/.aws/**`],
default: "---",
rules: { [`${HOME}/workspace/**`]: "rw-", [`${HOME}/.aws/**`]: "---" },
};
// Source path (the symlink) — allowed
expect(checkAccessPolicy(`${HOME}/workspace/creds`, "read", config, HOME)).toBe("allow");
@ -980,7 +846,7 @@ describe("resolveScriptKey", () => {
describe("applyScriptPolicyOverride", () => {
it("returns base policy unchanged when no scripts block", () => {
const base: AccessPolicyConfig = { rules: { "/**": "r--" }, default: "---" };
const base: AccessPolicyConfig = { rules: { "/**": "r--" } };
const { policy, hashMismatch } = applyScriptPolicyOverride(base, "/any/path");
expect(hashMismatch).toBeUndefined();
expect(policy).toBe(base);
@ -1021,10 +887,9 @@ describe("applyScriptPolicyOverride", () => {
}
});
it("returns override rules separately so seatbelt emits them after deny", () => {
it("returns override rules separately so seatbelt emits them after base rules", () => {
const base: AccessPolicyConfig = {
rules: { "/**": "r--" },
default: "---",
scripts: { "/my/script.sh": { rules: { [`${HOME}/.openclaw/credentials/`]: "r--" } } },
};
const { policy, overrideRules, hashMismatch } = applyScriptPolicyOverride(
@ -1035,23 +900,11 @@ describe("applyScriptPolicyOverride", () => {
// Base rules unchanged in policy
expect(policy.rules?.["/**"]).toBe("r--");
expect(policy.rules?.[`${HOME}/.openclaw/credentials/`]).toBeUndefined();
// Override rules returned separately — caller emits them after deny in seatbelt profile
// Override rules returned separately — caller emits them last in seatbelt profile
expect(overrideRules?.[`${HOME}/.openclaw/credentials/`]).toBe("r--");
expect(policy.scripts).toBeUndefined();
});
it("appends deny additively", () => {
const base: AccessPolicyConfig = {
deny: [`${HOME}/.ssh/**`],
scripts: {
"/my/script.sh": { deny: ["/tmp/**"] },
},
};
const { policy } = applyScriptPolicyOverride(base, "/my/script.sh");
expect(policy.deny).toContain(`${HOME}/.ssh/**`);
expect(policy.deny).toContain("/tmp/**");
});
it("override rules returned separately — base policy rule unchanged", () => {
const base: AccessPolicyConfig = {
rules: { [`${HOME}/workspace/**`]: "r--" },
@ -1099,7 +952,7 @@ describe("applyScriptPolicyOverride", () => {
// 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",
rules: { "/**": "rwx" },
scripts: { "~/bin/deploy.sh": { rules: { "/secret/**": "---" } } },
};
const { overrideRules, hashMismatch } = applyScriptPolicyOverride(base, absPath);

119
src/infra/access-policy.ts
View File

@ -43,17 +43,11 @@ function hasMidPathWildcard(pattern: string): boolean {
/**
* Validates and normalizes an AccessPolicyConfig for well-formedness.
* Returns an array of human-readable diagnostic strings; empty = valid.
* May mutate config.rules and config.deny in place (e.g. auto-expanding bare directory paths).
* May mutate config.rules in place (e.g. auto-expanding bare directory paths).
*/
export function validateAccessPolicyConfig(config: AccessPolicyConfig): string[] {
const errors: string[] = [];
if (config.default !== undefined && !PERM_STR_RE.test(config.default)) {
errors.push(
`access-policy.default "${config.default}" is invalid: must be exactly 3 chars (e.g. "rwx", "r--", "---")`,
);
}
if (config.rules) {
for (const [pattern, perm] of Object.entries(config.rules)) {
if (!pattern) {
@ -114,83 +108,6 @@ export function validateAccessPolicyConfig(config: AccessPolicyConfig): string[]
}
}
}
if (entry.deny) {
for (let i = 0; i < entry.deny.length; i++) {
if (!entry.deny[i]) {
errors.push(
`access-policy.scripts["${scriptPath}"].deny[${i}] must be a non-empty glob pattern`,
);
}
}
}
}
}
if (config.deny) {
for (let i = 0; i < config.deny.length; i++) {
const pattern = config.deny[i];
if (!pattern) {
errors.push(`access-policy.deny[${i}] must be a non-empty glob pattern`);
continue;
}
if (hasMidPathWildcard(pattern) && !_midPathWildcardWarned.has(`deny:${pattern}`)) {
_midPathWildcardWarned.add(`deny:${pattern}`);
errors.push(
`access-policy.deny entry "${pattern}" contains a mid-path wildcard — OS-level (bwrap/Seatbelt) enforcement cannot apply; tool-layer enforcement is still active.`,
);
}
// Bare-path auto-expand for directories: "~/.ssh" → "~/.ssh/**" so the
// entire directory tree is denied, not just the directory inode itself.
// For paths that exist and are confirmed files (statSync), keep the bare
// pattern — expanding to "/**" would only match non-existent children,
// leaving the file itself unprotected at both the tool layer and bwrap.
// Non-existent paths are treated as future directories and always expanded
// so the subtree is protected before the directory is created.
if (!pattern.endsWith("/") && !/[*?[]/.test(pattern)) {
const expandedForStat = pattern.startsWith("~")
? pattern.replace(/^~(?=$|[/\\])/, os.homedir())
: pattern;
let isExistingFile = false;
// For non-existent paths: treat as a future file (skip /**-expansion) when
// the last segment looks like a filename — has a dot but is not a dotfile-only
// name (e.g. ".ssh") and has a non-empty extension (e.g. "secrets.key").
// This preserves the intent of "deny: ['~/future-secrets.key']" where the
// user wants to protect that specific file once it is created.
// Plain names without an extension (e.g. "myfolder") are still treated as
// future directories and expanded to /**.
let looksLikeFile = false;
try {
isExistingFile = !fs.statSync(expandedForStat).isDirectory();
} catch {
const lastName =
expandedForStat
.replace(/[/\\]$/, "")
.split(/[/\\]/)
.pop() ?? "";
// Looks like a file when the last segment has a non-leading dot followed by a
// letter-containing extension — covers "secrets.key", "config.json".
// Note: pure dotnames like ".npmrc", ".env", ".ssh" do NOT match this regex
// (they have no non-leading dot) and are therefore expanded to /** below.
// 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}/**`;
config.deny[i] = fixed;
if (!_autoExpandedWarned.has(`deny:${pattern}`)) {
_autoExpandedWarned.add(`deny:${pattern}`);
errors.push(
`access-policy.deny["${pattern}"] auto-expanded to "${fixed}" so it covers all directory contents.`,
);
}
}
}
}
}
@ -313,9 +230,9 @@ export function findBestRule(
* Checks whether a given operation on targetPath is permitted by the config.
*
* Precedence:
* 1. deny[] — any matching glob always blocks, no override.
* 2. rules — longest matching glob wins; check the relevant bit.
* 3. default — catch-all (treated as "---" when absent).
* 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,
@ -341,16 +258,7 @@ export function checkAccessPolicy(
);
}
// 1. deny list always wins.
for (const pattern of config.deny ?? []) {
// Normalize so /private/tmp/** patterns match /tmp/** targets on macOS.
const expanded = normalizePlatformPath(expandPattern(pattern, homeDir));
if (matchesPattern(expanded)) {
return "deny";
}
}
// 2. rules — longest match wins (check both path and path + "/" variants).
// 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.rules ?? {})) {
@ -365,8 +273,8 @@ export function checkAccessPolicy(
return permAllows(bestPerm, op) ? "allow" : "deny";
}
// 3. default catch-all (absent = "---" = deny all).
return permAllows(config.default, op) ? "allow" : "deny";
// Implicit fallback: "---" (deny-all) when no rule matches.
return "deny";
}
/**
@ -637,16 +545,11 @@ export function applyScriptPolicyOverride(
// Build the merged policy WITHOUT the override rules merged in.
// Override rules are returned separately so the caller can emit them AFTER
// the deny list in the seatbelt profile (last-match-wins — grants must come
// after deny entries to override broad deny patterns like ~/.secrets/**).
// 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,
deny: [...(base.deny ?? []), ...(override.deny ?? [])],
};
if (merged.deny?.length === 0) {
delete merged.deny;
}
const merged: AccessPolicyConfig = { ...base };
return {
policy: merged,
overrideRules:

124
src/infra/exec-sandbox-bwrap.test.ts
View File

@ -14,14 +14,14 @@ const HOME = os.homedir();
// 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 default allows reads", () => {
const config: AccessPolicyConfig = { default: "r--" };
it("starts with --ro-bind / / when /** rule allows reads", () => {
const config: AccessPolicyConfig = { rules: { "/**": "r--" } };
const args = generateBwrapArgs(config, HOME);
expect(args.slice(0, 3)).toEqual(["--ro-bind", "/", "/"]);
});
it("does not use --ro-bind / / when default is ---", () => {
const config: AccessPolicyConfig = { default: "---" };
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(
@ -31,15 +31,14 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
});
it("ends with --", () => {
const config: AccessPolicyConfig = { default: "r--" };
const config: AccessPolicyConfig = { rules: { "/**": "r--" } };
const args = generateBwrapArgs(config, HOME);
expect(args[args.length - 1]).toBe("--");
});
it("adds --tmpfs for each deny entry", () => {
it('adds --tmpfs for "---" rules in permissive mode', () => {
const config: AccessPolicyConfig = {
deny: [`${HOME}/.ssh/**`, `${HOME}/.gnupg/**`],
default: "r--",
rules: { "/**": "r--", [`${HOME}/.ssh/**`]: "---", [`${HOME}/.gnupg/**`]: "---" },
};
const args = generateBwrapArgs(config, HOME);
const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean);
@ -47,10 +46,9 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
expect(tmpfsMounts).toContain(`${HOME}/.gnupg`);
});
it("expands ~ in deny patterns using homeDir", () => {
it('expands ~ in "---" rules using homeDir', () => {
const config: AccessPolicyConfig = {
deny: ["~/.ssh/**"],
default: "r--",
rules: { "/**": "r--", "~/.ssh/**": "---" },
};
const args = generateBwrapArgs(config, HOME);
const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean);
@ -59,8 +57,7 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
it("adds --bind for paths with w bit in rules", () => {
const config: AccessPolicyConfig = {
rules: { [`${HOME}/workspace/**`]: "rw-" },
default: "r--",
rules: { "/**": "r--", [`${HOME}/workspace/**`]: "rw-" },
};
const args = generateBwrapArgs(config, HOME);
const bindPairs: string[] = [];
@ -74,8 +71,7 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
it("does not add --bind for read-only rules on permissive base", () => {
const config: AccessPolicyConfig = {
rules: { "/usr/bin/**": "r--" },
default: "r--",
rules: { "/**": "r--", "/usr/bin/**": "r--" },
};
const args = generateBwrapArgs(config, HOME);
// /usr/bin should NOT appear as a --bind-try (it's already ro-bound via /)
@ -88,11 +84,9 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
expect(bindPairs).not.toContain("/usr/bin");
});
it("deny entry tmpfs appears in args regardless of rule for that path", () => {
it('"---" rule for sensitive path appears in args regardless of broader rule', () => {
const config: AccessPolicyConfig = {
rules: { [`${HOME}/**`]: "rwx" },
deny: [`${HOME}/.ssh/**`],
default: "r--",
rules: { "/**": "r--", [`${HOME}/**`]: "rwx", [`${HOME}/.ssh/**`]: "---" },
};
const args = generateBwrapArgs(config, HOME);
const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean);
@ -106,25 +100,22 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
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({ default: "r--" }, HOME);
const args = generateBwrapArgs({ rules: { "/**": "r--" } }, HOME);
const procIdx = args.indexOf("--proc");
expect(procIdx).toBeGreaterThan(-1);
expect(args[procIdx + 1]).toBe("/proc");
});
it("adds --tmpfs /tmp in permissive mode", () => {
const config: AccessPolicyConfig = { default: "r--" };
it("adds --tmpfs /tmp in permissive mode (/** allows reads)", () => {
const config: AccessPolicyConfig = { rules: { "/**": "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 even with no explicit /tmp rule", () => {
// Regression guard: the defaultAllowsRead guard on the /tmp block must prevent
// a writable tmpfs being mounted under default:"---" when no /tmp rule exists.
// explicitTmpPerm === null is true (no rule), but defaultAllowsRead is false,
// so the entire /tmp block must be skipped.
const config: AccessPolicyConfig = { default: "---" };
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");
@ -133,7 +124,7 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
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 = { default: "r--", rules: { "/tmp/**": "r--" } };
const config: AccessPolicyConfig = { rules: { "/**": "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");
@ -144,7 +135,7 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
// 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 config: AccessPolicyConfig = { rules: { "/**": "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
@ -154,19 +145,19 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
expect(bindMounts).toContain("/tmp");
});
it("does not add --tmpfs /tmp in restrictive mode (default: ---)", () => {
const config: AccessPolicyConfig = { default: "---" };
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 = { rules: { [`${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 default:"r--", --ro-bind / / makes everything readable. A narrowing
// With "/**":"r--", --ro-bind / / makes everything readable. A narrowing
// rule like "/secret/**": "---" must overlay --tmpfs so the path is hidden.
const config: AccessPolicyConfig = {
default: "r--",
rules: { [`${HOME}/secret/**`]: "---" },
rules: { "/**": "r--", [`${HOME}/secret/**`]: "---" },
};
const args = generateBwrapArgs(config, HOME);
const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean);
@ -179,15 +170,10 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
});
it("narrowing rule on an existing file does not emit --tmpfs (bwrap only accepts dirs)", () => {
// Reproducer: { "default": "r--", "rules": { "~/secrets.key": "---" } }
// where ~/secrets.key is an existing file. The old code emitted --tmpfs on the
// file path, causing bwrap to abort with "Not a directory". Fix: mirror the
// isDir guard already present in the deny[] branch.
// process.execPath is always an existing file — use it as the test target.
const filePath = process.execPath;
const config: AccessPolicyConfig = {
default: "r--",
rules: { [filePath]: "---" },
rules: { "/**": "r--", [filePath]: "---" },
};
const args = generateBwrapArgs(config, HOME);
const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean);
@ -198,8 +184,7 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
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 = {
default: "r--",
rules: { [`${HOME}/scripts/**`]: "--x" },
rules: { "/**": "r--", [`${HOME}/scripts/**`]: "--x" },
};
const args = generateBwrapArgs(config, HOME);
const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean);
@ -212,7 +197,6 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
// for a "---" rule would silently grant read access to paths that should
// be fully blocked.
const config: AccessPolicyConfig = {
default: "---",
rules: {
[`${HOME}/workspace/**`]: "rwx", // allowed: should produce --bind-try
[`${HOME}/workspace/private/**`]: "---", // denied: must NOT produce any mount
@ -232,7 +216,6 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
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 = {
default: "---",
rules: { [`${HOME}/scripts/**`]: "--x" },
};
const args = generateBwrapArgs(config, HOME);
@ -243,12 +226,11 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
});
it('"-w-" rule in restrictive mode emits --bind-try so writes do not silently fail', () => {
// A write-only rule ("-w-") under default:"---" now emits --bind-try so the path
// 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 = {
default: "---",
rules: { [`${HOME}/logs/**`]: "-w-" },
};
const args = generateBwrapArgs(config, HOME);
@ -259,12 +241,11 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
});
it('"-w-" rule in permissive mode emits --bind-try (write upgrade, reads already allowed)', () => {
// Under default:"r--", --ro-bind / / already grants reads everywhere.
// 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 = {
default: "r--",
rules: { [`${HOME}/output/**`]: "-w-" },
rules: { "/**": "r--", [`${HOME}/output/**`]: "-w-" },
};
const args = generateBwrapArgs(config, HOME);
const bindMounts = args
@ -278,9 +259,7 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
// The pattern must be silently ignored rather than binding /home.
const fakeHome = "/home/testuser";
const config: AccessPolicyConfig = {
default: "r--",
deny: ["/home/*/.ssh/**"],
rules: { "/home/*/.config/**": "---" },
rules: { "/**": "r--", "/home/*/.config/**": "---" },
};
const args = generateBwrapArgs(config, fakeHome);
const allMountTargets = args
@ -296,8 +275,7 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
// "/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 = {
default: "r--",
deny: ["/var/log/secret*"],
rules: { "/**": "r--", "/var/log/secret*": "---" },
};
const args = generateBwrapArgs(config, HOME);
const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean);
@ -315,7 +293,6 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
[`${HOME}/dev/secret/**`]: "r--",
[`${HOME}/dev/**`]: "rw-",
},
default: "---",
};
const args = generateBwrapArgs(config, HOME);
const bindArgs = args.filter((a) => a === "--bind-try" || a === "--ro-bind-try");
@ -333,12 +310,11 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
expect(bindArgs[secretIdx]).toBe("--ro-bind-try");
});
it('script override "-w-" under restrictive default emits --bind-try, not --tmpfs', () => {
// Greptile: permAllowsWrite && (r || defaultR) condition was wrong — for -w- under ---
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 = {
default: "---",
rules: { [`${HOME}/workspace/**`]: "rwx" },
};
const overrides = { [`${HOME}/logs/**`]: "-w-" as const };
@ -351,20 +327,16 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
expect(tmpfsMounts).not.toContain(`${HOME}/logs`);
});
it("skips --tmpfs for deny[] entry that resolves to an existing file (not a directory)", () => {
// /etc/hosts is a file on both macOS and Linux; bwrap --tmpfs rejects file paths.
// The deny entry is expanded to "/etc/hosts/**" by validateAccessPolicyConfig, and
// patternToPath strips the "/**" back to "/etc/hosts". generateBwrapArgs must not
// emit "--tmpfs /etc/hosts" — it should be silently skipped.
const config: AccessPolicyConfig = { default: "r--", deny: ["/etc/hosts/**"] };
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 = { rules: { "/**": "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 deny[] entry is skipped", () => {
// Use /etc/passwd (always a file) rather than /etc/hosts which is already in
// _bwrapFileDenyWarnedPaths from the generateBwrapArgs test above.
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");
@ -375,9 +347,11 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
}
});
it("still emits --tmpfs for deny[] entry that resolves to a directory", () => {
it('still emits --tmpfs for "---" rule that resolves to a directory', () => {
// Non-existent paths are treated as directories (forward-protection).
const config: AccessPolicyConfig = { default: "r--", deny: [`${HOME}/.nonexistent-dir/**`] };
const config: AccessPolicyConfig = {
rules: { "/**": "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`);
@ -386,8 +360,8 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
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({ default: "---", rules: { "/tmp/": "rw-" } }, HOME);
const withGlob = generateBwrapArgs({ default: "---", rules: { "/tmp/**": "rw-" } }, HOME);
const withSlash = generateBwrapArgs({ rules: { "/tmp/": "rw-" } }, HOME);
const withGlob = generateBwrapArgs({ rules: { "/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");
@ -397,17 +371,17 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
describe("wrapCommandWithBwrap", () => {
it("starts with bwrap", () => {
const result = wrapCommandWithBwrap("ls /tmp", { default: "r--" }, HOME);
const result = wrapCommandWithBwrap("ls /tmp", { rules: { "/**": "r--" } }, HOME);
expect(result).toMatch(/^bwrap /);
});
it("contains -- separator before the command", () => {
const result = wrapCommandWithBwrap("ls /tmp", { default: "r--" }, HOME);
const result = wrapCommandWithBwrap("ls /tmp", { rules: { "/**": "r--" } }, HOME);
expect(result).toContain("-- /bin/sh -c");
});
it("wraps command in /bin/sh -c", () => {
const result = wrapCommandWithBwrap("cat /etc/hosts", { default: "r--" }, HOME);
const result = wrapCommandWithBwrap("cat /etc/hosts", { rules: { "/**": "r--" } }, HOME);
expect(result).toContain("/bin/sh -c");
expect(result).toContain("cat /etc/hosts");
});

61
src/infra/exec-sandbox-bwrap.ts
View File

@ -27,11 +27,10 @@ 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
// 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.
// See docs/tools/access-policy.md — "File-specific deny[] entries on Linux".
const _bwrapFileDenyWarnedPaths = new Set<string>();
/** Reset the one-time file-deny warning set. Only for use in tests. */
export function _resetBwrapFileDenyWarnedPathsForTest(): void {
@ -43,10 +42,10 @@ export function _warnBwrapFileDenyOnce(filePath: string): void {
}
_bwrapFileDenyWarnedPaths.add(filePath);
console.error(
`[access-policy] bwrap: deny[] entry "${filePath}" resolves to a file — ` +
`[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, deny its parent directory instead.`,
`To protect this file at the OS layer on Linux, use a "---" rule on its parent directory instead.`,
);
}
@ -119,25 +118,27 @@ function permAllowsWrite(perm: PermStr): boolean {
* Generate bwrap argument array for the given permissions config.
*
* Strategy:
* 1. Start with --ro-bind / / (read-only view of entire host FS)
* 2. For each rule with w bit: upgrade to --bind (read-write)
* 3. For each deny[] entry: overlay with --tmpfs (empty, blocks reads too)
* 4. Add /tmp and /dev as writable tmpfs mounts (required for most processes)
* 5. When default is "---": use a more restrictive base (only bind explicit allow paths)
* 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 deny list so they win over
* broad deny patterns — mirrors the Seatbelt scriptOverrideRules behaviour.
* 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, PermStr>,
): string[] {
const args: string[] = [];
const defaultPerm = config.default ?? "---";
const defaultAllowsRead = defaultPerm[0] === "r";
// Determine base stance from the "/**" catch-all rule (replaces the removed `default` field).
const catchAllPerm = findBestRule("/**", config.rules ?? {}, homeDir) ?? "---";
const defaultAllowsRead = catchAllPerm[0] === "r";
if (defaultAllowsRead) {
// Permissive base: everything is read-only by default.
@ -199,12 +200,12 @@ export function generateBwrapArgs(
// 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 (defaultPerm[0] !== "r" && perm[0] === "r") {
} else if (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.
args.push("--ro-bind-try", p, p);
} else if (defaultPerm[0] === "r" && perm[0] !== "r") {
} else if (catchAllPerm[0] === "r" && perm[0] !== "r") {
// Permissive base + narrowing rule (no read bit): overlay with tmpfs so the
// path is hidden even though --ro-bind / / made it readable by default.
// This mirrors what deny[] does — without this, "---" rules under a permissive
@ -227,35 +228,7 @@ export function generateBwrapArgs(
// Permissive base + read-only rule: already covered by --ro-bind / /; no extra mount.
}
// deny[] entries: overlay with empty tmpfs — path exists but is empty.
// tmpfs overlay hides the real contents regardless of how the path was expressed.
// Guard: bwrap --tmpfs only accepts a directory as the mount point. deny[] entries
// like "~/.ssh/id_rsa" are unconditionally expanded to "~/.ssh/id_rsa/**" by
// validateAccessPolicyConfig and resolve back to the file path here. Passing a
// file to --tmpfs causes bwrap to error out ("Not a directory"). Non-existent
// paths are assumed to be directories (the common case for protecting future dirs).
for (const pattern of config.deny ?? []) {
const p = patternToPath(pattern, homeDir);
if (!p || p === "/") {
continue;
}
let isDir = true;
try {
isDir = fs.statSync(p).isDirectory();
} catch {
// Non-existent path — assume directory (forward-protection for dirs not yet created).
}
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);
}
}
// Script-specific override mounts — emitted after deny[] so they can reopen
// 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]) => {

125
src/infra/exec-sandbox-seatbelt.test.ts
View File

@ -16,54 +16,48 @@ describe("generateSeatbeltProfile", () => {
expect(profile).toMatch(/^\(version 1\)/);
});
it("uses (deny default) when default is ---", () => {
const profile = generateSeatbeltProfile({ default: "---" }, HOME);
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 default has any permission", () => {
const profile = generateSeatbeltProfile({ default: "r--" }, HOME);
it("uses (allow default) when /** rule has any permission", () => {
const profile = generateSeatbeltProfile({ rules: { "/**": "r--" } }, HOME);
expect(profile).toContain("(allow default)");
expect(profile).not.toContain("(deny default)");
});
it("includes system baseline reads when default is ---", () => {
const profile = generateSeatbeltProfile({ default: "---" }, HOME);
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("deny list entries appear as deny file-read*, file-write*, process-exec*", () => {
skipOnWindows("--- rule emits deny file-read*, file-write*, process-exec* for that path", () => {
const config: AccessPolicyConfig = {
deny: [`${HOME}/.ssh/**`],
default: "rwx",
rules: { "/**": "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*`);
// Should contain the path
expect(profile).toContain(HOME + "/.ssh");
});
skipOnWindows("expands ~ in deny patterns using provided homeDir", () => {
skipOnWindows("expands ~ in --- rules using provided homeDir", () => {
const config: AccessPolicyConfig = {
deny: ["~/.ssh/**"],
default: "rwx",
rules: { "/**": "rwx", "~/.ssh/**": "---" },
};
const profile = generateSeatbeltProfile(config, HOME);
expect(profile).toContain(HOME + "/.ssh");
// Should NOT contain literal ~
const denySection = profile.split("; Deny list")[1] ?? "";
expect(denySection).not.toContain("~/.ssh");
expect(profile).not.toContain("~/.ssh");
});
skipOnWindows("expands ~ in rules using provided homeDir", () => {
const config: AccessPolicyConfig = {
rules: { "~/**": "rw-" },
default: "---",
};
const profile = generateSeatbeltProfile(config, HOME);
expect(profile).toContain(HOME);
@ -72,7 +66,6 @@ describe("generateSeatbeltProfile", () => {
it("rw- rule emits allow read+write, deny exec for that path", () => {
const config: AccessPolicyConfig = {
rules: { [`${HOME}/workspace/**`]: "rw-" },
default: "---",
};
const profile = generateSeatbeltProfile(config, HOME);
expect(profile).toContain(`(allow file-read*`);
@ -83,7 +76,6 @@ describe("generateSeatbeltProfile", () => {
it("r-x rule emits allow read+exec, deny write for that path", () => {
const config: AccessPolicyConfig = {
rules: { "/usr/bin/**": "r-x" },
default: "---",
};
const profile = generateSeatbeltProfile(config, HOME);
const rulesSection = profile.split("; User-defined path rules")[1] ?? "";
@ -92,17 +84,19 @@ describe("generateSeatbeltProfile", () => {
expect(rulesSection).toContain("(deny file-write*");
});
it("deny list section appears after rules section", () => {
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 = {
rules: { [`${HOME}/**`]: "rwx" },
deny: [`${HOME}/.ssh/**`],
default: "r--",
rules: { [`${HOME}/**`]: "rwx", [`${HOME}/.ssh/**`]: "---" },
};
const profile = generateSeatbeltProfile(config, HOME);
const rulesIdx = profile.indexOf("; User-defined path rules");
const denyIdx = profile.indexOf("; Deny list");
expect(rulesIdx).toBeGreaterThan(-1);
expect(denyIdx).toBeGreaterThan(rulesIdx);
// 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", () => {
@ -110,18 +104,17 @@ describe("generateSeatbeltProfile", () => {
});
it("permissive base with no exec bit includes system baseline exec paths", () => {
// default:"r--" emits (deny process-exec* (subpath "/")) but must also allow
// "/**": "r--" emits (deny process-exec* (subpath "/")) but must also allow
// system binaries — otherwise ls, grep, cat all fail inside the sandbox.
const profile = generateSeatbeltProfile({ default: "r--" }, HOME);
const profile = generateSeatbeltProfile({ rules: { "/**": "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", () => {
// default:"rwx" already allows everything including exec — no extra baseline needed.
const profile = generateSeatbeltProfile({ default: "rwx" }, HOME);
// (allow default) covers exec; no separate baseline exec section needed
// "/**": "rwx" already allows everything including exec — no extra baseline needed.
const profile = generateSeatbeltProfile({ rules: { "/**": "rwx" } }, HOME);
expect(profile).toContain("(allow default)");
expect(profile).not.toContain("System baseline exec");
});
@ -131,7 +124,6 @@ describe("generateSeatbeltProfile", () => {
// Without deny ops in the override block, write would still be allowed.
const config: AccessPolicyConfig = {
rules: { [`${HOME}/workspace/**`]: "rw-" },
default: "---",
};
const overrideRules: Record<string, string> = { [`${HOME}/workspace/locked/**`]: "r--" };
const profile = generateSeatbeltProfile(config, HOME, overrideRules);
@ -141,70 +133,37 @@ describe("generateSeatbeltProfile", () => {
expect(overrideSection).toContain(`${HOME}/workspace/locked`);
});
it("omits /private/tmp baseline when default is --- and no rule grants /tmp", () => {
// In restrictive mode without an explicit /tmp rule, /tmp should NOT be in
// the baseline — emitting it unconditionally would contradict default: "---".
const config: AccessPolicyConfig = { default: "---" };
const profile = generateSeatbeltProfile(config, HOME);
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 config: AccessPolicyConfig = {
default: "---",
rules: { "/tmp/**": "rw-" },
};
const profile = generateSeatbeltProfile(config, HOME);
const profile = generateSeatbeltProfile({ rules: { "/tmp/**": "rw-" } }, HOME);
expect(profile).toContain(`(subpath "/private/tmp")`);
});
it("read-only /tmp rule does not grant file-write* on /private/tmp", () => {
// A policy of "/tmp/**": "r--" must grant reads but NOT writes to /tmp.
// The old code used (r || w) as the gate for both ops, so r-- inadvertently
// granted file-write* alongside read ops.
const config: AccessPolicyConfig = {
default: "---",
rules: { "/tmp/**": "r--" },
};
const profile = generateSeatbeltProfile(config, HOME);
// Read ops must be allowed for /tmp.
const profile = generateSeatbeltProfile({ rules: { "/tmp/**": "r--" } }, HOME);
expect(profile).toMatch(/allow file-read[^)]*\(subpath "\/private\/tmp"\)/);
// Write must NOT be present for /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 config: AccessPolicyConfig = {
default: "---",
rules: { "/tmp/**": "-w-" },
};
const profile = generateSeatbeltProfile(config, HOME);
const profile = generateSeatbeltProfile({ rules: { "/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 in restrictive mode", () => {
// Regression: when default:"---" and a rule grants exec on /tmp (e.g. "--x"),
// the seatbelt profile must emit (allow process-exec* (subpath "/private/tmp")).
// Without this fix, no exec allowance was emitted and binaries in /tmp could not
// be executed even though the policy explicitly permitted it.
const config: AccessPolicyConfig = {
default: "---",
rules: { "/tmp/**": "--x" },
};
const profile = generateSeatbeltProfile(config, HOME);
it("exec-only /tmp rule grants process-exec* on /private/tmp", () => {
const profile = generateSeatbeltProfile({ rules: { "/tmp/**": "--x" } }, HOME);
expect(profile).toMatch(/allow process-exec\*[^)]*\(subpath "\/private\/tmp"\)/);
// Read and write must NOT be granted.
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 in restrictive mode", () => {
const config: AccessPolicyConfig = {
default: "---",
rules: { "/tmp/**": "r-x" },
};
const profile = generateSeatbeltProfile(config, HOME);
it("r-x /tmp rule grants both read and exec on /private/tmp", () => {
const profile = generateSeatbeltProfile({ rules: { "/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"\)/);
@ -221,14 +180,15 @@ describe("generateSeatbeltProfile", () => {
// ---------------------------------------------------------------------------
skipOnWindows(
"deny list for sensitive path appears after workspace allow — symlink to deny target is blocked",
"--- 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 deny entry for ~/.ssh must appear after the workspace allow so it wins.
// The --- rule for ~/.ssh must appear after the workspace allow so it wins.
const config: AccessPolicyConfig = {
rules: { [`${HOME}/workspace/**`]: "rw-" },
deny: [`${HOME}/.ssh/**`],
default: "---",
rules: {
[`${HOME}/workspace/**`]: "rw-",
[`${HOME}/.ssh/**`]: "---",
},
};
const profile = generateSeatbeltProfile(config, HOME);
const workspaceAllowIdx = profile.indexOf("(allow file-read*");
@ -243,16 +203,11 @@ describe("generateSeatbeltProfile", () => {
skipOnWindows(
"restrictive rule on subdir appears after broader rw rule — covers symlink to restricted subtree",
() => {
// ~/workspace/** is rw-, ~/workspace/secret/** is r--.
// A symlink ~/workspace/link → ~/workspace/secret/file: seatbelt sees the
// real path ~/workspace/secret/... which must hit the narrower r-- rule.
// The deny write for secret must appear after the allow write for workspace.
const config: AccessPolicyConfig = {
rules: {
[`${HOME}/workspace/**`]: "rw-",
[`${HOME}/workspace/secret/**`]: "r--",
},
default: "---",
};
const profile = generateSeatbeltProfile(config, HOME);
const workspaceWriteIdx = profile.indexOf("(allow file-write*");
@ -265,11 +220,9 @@ describe("generateSeatbeltProfile", () => {
it("glob patterns are stripped to their longest concrete prefix", () => {
const config: AccessPolicyConfig = {
deny: ["/Users/kaveri/.ssh/**"],
default: "rwx",
rules: { "/Users/kaveri/.ssh/**": "---", "/**": "rwx" },
};
const profile = generateSeatbeltProfile(config, "/Users/kaveri");
// ** should not appear in profile — stripped to subpath
expect(profile).not.toContain("**");
expect(profile).toContain("/Users/kaveri/.ssh");
});

28
src/infra/exec-sandbox-seatbelt.ts
View File

@ -170,10 +170,11 @@ export function generateSeatbeltProfile(
lines.push("(version 1)");
lines.push("");
// Determine base stance from default permission.
const defaultPerm = config.default ?? "---";
// Determine base stance from the "/**" catch-all rule (replaces the removed `default` field).
const catchAllPerm = findBestRule("/**", config.rules ?? {}, homeDir) ?? "---";
const defaultPerm = catchAllPerm; // alias for readability below
const defaultAllowsAnything =
defaultPerm[0] === "r" || defaultPerm[1] === "w" || defaultPerm[2] === "x";
catchAllPerm[0] === "r" || catchAllPerm[1] === "w" || catchAllPerm[2] === "x";
if (defaultAllowsAnything) {
// Permissive base: allow everything, then restrict.
@ -209,7 +210,7 @@ export function generateSeatbeltProfile(
// unconditionally granting /tmp access when default: "---".
// Use "/tmp/." so glob rules like "/tmp/**" match correctly — findBestRule
// on "/tmp" alone would miss "/**"-suffixed patterns that only match descendants.
const tmpPerm = findBestRule("/tmp/.", config.rules ?? {}, homeDir) ?? config.default ?? "---";
const tmpPerm = findBestRule("/tmp/.", config.rules ?? {}, homeDir) ?? "---";
// 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") {
@ -262,24 +263,7 @@ export function generateSeatbeltProfile(
}
}
// deny[] entries — always win over base rules.
if ((config.deny ?? []).length > 0) {
lines.push("");
lines.push("; Deny list — wins over base rules");
for (const pattern of config.deny ?? []) {
for (const expanded of expandSbplAliases(pattern)) {
const matcher = patternToSbplMatcher(expanded, homeDir);
if (!matcher) {
continue;
}
lines.push(`(deny ${SEATBELT_READ_OPS} ${matcher})`);
lines.push(`(deny ${SEATBELT_WRITE_OPS} ${matcher})`);
lines.push(`(deny ${SEATBELT_EXEC_OPS} ${matcher})`);
}
}
}
// Script-override rules emitted last — they win over deny entries above.
// 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) {