From cea505514c2853a099673617fb226efea12bdbe7 Mon Sep 17 00:00:00 2001
From: Alexander Viktorov <3058703+alexviktorov@users.noreply.github.com>
Date: Wed, 27 May 2026 11:30:58 -0700
Subject: [PATCH] fix(mcp): only derive a package from the manager's executor
 subcommand
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

MCP servers launched through a package-manager script runner leaked the
script or file name as the package. The official Claude messaging plugins
use `bun run … start` and were recorded as package_name "start" — a real
npm package, so an advisory for it would spuriously match. `bun server.ts`
/ `bun run src/index.ts` leaked the file path, and bare `yarn dev` /
`pnpm build` leaked the script name the same way.

The parser already handles this for uv: `uv run <script>` without --from
yields no package identity and the record falls back to the server id at
low confidence. Apply the same rule to the npm family, per manager: only
`npm exec`/`npm x`, `pnpm dlx`, `yarn dlx` and `bun x` (or the separate
npx/bunx) fetch and run a published package. `exec` under pnpm, yarn and
bun runs a local bin or a shell command, so it is not an executor there
(`yarn exec node dist/index.js` must not become the package "node").
Every other first token — `run <script>`, lifecycle aliases, bare
scripts, files, create/init — returns no spec.

Executor launches are unchanged: --package, the -- terminator and
credential-bearing value flags keep their identity. Trade-off: a bare
`yarn <installed-bin>` also falls back to the server id, the same
conservative choice as `uv run`.

Updates the inferPackageFromArgs doc comment and docs/inventory-sources.md;
adds unit rows and corpus entries that fail on main with the leaked values.
---
 docs/inventory-sources.md          |  10 +++
 internal/ecosystem/mcp/mcp.go      |  55 ++++++++++----
 internal/ecosystem/mcp/mcp_test.go | 117 +++++++++++++++++++++++++++++
 3 files changed, 166 insertions(+), 16 deletions(-)

diff --git a/docs/inventory-sources.md b/docs/inventory-sources.md
index 1e64555..0121bef 100644
--- a/docs/inventory-sources.md
+++ b/docs/inventory-sources.md
@@ -339,6 +339,16 @@ is inferred; the record falls back to the server id with `confidence=low`.
 `uvx`, `uv tool run <pkg>`, and `uv run --from <pkg> ...` use the
 published package name.
 
+For `npm`/`pnpm`/`yarn`/`bun`, only the manager's package executor names a
+published package: `npm exec`/`npm x`, `pnpm dlx`, `yarn dlx`, `bun x`, or
+the separate `npx`/`bunx`. `exec` under pnpm, yarn, and bun runs a locally
+installed binary or a shell command, so its first token is not a package.
+`run <script>`, the npm lifecycle aliases (`start`/`stop`/`restart`/`test`),
+bare `yarn dev` / `bun start`, `bun <file>` / `bun run <file>`, and the
+`create`/`init` initializers (which resolve to a `create-<name>` package that
+is intentionally not inferred) yield no package identity; the record falls
+back to the server id with `confidence=low`.
+
 Docker/OCI image refs split a pinned tag into `version`:
 `hashicorp/terraform-mcp-server:0.4.0` becomes
 `package_name=hashicorp/terraform-mcp-server`, `version=0.4.0`. Untagged
diff --git a/internal/ecosystem/mcp/mcp.go b/internal/ecosystem/mcp/mcp.go
index c55ae10..a2304a4 100644
--- a/internal/ecosystem/mcp/mcp.go
+++ b/internal/ecosystem/mcp/mcp.go
@@ -402,7 +402,8 @@ func looksUnresolvedShellVar(s string) bool {
 // Supported launchers:
 //
 //	npx / bunx                          -> first non-flag arg
-//	pnpm/yarn/bun/npm dlx|exec|x|run    -> first non-flag arg past sub
+//	npm exec|x, pnpm dlx, yarn dlx, bun x -> first non-flag arg past sub
+//	  (any other first token             -> no spec; caller uses server id)
 //	uvx / pipx                          -> first non-flag arg
 //	uv / uv tool run                    -> first non-flag arg past sub,
 //	                                       also honors --from <pkg>
@@ -427,23 +428,33 @@ func inferPackageFromArgs(cmd string, args []string) (spec, launcher string) {
 		}
 		return firstNonFlag(args, nil, npmValueTakingFlags), ""
 	case "pnpm", "yarn", "bun", "npm":
-		// These wrappers take a subcommand (dlx, exec, x, run) before the
-		// package. Skip the subcommand so we return the actual package
-		// argument rather than "dlx" / "exec" / "x". Honor
-		// "npm exec --package=<pkg>" / "npm exec --package <pkg>" since
-		// those configs name the package explicitly via flag rather than
-		// positional.
+		// We read a package identity only from the manager's package
+		// executor — the subcommand that fetches a published package and
+		// runs it (packageExecutors; npx/bunx are handled above). The
+		// positional after it — or --package — is the spec.
 		//
-		// Restrict the --package scan to args before "--": npm does not
-		// parse options past "--", so `npm exec foo -- --package @npmcli/bar`
-		// must resolve to foo, not @npmcli/bar.
-		subcommands := map[string]bool{
-			"dlx": true, "exec": true, "x": true, "run": true,
+		// Any other first token gets no package identity: return an empty
+		// spec and let the caller fall back to the server id at low
+		// confidence, like "uv run <script>" below. That stops a local-script
+		// launcher (`run <script>`, `npm start`, bare `yarn dev`, `bun <file>`)
+		// from leaking its script or file name as a package — the reported
+		// bug where `bun run … start` became the package "start". The same
+		// holds for `exec` under pnpm/yarn/bun, which runs a local bin or a
+		// shell command (`yarn exec node …` would otherwise become "node"),
+		// and for create/init, which name a create-<name> package we
+		// deliberately don't resolve (initializers don't launch servers).
+		// See mcp_test.go for these and the flag-ordering edge case.
+		//
+		// scanExplicitPackage / firstNonFlag honor "npm exec --package=<pkg>"
+		// and stop at "--", so `npm exec foo -- --package @npmcli/bar` -> foo.
+		executors := packageExecutors[bn]
+		if !executors[firstNonFlag(args, nil, npmValueTakingFlags)] {
+			return "", ""
 		}
-		if spec := scanExplicitPackage(args, subcommands); spec != "" {
+		if spec := scanExplicitPackage(args, executors); spec != "" {
 			return spec, ""
 		}
-		return firstNonFlag(args, subcommands, npmValueTakingFlags), ""
+		return firstNonFlag(args, executors, npmValueTakingFlags), ""
 	case "uvx":
 		return firstNonFlag(args, nil, nil), "uv"
 	case "uv":
@@ -547,8 +558,8 @@ func inferPackageFromArgs(cmd string, args []string) (spec, launcher string) {
 // scanExplicitPackage looks for "--package <pkg>" / "--package=<pkg>" in
 // the launcher-parsed prefix of args. The scan stops at "--" (npm/npx do
 // not interpret options past it) and at the first positional non-flag
-// token that is not in the skip set of recognized subcommands
-// (dlx/exec/x/run). Other value-taking flags are consumed so their values
+// token that is not in the skip set of recognized executor subcommands
+// (packageExecutors). Other value-taking flags are consumed so their values
 // are not misread as --package. Returns the explicit package spec when
 // found, otherwise "".
 func scanExplicitPackage(args []string, skip map[string]bool) string {
@@ -604,6 +615,18 @@ func firstNonFlag(args []string, skip map[string]bool, valueTaking map[string]bo
 	return ""
 }
 
+// packageExecutors lists, per package manager, the subcommands that fetch a
+// published package and run it. Only these name a package in an MCP launch.
+// `exec` qualifies under npm alone: pnpm `exec` runs a locally installed bin,
+// and yarn `exec` / bun `exec` run a shell command, so their first token is a
+// binary or script, not a package identity (per each manager's own CLI docs).
+var packageExecutors = map[string]map[string]bool{
+	"npm":  {"exec": true, "x": true},
+	"pnpm": {"dlx": true},
+	"yarn": {"dlx": true},
+	"bun":  {"x": true},
+}
+
 // npmValueTakingFlags lists npm/pnpm/yarn/bun flags that consume a
 // separate value argument. Without skipping past these values, a value
 // like a registry URL (often credential-bearing) can be misread as the
diff --git a/internal/ecosystem/mcp/mcp_test.go b/internal/ecosystem/mcp/mcp_test.go
index c5925c8..bdb8a7a 100644
--- a/internal/ecosystem/mcp/mcp_test.go
+++ b/internal/ecosystem/mcp/mcp_test.go
@@ -55,6 +55,50 @@ func TestInferPackageFromArgs(t *testing.T) {
 		{"docker", []string{"run", "-e", "FOO=bar", "--name", "x", "mcp/slack"}, "mcp/slack", "docker"},
 		{"docker", []string{"run", "--env-file=.env", "ghcr.io/github/github-mcp-server"}, "ghcr.io/github/github-mcp-server", "docker"},
 		{"/usr/local/bin/docker", []string{"run", "mcp/slack"}, "mcp/slack", "docker"},
+		// Non-executor first tokens name no published package: the spec is
+		// empty and ScanConfig falls back to the server id, like `uv run
+		// <script>` below. The case body is identical for npm/pnpm/yarn/bun,
+		// so one row per distinct shape suffices.
+		//
+		// `run <script>` — incl. the reported `bun run … start` from the
+		// official Claude messaging plugins, which had leaked package "start".
+		{"bun", []string{"run", "--cwd", "/x", "--shell=bun", "--silent", "start"}, "", ""},
+		{"npm", []string{"run", "start"}, "", ""},
+		// Bare `<script>`: an npm lifecycle alias and a plain script. The gate
+		// returns at the first positional, so trailing args are never read.
+		{"npm", []string{"start"}, "", ""},
+		{"yarn", []string{"dev"}, "", ""},
+		// `bun <file>` / `bun run <file>`: bun executes the file directly. The
+		// path passes looksLikePackageSpec and previously leaked as the package.
+		{"bun", []string{"server.ts"}, "", ""},
+		{"bun", []string{"run", "src/index.ts"}, "", ""},
+		// Accepted false negative, pinned as deliberate: a bare
+		// `yarn <installed-bin>` is indistinguishable from a script without
+		// reading package.json, so it also falls back to the server id — the
+		// same conservative choice as `uv run`.
+		{"yarn", []string{"mcp-server-github"}, "", ""},
+		// create/init DO name a published create-<name> package, but resolving
+		// that is intentionally out of scope (initializers don't launch MCP
+		// servers); pinned so the fallback is deliberate, not an accidental drop.
+		{"npm", []string{"create", "vite"}, "", ""},
+		{"npm", []string{"init", "foo"}, "", ""},
+		// Known limitation: a value-taking global flag NOT in npmValueTakingFlags
+		// before the subcommand shifts detection, missing a genuine `exec`
+		// package. Does not occur in real MCP configs; documented, not desired.
+		{"npm", []string{"--unknownflag", "val", "exec", "real-pkg"}, "", ""},
+		// Value-taking flags before the subcommand are consumed by the gate, so
+		// a credential-bearing registry URL is neither the gate token nor the
+		// package.
+		{"npm", []string{"--registry", "https://t@reg.example.com/", "exec", "@scope/pkg"}, "@scope/pkg", ""},
+		// `exec` fetches a published package only under npm (`npm exec`/`npm x`).
+		// pnpm `exec` runs a locally installed bin; yarn `exec` and bun `exec`
+		// run a shell command. Their first token is a binary or script, not a
+		// package — and `node` is a real npm package, the same failure class
+		// as `start`.
+		{"npm", []string{"x", "left-pad"}, "left-pad", ""},
+		{"pnpm", []string{"exec", "node", "server.js"}, "", ""},
+		{"yarn", []string{"exec", "node", "dist/index.js"}, "", ""},
+		{"bun", []string{"exec", "node server.js"}, "", ""},
 	}
 	for _, c := range cases {
 		gotSpec, gotLauncher := inferPackageFromArgs(c.cmd, c.args)
@@ -345,6 +389,79 @@ func TestScanConfig_UVRunDirectory(t *testing.T) {
 	}
 }
 
+// TestScanConfig_RealWorldCorpus runs the parser end to end over command/args
+// shapes representative of real MCP configurations (the modelcontextprotocol
+// servers, the Claude/Cursor setup guides, the official Claude bundled-server
+// plugins) plus a few script-runner shapes. It is the regression guard for
+// the script-runner change: genuine package launchers (npx/uvx/docker) keep
+// their package identity, while local-script launchers (`bun run … start`,
+// `bun run <file>`, bare `yarn <script>`, `yarn exec <bin>`, `node <file>`,
+// `npm create`) fall back to the server id at low confidence with no
+// requested_spec, rather than leak a script, file, or bin token as a package.
+func TestScanConfig_RealWorldCorpus(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "mcp.json")
+	body := `{
+  "mcpServers": {
+    "seq-think":  {"command":"npx","args":["-y","@modelcontextprotocol/server-sequential-thinking"]},
+    "omnisearch": {"command":"npx","args":["-y","mcp-omnisearch"]},
+    "sqlite":     {"command":"uvx","args":["mcp-server-sqlite","--db-path","test.db"]},
+    "chronulus":  {"command":"uvx","args":["chronulus-mcp"]},
+    "github":     {"command":"docker","args":["run","-i","--rm","ghcr.io/github/github-mcp-server"]},
+    "local-node": {"command":"node","args":["/home/u/mcp-tools/build/index.js"]},
+    "discord":    {"command":"bun","args":["run","--cwd","/plugins/discord","--shell=bun","--silent","start"]},
+    "yarn-dev":   {"command":"yarn","args":["dev"]},
+    "scaffold":   {"command":"npm","args":["create","vite"]},
+    "bun-file":   {"command":"bun","args":["run","src/index.ts"]},
+    "yarn-exec":  {"command":"yarn","args":["exec","node","dist/index.js"]}
+  }
+}`
+	if err := os.WriteFile(path, []byte(body), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	var out []model.Record
+	s := &Scanner{MaxFileSize: 1 << 20, Emit: func(r model.Record) { out = append(out, r) }}
+	if err := s.ScanConfig(path, model.Record{}); err != nil {
+		t.Fatal(err)
+	}
+	byServer := map[string]model.Record{}
+	for _, r := range out {
+		byServer[r.ServerName] = r
+	}
+	want := map[string]struct {
+		pkg, pm  string
+		fallback bool // server-id fallback: confidence=low, no requested_spec
+	}{
+		// Genuine package launchers — identity preserved.
+		"seq-think":  {"@modelcontextprotocol/server-sequential-thinking", "mcp", false},
+		"omnisearch": {"mcp-omnisearch", "mcp", false},
+		"sqlite":     {"mcp-server-sqlite", "uv", false},
+		"chronulus":  {"chronulus-mcp", "uv", false},
+		"github":     {"ghcr.io/github/github-mcp-server", "docker", false},
+		// Local-script / non-package launchers — fall back to the server id.
+		"local-node": {"local-node", "mcp", true}, // node <file>
+		"discord":    {"discord", "mcp", true},    // claude-plugins-official template: bun run … start
+		"yarn-dev":   {"yarn-dev", "mcp", true},   // bare yarn <script>
+		"scaffold":   {"scaffold", "mcp", true},   // npm create (initializer, out of scope)
+		"bun-file":   {"bun-file", "mcp", true},   // bun run <file>
+		"yarn-exec":  {"yarn-exec", "mcp", true},  // yarn exec runs a local bin, not a package
+	}
+	for id, w := range want {
+		r, ok := byServer[id]
+		if !ok {
+			t.Fatalf("%s: no record emitted", id)
+		}
+		if r.PackageName != w.pkg || r.PackageManager != w.pm {
+			t.Errorf("%s: got package_name=%q package_manager=%q, want %q/%q",
+				id, r.PackageName, r.PackageManager, w.pkg, w.pm)
+		}
+		if w.fallback && (r.Confidence != "low" || r.RequestedSpec != "") {
+			t.Errorf("%s: fallback record got confidence=%q requested_spec=%q, want low/empty",
+				id, r.Confidence, r.RequestedSpec)
+		}
+	}
+}
+
 // TestScanConfig_MalformedJSONEmitsWarn verifies that a malformed MCP
 // config file is surfaced as a warn diagnostic rather than silently
 // swallowed. Operators rely on diagnostics to find configs the scanner