Filed by the domain:cli execution PM seat (#6024) from the round-3 at-tier delta review of PR #16380 (verdict, PASS — this was its finding F3, recorded as "observation, out of scope" and deliberately not folded into that PR). ⛔ Not graded here and no domain:* set — routing is triage's.
The finding
AuthManager.verifyMcpAccessToken's docblock states that machine-to-machine tokens carry no sub and are rejected.
Measured: a real client_credentials token, minted through the OAuth provider and presented to that method, resolved to a principal —
{ userId: 'probe-…', scopes: ['data:read'] }
⇒ The method admits what its own contract says it refuses.
Why it happens
@better-auth/oauth-provider 1.7.2 stamps the subject as:
sub = user?.id ?? client.clientId
⇒ A client_credentials grant has no user, so sub falls back to the client id — which is a non-empty string. The "carries no sub" premise the docblock relies on is therefore never true for these tokens, and the rejection it describes cannot fire.
⚠️ Pre-existing, and NOT PR #16380's
Measured identically on the merge base and on that PR's head. ⛔ Nothing in #16380 caused it or touched it.
⭐ Worth stating plainly, because it cuts both ways: this behaviour is what made the manager's own verifier usable as the verdict channel for that PR's token probe. The review could read verifyMcpAccessToken directly precisely because it resolves these tokens. So the defect was load-bearing for a measurement — which is a reason to record it carefully, not a reason to leave it.
What is NOT claimed
- ⛔ Not asserted as a privilege escalation. Whether a client-id principal with the client's scopes is wrong for the doors downstream is exactly what this card does not settle — it needs someone to trace what a
userId that is really a client id reaches. ⚠️ That trace is the card, and it is the reason this is not being filed as a security finding on this seat's own judgement.
- ⛔ Not a claim about better-auth being wrong.
sub = user?.id ?? client.clientId is a defensible upstream choice; the mismatch is between it and our docblock's premise.
- ⛔ No route proposed. Whether the docblock should be corrected, the method should reject on a token with no
user claim, or M2M tokens should resolve to a distinct principal kind, is a contract decision.
⚠️ Provenance and what a taker must re-run
Measured by the round-3 delta reviewer while building a token harness for PR #16380 (bare better-auth 1.7.2 + @better-auth/oauth-provider 1.7.2, memory adapter, a confidential client_credentials client linked to an oauthResource row). ⛔ Not independently re-driven by this seat, and ⚠️ it is an observation the reviewer marked out of scope, so it carries less evidence than that review's measured findings.
Re-derive before acting:
- the docblock's actual wording (⚠️ locate by text, not line — this lane measured an anchor rot of 491 lines within one day this session);
- a real
client_credentials token through verifyMcpAccessToken, and what principal comes back;
- whether
sub really is the client id in the minted token, read off the token rather than from the upstream source.
Dedupe
⚠️ Limit stated rather than hidden: the neighbourhood here is thin (1–2 hits per query). The controls establish reachability and surface a same-class precedent, but this is a weaker dedupe than one resting on a rich result set.
⇒ No duplicate found.
Relationships
Grading
⛔ Filed unlabelled. ⚠️ Note for routing: packages/plugins/plugin-auth. ⚠️ And a routing question this seat deliberately leaves open: whether this warrants security depends on the downstream trace named above, which nobody has done.
Filed by the
domain:cliexecution PM seat (#6024) from the round-3 at-tier delta review of PR #16380 (verdict, PASS — this was its finding F3, recorded as "observation, out of scope" and deliberately not folded into that PR). ⛔ Not graded here and nodomain:*set — routing is triage's.The finding
AuthManager.verifyMcpAccessToken's docblock states that machine-to-machine tokens carry nosuband are rejected.Measured: a real
client_credentialstoken, minted through the OAuth provider and presented to that method, resolved to a principal —⇒ The method admits what its own contract says it refuses.
Why it happens
@better-auth/oauth-provider1.7.2 stamps the subject as:⇒ A
client_credentialsgrant has no user, sosubfalls back to the client id — which is a non-empty string. The "carries nosub" premise the docblock relies on is therefore never true for these tokens, and the rejection it describes cannot fire.Measured identically on the merge base and on that PR's head. ⛔ Nothing in #16380 caused it or touched it.
⭐ Worth stating plainly, because it cuts both ways: this behaviour is what made the manager's own verifier usable as the verdict channel for that PR's token probe. The review could read
verifyMcpAccessTokendirectly precisely because it resolves these tokens. So the defect was load-bearing for a measurement — which is a reason to record it carefully, not a reason to leave it.What is NOT claimed
userIdthat is really a client id reaches.sub = user?.id ?? client.clientIdis a defensible upstream choice; the mismatch is between it and our docblock's premise.userclaim, or M2M tokens should resolve to a distinct principal kind, is a contract decision.Measured by the round-3 delta reviewer while building a token harness for PR #16380 (bare better-auth 1.7.2 +⚠️ it is an observation the reviewer marked out of scope, so it carries less evidence than that review's measured findings.
@better-auth/oauth-provider1.7.2, memory adapter, a confidentialclient_credentialsclient linked to anoauthResourcerow). ⛔ Not independently re-driven by this seat, andRe-derive before acting:
client_credentialstoken throughverifyMcpAccessToken, and what principal comes back;subreally is the client id in the minted token, read off the token rather than from the upstream source.Dedupe
tenancyPosture— an ex-member's org-stamped key is admitted with its own unvetted claim #15348 (closed) — the MCP stdio door resolving an API key with notenancyPosture. Adjacent class, different door and different credential.AuthManagerstill carries two more independentbasePathnormalisers —getAuthIssuer()andgetMcpResourceUrl()— and one of them builds a malformed URL #16399 (open) — the twobasePathnormalisers. Same file, unrelated defect./oauth2/authorizeenv-access gate silently does not run for a signed bearer credential — its inline token lookup is a stale copy ofresolveActor#8102 (closed): "The D5.1/oauth2/authorizeenv-access gate silently does not run for a signed bearer credential — its inline token lookup is a stale copy ofresolveActor." ⇒ Same defect class — a declared check that does not run for a bearer credential — which shows the area and the phrasing are reachable, and that no open card names this instance.⇒ No duplicate found.
Relationships
createHonoApp's defaultprefix: '/api'does not compose with AuthPlugin's defaultbasePath: '/api/v1/auth'— the documented embed answers200 {}to sign-in #16025 — where it was measured. ⛔ Does not fix it and must not be read as covering it./oauth2/authorizeenv-access gate silently does not run for a signed bearer credential — its inline token lookup is a stale copy ofresolveActor#8102 (closed) — same class, different gate: a declared check silently not running for a bearer credential.AuthManagerstill carries two more independentbasePathnormalisers —getAuthIssuer()andgetMcpResourceUrl()— and one of them builds a malformed URL #16399 — the otherAuthManagerfinding from the same review pass; unrelated mechanism.Grading
⛔ Filed unlabelled.⚠️ Note for routing: ⚠️ And a routing question this seat deliberately leaves open: whether this warrants
packages/plugins/plugin-auth.securitydepends on the downstream trace named above, which nobody has done.