Skip to content

Commit 0041dfd

Browse files
committed
fix(scanner): require subcommand keyword for subcommand-style runners
Codex re-review (MCP-2445): runnerPackageSpec returned the first positional even when the runner's required subcommand keyword was absent, so with pnpm/yarn/bun gated as package runners: pnpm start -> "start" bun server.ts -> "server.ts" pipx install x -> "install" The wrong token was then fetched/scanned = false coverage + typosquat risk. Fix: for subcommand-style runners (pnpm/yarn `dlx`, `bun x`, `pipx run`), a package spec is resolved ONLY when the keyword is present. A bare local invocation (`pnpm start`, `bun server.ts`, `pipx install foo`) now yields "" (no package) and degrades to tool-definitions-only instead of fetching a bogus package. npx/uvx/bunx (no subcommand) are unchanged. TDD: added pnpm start / pnpm run build / bun server.ts / bun run dev / yarn build / pipx install cases (-> ""); pnpm dlx <pkg> / npx -y <pkg> / uvx --from still resolve. Rebased onto origin/main (picks up #676 archive-extraction hardening + the teams/broker -> serveredition/broker move). Related MCP-2445
1 parent 3405e9d commit 0041dfd

2 files changed

Lines changed: 25 additions & 5 deletions

File tree

internal/security/scanner/package_fetch.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,9 @@ func runnerFlagTakesValue(npm bool, flag string) bool {
168168
// returned verbatim) that a package-runner command will execute, from its command
169169
// base and argument list. It understands:
170170
// - a leading runner subcommand keyword (`pipx run`, `pnpm dlx`, `yarn dlx`,
171-
// `bun x`) that precedes the package token;
171+
// `bun x`) that precedes the package token — and which is REQUIRED for those
172+
// runners: a bare `pnpm start` / `bun server.ts` is a local invocation, not a
173+
// remote fetch, and yields "" (no package);
172174
// - flags that NAME the target package — uv `--from <pkg>`, npx `--package`/
173175
// `-p <pkg>` — whose value is returned directly;
174176
// - value-bearing flags whose argument is NOT the target (`--with <dep>`,
@@ -198,10 +200,19 @@ func runnerPackageSpec(commandBase string, args []string) string {
198200
}
199201
continue
200202
}
201-
// First positional token. Skip a leading runner subcommand keyword once.
202-
if !subSkipped && arg == sub {
203-
subSkipped = true
204-
continue
203+
// First positional token. Subcommand-style runners (pnpm/yarn/bun/pipx)
204+
// REQUIRE their keyword (dlx/x/run) before the package token: a bare
205+
// `pnpm start`, `bun server.ts`, or `pipx install foo` is a LOCAL
206+
// invocation, NOT a remote package fetch, so it must not resolve a spec —
207+
// fetching the wrong token would mean false coverage and typosquat risk
208+
// (MCP-2445 re-review). Once the keyword is consumed the next positional
209+
// is the package.
210+
if !subSkipped {
211+
if arg == sub {
212+
subSkipped = true
213+
continue
214+
}
215+
return ""
205216
}
206217
return arg
207218
}

internal/security/scanner/package_fetch_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,15 @@ func TestRunnerPackageSpec(t *testing.T) {
6161
// uvx -p/--python carries a version, NOT the package.
6262
{"uvx -p python", "uvx", []string{"-p", "3.11", "main-pkg"}, "main-pkg"},
6363
{"uvx --python", "uvx", []string{"--python", "3.12", "--from", "main-pkg", "cmd"}, "main-pkg"},
64+
// Subcommand runners WITHOUT the keyword must NOT resolve a package spec —
65+
// these are local invocations, not remote package fetches (MCP-2445 re-review).
66+
// Fetching the first positional here = false coverage + typosquat risk.
67+
{"pnpm start (local script)", "pnpm", []string{"start"}, ""},
68+
{"pnpm run build (local script)", "pnpm", []string{"run", "build"}, ""},
69+
{"bun server.ts (local file)", "bun", []string{"server.ts"}, ""},
70+
{"bun run dev (local script)", "bun", []string{"run", "dev"}, ""},
71+
{"yarn build (local script)", "yarn", []string{"build"}, ""},
72+
{"pipx install foo (not ephemeral run)", "pipx", []string{"install", "foo"}, ""},
6473
// Degenerate inputs.
6574
{"npx flag only", "npx", []string{"-y"}, ""},
6675
{"nil", "npx", nil, ""},

0 commit comments

Comments
 (0)