fix(runner): the MCP guard tested for real, and the MCP routes tested at all (DEV-2501)

[danielzytohoc]

Closes the two worst findings of the DEV-2501 coverage audit on the MCP endpoints (POST /api/mcp/demos, PATCH /api/mcp/demos/:id — PRs #170/#177, ADR-0033).

G1 — a security control asserted against a copy of itself

The test "only demos the MCP created are updatable through it" declared its own local predicate:

const fromMcp = (row) => Boolean(row.forked_from?.startsWith("mcp:"));

and asserted against that, not against the route. Deleting the containment guard from index.ts left the test green — the test could not fail when the control it names went away, which is the defining property of a hollow test.

Fixed in three layers:

• The predicate now lives in production code: isMcpCreated() in workers/api/src/mcp-create.ts, with JSDoc naming it as the containment control from the PR feat(runner): update a demo in place from the MCP (DEV-2501) #177 security review (a leaked shared secret must never reach demos built in the browser).
• The PATCH /api/mcp/demos/:id route calls it, and the rewritten test imports it (same cases as before: mcp:javascript/mcp:react pass; catalog:javascript, null/undefined, {} refuse).
• A source-grep test (in the style of pipeline/theme-codegen.test.mjs) asserts the route's source matches /isMcpCreated\(/ and does not match an inline forked_from?.startsWith — so the check cannot silently drift back to a re-inlined copy that diverges from what the tests import.

G2 — no route-level tests at all

Nothing imported the router (workers/api/src/index.ts), so every status code and response shape it promises was untested. New spec pipeline/mcp-routes.test.mjs drives the real default export — request in, Response out — under the existing plain node --test runner.

How it loads: module.register() hooks (pipeline/fixtures/worker-hooks.mjs) map the worker's .js import specifiers onto its .ts sources, and stand a structural stub in for @cloudflare/sandbox, whose cloudflare: scheme imports only exist inside workerd. node --test isolates each spec in its own process, so the hooks never touch other specs. Bindings are in-memory fakes for exactly what the two routes use (D1 with a recorded write log, KV, R2); the create path is steered through createDemo()'s build-cache-hit branch — itself production code — so no test boots a container, and the sandbox stub throws if one is ever requested.

Acceptance criteria now proven at route level:

• required-description 400, with an assertion that no D1 write and no R2 put happened;
• 201 create shape: body keys exactly {id, url, embedUrl, editUrl, shareUrl, createdBy} with url === /d/:id etc. — this one looked unreachable (createDemo runs a container build) but the build-cache branch made it testable without any product change;
• the D1 insert carries created_by = <asserted author> and forked_from starting mcp:, and that row matches demoListQuery("mine", …) — the audit's G5-router variant;
• 403 for someone else's demo, even with a valid secret;
• 403 for a browser-made demo, with the pointer to /edit/:id;
• 410 for a revoked demo (and no write, no artifact — gone, not rebuilt); 404 for an unknown id.

Nothing on the list had to be skipped.

Verification

• pnpm test — 417/417 green (was 410); pnpm typecheck — clean.
• Guard-bite check, both layers: re-inlining the forked_from check fails the source-grep test; neutering the guard behind false && (invisible to grep) fails the route-level 403 test. Both mutations reverted.
• npx wrangler deploy --dry-run --containers-rollout=none in workers/api — passes. The bare --dry-run stops at the container image build because Docker isn't installed on this machine; identical error on untouched master, i.e. pre-existing and environmental, and the Worker bundle itself builds either way.

---

Note

Low Risk
Production change is a refactor of an existing MCP update guard to a shared helper; risk is low and offset by new route-level tests.

Overview
Closes DEV-2501 audit gaps on MCP create/update by making the containment guard (forked_from must start with mcp:) a shared isMcpCreated() in mcp-create.ts, wiring PATCH /api/mcp/demos/:id to call it instead of an inline check, and fixing mcp-create.test.mjs so it imports that predicate plus a source-grep that the route still calls isMcpCreated()—so removing the guard can fail tests.

Adds mcp-routes.test.mjs, the first spec that drives the real index.ts router via worker.fetch, using worker-hooks.mjs (.js→.ts resolution, @cloudflare/sandbox stub) and fake D1/KV/R2. It covers create validation (400 without description, no writes), 201 response contract, owner/forked_from stamping, and update refusals (403 wrong owner or browser demo, 410 revoked, 404 missing id).

^{Reviewed by Cursor Bugbot for commit c697206. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit c697206. Configure here.}

[cursor]

Hollow MCP guard source-grep

Medium Severity

The structural check that the PATCH MCP route still calls isMcpCreated( also matches the nearby comment that already contains that same text. Removing the real call while leaving the comment keeps this assertion green, which is the hollow-control failure mode this PR set out to close for the containment guard.

Additional Locations (1)

• runner/workers/api/src/index.ts#L775-L776

 

^{Reviewed by Cursor Bugbot for commit c697206. Configure here.}