Uh oh!
There was an error while loading. Please reload this page.
fix(runtime): refuse an unsupported verb on /metadata/:type/:name instead of serving it as a read (#8848) - #8871
Merged
Conversation
…tead of serving it as a read (#8848) The `parts.length >= 2` block in the `/meta` domain carried exactly one method-sensitive branch — the `PUT` save landed by #8842 — and the read `try` that follows it had no method guard at all. Every other verb fell into it and was answered with the ordinary metadata read. Measured through a real composed host (`createHonoApp`'s catch-all → `dispatch()` → domain registry → this handler), authenticated caller, `/api/v1/meta/object/account`: DELETE → 200, getMetaItem x1, deleteMetaItem x0 PATCH → 200, getMetaItem x1 POST → 200, getMetaItem x1 `DELETE` is the sharpest case: `200` plus the item document is indistinguishable from a successful destructive call, and nothing was deleted. This is the same defect class #8842 closed for the falsy-body `PUT`, reached by a different door — a write verb answered as a read, with no status, header or field telling the caller (AGENTS.md, Route & surface ownership §3 "Absence must be loud"). Not a privilege escalation: the read path runs the ADR-0106 mask, so the caller received exactly what `GET` would return and nothing was written. What was wrong is that the answer lied about which operation happened. The block now answers `405` with `Allow: GET, HEAD, PUT`, aligning it with every other route in this file (which already guard their verb). `HEAD` is in the allowed set because it is measured to be served today — refusing it would regress a working read verb, not restore an invariant. Scope: this REFUSES the verbs, it does not implement them. Mounting a real metadata delete on this transport expands the public surface and needs its own card. `scripts/check-route-envelope.mjs` moves `meta.ts` to `handBuilt: 1`: the gate mechanically requires classifying the hand-built response, and the 405 must be hand-built because it carries an `Allow` header that `deps.error` cannot express — the same reason `domains/mcp.ts` hand-rolls its own 405. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn
…adata-verb-fallthrough
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 20 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also reference the affected code. These are read-only:
|
qq9340100
marked this pull request as ready for review
August 15, 2026 12:46
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes#8848
The
parts.length >= 2block in the/metadomain carried exactly one method-sensitive branch — thePUTsave landed by #8842 — and the readtrythat follows it had no method guard at all. Every other verb fell into it and was answered with the ordinary metadata read.Reachability — measured first, because the card said it was not
The card was explicit that per-verb routing precedence in a fully composed host was not measured, and that
premise_still_valid: falsewas a legitimate delivery. It was measured before anything was built. The premise holds.Driven against a real
createHonoAppapp (the${prefix}/*catch-all →dispatch()→ domain registry → this handler), authenticated caller,/api/v1/meta/object/account, onorigin/mainbefore the fix:createMetaDomainregisters nomethodsrestriction (DomainRoute.methodsis optional, "Omit = all methods"), sodomainRegistry.resolve(path, method)matches every verb.The other composition, enumerated rather than assumed.
RestServer.registerRoutes()registersGET,PUTandDELETEon/api/v1/meta/:type/:name— but notPOSTorPATCH. So where REST is mounted alongside the catch-all (ADR-0076 D11: "REST-shadowed but still catch REST misses"),DELETEis shadowed by REST's real delete, whilePOSTandPATCHstay REST misses and still land here. In a catch-all-only host —n()on Vercel, the documented embed shape — all three land here. Both statements are in the test file's header.What changed
The block now answers
405withAllow: GET, HEAD, PUT, aligning it with every other route in this file, which already guard their verb (thestateandpublishedreads use!method || method === 'GET';_draftsand_migrate-storedguard theirs too). This block was the outlier.DELETEwas the sharpest case:200plus the item document is indistinguishable from a successful destructive call, and nothing was deleted. No status, header or field separated any of those answers from a realGET— the shape "Absence must be loud" exists to prevent (AGENTS.md, Route and surface ownership §3), and §4's "machine-readable surfaces must not lie" is why the refusal carriesAllowrather than only a message.This is not a privilege escalation, and is deliberately not described as one. These verbs were answered by the read path, which runs the ADR-0106 mask, so the caller received exactly what
GETwould return and nothing was written. A request that does not write does not escalate by skipping a write gate. What was wrong is that the answer lied about which operation happened.HEADis allowed, not refused. It is measured above as served today (200,getMetaItemcalled once, transport strips the body). That is correct HTTP for a readable resource, so refusing it would regress a working read verb rather than restore an invariant. The allowed set is spelled once inMETADATA_ITEM_METHODSso the header and the message cannot drift apart.Scope
⛔ This refuses the unsupported verbs; it does not implement them. A real metadata delete exists (
protocol.deleteMetaItem) and REST already exposesDELETE /api/v1/meta/:type/:name, but mounting it on this transport expands the public surface and needs its own card. Per the ruling on the issue, only the invariant restoration is in scope here.The card's surface was
packages/runtime/src/domains/meta.ts+ its tests. This PR also touchesscripts/check-route-envelope.mjs— one ledger entry, movingmeta.tsfromhandBuilt: 0tohandBuilt: 1with the required note.It is mechanically forced, not chosen. The 405 must be hand-built because it carries an
Allowheader anddeps.errortakes no headers — the same reasondomains/mcp.tshand-rolls its own 405, which the gate's own table already classifies that way. Measured before editing the gate:The gate treats an unclassified hand-built response as an error by design, so the fix cannot land without this line. The body still goes through the one builder (
buildApiError) and the error code is derived from the status (METHOD_NOT_ALLOWED) rather than spelled, matching the other 405 sites.Tests
packages/runtime/src/domains/meta-verb-fallthrough.test.ts— 12 cases, drivingdispatch()(nothandleMetadata()) for the routing-level cases so the domain-registry lookup is inside the pin.DELETE/PATCH/POSTasserts the full ADR-0112 envelope —statusanderror.code— plus theAllowheader, plusgetMetaItemnever called, plussaveMetaItem/deleteMetaItemnever called. Also the compound-name form, a caller with no authoring capability, and that the document no longer rides in the body.GETstill reads,HEADstill reads, an absent method still defaults to the read,PUTstill saves,PUT's [finding] http-dispatcher: a PUT /metadata/:type/:name with a falsy body falls through the save branch and is served as a READ #8842 capability gate still answers first (an ordering pin — were the guard moved ahead of the save branch, this would become a 405), and the sibling routes are untouched.Reverse verification, direction predicted in writing before running: plain red on the refusal half only, green on the over-refusal half, no inversion and no diagnostic-count increase, since the guard is a pure addition nothing else reads. Observed exactly that — with the guard reverted to
origin/main, 6 failed / 6 passed, every failureexpected 200 to be 405. The fix was committed first, so restoring wasgit checkout HEAD --on the file;git status --porcelainconfirmed byte-identity afterwards.Verification
All at HEAD
5cb492fb5(the merge commit; tree clean, nothing committed after the union ran).Gate union re-derived from the actual changed paths with
node scripts/pm/dispatch-gates.mjs, which surfaced four families beyond the dispatch list (all changeset-triggered) — every one run, all exit 0:check:type-check-debtfirst refused to run ("24 workspace dependencies have no built type entry point") — recorded as not measured, not as a pass. The full closure was then built (turbo run build --filter='./packages/*' --filter='./packages/*/*', 70/70) and it was re-run to a real result: 33 ledger entries re-measured, 1926 raw errors, none above its recorded number. The one informational note (@objectstack/lintsits 1 below its ceiling) is pre-existing and untouched by this diff.origin/mainwas merged before opening (bringing #8854 and #8857); the tree carries no deferred regen debt.Generated by Claude Code