Define SPIFFE trust configuration

[jhrozek]

Summary

Both SPIFFE credential methods (X.509-SVID and JWT-SVID) need one fail-closed identity and association model before authentication can produce equivalent authorization outcomes for either method. Without a shared model, adding live SVID verification later would force a choice between duplicating trust-domain/scope/audience checks per credential type or bolting authorization onto whichever method lands first.

This PR defines that model as validated, normalized config, wired into RunConfig.Validate() so a malformed or ambiguous declaration fails server startup rather than degrading silently later. It deliberately does not perform any live SVID/bundle verification, and does not register any client with the auth server's storage — those are separate, later steps on this stacked branch (refs #6200) so each piece can be reviewed independently. Stacked on #6465.

• Adds SPIFFETrustDomainRunConfig, declaring a named trust domain, the credential methods (spiffe_x509 / spiffe_jwt) it enables, and a required BundleSource (bundle_endpoint with an https_web/https_spiffe authentication profile, or workload_api) — validated for shape only; fetching or loading a bundle is a later step.
• Adds SPIFFEClientAuthRunConfig, associating a SPIFFE principal pattern (a concrete ID or a terminal /* wildcard) within a declared trust domain with an explicit OAuth client_id, methods, scopes, resources, audiences, and grant types. client_id is never derived from the SPIFFE ID, and client authentication never implies a grant by itself — grant_types must be declared explicitly.
• Resources (RFC 8707) and Audiences (RFC 8693) are independent request dimensions: only Resources is bounded by the server's allowed_audiences allowlist (the same list DelegateClientRunConfig.Audiences validates against); Audiences is not, and may hold non-URI logical identifiers. Permission in one dimension never implies permission in the other.
• Adds NewSPIFFETrustConfig, which validates and normalizes these declarations into an immutable SPIFFETrustConfig (unconstructible from outside the package except through the constructor), and ValidateSPIFFETrust, called directly from RunConfig.Validate().
• Validation fails closed on anything that could make authorization ambiguous or order-dependent: duplicate trust-domain names/canonical trust domains, a principal whose trust domain doesn't match its declared reference, overlapping principal patterns across associations (segment-aware, so /agent/* doesn't collide with /agent-x/*), client IDs colliding with the reserved synthetic-client namespace or shaped as absolute URLs (reserved for CIMD-resolved clients), and resources/scopes outside the server's global allowlists.

Fixes #

Type of change

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

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

pkg/authserver/spiffe_trust_test.go covers principal normalization/pattern matching (wildcard boundaries, unicode/port/userinfo/query/dot-segment rejection), trust-domain validation (duplicate names, duplicate canonical trust domains, bundle-source shape and endpoint profile), and association validation (unknown/wrong trust domain, duplicate/overlapping principals, reserved client-ID prefixes, absolute-URL client IDs, resource/scope allowlisting audience independence, grant-type restriction, method enablement). No client is ever registered with the auth server's storage in this PR, so no e2e coverage is needed here.

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Special notes for reviewers

This is a "define the types" commit in a stacked SPIFFE client-auth epic. RunConfig.SPIFFETrustDomains and RunConfig.InboundGrants.SPIFFEClientAuth are validated by RunConfig.Validate() in this PR, but nothing in the auth server yet consumes the normalized SPIFFETrustConfig — no client is registered, and no credential (X.509-SVID/JWT-SVID) is ever verified. A non-empty, valid configuration therefore currently has no runtime effect beyond passing validation. Static client registration lands in a follow-on commit on this stack (#6474); live SVID/bundle verification is a further step beyond this stack. Please review this PR purely as the declarative validation/model layer.

[codecov]

Codecov Report

❌ Patch coverage is 88.35821% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.13%. Comparing base (65eaa1b) to head (fd9afd5).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/authserver/spiffe_trust.go	87.98%	37 Missing ⚠️
pkg/authserver/config.go	95.23%	1 Missing ⚠️
pkg/authserver/runner/embeddedauthserver.go	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6467      +/-   ##
==========================================
+ Coverage   78.09%   78.13%   +0.04%     
==========================================
  Files         767      769       +2     
  Lines       74306    74931     +625     
==========================================
+ Hits        58026    58551     +525     
- Misses      16275    16375     +100     
  Partials        5        5              

☔ 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.

[JAORMX]

I reviewed this against the stacked base and #6200. The validation itself is careful, especially around SPIFFE parsing and pattern overlap, but I found three model problems that I think we should resolve before building the authentication paths on top of it: the serialized config is currently accepted and ignored, resource and audience permissions are conflated, and the runtime model discards its validated trust-domain records. I left those inline, plus the malformed CIMD client ID case and two smaller standards issues.

A few review-wide notes that do not have a useful inline location:

• The issue requires a normalized authenticated principal carrying the exact SPIFFE ID, canonical trust domain, OAuth client ID, and selected method. This PR does not define that result yet. That can land with runtime integration, but we should keep the acceptance criterion open until both credential paths produce it.
• The PR adds 513 non-test, non-generated code lines, above the repository's 400-line guideline.
• Commit 44adbfd is missing the Signed-off-by trailer required by CONTRIBUTING.md.
• The exported parsing and validation helpers have no production callers yet. Keeping them private until runtime integration would avoid committing their semantics as public API too early.

CI is green. I also ran task lint successfully. My local task test run exceeded 15 minutes without reporting a failure; the GitHub Go test job passed.

[jhrozek]

Replied inline on the three model issues and the two smaller ones — thanks for the thorough pass. Summary of what changed (pushed as an amended commit):

• spiffe_trust_domains silent no-op: confirmed this is wired in a not-yet-PRed follow-on branch via a bigger InboundGrantsRunConfig normalization layer — replied inline with details. Left as-is here.
• RFC 8693 audience / RFC 8707 resource conflation: confirmed not resolved anywhere downstream either. Leaving open rather than bolting on a schema change under review pressure — happy to discuss the right shape separately.
• Trust-domain records discarded: fixed. SPIFFETrustConfig now retains canonicalized trust-domain records with a TrustDomain(name) lookup.
• CIMD predicate mismatch: fixed. Now uses oauthproto.IsClientIDMetadataDocumentURL, with a regression test for the malformed-URL case.
• Constructor-only invariant unenforceable: documented the zero value as intentionally valid instead of adding back distinguishing state, since it's now behaviorally identical to what the constructor returns for empty input.
• Mutable variable / misleading comment: both fixed.

On the review-wide notes:

• Signed-off-by: fixed, the amended commit now has the trailer.
• Exported helpers with no callers: checked actual usage across the whole (unpushed) branch stack. NormalizeSPIFFEPrincipal/MatchSPIFFEPrincipalPattern are confirmed unused outside this package's own tests anywhere in the stack, so I unexported them. SPIFFEGrantTypeTokenExchange turned out to have a real downstream consumer (a test a few commits later), so I left it exported.
• 400-line guideline: acknowledging — the trust-domain + association model reads as one logical unit; I don't think splitting it further would help review here, but open to hearing otherwise.
• Missing normalized "authenticated principal" result type: agreed this can land with runtime integration as you said; no action here.

CI is green on the amended commit.

[JAORMX]

I re-reviewed the amended head and the rest of the stack against #6200. The fixes for retaining normalized trust domains, aligning CIMD client-ID detection, documenting the zero value, immutable assignment, comments, helper visibility, and the DCO trailer all look good. CI is green.

I still think the public model needs changes before we build the credential paths on it:

1. SPIFFEClientAuthRunConfig.Audiences authorizes both RFC 8693 audience and RFC 8707 resource (pkg/authserver/spiffe_trust.go:66). Those are independent request dimensions with different syntax and policy semantics. The issue explicitly requires resources and audiences per association; the current model cannot permit one without permitting the other and cannot represent a non-URI logical audience because everything is bounded by URI-only allowed_audiences.

2. The association has no configurable grants or independent token-exchange permission. SPIFFEGrantTypeTokenExchange makes every association token-exchange-capable by construction. #6200 requires grants and token-exchange permission to be narrowed per association; client authentication should not implicitly confer a grant.

3. The trust-domain declaration omits the bundle source required by #6200. Bundle loading can be deferred, but the source belongs in the authoritative serialized model before that model becomes public.

4. RunConfig.SPIFFETrustDomains is still accepted and exposed in generated Swagger without being validated or consumed in this PR. #6473 wires it later, but this PR is not independently fail-closed: a non-empty configuration starts successfully and has no effect.

Please settle these schema commitments in the foundation PR. They become substantially harder to correct once released or consumed by the follow-on stack.

[jhrozek]

Pushed a commit that resolves all four points — thanks for pushing on this, and for confirming the earlier round of fixes.

1. Audience/resource conflation: SPIFFEClientAuthRunConfig now has separate Resources []string (RFC 8707, optional, must be an absolute HTTP(S) URI) and Audiences []string (RFC 8693, required, bounded by allowed_audiences) fields. Permitting one no longer implies the other, and a non-URI logical audience is representable.

2. No per-association grant field: added GrantTypes []string, validated to be exactly ["urn:ietf:params:oauth:grant-type:token-exchange"] for now (the only grant this surface currently supports) — client authentication no longer implicitly confers a grant; it has to be declared.

3. No bundle-source field: SPIFFETrustDomainRunConfig now has a required, discriminated BundleSource (a HTTPS SPIFFE Bundle Endpoint or the local Workload API), validated for shape only — still no fetching or loading, but the field is in the authoritative model now.

4. RunConfig no-op: RunConfig.Validate() now calls ValidateSPIFFETrust directly, and Config.SPIFFETrust is built in the embedded-auth-server constructor. A malformed or half-configured spiffe_trust_domains/inbound_grants.spiffe_client_auth now fails to start, and a valid one is actually wired into the runtime config — this PR is independently fail-closed.

CI is green on the amended commit.

[JAORMX]

Thanks for the amendment. I re-reviewed e0320a0..8abeeaa. The new head fixes several earlier blockers: spiffe_client_auth is now independent of token exchange, grant permission is explicit, a bundle-source discriminator exists, validation runs from RunConfig.Validate, and the normalized model is built at the runner boundary.

One authorization-model blocker remains:

• pkg/authserver/spiffe_trust.go:578-587 applies the global boundary backwards. RunConfig.AllowedAudiences is documented and validated as the server's RFC 8707 resource-URI allowlist. The new Resources values receive only URI syntax validation, so an association can declare a resource the server does not globally allow. Meanwhile RFC 8693 Audiences are required to be members of that URI-only resource list, which still prevents valid logical/non-URI audience identifiers. Please validate Resources as a subset of the existing global resource list and give RFC 8693 audiences an independent policy boundary. Add regression cases proving permission in either dimension does not imply permission in the other.

Two schema/lifecycle issues also need resolution before this becomes a public configuration surface:

• A SPIFFE federation bundle endpoint needs an endpoint profile (https_web or https_spiffe) in addition to its URL. Without it, the future loader cannot know whether to authenticate through Web PKI or SPIFFE, so the just-added schema already requires another compatibility change.
• The normalized Config.SPIFFETrust is still not consumed or retained by the server. Valid non-empty SPIFFE configuration now validates and starts, but provides no authentication or policy enforcement. That is fail-closed for access, but remains an operationally inert security setting. Either reject non-empty configuration until an enforcement path exists or land the first consumer with this exposed surface.

The PR description and test-plan text still describe the pre-amendment model (no RunConfig.Validate wiring, combined audiences, and no bundle source); please update them so reviewers and generated release context match the new behavior.

The amended commit has the required DCO trailer. CI is still running; completed checks are green so far.

[JAORMX]

I checked the latest amendment (108bc9c). It only adds omitempty to resources; it does not address the blockers in my latest review:

• Resources still are not bounded by the server's global RFC 8707 resource allowlist, while Audiences are still incorrectly bounded by that URI-only list.
• The bundle endpoint still lacks an https_web/https_spiffe profile.
• Valid SPIFFE configuration is still accepted without an authentication/enforcement consumer.
• The PR description still documents the pre-amendment behavior.

Leaving the change request in place. CI is still running; completed checks are green so far.

[jhrozek]

Pushed a commit that resolves the two schema findings, and updated the PR description.

1. Resources/Audiences boundary. Resources (RFC 8707) is now validated as a subset of allowed_audiences — the same list DelegateClientRunConfig.Audiences is checked against. Audiences (RFC 8693) is now independent: no longer bounded by that list, and it can hold non-URI logical identifiers. Added regression cases proving permission in one dimension doesn't imply permission in the other (audiences need not be in the resource allowlist, resource must be in global allowlist).

2. Bundle endpoint profile. Added SPIFFEBundleEndpointSourceRunConfig.Profile, required, one of https_web/https_spiffe.

3. Enforcement consumer. I went back and checked the full drafted stack, not just this PR — client registration lands two commits later (Register and harden static SPIFFE clients #6474, already open and green), but nothing anywhere in the current epic actually verifies an X.509-SVID or JWT-SVID against a bundle yet. That's a further, not-yet-scoped step. Rather than reject non-empty config here (which would have to be threaded through and then lifted across Normalize canonical inbound grants #6473/Register and harden static SPIFFE clients #6474 too, since neither adds real credential verification either), I've left the config accepting and validating, and made the PR description say plainly that a valid config has no runtime effect yet. For what it's worth, a later commit in the drafted epic (not yet opened as a PR) exists specifically to document this gap rather than resolve it, so this is a tracked, deliberate state rather than an oversight — happy to reconsider if you'd rather see it hard-rejected in this PR.

4. PR description. Rewritten to match current behavior (RunConfig.Validate wiring, resources/audiences split, bundle profile, and the enforcement-gap disclosure above).

CI is green on the amended commit.

[JAORMX]

I re-reviewed 108bc9c..e497cae. The resource/audience boundary is now correct, the bundle endpoint profile is present and validated, regression coverage was added, and the PR description is current. Thanks for fixing those.

The remaining blocker is the enforcement lifecycle. The new comment confirms that no open or drafted part of this epic verifies either SVID type against the configured bundle. Building Config.SPIFFETrust is conversion, not enforcement: a valid non-empty security configuration still starts and has no runtime effect. I do want this hard-rejected until the first authentication consumer lands. That preserves an honest, fail-loud contract while still allowing all types and validation to merge; the follow-up that introduces verification can remove the temporary rejection. Documentation alone is not enough for a setting that appears to configure client authentication.

CI has one failed network-isolation E2E unrelated to this authserver diff (--allow-docker-gateway); it needs a clean rerun before approval. The DCO trailer is present.

[jhrozek]

Pushed the hard-reject: RunConfig.Validate() now rejects any non-empty spiffe_trust_domains outright (validateSPIFFENotYetEnforced in config.go), with a clear error explaining why and what removes it. ValidateSPIFFETrust/NewSPIFFETrustConfig themselves are untouched — they remain independently callable and still validate well-formed configs; this is a policy-layer rejection in RunConfig.Validate() only.

This has real ripple: it makes it impossible to construct a full server with non-empty SPIFFE config via the normal path, which affects #6473 (fixed, unaffected) and #6474 (several integration-style tests that proved SPIFFE behavior end-to-end could no longer do so). I worked through that across both PRs — see #6474 for details, since that's where most of the affected tests live.

CI is green on this commit (network-isolation passed clean this run).

[JAORMX]

Re-review complete. The previous model blockers are addressed on this head: non-empty RunConfig SPIFFE configuration is rejected until an enforcement consumer exists; resources and audiences are separated; grant permission and bundle-source profiles are explicit; and normalized trust domains are retained.

One remaining medium-severity lifecycle bypass: Config exposes SPIFFETrust, but Config.Validate() and authserver.New accept a non-empty direct Config.SPIFFETrust without rejecting or consuming it (pkg/authserver/config.go:1028-1032, 1052-1151; pkg/authserver/server_impl.go:137-158). A direct caller can therefore start an auth server with an inert SPIFFE policy, bypassing the fail-loud protection applied to RunConfig. Please reject non-empty Config.SPIFFETrust until an authentication consumer is wired, or wire enforcement through that path too.

Current CI is green.

[jhrozek]

Pushed the fix — added validateConfigSPIFFENotYetEnforced to Config.Validate(), mirroring RunConfig.Validate()'s validateSPIFFENotYetEnforced with the same wording and rationale. Config.SPIFFETrust's zero value is nil-safe, so the check is a simple len(trust.Associations()) > 0.

This rippled one test in #6474 (TestNewServer_SPIFFEAndCIMD_WrapStorageInOrder, which constructed Config directly with non-empty SPIFFETrust and called newServer) — fixed properly rather than skipped, since decorateStorageForSPIFFE/decorateStorageForCIMD are callable directly in the same package without ever touching Config.Validate(), so the storage-decoration-order property it actually tests is still fully exercised.

CI is green.

[JAORMX]

Verified the follow-up: the Config.Validate() gate closes the direct-Config lifecycle bypass. No remaining high-confidence lifecycle finding from this review. The current CI run is green.

[JAORMX]

Re-reviewed the current head. The SPIFFE trust-model requirements and all prior review findings are addressed, including both RunConfig and direct Config fail-closed validation; no additional material regressions found. CI is green.