Validate origin header #771

Kehrlann · 2026-02-04T18:20:32Z

Fixes #695 . Updates conformance tests.

New APIs: example usage

HttpServletStreamableServerTransportProvider.builder()
// ...
.securityValidator(
    DefaultServerTransportSecurityValidator.builder()
        .allowedOrigin("http://localhost:*")
        .build())
// ...

Points of interest

Integration tests use the new JUnit 6 @ParameterizedClass API, which is another way of implementation "abstract base classes" for tests.

- Fixes #695 - Does not implement Host header validation yet Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>

Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>

tzolov

LGTM. Great work @Kehrlann !

tzolov · 2026-02-05T08:35:51Z

...a/io/modelcontextprotocol/server/transport/DefaultServerTransportSecurityValidatorTests.java

+
+		@Test
+		void differentSchemeWithWildcard() {
+			var headers = originHeader("https://localhost:3000");


shouldn't the port be 8080 to ensure that it fails because of the schema?

It's using the the wildcard validator, so the port does not matter:

private final DefaultServerTransportSecurityValidator wildcardValidator = DefaultServerTransportSecurityValidator .builder() .allowedOrigin("http://localhost:*") .build();

chemicL

Thank you @Kehrlann for taking care of this! I added a few comments post merge. Perhaps eliminating the duplicated code can be addressed as a follow-up. Great job overall!

chemicL · 2026-02-10T20:17:57Z

...n/java/io/modelcontextprotocol/server/transport/DefaultServerTransportSecurityValidator.java

+
+		}
+
+		throw INVALID_ORIGIN;


Due to the exception being a static, the stack trace is inaccurate with regards to where it is raised. I understand the optimization, however it might make it harder to investigate where the issue happened. Perhaps we can create an instance each time it's raised?

chemicL · 2026-02-10T20:19:27Z

...n/java/io/modelcontextprotocol/server/transport/DefaultServerTransportSecurityValidator.java

+	 * @param allowedOrigins List of allowed origin patterns. Supports exact matches
+	 * (e.g., "http://example.com:8080") and wildcard ports (e.g., "http://example.com:*")
+	 */
+	public DefaultServerTransportSecurityValidator(List<String> allowedOrigins) {


There are two ways to create the validator - either via a constructor or via the builder. I'd vouch for a single version. The builder makes sense with no inheritance, but since users are encouraged to customize by inheriting from this class, perhaps we can skip the builder?

chemicL · 2026-02-10T20:20:55Z

...ain/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java

+		try {
+			Map<String, List<String>> headers = extractHeaders(request);
+			this.securityValidator.validateHeaders(headers);
+		}
+		catch (ServerTransportSecurityException e) {
+			response.sendError(e.getStatusCode(), e.getMessage());
+			return;
+		}
+


This bit is duplicated in doGet, we can create a helper method for it perhaps?

chemicL · 2026-02-10T20:22:39Z

.../main/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java

+		try {
+			Map<String, List<String>> headers = extractHeaders(request);
+			this.securityValidator.validateHeaders(headers);
+		}
+		catch (ServerTransportSecurityException e) {
+			response.sendError(e.getStatusCode(), e.getMessage());
+			return;
+		}


This code is duplicated across three implementations, perhaps we can have a centralized static utility for it?

chemicL · 2026-02-10T20:23:15Z

.../main/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java

+	 * @param request The HTTP servlet request
+	 * @return A map of header names to their values
+	 */
+	private Map<String, List<String>> extractHeaders(HttpServletRequest request) {


This private method is also duplicated across implementations, another candidate for sharing.

chemicL · 2026-02-10T20:23:58Z

...a/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java

+		try {
+			Map<String, List<String>> headers = extractHeaders(request);
+			this.securityValidator.validateHeaders(headers);
+		}
+		catch (ServerTransportSecurityException e) {
+			response.sendError(e.getStatusCode(), e.getMessage());
+			return;
+		}


Duplicated :)

chemicL · 2026-02-10T20:24:30Z

...a/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java

+		try {
+			Map<String, List<String>> headers = extractHeaders(request);
+			this.securityValidator.validateHeaders(headers);
+		}
+		catch (ServerTransportSecurityException e) {
+			response.sendError(e.getStatusCode(), e.getMessage());
+			return;
+		}


Another duplicate that can reuse a shared util.

chemicL · 2026-02-10T20:24:40Z

...a/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java

+	 * @param request The HTTP servlet request
+	 * @return A map of header names to their values
+	 */
+	private Map<String, List<String>> extractHeaders(HttpServletRequest request) {


chemicL · 2026-02-10T20:28:55Z

...t/java/io/modelcontextprotocol/server/transport/ServerTransportSecurityIntegrationTests.java

+	 * All transport types we want to test. We use a {@link MethodSource} rather than a
+	 * {@link org.junit.jupiter.params.provider.ValueSource} to provide a readable name.
+	 */
+	static Stream<Arguments> transports() {


chemicL · 2026-02-10T20:30:30Z

...rc/main/java/io/modelcontextprotocol/server/transport/WebFluxSseServerTransportProvider.java

+		try {
+			Map<String, List<String>> headers = request.headers().asHttpHeaders();
+			this.securityValidator.validateHeaders(headers);
+		}
+		catch (ServerTransportSecurityException e) {
+			return ServerResponse.status(e.getStatusCode()).bodyValue(e.getMessage());
+		}


This can be unified perhaps too.

Add Origin header validation

9903b56

- Fixes #695 - Does not implement Host header validation yet Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>

Kehrlann requested review from chemicL and tzolov and removed request for chemicL February 4, 2026 20:19

Update conformance tests with DNS rebinding protection

68ed795

Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>

Kehrlann force-pushed the dgarnier/validate-origin-header branch from e5624d4 to 68ed795 Compare February 4, 2026 20:20

Kehrlann requested a review from chemicL February 4, 2026 20:23

Kehrlann marked this pull request as ready for review February 4, 2026 20:23

tzolov approved these changes Feb 5, 2026

View reviewed changes

Kehrlann merged commit 5ed6063 into main Feb 5, 2026
6 checks passed

Kehrlann deleted the dgarnier/validate-origin-header branch February 5, 2026 09:20

chemicL reviewed Feb 10, 2026

View reviewed changes

Validate origin header #771

Validate origin header #771

Uh oh!

Conversation

Kehrlann commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contents

New APIs: example usage

Points of interest

Uh oh!

tzolov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chemicL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kehrlann commented Feb 4, 2026 •

edited

Loading