From 8997659333655ff55478b9b1a7275be4d53e6bd1 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Wed, 15 Jul 2026 12:35:09 +0200 Subject: [PATCH] feat(protect): hardening follow-ups from the security review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the gaps surfaced reviewing #79: - Body cap → partial-scan (was discard). An oversize body is now TRUNCATED to the cap and its prefix scanned (front-loaded payloads are caught), with memory bounded by a 4× hard ceiling; only a declared body over that ceiling is skipped. Content- Length is compared in bytes; the prefix slice is by char (can only over-scan). - IPv6 host extraction on the node:http egress path. extractHttpTarget no longer mangles `::1` / `[::1]` / `[::1]:port` via split(':') — normalizeHost preserves them so isInternalHost (which strips brackets) blocks IPv6-internal SSRF via options host. - Redact array-valued headers. Header screening now masks multi-valued Set-Cookie (via getSetCookie / node arrays) and re-emits each cookie separately, in both the fetch and node paths. - Multipart parser robustness: tolerate LF-only line endings and collect duplicate field names (arrays), matching the body/urlencoded behavior. - WebSocket egress screening: the guard now also wraps the global WebSocket so ws:// /wss:// egress is screened; restored on uninstall. - Verbose-error breadth: a default response rule redacts multi-language exception/ traceback signatures (Python/JVM/.NET/Go) beyond the node stack-trace + SQL rules. +6 tests (body-cap partial-scan incl. the past-cap residual, IPv6 node egress, Set-Cookie array redaction, multipart LF/dup, WebSocket block, exception redaction, allowHosts override). 443 tests pass, typecheck + build clean. Co-Authored-By: Claude Opus 4.8 --- src/protect/defaults.js | 11 ++++ src/protect/egress.js | 39 +++++++++++++- src/protect/engine/fetch.js | 26 +++++---- src/protect/runtime.js | 28 ++++++++-- tests/protect/egress-followups.test.ts | 68 ++++++++++++++++++++++++ tests/protect/fetch-body-cap.test.ts | 35 +++++++----- tests/protect/multipart.test.ts | 18 +++++++ tests/protect/response-hardening.test.ts | 24 +++++++++ 8 files changed, 219 insertions(+), 30 deletions(-) create mode 100644 tests/protect/egress-followups.test.ts diff --git a/src/protect/defaults.js b/src/protect/defaults.js index 0495fa4..9c542f2 100644 --- a/src/protect/defaults.js +++ b/src/protect/defaults.js @@ -63,6 +63,17 @@ export const DEFAULT_RESPONSE_RULES = [ category: 'info-exposure', action: 'redact', rule_v2: [{ parameter: 'response.body', match: { type: 'regex', value: '/(SQLSTATE\\[[0-9A-Z]+\\]|SequelizeDatabaseError|ER_[A-Z_]+|ORA-\\d{5}|PG::[A-Za-z]+Error|SQLITE_ERROR|You have an error in your SQL syntax)/i' } }] + }, + { + id: 'resp-exception-trace', + title: 'Backend exception / stack trace disclosure in response body', + phase: 'response', + category: 'info-exposure', + action: 'redact', + // Multi-language exception/traceback signatures a normal API response never carries: + // Python traceback, Java "Exception in thread", .NET System.*Exception, JVM stack frames, + // Go goroutine dumps. (Node `at fn (file:line:col)` frames are handled by resp-stack-trace.) + rule_v2: [{ parameter: 'response.body', match: { type: 'regex', value: '/(Traceback \\(most recent call last\\)|Exception in thread "|System\\.[A-Za-z.]+Exception|\\bat [\\w.$]+\\([\\w]+\\.(?:java|kt|scala|rb|py|cs):\\d+\\)|goroutine \\d+ \\[)/' } }] } ]; diff --git a/src/protect/egress.js b/src/protect/egress.js index ea69519..3cb0720 100644 --- a/src/protect/egress.js +++ b/src/protect/egress.js @@ -55,6 +55,32 @@ export async function installEgressGuard({ shouldBlock, onBlock } = {}) { } } + // 3. global WebSocket — ws:// / wss:// egress that never touches fetch or node:http. + const OriginalWS = globalThis.WebSocket; + if (typeof OriginalWS === 'function' && !OriginalWS.__patchstackGuarded) { + const GuardedWS = new Proxy(OriginalWS, { + construct(target, args, newTarget) { + const url = String(args?.[0] ?? ''); + let host = null; + try { + host = new URL(url).hostname; + } catch { + host = null; + } + if (block(url, host, 'WEBSOCKET')) { + throw new Error(`Patchstack blocked an outbound WebSocket to a disallowed address: ${host ?? url}`); + } + return Reflect.construct(target, args, newTarget); + }, + }); + OriginalWS.__patchstackGuarded = true; // marker on the original guards against double-wrap + globalThis.WebSocket = GuardedWS; + restores.push(() => { + if (globalThis.WebSocket === GuardedWS) globalThis.WebSocket = OriginalWS; + delete OriginalWS.__patchstackGuarded; + }); + } + return () => { for (const restore of restores) { try { @@ -102,7 +128,7 @@ function extractHttpTarget(args) { return { url: url.href, host: url.hostname, method: (opts && opts.method) || 'GET' }; } if (first && typeof first === 'object') { - const host = String(first.hostname || first.host || '').split(':')[0]; + const host = normalizeHost(first.hostname || first.host); const protocol = first.protocol || 'http:'; const port = first.port ? `:${first.port}` : ''; const path = first.path || '/'; @@ -113,3 +139,14 @@ function extractHttpTarget(args) { } return null; } + +// Extract the bare host from a node http(s) options `host`/`hostname`, WITHOUT mangling IPv6. +// `[::1]:8080` → `::1`, bare `::1`/`fe80::1` → unchanged, `example.com:443` → `example.com`. +// (isInternalHost strips brackets and matches ::1 / fe80: / fc / fd, so this must not corrupt them.) +function normalizeHost(raw) { + const host = String(raw || '').trim(); + const bracketed = /^\[([^\]]+)\]/.exec(host); + if (bracketed) return bracketed[1]; + if ((host.match(/:/g) || []).length > 1) return host; // bare IPv6 — no host:port to split + return host.split(':')[0]; +} diff --git a/src/protect/engine/fetch.js b/src/protect/engine/fetch.js index c9deaff..281281c 100644 --- a/src/protect/engine/fetch.js +++ b/src/protect/engine/fetch.js @@ -83,13 +83,16 @@ export async function fromFetchRequest(request, options = {}) { }; } -// Read a request body as text, but leave it UNSCANNED (fail-open) past `max` bytes so a huge -// upload can't be buffered for inspection. A declared oversize (Content-Length) is skipped before -// reading; anything else is read from a clone (so the downstream handler keeps an intact body) and -// discarded if it turns out over the cap. +// Read a request body as text for inspection. Memory is bounded by a hard ceiling (4× the scan +// cap): a body whose declared Content-Length exceeds it is left UNSCANNED (fail-open). Within the +// ceiling, an oversize body is TRUNCATED to `max` and its prefix is still scanned — so a +// front-loaded payload is caught — rather than being discarded outright. Reads a clone so the +// downstream handler keeps an intact body. (`max` is compared in bytes against Content-Length; the +// prefix slice is by character, which can only over-scan a multibyte body — the safe direction.) async function readCappedText(request, max) { + const ceiling = Math.max(max, max * 4); const declared = Number(request.headers?.get?.('content-length') || 0); - if (declared && declared > max) return ''; + if (declared && declared > ceiling) return ''; let clone; try { clone = request.clone(); @@ -98,7 +101,7 @@ async function readCappedText(request, max) { } try { const text = await clone.text(); - return text.length > max ? '' : text; + return text.length > max ? text.slice(0, max) : text; } catch { return ''; } @@ -111,17 +114,18 @@ function parseMultipart(rawBody, boundary) { const body = {}; const files = {}; for (const part of rawBody.split('--' + boundary)) { - const headerEnd = part.indexOf('\r\n\r\n'); - if (headerEnd === -1) continue; - const rawHeaders = part.slice(0, headerEnd); + // Tolerate both CRLF and LF-only line endings (some clients/proxies send bare \n). + const sep = /\r?\n\r?\n/.exec(part); + if (!sep) continue; + const rawHeaders = part.slice(0, sep.index); const disposition = /content-disposition:[^\r\n]*/i.exec(rawHeaders)?.[0]; if (!disposition) continue; const name = /name="([^"]*)"/i.exec(disposition)?.[1]; if (name == null) continue; - const content = part.slice(headerEnd + 4).replace(/\r\n$/, ''); + const content = part.slice(sep.index + sep[0].length).replace(/\r?\n$/, ''); const filename = /filename="([^"]*)"/i.exec(disposition)?.[1]; if (filename !== undefined) { - files[name] = filename; // engine resolves files. → the uploaded filename + files[name] = name in files ? [].concat(files[name], filename) : filename; } else { body[name] = name in body ? [].concat(body[name], content) : content; } diff --git a/src/protect/runtime.js b/src/protect/runtime.js index 09ef1af..6aed194 100644 --- a/src/protect/runtime.js +++ b/src/protect/runtime.js @@ -133,7 +133,13 @@ export async function createProtection(options = {}) { const mask = maskFn(rule.category); body = applyRedactors(body, redactors, mask); for (const name of Object.keys(headers)) { - if (typeof headers[name] === 'string') headers[name] = applyRedactors(headers[name], redactors, mask); + const value = headers[name]; + if (typeof value === 'string') { + headers[name] = applyRedactors(value, redactors, mask); + } else if (Array.isArray(value)) { + // Multi-valued headers (Set-Cookie) — redact each entry. + headers[name] = value.map((item) => (typeof item === 'string' ? applyRedactors(item, redactors, mask) : item)); + } } } return { verdict: 'redact', body, headers }; @@ -200,7 +206,9 @@ export async function createProtection(options = {}) { if (r.headers && res.setHeader) { const current = res.getHeaders ? res.getHeaders() : {}; for (const [name, value] of Object.entries(r.headers)) { - if (typeof value === 'string' && current[name] !== value) { + if (Array.isArray(value)) { + try { res.setHeader(name, value); } catch { /* ignore invalid header */ } // Set-Cookie array + } else if (typeof value === 'string' && current[name] !== value) { try { res.setHeader(name, value); } catch { /* ignore invalid header */ } } } @@ -380,6 +388,10 @@ async function readTextResponse(response) { function headerObject(headers) { const out = {}; headers?.forEach?.((v, k) => { out[k.toLowerCase()] = v; }); + // Set-Cookie is multi-valued; forEach collapses it. Recover the individual cookies so each can + // be screened (and re-emitted) separately. + const setCookies = headers?.getSetCookie?.(); + if (setCookies && setCookies.length) out['set-cookie'] = setCookies; return out; } @@ -426,8 +438,16 @@ function rebuildResponse(response, body, redactedHeaders) { headers.delete('content-length'); // body length changed after redaction if (redactedHeaders) { for (const [name, value] of Object.entries(redactedHeaders)) { - if (typeof value === 'string' && headers.get(name) !== value) { - try { headers.set(name, value); } catch { /* invalid header name — skip */ } + if (typeof value === 'string') { + if (headers.get(name) !== value) { + try { headers.set(name, value); } catch { /* invalid header name — skip */ } + } + } else if (Array.isArray(value)) { + // Re-emit each (possibly redacted) Set-Cookie separately (Headers collapses them otherwise). + try { + headers.delete(name); + for (const item of value) headers.append(name, String(item)); + } catch { /* skip */ } } } } diff --git a/tests/protect/egress-followups.test.ts b/tests/protect/egress-followups.test.ts new file mode 100644 index 0000000..5b9de6d --- /dev/null +++ b/tests/protect/egress-followups.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from 'vitest'; +import { createProtection } from '../../src/protect/runtime.js'; + +// Egress hardening follow-ups: IPv6 host handling on the node:http path (#2), WebSocket +// screening (#6), and the allowHosts allowlist overriding an internal-host block (#9). + +async function withEgress(opts: any, fn: (p: any) => Promise) { + const origFetch = globalThis.fetch; + globalThis.fetch = (async () => new Response('stub')) as any; + const p: any = await createProtection({ egress: true, mode: 'block', ...opts }); + try { + await fn(p); + } finally { + p.uninstallEgress?.(); + globalThis.fetch = origFetch; + } +} + +async function nodeHttp() { + const ns: any = await import('node:http'); + return ns.default ?? ns; +} + +describe('egress — node:http IPv6 hosts (#2)', () => { + it('blocks IPv6 loopback given as options host/hostname (not just as a URL)', async () => { + await withEgress({ allowHosts: [] }, async () => { + const http = await nodeHttp(); + const throws = (fn: () => void) => { + try { + fn(); + return false; + } catch (e) { + return /Patchstack blocked/.test(String(e)); + } + }; + expect(throws(() => http.request('http://[::1]/'))).toBe(true); // URL form + expect(throws(() => http.request({ host: '::1', path: '/' }))).toBe(true); // bare ::1 + expect(throws(() => http.request({ hostname: '[::1]', port: 8080, path: '/' }))).toBe(true); // bracketed + port + }); + }); +}); + +describe('egress — WebSocket screening (#6)', () => { + it('blocks a ws:// connection to an internal host and restores the global on uninstall', async () => { + if (typeof globalThis.WebSocket !== 'function') return; // runtime without global WebSocket + await withEgress({ allowHosts: [] }, async () => { + let blocked = false; + try { + // eslint-disable-next-line no-new + new WebSocket('ws://169.254.169.254/'); + } catch (e) { + blocked = /Patchstack blocked/.test(String(e)); + } + expect(blocked).toBe(true); + }); + // After uninstall the guard marker is gone (global restored). + expect((globalThis.WebSocket as any)?.__patchstackGuarded).toBeUndefined(); + }); +}); + +describe('egress — allowHosts overrides an internal block (#9)', () => { + it('permits a normally-internal host that is explicitly allowlisted', async () => { + await withEgress({ allowHosts: ['169.254.169.254'] }, async () => { + const res = await globalThis.fetch('http://169.254.169.254/latest/meta-data/'); + expect(await res.text()).toBe('stub'); // allowed → reached the stubbed fetch + }); + }); +}); diff --git a/tests/protect/fetch-body-cap.test.ts b/tests/protect/fetch-body-cap.test.ts index b955a1a..40bc6c4 100644 --- a/tests/protect/fetch-body-cap.test.ts +++ b/tests/protect/fetch-body-cap.test.ts @@ -11,30 +11,37 @@ const post = (body: string, ct = 'application/json', maxBodyBytes?: number) => maxBodyBytes ? { maxBodyBytes } : {}, ); +const rules = { + firewall: [{ id: 'proto', title: 'proto', rule_v2: [{ parameter: 'raw', match: { type: 'contains', value: '__proto__' } }] }], + whitelists: [], + whitelist_keys: {}, +}; +const jsonReq = (body: string) => + new Request('https://app/x', { method: 'POST', headers: { 'content-type': 'application/json' }, body }); + describe('fetch request-body cap', () => { - it('skips a body larger than the cap, keeps a small one', async () => { + it('truncates an oversize body to the cap and still scans the prefix', async () => { const overCap = await post('x'.repeat(200), 'text/plain', 32); - expect(overCap._rawBody).toBe(''); - expect(overCap.body).toEqual({}); + expect(overCap._rawBody).toBe('x'.repeat(32)); // prefix kept for scanning, not discarded const underCap = await post('small', 'text/plain', 32); expect(underCap._rawBody).toBe('small'); }); - it('does not block a malicious payload buried in an oversized body (fail-open)', async () => { - const rules = { - firewall: [{ id: 'proto', title: 'proto', rule_v2: [{ parameter: 'raw', match: { type: 'contains', value: '__proto__' } }] }], - whitelists: [], - whitelist_keys: {}, - }; + it('catches a front-loaded payload in an oversize body; a payload pushed past the cap still slips', async () => { const p = await createProtection({ rules, mode: 'block' }); const guard = p.fetchGuard(); - const huge = '{"__proto__":{"x":1},"pad":"' + 'a'.repeat(1024 * 1024 + 64) + '"}'; - const oversized = await guard(new Request('https://app/x', { method: 'POST', headers: { 'content-type': 'application/json' }, body: huge })); - expect(oversized).toBeNull(); // over the 1 MiB cap → body unscanned → allowed + // __proto__ at the front → within the scanned first 1 MiB → blocked. + const frontLoaded = '{"__proto__":{"x":1},"pad":"' + 'a'.repeat(1024 * 1024 + 64) + '"}'; + expect(await guard(jsonReq(frontLoaded))).not.toBeNull(); + + // __proto__ pushed beyond the 1 MiB cap by leading padding → outside the prefix → slips + // (documented residual: partial-scan can't see past the cap). + const buried = '{"pad":"' + 'a'.repeat(1024 * 1024 + 64) + '","__proto__":{"x":1}}'; + expect(await guard(jsonReq(buried))).toBeNull(); - const small = await guard(new Request('https://app/x', { method: 'POST', headers: { 'content-type': 'application/json' }, body: '{"__proto__":{"x":1}}' })); - expect(small).not.toBeNull(); // within cap → scanned → blocked + // a small body is still fully scanned. + expect(await guard(jsonReq('{"__proto__":{"x":1}}'))).not.toBeNull(); }); }); diff --git a/tests/protect/multipart.test.ts b/tests/protect/multipart.test.ts index 3f89fea..ab507e3 100644 --- a/tests/protect/multipart.test.ts +++ b/tests/protect/multipart.test.ts @@ -46,6 +46,24 @@ describe('multipart/form-data parsing', () => { expect(shaped._rawBody).toContain('__proto__'); }); + it('tolerates LF-only line endings and collects duplicate field names', async () => { + const lfBody = [ + `--${BOUNDARY}`, + 'Content-Disposition: form-data; name="tag"', + '', + 'first', + `--${BOUNDARY}`, + 'Content-Disposition: form-data; name="tag"', + '', + '', + `--${BOUNDARY}--`, + '', + ].join('\n'); // bare LF, not CRLF + const shaped: any = await fromFetchRequest(req(lfBody)); + expect(shaped.body.tag).toEqual(['first', '']); // duplicates collected + expect(shaped._rawBody).toContain('