Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 22 additions & 10 deletions internal/mcp/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,20 @@ var allowedCommands = map[string]bool{
// Shell metacharacters that indicate injection attempt.
var shellMetaChars = regexp.MustCompile(`[;|&$` + "`" + `(){}[\]<>]`)

// Dangerous arg flags that enable code execution.
var dangerousArgPatterns = []string{
"--eval", "-e", "-c", // Code execution flags
"--require", "-r", // Module injection
"--import", // ES module injection
"exec(", "eval(", // Inline code
"__import__", // Python import injection
"child_process", // Node.js process spawning
"subprocess", // Python subprocess
// Dangerous arg flags that must match the entire arg (or appear as --flag=value).
// These are short/long flags that enable code execution and would produce false
// positives if checked as substrings (e.g. "-c" matches "clickup-cli").
var dangerousArgFlags = []string{
"-c", "-e", "-r", // Short code execution / module injection flags
"--eval", "--require", "--import", // Long flags
}

// Dangerous code-execution substrings that may appear anywhere inside an arg.
var dangerousArgSubstrings = []string{
"exec(", "eval(", // Inline code
"__import__", // Python import injection
"child_process", // Node.js process spawning
"subprocess", // Python subprocess
}

// Fail-closed env var allowlist — only these are permitted for env: resolution.
Expand Down Expand Up @@ -96,7 +101,14 @@ func ValidateCommand(cmd string) error {
func ValidateArgs(args []string) error {
for i, arg := range args {
argLower := strings.ToLower(arg)
for _, pattern := range dangerousArgPatterns {
// Flags: exact match, or "--flag=value" prefix. Avoids false positives
// like "clickup-cli" matching "-c".
for _, flag := range dangerousArgFlags {
if argLower == flag || strings.HasPrefix(argLower, flag+"=") {
return fmt.Errorf("arg[%d] is dangerous flag %q", i, flag)
}
}
for _, pattern := range dangerousArgSubstrings {
if strings.Contains(argLower, pattern) {
return fmt.Errorf("arg[%d] contains dangerous pattern %q", i, pattern)
}
Expand Down
5 changes: 5 additions & 0 deletions internal/mcp/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ func TestValidateArgs_DangerousPatterns_Rejected(t *testing.T) {
{"valid args", []string{"server.js", "--port", "3000"}, false},
{"valid path args", []string{"/path/to/script.js"}, false},
{"empty args", []string{}, false},
// Regression: package names containing dangerous flag substrings must not trip the check.
{"package name with -c substring", []string{"@nick.bester/clickup-cli", "mcp", "serve"}, false},
{"package name with -e substring", []string{"some-experimental-pkg"}, false},
{"package name with -r substring", []string{"some-runner-pkg"}, false},
{"flag with equals form", []string{"--eval=process.exit(1)"}, true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
Loading