stacklok / toolhive / 28665527202

68%

Build Type

push

github

Committed by web-flow

Commit Message

Unify subject provider defaulting, hard-error xaa (#5708)

* Unify subject provider defaulting, hard-error xaa

The YAML and operator paths each defaulted a backend's
SubjectProviderName to the first configured upstream, but drifted
apart: the YAML path was missing aws_sts entirely (#5687), so an
aws_sts backend with an unset field failed at request time instead
of being defaulted. Separately, both paths silently picked
upstream[0] for xaa even with multiple upstreams configured, a
security-relevant footgun since sending the wrong subject token to
Step A yields a confusing IdP error or a token minted for the wrong
subject (#5697).

Extract a shared authtypes.DefaultSubjectProviderName helper used by
both the YAML (pkg/vmcp/config/defaults.go) and operator
(cmd/thv-operator/controllers/virtualmcpserver_controller.go) paths,
covering token_exchange, aws_sts, and xaa. xaa now hard-errors on
ambiguous multi-upstream configs on both paths; on the operator side
this surfaces as a non-fatal per-backend AuthConfigError condition
rather than a Reconcile failure.

Fixes #5687, #5697

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Deep-copy backend strategy, correct xaa risk framing

DefaultSubjectProviderName shallow-copied each strategy's sub-config
by hand, leaving slice fields (Scopes, RoleMappings) aliased with the
caller's original struct. Use the generated DeepCopy() instead, which
removes the aliasing risk and lets the doc comment drop its "never
mutated" hedge.

The doc comment also framed the wrong-subject-token risk as specific
to xaa, which isn't accurate: token_exchange and aws_sts read from
the same UpstreamTokens map and are exposed to the identical risk.
xaa hard-errors first purely because it has no existing deployments
to break; hard-erroring the other two would be a breaking change
needing a deprecation path, tracked separately in #5697.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Exclude backends with failed auth f... (continued)

Coverage Stats

110 of 117 new or added lines in 5 files covered. (94.02%)

5 existing lines in 2 files now uncovered.

72551 of 107038 relevant lines covered (67.78%)

63.58 hits per line