refactor(exec-approvals): split allowlist evaluation module

src/infra/exec-approvals-allowlist.ts

@@ -0,0 +1,296 @@
+import fs from "node:fs";
+import path from "node:path";
+import type { ExecAllowlistEntry } from "./exec-approvals.js";
+import {
+  DEFAULT_SAFE_BINS,
+  analyzeShellCommand,
+  isWindowsPlatform,
+  matchAllowlist,
+  resolveAllowlistCandidatePath,
+  splitCommandChain,
+  type ExecCommandAnalysis,
+  type CommandResolution,
+  type ExecCommandSegment,
+} from "./exec-approvals-analysis.js";
+
+function isPathLikeToken(value: string): boolean {
+  const trimmed = value.trim();
+  if (!trimmed) {
+    return false;
+  }
+  if (trimmed === "-") {
+    return false;
+  }
+  if (trimmed.startsWith("./") || trimmed.startsWith("../") || trimmed.startsWith("~")) {
+    return true;
+  }
+  if (trimmed.startsWith("/")) {
+    return true;
+  }
+  return /^[A-Za-z]:[\\/]/.test(trimmed);
+}
+
+function defaultFileExists(filePath: string): boolean {
+  try {
+    return fs.existsSync(filePath);
+  } catch {
+    return false;
+  }
+}
+
+export function normalizeSafeBins(entries?: string[]): Set<string> {
+  if (!Array.isArray(entries)) {
+    return new Set();
+  }
+  const normalized = entries
+    .map((entry) => entry.trim().toLowerCase())
+    .filter((entry) => entry.length > 0);
+  return new Set(normalized);
+}
+
+export function resolveSafeBins(entries?: string[] | null): Set<string> {
+  if (entries === undefined) {
+    return normalizeSafeBins(DEFAULT_SAFE_BINS);
+  }
+  return normalizeSafeBins(entries ?? []);
+}
+
+export function isSafeBinUsage(params: {
+  argv: string[];
+  resolution: CommandResolution | null;
+  safeBins: Set<string>;
+  cwd?: string;
+  fileExists?: (filePath: string) => boolean;
+}): boolean {
+  if (params.safeBins.size === 0) {
+    return false;
+  }
+  const resolution = params.resolution;
+  const execName = resolution?.executableName?.toLowerCase();
+  if (!execName) {
+    return false;
+  }
+  const matchesSafeBin =
+    params.safeBins.has(execName) ||
+    (process.platform === "win32" && params.safeBins.has(path.parse(execName).name));
+  if (!matchesSafeBin) {
+    return false;
+  }
+  if (!resolution?.resolvedPath) {
+    return false;
+  }
+  const cwd = params.cwd ?? process.cwd();
+  const exists = params.fileExists ?? defaultFileExists;
+  const argv = params.argv.slice(1);
+  for (let i = 0; i < argv.length; i += 1) {
+    const token = argv[i];
+    if (!token) {
+      continue;
+    }
+    if (token === "-") {
+      continue;
+    }
+    if (token.startsWith("-")) {
+      const eqIndex = token.indexOf("=");
+      if (eqIndex > 0) {
+        const value = token.slice(eqIndex + 1);
+        if (value && (isPathLikeToken(value) || exists(path.resolve(cwd, value)))) {
+          return false;
+        }
+      }
+      continue;
+    }
+    if (isPathLikeToken(token)) {
+      return false;
+    }
+    if (exists(path.resolve(cwd, token))) {
+      return false;
+    }
+  }
+  return true;
+}
+
+export type ExecAllowlistEvaluation = {
+  allowlistSatisfied: boolean;
+  allowlistMatches: ExecAllowlistEntry[];
+};
+
+function evaluateSegments(
+  segments: ExecCommandSegment[],
+  params: {
+    allowlist: ExecAllowlistEntry[];
+    safeBins: Set<string>;
+    cwd?: string;
+    skillBins?: Set<string>;
+    autoAllowSkills?: boolean;
+  },
+): { satisfied: boolean; matches: ExecAllowlistEntry[] } {
+  const matches: ExecAllowlistEntry[] = [];
+  const allowSkills = params.autoAllowSkills === true && (params.skillBins?.size ?? 0) > 0;
+
+  const satisfied = segments.every((segment) => {
+    const candidatePath = resolveAllowlistCandidatePath(segment.resolution, params.cwd);
+    const candidateResolution =
+      candidatePath && segment.resolution
+        ? { ...segment.resolution, resolvedPath: candidatePath }
+        : segment.resolution;
+    const match = matchAllowlist(params.allowlist, candidateResolution);
+    if (match) {
+      matches.push(match);
+    }
+    const safe = isSafeBinUsage({
+      argv: segment.argv,
+      resolution: segment.resolution,
+      safeBins: params.safeBins,
+      cwd: params.cwd,
+    });
+    const skillAllow =
+      allowSkills && segment.resolution?.executableName
+        ? params.skillBins?.has(segment.resolution.executableName)
+        : false;
+    return Boolean(match || safe || skillAllow);
+  });
+
+  return { satisfied, matches };
+}
+
+export function evaluateExecAllowlist(params: {
+  analysis: ExecCommandAnalysis;
+  allowlist: ExecAllowlistEntry[];
+  safeBins: Set<string>;
+  cwd?: string;
+  skillBins?: Set<string>;
+  autoAllowSkills?: boolean;
+}): ExecAllowlistEvaluation {
+  const allowlistMatches: ExecAllowlistEntry[] = [];
+  if (!params.analysis.ok || params.analysis.segments.length === 0) {
+    return { allowlistSatisfied: false, allowlistMatches };
+  }
+
+  // If the analysis contains chains, evaluate each chain part separately
+  if (params.analysis.chains) {
+    for (const chainSegments of params.analysis.chains) {
+      const result = evaluateSegments(chainSegments, {
+        allowlist: params.allowlist,
+        safeBins: params.safeBins,
+        cwd: params.cwd,
+        skillBins: params.skillBins,
+        autoAllowSkills: params.autoAllowSkills,
+      });
+      if (!result.satisfied) {
+        return { allowlistSatisfied: false, allowlistMatches: [] };
+      }
+      allowlistMatches.push(...result.matches);
+    }
+    return { allowlistSatisfied: true, allowlistMatches };
+  }
+
+  // No chains, evaluate all segments together
+  const result = evaluateSegments(params.analysis.segments, {
+    allowlist: params.allowlist,
+    safeBins: params.safeBins,
+    cwd: params.cwd,
+    skillBins: params.skillBins,
+    autoAllowSkills: params.autoAllowSkills,
+  });
+  return { allowlistSatisfied: result.satisfied, allowlistMatches: result.matches };
+}
+
+export type ExecAllowlistAnalysis = {
+  analysisOk: boolean;
+  allowlistSatisfied: boolean;
+  allowlistMatches: ExecAllowlistEntry[];
+  segments: ExecCommandSegment[];
+};
+
+/**
+ * Evaluates allowlist for shell commands (including &&, ||, ;) and returns analysis metadata.
+ */
+export function evaluateShellAllowlist(params: {
+  command: string;
+  allowlist: ExecAllowlistEntry[];
+  safeBins: Set<string>;
+  cwd?: string;
+  env?: NodeJS.ProcessEnv;
+  skillBins?: Set<string>;
+  autoAllowSkills?: boolean;
+  platform?: string | null;
+}): ExecAllowlistAnalysis {
+  const chainParts = isWindowsPlatform(params.platform) ? null : splitCommandChain(params.command);
+  if (!chainParts) {
+    const analysis = analyzeShellCommand({
+      command: params.command,
+      cwd: params.cwd,
+      env: params.env,
+      platform: params.platform,
+    });
+    if (!analysis.ok) {
+      return {
+        analysisOk: false,
+        allowlistSatisfied: false,
+        allowlistMatches: [],
+        segments: [],
+      };
+    }
+    const evaluation = evaluateExecAllowlist({
+      analysis,
+      allowlist: params.allowlist,
+      safeBins: params.safeBins,
+      cwd: params.cwd,
+      skillBins: params.skillBins,
+      autoAllowSkills: params.autoAllowSkills,
+    });
+    return {
+      analysisOk: true,
+      allowlistSatisfied: evaluation.allowlistSatisfied,
+      allowlistMatches: evaluation.allowlistMatches,
+      segments: analysis.segments,
+    };
+  }
+
+  const allowlistMatches: ExecAllowlistEntry[] = [];
+  const segments: ExecCommandSegment[] = [];
+
+  for (const part of chainParts) {
+    const analysis = analyzeShellCommand({
+      command: part,
+      cwd: params.cwd,
+      env: params.env,
+      platform: params.platform,
+    });
+    if (!analysis.ok) {
+      return {
+        analysisOk: false,
+        allowlistSatisfied: false,
+        allowlistMatches: [],
+        segments: [],
+      };
+    }
+
+    segments.push(...analysis.segments);
+    const evaluation = evaluateExecAllowlist({
+      analysis,
+      allowlist: params.allowlist,
+      safeBins: params.safeBins,
+      cwd: params.cwd,
+      skillBins: params.skillBins,
+      autoAllowSkills: params.autoAllowSkills,
+    });
+    allowlistMatches.push(...evaluation.allowlistMatches);
+    if (!evaluation.allowlistSatisfied) {
+      return {
+        analysisOk: true,
+        allowlistSatisfied: false,
+        allowlistMatches,
+        segments,
+      };
+    }
+  }
+
+  return {
+    analysisOk: true,
+    allowlistSatisfied: true,
+    allowlistMatches,
+    segments,
+  };
+}

src/infra/exec-approvals-analysis.ts

@@ -18,7 +18,7 @@ function expandHome(value: string): string {
   return value;
 }
 
-type CommandResolution = {
+export type CommandResolution = {
   rawExecutable: string;
   resolvedPath?: string;
   executableName: string;
@@ -185,7 +185,7 @@ function matchesPattern(pattern: string, target: string): boolean {
   return regex.test(normalizedTarget);
 }
 
-function resolveAllowlistCandidatePath(
+export function resolveAllowlistCandidatePath(
   resolution: CommandResolution | null,
   cwd?: string,
 ): string | undefined {
@@ -575,7 +575,7 @@ function analyzeWindowsShellCommand(params: {
   };
 }
 
-function isWindowsPlatform(platform?: string | null): boolean {
+export function isWindowsPlatform(platform?: string | null): boolean {
   const normalized = String(platform ?? "")
     .trim()
     .toLowerCase();
@@ -671,258 +671,11 @@ function parseSegmentsFromParts(
   return segments;
 }
 
-export function analyzeShellCommand(params: {
-  command: string;
-  cwd?: string;
-  env?: NodeJS.ProcessEnv;
-  platform?: string | null;
-}): ExecCommandAnalysis {
-  if (isWindowsPlatform(params.platform)) {
-    return analyzeWindowsShellCommand(params);
-  }
-  // First try splitting by chain operators (&&, ||, ;)
-  const chainParts = splitCommandChain(params.command);
-  if (chainParts) {
-    const chains: ExecCommandSegment[][] = [];
-    const allSegments: ExecCommandSegment[] = [];
-
-    for (const part of chainParts) {
-      const pipelineSplit = splitShellPipeline(part);
-      if (!pipelineSplit.ok) {
-        return { ok: false, reason: pipelineSplit.reason, segments: [] };
-      }
-      const segments = parseSegmentsFromParts(pipelineSplit.segments, params.cwd, params.env);
-      if (!segments) {
-        return { ok: false, reason: "unable to parse shell segment", segments: [] };
-      }
-      chains.push(segments);
-      allSegments.push(...segments);
-    }
-
-    return { ok: true, segments: allSegments, chains };
-  }
-
-  // No chain operators, parse as simple pipeline
-  const split = splitShellPipeline(params.command);
-  if (!split.ok) {
-    return { ok: false, reason: split.reason, segments: [] };
-  }
-  const segments = parseSegmentsFromParts(split.segments, params.cwd, params.env);
-  if (!segments) {
-    return { ok: false, reason: "unable to parse shell segment", segments: [] };
-  }
-  return { ok: true, segments };
-}
-
-export function analyzeArgvCommand(params: {
-  argv: string[];
-  cwd?: string;
-  env?: NodeJS.ProcessEnv;
-}): ExecCommandAnalysis {
-  const argv = params.argv.filter((entry) => entry.trim().length > 0);
-  if (argv.length === 0) {
-    return { ok: false, reason: "empty argv", segments: [] };
-  }
-  return {
-    ok: true,
-    segments: [
-      {
-        raw: argv.join(" "),
-        argv,
-        resolution: resolveCommandResolutionFromArgv(argv, params.cwd, params.env),
-      },
-    ],
-  };
-}
-
-function isPathLikeToken(value: string): boolean {
-  const trimmed = value.trim();
-  if (!trimmed) {
-    return false;
-  }
-  if (trimmed === "-") {
-    return false;
-  }
-  if (trimmed.startsWith("./") || trimmed.startsWith("../") || trimmed.startsWith("~")) {
-    return true;
-  }
-  if (trimmed.startsWith("/")) {
-    return true;
-  }
-  return /^[A-Za-z]:[\\/]/.test(trimmed);
-}
-
-function defaultFileExists(filePath: string): boolean {
-  try {
-    return fs.existsSync(filePath);
-  } catch {
-    return false;
-  }
-}
-
-export function normalizeSafeBins(entries?: string[]): Set<string> {
-  if (!Array.isArray(entries)) {
-    return new Set();
-  }
-  const normalized = entries
-    .map((entry) => entry.trim().toLowerCase())
-    .filter((entry) => entry.length > 0);
-  return new Set(normalized);
-}
-
-export function resolveSafeBins(entries?: string[] | null): Set<string> {
-  if (entries === undefined) {
-    return normalizeSafeBins(DEFAULT_SAFE_BINS);
-  }
-  return normalizeSafeBins(entries ?? []);
-}
-
-export function isSafeBinUsage(params: {
-  argv: string[];
-  resolution: CommandResolution | null;
-  safeBins: Set<string>;
-  cwd?: string;
-  fileExists?: (filePath: string) => boolean;
-}): boolean {
-  if (params.safeBins.size === 0) {
-    return false;
-  }
-  const resolution = params.resolution;
-  const execName = resolution?.executableName?.toLowerCase();
-  if (!execName) {
-    return false;
-  }
-  const matchesSafeBin =
-    params.safeBins.has(execName) ||
-    (process.platform === "win32" && params.safeBins.has(path.parse(execName).name));
-  if (!matchesSafeBin) {
-    return false;
-  }
-  if (!resolution?.resolvedPath) {
-    return false;
-  }
-  const cwd = params.cwd ?? process.cwd();
-  const exists = params.fileExists ?? defaultFileExists;
-  const argv = params.argv.slice(1);
-  for (let i = 0; i < argv.length; i += 1) {
-    const token = argv[i];
-    if (!token) {
-      continue;
-    }
-    if (token === "-") {
-      continue;
-    }
-    if (token.startsWith("-")) {
-      const eqIndex = token.indexOf("=");
-      if (eqIndex > 0) {
-        const value = token.slice(eqIndex + 1);
-        if (value && (isPathLikeToken(value) || exists(path.resolve(cwd, value)))) {
-          return false;
-        }
-      }
-      continue;
-    }
-    if (isPathLikeToken(token)) {
-      return false;
-    }
-    if (exists(path.resolve(cwd, token))) {
-      return false;
-    }
-  }
-  return true;
-}
-
-export type ExecAllowlistEvaluation = {
-  allowlistSatisfied: boolean;
-  allowlistMatches: ExecAllowlistEntry[];
-};
-
-function evaluateSegments(
-  segments: ExecCommandSegment[],
-  params: {
-    allowlist: ExecAllowlistEntry[];
-    safeBins: Set<string>;
-    cwd?: string;
-    skillBins?: Set<string>;
-    autoAllowSkills?: boolean;
-  },
-): { satisfied: boolean; matches: ExecAllowlistEntry[] } {
-  const matches: ExecAllowlistEntry[] = [];
-  const allowSkills = params.autoAllowSkills === true && (params.skillBins?.size ?? 0) > 0;
-
-  const satisfied = segments.every((segment) => {
-    const candidatePath = resolveAllowlistCandidatePath(segment.resolution, params.cwd);
-    const candidateResolution =
-      candidatePath && segment.resolution
-        ? { ...segment.resolution, resolvedPath: candidatePath }
-        : segment.resolution;
-    const match = matchAllowlist(params.allowlist, candidateResolution);
-    if (match) {
-      matches.push(match);
-    }
-    const safe = isSafeBinUsage({
-      argv: segment.argv,
-      resolution: segment.resolution,
-      safeBins: params.safeBins,
-      cwd: params.cwd,
-    });
-    const skillAllow =
-      allowSkills && segment.resolution?.executableName
-        ? params.skillBins?.has(segment.resolution.executableName)
-        : false;
-    return Boolean(match || safe || skillAllow);
-  });
-
-  return { satisfied, matches };
-}
-
-export function evaluateExecAllowlist(params: {
-  analysis: ExecCommandAnalysis;
-  allowlist: ExecAllowlistEntry[];
-  safeBins: Set<string>;
-  cwd?: string;
-  skillBins?: Set<string>;
-  autoAllowSkills?: boolean;
-}): ExecAllowlistEvaluation {
-  const allowlistMatches: ExecAllowlistEntry[] = [];
-  if (!params.analysis.ok || params.analysis.segments.length === 0) {
-    return { allowlistSatisfied: false, allowlistMatches };
-  }
-
-  // If the analysis contains chains, evaluate each chain part separately
-  if (params.analysis.chains) {
-    for (const chainSegments of params.analysis.chains) {
-      const result = evaluateSegments(chainSegments, {
-        allowlist: params.allowlist,
-        safeBins: params.safeBins,
-        cwd: params.cwd,
-        skillBins: params.skillBins,
-        autoAllowSkills: params.autoAllowSkills,
-      });
-      if (!result.satisfied) {
-        return { allowlistSatisfied: false, allowlistMatches: [] };
-      }
-      allowlistMatches.push(...result.matches);
-    }
-    return { allowlistSatisfied: true, allowlistMatches };
-  }
-
-  // No chains, evaluate all segments together
-  const result = evaluateSegments(params.analysis.segments, {
-    allowlist: params.allowlist,
-    safeBins: params.safeBins,
-    cwd: params.cwd,
-    skillBins: params.skillBins,
-    autoAllowSkills: params.autoAllowSkills,
-  });
-  return { allowlistSatisfied: result.satisfied, allowlistMatches: result.matches };
-}
-
 /**
  * Splits a command string by chain operators (&&, ||, ;) while respecting quotes.
  * Returns null when no chain is present or when the chain is malformed.
  */
-function splitCommandChain(command: string): string[] | null {
+export function splitCommandChain(command: string): string[] | null {
   const parts: string[] = [];
   let buf = "";
   let inSingle = false;
@@ -1023,101 +776,66 @@ function splitCommandChain(command: string): string[] | null {
   return parts.length > 0 ? parts : null;
 }
 
-export type ExecAllowlistAnalysis = {
+export function analyzeShellCommand(params: {
-  analysisOk: boolean;
-  allowlistSatisfied: boolean;
-  allowlistMatches: ExecAllowlistEntry[];
-  segments: ExecCommandSegment[];
-};
-
-/**
- * Evaluates allowlist for shell commands (including &&, ||, ;) and returns analysis metadata.
- */
-export function evaluateShellAllowlist(params: {
   command: string;
-  allowlist: ExecAllowlistEntry[];
-  safeBins: Set<string>;
   cwd?: string;
   env?: NodeJS.ProcessEnv;
-  skillBins?: Set<string>;
-  autoAllowSkills?: boolean;
   platform?: string | null;
-}): ExecAllowlistAnalysis {
+}): ExecCommandAnalysis {
-  const chainParts = isWindowsPlatform(params.platform) ? null : splitCommandChain(params.command);
+  if (isWindowsPlatform(params.platform)) {
-  if (!chainParts) {
+    return analyzeWindowsShellCommand(params);
-    const analysis = analyzeShellCommand({
+  }
-      command: params.command,
+  // First try splitting by chain operators (&&, ||, ;)
-      cwd: params.cwd,
+  const chainParts = splitCommandChain(params.command);
-      env: params.env,
+  if (chainParts) {
-      platform: params.platform,
+    const chains: ExecCommandSegment[][] = [];
-    });
+    const allSegments: ExecCommandSegment[] = [];
-    if (!analysis.ok) {
+
-      return {
+    for (const part of chainParts) {
-        analysisOk: false,
+      const pipelineSplit = splitShellPipeline(part);
-        allowlistSatisfied: false,
+      if (!pipelineSplit.ok) {
-        allowlistMatches: [],
+        return { ok: false, reason: pipelineSplit.reason, segments: [] };
-        segments: [],
+      }
-      };
+      const segments = parseSegmentsFromParts(pipelineSplit.segments, params.cwd, params.env);
+      if (!segments) {
+        return { ok: false, reason: "unable to parse shell segment", segments: [] };
+      }
+      chains.push(segments);
+      allSegments.push(...segments);
     }
-    const evaluation = evaluateExecAllowlist({
+
-      analysis,
+    return { ok: true, segments: allSegments, chains };
-      allowlist: params.allowlist,
-      safeBins: params.safeBins,
-      cwd: params.cwd,
-      skillBins: params.skillBins,
-      autoAllowSkills: params.autoAllowSkills,
-    });
-    return {
-      analysisOk: true,
-      allowlistSatisfied: evaluation.allowlistSatisfied,
-      allowlistMatches: evaluation.allowlistMatches,
-      segments: analysis.segments,
-    };
   }
 
-  const allowlistMatches: ExecAllowlistEntry[] = [];
+  // No chain operators, parse as simple pipeline
-  const segments: ExecCommandSegment[] = [];
+  const split = splitShellPipeline(params.command);
-
+  if (!split.ok) {
-  for (const part of chainParts) {
+    return { ok: false, reason: split.reason, segments: [] };
-    const analysis = analyzeShellCommand({
-      command: part,
-      cwd: params.cwd,
-      env: params.env,
-      platform: params.platform,
-    });
-    if (!analysis.ok) {
-      return {
-        analysisOk: false,
-        allowlistSatisfied: false,
-        allowlistMatches: [],
-        segments: [],
-      };
-    }
-
-    segments.push(...analysis.segments);
-    const evaluation = evaluateExecAllowlist({
-      analysis,
-      allowlist: params.allowlist,
-      safeBins: params.safeBins,
-      cwd: params.cwd,
-      skillBins: params.skillBins,
-      autoAllowSkills: params.autoAllowSkills,
-    });
-    allowlistMatches.push(...evaluation.allowlistMatches);
-    if (!evaluation.allowlistSatisfied) {
-      return {
-        analysisOk: true,
-        allowlistSatisfied: false,
-        allowlistMatches,
-        segments,
-      };
-    }
   }
+  const segments = parseSegmentsFromParts(split.segments, params.cwd, params.env);
+  if (!segments) {
+    return { ok: false, reason: "unable to parse shell segment", segments: [] };
+  }
+  return { ok: true, segments };
+}
 
+export function analyzeArgvCommand(params: {
+  argv: string[];
+  cwd?: string;
+  env?: NodeJS.ProcessEnv;
+}): ExecCommandAnalysis {
+  const argv = params.argv.filter((entry) => entry.trim().length > 0);
+  if (argv.length === 0) {
+    return { ok: false, reason: "empty argv", segments: [] };
+  }
   return {
-    analysisOk: true,
+    ok: true,
-    allowlistSatisfied: true,
+    segments: [
-    allowlistMatches,
+      {
-    segments,
+        raw: argv.join(" "),
+        argv,
+        resolution: resolveCommandResolutionFromArgv(argv, params.cwd, params.env),
+      },
+    ],
   };
 }

src/infra/exec-approvals.ts

@@ -5,6 +5,7 @@ import os from "node:os";
 import path from "node:path";
 import { DEFAULT_AGENT_ID } from "../routing/session-key.js";
 export * from "./exec-approvals-analysis.js";
+export * from "./exec-approvals-allowlist.js";
 
 export type ExecHost = "sandbox" | "gateway" | "node";
 export type ExecSecurity = "deny" | "allowlist" | "full";