diff --git a/knip.json b/knip.json index 1f4956faa6..111f022102 100644 --- a/knip.json +++ b/knip.json @@ -16,7 +16,12 @@ "playwright-ct": false, "workspaces": { "src": { - "entry": ["extension.ts!", "extension/api.ts!", "workers/countTokens.ts!"], + "entry": [ + "extension.ts!", + "extension/api.ts!", + "workers/countTokens.ts!", + "services/file-safety/safeWriteText.ts!" + ], "project": [ "**/*.ts!", "!**/__tests__/**!", diff --git a/src/services/file-safety/__tests__/safeWriteText.integration.spec.ts b/src/services/file-safety/__tests__/safeWriteText.integration.spec.ts new file mode 100644 index 0000000000..4a3ee6e8e5 --- /dev/null +++ b/src/services/file-safety/__tests__/safeWriteText.integration.spec.ts @@ -0,0 +1,70 @@ +import * as fs from "fs/promises" +import * as os from "os" +import * as path from "path" + +import { safeWriteText } from "../safeWriteText" + +// No fs mocks in this file: the point is to assert what a real filesystem ends up +// holding after a publish attempt, which the mocked spec cannot show. The failure is +// provoked with real filesystem semantics rather than with a stubbed call. +describe("safeWriteText against a real filesystem", () => { + let dir: string + + beforeEach(async () => { + dir = await fs.mkdtemp(path.join(os.tmpdir(), "safe-write-text-int-")) + }) + + afterEach(async () => { + await fs.rm(dir, { recursive: true, force: true }) + }) + + it("publishes the new bytes and leaves no staging or backup residue", async () => { + const targetPath = path.join(dir, "target.txt") + await fs.writeFile(targetPath, "old bytes") + + // No platform override: the real platform's own durability and ACL steps run, and this + // case is the successful restore. A failed restore now throws DaclRestoreError after + // the commit rename has already happened (the focused unit test "win32 DACL: a failed + // restore is an error and the dump is still unlinked" covers that path), so on a + // machine where icacls cannot put a saved ACL back into a throwaway temp directory + // this publish reports that error instead of resolving, and the residue assertions + // below are reached only where the restore worked. + await safeWriteText(targetPath, "new bytes", { backup: true }) + + expect(await fs.readFile(targetPath, "utf8")).toBe("new bytes") + expect(await fs.readdir(dir)).toEqual(["target.txt"]) + }) + + it("leaves the target bytes untouched when the commit cannot replace it", async () => { + // A regular file cannot be renamed over a directory, so the backup copy and + // the commit both fail on a real filesystem with no mocking at all. + const targetPath = path.join(dir, "target-dir") + await fs.mkdir(targetPath) + const inside = path.join(targetPath, "payload.txt") + await fs.writeFile(inside, "original bytes") + + await expect(safeWriteText(targetPath, "new data", { backup: true })).rejects.toThrow() + + // The directory and its content are exactly as they were, and no backup copy + // or staging directory was left behind next to them. + expect(await fs.readFile(inside, "utf8")).toBe("original bytes") + expect(await fs.readdir(dir)).toEqual(["target-dir"]) + }) + + it("cleans up the staging file when the commit rename itself fails", async () => { + // With backup:false the backup step is skipped, so this is the only shape of + // this scenario that reaches Step 4: the rename of the staging FILE over the + // target DIRECTORY fails (EISDIR / ENOTDIR / EPERM depending on platform), and + // the cleanup must remove the staging file. The backup:true case above bails + // out in the backup step and never exercises the commit-failure path. + const targetPath = path.join(dir, "target-dir") + await fs.mkdir(targetPath) + const inside = path.join(targetPath, "payload.txt") + await fs.writeFile(inside, "original bytes") + + await expect(safeWriteText(targetPath, "new data", { backup: false })).rejects.toThrow() + + expect(await fs.readFile(inside, "utf8")).toBe("original bytes") + expect(await fs.readdir(dir)).toEqual(["target-dir"]) + }) +}) diff --git a/src/services/file-safety/__tests__/safeWriteText.spec.ts b/src/services/file-safety/__tests__/safeWriteText.spec.ts new file mode 100644 index 0000000000..cc56a5c3e5 --- /dev/null +++ b/src/services/file-safety/__tests__/safeWriteText.spec.ts @@ -0,0 +1,2422 @@ +import * as fs from "fs/promises" +import type { BigIntStats } from "fs" +import * as fsSync from "fs" +import { execFile } from "child_process" +import type { ChildProcess } from "child_process" +import * as path from "path" + +import { + PostCommitDurabilityError, + DaclInspectionError, + resolveLockKey, + safeWriteText, + StagingPathError, + TargetExistsError, + UnsupportedFilesystemError, + TargetMovedError, + AncestorReplacedError, + OrphanedBackupAfterFailedWriteError, + OrphanedBackupError, + OrphanedPublishedBackupError, + PublishedEntryMismatchError, +} from "../safeWriteText" + +// Full mock for fs/promises — all methods are vi.fn() stubs +vi.mock("fs/promises", () => ({ + mkdir: vi.fn(), + access: vi.fn(), + rename: vi.fn(), + link: vi.fn(), + copyFile: vi.fn(), + chmod: vi.fn(), + unlink: vi.fn(), + rmdir: vi.fn(), + realpath: vi.fn(), + stat: vi.fn(), + lstat: vi.fn(), + readlink: vi.fn(), +})) + +// Full mock for fs — all sync methods are vi.fn() stubs. Stats is a bare +// class stub so tests can build minimal Stats stand-ins via its prototype. +vi.mock("fs", () => ({ + // Mirrors Node's value; the no-replace fallback passes it to copyFile. + constants: { COPYFILE_EXCL: 1 }, + openSync: vi.fn(), + // fstatSync is consulted after openSync on the caller-staged path, to compare the descriptor's + // identity with the one lstat authorized. The default below is an empty stand-in: the fields + // the comparison reads stay undefined on both sides, so existing cases pass it the same way + // they pass the identity comparisons they already perform. + fstatSync: vi.fn(() => ({})), + writeSync: vi.fn(), + closeSync: vi.fn(), + mkdirSync: vi.fn(), + fsyncSync: vi.fn(), + chmodSync: vi.fn(), + fchmodSync: vi.fn(), + statSync: vi.fn(), + Stats: class Stats {}, +})) + +// Mock child_process.execFile (callback-based — must invoke callback to resolve) +vi.mock("child_process", () => ({ + execFile: vi.fn((cmd, args, opts, cb) => { + if (typeof cb === "function") cb(null) + }), +})) + +// Minimal stand-in for the ChildProcess that callback-form execFile returns. +const fakeChild = { kill: () => true } as unknown as ChildProcess + +// Helper that mirrors safeWriteText's path resolution exactly +function _resolvedTarget(filePath: string): string { + return path.resolve(filePath) +} +function _dirPath(filePath: string): string { + return path.dirname(_resolvedTarget(filePath)) +} +// Minimal Stats stand-in: the SUT only reads `.mode` from it. +// Async lstat stand-in: the SUT only asks whether the path is a link or a file. +// Built on the Stats prototype so the mock value still satisfies fsSync.Stats. +function _fileStats(isLink: boolean): fsSync.Stats { + const s = Object.create(fsSync.Stats.prototype) as fsSync.Stats + s.isSymbolicLink = () => isLink + s.isFile = () => !isLink + return s +} + +function mockDefaults(): void { + vi.resetAllMocks() + // After resetAllMocks, vi.fn() returns undefined — restore promise defaults. + vi.mocked(fs.mkdir).mockResolvedValue(undefined) + vi.mocked(fs.access).mockResolvedValue(undefined) + vi.mocked(fs.rename).mockResolvedValue(undefined) + vi.mocked(fs.link).mockResolvedValue(undefined) + vi.mocked(fs.unlink).mockResolvedValue(undefined) + vi.mocked(fs.rmdir).mockResolvedValue(undefined) + // Existing-target default: a regular 0o644 file. + vi.mocked(fsSync.statSync).mockReturnValue(_stats(0o644)) + // Staged-file default: a regular file, not a link, so a caller-supplied + // tempPath passes the location and file-type check by default. + vi.mocked(fs.lstat).mockResolvedValue(_fileStats(false)) + // Ancestor-identity pin: default to one stable directory identity so the pin + // passes unless a test overrides it. + vi.mocked(fs.stat).mockResolvedValue(_dirIdentity(100n)) +} + +// BigIntStats stand-in for the ancestor pin: the SUT reads only dev/ino off it. +// BigIntStats is class-backed with no public constructor, so this is a +// last-resort double assertion (test-local, per AGENTS.md). +function _dirIdentity(ino: bigint): BigIntStats { + return { dev: 1n, ino } as unknown as BigIntStats +} +// BigIntStats stand-in for a regular staging file: the SUT reads dev/ino off it and asks +// whether it is a link. Same last-resort double assertion as _dirIdentity, test-local. +function _fileIdentity(dev: bigint, ino: bigint): BigIntStats { + return { dev, ino, isSymbolicLink: () => false, isFile: () => true } as unknown as BigIntStats +} +// BigIntStats stand-in with a controllable device, for pinning the dev half of an identity test. +// Same last-resort double assertion as _dirIdentity, test-local. +function _crossIdentity(dev: bigint, ino: bigint): BigIntStats { + return { dev, ino } as unknown as BigIntStats +} + +function _stats(mode: number): fsSync.Stats { + const s = Object.create(fsSync.Stats.prototype) as fsSync.Stats + Object.assign(s, { mode }) + return s +} + +// ── Test 1: staging file created then cleaned after success ──────────────── + +describe("safeWriteText", () => { + beforeEach(() => { + mockDefaults() + // Default sync-write behaviour: report that all requested bytes were + // written. The Buffer overload passes (fd, buffer, offset, length), + // so the fourth argument is the requested length. + vi.mocked(fsSync.writeSync).mockImplementation((...args: unknown[]) => + typeof args[3] === "number" ? args[3] : 0, + ) + }) + + describe("staging and cleanup", () => { + it("creates a temp file in the staging dir, fsyncs it, renames to target, and cleans up on success", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) // fd=1 + vi.mocked(fsSync.closeSync).mockReturnValue(undefined) + + await safeWriteText(targetPath, "hello world", { platform: "linux" }) + + // staging dir was created with private permissions — use + // stringContaining to handle Windows path resolution + expect(fsSync.mkdirSync).toHaveBeenCalledWith(expect.stringContaining(".file-safety-staging"), { + recursive: true, + mode: 0o700, + }) + // a pre-existing staging dir is repaired to private permissions too + expect(fsSync.chmodSync).toHaveBeenCalledWith(expect.stringContaining(".file-safety-staging"), 0o700) + + // temp file was opened for writing with the existing target's mode + // (default 0o644 from the statSync default mock) + expect(fsSync.openSync).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), "w", 0o644) + + // content was written as a buffer (partial-write loop, full write) + expect(fsSync.writeSync).toHaveBeenCalledWith(1, Buffer.from("hello world", "utf8"), 0, 11) + + // fsync (sync form) was called on the fd + expect(fsSync.fsyncSync).toHaveBeenCalledWith(1) + + // file was closed + expect(fsSync.closeSync).toHaveBeenCalledWith(1) + + // atomic rename happened — realpath mock returns targetPath, so that's the dest + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + + // no unlink of temp (it's now the committed file; DACL skipped via platform:linux) + expect(fs.unlink).not.toHaveBeenCalled() + }) + + it("removes the now-empty staging directory after a successful self-staged commit", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetPath, "hello", { platform: "linux" }) + + // the staging subdir is removed best-effort after the commit rename + // (stringContaining: the SUT and the test helper resolve Windows + // drive-relative paths differently, as in the existing staging tests) + expect(fs.rmdir).toHaveBeenCalledTimes(1) + expect(fs.rmdir).toHaveBeenCalledWith(expect.stringContaining(".file-safety-staging")) + // the win32 DACL restore gate must stay closed on other platforms: + // no icacls save or restore is attempted + expect(execFile).not.toHaveBeenCalled() + }) + + it("still removes the staging directory when no options are supplied at all", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + // options is undefined: the self-staged check and the optional-chained + // DACL runner lookup must not dereference it + await expect(safeWriteText(targetPath, "hello")).resolves.toBeUndefined() + + expect(fs.rmdir).toHaveBeenCalledTimes(1) + expect(fs.rmdir).toHaveBeenCalledWith(expect.stringContaining(".file-safety-staging")) + if (process.platform === "win32") { + // default platform is win32: the DACL save + restore still ran + // through the default icacls path (options?.execFileRunner must + // not throw when options is undefined) + expect(vi.mocked(execFile)).toHaveBeenCalledTimes(2) + expect(vi.mocked(fs.unlink)).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.acl")) + } + }) + + it("does not remove the staging directory when the caller supplies its own tempPath", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const callerTemp = "/tmp/test-dir/caller-staged.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetPath, "hello", { platform: "linux", tempPath: callerTemp }) + + // the caller owns its temp file's directory; safeWriteText must not + // rmdir a directory it did not create + expect(fs.rmdir).not.toHaveBeenCalled() + }) + + it("a failed staging-dir removal never fails the committed write", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fs.rmdir).mockRejectedValue(Object.assign(new Error("ENOTEMPTY"), { code: "ENOTEMPTY" })) + + await expect(safeWriteText(targetPath, "hello", { platform: "linux" })).resolves.toBeUndefined() + + // the commit rename still happened and the rmdir error was swallowed + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining(".file-safety-staging"), targetPath) + expect(fs.rmdir).toHaveBeenCalledTimes(1) + expect(fs.rmdir).toHaveBeenCalledWith(expect.stringContaining(".file-safety-staging")) + }) + + it("gives each self-staged write its own staging directory so a concurrent write cannot remove it", async () => { + const targetA = "/tmp/test-dir/target-a.txt" + const targetB = "/tmp/test-dir/target-b.txt" + vi.mocked(fs.realpath).mockImplementation((p) => Promise.resolve(p as string)) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetA, "a", { platform: "linux" }) + await safeWriteText(targetB, "b", { platform: "linux" }) + + // Two self-staged writes in the same directory must not share one staging + // directory: the first write's best-effort rmdir would otherwise delete the + // directory the second write had created but not yet opened (ENOENT on openSync). + const created = vi.mocked(fsSync.mkdirSync).mock.calls.map((c) => String(c[0])) + const staging = created.filter((p) => p.includes(".file-safety-staging_")) + expect(staging).toHaveLength(2) + expect(staging[0]).not.toBe(staging[1]) + // Uniqueness comes from the documented name shape + // /.file-safety-staging__: pinning the shape + // keeps the separator and the random suffix meaningful, not just the prefix. + for (const dir of staging) { + expect(dir).toMatch(/\.file-safety-staging_\d+_[a-z0-9]+$/) + } + const removed = vi.mocked(fs.rmdir).mock.calls.map((c) => String(c[0])) + expect(removed).toEqual([staging[0], staging[1]]) + }) + + it("removes its own staging directory when a self-staged write fails", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fs.rename).mockRejectedValue(Object.assign(new Error("EACCES"), { code: "EACCES" })) + + await expect(safeWriteText(targetPath, "hello", { platform: "linux" })).rejects.toThrow("EACCES") + + // The failed write's temp file is unlinked, then the directory it + // created is removed — a failed write must not leave an empty + // .file-safety-staging directory behind. + // mkdirSync created this write's staging directory; the temp file lives + // inside it, so the unlink targets a path under that directory. + const staging = vi.mocked(fsSync.mkdirSync).mock.calls.map((c) => String(c[0])) + expect(staging).toHaveLength(1) + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining(staging[0])) + expect(fs.rmdir).toHaveBeenCalledWith(staging[0]) + // The directory is only empty after its temp file is gone, so the + // unlink must happen before the rmdir. + expect(vi.mocked(fs.unlink).mock.invocationCallOrder[0]).toBeLessThan( + vi.mocked(fs.rmdir).mock.invocationCallOrder[0], + ) + }) + + it("does not remove a staging directory it did not create when a caller-staged write fails", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const callerTemp = "/tmp/test-dir/caller-staged.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fs.rename).mockRejectedValue(Object.assign(new Error("EACCES"), { code: "EACCES" })) + + await expect( + safeWriteText(targetPath, "hello", { platform: "linux", tempPath: callerTemp }), + ).rejects.toThrow("EACCES") + + // The caller owns that directory: only the caller's temp file is cleaned, + // never a rmdir of a directory safeWriteText never created. + expect(fs.unlink).toHaveBeenCalledWith(callerTemp) + expect(fs.rmdir).not.toHaveBeenCalled() + }) + }) + + // ── Test 2: fsync ordering ─────────────────────────────────────────────── + + describe("fsync ordering", () => { + it("calls fsync on the fd before close, and rename after close", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetPath, "data", { platform: "linux" }) + + // Verify call order: openSync(temp) → writeSync → fsyncSync(temp) + // → closeSync(temp) → rename. On POSIX the parent directory is then + // opened and fsynced after the commit rename, so openSync/fsyncSync/ + // closeSync each have a second (directory) call. + expect(vi.mocked(fsSync.openSync).mock.calls.length).toBe(2) + expect(vi.mocked(fsSync.writeSync).mock.calls.length).toBe(1) + expect(vi.mocked(fsSync.fsyncSync).mock.calls.length).toBe(2) + expect(vi.mocked(fsSync.closeSync).mock.calls.length).toBe(2) + + // the temp file was fully closed before the commit rename + expect(vi.mocked(fsSync.closeSync).mock.calls[0][0]).toBe(1) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + // The title promises the order, so compare the invocations rather than + // only count them: a rename before closeSync, or a close before fsync, + // would not be a durable commit. + const fsyncOrder = vi.mocked(fsSync.fsyncSync).mock.invocationCallOrder[0] + const closeOrder = vi.mocked(fsSync.closeSync).mock.invocationCallOrder[0] + const renameOrder = vi.mocked(fs.rename).mock.invocationCallOrder[0] + expect(fsyncOrder).toBeLessThan(closeOrder) + expect(closeOrder).toBeLessThan(renameOrder) + }) + }) + + // ── Test 3: simulated failure between write and rename leaves target intact ── + + describe("crash/torn-write safety", () => { + it("simulated failure between fsync and rename leaves the target byte-identical and no temp left behind", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fs.rename).mockRejectedValue(new Error("ENOSPC")) + + await expect(safeWriteText(targetPath, "new data", { platform: "linux" })).rejects.toThrow("ENOSPC") + + // rename was attempted (the failure point) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + + // temp file was cleaned up on failure + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_")) + + // backup was NOT created (backup:false by default), so target is untouched + // The only rename call was temp→target, not a rollback rename + expect(fs.rename).toHaveBeenCalledTimes(1) + }) + + it("a post-commit backup cleanup failure is non-fatal: the target stays committed and no temp is left behind", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // The post-commit backup unlink (SUT step 6) fails — the write must + // still succeed; an orphaned backup is the documented acceptable + // outcome, so the failure is swallowed instead of rolling back. + vi.mocked(fs.unlink).mockRejectedValueOnce(new Error("EPERM")) + + await safeWriteText(targetPath, "data", { backup: true, platform: "linux" }) + + // the commit rename (temp -> target) still happened; it is the only rename + expect(fs.rename).toHaveBeenCalledTimes(1) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + + // the failing cleanup was the post-commit backup unlink, aimed at the copy this + // write created: shape alone would pass for a cleanup aimed at somebody else's + // backup, so the name that was created is read off the copyFile call. + const createdBackup = String((vi.mocked(fs.copyFile).mock.calls.at(-1) || [])[1]) + expect(createdBackup).toContain("safeWriteText.bak_") + expect(fs.unlink).toHaveBeenCalledWith(createdBackup) + + // the staging temp was already committed by the rename; nothing + // temp-shaped is unlinked afterwards + expect(fs.unlink).not.toHaveBeenCalledWith(expect.stringContaining("safeWriteText_")) + }) + }) + + // ── Test 4: backup:true keeps old safeWriteJson semantics incl. rollback ── + + describe("backup:true", () => { + it("copies target -> backup before commit without moving the target, deletes the copy on success", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetPath, "new data", { backup: true }) + + // target was accessed (exists check) + expect(fs.access).toHaveBeenCalledWith(targetPath) + + // The backup is a copy: the canonical target is never moved away, so readers + // never see a missing file and no later step can clobber a concurrent publish. + expect(fs.copyFile).toHaveBeenCalledWith(targetPath, expect.stringContaining("safeWriteText.bak_")) + expect(fs.rename).not.toHaveBeenCalledWith(targetPath, expect.stringContaining("safeWriteText.bak_")) + + // the only rename is the atomic commit temp -> target + expect(fs.rename).toHaveBeenCalledTimes(1) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + + // backup copy was deleted on success + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.bak_")) + }) + + it("a failed commit does not move the target, so nothing has to be rolled back", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // The commit rename is the only rename in the flow and it fails. + vi.mocked(fs.rename).mockRejectedValue(new Error("ENOSPC")) + + await expect(safeWriteText(targetPath, "new data", { backup: true })).rejects.toThrow("ENOSPC") + + // The target never left its path, so there is no restore rename and the + // pre-write content is still what a reader sees at targetPath. + expect(fs.rename).toHaveBeenCalledTimes(1) + expect(fs.copyFile).toHaveBeenCalledWith(targetPath, expect.stringContaining("safeWriteText.bak_")) + + // Both the backup copy and the staging temp are cleaned up on failure. + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.bak_")) + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_")) + }) + + it("retries the post-commit backup removal and succeeds when the retry clears", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + const backupUnlinks: string[] = [] + vi.mocked(fs.unlink).mockImplementation(async (candidate: unknown) => { + const asString = String(candidate) + if (asString.includes("safeWriteText.bak_")) { + backupUnlinks.push(asString) + // The first attempt hits a transient Windows handle; the retry succeeds. + if (backupUnlinks.length === 1) { + throw Object.assign(new Error("EBUSY"), { code: "EBUSY" }) + } + } + }) + + await safeWriteText(targetPath, "new data", { backup: true }) + + expect(backupUnlinks).toHaveLength(2) + // Identity, not shape: both attempts must be aimed at the copy this write created. + // A retry that retargeted to another backup-shaped name would still produce two + // unlink calls and clear nothing. + const createdBackupPath = String((vi.mocked(fs.copyFile).mock.calls.at(-1) || [])[1]) + expect(createdBackupPath).toContain("safeWriteText.bak_") + expect(backupUnlinks[0]).toBe(createdBackupPath) + expect(backupUnlinks[1]).toBe(createdBackupPath) + }) + + it("surfaces an orphaned pre-write copy instead of returning success", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fs.unlink).mockImplementation(async (candidate: unknown) => { + if (String(candidate).includes("safeWriteText.bak_")) { + throw Object.assign(new Error("EPERM"), { code: "EPERM" }) + } + }) + + const error = await safeWriteText(targetPath, "new data", { backup: true }).catch( + (caught: unknown) => caught, + ) + + expect(error).toBeInstanceOf(OrphanedPublishedBackupError) + const orphan = error as OrphanedPublishedBackupError + expect(orphan.orphanedBackupPath).toContain("safeWriteText.bak_") + expect(orphan.publishedPath).toBe(targetPath) + // Two attempts, then the failure is reported instead of swallowed. + expect( + vi.mocked(fs.unlink).mock.calls.filter((call) => String(call[0]).includes("safeWriteText.bak_")), + ).toHaveLength(2) + }) + + it("creates the backup privately before its content exists, then fsyncs it", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetPath, "new data", { backup: true }) + + // The destination must exist with a private mode before copyFile writes anything + // into it: copyFile chooses the destination mode itself, so a restrictive target + // could otherwise leave a group/world-readable copy that a later chmod cannot + // undo. "wx" also means a pre-existing path is never silently reused. + const seedOpen = vi.mocked(fsSync.openSync).mock.calls.find(function (call) { + return String(call[0]).includes("safeWriteText.bak_") && call[1] === "wx" + }) + expect(seedOpen).toBeDefined() + expect(seedOpen?.[2]).toBe(0o600) + const seedOrder = vi.mocked(fsSync.openSync).mock.invocationCallOrder[ + vi.mocked(fsSync.openSync).mock.calls.indexOf(seedOpen!) + ] + expect(seedOrder).toBeLessThan(vi.mocked(fs.copyFile).mock.invocationCallOrder[0]) + + // The chmod keeps a copied read-only attribute (Windows) from breaking the fsync + // open, and keeps a backup of a permissive file private. + expect(fs.copyFile).toHaveBeenCalledWith(targetPath, expect.stringContaining("safeWriteText.bak_")) + expect(fs.chmod).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.bak_"), 0o600) + expect(vi.mocked(fs.chmod).mock.invocationCallOrder[0]).toBeGreaterThan( + vi.mocked(fs.copyFile).mock.invocationCallOrder[0], + ) + + // The copy is then opened for fsync with the writable flag. + const backupOpen = vi.mocked(fsSync.openSync).mock.calls.find(function (call) { + return String(call[0]).includes("safeWriteText.bak_") && call[1] === "r+" + }) + expect(backupOpen).toBeDefined() + }) + + it("a failed backup flush is reported and leaves no partial backup behind", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // The copy lands, but the fsync of the copy fails: the retained content is not + // known to be durable, so the write must not proceed on a half-written backup. + // The staged temp is fsynced earlier with a different handle, so target the + // backup's fd specifically. + vi.mocked(fsSync.openSync).mockImplementation((p: unknown) => + String(p).includes("safeWriteText.bak_") ? 7 : 1, + ) + vi.mocked(fsSync.fsyncSync).mockImplementation((fd: unknown) => { + if (fd === 7) { + throw new Error("EIO") + } + }) + + await expect(safeWriteText(targetPath, "new data", { backup: true, platform: "linux" })).rejects.toThrow( + "EIO", + ) + + // Nothing was published, and the incomplete copy is removed rather than left + // next to the target looking like a usable backup. + expect(fs.rename).not.toHaveBeenCalled() + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.bak_")) + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_")) + }) + + it("carries the leftover path when the partial backup cannot be removed either", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockImplementation((p: unknown) => + String(p).includes("safeWriteText.bak_") ? 7 : 1, + ) + vi.mocked(fsSync.fsyncSync).mockImplementation((fd: unknown) => { + if (fd === 7) { + throw new Error("EIO") + } + }) + let unlinkAttempts = 0 + vi.mocked(fs.unlink).mockImplementation(async (p: unknown) => { + if (String(p).includes("safeWriteText.bak_")) { + unlinkAttempts++ + } + throw Object.assign(new Error("EPERM"), { code: "EPERM" }) + }) + + // The copy failed and the locked leftover could not be unlinked even after the + // retry. Dropping the path would leave a partial copy of the previous content on + // disk with no reference to it anywhere, so the error carries it. + const error = await safeWriteText(targetPath, "new data", { + backup: true, + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(OrphanedBackupError) + const orphan = error as OrphanedBackupError + expect(orphan.orphanedBackupPath).toContain("safeWriteText.bak_") + expect(orphan.message).toContain("safeWriteText.bak_") + expect(orphan.message).toContain("EPERM") + expect(orphan.originalError).toBeInstanceOf(Error) + expect((orphan.originalError as Error).message).toBe("EIO") + expect(orphan.cause).toBe(orphan.originalError) + expect(unlinkAttempts).toBe(2) + expect(fs.rename).not.toHaveBeenCalled() + }) + + it("refuses a backup name another writer already owns instead of unlinking it", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + // The exclusive seed open hits a backup name this write never created. + vi.mocked(fsSync.openSync).mockImplementation((...args: unknown[]) => { + if (typeof args[0] === "string" && args[0].includes("safeWriteText.bak_")) { + throw Object.assign(new Error("EEXIST"), { code: "EEXIST" }) + } + return 1 + }) + + const error = await safeWriteText(targetPath, "new data", { + backup: true, + platform: "linux", + }).catch((caught: unknown) => caught) + + expect((error as Error).message).toContain("EEXIST") + // The name is not this write's orphan: unlinking it on the way out would + // delete another writer's backup to clean up a file this write never made. + const backupUnlinks = vi.mocked(fs.unlink).mock.calls.filter(function (call: unknown[]) { + return typeof call[0] === "string" && call[0].includes("safeWriteText.bak_") + }) + expect(backupUnlinks).toHaveLength(0) + expect(fs.copyFile).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + }) + + it("reports a backup copy it cannot remove after a failed write instead of dropping it", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // The copy is seeded and flushed fine; only its removal on the failure + // path fails, twice, with the same handle-not-released error Windows + // reports for a fresh file. + vi.mocked(fs.unlink).mockImplementation(async (p: unknown) => { + if (String(p).includes("safeWriteText.bak_")) { + throw Object.assign(new Error("EPERM"), { code: "EPERM" }) + } + }) + // The commit is what fails here, so nothing was published. + vi.mocked(fs.rename).mockRejectedValue(Object.assign(new Error("EIO"), { code: "EIO" })) + + const error = await safeWriteText(targetPath, "new data", { + backup: true, + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(OrphanedBackupAfterFailedWriteError) + const orphan = error as OrphanedBackupAfterFailedWriteError + expect(orphan.orphanedBackupPath).toContain("safeWriteText.bak_") + expect(orphan.message).toContain("safeWriteText.bak_") + expect(orphan.message).toContain("EPERM") + // The write's own failure stays the primary fact: it travels as + // originalError and as the cause, and cleanupError says why the copy + // is still on disk. + expect((orphan.originalError as Error).message).toBe("EIO") + expect((orphan.cleanupError as Error).message).toBe("EPERM") + expect(orphan.cause).toBe(orphan.originalError) + // The commit never succeeded, so the target still holds the previous + // content and the copy duplicates it rather than going stale. + expect(orphan.contentPublished).toBe(false) + const backupUnlinks = vi.mocked(fs.unlink).mock.calls.filter(function (call: unknown[]) { + return typeof call[0] === "string" && call[0].includes("safeWriteText.bak_") + }) + expect(backupUnlinks).toHaveLength(2) + // Reporting the leftover must not skip cleaning the write's own staged + // file and staging directory: both ran before the leftover was reported. + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_")) + expect(fs.rmdir).toHaveBeenCalled() + }) + + it("a backup copy that fails part-way is removed, not left as a usable-looking backup", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // The destination is seeded with openSync("wx") BEFORE any content exists at it, + // and the copy then fails part-way: the name is there, holding a partial copy of + // nothing usable. Cleanup has to key off the attempt, not off a copy that + // succeeded, or this file outlives the failed write beside the target. + vi.mocked(fs.copyFile).mockRejectedValue(Object.assign(new Error("EIO"), { code: "EIO" })) + + await expect(safeWriteText(targetPath, "new data", { backup: true, platform: "linux" })).rejects.toThrow( + "EIO", + ) + + // Nothing was published, and the half-written copy is removed rather than left + // next to the target looking like a backup someone could restore. + expect(fs.rename).not.toHaveBeenCalled() + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.bak_")) + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_")) + }) + + it("backup:true when target does not exist: no backup created, just commit", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // fs.access resolves for dirPath check, but rejects for target check (backup path) + vi.mocked(fs.access).mockImplementation(async (p) => { + if (typeof p === "string" && p.endsWith("target.txt")) throw { code: "ENOENT" } + }) + + await safeWriteText(targetPath, "new data", { backup: true, platform: "linux" }) + + // no backup rename (target didn't exist) + expect(fs.access).toHaveBeenCalledWith(targetPath) + + // only one rename: temp -> target + expect(fs.rename).toHaveBeenCalledTimes(1) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + + // no unlink (no backup to delete; DACL skipped via platform:linux) + expect(fs.unlink).not.toHaveBeenCalled() + }) + + // The four tests below are the acceptance criteria the follow-up issue #2389 records for + // this behavior (issue body, 1,662 bytes, opened 2026-11-09T18:20:58Z from this thread). + // The two ENOENT-injection tests describe the proposed behavior and fail on the code as + // committed before the narrowing; the other two pin the halves the criteria say must not + // change, and pass on both trees. + + it("publishes without a backup when the target is absent at the access check (AC1, unchanged)", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // The target access check is the one ENOENT this write may answer with a publish: + // with nothing at the target there was no backup to make. + vi.mocked(fs.access).mockImplementation(async (p) => { + if (typeof p === "string" && p.endsWith("target.txt")) throw { code: "ENOENT" } + }) + + await safeWriteText(targetPath, "new data", { backup: true, platform: "linux" }) + + expect(fs.access).toHaveBeenCalledWith(targetPath) + // Nothing was copied and nothing was chmod-ed: the seed block never ran. + expect(fs.copyFile).not.toHaveBeenCalled() + expect(fs.chmod).not.toHaveBeenCalled() + // The commit still happens, which is what "publishes without a backup" means. + expect(fs.rename).toHaveBeenCalledTimes(1) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + expect(fs.unlink).not.toHaveBeenCalled() + }) + + it("rejects instead of publishing when copyFile raises ENOENT after the target was found (AC2)", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // The target is there for the access check; the copy is what reports it missing. + vi.mocked(fs.copyFile).mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + + await expect(safeWriteText(targetPath, "new data", { backup: true, platform: "linux" })).rejects.toThrow( + "ENOENT", + ) + + // Nothing was published: the commit rename never ran, so the target still holds the + // content it had before this call rather than the content this call wanted to write. + expect(fs.rename).not.toHaveBeenCalled() + // Cleanup ran for this write's own leftovers: the half-seeded copy, the staged temp + // and the staging directory. + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.bak_")) + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_")) + expect(fs.rmdir).toHaveBeenCalled() + }) + + it("rejects instead of publishing when chmod raises ENOENT on a copy that vanished after it (AC2)", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // copyFile resolves - the copy was made - and the chmod that follows it is told the + // backup is gone. This is the exact sequence the row describes. + vi.mocked(fs.chmod).mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + + await expect(safeWriteText(targetPath, "new data", { backup: true, platform: "linux" })).rejects.toThrow( + "ENOENT", + ) + + expect(fs.copyFile).toHaveBeenCalledTimes(1) + expect(fs.chmod).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.bak_"), 0o600) + expect(fs.rename).not.toHaveBeenCalled() + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.bak_")) + // The target itself was never removed, so its previous content is what remains. + expect(fs.unlink).not.toHaveBeenCalledWith(targetPath) + }) + + it("reports an owned partial backup it cannot remove when the seed fails with ENOENT (AC3, unchanged)", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fs.copyFile).mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + // The name this write opened with "wx" is still on disk and its removal keeps failing + // with the error Windows reports for a file whose handle is not released yet. The other + // half of this criterion - a backup name another writer owns is never unlinked - is the + // test above this one. + vi.mocked(fs.unlink).mockImplementation(async (p: unknown) => { + if (String(p).includes("safeWriteText.bak_")) { + throw Object.assign(new Error("EPERM"), { code: "EPERM" }) + } + }) + + const error = await safeWriteText(targetPath, "new data", { + backup: true, + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(OrphanedBackupError) + const orphan = error as OrphanedBackupError + expect(orphan.orphanedBackupPath).toContain("safeWriteText.bak_") + // The seed failure stays the primary fact; the removal failure says why the copy is + // still there. + expect((orphan.originalError as Error).message).toBe("ENOENT") + expect((orphan.cleanupError as Error).message).toBe("EPERM") + expect(orphan.cause).toBe(orphan.originalError) + expect(fs.rename).not.toHaveBeenCalled() + }) + }) + + // ── Test 5: win32 DACL path ────────────────────────────────────────────── + + describe("win32 DACL", () => { + it.skipIf(process.platform !== "win32")( + "copies target DACL onto staging file via icacls before rename on Windows", + async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + await safeWriteText(targetPath, "data", { platform: "win32" }) + + // icacls dump + restore were called (execFile is callback-based mock) + expect(execFile).toHaveBeenCalledTimes(2) + }, + ) + + it("non-win32: DACL path is unreachable when platform is not win32", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetPath, "data", { platform: "linux" }) + + // icacls was NOT called on non-win32 + expect(execFile).not.toHaveBeenCalled() + }) + + it("win32 DACL failure refuses the publish instead of renaming over the target", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // icacls dump fails — the callback-based mock must invoke cb with an error. + vi.mocked(execFile).mockImplementation((_cmd, _args, _opts, cb) => { + if (typeof cb === "function") cb(new Error("icacls error"), "", "") + return fakeChild + }) + + // Captured once: calling safeWriteText twice here would double-count the single + // icacls attempt asserted below. + const refusal = await safeWriteText(targetPath, "data", { platform: "win32" }).catch( + (error: unknown) => error, + ) + + expect(refusal).toBeInstanceOf(DaclInspectionError) + // The class alone does not pin the contract: swapping the phase or dropping the + // target path still satisfies toBeInstanceOf, and both are caller-visible - the + // phase says which check refused, the path names the file the caller must not + // assume was saved. + expect(refusal).toMatchObject({ + name: "DaclInspectionError", + phase: "save", + targetPath, + message: expect.stringContaining("its DACL could not be saved"), + }) + + // The refusal names the phase and the path but not what icacls did. The + // platform failure travels on the error so a caller can tell a refused + // icacls from an icacls that could not be started at all. + const cause = (refusal as DaclInspectionError).causeError + expect(cause).toBeInstanceOf(Error) + expect((cause as Error).message).toBe("icacls error") + + // Contract change (Security Boundaries row): a target whose DACL could not be + // saved is no longer replaced by a file that inherits different rights. + expect(fs.rename).not.toHaveBeenCalled() + // one icacls attempt only: a failed capture must not try to restore + expect(execFile).toHaveBeenCalledTimes(1) + }) + + it("win32 DACL: a partial dump left by a failed save is removed and the publish is refused", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // icacls save fails - a real icacls may have written a partial dump + // before erroring, so the dump path must be cleaned up and must never + // be used for a restore. + vi.mocked(execFile).mockImplementation((_cmd, _args, _opts, cb) => { + if (typeof cb === "function") cb(new Error("icacls save error"), "", "") + return fakeChild + }) + + const refusal = await safeWriteText(targetPath, "data", { platform: "win32" }).catch( + (error: unknown) => error, + ) + + // The contract changed with the refusal: a target whose DACL could not be captured is + // not replaced by a file that inherits different rights, so no rename happens at all. + expect(refusal).toBeInstanceOf(DaclInspectionError) + expect(refusal).toMatchObject({ name: "DaclInspectionError", phase: "save", targetPath }) + expect(fs.rename).not.toHaveBeenCalled() + expect(execFile).toHaveBeenCalledTimes(1) + const saveArgs = vi.mocked(execFile).mock.calls[0]?.[1] + expect(saveArgs?.[1]).toBe("/save") + // the dump path (possibly partially created by icacls) was unlinked + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.acl")) + }) + it("win32: refuses the publish when the target exists but its access rights cannot be read", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // The target exists but is not readable: that is not "absent", and skipping DACL + // preservation has to stop the publish rather than be reported beside it. + vi.mocked(fs.access).mockImplementation(async (p) => { + if (String(p) === targetPath) { + throw Object.assign(new Error("EACCES"), { code: "EACCES" }) + } + return undefined + }) + + // Captured once: a second call would double-count the assertions below. + const refusal = await safeWriteText(targetPath, "data", { platform: "win32" }).catch( + (error: unknown) => error, + ) + + expect(refusal).toBeInstanceOf(DaclInspectionError) + // The class alone does not pin the contract: the phase says which check refused and + // the path names the file the caller must not assume was saved. + expect(refusal).toMatchObject({ + name: "DaclInspectionError", + phase: "inspect", + targetPath, + message: expect.stringContaining("its access rights could not be checked"), + }) + + // The publish is refused before any icacls runs, and nothing is renamed. + expect(execFile).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + }) + + it("win32 DACL save args are [targetPath, /save, dumpPath, /T] before backup rename", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetPath, "data", { backup: true, platform: "win32" }) + + // icacls was called twice (save + restore) + expect(execFile).toHaveBeenCalledTimes(2) + + // First call: save DACL from target before backup rename + const firstCall = vi.mocked(execFile).mock.calls[0] + expect(firstCall[0]).toBe("icacls") + expect(firstCall[1]).toEqual([targetPath, "/save", expect.stringContaining("safeWriteText.acl"), "/T"]) + + // Second call: restore DACL onto directory after commit rename + const secondCall = vi.mocked(execFile).mock.calls[1] + expect(secondCall[0]).toBe("icacls") + expect(secondCall[1]).toEqual([ + expect.stringContaining("/tmp/test-dir"), + "/restore", + expect.stringContaining("safeWriteText.acl"), + ]) + + // dump file was unlinked after restore + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.acl")) + }) + + it("win32 DACL: dump is unlinked even when restore fails", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + // icacls save succeeds, restore fails + let callCount = 0 + vi.mocked(execFile).mockImplementation((_cmd, _args, _opts, cb) => { + callCount++ + if (typeof cb === "function") { + cb(callCount === 1 ? null : new Error("icacls restore error"), "", "") + } + return fakeChild + }) + + await safeWriteText(targetPath, "data", { platform: "win32" }) + + // write succeeded despite restore failure (best-effort) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + expect(fs.rename).toHaveBeenCalledTimes(1) + + // dump file was still unlinked in finally + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.acl")) + }) + + it("win32 DACL: a failed restore is reported on the warning channel and the write still resolves", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + // icacls save succeeds, restore fails + let callCount = 0 + vi.mocked(execFile).mockImplementation((_cmd, _args, _opts, cb) => { + callCount++ + if (typeof cb === "function") { + cb(callCount === 1 ? null : new Error("icacls restore error"), "", "") + } + return fakeChild + }) + const warnings: { kind: string; path: string; cause: unknown }[] = [] + + await expect( + safeWriteText(targetPath, "data", { + platform: "win32", + onPostCommitWarning: (warning) => { + warnings.push({ kind: warning.kind, path: warning.path, cause: warning.cause }) + }, + }), + ).resolves.toBeUndefined() + + expect(warnings).toHaveLength(1) + expect(warnings[0]?.kind).toBe("dacl-restore-failed") + expect(warnings[0]?.path).toBe(path.dirname(targetPath)) + expect((warnings[0]?.cause as Error | undefined)?.message).toBe("icacls restore error") + // still best-effort in every other respect: the dump is unlinked, as the test above pins + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.acl")) + }) + + it("win32 DACL: when target does not exist, no save/restore/dump", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + // fs.access rejects for targetPath (ENOENT), but resolves for dirPath + vi.mocked(fs.access).mockImplementation(async (p) => { + if (typeof p === "string" && p.endsWith("target.txt")) throw { code: "ENOENT" } + return undefined + }) + + await safeWriteText(targetPath, "data", { platform: "win32" }) + + // icacls was NOT called (target absent → skip DACL entirely) + expect(execFile).not.toHaveBeenCalled() + + // no dump file created or unlinked + expect(fs.unlink).not.toHaveBeenCalled() + }) + }) + + // ── Test 6: pre-written temp path (tempPath option) ────────────────────── + + describe("pre-written temp path", () => { + it("uses the provided tempPath, fsyncs it, and renames to target", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + const customTempPath = "/tmp/test-dir/custom-temp.tmp" + + // platform:linux skips DACL entirely so this test focuses on tempPath only + await safeWriteText(targetPath, "", { tempPath: customTempPath, platform: "linux" }) + + // openSync was called on the custom temp path (r+ mode for fsync) + expect(fsSync.openSync).toHaveBeenCalledWith(customTempPath, "r+") + + // fsync was called + expect(fsSync.fsyncSync).toHaveBeenCalledWith(1) + + // rename happened — realpath mock returns targetPath + expect(fs.rename).toHaveBeenCalledWith(customTempPath, targetPath) + + // no unlink of custom temp (caller's concern; DACL skipped via platform:linux) + expect(fs.unlink).not.toHaveBeenCalled() + + // a caller-supplied tempPath must not create the staging directory + expect(fsSync.mkdirSync).not.toHaveBeenCalled() + }) + + it("applies the existing target's mode to a caller-supplied tempPath before publishing", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.statSync).mockReturnValue(_stats(0o600)) + vi.mocked(fsSync.openSync).mockReturnValue(2) + + const customTempPath = "/tmp/test-dir/custom-temp.tmp" + + await safeWriteText(targetPath, "", { tempPath: customTempPath, platform: "linux" }) + + // the caller-staged temp is fchmod'd to the restrictive target mode so + // the atomic rename cannot widen a 0o600 target (CWE-732 regression) + expect(fsSync.fchmodSync).toHaveBeenCalledWith(2, 0o600) + expect(fsSync.openSync).toHaveBeenCalledWith(customTempPath, "r+") + expect(fs.rename).toHaveBeenCalledWith(customTempPath, targetPath) + }) + + it("keeps the temp's default mode when the target does not exist yet (ENOENT)", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + const enoent = Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }) + vi.mocked(fsSync.statSync).mockImplementation(() => { + throw enoent + }) + vi.mocked(fsSync.openSync).mockReturnValue(2) + + const customTempPath = "/tmp/test-dir/custom-temp.tmp" + + await safeWriteText(targetPath, "", { tempPath: customTempPath, platform: "linux" }) + + // no existing target, so nothing to preserve and no fchmod on the temp + expect(fsSync.fchmodSync).not.toHaveBeenCalled() + expect(fs.rename).toHaveBeenCalledWith(customTempPath, targetPath) + }) + + it("propagates a non-ENOENT stat failure rather than defaulting the mode (caller-staged)", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + const eacces = Object.assign(new Error("EACCES: permission denied"), { code: "EACCES" }) + vi.mocked(fsSync.statSync).mockImplementation(() => { + throw eacces + }) + vi.mocked(fsSync.openSync).mockReturnValue(2) + + const customTempPath = "/tmp/test-dir/custom-temp.tmp" + + // A target that cannot be stat'd is not a fresh target: publishing with + // the default mode would widen a restrictive target through the rename. + await expect( + safeWriteText(targetPath, "", { tempPath: customTempPath, platform: "linux" }), + ).rejects.toThrow("EACCES") + expect(fsSync.fchmodSync).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + }) + + it("propagates a non-ENOENT stat failure rather than defaulting the mode (self-staged)", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + const eio = Object.assign(new Error("EIO: i/o error"), { code: "EIO" }) + vi.mocked(fsSync.statSync).mockImplementation(() => { + throw eio + }) + + // The mode is read before the temp is opened, so a real I/O failure stops + // the write before anything is staged. + await expect(safeWriteText(targetPath, "hello world", { platform: "linux" })).rejects.toThrow("EIO") + expect(fsSync.openSync).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + }) + + it("opens the temp before applying a read-only target's mode (0o444 does not block the open)", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.statSync).mockReturnValue(_stats(0o444)) + vi.mocked(fsSync.openSync).mockReturnValue(3) + + const customTempPath = "/tmp/test-dir/custom-temp.tmp" + + await safeWriteText(targetPath, "", { tempPath: customTempPath, platform: "linux" }) + + // a 0o444 target must not make openSync(tempPath, "r+") fail: the mode + // is applied with fchmodSync on the already-open fd, after the open + expect(fsSync.openSync).toHaveBeenCalledWith(customTempPath, "r+") + expect(fsSync.fchmodSync).toHaveBeenCalledWith(3, 0o444) + const openIdx = vi.mocked(fsSync.openSync).mock.invocationCallOrder[0] + const fchmodIdx = vi.mocked(fsSync.fchmodSync).mock.invocationCallOrder[0] + expect(openIdx).toBeLessThan(fchmodIdx) + expect(fs.rename).toHaveBeenCalledWith(customTempPath, targetPath) + }) + + it("applies the existing target's exact mode to the self-staged temp (umask must not narrow it)", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.statSync).mockReturnValue(_stats(0o664)) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetPath, "data", { platform: "linux" }) + + // openSync's creation mode is narrowed by the process umask (0o664 -> 0o644 with + // the common 0o022), and the rename publishes the temp's mode onto the target, + // so the existing target's mode must be applied on the fd before the commit. + expect(fsSync.fchmodSync).toHaveBeenCalledWith(1, 0o664) + const openIdx = vi.mocked(fsSync.openSync).mock.invocationCallOrder[0] + const fchmodIdx = vi.mocked(fsSync.fchmodSync).mock.invocationCallOrder[0] + expect(openIdx).toBeLessThan(fchmodIdx) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + }) + + it("a failed post-commit directory fsync does not roll the backup back over the published content", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const dirPath = path.dirname(targetPath) + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + // The file fd opens normally; the parent-directory open after the commit + // rename fails, which is the post-commit durability failure. + vi.mocked(fsSync.openSync).mockImplementation((target) => { + if (String(target) === dirPath) throw new Error("EBADF") + return 1 + }) + + await expect(safeWriteText(targetPath, "new data", { backup: true, platform: "linux" })).rejects.toThrow( + PostCommitDurabilityError, + ) + + // The commit rename already published the new content, and the backup was only + // ever a copy: the target was never moved, so there is nothing to rename back. + expect(fs.copyFile).toHaveBeenCalledWith(targetPath, expect.stringContaining("safeWriteText.bak_")) + expect(fs.rename).toHaveBeenCalledTimes(1) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + + // The durability failure is reported, not swallowed - and the backup copy is not + // left beside the target where no caller could find it. + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.bak_")) + }) + + it("does not fchmod the self-staged temp for a fresh target", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + const enoent = Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }) + vi.mocked(fsSync.statSync).mockImplementation(() => { + throw enoent + }) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetPath, "data", { platform: "linux" }) + + // Nothing exists to preserve: the default creation mode is the intended one. + expect(fsSync.fchmodSync).not.toHaveBeenCalled() + }) + }) + + // ── Test 7: symlink handling (Finding 4 regression test) ───────────────── + + describe("symlink handling", () => { + it("a write through a symlink commits onto the resolved referent, never the link path", async () => { + const linkPath = "/tmp/links/link.txt" + const referentPath = "/tmp/targets/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(referentPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(linkPath, "new-content", { platform: "linux" }) + + // The commit rename must target the realpath result (the referent), never the link itself — + // that is what guarantees a write through a symlink replaces the referent's content + // and preserves the link. + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), referentPath) + expect(fs.rename).not.toHaveBeenCalledWith(expect.anything(), linkPath) + }) + + it("when realpath reports ENOENT (target absent), uses the given path as-is", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + // lstat reports the path itself as absent, so this is a new target and + // the fallback is allowed. + vi.mocked(fs.lstat).mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetPath, "data", { platform: "linux" }) + + // rename still happened with the fallback path (path.resolve on /tmp → C:\tmp) + const resolvedFallback = _resolvedTarget(targetPath) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), resolvedFallback) + }) + + it("propagates a dangling symlink instead of writing through the link path", async () => { + // realpath resolves the referent, so a link whose target is missing reports + // ENOENT. Falling back to the link path would replace the symlink with a + // regular file, so the error must propagate and nothing may be committed. + const linkPath = "/tmp/test-dir/dangling-link.txt" + vi.mocked(fs.realpath).mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + const linkStats = Object.create(fsSync.Stats.prototype) as fsSync.Stats + linkStats.isSymbolicLink = () => true + vi.mocked(fs.lstat).mockResolvedValue(linkStats) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await expect(safeWriteText(linkPath, "data", { platform: "linux" })).rejects.toThrow("ENOENT") + + expect(fs.rename).not.toHaveBeenCalled() + }) + }) + + // ── Test 8: review fixes (permissions, partial writes, resolution, durability) ── + + describe("review fixes", () => { + it("preserves the target's restrictive mode and tolerates a failed staging-dir permission repair", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fsSync.statSync).mockReturnValue(_stats(0o600)) + // a pre-existing staging dir may fail its best-effort permission repair + vi.mocked(fsSync.chmodSync).mockImplementationOnce(() => { + throw new Error("EACCES") + }) + + await safeWriteText(targetPath, "secret", { platform: "linux" }) + + // the staging file inherits the target's 0o600 mode and the write commits + expect(fsSync.openSync).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), "w", 0o600) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + }) + + it("falls back to the 0o644 default when the target does not exist yet", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fsSync.statSync).mockImplementation(() => { + throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }) + }) + + await safeWriteText(targetPath, "fresh", { platform: "linux" }) + + expect(fsSync.openSync).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), "w", 0o644) + }) + + it("loops on short writes until the full content is durable before fsync", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const content = "0123456789" // 10 bytes + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + const buffer = Buffer.from(content, "utf8") + // first write (offset 0) reports 4 bytes (short write); the loop continues + vi.mocked(fsSync.writeSync).mockImplementation((...args: unknown[]) => + args[2] === 0 ? 4 : typeof args[3] === "number" ? args[3] : 0, + ) + + await safeWriteText(targetPath, content, { platform: "linux" }) + + // [0,10) reports 4 bytes, then [4,10) writes the remaining 6 + expect(fsSync.writeSync).toHaveBeenCalledTimes(2) + expect(fsSync.writeSync).toHaveBeenNthCalledWith(1, 1, buffer, 0, 10) + expect(fsSync.writeSync).toHaveBeenNthCalledWith(2, 1, buffer, 4, 6) + expect(fsSync.fsyncSync).toHaveBeenCalledWith(1) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + }) + + it("stops instead of looping when a write makes no progress at all", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const content = "abcdefg" // 7 bytes + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // Every write reports zero bytes. The short-write loop is bounded by + // offset < buffer.length only, so a zero return would spin on the same + // offset forever, holding the fd and the event loop. + vi.mocked(fsSync.writeSync).mockReturnValue(0) + + const error = await safeWriteText(targetPath, content, { + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(Error) + expect((error as Error).message).toContain("stalled after 0 of 7 bytes") + // The stalled staging file never reached durability or the commit. + expect(fsSync.fsyncSync).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + // The finally around the write closes the descriptor, so a stalled write + // strands neither the fd nor the process. + expect(fsSync.closeSync).toHaveBeenCalledWith(1) + }) + + it("fsyncs the parent directory after the commit rename on POSIX", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + // temp fd=1 then parent-dir fd=2 - distinct fds prove the ordering + vi.mocked(fsSync.openSync).mockReturnValueOnce(1).mockReturnValue(2) + + await safeWriteText(targetPath, "data", { platform: "linux" }) + + // the directory fsync (fd 2) happens only after the file fsync (fd 1); + // the dir path assertion is path-agnostic (stringContaining) because + // path.dirname renders the same input differently on Windows + expect(fsSync.openSync).toHaveBeenCalledWith(expect.stringContaining("test-dir"), "r") + expect(fsSync.fsyncSync).toHaveBeenNthCalledWith(1, 1) + expect(fsSync.fsyncSync).toHaveBeenNthCalledWith(2, 2) + expect(fsSync.closeSync).toHaveBeenCalledWith(2) + }) + + it("reports a failed parent-directory fsync instead of claiming a durable write", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync) + .mockReturnValueOnce(1) + .mockImplementationOnce(() => { + throw new Error("EBADF") + }) + + // The content rename committed, so the caller can still find the data at + // the target; what the write cannot claim is that the directory entry + // reached the disk. Returning success here would claim durability the + // filesystem did not grant. + await expect(safeWriteText(targetPath, "data", { platform: "linux" })).rejects.toThrow( + PostCommitDurabilityError, + ) + + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + }) + + it("propagates realpath errors (EACCES and code-less) instead of the fallback path", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const eacces = Object.assign(new Error("EACCES: permission denied"), { code: "EACCES" }) + vi.mocked(fs.realpath).mockRejectedValueOnce(eacces) + await expect(safeWriteText(targetPath, "data", { platform: "linux" })).rejects.toBe(eacces) + expect(fs.rename).not.toHaveBeenCalled() + + const plain = new Error("resolution failed") + vi.mocked(fs.realpath).mockRejectedValueOnce(plain) + await expect(safeWriteText(targetPath, "data", { platform: "linux" })).rejects.toBe(plain) + expect(fs.rename).not.toHaveBeenCalled() + }) + + it("backup:true propagates access errors (EACCES and code-less) instead of skipping the backup", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const eacces = Object.assign(new Error("EACCES"), { code: "EACCES" }) + const plain = new Error("access failed") + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // each write accesses dirPath then target; only the target access rejects + const rejectTarget = (error: Error) => async (p: unknown) => { + if (typeof p === "string" && p.endsWith("target.txt")) throw error + } + vi.mocked(fs.access) + .mockImplementationOnce(rejectTarget(eacces)) + .mockImplementationOnce(rejectTarget(eacces)) + .mockImplementationOnce(rejectTarget(plain)) + .mockImplementationOnce(rejectTarget(plain)) + + await expect(safeWriteText(targetPath, "data", { backup: true, platform: "linux" })).rejects.toEqual( + expect.objectContaining({ code: "EACCES" }), + ) + await expect(safeWriteText(targetPath, "data", { backup: true, platform: "linux" })).rejects.toThrow( + "access failed", + ) + expect(fs.rename).not.toHaveBeenCalled() + }) + }) + + it("a failed post-commit directory fsync never unlinks the committed target", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const dirPath = path.dirname(targetPath) + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockImplementation((target) => { + if (String(target) === dirPath) throw new Error("EBADF") + return 1 + }) + + await expect(safeWriteText(targetPath, "new data", { backup: true, platform: "linux" })).rejects.toThrow( + PostCommitDurabilityError, + ) + + // The commit state is what cleanup is allowed to act on: the staging file may be + // unlinked only while it is still the staging file. Once the rename has committed, + // the only path this write owns at the target is the published content itself, and + // removing it would turn a durability warning into data loss. + expect(fs.unlink).not.toHaveBeenCalledWith(targetPath) + }) + + it("names the commit rather than a rename when a no-replace publish commits by hard link and the directory fsync fails", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const dirPath = path.dirname(targetPath) + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockImplementation((target) => { + if (String(target) === dirPath) throw new Error("EBADF") + return 1 + }) + + // failIfExist commits with link(2), not rename(2): the target name is created at + // the target and the staged name is removed afterwards. A durability error that + // claims a rename misdescribes the syscall that committed for a caller logging + // it, so the wording is asserted, not only the error class. + const error = await safeWriteText(targetPath, "new data", { failIfExist: true, platform: "linux" }).catch( + (caught: unknown) => caught, + ) + expect(error).toBeInstanceOf(PostCommitDurabilityError) + expect(fs.link).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + expect(fs.rename).not.toHaveBeenCalled() + expect((error as PostCommitDurabilityError).message).toContain("The commit published the content") + expect((error as PostCommitDurabilityError).message).not.toContain("rename") + }) + + it("a failed post-commit directory fsync does not unlink the staging path this write no longer owns", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const dirPath = path.dirname(targetPath) + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + // A caller-supplied staging file: the name is the caller's, not one this write made + // up, so after the commit it is a path another writer may already be using again. + // The caller's staging file sits beside the target, as this contract requires. + const suppliedTemp = path.join(dirPath, "caller.new_data.json") + vi.mocked(fsSync.openSync).mockImplementation((target) => { + if (String(target) === dirPath) throw new Error("EBADF") + return 1 + }) + + await expect( + safeWriteText(targetPath, "new data", { backup: true, platform: "linux", tempPath: suppliedTemp }), + ).rejects.toThrow(PostCommitDurabilityError) + + // The rename consumed the staging file; the commit is what says so. Unlinking the + // staging NAME afterwards is an unlink of whatever now answers to that name. + expect(fs.unlink).not.toHaveBeenCalledWith(suppliedTemp) + expect(fs.unlink).not.toHaveBeenCalledWith(targetPath) + }) + + describe("content bytes", () => { + const targetPath = "/tmp/enc-dir/target.txt" + + beforeEach(() => { + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + }) + + it("stages UTF-8 bytes for string content", async () => { + await safeWriteText(targetPath, "héllo", { platform: "linux" }) + expect(fsSync.writeSync).toHaveBeenCalledWith(1, Buffer.from("héllo", "utf8"), 0, 6) + }) + + it("publishes caller-supplied bytes unchanged instead of re-encoding them", async () => { + // The extension host encodes a document with VS Code's own codec, which + // covers the legacy code pages and BOMs Node cannot represent, and hands + // the result over: those bytes must reach the commit rename exactly as + // they were given. + const bytes = Buffer.from([0x00, 0x68, 0x00, 0x69]) + await safeWriteText(targetPath, bytes, { platform: "linux" }) + expect(fsSync.writeSync).toHaveBeenCalledWith(1, bytes, 0, 4) + }) + }) + it("a no-replace commit links the staged file into place when the target is absent", async () => { + // Isolate the commit-phase counters from any async cleanup a previous + // test left in flight. + vi.mocked(fs.rename).mockClear() + vi.mocked(fs.unlink).mockClear() + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + + await safeWriteText(targetPath, "new data", { failIfExist: true, platform: "linux" }) + + // link(2) is the only atomic no-replace publish: a rename would displace a + // target that appeared after the caller's absence check. + expect(fs.link).toHaveBeenCalledWith(expect.stringContaining("safeWriteText"), targetPath) + expect(fs.rename).not.toHaveBeenCalled() + // The staged copy is dropped once the name points at it. + expect( + vi.mocked(fs.unlink).mock.calls.some(function (call) { + return String(call[0]).includes("safeWriteText") + }), + ).toBe(true) + }) + + it("a no-replace commit refuses a target that appeared after the caller checked", async () => { + // Isolate the commit-phase counters from any async cleanup a previous + // test left in flight. + vi.mocked(fs.rename).mockClear() + vi.mocked(fs.unlink).mockClear() + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.link).mockRejectedValue(Object.assign(new Error("EEXIST"), { code: "EEXIST" })) + // The failure path still cleans up its staging file and directory; keep those + // promises real so the cleanup does not mask the rejection under test. + vi.mocked(fs.unlink).mockResolvedValue(undefined) + vi.mocked(fs.rmdir).mockResolvedValue(undefined) + + const error = await safeWriteText(targetPath, "new data", { + failIfExist: true, + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(TargetExistsError) + expect((error as TargetExistsError).targetPath).toBe(targetPath) + // Nothing replaced the newer file, and the staged copy is not left behind. + expect(fs.rename).not.toHaveBeenCalled() + expect( + vi.mocked(fs.unlink).mock.calls.some(function (call) { + return String(call[0]).includes("safeWriteText") + }), + ).toBe(true) + }) + + it("resolves an absent target through a symlinked ancestor to the canonical path", async () => { + // The guard pins the publish to the canonical nearest-ancestor path. An absent + // target has to resolve to that same path, or a create through a symlinked + // ancestor aborts with TargetMovedError although nothing moved. + const linkDir = path.resolve("/tmp/link-dir") + const canonicalDir = path.resolve("/real/dir") + const canonical = path.join(canonicalDir, "target.txt") + vi.mocked(fs.realpath).mockImplementation(async (p: unknown) => { + if (String(p) === path.join(linkDir, "target.txt")) { + throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }) + } + if (String(p) === linkDir) return canonicalDir + return String(p) + }) + vi.mocked(fs.lstat).mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + + await safeWriteText(path.join(linkDir, "target.txt"), "new data", { + expectedResolvedPath: canonical, + platform: "linux", + }) + + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText"), canonical) + }) + + it("fails closed when the filesystem has no atomic no-replace primitive", async () => { + // FAT32/exFAT and some SMB mounts reject link(2). Publishing by an exclusive + // copy would put bytes at the final name that readers can observe mid-copy, + // and a crash or an EIO mid-copy leaves damaged bytes there with the + // 'already exists' verdict on retry. No atomic no-replace primitive exists + // on those volumes, so the guarded create refuses to publish at all. + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.link).mockRejectedValue(Object.assign(new Error("ENOTSUP"), { code: "ENOTSUP" })) + + await expect( + safeWriteText(targetPath, "new data", { failIfExist: true, platform: "linux" }), + ).rejects.toBeInstanceOf(UnsupportedFilesystemError) + + // Nothing was published: no copy at the final name, no rename - the name + // stays absent for a retry on a capable volume. + // Every "no hard links here" code takes the same fail-closed path. + for (const code of ["EPERM", "ENOTSUP", "ENOSYS"]) { + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.link).mockRejectedValue(Object.assign(new Error(code), { code })) + + await expect( + safeWriteText(targetPath, "new data", { failIfExist: true, platform: "linux" }), + ).rejects.toBeInstanceOf(UnsupportedFilesystemError) + } + expect(fs.copyFile).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + // Every code reached the link path (the first call plus the three codes + // below), so the loop above really exercised all four refusals. + expect(fs.link).toHaveBeenCalledTimes(4) + // The refusal must not leave its staging file or directory beside the + // target: a regression here would drop a .file-safety-staging_* directory + // on FAT32/exFAT/SMB volumes after every refused create. + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_")) + const stagedDirs = vi.mocked(fsSync.mkdirSync).mock.calls.map((call) => String(call[0])) + expect(vi.mocked(fs.rmdir).mock.calls.map((call) => String(call[0]))).toEqual(stagedDirs) + }) + it("still reports success when the staged copy cannot be unlinked after the link", async () => { + // An antivirus handle on Windows can make unlink fail with EBUSY after link(2) + // published the name. The content is readable at the target, so the write must + // not be reported as a failure the model then retries into an 'already exists'. + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.unlink).mockRejectedValue(Object.assign(new Error("EBUSY"), { code: "EBUSY" })) + + await expect( + safeWriteText(targetPath, "new data", { failIfExist: true, platform: "linux" }), + ).resolves.toBeUndefined() + expect(fs.link).toHaveBeenCalledWith(expect.stringContaining("safeWriteText"), targetPath) + }) + + it("names the orphaned staged file on the warning channel when the unlink after the link fails", async () => { + // Same Windows antivirus case as the test above, now with the residue named instead of + // dropped: the write still resolves, and the caller learns the staged name is still there. + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.unlink).mockRejectedValue(Object.assign(new Error("EBUSY"), { code: "EBUSY" })) + const warnings: { kind: string; path: string; cause: unknown }[] = [] + + await expect( + safeWriteText(targetPath, "new data", { + failIfExist: true, + platform: "linux", + onPostCommitWarning: (warning) => { + warnings.push({ kind: warning.kind, path: warning.path, cause: warning.cause }) + }, + }), + ).resolves.toBeUndefined() + + expect(warnings).toHaveLength(1) + expect(warnings[0]?.kind).toBe("staging-link-orphan") + expect(String(warnings[0]?.path)).toContain("safeWriteText_") + expect((warnings[0]?.cause as { code?: string } | undefined)?.code).toBe("EBUSY") + }) + + it("refuses to report success when the published name is not the staged file", async () => { + // A third party replaced the staging file between the staging checks and link(2), so the + // name link(2) published holds bytes this write never staged. Comparing both names after the + // link is what catches it, and a mismatch is a refusal, not a success. + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.stat).mockImplementation(async (p) => + String(p).includes("safeWriteText_") ? _dirIdentity(11n) : _dirIdentity(12n), + ) + + const error = await safeWriteText(targetPath, "new data", { failIfExist: true, platform: "linux" }).catch( + (caught: unknown) => caught, + ) + + expect(error).toBeInstanceOf(PublishedEntryMismatchError) + expect((error as PublishedEntryMismatchError).targetPath).toBe(targetPath) + expect((error as PublishedEntryMismatchError).message).toContain("both names were left in place") + // Neither name is removed: after a mismatch the staged name may be another process's file + // and the published name is content a reader can already see, so an unlink by pathname here + // could take away an entry this write does not own. + expect(fs.unlink).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + }) + + it("refuses to report success when the published name holds a file from another device", async () => { + // The same refusal through the other half of the identity comparison: same inode number, + // different device. Pinning both halves is the only way to know that neither arm of the + // (dev, ino) test is dead code, which is why this case exists beside the inode case above. + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.stat).mockImplementation(async (p) => + String(p).includes("safeWriteText_") ? _crossIdentity(1n, 11n) : _crossIdentity(2n, 11n), + ) + + const error = await safeWriteText(targetPath, "new data", { failIfExist: true, platform: "linux" }).catch( + (caught: unknown) => caught, + ) + + expect(error).toBeInstanceOf(PublishedEntryMismatchError) + expect((error as PublishedEntryMismatchError).message).toContain("both names were left in place") + expect(fs.unlink).not.toHaveBeenCalled() + }) + + it("refuses to publish when the path no longer resolves to the authorized target", async () => { + // Isolate the commit-phase counters from any async cleanup a previous test left + // in flight. + vi.mocked(fs.rename).mockClear() + vi.mocked(fs.link).mockClear() + const authorized = "/tmp/test-dir/target.txt" + // The caller authorized this path while it was a real file in the workspace; by the + // time the publish resolves it, a local process has swapped in a link to somewhere + // else. The decision was never made about that other file, so nothing is published. + vi.mocked(fs.realpath).mockResolvedValue("/outside/the-victim.txt") + + const error = await safeWriteText(authorized, "new data", { + expectedResolvedPath: authorized, + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(TargetMovedError) + const moved = error as TargetMovedError + expect(moved.authorizedPath).toBe(authorized) + expect(moved.resolvedPath).toBe("/outside/the-victim.txt") + expect(moved.message).toContain("/outside/the-victim.txt") + expect(fs.rename).not.toHaveBeenCalled() + expect(fs.link).not.toHaveBeenCalled() + // Nothing was staged either: the check runs before the staging directory is made. + expect(fs.mkdir).not.toHaveBeenCalled() + }) + + it("refuses to publish when an authorized parent directory was replaced after the check", async () => { + // expectedResolvedPath pins the NAME being published, so it cannot notice the + // directory the name lives in being swapped for another one (a link to outside the + // workspace looks exactly like this). The recorded ancestor identities are what + // catch it, and the commit is aborted before anything is moved into place. + const dir = path.resolve("/tmp/test-dir") + const targetPath = path.join(dir, "target.txt") + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.rename).mockClear() + vi.mocked(fs.link).mockClear() + vi.mocked(fs.stat).mockResolvedValue(_dirIdentity(999n)) // was 100n when authorized + + const error = await safeWriteText(targetPath, "new data", { + expectedResolvedPath: targetPath, + expectedAncestorIdentities: [{ dir, dev: 1n, ino: 100n }], + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(AncestorReplacedError) + expect((error as AncestorReplacedError).directory).toBe(dir) + expect((error as AncestorReplacedError).message).toContain("no longer the directory that was authorized") + expect(fs.rename).not.toHaveBeenCalled() + expect(fs.link).not.toHaveBeenCalled() + // The pre-commit revalidation is no longer the first check to notice: the ancestor + // preflight runs before mkdir, so this write never created the parent, never made a staging + // directory beside the target, and has no staged copy to clean up. + expect(fs.mkdir).not.toHaveBeenCalled() + expect( + vi.mocked(fsSync.mkdirSync).mock.calls.some((call) => String(call[0]).includes(".file-safety-staging")), + ).toBe(false) + expect(fs.unlink).not.toHaveBeenCalled() + }) + + it("refuses before creating anything when an authorized parent was already replaced", async () => { + const dir = path.resolve("/tmp/test-dir") + const targetPath = path.join(dir, "target.txt") + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.stat).mockResolvedValue(_dirIdentity(999n)) // was 100n when authorized + + const error = await safeWriteText(targetPath, "new data", { + expectedResolvedPath: targetPath, + expectedAncestorIdentities: [{ dir, dev: 1n, ino: 100n }], + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(AncestorReplacedError) + expect((error as AncestorReplacedError).directory).toBe(dir) + expect((error as AncestorReplacedError).message).toContain("no longer the directory that was authorized") + // The point of the preflight is the ORDER: mkdir(recursive:true) follows whatever the name + // resolves to at the moment it runs, so a parent that is already unauthorized gets created + // through before any identity comparison happens. Nothing was created and nothing was staged. + expect(fs.mkdir).not.toHaveBeenCalled() + expect(fsSync.mkdirSync).not.toHaveBeenCalled() + expect(fs.unlink).not.toHaveBeenCalled() + expect(fs.link).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + }) + + it("refuses to publish when an authorized ancestor name was moved and replaced by a link", async () => { + // Moving a directory preserves (dev, ino), and replacing the old name with a + // symlink to the moved location keeps the identity check matching - stat + // follows the link. The commit takes path strings and would follow it too, + // publishing at the moved location outside the chain the caller walked, so + // the authorized name must still RESOLVE to the authorized location. + const dir = path.resolve("/tmp/test-dir") + const movedDir = path.resolve("/outside/workspace") + const targetPath = path.join(dir, "target.txt") + vi.mocked(fs.realpath).mockImplementation(async (p) => + path.resolve(String(p)) === dir ? movedDir : targetPath, + ) + vi.mocked(fs.rename).mockClear() + vi.mocked(fs.link).mockClear() + vi.mocked(fs.stat).mockResolvedValue(_dirIdentity(100n)) // SAME identity as authorized + + const error = await safeWriteText(targetPath, "new data", { + expectedResolvedPath: targetPath, + expectedAncestorIdentities: [{ dir, dev: 1n, ino: 100n }], + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(AncestorReplacedError) + expect((error as AncestorReplacedError).message).toContain("no longer resolves to the authorized location") + // Nothing was published at either location. + expect(fs.rename).not.toHaveBeenCalled() + expect(fs.link).not.toHaveBeenCalled() + }) + + it("propagates a transient realpath failure instead of reporting a replaced ancestor", async () => { + // EACCES/EIO while re-validating the authorized ancestry is not a containment + // violation: reporting AncestorReplacedError would point the user (and the + // model) at a security problem that is not there. + const dir = path.resolve("/tmp/test-dir") + const targetPath = path.join(dir, "target.txt") + const denied = Object.assign(new Error("EACCES: permission denied"), { code: "EACCES" }) + vi.mocked(fs.realpath).mockImplementation(async (p) => { + if (path.resolve(String(p)) === dir) throw denied + return targetPath + }) + vi.mocked(fs.stat).mockResolvedValue(_dirIdentity(100n)) + + const error = await safeWriteText(targetPath, "new data", { + expectedResolvedPath: targetPath, + expectedAncestorIdentities: [{ dir, dev: 1n, ino: 100n }], + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBe(denied) + expect(fs.rename).not.toHaveBeenCalled() + expect(fs.link).not.toHaveBeenCalled() + }) + + it("publishes when the recorded ancestor identities are unchanged", async () => { + // The pin must not turn every guarded write into a false rejection: an unchanged + // directory chain publishes normally. + const dir = path.resolve("/tmp/test-dir") + const targetPath = path.join(dir, "target.txt") + // The unchanged chain resolves where it names: the ancestor realpath check + // sees the authorized directory itself, not a moved location. + vi.mocked(fs.realpath).mockImplementation(async (p) => (path.resolve(String(p)) === dir ? dir : targetPath)) + vi.mocked(fs.stat).mockResolvedValue(_dirIdentity(100n)) + + await safeWriteText(targetPath, "new data", { + expectedResolvedPath: targetPath, + expectedAncestorIdentities: [{ dir, dev: 1n, ino: 100n }], + platform: "linux", + }) + + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText"), targetPath) + }) + + it("publishes when a pinned ancestor is a stable symlink alias", async () => { + // A caller that authorized a chain running THROUGH an alias records the alias spelling as + // dir and the location that alias resolves to as resolvedPath. The alias's realpath differs + // from dir by definition while nothing moved, so comparing realpath(dir) against dir alone + // refused a write whose ancestry and target never changed. The (dev, ino) check is what + // still catches the alias being re-pointed; this row is the control on that. + const aliasDir = path.resolve("/tmp/alias-dir") + const realDir = path.resolve("/tmp/real-dir") + const targetPath = path.join(aliasDir, "target.txt") + vi.mocked(fs.realpath).mockImplementation(async (p: unknown) => + path.resolve(String(p)) === aliasDir ? realDir : targetPath, + ) + vi.mocked(fs.stat).mockResolvedValue(_dirIdentity(100n)) + vi.mocked(fs.rename).mockClear() + vi.mocked(fs.link).mockClear() + + await safeWriteText(targetPath, "new data", { + expectedResolvedPath: targetPath, + expectedAncestorIdentities: [{ dir: aliasDir, dev: 1n, ino: 100n, resolvedPath: realDir }], + platform: "linux", + }) + + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText"), targetPath) + }) + + it("refuses the publish when a pinned alias no longer resolves to the authorized location", async () => { + // resolvedPath has to stay a comparison, not an escape hatch: an alias re-pointed to a third + // location resolves to neither the location it was authorized against nor its own spelling, + // and the commit must stop there instead of publishing through the new link. + const aliasDir = path.resolve("/tmp/alias-dir") + const authorizedLocation = path.resolve("/tmp/real-dir") + const repointedTo = path.resolve("/tmp/elsewhere-dir") + const targetPath = path.join(aliasDir, "target.txt") + vi.mocked(fs.realpath).mockImplementation(async (p: unknown) => + path.resolve(String(p)) === aliasDir ? repointedTo : targetPath, + ) + vi.mocked(fs.stat).mockResolvedValue(_dirIdentity(100n)) + vi.mocked(fs.rename).mockClear() + vi.mocked(fs.link).mockClear() + + const error = await safeWriteText(targetPath, "new data", { + expectedResolvedPath: targetPath, + expectedAncestorIdentities: [{ dir: aliasDir, dev: 1n, ino: 100n, resolvedPath: authorizedLocation }], + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(AncestorReplacedError) + expect((error as AncestorReplacedError).message).toContain("no longer resolves to the authorized location") + expect(fs.rename).not.toHaveBeenCalled() + expect(fs.link).not.toHaveBeenCalled() + }) + + it("fsyncs every parent directory it had to create, not only the target's immediate parent", async () => { + // The commit rename makes the entry INSIDE the immediate parent durable. A grandparent this + // call created is itself an entry in ITS parent, so syncing only the immediate parent leaves + // the newer ancestors unsynced: a crash can drop the chain and take the published file with + // it. The double reports the two nested components as created, which is what mkdir answers + // on the real filesystem, so the sync order is observable: deepest first, ending with the + // directory that holds the outermost new entry. Only directories are opened with "r", so + // the calls below are directory syncs and nothing else. + const root = path.resolve("/tmp/test-dir") + const nested = path.join(root, "nested") + const deep = path.join(nested, "deep") + const targetPath = path.join(deep, "target.txt") + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + // The outermost component mkdir had to create, in the spelling Node answers with. + vi.mocked(fs.mkdir).mockResolvedValue(nested) + vi.mocked(fsSync.openSync).mockClear() + // Distinct descriptors per directory, so a flush is attributable to the directory it belongs + // to rather than being one indistinguishable call on an undefined handle. + const dirDescriptors: Record = { [deep]: 901, [nested]: 902, [root]: 903 } + vi.mocked(fsSync.openSync).mockImplementation(function (p: unknown) { + return dirDescriptors[path.resolve(String(p))] ?? 1 + }) + + await safeWriteText(targetPath, "new data", { platform: "linux" }) + + const dirSyncs = vi + .mocked(fsSync.openSync) + .mock.calls.filter(function (call: unknown[]) { + return call[1] === "r" + }) + .map(function (call: unknown[]) { + return String(call[0]) + }) + expect(dirSyncs).toEqual([deep, nested, root]) + // The open list alone only proves handles were taken. The durable effect is the flush: a + // production change that opened each created ancestor and closed it without fsyncing would leave + // this row green while the chain it just created stayed volatile, so each descriptor the open + // list names is pinned to its own flush and its own close, in the same deepest-first order. + const dirFdSet = new Set([901, 902, 903]) + const dirFlushes = vi + .mocked(fsSync.fsyncSync) + .mock.calls.filter(function (call: unknown[]) { + return dirFdSet.has(call[0] as number) + }) + .map(function (call: unknown[]) { + return call[0] + }) + expect(dirFlushes).toEqual([901, 902, 903]) + const dirCloses = vi + .mocked(fsSync.closeSync) + .mock.calls.filter(function (call: unknown[]) { + return dirFdSet.has(call[0] as number) + }) + .map(function (call: unknown[]) { + return call[0] + }) + expect(dirCloses).toEqual([901, 902, 903]) + }) + + // A row about what the Windows filesystem reports, so it belongs to the Windows leg: it is + // gated exactly like the icacls rows in this describe. The marker is stripped by a string + // comparison, but deciding WHICH directories the answer names walks the runner's own path + // semantics, and on POSIX a drive-lettered path like "C:/test-dir/nested" is not even + // absolute, so a marker built that way is an input the platform cannot produce. The option + // keeps the write on the non-win32 durability branch - the branch this row is about - while + // the runner supplies win32 path semantics. The POSIX spelling of the same answer is pinned + // by the preceding row, and the answer this write cannot place by the row after it. + it.skipIf(process.platform !== "win32")( + "maps the namespace marker Windows reports for the directory it created", + async () => { + const nested = path.resolve("C:/test-dir/nested") + const deep = path.join(nested, "deep") + const targetPath = path.join(deep, "target.txt") + const stripped = nested.startsWith("/") ? nested.slice(1) : nested + const reported = "\\\\?\\" + stripped.split("/").join("\\") + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.mkdir).mockResolvedValue(reported) + vi.mocked(fsSync.openSync).mockClear() + + await safeWriteText(targetPath, "new data", { platform: "linux" }) + + const dirSyncs = vi + .mocked(fsSync.openSync) + .mock.calls.filter(function (call: unknown[]) { + return call[1] === "r" + }) + .map(function (call: unknown[]) { + return String(call[0]) + }) + // The marker had to map onto the write's own paths: nothing under the marker is synced. + expect(dirSyncs).toEqual([deep, nested, path.dirname(nested)]) + expect( + dirSyncs.some(function (p: string) { + return p.includes("?") + }), + ).toBe(false) + }, + ) + + it("syncs no ancestor when mkdir names a directory this write cannot place", async () => { + // The chain is derived by walking up from the target's own directory until it reaches what + // mkdir reported. An answer that is not on that walk is not a directory this write created, + // and it must not be synced: flushing a directory someone else owns, while the published + // file's own new ancestors stay unsynced, is the opposite of what step 4b is for. Only the + // immediate parent is flushed, because the commit rename put its entry there. Constructed + // from path.resolve so the answer is absolute, and off the walk, on both platforms. + const targetPath = path.join(path.resolve("/tmp/test-dir/deep"), "target.txt") + const unrelated = path.resolve("/tmp/other-tree/nested") + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.mkdir).mockResolvedValue(unrelated) + vi.mocked(fsSync.openSync).mockClear() + + await safeWriteText(targetPath, "new data", { platform: "linux" }) + + const dirSyncs = vi + .mocked(fsSync.openSync) + .mock.calls.filter(function (call: unknown[]) { + return call[1] === "r" + }) + .map(function (call: unknown[]) { + return String(call[0]) + }) + expect(dirSyncs).toEqual([path.dirname(targetPath)]) + expect( + dirSyncs.some(function (p: string) { + return p.includes(unrelated) + }), + ).toBe(false) + }) + + it("reports a post-commit durability failure together with the backup it cannot remove", async () => { + // The commit rename succeeded and then the parent-directory fsync failed. Nothing is rolled + // back - the published bytes stay at the target - and the failure path still removes the + // backup. When that removal also fails the caller gets both facts: contentPublished says the + // leftover is stale beside published content, and originalError names the durability failure. + const targetPath = "/tmp/test-dir/target.txt" + const eio = Object.assign(new Error("EIO"), { code: "EIO" }) + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + // Only directories are opened with "r", so this fd marks the post-commit directory sync: + // the failure lands exactly after the rename instead of on the staged file's fsync. + vi.mocked(fsSync.openSync).mockImplementation(function (p: unknown, flags?: unknown): number { + return flags === "r" ? 77 : 1 + }) + vi.mocked(fsSync.fsyncSync).mockImplementation(function (fd: number): void { + if (fd === 77) { + throw eio + } + }) + vi.mocked(fs.unlink).mockImplementation(async (p: unknown) => { + if (String(p).includes("safeWriteText.bak_")) { + throw Object.assign(new Error("EPERM"), { code: "EPERM" }) + } + }) + vi.mocked(fs.rename).mockClear() + + const error = await safeWriteText(targetPath, "new data", { + backup: true, + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(OrphanedBackupAfterFailedWriteError) + const orphan = error as OrphanedBackupAfterFailedWriteError + // The commit ran: the durability failure is downstream of the rename, not before it. + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText"), targetPath) + expect(orphan.contentPublished).toBe(true) + expect(orphan.originalError).toBeInstanceOf(PostCommitDurabilityError) + expect((orphan.cleanupError as Error).message).toBe("EPERM") + expect(orphan.message).toContain("the target holds the new content") + // Both removal attempts ran, and the failure path still cleaned what it owned. + const backupUnlinks = vi.mocked(fs.unlink).mock.calls.filter(function (call: unknown[]) { + return typeof call[0] === "string" && call[0].includes("safeWriteText.bak_") + }) + expect(backupUnlinks).toHaveLength(2) + expect(fs.rmdir).toHaveBeenCalled() + }) + + it("refuses the publish when an authorized ancestor disappears before the commit", async () => { + const dir = path.resolve("/tmp/test-dir") + const targetPath = path.join(dir, "target.txt") + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.stat).mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + + const error = await safeWriteText(targetPath, "new data", { + expectedResolvedPath: targetPath, + expectedAncestorIdentities: [{ dir, dev: 1n, ino: 100n }], + platform: "linux", + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(AncestorReplacedError) + expect((error as AncestorReplacedError).message).toContain("no longer exists") + expect(fs.rename).not.toHaveBeenCalled() + expect(fs.link).not.toHaveBeenCalled() + }) +}) + +// ── Test 12: lock key, staging path, and post-commit durability ───────────── + +describe("resolveLockKey", () => { + beforeEach(() => mockDefaults()) + + it("canonicalizes the parent directory, not just the file", async () => { + vi.mocked(fs.realpath).mockImplementation(async (target) => { + const key = String(target) + if (key === "/tmp/linkdir/file.json") return "/real/dir/file.json" + if (key === "/real/dir") return "/real/dir" + return key + }) + + // The key is the canonical directory plus the basename, so a symlinked + // ancestor and its referent share one lock. + await expect(resolveLockKey("/tmp/linkdir/file.json")).resolves.toBe(path.join("/real/dir", "file.json")) + }) + + it("computes a key for a dangling link, which resolvePublishTarget refuses", async () => { + const enoent = Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }) + vi.mocked(fs.realpath).mockRejectedValue(enoent) + vi.mocked(fs.lstat).mockResolvedValue(_fileStats(true)) + // The walk ends when readlink throws, because a plain file is not a link. + // Answering every call with the same value would let a walk that never + // stopped return the same key, so the mock has to distinguish the link from + // the referent and the test has to check that the walk stopped. + const notALink = Object.assign(new Error("EINVAL: not a link"), { code: "EINVAL" }) + vi.mocked(fs.readlink).mockImplementation(async (target) => { + if (String(target) === "/tmp/linkdir/file.json") return "referent.json" + throw notALink + }) + + // Mid-commit a peer writer renames the referent away and back, so the key + // must still be computable while the link dangles. + await expect(resolveLockKey("/tmp/linkdir/file.json")).resolves.toBe( + path.resolve(path.join("/tmp/linkdir", "referent.json")), + ) + // Two calls means the walk stopped at the referent: the link, then one more + // on the referent that reports it is not a link. A walk that never stopped + // would run the full 8 steps. + expect(fs.readlink).toHaveBeenCalledTimes(2) + }) + + it("terminates on a two-link cycle instead of walking forever", async () => { + const enoent = Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }) + vi.mocked(fs.realpath).mockRejectedValue(enoent) + vi.mocked(fs.lstat).mockResolvedValue(_fileStats(true)) + // Every readlink answers with the same link, so an unbounded walk would + // never end; the bounded walk returns the key it actually reached. + vi.mocked(fs.readlink).mockImplementation(async () => "a.json") + + await expect(resolveLockKey("/tmp/linkdir/a.json")).resolves.toBe( + path.resolve(path.join("/tmp/linkdir", "a.json")), + ) + expect(fs.readlink).toHaveBeenCalledTimes(8) + }) +}) + +describe("caller-supplied staging path", () => { + beforeEach(() => mockDefaults()) + + it("rejects a staging file outside the target's directory before writing anything", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + + // A rename across filesystems fails with EXDEV, and a path elsewhere lets + // a caller publish an unrelated file onto the target. + await expect( + safeWriteText(targetPath, "data", { tempPath: "/tmp/other-dir/x.tmp", platform: "linux" }), + ).rejects.toThrow(StagingPathError) + expect(fsSync.openSync).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + }) + + it("rejects a staging path that is a symlink", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.lstat).mockResolvedValue(_fileStats(true)) + + // Renaming a link over the target publishes whatever the link points at. + await expect( + safeWriteText(targetPath, "data", { tempPath: "/tmp/test-dir/x.tmp", platform: "linux" }), + ).rejects.toThrow(StagingPathError) + expect(fsSync.openSync).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + }) + + it("rejects a staging path that is the target itself", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + // Same inode and device for the supplied staging path and the target: the + // failure handler would unlink the only copy of the content, so a failed + // write would delete the file it was meant to protect. + const stats = _fileIdentity(42n, 7n) + vi.mocked(fs.lstat).mockResolvedValue(stats) + + await expect(safeWriteText(targetPath, "data", { tempPath: targetPath, platform: "linux" })).rejects.toThrow( + StagingPathError, + ) + expect(fsSync.openSync).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + // The comparison is only sound when both stats are read as bigint: on NTFS/ReFS the file + // identifiers exceed Number.MAX_SAFE_INTEGER. + // Filter on the options, not the spelling: path.resolve prefixes a drive letter on Windows, + // so the two identity reads are the calls that asked for options at all. + const identityLookups = vi.mocked(fs.lstat).mock.calls.filter((c) => c[1] !== undefined) + expect(identityLookups.length).toBeGreaterThanOrEqual(2) + for (const c of identityLookups) { + expect(c[1]).toEqual({ bigint: true }) + } + expect(fs.unlink).not.toHaveBeenCalled() + }) + + it("rejects when the target identity cannot be compared for a reason other than a missing target", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + // A hard-linked staging file shares the target's inode, so the identity comparison is the only thing + // between this write and a rename onto the very file the guard protects. An EACCES from the target + // lstat must not be mistaken for "there is no target". + const stagingStats = _fileIdentity(42n, 7n) + vi.mocked(fs.lstat).mockImplementation(async (p) => { + if (String(p) === targetPath) { + throw Object.assign(new Error("EACCES"), { code: "EACCES" }) + } + return stagingStats + }) + + await expect( + safeWriteText(targetPath, "data", { tempPath: "/tmp/test-dir/hardlink.txt", platform: "linux" }), + ).rejects.toThrow("Staging file could not be compared with the target") + expect(fsSync.openSync).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + // Both identity reads must ask for bigint stats, or the comparison silently falls + // back to rounded numbers on NTFS/ReFS. + const identityLookups = vi.mocked(fs.lstat).mock.calls.filter((c) => c[1] !== undefined) + expect(identityLookups).toHaveLength(2) + for (const c of identityLookups) { + expect(c[1]).toEqual({ bigint: true }) + } + }) +}) + +describe("cleanup before a rollback failure is reported", () => { + beforeEach(() => mockDefaults()) + + it("releases the staged file, its copy and its own staging directory before throwing", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // The commit rename is the only rename in this flow and it fails. + vi.mocked(fs.rename).mockRejectedValue(new Error("ENOSPC")) + + await expect(safeWriteText(targetPath, "data", { backup: true, platform: "linux" })).rejects.toThrow("ENOSPC") + + // The staging file and this write's own directory must not leak, and neither may + // the backup copy: the target still holds the pre-write content on disk. + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_")) + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.bak_")) + const stagingDirs = vi.mocked(fsSync.mkdirSync).mock.calls.map((call) => String(call[0])) + expect(stagingDirs.length).toBe(1) + expect(fs.rmdir).toHaveBeenCalledWith(stagingDirs[0]) + + const failingRenameOrder = vi.mocked(fs.rename).mock.invocationCallOrder[0] + const unlinkOrder = vi.mocked(fs.unlink).mock.invocationCallOrder[0] + const rmdirOrder = vi.mocked(fs.rmdir).mock.invocationCallOrder[0] + expect(unlinkOrder).toBeGreaterThan(failingRenameOrder) + expect(rmdirOrder).toBeGreaterThan(failingRenameOrder) + }) +}) + +describe("resolvePublishTarget", () => { + beforeEach(() => mockDefaults()) + + it("propagates an lstat failure that is not ENOENT instead of falling back to the link path", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const enoent = Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }) + const eacces = Object.assign(new Error("EACCES: permission denied"), { code: "EACCES" }) + vi.mocked(fs.realpath).mockRejectedValue(enoent) + vi.mocked(fs.lstat).mockRejectedValue(eacces) + + // A failed lstat says nothing about whether the path is a link, so the + // fallback would publish through a link we were not allowed to inspect. + await expect(safeWriteText(targetPath, "data", { platform: "linux" })).rejects.toBe(eacces) + expect(fs.rename).not.toHaveBeenCalled() + }) + + it("still falls back to the given path when lstat also reports the path as absent", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const enoent = Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }) + vi.mocked(fs.realpath).mockRejectedValue(enoent) + vi.mocked(fs.lstat).mockRejectedValue(enoent) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + await safeWriteText(targetPath, "data", { platform: "linux" }) + + // The fallback is the resolved path, not the string that was handed in. + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), path.resolve(targetPath)) + }) +}) + +describe("resolveLockKey when the parent directory does not exist yet", () => { + beforeEach(() => mockDefaults()) + + it("canonicalizes through the nearest existing ancestor instead of the literal parent", async () => { + // Two writers must take ONE lock: the one whose parent directory is already there, + // and the one racing to create it. Resolving only the immediate parent and falling + // back to its literal spelling on ENOENT gave them different keys whenever an + // ancestor was a symlink or a Windows short name, so a read-modify-write under the + // advisory lock lost one side. + const aliasDir = path.resolve("/tmp/alias-parent") + const canonicalDir = path.resolve("/tmp/real-parent") + const nested = path.join(aliasDir, "nested") + const target = path.join(nested, "history_item.json") + vi.mocked(fs.lstat).mockResolvedValue(_fileStats(false)) + vi.mocked(fs.realpath).mockImplementation(async (p) => { + const s = String(p) + if (s === target || s === nested) { + throw Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }) + } + if (s === aliasDir) { + return canonicalDir + } + return s + }) + + expect(await resolveLockKey(target)).toBe(path.join(canonicalDir, "nested", "history_item.json")) + }) + + it("propagates a realpath failure that is not ENOENT instead of guessing a key", async () => { + // A realpath that fails for another reason says nothing about the canonical form; + // returning a literal key would silently put this writer on a different lock. + const target = path.join(path.resolve("/tmp/test-dir"), "history_item.json") + vi.mocked(fs.lstat).mockResolvedValue(_fileStats(false)) + // Not a link: the walk must reach the canonicalization step, which is where the + // non-ENOENT failure has to surface. + vi.mocked(fs.readlink).mockRejectedValue( + Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }), + ) + vi.mocked(fs.realpath).mockImplementation(async (p) => { + if (String(p) === target) return target + throw Object.assign(new Error("EACCES: permission denied"), { code: "EACCES" }) + }) + + await expect(resolveLockKey(target)).rejects.toThrow("EACCES") + }) +}) + +describe("post-commit steps after a published write", () => { + beforeEach(() => vi.clearAllMocks()) + + it("does not let a throwing warning callback unlink a published staging name", async () => { + // The staged name is removed best-effort after the link, and the residue is reported on + // the warning channel. A caller whose listener throws must learn about the throw, but the + // write has already published its content: the failure handler may not treat it as an + // uncommitted write and unlink the staging name a second time underneath the caller. + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.unlink).mockRejectedValue(Object.assign(new Error("EBUSY"), { code: "EBUSY" })) + const callbackError = new Error("warning listener threw") + + await expect( + safeWriteText(targetPath, "new data", { + failIfExist: true, + platform: "linux", + onPostCommitWarning: () => { + throw callbackError + }, + }), + ).rejects.toBe(callbackError) + + // Once for the post-commit removal. A second unlink is the failure handler cleaning up a + // write it wrongly believes is uncommitted. + expect(fs.unlink).toHaveBeenCalledTimes(1) + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_")) + }) + + it("refuses a caller-supplied staging file that changed identity between validation and open", async () => { + // The staging path is authorized by lstat and re-opened by name for the chmod and the + // fsync. A third party that renames another file onto that name in between would have its + // bytes chmod'd, fsync'd and published under this target. The commit must compare the + // descriptor's own identity against the one that was authorized. + const targetPath = "/tmp/test-dir/target.txt" + const customTempPath = "/tmp/test-dir/custom-temp.tmp" + // The SUT lstats the resolved spelling of the staging path, which carries a drive letter on + // Windows, so the fixture branches on the same resolution rather than on the literal. + const stagingSpelling = path.resolve(customTempPath) + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.lstat).mockImplementation(async (p) => { + // The staging file as validated, and a target with its own identity so the + // "staging must not be the target" check is not what fires here. + return String(p) === stagingSpelling ? _fileIdentity(7n, 700n) : _fileIdentity(7n, 777n) + }) + vi.mocked(fsSync.openSync).mockReturnValue(3) + // The entry behind the staging name is now a different file. + vi.mocked(fsSync.fstatSync).mockReturnValue(_fileIdentity(7n, 999n)) + + await expect(safeWriteText(targetPath, "", { tempPath: customTempPath, platform: "linux" })).rejects.toThrow( + "Staging file was replaced between validation and open", + ) + + expect(fsSync.openSync).toHaveBeenCalledWith(customTempPath, "r+") + // Refused before acting on the substituted entry or publishing it. + expect(fsSync.fchmodSync).not.toHaveBeenCalled() + expect(fsSync.fsyncSync).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + expect(fs.link).not.toHaveBeenCalled() + }) +}) diff --git a/src/services/file-safety/safeWriteText.ts b/src/services/file-safety/safeWriteText.ts new file mode 100644 index 0000000000..3b36858433 --- /dev/null +++ b/src/services/file-safety/safeWriteText.ts @@ -0,0 +1,1235 @@ +import * as fs from "fs/promises" +import * as fsSync from "fs" +import * as path from "path" +import { execFile } from "child_process" + +import { getErrorCode } from "../../utils/errorCode" + +export interface SafeWriteTextOptions { + /** + * + * When true, keep a durable copy of the previous content at a sibling + * safeWriteText.bak_* file for the duration of the write: the target is copied + * before the commit rename and the copy is removed after it. The copy is never + * a rollback source - nothing is restored on failure, because the copy never + * displaced the target: before the commit the target still holds its previous + * content, and after it the target holds the new content and this call removes + * the copy. A removal that fails AFTER the commit reports OrphanedPublishedBackupError + * and leaves the new content at the target: nothing is rolled back, because rolling back + * would replace published bytes with an older copy the caller never asked to restore. A + * removal that fails after a write that never committed reports + * OrphanedBackupAfterFailedWriteError, whose contentPublished says which of the two + * happened. When false (default) the atomic rename simply replaces the target -- + * crash-safe window is zero. + */ + backup?: boolean + + /** + * Platform override for testing. When omitted the real process.platform + * value is used. Set to "win32" or "linux" / "darwin" from tests so that + * both branches are reachable without needing a real Windows runner. + */ + platform?: string + + /** + * Custom execFile runner for testing (e.g. vi.fn). When omitted the real + * child_process.execFile is used. + */ + execFileRunner?: typeof execFile + + /** + * Pre-written temp path to use for the commit phase. When provided, + * safeWriteText skips creating its own staging file and uses this path + * instead (it still fsyncs before rename). Useful when a caller has + * already written data to a temp file via a custom stream. + */ + tempPath?: string + + /** + * Commit only if the target name does NOT exist at commit time. + * rename(2) has no no-replace form - it silently displaces a target that another + * writer created after the caller's absence check - so the commit is done with + * link(2) instead, which fails EEXIST for an existing name (and does not follow a + * symlink placed at that name). A conditional create therefore cannot overwrite a + * file it never saw. + */ + failIfExist?: boolean + + /** + * The publish target the caller already authorized. + * A caller that checks workspace containment and then calls this primitive resolves + * the path twice: once for its own decision and once here. If a link is swapped in + * between the two, the decision was about a different file than the one published. + * Passing the authorized value makes that drift fatal instead of silent. + */ + expectedResolvedPath?: string + + /** + * The directories the caller walked when it authorized this target, with the + * (dev, ino) identity each had at that moment. + * + * expectedResolvedPath pins the NAME this write publishes; it cannot see a parent + * directory being replaced by a link between the caller's containment check and the + * commit. Re-checking the ancestor identities immediately before the commit makes a + * swapped parent fatal too: the publish then aborts instead of writing through the new + * link. Node has no descriptor-relative rename, so a swap that lands after this check + * and before the rename is not eliminable here - it narrows the window to the commit + * itself rather than the whole guard. + * The identity check is paired with a realpath comparison: moving a directory + * preserves (dev, ino), and replacing its old name with a symlink to the moved + * location keeps stat() matching while sending the commit to the new location. + * The name must still resolve to the location it was authorized against - the + * recorded resolvedPath when the caller walked a symlink alias, and dir itself + * when it recorded a plain spelling - not merely to the same inode. + */ + expectedAncestorIdentities?: DirectoryIdentity[] + /** + * A post-commit step failed while the content this call published stayed readable: either the staged name + * could not be removed after a no-replace commit, or the win32 DACL restore failed. The write + * resolves in both cases - failing it would report a failure for bytes the caller can now read + * back - and the residue is reported here instead of dropped. Kinds are spelled inline rather + * than exported: no production caller selects on them today, and an export whose only reader + * is a spec is dead API. Omitting the callback means this call chose not to hear: the residue is + * still where it was and the write still resolves, so the absence of a report is not evidence + * that there was none. + */ + onPostCommitWarning?: (warning: { + kind: "staging-link-orphan" | "dacl-restore-failed" + path: string + cause: unknown + }) => void +} + +/** One directory's on-disk identity, read with bigint stats so NTFS 64-bit values survive. */ +export type DirectoryIdentity = { + dir: string + dev: bigint + ino: bigint + /** + * Where `dir` resolved to when this identity was taken (its realpath). + * + * A caller that authorized a chain running THROUGH a symlink alias records the alias + * spelling as `dir`; that alias's realpath legitimately differs from `dir` while nothing + * moved, so comparing the current realpath against `dir` alone refuses a write whose + * ancestry never changed. The pre-commit revalidation compares against this value when it + * is present, and against `dir` when it is not. + */ + resolvedPath?: string +} + +/** + * The backup copy could not be created AND the partial copy could not be removed. + * The write failed either way, but the leftover is a copy of the previous content that + * is still on disk: its path travels on the error so the caller can remove it, instead + * of the cleanup silently discarding the only reference to it. + */ +export class OrphanedBackupError extends Error { + readonly orphanedBackupPath: string + readonly originalError: unknown + readonly cleanupError: unknown + constructor(backupPath: string, targetPath: string, cause: unknown, cleanupError: unknown) { + super(_orphanedBackupMessage(targetPath, backupPath, cause, cleanupError), { cause }) + this.name = "OrphanedBackupError" + this.orphanedBackupPath = backupPath + this.originalError = cause + this.cleanupError = cleanupError + } +} + +/** + * The write committed and the content is published, but the pre-write backup could not be + * removed after two attempts (a transient handle - antivirus scanner, indexer - usually + * causes this). Returning success would hide a stale copy of the previous content that + * only a recovery tool can find, so the path travels on this error instead. + */ +export class OrphanedPublishedBackupError extends Error { + readonly publishedPath: string + readonly orphanedBackupPath: string + readonly cleanupError: unknown + + constructor(targetPath: string, backupPath: string, cleanupError: unknown) { + const reason = cleanupError instanceof Error ? cleanupError.message : String(cleanupError) + super( + `safeWriteText: the content was published at ${targetPath}, but the pre-write copy at ${backupPath} ` + + `could not be removed (${reason}). The copy is still on disk and should be removed by the caller or a recovery tool.`, + { cause: cleanupError }, + ) + this.name = "OrphanedPublishedBackupError" + this.publishedPath = targetPath + this.orphanedBackupPath = backupPath + this.cleanupError = cleanupError + } +} + +/** + * A write that is already failing left its complete backup copy on disk because removing it + * failed twice. Reported separately from OrphanedPublishedBackupError, which says the write + * SUCCEEDED and only the copy is leftover: here the write's own failure is the primary fact + * and is kept as originalError (and as this error's cause), cleanupError says why the copy is + * still there, and contentPublished says which of the two meanings the leftover has - a stale + * copy beside published content, or a duplicate of the content the target still holds. + */ +export class OrphanedBackupAfterFailedWriteError extends Error { + readonly targetPath: string + readonly orphanedBackupPath: string + readonly originalError: unknown + readonly cleanupError: unknown + readonly contentPublished: boolean + + constructor( + targetPath: string, + backupPath: string, + cause: unknown, + cleanupError: unknown, + contentPublished: boolean, + ) { + const reason = (error: unknown): string => (error instanceof Error ? error.message : String(error)) + super( + `safeWriteText: the write to ${targetPath} failed (${reason(cause)}), and the backup copy at ${backupPath} ` + + `could not be removed (${reason(cleanupError)}). The copy of the previous content is still on disk; ` + + (contentPublished + ? `the target holds the new content, so the copy is stale and must be removed.` + : `the target still holds the previous content, so the copy duplicates it and must be removed.`), + { cause }, + ) + this.name = "OrphanedBackupAfterFailedWriteError" + this.targetPath = targetPath + this.orphanedBackupPath = backupPath + this.originalError = cause + this.cleanupError = cleanupError + this.contentPublished = contentPublished + } +} + +function _orphanedBackupMessage(targetPath: string, backupPath: string, cause: unknown, cleanupError: unknown): string { + const reason = (error: unknown): string => (error instanceof Error ? error.message : String(error)) + return ( + `safeWriteText: could not create the backup of ${targetPath} (${reason(cause)}), and the ` + + `partial copy at ${backupPath} could not be removed (${reason(cleanupError)}). The copy is ` + + `still on disk and must be deleted.` + ) +} + +/** + * A caller-supplied staging path that is not a file this write may publish: it + * sits outside the target's directory (so the commit rename would cross + * filesystems) or is not a regular file. Rejecting it before any write keeps the + * target from being replaced by whatever the path points at. + */ +export class StagingPathError extends Error { + readonly stagingPath: string + + constructor(message: string, stagingPath: string) { + super(message) + this.name = "StagingPathError" + this.stagingPath = stagingPath + } +} + +/** + * The commit succeeded - a rename, or a hard link under failIfExist - but the parent-directory fsync did not, so the + * directory entry is not known to be durable. The content is at the target; the + * caller cannot assume it survives a crash. Reported as its own error so a + * successful return never claims durability the filesystem did not grant. + */ +export class PostCommitDurabilityError extends Error { + readonly targetPath: string + + constructor(targetPath: string, cause: unknown, copiedTargetFsync = false) { + super( + copiedTargetFsync + ? "The exclusive copy committed but the copied target could not be fsynced -- the content is at the target path reported on this error, and it may not survive a crash on a volume without hard links." + : "The commit published the content but the parent directory could not be fsynced -- the content is at the target path reported on this error, and the directory entry may not be durable.", + { cause }, + ) + this.name = "PostCommitDurabilityError" + this.targetPath = targetPath + } +} + +/** + * The target exists but its access rights could not be inspected or saved, so publishing would + * replace it with a file that inherits different rights. On Windows the publish is refused before + * anything is committed: a save that silently widened who can read the file is not a save the + * caller can trust, and nothing has been published yet, so the target still holds its content. + * causeError carries the platform failure behind the refusal - the access check + * error or the icacls error - so a caller can tell a command that ran and refused + * from one that could not be started at all; it is the runner's own error, not a + * summary of it. + */ +export class DaclInspectionError extends Error { + constructor( + readonly targetPath: string, + readonly phase: "inspect" | "save", + readonly causeError: unknown, + ) { + const detail = phase === "save" ? "its DACL could not be saved" : "its access rights could not be checked" + super(`safeWriteText: refusing to publish over ${targetPath} because ${detail}`) + this.name = "DaclInspectionError" + } +} + +/** + * A conditional create (failIfExist) found a target at commit time. The write is + * refused without touching it: whatever is at the path was created by someone else + * and stays exactly as it is. + */ +export class TargetExistsError extends Error { + readonly targetPath: string + constructor(targetPath: string) { + super(`A file already exists at ${targetPath} -- the no-replace commit refused it.`) + this.name = "TargetExistsError" + this.targetPath = targetPath + } +} + +/** + * A conditional create (failIfExist) found that the volume hosting the target has + * no atomic no-replace primitive: link(2) reported that hard links do not exist + * here, and publishing by copy would put bytes at the final name that readers can + * observe mid-copy. Nothing is published - the write fails closed so a retry on a + * capable volume (or a caller that accepts non-atomic publication) decides. + */ +export class UnsupportedFilesystemError extends Error { + readonly targetPath: string + constructor(targetPath: string, cause: unknown) { + super( + `Filesystem hosting ${targetPath} has no atomic no-replace commit primitive -- the guarded create refused to publish by copy.`, + { cause }, + ) + this.name = "UnsupportedFilesystemError" + this.targetPath = targetPath + } +} + +/** + * The path resolved to something other than the target the caller authorized. + * Nothing is staged or published: a link swapped in after the caller's containment + * check would otherwise send the write to a file that check never covered. + */ +export class TargetMovedError extends Error { + readonly authorizedPath: string + readonly resolvedPath: string + constructor(authorizedPath: string, resolvedPath: string) { + super( + `The write was authorized for ${authorizedPath}, but that path now resolves to ${resolvedPath} -- nothing was published.`, + ) + this.name = "TargetMovedError" + this.authorizedPath = authorizedPath + this.resolvedPath = resolvedPath + } +} + +/** + * A directory the caller walked on its way to the authorized target is no longer the + * directory it was: it was removed, replaced, or turned into a link to somewhere else. + * Publishing through it would act on a decision that was never made about the file now + * reachable there, so the commit is aborted. + */ +export class AncestorReplacedError extends Error { + readonly directory: string + constructor(directory: string, reason: string) { + super(`A directory on the authorized path (${directory}) ${reason} -- nothing was published.`) + this.name = "AncestorReplacedError" + this.directory = directory + } +} +/** + * The target name does not hold the file this write staged. + * + * A third party replaced the staging file between the staging checks and link(2), so the name now + * published is bytes this write never staged. Nothing is removed: after the link both names may + * belong to somebody else, and an unlink by pathname can take away an entry this write does not + * own. The two identities travel on the error because the only useful next step is comparing them. + */ +export class PublishedEntryMismatchError extends Error { + readonly targetPath: string + constructor(targetPath: string, publishedIdentity: string, stagedIdentity: string) { + super( + `The published entry (${targetPath}) is not the staged file (published ${publishedIdentity}, staged ${stagedIdentity}); both names were left in place.`, + ) + this.name = "PublishedEntryMismatchError" + this.targetPath = targetPath + } +} +// -- helpers --------------------------------------------------------------- + +/** + * Reject an unauthorized parent BEFORE mkdir walks through it. + * + * mkdir(recursive:true) follows whatever parent chain the name currently resolves to, and the + * step 2b revalidation below runs after it, so a parent swapped for a link to outside the + * authorized chain gets created through before anything compares identities. Only an ancestor that + * is already present is compared: an ENOENT here is a directory this call is about to create, not a + * containment violation, and the recorded identity for it cannot be checked against nothing. Step + * 2b stays in place: it is what catches a swap in the window between this check and the commit. + */ +async function _preflightAncestors(identities: DirectoryIdentity[] | undefined): Promise { + if (!identities) { + return + } + for (const expected of identities) { + const current = await fs.stat(expected.dir, { bigint: true }).catch((error: unknown) => { + if (getErrorCode(error) === "ENOENT") { + return null + } + throw error + }) + if (current !== null && (current.dev !== expected.dev || current.ino !== expected.ino)) { + throw new AncestorReplacedError(expected.dir, "is no longer the directory that was authorized") + } + } +} + +/** fsync one directory so the entry it holds (a rename, or a newly created child) is durable. */ +function _syncDirectory(dir: string): void { + const dirFd = fsSync.openSync(dir, "r") + try { + fsSync.fsyncSync(dirFd) + } finally { + fsSync.closeSync(dirFd) + } +} + +/** + * The directories *mkdir(recursive:true)* had to create for *dirPath*, outermost first. + * + * Node reports the outermost component it created - or nothing when the path already existed - + * and that answer is the whole input, so no extra stat is needed to learn what appeared. The + * commit rename only makes the entry INSIDE the immediate parent durable, and every newly + * created directory is itself an entry in its own parent, so step 4b fsyncs this chain (deepest + * first, after the immediate parent) and then the directory that first links it into the tree; + * otherwise a crash can drop an ancestor and with it the published file. + * + * On Windows the reported path carries the extended-length namespace marker, which is stripped + * so the chain compares in the spelling the write itself works in. An answer that is not on + * this path yields nothing, rather than syncing directories this write cannot place. + */ +function _createdAncestorChain(createdRoot: string | undefined, dirPath: string): string[] { + if (typeof createdRoot !== "string") return [] + const namespacePrefix = "\\\\?\\" + const unprefixed = createdRoot.startsWith(namespacePrefix) ? createdRoot.slice(namespacePrefix.length) : createdRoot + const root = path.resolve(unprefixed) + const chain: string[] = [] + let cursor = dirPath + for (;;) { + chain.push(cursor) + if (cursor === root) return chain.reverse() + const parent = path.dirname(cursor) + if (parent === cursor) return [] + cursor = parent + } +} + +/** Generate a unique temp file name in the given directory. */ +function _tempName(dir: string, prefix: string): string { + return path.join(dir, "." + prefix + "_" + Date.now() + "_" + Math.random().toString(36).substring(2) + ".tmp") +} + +/** Create a private per-write staging sub-directory inside *dir*. The name is + * unique per write, so concurrent writes never collide on their temp names and + * never remove a staging directory another write is still using: with one shared + * name, one write's best-effort rmdir could delete the directory another write + * had just created but not yet opened, failing its openSync with ENOENT. */ +function _stagingDir(dir: string): string { + const sd = path.join(dir, ".file-safety-staging_" + Date.now() + "_" + Math.random().toString(36).substring(2)) + // mode:0o700 protects a freshly created staging dir; the best-effort chmod + // repairs a pre-existing one (mkdirSync with recursive:true never chmods an + // existing directory), so staged temp files are never group/world readable. + fsSync.mkdirSync(sd, { recursive: true, mode: 0o700 }) + try { + fsSync.chmodSync(sd, 0o700) + } catch { + // best-effort: chmod denied or unavailable; a fresh dir was still + // created with the requested mode + } + return sd +} + +/** Save the DACL of *srcPath* to a dump file on Windows. + * Returns null when the dump was written - the caller keeps it and restores it onto the + * committed file at step 5 - and returns the runner's error when it was not, because the + * caller refuses the publish in that case and has to be able to say which failure it is + * refusing: "icacls ran and failed" and "icacls could not be started at all" are different + * facts to act on. Never throws. */ +async function _saveDaclWindows(srcPath: string, dumpPath: string, execFileRunner?: typeof execFile): Promise { + const runner = execFileRunner ?? execFile + try { + await new Promise((resolve, reject) => { + runner("icacls", [srcPath, "/save", dumpPath, "/T"], { windowsHide: true }, (err) => + err ? reject(err) : resolve(), + ) + }) + return null + } catch (error: unknown) { + return error + } +} +/** Restore a DACL dump onto *dirPath* on Windows. + * + * Best-effort: the content is already committed, so a failed restore does not fail the write. The + * failure is RETURNED rather than swallowed, because the caller is the only party that can do + * something about a directory whose ACL did not come back, and a swallowed error never reaches it. */ +async function _restoreDaclWindows( + dirPath: string, + dumpPath: string, + execFileRunner?: typeof execFile, +): Promise { + const runner = execFileRunner ?? execFile + try { + await new Promise((resolve, reject) => { + runner("icacls", [dirPath, "/restore", dumpPath], { windowsHide: true }, (err) => + err ? reject(err) : resolve(), + ) + }) + return null + } catch (error: unknown) { + // best-effort: content already committed, so the caller is told rather than the write failed + return error + } +} + +// -- public API ------------------------------------------------------------ + +/** + * Resolve the publish target: the symlink referent when the given path is an + * existing symlink, the path itself otherwise. Only ENOENT (target absent yet) + * may fall back to the given path; any other resolution error (EACCES, EIO, ...) + * propagates so a broken or unreadable symlink is never written through its + * link path. Callers that stage a temp file themselves must stage it beside + * the resolved path: the commit is a rename onto the referent, and a rename + * across filesystems fails with EXDEV. + */ +export async function resolvePublishTarget(absoluteFilePath: string): Promise { + return fs.realpath(absoluteFilePath).catch(async (error: unknown) => { + if (getErrorCode(error) !== "ENOENT") throw error + // ENOENT also covers a dangling symlink, which must never be written through. + // Only a lstat that also reports the path as absent may fall back to the + // given path; a real lstat failure (EACCES, EIO) says nothing about whether + // the path is a link, so falling back would write through a link we were + // simply not allowed to inspect. + const linkStat = await fs.lstat(absoluteFilePath).catch((lstatError: unknown) => { + if (getErrorCode(lstatError) === "ENOENT") return undefined + throw lstatError + }) + if (linkStat?.isSymbolicLink()) throw error + return await canonicalizeNearestAncestor(absoluteFilePath) + }) +} + +/** + * Canonicalize the parent directory and re-join the basename. fs.realpath + * canonicalizes every component, including a symlinked ancestor directory or a + * Windows 8.3 short name, so a lock key must be canonical even when the file + * itself is not there yet -- otherwise the key for one file depends on whether + * the file exists when the key is computed, and two writers take two locks. + */ + +async function canonicalDirKey(absoluteFilePath: string): Promise { + const dirPath = path.dirname(absoluteFilePath) + // Walk up to the nearest ancestor that EXISTS, canonicalize that, and re-join the + // components that are not there yet. Falling back to the unresolved spelling of the + // whole parent - what a single realpath(...).catch(() => dirPath) used to do - makes + // the key depend on whether the directory happens to exist: a writer whose parent is + // already there canonicalizes through a symlinked ancestor (or a short name) while a + // writer racing to create the same directory gets the literal spelling, so the two + // take different locks for one file and a read-modify-write loses one side. + // A realpath failure that is not "not there yet" says nothing about the canonical + // form, so it is propagated rather than papered over with a key that may be wrong. + let cursor = dirPath + const missing: string[] = [] + for (;;) { + const canonical = await fs.realpath(cursor).catch((error: unknown) => { + if (getErrorCode(error) === "ENOENT") return undefined + throw error + }) + if (canonical !== undefined) { + return path.join(canonical, ...missing.reverse(), path.basename(absoluteFilePath)) + } + missing.push(path.basename(cursor)) + const parent = path.dirname(cursor) + if (parent === cursor) { + // Every component up to the root is missing: there is nothing to canonicalize + // against, and the literal path is the only key left. + return path.join(dirPath, path.basename(absoluteFilePath)) + } + cursor = parent + } +} + +/** + * Canonicalize the nearest EXISTING ancestor and re-append the missing components. + * The guard hands back a pin produced exactly this way (guardedWrite#realpathNearest), + * so an absent target has to resolve identically here: a lexical fallback differs from + * that pin whenever an ancestor is a symlink, and the create would then abort with + * TargetMovedError even though nothing moved. + */ +async function canonicalizeNearestAncestor(absoluteFilePath: string): Promise { + const missing: string[] = [] + let cursor = absoluteFilePath + for (;;) { + try { + const realPath = await fs.realpath(cursor) + return missing.length > 0 ? path.join(realPath, ...missing.reverse()) : realPath + } catch (error: unknown) { + const code = getErrorCode(error) + if (code !== "ENOENT" && code !== "ENOTDIR") { + // A real inspection failure (EACCES, EIO) says nothing about the path's + // identity. Returning a lexical key here would silently canonicalize a + // different path than the guard's pin, so fail closed instead. + throw error + } + const parent = path.dirname(cursor) + if (parent === cursor) return absoluteFilePath + missing.push(path.basename(cursor)) + cursor = parent + } + } +} + +/** + * Lock key for a publish target: the symlink referent when the path is an + * existing symlink, the path itself otherwise. Unlike resolvePublishTarget this + * tolerates a dangling link, because the lock key has to be computable while a + * peer writer is mid-commit (backup mode renames the referent away and back). + * The walk is bounded so a two-link cycle terminates, and every key it returns is + * canonicalized through canonicalDirKey. + */ +export async function resolveLockKey(absoluteFilePath: string): Promise { + try { + return await canonicalDirKey(await resolvePublishTarget(absoluteFilePath)) + } catch { + // A real readlink throws for anything that is not a link, so a normal chain + // ends the walk. Two links that point at each other never would, so the + // walk is bounded and callers use the key they actually reached. + let key = absoluteFilePath + for (let depth = 0; depth < 8; depth++) { + const target = await fs.readlink(key).catch(() => undefined) + if (target === undefined) return await canonicalDirKey(key) + key = await canonicalDirKey(path.resolve(path.dirname(key), target)) + } + return await canonicalDirKey(key) + } +} + +export async function safeWriteText( + filePath: string, + content: string | Uint8Array, + options?: SafeWriteTextOptions, +): Promise { + const absoluteFilePath = path.resolve(filePath) + + // Resolve the symlink referent (see resolvePublishTarget). + const targetPath = await resolvePublishTarget(absoluteFilePath) + + // The caller authorized a specific resolved path; publishing at a different one + // would act on a decision that was never made about this file. + if (options?.expectedResolvedPath && path.resolve(options.expectedResolvedPath) !== targetPath) { + throw new TargetMovedError(options.expectedResolvedPath, targetPath) + } + + const dirPath = path.dirname(targetPath) + + // Ensure parent directory exists (mirrors safeWriteJson behaviour). What mkdir reports it + // created feeds step 4b's fsync of the ancestors this write introduced; see + // _createdAncestorChain. + await _preflightAncestors(options?.expectedAncestorIdentities) + const createdAncestors = _createdAncestorChain(await fs.mkdir(dirPath, { recursive: true }), dirPath) + await fs.access(dirPath) + + // Create the staging directory only when we generate the temp file there; + // callers supplying their own tempPath (e.g. safeWriteJson) must not be left + // with an empty .file-safety-staging directory behind. Track the directory this + // write created so its cleanup removes its own directory, not a shared one. + let stagingDir: string | null = null + let tempPath: string + // Whether the commit rename has consumed the staging file. Cleanup is allowed to unlink + // the staging path only while this write still owns it: after the rename the name belongs to + // whatever is filed under it next, and a caller-supplied staging name is reused by the next + // write to the same target. + let stagingCommitted = false + // The identity of a caller-supplied staging file, taken by lstat when it was validated. The + // staging path is re-opened by name further below, so the entry behind the name can be swapped + // between that validation and the open; the open's own descriptor identity is compared + // against this value before anything touches the file. + let stagingIdentity: { dev: bigint; ino: bigint } | null = null + if (options?.tempPath) { + // A caller-supplied staging file is only safe when it is the file this + // write is staging, not an arbitrary path. Two properties are checked: + // it must sit beside the resolved target (a rename across filesystems + // fails with EXDEV, and a path elsewhere lets a caller publish an + // unrelated file onto the target), and it must be a regular file rather + // than a link — renaming a link over the target publishes whatever the + // link points at, which is the same trust problem as writing through a + // dangling symlink in resolvePublishTarget. + const supplied = path.resolve(options.tempPath) + if (path.dirname(supplied) !== path.resolve(dirPath)) { + throw new StagingPathError( + `Staging file must sit in the target's directory (${dirPath}), got ${supplied}`, + supplied, + ) + } + // BigInt stats: on NTFS/ReFS the file identity can exceed Number.MAX_SAFE_INTEGER, and + // a rounded number makes two different files look identical (rejecting a valid staging + // file) or hides a real alias. + const stagingStat = await fs.lstat(supplied, { bigint: true }) + if (stagingStat.isSymbolicLink() || !stagingStat.isFile()) { + throw new StagingPathError( + `Staging file must be a regular file, not ${stagingStat.isSymbolicLink() ? "a symlink" : "another file type"}`, + supplied, + ) + } + // A staging path that is the target would be unlinked by the failure handler + // while it still holds the only copy of the content, so a failed write would + // delete the file it was meant to protect. Compare identities, not spellings: + // an alias of the target is the same hazard. + // Only a missing target may be skipped: an EACCES/ELOOP/ENOTDIR here means the + // identity comparison could not be made, and treating that as "no target" would let a + // staging alias reach the commit and let cleanup delete the file it was meant to + // protect. + const targetStat = await fs.lstat(targetPath, { bigint: true }).catch((error: unknown) => { + if (getErrorCode(error) !== "ENOENT") { + throw new StagingPathError("Staging file could not be compared with the target", supplied) + } + return null + }) + if ( + targetStat && + typeof stagingStat.ino === "bigint" && + typeof targetStat.ino === "bigint" && + stagingStat.ino === targetStat.ino && + stagingStat.dev === targetStat.dev + ) { + throw new StagingPathError("Staging file must not be the target itself", supplied) + } + stagingIdentity = { dev: stagingStat.dev, ino: stagingStat.ino } + // The caller's own path is used as given; only the check is canonical. + tempPath = options.tempPath + } else { + stagingDir = _stagingDir(dirPath) + tempPath = _tempName(stagingDir, "safeWriteText") + } + + let backupPath: string | null = null + let releaseBackupOnSuccess = false + // True only when this write's own seed open created the backup file. A name that already + // existed is not this write's file, and the failure path below must not unlink it. + let backupSeeded = false + // Set once the target access check in step 3 below has found a target there. It is what + // separates the one ENOENT this write may answer by publishing without a backup - the + // target not being there at all - from an ENOENT raised while seeding the backup after + // the target was found. + let backupTargetFound = false + // Non-null only when the win32 step-2 block saved a successful DACL dump: + // it gates the step-5 restore and is tracked for the cleanup unlinks. + let daclDumpPath: string | null = null + + try { + // -- Step 1: write content to staging temp file ------------------- + if (!options?.tempPath) { + // Preserve the existing target's permissions: the staging file must + // not be published wider than the file it replaces (a 0o600 target + // must not become 0o644 through the atomic rename). + // Encode before opening the staging file: an encoding Node cannot + // represent must not leave a half-written temp file behind. + // A string is encoded as UTF-8; bytes handed in by the caller (the + // extension host encodes a document with VS Code's own codec, which + // covers the legacy code pages Node cannot represent) are published + // unchanged. + const buffer = Buffer.from(content) + let targetMode = 0o644 // default for a fresh target + let targetExists = false + try { + targetMode = fsSync.statSync(targetPath).mode & 0o777 + targetExists = true + } catch (error: unknown) { + if (getErrorCode(error) !== "ENOENT") throw error + // target does not exist yet - keep the default + } + // openSync's creation mode is narrowed by the process umask, so an + // existing 0o664 target would be published as 0o644 through the + // rename. Apply the existing target's exact mode on the fd, as the + // caller-staged branch does; a fresh target keeps the default mode. + const fd = fsSync.openSync(tempPath, "w", targetMode) + try { + if (targetExists) { + fsSync.fchmodSync(fd, targetMode) + } + // Loop until every byte is written: writeSync can report a short + // (partial) write, and publishing a truncated staging file would + // commit corrupt content. + // A zero return is not a short write - it means the descriptor made no progress - + // and it is handled below rather than looped on. + let offset = 0 + while (offset < buffer.length) { + const written = fsSync.writeSync(fd, buffer, offset, buffer.length - offset) + if (written === 0) { + // Not a short write: the descriptor made no progress at all, so looping again + // would hold the fd and the event loop forever without ever reaching fsyncSync + // or the commit. The finally below closes the fd and the outer cleanup removes + // the staged file, so a stalled write strands neither. + throw new Error( + `safeWriteText: write of ${tempPath} stalled after ${offset} of ${buffer.length} bytes`, + ) + } + offset += written + } + fsSync.fsyncSync(fd) + } finally { + fsSync.closeSync(fd) + } + } else { + // Preserve the existing target's mode (CWE-732): the caller-staged + // temp carries its own creation mode, and publishing it as-is would + // widen a restrictive target (e.g. 0o600 -> 0o644) through rename. + // The mode is applied with fchmodSync on the open fd (AFTER openSync): + // chmodSync on the path before the open would make a read-only target + // (0o400/0o444) fail openSync(tempPath, "r+") with EACCES. + let targetMode: number | null = null + try { + targetMode = fsSync.statSync(targetPath).mode & 0o777 + } catch (error: unknown) { + if (getErrorCode(error) !== "ENOENT") throw error + // target does not exist yet - keep the temp's default mode + } + const fd = fsSync.openSync(tempPath, "r+") + try { + // The name was authorized by lstat and is re-opened here by string, so the entry behind it may + // have been replaced in between. Compare the descriptor's own identity with the one that + // was authorized and refuse before fchmodSync or fsyncSync acts on whatever the name + // holds now. + if (stagingIdentity !== null) { + const opened = fsSync.fstatSync(fd, { bigint: true }) + if (opened.dev !== stagingIdentity.dev || opened.ino !== stagingIdentity.ino) { + throw new StagingPathError("Staging file was replaced between validation and open", tempPath) + } + } + if (targetMode !== null) { + fsSync.fchmodSync(fd, targetMode) + } + fsSync.fsyncSync(fd) + } finally { + fsSync.closeSync(fd) + } + } + + // -- Step 2 (win32): save DACL BEFORE backup rename --------------- + const platform = options?.platform ?? process.platform + if (platform === "win32") { + let accessError: unknown = null + try { + await fs.access(targetPath) // target exists? + } catch (error: unknown) { + accessError = error + } + if (accessError === null) { + const dumpPath = _tempName(dirPath, "safeWriteText.acl") + const saveFailure = await _saveDaclWindows(targetPath, dumpPath, options?.execFileRunner) + if (saveFailure === null) { + // Only a successfully saved dump may be restored onto the + // committed file (step 5). + daclDumpPath = dumpPath + } else { + // A failed icacls may have left a partial dump behind; + // remove it now (best-effort) so no partial dump survives and + // no later step can restore from it. + await fs.unlink(dumpPath).catch(() => {}) + // The target exists and its DACL could not be captured, so the commit rename + // would replace it with a file that inherits different access rights, and nothing + // here can verify an equivalent restrictive ACL on that replacement. The publish is + // refused instead of warned about: a save that silently changed who can read the + // file is not a save the caller can trust. Nothing is committed yet, so the target + // still holds its content, and this throw lands in the handler at the bottom of + // this try - the one place that removes the staged file and this write's own + // staging directory - so a refused publish strands neither. + // The runner's error travels with the refusal: the phase already says which check + // refused, and without the error a caller cannot tell a failed icacls from an + // icacls that could not be started at all. + throw new DaclInspectionError(targetPath, "save", saveFailure) + } + } else if (getErrorCode(accessError) !== "ENOENT") { + // Not "absent": the target is there but its access rights could not be read + // (EACCES, ...), so publishing would replace a file whose rights this call never + // learned. Same rule as the failed save above: refuse before anything is committed. + throw new DaclInspectionError(targetPath, "inspect", accessError) + } + } + + try { + // -- Step 2b: re-validate the authorized ancestry before committing -- + // The caller decided containment by walking this directory chain. A parent + // swapped for a link to outside that chain would send the commit somewhere the + // caller never authorized, even though expectedResolvedPath still matches (the + // name is unchanged). Compare each recorded identity now, before anything is + // moved into place. + if (options?.expectedAncestorIdentities) { + for (const expected of options.expectedAncestorIdentities) { + const current = await fs.stat(expected.dir, { bigint: true }).catch((error: unknown) => { + if (getErrorCode(error) === "ENOENT") { + throw new AncestorReplacedError(expected.dir, "no longer exists") + } + throw error + }) + if (current.dev !== expected.dev || current.ino !== expected.ino) { + throw new AncestorReplacedError(expected.dir, "is no longer the directory that was authorized") + } + // A move preserves dev+ino, so the identity check above passes for a + // directory that was relocated and whose old name was replaced by a + // symlink to the moved location - stat follows that link. The commit + // below takes path strings and would follow it too, publishing at the + // moved location outside the chain the caller walked. Require that the + // authorized name still resolves to the very location it named. + // Only a vanished name is a containment violation. A transient EACCES + // or EIO would otherwise be reported as a replaced ancestor, pointing + // the user at a security problem that is not there - mirror the stat + // call above, which maps only ENOENT and rethrows the rest. + const resolvedDir = await fs.realpath(expected.dir).catch((error: unknown) => { + const code = getErrorCode(error) + if (code === "ENOENT" || code === "ENOTDIR") { + return undefined + } + throw error + }) + // The location the caller authorized. A recorded resolvedPath is the spelling the + // caller walked - an alias ancestor resolves to something else by definition, and a + // stable alias must pass here; the (dev, ino) check above is what catches the alias + // being re-pointed. Without a resolvedPath the authorized name is the lexical dir. + const authorizedLocation = expected.resolvedPath + ? path.resolve(expected.resolvedPath) + : path.resolve(expected.dir) + if (resolvedDir === undefined || path.resolve(resolvedDir) !== authorizedLocation) { + throw new AncestorReplacedError(expected.dir, "no longer resolves to the authorized location") + } + } + } + + // -- Step 3 (backup:true): durable copy target -> backup ---- + if (options?.backup) { + try { + await fs.access(targetPath) + backupTargetFound = true + backupPath = _tempName(dirPath, "safeWriteText.bak") + // Copy, never move. Renaming the target away leaves the canonical path absent for + // the whole commit window: readers see a missing file, and a concurrent + // writer can create a new target that a later rollback would destroy. A copy + // keeps the target present, so the step 4 rename is the only change to the + // canonical path. The copy is flushed so the retained content survives a crash. + try { + // Create the destination BEFORE any content exists at it, with the mode fixed + // at open time. fs.copyFile picks the destination mode itself (the platform + // creation mask subject to umask on some platforms, the source's mode - or its + // read-only attribute - on others), so letting it create the file would either + // leave a restrictive target's bytes briefly readable to others, or leave the + // copy unwritable so the fsync open below fails with EACCES. open() ignores its + // mode argument for an existing file, so this 0o600 survives the copy on POSIX; + // the chmod afterwards is what clears a copied read-only attribute on Windows + // and keeps a backup of a permissive file private. + const seedFd = fsSync.openSync(backupPath, "wx", 0o600) + // The name is ours from the moment the exclusive open succeeds, so the failure + // path may remove it even when the close below reports an error. + backupSeeded = true + // Single close, no retry: on POSIX close(2) can release the descriptor before it + // reports an error (and leaves its state unspecified after EINTR), so a second + // close could release a descriptor some other operation has meanwhile reused. + // The failure propagates; backupPath is already recorded, so the outer cleanup + // removes the seeded file instead of leaving it beside the target. + fsSync.closeSync(seedFd) + await fs.copyFile(targetPath, backupPath) + await fs.chmod(backupPath, 0o600) + // "r+" not "r": fsync on a read-only handle is EPERM on Windows, and the same + // flag the staged temp file uses above. + const backupFd = fsSync.openSync(backupPath, "r+") + try { + fsSync.fsyncSync(backupFd) + } finally { + fsSync.closeSync(backupFd) + } + } catch (backupError: unknown) { + // A partial backup must not outlive this attempt: it is not a complete copy + // of anything, and once the write fails nothing else removes it. The unlink is + // retried once (Windows reports EPERM for a file whose handle has not been + // released yet); if it still fails the path is carried on the thrown error + // instead of being dropped where no caller can act on it. + const orphanPath = backupPath + let backupCleanupError: unknown = null + if (backupSeeded) { + for (let attempt = 0; attempt < 2; attempt++) { + try { + await fs.unlink(orphanPath) + backupCleanupError = null + break + } catch (cleanupError: unknown) { + if (getErrorCode(cleanupError) === "ENOENT") { + // Already gone: that is exactly the outcome the cleanup wanted, so stop + // rather than unlinking the same path a second time. + backupCleanupError = null + break + } + backupCleanupError = cleanupError + } + } + } + backupPath = null + // When the seed open never succeeded - an EEXIST on a name another writer owns, or a + // failure before anything existed at it - there is nothing here to remove: an entry + // this write did not create is not this write's orphan, and unlinking it would take + // away somebody else's backup on the way out. backupCleanupError stays null, so the + // original failure below is what the caller sees. + if (backupCleanupError !== null) { + throw new OrphanedBackupError(orphanPath, targetPath, backupError, backupCleanupError) + } + throw backupError + } + releaseBackupOnSuccess = true + } catch (err: unknown) { + // Only the access check above may be answered by a missing target: with nothing at + // the target there was no backup to make. An ENOENT from the seed block - the copy + // gone before copyFile, or a chmod on a backup that vanished after it - says the + // durability this call asked for could not be made, and publishing would drop the + // backup the caller requested; that block has already removed its own partial copy + // and reported an unremovable one, so the failure is propagated, not swallowed. + if (getErrorCode(err) !== "ENOENT" || backupTargetFound) throw err + } + } + + // -- Step 4: atomic commit temp -> target --------------------- + if (options?.failIfExist) { + // No-replace commit: link(2) fails EEXIST when the name already exists, + // where a rename would silently displace whatever a non-participating + // writer put there after the caller checked. The staged copy is removed + // once the name points at it. + try { + await fs.link(tempPath, targetPath) + } catch (error: unknown) { + const code = getErrorCode(error) + if (code === "EEXIST") { + throw new TargetExistsError(targetPath) + } + // FAT32/exFAT volumes and some SMB/network mounts have no hard links, where + // link(2) fails EPERM/ENOTSUP/ENOSYS. The fallback this replaces published by + // an exclusive copy directly at the final name, but a copy is not atomic: + // readers can observe a partial file, and a crash or an EIO mid-copy leaves + // damaged bytes under the final name with the 'already exists' verdict on + // retry. No atomic no-replace primitive exists on those volumes, so the + // guarded create fails closed - nothing is published and the caller learns + // the volume cannot honor the contract. + if (code !== "EPERM" && code !== "ENOTSUP" && code !== "ENOSYS") { + throw error + } + throw new UnsupportedFilesystemError(targetPath, error) + } + } else { + await fs.rename(tempPath, targetPath) + } + + // Marked as soon as the commit has consumed the staging name, on both paths: the rename + // consumed it, and on the link path the target now holds the inode this write staged. + // Everything below is a post-commit step - the identity comparison, the removal of the + // staged name, the warnings, the fsyncs and the DACL restore - and none of them, including + // a caller callback that throws, may leave the failure handler free to unlink a staging + // name whose content is already published. + stagingCommitted = true + + if (options?.failIfExist) { + // Both names should now be one inode. They diverge when the entry under either name is + // replaced after the commit, and the published name then holds bytes this write never + // staged. A substitution completed BEFORE the link is not detectable from here: link(2) + // links whatever entry the staging name holds at that moment, so both stats report the + // substituted inode and the comparison passes. Checked after the link because link(2) is + // the first point at which the target name exists, and a check of either name before it + // cannot see what landed at the other. Neither name is removed on a mismatch and the + // commit marker above is already set, so the + // cleanup below cannot unlink a name that may belong to another process. The ordinary + // rename path cannot be verified this way: rename(2) moves the staged inode itself and + // leaves no second name to compare, and a re-stat of the target proves only that the + // rename took effect, not that the staging file was still ours when it did. Issue + // #2391 is where that gap is carried; what it must prove is how a commit that leaves + // no second name detects a staging substitution before publication and how its cleanup + // avoids removing an entry it does not own. This unit does not claim that answer. + const [stagedNow, publishedNow] = await Promise.all([ + fs.stat(tempPath, { bigint: true }), + fs.stat(targetPath, { bigint: true }), + ]) + if (stagedNow.dev !== publishedNow.dev || stagedNow.ino !== publishedNow.ino) { + throw new PublishedEntryMismatchError( + targetPath, + `${publishedNow.dev}:${publishedNow.ino}`, + `${stagedNow.dev}:${stagedNow.ino}`, + ) + } + // The target name is published at this point; the staged name is only a second + // link to the same inode (or a copy of it). Removal is best-effort: on Windows + // an antivirus handle can make unlink fail with EBUSY/EPERM after the content + // is already visible, and the write must not be reported as failed for content + // the caller can now read back. The residue is named on the warning channel rather + // than dropped, because a caller that keeps its own staging name needs to learn the + // orphan is still there. + const stagingResidue = await fs.unlink(tempPath).catch((error: unknown) => error) + if (stagingResidue !== undefined) { + options?.onPostCommitWarning?.({ + kind: "staging-link-orphan", + path: tempPath, + cause: stagingResidue, + }) + } + } + + // -- Step 4b (POSIX): fsync the parent directory so the directory entry + // changed by the commit rename is durable, not just the file content. + if (platform !== "win32") { + try { + _syncDirectory(dirPath) + // The rename above made the entry INSIDE dirPath durable. A parent this call had to + // create is itself an entry in its own parent, so syncing only the immediate parent + // leaves the newer ancestors unsynced and a crash can remove the chain - taking the + // published file with it. Sync deepest-first, ending with the pre-existing directory + // that holds the outermost new entry. + for (let i = createdAncestors.length - 2; i >= 0; i--) { + _syncDirectory(createdAncestors[i]) + } + if (createdAncestors.length > 0) { + _syncDirectory(path.dirname(createdAncestors[0])) + } + } catch (error: unknown) { + // The content rename committed, but the directory entry that + // points at it is not known to be durable. Reporting success + // here would let a caller believe the write survives a crash, + // so the failure is surfaced as its own error: the caller can + // still find the content at the target, it just cannot rely on + // the directory entry having reached the disk. + throw new PostCommitDurabilityError(targetPath, error) + } + } + + // -- Step 5 (win32): restore DACL AFTER commit rename --------- + // daclDumpPath is non-null only when the win32 step-2 block saved a + // successful dump, so this gate is closed on every other platform + // and on every failed save. + if (daclDumpPath !== null) { + const restoredDir = path.dirname(targetPath) + const restoreFailure = await _restoreDaclWindows(restoredDir, daclDumpPath, options?.execFileRunner) + if (restoreFailure !== null) { + options?.onPostCommitWarning?.({ + kind: "dacl-restore-failed", + path: restoredDir, + cause: restoreFailure, + }) + } + } + + // -- Step 6 (backup:true): delete backup on success ----------- + if (releaseBackupOnSuccess && backupPath) { + const orphanedBackupPath = backupPath + let backupCleanupError: unknown = null + // A transient handle (antivirus scanner, indexer) can block the first unlink + // on Windows; one retry usually clears it. + for (let attempt = 0; attempt < 2; attempt++) { + try { + await fs.unlink(orphanedBackupPath) + backupCleanupError = null + break + } catch (cleanupError: unknown) { + if (getErrorCode(cleanupError) === "ENOENT") { + backupCleanupError = null + break + } + backupCleanupError = cleanupError + } + } + backupPath = null + if (backupCleanupError !== null) { + // The content is published: reporting success here would abandon a stale + // copy of the previous content, so the failure travels as its own error. + throw new OrphanedPublishedBackupError(targetPath, orphanedBackupPath, backupCleanupError) + } + } + } finally { + // Unlink DACL dump regardless of success/failure in this span. + if (daclDumpPath !== null) { + await fs.unlink(daclDumpPath).catch(() => {}) + } + } + + // tempPath is now the committed file; no cleanup needed. + + // Best-effort: remove the now-empty staging directory. Self-staged + // writes only, and only this write's own directory: a per-write directory + // cannot be the one another concurrent write is still using. A failure must + // never un-commit a published file, so the removal swallows all errors. + if (stagingDir) { + await fs.rmdir(stagingDir).catch(() => {}) + } + } catch (originalError: unknown) { + // The backup is a copy, never a restore source: whether the failure happened before + // or after the commit rename, the copy is removed below so no stale duplicate of the + // previous content survives next to the target. + // Set when a backup copy this write made could not be removed; reported at the end of + // this catch so the staging cleanup above is not skipped for a cleanup failure. + let pendingBackupError: OrphanedBackupAfterFailedWriteError | null = null + if (backupPath && releaseBackupOnSuccess) { + // Nothing to restore: the backup is a copy, so the target still holds whatever + // the commit left there - before the commit that is the pre-write content, and + // after it the published content. Either way the copy has served its purpose + // and must not be left beside the target where no caller can find it. + let removalError: unknown = null + for (let attempt = 0; attempt < 2; attempt++) { + try { + await fs.unlink(backupPath) + removalError = null + break + } catch (error: unknown) { + if (getErrorCode(error) === "ENOENT") { + // Already gone: that is exactly the outcome this cleanup wanted. + removalError = null + break + } + // Retry once: Windows reports EPERM for a file whose handle has not been + // released yet, which is the same failure the success-path cleanup retries. + removalError = error + } + } + if (removalError !== null) { + // The copy of the previous content is still on disk and only this function knew + // where. Reporting it is what keeps the original error the primary fact while + // leaving the leftover findable; swallowing the removal left both unreported. + // Constructed, not thrown yet: the staging cleanup below has to run first, so + // the leftover is reported once this catch has finished what it can do about the + // write's own mess. + pendingBackupError = new OrphanedBackupAfterFailedWriteError( + targetPath, + backupPath, + originalError, + removalError, + stagingCommitted, + ) + } + backupPath = null + } + if (!stagingCommitted) { + try { + await fs.unlink(tempPath).catch(() => {}) + } catch { + // cleanup failure is non-fatal + } + } + + // A failed self-staged write must not leave its staging directory behind. + // Only the directory this write created, and only after its temp file is + // gone, so the directory is empty and the removal stays best-effort. + if (stagingDir) { + await fs.rmdir(stagingDir).catch(() => {}) + } + + if (daclDumpPath !== null) { + await fs.unlink(daclDumpPath).catch(() => {}) + } + + if (pendingBackupError) { + // The write's own failure is still the primary fact - it travels on this error as + // originalError and as its cause - and the copy of the previous content now has a + // path a caller can act on instead of being unreported. + throw pendingBackupError + } + throw originalError + } +} diff --git a/src/utils/__tests__/fileLock.integration.spec.ts b/src/utils/__tests__/fileLock.integration.spec.ts new file mode 100644 index 0000000000..72eca6f19f --- /dev/null +++ b/src/utils/__tests__/fileLock.integration.spec.ts @@ -0,0 +1,41 @@ +import * as fsSync from "fs" +import * as os from "os" +import * as path from "path" + +import { lockArtefactSpelling } from "../fileLock" + +// No fs mock in this file. The function's whole answer comes from what the volume reports +// for the nearest existing ancestor, so a mocked realpath proves only that the code reads +// the mock. This is the case the canonicalization exists for: a caller that reaches the +// directory through a link must contend on the same artefact as a caller that reaches it +// directly. +describe("lockArtefactSpelling against a real filesystem", () => { + let dir: string + + beforeEach(() => { + dir = fsSync.mkdtempSync(path.join(os.tmpdir(), "file-lock-int-")) + }) + + afterEach(() => { + fsSync.rmSync(dir, { recursive: true, force: true }) + }) + + it("names the canonical ancestor when the caller reaches it through a link", () => { + const realDir = path.join(dir, "real-dir") + fsSync.mkdirSync(realDir) + const linkDir = path.join(dir, "link-dir") + // A junction on Windows and a directory symlink elsewhere: both are reinterpreted by + // realpathSync.native, and neither needs elevated privileges. + fsSync.symlinkSync(realDir, linkDir, process.platform === "win32" ? "junction" : "dir") + + const target = path.join(linkDir, "missing-component", "history_item.json") + const canonicalRealDir = fsSync.realpathSync.native(realDir) + + expect(lockArtefactSpelling(target)).toBe( + path.join(canonicalRealDir, "missing-component", "history_item.json") + ".lock", + ) + // The link name must not survive into the artefact, or the two spellings take two + // lock files and the writers never see each other. + expect(lockArtefactSpelling(target)).not.toContain(path.basename(linkDir)) + }) +}) diff --git a/src/utils/__tests__/fileLock.test.ts b/src/utils/__tests__/fileLock.test.ts index 92637fe7a0..39e7441fe1 100644 --- a/src/utils/__tests__/fileLock.test.ts +++ b/src/utils/__tests__/fileLock.test.ts @@ -1,8 +1,21 @@ +import * as fsSync from "fs" +import * as path from "path" + import * as lockfile from "proper-lockfile" -import { acquireFileLock, LOCK_STALE_MS, withFileLock } from "../fileLock" +import { acquireFileLock, LOCK_STALE_MS, lockArtefactSpelling, withFileLock } from "../fileLock" vi.mock("proper-lockfile", () => ({ lock: vi.fn() })) +// Only realpathSync.native is replaced, and it is replaced with a stub rather than a fixture: +// the spelling cases below are about how the function composes an answer from whatever the +// volume reports, and the case where the volume really reports something is in +// fileLock.integration.spec.ts. Everything else in "fs" stays the real thing so the module's +// own imports keep working. +vi.mock("fs", async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, realpathSync: Object.assign(vi.fn(), { native: vi.fn() }) } +}) + describe("file lock protocol", () => { beforeEach(() => vi.clearAllMocks()) @@ -32,3 +45,67 @@ describe("file lock protocol", () => { expect(release).toHaveBeenCalledOnce() }) }) + +describe("lock artefact spelling", () => { + beforeEach(() => vi.clearAllMocks()) + + // Only the one ancestor below resolves; every deeper component is absent, which is the + // state the function exists for: the leaf may be mid-rename or missing entirely. + const _only = + (existing: string) => + (candidate: string | Buffer | URL): string => { + if (String(candidate) === existing) { + return existing + } + const error = new Error( + `ENOENT: no such file or directory, realpath '${candidate}'`, + ) as NodeJS.ErrnoException + error.code = "ENOENT" + throw error + } + + it("spells the artefact from the nearest existing ancestor and keeps the missing components in order", () => { + const existingDir = path.resolve("/tmp/canonical-dir") + const target = path.join(existingDir, "missing-one", "missing-two", "history_item.json") + vi.mocked(fsSync.realpathSync.native).mockImplementation(_only(existingDir)) + + expect(lockArtefactSpelling(target)).toBe( + path.join(existingDir, "missing-one", "missing-two", "history_item.json") + ".lock", + ) + }) + + it("keeps the caller's own spelling when nothing on the path resolves", () => { + vi.mocked(fsSync.realpathSync.native).mockImplementation(_only("/does-not-exist")) + const target = path.resolve("/tmp/never-created-dir/history_item.json") + + expect(lockArtefactSpelling(target)).toBe(target + ".lock") + }) + + it("hands a caller-captured spelling to lockfile.lock verbatim", async () => { + vi.mocked(lockfile.lock).mockResolvedValue(async () => {}) + const target = path.resolve("/tmp/canonical-dir/history_item.json") + const spelling = path.resolve("/tmp/locks/captured.json.lock") + + await acquireFileLock(target, { artefactSpelling: spelling }) + + // A writer that renamed the leaf captured this spelling before the rename. Re-deriving + // the name here would contend with nobody and lock a second artefact. + expect(lockfile.lock).toHaveBeenCalledWith(target, expect.objectContaining({ lockfilePath: spelling })) + }) + + it("derives the artefact itself when the caller supplies no spelling", async () => { + vi.mocked(lockfile.lock).mockResolvedValue(async () => {}) + const existingDir = path.resolve("/tmp/canonical-dir") + const target = path.join(existingDir, "missing-one", "history_item.json") + vi.mocked(fsSync.realpathSync.native).mockImplementation(_only(existingDir)) + + await acquireFileLock(target) + + expect(lockfile.lock).toHaveBeenCalledWith( + target, + expect.objectContaining({ + lockfilePath: path.join(existingDir, "missing-one", "history_item.json") + ".lock", + }), + ) + }) +}) diff --git a/src/utils/fileLock.ts b/src/utils/fileLock.ts index 49807ea154..344519ac56 100644 --- a/src/utils/fileLock.ts +++ b/src/utils/fileLock.ts @@ -1,4 +1,5 @@ import * as path from "path" +import * as fsSync from "fs" import { tracePersistenceEvent } from "../core/task-persistence/persistenceTrace" import * as lockfile from "proper-lockfile" @@ -17,6 +18,56 @@ export interface FileLockOptions { waitForStaleLock?: boolean /** Startup scans defer locked files instead of waiting under the store mutex. */ immediate?: boolean + /** + * An artefact spelling the caller captured before it started moving the leaf, used verbatim + * for this acquisition. A writer that renames the leaf as part of its own write must capture + * the spelling once, before the first rename, and hand it back here: re-deriving it after the + * rename names a different artefact for one file. + */ + artefactSpelling?: string +} + +/** + * The artefact a lock lives in: the caller's path with its nearest existing ancestor spelled + * the way the volume spells it, plus ".lock". Nothing in the name comes from what the leaf + * currently holds, because `safeWriteJson`'s EPERM fallback renames the leaf itself to a backup + * mid-write: a name derived from the leaf at that moment belongs to whichever spelling the + * volume happens to hold, so two writers that compute one identity take two artefacts and + * contend with nobody. The second then acquires while the first still holds, runs its recovery + * pass over the first writer's in-flight temp file, and the first fails its commit rename with + * ENOENT. Only the nearest EXISTING ancestor is canonicalized and the components below it are + * re-joined, which is what keeps the name computable while the leaf is absent or dangling. The + * basename below that ancestor is compared as written, so two spellings of the file itself (an + * 8.3 alias, a case variant on a case-insensitive volume) still take one artefact each: the + * same residual the identity key carries, and a residual this function does not remove. + */ +export function lockArtefactSpelling(absoluteFilePath: string): string { + const absolute = path.resolve(absoluteFilePath) + const dirPath = path.dirname(absolute) + let cursor = dirPath + const missing: string[] = [] + for (;;) { + let canonical: string | undefined + try { + canonical = fsSync.realpathSync.native(cursor) + } catch { + // An ancestor that is not there yet, or that cannot be inspected, says nothing about the + // canonical spelling. Falling back to the caller's own parent is the conservative + // direction here: it can only ever give a writer the same artefact it takes today, never a + // second writer one it would not have taken. + canonical = undefined + } + if (canonical !== undefined) { + return path.join(canonical, ...missing.reverse(), path.basename(absolute)) + ".lock" + } + missing.push(path.basename(cursor)) + const parent = path.dirname(cursor) + if (parent === cursor) { + // Nothing on the path exists yet, so the caller's spelling is the only name left. + return absolute + ".lock" + } + cursor = parent + } } /** @@ -28,9 +79,11 @@ export interface FileLockOptions { */ export async function acquireFileLock(filePath: string, options?: FileLockOptions): Promise<() => Promise> { const absoluteFilePath = path.resolve(filePath) + const lockfilePath = options?.artefactSpelling ?? lockArtefactSpelling(absoluteFilePath) if (options?.waitForStaleLock) tracePersistenceEvent("json-lock-wait", { filePath: absoluteFilePath }) try { return await lockfile.lock(absoluteFilePath, { + lockfilePath, // named by the caller's path and its canonical ancestor, never by what the leaf currently holds stale: LOCK_STALE_MS, update: 10000, // Update mtime every 10 seconds to prevent staleness if operation is long realpath: false, // the file may not exist yet, which is acceptable diff --git a/src/utils/safeWriteJson.ts b/src/utils/safeWriteJson.ts index e54b8c51d3..c307699d62 100644 --- a/src/utils/safeWriteJson.ts +++ b/src/utils/safeWriteJson.ts @@ -4,7 +4,7 @@ import * as path from "path" import { JsonStreamStringify } from "json-stream-stringify" import { getErrorCode } from "./errorCode" -import { acquireFileLock } from "./fileLock" +import { acquireFileLock, lockArtefactSpelling } from "./fileLock" import { recoverInterruptedReplacement } from "./jsonFileRecovery" /** @@ -97,7 +97,16 @@ async function safeWriteJson(filePath: string, data: unknown, options?: SafeWrit // remains a no-op, so the finally block in the main file operations // try-catch-finally won't try to release an unacquired lock if this // path is taken. - releaseLock = await acquireFileLock(absoluteFilePath) + // The artefact spelling is captured here, before this write moves the leaf. The EPERM fallback + // below renames the target itself to a backup, so a spelling taken after that rename — or + // re-derived on a second acquisition — names a different artefact for the same file: a second + // writer would then acquire while this one still holds, recover over this writer's in-flight + // temp file, and leave this writer's commit rename to fail with ENOENT. Capturing once and + // handing the spelling to the lock is what keeps one file under one artefact for the whole + // write. The lock names uncommitted callers the same way, so this changes nothing about which + // artefact a peer takes; it only fixes the name for the duration of this write. + const artefactSpelling = lockArtefactSpelling(absoluteFilePath) + releaseLock = await acquireFileLock(absoluteFilePath, { artefactSpelling }) // Variables to hold the actual paths of temp files if they are created. let actualTempNewFilePath: string | null = null