Skip to content

Require a trust mode for private_key_jwt delegates - #7004

Merged
jhrozek merged 1 commit into
mainfrom
stable-client-v2-f-1
Oct 30, 2026
Merged

jhrozek merged 1 commit into
mainfrom
stable-client-v2-f-1

Conversation

@jhrozek

@jhrozek jhrozek commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Summary

A delegate client can exchange a user's self-issued ToolHive token for a delegated token asserting that user's sub. Its token_exchange.self_issued_subject_tokens mode decides which subject tokens it accepts: any takes every self-issued token for any user, whichever client obtained it; restricted takes only tokens issued to the client itself or to its allowed_source_clients. A client that omits the block gets any, and the only signal is a startup WARN.

Only clients that set grant_types had to declare the block. A private_key_jwt delegate (added in #6952, released in v0.59.0) without grant_types still inherited blanket trust silently, for example when a templating step dropped the block. private_key_jwt is part of the newer client metadata, so no pre-existing config depends on that fallback.

  • requireDeclaredTrustMode now requires the token_exchange block for private_key_jwt delegates too, on both the RunConfig and the resolved Config validation paths. The implicit legacy rule stays only for secret-authenticated clients without grant_types.
  • The startup WARN marked a client explicit_any=true only when it set grant_types. It now also marks private_key_jwt clients, which must declare their mode as well, instead of reporting them like clients that inherited the legacy rule. Both checks share a new trustModeRequired helper so they cannot drift apart. redeems_id_jag is now set independently of explicit_any, still from grant_types and the JWT-bearer grant alone.
  • docs/arch/17-token-exchange-delegation.md, the DelegateClientRunConfig.TokenExchange doc comment and the generated swagger/SDK schemas describe the new rule.
Delegate client Before (v0.59.0) After
secret, no grant_types, no block implicit any + WARN implicit any + WARN
secret, grant_types, no block startup error startup error
private_key_jwt, grant_types, no block startup error startup error
private_key_jwt, no grant_types, no block implicit any + WARN startup error
any client, block declared declared mode declared mode

Part of #6893

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)

Unit tests were scoped to pkg/authserver, the only package with logic changes, using the Taskfile's go test -race flags. New cases cover a private_key_jwt client without the block (rejected on both validation paths) and a private_key_jwt client with explicit any in TestWarnBlanketSelfIssuedExchange (explicit_any=true, plus redeems_id_jag=true when JWT-bearer is enabled). task lint-fix reports 0 issues.

Does this introduce a user-facing change?

Yes. Before: v0.59.0 accepted a private_key_jwt delegate client with neither grant_types nor a token_exchange block and gave it blanket any trust. After: the auth server refuses to start with delegate_clients: client_id "<id>": private_key_jwt clients must declare a token_exchange block with self_issued_subject_tokens ("any" or "restricted"). To keep the old behaviour, add token_exchange: {self_issued_subject_tokens: any}; to narrow it, use restricted with allowed_source_clients.

Also: the startup WARN for a private_key_jwt delegate in any mode now carries explicit_any=true.

Special notes for reviewers

  • This deliberately breaks a config shape that shipped in v0.59.0 two days ago. Failing at startup with an error that names the missing block was preferred over keeping a silent fail-open default for a client type that has no legacy users. Worth a line in the release notes.
  • The operator CRD does not expose private_key_jwt delegates yet, so there is no CEL rule to update here. When the CRD gains tokenEndpointAuthMethod, its CEL rule (grantTypes requires tokenExchange) must also require tokenExchange for private_key_jwt, so the CRD cannot produce a config the server refuses.
  • One full-package run failed in TestIntegration_JWTBearerGrant_NoJTISyntheticKeyDoesNotCollideWithJTI (400 invalid_grant, "assertion invalid or could not be verified"). That test configures no delegate clients and does not reach the changed code; it passed 3/3 in isolation and the next full-package run passed. It looks like a pre-existing flake under -race load.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label Oct 30, 2026
@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.38%. Comparing base (00feee7) to head (4e8a286).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7004   +/-   ##
=======================================
  Coverage   80.37%   80.38%           
=======================================
  Files         849      849           
  Lines       86715    86720    +5     
=======================================
+ Hits        69701    69706    +5     
  Misses      17009    17009           
  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.

ChrisJBurns
ChrisJBurns previously approved these changes Oct 30, 2026
A delegate client may exchange a user's self-issued ToolHive token for
a delegated token asserting that user's sub. Its token_exchange block
picks which subject tokens it accepts:

  any         every self-issued token, for any user, whichever client
              originally obtained it
  restricted  only tokens issued to the client itself or to one of
              its allowed_source_clients

If the delegate is compromised, its private key is stolen, or a bug
hands it tokens meant for other clients, "any" lets it turn every
token it can get into a delegated token for its own audiences. Scope
and audience narrowing and the lifetime cap still apply in both modes;
there is no single-use enforcement in either. The only signal for
blanket trust is a startup WARN.

A client that omits the block gets "any". Only clients that set
grant_types were required to declare the block, so a private_key_jwt
client without grant_types still inherited blanket trust silently, for
example when a templating step dropped the block.

Require the block for private_key_jwt clients too. The implicit legacy
rule now applies only to secret-authenticated clients without
grant_types, the shape that predates the newer client metadata:

  client                                 before            after
  secret, no grant_types, no block       implicit any      implicit any
  secret, grant_types, no block          startup error     startup error
  private_key_jwt, grant_types, no block startup error     startup error
  private_key_jwt, no grant_types,
    no block                             implicit any      startup error
  any client, block declared             declared mode     declared mode

The startup WARN marked a client explicit_any=true only when it set
grant_types, on the reasoning that such a client had to declare its
mode. A private_key_jwt client now also has to, so the WARN marks it
explicit_any=true as well, instead of reporting it like a client that
inherited the legacy rule. Both checks share trustModeRequired so they
cannot drift apart. redeems_id_jag still depends only on grant_types
and the JWT-bearer grant.

A private_key_jwt delegate with neither grant_types nor a
token_exchange block, accepted since v0.59.0, now fails validation at
startup. Add token_exchange.self_issued_subject_tokens: any to keep the
old behaviour, or restricted to narrow it.

Co-authored-by: Mecatl <noreply@mecatl.dev>
@jhrozek
jhrozek dismissed stale reviews from ChrisJBurns via 4e8a286 October 30, 2026 15:58
@jhrozek
jhrozek force-pushed the stable-client-v2-f-1 branch from 48f8fd6 to 4e8a286 Compare October 30, 2026 15:58
@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Oct 30, 2026
@jhrozek
jhrozek merged commit 20a3b96 into main Oct 30, 2026
54 of 70 checks passed
@jhrozek
jhrozek deleted the stable-client-v2-f-1 branch October 30, 2026 17:08
@github-actions github-actions Bot mentioned this pull request Nov 2, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants