fix(scanner): spec-parsing + version-pin correctness in package-runner source resolution (MCP-2445)#677
Conversation
Deploying mcpproxy-docs with
|
| Latest commit: |
312b263
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5d7db630.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://mcp-2445-scanner-spec-parsin.mcpproxy-docs.pages.dev |
CI note — pre-existing
|
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27539944945 --repo smart-mcp-proxy/mcpproxy-go
|
Dumbris
force-pushed
the
mcp-2445-scanner-spec-parsing
branch
2 times, most recently
from
1097eaa to
9a7f874
Compare
|
codex review (gpt-5.5) → REQUEST_CHANGES (independently re-verified) Blocking bug: Fix: only treat the first positional as a package spec when the runner subcommand keyword is present ( |
Dumbris
force-pushed
the
mcp-2445-scanner-spec-parsing
branch
from
9a7f874 to
0041dfd
Compare
|
codex RE-REVIEW (gpt-5.5) → REQUEST_CHANGES (round 2 — the
Add tests for |
…urce resolution The scanner mis-parsed several package-runner launch commands and ignored requested version pins, so it scanned the wrong source (or none). - Add runnerPackageSpec(): a command-aware parser that understands subcommand runners (`pipx run X`, `pnpm dlx X`, `yarn dlx X`, `bun x X`), package-naming flags (`npx --package/-p X`, `uvx --from X`), and skips non-target value flags (`uvx --with <dep>`, `-p/--python <ver>`, `-c <cmd>`). Replaces firstPackageArg and the ad-hoc loops in resolveNpxCache/resolveUvxCache/npx+uvxTargetPackage. - Enable pnpm/yarn in isPackageRunnerCommand so the pnpm fetch dispatch (a dead branch before) actually runs; wire bun/yarn through the npm fetch path too. - Honor an exact version pin in the local caches: resolveNpxCache prefers the package.json-version match, findUvxArchiveDir prefers the .dist-info-version match, and the container npx locate script prefers the matching bucket — all falling back to newest/first when unpinned or unmatched. - Bound the published-source fetch (download + extract) with a 90s timeout so a hung registry degrades to tool-definitions-only instead of stalling the scan. - Pin cisco-ai-mcp-scanner==4.7.3 in the cisco scanner Dockerfile (reproducible, rug-pull guard). TDD: table tests for the parser (pipx run / pnpm dlx / uvx --with / npx -p), version-pinned cache selection (npx + uvx archive), and the container locate script (run under /bin/sh). `go test ./internal/security/... -race` green. Related MCP-2445
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. Flag-bypass: runnerPackageSpec parsed package flags (-p/--package/--from) BEFORE enforcing the dlx/x/run subcommand, so `pnpm -p start` / `bun -p server.ts` still produced a spec. Now the required subcommand keyword is enforced FIRST (skipping only leading global flags); flag parsing happens only on the args that follow it. 2. Argument-aware classification: isPackageRunnerCommand classified bare pnpm/yarn/bun/pipx as runners by name alone. It now takes args and treats them as runners only when the ephemeral-run keyword is actually present (npx/uvx/bunx remain runners by name). Decided up front, not via a later parser failure. 3. Version-pin miss never substitutes: when an exact pin is requested but absent from a local cache, resolveNpxCache, findUvxArchiveDir, the uv tools dir (Strategy 2), and the container npx locate script all now resolve NOTHING instead of falling back to a mismatched cached version (false coverage). The published-source fetch then gets the pinned version. Also fixed cleanPkg to strip PEP 440 specifiers (pkg==1.0), not just @ver. TDD: pnpm -p start / bun -p server.ts -> no spec; isPackageRunnerCommand arg-aware table; npx + uvx-archive + uv-tools-dir pin-miss -> not resolved; container locate pin-miss -> empty; exact-pin hits + unpinned still resolve. go test ./internal/security/... -race green; golangci v2 0 issues. Related MCP-2445
Dumbris
force-pushed
the
mcp-2445-scanner-spec-parsing
branch
from
0041dfd to
41e9a59
Compare
|
codex RE-REVIEW (gpt-5.5) → REQUEST_CHANGES (round 3 — the 3 prior blockers are now FIXED ✅: subcommand-keyword-first parsing, argument-aware classification, host-side npx/uvx pin-miss returns error). One new in-scope blocker: Container-side version match uses unescaped |
Codex round-3 (MCP-2445): npxContainerLocateScript embedded the requested
version inside a `grep -E` pattern, so regex metacharacters were active —
a pin like `1.0.0-alpha.1` matched a cached `1.0.0-alpha-1` ('.' as wildcard),
substituting the WRONG cached package despite the host-side pin-miss guard.
Fix: extract the package.json "version" VALUE (grep matches only the key; the
value is captured as "[^\"]*", never the pin), then compare it to the pin with
a LITERAL shell string test (`[ "$v" = '<pin>' ]`). No regex over the version.
The `if`-form keeps a no-match iteration's exit status 0 (clean empty result).
TDD: cached `1.0.0-alpha-1` + pin `1.0.0-alpha.1` -> no match (literal); exact
`1.0.0-alpha-1` pin still resolves; prior pin-hit/miss/unpinned cases unchanged.
go test ./internal/security/... -race green; golangci v2 0 issues.
Related MCP-2445
![mcpproxy-gatekeeper[bot]](https://avatars.githubusercontent.com/u/216428285?s=60&v=4){kind=small}
There was a problem hiding this comment.
codex ACCEPT (gpt-5.5, reviewed in-checkout): subcommand-keyword-first parsing (pnpm -p start / bun -p server.ts do not resolve), argument-aware classification, host pin-miss errors instead of substituting, and container npx version match now LITERAL (test asserts 1.0.0-alpha.1 != 1.0.0-alpha-1). All 4 review rounds' findings closed; no regression.
2 participants
Summary
Fixes a cluster of correctness bugs in scanner source resolution (Codex scanner audit, PRs #653–#670). The scanner mis-parsed several package-runner launch commands and ignored requested version pins, so it scanned the wrong source (or degraded to tool-definitions-only).
Refs against
origin/main.Bugs fixed
pipx run X→"run",pnpm dlx X→"dlx"(returned the subcommand, not the package).resolveFromPackageFetchdispatched pnpm butisPackageRunnerCommandexcluded it → pnpm never reached the fetch.resolveNpxCache/findUvxArchiveDir/findContainerTargetDirpicked newest-mtime, ignoring the requested version.--withmis-handled.resolveUvxCacheonly honored--from;uvx --with <dep> <pkg>resolved the extra dep, not the target.docker/scanners/cisco/Dockerfileinstalledcisco-ai-mcp-scannerunpinned.Changes
runnerPackageSpec()parser: subcommand runners (pipx run,pnpm dlx,yarn dlx,bun x), package-naming flags (npx --package/-p,uvx --from), and skips non-target value flags (uvx --with <dep>,-p/--python <ver>,-c <cmd>). ReplacesfirstPackageArgand the ad-hoc parse loops inresolveNpxCache/resolveUvxCache/npxTargetPackage/uvxTargetPackage(host/container parity).isPackageRunnerCommandnow includespnpm/yarn/bun; npm fetch path dispatches them..dist-infoversion, container npx bucket), falling back to newest/first when unpinned or unmatched.cisco-ai-mcp-scanner==4.7.3(reproducible, rug-pull guard).Tests (TDD)
TestRunnerPackageSpec— table tests:pipx run X,pnpm dlx X,yarn dlx X,bun x X,npx -p,uvx --from,uvx --with,uvx -p/--python→X.TestResolveNpxCacheHonorsPinnedVersion,TestResolveUvxCacheArchiveHonorsPinnedVersion— pinned version selected over newer mismatch.TestResolveUvxCacheWithFlag—--with <dep>does not shadow the target.TestNpxContainerLocateScript— generated/bin/shlocate script run on the host: version pin selects the matching bucket; no pin returns first match.Docs updated in the same PR (
docs/features/security-scanner-plugins.md).Related MCP-2445