test_runner: parent runner still crashes on child stdout bytes that mimic an event frame (survives the #64706 fix)

Open
#66,164 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
68/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
javascript
Domain
testing-qa

Research direction

Start in lib/internal/test_runner/runner.js, tracing FileTest.parseMessage into #processRawBuffer, and run the deterministic poison.test.mjs reproduction with node --test. The fix is done when arbitrary child stdout, including bytes resembling a frame, no longer crashes the parent and subsequent test frames are still processed; verify the control and reproduction cases.

Written by the indexing model from the issue text.

Description

confirmed-bug test_runner
  • Version: v24.21.0 (this release includes the >>> 0 fix from #64706); also verified on v26.4.0 and v22.23.2
  • Platform: macOS 26.6.2 (25G83), arm64
  • Subsystem: test_runner

Current behavior

node --test crashes the parent runner (exit code 1; the failing component is the parent's stdout frame parser, not any test assertion) with:

Error: Unable to deserialize cloned data due to invalid or unsupported version.
    at #processRawBuffer (node:internal/test_runner/runner:497:20)
    at FileTest.parseMessage (node:internal/test_runner/runner:404:29)
    at Socket.<anonymous> (node:internal/test_runner/runner:552:15)

How often: deterministic — 10/10 runs on every version below. Control (same file minus the stdout write): 10/10 green on every version.

node has #64706 >>> 0 fix repro control
v24.21.0 yes (verified in shipped runner.js source) 10/10 crash 10/10 pass
v26.4.0 no (compiled-in source still has the signed read) 10/10 crash 10/10 pass
v22.23.2 no (v22 backport tracked in #65934) 10/10 crash 10/10 pass

Expected behavior

Anything a test (or code it imports) writes to stdout — text, banners, progress bars, QR codes, binary — should not be able to take down the parent runner. At worst it should degrade to test:stdout output or a per-file diagnostic, not abort the whole run with an error pointing at runner internals.

Steps to reproduce

Save as poison.test.mjs, then run node --test poison.test.mjs:

import { test } from 'node:test'

test('writes bytes that mimic a frame header', () => {
  const poison = Buffer.from([
    0xFF, 0x0F,             // frame magic (v8 serdes header tag)
    0x00, 0x00, 0x00, 0x08, // fake payload size = 8
    0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, // "ABCDEFGH" payload
  ])
  process.stdout.write(poison)
})

test('subsequent test whose real frames must also be parsed', () => {
  // trivial
})

Removing the process.stdout.write(poison) line makes it 10/10 green on all three versions.

Analysis

In lib/internal/test_runner/runner.js:

  • runTestFile spawns each test file with stdio: ['pipe', 'pipe', 'pipe'] and pipes the child's raw stdout into FileTest.parseMessage → #processRawBuffer.
  • The child's stdout therefore carries two unrelated producers on one pipe: the runner's own length-prefixed frames (FF 0F | uint32 BE size | v8 payload) and arbitrary application output. There is no isolation or escape mechanism for user bytes.
  • The first loop in #processRawBuffer skips bytes up to the next FF 0F magic and reports them as test:stdout. But after consuming a frame, the second loop continues without re-checking the magic — it trusts the next 4 bytes as a size and hands the slice straight to readHeader() / readValue(), with no try/catch anywhere on this path (the escaping stack above is the proof).
  • #64706 hardened the size read itself (>>> 0), which fixes the negative/garbage-size case. What remains — and what the repro above hits on v24.21.0 — is the rest of the path: any stdout bytes that begin with FF 0F plus a plausible small size pass every guard, readHeader() throws on the payload, and the exception takes down the entire run.
  • Binary output containing FF 0F at a buffer head reaches exactly this state; plain text mostly takes the negative-size branch instead, which pre-#64706 silently drops buffered frames (see #65934) rather than crashing.

Real-world impact: one test's stdout can abort the whole node --test run with an internals-only error, which is hard for users to act on. We hit this as CI flakiness: a test printed a large terminal-QR block graphic and CI failed intermittently with the identical error; removing that output made the failures disappear.

Possible directions (not exhaustive): re-validate the FF 0F magic in the second loop before trusting the size; treat readHeader() / readValue() failure as lost/stray-output diagnostics for that file instead of a fatal exception; or an escaping mechanism so user stdout is never frame-parsed.

Related

  • #64061 — signed size-read crash (same error/stack); fixed by #64706. This issue is about what that fix does not cover.
  • #65934 — pending v22.x backport of #64706.

Secondary repro (plain text, intermittent)

The deterministic file above is a minimized stand-in. The organic trigger is chunk-boundary dependent — text landing in the same parse buffer as the end of a frame gets its bytes read as a size. This file matches our real-world case (QR-style block text) and crashes only intermittently:

import { test } from 'node:test'

const tick = () => new Promise((r) => setTimeout(r, 0))

test('passes some ordinary assertions', async () => { await tick() })

test('prints a large block of text like a QR code / ASCII banner', async () => {
  for (let i = 0; i < 54; i++) {
    console.log('█▀▄ '.repeat(25))
    await tick() // let runner event frames flush between text writes
  }
})

test('keeps running tests after the noisy one', async () => { await tick() })
Dominant language
JavaScript
Stars
122k
Forks
37.4k
Avg merge
4d 3h
Merged PRs (30d)
273

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from nodejs/node

All issues in nodejs/node

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.