fix(scanner): reject non-registry package specs before fetch (P0 ACE)

Codex audit (MCP-2442): resolveFromPackageFetch passed the server-configured
package spec to npm pack / uv pip download / pip download UNVALIDATED. For a
uvx/pipx server with a local-path, git+/VCS, URL, or file: spec, the download
STILL executes the package's build backend (setup.py / PEP 517 egg_info) even
with --only-binary=:all: / --ignore-scripts — arbitrary code execution during
a supposedly static scan.

Add isBareRegistrySpec(spec, ecosystem) and gate both ecosystems in
resolveFromPackageFetch: only a bare PEP 503 (python) / npm registry name
(+ optional version pin) is fetched. Paths, git+/hg+/svn+/bzr+, URLs (://),
file:, PEP 508 direct references (name @ url), and shell metacharacters are
refused → caller falls back to tool_definitions_only (no fetch, no execution).
The registry-name path (already wheel-only / --ignore-scripts) is unchanged.

TDD: isBareRegistrySpec accept/reject table; resolveFromPackageFetch rejection
for git+/local/URL specs; and an execution-marker test proving a local-path
uvx spec never runs its setup.py (validation short-circuits before any
download, independent of whether uv/pip is installed).

Related #MCP-2442

Co-Authored-By: Paperclip <noreply@paperclip.ing>

Author: Dumbris

internal/security/scanner/package_fetch.go

@@ -10,6 +10,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 
 	"go.uber.org/zap"
@@ -79,6 +80,59 @@ func parsePackageSpec(spec string) (name, version string) {
 	return spec, ""
 }
 
+// pep503NameRe matches a bare PEP 503 Python distribution name (the only thing
+// we will hand to pip/uv on the scan path). No path separators, no scheme.
+var pep503NameRe = regexp.MustCompile(`^[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?$`)
+
+// npmNameRe matches a bare npm registry name, optionally scoped (@scope/name).
+// The single '/' of a scope is the ONLY slash permitted — a real filesystem
+// path (or "@scope/../escape") must not pass.
+var npmNameRe = regexp.MustCompile(`^(?:@[A-Za-z0-9][A-Za-z0-9._-]*/)?[A-Za-z0-9][A-Za-z0-9._-]*$`)
+
+// isBareRegistrySpec reports whether spec is a bare registry package name
+// (with an optional version pin) for the given ecosystem ("python" or "npm").
+//
+// MCP-2442 (CRITICAL/P0): the package-fetch scan path must REFUSE anything that
+// is not a bare registry name. pip/uv/npm execute the package's build backend
+// (setup.py / PEP 517 egg_info) when given a local path, git+/VCS, URL, file:,
+// or PEP 508 direct-reference spec — even with --only-binary=:all: /
+// --ignore-scripts — which is arbitrary code execution during a static scan.
+// Bare registry names are safe (wheel-only download / `npm pack --ignore-scripts`).
+// A rejected spec makes the fetch caller fall back to tool_definitions_only.
+func isBareRegistrySpec(spec, ecosystem string) bool {
+	if spec == "" || strings.TrimSpace(spec) != spec {
+		return false
+	}
+	// No whitespace (PEP 508 direct references use "name @ url").
+	if strings.ContainsAny(spec, " \t\r\n;") {
+		return false
+	}
+	// No URLs, VCS prefixes, file: refs, or direct references.
+	lower := strings.ToLower(spec)
+	for _, bad := range []string{"://", "git+", "hg+", "svn+", "bzr+", "file:", "@http", "@git"} {
+		if strings.Contains(lower, bad) {
+			return false
+		}
+	}
+	// No path indicators (absolute, relative, home, backslash, parent-dir).
+	if strings.HasPrefix(spec, ".") || strings.HasPrefix(spec, "/") ||
+		strings.HasPrefix(spec, "~") || strings.HasPrefix(spec, "\\") {
+		return false
+	}
+	if strings.Contains(spec, "\\") || strings.Contains(spec, "..") {
+		return false
+	}
+	// Validate the bare NAME (version specifier stripped) against the ecosystem.
+	name, _ := parsePackageSpec(spec)
+	switch ecosystem {
+	case "python":
+		return pep503NameRe.MatchString(name)
+	case "npm":
+		return npmNameRe.MatchString(name)
+	}
+	return false
+}
+
 // firstPackageArg returns the package spec from a runner command's args. It
 // honors `--from <pkg>` (uvx) and otherwise returns the first non-flag arg.
 func firstPackageArg(args []string) string {
@@ -321,8 +375,14 @@ func (r *SourceResolver) resolveFromPackageFetch(ctx context.Context, info Serve
 
 	switch cmdBase {
 	case "npx", "bunx", "pnpm":
+		if !isBareRegistrySpec(spec, "npm") {
+			return nil, fmt.Errorf("refusing to fetch non-registry npm spec %q for %s: path/URL/VCS/file specs can execute package code during a static scan (MCP-2442); falling back to tool_definitions_only", spec, info.Name)
+		}
 		return r.fetchNpmPackage(ctx, info, spec)
 	case "uvx", "pipx":
+		if !isBareRegistrySpec(spec, "python") {
+			return nil, fmt.Errorf("refusing to fetch non-registry python spec %q for %s: path/URL/VCS/file specs execute setup.py during a static scan (MCP-2442); falling back to tool_definitions_only", spec, info.Name)
+		}
 		return r.fetchPythonPackage(ctx, info, spec)
 	}
 	return nil, fmt.Errorf("package fetch unsupported for command %q", cmdBase)

internal/security/scanner/package_fetch_spec_validation_test.go

@@ -0,0 +1,115 @@
+package scanner
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"testing"
+
+	"go.uber.org/zap"
+)
+
+// TestIsBareRegistrySpec is the MCP-2442 (CRITICAL/P0) guard. The package-fetch
+// scan path must only ever hand the package manager a BARE registry name.
+// pip/uv/npm execute the package's build backend (setup.py / PEP 517) for
+// local-path, git+/VCS, URL, and file: specs even with --only-binary /
+// --ignore-scripts — arbitrary code execution during a supposedly static scan.
+func TestIsBareRegistrySpec(t *testing.T) {
+	cases := []struct {
+		spec      string
+		ecosystem string
+		want      bool
+	}{
+		// Accepted: bare registry names (+ optional version pin).
+		{"requests", "python", true},
+		{"requests==2.31.0", "python", true},
+		{"Flask>=2.0", "python", true},
+		{"mcp-server-git", "python", true},
+		{"package_with_underscores", "python", true},
+		{"left-pad", "npm", true},
+		{"left-pad@1.3.0", "npm", true},
+		{"@modelcontextprotocol/server-everything", "npm", true},
+		{"@scope/name@1.2.3", "npm", true},
+
+		// Rejected (Python): paths, VCS, URLs, file:, direct refs.
+		{"./local-pkg", "python", false},
+		{"../evil", "python", false},
+		{"/abs/path/pkg", "python", false},
+		{"~user/pkg", "python", false},
+		{"git+https://github.com/evil/repo.git", "python", false},
+		{"git+ssh://git@github.com/evil/repo.git", "python", false},
+		{"hg+https://example.com/repo", "python", false},
+		{"https://evil.example.com/pkg.tar.gz", "python", false},
+		{"file:./local", "python", false},
+		{"file:///abs/pkg", "python", false},
+		{"requests @ https://evil.example.com/x.whl", "python", false},
+		{"pkg; rm -rf /", "python", false},
+		{"a\\b", "python", false},
+		{"", "python", false},
+		{"  spaced  ", "python", false},
+
+		// Rejected (npm): the slash-name path must not let a real filesystem path through.
+		{"./local-pkg", "npm", false},
+		{"/abs/path", "npm", false},
+		{"git+https://github.com/evil/repo.git", "npm", false},
+		{"file:../local", "npm", false},
+		{"https://evil.example.com/x.tgz", "npm", false},
+		{"@scope/../escape", "npm", false},
+	}
+	for _, c := range cases {
+		t.Run(c.ecosystem+"/"+c.spec, func(t *testing.T) {
+			if got := isBareRegistrySpec(c.spec, c.ecosystem); got != c.want {
+				t.Errorf("isBareRegistrySpec(%q, %q) = %v, want %v", c.spec, c.ecosystem, got, c.want)
+			}
+		})
+	}
+}
+
+// TestResolveFromPackageFetch_RejectsNonRegistrySpec proves the validation
+// short-circuits BEFORE any download command runs, for both ecosystems.
+func TestResolveFromPackageFetch_RejectsNonRegistrySpec(t *testing.T) {
+	r := NewSourceResolver(zap.NewNop())
+	cases := []ServerInfo{
+		{Name: "py-git", Command: "uvx", Args: []string{"--from", "git+https://github.com/evil/repo.git", "evil"}},
+		{Name: "py-local", Command: "uvx", Args: []string{"--from", "/tmp/evil-pkg", "evil"}},
+		{Name: "npm-url", Command: "npx", Args: []string{"https://evil.example.com/x.tgz"}},
+	}
+	for _, info := range cases {
+		t.Run(info.Name, func(t *testing.T) {
+			_, err := r.resolveFromPackageFetch(context.Background(), info)
+			if err == nil {
+				t.Fatalf("expected rejection for non-registry spec, got nil error")
+			}
+			if !strings.Contains(err.Error(), "non-registry") {
+				t.Errorf("expected a non-registry rejection, got: %v", err)
+			}
+		})
+	}
+}
+
+// TestResolveFromPackageFetch_LocalPathSpecDoesNotExecute is the execution-marker
+// proof requested by MCP-2442. A uvx server configured with a LOCAL PATH whose
+// setup.py writes a marker file must never have that marker appear — the spec is
+// rejected at validation, so pip/uv is never invoked. This holds regardless of
+// whether uv/pip is installed on the host (validation fires first).
+func TestResolveFromPackageFetch_LocalPathSpecDoesNotExecute(t *testing.T) {
+	pkgDir := t.TempDir()
+	marker := filepath.Join(t.TempDir(), "EXECUTED")
+	// A malicious setup.py that, if built/executed, would create the marker.
+	setupPy := "import pathlib; pathlib.Path(" + strconv.Quote(marker) + ").write_text('pwned')\n"
+	if err := os.WriteFile(filepath.Join(pkgDir, "setup.py"), []byte(setupPy), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	r := NewSourceResolver(zap.NewNop())
+	info := ServerInfo{Name: "evil-local", Command: "uvx", Args: []string{"--from", pkgDir, "evil"}}
+
+	if _, err := r.resolveFromPackageFetch(context.Background(), info); err == nil {
+		t.Fatalf("expected local-path spec to be rejected")
+	}
+	if _, err := os.Stat(marker); err == nil {
+		t.Fatalf("setup.py was EXECUTED during scan — marker file %q exists (arbitrary code execution)", marker)
+	}
+}