Compare commits

..

No commits in common. "main" and "ci/windows-runner" have entirely different histories.

16 changed files with 54 additions and 197 deletions

View File

@ -23,18 +23,14 @@ jobs:
uses: rhysd/actionlint@914e7df21a07ef503a81201c76d2b11c789d3fca # v1.7.12
check:
name: Node ${{ matrix.node }} check (${{ matrix.os }})
name: Node 22 check (${{ matrix.os }})
runs-on: ${{ matrix.os }}
timeout-minutes: 20
strategy:
fail-fast: false
matrix:
node:
- 22
- 24
os:
- ubuntu-latest
- macos-latest
- windows-latest
steps:
- name: Check out
@ -49,7 +45,7 @@ jobs:
- name: Set up Node
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
node-version: ${{ matrix.node }}
node-version: "22"
cache: pnpm
cache-dependency-path: pnpm-lock.yaml

View File

@ -1,15 +1,5 @@
# Changelog
## Unreleased
## 0.2.1 - 2026-05-08
### Fixes
- Align POSIX and Windows handling for literal `..`-prefixed write targets, preserve whitespace in direct home-relative path inputs, and run the check suite on Windows CI. (#14; thanks @sjf)
- Keep source prepack builds isolated from parent monorepo ambient type packages such as Bun typings. (#13; thanks @Kaspre)
- Let secret-file reads follow symlink paths through the pinned real target unless callers opt into `rejectSymlink: true`.
## 0.2.0 - 2026-05-07
### Features

View File

@ -1,6 +1,6 @@
{
"name": "@openclaw/fs-safe",
"version": "0.2.1",
"version": "0.2.0",
"description": "Capability-style filesystem roots for Node.js apps that handle untrusted relative paths.",
"license": "MIT",
"repository": {

View File

@ -6,9 +6,9 @@ const LINE_BUDGETS = new Map([
["src/file-store.ts", 580],
["src/permissions.ts", 566],
["src/pinned-python.ts", 655],
["src/root-impl.ts", 1750],
["src/root-impl.ts", 1744],
["src/root-path.ts", 862],
["test/api-coverage.test.ts", 983],
["test/api-coverage.test.ts", 982],
["test/new-primitives.test.ts", 1500],
]);

View File

@ -31,21 +31,18 @@ export function resolveOsHomeDir(
function resolveRawHomeDir(env: NodeJS.ProcessEnv, homedir: () => string): string | undefined {
const explicitHome = normalize(env.OPENCLAW_HOME);
if (!explicitHome) {
return resolveRawOsHomeDir(env, homedir);
}
const segments = path.normalize(explicitHome).split(path.sep);
if (segments[0] !== "~") {
if (explicitHome) {
if (explicitHome === "~" || explicitHome.startsWith("~/") || explicitHome.startsWith("~\\")) {
const fallbackHome = resolveRawOsHomeDir(env, homedir);
if (fallbackHome) {
return explicitHome.replace(/^~(?=$|[\\/])/, fallbackHome);
}
return undefined;
}
return explicitHome;
}
// OPENCLAW_HOME starts with "~"; expand against the os home dir. Fall
// back to undefined when there is no os home to expand against rather
// than returning a raw "~"-prefixed path the caller cannot use.
const fallbackHome = resolveRawOsHomeDir(env, homedir);
if (!fallbackHome) {
return undefined;
}
return expandHomePrefix(explicitHome, { home: fallbackHome });
return resolveRawOsHomeDir(env, homedir);
}
function resolveRawOsHomeDir(env: NodeJS.ProcessEnv, homedir: () => string): string | undefined {
@ -90,6 +87,9 @@ export function expandHomePrefix(
homedir?: () => string;
},
): string {
// Normalize and split into path segments. path.normalize converts "/"
// to the native separator on Windows and leaves "\" as a literal name
// character on POSIX, so the segment check is platform-correct.
const segments = path.normalize(input).split(path.sep);
if (segments[0] !== "~") {
return input;

View File

@ -6,6 +6,7 @@ import { normalizeLowercaseStringOrEmpty } from "./string-coerce.js";
const NOT_FOUND_CODES = new Set(["ENOENT", "ENOTDIR"]);
const SYMLINK_OPEN_CODES = new Set(["ELOOP", "EINVAL", "ENOTSUP"]);
const PARENT_SEGMENT_PREFIX = /^\.\.(?:[\\/]|$)/u;
const POSIX_SEPARATOR_CHAR_CODE = 0x2f;
export function normalizeWindowsPathForComparison(input: string): string {
@ -48,9 +49,8 @@ export function isPathInside(root: string, target: string): boolean {
const rootForCompare = normalizeWindowsPathForComparison(path.win32.resolve(root));
const targetForCompare = normalizeWindowsPathForComparison(path.win32.resolve(target));
const relative = path.win32.relative(rootForCompare, targetForCompare);
const firstSegment = relative.split(path.win32.sep)[0];
return (
relative === "" || (firstSegment !== ".." && !path.win32.isAbsolute(relative))
relative === "" || (!PARENT_SEGMENT_PREFIX.test(relative) && !path.win32.isAbsolute(relative))
);
}
@ -69,8 +69,7 @@ export function isPathInside(root: string, target: string): boolean {
const resolvedRoot = path.resolve(root);
const resolvedTarget = path.resolve(target);
const relative = path.relative(resolvedRoot, resolvedTarget);
const firstSegment = relative.split(path.posix.sep)[0];
return relative === "" || (firstSegment !== ".." && !path.isAbsolute(relative));
return relative === "" || (!PARENT_SEGMENT_PREFIX.test(relative) && !path.isAbsolute(relative));
}
export function resolveSafeBaseDir(rootDir: string): string {

View File

@ -1250,10 +1250,8 @@ async function resolvePinnedWriteTargetInRoot(
throw new FsSafeError("path-alias", "path alias escape blocked", { cause: err });
}
// resolvePathInRoot already enforces isPathInside, so any actual escape
// is rejected upstream.
const relativeResolved = path.relative(rootReal, resolved);
if (path.isAbsolute(relativeResolved)) {
if (relativeResolved.startsWith("..") || path.isAbsolute(relativeResolved)) {
throw new FsSafeError("outside-workspace", "file is outside workspace root");
}
const relativePosix = relativeResolved
@ -1334,11 +1332,10 @@ async function resolvePinnedOperationPathInRoot(
if ((relativeResolved === "" || relativeResolved === ".") && params.allowRoot === true) {
return { rootReal: resolved.rootReal, resolved: resolved.canonicalPath, relativePosix: "" };
}
const firstSegment = relativeResolved.split(path.sep)[0];
if (
relativeResolved === "" ||
relativeResolved === "." ||
firstSegment === ".." ||
relativeResolved.startsWith("..") ||
path.isAbsolute(relativeResolved)
) {
throw new FsSafeError("outside-workspace", "file is outside workspace root");

View File

@ -55,26 +55,12 @@ function readSecretFileOutcomeSync(
};
}
if (previewStat.isSymbolicLink()) {
if (!options.rejectSymlink) {
try {
previewStat = fs.statSync(resolvedPath);
} catch (error) {
const normalized = normalizeSecretReadError(error);
return {
ok: false,
code: (error as NodeJS.ErrnoException).code === "ENOENT" ? "not-found" : "invalid-path",
error: normalized,
message: `Failed to inspect ${label} file at ${resolvedPath}: ${String(normalized)}`,
};
}
} else {
return {
ok: false,
code: "symlink",
message: `${label} file at ${resolvedPath} must not be a symlink.`,
};
}
if (options.rejectSymlink && previewStat.isSymbolicLink()) {
return {
ok: false,
code: "symlink",
message: `${label} file at ${resolvedPath} must not be a symlink.`,
};
}
if (!previewStat.isFile()) {
return {

View File

@ -1,5 +1,4 @@
import fs from "node:fs/promises";
import { realpathSync } from "node:fs";
import os from "node:os";
import path from "node:path";
import { Readable } from "node:stream";
@ -243,11 +242,8 @@ describe("path helpers", () => {
expect(isSymlinkOpenError(Object.assign(new Error("x"), { code: "ELOOP" }))).toBe(true);
expect(isPathInside(root, file)).toBe(true);
expect(resolveSafeBaseDir(root)).toBe(`${path.resolve(root)}${path.sep}`);
// Use the sync realpath to compare against safeRealpathSync. On windows
// fs.realpathSync and fs.realpath (async) sometimes disagree on 8.3
// short-name canonicalization (e.g. "RUNNER~1" vs "runneradmin").
expect(safeRealpathSync(file, cache)).toBe(realpathSync(file));
expect(safeRealpathSync(file, cache)).toBe(realpathSync(file));
expect(safeRealpathSync(file, cache)).toBe(await fs.realpath(file));
expect(safeRealpathSync(file, cache)).toBe(await fs.realpath(file));
expect(safeRealpathSync(path.join(root, "missing"), cache)).toBeNull();
expect(isPathInsideWithRealpath(root, file, { cache })).toBe(true);
expect(isPathInsideWithRealpath(root, path.join(root, "missing"), { requireRealpath: false }))
@ -461,7 +457,7 @@ describe("URL, install, and local-root helpers", () => {
label: "media roots",
requireFile: true,
}),
).toMatchObject({ path: realpathSync(file) });
).toMatchObject({ path: await fs.realpath(file) });
expect(() =>
resolveLocalPathFromRootsSync({
filePath: "bad\0path",

View File

@ -2,17 +2,7 @@ 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, vi } from "vitest";
import { createBoundedReadStream, createMaxBytesTransform } from "../src/bounded-read-stream.js";
import {
assertAsyncDirectoryGuard,
assertSyncDirectoryGuard,
createAsyncDirectoryGuard,
createNearestExistingDirectoryGuard,
createNearestExistingSyncDirectoryGuard,
createSyncDirectoryGuard,
} from "../src/directory-guard.js";
import { drainFileLockManagerForTest, resetFileLockManagerForTest } from "../src/file-lock.js";
import { sameFileIdentity } from "../src/file-identity.js";
import { readLocalFileFromRoots, resolveLocalPathFromRootsSync } from "../src/local-roots.js";
@ -148,53 +138,6 @@ describe("small identity and lock wrappers", () => {
});
});
describe("bounded streams and directory guard coverage", () => {
it("returns raw streams without limits and rejects oversized limited streams", async () => {
const raw = Readable.from(["ok"]);
const returned = createBoundedReadStream({ handle: { createReadStream: () => raw } }, undefined);
expect(returned).toBe(raw);
await expect(async () => {
for await (const _chunk of Readable.from(["ab", "cd"]).pipe(createMaxBytesTransform(3))) {
// Drain the stream so transform errors surface.
}
}).rejects.toMatchObject({ code: "too-large" });
});
it("detects changed or invalid directory guards", async () => {
const root = await tempRoot("fs-safe-dir-guard-more-");
const nested = path.join(root, "nested");
const filePath = path.join(root, "file.txt");
await fs.mkdir(nested);
await fs.writeFile(filePath, "not a dir", "utf8");
await expect(createAsyncDirectoryGuard(filePath)).rejects.toMatchObject({ code: "not-file" });
expect(() => createSyncDirectoryGuard(filePath)).toThrow("directory component");
const asyncGuard = await createAsyncDirectoryGuard(nested);
const syncGuard = createSyncDirectoryGuard(nested);
await expect(assertAsyncDirectoryGuard({ ...asyncGuard, realPath: root })).rejects.toMatchObject({
code: "path-mismatch",
});
expect(() => assertSyncDirectoryGuard({ ...syncGuard, realPath: root })).toThrow(
"directory changed",
);
await fs.rm(nested, { recursive: true });
await fs.writeFile(nested, "not a dir", "utf8");
await expect(assertAsyncDirectoryGuard(asyncGuard)).rejects.toMatchObject({
code: "not-file",
});
expect(() => assertSyncDirectoryGuard(syncGuard)).toThrow("directory component");
const nearest = await createNearestExistingDirectoryGuard(root, path.join(root, "missing", "x"));
expect(nearest.dir).toBe(root);
expect(createNearestExistingSyncDirectoryGuard(root, path.join(root, "missing", "x")).dir)
.toBe(root);
});
});
describe("sibling temp coverage", () => {
it("syncs temp files and parent dirs when requested", async () => {
const root = await tempRoot("fs-safe-sibling-more-");

View File

@ -56,19 +56,17 @@ export const LITERAL_SUSPICIOUS_WRITE_PAYLOADS = [
"%2e%2e/pwned.txt",
"%2e%2e%2fpwned.txt",
"%252e%252e%252fpwned.txt",
// ".." prefix without an actual separator: a single literal filename
// ("..%2fpwned.txt") or two literal segments ("..%00", "pwned.txt") that
// resolve fully inside root. Accepted on both platforms.
"..%2fpwned.txt",
"..%00/pwned.txt",
] as const;
export const POSIX_LITERAL_SUSPICIOUS_WRITE_PAYLOADS = [
"nested\\..\\..\\pwned.txt",
"C:\\Windows\\win.ini",
"\\\\server\\share\\pwned.txt",
// "..\\" is a real traversal on Windows (separator) but a literal filename
// on POSIX (where "\\" is a regular name character).
] as const;
export const SAFE_REJECTED_SUSPICIOUS_WRITE_PAYLOADS = [
"..%2fpwned.txt",
"..%00/pwned.txt",
"..\\pwned.txt",
] as const;
@ -100,15 +98,10 @@ export async function makeTempLayout(
return { outside, outsideFile, root };
}
export function expectFsSafeCode(
error: unknown,
codes: readonly string[],
opts: { allowUnsupportedPlatformOnWindows?: boolean } = {},
): void {
export function expectFsSafeCode(error: unknown, codes: readonly string[]): void {
expect(error).toBeInstanceOf(FsSafeError);
const accepted = process.platform === "win32" && opts.allowUnsupportedPlatformOnWindows
? [...codes, "unsupported-platform"]
: codes;
const accepted =
process.platform === "win32" ? [...codes, "unsupported-platform"] : codes;
expect(accepted).toContain((error as FsSafeError).code);
}

View File

@ -72,15 +72,11 @@ describe("read boundary bypass attempts", () => {
return true;
});
await expect(safeRoot.stat("../secret.txt")).rejects.toSatisfy((error: unknown) => {
expectFsSafeCode(error, ["outside-workspace", "invalid-path", "path-alias"], {
allowUnsupportedPlatformOnWindows: true,
});
expectFsSafeCode(error, ["outside-workspace", "invalid-path", "path-alias"]);
return true;
});
await expect(safeRoot.list(".." as string)).rejects.toSatisfy((error: unknown) => {
expectFsSafeCode(error, ["outside-workspace", "invalid-path", "path-alias"], {
allowUnsupportedPlatformOnWindows: true,
});
expectFsSafeCode(error, ["outside-workspace", "invalid-path", "path-alias"]);
return true;
});
await expect(scope.files(["../secret.txt"])).resolves.toMatchObject({ ok: false });
@ -100,15 +96,11 @@ describe("read boundary bypass attempts", () => {
return true;
});
await expect(safeRoot.stat("link/secret.txt")).rejects.toSatisfy((error: unknown) => {
expectFsSafeCode(error, ["outside-workspace", "path-alias", "symlink"], {
allowUnsupportedPlatformOnWindows: true,
});
expectFsSafeCode(error, ["outside-workspace", "path-alias", "symlink"]);
return true;
});
await expect(safeRoot.list("link")).rejects.toSatisfy((error: unknown) => {
expectFsSafeCode(error, ["outside-workspace", "path-alias", "symlink"], {
allowUnsupportedPlatformOnWindows: true,
});
expectFsSafeCode(error, ["outside-workspace", "path-alias", "symlink"]);
return true;
});
});
@ -128,9 +120,7 @@ describe("read boundary bypass attempts", () => {
return true;
});
await expect(safeRoot.stat("secret-link.txt")).rejects.toSatisfy((error: unknown) => {
expectFsSafeCode(error, ["outside-workspace", "path-alias", "symlink"], {
allowUnsupportedPlatformOnWindows: true,
});
expectFsSafeCode(error, ["outside-workspace", "path-alias", "symlink"]);
return true;
});
@ -178,9 +168,7 @@ describe("read boundary bypass attempts", () => {
return true;
});
await expect(safeRoot.stat(layout.outsideFile)).rejects.toSatisfy((error: unknown) => {
expectFsSafeCode(error, ["outside-workspace", "path-alias", "invalid-path"], {
allowUnsupportedPlatformOnWindows: true,
});
expectFsSafeCode(error, ["outside-workspace", "path-alias", "invalid-path"]);
return true;
});

View File

@ -1,5 +1,4 @@
import fs from "node:fs/promises";
import { realpathSync } from "node:fs";
import os from "node:os";
import path from "node:path";
import { describe, expect, it } from "vitest";
@ -174,10 +173,7 @@ describe("local roots helpers", () => {
const missingPath = path.join(uploadsDir, "new", "later.txt");
const outsidePath = path.join(baseDir, "outside.txt");
await fs.writeFile(filePath, "ok", "utf8");
// Use the sync realpath to compare against resolveLocalPathFromRootsSync.
// On windows fs.realpathSync and fs.realpath (async) sometimes disagree
// on 8.3 short-name canonicalization (e.g. "RUNNER~1" vs "runneradmin").
const uploadsReal = realpathSync(uploadsDir);
const uploadsReal = await fs.realpath(uploadsDir);
expect(
resolveLocalPathFromRootsSync({
@ -186,7 +182,7 @@ describe("local roots helpers", () => {
label: "media roots",
requireFile: true,
}),
).toEqual({ path: realpathSync(filePath), root: uploadsReal });
).toEqual({ path: await fs.realpath(filePath), root: uploadsReal });
expect(
resolveLocalPathFromRootsSync({
filePath: missingPath,

View File

@ -68,15 +68,9 @@ describe("secret file helpers", () => {
const root = await tempRoot("fs-safe-secret-");
const target = path.join(root, "target.txt");
const link = path.join(root, "link.txt");
const broken = path.join(root, "broken.txt");
await fs.writeFile(target, "secret", "utf8");
await fs.symlink(target, link);
await fs.symlink(path.join(root, "missing.txt"), broken);
expect(readSecretFileSync(link, "API token")).toBe("secret");
expect(tryReadSecretFileSync(link, "API token")).toBe("secret");
expectSecretReadCode(() => readSecretFileSync(broken, "API token"), "not-found");
expect(tryReadSecretFileSync(broken, "API token")).toBeUndefined();
expect(() => readSecretFileSync(link, "API token", { rejectSymlink: true })).toThrow(
`API token file at ${link} must not be a symlink.`,
);

View File

@ -3,7 +3,7 @@ import path from "node:path";
import { Readable } from "node:stream";
import { afterEach, describe, expect, it } from "vitest";
import { fileStore, fileStoreSync } from "../src/file-store.js";
import { configureFsSafePython, root as openRoot } from "../src/index.js";
import { root as openRoot } from "../src/index.js";
import {
ESCAPING_DIRECTORY_PAYLOADS,
ESCAPING_WRITE_PAYLOADS,
@ -14,6 +14,7 @@ import {
makeTempLayout as makeSecurityTempLayout,
POSIX_LITERAL_SUSPICIOUS_WRITE_PAYLOADS,
SAFE_REJECTED_SUSPICIOUS_DIRECTORY_PAYLOADS,
SAFE_REJECTED_SUSPICIOUS_WRITE_PAYLOADS,
WINDOWS_REJECTED_SUSPICIOUS_DIRECTORY_PAYLOADS,
} from "./helpers/security.js";
@ -24,7 +25,6 @@ async function makeTempLayout(prefix: string) {
}
afterEach(async () => {
configureFsSafePython({ mode: "auto", pythonPath: undefined });
await Promise.all(tempDirs.splice(0).map((dir) => fsp.rm(dir, { force: true, recursive: true })));
});
@ -197,8 +197,6 @@ describe("write, move, and delete boundary bypass attempts", () => {
await expectNoOutsideWrite(layout);
});
// This test exercises many fs writes/reads/mkdirs; bump the timeout for
// slow windows fs under parallel test load (default 5s is sometimes tight).
it("keeps encoded, backslash, Windows, and UNC-looking write payloads literal and inside root", async () => {
const layout = await makeTempLayout("fs-safe-write-encoded-literal");
const safeRoot = await openRoot(layout.root);
@ -215,6 +213,9 @@ describe("write, move, and delete boundary bypass attempts", () => {
await expect(safeRoot.write(payload, "rejected"), `write safely rejects ${payload}`).rejects.toBeTruthy();
}
}
for (const payload of SAFE_REJECTED_SUSPICIOUS_WRITE_PAYLOADS) {
await expect(safeRoot.write(payload, "rejected"), `write safely rejects ${payload}`).rejects.toBeTruthy();
}
for (const payload of SAFE_REJECTED_SUSPICIOUS_DIRECTORY_PAYLOADS) {
await expect(safeRoot.mkdir(payload), `mkdir safely rejects ${payload}`).rejects.toBeTruthy();
}
@ -232,29 +233,8 @@ describe("write, move, and delete boundary bypass attempts", () => {
}
for (const payload of LITERAL_SUSPICIOUS_DIRECTORY_PAYLOADS) {
await safeRoot.mkdir(payload);
if (process.platform === "win32") {
// safeRoot.list uses the pinned helper which is unavailable on
// windows; verify the directory exists via fsp.stat instead.
await expect(fsp.stat(path.join(layout.root, payload)), `created literal ${payload}`)
.resolves.toSatisfy((stat) => stat.isDirectory());
} else {
await expect(safeRoot.list(payload), `list literal ${payload}`).resolves.toBeInstanceOf(Array);
}
await expect(safeRoot.list(payload), `list literal ${payload}`).resolves.toBeInstanceOf(Array);
}
await expectNoOutsideWrite(layout);
}, 15000);
it.runIf(process.platform !== "win32")("keeps literal '..'-prefixed paths available when the helper is disabled", async () => {
configureFsSafePython({ mode: "off" });
const layout = await makeTempLayout("fs-safe-write-helper-off-literal");
const safeRoot = await openRoot(layout.root);
await safeRoot.write("..%2fpwned.txt", "literal");
await expect(safeRoot.stat("..%2fpwned.txt")).resolves.toMatchObject({ isFile: true });
await safeRoot.remove("..%2fpwned.txt");
await expect(fsp.stat(path.join(layout.root, "..%2fpwned.txt"))).rejects.toMatchObject({
code: "ENOENT",
});
await expectNoOutsideWrite(layout);
});
});

View File

@ -11,7 +11,6 @@
"rootDir": "src",
"strict": true,
"target": "ES2022",
"types": ["node"],
"verbatimModuleSyntax": true
},
"include": ["src/**/*.ts"]