mirror of
https://github.com/garrytan/gstack.git
synced 2026-06-19 00:00:13 +02:00
fix(sync): fail-closed staging-dir ownership guard — prevent rm -rf of repo (#1802)
Adopts community fix #1827 by @diazMelgarejo (cyre). New lib/staging-guard.ts exports checkOwnedStagingDir(), the single fail-closed predicate for 'safe to recurse-delete or resume into', wired at cleanupStagingDir() (the deletion chokepoint), decideResume(), the ingest entry point, and makeStagingDir() (mints the .gstack-staging marker). Fixes #1802. Co-Authored-By: cyre <diazMelgarejo@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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\)/);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user