diff --git a/bin/gstack-gbrain-sync.ts b/bin/gstack-gbrain-sync.ts index d88fc51a4..10c1f215b 100644 --- a/bin/gstack-gbrain-sync.ts +++ b/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)) { - return { kind: "stale-staging-missing", stagingDir }; - } - // Treat "non-empty" as the safe-to-resume signal. statSync on a missing - // file throws; we already handled missing above so this is dir-level shape. - try { - const st = statSync(stagingDir); - if (!st.isDirectory()) return { kind: "stale-staging-missing", stagingDir }; - } catch { - return { kind: "stale-staging-missing", stagingDir }; + // #1802: only resume into a path we can PROVE is a gstack-minted staging dir. + // 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 + // dir and later recursively deleted by cleanupStagingDir(). Fail-closed: any + // unprovable path restages from scratch (cost: one re-stage; never data loss). + // Pure decision: return the verdict (with reason) and let the caller log, + // so we don't double-log the same event from here and the call site. + const verdict = checkOwnedStagingDir(stagingDir, gstackHome); + if (!verdict.ok) { + 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.`, ); } diff --git a/bin/gstack-memory-ingest.ts b/bin/gstack-memory-ingest.ts index 98907eeee..fd7e53a73 100644 --- a/bin/gstack-memory-ingest.ts +++ b/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 { // 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 diff --git a/lib/staging-guard.ts b/lib/staging-guard.ts new file mode 100644 index 000000000..2487cafac --- /dev/null +++ b/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; +} diff --git a/test/regression-1611-gbrain-sync-resume.test.ts b/test/regression-1611-gbrain-sync-resume.test.ts index c1da8bb8c..481d3284c 100644 --- a/test/regression-1611-gbrain-sync-resume.test.ts +++ b/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\)/); }); });