Skip to content

feat(security): expand inbound passthrough denylist to block protocol-level headers#4726

Open
bogdanmariusc10 wants to merge 2 commits into
mainfrom
4450-security-expand-inbound-passthrough-denylist-beyond-content-type-to-cover-host-and-other-protocol-level-headers
Open

feat(security): expand inbound passthrough denylist to block protocol-level headers#4726
bogdanmariusc10 wants to merge 2 commits into
mainfrom
4450-security-expand-inbound-passthrough-denylist-beyond-content-type-to-cover-host-and-other-protocol-level-headers

Conversation

@bogdanmariusc10
Copy link
Copy Markdown
Collaborator

🔗 Related Issue

Closes #4450


📝 Summary

Expands the inbound passthrough header denylist from a single entry (content-type) to 10 protocol-level headers that pose security risks when client-controlled. This prevents vhost selection attacks, cache poisoning, request smuggling, and encoding-dispatch bypasses.

What changed:

  • Added _INBOUND_PASSTHROUGH_DENYLIST constant with 10 security-critical headers
  • Enforced denylist in both get_passthrough_headers() and compute_passthrough_headers_cached()
  • Added 13 comprehensive security tests covering individual and combined denial scenarios
  • Updated existing tests to use safe alternative headers (X-Custom-Type instead of Content-Type)

Headers now blocked from inbound passthrough:

Note: authorization is intentionally NOT in the denylist as it has existing gateway-auth-aware handling via X-Upstream-Authorization rename logic.


🏷️ Type of Change

  • Bug fix
  • Feature / Enhancement
  • Documentation
  • Refactor
  • Chore (deps, CI, tooling)
  • Other (describe below)

🧪 Verification

Check Command Status
Lint suite make lint ✅ Pass
Unit tests make test ✅ Pass
Coverage ≥ 80% make coverage ✅ Pass

Test coverage:

  • 13 new security tests in TestInboundPassthroughDenylist class
  • Tests verify each header is denied via both default_passthrough_headers and per-gateway passthrough_headers
  • Tests verify case-insensitive matching and WARNING-level logging
  • Tests verify authorization is NOT blocked (existing special handling preserved)

✅ Checklist

  • Code formatted (make black isort pre-commit)
  • Tests added/updated for changes
  • Documentation updated (if applicable)
  • No secrets or credentials committed

📓 Notes

Security rationale per header:

Header Risk if client-supplied Severity
host Wrong upstream vhost selection on multi-tenant origins; cache-poisoning class Medium
transfer-encoding Request-smuggling primitive against fragile upstream parsers Medium
content-type Encoding-dispatch bypass (already fixed in PR #4139) Medium
content-length httpx auto-recalculates; defence-in-depth Low
connection, keep-alive, proxy-connection, te, trailer, upgrade Hop-by-hop — httpx manages; defence-in-depth Low

Design decisions:

  • Mirrored the protocol-level subset of existing _LOOPBACK_SKIP_HEADERS constant (same file, line 552)
  • Excluded gateway-internal routing headers (mcp-session-id, x-forwarded-internally) as they only matter in loopback contexts
  • Excluded authorization to preserve existing gateway-auth-aware handling
  • Logs at WARNING level when blocking headers for visibility and troubleshooting

Files modified:

  • mcpgateway/utils/passthrough_headers.py — added denylist constant and enforcement logic
  • tests/unit/mcpgateway/utils/test_passthrough_headers_security.py — added 13 new security tests
  • tests/unit/mcpgateway/utils/test_passthrough_headers.py — fixed 8 tests expecting Content-Type passthrough
  • tests/unit/mcpgateway/utils/test_passthrough_headers_fixed.py — fixed 4 tests expecting Content-Type passthrough
  • .secrets.baseline — updated baseline for new test content

Closes #4450

- Add _INBOUND_PASSTHROUGH_DENYLIST with 10 protocol-level headers
- Block content-type, content-length, host, transfer-encoding, and hop-by-hop headers
- Enforce denylist in get_passthrough_headers() and compute_passthrough_headers_cached()
- Add 13 comprehensive security tests for each denied header
- Update existing tests to use X-Custom-Type instead of Content-Type
- Log WARNING when blocking protocol-level headers

Security rationale:
- host: prevents vhost selection / cache-poisoning attacks
- transfer-encoding: prevents request-smuggling attacks
- content-type: prevents encoding-dispatch bypass (PR #4139)
- content-length: httpx-managed; defence-in-depth
- connection, keep-alive, proxy-connection, te, trailer, upgrade: hop-by-hop headers

Note: authorization intentionally NOT in denylist (has gateway-auth-aware handling)
Signed-off-by: Bogdan-Marius-Catanus <[email protected]>
Signed-off-by: Bogdan-Marius-Catanus <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request security Improves security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SECURITY]: Expand inbound passthrough denylist beyond Content-Type to cover Host and other protocol-level headers

1 participant