admin/Moltbot
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

refactor(security): centralize path guard helpers

Browse Source
This commit is contained in:
Peter Steinberger
2026-02-21 19:54:19 +01:00
parent 6ffca36284
commit ed960ba4eb
5 changed files with 178 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
src/agents/sandbox-paths.ts
View File

@@ -2,6 +2,7 @@ import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { fileURLToPath } from "node:url";
import { isNotFoundPathError, isPathInside } from "../infra/path-guards.js";
const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g;
const HTTP_URL_RE = /^https?:\/\//i;
@@ -129,8 +130,7 @@ async function assertNoSymlinkEscape(
current = target;
}
} catch (err) {
const anyErr = err as { code?: string };
if (anyErr.code === "ENOENT") {
if (isNotFoundPathError(err)) {
return;
}
throw err;
@@ -146,14 +146,6 @@ async function tryRealpath(value: string): Promise<string> {
}
}
function isPathInside(root: string, target: string): boolean {
const relative = path.relative(root, target);
if (!relative || relative === "") {
return true;
}
return !(relative.startsWith("..") || path.isAbsolute(relative));
}
function shortPath(value: string) {
if (value.startsWith(os.homedir())) {
return `~${value.slice(os.homedir().length)}`;

5
src/infra/archive.test.ts
View File

@@ -4,6 +4,7 @@ import path from "node:path";
import JSZip from "jszip";
import * as tar from "tar";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import type { ArchiveSecurityError } from "./archive.js";
import { extractArchive, resolveArchiveKind, resolvePackedRootDir } from "./archive.js";
let fixtureRoot = "";
@@ -95,7 +96,9 @@ describe("archive utils", () => {
await expect(
extractArchive({ archivePath, destDir: extractDir, timeoutMs: 5_000 }),
).rejects.toThrow(/symlink/i);
).rejects.toMatchObject({
code: "destination-symlink-traversal",
} satisfies Partial<ArchiveSecurityError>);
const outsideFile = path.join(outsideDir, "pwn.txt");
const outsideExists = await fs

207
src/infra/archive.ts
View File

@@ -10,6 +10,7 @@ import {
stripArchivePath,
validateArchiveEntryPath,
} from "./archive-path.js";
import { isNotFoundPathError, isPathInside, isSymlinkOpenError } from "./path-guards.js";
export type ArchiveKind = "tar" | "zip";
@@ -32,6 +33,21 @@ export type ArchiveExtractLimits = {
maxEntryBytes?: number;
};
export type ArchiveSecurityErrorCode =
| "destination-not-directory"
| "destination-symlink"
| "destination-symlink-traversal";
export class ArchiveSecurityError extends Error {
code: ArchiveSecurityErrorCode;
constructor(code: ArchiveSecurityErrorCode, message: string, options?: ErrorOptions) {
super(message, options);
this.code = code;
this.name = "ArchiveSecurityError";
}
}
/** @internal */
export const DEFAULT_MAX_ARCHIVE_BYTES_ZIP = 256 * 1024 * 1024;
/** @internal */
@@ -196,43 +212,27 @@ function createExtractBudgetTransform(params: {
});
}
function isNodeError(value: unknown): value is NodeJS.ErrnoException {
return Boolean(
value && typeof value === "object" && "code" in (value as Record<string, unknown>),
function symlinkTraversalError(originalPath: string): ArchiveSecurityError {
return new ArchiveSecurityError(
"destination-symlink-traversal",
`${ERROR_ARCHIVE_ENTRY_TRAVERSES_SYMLINK}: ${originalPath}`,
);
}
function isNotFoundError(value: unknown): boolean {
return isNodeError(value) && (value.code === "ENOENT" || value.code === "ENOTDIR");
}
function isSymlinkOpenError(value: unknown): boolean {
return (
isNodeError(value) &&
(value.code === "ELOOP" || value.code === "EINVAL" || value.code === "ENOTSUP")
);
}
function symlinkTraversalError(originalPath: string): Error {
return new Error(`${ERROR_ARCHIVE_ENTRY_TRAVERSES_SYMLINK}: ${originalPath}`);
}
async function assertDestinationDirReady(destDir: string): Promise<string> {
const stat = await fs.lstat(destDir);
if (stat.isSymbolicLink()) {
throw new Error("archive destination is a symlink");
throw new ArchiveSecurityError("destination-symlink", "archive destination is a symlink");
}
if (!stat.isDirectory()) {
throw new Error("archive destination is not a directory");
throw new ArchiveSecurityError(
"destination-not-directory",
"archive destination is not a directory",
);
}
return await fs.realpath(destDir);
}
function pathInside(root: string, target: string): boolean {
const rel = path.relative(root, target);
return rel === "" || (!rel.startsWith("..") && !path.isAbsolute(rel));
}
async function assertNoSymlinkTraversal(params: {
rootDir: string;
relPath: string;
@@ -246,7 +246,7 @@ async function assertNoSymlinkTraversal(params: {
try {
stat = await fs.lstat(current);
} catch (err) {
if (isNotFoundError(err)) {
if (isNotFoundPathError(err)) {
continue;
}
throw err;
@@ -266,12 +266,12 @@ async function assertResolvedInsideDestination(params: {
try {
resolved = await fs.realpath(params.targetPath);
} catch (err) {
if (isNotFoundError(err)) {
if (isNotFoundPathError(err)) {
return;
}
throw err;
}
if (!pathInside(params.destinationRealDir, resolved)) {
if (!isPathInside(params.destinationRealDir, resolved)) {
throw symlinkTraversalError(params.originalPath);
}
}
@@ -292,7 +292,7 @@ async function cleanupPartialRegularFile(filePath: string): Promise<void> {
try {
stat = await fs.lstat(filePath);
} catch (err) {
if (isNotFoundError(err)) {
if (isNotFoundPathError(err)) {
return;
}
throw err;
@@ -310,6 +310,8 @@ type ZipEntry = {
async: (type: "nodebuffer") => Promise<Buffer>;
};
type ZipExtractBudget = ReturnType<typeof createByteBudgetTracker>;
async function readZipEntryStream(entry: ZipEntry): Promise<NodeJS.ReadableStream> {
if (typeof entry.nodeStream === "function") {
return entry.nodeStream();
@@ -319,6 +321,90 @@ async function readZipEntryStream(entry: ZipEntry): Promise<NodeJS.ReadableStrea
return Readable.from(buf);
}
function resolveZipOutputPath(params: {
entryPath: string;
strip: number;
destinationDir: string;
}): { relPath: string; outPath: string } | null {
validateArchiveEntryPath(params.entryPath);
const relPath = stripArchivePath(params.entryPath, params.strip);
if (!relPath) {
return null;
}
validateArchiveEntryPath(relPath);
return {
relPath,
outPath: resolveArchiveOutputPath({
rootDir: params.destinationDir,
relPath,
originalPath: params.entryPath,
}),
};
}
async function prepareZipOutputPath(params: {
destinationDir: string;
destinationRealDir: string;
relPath: string;
outPath: string;
originalPath: string;
isDirectory: boolean;
}): Promise<void> {
await assertNoSymlinkTraversal({
rootDir: params.destinationDir,
relPath: params.relPath,
originalPath: params.originalPath,
});
if (params.isDirectory) {
await fs.mkdir(params.outPath, { recursive: true });
await assertResolvedInsideDestination({
destinationRealDir: params.destinationRealDir,
targetPath: params.outPath,
originalPath: params.originalPath,
});
return;
}
const parentDir = path.dirname(params.outPath);
await fs.mkdir(parentDir, { recursive: true });
await assertResolvedInsideDestination({
destinationRealDir: params.destinationRealDir,
targetPath: parentDir,
originalPath: params.originalPath,
});
}
async function writeZipFileEntry(params: {
entry: ZipEntry;
outPath: string;
budget: ZipExtractBudget;
}): Promise<void> {
const handle = await openZipOutputFile(params.outPath, params.entry.name);
params.budget.startEntry();
const readable = await readZipEntryStream(params.entry);
const writable = handle.createWriteStream();
try {
await pipeline(
readable,
createExtractBudgetTransform({ onChunkBytes: params.budget.addBytes }),
writable,
);
} catch (err) {
await cleanupPartialRegularFile(params.outPath).catch(() => undefined);
throw err;
}
// Best-effort permission restore for zip entries created on unix.
if (typeof params.entry.unixPermissions === "number") {
const mode = params.entry.unixPermissions & 0o777;
if (mode !== 0) {
await fs.chmod(params.outPath, mode).catch(() => undefined);
}
}
}
async function extractZip(params: {
archivePath: string;
destDir: string;
@@ -342,63 +428,32 @@ async function extractZip(params: {
const budget = createByteBudgetTracker(limits);
for (const entry of entries) {
validateArchiveEntryPath(entry.name);
const relPath = stripArchivePath(entry.name, strip);
if (!relPath) {
const output = resolveZipOutputPath({
entryPath: entry.name,
strip,
destinationDir: params.destDir,
});
if (!output) {
continue;
}
validateArchiveEntryPath(relPath);
const outPath = resolveArchiveOutputPath({
rootDir: params.destDir,
relPath,
originalPath: entry.name,
});
await assertNoSymlinkTraversal({
rootDir: params.destDir,
relPath,
await prepareZipOutputPath({
destinationDir: params.destDir,
destinationRealDir,
relPath: output.relPath,
outPath: output.outPath,
originalPath: entry.name,
isDirectory: entry.dir,
});
if (entry.dir) {
await fs.mkdir(outPath, { recursive: true });
await assertResolvedInsideDestination({
destinationRealDir,
targetPath: outPath,
originalPath: entry.name,
});
continue;
}
await fs.mkdir(path.dirname(outPath), { recursive: true });
await assertResolvedInsideDestination({
destinationRealDir,
targetPath: path.dirname(outPath),
originalPath: entry.name,
await writeZipFileEntry({
entry,
outPath: output.outPath,
budget,
});
const handle = await openZipOutputFile(outPath, entry.name);
budget.startEntry();
const readable = await readZipEntryStream(entry);
const writable = handle.createWriteStream();
try {
await pipeline(
readable,
createExtractBudgetTransform({ onChunkBytes: budget.addBytes }),
writable,
);
} catch (err) {
await cleanupPartialRegularFile(outPath).catch(() => undefined);
throw err;
}
// Best-effort permission restore for zip entries created on unix.
if (typeof entry.unixPermissions === "number") {
const mode = entry.unixPermissions & 0o777;
if (mode !== 0) {
await fs.chmod(outPath, mode).catch(() => undefined);
}
}
}
}

21
src/infra/fs-safe.ts
View File

@@ -3,6 +3,7 @@ import { constants as fsConstants } from "node:fs";
import type { FileHandle } from "node:fs/promises";
import fs from "node:fs/promises";
import path from "node:path";
import { isNotFoundPathError, isPathInside, isSymlinkOpenError } from "./path-guards.js";
export type SafeOpenErrorCode =
| "invalid-path"
@@ -34,27 +35,17 @@ export type SafeLocalReadResult = {
stat: Stats;
};
const NOT_FOUND_CODES = new Set(["ENOENT", "ENOTDIR"]);
const SUPPORTS_NOFOLLOW = process.platform !== "win32" && "O_NOFOLLOW" in fsConstants;
const OPEN_READ_FLAGS = fsConstants.O_RDONLY | (SUPPORTS_NOFOLLOW ? fsConstants.O_NOFOLLOW : 0);
const ensureTrailingSep = (value: string) => (value.endsWith(path.sep) ? value : value + path.sep);
const isNodeError = (err: unknown): err is NodeJS.ErrnoException =>
Boolean(err && typeof err === "object" && "code" in (err as Record<string, unknown>));
const isNotFoundError = (err: unknown) =>
isNodeError(err) && typeof err.code === "string" && NOT_FOUND_CODES.has(err.code);
const isSymlinkOpenError = (err: unknown) =>
isNodeError(err) && (err.code === "ELOOP" || err.code === "EINVAL" || err.code === "ENOTSUP");
async function openVerifiedLocalFile(filePath: string): Promise<SafeOpenResult> {
let handle: FileHandle;
try {
handle = await fs.open(filePath, OPEN_READ_FLAGS);
} catch (err) {
if (isNotFoundError(err)) {
if (isNotFoundPathError(err)) {
throw new SafeOpenError("not-found", "file not found");
}
if (isSymlinkOpenError(err)) {
@@ -87,7 +78,7 @@ async function openVerifiedLocalFile(filePath: string): Promise<SafeOpenResult>
if (err instanceof SafeOpenError) {
throw err;
}
if (isNotFoundError(err)) {
if (isNotFoundPathError(err)) {
throw new SafeOpenError("not-found", "file not found");
}
throw err;
@@ -102,14 +93,14 @@ export async function openFileWithinRoot(params: {
try {
rootReal = await fs.realpath(params.rootDir);
} catch (err) {
if (isNotFoundError(err)) {
if (isNotFoundPathError(err)) {
throw new SafeOpenError("not-found", "root dir not found");
}
throw err;
}
const rootWithSep = ensureTrailingSep(rootReal);
const resolved = path.resolve(rootWithSep, params.relativePath);
if (!resolved.startsWith(rootWithSep)) {
if (!isPathInside(rootWithSep, resolved)) {
throw new SafeOpenError("invalid-path", "path escapes root");
}
@@ -128,7 +119,7 @@ export async function openFileWithinRoot(params: {
throw err;
}
if (!opened.realPath.startsWith(rootWithSep)) {
if (!isPathInside(rootWithSep, opened.realPath)) {
await opened.handle.close().catch(() => {});
throw new SafeOpenError("invalid-path", "path escapes root");
}

35
src/infra/path-guards.ts Normal file
View File

@@ -0,0 +1,35 @@
import path from "node:path";
const NOT_FOUND_CODES = new Set(["ENOENT", "ENOTDIR"]);
const SYMLINK_OPEN_CODES = new Set(["ELOOP", "EINVAL", "ENOTSUP"]);
export function isNodeError(value: unknown): value is NodeJS.ErrnoException {
return Boolean(
value && typeof value === "object" && "code" in (value as Record<string, unknown>),
);
}
export function hasNodeErrorCode(value: unknown, code: string): boolean {
return isNodeError(value) && value.code === code;
}
export function isNotFoundPathError(value: unknown): boolean {
return isNodeError(value) && typeof value.code === "string" && NOT_FOUND_CODES.has(value.code);
}
export function isSymlinkOpenError(value: unknown): boolean {
return isNodeError(value) && typeof value.code === "string" && SYMLINK_OPEN_CODES.has(value.code);
}
export function isPathInside(root: string, target: string): boolean {
const resolvedRoot = path.resolve(root);
const resolvedTarget = path.resolve(target);
if (process.platform === "win32") {
const relative = path.win32.relative(resolvedRoot.toLowerCase(), resolvedTarget.toLowerCase());
return relative === "" || (!relative.startsWith("..") && !path.win32.isAbsolute(relative));
}
const relative = path.relative(resolvedRoot, resolvedTarget);
return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative));
}