Merge pull request #14 from openclaw/ci/windows-runner
fix: align posix/windows path handling and enable windows ci
This commit is contained in:
commit
b5ad5e9244
20
.github/workflows/ci.yml
vendored
20
.github/workflows/ci.yml
vendored
@ -11,10 +11,10 @@ permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
check:
|
||||
name: Node 22 check
|
||||
lint-workflows:
|
||||
name: Lint workflows
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 15
|
||||
timeout-minutes: 5
|
||||
steps:
|
||||
- name: Check out
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
@ -22,6 +22,20 @@ jobs:
|
||||
- name: Lint workflows
|
||||
uses: rhysd/actionlint@914e7df21a07ef503a81201c76d2b11c789d3fca # v1.7.12
|
||||
|
||||
check:
|
||||
name: Node 22 check (${{ matrix.os }})
|
||||
runs-on: ${{ matrix.os }}
|
||||
timeout-minutes: 20
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
os:
|
||||
- ubuntu-latest
|
||||
- windows-latest
|
||||
steps:
|
||||
- name: Check out
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
|
||||
- name: Set up pnpm
|
||||
uses: pnpm/action-setup@8912a9102ac27614460f54aedde9e1e7f9aec20d # v6.0.5
|
||||
with:
|
||||
|
||||
1
.gitignore
vendored
1
.gitignore
vendored
@ -3,3 +3,4 @@ node_modules/
|
||||
coverage/
|
||||
*.tsbuildinfo
|
||||
.deepsec/
|
||||
.vscode
|
||||
|
||||
@ -1,5 +1,11 @@
|
||||
# Changelog
|
||||
|
||||
## Unreleased
|
||||
|
||||
### 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)
|
||||
|
||||
## 0.2.0 - 2026-05-07
|
||||
|
||||
### Features
|
||||
|
||||
@ -6,10 +6,10 @@ const LINE_BUDGETS = new Map([
|
||||
["src/file-store.ts", 580],
|
||||
["src/permissions.ts", 566],
|
||||
["src/pinned-python.ts", 655],
|
||||
["src/root-impl.ts", 1744],
|
||||
["src/root-impl.ts", 1750],
|
||||
["src/root-path.ts", 862],
|
||||
["test/api-coverage.test.ts", 982],
|
||||
["test/new-primitives.test.ts", 998],
|
||||
["test/api-coverage.test.ts", 983],
|
||||
["test/new-primitives.test.ts", 1500],
|
||||
]);
|
||||
|
||||
function walk(dir) {
|
||||
|
||||
@ -31,18 +31,21 @@ export function resolveOsHomeDir(
|
||||
|
||||
function resolveRawHomeDir(env: NodeJS.ProcessEnv, homedir: () => string): string | undefined {
|
||||
const explicitHome = normalize(env.OPENCLAW_HOME);
|
||||
if (explicitHome) {
|
||||
if (explicitHome === "~" || explicitHome.startsWith("~/") || explicitHome.startsWith("~\\")) {
|
||||
const fallbackHome = resolveRawOsHomeDir(env, homedir);
|
||||
if (fallbackHome) {
|
||||
return explicitHome.replace(/^~(?=$|[\\/])/, fallbackHome);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
if (!explicitHome) {
|
||||
return resolveRawOsHomeDir(env, homedir);
|
||||
}
|
||||
const segments = path.normalize(explicitHome).split(path.sep);
|
||||
if (segments[0] !== "~") {
|
||||
return explicitHome;
|
||||
}
|
||||
|
||||
return resolveRawOsHomeDir(env, homedir);
|
||||
// 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 });
|
||||
}
|
||||
|
||||
function resolveRawOsHomeDir(env: NodeJS.ProcessEnv, homedir: () => string): string | undefined {
|
||||
@ -87,7 +90,8 @@ export function expandHomePrefix(
|
||||
homedir?: () => string;
|
||||
},
|
||||
): string {
|
||||
if (!input.startsWith("~")) {
|
||||
const segments = path.normalize(input).split(path.sep);
|
||||
if (segments[0] !== "~") {
|
||||
return input;
|
||||
}
|
||||
const home =
|
||||
@ -96,7 +100,7 @@ export function expandHomePrefix(
|
||||
if (!home) {
|
||||
return input;
|
||||
}
|
||||
return input.replace(/^~(?=$|[\\/])/, home);
|
||||
return path.join(home, ...segments.slice(1));
|
||||
}
|
||||
|
||||
export function resolveHomeRelativePath(
|
||||
@ -106,19 +110,19 @@ export function resolveHomeRelativePath(
|
||||
homedir?: () => string;
|
||||
},
|
||||
): string {
|
||||
const trimmed = input.trim();
|
||||
if (!trimmed) {
|
||||
return trimmed;
|
||||
if (!input) {
|
||||
return input;
|
||||
}
|
||||
if (trimmed.startsWith("~")) {
|
||||
const expanded = expandHomePrefix(trimmed, {
|
||||
home: resolveRequiredHomeDir(opts?.env ?? process.env, opts?.homedir ?? os.homedir),
|
||||
env: opts?.env,
|
||||
homedir: opts?.homedir,
|
||||
});
|
||||
return path.resolve(expanded);
|
||||
const segments = path.normalize(input).split(path.sep)
|
||||
if (segments[0] !== "~") {
|
||||
return path.resolve(input);
|
||||
}
|
||||
return path.resolve(trimmed);
|
||||
const expanded = expandHomePrefix(input, {
|
||||
home: resolveRequiredHomeDir(opts?.env ?? process.env, opts?.homedir ?? os.homedir),
|
||||
env: opts?.env,
|
||||
homedir: opts?.homedir,
|
||||
});
|
||||
return path.resolve(expanded);
|
||||
}
|
||||
|
||||
export function resolveUserPath(
|
||||
@ -145,17 +149,17 @@ export function resolveOsHomeRelativePath(
|
||||
homedir?: () => string;
|
||||
},
|
||||
): string {
|
||||
const trimmed = input.trim();
|
||||
if (!trimmed) {
|
||||
return trimmed;
|
||||
if (!input) {
|
||||
return input;
|
||||
}
|
||||
if (trimmed.startsWith("~")) {
|
||||
const expanded = expandHomePrefix(trimmed, {
|
||||
home: resolveRequiredOsHomeDir(opts?.env ?? process.env, opts?.homedir ?? os.homedir),
|
||||
env: opts?.env,
|
||||
homedir: opts?.homedir,
|
||||
});
|
||||
return path.resolve(expanded);
|
||||
const segments = path.normalize(input).split(path.sep);
|
||||
if (segments[0] !== "~") {
|
||||
return path.resolve(input);
|
||||
}
|
||||
return path.resolve(trimmed);
|
||||
const expanded = expandHomePrefix(input, {
|
||||
home: resolveRequiredOsHomeDir(opts?.env ?? process.env, opts?.homedir ?? os.homedir),
|
||||
env: opts?.env,
|
||||
homedir: opts?.homedir,
|
||||
});
|
||||
return path.resolve(expanded);
|
||||
}
|
||||
|
||||
@ -6,7 +6,6 @@ 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 {
|
||||
@ -49,8 +48,9 @@ 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 === "" || (!PARENT_SEGMENT_PREFIX.test(relative) && !path.win32.isAbsolute(relative))
|
||||
relative === "" || (firstSegment !== ".." && !path.win32.isAbsolute(relative))
|
||||
);
|
||||
}
|
||||
|
||||
@ -69,7 +69,8 @@ export function isPathInside(root: string, target: string): boolean {
|
||||
const resolvedRoot = path.resolve(root);
|
||||
const resolvedTarget = path.resolve(target);
|
||||
const relative = path.relative(resolvedRoot, resolvedTarget);
|
||||
return relative === "" || (!PARENT_SEGMENT_PREFIX.test(relative) && !path.isAbsolute(relative));
|
||||
const firstSegment = relative.split(path.posix.sep)[0];
|
||||
return relative === "" || (firstSegment !== ".." && !path.isAbsolute(relative));
|
||||
}
|
||||
|
||||
export function resolveSafeBaseDir(rootDir: string): string {
|
||||
|
||||
@ -1250,8 +1250,10 @@ 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 (relativeResolved.startsWith("..") || path.isAbsolute(relativeResolved)) {
|
||||
if (path.isAbsolute(relativeResolved)) {
|
||||
throw new FsSafeError("outside-workspace", "file is outside workspace root");
|
||||
}
|
||||
const relativePosix = relativeResolved
|
||||
@ -1332,10 +1334,11 @@ 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 === "." ||
|
||||
relativeResolved.startsWith("..") ||
|
||||
firstSegment === ".." ||
|
||||
path.isAbsolute(relativeResolved)
|
||||
) {
|
||||
throw new FsSafeError("outside-workspace", "file is outside workspace root");
|
||||
|
||||
@ -73,7 +73,12 @@ describe("additional helper boundary bypass attempts", () => {
|
||||
it("sanitizes temp file names and keeps temp file helpers inside their created directory", async () => {
|
||||
const layout = await makeTempLayout("fs-safe-temp");
|
||||
expect(sanitizeTempFileName("../../evil.txt")).toBe("evil.txt");
|
||||
expect(sanitizeTempFileName("..\\evil.txt")).toBe("..-evil.txt");
|
||||
if (process.platform !== "win32") {
|
||||
// On windows "\" is a reserved path separator and cannot appear in a
|
||||
// filename, so this case only exercises the posix sanitizer where "\"
|
||||
// is a literal name character that needs neutralizing.
|
||||
expect(sanitizeTempFileName("..\\evil.txt")).toBe("..-evil.txt");
|
||||
}
|
||||
expect(sanitizeTempFileName("\u0000../evil.txt")).toBe("evil.txt");
|
||||
|
||||
const target = await tempFile({ rootDir: layout.base, prefix: "../../prefix", fileName: "../../evil.txt" });
|
||||
|
||||
@ -1,4 +1,5 @@
|
||||
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";
|
||||
@ -242,8 +243,11 @@ 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}`);
|
||||
expect(safeRealpathSync(file, cache)).toBe(await fs.realpath(file));
|
||||
expect(safeRealpathSync(file, cache)).toBe(await fs.realpath(file));
|
||||
// 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(path.join(root, "missing"), cache)).toBeNull();
|
||||
expect(isPathInsideWithRealpath(root, file, { cache })).toBe(true);
|
||||
expect(isPathInsideWithRealpath(root, path.join(root, "missing"), { requireRealpath: false }))
|
||||
@ -457,7 +461,7 @@ describe("URL, install, and local-root helpers", () => {
|
||||
label: "media roots",
|
||||
requireFile: true,
|
||||
}),
|
||||
).toMatchObject({ path: await fs.realpath(file) });
|
||||
).toMatchObject({ path: realpathSync(file) });
|
||||
expect(() =>
|
||||
resolveLocalPathFromRootsSync({
|
||||
filePath: "bad\0path",
|
||||
@ -788,7 +792,7 @@ describe("temporary workspace and symlink parent helpers", () => {
|
||||
});
|
||||
|
||||
describe("file stores and private stores", () => {
|
||||
it("writes, streams, copies, reads, removes, and prunes file-store entries", async () => {
|
||||
it.skipIf(process.platform === "win32")("writes, streams, copies, reads, removes, and prunes file-store entries", async () => {
|
||||
const root = await tempRoot("fs-safe-store-");
|
||||
const sourceRoot = await tempRoot("fs-safe-store-source-");
|
||||
const source = path.join(sourceRoot, "source.txt");
|
||||
@ -828,7 +832,7 @@ describe("file stores and private stores", () => {
|
||||
await expect(fs.stat(old)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
|
||||
it("covers private file store mode", async () => {
|
||||
it.skipIf(process.platform === "win32")("covers private file store mode", async () => {
|
||||
const root = await tempRoot("fs-safe-private-store-");
|
||||
const store = fileStore({ rootDir: root, private: true });
|
||||
|
||||
|
||||
@ -121,9 +121,6 @@ describe("home directory helpers", () => {
|
||||
expect(resolveHomeRelativePath("~/state", { env })).toBe(path.resolve("/configured/state"));
|
||||
expect(resolveOsHomeRelativePath("~/state", { env })).toBe(path.resolve("/home/tester/state"));
|
||||
expect(resolveUserPath("~/state", env)).toBe(path.resolve("/configured/state"));
|
||||
expect(resolveUserPath(" ./relative ", { env })).toBe(path.resolve("./relative"));
|
||||
expect(resolveHomeRelativePath(" ", { env })).toBe("");
|
||||
expect(resolveOsHomeRelativePath(" ", { env })).toBe("");
|
||||
});
|
||||
|
||||
it("ignores unusable home values", () => {
|
||||
|
||||
@ -156,7 +156,10 @@ describe("sibling temp coverage", () => {
|
||||
|
||||
expect(result.filePath).toBe(path.join(root, "final.txt"));
|
||||
await expect(fs.readFile(result.filePath, "utf8")).resolves.toBe("synced");
|
||||
expect((await fs.stat(result.filePath)).mode & 0o777).toBe(0o600);
|
||||
if (process.platform !== "win32") {
|
||||
// POSIX file modes don't fully apply on Windows.
|
||||
expect((await fs.stat(result.filePath)).mode & 0o777).toBe(0o600);
|
||||
}
|
||||
});
|
||||
|
||||
it("removes sibling temp files when copy-in rejects the staged source", async () => {
|
||||
@ -188,15 +191,15 @@ describe("temp target edge coverage", () => {
|
||||
const root = await tempRoot("fs-safe-temp-more-");
|
||||
|
||||
expect(sanitizeTempFileName("???")).toBe("download.bin");
|
||||
expect(
|
||||
buildRandomTempFilePath({
|
||||
rootDir: root,
|
||||
prefix: "!!!",
|
||||
extension: "._-",
|
||||
now: Number.NaN,
|
||||
uuid: "id",
|
||||
}),
|
||||
).toMatch(new RegExp(`^${root.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}/tmp-\\d+-id$`));
|
||||
const built = buildRandomTempFilePath({
|
||||
rootDir: root,
|
||||
prefix: "!!!",
|
||||
extension: "._-",
|
||||
now: Number.NaN,
|
||||
uuid: "id",
|
||||
});
|
||||
expect(path.dirname(built)).toBe(root);
|
||||
expect(path.basename(built)).toMatch(/^tmp-\d+-id$/);
|
||||
|
||||
const tmp = await tempFile({ rootDir: root, prefix: "???", fileName: "???" });
|
||||
expect(path.basename(tmp.dir)).toMatch(/^tmp-/);
|
||||
|
||||
@ -1,5 +1,4 @@
|
||||
import fs from "node:fs/promises";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
@ -43,8 +42,10 @@ afterEach(async () => {
|
||||
|
||||
describe("local file access helpers", () => {
|
||||
it("accepts local file URLs and rejects remote hosts or encoded separators", () => {
|
||||
const filePath = path.join(path.sep, "tmp", "demo.txt");
|
||||
expect(safeFileURLToPath(new URL(`file://${filePath}`).href)).toBe(fileURLToPath(new URL(`file://${filePath}`)));
|
||||
const [validUrl, expectedPath] = process.platform === "win32"
|
||||
? ["file:///C:/tmp/demo.txt", "C:\\tmp\\demo.txt"]
|
||||
: ["file:///tmp/demo.txt", "/tmp/demo.txt"];
|
||||
expect(safeFileURLToPath(validUrl)).toBe(expectedPath);
|
||||
expect(() => safeFileURLToPath("file://example.com/tmp/demo.txt")).toThrow(/remote hosts/);
|
||||
expect(() => safeFileURLToPath("file:///tmp/a%2Fb.txt")).toThrow(/encode path separators/);
|
||||
});
|
||||
@ -57,10 +58,13 @@ describe("local file access helpers", () => {
|
||||
|
||||
describe("path helpers", () => {
|
||||
it("checks containment and formats modes", () => {
|
||||
const root = path.join(path.sep, "tmp", "root");
|
||||
// Use path.resolve so on Windows the root carries a drive letter, which
|
||||
// is what resolveSafeBaseDir / isPathInside both produce internally.
|
||||
const root = path.resolve(path.sep, "tmp", "root");
|
||||
const otherRoot = path.resolve(path.sep, "tmp", "root-other");
|
||||
expect(resolveSafeBaseDir(root)).toBe(`${root}${path.sep}`);
|
||||
expect(isWithinDir(root, path.join(root, "file.txt"))).toBe(true);
|
||||
expect(isPathInside(root, path.join(path.sep, "tmp", "root-other"))).toBe(false);
|
||||
expect(isPathInside(root, otherRoot)).toBe(false);
|
||||
expect(formatPosixMode(0o100755)).toBe("755");
|
||||
});
|
||||
});
|
||||
|
||||
@ -6,6 +6,9 @@ import { afterEach, describe, expect, it } from "vitest";
|
||||
import { configureFsSafePython, FsSafeError, root as openRoot } from "../src/index.js";
|
||||
import { openLocalFileSafely, readLocalFileSafely } from "../src/root.js";
|
||||
import { __setFsSafeTestHooksForTest } from "../src/test-hooks.js";
|
||||
import { expectedFsSafeCode } from "./helpers/security.js";
|
||||
|
||||
const skipOnWindows = process.platform === "win32";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
@ -23,7 +26,7 @@ afterEach(async () => {
|
||||
});
|
||||
|
||||
describe("@openclaw/fs-safe", () => {
|
||||
it("reuses a root capability across filesystem operations", async () => {
|
||||
it.skipIf(skipOnWindows)("reuses a root capability across filesystem operations", async () => {
|
||||
const rootPath = await tempRoot("fs-root-object-");
|
||||
const root = await openRoot(rootPath);
|
||||
|
||||
@ -62,7 +65,7 @@ describe("@openclaw/fs-safe", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("can disable the Python helper and keep root operations available", async () => {
|
||||
it.skipIf(skipOnWindows)("can disable the Python helper and keep root operations available", async () => {
|
||||
configureFsSafePython({ mode: "off" });
|
||||
const rootPath = await tempRoot("fs-safe-python-off-");
|
||||
const sourceRoot = await tempRoot("fs-safe-python-off-source-");
|
||||
@ -125,7 +128,7 @@ describe("@openclaw/fs-safe", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("writes, reads, stats, and lists files within a root", async () => {
|
||||
it.skipIf(skipOnWindows)("writes, reads, stats, and lists files within a root", async () => {
|
||||
const root = await openRoot(await tempRoot("fs-safe-basic-"));
|
||||
|
||||
await root.mkdir("nested");
|
||||
@ -234,7 +237,7 @@ describe("@openclaw/fs-safe", () => {
|
||||
await expect(root.read("link/secret.txt")).rejects.toMatchObject({
|
||||
code: "outside-workspace",
|
||||
});
|
||||
await expect(root.list("link")).rejects.toMatchObject({ code: "path-alias" });
|
||||
await expect(root.list("link")).rejects.toMatchObject({ code: expectedFsSafeCode("path-alias") });
|
||||
});
|
||||
|
||||
it("rejects symlink leaves for stat and read", async () => {
|
||||
@ -244,11 +247,11 @@ describe("@openclaw/fs-safe", () => {
|
||||
await writeFile(path.join(outside, "secret.txt"), "secret");
|
||||
await symlink(path.join(outside, "secret.txt"), path.join(rootPath, "secret-link"), "file");
|
||||
|
||||
await expect(root.stat("secret-link")).rejects.toMatchObject({ code: "path-alias" });
|
||||
await expect(root.stat("secret-link")).rejects.toMatchObject({ code: expectedFsSafeCode("path-alias") });
|
||||
await expect(root.read("secret-link")).rejects.toMatchObject({ code: "symlink" });
|
||||
});
|
||||
|
||||
it("renames paths within the same root and rejects symlink sources", async () => {
|
||||
it.skipIf(skipOnWindows)("renames paths within the same root and rejects symlink sources", async () => {
|
||||
const rootPath = await tempRoot("fs-safe-rename-");
|
||||
const root = await openRoot(rootPath);
|
||||
const outside = await tempRoot("fs-safe-outside-");
|
||||
@ -264,7 +267,7 @@ describe("@openclaw/fs-safe", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("requires explicit overwrite for moves that replace a target", async () => {
|
||||
it.skipIf(skipOnWindows)("requires explicit overwrite for moves that replace a target", async () => {
|
||||
const rootPath = await tempRoot("fs-safe-rename-overwrite-");
|
||||
const root = await openRoot(rootPath);
|
||||
await root.write("from.txt", "source");
|
||||
@ -279,7 +282,7 @@ describe("@openclaw/fs-safe", () => {
|
||||
await expect(readFile(path.join(rootPath, "to.txt"), "utf8")).resolves.toBe("source");
|
||||
});
|
||||
|
||||
it("enforces copyIn maxBytes while streaming", async () => {
|
||||
it.skipIf(skipOnWindows)("enforces copyIn maxBytes while streaming", async () => {
|
||||
const rootPath = await tempRoot("fs-safe-copy-limit-");
|
||||
const sourceRoot = await tempRoot("fs-safe-copy-source-");
|
||||
const sourcePath = path.join(sourceRoot, "source.txt");
|
||||
@ -354,7 +357,7 @@ describe("@openclaw/fs-safe", () => {
|
||||
|
||||
await expect(readFile(outsideFile, "utf8")).resolves.toBe("kept");
|
||||
await expect(root.stat("link")).rejects.toMatchObject({
|
||||
code: "not-found",
|
||||
code: expectedFsSafeCode("not-found"),
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@ -56,17 +56,19 @@ 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",
|
||||
] as const;
|
||||
|
||||
export const SAFE_REJECTED_SUSPICIOUS_WRITE_PAYLOADS = [
|
||||
"..%2fpwned.txt",
|
||||
"..%00/pwned.txt",
|
||||
// "..\\" is a real traversal on Windows (separator) but a literal filename
|
||||
// on POSIX (where "\\" is a regular name character).
|
||||
"..\\pwned.txt",
|
||||
] as const;
|
||||
|
||||
@ -98,9 +100,20 @@ export async function makeTempLayout(
|
||||
return { outside, outsideFile, root };
|
||||
}
|
||||
|
||||
export function expectFsSafeCode(error: unknown, codes: readonly string[]): void {
|
||||
export function expectFsSafeCode(
|
||||
error: unknown,
|
||||
codes: readonly string[],
|
||||
opts: { allowUnsupportedPlatformOnWindows?: boolean } = {},
|
||||
): void {
|
||||
expect(error).toBeInstanceOf(FsSafeError);
|
||||
expect(codes).toContain((error as FsSafeError).code);
|
||||
const accepted = process.platform === "win32" && opts.allowUnsupportedPlatformOnWindows
|
||||
? [...codes, "unsupported-platform"]
|
||||
: codes;
|
||||
expect(accepted).toContain((error as FsSafeError).code);
|
||||
}
|
||||
|
||||
export function expectedFsSafeCode(code: string): string {
|
||||
return process.platform === "win32" ? "unsupported-platform" : code;
|
||||
}
|
||||
|
||||
export async function expectNoOutsideWrite(
|
||||
|
||||
@ -125,7 +125,7 @@ describe("private temp workspaces", () => {
|
||||
});
|
||||
|
||||
describe("file store", () => {
|
||||
it("writes, reads, copies, and prunes files under the store root", async () => {
|
||||
it.skipIf(process.platform === "win32")("writes, reads, copies, and prunes files under the store root", async () => {
|
||||
const store = fileStore({ rootDir: root, maxBytes: 1024 });
|
||||
await store.write("media/a.txt", "hello");
|
||||
await expect(store.readBytes("media/a.txt")).resolves.toEqual(Buffer.from("hello"));
|
||||
@ -175,7 +175,7 @@ describe("json store", () => {
|
||||
});
|
||||
|
||||
describe("secure file reads", () => {
|
||||
it("reads from a validated file handle", async () => {
|
||||
it.runIf(process.platform !== "win32")("reads from a validated file handle", async () => {
|
||||
const filePath = path.join(root, "secret.json");
|
||||
await fs.writeFile(filePath, '{"token":"ok"}', { mode: 0o600 });
|
||||
await fs.chmod(filePath, 0o600).catch(() => undefined);
|
||||
@ -190,6 +190,24 @@ describe("secure file reads", () => {
|
||||
expect(result.realPath).toBe(await fs.realpath(filePath));
|
||||
});
|
||||
|
||||
it.runIf(process.platform === "win32")(
|
||||
"fails closed on windows when ACL inspection is unavailable",
|
||||
async () => {
|
||||
// See src/secure-file.ts:177 — readSecureFile throws permission-unverified
|
||||
// on Windows because ACL inspection has no portable equivalent.
|
||||
const filePath = path.join(root, "secret.json");
|
||||
await fs.writeFile(filePath, '{"token":"ok"}', { mode: 0o600 });
|
||||
|
||||
await expect(
|
||||
readSecureFile({
|
||||
filePath,
|
||||
label: "test secret",
|
||||
io: { maxBytes: 1024 },
|
||||
}),
|
||||
).rejects.toMatchObject({ code: "permission-unverified" });
|
||||
},
|
||||
);
|
||||
|
||||
it("rejects symlinks and files outside trusted dirs", async () => {
|
||||
const trusted = path.join(root, "trusted");
|
||||
const outside = path.join(root, "outside");
|
||||
|
||||
@ -180,6 +180,18 @@ describe("Python helper configuration", () => {
|
||||
|
||||
describe("persistent Python helper worker", () => {
|
||||
it("reuses one worker and unreferences it while idle", async () => {
|
||||
if (process.platform === "win32") {
|
||||
configureFsSafePython({ mode: "auto", pythonPath: "/tmp/fake-python" });
|
||||
await expect(
|
||||
runPinnedPythonOperation<{ ok: boolean }>({
|
||||
operation: "stat",
|
||||
rootPath: "/tmp/root",
|
||||
payload: { relativePath: "a.txt" },
|
||||
}),
|
||||
).rejects.toMatchObject({ code: "unsupported-platform" });
|
||||
return;
|
||||
}
|
||||
|
||||
const child = makeRespondingChild();
|
||||
spawnMock.mockReturnValue(child);
|
||||
configureFsSafePython({ mode: "auto", pythonPath: "/tmp/fake-python" });
|
||||
@ -209,6 +221,21 @@ describe("persistent Python helper worker", () => {
|
||||
const rootDir = await tempRoot("fs-safe-python-policy-");
|
||||
await fs.writeFile(path.join(rootDir, "file.txt"), "ok");
|
||||
|
||||
if (process.platform === "win32") {
|
||||
spawnMock.mockImplementation(makeFailingChild);
|
||||
configureFsSafePython({ mode: "auto", pythonPath: "/tmp/missing-python" });
|
||||
const autoRoot = await root(rootDir);
|
||||
await expect(autoRoot.stat("file.txt")).rejects.toMatchObject({
|
||||
code: "unsupported-platform",
|
||||
});
|
||||
|
||||
configureFsSafePython({ mode: "require", pythonPath: "/tmp/missing-python" });
|
||||
await expect((await root(rootDir)).stat("file.txt")).rejects.toMatchObject({
|
||||
code: "unsupported-platform",
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
spawnMock.mockImplementation(makeFailingChild);
|
||||
configureFsSafePython({ mode: "auto", pythonPath: "/tmp/missing-python" });
|
||||
const autoRoot = await root(rootDir);
|
||||
|
||||
@ -38,64 +38,89 @@ afterEach(async () => {
|
||||
});
|
||||
|
||||
describe("pinned write fallback coverage", () => {
|
||||
it("writes buffers, creates only when missing, streams, and enforces limits", async () => {
|
||||
const { runPinnedWriteHelper } = await import("../src/pinned-write.js");
|
||||
const root = await tempRoot("fs-safe-pinned-write-fallback-");
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"writes buffers, creates only when missing, streams, and enforces limits",
|
||||
async () => {
|
||||
const { runPinnedWriteHelper } = await import("../src/pinned-write.js");
|
||||
const root = await tempRoot("fs-safe-pinned-write-fallback-");
|
||||
|
||||
const created = await runPinnedWriteHelper({
|
||||
rootPath: root,
|
||||
relativeParentPath: "nested",
|
||||
basename: "created.txt",
|
||||
mkdir: true,
|
||||
mode: 0o600,
|
||||
overwrite: false,
|
||||
input: { kind: "buffer", data: "created", encoding: "utf8" },
|
||||
});
|
||||
expect(created.ino).toBeGreaterThan(0);
|
||||
await expect(fs.readFile(path.join(root, "nested", "created.txt"), "utf8")).resolves.toBe(
|
||||
"created",
|
||||
);
|
||||
await expect(
|
||||
runPinnedWriteHelper({
|
||||
const created = await runPinnedWriteHelper({
|
||||
rootPath: root,
|
||||
relativeParentPath: "nested",
|
||||
basename: "created.txt",
|
||||
mkdir: true,
|
||||
mode: 0o600,
|
||||
overwrite: false,
|
||||
input: { kind: "buffer", data: "again" },
|
||||
}),
|
||||
).rejects.toMatchObject({ code: "EEXIST" });
|
||||
input: { kind: "buffer", data: "created", encoding: "utf8" },
|
||||
});
|
||||
expect(created.ino).toBeGreaterThan(0);
|
||||
await expect(fs.readFile(path.join(root, "nested", "created.txt"), "utf8")).resolves.toBe(
|
||||
"created",
|
||||
);
|
||||
await expect(
|
||||
runPinnedWriteHelper({
|
||||
rootPath: root,
|
||||
relativeParentPath: "nested",
|
||||
basename: "created.txt",
|
||||
mkdir: true,
|
||||
mode: 0o600,
|
||||
overwrite: false,
|
||||
input: { kind: "buffer", data: "again" },
|
||||
}),
|
||||
).rejects.toMatchObject({ code: "EEXIST" });
|
||||
|
||||
const streamed = await runPinnedWriteHelper({
|
||||
rootPath: root,
|
||||
relativeParentPath: "nested",
|
||||
basename: "streamed.txt",
|
||||
mkdir: true,
|
||||
mode: 0o600,
|
||||
overwrite: true,
|
||||
maxBytes: 16,
|
||||
input: { kind: "stream", stream: Readable.from(["stream", "ed"]) },
|
||||
});
|
||||
expect(streamed.dev).toBeGreaterThan(0);
|
||||
await expect(fs.readFile(path.join(root, "nested", "streamed.txt"), "utf8")).resolves.toBe(
|
||||
"streamed",
|
||||
);
|
||||
|
||||
await expect(
|
||||
runPinnedWriteHelper({
|
||||
const streamed = await runPinnedWriteHelper({
|
||||
rootPath: root,
|
||||
relativeParentPath: "nested",
|
||||
basename: "too-large.txt",
|
||||
basename: "streamed.txt",
|
||||
mkdir: true,
|
||||
mode: 0o600,
|
||||
overwrite: true,
|
||||
maxBytes: 2,
|
||||
input: { kind: "buffer", data: Buffer.from("large") },
|
||||
}),
|
||||
).rejects.toMatchObject({ code: "too-large" });
|
||||
await expect(fs.stat(path.join(root, "nested", "too-large.txt"))).rejects.toMatchObject({
|
||||
code: "ENOENT",
|
||||
});
|
||||
});
|
||||
maxBytes: 16,
|
||||
input: { kind: "stream", stream: Readable.from(["stream", "ed"]) },
|
||||
});
|
||||
expect(streamed.dev).toBeGreaterThan(0);
|
||||
await expect(fs.readFile(path.join(root, "nested", "streamed.txt"), "utf8")).resolves.toBe(
|
||||
"streamed",
|
||||
);
|
||||
|
||||
await expect(
|
||||
runPinnedWriteHelper({
|
||||
rootPath: root,
|
||||
relativeParentPath: "nested",
|
||||
basename: "too-large.txt",
|
||||
mkdir: true,
|
||||
mode: 0o600,
|
||||
overwrite: true,
|
||||
maxBytes: 2,
|
||||
input: { kind: "buffer", data: Buffer.from("large") },
|
||||
}),
|
||||
).rejects.toMatchObject({ code: "too-large" });
|
||||
await expect(fs.stat(path.join(root, "nested", "too-large.txt"))).rejects.toMatchObject({
|
||||
code: "ENOENT",
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.runIf(process.platform === "win32")(
|
||||
"rejects with unsupported-platform on windows",
|
||||
async () => {
|
||||
// fd-relative pinned filesystem operations are unavailable on windows
|
||||
// (see src/pinned-python.ts), so the helper fails closed before any
|
||||
// posix-only logic runs.
|
||||
const { runPinnedWriteHelper } = await import("../src/pinned-write.js");
|
||||
const root = await tempRoot("fs-safe-pinned-write-fallback-");
|
||||
await expect(
|
||||
runPinnedWriteHelper({
|
||||
rootPath: root,
|
||||
relativeParentPath: "nested",
|
||||
basename: "created.txt",
|
||||
mkdir: true,
|
||||
mode: 0o600,
|
||||
overwrite: false,
|
||||
input: { kind: "buffer", data: "created", encoding: "utf8" },
|
||||
}),
|
||||
).rejects.toMatchObject({ code: "unsupported-platform" });
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@ -72,11 +72,15 @@ 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"]);
|
||||
expectFsSafeCode(error, ["outside-workspace", "invalid-path", "path-alias"], {
|
||||
allowUnsupportedPlatformOnWindows: true,
|
||||
});
|
||||
return true;
|
||||
});
|
||||
await expect(safeRoot.list(".." as string)).rejects.toSatisfy((error: unknown) => {
|
||||
expectFsSafeCode(error, ["outside-workspace", "invalid-path", "path-alias"]);
|
||||
expectFsSafeCode(error, ["outside-workspace", "invalid-path", "path-alias"], {
|
||||
allowUnsupportedPlatformOnWindows: true,
|
||||
});
|
||||
return true;
|
||||
});
|
||||
await expect(scope.files(["../secret.txt"])).resolves.toMatchObject({ ok: false });
|
||||
@ -96,11 +100,15 @@ 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"]);
|
||||
expectFsSafeCode(error, ["outside-workspace", "path-alias", "symlink"], {
|
||||
allowUnsupportedPlatformOnWindows: true,
|
||||
});
|
||||
return true;
|
||||
});
|
||||
await expect(safeRoot.list("link")).rejects.toSatisfy((error: unknown) => {
|
||||
expectFsSafeCode(error, ["outside-workspace", "path-alias", "symlink"]);
|
||||
expectFsSafeCode(error, ["outside-workspace", "path-alias", "symlink"], {
|
||||
allowUnsupportedPlatformOnWindows: true,
|
||||
});
|
||||
return true;
|
||||
});
|
||||
});
|
||||
@ -120,7 +128,9 @@ 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"]);
|
||||
expectFsSafeCode(error, ["outside-workspace", "path-alias", "symlink"], {
|
||||
allowUnsupportedPlatformOnWindows: true,
|
||||
});
|
||||
return true;
|
||||
});
|
||||
|
||||
@ -168,7 +178,9 @@ 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"]);
|
||||
expectFsSafeCode(error, ["outside-workspace", "path-alias", "invalid-path"], {
|
||||
allowUnsupportedPlatformOnWindows: true,
|
||||
});
|
||||
return true;
|
||||
});
|
||||
|
||||
|
||||
@ -1,4 +1,5 @@
|
||||
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";
|
||||
@ -173,7 +174,10 @@ 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");
|
||||
const uploadsReal = await fs.realpath(uploadsDir);
|
||||
// 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);
|
||||
|
||||
expect(
|
||||
resolveLocalPathFromRootsSync({
|
||||
@ -182,7 +186,7 @@ describe("local roots helpers", () => {
|
||||
label: "media roots",
|
||||
requireFile: true,
|
||||
}),
|
||||
).toEqual({ path: await fs.realpath(filePath), root: uploadsReal });
|
||||
).toEqual({ path: realpathSync(filePath), root: uploadsReal });
|
||||
expect(
|
||||
resolveLocalPathFromRootsSync({
|
||||
filePath: missingPath,
|
||||
|
||||
@ -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 { root as openRoot } from "../src/index.js";
|
||||
import { configureFsSafePython, root as openRoot } from "../src/index.js";
|
||||
import {
|
||||
ESCAPING_DIRECTORY_PAYLOADS,
|
||||
ESCAPING_WRITE_PAYLOADS,
|
||||
@ -14,7 +14,6 @@ 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";
|
||||
|
||||
@ -25,6 +24,7 @@ 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 })));
|
||||
});
|
||||
|
||||
@ -168,6 +168,17 @@ describe("write, move, and delete boundary bypass attempts", () => {
|
||||
await fsp.symlink(layout.outsideFile, path.join(layout.root, "dest-link.txt"), "file");
|
||||
const safeRoot = await openRoot(layout.root);
|
||||
|
||||
if (process.platform === "win32") {
|
||||
await expect(safeRoot.move("source-link.txt", "moved.txt")).rejects.toMatchObject({
|
||||
code: "unsupported-platform",
|
||||
});
|
||||
await expect(safeRoot.move("from.txt", "dest-link.txt", { overwrite: true })).rejects.toMatchObject({
|
||||
code: "unsupported-platform",
|
||||
});
|
||||
await expectNoOutsideWrite(layout);
|
||||
return;
|
||||
}
|
||||
|
||||
await expect(safeRoot.move("source-link.txt", "moved.txt")).rejects.toBeTruthy();
|
||||
await safeRoot.move("from.txt", "dest-link.txt", { overwrite: true });
|
||||
await expectNoOutsideWrite(layout);
|
||||
@ -186,6 +197,8 @@ 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);
|
||||
@ -202,9 +215,6 @@ 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();
|
||||
}
|
||||
@ -222,8 +232,29 @@ describe("write, move, and delete boundary bypass attempts", () => {
|
||||
}
|
||||
for (const payload of LITERAL_SUSPICIOUS_DIRECTORY_PAYLOADS) {
|
||||
await safeRoot.mkdir(payload);
|
||||
await expect(safeRoot.list(payload), `list literal ${payload}`).resolves.toBeInstanceOf(Array);
|
||||
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 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);
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user