Skip to content

fix(plugin-auth)!: an anonymous get-session is refused with the declared 401 envelope, not answered 200 null - #17881

Merged
os-project-manager merged 6 commits into
mainfrom
claude/issue-17238-anonymous-session-401
Sep 12, 2026
Merged

fix(plugin-auth)!: an anonymous get-session is refused with the declared 401 envelope, not answered 200 null#17881
os-project-manager merged 6 commits into
mainfrom
claude/issue-17238-anonymous-session-401

Conversation

@claude

@claude claude Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #17238

Ruled B by the director seat (decision batch #117 item 4, comment 5642584682). Maintainer, verbatim: 「17238 B」. The seat had recommended A and was corrected, verbatim: 「我改推荐的都是从平台长远合理性角度考虑的」, under the charter rule 「spec 与代码不一致默认改代码,改协议单独立卡非选项」.

GET /api/v1/auth/get-session now answers an anonymous caller the platform's standard ADR-0112 failure envelope at HTTP 401, instead of 200 + the literal JSON null. SessionResponseSchema is untouched.

The defect, and why the code is what moves

ObjectStackClient.auth.me() declares Promise<SessionResponse>, and SessionResponseSchema requires data.session and data.user — so no value of that type means "nobody is signed in", and the most ordinary call a logged-out caller can make resolved to something outside the method's own declared type.

⇒ Every value auth.me() returns is now inside SessionResponse. The anonymous case is delivered as a rejection instead: the SDK's fetch wrapper already throws on every non-2xx, so the caller gets an error carrying code: 'UNAUTHENTICATED' and httpStatus: 401. No runtime edit in packages/client was needed — that fell out of the measurement rather than being assumed.

Measured, on the merge base and after

Driven through a real AuthManager (better-auth, default plugin set, in-memory engine) via handleRequest:

                                          BEFORE                      AFTER
GET  /get-session  (no credential)        200  application/json null  401  {"success":false,"error":{"code":"UNAUTHENTICATED","message":"Sign in first"}}
GET  /get-session  (unknown cookie)       200  application/json null  401  (same)
GET  /get-session  (signed in)            200  {user,session}         200  {user,session}   ← unchanged, asserted by identity
POST /get-session  (no credential)        404  (no content-type) ''   404  (unchanged)

The POST row is why the rule is keyed on the 200-null answer and not on the path alone: a path-only rule would have converted that 404 into a 401 and claimed a route better-auth does not serve.

Where it lives

packages/plugins/plugin-auth/src/anonymous-session-refusal.ts, wired into AuthManager.handleRequest beside the existing /admin/ envelope normalization — the one seam every vendor route passes through. ⚠️ It differs from that sibling in exactly one way, stated in both files: the /admin/ seam is forbidden to move admission, and this one is the admission move.

Three narrowings hold everything else still, each pinned by a toBe identity assertion:

  1. The /get-session endpoint exactly — not a prefix, not the session family.
  2. Status 200 only — anything better-auth already refuses, redirects or fails reaches the caller untouched.
  3. A body that is exactly the literal null — compared as text, so a body that merely parses to something falsy (0, "", false, {}, []) is left alone.

⛔ better-auth's JS API is untouched. auth.api.getSession() still returns null for an anonymous caller, so execution-context resolution, the platform-admin gates and the SSO bridges behave exactly as before. Only the wire moves. There is a negative-control test for precisely this.

No error code is minted. UNAUTHENTICATED is an existing StandardErrorCode member (packages/spec/src/api/errors.zod.ts:69), derived from the status through ADR-0112's own standardErrorCodeForHttpStatus map — so there is no string literal here to drift, ERROR_CODE_LEDGER is unchanged, and packages/spec is not touched by this PR.

Client consequences, enumerated (ruling note 3)

Note 3 requires these be listed here rather than discovered downstream. The two readings behind the list are posted on the card (comment 5647024737).

consumer effect
ObjectStackClient.auth.me() rejects for an anonymous caller instead of resolving null. No source change — this.fetch already throws on non-2xx
packages/client/src/auth-get-session-envelope.test.ts block ③ flipped, reversal named in place. It drives the real server, so it went red the moment the 401 landed
packages/client/src/client.test.ts re-pointed by hand. ⚠️ Fetch-mock driven, so it would have stayed green while pinning a wire shape no server produces any more — the more dangerous of the two, because it announces nothing
packages/cli/src/commands/whoami.ts behaviour improves with no code change: a stored-but-invalid token now prints the refusal and exits 1 instead of printing an empty Current Session. requireAuth(token) already exits earlier when no token is stored
packages/plugins/plugin-auth/src/register-sso-provider.ts unaffected. It guards if (!resp.ok) return undefined; — before: ok, body null, org undefined; after: not ok. Same value, best-effort by its own docblock
console / ../objectui no consumer found; nothing branches on the null body

packages/client was originally fenced out of this card as domain:cli's serial hot file. The dispatching seat lifted the fence for those two test files only, having measured that no open PR holds any packages/client path and that this PR cannot go green without the block ③ flip. ⛔ No runtime edit was made in that package.

Ablation — the negative pins are proven able to fail

This lane requires every negative pin carry one. The old 200 null was put back by neutering the guard in anonymous-session-refusal.ts, plugin-auth was rebuilt, and the marker was proven live in the artifact the client consumes before anything was read:

SRC ablation added : 1  (expect 1)
SRC blob b7e8f76e2915cae2581a158384822cd8367bcfd3 != HEAD ba3aeb861171543e87d4841766bc84e6a0fe346e -> MUTATION ON DISK
✓ dist/: marker present in 2 built files -- the ablation is live in the artifact the suite consumes.

⚠️ The rebuild is load-bearing and the first attempt caught me: packages/client resolves @objectstack/plugin-auth through dist, so a source-only mutation would have left block ③ green and certified a test that could never fail. A second trap the preflight caught: the first marker was written inside a /* */ comment, which tsup strips — it never reached dist at all.

Ablated, server suite (4 of 11 red):

FAIL src/anonymous-session-refusal.test.ts > ⭐ answers 401 AND the registered code — both, not either
AssertionError: expected 200 to be 401

FAIL src/anonymous-session-refusal.test.ts > the refusal body is a conforming ADR-0112 envelope, exhaustively
AssertionError: expected [ Array(1) ] to deeply equal []
+ [ "body is null, not an envelope object" ]

Ablated, client block ③ (2 red) — and this is the card's own loop closing:

FAIL src/auth-get-session-envelope.test.ts > ③ anonymous is REFUSED > rejects with the declared refusal instead of resolving outside its type
AssertionError: promise resolved "null" instead of rejecting

The card records that block ③ "is green under [the ablation] and CANNOT redden on it … it pins the residue, not the fix." It reddens now. That is the difference between a pin that records a gap and a pin that protects a fix.

Restored, rebuilt, and re-run green: ✓ dist/: marker absent from all 12 built files, ✓ tree: working tree clean against HEAD, 11/11 and 223/223.

Verification

All at b8c2b00f8, the final commit.

run result
plugin-auth full suite 2283 passed (108 files)
plugin-auth typecheck (incl. check:test-typecheck) pass — test layer compiles
client — the 5 affected files 250 passed
dispatch-gates --commands → all run → --ran 70 derived, 70 run, 0 NOT-MEASURED, 0 UNRUN, all exit 0 (a derived zero: every command recorded its exit code)
eslint, narrowed 11 of 6657 files, 0 errors / 0 warnings

The eslint narrowing is a measurement, not a skip: ① the population (6657) is eslint's own enumeration, ② the 11 is the --format json count, ③ this repo runs one eslint.config.mjs which never enables type-aware linting for any file (its own header, line 328, measured there with a positive control) ⇒ no verdict on an untouched file can have moved.

One test outside the new file needed re-pointing: auth-manager.optional-plugin-isolation.test.ts's "get-session also survives a degraded optional plugin" used the anonymous 200 as its vehicle for "the route reached better-auth and came back". The claim is unchanged; the assertion now pins 401 and error.code, plus the recorded degradation, so it cannot pass on a 401 produced for some other reason.

Changeset

minor, declared BREAKING, with the observable behaviour change stated in as many words (ruling note 6). Its ADR-0087 disposition is not-required (no-migration-prescription): this retires no metadata surface, so objectstack migrate meta has nothing to rewrite and no ledger entry can be written for it — what moves is an HTTP status plus an SDK method's promise contract. The same disposition and reasoning a previous HTTP-status unification used.

⚠️ The first draft carried a FROM -> TO table, which that gate reads as a migration prescription — correctly, by its own rules — and a prescription refuses this category. The consumer guidance is kept, restated as prose plus a try/catch example. ⛔ The bang was not dropped to get around it; the gate names that move as the one it exists to stop.

Acceptance notes

Noted, not filed — all three are the same class (a pin or a docblock describing a wire shape the server no longer serves), and none is in this card's face:

Two stale card references found while re-verifying the ruling's premises: #14313 (named in the issue body as the auth.* family card) returns HTTP 404 — it does not exist in this repository; control cards read with the same credential in the same batch resolved normally. And #17234 as above.

Scope declaration

Clause-②: no, and the delivery is consistent with it: the accept set narrows (a 401 replaces a 200) rather than widening, UNAUTHENTICATED is reused rather than minted, packages/spec is not touched, and the new module is package-internal — it is not re-exported from index.ts and appears zero times in the published .d.ts.

⚠️ This PR is not independently landable and is deliberately left as a draft. Block ③ drives the real server, so server and client halves have to land together; they are in this one PR for that reason.


🤖 Generated with Claude Code

https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj


Generated by Claude Code

…401 envelope

`GET /api/v1/auth/get-session` answered an unauthenticated caller with HTTP 200
and the literal JSON `null` — a value no `SessionResponse` can express, so
`ObjectStackClient.auth.me()` resolved outside its own declared return type on
the most ordinary call a logged-out caller makes.

The route now answers the platform's standard ADR-0112 failure envelope:
`401 { success: false, error: { code: 'UNAUTHENTICATED', message } }`.
`SessionResponseSchema` is untouched — the implementation is corrected to the
published contract, not the other way round.

The refusal lives at the one seam every vendor route passes through, beside the
`/admin/` envelope normalization, and is held to three narrowings: the
`/get-session` endpoint exactly, status 200 only, and a body that is exactly the
literal `null`. The signed-in answer and better-auth's JS API are untouched.

No error code is minted: `UNAUTHENTICATED` is an existing `StandardErrorCode`
member, derived from the status through ADR-0112's own map.

Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj
Co-authored-by: Claude <noreply@anthropic.com>
…e 401 refusal

`get-session also survives a degraded optional plugin` used the anonymous
`200` as its vehicle for "the route reached better-auth and came back". The
request is anonymous, so that vehicle is now the declared 401 envelope.

The claim under test is unchanged. The assertion is also strengthened rather
than merely moved: it now pins `error.code === 'UNAUTHENTICATED'` and the
recorded degradation, so it cannot pass on a 401 produced for some other
reason by a degraded instance.

Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj
Co-authored-by: Claude <noreply@anthropic.com>
…DR-0087 disposition

The first draft carried a `FROM -> TO` rewrite table, which the ADR-0087 gate
reads as a migration prescription — correctly, by its own rules — and a
prescription refuses the `no-migration-prescription` category.

The category is nonetheless the right one: this change retires no metadata
surface, so `objectstack migrate meta` has nothing to rewrite and no ledger
entry can be written for it. What moves is an HTTP status plus an SDK method's
promise contract, and the changeset is the only channel that reaches those
consumers.

So the consumer guidance stays — restated as prose plus a `try`/`catch` example,
the shape the automation-trigger status unification used for the same class of
change — and the disposition is recorded in writing.

Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj
Co-authored-by: Claude <noreply@anthropic.com>
…limit contracts

`check:engine-double-contract` and `check:objectql-double-limit` both read this
new double. Its `update()` now routes through `assertEngineUpdateDispatch` and
reads the id back off the predicate, so the fake cannot be looser than the
engine it stands in for; its `find()` applies `limit`/`offset` by PRESENCE, so
`limit: 0` means no rows instead of every row.

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

Block 3 of `auth-get-session-envelope.test.ts` pinned the anonymous `null`
passing through. Its own comment recorded what it was: a pin of the RESIDUE,
green with the lift and without it, unable to redden on the lift's ablation.
The producer now closes that residue, so the block asserts the refusal instead
and names the reversal in place, with a control that fails a `me()` which
silently starts resolving a falsy session again.

`client.test.ts`'s twin is fetch-MOCK driven and so would have stayed GREEN
while pinning a wire shape no server produces any more — the more dangerous of
the two, since it announces nothing. Re-pointed by hand.

No runtime change in this package: `this.fetch` already throws on every non-2xx,
so the anonymous answer arrives as a rejection carrying `code` and `httpStatus`
with no edit to `auth.me()` itself.

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

github-actions Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 8 documentable anchor(s).

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

  • content/docs/api/client-sdk.mdx (via auth.me (sdk, the route ledger binds it to GET /api/v1/auth/get-session))
  • content/docs/kernel/contracts/auth-service.mdx (via handleRequest (symbol, a method of class AuthManager))
  • content/docs/permissions/authentication.mdx (via handleRequest (symbol, a method of class AuthManager), auth.me (sdk, the route ledger binds it to GET /api/v1/auth/get-session), /api/v1/auth/get-session (route, a path literal on a changed line))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-4.mdx (via /api/v1/auth/get-session (route, a path literal on a changed line))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 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; 100 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 — 14 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 2266438ce0ba039634f9dcec06db7be12c0d58eepackageMentionDocs.

Which tree this was computed on

This run read content/docs from 1d0dfb0957393d795bb013194a7960a833263afe — the merge of head a249cc69081c4a0f7c17b9acbc2224a4a5dfc5e0 into base 2266438ce0ba039634f9dcec06db7be12c0d58ee, 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 1d0dfb0957393d795bb013194a7960a833263afe && git checkout 1d0dfb0957393d795bb013194a7960a833263afe
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2266438ce0ba039634f9dcec06db7be12c0d58ee a249cc69081c4a0f7c17b9acbc2224a4a5dfc5e0 && git checkout -B drift-repro 2266438ce0ba039634f9dcec06db7be12c0d58ee && git merge --no-ff a249cc69081c4a0f7c17b9acbc2224a4a5dfc5e0

node scripts/docs-audit/affected-docs.mjs --json 2266438ce0ba039634f9dcec06db7be12c0d58ee

⚠️ 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 2266438ce0ba039634f9dcec06db7be12c0d58ee → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

auth.me() returns the literal null for an anonymous caller, which no value of its declared SessionResponse can express

2 participants