FN-8367: enforce bounded engine shellouts

Enforce bounded synchronous shellout use across the engine. - Audit every production synchronous shellout against a call-site allowlist. - Bound data-dependent git diff commands by timeout and output size. - Document the async shellout invariant and align focused command guards. Files changed: AGENTS.md | 2 +- docs/architecture.md | 1 + .../__tests__/engine-no-blocking-shellout.test.ts | 135 +++++++++++++++++++++ .../user-configured-command-no-execsync.test.ts | 5 +- packages/engine/src/merger-git-parse.ts | 16 ++- .../engine/src/merger-workspace-test-commands.ts | 27 ++++- 6 files changed, 181 insertions(+), 5 deletions(-) Fusion-Task-Id: FN-8367 Fusion-Task-Lineage: 976384e6-f283-4464-9f74-f328f2be3430 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
2026-07-19 16:00:13 -07:00
parent e0e395a715
commit ccb7d4e8ff
6 changed files with 181 additions and 5 deletions
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -201,7 +201,7 @@ When you need a Fusion temp artifact, target the known prefix directly and list

 #### Never use `execSync` for user-configured commands

-Run user-configured commands (test/build/workflow scripts) via async `exec` with timeout. `execSync` is only acceptable for short deterministic git plumbing.
+Run user-configured commands (test/build/workflow scripts) via async `exec` with timeout. `execSync` is only acceptable for short deterministic git plumbing. `packages/engine/src/__tests__/engine-no-blocking-shellout.test.ts` enforces the engine-wide call-site allowlist for all synchronous shellout primitives.

 #### Move-Task contract

--- a/docs/architecture.md
+++ b/docs/architecture.md
@@ -647,6 +647,7 @@ See [Memory Plugin Contract](./memory-plugin-contract.md) for the full plan.
 ### Sandbox backend seam (FN-4636)
 - Engine user-configured command runners now route through `packages/engine/src/sandbox/` via a shared `SandboxBackend` abstraction (`resolveSandboxBackend()`), currently implemented only by the transparent `NativeSandboxBackend` passthrough (no behavior change).
 - The seam now covers both exec-shaped commands (`run`) and spawn-shaped verification commands (`runStreaming`), with `packages/engine/src/verification-utils.ts` delegating `runVerificationCommand`/`execWithProcessGroup` through `runStreaming`.
+- **Async shellout invariant (FN-8367):** executor, scheduler, merger, self-healing, and dashboard activity share the engine's Node event loop, so user-configured or potentially long-running work must use bounded async execution. Production `execSync`, `spawnSync`, and `execFileSync` are restricted to the audited short git-plumbing call sites in `packages/engine/src/__tests__/engine-no-blocking-shellout.test.ts`; its call-site-level allowlist is the enforced source of truth. Data-dependent `git diff` is only permitted there when both `timeout` and `maxBuffer` bound it; otherwise it must use async `exec`/`execFile`.
 - Follow-up chain: FN-4637 (bubblewrap), FN-4638 (sandbox-exec), FN-4639 (settings selection), FN-4640 (run-audit telemetry), FN-4641 (action-gate), FN-4642 (container backends).
 - FN-4641 adds dedicated `sandbox_provisioning` approval-gate plumbing for first-time backend bootstrap. Backends call `requireSandboxProvisioningApproval()` (`packages/engine/src/sandbox/provisioning-gate.ts`) from `prepare()` when prerequisites are missing, and policy is resolved via `resolveSandboxProvisioningPolicy()` (`packages/core/src/sandbox-provisioning-policy.ts`). Initial callers land in FN-4637/FN-4638/FN-4642.
 - FN-4642 adds an experimental `ContainerSandboxBackend` (Podman-first, Docker-compatible) plus `buildContainerArgv()` for rootless container runs. It is opt-in only via explicit `resolveSandboxBackend({ backendId: "podman" | "docker" })` and is not wired through settings yet; known prototype limits are no SELinux `:Z` relabel on bind mounts, no filesystem policy beyond cwd bind-mounting, and a fixed default image (`docker.io/library/alpine:3.20`) with override via `FUSION_SANDBOX_CONTAINER_IMAGE`.
--- a/packages/engine/src/tests/engine-no-blocking-shellout.test.ts
+++ b/packages/engine/src/tests/engine-no-blocking-shellout.test.ts
@@ -0,0 +1,135 @@
+import { readdirSync, readFileSync } from "node:fs";
+import { join, relative } from "node:path";
+import { describe, expect, it } from "vitest";
+import { createSourceFile, forEachChild, isCallExpression, isIdentifier, ScriptTarget } from "typescript";
+
+/*
+FNXC:EngineAsyncInvariant 2026-07-29-00:00:
+The engine's executor, scheduler, merger, self-healing, and dashboard activity
+share one Node event loop. User-configured and potentially long-running work
+must therefore stay async and bounded. This guard covers execSync, spawnSync,
+and execFileSync across production source.
+
+The allowlist is call-site-level (path, line, and signature), not file-level,
+and is the single enforced source of truth for sanctioned short git plumbing.
+Data-dependent git diff calls are present only after proving timeout and
+maxBuffer bounds in their production modules; new or unbounded sync shellouts
+must migrate to bounded async execution instead.
+*/
+
+type SyncPrimitive = "execSync" | "spawnSync" | "execFileSync";
+type ShelloutSite = {
+  file: string;
+  line: number;
+  primitive: SyncPrimitive;
+  signature: string;
+};
+type AllowlistEntry = ShelloutSite & { reason: string };
+
+const SHORT_GIT_PLUMBING = "short deterministic git plumbing";
+const BOUNDED_GIT_DIFF = "bounded data-dependent git diff plumbing";
+
+const allowlist: AllowlistEntry[] = [
+  { file: "src/review-checkout.ts", line: 35, primitive: "execFileSync", signature: "const topLevel = execFileSync(\"git\", [\"rev-parse\", \"--show-toplevel\"], {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/worktree-prune.ts", line: 69, primitive: "execSync", signature: "execSync(\"git worktree prune\", {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger-git-parse.ts", line: 102, primitive: "execFileSync", signature: "const output = execFileSync(", reason: BOUNDED_GIT_DIFF },
+  { file: "src/already-merged-detector.ts", line: 204, primitive: "execSync", signature: "branchTip = execSync(`git rev-parse --verify ${shellQuote(branchName)}`, {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/already-merged-detector.ts", line: 223, primitive: "execSync", signature: "execSync(`git merge-base --is-ancestor ${shellQuote(branchTip)} ${shellQuote(baseBranch)}`, {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/already-merged-detector.ts", line: 270, primitive: "execSync", signature: "branchTip = execSync(`git rev-parse --verify ${shellQuote(branchName)}`, {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/already-merged-detector.ts", line: 345, primitive: "execSync", signature: "execSync(`git rev-parse --verify ${shellQuote(treeBranchName)}`, {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/self-healing.ts", line: 4028, primitive: "execSync", signature: "const tipSha = String(execSync(`git rev-parse --verify ${shellQuote(branch)}`, {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/self-healing.ts", line: 4034, primitive: "execSync", signature: "const uniqueCommitCount = Number.parseInt(String(execSync(`git rev-list --count ${shellQuote(branch)} --not ${shellQuote(\"main\")}`, {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/self-healing.ts", line: 4071, primitive: "execSync", signature: "const branchesRaw = String(execSync(\"git branch --list 'fusion/*'\", {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/self-healing.ts", line: 12470, primitive: "execSync", signature: "execSync(`git branch -d ${shellQuote(branch)}`, {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger-workspace-test-commands.ts", line: 204, primitive: "execSync", signature: "changedFilesOutput = execSync(", reason: BOUNDED_GIT_DIFF },
+  { file: "src/merger-workspace-test-commands.ts", line: 301, primitive: "execSync", signature: "changedFilesOutput = execSync(", reason: BOUNDED_GIT_DIFF },
+  { file: "src/integration-branch.ts", line: 71, primitive: "execSync", signature: "const stdout = execSync(\"git symbolic-ref --short refs/remotes/origin/HEAD\", {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/integration-branch.ts", line: 107, primitive: "execSync", signature: "const stdout = execSync(\"git remote\", {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 734, primitive: "execSync", signature: "const output = execSync(command, options);", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 781, primitive: "execSync", signature: "treeSha = execSync(\"git rev-parse HEAD^{tree}\", { cwd: rootDir, stdio: \"pipe\" })", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 1388, primitive: "execSync", signature: "execSync(\"git reset --merge\", { cwd: rootDir, stdio: \"pipe\" });", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 1600, primitive: "execSync", signature: "beforeRaw = execSync(\"git status -z --porcelain\", { cwd: rootDir, stdio: [\"ignore\", \"pipe\", \"ignore\"] }).toString(\"utf-8\");", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 1612, primitive: "execSync", signature: "afterRaw = execSync(\"git status -z --porcelain\", { cwd: rootDir, stdio: [\"ignore\", \"pipe\", \"ignore\"] }).toString(\"utf-8\");", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 5754, primitive: "execSync", signature: "execSync(\"git rev-parse --verify REBASE_HEAD\", {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 7618, primitive: "execSync", signature: "execSync(`git rev-parse --verify \"${branch}\"`, {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 8573, primitive: "execSync", signature: "execSync(\"git reset --merge\", { cwd: rootDir, stdio: \"pipe\" });", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 8586, primitive: "execSync", signature: "execSync(\"git reset --merge\", { cwd: rootDir, stdio: \"pipe\" });", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 8598, primitive: "execSync", signature: "execSync(\"git reset --merge\", { cwd: rootDir, stdio: \"pipe\" });", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 8936, primitive: "execSync", signature: "execSync(\"git reset --merge\", { cwd: rootDir, stdio: \"pipe\" });", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 8956, primitive: "execSync", signature: "execSync(\"git reset --merge\", { cwd: rootDir, stdio: \"pipe\" });", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 8965, primitive: "execSync", signature: "execSync(\"git reset --merge\", { cwd: rootDir, stdio: \"pipe\" });", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 9055, primitive: "execSync", signature: "execSync(\"git reset --merge\", { cwd: rootDir, stdio: \"pipe\" });", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 9630, primitive: "execSync", signature: "const postPushSha = execSync(\"git rev-parse HEAD\", {", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 10189, primitive: "execSync", signature: "const squashIsEmpty = execSync(", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 10223, primitive: "execSync", signature: "const squashIsEmpty = execSync(", reason: SHORT_GIT_PLUMBING },
+  { file: "src/merger.ts", line: 10410, primitive: "execSync", signature: "execSync(\"git reset --merge\", { cwd: rootDir, stdio: \"pipe\" });", reason: SHORT_GIT_PLUMBING },
+  { file: "src/executor.ts", line: 15748, primitive: "execSync", signature: "execSync(`git merge-base --is-ancestor ${task.baseCommitSha} HEAD`, {", reason: SHORT_GIT_PLUMBING },
+];
+
+function scanSource(file: string, source: string): ShelloutSite[] {
+  // The TypeScript parser excludes comments and quoted literals from call
+  // expressions, avoiding false positives from documentation or examples.
+  const sourceFile = createSourceFile(file, source, ScriptTarget.Latest, false);
+  const sites: ShelloutSite[] = [];
+  const visit = (node: Parameters<typeof forEachChild>[0]): void => {
+    if (isCallExpression(node) && isIdentifier(node.expression)) {
+      const primitive = node.expression.text;
+      if (primitive === "execSync" || primitive === "spawnSync" || primitive === "execFileSync") {
+        const offset = node.expression.getStart(sourceFile);
+        const { line } = sourceFile.getLineAndCharacterOfPosition(offset);
+        sites.push({
+          file,
+          line: line + 1,
+          primitive,
+          signature: source.split("\n")[line].trim(),
+        });
+      }
+    }
+    forEachChild(node, visit);
+  };
+  visit(sourceFile);
+  return sites;
+}
+
+function listProductionSource(dir: string): string[] {
+  return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
+    const path = join(dir, entry.name);
+    if (entry.isDirectory()) return entry.name === "__tests__" ? [] : listProductionSource(path);
+    return entry.isFile() && entry.name.endsWith(".ts") && !entry.name.endsWith(".test.ts") && !entry.name.endsWith(".spec.ts") ? [path] : [];
+  });
+}
+
+function scanEngineSource(): ShelloutSite[] {
+  const root = join(process.cwd(), "src");
+  return listProductionSource(root).flatMap((path) => scanSource(relative(process.cwd(), path), readFileSync(path, "utf-8")));
+}
+
+function classifySites(sites: ShelloutSite[]): { unmatched: ShelloutSite[]; stale: AllowlistEntry[] } {
+  const remaining = new Set(allowlist.map((entry) => `${entry.file}:${entry.line}:${entry.primitive}:${entry.signature}`));
+  const unmatched = sites.filter((site) => {
+    const key = `${site.file}:${site.line}:${site.primitive}:${site.signature}`;
+    if (!remaining.has(key)) return true;
+    remaining.delete(key);
+    return false;
+  });
+  return { unmatched, stale: allowlist.filter((entry) => remaining.has(`${entry.file}:${entry.line}:${entry.primitive}:${entry.signature}`)) };
+}
+
+describe("engine blocking-shellout static guard", () => {
+  it("confines every production synchronous shellout to an audited call-site allowlist", () => {
+    const { unmatched, stale } = classifySites(scanEngineSource());
+    expect(unmatched).toEqual([]);
+    expect(stale).toEqual([]);
+  });
+
+  it("flags a synchronous call in a non-allowlisted file", () => {
+    const { unmatched } = classifySites(scanSource("src/fake-runner.ts", 'const child = execSync("git status");'));
+    expect(unmatched).toHaveLength(1);
+  });
+
+  it("flags an extra synchronous call in an allowlisted file", () => {
+    const source = readFileSync(join(process.cwd(), "src", "worktree-prune.ts"), "utf-8") + '\nconst child = execSync("git status");\n';
+    const { unmatched } = classifySites(scanSource("src/worktree-prune.ts", source));
+    expect(unmatched).toHaveLength(1);
+  });
+});
--- a/packages/engine/src/tests/user-configured-command-no-execsync.test.ts
+++ b/packages/engine/src/tests/user-configured-command-no-execsync.test.ts
@@ -19,7 +19,10 @@ import { describe, expect, it } from "vitest";
 * - packages/engine/src/sandbox/bubblewrap-backend.ts :: BubblewrapBackend.runBwrapSpawn — concrete bubblewrap spawn path uses setTimeout(options.timeoutMs) and options.maxBuffer.
 * - packages/engine/src/sandbox/sandbox-exec-backend.ts :: SandboxExecBackend.run — macOS isolating backend uses async exec with timeout, maxBuffer, and signal.
 *
- * Explicit exclusions: git-only execSync in merger.ts, self-healing.ts, already-merged-detector.ts, integration-branch.ts, worktree-prune.ts, and executor.ts git merge-base ancestry checks. The guard slices only registry function bodies instead of asserting over whole files.
+ * Explicit exclusions: audited short git plumbing is enforced call-site-by-call-site
+ * by engine-no-blocking-shellout.test.ts (including review-checkout.ts and bounded
+ * data-dependent git diffs). This focused registry slices only user-command
+ * function bodies instead of asserting over whole files.
 */

 type GuardEntry = {
--- a/packages/engine/src/merger-git-parse.ts
+++ b/packages/engine/src/merger-git-parse.ts
@@ -7,6 +7,9 @@
 */
 import { execFileSync } from "node:child_process";

+const BOUNDED_GIT_DIFF_TIMEOUT_MS = 5_000;
+const BOUNDED_GIT_DIFF_MAX_BUFFER = 10 * 1024 * 1024;
+

 export function parseFailingFilesFromOutput(output: string): string[] {
  const paths = new Set<string>();
@@ -86,6 +89,11 @@ export function quoteArg(value: string): string {
 * quoting on Windows cmd.exe, and parse NUL-delimited paths so whitespace/
 * newlines in filenames are preserved. Empty array on git errors (unknown).
 *
+ * FNXC:EngineAsyncInvariant 2026-07-29-00:00:
+ * This data-dependent git diff remains short plumbing only with an explicit
+ * wall-clock timeout and bounded output. Do not remove either bound or add an
+ * unbounded synchronous shellout on the engine's shared event loop.
+ *
 * @internal Exported for testing only.
 */
 export function getBranchChangedFiles(rootDir: string, baseBranch: string, branch: string): string[] {
@@ -94,7 +102,13 @@ export function getBranchChangedFiles(rootDir: string, baseBranch: string, branc
    const output = execFileSync(
      "git",
      ["diff", "--name-only", "-z", `${baseBranch}...${headRef}`],
-      { cwd: rootDir, encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"] },
+      {
+        cwd: rootDir,
+        encoding: "utf-8",
+        stdio: ["ignore", "pipe", "pipe"],
+        timeout: BOUNDED_GIT_DIFF_TIMEOUT_MS,
+        maxBuffer: BOUNDED_GIT_DIFF_MAX_BUFFER,
+      },
    );
    return String(output).split("\0").map((f) => f.trim()).filter(Boolean);
  } catch {
--- a/packages/engine/src/merger-workspace-test-commands.ts
+++ b/packages/engine/src/merger-workspace-test-commands.ts
@@ -8,6 +8,9 @@ import { basename, dirname, join } from "node:path";
 import { execSync } from "node:child_process";
 import { mergerLog } from "./logger.js";

+const BOUNDED_GIT_DIFF_TIMEOUT_MS = 5_000;
+const BOUNDED_GIT_DIFF_MAX_BUFFER = 10 * 1024 * 1024;
+
 /** Shell-safe single-argument quoting for command composition. */
 function quoteArg(value: string): string {
  return `'${value.replace(/'/g, "'\\''")}'`;
@@ -173,6 +176,10 @@ export function packageNamesForFiles(rootDir: string, files: string[]): string[]
 * Returns null when scoping cannot be determined (missing git context, no
 * workspace file, root-only changes, etc.) — callers fall back to `pnpm test`.
 *
+ * FNXC:EngineAsyncInvariant 2026-07-29-00:00:
+ * The branch diff is data-dependent, so its synchronous git-plumbing call is
+ * allowed only with an explicit wall-clock timeout and bounded captured output.
+ *
 * @internal Exported for testing only.
 */
 export function deriveScopedPnpmTestCommand(rootDir: string, baseBranch: string, branch: string): string | null {
@@ -196,7 +203,13 @@ export function deriveScopedPnpmTestCommand(rootDir: string, baseBranch: string,
  try {
    changedFilesOutput = execSync(
      `git diff --name-only ${quoteArg(baseBranch)}...${quoteArg(branch)}`,
-      { cwd: rootDir, stdio: "pipe", encoding: "utf-8" },
+      {
+        cwd: rootDir,
+        stdio: "pipe",
+        encoding: "utf-8",
+        timeout: BOUNDED_GIT_DIFF_TIMEOUT_MS,
+        maxBuffer: BOUNDED_GIT_DIFF_MAX_BUFFER,
+      },
    ).toString();
  } catch {
    return null;
@@ -259,6 +272,10 @@ const TEST_FILE_RE = /\.(test|spec)\.(ts|tsx|js|jsx)$/;
 * files and test paths come from `git diff`, so every shell argument is quoted
 * via `quoteArg`.
 *
+ * FNXC:EngineAsyncInvariant 2026-07-29-00:00:
+ * The branch diff is data-dependent, so its synchronous git-plumbing call is
+ * allowed only with an explicit wall-clock timeout and bounded captured output.
+ *
 * @internal Exported for testing only.
 */
 export function deriveFileScopedPnpmTestCommand(
@@ -283,7 +300,13 @@ export function deriveFileScopedPnpmTestCommand(
  try {
    changedFilesOutput = execSync(
      `git diff --name-only ${quoteArg(baseBranch)}...${quoteArg(branch)}`,
-      { cwd: rootDir, stdio: "pipe", encoding: "utf-8" },
+      {
+        cwd: rootDir,
+        stdio: "pipe",
+        encoding: "utf-8",
+        timeout: BOUNDED_GIT_DIFF_TIMEOUT_MS,
+        maxBuffer: BOUNDED_GIT_DIFF_MAX_BUFFER,
+      },
    ).toString();
  } catch {
    return null;