diff --git a/.specify/memory/constitution.md b/.specify/memory/constitution.md index 26457bc..88fc398 100644 --- a/.specify/memory/constitution.md +++ b/.specify/memory/constitution.md @@ -1,18 +1,22 @@ # Reactome MCP Server Constitution @@ -62,24 +66,36 @@ shipped. ### IV. Reactome data, through the public services -Tools read the Content Service and the Analysis Service over HTTP. The server -holds no Neo4j connection, because there is no code that can open one: the Cypher -tools, the graph schema resource and the `neo4j-driver` dependency were removed on -2026-09-21. +Tools read the Content Service and the Analysis Service over HTTP. **A publicly +reachable instance holds no Neo4j connection.** An internal instance may be +given read-only Cypher access to a dedicated Reactome graph database, and only +when both `NEO4J_URI` and `MCP_ALLOW_CYPHER=1` are set. This is a decision about blast radius, not capability. A public MCP endpoint with -database credentials is a different security proposition from one that can only -make the same calls a browser can. - -**Why removed rather than gated.** This principle was enforced by a gate, which -made it a property of a configuration rather than of the code. The gate was -consulted in four places and one of them — `src/http-server.ts`, the entrypoint -that actually runs in the hosted deployment — still opened a connection on -`NEO4J_URI` alone after the other three were corrected. Neither of the two -consumers needed graph access in any case: the chatbot queries the graph directly -with its own driver when it builds embeddings, and the website needs the -REST-backed tools only. `tests/no-graph-access.test.ts` asserts the absence with -the old switches turned ON, so it cannot pass by being configured off. +database access is a different security proposition from one that can only +make the same calls a browser can: an arbitrary query from the internet can +take the database, and what depends on it, down. + +**History.** Until 2026-09-21 this was enforced by a gate that each surface +consulted for itself. It was consulted in four places and one of them -- +`src/http-server.ts`, the entrypoint that actually runs in the hosted +deployment -- still opened a connection on `NEO4J_URI` alone after the other +three were corrected. So the capability was removed outright. On 2026-10-14 it +was restored for internal use, on a different footing: + +- **One predicate**, `isGraphAccessEnabled()` in `src/graph/gate.ts`, read from + the environment at call time so there is no second copy of the answer. +- **The driver enforces it.** `getDriver()` asks the predicate itself, and + `src/clients/neo4j.ts` is the only file allowed to import `neo4j-driver`. A + surface that forgets to ask still cannot connect. +- **Every door is tested, both ways.** `tests/graph-access-gate.test.ts` drives + the stdio entrypoint, the HTTP entrypoint, `startHttpServer`, `createServer` + and the registrars in every combination of the two variables and counts + driver constructions. A new entrypoint belongs in that matrix. + +What would reverse this: graph access being needed on a public instance, or the +gate failing open anywhere. Either means removing the capability again, not +adding another check. ### V. Analysis runs in the service, not in the agent @@ -114,7 +130,7 @@ for model consumption, transports (stdio today; Streamable HTTP is the open question for hosted use), and the tests and sweeps that keep the above honest. **Out of scope by decision, not by oversight.** Direct Neo4j access from any -deployed instance (Principle IV). Reimplementing analysis (Principle V). +publicly reachable instance (Principle IV). Reimplementing analysis (Principle V). Non-Reactome data sources — the chatbot's cascade reaches Tavily and external search, and that logic belongs in the chatbot, not here. @@ -152,4 +168,4 @@ Amendments bump the version: MAJOR for a principle removed or redefined, MINOR for one added, PATCH for wording. Specs live in `specs/`; the ones that record why something is the way it is are as valuable as the ones that plan what is next. -**Version**: 1.0.0 | **Ratified**: 2026-09-14 | **Last amended**: 2026-09-14 +**Version**: 2.0.0 | **Ratified**: 2026-09-14 | **Last amended**: 2026-10-14 diff --git a/CHANGELOG.md b/CHANGELOG.md index 86fc0f4..6b90e28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ All notable changes to this project are documented here. This project adheres to ### Added +- **Graph database access, restored and off by default.** The three `reactome_cypher_*` tools and the `reactome://graph/schema` resource are back for **internal** instances pointed at a dedicated Reactome graph database container. They exist only when **both** `NEO4J_URI` is set and `MCP_ALLOW_CYPHER=1`; a publicly reachable instance must set neither. + + One predicate, `isGraphAccessEnabled()`, is asked by every surface — tool registration (a new `cypher` tool group), the resource, the server instructions, the startup schema warm-up — and by `getDriver()` itself, which is the only code that imports `neo4j-driver`. With the gate closed no driver is ever constructed, in the stdio entrypoint, the HTTP entrypoint, `startHttpServer` or `createServer`. `MCP_TOOL_GROUPS` naming `cypher` with the gate closed refuses to start. `/health` reports `graphAccess`. + + Schema extraction now uses Neo4j's built-in procedures only (`db.schema.*`, `SHOW INDEXES`/`SHOW CONSTRAINTS` with `db.indexes()`/`db.constraints()` as fallback), because the Reactome graph database image runs Neo4j 4.4 Community without APOC. Each section degrades on its own and is reported as unavailable rather than rendered empty. + + The query guard is now an **allowlist**: `CALL` only for the built-in schema procedures, `SHOW` only for indexes and constraints, and no APOC name, `LOAD CSV`, `USE` or `TERMINATE`. The old denylist let through `apoc.cypher.runMany`/`runFile`/`runSchema`/`parallel`, `apoc.schema.assert`, `apoc.util.sleep`, `apoc.custom.*`, `apoc.systemdb.*`, `apoc.lock.*`, `dbms.killQuery`/`listQueries`/`listConfig` and `USE`, and a comment splitting a keyword (`LOAD/**/CSV`). The check runs on one left-to-right tokenisation (strings, backtick names, line and block comments, Unicode separators), so a comment opener inside a comment or a string can no longer hide text, `CALL \`db\`.\`labels\`()` is allowed, and CALL subqueries and write/admin keywords are refused. Number literals are read whole (`1e0CALL` no longer hides a CALL) and a name running on from one is refused; `gds` is refused like `apoc`; `start`/`stop`/`use` work as variable names; the fulltext query procedures are allowed. The `CYPHER` options prefix is refused, since the pre-parser strips it and a `USE` after it would begin the query. Query rows are streamed and reading stops at `max_rows`, rather than the whole result being read into memory and capped afterwards. A schema section that failed is retried after five minutes instead of staying unavailable for the life of the process. `neo4j-driver` is loaded only once the gate is open. + + `NEO4J_DATABASE` now defaults to the server's default database, and `NEO4J_PASSWORD` to none (connect without credentials), rather than `graph.db` and `neo4j`. + - **`MCP_TOOL_GROUPS` — choose which tools an instance publishes.** All 59 register by default, so a local stdio user is unaffected; a hosted instance names the groups it means to serve (`search`, `pathway`, `entity`, `analysis`, `export`, `interactors`, `gsa`, `utilities`). It is a **registration** switch, not a documentation one. An omitted group's tools are not registered, do not appear in `tools/list`, are not mentioned in the instructions the server sends on connection, and neither are its **resources** — omitting `analysis` withholds `reactome://analysis/{token}` as well, since a capability moved to a URI is not a capability withheld. A tool described nowhere but still answering is the divergence that let this server advertise Cypher tools it had not registered. @@ -16,15 +26,7 @@ All notable changes to this project are documented here. This project adheres to ### Removed -- **BREAKING: all graph database access.** The three `reactome_cypher_*` tools, the `reactome://graph/schema` resource, the Cypher section of the server instructions, the startup schema prefetch and the `neo4j-driver` dependency are gone. `NEO4J_URI`, `NEO4J_USER`, `NEO4J_PASSWORD`, `NEO4J_DATABASE`, `CYPHER_QUERY_TIMEOUT_MS` and `MCP_ALLOW_CYPHER` are inert — nothing reads them. - - **Who this affects:** a curator running this server locally over stdio against `reactome_neo4j_env`, which is who the tools were built for. There is no replacement in this server; query the graph directly, as the Reactome chatbot does with its own driver when it builds embeddings. - - **Why now.** This server is being hosted publicly, behind the website's nginx. Constitution Principle IV already said no deployment holds a Neo4j connection, and enforced it with a gate — which made the property true of a *configuration* rather than of the code. The gate was consulted in four places and one of them, `src/http-server.ts`, the entrypoint that actually runs in the hosted deployment, still opened a connection on `NEO4J_URI` alone after the other three were corrected. Neither consumer of this server needed graph access in the first place. - - The public tool surface is **unchanged at 59** — the Cypher tools were only ever the 60th to 62nd, and only with both switches on. - - `tests/no-graph-access.test.ts` asserts the absence with the old switches turned **on**, so it cannot pass by being configured off. +- ~~**BREAKING: all graph database access.**~~ Reverted before release — see *Graph database access, restored and off by default* above. The tools came back behind a gate that `getDriver()` enforces itself, rather than one each call site had to remember. ### Fixed - **Ten tools read field paths the Reactome services never return.** Each called the right endpoint and reported success, so nothing flagged them: `contentClient.get` asserts `T`, it does not verify it, and 51 of 56 tools had no test. `search_suggest` and `search_spellcheck` expected `{suggestions: []}` where the API returns a bare array; `entity_component_of` expected `Complex` objects where the API returns one entry per relationship type with parallel `names`/`stIds`/`schemaClasses` arrays; `participants` expected `stId` where the API returns `peDbId`; the four interactor tools read `score` one level above where it lives, throwing on `.toFixed`; `analysis_found_entities` read `mapsTo[].identifier` where the API returns `ids[]`. **`search_diagram` had never returned an answer** — it shared the grouped-results helper, but that endpoint returns a flat `entries` array, so every call threw `result.results is not iterable`. diff --git a/README.md b/README.md index b41b161..4eb9b2f 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ An [MCP (Model Context Protocol)](https://modelcontextprotocol.io/) server that - **Species & disease** — list available species and disease annotations - **ID mapping** — map external identifiers (UniProt, Ensembl, CHEBI, etc.) to Reactome pathways and reactions -59 tools and 10 resources are registered — see [Tools](#tools) and [Resources](#resources) below for the full list. `tests/readme-tools.test.ts` fails if that count drifts or a tool goes undocumented. +59 tools and 10 resources are registered by default — see [Tools](#tools) and [Resources](#resources) below for the full list. An internal instance can additionally be given read-only Cypher access to a Reactome graph database, which makes 62 with graph access and 11 resources; it is **off** unless two variables are set, and a public instance must never set them (see [Graph database access](#graph-database-access-internal-instances-only)). `tests/readme-tools.test.ts` fails if those counts drift or a tool goes undocumented. ## Prerequisites @@ -57,6 +57,7 @@ MCP_TOOL_GROUPS=search,pathway,entity,utilities,analysis | `interactors` | 6 | | `gsa` | 5 | | `utilities` | 7 | +| `cypher` | 3 — only with [graph database access](#graph-database-access-internal-instances-only) on | This is a **registration** switch, not a documentation one. An omitted group's tools are not registered, are not listed by `tools/list`, and are not @@ -68,9 +69,94 @@ described nowhere but still answering is the failure this avoids. Three cases behave differently on purpose, because the dangerous one is a restriction that quietly becomes "everything": -- **unset** — all groups +- **unset** — all groups (`cypher` only if graph database access is on) - **set and empty** — the server refuses to start - **set with an unknown name** — the server refuses to start, naming the typo +- **names `cypher` while graph database access is off** — the server refuses to start, rather than quietly serving without it + +## Graph database access (internal instances only) + +Three extra tools — `reactome_cypher_schema`, `reactome_cypher_sample` and +`reactome_cypher_query` — and the `reactome://graph/schema` resource let an +agent query a Reactome Neo4j graph database directly. They exist for internal +instances pointed at a **dedicated** graph database container. + +**They are off by default, and a publicly reachable instance must never turn +them on.** An arbitrary Cypher query can run for a long time and hold the +database's memory and CPU while it does; on a public endpoint that is a way for +anyone on the internet to degrade the database and whatever else depends on it. +The read-only safeguards below limit the damage a query can do to the data, not +the load it can put on the server. + +Graph access exists only when **both** of these are set: + +| Variable | Value | | +|---|---|---| +| `NEO4J_URI` | e.g. `bolt://127.0.0.1:7687` | where the graph database listens | +| `MCP_ALLOW_CYPHER` | exactly `1` | the explicit opt-in; any other value is off | + +Either one alone does nothing: no tools, no schema resource, no mention in the +instructions the server sends on connection, and no database connection is +ever opened. `NEO4J_URI` alone is deliberately not enough — a connection string +is easy to inherit from a shared environment and is not a decision to publish +query access. The same check guards every surface (tool registration, the +resource, the instructions, the startup schema warm-up) and the driver factory +itself, in both the stdio and the HTTP entrypoints; +`tests/graph-access-gate.test.ts` drives each of them in every combination of +the two variables. `GET /health` on the HTTP transport reports `graphAccess`. + +Optional: + +| Variable | Default | | +|---|---|---| +| `NEO4J_USER` | `neo4j` | used only when `NEO4J_PASSWORD` is set | +| `NEO4J_PASSWORD` | _(unset)_ | unset connects without credentials, for a database with authentication disabled | +| `NEO4J_DATABASE` | _(server default)_ | Neo4j Community serves a single database, so this is rarely needed | +| `CYPHER_QUERY_TIMEOUT_MS` | `30000` | server-side transaction timeout for `reactome_cypher_*` queries | + +**Read-only safeguards.** Every query runs in a READ-mode session, so the +database refuses `CREATE`, `MERGE`, `DELETE`, `SET` and `REMOVE`. Before a +query reaches the database it is checked against an allowlist: `CALL` only for +the built-in schema procedures (`db.labels`, `db.relationshipTypes`, +`db.propertyKeys`, `db.schema.*`, `db.indexes`, `db.constraints`, +`dbms.components`) and the fulltext query procedures +(`db.index.fulltext.queryNodes`, `db.index.fulltext.queryRelationships`), no +`CALL { ... }` subqueries, `SHOW` only for indexes and constraints, and no +APOC or GDS name, `LOAD CSV`, `USE`, `TERMINATE`, `CYPHER` options prefix +(version, planner, runtime) or write/admin clause. `SET`, `SHOW` and `CYPHER` +cannot be used as variable names. The check runs on the query as Neo4j tokenises +it (comments, string literals and quoted names read as such), and anything it +cannot finish reading is refused. Each query runs under a server-side timeout; +rows are streamed and reading stops at `max_rows`, with the rest discarded on +the server, and row width and total response size are capped. These are +guardrails; the boundary is that graph access is not enabled where the public +can reach it. + +### Running against the Reactome graph database container + +Reactome publishes its graph database as an image. Run it bound to loopback, so +nothing but processes on the same machine can reach it: + +```bash +docker run -d --name reactome-graphdb \ + -p 127.0.0.1:7687:7687 \ + public.ecr.aws/reactome/graphdb:Release97 +``` + +Then point an internal instance at it: + +```bash +NEO4J_URI=bolt://127.0.0.1:7687 MCP_ALLOW_CYPHER=1 node dist/index.js +``` + +Add `NEO4J_USER` / `NEO4J_PASSWORD` if the container is configured with +authentication. The server reads the schema with Neo4j's built-in procedures +(`db.schema.nodeTypeProperties()`, `db.schema.relTypeProperties()`, +`db.schema.visualization()`, `SHOW INDEXES`, `SHOW CONSTRAINTS`), so APOC is not +required — the Reactome image runs Neo4j 4.4 Community without it. The schema +is read once in the background at startup and cached; on the full graph that +can take a while, and a section the database cannot provide is reported as +unavailable rather than shown empty. ## Usage @@ -235,6 +321,17 @@ Starts a local web UI with an MCP bridge for browser-based exploration. | `reactome_orthology` | Get orthologous events/entities in another species | | `reactome_query` | Query any Reactome database object by identifier | +### Graph database / Cypher (3 tools — internal instances only) + +Registered only when `NEO4J_URI` and `MCP_ALLOW_CYPHER=1` are both set; see +[Graph database access](#graph-database-access-internal-instances-only). + +| Tool | Description | +|------|-------------| +| `reactome_cypher_schema` | Labels, relationship types, connecting patterns, properties, indexes and constraints of the graph | +| `reactome_cypher_sample` | A few nodes of one label, to see their shape | +| `reactome_cypher_query` | Run a read-only Cypher query (READ session, timeout, capped output) | + ## Resources ### Static @@ -245,6 +342,7 @@ Starts a local web UI with an MCP bridge for browser-based exploration. | `reactome://species/main` | Main species with curated pathways | | `reactome://diseases` | All annotated diseases | | `reactome://database/info` | Database version and name | +| `reactome://graph/schema` | Graph database schema as JSON — only with graph database access on | ### Templates @@ -280,6 +378,9 @@ MCP_HTTP_PORT=4320 node dist/http-server.js | `MCP_SESSION_TTL_MS` | `1800000` | idle session reaped after 30 min | | `MCP_MAX_SESSIONS` | `256` | concurrent session ceiling | +A hosted instance must leave `NEO4J_URI` and `MCP_ALLOW_CYPHER` unset; see +[Graph database access](#graph-database-access-internal-instances-only). + Endpoints: `POST /mcp` (initialize, then requests), `GET /mcp` (server stream), `DELETE /mcp` (end session), `GET /health`. diff --git a/SECURITY.md b/SECURITY.md index abc4e03..2346bc4 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -11,8 +11,8 @@ Include, where possible: - A description of the issue and the potential impact - Steps to reproduce (a minimal query, MCP config snippet, or command) -- Whether the issue affects the REST-client path, the HTTP transport, or the web demo -- The version of `reactome-mcp` (see `package.json`) and any relevant runtime versions (Node) +- Whether the issue affects the REST-client path, the HTTP transport, the graph database (Cypher) path, or the web demo +- The version of `reactome-mcp` (see `package.json`) and any relevant runtime versions (Node, Neo4j) You can expect an acknowledgement within a few business days. We will coordinate a fix and, where appropriate, credit you in the advisory. @@ -20,21 +20,24 @@ You can expect an acknowledgement within a few business days. We will coordinate In scope: -- The MCP server (`src/`, `dist/`) and its REST clients. +- The MCP server (`src/`, `dist/`), its REST clients and its optional Neo4j client. - The bundled web demo (`web/`). Out of scope (report upstream): - Vulnerabilities in `https://reactome.org` — report via the Reactome website. +- Vulnerabilities in Neo4j or `neo4j-driver` — report via those projects. - Vulnerabilities in Claude Desktop / Claude Code / the MCP SDK — report to the respective vendor. ## Threat Model (brief) -- The server reads the public Reactome Content and Analysis Services over HTTP and holds no database credentials. As of 2026-09-21 it has **no graph database access at all**: the Cypher tools, the graph schema resource and the `neo4j-driver` dependency were removed when the server began to be hosted publicly. `NEO4J_URI` and `MCP_ALLOW_CYPHER` are inert — nothing reads them. -- It is still **not** self-hardened for internet-facing deployment: it has no authentication, no TLS and no rate limiting of its own. A public deployment must put those at the reverse proxy, and should bind the server to loopback so the proxy is the only way in. What removing graph access changes is the blast radius if that proxy is misconfigured — a caller reaching the server directly can make the same calls a browser can make against reactome.org, and nothing more. +- By default the server reads the public Reactome Content and Analysis Services over HTTP and holds no database credentials. +- **Graph database access is off unless both `NEO4J_URI` and `MCP_ALLOW_CYPHER=1` are set**, and a publicly reachable instance must never set them. With either missing there are no Cypher tools, no graph schema resource, no mention of either in the server instructions, and no Neo4j driver is ever constructed — `getDriver()` checks the same predicate as every surface, so a surface that forgot to check still could not connect. `tests/graph-access-gate.test.ts` holds this for every entrypoint in every combination of the two variables. +- With graph access on, queries run in READ-mode sessions under a server-side timeout, queries are checked against an allowlist before execution (`CALL` only for the built-in schema procedures and no `CALL` subqueries, `SHOW` only for indexes and constraints, no APOC, `LOAD CSV`, `USE`, `TERMINATE` or write/admin clause), checked on a tokenisation of the query rather than its raw text,, and rows are streamed and cut off at the row cap rather than read whole. That is a guardrail against accidents, not a boundary against a hostile caller: an expensive read can still load the database. Use a dedicated database, reachable on loopback only, and configure Neo4j's own authentication and plugin settings accordingly. +- It is **not** self-hardened for internet-facing deployment: it has no authentication, no TLS and no rate limiting of its own. A public deployment must put those at the reverse proxy, and should bind the server to loopback so the proxy is the only way in. With graph access off, a caller reaching the server directly can make the same calls a browser can make against reactome.org, and nothing more. - Tool inputs are bounded: every list argument has an explicit maximum, and `MAX_ANALYSIS_IDENTIFIERS` caps what one call may submit to the Analysis Service. The HTTP transport separately refuses a request body over 100 KiB. - The web demo (`web/mcp-bridge.js`) uses an allow-list CORS policy defaulting to localhost. Do not deploy it publicly without adding authentication. ## Handling of Secrets -- The server holds no credentials of its own. REST error bodies are logged verbatim on failure — if you deploy behind an authenticated proxy, ensure upstream errors do not echo credentials. +- With graph access off the server holds no credentials of its own. `NEO4J_PASSWORD`, when set, is read when the driver is built and passed to it; it is never logged. REST error bodies are logged verbatim on failure — if you deploy behind an authenticated proxy, ensure upstream errors do not echo credentials. diff --git a/package-lock.json b/package-lock.json index 133ea8b..65b2d0f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,6 +11,7 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", "express": "^5.2.1", + "neo4j-driver": "^6.2.0", "zod": "^4.6.5" }, "bin": { @@ -1441,6 +1442,26 @@ "node": "18 || 20 || >=22" } }, + "node_modules/base64-js": { + "version": "1.5.1", + "resolved": "https://registry.npmjs.org/base64-js/-/base64-js-1.5.1.tgz", + "integrity": "sha512-AKpaYlHn8t4SVbOHCy+b5+KKgvR4vrsD8vbvrbiQJps7fKDTkjkDry6ji0rUJjC0kzbNePLwzxq8iypo41qeWA==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/feross" + }, + { + "type": "patreon", + "url": "https://www.patreon.com/feross" + }, + { + "type": "consulting", + "url": "https://feross.org/support" + } + ], + "license": "MIT" + }, "node_modules/body-parser": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.3.0.tgz", @@ -1491,6 +1512,30 @@ "node": "20 || >=22" } }, + "node_modules/buffer": { + "version": "6.0.3", + "resolved": "https://registry.npmjs.org/buffer/-/buffer-6.0.3.tgz", + "integrity": "sha512-FTiCpNxtwiZZHEZbcbTIcZjERVICn9yq/pDFkTl95/AxzD1naBctN7YO68riM/gLSDY7sdrMby8hofADYuuqOA==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/feross" + }, + { + "type": "patreon", + "url": "https://www.patreon.com/feross" + }, + { + "type": "consulting", + "url": "https://feross.org/support" + } + ], + "license": "MIT", + "dependencies": { + "base64-js": "^1.3.1", + "ieee754": "^1.2.1" + } + }, "node_modules/bytes": { "version": "3.1.2", "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.1.2.tgz", @@ -2375,6 +2420,26 @@ "url": "https://opencollective.com/express" } }, + "node_modules/ieee754": { + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/ieee754/-/ieee754-1.2.1.tgz", + "integrity": "sha512-dcyqhDvX1C46lXZcVqCpK+FtMRQVdIMN6/Df5js2zouUsqG7I6sFxitIC+7KYK29KdXOLHdu9zL4sFnoVQnqaA==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/feross" + }, + { + "type": "patreon", + "url": "https://www.patreon.com/feross" + }, + { + "type": "consulting", + "url": "https://feross.org/support" + } + ], + "license": "BSD-3-Clause" + }, "node_modules/ignore": { "version": "5.3.2", "resolved": "https://registry.npmjs.org/ignore/-/ignore-5.3.2.tgz", @@ -2973,6 +3038,37 @@ "url": "https://opencollective.com/express" } }, + "node_modules/neo4j-driver": { + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/neo4j-driver/-/neo4j-driver-6.2.0.tgz", + "integrity": "sha512-9W/Tk7EyjZHtv87NFpqoIbl0mlQx8bX8phUFzAM9xmeJrRpGDvqkpdQzc11DjwzywQCdhifaZPSvokSnAp7YTg==", + "license": "Apache-2.0", + "dependencies": { + "neo4j-driver-bolt-connection": "6.2.0", + "neo4j-driver-core": "6.2.0", + "rxjs": "^7.8.2" + }, + "engines": { + "node": ">=18.0.0" + } + }, + "node_modules/neo4j-driver-bolt-connection": { + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/neo4j-driver-bolt-connection/-/neo4j-driver-bolt-connection-6.2.0.tgz", + "integrity": "sha512-SEnFSBfumleDxmDAZKkjXxZ5LgL2iZSTx08sO+qXX45ZKaxZJOcg9oxwoXACXZ1RbIq3XbDfHU8I5tExanYEOA==", + "license": "Apache-2.0", + "dependencies": { + "buffer": "^6.0.3", + "neo4j-driver-core": "6.2.0", + "string_decoder": "^1.3.0" + } + }, + "node_modules/neo4j-driver-core": { + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/neo4j-driver-core/-/neo4j-driver-core-6.2.0.tgz", + "integrity": "sha512-U8PSdvDECWmCgSWyhqNNVzMONXFHWPcTd5gqi12+EDxxt0aETA0m+XCXf9H7q6qQXdisc40rUJEOcVQOu6peEA==", + "license": "Apache-2.0" + }, "node_modules/object-assign": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/object-assign/-/object-assign-4.1.1.tgz", @@ -3350,6 +3446,35 @@ "node": ">= 18" } }, + "node_modules/rxjs": { + "version": "7.8.2", + "resolved": "https://registry.npmjs.org/rxjs/-/rxjs-7.8.2.tgz", + "integrity": "sha512-dhKf903U/PQZY6boNNtAGdWbG85WAbjT/1xYoZIC7FAY0yWapOBQVsVrDl58W86//e1VpMNBtRV4MaXfdMySFA==", + "license": "Apache-2.0", + "dependencies": { + "tslib": "^2.1.0" + } + }, + "node_modules/safe-buffer": { + "version": "5.2.1", + "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.1.tgz", + "integrity": "sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/feross" + }, + { + "type": "patreon", + "url": "https://www.patreon.com/feross" + }, + { + "type": "consulting", + "url": "https://feross.org/support" + } + ], + "license": "MIT" + }, "node_modules/safer-buffer": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz", @@ -3553,6 +3678,15 @@ "dev": true, "license": "MIT" }, + "node_modules/string_decoder": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.3.0.tgz", + "integrity": "sha512-hkRX8U1WjJFd8LsDJ2yQ/wWWxaopEsABU1XfkM8A+j0+85JAGppt16cr1Whg6KIbb4okU6Mql6BOj+uup/wKeA==", + "license": "MIT", + "dependencies": { + "safe-buffer": "~5.2.0" + } + }, "node_modules/tinybench": { "version": "6.1.4", "resolved": "https://registry.npmjs.org/tinybench/-/tinybench-6.1.4.tgz", @@ -3622,6 +3756,12 @@ "typescript": ">=4.8.4" } }, + "node_modules/tslib": { + "version": "2.8.1", + "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", + "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", + "license": "0BSD" + }, "node_modules/type-check": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/type-check/-/type-check-0.4.0.tgz", diff --git a/package.json b/package.json index 1f4e68e..9932305 100644 --- a/package.json +++ b/package.json @@ -46,6 +46,7 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", "express": "^5.2.1", + "neo4j-driver": "^6.2.0", "zod": "^4.6.5" }, "devDependencies": { diff --git a/scripts/smoke-guard-timeout.mjs b/scripts/smoke-guard-timeout.mjs index 8239f4e..adbd3d9 100644 --- a/scripts/smoke-guard-timeout.mjs +++ b/scripts/smoke-guard-timeout.mjs @@ -1,31 +1,42 @@ #!/usr/bin/env node -// Verify the APOC write-through guard and the Cypher query timeout. -// Requires NEO4J_URI and a running Neo4j. Uses a short timeout to force it. +// Verify the write-through guard and the Cypher query timeout against a live +// database. Requires NEO4J_URI and MCP_ALLOW_CYPHER=1. Uses a short timeout on +// a deliberately slow read to force the server to terminate it. Needs no APOC. +// Keep the range modest: range() builds its list before the transaction guard +// gets a look in, so a huge one costs the database memory regardless. -import { rejectWriteThroughCalls } from "../dist/tools/cypher-guard.js"; -import { runRead } from "../dist/clients/neo4j.js"; +import { assertReadOnlyQuery } from "../dist/tools/cypher-guard.js"; +import { runRead, closeDriver } from "../dist/clients/neo4j.js"; let failures = 0; -try { - rejectWriteThroughCalls("CALL apoc.cypher.runWrite('CREATE (n)', {})"); - console.log("FAIL: guard did not reject apoc.cypher.runWrite"); - failures++; -} catch (e) { - console.log("OK: guard rejected apoc.cypher.runWrite —", e.message.slice(0, 80)); +for (const query of [ + "CALL apoc.cypher.runWrite('CREATE (n)', {})", + "LOAD CSV FROM 'http://example.invalid/x.csv' AS row RETURN row", +]) { + try { + assertReadOnlyQuery(query); + console.log("FAIL: guard did not reject:", query); + failures++; + } catch (e) { + console.log("OK: guard rejected —", e.message.slice(0, 80)); + } } try { - rejectWriteThroughCalls("MATCH (n:Pathway) RETURN n LIMIT 1"); + assertReadOnlyQuery("MATCH (n:Pathway) RETURN n LIMIT 1"); console.log("OK: guard accepts a plain read query"); } catch (e) { console.log("FAIL: guard rejected a plain read query:", e.message); failures++; } -// Force a server-side timeout via apoc.util.sleep which blocks for a known duration. try { - await runRead("CALL apoc.util.sleep(5000) RETURN 1", {}, { timeoutMs: 300 }); + await runRead( + "UNWIND range(1, 20000000) AS x WITH x WHERE x % 7 = 0 RETURN count(x)", + {}, + { timeoutMs: 300 } + ); console.log("FAIL: expected timeout but runRead returned"); failures++; } catch (e) { @@ -38,4 +49,5 @@ try { } } +await closeDriver(); process.exit(failures > 0 ? 1 : 0); diff --git a/scripts/smoke-neo4j.mjs b/scripts/smoke-neo4j.mjs index aac93be..11e258e 100644 --- a/scripts/smoke-neo4j.mjs +++ b/scripts/smoke-neo4j.mjs @@ -1,47 +1,63 @@ #!/usr/bin/env node -// Live smoke test for the Cypher tools against a running Reactome Neo4j. -// Requires: NEO4J_URI set (e.g. bolt://localhost:7687) and the DB reachable. -// Not part of the automated test suite — run manually after `npm run build`. - -import { isNeo4jConfigured, fetchGraphSchema, runRead } from "../dist/clients/neo4j.js"; -import { capCypherRows } from "../dist/tools/cypher.js"; - -if (!isNeo4jConfigured()) { - console.error("Set NEO4J_URI (e.g. bolt://localhost:7687) to run this smoke test."); +// Live smoke test for the Cypher tools against a running Reactome graph database. +// Requires: NEO4J_URI (e.g. bolt://127.0.0.1:7687) and MCP_ALLOW_CYPHER=1, and +// the database reachable. Read-only. Not part of the automated test suite -- +// run manually after `npm run build`. Never point it at a database you do not +// own. + +import { isGraphAccessEnabled } from "../dist/graph/gate.js"; +import { runRead, closeDriver } from "../dist/clients/neo4j.js"; +import { fetchGraphSchema } from "../dist/graph/schema.js"; +import { formatGraphSchemaMarkdown } from "../dist/graph/format-schema.js"; + +if (!isGraphAccessEnabled()) { + console.error("Set NEO4J_URI and MCP_ALLOW_CYPHER=1 to run this smoke test."); process.exit(2); } async function main() { - console.log("[smoke] fetching graph schema…"); - const schema = await fetchGraphSchema(); - console.log(` labels: ${schema.labels.length}`); - console.log(` relationship types: ${schema.relationshipTypes.length}`); - console.log(` labels with properties: ${Object.keys(schema.propertiesByLabel).length}`); - console.log(` first 5 labels: ${schema.labels.slice(0, 5).join(", ")}`); - - console.log("[smoke] database info query…"); - const info = await runRead( - "CALL dbms.components() YIELD name, versions, edition RETURN name, versions, edition" + console.log("[smoke] database components…"); + console.log( + " ", + JSON.stringify(await runRead("CALL dbms.components() YIELD name, versions, edition RETURN *")) ); - console.log(" components:", JSON.stringify(info)); + + console.log("[smoke] fetching graph schema (built-in procedures only)…"); + const started = Date.now(); + const schema = await fetchGraphSchema(); + console.log(` took ${Date.now() - started} ms`); + console.log(` nodes: ${schema.stats.nodeCount}, relationships: ${schema.stats.relCount}`); + console.log(` labels: ${Object.keys(schema.stats.labels).length}`); + console.log(` relationship types: ${Object.keys(schema.stats.relTypesCount).length}`); + console.log(` patterns: ${schema.patterns.length}`); + console.log(` node property rows: ${schema.nodeTypeProperties.length}`); + console.log(` rel property rows: ${schema.relTypeProperties.length}`); + console.log(` indexes: ${schema.indexes.length}, constraints: ${schema.constraints.length}`); + console.log(` unavailable: ${JSON.stringify(schema.unavailable)}`); + console.log(` digest: ${formatGraphSchemaMarkdown(schema).length} chars`); console.log("[smoke] sampling Pathway nodes…"); const pathways = await runRead( "MATCH (n:Pathway) RETURN n.stId AS stId, n.displayName AS name LIMIT $limit", { limit: 3 } ); - console.log(" pathways:", JSON.stringify(pathways, null, 2)); - - console.log("[smoke] exercising capCypherRows with a wide row…"); - const wide = Array.from({ length: 5 }, () => ({ big: "x".repeat(4000) })); - const { output, stats } = capCypherRows(wide, 10, 500, 40000); - console.log(` output rows: ${output.length}, stats: ${JSON.stringify(stats)}`); - - console.log("[smoke] OK"); - process.exit(0); + console.log(" pathways:", JSON.stringify(pathways)); + + console.log("[smoke] a write is refused by the READ-mode session…"); + try { + await runRead("CREATE (n:SmokeTestShouldNotExist) RETURN n"); + console.log(" FAIL: the write was accepted"); + process.exitCode = 1; + } catch (error) { + console.log(" OK:", String(error?.message ?? error).slice(0, 120)); + } + + await closeDriver(); + console.log(process.exitCode ? "[smoke] FAILED" : "[smoke] OK"); } -main().catch(err => { +main().catch(async err => { console.error("[smoke] FAILED:", err); + await closeDriver(); process.exit(1); }); diff --git a/specs/002-transport-and-hosting/spec.md b/specs/002-transport-and-hosting/spec.md index 9bcb87d..cc959dd 100644 --- a/specs/002-transport-and-hosting/spec.md +++ b/specs/002-transport-and-hosting/spec.md @@ -22,14 +22,22 @@ session and a second transport from the same registrations — neither is possib while the only instance is a module-scope constant. The idea is harvested from #5 by @adidev001. -**No Neo4j from a deployed instance.** Superseded 2026-09-21 by something -stronger: the graph tools were removed outright, along with the schema resource -and the `neo4j-driver` dependency, so there is no gate to hold. A public endpoint -holding database credentials is a different security proposition from one that -can only make the calls a browser can — and a gate makes that a property of the -configuration, which is how `src/http-server.ts` came to keep opening a -connection on `NEO4J_URI` alone after the other call sites were fixed. -`tests/no-graph-access.test.ts` asserts the absence with the old switches on. +**No Neo4j from a public instance.** History: graph tools sat behind a +`NEO4J_URI` gate, then behind `NEO4J_URI` plus `MCP_ALLOW_CYPHER=1`; on +2026-09-21 they were removed outright, because the gate had been asked in four +places and `src/http-server.ts` still opened a connection on `NEO4J_URI` alone. + +Restored 2026-10-14, switched off by default, for internal instances pointed at +a dedicated Reactome graph database container. The fix for the missed door is +structural rather than a fifth call site: one predicate, +`isGraphAccessEnabled()` in `src/graph/gate.ts`, asked by every surface **and** +by `getDriver()`, the only code that imports `neo4j-driver`. A surface that +forgets the check still gets no driver. `tests/graph-access-gate.test.ts` +drives both entrypoints, `startHttpServer`, `createServer` and the registrars in +every combination of the two variables, counting driver constructions on a fake +driver. A public endpoint holding database credentials is still a different +security proposition from one that can only make the calls a browser can; a +hosted instance does not set either variable. **Analysis runs in the Analysis Service.** The server submits identifiers, holds the token, and formats the reply. diff --git a/src/clients/neo4j.ts b/src/clients/neo4j.ts new file mode 100644 index 0000000..e272264 --- /dev/null +++ b/src/clients/neo4j.ts @@ -0,0 +1,179 @@ +import type { Driver, Session } from "neo4j-driver"; +import { CYPHER_QUERY_TIMEOUT_MS, neo4jConnection } from "../config.js"; +import { GRAPH_ACCESS_OFF, isGraphAccessEnabled } from "../graph/gate.js"; +import { logger } from "../logger.js"; + +/** + * The only place in this server that loads `neo4j-driver` or constructs a + * driver. + * + * `tests/graph-access-gate.test.ts` asserts no other source file imports + * `neo4j-driver`, so every connection comes through `getDriver()`, and + * `getDriver()` asks the gate itself: a surface that forgot to check the gate + * still gets no driver. The library is imported dynamically, after that + * check, so an instance with graph access off never loads it at all. The type + * import above is erased at compile time. + */ + +/** The parts of the library this file uses. */ +type Neo4jModule = Pick; + +let driverPromise: Promise<{ neo4j: Neo4jModule; driver: Driver }> | null = null; + +function isLoopback(uri: string): boolean { + try { + const host = new URL(uri).hostname; + return host === "localhost" || host === "127.0.0.1" || host === "[::1]" || host === "::1"; + } catch { + return false; + } +} + +async function build(): Promise<{ neo4j: Neo4jModule; driver: Driver }> { + const { uri, user, password, database } = neo4jConnection(); + if (!uri) throw new Error(GRAPH_ACCESS_OFF); + + const neo4j: Neo4jModule = (await import("neo4j-driver")).default; + + if (!isLoopback(uri) && !password) { + logger.warn("NEO4J_URI is not a loopback address and NEO4J_PASSWORD is unset", { + hint: "Keep the graph database on loopback, or require authentication.", + }); + } + + const auth = password === undefined ? undefined : neo4j.auth.basic(user, password); + const driver = neo4j.driver(uri, auth, { disableLosslessIntegers: true }); + logger.info("neo4j driver initialized", { database: database ?? "(server default)" }); + + // process.once expects a void-returning listener. Handing it an async + // function meant a failed close rejected with nobody listening, which Node + // turns into a hard exit on unhandled rejection. + const onShutdown = () => { + void closeDriver().catch((error: unknown) => { + logger.warn("neo4j driver close failed during shutdown", { + error: error instanceof Error ? error.message : String(error), + }); + }); + }; + process.once("SIGINT", onShutdown); + process.once("SIGTERM", onShutdown); + process.once("beforeExit", onShutdown); + + return { neo4j, driver }; +} + +async function connection(): Promise<{ neo4j: Neo4jModule; driver: Driver }> { + // Asked on every call, not only the first: a driver built while the gate + // was open is not a licence to keep using it once it is not. + if (!isGraphAccessEnabled()) throw new Error(GRAPH_ACCESS_OFF); + // One promise, so concurrent first callers share one driver. + driverPromise ??= build().catch((error: unknown) => { + driverPromise = null; + throw error; + }); + return driverPromise; +} + +export async function getDriver(): Promise { + return (await connection()).driver; +} + +/** Close and forget the driver, if one was built. Safe to call when none was. */ +export async function closeDriver(): Promise { + const pending = driverPromise; + driverPromise = null; + if (!pending) return; + const built = await pending.catch(() => null); + if (built) await built.driver.close(); +} + +// JS `number` always serializes to Float64 over Bolt. Cypher features like +// LIMIT / SKIP require an Integer type on the server side, so coerce any +// safe-integer number (and bigint) params into the driver's Integer wrapper. +function coerceIntParams(neo4j: Neo4jModule, params: Record) { + const out: Record = {}; + for (const [k, v] of Object.entries(params)) { + if (typeof v === "number" && Number.isInteger(v) && Number.isSafeInteger(v)) { + out[k] = neo4j.int(v); + } else if (typeof v === "bigint") { + out[k] = neo4j.int(v.toString()); + } else { + out[k] = v; + } + } + return out; +} + +export interface RunReadOptions { + timeoutMs?: number; +} + +async function readSession(fetchSize?: number): Promise<{ neo4j: Neo4jModule; session: Session }> { + const { neo4j, driver } = await connection(); + const { database } = neo4jConnection(); + const session = driver.session({ + ...(database ? { database } : {}), + defaultAccessMode: neo4j.session.READ, + ...(fetchSize ? { fetchSize } : {}), + }); + return { neo4j, session }; +} + +/** + * Run one query in a READ-mode session, under a server-side timeout, and + * return every row. For the schema queries, whose results are bounded by the + * schema; a caller's query goes through `runReadLimited`. + * + * READ mode makes the database refuse native writes (CREATE, MERGE, DELETE, + * SET, REMOVE). The timeout is a transaction timeout: Neo4j terminates the + * transaction if it runs longer, so a runaway traversal ends on the server + * rather than merely being abandoned by this client. + */ +export async function runRead>( + cypher: string, + params: Record = {}, + options: RunReadOptions = {} +): Promise { + const { neo4j, session } = await readSession(); + const timeout = options.timeoutMs ?? CYPHER_QUERY_TIMEOUT_MS; + try { + const result = await session.run(cypher, coerceIntParams(neo4j, params), { timeout }); + return result.records.map(r => r.toObject() as T); + } finally { + await session.close(); + } +} + +/** + * Like `runRead`, but reads at most `maxRows + 1` rows and then stops. + * + * The rows are streamed, in batches no larger than that, and the rest of the + * result is discarded on the server once enough have arrived -- so + * `MATCH (n) RETURN n` reads a handful of nodes into this process rather than + * every node in the graph. The extra row is how `more` is known without + * counting the rest. + */ +export async function runReadLimited>( + cypher: string, + params: Record, + maxRows: number, + options: RunReadOptions = {} +): Promise<{ rows: T[]; more: boolean }> { + const { neo4j, session } = await readSession(Math.min(maxRows + 1, 1000)); + const timeout = options.timeoutMs ?? CYPHER_QUERY_TIMEOUT_MS; + const rows: T[] = []; + let more = false; + try { + const result = session.run(cypher, coerceIntParams(neo4j, params), { timeout }); + for await (const record of result) { + if (rows.length === maxRows) { + more = true; + break; // ends the iteration, which discards the rest of the stream + } + rows.push(record.toObject()); + } + return { rows, more }; + } finally { + await session.close(); + } +} diff --git a/src/config.ts b/src/config.ts index 0da7109..31d2a4e 100644 --- a/src/config.ts +++ b/src/config.ts @@ -61,21 +61,6 @@ export const RESOURCE_TYPES = [ export const DIAGRAM_FORMATS = ["png", "jpg", "jpeg", "svg", "gif"] as const; -/** - * There is no Neo4j configuration here, deliberately. - * - * Constitution Principle IV said the server holds no Neo4j connection in any - * deployment the team runs, and enforced it with a gate. On 2026-09-21 the - * gate was removed along with the graph tools: this server is hosted - * publicly, and neither consumer needed it. The chatbot queries the graph - * directly with its own driver when it builds embeddings, and the website - * needs the REST-backed tools only. - * - * `NEO4J_URI`, `MCP_ALLOW_CYPHER` and the rest are now inert -- setting them - * does nothing, because nothing reads them and no code path opens a - * connection. `tests/no-graph-access.test.ts` keeps that true. - */ - function parsePositiveInt(raw: string | undefined, fallback: number): number { if (!raw) return fallback; const n = Number(raw); @@ -83,6 +68,42 @@ function parsePositiveInt(raw: string | undefined, fallback: number): number { return n; } +/** + * Graph database connection, for internal instances only. + * + * Whether any of this is used is decided by `isGraphAccessEnabled()` in + * src/graph/gate.ts -- `NEO4J_URI` **and** `MCP_ALLOW_CYPHER=1` -- and by + * nothing else. These values are read when a driver is built rather than + * captured at import, so the gate and the connection cannot disagree about + * what the environment says. + * + * `NEO4J_DATABASE` is optional. Neo4j Community serves one user database, and + * leaving it unset uses whichever that is. + */ +export function neo4jConnection(env: NodeJS.ProcessEnv = process.env) { + return { + uri: env.NEO4J_URI?.trim() || undefined, + user: env.NEO4J_USER ?? "neo4j", + password: env.NEO4J_PASSWORD, + database: env.NEO4J_DATABASE?.trim() || undefined, + }; +} + +/** + * Server-side timeout on every Cypher query, in milliseconds. The database + * terminates a transaction that runs longer. + * + * It bounds time, not memory, and not precisely: Neo4j checks the deadline + * periodically, so on the Release 97 graph a 300 ms timeout ended a query + * after about 4 s, and an expression it evaluates eagerly (a huge `range()`) + * can hold memory past the deadline. That is one more reason graph access is + * never enabled on a publicly reachable instance. + */ +export const CYPHER_QUERY_TIMEOUT_MS = parsePositiveInt( + process.env.CYPHER_QUERY_TIMEOUT_MS, + 30_000 +); + /** * Backstop on how much text one tool may return. * diff --git a/src/graph/format-schema.ts b/src/graph/format-schema.ts new file mode 100644 index 0000000..91948b1 --- /dev/null +++ b/src/graph/format-schema.ts @@ -0,0 +1,185 @@ +import type { GraphSchema } from "./schema.js"; + +/** How many labels to name on each side of a relationship type. */ +const LABELS_PER_SIDE = 4; + +type Prop = { propertyName: string; propertyTypes: string[]; mandatory: boolean }; + +/** `stId String!` -- `!` marks a property every node of the set has. */ +const propText = (p: Prop) => + `${p.propertyName} ${(p.propertyTypes ?? []).join("|") || "?"}${p.mandatory ? "!" : ""}`; + +/** + * Labels, most specific first, cut to a few. + * + * Reactome nodes carry their whole class hierarchy as labels -- a Pathway is + * also an Event and a DatabaseObject -- so `db.schema.visualization()` lists a + * relationship type once per pair of labels on its two ends: 749 pairs for one + * type on the Release 97 graph. Rarer labels are the specific classes, so + * sorting by node count puts the useful ones first. + */ +function labelList(labels: Set, counts: Record): string { + const sorted = [...labels].sort( + (a, b) => (counts[a] ?? Infinity) - (counts[b] ?? Infinity) || a.localeCompare(b) + ); + const shown = sorted.slice(0, LABELS_PER_SIDE).join("|"); + return sorted.length > LABELS_PER_SIDE ? `${shown}|… +${sorted.length - LABELS_PER_SIDE}` : shown; +} + +/** + * Render a GraphSchema as a compact markdown digest for a model to read before + * writing Cypher: labels and relationship types by count, which labels each + * type connects, property types, indexes and constraints. The full structure + * is the `reactome://graph/schema` resource. + * + * Sized for the tool response cap. Rendered naively, the Release 97 graph came + * to 87,000 characters -- more than twice the cap, so the cap would have cut + * it mid-way and dropped the indexes and constraints. The shape below came to + * 34,341 characters against the same graph, inside the default 40,000 cap: + * properties common to every label set are listed once, relationship types + * with the same properties share a line, and each type's endpoints are + * summarised as label sets rather than enumerated pair by pair. A much larger + * graph could still reach the cap, which announces its cut. + * + * Sections the database would not give up are named under "Unavailable" + * rather than left out, so a missing property list is not read as an empty + * one. + */ +export function formatGraphSchemaMarkdown(schema: GraphSchema): string { + const { stats, nodeTypeProperties, relTypeProperties, indexes, constraints, patterns } = schema; + + const labelEntries = Object.entries(stats.labels).sort(([, a], [, b]) => b - a); + const relEntries = Object.entries(stats.relTypesCount).sort(([, a], [, b]) => b - a); + + const lines: string[] = []; + lines.push(`## Reactome Graph Schema`); + const dbComp = schema.dbComponents[0]; + lines.push( + `**Neo4j:** ${dbComp?.versions?.[0] ?? "?"} ${dbComp?.edition ?? ""} · **Fetched:** ${schema.fetchedAt}` + ); + lines.push( + `**Totals:** ${stats.nodeCount.toLocaleString()} nodes · ${stats.relCount.toLocaleString()} relationships · ${labelEntries.length} labels · ${relEntries.length} relationship types` + ); + lines.push(""); + + if (schema.unavailable.length > 0) { + lines.push(`### Unavailable (${schema.unavailable.length})`); + lines.push("These sections could not be read; do not assume they are empty."); + for (const u of schema.unavailable) lines.push(`- \`${u.section}\` — ${u.reason}`); + lines.push(""); + } + + lines.push(`### Labels (${labelEntries.length}, by node count)`); + lines.push( + labelEntries.map(([label, count]) => `\`${label}\` ${count.toLocaleString()}`).join(" · ") + ); + lines.push(""); + + // Endpoints per relationship type. + const ends = new Map; end: Set }>(); + for (const p of patterns) { + const entry = ends.get(p.type) ?? { start: new Set(), end: new Set() }; + entry.start.add(p.start); + entry.end.add(p.end); + ends.set(p.type, entry); + } + + lines.push(`### Relationship types (${relEntries.length}, by relationship count)`); + if (ends.size > 0) { + lines.push("Each with the labels seen at its start and end, most specific first."); + } + const relTypes = new Set([...relEntries.map(([t]) => t), ...ends.keys()]); + for (const relType of relTypes) { + const count = stats.relTypesCount[relType]; + const side = ends.get(relType); + const shape = side + ? ` — (:${labelList(side.start, stats.labels)})-[:${relType}]->(:${labelList(side.end, stats.labels)})` + : ""; + lines.push(`- \`${relType}\` ${count === undefined ? "" : count.toLocaleString()}${shape}`); + } + lines.push(""); + + // Node properties by label set, with what every set shares listed once. + const bySet = new Map(); + for (const p of nodeTypeProperties) { + const key = p.nodeType; + const entry = bySet.get(key) ?? { labels: p.nodeLabels ?? [], props: [] }; + if (p.propertyName != null) entry.props.push(p); + bySet.set(key, entry); + } + if (bySet.size > 0) { + const sets = [...bySet.values()]; + const signature = (p: Prop) => propText(p); + const shared = (sets[0]?.props ?? []) + .map(signature) + .filter(sig => sets.every(s => s.props.some(p => signature(p) === sig))); + const sharedSet = new Set(shared); + const everywhereLabels = (sets[0]?.labels ?? []).filter(l => + sets.every(s => s.labels.includes(l)) + ); + + lines.push(`### Node properties (${bySet.size} label sets)`); + lines.push("`!` marks a property every node of that set has."); + if (shared.length > 0) lines.push(`**On every label set:** ${shared.join(", ")}`); + if (everywhereLabels.length > 0) { + lines.push(`**Labels on every node, omitted below:** ${everywhereLabels.join(", ")}`); + } + const rows = sets + .map(s => ({ + name: s.labels.filter(l => !everywhereLabels.includes(l)).join(":") || "(none)", + extra: s.props.map(signature).filter(sig => !sharedSet.has(sig)), + })) + .sort((a, b) => a.name.localeCompare(b.name)); + for (const row of rows) { + lines.push(`- **${row.name}**${row.extra.length ? `: ${row.extra.join(", ")}` : ""}`); + } + lines.push(""); + } + + // Relationship properties, grouped by identical property lists. + if (relTypeProperties.length > 0) { + const propsByRel = new Map(); + for (const p of relTypeProperties) { + const entry = propsByRel.get(p.relType) ?? []; + entry.push(propText(p)); + propsByRel.set(p.relType, entry); + } + const bySignature = new Map(); + for (const [rel, props] of propsByRel) { + const sig = [...props].sort().join(", "); + bySignature.set(sig, [...(bySignature.get(sig) ?? []), rel]); + } + lines.push(`### Relationship properties`); + for (const [sig, rels] of [...bySignature].sort(([a], [b]) => a.localeCompare(b))) { + lines.push(`- ${sig} — on ${rels.sort().join(", ")}`); + } + lines.push(""); + } + + if (indexes.length > 0) { + lines.push(`### Indexes (${indexes.length})`); + for (const ix of indexes) { + const labels = ix.labelsOrTypes?.join(",") ?? "?"; + const props = ix.properties?.join(",") ?? "?"; + lines.push(`- ${labels}(${props}) [${ix.type ?? "?"}, ${ix.state ?? "?"}]`); + } + lines.push(""); + } + + if (constraints.length > 0) { + lines.push(`### Constraints (${constraints.length})`); + for (const c of constraints) { + const what = + c.description ?? + `${c.type ?? "?"} on ${c.labelsOrTypes?.join(",") ?? "?"}(${c.properties?.join(",") ?? "?"})`; + lines.push(`- ${what}`); + } + lines.push(""); + } + + lines.push( + "_For the full structure as JSON, including every label pair each relationship type connects, read the `reactome://graph/schema` resource._" + ); + + return lines.join("\n"); +} diff --git a/src/graph/gate.ts b/src/graph/gate.ts new file mode 100644 index 0000000..06d4bb6 --- /dev/null +++ b/src/graph/gate.ts @@ -0,0 +1,33 @@ +/** + * Whether this instance has graph database access at all. + * + * **Both** of these, or nothing: + * + * NEO4J_URI set to a non-blank connection string + * MCP_ALLOW_CYPHER exactly "1" + * + * This is the one predicate. Every surface that touches the graph asks it -- + * tool registration, the `reactome://graph/schema` resource, the server + * instructions, the startup schema warm-up, and `getDriver()` itself, which + * refuses to construct a driver (or load the library) when it is false. + * Asking at the driver as well as at each surface means a surface that + * forgets to ask still cannot get a driver. Every entrypoint and factory is + * exercised against it in `tests/graph-access-gate.test.ts`. + * + * Two variables rather than one, on purpose. A connection string can be set + * for many reasons and is easy to inherit from a shared environment file; + * it is not consent to publish arbitrary query access. Forgetting the opt-in + * costs a missing tool on an internal instance, which is visible and + * harmless; the gate is designed so that the mistake goes that way. + * + * Read from the environment on every call rather than captured at import, so + * there is one copy of the answer and it cannot go stale. + */ +export function isGraphAccessEnabled(env: NodeJS.ProcessEnv = process.env): boolean { + return Boolean(env.NEO4J_URI?.trim()) && env.MCP_ALLOW_CYPHER === "1"; +} + +/** Why a graph operation was refused, with what would change the answer. */ +export const GRAPH_ACCESS_OFF = + "Graph database access is off. It needs both NEO4J_URI and MCP_ALLOW_CYPHER=1, " + + "and must never be enabled on a publicly reachable instance."; diff --git a/src/graph/schema.ts b/src/graph/schema.ts new file mode 100644 index 0000000..41fce0a --- /dev/null +++ b/src/graph/schema.ts @@ -0,0 +1,335 @@ +import { runRead } from "../clients/neo4j.js"; +import { logger } from "../logger.js"; + +/** + * The live graph schema, built from Neo4j's **built-in** procedures only. + * + * The Reactome graph database container runs Neo4j 4.4 Community with no + * APOC. The previous version of this file read `apoc.meta.stats()`, + * `apoc.meta.schema()` and `apoc.meta.nodeTypeProperties()`, and would have + * failed on its first query there. Everything below exists in 4.4 Community: + * + * dbms.components() version and edition + * db.labels(), db.relationshipTypes() + * MATCH (n:`L`) RETURN count(n) per-token counts, answered from the count + * store rather than by scanning + * db.schema.nodeTypeProperties() property names, types, mandatory flags + * db.schema.relTypeProperties() + * db.schema.visualization() which label connects to which, by type + * SHOW INDEXES / SHOW CONSTRAINTS, falling back to db.indexes() / + * db.constraints() where SHOW is not known + * + * Each section is fetched on its own and fails on its own. A section that + * cannot be read is listed in `unavailable` with the reason, rather than + * rendered empty -- an empty property list reads as "these labels have no + * properties", which is a confident wrong answer. + * + * Row shapes, recorded from the Reactome graph database (Neo4j 4.4.48 + * Community, Release 97), trimmed: + * db.schema.nodeTypeProperties {nodeType: ":`CandidateSet`:`DatabaseObject`:...", + * nodeLabels: ["CandidateSet", "DatabaseObject", ...], + * propertyName: "dbId", propertyTypes: ["Long"], + * mandatory: true} + * db.schema.relTypeProperties {relType: ":`author`", propertyName: "order", + * propertyTypes: ["Integer", "Long"], mandatory: true} + * db.schema.visualization {nodes: [{identity: -127, labels: ["TopLevelPathway"], + * properties: {name: "TopLevelPathway", ...}}], + * relationships: [{identity: -7911, start: -134, + * end: -179, type: "authored"}]} + * SHOW INDEXES {name, type: "BTREE", entityType: "NODE", + * labelsOrTypes: ["DatabaseObject"], + * properties: ["dbId"], state: "ONLINE"} + * SHOW CONSTRAINTS {name, type: "UNIQUENESS", entityType: "NODE", + * labelsOrTypes: ["DatabaseObject"], properties: ["dbId"]} + * + * On that graph the whole fetch took about 65 s (2.96M nodes, 11.5M + * relationships, 108 labels, 88 types, 5,669 label-level patterns), almost + * all of it `db.schema.nodeTypeProperties()`. Hence the warm-up, and the + * timeout below. + */ +export interface GraphSchema { + fetchedAt: string; + dbComponents: Array<{ name: string; versions: string[]; edition: string }>; + stats: { + nodeCount: number; + relCount: number; + labels: Record; + relTypesCount: Record; + }; + /** Label-level connections: (start)-[type]->(end). */ + patterns: Array<{ start: string; type: string; end: string }>; + nodeTypeProperties: Array<{ + nodeType: string; + nodeLabels: string[]; + propertyName: string; + propertyTypes: string[]; + mandatory: boolean; + }>; + relTypeProperties: Array<{ + relType: string; + propertyName: string; + propertyTypes: string[]; + mandatory: boolean; + }>; + indexes: Array<{ + name?: string; + type?: string; + entityType?: string; + labelsOrTypes?: string[] | null; + properties?: string[] | null; + state?: string; + }>; + constraints: Array<{ + name?: string; + type?: string; + entityType?: string; + labelsOrTypes?: string[] | null; + properties?: string[] | null; + description?: string; + }>; + /** Sections that could not be read, and why. Empty when complete. */ + unavailable: Array<{ section: string; reason: string }>; +} + +/** + * Schema queries get a longer budget than an ordinary Cypher query: + * `db.schema.nodeTypeProperties()` reads every node's property keys on 4.4, + * which on the Reactome graph is not quick. The warm-up at startup pays it + * once, so callers do not. + */ +const SCHEMA_FETCH_TIMEOUT_MS = 120_000; + +/** + * How long a schema with unavailable sections is served before those + * sections are tried again. Not on every call: a section that failed by + * timing out would make every call wait for the timeout again. + */ +export const SCHEMA_RETRY_AFTER_MS = 5 * 60_000; + +let schemaCache: { schema: GraphSchema; at: number } | null = null; +let schemaPending: Promise | null = null; + +type Section = keyof Omit; + +class SectionUnavailable extends Error {} + +const message = (error: unknown) => (error instanceof Error ? error.message : String(error)); + +/** Run the first of `queries` that works; throw with every failure if none does. */ +async function firstThatWorks(queries: string[], params: Record = {}) { + const failures: string[] = []; + for (const query of queries) { + try { + return await runRead(query, params, { timeoutMs: SCHEMA_FETCH_TIMEOUT_MS }); + } catch (error) { + failures.push(message(error)); + } + } + throw new SectionUnavailable(failures.join(" | ")); +} + +/** Backticks, doubled, are how Cypher escapes a backtick inside a quoted name. */ +const quoteName = (name: string) => "`" + name.replace(/`/g, "``") + "`"; + +/** ":`input`" -> "input", as db.schema.relTypeProperties() reports it. */ +const bareRelType = (relType: string) => + relType + .replace(/^:/, "") + .replace(/^`(.*)`$/, "$1") + .replace(/``/g, "`"); + +/** + * One query that counts every token, answered from the count store. Names go + * in as parameters for the returned key and quoted for the pattern, so a label + * containing a backtick can neither break nor extend the query. + */ +async function countEach(tokens: string[], pattern: (quoted: string) => string) { + if (tokens.length === 0) return {}; + const params: Record = {}; + const parts = tokens.map((token, i) => { + params[`k${i}`] = token; + // The count sits alone in a subquery so the planner can answer it from + // the count store; with the key projected beside it, it might scan. + return `CALL { ${pattern(quoteName(token))} } RETURN $k${i} AS key, c AS count`; + }); + const rows = await firstThatWorks<{ key: string; count: number }>( + [parts.join(" UNION ALL ")], + params + ); + return Object.fromEntries(rows.map(r => [r.key, Number(r.count)])); +} + +async function fetchStats(): Promise { + const [labelRows, relRows, nodeTotal, relTotal] = await Promise.all([ + firstThatWorks<{ label: string }>(["CALL db.labels() YIELD label RETURN label"]), + firstThatWorks<{ relationshipType: string }>([ + "CALL db.relationshipTypes() YIELD relationshipType RETURN relationshipType", + ]), + firstThatWorks<{ count: number }>(["MATCH (n) RETURN count(n) AS count"]), + firstThatWorks<{ count: number }>(["MATCH ()-[r]->() RETURN count(r) AS count"]), + ]); + const [labels, relTypesCount] = await Promise.all([ + countEach( + labelRows.map(r => r.label), + q => `MATCH (n:${q}) RETURN count(n) AS c` + ), + countEach( + relRows.map(r => r.relationshipType), + q => `MATCH ()-[r:${q}]->() RETURN count(r) AS c` + ), + ]); + return { + nodeCount: Number(nodeTotal[0]?.count ?? 0), + relCount: Number(relTotal[0]?.count ?? 0), + labels, + relTypesCount, + }; +} + +interface VizNode { + identity: unknown; + labels?: string[]; + properties?: { name?: string }; +} +interface VizRel { + start: unknown; + end: unknown; + type: string; +} + +async function fetchPatterns(): Promise { + const rows = await firstThatWorks<{ nodes: VizNode[]; relationships: VizRel[] }>([ + "CALL db.schema.visualization() YIELD nodes, relationships RETURN nodes, relationships", + ]); + const row = rows[0]; + if (!row) return []; + const labelOf = new Map(); + for (const node of row.nodes ?? []) { + labelOf.set(String(node.identity), node.labels?.[0] ?? node.properties?.name ?? "?"); + } + return (row.relationships ?? []) + .map(rel => ({ + start: labelOf.get(String(rel.start)) ?? "?", + type: rel.type, + end: labelOf.get(String(rel.end)) ?? "?", + })) + .sort( + (a, b) => + a.type.localeCompare(b.type) || a.start.localeCompare(b.start) || a.end.localeCompare(b.end) + ); +} + +const SECTIONS: { [K in Section]: () => Promise } = { + dbComponents: () => + firstThatWorks([ + "CALL dbms.components() YIELD name, versions, edition RETURN name, versions, edition", + ]), + stats: fetchStats, + patterns: fetchPatterns, + nodeTypeProperties: () => + firstThatWorks([ + "CALL db.schema.nodeTypeProperties() YIELD nodeType, nodeLabels, propertyName, propertyTypes, mandatory RETURN nodeType, nodeLabels, propertyName, propertyTypes, mandatory", + ]), + relTypeProperties: async () => { + const rows = await firstThatWorks([ + "CALL db.schema.relTypeProperties() YIELD relType, propertyName, propertyTypes, mandatory RETURN relType, propertyName, propertyTypes, mandatory", + ]); + // A relationship type with no properties at all is reported once with a + // null propertyName; that is "no properties", not a property. + return rows + .filter(r => r.propertyName != null) + .map(r => ({ ...r, relType: bareRelType(r.relType) })); + }, + indexes: () => + firstThatWorks([ + "SHOW INDEXES YIELD name, type, entityType, labelsOrTypes, properties, state RETURN name, type, entityType, labelsOrTypes, properties, state", + "CALL db.indexes() YIELD name, type, entityType, labelsOrTypes, properties, state RETURN name, type, entityType, labelsOrTypes, properties, state", + ]), + constraints: () => + firstThatWorks([ + "SHOW CONSTRAINTS YIELD name, type, entityType, labelsOrTypes, properties RETURN name, type, entityType, labelsOrTypes, properties", + "CALL db.constraints() YIELD name, description RETURN name, description", + ]), +}; + +const EMPTY: Omit = { + dbComponents: [], + stats: { nodeCount: 0, relCount: 0, labels: {}, relTypesCount: {} }, + patterns: [], + nodeTypeProperties: [], + relTypeProperties: [], + indexes: [], + constraints: [], +}; + +/** + * Fetch the schema. Cached in memory; concurrent callers share one fetch. + * + * A complete schema is cached for the life of the process. One with + * unavailable sections is served as it is for `SCHEMA_RETRY_AFTER_MS`, and + * then the failed sections -- only those -- are read again, so a section + * that timed out during the startup warm-up is not missing for good. + * + * A fetch in which **no** section could be read -- the database is down, the + * credentials are wrong -- throws and is not cached, so the next call tries + * again instead of serving an empty schema. + */ +export async function fetchGraphSchema(): Promise { + if (schemaCache) { + const { schema, at } = schemaCache; + if (schema.unavailable.length === 0) return schema; + if (Date.now() - at < SCHEMA_RETRY_AFTER_MS) return schema; + } + if (schemaPending) return schemaPending; + + const previous = schemaCache?.schema; + const start = Date.now(); + schemaPending = (async () => { + try { + const names = previous + ? previous.unavailable.map(u => u.section as Section) + : (Object.keys(SECTIONS) as Section[]); + const settled = await Promise.allSettled(names.map(name => SECTIONS[name]())); + + const result: GraphSchema = previous + ? { ...previous, unavailable: [] } + : { fetchedAt: "", ...structuredClone(EMPTY), unavailable: [] }; + result.fetchedAt = new Date().toISOString(); + settled.forEach((outcome, i) => { + const name = names[i] as Section; + if (outcome.status === "fulfilled") { + (result as unknown as Record)[name] = outcome.value; + } else { + result.unavailable.push({ section: name, reason: message(outcome.reason) }); + } + }); + + if (!previous && result.unavailable.length === names.length) { + throw new Error( + `Could not read the graph schema: ${result.unavailable[0]?.reason ?? "no section answered"}` + ); + } + + logger.info(previous ? "graph schema sections retried" : "graph schema fetched", { + durationMs: Date.now() - start, + sections: names.length, + nodeCount: result.stats.nodeCount, + labels: Object.keys(result.stats.labels).length, + unavailable: result.unavailable.map(u => u.section), + }); + + schemaCache = { schema: result, at: Date.now() }; + return result; + } finally { + schemaPending = null; + } + })(); + + return schemaPending; +} + +/** For tests -- clears both the cached value and any in-flight fetch. */ +export function _resetGraphSchemaCache(): void { + schemaCache = null; + schemaPending = null; +} diff --git a/src/graph/warmup.ts b/src/graph/warmup.ts new file mode 100644 index 0000000..360f4ca --- /dev/null +++ b/src/graph/warmup.ts @@ -0,0 +1,43 @@ +import { logger } from "../logger.js"; +import { isGraphAccessEnabled } from "./gate.js"; +import { fetchGraphSchema } from "./schema.js"; +import { GRAPH_GROUP, resolveToolGroups } from "../tools/index.js"; + +/** + * Called by every entrypoint once it is serving. Never rejects. + * + * Warms the graph schema cache in the background when -- and only when -- + * this instance offers graph access, so the first `reactome_cypher_schema` + * call does not pay for the schema procedures. With the gate closed, or the + * `cypher` group withheld, nothing could read that cache, and warming it would + * open a database connection for nobody. + * + * The decision lives here rather than at each call site, so every entrypoint + * makes the same one. + */ +export function warmGraphSchema(): void { + let offered: boolean; + try { + offered = resolveToolGroups().includes(GRAPH_GROUP); + } catch { + // A bad MCP_TOOL_GROUPS is reported where the server is built. + return; + } + + if (!offered) { + if (process.env.NEO4J_URI?.trim() && !isGraphAccessEnabled()) { + // Said out loud: an operator who set NEO4J_URI expecting the Cypher + // tools needs to know why they are absent. + logger.warn("graph database access is OFF: MCP_ALLOW_CYPHER is not 1", { + hint: "Set MCP_ALLOW_CYPHER=1 only on an instance that is not publicly reachable.", + }); + } + return; + } + + fetchGraphSchema().catch((error: unknown) => { + logger.warn("graph schema warm-up failed; will retry on first use", { + error: error instanceof Error ? error.message : String(error), + }); + }); +} diff --git a/src/http-server.ts b/src/http-server.ts index ab2a963..c21a542 100644 --- a/src/http-server.ts +++ b/src/http-server.ts @@ -12,6 +12,7 @@ import { startHttpServer } from "./http.js"; import { logger } from "./logger.js"; import { MCP_HTTP_PORT, MCP_HTTP_HOST } from "./config.js"; +import { warmGraphSchema } from "./graph/warmup.js"; const port = MCP_HTTP_PORT; @@ -26,8 +27,12 @@ if (!port) { // validates the tool-group configuration synchronously, so a bad value threw // past the .catch below and killed the process with a raw stack trace. Dying // was correct; dying without the log line that says why was not. +// +// The graph schema warm-up decides for itself whether graph access is on; +// this entrypoint does not. Promise.resolve() .then(() => startHttpServer(port, MCP_HTTP_HOST)) + .then(() => warmGraphSchema()) .catch((error: unknown) => { logger.error("fatal error starting http server", { error: error instanceof Error ? error.message : String(error), diff --git a/src/http.ts b/src/http.ts index 224a68c..5bcec91 100644 --- a/src/http.ts +++ b/src/http.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { resolveToolGroups } from "./tools/index.js"; +import { GRAPH_GROUP, resolveToolGroups } from "./tools/index.js"; import { estimateAnalysisBodyBytes } from "./tools/limits.js"; import type { Server } from "node:http"; import type { Request, Response } from "express"; @@ -57,6 +57,17 @@ export function startHttpServer(port: number, host: string = MCP_HTTP_HOST): Pro // transport that is actually deployed. resolveToolGroups(); + // Whether a session opened now would be offered the Cypher tools. Asked per + // request, like the sessions themselves, so /health cannot report a value + // captured at startup that a new session no longer matches. + const graphAccess = (): boolean => { + try { + return resolveToolGroups().includes(GRAPH_GROUP); + } catch { + return false; + } + }; + // Defaults to 127.0.0.1 and turns on DNS-rebinding protection for localhost // hosts, which is what stops a web page in the user's browser from driving // this server. @@ -229,6 +240,7 @@ export function startHttpServer(port: number, host: string = MCP_HTTP_HOST): Pro sessions: sessions.size, contentService: CONTENT_SERVICE_URL, analysisService: ANALYSIS_SERVICE_URL, + graphAccess: graphAccess(), }); }); @@ -239,6 +251,7 @@ export function startHttpServer(port: number, host: string = MCP_HTTP_HOST): Pro port, contentService: CONTENT_SERVICE_URL, analysisService: ANALYSIS_SERVICE_URL, + graphAccess: graphAccess(), }); resolve(http); }); diff --git a/src/index.ts b/src/index.ts index 60cbf43..c0fea30 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,6 +4,8 @@ import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js" import { createServer } from "./server.js"; import { logger } from "./logger.js"; import { CONTENT_SERVICE_URL, ANALYSIS_SERVICE_URL } from "./config.js"; +import { GRAPH_GROUP, resolveToolGroups } from "./tools/index.js"; +import { warmGraphSchema } from "./graph/warmup.js"; async function main() { // Built here rather than at module scope, so importing this file does not @@ -14,7 +16,10 @@ async function main() { logger.info("reactome mcp server started", { contentService: CONTENT_SERVICE_URL, analysisService: ANALYSIS_SERVICE_URL, + // Whether this instance offers the Cypher tools -- the gate, and the group. + graphAccess: resolveToolGroups().includes(GRAPH_GROUP), }); + warmGraphSchema(); } main().catch(error => { diff --git a/src/instructions.ts b/src/instructions.ts index ddae761..03a0b4a 100644 --- a/src/instructions.ts +++ b/src/instructions.ts @@ -1,4 +1,4 @@ -import { resolveToolGroups, type ToolGroup } from "./tools/index.js"; +import { GRAPH_GROUP, permittedGroups, resolveToolGroups, type ToolGroup } from "./tools/index.js"; const CORE_INSTRUCTIONS = ` This server exposes the Reactome pathway knowledgebase (https://reactome.org) to LLM clients. Reactome is a manually curated, peer-reviewed database of biological pathways: reactions grouped into pathways grouped into hierarchies, annotated with participants (proteins, complexes, small molecules), regulation, literature, species, and disease. @@ -69,6 +69,10 @@ const RESOURCE_LINES: { group: ToolGroup; text: string }[] = [ { group: "pathway", text: "`reactome://pathway/{id}` — templated pathway details." }, { group: "entity", text: "`reactome://entity/{id}` — templated entity details." }, { group: "analysis", text: "`reactome://analysis/{token}` — templated analysis results." }, + { + group: "cypher", + text: "`reactome://graph/schema` — the graph database schema as JSON (labels, relationship types, properties, indexes, constraints).", + }, ]; const CATEGORY_LINES: Record = { @@ -79,23 +83,42 @@ const CATEGORY_LINES: Record = { interactors: `- **Interactors** (\`reactome_psicquic_*\`, \`reactome_static_interactors\`, \`reactome_interactor_pathways\`) — protein–protein interaction data.`, export: `- **Export** (\`reactome_export_*\`) — diagrams (PNG/SVG), SBGN, SBML, PDF reports, CSV/JSON analysis exports.`, utilities: `- **Utilities** (\`reactome_species\`, \`reactome_diseases\`, \`reactome_database_info\`, \`reactome_mapping_*\`, \`reactome_orthology\`, \`reactome_query\`).`, + cypher: `- **Graph database** (\`reactome_cypher_schema\`, \`reactome_cypher_sample\`, \`reactome_cypher_query\`) — read-only Cypher against the Reactome Neo4j graph. See the section below before using it.`, gsa: `- **Gene set analysis** (\`reactome_gsa_*\`) — ReactomeGSA: available methods, data types, and searchable public expression datasets. Distinct from Analysis, which is over-representation on a list of identifiers.`, }; +const CYPHER_INSTRUCTIONS = ` +# Graph database (Cypher) + +This instance can query the Reactome graph database directly. Use it for what the REST tools do not expose — arbitrary traversals, joins across relationship types, aggregate counts across labels. + +1. Call \`reactome_cypher_schema\` (or read \`reactome://graph/schema\`) **before writing any query**. It lists labels and relationship types with counts, which labels each relationship type connects, property types with mandatory flags, indexes and constraints. Never guess the schema. +2. Use \`reactome_cypher_sample\` on a label to see a representative node. +3. Run the query with \`reactome_cypher_query\`: + - It runs in a READ-mode session; write clauses are refused by the database. + - \`CALL\` is allowed only for the built-in schema procedures (\`db.labels\`, \`db.schema.*\`, \`db.indexes\`, \`db.constraints\`, \`dbms.components\`, ...) and fulltext queries (\`db.index.fulltext.queryNodes\`, \`db.index.fulltext.queryRelationships\`), and \`SHOW\` only for indexes and constraints. Other procedures, \`CALL { ... }\` subqueries, APOC/GDS, \`LOAD CSV\`, \`USE\` and the \`CYPHER\` options prefix are refused. + - A server-side timeout ends long queries. Always include a \`LIMIT\`, and anchor on an indexed property (for example \`stId\` or \`dbId\`) rather than scanning a whole label. + - Return specific properties (\`RETURN p.stId, p.displayName\`) rather than whole nodes; reading stops at \`max_rows\`, and response size is capped. + +**When not to use it:** lookups by ID or name, pathway hierarchies and enrichment — the REST tools are cached, cheaper, and format the answer for you. +`.trim(); + /** * The instructions a client reads on connecting. * * **It describes the groups this instance actually registered, and nothing - * else.** The last time these two facts were allowed to drift apart, the - * server told every client to call `reactome_cypher_query` on an instance - * that had declined to register it. That is invisible to any check which - * asks the server what it can do, because the answer *is* the claim. + * else.** A server describing tools it has not registered is invisible to + * any check which asks the server what it can do, because the answer *is* + * the claim. * * So the category list is built from the same `ToolGroup` values that drive * registration, and `tests/tool-groups.test.ts` asserts a restricted * instance does not mention what it withheld. */ -export function buildServerInstructions(groups: ToolGroup[] = resolveToolGroups()): string { +export function buildServerInstructions(requested: ToolGroup[] = resolveToolGroups()): string { + // The same filter the registrars apply, so the text cannot describe graph + // access the gate withheld -- even for a caller that passes its own list. + const groups = permittedGroups(requested); const categoryLines = groups.map(group => CATEGORY_LINES[group]).join("\n"); const steps = WORKFLOW_STEPS.filter( step => step.group === null || groups.includes(step.group) @@ -105,7 +128,9 @@ export function buildServerInstructions(groups: ToolGroup[] = resolveToolGroups( line => `- ${line.text}` ); - return CORE_INSTRUCTIONS.replace("__CATEGORY_LINES__", categoryLines) + const core = CORE_INSTRUCTIONS.replace("__CATEGORY_LINES__", categoryLines) .replace("__WORKFLOW_STEPS__", steps.join("\n")) .replace("__RESOURCE_LINES__", resourceLines.join("\n")); + + return groups.includes(GRAPH_GROUP) ? `${core}\n\n${CYPHER_INSTRUCTIONS}` : core; } diff --git a/src/resources/graph.ts b/src/resources/graph.ts new file mode 100644 index 0000000..69dc3a4 --- /dev/null +++ b/src/resources/graph.ts @@ -0,0 +1,29 @@ +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { isGraphAccessEnabled } from "../graph/gate.js"; +import { fetchGraphSchema } from "../graph/schema.js"; + +/** + * `reactome://graph/schema` -- the graph model as JSON. + * + * Not a query surface, but it runs schema procedures against the database on + * the caller's behalf and publishes the internal graph model, so it sits + * behind the same gate as the Cypher tools. Checked here as well as through + * the `cypher` group in registerAllResources, so calling this registrar + * directly cannot publish it either. + */ +export function registerGraphResources(server: McpServer) { + if (!isGraphAccessEnabled()) return; + + server.resource("reactome://graph/schema", "reactome://graph/schema", async () => { + const schema = await fetchGraphSchema(); + return { + contents: [ + { + uri: "reactome://graph/schema", + mimeType: "application/json", + text: JSON.stringify(schema, null, 2), + }, + ], + }; + }); +} diff --git a/src/resources/index.ts b/src/resources/index.ts index c393b83..251249b 100644 --- a/src/resources/index.ts +++ b/src/resources/index.ts @@ -1,7 +1,8 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { registerStaticResources } from "./static.js"; import { registerResourceTemplates } from "./templates.js"; -import { resolveToolGroups, type ToolGroup } from "../tools/index.js"; +import { registerGraphResources } from "./graph.js"; +import { permittedGroups, resolveToolGroups, type ToolGroup } from "../tools/index.js"; /** * Which group each resource belongs to. @@ -27,6 +28,7 @@ const RESOURCE_GROUPS: Record = { "reactome://analysis/{token}": "analysis", "reactome://top-pathways/{species}": "pathway", "reactome://events-hierarchy/{species}": "pathway", + "reactome://graph/schema": "cypher", }; export const RESOURCE_URIS = Object.keys(RESOURCE_GROUPS); @@ -41,6 +43,8 @@ function uriOf(args: unknown[]): string | undefined { } export function registerAllResources(server: McpServer, groups: ToolGroup[] = resolveToolGroups()) { + // The graph schema needs the graph access gate as well as its group. + const allowed = permittedGroups(groups); const original = server.resource.bind(server); (server as unknown as { resource: (...a: unknown[]) => unknown }).resource = ( ...args: unknown[] @@ -53,10 +57,11 @@ export function registerAllResources(server: McpServer, groups: ToolGroup[] = re `Add it to RESOURCE_GROUPS in src/resources/index.ts so MCP_TOOL_GROUPS can govern it.` ); } - if (!groups.includes(group)) return undefined; + if (!allowed.includes(group)) return undefined; return (original as (...a: unknown[]) => unknown)(...args); }; registerStaticResources(server); registerResourceTemplates(server); + registerGraphResources(server); } diff --git a/src/tools/cypher-guard.ts b/src/tools/cypher-guard.ts new file mode 100644 index 0000000..5f939ef --- /dev/null +++ b/src/tools/cypher-guard.ts @@ -0,0 +1,353 @@ +// What a `reactome_cypher_query` may ask the database to do. +// +// Neo4j's READ access mode refuses native writes. It does not refuse +// everything that reaches outside the graph or costs the server: procedures +// can read server configuration, kill other queries or sleep; LOAD CSV is a +// read clause and fetches URLs; USE switches database, the system database +// included; SHOW lists transactions, users and procedures. +// +// So this is an allowlist, applied to the query as Neo4j will read it. One +// left-to-right scan splits the text into tokens the way the Cypher lexer +// does -- string literals, backtick-quoted names, line and block comments, +// identifiers, numbers, punctuation -- and the rules are checked on those +// tokens. Separate regex passes over the raw text did not compose: a `/*` +// inside a `//` comment or a string literal made one pass see a block comment +// that Neo4j does not, and hide the text after it. +// +// Rules: +// - CALL only for the built-in schema procedures below, compared by name +// after backticks are removed; no CALL subqueries. +// - SHOW only for indexes and constraints. +// - No name in the apoc or gds namespaces, however it is quoted. +// - No LOAD CSV, TERMINATE, PERIODIC COMMIT, START/STOP DATABASE, no USE +// where a query part begins, and no write or administration keyword +// (CREATE, MERGE, SET, DELETE, REMOVE, DETACH, DROP, GRANT, DENY, REVOKE, +// ALTER, RENAME). SET and SHOW are refused even as variable names: they +// can begin a clause in too many places to tell apart safely. +// - A number literal with a name running straight on from it is refused. +// - No CYPHER options prefix (version, planner, runtime), which would +// otherwise move where a query begins; `cypher` is not usable as a +// variable name either. +// - Text the scanner cannot finish reading -- an unterminated string, +// comment or quoted name -- is refused. +// +// It is a guardrail, not the security boundary, and it errs towards refusing. +// The boundary is that graph access is never enabled on a publicly reachable +// instance (src/graph/gate.ts), plus Neo4j's own configuration. + +/** Procedures a query may CALL, lower-cased. Everything else is refused. */ +const ALLOWED_PROCEDURES = new Set([ + "db.labels", + "db.relationshiptypes", + "db.propertykeys", + "db.schema.nodetypeproperties", + "db.schema.reltypeproperties", + "db.schema.visualization", + "db.indexes", + "db.constraints", + "dbms.components", + // READ-mode procedures in Neo4j 4.3/4.4: they query an existing fulltext + // index. Creating and dropping one are separate procedures, not listed. + "db.index.fulltext.querynodes", + "db.index.fulltext.queryrelationships", +]); + +/** + * Plugin namespaces. Any name in them is refused, called or not: their + * procedures and functions are not part of what a read needs, and several + * write, load from outside the graph, or run other queries. + */ +const PLUGIN_NAMESPACES = ["apoc", "gds"]; + +/** Keywords refused wherever they appear as a keyword. */ +const FORBIDDEN_KEYWORDS = new Set([ + "terminate", + "periodic", + "create", + "merge", + "set", + "delete", + "remove", + "detach", + "drop", + "grant", + "deny", + "revoke", + "alter", + "rename", +]); + +/** Words that may sit between SHOW and INDEXES / CONSTRAINTS. */ +const SHOW_MODIFIERS = new Set([ + "all", + "btree", + "fulltext", + "lookup", + "text", + "range", + "point", + "vector", + "unique", + "uniqueness", + "existence", + "exist", + "exists", + "node", + "relationship", + "rel", + "key", + "property", +]); +const SHOW_TARGETS = new Set(["index", "indexes", "constraint", "constraints"]); + +export class QueryRejected extends Error { + constructor(public readonly reason: string) { + super( + `Query rejected: ${reason}. This server runs read-only queries against the graph and allows ` + + `CALL only for ${[...ALLOWED_PROCEDURES].join(", ")} (no CALL subqueries), ` + + `and SHOW only for indexes and constraints.` + ); + this.name = "QueryRejected"; + } +} + +type Token = + | { kind: "word"; text: string } // an unquoted identifier or keyword + | { kind: "quoted"; text: string } // a backtick-quoted name, unquoted + | { kind: "string" } // a string literal; its contents are not kept + | { kind: "number" } + | { kind: "punct"; text: string }; + +const DIGIT = /[0-9]/; +const IDENTIFIER_START = /[\p{ID_Start}_]/u; +const IDENTIFIER_PART = /[\p{ID_Continue}_]/u; + +/** + * Whitespace as Cypher reads it, and then some: everything Unicode calls + * white space, every control and format character (U+001C-U+001F included), + * and NEL. Treating more as a separator than Neo4j does can only split a + * token the guard then refuses; treating less could join two. + */ +const SEPARATOR = /[\p{White_Space}\p{Cc}\p{Cf}\p{Zs}\p{Zl}\p{Zp}]/u; + +/** Split a query into tokens. Throws QueryRejected on text it cannot finish. */ +export function tokenize(query: string): Token[] { + // Line separators other than LF and CR, anywhere -- inside a comment or a + // string included. Where a `//` comment ends at one of these is exactly the + // kind of disagreement with the Cypher lexer that hides text, so a query + // containing one is refused rather than guessed at. + if (/[\u0085\u2028\u2029]/u.test(query)) { + throw new QueryRejected("the query contains a Unicode line separator"); + } + + const tokens: Token[] = []; + const chars = [...query]; // by code point, so astral characters stay whole + let i = 0; + + const unterminated = (what: string) => { + throw new QueryRejected(`unterminated ${what}`); + }; + + while (i < chars.length) { + const c = chars[i] as string; + const next = chars[i + 1]; + + if (SEPARATOR.test(c)) { + i++; + continue; + } + + // Line comment: to the end of the line. + if (c === "/" && next === "/") { + i += 2; + while (i < chars.length && chars[i] !== "\n" && chars[i] !== "\r") i++; + continue; + } + + // Block comment: to the first */. Nothing inside it means anything. + if (c === "/" && next === "*") { + i += 2; + while (i < chars.length && !(chars[i] === "*" && chars[i + 1] === "/")) i++; + if (i >= chars.length) unterminated("comment"); + i += 2; + continue; + } + + // String literal, single or double quoted, with backslash escapes. + if (c === "'" || c === '"') { + i++; + while (i < chars.length && chars[i] !== c) i += chars[i] === "\\" ? 2 : 1; + if (i >= chars.length) unterminated("string literal"); + i++; + tokens.push({ kind: "string" }); + continue; + } + + // Backtick-quoted name; a doubled backtick is a literal one. + if (c === "`") { + let text = ""; + i++; + for (;;) { + if (i >= chars.length) unterminated("quoted name"); + if (chars[i] === "`") { + if (chars[i + 1] === "`") { + text += "`"; + i += 2; + continue; + } + i++; + break; + } + text += chars[i]; + i++; + } + tokens.push({ kind: "quoted", text }); + continue; + } + + // A number literal, read the way Cypher defines one: decimal with an + // optional fraction and exponent (`1`, `1.5`, `.5`, `1e3`, `1.5E-3`), + // hexadecimal `0x1F`, octal `0o17` (or `017`, which the decimal rule + // covers). A name running straight on from one -- `1e0CALL`, `0x1LOAD`, + // `1_000` -- is refused rather than guessed at: where the lexer splits + // that is exactly what decides whether a keyword is seen. + if (DIGIT.test(c) || (c === "." && DIGIT.test(next ?? ""))) { + const at = (k: number) => chars[k] ?? ""; + if (c === "0" && /[xX]/.test(at(i + 1))) { + i += 2; + while (/[0-9a-fA-F]/.test(at(i))) i++; + } else if (c === "0" && /[oO]/.test(at(i + 1))) { + i += 2; + while (/[0-7]/.test(at(i))) i++; + } else { + while (DIGIT.test(at(i))) i++; + if (at(i) === "." && DIGIT.test(at(i + 1))) { + i++; + while (DIGIT.test(at(i))) i++; + } + const signed = /[+-]/.test(at(i + 1)); + if (/[eE]/.test(at(i)) && DIGIT.test(at(i + (signed ? 2 : 1)))) { + i += signed ? 2 : 1; + while (DIGIT.test(at(i))) i++; + } + } + if (IDENTIFIER_PART.test(at(i)) || at(i) === "`") { + throw new QueryRejected("a name runs straight on from a number literal"); + } + tokens.push({ kind: "number" }); + continue; + } + + if (IDENTIFIER_START.test(c)) { + let text = ""; + while (i < chars.length && IDENTIFIER_PART.test(chars[i] as string)) { + text += chars[i]; + i++; + } + tokens.push({ kind: "word", text }); + continue; + } + + tokens.push({ kind: "punct", text: c }); + i++; + } + + return tokens; +} + +const isName = (t: Token | undefined): t is Extract => + t?.kind === "word" || t?.kind === "quoted"; +const isPunct = (t: Token | undefined, text: string) => t?.kind === "punct" && t.text === text; +const lower = (t: Token | undefined) => (isName(t) ? t.text.toLowerCase() : undefined); + +/** + * Whether the word at `i` is used as a keyword rather than as a name: not a + * property or label (`n.set`, `:Create`), a parameter (`$use`) or a map key + * (`{delete: 1}`). + */ +function isKeyword(tokens: Token[], i: number): boolean { + if (tokens[i]?.kind !== "word") return false; + const before = tokens[i - 1]; + if (isPunct(before, ".") || isPunct(before, ":") || isPunct(before, "$")) return false; + if (isPunct(tokens[i + 1], ":")) return false; + return true; +} + +/** + * Whether token `i` is where a query part begins: the start of the text, or + * after EXPLAIN / PROFILE, UNION [ALL], `;` or `{`. + */ +function beginsQueryPart(tokens: Token[], i: number): boolean { + const before = tokens[i - 1]; + if (before === undefined) return true; + if (isPunct(before, ";") || isPunct(before, "{")) return true; + const word = before.kind === "word" ? before.text.toLowerCase() : undefined; + if (word === "explain" || word === "profile" || word === "union") return true; + return word === "all" && lower(tokens[i - 2]) === "union"; +} + +function check(tokens: Token[]): string | undefined { + for (let i = 0; i < tokens.length; i++) { + const token = tokens[i]; + const word = lower(token); + if (word === undefined) continue; + + // Any plugin name, procedure or function, however it is quoted: + // apoc.x(...), `apoc`.x(...) and `apoc.x`(...) are the same call. + for (const ns of PLUGIN_NAMESPACES) { + if ( + (word === ns && isPunct(tokens[i + 1], ".")) || + (token?.kind === "quoted" && word.startsWith(`${ns}.`)) + ) { + return `${ns.toUpperCase()} procedures and functions are not allowed`; + } + } + + if (!isKeyword(tokens, i)) continue; + + if (word === "load" && lower(tokens[i + 1]) === "csv") return "LOAD CSV is not allowed"; + // The pre-parser prefix `CYPHER [version] [key=value]*` is stripped before + // the query is parsed, so whatever follows it begins the query -- a USE + // included -- though the token before that is not where a query begins. + // No tool needs planner or runtime options, so it is refused outright, + // wherever it is a keyword (`n.cypher` and `:Cypher` are names). + if (word === "cypher") return "the CYPHER options prefix is not allowed"; + // Administration only when followed by DATABASE; otherwise start and + // stop are ordinary names, common in path queries. + if ((word === "start" || word === "stop") && lower(tokens[i + 1]) === "database") { + return `${word.toUpperCase()} DATABASE is not allowed`; + } + // USE is a clause only where a query, a UNION part or a subquery begins; + // anywhere else `use` is a name. + if (word === "use" && beginsQueryPart(tokens, i)) return "USE is not allowed"; + if (FORBIDDEN_KEYWORDS.has(word)) return `${word.toUpperCase()} is not allowed`; + + if (word === "show") { + let j = i + 1; + while (SHOW_MODIFIERS.has(lower(tokens[j]) ?? "")) j++; + if (!SHOW_TARGETS.has(lower(tokens[j]) ?? "")) { + return "SHOW is allowed only for indexes and constraints"; + } + } + + if (word === "call") { + const parts: string[] = []; + let j = i + 1; + while (isName(tokens[j])) { + parts.push((tokens[j] as { text: string }).text.toLowerCase()); + if (!isPunct(tokens[j + 1], ".")) break; + j += 2; + } + if (parts.length === 0) return "CALL subqueries are not allowed"; + const name = parts.join("."); + if (!ALLOWED_PROCEDURES.has(name)) return `CALL ${name} is not allowed`; + } + } + return undefined; +} + +/** Throw `QueryRejected` unless the query passes every rule. */ +export function assertReadOnlyQuery(query: string): void { + const reason = check(tokenize(query)); + if (reason) throw new QueryRejected(reason); +} diff --git a/src/tools/cypher.ts b/src/tools/cypher.ts new file mode 100644 index 0000000..830ab2d --- /dev/null +++ b/src/tools/cypher.ts @@ -0,0 +1,222 @@ +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { z } from "zod"; +import { runRead, runReadLimited } from "../clients/neo4j.js"; +import { fetchGraphSchema } from "../graph/schema.js"; +import { formatGraphSchemaMarkdown } from "../graph/format-schema.js"; +import { logger } from "../logger.js"; +import { assertReadOnlyQuery } from "./cypher-guard.js"; +import { isGraphAccessEnabled } from "../graph/gate.js"; + +const MAX_QUERY_CHARS = 50_000; +const MAX_ROWS_DEFAULT = 100; +const MAX_ROWS_CAP = 1000; +const MAX_ROW_CHARS_DEFAULT = 2000; +const MAX_ROW_CHARS_CAP = 20000; +const MAX_TOTAL_CHARS_DEFAULT = 40000; +const MAX_TOTAL_CHARS_CAP = 200000; + +interface RowSummary { + __truncated: true; + reason: "row_too_large"; + keys: string[]; + original_chars: number; +} + +function summarizeRow(row: Record, json: string): RowSummary { + return { + __truncated: true, + reason: "row_too_large", + keys: Object.keys(row), + original_chars: json.length, + }; +} + +export interface CapRowsStats { + rowCountTruncated: boolean; + rowsWidthTruncated: number; + stoppedAtTotal: boolean; +} + +export function capCypherRows( + rows: Record[], + maxRows: number, + maxRowChars: number, + maxTotalChars: number +): { output: unknown[]; stats: CapRowsStats } { + const rowCountTruncated = rows.length > maxRows; + const limitedRows = rowCountTruncated ? rows.slice(0, maxRows) : rows; + + const output: unknown[] = []; + let totalChars = 0; + let rowsWidthTruncated = 0; + let stoppedAtTotal = false; + + for (const row of limitedRows) { + const raw = JSON.stringify(row); + const entry: unknown = raw.length > maxRowChars ? summarizeRow(row, raw) : row; + const entryChars = JSON.stringify(entry).length; + if (totalChars + entryChars > maxTotalChars) { + stoppedAtTotal = true; + break; + } + if (entry !== row) rowsWidthTruncated++; + output.push(entry); + totalChars += entryChars; + } + + return { + output, + stats: { rowCountTruncated, rowsWidthTruncated, stoppedAtTotal }, + }; +} + +/** + * The `cypher` tool group. + * + * Registers nothing unless the graph access gate is open -- checked here as + * well as by the caller, so a path that reaches this registrar without going + * through `resolveToolGroups()` still cannot publish these tools. + */ +export function registerCypherTools(server: McpServer) { + if (!isGraphAccessEnabled()) return; + + server.tool( + "reactome_cypher_query", + "Run a read-only Cypher query against the Reactome graph database (Neo4j). Call reactome_cypher_schema first; never guess labels or properties. The session runs in READ mode, which rejects CREATE/MERGE/DELETE/SET/REMOVE. CALL is allowed only for the built-in schema procedures (db.labels, db.relationshipTypes, db.propertyKeys, db.schema.*, db.indexes, db.constraints, dbms.components) and the fulltext query procedures (db.index.fulltext.queryNodes, db.index.fulltext.queryRelationships). Rejected before execution: every other procedure (creating or dropping a fulltext index included), CALL subqueries (CALL { ... }), APOC and GDS, LOAD CSV, USE, SHOW other than indexes/constraints, and write or admin clauses. The CYPHER options prefix (version, planner, runtime) is refused. SET, SHOW and CYPHER cannot be used as variable names. A server-side timeout terminates long queries. Rows are streamed and reading stops at max_rows; per-row size and total response size are also capped, so always include a LIMIT and return specific properties (RETURN p.stId, p.displayName) rather than whole nodes.", + { + query: z + .string() + .min(1) + .max(MAX_QUERY_CHARS) + .describe(`Cypher query to execute (read-only; max ${MAX_QUERY_CHARS} chars)`), + params: z + .record(z.string(), z.unknown()) + .optional() + .describe("Optional parameter map passed to the query"), + max_rows: z + .number() + .int() + .positive() + .max(MAX_ROWS_CAP) + .optional() + .default(MAX_ROWS_DEFAULT) + .describe(`Maximum rows to return (default ${MAX_ROWS_DEFAULT}, cap ${MAX_ROWS_CAP})`), + max_row_chars: z + .number() + .int() + .positive() + .max(MAX_ROW_CHARS_CAP) + .optional() + .default(MAX_ROW_CHARS_DEFAULT) + .describe( + `Maximum JSON chars per row before replacing with a summary (default ${MAX_ROW_CHARS_DEFAULT}, cap ${MAX_ROW_CHARS_CAP})` + ), + max_total_chars: z + .number() + .int() + .positive() + .max(MAX_TOTAL_CHARS_CAP) + .optional() + .default(MAX_TOTAL_CHARS_DEFAULT) + .describe( + `Maximum total JSON chars across all rows before truncation (default ${MAX_TOTAL_CHARS_DEFAULT}, cap ${MAX_TOTAL_CHARS_CAP})` + ), + }, + async ({ query, params, max_rows, max_row_chars, max_total_chars }) => { + assertReadOnlyQuery(query); + logger.info("cypher_query", { chars: query.length, max_rows }); + // Streamed and cut off at max_rows + 1 -- the cap is applied while + // reading, not after the whole result has been pulled into memory. + const { rows, more } = await runReadLimited>( + query, + params ?? {}, + max_rows + ); + const { output, stats } = capCypherRows(rows, max_rows, max_row_chars, max_total_chars); + + const notes: string[] = []; + notes.push(`**Rows returned:** ${output.length}`); + if (more) { + notes.push( + `(more rows exist; reading stopped at the max_rows cap of ${max_rows} -- add a LIMIT, ORDER BY or a narrower MATCH)` + ); + } + if (stats.rowsWidthTruncated > 0) + notes.push(`**Wide rows summarized:** ${stats.rowsWidthTruncated}`); + if (stats.stoppedAtTotal) + notes.push(`**Total size cap reached (${max_total_chars} chars); later rows omitted.**`); + + const body = [ + `## Cypher Result`, + ...notes, + "", + "```json", + JSON.stringify(output, null, 2), + "```", + ]; + + return { + content: [{ type: "text", text: body.join("\n") }], + }; + } + ); + + server.tool( + "reactome_cypher_schema", + "Introspect the Reactome graph schema: labels and relationship types with counts, which labels each relationship type connects, property names and types with mandatory flags, indexes and constraints. Read live from Neo4j's built-in schema procedures and cached for the life of the server (the server warms it at startup; a cold first call can take tens of seconds). Call this before writing Cypher. Sections the database could not provide are listed as unavailable rather than shown empty. For the full JSON, read the `reactome://graph/schema` resource.", + {}, + async () => { + logger.info("cypher_schema"); + const schema = await fetchGraphSchema(); + return { + content: [{ type: "text", text: formatGraphSchemaMarkdown(schema) }], + }; + } + ); + + server.tool( + "reactome_cypher_sample", + "Return a small sample of nodes for a given label, to inspect shape and typical property values.", + { + label: z + .string() + .min(1) + .max(200) + .describe("Node label to sample (e.g. 'Pathway', 'ReactionLikeEvent', 'PhysicalEntity')"), + limit: z + .number() + .int() + .positive() + .max(50) + .optional() + .default(5) + .describe("Number of nodes to return (default 5, max 50)"), + }, + async ({ label, limit }) => { + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(label)) { + throw new Error(`Invalid label: ${label}`); + } + logger.info("cypher_sample", { label, limit }); + const rows = await runRead<{ n: Record }>( + `MATCH (n:\`${label}\`) RETURN n LIMIT $limit`, + { limit } + ); + + const lines = [ + `## Sample of \`${label}\` (${rows.length})`, + "", + "```json", + JSON.stringify( + rows.map(r => r.n), + null, + 2 + ), + "```", + ]; + + return { + content: [{ type: "text", text: lines.join("\n") }], + }; + } + ); +} diff --git a/src/tools/index.ts b/src/tools/index.ts index d011fee..63a29d5 100644 --- a/src/tools/index.ts +++ b/src/tools/index.ts @@ -12,6 +12,8 @@ import { registerEntityTools } from "./entity.js"; import { registerExportTools } from "./export.js"; import { registerInteractorTools } from "./interactors.js"; import { registerGsaTools } from "./gsa.js"; +import { registerCypherTools } from "./cypher.js"; +import { isGraphAccessEnabled } from "../graph/gate.js"; import { withNewRequestContext } from "../context.js"; import { capToolResult } from "../response-limits.js"; import { MAX_TOOL_RESPONSE_CHARS } from "../config.js"; @@ -70,12 +72,29 @@ export const TOOL_GROUPS = { interactors: registerInteractorTools, gsa: registerGsaTools, utilities: registerUtilityTools, + // Graph database access. Never registered unless the gate in + // src/graph/gate.ts is open -- see permittedGroups(). + cypher: registerCypherTools, } as const satisfies Record void>; export type ToolGroup = keyof typeof TOOL_GROUPS; export const ALL_TOOL_GROUPS = Object.keys(TOOL_GROUPS) as ToolGroup[]; +/** The one group that additionally needs the graph access gate open. */ +export const GRAPH_GROUP: ToolGroup = "cypher"; + +/** + * `groups`, less the graph group unless the graph access gate is open. + * + * Applied by every registrar and by the instructions -- not only by + * `resolveToolGroups()` -- so a caller that builds its own group list still + * cannot get past the gate. + */ +export function permittedGroups(groups: readonly ToolGroup[]): ToolGroup[] { + return isGraphAccessEnabled() ? [...groups] : groups.filter(g => g !== GRAPH_GROUP); +} + /** * Which groups of tools this instance registers, from `MCP_TOOL_GROUPS`. * @@ -83,9 +102,8 @@ export const ALL_TOOL_GROUPS = Object.keys(TOOL_GROUPS) as ToolGroup[]; * deployment names the groups it means to publish. * * **This is a registration switch, not a documentation one.** A tool that is - * described nowhere but still answers is exactly the divergence that cost - * this repo a public Cypher surface and a server advertising tools it had not - * registered: what a server *offers* must be the same fact as what it *says*, + * described nowhere but still answers, or one described but not registered, + * is a divergence: what a server *offers* must be the same fact as what it *says*, * and the only way to be sure is for the unwanted tool not to exist on the * instance. * @@ -106,7 +124,9 @@ export const ALL_TOOL_GROUPS = Object.keys(TOOL_GROUPS) as ToolGroup[]; export function resolveToolGroups( raw: string | undefined = process.env.MCP_TOOL_GROUPS ): ToolGroup[] { - if (raw === undefined) return ALL_TOOL_GROUPS; + // "Everything" means everything this instance is permitted: the graph + // group needs NEO4J_URI and MCP_ALLOW_CYPHER=1 as well. + if (raw === undefined) return permittedGroups(ALL_TOOL_GROUPS); const requested = raw .split(",") @@ -128,6 +148,17 @@ export function resolveToolGroups( ); } + // Asking for graph access and not getting it should be loud. The opposite + // -- quietly dropping the group -- would leave an operator wondering why + // the tools are missing; quietly *adding* it is what the gate prevents. + if (requested.includes(GRAPH_GROUP) && !isGraphAccessEnabled()) { + throw new Error( + `MCP_TOOL_GROUPS names "${GRAPH_GROUP}", but graph database access is off. ` + + `It needs both NEO4J_URI and MCP_ALLOW_CYPHER=1, and must never be enabled ` + + `on a publicly reachable instance.` + ); + } + // Deduplicated, and in the map's order rather than the caller's, so the // registration order does not depend on how the variable was written. return ALL_TOOL_GROUPS.filter(name => requested.includes(name)); @@ -136,17 +167,14 @@ export function resolveToolGroups( export function registerAllTools(server: McpServer, groups: ToolGroup[] = resolveToolGroups()) { installToolWrapper(server); - // No graph database tools. They were removed on 2026-09-21 when this - // server became publicly hosted: Constitution Principle IV already said no - // deployment holds a Neo4j connection, and a gate enforcing that is a gate - // somebody can flip. Nothing here opens one now. - - for (const group of groups) TOOL_GROUPS[group](server); + const registering = permittedGroups(groups); + for (const group of registering) TOOL_GROUPS[group](server); - if (groups.length < ALL_TOOL_GROUPS.length) { - const omitted = ALL_TOOL_GROUPS.filter(g => !groups.includes(g)); + const available = permittedGroups(ALL_TOOL_GROUPS); + if (registering.length < available.length) { + const omitted = available.filter(g => !registering.includes(g)); logger.info("registering a subset of tool groups", { - registered: groups, + registered: registering, omitted, source: "MCP_TOOL_GROUPS", }); diff --git a/tests/cypher-guard.test.ts b/tests/cypher-guard.test.ts new file mode 100644 index 0000000..311feff --- /dev/null +++ b/tests/cypher-guard.test.ts @@ -0,0 +1,327 @@ +import { describe, it, expect } from "vitest"; +import { assertReadOnlyQuery, QueryRejected } from "../src/tools/cypher-guard.js"; + +/** + * The query guard is an allowlist. A query may CALL only the read-only + * built-ins the schema tooling uses; every other procedure, every APOC name, + * LOAD CSV, USE, and any SHOW other than indexes and constraints is refused + * before it reaches the database. + * + * It is a guardrail, not the boundary -- the boundary is that graph access is + * off on any publicly reachable instance -- but a denylist is a list of the + * ways somebody already thought of, and the review found eleven it missed. + */ + +const accepted = (query: string) => expect(() => assertReadOnlyQuery(query)).not.toThrow(); +const refused = (query: string) => expect(() => assertReadOnlyQuery(query)).toThrow(QueryRejected); + +describe("assertReadOnlyQuery: what is allowed", () => { + it("accepts plain read queries", () => { + accepted("MATCH (n:Pathway) RETURN n LIMIT 10"); + accepted("MATCH (p:Pathway {stId: $id})-[:hasEvent]->(e) RETURN e.displayName LIMIT 25"); + accepted("MATCH (n) WHERE n.displayName STARTS WITH 'Signal' RETURN n.stId LIMIT 5"); + accepted("MATCH (n:Pathway) RETURN n.create, n.set, n.delete, n.use LIMIT 1"); + accepted("RETURN {set: 1, delete: 2, call: 3} AS m"); + }); + + it("accepts apoc, call, use and load as words inside data", () => { + accepted(`MATCH (n) WHERE n.note = 'see apoc docs' RETURN n`); + accepted("MATCH (n) RETURN n.csv, n.load, n.use LIMIT 1"); + }); + + it("accepts the read-only built-in procedures", () => { + for (const q of [ + "CALL db.labels() YIELD label RETURN label", + "CALL db.relationshipTypes()", + "CALL db.propertyKeys()", + "CALL db.schema.nodeTypeProperties()", + "CALL db.schema.relTypeProperties()", + "CALL db.schema.visualization()", + "CALL db.indexes()", + "CALL db.constraints()", + "CALL dbms.components() YIELD name, versions, edition RETURN *", + "call DB.LABELS()", + // Backtick-quoted parts are the same name once unquoted. + "CALL `db`.`labels`() YIELD label RETURN label", + "CALL db.`schema`.nodeTypeProperties()", + "SHOW INDEXES", + "SHOW ALL INDEXES YIELD name RETURN name", + "SHOW CONSTRAINTS", + "SHOW UNIQUE CONSTRAINTS", + ]) { + accepted(q); + } + }); +}); + +describe("assertReadOnlyQuery: refuses", () => { + it("every APOC name, called or not", () => { + for (const q of [ + "CALL apoc.cypher.runWrite('CREATE (n) RETURN n', {})", + "CALL APOC.CYPHER.DOIT('CREATE (n)', {})", + "CALL apoc.periodic.iterate('MATCH (n) RETURN n', 'SET n.flag = 1', {})", + "CALL apoc.create.node(['X'], {}) YIELD node RETURN node", + "CALL apoc.refactor.mergeNodes([n1, n2])", + "CALL apoc.load.json('http://evil.example/data') YIELD value RETURN value", + "CALL apoc.export.csv.all('/tmp/x.csv', {})", + "CALL apoc.nodes.delete([1,2,3], 100)", + "CALL apoc.meta.schema() YIELD value RETURN value", + "MATCH (n) CALL apoc.path.subgraphAll(n, {}) YIELD nodes RETURN nodes", + // The review's list: none of these was on the old denylist. + "CALL apoc.cypher.runMany('CREATE (n);', {})", + "CALL apoc.cypher.runFile('x.cypher')", + "CALL apoc.cypher.runSchema('CREATE INDEX FOR (n:X) ON (n.y)', {})", + "CALL apoc.cypher.parallel('MATCH (n) RETURN n', {}, 'x')", + "CALL apoc.schema.assert({}, {})", + "CALL apoc.util.sleep(600000)", + "CALL apoc.custom.asProcedure('x', 'CREATE (n)')", + "CALL apoc.systemdb.execute('SHOW USERS')", + "CALL apoc.lock.nodes([n])", + "RETURN apoc.text.join(['a'], ',')", + ]) { + refused(q); + } + }); + + it("dbms and db procedures outside the allowlist", () => { + for (const q of [ + "CALL dbms.killQuery('query-1')", + "CALL dbms.listQueries()", + "CALL dbms.listConfig()", + "CALL dbms.security.createUser('x', 'y', false)", + "CALL db.createLabel('X')", + "CALL db.index.fulltext.createNodeIndex('x', ['X'], ['y'])", + "CALL db.clearQueryCaches()", + "CALL db.stats.retrieve('GRAPH COUNTS')", + // A known name with a suffix is a different procedure. + "CALL db.labelsAndMore()", + ]) { + refused(q); + } + }); + + it("USE, which switches database -- system included", () => { + refused("USE system SHOW USERS"); + refused("use neo4j MATCH (n) RETURN n LIMIT 1"); + refused("CALL { USE system RETURN 1 AS x } RETURN x"); + }); + + it("SHOW and TERMINATE other than indexes and constraints", () => { + refused("SHOW TRANSACTIONS"); + refused("SHOW PROCEDURES"); + refused("SHOW DATABASES"); + refused("TERMINATE TRANSACTIONS 'neo4j-transaction-1'"); + }); + + it("LOAD CSV, which READ mode allows and which fetches URLs and files", () => { + refused("LOAD CSV FROM 'http://evil.example/x.csv' AS row RETURN row"); + refused("load csv with headers from 'file:///etc/passwd' as r return r"); + }); +}); + +describe("assertReadOnlyQuery: is not bypassed by", () => { + it("a comment splitting a keyword", () => { + // Comments used to be deleted outright, so LOAD/**/CSV became LOADCSV and + // matched nothing. A comment separates tokens; it is read as a space. + refused("LOAD/**/CSV FROM 'http://x' AS r RETURN r"); + refused("LOAD/* a */CSV FROM 'http://x' AS r RETURN r"); + refused("LOAD//\nCSV FROM 'http://x' AS r RETURN r"); + refused("CALL/**/dbms.listConfig()"); + refused("CALL/**/apoc.util.sleep(1000)"); + refused("USE/**/system SHOW USERS"); + refused("SHOW/**/TRANSACTIONS"); + }); + + it("a comment splitting a procedure name", () => { + refused("CALL apoc.load/* x */.json('http://evil.example/') YIELD value"); + refused("CALL dbms/* x */.listConfig()"); + }); + + it("backtick-quoting or spacing the name", () => { + refused("CALL `apoc`.`load`.`json`('http://evil.example/') YIELD value"); + refused("CALL `dbms`.`listConfig`()"); + refused("CALL dbms . listConfig ( )"); + refused("CALL `dbms.listConfig`()"); + }); + + it("a '//' inside a string literal", () => { + refused( + "WITH 'http://x' AS u CALL apoc.load.json('http://evil.example/') YIELD value RETURN value" + ); + refused("WITH 'a//' AS s CALL dbms.listConfig() YIELD name RETURN name"); + }); + + it("a comment opener inside a line comment or a string", () => { + // Regex passes over the whole text treated `/*` inside a `//` comment as + // opening a block comment, and swallowed what Neo4j actually runs. + refused("// /*\nLOAD/**/CSV FROM 'http://x/' AS r RETURN r"); + refused("// /*\nUSE/**/otherdb MATCH (n) RETURN n"); + refused( + "// /*\nRETURN apoc/**/.cypher.runFirstColumnSingle('CA'+'LL dbms.listQueries() YIELD queryId RETURN queryId', {})" + ); + refused("RETURN '/*' AS a // */\nLOAD CSV FROM 'http://x/' AS r RETURN r"); + refused('RETURN "/*" AS a\nCALL dbms.listConfig() // */'); + refused("RETURN '\\'/*' AS a\nCALL dbms.listConfig()"); + }); + + it("separators Cypher treats as whitespace, and tokens run together", () => { + refused("LOAD\u001CCSV FROM 'http://x/' AS r RETURN r"); + refused("USE\u001Cdb MATCH (n) RETURN n"); + refused("RETURN apoc\u001C.text.join(['a'], ',')"); + refused("MATCH (n) RETURN n LIMIT 1CALL dbms.listConfig()"); + refused("LOAD\u00A0CSV FROM 'http://x/' AS r RETURN r"); + refused("LOAD\u2003CSV FROM 'http://x/' AS r RETURN r"); + refused("LOAD\u0085CSV FROM 'http://x/' AS r RETURN r"); + }); + + it("a line separator a comment might or might not end at", () => { + refused("// x\u2028LOAD CSV FROM 'http://x/' AS r RETURN r"); + refused("// it's\u2029MATCH (n) RETURN n"); + }); + + it("anything it cannot finish reading", () => { + refused("MATCH (n) RETURN 'unterminated"); + refused('MATCH (n) RETURN "unterminated'); + refused("MATCH (n) RETURN n /* unterminated"); + refused("MATCH (n) RETURN n.`unterminated"); + }); +}); + +describe("assertReadOnlyQuery: ignores what Neo4j ignores", () => { + it("text inside comments and string literals", () => { + accepted("// CALL apoc.cypher.runWrite('...')\nMATCH (n) RETURN n LIMIT 1"); + accepted("/* LOAD CSV FROM 'x' */ MATCH (n) RETURN n LIMIT 1"); + accepted("MATCH (n) WHERE n.name = 'USE system; CALL dbms.listConfig()' RETURN n LIMIT 1"); + accepted('MATCH (n) WHERE n.name = "it\\"s apoc.x" RETURN n LIMIT 1'); + }); +}); + +describe("assertReadOnlyQuery: refuses writes and admin as keywords", () => { + it("write and administration clauses", () => { + for (const q of [ + "CREATE (n:X) RETURN n", + "MATCH (n) SET n.x = 1", + "MATCH (n) DETACH DELETE n", + "MATCH (n) REMOVE n.x", + "MERGE (n:X {id: 1})", + "DROP INDEX x", + "GRANT ROLE admin TO bob", + "DENY READ {*} ON GRAPH * TO bob", + "REVOKE ROLE admin FROM bob", + "ALTER USER bob SET PASSWORD 'x'", + "START DATABASE neo4j", + "STOP DATABASE neo4j", + "USING PERIODIC COMMIT LOAD CSV FROM 'x' AS r RETURN r", + ]) { + refused(q); + } + }); + + it("CALL subqueries, which are not needed to read and can run in transactions", () => { + refused("MATCH (n) WITH n LIMIT 5 CALL { WITH n RETURN n.dbId AS id } RETURN id"); + refused("MATCH (n) CALL (n) { RETURN n.dbId AS id } RETURN id"); + }); + + it("fulltext procedures that create or drop, though querying is allowed", () => { + refused("CALL db.index.fulltext.createNodeIndex('x', ['X'], ['y'])"); + refused("CALL db.index.fulltext.drop('x')"); + accepted("CALL db.index.fulltext.queryNodes('x', 'y') YIELD node RETURN node LIMIT 5"); + accepted( + "CALL db.index.fulltext.queryRelationships('x', 'y') YIELD relationship RETURN relationship" + ); + }); +}); + +describe("assertReadOnlyQuery: number literals", () => { + it("are read whole, and refused when a name runs straight on from one", () => { + // Reading only the leading digits of 1e0CALL left `e0CALL` as one name, + // so the CALL was never seen. Neo4j reads 1e0 and then CALL. + for (const q of [ + "MATCH (n) WHERE n.x = 1e0CALL dbms.listQueries() YIELD queryId RETURN queryId", + "MATCH (n) WITH n LIMIT 1E0LOAD CSV FROM 'http://x/' AS r RETURN r", + "WITH 1 AS a LIMIT 0x1LOAD CSV FROM 'http://x/' AS r RETURN r", + "MATCH (n) WHERE n.x = 1.5e3DELETE n", + "RETURN 1e0CALL{ MATCH (n) RETURN n }", + "MATCH (n) WHERE n.x = .5e1DETACH n", + "MATCH (n) WHERE n.x = 0o7LOAD CSV FROM 'x' AS r RETURN r", + "MATCH (n) WHERE n.x = 1_000 RETURN n", + "MATCH (n) WHERE n.x = 1e+5CALL dbms.listConfig() RETURN n", + ]) { + refused(q); + } + }); + + it("in their ordinary forms", () => { + for (const q of [ + "MATCH (n) WHERE n.x = 1.5e3 RETURN n LIMIT 10", + "MATCH (n) WHERE n.x > .5 AND n.y < 1E-3 RETURN n", + "MATCH (n) WHERE n.x = 0x1F OR n.x = 0o17 OR n.x = 017 RETURN n", + "MATCH p = (a)-[*1..3]->(b) RETURN p LIMIT 5", + "RETURN [1,2,3][0..2] AS xs", + "RETURN 1+2-3*4/5%6^7 AS x", + ]) { + accepted(q); + } + }); +}); + +describe("assertReadOnlyQuery: plugin namespaces", () => { + it("refuses apoc and gds however the name is quoted", () => { + for (const q of [ + "RETURN `apoc.text.join`(['a'], ',')", + "CALL `apoc.util.sleep`(1000)", + "RETURN `APOC`.text.join(['a'], ',')", + "CALL gds.graph.project('g', '*', '*')", + "RETURN gds.util.asNode(1)", + "CALL `gds.graph.drop`('g')", + "RETURN `gds`.version()", + ]) { + refused(q); + } + }); +}); + +describe("assertReadOnlyQuery: names that are keywords elsewhere", () => { + it("accepts start and stop as variables; refuses only START/STOP DATABASE", () => { + accepted("MATCH p=(start:Pathway)-[:hasEvent]->(e) RETURN start.displayName LIMIT 5"); + accepted("MATCH (a:Pathway) WITH a AS start RETURN start.stId LIMIT 5"); + accepted("MATCH (stop:Pathway) RETURN stop LIMIT 1"); + refused("START DATABASE neo4j"); + refused("STOP DATABASE neo4j"); + refused("start database neo4j"); + }); + + it("accepts use as a variable; refuses USE where a query or a UNION part starts", () => { + accepted("MATCH (use:Pathway) RETURN use.stId LIMIT 1"); + accepted("WITH 1 AS use RETURN use"); + refused("USE system SHOW USERS"); + refused("EXPLAIN USE neo4j MATCH (n) RETURN n"); + refused("PROFILE USE neo4j MATCH (n) RETURN n"); + refused("MATCH (n) RETURN n.x AS x UNION USE other MATCH (n) RETURN n.x AS x"); + refused("MATCH (n) RETURN n.x AS x UNION ALL USE other MATCH (n) RETURN n.x AS x"); + refused("RETURN 1 AS x; USE system SHOW USERS"); + refused("MATCH (n) WHERE EXISTS { USE other MATCH (m) RETURN m } RETURN n"); + }); +}); + +describe("assertReadOnlyQuery: the CYPHER pre-parser prefix", () => { + it("is refused, so options cannot sit between a query's start and USE", () => { + // Neo4j strips `CYPHER [version] [key=value]*` before parsing, so USE + // after it begins the query even though the token before it is not. + for (const q of [ + "CYPHER 4.4 USE system CALL dbms.components()", + "CYPHER runtime=slotted USE neo4j MATCH (n) RETURN n", + "CYPHER USE system SHOW USERS", + "EXPLAIN CYPHER planner=cost USE x MATCH (n) RETURN n", + "PROFILE CYPHER 4.4 USE x RETURN 1", + "CYPHER 4.4 MATCH (n) RETURN n LIMIT 1", + ]) { + refused(q); + } + }); + + it("does not refuse cypher as a property or label", () => { + accepted("MATCH (n:Pathway) RETURN n.cypher LIMIT 1"); + accepted("MATCH (n:Cypher) RETURN n LIMIT 1"); + }); +}); diff --git a/tests/cypher-tools.test.ts b/tests/cypher-tools.test.ts new file mode 100644 index 0000000..def3b90 --- /dev/null +++ b/tests/cypher-tools.test.ts @@ -0,0 +1,146 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import type { FakeNeo4jState } from "./helpers/fake-neo4j.js"; + +/** + * The Cypher tools, driven through the registration path with a fake driver. + * + * What these hold is the read-only contract: every query runs in a READ + * session under a server-side timeout, the write-through guard refuses before + * a session is even opened, and the response caps apply. + */ + +const fake = vi.hoisted((): FakeNeo4jState => ({ drivers: [], sessions: [], respond: () => [] })); + +vi.mock("neo4j-driver", async () => { + const { fakeNeo4jModule } = await import("./helpers/fake-neo4j.js"); + return fakeNeo4jModule(fake); +}); + +import { createFakeServer, textOf } from "./helpers/fake-server.js"; +import { registerCypherTools } from "../src/tools/cypher.js"; +import { closeDriver } from "../src/clients/neo4j.js"; +import { CYPHER_QUERY_TIMEOUT_MS } from "../src/config.js"; + +describe("reactome_cypher_* tools", () => { + let fakeServer: ReturnType; + + beforeEach(() => { + process.env.NEO4J_URI = "bolt://127.0.0.1:7687"; + process.env.MCP_ALLOW_CYPHER = "1"; + fake.drivers.length = 0; + fake.sessions.length = 0; + fake.respond = () => []; + fakeServer = createFakeServer(); + registerCypherTools(fakeServer.server); + }); + + afterEach(async () => { + await closeDriver(); + delete process.env.NEO4J_URI; + delete process.env.MCP_ALLOW_CYPHER; + }); + + it("registers the three tools when the gate is open", () => { + expect(fakeServer.toolNames().sort()).toEqual([ + "reactome_cypher_query", + "reactome_cypher_sample", + "reactome_cypher_schema", + ]); + }); + + it("says in its description what the guard refuses", () => { + const description = fakeServer.tools.get("reactome_cypher_query")?.description ?? ""; + expect(description).toContain("db.index.fulltext.queryRelationships"); + expect(description).toContain("CALL subqueries"); + expect(description).toContain("LOAD CSV"); + expect(description).toContain("CYPHER"); + }); + + it("runs a query in a READ session, under the configured timeout", async () => { + fake.respond = () => [{ stId: "R-HSA-109582" }]; + const result = await fakeServer.invoke("reactome_cypher_query", { + query: "MATCH (p:Pathway) RETURN p.stId AS stId LIMIT 1", + }); + + expect(textOf(result)).toContain("R-HSA-109582"); + expect(fake.sessions).toHaveLength(1); + expect(fake.sessions[0]?.config.defaultAccessMode).toBe("READ"); + expect(fake.sessions[0]?.runs[0]?.config?.timeout).toBe(CYPHER_QUERY_TIMEOUT_MS); + expect(fake.sessions[0]?.closed).toBe(true); + }); + + it("refuses a write-through procedure before opening a session", async () => { + await expect( + fakeServer.invoke("reactome_cypher_query", { + query: "CALL apoc.periodic.iterate('MATCH (n) RETURN n', 'DETACH DELETE n', {})", + }) + ).rejects.toThrow(/Query rejected/); + expect(fake.drivers).toHaveLength(0); + expect(fake.sessions).toHaveLength(0); + }); + + it("stops reading at max_rows + 1 rather than materialising the whole result", async () => { + // `MATCH (n) RETURN n` on the Reactome graph is ~3M nodes. Capping after + // the fact pulled every one of them into this process first. + let produced = 0; + fake.respond = function* () { + for (let i = 0; i < 3_000_000; i++) { + produced++; + yield { i }; + } + }; + const result = await fakeServer.invoke("reactome_cypher_query", { + query: "MATCH (n) RETURN n", + max_rows: 10, + }); + + const run = fake.sessions[0]?.runs[0]; + expect(run?.pulled).toBe(11); + expect(run?.cancelled).toBe(true); + expect(produced).toBeLessThanOrEqual(11); + // And the driver is asked to fetch in batches no bigger than that. + expect(Number(fake.sessions[0]?.config.fetchSize)).toBeLessThanOrEqual(11); + + expect(textOf(result)).toContain("**Rows returned:** 10"); + expect(textOf(result)).toMatch(/more rows exist/); + }); + + it("reports an exact count, and reads to the end, when the result fits", async () => { + fake.respond = () => [{ i: 1 }, { i: 2 }, { i: 3 }]; + const result = await fakeServer.invoke("reactome_cypher_query", { + query: "UNWIND [1,2,3] AS i RETURN i", + max_rows: 10, + }); + expect(textOf(result)).toContain("**Rows returned:** 3"); + expect(textOf(result)).not.toMatch(/more rows exist/); + expect(fake.sessions[0]?.runs[0]?.cancelled).toBe(false); + }); + + it("refuses a max_rows above the cap, before running anything", () => { + expect(() => + fakeServer.invoke("reactome_cypher_query", { query: "RETURN 1", max_rows: 1_000_000 }) + ).toThrow(/<=1000/); + expect(fake.sessions).toHaveLength(0); + }); + + it("samples a label read-only, and refuses a label that could inject Cypher", async () => { + fake.respond = () => [{ n: { stId: "R-HSA-1" } }]; + const ok = await fakeServer.invoke("reactome_cypher_sample", { label: "Pathway" }); + expect(textOf(ok)).toContain("R-HSA-1"); + expect(fake.sessions[0]?.config.defaultAccessMode).toBe("READ"); + + await expect( + fakeServer.invoke("reactome_cypher_sample", { label: "Pathway`) DETACH DELETE n //" }) + ).rejects.toThrow(/Invalid label/); + }); +}); + +describe("reactome_cypher_* tools with the gate closed", () => { + it("registers nothing, even when called directly", () => { + delete process.env.NEO4J_URI; + delete process.env.MCP_ALLOW_CYPHER; + const fakeServer = createFakeServer(); + registerCypherTools(fakeServer.server); + expect(fakeServer.toolNames()).toEqual([]); + }); +}); diff --git a/tests/cypher.test.ts b/tests/cypher.test.ts new file mode 100644 index 0000000..175d895 --- /dev/null +++ b/tests/cypher.test.ts @@ -0,0 +1,46 @@ +import { describe, it, expect } from "vitest"; +import { capCypherRows } from "../src/tools/cypher.js"; + +describe("capCypherRows", () => { + it("passes small rows through unchanged", () => { + const rows = [{ a: 1 }, { b: 2 }, { c: 3 }]; + const { output, stats } = capCypherRows(rows, 100, 2000, 40000); + expect(output).toEqual(rows); + expect(stats).toEqual({ + rowCountTruncated: false, + rowsWidthTruncated: 0, + stoppedAtTotal: false, + }); + }); + + it("truncates row count when rows.length exceeds max_rows", () => { + const rows = Array.from({ length: 10 }, (_, i) => ({ i })); + const { output, stats } = capCypherRows(rows, 3, 2000, 40000); + expect(output).toHaveLength(3); + expect(stats.rowCountTruncated).toBe(true); + }); + + it("replaces wide rows with a summary object", () => { + const wide = { big: "x".repeat(3000) }; + const { output, stats } = capCypherRows([wide], 10, 500, 40000); + expect(stats.rowsWidthTruncated).toBe(1); + const entry = output[0] as Record; + expect(entry.__truncated).toBe(true); + expect(entry.reason).toBe("row_too_large"); + expect(entry.keys).toEqual(["big"]); + expect(entry.original_chars).toBeGreaterThan(3000); + }); + + it("stops accumulating rows when total char budget is exhausted", () => { + const rows = Array.from({ length: 10 }, () => ({ payload: "y".repeat(100) })); + const { output, stats } = capCypherRows(rows, 100, 2000, 300); + expect(output.length).toBeLessThan(10); + expect(stats.stoppedAtTotal).toBe(true); + }); + + it("reports both width and total-cap truncation together", () => { + const rows = [{ big: "z".repeat(3000) }, { small: "ok" }, { small: "ok" }]; + const { stats } = capCypherRows(rows, 10, 500, 500); + expect(stats.rowsWidthTruncated).toBeGreaterThan(0); + }); +}); diff --git a/tests/graph-access-gate.test.ts b/tests/graph-access-gate.test.ts new file mode 100644 index 0000000..6a991d3 --- /dev/null +++ b/tests/graph-access-gate.test.ts @@ -0,0 +1,421 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { readFileSync, readdirSync, statSync } from "node:fs"; +import { join, relative } from "node:path"; +import type { AddressInfo } from "node:net"; +import type { FakeNeo4jState } from "./helpers/fake-neo4j.js"; + +/** + * Graph database access is off unless an operator turns it on, twice. + * + * The Cypher tools exist for internal instances pointed at a dedicated + * Reactome graph database. A publicly hosted instance must never offer them: + * an arbitrary query from the internet can take the database -- and the + * website in front of it -- down. + * + * The gate is `NEO4J_URI` set AND `MCP_ALLOW_CYPHER=1`. These tests do not + * check the predicate once and trust the call sites: they drive **every** + * surface, through every entrypoint and factory, in every combination of the + * two variables, and count both loads of `neo4j-driver` and driver + * constructions on a fake that cannot open a socket. Each surface is asserted + * in both directions: "absent when closed" alone would pass against a build + * that never offers graph access at all. A new entrypoint belongs here. + */ + +const fake = vi.hoisted((): FakeNeo4jState => ({ drivers: [], sessions: [], respond: () => [] })); + +/** + * Registered per test, after `vi.resetModules()`, rather than once with + * `vi.mock`: a hoisted mock's factory runs once per file, so it could not + * count whether a given entrypoint loaded `neo4j-driver` at all. Re-registered + * like this, the factory runs on the first import after each reset, and + * `fake.moduleLoads` says whether anything imported it. + */ +function mockDriverModule() { + vi.doMock("neo4j-driver", async () => { + const { fakeNeo4jModule } = await import("./helpers/fake-neo4j.js"); + return fakeNeo4jModule(fake); + }); +} + +// The stdio entrypoint connects a transport on import. This one talks to +// nothing, so importing `src/index.ts` runs the real startup path -- server +// construction, and the schema warm-up -- without touching stdin/stdout. +vi.mock("@modelcontextprotocol/sdk/server/stdio.js", () => ({ + StdioServerTransport: class { + onclose?: () => void; + onerror?: (e: Error) => void; + onmessage?: (m: unknown) => void; + async start() {} + async close() {} + async send() {} + }, +})); + +const URI = "bolt://127.0.0.1:7687"; +const GRAPH_VARS = ["NEO4J_URI", "MCP_ALLOW_CYPHER", "MCP_TOOL_GROUPS", "MCP_HTTP_PORT"] as const; + +interface GateCase { + name: string; + env: Partial>; + open: boolean; +} + +const CASES: GateCase[] = [ + { name: "neither variable set", env: {}, open: false }, + // The case that matters most: a connection string is not consent. This is + // the configuration a public instance would most plausibly arrive at. + { name: "NEO4J_URI alone", env: { NEO4J_URI: URI }, open: false }, + { name: "MCP_ALLOW_CYPHER=1 alone", env: { MCP_ALLOW_CYPHER: "1" }, open: false }, + { + name: "NEO4J_URI with MCP_ALLOW_CYPHER=true (only 1 counts)", + env: { NEO4J_URI: URI, MCP_ALLOW_CYPHER: "true" }, + open: false, + }, + { + name: "a blank NEO4J_URI with MCP_ALLOW_CYPHER=1", + env: { NEO4J_URI: " ", MCP_ALLOW_CYPHER: "1" }, + open: false, + }, + { name: "both set", env: { NEO4J_URI: URI, MCP_ALLOW_CYPHER: "1" }, open: true }, +]; + +const CYPHER_TOOLS = ["reactome_cypher_query", "reactome_cypher_schema", "reactome_cypher_sample"]; +const SCHEMA_URI = "reactome://graph/schema"; + +let saved: Record = {}; + +function applyEnv(env: GateCase["env"]) { + saved = {}; + for (const key of GRAPH_VARS) { + saved[key] = process.env[key]; + delete process.env[key]; + } + for (const [key, value] of Object.entries(env)) process.env[key] = value; +} + +function restoreEnv() { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } +} + +function resetFake() { + fake.drivers.length = 0; + fake.sessions.length = 0; + fake.respond = () => []; + fake.moduleLoads = 0; +} + +/** Let fire-and-forget startup work (the schema warm-up) run to completion. */ +async function settle() { + for (let i = 0; i < 5; i++) await new Promise(resolve => setTimeout(resolve, 5)); +} + +const cypherNames = (names: string[]) => names.filter(n => n.startsWith("reactome_cypher")); + +function recordingServer() { + const tools: string[] = []; + const resources: string[] = []; + const server = { + tool: (...args: unknown[]) => { + if (typeof args[0] === "string") tools.push(args[0]); + return undefined; + }, + resource: (...args: unknown[]) => { + const t = args[1] as { uriTemplate?: { toString(): string } } | undefined; + const uri = typeof args[1] === "string" ? args[1] : t?.uriTemplate?.toString(); + if (uri) resources.push(uri); + return undefined; + }, + }; + return { server: server as never, tools, resources }; +} + +describe.each(CASES)("graph access gate: $name", ({ env, open }) => { + beforeEach(() => { + applyEnv(env); + resetFake(); + vi.resetModules(); + mockDriverModule(); + }); + + afterEach(() => { + restoreEnv(); + vi.doUnmock("../src/http.js"); + vi.resetModules(); + }); + + it(`the shared predicate reports ${open ? "open" : "closed"}`, async () => { + const { isGraphAccessEnabled } = await import("../src/graph/gate.js"); + expect(isGraphAccessEnabled()).toBe(open); + }); + + it(`createServer ${open ? "offers" : "withholds"} the tools, the resource and the instructions, and constructs no driver`, async () => { + const { createServer } = await import("../src/server.js"); + const server = createServer() as unknown as { + _registeredTools: Record; + _registeredResources: Record; + server: { _instructions?: string }; + }; + + const tools = Object.keys(server._registeredTools); + const resources = Object.keys(server._registeredResources); + const instructions = server.server._instructions ?? ""; + + // The REST surface is there either way; the server is usable. + expect(tools).toContain("reactome_search"); + expect(instructions).toContain("reactome_search"); + + if (open) { + expect(cypherNames(tools).sort()).toEqual([...CYPHER_TOOLS].sort()); + expect(resources).toContain(SCHEMA_URI); + expect(instructions).toContain("reactome_cypher_query"); + } else { + expect(cypherNames(tools)).toEqual([]); + expect(resources).not.toContain(SCHEMA_URI); + expect(instructions.toLowerCase()).not.toContain("cypher"); + expect(instructions.toLowerCase()).not.toContain("neo4j"); + } + + // Building a server is not a reason to connect: the driver is lazy. + expect(fake.drivers).toHaveLength(0); + }); + + it(`registrars given every group explicitly still ${open ? "offer" : "withhold"} graph access`, async () => { + // Defence in depth: a caller that bypasses resolveToolGroups() and hands + // the registrars the full group list must not get past the gate either. + const { registerAllTools, TOOL_GROUPS } = await import("../src/tools/index.js"); + const { registerAllResources } = await import("../src/resources/index.js"); + const { buildServerInstructions } = await import("../src/instructions.js"); + const everyGroup = Object.keys(TOOL_GROUPS) as Parameters[1]; + + const rec = recordingServer(); + registerAllTools(rec.server, everyGroup); + registerAllResources(rec.server, everyGroup); + const text = buildServerInstructions(everyGroup); + + if (open) { + expect(cypherNames(rec.tools)).toHaveLength(3); + expect(rec.resources).toContain(SCHEMA_URI); + expect(text).toContain("reactome_cypher_schema"); + } else { + expect(cypherNames(rec.tools)).toEqual([]); + expect(rec.resources).not.toContain(SCHEMA_URI); + expect(text.toLowerCase()).not.toContain("cypher"); + } + expect(fake.drivers).toHaveLength(0); + }); + + it(`MCP_TOOL_GROUPS naming cypher ${open ? "is honoured" : "refuses to start"}`, async () => { + const { resolveToolGroups } = await import("../src/tools/index.js"); + if (open) { + expect(resolveToolGroups("search,cypher")).toEqual(["search", "cypher"]); + } else { + // Asking for graph access and not getting it is loud, not silent. + expect(() => resolveToolGroups("search,cypher")).toThrow(/MCP_ALLOW_CYPHER/); + } + }); + + it(`the driver factory ${open ? "constructs a driver" : "refuses to construct one"}`, async () => { + const { getDriver, runRead } = await import("../src/clients/neo4j.js"); + if (open) { + expect(await getDriver()).toBeDefined(); + expect(fake.drivers).toHaveLength(1); + } else { + await expect(getDriver()).rejects.toThrow(/MCP_ALLOW_CYPHER/); + await expect(runRead("RETURN 1")).rejects.toThrow(/MCP_ALLOW_CYPHER/); + expect(fake.drivers).toHaveLength(0); + // Refused before the library is even loaded. + expect(fake.moduleLoads).toBe(0); + } + }); + + it(`the stdio entrypoint ${open ? "warms the schema over one driver" : "never constructs a driver"}`, async () => { + await import("../src/index.js"); + await settle(); + expect(fake.drivers).toHaveLength(open ? 1 : 0); + // A closed instance never loads neo4j-driver at all. + expect(fake.moduleLoads).toBe(open ? 1 : 0); + if (open) { + // The warm-up ran, read-only, on the dedicated graph. + expect(fake.drivers[0]?.uri).toBe(URI); + expect(fake.sessions.length).toBeGreaterThan(0); + for (const s of fake.sessions) expect(s.config.defaultAccessMode).toBe("READ"); + } + }); + + it(`the HTTP entrypoint ${open ? "warms the schema over one driver" : "never constructs a driver"}`, async () => { + // src/http-server.ts is the entrypoint a hosted instance runs. Its + // transport is stubbed so nothing binds; everything it does after the + // server starts is real. + vi.doMock("../src/http.js", () => ({ startHttpServer: vi.fn(async () => ({})) })); + process.env.MCP_HTTP_PORT = "4999"; + await import("../src/http-server.js"); + await settle(); + expect(fake.drivers).toHaveLength(open ? 1 : 0); + expect(fake.moduleLoads).toBe(open ? 1 : 0); + }); + + it(`startHttpServer sessions ${open ? "offer" : "withhold"} graph access, and construct no driver`, async () => { + const { startHttpServer } = await import("../src/http.js"); + const http = await startHttpServer(0, "127.0.0.1"); + try { + const { port } = http.address() as AddressInfo; + const base = `http://127.0.0.1:${port}`; + const headers = { + "Content-Type": "application/json", + Accept: "application/json, text/event-stream", + }; + + const init = await fetch(`${base}/mcp`, { + method: "POST", + headers, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2024-11-05", + capabilities: {}, + clientInfo: { name: "vitest", version: "0" }, + }, + }), + }); + const initText = await init.text(); + const sessionId = init.headers.get("mcp-session-id"); + expect(sessionId).toBeTruthy(); + const session = { ...headers, "mcp-session-id": sessionId ?? "" }; + + await fetch(`${base}/mcp`, { + method: "POST", + headers: session, + body: JSON.stringify({ jsonrpc: "2.0", method: "notifications/initialized" }), + }); + + const list = async (method: string) => { + const res = await fetch(`${base}/mcp`, { + method: "POST", + headers: session, + body: JSON.stringify({ jsonrpc: "2.0", id: 2, method }), + }); + return res.text(); + }; + const toolsText = await list("tools/list"); + const resourcesText = await list("resources/list"); + const health = (await (await fetch(`${base}/health`)).json()) as Record; + + expect(toolsText).toContain("reactome_search"); + if (open) { + for (const name of CYPHER_TOOLS) expect(toolsText).toContain(`"${name}"`); + expect(resourcesText).toContain(SCHEMA_URI); + expect(initText).toContain("reactome_cypher_query"); + } else { + expect(toolsText).not.toContain("reactome_cypher"); + expect(resourcesText).not.toContain(SCHEMA_URI); + expect(initText.toLowerCase()).not.toContain("cypher"); + } + // An operator can see which way the gate went without reading the env. + expect(health.graphAccess).toBe(open); + // Serving sessions is not a reason to connect, or to load the driver. + expect(fake.drivers).toHaveLength(0); + expect(fake.moduleLoads).toBe(0); + } finally { + await new Promise(resolve => http.close(resolve)); + } + }); +}); + +describe("graph access gate: open, but the cypher group withheld", () => { + beforeEach(() => { + applyEnv({ NEO4J_URI: URI, MCP_ALLOW_CYPHER: "1", MCP_TOOL_GROUPS: "search,pathway" }); + resetFake(); + vi.resetModules(); + mockDriverModule(); + }); + + afterEach(() => { + restoreEnv(); + vi.resetModules(); + }); + + it("offers nothing graph-shaped and opens no connection for nobody", async () => { + const { createServer } = await import("../src/server.js"); + const server = createServer() as unknown as { + _registeredTools: Record; + _registeredResources: Record; + }; + expect(cypherNames(Object.keys(server._registeredTools))).toEqual([]); + expect(Object.keys(server._registeredResources)).not.toContain(SCHEMA_URI); + + // Without the tools or the resource nothing can read a warmed cache, so + // the warm-up must not connect. + await import("../src/index.js"); + await settle(); + expect(fake.drivers).toHaveLength(0); + }); +}); + +describe("graph access gate: structure", () => { + it("imports neo4j-driver from exactly one file, the gated factory", () => { + // Every connection has to go through getDriver(), which checks the gate + // itself. A second import would be a second door. + const importers: string[] = []; + const root = new URL("../src", import.meta.url).pathname; + const walk = (dir: string) => { + for (const entry of readdirSync(dir)) { + const full = join(dir, entry); + if (statSync(full).isDirectory()) walk(full); + else if (entry.endsWith(".ts")) { + const text = readFileSync(full, "utf8"); + if (/(from\s+|import\s*\(\s*)["']neo4j-driver["']/.test(text)) { + importers.push(relative(root, full)); + } + } + } + }; + walk(root); + expect(importers).toEqual(["clients/neo4j.ts"]); + }); + + it("keeps neo4j-driver as a declared dependency", () => { + const pkg = JSON.parse(readFileSync(new URL("../package.json", import.meta.url), "utf8")); + expect(pkg.dependencies["neo4j-driver"]).toBeDefined(); + }); +}); + +describe("graph access gate: /health reports the gate as it is now", () => { + beforeEach(() => { + applyEnv({}); + resetFake(); + vi.resetModules(); + mockDriverModule(); + }); + + afterEach(() => { + restoreEnv(); + vi.resetModules(); + }); + + it("follows the environment per request, as sessions do", async () => { + // Sessions are built per request and ask the gate each time, so a value + // captured at startup could disagree with what a new session is offered. + const { startHttpServer } = await import("../src/http.js"); + const http = await startHttpServer(0, "127.0.0.1"); + try { + const { port } = http.address() as AddressInfo; + const health = async () => + ((await (await fetch(`http://127.0.0.1:${port}/health`)).json()) as Record) + .graphAccess; + + expect(await health()).toBe(false); + process.env.NEO4J_URI = URI; + process.env.MCP_ALLOW_CYPHER = "1"; + expect(await health()).toBe(true); + delete process.env.MCP_ALLOW_CYPHER; + expect(await health()).toBe(false); + } finally { + await new Promise(resolve => http.close(resolve)); + } + }); +}); diff --git a/tests/graph-schema.test.ts b/tests/graph-schema.test.ts new file mode 100644 index 0000000..b5a8669 --- /dev/null +++ b/tests/graph-schema.test.ts @@ -0,0 +1,430 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import type { FakeNeo4jState } from "./helpers/fake-neo4j.js"; +import type { GraphSchema } from "../src/graph/schema.js"; + +/** + * Schema extraction against the database the tools are actually pointed at: + * the Reactome graph database container, which runs Neo4j 4.4 Community with + * **no APOC**. The previous version read `apoc.meta.*` and would have failed + * there on its first query. These script a fake driver that answers like that + * database -- APOC procedures do not exist -- and assert the schema is built + * from built-in procedures, with each optional section degrading on its own. + */ + +const fake = vi.hoisted((): FakeNeo4jState => ({ drivers: [], sessions: [], respond: () => [] })); + +vi.mock("neo4j-driver", async () => { + const { fakeNeo4jModule } = await import("./helpers/fake-neo4j.js"); + return fakeNeo4jModule(fake); +}); + +import { queriesRun } from "./helpers/fake-neo4j.js"; +import { + fetchGraphSchema, + _resetGraphSchemaCache, + SCHEMA_RETRY_AFTER_MS, +} from "../src/graph/schema.js"; +import { formatGraphSchemaMarkdown } from "../src/graph/format-schema.js"; +import { closeDriver } from "../src/clients/neo4j.js"; + +const NO_PROCEDURE = (name: string) => + new Error( + `There is no procedure with the name \`${name}\` registered for this database instance.` + ); + +/** + * Answers the way Neo4j 4.4 Community without APOC does. `overrides` replaces + * the answer for any query containing its key. + */ +function community44( + overrides: Record) => unknown[]> = {} +) { + return (query: string, params: Record): unknown[] => { + for (const [needle, answer] of Object.entries(overrides)) { + if (query.includes(needle)) return answer(params); + } + const apoc = /\b(apoc\.[\w.]+)/.exec(query); + if (apoc?.[1]) throw NO_PROCEDURE(apoc[1]); + + if (query.includes("dbms.components")) + return [{ name: "Neo4j Kernel", versions: ["4.4.40"], edition: "community" }]; + if (query.includes("db.labels")) return [{ label: "Pathway" }, { label: "Reaction" }]; + if (query.includes("db.relationshipTypes")) + return [{ relationshipType: "hasEvent" }, { relationshipType: "input" }]; + if (query.includes("db.schema.nodeTypeProperties")) + return [ + { + nodeType: ":`Pathway`", + nodeLabels: ["Pathway"], + propertyName: "stId", + propertyTypes: ["String"], + mandatory: true, + }, + ]; + if (query.includes("db.schema.relTypeProperties")) + return [ + { + relType: ":`input`", + propertyName: "stoichiometry", + propertyTypes: ["Long"], + mandatory: true, + }, + ]; + if (query.includes("db.schema.visualization")) + return [ + { + nodes: [ + { identity: -1, labels: ["Pathway"], properties: { name: "Pathway" } }, + { identity: -2, labels: ["Reaction"], properties: { name: "Reaction" } }, + ], + relationships: [{ identity: -3, start: -1, end: -2, type: "hasEvent", properties: {} }], + }, + ]; + if (query.includes("SHOW INDEXES")) + return [ + { + name: "index_pathway_stId", + type: "BTREE", + entityType: "NODE", + labelsOrTypes: ["Pathway"], + properties: ["stId"], + state: "ONLINE", + }, + ]; + if (query.includes("SHOW CONSTRAINTS")) + return [ + { + name: "constraint_dbId", + type: "UNIQUENESS", + entityType: "NODE", + labelsOrTypes: ["DatabaseObject"], + properties: ["dbId"], + }, + ]; + // Per-token counts: one UNION ALL query each for labels and for + // relationship types, keyed by parameter so the names need no escaping. + if (query.includes("UNION ALL") || /RETURN \$\w+ AS key/.test(query)) { + return Object.entries(params).map(([, key]) => ({ + key, + count: key === "Pathway" ? 2_000 : key === "hasEvent" ? 9_000 : 500, + })); + } + if (/MATCH \(n\) RETURN count\(n\)/.test(query)) return [{ count: 3_000_000 }]; + if (/MATCH \(\)-\[r\]->\(\) RETURN count\(r\)/.test(query)) return [{ count: 12_000_000 }]; + throw new Error(`unscripted query: ${query}`); + }; +} + +describe("fetchGraphSchema on Neo4j 4.4 Community without APOC", () => { + beforeEach(() => { + process.env.NEO4J_URI = "bolt://127.0.0.1:7687"; + process.env.MCP_ALLOW_CYPHER = "1"; + fake.drivers.length = 0; + fake.sessions.length = 0; + fake.respond = community44(); + _resetGraphSchemaCache(); + }); + + afterEach(async () => { + _resetGraphSchemaCache(); + await closeDriver(); + delete process.env.NEO4J_URI; + delete process.env.MCP_ALLOW_CYPHER; + }); + + it("builds every section from built-in procedures and never calls APOC", async () => { + const schema = await fetchGraphSchema(); + + expect(queriesRun(fake).filter(q => /apoc\./i.test(q))).toEqual([]); + expect(schema.unavailable).toEqual([]); + expect(schema.dbComponents[0]?.edition).toBe("community"); + expect(schema.stats.nodeCount).toBe(3_000_000); + expect(schema.stats.relCount).toBe(12_000_000); + expect(schema.stats.labels).toEqual({ Pathway: 2_000, Reaction: 500 }); + expect(schema.stats.relTypesCount).toEqual({ hasEvent: 9_000, input: 500 }); + expect(schema.patterns).toEqual([{ start: "Pathway", type: "hasEvent", end: "Reaction" }]); + expect(schema.nodeTypeProperties[0]?.propertyName).toBe("stId"); + // The built-in reports ":`input`"; the schema carries the bare name. + expect(schema.relTypeProperties[0]?.relType).toBe("input"); + expect(schema.indexes[0]?.name).toBe("index_pathway_stId"); + expect(schema.constraints[0]?.name).toBe("constraint_dbId"); + }); + + it("reads only, and under a timeout", async () => { + await fetchGraphSchema(); + expect(fake.sessions.length).toBeGreaterThan(0); + for (const session of fake.sessions) { + expect(session.config.defaultAccessMode).toBe("READ"); + for (const run of session.runs) expect(run.config?.timeout).toBeGreaterThan(0); + } + }); + + it("falls back to db.indexes() and db.constraints() where SHOW is not understood", async () => { + const unsupported = () => { + throw new Error("Invalid input 'SHOW'"); + }; + fake.respond = community44({ + "SHOW INDEXES": unsupported, + "SHOW CONSTRAINTS": unsupported, + "db.indexes": () => [ + { + name: "legacy_index", + type: "BTREE", + entityType: "NODE", + labelsOrTypes: ["Pathway"], + properties: ["stId"], + state: "ONLINE", + }, + ], + "db.constraints": () => [ + { name: "legacy_constraint", description: "CONSTRAINT ON (d:DatabaseObject) ..." }, + ], + }); + + const schema = await fetchGraphSchema(); + expect(schema.indexes.map(i => i.name)).toEqual(["legacy_index"]); + expect(schema.constraints.map(c => c.name)).toEqual(["legacy_constraint"]); + expect(schema.unavailable).toEqual([]); + }); + + it("reports a section that fails everywhere, and still returns the rest", async () => { + fake.respond = community44({ + "db.schema.nodeTypeProperties": () => { + throw new Error("The transaction has been terminated (timeout)"); + }, + "db.indexes": () => { + throw NO_PROCEDURE("db.indexes"); + }, + "SHOW INDEXES": () => { + throw new Error("Invalid input 'SHOW'"); + }, + }); + + const schema = await fetchGraphSchema(); + expect(schema.nodeTypeProperties).toEqual([]); + expect(schema.indexes).toEqual([]); + expect(schema.unavailable.map(u => u.section).sort()).toEqual( + ["indexes", "nodeTypeProperties"].sort() + ); + // The rest is intact. + expect(schema.stats.labels.Pathway).toBe(2_000); + + // And the digest says so, rather than rendering an empty section as if + // the graph had no properties. + const md = formatGraphSchemaMarkdown(schema); + expect(md).toContain("Unavailable"); + expect(md).toContain("nodeTypeProperties"); + }); + + it("retries a failed section on a later fetch rather than caching the failure for good", async () => { + // A section that timed out during the startup warm-up used to stay + // "unavailable" for the life of the process. + vi.useFakeTimers({ toFake: ["Date"] }); + try { + let failing = true; + fake.respond = community44({ + "db.schema.nodeTypeProperties": () => { + if (failing) throw new Error("The transaction has been terminated (timeout)"); + return [ + { + nodeType: ":`Pathway`", + nodeLabels: ["Pathway"], + propertyName: "stId", + propertyTypes: ["String"], + mandatory: true, + }, + ]; + }, + }); + + const first = await fetchGraphSchema(); + expect(first.unavailable.map(u => u.section)).toEqual(["nodeTypeProperties"]); + + // Straight away: served from cache, not retried on every call. + const before = queriesRun(fake).length; + await fetchGraphSchema(); + expect(queriesRun(fake).length).toBe(before); + + // Later: only the failed section is read again, and it now succeeds. + failing = false; + vi.setSystemTime(Date.now() + SCHEMA_RETRY_AFTER_MS + 1); + const later = await fetchGraphSchema(); + expect(later.unavailable).toEqual([]); + expect(later.nodeTypeProperties).toHaveLength(1); + expect(later.stats.labels.Pathway).toBe(2_000); + const rerun = queriesRun(fake).slice(before); + expect(rerun.length).toBeGreaterThan(0); + expect(rerun.every(q => q.includes("db.schema.nodeTypeProperties"))).toBe(true); + + // Complete now, so cached for good. + vi.setSystemTime(Date.now() + SCHEMA_RETRY_AFTER_MS * 10); + const settledCount = queriesRun(fake).length; + expect(await fetchGraphSchema()).toBe(later); + expect(queriesRun(fake).length).toBe(settledCount); + } finally { + vi.useRealTimers(); + } + }); + + it("refuses rather than caching an empty schema when nothing answers", async () => { + fake.respond = () => { + throw new Error("Could not perform discovery. No routing servers available."); + }; + await expect(fetchGraphSchema()).rejects.toThrow(/schema/i); + + // Not cached: the next call tries again. + fake.respond = community44(); + const schema = await fetchGraphSchema(); + expect(schema.stats.labels.Pathway).toBe(2_000); + }); + + it("caches; a second call runs no queries", async () => { + const first = await fetchGraphSchema(); + const before = queriesRun(fake).length; + const second = await fetchGraphSchema(); + expect(second).toBe(first); + expect(queriesRun(fake).length).toBe(before); + }); + + it("shares one fetch between concurrent first callers", async () => { + const [a, b, c] = await Promise.all([ + fetchGraphSchema(), + fetchGraphSchema(), + fetchGraphSchema(), + ]); + expect(a).toBe(b); + expect(b).toBe(c); + expect(queriesRun(fake).filter(q => q.includes("dbms.components"))).toHaveLength(1); + }); +}); + +function minimalSchema(): GraphSchema { + return { + fetchedAt: "2026-10-14T15:00:00Z", + dbComponents: [{ name: "Neo4j Kernel", versions: ["4.4.40"], edition: "community" }], + stats: { + nodeCount: 100, + relCount: 200, + labels: { Pathway: 50, Reaction: 30, Entity: 20 }, + relTypesCount: { hasEvent: 80, input: 120 }, + }, + patterns: [{ start: "Pathway", type: "hasEvent", end: "Reaction" }], + nodeTypeProperties: [ + { + nodeType: ":`Pathway`", + nodeLabels: ["Pathway"], + propertyName: "stId", + propertyTypes: ["String"], + mandatory: true, + }, + { + nodeType: ":`Pathway`", + nodeLabels: ["Pathway"], + propertyName: "displayName", + propertyTypes: ["String"], + mandatory: false, + }, + ], + relTypeProperties: [ + { relType: "input", propertyName: "stoichiometry", propertyTypes: ["Long"], mandatory: true }, + ], + indexes: [ + { + name: "pathway_stId", + labelsOrTypes: ["Pathway"], + properties: ["stId"], + type: "BTREE", + state: "ONLINE", + }, + ], + constraints: [ + { name: "pathway_stId_unique", labelsOrTypes: ["Pathway"], properties: ["stId"] }, + ], + unavailable: [], + }; +} + +describe("formatGraphSchemaMarkdown", () => { + it("covers totals, labels, rel types, patterns, properties, indexes, constraints", () => { + const md = formatGraphSchemaMarkdown(minimalSchema()); + expect(md).toContain("Neo4j:** 4.4.40"); + expect(md).toContain("100 nodes"); + expect(md).toContain("`Pathway` 50"); + expect(md).toContain("`hasEvent` 80"); + expect(md).toContain("(:Pathway)-[:hasEvent]->(:Reaction)"); + expect(md).toContain("stId String!"); + expect(md).toContain("stoichiometry Long! — on input"); + expect(md).toContain("### Indexes (1)"); + expect(md).toContain("### Constraints (1)"); + expect(md).toContain("reactome://graph/schema"); + expect(md).not.toContain("Unavailable"); + // Nothing in the digest should send a reader looking for APOC. + expect(md.toLowerCase()).not.toContain("apoc"); + }); + + it("stays small enough for an LLM context window", () => { + expect(formatGraphSchemaMarkdown(minimalSchema()).length).toBeLessThan(10_000); + }); + + it("summarises a type's endpoints as label sets, most specific first", () => { + // Reactome nodes carry their class hierarchy as labels, so the real + // visualization lists hundreds of pairs per type. Listing them pair by + // pair took the digest past the response cap. + const md = formatGraphSchemaMarkdown({ + ...minimalSchema(), + patterns: [ + { start: "Pathway", type: "hasEvent", end: "Reaction" }, + { start: "Pathway", type: "hasEvent", end: "Entity" }, + { start: "Reaction", type: "hasEvent", end: "Reaction" }, + ], + }); + expect(md).toContain("(:Reaction|Pathway)-[:hasEvent]->(:Entity|Reaction)"); + }); + + it("lists properties every label set shares once", () => { + const md = formatGraphSchemaMarkdown({ + ...minimalSchema(), + nodeTypeProperties: [ + { + nodeType: ":`A`", + nodeLabels: ["A"], + propertyName: "dbId", + propertyTypes: ["Long"], + mandatory: true, + }, + { + nodeType: ":`A`", + nodeLabels: ["A"], + propertyName: "a", + propertyTypes: ["String"], + mandatory: false, + }, + { + nodeType: ":`B`", + nodeLabels: ["B"], + propertyName: "dbId", + propertyTypes: ["Long"], + mandatory: true, + }, + ], + }); + expect(md).toContain("**On every label set:** dbId Long!"); + expect(md).toContain("- **A**: a String"); + expect(md).toContain("- **B**\n"); + }); + + it("omits empty index and constraint sections", () => { + const md = formatGraphSchemaMarkdown({ ...minimalSchema(), indexes: [], constraints: [] }); + expect(md).not.toContain("### Indexes"); + expect(md).not.toContain("### Constraints"); + }); + + it("orders labels by count, descending", () => { + const md = formatGraphSchemaMarkdown(minimalSchema()); + const p = md.indexOf("`Pathway` 50"); + const r = md.indexOf("`Reaction` 30"); + const e = md.indexOf("`Entity` 20"); + expect(p).toBeLessThan(r); + expect(r).toBeLessThan(e); + }); +}); diff --git a/tests/helpers/fake-neo4j.ts b/tests/helpers/fake-neo4j.ts new file mode 100644 index 0000000..6210517 --- /dev/null +++ b/tests/helpers/fake-neo4j.ts @@ -0,0 +1,143 @@ +/** + * A stand-in for `neo4j-driver`, so no test ever opens a socket to a database. + * + * Use it with `vi.mock("neo4j-driver", () => fakeNeo4jModule(state))`, where + * `state` comes from `vi.hoisted(newFakeNeo4jState)` so the mock factory and + * the test share it across `vi.resetModules()`. + * + * It records every driver constructed, every session opened (with its config) + * and every query run (with its transaction config), and answers queries + * through `state.respond`, which a test replaces to script the database. + */ + +export interface FakeRun { + query: string; + params: Record; + config: Record | undefined; + /** Records handed to the caller so far. */ + pulled: number; + /** The caller stopped iterating early, which on a real driver discards the rest. */ + cancelled: boolean; +} + +export interface FakeSession { + config: Record; + runs: FakeRun[]; + closed: boolean; +} + +/** Rows may be an array, or any iterable -- a generator stands in for a huge result. */ +export type FakeRows = Iterable; + +export interface FakeNeo4jState { + drivers: Array<{ uri: string; config: Record }>; + sessions: FakeSession[]; + /** Rows for a query; throw to simulate a database error. */ + respond: (query: string, params: Record) => FakeRows | Promise; + /** How many times the module itself was loaded (set by fakeNeo4jModule). */ + moduleLoads?: number; +} + +export function newFakeNeo4jState(): FakeNeo4jState { + return { drivers: [], sessions: [], respond: () => [], moduleLoads: 0 }; +} + +export function resetFakeNeo4j(state: FakeNeo4jState): void { + state.drivers.length = 0; + state.sessions.length = 0; + state.respond = () => []; + state.moduleLoads = 0; +} + +/** All query texts run so far, across every session, in order. */ +export function queriesRun(state: FakeNeo4jState): string[] { + return state.sessions.flatMap(s => s.runs.map(r => r.query)); +} + +const toRecord = (row: unknown) => ({ toObject: () => row }); + +/** + * Like the driver's Result: awaitable for every record at once, or + * async-iterable one record at a time, where `return()` -- what `break` in a + * `for await` calls -- stops the stream. + */ +function fakeResult(state: FakeNeo4jState, run: FakeRun) { + let rows: Promise | undefined; + const produce = () => + (rows ??= Promise.resolve().then(() => state.respond(run.query, run.params))); + + return { + then( + onFulfilled?: (value: { records: Array<{ toObject: () => unknown }> }) => A, + onRejected?: (reason: unknown) => B + ) { + return produce() + .then(all => { + const records: Array<{ toObject: () => unknown }> = []; + for (const row of all) { + run.pulled++; + records.push(toRecord(row)); + } + return { records }; + }) + .then(onFulfilled, onRejected); + }, + [Symbol.asyncIterator]() { + let it: Iterator | undefined; + return { + async next() { + it ??= (await produce())[Symbol.iterator](); + const step = it.next(); + if (step.done) return { done: true as const, value: undefined }; + run.pulled++; + return { done: false as const, value: toRecord(step.value) }; + }, + async return() { + run.cancelled = true; + it?.return?.(); + return { done: true as const, value: undefined }; + }, + }; + }, + }; +} + +export function fakeNeo4jModule(state: FakeNeo4jState) { + state.moduleLoads = (state.moduleLoads ?? 0) + 1; + const neo4j = { + driver(uri: string, _auth: unknown, config: Record = {}) { + state.drivers.push({ uri, config }); + return { + session(sessionConfig: Record = {}) { + const session: FakeSession = { config: sessionConfig, runs: [], closed: false }; + state.sessions.push(session); + return { + run( + query: string, + params: Record = {}, + txConfig?: Record + ) { + const run: FakeRun = { + query, + params, + config: txConfig, + pulled: 0, + cancelled: false, + }; + session.runs.push(run); + return fakeResult(state, run); + }, + async close() { + session.closed = true; + }, + }; + }, + async close() {}, + }; + }, + auth: { basic: (user: string, _password: string) => ({ scheme: "basic", principal: user }) }, + session: { READ: "READ", WRITE: "WRITE" }, + int: (value: number | string) => ({ __int: Number(value) }), + }; + return { default: neo4j, ...neo4j }; +} diff --git a/tests/input-bounds.test.ts b/tests/input-bounds.test.ts index 4eeb716..1440015 100644 --- a/tests/input-bounds.test.ts +++ b/tests/input-bounds.test.ts @@ -94,13 +94,13 @@ const ABSURD = Array.from({ length: 100_001 }, () => "R-HSA-109582"); const ONE = ["R-HSA-109582"]; describe("tool input bounds", () => { - it("covers every tool, and the set does not change with the environment", async () => { + it("covers every tool in every configuration, including Cypher", async () => { const schemas = await allSchemas(); - // Without this the sweep would pass vacuously against an empty map. The - // exact count is asserted rather than a floor, so a tool that appears - // only under some environment shows up here as a number that moved. - expect(schemas.size).toBe(59); - expect([...schemas.keys()].filter(n => n.includes("cypher"))).toEqual([]); + // Without this the sweep would pass vacuously against an empty map, and + // without the Cypher names it would pass while missing the three tools + // that only register behind the graph access gate. + expect(schemas.size).toBe(62); + expect([...schemas.keys()].filter(n => n.startsWith("reactome_cypher"))).toHaveLength(3); }); it("has no argument anywhere that accepts an unbounded list", async () => { diff --git a/tests/no-graph-access.test.ts b/tests/no-graph-access.test.ts deleted file mode 100644 index 08f53c9..0000000 --- a/tests/no-graph-access.test.ts +++ /dev/null @@ -1,100 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { readFileSync, readdirSync, statSync } from "node:fs"; -import { join } from "node:path"; -import { registerAllTools } from "../src/tools/index.js"; -import { registerStaticResources } from "../src/resources/static.js"; -import { buildServerInstructions } from "../src/instructions.js"; - -/** - * This server does not reach Neo4j, and cannot be configured to. - * - * It is hosted publicly. Constitution Principle IV always said no deployment - * holds a graph connection, and until 2026-09-21 that was enforced by a gate - * — which meant the property was true of a *configuration* rather than of the - * code. Gates get flipped, and this one was tested in four places and got - * three of them: `src/http-server.ts`, the entrypoint that actually runs in - * production, still opened a connection on `NEO4J_URI` alone. - * - * So the tools are gone instead. These tests set the environment that used to - * switch graph access ON, because asserting absence with the switch OFF would - * only confirm the old gate still works. - */ - -const GRAPH_ENV = { NEO4J_URI: "bolt://localhost:7690", MCP_ALLOW_CYPHER: "1" }; - -function withGraphEnv(body: () => T): T { - const previous: Record = {}; - for (const [k, v] of Object.entries(GRAPH_ENV)) { - previous[k] = process.env[k]; - process.env[k] = v; - } - try { - return body(); - } finally { - for (const [k, v] of Object.entries(previous)) { - if (v === undefined) delete process.env[k]; - else process.env[k] = v; - } - } -} - -function collect(register: (server: never) => void, key: "tool" | "resource"): string[] { - const names: string[] = []; - const server = { - [key]: (...args: unknown[]) => { - if (typeof args[0] === "string") names.push(args[0]); - return undefined; - }, - }; - register(server as never); - return names; -} - -describe("no graph access", () => { - it("registers no Cypher tool even with the old switches on", () => { - const names = withGraphEnv(() => collect(registerAllTools, "tool")); - expect(names.filter(n => n.includes("cypher"))).toEqual([]); - // The sweep is only meaningful if the other tools did register. - expect(names.length).toBeGreaterThan(50); - }); - - it("publishes no graph schema resource", () => { - const names = withGraphEnv(() => collect(registerStaticResources, "resource")); - expect(names).not.toContain("reactome://graph/schema"); - expect(names).toContain("reactome://species"); - }); - - it("does not tell clients a graph is available", () => { - // The instructions were the surface that survived the gate: a server - // advertising tools it had not registered is invisible to any check that - // asks the server what it can do, because it says it can. - const text = withGraphEnv(buildServerInstructions); - expect(text.toLowerCase()).not.toContain("cypher"); - expect(text.toLowerCase()).not.toContain("neo4j"); - expect(text).toContain("reactome_search"); - }); - - it("has no neo4j driver dependency", () => { - const pkg = JSON.parse(readFileSync(new URL("../package.json", import.meta.url), "utf8")); - const deps = { ...pkg.dependencies, ...pkg.devDependencies }; - expect(Object.keys(deps).filter(d => d.includes("neo4j"))).toEqual([]); - }); - - it("has no source file that imports a graph driver", () => { - // The tools could come back one import at a time. This is the check that - // notices before a tool exists to catch. - const offenders: string[] = []; - const walk = (dir: string) => { - for (const entry of readdirSync(dir)) { - const full = join(dir, entry); - if (statSync(full).isDirectory()) walk(full); - else if (entry.endsWith(".ts")) { - const text = readFileSync(full, "utf8"); - if (/from\s+["']neo4j-driver["']/.test(text)) offenders.push(full); - } - } - }; - walk(new URL("../src", import.meta.url).pathname); - expect(offenders).toEqual([]); - }); -}); diff --git a/tests/readme-tools.test.ts b/tests/readme-tools.test.ts index 94f4cfd..0bc10fd 100644 --- a/tests/readme-tools.test.ts +++ b/tests/readme-tools.test.ts @@ -33,6 +33,21 @@ describe("README tool coverage", () => { expect(missing).toEqual([]); }); + it("documents the graph tools, which only register behind the gate", () => { + process.env.NEO4J_URI = "bolt://127.0.0.1:7687"; + process.env.MCP_ALLOW_CYPHER = "1"; + try { + const names = registered(); + expect(names.filter(n => n.startsWith("reactome_cypher"))).toHaveLength(3); + expect(names.filter(name => !readme.includes(name))).toEqual([]); + expect(readme).toContain(`${names.length} with graph access`); + } finally { + delete process.env.NEO4J_URI; + delete process.env.MCP_ALLOW_CYPHER; + } + expect(readme).toContain("MCP_ALLOW_CYPHER"); + }); + it("states the tool count, and states it correctly", () => { const names = registered(); expect(names.length).toBeGreaterThan(0); diff --git a/tests/server.test.ts b/tests/server.test.ts index 7b078eb..6b27d16 100644 --- a/tests/server.test.ts +++ b/tests/server.test.ts @@ -43,20 +43,21 @@ describe("createServer", () => { expect(toolNames).toContain("reactome_analyze_identifiers"); }); - it("registers no graph tools, whatever the environment says", () => { - // Principle IV, now structural rather than configured: there are no graph - // tools to gate. The environment is set to what used to switch them ON, - // because a test that unsets it would be asserting the old gate still - // works rather than that the tools are gone. - process.env.NEO4J_URI = "bolt://localhost:7690"; - process.env.MCP_ALLOW_CYPHER = "1"; + it("offers graph tools only behind both switches", () => { + // The full matrix, every entrypoint, lives in graph-access-gate.test.ts. + // This is the factory's own check, in both directions: absent on a + // connection string alone, present once the opt-in is added too. + const cypherTools = () => { + const server = createServer() as unknown as { _registeredTools?: Record }; + return Object.keys(server._registeredTools ?? {}).filter(n => + n.startsWith("reactome_cypher") + ); + }; + process.env.NEO4J_URI = "bolt://127.0.0.1:7687"; try { - const server = createServer(); - const registered = server as unknown as { _registeredTools?: Record }; - const toolNames = Object.keys(registered._registeredTools ?? {}); - - expect(toolNames.filter(n => n.includes("cypher"))).toEqual([]); - expect(toolNames.length).toBeGreaterThan(40); + expect(cypherTools()).toEqual([]); + process.env.MCP_ALLOW_CYPHER = "1"; + expect(cypherTools()).toHaveLength(3); } finally { delete process.env.NEO4J_URI; delete process.env.MCP_ALLOW_CYPHER; diff --git a/tests/tool-groups.test.ts b/tests/tool-groups.test.ts index a39a669..40bcf6a 100644 --- a/tests/tool-groups.test.ts +++ b/tests/tool-groups.test.ts @@ -31,6 +31,17 @@ function namesFor(register: (server: never) => void): string[] { return names; } +function withGraphAccess(body: () => T): T { + process.env.NEO4J_URI = "bolt://127.0.0.1:7687"; + process.env.MCP_ALLOW_CYPHER = "1"; + try { + return body(); + } finally { + delete process.env.NEO4J_URI; + delete process.env.MCP_ALLOW_CYPHER; + } +} + function registeredWith(groups: string | undefined): string[] { const previous = process.env.MCP_TOOL_GROUPS; if (groups === undefined) delete process.env.MCP_TOOL_GROUPS; @@ -82,7 +93,10 @@ describe("MCP_TOOL_GROUPS parsing", () => { }); it("registers everything only when genuinely unset", () => { - expect(resolveToolGroups(undefined)).toEqual(ALL_TOOL_GROUPS); + // Everything the instance is allowed: `cypher` additionally needs the + // graph access gate open, and it is closed here. + expect(resolveToolGroups(undefined)).toEqual(ALL_TOOL_GROUPS.filter(g => g !== "cypher")); + withGraphAccess(() => expect(resolveToolGroups(undefined)).toEqual(ALL_TOOL_GROUPS)); }); it("refuses an empty value rather than falling back to everything", () => { @@ -154,6 +168,16 @@ describe("instructions match what is registered", () => { expect(dangling).toEqual([]); }); + it("names no tool or resource the full server does not register, with graph access on", () => { + withGraphAccess(() => { + const registered = registeredNames(ALL_TOOL_GROUPS); + const text = buildServerInstructions(ALL_TOOL_GROUPS); + expect(text).toContain("reactome_cypher_query"); + const dangling = referenced(text).filter(r => !satisfied(r, registered)); + expect(dangling).toEqual([]); + }); + }); + it("names no tool or resource a restricted server does not register", () => { const groups: ToolGroup[] = ["search", "pathway", "entity", "utilities"]; const registered = registeredNames(groups); @@ -182,7 +206,13 @@ describe("resources follow the same switch", () => { }; it("registers every known resource when all groups are on", () => { - expect([...resourceUris(ALL_TOOL_GROUPS)].sort()).toEqual([...RESOURCE_URIS].sort()); + withGraphAccess(() => + expect([...resourceUris(ALL_TOOL_GROUPS)].sort()).toEqual([...RESOURCE_URIS].sort()) + ); + // With the gate closed, everything but the graph schema. + expect([...resourceUris(ALL_TOOL_GROUPS)].sort()).toEqual( + RESOURCE_URIS.filter(u => u !== "reactome://graph/schema").sort() + ); }); it("withholds the resources of an omitted group", () => {