This is an automated email from the ASF dual-hosted git repository.
Aias00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new bb844d6132 fix(mcp): keep URL-valued tool arguments in their
configured positions (#7576)
bb844d6132 is described below
commit bb844d613251233ca243aadc6ec3cf0e12e452d2
Author: Carlos Acosta <[email protected]>
AuthorDate: Mon Oct 12 21:57:20 2026 -0500
fix(mcp): keep URL-valued tool arguments in their configured positions
(#7576)
* fix(mcp): restrict URL override to explicit target mapping
Fixes apache/shenyu#7453
* test(mcp): preserve query mapping with explicit URL targets
Document explicit target mapping and assert callback URL data placement.
---
.../MCP_TOOL_EXAMPLES_EN.md | 3 +
.../mcp/server/request/RequestConfigHelper.java | 17 +++---
.../server/callback/ShenyuToolCallbackTest.java | 66 ++++++++++++++++++++++
.../server/request/RequestConfigHelperTest.java | 53 +++++++++++++++++
4 files changed, 130 insertions(+), 9 deletions(-)
diff --git a/shenyu-plugin/shenyu-plugin-mcp-server/MCP_TOOL_EXAMPLES_EN.md
b/shenyu-plugin/shenyu-plugin-mcp-server/MCP_TOOL_EXAMPLES_EN.md
index 933e8c833f..becf3312e4 100644
--- a/shenyu-plugin/shenyu-plugin-mcp-server/MCP_TOOL_EXAMPLES_EN.md
+++ b/shenyu-plugin/shenyu-plugin-mcp-server/MCP_TOOL_EXAMPLES_EN.md
@@ -429,6 +429,8 @@ This plugin is developed and tested with the following SDK
versions:
1. **URL Path Parameters**: `"path"` - Corresponds to `{{.paramName}}` in URL
template
2. **Query Parameters**: `"query"` - Corresponds to parameters in
`queryParams` array
3. **Request Body**: `"body"` - Used with `argsToJsonBody: true`
+4. **Request Headers**: `"header"` - Use `{{.paramName}}` in a configured
header value
+5. **Complete URL Target**: `"target"` - Only for a tool explicitly configured
to accept a complete `http://` or `https://` URL as the request target.
Ordinary callback/resource URLs must keep their `query`, `body`, or `header`
mapping.
### Mapping Examples:
```json
@@ -492,6 +494,7 @@ This plugin is developed and tested with the following SDK
versions:
5. **Query Parameters**: Configure using `queryParams` array with
`${paramName}` format values
6. **Path Parameters**: Use `{{.paramName}}` format in URL
7. **Parameter Mapping**: Use `argsPosition` to specify parameter location:
`path`, `query`, `body`, `header`
+8. **Explicit Complete URL**: To intentionally replace the configured request
target, use `"argsPosition": {"endpoint": "target"}` with a trusted URL input.
Do not grant arbitrary target control to untrusted callers.
## Best Practices
diff --git
a/shenyu-plugin/shenyu-plugin-mcp-server/src/main/java/org/apache/shenyu/plugin/mcp/server/request/RequestConfigHelper.java
b/shenyu-plugin/shenyu-plugin-mcp-server/src/main/java/org/apache/shenyu/plugin/mcp/server/request/RequestConfigHelper.java
index 2db1573dcd..86742d8f8c 100644
---
a/shenyu-plugin/shenyu-plugin-mcp-server/src/main/java/org/apache/shenyu/plugin/mcp/server/request/RequestConfigHelper.java
+++
b/shenyu-plugin/shenyu-plugin-mcp-server/src/main/java/org/apache/shenyu/plugin/mcp/server/request/RequestConfigHelper.java
@@ -127,17 +127,16 @@ public class RequestConfigHelper {
*/
public static String buildPath(final String urlTemplate, final JsonObject
argsPosition,
final JsonObject inputJson) {
- // First, check if the input value is already a complete URL
- if (isCompleteUrl(argsPosition, inputJson)) {
- return getCompleteUrl(argsPosition, inputJson);
- }
+ // Only an explicitly target-mapped argument may replace the
configured endpoint.
+ String requestUrl = isCompleteUrl(argsPosition, inputJson)
+ ? getCompleteUrl(argsPosition, inputJson) : urlTemplate;
StringBuilder queryBuilder = new StringBuilder();
// Check whether the URL template already contains the query parameters
- boolean hasExistingQuery = urlTemplate.contains("?");
- String basePath = hasExistingQuery ? urlTemplate.substring(0,
urlTemplate.indexOf("?")) : urlTemplate;
- String existingQuery = hasExistingQuery ?
urlTemplate.substring(urlTemplate.indexOf("?") + 1) : "";
+ boolean hasExistingQuery = requestUrl.contains("?");
+ String basePath = hasExistingQuery ? requestUrl.substring(0,
requestUrl.indexOf("?")) : requestUrl;
+ String existingQuery = hasExistingQuery ?
requestUrl.substring(requestUrl.indexOf("?") + 1) : "";
// Handle new query parameters
basePath = processArguments(argsPosition, inputJson, basePath,
queryBuilder, existingQuery);
@@ -158,7 +157,7 @@ public class RequestConfigHelper {
*/
private static boolean isCompleteUrl(final JsonObject argsPosition, final
JsonObject inputJson) {
for (String key : argsPosition.keySet()) {
- if (inputJson.has(key)) {
+ if ("target".equals(argsPosition.get(key).getAsString()) &&
inputJson.has(key)) {
try {
String value = inputJson.get(key).getAsString();
if (value.startsWith("http://";) ||
value.startsWith("https://";)) {
@@ -181,7 +180,7 @@ public class RequestConfigHelper {
*/
private static String getCompleteUrl(final JsonObject argsPosition, final
JsonObject inputJson) {
for (String key : argsPosition.keySet()) {
- if (inputJson.has(key)) {
+ if ("target".equals(argsPosition.get(key).getAsString()) &&
inputJson.has(key)) {
try {
String value = inputJson.get(key).getAsString();
if (value.startsWith("http://";) ||
value.startsWith("https://";)) {
diff --git
a/shenyu-plugin/shenyu-plugin-mcp-server/src/test/java/org/apache/shenyu/plugin/mcp/server/callback/ShenyuToolCallbackTest.java
b/shenyu-plugin/shenyu-plugin-mcp-server/src/test/java/org/apache/shenyu/plugin/mcp/server/callback/ShenyuToolCallbackTest.java
index 60fa36bc84..20f68ee0a1 100644
---
a/shenyu-plugin/shenyu-plugin-mcp-server/src/test/java/org/apache/shenyu/plugin/mcp/server/callback/ShenyuToolCallbackTest.java
+++
b/shenyu-plugin/shenyu-plugin-mcp-server/src/test/java/org/apache/shenyu/plugin/mcp/server/callback/ShenyuToolCallbackTest.java
@@ -17,6 +17,8 @@
package org.apache.shenyu.plugin.mcp.server.callback;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonParser;
import io.modelcontextprotocol.common.McpTransportContext;
import io.modelcontextprotocol.server.McpSyncServerExchange;
import org.apache.shenyu.common.constant.Constants;
@@ -35,6 +37,8 @@ import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.ai.chat.model.ToolContext;
+import org.springframework.core.io.buffer.DataBuffer;
+import org.springframework.core.io.buffer.DataBufferUtils;
import org.springframework.http.server.reactive.ServerHttpRequest;
import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
import org.springframework.mock.web.server.MockServerWebExchange;
@@ -43,6 +47,7 @@ import org.springframework.web.server.ServerWebExchange;
import reactor.core.publisher.Mono;
import java.net.URI;
+import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
@@ -363,6 +368,67 @@ class ShenyuToolCallbackTest {
assertEquals("/users", context.getRealUrl());
}
+ @Test
+ void testUrlValuedArgumentsPreserveToolEndpointAndContext() {
+ shenyuToolCallback = new ShenyuToolCallback(toolDefinition);
+ for (String position : new String[]{"query", "body", "body.callback",
"header"}) {
+ MockServerWebExchange originExchange = MockServerWebExchange.from(
+
MockServerHttpRequest.get("http://gateway.example:9195/mcp";).build());
+ ShenyuContext context = new ShenyuContext();
+ originExchange.getAttributes().put(Constants.CONTEXT, context);
+ String headerTemplate = "header".equals(position)
+ ?
",\"headers\":[{\"key\":\"X-Callback-URL\",\"value\":\"{{.callbackUrl}}\"}]" :
"";
+ String config =
"{\"requestTemplate\":{\"url\":\"/search\",\"method\":\"POST\",\"argsToJsonBody\":true"
+ + headerTemplate +
"},\"argsPosition\":{\"callbackUrl\":\"" + position + "\"}}";
+ String input =
"{\"callbackUrl\":\"https://callback.invalid/return\"}";;
+
+ ServerWebExchange decoratedExchange =
ReflectionTestUtils.invokeMethod(shenyuToolCallback, "buildDecoratedExchange",
+ originExchange, new CompletableFuture<String>(),
"session123", config, input);
+
+ assertEquals("/search",
decoratedExchange.getRequest().getPath().value(), position);
+ assertEquals("/search", context.getPath(), position);
+ assertEquals("/search", context.getRealUrl(), position);
+ if ("query".equals(position)) {
+ assertEquals("https://callback.invalid/return";,
+
decoratedExchange.getRequest().getQueryParams().getFirst("callbackUrl"));
+ }
+ if ("header".equals(position)) {
+ assertEquals("https://callback.invalid/return";,
+
decoratedExchange.getRequest().getHeaders().getFirst("X-Callback-URL"));
+ }
+ if (position.startsWith("body")) {
+ DataBuffer buffer =
DataBufferUtils.join(decoratedExchange.getRequest().getBody()).block();
+ assertNotNull(buffer);
+ try {
+ JsonObject body =
JsonParser.parseString(buffer.toString(StandardCharsets.UTF_8)).getAsJsonObject();
+ String bodyKey = "body".equals(position) ? "callbackUrl" :
"callback";
+ assertEquals("https://callback.invalid/return";,
body.get(bodyKey).getAsString());
+ } finally {
+ DataBufferUtils.release(buffer);
+ }
+ }
+ }
+ }
+
+ @Test
+ void testExplicitTargetMappingRoutesToCompleteUrl() {
+ shenyuToolCallback = new ShenyuToolCallback(toolDefinition);
+ MockServerWebExchange originExchange = MockServerWebExchange.from(
+
MockServerHttpRequest.get("http://gateway.example:9195/mcp";).build());
+ ShenyuContext context = new ShenyuContext();
+ originExchange.getAttributes().put(Constants.CONTEXT, context);
+ String config =
"{\"requestTemplate\":{\"url\":\"/search\",\"method\":\"GET\"},"
+ + "\"argsPosition\":{\"endpoint\":\"target\"}}";
+
+ ServerWebExchange decoratedExchange =
ReflectionTestUtils.invokeMethod(shenyuToolCallback, "buildDecoratedExchange",
+ originExchange, new CompletableFuture<String>(), "session123",
config,
+ "{\"endpoint\":\"https://target.example/api?fixed=1\"}";);
+
+ assertEquals(URI.create("https://target.example/api?fixed=1";),
decoratedExchange.getRequest().getURI());
+ assertEquals("/api", context.getPath());
+ assertEquals("/api", context.getRealUrl());
+ }
+
@Test
void testConstructorWithValidToolDefinition() {
ShenyuToolCallback callback = new ShenyuToolCallback(toolDefinition);
diff --git
a/shenyu-plugin/shenyu-plugin-mcp-server/src/test/java/org/apache/shenyu/plugin/mcp/server/request/RequestConfigHelperTest.java
b/shenyu-plugin/shenyu-plugin-mcp-server/src/test/java/org/apache/shenyu/plugin/mcp/server/request/RequestConfigHelperTest.java
index f1ae261150..c3a3afa7e5 100644
---
a/shenyu-plugin/shenyu-plugin-mcp-server/src/test/java/org/apache/shenyu/plugin/mcp/server/request/RequestConfigHelperTest.java
+++
b/shenyu-plugin/shenyu-plugin-mcp-server/src/test/java/org/apache/shenyu/plugin/mcp/server/request/RequestConfigHelperTest.java
@@ -251,4 +251,57 @@ class RequestConfigHelperTest {
assertEquals("/items/a%2Fb%20%23%3F+%25", result);
}
+
+ @Test
+ void testUrlValuedDataDoesNotReplaceFixedEndpoint() {
+ for (String position : new String[]{"query", "body", "body.callback",
"header", "path"}) {
+ JsonObject argsPosition = new JsonObject();
+ argsPosition.addProperty("callbackUrl", position);
+ JsonObject inputJson = new JsonObject();
+ inputJson.addProperty("callbackUrl",
"https://callback.invalid/return";);
+
+ String path = RequestConfigHelper.buildPath("/search",
argsPosition, inputJson);
+
+ if ("query".equals(position)) {
+ assertTrue(path.startsWith("/search?callbackUrl="), position);
+ assertTrue(path.contains("callback.invalid"), position);
+ } else {
+ assertEquals("/search", path, position);
+ }
+ }
+ }
+
+ @Test
+ void testExplicitTargetArgumentCanSelectFullUrl() {
+ JsonObject argsPosition = new JsonObject();
+ argsPosition.addProperty("endpoint", "target");
+ JsonObject inputJson = new JsonObject();
+ inputJson.addProperty("endpoint", "https://target.example/api";);
+
+ assertEquals("https://target.example/api";,
+ RequestConfigHelper.buildPath("/search", argsPosition,
inputJson));
+ }
+
+ @Test
+ void testUnmappedUrlValueDoesNotChangeEndpoint() {
+ JsonObject argsPosition = new JsonObject();
+ JsonObject inputJson = new JsonObject();
+ inputJson.addProperty("endpoint", "https://target.example/api";);
+
+ assertEquals("/search", RequestConfigHelper.buildPath("/search",
argsPosition, inputJson));
+ }
+
+ @Test
+ void testExplicitTargetStillAppliesMappedQuery() {
+ JsonObject positions = new JsonObject();
+ positions.addProperty("endpoint", "target");
+ positions.addProperty("page", "query");
+ JsonObject input = new JsonObject();
+ input.addProperty("endpoint", "https://target.example/api?fixed=1";);
+ input.addProperty("page", "2");
+
+ assertEquals("https://target.example/api?page=2&fixed=1";,
+ RequestConfigHelper.buildPath("/search", positions, input));
+ }
+
}
