From 4d7ccad542256fbc34b85229494b9c3654c5163c Mon Sep 17 00:00:00 2001 From: Yigtwxx Date: Sun, 2 Aug 2026 10:34:35 +0300 Subject: [PATCH 1/4] fix(path): reject drive-relative untrusted relative paths splitSafeRelativePath() rejected absolute paths via path.win32.isAbsolute(), which reports false for the drive-relative spelling "C:name". The segment was returned unchanged and path.resolve() then consumed the drive prefix, so on Windows "C:secret.txt" and "secret.txt" resolved to the same file. Measured with path.win32.resolve("C:\root", ...): ["C:evil"] -> C:\root\evil ["a","C:b"] -> C:\root\a\b ["a","D:b"] -> D:\a\b Reject any segment starting with a drive letter, checked per segment because a drive-relative spelling anywhere in the path aliases the whole prefix. The check is unconditional, matching the existing path.win32.isAbsolute() guard. --- CHANGELOG.md | 4 ++++ docs/errors.md | 2 +- src/path.ts | 7 +++++++ test/api-coverage.test.ts | 2 +- test/windows-path.test.ts | 41 ++++++++++++++++++++++++++++++++++++++- 5 files changed, 53 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45dae4f..1647763 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Suffix Windows reserved basenames with `_` in `sanitizeUntrustedFileName()` while preserving case and extensions on every platform, including dollar names and superscript COM/LPT variants; thanks @SebTardif (#67). +### Security and Correctness + +- Reject drive-relative segments such as `C:secret.txt` and `a/C:b` in untrusted relative paths with `invalid-path`. `path.win32.isAbsolute()` reports these as relative, so on Windows `path.resolve()` consumed the drive prefix and silently aliased them onto the plain in-root path, letting two distinct store keys resolve to the same file. + ## 0.5.1 - 2026-08-01 ### Security and Correctness diff --git a/docs/errors.md b/docs/errors.md index 3235ede..822d133 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -92,7 +92,7 @@ type FsSafeErrorCode = | `helper-failed` | A native mechanism or multi-step operational helper failed. | Inspect `cause` and any operation-specific `details`; retrying may be unsafe if the operation partially completed. | | `helper-unavailable` | Required native binding could not be loaded. | Unsupported platform, missing/incompatible bundled binary, or `FS_SAFE_NATIVE_MODE=require`. `auto` falls back where possible; `require` fails closed. | | `insecure-permissions` | A secure file or path permission check found a mode/ACL that allows broader access than requested. | File or directory is group/world writable/readable; Windows ACL grants broad read. | -| `invalid-path` | Input was empty, contained NUL, was an unparseable URL, or otherwise unusable. | Caller didn't validate input; input was a network path on Windows. | +| `invalid-path` | Input was empty, contained NUL, was an unparseable URL, or otherwise unusable. | Caller didn't validate input; input was a network path on Windows, or a drive-relative path such as `C:name`. | | `not-empty` | `remove()` on a non-empty directory. | Use `replaceDirectoryAtomic` or remove children first. | | `not-file` | Read or copy targeted a non-regular file. | Target was a directory, FIFO, socket, device. | | `not-found` | The target does not exist (or its parent does not, with `mkdir: false`). | Typical missing-file case. | diff --git a/src/path.ts b/src/path.ts index 82505ae..28a5d9f 100644 --- a/src/path.ts +++ b/src/path.ts @@ -12,6 +12,10 @@ export { type UnsafeDeviceReadPathReason, } from "./device-path.js"; +// Windows treats "C:name" as relative to the drive's own working directory, so +// path.win32.isAbsolute() reports false while path.resolve() still consumes the +// prefix. Reject the spelling outright rather than let it alias a plain segment. +const DRIVE_RELATIVE_SEGMENT = /^[A-Za-z]:/; const NOT_FOUND_CODES = new Set(["ENOENT", "ENOTDIR"]); const SYMLINK_OPEN_CODES = new Set(["ELOOP", "EINVAL", "ENOTSUP"]); const POSIX_SEPARATOR_CHAR_CODE = 0x2f; @@ -156,6 +160,9 @@ export function splitSafeRelativePath(relativePath: string): string[] { if (segment === "..") { throw new FsSafeError("invalid-path", "relative path must not contain '..'"); } + if (DRIVE_RELATIVE_SEGMENT.test(segment)) { + throw new FsSafeError("invalid-path", "relative path must not contain a drive letter"); + } } return segments; } diff --git a/test/api-coverage.test.ts b/test/api-coverage.test.ts index 4425d73..9f041bf 100644 --- a/test/api-coverage.test.ts +++ b/test/api-coverage.test.ts @@ -257,7 +257,7 @@ describe("path helpers", () => { expect(safeStatSync(path.join(root, "missing"))).toBeNull(); expect(() => assertNoNulPathInput("a\0b")).toThrow("NUL"); expect(splitSafeRelativePath("./a//b")).toEqual(["a", "b"]); - for (const bad of ["../x", "/x", "C:\\x", "a\\b", "a\0b"]) { + for (const bad of ["../x", "/x", "C:\\x", "C:evil", "C:..", "a/C:b", "a\\b", "a\0b"]) { expect(() => splitSafeRelativePath(bad)).toThrow(); } expect(resolveSafeRelativePath(root, "a/b")).toBe(path.join(root, "a", "b")); diff --git a/test/windows-path.test.ts b/test/windows-path.test.ts index cf5d627..6d6dd12 100644 --- a/test/windows-path.test.ts +++ b/test/windows-path.test.ts @@ -1,6 +1,14 @@ +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; import { describe, expect, it } from "vitest"; +import { fileStore } from "../src/file-store.js"; import { isWindowsNetworkPath } from "../src/local-file-access.js"; -import { isPathInside, normalizeWindowsPathForComparison } from "../src/path.js"; +import { + isPathInside, + normalizeWindowsPathForComparison, + splitSafeRelativePath, +} from "../src/path.js"; describe("Windows path classification", () => { it("distinguishes extended local paths from UNC paths", () => { @@ -33,6 +41,37 @@ describe("Windows comparison normalization", () => { }); }); +describe("drive-relative relative paths", () => { + it("rejects drive-letter segments that Windows would resolve away", () => { + const inputs = ["C:evil", "c:evil", "C:", "C:..", "C:evil/sub", "a/C:b", "./C:evil", "a/D:b"]; + for (const input of inputs) { + expect(() => splitSafeRelativePath(input)).toThrow("drive letter"); + } + }); + + it("keeps accepting ordinary segments that merely contain a colon", () => { + expect(splitSafeRelativePath("logs/2026-08-02T10:30:00Z.log")).toEqual([ + "logs", + "2026-08-02T10:30:00Z.log", + ]); + }); + + it("stops a drive-relative store key from aliasing a plain key", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "fs-safe-drive-relative-")); + try { + const store = fileStore({ rootDir }); + await store.writeText("secret.txt", "real"); + + await expect(store.writeText("C:secret.txt", "aliased")).rejects.toMatchObject({ + code: "invalid-path", + }); + await expect(readFile(path.join(rootDir, "secret.txt"), "utf8")).resolves.toBe("real"); + } finally { + await rm(rootDir, { force: true, recursive: true }); + } + }); +}); + describe.skipIf(process.platform !== "win32")("Windows containment with padded roots", () => { it("does not report a sibling directory as inside a space-padded root", () => { expect(isPathInside("C:\\root ", "C:\\root\\secret.txt")).toBe(false); From 7024025e7420e4ac2a02fb1d44a6a2e1ac8910cc Mon Sep 17 00:00:00 2001 From: Yigtwxx Date: Sun, 2 Aug 2026 13:11:40 +0300 Subject: [PATCH 2/4] fix(root): reject drive-relative paths on every root entry point The first commit guarded splitSafeRelativePath(), but Root never calls it: assertValidRootRelativePath() only checked for NUL bytes, and resolvePathInRoot() then ran path.resolve(root, "C:secret.txt"), which consumes the drive prefix. Every Root method, and every fileStore() key that delegates to one (open, the read variants, remove, exists), still aliased. Measured on Windows before this change, with the new regression test: AssertionError: promise resolved to a ReadResult instead of rejecting + realPath: \fs-safe-drive-relative-delegates-XXXXXX\secret.txt Move the predicate into isDriveRelativePath() and apply it in assertValidRootRelativePath(), the funnel every Root method reaches directly or through resolvePathInRoot(). The regex now excludes drive-absolute spellings such as C:\root\file.txt, which Root accepts today when they stay inside the root, and a positive test pins that. The per-segment rule stays in splitSafeRelativePath(), where segments become separate path.resolve() arguments and an embedded "a/C:b" really does alias. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LfSdVPKK9F2JvDqpkzS3ND --- CHANGELOG.md | 2 +- docs/root.md | 2 +- src/path.ts | 8 ++++-- src/root-context.ts | 10 ++++++- test/windows-path.test.ts | 56 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 73 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1647763..a5906fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ ### Security and Correctness -- Reject drive-relative segments such as `C:secret.txt` and `a/C:b` in untrusted relative paths with `invalid-path`. `path.win32.isAbsolute()` reports these as relative, so on Windows `path.resolve()` consumed the drive prefix and silently aliased them onto the plain in-root path, letting two distinct store keys resolve to the same file. +- Reject drive-relative paths such as `C:secret.txt` with `invalid-path` on every `Root` method and on the `fileStore()` keys that delegate to them, and reject drive-relative segments such as `a/C:b` in the resolving path parser. `path.win32.isAbsolute()` reports these as relative, so on Windows `path.resolve()` consumed the drive prefix and silently aliased them onto the plain in-root path, letting two distinct keys resolve to the same file. Drive-absolute inputs such as `C:\root\file.txt` keep working. ## 0.5.1 - 2026-08-01 diff --git a/docs/root.md b/docs/root.md index b985736..63bb96e 100644 --- a/docs/root.md +++ b/docs/root.md @@ -151,7 +151,7 @@ Every method throws `FsSafeError` with a `code`. Branch on `err.code`, not messa | Code | When it fires | |---|---| -| `invalid-path` | The input path is malformed, including embedded NUL bytes. | +| `invalid-path` | The input path is malformed, including embedded NUL bytes and drive-relative spellings such as `C:name`. | | `outside-workspace` | The input resolves outside the root, or contains a `..` segment that would escape it. | | `not-found` | The target does not exist (or its parent does not, with `mkdir: false`). | | `not-file` | A read or copy targeted a non-regular file (directory, FIFO, socket, …). | diff --git a/src/path.ts b/src/path.ts index 28a5d9f..698cbfa 100644 --- a/src/path.ts +++ b/src/path.ts @@ -15,7 +15,7 @@ export { // Windows treats "C:name" as relative to the drive's own working directory, so // path.win32.isAbsolute() reports false while path.resolve() still consumes the // prefix. Reject the spelling outright rather than let it alias a plain segment. -const DRIVE_RELATIVE_SEGMENT = /^[A-Za-z]:/; +const DRIVE_RELATIVE_PREFIX = /^[A-Za-z]:(?![\\/])/; const NOT_FOUND_CODES = new Set(["ENOENT", "ENOTDIR"]); const SYMLINK_OPEN_CODES = new Set(["ELOOP", "EINVAL", "ENOTSUP"]); const POSIX_SEPARATOR_CHAR_CODE = 0x2f; @@ -139,6 +139,10 @@ export function safeStatSync(targetPath: string): fs.Stats | null { } } +export function isDriveRelativePath(value: string): boolean { + return DRIVE_RELATIVE_PREFIX.test(value); +} + export function splitSafeRelativePath(relativePath: string): string[] { if (relativePath.length === 0 || relativePath === ".") { return []; @@ -160,7 +164,7 @@ export function splitSafeRelativePath(relativePath: string): string[] { if (segment === "..") { throw new FsSafeError("invalid-path", "relative path must not contain '..'"); } - if (DRIVE_RELATIVE_SEGMENT.test(segment)) { + if (isDriveRelativePath(segment)) { throw new FsSafeError("invalid-path", "relative path must not contain a drive letter"); } } diff --git a/src/root-context.ts b/src/root-context.ts index 14a07c9..c8160a9 100644 --- a/src/root-context.ts +++ b/src/root-context.ts @@ -3,7 +3,12 @@ import os from "node:os"; import path from "node:path"; import { FsSafeError } from "./errors.js"; import { expandHomePrefix } from "./home-dir.js"; -import { assertNoNulPathInput, isNotFoundPathError, isPathInside } from "./path.js"; +import { + assertNoNulPathInput, + isDriveRelativePath, + isNotFoundPathError, + isPathInside, +} from "./path.js"; import { ROOT_PATH_ALIAS_POLICIES, resolveRootPath } from "./root-path.js"; export type RootContext = { @@ -17,6 +22,9 @@ export const ensureTrailingSep = (value: string) => export function assertValidRootRelativePath(relativePath: string): void { assertNoNulPathInput(relativePath, "relative path contains a NUL byte"); + if (isDriveRelativePath(relativePath)) { + throw new FsSafeError("invalid-path", "relative path must not start with a drive letter"); + } } let cachedHomePath: { raw: string; real: string } | undefined; diff --git a/test/windows-path.test.ts b/test/windows-path.test.ts index 6d6dd12..ac3b1a7 100644 --- a/test/windows-path.test.ts +++ b/test/windows-path.test.ts @@ -5,10 +5,12 @@ import { describe, expect, it } from "vitest"; import { fileStore } from "../src/file-store.js"; import { isWindowsNetworkPath } from "../src/local-file-access.js"; import { + isDriveRelativePath, isPathInside, normalizeWindowsPathForComparison, splitSafeRelativePath, } from "../src/path.js"; +import { root as openRoot } from "../src/root.js"; describe("Windows path classification", () => { it("distinguishes extended local paths from UNC paths", () => { @@ -49,6 +51,15 @@ describe("drive-relative relative paths", () => { } }); + it("separates drive-relative spellings from drive-absolute ones", () => { + for (const value of ["C:evil", "c:", "Z:.."]) { + expect(isDriveRelativePath(value), value).toBe(true); + } + for (const value of ["C:\\root\\file.txt", "C:/root/file.txt", "notes/c:file"]) { + expect(isDriveRelativePath(value), value).toBe(false); + } + }); + it("keeps accepting ordinary segments that merely contain a colon", () => { expect(splitSafeRelativePath("logs/2026-08-02T10:30:00Z.log")).toEqual([ "logs", @@ -56,6 +67,51 @@ describe("drive-relative relative paths", () => { ]); }); + it("rejects a drive-relative key on every root and store entry point", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "fs-safe-drive-relative-delegates-")); + try { + const store = fileStore({ rootDir }); + await store.writeText("secret.txt", "real"); + const root = await openRoot(rootDir); + const key = "C:secret.txt"; + + const calls: Array<[string, () => Promise]> = [ + ["root.read", () => root.read(key)], + ["root.readText", () => root.readText(key)], + ["root.open", () => root.open(key)], + ["root.stat", () => root.stat(key)], + ["root.exists", () => root.exists(key)], + ["root.list", () => root.list(key)], + ["root.remove", () => root.remove(key)], + ["root.write", () => root.write(key, "aliased")], + ["root.move", () => root.move(key, "moved.txt")], + ["store.readText", () => store.readText(key)], + ["store.open", () => store.open(key)], + ["store.exists", () => store.exists(key)], + ["store.remove", () => store.remove(key)], + ]; + for (const [label, call] of calls) { + await expect(call(), label).rejects.toMatchObject({ code: "invalid-path" }); + } + + await expect(readFile(path.join(rootDir, "secret.txt"), "utf8")).resolves.toBe("real"); + } finally { + await rm(rootDir, { force: true, recursive: true }); + } + }); + + it("keeps accepting an absolute path that stays inside the root", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "fs-safe-drive-relative-absolute-")); + try { + const root = await openRoot(rootDir); + await root.write("note.txt", "kept"); + + await expect(root.readText(path.join(rootDir, "note.txt"))).resolves.toBe("kept"); + } finally { + await rm(rootDir, { force: true, recursive: true }); + } + }); + it("stops a drive-relative store key from aliasing a plain key", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "fs-safe-drive-relative-")); try { From 2a10498bcc9f6f4bae9f821fd4475cd64c9f22a6 Mon Sep 17 00:00:00 2001 From: Yigtwxx Date: Sun, 2 Aug 2026 20:55:43 +0300 Subject: [PATCH 3/4] fix(root): reject drive-relative input before absolute reads resolve it readAbsolute() and reader() enter through readPathInRoot(), which resolved the raw input against the root before any validation ran. On Windows that consumed a drive-relative prefix, so C:secret.txt reached readFileInRoot() as secret.txt and aliased the plain key the rest of the branch already rejected. Validate the raw input first. The check is the drive-relative one only, so drive-absolute paths inside the root keep working. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ah1kwRo32A6sGU1tbMFEFk --- src/root-context.ts | 7 +++++++ src/root-impl.ts | 2 ++ test/windows-path.test.ts | 8 ++++++++ 3 files changed, 17 insertions(+) diff --git a/src/root-context.ts b/src/root-context.ts index c8160a9..d8c357c 100644 --- a/src/root-context.ts +++ b/src/root-context.ts @@ -27,6 +27,13 @@ export function assertValidRootRelativePath(relativePath: string): void { } } +export function assertValidRootPathInput(filePath: string): void { + assertNoNulPathInput(filePath, "file path contains a NUL byte"); + if (isDriveRelativePath(filePath)) { + throw new FsSafeError("invalid-path", "file path must not start with a drive letter"); + } +} + let cachedHomePath: { raw: string; real: string } | undefined; export async function expandRelativePathWithHome(relativePath: string): Promise { diff --git a/src/root-impl.ts b/src/root-impl.ts index 87665fc..c77b885 100644 --- a/src/root-impl.ts +++ b/src/root-impl.ts @@ -36,6 +36,7 @@ import { readOpenedFileSafely, type ReadResult } from "./read-opened-file.js"; import { pathStatFromStats } from "./path-stat.js"; import { resolveRootPath } from "./root-path.js"; import { + assertValidRootPathInput, assertValidRootRelativePath, ensureTrailingSep, expandRelativePathWithHome, @@ -683,6 +684,7 @@ async function readPathInRoot( symlinks?: SymlinkPolicy; }, ): Promise { + assertValidRootPathInput(params.filePath); const rootDir = root.rootDir; const candidatePath = path.isAbsolute(params.filePath) ? path.resolve(params.filePath) diff --git a/test/windows-path.test.ts b/test/windows-path.test.ts index ac3b1a7..efb48ee 100644 --- a/test/windows-path.test.ts +++ b/test/windows-path.test.ts @@ -85,6 +85,8 @@ describe("drive-relative relative paths", () => { ["root.remove", () => root.remove(key)], ["root.write", () => root.write(key, "aliased")], ["root.move", () => root.move(key, "moved.txt")], + ["root.readAbsolute", () => root.readAbsolute(key)], + ["root.reader", () => root.reader()(key)], ["store.readText", () => store.readText(key)], ["store.open", () => store.open(key)], ["store.exists", () => store.exists(key)], @@ -107,6 +109,12 @@ describe("drive-relative relative paths", () => { await root.write("note.txt", "kept"); await expect(root.readText(path.join(rootDir, "note.txt"))).resolves.toBe("kept"); + await expect( + root.readAbsolute(path.join(rootDir, "note.txt")), + ).resolves.toMatchObject({ buffer: Buffer.from("kept") }); + await expect(root.reader()(path.join(rootDir, "note.txt"))).resolves.toEqual( + Buffer.from("kept"), + ); } finally { await rm(rootDir, { force: true, recursive: true }); } From 8d82fb7da225828840c6b8008af0cb96c9fa141f Mon Sep 17 00:00:00 2001 From: Yigtwxx Date: Sun, 2 Aug 2026 21:11:11 +0300 Subject: [PATCH 4/4] test(path): resolve the temp root before asserting absolute reads The drive-relative absolute-path test opened a root at the raw mkdtemp() result, which is /var/... on macOS and an 8.3 short name on Windows CI. openRoot() resolves the root, so the unresolved path the test passed back in read as outside-workspace and failed every non-Linux leg. main fails an equivalent repro the same way, so this is the test skipping the realpath convention the rest of the suite follows, not a regression in the drive-relative rejection. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Dpz7Mnp2xh7WHWHVAnPWtK --- test/windows-path.test.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/windows-path.test.ts b/test/windows-path.test.ts index efb48ee..ce60e93 100644 --- a/test/windows-path.test.ts +++ b/test/windows-path.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { mkdtemp, readFile, realpath, rm } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; @@ -103,7 +103,12 @@ describe("drive-relative relative paths", () => { }); it("keeps accepting an absolute path that stays inside the root", async () => { - const rootDir = await mkdtemp(path.join(os.tmpdir(), "fs-safe-drive-relative-absolute-")); + // The temp dir must be resolved: macOS hands back /var/... for a /private/var/... + // root and Windows CI hands back an 8.3 short name, either of which reads as + // outside the root once openRoot() resolves it. + const rootDir = await realpath( + await mkdtemp(path.join(os.tmpdir(), "fs-safe-drive-relative-absolute-")), + ); try { const root = await openRoot(rootDir); await root.write("note.txt", "kept");