diff --git a/test/archive-portable-collisions.test.ts b/test/archive-portable-collisions.test.ts index 338af96..71099de 100644 --- a/test/archive-portable-collisions.test.ts +++ b/test/archive-portable-collisions.test.ts @@ -1,6 +1,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { tarFixture } from "./helpers/archive-fuzz.js"; import { extractArchive } from "../src/archive.js"; import { __resetFsSafeNativeConfigForTest, @@ -22,32 +23,6 @@ try { // JS-only jobs intentionally exercise the fallback without a built binding. } -function writeOctal(block: Buffer, offset: number, length: number, value: number): void { - block.write(`${value.toString(8).padStart(length - 1, "0")}\0`, offset, length, "ascii"); -} - -function tarFixture(paths: string[]): Buffer { - const blocks: Buffer[] = []; - for (const [index, entryPath] of paths.entries()) { - const body = Buffer.from(index === 0 ? "first" : "second"); - const header = Buffer.alloc(512); - header.write(entryPath, 0, 100, "utf8"); - writeOctal(header, 100, 8, 0o644); - writeOctal(header, 108, 8, 0); - writeOctal(header, 116, 8, 0); - writeOctal(header, 124, 12, body.length); - writeOctal(header, 136, 12, 0); - header.fill(0x20, 148, 156); - header[156] = 0x30; - header.write("ustar\0", 257, 6, "ascii"); - header.write("00", 263, 2, "ascii"); - const checksum = header.reduce((sum, byte) => sum + byte, 0); - header.write(`${checksum.toString(8).padStart(6, "0")}\0 `, 148, 8, "ascii"); - blocks.push(header, body, Buffer.alloc((512 - (body.length % 512)) % 512)); - } - return Buffer.concat([...blocks, Buffer.alloc(1024)]); -} - function useBackend(backend: "native" | "javascript"): void { if (backend === "native") { __setNativeLoaderForTest(() => native!); @@ -73,7 +48,10 @@ describe.each(backends)("%s portable archive collisions", (backend) => { const root = await tempRoot("fs-safe-archive-portable-tar-"); const archivePath = path.join(root, "payload.tar"); const destDir = path.join(root, "dest"); - await fs.writeFile(archivePath, tarFixture([firstName, secondName])); + await fs.writeFile(archivePath, tarFixture([ + { path: firstName, body: "first" }, + { path: secondName, body: "second" }, + ])); await fs.mkdir(destDir); await expect( diff --git a/test/archive-read-boundaries.test.ts b/test/archive-read-boundaries.test.ts index fd9dfc2..124b146 100644 --- a/test/archive-read-boundaries.test.ts +++ b/test/archive-read-boundaries.test.ts @@ -3,6 +3,7 @@ import path from "node:path"; import { gzipSync } from "node:zlib"; import JSZip from "jszip"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { tarFixture } from "./helpers/archive-fuzz.js"; import { useTempDirs } from "./helpers/vitest.js"; import { ARCHIVE_LIMIT_ERROR_CODE, @@ -23,45 +24,6 @@ import { const { tempRoot } = useTempDirs(); -type TarEntry = { - path: string; - body?: Buffer | string; - type?: string; - mutateHeader?: (header: Buffer) => void; -}; - -function writeString(block: Buffer, offset: number, length: number, value: string): void { - block.write(value, offset, Math.min(length, Buffer.byteLength(value)), "utf8"); -} - -function writeOctal(block: Buffer, offset: number, length: number, value: number): void { - writeString(block, offset, length, `${value.toString(8).padStart(length - 1, "0")}\0`); -} - -function tarFixture(entries: TarEntry[], endBlocks = true): Buffer { - const blocks: Buffer[] = []; - for (const entry of entries) { - const body = Buffer.isBuffer(entry.body) ? entry.body : Buffer.from(entry.body ?? ""); - const header = Buffer.alloc(512); - writeString(header, 0, 100, entry.path); - writeOctal(header, 100, 8, 0o644); - writeOctal(header, 108, 8, 0); - writeOctal(header, 116, 8, 0); - writeOctal(header, 124, 12, body.length); - writeOctal(header, 136, 12, 0); - header.fill(0x20, 148, 156); - writeString(header, 156, 1, entry.type ?? "0"); - writeString(header, 257, 6, "ustar\0"); - writeString(header, 263, 2, "00"); - entry.mutateHeader?.(header); - const checksum = header.reduce((sum, byte) => sum + byte, 0); - writeString(header, 148, 8, `${checksum.toString(8).padStart(6, "0")}\0 `); - blocks.push(header, body, Buffer.alloc((512 - (body.length % 512)) % 512)); - } - if (endBlocks) blocks.push(Buffer.alloc(1024)); - return Buffer.concat(blocks); -} - function rawHeader(type = "0", size = 0): Buffer { return tarFixture([{ path: "entry", type, body: Buffer.alloc(size) }], false).subarray(0, 512); } diff --git a/test/file-lock-sync-exit.test.ts b/test/file-lock-sync-exit.test.ts index ae49ab3..da51b5c 100644 --- a/test/file-lock-sync-exit.test.ts +++ b/test/file-lock-sync-exit.test.ts @@ -7,41 +7,33 @@ import { acquireFileLockSync } from "../src/file-lock.js"; const { tempRoot } = useTempDirs(); describe("synchronous file-lock process-exit cleanup", () => { - it("registers identity-checked cleanup for held locks", async () => { - const base = await tempRoot("fs-safe-sync-lock-exit-"); - const targetPath = path.join(base, "state.json"); - const lock = acquireFileLockSync(targetPath, { - staleMs: 60_000, - payload: () => ({ pid: process.pid, createdAt: new Date().toISOString() }), - }); - expect(fs.existsSync(lock.lockPath)).toBe(true); + it.each([false, true])( + "registers identity-checked cleanup (replacement=%s)", + async (replaceLock) => { + const base = await tempRoot("fs-safe-sync-lock-exit-"); + const targetPath = path.join(base, "state.json"); + const lock = acquireFileLockSync(targetPath, { + staleMs: 60_000, + payload: () => ({ pid: process.pid, createdAt: new Date().toISOString() }), + }); + expect(fs.existsSync(lock.lockPath)).toBe(true); + if (replaceLock) { + fs.renameSync(lock.lockPath, `${lock.lockPath}.displaced`); + fs.writeFileSync(lock.lockPath, "replacement"); + } - const cleanup = Reflect.get( - globalThis, - Symbol.for("fsSafe.syncSidecarLockCleanupHandler"), - ) as (() => void) | undefined; - expect(cleanup).toBeTypeOf("function"); - cleanup?.(); + const cleanup = Reflect.get( + globalThis, + Symbol.for("fsSafe.syncSidecarLockCleanupHandler"), + ) as () => void; + expect(cleanup).toBeTypeOf("function"); + cleanup(); - expect(fs.existsSync(lock.lockPath)).toBe(false); - }); - - it("preserves a replacement during process-exit cleanup", async () => { - const base = await tempRoot("fs-safe-sync-lock-exit-swap-"); - const targetPath = path.join(base, "state.json"); - const lock = acquireFileLockSync(targetPath, { - staleMs: 60_000, - payload: () => ({ pid: process.pid, createdAt: new Date().toISOString() }), - }); - fs.renameSync(lock.lockPath, `${lock.lockPath}.displaced`); - fs.writeFileSync(lock.lockPath, "replacement"); - - const cleanup = Reflect.get( - globalThis, - Symbol.for("fsSafe.syncSidecarLockCleanupHandler"), - ) as () => void; - cleanup(); - - expect(fs.readFileSync(lock.lockPath, "utf8")).toBe("replacement"); - }); + if (replaceLock) { + expect(fs.readFileSync(lock.lockPath, "utf8")).toBe("replacement"); + } else { + expect(fs.existsSync(lock.lockPath)).toBe(false); + } + }, + ); }); diff --git a/test/helpers/archive-fuzz.ts b/test/helpers/archive-fuzz.ts index 4ba0117..46eb84e 100644 --- a/test/helpers/archive-fuzz.ts +++ b/test/helpers/archive-fuzz.ts @@ -9,6 +9,16 @@ export type TarSizeEncoding = | "base256-negative" | "invalid-octal"; +export type TarFixtureEntry = { + path: string; + body?: Buffer | string; + mode?: number; + type?: string; + linkPath?: string; + base256Size?: number; + mutateHeader?: (header: Buffer) => void; +}; + function writeString(block: Buffer, offset: number, length: number, value: string): void { block.write(value, offset, Math.min(length, Buffer.byteLength(value)), "utf8"); } @@ -23,6 +33,42 @@ function updateTarChecksum(header: Buffer): void { writeString(header, 148, 8, `${checksum.toString(8).padStart(6, "0")}\0 `); } +export function tarFixture( + entries: readonly TarFixtureEntry[], + endBlocks = true, +): Buffer { + const blocks: Buffer[] = []; + for (const entry of entries) { + const body = Buffer.isBuffer(entry.body) ? entry.body : Buffer.from(entry.body ?? ""); + const type = entry.type ?? "0"; + const hasBody = ["0", "7", "K", "L", "g", "x"].includes(type); + const header = Buffer.alloc(512); + writeString(header, 0, 100, entry.path); + writeOctal(header, 100, 8, entry.mode ?? (type === "5" ? 0o755 : 0o644)); + writeOctal(header, 108, 8, 0); + writeOctal(header, 116, 8, 0); + if (entry.base256Size === undefined) { + writeOctal(header, 124, 12, hasBody ? body.length : 0); + } else { + header[124] = 0x80; + header.writeBigUInt64BE(BigInt(entry.base256Size), 128); + } + writeOctal(header, 136, 12, 0); + writeString(header, 156, 1, type); + writeString(header, 157, 100, entry.linkPath ?? ""); + writeString(header, 257, 6, "ustar\0"); + writeString(header, 263, 2, "00"); + entry.mutateHeader?.(header); + updateTarChecksum(header); + blocks.push(header); + if (hasBody && entry.base256Size === undefined) { + blocks.push(body, Buffer.alloc((512 - (body.length % 512)) % 512)); + } + } + if (endBlocks) blocks.push(Buffer.alloc(1024)); + return Buffer.concat(blocks); +} + export function tarBytes(params: { name: string; body?: Buffer; @@ -76,10 +122,7 @@ export function tarBytes(params: { export function tarEntriesBytes( entries: ReadonlyArray<{ name: string; body?: Buffer }>, ): Buffer { - return Buffer.concat([ - ...entries.map((entry) => tarBytes(entry).subarray(0, -1024)), - Buffer.alloc(1024), - ]); + return tarFixture(entries.map(({ name, body }) => ({ path: name, body }))); } export async function zipBytes(params: { diff --git a/test/native-archive-equivalence.test.ts b/test/native-archive-equivalence.test.ts index 7652bca..89e7376 100644 --- a/test/native-archive-equivalence.test.ts +++ b/test/native-archive-equivalence.test.ts @@ -1,8 +1,9 @@ import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { expectFsSafeErrorSync } from "./helpers/security.js"; +import { tarFixture, type TarFixtureEntry } from "./helpers/archive-fuzz.js"; +import { useTempDirs } from "./helpers/vitest.js"; import { ARCHIVE_LIMIT_ERROR_CODE, extractArchive, @@ -25,71 +26,8 @@ try { } catch { // JS-only jobs intentionally exercise the fallback without a built binding. } -const tempDirs: string[] = []; - -type TarFixtureEntry = { - path: string; - body?: string; - mode?: number; - type?: "0" | "1" | "2" | "5" | "7" | "K" | "L" | "S" | "g" | "x"; - linkPath?: string; - base256Size?: number; -}; - -function writeString(block: Buffer, offset: number, length: number, value: string): void { - block.write(value, offset, Math.min(length, Buffer.byteLength(value)), "utf8"); -} - -function writeOctal(block: Buffer, offset: number, length: number, value: number): void { - writeString(block, offset, length, `${value.toString(8).padStart(length - 1, "0")}\0`); -} - -function tarFixture(entries: TarFixtureEntry[]): Buffer { - const blocks: Buffer[] = []; - for (const fixture of entries) { - const body = Buffer.from(fixture.body ?? ""); - const type = fixture.type ?? "0"; - const header = Buffer.alloc(512); - writeString(header, 0, 100, fixture.path); - writeOctal(header, 100, 8, fixture.mode ?? (type === "5" ? 0o755 : 0o644)); - writeOctal(header, 108, 8, 0); - writeOctal(header, 116, 8, 0); - const hasBody = - type === "0" || - type === "7" || - type === "K" || - type === "L" || - type === "g" || - type === "x"; - const size = fixture.base256Size ?? (hasBody ? body.length : 0); - if (fixture.base256Size === undefined) { - writeOctal(header, 124, 12, size); - } else { - header[124] = 0x80; - header.writeBigUInt64BE(BigInt(size), 128); - } - writeOctal(header, 136, 12, 0); - header.fill(0x20, 148, 156); - writeString(header, 156, 1, type); - writeString(header, 157, 100, fixture.linkPath ?? ""); - writeString(header, 257, 6, "ustar\0"); - writeString(header, 263, 2, "00"); - const checksum = header.reduce((sum, byte) => sum + byte, 0); - writeString(header, 148, 8, `${checksum.toString(8).padStart(6, "0")}\0 `); - blocks.push(header); - if (hasBody && fixture.base256Size === undefined) { - blocks.push(body, Buffer.alloc((512 - (body.length % 512)) % 512)); - } - } - blocks.push(Buffer.alloc(1024)); - return Buffer.concat(blocks); -} - -async function tempRoot(): Promise { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-archive-")); - tempDirs.push(root); - return root; -} +const { tempRoot: createTempRoot } = useTempDirs(); +const tempRoot = () => createTempRoot("fs-safe-native-archive-"); function useBackend(backend: "native" | "javascript"): void { if (backend === "native") { @@ -117,10 +55,9 @@ async function settleWithin(promise: Promise, milliseconds = 2_000): Promi } } -afterEach(async () => { +afterEach(() => { __resetFsSafeNativeConfigForTest(); __resetNativeLoaderForTest(); - await Promise.all(tempDirs.splice(0).map((root) => fs.rm(root, { recursive: true, force: true }))); }); const archiveBackends = native diff --git a/test/native-integration.test.ts b/test/native-integration.test.ts index a97f837..8d604d1 100644 --- a/test/native-integration.test.ts +++ b/test/native-integration.test.ts @@ -1,11 +1,10 @@ import fsSync from "node:fs"; import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; import { Readable } from "node:stream"; import { afterEach, describe, expect, it } from "vitest"; import { expectFsSafeError } from "./helpers/security.js"; -import { itWin32 } from "./helpers/vitest.js"; +import { itWin32, useTempDirs } from "./helpers/vitest.js"; import { configureFsSafeNative } from "../src/native-config.js"; import { acquireFileLock } from "../src/file-lock.js"; import { @@ -25,18 +24,25 @@ try { // matrix deliberately proves that installation without them still works. } -const roots: string[] = []; +const { tempRoot } = useTempDirs(); -afterEach(async () => { +afterEach(() => { configureFsSafeNative({ mode: "auto" }); __resetNativeLoaderForTest(); - await Promise.all(roots.splice(0).map((root) => fs.rm(root, { recursive: true, force: true }))); }); +async function pinnedWriteRoot( + mode: "off" | "require", + suffix: string, +): Promise { + if (mode === "require") __setNativeLoaderForTest(() => native!); + configureFsSafeNative({ mode }); + return await tempRoot(`fs-safe-${mode}-${suffix}-`); +} + describe.runIf(native)("native filesystem primitives", () => { it("opens beneath a directory descriptor and reports containment and fd identity", async () => { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-open-")); - roots.push(root); + const root = await tempRoot("fs-safe-native-open-"); await fs.mkdir(path.join(root, "nested")); await fs.writeFile(path.join(root, "nested", "value"), "ok"); const rootFd = fsSync.openSync(root, fsSync.constants.O_RDONLY); @@ -57,8 +63,7 @@ describe.runIf(native)("native filesystem primitives", () => { }); it("maps no-replace collisions to EEXIST without changing either file", async () => { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-rename-")); - roots.push(root); + const root = await tempRoot("fs-safe-native-rename-"); await fs.writeFile(path.join(root, "source"), "source"); await fs.writeFile(path.join(root, "target"), "target"); const rootFd = fsSync.openSync(root, fsSync.constants.O_RDONLY); @@ -74,8 +79,7 @@ describe.runIf(native)("native filesystem primitives", () => { }); it("replaces an existing target by descriptor-relative native rename", async () => { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-rename-replace-")); - roots.push(root); + const root = await tempRoot("fs-safe-native-rename-replace-"); await fs.writeFile(path.join(root, "source"), "source"); await fs.writeFile(path.join(root, "target"), "target"); const rootFd = fsSync.openSync(root, fsSync.constants.O_RDONLY); @@ -89,8 +93,7 @@ describe.runIf(native)("native filesystem primitives", () => { }); itWin32("rejects reparse-point directory components", async () => { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-reparse-")); - roots.push(root); + const root = await tempRoot("fs-safe-native-reparse-"); const real = path.join(root, "real"); await fs.mkdir(real); await fs.writeFile(path.join(real, "value"), "ok"); @@ -114,8 +117,7 @@ describe.runIf(native)("native filesystem primitives", () => { }, })); configureFsSafeNative({ mode: "require" }); - const directory = await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-write-")); - roots.push(directory); + const directory = await tempRoot("fs-safe-native-write-"); await runPinnedWriteHelper({ rootPath: directory, relativeParentPath: "nested", @@ -143,14 +145,7 @@ describe.runIf(native)("native filesystem primitives", () => { it.each(["off", "require"] as const)( "rejects a create-only collision before consuming the input stream in %s mode", async (mode) => { - if (mode === "require") { - __setNativeLoaderForTest(() => native!); - } - configureFsSafeNative({ mode }); - const directory = await fs.mkdtemp( - path.join(os.tmpdir(), `fs-safe-${mode}-collision-stream-`), - ); - roots.push(directory); + const directory = await pinnedWriteRoot(mode, "collision-stream"); await fs.writeFile(path.join(directory, "value"), "original"); let consumed = false; const stream = Readable.from((async function* () { @@ -179,12 +174,7 @@ describe.runIf(native)("native filesystem primitives", () => { it.runIf(process.platform !== "win32").each(["off", "require"] as const)( "preserves an explicit zero file mode in %s mode", async (mode) => { - if (mode === "require") { - __setNativeLoaderForTest(() => native!); - } - configureFsSafeNative({ mode }); - const directory = await fs.mkdtemp(path.join(os.tmpdir(), `fs-safe-${mode}-zero-mode-`)); - roots.push(directory); + const directory = await pinnedWriteRoot(mode, "zero-mode"); await runPinnedWriteHelper({ rootPath: directory, @@ -203,12 +193,7 @@ describe.runIf(native)("native filesystem primitives", () => { it.each(["off", "require"] as const)( "removes partial output when a streamed write exceeds its limit in %s mode", async (mode) => { - if (mode === "require") { - __setNativeLoaderForTest(() => native!); - } - configureFsSafeNative({ mode }); - const directory = await fs.mkdtemp(path.join(os.tmpdir(), `fs-safe-${mode}-stream-limit-`)); - roots.push(directory); + const directory = await pinnedWriteRoot(mode, "stream-limit"); const stream = Readable.from([Buffer.from("12"), Buffer.from("34")]); await expect( @@ -230,12 +215,7 @@ describe.runIf(native)("native filesystem primitives", () => { it.each(["off", "require"] as const)( "allows exactly one of many concurrent create-only writes in %s mode", async (mode) => { - if (mode === "require") { - __setNativeLoaderForTest(() => native!); - } - configureFsSafeNative({ mode }); - const directory = await fs.mkdtemp(path.join(os.tmpdir(), `fs-safe-${mode}-write-race-`)); - roots.push(directory); + const directory = await pinnedWriteRoot(mode, "write-race"); const attempts = Array.from({ length: 32 }, (_, index) => runPinnedWriteHelper({ rootPath: directory, @@ -260,8 +240,7 @@ describe.runIf(native)("native filesystem primitives", () => { it("uses the native transaction for root-level pinned writes", async () => { __setNativeLoaderForTest(() => native!); configureFsSafeNative({ mode: "require" }); - const directory = await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-root-write-")); - roots.push(directory); + const directory = await tempRoot("fs-safe-native-root-write-"); await runPinnedWriteHelper({ rootPath: directory, relativeParentPath: "", @@ -286,8 +265,7 @@ describe.runIf(native)("native filesystem primitives", () => { }, })); configureFsSafeNative({ mode: "require" }); - const directory = await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-overwrite-")); - roots.push(directory); + const directory = await tempRoot("fs-safe-native-overwrite-"); await fs.mkdir(path.join(directory, "nested")); await fs.writeFile(path.join(directory, "nested/value"), "old"); await runPinnedWriteHelper({ @@ -306,8 +284,7 @@ describe.runIf(native)("native filesystem primitives", () => { it("rejects native writes when the expected root identity does not match", async () => { __setNativeLoaderForTest(() => native!); configureFsSafeNative({ mode: "require" }); - const directory = await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-identity-")); - roots.push(directory); + const directory = await tempRoot("fs-safe-native-identity-"); const identity = await fs.lstat(directory); await expectFsSafeError(runPinnedWriteHelper({ rootPath: directory, @@ -333,8 +310,7 @@ describe.runIf(native)("native filesystem primitives", () => { }, })); configureFsSafeNative({ mode: "require" }); - const directory = await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-lock-")); - roots.push(directory); + const directory = await tempRoot("fs-safe-native-lock-"); const targetPath = path.join(directory, "state.json"); const lock = await acquireFileLock(targetPath, { payload: () => ({ pid: process.pid }) }); try { @@ -364,10 +340,7 @@ describe.runIf(native)("native filesystem primitives", () => { }, })); configureFsSafeNative({ mode: "require" }); - const directory = await fs.realpath( - await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-lock-denial-")), - ); - roots.push(directory); + const directory = await fs.realpath(await tempRoot("fs-safe-native-lock-denial-")); const targetPath = path.join(directory, "state.json"); const lock = await acquireFileLock(targetPath, { retry: { minTimeout: 1, maxTimeout: 2 }, @@ -396,10 +369,7 @@ describe.runIf(native)("native filesystem primitives", () => { }, })); configureFsSafeNative({ mode: "require" }); - const directory = await fs.realpath( - await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-lock-permission-")), - ); - roots.push(directory); + const directory = await fs.realpath(await tempRoot("fs-safe-native-lock-permission-")); const targetPath = path.join(directory, "state.json"); const lockPath = `${targetPath}.lock`; @@ -416,8 +386,7 @@ describe.runIf(native)("native filesystem primitives", () => { it("publishes by native rename without replacing an existing target", async () => { __setNativeLoaderForTest(() => native!); configureFsSafeNative({ mode: "require" }); - const directory = await fs.mkdtemp(path.join(os.tmpdir(), "fs-safe-native-publish-")); - roots.push(directory); + const directory = await tempRoot("fs-safe-native-publish-"); const sourcePath = path.join(directory, "source"); const targetPath = path.join(directory, "target"); await fs.writeFile(sourcePath, "content"); diff --git a/test/opened-realpath.test.ts b/test/opened-realpath.test.ts index eea7e01..6e15142 100644 --- a/test/opened-realpath.test.ts +++ b/test/opened-realpath.test.ts @@ -11,6 +11,26 @@ describe("opened file realpath resolution", () => { vi.restoreAllMocks(); }); + async function withOpenedFile( + prefix: string, + name: string, + run: (fixture: { + directory: string; + filePath: string; + handle: Awaited>; + }) => Promise, + ): Promise { + const directory = await tempRoot(prefix); + const filePath = path.join(directory, name); + await fs.writeFile(filePath, name, "utf8"); + const handle = await fs.open(filePath, "r"); + try { + await run({ directory, filePath, handle }); + } finally { + await handle.close(); + } + } + function blockDescriptorPathLookups( fd: number, pathError?: { path: string; code: string }, @@ -30,92 +50,60 @@ describe("opened file realpath resolution", () => { } it("falls back to the supplied path when descriptor paths are unavailable", async () => { - const directory = await tempRoot("fs-safe-opened-realpath-"); - const filePath = path.join(directory, "current.txt"); - await fs.writeFile(filePath, "current", "utf8"); - const handle = await fs.open(filePath, "r"); - - try { + await withOpenedFile("fs-safe-opened-realpath-", "current.txt", async ({ filePath, handle }) => { blockDescriptorPathLookups(handle.fd); - await expect(resolveOpenedFileRealPathForHandle(handle, filePath)).resolves.toBe( await fs.realpath(filePath), ); - } finally { - await handle.close(); - } + }); }); it("preserves non-not-found errors from the supplied path", async () => { - const directory = await tempRoot("fs-safe-opened-realpath-error-"); - const filePath = path.join(directory, "denied.txt"); - await fs.writeFile(filePath, "denied", "utf8"); - const handle = await fs.open(filePath, "r"); - - try { + await withOpenedFile("fs-safe-opened-realpath-error-", "denied.txt", async ({ filePath, handle }) => { blockDescriptorPathLookups(handle.fd, { path: filePath, code: "EACCES" }); - await expect(resolveOpenedFileRealPathForHandle(handle, filePath)).rejects.toMatchObject({ code: "EACCES", }); - } finally { - await handle.close(); - } + }); }); itPosix("finds a renamed open file by identity in its original parent", async () => { - const directory = await tempRoot("fs-safe-opened-realpath-renamed-"); - const originalPath = path.join(directory, "before.txt"); - const renamedPath = path.join(directory, "after.txt"); - await fs.writeFile(originalPath, "renamed", "utf8"); - const handle = await fs.open(originalPath, "r"); - - try { - await fs.rename(originalPath, renamedPath); + await withOpenedFile("fs-safe-opened-realpath-renamed-", "before.txt", async ({ + directory, + filePath, + handle, + }) => { + const renamedPath = path.join(directory, "after.txt"); + await fs.rename(filePath, renamedPath); blockDescriptorPathLookups(handle.fd); - - await expect(resolveOpenedFileRealPathForHandle(handle, originalPath)).resolves.toBe( + await expect(resolveOpenedFileRealPathForHandle(handle, filePath)).resolves.toBe( await fs.realpath(renamedPath), ); - } finally { - await handle.close(); - } + }); }); itPosix("rejects an unlinked open file when no path with its identity remains", async () => { - const directory = await tempRoot("fs-safe-opened-realpath-unlinked-"); - const filePath = path.join(directory, "unlinked.txt"); - await fs.writeFile(filePath, "unlinked", "utf8"); - const handle = await fs.open(filePath, "r"); - - try { + await withOpenedFile("fs-safe-opened-realpath-unlinked-", "unlinked.txt", async ({ filePath, handle }) => { await fs.unlink(filePath); blockDescriptorPathLookups(handle.fd); - await expect(resolveOpenedFileRealPathForHandle(handle, filePath)).rejects.toMatchObject({ code: "path-mismatch", }); - } finally { - await handle.close(); - } + }); }); itPosix("rejects cleanly when an opened file and its parent are both removed", async () => { - const directory = await tempRoot("fs-safe-opened-realpath-removed-parent-"); - const filePath = path.join(directory, "removed.txt"); - await fs.writeFile(filePath, "removed", "utf8"); - const handle = await fs.open(filePath, "r"); - - try { + await withOpenedFile("fs-safe-opened-realpath-removed-parent-", "removed.txt", async ({ + directory, + filePath, + handle, + }) => { await fs.unlink(filePath); await fs.rmdir(directory); blockDescriptorPathLookups(handle.fd); - await expect(resolveOpenedFileRealPathForHandle(handle, filePath)).rejects.toMatchObject({ code: "path-mismatch", }); - } finally { - await handle.close(); - } + }); }); }); diff --git a/test/permission-secret-stress.test.ts b/test/permission-secret-stress.test.ts index f6a2639..a01bef9 100644 --- a/test/permission-secret-stress.test.ts +++ b/test/permission-secret-stress.test.ts @@ -3,7 +3,6 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { itPosix, useTempDirs } from "./helpers/vitest.js"; -import { FsSafeError } from "../src/errors.js"; import { formatPermissionRemediation, inspectPathPermissions, @@ -14,28 +13,40 @@ import { readSecretFileSync } from "../src/secret-file.js"; import { readSecureFile } from "../src/secure-file.js"; const { tempRoot } = useTempDirs(); +const POSIX_PERMISSIONS: PermissionCheck = { + ok: true, + isSymlink: false, + isDir: false, + mode: 0o100644, + bits: 0o644, + source: "posix", + worldWritable: false, + groupWritable: false, + worldReadable: true, + groupReadable: true, +}; afterEach(() => { vi.restoreAllMocks(); }); -function expectCode(error: unknown, code: FsSafeError["code"]): boolean { - expect(error).toBeInstanceOf(FsSafeError); - expect((error as FsSafeError).code).toBe(code); - return true; -} - describe("permission and secret stress matrix", () => { itPosix.each([ - [0o400, true], - [0o500, true], - [0o600, true], - [0o700, true], - [0o440, false], - [0o604, false], - [0o620, false], - [0o602, false], - ] as const)("classifies secure-file mode %s independently of owner execute bits", async (mode, accepted) => { + [0o400, false, true], + [0o500, false, true], + [0o600, false, true], + [0o700, false, true], + [0o440, false, false], + [0o404, false, false], + [0o440, true, true], + [0o404, true, true], + [0o620, true, false], + [0o602, true, false], + ] as const)("classifies secure-file mode %s (allow readable=%s)", async ( + mode, + allowReadableByOthers, + accepted, + ) => { const root = await tempRoot("fs-safe-secure-mode-matrix-"); const filePath = path.join(root, "credential"); await fs.writeFile(filePath, "secret", { mode: 0o600 }); @@ -44,36 +55,14 @@ describe("permission and secret stress matrix", () => { const permissions = await inspectPathPermissions(filePath); expect(permissions.bits).toBe(mode); - const read = readSecureFile({ filePath }); + const read = readSecureFile({ filePath, permissions: { allowReadableByOthers } }); if (accepted) { await expect(read).resolves.toMatchObject({ buffer: Buffer.from("secret") }); } else { - await expect(read).rejects.toSatisfy((error: unknown) => - expectCode(error, "insecure-permissions"), - ); - } - }); - - itPosix("allows read-only group/world access only through the explicit option", async () => { - const root = await tempRoot("fs-safe-secure-readable-"); - for (const mode of [0o440, 0o404]) { - const filePath = path.join(root, `credential-${mode.toString(8)}`); - await fs.writeFile(filePath, "secret", { mode: 0o600 }); - await fs.chmod(filePath, mode); - - await expect( - readSecureFile({ filePath, permissions: { allowReadableByOthers: true } }), - ).resolves.toMatchObject({ buffer: Buffer.from("secret") }); - } - - for (const mode of [0o620, 0o602]) { - const filePath = path.join(root, `credential-${mode.toString(8)}`); - await fs.writeFile(filePath, "secret", { mode: 0o600 }); - await fs.chmod(filePath, mode); - - await expect( - readSecureFile({ filePath, permissions: { allowReadableByOthers: true } }), - ).rejects.toSatisfy((error: unknown) => expectCode(error, "insecure-permissions")); + await expect(read).rejects.toMatchObject({ + name: "FsSafeError", + code: "insecure-permissions", + }); } }); @@ -103,43 +92,18 @@ describe("permission and secret stress matrix", () => { }); }); - it("shell-quotes unsafe POSIX remediation paths", () => { - const perms: PermissionCheck = { - ok: true, - isSymlink: false, - isDir: false, - mode: 0o100644, - bits: 0o644, - source: "posix", - worldWritable: false, - groupWritable: false, - worldReadable: true, - groupReadable: true, - }; - - expect( - formatPermissionRemediation({ - targetPath: "/tmp/token; touch /tmp/unintended", - perms, - isDir: false, - posixMode: 0o600, - }), - ).toBe("chmod 600 '/tmp/token; touch /tmp/unintended'"); - expect( - formatPermissionRemediation({ - targetPath: "/tmp/owner's token", - perms, - isDir: false, - posixMode: 0o600, - }), - ).toBe("chmod 600 '/tmp/owner'\\''s token'"); + it.each([ + ["/tmp/token; touch /tmp/unintended", "chmod 600 '/tmp/token; touch /tmp/unintended'"], + ["/tmp/owner's token", "chmod 600 '/tmp/owner'\\''s token'"], + ["-unexpected-option", "chmod 600 -- -unexpected-option"], + ])("shell-quotes POSIX remediation path %s", (targetPath, expected) => { expect( formatPermissionRemediation({ - targetPath: "-unexpected-option", - perms, + targetPath, + perms: POSIX_PERMISSIONS, isDir: false, posixMode: 0o600, }), - ).toBe("chmod 600 -- -unexpected-option"); + ).toBe(expected); }); }); diff --git a/test/store-stress.test.ts b/test/store-stress.test.ts index 2612572..0fc964d 100644 --- a/test/store-stress.test.ts +++ b/test/store-stress.test.ts @@ -162,20 +162,6 @@ describe("store stress matrix", () => { await expect(store.readText("one/fresh.txt")).resolves.toBe("fresh"); }); - it("rejects unsafe keys consistently across async and sync stores", async () => { - const root = await tempRoot("fs-safe-store-keys-"); - const asyncStore = fileStore({ rootDir: root }); - const syncStore = fileStoreSync({ rootDir: root }); - const unsafeKeys = ["", " ", "../escape", "/absolute", "a/../../escape", "C:name", "a/C:name"]; - - for (const key of unsafeKeys) { - expect(() => asyncStore.path(key)).toThrow(); - expect(() => syncStore.path(key)).toThrow(); - await expect(asyncStore.writeText(key, "nope")).rejects.toThrow(); - expect(() => syncStore.writeText(key, "nope")).toThrow(); - } - }); - itPosix("keeps exact private and queue modes across repeated atomic replacement", async () => { const root = await tempRoot("fs-safe-store-modes-"); const privateStore = fileStore({ rootDir: path.join(root, "private"), private: true }); @@ -190,7 +176,5 @@ describe("store stress matrix", () => { expect((await fs.stat(privateStore.path("nested/value.txt"))).mode & 0o777).toBe(0o600); expect((await fs.stat(path.join(root, "private", "nested"))).mode & 0o777).toBe(0o700); expect((await fs.stat(paths.jsonPath)).mode & 0o777).toBe(0o600); - expect((await fs.stat(queueDir)).mode & 0o777).toBe(0o700); - expect((await fs.stat(failedDir)).mode & 0o777).toBe(0o700); }); });