Skip to content

Commit c087de7

Browse files
authored
fix(scanner): classify Windows npx.cmd/uvx.exe as package runners (#1779)
## Problem The security scanner's source resolver gets the runner name from `filepath.Base(info.Command)` and compares it exactly against `npx`, `uvx`, `pnpm` and so on. On Windows the command is commonly `npx.cmd`, `uvx.exe` or `pnpm.cmd`, so: - `isPackageRunnerCommand` returns false and the package-cache-first lookup is skipped. Arg scanning can then pick a data-dir argument (for example the filesystem server's allowed root) as the server source, which is the case that ordering exists to prevent. - The published-package fetch (`resolveFromPackageFetch`) is gated on the same check, so a quarantined server falls back to a tool-definitions-only scan. - `npxTargetPackage` and `uvxTargetPackage` return "". ## Fix Add `runnerBase(command)`, which returns the lowercased base name with a `.exe`, `.cmd` or `.bat` suffix removed. Use it everywhere the runner name is derived: `isPackageRunnerCommand`, `npxTargetPackage`, `uvxTargetPackage`, `resolveFromPackageCache`, the two `runnerPackageSpec` calls, and the package-fetch dispatch. Unsuffixed names are unchanged. ## Test `TestPackageRunner_WindowsExecutableSuffix` covers `npx.cmd`, `NPX.CMD`, `uvx.exe` and `pnpm.cmd dlx`, a non-runner `node.exe`, and the npx/uvx target-package helpers. On main the four runner checks and both target lookups fail; with this change they pass. `go test -count=1 ./internal/security/scanner/` passes. Made with [Cursor](https://cursor.com)
1 parent 6ec9414 commit c087de7

3 files changed

Lines changed: 44 additions & 7 deletions

File tree

‎internal/security/scanner/package_fetch.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ func writeArchiveFile(target string, src io.Reader, limit int64) (int64, error)
575575
// the toolchain is missing, the host is offline, or the fetch/extract fails —
576576
// guaranteeing no regression versus today's behavior.
577577
func (r *SourceResolver) resolveFromPackageFetch(ctx context.Context, info ServerInfo) (*ResolvedSource, error) {
578-
cmdBase := strings.ToLower(filepath.Base(info.Command))
578+
cmdBase := runnerBase(info.Command)
579579
spec := runnerPackageSpec(cmdBase, info.Args)
580580
if spec == "" {
581581
return nil, fmt.Errorf("no package spec in args for %s", info.Name)
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package scanner
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
// Windows configs name package runners with their executable suffix (npx.cmd,
10+
// uvx.exe). Missing them skips the package-cache-first path, so a data-dir
11+
// argument can be scanned as server source, and the published-package fetch
12+
// never runs.
13+
func TestPackageRunner_WindowsExecutableSuffix(t *testing.T) {
14+
fs := []string{"-y", "@modelcontextprotocol/server-filesystem", "/data"}
15+
16+
assert.True(t, isPackageRunnerCommand("npx.cmd", fs))
17+
assert.True(t, isPackageRunnerCommand("NPX.CMD", fs))
18+
assert.True(t, isPackageRunnerCommand("uvx.exe", []string{"mcp-server-fetch"}))
19+
assert.True(t, isPackageRunnerCommand("pnpm.cmd", []string{"dlx", "some-mcp"}))
20+
assert.False(t, isPackageRunnerCommand("node.exe", []string{"server.js"}))
21+
22+
assert.Equal(t, "@modelcontextprotocol/server-filesystem",
23+
npxTargetPackage(ServerInfo{Command: "npx.cmd", Args: fs}))
24+
assert.Equal(t, "mcp-server-fetch",
25+
uvxTargetPackage(ServerInfo{Command: "uvx.exe", Args: []string{"mcp-server-fetch"}}))
26+
}

‎internal/security/scanner/source_resolver.go‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,19 @@ func (r *SourceResolver) Resolve(ctx context.Context, info ServerInfo) (*Resolve
342342
// `bun server.ts` as a runner by name alone would route a local invocation into
343343
// the package cache/fetch path and scan the wrong token. We decide this up front
344344
// rather than relying on a later parser failure.
345-
func isPackageRunnerCommand(command string, args []string) bool {
345+
// runnerBase returns a command's lowercased base name without a Windows
346+
// executable suffix, so npx.cmd and uvx.exe classify as npx and uvx.
347+
func runnerBase(command string) string {
346348
base := strings.ToLower(filepath.Base(command))
349+
switch filepath.Ext(base) {
350+
case ".exe", ".cmd", ".bat":
351+
base = strings.TrimSuffix(base, filepath.Ext(base))
352+
}
353+
return base
354+
}
355+
356+
func isPackageRunnerCommand(command string, args []string) bool {
357+
base := runnerBase(command)
347358
switch base {
348359
case "npx", "uvx", "bunx":
349360
return true
@@ -853,7 +864,7 @@ func extractNpxPackageDir(path, targetPkg string) string {
853864
// stripped) that an npx-based server launches. Returns "" if the command is
854865
// not npx or no package name can be determined.
855866
func npxTargetPackage(info ServerInfo) string {
856-
if info.Command == "" || filepath.Base(info.Command) != "npx" {
867+
if info.Command == "" || runnerBase(info.Command) != "npx" {
857868
return ""
858869
}
859870
name, _ := parsePackageSpec(runnerPackageSpec("npx", info.Args))
@@ -865,7 +876,7 @@ func npxTargetPackage(info ServerInfo) string {
865876
// Git URLs (git+https://...) are reduced to the repo name. Returns "" if the
866877
// command is not uvx or no package name can be determined.
867878
func uvxTargetPackage(info ServerInfo) string {
868-
if info.Command == "" || filepath.Base(info.Command) != "uvx" {
879+
if info.Command == "" || runnerBase(info.Command) != "uvx" {
869880
return ""
870881
}
871882
// runnerPackageSpec skips `--with <dep>` / `-p <python>` values so the target
@@ -1364,7 +1375,7 @@ func (r *SourceResolver) EnrichWithFileList(resolved *ResolvedSource) {
13641375
// resolveFromPackageCache attempts to find server source in local package manager caches.
13651376
// Supports npx (npm), uvx (uv/pip), and pipx.
13661377
func (r *SourceResolver) resolveFromPackageCache(ctx context.Context, info ServerInfo) (*ResolvedSource, error) {
1367-
cmdBase := filepath.Base(info.Command)
1378+
cmdBase := runnerBase(info.Command)
13681379

13691380
switch cmdBase {
13701381
case "npx":
@@ -1380,7 +1391,7 @@ func (r *SourceResolver) resolveFromPackageCache(ctx context.Context, info Serve
13801391
func (r *SourceResolver) resolveNpxCache(info ServerInfo) (*ResolvedSource, error) {
13811392
// Extract the target package spec (handles `npx -p <pkg>`, `pnpm dlx <pkg>`,
13821393
// version pins, etc.) then split name from any exact version pin.
1383-
spec := runnerPackageSpec(strings.ToLower(filepath.Base(info.Command)), info.Args)
1394+
spec := runnerPackageSpec(runnerBase(info.Command), info.Args)
13841395
if spec == "" {
13851396
return nil, fmt.Errorf("no package name found in npx args")
13861397
}
@@ -1563,7 +1574,7 @@ func (r *SourceResolver) resolveUvxCache(info ServerInfo) (*ResolvedSource, erro
15631574
// `uvx --from <pkg> <cmd>`, `uvx git+<url>`, and crucially skips `--with <dep>`
15641575
// / `-p <python>` values so the additional dependency or python version is not
15651576
// mistaken for the target (MCP-2445).
1566-
pkgName := runnerPackageSpec(strings.ToLower(filepath.Base(info.Command)), info.Args)
1577+
pkgName := runnerPackageSpec(runnerBase(info.Command), info.Args)
15671578
if pkgName == "" {
15681579
return nil, fmt.Errorf("no package name found in uvx args")
15691580
}

0 commit comments

Comments
 (0)