fix(broker): stop forwarding Cookie/Proxy-Authorization to user-specific upstreams

[PRAteek-singHWY]

What

When the broker builds a federated tool list, it forwards the client's request
headers to each userSpecificList upstream so the upstream can return a
per-user tool list. filterUserHeaders stripped only mcp-session-id and
x-mcp-*, so it also forwarded Cookie and Proxy-Authorization.

This adds those two headers to the strip set, so they are never sent to
upstreams. The client's Authorization header is intentionally still forwarded
— user-specific servers rely on it for per-user identity.

A case is added to the existing TestFilterUserHeaders table covering the new
behaviour.

Why

Cookie and Proxy-Authorization are scoped to the gateway origin/hop, not the
upstream. FetchUserSpecificTools computes the forwarded headers once and fans
them out to every user-specific upstream concurrently, over direct
broker→upstream connections that bypass Envoy. As a result a client's
gateway-/origin-scoped session cookie was disclosed to every user-specific
upstream queried for a single tools/list; a malicious or compromised upstream
could replay it against the gateway origin. Proxy-Authorization is a
hop-by-hop credential and must not be proxied either.

Forwarding Authorization to upstreams is documented and intended
(docs/design/security-architecture.md), so it is left unchanged.

Changes

• internal/broker/user_specific_tools.go: add sensitiveForwardHeaders
  (cookie, proxy-authorization) and skip them in filterUserHeaders;
  clarify in the doc comment that Authorization is preserved by design.
• internal/broker/user_specific_tools_test.go: add a TestFilterUserHeaders
  case asserting Cookie/Proxy-Authorization are stripped and Authorization
  is preserved.

Testing

• go test ./internal/broker/... passes.
• gofmt/goimports clean on the changed files.

Notes

Security fix, exempt from the issue-first requirement per CONTRIBUTING.md.
Tracking issue: #1170. Commit is signed off (git commit -s).

Out of scope: whether Authorization should reach multiple distinct upstreams
on the user-specific fan-out, and how RFC 8693 token exchange applies on a path
that bypasses Envoy/Authorino — tracked separately for design discussion.

Fixes #1170

Summary by CodeRabbit

• Bug Fixes
  • Enhanced security by preventing sensitive gateway credentials from being forwarded to upstream servers during user-specific tool operations. The system now filters out cookies and proxy authorization headers to mitigate credential leakage risks.

[github-actions]

Thanks for the contribution, @PRAteek-singHWY! You currently have other non-draft PR(s) open:

• fix(broker-router): honor --log-level=4 (warn) instead of defaulting to debug #1141

To help us review and merge changes as efficiently as possible, we ask contributors to focus on one PR at a time. Activity on this project can be high, and maintainers have other priorities outside the project, so having a single active PR helps everyone get changes landed faster.

This PR has been moved to draft. Once your other PR(s) are merged or closed, mark this one as ready for review and we will take a look.

Note

This is an experimental process and may change or need manual intervention while we trial it.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0f7cc18e-60aa-476f-ba92-bca0ffc476fe

📥 Commits

Reviewing files that changed from the base of the PR and between a9c48ab and acb7690.

📒 Files selected for processing (2)

• internal/broker/user_specific_tools.go
• internal/broker/user_specific_tools_test.go

---

📝 Walkthrough

Walkthrough

filterUserHeaders in user_specific_tools.go gains a sensitiveForwardHeaders set (cookie, proxy-authorization) and a check to drop matching headers before forwarding to user-specific MCP upstreams. A new TestFilterUserHeaders case validates the strip while confirming Authorization passes through.

Sensitive header filter

Layer / File(s)	Summary
sensitiveForwardHeaders set, filter logic, and test internal/broker/user_specific_tools.go, internal/broker/user_specific_tools_test.go	Defines the sensitiveForwardHeaders set (cookie, proxy-authorization), adds a loop check in filterUserHeaders to skip those keys, and extends TestFilterUserHeaders to assert Cookie/Proxy-Authorization are stripped while Authorization is retained.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Suggested labels

review-effort/medium, high-risk

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main security fix: preventing Cookie and Proxy-Authorization header forwarding to user-specific upstreams.
Linked Issues check	✅ Passed	The PR fully implements the fix required by issue #1170: Cookie and Proxy-Authorization headers are stripped via sensitiveForwardHeaders set while Authorization is preserved.
Out of Scope Changes check	✅ Passed	All changes directly address the security vulnerability in #1170; no out-of-scope modifications are present beyond the header filtering fix and corresponding test coverage.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[PRAteek-singHWY]

Hello @jasonmadigan @david-martin , could you please have a look whenever you get a chance?

This change is small and self-contained: it stops the broker forwarding Cookie and Proxy-Authorization to user-specific upstreams on the tools/list fan-out (Fixes #1170), with a unit test added to TestFilterUserHeaders. CodeRabbit had no actionable comments and all checks are green.

Whenever you get a chance to review, I'd really appreciate it. Once this and #1141 are reviewed I'll pick up the next change and keep exploring other areas of the gateway.

Looking forward to your guidance and happy to iterate on anything.

[david-martin]

These two headers are extremely unlikely to be intended for upstream MCP servers. Cookie in particular could contain a value scoped to the gateway or something in front of it (which ideally gets dropped after verification, but that is not guaranteed). Safe change.

If this list needs to grow in future, users have options at the gateway layer: AuthPolicy or HTTPRoute header filters can strip headers before they reach the broker, so this does not need to become an unbounded blocklist.

[david-martin]

Thanks @PRAteek-singHWY

[PRAteek-singHWY]

These two headers are extremely unlikely to be intended for upstream MCP servers. Cookie in particular could contain a value scoped to the gateway or something in front of it (which ideally gets dropped after verification, but that is not guaranteed). Safe change.

If this list needs to grow in future, users have options at the gateway layer: AuthPolicy or HTTPRoute header filters can strip headers before they reach the broker, so this does not need to become an unbounded blocklist.

Hi @david-martin Sir

Noted on the blocklist point. For anything beyond these clearly gateway-scoped headers I'll lean on the AuthPolicy / HTTPRoute filter approach rather than expanding the strip set. Good learning, thanks for your guidance and reviewing it with clarity.

Looking forward to finding more interesting ways I can help.

Thank you