Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 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

### Security and Correctness
Expand Down
2 changes: 1 addition & 1 deletion docs/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
2 changes: 1 addition & 1 deletion docs/root.md
Original file line number Diff line number Diff line change
Expand Up @@ -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, …). |
Expand Down
11 changes: 11 additions & 0 deletions src/path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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_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;
Expand Down Expand Up @@ -135,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 [];
Expand All @@ -156,6 +164,9 @@ export function splitSafeRelativePath(relativePath: string): string[] {
if (segment === "..") {
throw new FsSafeError("invalid-path", "relative path must not contain '..'");
}
if (isDriveRelativePath(segment)) {
throw new FsSafeError("invalid-path", "relative path must not contain a drive letter");
}
}
return segments;
}
Expand Down
17 changes: 16 additions & 1 deletion src/root-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -17,6 +22,16 @@ 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");
}
}

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;
Expand Down
2 changes: 2 additions & 0 deletions src/root-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -683,6 +684,7 @@ async function readPathInRoot(
symlinks?: SymlinkPolicy;
},
): Promise<ReadResult> {
assertValidRootPathInput(params.filePath);
const rootDir = root.rootDir;
const candidatePath = path.isAbsolute(params.filePath)
? path.resolve(params.filePath)
Expand Down
2 changes: 1 addition & 1 deletion test/api-coverage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
110 changes: 109 additions & 1 deletion test/windows-path.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
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";
import { fileStore } from "../src/file-store.js";
import { isWindowsNetworkPath } from "../src/local-file-access.js";
import { isPathInside, normalizeWindowsPathForComparison } from "../src/path.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", () => {
Expand Down Expand Up @@ -33,6 +43,104 @@ 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("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",
"2026-08-02T10:30:00Z.log",
]);
});

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<unknown>]> = [
["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")],
["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)],
["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 () => {
// 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");

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 });
}
});

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);
Expand Down