feat: add DNS rebinding protection conformance tests #115
Conversation
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
Add a new server conformance scenario to verify that localhost MCP servers properly validate Host headers to prevent DNS rebinding attacks. Checks: - localhost-host-rebinding-rejected: Verifies server returns 403 for non-localhost Host headers (e.g., evil.example.com) - localhost-host-valid-accepted: Verifies server accepts requests with valid localhost Host headers Also updates the everything-server example to use createMcpExpressApp() which includes DNS rebinding protection by default. Closes #103 Co-Authored-By: Claude <noreply@anthropic.com>
Add a minimal MCP server that intentionally omits DNS rebinding protection to serve as a negative test case. This server is expected to FAIL the dns-rebinding-protection scenario. Also update README to document the negative test case. Co-Authored-By: Claude <noreply@anthropic.com>
Update the DNS rebinding protection test to accept any 4xx status code as a valid rejection response, not just 403. This accommodates different HTTP semantics: - 403 Forbidden: Per MCP spec - 421 Misdirected Request: RFC 7540 (semantically correct for DNS rebinding) - Other 4xx: Implementation-specific error codes
commit: |
pcarleton
commented
Jan 22, 2026
| EventId, | ||
| StreamId | ||
| } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; | ||
| import { createMcpExpressApp } from '@modelcontextprotocol/sdk/server/express.js'; |
Member
Author
There was a problem hiding this comment.
note: this example is going to get deleted in a follow-up PR to make the typescript-sdk example the source of truth. updating it in place for now.
- Simplify negative test server (remove event store, sessions) - Update spec reference URL to security_best_practices - Clarify scope: localhost servers without HTTPS/auth - DRY up checks.push using spread operator - Fix valid host check to require 2xx response
maxisbey
requested changes
Jan 22, 2026
…tion - Send both Host and Origin headers in test requests so servers checking either header will pass the conformance test - Add second spec reference for Origin check (transports#security-warning) - Update descriptions to mention both Host and Origin headers - Fix protocol version to 2025-11-25 - Remove unnecessary 'as const' type assertions
maxisbey
reviewed
Jan 22, 2026
Comment on lines
207
to
208
| hostHeader: validHost, | ||
| originHeader: `http://${validHost}`, |
Contributor
There was a problem hiding this comment.
Maybe we should actually be testing these separately? Otherwise we don't know whether the server will reject both a bad host and a bad origin, we only know that it will reject either:
- both
- only header
- only origin
but not which specifically
Member
Author
There was a problem hiding this comment.
that's intentional, we want either protection to be valid. Checking host or origin will prevent the attack.
maxisbey
approved these changes
Jan 22, 2026
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.
Summary
Add conformance tests to verify localhost MCP servers properly validate Host headers to prevent DNS rebinding attacks.
Closes #103
Changes
New scenario:
dns-rebinding-protectionTwo checks:
localhost-host-rebinding-rejected: Server returns 4xx for non-localhost Host headers (e.g.,Host: evil.example.com)localhost-host-valid-accepted: Server accepts valid localhost Host headersImplementation details
undicifor making HTTP requests with custom Host headers (native fetch doesn't allow this)Additional changes
everything-serverexample to usecreateMcpExpressApp()for DNS rebinding protectionno-dns-rebinding-protection.tsthat intentionally omits protectionTesting
Verified against:
createMcpExpressApp())Related