Repository navigation
Commit ab3ba2c
fix(plugin-auth)!: sixteen auth routes refuse an unparseable declared-JSON request body with 400 INVALID_REQUEST instead of reading it as {} (#24049)
Fixes #24032
Clause-②: no (narrowing)
## What
Sixteen of `@objectstack/plugin-auth`'s own routes read a request body
that does not parse as `{}`. There were nine at round 0, and patch round
1 added seven more: claim amendment `6225083981`, addendum `6225763180`.
A truncated body was answered as if the client had sent none. Each of
them now refuses a body that is declared `application/json`, is not
blank, and does not parse. The refusal is `400 INVALID_REQUEST` in the
ADR-0112 envelope, with the same code and message the HTTP transports
(`plugin-hono-server`, `hono`) and `cloud-connection` give the same body
since PR #24005. `INVALID_REQUEST` is an existing `ERROR_CODE_LEDGER`
code, and nothing is added to the ledger. The message never echoes the
body.
The reader is one package-local module, `src/request-json-body.ts`. It
is not re-exported from `index.ts`, and nothing is imported from
`plugin-hono-server` or `cloud-connection` (ruling B on #23914: no new
export). As in `cloud-connection`, it answers a message, and each route
builds its own `400` body beside its other refusals.
## Premise, measured on `origin/main` `2b274600b1`
All nine sites swallow a parse failure into `{}`. Triage counted 9 and
the card listed 7. The card's grep missed `register-sso-provider.ts:101`
and `send-verification-email.ts:96`, because both spell the swallow over
several lines (`body = {};` on its own line). They have the same shape,
so they are the same family. `:96` is the one site where `{}` is an
action rather than a refusal.
| site | route (`/api/v1/auth`) | what the handler answers for `{}` |
family |
|---|---|---|---|
| `auth-plugin.ts:2735` | `POST /admin/oauth2/toggle-disabled` | 400
`INVALID_REQUEST` "client_id is required", before the platform-admin
gate | A, inline mount |
| `auth-plugin.ts:2870` | `POST /admin/unlock-user` | 400 "userId is
required", before the gate | A |
| `auth-plugin.ts:3423` | `POST /sys-oauth-application/register` | 400
"name is required" | A |
| `admin-import-users.ts:493` | `POST /admin/import-users` | after the
gate: 400 "Provide format:..." (503 when no data engine) | B, identity
import |
| `register-sso-provider.ts:101` | `POST /admin/sso/register` | after
the gate: 400 "Missing required field(s): providerId, issuer, domain,
clientId, clientSecret" | C, SSO bridges |
| `register-sso-provider.ts:250` | `POST /admin/sso/register-saml` | 400
"Missing required field(s): providerId, issuer, domain, entryPoint,
cert" | C |
| `register-sso-provider.ts:444` | `POST
/admin/sso/request-domain-verification` | 400 "Missing required field:
providerId" | C |
| `register-sso-provider.ts:501` | `POST /admin/sso/verify-domain` | 400
"Missing required field: providerId" | C |
| `send-verification-email.ts:96` | `POST /send-verification-email` |
**an action**: with a session, it resends the verification link to the
caller's own address (200). Without a session, 400 "email is required
(sign in to resend to your own address)" | D, self-service resend |
So, for eight sites, a truncated body got a required-field refusal for a
field the body carried. At the resend, a truncated body that named
another address (`{"email": "other@example.test"`) was answered as the
one-click resend and mailed the caller.
## What a reading answers
- **A body that parses:** that value, as before, whatever its
`Content-Type`. These routes never read the media type to parse:
`request.json()` parses a valid JSON body sent as `text/plain`, and so
does `JSON.parse` over `text()`. They still do not, so the transports'
`text/plain` refusal (PR #24005 step 4) is not matched. Matching it
would narrow a second input that these routes never distinguished.
- **Declared `application/json`, not blank, does not parse:** refused,
"The request body could not be parsed as JSON (it was sent as
application/json). It was refused rather than read as an empty body:
send a well-formed JSON document, or no body at all."
- **Declared JSON whose stream cannot be read:** refused with the same
code, "The request body could not be read. It was refused rather than
read as an empty body."
- **Empty:** unchanged. That is no body and no `Content-Type`, a
zero-byte body with `Content-Type: application/json` and
`Content-Length: 0`, or a whitespace-only body. Each family's pin drives
all three, and each still gets today's answer (the table above). For the
resend, that answer is still the one-click resend to the caller's own
address, which the `sys_user` action fires with no body.
- **Undeclared and does not parse:** still `{}`, the same rule as
`cloud-connection` and `createHonoApp` (Acceptance notes).
**Order.** Each route refuses where it read its body. `toggle-disabled`,
`unlock-user`, `sys-oauth-application/register` and the resend read
before anything else runs, as they always did. An anonymous caller's
malformed body there is answered 400, and it was answered 400 "... is
required" before. The import and the four SSO bridges read after their
platform-admin gate. Nothing is looked up, written, sent or forwarded to
better-auth.
**Exported bridges.** `runAdminImportUsers`,
`runRegisterSsoProviderFromForm`, `runRegisterSamlProviderFromForm`,
`runRequestDomainVerification`, `runVerifyDomain` and
`runResendVerificationEmail` answer the same refusal. A host that mounts
them itself, such as the cloud auth proxy, gets the refusal with its
next dependency bump. Signatures are unchanged.
## Changeset: `minor`, not the dispatched `patch`
The dispatch asked for a `patch` changeset. Under this PR's `Clause-②:
no (narrowing)` line, the required `Check Changeset` level axis refuses
`patch`. Both were measured at the same tree with `node
scripts/check-changeset-no-major.mjs --base 2b27460 --head ...
--event` and a `pull_request` payload carrying this body's first three
lines:
- `plugin-auth: patch` (a scratch commit object, never pushed) exits 1:
"clause-② declares `no (narrowing)` while no moved package is graded
`minor` or above".
- `plugin-auth: minor` (`dccf68bae1`) exits 0: "LEVEL AXIS: this PR
declares clause-② `no (narrowing)`, and no package whose published
source ... it moves is graded `patch`".
So `.changeset/24032-plugin-auth-body-refusal.md` is `minor` under the
launch-window convention for breaking changes, the same grade as PR
#24005's three changesets. Its first line after the frontmatter is
`Clause-②: no (narrowing)`. It carries the `**BREAKING**` banner, an
ADR-0087 marker of kind `not-required (no-migration-prescription)` with
the other categories closed on facts, the routes, the FROM / TO for an
integrator, the one-line remedy, and a statement that an empty body is
unchanged.
## Tests
New: `src/request-json-body-refusal.test.ts`. It had 43 cases at round
0, and 71 after patch round 1 added blocks E to I for the seven more
readers. The ablation turns 21 red, and no CONTROL goes red (addendum
`6225763180`). It drives the plugin's real `registerAuthRoutes` on a
real Hono app (the `admin-sso-bridge-gate.test.ts` harness), with one
block per family:
- **A:** the three inline routes. Each gets a truncated body and `{ not
json`, both refused, plus three empty CONTROLs. Further rows on
`unlock-user`: an unreadable stream, refused; CONTROL, a well-formed
body acted on; CONTROL, valid JSON sent as `text/plain` acted on;
CONTROL, an undeclared truncated body is still `{}`.
- **B:** the import. A truncated body is refused, plus three empty
CONTROLs.
- **C:** the four SSO bridges. Each gets a truncated body, refused, plus
three empty CONTROLs.
- **D:** the resend. A truncated body naming another address is refused,
plus three empty CONTROLs that still resend to the session address.
Each refusal asserts four things:
- the status, `envelopeViolations(body)` empty, and `code`;
- the message's first sentence, since that message family is the card's
contract;
- that the message does not echo the body;
- that nothing after the read ran: no `unlockUser`, no engine call, no
`createOAuthClient`, no better-auth call, nothing sent.
`admin-import-users.js` is loaded at module top. The route's own
`import()` measured 8.7 s inside a 10 s clocked test on the first run.
Fixture triage: in `send-verification-email.test.ts`, the row "tolerates
a non-JSON body and falls back to the session email" sent `not-json{` as
`application/json`. It pinned exactly the branch this removes. That row
now pins the undeclared spelling (`text/plain`), which is unchanged, and
a new row pins the declared refusal with no session lookup and nothing
sent.
Runs (all under `scripts/pm/os-verify-lock.sh`, exit codes captured
before any pipe):
- **Today's code** (the new file against `2b274600b1`'s sources): 13
failed, 30 passed. The 13 are every refusal row, each failing on today's
required-field answer or the 200 resend. The 30 are every CONTROL.
- **Fix** (`8d97b20b34`): the new file plus the 7 neighbouring files
(`send-verification-email`, `register-sso-provider`,
`admin-sso-bridge-gate`, `admin-import-users`,
`principal-less-producers-system-context`, both
`sso-domain-verification` pins): 8 files, 175 passed.
- **Head `1f1a7fd026`** (merges `origin/main` `b74434c581`; install,
closure build and full build refreshed after the merge):
- `pnpm --filter @objectstack/plugin-auth exec vitest run
--maxWorkers=2`: 151 files, 2986 passed, 10 skipped (pre-existing; none
in these files).
- `pnpm --filter @objectstack/plugin-auth typecheck`: exit 0,
`check:test-typecheck` OK, its debt ledger unchanged.
- `pnpm exec eslint --no-inline-config` over the 7 changed `.ts` files:
exit 0.
## Ablation (WRAP mode through `scripts/ablation-replace.mjs`, on
`8d97b20b34`)
The suites import their subject by relative path, so they run on source
and need no rebuild.
| leg | mutation in `request-json-body.ts` | blob | result |
|---|---|---|---|
| malformed | `return { ok: false, message: MALFORMED_JSON_BODY_MESSAGE
};` becomes `return { ok: true, body: {} };` | `71516e4a3cd6` to
`a2d9e371a38c` | 13 failed: the 12 malformed refusal rows across
families A to D, and the declared-refusal row in
`send-verification-email.test.ts`. 38 passed: every CONTROL, and the
unreadable row, which is the other branch |
| unreadable | `{ ok: false, message: UNREADABLE_BODY_MESSAGE }` becomes
`{ ok: true, body: {} }` | `71516e4a3cd6` to `97a4b1b53117` | 1 failed
(the unreadable row), 42 passed |
Each leg's anchor hit once, and the write was proven on disk. The
restore was proven by blob `71516e4a3cd6` matching `HEAD`, with `git
diff HEAD` empty. After both legs, the marker count is 0 and `git status
--porcelain` is 0 lines.
## Gates
`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` on `1f1a7fd026` (merge base `b74434c58`, 8
paths) derived 69 commands. All 69 ran with the documented capture
idiom. 68 exited 0 on the first pass. `pnpm check:dual-build-cjs-loads`
exited 3 (PREREQUISITE NOT MET, no `dist/` for 41 packages). After `pnpm
turbo run build --filter='!@objectstack/docs' --concurrency=2` (75 of 75
tasks), the same command exited 0: 110 entry points across 68 packages
load. `--ran` over the final record: "69 derived, 69 run, 0
NOT-MEASURED, 0 UNRUN".
These include `check:route-envelope`,
`check:dispatcher-error-vocabulary`, `check:error-status-conformance`,
`check:auth-mount-ledger`, `check:test-source-alias`,
`check:cross-package-test-inputs`, `check:type-check-coverage`,
`check:type-check-debt`, `check:nul-bytes`, `check:published-files`,
`check:dts-closure`, `check-adr-0087-registration`,
`check-changeset-no-major` and `check-empty-changeset`. Not run locally:
`test:repo` and the repo-wide `pnpm lint`. CI owns both.
`origin/main` has moved three commits past `b74434c581`. One of them,
`invitation-role-cap.ts` and its test, touches this package but none of
these files. `git merge-tree --write-tree HEAD origin/main` exits 0.
## Acceptance notes
- **A body that does not declare JSON and does not parse is still
`{}`.** This is the same rule as `cloud-connection` and `createHonoApp`.
At the resend, it still means the one-click resend to the caller's own
address, reachable only by a client that sends a non-JSON media type.
Refusing it too would narrow a second input. Carrier: none.
- **A fourth copy of the two messages.** `plugin-hono-server`, `hono`,
`cloud-connection` and this package each keep a module-private copy, per
ruling B. A cross-package parity test would import
`plugin-hono-server`'s built `dist/` into this package's tests, which
`check:test-source-alias` reports as a new unaliased import. So the
first sentence is pinned here instead.
- **Seven more routes in this package carry the same swallow under
another spelling**, outside this claim's file surface. Each reads
through a module-private `parseJson` that returns `{}`, or through
`parsed = {}`:
- `admin-ban-endpoints.ts:106`: `ban-user`, `unban-user`;
- `admin-set-user-manager.ts:217`: `set-user-manager`;
- `admin-user-endpoints.ts:217`: `create-user`, `set-user-password`;
- `organization-add-member.ts:100`: `organization/add-member`;
- `set-initial-password.ts:57`: `set-initial-password`, also an exported
bridge.
Each answered `{}` with a required-field refusal. Neither the card's
grep nor triage's `body = {}` count reached them. **These are now in
this PR:** the seat answered the dev's open question with A (claim
amendment `6225083981`), and patch round 1 converted all seven readers
to `readJsonRequestBody`, one pin block each. `set-initial-password`'s
signature and result are unchanged; its callers see only the refusal's
message.
- **Two other body reads in the package are not this family.**
`auth-manager.ts:7201` is a best-effort probe over a clone for trusted
origins: a parse failure there is `[]`, and better-auth then judges the
body itself. `/admin/has-permission` (`auth-plugin.ts:3206`) hands an
unreadable body to better-auth's own 400.
## Contract review
`Clause-②: no (narrowing)`: this owes one `CONTRACT_REVIEW_TIER` review
before the queue, as PR #24005 did. No governed surface is touched. The
record is `6226126497`, PASS, at `7160ac93d7`.
_Seat edit · `domain:services` seat 2 ·
`session_018VqJPGA1NDNSCZsnecsChP` · 2026-10-16T22:55Z: brought to patch
round 1. The route count is sixteen, the seven routes are now in this
PR, the test count is updated, the contract record is linked, and the
title follows._
---
_Generated by [Claude
Code](https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 44fe210 commit ab3ba2c
14 files changed
Lines changed: 754 additions & 68 deletions
File tree
- .changeset
- packages/plugins/plugin-auth/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
| 78 | + | |
78 | 79 | | |
79 | 80 | | |
80 | 81 | | |
| |||
103 | 104 | | |
104 | 105 | | |
105 | 106 | | |
106 | | - | |
107 | | - | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | | - | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
113 | 120 | | |
114 | 121 | | |
115 | 122 | | |
| |||
128 | 135 | | |
129 | 136 | | |
130 | 137 | | |
131 | | - | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
132 | 141 | | |
133 | 142 | | |
134 | 143 | | |
| |||
191 | 200 | | |
192 | 201 | | |
193 | 202 | | |
194 | | - | |
195 | | - | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
196 | 206 | | |
197 | 207 | | |
198 | 208 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
113 | 113 | | |
114 | 114 | | |
115 | 115 | | |
| 116 | + | |
116 | 117 | | |
117 | 118 | | |
118 | 119 | | |
| |||
489 | 490 | | |
490 | 491 | | |
491 | 492 | | |
492 | | - | |
493 | | - | |
494 | 493 | | |
495 | 494 | | |
496 | 495 | | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
497 | 502 | | |
498 | 503 | | |
499 | 504 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
122 | 122 | | |
123 | 123 | | |
124 | 124 | | |
| 125 | + | |
125 | 126 | | |
126 | 127 | | |
127 | 128 | | |
| |||
214 | 215 | | |
215 | 216 | | |
216 | 217 | | |
217 | | - | |
218 | | - | |
219 | | - | |
220 | | - | |
221 | | - | |
222 | | - | |
223 | | - | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
224 | 232 | | |
225 | 233 | | |
226 | 234 | | |
| |||
361 | 369 | | |
362 | 370 | | |
363 | 371 | | |
364 | | - | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
365 | 375 | | |
366 | 376 | | |
367 | 377 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
174 | 174 | | |
175 | 175 | | |
176 | 176 | | |
| 177 | + | |
177 | 178 | | |
178 | 179 | | |
179 | 180 | | |
| |||
214 | 215 | | |
215 | 216 | | |
216 | 217 | | |
217 | | - | |
218 | | - | |
219 | | - | |
220 | | - | |
221 | | - | |
222 | | - | |
223 | | - | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
224 | 232 | | |
225 | 233 | | |
226 | 234 | | |
| |||
551 | 559 | | |
552 | 560 | | |
553 | 561 | | |
554 | | - | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
555 | 565 | | |
556 | 566 | | |
557 | 567 | | |
| |||
680 | 690 | | |
681 | 691 | | |
682 | 692 | | |
683 | | - | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
684 | 696 | | |
685 | 697 | | |
686 | 698 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
71 | 71 | | |
72 | 72 | | |
73 | 73 | | |
| 74 | + | |
74 | 75 | | |
75 | 76 | | |
76 | 77 | | |
| |||
2741 | 2742 | | |
2742 | 2743 | | |
2743 | 2744 | | |
2744 | | - | |
2745 | | - | |
| 2745 | + | |
| 2746 | + | |
| 2747 | + | |
| 2748 | + | |
| 2749 | + | |
| 2750 | + | |
| 2751 | + | |
| 2752 | + | |
2746 | 2753 | | |
2747 | 2754 | | |
2748 | 2755 | | |
| |||
2876 | 2883 | | |
2877 | 2884 | | |
2878 | 2885 | | |
2879 | | - | |
2880 | | - | |
| 2886 | + | |
| 2887 | + | |
| 2888 | + | |
| 2889 | + | |
| 2890 | + | |
| 2891 | + | |
| 2892 | + | |
2881 | 2893 | | |
2882 | 2894 | | |
2883 | 2895 | | |
| |||
3429 | 3441 | | |
3430 | 3442 | | |
3431 | 3443 | | |
3432 | | - | |
3433 | | - | |
| 3444 | + | |
| 3445 | + | |
| 3446 | + | |
| 3447 | + | |
| 3448 | + | |
| 3449 | + | |
3434 | 3450 | | |
3435 | 3451 | | |
3436 | 3452 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
72 | 72 | | |
73 | 73 | | |
74 | 74 | | |
| 75 | + | |
75 | 76 | | |
76 | 77 | | |
77 | 78 | | |
| |||
97 | 98 | | |
98 | 99 | | |
99 | 100 | | |
100 | | - | |
101 | | - | |
102 | | - | |
103 | | - | |
104 | | - | |
105 | | - | |
106 | | - | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
107 | 114 | | |
108 | 115 | | |
109 | 116 | | |
| |||
155 | 162 | | |
156 | 163 | | |
157 | 164 | | |
158 | | - | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
159 | 168 | | |
160 | 169 | | |
161 | 170 | | |
| |||
0 commit comments