From 078cf84fdbf9f07bd724c25eeca4d1039cabec9a Mon Sep 17 00:00:00 2001 From: Eric Hare Date: Thu, 5 Nov 2026 11:00:02 -0800 Subject: [PATCH] fix: validate MCP commands with the HTTP API constraints --- README.md | 1 + docs/README.md | 4 + docs/mcp.md | 31 +++ .../sgv2/jsonapi/api/v1/mcp/McpResource.java | 17 ++ .../mappers/FrameworkExceptionMapper.java | 10 +- .../api/v1/mcp/McpCommandValidationTest.java | 185 ++++++++++++++++++ .../jsonapi/api/v1/mcp/McpResourceTest.java | 2 + .../api/v1/mcp/McpSetupErrorsTest.java | 2 + 8 files changed, 250 insertions(+), 2 deletions(-) create mode 100644 docs/mcp.md create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpCommandValidationTest.java diff --git a/README.md b/README.md index 6659a5897b..9dc7688f63 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,7 @@ This project implements the stand-alone Data API microservice for Stargate. Data API is an HTTP service that gives access to data stored in a Cassandra cluster using a JSON Document based interface. Specifications and design documents for this service are defined in the [docs](docs) directory. +See the [MCP guide](docs/mcp.md) for access through MCP tools. ##### Table of Contents diff --git a/docs/README.md b/docs/README.md index bed89e092e..939797722b 100644 --- a/docs/README.md +++ b/docs/README.md @@ -7,3 +7,7 @@ This folder contains documentation for the Data API. * The [network specification](dataapi-network-spec.md) specifies the HTTP API for the Data API, how clients can communicate with the service. * The [query specification](dataapi-spec.md) provides details on data modelling and queries. + +## MCP + +* The [MCP guide](mcp.md) covers the endpoint, request behavior, results, and command validation. diff --git a/docs/mcp.md b/docs/mcp.md new file mode 100644 index 0000000000..2b283f9d7f --- /dev/null +++ b/docs/mcp.md @@ -0,0 +1,31 @@ +# MCP tools + +The Data API exposes commands as MCP tools over Streamable HTTP. Use `tools/list` to discover their input schemas. The [tool behavior guide](mcp-tool-behavior-hints.md) explains the catalog hints and retry behavior. + +## Endpoint and requests + +The default endpoint is `/v1/mcp`. It follows the HTTP and MCP root paths described in the [configuration guide](../CONFIGURATION.md#mcp-logging). Keep authentication paths aligned when changing the endpoint. + +Tool execution is disabled by default. Enable it with `stargate.feature.flags.mcp=true`, or send `Feature-Flag-mcp` with value `true` when the server has no configured override. This flag does not grant database permissions. + +Current tests send protocol version `2026-07-28` and call tools without initialization or a session ID. Each request carries its own protocol metadata. [McpHttpTestSupport](../src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpHttpTestSupport.java) defines the tested headers and JSON-RPC envelope. + +The current implementation uses the existing Data API `Token` header. A missing token returns HTTP 401 before tool execution. OAuth connection and migration instructions remain separate work in [#2643](https://github.com/stargate/data-api/issues/2643). These tests do not establish compatibility with every MCP client or protocol revision. + +## Results and errors + +A Data API command result keeps text content empty. Successful structured content contains the data, while status remains in `_meta.status`. For application errors, structured content contains the errors and `isError` is true. When a command partly fails, structured content holds only the errors, and status such as inserted IDs stays in `_meta.status`. + +Unknown keyspaces and collections return application errors in a completed tool result. Unexpected setup failures return a generic message. Malformed JSON, unknown methods, and unknown tools remain JSON-RPC errors. Missing required tool arguments remain framework tool errors and can contain plain text without structured content. Authentication failures retain their HTTP handling. + +## Command validation + +MCP applies the same command validation annotations as HTTP before schema lookup or command execution. Invalid arguments return `COMMAND_FIELD_VALUE_INVALID` as an application error. Validation rules and configurable limits remain on the shared command models. + +The [endpoint comparison tests](../src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpCommandValidationTest.java) cover empty and oversized insert batches, invalid nested find options, and skip without sort. Valid inputs at the batch limit and omitted, null, or empty optional arguments remain accepted. + +Use the schemas returned by `tools/list` for accepted arguments. The [query specification](dataapi-spec.md) describes command semantics, and the [configuration guide](../CONFIGURATION.md#operations-configuration) records configurable operation limits. + +## Diagnostics + +Follow [MCP logging](../CONFIGURATION.md#mcp-logging) for request correlation, redaction, and temporary diagnostics. That section also covers custom endpoint paths and log handlers. diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResource.java index 0545054a74..29eb50ea2a 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResource.java @@ -22,6 +22,7 @@ import io.stargate.sgv2.jsonapi.exception.APIException; import io.stargate.sgv2.jsonapi.exception.SchemaException; import io.stargate.sgv2.jsonapi.exception.ServerException; +import io.stargate.sgv2.jsonapi.exception.mappers.FrameworkExceptionMapper; import io.stargate.sgv2.jsonapi.metrics.JsonProcessingMetricsReporter; import io.stargate.sgv2.jsonapi.service.cqldriver.CqlSessionCacheSupplier; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.*; @@ -35,6 +36,8 @@ import jakarta.enterprise.inject.Instance; import jakarta.inject.Inject; import jakarta.inject.Provider; +import jakarta.validation.ConstraintViolationException; +import jakarta.validation.Validator; import jakarta.ws.rs.WebApplicationException; import java.util.concurrent.CancellationException; import java.util.concurrent.CompletionException; @@ -52,6 +55,7 @@ public class McpResource { private static final Logger LOGGER = LoggerFactory.getLogger(McpResource.class); private final ObjectMapper objectMapper; + private final Validator validator; private final MeteredCommandProcessor meteredCommandProcessor; private final CommandContext.BuilderSupplier contextBuilderSupplier; private final EmbeddingProviderFactory embeddingProviderFactory; @@ -66,6 +70,7 @@ public class McpResource { @Inject public McpResource( ObjectMapper objectMapper, + Validator validator, MeteredCommandProcessor meteredCommandProcessor, MeterRegistry meterRegistry, JsonProcessingMetricsReporter jsonProcessingMetricsReporter, @@ -81,6 +86,7 @@ public McpResource( // TODO: these vars are needed to replicate what we do in GeneralResource, // KeyspaceResource, etc. We should refactor to avoid duplication later. this.objectMapper = objectMapper; + this.validator = validator; this.meteredCommandProcessor = meteredCommandProcessor; this.embeddingProviderFactory = embeddingProviderFactory; this.schemaObjectCacheSupplier = schemaObjectCacheSupplier; @@ -231,6 +237,10 @@ public Uni processCommand( try { // Mutiny captures context when callbacks are assembled. McpRequestLogging.activate(routingContextProvider.get()); + var violations = validator.validate(command); + if (!violations.isEmpty()) { + throw new ConstraintViolationException(violations); + } contextUni = contextSupplier.get(); } catch (Exception failure) { contextUni = Uni.createFrom().failure(failure); @@ -283,6 +293,13 @@ private Uni handleSetupFailure(Throwable failure) { failure = failure.getCause(); } + if (failure instanceof ConstraintViolationException validationFailure) { + return Uni.createFrom() + .item( + FrameworkExceptionMapper.constraintViolationResult( + validationFailure.getConstraintViolations())); + } + // HTTP authentication, protocol errors, and framework control signals retain their own // handling. APISecurityException is not among them: the database raises it after HTTP // authentication has admitted the request, so it is returned below as a tool error. diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/FrameworkExceptionMapper.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/FrameworkExceptionMapper.java index c8b315819b..19202136dc 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/FrameworkExceptionMapper.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/FrameworkExceptionMapper.java @@ -20,6 +20,7 @@ import java.util.Collections; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import org.jboss.resteasy.reactive.RestResponse; import org.jboss.resteasy.reactive.server.ServerExceptionMapper; @@ -132,17 +133,22 @@ public RestResponse mapJakartaException( @ServerExceptionMapper({ConstraintViolationException.class}) public RestResponse constraintViolationException( ConstraintViolationException exception) { + return constraintViolationResult(exception.getConstraintViolations()).toRestResponse(); + } + /** Maps command constraints to the same API errors for HTTP and MCP. */ + public static CommandResult constraintViolationResult( + Set> violations) { var builder = CommandResult.statusOnlyBuilder(RequestTracing.NO_OP); // this used to have a distinct() call, but it was doing it on all CommandError objects which // like the ApiException has a unique ID so had not affect. // LOGGING is in apiExceptionFor() - exception.getConstraintViolations().stream() + violations.stream() .map(FrameworkExceptionMapper::apiExceptionFor) .forEach(builder::addThrowable); - return builder.build().toRestResponse(); + return builder.build(); } private static RestResponse responseForException(RuntimeException exception) { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpCommandValidationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpCommandValidationTest.java new file mode 100644 index 0000000000..6b93970951 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpCommandValidationTest.java @@ -0,0 +1,185 @@ +package io.stargate.sgv2.jsonapi.api.v1.mcp; + +import static io.restassured.RestAssured.given; +import static io.stargate.sgv2.jsonapi.api.v1.mcp.McpHttpTestSupport.assertResponseComplete; +import static io.stargate.sgv2.jsonapi.api.v1.mcp.McpHttpTestSupport.request; +import static io.stargate.sgv2.jsonapi.api.v1.mcp.McpHttpTestSupport.toolPayload; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import io.quarkus.test.InjectMock; +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; +import io.restassured.http.ContentType; +import io.smallrye.mutiny.Uni; +import io.stargate.sgv2.jsonapi.TestConstants; +import io.stargate.sgv2.jsonapi.api.model.command.Command; +import io.stargate.sgv2.jsonapi.api.model.command.CommandName; +import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; +import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; +import io.stargate.sgv2.jsonapi.api.model.command.tracing.RequestTracing; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeature; +import io.stargate.sgv2.jsonapi.exception.RequestException; +import io.stargate.sgv2.jsonapi.service.processor.MeteredCommandProcessor; +import io.stargate.sgv2.jsonapi.service.schema.SchemaObjectCache; +import io.stargate.sgv2.jsonapi.service.schema.SchemaObjectCacheSupplier; +import io.stargate.sgv2.jsonapi.testresource.NoGlobalResourcesTestProfile; +import io.vertx.core.json.JsonObject; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.ArgumentCaptor; + +/** Compares command validation at the real HTTP and MCP boundaries before schema or execution. */ +@QuarkusTest +@TestProfile(McpCommandValidationTest.Profile.class) +class McpCommandValidationTest { + private static final TestConstants TEST_CONSTANTS = new TestConstants(); + private static final String TOKEN = "command-validation-test-credential"; + + @InjectMock SchemaObjectCacheSupplier schemaObjectCacheSupplier; + @InjectMock MeteredCommandProcessor commandProcessor; + private SchemaObjectCache schemaObjectCache; + + @BeforeEach + void setUp() { + schemaObjectCache = mock(SchemaObjectCache.class); + when(schemaObjectCacheSupplier.get()).thenReturn(schemaObjectCache); + doReturn(Uni.createFrom().item(TEST_CONSTANTS.COLLECTION_SCHEMA_OBJECT)) + .when(schemaObjectCache) + .getTableBased(any(), any(), any(), anyBoolean()); + when(commandProcessor.processCommand(any(), any())) + .thenReturn( + Uni.createFrom() + .item( + CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) + .addStatus(CommandStatus.OK, 1) + .build())); + } + + @ParameterizedTest + @MethodSource("invalidCommands") + void rejectsInvalidCommandsBeforeSchemaOrExecution( + String command, Map arguments, String reason) { + var httpResult = callHttp(command, arguments); + assertValidationError(httpResult, reason); + verifyNoInteractions(schemaObjectCacheSupplier, schemaObjectCache, commandProcessor); + + var mcpResult = callMcp(command, arguments); + assertThat(mcpResult.getBoolean("isError")).isTrue(); + assertThat(mcpResult.getJsonArray("content")).isEmpty(); + assertValidationError(mcpResult.getJsonObject("structuredContent"), reason); + verifyNoInteractions(schemaObjectCacheSupplier, schemaObjectCache, commandProcessor); + } + + static Stream invalidCommands() { + return Stream.of( + Arguments.of( + CommandName.Names.INSERT_MANY, Map.of("documents", List.of()), "must not be empty"), + Arguments.of( + CommandName.Names.INSERT_MANY, + Map.of("documents", List.of(Map.of(), Map.of(), Map.of())), + "amount of documents to insert is over the max limit (3 vs 2)"), + Arguments.of( + CommandName.Names.FIND, + Map.of("options", Map.of("limit", 0)), + "limit should be greater than `0`"), + Arguments.of( + CommandName.Names.FIND, + Map.of("options", Map.of("skip", 1)), + "skip options should be used with sort clause")); + } + + @ParameterizedTest + @MethodSource("validCommands") + void acceptsValidAndOptionalArguments(String command, Map arguments) { + var httpResult = callHttp(command, arguments); + var mcpResult = callMcp(command, arguments); + + assertThat(httpResult.getJsonObject("status")).as(httpResult.encode()).isNotNull(); + assertThat(httpResult.getJsonObject("status").getInteger("ok")).isEqualTo(1); + assertThat(httpResult.containsKey("errors")).isFalse(); + assertThat(mcpResult.getBoolean("isError")).isFalse(); + assertThat(mcpResult.getJsonArray("content")).isEmpty(); + assertThat(mcpResult.getValue("structuredContent")).isNull(); + assertThat(mcpResult.getJsonObject("_meta").getJsonObject("status")) + .isEqualTo(httpResult.getJsonObject("status")); + var commands = ArgumentCaptor.forClass(Command.class); + verify(commandProcessor, times(2)).processCommand(any(), commands.capture()); + assertThat(commands.getAllValues().get(0)).isEqualTo(commands.getAllValues().get(1)); + verify(schemaObjectCache, times(2)).getTableBased(any(), any(), any(), anyBoolean()); + } + + static Stream validCommands() { + var nullOptions = new HashMap(); + nullOptions.put("options", null); + return Stream.of( + Arguments.of( + CommandName.Names.INSERT_MANY, + Map.of("documents", List.of(Map.of("text", "café 🐘 'quoted'"), Map.of()))), + Arguments.of(CommandName.Names.FIND, Map.of()), + Arguments.of(CommandName.Names.FIND, nullOptions), + Arguments.of(CommandName.Names.FIND, Map.of("options", Map.of())), + Arguments.of(CommandName.Names.FIND, Map.of("options", Map.of("limit", 1)))); + } + + private JsonObject callHttp(String command, Map arguments) { + return new JsonObject( + given() + .contentType(ContentType.JSON) + .header("Token", TOKEN) + .body(Map.of(command, arguments)) + .post("/v1/" + TEST_CONSTANTS.KEYSPACE_NAME + "/" + TEST_CONSTANTS.COLLECTION_NAME) + .then() + .statusCode(200) + .extract() + .asString()); + } + + private JsonObject callMcp(String command, Map arguments) { + var toolArguments = new HashMap<>(arguments); + toolArguments.put("keyspace", TEST_CONSTANTS.KEYSPACE_NAME); + toolArguments.put("collection", TEST_CONSTANTS.COLLECTION_NAME); + return assertResponseComplete( + request("tools/call") + .header("Token", TOKEN) + .header(ApiFeature.MCP.httpHeaderName(), "true") + .header("Mcp-Name", command) + .body(toolPayload(1, command, toolArguments)) + .post("/v1/mcp"), + 1); + } + + private static void assertValidationError(JsonObject result, String reason) { + var errors = result.getJsonArray("errors"); + assertThat(errors).as(result.encode()).hasSize(1); + var error = errors.getJsonObject(0); + assertThat(error.getString("errorCode")) + .as(result.encode()) + .isEqualTo(RequestException.Code.COMMAND_FIELD_VALUE_INVALID.name()); + assertThat(error.getString("message")).contains(reason); + } + + public static class Profile implements NoGlobalResourcesTestProfile { + @Override + public Map getConfigOverrides() { + return Map.of( + "stargate.jsonapi.operations.max-document-insert-count", "2", + "stargate.multi-tenancy.enabled", "true", + "stargate.multi-tenancy.tenant-resolver.type", "fixed", + "stargate.multi-tenancy.tenant-resolver.fixed.tenant-id", "command-validation-test"); + } + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java index ff95fde389..efc3bb2a41 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java @@ -41,6 +41,7 @@ import io.vertx.ext.web.RoutingContext; import jakarta.enterprise.inject.Instance; import jakarta.inject.Provider; +import jakarta.validation.Validator; import java.security.Principal; import java.time.Duration; import java.util.Map; @@ -247,6 +248,7 @@ private McpResource mcpResource( Instance embeddingCredentialsResolver) { return new McpResource( mock(ObjectMapper.class), + mock(Validator.class), commandProcessor, mock(MeterRegistry.class), mock(JsonProcessingMetricsReporter.class), diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpSetupErrorsTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpSetupErrorsTest.java index 17769f2470..636045ff94 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpSetupErrorsTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpSetupErrorsTest.java @@ -65,6 +65,7 @@ import io.vertx.core.http.HttpServerRequest; import io.vertx.ext.web.RoutingContext; import jakarta.enterprise.inject.Instance; +import jakarta.validation.Validator; import jakarta.ws.rs.WebApplicationException; import java.security.Principal; import java.time.Duration; @@ -142,6 +143,7 @@ void setUp() { resource = new McpResource( objectMapper, + mock(Validator.class), commandProcessor, mock(MeterRegistry.class), mock(JsonProcessingMetricsReporter.class),