Review an MCP server for common security gaps: LLM-facing surfaces as injection vector (tools, resources, prompts, descriptions), scope blast radius, destructive ops without consent, upstream auth shape, input sinks (URL / path / roots / shell / schema strictness / ReDoS), tenant isolation, leakage through errors and telemetry, unbounded resources, and HTTP-mode deployment surface. Use before a release, after a batch of handler changes, or when the user asks for a security review, audit, or hardening pass. Produces grouped findings and a numbered options list.
Review an MCP server for common security gaps: LLM-facing surfaces as injection vector (tools, resources, prompts, descriptions), scope blast radius, destructive ops without consent, upstream auth shape, input sinks (URL / path / roots / shell / schema strictness / ReDoS), tenant isolation, leakage through errors and telemetry, unbounded resources, and HTTP-mode deployment surface. Use before a release, after a batch of handler changes, or when the user asks for a security review, audit, or hardening pass. Produces grouped findings and a numbered options list.
An MCP server is a new attack surface with unique properties — tool output feeds back into the LLM's context, scopes gate what the model can do on the user's behalf, and per-request state must stay tenant-scoped. This skill walks a server through eight axes shaped around what the server builder actually controls. Framework-level concerns (transport, JSON-RPC parsing, auto-correlation, error classification) are out of scope — mcp-ts-core handles those.
Read the code. Don't trust patterns from memory.
When to Use
Before a release
After adding or modifying a batch of handlers or services
Periodically (quarterly-ish)
User asks for a "security review", "audit", "hardening pass", or similar
Inputs
Gather before starting. Ask if unclear:
Scope — whole server, specific module, or recent diff?
Known concerns — anything the user already suspects?
Note: tool / resource / prompt counts, auth mode, storage provider, upstream APIs, which tools have destructiveHint, which handlers request a consent round via ctx.requestInput, which services hold module-scope state, whether the server reads roots.
If transport is streamable HTTP or SSE, also capture:
Bind address (127.0.0.1 for local, or 0.0.0.0 / public interface?)
Session ID source (framework CSPRNG, or builder-supplied?) and binding to auth identity
Any unauthenticated routes (/healthz, /sse, metadata endpoints) — do they leak tool lists or tenant hints?
MCP Authorization spec: if implemented, PKCE enforced, token audience (aud) checked, resource indicators used
If CANVAS_PROVIDER_TYPE=duckdb is set, also capture:
Auth mode — canvas in MCP_AUTH_MODE=none collapses the composite (tenantId, canvasId) scope to ('default', canvasId), where the ID is the only differentiator
Whether external rate limiting (CDN, reverse proxy, WAF) fronts the deployment — required to keep the ~10¹⁸ canvasId keyspace operationally infeasible to brute-force
Use TaskCreate — one task per axis. Mark complete as you go.
Run fuzzTool in parallel.@cyanheads/mcp-ts-core/testing/fuzz catches crashes, memory leaks, and prototype pollution automatically on each tool — start it now so results are ready when you reach Axis 5.
2. Walk the eight axes
Axis 1 — LLM-facing surfaces as injection vector
Anything the server sends to the client that reaches the LLM's context is a potential injection surface: tool output, resource content, prompt text, and the metadata the LLM reads to decide what to call. Relayed upstream content (tickets, scraped text, emails, DB rows) can carry adversarial instructions even when your code is honest.
Look in:
Every *.tool.ts — output schema + format()
Every *.resource.ts — content returned from resources/read
Every *.prompt.ts — templated message content
Every definition file — description, title, annotations, and inputSchema field descriptions (templated from untrusted data?)
Check:
Handlers that return raw upstream text / DB rows without structural framing?
Does format() wrap untrusted content in delimiters (blockquote, fenced code, <data> tags)?
Output schema distinguishes "data" fields from free-form text?
Resource content (resources/read) framed the same way tool output is?
Prompt templates interpolate untrusted data without escaping — treating tenant-controlled strings as trusted instructions?
Tool / resource / prompt descriptions templated from runtime data? Static strings are safer; templated descriptions enable "tool poisoning" (adversarial metadata steering the LLM toward a dangerous tool).
Descriptions mutated mid-session? Rug-pull surface: client approved the v1 description, server now advertises v2 behavior.
Smell:return { body: await fetch(url).then(r => r.text()) } rendered directly in format(). Or: description: \Look up ${tenant.customLabel}`wherecustomLabel` is tenant-supplied.
A single scope covering two capabilities that should be separated (read vs write)?
Read-only tools never require write scopes?
MCP_AUTH_DISABLE_SCOPE_CHECKS=true set in production? When on, both withRequiredScopes and checkScopes early-return — every authenticated user gets every tool, and runtime tenant patterns like team:${input.teamId}:write no longer guard. Acceptable only when paired with a real server-side ACL (path filter, allowlist, upstream API enforcement).
Smell: every tool shares the same scope string. Or: MCP_AUTH_DISABLE_SCOPE_CHECKS=true set without a documented compensating ACL — confirm the deployment relies on a meaningful access control layer below the framework before approving.
Axis 3 — Destructive ops without a consent round
ctx.requestInput moves consent off the LLM and onto the user: the handler returns an input_required result and only runs the side effect once it is re-entered with an accepted response. Destructive tools without that round trust the LLM not to be tricked.
Look in: handlers with destructiveHint: true or side-effecting verbs in names (delete_*, send_*, pay_*, publish_*, drop_*).
Each destructive handler reads ctx.inputs for the confirmation and returns ctx.requestInput(...) before the side effect?
The confirmation response is validated — ctx.inputs.accepted(key, Schema) with a schema, not the bare overload. The SDK never re-validates a response against the schema its request advertised, and the payload is LLM-mediated: "user confirmed" does not mean "user authored these exact fields."
A declined or cancelled response is terminal — checked via ctx.inputs.view(key) and thrown on, never re-asked (a re-ask loops until the round budget runs out) and never treated as consent.
Consent is scoped to the specific target (e.g., record ID rendered in the message), not a generic "proceed?"
Any requestState carried across rounds is integrity-protected if it influences authorization, resource access, or which target gets mutated. It round-trips through the client and comes back attacker-controlled; the SDK does not sign or verify it.
The weak point is answerability, not availability.ctx.requestInput is present on every transport and both protocol eras — the 2025-era shim issues the real elicitation/create round trip, the 2026-07-28 client fulfils the embedded request directly. A client that never retries simply leaves the destructive step un-run, which fails safe. Keep destructiveHint: true so client-side approval flows still surface the risk, and do not accept "proceed anyway when the round is unavailable" as a fallback — there is no such state to detect.
Smell:destructiveHint: true file with no ctx.requestInput in it. Or ctx.inputs.accepted('confirm') with no schema argument — the content could be anything. Or a handler that re-issues the same request after a decline.
Axis 4 — Upstream auth shape
What credentials the server holds, and the blast radius if one leaks.
Each upstream API key scoped to minimum required? (No admin keys for read workflows.)
Services re-mint downstream tokens with correct aud, or passthrough the caller's?
Server holds OAuth for N services × M tenants — what does one-tenant compromise expose?
Per-tenant rate limits on upstream calls?
Smell: one global API_KEY used across all tenants + retry loop with no upper bound.
Axis 5 — Input sinks
LLM-supplied inputs feel internal but aren't. Classic sinks apply, amplified. Sampling responses and roots-derived paths are MCP-specific sinks that look internal but carry LLM/client trust.
Input schemas .strict() — unknown fields rejected, not silently passed to downstream code that destructures with ...rest?
Output schemas without .passthrough() / .catchall() — no accidental exfiltration of fields your schema didn't declare?
Smell:z.string().url() with no allowlist; readFile(input.path) with no canonicalization.
Axis 6 — Tenant isolation
ctx.state is tenant-scoped. Module-scope state is not.
Module-scope Map / Set / cache near tenant-handling code?
Upstream connections pooled per-tenant or shared?
Any code path uses the global logger while carrying per-tenant data (bypassing auto-correlated ctx.log)?
Could tenant B, served after tenant A, read tenant A's cached data?
Smell: service file with top-level const cache = new Map().
Axis 7 — Leakage back
What accidentally reaches the LLM, user, or observability sinks.
Look in:throw new McpError(...) and ctx.fail(reason, msg, data) sites, error factory calls (notFound, httpErrorFromResponse, …), McpError.data fields (the data arg flows through both paths), output schemas, and every logging / telemetry surface — not just ctx.log.
Error data fields (whether passed via ctx.fail(reason, msg, data), new McpError(code, msg, data), or factory calls) carry upstream response bodies, auth headers, stack traces?
httpErrorFromResponse body capture sweeping in too much (default 500-byte cap is fine for most APIs but consider captureBody: false when the upstream returns auth-bearing payloads)?
Output schemas include token prefixes, internal IDs, session identifiers?
format() renders fields that shouldn't leave the server?
ctx.log.info(msg, body) where body is the raw request (may contain secrets)?
OpenTelemetry span attributes / Sentry breadcrumbs carry tokens, PII, or full request bodies?
Secret / token / HMAC comparisons use === or == instead of constant-time (timingSafeEqual / crypto.timingSafeEqual) — leaks length and prefix via timing?
Smell:throw new McpError(code, upstream.message, { raw: upstream.body }) or throw ctx.fail('upstream_failed', e.message, { raw: e.response.body }). Or: if (apiKey === expected) on a request-auth path.
Axis 8 — Resource bounds
Unbounded = DoS of self, upstream, or the LLM's context window (billing-DoS is real).
Look in: handlers with loops, pagination, retries, or inputs that feed JSON.parse / schema validation.
Retry logic has max attempts + exponential backoff?
Output size proportional to input — is there a ceiling?
Tools callable in a loop fail-fast on degenerate input (empty string, 0, null)?
JSON.parse / Zod .parse() inputs have a size + nesting-depth limit applied before parse?
Per-tenant per-tool call rate limit (a single tenant looping delete_record 10k/sec hits you before it hits upstream)?
Concurrency cap on long-running tools so one tenant can't starve the event loop?
Smell:while (cursor) { results.push(...); cursor = next; } with no max count. Or: JSON.parse(await req.text()) with no Content-Length check upstream.
Axis 9 — Canvas (only if CANVAS_PROVIDER_TYPE=duckdb)
DataCanvas is opt-in and deliberately trades isolation for cross-agent token-shareable working sets — designed for public-data tabular servers (BrAPI, OpenAlex, etc.) where session-pinning isn't desired. The trade only holds when the deployment matches that assumption. Skip this axis entirely when canvas is disabled (CANVAS_PROVIDER_TYPE=none, the default).
Data registered on canvases is already public or already-shared-out-of-band. The composite (tenantId, canvasId) scope collapses to ('default', canvasId) in MCP_AUTH_MODE=none — anyone with the canvasId attaches.
External rate limiting (CDN, reverse proxy, WAF) fronts the deployment so the ~10¹⁸ keyspace can't be brute-forced. Without it, the entropy assumption breaks and discovery becomes feasible.
CANVAS_MAX_CANVASES_PER_TENANT sized for the memory budget — default 100 is the floor; raising it lets a single tenant exhaust memory faster.
CANVAS_TTL_MS / CANVAS_ABSOLUTE_CAP_MS not absurdly long. Defaults (24 h sliding / 7 d absolute) are reasonable; longer widens the window an unreferenced canvasId stays guessable.
CANVAS_EXPORT_PATH doesn't point into a shared mount, the repo, or a directory another service serves from. The path-sandbox blocks .. traversal but doesn't prevent the configured root from being a bad choice.
Axis 1 (description templating from canvas-supplied content), Axis 5 (no parallel service runs raw SQL outside the canvas API and bypasses assertReadOnlyQuery), and Axis 7 (errors from canvas operations don't leak the failed SQL string back through McpError.data) all apply.
Smell:MCP_AUTH_MODE=none deployment registering per-user data (recent activity, account state, cart contents) onto a canvas. Or: CANVAS_EXPORT_PATH=/srv/static with a static file server pointing at the same root.
3. Quick sanity pass
Fast, sometimes high-leverage. Outside the eight axes.
bun audit — any direct high/critical?
package.json — postinstall / lifecycle scripts on added deps?
New deps have npm provenance? npm view <pkg> --json | jq .dist.attestations — missing attestation on a security-critical dep is a yellow flag
.env.example — placeholder values only, never real?
Structural gap with clear attacker benefit even without immediate PoC (destructive op without a consent round, admin scope on read tool, SSRF-capable URL input)
medium
Defense-in-depth gap weakening a boundary (missing per-tenant rate limit, error carries upstream response)
If CANVAS_PROVIDER_TYPE=duckdb: Axis 9 — public-data assumption holds, external rate limiting in place, max-canvases-per-tenant + TTLs sized for the deployment, CANVAS_EXPORT_PATH doesn't escape into shared / served paths, assertReadOnlyQuery is the only SQL path
