feat(ui): UI does not have ability to forward headers that are not authorization#1776
Open
jholt96 wants to merge 2 commits intokagent-dev:mainfrom
Open
feat(ui): UI does not have ability to forward headers that are not authorization#1776jholt96 wants to merge 2 commits intokagent-dev:mainfrom
jholt96 wants to merge 2 commits intokagent-dev:mainfrom
Conversation
jholt96
requested review from
EItanya,
ilackarms,
iplay88keys,
jmhbh,
peterj,
supreme-gg-gg and
yuval-k
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a configurable allowlist for forwarding additional request headers from the UI (behind oauth2-proxy) to the backend, enabling propagation of identity/metadata headers beyond Authorization across the UI → controller/agent → MCP chain.
Changes:
- Add
KAGENT_ADDITIONAL_FORWARDED_HEADERSenv parsing + blocklist enforcement to forward extra headers alongsideauthorization. - Update UI backend proxying calls to spread forwarded headers first so explicit
Content-Type/Acceptvalues win. - Expand A2A/A2A-sandbox CORS
Access-Control-Allow-Headersto include the configured forwarded header names; add Helm values + deployment env wiring.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/lib/auth.ts | Implements additional-forwarded-header allowlist with a blocked header set and exposes allowed header names. |
| ui/src/app/actions/utils.ts | Adjusts header merge order so explicit content negotiation headers override forwarded ones. |
| ui/src/app/a2a/[namespace]/[agentName]/route.ts | Forwards additional headers to backend and derives CORS allow-headers from the allowed-forward list. |
| ui/src/app/a2a-sandboxes/[namespace]/[agentName]/route.ts | Same as above for sandbox A2A route. |
| helm/kagent/values.yaml | Adds ui.additionalForwardedHeaders values knob and docs. |
| helm/kagent/templates/ui-deployment.yaml | Wires ui.additionalForwardedHeaders into KAGENT_ADDITIONAL_FORWARDED_HEADERS env var. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+7
to
+19
| const BLOCKED_FORWARD_HEADERS = new Set([ | ||
| "host", | ||
| "connection", | ||
| "keep-alive", | ||
| "transfer-encoding", | ||
| "upgrade", | ||
| "te", | ||
| "trailer", | ||
| "proxy-authenticate", | ||
| "proxy-authorization", | ||
| "content-length", | ||
| "content-encoding", | ||
| ]); |
Contributor
Author
There was a problem hiding this comment.
personally i think this is not needed as at least for the oauth2-proxy cookie that should be encrypted. lmk what you all think and I can add it
Comment on lines
+27
to
+63
| function parseAllowedHeaders(): Set<string> { | ||
| const allowed = new Set<string>(DEFAULT_FORWARD_HEADERS); | ||
| const raw = process.env.KAGENT_ADDITIONAL_FORWARDED_HEADERS; | ||
| if (!raw) { | ||
| return allowed; | ||
| } | ||
|
|
||
| const blockedFromEnv: string[] = []; | ||
| for (const part of raw.split(",")) { | ||
| const name = part.trim().toLowerCase(); | ||
| if (!name) continue; | ||
| if (BLOCKED_FORWARD_HEADERS.has(name)) { | ||
| blockedFromEnv.push(name); | ||
| continue; | ||
| } | ||
| allowed.add(name); | ||
| } | ||
|
|
||
| if (blockedFromEnv.length > 0 && !warnedBlocked) { | ||
| warnedBlocked = true; | ||
| console.warn( | ||
| `KAGENT_ADDITIONAL_FORWARDED_HEADERS contains hop-by-hop or routing headers that will not be forwarded: ${blockedFromEnv.join(", ")}` | ||
| ); | ||
| } | ||
|
|
||
| return allowed; | ||
| } | ||
|
|
||
| const ALLOWED_FORWARD_HEADERS = parseAllowedHeaders(); | ||
|
|
||
| /** | ||
| * Extract authentication headers from a headers-like object. | ||
| * Common implementation used by both server actions and route handlers. | ||
| * Returns the list of header names (lowercased) the proxy will forward. | ||
| * Always includes "authorization"; extends with KAGENT_ADDITIONAL_FORWARDED_HEADERS. | ||
| */ | ||
| function extractAuthHeaders(getHeader: (name: string) => string | null): Record<string, string> { | ||
| const authHeaders: Record<string, string> = {}; | ||
| export function getAllowedForwardHeaderNames(): string[] { | ||
| return Array.from(ALLOWED_FORWARD_HEADERS); | ||
| } |
Comment on lines
+5
to
+11
| function buildCorsAllowHeaders(): string { | ||
| const names = new Set<string>(['content-type', 'accept']); | ||
| for (const name of getAllowedForwardHeaderNames()) { | ||
| names.add(name); | ||
| } | ||
| return Array.from(names).join(', '); | ||
| } |
Comment on lines
+5
to
+11
| function buildCorsAllowHeaders(): string { | ||
| const names = new Set<string>(['content-type', 'accept']); | ||
| for (const name of getAllowedForwardHeaderNames()) { | ||
| names.add(name); | ||
| } | ||
| return Array.from(names).join(', '); | ||
| } |
| env: {} # Additional configuration key-value pairs for the ui ConfigMap | ||
| # -- Additional request headers (beyond Authorization) the UI proxy will forward | ||
| # to the backend. Names are case-insensitive. Hop-by-hop headers (Connection, | ||
| # Transfer-Encoding, etc.) are silently dropped. |
EItanya
reviewed
Apr 29, 2026
Contributor
EItanya
left a comment
EItanya
left a comment
There was a problem hiding this comment.
Overall this PR makes a ton of sense to me, I do think the copilot comments are worth addressing first.
peterj
reviewed
May 1, 2026
| import { getAuthHeadersFromRequest } from '@/lib/auth'; | ||
| import { getAuthHeadersFromRequest, getAllowedForwardHeaderNames } from '@/lib/auth'; | ||
|
|
||
| function buildCorsAllowHeaders(): string { |
Collaborator
There was a problem hiding this comment.
this looks the same as buildCorsAllowHeaders
Contributor
Author
There was a problem hiding this comment.
Not sure what you mean by this but can you re review? I am fairly rusty with Typescript so want to make sure this looks good to everyone
…thorization Add an allowlist env for forwarding additional headers to the backend from oauth2-proxy Signed-off-by: Josh Holt <jholt96@live.com>
Contributor
Author
|
implemented copilot suggested changes and added tests for this |
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.
Add an allowlist env for forwarding additional headers to the backend from oauth2-proxy. This allowed for setting additional headers from oauth2-proxy or other services such as X-Auth-Request-User, X-Auth-Request-Email, X-Auth-Request-Groups, X-Forwarded-User, X-Forwarded-Groups.
We are working on an use case where we set a custom header -> goes through oauth2-proxy -> UI -> controller -> agent -> MCP server. The MCP server uses that header to validate the user before running any tool call as an additional layer of rbac