mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-07 05:56:41 +02:00
gus
693eadf6f3
The direct `load-html <file>` path runs every caller-supplied file path
through validateReadPath() so reads stay confined to SAFE_DIRECTORIES
(cwd, TEMP_DIR). The `load-html --from-file <payload.json>` shortcut
and its sibling `pdf --from-file <payload.json>` skipped that check and
went straight to fs.readFileSync(). An MCP caller that picks the
payload path (or any caller whose payload argument is reachable from
attacker-influenced text) could use --from-file as a read-anywhere
escape hatch for the safe-dirs policy.
Fix: call validateReadPath(path.resolve(payloadPath)) before readFileSync
at both sites. Error surface mirrors the direct-path branch so ops and
agent errors stay consistent.
Test coverage in browse/test/from-file-path-validation.test.ts:
- source-level: validateReadPath precedes readFileSync in the load-html
--from-file branch (write-commands.ts) and the pdf --from-file parser
(meta-commands.ts)
- error-message parity: both sites reference SAFE_DIRECTORIES
Related security audit pattern: R3 F002 (validateNavigationUrl gap on
download/scrape) and R3 F008 (markHiddenElements gap on 10 DOM commands)
were the same shape — a defense that existed on the primary code path
but not its shortcut sibling. This PR closes the same class of gap on
the --from-file shortcuts.
69 lines
2.9 KiB
TypeScript
69 lines
2.9 KiB
TypeScript
/**
|
|
* Source-level guardrail for the --from-file shortcut flags.
|
|
*
|
|
* Context: both `load-html <file>` (write-commands.ts) and `pdf <url>`
|
|
* (meta-commands.ts) support a `--from-file <payload.json>` shortcut that
|
|
* reads a JSON payload with the inline content (HTML body / PDF options).
|
|
* The DIRECT `load-html <file>` path runs every caller-supplied file path
|
|
* through `validateReadPath()` so reads are confined to SAFE_DIRECTORIES.
|
|
* The `--from-file` paths historically skipped this validation, opening a
|
|
* parity gap: an MCP caller that can pick the payload path could route
|
|
* reads through --from-file to bypass the safe-dirs policy.
|
|
*
|
|
* This test inspects the source to make sure both --from-file sites call
|
|
* validateReadPath before fs.readFileSync. Pattern mirrors
|
|
* postgres-engine.test.ts and pglite-search-timeout.test.ts.
|
|
*/
|
|
|
|
import { describe, test, expect } from 'bun:test';
|
|
import { readFileSync } from 'fs';
|
|
import { join } from 'path';
|
|
|
|
const ROOT = join(import.meta.dir, '..', 'src');
|
|
const WRITE_SRC = readFileSync(join(ROOT, 'write-commands.ts'), 'utf-8');
|
|
const META_SRC = readFileSync(join(ROOT, 'meta-commands.ts'), 'utf-8');
|
|
|
|
function stripComments(s: string): string {
|
|
return s.replace(/\/\*[\s\S]*?\*\//g, '').replace(/(^|\s)\/\/[^\n]*/g, '$1');
|
|
}
|
|
|
|
describe('--from-file path validation parity', () => {
|
|
test('load-html --from-file validates payload path before reading', () => {
|
|
const stripped = stripComments(WRITE_SRC);
|
|
// Grab the --from-file branch body.
|
|
const idx = stripped.indexOf("'--from-file'");
|
|
expect(idx).toBeGreaterThan(-1);
|
|
const fromFileBranch = stripped.slice(idx, idx + 1200);
|
|
|
|
// validateReadPath must appear BEFORE the readFileSync in the branch.
|
|
const vIdx = fromFileBranch.indexOf('validateReadPath');
|
|
const rIdx = fromFileBranch.indexOf('readFileSync');
|
|
expect(vIdx).toBeGreaterThan(-1);
|
|
expect(rIdx).toBeGreaterThan(-1);
|
|
expect(vIdx).toBeLessThan(rIdx);
|
|
});
|
|
|
|
test('pdf --from-file validates payload path before reading', () => {
|
|
const stripped = stripComments(META_SRC);
|
|
const idx = stripped.indexOf('function parsePdfFromFile');
|
|
expect(idx).toBeGreaterThan(-1);
|
|
const fnBody = stripped.slice(idx, idx + 1200);
|
|
|
|
const vIdx = fnBody.indexOf('validateReadPath');
|
|
const rIdx = fnBody.indexOf('readFileSync');
|
|
expect(vIdx).toBeGreaterThan(-1);
|
|
expect(rIdx).toBeGreaterThan(-1);
|
|
expect(vIdx).toBeLessThan(rIdx);
|
|
});
|
|
|
|
test('both sites reference SAFE_DIRECTORIES in the error message', () => {
|
|
// Error shape parity so ops teams / agents see a consistent message.
|
|
const write = stripComments(WRITE_SRC);
|
|
const meta = stripComments(META_SRC);
|
|
// load-html --from-file error
|
|
expect(write).toMatch(/load-html: --from-file [\s\S]{0,80}SAFE_DIRECTORIES/);
|
|
// pdf --from-file error
|
|
expect(meta).toMatch(/pdf: --from-file [\s\S]{0,80}SAFE_DIRECTORIES/);
|
|
});
|
|
});
|