fix(mcp): only derive a package from executor subcommands#34
Open
alexviktorov wants to merge 1 commit into
Open
fix(mcp): only derive a package from executor subcommands#34alexviktorov wants to merge 1 commit into
alexviktorov wants to merge 1 commit into
Conversation
MCP servers launched through a package-manager script runner had their script name recorded as the package. The bundled Claude plugins use `bun run … start`, which was emitted as package_name "start" — a real npm package, so an advisory for "start" could spuriously match. Bare `yarn dev` / `pnpm build` leaked "dev" / "build" the same way. This is the npm-family counterpart of a case the parser already guards for uv: `uv run <script>` (without --from) returns no package identity because its argument is a script or path, not a published package. The npm family was inconsistently grouped with the package executors, so its `run`/bare-script argument leaked through. Apply the same rule. For npm/pnpm/yarn/bun, only an executor subcommand names a published package — dlx/exec/x (or the separate npx/bunx); the positional after it, or --package, is the spec. Any other first token runs a local script or initializer, not a configured package (`run <script>`, the npm lifecycle aliases start/stop/restart/test, bare `yarn dev`, and the create/init verbs), so return no spec and fall back to the server id at low confidence — exactly as the uv branch already does. Package executors are unchanged: npx/bunx/dlx/exec/x, --package, the -- terminator, and credential-bearing value flags all keep their identity. A full-host scan diff confirms only mcp records change; no other ecosystem is affected. Updates the inferPackageFromArgs doc comment and docs/inventory-sources.md, and adds unit + real-world-corpus tests.
a600a96 to
b05327c
Compare
Author
|
@adel-pplx Let me know if you need any context on this. Basically after scanning my Mac I saw several records listing "start" as the package, where it's evidently a command. Specifically it was for some Claude official plugins. This PR fixes the discrepancy and instead you get the actual names, similar to other places in code. Tested on my local config and generated synthetic unit tests from real world examples. Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
For
npm/pnpm/yarn/bun, derive a package identity only from an executorsubcommand —
dlx/exec/x(or the separatenpx/bunx). Any other first tokenruns a local
package.jsonscript or an initializer, not a configured package, so ityields no spec and the record falls back to the server id at
confidence=low— thesame way
uv run <script>is already handled.Why (the bug)
MCP servers launched through a package-manager script runner had their script
name recorded as the package. The
claude-plugins-officialmessaging plugins launchtheir bundled server like this:
{ "command": "bun", "args": ["run", "--cwd", "${CLAUDE_PLUGIN_ROOT}", "--shell=bun", "--silent", "start"] }A
baselinescan of a machine with those plugins installed emitted:discordstartdiscordimessagestartimessagetelegramstarttelegramfakechatstartfakechatstartis a real npm package, so an exposure catalog entry forstartwouldspuriously match these servers. Bare
yarn dev/pnpm buildleakeddev/buildthe same way.
This is the npm-family counterpart of a case the parser already guards for uv:
uv run <script>(without--from) returns no package identity because its argumentis a script or path, not a published package. The npm family was inconsistently
grouped with the package executors, so its
run/bare-script argument leaked through.This change applies the same rule.
Scope / safety
npx/bunx/dlx/exec/x,--package, the--terminator, and credential-bearing value flags all keep their package identity.baselinescan before/after this change, onlymcprecordsdiffer —
go/npm/pypi/rubygems/editor/browser records are byte-identical.The changed records are exactly the script-launched servers.
confidence: lowconfigured references; the server-idfallback replaces a misleading (and false-positive-prone) package guess with the
configured server alias rather than inventing a package name.
Tests / docs
TestInferPackageFromArgscoveringrun <script>, bare scripts,create/init, and a documented flag-ordering limitation.TestScanConfig_RealWorldCorpusproves the contrast end-to-end: real launchers(
npx/uvx/docker) keep their identity while script/initializer launchers fallback to the server id.
inferPackageFromArgsdoc comment anddocs/inventory-sources.md.go build,go test ./...,go test -race ./...,go vet ./...,gofmt -l .(clean), and
bumblebee selftestall pass.