[ENG-4059] Screen the beginning of an oversized Node body

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,38 +169,27 @@ 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 =
     rulesData && typeof rulesData.evaluate === 'function' ? rulesData : new RuleEngine(rulesData);
   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();
     });
   };

src/protect/protect.d.ts

@@ -28,6 +28,12 @@ export interface Protection {
    */
   screenResponse(response: Response, request?: Request): Promise<Response>;
   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;

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';

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.<field>` at all.
-      function screenNodeRequest(req, res, next, rawBody, parsedBody) {
+      // body handed back as JSON resolves no `post.<field>` 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();

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/);
+  });
+});