Fix contradictory upstream ID token exp contract

[JAORMX]

Summary

Follow-up to #6022, addressing the three non-blocking points @jhrozek raised in that
PR's approval. Comment and documentation only — no logic changes, no behaviour
change. Investigating them turned up two further instances of the same defect and
one overstated claim, so the diff is slightly wider than the three items; details
below.

• Claim names are no longer borrowed from the auth server. profileClaimsFromIDToken
  spelled name/email with session.NameClaimKey/session.EmailClaimKey. What
  this code reads is an upstream provider's id_token, where those are OIDC Core
  §5.1 wire names fixed by the spec — not names ToolHive chooses. Replaced with
  local oidcNameClaim/oidcEmailClaim consts and dropped the
  pkg/authserver/server/session import.
• The exp contract no longer contradicts its own consumers. The docs said
  callers "MUST check exp" before using a stored id_token, while the Cedar
  authorizer is now a consumer that deliberately does not. Rewritten to distinguish
  presenting the token as a credential from reading its claims, at all four
  sites plus the stale TODO (see below).
• resolveClaims' doc comment trimmed from ~60 to ~45 lines, keeping the
  widening-hazard bullets and pointing at Identity.UpstreamIDTokens for the exp
  rationale instead of restating it.
• Operator guide: documents that the stored id_token is used even after it
  expires, and why — the one user-visible consequence of Fix Cedar deny-all when upstream JWT lacks identity claims #6022 that lived only in Go
  comments.

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• Linting (task lint-fix)
• E2E tests (task test-e2e)

No tests added or changed: the diff is comment/doc-only, so there is no new
behaviour to pin. Verification was hash-bracketed against the exact committed bytes
— task lint-fix 0 issues, codespell clean over all 5 changed files,
task license-check pass, and go test -race clean over ./pkg/authz/... ./pkg/auth/....

task test reproduces three failures that are pre-existing limitations of my
sandbox and unrelated to this change: pkg/api TestNewServer_ReadTimeoutConfigured
and TestSecurityHeaders (no container runtime — "no available runtime found"), and
pkg/secrets/keyring TestCompositeProvider_RealProviders (no keyring). Each fails
on the environment rather than on any assertion about behaviour, and all three fail
identically on origin/main content in a clean worktree with none of this change
present.

One further failure, disclosed rather than filtered: an earlier suite run also
produced pkg/vmcp/server TestForwarding_Progress_RealBackend — a 60s context
timeout waiting for a notifications/progress message, at 1m1.1s. It did not
reproduce, and is not attributable to this change:

• A later full task test, run against exactly these committed bytes (the five file
  hashes verified identical before and after the run), passed that test — the only
  failures were the three environmental ones above. That is the strongest evidence
  here because it reproduces the failing configuration: same full parallel -race
  suite, same box, same test. The isolated runs below could be waved away as "of
  course it passes alone"; this one cannot.
• The failing run happened while the machine was OOM-killing two unrelated background
  jobs of mine. So the environment was demonstrably failing at the time — independent
  of this test's own behaviour — and on a box that starves processes badly enough to
  reap them, any deadline can be missed.
• The test has a thin margin by construction, so a loaded box is enough to explain it.
  Its body normally takes 30.03s (measured with go test -v; teardown is a further
  21ms) against a 60s context.WithTimeout. -race adds only ~1.0s, so the 30s is
  a fixed wait inside the body rather than CPU-bound work — leaving roughly 30s of
  slack, which a full parallel suite on a starving box will consume. Observed at
  1m1.1s, i.e. a ~2× overrun, which is what a missed deadline looks like rather than a
  hang.
• Running that test in isolation on a quiet box passes 12/12 — 3× and 3× per side,
  with and without -race, comparing origin/main content in a separate worktree
  against this branch. Timings 30.039s–31.103s; the largest branch-minus-main
  difference in either configuration is ~4ms. If this change had slowed that relay,
  that is not what the numbers would look like.
• Causally there is nothing to slow: the entire non-comment delta is three lines (an
  import removal, a two-const block, and a var referencing it), and the new const
  values are byte-identical to the constants they replace. The one mechanism by which
  removing an import can change runtime behaviour — losing a package's init() — does
  not apply: pkg/authserver/server/session is still in the authorizer's transitive
  dependency tree via pkg/auth, so nothing left the build, and that package has no
  init() regardless.

CI is a better arbiter than my sandbox for a real-backend integration test needing a
quiet machine, and it passed that test on e837d69.

Changes

File	Change
pkg/authz/authorizers/cedar/core.go	local OIDC claim-name consts; session import dropped; resolveClaims doc trimmed; provenance mechanism restored with code pointer
pkg/auth/identity.go	Identity.UpstreamIDTokens: the exp contract rewritten around present-as-credential vs read-its-claims
pkg/auth/upstreamtoken/types.go	same treatment on UpstreamCredential.IDToken, TokenReader.GetAllUpstreamCredentials, Service.GetValidTokens
pkg/auth/upstreamtoken/service.go	same on GetAllUpstreamCredentials; replaced a stale TODO(auth) (see notes)
docs/operator/virtualmcpserver-kubernetes-guide.md	expired-id_token behaviour documented for operators

Does this introduce a user-facing change?

No. Comments and one operator-guide paragraph; no code paths altered.

Special notes for reviewers

Two corrections to the reasoning in #6022's review, both measured rather than
argued. I'd have repeated both as fact otherwise:

1. Dropping the session import removes zero packages. The review estimated it
   pulls ~13 packages in to spell two literals. Measured directly on both sides —
   go list -deps ./pkg/authz/authorizers/cedar | grep -c fosite returns 8 on
   origin/main content and 8 on this branch (the worktree used for the "before"
   reading does still carry the session import, confirming it is pre-change
   content). Fosite reaches the package via pkg/auth, which cedar must import for
   auth.Identity, and session contributed a strict subset of 7. So the
   justification is dependency direction only, not dependency count.
2. The comment being removed promised a guarantee that never existed. It claimed
   "a rename on either side is a compile error". But pkg/authserver/upstream/oidc.go
   extracts these claims via hardcoded json tags (json:"name", json:"email"), not
   via the session constants — so a rename on the extraction side would never have
   been caught. That makes this item a correctness fix rather than a matter of taste.

I replaced a pre-existing TODO(auth), which is the biggest liberty in this diff.
service.go previously read: "the 'check exp' contract on UpstreamCredential.IDToken
is documented but not enforced here. Enforcement belongs at the RFC 8693
token-exchange consumer when it receives the credential." Once #6022 documented the
Cedar path as deliberately not checking exp, that TODO asserted outstanding work
where there is none. I replaced it with a statement of the design. The reasoning, so
you can judge it rather than take my word:

• For the claims-reading consumer, enforcement is affirmatively unwanted — that is
  the substance of Fix Cedar deny-all when upstream JWT lacks identity claims #6022.
• For the credential-presenting consumer, the party obliged to validate exp is the
  recipient (the IdP verifying its own id_token as an RFC 8693 subject_token). A
  presenter's local pre-check is a latency and error-message optimisation, not a
  security control: skipping it cannot cause an expired assertion to be accepted,
  because acceptance is the IdP's decision. pkg/vmcp/auth/strategies/xaa.go:38-44
  documents exactly this and surfaces the resulting invalid_grant.
• A local check would use ToolHive's own clock-skew leeway and could reject tokens
  the IdP would accept.
• There is no third consumer. The only two sites that read a token out of
  UpstreamIDTokens are pkg/authz/authorizers/cedar/core.go:666 and
  pkg/vmcp/auth/strategies/xaa.go:113. (A plain grep for the field name returns
  ~24 hits — most are doc comments, plus the write at pkg/auth/token.go:1278 and
  identity.go's MarshalJSON loop, which reads it only to redact.) That census is
  true as of this commit, which is why the replacement text carries an explicit
  clause about what a future consumer presenting to a non-validating sink must do.
  Please keep that clause if you edit the comment.

If you think the TODO was still live, say so and I'll restore it and flag the
contradiction another way.

identity.go is now the single home for the exp rationale, with the other four
sites pointing at it. Worth knowing that the canonical doc (UpstreamCredential) was
the last site still stating the strict rule while three docs pointed at it — so a
reader following a cross-reference landed on the version contradicting where they came
from. That's fixed; a grep for MUST check|MUST validate|must check its|MUST honour
over pkg/auth now returns one hit, properly qualified.

Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.19%. Comparing base (e837d69) to head (395311a).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6036      +/-   ##
==========================================
+ Coverage   72.16%   72.19%   +0.02%     
==========================================
  Files         721      721              
  Lines       75069    75069              
==========================================
+ Hits        54174    54193      +19     
+ Misses      17033    17013      -20     
- Partials     3862     3863       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.