From a8591f88c8945fa8128903a432288ba0a1f2b13a Mon Sep 17 00:00:00 2001
From: Garry Tan <garrytan@gmail.com>
Date: Wed, 3 Jun 2026 07:21:58 -0700
Subject: [PATCH] harden(sync): close staging-guard TOCTOU + fail hard on
 marker write (#1802 C5)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

checkOwnedStagingDir() now returns the realpath-resolved canonicalPath on a
pass, and cleanupStagingDir() rmSync's that instead of the raw input — closing
the gap where the input is a symlink swapped between the ownership check and the
delete. makeStagingDir() tears down the partial dir and rethrows if the marker
write fails, so a marker-less dir (which the guard would refuse forever) can
never leak.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 bin/gstack-memory-ingest.ts                   | 15 ++++++++--
 lib/staging-guard.ts                          |  9 +++++-
 ...regression-1611-gbrain-sync-resume.test.ts | 29 +++++++++++++++++++
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/bin/gstack-memory-ingest.ts b/bin/gstack-memory-ingest.ts
index 32b3b038e..653d4069a 100644
--- a/bin/gstack-memory-ingest.ts
+++ b/bin/gstack-memory-ingest.ts
@@ -1211,7 +1211,15 @@ function makeStagingDir(): string {
   mkdirSync(dir, { recursive: true });
   // Mint the ownership marker (#1802) so cleanupStagingDir() and decideResume()
   // can prove this dir was created by us before any recursive delete or resume.
-  writeFileSync(join(dir, STAGING_MARKER), `${process.pid}\n${Date.now()}\n`, "utf-8");
+  // #1802 C5: fail hard if the marker can't be written — a marker-less dir would
+  // be refused by the guard forever (leaked, never cleaned). Tear down the
+  // partial dir and rethrow so the caller fails loudly instead of leaking.
+  try {
+    writeFileSync(join(dir, STAGING_MARKER), `${process.pid}\n${Date.now()}\n`, "utf-8");
+  } catch (err) {
+    try { rmSync(dir, { recursive: true, force: true }); } catch { /* best-effort */ }
+    throw err;
+  }
   return dir;
 }
 
@@ -1284,7 +1292,10 @@ function cleanupStagingDir(dir: string): void {
     return;
   }
   try {
-    rmSync(dir, { recursive: true, force: true });
+    // #1802 C5: delete the realpath-resolved dir the guard validated, not the
+    // raw input — closes the TOCTOU gap where `dir` is a symlink swapped between
+    // the check above and this rmSync. canonicalPath is always set when ok.
+    rmSync(verdict.canonicalPath ?? dir, { recursive: true, force: true });
   } catch {
     // best-effort
   }
diff --git a/lib/staging-guard.ts b/lib/staging-guard.ts
index 2487cafac..998ae0f33 100644
--- a/lib/staging-guard.ts
+++ b/lib/staging-guard.ts
@@ -50,6 +50,13 @@ export interface StagingVerdict {
   ok: boolean;
   /** Precise rejection reason, for actionable logging. Undefined when ok. */
   reason?: string;
+  /**
+   * The realpath-resolved directory the verdict actually validated. Present only
+   * when ok. Callers that delete MUST `rmSync` this path, not the raw input —
+   * deleting the canonical path closes the TOCTOU gap where the input is a
+   * symlink swapped between this check and the delete (#1802 C5).
+   */
+  canonicalPath?: string;
 }
 
 /**
@@ -100,7 +107,7 @@ export function checkOwnedStagingDir(dir: string, gstackHome: string): StagingVe
   } catch {
     return { ok: false, reason: `missing "${STAGING_MARKER}" marker — not minted by makeStagingDir` };
   }
-  return { ok: true };
+  return { ok: true, canonicalPath: canon };
 }
 
 /** Boolean convenience wrapper around {@link checkOwnedStagingDir}. */
diff --git a/test/regression-1611-gbrain-sync-resume.test.ts b/test/regression-1611-gbrain-sync-resume.test.ts
index fc1c11344..1f4a06a89 100644
--- a/test/regression-1611-gbrain-sync-resume.test.ts
+++ b/test/regression-1611-gbrain-sync-resume.test.ts
@@ -287,6 +287,14 @@ describe("#1802 checkOwnedStagingDir — ownership matrix", () => {
     expect(checkOwnedStagingDir(mintStaging(), home).ok).toBe(true);
   });
 
+  test("#1802 C5: ok verdict carries the realpath-resolved canonicalPath", () => {
+    const d = mintStaging();
+    const v = checkOwnedStagingDir(d, home);
+    expect(v.ok).toBe(true);
+    // Callers must delete this (not the raw input) to close the symlink TOCTOU.
+    expect(v.canonicalPath).toBe(fs.realpathSync(d));
+  });
+
   test("repo root (direct child, has .git, no marker) → refused", () => {
     const repo = path.join(home, "my-repo");
     fs.mkdirSync(path.join(repo, ".git"), { recursive: true });
@@ -409,6 +417,27 @@ describe("#1802 C3 — import-timeout preserve (static invariant)", () => {
   });
 });
 
+// ── #1802 C5: hardening (static invariant) ─────────────────────────────────
+describe("#1802 C5 — hardening (static invariant)", () => {
+  const ingest = fs.readFileSync(
+    path.join(ROOT, "bin", "gstack-memory-ingest.ts"),
+    "utf-8",
+  );
+
+  test("cleanupStagingDir deletes the canonical path, not the raw input", () => {
+    expect(ingest).toMatch(/rmSync\(verdict\.canonicalPath \?\? dir/);
+  });
+
+  test("makeStagingDir tears down + rethrows if the marker write fails", () => {
+    const at = ingest.indexOf("function makeStagingDir");
+    expect(at).toBeGreaterThan(-1);
+    const slice = ingest.slice(at, at + 800);
+    expect(slice).toMatch(/catch \(err\)/);
+    expect(slice).toMatch(/rmSync\(dir, \{ recursive: true, force: true \}\)/);
+    expect(slice).toMatch(/throw err/);
+  });
+});
+
 // ── #1802 C4: resume must not mark failed files as ingested ─────────────────
 // readNewFailures() maps gbrain's per-file failures (keyed by staging-relative
 // path) back to source paths so the caller can EXCLUDE them from state