fix: recheck rebuilt system run argv

pgondhi987 · pgondhi987 · commit faa66b680da2 · 2026-05-19T10:44:23.000Z
diff --git a/src/node-host/invoke-system-run-allowlist.test.ts b/src/node-host/invoke-system-run-allowlist.test.ts
@@ -0,0 +1,108 @@
+import { describe, expect, it } from "vitest";
+import { analyzeShellCommand } from "../infra/exec-approvals-analysis.js";
+import { resolveExecApprovalsFromFile } from "../infra/exec-approvals.js";
+import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js";
+import { resolveSystemRunExecArgv } from "./invoke-system-run-allowlist.js";
+
+function resolveAllowlistApprovals() {
+  return resolveExecApprovalsFromFile({
+    file: {
+      version: 1,
+      defaults: {
+        security: "allowlist",
+        ask: "off",
+        askFallback: "deny",
+      },
+    },
+  });
+}
+
+describe("resolveSystemRunExecArgv", () => {
+  it.runIf(process.platform !== "win32")(
+    "keeps rebuilt shell argv behind a final allowlist check",
+    () => {
+      const env = { PATH: "/usr/bin:/bin" };
+      const analysis = analyzeShellCommand({
+        command: "head -c 16",
+        env,
+        platform: process.platform,
+      });
+      expect(analysis.ok).toBe(true);
+      if (!analysis.ok) {
+        return;
+      }
+
+      const result = resolveSystemRunExecArgv({
+        plannedAllowlistArgv: undefined,
+        argv: ["/bin/sh", "-lc", "head -c 16"],
+        security: "allowlist",
+        approvals: resolveAllowlistApprovals(),
+        safeBins: new Set(),
+        safeBinProfiles: {},
+        trustedSafeBinDirs: new Set(),
+        skillBins: [],
+        autoAllowSkills: false,
+        isWindows: false,
+        policy: {
+          approvedByAsk: false,
+          analysisOk: true,
+          allowlistSatisfied: true,
+        },
+        shellCommand: "head -c 16",
+        segments: analysis.segments,
+        segmentSatisfiedBy: ["safeBins"],
+        cwd: undefined,
+        env,
+      });
+
+      expect(result).toBeNull();
+    },
+  );
+
+  it.runIf(process.platform !== "win32")(
+    "returns rebuilt shell argv when the final allowlist check passes",
+    () => {
+      const env = { PATH: "/usr/bin:/bin" };
+      const analysis = analyzeShellCommand({
+        command: "head -c 16",
+        env,
+        platform: process.platform,
+      });
+      expect(analysis.ok).toBe(true);
+      if (!analysis.ok) {
+        return;
+      }
+      const safeBinPolicy = resolveExecSafeBinRuntimePolicy({
+        global: { safeBins: ["head"] },
+      });
+
+      const result = resolveSystemRunExecArgv({
+        plannedAllowlistArgv: undefined,
+        argv: ["/bin/sh", "-lc", "head -c 16"],
+        security: "allowlist",
+        approvals: resolveAllowlistApprovals(),
+        safeBins: safeBinPolicy.safeBins,
+        safeBinProfiles: safeBinPolicy.safeBinProfiles,
+        trustedSafeBinDirs: safeBinPolicy.trustedSafeBinDirs,
+        skillBins: [],
+        autoAllowSkills: false,
+        isWindows: false,
+        policy: {
+          approvedByAsk: false,
+          analysisOk: true,
+          allowlistSatisfied: true,
+        },
+        shellCommand: "head -c 16",
+        segments: analysis.segments,
+        segmentSatisfiedBy: ["safeBins"],
+        cwd: undefined,
+        env,
+      });
+
+      expect(result).not.toBeNull();
+      expect(result?.[0]).toBe("/bin/sh");
+      expect(result?.[2]).toContain("head");
+      expect(result?.[2]).not.toBe("head -c 16");
+    },
+  );
+});
diff --git a/src/node-host/invoke-system-run-allowlist.ts b/src/node-host/invoke-system-run-allowlist.ts
@@ -123,6 +123,12 @@ export function resolveSystemRunExecArgv(params: {
   plannedAllowlistArgv: string[] | undefined;
   argv: string[];
   security: ExecSecurity;
+  approvals: ReturnType<typeof resolveExecApprovals>;
+  safeBins: ReturnType<typeof resolveExecSafeBinRuntimePolicy>["safeBins"];
+  safeBinProfiles: ReturnType<typeof resolveExecSafeBinRuntimePolicy>["safeBinProfiles"];
+  trustedSafeBinDirs: ReturnType<typeof resolveExecSafeBinRuntimePolicy>["trustedSafeBinDirs"];
+  skillBins: SkillBinTrustEntry[];
+  autoAllowSkills: boolean;
   isWindows: boolean;
   policy: {
     approvedByAsk: boolean;
@@ -177,6 +183,22 @@ export function resolveSystemRunExecArgv(params: {
     if (!rewrittenArgv) {
       return null;
     }
+    const rebuiltAllowlist = evaluateSystemRunAllowlist({
+      shellCommand: rebuilt.command,
+      argv: rewrittenArgv,
+      approvals: params.approvals,
+      security: params.security,
+      safeBins: params.safeBins,
+      safeBinProfiles: params.safeBinProfiles,
+      trustedSafeBinDirs: params.trustedSafeBinDirs,
+      cwd: params.cwd,
+      env: params.env,
+      skillBins: params.skillBins,
+      autoAllowSkills: params.autoAllowSkills,
+    });
+    if (!rebuiltAllowlist.analysisOk || !rebuiltAllowlist.allowlistSatisfied) {
+      return null;
+    }
     execArgv = rewrittenArgv;
   }
   return execArgv;
diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts
@@ -16,7 +16,9 @@ import {
   type ExecAllowlistEntry,
   type ExecAsk,
   type ExecCommandSegment,
+  type ExecSegmentSatisfiedBy,
   type ExecSecurity,
+  type SkillBinTrustEntry,
 } from "../infra/exec-approvals.js";
 import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../infra/exec-host.js";
 import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js";
@@ -111,8 +113,13 @@ type SystemRunPolicyPhase = SystemRunParsePhase & {
   allowlistMatches: ExecAllowlistEntry[];
   analysisOk: boolean;
   allowlistSatisfied: boolean;
+  safeBins: ReturnType<typeof resolveExecSafeBinRuntimePolicy>["safeBins"];
+  safeBinProfiles: ReturnType<typeof resolveExecSafeBinRuntimePolicy>["safeBinProfiles"];
+  trustedSafeBinDirs: ReturnType<typeof resolveExecSafeBinRuntimePolicy>["trustedSafeBinDirs"];
+  skillBins: SkillBinTrustEntry[];
+  autoAllowSkills: boolean;
   segments: ExecCommandSegment[];
-  segmentSatisfiedBy: import("../infra/exec-approvals.js").ExecSegmentSatisfiedBy[];
+  segmentSatisfiedBy: ExecSegmentSatisfiedBy[];
   plannedAllowlistArgv: string[] | undefined;
   isWindows: boolean;
   approvedCwdSnapshot: ApprovedCwdSnapshot | undefined;
@@ -523,6 +530,11 @@ async function evaluateSystemRunPolicyPhase(
     allowlistMatches,
     analysisOk,
     allowlistSatisfied,
+    safeBins,
+    safeBinProfiles,
+    trustedSafeBinDirs,
+    skillBins: bins,
+    autoAllowSkills,
     segments,
     segmentSatisfiedBy,
     plannedAllowlistArgv: plannedAllowlistArgv ?? undefined,
@@ -589,6 +601,12 @@ async function executeSystemRunPhase(
     plannedAllowlistArgv: phase.plannedAllowlistArgv,
     argv: phase.argv,
     security: phase.security,
+    approvals: phase.approvals,
+    safeBins: phase.safeBins,
+    safeBinProfiles: phase.safeBinProfiles,
+    trustedSafeBinDirs: phase.trustedSafeBinDirs,
+    skillBins: phase.skillBins,
+    autoAllowSkills: phase.autoAllowSkills,
     isWindows: phase.isWindows,
     policy: phase.policy,
     shellCommand: phase.shellPayload,
