From e48016bf319b86aca599d37d3ff4f5881d047726 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Sat, 31 Oct 2026 07:14:41 +0900 Subject: [PATCH 1/8] fix: compare Lab file identity with bigint stat NTFS file IDs can exceed 2^53, where adjacent IDs are equal as JS numbers. Take dev/ino from bigint stats in the Lab mutation lock, private-file stage cleanup, private reads, artifact and scratch directory checks. --- src/lab/artifacts/secure-fs.ts | 23 +++++++++--------- src/lab/fabric/scratch.ts | 20 +++++++++------- src/lab/public/file-safety.ts | 24 +++++++++---------- src/lab/public/mutation-lock.ts | 12 ++++------ src/lab/public/private-file.ts | 7 +++--- tests/lab/lab-community-mutation-lock.test.ts | 17 +++++++++++++ tests/lab/lab-fabric-task.test.ts | 19 +++++++++++++++ tests/lab/lab-post-merge-hardening.test.ts | 23 +++++++++++++++++- tests/lab/lab-private-file-durability.test.ts | 21 +++++++++++++++- tests/lab/lab-public-file-safety.test.ts | 24 ++++++++++++++++++- tests/lib/file-identity-callers.test.ts | 9 ++++++- 11 files changed, 153 insertions(+), 46 deletions(-) diff --git a/src/lab/artifacts/secure-fs.ts b/src/lab/artifacts/secure-fs.ts index 80cd82d0f6c..c10a02ac6ad 100644 --- a/src/lab/artifacts/secure-fs.ts +++ b/src/lab/artifacts/secure-fs.ts @@ -19,8 +19,10 @@ import { unlinkSync, writeSync, type Stats, + type BigIntStats, } from "node:fs"; import { join } from "node:path"; +import { fileIdentityKey, sameFileIdentity, type FileIdentity } from "../../lib/file-identity"; import { ARTIFACT_FILENAME_EXT, MAX_BYTES_PER_ARTIFACT, @@ -72,7 +74,7 @@ function openFlags(base: number, noFollow: boolean): number { return base; } -function assertRegularFileStats(stats: Stats, label: string): void { +function assertRegularFileStats(stats: Stats | BigIntStats, label: string): void { if ( !stats.isFile() || stats.isSymbolicLink() || @@ -84,19 +86,19 @@ function assertRegularFileStats(stats: Stats, label: string): void { ) { harnessFailure(`${label}: not a regular file`, "artifact_unsafe_target"); } - if (stats.nlink !== 1) { + if (Number(stats.nlink) !== 1) { harnessFailure(`${label}: hard links prohibited (nlink=${stats.nlink})`, "artifact_unsafe_target"); } } -function assertDirectoryStats(stats: Stats, label: string): void { +function assertDirectoryStats(stats: Stats | BigIntStats, label: string): void { if (!stats.isDirectory() || stats.isSymbolicLink()) { harnessFailure(`${label}: must be a real directory (no symlink/reparse redirection)`, "artifact_unsafe_target"); } } -function identityOf(stats: Stats): string { - return `${stats.dev}:${stats.ino}`; +function identityOf(stats: FileIdentity): string { + return fileIdentityKey(stats); } function assertRelativeName(name: string): void { @@ -154,14 +156,13 @@ function childPath(dir: TrustedArtifactDir, name: string): string { function assertOpenedPathMatchesDescriptor(dir: TrustedArtifactDir, name: string, fd: number): void { if (detectArtifactIoMode(dir) !== "win32_pinned") return; - const opened = fstatSync(fd); + const opened = fstatSync(fd, { bigint: true }); assertRegularFileStats(opened, "artifact fd"); - const pathEntry = lstatSync(childPath(dir, name)); + const pathEntry = lstatSync(childPath(dir, name), { bigint: true }); if ( !pathEntry.isFile() || pathEntry.isSymbolicLink() || - pathEntry.dev !== opened.dev || - pathEntry.ino !== opened.ino + !sameFileIdentity(pathEntry, opened) ) { closeSync(fd); harnessFailure("artifact path identity mismatch after open", "artifact_unsafe_target"); @@ -202,7 +203,7 @@ function unlinkAtDir(dir: TrustedArtifactDir, name: string): void { } function revalidateDir(dir: TrustedArtifactDir): void { - const stats = fstatSync(dir.fd); + const stats = fstatSync(dir.fd, { bigint: true }); assertDirectoryStats(stats, "artifacts dir"); if (identityOf(stats) !== dir.identity) { harnessFailure("artifacts directory identity changed", "artifact_unsafe_target"); @@ -225,7 +226,7 @@ export function openTrustedArtifactDir(artifactsDir: string): TrustedArtifactDir fd = openSync(abs, O_RDONLY); } - const stats = fstatSync(fd); + const stats = fstatSync(fd, { bigint: true }); assertDirectoryStats(stats, "artifacts dir"); const trusted = { path: abs, fd, identity: identityOf(stats) }; detectArtifactIoMode(trusted); diff --git a/src/lab/fabric/scratch.ts b/src/lab/fabric/scratch.ts index a634df8022e..64282a0b9f7 100644 --- a/src/lab/fabric/scratch.ts +++ b/src/lab/fabric/scratch.ts @@ -12,9 +12,11 @@ import { statSync, writeSync, type Stats, + type BigIntStats, } from "node:fs"; import { dirname, join, posix, relative, resolve, sep } from "node:path"; import { randomBytes } from "node:crypto"; +import { fileIdentityKey, type FileIdentity } from "../../lib/file-identity"; import { ensureLabDirs, ensureRestrictedDir, labRoot, labScratchDir } from "../paths"; import { FABRIC_LIMITS, SYNTHETIC_BEFORE_UTF8, SYNTHETIC_VALUE_PATH } from "./constants"; import { FabricTaskError } from "./types"; @@ -35,22 +37,22 @@ interface TrustedScratchDir { // inside producer-writable scratch nor its age grants later deletion authority. /** Require stats to describe a regular file, not a symlink or special node. */ -function assertRegularFile(stats: Stats, label: string): void { +function assertRegularFile(stats: Stats | BigIntStats, label: string): void { if (!stats.isFile() || stats.isSymbolicLink() || stats.isDirectory() || stats.isFIFO() || stats.isSocket() || stats.isCharacterDevice() || stats.isBlockDevice()) { throw new FabricTaskError(`${label} must be a regular file`, "sandbox_violation", "harness"); } } /** Require stats to describe a real directory, not a symlink. */ -function assertRealDirectory(stats: Stats, label: string): void { +function assertRealDirectory(stats: Stats | BigIntStats, label: string): void { if (stats.isSymbolicLink() || !stats.isDirectory()) { throw new FabricTaskError(`${label} must be a real directory`, "sandbox_violation", "harness"); } } /** Stable device/inode identity string for a filesystem node. */ -function identityOf(stats: Stats): string { - return `${stats.dev}:${stats.ino}`; +function identityOf(stats: FileIdentity): string { + return fileIdentityKey(stats); } /** Whether the host supports O_NOFOLLOW for scratch open operations. */ @@ -73,7 +75,7 @@ function assertScratchName(name: string): void { /** Confirm the scratch root path still refers to the same directory inode. */ function revalidateScratchDir(dir: TrustedScratchDir): void { - const stats = lstatSync(dir.path); + const stats = lstatSync(dir.path, { bigint: true }); assertRealDirectory(stats, "scratch dir"); if (identityOf(stats) !== dir.identity) { throw new FabricTaskError("scratch directory identity changed", "sandbox_violation", "harness"); @@ -93,12 +95,12 @@ function openAtScratch(dir: TrustedScratchDir, name: string, flags: number, mode assertScratchName(name); const path = childScratchPath(dir, name); const fd = openSync(path, openFlags(flags), mode); - const opened = fstatSync(fd); + const opened = fstatSync(fd, { bigint: true }); if (expectDirectory) { assertRealDirectory(opened, name); } else { assertRegularFile(opened, name); - const pathEntry = lstatSync(path); + const pathEntry = lstatSync(path, { bigint: true }); if (identityOf(opened) !== identityOf(pathEntry)) { throw new FabricTaskError("scratch path redirection detected", "sandbox_violation", "harness"); } @@ -115,7 +117,7 @@ function openTrustedScratchRoot(scratchRoot: string): TrustedScratchDir { } else { fd = openSync(abs, fsConstants.O_RDONLY); } - const stats = fstatSync(fd); + const stats = fstatSync(fd, { bigint: true }); assertRealDirectory(stats, "scratch root"); return { path: abs, fd, identity: identityOf(stats) }; } @@ -149,7 +151,7 @@ function openScratchRelativePath( : openFlags(fsConstants.O_RDONLY, true); const subFd = openAtScratch(current, part, dirFlags, 0, true); intermediateFds.push(subFd); - const subStats = fstatSync(subFd); + const subStats = fstatSync(subFd, { bigint: true }); assertRealDirectory(subStats, part); current = { path: join(current.path, part), fd: subFd, identity: identityOf(subStats) }; continue; diff --git a/src/lab/public/file-safety.ts b/src/lab/public/file-safety.ts index 78134929cf6..fae5365d94d 100644 --- a/src/lab/public/file-safety.ts +++ b/src/lab/public/file-safety.ts @@ -10,6 +10,7 @@ import { unlinkSync, } from "node:fs"; import { basename, dirname, join } from "node:path"; +import { sameFileIdentity } from "../../lib/file-identity"; import { isPrivateFileStageName } from "./private-file"; import { PublicEvidenceValidationError } from "./validate"; @@ -95,32 +96,31 @@ function withPrivateRegularFile( options: PrivateRegularFileReadOptions, consume: (fd: number, size: number) => T, ): T { - let pathStats = lstatSync(path); - if (!pathStats.isSymbolicLink() && pathStats.isFile() && pathStats.nlink > 1) { + let pathStats = lstatSync(path, { bigint: true }); + if (!pathStats.isSymbolicLink() && pathStats.isFile() && Number(pathStats.nlink) > 1) { recoverPublishedPrivateFileStage(path); - pathStats = lstatSync(path); + pathStats = lstatSync(path, { bigint: true }); } - if (pathStats.isSymbolicLink() || !pathStats.isFile() || pathStats.nlink !== 1) { + if (pathStats.isSymbolicLink() || !pathStats.isFile() || Number(pathStats.nlink) !== 1) { throw new PublicEvidenceValidationError(options.errorCode, options.errorMessage); } - if (pathStats.size > options.maxBytes) throw sizeError(options); + if (Number(pathStats.size) > options.maxBytes) throw sizeError(options); const fd = openSync(path, fsConstants.O_RDONLY | O_NOFOLLOW); try { - const stats = fstatSync(fd); + const stats = fstatSync(fd, { bigint: true }); if ( !stats.isFile() - || stats.nlink !== 1 - || stats.dev !== pathStats.dev - || stats.ino !== pathStats.ino + || Number(stats.nlink) !== 1 + || !sameFileIdentity(stats, pathStats) ) { throw new PublicEvidenceValidationError(options.errorCode, options.errorMessage); } - if (stats.size > options.maxBytes) throw sizeError(options); - if (options.requireMode600 && process.platform !== "win32" && (stats.mode & 0o777) !== 0o600) { + if (Number(stats.size) > options.maxBytes) throw sizeError(options); + if (options.requireMode600 && process.platform !== "win32" && (Number(stats.mode) & 0o777) !== 0o600) { throw new PublicEvidenceValidationError(options.errorCode, options.errorMessage); } - return consume(fd, stats.size); + return consume(fd, Number(stats.size)); } finally { closeSync(fd); } diff --git a/src/lab/public/mutation-lock.ts b/src/lab/public/mutation-lock.ts index 8b52012e28e..81261f97274 100644 --- a/src/lab/public/mutation-lock.ts +++ b/src/lab/public/mutation-lock.ts @@ -9,6 +9,7 @@ import { writeFileSync, } from "node:fs"; import { dirname, join } from "node:path"; +import { sameFileIdentity, statTimeMs, type FileIdentity } from "../../lib/file-identity"; import { ensureLabDirs, labCommunityDir } from "../paths"; import { readPrivateRegularFile } from "./file-safety"; import { PublicEvidenceValidationError } from "./validate"; @@ -40,10 +41,7 @@ type MutationLockReclaim = { createdAt: number; }; -type MutationLockDirectoryIdentity = { - dev: number; - ino: number; -}; +type MutationLockDirectoryIdentity = FileIdentity; function mutationLockPath(configDir?: string): string { return join(labCommunityDir(configDir), PUBLIC_EVIDENCE_MUTATION_LOCK_NAME); @@ -106,7 +104,7 @@ function readMutationLockReclaim(lockPath: string): MutationLockReclaim | null { } function assertMutationLockDirectory(lockPath: string) { - const stat = lstatSync(lockPath); + const stat = lstatSync(lockPath, { bigint: true }); if (!stat.isDirectory() || stat.isSymbolicLink()) { throw new PublicEvidenceValidationError( "community_cache_lock", @@ -120,7 +118,7 @@ function sameDirectoryIdentity( left: MutationLockDirectoryIdentity, right: MutationLockDirectoryIdentity, ): boolean { - return left.dev === right.dev && left.ino === right.ino; + return sameFileIdentity(left, right); } function currentDirectoryIdentity(lockPath: string): MutationLockDirectoryIdentity { @@ -139,7 +137,7 @@ function mutationLockIsReclaimable(lockPath: string, nowMs: number): boolean { // The only ownerless state is the tiny mkdir-to-owner publication window. Use // a deliberately long fallback so a crashed acquisition can eventually heal // without treating an ordinary pause as proof that the owner disappeared. - return nowMs - stat.mtimeMs > PUBLIC_EVIDENCE_MUTATION_LOCK_INCOMPLETE_STALE_MS; + return nowMs - statTimeMs(stat.mtimeNs) > PUBLIC_EVIDENCE_MUTATION_LOCK_INCOMPLETE_STALE_MS; } function unlinkIfPresent(path: string): void { diff --git a/src/lab/public/private-file.ts b/src/lab/public/private-file.ts index 65e98325f1a..ec2b76937c2 100644 --- a/src/lab/public/private-file.ts +++ b/src/lab/public/private-file.ts @@ -12,6 +12,7 @@ import { } from "node:fs"; import type { Stats } from "node:fs"; import { basename, dirname, join } from "node:path"; +import { sameFileIdentity } from "../../lib/file-identity"; export type PrivateFileCommitFault = "before_publish" | "parent_directory_sync" | null; let privateFileCommitFaultForTests: PrivateFileCommitFault = null; @@ -152,7 +153,7 @@ export function cleanupStalePrivateFileStages(finalPath: string): void { function cleanupPublishedPrivateFileStages(finalPath: string): void { let finalStats; try { - finalStats = lstatSync(finalPath); + finalStats = lstatSync(finalPath, { bigint: true }); } catch (error) { if ((error as NodeJS.ErrnoException).code === "ENOENT") return; throw error; @@ -166,9 +167,9 @@ function cleanupPublishedPrivateFileStages(finalPath: string): void { if (!name.startsWith(prefix) || !PRIVATE_STAGE_RE.test(name)) continue; const stagePath = join(dir, name); try { - const stageStats = lstatSync(stagePath); + const stageStats = lstatSync(stagePath, { bigint: true }); if (!stageStats.isFile() || stageStats.isSymbolicLink()) continue; - if (stageStats.dev !== finalStats.dev || stageStats.ino !== finalStats.ino) continue; + if (!sameFileIdentity(stageStats, finalStats)) continue; unlinkSync(stagePath); changed = true; } catch (error) { diff --git a/tests/lab/lab-community-mutation-lock.test.ts b/tests/lab/lab-community-mutation-lock.test.ts index 7c3132d155d..7e27ed34d91 100644 --- a/tests/lab/lab-community-mutation-lock.test.ts +++ b/tests/lab/lab-community-mutation-lock.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, spyOn, test } from "bun:test"; +import * as fs from "node:fs"; import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -41,6 +42,22 @@ function createLiveOwnerLock(config: string): string { } describe("community mutation lock", () => { + test("does not publish into a lock replaced with an adjacent NTFS-sized identity", () => { + const config = configDir(); + const lockPath = join(labCommunityDir(config), ".mutation-lock"); + const original = fs.lstatSync; + let observations = 0; + const spy = spyOn(fs, "lstatSync").mockImplementation(((path, options) => { + const stats = options?.bigint ? original(path, { bigint: true }) : original(path); + if (path !== lockPath) return stats; + const ino = 9_007_199_254_740_992n + BigInt(Math.min(observations++, 1)); + return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + { ino: options?.bigint ? ino : Number(ino) }); + }) as typeof fs.lstatSync); + try { + expect(() => withPublicEvidenceMutationLock(config, () => {})).toThrow(PublicEvidenceValidationError); + } finally { spy.mockRestore(); } + }); test("recovers an ancient incomplete lock before reading committed cache state", () => { const config = configDir(); const lockPath = join(labCommunityDir(config), ".mutation-lock"); diff --git a/tests/lab/lab-fabric-task.test.ts b/tests/lab/lab-fabric-task.test.ts index 4ee3acaa5b0..d62cb60ec09 100644 --- a/tests/lab/lab-fabric-task.test.ts +++ b/tests/lab/lab-fabric-task.test.ts @@ -70,6 +70,25 @@ import { removeTreeWithRetry } from "../helpers/remove-tree"; import { repoRoot } from "../helpers/repo-root"; const REPO_ROOT = repoRoot(); + +test("scratch directory rejects an adjacent NTFS-sized identity", () => { + const root = nodeFs.mkdtempSync(join(tmpdir(), "ocx-scratch-identity-")); + const originalFstat = nodeFs.fstatSync, originalLstat = nodeFs.lstatSync; + nodeFs.writeFileSync(join(root, "value"), "test"); + const fdSpy = spyOn(nodeFs, "fstatSync").mockImplementation(((fd, options) => { + const stats = options?.bigint ? originalFstat(fd, { bigint: true }) : originalFstat(fd); + return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + { ino: options?.bigint ? 9_007_199_254_740_992n : Number(9_007_199_254_740_992n) }); + }) as typeof nodeFs.fstatSync); + const pathSpy = spyOn(nodeFs, "lstatSync").mockImplementation(((path, options) => { + const stats = options?.bigint ? originalLstat(path, { bigint: true }) : originalLstat(path); + return path === root && options?.bigint ? Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + { ino: 9_007_199_254_740_993n }) : path === root ? Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + { ino: Number(9_007_199_254_740_993n) }) : stats; + }) as typeof nodeFs.lstatSync); + try { expect(() => readScratchFileUtf8(root, "value", 1024)).toThrow("scratch directory identity changed"); } + finally { fdSpy.mockRestore(); pathSpy.mockRestore(); removeTreeWithRetry(root); } +}); const CHILD_REAP_GRACE_MS = 2_000; async function awaitChildExitWithin(child: Bun.Subprocess, timeoutMs: number): Promise { diff --git a/tests/lab/lab-post-merge-hardening.test.ts b/tests/lab/lab-post-merge-hardening.test.ts index 6504d7606aa..1ba8eb38035 100644 --- a/tests/lab/lab-post-merge-hardening.test.ts +++ b/tests/lab/lab-post-merge-hardening.test.ts @@ -1,4 +1,5 @@ -import { afterEach, expect, test } from "bun:test"; +import { afterEach, expect, spyOn, test } from "bun:test"; +import * as fs from "node:fs"; import { existsSync, mkdirSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -28,6 +29,26 @@ import { removeTreeWithRetry } from "../helpers/remove-tree"; const HOMES: string[] = []; const previousHome = process.env.OPENCODEX_HOME; +test("artifact directory rejects an adjacent NTFS-sized identity", () => { + const path = join(tempHome(), "artifacts"); + const original = fs.fstatSync; + let observedFd: number | undefined, observations = 0; + const spy = spyOn(fs, "fstatSync").mockImplementation(((fd, options) => { + const stats = options?.bigint ? original(fd, { bigint: true }) : original(fd); + if (observedFd === undefined) observedFd = fd; + if (fd !== observedFd) return stats; + const ino = 9_007_199_254_740_992n + BigInt(Math.min(observations++, 1)); + return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + { ino: options?.bigint ? ino : Number(ino) }); + }) as typeof fs.fstatSync); + let dir: ReturnType | undefined; + try { + dir = openTrustedArtifactDir(path); + expect(() => putNamedDigestBytes(dir!, "a".repeat(64), Buffer.from("test"), () => "a".repeat(64))) + .toThrow("artifacts directory identity changed"); + } finally { if (dir) closeTrustedArtifactDir(dir); spy.mockRestore(); } +}); + function tempHome(): string { const dir = join(tmpdir(), `ocx-lab-hardening-${process.pid}-${Math.random().toString(16).slice(2)}`); mkdirSync(dir, { recursive: true, mode: 0o700 }); diff --git a/tests/lab/lab-private-file-durability.test.ts b/tests/lab/lab-private-file-durability.test.ts index 8956451a29c..7a985e922d0 100644 --- a/tests/lab/lab-private-file-durability.test.ts +++ b/tests/lab/lab-private-file-durability.test.ts @@ -1,4 +1,5 @@ -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, spyOn, test } from "bun:test"; +import * as fs from "node:fs"; import { existsSync, mkdtempSync, readFileSync, readdirSync, utimesSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -27,6 +28,24 @@ function tempRoot(): string { } describe("CL-10 private-file durability", () => { + test("published-stage cleanup preserves an adjacent NTFS-sized replacement", () => { + const root = tempRoot(), finalPath = join(root, "bundle.json"); + const stagePath = join(root, `.bundle.json.${process.pid}.00000000-0000-4000-8000-000000000000.tmp`); + fs.writeFileSync(finalPath, "final"); + fs.writeFileSync(stagePath, "replacement"); + const original = fs.lstatSync; + const spy = spyOn(fs, "lstatSync").mockImplementation(((path, options) => { + const stats = options?.bigint ? original(path, { bigint: true }) : original(path); + if (path !== finalPath && path !== stagePath) return stats; + const ino = 9_007_199_254_740_992n + (path === stagePath ? 1n : 0n); + return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + { ino: options?.bigint ? ino : Number(ino) }); + }) as typeof fs.lstatSync); + try { + expect(publishPrivateFileExclusive(finalPath, Buffer.from("candidate"))).toEqual({ created: false }); + expect(existsSync(stagePath)).toBe(true); + } finally { spy.mockRestore(); } + }); test("POSIX parent-directory sync failure preserves the published stage until retry", () => { if (process.platform === "win32") return; const root = tempRoot(); diff --git a/tests/lab/lab-public-file-safety.test.ts b/tests/lab/lab-public-file-safety.test.ts index 113d7837acc..b5687188257 100644 --- a/tests/lab/lab-public-file-safety.test.ts +++ b/tests/lab/lab-public-file-safety.test.ts @@ -1,4 +1,5 @@ -import { afterEach, expect, test } from "bun:test"; +import { afterEach, expect, spyOn, test } from "bun:test"; +import * as fs from "node:fs"; import { linkSync, mkdtempSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -17,6 +18,27 @@ function tempRoot(): string { return root; } +test("private reads reject adjacent NTFS-sized descriptor identities", () => { + const path = join(tempRoot(), "target.txt"); + writeFileSync(path, "safe-bytes"); + const original = fs.fstatSync; + const spy = spyOn(fs, "fstatSync").mockImplementation(((fd, options) => { + const stats = options?.bigint ? original(fd, { bigint: true }) : original(fd); + return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + { ino: options?.bigint ? 9_007_199_254_740_993n : Number(9_007_199_254_740_993n) }); + }) as typeof fs.fstatSync); + const originalLstat = fs.lstatSync; + const pathSpy = spyOn(fs, "lstatSync").mockImplementation(((p, options) => { + const stats = options?.bigint ? originalLstat(p, { bigint: true }) : originalLstat(p); + return p === path ? Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + { ino: options?.bigint ? 9_007_199_254_740_992n : Number(9_007_199_254_740_992n) }) : stats; + }) as typeof fs.lstatSync); + try { + expect(() => readPrivateRegularFile(path, { maxBytes: 1024, errorCode: "unsafe", errorMessage: "unsafe" })) + .toThrow(PublicEvidenceValidationError); + } finally { spy.mockRestore(); pathSpy.mockRestore(); } +}); + test("descriptor-bound private reads reject a symlink even when O_NOFOLLOW is unavailable", () => { const root = tempRoot(); const target = join(root, "target.txt"); diff --git a/tests/lib/file-identity-callers.test.ts b/tests/lib/file-identity-callers.test.ts index 43004e286f2..adb794664a4 100644 --- a/tests/lib/file-identity-callers.test.ts +++ b/tests/lib/file-identity-callers.test.ts @@ -6,6 +6,11 @@ import { repoPath } from "../helpers/repo-root"; // calls made with { bigint: true }. A lexical tripwire over the named declarations, not a // full parser: it catches the regression forms below and fails closed on scopes it cannot find. const scopes: Record = { + "src/lab/public/mutation-lock.ts": ["assertMutationLockDirectory"], + "src/lab/public/private-file.ts": ["cleanupPublishedPrivateFileStages"], + "src/lab/public/file-safety.ts": ["withPrivateRegularFile"], + "src/lab/artifacts/secure-fs.ts": ["assertOpenedPathMatchesDescriptor", "revalidateDir", "openTrustedArtifactDir"], + "src/lab/fabric/scratch.ts": ["revalidateScratchDir", "openAtScratch", "openTrustedScratchRoot", "openScratchRelativePath"], "src/codex/codex-home-owner.ts": ["readCodexHomeJournal"], "src/codex/cli-update-artifacts.ts": ["verifyCodexCliArchive"], "src/codex/cli-install-provenance.ts": ["readBoundedFile", "observeHeldFileSet"], @@ -111,7 +116,7 @@ function matching(code: string, open: number, left: string, right: string): numb /** The body of NAME's declaration: a function, or a method head whose body opens on the same line. */ function declarationBody(code: string, name: string): [number, number] | null { - const head = new RegExp(`(?:\\bfunction\\s+|^[ \\t]*(?:(?:export|async|public|private|protected|static|override)\\s+)*)${name}\\s*\\(`, "gm"); + const head = new RegExp(`(?:\\bfunction\\s+|^[ \\t]*(?:(?:export|async|public|private|protected|static|override)\\s+)*)${name}(?:<[^<>]*>)?\\s*\\(`, "gm"); for (const match of code.matchAll(head)) { const close = matching(code, match.index! + match[0].length - 1, "(", ")"); if (close < 0) continue; @@ -199,6 +204,8 @@ describe("file identity caller stats", () => { }); test("resolves the declaration, not an earlier call, and needs a real observation", () => { + expect(inspect("function check(value: T) { return lstatSync(path).ino; }", ["check"])) + .toEqual(["numeric stat: lstatSync(path)"]); expect(inspect("function caller() { return check(); }\nfunction check() { return lstatSync(path).ino; }", ["check"])) .toEqual(["numeric stat: lstatSync(path)"]); expect(inspect("check()\n{ lstatSync(p, { bigint: true }); }\nfunction check() { return lstatSync(path).ino; }", ["check"])) From 0644d73fca3accd1e28d01969f5bdefd7e12ca75 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Sat, 31 Oct 2026 07:50:21 +0900 Subject: [PATCH 2/8] test: select Lab injected stat targets by canonical path --- tests/lab/lab-community-mutation-lock.test.ts | 3 ++- tests/lab/lab-fabric-task.test.ts | 5 +++-- tests/lab/lab-private-file-durability.test.ts | 5 +++-- tests/lab/lab-public-file-safety.test.ts | 3 ++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/lab/lab-community-mutation-lock.test.ts b/tests/lab/lab-community-mutation-lock.test.ts index 7e27ed34d91..1d228bb3ca3 100644 --- a/tests/lab/lab-community-mutation-lock.test.ts +++ b/tests/lab/lab-community-mutation-lock.test.ts @@ -1,3 +1,4 @@ +import { samePath } from "../helpers/same-path"; import { afterEach, describe, expect, spyOn, test } from "bun:test"; import * as fs from "node:fs"; import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, utimesSync, writeFileSync } from "node:fs"; @@ -49,7 +50,7 @@ describe("community mutation lock", () => { let observations = 0; const spy = spyOn(fs, "lstatSync").mockImplementation(((path, options) => { const stats = options?.bigint ? original(path, { bigint: true }) : original(path); - if (path !== lockPath) return stats; + if (!samePath(path, lockPath)) return stats; const ino = 9_007_199_254_740_992n + BigInt(Math.min(observations++, 1)); return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, { ino: options?.bigint ? ino : Number(ino) }); diff --git a/tests/lab/lab-fabric-task.test.ts b/tests/lab/lab-fabric-task.test.ts index d62cb60ec09..7f7e0def9b8 100644 --- a/tests/lab/lab-fabric-task.test.ts +++ b/tests/lab/lab-fabric-task.test.ts @@ -1,3 +1,4 @@ +import { samePath } from "../helpers/same-path"; import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import * as nodeFs from "node:fs"; import { @@ -82,8 +83,8 @@ test("scratch directory rejects an adjacent NTFS-sized identity", () => { }) as typeof nodeFs.fstatSync); const pathSpy = spyOn(nodeFs, "lstatSync").mockImplementation(((path, options) => { const stats = options?.bigint ? originalLstat(path, { bigint: true }) : originalLstat(path); - return path === root && options?.bigint ? Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, - { ino: 9_007_199_254_740_993n }) : path === root ? Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + return samePath(path, root) && options?.bigint ? Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + { ino: 9_007_199_254_740_993n }) : samePath(path, root) ? Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, { ino: Number(9_007_199_254_740_993n) }) : stats; }) as typeof nodeFs.lstatSync); try { expect(() => readScratchFileUtf8(root, "value", 1024)).toThrow("scratch directory identity changed"); } diff --git a/tests/lab/lab-private-file-durability.test.ts b/tests/lab/lab-private-file-durability.test.ts index 7a985e922d0..0f5b9149623 100644 --- a/tests/lab/lab-private-file-durability.test.ts +++ b/tests/lab/lab-private-file-durability.test.ts @@ -1,3 +1,4 @@ +import { samePath } from "../helpers/same-path"; import { afterEach, describe, expect, spyOn, test } from "bun:test"; import * as fs from "node:fs"; import { existsSync, mkdtempSync, readFileSync, readdirSync, utimesSync } from "node:fs"; @@ -36,8 +37,8 @@ describe("CL-10 private-file durability", () => { const original = fs.lstatSync; const spy = spyOn(fs, "lstatSync").mockImplementation(((path, options) => { const stats = options?.bigint ? original(path, { bigint: true }) : original(path); - if (path !== finalPath && path !== stagePath) return stats; - const ino = 9_007_199_254_740_992n + (path === stagePath ? 1n : 0n); + if (!samePath(path, finalPath) && !samePath(path, stagePath)) return stats; + const ino = 9_007_199_254_740_992n + (samePath(path, stagePath) ? 1n : 0n); return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, { ino: options?.bigint ? ino : Number(ino) }); }) as typeof fs.lstatSync); diff --git a/tests/lab/lab-public-file-safety.test.ts b/tests/lab/lab-public-file-safety.test.ts index b5687188257..e11b9e3ed40 100644 --- a/tests/lab/lab-public-file-safety.test.ts +++ b/tests/lab/lab-public-file-safety.test.ts @@ -1,3 +1,4 @@ +import { samePath } from "../helpers/same-path"; import { afterEach, expect, spyOn, test } from "bun:test"; import * as fs from "node:fs"; import { linkSync, mkdtempSync, symlinkSync, writeFileSync } from "node:fs"; @@ -30,7 +31,7 @@ test("private reads reject adjacent NTFS-sized descriptor identities", () => { const originalLstat = fs.lstatSync; const pathSpy = spyOn(fs, "lstatSync").mockImplementation(((p, options) => { const stats = options?.bigint ? originalLstat(p, { bigint: true }) : originalLstat(p); - return p === path ? Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + return samePath(p, path) ? Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, { ino: options?.bigint ? 9_007_199_254_740_992n : Number(9_007_199_254_740_992n) }) : stats; }) as typeof fs.lstatSync); try { From 592a98601f9681861d17237d1f83eab3262c3454 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Sat, 31 Oct 2026 10:49:11 +0900 Subject: [PATCH 3/8] fix: bind Lab lock release to the checked directory Release, stale reclaim and failed-acquisition cleanup move the canonical lock to a unique detached name first and remove it only when the moved directory has the checked identity and the caller's owner or reclaim token; any other directory is restored or left detached. --- src/lab/public/mutation-lock.ts | 106 +++++++++++----- structure/adapters/compatibility-lab.md | 7 ++ tests/lab/lab-community-mutation-lock.test.ts | 117 ++++++++++++++++++ 3 files changed, 196 insertions(+), 34 deletions(-) diff --git a/src/lab/public/mutation-lock.ts b/src/lab/public/mutation-lock.ts index 81261f97274..bdce0b321f1 100644 --- a/src/lab/public/mutation-lock.ts +++ b/src/lab/public/mutation-lock.ts @@ -159,6 +159,67 @@ function cleanupDetachedMutationLocks(lockPath: string): void { } } +type DetachedMutationLockOutcome = "removed" | "absent" | "restored" | "retained"; + +/** + * Move the canonical lock to a unique detached name, then decide on what was actually moved. + * + * A pathname rename cannot be conditioned on the directory a caller checked earlier: another + * process can replace the canonical lock between that check and the rename. Checking after the + * move, under a name only this process uses, binds removal to the checked directory. Anything + * else goes back to the canonical name when that name is still free; when it is taken it stays + * detached, where it is never removed by this call. + */ +function detachVerifiedMutationLock( + lockPath: string, + kind: "release" | "stale", + isExpected: (detachedPath: string) => boolean, +): DetachedMutationLockOutcome { + const detachedPath = join( + dirname(lockPath), + `.mutation-lock-${kind}-${process.pid}-${randomUUID()}`, + ); + try { + renameSync(lockPath, detachedPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return "absent"; + throw error; + } + let expected = false; + try { + expected = isExpected(detachedPath); + } catch { + // An unreadable or non-directory entry is not proof of ownership; treat it as foreign. + } + if (expected) { + rmSync(detachedPath, { recursive: true, force: true }); + return "removed"; + } + try { + lstatSync(lockPath); + return "retained"; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") return "retained"; + } + try { + renameSync(detachedPath, lockPath); + return "restored"; + } catch { + // The canonical name was taken again; the foreign directory stays detached and intact. + return "retained"; + } +} + +function detachedOwnerMatches( + detachedPath: string, + owner: MutationLockOwner, + expectedDirectory: MutationLockDirectoryIdentity, +): boolean { + if (!sameDirectoryIdentity(currentDirectoryIdentity(detachedPath), expectedDirectory)) return false; + const current = readMutationLockOwner(detachedPath); + return current?.pid === owner.pid && current.token === owner.token; +} + function publishMutationLockOwner( lockPath: string, owner: MutationLockOwner, @@ -288,22 +349,13 @@ function tryReclaimMutationLock(lockPath: string, nowMs: number): boolean { ) return false; if (!reclaimClaimStillOwned(lockPath, claim)) return false; - const quarantinePath = join( - dirname(lockPath), - `.mutation-lock-stale-${process.pid}-${randomUUID()}`, - ); - try { - // Rename the exact claimed directory away from the canonical pathname before - // deleting it. A successor can create a new lock immediately afterwards, but - // cleanup is confined to this unique quarantine path and cannot delete it. - renameSync(lockPath, quarantinePath); - moved = true; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return true; - throw error; - } - rmSync(quarantinePath, { recursive: true, force: true }); - return true; + // The claim file lives inside the stale directory, so finding this claim's token in the + // moved directory, with the identity captured before claiming, proves it is the one claimed. + const outcome = detachVerifiedMutationLock(lockPath, "stale", detachedPath => + sameDirectoryIdentity(currentDirectoryIdentity(detachedPath), staleDirectory) + && reclaimClaimStillOwned(detachedPath, claim)); + moved = outcome === "removed"; + return outcome === "removed" || outcome === "absent"; } finally { if (!moved) releaseReclaimClaim(lockPath, claim); } @@ -315,12 +367,8 @@ function discardUncommittedMutationLock( ): void { try { if (!sameDirectoryIdentity(currentDirectoryIdentity(lockPath), expectedDirectory)) return; - const quarantinePath = join( - dirname(lockPath), - `.mutation-lock-release-${process.pid}-${randomUUID()}`, - ); - renameSync(lockPath, quarantinePath); - rmSync(quarantinePath, { recursive: true, force: true }); + detachVerifiedMutationLock(lockPath, "release", detachedPath => + sameDirectoryIdentity(currentDirectoryIdentity(detachedPath), expectedDirectory)); } catch { // Preserve the owner-publication error. An unrecoverable cleanup witness stays // ownerless and can be reclaimed by the long incomplete-acquisition fallback. @@ -342,18 +390,8 @@ function releaseMutationLock( if (!sameDirectoryIdentity(currentDirectory, expectedDirectory)) return; const current = readMutationLockOwner(lockPath); if (!current || current.pid !== owner.pid || current.token !== owner.token) return; - - const quarantinePath = join( - dirname(lockPath), - `.mutation-lock-release-${process.pid}-${randomUUID()}`, - ); - try { - renameSync(lockPath, quarantinePath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return; - throw error; - } - rmSync(quarantinePath, { recursive: true, force: true }); + detachVerifiedMutationLock(lockPath, "release", detachedPath => + detachedOwnerMatches(detachedPath, owner, expectedDirectory)); } /** Serialize public-evidence mutations across CLI/server processes with ownership-safe stale recovery. */ diff --git a/structure/adapters/compatibility-lab.md b/structure/adapters/compatibility-lab.md index 72d374743fd..0c4da24595e 100644 --- a/structure/adapters/compatibility-lab.md +++ b/structure/adapters/compatibility-lab.md @@ -175,6 +175,13 @@ code `community_cache_busy` without running the protected work, and public-evidence validation failures stay 400. Rejection leaves the owner's lock bytes and directory identity untouched. +Release, stale reclaim and failed-acquisition cleanup first move the canonical lock directory to a +unique detached name and remove it only when the moved directory is the one they checked: the same +directory identity, plus the caller's owner token on release or reclaim token on reclaim. Any other +directory is moved back to the canonical name when that name is free, and otherwise stays detached +and intact. `tests/lab/lab-community-mutation-lock.test.ts` replaces the lock between each path's +checks and its move. + Sensitive purge removes a community cache pathname that durable local provenance marks as locally originated, even when the cached object is oversized, hardlinked, symlinked or otherwise unreadable as a community object. It unlinks the pathname only: it never follows a symlink and never removes a diff --git a/tests/lab/lab-community-mutation-lock.test.ts b/tests/lab/lab-community-mutation-lock.test.ts index 1d228bb3ca3..7b2bee20cf7 100644 --- a/tests/lab/lab-community-mutation-lock.test.ts +++ b/tests/lab/lab-community-mutation-lock.test.ts @@ -42,7 +42,124 @@ function createLiveOwnerLock(config: string): string { return lockPath; } +const SUCCESSOR_TOKEN = "11111111-1111-4111-8111-111111111111"; + +/** Put a different live owner's lock directory at the canonical path and return its identity. */ +function installSuccessorLock(lockPath: string): { dev: bigint; ino: bigint } { + fs.rmSync(lockPath, { recursive: true, force: true }); + mkdirSync(lockPath, { mode: 0o700 }); + writeFileSync(join(lockPath, "owner.json"), + JSON.stringify({ pid: process.pid, token: SUCCESSOR_TOKEN, createdAt: Date.now() }), + { encoding: "utf8", mode: 0o600 }); + const stat = lstatSync(lockPath, { bigint: true }); + return { dev: stat.dev, ino: stat.ino }; +} + +function expectSuccessorIntact(lockPath: string, identity: { dev: bigint; ino: bigint }): void { + const stat = lstatSync(lockPath, { bigint: true }); + expect([stat.dev, stat.ino]).toEqual([identity.dev, identity.ino]); + expect(JSON.parse(readFileSync(join(lockPath, "owner.json"), "utf8")).token).toBe(SUCCESSOR_TOKEN); +} + +/** + * Replace the canonical lock with a successor's directory immediately before the first rename + * that moves the canonical lock to a detached name matching `detached`: the state a paused + * process finds after its checks when another process replaced the lock in between. + */ +function replaceLockBeforeDetach(lockPath: string, detached: RegExp, afterMove?: () => void) { + const realRename = fs.renameSync; + const state: { successor: { dev: bigint; ino: bigint } | null } = { successor: null }; + const spy = spyOn(fs, "renameSync").mockImplementation(((from, to) => { + if (!state.successor && samePath(from, lockPath) && detached.test(String(to))) { + state.successor = installSuccessorLock(lockPath); + realRename(from, to); + afterMove?.(); + return; + } + return realRename(from, to); + }) as typeof fs.renameSync); + return { spy, state }; +} + +function detachedLockNames(config: string): string[] { + return fs.readdirSync(labCommunityDir(config)).filter(name => name.startsWith(".mutation-lock-")); +} + describe("community mutation lock", () => { + test("release leaves a successor lock that replaced the checked directory in place", () => { + const config = configDir(); + const lockPath = join(labCommunityDir(config), ".mutation-lock"); + const { spy, state } = replaceLockBeforeDetach(lockPath, /\.mutation-lock-release-/); + try { + withPublicEvidenceMutationLock(config, () => {}); + } finally { spy.mockRestore(); } + expect(state.successor).not.toBeNull(); + expectSuccessorIntact(lockPath, state.successor!); + expect(detachedLockNames(config)).toEqual([]); + }); + + test("stale reclaim leaves a successor lock that replaced the claimed directory in place", () => { + const config = configDir(); + const lockPath = join(labCommunityDir(config), ".mutation-lock"); + mkdirSync(lockPath, { mode: 0o700 }); + const stale = new Date(Date.now() - (25 * 60 * 60 * 1000)); + utimesSync(lockPath, stale, stale); + const { spy, state } = replaceLockBeforeDetach(lockPath, /\.mutation-lock-stale-/); + let reclaimed: boolean; + try { + reclaimed = publicEvidenceTryReclaimMutationLockForTests(config); + } finally { spy.mockRestore(); } + expect(reclaimed).toBe(false); + expectSuccessorIntact(lockPath, state.successor!); + expect(existsSync(join(lockPath, ".reclaim.json"))).toBe(false); + expect(detachedLockNames(config)).toEqual([]); + }); + + test("discarding a failed acquisition leaves a successor lock that replaced it in place", () => { + const config = configDir(); + const lockPath = join(labCommunityDir(config), ".mutation-lock"); + const realWrite = fs.writeFileSync; + const write = spyOn(fs, "writeFileSync").mockImplementation(((path, data, options) => { + if (/\.owner-[0-9a-f-]{36}\.tmp$/.test(String(path))) throw new Error("injected owner publication failure"); + return realWrite(path, data, options); + }) as typeof fs.writeFileSync); + const { spy, state } = replaceLockBeforeDetach(lockPath, /\.mutation-lock-release-/); + try { + expect(() => withPublicEvidenceMutationLock(config, () => {})).toThrow("injected owner publication failure"); + } finally { spy.mockRestore(); write.mockRestore(); } + expectSuccessorIntact(lockPath, state.successor!); + expect(detachedLockNames(config)).toEqual([]); + }); + + test("release never deletes a moved successor lock when the canonical path is taken again", () => { + const config = configDir(); + const lockPath = join(labCommunityDir(config), ".mutation-lock"); + const third = { identity: null as { dev: bigint; ino: bigint } | null }; + const { spy, state } = replaceLockBeforeDetach(lockPath, /\.mutation-lock-release-/, () => { + mkdirSync(lockPath, { mode: 0o700 }); + const stat = lstatSync(lockPath, { bigint: true }); + third.identity = { dev: stat.dev, ino: stat.ino }; + }); + try { + withPublicEvidenceMutationLock(config, () => {}); + } finally { spy.mockRestore(); } + const canonical = lstatSync(lockPath, { bigint: true }); + expect([canonical.dev, canonical.ino]).toEqual([third.identity!.dev, third.identity!.ino]); + const detached = detachedLockNames(config); + expect(detached).toHaveLength(1); + expectSuccessorIntact(join(labCommunityDir(config), detached[0]!), state.successor!); + }); + + test("release removes its own lock directory and leaves no detached copy", () => { + const config = configDir(); + const lockPath = join(labCommunityDir(config), ".mutation-lock"); + let held = false; + withPublicEvidenceMutationLock(config, () => { held = existsSync(join(lockPath, "owner.json")); }); + expect(held).toBe(true); + expect(existsSync(lockPath)).toBe(false); + expect(detachedLockNames(config)).toEqual([]); + }); + test("does not publish into a lock replaced with an adjacent NTFS-sized identity", () => { const config = configDir(); const lockPath = join(labCommunityDir(config), ".mutation-lock"); From 31f8bb63533db8c0676522c685ec716e73b90bfa Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Sat, 31 Oct 2026 11:06:11 +0900 Subject: [PATCH 4/8] fix: release the Lab lock without moving a directory it does not own Release and failed-acquisition cleanup move only their own owner record and remove the lock directory only while empty; detached cleanup skips a live owner; an acquisition whose fresh directory vanishes retries. --- src/lab/public/mutation-lock.ts | 118 +++++++++++++----- structure/adapters/compatibility-lab.md | 15 ++- tests/lab/lab-community-mutation-lock.test.ts | 103 ++++++++++++--- 3 files changed, 183 insertions(+), 53 deletions(-) diff --git a/src/lab/public/mutation-lock.ts b/src/lab/public/mutation-lock.ts index bdce0b321f1..9619475799b 100644 --- a/src/lab/public/mutation-lock.ts +++ b/src/lab/public/mutation-lock.ts @@ -4,6 +4,7 @@ import { mkdirSync, readdirSync, renameSync, + rmdirSync, rmSync, unlinkSync, writeFileSync, @@ -150,34 +151,89 @@ function unlinkIfPresent(path: string): void { function cleanupDetachedMutationLocks(lockPath: string): void { const dir = dirname(lockPath); + const nowMs = Date.now(); for (const name of readdirSync(dir)) { if (!DETACHED_MUTATION_LOCK_RE.test(name)) continue; - // Detached lock directories are no longer authoritative once a new canonical - // lock has been acquired. Removing only their UUID-scoped names prevents them - // from leaking storage or being mistaken for community cache objects. - rmSync(join(dir, name), { recursive: true, force: true }); + const detachedPath = join(dir, name); + // A detached directory that still carries a live owner was moved away from a running + // holder; it is not this process's to remove. Everything else is leftover from a + // completed release or reclaim, and removing only UUID-scoped names keeps it from + // leaking storage or being mistaken for community cache objects. + const owner = readMutationLockOwner(detachedPath); + if (owner && !pidDefinitelyDead(owner.pid) && !exceedsAbsoluteLockAge(owner.createdAt, nowMs)) continue; + rmSync(detachedPath, { recursive: true, force: true }); } } +/** + * Remove the canonical lock directory only while it is empty. + * + * Every committed lock holds an owner record and every reclaim holds a claim file, so an + * empty-only removal cannot take a directory another process still owns or is reclaiming. The + * one empty foreign state is an acquisition between mkdir and its first owner write, and that + * acquisition retries when its directory disappears. + */ +function removeEmptyMutationLock(lockPath: string): void { + try { + rmdirSync(lockPath); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === "ENOENT" || code === "ENOTEMPTY" || code === "EEXIST") return; + throw error; + } +} + +function releasedOwnerPath(lockPath: string, token: string): string { + return join(lockPath, `.owner-released-${token}.json`); +} + +/** + * Give up this owner's record without touching any other owner's. + * + * The record moves to a name only this owner uses before it is judged, so the decision is made + * on what actually moved. Another owner's record goes straight back. While a record is moved the + * directory is ownerless with a fresh mtime, which no stale reclaim accepts, so the canonical + * directory cannot be replaced in between. + */ +function relinquishOwnerRecord( + lockPath: string, + owner: MutationLockOwner, +): "relinquished" | "missing" | "foreign" { + const ownerPath = mutationLockOwnerPath(lockPath); + const releasedPath = releasedOwnerPath(lockPath, owner.token); + try { + renameSync(ownerPath, releasedPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return "missing"; + throw error; + } + const moved = readLockMetadata(releasedPath); + if (moved?.pid === owner.pid && moved.token === owner.token) { + unlinkIfPresent(releasedPath); + return "relinquished"; + } + renameSync(releasedPath, ownerPath); + return "foreign"; +} + type DetachedMutationLockOutcome = "removed" | "absent" | "restored" | "retained"; /** - * Move the canonical lock to a unique detached name, then decide on what was actually moved. + * Move a claimed stale lock to a unique detached name, then decide on what was actually moved. * - * A pathname rename cannot be conditioned on the directory a caller checked earlier: another - * process can replace the canonical lock between that check and the rename. Checking after the - * move, under a name only this process uses, binds removal to the checked directory. Anything - * else goes back to the canonical name when that name is still free; when it is taken it stays - * detached, where it is never removed by this call. + * The reclaim claim inside the stale directory keeps release, failed-acquisition cleanup and + * other reclaimers from removing it, so the canonical name still names the claimed directory. + * Checking again after the move, under a name only this process uses, keeps removal bound to + * that directory regardless. Anything else goes back to the canonical name when that name is + * free; otherwise it stays detached, and detached cleanup skips a directory with a live owner. */ function detachVerifiedMutationLock( lockPath: string, - kind: "release" | "stale", isExpected: (detachedPath: string) => boolean, ): DetachedMutationLockOutcome { const detachedPath = join( dirname(lockPath), - `.mutation-lock-${kind}-${process.pid}-${randomUUID()}`, + `.mutation-lock-stale-${process.pid}-${randomUUID()}`, ); try { renameSync(lockPath, detachedPath); @@ -210,16 +266,6 @@ function detachVerifiedMutationLock( } } -function detachedOwnerMatches( - detachedPath: string, - owner: MutationLockOwner, - expectedDirectory: MutationLockDirectoryIdentity, -): boolean { - if (!sameDirectoryIdentity(currentDirectoryIdentity(detachedPath), expectedDirectory)) return false; - const current = readMutationLockOwner(detachedPath); - return current?.pid === owner.pid && current.token === owner.token; -} - function publishMutationLockOwner( lockPath: string, owner: MutationLockOwner, @@ -326,6 +372,9 @@ function reclaimClaimStillOwned(lockPath: string, claim: MutationLockReclaim): b function releaseReclaimClaim(lockPath: string, claim: MutationLockReclaim): void { if (!reclaimClaimStillOwned(lockPath, claim)) return; unlinkIfPresent(mutationLockReclaimPath(lockPath)); + // A holder that released while this claim sat in its directory could not remove it + // (it was not empty). Finish that removal; a directory with an owner is never empty. + removeEmptyMutationLock(lockPath); } function tryReclaimMutationLock(lockPath: string, nowMs: number): boolean { @@ -351,7 +400,7 @@ function tryReclaimMutationLock(lockPath: string, nowMs: number): boolean { // The claim file lives inside the stale directory, so finding this claim's token in the // moved directory, with the identity captured before claiming, proves it is the one claimed. - const outcome = detachVerifiedMutationLock(lockPath, "stale", detachedPath => + const outcome = detachVerifiedMutationLock(lockPath, detachedPath => sameDirectoryIdentity(currentDirectoryIdentity(detachedPath), staleDirectory) && reclaimClaimStillOwned(detachedPath, claim)); moved = outcome === "removed"; @@ -364,11 +413,12 @@ function tryReclaimMutationLock(lockPath: string, nowMs: number): boolean { function discardUncommittedMutationLock( lockPath: string, expectedDirectory: MutationLockDirectoryIdentity, + candidate: MutationLockOwner, ): void { try { if (!sameDirectoryIdentity(currentDirectoryIdentity(lockPath), expectedDirectory)) return; - detachVerifiedMutationLock(lockPath, "release", detachedPath => - sameDirectoryIdentity(currentDirectoryIdentity(detachedPath), expectedDirectory)); + if (relinquishOwnerRecord(lockPath, candidate) === "foreign") return; + removeEmptyMutationLock(lockPath); } catch { // Preserve the owner-publication error. An unrecoverable cleanup witness stays // ownerless and can be reclaimed by the long incomplete-acquisition fallback. @@ -390,8 +440,8 @@ function releaseMutationLock( if (!sameDirectoryIdentity(currentDirectory, expectedDirectory)) return; const current = readMutationLockOwner(lockPath); if (!current || current.pid !== owner.pid || current.token !== owner.token) return; - detachVerifiedMutationLock(lockPath, "release", detachedPath => - detachedOwnerMatches(detachedPath, owner, expectedDirectory)); + if (relinquishOwnerRecord(lockPath, owner) !== "relinquished") return; + removeEmptyMutationLock(lockPath); } /** Serialize public-evidence mutations across CLI/server processes with ownership-safe stale recovery. */ @@ -403,6 +453,9 @@ export function withPublicEvidenceMutationLock( const lockPath = mutationLockPath(configDir); let owner: MutationLockOwner | null = null; let ownedDirectory: MutationLockDirectoryIdentity | null = null; + // Bounded: a fresh directory can vanish only when a concurrent release or reclaim removes + // it as empty in the instant before its first owner write. + let vanishedAcquisitions = 0; while (true) { try { @@ -418,16 +471,23 @@ export function withPublicEvidenceMutationLock( throw new PublicEvidenceValidationError("community_cache_busy", "community cache is busy"); } - const directory = currentDirectoryIdentity(lockPath); const candidate: MutationLockOwner = { pid: process.pid, token: randomUUID(), createdAt: Date.now(), }; + let directory: MutationLockDirectoryIdentity; + try { + directory = currentDirectoryIdentity(lockPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT" && vanishedAcquisitions++ < 3) continue; + throw error; + } try { publishMutationLockOwner(lockPath, candidate, directory); } catch (error) { - discardUncommittedMutationLock(lockPath, directory); + discardUncommittedMutationLock(lockPath, directory, candidate); + if ((error as NodeJS.ErrnoException).code === "ENOENT" && vanishedAcquisitions++ < 3) continue; throw error; } owner = candidate; diff --git a/structure/adapters/compatibility-lab.md b/structure/adapters/compatibility-lab.md index 0c4da24595e..888afc5af06 100644 --- a/structure/adapters/compatibility-lab.md +++ b/structure/adapters/compatibility-lab.md @@ -175,12 +175,15 @@ code `community_cache_busy` without running the protected work, and public-evidence validation failures stay 400. Rejection leaves the owner's lock bytes and directory identity untouched. -Release, stale reclaim and failed-acquisition cleanup first move the canonical lock directory to a -unique detached name and remove it only when the moved directory is the one they checked: the same -directory identity, plus the caller's owner token on release or reclaim token on reclaim. Any other -directory is moved back to the canonical name when that name is free, and otherwise stays detached -and intact. `tests/lab/lab-community-mutation-lock.test.ts` replaces the lock between each path's -checks and its move. +Release and failed-acquisition cleanup never move the canonical lock directory. The holder moves +its `owner.json` to a name only it uses, puts the record straight back if the moved token is not +its own, and otherwise removes the directory only while it is empty, so a directory holding +another owner's record or a reclaim claim stays. Stale reclaim still detaches the directory it +claimed, but removes it only when the moved directory has the claimed identity and its own claim +token; anything else goes back or stays detached. Detached cleanup skips a directory with a live +owner. An acquisition whose fresh directory is removed as empty before its first owner write +retries. `tests/lab/lab-community-mutation-lock.test.ts` replaces the lock between each path's +checks and its first move or removal. Sensitive purge removes a community cache pathname that durable local provenance marks as locally originated, even when the cached object is oversized, hardlinked, symlinked or otherwise unreadable diff --git a/tests/lab/lab-community-mutation-lock.test.ts b/tests/lab/lab-community-mutation-lock.test.ts index 7b2bee20cf7..54e136a0c30 100644 --- a/tests/lab/lab-community-mutation-lock.test.ts +++ b/tests/lab/lab-community-mutation-lock.test.ts @@ -63,14 +63,18 @@ function expectSuccessorIntact(lockPath: string, identity: { dev: bigint; ino: b /** * Replace the canonical lock with a successor's directory immediately before the first rename - * that moves the canonical lock to a detached name matching `detached`: the state a paused - * process finds after its checks when another process replaced the lock in between. + * that `matches`: the state a paused process finds after its checks when another process + * replaced the lock in between. `afterMove` runs right after that rename. */ -function replaceLockBeforeDetach(lockPath: string, detached: RegExp, afterMove?: () => void) { +function replaceLockBeforeRename( + lockPath: string, + matches: (from: string, to: string) => boolean, + afterMove?: () => void, +) { const realRename = fs.renameSync; const state: { successor: { dev: bigint; ino: bigint } | null } = { successor: null }; const spy = spyOn(fs, "renameSync").mockImplementation(((from, to) => { - if (!state.successor && samePath(from, lockPath) && detached.test(String(to))) { + if (!state.successor && matches(String(from), String(to))) { state.successor = installSuccessorLock(lockPath); realRename(from, to); afterMove?.(); @@ -81,20 +85,46 @@ function replaceLockBeforeDetach(lockPath: string, detached: RegExp, afterMove?: return { spy, state }; } +/** + * Replace the canonical lock with a successor's directory immediately before the first step + * that moves or removes the lock or its owner record, whichever protocol takes it. + */ +function replaceLockBeforeFirstMutation(lockPath: string) { + const realRename = fs.renameSync, realRmdir = fs.rmdirSync; + const state: { successor: { dev: bigint; ino: bigint } | null } = { successor: null }; + const before = (path: fs.PathLike) => { + if (!state.successor && (samePath(path, lockPath) || samePath(path, join(lockPath, "owner.json")))) { + state.successor = installSuccessorLock(lockPath); + } + }; + const rename = spyOn(fs, "renameSync").mockImplementation(((from, to) => { + before(from); return realRename(from, to); + }) as typeof fs.renameSync); + const rmdir = spyOn(fs, "rmdirSync").mockImplementation(((path, options) => { + before(path); return realRmdir(path, options); + }) as typeof fs.rmdirSync); + return { spy: { mockRestore: () => { rename.mockRestore(); rmdir.mockRestore(); } }, state }; +} + function detachedLockNames(config: string): string[] { return fs.readdirSync(labCommunityDir(config)).filter(name => name.startsWith(".mutation-lock-")); } +function lockEntries(lockPath: string): string[] { + return fs.readdirSync(lockPath).sort(); +} + describe("community mutation lock", () => { test("release leaves a successor lock that replaced the checked directory in place", () => { const config = configDir(); const lockPath = join(labCommunityDir(config), ".mutation-lock"); - const { spy, state } = replaceLockBeforeDetach(lockPath, /\.mutation-lock-release-/); + const { spy, state } = replaceLockBeforeFirstMutation(lockPath); try { withPublicEvidenceMutationLock(config, () => {}); } finally { spy.mockRestore(); } expect(state.successor).not.toBeNull(); expectSuccessorIntact(lockPath, state.successor!); + expect(lockEntries(lockPath)).toEqual(["owner.json"]); expect(detachedLockNames(config)).toEqual([]); }); @@ -104,14 +134,15 @@ describe("community mutation lock", () => { mkdirSync(lockPath, { mode: 0o700 }); const stale = new Date(Date.now() - (25 * 60 * 60 * 1000)); utimesSync(lockPath, stale, stale); - const { spy, state } = replaceLockBeforeDetach(lockPath, /\.mutation-lock-stale-/); + const { spy, state } = replaceLockBeforeRename(lockPath, (from, to) => + samePath(from, lockPath) && /\.mutation-lock-stale-/.test(to)); let reclaimed: boolean; try { reclaimed = publicEvidenceTryReclaimMutationLockForTests(config); } finally { spy.mockRestore(); } expect(reclaimed).toBe(false); expectSuccessorIntact(lockPath, state.successor!); - expect(existsSync(join(lockPath, ".reclaim.json"))).toBe(false); + expect(lockEntries(lockPath)).toEqual(["owner.json"]); expect(detachedLockNames(config)).toEqual([]); }); @@ -123,7 +154,7 @@ describe("community mutation lock", () => { if (/\.owner-[0-9a-f-]{36}\.tmp$/.test(String(path))) throw new Error("injected owner publication failure"); return realWrite(path, data, options); }) as typeof fs.writeFileSync); - const { spy, state } = replaceLockBeforeDetach(lockPath, /\.mutation-lock-release-/); + const { spy, state } = replaceLockBeforeFirstMutation(lockPath); try { expect(() => withPublicEvidenceMutationLock(config, () => {})).toThrow("injected owner publication failure"); } finally { spy.mockRestore(); write.mockRestore(); } @@ -131,25 +162,61 @@ describe("community mutation lock", () => { expect(detachedLockNames(config)).toEqual([]); }); - test("release never deletes a moved successor lock when the canonical path is taken again", () => { + test("a moved lock with a live owner survives a later acquisition's detached cleanup", () => { const config = configDir(); const lockPath = join(labCommunityDir(config), ".mutation-lock"); - const third = { identity: null as { dev: bigint; ino: bigint } | null }; - const { spy, state } = replaceLockBeforeDetach(lockPath, /\.mutation-lock-release-/, () => { - mkdirSync(lockPath, { mode: 0o700 }); - const stat = lstatSync(lockPath, { bigint: true }); - third.identity = { dev: stat.dev, ino: stat.ino }; - }); + mkdirSync(lockPath, { mode: 0o700 }); + const stale = new Date(Date.now() - (25 * 60 * 60 * 1000)); + utimesSync(lockPath, stale, stale); + // The successor is moved, then the canonical name is taken before it can be put back. + const { spy, state } = replaceLockBeforeRename(lockPath, (from, to) => + samePath(from, lockPath) && /\.mutation-lock-stale-/.test(to), () => mkdirSync(lockPath, { mode: 0o700 })); try { - withPublicEvidenceMutationLock(config, () => {}); + expect(publicEvidenceTryReclaimMutationLockForTests(config)).toBe(false); } finally { spy.mockRestore(); } - const canonical = lstatSync(lockPath, { bigint: true }); - expect([canonical.dev, canonical.ino]).toEqual([third.identity!.dev, third.identity!.ino]); const detached = detachedLockNames(config); expect(detached).toHaveLength(1); + fs.rmdirSync(lockPath); + let ran = false; + withPublicEvidenceMutationLock(config, () => { ran = true; }); + expect(ran).toBe(true); + expect(detachedLockNames(config)).toEqual(detached); expectSuccessorIntact(join(labCommunityDir(config), detached[0]!), state.successor!); }); + test("release keeps a lock directory that holds another process's reclaim claim", () => { + const config = configDir(); + const lockPath = join(labCommunityDir(config), ".mutation-lock"); + withPublicEvidenceMutationLock(config, () => { + writeFileSync(join(lockPath, ".reclaim.json"), + JSON.stringify({ pid: process.pid, token: SUCCESSOR_TOKEN, createdAt: Date.now() }), + { encoding: "utf8", mode: 0o600 }); + }); + expect(lockEntries(lockPath)).toEqual([".reclaim.json"]); + expect(detachedLockNames(config)).toEqual([]); + }); + + test("an acquisition whose fresh directory is removed before its owner write retries", () => { + const config = configDir(); + const lockPath = join(labCommunityDir(config), ".mutation-lock"); + const realWrite = fs.writeFileSync; + let removed = 0; + const write = spyOn(fs, "writeFileSync").mockImplementation(((path, data, options) => { + if (removed === 0 && /\.owner-[0-9a-f-]{36}\.tmp$/.test(String(path))) { + removed += 1; + fs.rmdirSync(lockPath); + } + return realWrite(path, data, options); + }) as typeof fs.writeFileSync); + let ran = false; + try { + withPublicEvidenceMutationLock(config, () => { ran = existsSync(join(lockPath, "owner.json")); }); + } finally { write.mockRestore(); } + expect(removed).toBe(1); + expect(ran).toBe(true); + expect(existsSync(lockPath)).toBe(false); + }); + test("release removes its own lock directory and leaves no detached copy", () => { const config = configDir(); const lockPath = join(labCommunityDir(config), ".mutation-lock"); From f07bba98df52fc6dd1caf6f7560940fce64ed6b0 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Sat, 31 Oct 2026 11:18:42 +0900 Subject: [PATCH 5/8] fix: serialize Lab public-evidence mutation with an OS-held SQLite lock Replace the directory lock and its stale-owner reclaim with a BEGIN IMMEDIATE transaction on lab/public-evidence-mutation.sqlite, released by the OS when the handle closes. Older builds' live directory locks are still honoured and never moved or removed. --- src/lab/public/mutation-lock.ts | 516 +++--------------- structure/adapters/compatibility-lab.md | 31 +- tests/lab/lab-community-mutation-lock.test.ts | 369 +++++-------- .../lab-public-coderabbit-regressions.test.ts | 16 +- 4 files changed, 232 insertions(+), 700 deletions(-) diff --git a/src/lab/public/mutation-lock.ts b/src/lab/public/mutation-lock.ts index 9619475799b..ec2583b1d21 100644 --- a/src/lab/public/mutation-lock.ts +++ b/src/lab/public/mutation-lock.ts @@ -1,27 +1,18 @@ -import { randomUUID } from "node:crypto"; -import { - lstatSync, - mkdirSync, - readdirSync, - renameSync, - rmdirSync, - rmSync, - unlinkSync, - writeFileSync, -} from "node:fs"; -import { dirname, join } from "node:path"; -import { sameFileIdentity, statTimeMs, type FileIdentity } from "../../lib/file-identity"; -import { ensureLabDirs, labCommunityDir } from "../paths"; +import { Database } from "bun:sqlite"; +import { chmodSync, lstatSync, readdirSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import { statTimeMs } from "../../lib/file-identity"; +import { ensureLabDirs, labCommunityDir, labRoot } from "../paths"; import { readPrivateRegularFile } from "./file-safety"; import { PublicEvidenceValidationError } from "./validate"; -const PUBLIC_EVIDENCE_MUTATION_LOCK_NAME = ".mutation-lock"; -const PUBLIC_EVIDENCE_MUTATION_LOCK_OWNER = "owner.json"; -const PUBLIC_EVIDENCE_MUTATION_LOCK_RECLAIM = ".reclaim.json"; -const PUBLIC_EVIDENCE_MUTATION_LOCK_INCOMPLETE_STALE_MS = 24 * 60 * 60 * 1000; -const PUBLIC_EVIDENCE_MUTATION_LOCK_ABSOLUTE_STALE_MS = 7 * 24 * 60 * 60 * 1000; +const PUBLIC_EVIDENCE_MUTATION_DB_NAME = "public-evidence-mutation.sqlite"; +const LEGACY_MUTATION_LOCK_NAME = ".mutation-lock"; +const LEGACY_MUTATION_LOCK_OWNER = "owner.json"; +const LEGACY_MUTATION_LOCK_INCOMPLETE_STALE_MS = 24 * 60 * 60 * 1000; +const LEGACY_MUTATION_LOCK_ABSOLUTE_STALE_MS = 7 * 24 * 60 * 60 * 1000; const DETACHED_MUTATION_LOCK_RE = /^\.mutation-lock-(?:stale|release)-\d+-[0-9a-f-]{36}$/; -const MUTATION_LOCK_META_FILE_OPTIONS = { +const LEGACY_OWNER_FILE_OPTIONS = { maxBytes: 1024, errorCode: "community_cache_lock", errorMessage: "community cache mutation lock metadata is unsafe", @@ -30,30 +21,17 @@ const MUTATION_LOCK_META_FILE_OPTIONS = { requireMode600: true, } as const; -type MutationLockOwner = { +type LegacyMutationLockOwner = { pid: number; - token: string; createdAt: number; }; -type MutationLockReclaim = { - pid: number; - token: string; - createdAt: number; -}; - -type MutationLockDirectoryIdentity = FileIdentity; - -function mutationLockPath(configDir?: string): string { - return join(labCommunityDir(configDir), PUBLIC_EVIDENCE_MUTATION_LOCK_NAME); -} - -function mutationLockOwnerPath(lockPath: string): string { - return join(lockPath, PUBLIC_EVIDENCE_MUTATION_LOCK_OWNER); +function mutationLockDatabasePath(configDir?: string): string { + return join(labRoot(configDir), PUBLIC_EVIDENCE_MUTATION_DB_NAME); } -function mutationLockReclaimPath(lockPath: string): string { - return join(lockPath, PUBLIC_EVIDENCE_MUTATION_LOCK_RECLAIM); +function legacyMutationLockPath(configDir?: string): string { + return join(labCommunityDir(configDir), LEGACY_MUTATION_LOCK_NAME); } function pidDefinitelyDead(pid: number): boolean { @@ -65,45 +43,20 @@ function pidDefinitelyDead(pid: number): boolean { } } -function exceedsAbsoluteLockAge(createdAt: number, nowMs: number): boolean { - return nowMs - createdAt > PUBLIC_EVIDENCE_MUTATION_LOCK_ABSOLUTE_STALE_MS; -} - -function readLockMetadata( - path: string, -): T | null { +function readLegacyMutationLockOwner(lockPath: string): LegacyMutationLockOwner | null { try { - const bytes = readPrivateRegularFile(path, MUTATION_LOCK_META_FILE_OPTIONS); - const raw = JSON.parse(bytes.toString("utf8")) as Partial; + const bytes = readPrivateRegularFile(join(lockPath, LEGACY_MUTATION_LOCK_OWNER), LEGACY_OWNER_FILE_OPTIONS); + const raw = JSON.parse(bytes.toString("utf8")) as Partial; if ( - Number.isSafeInteger(raw.pid) - && Number(raw.pid) > 0 - && typeof raw.token === "string" - && /^[0-9a-f-]{36}$/.test(raw.token) - && Number.isSafeInteger(raw.createdAt) - && Number(raw.createdAt) > 0 - ) { - return { - pid: Number(raw.pid), - token: raw.token, - createdAt: Number(raw.createdAt), - } as T; - } + Number.isSafeInteger(raw.pid) && Number(raw.pid) > 0 + && Number.isSafeInteger(raw.createdAt) && Number(raw.createdAt) > 0 + ) return { pid: Number(raw.pid), createdAt: Number(raw.createdAt) }; } catch { - // Incomplete metadata is handled conservatively by the age fallback where - // applicable. Normal acquisition never trusts malformed metadata. + // Missing or malformed metadata falls back to the directory-age rule below. } return null; } -function readMutationLockOwner(lockPath: string): MutationLockOwner | null { - return readLockMetadata(mutationLockOwnerPath(lockPath)); -} - -function readMutationLockReclaim(lockPath: string): MutationLockReclaim | null { - return readLockMetadata(mutationLockReclaimPath(lockPath)); -} - function assertMutationLockDirectory(lockPath: string) { const stat = lstatSync(lockPath, { bigint: true }); if (!stat.isDirectory() || stat.isSymbolicLink()) { @@ -115,406 +68,103 @@ function assertMutationLockDirectory(lockPath: string) { return stat; } -function sameDirectoryIdentity( - left: MutationLockDirectoryIdentity, - right: MutationLockDirectoryIdentity, -): boolean { - return sameFileIdentity(left, right); -} - -function currentDirectoryIdentity(lockPath: string): MutationLockDirectoryIdentity { - const stat = assertMutationLockDirectory(lockPath); - return { dev: stat.dev, ino: stat.ino }; -} - -function mutationLockIsReclaimable(lockPath: string, nowMs: number): boolean { - const stat = assertMutationLockDirectory(lockPath); - const owner = readMutationLockOwner(lockPath); - if (owner) { - // A live PID is strong evidence only while the recorded ownership generation is - // reasonably recent. The absolute ceiling recovers from PID reuse after a crash. - return pidDefinitelyDead(owner.pid) || exceedsAbsoluteLockAge(owner.createdAt, nowMs); - } - // The only ownerless state is the tiny mkdir-to-owner publication window. Use - // a deliberately long fallback so a crashed acquisition can eventually heal - // without treating an ordinary pause as proof that the owner disappeared. - return nowMs - statTimeMs(stat.mtimeNs) > PUBLIC_EVIDENCE_MUTATION_LOCK_INCOMPLETE_STALE_MS; -} - -function unlinkIfPresent(path: string): void { - try { - unlinkSync(path); - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; - } -} - -function cleanupDetachedMutationLocks(lockPath: string): void { - const dir = dirname(lockPath); - const nowMs = Date.now(); - for (const name of readdirSync(dir)) { - if (!DETACHED_MUTATION_LOCK_RE.test(name)) continue; - const detachedPath = join(dir, name); - // A detached directory that still carries a live owner was moved away from a running - // holder; it is not this process's to remove. Everything else is leftover from a - // completed release or reclaim, and removing only UUID-scoped names keeps it from - // leaking storage or being mistaken for community cache objects. - const owner = readMutationLockOwner(detachedPath); - if (owner && !pidDefinitelyDead(owner.pid) && !exceedsAbsoluteLockAge(owner.createdAt, nowMs)) continue; - rmSync(detachedPath, { recursive: true, force: true }); - } -} - -/** - * Remove the canonical lock directory only while it is empty. - * - * Every committed lock holds an owner record and every reclaim holds a claim file, so an - * empty-only removal cannot take a directory another process still owns or is reclaiming. The - * one empty foreign state is an acquisition between mkdir and its first owner write, and that - * acquisition retries when its directory disappears. - */ -function removeEmptyMutationLock(lockPath: string): void { - try { - rmdirSync(lockPath); - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code === "ENOENT" || code === "ENOTEMPTY" || code === "EEXIST") return; - throw error; - } -} - -function releasedOwnerPath(lockPath: string, token: string): string { - return join(lockPath, `.owner-released-${token}.json`); -} - /** - * Give up this owner's record without touching any other owner's. + * Whether a directory lock left by an older OpenCodex build still has a holder. * - * The record moves to a name only this owner uses before it is judged, so the decision is made - * on what actually moved. Another owner's record goes straight back. While a record is moved the - * directory is ownerless with a fresh mtime, which no stale reclaim accepts, so the canonical - * directory cannot be replaced in between. + * Older builds serialized through a `.mutation-lock` directory. This build never creates, + * renames or removes one; it only declines to run beside a holder that still looks alive, using + * the same evidence the older builds used: a live owner PID within the absolute age ceiling, or + * an ownerless directory recent enough to be an acquisition in progress. */ -function relinquishOwnerRecord( - lockPath: string, - owner: MutationLockOwner, -): "relinquished" | "missing" | "foreign" { - const ownerPath = mutationLockOwnerPath(lockPath); - const releasedPath = releasedOwnerPath(lockPath, owner.token); +function legacyMutationLockBlocks(lockPath: string, nowMs: number): boolean { + let stat: ReturnType; try { - renameSync(ownerPath, releasedPath); + stat = assertMutationLockDirectory(lockPath); } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return "missing"; + if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; throw error; } - const moved = readLockMetadata(releasedPath); - if (moved?.pid === owner.pid && moved.token === owner.token) { - unlinkIfPresent(releasedPath); - return "relinquished"; + const owner = readLegacyMutationLockOwner(lockPath); + if (owner) { + return !pidDefinitelyDead(owner.pid) && nowMs - owner.createdAt <= LEGACY_MUTATION_LOCK_ABSOLUTE_STALE_MS; } - renameSync(releasedPath, ownerPath); - return "foreign"; + return nowMs - statTimeMs(stat.mtimeNs) <= LEGACY_MUTATION_LOCK_INCOMPLETE_STALE_MS; } -type DetachedMutationLockOutcome = "removed" | "absent" | "restored" | "retained"; - -/** - * Move a claimed stale lock to a unique detached name, then decide on what was actually moved. - * - * The reclaim claim inside the stale directory keeps release, failed-acquisition cleanup and - * other reclaimers from removing it, so the canonical name still names the claimed directory. - * Checking again after the move, under a name only this process uses, keeps removal bound to - * that directory regardless. Anything else goes back to the canonical name when that name is - * free; otherwise it stays detached, and detached cleanup skips a directory with a live owner. - */ -function detachVerifiedMutationLock( - lockPath: string, - isExpected: (detachedPath: string) => boolean, -): DetachedMutationLockOutcome { - const detachedPath = join( - dirname(lockPath), - `.mutation-lock-stale-${process.pid}-${randomUUID()}`, - ); - try { - renameSync(lockPath, detachedPath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return "absent"; - throw error; - } - let expected = false; - try { - expected = isExpected(detachedPath); - } catch { - // An unreadable or non-directory entry is not proof of ownership; treat it as foreign. - } - if (expected) { - rmSync(detachedPath, { recursive: true, force: true }); - return "removed"; - } - try { - lstatSync(lockPath); - return "retained"; - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "ENOENT") return "retained"; - } - try { - renameSync(detachedPath, lockPath); - return "restored"; - } catch { - // The canonical name was taken again; the foreign directory stays detached and intact. - return "retained"; +function cleanupDetachedMutationLocks(configDir?: string): void { + const dir = labCommunityDir(configDir); + for (const name of readdirSync(dir)) { + if (!DETACHED_MUTATION_LOCK_RE.test(name)) continue; + // Older builds moved a released or reclaimed lock directory to one of these UUID-scoped + // names before deleting it. Removing leftovers keeps them from leaking storage or being + // mistaken for community cache objects. + rmSync(join(dir, name), { recursive: true, force: true }); } } -function publishMutationLockOwner( - lockPath: string, - owner: MutationLockOwner, - expectedDirectory: MutationLockDirectoryIdentity, -): void { - // Write through a unique temporary pathname first. If an ancient ownerless - // lock is reclaimed while this process was suspended, the inode check prevents - // this acquisition from publishing its owner metadata into a replacement lock. - const tempPath = join(lockPath, `.owner-${owner.token}.tmp`); +function openMutationLockDatabase(configDir?: string): Database { + const path = mutationLockDatabasePath(configDir); try { - writeFileSync(tempPath, JSON.stringify(owner), { - encoding: "utf8", - flag: "wx", - mode: 0o600, - }); - if (!sameDirectoryIdentity(currentDirectoryIdentity(lockPath), expectedDirectory)) { - throw new PublicEvidenceValidationError( - "community_cache_lock", - "community cache mutation lock changed during owner publication", - ); - } - renameSync(tempPath, mutationLockOwnerPath(lockPath)); - if (!sameDirectoryIdentity(currentDirectoryIdentity(lockPath), expectedDirectory)) { - throw new PublicEvidenceValidationError( - "community_cache_lock", - "community cache mutation lock changed after owner publication", - ); + const stat = lstatSync(path); + if (!stat.isFile() || stat.isSymbolicLink() || stat.nlink !== 1) { + throw new PublicEvidenceValidationError("community_cache_lock", "community cache mutation lock is unsafe"); } - const persisted = readMutationLockOwner(lockPath); - if (!persisted || persisted.pid !== owner.pid || persisted.token !== owner.token) { - throw new PublicEvidenceValidationError( - "community_cache_lock", - "community cache mutation lock owner publication was not durable", - ); - } - } finally { - // The rename normally makes this ENOENT. If the lock pathname was replaced, - // the UUID-scoped temporary name can be removed without touching successor state. - unlinkIfPresent(tempPath); - } -} - -function reclaimClaimIsRecoverable(lockPath: string, nowMs: number): boolean { - const claimPath = mutationLockReclaimPath(lockPath); - const claim = readMutationLockReclaim(lockPath); - if (claim) { - return pidDefinitelyDead(claim.pid) || exceedsAbsoluteLockAge(claim.createdAt, nowMs); - } - try { - const stat = lstatSync(claimPath); - return nowMs - stat.mtimeMs > PUBLIC_EVIDENCE_MUTATION_LOCK_INCOMPLETE_STALE_MS; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return true; - throw error; - } -} - -function recoverStaleReclaimClaim(lockPath: string, nowMs: number): boolean { - if (!reclaimClaimIsRecoverable(lockPath, nowMs)) return false; - const claimPath = mutationLockReclaimPath(lockPath); - const quarantinePath = join(lockPath, `.reclaim-stale-${randomUUID()}.json`); - try { - renameSync(claimPath, quarantinePath); } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return true; - throw error; - } - unlinkIfPresent(quarantinePath); - return true; -} - -function tryAcquireReclaimClaim( - lockPath: string, - nowMs: number, -): MutationLockReclaim | null { - for (let attempt = 0; attempt < 2; attempt += 1) { - const claim: MutationLockReclaim = { - pid: process.pid, - token: randomUUID(), - createdAt: nowMs, - }; - try { - writeFileSync(mutationLockReclaimPath(lockPath), JSON.stringify(claim), { - encoding: "utf8", - flag: "wx", - mode: 0o600, - }); - return claim; - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code === "ENOENT") return null; - if (code !== "EEXIST") throw error; - if (!recoverStaleReclaimClaim(lockPath, nowMs)) return null; - } - } - return null; -} - -function reclaimClaimStillOwned(lockPath: string, claim: MutationLockReclaim): boolean { - const current = readMutationLockReclaim(lockPath); - return current?.pid === claim.pid && current.token === claim.token; -} - -function releaseReclaimClaim(lockPath: string, claim: MutationLockReclaim): void { - if (!reclaimClaimStillOwned(lockPath, claim)) return; - unlinkIfPresent(mutationLockReclaimPath(lockPath)); - // A holder that released while this claim sat in its directory could not remove it - // (it was not empty). Finish that removal; a directory with an owner is never empty. - removeEmptyMutationLock(lockPath); -} - -function tryReclaimMutationLock(lockPath: string, nowMs: number): boolean { - // Capture the exact stale directory before the claim write changes its mtime. - // After claiming, revalidate the inode and owner instead of reusing an age check - // that our own .reclaim.json creation would make appear fresh. - if (!mutationLockIsReclaimable(lockPath, nowMs)) return false; - const staleDirectory = currentDirectoryIdentity(lockPath); - const claim = tryAcquireReclaimClaim(lockPath, nowMs); - if (!claim) return false; - - let moved = false; - try { - if (!reclaimClaimStillOwned(lockPath, claim)) return false; - if (!sameDirectoryIdentity(currentDirectoryIdentity(lockPath), staleDirectory)) return false; - const currentOwner = readMutationLockOwner(lockPath); - if ( - currentOwner - && !pidDefinitelyDead(currentOwner.pid) - && !exceedsAbsoluteLockAge(currentOwner.createdAt, nowMs) - ) return false; - if (!reclaimClaimStillOwned(lockPath, claim)) return false; - - // The claim file lives inside the stale directory, so finding this claim's token in the - // moved directory, with the identity captured before claiming, proves it is the one claimed. - const outcome = detachVerifiedMutationLock(lockPath, detachedPath => - sameDirectoryIdentity(currentDirectoryIdentity(detachedPath), staleDirectory) - && reclaimClaimStillOwned(detachedPath, claim)); - moved = outcome === "removed"; - return outcome === "removed" || outcome === "absent"; - } finally { - if (!moved) releaseReclaimClaim(lockPath, claim); + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; } -} - -function discardUncommittedMutationLock( - lockPath: string, - expectedDirectory: MutationLockDirectoryIdentity, - candidate: MutationLockOwner, -): void { try { - if (!sameDirectoryIdentity(currentDirectoryIdentity(lockPath), expectedDirectory)) return; - if (relinquishOwnerRecord(lockPath, candidate) === "foreign") return; - removeEmptyMutationLock(lockPath); + const database = new Database(path, { create: true }); + try { chmodSync(path, 0o600); } catch { /* the platform may ignore chmod */ } + return database; } catch { - // Preserve the owner-publication error. An unrecoverable cleanup witness stays - // ownerless and can be reclaimed by the long incomplete-acquisition fallback. - } -} - -function releaseMutationLock( - lockPath: string, - owner: MutationLockOwner, - expectedDirectory: MutationLockDirectoryIdentity, -): void { - let currentDirectory: MutationLockDirectoryIdentity; - try { - currentDirectory = currentDirectoryIdentity(lockPath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return; - throw error; + // The SQLite error text can name local paths; the stable code is what callers act on. + throw new PublicEvidenceValidationError("community_cache_lock", "community cache mutation lock is unavailable"); } - if (!sameDirectoryIdentity(currentDirectory, expectedDirectory)) return; - const current = readMutationLockOwner(lockPath); - if (!current || current.pid !== owner.pid || current.token !== owner.token) return; - if (relinquishOwnerRecord(lockPath, owner) !== "relinquished") return; - removeEmptyMutationLock(lockPath); } -/** Serialize public-evidence mutations across CLI/server processes with ownership-safe stale recovery. */ +/** + * Serialize public-evidence mutations across CLI/server processes. + * + * The lock is an OS-held SQLite write transaction on a dedicated database, with + * `busy_timeout=0` so a second holder fails fast as `community_cache_busy`. Closing the handle, + * including at process exit or crash, releases it, so there is no stale-owner recovery and no + * step that renames or deletes another process's lock state. Never return a Promise from `run`. + */ export function withPublicEvidenceMutationLock( configDir: string | undefined, run: () => T, ): T { ensureLabDirs(configDir); - const lockPath = mutationLockPath(configDir); - let owner: MutationLockOwner | null = null; - let ownedDirectory: MutationLockDirectoryIdentity | null = null; - // Bounded: a fresh directory can vanish only when a concurrent release or reclaim removes - // it as empty in the instant before its first owner write. - let vanishedAcquisitions = 0; - - while (true) { + const database = openMutationLockDatabase(configDir); + let transactionOpen = false; + try { try { - mkdirSync(lockPath, { mode: 0o700 }); - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; - try { - if (tryReclaimMutationLock(lockPath, Date.now())) continue; - } catch (reclaimError) { - if ((reclaimError as NodeJS.ErrnoException).code === "ENOENT") continue; - throw reclaimError; + database.exec("PRAGMA busy_timeout = 0; BEGIN IMMEDIATE"); + transactionOpen = true; + } catch (cause) { + const code = cause && typeof cause === "object" && "code" in cause ? String((cause as { code?: unknown }).code) : ""; + if (code.startsWith("SQLITE_BUSY") || code.startsWith("SQLITE_LOCKED")) { + throw new PublicEvidenceValidationError("community_cache_busy", "community cache is busy"); } - throw new PublicEvidenceValidationError("community_cache_busy", "community cache is busy"); - } - - const candidate: MutationLockOwner = { - pid: process.pid, - token: randomUUID(), - createdAt: Date.now(), - }; - let directory: MutationLockDirectoryIdentity; - try { - directory = currentDirectoryIdentity(lockPath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT" && vanishedAcquisitions++ < 3) continue; - throw error; + throw new PublicEvidenceValidationError("community_cache_lock", "community cache mutation lock is unavailable"); } - try { - publishMutationLockOwner(lockPath, candidate, directory); - } catch (error) { - discardUncommittedMutationLock(lockPath, directory, candidate); - if ((error as NodeJS.ErrnoException).code === "ENOENT" && vanishedAcquisitions++ < 3) continue; - throw error; + if (legacyMutationLockBlocks(legacyMutationLockPath(configDir), Date.now())) { + throw new PublicEvidenceValidationError("community_cache_busy", "community cache is busy"); } - owner = candidate; - ownedDirectory = directory; - break; - } - - try { - cleanupDetachedMutationLocks(lockPath); + cleanupDetachedMutationLocks(configDir); return run(); } finally { - if (owner && ownedDirectory) releaseMutationLock(lockPath, owner, ownedDirectory); + if (transactionOpen) { + try { database.exec("ROLLBACK"); } catch { /* closing the handle below still releases the OS lock */ } + } + try { database.close(); } catch { /* the OS lock is released with the handle */ } } } -/** Test-only seam for stale-owner policy. This module is not barrel-exported. */ -export function publicEvidenceMutationLockIsReclaimableForTests( - configDir: string | undefined, - nowMs = Date.now(), -): boolean { - return mutationLockIsReclaimable(mutationLockPath(configDir), nowMs); -} - -/** Test-only seam for the exclusive stale-reclaimer claim. */ -export function publicEvidenceTryReclaimMutationLockForTests( +/** Test-only seam for the older-build lock policy. This module is not barrel-exported. */ +export function publicEvidenceLegacyMutationLockBlocksForTests( configDir: string | undefined, nowMs = Date.now(), ): boolean { - return tryReclaimMutationLock(mutationLockPath(configDir), nowMs); + return legacyMutationLockBlocks(legacyMutationLockPath(configDir), nowMs); } diff --git a/structure/adapters/compatibility-lab.md b/structure/adapters/compatibility-lab.md index 888afc5af06..558e3f3d638 100644 --- a/structure/adapters/compatibility-lab.md +++ b/structure/adapters/compatibility-lab.md @@ -168,22 +168,23 @@ CL-05 adds a read-only Models tab (`#models/compatibility`) that visualizes the ## Public-evidence mutation, purge and revocation -Public-evidence mutation is serialized across processes by `src/lab/public/mutation-lock.ts`. A live, -non-reclaimable owner is a fail-fast condition: the caller receives `PublicEvidenceValidationError` -code `community_cache_busy` without running the protected work, and +Public-evidence mutation is serialized across processes by `src/lab/public/mutation-lock.ts`: an +OS-held SQLite write transaction (`BEGIN IMMEDIATE`, `busy_timeout=0`) on +`lab/public-evidence-mutation.sqlite`, outside the community cache, the same primitive as the +config mutation lock. A second holder is a fail-fast condition: the caller receives +`PublicEvidenceValidationError` code `community_cache_busy` without running the protected work, and `src/server/management/lab-routes.ts` maps that code to HTTP 503 with `Retry-After: 1`. Other -public-evidence validation failures stay 400. Rejection leaves the owner's lock bytes and directory -identity untouched. - -Release and failed-acquisition cleanup never move the canonical lock directory. The holder moves -its `owner.json` to a name only it uses, puts the record straight back if the moved token is not -its own, and otherwise removes the directory only while it is empty, so a directory holding -another owner's record or a reclaim claim stays. Stale reclaim still detaches the directory it -claimed, but removes it only when the moved directory has the claimed identity and its own claim -token; anything else goes back or stays detached. Detached cleanup skips a directory with a live -owner. An acquisition whose fresh directory is removed as empty before its first owner write -retries. `tests/lab/lab-community-mutation-lock.test.ts` replaces the lock between each path's -checks and its first move or removal. +public-evidence validation failures stay 400. Closing the handle, including at process exit or +crash, releases the lock, so there is no stale-owner recovery and no step renames or removes another +process's lock state. A lock path that is not a single-link regular file fails closed. + +Older builds serialized through a `lab/community/.mutation-lock` directory. While holding the +transaction, this build still refuses to run beside such a directory whose owner PID is alive +within the seven-day ceiling, or which is ownerless and less than a day old, and fails closed when +that path is not a directory. It never creates, moves or removes that directory. It only removes +the older builds' UUID-named detached copies. `tests/lab/lab-community-mutation-lock.test.ts` +covers cross-process refusal, release on crash, and that acquire and release never rename or +remove the canonical lock path. Sensitive purge removes a community cache pathname that durable local provenance marks as locally originated, even when the cached object is oversized, hardlinked, symlinked or otherwise unreadable diff --git a/tests/lab/lab-community-mutation-lock.test.ts b/tests/lab/lab-community-mutation-lock.test.ts index 54e136a0c30..d97e838a9e8 100644 --- a/tests/lab/lab-community-mutation-lock.test.ts +++ b/tests/lab/lab-community-mutation-lock.test.ts @@ -1,18 +1,18 @@ -import { samePath } from "../helpers/same-path"; import { afterEach, describe, expect, spyOn, test } from "bun:test"; import * as fs from "node:fs"; -import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, utimesSync, writeFileSync } from "node:fs"; +import { existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { ensureLabDirs, labCommunityDir } from "../../src/lab/paths"; +import { ensureLabDirs, labCommunityDir, labRoot } from "../../src/lab/paths"; import { listCommunityEvidence } from "../../src/lab/public/community"; import { - publicEvidenceMutationLockIsReclaimableForTests, - publicEvidenceTryReclaimMutationLockForTests, + publicEvidenceLegacyMutationLockBlocksForTests, withPublicEvidenceMutationLock, } from "../../src/lab/public/mutation-lock"; import { PublicEvidenceValidationError } from "../../src/lab/public/validate"; import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { repoPath } from "../helpers/repo-root"; +import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; const roots: string[] = []; @@ -27,299 +27,178 @@ function configDir(): string { return root; } -function createLiveOwnerLock(config: string): string { - const lockPath = join(labCommunityDir(config), ".mutation-lock"); +function legacyLockPath(config: string): string { + return join(labCommunityDir(config), ".mutation-lock"); +} + +function createLegacyOwnerLock(config: string, createdAt = Date.now(), pid = process.pid): string { + const lockPath = legacyLockPath(config); mkdirSync(lockPath, { mode: 0o700 }); writeFileSync( join(lockPath, "owner.json"), - JSON.stringify({ - pid: process.pid, - token: "00000000-0000-4000-8000-000000000000", - createdAt: Date.now(), - }), + JSON.stringify({ pid, token: "00000000-0000-4000-8000-000000000000", createdAt }), { encoding: "utf8", mode: 0o600 }, ); return lockPath; } -const SUCCESSOR_TOKEN = "11111111-1111-4111-8111-111111111111"; - -/** Put a different live owner's lock directory at the canonical path and return its identity. */ -function installSuccessorLock(lockPath: string): { dev: bigint; ino: bigint } { - fs.rmSync(lockPath, { recursive: true, force: true }); - mkdirSync(lockPath, { mode: 0o700 }); - writeFileSync(join(lockPath, "owner.json"), - JSON.stringify({ pid: process.pid, token: SUCCESSOR_TOKEN, createdAt: Date.now() }), - { encoding: "utf8", mode: 0o600 }); - const stat = lstatSync(lockPath, { bigint: true }); - return { dev: stat.dev, ino: stat.ino }; -} - -function expectSuccessorIntact(lockPath: string, identity: { dev: bigint; ino: bigint }): void { - const stat = lstatSync(lockPath, { bigint: true }); - expect([stat.dev, stat.ino]).toEqual([identity.dev, identity.ino]); - expect(JSON.parse(readFileSync(join(lockPath, "owner.json"), "utf8")).token).toBe(SUCCESSOR_TOKEN); -} - -/** - * Replace the canonical lock with a successor's directory immediately before the first rename - * that `matches`: the state a paused process finds after its checks when another process - * replaced the lock in between. `afterMove` runs right after that rename. - */ -function replaceLockBeforeRename( - lockPath: string, - matches: (from: string, to: string) => boolean, - afterMove?: () => void, -) { - const realRename = fs.renameSync; - const state: { successor: { dev: bigint; ino: bigint } | null } = { successor: null }; - const spy = spyOn(fs, "renameSync").mockImplementation(((from, to) => { - if (!state.successor && matches(String(from), String(to))) { - state.successor = installSuccessorLock(lockPath); - realRename(from, to); - afterMove?.(); - return; - } - return realRename(from, to); - }) as typeof fs.renameSync); - return { spy, state }; -} - -/** - * Replace the canonical lock with a successor's directory immediately before the first step - * that moves or removes the lock or its owner record, whichever protocol takes it. - */ -function replaceLockBeforeFirstMutation(lockPath: string) { - const realRename = fs.renameSync, realRmdir = fs.rmdirSync; - const state: { successor: { dev: bigint; ino: bigint } | null } = { successor: null }; - const before = (path: fs.PathLike) => { - if (!state.successor && (samePath(path, lockPath) || samePath(path, join(lockPath, "owner.json")))) { - state.successor = installSuccessorLock(lockPath); - } - }; - const rename = spyOn(fs, "renameSync").mockImplementation(((from, to) => { - before(from); return realRename(from, to); - }) as typeof fs.renameSync); - const rmdir = spyOn(fs, "rmdirSync").mockImplementation(((path, options) => { - before(path); return realRmdir(path, options); - }) as typeof fs.rmdirSync); - return { spy: { mockRestore: () => { rename.mockRestore(); rmdir.mockRestore(); } }, state }; -} - -function detachedLockNames(config: string): string[] { - return fs.readdirSync(labCommunityDir(config)).filter(name => name.startsWith(".mutation-lock-")); -} - -function lockEntries(lockPath: string): string[] { - return fs.readdirSync(lockPath).sort(); +function busyFailure(run: () => unknown): PublicEvidenceValidationError { + let failure: unknown; + try { run(); } catch (error) { failure = error; } + expect(failure).toBeInstanceOf(PublicEvidenceValidationError); + expect((failure as PublicEvidenceValidationError).code).toBe("community_cache_busy"); + return failure as PublicEvidenceValidationError; } describe("community mutation lock", () => { - test("release leaves a successor lock that replaced the checked directory in place", () => { + test("acquire and release never rename or remove the canonical lock path or its contents", () => { const config = configDir(); - const lockPath = join(labCommunityDir(config), ".mutation-lock"); - const { spy, state } = replaceLockBeforeFirstMutation(lockPath); + const lockPath = legacyLockPath(config); + const touched: string[] = []; + const watch = (name: string, path: fs.PathLike) => { + const text = String(path); + if (text === lockPath || text.startsWith(lockPath + "\\") || text.startsWith(lockPath + "/")) touched.push(`${name} ${text}`); + }; + const realRename = fs.renameSync, realRm = fs.rmSync, realRmdir = fs.rmdirSync, realUnlink = fs.unlinkSync; + const spies = [ + spyOn(fs, "renameSync").mockImplementation(((from, to) => { watch("rename", from); return realRename(from, to); }) as typeof fs.renameSync), + spyOn(fs, "rmSync").mockImplementation(((path, options) => { watch("rm", path); return realRm(path, options); }) as typeof fs.rmSync), + spyOn(fs, "rmdirSync").mockImplementation(((path, options) => { watch("rmdir", path); return realRmdir(path, options); }) as typeof fs.rmdirSync), + spyOn(fs, "unlinkSync").mockImplementation(((path) => { watch("unlink", path); return realUnlink(path); }) as typeof fs.unlinkSync), + ]; + let ran = false; try { + withPublicEvidenceMutationLock(config, () => { ran = true; }); + // An older build's ancient lock no longer blocks, and it is still never moved or removed. + mkdirSync(lockPath, { mode: 0o700 }); + const stale = new Date(Date.now() - (25 * 60 * 60 * 1000)); + utimesSync(lockPath, stale, stale); withPublicEvidenceMutationLock(config, () => {}); - } finally { spy.mockRestore(); } - expect(state.successor).not.toBeNull(); - expectSuccessorIntact(lockPath, state.successor!); - expect(lockEntries(lockPath)).toEqual(["owner.json"]); - expect(detachedLockNames(config)).toEqual([]); - }); - - test("stale reclaim leaves a successor lock that replaced the claimed directory in place", () => { - const config = configDir(); - const lockPath = join(labCommunityDir(config), ".mutation-lock"); - mkdirSync(lockPath, { mode: 0o700 }); - const stale = new Date(Date.now() - (25 * 60 * 60 * 1000)); - utimesSync(lockPath, stale, stale); - const { spy, state } = replaceLockBeforeRename(lockPath, (from, to) => - samePath(from, lockPath) && /\.mutation-lock-stale-/.test(to)); - let reclaimed: boolean; - try { - reclaimed = publicEvidenceTryReclaimMutationLockForTests(config); - } finally { spy.mockRestore(); } - expect(reclaimed).toBe(false); - expectSuccessorIntact(lockPath, state.successor!); - expect(lockEntries(lockPath)).toEqual(["owner.json"]); - expect(detachedLockNames(config)).toEqual([]); - }); - - test("discarding a failed acquisition leaves a successor lock that replaced it in place", () => { - const config = configDir(); - const lockPath = join(labCommunityDir(config), ".mutation-lock"); - const realWrite = fs.writeFileSync; - const write = spyOn(fs, "writeFileSync").mockImplementation(((path, data, options) => { - if (/\.owner-[0-9a-f-]{36}\.tmp$/.test(String(path))) throw new Error("injected owner publication failure"); - return realWrite(path, data, options); - }) as typeof fs.writeFileSync); - const { spy, state } = replaceLockBeforeFirstMutation(lockPath); - try { - expect(() => withPublicEvidenceMutationLock(config, () => {})).toThrow("injected owner publication failure"); - } finally { spy.mockRestore(); write.mockRestore(); } - expectSuccessorIntact(lockPath, state.successor!); - expect(detachedLockNames(config)).toEqual([]); - }); - - test("a moved lock with a live owner survives a later acquisition's detached cleanup", () => { - const config = configDir(); - const lockPath = join(labCommunityDir(config), ".mutation-lock"); - mkdirSync(lockPath, { mode: 0o700 }); - const stale = new Date(Date.now() - (25 * 60 * 60 * 1000)); - utimesSync(lockPath, stale, stale); - // The successor is moved, then the canonical name is taken before it can be put back. - const { spy, state } = replaceLockBeforeRename(lockPath, (from, to) => - samePath(from, lockPath) && /\.mutation-lock-stale-/.test(to), () => mkdirSync(lockPath, { mode: 0o700 })); - try { - expect(publicEvidenceTryReclaimMutationLockForTests(config)).toBe(false); - } finally { spy.mockRestore(); } - const detached = detachedLockNames(config); - expect(detached).toHaveLength(1); - fs.rmdirSync(lockPath); - let ran = false; - withPublicEvidenceMutationLock(config, () => { ran = true; }); + } finally { + for (const spy of spies) spy.mockRestore(); + } expect(ran).toBe(true); - expect(detachedLockNames(config)).toEqual(detached); - expectSuccessorIntact(join(labCommunityDir(config), detached[0]!), state.successor!); + expect(touched).toEqual([]); + expect(existsSync(lockPath)).toBe(true); }); - test("release keeps a lock directory that holds another process's reclaim claim", () => { + test("a second holder in the same process is refused without running its work", () => { const config = configDir(); - const lockPath = join(labCommunityDir(config), ".mutation-lock"); + let inner = false, outer = false; withPublicEvidenceMutationLock(config, () => { - writeFileSync(join(lockPath, ".reclaim.json"), - JSON.stringify({ pid: process.pid, token: SUCCESSOR_TOKEN, createdAt: Date.now() }), - { encoding: "utf8", mode: 0o600 }); + busyFailure(() => withPublicEvidenceMutationLock(config, () => { inner = true; })); + outer = true; }); - expect(lockEntries(lockPath)).toEqual([".reclaim.json"]); - expect(detachedLockNames(config)).toEqual([]); + expect(outer).toBe(true); + expect(inner).toBe(false); + let after = false; + withPublicEvidenceMutationLock(config, () => { after = true; }); + expect(after).toBe(true); }); - test("an acquisition whose fresh directory is removed before its owner write retries", () => { + test("a holder that throws releases the lock", () => { const config = configDir(); - const lockPath = join(labCommunityDir(config), ".mutation-lock"); - const realWrite = fs.writeFileSync; - let removed = 0; - const write = spyOn(fs, "writeFileSync").mockImplementation(((path, data, options) => { - if (removed === 0 && /\.owner-[0-9a-f-]{36}\.tmp$/.test(String(path))) { - removed += 1; - fs.rmdirSync(lockPath); - } - return realWrite(path, data, options); - }) as typeof fs.writeFileSync); + expect(() => withPublicEvidenceMutationLock(config, () => { throw new Error("protected work failed"); })) + .toThrow("protected work failed"); let ran = false; - try { - withPublicEvidenceMutationLock(config, () => { ran = existsSync(join(lockPath, "owner.json")); }); - } finally { write.mockRestore(); } - expect(removed).toBe(1); + withPublicEvidenceMutationLock(config, () => { ran = true; }); expect(ran).toBe(true); - expect(existsSync(lockPath)).toBe(false); - }); - - test("release removes its own lock directory and leaves no detached copy", () => { - const config = configDir(); - const lockPath = join(labCommunityDir(config), ".mutation-lock"); - let held = false; - withPublicEvidenceMutationLock(config, () => { held = existsSync(join(lockPath, "owner.json")); }); - expect(held).toBe(true); - expect(existsSync(lockPath)).toBe(false); - expect(detachedLockNames(config)).toEqual([]); }); - test("does not publish into a lock replaced with an adjacent NTFS-sized identity", () => { + test("another process holding the lock is refused, and its crash releases the lock", async () => { const config = configDir(); - const lockPath = join(labCommunityDir(config), ".mutation-lock"); - const original = fs.lstatSync; - let observations = 0; - const spy = spyOn(fs, "lstatSync").mockImplementation(((path, options) => { - const stats = options?.bigint ? original(path, { bigint: true }) : original(path); - if (!samePath(path, lockPath)) return stats; - const ino = 9_007_199_254_740_992n + BigInt(Math.min(observations++, 1)); - return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, - { ino: options?.bigint ? ino : Number(ino) }); - }) as typeof fs.lstatSync); + const child = Bun.spawn([process.execPath, "-e", [ + `const { writeSync } = require("node:fs");`, + `const { withPublicEvidenceMutationLock } = require(${JSON.stringify(repoPath("src/lab/public/mutation-lock.ts"))});`, + `withPublicEvidenceMutationLock(${JSON.stringify(config)}, () => {`, + ` writeSync(1, "held\\n");`, + ` Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ${SPAWN_BUDGET_MS});`, + `});`, + ].join("\n")], { env: process.env, stdin: "ignore", stdout: "pipe", stderr: "pipe" }); try { - expect(() => withPublicEvidenceMutationLock(config, () => {})).toThrow(PublicEvidenceValidationError); - } finally { spy.mockRestore(); } - }); - test("recovers an ancient incomplete lock before reading committed cache state", () => { - const config = configDir(); - const lockPath = join(labCommunityDir(config), ".mutation-lock"); - mkdirSync(lockPath, { mode: 0o700 }); - const stale = new Date(Date.now() - (25 * 60 * 60 * 1000)); - utimesSync(lockPath, stale, stale); + const reader = child.stdout.getReader(); + let seen = ""; + while (!seen.includes("held\n")) { + const chunk = await reader.read(); + if (chunk.done) throw new Error(`lock holder exited early: ${await new Response(child.stderr).text()}`); + seen += new TextDecoder().decode(chunk.value); + } + reader.releaseLock(); + let ran = false; + busyFailure(() => withPublicEvidenceMutationLock(config, () => { ran = true; })); + expect(ran).toBe(false); + child.kill("SIGKILL"); + await child.exited; + withPublicEvidenceMutationLock(config, () => { ran = true; }); + expect(ran).toBe(true); + } finally { + if (child.exitCode === null) child.kill("SIGKILL"); + await child.exited; + } + }, SPAWN_BUDGET_MS); + test("the lock database lives outside the community cache and is never followed when unsafe", () => { + const config = configDir(); + withPublicEvidenceMutationLock(config, () => {}); + const lockDb = join(labRoot(config), "public-evidence-mutation.sqlite"); + expect(lstatSync(lockDb).isFile()).toBe(true); expect(listCommunityEvidence(config)).toEqual([]); - expect(existsSync(lockPath)).toBe(false); - }); - test("does not reclaim an old lock while its recorded owner process is alive", () => { - const config = configDir(); - const lockPath = join(labCommunityDir(config), ".mutation-lock"); - mkdirSync(lockPath, { mode: 0o700 }); - writeFileSync( - join(lockPath, "owner.json"), - JSON.stringify({ - pid: process.pid, - token: "00000000-0000-4000-8000-000000000000", - createdAt: Date.now() - (25 * 60 * 60 * 1000), - }), - { encoding: "utf8", mode: 0o600 }, - ); + const linked = configDir(); + const target = join(linked, "elsewhere.sqlite"); + writeFileSync(target, ""); + linkSync(target, join(labRoot(linked), "public-evidence-mutation.sqlite")); + expect(() => withPublicEvidenceMutationLock(linked, () => {})).toThrow(/mutation lock is unsafe/); - expect(publicEvidenceMutationLockIsReclaimableForTests(config)).toBe(false); - expect(existsSync(lockPath)).toBe(true); + const directory = configDir(); + mkdirSync(join(labRoot(directory), "public-evidence-mutation.sqlite")); + let ran = false; + expect(() => withPublicEvidenceMutationLock(directory, () => { ran = true; })).toThrow(/mutation lock is unsafe/); + expect(ran).toBe(false); }); - test("rejects a live owner after one check without running protected work or changing ownership", () => { + test("an older build's live directory lock is honoured once and left untouched", () => { const config = configDir(); - const lockPath = createLiveOwnerLock(config); - const lockBefore = lstatSync(lockPath); + const lockPath = createLegacyOwnerLock(config); + const lockBefore = lstatSync(lockPath, { bigint: true }); const ownerBefore = readFileSync(join(lockPath, "owner.json")); let ranProtectedWork = false; - let failure: unknown; - // Observe the real signal-zero owner check. A retry loop must not poll a - // live owner before eventually returning the same refusal. + // Observe the real signal-zero owner check: exactly one, with no retry loop. const ownerCheck = spyOn(process, "kill"); try { - try { - withPublicEvidenceMutationLock(config, () => { ranProtectedWork = true; }); - } catch (error) { - failure = error; - } - - expect(failure).toBeInstanceOf(PublicEvidenceValidationError); - expect((failure as PublicEvidenceValidationError).code).toBe("community_cache_busy"); - expect(ranProtectedWork).toBe(false); - expect(readFileSync(join(lockPath, "owner.json"))).toEqual(ownerBefore); - const lockAfter = lstatSync(lockPath); - expect([lockAfter.dev, lockAfter.ino]).toEqual([lockBefore.dev, lockBefore.ino]); + busyFailure(() => withPublicEvidenceMutationLock(config, () => { ranProtectedWork = true; })); expect(ownerCheck).toHaveBeenCalledTimes(1); expect(ownerCheck).toHaveBeenCalledWith(process.pid, 0); } finally { ownerCheck.mockRestore(); } + expect(ranProtectedWork).toBe(false); + expect(readFileSync(join(lockPath, "owner.json"))).toEqual(ownerBefore); + const lockAfter = lstatSync(lockPath, { bigint: true }); + expect([lockAfter.dev, lockAfter.ino]).toEqual([lockBefore.dev, lockBefore.ino]); }); - test("a competing reclaim claim prevents a second stale reclaimer from deleting the lock", () => { + test("an older build's recent ownerless directory blocks, and an ancient one does not", () => { const config = configDir(); - const lockPath = join(labCommunityDir(config), ".mutation-lock"); + const lockPath = legacyLockPath(config); mkdirSync(lockPath, { mode: 0o700 }); + expect(publicEvidenceLegacyMutationLockBlocksForTests(config)).toBe(true); + const stale = new Date(Date.now() - (25 * 60 * 60 * 1000)); utimesSync(lockPath, stale, stale); - writeFileSync( - join(lockPath, ".reclaim.json"), - JSON.stringify({ token: "00000000-0000-4000-8000-000000000000" }), - { encoding: "utf8", mode: 0o600 }, - ); - - expect(publicEvidenceTryReclaimMutationLockForTests(config)).toBe(false); + expect(publicEvidenceLegacyMutationLockBlocksForTests(config)).toBe(false); + expect(listCommunityEvidence(config)).toEqual([]); expect(existsSync(lockPath)).toBe(true); }); - test("cleans detached lock quarantine before scanning community quota state", () => { + test("an older build's lock with a live owner blocks even when old enough for the ownerless rule", () => { + const config = configDir(); + createLegacyOwnerLock(config, Date.now() - (25 * 60 * 60 * 1000)); + expect(publicEvidenceLegacyMutationLockBlocksForTests(config)).toBe(true); + }); + + test("cleans an older build's detached lock quarantine before scanning community quota state", () => { const config = configDir(); const quarantinePath = join( labCommunityDir(config), @@ -332,9 +211,9 @@ describe("community mutation lock", () => { expect(existsSync(quarantinePath)).toBe(false); }); - test("fails closed when the lock path is not a directory", () => { + test("fails closed when the older build's lock path is not a directory", () => { const config = configDir(); - writeFileSync(join(labCommunityDir(config), ".mutation-lock"), "unsafe", "utf8"); + writeFileSync(legacyLockPath(config), "unsafe", "utf8"); expect(() => listCommunityEvidence(config)).toThrow(/mutation lock is not a directory/i); }); diff --git a/tests/lab/lab-public-coderabbit-regressions.test.ts b/tests/lab/lab-public-coderabbit-regressions.test.ts index 7653de2d682..6e90f71bddb 100644 --- a/tests/lab/lab-public-coderabbit-regressions.test.ts +++ b/tests/lab/lab-public-coderabbit-regressions.test.ts @@ -28,8 +28,8 @@ import { type PublicEvidenceRecordV1, } from "../../src/lab/public"; import { - publicEvidenceMutationLockIsReclaimableForTests, - publicEvidenceTryReclaimMutationLockForTests, + publicEvidenceLegacyMutationLockBlocksForTests, + withPublicEvidenceMutationLock, } from "../../src/lab/public/mutation-lock"; import { removeTreeWithRetry } from "../helpers/remove-tree"; @@ -102,7 +102,7 @@ async function captureCli(argv: string[], home: string): Promise<{ code: number; } } -test("ancient live-PID mutation owner becomes reclaimable after the absolute ceiling", () => { +test("an older build's ancient live-PID mutation owner stops blocking after the absolute ceiling", () => { const home = configDir("ocx-cl10-lock-owner-ceiling-"); ensureLabDirs(home); const lockPath = join(labCommunityDir(home), ".mutation-lock"); @@ -118,10 +118,10 @@ test("ancient live-PID mutation owner becomes reclaimable after the absolute cei { mode: 0o600 }, ); - expect(publicEvidenceMutationLockIsReclaimableForTests(home, now)).toBe(true); + expect(publicEvidenceLegacyMutationLockBlocksForTests(home, now)).toBe(false); }); -test("ancient live-PID reclaim claim cannot block stale lock recovery forever", () => { +test("an older build's ancient lock with a live-PID reclaim claim does not block mutation", () => { const home = configDir("ocx-cl10-lock-claim-ceiling-"); ensureLabDirs(home); const lockPath = join(labCommunityDir(home), ".mutation-lock"); @@ -147,8 +147,10 @@ test("ancient live-PID reclaim claim cannot block stale lock recovery forever", { mode: 0o600 }, ); - expect(publicEvidenceTryReclaimMutationLockForTests(home, now)).toBe(true); - expect(existsSync(lockPath)).toBe(false); + let ran = false; + withPublicEvidenceMutationLock(home, () => { ran = true; }); + expect(ran).toBe(true); + expect(existsSync(lockPath)).toBe(true); }); test("foreign origin-directory entries do not consume marker quota", () => { From effe618fc911cbf9525e786c75ed584111603dc0 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Sat, 31 Oct 2026 11:24:16 +0900 Subject: [PATCH 6/8] fix: bind the Lab lock file to the inspected file and keep live detached locks Create the lock database exclusively with mode 0600, re-prove its identity after locking, and skip older builds' detached copies that still record a live owner. --- src/lab/public/mutation-lock.ts | 59 +++++++++++++------ tests/lab/lab-community-mutation-lock.test.ts | 40 +++++++++++++ 2 files changed, 80 insertions(+), 19 deletions(-) diff --git a/src/lab/public/mutation-lock.ts b/src/lab/public/mutation-lock.ts index ec2583b1d21..d8fa612ae7f 100644 --- a/src/lab/public/mutation-lock.ts +++ b/src/lab/public/mutation-lock.ts @@ -1,7 +1,7 @@ import { Database } from "bun:sqlite"; -import { chmodSync, lstatSync, readdirSync, rmSync } from "node:fs"; +import { lstatSync, readdirSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; -import { statTimeMs } from "../../lib/file-identity"; +import { sameFileIdentity, statTimeMs, type FileIdentity } from "../../lib/file-identity"; import { ensureLabDirs, labCommunityDir, labRoot } from "../paths"; import { readPrivateRegularFile } from "./file-safety"; import { PublicEvidenceValidationError } from "./validate"; @@ -91,31 +91,47 @@ function legacyMutationLockBlocks(lockPath: string, nowMs: number): boolean { return nowMs - statTimeMs(stat.mtimeNs) <= LEGACY_MUTATION_LOCK_INCOMPLETE_STALE_MS; } -function cleanupDetachedMutationLocks(configDir?: string): void { +function cleanupDetachedMutationLocks(configDir: string | undefined, nowMs: number): void { const dir = labCommunityDir(configDir); for (const name of readdirSync(dir)) { if (!DETACHED_MUTATION_LOCK_RE.test(name)) continue; + const detachedPath = join(dir, name); // Older builds moved a released or reclaimed lock directory to one of these UUID-scoped - // names before deleting it. Removing leftovers keeps them from leaking storage or being - // mistaken for community cache objects. - rmSync(join(dir, name), { recursive: true, force: true }); + // names before deleting it. One that still records a live owner may belong to a running + // older-build holder, so it is left alone. Removing the rest keeps them from leaking + // storage or being mistaken for community cache objects. + const owner = readLegacyMutationLockOwner(detachedPath); + if (owner && !pidDefinitelyDead(owner.pid) && nowMs - owner.createdAt <= LEGACY_MUTATION_LOCK_ABSOLUTE_STALE_MS) continue; + rmSync(detachedPath, { recursive: true, force: true }); } } -function openMutationLockDatabase(configDir?: string): Database { - const path = mutationLockDatabasePath(configDir); +/** The lock file's identity, after proving it is a single-link regular file. */ +function mutationLockFileIdentity(path: string): FileIdentity { + const stat = lstatSync(path, { bigint: true }); + if (!stat.isFile() || stat.isSymbolicLink() || stat.nlink !== 1n) { + throw new PublicEvidenceValidationError("community_cache_lock", "community cache mutation lock is unsafe"); + } + return { dev: stat.dev, ino: stat.ino }; +} + +/** + * Open the lock database on a file this module created as a private regular file. + * + * The Lab root is owner-only, so the processes that can replace this pathname are the user's own, + * the same trust boundary as the config mutation lock. Within it, the file is created exclusively + * with mode 0600 (never chmod-ed through the pathname), and the caller re-proves after locking + * that the pathname still names the file inspected before the open. + */ +function openMutationLockDatabase(path: string): { database: Database; identity: FileIdentity } { try { - const stat = lstatSync(path); - if (!stat.isFile() || stat.isSymbolicLink() || stat.nlink !== 1) { - throw new PublicEvidenceValidationError("community_cache_lock", "community cache mutation lock is unsafe"); - } + writeFileSync(path, "", { flag: "wx", mode: 0o600 }); } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; } + const identity = mutationLockFileIdentity(path); try { - const database = new Database(path, { create: true }); - try { chmodSync(path, 0o600); } catch { /* the platform may ignore chmod */ } - return database; + return { database: new Database(path, { create: false, readwrite: true }), identity }; } catch { // The SQLite error text can name local paths; the stable code is what callers act on. throw new PublicEvidenceValidationError("community_cache_lock", "community cache mutation lock is unavailable"); @@ -135,7 +151,8 @@ export function withPublicEvidenceMutationLock( run: () => T, ): T { ensureLabDirs(configDir); - const database = openMutationLockDatabase(configDir); + const path = mutationLockDatabasePath(configDir); + const { database, identity } = openMutationLockDatabase(path); let transactionOpen = false; try { try { @@ -148,10 +165,14 @@ export function withPublicEvidenceMutationLock( } throw new PublicEvidenceValidationError("community_cache_lock", "community cache mutation lock is unavailable"); } - if (legacyMutationLockBlocks(legacyMutationLockPath(configDir), Date.now())) { + if (!sameFileIdentity(mutationLockFileIdentity(path), identity)) { + throw new PublicEvidenceValidationError("community_cache_lock", "community cache mutation lock changed while opening"); + } + const nowMs = Date.now(); + if (legacyMutationLockBlocks(legacyMutationLockPath(configDir), nowMs)) { throw new PublicEvidenceValidationError("community_cache_busy", "community cache is busy"); } - cleanupDetachedMutationLocks(configDir); + cleanupDetachedMutationLocks(configDir, nowMs); return run(); } finally { if (transactionOpen) { diff --git a/tests/lab/lab-community-mutation-lock.test.ts b/tests/lab/lab-community-mutation-lock.test.ts index d97e838a9e8..9da2be711b0 100644 --- a/tests/lab/lab-community-mutation-lock.test.ts +++ b/tests/lab/lab-community-mutation-lock.test.ts @@ -198,6 +198,46 @@ describe("community mutation lock", () => { expect(publicEvidenceLegacyMutationLockBlocksForTests(config)).toBe(true); }); + test("a lock file replaced between inspection and locking is refused before protected work", () => { + const config = configDir(); + const lockDb = join(labRoot(config), "public-evidence-mutation.sqlite"); + const original = fs.lstatSync; + let observations = 0; + const spy = spyOn(fs, "lstatSync").mockImplementation(((path, options) => { + const stats = options?.bigint ? original(path, { bigint: true }) : original(path); + if (String(path) !== lockDb) return stats; + // Adjacent NTFS-sized identities: equal as numbers, distinct as bigints. + const ino = 9_007_199_254_740_992n + BigInt(Math.min(observations++, 1)); + return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, { ino: options?.bigint ? ino : Number(ino) }); + }) as typeof fs.lstatSync); + let ran = false; + try { + expect(() => withPublicEvidenceMutationLock(config, () => { ran = true; })).toThrow(/changed while opening/); + } finally { spy.mockRestore(); } + expect(observations).toBe(2); + expect(ran).toBe(false); + let after = false; + withPublicEvidenceMutationLock(config, () => { after = true; }); + expect(after).toBe(true); + }); + + test.skipIf(process.platform === "win32")("the lock file is created private to the user", () => { + const config = configDir(); + withPublicEvidenceMutationLock(config, () => {}); + expect(lstatSync(join(labRoot(config), "public-evidence-mutation.sqlite")).mode & 0o777).toBe(0o600); + }); + + test("detached cleanup keeps an older build's moved lock whose owner is alive", () => { + const config = configDir(); + const live = join(labCommunityDir(config), ".mutation-lock-release-123-11111111-1111-4111-8111-111111111111"); + mkdirSync(live, { mode: 0o700 }); + writeFileSync(join(live, "owner.json"), + JSON.stringify({ pid: process.pid, token: "11111111-1111-4111-8111-111111111111", createdAt: Date.now() }), + { encoding: "utf8", mode: 0o600 }); + withPublicEvidenceMutationLock(config, () => {}); + expect(existsSync(join(live, "owner.json"))).toBe(true); + }); + test("cleans an older build's detached lock quarantine before scanning community quota state", () => { const config = configDir(); const quarantinePath = join( From 8ce3dd5748bf2efffeb22236ce56426d6f1fb065 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Sat, 31 Oct 2026 11:31:43 +0900 Subject: [PATCH 7/8] fix: skip Lab lock bookkeeping names in community cache scans --- src/lab/public/community.ts | 5 ++--- src/lab/public/mutation-lock.ts | 5 +++++ tests/lab/lab-community-mutation-lock.test.ts | 3 +++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/lab/public/community.ts b/src/lab/public/community.ts index 4a6d0d6b12a..23f8bab5061 100644 --- a/src/lab/public/community.ts +++ b/src/lab/public/community.ts @@ -5,7 +5,7 @@ import { ensureLabDirs, labCommunityDir } from "../paths"; import { validateCommunityEvidenceAuthorities } from "./community-authority"; import { communityBundleFileName } from "./community-files"; import { privateRegularFileSize, readPrivateRegularFile } from "./file-safety"; -import { withPublicEvidenceMutationLock } from "./mutation-lock"; +import { isMutationLockEntryName, withPublicEvidenceMutationLock } from "./mutation-lock"; import { recordLocalPublicOrigin } from "./origin"; import { cleanupStalePrivateFileStages, @@ -31,7 +31,6 @@ const MAX_DEPTH = 8; const MAX_OBJECT_KEYS = 64; const MAX_ARRAY_ELEMENTS = 512; const MAX_GENERIC_STRING_BYTES = 384 * 1024; -const COMMUNITY_MUTATION_LOCK_NAME = ".mutation-lock"; const COMMUNITY_BUNDLE_FILE_RE = /^bundle-([0-9a-f]{64})-([0-9a-f]{64})\.json$/; const COMMUNITY_REVOCATION_FILE_RE = /^revocation-([0-9a-f]{64})\.json$/; @@ -148,7 +147,7 @@ function cacheUsage(configDir?: string): { names: string[]; bytes: number } { const dir = labCommunityDir(configDir); cleanupStalePrivateFileStagesInDir(dir); const names = readdirSync(dir) - .filter((name) => name !== COMMUNITY_MUTATION_LOCK_NAME && !isPrivateFileStageName(name)) + .filter((name) => !isMutationLockEntryName(name) && !isPrivateFileStageName(name)) .sort(); if (names.length > MAX_CACHE_FILES) { throw new PublicEvidenceValidationError("community_cache_bound", "community cache file bound exceeded"); diff --git a/src/lab/public/mutation-lock.ts b/src/lab/public/mutation-lock.ts index d8fa612ae7f..6fc1f37890d 100644 --- a/src/lab/public/mutation-lock.ts +++ b/src/lab/public/mutation-lock.ts @@ -30,6 +30,11 @@ function mutationLockDatabasePath(configDir?: string): string { return join(labRoot(configDir), PUBLIC_EVIDENCE_MUTATION_DB_NAME); } +/** Lock bookkeeping entries an older build may leave in the community cache directory. */ +export function isMutationLockEntryName(name: string): boolean { + return name === LEGACY_MUTATION_LOCK_NAME || DETACHED_MUTATION_LOCK_RE.test(name); +} + function legacyMutationLockPath(configDir?: string): string { return join(labCommunityDir(configDir), LEGACY_MUTATION_LOCK_NAME); } diff --git a/tests/lab/lab-community-mutation-lock.test.ts b/tests/lab/lab-community-mutation-lock.test.ts index 9da2be711b0..c7de91d580b 100644 --- a/tests/lab/lab-community-mutation-lock.test.ts +++ b/tests/lab/lab-community-mutation-lock.test.ts @@ -236,6 +236,9 @@ describe("community mutation lock", () => { { encoding: "utf8", mode: 0o600 }); withPublicEvidenceMutationLock(config, () => {}); expect(existsSync(join(live, "owner.json"))).toBe(true); + // A kept lock directory is lock bookkeeping, not a cache object: scans skip it. + expect(listCommunityEvidence(config)).toEqual([]); + expect(existsSync(join(live, "owner.json"))).toBe(true); }); test("cleans an older build's detached lock quarantine before scanning community quota state", () => { From 568c4d6a132fd46195d5723b5a4516f5b67fe174 Mon Sep 17 00:00:00 2001 From: JUN Date: Sat, 7 Nov 2026 22:22:57 +0900 Subject: [PATCH 8/8] fix(lab): preserve distinct recovery stages --- src/lab/public/file-safety.ts | 14 +++++----- structure/adapters/compatibility-lab.md | 11 +++++--- tests/lab/lab-public-file-safety.test.ts | 33 ++++++++++++++++++++++++ tests/lib/file-identity-callers.test.ts | 2 +- 4 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/lab/public/file-safety.ts b/src/lab/public/file-safety.ts index fae5365d94d..530a612edd7 100644 --- a/src/lab/public/file-safety.ts +++ b/src/lab/public/file-safety.ts @@ -39,8 +39,8 @@ function sizeError(options: PrivateRegularFileReadOptions): PublicEvidenceValida */ function recoverPublishedPrivateFileStage(path: string): void { if (process.platform === "win32") return; - const finalStats = lstatSync(path); - if (finalStats.isSymbolicLink() || !finalStats.isFile() || finalStats.nlink <= 1) return; + const finalStats = lstatSync(path, { bigint: true }); + if (finalStats.isSymbolicLink() || !finalStats.isFile() || finalStats.nlink <= 1n) return; const dir = dirname(path); const prefix = `.${basename(path)}.`; @@ -49,12 +49,11 @@ function recoverPublishedPrivateFileStage(path: string): void { if (!name.startsWith(prefix) || !isPrivateFileStageName(name)) continue; const stagePath = join(dir, name); try { - const stageStats = lstatSync(stagePath); + const stageStats = lstatSync(stagePath, { bigint: true }); if ( stageStats.isFile() && !stageStats.isSymbolicLink() - && stageStats.dev === finalStats.dev - && stageStats.ino === finalStats.ino + && sameFileIdentity(stageStats, finalStats) ) { candidates.push(stagePath); } @@ -76,12 +75,11 @@ function recoverPublishedPrivateFileStage(path: string): void { for (const stagePath of candidates) { try { - const stageStats = lstatSync(stagePath); + const stageStats = lstatSync(stagePath, { bigint: true }); if ( stageStats.isFile() && !stageStats.isSymbolicLink() - && stageStats.dev === finalStats.dev - && stageStats.ino === finalStats.ino + && sameFileIdentity(stageStats, finalStats) ) { unlinkSync(stagePath); } diff --git a/structure/adapters/compatibility-lab.md b/structure/adapters/compatibility-lab.md index 558e3f3d638..5984aca235d 100644 --- a/structure/adapters/compatibility-lab.md +++ b/structure/adapters/compatibility-lab.md @@ -168,7 +168,7 @@ CL-05 adds a read-only Models tab (`#models/compatibility`) that visualizes the ## Public-evidence mutation, purge and revocation -Public-evidence mutation is serialized across processes by `src/lab/public/mutation-lock.ts`: an +Public-evidence mutation is serialized among cooperating current-build processes by `src/lab/public/mutation-lock.ts`: an OS-held SQLite write transaction (`BEGIN IMMEDIATE`, `busy_timeout=0`) on `lab/public-evidence-mutation.sqlite`, outside the community cache, the same primitive as the config mutation lock. A second holder is a fail-fast condition: the caller receives @@ -176,13 +176,16 @@ config mutation lock. A second holder is a fail-fast condition: the caller recei `src/server/management/lab-routes.ts` maps that code to HTTP 503 with `Retry-After: 1`. Other public-evidence validation failures stay 400. Closing the handle, including at process exit or crash, releases the lock, so there is no stale-owner recovery and no step renames or removes another -process's lock state. A lock path that is not a single-link regular file fails closed. +process's lock state. A lock path that is not a single-link regular file fails closed. The lock +assumes the owner-only Lab root and does not exclude a hostile process running as the same user. +Concurrent mutations across older and current builds during upgrades are unsupported. Older builds serialized through a `lab/community/.mutation-lock` directory. While holding the transaction, this build still refuses to run beside such a directory whose owner PID is alive within the seven-day ceiling, or which is ownerless and less than a day old, and fails closed when -that path is not a directory. It never creates, moves or removes that directory. It only removes -the older builds' UUID-named detached copies. `tests/lab/lab-community-mutation-lock.test.ts` +that path is not a directory. It never creates, moves or removes that directory. It removes +older builds' UUID-named detached copies while preserving a live older holder. +`tests/lab/lab-community-mutation-lock.test.ts` covers cross-process refusal, release on crash, and that acquire and release never rename or remove the canonical lock path. diff --git a/tests/lab/lab-public-file-safety.test.ts b/tests/lab/lab-public-file-safety.test.ts index e11b9e3ed40..78633ce2c33 100644 --- a/tests/lab/lab-public-file-safety.test.ts +++ b/tests/lab/lab-public-file-safety.test.ts @@ -40,6 +40,39 @@ test("private reads reject adjacent NTFS-sized descriptor identities", () => { } finally { spy.mockRestore(); pathSpy.mockRestore(); } }); +for (const replaceOnRecheck of [false, true]) { + test.skipIf(process.platform === "win32")( + `private reads preserve a distinct large-inode stage ${replaceOnRecheck ? "on recheck" : "on discovery"}`, + () => { + const root = tempRoot(); + const target = join(root, "target.txt"); + const alias = join(root, "alias.txt"); + const stage = join(root, `.target.txt.${process.pid}.00000000-0000-4000-8000-000000000000.tmp`); + writeFileSync(target, "safe-bytes", { mode: 0o600 }); + linkSync(target, alias); + writeFileSync(stage, "unrelated", { mode: 0o600 }); + const original = fs.lstatSync; + let stageChecks = 0; + const spy = spyOn(fs, "lstatSync").mockImplementation(((path, options) => { + const stats = options?.bigint ? original(path, { bigint: true }) : original(path); + if (!samePath(path, target) && !samePath(path, stage)) return stats; + const isStage = samePath(path, stage); + if (isStage) stageChecks++; + const different = isStage && (!replaceOnRecheck || stageChecks > 1); + const ino = 9_007_199_254_740_992n + (different ? 1n : 0n); + return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, + { ino: options?.bigint ? ino : Number(ino) }); + }) as typeof fs.lstatSync); + try { + expect(() => readPrivateRegularFile(target, { + maxBytes: 1024, errorCode: "unsafe", errorMessage: "unsafe", + })).toThrow(PublicEvidenceValidationError); + expect(fs.existsSync(stage)).toBe(true); + } finally { spy.mockRestore(); } + }, + ); +} + test("descriptor-bound private reads reject a symlink even when O_NOFOLLOW is unavailable", () => { const root = tempRoot(); const target = join(root, "target.txt"); diff --git a/tests/lib/file-identity-callers.test.ts b/tests/lib/file-identity-callers.test.ts index adb794664a4..88cda1266ae 100644 --- a/tests/lib/file-identity-callers.test.ts +++ b/tests/lib/file-identity-callers.test.ts @@ -8,7 +8,7 @@ import { repoPath } from "../helpers/repo-root"; const scopes: Record = { "src/lab/public/mutation-lock.ts": ["assertMutationLockDirectory"], "src/lab/public/private-file.ts": ["cleanupPublishedPrivateFileStages"], - "src/lab/public/file-safety.ts": ["withPrivateRegularFile"], + "src/lab/public/file-safety.ts": ["withPrivateRegularFile", "recoverPublishedPrivateFileStage"], "src/lab/artifacts/secure-fs.ts": ["assertOpenedPathMatchesDescriptor", "revalidateDir", "openTrustedArtifactDir"], "src/lab/fabric/scratch.ts": ["revalidateScratchDir", "openAtScratch", "openTrustedScratchRoot", "openScratchRelativePath"], "src/codex/codex-home-owner.ts": ["readCodexHomeJournal"],