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