stacklok / toolhive / 30286866222

70%

Build Type

push

github

Committed by web-flow

Commit Message

Fix Cedar deny-all when upstream JWT lacks identity claims (#6022)

With the embedded auth server active, the runner force-injects the first
upstream as Cedar's PrimaryUpstreamProvider, so policies were evaluated
against that provider's access token alone. Many OIDC providers assert
profile claims only in the id_token, so a JWT-shaped access token without
`email` parsed cleanly, never hit the opaque-token fallback from #5147, and
left `principal has claim_email` false for every user: tools/list returned
empty and tools/call 403, with no diagnostic. Audit logging reads the
AS-issued token, so it kept naming the correct user and pointed
investigations away from authorization.

Supplement the missing profile claims from the same provider's stored
id_token. `name` and `email` fall back to it when the access token omits
them; access-token values always win. Provenance is preserved by
construction, so `principal has claim_email` still means "this upstream
asserted an email".

The ToolHive-issued token is deliberately not a claim source here. In a
multi-upstream chain its mirrored name/email always belong to the FIRST
configured upstream (validateChain pins chain[0] to upstreams[0], and only
the first leg resolves identity from its provider), while
primaryUpstreamProvider may name any upstream — so using it would attribute
one IdP's email to another.

`sub` is never supplemented: it becomes the Cedar principal entity ID, where
Cedar offers no `has`-style guard, so a missing access-token `sub` fails
closed with ErrMissingPrincipal rather than having a principal chosen for it.

The id_token is read without checking `exp`, deliberately. It is a record of
what the upstream asserted at login, not a presented credential; enforcement
of that contract belongs at the RFC 8693 consumer. Enforcing it here would
let a policy permit early in a session and silently deny later, since
sessions outlive id_tokens by days.

Two rate-limited warnings make the condition diagnosa... (continued)

Coverage Stats

89 of 92 new or added lines in 1 file covered. (96.74%)

34 existing lines in 9 files now uncovered.

80590 of 115789 relevant lines covered (69.6%)

86.47 hits per line