fix(runtime): stop the /auth domain claiming every path that merely starts with auth - #16265
Conversation
…tarts with `auth`
`createAuthDomain` registered `{ prefix: '/auth' }` with no `match`, and
`DomainRoute.match` defaults to `'prefix'` — `path.startsWith('/auth')`, no
segment boundary. `DomainHandlerRegistry` preserved that rough edge on purpose
when the domains were lifted out of the legacy if-chain, and on this prefix it
claims SIBLING NAMESPACES.
Measured on a real boot before the fix (a real `ObjectKernel` with `AuthPlugin`,
`createHonoApp({ kernel, prefix: '/api/v1' })`, authenticated as the dev admin):
GET /api/v1/authx -> 200 {} claimed
GET /api/v1/authx/foo -> 200 {} claimed
GET /api/v1/authentication/foo -> 200 {} claimed
GET /api/v1/aut/foo -> 404 ROUTE_NOT_FOUND control
GET /api/v1/zzz/foo -> 404 ROUTE_NOT_FOUND control
The route now declares `match: 'segment'`, the spelling the registry's other
boundary-correct domains (`/keys`, `/mcp`, `/mcp/skill`) already use.
The fallthrough is NOT removed and must not be: `/auth/me/permissions` and
`/auth/me/localization` are not better-auth endpoints, so the adapter's
`/auth/*` mount disclaims them and they reach `dispatch()` here (#4088 —
objectui's permission layer reads the former). `'segment'` keeps claiming
`/auth` exactly and everything under `/auth/`, and the new suite pins those
rows as the overshoot control with the same weight as the narrowed ones.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
📓 Docs Drift Check6 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 24 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 62ad6b9a2174f4db3e5b459f252db2bf4d6b1975 && git checkout 62ad6b9a2174f4db3e5b459f252db2bf4d6b1975
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 61362932b5ad4c85b39169e70cf9be64d4332ce5 6a04a501d5bd8301e85612f89fcb51a569d971d2 && git checkout -B drift-repro 61362932b5ad4c85b39169e70cf9be64d4332ce5 && git merge --no-ff 6a04a501d5bd8301e85612f89fcb51a569d971d2
node scripts/docs-audit/affected-docs.mjs --json 61362932b5ad4c85b39169e70cf9be64d4332ce5 |
Contract review of record — PASS, with one rider this seat is upgrading to blockingDispatched by the Provenance and independence
⛔ The reviewer's text below is reproduced verbatim. The seat may adopt it word-for-word or void it entirely; it may not rewrite, trim or polish it. The one place this seat departs is stated after it, as this seat's own call, not as an edit to theirs.
This seat's one departure: rider §6(d) is blocking, and it is the seat's call, not a revision of the reviewThe reviewer graded the rider non-blocking and gave sound reasons. This seat is upgrading it, and owes the reason rather than the ruling: The changeset promises that the shadowing is gone. Mutation (d) shows the pin cannot tell. A future diff that keeps the wide ⭐ It is also, precisely, the failure this lane has been correcting all week and which this seat wrote into its own dispatch template yesterday: a claim with no falsification condition of its own. Requiring one case here costs one short round; not requiring it leaves the PR's headline promise unpinned. Consistency with the standard this seat has been holding other PRs to is the whole argument. The reviewer specified the case exactly — register a probe domain at Everything else in the verdict is adopted as written. Landing sequence once the rider is in: 满页绿 ⇒ strip ⛔ Unchanged and not landing with this PR: the Sibling defect §5(4) — Generated by Claude Code |
…ot merely that auth stops answering
The nine cases in this pin all read the auth service spy: "not called" is how
they conclude the `/auth` domain did not claim a path. The contract review
measured what that cannot see — a repair which KEEPS the wide
`startsWith('/auth')` claim and moves the refusal INSIDE `handleAuthRequest`
passes all nine green, because the service is still never called and the
`ROUTE_NOT_FOUND` envelope is still what comes back. Under that shape `/authx`
is still SHADOWED: a domain mounted there never runs, which is the harm the
card names and the changeset says is gone.
The new case observes the REGISTRY instead. `registerDomainHandler` appends to
a first-match-wins table, so a probe domain registered at `/authx` AFTER
construction sits BEHIND the auth route — exactly where a package mounting
that namespace later would sit — and is reachable only if the auth route
declines the path. The evidence asserted is the probe's OWN response coming
back out of `dispatch()` for `/authx` and `/authx/foo`, not the absence of a
call.
Test-only: no production file changes, and the existing `patch` changeset on
`@objectstack/runtime` is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
Landing provenance —
|
| leg | result |
|---|---|
delivered head 6a04a501d5b |
Tests 10 passed (10) |
mutation (d) — wide claim kept, refusal moved inside handleAuthRequest |
Tests 1 failed | 9 passed (10) |
| restored, re-run | Tests 10 passed (10) |
The single failure under (d) is case 10 alone — AssertionError: expected 404 to be 200 on the probe-reached assertion — with the nine spy cases staying green, reproducing the blind spot exactly. Mutation proven on disk before the run (anchor count 1→0, injected marker 0→1, non-empty git diff --stat, with the editing script aborting as VOID if its anchor did not match exactly once); restore proven by blob identity 824ff7d0… against HEAD plus an empty git diff HEAD, ⛔ never by an exit code.
⭐ A correction to the review's own wording, made by the implementer and worth recording. The review said the pin "does not resolve on a source-only tree." Measured more precisely: the pin's subject resolves from src via the relative import ../http-dispatcher.js, and packages/runtime/dist does not exist in that worktree at all — a src-only mutation flipped the case with no build, which is itself the positive proof of the resolution path. What needs dist is the dependency @objectstack/observability, reached through package exports and not aliased by the vitest config. ⇒ no rebuild leg was needed or used. The review's finding was real; its attribution was one layer off.
⭐ One standing assumption, written down rather than left implicit: the case assumes registerDomainHandler appends rather than prepending or sorting by specificity. Were that order ever reversed, the probe would win regardless of the auth claim and this case would go quiet without failing. Nothing in the file pins that ordering. Naming a case's own silent-failure mode is the right instinct and it is why this is in the PR body.
CI — the full population
36 of 36 complete, every one success or skipped, none failed, none pending, at head 6a04a501d5b; page 2 of the listing empty, which fixes the population.
⭐ New pre-landing check, added this round and applied here first
Before flipping ready, this seat now reads the commit messages that the queue will squash (git log <merge-base>..<head> --format='%B'). Result here, clean:
- commits to be squashed:
59b1001a6d8+6a04a501d5b; - card-relation trailers inside commit bodies: none;
- falsified-claim phrases: none.
The check exists because of what landed an hour ago on #16247: the queue squashes, and squashing concatenates every commit message into main, so two sentences that had been measured false and corrected in the changeset, the PR body and the code comments still went into permanent history through the commit messages nobody re-read. Recorded on #16158. ⛔ "It will be squashed anyway" is a reason a false sentence survives, not a reason it disappears.
⛔ NOT MEASURED — recorded as such, not as green
check-clause2-carriers.mjs --pair 16265 — exit 3, verbatim:
check-clause2-carriers: PREREQUISITE NOT MET — GET /repos/objectstack-ai/objectstack/pulls?state=open&per_page=100&page=1 -> HTTP 403.
⛔ Not a clean board. The carrier strip is evidenced instead by direct read-back on both sides: card #16026 now bug · priority:p2 · pm:dispatched · domain:cli · pm:retriage, and this PR now documentation · size/m · tests · tooling.
Two gate families NOT MEASURED in the implementer's run, both exit 3 PREREQUISITE NOT MET for want of a built workspace closure: check:type-check-debt and check:dual-build-cjs-loads. Their declared narrowing, which is a measurement because it is justified: runtime's tsup entry is ['src/index.ts'] alone and its tsconfig.json excludes **/*.test.ts, so added test code contributes zero bytes to any dist and cannot change what a CJS entry loads. dispatch-gates warned its own derivation came from a tree at least 23 commits behind origin/main, so a family added in that window would not appear in its 54 — CI runs the farm regardless.
⛔ What is NOT landing with this PR
The 200 {} half stays open. Part of #16026, not a closing keyword; pm:retriage stands on the card; options A/B/C are laid out and ⛔ this seat has not changed that card's grade. The review independently confirmed the deferral is coherent: the fix lives in packages/adapters/hono, a different package, and the four still-claimed rows answer exactly what they did before.
Flipping ready and arming. ⛔ Card #16026's pm:dispatched comes off after the merge; pm:retriage stays.
Generated by Claude Code
Part of #16026
Part of, not a closing keyword, and deliberately: the card carries two independent defects and this branch repairs one. The second one's measurement fired the card's own escalation clause and is routed to the PM rather than absorbed here — see "The200 {}" below. The card must stay open.The predicate, and how it was located
Located by symbol and text, never by line number.
packages/runtime/src/domains/auth.ts—createAuthDomainreturned:No
match.DomainRoute.matchdefaults to'prefix', andDomainHandlerRegistry.matches'default:branch is a barepath.startsWith(route.prefix)— no segment boundary. That registry preserved the rough edge on purpose ("match: 'prefix'on/i18nalso matches/i18nxx, exactly asstartsWithdid"), which is why nothing flagged it.Triage narrowed this to the package and the file and said plainly that it had not found the branch. Two corrections to that trail, both worth recording:
http-dispatcher.ts. It is split across the route's declaration (domains/auth.ts) and the registry's default branch (domain-handler-registry.ts). Searchinghttp-dispatcher.tsfor the boundary finds only/auth moved to the domain registry (D11 step ③).HttpDispatcherclasses held exactly as written, and theservice-messagingand hono__mocks__ones are indeed unrelated.The repair
One line: the route declares
match: 'segment'—path === '/auth' || path.startsWith('/auth/'). This is the codebase's own established spelling for this defect, not a new convention:/keys,/mcp,/mcp/skill,/securityand/share-linksalready declare it.⛔ The fallthrough is not removed and must not be.
/auth/me/permissionsand/auth/me/localizationare not better-auth endpoints, so the adapter's/auth/*mount disclaims them and they arrive at this domain (#4088; objectui's permission layer reads the former).'segment'keeps claiming them.Measured on a real boot, before and after
A real
ObjectKernelwithAuthPlugin(a realAuthManagerover better-auth),createHonoApp({ kernel, prefix: '/api/v1' }), authenticated as the dev admin, requests injected through the returned app. The card's seven rows plus the two boundary rows:/api/v1/auth200 {}200 {}(claimed — unchanged)/api/v1/auth/200 {}200 {}(claimed — unchanged)/api/v1/authx200 {}404 ROUTE_NOT_FOUND/api/v1/authx/foo200 {}404 ROUTE_NOT_FOUND/api/v1/authentication/foo200 {}404 ROUTE_NOT_FOUND/api/v1/aut/foo404 ROUTE_NOT_FOUND404 ROUTE_NOT_FOUND(control)/api/v1/zzz/foo404 ROUTE_NOT_FOUND404 ROUTE_NOT_FOUND(control)/api/v1/auth/me/permissions200 {}200 {}(boundary — still reachesdispatch())/api/v1/auth/me/localization200 {}200 {}(boundary — still reachesdispatch())Three rows change, not four.
/authand/auth/are claimed by'segment'exactly as before —dispatch()strips the trailing slash before the registry sees it, and neither is a sibling namespace.The
200 {}— measured, and NOT repaired hereThe card listed "whether the
200 {}is a deliberate empty envelope or an unintended default" as unmeasured. Measured:It is an unintended default, and the mechanism is one layer out of this package.
HttpDispatcherResult.resultis declared as "For flexible return types or direct response objects (Response/NextResponse)", and this domain puts better-auth's realResponsethere. The@objectstack/honoadapter'stoResponseimplements two of that slot's shapes (redirect,stream) and then falls through toreturn c.json(res, 200)— which JSON-stringifies a FetchResponseto{}(it has no own enumerable properties) and hard-codes the status. Measured on the same boot: the auth service answers an honest bodyless404for every row in the table above, including/auth/me/permissions.Nobody designed an empty envelope. The body is empty because a
Responsestringifies to{}, and the status is200because the branch hard-codes it. The adapter's own comment already calls the shape out.That measurement fires the card's escalation clause, so the second defect is reported to the PM for retriage rather than repaired at p2 in this branch. ⛔ Do not read this PR as closing it.
Also measured: the other front door
The card listed
plugin-hono-server-fronted deployments as unmeasured. Measured on the same kernel, and they do not show the shape:/authx,/authx/foo,/authentication/foo,/aut/fooand/zzz/fooall answer404 ENDPOINT_NOT_FOUNDthere, and/auth/me/permissionsanswers200with its real payload. The200 {}is specific to thecreateHonoAppcatch-all. That package was not edited — a sibling card holds it.Tests
packages/runtime/src/domains/auth-claim-segment-boundary.test.ts, drivingdispatch()so the registry lookup is inside the pin. Ten cases in four groups.Nine of them read the auth service's
handleRequestspy: called means claimed, not called means fell through.code+httpStatus+route), never a bare "did not succeed";ROUTE_NOT_FOUND, so a run where everything 404s is distinguishable from the repair;/auth,/auth/me/permissions,/auth/me/localization, and/auth/— still claimed. A repair that stopped claiming/authaltogether would pass group 1 and break the surface this card forbids touching, so the overshoot control carries the same weight as the defect rows.⭐ Group 4 — the registry-resolution case, and why the spy cannot stand alone
The spy observes "the auth service was not called". That cannot separate the delivered repair from one which keeps the wide
startsWith('/auth')claim and moves the refusal insidehandleAuthRequest. Under that shape the service is still never called and theROUTE_NOT_FOUNDenvelope is still what comes back, so all nine cases stay green — while/authxis still shadowed, and a domain mounted there still never runs. That shadowing is the harm the card names and the changeset states is gone, so it needs an observation of its own.The tenth case observes the registry instead.
registerDomainHandlerappends to a first-match-wins table, so a probe domain registered at/authxafter construction sits behind the auth route — exactly where a package mounting that namespace later would sit — and is reachable only if the auth route declines the path. The evidence asserted is the probe's own response coming back out ofdispatch()for both/authxand/authx/foo— a status and a body only the probe produces — not the absence of a call.What falsifies it. The registry resolving
/authxor/authx/footo anything other than the probe; in this fixture the auth route is the only other claimant, so red here means the claim did not stop at the segment boundary. Its one standing assumption is thatregisterDomainHandlerappends rather than prepending or sorting by specificity — were that order ever reversed, the probe would win regardless of the auth claim and this case would go quiet without failing. Nothing else in the file pins that ordering, so it is written down here.Both poles, re-driven at this PR's head
6a04a501d5bDriven here rather than inherited; the pin file's own suite each time.
Tests 10 passed (10)handleAuthRequestTests 1 failed | 9 passed (10)Tests 10 passed (10)Under the mutation the single failure is group 4 and nothing else —
AssertionError: expected 404 to be 200, on the assertion that the probe's response came back — which is precisely the discrimination the other nine cannot make.The mutation was proven on disk before the run (anchor-line count 1 to 0, injected marker 0 to 1, a non-empty
git diff --stat) and the restore proven after it by blob-hash identity againstHEAD(824ff7d0) together with an emptygit diff HEAD— never by an exit code. No rebuild leg was needed, and none was used: the pin imports its subject relatively (../http-dispatcher.js),packages/runtime/distdoes not exist in the tree these runs were made on, and the mutation flipped the case anyway — which is what proves the subject resolves fromsrc. The pin does still need the dependency closure built:http-dispatcher.tsreaches@objectstack/observabilitythrough packageexports, which resolves to that package'sdist.Earlier round, kept for the record
The
matchdeclaration's own reverse verification was measured in the first round at59b1001a6d8, when this file carried nine cases: deleting the declaredmatchgave exactly 3 failed, 6 passed — the three sibling-namespace cases red, both control groups green — with the mutation proven by anchor count and blob hash and the restore proven by blob-hash equality againstHEAD. Those counts belong to that head and that nine-case file, not to the ten-case file above.Changeset
patchfor@objectstack/runtime, unchanged by the test addition. AGENTS.md: "A bug fix in a released package takes apatchchangeset — never none, and neverskip-changeset: that label is for a diff that publishes nothing from any released package."@objectstack/runtimeis released (17.3.0, not private) and this is a bug fix in it, soskip-changesetis refused andpatchis the level — the change narrows a route claim, it removes nothing an author can write, so it is not breaking.Contract note
The diff narrows what a shipped HTTP surface answers: three paths that returned
200now return404. Anything mounted under a first segment beginningauthwas previously shadowed by this domain and is now reachable — called out in the changeset body.🤖 Generated with Claude Code
https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
Generated by Claude Code