From 3227ba765136936059d9d541092e136789d303b5 Mon Sep 17 00:00:00 2001 From: lazerg Date: Thu, 30 Jul 2026 05:14:20 +0500 Subject: [PATCH 1/4] fix(fs): write files atomically to avoid truncated concurrent reads --- src/drivers/utils/node-fs.ts | 10 +++++++++- test/drivers/fs-lite.test.ts | 17 +++++++++++++++++ test/drivers/fs.test.ts | 17 +++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/drivers/utils/node-fs.ts b/src/drivers/utils/node-fs.ts index 7f031af0e..1fdedcc2c 100644 --- a/src/drivers/utils/node-fs.ts +++ b/src/drivers/utils/node-fs.ts @@ -1,5 +1,6 @@ import { Dirent, existsSync, promises as fsPromises } from "node:fs"; import { resolve, dirname } from "node:path"; +import { randomUUID } from "node:crypto"; function ignoreNotfound(err: any) { return err.code === "ENOENT" || err.code === "EISDIR" ? null : err; @@ -16,7 +17,14 @@ export async function writeFile( encoding?: BufferEncoding, ): Promise { await ensuredir(dirname(path)); - return fsPromises.writeFile(path, data, encoding); + const tmp = `${path}.${process.pid}.${randomUUID()}.tmp`; + try { + await fsPromises.writeFile(tmp, data, encoding); + await fsPromises.rename(tmp, path); + } catch (error) { + await fsPromises.unlink(tmp).catch(() => {}); + throw error; + } } export function readFile(path: string, encoding?: BufferEncoding): Promise { diff --git a/test/drivers/fs-lite.test.ts b/test/drivers/fs-lite.test.ts index e5e5b3b72..41100a60e 100644 --- a/test/drivers/fs-lite.test.ts +++ b/test/drivers/fs-lite.test.ts @@ -14,6 +14,23 @@ describe("drivers: fs-lite", () => { await ctx.storage.setItem("s1:a", "test_data"); expect(await readFile(resolve(dir, "s1/a"), "utf8")).toBe("test_data"); }); + it("reads concurrent with a write never observe a truncated value", async () => { + const size = 256 * 1024; + const a = new Uint8Array(size).fill(0xaa); + const b = new Uint8Array(size).fill(0xbb); + await ctx.storage.setItemRaw("atomic:key", a); + for (let i = 0; i < 20; i++) { + const [, ...reads] = await Promise.all([ + ctx.storage.setItemRaw("atomic:key", i % 2 === 0 ? b : a), + ctx.storage.getItemRaw("atomic:key"), + ctx.storage.getItemRaw("atomic:key"), + ctx.storage.getItemRaw("atomic:key"), + ]); + for (const read of reads) { + expect((read as Uint8Array).length).toBe(size); + } + } + }); it("native meta", async () => { await ctx.storage.setItem("s1:a", "test_data"); const meta = await ctx.storage.getMeta("/s1/a"); diff --git a/test/drivers/fs.test.ts b/test/drivers/fs.test.ts index 366f5f0e0..b9e9189d7 100644 --- a/test/drivers/fs.test.ts +++ b/test/drivers/fs.test.ts @@ -15,6 +15,23 @@ describe("drivers: fs", () => { await ctx.storage.setItem("s1:a", "test_data"); expect(await readFile(resolve(dir, "s1/a"), "utf8")).toBe("test_data"); }); + it("reads concurrent with a write never observe a truncated value", async () => { + const size = 256 * 1024; + const a = new Uint8Array(size).fill(0xaa); + const b = new Uint8Array(size).fill(0xbb); + await ctx.storage.setItemRaw("atomic:key", a); + for (let i = 0; i < 20; i++) { + const [, ...reads] = await Promise.all([ + ctx.storage.setItemRaw("atomic:key", i % 2 === 0 ? b : a), + ctx.storage.getItemRaw("atomic:key"), + ctx.storage.getItemRaw("atomic:key"), + ctx.storage.getItemRaw("atomic:key"), + ]); + for (const read of reads) { + expect((read as Uint8Array).length).toBe(size); + } + } + }); it("native meta", async () => { await ctx.storage.setItem("s1:a", "test_data"); const meta = await ctx.storage.getMeta("/s1/a"); From 4eb91cca8a875c31b4222c4257a3d1acaf01e3b3 Mon Sep 17 00:00:00 2001 From: lazerg Date: Thu, 30 Jul 2026 06:19:02 +0500 Subject: [PATCH 2/4] fix(fs): exclude in-progress temp files from key listing --- src/drivers/utils/node-fs.ts | 4 +++- test/drivers/fs-lite.test.ts | 17 ++++++++++++++++- test/drivers/fs.test.ts | 17 ++++++++++++++++- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/drivers/utils/node-fs.ts b/src/drivers/utils/node-fs.ts index 1fdedcc2c..abb9754d9 100644 --- a/src/drivers/utils/node-fs.ts +++ b/src/drivers/utils/node-fs.ts @@ -10,6 +10,8 @@ function ignoreExists(err: any) { return err.code === "EEXIST" ? null : err; } +const TMP_FILE_RE = /\.\d+\.[\da-f]{8}(?:-[\da-f]{4}){3}-[\da-f]{12}\.tmp$/; + type WriteFileData = Parameters[1]; export async function writeFile( path: string, @@ -77,7 +79,7 @@ export async function readdirRecursive( files.push(...dirFiles.map((f) => entry.name + "/" + f)); } } else { - if (!(ignore && ignore(entryPath))) { + if (!(ignore && ignore(entryPath)) && !TMP_FILE_RE.test(entry.name)) { files.push(entry.name); } } diff --git a/test/drivers/fs-lite.test.ts b/test/drivers/fs-lite.test.ts index 41100a60e..46ae82e72 100644 --- a/test/drivers/fs-lite.test.ts +++ b/test/drivers/fs-lite.test.ts @@ -27,10 +27,25 @@ describe("drivers: fs-lite", () => { ctx.storage.getItemRaw("atomic:key"), ]); for (const read of reads) { - expect((read as Uint8Array).length).toBe(size); + const bytes = read as Uint8Array; + expect(bytes.length).toBe(size); + const first = bytes[0]; + expect(first === 0xaa || first === 0xbb).toBe(true); + expect(bytes.every((byte) => byte === first)).toBe(true); } } }); + it("getKeys never observes in-progress temp files", async () => { + const size = 256 * 1024; + const value = new Uint8Array(size).fill(0xaa); + for (let i = 0; i < 20; i++) { + const [, keys] = await Promise.all([ + ctx.storage.setItemRaw("tmp:key", value), + ctx.driver.getKeys("", {}), + ]); + expect(keys.every((key) => !key.includes(".tmp"))).toBe(true); + } + }); it("native meta", async () => { await ctx.storage.setItem("s1:a", "test_data"); const meta = await ctx.storage.getMeta("/s1/a"); diff --git a/test/drivers/fs.test.ts b/test/drivers/fs.test.ts index b9e9189d7..4b8230fdf 100644 --- a/test/drivers/fs.test.ts +++ b/test/drivers/fs.test.ts @@ -28,10 +28,25 @@ describe("drivers: fs", () => { ctx.storage.getItemRaw("atomic:key"), ]); for (const read of reads) { - expect((read as Uint8Array).length).toBe(size); + const bytes = read as Uint8Array; + expect(bytes.length).toBe(size); + const first = bytes[0]; + expect(first === 0xaa || first === 0xbb).toBe(true); + expect(bytes.every((byte) => byte === first)).toBe(true); } } }); + it("getKeys never observes in-progress temp files", async () => { + const size = 256 * 1024; + const value = new Uint8Array(size).fill(0xaa); + for (let i = 0; i < 20; i++) { + const [, keys] = await Promise.all([ + ctx.storage.setItemRaw("tmp:key", value), + ctx.driver.getKeys("", {}), + ]); + expect(keys.every((key) => !key.includes(".tmp"))).toBe(true); + } + }); it("native meta", async () => { await ctx.storage.setItem("s1:a", "test_data"); const meta = await ctx.storage.getMeta("/s1/a"); From 6d85e18f73dd08a3ba81dbe1baffc54341323a45 Mon Sep 17 00:00:00 2001 From: lazerg Date: Thu, 30 Jul 2026 06:29:08 +0500 Subject: [PATCH 3/4] fix(fs): preserve destination file permissions on atomic overwrite --- src/drivers/utils/node-fs.ts | 7 +++++++ test/drivers/fs-lite.test.ts | 12 ++++++++++++ test/drivers/fs.test.ts | 12 ++++++++++++ 3 files changed, 31 insertions(+) diff --git a/src/drivers/utils/node-fs.ts b/src/drivers/utils/node-fs.ts index abb9754d9..13a6aa3d7 100644 --- a/src/drivers/utils/node-fs.ts +++ b/src/drivers/utils/node-fs.ts @@ -22,6 +22,13 @@ export async function writeFile( const tmp = `${path}.${process.pid}.${randomUUID()}.tmp`; try { await fsPromises.writeFile(tmp, data, encoding); + const destMode = await fsPromises + .stat(path) + .then((s) => s.mode) + .catch(() => undefined); + if (destMode !== undefined) { + await fsPromises.chmod(tmp, destMode); + } await fsPromises.rename(tmp, path); } catch (error) { await fsPromises.unlink(tmp).catch(() => {}); diff --git a/test/drivers/fs-lite.test.ts b/test/drivers/fs-lite.test.ts index 46ae82e72..6e2db873c 100644 --- a/test/drivers/fs-lite.test.ts +++ b/test/drivers/fs-lite.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from "vitest"; import { resolve } from "node:path"; +import { chmod, stat } from "node:fs/promises"; import { readFile } from "../../src/drivers/utils/node-fs.ts"; import { testDriver } from "./utils.ts"; import driver from "../../src/drivers/fs-lite.ts"; @@ -46,6 +47,17 @@ describe("drivers: fs-lite", () => { expect(keys.every((key) => !key.includes(".tmp"))).toBe(true); } }); + it.skipIf(process.platform === "win32")( + "preserves file permissions when overwriting", + async () => { + await ctx.storage.setItem("perm:key", "original"); + const filePath = resolve(dir, "perm/key"); + await chmod(filePath, 0o600); + await ctx.storage.setItem("perm:key", "overwritten"); + const mode = (await stat(filePath)).mode & 0o777; + expect(mode).toBe(0o600); + }, + ); it("native meta", async () => { await ctx.storage.setItem("s1:a", "test_data"); const meta = await ctx.storage.getMeta("/s1/a"); diff --git a/test/drivers/fs.test.ts b/test/drivers/fs.test.ts index 4b8230fdf..2bcba7fef 100644 --- a/test/drivers/fs.test.ts +++ b/test/drivers/fs.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, afterEach } from "vitest"; import { resolve } from "node:path"; +import { chmod, stat } from "node:fs/promises"; import { readFile, writeFile } from "../../src/drivers/utils/node-fs.ts"; import { testDriver, type TestContext } from "./utils.ts"; import driver from "../../src/drivers/fs.ts"; @@ -47,6 +48,17 @@ describe("drivers: fs", () => { expect(keys.every((key) => !key.includes(".tmp"))).toBe(true); } }); + it.skipIf(process.platform === "win32")( + "preserves file permissions when overwriting", + async () => { + await ctx.storage.setItem("perm:key", "original"); + const filePath = resolve(dir, "perm/key"); + await chmod(filePath, 0o600); + await ctx.storage.setItem("perm:key", "overwritten"); + const mode = (await stat(filePath)).mode & 0o777; + expect(mode).toBe(0o600); + }, + ); it("native meta", async () => { await ctx.storage.setItem("s1:a", "test_data"); const meta = await ctx.storage.getMeta("/s1/a"); From 504014a541a75a93d95251d6d7e0bdb8aacdda00 Mon Sep 17 00:00:00 2001 From: lazerg Date: Thu, 30 Jul 2026 06:36:12 +0500 Subject: [PATCH 4/4] fix(fs): only ignore ENOENT when reading destination mode --- src/drivers/utils/node-fs.ts | 7 ++++++- test/drivers/fs.test.ts | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/drivers/utils/node-fs.ts b/src/drivers/utils/node-fs.ts index 13a6aa3d7..6d12b8f1f 100644 --- a/src/drivers/utils/node-fs.ts +++ b/src/drivers/utils/node-fs.ts @@ -25,7 +25,12 @@ export async function writeFile( const destMode = await fsPromises .stat(path) .then((s) => s.mode) - .catch(() => undefined); + .catch((error) => { + if (error.code === "ENOENT") { + return undefined; + } + throw error; + }); if (destMode !== undefined) { await fsPromises.chmod(tmp, destMode); } diff --git a/test/drivers/fs.test.ts b/test/drivers/fs.test.ts index 2bcba7fef..af7425142 100644 --- a/test/drivers/fs.test.ts +++ b/test/drivers/fs.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, afterEach } from "vitest"; import { resolve } from "node:path"; +import { promises as fsPromises } from "node:fs"; import { chmod, stat } from "node:fs/promises"; import { readFile, writeFile } from "../../src/drivers/utils/node-fs.ts"; import { testDriver, type TestContext } from "./utils.ts"; @@ -59,6 +60,26 @@ describe("drivers: fs", () => { expect(mode).toBe(0o600); }, ); + it.skipIf(process.platform === "win32")( + "rethrows non-ENOENT stat errors when overwriting", + async () => { + const filePath = resolve(dir, "stat-error/key"); + await writeFile(filePath, "original", "utf8"); + const statSpy = vi + .spyOn(fsPromises, "stat") + .mockRejectedValueOnce( + Object.assign(new Error("permission denied"), { code: "EACCES" }), + ); + try { + await expect(writeFile(filePath, "overwritten", "utf8")).rejects.toThrow( + "permission denied", + ); + } finally { + statSpy.mockRestore(); + } + expect(await readFile(filePath, "utf8")).toBe("original"); + }, + ); it("native meta", async () => { await ctx.storage.setItem("s1:a", "test_data"); const meta = await ctx.storage.getMeta("/s1/a");