stacklok / toolhive / 24804357731

67%

Build Type

push

github

Committed by web-flow

Commit Message

Fix Cedar upstream-claim evaluation on VirtualMCPServer (#5002)

* Populate Cedar PrimaryUpstreamProvider for vMCP

VirtualMCPServer's operator converter never set
AuthzConfig.PrimaryUpstreamProvider, so Cedar policies that referenced
upstream claims (e.g. principal.claim_department) failed at runtime.
Cedar evaluated against the ToolHive-issued AS token rather than the
upstream IDP token, and the claim was missing.

Derive the field from Spec.AuthServerConfig.UpstreamProviders[0].Name in
convertIncomingAuth when an embedded auth server with upstream providers
is configured. Mirrors injectSubjectProviderIfNeeded in
virtualmcpserver_controller.go (outgoing auth) and
injectUpstreamProviderIfNeeded in pkg/runner/middleware.go (thv run
path). Leaves the field empty when no embedded AS or no upstreams so
Cedar correctly falls back to ToolHive-issued claims in those modes.

Fixes #4997

* Reject VirtualMCPServer authz without an upstream IDP

When IncomingAuth.AuthzConfig is set but no upstream IDP is configured,
Cedar silently evaluates policies against the ToolHive-issued AS token.
That token's claim namespace (sub, aud, tsid) can overlap upstream
claims and authorize against the wrong identity, so the misconfig must
be surfaced rather than deployed.

Add validateAuthzUpstreamAvailable to the VirtualMCPServer reconciler
chain. When AuthzConfig is set but AuthServerConfig is nil or
UpstreamProviders is empty, mark the server Failed and set
AuthServerConfigValidated=False with reason AuthzRequiresUpstream. The
user-facing message points at spec.authServerConfig.upstreamProviders,
which is where the fix belongs.

Extract runAuthValidations from runValidations so the auth-related
checks live together and gocyclo stays happy. No behavior change in the
moved block.

Belt-and-suspenders companion to the converter fix in the previous
commit: the converter wires the provider name when upstreams exist; this
validator makes the absence of upstreams an explicit failu... (continued)

Coverage Stats

73 of 80 new or added lines in 2 files covered. (91.25%)

16 existing lines in 6 files now uncovered.

58727 of 88606 relevant lines covered (66.28%)

61.42 hits per line