Merge f8fcf45867 into c43c850cae

bin/gstack-gbrain-sync.ts

@@ -41,6 +41,7 @@ import { ensureSourceRegistered, sourcePageCount, parseSourcesList } from "../li
 import { detectAutopilot, decideSourceRemove, decideCodeSync } from "../lib/gbrain-guards";
 import { localEngineStatus, type LocalEngineStatus } from "../lib/gbrain-local-status";
 import { buildGbrainEnv, spawnGbrain, execGbrainJson, NEEDS_SHELL_ON_WINDOWS } from "../lib/gbrain-exec";
+import { checkOwnedStagingDir } from "../lib/staging-guard";
 
 // ── Types ──────────────────────────────────────────────────────────────────
 
@@ -160,7 +161,7 @@ export function readGbrainCheckpoint(): GbrainCheckpoint | null {
 export type ResumeVerdict =
   | { kind: "no-checkpoint" }
   | { kind: "resume"; stagingDir: string; processedIndex: number; totalFiles: number }
-  | { kind: "stale-staging-missing"; stagingDir: string };
+  | { kind: "stale-staging-missing"; stagingDir: string; reason?: string };
 
 /**
  * Decide whether the next memory-ingest run should resume from gbrain's
@@ -169,20 +170,20 @@ export type ResumeVerdict =
  *   - checkpoint + staging ok    → resume (gbrain picks up at processedIndex+1)
  *   - checkpoint + staging gone  → warn, fall through to fresh restage
  */
-export function decideResume(): ResumeVerdict {
+export function decideResume(gstackHome: string = GSTACK_HOME): ResumeVerdict {
   const cp = readGbrainCheckpoint();
   if (!cp || !cp.dir) return { kind: "no-checkpoint" };
   const stagingDir = cp.dir;
-  if (!existsSync(stagingDir)) {
+  // #1802: only resume into a path we can PROVE is a gstack-minted staging dir.
-    return { kind: "stale-staging-missing", stagingDir };
+  // A poisoned checkpoint (dir = repo root, written when an autopilot import was
-  }
+  // SIGTERM'd while CWD was the repo) would otherwise be adopted as the staging
-  // Treat "non-empty" as the safe-to-resume signal. statSync on a missing
+  // dir and later recursively deleted by cleanupStagingDir(). Fail-closed: any
-  // file throws; we already handled missing above so this is dir-level shape.
+  // unprovable path restages from scratch (cost: one re-stage; never data loss).
-  try {
+  // Pure decision: return the verdict (with reason) and let the caller log,
-    const st = statSync(stagingDir);
+  // so we don't double-log the same event from here and the call site.
-    if (!st.isDirectory()) return { kind: "stale-staging-missing", stagingDir };
+  const verdict = checkOwnedStagingDir(stagingDir, gstackHome);
-  } catch {
+  if (!verdict.ok) {
-    return { kind: "stale-staging-missing", stagingDir };
+    return { kind: "stale-staging-missing", stagingDir, reason: verdict.reason };
   }
   return {
     kind: "resume",
@@ -953,8 +954,15 @@ function runMemoryIngest(args: CliArgs): StageResult {
     );
     childEnv.GSTACK_INGEST_RESUME_DIR = resume.stagingDir;
   } else if (resume.kind === "stale-staging-missing") {
+    // The reason distinguishes "actually gone" (disk cleanup / reboot) from
+    // "refused as unowned" (#1802 poison: the path may still exist on disk).
+    // Logging "gone" for a refused poison path misdirects incident diagnosis.
+    const why = resume.reason
+      ? `staging dir not usable: ${resume.reason}`
+      : `staging dir ${resume.stagingDir} gone`;
     console.error(
-      `[sync:memory] previous checkpoint stale (staging dir ${resume.stagingDir} gone), restaging from scratch`,
+      `[sync:memory] previous checkpoint stale (${why}), restaging from scratch. ` +
+        `Remove ~/.gbrain/import-checkpoint.json to silence.`,
     );
   }
 

bin/gstack-memory-ingest.ts

@@ -65,6 +65,7 @@ import {
   withErrorContext,
 } from "../lib/gstack-memory-helpers";
 import { execGbrainText, spawnGbrainAsync } from "../lib/gbrain-exec";
+import { checkOwnedStagingDir, STAGING_MARKER } from "../lib/staging-guard";
 
 // ── Types ──────────────────────────────────────────────────────────────────
 
@@ -1198,6 +1199,9 @@ function preparePages(
 function makeStagingDir(): string {
   const dir = join(GSTACK_HOME, `.staging-ingest-${process.pid}-${Date.now()}`);
   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");
   return dir;
 }
 
@@ -1259,6 +1263,16 @@ function isRemoteHttpMcpMode(): boolean {
  * cleanup failure.
  */
 function cleanupStagingDir(dir: string): void {
+  // #1802 deletion chokepoint: never recurse-delete a path we cannot PROVE we
+  // own. A poisoned resume could otherwise route the repo root here.
+  const verdict = checkOwnedStagingDir(dir, GSTACK_HOME);
+  if (!verdict.ok) {
+    console.error(
+      `[gbrain] staging cleanup REFUSED: "${dir}" is not an owned staging dir ` +
+        `(${verdict.reason}). Skipping rm -rf to prevent data loss (#1802).`,
+    );
+    return;
+  }
   try {
     rmSync(dir, { recursive: true, force: true });
   } catch {
@@ -1515,10 +1529,20 @@ async function ingestPass(args: CliArgs): Promise<BulkResult> {
   // tells it where to resume.
   const remoteHttpMode = isRemoteHttpMcpMode();
   const resumeDir = process.env.GSTACK_INGEST_RESUME_DIR;
+  // #1802 second entry point: this binary is runnable directly, so it must not
+  // trust GSTACK_INGEST_RESUME_DIR just because it exists — a stale/poisoned env
+  // could make us `gbrain import` (and later clean up) an arbitrary directory.
+  // Prove ownership here too, independently of the orchestrator's decideResume.
   const resuming = !remoteHttpMode
     && typeof resumeDir === "string"
     && resumeDir.length > 0
-    && existsSync(resumeDir);
+    && existsSync(resumeDir)
+    && checkOwnedStagingDir(resumeDir, GSTACK_HOME).ok;
+  if (!remoteHttpMode && resumeDir && resumeDir.length > 0 && !resuming) {
+    console.error(
+      `[memory-ingest] ignoring GSTACK_INGEST_RESUME_DIR="${resumeDir}" — not a proven staging dir (#1802); staging fresh.`,
+    );
+  }
   const stagingDir = resuming
     ? resumeDir!
     : remoteHttpMode

lib/staging-guard.ts

@@ -0,0 +1,109 @@
+/**
+ * staging-guard — fail-closed ownership proof for gstack ingest staging dirs.
+ *
+ * Fixes #1802. The /sync-gbrain memory stage stages prepared pages to a
+ * throwaway dir under ~/.gstack and `rm -rf`s it when done. The resume path
+ * (#1611) reused gbrain's `import-checkpoint.json` `dir` field as that staging
+ * dir WITHOUT proving it was one. A poisoned checkpoint — `dir` = the repo
+ * root, written when an autopilot `gbrain import` was SIGTERM'd while CWD was
+ * the repo — was then adopted as the staging dir and recursively deleted,
+ * destroying the user's working tree.
+ *
+ * Root cause is a TRUST failure, not path math: code deleted a path it never
+ * proved it owned. This module is the single definition of "a path gstack is
+ * allowed to recurse-delete or resume into", shared by the resume gate
+ * (decideResume) and the deletion chokepoint (cleanupStagingDir).
+ *
+ * Ownership requires ALL of the following (fail-closed — any failure ⇒ refuse):
+ *   1. Resolvable    — realpathSync succeeds (resolves symlinks and `..` to a
+ *                      real location before any structural reasoning).
+ *   2. Structural    — canonical path is a DIRECT child of $GSTACK_HOME named
+ *                      `.staging-ingest-*` (makeStagingDir's contract).
+ *   3. Not a repo    — no `.git` entry inside. A screaming last-line tripwire:
+ *                      even a logic error elsewhere can never recurse-delete a
+ *                      git working tree.
+ *   4. Minted by us  — a `.gstack-staging` marker file (written by
+ *                      makeStagingDir) is present. Turns "looks like ours"
+ *                      into "was created by us this lineage".
+ *
+ * Design note (steelman, 2026-06-02): a 4-model review panel split 3-1 on the
+ * marker. The dissent argued the structural check alone is sufficient and the
+ * marker adds a missing-token failure mode. Adopted anyway because that failure
+ * mode is fail-SAFE: a missing marker only forces an unnecessary re-stage
+ * (seconds), never a wrong deletion. The asymmetry — the marker can cost work
+ * but never data — settles it. The structural check still runs first and cheap.
+ *
+ * The deeper, "inevitable" fix lives upstream in gbrain: checkpoint.dir should
+ * always be a gbrain-minted staging dir, never CWD. This guard is the
+ * mitigation at gstack's own rm -rf boundary; see the companion gbrain issue.
+ */
+import { realpathSync, existsSync, statSync, lstatSync } from "fs";
+import { join, dirname, basename } from "path";
+
+/** Basename prefix every makeStagingDir() directory carries. */
+export const STAGING_PREFIX = ".staging-ingest-";
+
+/** Marker file minted inside each staging dir at creation. */
+export const STAGING_MARKER = ".gstack-staging";
+
+export interface StagingVerdict {
+  ok: boolean;
+  /** Precise rejection reason, for actionable logging. Undefined when ok. */
+  reason?: string;
+}
+
+/**
+ * Prove (fail-closed) that `dir` is a gstack-owned ingest staging directory
+ * that is safe to recurse-delete or resume into. Returns a structured verdict
+ * so callers can log exactly why a path was rejected.
+ *
+ * @param dir         Candidate path (e.g. gbrain checkpoint.dir, or the active staging dir).
+ * @param gstackHome  Resolved $GSTACK_HOME (injected for testability).
+ */
+export function checkOwnedStagingDir(dir: string, gstackHome: string): StagingVerdict {
+  if (!dir || typeof dir !== "string") {
+    return { ok: false, reason: "empty or non-string path" };
+  }
+  let canon: string;
+  let home: string;
+  try {
+    canon = realpathSync(dir);
+    home = realpathSync(gstackHome);
+  } catch {
+    // Missing path or broken symlink ⇒ cannot prove ownership ⇒ refuse.
+    return { ok: false, reason: "unresolvable path (missing dir or broken symlink)" };
+  }
+  // The target itself must be a directory (not a file/socket/etc named like one).
+  try {
+    if (!statSync(canon).isDirectory()) {
+      return { ok: false, reason: "not a directory" };
+    }
+  } catch {
+    return { ok: false, reason: "unstattable target" };
+  }
+  if (dirname(canon) !== home) {
+    return { ok: false, reason: `not a direct child of GSTACK_HOME (${home})` };
+  }
+  if (!basename(canon).startsWith(STAGING_PREFIX)) {
+    return { ok: false, reason: `basename does not start with "${STAGING_PREFIX}"` };
+  }
+  if (existsSync(join(canon, ".git"))) {
+    // Tripwire: never recurse-delete anything that looks like a git work tree.
+    return { ok: false, reason: "path contains .git — refusing to touch a git working tree" };
+  }
+  // Marker must be a REGULAR FILE we minted — not a directory or symlink that
+  // merely shares the name (lstat, not stat, so a symlink can't impersonate it).
+  try {
+    if (!lstatSync(join(canon, STAGING_MARKER)).isFile()) {
+      return { ok: false, reason: `"${STAGING_MARKER}" exists but is not a regular file` };
+    }
+  } catch {
+    return { ok: false, reason: `missing "${STAGING_MARKER}" marker — not minted by makeStagingDir` };
+  }
+  return { ok: true };
+}
+
+/** Boolean convenience wrapper around {@link checkOwnedStagingDir}. */
+export function isOwnedStagingDir(dir: string, gstackHome: string): boolean {
+  return checkOwnedStagingDir(dir, gstackHome).ok;
+}

test/regression-1611-gbrain-sync-resume.test.ts

@@ -35,6 +35,7 @@ import {
   readGbrainCheckpoint,
   decideResume,
 } from "../bin/gstack-gbrain-sync";
+import { checkOwnedStagingDir, STAGING_MARKER } from "../lib/staging-guard";
 
 const ROOT = path.resolve(import.meta.dir, "..");
 const DEFAULT_MS = 35 * 60 * 1000;
@@ -132,9 +133,11 @@ describe("#1611 decideResume — checkpoint + staging detection", () => {
     expect(decideResume().kind).toBe("no-checkpoint");
   });
 
-  test("checkpoint + staging dir exists → resume verdict", () => {
+  test("checkpoint + minted staging dir exists → resume verdict", () => {
     fs.mkdirSync(stagingDir, { recursive: true });
     fs.writeFileSync(stagingDir + "/page1.md", "content", "utf-8");
+    // #1802: a real staging dir carries the ownership marker minted by makeStagingDir.
+    fs.writeFileSync(path.join(stagingDir, STAGING_MARKER), "99\n99\n", "utf-8");
     fs.writeFileSync(cpPath, JSON.stringify({
       dir: stagingDir,
       totalFiles: 1989,
@@ -143,7 +146,8 @@ describe("#1611 decideResume — checkpoint + staging detection", () => {
       timestamp: "2026-05-19T19:30:05.008Z",
     }), "utf-8");
 
-    const v = decideResume();
+    // gstackHome is injected so the ownership check anchors on the test home.
+    const v = decideResume(tmpHome);
     expect(v.kind).toBe("resume");
     if (v.kind === "resume") {
       expect(v.stagingDir).toBe(stagingDir);
@@ -160,13 +164,41 @@ describe("#1611 decideResume — checkpoint + staging detection", () => {
       processedIndex: 1000,
     }), "utf-8");
 
-    const v = decideResume();
+    const v = decideResume(tmpHome);
     expect(v.kind).toBe("stale-staging-missing");
     if (v.kind === "stale-staging-missing") {
       expect(v.stagingDir).toBe(stagingDir);
     }
   });
 
+  // ── #1802 regression: poisoned checkpoint must never be adopted/deleted ────
+
+  test("#1802 checkpoint.dir = repo root with .git → stale-staging-missing (not resumed)", () => {
+    // Reproduces the exact poison: an interrupted import wrote checkpoint.dir =
+    // the repo working tree. It exists and is a directory, so the pre-#1802
+    // code resumed (and cleanup later rm -rf'd it). It must now be refused.
+    const repoRoot = path.join(tmpHome, "my-repo");
+    fs.mkdirSync(path.join(repoRoot, ".git"), { recursive: true });
+    fs.writeFileSync(path.join(repoRoot, "important.py"), "# real work\n", "utf-8");
+    fs.writeFileSync(cpPath, JSON.stringify({ dir: repoRoot, totalFiles: 10, processedIndex: 3 }), "utf-8");
+
+    const v = decideResume(tmpHome);
+    expect(v.kind).toBe("stale-staging-missing");
+    // decideResume never deletes, but prove the repo is untouched by the verdict.
+    expect(fs.existsSync(path.join(repoRoot, "important.py"))).toBe(true);
+  });
+
+  test("#1802 staging-named dir WITHOUT marker → stale-staging-missing (not minted by us)", () => {
+    fs.mkdirSync(stagingDir, { recursive: true }); // .staging-ingest-99-99, but no marker
+    fs.writeFileSync(cpPath, JSON.stringify({ dir: stagingDir, totalFiles: 1, processedIndex: 0 }), "utf-8");
+    expect(decideResume(tmpHome).kind).toBe("stale-staging-missing");
+  });
+
+  test("#1802 checkpoint.dir = '/' → stale-staging-missing", () => {
+    fs.writeFileSync(cpPath, JSON.stringify({ dir: "/", totalFiles: 1, processedIndex: 0 }), "utf-8");
+    expect(decideResume(tmpHome).kind).toBe("stale-staging-missing");
+  });
+
   test("checkpoint with no dir field → no-checkpoint verdict", () => {
     fs.writeFileSync(cpPath, JSON.stringify({
       totalFiles: 1989,
@@ -222,6 +254,99 @@ describe("#1611 SIGTERM staging preservation — static invariants", () => {
     );
     expect(body).toMatch(/GSTACK_INGEST_RESUME_DIR/);
     expect(body).toMatch(/resuming from gbrain checkpoint/);
-    expect(body).toMatch(/previous checkpoint stale.*staging dir.*gone.*restaging from scratch/);
+    expect(body).toMatch(/previous checkpoint stale/);
+    expect(body).toMatch(/restaging from scratch/);
+    // #1802: the caller distinguishes "refused as unowned" from "actually gone".
+    expect(body).toMatch(/staging dir not usable/);
+  });
+});
+
+// ── #1802 checkOwnedStagingDir — fail-closed ownership matrix ───────────────
+// The single predicate guarding both the resume gate (decideResume) and the
+// deletion chokepoint (cleanupStagingDir). Every branch is fail-closed: any
+// case it cannot prove is owned must return ok:false.
+describe("#1802 checkOwnedStagingDir — ownership matrix", () => {
+  let home: string;
+
+  beforeEach(() => {
+    home = fs.mkdtempSync(path.join(os.tmpdir(), "gstack-1802-"));
+  });
+  afterEach(() => {
+    try { fs.rmSync(home, { recursive: true, force: true }); } catch { /* best-effort */ }
+  });
+
+  function mintStaging(name = ".staging-ingest-1-1"): string {
+    const d = path.join(home, name);
+    fs.mkdirSync(d, { recursive: true });
+    fs.writeFileSync(path.join(d, STAGING_MARKER), "1\n1\n", "utf-8");
+    return d;
+  }
+
+  test("minted staging dir → ok", () => {
+    expect(checkOwnedStagingDir(mintStaging(), home).ok).toBe(true);
+  });
+
+  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 });
+    expect(checkOwnedStagingDir(repo, home).ok).toBe(false);
+  });
+
+  test("staging-named dir containing .git → refused by tripwire even with marker", () => {
+    const d = mintStaging(".staging-ingest-9-9");
+    fs.mkdirSync(path.join(d, ".git"), { recursive: true });
+    const v = checkOwnedStagingDir(d, home);
+    expect(v.ok).toBe(false);
+    expect(v.reason).toMatch(/\.git/);
+  });
+
+  test("staging-named dir without marker → refused (not minted)", () => {
+    const d = path.join(home, ".staging-ingest-2-2");
+    fs.mkdirSync(d, { recursive: true });
+    expect(checkOwnedStagingDir(d, home).ok).toBe(false);
+  });
+
+  test("right name but NOT a direct child of home → refused", () => {
+    const nested = path.join(home, "sub", ".staging-ingest-3-3");
+    fs.mkdirSync(nested, { recursive: true });
+    fs.writeFileSync(path.join(nested, STAGING_MARKER), "x", "utf-8");
+    expect(checkOwnedStagingDir(nested, home).ok).toBe(false);
+  });
+
+  test("direct child of home but wrong name → refused", () => {
+    const d = path.join(home, "notstaging");
+    fs.mkdirSync(d, { recursive: true });
+    fs.writeFileSync(path.join(d, STAGING_MARKER), "x", "utf-8");
+    expect(checkOwnedStagingDir(d, home).ok).toBe(false);
+  });
+
+  test("missing path → refused (unresolvable)", () => {
+    expect(checkOwnedStagingDir(path.join(home, ".staging-ingest-gone"), home).ok).toBe(false);
+  });
+
+  test("'/' and '' → refused", () => {
+    expect(checkOwnedStagingDir("/", home).ok).toBe(false);
+    expect(checkOwnedStagingDir("", home).ok).toBe(false);
+  });
+
+  test("symlink whose target escapes home → refused (realpath resolves first)", () => {
+    const outside = path.join(home, "..", path.basename(home) + "-outside");
+    fs.mkdirSync(outside, { recursive: true });
+    const link = path.join(home, ".staging-ingest-link");
+    fs.symlinkSync(outside, link);
+    try {
+      // realpathSync resolves the link to `outside`, whose parent is not `home`.
+      expect(checkOwnedStagingDir(link, home).ok).toBe(false);
+    } finally {
+      try { fs.rmSync(outside, { recursive: true, force: true }); } catch { /* best-effort */ }
+    }
+  });
+
+  test("cleanupStagingDir + decideResume both call the guard (static invariant)", () => {
+    const ingest = fs.readFileSync(path.join(ROOT, "bin", "gstack-memory-ingest.ts"), "utf-8");
+    const sync = fs.readFileSync(path.join(ROOT, "bin", "gstack-gbrain-sync.ts"), "utf-8");
+    expect(ingest).toMatch(/checkOwnedStagingDir\(dir, GSTACK_HOME\)/);
+    expect(ingest).toMatch(/staging cleanup REFUSED/);
+    expect(sync).toMatch(/checkOwnedStagingDir\(stagingDir, gstackHome\)/);
   });
 });