fix: enable DNS rebinding protection by default by devcrocod · Pull Request #659 · modelcontextprotocol/kotlin-sdk

devcrocod · 2026-03-30T23:34:08Z

Add DnsRebindingProtection Ktor route-scoped plugin and enable it by default across all http transports

Motivation and Context

DNS rebinding protection was disabled by default (enableDnsRebindingProtection = false), and sse endpoints (Route.mcp()) had zero protection. Same vulnerability as typescript sdk GHSA-w48q-cv73-mx4w

How Has This Been Tested?

all tests pass

Breaking Changes

Route.mcp(), Application.mcp() signatures changed (new enableDnsRebindingProtection, allowedHosts, allowedOrigins params), binary incompatible on JVM
enableDnsRebindingProtection default changed from false to true on mcpStreamableHttp/mcpStatelessStreamableHttp
Default allowed hosts: localhost, 127.0.0.1, [::1] — opt out with enableDnsRebindingProtection = false
Host validation is now port-agnostic (matches ts sdk)
Origin validation now allows missing Origin header (non-browser clients)
Configuration.enableDnsRebindingProtection, .allowedHosts, .allowedOrigins deprecated in favor of the plugin

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Copilot

Pull request overview

Adds a Ktor route-scoped DNS rebinding protection plugin and enables it by default for MCP HTTP/SSE endpoints, aligning behavior with the TypeScript SDK advisory.

Changes:

Introduces DnsRebindingProtection (RouteScopedPlugin) with default localhost allowlist and optional Origin allowlist.
Enables DNS rebinding protection by default in Route.mcp, Application.mcp, mcpStreamableHttp, and mcpStatelessStreamableHttp; deprecates the old transport configuration flags.
Updates/extends JVM tests and conformance server setup to reflect the new defaults and host parsing behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
kotlin-sdk-server/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/KtorRouteExtensionsTest.kt	Updates integration tests to explicitly disable protection where not under test.
kotlin-sdk-server/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/KtorApplicationExtensionsTest.kt	Updates Application extension test to explicitly disable protection where not under test.
kotlin-sdk-server/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/DnsRebindingProtectionTest.kt	Adds new tests covering Host/Origin validation and `extractHostname`.
kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/StreamableHttpServerTransport.kt	Deprecates old DNS config fields and updates header validation to be port-agnostic.
kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/KtorServer.kt	Adds new parameters to MCP helpers and enables protection by default via the new plugin.
kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/HostValidation.kt	New plugin + config + hostname extraction utility + default localhost allowlist.
kotlin-sdk-server/api/kotlin-sdk-server.api	Updates public API surface for new plugin/config and new MCP helper signatures.
conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/ConformanceServer.kt	Removes redundant explicit DNS settings now covered by new defaults.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov-commenter · 2026-03-30T23:50:36Z

Codecov Report

❌ Patch coverage is 68.75000% with 25 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...delcontextprotocol/kotlin/sdk/server/KtorServer.kt	51.61%	10 Missing and 5 partials ⚠️
...kotlin/sdk/server/StreamableHttpServerTransport.kt	33.33%	6 Missing ⚠️
...ontextprotocol/kotlin/sdk/server/HostValidation.kt	90.00%	0 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

e5l

lgtm, except comments. Please check comments before merging

kpavlov

The DNS rebinding protection plugin architecture is secure and well-designed, but the PR should address three medium-severity gaps before merge: the deprecated validateHeaders() still defaults to no protection for direct transport usage, error responses reflect raw attacker-controlled header values, and there are no integration tests for the primary mcpStreamableHttp/mcpStatelessStreamableHttp entry points.

devcrocod · 2026-04-10T17:04:39Z

@kpavlov Why did you block the PR on yourself if you were not even listed as a reviewer?

Also, I have already asked you to check AI reviews before posting them. We already have copilot for automated AI reviews

kpavlov · 2026-04-12T18:00:59Z

@devcrocod, as I mentioned in the review, the response currently exposes raw request headers, which is a potential security risk — that’s why I stepped in.

The AI-review also flags this and I agree it should be fixed.

kpavlov

I’ve unblocked the PR to avoid slowing things down. The comments I left still highlight a potential issue with exposing raw request headers, so it would be good to address that in the next update.

devcrocod · 2026-04-13T10:37:53Z

@kpavlov

as I mentioned in the review, the response currently exposes raw request headers, which is a potential security risk — that’s why I stepped in.

This had already been pointed out before you reviewed it. I don't have a habit of merging PRs without approval or of ignoring comments. That is exactly why I did not merge it, even after receiving approval. Therefore, your intervention remains unclear to me.

In addition, your own contribution is not entirely clear to me either: not all the comments seem substantive, and some of their wording appears to be AI-generated. That is why I have questions and would ask you not to limit your involvement to merely providing your github account for AI use

kpavlov · 2026-04-14T04:36:36Z

@devcrocod

This had already been pointed out before you reviewed it.

I don't see any comment about headers exposure neither from Copilot nor from another reviewer. Maybe I am missing something. Could you please share the link to the comment where it was pointed out?

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… plugin

…tests

…ling

e5l

lgtm

# Conflicts: # kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/StreamableHttpServerTransport.kt

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+     * **Note:** this method does not perform DNS rebinding protection on its own.
+     * When using this transport outside the [mcpStreamableHttp] / [mcpStatelessStreamableHttp]
+     * DSL, install the [DnsRebindingProtection] plugin on your Ktor route
+     * (or apply equivalent middleware) to validate `Host` / `Origin` headers.


+            if (hostname == null || hostname !in allowedHostsLowercase) {
                return "Invalid Host header: $hostHeader"
            }


Copilot AI review requested due to automatic review settings March 30, 2026 23:34

Copilot started reviewing on behalf of devcrocod March 30, 2026 23:34 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

devcrocod requested a review from e5l March 31, 2026 17:33

devcrocod marked this pull request as ready for review March 31, 2026 17:33

Copilot AI review requested due to automatic review settings March 31, 2026 17:33

Copilot started reviewing on behalf of devcrocod March 31, 2026 17:34 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

e5l previously approved these changes Apr 1, 2026

View reviewed changes

kpavlov requested changes Apr 1, 2026

View reviewed changes

kpavlov reviewed Apr 1, 2026

View reviewed changes

Comment thread kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/KtorServer.kt

Comment thread ...sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/HostValidation.kt Outdated

devcrocod added the security Security-related label Apr 10, 2026

kpavlov previously approved these changes Apr 12, 2026

View reviewed changes

devcrocod dismissed stale reviews from kpavlov and e5l via 61ed19b April 23, 2026 14:35

devcrocod force-pushed the devcrocod/dns-rebinding-protection branch from 8b7b725 to 61ed19b Compare April 23, 2026 14:35

Copilot AI review requested due to automatic review settings April 28, 2026 16:55

devcrocod force-pushed the devcrocod/dns-rebinding-protection branch from 61ed19b to ad29817 Compare April 28, 2026 16:55

Copilot started reviewing on behalf of devcrocod April 28, 2026 16:55 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

devcrocod added 5 commits April 28, 2026 19:13

fix: enable DNS rebinding protection by default via Ktor route-scoped…

d8a1f87

… plugin

refactor: make LOCALHOST_ALLOWED_HOSTS internal to limit API exposure

202326c

replace URLBuilder-based host parsing with strict Host header parser

14cfc9d

extract DnsRebindingProtection install into private helper

0920f6d

add test to ensure plugin with empty allowedHosts rejects all requests

5be53e2

devcrocod added 7 commits April 28, 2026 19:13

fix: update RPC error code for CONNECTION_CLOSED to standard value

18821bc

compare Origin by hostname and avoid echoing raw input

f3803df

inline LOCALHOST_ALLOWED_HOSTS defaults in public KDoc

c447b97

fail fast on invalid DnsRebindingProtection.allowedHosts entries

8a0bfb0

improve Host header validation and refactor DNS rebinding protection …

36e2610

…tests

set explicit Host header in mcpStreamableHttp SSE test

7b79695

compare Origin header by hostname and improve case sensitivity hand…

f4ac6fa

…ling

devcrocod force-pushed the devcrocod/dns-rebinding-protection branch from ad29817 to f4ac6fa Compare April 28, 2026 17:23

devcrocod requested a review from e5l April 28, 2026 17:24

e5l previously approved these changes Apr 29, 2026

View reviewed changes

devcrocod added 2 commits April 29, 2026 13:12

Merge branch 'main' into devcrocod/dns-rebinding-protection

410cd84

# Conflicts: # kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/StreamableHttpServerTransport.kt

set explicit Host header in StreamableHttpServerTransport tests

3501b53

Copilot AI review requested due to automatic review settings April 29, 2026 11:23

devcrocod dismissed e5l’s stale review via 3501b53 April 29, 2026 11:24

devcrocod requested a review from e5l April 29, 2026 11:24

Copilot started reviewing on behalf of devcrocod April 29, 2026 11:24 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Conversation

devcrocod commented Mar 30, 2026

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

e5l left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kpavlov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devcrocod commented Apr 10, 2026

Uh oh!

kpavlov commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kpavlov left a comment

Choose a reason for hiding this comment

Uh oh!

devcrocod commented Apr 13, 2026

Uh oh!

kpavlov commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

e5l left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

codecov-commenter commented Mar 30, 2026 •

edited

Loading

kpavlov left a comment •

edited

Loading

kpavlov commented Apr 12, 2026 •

edited

Loading

kpavlov commented Apr 14, 2026 •

edited

Loading