fix: close review issues #6-#13

.github/workflows/ci.yml

@@ -11,41 +11,36 @@ jobs:
       fail-fast: false
       matrix:
         # windows-2022 ships VS 2022. windows-latest currently has VS 2026
-        # (18.x), which node-gyp 10/11 bundled with Node 18/20/22 reports as
+        # (18.x), which node-gyp 10/11 bundled with Node 22/24 reports as
         # unknown version "undefined" and then fails the native rebuild.
         os: [ubuntu-latest, macos-latest, windows-2022]
-        node: [18, 20, 22]
+        node: [22, 24]
     steps:
-      - uses: actions/checkout@v4
-      - uses: actions/setup-node@v4
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+      - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
         with:
           node-version: ${{ matrix.node }}
       - name: Install and test
         # bash so `&&` is a hard stop on Windows too (PowerShell would still
         # run npm test after a failed rebuild).
         shell: bash
-        run: npm install && npm test
+        run: npm ci && npm test
 
   coverage:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
-      - uses: actions/setup-node@v4
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+      - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
         with:
-          node-version: 20
-      - uses: actions/setup-python@v5
+          node-version: 22
+      - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
         with:
           python-version: '3.x'
       - name: Install gcovr
         run: |
           gcovr --version || pip install --upgrade gcovr
       - name: Install dependencies
-        run: |
-          if [ -f package-lock.json ]; then
-            npm ci || npm install
-          else
-            npm install
-          fi
+        run: npm ci
       # Fails the job when JS or native coverage drops below 95%.
       - name: Coverage
         run: npm run coverage

README.md

@@ -2,7 +2,7 @@
 and de-serializes JavaScript values with [MessagePack](https://msgpack.org).
 Packed output is a `Buffer` and is typically much smaller than JSON.
 
-Version 2.0 requires **Node.js 18+**, vendors **msgpack-c c-7.0.2**, and
+Version 2.0 requires **Node.js 22+**, vendors **msgpack-c c-7.0.2**, and
 rejects oversized unpack headers instead of allocating them. See
 [`SECURITY.md`](SECURITY.md).
 
@@ -25,7 +25,9 @@ the buffer is a truncated (incomplete) MessagePack object. Oversized
 array/map/string bombs throw.
 
 A streaming helper wraps a readable socket and emits `msg`, plus `error` when
-a packet cannot be unpacked (the offending buffer is dropped):
+a packet cannot be unpacked, the receive buffer would exceed
+`MAX_STREAM_BYTES` (the offending buffer is dropped, and the socket is
+destroyed when possible), or the underlying stream errors:
 
 ```javascript
 const msgpack = require('msgpack');
@@ -71,26 +73,31 @@ successful (or attempted) unpack. Stream uses that to splice leftover data.
 
 ### Limits
 
-* array/map length ≤ 1,000,000
+* array/map length ≤ 1,000,000 on both pack and unpack
 * str/bin/ext length ≤ 32 MiB
 * nesting depth ≤ 512 on both pack and unpack
+* Stream receive buffer ≤ `MAX_STREAM_BYTES` (32 MiB + 16 bytes of framing)
+* CLI stdin ≤ `MAX_STDIN_BYTES` (32 MiB) before concat/parse
 
 The payload `dd ff 00 00 00` throws `msgpack unpack limit exceeded`. Packing a
-value nested deeper than 512 throws `Cowardly refusing to pack object nested
-more than 512 levels deep` instead of overflowing the C stack.
+sparse array or map whose length exceeds 1,000,000 throws
+`msgpack pack limit exceeded`. Packing a value nested deeper than 512 throws
+`Cowardly refusing to pack object nested more than 512 levels deep` instead of
+overflowing the C stack. Incomplete Stream frames that would grow past
+`MAX_STREAM_BYTES` throw `msgpack stream limit exceeded` before allocate.
 
 ### Building, installation, testing
 
 ```
-npm install
+npm ci
 npm test
 npm run coverage
 ```
 
-Needs a C/C++ toolchain and Python (node-gyp). GitHub Actions runs Node 18/20/22
-on Ubuntu and macOS. `npm run coverage` instruments JavaScript with c8 and the
-native addon with gcov, and fails under 95%. Gates and remaining uncovered
-lines are documented in [`COVERAGE.md`](COVERAGE.md).
+Needs a C/C++ toolchain and Python (node-gyp). GitHub Actions runs Node 22/24
+on Ubuntu, macOS, and windows-2022. `npm run coverage` instruments JavaScript
+with c8 and the native addon with gcov, and fails under 95%. Gates and remaining
+uncovered lines are documented in [`COVERAGE.md`](COVERAGE.md).
 
 ### Command Line Utilities
 
@@ -126,7 +133,8 @@ echo '{"hello":"world"}' | bin/json2msgpack | bin/msgpack2json
 ```
 
 `msgpack2json` prints one JSON value per line and consumes every complete
-message in its input. Both exit non-zero on invalid or truncated input.
+message in its input. Both exit non-zero on invalid or truncated input, and
+both refuse stdin larger than `MAX_STDIN_BYTES` (32 MiB).
 
 ### Benchmarks
 

SECURITY.md

@@ -50,6 +50,43 @@ on circular refs / unencodable values without freeing it. The sbuffer is now
 owned by an RAII guard that returns pooled buffers or `msgpack_sbuffer_free`s
 on every exit path, including C++ exceptions.
 
+## Stream receive buffer
+
+`msgpack.Stream` concatenates incomplete frames into `self.buf`. Before any
+allocate, a new chunk is rejected when `self.buf.length + chunk.length` would
+exceed `MAX_STREAM_BYTES` (32 MiB plus 16 bytes of MessagePack framing). The
+buffer is dropped, `'error'` is emitted (`msgpack stream limit exceeded`),
+and the underlying stream is `destroy()`ed when that method exists. `close`,
+`end`, and `error` on the underlying stream also drop `self.buf`.
+
+## CLI stdin
+
+`bin/json2msgpack` and `bin/msgpack2json` refuse stdin larger than
+`MAX_STDIN_BYTES` (32 MiB) before `Buffer.concat` / `JSON.parse` /
+`unpack`. They exit 1 with `stdin exceeds MAX_STDIN_BYTES`.
+
+## Pack container size
+
+`pack()` rejects arrays and maps whose own length exceeds 1,000,000
+(`kMaxContainer`), the same policy as unpack. Sparse `Array.length` above
+that cap throws `msgpack pack limit exceeded` without walking the holes.
+
+## Dependency bump window
+
+The pins below are inventory, not a calendar SLA:
+
+- msgpack-c **c-7.0.2** (`e17beb371b59459a13b48e166a11e123bda5bf93`)
+- NAN **2.28.0** (compile-in; exact in `package.json` / shrinkwrap)
+
+Risk-based window:
+
+- **Critical or high** native advisories in vendored msgpack-c or in
+  compile-in NAN: bump to a reviewed fix, or document why the pin stays,
+  within **14 days** of that fix being available.
+- **Medium**: next minor of this package.
+- **Low / no advisory**: no scheduled bump. The inventory pin is not a
+  commitment to track upstream on a calendar.
+
 ## License
 
 First-party code in this repository is BSD-3-Clause (see `LICENSE`). The

bin/json2msgpack

@@ -5,9 +5,18 @@
 
 const msgpack = require('../lib/msgpack');
 
+const MAX_STDIN_BYTES = msgpack.MAX_STDIN_BYTES;
 const chunks = [];
+let n = 0;
 
-process.stdin.on('data', (d) => chunks.push(d));
+process.stdin.on('data', (d) => {
+    n += d.length;
+    if (n > MAX_STDIN_BYTES) {
+        console.error('json2msgpack: stdin exceeds MAX_STDIN_BYTES (' + MAX_STDIN_BYTES + ')');
+        process.exit(1);
+    }
+    chunks.push(d);
+});
 
 process.stdin.on('end', () => {
     const text = Buffer.concat(chunks).toString('utf8');

bin/msgpack2json

@@ -5,9 +5,18 @@
 
 const msgpack = require('../lib/msgpack');
 
+const MAX_STDIN_BYTES = msgpack.MAX_STDIN_BYTES;
 const chunks = [];
+let n = 0;
 
-process.stdin.on('data', (d) => chunks.push(d));
+process.stdin.on('data', (d) => {
+    n += d.length;
+    if (n > MAX_STDIN_BYTES) {
+        console.error('msgpack2json: stdin exceeds MAX_STDIN_BYTES (' + MAX_STDIN_BYTES + ')');
+        process.exit(1);
+    }
+    chunks.push(d);
+});
 
 process.stdin.on('end', () => {
     let buf = Buffer.concat(chunks);

index.d.ts

@@ -9,7 +9,8 @@ import { EventEmitter } from 'events';
  * Serialize values to MessagePack.
  *
  * A single argument is packed as itself; two or more are packed as an array
- * of that many elements.
+ * of that many elements. Arrays and maps whose length exceeds 1,000,000
+ * throw `msgpack pack limit exceeded`.
  */
 export function pack(...values: any[]): Buffer;
 
@@ -31,11 +32,26 @@ export namespace unpack {
     let bytes_remaining: number;
 }
 
+/**
+ * Cap on `Stream` receive concatenation (32 MiB plus 16 bytes of framing).
+ * A chunk that would take `buf.length + chunk.length` past this is rejected
+ * before allocate.
+ */
+export const MAX_STREAM_BYTES: number;
+
+/**
+ * Cap on CLI stdin accumulation in `json2msgpack` / `msgpack2json` (32 MiB).
+ * Overflow exits 1 before concat/parse.
+ */
+export const MAX_STDIN_BYTES: number;
+
 /**
  * Frames MessagePack messages over a stream.
  *
  * Emits `'msg'` with each decoded value, and `'error'` if a packet cannot be
- * decoded (the buffered data is then dropped).
+ * decoded or the receive buffer would exceed `MAX_STREAM_BYTES` (the buffered
+ * data is then dropped; the underlying stream is destroyed when possible).
+ * `buf` is also dropped on the underlying stream's `close` / `end` / `error`.
  */
 export class Stream extends EventEmitter {
     constructor(s: NodeJS.ReadWriteStream);

lib/msgpack.js

@@ -11,6 +11,12 @@ const mpBindings = require(__dirname + '/../build/Release/msgpackBinding');
 const bpack = mpBindings.pack;
 const rawUnpack = mpBindings.unpack;
 
+/* 32 MiB payload (same as unpack str/bin) plus 16 bytes of MessagePack
+ * framing so a max-legal bin32/str32 still fits in one Stream buffer. */
+const MAX_STREAM_BYTES = 32 * 1024 * 1024 + 16;
+/* CLI stdin is capped at 32 MiB before concat/parse. */
+const MAX_STDIN_BYTES = 32 * 1024 * 1024;
+
 /* No JS pre-pass: the binding already applies toJSON at every level and
  * packs Dates as ISO strings. Calling toJSON here instead turned a top-level
  * Buffer into Buffer.prototype.toJSON's {type,data} map rather than bin. */
@@ -30,6 +36,22 @@ function Stream(s) {
   const self = this;
   events.EventEmitter.call(self);
   self.buf = null;
+  let dead = false;
+
+  function dropBuf() {
+    self.buf = null;
+  }
+
+  function rejectLimit() {
+    if (dead) return;
+    dead = true;
+    dropBuf();
+    const err = new Error('msgpack stream limit exceeded');
+    self.emit('error', err);
+    if (typeof s.destroy === 'function') {
+      s.destroy(err);
+    }
+  }
 
   self.send = function (m) {
     const args = [pack(m)];
@@ -40,10 +62,16 @@ function Stream(s) {
   };
 
   s.addListener('data', function (d) {
+    if (dead) return;
+    const have = self.buf ? self.buf.length : 0;
+    if (have + d.length > MAX_STREAM_BYTES) {
+      rejectLimit();
+      return;
+    }
     if (self.buf) {
-      const b = buffer.Buffer.allocUnsafe(self.buf.length + d.length);
-      self.buf.copy(b, 0, 0, self.buf.length);
-      d.copy(b, self.buf.length, 0, d.length);
+      const b = buffer.Buffer.allocUnsafe(have + d.length);
+      self.buf.copy(b, 0, 0, have);
+      d.copy(b, have, 0, d.length);
       self.buf = b;
     } else {
       self.buf = d;
@@ -79,10 +107,24 @@ function Stream(s) {
       self.emit('msg', msg);
     }
   });
+
+  s.addListener('close', dropBuf);
+  s.addListener('end', dropBuf);
+  /* Any 'error' listener counts as handling in Node, so this must re-emit
+   * on Stream. Skip when already dead: rejectLimit emits then destroy(err),
+   * which fires this listener again. */
+  s.addListener('error', function (err) {
+    dropBuf();
+    if (!dead) {
+      self.emit('error', err);
+    }
+  });
 }
 
 util.inherits(Stream, events.EventEmitter);
 
 exports.pack = pack;
 exports.unpack = unpack;
 exports.Stream = Stream;
+exports.MAX_STREAM_BYTES = MAX_STREAM_BYTES;
+exports.MAX_STDIN_BYTES = MAX_STDIN_BYTES;

package.json

@@ -28,10 +28,10 @@
     "lib": "lib"
   },
   "engines": {
-    "node": ">=18"
+    "node": ">=22"
   },
   "dependencies": {
-    "nan": "^2.23.1"
+    "nan": "2.28.0"
   },
   "scripts": {
     "test": "node --test test/cli.test.js test/coverage-native.test.js test/msgpack.test.js test/regression.test.js test/security.test.js test/worker.test.js",
@@ -48,6 +48,6 @@
   "gypfile": true,
   "license": "BSD-3-Clause",
   "devDependencies": {
-    "c8": "^12.0.0"
+    "c8": "12.0.0"
   }
 }

src/msgpack.cc

@@ -426,7 +426,13 @@ static void PackArray(msgpack_packer* pk, v8::Local<v8::Array> arr, int depth) {
     throw MsgpackException(Error("Cowardly refusing to pack circular reference"));
   }
   Mark(arr);
+  /* Snapshot Length() once and freeze it for the walk so a growing getter
+   * cannot extend the loop. Same 1e6 policy as ScanOne. */
   uint32_t len = arr->Length();
+  if (len > kMaxContainer) {
+    Unmark(arr);
+    throw MsgpackException(Error("msgpack pack limit exceeded"));
+  }
   /* GCOVR_EXCL_BR_START: msgpack_sbuffer_write only fails on realloc
    * failure, which no JS-reachable input can force. */
   if (msgpack_pack_array(pk, len)) {
@@ -481,6 +487,10 @@ static void PackObject(msgpack_packer* pk, v8::Local<v8::Object> obj, int depth)
     throw;
   }
   uint32_t len = names->Length();
+  if (len > kMaxContainer) {
+    Unmark(obj);
+    throw MsgpackException(Error("msgpack pack limit exceeded"));
+  }
   /* GCOVR_EXCL_BR_START: allocation failure only, as in PackArray. */
   if (msgpack_pack_map(pk, len)) {
     Unmark(obj);