fix(plugin-auth): the MCP surface refuses a client_credentials token — principal-bound to a human

[claude]

Fixes #16418

Clause-②: no

(declaration line written by the domain:services review seat, not by the implementer. This diff adds 0 exported symbols and narrows the accept set — a client_credentials token that resolved now returns null — so it is a pull-back to the contract auth-manager.ts:6133 already published ("Client-credentials (M2M) tokens carry no sub and are rejected"), not a widening. The patch level stands. Full two-limb measurement and the seat's correction of its own earlier yes: #16418 comment 5620303472. The needs:contract-review carrier was cleared for the same reason — ⚠️ STRIPPED because the declaration was wrong, ⛔ NOT because a contract review happened.)

Route (b) as ruled (director seat, summon #18, decision batch #104, 2026-09-09). The MCP surface stays principal-bound to a human; the API-key door is untouched; no machine principal is minted here.

What changed

AuthManager.verifyMcpAccessToken resolved a client_credentials (machine-to-machine) access token to a principal while its own docblock declared such tokens rejected. The docblock's premise was that they "carry no sub". The provider stamps the subject as the user id when a user exists and the client id otherwise, so the premise is never true for an M2M token and the rejection it described could not fire.

The method now reads the subject and the client identity as a pair and returns null when they are the same principal. The docblock names that mechanism in place of the false sentence. Nothing else on this path moved.

The discriminator — chosen on measurement

The ruling deliberately did not choose it: "What the ruling does not choose — the dev's, under the services seat's Clause-②: yes claim: the discriminator." So it is defended here.

What was measured

Two tokens minted by one real authorization server, booted in-test from the exact options AuthManager hands @better-auth/oauth-provider (1.7.2, the version today's lockfile resolves — M2 re-measured, unchanged since the card was filed). Both payloads were read off the token, never inferred from provider source (card re-derive step 3):

claim	authorization_code (human)	client_credentials (machine)
sub	the sys_user id	the client id
client_id	the client id	the client id
azp	the client id	the client id
sid	present	absent
aud	array — MCP resource and userinfo	string — MCP resource only

A third leg was measured for the fail-closed check below: an access token re-minted through the refresh_token grant carries client_id and a sub that still differs from it, so nothing on the long-lived MCP client path is refused by the new rule.

Why the sub / client_id pair

It is not a quirk of this provider — it is what RFC 9068 (JWT Profile for OAuth 2.0 Access Tokens) defines, and the tokens this deployment mints carry that profile's own media type in their header:

• §2.2 makes client_id REQUIRED on a JWT access token.
• §2.2.3.1 fixes what sub means beside it: the resource owner for a grant that had one, and "an identifier the authorization server uses to indicate the client application" for a grant that did not.

So a token whose sub equals its own client_id states, in the authorization server's own words, that no human delegated it. That is a first-class fact about the grant, carried by the credential itself, and it survives a provider bump — and even a provider swap — in a way an implementation detail does not.

Two halves make it fail closed rather than fail quiet:

1. Both spellings are compared independently (client_id and azp), never collapsed through a ?? chain first. A token that disagrees with itself across the two is refused on either match; a precedence chain would let the losing spelling carry the client id past the comparison. There is a pin for exactly that shape.
2. Neither claim present ⇒ refused. The discriminator then has no input, and a check that cannot run must not silently pass (Route & surface ownership §3, and the defect class of The D5.1 /oauth2/authorize env-access gate silently does not run for a signed bearer credential — its inline token lookup is a stale copy of resolveActor #8102 — a declared check that never fires). No token this AS mints is affected: client_id and azp are both stamped unconditionally.

The two alternatives, and why not

• ⛔ sid presence. It does separate today's two shapes, and it is a positive claim, which is what the ruling's input paragraph asked for. But it is an OIDC session-management convenience that the installed provider already gates per client on ID tokens (enableEndSession || backchannelLogoutUri). A bump that gated it on access tokens the same way would refuse every human on this surface — a total MCP outage, which is precisely the failure mode this method must not have (constraint C1: nothing here may refuse a user token that resolves today). The absence is recorded as an assertion in the new pin so the fact stays measured, but it is not the gate.
• ⛔ A sys_user resolution before the principal is assembled. This was the ruling's other suggested positive form, and it is not taken, for four reasons. (a) It converts a deliberately local, I/O-free verifier — its own docblock says so — into one that depends on the data engine, turning a transient store error into a 401 for a legitimate human on every MCP request. (b) It answers the wrong question: a row's existence is not humanity. isHumanUserRow exists in this very package precisely because usr_system is a row and not a person, so the honest predicate would be a human-row read, not an existence read — more machinery, still in the token verifier. (c) It would fork the identity model: the existence check would hold on the MCP OAuth chain only, and not for API keys or sessions, which is a partial invariant rather than a contract. (d) It cannot be adopted without editing the negative-control suite the ruling requires to stay unchanged — those tests resolve a principal with no data engine wired, so requiring one would redden acceptance (2)'s own pins.

The residual gap is stated rather than hidden: if a future provider version minted an M2M token whose sub is neither the client id nor a sys_user id, the pair rule would admit it. That gap is closed by the option-liveness and minted-token pins in auth-manager.mcp-oauth-resource.test.ts — both read the installed provider, so a bump that changes the shape reddens there rather than passing silently.

验收备注

Carried verbatim from the ruling (comment 5595112789), each with what discharges it.

1. "A real client_credentials token minted through the real @better-auth/oauth-provider (the trace's recipe, incl. client_credentials_scopes on the client row) → verifyMcpAccessToken returns null; through the MCP HTTP door → 401, fail-closed, no context assembled."
   ✅ First half: auth-manager.mcp-oauth-resource.test.ts boots the real AS, registers a confidential client authorised for that grant and linked to the MCP resource, drives the AS's own token endpoint, and hands the minted token to a real AuthManager verifying against that server's JWKS. Second half: the door's null → 401 with nothing assembled mapping is pinned in packages/runtime/src/http-dispatcher.mcp-oauth.test.ts, which now asserts the MCP runtime is never reached and no execution context is built — not merely that the status is 401. That file fakes the verifier by design (its own docblock says the verifier half lives in plugin-auth); the two pins compose to the acceptance sentence, and neither carries it alone.
2. "Negative control: a real user token through the full discovery → DCR → authorize → consent → token flow behaves byte-for-byte as today; auth-manager.mcp-oauth.test.ts + auth-manager.mcp-oauth-resource.test.ts stay 44/44."
   ✅ The two files were 44/44 before this change and all 44 are still green; they are now 51/51, the 7 additions being this card's pins. The user leg is not asserted only by the old fixtures: the new differential runs the full flow on the same server, same JWKS, same audience as the machine leg, one variable apart, and requires the resolved principal to be exactly the user id, the granted scopes and the client. Without that half a method that refuses everything would satisfy the refusal.
3. "The green test 「rejects a sub-less (client-credentials / M2M) token」 is replaced or joined by one that uses a minted M2M token, so the pin covers the behaviour and not the docblock's former premise (the dev's own out-of-scope note; same family as The D5.1 /oauth2/authorize env-access gate silently does not run for a signed bearer credential — its inline token lookup is a stale copy of resolveActor #8102)."
   ✅ Joined. The old test kept its subject (a sub-less token is still refused) and lost its false title — it no longer claims to be the M2M pin. The M2M pin is the minted-token differential, plus claim-shape cases for both client spellings and for the no-client-claim case.
4. "The API-key door is untouched and pinned: x-api-key / Bearer osk_… on MCP HTTP and OS_MCP_STDIO_API_KEY on stdio (ADR-0101 D1) resolve exactly as before — separate chain (resolveApiKeyPrincipal), separate credential shape (extractJwtBearer routes only three-segment non-osk_ bearers to the JWT verifier)."
   ✅ No file on that chain is in this diff. The pins are the two REGRESSION: cases in http-dispatcher.mcp-oauth.test.ts (header key and prefixed bearer both resolve the key principal, unscoped) and the bearer-routing case in resolve-execution-context.test.ts; all are green in the runs below, alongside the pin that a non-JWT opaque bearer still takes the session path. The credential shapes cannot collide: the prefixed key and the opaque session bearer are both rejected by the router before the verifier, and the verifier itself still refuses a prefixed key without touching the JWKS.
5. "The docblock names the mechanism actually used; content/docs/ai/agents.mdx lines 58–88 already describe the two connection routes (OAuth for interactive clients, API key for headless) and need no change unless the dev finds a sentence that contradicts the ruling."
   ✅ Docblock rewritten around the pair rule, with the two rejected discriminators named so the next author does not "improve" it into the fragile one. The docs were read: that section says the OAuth client "acts as an agent on your behalf" and names the API key as the headless route. Nothing there contradicts the ruling, and client_credentials does not occur anywhere under content/docs. No docs change.
6. "fix(hono): mount /auth where the auth service serves, and refuse a prefix it cannot serve under #16380: trace §4 established that no landed asset depends on the old admission — nothing to re-route; a future reviewer wanting a machine-credential verdict channel reads the AS's /oauth2/token response, not this method."
   ✅ Taken as established; nothing re-routed. Recorded for the next reviewer: this PR's own probe uses exactly that channel — the token endpoint's response is read directly, and the claims are asserted on the decoded payload before the verifier is consulted at all.

Ablation

Required by the card's security discipline: put the admitted behaviour back and show the new pins go red.

Run from the committed state; the mutation removed the two guard statements and left a marker in their place. On-disk proof, not an exit code — occurrence counts across the mutation: guard-pair 1 → 0, no-client-guard 1 → 0, marker 0 → 1; worktree blob 306cd6aa… → e6aa7c5c….

5 tests red under ablation, and the real minted M2M token resolved to a principal again — reproducing the card's finding, with the client id in the userId slot:

FAIL  src/auth-manager.mcp-oauth-resource.test.ts > [#16418] ... > DIFFERENTIAL: same server, same JWKS ...
AssertionError: a client_credentials token must assemble NO principal on the MCP surface ...: expected { …(3) } to be null
+ Received:
{ "clientId": "headless-integration-client",
  "scopes": [ "data:read" ],
  "userId": "headless-integration-client" }

The other four are the claim-shape cases (sub equal to azp; sub equal to client_id; the self-disagreeing token; the no-client-claim token). The 44 pre-existing tests stayed green under ablation — the ablation moves this card's pins and nothing else.

Restoration proven by state, not by exit code: git checkout HEAD -- naming the file (never a bare git checkout --, which restores from the polluted index), then git diff --quiet HEAD exits 0, the worktree blob hash is 306cd6aa1f4713e4fea363e74253131b3a7adab0 — byte-identical to the HEAD blob — the marker count is back to 0, and git status --porcelain is empty. The mutation script carried an EXIT INT TERM trap on an absolute path throughout.

Verification

Exit codes captured before any pipe.

• Dependency closure build — pnpm --filter '@objectstack/plugin-auth^...' build and then the full package build CI's typecheck job requires (turbo run build over both package roots): 72 successful, 72 total, exit 0. The full build was needed because two gates answer PREREQUISITE NOT MET (exit 3, not a finding) without every dist/ present — recorded here so the pair is not misread as a red.
• Gates — derived from the actual diff with node scripts/pm/dispatch-gates.mjs --commands (no hand-written path list; the tool takes the change set from the merge base itself), re-derived after merging main so the derivation is not from a stale tree. 61 families, 61 run, all exit 0. Reconciled with --ran carrying every exit code: "61 derived famil(ies) accounted for — 61 run, 0 NOT-MEASURED (a DERIVED zero — all 61 recorded an exit code and none of them is 3)".
• Affected packages — full suites, not just the touched files. @objectstack/plugin-auth: 106 files / 2252 tests passed. @objectstack/runtime: 251 files / 3533 tests passed. Both typecheck scripts exit 0, test-layer ledgers unchanged (plugin-auth 10 files / 94 errors / 23 pinned; runtime 27 / 191 / 69). The two MCP OAuth suites alone: 51/51, up from 44/44 by this card's 7 pins.
• Repo-wide lint — pnpm lint (eslint . --no-inline-config, the only style authority here) run in full rather than narrowed: exit 0, no output, at 2e8563b7b.
• Not measured locally, by declaration: CI's own path-scheduled jobs — Test Core shards, Build Core, Dogfood Regression, Dogfood Verify CLI, Temporal Conformance — plus the 47 artifact-roster families, the 11 wide-population families and the 5 workflow-valued families the derivation prints as outside the 61. Those are CI's runs; the derivation names them as outside the accounted set rather than cleared by it.

Acceptance notes

Observations from this lane, noted, not filed — none is a reproducible defect, a contract violation, or a metadata-authoring trap, so none meets the filing bar:

• This deployment has no route that can produce such a client at all. The AS's DCR endpoint refuses the client_credentials grant for an unauthenticated registration, and the only registration path that may set a client's machine-grant scope ceiling requires an administrative privilege hook this deployment does not configure — so today the ceiling can only arrive by an operator editing the stored row. That narrows who could have reached the old admission; it does not change the ruling, and it is why the probe seeds the client through the server's own adapter rather than through a public endpoint. Carrier: whoever next builds an admin surface over OAuth clients.
• The synthetic session the MCP door builds for a verified token is { user: { id } } and nothing else, so no layer below the verifier ever asserts that the id names a user. That is exactly why the refusal had to land in the verifier and why the sys_user route was tempting; it is a shape worth knowing, not a defect on its own — every id that reaches it now came from a delegated grant. Carrier: any future card that widens what may reach this door.
• aud is an array on a token whose scopes include openid and a bare string otherwise, so it is grant- and scope-dependent and cannot serve as a token-shape discriminator. Recorded because the ruling listed it among the measured differences. Carrier: none.

⛔ No packages/spec change was needed (constraint C2): the fix returns null and the door's existing 401 envelope carries it, so no ERROR_CODE_LEDGER row is required.

---

Generated by Claude Code

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

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

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

• content/docs/kernel/contracts/auth-service.mdx (via AuthManager (symbol, a top-level class), verifyMcpAccessToken (symbol, a method of class AuthManager))
• content/docs/kernel/services-checklist.mdx (via AuthManager (symbol, a top-level class))
• content/docs/permissions/authentication.mdx (via AuthManager (symbol, a top-level class))

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 — 13 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 ba9f0299086cb589350b38884377caf1d138147d → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json ba9f0299086cb589350b38884377caf1d138147d

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

[claude]

Docs-drift advisory — answered by the review seat: no page falsified, measured

domain:services seat, session session_01ToDPcx9AESFubJkDiFMtKW, 2026-09-10T14:4xZ. Answering the docs-drift-check advisory above so the next reader does not re-derive it. ⛔ Advisory, not a gate — but it named the one page that could plausibly have been falsified, so it was worth measuring rather than waving through.

The measurement, on origin/main

The risk this advisory raises is specific: this PR deleted a false sentence from a code docblock (「Client-credentials (M2M) tokens carry no sub and are rejected」). If a hand-written page restated that same mechanism, the page would now be wrong. So the query is not "does the page mention auth" — it is does any listed page restate the M2M mechanism.

page	why it was listed	verifyMcpAccessToken	client_credentials / M2M / 「carry no sub」
content/docs/kernel/contracts/auth-service.mdx	the METHOD, by name	1	0
content/docs/kernel/services-checklist.mdx	the class AuthManager only	0	0
content/docs/permissions/authentication.mdx	the class AuthManager only	0	0

⇒ Zero across all three. The false premise lived only in the code docblock this PR rewrote; it was never restated in hand-written docs. Two of the three pages are anchored on the class name alone, which this diff does not change.

The one page that names the method carries it as an interface signature excerpt, not as a behavioural description:

/** Verify an OAuth 2.1 access token from the embedded AS (MCP surface). Optional. */
verifyMcpAccessToken?(token: string): Promise<{ userId: string; scopes: string[]; clientId?: string } | undefined>;

⇒ The behaviour change (which tokens resolve) is invisible to that signature, and the signature's shape is unchanged by this PR — the refused case was already a non-value before it.

⚠️ One observation, at the epistemic level it deserves — ⛔ NOT a finding filed against this PR. That excerpt types the non-value as undefined, while the implementation returns null (it did so before this PR too — the pre-image if (!userId) return null; is visible in this diff's removed lines). So the null/undefined question is pre-existing and not this PR's, and I have measured only the doc excerpt, ⛔ not the real interface declaration — an excerpt may legitimately be abridged. Worth one check by whoever owns that page (content/docs/** is domain:devx, not this lane); ⛔ I am not asserting a defect and this PR is ⛔ not widened to chase it.

⛔ Release-owned pages

This advisory listed none for this PR, and this PR touches 0 files under content/docs/ of any kind — the guardrail is not in play here.

⇒ Advisory answered, no action. ⛔ Nothing to change in this PR on account of it.

---

Generated by Claude Code