fix: reject package/image names starting with '-' to prevent argument injection (#23070)

pkg/workflow/argument_injection_test.go

@@ -0,0 +1,207 @@
+//go:build !integration
+
+// Tests for argument injection prevention: package and image names starting with '-'
+// must be rejected before being passed to exec.Command calls (npm, pip, uv, docker).
+
+package workflow
+
+import (
+	"strings"
+	"testing"
+)
+
+// TestRejectHyphenPrefixPackages tests the shared helper that guards against
+// argument injection in all package validation paths.
+func TestRejectHyphenPrefixPackages(t *testing.T) {
+	tests := []struct {
+		name        string
+		packages    []string
+		kind        string
+		expectError bool
+		errContains string
+	}{
+		{
+			name:        "empty list is accepted",
+			packages:    []string{},
+			kind:        "npx",
+			expectError: false,
+		},
+		{
+			name:        "valid package names are accepted",
+			packages:    []string{"express", "@scope/pkg", "requests==2.28.0"},
+			kind:        "pip",
+			expectError: false,
+		},
+		{
+			name:        "single hyphen prefix is rejected",
+			packages:    []string{"-exploit"},
+			kind:        "npx",
+			expectError: true,
+			errContains: "must not start with '-'",
+		},
+		{
+			name:        "double hyphen prefix is rejected",
+			packages:    []string{"--privileged"},
+			kind:        "uv",
+			expectError: true,
+			errContains: "must not start with '-'",
+		},
+		{
+			name:        "mixed list with one invalid is rejected",
+			packages:    []string{"valid-pkg", "-exploit"},
+			kind:        "pip",
+			expectError: true,
+			errContains: "-exploit",
+		},
+		{
+			name:        "package with version specifier starting with hyphen is rejected",
+			packages:    []string{"-exploit==1.0"},
+			kind:        "uv",
+			expectError: true,
+			errContains: "-exploit==1.0",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := rejectHyphenPrefixPackages(tt.packages, tt.kind)
+			if tt.expectError {
+				if err == nil {
+					t.Errorf("expected error for packages %v but got none", tt.packages)
+					return
+				}
+				if tt.errContains != "" && !strings.Contains(err.Error(), tt.errContains) {
+					t.Errorf("expected error to contain %q, got: %v", tt.errContains, err)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("unexpected error for packages %v: %v", tt.packages, err)
+				}
+			}
+		})
+	}
+}
+
+// TestValidateDockerImage_RejectsHyphenPrefix tests that Docker image names starting
+// with '-' are rejected without invoking docker. This is the primary injection path
+// because image names are taken directly from frontmatter (not filtered by extractors).
+func TestValidateDockerImage_RejectsHyphenPrefix(t *testing.T) {
+	tests := []struct {
+		name        string
+		image       string
+		errContains string
+	}{
+		{
+			name:        "single hyphen prefix",
+			image:       "-exploit",
+			errContains: "must not start with '-'",
+		},
+		{
+			name:        "double hyphen prefix",
+			image:       "--privileged",
+			errContains: "must not start with '-'",
+		},
+		{
+			name:        "looks like a docker flag",
+			image:       "-rm",
+			errContains: "must not start with '-'",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateDockerImage(tt.image, false)
+			if err == nil {
+				t.Errorf("expected error for image %q but got none", tt.image)
+				return
+			}
+			if !strings.Contains(err.Error(), tt.errContains) {
+				t.Errorf("expected error to contain %q, got: %v", tt.errContains, err)
+			}
+		})
+	}
+}
+
+// TestExtractNpxFromCommands_HyphenPrefixFiltered shows that the npx package
+// extractor already filters out names starting with '-' (treating them as flags),
+// so they never reach the validation guard.
+func TestExtractNpxFromCommands_HyphenPrefixFiltered(t *testing.T) {
+	tests := []struct {
+		name     string
+		commands string
+		wantLen  int
+	}{
+		{
+			name:     "lone hyphen-prefix arg yields no packages",
+			commands: "npx -exploit",
+			wantLen:  0,
+		},
+		{
+			name:     "double-hyphen arg yields no packages",
+			commands: "npx --exploit",
+			wantLen:  0,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			packages := extractNpxFromCommands(tt.commands)
+			if len(packages) != tt.wantLen {
+				t.Errorf("expected %d packages, got %d: %v", tt.wantLen, len(packages), packages)
+			}
+		})
+	}
+}
+
+// TestExtractPipFromCommands_HyphenPrefixFiltered shows that the pip package
+// extractor already filters out names starting with '-', so they never reach
+// the validation guard.
+func TestExtractPipFromCommands_HyphenPrefixFiltered(t *testing.T) {
+	tests := []struct {
+		name     string
+		commands string
+		wantLen  int
+	}{
+		{
+			name:     "hyphen-prefix pip package yields no packages",
+			commands: "pip install -exploit",
+			wantLen:  0,
+		},
+		{
+			name:     "double-hyphen pip package yields no packages",
+			commands: "pip install --exploit",
+			wantLen:  0,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			packages := extractPipFromCommands(tt.commands)
+			if len(packages) != tt.wantLen {
+				t.Errorf("expected %d packages, got %d: %v", tt.wantLen, len(packages), packages)
+			}
+		})
+	}
+}
+
+// TestCollectPackagesFromWorkflow_HyphenPrefixInArgs verifies that the MCP tool
+// args extractor also filters out names starting with '-', providing an additional
+// layer of defense for the structured args format.
+func TestCollectPackagesFromWorkflow_HyphenPrefixInArgs(t *testing.T) {
+	workflowData := &WorkflowData{
+		Tools: map[string]any{
+			"test-tool": map[string]any{
+				"command": "npx",
+				"args":    []any{"-exploit", "safe-package"},
+			},
+		},
+	}
+
+	packages := collectPackagesFromWorkflow(workflowData, extractNpxFromCommands, "npx")
+
+	for _, pkg := range packages {
+		if strings.HasPrefix(pkg, "-") {
+			t.Errorf("package starting with '-' should not be collected: %s", pkg)
+		}
+	}
+}

pkg/workflow/docker_validation.go

@@ -89,6 +89,11 @@ func isDockerDaemonRunning() bool {
 func validateDockerImage(image string, verbose bool) error {
 	dockerValidationLog.Printf("Validating Docker image: %s", image)
 
+	// Reject names starting with '-' to prevent argument injection
+	if strings.HasPrefix(image, "-") {
+		return fmt.Errorf("container image name '%s' is invalid: names must not start with '-'", image)
+	}
+
 	// Check if docker CLI is available on PATH
 	_, err := exec.LookPath("docker")
 	if err != nil {

pkg/workflow/name_validation.go

@@ -0,0 +1,44 @@
+//go:build !js && !wasm
+
+// This file provides package and image name validation utilities for agentic workflows.
+//
+// # Name Validation
+//
+// Package and image names passed to external tools (npm, pip, uv, docker) must not
+// start with '-'. A leading '-' would be interpreted as a command-line flag by the
+// downstream tool, causing unintended argument injection.
+//
+// Note: exec.Command uses argv directly (not sh -c), so this is argument injection,
+// not shell injection. The risk is low — compilation runs on the developer's local
+// machine with the developer's own privileges.
+
+package workflow
+
+import (
+	"fmt"
+	"strings"
+)
+
+// rejectHyphenPrefixPackages returns a ValidationError if any of the provided
+// names starts with '-'. The kind parameter (e.g. "npx", "pip", "uv") is used
+// in the error messages.
+//
+// Names starting with '-' would be interpreted as flags by the downstream CLI
+// tool, constituting argument injection into the exec.Command call.
+func rejectHyphenPrefixPackages(names []string, kind string) error {
+	var invalid []string
+	for _, name := range names {
+		if strings.HasPrefix(name, "-") {
+			invalid = append(invalid, fmt.Sprintf("%s package name '%s' is invalid: names must not start with '-'", kind, name))
+		}
+	}
+	if len(invalid) == 0 {
+		return nil
+	}
+	return NewValidationError(
+		kind+".packages",
+		fmt.Sprintf("%d invalid package names", len(invalid)),
+		kind+" package names must not start with '-'",
+		"Fix invalid package names:\n\n"+strings.Join(invalid, "\n"),
+	)
+}

pkg/workflow/npm_validation.go

@@ -65,6 +65,13 @@ func (c *Compiler) validateNpxPackages(workflowData *WorkflowData) error {
 
 	npmValidationLog.Printf("Validating %d npx packages", len(packages))
 
+	// Reject any package names starting with '-' before invoking npm.
+	// These would be interpreted as flags by the npm CLI (argument injection).
+	if err := rejectHyphenPrefixPackages(packages, "npx"); err != nil {
+		npmValidationLog.Printf("npx package name validation failed: %v", err)
+		return err
+	}
+
 	// Check if npm is available
 	_, err := exec.LookPath("npm")
 	if err != nil {

pkg/workflow/pip_validation.go

@@ -59,6 +59,12 @@ func (c *Compiler) validatePythonPackagesWithPip(packages []string, packageType
 			pkgName = pkg[:eqIndex]
 		}
 
+		// Reject names starting with '-' to prevent argument injection
+		if strings.HasPrefix(pkgName, "-") {
+			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("%s package name '%s' is invalid: names must not start with '-'", packageType, pkg)))
+			continue
+		}
+
 		pipValidationLog.Printf("Validating %s package: %s", packageType, pkgName)
 
 		// Use pip index to check if package exists on PyPI
@@ -123,6 +129,13 @@ func (c *Compiler) validateUvPackages(workflowData *WorkflowData) error {
 
 	pipValidationLog.Printf("Starting uv package validation for %d packages", len(packages))
 
+	// Reject any package names starting with '-' before invoking uv or pip.
+	// These would be interpreted as flags by the CLI tools (argument injection).
+	if err := rejectHyphenPrefixPackages(packages, "uv"); err != nil {
+		pipValidationLog.Printf("uv package name validation failed: %v", err)
+		return err
+	}
+
 	// Check if uv is available
 	_, err := exec.LookPath("uv")
 	if err != nil {