mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 11:45:20 +02:00
refactor: extract path-security.ts shared module
validateOutputPath, validateReadPath, and SAFE_DIRECTORIES were duplicated across write-commands.ts, meta-commands.ts, and read-commands.ts. Extract to a single shared module with re-exports for backward compatibility. Also adds validateTempPath() for the upcoming GET /file endpoint (TEMP_DIR only, not cwd, to prevent remote agents from reading project files). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
@@ -8,48 +8,16 @@ import { getCleanText } from './read-commands';
|
||||
import { READ_COMMANDS, WRITE_COMMANDS, META_COMMANDS, PAGE_CONTENT_COMMANDS, wrapUntrustedContent } from './commands';
|
||||
import { validateNavigationUrl } from './url-validation';
|
||||
import { checkScope, type TokenInfo } from './token-registry';
|
||||
import { validateOutputPath, escapeRegExp } from './path-security';
|
||||
// Re-export for backward compatibility (tests import from meta-commands)
|
||||
export { validateOutputPath, escapeRegExp } from './path-security';
|
||||
import * as Diff from 'diff';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import { TEMP_DIR, isPathWithin } from './platform';
|
||||
import { TEMP_DIR } from './platform';
|
||||
import { resolveConfig } from './config';
|
||||
import type { Frame } from 'playwright';
|
||||
|
||||
// Security: Path validation to prevent path traversal attacks
|
||||
// Resolve safe directories through realpathSync to handle symlinks (e.g., macOS /tmp → /private/tmp)
|
||||
const SAFE_DIRECTORIES = [TEMP_DIR, process.cwd()].map(d => {
|
||||
try { return fs.realpathSync(d); } catch { return d; }
|
||||
});
|
||||
|
||||
export function validateOutputPath(filePath: string): void {
|
||||
const resolved = path.resolve(filePath);
|
||||
|
||||
// Resolve real path of the parent directory to catch symlinks.
|
||||
// The file itself may not exist yet (e.g., screenshot output).
|
||||
let dir = path.dirname(resolved);
|
||||
let realDir: string;
|
||||
try {
|
||||
realDir = fs.realpathSync(dir);
|
||||
} catch {
|
||||
try {
|
||||
realDir = fs.realpathSync(path.dirname(dir));
|
||||
} catch {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
}
|
||||
|
||||
const realResolved = path.join(realDir, path.basename(resolved));
|
||||
const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realResolved, dir));
|
||||
if (!isSafe) {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
}
|
||||
|
||||
/** Escape special regex metacharacters in a user-supplied string to prevent ReDoS. */
|
||||
export function escapeRegExp(s: string): string {
|
||||
return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
}
|
||||
|
||||
/** Tokenize a pipe segment respecting double-quoted strings. */
|
||||
function tokenizePipeSegment(segment: string): string[] {
|
||||
const tokens: string[] = [];
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
/**
|
||||
* Shared path validation — single source of truth for file path security.
|
||||
*
|
||||
* Previously duplicated across write-commands.ts, meta-commands.ts, and read-commands.ts.
|
||||
* All file I/O commands (screenshot, pdf, download, scrape, archive, eval) must
|
||||
* validate paths through these functions.
|
||||
*
|
||||
* validateOutputPath(path) — for writing files (screenshot, pdf, download, scrape, archive)
|
||||
* validateReadPath(path) — for reading files (eval)
|
||||
* validateTempPath(path) — for serving files to remote agents (GET /file, TEMP_DIR only)
|
||||
*
|
||||
* Security invariants:
|
||||
* 1. All paths resolved to absolute before checking
|
||||
* 2. Symlinks resolved to catch traversal via symlink inside safe dir
|
||||
* 3. SAFE_DIRECTORIES = [TEMP_DIR, cwd] for local commands
|
||||
* 4. TEMP_ONLY = [TEMP_DIR] for remote file serving (prevents project file exfil)
|
||||
*/
|
||||
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import { TEMP_DIR, isPathWithin } from './platform';
|
||||
|
||||
// Resolve safe directories through realpathSync to handle symlinks (e.g., macOS /tmp → /private/tmp)
|
||||
export const SAFE_DIRECTORIES = [TEMP_DIR, process.cwd()].map(d => {
|
||||
try { return fs.realpathSync(d); } catch { return d; }
|
||||
});
|
||||
|
||||
const TEMP_ONLY = [TEMP_DIR].map(d => {
|
||||
try { return fs.realpathSync(d); } catch { return d; }
|
||||
});
|
||||
|
||||
/** Validate a file path for writing (screenshot, pdf, download, scrape, archive). */
|
||||
export function validateOutputPath(filePath: string): void {
|
||||
const resolved = path.resolve(filePath);
|
||||
|
||||
// Resolve real path of the parent directory to catch symlinks.
|
||||
// The file itself may not exist yet (e.g., screenshot output).
|
||||
// This also handles macOS /tmp → /private/tmp transparently.
|
||||
let dir = path.dirname(resolved);
|
||||
let realDir: string;
|
||||
try {
|
||||
realDir = fs.realpathSync(dir);
|
||||
} catch {
|
||||
try {
|
||||
realDir = fs.realpathSync(path.dirname(dir));
|
||||
} catch {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
}
|
||||
|
||||
const realResolved = path.join(realDir, path.basename(resolved));
|
||||
const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realResolved, dir));
|
||||
if (!isSafe) {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
}
|
||||
|
||||
/** Validate a file path for reading (eval command). */
|
||||
export function validateReadPath(filePath: string): void {
|
||||
const resolved = path.resolve(filePath);
|
||||
let realPath: string;
|
||||
try {
|
||||
realPath = fs.realpathSync(resolved);
|
||||
} catch (err: any) {
|
||||
if (err.code === 'ENOENT') {
|
||||
try {
|
||||
const dir = fs.realpathSync(path.dirname(resolved));
|
||||
realPath = path.join(dir, path.basename(resolved));
|
||||
} catch {
|
||||
realPath = resolved;
|
||||
}
|
||||
} else {
|
||||
throw new Error(`Cannot resolve real path: ${filePath} (${err.code})`);
|
||||
}
|
||||
}
|
||||
const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realPath, dir));
|
||||
if (!isSafe) {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
}
|
||||
|
||||
/** Validate a file path for remote serving (GET /file). TEMP_DIR only, not cwd. */
|
||||
export function validateTempPath(filePath: string): void {
|
||||
const resolved = path.resolve(filePath);
|
||||
let realPath: string;
|
||||
try {
|
||||
realPath = fs.realpathSync(resolved);
|
||||
} catch (err: any) {
|
||||
if (err.code === 'ENOENT') {
|
||||
throw new Error('File not found');
|
||||
}
|
||||
throw new Error(`Cannot resolve path: ${filePath}`);
|
||||
}
|
||||
const isSafe = TEMP_ONLY.some(dir => isPathWithin(realPath, dir));
|
||||
if (!isSafe) {
|
||||
throw new Error(`Path must be within: ${TEMP_ONLY.join(', ')} (remote file serving is restricted to temp directory)`);
|
||||
}
|
||||
}
|
||||
|
||||
/** Escape special regex metacharacters in a user-supplied string to prevent ReDoS. */
|
||||
export function escapeRegExp(s: string): string {
|
||||
return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
}
|
||||
@@ -10,8 +10,11 @@ import { consoleBuffer, networkBuffer, dialogBuffer } from './buffers';
|
||||
import type { Page, Frame } from 'playwright';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import { TEMP_DIR, isPathWithin } from './platform';
|
||||
import { TEMP_DIR } from './platform';
|
||||
import { inspectElement, formatInspectorResult, getModificationHistory } from './cdp-inspector';
|
||||
import { validateReadPath } from './path-security';
|
||||
// Re-export for backward compatibility (tests import from read-commands)
|
||||
export { validateReadPath } from './path-security';
|
||||
|
||||
// Redaction patterns for sensitive cookie/storage values — exported for test coverage
|
||||
export const SENSITIVE_COOKIE_NAME = /(^|[_.-])(token|secret|key|password|credential|auth|jwt|session|csrf|sid)($|[_.-])|api.?key/i;
|
||||
@@ -41,38 +44,6 @@ function wrapForEvaluate(code: string): string {
|
||||
: `(async()=>(${trimmed}))()`;
|
||||
}
|
||||
|
||||
// Security: Path validation to prevent path traversal attacks
|
||||
// Resolve safe directories through realpathSync to handle symlinks (e.g., macOS /tmp → /private/tmp)
|
||||
const SAFE_DIRECTORIES = [TEMP_DIR, process.cwd()].map(d => {
|
||||
try { return fs.realpathSync(d); } catch { return d; }
|
||||
});
|
||||
|
||||
export function validateReadPath(filePath: string): void {
|
||||
// Always resolve to absolute first (fixes relative path symlink bypass)
|
||||
const resolved = path.resolve(filePath);
|
||||
// Resolve symlinks — throw on non-ENOENT errors
|
||||
let realPath: string;
|
||||
try {
|
||||
realPath = fs.realpathSync(resolved);
|
||||
} catch (err: any) {
|
||||
if (err.code === 'ENOENT') {
|
||||
// File doesn't exist — resolve directory part for symlinks (e.g., /tmp → /private/tmp)
|
||||
try {
|
||||
const dir = fs.realpathSync(path.dirname(resolved));
|
||||
realPath = path.join(dir, path.basename(resolved));
|
||||
} catch {
|
||||
realPath = resolved;
|
||||
}
|
||||
} else {
|
||||
throw new Error(`Cannot resolve real path: ${filePath} (${err.code})`);
|
||||
}
|
||||
}
|
||||
const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realPath, dir));
|
||||
if (!isSafe) {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract clean text from a page (strips script/style/noscript/svg).
|
||||
* Exported for DRY reuse in meta-commands (diff).
|
||||
|
||||
@@ -8,54 +8,12 @@
|
||||
import type { BrowserManager } from './browser-manager';
|
||||
import { findInstalledBrowsers, importCookies, listSupportedBrowserNames } from './cookie-import-browser';
|
||||
import { validateNavigationUrl } from './url-validation';
|
||||
import { validateOutputPath } from './path-security';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import { TEMP_DIR, isPathWithin } from './platform';
|
||||
import { TEMP_DIR } from './platform';
|
||||
import { modifyStyle, undoModification, resetModifications, getModificationHistory } from './cdp-inspector';
|
||||
|
||||
// Security: Path validation for screenshot output
|
||||
// Resolve safe directories through realpathSync to handle symlinks (e.g., macOS /tmp -> /private/tmp)
|
||||
const SAFE_DIRECTORIES = [TEMP_DIR, process.cwd()].map(d => {
|
||||
try { return fs.realpathSync(d); } catch { return d; }
|
||||
});
|
||||
|
||||
function validateOutputPath(filePath: string): void {
|
||||
const resolved = path.resolve(filePath);
|
||||
|
||||
// Basic containment check using lexical resolution only.
|
||||
// This catches obvious traversal (../../../etc/passwd) but NOT symlinks.
|
||||
const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(resolved, dir));
|
||||
if (!isSafe) {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
|
||||
// Symlink check: resolve the real path of the nearest existing ancestor
|
||||
// directory and re-validate. This closes the symlink bypass where a
|
||||
// symlink inside /tmp or cwd points outside the safe zone.
|
||||
//
|
||||
// We resolve the parent dir (not the file itself — it may not exist yet).
|
||||
// If the parent doesn't exist either we fall back up the tree.
|
||||
let dir = path.dirname(resolved);
|
||||
let realDir: string;
|
||||
try {
|
||||
realDir = fs.realpathSync(dir);
|
||||
} catch {
|
||||
// Parent doesn't exist — check the grandparent, or skip if inaccessible
|
||||
try {
|
||||
realDir = fs.realpathSync(path.dirname(dir));
|
||||
} catch {
|
||||
// Can't resolve — fail safe
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
}
|
||||
|
||||
const realResolved = path.join(realDir, path.basename(resolved));
|
||||
const isRealSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realResolved, dir));
|
||||
if (!isRealSafe) {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')} (symlink target blocked)`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Aggressive page cleanup selectors and heuristics.
|
||||
* Goal: make the page readable and clean while keeping it recognizable.
|
||||
|
||||
@@ -17,6 +17,7 @@ const WRITE_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/write-comma
|
||||
const SERVER_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/server.ts'), 'utf-8');
|
||||
const AGENT_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/sidebar-agent.ts'), 'utf-8');
|
||||
const SNAPSHOT_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/snapshot.ts'), 'utf-8');
|
||||
const PATH_SECURITY_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/path-security.ts'), 'utf-8');
|
||||
|
||||
// ─── Helper ─────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -159,26 +160,25 @@ describe('Task 2: CSS value validator blocks dangerous patterns', () => {
|
||||
|
||||
describe('Task 1: validateOutputPath uses realpathSync', () => {
|
||||
describe('source-level checks', () => {
|
||||
it('meta-commands.ts validateOutputPath contains realpathSync', () => {
|
||||
const fn = extractFunction(META_SRC, 'validateOutputPath');
|
||||
it('path-security.ts validateOutputPath contains realpathSync', () => {
|
||||
const fn = extractFunction(PATH_SECURITY_SRC, 'validateOutputPath');
|
||||
expect(fn).toBeTruthy();
|
||||
expect(fn).toContain('realpathSync');
|
||||
});
|
||||
|
||||
it('write-commands.ts validateOutputPath contains realpathSync', () => {
|
||||
const fn = extractFunction(WRITE_SRC, 'validateOutputPath');
|
||||
expect(fn).toBeTruthy();
|
||||
expect(fn).toContain('realpathSync');
|
||||
});
|
||||
|
||||
it('meta-commands.ts SAFE_DIRECTORIES resolves with realpathSync', () => {
|
||||
const safeBlock = sliceBetween(META_SRC, 'const SAFE_DIRECTORIES', ';');
|
||||
it('path-security.ts SAFE_DIRECTORIES resolves with realpathSync', () => {
|
||||
const safeBlock = sliceBetween(PATH_SECURITY_SRC, 'const SAFE_DIRECTORIES', ';');
|
||||
expect(safeBlock).toContain('realpathSync');
|
||||
});
|
||||
|
||||
it('write-commands.ts SAFE_DIRECTORIES resolves with realpathSync', () => {
|
||||
const safeBlock = sliceBetween(WRITE_SRC, 'const SAFE_DIRECTORIES', ';');
|
||||
expect(safeBlock).toContain('realpathSync');
|
||||
it('meta-commands.ts re-exports validateOutputPath from path-security', () => {
|
||||
expect(META_SRC).toContain("from './path-security'");
|
||||
expect(META_SRC).toContain('validateOutputPath');
|
||||
});
|
||||
|
||||
it('write-commands.ts imports validateOutputPath from path-security', () => {
|
||||
expect(WRITE_SRC).toContain("from './path-security'");
|
||||
expect(WRITE_SRC).toContain('validateOutputPath');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user