Skip to content

fix(hono): the /auth/* mount yields only a 404 that disclaims ownership - #16027

Draft
os-warren wants to merge 1 commit into
mainfrom
claude/issue-15928-hono-auth-mount-yield
Draft

fix(hono): the /auth/* mount yields only a 404 that disclaims ownership#16027
os-warren wants to merge 1 commit into
mainfrom
claude/issue-15928-hono-auth-mount-yield

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes #15928

All readings below are at 5ddff5274, the head of this branch, on a real boot through the adapter: a real ObjectKernel with AuthPlugin (so a real AuthManager and a real better-auth with 100 auth.api entries) booted via @objectstack/verify's bootStack, then createHonoApp({ kernel, prefix: '/api/v1' }), with requests injected through the returned app.

Step 1 — the measurement, before any code

The card recorded the location and the "identical unconditioned yield" reading from a reviewer of #15918 and said plainly that this seat had not measured it. Measured now: the reading is confirmed, and the blast radius at this layer is wider than the plugin's.

GET /api/v1/auth/delete-user/callback?token=abc&callbackURL=/x
  better-auth direct   404  application/json  {"message":"Not found","code":"NOT_FOUND"}
  through this mount   200  application/json  {}

plugin-auth's auth-route-ledger.ts carries POST /api/v1/auth/delete-user and GET /api/v1/auth/delete-user/callback under the disabled disposition precisely because they are published and answer 404 (user.deleteUser deliberately unconfigured, maintainer ruling 2026-08-12). So the ledger's recorded answer was true of the auth service and false on this adapter's wire.

Wider because #15918's defect needed a composition to mount a downstream wildcard, and this one does not. The overwriting layer is registered by createHonoApp itself: the FIELD_PREFIX + '/*' dispatcher catch-all is terminal and answers 200 {} for paths under /auth/. Measured on the same boot, before the fix, POST /api/v1/auth/definitely-not-a-route-1989 and GET /api/v1/auth/me/permissions both came back 200 {} from it.

One correction to the card's framing: the card's own example, POST /api/v1/auth/delete-user, does not produce a 404 on this composition — an ObjectStack guard pre-empts it with 409 "Cannot remove the last local password login", which was returned unchanged both before and after this change. The 404 case is its GET callback half.

The two portability questions, answered by measurement

Does the adapter reach the same auth.api instance? Yes, literally the same object. plugin-auth does ctx.registerService('auth', this.authManager), and kernel.getServiceAsync('auth') from the adapter's side returns _AuthManager with typeof handleRequest === 'function' and typeof ownsRoute === 'function'. getAuthInstance() returns a memoized instance stable across calls (===), 100 auth.api entries, and ownsRoute('POST', '/api/v1/auth/delete-user') answers true from there.

Is the same owns() walk available at that layer? Yes — through the service instance, not by import. @objectstack/hono depends only on plugin-hono-server, runtime and types, so buildBetterAuthRouteOwnership is not reachable and should not be made reachable. What is reachable is AuthManager.ownsRoute(request), the seam the plugin-side fix added. So the fix is a port of the shape, not of the code: the adapter's structural AuthService interface grows an optional ownsRoute?(request), called when present, exactly as the mount already duck-types getPublicConfig?.() and isSsoUsable.

A bounded limitation the port inherits, measured, not assumed. ownsRoute derives better-auth's endpoint path from the auth service's configured basePath, not from the adapter's prefix. Measured: ownsRoute('POST', '/api/v1/auth/delete-user') is true while ownsRoute('POST', '/api/auth/delete-user') is false. A deployment whose two disagree therefore gets false for everything and keeps the pre-change yield — the safe direction, and the reason every undecidable answer here is false.

The trailing-slash divergence the card named, measured at this layer. GET /api/v1/auth/get-session/ and /api/v1/auth//get-session: ownsRoute claims both (true) while better-auth refuses both as unrouted (404). Non-blocking here for the same reason as in the plugin, plus one this layer adds: what those spellings lose is the dispatcher's 200 {}, so the change is from a silent success to the framework's honest 404. Not fixed here — the one-line alignment is the plugin-side follow-up, and no measurement at this layer argues for pulling it in.

The change

packages/adapters/hono/src/index.ts only:

  • AuthService grows an optional ownsRoute?(request: Request): Promise;
  • a local authOwnsRoute(authService, request) helper — every answer that is not a literal true (no such method, a throw, anything else) means yield
  • the yield site becomes if (response.status === 404 && !(await authOwnsRoute(authService, c.req.raw)))

⛔ The mount is untouched and still claims FIELD_PREFIX + '/auth/*'. 401/403 were never yielded and still are not. What narrowed is only which 404 may be handed on.

#4088's contract survives — pinned, and measured on the real boot after the fix

The catch-all is still non-terminal and still ordering-independent; objectui's permission layer reads /auth/me/permissions. After the fix, on the same boot:

GET  /api/v1/auth/me/permissions              ownsRoute=false  ->  200 {}   (unchanged, reaches dispatch())
GET  /api/v1/auth/me/localization             ownsRoute=false  ->  200 {}   (unchanged)
POST /api/v1/auth/definitely-not-a-route-1989 ownsRoute=false  ->  200 {}   (unchanged)
POST /api/v1/auth/admin/remove-user           ownsRoute=false  ->  200 {}   (unchanged)

and the defect case flips: GET /api/v1/auth/delete-user/callback?token=abc&callbackURL=/x now answers 404 {"message":"Not found","code":"NOT_FOUND"} instead of 200 {}.

Pin population — what the new cases cover, and what they do NOT

packages/adapters/hono/src/hono-auth-owned-404.test.ts, 8 cases. Its header carries this in full.

Covered: the adapter's decision logic — which 404s are yielded and which returned, behaviour with a service that has no ownsRoute at all, with one that throws, with one returning a non-true value, that the predicate is not consulted on a non-404, that it is handed the raw request (full wire URL and method, not the stripped subpath), and that a non-default prefix is honoured.

⛔ NOT covered by any case in this file — these are measured above and pinned nowhere here:

  • better-auth's real route table. The fixture's ownsRoute is a path SET, not buildBetterAuthRouteOwnership over a real auth.api; that matcher is plugin-auth's and is pinned there.
  • that the kernel's auth service really carries ownsRoute. Measured on the real boot, not pinned.
  • the basePath/prefix alignment described above.
  • trailing-slash and doubled-slash spellings.

Mutation proof

Fix committed first, then reverted on disk to the pre-change if (response.status === 404) return yieldUnowned(...), under trap ... EXIT INT TERM with absolute paths. The mutation anchor was asserted unique in the form written (1 occurrence) and its landing on disk proved by counting both the removed and the injected text (removed && !(await authOwnsRoute: 0; injected: 1) plus a blob-hash change ba7ea919... -> f037abf0....

The pin went RED — 4 failed, 4 passed — with the headline assertion text:

× does NOT yield a 404 from a path the auth service OWNS — the answer reaches the caller
  AssertionError: expected 200 to be 404 // Object.is equality

which is exactly the defect shape. Restore proved by an empty git diff HEAD, an empty git status --porcelain, and blob equality with both sides non-empty: HEAD ba7ea91924b4310113204b842cd4959c5718a765 = on disk ba7ea91924b4310113204b842cd4959c5718a765.

No rebuild step is claimed for this ablation and none is needed: the pin imports ./index by relative path, so dist/ is not in its resolution path. The separate real-boot probes DO resolve through exports to dist/, and for those the adapter was rebuilt after the fix and the marker confirmed present in dist/index.mjs.

Verification

  • pnpm --filter @objectstack/hono test — exit 0, Test Files 3 passed (3), Tests 82 passed (82)
  • 42 gate commands from node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (51 families matched, derived at 5ddff5274) — 41 exit 0. Verdict lines include check-test-source-alias OK — 72 packages with tests scanned, OK: 27 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob (check:cross-package-test-inputs), and Route-envelope conformance — 10 route module(s) audited: 7 conformant, 2 ratcheted, 1 exempt.
  • ⚠️ pnpm check:dual-build-cjs-loads exit 3 — NOT MEASURED, not a pass. Its own verdict: PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/. It needs a whole-repo pnpm build (32 packages listed as missing dist); CI's Build Core runs it.
  • ⚠️ This package declares no typecheck script, so pnpm --filter @objectstack/hono typecheck is not a reading — it exits 1 with ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT. check:check-type-check-coverage passes and names @objectstack/hono explicitly as one of five packages with no such script. A real tsc --noEmit -p tsconfig.json over the package reports 3 errors, all in src/hono.test.ts (lines 36, 69, 70), a file this diff does not touch. Proved pre-existing rather than assumed: with index.ts restored to the merge base and the new test file moved aside, the diagnostics are byte-identical — same file, same lines, same codes. This diff adds zero type errors.
  • Repo-wide pnpm lint was narrowed, and the narrowing is declared: eslint --no-inline-config --format json over the two changed source files — 2 files, 0 errors, 0 warnings, exit 0. The narrowing is safe by configuration, not by hope: this repo runs one eslint.config.mjs which never enables type-aware linting for any file (its own comment at line 328: "no parserOptions.project, no typed @typescript-eslint rules"), so a source-only diff cannot move the verdict on a file it does not touch. CI runs the full sweep regardless.

Out-of-scope findings, filed unassigned

Both were measured on the same boots and are not fixed here:

⚠️ Lint & Repo Gates is expected red on main itself at the Merge-driver wiring gate (#15992, another seat's). Read this job's own failing step number before attributing a failure here — no fix for it is carried in this diff.


🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y


Generated by Claude Code

`createHonoApp`'s `${prefix}/auth/*` mount handed the request on whenever the
auth service answered 404, with only the status to go on, so a 404 a routed
endpoint produced — a real answer — was replaceable by whatever matched next.

Measured on a real boot through this adapter (real kernel + AuthPlugin,
`prefix: '/api/v1'`):

    GET /api/v1/auth/delete-user/callback?token=abc&callbackURL=/x
      better-auth direct : 404 {"message":"Not found","code":"NOT_FOUND"}
      through the mount  : 200 {}

No composition had to install the overwriting layer: the `${prefix}/*`
dispatcher catch-all registered by the same function is terminal and answers
`200 {}` for paths under `/auth/`. `auth-route-ledger.ts` carries that route
under its `disabled` disposition because it is published and answers 404, so
the ledger's answer was true of the auth service and false on the wire.

The mount now asks the auth service whether its own router serves the path,
through an optional `ownsRoute(request)` — the seam `AuthManager` grew for the
plugin-side half of the same defect — and yields only when it does not. The
adapter does not import the ownership walk: it does not depend on
`@objectstack/plugin-auth` and gains no dependency here. Every answer that is
not a literal `true` means yield, so a service predating the method is
unaffected and a failure to decide can never cost #4088's ordering-independent
surface.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
@github-actions github-actions Bot added the size/m label Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Read-back note on the PR body: one token did not survive the platform's body sanitizer. Under "The change" it now reads

`AuthService` grows an optional `ownsRoute?(request: Request): Promise;`

The angle-bracketed type parameter was eaten. The signature in the diff is ownsRoute?(request: Request) returning a Promise of boolean, spelled with the normal generic syntax in packages/adapters/hono/src/index.ts. Recorded here rather than by rewriting the body, since an edit normalises the body's footer and buys nothing else.


Generated by Claude Code

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/hono, touching 5 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/data-flow.mdx (via AuthService (symbol, a top-level interface))
  • content/docs/kernel/runtime-services/sharing-service.mdx (via NOT_FOUND (literal, a string literal in createHonoApp))
What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: /api/v1 (route, 82 pages)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 2da2901efa2708d8895e436867838cbeeda191cbpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 3a184c671bfbaf5958600cb9cc2db3a2ea07f8d8 — the merge of head 5ddff5274c5846a18b05eb181d794161311e8e29 into base 2da2901efa2708d8895e436867838cbeeda191cb, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 3a184c671bfbaf5958600cb9cc2db3a2ea07f8d8 && git checkout 3a184c671bfbaf5958600cb9cc2db3a2ea07f8d8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2da2901efa2708d8895e436867838cbeeda191cb 5ddff5274c5846a18b05eb181d794161311e8e29 && git checkout -B drift-repro 2da2901efa2708d8895e436867838cbeeda191cb && git merge --no-ff 5ddff5274c5846a18b05eb181d794161311e8e29

node scripts/docs-audit/affected-docs.mjs --json 2da2901efa2708d8895e436867838cbeeda191cb

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 2da2901efa2708d8895e436867838cbeeda191cb → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

PM note — contract review deferred, and ⛔ not silently downgraded

The Clause-② review dispatched for this PR terminated before measuring anything: HTTP 429, "You've reached your Fable limit", on claude-fable-5-1. It got as far as stating its plan. ⛔ No verdict exists for this PR, and nothing in its transcript should be read as one.

CONTRACT_REVIEW_TIER = 'claude-fable-5-1' is declared in scripts/pm/dispatch-gates.mjs. Running the review on a different model to keep moving would be a deviation from a declared governance contract, and it is not this seat's to take quietly — a reviewer's tier line is part of what a PASS means here. So the review is held until the tier is available again, or until the maintainer rules otherwise.

⇒ This PR stays in draft, unarmed, and nothing lands. That is the correct state, not a stall to work around.

⚠️ The same constraint applies to every other contract review this seat owes — #16020 round 2 and #15966's next round included — so it is a throughput limit on the whole loop, not one PR's problem.

What is already established and does not need re-doing when the review resumes (from the dev's report, ⛔ none of it independently verified yet):

  • Step 1 was measured on a real boot and confirms the card: GET /api/v1/auth/delete-user/callback?token=… answers 404 from better-auth directly and 200 {} on the adapter's wire.
  • The blast radius is wider than the plugin's — no composition needs a downstream wildcard, because createHonoApp's own ${prefix}/* catch-all is terminal.
  • One correction to the card's framing: its own example POST /api/v1/auth/delete-user does not 404 here; a break-glass guard pre-empts with 409.
  • The fix ports the shape, not the code — an optional ownsRoute? on the adapter's structural AuthService, duck-typed like the existing getPublicConfig?.().

The first thing the review must attack when it runs — recorded now so it does not get lost: the dev's 89-pair population sweep reported zero owned-404s, and stated its own limit — the real owned-404 needs a token+callbackURL query string the sweep never sent. ⇒ That zero is not a clearance; the sweep could not produce the case it was looking for. ⭐ Credit to the dev for stating the limit rather than letting the zero read as coverage.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants