From a554dfe82bb17fa74f45d8ccd10948301aa83204 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 28 Sep 2026 00:00:12 -0700 Subject: [PATCH] fix(crabbox): staging recovery rejects intact witness repos with stray ref files or large stores (#159989) * fix(crabbox): verify witness objects without ref-database hygiene Staging recovery rejected intact witness repositories whenever Git 2.50+ fsck's reference-database check found unrelated files such as Finder .DS_Store under .git/refs, and large stores exceeded the fixed 120 s budget; both surfaced as "could not verify local objects". Pass --no-references on Git 2.50+ (older Git never checks refs for explicit objects), classify fsck exit bits and spawn timeouts into distinct reasons, and derive the verification budget from measured object-storage allocation (120 s + 30 s/GiB, capped at 30 min) with a fresh base budget for revalidation. Missing or unconnected objects still fail closed. * fix(crabbox): keep automatic staging recovery on the base witness budget Automatic recovery runs before a successful wrapper command exits, so it keeps the 120 s witness bound; only explicit `staging recover` uses the measured object-storage budget. A test pins both paths. * fix(crabbox): keep automatic witness rechecks inside the original bound --- docs/reference/test/remote-proof.md | 9 + scripts/crabbox-staging-witness.mts | 95 +++++++- scripts/crabbox-staging.mts | 1 + test/scripts/crabbox-staging-witness.test.ts | 219 +++++++++++++++++++ 4 files changed, 312 insertions(+), 12 deletions(-) create mode 100644 test/scripts/crabbox-staging-witness.test.ts diff --git a/docs/reference/test/remote-proof.md b/docs/reference/test/remote-proof.md index 8297974362f2..47118af08cf6 100644 --- a/docs/reference/test/remote-proof.md +++ b/docs/reference/test/remote-proof.md @@ -314,6 +314,15 @@ node scripts/crabbox-wrapper.mjs staging recover \ --witness-repo /path/to/retained-repository --witness-ref refs/heads/saved-source ``` +Witness verification proves that the selected ref's objects are present and connected. +On Git 2.50+, it skips unrelated reference-database checks, so stray files such as +Finder `.DS_Store` under `.git/refs` do not block recovery. Explicit `staging recover` +scales its work budget with witness object storage: 120 seconds plus 30 seconds per +GiB, at most 30 minutes. Automatic recovery after a wrapper command keeps the +120-second bound. +Failure reasons distinguish an exhausted budget, reference-database errors, and +missing or unconnected objects. + Recovery does not create backup repositories, archives, or permanent refs. A stage's own Git objects or bundle do not count as another copy. Live or uncertain owners, unrecorded writer settlement, interrupted recovery ownership, substituted metadata, diff --git a/scripts/crabbox-staging-witness.mts b/scripts/crabbox-staging-witness.mts index 9cdf1e449df2..174d296b0a06 100644 --- a/scripts/crabbox-staging-witness.mts +++ b/scripts/crabbox-staging-witness.mts @@ -22,7 +22,13 @@ const sourceModes = new Set(["100644", "100755", "120000"]); const maxEntries = 100_000; const maxMetadataBytes = 64 * 1024 * 1024; const maxSourceBytes = 8 * 1024 * 1024 * 1024; -const verificationBudgetMs = 120_000; +// fsck --connectivity-only enumerates every stored object and walks the ref's full history, +// so cost follows object-store size (~126 s for a 19 GiB, 1,679-pack store on current hardware). +// 30 s per GiB leaves ~5x headroom for slower hosts. Automatic recovery runs before a +// successful wrapper command exits, so it keeps the base bound; explicit recovery scales. +const baseBudgetMs = 120_000; +const budgetPerGiBMs = 30_000; +const maxBudgetMs = 30 * 60_000; function gitEnvironment(): NodeJS.ProcessEnv { return { @@ -53,10 +59,16 @@ const gitOptions = [ "core.multiPackIndex=false", ]; +function workBudgetExceeded() { + return new Error( + "source witness verification exceeded its work budget; retry later, repack the witness repository, or choose another --witness-repo/--witness-ref", + ); +} + function remainingTime(deadline: number) { const remaining = deadline - Date.now(); if (remaining <= 0) { - throw new Error("source witness verification exceeded its work budget"); + throw workBudgetExceeded(); } return remaining; } @@ -65,7 +77,11 @@ function gitRead( location: string[], args: string[], deadline: number, - options: { absent?: boolean; quiet?: boolean } = {}, + options: { + absent?: boolean; + quiet?: boolean; + failure?: (status: number) => string | undefined; + } = {}, ): Buffer | undefined { const result = spawnSync("git", [...gitOptions, ...location, ...args], { env: gitEnvironment(), @@ -77,8 +93,15 @@ function gitRead( if (!result.error && options.absent && result.status === 1) { return undefined; } + if (result.error && "code" in result.error && result.error.code === "ETIMEDOUT") { + throw workBudgetExceeded(); + } if (result.error || result.status !== 0) { - throw new Error(`source witness Git ${args[0]} could not verify local objects`); + const reason = + !result.error && typeof result.status === "number" + ? options.failure?.(result.status) + : undefined; + throw new Error(reason ?? `source witness Git ${args[0]} could not verify local objects`); } return result.stdout ?? Buffer.alloc(0); } @@ -247,6 +270,7 @@ function validateSource(source: FrozenSource) { function storageIdentity(gitDir: string, payloadRoot: string, ref: string, deadline: number) { const digest = createHash("sha256"); let count = 0; + let objectBytes = 0n; const record = (path: string) => { remainingTime(deadline); const stat = lstatSync(path, { bigint: true, throwIfNoEntry: false }); @@ -323,11 +347,13 @@ function storageIdentity(gitDir: string, payloadRoot: string, ref: string, deadl } if (stat.isDirectory()) { walk(path, depth + 1); + } else if (stat.isFile()) { + objectBytes += stat.size > 4096n ? stat.size : 4096n; } } }; walk(join(gitDir, "objects"), 0); - return digest.digest("hex"); + return { identity: digest.digest("hex"), objectBytes }; } async function verifyBlobs( @@ -419,11 +445,31 @@ async function verifyBlobs( } } +function fsckFailure(status: number): string | undefined { + // ERROR_OBJECT/ERROR_REACHABLE/ERROR_PACK use 0o7; ERROR_REFS uses 0o10; >=128 overlaps fatal/usage exits. + if (status < 128 && (status & 0o7) !== 0) { + return "source witness Git fsck found missing, corrupt, or unconnected objects; choose a complete --witness-repo/--witness-ref"; + } + if (status === 0o10) { + return "source witness Git reference database has errors; repair its refs or choose another --witness-repo/--witness-ref"; + } + return undefined; +} + +function supportsNoReferences(location: string[], deadline: number) { + const version = /^git version (\d+)\.(\d+)/u.exec(gitText(location, ["version"], deadline)); + return ( + version !== null && + (Number(version[1]) > 2 || (Number(version[1]) === 2 && Number(version[2]) >= 50)) + ); +} + /** Read-only proof of another retained copy; the stage owner still owns disposal. */ export async function verifySourceWitness(params: { source: FrozenSource; witness: SourceWitness; payloadRoot: string; + automatic?: boolean; signal?: AbortSignal; }): Promise { try { @@ -433,11 +479,19 @@ export async function verifySourceWitness(params: { if (!objectId.test(commit) || !retainedRef(refName)) { throw new Error("source witness needs an exact commit and named non-staging ref"); } - const deadline = Date.now() + verificationBudgetMs; + const started = Date.now(); const payloadRoot = realpathSync(params.payloadRoot); const gitDir = realpathSync(params.witness.gitDir); const location = [`--git-dir=${gitDir}`]; - const before = storageIdentity(gitDir, payloadRoot, refName, deadline); + const before = storageIdentity(gitDir, payloadRoot, refName, started + baseBudgetMs); + const deadline = + started + + (params.automatic + ? baseBudgetMs + : Math.min( + maxBudgetMs, + baseBudgetMs + Math.ceil((Number(before.objectBytes) / 1024 ** 3) * budgetPerGiBMs), + )); const unsupported = gitRead( location, [ @@ -459,12 +513,23 @@ export async function verifySourceWitness(params: { } const ref = resolveRef(location, refName, deadline); gitRead(location, ["merge-base", "--is-ancestor", commit, ref.commit], deadline); + // Git 2.50 added fsck ref-database checks that judge unrelated refs (e.g. Finder .DS_Store + // under refs/); older Git never checks refs for explicit objects, so omitting the flag keeps the same proof. + const noReferences = supportsNoReferences(location, deadline); // No history object-ID list is buffered. Promisor/alternate routing was rejected above. gitRead( location, - ["fsck", "--connectivity-only", "--no-dangling", "--no-reflogs", "--no-progress", ref.oid], + [ + "fsck", + "--connectivity-only", + "--no-dangling", + "--no-reflogs", + "--no-progress", + ...(noReferences ? ["--no-references"] : []), + ref.oid, + ], deadline, - { quiet: true }, + { quiet: true, failure: fsckFailure }, ); const listing = gitRead( location, @@ -504,9 +569,15 @@ export async function verifySourceWitness(params: { ); const revalidate = () => { params.signal?.throwIfAborted(); - const currentStorage = storageIdentity(gitDir, payloadRoot, refName, deadline); - const after = resolveRef(location, refName, deadline); - if (currentStorage !== before || after.oid !== ref.oid || after.commit !== ref.commit) { + // Automatic rechecks share the original bound so post-command cleanup never exceeds it. + const revalidationDeadline = params.automatic ? deadline : Date.now() + baseBudgetMs; + const currentStorage = storageIdentity(gitDir, payloadRoot, refName, revalidationDeadline); + const after = resolveRef(location, refName, revalidationDeadline); + if ( + currentStorage.identity !== before.identity || + after.oid !== ref.oid || + after.commit !== ref.commit + ) { throw new Error("source witness changed while preservation was being verified"); } }; diff --git a/scripts/crabbox-staging.mts b/scripts/crabbox-staging.mts index ae7d42bf7dd9..fd81b117b395 100644 --- a/scripts/crabbox-staging.mts +++ b/scripts/crabbox-staging.mts @@ -1939,6 +1939,7 @@ async function recoverStaging(syncRoot: string, id: string, options: RecoveryOpt source: manifest.source, witness: selectedWitness!, payloadRoot: root, + automatic: options.automatic, signal: options.signal, }); if (!witness.ok) { diff --git a/test/scripts/crabbox-staging-witness.test.ts b/test/scripts/crabbox-staging-witness.test.ts new file mode 100644 index 000000000000..cad54d343a8b --- /dev/null +++ b/test/scripts/crabbox-staging-witness.test.ts @@ -0,0 +1,219 @@ +import { execFileSync, type SpawnSyncOptions } from "node:child_process"; +import { + existsSync, + mkdirSync, + realpathSync, + truncateSync, + unlinkSync, + writeFileSync, +} from "node:fs"; +import { delimiter, join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { verifySourceWitness, type FrozenSource } from "../../scripts/crabbox-staging-witness.mts"; +import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js"; +import { createNestedGitEnv } from "../helpers/temp-repo.js"; + +const fsck = vi.hoisted(() => ({ timeOut: false, budgets: [] as Array })); + +vi.mock("node:child_process", async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + spawnSync: (command: string, args: readonly string[], options: SpawnSyncOptions) => { + if (!args.includes("fsck")) { + return original.spawnSync(command, args, options); + } + fsck.budgets.push(options.timeout); + if (fsck.timeOut) { + return { + pid: 0, + output: [], + stdout: Buffer.alloc(0), + stderr: Buffer.alloc(0), + status: null, + signal: "SIGKILL", + error: Object.assign(new Error("spawnSync git ETIMEDOUT"), { code: "ETIMEDOUT" }), + }; + } + return original.spawnSync(command, args, options); + }, + }; +}); + +const temporary = useAutoCleanupTempDirTracker(afterEach); +afterEach(() => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + fsck.timeOut = false; + fsck.budgets.length = 0; +}); + +function fixture() { + const root = temporary.make("openclaw-staging-witness-"); + const repository = join(root, "witness"); + const payloadRoot = join(root, "payload"); + const home = join(root, "home"); + for (const directory of [repository, payloadRoot, home]) { + mkdirSync(directory); + } + const env: NodeJS.ProcessEnv = { + ...createNestedGitEnv(), + HOME: home, + USERPROFILE: home, + XDG_CONFIG_HOME: join(home, ".config"), + GIT_CONFIG_GLOBAL: join(home, "empty-global"), + GIT_CONFIG_SYSTEM: join(home, "empty-system"), + GIT_CONFIG_COUNT: "0", + GIT_AUTHOR_NAME: "Fixture", + GIT_AUTHOR_EMAIL: "fixture@example.invalid", + GIT_COMMITTER_NAME: "Fixture", + GIT_COMMITTER_EMAIL: "fixture@example.invalid", + }; + delete env.GIT_CONFIG_PARAMETERS; + writeFileSync(env.GIT_CONFIG_GLOBAL!, ""); + writeFileSync(env.GIT_CONFIG_SYSTEM!, ""); + const git = (...args: string[]) => + execFileSync("git", ["-C", repository, ...args], { env, encoding: "utf8" }).trim(); + git("init", "--quiet", "--initial-branch=main", "--template="); + writeFileSync(join(repository, "source.txt"), "old source\n"); + writeFileSync(join(repository, "history.txt"), "retained history only\n"); + git("add", "source.txt", "history.txt"); + git("-c", "commit.gpgsign=false", "commit", "--quiet", "-m", "retained history"); + const historyBlob = git("rev-parse", "HEAD:history.txt"); + writeFileSync(join(repository, "source.txt"), "witnessed source\n"); + unlinkSync(join(repository, "history.txt")); + git("add", "source.txt", "history.txt"); + git("-c", "commit.gpgsign=false", "commit", "--quiet", "-m", "witnessed source"); + const source: FrozenSource = { + files: [{ path: "source.txt", mode: "100644", blob: git("hash-object", "source.txt") }], + deleted: ["history.txt"], + }; + const witness = { + gitDir: realpathSync(join(repository, ".git")), + ref: "refs/heads/main", + commit: git("rev-parse", "HEAD"), + }; + for (const key of ["HOME", "USERPROFILE", "XDG_CONFIG_HOME"]) { + vi.stubEnv(key, env[key]); + } + const metadataPaths = ["refs/.DS_Store", "refs/heads/.DS_Store"].map((path) => + join(witness.gitDir, path), + ); + const addFinderMetadata = () => { + for (const path of metadataPaths) { + writeFileSync(path, Buffer.from([0, 0, 0, 1, 0x42, 0x75, 0x64, 0x31])); + } + }; + return { + root, + git, + historyBlob, + metadataPaths, + addFinderMetadata, + params: { source, witness, payloadRoot }, + }; +} + +describe.skipIf(process.platform === "win32")("Crabbox staging witness object proof", () => { + it("Finder metadata in the ref database does not block the object proof", async () => { + const f = fixture(); + f.addFinderMetadata(); + + expect(await verifySourceWitness(f.params)).toMatchObject({ ok: true }); + for (const path of f.metadataPaths) { + expect(existsSync(path)).toBe(true); + } + }); + + it("missing reachable history objects fail closed with the object reason", async () => { + const f = fixture(); + unlinkSync( + join(f.params.witness.gitDir, "objects", f.historyBlob.slice(0, 2), f.historyBlob.slice(2)), + ); + + expect(await verifySourceWitness(f.params)).toMatchObject({ + ok: false, + reason: expect.stringContaining("missing, corrupt, or unconnected objects"), + }); + }); + + it("older Git without --[no-]references keeps recovering", async () => { + const f = fixture(); + const realGit = execFileSync("/bin/sh", ["-c", "command -v git"], { + encoding: "utf8", + }).trim(); + const version = /^git version (\d+)\.(\d+)/u.exec(f.git("version")); + const shimDir = join(f.root, "bin"); + mkdirSync(shimDir); + writeFileSync( + join(shimDir, "git"), + `#!/bin/sh +for arg in "$@"; do + case "$arg" in + version) printf 'git version 2.49.0\\n'; exit 0 ;; + --no-references) printf 'unknown option: no-references\\n' >&2; exit 129 ;; + esac +done +exec '${realGit.replaceAll("'", "'\\''")}' "$@" +`, + { mode: 0o755 }, + ); + vi.stubEnv("PATH", shimDir + delimiter + process.env.PATH); + + expect(await verifySourceWitness(f.params)).toMatchObject({ ok: true }); + if ( + version && + (Number(version[1]) > 2 || (Number(version[1]) === 2 && Number(version[2]) >= 50)) + ) { + f.addFinderMetadata(); + expect(await verifySourceWitness(f.params)).toMatchObject({ + ok: false, + reason: expect.stringContaining("reference database has errors"), + }); + } + }); + + it("a Git read cut off by the budget reports budget exhaustion, not missing objects", async () => { + const f = fixture(); + fsck.timeOut = true; + + const result = await verifySourceWitness(f.params); + expect(result).toMatchObject({ + ok: false, + reason: expect.stringContaining("exceeded its work budget"), + }); + if (!result.ok) { + expect(result.reason).not.toContain("fsck"); + } + }); + + it("explicit recovery scales fsck time with object storage; automatic recovery keeps the base bound", async () => { + const f = fixture(); + // A sparse file stands in for a 4 GiB object store without allocating disk. + const sizing = join(f.params.witness.gitDir, "objects", "info", "sizing"); + mkdirSync(join(sizing, ".."), { recursive: true }); + writeFileSync(sizing, ""); + truncateSync(sizing, 4 * 1024 ** 3); + + expect(await verifySourceWitness(f.params)).toMatchObject({ ok: true }); + expect(await verifySourceWitness({ ...f.params, automatic: true })).toMatchObject({ + ok: true, + }); + const [explicit, automatic] = fsck.budgets; + expect(explicit).toBeGreaterThan(200_000); + expect(automatic).toBeLessThanOrEqual(120_000); + }); + + it("automatic revalidation before disposal stays inside the original bound", async () => { + const f = fixture(); + const explicit = await verifySourceWitness(f.params); + const automatic = await verifySourceWitness({ ...f.params, automatic: true }); + if (!explicit.ok || !automatic.ok) { + throw new Error("fixture witness must verify"); + } + vi.spyOn(Date, "now").mockReturnValue(Date.now() + 121_000); + + expect(() => automatic.revalidate()).toThrow("exceeded its work budget"); + expect(() => explicit.revalidate()).not.toThrow(); + }); +});