diff --git a/config/src/main/resources/custom-application-schemas/schema.json b/config/src/main/resources/custom-application-schemas/schema.json index 332cf0d5b..b80a4dd04 100644 --- a/config/src/main/resources/custom-application-schemas/schema.json +++ b/config/src/main/resources/custom-application-schemas/schema.json @@ -216,7 +216,7 @@ "items": { "type": "string" }, - "description": "Optional. Metadata for DIAL Core that defines the allowed tool names. When specified, DIAL Core filters the tools/list response from the MCP server to only include tools whose names match this list. When omitted, DIAL Core proxies the tools/list response from the MCP server without filtering." + "description": "Optional. Metadata for DIAL Core that defines the allowed tool names. When specified, DIAL Core filters the tools/list response from the MCP server to only include tools whose names match this list. When omitted, DIAL Core proxies the tools/list response from the MCP server without filtering. If an application instance using this type also specifies its own mcp.allowedTools, the effective allowed tools are the intersection of the two lists (the instance can only narrow this list, never widen it). A no-overlap override is meaningless and is never applied: it is rejected when the application is written through the applications API, and ignored with a warning for an application defined in the static config file, so that this list stays in effect." }, "dial:mcpConfigDelivery": { "type": "string", diff --git a/docs/dynamic-settings/applications.md b/docs/dynamic-settings/applications.md index bed44724f..0c9311401 100644 --- a/docs/dynamic-settings/applications.md +++ b/docs/dynamic-settings/applications.md @@ -259,6 +259,8 @@ Supported configuration parameters: } ``` +> **Note**: For a schema-rich application (`applicationTypeSchemaId` is set), `endpoint`, `transport`, `configDelivery`, and `forwardPerRequestKey` are always taken from the application type's `dial:applicationTypeMcp` schema, per the Effective Parameter Rule above. `allowedTools` is the one exception: if the application instance also specifies its own `mcp.allowedTools`, the effective allowed tools are the **intersection** of the instance's list and the application type's list — an instance can only narrow the type's allowed tools, never widen them. If the type's `allowedTools` is empty (unrestricted), the instance's list is used as-is. A no-overlap override is meaningless and is never applied: requests through the applications API are rejected with `400`, and for an application defined in the static config file the override is ignored with a warning so that the application type's own (restricting) list stays in effect. + #### applications..routes > **Effective Parameter Rule**: When `applicationTypeSchemaId` and `applicationProperties` are specified, parameters defined in the corresponding schema will take precedence and will override the corresponding parameters specified in the `application` object. diff --git a/docs/open_api_core.yaml b/docs/open_api_core.yaml index f026251cd..e4c07103c 100644 --- a/docs/open_api_core.yaml +++ b/docs/open_api_core.yaml @@ -3508,7 +3508,7 @@ paths: If you do not provide `application_type_schema_id`, refer to [DIAL Core](https://github.com/epam/ai-dial-core/blob/development/docs/dynamic-settings/applications.md) documentation to learn about available properties of applications you can pass in the JSON object describing the structure of the application. - **Note**: When `applicationTypeSchemaId` and `applicationProperties` are specified, parameters defined in the corresponding JSON schema will take precedence and will override the corresponding parameters specified in the `application` object. + **Note**: When `applicationTypeSchemaId` and `applicationProperties` are specified, parameters defined in the corresponding JSON schema will take precedence and will override the corresponding parameters specified in the `application` object. The exception is `mcp.allowedTools`: if the `application` object also specifies its own `mcp.allowedTools`, the effective allowed tools are the intersection of the two lists — the `application` object can only narrow the schema's allowed tools, never widen them. A no-overlap override is meaningless and is never applied: the request is rejected with a bad request status rather than falling back to a wider set of tools than was asked for. content: application/json: schema: @@ -13692,6 +13692,8 @@ components: type: array items: type: string + description: | + A list of allowed MCP tool names. When the application also has an `applicationTypeSchemaId`, this list is intersected with the application type's `allowedTools` — it can only narrow the type's list, never widen it. A no-overlap override is meaningless and is never applied: the request is rejected rather than falling back to a wider set of tools than was asked for. configDelivery: $ref: "#/components/schemas/ApplicationMcpConfigDelivery" endpoint: diff --git a/server/src/main/java/com/epam/aidial/core/server/service/ApplicationSchemaService.java b/server/src/main/java/com/epam/aidial/core/server/service/ApplicationSchemaService.java index 606e82d35..504fc1821 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/ApplicationSchemaService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/ApplicationSchemaService.java @@ -492,11 +492,31 @@ public Application.Mcp getMcp(Application application) { return null; } JsonNode schemaNode = ProxyUtil.MAPPER.readTree(customApplicationSchema); - JsonNode mcp = schemaNode.get(DIAL_APPLICATION_TYPE_MCP); - if (mcp == null) { + JsonNode mcpNode = schemaNode.get(DIAL_APPLICATION_TYPE_MCP); + if (mcpNode == null) { return null; } - return ProxyUtil.MAPPER.treeToValue(mcp, APP_MCP_TYPE_REF); + Application.Mcp mcp = ProxyUtil.MAPPER.treeToValue(mcpNode, APP_MCP_TYPE_REF); + + // an instance-level allowedTools can only narrow the application type's allowedTools, never widen it + Application.Mcp instanceMcp = application.getMcp(); + if (instanceMcp != null && !instanceMcp.getAllowedTools().isEmpty()) { + List typeAllowedTools = mcp.getAllowedTools(); + List effectiveAllowedTools = typeAllowedTools.isEmpty() + ? instanceMcp.getAllowedTools() + : instanceMcp.getAllowedTools().stream() + .filter(typeAllowedTools::contains) + .toList(); + // an empty list means "unrestricted" downstream, so a no-overlap override must not be applied: + // keeping the application type's list restricts the app instead of exposing every tool + if (effectiveAllowedTools.isEmpty()) { + log.warn("Ignoring mcp.allowedTools of application {}: no overlap with the application type's allowedTools", + application.getName()); + } else { + mcp.setAllowedTools(effectiveAllowedTools); + } + } + return mcp; } @SneakyThrows diff --git a/server/src/main/java/com/epam/aidial/core/server/service/ApplicationService.java b/server/src/main/java/com/epam/aidial/core/server/service/ApplicationService.java index dd4fbbdd8..39d94309a 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/ApplicationService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/ApplicationService.java @@ -585,12 +585,26 @@ private void prepareApplication(ResourceDescriptor resource, Application applica } URI applicationSchemaId = application.getApplicationTypeSchemaId(); if (applicationSchemaId != null) { - if (application.getEndpoint() != null || application.getFunction() != null || application.getMcp() != null) { - throw new IllegalArgumentException("Neither application endpoint, MCP or function must be set for schema based application"); + if (application.getEndpoint() != null || application.getFunction() != null + || (application.getMcp() != null && application.getMcp().getEndpoint() != null)) { + throw new IllegalArgumentException( + "Neither application endpoint, MCP endpoint or function must be set for schema based application"); } if (configStore.get().getCustomApplicationSchema(applicationSchemaId) == null) { throw new IllegalArgumentException("Application schema is not found by schema id: " + applicationSchemaId); } + // an instance-level allowedTools may only narrow the application type's allowedTools, so a + // no-overlap override is meaningless: getMcp ignores it and keeps the type's list, which + // would silently apply a wider set of tools than asked for. Reject it up front instead. + Application.Mcp instanceMcp = application.getMcp(); + if (instanceMcp != null && !instanceMcp.getAllowedTools().isEmpty()) { + Application.Mcp effectiveMcp = applicationSchemaService.getMcp(application); + if (effectiveMcp != null + && effectiveMcp.getAllowedTools().stream().noneMatch(instanceMcp.getAllowedTools()::contains)) { + throw new IllegalArgumentException( + "mcp.allowedTools has no overlap with the application type's allowed tools"); + } + } } else if (application.getEndpoint() == null && application.getFunction() == null && (application.getMcp() == null || application.getMcp().getEndpoint() == null)) { throw new IllegalArgumentException("At least application endpoint, MCP endpoint or function must be provided"); @@ -663,7 +677,9 @@ private void prepareApplication(ResourceDescriptor resource, Application applica } Application.Mcp mcp = application.getMcp(); - if (mcp != null) { + // a schema-rich application takes its MCP endpoint from the application type schema, so its own + // mcp block may legitimately carry only allowedTools to narrow the type's allowed tools + if (mcp != null && applicationSchemaId == null) { if (mcp.getEndpoint() == null) { throw new IllegalArgumentException("MCP endpoint must be provided"); } diff --git a/server/src/test/java/com/epam/aidial/core/server/CustomApplicationApiTest.java b/server/src/test/java/com/epam/aidial/core/server/CustomApplicationApiTest.java index 6558b806a..0d13f0417 100644 --- a/server/src/test/java/com/epam/aidial/core/server/CustomApplicationApiTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/CustomApplicationApiTest.java @@ -10,7 +10,9 @@ import com.epam.aidial.core.server.util.ResourceDescriptorFactory; import com.epam.aidial.core.storage.resource.ResourceDescriptor; import com.epam.aidial.core.storage.util.EtagHeader; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; import io.vertx.core.http.HttpMethod; import io.vertx.core.json.JsonObject; import okhttp3.mockwebserver.MockResponse; @@ -1773,6 +1775,179 @@ void testMcpCall() { } } + @Test + void testMcpCall_InstanceAllowedToolsNarrowsTypeAllowedTools() throws JsonProcessingException { + // "specific_toolset_type" application type has dial:allowedTools: ["classify_text", "extract_text"]. + // The instance below narrows to just "classify_text" (a genuine subset), so the effective + // allowed tools must be ["classify_text"], excluding "extract_text" even though the type allows it. + var response = send(HttpMethod.PUT, "/v1/applications/3CcedGxCx23EwiVbVmscVktScRyf46KypuBQ65miviST/my%20app", null, """ + { + "display_name": "My App", + "display_version": "1.0", + "icon_url": "http://apprunner/icon.svg", + "description": "My app Description", + "applicationTypeSchemaId": "https://mydial.somewhere.com/custom_application_schemas/specific_toolset_type", + "applicationProperties": { + "property1": "foo", + "property2": "bar" + }, + "mcp": { + "allowedTools": ["classify_text"] + } + } + """); + verify(response, 200); + + String mcpRequest = """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "tools/list", + "params": {} + } + """; + String mcpResponse = """ + { + "jsonrpc": "2.0", + "id": 1, + "result": { + "tools": [ + { + "name": "classify_text", + "title": "Classify text" + }, + { + "name": "extract_text", + "title": "Extract text" + } + ] + } + } + """; + TestWebServer.Handler handler = request -> + new MockResponse().setBody(mcpResponse).setHeader("Content-Type", "application/json"); + try (TestWebServer ignore = new TestWebServer(9876, handler)) { + Response resp = send(HttpMethod.POST, "/v1/deployments/applications/3CcedGxCx23EwiVbVmscVktScRyf46KypuBQ65miviST/my%20app/mcp", + null, mcpRequest, "Content-type", "application/json"); + + assertEquals(200, resp.status()); + JsonNode json = ProxyUtil.MAPPER.readTree(resp.body()); + ArrayNode tools = (ArrayNode) json.get("result").get("tools"); + assertEquals(1, tools.size()); + assertEquals("classify_text", tools.get(0).get("name").asText()); + } + + String toolCallRequest = """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": { + "name": "extract_text", + "arguments": {} + } + } + """; + try (TestWebServer ignore = new TestWebServer(9876, + request -> new MockResponse().setBody(mcpResponse).setHeader("Content-Type", "application/json"))) { + Response resp = send(HttpMethod.POST, "/v1/deployments/applications/3CcedGxCx23EwiVbVmscVktScRyf46KypuBQ65miviST/my%20app/mcp", + null, toolCallRequest, "Content-type", "application/json"); + + // "extract_text" is allowed by the type schema but excluded by the instance-level narrowing + assertEquals(403, resp.status()); + } + } + + @Test + void testCreateSchemaRichApplication_McpAllowedToolsDisjointFromType_IsRejected() { + // The type's allowedTools is ["classify_text", "extract_text"]; an instance-level allowedTools + // with no overlap at all must be rejected at write time, since downstream MCP tool filtering + // treats an empty allowedTools list as "unrestricted" and could otherwise silently expose every tool. + var response = send(HttpMethod.PUT, "/v1/applications/3CcedGxCx23EwiVbVmscVktScRyf46KypuBQ65miviST/my%20app", null, """ + { + "display_name": "My App", + "display_version": "1.0", + "icon_url": "http://apprunner/icon.svg", + "description": "My app Description", + "applicationTypeSchemaId": "https://mydial.somewhere.com/custom_application_schemas/specific_toolset_type", + "applicationProperties": { + "property1": "foo", + "property2": "bar" + }, + "mcp": { + "allowedTools": ["other_tool"] + } + } + """); + assertEquals(400, response.status()); + } + + @Test + void testCreateSchemaRichApplication_McpAllowedToolsOnly_IsAccepted() { + var response = send(HttpMethod.PUT, "/v1/applications/3CcedGxCx23EwiVbVmscVktScRyf46KypuBQ65miviST/my%20app", null, """ + { + "display_name": "My App", + "display_version": "1.0", + "icon_url": "http://apprunner/icon.svg", + "description": "My app Description", + "applicationTypeSchemaId": "https://mydial.somewhere.com/custom_application_schemas/specific_toolset_type", + "applicationProperties": { + "property1": "foo", + "property2": "bar" + }, + "mcp": { + "allowedTools": ["classify_text"] + } + } + """); + verify(response, 200); + } + + @Test + void testCreateSchemaRichApplication_EmptyMcpAllowedTools_IsAccepted() { + // an empty allowedTools list means "no instance-level restriction" and must be accepted; + // the application type's own allowedTools then applies unchanged + var response = send(HttpMethod.PUT, "/v1/applications/3CcedGxCx23EwiVbVmscVktScRyf46KypuBQ65miviST/my%20app", null, """ + { + "display_name": "My App", + "display_version": "1.0", + "icon_url": "http://apprunner/icon.svg", + "description": "My app Description", + "applicationTypeSchemaId": "https://mydial.somewhere.com/custom_application_schemas/specific_toolset_type", + "applicationProperties": { + "property1": "foo", + "property2": "bar" + }, + "mcp": { + "allowedTools": [] + } + } + """); + verify(response, 200); + } + + @Test + void testCreateSchemaRichApplication_McpEndpoint_IsRejected() { + var response = send(HttpMethod.PUT, "/v1/applications/3CcedGxCx23EwiVbVmscVktScRyf46KypuBQ65miviST/my%20app", null, """ + { + "display_name": "My App", + "display_version": "1.0", + "icon_url": "http://apprunner/icon.svg", + "description": "My app Description", + "applicationTypeSchemaId": "https://mydial.somewhere.com/custom_application_schemas/specific_toolset_type", + "applicationProperties": { + "property1": "foo", + "property2": "bar" + }, + "mcp": { + "endpoint": "http://localhost:9999/mcp", + "allowedTools": ["classify_text"] + } + } + """); + assertEquals(400, response.status()); + } + private HttpUriRequest createHttpUriRequest(int port, String deployment, String apiKey) { String uri = "http://127.0.0.1:" + port + "/openai/deployments/" + deployment + "/chat/completions"; diff --git a/server/src/test/java/com/epam/aidial/core/server/service/ApplicationSchemaServiceTest.java b/server/src/test/java/com/epam/aidial/core/server/service/ApplicationSchemaServiceTest.java index d506da2cf..9f99c8283 100644 --- a/server/src/test/java/com/epam/aidial/core/server/service/ApplicationSchemaServiceTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/service/ApplicationSchemaServiceTest.java @@ -302,6 +302,135 @@ public void getCustomApplicationSchemaOrThrow_returnsNull_whenSchemaIdIsNull() { Assertions.assertNull(result); } + @Test + void getMcp_returnsTypeAllowedTools_whenInstanceHasNoOverride() { + when(configStore.get()).thenReturn(config); + URI schemaId = URI.create("schemaId"); + application.setApplicationTypeSchemaId(schemaId); + when(config.getCustomApplicationSchema(schemaId)).thenReturn(""" + { + "dial:applicationTypeMcp": { + "dial:endpoint": "http://localhost:9876/mcp", + "dial:transport": "HTTP", + "dial:allowedTools": ["classify_text"] + } + } + """); + + Application.Mcp mcp = service.getMcp(application); + + Assertions.assertEquals(List.of("classify_text"), mcp.getAllowedTools()); + } + + @Test + void getMcp_narrowsAllowedTools_whenInstanceOverridePartiallyOverlapsType() { + when(configStore.get()).thenReturn(config); + URI schemaId = URI.create("schemaId"); + application.setApplicationTypeSchemaId(schemaId); + when(config.getCustomApplicationSchema(schemaId)).thenReturn(""" + { + "dial:applicationTypeMcp": { + "dial:endpoint": "http://localhost:9876/mcp", + "dial:transport": "HTTP", + "dial:allowedTools": ["classify_text", "extract_text"] + } + } + """); + Application.Mcp instanceMcp = new Application.Mcp(); + instanceMcp.setAllowedTools(List.of("extract_text", "summarize_text")); + application.setMcp(instanceMcp); + + Application.Mcp mcp = service.getMcp(application); + + Assertions.assertEquals(List.of("extract_text"), mcp.getAllowedTools()); + } + + @Test + void getMcp_keepsTypeAllowedTools_whenInstanceOverrideIsDisjointFromType() { + // an empty effective list would mean "unrestricted" downstream, so a no-overlap override + // must be ignored in favour of the application type's own (restricting) list + when(configStore.get()).thenReturn(config); + URI schemaId = URI.create("schemaId"); + application.setApplicationTypeSchemaId(schemaId); + when(config.getCustomApplicationSchema(schemaId)).thenReturn(""" + { + "dial:applicationTypeMcp": { + "dial:endpoint": "http://localhost:9876/mcp", + "dial:transport": "HTTP", + "dial:allowedTools": ["classify_text", "extract_text"] + } + } + """); + Application.Mcp instanceMcp = new Application.Mcp(); + instanceMcp.setAllowedTools(List.of("other_tool")); + application.setMcp(instanceMcp); + + Application.Mcp mcp = service.getMcp(application); + + Assertions.assertEquals(List.of("classify_text", "extract_text"), mcp.getAllowedTools()); + } + + @Test + void getMcp_returnsTypeAllowedTools_whenInstanceOverrideIsEmpty() { + when(configStore.get()).thenReturn(config); + URI schemaId = URI.create("schemaId"); + application.setApplicationTypeSchemaId(schemaId); + when(config.getCustomApplicationSchema(schemaId)).thenReturn(""" + { + "dial:applicationTypeMcp": { + "dial:endpoint": "http://localhost:9876/mcp", + "dial:transport": "HTTP", + "dial:allowedTools": ["classify_text"] + } + } + """); + application.setMcp(new Application.Mcp()); + + Application.Mcp mcp = service.getMcp(application); + + Assertions.assertEquals(List.of("classify_text"), mcp.getAllowedTools()); + } + + @Test + void getMcp_returnsNull_whenTypeHasNoMcpBlock() { + when(configStore.get()).thenReturn(config); + URI schemaId = URI.create("schemaId"); + application.setApplicationTypeSchemaId(schemaId); + when(config.getCustomApplicationSchema(schemaId)).thenReturn(""" + { + "dial:applicationTypeDisplayName": "No MCP Type" + } + """); + Application.Mcp instanceMcp = new Application.Mcp(); + instanceMcp.setAllowedTools(List.of("classify_text")); + application.setMcp(instanceMcp); + + Assertions.assertNull(service.getMcp(application)); + } + + @Test + void getMcp_usesInstanceAllowedTools_whenTypeIsUnrestricted() { + when(configStore.get()).thenReturn(config); + URI schemaId = URI.create("schemaId"); + application.setApplicationTypeSchemaId(schemaId); + when(config.getCustomApplicationSchema(schemaId)).thenReturn(""" + { + "dial:applicationTypeMcp": { + "dial:endpoint": "http://localhost:9876/mcp", + "dial:transport": "HTTP", + "dial:allowedTools": [] + } + } + """); + Application.Mcp instanceMcp = new Application.Mcp(); + instanceMcp.setAllowedTools(List.of("classify_text")); + application.setMcp(instanceMcp); + + Application.Mcp mcp = service.getMcp(application); + + Assertions.assertEquals(List.of("classify_text"), mcp.getAllowedTools()); + } + @Test void consumeMetadataProperties_returnsProperties_whenSchemaExists() { when(configStore.get()).thenReturn(config); diff --git a/server/src/test/resources/aidial.config.json b/server/src/test/resources/aidial.config.json index 20f678586..197da338b 100644 --- a/server/src/test/resources/aidial.config.json +++ b/server/src/test/resources/aidial.config.json @@ -533,7 +533,7 @@ "dial:applicationTypeMcp": { "dial:endpoint": "http://localhost:9876/mcp", "dial:transport": "HTTP", - "dial:allowedTools": ["classify_text"], + "dial:allowedTools": ["classify_text", "extract_text"], "dial:mcpConfigDelivery": "META", "dial:forwardPerRequestKey": true },