fix(sdk): BRC-104 response preimage encodes an empty body as 0 instead of -1, so every bodyless signed response fails verification - #550
Conversation
BRC-104 §6.7.3 requires an absent or empty body to be encoded as a length of -1, and §6.9 lists the response signature preimage's final field as 'Body length + body bytes (or -1 if none)'. AuthFetch's own request side already implements this (writeRequestBody and writeOptionalText both write -1), but writeGeneralResponsePayload encoded an empty body as 0 with no -1 branch. A conforming counterparty therefore signs a preimage this client can never reproduce, so every signed response with no body — a bare 404, 204 or empty 401/403 — fails signature verification with 'Signature is not valid'. Regression tests pin the terminal varint to exactly -1 with nothing following it for an empty body, and to true length plus bytes otherwise; the empty-body case fails against the previous encoding. AuthFetch's response reader already treats a non-positive length as no body, so verified traffic is unchanged apart from now verifying. Signed-off-by: Elis Jackson <elisjackson@icloud.com>
…-integration # Conflicts: # docs/reference/package-api-migrations.md
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ty-everett
left a comment
ty-everett
left a comment
There was a problem hiding this comment.
Reviewed the complete integrated change. Empty response bodies now use the BRC-104 -1 preimage marker; nonempty bodies and request encoding remain unchanged. Public transport vectors, middleware round trips, tampered-status rejection, shared conformance and negative proof against the old code cover the boundary. Full SDK/middleware suites, packed/browser consumers and exact-head CI passed. The real contributor fork now produces required Codecov status at 100% patch coverage, with zero new Sonar findings, no open CodeQL alerts and no unresolved review threads. Existing unpublished SDK 2.8.0 release metadata includes this compatible correction. Main’s subsequent maintenance-only changes will be covered by final combined release acceptance.
2 participants
Summary
SimplifiedFetchTransport.writeGeneralResponsePayloadencodes an empty HTTP response body as varint0. BRC-104 requires-1. A conforming counterparty therefore signs a preimage this client cannot reproduce, and every signed response with no body fails verification — a bare404,204, or empty401/403all die withSignature is not valid.What the spec requires
BRC-104 §6.7.3: "If the body is empty, specify a length of
-1in the payload."BRC-104 §6.9, on HTTP response messages: "Body length + body bytes (or
-1if none)."This repo's own
specs/auth/brc103-mutual-auth.yamlresponse-payload table already statesVarInt(len) or VarInt(-1); only the code and one summary line (also fixed here) disagreed.What the code did
bodyis always an array (Array.from(new Uint8Array(await response.arrayBuffer()))), so an empty body reached this as length0and there was no-1branch. The absent-value paths on the request side (AuthFetch.writeRequestBody,writeOptionalText) already write-1, and this package's ownauth-express-middlewaresigns empty responses with-1(buildResponsePayload), so the two halves of this repo could not verify each other's bodyless responses.Independent confirmation against a conforming server
A conforming Rust implementation (
bsv-auth-axum-middleware,serialize_response_payload) writes-1for an empty body. Run end to end against such a server, an authenticated caller receiving a correctly signed bodyless404gets:Everything else in the same run verifies — non-empty signed responses round-trip fine — so the failure is isolated to the empty-body encoding. Also reproducible standalone against the published 2.6.1 artifact: the terminal varint of the response preimage for an empty body is
0with no trailing bytes.The fix
Encode
-1for an empty body, mirroring the middleware's response side.AuthFetch.parseAuthenticatedResponsealready treats a non-positive length as "no body", so verified traffic parses identically — it just verifies now.Regression tests pin the terminal varint to exactly
-1with nothing following it (with and without a preceding request id), and to true length plus bytes for a non-empty body. Checked against the pre-fix encoding: the empty-body assertions fail on the unfixed code (0 !== -1), so a reversion cannot stay green.Compatibility note
A server that "fixed" this by signing
0instead would move every conforming implementation off the spec — the correct side to change is this client's response verification preimage, which this PR does. Deployed pairs of this SDK talking toauth-express-middlewareare unaffected for non-empty bodies (unchanged) and for empty bodies were already failing.Sibling defect found while verifying this fix — deliberately NOT fixed here
The present-but-empty request body has the same class of bug on both sides of this repo, and the two currently agree with each other on the non-conforming encoding, so they must be fixed together, not in this PR:
AuthFetch.writeRequestBody'sif (!body)guard is truthiness-based, sobody: new Uint8Array(0)or[]skips the-1branch and signs0;auth-express-middleware'swriteBodyToWriterreconstructs the request preimage the same way (Buffer.alloc(0)from an empty raw body writes0, never reaching its-1fallback).In-repo client↔server traffic matches today because both are wrong the same way; either one fixed alone would break the other, and both together are non-conformant against third parties. Happy to follow up with a coordinated PR for that pair if you want it.
Maintainer integration and verification — 2026-09-23
The contributor commit is preserved. Current main is merged into this branch, and the tests now use public transport calls because main made the serializer JavaScript-private. The production correction remains the same empty-response sentinel fix.
3e1488ea94fa34a715e7c1a66afc203791398cf9; BotBoard [BotBoard][active] ty-everett / Codex — release readiness and bounded correctness fixes #558. Draft while hosted exact-head checks complete.pnpm build,health:check,lint,format:check,typecheck,audit:security: passed; no known vulnerabilities.Current main, including #559, is integrated. GitHub's stale base reference was refreshed after it incorrectly made the first hosted coverage run compare unrelated historical changes. Local health, lint, format and typecheck passed again; hosted validation remains pending.
Final fork-reporting verification: head
8173b5cdb835be670d523c9ff1106f776bfc50c8includes merged CI repair #560; health, lint, format and typecheck passed after integration. All package behavior and regressions are unchanged from the preceding green repository run. CI 35809762029 must finish and produce required Codecov status before merging.Final acceptance: all applicable checks on
8173b5cdb835be670d523c9ff1106f776bfc50c8are successful, including CI 35809762029, SDK-auth mutation, required conformance, zero-new-Sonar, CodeQL and 100% Codecov patch coverage from the real tokenless fork upload. No unresolved review threads or open merge-ref CodeQL alerts. The pending statements above are superseded by this result.