Skip to content

fix(plugin-auth): a refused OAuth application registration answers the status the refusal declared, not 500 - #24853

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24773-oauth-register-refusal
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24773-oauth-register-refusal

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Part of #24773 — this PR does the first half only: a refused registration now answers 4xx instead of 500. The second half, whether a web client may register an http://localhost / http://127.0.0.1 redirect URI, is an authentication-flow acceptance rule. It was measured here and nothing about it changed. It goes to a decision card. #24773 remains open.

Clause-②: no

What changed

POST /api/v1/auth/sys-oauth-application/register (packages/plugins/plugin-auth/src/auth-plugin.ts) answered most refusals from better-auth as 500 OAUTH_REGISTER_FAILED.

  • Mechanism, measured against the installed @better-auth/oauth-provider@1.7.3 / better-call@1.4.0. Every refusal better-auth raises is better-call's APIError. It carries its status twice: as the number statusCode (400) and as the name status ('BAD_REQUEST'). The door read only a numeric status, so every refusal with a named status fell through to 500. The body-schema ValidationError has the number 400 in its status, so that refusal answered 400 from the same door. One door gave two answers for the same class of refusal.
  • Fix. The door now reads the status through resolveThrownHttpError (@objectstack/types). That is the repo's one rule for which status a thrown error declares: a numeric status, then statusCode, and the caller's fallback (500) when the throw declares neither. The code stays OAUTH_REGISTER_FAILED (already in the ledger; no new code). details.upstreamError still carries the vendor's discriminator.
  • Effect, measured on the real stack. web + http://localhost:4001/callback or http://127.0.0.1:4001/callback: 500 → 400 (invalid_redirect_uri). No session: 500 → 401. A storage fault while persisting the client: still 500. Body-schema refusal (http://app.example.com/callback): still 400. https://app.example.com/callback: still 200.

Tests

New packages/plugins/plugin-auth/src/oauth-register-refusal-status.test.ts. It runs real better-auth with the OIDC provider on, behind the plugin's real route registration on a real Hono app, using the in-process-session-renewal.pin.test.ts harness. Each refusal asserts the ADR-0112 envelope (code + status) and details.upstreamError where the vendor sends one, and asserts that no sys_oauth_application row was written.

  • A precondition asserts that the fixture's refusal is a named-status APIError (status: 'BAD_REQUEST', statusCode: 400). If the vendor ever switched to a numeric status, the pins below would pass on the old door too, and this precondition says so.
  • Ablation, leg 1 (put the old read back: const status = typeof err?.status === 'number' ? err.status : 500;). The mutation was applied with scripts/ablation-replace.mjs, which confirmed it on disk (anchor 1 → 0, replacement 0 → 1, blob 03e026dea → a6bb1a7f3). Result: Tests 3 failed | 14 passed (17). The failures were expected 500 to be 400 (localhost), expected 500 to be 400 (127.0.0.1) and expected 500 to be 401 (no session). Restored: blob equal to HEAD, git diff HEAD empty.
  • Ablation, leg 2 (unknown throw falls back to 400: resolveThrownHttpError(err, 400)). Disk-verified the same way (blob 03e026dea → 16aa6cab0). Result: Tests 1 failed | 16 passed (17) with expected 400 to be 500 on the storage-fault case. Restored the same way.
  • Both legs went red in the direction predicted and nothing else moved. Both ran on head 5dc8add4f, the commit that carries the fix. The merge of main that followed left both files byte-identical (blobs 03e026dea and 8b2336493).

Head 622e30aa9 is the merge of main at 5cb6e6e0a. The incoming commits touch none of this diff's files, and packages/spec check:generated is all up to date after the rebuild. On that head:

  • pnpm --filter @objectstack/plugin-auth exec vitest run: Test Files 169 passed (169), Tests 3241 passed | 10 skipped (3251).
  • pnpm --filter @objectstack/plugin-auth typecheck: exit 0, with check:test-typecheck: OK.
  • The derived local gate union (node scripts/pm/dispatch-gates.mjs --commands, 72 commands) ran with every exit code captured before any pipe: 72 of 72 exit 0. The --ran reconciliation printed 72 derived, 72 run, 0 NOT-MEASURED, 0 UNRUN.

Half 2 — measured, not changed (for the decision card)

  • The vendor's predicate. This is validateClientRedirectUri, @better-auth/oauth-provider@1.7.3. A web client is refused unless the URI is https: and the host is not loopback (localhost or a loopback IP). The message "web clients require https redirect URIs on non-loopback hosts" states that conjunction. It does not exempt loopback hosts. Measured through this door, every loopback form is refused for web: http://localhost, http://localhost:4001, http://127.0.0.1:4001, http://[::1]:4001, and also https://localhost and https://127.0.0.1. Port and IP family make no difference. A native client may use http on exactly localhost, 127.0.0.1 or [::1], with any port. Called directly with application_type: 'native', the vendor accepted http://localhost:4001/callback and http://127.0.0.1:4001/callback. The predicate takes no options, so no configuration plugin-auth passes (trusted origins, dev mode, dynamic client registration) governs it.
  • The door drops the Console's Application Type. The form's type select (web / native / user-agent-based) is forwarded as the body key type. The installed vendor's /oauth2/create-client body schema declares no type. It declares application_type: 'web' | 'native'. So the key is stripped, and every registration is judged and stored as web. Measured: type: 'native' + http://127.0.0.1:4001/callback is refused with the web rule. type: 'native' + an https URI is stored with application_type: "web".
  • The field help. "One URL per line. Must use https:// in production." (packages/platform-objects, sys_oauth_application create action) does not match the measured rule for web: https always, never a loopback host.

The options, analysed on the four axes, are in the dev report on the card. This PR changes none of them.

Acceptance notes

  • Not changed here, and noted without a card: on a server fault the door answers with the thrown error's raw message (in the test, storage fault (test)) and logs nothing on that branch. This is the existing behaviour and out of this card's scope.
  • test:repo (vitest repo project) was not run locally; it is CI's (Test Core).

Generated by Claude Code

…e status the refusal declared, not 500

better-auth raises each refusal as better-call's APIError, whose HTTP
status is the number `statusCode` and the name `status` ('BAD_REQUEST').
The sys-oauth-application/register wrapper read only a numeric `status`,
so a named refusal (an invalid_redirect_uri redirect URI, a missing
session's UNAUTHORIZED) answered 500. It now reads `statusCode` first; a
throw that declares no status stays 500.

Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y
Co-authored-by: Claude <noreply@anthropic.com>
…s through unknown

Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y
Co-authored-by: Claude <noreply@anthropic.com>
…ough resolveThrownHttpError

The repo's one rule for "what status does a thrown error declare" already
reads both spellings (numeric `status`, then `statusCode`) and falls back
to 500 for a throw that declares none. The door calls it instead of a
second hand-written copy.

Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 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
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 16 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 41646a5788f5362a66527a287200f9832fb00602 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 1529c2159c87fd14fc742341bd9993a3dab9336f — the merge of head 622e30aa99033ab38fca59216d48b70b8e3d76a6 into base 41646a5788f5362a66527a287200f9832fb00602, 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 1529c2159c87fd14fc742341bd9993a3dab9336f && git checkout 1529c2159c87fd14fc742341bd9993a3dab9336f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 41646a5788f5362a66527a287200f9832fb00602 622e30aa99033ab38fca59216d48b70b8e3d76a6 && git checkout -B drift-repro 41646a5788f5362a66527a287200f9832fb00602 && git merge --no-ff 622e30aa99033ab38fca59216d48b70b8e3d76a6

node scripts/docs-audit/affected-docs.mjs --json 41646a5788f5362a66527a287200f9832fb00602

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · #24773 half 1 · domain:services seat 2 · os-support-ai · session_01UmqzD97e14iws6hmVhgZ5y · 2026-10-20T10:42Z · head 622e30aa99, the review of record for claim 6287244376.

  1. Form:
    • Draft, base main. The first line is Part of #24773; the seat's grep of the body finds no closing keyword beside any issue number, so the card stays open for half 2.
    • Clause-②: no at line start in the body and in the changeset. Every refusal stays a refusal; only its envelope status moves from 500 to the status the refusal declared. No accepted value or error code moves.
    • One patch changeset for @objectstack/plugin-auth.
    • 3 files, +227/−2, inside the claim's file surface: only the sys-oauth-application/register wrapper in auth-plugin.ts.
  2. The fix, read by the seat:
    • The diff. One line: const { status } = resolveThrownHttpError(err, 500), the repo's shared rule (@objectstack/types), plus a comment. The code stays OAUTH_REGISTER_FAILED with details.upstreamError.
    • M1 confirmed by measurement. better-call's APIError carries the number statusCode and the name status, and the door read only a numeric status.
    • The same line also answered a missing session as 500. It now answers 401, the same defect; disclosed in the PR and the changeset, and pinned.
    • Pins on real better-auth behind the real route. localhost and 127.0.0.1 answer 400 with invalid_redirect_uri and no row written; no session answers 401; the body-schema refusal stays 400; a storage fault stays 500; https answers 200 with one row. Each refusal asserts code + status.
    • Ablation. Two legs went red in the predicted direction and were each restored to the HEAD blob: the old numeric-only read (3 failed), and a blanket 400 fallback (the storage-fault pin failed).
  3. Gates:
    • CI on 622e30aa99 at this read: 38 check-runs, 21 success, 7 skipped, 10 in progress, 0 failed. Not yet green, so not yet ready.
    • Local on the head: 72 derived, 72 run, 0 NOT-MEASURED, 0 UNRUN.
    • Package suites: plugin-auth 169 files, 3241 tests; typecheck exit 0.
  4. Accepted deviations:
    • The suggested route was "map the vendor refusal to 400". The landed route reads the refusal's own declared status through the shared rule. That is better: 401 for a missing session is right, and there is no second rule.
    • origin/main (5cb6e6e0a) was merged once before the PR opened.
  5. Out of scope:
    • Half 2 (which redirect URIs a client may register): the measurement falsified the card's reading. The vendor's web rule is https and non-loopback by design, in every environment. The Console's Application Type select is sent as type, which the vendor schema strips, so every registration is judged and stored as web. Changing that changes acceptance at an authentication door (the human floor). The seat files the decision on OAuth app registration rejects http://localhost and http://127.0.0.1 redirect URIs as "non-loopback", with HTTP 500 #24773 when this PR lands, with the dev's options A–D and its two folded findings (the dropped type; the field help).
    • The raw thrown message on a server fault (pre-existing, no measured leak) is in the PR's Acceptance notes. Not filed.
  6. Landing: ordinary queue landing (check-governed-merges --pr 24853: not governed, 229 lines). When every check on the current head is green, the seat sends pr_ready + automerge_enable, confirms MERGED, then moves OAuth app registration rejects http://localhost and http://127.0.0.1 redirect URIs as "non-loopback", with HTTP 500 #24773 to needs-user-decision with the half-2 analysis.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 11:04
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 11:04
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 950f949 Oct 20, 2026
42 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24773-oauth-register-refusal branch October 20, 2026 11:29
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

Development

Successfully merging this pull request may close these issues.

2 participants