mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-08 06:26:45 +02:00
security: validate --from-file payload paths for parity with direct paths
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.
This commit is contained in:
@@ -8,7 +8,7 @@ import { getCleanText } from './read-commands';
|
||||
import { READ_COMMANDS, WRITE_COMMANDS, META_COMMANDS, PAGE_CONTENT_COMMANDS, wrapUntrustedContent, canonicalizeCommand } from './commands';
|
||||
import { validateNavigationUrl } from './url-validation';
|
||||
import { checkScope, type TokenInfo } from './token-registry';
|
||||
import { validateOutputPath, escapeRegExp } from './path-security';
|
||||
import { validateOutputPath, validateReadPath, SAFE_DIRECTORIES, 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';
|
||||
@@ -134,6 +134,17 @@ function parsePdfArgs(args: string[]): ParsedPdfArgs {
|
||||
}
|
||||
|
||||
function parsePdfFromFile(payloadPath: string): ParsedPdfArgs {
|
||||
// Parity with load-html --from-file (browse/src/write-commands.ts) and
|
||||
// the direct load-html <file> path: every caller-supplied file path
|
||||
// must pass validateReadPath so the safe-dirs policy can't be skirted
|
||||
// by routing reads through the --from-file shortcut.
|
||||
try {
|
||||
validateReadPath(path.resolve(payloadPath));
|
||||
} catch {
|
||||
throw new Error(
|
||||
`pdf: --from-file ${payloadPath} must be under ${SAFE_DIRECTORIES.join(' or ')} (security policy). Copy the payload into the project tree or /tmp first.`
|
||||
);
|
||||
}
|
||||
const raw = fs.readFileSync(payloadPath, 'utf8');
|
||||
const json = JSON.parse(raw);
|
||||
const out: ParsedPdfArgs = {
|
||||
|
||||
@@ -188,6 +188,19 @@ export async function handleWriteCommand(
|
||||
if (args[i] === '--from-file') {
|
||||
const payloadPath = args[++i];
|
||||
if (!payloadPath) throw new Error('load-html: --from-file requires a path');
|
||||
// Parity with the sibling `load-html <file>` path below (line 249):
|
||||
// that branch runs every `file://` target through validateReadPath
|
||||
// so the safe-dirs policy can't be side-stepped. Same policy must
|
||||
// apply here — otherwise --from-file becomes a read-anywhere escape
|
||||
// hatch for any caller that can pick the payload path (e.g., an
|
||||
// MCP caller issuing load-html with an attacker-influenced path).
|
||||
try {
|
||||
validateReadPath(path.resolve(payloadPath));
|
||||
} catch {
|
||||
throw new Error(
|
||||
`load-html: --from-file ${payloadPath} must be under ${SAFE_DIRECTORIES.join(' or ')} (security policy). Copy the payload into the project tree or /tmp first.`
|
||||
);
|
||||
}
|
||||
const raw = fs.readFileSync(payloadPath, 'utf8');
|
||||
let json: any;
|
||||
try { json = JSON.parse(raw); }
|
||||
|
||||
Reference in New Issue
Block a user