Skip to content

fix(mcp): Direct proxy paths use shared passthrough header utility - #3677

Merged
jonpspri merged 3 commits into
mainfrom
green-3643-direct-proxy
Mar 26, 2026
Merged

fix(mcp): Direct proxy paths use shared passthrough header utility#3677
jonpspri merged 3 commits into
mainfrom
green-3643-direct-proxy

Conversation

@crivetimihai

Copy link
Copy Markdown
Member

Summary

  • Replace manual header forwarding loops in _proxy_list_tools_to_gateway, _proxy_list_resources_to_gateway, and _proxy_read_resource_to_gateway with calls to compute_passthrough_headers_cached()
  • This ensures X-Upstream-AuthorizationAuthorization rename, global passthrough config, and header sanitization are applied consistently across all direct proxy operations in Streamable HTTP transport
  • Add 5 regression tests verifying upstream auth rename on all three proxy paths, negative case, and feature-flag-disabled behavior

Closes #3643

Test plan

  • All 355 existing test_streamablehttp_transport.py tests pass
  • All 75 passthrough header unit tests pass
  • 3 updated passthrough header tests verify gateway-specific headers via compute_passthrough_headers_cached
  • 5 new TestProxyUpstreamAuthorizationRename tests cover tools/list, resources/list, resources/read, negative case, and feature-flag-disabled scenario
  • isort and black clean

cafalchio
cafalchio previously approved these changes Mar 14, 2026

@cafalchio cafalchio left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is cleaner to use compute_passthrough_headers_cached.
Good test coverage. LGTM

@bogdanmariusc10
bogdanmariusc10 self-requested a review March 20, 2026 08:57

@bogdanmariusc10 bogdanmariusc10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Verified - Bug Fixed

Tested this PR against the reproduction steps from the bug report.

Setup:

  • mcpgateway running from green-3643-direct-proxy branch via uv run
  • Gateway configured in direct_proxy mode with STREAMABLEHTTP transport
  • Mock upstream server logging all incoming headers

Test: Sent a tools/list request with X-Upstream-Authorization: Bearer upstream-secret-token and X-Context-Forge-Gateway-Id pointing to the direct proxy gateway.

Result before this PR (on main):

[UPSTREAM] Authorization: None

Result with this PR:

[UPSTREAM] Authorization: Bearer upstream-secret-token

The X-Upstream-Authorization header is now correctly renamed to Authorization and forwarded to the upstream server on tools/list. The fix replaces the manual header loop in _proxy_list_tools_to_gateway, _proxy_list_resources_to_gateway and the resource read proxy with a call to compute_passthrough_headers_cached(), consistent with the tools/call path.

@crivetimihai crivetimihai added MUST P1: Non-negotiable, critical requirements without which the product is non-functional or unsafe and removed release-fix Critical bugfix required for the release labels Mar 20, 2026
@jonpspri
jonpspri dismissed stale reviews from bogdanmariusc10 and cafalchio via 4f951e0 March 26, 2026 20:10
@jonpspri
jonpspri force-pushed the green-3643-direct-proxy branch from 7970f7b to 4f951e0 Compare March 26, 2026 20:10
crivetimihai and others added 3 commits March 26, 2026 22:27
…irect proxy paths

Replace manual header loops in _proxy_list_tools_to_gateway,
_proxy_list_resources_to_gateway, and _proxy_read_resource_to_gateway
with calls to compute_passthrough_headers_cached so that
X-Upstream-Authorization → Authorization rename, global passthrough
config, and header sanitization are applied consistently.

Closes #3643

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
The function-scoped `app` fixture was causing ~25s setup per test due to
repeated mcpgateway.main imports. Module scope matches the existing
`app_with_temp_db` fixture and pays the import cost once per module.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
…is explicit

When a gateway has explicit passthrough_headers (including empty []),
use them directly instead of querying the global config cache. This
avoids an unnecessary DB round-trip and ensures an explicit empty list
does not fall through to the global allowlist.

Closes #3643

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
@jonpspri
jonpspri force-pushed the green-3643-direct-proxy branch from 4f951e0 to b2ea286 Compare March 26, 2026 22:28
@jonpspri
jonpspri merged commit 868bf5f into main Mar 26, 2026
27 checks passed
@jonpspri
jonpspri deleted the green-3643-direct-proxy branch March 26, 2026 22:43
brian-hussey pushed a commit that referenced this pull request Mar 27, 2026
…3677)

* fix(mcp): Use compute_passthrough_headers_cached in Streamable HTTP direct proxy paths

Replace manual header loops in _proxy_list_tools_to_gateway,
_proxy_list_resources_to_gateway, and _proxy_read_resource_to_gateway
with calls to compute_passthrough_headers_cached so that
X-Upstream-Authorization → Authorization rename, global passthrough
config, and header sanitization are applied consistently.

Closes #3643

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(test): Widen `app` fixture scope from function to module

The function-scoped `app` fixture was causing ~25s setup per test due to
repeated mcpgateway.main imports. Module scope matches the existing
`app_with_temp_db` fixture and pays the import cost once per module.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>

* fix(mcp): Skip DB lookup for passthrough headers when gateway config is explicit

When a gateway has explicit passthrough_headers (including empty []),
use them directly instead of querying the global config cache. This
avoids an unnecessary DB round-trip and ensures an explicit empty list
does not fall through to the global allowlist.

Closes #3643

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>

---------

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
Co-authored-by: Jonathan Springer <jps@s390x.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
madhu-mohan-jaishankar pushed a commit that referenced this pull request Mar 27, 2026
…3677)

* fix(mcp): Use compute_passthrough_headers_cached in Streamable HTTP direct proxy paths

Replace manual header loops in _proxy_list_tools_to_gateway,
_proxy_list_resources_to_gateway, and _proxy_read_resource_to_gateway
with calls to compute_passthrough_headers_cached so that
X-Upstream-Authorization → Authorization rename, global passthrough
config, and header sanitization are applied consistently.

Closes #3643

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(test): Widen `app` fixture scope from function to module

The function-scoped `app` fixture was causing ~25s setup per test due to
repeated mcpgateway.main imports. Module scope matches the existing
`app_with_temp_db` fixture and pays the import cost once per module.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>

* fix(mcp): Skip DB lookup for passthrough headers when gateway config is explicit

When a gateway has explicit passthrough_headers (including empty []),
use them directly instead of querying the global config cache. This
avoids an unnecessary DB round-trip and ensures an explicit empty list
does not fall through to the global allowlist.

Closes #3643

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>

---------

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
Co-authored-by: Jonathan Springer <jps@s390x.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
msureshkumar88 pushed a commit that referenced this pull request May 13, 2026
…3677)

* fix(mcp): Use compute_passthrough_headers_cached in Streamable HTTP direct proxy paths

Replace manual header loops in _proxy_list_tools_to_gateway,
_proxy_list_resources_to_gateway, and _proxy_read_resource_to_gateway
with calls to compute_passthrough_headers_cached so that
X-Upstream-Authorization → Authorization rename, global passthrough
config, and header sanitization are applied consistently.

Closes #3643

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(test): Widen `app` fixture scope from function to module

The function-scoped `app` fixture was causing ~25s setup per test due to
repeated mcpgateway.main imports. Module scope matches the existing
`app_with_temp_db` fixture and pays the import cost once per module.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>

* fix(mcp): Skip DB lookup for passthrough headers when gateway config is explicit

When a gateway has explicit passthrough_headers (including empty []),
use them directly instead of querying the global config cache. This
avoids an unnecessary DB round-trip and ensures an explicit empty list
does not fall through to the global allowlist.

Closes #3643

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>

---------

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
Co-authored-by: Jonathan Springer <jps@s390x.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
kamath-a pushed a commit to kamath-a/mcp-context-forge that referenced this pull request Jun 26, 2026
…BM#3677)

* fix(mcp): Use compute_passthrough_headers_cached in Streamable HTTP direct proxy paths

Replace manual header loops in _proxy_list_tools_to_gateway,
_proxy_list_resources_to_gateway, and _proxy_read_resource_to_gateway
with calls to compute_passthrough_headers_cached so that
X-Upstream-Authorization → Authorization rename, global passthrough
config, and header sanitization are applied consistently.

Closes IBM#3643

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(test): Widen `app` fixture scope from function to module

The function-scoped `app` fixture was causing ~25s setup per test due to
repeated mcpgateway.main imports. Module scope matches the existing
`app_with_temp_db` fixture and pays the import cost once per module.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>

* fix(mcp): Skip DB lookup for passthrough headers when gateway config is explicit

When a gateway has explicit passthrough_headers (including empty []),
use them directly instead of querying the global config cache. This
avoids an unnecessary DB round-trip and ensures an explicit empty list
does not fall through to the global allowlist.

Closes IBM#3643

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>

---------

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
Co-authored-by: Jonathan Springer <jps@s390x.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-queue Rebased and ready to merge MUST P1: Non-negotiable, critical requirements without which the product is non-functional or unsafe

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Direct proxy paths in Streamable HTTP skip X-Upstream-Authorization rename

4 participants