feat(adk): forward displaced static token as actor for OBO delegation#2087
Open
QuentinBisson wants to merge 4 commits into
Open
feat(adk): forward displaced static token as actor for OBO delegation#2087QuentinBisson wants to merge 4 commits into
QuentinBisson wants to merge 4 commits into
Conversation
Add KAGENT_PROPAGATE_TOKEN_OVERRIDES_STATIC. When set, a forwarded or STS-exchanged Authorization takes precedence over a static Authorization configured on an MCP server, and the displaced static (M2M) token is sent as X-Actor-Token. A downstream gateway can then run an RFC 8693 delegation with subject=end user and actor=agent, instead of the static M2M identity winning on every tool call. With no forwarded token the static Authorization is left in place and no actor token is added, so autonomous runs stay pure M2M. Default behaviour is unchanged: static headers keep the highest precedence. Signed-off-by: QuentinBisson <quentin@giantswarm.io>
…hrough the registry Addresses review of the static-token-as-actor change. - Replace the OverrideStaticWithForwardedToken bool threaded through CreateToolsets, mcpServerParams and headerRoundTripper with a TokenPrecedence type (StaticTokenWins | ForwardedTokenWins), removing the opaque second positional bool from CreateToolsets. - Scope the forwarded-wins override to the Authorization header only. Other static headers now keep the highest precedence in both modes; previously enabling the flag silently demoted every static header to a default, so a forwarded header could override a static one of the same name. - Collapse the duplicated static-header application in RoundTrip into a single applyStaticHeaders pass. - Register KAGENT_PROPAGATE_TOKEN and KAGENT_PROPAGATE_TOKEN_OVERRIDES_STATIC as bools and read them through env.*.Get() in agent.go and adapter.go, replacing three divergent raw os.Getenv parses that bypassed the registry. - Add tests: non-Authorization static header still wins under ForwardedTokenWins, forwarded token equal to static adds no actor, pre-existing X-Actor-Token is preserved. Signed-off-by: QuentinBisson <quentin@giantswarm.io>
deea15d to
6433b97
Compare
- collapse the triplicated displacement narrative to a single home in applyStaticHeaders; struct and CreateToolsets docs point to it - document tokenPrecedence as a runtime-global policy, not per-server - warn when ForwardedTokenWins is active on a TLS-insecure MCP server, since the actor token carries a privileged M2M credential - route STS_WELL_KNOWN_URI through the env registry (env.StsWellKnownURI), matching the other propagate-token vars - note the single-Authorization-key assumption in applyStaticHeaders - cover ForwardedTokenWins with no static Authorization configured Signed-off-by: QuentinBisson <quentin@giantswarm.io>
4d3e656 to
cd2a52d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in token precedence policy for MCP requests so that forwarded/STS Authorization tokens can override a server’s static Authorization, while still preserving the displaced static credential as an X-Actor-Token to support downstream RFC 8693-style delegation (subject=user, actor=agent).
Changes:
- Adds
KAGENT_PROPAGATE_TOKEN_OVERRIDES_STATICand threads aTokenPrecedencepolicy through MCP toolset creation. - Updates MCP request header application so forwarded/STS
Authorizationcan win (opt-in) and the displaced static token becomesX-Actor-Token. - Moves
KAGENT_PROPAGATE_TOKEN/STS_WELL_KNOWN_URIreads to the env registry and adds comprehensive precedence/actor-header tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| go/core/pkg/env/kagent.go | Changes KAGENT_PROPAGATE_TOKEN to a registered bool var and adds KAGENT_PROPAGATE_TOKEN_OVERRIDES_STATIC. |
| go/adk/pkg/runner/adapter.go | Switches env reads for propagation/STS wiring to the env registry. |
| go/adk/pkg/mcp/registry.go | Introduces TokenPrecedence, implements Authorization override + X-Actor-Token displacement, and adds TLS warning when verification is disabled. |
| go/adk/pkg/mcp/registry_test.go | Adds tests covering precedence/actor-token displacement scenarios and edge cases. |
| go/adk/pkg/constants/const.go | Adds ActorTokenHeader constant (X-Actor-Token). |
| go/adk/pkg/agent/agent.go | Threads TokenPrecedence from env into MCP toolset creation and continues using propagation for remote A2A tooling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+350
to
359
| // headers is assumed to hold at most one Authorization key; with case-variant | ||
| // duplicates map iteration order decides which wins. | ||
| staticAuthorization := "" | ||
| for key, value := range rt.headers { | ||
| if strings.EqualFold(key, constants.AuthorizationHeader) { | ||
| staticAuthorization = value | ||
| continue | ||
| } | ||
| req.Header.Set(key, value) | ||
| } |
Contributor
Author
There was a problem hiding this comment.
headers are sourced from YAML/JSON config; duplicate case-variant Authorization keys cannot arise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Static headers on an MCP server (
RemoteMCPServer.headersFrom) are applied at the highest precedence inheaderRoundTripper, so a static M2MAuthorizationoverrides anAuthorizationforwarded viaKAGENT_PROPAGATE_TOKENor minted by the STS plugin. This breaks the common gateway topology where:Because the static M2M token wins, the end-user token never reaches the gateway. This reworks the precedence (originally proposed in #2044) and adds the missing piece for true on-behalf-of: the actor token.
Change
Add
KAGENT_PROPAGATE_TOKEN_OVERRIDES_STATIC(defaultfalse). When set:Authorizationwins over a staticAuthorizationon the MCP server;X-Actor-Token, so a downstream gateway can perform an RFC 8693 delegation withsubject= end user andactor= agent, rather than the static M2M identity winning on every call;Authorizationonly. Every other static header keeps the highest precedence in both modes, so enabling the flag never silently demotes an unrelated static header;Authorizationis left in place and no actor token is added, so autonomous runs stay pure M2M. A forwarded token equal to the static one is treated as M2M (no actor). AnX-Actor-Tokenalready forwarded viaallowedHeadersis left untouched.X-Actor-Tokenis a contract with the downstream gateway (e.g. an MCP aggregator that supports RFC 8693 delegation from a subject + actor token), not a standard header; the token exchange itself stays in the gateway. This is relevant now that the controller-based OBO exchange is deprecated.Default behaviour is unchanged; existing precedence tests stay green.
Modelling and consistency
TokenPrecedencetype (StaticTokenWins/ForwardedTokenWins) threaded throughCreateToolsetsrather than an opaque positionalbool. The zero value isStaticTokenWins, so the safe default falls out of the type.RemoteMCPServerfield and is left as a follow-up; the doc comment now says so rather than implying per-server configurability.RoundTripis a single pass (applyStaticHeaders) for both modes, and the displacement semantics are documented in one place instead of being duplicated across the struct,CreateToolsets, and the method.KAGENT_PROPAGATE_TOKEN,KAGENT_PROPAGATE_TOKEN_OVERRIDES_STATIC, andSTS_WELL_KNOWN_URIare read through the env registry (env.*.Get()) inagent.goandadapter.goinstead of rawos.Getenv. The remaining provider-specific vars inagent.go(GOOGLE_API_KEY,OLLAMA_API_BASE, ...) are unchanged and out of scope here.Security
X-Actor-Tokencarries a privileged M2M credential alongside the userAuthorization. It must only be sent to a trusted gateway over verified TLS; do not enable this flag against an endpoint withtlsInsecureSkipVerifyset. WhenForwardedTokenWinsis active on a server with TLS verification disabled, the runtime now logs a warning at transport creation, since the actor token can otherwise leak to an unverified endpoint.Tests
TestOverrideStatic_PropagatedTokenWinsAsSubjectTestOverrideStatic_DisplacedStaticBecomesActorTestOverrideStatic_NoForwardedToken_StaticStaysNoActorTestOverrideStatic_NonAuthStaticHeaderWinsTestOverrideStatic_ForwardedEqualsStatic_NoActorTestOverrideStatic_PreexistingActorTokenPreservedTestOverrideStatic_NoStaticAuthorization_ForwardedPassesThroughheaderRoundTripperprecedence tests unchanged.Notes
Go runtime only. Supersedes the closed #2044, which flipped precedence but did not carry the actor token. Related: #2071 (M2M minting from the agent's own ServiceAccount).