From f328a5c8890b866764c829fce9ce405996ebd942 Mon Sep 17 00:00:00 2001
From: Max Flanagan <mf.qozle@gmail.com>
Date: Sat, 4 Apr 2026 23:21:28 -0400
Subject: [PATCH] fix: prevent hang in restoreConfigFromBase on repos with
 .gitmodules (#1166)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When a PR head contains `.gitmodules`, git's default
`fetch.recurseSubmodules=on-demand` config causes `git fetch` to attempt
submodule object fetches. In CI (no credentials), this blocks indefinitely
waiting for auth — producing ~4-hour hangs reported in #1088.

Two changes, both defence-in-depth:

1. Delete SENSITIVE_PATHS *before* fetching. The attacker-controlled
   `.gitmodules` is absent during the network operation, so git never
   sees a submodule config to follow regardless of git settings.

2. Pass `--no-recurse-submodules` to the fetch. Suppresses submodule
   fetching explicitly, independent of any git config on the runner.

The original order (fetch-then-delete) was a brief window where
`.gitmodules` from the PR head could influence the fetch. Reordering
also tightens the security property: if `git checkout` below fails, the
attacker-controlled file is already gone rather than present during fetch.

Fixes #1088.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 src/github/operations/restore-config.ts | 30 ++++++++++++++++---------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/src/github/operations/restore-config.ts b/src/github/operations/restore-config.ts
index a0c06cc..0806787 100644
--- a/src/github/operations/restore-config.ts
+++ b/src/github/operations/restore-config.ts
@@ -44,20 +44,30 @@ export function restoreConfigFromBase(baseBranch: string): void {
     `Restoring ${SENSITIVE_PATHS.join(", ")} from origin/${baseBranch} (PR head is untrusted)`,
   );
 
-  // Fetch base first — if this fails we haven't touched the workspace and the
-  // caller sees a clean error.
-  execFileSync("git", ["fetch", "origin", baseBranch, "--depth=1"], {
-    stdio: "inherit",
-    env: process.env,
-  });
-
-  // Delete PR-controlled versions. If the restore below fails for a given path,
-  // that path stays deleted — the safe fallback (no attacker-controlled config).
-  // A bare `git checkout` alone wouldn't remove files the PR added, so nuke first.
+  // Delete PR-controlled versions BEFORE fetching so the attacker-controlled
+  // .gitmodules is absent during the network operation. If git reads .gitmodules
+  // during fetch (fetch.recurseSubmodules=on-demand, the git default), it will
+  // attempt to fetch submodule objects and block on credential prompts in CI —
+  // causing an indefinite hang. Deleting first closes that window.
+  //
+  // If the restore below fails for a given path, that path stays deleted —
+  // the safe fallback (no attacker-controlled config). A bare `git checkout`
+  // alone wouldn't remove files the PR added, so nuke first.
   for (const p of SENSITIVE_PATHS) {
     rmSync(p, { recursive: true, force: true });
   }
 
+  // --no-recurse-submodules: explicitly suppress submodule fetching regardless of
+  // fetch.recurseSubmodules config. Defense-in-depth alongside the delete above.
+  execFileSync(
+    "git",
+    ["fetch", "origin", baseBranch, "--depth=1", "--no-recurse-submodules"],
+    {
+      stdio: "inherit",
+      env: process.env,
+    },
+  );
+
   for (const p of SENSITIVE_PATHS) {
     try {
       execFileSync("git", ["checkout", `origin/${baseBranch}`, "--", p], {