fix(security): validate package-spec @-tail, close direct-reference bypass

Codex re-review of #672 found validatePackageSpec only validated the
parsed NAME, not the version/@-tail. A PEP 508 / npm direct reference
("pkg@./local", "pkg@/abs/path", "pkg@git+https://…") therefore passed
— the name "pkg" is a valid registry name — while the full untrusted
spec still reached `pip download` / `uv pip download` / `npm pack` and
executed setup.py. The P0 was not actually closed.

Validate the WHOLE spec:
- New bareVersionRe: the "name@<tail>" / "name==<tail>" tail must be a
  bare version / range / npm dist-tag — must start alphanumeric and
  contain no '/' or ':' — so a path/URL/VCS tail is rejected.
- Python: any '@' is a PEP 508 direct reference (pins use '=='), so it
  is refused outright.
- Bare version pins / dist-tags (pkg@1.2.3, pkg@latest, @scope/pkg@2.0.0)
  stay valid — no regression.

TDD: TestValidatePackageSpec gains npm+python "pkg@<path/url/vcs>" reject
cases and bare-version positive cases; TestResolveFromPackageFetch_
RejectsNonRegistrySpec gains uvx/npx direct-reference cases asserting the
setup.py execution marker stays absent.

Related MCP-2442

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

Author: Dumbris

docs/features/security-scanner-plugins.md

@@ -259,7 +259,7 @@ Package-runner servers (`npx`, `uvx`) are the **primary** quarantine/scan target
 
 **The source is fetched but never executed.** A scanner must not run the untrusted code it is scanning. The fetch only ever downloads and unpacks archives:
 
-- **Registry name required.** Only a server whose configured spec is a **bare registry name** (PEP 503 for PyPI, npm package name — optionally version-pinned) is fetched. Local-path, `file:`, URL, and VCS (`git+…`, `[email protected]:…`) specs are **rejected** and fall back to tool definitions only. This is mandatory: for a non-registry spec, `pip download` / `uv pip download` still invokes the package's `setup.py` / PEP 517 build backend to resolve metadata **even with `--only-binary=:all:`** — executing untrusted code on the (static) scan path. `--only-binary=:all:` alone protects only bare registry names.
+- **Registry name required.** Only a server whose configured spec is a **bare registry name** (PEP 503 for PyPI, npm package name — optionally version-pinned) is fetched. Local-path, `file:`, URL, and VCS (`git+…`, `[email protected]:…`) specs are **rejected** and fall back to tool definitions only. This is mandatory: for a non-registry spec, `pip download` / `uv pip download` still invokes the package's `setup.py` / PEP 517 build backend to resolve metadata **even with `--only-binary=:all:`** — executing untrusted code on the (static) scan path. `--only-binary=:all:` alone protects only bare registry names. Validation covers the **whole spec including the `@`-tail**: a PEP 508 / npm direct reference such as `pkg@./local`, `pkg@/abs/path`, or `pkg@git+https://…` is rejected (the `name@<ref>` tail must be a bare version specifier, not a path/URL/VCS); for PyPI, any `@` is treated as a direct reference and refused.
 - **npm (`npx`)** — `npm pack <pkg>@<version> --ignore-scripts` downloads the published tarball without running any lifecycle scripts (`install`/`postinstall`), then it is extracted (`source_method=npm_pack`).
 - **PyPI (`uvx`)** — `uv pip download <pkg>==<version> --no-deps --only-binary=:all:` (falling back to `pip download`) fetches **only a prebuilt wheel**, which is unpacked without building or running `setup.py` (`source_method=pip_download`). `--only-binary=:all:` is mandatory: downloading an sdist would invoke the package's PEP 517 build backend (`setup.py egg_info`) to resolve metadata, executing the untrusted code. A package that ships **no wheel** therefore fails the fetch and falls back to tool definitions only — sdists are never built or extracted.
 

internal/security/scanner/package_fetch.go

@@ -114,6 +114,13 @@ var (
 	// pep503NameRe matches a PEP 503 / PEP 508 distribution name (case
 	// insensitive). No path/URL/VCS characters are permitted.
 	pep503NameRe = regexp.MustCompile(`^[A-Za-z0-9]([A-Za-z0-9._-]*[A-Za-z0-9])?$`)
+	// bareVersionRe matches a plain version pin, range, or npm dist-tag. It must
+	// START with an alphanumeric (so it rejects "./x", "/abs", "~/x", "../x") and
+	// contains NO '/' or ':' (so it rejects local paths and direct-reference
+	// URLs). This validates the 'name@<tail>' / 'name==<tail>' tail so a PEP 508 /
+	// npm direct reference (e.g. "pkg@./local", "pkg@/abs") cannot smuggle a
+	// path/URL/VCS past the name check and on to pip/npm (MCP-2442 re-review).
+	bareVersionRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9.+_~^*<>=!,-]*$`)
 )
 
 // validatePackageSpec rejects any package spec that is not a bare registry name
@@ -145,19 +152,33 @@ func validatePackageSpec(ecosystem, spec string) error {
 	if strings.Contains(spec, "..") {
 		return fmt.Errorf("package spec %q contains a path traversal", spec)
 	}
-	name, _ := parsePackageSpec(spec)
+	name, version := parsePackageSpec(spec)
 	switch ecosystem {
 	case "npm":
 		if !npmNameRe.MatchString(name) {
 			return fmt.Errorf("%q is not a valid npm package name", name)
 		}
 	case "python":
+		// Python version pins use "==" / ">=" etc., never "@". The ONLY use of
+		// "@" in a PEP 508 spec is a direct reference ("name @ url"), which is
+		// always a path/URL/VCS — never a registry fetch. Reject any "@" outright.
+		if strings.Contains(spec, "@") {
+			return fmt.Errorf("python package spec %q is a PEP 508 direct reference (@), not a registry name", spec)
+		}
 		if !pep503NameRe.MatchString(name) {
 			return fmt.Errorf("%q is not a valid PEP 503 package name", name)
 		}
 	default:
 		return fmt.Errorf("unknown ecosystem %q", ecosystem)
 	}
+	// Validate the version / tail too. parsePackageSpec splits "name@tail" on the
+	// last '@'; without this check a direct-reference tail (e.g. "./local",
+	// "/abs", "~/x") would pass because only the NAME was validated, and the full
+	// untrusted spec would still reach pip/npm and execute setup.py (MCP-2442
+	// re-review). The tail must be a bare version specifier / dist-tag.
+	if version != "" && !bareVersionRe.MatchString(version) {
+		return fmt.Errorf("package spec %q has a non-version reference tail %q (path/URL/VCS not allowed)", spec, version)
+	}
 	return nil
 }
 

internal/security/scanner/package_fetch_test.go

@@ -510,6 +510,27 @@ func TestValidatePackageSpec(t *testing.T) {
 		{"npm", "file:../evil", false},
 		{"npm", "https://example.com/pkg.tgz", false},
 		{"npm", "", false},
+		// MCP-2442 re-review: PEP 508 / npm direct-reference '@'-tail bypass — the
+		// NAME parses as a bare registry name but the tail is a path/URL/VCS, which
+		// reaches pip/npm verbatim and STILL executes setup.py. The WHOLE spec
+		// (including the tail after '@') must be validated.
+		{"npm", "pkg@./local", false},
+		{"npm", "pkg@/abs/path", false},
+		{"npm", "pkg@~/evil", false},
+		{"npm", "pkg@git+https://github.com/user/repo.git", false},
+		{"npm", "pkg@file:./evil", false},
+		{"npm", "pkg@https://example.com/pkg.tgz", false},
+		{"npm", "@scope/pkg@./local", false},
+		{"python", "pkg@./local", false},
+		{"python", "pkg@/abs/path", false},
+		{"python", "pkg@git+https://github.com/user/repo.git", false},
+		{"python", "pkg@~/evil", false},
+		{"python", "flights @ git+https://example.com/repo.git", false},
+		// Bare version pins / dist-tags after '@' stay valid (no regression).
+		{"npm", "pkg@1.2.3", true},
+		{"npm", "pkg@1.2.3-beta.1", true},
+		{"npm", "pkg@latest", true},
+		{"npm", "@scope/pkg@2.0.0", true},
 	}
 	for _, c := range cases {
 		err := validatePackageSpec(c.ecosystem, c.spec)
@@ -541,6 +562,9 @@ func TestResolveFromPackageFetch_RejectsNonRegistrySpec(t *testing.T) {
 		{Name: "evil-path", Protocol: "stdio", Command: "uvx", Args: []string{"--from", pkgDir, "evil"}},
 		{Name: "evil-git", Protocol: "stdio", Command: "uvx", Args: []string{"--from", "git+https://example.com/repo.git", "evil"}},
 		{Name: "evil-npm-path", Protocol: "stdio", Command: "npx", Args: []string{"-y", "./local-evil"}},
+		// MCP-2442 re-review: PEP 508 / npm direct-reference '@'-tail bypass.
+		{Name: "evil-uvx-directref", Protocol: "stdio", Command: "uvx", Args: []string{"--from", "evilpkg@" + pkgDir, "evil"}},
+		{Name: "evil-npm-directref", Protocol: "stdio", Command: "npx", Args: []string{"-y", "evilpkg@./local-evil"}},
 	}
 	for _, info := range cases {
 		res, err := r.resolveFromPackageFetch(context.Background(), info)