Cedar must not take its principal from an unverified id_token

[jhrozek]

Problem

Since #6048, when a pinned upstream provider's access token is opaque, the Cedar authorizer sources policy claims from that provider's stored id_token — including sub, which becomes the Cedar principal entity ID.

For type: oidc upstreams that token is cryptographically validated before storage (at login, upstream/oidc.go:318, and again on refresh with a subject-stability check, oidc.go:506-524). For type: oauth2 upstreams it is not validated at all:

• convertOAuth2Token (pkg/authserver/upstream/oauth2.go:257) extracts token.Extra("id_token") with no provider-type gate.
• BaseOAuth2Provider — used standalone for type: oauth2 — has no verifier field.
• callback.go:163 persists it verbatim; pkg/auth/token.go:1273-1278 projects it into Identity.UpstreamIDTokens.

So a type: oauth2 upstream that returns an id_token has its unverified sub become the authorization principal, while the authenticated subject comes from a separate path (userinfo / identityFromToken / synthesis). Nothing anywhere compares the two — grepped callback.go, oauth2.go, storage/*.go.

Severity

Not exploitable, and reviewed as such by four independent passes:

• Skipping signature validation is permitted here — the token arrives over TLS in a direct client↔token-endpoint response (OIDC Core 1.0 §3.1.3.7 item 6).
• A hostile upstream already controls Identity.Subject via userinfo, so it gains nothing by lying in an id_token.

What remains is a provenance defect: for an honest upstream, ToolHive authenticates via one path and authorizes via a second that nothing cross-checks. Filed because it is a live, ungated inconsistency that will become a real bug for the next non-GitHub oauth2-typed upstream.

d0f5d9b (PR #6052 follow-up) corrected the doc comment that previously asserted this could not happen. This issue tracks actually enforcing it.

Why the obvious fix is not a one-liner

Clearing Tokens.IDToken for non-OIDC providers is ~4 lines, but a prototype surfaced four problems:

1. It breaks XAA in a legitimate config. pkg/vmcp/auth/strategies/xaa.go:113 presents the stored id_token as an RFC 8693 subject_token, failing with ErrUpstreamTokenNotFound when empty. InjectSubjectProviderNames (pkg/vmcp/config/defaults.go:114-119) auto-selects the subject provider from rc.Upstreams with no provider-type filter, and nothing cross-validates strategy against upstream type. Critically, validateEndpointOrigin (upstream/oidc.go:554) requires token_endpoint to share the issuer's origin, so a genuine OIDC IdP behind a gateway cannot be type: oidc and is forced onto type: oauth2 — where its id_token is real and signed, and XAA presenting it is correct.
2. Flag polarity must be fail-safe. A discardUnverifiedIDToken bool defaults to "keep the token", so safety depends on remembering to opt in, and any future provider built by struct literal (the pattern oidc.go:188-191 already uses) silently inherits unverified storage. Invert it: verifiesIDToken, set only by OIDCProviderImpl, so the zero value discards.
3. The invariant would not hold for existing sessions. refresher.go:156-158 deliberately carries the stored id_token forward when a refresh omits one, so every pre-upgrade session keeps re-persisting its old unverified id_token for the rest of its life (bounded by RefreshTokenLifespan).
4. The break would be silent. No WARN when a non-empty id_token is dropped, and docs/operator/virtualmcpserver-kubernetes-guide.md:712 advises "Requesting the openid scope for that upstream is what fixes it" — which becomes a literal no-op for type: oauth2. Correct advice is to configure the upstream as type: oidc.

Options

A. Gate at storage (root-cause location; all current and future consumers inherit it). Requires verifiesIDToken polarity, a WARN naming the type: oidc remedy, the guide fix, a decision on pre-upgrade sessions, and a release note for the XAA behaviour change.

B. Gate at the consumer — plumb OIDC-verified provenance through to Cedar so it refuses an unverified id_token principal while XAA keeps its token. Preserves the hybrid-flow capability, but duplicates upstream-type knowledge and needs a new field through UpstreamCredential ({AccessToken, IDToken} today) → Identity → cedar ConfigOptions.

A is the smaller, more durable shape and is preferred on root-cause grounds; B is the only one that does not break XAA-on-oauth2. The tradeoff is a product call about whether that configuration is supported.

Either way, pkg/auth/identity.go's UpstreamIDTokens doc should record the resulting provenance guarantee — it currently documents only the expiry contract, which is where a future consumer will look.

Test gap to close alongside

The seam is untested, which is why the prototype looked free. xaa_test.go:42 and the Cedar tests all construct Identity directly from literal maps, so no test connects "the provider stored an id_token" to "a downstream consumer reads it". A provider-layer change cannot fail any XAA or Cedar test. An integration test across provider → storage → Identity → consumer would have caught the regression immediately.