diff --git a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsTest.kt b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsTest.kt index c2e82015..54c998e9 100644 --- a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsTest.kt +++ b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerToolsTest.kt @@ -98,6 +98,16 @@ class ServerToolsTest : AbstractServerFeaturesTest() { assertFalse(toolListChangedNotificationReceived, "No notification should be sent when tool doesn't exist") } + @Test + fun `addTool should succeed with non-conforming tool name`() = runTest { + server.addTool("my invalid tool!", "Tool with non-conforming name", ToolSchema()) { + CallToolResult(listOf(TextContent("It works"))) + } + + val result = server.removeTool("my invalid tool!") + assertTrue(result, "Tool with non-conforming name should have been registered and removable") + } + @Test fun `removeTool should throw when tools capability is not supported`() = runTest { var toolListChangedNotificationReceived = false diff --git a/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt b/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt index de219d4e..d9b9b9a7 100644 --- a/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt +++ b/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt @@ -1,6 +1,7 @@ package io.modelcontextprotocol.kotlin.sdk.server import io.github.oshai.kotlinlogging.KotlinLogging +import io.modelcontextprotocol.kotlin.sdk.server.utils.warnIfInvalidToolName import io.modelcontextprotocol.kotlin.sdk.shared.ProtocolOptions import io.modelcontextprotocol.kotlin.sdk.shared.RequestOptions import io.modelcontextprotocol.kotlin.sdk.shared.Transport @@ -303,7 +304,7 @@ public open class Server( logger.error { "Failed to add tool '${tool.name}': Server does not support tools capability" } "Server does not support tools capability. Enable it in ServerOptions." } - + warnIfInvalidToolName(tool.name) toolRegistry.add(RegisteredTool(tool, handler)) } @@ -357,6 +358,7 @@ public open class Server( logger.error { "Failed to add tools: Server does not support tools capability" } "Server does not support tools capability." } + toolsToAdd.forEach { warnIfInvalidToolName(it.tool.name) } toolRegistry.addAll(toolsToAdd) } diff --git a/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/utils/ToolNameValidation.kt b/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/utils/ToolNameValidation.kt new file mode 100644 index 00000000..a63c2e45 --- /dev/null +++ b/kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/utils/ToolNameValidation.kt @@ -0,0 +1,60 @@ +package io.modelcontextprotocol.kotlin.sdk.server.utils + +import io.github.oshai.kotlinlogging.KotlinLogging + +private val logger = KotlinLogging.logger("ToolNameValidation") + +private const val MAX_TOOL_NAME_LENGTH = 128 + +private const val SEP_986_URL = "https://github.com/modelcontextprotocol/modelcontextprotocol/issues/986" + +/** + * Validates a tool name against the MCP tool naming standard (SEP-986). + * Returns a list of warnings. An empty list means the name is valid. + */ +internal fun validateToolName(name: String): List { + if (name.isEmpty()) return listOf("Tool name cannot be empty") + + val invalidChars = name.toSet().filterNot { it.isAsciiAllowed() } + val remainingInvalidChars = invalidChars - setOf(' ', ',') + + return listOfNotNull( + "Tool name exceeds maximum length of $MAX_TOOL_NAME_LENGTH characters (current: ${name.length})" + .takeIf { name.length > MAX_TOOL_NAME_LENGTH }, + "Tool name contains spaces, which may cause parsing issues" + .takeIf { ' ' in invalidChars }, + "Tool name contains commas, which may cause parsing issues" + .takeIf { ',' in invalidChars }, + "Tool name contains invalid characters: ${remainingInvalidChars.joinToString(", ") { "\"$it\"" }}" + .takeIf { remainingInvalidChars.isNotEmpty() }, + "Allowed characters are: A-Z, a-z, 0-9, underscore (_), dash (-), and dot (.)" + .takeIf { invalidChars.isNotEmpty() }, + "Tool name starts or ends with a dash, which may cause parsing issues in some contexts" + .takeIf { name.startsWith('-') || name.endsWith('-') }, + "Tool name starts or ends with a dot, which may cause parsing issues in some contexts" + .takeIf { name.startsWith('.') || name.endsWith('.') }, + ) +} + +/** + * Validates the tool [name] and logs a warning if it does not conform to SEP-986. + * Does not block registration — tools with non-conforming names are still accepted. + */ +internal fun warnIfInvalidToolName(name: String) { + val warnings = validateToolName(name) + if (warnings.isEmpty()) return + + val warningLines = warnings.joinToString("\n") { " - $it" } + logger.warn { + """ + |Tool name validation warning for "$name": + |$warningLines + |Tool registration will proceed, but this may cause compatibility issues. + |Consider updating the tool name to conform to the MCP tool naming standard. + |See SEP-986: $SEP_986_URL + """.trimMargin() + } +} + +private fun Char.isAsciiAllowed(): Boolean = + this in 'a'..'z' || this in 'A'..'Z' || this in '0'..'9' || this == '_' || this == '-' || this == '.' diff --git a/kotlin-sdk-server/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/utils/ToolNameValidationTest.kt b/kotlin-sdk-server/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/utils/ToolNameValidationTest.kt new file mode 100644 index 00000000..f6760eea --- /dev/null +++ b/kotlin-sdk-server/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/utils/ToolNameValidationTest.kt @@ -0,0 +1,123 @@ +package io.modelcontextprotocol.kotlin.sdk.server.utils + +import io.kotest.matchers.booleans.shouldBeTrue +import io.kotest.matchers.collections.shouldBeEmpty +import io.kotest.matchers.collections.shouldContain +import io.kotest.matchers.collections.shouldHaveSize +import io.kotest.matchers.collections.shouldNotBeEmpty +import org.junit.jupiter.api.Test +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource + +class ToolNameValidationTest { + + @ParameterizedTest + @ValueSource( + strings = [ + "getUser", + "get_user_profile", + "user-profile-update", + "admin.tools.list", + "DATA_EXPORT_v2.1", + "a", + "Z", + "0", + "a-b.c_d", + ], + ) + fun `should return no warnings for valid tool names`(name: String) { + validateToolName(name).shouldBeEmpty() + } + + @Test + fun `should return no warnings for max length name`() { + validateToolName("a".repeat(128)).shouldBeEmpty() + } + + @Test + fun `should warn for empty name`() { + val warnings = validateToolName("") + warnings shouldHaveSize 1 + warnings shouldContain "Tool name cannot be empty" + } + + @Test + fun `should warn for name exceeding max length`() { + val warnings = validateToolName("a".repeat(129)) + warnings.shouldNotBeEmpty() + warnings.any { "exceeds maximum length" in it }.shouldBeTrue() + } + + @ParameterizedTest + @ValueSource(strings = ["get user profile", "my tool"]) + fun `should warn for names with spaces`(name: String) { + val warnings = validateToolName(name) + warnings shouldContain "Tool name contains spaces, which may cause parsing issues" + } + + @ParameterizedTest + @ValueSource(strings = ["get,user,profile", "a,b"]) + fun `should warn for names with commas`(name: String) { + val warnings = validateToolName(name) + warnings shouldContain "Tool name contains commas, which may cause parsing issues" + } + + @ParameterizedTest + @ValueSource(strings = ["user/profile/update", "a/b"]) + fun `should warn for names with forward slashes`(name: String) { + val warnings = validateToolName(name) + warnings.any { "invalid characters" in it }.shouldBeTrue() + } + + @ParameterizedTest + @ValueSource(strings = ["user@domain.com", "tool#1", "tool\$name"]) + fun `should warn for names with special characters`(name: String) { + val warnings = validateToolName(name) + warnings.any { "invalid characters" in it }.shouldBeTrue() + warnings shouldContain "Allowed characters are: A-Z, a-z, 0-9, underscore (_), dash (-), and dot (.)" + } + + @Test + fun `should warn for unicode characters`() { + val warnings = validateToolName("user-\u00f1ame") + warnings.any { "invalid characters" in it }.shouldBeTrue() + } + + @Test + fun `should warn for name starting with dash`() { + val warnings = validateToolName("-my-tool") + warnings shouldContain "Tool name starts or ends with a dash, which may cause parsing issues in some contexts" + } + + @Test + fun `should warn for name ending with dash`() { + val warnings = validateToolName("my-tool-") + warnings shouldContain "Tool name starts or ends with a dash, which may cause parsing issues in some contexts" + } + + @Test + fun `should warn for name starting with dot`() { + val warnings = validateToolName(".hidden") + warnings shouldContain "Tool name starts or ends with a dot, which may cause parsing issues in some contexts" + } + + @Test + fun `should warn for name ending with dot`() { + val warnings = validateToolName("config.") + warnings shouldContain "Tool name starts or ends with a dot, which may cause parsing issues in some contexts" + } + + @Test + fun `should produce multiple warnings for multiple issues`() { + val warnings = validateToolName("my tool,name") + warnings.any { "spaces" in it }.shouldBeTrue() + warnings.any { "commas" in it }.shouldBeTrue() + } + + @Test + fun `should not duplicate space or comma in generic invalid characters message`() { + val warnings = validateToolName("my tool") + warnings shouldContain "Tool name contains spaces, which may cause parsing issues" + warnings.none { it.startsWith("Tool name contains invalid characters") }.shouldBeTrue() + } +}