Skip to content

fix(scanner): npx resolver prefers real package source over tools.json stub (MCP-2397)#661

Merged
Dumbris merged 1 commit into
mainfrom
fix/mcp-2397-npx-resolver-stub
Jun 14, 2026
Merged

fix(scanner): npx resolver prefers real package source over tools.json stub (MCP-2397)#661
Dumbris merged 1 commit into
mainfrom
fix/mcp-2397-npx-resolver-stub

Conversation

@Dumbris

@Dumbris Dumbris commented Jun 14, 2026

Copy link
Copy Markdown
Member

Problem (MCP-2397)

An npx server resolved to ~/.npm/_npx/<hash>/.../server-everything containing only a tools.json stub (3.7 KB), while the real source (~/.npm/_npx/<otherhash>/.../dist/*.js, ~30 files) was never scanned → false coverage ("1 file scanned").

Root cause: resolveNpxCache selected the candidate by newest directory mtime. The same package name exists under multiple npx cache hashes; the tools.json stub (written by mcpproxy itself when dumping tool definitions) has a newer mtime than the real installed source, so the stub won.

Reproduced locally — two cache dirs for @modelcontextprotocol/server-everything:

  • 5b2dd62b…/…/server-everything → 46 files, package.json + dist/ (real)
  • test123/…/server-everything → 1 file (tools.json), newer mtime → was being picked

Fix

Rank candidates by real-source-ness first, falling back to the newest-mtime tiebreak only within the same class:

  • A candidate "looks real" if it has package.json (canonical npm marker), or a dist/lib/build/src subdir, or any .js/.mjs/.cjs/.ts file at its root.
  • Any real candidate beats any stub regardless of mtime.
  • If every candidate is a stub, scan it anyway (prior behavior) but Warn so the incomplete-coverage case is visible in logs.

Tests

  • New TestResolveNpxCachePrefersRealSourceOverStub (TDD): two hashes, stub forced strictly newer via os.Chtimes → asserts the real source dir is chosen. Fails on old code, passes on new.
  • Existing TestResolveNpxCache / …NotFound / npx filesystem-arg tests still green.
  • go test ./internal/security/... -race green; golangci-lint (CI config) 0 issues.

Relation to #658

Distinct and complementary. #658 (MCP-2206, still open) adds a published-source fetch fallback when no local source is found. This PR fixes the local-cache selection that found a stub and used it instead of the better local candidate. No overlap in the changed code path (selection logic vs. new fetch fallback); will re-verify interaction once #658 merges.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 14, 2026

Copy link
Copy Markdown

Deploying mcpproxy-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1849c9e
Status: ✅  Deploy successful!
Preview URL: https://2c4b3f55.mcpproxy-docs.pages.dev
Branch Preview URL: https://fix-mcp-2397-npx-resolver-st.mcpproxy-docs.pages.dev

View logs

@codecov-commenter

codecov-commenter commented Jun 14, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 66.66667% with 13 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/security/scanner/source_resolver.go 66.66% 9 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

…n stub

The npx cache can hold the same package under multiple hashes: one with the
real installed source (package.json + dist/*.js) and another with only a
tools.json stub that mcpproxy writes when dumping tool definitions. The stub's
mtime is usually newer, so resolveNpxCache's newest-mtime heuristic picked it
and the scan reported false coverage (1 stub file instead of ~30 real files).

Rank candidates by real-source-ness first (package.json, a dist/lib/build/src
subdir, or a JS/TS file at the root), falling back to the newest-mtime tiebreak
only within the same class. When EVERY candidate is a bare stub, return an error
instead of the stub so the published-source fetch fallback added in MCP-2206
(#658) fetches the real source — rather than short-circuiting it with false
1-file coverage.

Related MCP-2397

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@Dumbris Dumbris force-pushed the fix/mcp-2397-npx-resolver-stub branch from f730e70 to 1849c9e Compare June 14, 2026 14:25
@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown

📦 Build Artifacts

Workflow Run: View Run
Branch: fix/mcp-2397-npx-resolver-stub

Available Artifacts

  • archive-darwin-amd64 (28 MB)
  • archive-darwin-arm64 (25 MB)
  • archive-linux-amd64 (16 MB)
  • archive-linux-arm64 (14 MB)
  • archive-windows-amd64 (28 MB)
  • archive-windows-arm64 (25 MB)
  • frontend-dist-pr (0 MB)
  • installer-dmg-darwin-amd64 (21 MB)
  • installer-dmg-darwin-arm64 (19 MB)

How to Download

Option 1: GitHub Web UI (easiest)

  1. Go to the workflow run page linked above
  2. Scroll to the bottom "Artifacts" section
  3. Click on the artifact you want to download

Option 2: GitHub CLI

gh run download 27501795447 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.

@mcpproxy-gatekeeper mcpproxy-gatekeeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Claude Code review (Codex out): npx resolver real-source ranking (MCP-2397). Reviewer verified fix + tests + CI green; VERDICT ACCEPT.

@Dumbris Dumbris enabled auto-merge (squash) June 14, 2026 14:56
@Dumbris Dumbris merged commit 28b4143 into main Jun 14, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants