test: an empty-array POST body escapes the e2e unstubbed-request guard #187

Closed
opened 2026-08-09 18:45:47 +02:00 by clawbot · 1 comment
Collaborator

Follow-up from the fourth review of
#185, raised as non-blocking there and
filed rather than dropped.

Problem

In tests/e2e/network.js, the guard that reports unrecognised POST bodies uses
batch.every(...). On an empty array that predicate is vacuously true, so a
POST with body [] is fulfilled with 200 [] instead of being reported as an
unstubbed request. Verified by execution during review - the run stayed green.

This is a hole in the one mechanism whose entire job is to ensure unrecognised
outbound traffic fails the suite rather than passing silently. It is unreachable
in practice today, which is why it did not block the PR, but "unreachable today"
is exactly the qualifier that stops being true later.

Also in scope: a comment that overstates the guard's rationale

tests/e2e/network.js:137-140 claims postData() returns null for
undecodable bodies. That is not quite right.
playwright-core@1.56.0 (lib/client/network.js:89) does
buffer.toString("utf-8") || null, so a binary body is lossily decoded
rather than yielding null; only an empty decode produces null. The guard is
correct and load-bearing either way - both paths end up reported - but the
stated reason is wrong, and the PR author independently disclosed that they
could not reproduce the sendBeacon/Blob case the comment implies.

Correct the comment to describe the two real paths: an empty or absent body
decodes to null and hits the type guard; a binary body decodes lossily to
invalid JSON and hits the catch.

Definition of done

  • A POST with body [] is reported as an unstubbed request and fails the suite.
  • Demonstrated by execution, per the standing rule on this repo: show it green
    before and red after. A fix asserted but not demonstrated does not count.
  • While in there, confirm the sibling cases still report: bodyless POST, scalar
    JSON body, [null] batch. These were verified working during review and must
    not regress.
  • The comment at tests/e2e/network.js:137-140 states the actual mechanism.

Note

Keep this small. Do not redesign the guard, and do not touch TRAILING_WATCH_MS
or the canary - both were deliberately ruled out of scope during review and are
documented as they stand.

Follow-up from the fourth review of https://git.eeqj.de/sneak/AutistMask/pulls/185, raised as non-blocking there and filed rather than dropped. ## Problem In `tests/e2e/network.js`, the guard that reports unrecognised POST bodies uses `batch.every(...)`. On an empty array that predicate is **vacuously true**, so a POST with body `[]` is fulfilled with `200 []` instead of being reported as an unstubbed request. Verified by execution during review - the run stayed green. This is a hole in the one mechanism whose entire job is to ensure unrecognised outbound traffic fails the suite rather than passing silently. It is unreachable in practice today, which is why it did not block the PR, but "unreachable today" is exactly the qualifier that stops being true later. ## Also in scope: a comment that overstates the guard's rationale `tests/e2e/network.js:137-140` claims `postData()` returns `null` for undecodable bodies. That is not quite right. `playwright-core@1.56.0` (`lib/client/network.js:89`) does `buffer.toString("utf-8") || null`, so a binary body is **lossily decoded** rather than yielding `null`; only an empty decode produces `null`. The guard is correct and load-bearing either way - both paths end up reported - but the stated reason is wrong, and the PR author independently disclosed that they could not reproduce the `sendBeacon`/Blob case the comment implies. Correct the comment to describe the two real paths: an empty or absent body decodes to `null` and hits the type guard; a binary body decodes lossily to invalid JSON and hits the `catch`. ## Definition of done - A POST with body `[]` is reported as an unstubbed request and fails the suite. - Demonstrated by execution, per the standing rule on this repo: show it green before and red after. A fix asserted but not demonstrated does not count. - While in there, confirm the sibling cases still report: bodyless POST, scalar JSON body, `[null]` batch. These were verified working during review and must not regress. - The comment at `tests/e2e/network.js:137-140` states the actual mechanism. ## Note Keep this small. Do not redesign the guard, and do not touch `TRAILING_WATCH_MS` or the canary - both were deliberately ruled out of scope during review and are documented as they stand.
clawbot added this to the 1.0.0 milestone 2026-08-09 18:45:47 +02:00
Author
Collaborator

Done in #267 (branch test/issue-187-empty-batch-guard, base next).

batch.length === 0 added to the guard; comment corrected to the two real paths. Nothing else touched — TRAILING_WATCH_MS and the canary left as they stand, ALLOWED_ERRORS still [].

Verified by execution with a throwaway four-POST probe, added for the two runs and removed before the commit:

  • before: # probe [] -> fulfilled, ok 28 - PROBE empty-array batch [] — green, the hole
  • after: # probe [] -> rejected: Failed to fetch, not ok 28 with network: unstubbed request: POST https://e2e-probe.invalid/rpc — red

Siblings reported and failed in both runs: bodyless POST, scalar JSON body, [null] batch. Tests 1-27 stayed green throughout, so the widened ConfirmTx RPC fixtures are not misreported. Full captures are in the PR body.

make check green (576 tests, test-verify-build 18 cases, prettier clean); make test-e2e green at 27/27 with the probe removed.

Done in [#267](https://git.eeqj.de/sneak/AutistMask/pulls/267) (branch `test/issue-187-empty-batch-guard`, base `next`). `batch.length === 0` added to the guard; comment corrected to the two real paths. Nothing else touched — `TRAILING_WATCH_MS` and the canary left as they stand, `ALLOWED_ERRORS` still `[]`. Verified by execution with a throwaway four-POST probe, added for the two runs and removed before the commit: - before: `# probe [] -> fulfilled`, `ok 28 - PROBE empty-array batch []` — green, the hole - after: `# probe [] -> rejected: Failed to fetch`, `not ok 28` with `network: unstubbed request: POST https://e2e-probe.invalid/rpc` — red Siblings reported and failed in both runs: bodyless POST, scalar JSON body, `[null]` batch. Tests 1-27 stayed green throughout, so the widened ConfirmTx RPC fixtures are not misreported. Full captures are in the PR body. `make check` green (576 tests, `test-verify-build` 18 cases, prettier clean); `make test-e2e` green at 27/27 with the probe removed.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#187