Defense-in-depth: require own-property transport to skip prototype-pollution gadget

[ridingsa]

What

One-line change at lib/tools.js:337 — gate the opts.transport branch on Object.hasOwn(opts, 'transport'). When opts doesn't have its own transport key, skip the transport-loading branch instead of reading the value through the prototype chain.

Why

Per your reply on GHSA-7f85-5jq4-96m8, pino's threat model assumes no active prototype pollution on the process. That's fair — Node itself doesn't harden against PP, so libraries can't promise to either. This PR isn't asking pino to make that guarantee; it's accepting the existing threat model and adding a single own-property check at the place that triggers a dynamic import() via thread-stream.

The change costs one method call per pino() invocation (Object.hasOwn is V8-fast-path) and removes the one read site where a polluted Object.prototype.transport value would reach an import path.

What it does NOT do

• Doesn't claim to make pino "PP-safe". Any of the other dozen opts.* reads in this function still inherit from the prototype.
• Doesn't change behavior for any normal caller — if you pass pino({transport:...}), opts.transport is an own-property and hasOwn returns true.
• Doesn't add a dependency or a sanitization layer.

Test coverage

Verified locally against pino@main (0a10b332):

• npm test — 531 tests, 527 pass, 0 fail. One flaky retry on thread-stream async flush should call the passed callback; runner counted it 0-fail. Unrelated to the transport-read site.
• Gadget closed — with Object.prototype.transport = {target:'./evil.mjs'} polluted, calling pino() no longer spawns the worker → no attacker module loaded → no sentinel file created. (PoC from GHSA-7f85-5jq4-96m8 reproduced under the patched source.)
• Normal usage preserved — pino({transport: {target: <legit>}}) still spawns the worker and reaches the user-supplied transport module. Own-property transport passes hasOwn, branch fires as before.
• Coverage — tools.js lines: 97.65% pre-patch and post-patch; the patched line (337) sits in the already-covered region.

Happy to add an explicit regression test for the polluted-prototype case if you'd like — test/transport/transport.test.js would be the natural home. Drop the PoC in a tap.test('pino() ignores inherited Object.prototype.transport', ...) block.

Why this read site specifically

Of the 5 reads of opts.transport in normalizeArgs, this is the one that flows into transport({...opts.transport, ...}) which calls new ThreadStream({worker: target}) and ends up at worker_threads.Worker + dynamic import(). The other reads (opts && opts.transport truthy check at L330, instanceof SonicBoom at L338, .targets/.writable/._writableState checks) either throw an error on the polluted shape or short-circuit harmlessly. Hardening just the dangerous one keeps the patch minimal.

[mcollina]

Thanks for opening a PR! Can you please add a unit test?

[ridingsa]

Added the regression test in b6b0cd6 (b6b0cd6) — test/transport/protect-from-prototype-pollution.test.js plus a small fixture at test/fixtures/transport-pp-gadget-sentinel.mjs.

Two cases: (1) with Object.prototype.transport = {target: <attacker.mjs>} polluted and pino() called with no own transport, asserts the attacker module is never imported (sentinel file never created within a 300 ms grace window) — gadget-closed. (2) Same polluted prototype, explicit pino({transport: {target: }}), asserts the legit transport still spawns and receives the log line — no regression on the normal path.

Verified locally: patched, both pass in ~450 ms; patch reverted, test #1 fails with AssertionError: attacker module was imported via the prototype-polluted transport gadget (actual: true, expected: false) and test #2 still passes — confirms the signal is specific to the inherited-transport path. Lint clean. Uses node:test/node:assert to match the rest of test/transport/.

[mcollina]

lgtm

[mcollina]

Ci is very red

[luantaraschi]

Passing by on the "CI is very red" note, since this one is approved and only the checks are holding it. I measured it, and the red does not look like it belongs to this branch.

The failing run predates the fix for the test that fails in it. The last run here is 27361795864, on head b6b0cd6 over base 0a10b33 (June 11). Eleven of its thirteen jobs fail, and every one of them fails on the same single test:

✖ thread-stream async flush should call the passed callback
  Error: /tmp/pino-2965-4196a2c5674889f218dab4af hasn't been created within 10000 ms
  at Timeout.<anonymous> (test/helper.js:75:16)

Two unrelated branches failed the same way in the same window, down to the 11 of 13 job count:

run	branch	failing test
27361795864	this PR	thread-stream async flush should call the passed callback
27915356560	fix/multistream-write-error-isolation	same
28184120736	fix/proto-key-serializer-crash	same

That test was fixed on main by #2470 (c4d39d4, "test(transport): keep event loop alive while awaiting async flush"), merged July 13, which is after this PR's last run. Main has been green since.

Rebased on current main, the suite matches main. I cherry-picked 9857b9a and b6b0cd6 onto b394c2c and ran the full suite, then ran the same suite on unmodified b394c2c as a control. Windows 11, Node 22.20.0:

tree	tests	pass	fail	failing test
main b394c2c	542	537	1	pino.transport with worker destination overridden by bundler and mjs transport
main + this PR	544	539	1	the same one

The difference is exactly the two tests this PR adds, and both pass. The single failure shows up on unmodified main too, so it is my platform rather than this branch (it is the Windows transpile path that #2499 is about). npx eslint . is clean on the rebased tree.

The regression test does measure the change. On the rebased tree, test/transport/protect-from-prototype-pollution.test.js passes 2 of 2. Restoring only lib/tools.js to main and running it again drops it to 1 pass, 1 fail:

not ok 1 - pino() ignores inherited Object.prototype.transport (gadget closed)
  error: 'attacker module was imported via the prototype-polluted transport gadget'

So the assertion is tied to the Object.hasOwn guard and not passing for some unrelated reason.

One practical note: re-running the old workflow would replay the recorded merge commit, which still lacks #2470. Pushing a rebase onto current main is the surer way to get a green board. @ridingsa, if it helps, I am happy to hand you the rebased branch or open it as a PR against your fork.

[mcollina]

The same tests pass on main.