Skip to content

fix(passthrough): stop coercing an unset gateway allowlist to [] in the tool cache payload - #6964

Open
Meekohi wants to merge 1 commit into
IBM:mainfrom
Meekohi:fix/passthrough-null-gateway-allowlist
Open

Meekohi wants to merge 1 commit into
IBM:mainfrom
Meekohi:fix/passthrough-null-gateway-allowlist

Conversation

@Meekohi

@Meekohi Meekohi commented Sep 22, 2026 •

Copy link
Copy Markdown

Summary

MCP gateway tools drop globally allowed headers when their gateway has an unset header allowlist. Preserve inheritance while retaining explicit overrides.

Reproduction Steps

Enable header passthrough and configure a global allowlist. Register an MCP gateway without a gateway allowlist. Invoke its tool with an allowed header; the upstream does not receive it.

Closes #6963.

Root Cause

ToolService._build_tool_cache_payload() converts gateway.passthrough_headers=None to []. The forwarding policy interprets any list as a gateway override.

Fix Description

Preserve None in cached payloads. Unset allowlists inherit global settings; explicit empty lists block forwarding. A2A behavior remains outside this change.

Verification

Regression coverage includes real MCP calls, repeated tool lookups, disabled forwarding, unauthenticated requests, and insufficient token permissions.
A2A allowlist behavior remains outside this change.

Validation: focused regressions, six black-box cases, static checks, secret scanning, package verification, and production image construction pass.
Production MCP/RBAC E2E has 129 passing cases after correcting local fixture connectivity and installing Chromium; 14 feature-specific cases skip.
The full suite has 24,519 passes. Seven upstream setup errors resolve against the correct isolated upstream.
Doctests have 1,241 passes; overall coverage is 98%.

Validation exceptions: three A2A JWT-fixture failures reproduce on main and remain outside this PR.
Full Bandit reports 12 unchanged findings outside the diff.
Coverage also exposes a validator-cache test-order failure reproduced on main and a timing failure that passes independently.
These existing failures require exceptions to the full repository gate.

@Meekohi

Meekohi commented Sep 23, 2026

Copy link
Copy Markdown
Author

Hi @a-effort — same two process asks as on #6772, if you have a moment:

  • CI hasn't run. All 14 workflows are at "action required" pending approval. Could someone approve the runs so reviewers can see the checks?
  • [BUG]: Global passthrough headers are never forwarded to MCP gateway tools when the gateway has no per-gateway allowlist #6963 hasn't been through triage yet (no labels). Flagging it as a bug rather than a feature: with ENABLE_HEADER_PASSTHROUGH=true, the global allowlist is silently ignored for every gateway that doesn't repeat it in its own passthrough_headers, which contradicts the documented hierarchy in docs/docs/overview/passthrough.md. The fix is one line; the regression tests cover unset / [] / explicit-list gateways and fail without it.

Happy to turn any review feedback around quickly. Thanks!

@AmirF194 AmirF194 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Read compute_passthrough_headers_cached in mcpgateway/utils/passthrough_headers.py to confirm the None-vs-empty-list contract this fix relies on: effective_allowed = gateway_passthrough_headers if gateway_passthrough_headers is not None else allowed_headers, so None inherits the global allowlist and [] is an explicit override that forwards nothing.

The same coercion this PR removes at tool_service.py:1351 still exists for A2A agents, at tool_service.py:6815 on this head:

a2a_allowlist = a2a_agent_passthrough_headers or []

a2a_agent_passthrough_headers is read straight from a2a_agent.passthrough_headers a few lines above (an Optional[List[str]] DB field, None by default), so an A2A agent with no explicit allowlist gets a2a_allowlist = [], which is then passed as gateway_passthrough_headers=a2a_allowlist into compute_passthrough_headers_cached. That reads as an explicit empty override, so the global passthrough headers never reach an A2A tool call unless the agent has its own non-empty list configured, the same bug this PR's title and tests describe for MCP gateway tools, just at a different call site.

Fix would be the same shape as this PR's own change: a2a_allowlist = a2a_agent_passthrough_headers (keep None distinct from []), since compute_passthrough_headers_cached already handles None correctly.

I did not run this, it is a direct read of the same function this PR's own tests already exercise, not a new execution claim.

@Meekohi

Meekohi commented Oct 4, 2026

Copy link
Copy Markdown
Author

The A2A coercion at tool_service.py:6815 is intentional, though. A2A agents use a different contract from MCP gateways: their passthrough_headers is an explicit per-agent allowlist, and null and [] both block forwarding with no fallback to the global list. That's documented in docs/docs/overview/passthrough.md (lines 209, 221, 348) and docs/docs/using/agents/a2a.md:60. It also matches the direct A2A path (A2AAgentService._prepare_header_flows / _refilter_plugin_headers, which return {} when the list is falsy) and is pinned by test_a2a_pre_invoke_blocks_headers_when_agent_allowlist_unset in test_tool_service_coverage.py. Changing it to keep None would start forwarding the global allowlist, which can include sensitive headers when ENABLE_SENSITIVE_HEADER_PASSTHROUGH=true, to every A2A agent without its own list. So this PR deliberately scopes the fix to the gateway path, where the docs promise the gateway → global → env fallback. Happy to add a short comment at the A2A site noting the difference if that would help future readers.

@Meekohi
Meekohi force-pushed the fix/passthrough-null-gateway-allowlist branch from fd4ecc9 to a974543 Compare October 4, 2026 14:40
@Meekohi

Meekohi commented Oct 4, 2026

Copy link
Copy Markdown
Author

Rebased onto current main (9bb9c2a); the PR is mergeable again. Changes from the rebase:

The source fix is unchanged. The fail-before/pass-after check was re-run on the rebased tree, and the affected suites pass (1056 passed, 1 skipped). CI workflows still need maintainer approval to run.

@Meekohi

Meekohi commented Oct 4, 2026

Copy link
Copy Markdown
Author

@a-effort could you approve the workflows and triage if you have time? Thanks!

@Alex-vonAllmen

Copy link
Copy Markdown

This fix matches the root cause we reproduced in our ContextForge deployment.

We operate an MCP gateway in front of a per-user SugarCRM connector. With a global passthrough allowlist and an unset gateway-level allowlist, no headers reached the upstream connector. Adding the same list directly to the gateway made forwarding work, which confirms that the cache payload was converting an unset value into an explicit empty override.

The proposed change preserves the required contract:

  • None inherits the global allowlist
  • [] remains an explicit “forward nothing” override
  • a non-empty gateway list remains an explicit gateway-specific override

This is compatible with our SugarCRM connector and removes the need to duplicate the identity-header allowlist onto every gateway.

We will apply this commit to our ContextForge fork and run the affected regression tests plus the end-to-end header-forwarding test. One related review observation appears out of scope for this PR: the A2A path may contain a similar None/[] coercion. We suggest tracking that separately unless the maintainers want it included here.

From our side, the MCP gateway fix looks correctly scoped and addresses the production behavior described in #6963.

Preserve null gateway allowlists in cached tool payloads.
Unset allowlists inherit the global configuration; explicit empty lists block forwarding.

Cover inherited, empty, and explicit allowlists through real MCP calls.
Verify disabled forwarding, unauthenticated requests, and insufficient token permissions.

Closes IBM#6963

Signed-off-by: Michael Holroyd <meekohi@gmail.com>
Signed-off-by: Michael Holroyd <michael.holroyd@ga-ccri.com>

{"fundingSource": "RD1191", "team": "CTO", "fshGit": "d3463a9a", "fshDocker": "sha256:62e82fe2"}
@Meekohi
Meekohi force-pushed the fix/passthrough-null-gateway-allowlist branch from a974543 to aaaf3fc Compare October 22, 2026 22:02
@Meekohi

Meekohi commented Oct 22, 2026

Copy link
Copy Markdown
Author

Updated this PR to sync with the latest main branch.

@a-effort

Copy link
Copy Markdown
Collaborator

@a-effort could you approve the workflows and triage if you have time? Thanks!

Apologies for the delay; just triggered the workflow run. @jonpspri for triage

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Global passthrough headers are never forwarded to MCP gateway tools when the gateway has no per-gateway allowlist

4 participants