Add MCP package shorthand resolution to trust command

[thebenignhacker]

server-filesystem resolves to @modelcontextprotocol/server-filesystem. 7 new tests, 915 total pass.

[github-actions]

Claude Code Review

Security Review: PR #47 - Add MCP Package Shorthand Resolution to Trust Command

VERDICT: APPROVE

SUMMARY

This PR adds user-friendly shorthand resolution for MCP package names in the trust command (e.g., server-filesystem → @modelcontextprotocol/server-filesystem). The implementation is safe — no security vulnerabilities detected. The code uses simple string prefix checks and concatenation without exec/eval/network calls. Input validation is performed by the trust command's existing fetch-based registry lookup, which already handles malicious input. Seven new tests added, all 915 tests pass.

FINDINGS

None. The review identified zero security or correctness issues after full verification.

---

Detailed Analysis

1. SECURITY — No Vulnerabilities Found

Checked for:

• Command injection (child_process usage): ✅ None present
• Prototype pollution (Object.create, __proto__ manipulation): ✅ None present
• Path traversal (file operations with user input): ✅ No file operations in this module
• Unsafe eval/Function constructors: ✅ None present
• Regex DoS: ✅ Only uses .startsWith() and .slice() (O(1) operations)
• SSRF: ✅ No network calls in this module; trust command already validates package names via fetch to registry

Verification:

• resolveMcpShorthand() uses only deterministic string operations (startsWith, slice, concatenation)
• resolveAndLogMcpShorthand() only writes to stderr (safe output)
• No user input reaches exec(), spawn(), eval(), require(), or file system APIs
• Downstream consumers (trustCheck, trustBatch, parseDepsFile) already sanitize inputs before network/fs operations (lines 4646-4884)

2. CORRECTNESS — Logic Verified

Transformation rules (applied in order):

1. Scoped packages (@foo/bar) → pass through unchanged ✅
2. server-* → @modelcontextprotocol/server-* ✅
3. mcp/server-* → @modelcontextprotocol/server-* ✅
4. mcp-server-* → @modelcontextprotocol/server-* ✅
5. Other names → pass through unchanged ✅

Edge cases tested:

• Empty string: '' → '' (no special handling needed; trust command will reject via existing validation)
• Scoped names: @anthropic/claude-mcp → unchanged ✅
• Regular packages: express → unchanged ✅
• MCP shorthands: All forms correctly resolve ✅

Test coverage: 7 new tests, 100% coverage of resolution logic.

3. QUALITY — No Breaking Changes

Integration points verified:

1. trust command single lookup (line 4987): ✅ Logs resolution to stderr
2. trust --batch (line 4966): ✅ Maps array correctly
3. trust --audit (line 4936): ✅ Preserves other package.json fields via spread operator

CLI contract:

• Exit codes unchanged (trust command behavior preserved)
• JSON output unchanged (only input names affected, output format identical)
• Stderr logging non-breaking (informational only)

Type safety:

• resolveMcpShorthand(name: string): string — correct signature ✅
• All callers pass strings, receive strings ✅

---

Why No Findings?

This PR introduces a pure function for string transformation without side effects (except optional logging). The security boundary is at the trust command's network layer (fetch to registry), which already validates package names and handles malicious input. The shorthand resolver is a preprocessing step that cannot introduce new attack vectors because:

1. It doesn't interact with the file system, child processes, or external services
2. It uses only safe string operations (no regex, no eval, no dynamic property access)
3. Malformed input (e.g., ../../../etc/passwd) passes through unchanged and fails at the registry lookup stage with a 404 error
4. The worst case is a user providing a nonsense string like server-../../../../etc/passwd, which becomes @modelcontextprotocol/server-../../../../etc/passwd — still rejected by the registry API as an invalid package name

The implementation correctly applies defense in depth: input sanitization happens at the boundary where it matters (network request construction), not prematurely.

---

Reviewed 3 files changed (6099 bytes)