Repository navigation
fix(plugin-hono-server,hono,cloud-connection)!: refuse an unparseable request body with 400 INVALID_REQUEST instead of reading it as {} - #24005
Conversation
… 400 INVALID_REQUEST instead of reading it as {}
WIP: transports only; tests follow.
Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT
Co-authored-by: Claude <noreply@anthropic.com>
…tes with 400 INVALID_REQUEST Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT Co-authored-by: Claude <noreply@anthropic.com>
…te doors it leaves alone Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT Co-authored-by: Claude <noreply@anthropic.com>
…-hono-server Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT Co-authored-by: Claude <noreply@anthropic.com>
…zero writes on install-local Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT Co-authored-by: Claude <noreply@anthropic.com>
… changes Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT Co-authored-by: Claude <noreply@anthropic.com>
…here the route-envelope gate counts it Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 3 package(s): 15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 4c6f06fb65c34169ba14e5e75024230b7974f919 && git checkout 4c6f06fb65c34169ba14e5e75024230b7974f919
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 92fd343bcf986fbd8c7cbc9a3824d8096573f3d5 97d881a2e743bf4ab3ee4d5bf4ea7071942cc41c && git checkout -B drift-repro 92fd343bcf986fbd8c7cbc9a3824d8096573f3d5 && git merge --no-ff 97d881a2e743bf4ab3ee4d5bf4ea7071942cc41c
node scripts/docs-audit/affected-docs.mjs --json 92fd343bcf986fbd8c7cbc9a3824d8096573f3d5
|
|
Seat note:
Generated by Claude Code |
Contract reviewServed-tier: Net diff against the merge base ① Derived judgments1. plugin-hono-server,
2. The transport's 400 wins ( 3. Envelope. Right. 4. The dispatcher bridge ( 5.
6. cloud-connection ( 7. Legitimate callers. None found refused. 8. Exports and 9. The replaced fixture row ( ② Semver levelThree Sentences against the diff:
③ Boundary flags
Remedy, for the seat to choose, not this record: either the self-hosted plugin-webhooks mount (and the trigger-api hook mount, the same class) answers the transport's refusal with the same code and message, one wire spelling, with the conformance row re-pinned to those bytes; or Implemented-by: VERDICT: FAIL Generated by Claude Code |
…it, not before dispatch() A domain that forwards the request (webhooks redeliver, inbound hooks, approvals act) reads its own bytes and answers as before; the dispatcher's gates run before the refusal, as on plugin-hono-server. Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT Co-authored-by: Claude <noreply@anthropic.com>
…est and the method rows stated as they are Claude-Session: https://claude.ai/code/session_018t6a5rvqPRvEDMCVVW9kYT Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Net diff against the merge base ① Derived judgments1. plugin-hono-server,
2.
3. cloud-connection ( 4. Legitimate callers. None refused. The SDK sets 5. Envelope. Right. 6. Published 7. Pins. All pin the new behaviour; none loosens an assertion.
8. Residuals, non-blocking. A domain that caught the read's throw and still wrote would be answered 400 after writing; none found ( ② Semver levelThree Sentences against the diff:
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
席位核对ACCEPT · #23914 ·
Follow-ups owed by this seat (filed in its next filing window; this window's three are #24023, #24024, #24025):
Landing: Generated by Claude Code |
Fixes #23914
Clause-②: no (narrowing)
What
A request body that an HTTP door cannot parse is now refused with
400 INVALID_REQUESTand the ADR-0112 envelope. It is no longer handed to the handler as{}.INVALID_REQUESTis an existingERROR_CODE_LEDGERcode; no code or ledger row is added. The message names the failure and never echoes the body.@objectstack/plugin-hono-serverHonoHttpServer.runHandler(every route registered throughIHttpServer, plus the fallback seam): theobjectstack dev/os servedoorreq.bodyis an accessor. Reading a body the adapter could not parse throws the refusal, and the adapter answers it whatever the handler made of the throw@objectstack/honocreateHonoApp'sreadDispatchBody, used by the${prefix}/*catch-all and the/auth/*legacy fallbackapplication/jsonthat is not blank and does not parse is refused when the dispatched domain READS it, after the dispatcher's gates; a domain that forwards the request reads its own bytes and answers as before@objectstack/cloud-connection/bind/start,/bind/poll,/install, andmarketplace/install-localreadJsonRequestBodyMeasured at the door
These are fresh
objectstack dev --seed-adminshowcase boots, base2054a0888cagainstb9df5b1d3(origin/mainmerged in; the same answers were read on0660b5a16before the merge). This door isplugin-hono-server, which is byte-identical at the current head97d881a2e. Every body was sent from a file withcurl --data-binary. The admin session came fromPOST /api/v1/auth/sign-in/email.POST /data/showcase_account/query{"where": {"status": "churned"}}INVALID_REQUEST"The request body could not be parsed as JSON (it was sent as application/json). …"{ not jsonINVALID_REQUEST, same messagetext/plainINVALID_REQUEST"The request body was sent as text/plain, which this server does not read into a request body. …"PATCH /data/showcase_account/ID{"name": "HACKED"updated_atbumpedINVALID_REQUEST; name andupdated_atunchangedPOST /data/showcase_accounttruncated, withnameVALIDATION_FAILED"Account Name is required"INVALID_REQUEST, the parse refusalapplication/jsonPOSTwith no body and noContent-TypePATCH/ valid create / empty-body createVALIDATION_FAILEDVALIDATION_FAILEDPUTof a.txtfile astext/plain, then completePUTof a JSON-with-comments file asapplication/json, then complete0660b5a16):PUT /meta/object/showcase_account,POST /keys, truncatedINVALID_REQUEST, the parse refusalPOST /cloud-connection/bind/polltruncated{"device_code": "dc_1"/{}POST /marketplace/install-localtruncated{"packageId": "com.example.crm"INVALID_REQUEST, the parse refusalOn this boot the door is served by
plugin-hono-server.createHonoApp(@objectstack/hono) is the embed door, and nothing inpackages/climounts it. Both transports carried the swallow, so both change, as triage asked.Why
plugin-hono-serverrefuses whenreq.bodyis read, not before the handler runsThe PM's mechanism assumption 3 ("who else relies on
{}") was measured, and it moved the design. One transport serves JSON doors and byte doors, and only the route knows which one it is:headers: { "content-type": "application/json" }for a.jsonfile, so the platform's own upload flow PUTs a JSON-with-comments file (atsconfig.json, say) as declared JSON that does not parse. The raw-PUT route readsreq.rawBody()and stores it with200(measured on both boots above). objectui'sUploadProvidersendsContent-Type: mimeTypeon that PUT.text/plain(attachments-authz-harness.ts,field-file-collection, and others).Refusing before the handler would have turned both uploads into
400s. Raised on read, the refusal reaches exactly the handlers that would otherwise have acted on an invented{}, and it stops them at the read, so nothing after it runs and nothing is written. Body presence for the media types the adapter does not read is judged by the RFC 9112 framing headers (Content-Lengthother than0, orTransfer-Encoding). Reading the bytes to find out would buffer an upload before storage's declared-length check refuses it. ReadingRequest.bodycannot answer either, because@hono/node-servergives every non-GET/HEAD request a stream.One rule on both transports: the reader refuses (seat ruling B)
createHonoAppfollows the same rule. Its body is adispatch()argument rather than areqproperty, so for a declared-JSON body that does not parse,readDispatchBodyhandsdispatch()a body value whose every read (a property,in, its keys, a spread,JSON.stringify) throwsRequestBodyRefusaland records that it was read. Each arm (the${prefix}/*catch-all and the/auth/*legacy fallback) then answers that refusal if, and only if, the body was read, whatever the domain answered. This is the same overrideplugin-hono-serverapplies after itsreq.bodygetter. There is no new export and no list of routes: whether a domain reads the body is answered by the domain reading it.dispatch()itself never readsbody. Its liveness short-circuit, scope resolution, the ADR-0069 auth gate and project membership read the path and headers, andbodygoes straight to the domain's handler. So the gates still answer first, as they do ahead of a route handler onplugin-hono-server.body:/webhooks/redeliver, the inbound automation hooks, and the/approvals/actformPOST. It answers exactly as at base, a malformed body included, after its own sign-in or signature check.hono-webhooks-redeliver.conformance.test.ts'snot JSONrow, which holds the hosted door byte-equal to the self-hosted mount, stays as pinned and is green.b9df5b1d3refused beforedispatch()instead. That reddened exactly that row: the hosted door answered this adapter's message where the mount answers the member's"Request body must be JSON.". It also answered an anonymous caller400where the mount says401. The seat's contract review ruled B.Step 4 (
text/plain): refuse, not parsecontent/docs/api/wire-format.mdxdocuments the contract: "All requests and responses useapplication/json", andContent-Type: application/jsonis a required request header. Parsingtext/plainas JSON would be a lenient second dialect. Onplugin-hono-serverit would also mean reading every text upload eagerly. So step 4 is refused there.createHonoAppwas already content-type-blind (it parses atext/plainbody as JSON when it parses), and that is unchanged. See Acceptance notes.Which doors treat an empty body as
{}, legitimatelyAll three keep it. A blank or whitespace-only body declared as JSON, and a request with no body whatever it declares, are still
{}. The REST query handler documents the reliance ("An empty request body is notnull: the HTTP adapter hands it over as{}, an empty query"). Bodiless actionPOSTs rely on it too (/bind/start,/automation/NAME/trigger). The empty-body controls above and in every suite pin it.Sibling sweep (lane
domain:cli)The sweep covered
.json().catch(,catch {}around a body parse, andreturn {}from a body reader, overpackages/{cli,runtime,verify,types,rest,mcp,observability,client*,cloud-connection,create-objectstack,adapters/*,qa}plusplugin-hono-serverandplugin-dev.In scope (a door that takes a client body), changed here:
plugin-hono-server/src/adapter.tsrunHandler: thec.req.json()/parseBody()pair.adapters/hono/src/index.tsreadDispatchBody: both{}fallbacks, which serve both arms.cloud-connection/src/cloud-connection-plugin.ts:/bind/start,/bind/poll,/install.cloud-connection/src/marketplace-install-local-plugin.ts:handleInstall.Out, with the reason:
cloud-connection-plugin.tstokResp.json().catch,bindResp.json().catchand the install relay'sresp.json().catch: these read the cloud control plane's answers, a body the platform wrote itself.packages/qa/http-conformance/src/adapter.tsNodeHttpServer(catch { body = {} }):private: true, ships nothing, and serves only the conformance suite's own requests. It is a reference harness, not a deployed door (Acceptance notes).packages/qa/http-conformanceitself (this lane) gainssrc/hono-body-refusal.conformance.test.ts, which pins ruling B against the real dispatcher (Tests).runtime/src/domains/meta.ts:1314: a comment, not a body reader. It describes the catch-all's pre-runtime + hono: an exact /approvals/act dispatcher domain that forwards to the approvals service member, and a catch-all that stops consuming non-JSON bodies (segment 2 of ruling A on #22438) #22576 line. Its point, that falsy JSON values arrive falsy, still holds, someta.tsis untouched.Not in this lane, so not changed here: the same swallow in
plugin-auth, measured at the door. TruncatedPOST /api/v1/auth/admin/unlock-user{"userId": "usr_nobody"answers400 "userId is required". That is reported to the seat.plugin-webhooksalready refuses ("Request body must be JSON.").metadata's HMR reload reads a body the CLI's own watch loop posts.Tests
All runs are under
scripts/pm/os-verify-lock.sh, with each exit code captured before any pipe. The current head is97d881a2e:b9df5b1d3(which mergesorigin/mainatadce19297) plus ruling B in@objectstack/hono, its pins, and the correctedhonochangeset.plugin-hono-serverandcloud-connectionare byte-identical tob9df5b1d3, so their runs there stand.Ruling B on
0cee7c3d9(the code of97d881a2e, whose last commit only rewrites thehonochangeset):pnpm --filter @objectstack/hono exec vitest run --maxWorkers=2: 9 files, 180 passed.pnpm --filter @objectstack/http-conformance exec vitest run --maxWorkers=2: 14 files, 150 passed, including the redeliver row as pinned.pnpm --filter @objectstack/hono typecheckandpnpm --filter @objectstack/http-conformance typecheck: exit 0.@objectstack/http-conformanceat the merge baseadce19297(a detached compare worktree, closure built there): 13 files, 144 passed. Atb9df5b1d3it was 1 failed, 143 passed:hono-webhooks-redeliver.conformance.test.tsline 273, rownot JSON.pnpm turbo run build --filter='!@objectstack/docs' --concurrency=2on97d881a2e: exit 0, 75 tasks.After the merge (AGENTS.md §9/§10):
pnpm install --frozen-lockfileexit 0;pnpm turbo run build --filter='!@objectstack/docs' --concurrency=2exit 0, 75 tasks;pnpm --filter @objectstack/spec check:generatedexit 0.pnpm --filter @objectstack/plugin-hono-server typecheckonb9df5b1d3: exit 0, includingcheck:test-typecheckOK.@objectstack/honoexit 0 and@objectstack/cloud-connectionexit 0, eachtsc --noEmitplus-p tsconfig.test.json.On
b9df5b1d3,pnpm --filter @objectstack/plugin-hono-server exec vitest run --maxWorkers=2: 30 files, 386 tests passed.pnpm --filter @objectstack/hono exec vitest run --maxWorkers=2: 9 files, 176 passed.pnpm --filter @objectstack/cloud-connection exec vitest run --maxWorkers=2: 46 files, 561 passed.New pins:
plugin-hono-server/src/request-body-refusal.test.ts(20 cases):ApiErrorSchema/envelopeViolations, the handler never acting, and no echo;adapters/hono/src/hono-body-refusal.test.ts(18 cases), over a fixture dispatcher with a READER domain (readsbodyinside atrywhose catch answers its own 500) and a FORWARDER (reads the forwarded request):dispatch();/auth/*fallback,DELETEincluded;HonoHttpServer.packages/qa/http-conformance/src/hono-body-refusal.conformance.test.ts(6 cases), on the real dispatcher and data domain throughcreateHonoApp:HonoHttpServerroute;hono-catch-all-raw-request.test.tsgains the case that a declared-JSON body that does not parse still reaches a forwarding domain unread, byte for byte.cloud-connection: four refusal cases plus an empty-body CONTROL inerror-envelope.conformance.test.ts. Inmarketplace-install-local-install-modes.test.ts, a refusal with zero writes (no register, no sync, no import, no ledger entry), plus a CONTROL.hono-catch-all-raw-request.test.ts's['invalid JSON', 'application/json', '{ not json', {}]row pinned the branch this removes, so it is replaced. The refusal moved to the new file, and the row now pins the undeclared spelling, which is unchanged.Ablation (one per package; every suite resolves its subject from source, so no rebuild)
Each leg ran through
node scripts/ablation-replace.mjsin WRAP mode, with the anchor hitting once, the write proven on disk, and the restore proven.return {}a331a0f6ed81todee7fcca50450cee7c3d9)refusedOnRead(…)becomesreadable({})fb199e0ebefbto9c5714c12488hono: 10 failed (every reader refusal, the escaping read,/auth/*, parity), 8 passed. http-conformance: 4 failed (steps 2, 3, 5 and 6, parity), 9 passed (the order case, CONTROL, and every redeliver row)dispatch()(theb9df5b1d3shape, on0cee7c3d9)refusedOnRead(…)becomes an immediate throwfb199e0ebefbto4f27b2629aadhono: 7 failed (the reader-ownership cases,/auth/*, the forwarder), 11 passed. http-conformance: 2 failed (the redeliver byte-parity row and the gate-order case), 11 passed2d952cae1, the file unchanged since)ok: falseparse refusal becomes{ ok: true, body: {} }61561f4f3e17to7c27524a66d0The plugin-hono-server leg ran on
0660b5a16, and its file is byte-identical at the current head. Restore proof after each run:git diff HEADis 0 bytes andgit status --porcelainis 0 lines. Each file'sgit hash-objectequals itsHEAD:blob (a331a0f6ed81…,fb199e0ebefb…,61561f4f3e17…), and the marker count is 0.Gates
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackon97d881a2e(no paths, merge baseadce19297): 66 families. All 66 ran and every one exited 0, includingcheck:route-envelope,check:dispatcher-error-vocabulary,check:error-status-conformance,check:nul-bytes,check:published-files,check:dts-closure,check:test-source-alias,check:cross-package-test-inputs,check:adr-0087-registration,check:empty-changesetandcheck-changeset-no-major.--ranreconciliation withCOMMAND :: exit CODErecords: "66 derived, 66 run, 0 NOT-MEASURED, 0 UNRUN".scripts/check-route-envelope.mjschanged onmainafter the merge base (d132140eb, [maintainer] refactor(rest): splitpackages/rest/src/rest-server.ts(13,701 lines, 70 commits in 30 days) along itsregister*Endpointsseams, one move-only slice per PR,/metaafter #22877 stage 2 #23050 slice 10). That change only moves declared counts fromrest-server.tsto the newui-routes.ts, and this diff touches neither. The branch is merge-clean againstorigin/main, so it is not merged (git merge-treeexit 0).pnpm --filter @objectstack/http-conformance test(the suite that went red in CI'sTest Core (3/6)onb9df5b1d3): baseadce1929713 files, 144 passed; head 14 files, 150 passed.pnpm exec eslint --no-inline-configover the 12 changed.tsfiles: exit 0, no findings. The repo-wide run is CI'sLint & Repo Gates.0660b5a16, the first sweep reddenedcheck:route-envelope: the newcloud-connectionhelper built a response, so the gate wanted aPLUGIN_ROUTE_MODULESrow. That script is outside this card's file surface, so the helper now answers a message and each declared route module builds its own400body.Acceptance notes
createHonoAppreads every content type as JSON. This is pre-existing and unchanged. A JSON body sent astext/plainis parsed (step 4 is honoured there, not refused), and a non-JSON body that does not declare JSON is{}. The reader-refuses mechanism could carry that input too, but it would narrow a second input and is not this card's. Carrier: none.plugin-hono-server/src/request-body.ts,adapters/hono/src/index.tsandcloud-connection/src/request-json-body.ts, each module-private. A shared export would enlarge a package's public surface, which this card'sClause-②: no (narrowing)does not declare. The hono parity case holds the two transports' answers equal.@objectstack/http-conformance'sNodeHttpServerkeeps itscatch { body = {} }. It is a private reference harness. The suites do drive a malformed body:hono-webhooks-redeliver'snot JSONrow and the newhono-body-refusalfile, both through the two Hono doors. Neither drives one throughNodeHttpServer. (The v1 sentence said no suite drives a malformed body at all, which was false.)runtime/src/domains/meta.ts:1314's comment still quotes the catch-all's pre-runtime + hono: an exact /approvals/act dispatcher domain that forwards to the approvals service member, and a catch-all that stops consuming non-JSON bodies (segment 2 of ruling A on #22438) #22576 line,c.req.json().catch(() => ({})). Its subject (falsy JSON values arrive falsy) is still true.createHonoApp, and a route's own checks onplugin-hono-server. Oncloud-connection's/bind/startand/bind/poll, the body is read before the session check, as it always was. Neither refusal discloses anything.Contract review
Clause-②: no (narrowing): the seat serves theCONTRACT_REVIEW_TIERreview before the queue. Three changesets (.changeset/23914-*.md) areminorunder the launch-window convention for breaking changes. Each carries theClause-②line, an ADR-0087 marker of kindnot-required (no-migration-prescription)with the other categories closed on facts, and the FROM / TO for an integrator.Generated by Claude Code