shreemaan-abhishek commented on code in PR #14154:
URL: https://github.com/apache/apisix/pull/14154#discussion_r4477619917
##########
apisix/plugins/openapi-to-mcp/tools/handler.lua:
##########
@@ -124,19 +125,114 @@ local function apply_query_defaults(tool, query)
end
-local function build_path(template, path_params)
+-- A separator in a path parameter is percent-encoded, but servers disagree on
+-- whether to preserve it as data or decode it into path syntax. The secure
+-- default is to reject it; a Route may opt in for an API that deliberately
+-- accepts encoded separators.
+local function has_separator(value)
+ -- core.string.find is a plain search; this is a pattern, so it needs
+ -- string.find itself. A backslash is a separator on some servers.
+ return str_find(value, "[/\\]") ~= nil
+end
+
+
+-- Even with encoded separators enabled, refuse values whose components would
+-- be resolved away after a server decodes them. Empty components cover
leading,
+-- trailing and repeated separators. Both slash spellings are considered: a
+-- backslash is a separator on some servers.
+local function resolved_away(segment)
+ return segment == "" or segment == "." or segment == ".."
+end
+
+
+local function separator_creates_unsafe_component(value)
+ if not has_separator(value) then
+ return false
+ end
+
+ for component in str_gmatch(value .. "/", "([^/\\]*)[/\\]") do
+ if resolved_away(component) then
+ return true
+ end
+ end
+ return false
+end
+
+
+-- Return only the URI path. A template expression name may itself contain a
+-- question mark or hash, so only a delimiter outside braces starts the query
+-- or fragment. Parameter values cannot introduce one: escape_uri encodes it.
+local function uri_path(value)
+ local in_expression = false
+ for index = 1, #value do
+ local char = str_sub(value, index, index)
+ if char == "{" then
+ in_expression = true
+ elseif char == "}" then
+ in_expression = false
+ elseif not in_expression and (char == "?" or char == "#") then
+ return str_sub(value, 1, index - 1)
+ end
+ end
+ return value
+end
+
+
+-- The segments of a path, with the empty one a leading or trailing slash
+-- leaves: "/items/x" is { "", "items", "x" }.
+local function path_segments(value)
+ local segments = {}
+ local path = uri_path(value)
+ for segment in str_gmatch(path .. "/", "([^/]*)/") do
+ segments[#segments + 1] = segment
+ end
+ return segments
+end
+
+
+-- A server receiving the request resolves "." and ".." away, but only where
+-- one is a whole segment. A template expression may fill part of a segment,
+-- and "{name}.json" filled with ".." comes out as "...json", which addresses
+-- nothing else, so the value alone does not say whether it moves the request.
+-- The finished path does.
+--
+-- Only the segments an expression helped build are checked: one the template
+-- contributes by itself is not the caller's to shift. Encoded separators do
+-- not add a literal slash to the finished path, so the segment lists still
+-- line up by index when the Route opts in to them.
+local function build_path(template, path_params, allow_encoded_separators)
local path = template
for name, value in pairs(path_params) do
+ local text = tostring(value)
+ if has_separator(text) and not allow_encoded_separators then
Review Comment:
Addressed in e309d12f7. Both raw separator checks now run only when the
expression occurs in the actual URI path. Added coverage showing `a/b` remains
valid in a query expression with the secure default and `http://x/y` remains
valid with the separator opt-in.
##########
apisix/plugins/openapi-to-mcp/tools/handler.lua:
##########
@@ -124,19 +125,114 @@ local function apply_query_defaults(tool, query)
end
-local function build_path(template, path_params)
+-- A separator in a path parameter is percent-encoded, but servers disagree on
+-- whether to preserve it as data or decode it into path syntax. The secure
+-- default is to reject it; a Route may opt in for an API that deliberately
+-- accepts encoded separators.
+local function has_separator(value)
+ -- core.string.find is a plain search; this is a pattern, so it needs
+ -- string.find itself. A backslash is a separator on some servers.
+ return str_find(value, "[/\\]") ~= nil
+end
+
+
+-- Even with encoded separators enabled, refuse values whose components would
+-- be resolved away after a server decodes them. Empty components cover
leading,
+-- trailing and repeated separators. Both slash spellings are considered: a
+-- backslash is a separator on some servers.
+local function resolved_away(segment)
+ return segment == "" or segment == "." or segment == ".."
+end
+
+
+local function separator_creates_unsafe_component(value)
+ if not has_separator(value) then
+ return false
+ end
+
+ for component in str_gmatch(value .. "/", "([^/\\]*)[/\\]") do
+ if resolved_away(component) then
+ return true
+ end
+ end
+ return false
+end
+
+
+-- Return only the URI path. A template expression name may itself contain a
+-- question mark or hash, so only a delimiter outside braces starts the query
+-- or fragment. Parameter values cannot introduce one: escape_uri encodes it.
+local function uri_path(value)
+ local in_expression = false
+ for index = 1, #value do
+ local char = str_sub(value, index, index)
+ if char == "{" then
+ in_expression = true
+ elseif char == "}" then
+ in_expression = false
+ elseif not in_expression and (char == "?" or char == "#") then
Review Comment:
Addressed in e309d12f7. `build_path` now removes a fragment suffix before
returning, using the same brace-aware delimiter handling. Added coverage for
both a literal fragment suffix and an expression inside the discarded fragment;
the existing encoded-`#` parameter test still confirms parameter data is
preserved.
##########
t/plugin/openapi-to-mcp.t:
##########
@@ -1573,3 +1576,414 @@ streamable_http 5: true
streamable_http 6: true
streamable_http 7: true
streamable_http 8: true
+
+
+
+=== TEST 86: a route over a document with a string path parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local ok = require("lib.openapi_to_mcp_fixture").put_routes({
+ { 1, "/mcp", {
+ transport = "streamable_http",
+ base_url = "http://127.0.0.1:11460";,
+ openapi_url = "http://127.0.0.1:11460/defaults.json";,
+ } },
+ })
+ if ok then ngx.say("passed") end
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 87: a path parameter fills the segment it was declared for
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getItem","arguments":{"pathParameters":{"id":"abc"}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/seen_path.*items\/abc/
+
+
+
+=== TEST 88: a dot segment in a path parameter fails the call
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getItem","arguments":{"pathParameters":{"id":".."}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*path parameter in segment \{id\} cannot
produce an empty or dot path segment)/
+
+
+
+=== TEST 89: a single dot in a path parameter fails the call
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getItem","arguments":{"pathParameters":{"id":"."}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*path parameter in segment \{id\} cannot
produce an empty or dot path segment)/
+
+
+
+=== TEST 90: a separator in a path parameter fails the call
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getItem","arguments":{"pathParameters":{"id":"../items"}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*path parameter id cannot contain a path
separator)/
+
+
+
+=== TEST 91: a backslash in a path parameter fails the call
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getItem","arguments":{"pathParameters":{"id":"..\\\\items"}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*path parameter id cannot contain a path
separator)/
+
+
+
+=== TEST 92: an empty path parameter fails the call
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getItem","arguments":{"pathParameters":{"id":""}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*path parameter in segment \{id\} cannot
produce an empty or dot path segment)/
+
+
+
+=== TEST 93: a route over a document whose parameter fills part of a segment
+--- config
+ location /t {
+ content_by_lua_block {
+ local ok = require("lib.openapi_to_mcp_fixture").put_routes({
+ { 1, "/mcp", {
+ transport = "streamable_http",
+ base_url = "http://127.0.0.1:11460";,
+ openapi_url = "http://127.0.0.1:11460/partialsegment.json";,
+ } },
+ })
+ if ok then ngx.say("passed") end
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 94: a parameter that fills part of a segment is sent as given
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getFile","arguments":{"pathParameters":{"name":"report"}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/seen_path.*files\/report\.json/
+
+
+
+=== TEST 95: a dot-dot value that is not a whole segment is allowed through
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getFile","arguments":{"pathParameters":{"name":".."}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/seen_path.*files\/\.\.\.json/
+
+
+
+=== TEST 96: a single dot that is not a whole segment is allowed through
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getFile","arguments":{"pathParameters":{"name":"."}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/seen_path.*files\/\.\.json/
+
+
+
+=== TEST 97: an empty value that leaves a non-empty segment is allowed through
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getFile","arguments":{"pathParameters":{"name":""}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/seen_path.*files\/\.json/
+
+
+
+=== TEST 98: a separator is still rejected when the parameter fills part of a
segment
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getFile","arguments":{"pathParameters":{"name":"../secret"}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*path parameter name cannot contain a path
separator)/
+
+
+
+=== TEST 99: a route over path templates with several expressions and URI
suffixes
+--- config
+ location /t {
+ content_by_lua_block {
+ local ok = require("lib.openapi_to_mcp_fixture").put_routes({
+ { 1, "/mcp", {
+ transport = "streamable_http",
+ base_url = "http://127.0.0.1:11460";,
+ openapi_url = "http://127.0.0.1:11460/path-safety.json";,
+ } },
+ })
+ if ok then ngx.say("passed") end
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 100: adjacent expressions are judged as one finished segment
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"adjacentPathParameters","arguments":{"pathParameters":{"a":".","b":"."}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*cannot produce an empty or dot path segment)/
+
+
+
+=== TEST 101: literals between expressions take part in the finished segment
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"separatedPathParameters","arguments":{"pathParameters":{"a":"","b":""}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*cannot produce an empty or dot path segment)/
+
+
+
+=== TEST 102: a literal prefix can complete a dot segment
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"prefixedPathParameter","arguments":{"pathParameters":{"a":"."}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*cannot produce an empty or dot path segment)/
+
+
+
+=== TEST 103: a query suffix does not hide a dot path segment
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getBucketAcl","arguments":{"pathParameters":{"Bucket":".."}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*cannot produce an empty or dot path segment)/
+
+
+
+=== TEST 104: a query suffix does not hide an empty path segment
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getBucketAcl","arguments":{"pathParameters":{"Bucket":""}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*cannot produce an empty or dot path segment)/
+
+
+
+=== TEST 105: a fragment suffix does not hide a dot path segment
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getObjectAcl","arguments":{"pathParameters":{"Bucket":"b","Key":".."}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*cannot produce an empty or dot path segment)/
+
+
+
+=== TEST 106: a safe query-suffixed path is still sent unchanged
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getBucketAcl","arguments":{"pathParameters":{"Bucket":"reports"}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/seen_path.*reports\?acl/
+
+
+
+=== TEST 107: an ordinary separator is rejected by default and logged
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getEncodedPath","arguments":{"pathParameters":{"path":"group/project"}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*path parameter path cannot contain a path
separator)/
+--- error_log
+rejected an openapi-to-mcp tool call: path parameter path cannot contain a
path separator
+
+
+
+=== TEST 108: path validation also applies to flattened arguments
+--- config
+ location /t {
+ content_by_lua_block {
+ local ok = require("lib.openapi_to_mcp_fixture").put_routes({
+ { 1, "/mcp", {
+ transport = "streamable_http",
+ base_url = "http://127.0.0.1:11460";,
+ openapi_url = "http://127.0.0.1:11460/path-safety.json";,
+ flatten_parameters = true,
+ } },
+ })
+ if ok then ngx.say("passed") end
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 109: a flattened call is judged on its finished segment
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"adjacentPathParameters","arguments":{"a":".","b":"."}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/(?s)(?=.*"isError":true)(?=.*cannot produce an empty or dot path segment)/
+
+
+
+=== TEST 110: a route can opt in to encoded path separators
+--- config
+ location /t {
+ content_by_lua_block {
+ local ok = require("lib.openapi_to_mcp_fixture").put_routes({
+ { 1, "/mcp", {
+ transport = "streamable_http",
+ base_url = "http://127.0.0.1:11460";,
+ openapi_url = "http://127.0.0.1:11460/path-safety.json";,
+ allow_encoded_path_separators = true,
+ } },
+ })
+ if ok then ngx.say("passed") end
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 111: the opt-in sends a slash as data
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getEncodedPath","arguments":{"pathParameters":{"path":"group/project"}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/seen_path.*encoded\/group%2Fproject/
+
+
+
+=== TEST 112: the opt-in sends a backslash as data
+--- exec
+timeout 5 curl -X POST -N -sS http://localhost:1984/mcp \
+ -d
'{"method":"tools/call","jsonrpc":"2.0","id":1,"params":{"name":"getEncodedPath","arguments":{"pathParameters":{"path":"group\\project"}}}}'
\
+ -H "Content-Type: application/json" \
+ -H "Accept: application/json, text/event-stream" \
+ 2>&1 | cat
+--- response_body eval
+qr/seen_path.*encoded\/group%5Cproject/
+
+
+
+=== TEST 113: the opt-in still rejects a leading parent component
Review Comment:
Addressed in e309d12f7. Added an opt-in case with a separator-free `..`
value, which exercises the finished-segment check independently of
`separator_creates_unsafe_component`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
