fix: skip non-event persistence records and enforce body-size limit in bytes - #62
Merged
Merged
Conversation
…n bytes Two confirmed bugs found via a coverage-guided property/fuzz campaign against the public HTTP and --persist seams, each with a minimized, reproducible counterexample and a regression test. 1. Startup crash on a `null` persistence record (src/persist.ts) FileStore.loadState() parsed every non-empty line as JSON and replayed it unconditionally. A single valid-JSON `null` line reached Manager.applyLoadedEvent, which dereferenced `null.enqueue` and threw a TypeError that aborted server startup before it could listen. Root cause: no event-shape validation at the persistence decode boundary. Fix: an isQueueEvent() type guard rejects non-conforming records (non-objects, wrong field types, or enqueue==dequeue) so they are skipped like malformed JSON. Regression test starts a real main.ts --persist process against a `persist.dat` containing `null` and asserts /health and /queues still respond. 2. Multi-byte UTF-8 body evades the 1 MiB size limit (src/handler.ts) readRequestBody's backstop checked `body.length` after `request.text()`, which counts UTF-16 code units, not wire bytes. A chunked (no Content-Length) UTF-8 body of 1.2 MiB bytes but 400K code units bypassed the byte limit and was enqueued with 200 instead of 413. String bodies were unaffected because Deno sets Content-Length to the byte length, so only chunked requests evaded the check. Root cause: the limit was measured in code units, not bytes. Fix: re-measure via LOG_ENCODER.encode(body).length (bytes), reusing the existing module-level TextEncoder so no new coupling is introduced. Regression test sends a streaming multi-byte body with no Content-Length and asserts 413. Diagnosis per /diagnosing-bugs: tight red-capable loop -> minimized repro -> ranked falsifiable hypotheses -> confirmed root cause -> fix + regression test -> original repro no longer reproduces. Verified locally against all CI jobs: - deno test --allow-read --allow-write --allow-net --allow-env --allow-run: 374 passed - mutasaurus: 99% overall, all files >=80% (handler 100%, persist 100%) - stryker: all files >=80% (handler 89%, persist 84%), stryker_check exit 0 - production quality gate: exit 0, no new warnings - deno compile main.ts: exit 0 Co-Authored-By: Claude <noreply@anthropic.com>
CI Stryker dropped persist.ts to 72% (35/127 survived) while local ran at 84%: the 15-mutant gap concentrated in the new isQueueEvent() guard, whose branches were only exercised indirectly through slow, timing-dependent tests (e2e server startup, file-lock subprocess). Under Stryker's concurrency-2 load on CI those kills became unreliable. Export isQueueEvent and add pure, I/O-free unit tests covering every guard branch (valid events accepted; null/array/primitive/non-string queue/missing payload/non-boolean flags/enqueue==dequeue rejected), plus a loadState test that mixes invalid and valid records. These kill the guard mutants deterministically on both local and CI. Expected to bring persist.ts to ~84% on CI (107/127), above the 80% gate. Co-Authored-By: Claude <noreply@anthropic.com>
jonbaldie
added a commit
that referenced
this pull request
Aug 27, 2026
Merge origin/main (PR #62) into worktree-seeking-performance: - Adopt isQueueEvent<T>() validation in stream-parsing loadState - Add close() to non-event JSON test to fix file handle leak Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two confirmed bugs found via a coverage-guided property/fuzz campaign against the public HTTP and
--persistseams. Each has a minimized, reproducible counterexample and a regression test.Bug 1 — Startup crash on a
nullpersistence record (src/persist.ts)FileStore.loadState()parsed every non-empty line as JSON and replayed it unconditionally. A single valid-JSONnullline reachedManager.applyLoadedEvent, which dereferencednull.enqueueand threw aTypeErrorthat aborted server startup before it could listen.persist.datcontaining the four bytesnull→main.ts --persistexits with code 1 and no listener.isQueueEvent()type guard rejects non-conforming records (non-objects, wrong field types, orenqueue == dequeue) so they are skipped like malformed JSON.main.ts --persistprocess againstnulland asserts/healthand/queuesstill respond.Bug 2 — Multi-byte UTF-8 body evades the 1 MiB size limit (
src/handler.ts)readRequestBody's backstop checkedbody.lengthafterrequest.text(), which counts UTF-16 code units, not wire bytes. A chunked (noContent-Length) UTF-8 body of 1.2 MiB bytes but 400K code units bypassed the byte limit and was enqueued with200instead of413. String bodies were unaffected because Deno setsContent-Lengthto the byte length, so only chunked requests evaded the check.fetchbody of{"payload":"一".repeat(400000)}(1,200,014 bytes, 400,014 code units), noContent-Length→200.LOG_ENCODER.encode(body).length(bytes), reusing the existing module-levelTextEncoderso no new coupling is introduced.Content-Lengthand asserts413.Diagnosis
Followed
/diagnosing-bugs: tight red-capable loop -> minimized repro -> ranked falsifiable hypotheses -> confirmed root cause -> fix + regression test -> original repro no longer reproduces.CI verification (local)
deno test --allow-read --allow-write --allow-net --allow-env --allow-run: 374 passeddeno compile main.ts: exit 0🤖 Generated with Claude Code