feat(server): wire-input hardening — frame-shape guards, inbound limits, sanitized execute errors (ADR-0012)#15
Merged
Conversation
Per ADR-0012 (to be written): - wellFormed() shape-guard runs after decode, before any SQL binding so no arbitrary decoded value reaches lookupTx or sql.exec. Drops + logs bad frames (mirrors the undecodable-frame stance). - maxFrameBytes (1 MiB) checked before decode; mirrors Cloudflare's own cap and makes the bound testable/overrideable. - maxOpsPerMutation (128): reject-don't-truncate at the top of handleMut; a partial apply would silently drop client writes. - maxSubsPerSocket (256): cap checked in handleSub; re-subs (same subId) replace rather than count against the cap (SubscriptionRegistry.add semantics). Over-limit → reset + console.error. - SubscriptionRegistry.countFor(ws) accessor added for cap enforcement. - Execute errors sanitized: full detail logged server-side, generic "mutation failed"/"command failed" + EXECUTE_FAILED sent to client. Authorize errors in handleMut kept user-facing unchanged (README API). - plan-001 error-paths test updated: command boom test now asserts the generic message/code and verifies the detail does NOT leak. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds tests/wire-hardening.test.ts pinning the ADR-0012 invariants: 1. Malformed frame shapes are silently dropped; socket survives. 2. ops over maxOpsPerMutation → LIMIT_EXCEEDED, nothing applied. 3. Subscription cap: third sub on 2-cap DO gets reset; first two subs continue receiving deltas. 4. Oversized frame (>1 MiB) dropped before decode; socket survives (workerd passed the frame to webSocketMessage — the DO's own maxFrameBytes guard fired at 1,048,666 bytes). 5. Execute error → generic "mutation failed"/EXECUTE_FAILED; SQLite constraint detail does not leak; authorize error passes through. Also fixes the wellFormed shape guard to treat null == absent for optional fields (the client transport sends null, not omit, for absent fields in MessagePack serialisation — previous commit missed this and broke all existing sub-bearing tests on the full suite). LimitsTestDO (maxOpsPerMutation=2, maxSubsPerSocket=2) added to tests/test-worker.ts, wired in vitest.config.ts and tests/env.d.ts, following the MaintTestDO/SlowTickDO pattern exactly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Records decisions from plan-005: shape-guard drop policy (drop+log, no
reply — mirrors undecodable bytes), the three limits and their
defaults/override pattern, execute-error sanitization with authorize
exempt (mut path), and the explicit deferral of dedup identity binding.
Also applies one finding from the codex adversarial review (gpt-5.5):
Finding 3 — APPLIED: JSON.stringify(decoded) in the wellFormed drop
path could throw on bigint values (MessagePack useBigInt64 may decode
bigints). Fixed by using a replacer that stringifies bigints as strings,
with a fallback to String(decoded) on any other error. The drop+log
behaviour is now crash-safe for all decoded values.
Rebuttals for findings NOT applied:
Finding 1 — NOT a gap: replay stores the SANITIZED message ("mutation
failed") because rejectTx writes the generic string to recordTx before
the dedup record is written. Old records (pre-upgrade) with SQLite detail
would only replay if they exist at deploy time; that is a deploy-time
concern, not a code gap.
Finding 2 — BY DESIGN: "no mutation handler for '...:...'" is the
authorize-path catch, kept user-facing per the plan (README: "throw to
deny"). Exposing the collection:type name is consistent with the library's
author-configuring-the-DO model; the attacker is already authed. Noted
in ADR for future review.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Test 4 ("oversized frame is dropped") previously only filtered for
`rejected` frames referencing txId "wh-big1", which passes even when
the guard is disabled (the DO processes the payload and emits
`committed`, not `rejected`).
Two stronger assertions added:
- `expect(droppedFrames).toHaveLength(0)` — this socket has no
subscriptions so ANY frame arriving signals the guard failed;
a `committed` frame now causes the test to fail correctly.
- `runInDurableObject` DB check: `SELECT COUNT(*) FROM messages
WHERE id='big'` must be 0 — the strongest signal the write
was never applied.
Verified falsifiability: temporarily setting `if (false && byteLen >
this.maxFrameBytes)` causes the test to fail with
"expected [{ t: 'committed' }] to have length 0"; restoring the guard
makes it pass. Full suite: 183 tests, 43 files, all pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 13, 2026
Merged
grrowl
added a commit
that referenced
this pull request
Jun 13, 2026
Bug-fix batch from the deep-audit follow-up (audit-excluded SSR topic aside): client reconnect-wedge fix (#12), wire-input hardening + error sanitization (#15, ADR-0012), catch-up N+1 batching (#14), plus expanded error/wire test coverage (#11) and dead-code/identifier-quoting cleanup (#13). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
grrowl
added a commit
that referenced
this pull request
Jun 13, 2026
- transport: express the failed-open reconnect recovery (#12) via the existing scheduleReconnect() rather than the inline form main carried (main had no such method; feat/ssr does) — same behavior, one path. - regenerate package-lock against the merged package.json (0.4.0-dev.0, vendored PR-1564 tarballs). The 0.3.2 fixes (#11–#15) now coexist with the SSR feature: readSyncSnapshot + readChangesSinceFor + wire guards in sync-do; highWaterSeq + per-table indexed reads + quoted trigger DDL in changes; ADR index carries 0011+0012. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
grrowl
added a commit
that referenced
this pull request
Jun 13, 2026
- transport: express the failed-open reconnect recovery (#12) via the existing scheduleReconnect() rather than the inline form main carried (main had no such method; feat/ssr does) — same behavior, one path. - regenerate package-lock against the merged package.json (0.4.0-dev.0, vendored PR-1564 tarballs). The 0.3.2 fixes (#11–#15) now coexist with the SSR feature: readSyncSnapshot + readChangesSinceFor + wire guards in sync-do; highWaterSeq + per-table indexed reads + quoted trigger DDL in changes; ADR index carries 0011+0012. Co-Authored-By: Claude Fable 5 <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
1 participant
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.
Lands plan-005 on main (folded into the 0.3.2 bug-fix batch — see version note below).
Hardens the DO's wire boundary (ADR-0012):
LimitsTestDOsubclass).executethrow returns a generic "mutation failed"; SQLite/internal detail is no longer leaked to the client. Authorize errors still pass through (intentional). Updates 3 assertions inerror-paths.test.tsaccordingly.ADR numbering: kept as 0012 (not renumbered to main's next-free 0011). main has no 0011 — that's feat/ssr's SSR ADR. Keeping 0012 means feat/ssr's identical 0012 aligns on rebase and its 0011=SSR fills the gap, avoiding a renumber collision at reconciliation. main's index intentionally jumps 0010→0012 until SSR lands.
Semver note: this is technically a
feat(new guard behavior), normally a minor. Per maintainer steer ("not precious / one big patch is fine"), it rides the 0.3.2 patch. Say the word to split it out instead.Validated on main + published @tanstack/db 0.6.5: typecheck + full suite (160 passed, incl. wire-hardening.test.ts 5/5) + build green. Incl. the codex-review bigint-stringify fix. Supersedes feat/ssr PR #9.
🤖 Generated with Claude Code