diff --git a/src/services/file-safety/__tests__/safeWriteText.alias.spec.ts b/src/services/file-safety/__tests__/safeWriteText.alias.spec.ts new file mode 100644 index 0000000000..19a95611a5 --- /dev/null +++ b/src/services/file-safety/__tests__/safeWriteText.alias.spec.ts @@ -0,0 +1,148 @@ +import { execFileSync } from "child_process" +import * as fs from "fs" +import * as os from "os" +import * as path from "path" + +import { safeWriteText, StagingPathError } from "../safeWriteText" + +// NTFS hands an 8.3 alias to every component whose name is longer than eight characters, so one +// directory answers to two spellings. The hosted Windows runner is the case that matters: its +// temp directory arrives in the short form under the profile while the writer's resolved target +// arrives in the long spelling, and the staging file created beside that target carries whichever +// spelling the caller's canonicalisation produced. A textual comparison of the two directories then +// rejects a staging file that really is beside the target - that is the StagingPathError the +// windows job reports at the staging guard. +// +// The alias cannot be invented where 8.3 names are not generated, so this file asks cmd for the +// short spelling of a directory it just created and skips the suite where no alias appears: on any +// platform that is not win32, where cmd.exe does not exist and no alias could exist either, and on +// a win32 volume that has 8.3 name generation switched off. The probe has to answer that question +// without throwing: until now the probe ran while the module was being imported, so a worker with no +// cmd.exe failed the import and vitest recorded a failed suite instead of skipping the suite. +// What node 22.23.4 - the version .tool-versions and engines pin - +// does with those two spellings is asserted below rather than assumed: path.resolve preserves +// whichever spelling it is handed, fs.realpath preserves it too, and fs.stat reports one identity +// for both. The guard compares identities for exactly that reason. + +const IS_WINDOWS = process.platform === "win32" +const SHORT_NAME_MARKER = "~1" +const SHORT_NAME_PROBE_BIN = "cmd.exe" + +// Total: every failure mode answers null so that the caller skips instead of dying. A non-win32 +// worker has neither cmd.exe nor 8.3 aliases and is answered without spawning anything; a cmd that +// is missing, refuses the query, or answers without an alias means no alias to test against. +function shortPathOf(directory: string): string | null { + if (!IS_WINDOWS) { + return null + } + try { + const out = execFileSync(SHORT_NAME_PROBE_BIN, ["/c", "for %A in (" + directory + ") do @echo %~sA"], { + encoding: "utf8", + }).trim() + return out.includes(SHORT_NAME_MARKER) ? out : null + } catch { + return null + } +} + +// Answered once at import so that it.runIf can decide before any test body runs. Never allowed to +// throw: a throwing probe is a failed import, which vitest reports as a failed suite that no skip +// condition can rescue. +function probeGeneratesEightThreeNames(): boolean { + if (!IS_WINDOWS) { + return false + } + let probeBase: string | undefined + try { + probeBase = fs.mkdtempSync(path.join(os.tmpdir(), "safe-write-alias-probe-")) + const probeDir = path.join(probeBase, "EightThreeNameProbe") + fs.mkdirSync(probeDir) + return shortPathOf(probeDir) !== null + } catch { + return false + } finally { + if (probeBase !== undefined) { + fs.rmSync(probeBase, { recursive: true, force: true }) + } + } +} + +const EIGHT_THREE_NAMES_ARE_GENERATED = probeGeneratesEightThreeNames() + +describe("safeWriteText staging guard against a second spelling of the target directory", () => { + let tempDir: string + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "safeWriteText-alias-")) + }) + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }) + }) + + function createTargetDirectory(): { longPath: string; shortPath: string | null } { + const longPath = path.join(tempDir, "LONGTARGETDIRECTORYNAME") + fs.mkdirSync(longPath) + return { longPath, shortPath: shortPathOf(longPath) } + } + + it.runIf(EIGHT_THREE_NAMES_ARE_GENERATED)( + "publishes a staging file spelled through the target directory own 8.3 alias", + async () => { + const { longPath, shortPath } = createTargetDirectory() + if (shortPath === null) { + return + } + const targetPath = path.join(longPath, "target.txt") + fs.writeFileSync(targetPath, "committed", "utf8") + // The staging file sits beside the target but is spelled through the directory's 8.3 + // alias, which is what a caller's own canonicalisation hands the guard. + const stagingPath = path.join(shortPath, ".new_target.txt_1_x.tmp") + fs.writeFileSync(stagingPath, "staged", "utf8") + + // Preconditions, asserted rather than assumed: the two directory spellings differ as text + // and this runtime's fs.realpath does not unify them... + expect(path.dirname(stagingPath)).not.toBe(path.dirname(targetPath)) + const realViaAlias = fs.realpathSync(path.dirname(stagingPath)) + const realViaTarget = fs.realpathSync(path.dirname(targetPath)) + expect(realViaAlias).not.toBe(realViaTarget) + // ...while fs.stat reports the same directory for both spellings. That identity is what + // the guard compares, and it is why a rename from here onto the target cannot be EXDEV. + const statViaAlias = fs.statSync(path.dirname(stagingPath), { bigint: true }) + const statViaTarget = fs.statSync(path.dirname(targetPath), { bigint: true }) + expect(statViaAlias.dev).toBe(statViaTarget.dev) + expect(statViaAlias.ino).toBe(statViaTarget.ino) + + await safeWriteText(targetPath, "", { tempPath: stagingPath, backup: false }) + + expect(fs.readFileSync(targetPath, "utf8")).toBe("staged") + expect(fs.existsSync(stagingPath)).toBe(false) + }, + ) + + it.runIf(EIGHT_THREE_NAMES_ARE_GENERATED)( + "still rejects a staging file spelled through another directory 8.3 alias", + async () => { + const { longPath } = createTargetDirectory() + const otherPath = path.join(tempDir, "OTHERTARGETDIRECTORYNAME") + fs.mkdirSync(otherPath) + const otherShort = shortPathOf(otherPath) + if (otherShort === null) { + return + } + const targetPath = path.join(longPath, "target.txt") + fs.writeFileSync(targetPath, "committed", "utf8") + const stagingPath = path.join(otherShort, ".new_target.txt_1_x.tmp") + fs.writeFileSync(stagingPath, "staged", "utf8") + + // Same alias mechanism, different directory. Accepting the target directory's alias must + // not become accepting any directory, or a caller could publish an unrelated file onto the + // target through a name the guard was written to refuse. + await expect(safeWriteText(targetPath, "", { tempPath: stagingPath, backup: false })).rejects.toThrow( + StagingPathError, + ) + expect(fs.readFileSync(targetPath, "utf8")).toBe("committed") + expect(fs.existsSync(stagingPath)).toBe(true) + }, + ) +}) 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..d5f845d4f3 --- /dev/null +++ b/src/services/file-safety/__tests__/safeWriteText.spec.ts @@ -0,0 +1,1893 @@ +import * as fs from "fs/promises" +import * as fsSync from "fs" +import { execFile } from "child_process" +import type { ChildProcess } from "child_process" +import * as path from "path" + +import { + OrphanedBackupError, + PostCommitDurabilityError, + resolveLockKey, + safeWriteText, + StagingPathError, +} from "../safeWriteText" + +// Full mock for fs/promises — all methods are vi.fn() stubs +vi.mock("fs/promises", () => ({ + copyFile: vi.fn(), + chmod: vi.fn(), + mkdir: vi.fn(), + access: vi.fn(), + rename: 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", () => ({ + openSync: vi.fn(), + writeSync: vi.fn(), + closeSync: vi.fn(), + mkdirSync: vi.fn(), + fsyncSync: vi.fn(), + chmodSync: vi.fn(), + fchmodSync: vi.fn(), + fstatSync: 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 + // Single name by default: the post-open verification rejects a staged file that + // has gained a second (hard-link) name. + s.nlink = 1 + return s +} +// Directory stand-in: neither a symlink nor a regular file. The symlink double short-circuits +// on isSymbolicLink(), so only this double reaches the branch that rejects any other file type. +function _notARegularFileStats(): fsSync.Stats { + const s = Object.create(fsSync.Stats.prototype) as fsSync.Stats + s.isSymbolicLink = () => false + s.isFile = () => false + return s +} + +// fs.BigIntStats is a type-only export (fs.BigIntStats is undefined at runtime), so the stand-in is +// a Stats object carrying bigint ino/dev - exactly what fs.lstat(path, { bigint: true }) hands +// back at runtime. +function _fileStatsWithIdentity(ino: bigint, dev: bigint): fsSync.BigIntStats { + // Double assertion: the runtime value is the Stats stand-in, the type is the bigint variant. + const s = _fileStats(false) as unknown as fsSync.BigIntStats + s.ino = ino + s.dev = dev + 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.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)) + // Post-open default: the opened file is the same single-named regular file. + vi.mocked(fsSync.fstatSync).mockReturnValue(_fileStats(false)) + // Directory-identity default: unresolvable, so the identity check keeps the textual rejection + // wherever a test does not stage an alias of the target's own directory. + vi.mocked(fs.stat).mockRejectedValue( + Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }), + ) +} +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. + // Exact path read back from the create: the SUT releases the same + // directory it staged in, and a substring match would also accept a + // release aimed at some other staging directory. + expect(fs.rmdir).toHaveBeenCalledTimes(1) + const stagingDir = String(vi.mocked(fsSync.mkdirSync).mock.calls[0]?.[0]) + expect(stagingDir).toContain(".file-safety-staging") + expect(fs.rmdir).toHaveBeenCalledWith(stagingDir) + // 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) + const stagingDir = String(vi.mocked(fsSync.mkdirSync).mock.calls[0]?.[0]) + expect(stagingDir).toContain(".file-safety-staging") + expect(fs.rmdir).toHaveBeenCalledWith(stagingDir) + 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/.new_caller-staged.tmp" + 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("retries a failed staging-dir removal, never fails the committed write, and reports the exact path", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const warnings: string[] = [] + 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", onWarning: (m) => warnings.push(m) }), + ).resolves.toBeUndefined() + + // the commit rename still happened, and the removal was retried once + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining(".file-safety-staging"), targetPath) + expect(fs.rmdir).toHaveBeenCalledTimes(2) + const stagingDirs = vi.mocked(fsSync.mkdirSync).mock.calls.map((call) => String(call[0])) + expect(stagingDirs.length).toBe(1) + expect(fs.rmdir).toHaveBeenNthCalledWith(1, stagingDirs[0]) + expect(fs.rmdir).toHaveBeenNthCalledWith(2, stagingDirs[0]) + // the leftover is not swallowed: the exact directory path reaches the warning sink + const reported = warnings.filter((m) => m.includes(stagingDirs[0])) + expect(reported).toHaveLength(1) + expect(reported[0]).toContain("could not remove the staging directory") + }) + + it("treats a staging directory that someone else already removed as a completed cleanup", async () => { + // ENOENT from the rmdir is the goal, not a leftover: reporting it would send the reader + // looking for a directory that is already gone. + const targetPath = "/tmp/test-dir/target.txt" + const warnings: string[] = [] + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fs.rmdir).mockRejectedValue( + Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }), + ) + + await expect( + safeWriteText(targetPath, "hello", { platform: "linux", onWarning: (m) => warnings.push(m) }), + ).resolves.toBeUndefined() + + expect(fs.rmdir).toHaveBeenCalledTimes(1) + // The removal that reported ENOENT has to be the one aimed at this write's own + // staging directory: a cleanup that removed some other directory and got + // ENOENT is not this contract, and it would leave this directory behind. + const stagingDirs = vi.mocked(fsSync.mkdirSync).mock.calls.map((call) => String(call[0])) + expect(stagingDirs.length).toBe(1) + expect(fs.rmdir).toHaveBeenCalledWith(stagingDirs[0]) + expect(warnings).toEqual([]) + }) + + it("reports a staging directory it could not remove after a failed write, without replacing the write error", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const warnings: string[] = [] + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // The commit rename fails, and the post-failure cleanup then fails twice. + vi.mocked(fs.rename).mockRejectedValue(new Error("ENOSPC")) + vi.mocked(fs.rmdir).mockRejectedValue(Object.assign(new Error("EPERM"), { code: "EPERM" })) + + await expect( + safeWriteText(targetPath, "data", { platform: "linux", onWarning: (m) => warnings.push(m) }), + ).rejects.toThrow("ENOSPC") + + const stagingDirs = vi.mocked(fsSync.mkdirSync).mock.calls.map((call) => String(call[0])) + expect(stagingDirs.length).toBe(1) + expect(fs.rmdir).toHaveBeenCalledTimes(2) + expect(fs.rmdir).toHaveBeenNthCalledWith(2, stagingDirs[0]) + const reported = warnings.filter((m) => m.includes(stagingDirs[0])) + expect(reported).toHaveLength(1) + expect(reported[0]).toContain("could not remove the staging directory") + }) + + 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/.new_caller-staged.tmp" + 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() + }) + }) + + it("win32: a rejecting async onWarning does not abort the write or leak an unhandled rejection", async () => { + // TypeScript accepts an async sink where a void callback is expected, so the + // wrapper has to attach a handler to the returned promise: an unhandled + // rejection can end the process under Node's default mode, after a write that + // already succeeded. + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(execFile).mockImplementation((_cmd, _args, _opts, cb) => { + if (typeof cb === "function") cb(new Error("icacls error"), "", "") + return fakeChild + }) + const consoleWarn = vi.spyOn(console, "warn").mockImplementation(() => {}) + + await expect( + safeWriteText(targetPath, "data", { + platform: "win32", + onWarning: async () => { + throw new Error("async sink down") + }, + }), + ).resolves.toBeUndefined() + + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining(".file-safety-staging"), targetPath) + // The rejection is reported through the fallback sink rather than surfacing as an + // unhandled rejection. + expect(consoleWarn).toHaveBeenCalledWith(expect.stringContaining("onWarning callback rejected")) + consoleWarn.mockRestore() + }) + + // ── 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 + expect(fs.unlink).toHaveBeenCalledWith(expect.stringContaining("safeWriteText.bak_")) + + // the staging temp was already committed by the rename; nothing + // temp-shaped is unlinked afterwards + expect(fs.unlink).not.toHaveBeenCalledWith(expect.stringContaining("safeWriteText_")) + }) + + 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 let the failure cleanup unlink the file the commit already published", async () => { + // A tiny filesystem so that "the target is still there" is a STATE assertion: a cleanup + // that deleted the published file and a later step that recreated it would both satisfy + // a weaker "the content is at the target" check. + const targetPath = "/tmp/test-dir/target.txt" + const dirPath = path.dirname(targetPath) + const files = new Set([targetPath]) + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fs.rename).mockImplementation(async (from: unknown, to: unknown) => { + files.delete(String(from)) + files.add(String(to)) + }) + vi.mocked(fs.unlink).mockImplementation(async (p: unknown) => { + files.delete(String(p)) + }) + // The post-commit parent-directory fsync is the failure under test. + 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, + ) + + expect(files.has(targetPath)).toBe(true) + // The staged path is the committed file from the rename onwards: the cleanup must not + // reach for it at all. Before the commit point was tracked, every post-commit + // failure (directory fsync, DACL restore, a throwing warning sink) ran the temp + // unlink against the name the target now lives under. + const stagedPath = String(vi.mocked(fs.rename).mock.calls[0][0]) + expect( + vi.mocked(fs.unlink).mock.calls.filter(function (call) { + return String(call[0]) === stagedPath + }), + ).toHaveLength(0) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + expect(fs.rename).toHaveBeenCalledTimes(1) + }) + }) + + // ── Test 4: backup:true keeps old safeWriteJson semantics, copy-based ── + + 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("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("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() + }) + }) + + it("retries a failed post-commit backup cleanup once and reports the leftover path", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const onWarning = vi.fn() + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fsSync.closeSync).mockReturnValue(undefined) + // Step 6 removes the backup copy after the commit; the unlink keeps failing. + vi.mocked(fs.unlink).mockRejectedValue(new Error("EPERM")) + + await safeWriteText(targetPath, "data", { backup: true, platform: "linux", onWarning }) + + // The publish succeeded, so the write still resolves - but the leftover copy of the + // previous content must be retried once and then reported with its path, never dropped. + const backupPath = vi.mocked(fs.copyFile).mock.calls[0][1] + const backupUnlinks = vi.mocked(fs.unlink).mock.calls.filter(function (call) { + return String(call[0]).includes("safeWriteText.bak") + }) + expect(backupUnlinks.length).toBe(2) + expect( + backupUnlinks.map(function (call) { + return call[0] + }), + ).toEqual([backupPath, backupPath]) + expect(onWarning).toHaveBeenCalledTimes(1) + expect(String(onWarning.mock.calls[0][0])).toContain(String(backupPath)) + }) + + it("reports the exact orphan paths when cleanup fails after a failed write, keeping the write error", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const onWarning = vi.fn() + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fsSync.closeSync).mockReturnValue(undefined) + // The commit rename fails, and every cleanup unlink fails as well. + vi.mocked(fs.rename).mockRejectedValue(new Error("ENOSPC")) + vi.mocked(fs.unlink).mockRejectedValue(new Error("EACCES")) + + await expect(safeWriteText(targetPath, "data", { backup: true, platform: "linux", onWarning })).rejects.toThrow( + "ENOSPC", + ) + + const backupPath = vi.mocked(fs.copyFile).mock.calls[0][1] + const tempArg = vi.mocked(fs.rename).mock.calls[0][0] + // Both leftovers are retried once, and each leftover is reported with its exact path + // instead of being dropped - the caller still gets the original write error. + expect( + vi.mocked(fs.unlink).mock.calls.filter(function (call) { + return String(call[0]) === String(backupPath) + }).length, + ).toBe(2) + expect( + vi.mocked(fs.unlink).mock.calls.filter(function (call) { + return String(call[0]) === String(tempArg) + }).length, + ).toBe(2) + expect(onWarning).toHaveBeenCalledTimes(2) + const messages = onWarning.mock.calls.map(function (call) { + return String(call[0]) + }) + expect( + messages.some(function (m) { + return m.includes(String(backupPath)) + }), + ).toBe(true) + expect( + messages.some(function (m) { + return m.includes(String(tempArg)) + }), + ).toBe(true) + }) + + it("treats an already-gone staging temp as a completed cleanup, not as a leftover", async () => { + // ENOENT from the temp unlink means the goal is already met. Retrying it and then + // warning claimed an orphan that did not exist - a false alarm that trains the reader + // to ignore the leftover reports that matter. + const targetPath = "/tmp/test-dir/target.txt" + const onWarning = vi.fn() + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fs.rename).mockRejectedValue(new Error("ENOSPC")) + vi.mocked(fs.unlink).mockRejectedValue( + Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }), + ) + + await expect(safeWriteText(targetPath, "data", { platform: "linux", onWarning })).rejects.toThrow("ENOSPC") + + // ENOENT short-circuits: the goal is met, so there is nothing to retry and + // nothing to report. + expect(fs.unlink).toHaveBeenCalledTimes(1) + expect(onWarning).not.toHaveBeenCalled() + }) + + it("still reports a staging temp that could not be removed for any other reason", async () => { + // The tolerance is scoped to ENOENT. EACCES means the staged file is still on disk, and + // swallowing it would turn a false alarm into a false silence. + const targetPath = "/tmp/test-dir/target.txt" + const onWarning = vi.fn() + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fs.rename).mockRejectedValue(new Error("ENOSPC")) + vi.mocked(fs.unlink).mockRejectedValue( + Object.assign(new Error("EACCES: permission denied"), { code: "EACCES" }), + ) + + await expect(safeWriteText(targetPath, "data", { platform: "linux", onWarning })).rejects.toThrow("ENOSPC") + + const tempArg = vi.mocked(fs.rename).mock.calls[0][0] + expect(onWarning).toHaveBeenCalledTimes(1) + expect(String(onWarning.mock.calls[0][0])).toContain("could not remove the staging temp") + expect(String(onWarning.mock.calls[0][0])).toContain(String(tempArg)) + expect(String(onWarning.mock.calls[0][0])).toContain("EACCES") + }) + + it("treats an already-gone backup copy as removed instead of reporting a phantom orphan", async () => { + // Same rule on the backup copy: ENOENT means there is nothing left to recover by hand, + // so the warning would point at a file that is not there. + const targetPath = "/tmp/test-dir/target.txt" + const onWarning = vi.fn() + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fs.rename).mockRejectedValue(new Error("ENOSPC")) + const enoent = Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }) + vi.mocked(fs.unlink).mockImplementation(async (p: unknown) => { + if (String(p).includes("safeWriteText.bak_")) { + throw enoent + } + }) + + await expect(safeWriteText(targetPath, "data", { backup: true, platform: "linux", onWarning })).rejects.toThrow( + "ENOSPC", + ) + + const backupPath = String(vi.mocked(fs.copyFile).mock.calls[0][1]) + expect( + onWarning.mock.calls.filter(function (call) { + return String(call[0]).includes(backupPath) + }), + ).toHaveLength(0) + }) + + it("retries a failed DACL-dump cleanup once and reports the leftover dump path", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const onWarning = vi.fn() + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fsSync.closeSync).mockReturnValue(undefined) + // The dump unlink keeps failing. The published file is fine, but the dump is a text copy + // of the target's access rights that only this write knew the name of. + vi.mocked(fs.unlink).mockRejectedValue(new Error("EPERM")) + + await safeWriteText(targetPath, "data", { platform: "win32", onWarning }) + + const dumpUnlinks = vi.mocked(fs.unlink).mock.calls.filter(function (call) { + return String(call[0]).includes("safeWriteText.acl") + }) + expect(dumpUnlinks.length).toBe(2) + expect( + dumpUnlinks.map(function (call) { + return call[0] + }), + ).toEqual([dumpUnlinks[0][0], dumpUnlinks[0][0]]) + // Reported once, with the exact path, and the write still resolved. + const messages = onWarning.mock.calls.map(function (call) { + return String(call[0]) + }) + const dumpReports = messages.filter(function (m) { + return m.includes("could not remove the DACL dump") + }) + expect(dumpReports.length).toBe(1) + expect(dumpReports[0]).toContain(String(dumpUnlinks[0][0])) + }) + + it("reports the DACL dump path after a failed write without replacing the write error", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const onWarning = vi.fn() + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(fsSync.closeSync).mockReturnValue(undefined) + vi.mocked(fs.rename).mockRejectedValue(new Error("ENOSPC")) + vi.mocked(fs.unlink).mockRejectedValue(new Error("EPERM")) + + await expect(safeWriteText(targetPath, "data", { platform: "win32", onWarning })).rejects.toThrow("ENOSPC") + + const dumpUnlinks = vi.mocked(fs.unlink).mock.calls.filter(function (call) { + return String(call[0]).includes("safeWriteText.acl") + }) + // The commit span owns the dump removal, so the failure path must not retry the same file + // and report the same leftover twice. + expect(dumpUnlinks.length).toBe(2) + const messages = onWarning.mock.calls.map(function (call) { + return String(call[0]) + }) + expect( + messages.filter(function (m) { + return m.includes("could not remove the DACL dump") + }).length, + ).toBe(1) + expect( + messages.some(function (m) { + return m.includes("safeWriteText.acl") + }), + ).toBe(true) + }) + + it("backup:true closes the seed descriptor even when the copy fails", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.closeSync).mockReturnValue(undefined) + // Distinct descriptors, and the backup's own id recorded, so the assertion below cannot be + // satisfied by the close of some other descriptor. + let nextFd = 40 + const backupFds: number[] = [] + vi.mocked(fsSync.openSync).mockImplementation(function (p) { + const id = nextFd++ + if (String(p).includes("safeWriteText.bak")) backupFds.push(id) + return id + }) + // The copy is the statement that sits between the open and the close. + vi.mocked(fs.copyFile).mockRejectedValue(new Error("EIO: copy failed")) + + await expect(safeWriteText(targetPath, "data", { backup: true, platform: "linux" })).rejects.toThrow("EIO") + + expect(backupFds.length).toBe(1) + // Every successful openSync gets a close attempt, including this path, because an open + // descriptor holds the backup file and blocks the cleanup below. + expect(fsSync.closeSync).toHaveBeenCalledWith(backupFds[0]) + // And the half-made backup does not outlive the attempt. + expect( + vi.mocked(fs.unlink).mock.calls.some(function (call) { + return String(call[0]).includes("safeWriteText.bak") + }), + ).toBe(true) + }) + + // ── 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 falls back to plain rename (never fails the write)", 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 + }) + + await safeWriteText(targetPath, "data", { platform: "win32" }) + + // write succeeded despite icacls failure (fallback to plain rename) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining(".file-safety-staging"), targetPath) + // one icacls attempt only: a failed DACL apply must not try to restore + expect(execFile).toHaveBeenCalledTimes(1) + }) + + it("win32: reports that access rights may change when the DACL cannot be saved", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(execFile).mockImplementation((_cmd, _args, _opts, cb) => { + if (typeof cb === "function") cb(new Error("icacls error"), "", "") + return fakeChild + }) + const warnings: string[] = [] + + await safeWriteText(targetPath, "data", { platform: "win32", onWarning: (m) => warnings.push(m) }) + + // The write still commits - a failing icacls must not leave the user unable to save - + // but the caller is told the replacement may not carry the old ACL. + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining(".file-safety-staging"), targetPath) + expect(warnings.filter((m) => m.includes("different access rights"))).toHaveLength(1) + }) + + it("win32: reports when the target cannot be checked for DACL preservation", 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 be visible. + vi.mocked(fs.access).mockImplementation(async (p) => { + if (String(p) === targetPath) { + throw Object.assign(new Error("EACCES"), { code: "EACCES" }) + } + }) + const warnings: string[] = [] + + await safeWriteText(targetPath, "data", { platform: "win32", onWarning: (m) => warnings.push(m) }) + + expect(execFile).not.toHaveBeenCalled() + expect(warnings.filter((m) => m.includes("Could not check"))).toHaveLength(1) + }) + + // Warning delivery is advisory: it must not be able to fail the save it is reporting on. + it("win32: a throwing onWarning does not abort the write", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + vi.mocked(execFile).mockImplementation((_cmd, _args, _opts, cb) => { + if (typeof cb === "function") cb(new Error("icacls error"), "", "") + return fakeChild + }) + + await expect( + safeWriteText(targetPath, "data", { + platform: "win32", + onWarning: () => { + throw new Error("callback down") + }, + }), + ).resolves.toBeUndefined() + + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining(".file-safety-staging"), targetPath) + }) + + it("win32 DACL: a partial dump left by a failed save is removed and never restored", 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 + }) + + await safeWriteText(targetPath, "data", { platform: "win32" }) + + // write committed; only the save was attempted (no restore from a failed dump) + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + expect(fs.rename).toHaveBeenCalledTimes(1) + 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 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 save runs before the backup copy, not after it", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + + // The title is about order, so assert the order the mocks were actually + // called in. If the save ran after the backup copy the dump could describe a + // file that a concurrent publish had already replaced. + await safeWriteText(targetPath, "data", { backup: true, platform: "win32" }) + + const callOrder = vi.mocked(execFile).mock.invocationCallOrder + const copyOrder = vi.mocked(fs.copyFile).mock.invocationCallOrder + const renameOrder = vi.mocked(fs.rename).mock.invocationCallOrder + const saveCall = callOrder[0] + const restoreCall = callOrder[1] + const backupCopy = copyOrder[0] + const commitRename = renameOrder[0] + + expect(saveCall).toBeLessThan(backupCopy) + expect(backupCopy).toBeLessThan(commitRename) + expect(commitRename).toBeLessThan(restoreCall) + }) + + it("win32 DACL: a failed restore is reported and the dump is still unlinked", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + + // 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" }) + + // The content did commit: failing here would break every publish on a machine + // where icacls cannot reapply the saved ACEs. + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + expect(fs.rename).toHaveBeenCalledTimes(1) + + // The changed access rights are reported instead of being swallowed. + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("could not be restored")) + warnSpy.mockRestore() + + // dump file was still unlinked in finally + 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() + }) + + it("reports a partial DACL dump that the failed capture could not remove", async () => { + // A failed icacls /save can leave a partial dump behind. Dropping the path with a + // catch(() => {}) meant a transient EPERM - antivirus, a handle still being released - + // left a concrete file beside the target whose name nothing could recover. + const targetPath = "/tmp/test-dir/target.txt" + const onWarning = vi.fn() + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + vi.mocked(fsSync.openSync).mockReturnValue(1) + // The capture itself fails, so nothing may be restored onto the committed file. + vi.mocked(execFile).mockImplementation((_cmd, _args, _opts, cb) => { + if (typeof cb === "function") { + cb(new Error("icacls save error"), "", "") + } + return fakeChild + }) + vi.mocked(fs.unlink).mockRejectedValue( + Object.assign(new Error("EPERM: operation not permitted"), { code: "EPERM" }), + ) + + await safeWriteText(targetPath, "data", { platform: "win32", onWarning }) + + // The write still proceeds: a failing icacls must not leave the user unable to save. + expect(fs.rename).toHaveBeenCalledWith(expect.stringContaining("safeWriteText_"), targetPath) + // The partial dump is retried and then reported with its exact path. + const dumpUnlinks = vi.mocked(fs.unlink).mock.calls.filter(function (call) { + return String(call[0]).includes("safeWriteText.acl") + }) + expect(dumpUnlinks).toHaveLength(2) + expect( + onWarning.mock.calls.map(function (call) { + return String(call[0]) + }), + ).toEqual( + expect.arrayContaining([ + expect.stringContaining("could not remove the DACL dump"), + expect.stringContaining("safeWriteText.acl"), + ]), + ) + }) + }) + + // ── 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/.new_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/.new_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/.new_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/.new_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/.new_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("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("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() + }) + }) + 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) + }) + }) +}) + +// ── 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)) + // Only the link path is read, so a single answer is enough and keeps the mock's + // return type matching fs.promises.readlink. + vi.mocked(fs.readlink).mockResolvedValue("referent.json") + + // 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")), + ) + }) + + 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 a directory rather than a regular file", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const supplied = "/tmp/test-dir/staging-dir" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + // The symlink double above short-circuits on isSymbolicLink(), so the "another file + // type" branch is only reachable through a path that is neither link nor file. A + // directory would otherwise be renamed over the target, publishing a directory in + // place of the file. + vi.mocked(fs.lstat).mockResolvedValue(_notARegularFileStats()) + + await expect(safeWriteText(targetPath, "data", { tempPath: supplied, platform: "linux" })).rejects.toThrow( + "Staging file must be a regular file, not another file type", + ) + expect(fsSync.openSync).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + expect(fs.unlink).not.toHaveBeenCalled() + // The rejection names the path the caller has to clean up. + await expect( + safeWriteText(targetPath, "data", { tempPath: supplied, platform: "linux" }), + ).rejects.toMatchObject({ stagingPath: path.resolve(supplied) }) + }) + + 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 = _fileStatsWithIdentity(42n, 7n) + vi.mocked(fs.lstat).mockResolvedValue(stats) + + // A convention-conforming name: only the identity guard can reject this alias + // of the target, so the assertion pins the identity message rather than the + // error class - a non-conforming name would be rejected by the name check for + // the wrong reason and the guard could be deleted without reddening this test. + await expect( + safeWriteText(targetPath, "data", { tempPath: "/tmp/test-dir/.new_alias_1.tmp", platform: "linux" }), + ).rejects.toThrow("Staging file must not be the target itself") + 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 a caller-staged file that does not carry the staging name convention", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + // An arbitrary regular file beside the target passes the location, file-type and + // target-identity checks, yet renaming it onto the target would publish an + // unrelated file's content and delete its original name. Only the staging name + // convention this library's own writers use may be published. + await expect( + safeWriteText(targetPath, "data", { tempPath: "/tmp/test-dir/credentials.json", platform: "linux" }), + ).rejects.toThrow("Staging file must carry the staging name convention") + expect(fsSync.openSync).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + }) + + // NTFS hands an 8.3 alias to every path component longer than eight characters, and the hosted + // Windows runner's temp directory arrives in that spelling while the writer's resolved target + // arrives in the long spelling of the same directory; a junction or link does the same thing on + // any box. Two strings, one directory: the textual check above rejects a staging file that really + // is beside the target. What that check protects is the opposite - a staging file that is NOT + // beside the target, which is what makes the commit rename cross-filesystem (EXDEV) or lets a + // caller publish an unrelated file onto the target. Identity is that property, so a textual + // mismatch is resolved by comparing the two directories' dev and ino. path.resolve cannot settle + // it (it preserves whichever spelling it is given) and neither can fs.realpath: measured on node + // 22.23.4, Node's JavaScript realpath preserves an NTFS 8.3 spelling, and only + // fs.realpathSync.native expands it. These two tests drive the identity branch directly because a + // whole-file mock of fs/promises cannot present two spellings of a real directory. + it("accepts a caller-staged file whose directory is the target directory under another spelling", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const aliasDir = path.resolve("/tmp/alias-dir") + const targetDir = path.dirname(path.resolve(targetPath)) + // One directory under two spellings: each side hands back its own realpath, so only the pair's + // identity says they are the same directory. + // realpath here mirrors what node 22.23.4 does to an NTFS 8.3 name: it hands back the + // spelling it was given, so only the directory identity can tell an alias from another directory. + vi.mocked(fs.realpath).mockImplementation((p: unknown) => Promise.resolve(path.resolve(String(p)))) + vi.mocked(fs.stat).mockImplementation((p: unknown) => { + const s = String(p) + if (s === aliasDir || s === targetDir) { + return Promise.resolve(_fileStatsWithIdentity(4242n, 7n)) + } + return Promise.reject(Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" })) + }) + vi.mocked(fsSync.openSync).mockReturnValue(3) + + const customTempPath = path.join(aliasDir, ".new_target.txt_1_x.tmp") + + await safeWriteText(targetPath, "", { tempPath: customTempPath, platform: "linux" }) + + // The alias directory and the target directory are one directory, so the staging file is + // beside the target and the publish proceeds. + expect(fsSync.openSync).toHaveBeenCalledWith(customTempPath, "r+") + expect(fs.rename).toHaveBeenCalledWith(customTempPath, path.resolve(targetPath)) + }) + + it("still rejects a caller-staged file whose directory has a different identity", async () => { + const targetPath = "/tmp/test-dir/target.txt" + const otherDir = path.resolve("/tmp/other-dir") + const targetDir = path.dirname(path.resolve(targetPath)) + // Two directories that both resolve cleanly but are not the same directory: a staging file in + // one of them would publish an unrelated file onto the target, and across a mount the commit + // rename would fail with EXDEV. + // realpath here mirrors what node 22.23.4 does to an NTFS 8.3 name: it hands back the + // spelling it was given, so only the directory identity can tell an alias from another directory. + vi.mocked(fs.realpath).mockImplementation((p: unknown) => Promise.resolve(path.resolve(String(p)))) + vi.mocked(fs.stat).mockImplementation((p: unknown) => { + const s = String(p) + if (s === otherDir) { + return Promise.resolve(_fileStatsWithIdentity(9999n, 7n)) + } + if (s === targetDir) { + return Promise.resolve(_fileStatsWithIdentity(4242n, 7n)) + } + return Promise.reject(Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" })) + }) + vi.mocked(fsSync.openSync).mockReturnValue(3) + + const customTempPath = path.join(otherDir, ".new_target.txt_1_x.tmp") + + await expect(safeWriteText(targetPath, "", { tempPath: customTempPath, platform: "linux" })).rejects.toThrow( + "Staging file must sit in the target's directory", + ) + expect(fsSync.openSync).not.toHaveBeenCalled() + expect(fs.rename).not.toHaveBeenCalled() + }) + it("rejects a caller-staged file that gained a second hard link before the open", async () => { + const targetPath = "/tmp/test-dir/target.txt" + vi.mocked(fs.realpath).mockResolvedValue(targetPath) + // Between the lstat check and the open, an attacker who can write the directory + // can replace the staging path with a hard link to another file in it: the + // opened inode then carries a second name, and the commit rename would publish + // that other file and delete its original name. + const linked = _fileStats(false) + linked.nlink = 2 + vi.mocked(fsSync.fstatSync).mockReturnValue(linked) + vi.mocked(fsSync.openSync).mockReturnValue(7) + await expect( + safeWriteText(targetPath, "data", { tempPath: "/tmp/test-dir/.new_x_1.tmp", platform: "linux" }), + ).rejects.toThrow("Staging file changed between the check and the open") + expect(fs.rename).not.toHaveBeenCalled() + // The rejection lands after the open: the descriptor must be closed and the + // borrowed alias unlinked, so the failed write leaves no open handle and no + // leftover at the staging path. + expect(fsSync.closeSync).toHaveBeenCalledWith(7) + expect(fs.unlink).toHaveBeenCalledWith("/tmp/test-dir/.new_x_1.tmp") + }) + + 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 = _fileStatsWithIdentity(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 when a backed-up write fails before commit", () => { + 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)) + }) +}) diff --git a/src/services/file-safety/safeWriteText.ts b/src/services/file-safety/safeWriteText.ts new file mode 100644 index 0000000000..3846158a7e --- /dev/null +++ b/src/services/file-safety/safeWriteText.ts @@ -0,0 +1,858 @@ +import * as fs from "fs/promises" +import * as fsSync from "fs" +import * as path from "path" +import { execFile } from "child_process" + +export interface SafeWriteTextOptions { + /** + * When true, keep the old-file semantics without ever removing the target: the + * previous content is copied to a hidden backup path and flushed before the + * commit rename, the commit rename atomically replaces the target, and on success + * the backup copy is deleted. A failure before the commit leaves the target + * untouched (there is nothing to roll back) and removes the backup copy. When + * false (default) the atomic rename simply replaces the target. + */ + 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 + + /** + * Sink for non-fatal safety notices. A Windows DACL that could not be captured means the + * committed file may inherit different access rights: the write still proceeds (a missing or + * failing icacls must not block saving), but the caller is told instead of the change being + * silent. Defaults to console.warn. + */ + onWarning?: (message: string) => void + + /** + * 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 +} + +/** + * 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 rename succeeded 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) { + super( + "The rename committed 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 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 + } +} + +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.` + ) +} +// -- helpers --------------------------------------------------------------- + +/** 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 +} + +/** Remove this write's own staging directory, retrying once and reporting the exact path + * when it still fails. Shared by the success and the failure path so neither one silently + * discards a cleanup failure: an ENOTEMPTY from a racing writer, an EPERM while a handle + * inside the directory is still being released, or a transient filesystem error leaves a + * concrete directory on disk that no caller can find again. A cleanup failure must never + * un-commit a published file and must never replace the original write error, so it travels + * through the warning sink carrying the path a later cleanup pass needs. */ +async function _releaseStagingDir(stagingDir: string, warn: (message: string) => void): Promise { + let cleanupError: unknown = null + // Retry once: Windows reports EPERM while a handle inside the directory is still being + // released, and the second attempt usually succeeds. + for (let attempt = 0; attempt < 2; attempt++) { + try { + await fs.rmdir(stagingDir) + return + } catch (error: unknown) { + // A directory someone else already removed is exactly the goal, not a + // leftover to report. + if (errorCode(error) === "ENOENT") { + return + } + cleanupError = error + } + } + warn( + `safeWriteText: could not remove the staging directory ${stagingDir} (${ + cleanupError instanceof Error ? cleanupError.message : String(cleanupError) + }); it is left in place for a later cleanup pass`, + ) +} + +/** + * Remove this write's DACL dump. Retried once for the same reason the backup copy is: + * on Windows an unlink commonly reports EPERM while another handle to the file is still + * being released, although the file is gone a moment later. A dump that survives both + * attempts is a concrete file beside the target whose name no caller can recover, so the + * exact path is reported through the warning sink instead of being swallowed - on the + * success path as well as on the failure path, where it must also never replace the + * original write error. ENOENT means the goal is already met, so it is not a failure. + */ +async function _discardDaclDump(daclDumpPath: string, warn: (message: string) => void): Promise { + let cleanupError: unknown = null + for (let attempt = 0; attempt < 2; attempt++) { + try { + await fs.unlink(daclDumpPath) + return + } catch (error: unknown) { + if (errorCode(error) === "ENOENT") { + return + } + cleanupError = error + } + } + warn( + `safeWriteText: could not remove the DACL dump ${daclDumpPath} (${ + errorCode(cleanupError) ?? (cleanupError instanceof Error ? cleanupError.message : String(cleanupError)) + }); it is a text copy of the target's access rights and can be deleted.`, + ) +} + +function _fsyncFile(fd: number): void { + fsSync.fsyncSync(fd) +} + +/** Save the DACL of *srcPath* to a dump file on Windows. + * Returns true when the dump was written successfully; false otherwise. + * Never throws — callers treat failure as "skip DACL handling". */ +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 true + } catch { + return false + } +} + +/** Restore a DACL dump onto *dirPath* on Windows. + * Returns whether icacls succeeded; the caller reports a failure. */ +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 true + } catch { + return false + } +} + +// -- 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 (errorCode(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 (errorCode(lstatError) === "ENOENT") return undefined + throw lstatError + }) + if (linkStat?.isSymbolicLink()) throw error + return absoluteFilePath + }) +} +/** + * Distinguish "the target does not exist" from a real I/O failure (EACCES, + * EIO, ...). The mode-preservation path may only fall back to the fresh-file + * default on ENOENT; any other failure is propagated, otherwise a restrictive + * target (0o600) would be published with the default 0o644 through the rename. + */ +function errorCode(error: unknown): string | undefined { + return typeof error === "object" && error !== null && "code" in error + ? String((error as { code: unknown }).code) + : undefined +} + +/** + * 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) + const canonicalDir = await fs.realpath(dirPath).catch(() => dirPath) + return path.join(canonicalDir, path.basename(absoluteFilePath)) +} + +/** + * 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 (a publish renames the staged file onto the referent, + * and backup mode keeps a copy beside it). + * 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 (error: unknown) { + // Only a dangling link (ENOENT) may fall through to the walk: any other + // resolution error (EACCES, EIO) would key this writer to the alias path + // while a writer reaching the referent directly keys the referent, and + // the two writers would then take different locks for one file. + if (errorCode(error) !== "ENOENT") { + throw error + } + // 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) + const dirPath = path.dirname(targetPath) + + // Ensure parent directory exists (mirrors safeWriteJson behaviour). + await fs.mkdir(dirPath, { recursive: true }) + 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 + // The commit point of this write. Set the moment the rename succeeds: from that instant + // tempPath IS the target (the rename moved the staged file onto it), so the failure cleanup + // below must not unlink tempPath any more - a post-commit failure (the parent-directory fsync, + // the DACL restore, a throwing warning sink) would otherwise delete the file that was just + // published. The success path already knows this; the catch is the side that needs the flag. + let committed = false + let tempPath: string + 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)) { + // A textual mismatch is not yet evidence that the staging file sits elsewhere, because one + // directory answers to more than one spelling. NTFS hands out an 8.3 alias for every name + // longer than eight characters - the hosted Windows runner's temp directory arrives as + // RUNNER~1 under the profile while the writer's resolved target arrives in the long + // spelling - and a junction or link does the same thing on any box. path.resolve preserves + // whichever spelling it is handed, and fs.realpath (the JavaScript implementation Node 22 + // uses) preserves it too; only realpathSync.native expands the 8.3 form, measured on + // node 22.23.4. So neither a textual nor a plain-realpath comparison can tell an alias of + // the target's own directory from a different directory. The property this guard protects - + // no cross-filesystem commit rename (EXDEV) and no publishing of an unrelated file onto the + // target - is a property of the directory's identity, so compare identity: dev and ino, read + // as bigint for the same reason as the file check below. A directory that cannot be stat'd + // keeps the rejection, so this only ever accepts what the textual check would have accepted + // had the spelling been canonical on both sides. + const suppliedDir = path.dirname(supplied) + const [suppliedDirStat, targetDirStat] = await Promise.all([ + fs.stat(suppliedDir, { bigint: true }).catch(() => undefined), + fs.stat(dirPath, { bigint: true }).catch(() => undefined), + ]) + const sameDirectory = + suppliedDirStat !== undefined && + targetDirStat !== undefined && + suppliedDirStat.dev === targetDirStat.dev && + suppliedDirStat.ino === targetDirStat.ino + if (!sameDirectory) { + 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 (errorCode(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) + } + // Ownership convention: a caller-staged file must carry the staging name this + // library's own writers use (.new__.tmp). An arbitrary regular file + // beside the target (e.g. credentials.json) passes every check above, yet + // renaming it onto the target publishes an unrelated file's content and deletes + // its original name. Only files created under the staging convention may be + // published through this parameter. + if (!/^\.new_.*\.tmp$/.test(path.basename(supplied))) { + throw new StagingPathError( + `Staging file must carry the staging name convention .new_*.tmp, got ${path.basename(supplied)}`, + supplied, + ) + } + // 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 + // 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 + // Warning delivery must never abort the write: the notices below describe a + // committed-but-imperfect publish, and a caller whose callback throws (a UI sink, + // a logger that is mid-restart) must not turn that into a failed save. + const warn = (message: string) => { + const report = (label: string, error: unknown) => { + console.warn( + `safeWriteText: onWarning callback ${label}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + try { + const sink = options?.onWarning ?? ((m: string) => console.warn(m)) + const result: unknown = sink(message) + // A sink may be async - TypeScript accepts a value-returning callback where + // a void one is expected. Awaiting it would let warning delivery delay a + // write that has already committed (and hang it if the sink never settles), + // while leaving the promise unhandled turns a rejection into an unhandled + // rejection, which under Node's default mode can end the process after a + // successful write. Attach a handler without awaiting. + if (result instanceof Promise) { + result.catch((error: unknown) => report("rejected", error)) + } + } catch (error: unknown) { + report("failed", error) + } + } + + 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 (errorCode(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. + let offset = 0 + while (offset < buffer.length) { + offset += fsSync.writeSync(fd, buffer, offset, buffer.length - offset) + } + _fsyncFile(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 (errorCode(error) !== "ENOENT") throw error + // target does not exist yet - keep the temp's default mode + } + const fd = fsSync.openSync(tempPath, "r+") + try { + // Prove the opened file is not a borrowed name: between the lstat check + // and this open, a local attacker who can write the directory can replace + // the staging path with a hard link to another file in it, and the commit + // rename would then publish that file and delete its original name. While + // the other name exists the inode carries a second link, so nlink === 1 on + // the opened fd rules the substitution out; a swap that first unlinks the + // other name can only stage content the attacker already had, not a file + // the caller still references. + const opened = fsSync.fstatSync(fd) + if (!opened.isFile() || opened.nlink !== 1) { + throw new StagingPathError( + `Staging file changed between the check and the open: ${tempPath}`, + tempPath, + ) + } + if (targetMode !== null) { + fsSync.fchmodSync(fd, targetMode) + } + _fsyncFile(fd) + } finally { + fsSync.closeSync(fd) + } + } + + // -- Step 2 (win32): save DACL BEFORE the backup copy ----------- + 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 saved = await _saveDaclWindows(targetPath, dumpPath, options?.execFileRunner) + if (saved) { + // 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 so + // no partial dump survives and no later step can restore from it. The + // cleanup is the same retrying, reporting one used for a successful dump: + // a partial file is still a concrete file beside the target, and a + // transient EPERM (antivirus, a handle still being released) that survives + // the retry has to leave the exact path behind, not vanish into a catch. + await _discardDaclDump(dumpPath, warn) + // The target exists and its DACL could not be captured, so the commit rename + // replaces it with a file that inherits different access rights. The write still + // proceeds - a missing or failing icacls must not leave the user unable to save - + // but the replacement is no longer ACL-identical and that has to be visible + // instead of silent. + warn( + `Could not save the DACL of ${targetPath}; the replacement may inherit different access rights.`, + ) + } + } else if (errorCode(accessError) !== "ENOENT") { + // Not "absent": the target is there but could not be checked (EACCES, ...), so + // DACL preservation was skipped for a reason the caller cannot infer from the + // successful write alone. + warn( + `Could not check ${targetPath} for DACL preservation (${errorCode(accessError) ?? "unknown error"}); the replacement may inherit different access rights.`, + ) + } + } + try { + // -- Step 3 (backup:true): durable copy target -> backup ---- + if (options?.backup) { + try { + await fs.access(targetPath) + 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) + try { + // Nothing runs here today: the descriptor exists only to create the destination with + // a fixed mode. The block is what guarantees the close below runs for every + // successful openSync, including any statement added between the open and the close - + // an unclosed descriptor holds the backup open and blocks the cleanup that has to + // remove that file. + } finally { + 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 { + _fsyncFile(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 + for (let attempt = 0; attempt < 2; attempt++) { + try { + await fs.unlink(orphanPath) + backupCleanupError = null + break + } catch (cleanupError: unknown) { + backupCleanupError = errorCode(cleanupError) === "ENOENT" ? null : cleanupError + } + } + backupPath = null + if (backupCleanupError !== null) { + throw new OrphanedBackupError(orphanPath, targetPath, backupError, backupCleanupError) + } + throw backupError + } + releaseBackupOnSuccess = true + } catch (err: unknown) { + if (errorCode(err) !== "ENOENT") throw err + } + } + + // -- Step 4: atomic rename temp -> target --------------------- + // A symlink can be retargeted while the temp file is being written: re-resolve + // immediately before the commit rename so the locked resolution cannot publish + // onto a sibling file that replaced the referent mid-write. + const recomputedTargetPath = await resolvePublishTarget(absoluteFilePath) + if (recomputedTargetPath !== targetPath) { + throw new Error( + `safeWriteText: refusing to commit ${absoluteFilePath}: the resolved target changed from ${targetPath} to ${recomputedTargetPath} while the write was in progress`, + ) + } + + await fs.rename(tempPath, targetPath) + committed = true + + // -- 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 { + const dirFd = fsSync.openSync(dirPath, "r") + try { + _fsyncFile(dirFd) + } finally { + fsSync.closeSync(dirFd) + } + } 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 restored = await _restoreDaclWindows(restoredDir, daclDumpPath, options?.execFileRunner) + if (!restored) { + // The content is committed, but the published file may carry a different DACL + // from the one that was saved. Failing the write here would break every + // publish on machines where icacls cannot reapply the saved ACEs (a plain + // temp directory restore fails with "Not all privileges or groups referenced + // are assigned to the caller"), so the change of access rights is reported + // rather than thrown. + warn( + `safeWriteText: content committed at ${targetPath}, but the saved DACL could not be restored from ${daclDumpPath}; the file may carry different access rights than the one it replaced.`, + ) + } + } + + // -- Step 6 (backup:true): delete backup on success ----------- + if (releaseBackupOnSuccess && backupPath) { + // The backup is a full copy of the previous content sitting next to the + // published file. Dropping its path on a failed unlink would leave an artifact + // that no caller can find or remove, so the unlink is retried once (Windows + // commonly reports EPERM while another handle is still being released) and a + // persistent failure is reported with the path instead of swallowed. The publish + // itself succeeded, so the write still resolves: this is a leftover to clean up, + // not a failed save. + let backupRemoved = false + for (let attempt = 0; attempt < 2 && !backupRemoved; attempt++) { + try { + await fs.unlink(backupPath) + backupRemoved = true + } catch (cleanupError: unknown) { + if (errorCode(cleanupError) === "ENOENT") { + // Already gone: the cleanup goal is met, nothing to report. + backupRemoved = true + } else if (attempt === 1) { + warn( + `safeWriteText: committed ${targetPath} but could not remove its backup copy at ${backupPath} (${ + errorCode(cleanupError) ?? "unknown error" + }); the copy of the previous content is still on disk and needs to be removed.`, + ) + } + } + } + } + } finally { + // Unlink DACL dump regardless of success/failure in this span. A dump that survives is an + // artifact nobody else can name, so a failure is retried and reported with its path. + if (daclDumpPath !== null) { + await _discardDaclDump(daclDumpPath, warn) + // This span owns the removal: the catch below must not retry the same file and report + // the same leftover twice. + daclDumpPath = null + } + } + + // 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 it is reported through the warning sink with + // the leftover path instead of being swallowed. + if (stagingDir) { + await _releaseStagingDir(stagingDir, warn) + } + } catch (originalError: unknown) { + // The backup is a COPY taken before the commit, never a rename of the target, + // so there is nothing to restore here: the target still holds whatever the + // commit left - the pre-write content when the commit never ran, the published + // content when it did. The copy has served its purpose and must not be left + // beside the target where no caller can find it. + if (backupPath && releaseBackupOnSuccess) { + // Retry once (Windows reports EPERM while a handle is still being released), and + // if it still fails keep the path: clearing it would drop the only reference to + // the orphan. The original write error is what propagates; the leftover is + // reported through the warning sink with both paths. + let cleanupError: unknown = null + for (let attempt = 0; attempt < 2; attempt++) { + try { + await fs.unlink(backupPath) + cleanupError = null + break + } catch (error: unknown) { + // A copy that is already gone is not a leftover: reporting it would claim a + // recoverable orphan that no longer exists, and leaving backupPath set would + // tell the reader the copy is still there. + if (errorCode(error) === "ENOENT") { + cleanupError = null + break + } + cleanupError = error + } + } + if (cleanupError) { + warn( + `safeWriteText: the write to ${targetPath} failed and its backup copy could not be removed at ${backupPath} (${ + cleanupError instanceof Error ? cleanupError.message : String(cleanupError) + }); the copy is left in place so the previous content is still recoverable by hand`, + ) + } else { + backupPath = null + } + } + // Only a write that never committed has a staged file to remove. After the commit + // rename there is nothing at tempPath but the published target, and unlinking it here + // would un-commit the write the caller is being told about. + if (!committed) { + let cleanupError: unknown = null + for (let attempt = 0; attempt < 2; attempt++) { + try { + await fs.unlink(tempPath) + cleanupError = null + break + } catch (error: unknown) { + // ENOENT means the goal is already met - the temp is gone - which is not a + // leftover and must not raise the warning below. + if (errorCode(error) === "ENOENT") { + cleanupError = null + break + } + cleanupError = error + } + } + if (cleanupError) { + // A leftover staged temp is not the caller's failure, but its path must not + // be dropped silently: report it and keep the original error propagating. + warn( + `safeWriteText: could not remove the staging temp ${tempPath} (${ + cleanupError instanceof Error ? cleanupError.message : String(cleanupError) + })`, + ) + } + } + + // 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: the original + // write error keeps propagating, and a directory that still cannot be removed is + // reported with its path rather than discarded. + if (stagingDir) { + await _releaseStagingDir(stagingDir, warn) + } + + // Reported through the warning sink with its path, and never in place of originalError. + if (daclDumpPath !== null) { + await _discardDaclDump(daclDumpPath, warn) + } + + throw originalError + } +} diff --git a/src/utils/__tests__/jsonFileRecovery.test.ts b/src/utils/__tests__/jsonFileRecovery.test.ts index 1bd90d8935..6e67938010 100644 --- a/src/utils/__tests__/jsonFileRecovery.test.ts +++ b/src/utils/__tests__/jsonFileRecovery.test.ts @@ -107,3 +107,59 @@ describe("reader recovery", () => { await expect(fs.stat(dir)).rejects.toMatchObject({ code: "ENOENT" }) }) }) + +describe("replacement leftover name patterns", () => { + let dir: string + let target: string + beforeEach(async () => { + dir = await fs.mkdtemp(path.join(os.tmpdir(), "json-recovery-names-")) + target = path.join(dir, "a.json") + await fs.writeFile(target, JSON.stringify({ committed: true })) + }) + afterEach(async () => { + vi.restoreAllMocks() + await fs.rm(dir, { recursive: true, force: true }) + }) + + // The three names recovery accepts. The first two are the staging and backup + // names safeWriteJson used before it published through safeWriteText; the third + // is the name safeWriteText requires of a caller-staged file (.new_*.tmp), with + // this file's basename embedded after the anchor so the name stays scoped. + it.each([".a.json.new_1_legacy.tmp", ".a.json.bak_1_legacy.tmp", ".new_a.json_1_staged.tmp"])( + "garbage collects the accepted leftover named %s when the canonical file exists", + async (name: string) => { + const leftover = path.join(dir, name) + await fs.writeFile(leftover, "stale") + await expect(recoverMissingJsonFile(target)).resolves.toBe(true) + expect(JSON.parse(await fs.readFile(target, "utf8"))).toEqual({ committed: true }) + await expect(fs.access(leftover)).rejects.toMatchObject({ code: "ENOENT" }) + }, + ) + + // A shared directory is the failure mode the target-derived prefix exists to + // prevent: a live writer of b.json holds only b.json's lock, so recovery of a.json + // must not unlink the staging file that writer is about to rename onto b.json. + it("leaves a sibling target's staging file alone when two targets share one directory", async () => { + const sibling = path.join(dir, "b.json") + await fs.writeFile(sibling, JSON.stringify({ sibling: true })) + const mine = path.join(dir, ".new_a.json_1_mine.tmp") + const theirs = path.join(dir, ".new_b.json_2_theirs.tmp") + await fs.writeFile(mine, "stale") + await fs.writeFile(theirs, "in flight") + await expect(recoverMissingJsonFile(target)).resolves.toBe(true) + await expect(fs.access(mine)).rejects.toMatchObject({ code: "ENOENT" }) + expect(await fs.readFile(theirs, "utf8")).toBe("in flight") + expect(JSON.parse(await fs.readFile(sibling, "utf8"))).toEqual({ sibling: true }) + }) + + // The negative boundary: safeWriteText accepts any basename matching .new_*.tmp, so + // an unscoped name proves nothing about which target staged it, and recovery must + // not treat it as this target's leftover. + it("does not collect a bare .new_ staging file that is not scoped to this target", async () => { + const unscoped = path.join(dir, ".new_unscoped.tmp") + await fs.writeFile(unscoped, "someone else's staging file") + await expect(recoverMissingJsonFile(target)).resolves.toBe(false) + expect(await fs.readFile(unscoped, "utf8")).toBe("someone else's staging file") + expect(JSON.parse(await fs.readFile(target, "utf8"))).toEqual({ committed: true }) + }) +}) diff --git a/src/utils/__tests__/safeWriteJson.interruption.test.ts b/src/utils/__tests__/safeWriteJson.interruption.test.ts index c9c5993f84..5840f9bea6 100644 --- a/src/utils/__tests__/safeWriteJson.interruption.test.ts +++ b/src/utils/__tests__/safeWriteJson.interruption.test.ts @@ -1,4 +1,4 @@ -import { spawn, type ChildProcess } from "child_process" +import { execFileSync, spawn, type ChildProcess } from "child_process" import { createRequire } from "module" import { fileURLToPath } from "url" import * as fs from "fs/promises" @@ -29,15 +29,72 @@ const fs = require("fs") const path = require("path") const targetPath = path.resolve(process.env.INTERRUPTION_TARGET_PATH) +// The commit goes through safeWriteText, which renames onto the publish target it resolved +// itself. One directory answers to two spellings on NTFS - every component longer than eight +// characters also has an 8.3 alias, so the CI runner's temp directory arrives short-named +// (RUNNER~1 under the user profile) while realpath hands back the long spelling of the same +// directory - and a junction or a link does the same thing on any box. The writer's canonical +// handed to this child can therefore name the same file as different text, and a textual +// comparison would then let the commit land unnoticed. Canonicalise this side the same way, +// and fall back to the textual form for a target that does not exist yet and cannot be +// canonicalised. +const identityOf = (entry) => { + try { + const stat = fs.statSync(entry, { bigint: true }) + return stat.dev + ":" + stat.ino + } catch { + return null + } +} +const sameEntry = (candidate, expected) => { + const resolved = path.resolve(String(candidate)) + const wanted = path.resolve(String(expected)) + if (resolved === wanted) { + return true + } + if (process.platform === "win32" && resolved.toLowerCase() === wanted.toLowerCase()) { + return true + } + const candidateIdentity = identityOf(resolved) + const wantedIdentity = identityOf(wanted) + if (candidateIdentity !== null && wantedIdentity !== null && candidateIdentity === wantedIdentity) { + return true + } + // A destination that is not on disk yet has no identity of its own, so compare the directory it + // would land in and the name it would land under: same directory identity and same basename is + // the same file, whatever spelling either side used. + const candidateName = path.basename(resolved) + const wantedName = path.basename(wanted) + if ( + process.platform === "win32" + ? candidateName.toLowerCase() !== wantedName.toLowerCase() + : candidateName !== wantedName + ) { + return false + } + const candidateDirIdentity = identityOf(path.dirname(resolved)) + const wantedDirIdentity = identityOf(path.dirname(wanted)) + return ( + candidateDirIdentity !== null && wantedDirIdentity !== null && candidateDirIdentity === wantedDirIdentity + ) +} +// Text and fs.realpath cannot decide this one. On the hosted Windows runner one directory answers +// under two spellings and the canonicaliser hands back the short form for the profile component +// (RUNNER~1), so the writer's canonical destination and the parent's canonical target are different +// text for one file. Measured here: realpath returns whichever spelling it is handed while fs.stat +// reports a single identity for both; measured on the runner: the staging guard reported the long +// spelling expected against a short got for the same directory. Identity is the only spelling both +// sides agree on, so the interception decision is taken on identity. +const isCommitRename = (newPath) => sameEntry(newPath, targetPath) const fsPromises = require("fs/promises") const actualRename = fsPromises.rename // Fault injection: hard-kill before the replacement lands. Any rename -// whose destination is the canonical path is the commit step, whatever +// whose destination is the canonical path (or an alias of it) is the commit step, whatever // replacement protocol the writer uses; exit before performing it. let commitAttempts = 0 fsPromises.rename = async function interruptionRename(oldPath, newPath) { - if (path.resolve(String(newPath)) === targetPath) { + if (isCommitRename(newPath)) { commitAttempts++ if (process.env.INTERRUPTION_MODE === "fallback" && commitAttempts === 1) { return Promise.reject(Object.assign(new Error("Injected EPERM"), { code: "EPERM" })) @@ -51,6 +108,17 @@ fsPromises.rename = async function interruptionRename(oldPath, newPath) { return actualRename.apply(fsPromises, arguments) } +// Matcher self-check mode: answer the interception decision for candidates handed in from the +// parent, without building or writing anything. This is how the alias decisions are pinned by a +// test on a real second spelling instead of being trusted to a platform that cannot show one. +if (process.env.INTERRUPTION_MODE === "matcher") { + const cases = JSON.parse(process.env.INTERRUPTION_MATCHER_CASES || "[]") + for (const testCase of cases) { + fs.writeSync(1, "MATCH " + testCase.label + "=" + isCommitRename(testCase.candidate) + String.fromCharCode(10)) + } + process.exit(0) +} + require(process.env.ESBUILD_PATH).buildSync({ entryPoints: [process.env.SAFE_WRITE_JSON_ENTRY], bundle: true, @@ -239,4 +307,170 @@ describe("safeWriteJson subprocess interruption fault injection", () => { }, 45_000, ) + + // The child intercepts the commit rename by comparing the rename's destination with + // INTERRUPTION_TARGET_PATH. The commit now goes through safeWriteText, which renames onto the + // publish target it resolved itself, so the destination is not necessarily the spelling the + // parent passed down. On NTFS one directory answers to two spellings: every component longer + // than eight characters also has an 8.3 alias, and the Windows CI runner's temp directory + // arrives as C:\Users\RUNNER~1\... while realpath hands back C:\Users\runneradmin\... The + // interception has to survive that. A junction gives any Windows box the same shape without + // needing privileges: this case spells the target through an alias of the directory the + // writer resolves to, so the parent's spelling and the writer's canonical spelling of the same + // directory differ as text on a box that has no 8.3 alias on its own temp path. + it.runIf(process.platform === "win32")( + "intercepts the commit rename when the target path is spelled through a directory alias", + async () => { + const fileName = "ui_messages.json" + const message = { + ts: 1, + type: "say", + say: "text", + text: "RESTART_PERSISTENCE_MARKER", + role: "assistant", + content: "RESTART_PERSISTENCE_MARKER", + } + const committed = [message] + const replacement = [{ ...message, ts: 2, text: "ALIASED_WRITE", content: "ALIASED_WRITE" }] + await safeWriteJson(targetPath, committed) + + const aliasDir = path.join(tempDir, "storage-alias") + try { + await fs.symlink(storageDir, aliasDir, "junction") + } catch (error: unknown) { + const code = typeof error === "object" && error !== null ? (error as { code?: string }).code : undefined + // A box that cannot create a junction cannot present a second spelling of the + // directory; there is nothing to intercept differently and no alias to reject. + if (code === "EPERM" || code === "ENOSYS" || code === "EEXIST") { + return + } + throw error + } + const aliasTargetPath = path.join(aliasDir, fileName) + expect(path.resolve(aliasTargetPath)).not.toBe(path.resolve(targetPath)) + + const child = spawn(process.execPath, [childScriptPath], { + env: { + ...process.env, + INTERRUPTION_TARGET_PATH: aliasTargetPath, + INTERRUPTION_MODE: "atomic", + INTERRUPTION_BOUNDARY: "before-commit", + INTERRUPTION_PAYLOAD: JSON.stringify(replacement), + ESBUILD_PATH: esbuildPath, + SAFE_WRITE_JSON_ENTRY: safeWriteJsonEntryPath, + INTERRUPTION_BUNDLE_PATH: childBundlePath, + }, + stdio: ["ignore", "pipe", "pipe"], + }) + const exit = await waitForChildExit(child) + + expect(exit.stderr).not.toContain("WRITE_FAILED") + expect(exit.stdout).toContain("EXIT_AT_REPLACEMENT_RENAME") + expect(exit.stdout).not.toContain("WRITE_COMPLETED") + expect(exit.code).toBe(1) + // The canonical target is untouched by an interrupted commit, spelled through either name. + expect(JSON.parse(await readFileWithMissingRetry(targetPath))).toEqual(committed) + expect(JSON.parse(await readFileWithMissingRetry(aliasTargetPath))).toEqual(committed) + const interruptedFiles = await fs.readdir(storageDir) + expect(interruptedFiles.filter((entry) => entry.includes(".new_"))).toHaveLength(1) + + const staleTime = new Date(Date.now() - 60_000) + await fs.utimes(`${targetPath}.lock`, staleTime, staleTime) + const recovered = { ui_messages: [{ ts: 3, type: "say", text: "RECOVERED_AFTER_RESTART" }] } + await safeWriteJson(aliasTargetPath, recovered) + expect(JSON.parse(await readFileWithMissingRetry(targetPath))).toEqual(recovered) + const leftovers = (await fs.readdir(storageDir)).filter( + (entry) => entry.includes(".new_") || entry.includes(".bak_"), + ) + expect(leftovers).toEqual([]) + }, + 45_000, + ) + + // The interception decision, taken on a directory that answers to two spellings. The + // writer hands the child hook the spelling its own canonicalisation produced, and on the hosted + // Windows runner that spelling is the 8.3 alias of the directory the parent created, so the hook + // has to recognise the commit rename from the file identity rather than from the text it is + // handed. Windows only: there is no short-name alias elsewhere, and a volume that does not + // generate them cannot present one here either. + const shortSpellingOf = (directory: string): string | null => { + if (process.platform !== "win32") { + return null + } + try { + const out = execFileSync("cmd.exe", ["/c", "for %A in (" + directory + ") do @echo %~sA"], { + encoding: "utf8", + }).trim() + return out.includes("~1") ? out : null + } catch { + return null + } + } + + it("recognises the commit rename through a short-name alias, and refuses another directory", async () => { + const base = await fs.mkdtemp(path.join(os.tmpdir(), "interruption-matcher-")) + const longDir = path.join(base, "LONGMATCHERDIRECTORYNAME") + const otherDir = path.join(base, "OTHERMATCHERDIRECTORYNAME") + await fs.mkdir(longDir, { recursive: true }) + await fs.mkdir(otherDir, { recursive: true }) + const shortDir = shortSpellingOf(longDir) + const shortOther = shortSpellingOf(otherDir) + if (shortDir === null || shortOther === null) { + return + } + const fileName = "target.json" + await fs.writeFile(path.join(longDir, fileName), "committed", "utf8") + + const runMatcher = async ( + targetPath: string, + candidates: Array<[string, string]>, + ): Promise> => { + const child = spawn(process.execPath, [childScriptPath], { + env: { + ...process.env, + INTERRUPTION_TARGET_PATH: targetPath, + INTERRUPTION_MODE: "matcher", + INTERRUPTION_MATCHER_CASES: JSON.stringify( + candidates.map(([label, candidate]) => ({ label, candidate })), + ), + ESBUILD_PATH: esbuildPath, + SAFE_WRITE_JSON_ENTRY: safeWriteJsonEntryPath, + INTERRUPTION_BUNDLE_PATH: childBundlePath, + }, + stdio: ["ignore", "pipe", "pipe"], + }) + const exit = await waitForChildExit(child) + const decisions: Record = {} + for (const line of exit.stdout.split(String.fromCharCode(10))) { + const match = line.match(/^MATCH ([^=]+)=(true|false)$/) + if (match) { + decisions[match[1]] = match[2] + } + } + return decisions + } + + // Same file, spelled through the alias: the decision must be yes from either side's spelling. + const fromLong = await runMatcher(path.join(longDir, fileName), [ + ["self", path.join(longDir, fileName)], + ["alias", path.join(shortDir, fileName)], + ["other-directory-file", path.join(otherDir, fileName)], + ["other-directory-alias", path.join(shortOther ?? otherDir, fileName)], + ]) + expect(fromLong).toEqual({ + self: "true", + alias: "true", + "other-directory-file": "false", + "other-directory-alias": "false", + }) + + const fromShort = await runMatcher(path.join(shortDir, fileName), [ + ["self", path.join(shortDir, fileName)], + ["alias", path.join(longDir, fileName)], + ["other-directory-file", path.join(otherDir, fileName)], + ]) + expect(fromShort).toEqual({ self: "true", alias: "true", "other-directory-file": "false" }) + + await fs.rm(base, { recursive: true, force: true }) + }, 30_000) }) diff --git a/src/utils/__tests__/safeWriteJson.lockKey.spec.ts b/src/utils/__tests__/safeWriteJson.lockKey.spec.ts new file mode 100644 index 0000000000..20676c159b --- /dev/null +++ b/src/utils/__tests__/safeWriteJson.lockKey.spec.ts @@ -0,0 +1,217 @@ +// npx vitest run utils/__tests__/safeWriteJson.lockKey.spec.ts + +import * as os from "os" +import path from "path" +import type { BigIntStats } from "fs" +import * as fs from "fs/promises" +import { acquireFileLock } from "../fileLock" +import { safeWriteJson } from "../safeWriteJson" +import { resolveLockKey } from "../../services/file-safety/safeWriteText" + +vi.mock("../fileLock", () => ({ + acquireFileLock: vi.fn(async () => async () => {}), +})) + +vi.mock("fs/promises", async () => { + const actual = await vi.importActual("fs/promises") + return { ...actual, realpath: vi.fn(), lstat: vi.fn(), readlink: vi.fn() } +}) + +const mockedRealpath = vi.mocked(fs.realpath) +const mockedLstat = vi.mocked(fs.lstat) +const mockedReadlink = vi.mocked(fs.readlink) +const mockedAcquireFileLock = vi.mocked(acquireFileLock) + +const enoent = Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }) + +// Each test creates a real temp directory so the real fs calls still work. +// doubles between tests so an implementation from one test cannot carry over. +const createdDirs: string[] = [] +async function makeDir(prefix: string): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)) + createdDirs.push(dir) + return dir +} + +beforeEach(() => { + mockedRealpath.mockReset() + mockedLstat.mockReset() + mockedReadlink.mockReset() + mockedAcquireFileLock.mockReset() +}) + +afterEach(async () => { + for (const dir of createdDirs) { + await fs.rm(dir, { recursive: true, force: true }).catch(() => undefined) + } + createdDirs.length = 0 +}) + +// Only isSymbolicLink() is consulted by the guard, so the double carries just +// that method. The mocks reject asynchronously: a synchronous throw would bypass +// resolvePublishTarget's catch and skip the ENOENT/symlink branch under test. +const symlinkStat = (target: unknown) => + ({ + isSymbolicLink: () => target === currentLink, + // The staging-path check in safeWriteText also asks whether the path is a + // regular file, so the double carries that predicate as well. + isFile: () => target !== currentLink, + }) as unknown as BigIntStats +let currentLink = "" + +describe("safeWriteJson lock key under a peer commit", () => { + it("waits for the peer instead of rejecting, and locks the referent", async () => { + const order: string[] = [] + const dir = await makeDir("lockkey-") + const referent = path.join(dir, "history_item.json") + currentLink = path.join(dir, "link.json") + + // The peer writer has renamed the referent away and has not committed yet, + // so the first resolution fails with ENOENT while lstat still reports a + // symbolic link. A strict resolve here rejects the caller before it can ever + // queue behind the peer, and the caller's delta write is lost. + mockedRealpath + .mockImplementationOnce(async () => { + order.push("resolve-failed") + throw enoent + }) + .mockImplementation(async (target) => { + order.push("resolve") + // The second call happens under the lock, where the peer has committed. + return target === currentLink ? referent : String(target) + }) + mockedLstat.mockImplementation(async (target) => { + order.push("lstat") + return symlinkStat(target) + }) + mockedReadlink.mockImplementation(async (target) => + target === currentLink ? referent : Promise.reject(new Error("not a link")), + ) + mockedAcquireFileLock.mockImplementation(async () => { + order.push("lock") + return async () => {} + }) + + await safeWriteJson(currentLink, { id: "task-1" }) + + // The lock key is the key every other writer to this file uses, so the caller + // queued behind the peer instead of failing before the lock. + expect(mockedAcquireFileLock).toHaveBeenCalledWith(referent) + // The trailing lstat calls are safeWriteText's staging-path checks (the regular-file + // check on the temp file and the identity check that staging is not the target), and + // the final resolve re-checks the commit target immediately before the rename. All run + // after the key was resolved and the lock was taken, so they do not change the lock. + expect(order).toEqual([ + "resolve-failed", + "lstat", + "resolve", + "resolve", + "lock", + "resolve", + "resolve", + "lstat", + "lstat", + "resolve", + ]) + expect(JSON.parse(await fs.readFile(referent, "utf8"))).toEqual({ id: "task-1" }) + }) + + it("releases the lock when the resolution under the lock rejects", async () => { + const order: string[] = [] + let released = false + const dir = await makeDir("lockkey-") + const referent = path.join(dir, "history_item.json") + currentLink = path.join(dir, "link.json") + + // A real dangling link: the walk tolerates it so the caller can queue behind + // the peer, but once the lock is held the strict rejection still applies. A + // rejection outside the protected block would leave the lock held until the + // stale timeout for every other writer to the same file. + mockedRealpath.mockImplementation(async () => { + throw enoent + }) + mockedLstat.mockImplementation(async (target) => { + order.push("lstat") + return symlinkStat(target) + }) + mockedReadlink.mockImplementation(async (target) => + target === currentLink ? referent : Promise.reject(new Error("not a link")), + ) + mockedAcquireFileLock.mockImplementation(async () => { + order.push("lock") + return async () => { + order.push("release") + released = true + } + }) + + await expect(safeWriteJson(currentLink, { id: "task-1" })).rejects.toThrow(enoent) + expect(released).toBe(true) + // The strict rejection is reached through the ENOENT + symlink branch, not + // through a synchronous throw that skips it. + expect(order).toEqual(["lstat", "lock", "lstat", "release"]) + }) + + it("canonicalizes the parent directory when the file itself is not there yet", async () => { + // fs.realpath canonicalizes every component, including a symlinked ancestor + // directory or a Windows 8.3 short name. If the fallback returns the alias + // directory, the key depends on whether the file exists at the moment the key + // is computed, and a writer that resolved the canonical directory takes a + // different lock for the same file. + const aliasDir = path.join(os.tmpdir(), "alias-dir") + const canonicalDir = path.join(os.tmpdir(), "canonical-dir") + const file = path.join(aliasDir, "history_item.json") + mockedRealpath.mockImplementation(async (target) => { + if (target === file) throw enoent + return canonicalDir + }) + mockedLstat.mockImplementation( + async () => ({ isSymbolicLink: () => false, isFile: () => true }) as unknown as BigIntStats, + ) + + expect(await resolveLockKey(file)).toBe(path.join(canonicalDir, "history_item.json")) + }) + + it("propagates a non-ENOENT resolution failure instead of keying on the alias", async () => { + // EACCES from realpath is not a dangling link: falling back to the walk + // would key this writer to the alias path while a writer reaching the + // referent directly keys the referent, and the two writers would take + // different locks for the same file. + const aliasDir = path.join(os.tmpdir(), "alias-dir") + const file = path.join(aliasDir, "history_item.json") + const eacces = Object.assign(new Error("EACCES: permission denied"), { code: "EACCES" }) + mockedRealpath.mockImplementation(async () => { + throw eacces + }) + await expect(resolveLockKey(file)).rejects.toBe(eacces) + }) +}) + +it("does not log a cleanup error when the safety net finds the temp file already gone", async () => { + // safeWriteText removes its own temp file on failure, so the safety net in + // safeWriteJson normally finds it gone. That is the expected outcome, not a + // second failure, and it must not be logged as one. + const dir = await makeDir("cleanup-") + const target = path.join(dir, "history_item.json") + currentLink = "" + mockedRealpath.mockImplementation(async (t) => String(t)) + mockedLstat.mockImplementation(async (t) => symlinkStat(t)) + + const renameSpy = vi.spyOn(fs, "rename").mockRejectedValue(new Error("commit rename failed")) + const unlinkSpy = vi.spyOn(fs, "unlink").mockRejectedValue(enoent) + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}) + + try { + await expect(safeWriteJson(target, { id: "task-1" })).rejects.toThrow("commit rename failed") + + // Only the original failure is reported. + expect(consoleError).toHaveBeenCalledTimes(1) + } finally { + // Restore even when an assertion fails: the afterEach cleanup runs fs.rm + // with these spies still in place otherwise, and later tests inherit a + // rejecting rename. + renameSpy.mockRestore() + unlinkSpy.mockRestore() + consoleError.mockRestore() + } +}) diff --git a/src/utils/__tests__/safeWriteJson.test.ts b/src/utils/__tests__/safeWriteJson.test.ts index 30ac500212..fc8381206c 100644 --- a/src/utils/__tests__/safeWriteJson.test.ts +++ b/src/utils/__tests__/safeWriteJson.test.ts @@ -3,6 +3,7 @@ import { Writable } from "stream" import * as path from "path" import * as os from "os" +import { PostCommitDurabilityError } from "../../services/file-safety/safeWriteText" import { safeWriteJson } from "../safeWriteJson" // Capture actual implementations before the vi.mock factory runs, @@ -818,4 +819,190 @@ describe("safeWriteJson", () => { const content = await readFileContent(currentTestFilePath) expect(content).toEqual(newData) }) + + // The four cases below come from the branch side of the merge: they pin the + // lock-key and publish-target behaviour that motivated this unit, plus the + // mode-preservation and post-commit-durability behaviour that safeWriteText + // provides to the JSON path. + // The commit rename targets the symlink referent. The staged temp file must + // therefore be created beside the RESOLVED target — staging beside the link + // would make the commit rename fail with EXDEV when the referent is on + // another filesystem. (Real symlinks are unavailable in this CI lane, so the + // resolution is simulated by mocking fs.realpath the same way.) + test("stages the temp file beside the symlink referent and commits onto it", async () => { + const referentDir = path.join(tempDir, "referent") + const linkDir = path.join(tempDir, "link") + await fs.mkdir(referentDir, { recursive: true }) + await fs.mkdir(linkDir, { recursive: true }) + // caller-visible path (the link) vs the resolved referent path + const callerPath = path.join(linkDir, "test-file.json") + const referentPath = path.join(referentDir, "test-file.json") + // Seed the RESOLVED referent with real content (via the actual fs) so the + // write exercises replacement of an EXISTING referent: the lock, the + // backup, and the commit all target the resolved referent. + await fsPromisesActuals.writeFile!(referentPath, JSON.stringify({ seed: true })) + + // Only the file resolves through the link; the directory is already canonical, + // so the lock key is the referent rather than the alias directory + basename. + vi.spyOn(fs, "realpath").mockImplementation(async (target) => + target === callerPath ? referentPath : String(target), + ) + + await safeWriteJson(callerPath, { after: true }) + + // the temp file was created next to the resolved referent, NOT beside the link + const tempPaths = vi.mocked(fsSyncActual.createWriteStream).mock.calls.map((call) => String(call[0])) + expect(tempPaths.some((p) => p.startsWith(referentDir + path.sep) && p.includes(".new_"))).toBe(true) + expect(tempPaths.some((p) => p.startsWith(linkDir + path.sep))).toBe(false) + + // the content was committed onto the referent + expect(await readFileContent(referentPath)).toEqual({ after: true }) + }) + + // proper-lockfile with realpath:false keys the lock by the given path, so a + // symlink alias and its referent must coordinate through ONE lock on the + // resolved referent — otherwise a concurrent merge through both aliases + // reads the same JSON and overwrites one update. (Real symlinks are + // unavailable in this CI lane, so the resolution is simulated by mocking + // fs.realpath, the same way as the staging test above.) + test("acquires the lock on the resolved referent, not the caller alias", async () => { + vi.resetModules() // fresh module instances so the doMock below is picked up + + const referentDir = path.join(tempDir, "lock-referent") + const linkDir = path.join(tempDir, "lock-link") + await fs.mkdir(referentDir, { recursive: true }) + await fs.mkdir(linkDir, { recursive: true }) + // caller-visible path (the link) vs the resolved referent path + const callerPath = path.join(linkDir, "locked.json") + const referentPath = path.join(referentDir, "locked.json") + await fsPromisesActuals.writeFile!(referentPath, JSON.stringify({ seed: 1 })) + + // Only the file resolves through the link; the directory is already canonical, + // so the lock key is the referent rather than the alias directory + basename. + const realpathSpy = vi + .spyOn(fs, "realpath") + .mockImplementation(async (target) => (target === callerPath ? referentPath : String(target))) + + // Wrap the real lock in a capturing mock, and drive the two rare error paths + // (the onCompromised callback and a failing release) so they stay covered + // without real lockfile staleness. The callback rethrows by design, so + // the mock swallows that throw and lets the real lock proceed. + const realLockfile = await vi.importActual("proper-lockfile") + const lockMockFn = vi.fn( + async ( + file: Parameters[0], + options?: Parameters[1], + ) => { + try { + options?.onCompromised?.(new Error("lock compromised (test)")) + } catch { + // onCompromised rethrows by design; swallow so the real lock proceeds. + } + const release = await realLockfile.lock(file, options) + return async () => { + await release() + throw new Error("release failed (test)") + } + }, + ) + const lockMock = lockMockFn as unknown as typeof realLockfile.lock + vi.doMock("proper-lockfile", () => ({ + ...realLockfile, + lock: lockMock, + })) + + // Re-import safeWriteJson so it picks up the mocked proper-lockfile. + const { safeWriteJson: mockedSafeWriteJson } = await import("../safeWriteJson") + + const mergeFn = vi.fn((existing: unknown, incoming: unknown) => ({ + ...(existing as Record), + ...(incoming as Record), + })) + + // Capture the compromise + release-failure logs. + const consoleErrorSpy = vi.spyOn(console, "error") + try { + await mockedSafeWriteJson(callerPath, { added: true }, { merge: mergeFn }) + + // The lock was keyed by the resolved referent — every alias shares it. + expect(lockMock).toHaveBeenCalledTimes(1) + expect(String(lockMockFn.mock.calls[0][0])).toBe(referentPath) + // The merge read the referent's content through that single lock. + expect(mergeFn).toHaveBeenCalledWith({ seed: 1 }, { added: true }) + expect(await readFileContent(referentPath)).toEqual({ seed: 1, added: true }) + // The compromise callback and the failed release were logged, not thrown. + expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining("was compromised"), expect.any(Error)) + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining("Failed to release lock"), + expect.any(Error), + ) + } finally { + // Cleanup must run even when an assertion fails: a leaked mock + // registration or console spy changes later tests, and vi.unmock + // alone does not reset a module that already imported the mock. + realpathSpy.mockRestore() + vi.unmock("proper-lockfile") + vi.resetModules() + consoleErrorSpy.mockRestore() + } + }) + + test.skipIf(process.platform === "win32")( + "preserves a restrictive 0o600 target mode through the atomic publish", + async () => { + await fsPromisesActuals.writeFile!(currentTestFilePath, JSON.stringify({ before: true })) + fsSyncActual.chmodSync(currentTestFilePath, 0o600) + + await safeWriteJson(currentTestFilePath, { after: true }) + + expect(fsSyncActual.statSync(currentTestFilePath).mode & 0o777).toBe(0o600) + expect(await readFileContent(currentTestFilePath)).toEqual({ after: true }) + }, + ) + + // ── post-commit durability: the published target must survive (PI regression) ── + + it("keeps the published target with its new content when the post-commit directory fsync fails", async () => { + const fsSyncMod = await import("fs") + const realFsync = fsSyncMod.fsyncSync + // The parent-directory fsync is the only fsync that runs after the + // commit rename. Watching the rename itself - rather than fabricating a + // descriptor - keeps the SUT in charge of opening and closing its own + // real directory handle, so the injected fsync failure is the only + // error in flight on every platform. + const fsyncFailure = new Error("EIO fsync") + let committed = false + const actualRename = fsPromisesActuals.rename! + vi.mocked(fs.rename).mockImplementation((async (from: string, to: string) => { + await actualRename(from, to) + committed = true + }) as typeof fs.rename) + vi.spyOn(process, "platform", "get").mockReturnValue("linux") + vi.spyOn(fsSyncMod, "fsyncSync").mockImplementation(((fd: number) => { + if (committed) { + throw fsyncFailure + } + return (realFsync as (f: number) => unknown)(fd) + }) as never) + + // Earlier tests in this file leave unlink history on the shared mock. + vi.mocked(fs.unlink).mockClear() + + const failure: unknown = await safeWriteJson(currentTestFilePath, { fresh: "content" }).then( + () => null, + (error: unknown) => error, + ) + expect(failure).toBeInstanceOf(PostCommitDurabilityError) + // The wrapper must carry the injected fsync failure, not a cleanup error. + expect((failure as Error).cause).toBe(fsyncFailure) + + // The rename committed: the target must still hold the new JSON, not be + // unlinked away by the catch cleanup of the consumed staging path. + const content = await readFileContent(currentTestFilePath) + expect(content).toEqual({ fresh: "content" }) + // The cleanup must not unlink the consumed staging path: after the commit + // rename it is no longer this write's file to remove. + const stagingUnlinks = vi.mocked(fs.unlink).mock.calls.filter((call) => /\.new_.*\.tmp$/.test(String(call[0]))) + expect(stagingUnlinks).toHaveLength(0) + }) }) diff --git a/src/utils/jsonFileRecovery.ts b/src/utils/jsonFileRecovery.ts index 2350f30685..8b55170573 100644 --- a/src/utils/jsonFileRecovery.ts +++ b/src/utils/jsonFileRecovery.ts @@ -10,11 +10,21 @@ function getTempFilePrefix(absoluteFilePath: string, kind: "new" | "bak"): strin return `.${path.basename(absoluteFilePath)}.${kind}_` } +/** + * Staging name used by writers that publish through safeWriteText, which requires a + * caller-staged path named `.new_*.tmp` (see services/file-safety/safeWriteText). The + * target basename is embedded after the anchor so the name stays scoped to this file. + */ +function getStagedReplacementPrefix(absoluteFilePath: string): string { + return `.new_${path.basename(absoluteFilePath)}_` +} + function isReplacementLeftover(absoluteFilePath: string, entry: string): boolean { return ( entry.endsWith(".tmp") && (entry.startsWith(getTempFilePrefix(absoluteFilePath, "new")) || - entry.startsWith(getTempFilePrefix(absoluteFilePath, "bak"))) + entry.startsWith(getTempFilePrefix(absoluteFilePath, "bak")) || + entry.startsWith(getStagedReplacementPrefix(absoluteFilePath))) ) } diff --git a/src/utils/safeWriteJson.ts b/src/utils/safeWriteJson.ts index e54b8c51d3..f9e546a7fd 100644 --- a/src/utils/safeWriteJson.ts +++ b/src/utils/safeWriteJson.ts @@ -6,6 +6,12 @@ import { JsonStreamStringify } from "json-stream-stringify" import { getErrorCode } from "./errorCode" import { acquireFileLock } from "./fileLock" import { recoverInterruptedReplacement } from "./jsonFileRecovery" +import { + PostCommitDurabilityError, + resolveLockKey, + resolvePublishTarget, + safeWriteText, +} from "../services/file-safety/safeWriteText" /** * Stages reported through the optional `trace` callback. They mark the @@ -48,10 +54,28 @@ export interface SafeWriteJsonOptions { trace?: SafeWriteJsonTrace } -function createTempFilePath(absoluteFilePath: string, kind: "new" | "bak"): string { +/** + * Staging path for the replacement, created beside the target it will be renamed onto. + * + * The name has to satisfy two contracts at once. safeWriteText only publishes a + * caller-supplied staging path whose basename matches `.new_*.tmp` (a plain file + * beside the target would have its content published and its own name deleted), + * and jsonFileRecovery only garbage-collects leftovers whose name is scoped to + * this file, so it can never unlink another target's in-flight staging file in + * the same directory. `.new___.tmp` is the one shape that + * satisfies both. + */ +function createStagingFilePath(absoluteFilePath: string): string { + return path.join( + path.dirname(absoluteFilePath), + `.new_${path.basename(absoluteFilePath)}_${Date.now()}_${Math.random().toString(36).substring(2)}.tmp`, + ) +} + +function createBackupFilePath(absoluteFilePath: string): string { return path.join( path.dirname(absoluteFilePath), - `.${path.basename(absoluteFilePath)}.${kind}_${Date.now()}_${Math.random().toString(36).substring(2)}.tmp`, + `.${path.basename(absoluteFilePath)}.bak_${Date.now()}_${Math.random().toString(36).substring(2)}.tmp`, ) } @@ -59,9 +83,11 @@ function createTempFilePath(absoluteFilePath: string, kind: "new" | "bak"): stri * Safely writes JSON data to a file. * - Creates parent directories if they don't exist * - Uses 'proper-lockfile' for inter-process advisory locking to prevent concurrent writes to the same path. + * The lock is keyed on the symlink referent, so an alias and its referent share one lock. * - Recovers an interrupted previous write before touching the file: restores an orphaned backup when the canonical path is absent, and removes leftover temp files. - * - Writes to a temporary file first, then commits it with a single atomic rename, so the committed target stays readable through an interrupted write. - * - When a single rename over the target fails with EPERM (a reader holds the file open on Windows), falls back to backing the target up before replacing it. + * - Writes to a temporary file first via JsonStreamStringify streaming, then commits it through safeWriteText's single atomic rename, so the committed target stays readable through an interrupted write. + * - When that rename fails with EPERM (a reader holds the file open on Windows), re-stages and falls back to the two-step replacement that backs the target up before replacing it. + * - Delegates the commit, the Windows DACL capture, the target mode and the post-commit fsync to safeWriteText, which is the same primitive the guarded text writes use. * - Attempts to roll back and clean up in case of errors. * - Supports pretty-printing with indentation while maintaining streaming efficiency. * @@ -70,7 +96,6 @@ function createTempFilePath(absoluteFilePath: string, kind: "new" | "bak"): stri * @param {SafeWriteJsonOptions} options - Optional configuration for JSON formatting. * @returns {Promise} */ - async function safeWriteJson(filePath: string, data: unknown, options?: SafeWriteJsonOptions): Promise { const absoluteFilePath = path.resolve(filePath) let releaseLock = async () => {} // Initialized to a no-op @@ -79,35 +104,50 @@ async function safeWriteJson(filePath: string, data: unknown, options?: SafeWrit const dirPath = path.dirname(absoluteFilePath) // Ensure directory structure exists with improved reliability + // Declared outside the protected block so the catch and finally can still name + // the target when the resolution itself rejects. + let resolvedTargetPath: string | undefined + try { - // Create directory with recursive option await fs.mkdir(dirPath, { recursive: true }) - - // Verify directory exists after creation attempt await fs.access(dirPath) } catch (dirError: unknown) { console.error(`Failed to create or access directory for ${absoluteFilePath}:`, dirError) throw dirError } - // Acquire the lock before any file operations. `acquireFileLock` owns the - // shared advisory lock protocol, so callers that lock the same path with - // it (for example task-history deletion) serialize with this write. - // If lock acquisition fails, it throws immediately. The releaseLock - // 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) + // Lock key: the symlink referent when the path is an existing symlink, so a + // symlink alias and its referent share one lock. The key must be computable + // while a peer writer is mid-commit (a publish renames the staged file onto the + // referent and back), so the walk tolerates a dangling link instead of rejecting + // it here. + const lockKey = await resolveLockKey(absoluteFilePath) + + // Acquire the lock before any file operations. If acquisition fails it throws + // immediately, and releaseLock stays a no-op so the finally block does not try + // to release an unacquired lock. Callers that lock the same key with acquireFileLock + // (for example task-history deletion) serialize with this write. + releaseLock = await acquireFileLock(lockKey) // Variables to hold the actual paths of temp files if they are created. let actualTempNewFilePath: string | null = null + // The backup copy taken by the EPERM fallback, if that fallback runs. let actualTempBackupFilePath: string | null = null try { + // Resolve the publish target under the lock: a peer that held the lock has + // committed by now, so the strict dangling-link rejection still applies to a + // real dangling link. It must stay inside the protected block, otherwise a + // rejection here leaves the advisory lock held until the stale timeout for + // every other writer. + resolvedTargetPath = await resolvePublishTarget(absoluteFilePath) + // Heal any state an interrupted previous write left behind: an absent // canonical path with an orphaned backup, or leftover replacement - // files. Runs first so a merge callback reads the restored content. - const recovery = await recoverInterruptedReplacement(absoluteFilePath).catch((error: unknown) => { + // files. Runs on the resolved target, because that is the directory the + // staging and backup names live in, and runs before the merge callback so + // the callback reads the restored content. + const recovery = await recoverInterruptedReplacement(resolvedTargetPath).catch((error: unknown) => { const code = getErrorCode(error) ?? "unknown" options?.trace?.({ stage: "recover", detail: `failed:${code}` }) throw error @@ -122,7 +162,7 @@ async function safeWriteJson(filePath: string, data: unknown, options?: SafeWrit if (options?.merge) { let existing: unknown = null try { - existing = JSON.parse(await fs.readFile(absoluteFilePath, "utf8")) + existing = JSON.parse(await fs.readFile(resolvedTargetPath, "utf8")) } catch (error: unknown) { const code = getErrorCode(error) if (!(error instanceof SyntaxError) && code !== "ENOENT") { @@ -134,103 +174,131 @@ async function safeWriteJson(filePath: string, data: unknown, options?: SafeWrit // Step 1: Write data to a new temporary file and flush it to storage, // making a best-effort flush before committing the replacement. + // Staged beside the RESOLVED target (the symlink referent when the path is a + // symlink): the commit is a rename onto that referent, and a rename across + // filesystems would fail with EXDEV. The name is the staging convention the + // commit primitive requires and is scoped to this file, so the recovery pass + // above can still recognise it. See createStagingFilePath. options?.trace?.({ stage: "write-start" }) - actualTempNewFilePath = createTempFilePath(absoluteFilePath, "new") + actualTempNewFilePath = createStagingFilePath(resolvedTargetPath) await _streamDataToFile(actualTempNewFilePath, data, options?.prettyPrint) await _flushFileToDisk(actualTempNewFilePath) - // Step 2: Commit the replacement with a single atomic rename. The - // committed target stays in place until this rename lands, so a - // process exit before or during the commit can no longer leave the - // canonical path absent. + // Step 2: Commit through safeWriteText with the pre-written temp path (backup:false + // is its single atomic rename), so the committed target stays in place until the + // rename lands and a process exit before or during the commit cannot leave the + // canonical path absent. safeWriteText carries the Windows DACL across the + // replacement, preserves the target's mode, fsyncs the staged file and, off + // Windows, fsyncs the parent directory - which is why this file no longer renames + // onto the path itself. try { options?.trace?.({ stage: "replacement-rename" }) - await fs.rename(actualTempNewFilePath, absoluteFilePath) - } catch (renameError: unknown) { - if (getErrorCode(renameError) !== "EPERM") { - throw renameError + await safeWriteText(resolvedTargetPath, "", { tempPath: actualTempNewFilePath, backup: false }) + } catch (commitError: unknown) { + if (getErrorCode(commitError) !== "EPERM") { + throw commitError } - - // A single rename over a file that a reader holds open fails with - // EPERM on Windows, so fall back to the two-step replacement: - // rename the target to a backup, then the replacement into place. - // The rollback in the catch below restores the backup when the - // second rename fails, and the recovery pass at the start of the - // next write repairs a crash between the two renames. + // A rename over a file that a reader holds open fails with EPERM on + // Windows, so fall back to the two-step replacement: move the committed file + // aside, then rename the replacement into place. safeWriteText removed the + // staging path when its commit failed, so the data is re-staged under a fresh + // name. The recovery pass at the start of the next write repairs a crash + // between these two renames, so this sequence needs no pre-backup fsync here. options?.trace?.({ stage: "backup-rename", detail: "eperm-fallback" }) - actualTempBackupFilePath = createTempFilePath(absoluteFilePath, "bak") - await fs.rename(absoluteFilePath, actualTempBackupFilePath) + // The backup keeps this file's `.basename.bak_*` convention: that is the name + // the recovery pass restores from when a crash lands between the two renames. + actualTempBackupFilePath = createBackupFilePath(resolvedTargetPath) + await fs.rename(resolvedTargetPath, actualTempBackupFilePath) + actualTempNewFilePath = createStagingFilePath(resolvedTargetPath) + await _streamDataToFile(actualTempNewFilePath, data, options?.prettyPrint) + await _flushFileToDisk(actualTempNewFilePath) options?.trace?.({ stage: "replacement-rename", detail: "eperm-fallback" }) - await fs.rename(actualTempNewFilePath, absoluteFilePath) + await safeWriteText(resolvedTargetPath, "", { tempPath: actualTempNewFilePath, backup: false }) } - - // If we reach here, the new file is successfully in place. - // The original actualTempNewFilePath is now the main file, so we shouldn't try to clean it up as "temp". - // Mark as "used" or "committed" - actualTempNewFilePath = null - options?.trace?.({ stage: "write-complete" }) - - // Step 3: If a backup was created, attempt to delete it. + // Step 3: Delete the backup file taken by the EPERM fallback (the single-rename + // path never takes one). Failure here must not undo or fail the write, because the + // new content is already committed; it is logged and the write still resolves. if (actualTempBackupFilePath) { try { await fs.unlink(actualTempBackupFilePath) - // Mark backup as handled - actualTempBackupFilePath = null - } catch (unlinkBackupError) { - // Log this error, but do not re-throw. The main operation was successful. - // actualTempBackupFilePath remains set, indicating an orphaned backup. + } catch (cleanupError) { console.error( - `Successfully wrote ${absoluteFilePath}, but failed to clean up backup ${actualTempBackupFilePath}:`, - unlinkBackupError, + `Successfully wrote ${resolvedTargetPath} but failed to delete backup ${actualTempBackupFilePath}:`, + cleanupError, ) } + actualTempBackupFilePath = null } + + actualTempNewFilePath = null + options?.trace?.({ stage: "write-complete" }) } catch (originalError) { - console.error(`Operation failed for ${absoluteFilePath}: [Original Error Caught]`, originalError) + console.error( + `Operation failed for ${resolvedTargetPath ?? absoluteFilePath}: [Original Error Caught]`, + originalError, + ) - const newFileToCleanupWithinCatch = actualTempNewFilePath - const backupFileToRollbackOrCleanupWithinCatch = actualTempBackupFilePath + // PostCommitDurabilityError means the commit rename landed: the staged + // path was renamed onto the target, so it is no longer a leftover temp + // file. Unlinking it here would at best race a concurrent writer that has + // already reused the staging name, and the published target must survive + // the durability failure with its new content. The error rethrows below, + // so the write is never observed as a confirmed-durable success. + if (originalError instanceof PostCommitDurabilityError) { + actualTempNewFilePath = null + } - // Attempt rollback if a backup was made - if (backupFileToRollbackOrCleanupWithinCatch) { + // If the EPERM fallback had moved the committed file to the backup path, restore it + // so the target keeps its previous content. A failed restore leaves the backup as + // the only committed copy - it is preserved for recovery by a later writer rather + // than unlinked, and the error from the failed replacement is the one rethrown. + if (actualTempBackupFilePath) { try { - await fs.rename(backupFileToRollbackOrCleanupWithinCatch, absoluteFilePath) - // Mark as handled, prevent later unlink of this path - actualTempBackupFilePath = null + await fs.rename(actualTempBackupFilePath, resolvedTargetPath ?? absoluteFilePath) } catch (rollbackError) { - // actualTempBackupFilePath (outer scope) remains pointing to backupFileToRollbackOrCleanupWithinCatch console.error( - `[Catch] Failed to restore backup ${backupFileToRollbackOrCleanupWithinCatch} to ${absoluteFilePath}:`, + `Failed to restore backup ${actualTempBackupFilePath} to ${resolvedTargetPath ?? absoluteFilePath}:`, rollbackError, ) } + actualTempBackupFilePath = null } + const newFileToCleanupWithinCatch = actualTempNewFilePath - // Cleanup the .new file if it exists + // Clean up the .new file if it still exists (safeWriteText removes its own + // staging path on a failed commit, so this is a safety net for the paths that + // never reach it, such as a failure inside the EPERM fallback's re-stage). + // The fallback's backup is handled above: it is either restored to the target + // or preserved for a later recovery pass, never dropped here. if (newFileToCleanupWithinCatch) { try { await fs.unlink(newFileToCleanupWithinCatch) - } catch (cleanupError) { - console.error( - `[Catch] Failed to clean up temporary new file ${newFileToCleanupWithinCatch}:`, - cleanupError, - ) + } catch (cleanupError: unknown) { + // The expected case: safeWriteText already removed its own temp file, so a + // missing file here is not a cleanup failure worth logging. Returning would + // also swallow the original error the caller needs. + const isAbsent = + typeof cleanupError === "object" && + cleanupError !== null && + "code" in cleanupError && + cleanupError.code === "ENOENT" + if (!isAbsent) { + console.error( + `[Catch] Failed to clean up temporary new file ${newFileToCleanupWithinCatch}:`, + cleanupError, + ) + } } } - // A failed rollback leaves the backup as the only committed copy. - // Preserve it for recovery by a later writer. throw originalError // This MUST be the error that rejects the promise. } finally { // Release the lock in the main finally block. try { - // releaseLock will be the actual unlock function if lock was acquired, - // or the initial no-op if acquisition failed. await releaseLock() } catch (unlockError) { - // Do not re-throw here, as the originalError from the try/catch (if any) is more important. - console.error(`Failed to release lock for ${absoluteFilePath}:`, unlockError) + console.error(`Failed to release lock for ${resolvedTargetPath ?? absoluteFilePath}:`, unlockError) } } }