feat(server): add tool name validation per SEP-986 (#695)
Add warn-only validation for tool names at registration time, following the MCP tool naming standard (SEP-986) closes #417 ## How Has This Been Tested? unit tests ## Breaking Changes none ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [x] I have added appropriate error handling - [x] I have added or updated documentation as needed
This commit is contained in:
parent
61fab2ffe0
commit
e74ef2db6f
3 changed files with 196 additions and 1 deletions
|
|
@ -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)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<String> {
|
||||
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 == '.'
|
||||
|
|
@ -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()
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue