refactor(security): extract safeBins trust resolver

docs/tools/exec.md

@@ -51,7 +51,7 @@ Notes:
 - `tools.exec.ask` (default: `on-miss`)
 - `tools.exec.node` (default: unset)
 - `tools.exec.pathPrepend`: list of directories to prepend to `PATH` for exec runs (gateway + sandbox only).
-- `tools.exec.safeBins`: stdin-only safe binaries that can run without explicit allowlist entries (resolved path must come from trusted binary directories).
+- `tools.exec.safeBins`: stdin-only safe binaries that can run without explicit allowlist entries. For behavior details, see [Safe bins](/tools/exec-approvals#safe-bins-stdin-only).
 
 Example:
 

src/infra/exec-approvals-allowlist.ts

@@ -12,48 +12,7 @@ import {
   type CommandResolution,
   type ExecCommandSegment,
 } from "./exec-approvals-analysis.js";
-
-const DEFAULT_SAFE_BIN_TRUSTED_DIRS = [
-  "/bin",
-  "/usr/bin",
-  "/usr/local/bin",
-  "/opt/homebrew/bin",
-  "/opt/local/bin",
-  "/snap/bin",
-  "/run/current-system/sw/bin",
-];
-
-function normalizeTrustedDir(value: string): string | null {
-  const trimmed = value.trim();
-  if (!trimmed) {
-    return null;
-  }
-  return path.resolve(trimmed);
-}
-
-function collectTrustedSafeBinDirs(): Set<string> {
-  const trusted = new Set<string>();
-  for (const entry of DEFAULT_SAFE_BIN_TRUSTED_DIRS) {
-    const normalized = normalizeTrustedDir(entry);
-    if (normalized) {
-      trusted.add(normalized);
-    }
-  }
-  const pathEntries = (process.env.PATH ?? process.env.Path ?? "")
-    .split(path.delimiter)
-    .map((entry) => normalizeTrustedDir(entry))
-    .filter((entry): entry is string => Boolean(entry));
-  for (const entry of pathEntries) {
-    trusted.add(entry);
-  }
-  return trusted;
-}
-
-const TRUSTED_SAFE_BIN_DIRS = collectTrustedSafeBinDirs();
-
-function isTrustedSafeBinPath(resolvedPath: string): boolean {
-  return TRUSTED_SAFE_BIN_DIRS.has(path.dirname(path.resolve(resolvedPath)));
-}
+import { isTrustedSafeBinPath } from "./exec-safe-bin-trust.js";
 
 function isPathLikeToken(value: string): boolean {
   const trimmed = value.trim();
@@ -109,6 +68,7 @@ export function isSafeBinUsage(params: {
   safeBins: Set<string>;
   cwd?: string;
   fileExists?: (filePath: string) => boolean;
+  trustedSafeBinDirs?: ReadonlySet<string>;
 }): boolean {
   // Windows host exec uses PowerShell, which has different parsing/expansion rules.
   // Keep safeBins conservative there (require explicit allowlist entries).
@@ -132,7 +92,12 @@ export function isSafeBinUsage(params: {
   if (!resolution?.resolvedPath) {
     return false;
   }
-  if (!isTrustedSafeBinPath(resolution.resolvedPath)) {
+  if (
+    !isTrustedSafeBinPath({
+      resolvedPath: resolution.resolvedPath,
+      trustedDirs: params.trustedSafeBinDirs,
+    })
+  ) {
     return false;
   }
   const cwd = params.cwd ?? process.cwd();

src/infra/exec-approvals.test.ts

@@ -385,44 +385,60 @@ describe("exec approvals shell allowlist (chained commands)", () => {
 });
 
 describe("exec approvals safe bins", () => {
-  it("allows safe bins with non-path args", () => {
-    if (process.platform === "win32") {
-      return;
-    }
-    const dir = makeTempDir();
-    const ok = isSafeBinUsage({
-      argv: ["jq", ".foo"],
-      resolution: {
-        rawExecutable: "jq",
-        resolvedPath: "/usr/bin/jq",
-        executableName: "jq",
-      },
-      safeBins: normalizeSafeBins(["jq"]),
-      cwd: dir,
-    });
-    expect(ok).toBe(true);
-  });
+  type SafeBinCase = {
+    name: string;
+    argv: string[];
+    resolvedPath: string;
+    expected: boolean;
+    cwd?: string;
+    setup?: (cwd: string) => void;
+  };
 
-  it("blocks safe bins with file args", () => {
-    if (process.platform === "win32") {
-      return;
-    }
-    const dir = makeTempDir();
-    fs.writeFileSync(path.join(dir, "secret.json"), "{}");
-    const ok = isSafeBinUsage({
+  const cases: SafeBinCase[] = [
+    {
+      name: "allows safe bins with non-path args",
+      argv: ["jq", ".foo"],
+      resolvedPath: "/usr/bin/jq",
+      expected: true,
+    },
+    {
+      name: "blocks safe bins with file args",
       argv: ["jq", ".foo", "secret.json"],
-      resolution: {
-        rawExecutable: "jq",
-        resolvedPath: "/usr/bin/jq",
-        executableName: "jq",
-      },
-      safeBins: normalizeSafeBins(["jq"]),
-      cwd: dir,
-    });
-    expect(ok).toBe(false);
-  });
+      resolvedPath: "/usr/bin/jq",
+      expected: false,
+      setup: (cwd) => fs.writeFileSync(path.join(cwd, "secret.json"), "{}"),
+    },
+    {
+      name: "blocks safe bins resolved from untrusted directories",
+      argv: ["jq", ".foo"],
+      resolvedPath: "/tmp/evil-bin/jq",
+      expected: false,
+      cwd: "/tmp",
+    },
+  ];
 
-  it("blocks safe bins resolved from untrusted directories", () => {
+  for (const testCase of cases) {
+    it(testCase.name, () => {
+      if (process.platform === "win32") {
+        return;
+      }
+      const cwd = testCase.cwd ?? makeTempDir();
+      testCase.setup?.(cwd);
+      const ok = isSafeBinUsage({
+        argv: testCase.argv,
+        resolution: {
+          rawExecutable: "jq",
+          resolvedPath: testCase.resolvedPath,
+          executableName: "jq",
+        },
+        safeBins: normalizeSafeBins(["jq"]),
+        cwd,
+      });
+      expect(ok).toBe(testCase.expected);
+    });
+  }
+
+  it("supports injected trusted safe-bin dirs for tests/callers", () => {
     if (process.platform === "win32") {
       return;
     }
@@ -430,13 +446,14 @@ describe("exec approvals safe bins", () => {
       argv: ["jq", ".foo"],
       resolution: {
         rawExecutable: "jq",
-        resolvedPath: "/tmp/evil-bin/jq",
+        resolvedPath: "/custom/bin/jq",
         executableName: "jq",
       },
       safeBins: normalizeSafeBins(["jq"]),
+      trustedSafeBinDirs: new Set(["/custom/bin"]),
       cwd: "/tmp",
     });
-    expect(ok).toBe(false);
+    expect(ok).toBe(true);
   });
 });
 

src/infra/exec-safe-bin-trust.test.ts

@@ -0,0 +1,57 @@
+import path from "node:path";
+import { describe, expect, it } from "vitest";
+import {
+  buildTrustedSafeBinDirs,
+  getTrustedSafeBinDirs,
+  isTrustedSafeBinPath,
+} from "./exec-safe-bin-trust.js";
+
+describe("exec safe bin trust", () => {
+  it("builds trusted dirs from defaults and injected PATH", () => {
+    const dirs = buildTrustedSafeBinDirs({
+      pathEnv: "/custom/bin:/alt/bin:/custom/bin",
+      delimiter: ":",
+      baseDirs: ["/usr/bin"],
+    });
+
+    expect(dirs.has(path.resolve("/usr/bin"))).toBe(true);
+    expect(dirs.has(path.resolve("/custom/bin"))).toBe(true);
+    expect(dirs.has(path.resolve("/alt/bin"))).toBe(true);
+    expect(dirs.size).toBe(3);
+  });
+
+  it("memoizes trusted dirs per PATH snapshot", () => {
+    const a = getTrustedSafeBinDirs({
+      pathEnv: "/first/bin",
+      delimiter: ":",
+      refresh: true,
+    });
+    const b = getTrustedSafeBinDirs({
+      pathEnv: "/first/bin",
+      delimiter: ":",
+    });
+    const c = getTrustedSafeBinDirs({
+      pathEnv: "/second/bin",
+      delimiter: ":",
+    });
+
+    expect(a).toBe(b);
+    expect(c).not.toBe(b);
+  });
+
+  it("validates resolved paths using injected trusted dirs", () => {
+    const trusted = new Set([path.resolve("/usr/bin")]);
+    expect(
+      isTrustedSafeBinPath({
+        resolvedPath: "/usr/bin/jq",
+        trustedDirs: trusted,
+      }),
+    ).toBe(true);
+    expect(
+      isTrustedSafeBinPath({
+        resolvedPath: "/tmp/evil/jq",
+        trustedDirs: trusted,
+      }),
+    ).toBe(false);
+  });
+});

src/infra/exec-safe-bin-trust.ts

@@ -0,0 +1,101 @@
+import path from "node:path";
+
+const DEFAULT_SAFE_BIN_TRUSTED_DIRS = [
+  "/bin",
+  "/usr/bin",
+  "/usr/local/bin",
+  "/opt/homebrew/bin",
+  "/opt/local/bin",
+  "/snap/bin",
+  "/run/current-system/sw/bin",
+];
+
+type TrustedSafeBinDirsParams = {
+  pathEnv?: string | null;
+  delimiter?: string;
+  baseDirs?: readonly string[];
+};
+
+type TrustedSafeBinPathParams = {
+  resolvedPath: string;
+  trustedDirs?: ReadonlySet<string>;
+  pathEnv?: string | null;
+  delimiter?: string;
+};
+
+type TrustedSafeBinCache = {
+  key: string;
+  dirs: Set<string>;
+};
+
+let trustedSafeBinCache: TrustedSafeBinCache | null = null;
+
+function normalizeTrustedDir(value: string): string | null {
+  const trimmed = value.trim();
+  if (!trimmed) {
+    return null;
+  }
+  return path.resolve(trimmed);
+}
+
+function buildTrustedSafeBinCacheKey(pathEnv: string, delimiter: string): string {
+  return `${delimiter}\u0000${pathEnv}`;
+}
+
+export function buildTrustedSafeBinDirs(params: TrustedSafeBinDirsParams = {}): Set<string> {
+  const delimiter = params.delimiter ?? path.delimiter;
+  const pathEnv = params.pathEnv ?? "";
+  const baseDirs = params.baseDirs ?? DEFAULT_SAFE_BIN_TRUSTED_DIRS;
+  const trusted = new Set<string>();
+
+  for (const entry of baseDirs) {
+    const normalized = normalizeTrustedDir(entry);
+    if (normalized) {
+      trusted.add(normalized);
+    }
+  }
+
+  const pathEntries = pathEnv
+    .split(delimiter)
+    .map((entry) => normalizeTrustedDir(entry))
+    .filter((entry): entry is string => Boolean(entry));
+  for (const entry of pathEntries) {
+    trusted.add(entry);
+  }
+
+  return trusted;
+}
+
+export function getTrustedSafeBinDirs(
+  params: {
+    pathEnv?: string | null;
+    delimiter?: string;
+    refresh?: boolean;
+  } = {},
+): Set<string> {
+  const delimiter = params.delimiter ?? path.delimiter;
+  const pathEnv = params.pathEnv ?? process.env.PATH ?? process.env.Path ?? "";
+  const key = buildTrustedSafeBinCacheKey(pathEnv, delimiter);
+
+  if (!params.refresh && trustedSafeBinCache?.key === key) {
+    return trustedSafeBinCache.dirs;
+  }
+
+  const dirs = buildTrustedSafeBinDirs({
+    pathEnv,
+    delimiter,
+  });
+  trustedSafeBinCache = { key, dirs };
+  return dirs;
+}
+
+export function isTrustedSafeBinPath(params: TrustedSafeBinPathParams): boolean {
+  const trustedDirs =
+    params.trustedDirs ??
+    getTrustedSafeBinDirs({
+      pathEnv: params.pathEnv,
+      delimiter: params.delimiter,
+    });
+  const resolvedDir = path.dirname(path.resolve(params.resolvedPath));
+  return trustedDirs.has(resolvedDir);
+}