DefaultSseMessageEndpointValidator allows same-origin message endpoints
Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
This commit is contained in:
parent
563b1551a3
commit
bf30e21f0d
2 changed files with 25 additions and 21 deletions
|
|
@ -10,8 +10,8 @@ import io.modelcontextprotocol.util.Assert;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Default {@link SseMessageEndpointValidator} that validates the {@code message} endpoint
|
* Default {@link SseMessageEndpointValidator} that validates the {@code message} endpoint
|
||||||
* advertised by an SSE server. Message endpoints must be a relative URI, without path
|
* advertised by an SSE server. Message endpoints must either have the same origin as the
|
||||||
* traversal or authority.
|
* SSE uri, or be a relative uri.
|
||||||
*
|
*
|
||||||
* @author Daniel Garnier-Moiroux
|
* @author Daniel Garnier-Moiroux
|
||||||
*/
|
*/
|
||||||
|
|
@ -30,16 +30,19 @@ public final class DefaultSseMessageEndpointValidator implements SseMessageEndpo
|
||||||
messageEndpoint);
|
messageEndpoint);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (endpointUri.isAbsolute()) {
|
if (endpointUri.isAbsolute() || endpointUri.getRawAuthority() != null) {
|
||||||
// Exclude absolute URIs e.g. https://example.com/mcp
|
String scheme = endpointUri.getScheme();
|
||||||
throw new InvalidSseMessageEndpointException("messageEndpoint must be a relative path, not an absolute URI",
|
String host = endpointUri.getHost();
|
||||||
messageEndpoint);
|
int port = endpointUri.getPort();
|
||||||
}
|
|
||||||
|
|
||||||
if (endpointUri.getRawAuthority() != null) {
|
boolean sameScheme = scheme != null && scheme.equalsIgnoreCase(sseUri.getScheme());
|
||||||
// Exclude network paths e.g. //example.com/mcp
|
boolean sameHost = host != null && host.equalsIgnoreCase(sseUri.getHost());
|
||||||
throw new InvalidSseMessageEndpointException(
|
boolean samePort = port == sseUri.getPort();
|
||||||
"messageEndpoint must be a relative path and must not contain an authority", messageEndpoint);
|
|
||||||
|
if (!sameScheme || !sameHost || !samePort) {
|
||||||
|
throw new InvalidSseMessageEndpointException(
|
||||||
|
"messageEndpoint must be a relative path or a same-origin URI", messageEndpoint);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Exclude path-traversal
|
// Exclude path-traversal
|
||||||
|
|
|
||||||
|
|
@ -25,7 +25,7 @@ class DefaultSseMessageEndpointValidatorTests {
|
||||||
private final DefaultSseMessageEndpointValidator validator = new DefaultSseMessageEndpointValidator();
|
private final DefaultSseMessageEndpointValidator validator = new DefaultSseMessageEndpointValidator();
|
||||||
|
|
||||||
@ParameterizedTest
|
@ParameterizedTest
|
||||||
@ValueSource(strings = { "/messages", "messages?session=abc", "/" })
|
@ValueSource(strings = { "/messages", "messages?session=abc", "/", "https://mcp.example.com/messages" })
|
||||||
void valid(String endpoint) {
|
void valid(String endpoint) {
|
||||||
assertThatCode(() -> validator.validate(SSE_URI, endpoint)).doesNotThrowAnyException();
|
assertThatCode(() -> validator.validate(SSE_URI, endpoint)).doesNotThrowAnyException();
|
||||||
}
|
}
|
||||||
|
|
@ -41,20 +41,20 @@ class DefaultSseMessageEndpointValidatorTests {
|
||||||
@ParameterizedTest
|
@ParameterizedTest
|
||||||
@ValueSource(strings = { "/foo/../bar", "/foo/./bar", "../bar", "./bar", "/foo/%2E%2E/bar", "/foo/%2e/bar" })
|
@ValueSource(strings = { "/foo/../bar", "/foo/./bar", "../bar", "./bar", "/foo/%2E%2E/bar", "/foo/%2e/bar" })
|
||||||
void invalidPathTraversal(String endpoint) {
|
void invalidPathTraversal(String endpoint) {
|
||||||
assertThatThrownBy(() -> validator.validate(SSE_URI, endpoint)).hasMessageContaining("path-traversal")
|
assertThatThrownBy(() -> validator.validate(SSE_URI, endpoint))
|
||||||
|
.hasMessageContaining("must not contain path-traversal segments")
|
||||||
.asInstanceOf(type(InvalidSseMessageEndpointException.class))
|
.asInstanceOf(type(InvalidSseMessageEndpointException.class))
|
||||||
.extracting(InvalidSseMessageEndpointException::getMessageEndpoint)
|
.extracting(InvalidSseMessageEndpointException::getMessageEndpoint)
|
||||||
.isEqualTo(endpoint);
|
.isEqualTo(endpoint);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ParameterizedTest
|
@ParameterizedTest
|
||||||
@ValueSource(strings = { "https://mcp.example.com/messages", "https://127.0.0.1/messages",
|
@ValueSource(strings = { "https://127.0.0.1/messages", "https://mcp.example.com:8443/messages",
|
||||||
"https://mcp.example.com:8443/messages", "http://localhost:1234/messages", "file:///etc/passwd",
|
"http://localhost:1234/messages", "file:///etc/passwd", "gopher://mcp.example.com/_test" })
|
||||||
"gopher://mcp.example.com/_test" })
|
|
||||||
void invalidAbsoluteUris(String endpoint) {
|
void invalidAbsoluteUris(String endpoint) {
|
||||||
// Even an absolute URI on the same origin must be rejected: the contract
|
// Absolute URIs must be same-origin.
|
||||||
// is that the messageEndpoint is a path-only relative reference.
|
assertThatThrownBy(() -> validator.validate(SSE_URI, endpoint))
|
||||||
assertThatThrownBy(() -> validator.validate(SSE_URI, endpoint)).hasMessageContaining("must be a relative path")
|
.hasMessageContaining("must be a relative path or a same-origin URI")
|
||||||
.asInstanceOf(type(InvalidSseMessageEndpointException.class))
|
.asInstanceOf(type(InvalidSseMessageEndpointException.class))
|
||||||
.extracting(InvalidSseMessageEndpointException::getMessageEndpoint)
|
.extracting(InvalidSseMessageEndpointException::getMessageEndpoint)
|
||||||
.isEqualTo(endpoint);
|
.isEqualTo(endpoint);
|
||||||
|
|
@ -62,11 +62,12 @@ class DefaultSseMessageEndpointValidatorTests {
|
||||||
}
|
}
|
||||||
|
|
||||||
@ParameterizedTest
|
@ParameterizedTest
|
||||||
@ValueSource(strings = { "//example/messages", "//user:secret@example/messages" })
|
@ValueSource(strings = { "//example/messages", "//user:secret@example/messages", "//mcp.example.com/messages" })
|
||||||
void invalidNetworkReference(String endpoint) {
|
void invalidNetworkReference(String endpoint) {
|
||||||
// `//host/...` introduces an authority and is therefore not a pure path.
|
// `//host/...` introduces an authority and is therefore not a pure path.
|
||||||
|
// It is missing a scheme, so it fails same-origin check.
|
||||||
assertThatThrownBy(() -> validator.validate(SSE_URI, endpoint))
|
assertThatThrownBy(() -> validator.validate(SSE_URI, endpoint))
|
||||||
.hasMessageContaining("must not contain an authority")
|
.hasMessageContaining("must be a relative path or a same-origin URI")
|
||||||
.asInstanceOf(type(InvalidSseMessageEndpointException.class))
|
.asInstanceOf(type(InvalidSseMessageEndpointException.class))
|
||||||
.extracting(InvalidSseMessageEndpointException::getMessageEndpoint)
|
.extracting(InvalidSseMessageEndpointException::getMessageEndpoint)
|
||||||
.isEqualTo(endpoint);
|
.isEqualTo(endpoint);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue