refactor: split root internals and security test helpers
This commit is contained in:
parent
f0d5fe8ee6
commit
cc0757aa82
@ -145,6 +145,22 @@ it("writes and reads through the boundary", async () => {
|
||||
|
||||
For tests that need a private temp workspace, [`withTempWorkspace`](temp.md) makes the setup-and-teardown story trivial.
|
||||
|
||||
## Repo test shards
|
||||
|
||||
Run the full local gate before handoff:
|
||||
|
||||
```sh
|
||||
pnpm check
|
||||
```
|
||||
|
||||
Run only the security boundary corpus while iterating on root/path/archive/temp hardening:
|
||||
|
||||
```sh
|
||||
pnpm test:security
|
||||
```
|
||||
|
||||
`pnpm check` also runs `pnpm lint:file-size`. New source and test files should stay under 500 lines. Existing larger files have explicit budgets in `scripts/check-file-size.mjs`; do not increase those budgets as part of unrelated work.
|
||||
|
||||
## See also
|
||||
|
||||
- [Security model](security-model.md) — what the boundary is supposed to defend; design tests around the same threats.
|
||||
|
||||
@ -95,10 +95,12 @@
|
||||
"scripts": {
|
||||
"benchmark": "node scripts/benchmark.mjs",
|
||||
"build": "tsc -p tsconfig.json",
|
||||
"lint:file-size": "node scripts/check-file-size.mjs",
|
||||
"prepack": "node scripts/prepack-build.mjs",
|
||||
"test": "vitest run",
|
||||
"test:coverage": "vitest run --coverage",
|
||||
"check": "pnpm build && pnpm test",
|
||||
"test:security": "vitest run test/fs-safe.test.ts test/openclaw-read-bypass-parity.test.ts test/openclaw-write-bypass-parity.test.ts test/additional-bypass-parity.test.ts",
|
||||
"check": "pnpm lint:file-size && pnpm build && pnpm test",
|
||||
"docs:site": "node scripts/build-docs-site.mjs"
|
||||
},
|
||||
"optionalDependencies": {
|
||||
|
||||
46
scripts/check-file-size.mjs
Normal file
46
scripts/check-file-size.mjs
Normal file
@ -0,0 +1,46 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
|
||||
const DEFAULT_MAX_LINES = 500;
|
||||
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-path.ts", 862],
|
||||
["test/api-coverage.test.ts", 982],
|
||||
["test/new-primitives.test.ts", 998],
|
||||
]);
|
||||
|
||||
function walk(dir) {
|
||||
const entries = fs.readdirSync(dir, { withFileTypes: true });
|
||||
return entries.flatMap((entry) => {
|
||||
const fullPath = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
return walk(fullPath);
|
||||
}
|
||||
return fullPath.endsWith(".ts") ? [fullPath] : [];
|
||||
});
|
||||
}
|
||||
|
||||
const rootDir = process.cwd();
|
||||
const files = [...walk("src"), ...walk("test")].sort();
|
||||
const failures = [];
|
||||
|
||||
for (const file of files) {
|
||||
const normalized = file.split(path.sep).join("/");
|
||||
const text = fs.readFileSync(path.join(rootDir, file), "utf8");
|
||||
const lines = text.length === 0 ? 0 : text.split("\n").length;
|
||||
const budget = LINE_BUDGETS.get(normalized) ?? DEFAULT_MAX_LINES;
|
||||
if (lines > budget) {
|
||||
failures.push(`${normalized}: ${lines} lines > ${budget} budget`);
|
||||
}
|
||||
}
|
||||
|
||||
if (failures.length > 0) {
|
||||
console.error("File size budget exceeded:");
|
||||
for (const failure of failures) {
|
||||
console.error(`- ${failure}`);
|
||||
}
|
||||
process.exit(1);
|
||||
}
|
||||
80
src/root-context.ts
Normal file
80
src/root-context.ts
Normal file
@ -0,0 +1,80 @@
|
||||
import fs from "node:fs/promises";
|
||||
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";
|
||||
|
||||
export type RootContext = {
|
||||
rootDir: string;
|
||||
rootReal: string;
|
||||
rootWithSep: string;
|
||||
};
|
||||
|
||||
export const ensureTrailingSep = (value: string) =>
|
||||
value.endsWith(path.sep) ? value : value + path.sep;
|
||||
|
||||
export function assertValidRootRelativePath(relativePath: string): void {
|
||||
assertNoNulPathInput(relativePath, "relative path contains a NUL byte");
|
||||
}
|
||||
|
||||
let cachedHomePath: { raw: string; real: string } | undefined;
|
||||
|
||||
export async function expandRelativePathWithHome(relativePath: string): Promise<string> {
|
||||
const rawHome = process.env.HOME || process.env.USERPROFILE || os.homedir();
|
||||
if (cachedHomePath?.raw !== rawHome) {
|
||||
let realHome = rawHome;
|
||||
try {
|
||||
realHome = await fs.realpath(rawHome);
|
||||
} catch {
|
||||
// If the home dir cannot be canonicalized, keep lexical expansion behavior.
|
||||
}
|
||||
cachedHomePath = { raw: rawHome, real: realHome };
|
||||
}
|
||||
return expandHomePrefix(relativePath, { home: cachedHomePath.real });
|
||||
}
|
||||
|
||||
export async function resolveRootContext(rootDir: string): Promise<RootContext> {
|
||||
assertNoNulPathInput(rootDir, "root dir contains a NUL byte");
|
||||
let rootReal: string;
|
||||
try {
|
||||
rootReal = await fs.realpath(rootDir);
|
||||
const rootStat = await fs.stat(rootReal);
|
||||
if (!rootStat.isDirectory()) {
|
||||
throw new FsSafeError("invalid-path", "root dir is not a directory");
|
||||
}
|
||||
} catch (err) {
|
||||
if (err instanceof FsSafeError) {
|
||||
throw err;
|
||||
}
|
||||
if (isNotFoundPathError(err)) {
|
||||
throw new FsSafeError("not-found", "root dir not found");
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
return {
|
||||
rootDir: path.resolve(rootDir),
|
||||
rootReal,
|
||||
rootWithSep: ensureTrailingSep(rootReal),
|
||||
};
|
||||
}
|
||||
|
||||
export async function resolvePathInRoot(
|
||||
root: RootContext,
|
||||
relativePath: string,
|
||||
): Promise<{ rootReal: string; rootWithSep: string; resolved: string }> {
|
||||
assertValidRootRelativePath(relativePath);
|
||||
const expanded = await expandRelativePathWithHome(relativePath);
|
||||
const resolved = path.resolve(root.rootWithSep, expanded);
|
||||
if (!isPathInside(root.rootWithSep, resolved)) {
|
||||
throw new FsSafeError("outside-workspace", "file is outside workspace root");
|
||||
}
|
||||
return { rootReal: root.rootReal, rootWithSep: root.rootWithSep, resolved };
|
||||
}
|
||||
|
||||
export async function resolvePathWithinRoot(params: {
|
||||
rootDir: string;
|
||||
relativePath: string;
|
||||
}): Promise<{ rootReal: string; rootWithSep: string; resolved: string }> {
|
||||
return await resolvePathInRoot(await resolveRootContext(params.rootDir), params.relativePath);
|
||||
}
|
||||
24
src/root-errors.ts
Normal file
24
src/root-errors.ts
Normal file
@ -0,0 +1,24 @@
|
||||
import { FsSafeError } from "./errors.js";
|
||||
import { hasNodeErrorCode } from "./path.js";
|
||||
|
||||
export function isAlreadyExistsError(error: unknown): boolean {
|
||||
return hasNodeErrorCode(error, "EEXIST") || /File exists|EEXIST/i.test(String(error));
|
||||
}
|
||||
|
||||
export function normalizePinnedWriteError(error: unknown): Error {
|
||||
if (error instanceof FsSafeError) {
|
||||
return error;
|
||||
}
|
||||
return new FsSafeError("invalid-path", "path is not a regular file under root", {
|
||||
cause: error instanceof Error ? error : undefined,
|
||||
});
|
||||
}
|
||||
|
||||
export function normalizePinnedPathError(error: unknown): Error {
|
||||
if (error instanceof FsSafeError) {
|
||||
return error;
|
||||
}
|
||||
return new FsSafeError("path-alias", "path is not under root", {
|
||||
cause: error instanceof Error ? error : undefined,
|
||||
});
|
||||
}
|
||||
1743
src/root-impl.ts
Normal file
1743
src/root-impl.ts
Normal file
File diff suppressed because it is too large
Load Diff
1860
src/root.ts
1860
src/root.ts
File diff suppressed because it is too large
Load Diff
111
test/helpers/security.ts
Normal file
111
test/helpers/security.ts
Normal file
@ -0,0 +1,111 @@
|
||||
import fsp from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { expect } from "vitest";
|
||||
import { FsSafeError } from "../../src/index.js";
|
||||
|
||||
export type TempLayout = {
|
||||
outside: string;
|
||||
outsideFile: string;
|
||||
root: string;
|
||||
};
|
||||
|
||||
export const TRAVERSAL_PAYLOADS = [
|
||||
"../secret.txt",
|
||||
"../../secret.txt",
|
||||
"nested/../../secret.txt",
|
||||
"nested/../../../secret.txt",
|
||||
"./../secret.txt",
|
||||
"nested/..//../secret.txt",
|
||||
"nested/%2e%2e/secret.txt",
|
||||
"%2e%2e/secret.txt",
|
||||
"%2e%2e%2fsecret.txt",
|
||||
"..%2fsecret.txt",
|
||||
"%252e%252e%252fsecret.txt",
|
||||
"..%00/secret.txt",
|
||||
"..\\secret.txt",
|
||||
"nested\\..\\..\\secret.txt",
|
||||
"C:\\Windows\\win.ini",
|
||||
"\\\\server\\share\\secret.txt",
|
||||
] as const;
|
||||
|
||||
export const LIST_TRAVERSAL_PAYLOADS = [
|
||||
"..",
|
||||
"../",
|
||||
"../../",
|
||||
"nested/../..",
|
||||
"nested/../../outside",
|
||||
"%2e%2e",
|
||||
"%2e%2e%2f",
|
||||
"..\\",
|
||||
"C:\\Windows",
|
||||
"\\\\server\\share",
|
||||
] as const;
|
||||
|
||||
export const ESCAPING_WRITE_PAYLOADS = [
|
||||
"../pwned.txt",
|
||||
"../../pwned.txt",
|
||||
"nested/../../pwned.txt",
|
||||
"nested/../../../pwned.txt",
|
||||
"./../pwned.txt",
|
||||
"nested/..//../pwned.txt",
|
||||
] as const;
|
||||
|
||||
export const LITERAL_SUSPICIOUS_WRITE_PAYLOADS = [
|
||||
"nested/%2e%2e/pwned.txt",
|
||||
"%2e%2e/pwned.txt",
|
||||
"%2e%2e%2fpwned.txt",
|
||||
"%252e%252e%252fpwned.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",
|
||||
"..\\pwned.txt",
|
||||
] as const;
|
||||
|
||||
export const ESCAPING_DIRECTORY_PAYLOADS = [
|
||||
"..",
|
||||
"../",
|
||||
"../../",
|
||||
"nested/../..",
|
||||
"nested/../../outside",
|
||||
] as const;
|
||||
|
||||
export const LITERAL_SUSPICIOUS_DIRECTORY_PAYLOADS = ["%2e%2e", "%2e%2e%2f"] as const;
|
||||
export const SAFE_REJECTED_SUSPICIOUS_DIRECTORY_PAYLOADS = ["..\\"] as const;
|
||||
|
||||
export const WINDOWS_REJECTED_SUSPICIOUS_DIRECTORY_PAYLOADS = [
|
||||
"C:\\Windows",
|
||||
"\\\\server\\share",
|
||||
] as const;
|
||||
|
||||
export async function makeTempLayout(
|
||||
prefix: string,
|
||||
tempDirs: string[],
|
||||
): Promise<TempLayout> {
|
||||
const root = await fsp.mkdtemp(path.join(os.tmpdir(), `${prefix}-root-`));
|
||||
const outside = await fsp.mkdtemp(path.join(os.tmpdir(), `${prefix}-outside-`));
|
||||
tempDirs.push(root, outside);
|
||||
const outsideFile = path.join(outside, "secret.txt");
|
||||
await fsp.writeFile(outsideFile, "outside secret");
|
||||
return { outside, outsideFile, root };
|
||||
}
|
||||
|
||||
export function expectFsSafeCode(error: unknown, codes: readonly string[]): void {
|
||||
expect(error).toBeInstanceOf(FsSafeError);
|
||||
expect(codes).toContain((error as FsSafeError).code);
|
||||
}
|
||||
|
||||
export async function expectNoOutsideWrite(
|
||||
layout: TempLayout,
|
||||
expected = "outside secret",
|
||||
): Promise<void> {
|
||||
await expect(fsp.readFile(layout.outsideFile, "utf8")).resolves.toBe(expected);
|
||||
}
|
||||
@ -1,61 +1,23 @@
|
||||
import fs from "node:fs";
|
||||
import fsp from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { resolveAbsolutePathForRead } from "../src/absolute-path.js";
|
||||
import { FsSafeError, root as openRoot } from "../src/index.js";
|
||||
import { root as openRoot } from "../src/index.js";
|
||||
import { openPinnedFileSync } from "../src/pinned-open.js";
|
||||
import { pathScope } from "../src/root-paths.js";
|
||||
import { openRootFile, openRootFileSync } from "../src/root-file.js";
|
||||
|
||||
type TempLayout = {
|
||||
outside: string;
|
||||
outsideFile: string;
|
||||
root: string;
|
||||
};
|
||||
import {
|
||||
expectFsSafeCode,
|
||||
LIST_TRAVERSAL_PAYLOADS,
|
||||
makeTempLayout as makeSecurityTempLayout,
|
||||
TRAVERSAL_PAYLOADS,
|
||||
} from "./helpers/security.js";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
const TRAVERSAL_PAYLOADS = [
|
||||
"../secret.txt",
|
||||
"../../secret.txt",
|
||||
"nested/../../secret.txt",
|
||||
"nested/../../../secret.txt",
|
||||
"./../secret.txt",
|
||||
"nested/..//../secret.txt",
|
||||
"nested/%2e%2e/secret.txt",
|
||||
"%2e%2e/secret.txt",
|
||||
"%2e%2e%2fsecret.txt",
|
||||
"..%2fsecret.txt",
|
||||
"%252e%252e%252fsecret.txt",
|
||||
"..%00/secret.txt",
|
||||
"..\\secret.txt",
|
||||
"nested\\..\\..\\secret.txt",
|
||||
"C:\\Windows\\win.ini",
|
||||
"\\\\server\\share\\secret.txt",
|
||||
] as const;
|
||||
|
||||
const LIST_TRAVERSAL_PAYLOADS = [
|
||||
"..",
|
||||
"../",
|
||||
"../../",
|
||||
"nested/../..",
|
||||
"nested/../../outside",
|
||||
"%2e%2e",
|
||||
"%2e%2e%2f",
|
||||
"..\\",
|
||||
"C:\\Windows",
|
||||
"\\\\server\\share",
|
||||
] as const;
|
||||
|
||||
async function makeTempLayout(prefix: string): Promise<TempLayout> {
|
||||
const root = await fsp.mkdtemp(path.join(os.tmpdir(), `${prefix}-root-`));
|
||||
const outside = await fsp.mkdtemp(path.join(os.tmpdir(), `${prefix}-outside-`));
|
||||
tempDirs.push(root, outside);
|
||||
const outsideFile = path.join(outside, "secret.txt");
|
||||
await fsp.writeFile(outsideFile, "outside secret");
|
||||
return { outside, outsideFile, root };
|
||||
async function makeTempLayout(prefix: string) {
|
||||
return await makeSecurityTempLayout(prefix, tempDirs);
|
||||
}
|
||||
|
||||
async function closeIfOpen(value: unknown): Promise<void> {
|
||||
@ -67,11 +29,6 @@ async function closeIfOpen(value: unknown): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
function expectFsSafeCode(error: unknown, codes: readonly string[]): void {
|
||||
expect(error).toBeInstanceOf(FsSafeError);
|
||||
expect(codes).toContain((error as FsSafeError).code);
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(tempDirs.splice(0).map((dir) => fsp.rm(dir, { force: true, recursive: true })));
|
||||
});
|
||||
|
||||
@ -1,78 +1,25 @@
|
||||
import fsp from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { FsSafeError, root as openRoot } from "../src/index.js";
|
||||
|
||||
type TempLayout = {
|
||||
outside: string;
|
||||
outsideFile: string;
|
||||
root: string;
|
||||
};
|
||||
import { root as openRoot } from "../src/index.js";
|
||||
import {
|
||||
ESCAPING_DIRECTORY_PAYLOADS,
|
||||
ESCAPING_WRITE_PAYLOADS,
|
||||
expectFsSafeCode,
|
||||
expectNoOutsideWrite,
|
||||
LITERAL_SUSPICIOUS_DIRECTORY_PAYLOADS,
|
||||
LITERAL_SUSPICIOUS_WRITE_PAYLOADS,
|
||||
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";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
const ESCAPING_WRITE_PAYLOADS = [
|
||||
"../pwned.txt",
|
||||
"../../pwned.txt",
|
||||
"nested/../../pwned.txt",
|
||||
"nested/../../../pwned.txt",
|
||||
"./../pwned.txt",
|
||||
"nested/..//../pwned.txt",
|
||||
] as const;
|
||||
|
||||
const LITERAL_SUSPICIOUS_WRITE_PAYLOADS = [
|
||||
"nested/%2e%2e/pwned.txt",
|
||||
"%2e%2e/pwned.txt",
|
||||
"%2e%2e%2fpwned.txt",
|
||||
"%252e%252e%252fpwned.txt",
|
||||
] as const;
|
||||
|
||||
const POSIX_LITERAL_SUSPICIOUS_WRITE_PAYLOADS = [
|
||||
"nested\\..\\..\\pwned.txt",
|
||||
"C:\\Windows\\win.ini",
|
||||
"\\\\server\\share\\pwned.txt",
|
||||
] as const;
|
||||
|
||||
const SAFE_REJECTED_SUSPICIOUS_WRITE_PAYLOADS = [
|
||||
"..%2fpwned.txt",
|
||||
"..%00/pwned.txt",
|
||||
"..\\pwned.txt",
|
||||
] as const;
|
||||
|
||||
const ESCAPING_DIRECTORY_PAYLOADS = [
|
||||
"..",
|
||||
"../",
|
||||
"../../",
|
||||
"nested/../..",
|
||||
"nested/../../outside",
|
||||
] as const;
|
||||
|
||||
const LITERAL_SUSPICIOUS_DIRECTORY_PAYLOADS = ["%2e%2e", "%2e%2e%2f"] as const;
|
||||
|
||||
const SAFE_REJECTED_SUSPICIOUS_DIRECTORY_PAYLOADS = ["..\\"] as const;
|
||||
|
||||
const WINDOWS_REJECTED_SUSPICIOUS_DIRECTORY_PAYLOADS = [
|
||||
"C:\\Windows",
|
||||
"\\\\server\\share",
|
||||
] as const;
|
||||
|
||||
async function makeTempLayout(prefix: string): Promise<TempLayout> {
|
||||
const root = await fsp.mkdtemp(path.join(os.tmpdir(), `${prefix}-root-`));
|
||||
const outside = await fsp.mkdtemp(path.join(os.tmpdir(), `${prefix}-outside-`));
|
||||
tempDirs.push(root, outside);
|
||||
const outsideFile = path.join(outside, "secret.txt");
|
||||
await fsp.writeFile(outsideFile, "outside secret");
|
||||
return { outside, outsideFile, root };
|
||||
}
|
||||
|
||||
function expectFsSafeCode(error: unknown, codes: readonly string[]): void {
|
||||
expect(error).toBeInstanceOf(FsSafeError);
|
||||
expect(codes).toContain((error as FsSafeError).code);
|
||||
}
|
||||
|
||||
async function expectNoOutsideWrite(layout: TempLayout, expected = "outside secret"): Promise<void> {
|
||||
await expect(fsp.readFile(layout.outsideFile, "utf8")).resolves.toBe(expected);
|
||||
async function makeTempLayout(prefix: string) {
|
||||
return await makeSecurityTempLayout(prefix, tempDirs);
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user