diff --git a/src/protect/engine/node.js b/src/protect/engine/node.js index 4f742dfc..c847054f 100644 --- a/src/protect/engine/node.js +++ b/src/protect/engine/node.js @@ -12,6 +12,69 @@ import { parseBody } from './fetch.js'; import { notify } from '../notify.js'; import { appendOwn, setOwn } from './own.js'; +/** + * Read a Node request body, keeping at most `maxBytes` of it. + * + * The whole stream is consumed; bytes past the cap are counted and dropped rather than retained. `done` + * receives `(error)` or `(null, { text, overflow, size })`, where `text` is the retained prefix — the + * part that is screened, as on the Fetch path — and `overflow` says the body was longer than it. + * + * The cap and `size` are in bytes. A stream something upstream switched to text (`setEncoding`) delivers + * strings, which are measured and retained as the bytes they encode. A prefix that ends partway through a + * UTF-8 character ends before that character instead. `done` is called once. + * + * `error` is the stream's own error. A chunk this reader cannot use is not one: the body is then read to + * its end unscreened, and reported as `{ text: '', failed: true }` so the caller can fail open. + */ +export function readBodyPrefix(req, maxBytes, done) { + const chunks = []; + let retained = 0; + let size = 0; + let failed = false; + let finished = false; + const finish = (error, read) => { + if (finished) return; + finished = true; + done(error, read); + }; + req.on('data', (chunk) => { + if (failed) return; + try { + const bytes = typeof chunk === 'string' ? Buffer.from(chunk, req.readableEncoding || 'utf8') : chunk; + size += bytes.length; + const take = Math.min(bytes.length, maxBytes - retained); + if (take <= 0) return; + chunks.push(take === bytes.length ? bytes : bytes.subarray(0, take)); + retained += take; + } catch { + failed = true; + chunks.length = 0; + } + }); + req.on('error', (error) => finish(error)); + req.on('end', () => { + if (failed) { + finish(null, { text: '', overflow: false, size, failed: true }); + return; + } + const prefix = Buffer.concat(chunks); + const overflow = size > maxBytes; + const text = (overflow ? prefix.subarray(0, completeUtf8Length(prefix)) : prefix).toString('utf8'); + finish(null, { text, overflow, size, failed: false }); + }); +} + +// The length of `bytes` without a UTF-8 sequence left incomplete at its end. +function completeUtf8Length(bytes) { + let start = bytes.length - 1; + // Back over at most three continuation bytes (10xxxxxx) to the byte that begins the last sequence. + while (start >= 0 && bytes.length - start <= 3 && (bytes[start] & 0xc0) === 0x80) start--; + if (start < 0) return bytes.length; + const lead = bytes[start]; + const needed = lead >= 0xf0 ? 4 : lead >= 0xe0 ? 3 : lead >= 0xc0 ? 2 : 1; + return bytes.length - start < needed ? start : bytes.length; +} + // Build the engine's request shape from a Node IncomingMessage + its raw body text. export function fromNodeRequest(req, rawBody = '', options = {}) { const method = (req.method || 'GET').toUpperCase(); @@ -106,8 +169,9 @@ function defaultBlock(res, result) { /** * Connect/Express-style middleware `(req, res, next)` that buffers the body itself. * Accepts a `RuleEngine` instance or a `{ firewall, whitelists, whitelist_keys }` bundle. - * Fails open: an engine error (or oversized body) never blocks the request. - * Options: `{ maxBodyBytes = 1MiB, onBlock, onError, response }`. + * Fails open: an engine error never blocks the request. A body longer than `maxBodyBytes` has its first + * `maxBodyBytes` screened, is reported to `onSkip` as `body-cap`, and is not exposed as `req.body`. + * Options: `{ maxBodyBytes = 1MiB, onBlock, onError, onSkip, response }`. */ export function createNodeMiddleware(rulesData, options = {}) { const engine = @@ -115,29 +179,17 @@ export function createNodeMiddleware(rulesData, options = {}) { const maxBytes = options.maxBodyBytes ?? 1024 * 1024; return function guard(req, res, next) { - const chunks = []; - let size = 0; - let overflow = false; - - req.on('data', (chunk) => { - size += chunk.length; - if (size > maxBytes) { - overflow = true; - return; - } - chunks.push(chunk); - }); - - req.on('error', (err) => next(err)); + readBodyPrefix(req, maxBytes, (error, read) => { + if (error) return next(error); - req.on('end', () => { let result; let shaped; try { - const rawBody = overflow ? '' : Buffer.concat(chunks).toString('utf8'); // The caller's policy reaches the shaping, or this adapter would always report the socket peer // even where its caller declared a trusted front end. - shaped = fromNodeRequest(req, rawBody, { trustedProxy: options.trustedProxy }); // never crash + shaped = fromNodeRequest(req, read.text, { trustedProxy: options.trustedProxy }); // never crash + if (read.overflow) notify(options.onSkip, { phase: 'request', reason: 'body-cap' }, 'onSkip'); + if (read.failed) notify(options.onSkip, { phase: 'request', reason: 'read-failed' }, 'onSkip'); result = engine.evaluate(shaped); } catch (err) { notify(options.onError, err, 'onError'); @@ -155,8 +207,9 @@ export function createNodeMiddleware(rulesData, options = {}) { return (options.response || defaultBlock)(res, result); } - // Expose the parsed body downstream so a body-parser isn't also required. - req.body = shaped.body; + // Expose the parsed body downstream so a body-parser isn't also required — unless the body was + // longer than the cap, when what was parsed is only its beginning and is not handed on as the body. + if (!read.overflow && !read.failed) req.body = shaped.body; next(); }); }; diff --git a/src/protect/protect.d.ts b/src/protect/protect.d.ts index 1047bb30..f2ec26ba 100644 --- a/src/protect/protect.d.ts +++ b/src/protect/protect.d.ts @@ -28,6 +28,12 @@ export interface Protection { */ screenResponse(response: Response, request?: Request): Promise; express(options?: { screenResponses?: boolean }): (req: unknown, res: unknown, next: () => void) => void; + /** + * Node / Connect middleware that reads the request body itself and exposes it as `req.body`. + * + * A body longer than `maxBodyBytes` (default 1 MiB) has its first `maxBodyBytes` screened, is counted as + * a `body-cap` skip in `coverage()` / `onSkip`, and is not exposed as `req.body`. + */ node(options?: { maxBodyBytes?: number; screenResponses?: boolean }): (req: unknown, res: unknown, next: () => void) => void; /** Present when `egress: true` — restores the original global fetch. */ uninstallEgress?: () => void; diff --git a/src/protect/rules/contract.js b/src/protect/rules/contract.js index baf2ca1b..d176ccd1 100644 --- a/src/protect/rules/contract.js +++ b/src/protect/rules/contract.js @@ -540,6 +540,11 @@ export function conditionShapeProblem(condition) { if (nested && !named) { return `condition carries nested rules but its parameter is ${JSON.stringify(condition.parameter)}; a group must be {"parameter":"${GROUP_PARAMETER}"}`; } + // The engine reads `inclusive` for its truthiness, so the string "false" would AND a condition its + // author meant to OR. Only a boolean says what it means. + if (condition.inclusive !== undefined && typeof condition.inclusive !== 'boolean') { + return 'inclusive must be true or false'; + } if (named && nested) { if (condition.match !== undefined) return 'a group carries no match of its own; the engine ignores it'; if (condition.mutations !== undefined) return 'a group carries no mutations of its own; the engine ignores them'; diff --git a/src/protect/runtime.js b/src/protect/runtime.js index 51575f02..c5b3bdd7 100644 --- a/src/protect/runtime.js +++ b/src/protect/runtime.js @@ -24,7 +24,7 @@ import { requestField } from './engine/normalizer.js'; import { captureValues, createPlanCache, permitsAnything } from './capture-plan.js'; import { PulseRuleClient } from './engine/pulse-client.js'; import { fromFetchRequest } from './engine/fetch.js'; -import { fromNodeRequest } from './engine/node.js'; +import { fromNodeRequest, readBodyPrefix } from './engine/node.js'; import { appendOwn, setOwn } from './engine/own.js'; import { installEgressGuard } from './egress.js'; import { DEFAULT_RESPONSE_RULES, DEFAULT_EGRESS_RULES } from './defaults.js'; @@ -1524,31 +1524,24 @@ export async function createProtection(options = {}) { return; } - const chunks = []; - let size = 0; - let overflow = false; - req.on('data', (chunk) => { - size += chunk.length; - if (size > maxBytes) { - overflow = true; + // A body longer than the cap is screened up to the cap, as on the Fetch path, and reported. + readBodyPrefix(req, maxBytes, (err, read) => { + if (err) { + notify(onError, err, 'onError'); + next(); return; } - chunks.push(chunk); - }); - req.on('error', (err) => { - notify(onError, err, 'onError'); - next(); - }); - req.on('end', () => { - if (overflow) recordSkip('request', 'body-cap', { bytes: size, limit: maxBytes }); - screenNodeRequest(req, res, next, overflow ? '' : Buffer.concat(chunks).toString('utf8')); + if (read.overflow) recordSkip('request', 'body-cap', { bytes: read.size, limit: maxBytes }); + if (read.failed) recordSkip('request', 'read-failed', { bytes: read.size }); + screenNodeRequest(req, res, next, read.text, undefined, read.overflow || read.failed); }); }; // `parsedBody`, when given, is a body somebody else already parsed: it replaces the shaped body // rather than being re-serialized, because re-encoding it would have to guess a format and a form - // body handed back as JSON resolves no `post.` at all. - function screenNodeRequest(req, res, next, rawBody, parsedBody) { + // body handed back as JSON resolves no `post.` at all. `truncated` says `rawBody` is only the + // beginning of a longer body. + function screenNodeRequest(req, res, next, rawBody, parsedBody, truncated = false) { let shaped; let result; try { @@ -1574,8 +1567,9 @@ export async function createProtection(options = {}) { }, () => { // This guard consumed the request stream to screen it; re-expose the parsed - // body so a downstream handler (without its own body-parser) can read it. - if (req.body === undefined) req.body = shaped.body; + // body so a downstream handler (without its own body-parser) can read it. A body cut off at + // the cap is not re-exposed: what was parsed is only its beginning, not the request's body. + if (req.body === undefined && !truncated) req.body = shaped.body; // The resolution the shaping already made, carried into the response phase and the record. if (nodeOptions.screenResponses) wrapNodeResponse(res, reqContextFromNode(req, shaped?._clientIp)); next(); diff --git a/tests/protect/inclusive-flag.test.ts b/tests/protect/inclusive-flag.test.ts new file mode 100644 index 00000000..62085888 --- /dev/null +++ b/tests/protect/inclusive-flag.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from 'vitest'; +import { validateBundle } from '../../src/protect/rules/validate.js'; + +// `inclusive` decides whether a condition is ANDed with its siblings or ORed. Only a boolean is accepted, +// because anything else would be read for its truthiness rather than for what it says. + +const condition = (inclusive: unknown) => ({ parameter: 'get.q', inclusive, match: { type: 'contains', value: 'x' } }); +const rule = (conditions: unknown[]) => ({ id: 'r1', rule_v2: conditions }); + +describe('the inclusive flag', () => { + it.each([true, false])('accepts %s', (inclusive) => { + const { rejected } = validateBundle({ firewall: [rule([condition(inclusive), condition(inclusive)])], whitelists: [] } as any); + expect(rejected).toEqual([]); + }); + + it('accepts a condition without it', () => { + const { rejected } = validateBundle({ firewall: [rule([{ parameter: 'get.q', match: { type: 'contains', value: 'x' } }])], whitelists: [] } as any); + expect(rejected).toEqual([]); + }); + + it.each(['false', 'true', 0, 1, null, {}])('rejects %j with a reason', (inclusive) => { + const { bundle, rejected } = validateBundle({ firewall: [rule([condition(inclusive)])], whitelists: [] } as any); + expect(bundle.firewall).toHaveLength(0); + expect(rejected[0].reason).toMatch(/inclusive must be true or false/); + }); + + it('rejects it inside a group too', () => { + const grouped = rule([{ parameter: 'rules', rules: [condition('false')] }]); + const { rejected } = validateBundle({ firewall: [grouped], whitelists: [] } as any); + expect(rejected[0].reason).toMatch(/inclusive must be true or false/); + }); +}); diff --git a/tests/protect/node-body-prefix.test.ts b/tests/protect/node-body-prefix.test.ts new file mode 100644 index 00000000..c026b573 --- /dev/null +++ b/tests/protect/node-body-prefix.test.ts @@ -0,0 +1,182 @@ +import { describe, expect, it } from 'vitest'; +import { Readable } from 'node:stream'; +import { createProtection } from '../../src/protect/runtime.js'; +import { createNodeMiddleware, readBodyPrefix } from '../../src/protect/engine/node.js'; + +// A Node request body longer than the cap: the beginning is screened, as on the Fetch path, the cap is +// reported, and the cut-off body is not handed on as though it were the whole one. + +const rules = { + firewall: [{ id: 'b', title: 'body marker', rule_v2: [{ parameter: 'raw', match: { type: 'contains', value: 'sample-marker' } }] }], + whitelists: [], + whitelist_keys: {}, +}; + +// Delivered in small chunks, so the cap falls inside a chunk rather than on a boundary. +function mockReq(body: string) { + const bytes = Buffer.from(body); + const chunks: Buffer[] = []; + for (let i = 0; i < bytes.length; i += 7) chunks.push(bytes.subarray(i, i + 7)); + const req: any = Readable.from(chunks); + req.method = 'POST'; + req.url = '/'; + req.headers = { 'content-type': 'application/json', host: 'app.test' }; + req.socket = { remoteAddress: '198.51.100.7' }; + return req; +} + +function run(guard: any, req: any): Promise<{ passed: boolean; status: number }> { + const res: any = { statusCode: 200, setHeader() {}, getHeader() {}, end() {} }; + return new Promise((resolve) => { + res.end = () => resolve({ passed: false, status: res.statusCode }); + guard(req, res, () => resolve({ passed: true, status: res.statusCode })); + }); +} + +const early = JSON.stringify({ note: 'sample-marker', pad: 'x'.repeat(500) }); +const late = JSON.stringify({ pad: 'x'.repeat(500), note: 'sample-marker' }); +const small = JSON.stringify({ note: 'plain' }); + +describe.each([ + ['protection.node()', async () => (await createProtection({ rules, mode: 'block' } as any)).node({ maxBodyBytes: 64 })], + ['createNodeMiddleware', async () => createNodeMiddleware(rules, { maxBodyBytes: 64 })], +])('%s with a body longer than the cap', (_label, make) => { + it('screens the beginning of the body', async () => { + const { passed, status } = await run(await make(), mockReq(early)); + expect(passed).toBe(false); + expect(status).toBe(403); + }); + + it('screens exactly the first maxBodyBytes, including a chunk that crosses the cap', async () => { + const lead = '{"a":"'; + const endsAt = (end: number) => lead + 'x'.repeat(end - lead.length - 'sample-marker'.length) + 'sample-marker' + 'y'.repeat(200) + '"}'; + // 64 is not a multiple of the 7-byte chunk size, so the chunk holding the cap is partly retained. + expect((await run(await make(), mockReq(endsAt(64)))).passed).toBe(false); + expect((await run(await make(), mockReq(endsAt(65)))).passed).toBe(true); + }); + + it('does not expose the cut-off body as req.body', async () => { + const req = mockReq(late); + const { passed } = await run(await make(), req); + expect(passed).toBe(true); + expect(req.body).toBeUndefined(); + }); + + it('still exposes a body within the cap', async () => { + const req = mockReq(small); + const { passed } = await run(await make(), req); + expect(passed).toBe(true); + expect(req.body).toEqual({ note: 'plain' }); + }); +}); + +const readPrefix = (req: any, max: number) => + new Promise((resolve, reject) => readBodyPrefix(req, max, (error: any, read: any) => (error ? reject(error) : resolve(read)))); + +describe('the retained prefix', () => { + it.each([ + ['a two-byte', 'é'], + ['a three-byte', '€'], + ['a four-byte', '😀'], + ])('ends before %s character the cap falls inside', async (_label, char) => { + const width = Buffer.byteLength(char); + for (let max = width * 3 + 1; max < width * 4; max++) { + const read = await readPrefix(Readable.from([Buffer.from(char.repeat(10))]), max); + expect(read.text, `cap ${max}`).toBe(char.repeat(3)); + expect(read.overflow).toBe(true); + } + }); + + it.each(['utf8', 'latin1', 'hex'] as const)('counts bytes, not characters, after setEncoding(%s)', async (encoding) => { + const req: any = Readable.from([Buffer.from('é'.repeat(40))]); + req.setEncoding(encoding); + const read = await readPrefix(req, 63); + expect(read.text).toBe('é'.repeat(31)); + expect(read.size).toBe(80); + expect(read.overflow).toBe(true); + }); + + it('leaves the end of a complete body as it was sent', async () => { + // Only a prefix the cap cut short is trimmed; a whole body is decoded as the application decodes it. + const read = await readPrefix(Readable.from([Buffer.concat([Buffer.from('abc'), Buffer.from([0xc3])])]), 64); + expect(read.text).toBe('abc�'); + expect(read.overflow).toBe(false); + }); + + it('keeps a complete body whole after setEncoding', async () => { + const req: any = Readable.from([Buffer.from('{"note":"é"}')]); + req.setEncoding('utf8'); + const read = await readPrefix(req, 64); + expect(read).toEqual({ text: '{"note":"é"}', overflow: false, size: 13, failed: false }); + }); +}); + +describe.each([ + ['protection.node()', async () => (await createProtection({ rules, mode: 'block' } as any)).node({ maxBodyBytes: 64 })], + ['createNodeMiddleware', async () => createNodeMiddleware(rules, { maxBodyBytes: 64 })], +])('%s after setEncoding', (_label, make) => { + const encoded = (body: string) => { + const req = mockReq(body); + req.setEncoding('utf8'); + return req; + }; + + it('screens and exposes a body within the cap', async () => { + expect((await run(await make(), encoded(early.slice(0, 40) + '"}'))).passed).toBe(false); + const req = encoded(small); + expect((await run(await make(), req)).passed).toBe(true); + expect(req.body).toEqual({ note: 'plain' }); + }); + + it('screens the beginning of a body longer than the cap', async () => { + expect((await run(await make(), encoded(early))).passed).toBe(false); + }); +}); + +describe('a chunk the reader cannot use', () => { + // An object-mode stream delivers values that are neither bytes nor text. + const objectReq = () => { + const req: any = Readable.from([{ note: 'sample-marker' }]); + req.method = 'POST'; + req.url = '/'; + req.headers = { 'content-type': 'application/json', host: 'app.test' }; + req.socket = { remoteAddress: '198.51.100.7' }; + return req; + }; + + it('fails open on protection.node() and reports it', async () => { + const protection: any = await createProtection({ rules, mode: 'block' } as any); + const req = objectReq(); + expect((await run(protection.node(), req)).passed).toBe(true); + expect(req.body).toBeUndefined(); + expect(protection.coverage().skipped['request:read-failed']).toBe(1); + }); + + it('fails open on createNodeMiddleware and reports it', async () => { + const skips: any[] = []; + const req = objectReq(); + expect((await run(createNodeMiddleware(rules, { onSkip: (skip: any) => skips.push(skip) }), req)).passed).toBe(true); + expect(req.body).toBeUndefined(); + expect(skips).toEqual([{ phase: 'request', reason: 'read-failed' }]); + }); +}); + +describe('reporting the cap', () => { + it('counts it on protection.node()', async () => { + const protection: any = await createProtection({ rules, mode: 'block' } as any); + await run(protection.node({ maxBodyBytes: 64 }), mockReq(late)); + expect(protection.coverage().skipped['request:body-cap']).toBe(1); + }); + + it('reports it to onSkip on createNodeMiddleware', async () => { + const skips: any[] = []; + await run(createNodeMiddleware(rules, { maxBodyBytes: 64, onSkip: (skip: any) => skips.push(skip) }), mockReq(late)); + expect(skips).toEqual([{ phase: 'request', reason: 'body-cap' }]); + }); + + it('reports nothing for a body within the cap', async () => { + const skips: any[] = []; + await run(createNodeMiddleware(rules, { maxBodyBytes: 64, onSkip: (skip: any) => skips.push(skip) }), mockReq(small)); + expect(skips).toEqual([]); + }); +});