Nothing capped what a caller could send in

[adamjohnwright]

On the way to publishing this server through the website's nginx.

installToolWrapper caps what a tool call returns, and argues for doing it in one place — "a per-tool guard is a guard somebody forgets to add to the fifty-seventh". Nothing capped the other direction.

The hole

identifiers: z.array(nonEmptyString).describe("List of gene symbols, ...")
// ...
analysisClient.postIdentifiers(endpoint, identifiers.join("\n"), { ... })

No maximum, straight into a POST to the Analysis Service, which does real work and stores a result against a token. A few bytes of MCP request could commission an arbitrarily large job, and there is no ceiling anywhere else in the path. Private, that is academic. Behind a public nginx it is an amplification.

reactome_filter_analysis_pathways had the same shape; the export and search filters were smaller versions of it.

The change

Every list argument gets an explicit ceiling: 10,000 identifiers (MCP_MAX_ANALYSIS_IDENTIFIERS, above a whole human proteome and adjustable per deployment), 1,000 pathways, 100 export selections, 50 per search filter.

The part that will still be true next year

An input ceiling cannot live in the wrapper the way the output cap does — only the argument knows what a sane length is. So the requirement lives in a test instead: tests/input-bounds.test.ts walks every registered tool and fails on any argument that accepts an absurd array. A fifty-seventh tool with an unbounded list fails it without anyone remembering this PR exists.

It earned that immediately. I capped five arrays by grepping z.array, and it caught a sixth — reactome_search.types, written as z\n .array(...) across two lines. Grep matches how code is spelled; the sweep asks what the schema accepts.

It probes by parsing, not by reading zod internals, so it survives a zod upgrade and cannot pass by misreading a private field. It does not reach an array nested inside an object argument — no tool has one, and the file says so rather than implying coverage it lacks.

Adversarial review, before opening this

The first version put .min(1) on every list, including the optional search and export filters. Those were z.array(...).optional(), so a client sending [] to mean "no filter" worked today. I would have turned that into a validation error while bounding nothing — a new way to fail with no harm prevented. Required bodies keep the floor (an empty identifier list asked the service to enrich nothing); optional filters still accept [], pinned by its own test.

Verification

Sabotage, each against the test written for it:

• unbound the identifier list → three tests fail, including the sweep, which names reactome_analyze_identifiers.identifiers
• reintroduce the floor on optional filters → exactly the empty-list test fails

One earlier sabotage attempt produced invalid TypeScript and vitest reported "no tests". That proves nothing, so it was redone as valid code — a sabotage that fails to compile is not a passing check.

122 tests, lint/format/typecheck/build clean under node:22.

🤖 Generated with Claude Code

---

Second commit: the cap was unreachable over HTTP

The website session measured the deployed image and found the transport refuses a body over 100 KiB, so the 10,000-identifier cap above could never be hit. Reproduced here independently against a real server:

 5,000 identifiers =  90,125 bytes -> 400   (parsed, then refused for having no session)
 6,000 identifiers = 108,125 bytes -> 413
boundary           = 102,400 bytes         = express's `100kb`, to the byte
10,000 identifiers = 180,125 bytes -> 413

A caller at exactly the documented cap got a bare 413 from express before any validation ran. The error named the wrong thing, and the cap was undeliverable by the transport this server is deployed behind.

express.json({ limit: "4mb" }) in startHttpServer never ran. createMcpExpressApp mounts express.json() at its own line 29, and body-parser skips a body that has already been read, so the parser behind it never sees a request. That line was worse than absent — it was the number anyone reading the file would have believed, and four times larger than what applies. It is gone, replaced by the real ceiling and how it was measured.

Two body parsers where one was intended, and the redundant one is the invisible one: it is in a dependency, it is doing its job correctly, and the code that looks authoritative is the code that does nothing.

MAX_ANALYSIS_IDENTIFIERS is now 3,000, chosen to fit: at 20 characters per identifier that is ~69 KB, a third of the ceiling spare. A caller at the cap now gets a validation error naming the limit.

4,000 was tried first and left 10% headroom; the headroom assertion rejected it. A cap that only just fits is one identifier-length change away from being unreachable again.

tests/body-limit.test.ts asserts the relationship rather than either number — it fails if the cap rises past what the transport carries, or if the transport tightens beneath the cap. The byte ceiling is express's default reached through the SDK, so without this nothing in the repo would notice an upgrade moving it.

Verified by restoring the 10,000 cap: the at-the-cap test fails with 413, the headroom test names 230,125 bytes. The over-the-cap control stays green, as it must — which is why it is not the only test.

128 tests, lint/format/typecheck/build clean.

---

Adversarial review of the two commits above

1. The justification was overstated on the transport that matters

I said an uncapped identifier list was "an amplification with no ceiling anywhere in the path." Measured against main as it stood, before any of this:

12,000 identifiers =  84,125 bytes -> 400  (accepted)
15,000 identifiers = 105,125 bytes -> 413

HTTP was already bounded near 14,600 short identifiers, by express's 100 KiB default. The public transport — the one the whole exposure argument was about — had a ceiling the entire time. And the ceiling I later called "not a number anyone chose" was the only thing actually bounding the amplification I was alarmed about. What was genuinely unbounded is stdio, which is not the public transport.

The cap still earns its place, more modestly than claimed: over HTTP it makes the bound predictable — 3,000 regardless of identifier length, rather than somewhere between 4,000 and 14,600 depending on how long they happen to be — and turns an opaque 413 into a validation error naming the limit. Over stdio it is the only bound there is. Both source comments now say that instead of the stronger thing.

2. The sweep walked 59 of 62 tools

It called registerAllTools once with no environment, so the three Cypher tools — which need NEO4J_URI and MCP_ALLOW_CYPHER — were never swept. A check whose entire value is completeness, quietly covering a subset.

It is also the same shape as the bug that started this whole thread: the thing that varies by configuration, checked in one configuration.

It now merges both configurations and asserts it sees 62 tools and three Cypher names, so the coverage claim fails rather than shrinks. Verified by adding an unbounded array to reactome_cypher_schema — the widened sweep catches it, and the previous version could not have seen it at all.

3. The cap/transport agreement was guarded only at the default

MCP_MAX_ANALYSIS_IDENTIFIERS can raise the cap at runtime, putting the two ceilings back into disagreement on a deployment no test runs against, with a bare 413 as the only symptom. startHttpServer now checks the configured value and warns, naming both numbers and the effect.

It uses an estimate rather than a serialised request, so an estimate that under-stated the body would stay silent on exactly the misconfiguration it exists to report. Three tests hold it: never under-states a real request at the cap, fires on 10,000 (the cap that was actually wrong), stays quiet on the one in use.

Confirmed against the built image:

MCP_MAX_ANALYSIS_IDENTIFIERS=10000 -> warn ... worstCaseBodyBytes: 250256, bodyLimitBytes: 102400
default                            -> silent

131 tests, lint/format/typecheck/build clean.