From 55fc8860877f09c1e321ee44aa741fd98b0bbbfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D0=BC=D0=B8=D1=82=D1=80=D0=B8=D0=B9?= Date: Fri, 29 May 2026 19:56:09 +0300 Subject: [PATCH] =?UTF-8?q?fix(router-gate):=20Stream=20C=20review=20?= =?UTF-8?q?=E2=80=94=20browser=5Fnavigate=20host-boundary=20(SSRF=20spoof?= =?UTF-8?q?=20guard)=20+=20boot-scan=20best-effort=20note?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tools/framework-boot-scanner.mjs | 6 ++++++ tools/framework-boot-scanner.test.mjs | 13 +++++++++++++ tools/mcp-tool-classifier.mjs | 8 ++++++-- tools/mcp-tool-classifier.test.mjs | 9 +++++++++ 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/tools/framework-boot-scanner.mjs b/tools/framework-boot-scanner.mjs index a5b19e04..395c94d9 100644 --- a/tools/framework-boot-scanner.mjs +++ b/tools/framework-boot-scanner.mjs @@ -120,6 +120,12 @@ export function intersectEditedBootFiles(projectType, editedFiles) { * Extract a PHP/JS-style method body by brace matching (char-scan, no regex on * the body, no backtracking). Returns the text between the method's opening { and * its matching }. Empty string if not found. + * + * NOTE (best-effort): the matcher is string/comment-naive — a `}` inside a string + * literal or comment closes the body early, so this can under-report. It is only + * used to scope the `findings` evidence for Model files; the security control is + * the intersection-based hard-block in decideBootScan, which fires regardless of + * findings. See the "hidden behind a } inside a string" test. */ export function extractPhpMethodBody(source, methodName) { if (typeof source !== 'string' || typeof methodName !== 'string') return ''; diff --git a/tools/framework-boot-scanner.test.mjs b/tools/framework-boot-scanner.test.mjs index 9f5c5dce..9203528e 100644 --- a/tools/framework-boot-scanner.test.mjs +++ b/tools/framework-boot-scanner.test.mjs @@ -149,4 +149,17 @@ describe('decideBootScan', () => { expect(r.block).toBe(true); expect(r.findings.some((f) => f.name === 'exec')).toBe(false); }); + it('still blocks even if a payload is hidden behind a } inside a string (findings are best-effort, block is the control)', () => { + const r = decideBootScan({ + command: 'php artisan test', + projectTypes: ['laravel'], + editedFiles: ['app/Models/Deal.php'], + readFile: readerFor({ 'app/Models/Deal.php': 'class Deal { public function boot() { $s = "}"; exec($x); } }' }), + }); + // The brace matcher is string/comment-naive, so the hidden exec may be absent + // from `findings` — but the intersection-based hard-block still fires, which is + // the actual security control. This test pins that guarantee. + expect(r.block).toBe(true); + expect(r.intersection).toContain('app/Models/Deal.php'); + }); }); diff --git a/tools/mcp-tool-classifier.mjs b/tools/mcp-tool-classifier.mjs index a929f59b..bf4a626a 100644 --- a/tools/mcp-tool-classifier.mjs +++ b/tools/mcp-tool-classifier.mjs @@ -59,8 +59,12 @@ export const DEFAULT_MCP_CLASSIFICATION = Object.freeze({ 'mcp__playwright__browser_navigate': { category: 'conditional', args_key_to_scan: 'url', - url_whitelist_patterns: ['^https?://(?:localhost|127\\.0\\.0\\.1|liderra\\.ru)'], - url_blocked_patterns: ['^https?://(?!localhost|127\\.|liderra)'], + // Host token MUST be followed by a port/path/query/fragment delimiter or end — + // otherwise a subdomain-suffix spoof (liderra.ru.evil.com / localhost.evil.com) + // slips past. (The v4.0 design §5.3 regex omitted this boundary; corrected here, + // spec to be synced in Stream H.) + url_whitelist_patterns: ['^https?://(?:localhost|127\\.0\\.0\\.1|liderra\\.ru)(?:[:/?#]|$)'], + url_blocked_patterns: ['^https?://(?!(?:localhost|127\\.0\\.0\\.1|liderra\\.ru)(?:[:/?#]|$))'], }, 'mcp__playwright__browser_click': { category: 'hard_blacklist' }, 'mcp__playwright__browser_fill_form': { category: 'hard_blacklist' }, diff --git a/tools/mcp-tool-classifier.test.mjs b/tools/mcp-tool-classifier.test.mjs index 893139e1..70e5252b 100644 --- a/tools/mcp-tool-classifier.test.mjs +++ b/tools/mcp-tool-classifier.test.mjs @@ -110,6 +110,15 @@ describe('classifyMcpTool — URL whitelist (WebFetch / browser_navigate)', () = expect(classifyMcpTool('mcp__playwright__browser_navigate', { url: 'http://localhost:8000' }).decision).toBe('allow'); expect(classifyMcpTool('mcp__playwright__browser_navigate', { url: 'http://evil.com' }).decision).toBe('block'); }); + it('blocks subdomain-suffix spoof of a whitelisted host (SSRF guard)', () => { + expect(classifyMcpTool('mcp__playwright__browser_navigate', { url: 'https://liderra.ru.evil.com/x' }).decision).toBe('block'); + expect(classifyMcpTool('mcp__playwright__browser_navigate', { url: 'http://localhost.evil.com/x' }).decision).toBe('block'); + expect(classifyMcpTool('mcp__playwright__browser_navigate', { url: 'http://127.0.0.1.evil.com/x' }).decision).toBe('block'); + }); + it('still allows genuine whitelisted hosts with port / path / query', () => { + expect(classifyMcpTool('mcp__playwright__browser_navigate', { url: 'https://liderra.ru/admin?x=1' }).decision).toBe('allow'); + expect(classifyMcpTool('mcp__playwright__browser_navigate', { url: 'http://127.0.0.1:5173' }).decision).toBe('allow'); + }); }); describe('classifyMcpTool — WebSearch llm-judge flag (G1)', () => {