Four numeric comparisons run on values taken straight from tool arguments, with no validation of their own:
mcp-server-gh/lib/common.sh:203 — max_lines
mcp-server-gh/lib/common.sh:207 — tail_lines
mcp-server-gh/lib/common.sh:197-198 — grep_before / grep_after (the grep_context_before / grep_context_after parameters)
mcp-server-gh/lib/release.sh:76 — per_page, from release_list's limit
[[ x -gt y ]] evaluates its operands as arithmetic expressions, and bash evaluates a command substitution written inside an array subscript there. A value such as PATH[$(command)0] reaching any of these lines runs command inside the MCP server process, which holds the user's gh credentials and filesystem access.
Nothing at these sites prevents that. What prevents it today is the protocol layer: validate_tool_arguments in the vendored shared/mcpserver_core.sh enforces a property's declared type, and every parameter feeding these comparisons declares "type": "integer" — max_lines (18 tools), tail_lines (7), grep_context_before and grep_context_after (5 each), limit (10, including release_list). Every tool in tools-read.json and tools-write.json carries an inputSchema, so no tool is dispatched unvalidated. A crafted call is refused before dispatch:
{"jsonrpc":"2.0","id":2,"result":{"content":[{"type":"text","text":"Invalid type(s): max_lines expected integer, got string (\"PATH[$(touch /tmp/marker)0]\")."}],"isError":true}}
That leaves the guarantee resting entirely on 45 schema declarations that nothing downstream re-checks. A dropped or mistyped type in a future schema edit re-opens command execution silently, and any call that does not pass through the protocol layer is unguarded:
source plugins/github-mcp/mcp-server-gh/lib/common.sh
log() { :; }
_gh_post_process "data" "" "" 0 0 false false 'PATH[$(touch /tmp/marker)0]' ""
/tmp/marker exists afterwards. A payload naming a variable that is unset (x[$(...)0]) aborts under the server's set -u with unbound variable before the substitution runs, which is why the payload has to name a variable that exists.
The other -gt comparisons in release.sh (lines 48, 63, 150) take jq-computed counts, not tool input, and are not affected.
Fix
Validate before comparing, at each of the four sites. _gh_validate_number in lib/common.sh:8 already has the right shape, or inline:
if [[ "${max_lines}" =~ ^[0-9]+$ ]] && [[ "${max_lines}" -gt 0 ]]; then
A regex match is not an arithmetic context, so the guard comes first and the numeric comparison never runs on an unvalidated value. Each site needs a test asserting that a non-numeric value is rejected rather than evaluated.
Four numeric comparisons run on values taken straight from tool arguments, with no validation of their own:
mcp-server-gh/lib/common.sh:203—max_linesmcp-server-gh/lib/common.sh:207—tail_linesmcp-server-gh/lib/common.sh:197-198—grep_before/grep_after(thegrep_context_before/grep_context_afterparameters)mcp-server-gh/lib/release.sh:76—per_page, fromrelease_list'slimit[[ x -gt y ]]evaluates its operands as arithmetic expressions, and bash evaluates a command substitution written inside an array subscript there. A value such asPATH[$(command)0]reaching any of these lines runscommandinside the MCP server process, which holds the user'sghcredentials and filesystem access.Nothing at these sites prevents that. What prevents it today is the protocol layer:
validate_tool_argumentsin the vendoredshared/mcpserver_core.shenforces a property's declaredtype, and every parameter feeding these comparisons declares"type": "integer"—max_lines(18 tools),tail_lines(7),grep_context_beforeandgrep_context_after(5 each),limit(10, includingrelease_list). Every tool intools-read.jsonandtools-write.jsoncarries aninputSchema, so no tool is dispatched unvalidated. A crafted call is refused before dispatch:That leaves the guarantee resting entirely on 45 schema declarations that nothing downstream re-checks. A dropped or mistyped
typein a future schema edit re-opens command execution silently, and any call that does not pass through the protocol layer is unguarded:/tmp/markerexists afterwards. A payload naming a variable that is unset (x[$(...)0]) aborts under the server'sset -uwithunbound variablebefore the substitution runs, which is why the payload has to name a variable that exists.The other
-gtcomparisons inrelease.sh(lines 48, 63, 150) take jq-computed counts, not tool input, and are not affected.Fix
Validate before comparing, at each of the four sites.
_gh_validate_numberinlib/common.sh:8already has the right shape, or inline:A regex match is not an arithmetic context, so the guard comes first and the numeric comparison never runs on an unvalidated value. Each site needs a test asserting that a non-numeric value is rejected rather than evaluated.