refactor(infra): dedupe exec approval allowlist evaluation flow

src/infra/exec-approvals-allowlist.ts

@@ -109,6 +109,29 @@ export type SkillBinTrustEntry = {
   name: string;
   resolvedPath: string;
 };
+type ExecAllowlistContext = {
+  allowlist: ExecAllowlistEntry[];
+  safeBins: Set<string>;
+  safeBinProfiles?: Readonly<Record<string, SafeBinProfile>>;
+  cwd?: string;
+  platform?: string | null;
+  trustedSafeBinDirs?: ReadonlySet<string>;
+  skillBins?: readonly SkillBinTrustEntry[];
+  autoAllowSkills?: boolean;
+};
+
+function pickExecAllowlistContext(params: ExecAllowlistContext): ExecAllowlistContext {
+  return {
+    allowlist: params.allowlist,
+    safeBins: params.safeBins,
+    safeBinProfiles: params.safeBinProfiles,
+    cwd: params.cwd,
+    platform: params.platform,
+    trustedSafeBinDirs: params.trustedSafeBinDirs,
+    skillBins: params.skillBins,
+    autoAllowSkills: params.autoAllowSkills,
+  };
+}
 
 function normalizeSkillBinName(value: string | undefined): string | null {
   const trimmed = value?.trim().toLowerCase();
@@ -173,16 +196,7 @@ function isSkillAutoAllowedSegment(params: {
 
 function evaluateSegments(
   segments: ExecCommandSegment[],
-  params: {
-    allowlist: ExecAllowlistEntry[];
-    safeBins: Set<string>;
-    safeBinProfiles?: Readonly<Record<string, SafeBinProfile>>;
-    cwd?: string;
-    platform?: string | null;
-    trustedSafeBinDirs?: ReadonlySet<string>;
-    skillBins?: readonly SkillBinTrustEntry[];
-    autoAllowSkills?: boolean;
-  },
+  params: ExecAllowlistContext,
 ): {
   satisfied: boolean;
   matches: ExecAllowlistEntry[];
@@ -245,35 +259,21 @@ function resolveAnalysisSegmentGroups(analysis: ExecCommandAnalysis): ExecComman
   return [analysis.segments];
 }
 
-export function evaluateExecAllowlist(params: {
-  analysis: ExecCommandAnalysis;
-  allowlist: ExecAllowlistEntry[];
-  safeBins: Set<string>;
-  safeBinProfiles?: Readonly<Record<string, SafeBinProfile>>;
-  cwd?: string;
-  platform?: string | null;
-  trustedSafeBinDirs?: ReadonlySet<string>;
-  skillBins?: readonly SkillBinTrustEntry[];
-  autoAllowSkills?: boolean;
-}): ExecAllowlistEvaluation {
+export function evaluateExecAllowlist(
+  params: {
+    analysis: ExecCommandAnalysis;
+  } & ExecAllowlistContext,
+): ExecAllowlistEvaluation {
   const allowlistMatches: ExecAllowlistEntry[] = [];
   const segmentSatisfiedBy: ExecSegmentSatisfiedBy[] = [];
   if (!params.analysis.ok || params.analysis.segments.length === 0) {
     return { allowlistSatisfied: false, allowlistMatches, segmentSatisfiedBy };
   }
 
+  const allowlistContext = pickExecAllowlistContext(params);
   const hasChains = Boolean(params.analysis.chains);
   for (const group of resolveAnalysisSegmentGroups(params.analysis)) {
-    const result = evaluateSegments(group, {
-      allowlist: params.allowlist,
-      safeBins: params.safeBins,
-      safeBinProfiles: params.safeBinProfiles,
-      cwd: params.cwd,
-      platform: params.platform,
-      trustedSafeBinDirs: params.trustedSafeBinDirs,
-      skillBins: params.skillBins,
-      autoAllowSkills: params.autoAllowSkills,
-    });
+    const result = evaluateSegments(group, allowlistContext);
     if (!result.satisfied) {
       if (!hasChains) {
         return {
@@ -339,16 +339,12 @@ function collectAllowAlwaysPatterns(params: {
     return;
   }
 
-  if (isDispatchWrapperSegment(params.segment)) {
-    const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(params.segment.argv);
-    if (dispatchUnwrap.kind !== "unwrapped" || dispatchUnwrap.argv.length === 0) {
-      return;
-    }
+  const recurseWithArgv = (argv: string[]): void => {
     collectAllowAlwaysPatterns({
       segment: {
-        raw: dispatchUnwrap.argv.join(" "),
-        argv: dispatchUnwrap.argv,
-        resolution: resolveCommandResolutionFromArgv(dispatchUnwrap.argv, params.cwd, params.env),
+        raw: argv.join(" "),
+        argv,
+        resolution: resolveCommandResolutionFromArgv(argv, params.cwd, params.env),
       },
       cwd: params.cwd,
       env: params.env,
@@ -356,6 +352,14 @@ function collectAllowAlwaysPatterns(params: {
       depth: params.depth + 1,
       out: params.out,
     });
+  };
+
+  if (isDispatchWrapperSegment(params.segment)) {
+    const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(params.segment.argv);
+    if (dispatchUnwrap.kind !== "unwrapped" || dispatchUnwrap.argv.length === 0) {
+      return;
+    }
+    recurseWithArgv(dispatchUnwrap.argv);
     return;
   }
 
@@ -364,22 +368,7 @@ function collectAllowAlwaysPatterns(params: {
     return;
   }
   if (shellMultiplexerUnwrap.kind === "unwrapped") {
-    collectAllowAlwaysPatterns({
-      segment: {
-        raw: shellMultiplexerUnwrap.argv.join(" "),
-        argv: shellMultiplexerUnwrap.argv,
-        resolution: resolveCommandResolutionFromArgv(
-          shellMultiplexerUnwrap.argv,
-          params.cwd,
-          params.env,
-        ),
-      },
-      cwd: params.cwd,
-      env: params.env,
-      platform: params.platform,
-      depth: params.depth + 1,
-      out: params.out,
-    });
+    recurseWithArgv(shellMultiplexerUnwrap.argv);
     return;
   }
 
@@ -444,18 +433,13 @@ export function resolveAllowAlwaysPatterns(params: {
 /**
  * Evaluates allowlist for shell commands (including &&, ||, ;) and returns analysis metadata.
  */
-export function evaluateShellAllowlist(params: {
-  command: string;
-  allowlist: ExecAllowlistEntry[];
-  safeBins: Set<string>;
-  safeBinProfiles?: Readonly<Record<string, SafeBinProfile>>;
-  cwd?: string;
-  env?: NodeJS.ProcessEnv;
-  trustedSafeBinDirs?: ReadonlySet<string>;
-  skillBins?: readonly SkillBinTrustEntry[];
-  autoAllowSkills?: boolean;
-  platform?: string | null;
-}): ExecAllowlistAnalysis {
+export function evaluateShellAllowlist(
+  params: {
+    command: string;
+    env?: NodeJS.ProcessEnv;
+  } & ExecAllowlistContext,
+): ExecAllowlistAnalysis {
+  const allowlistContext = pickExecAllowlistContext(params);
   const analysisFailure = (): ExecAllowlistAnalysis => ({
     analysisOk: false,
     allowlistSatisfied: false,
@@ -481,17 +465,7 @@ export function evaluateShellAllowlist(params: {
     if (!analysis.ok) {
       return analysisFailure();
     }
-    const evaluation = evaluateExecAllowlist({
-      analysis,
-      allowlist: params.allowlist,
-      safeBins: params.safeBins,
-      safeBinProfiles: params.safeBinProfiles,
-      cwd: params.cwd,
-      platform: params.platform,
-      trustedSafeBinDirs: params.trustedSafeBinDirs,
-      skillBins: params.skillBins,
-      autoAllowSkills: params.autoAllowSkills,
-    });
+    const evaluation = evaluateExecAllowlist({ analysis, ...allowlistContext });
     return {
       analysisOk: true,
       allowlistSatisfied: evaluation.allowlistSatisfied,
@@ -517,17 +491,7 @@ export function evaluateShellAllowlist(params: {
     }
 
     segments.push(...analysis.segments);
-    const evaluation = evaluateExecAllowlist({
-      analysis,
-      allowlist: params.allowlist,
-      safeBins: params.safeBins,
-      safeBinProfiles: params.safeBinProfiles,
-      cwd: params.cwd,
-      platform: params.platform,
-      trustedSafeBinDirs: params.trustedSafeBinDirs,
-      skillBins: params.skillBins,
-      autoAllowSkills: params.autoAllowSkills,
-    });
+    const evaluation = evaluateExecAllowlist({ analysis, ...allowlistContext });
     allowlistMatches.push(...evaluation.allowlistMatches);
     segmentSatisfiedBy.push(...evaluation.segmentSatisfiedBy);
     if (!evaluation.allowlistSatisfied) {