Fix argument injection in npm/pip/docker package validators (#23374)

* Initial plan

* Fix argument injection in npm/pip/docker package validators

- Add npm package name regex validation (validateNpmPackageName) in name_validation.go
- Add PyPI package name regex validation (validatePipPackageName) per PEP 508 in name_validation.go
- Add -- end-of-options separator to npm view command in npm_validation.go
- Apply npm name regex validation before invoking npm CLI
- Apply pip name regex validation in validatePythonPackagesWithPip
- Add tests for single-character names and injection-style names in argument_injection_test.go

Closes #XXX

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/db1ddaab-af87-4f48-8b54-597c4bcbe3de

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>

* Fix npm name regex to allow version suffixes in package specifiers

The npmPackageNameRE regex rejected valid npm package specifiers that
include version suffixes (e.g. @sentry/mcp-server@0.29.0). This caused
mcp-inspector.md to fail compilation in CI (177/178 instead of 178/178).

Update the regex to accept an optional @Version suffix and add test
cases for scoped+versioned, unscoped+versioned, caret ranges, and tags.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: Landon Cox <landon.cox@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: 3 people

pkg/workflow/argument_injection_test.go

@@ -205,3 +205,184 @@ func TestCollectPackagesFromWorkflow_HyphenPrefixInArgs(t *testing.T) {
 		}
 	}
 }
+
+// TestValidateNpmPackageName tests that npm package names are validated against
+// the npm naming rules, preventing argument injection via names that aren't caught
+// by the hyphen-prefix check alone.
+func TestValidateNpmPackageName(t *testing.T) {
+	tests := []struct {
+		name        string
+		pkg         string
+		expectError bool
+		errContains string
+	}{
+		{
+			name:        "simple valid name",
+			pkg:         "express",
+			expectError: false,
+		},
+		{
+			name:        "scoped valid name",
+			pkg:         "@scope/pkg",
+			expectError: false,
+		},
+		{
+			name:        "name with hyphen",
+			pkg:         "my-package",
+			expectError: false,
+		},
+		{
+			name:        "name with dot",
+			pkg:         "my.package",
+			expectError: false,
+		},
+		{
+			name:        "single character name",
+			pkg:         "x",
+			expectError: false,
+		},
+		{
+			name:        "scoped name with version suffix",
+			pkg:         "@sentry/mcp-server@0.29.0",
+			expectError: false,
+		},
+		{
+			name:        "unscoped name with version suffix",
+			pkg:         "express@4.18.2",
+			expectError: false,
+		},
+		{
+			name:        "name with semver caret range",
+			pkg:         "express@^4.0.0",
+			expectError: false,
+		},
+		{
+			name:        "name with tag version",
+			pkg:         "express@latest",
+			expectError: false,
+		},
+		{
+			name:        "name starting with hyphen is rejected",
+			pkg:         "--registry=https://attacker.example",
+			expectError: true,
+			errContains: "invalid npm package name",
+		},
+		{
+			name:        "name with equals sign is rejected",
+			pkg:         "pkg=https://attacker.example",
+			expectError: true,
+			errContains: "invalid npm package name",
+		},
+		{
+			name:        "name with spaces is rejected",
+			pkg:         "my package",
+			expectError: true,
+			errContains: "invalid npm package name",
+		},
+		{
+			name:        "uppercase name is rejected",
+			pkg:         "MyPackage",
+			expectError: true,
+			errContains: "invalid npm package name",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateNpmPackageName(tt.pkg)
+			if tt.expectError {
+				if err == nil {
+					t.Errorf("expected error for package %q but got none", tt.pkg)
+					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 package %q: %v", tt.pkg, err)
+				}
+			}
+		})
+	}
+}
+
+// TestValidatePipPackageName tests that PyPI package names are validated against
+// PEP 508 naming rules, preventing argument injection via names not caught by
+// the hyphen-prefix check alone.
+func TestValidatePipPackageName(t *testing.T) {
+	tests := []struct {
+		name        string
+		pkg         string
+		expectError bool
+		errContains string
+	}{
+		{
+			name:        "simple valid name",
+			pkg:         "requests",
+			expectError: false,
+		},
+		{
+			name:        "name with hyphen",
+			pkg:         "my-package",
+			expectError: false,
+		},
+		{
+			name:        "name with underscore",
+			pkg:         "my_package",
+			expectError: false,
+		},
+		{
+			name:        "name with dot",
+			pkg:         "my.package",
+			expectError: false,
+		},
+		{
+			name:        "mixed case is valid per PEP 508",
+			pkg:         "MyPackage",
+			expectError: false,
+		},
+		{
+			name:        "single character name",
+			pkg:         "z",
+			expectError: false,
+		},
+		{
+			name:        "name starting with hyphen is rejected",
+			pkg:         "--index-url=https://attacker.example",
+			expectError: true,
+			errContains: "invalid pip package name",
+		},
+		{
+			name:        "name with equals sign is rejected",
+			pkg:         "pkg=https://attacker.example",
+			expectError: true,
+			errContains: "invalid pip package name",
+		},
+		{
+			name:        "name with spaces is rejected",
+			pkg:         "my package",
+			expectError: true,
+			errContains: "invalid pip package name",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validatePipPackageName(tt.pkg)
+			if tt.expectError {
+				if err == nil {
+					t.Errorf("expected error for package %q but got none", tt.pkg)
+					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 package %q: %v", tt.pkg, err)
+				}
+			}
+		})
+	}
+}

pkg/workflow/name_validation.go

@@ -16,9 +16,39 @@ package workflow
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 )
 
+// npmPackageNameRE matches valid npm package specifiers:
+// - Scoped: @scope/name where scope and name are lowercase alphanumeric + hyphens + dots + underscores
+// - Unscoped: lowercase alphanumeric + hyphens + dots + underscores
+// - Optional version suffix: @version (e.g. @1.2.3, @^1.0.0, @latest)
+// This rejects any name that could be interpreted as a CLI flag.
+var npmPackageNameRE = regexp.MustCompile(`^(@[a-z0-9][a-z0-9._-]*/)?[a-z0-9][a-z0-9._-]*(@[a-zA-Z0-9^~>=<.*|-]+)?$`)
+
+// pypiPackageNameRE matches valid PyPI package names per PEP 508 / PEP 625:
+// must start and end with alphanumeric; interior may include dots, hyphens, underscores.
+var pypiPackageNameRE = regexp.MustCompile(`^[A-Za-z0-9]([A-Za-z0-9._-]*[A-Za-z0-9])?$`)
+
+// validateNpmPackageName returns an error if the package name does not conform
+// to the npm package naming rules. This prevents argument injection into the npm CLI.
+func validateNpmPackageName(pkg string) error {
+	if !npmPackageNameRE.MatchString(pkg) {
+		return fmt.Errorf("invalid npm package name: %q", pkg)
+	}
+	return nil
+}
+
+// validatePipPackageName returns an error if the package name does not conform
+// to the PyPI naming rules (PEP 508). This prevents argument injection into pip/uv.
+func validatePipPackageName(pkgName string) error {
+	if !pypiPackageNameRE.MatchString(pkgName) {
+		return fmt.Errorf("invalid pip package name: %q", pkgName)
+	}
+	return nil
+}
+
 // 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.

pkg/workflow/npm_validation.go

@@ -72,6 +72,21 @@ func (c *Compiler) validateNpxPackages(workflowData *WorkflowData) error {
 		return err
 	}
 
+	// Validate each package name against the npm naming rules.
+	// This provides a second layer of defence against names that could be
+	// misinterpreted by the npm CLI even after the hyphen-prefix check.
+	for _, pkg := range packages {
+		if err := validateNpmPackageName(pkg); err != nil {
+			npmValidationLog.Printf("npm package name validation failed: %v", err)
+			return NewValidationError(
+				"npx.packages",
+				"invalid npm package name",
+				err.Error(),
+				fmt.Sprintf("npm package names must match @scope/name or name (lowercase alphanumeric, hyphens, dots, underscores).\n\nInvalid name: %q", pkg),
+			)
+		}
+	}
+
 	// Check if npm is available
 	_, err := exec.LookPath("npm")
 	if err != nil {
@@ -83,8 +98,9 @@ func (c *Compiler) validateNpxPackages(workflowData *WorkflowData) error {
 	for _, pkg := range packages {
 		npmValidationLog.Printf("Validating npm package: %s", pkg)
 
-		// Use npm view to check if package exists
-		cmd := exec.Command("npm", "view", pkg, "name")
+		// Use npm view to check if package exists.
+		// Pass -- to prevent the package name being interpreted as a flag (argument injection defence).
+		cmd := exec.Command("npm", "view", "--", pkg, "name")
 		output, err := cmd.CombinedOutput()
 
 		if err != nil {

pkg/workflow/pip_validation.go

@@ -65,6 +65,13 @@ func (c *Compiler) validatePythonPackagesWithPip(packages []string, packageType
 			continue
 		}
 
+		// Validate the package name against PyPI naming rules (PEP 508).
+		// pip does not universally honour '--', so we validate upfront.
+		if err := validatePipPackageName(pkgName); err != nil {
+			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("%s package name '%s' is invalid: %v", packageType, pkg, err)))
+			continue
+		}
+
 		pipValidationLog.Printf("Validating %s package: %s", packageType, pkgName)
 
 		// Use pip index to check if package exists on PyPI