diff --git a/.specify/memory/constitution.md b/.specify/memory/constitution.md index 1567830..26457bc 100644 --- a/.specify/memory/constitution.md +++ b/.specify/memory/constitution.md @@ -63,13 +63,24 @@ shipped. ### IV. Reactome data, through the public services Tools read the Content Service and the Analysis Service over HTTP. The server -does not hold a Neo4j connection in any deployment the team runs: graph tools stay -behind the `NEO4J_URI` gate and stay off by default. +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. 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. + ### V. Analysis runs in the service, not in the agent Over-representation analysis, species comparison and pathway filtering are diff --git a/CHANGELOG.md b/CHANGELOG.md index 2741731..c7ca999 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ All notable changes to this project are documented here. This project adheres to ## [Unreleased] +### 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. + ### 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`. - **Two tools dropped data silently, with no `undefined` to give it away.** `participants` never rendered external identifiers at all — the endpoint returns `refEntities` (an array), never a singular `referenceEntity`, so UniProt accessions were simply absent. `search_facets` returned nothing but its heading: each facet is an object with an `available` list, so `.length` on it was `undefined` and every section was skipped as falsy. diff --git a/README.md b/README.md index 8d6d7b7..07ada3e 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 -Over 40 tools and 10 resources are registered — see [Tools](#tools) and [Resources](#resources) below for the full list. Curators can additionally opt in to direct **Cypher / Neo4j** access against a local Reactome graph database (see [Graph Database / Cypher](#graph-database--cypher-3-tools-opt-in)). +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. ## Prerequisites @@ -36,11 +36,6 @@ All configuration is via environment variables — pass them in the `env` block | `REACTOME_BASE_URL` | `https://reactome.org` | Base URL for the Content + Analysis Services. Override to point at staging / a specific release host. | | `REACTOME_CONTENT_SERVICE_URL` | derived from `REACTOME_BASE_URL` | Fine-grained override for the Content Service only. | | `REACTOME_ANALYSIS_SERVICE_URL` | derived from `REACTOME_BASE_URL` | Fine-grained override for the Analysis Service only. | -| `NEO4J_URI` | _(unset)_ | Set to enable the optional Cypher tools (see below). | -| `NEO4J_USER` | `neo4j` | | -| `NEO4J_PASSWORD` | `neo4j` | Works against auth-disabled local images (`reactome_neo4j_env`). Set explicitly for any remote database. | -| `NEO4J_DATABASE` | `graph.db` | Matches the default in `reactome_neo4j_env`. | -| `CYPHER_QUERY_TIMEOUT_MS` | `30000` | Server-side transaction timeout (ms) for `reactome_cypher_*` tools. Runaway queries are terminated after this. | | `LOG_LEVEL` | `info` | `debug` / `info` / `warn` / `error`. Logs are JSON on stderr; stdout is reserved for the MCP protocol. | ## Usage @@ -66,8 +61,6 @@ Add the server to your Claude Desktop configuration (`claude_desktop_config.json claude mcp add reactome node /absolute/path/to/reactome-mcp/dist/index.js ``` -Add `--env NEO4J_URI=bolt://localhost:7687` (and friends) to enable the Cypher tools. See [Configuration](#configuration) for the full list. - ### Example prompts Once the server is registered, try asking Claude: @@ -81,12 +74,6 @@ Once the server is registered, try asking Claude: - "Find pathways in the HHV8 infection area and show me the contained reactions of the best match." - "Export the SBGN for pathway R-HSA-1640170." -**Graph queries (requires `NEO4J_URI`):** - -- "Show me the Reactome graph schema, then find all `Pathway` nodes that are disease pathways *and* have a human species annotation." -- "Using the graph DB: for reaction R-HSA-199420, list every input and output entity with its display name and stable ID." -- "Count how many `ReactionLikeEvent` nodes exist per species." - Claude reads per-server instructions on connection explaining the tool categories, ID conventions, and a recommended workflow, so it can usually chain the right calls without hand-holding. If an answer looks off, ask it to "show me the tool call and its result" and correct from there. ### Standalone @@ -129,7 +116,7 @@ Starts a local web UI with an MCP bridge for browser-based exploration. | `reactome_analysis_pathway_sizes` | Get pathway size distribution from an analysis result | | `reactome_filter_analysis_pathways` | Filter an analysis result to specific pathways | -### Pathways (7 tools) +### Pathways (8 tools) | Tool | Description | |------|-------------| @@ -140,6 +127,7 @@ Starts a local web UI with an MCP bridge for browser-based exploration. | `reactome_pathways_for_entity` | Find pathways containing a specific entity | | `reactome_diagram_pathways_for_entity` | Find diagram-level pathways containing an entity | | `reactome_events_hierarchy` | Get the complete event hierarchy for a species | +| `reactome_preceding_events` | Find the events that must occur before a reaction or pathway — Reactome's event ordering | ### Search (7 tools) @@ -191,39 +179,15 @@ Starts a local web UI with an MCP bridge for browser-based exploration. | `reactome_interactor_pathways` | Find pathways where a protein's interactors appear | | `reactome_interactor_summary` | Summarise curated interactions for a protein | -### Graph Database / Cypher (3 tools, opt-in) - -Only registered when `NEO4J_URI` is set. Designed for curators running the [`reactome_neo4j_env`](https://github.com/reactome/reactome_neo4j_env) Docker image locally (or pointing at a remote Reactome Neo4j). +### Gene Set Analysis / ReactomeGSA (5 tools) | Tool | Description | |------|-------------| -| `reactome_cypher_query` | Run a Cypher query with optional parameters; row count, per-row size, and total response size are all capped; a server-side timeout terminates runaway queries | -| `reactome_cypher_schema` | Live APOC introspection: labels with node counts, relationship cardinalities, per-label and per-rel property types (with mandatory flags), indexes, constraints. Cached for the session after first call; pre-warmed at MCP startup. | -| `reactome_cypher_sample` | Return a small sample of nodes for a given label | - -**Read-only posture — what it is and isn't.** Sessions run in Neo4j READ mode, which rejects native write clauses (`CREATE`, `MERGE`, `DELETE`, `SET`, `REMOVE`). On top of that, `reactome_cypher_query` rejects APOC procedures that can write or reach outside the graph through back-channels (`apoc.cypher.runWrite` / `apoc.cypher.doIt`, `apoc.periodic.*`, `apoc.create/merge/refactor.*`, `apoc.load/import/export.*`, `apoc.trigger.*`, `apoc.nodes.delete`). Treat this as a guardrail against accidental mutation, not a security boundary — a real trust boundary should live at the Neo4j RBAC / plugin configuration layer, or by pointing at a read-only replica. - -**Configuration** (add to your Claude MCP config `env` block): - -```json -{ - "mcpServers": { - "reactome": { - "command": "node", - "args": ["/absolute/path/to/reactome-mcp/dist/index.js"], - "env": { - "NEO4J_URI": "bolt://localhost:7687", - "NEO4J_USER": "neo4j", - "NEO4J_PASSWORD": "neo4j", - "NEO4J_DATABASE": "graph.db", - "CYPHER_QUERY_TIMEOUT_MS": "30000" - } - } - } -} -``` - -`NEO4J_USER` / `NEO4J_PASSWORD` default to `neo4j` / `neo4j` (which works when the server has auth disabled, as in `reactome_neo4j_env`). `NEO4J_DATABASE` defaults to `graph.db`. `CYPHER_QUERY_TIMEOUT_MS` defaults to 30000 ms. +| `reactome_gsa_methods` | List the gene set analysis methods ReactomeGSA offers (PADOG, Camera, ssGSEA, terapadog) | +| `reactome_gsa_data_types` | List the kinds of experimental data ReactomeGSA can analyse (RNA-seq counts, normalised RNA-seq, proteomics, microarray, Ribo-seq) | +| `reactome_gsa_search_datasets` | Search public expression datasets ReactomeGSA can load — Expression Atlas, Single Cell Expression Atlas, GREIN, GEO | +| `reactome_gsa_examples` | List the bundled example datasets | +| `reactome_gsa_sources` | List the external dataset sources ReactomeGSA can load from | ### Utilities (7 tools) diff --git a/SECURITY.md b/SECURITY.md index d3ec266..abc4e03 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 Neo4j / Cypher path, or the web demo -- The version of `reactome-mcp` (see `package.json`) and any relevant runtime versions (Node, Neo4j server, APOC) +- 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) 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,21 @@ You can expect an acknowledgement within a few business days. We will coordinate In scope: -- The MCP server (`src/`, `dist/`) and its REST / Neo4j clients. +- The MCP server (`src/`, `dist/`) and its REST clients. - The bundled web demo (`web/`). Out of scope (report upstream): - Vulnerabilities in `https://reactome.org` — report via the Reactome website. -- Vulnerabilities in Neo4j, APOC, 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 is designed to be run locally by a trusted curator against either the public Reactome APIs or a local Reactome Neo4j image. It is **not** hardened for multi-tenant or internet-facing deployment without additional controls (auth, TLS, network isolation). -- The `reactome_cypher_query` tool runs in a Neo4j READ-mode session and rejects known APOC write-through procedures, but this is a guardrail, not a security boundary. For untrusted inputs, point at a read-only replica and configure Neo4j RBAC / plugin loading accordingly. +- 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. +- 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 -- `NEO4J_PASSWORD` and any future credential env vars are read at process start and passed to the driver. They are never logged. REST error bodies are logged verbatim on failure — if you deploy against an authenticated proxy, ensure upstream errors do not echo credentials. +- 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. diff --git a/package-lock.json b/package-lock.json index ee6e752..61cdb05 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,11 +11,11 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", "express": "^5.2.1", - "neo4j-driver": "^6.2.0", "zod": "^4.6.5" }, "bin": { - "reactome-mcp": "dist/index.js" + "reactome-mcp": "dist/index.js", + "reactome-mcp-http": "dist/http-server.js" }, "devDependencies": { "@eslint/js": "^10.0.1", @@ -543,9 +543,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -564,9 +561,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -585,9 +579,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -606,9 +597,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -627,9 +615,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -648,9 +633,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -1441,26 +1423,6 @@ "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", @@ -1511,30 +1473,6 @@ "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", @@ -2419,26 +2357,6 @@ "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", @@ -2726,9 +2644,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2751,9 +2666,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2776,9 +2688,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2801,9 +2710,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -3037,37 +2943,6 @@ "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", @@ -3445,35 +3320,6 @@ "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", @@ -3677,15 +3523,6 @@ "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", @@ -3755,12 +3592,6 @@ "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 e574dee..5647e27 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,6 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", "express": "^5.2.1", - "neo4j-driver": "^6.2.0", "zod": "^4.6.5" }, "devDependencies": { diff --git a/specs/002-transport-and-hosting/spec.md b/specs/002-transport-and-hosting/spec.md index b53d1b3..9bcb87d 100644 --- a/specs/002-transport-and-hosting/spec.md +++ b/specs/002-transport-and-hosting/spec.md @@ -22,10 +22,14 @@ 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.** Graph tools stay behind the `NEO4J_URI` -gate, off by default, and a hosted instance does not set it. A public endpoint +**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. A test asserts the gate holds. +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. **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 deleted file mode 100644 index 226bbff..0000000 --- a/src/clients/neo4j.ts +++ /dev/null @@ -1,133 +0,0 @@ -import neo4j, { Driver } from "neo4j-driver"; -import { - NEO4J_URI, - NEO4J_USER, - NEO4J_PASSWORD, - NEO4J_DATABASE, - CYPHER_QUERY_TIMEOUT_MS, - ALLOW_CYPHER_TOOLS, -} from "../config.js"; -import { logger } from "../logger.js"; - -let driverInstance: Driver | null = null; - -export function isNeo4jConfigured(): boolean { - return Boolean(NEO4J_URI); -} - -/** - * Whether anything Cypher-shaped is offered to clients: the three - * `reactome_cypher_*` tools, the `reactome://graph/schema` resource, and the - * Cypher section of the server instructions. - * - * All three are the same decision, so they ask the same question here. They - * used to each test `isNeo4jConfigured()` independently, which is how the - * opt-in added on 2026-09-21 reached the tools and left the other two behind: - * a server that had been told not to offer Cypher still published the graph - * schema and still instructed clients to call tools it had not registered. - */ -export function isCypherEnabled(): boolean { - return isNeo4jConfigured() && ALLOW_CYPHER_TOOLS; -} - -function isLocalhost(uri: string): boolean { - try { - const host = new URL(uri).hostname; - return host === "localhost" || host === "127.0.0.1" || host === "::1"; - } catch { - return false; - } -} - -function warnIfInsecureRemote() { - if (!NEO4J_URI) return; - if (isLocalhost(NEO4J_URI)) return; - const passwordIsDefault = !process.env.NEO4J_PASSWORD; - if (passwordIsDefault) { - logger.warn( - "NEO4J_URI points to a non-localhost host but NEO4J_PASSWORD is unset; using the default 'neo4j' password. Set NEO4J_PASSWORD explicitly for remote databases.", - { uri: NEO4J_URI } - ); - } -} - -export function getDriver(): Driver { - if (!NEO4J_URI) { - throw new Error("Neo4j is not configured. Set NEO4J_URI to enable Cypher tools."); - } - if (!driverInstance) { - warnIfInsecureRemote(); - driverInstance = neo4j.driver(NEO4J_URI, neo4j.auth.basic(NEO4J_USER, NEO4J_PASSWORD), { - disableLosslessIntegers: true, - }); - logger.info("neo4j driver initialized", { - uri: NEO4J_URI, - database: NEO4J_DATABASE, - }); - const shutdown = async () => { - if (driverInstance) { - await driverInstance.close(); - driverInstance = null; - } - }; - // process.once expects a void-returning listener. Handing it an async - // function meant a failed close rejected with nobody listening, which Node - // will turn into a hard exit on unhandled rejection. - const onShutdown = () => { - void shutdown().catch(error => { - 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 driverInstance; -} - -// 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(params: Record): 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; -} - -export async function runRead>( - cypher: string, - params: Record = {}, - options: RunReadOptions = {} -): Promise { - const session = getDriver().session({ - database: NEO4J_DATABASE, - defaultAccessMode: neo4j.session.READ, - }); - const timeout = options.timeoutMs ?? CYPHER_QUERY_TIMEOUT_MS; - try { - // TransactionConfig.timeout is enforced server-side: the transaction - // is terminated if it runs longer than this. Guards against runaway - // queries on a large graph. - const result = await session.run(cypher, coerceIntParams(params), { timeout }); - return result.records.map(r => r.toObject() as T); - } finally { - await session.close(); - } -} - -// Graph-schema access lives in src/graph/schema.ts — split out so tests -// can mock runRead across the module boundary. diff --git a/src/config.ts b/src/config.ts index 3a283a6..9f95d8b 100644 --- a/src/config.ts +++ b/src/config.ts @@ -61,28 +61,20 @@ export const RESOURCE_TYPES = [ export const DIAGRAM_FORMATS = ["png", "jpg", "jpeg", "svg", "gif"] as const; -export const NEO4J_URI = process.env.NEO4J_URI; -export const NEO4J_USER = process.env.NEO4J_USER ?? "neo4j"; -export const NEO4J_PASSWORD = process.env.NEO4J_PASSWORD ?? "neo4j"; -export const NEO4J_DATABASE = process.env.NEO4J_DATABASE ?? "graph.db"; - /** - * Whether the Cypher tools may be registered at all. - * - * Default-deny, and deliberately a separate switch from `NEO4J_URI`. + * There is no Neo4j configuration here, deliberately. * - * Until 2026-09-21 the Cypher tools appeared whenever `NEO4J_URI` was set, - * which made "can the public run arbitrary graph queries" a side effect of a - * connection string rather than a decision. That is the wrong shape for this - * particular capability: a deployment might set `NEO4J_URI` for any number of - * good reasons -- the graph-schema warm-up, a future non-Cypher graph tool -- - * and would silently publish `reactome_cypher_query` by doing so. + * 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. * - * So it is an opt-in. Forgetting it costs a missing tool on an internal - * instance, which is visible and harmless. Forgetting the inverse would have - * cost arbitrary query access on a public one. + * `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. */ -export const ALLOW_CYPHER_TOOLS = process.env.MCP_ALLOW_CYPHER === "1"; function parsePositiveInt(raw: string | undefined, fallback: number): number { if (!raw) return fallback; @@ -91,11 +83,6 @@ function parsePositiveInt(raw: string | undefined, fallback: number): number { return n; } -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 deleted file mode 100644 index 7c10b10..0000000 --- a/src/graph/format-schema.ts +++ /dev/null @@ -1,123 +0,0 @@ -import type { GraphSchema } from "./schema.js"; - -/** - * Render a GraphSchema (as produced by fetchGraphSchema) as a compact - * markdown summary suitable for direct LLM consumption. The raw APOC - * payload is ~500 KB — much too large to return whole. This digest keeps - * the signal (labels with counts, relationship cardinalities, property - * types with mandatory flags, indexes, constraints) and drops the - * verbose apoc.meta.schema() object. Clients that need the full - * structure can read the `reactome://graph/schema` resource. - */ -export function formatGraphSchemaMarkdown(schema: GraphSchema): string { - const { stats, nodeTypeProperties, relTypeProperties, indexes, constraints } = schema; - - const labelEntries = Object.entries(stats.labels ?? {}).sort(([, a], [, b]) => b - a); - const relEntries = Object.entries(stats.relTypesCount ?? {}).sort(([, a], [, b]) => b - a); - - const propsByLabel = new Map< - string, - Array<{ name: string; types: string[]; mandatory: boolean }> - >(); - for (const p of nodeTypeProperties) { - const key = (p.nodeLabels?.join(":") || p.nodeType) ?? p.nodeType; - const entry = propsByLabel.get(key) ?? []; - entry.push({ name: p.propertyName, types: p.propertyTypes ?? [], mandatory: p.mandatory }); - propsByLabel.set(key, entry); - } - - const propsByRel = new Map< - string, - Array<{ name: string; types: string[]; mandatory: boolean }> - >(); - for (const p of relTypeProperties) { - const entry = propsByRel.get(p.relType) ?? []; - entry.push({ name: p.propertyName, types: p.propertyTypes ?? [], mandatory: p.mandatory }); - propsByRel.set(p.relType, entry); - } - - 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 · ${Object.keys(stats.relTypes ?? {}).length} relationship types` - ); - lines.push(""); - - lines.push(`### Labels (${labelEntries.length}, by node count)`); - for (const [label, count] of labelEntries) { - lines.push(`- \`${label}\` — ${count.toLocaleString()}`); - } - lines.push(""); - - lines.push(`### Relationship types (${relEntries.length}, by relationship count)`); - for (const [relType, count] of relEntries) { - lines.push(`- \`${relType}\` — ${count.toLocaleString()}`); - } - lines.push(""); - - lines.push(`### Node properties (by label)`); - const sortedLabels = Array.from(propsByLabel.keys()).sort(); - for (const label of sortedLabels) { - lines.push(`- **${label}**`); - for (const p of propsByLabel.get(label)!) { - const t = p.types.length ? ` _(${p.types.join("|")})_` : ""; - const m = p.mandatory ? " **required**" : ""; - lines.push(` - \`${p.name}\`${t}${m}`); - } - } - lines.push(""); - - if (propsByRel.size > 0) { - lines.push(`### Relationship properties (by type)`); - const sortedRels = Array.from(propsByRel.keys()).sort(); - for (const rel of sortedRels) { - const props = propsByRel.get(rel)!; - if (props.length === 0) continue; - lines.push(`- **${rel}**`); - for (const p of props) { - const t = p.types.length ? ` _(${p.types.join("|")})_` : ""; - const m = p.mandatory ? " **required**" : ""; - lines.push(` - \`${p.name}\`${t}${m}`); - } - } - lines.push(""); - } - - if (indexes.length > 0) { - lines.push(`### Indexes (${indexes.length})`); - for (const ix of indexes) { - const row = ix as { - name?: string; - labelsOrTypes?: string[]; - properties?: string[]; - type?: string; - state?: string; - }; - const labels = row.labelsOrTypes?.join(",") ?? "?"; - const props = row.properties?.join(",") ?? "?"; - lines.push( - `- \`${row.name ?? "?"}\` — ${labels}(${props}) [${row.type ?? "?"}, ${row.state ?? "?"}]` - ); - } - lines.push(""); - } - - if (constraints.length > 0) { - lines.push(`### Constraints (${constraints.length})`); - for (const c of constraints) { - const row = c as { name?: string; description?: string }; - lines.push(`- \`${row.name ?? "?"}\` — ${row.description ?? ""}`); - } - lines.push(""); - } - - lines.push( - "_For programmatic access to the full schema (including the raw `apoc.meta.schema()` output with per-relationship cardinalities and full property type inventories), read the `reactome://graph/schema` resource._" - ); - - return lines.join("\n"); -} diff --git a/src/graph/schema.ts b/src/graph/schema.ts deleted file mode 100644 index 01a2642..0000000 --- a/src/graph/schema.ts +++ /dev/null @@ -1,138 +0,0 @@ -import { runRead } from "../clients/neo4j.js"; -import { logger } from "../logger.js"; - -export interface GraphSchema { - fetchedAt: string; - dbComponents: Array<{ name: string; versions: string[]; edition: string }>; - stats: { - nodeCount: number; - relCount: number; - labels: Record; - relTypes: Record; - relTypesCount: Record; - }; - schema: Record; - nodeTypeProperties: Array<{ - nodeType: string; - nodeLabels: string[]; - propertyName: string; - propertyTypes: string[]; - mandatory: boolean; - }>; - relTypeProperties: Array<{ - relType: string; - sourceNodeLabels: string[]; - targetNodeLabels: string[]; - propertyName: string; - propertyTypes: string[]; - mandatory: boolean; - }>; - indexes: unknown[]; - constraints: unknown[]; -} - -// apoc.meta.schema() can scan many nodes; give the schema queries a longer -// budget than the default Cypher-query timeout. -const SCHEMA_FETCH_TIMEOUT_MS = 60_000; - -let schemaCache: GraphSchema | null = null; -let schemaPending: Promise | null = null; - -/** - * Fetch the live graph schema via APOC (+ fallbacks for indexes and - * constraints). Cached in-memory after the first successful call so - * subsequent tool invocations are free. Concurrent first-callers share - * one round-trip via the `schemaPending` promise. - */ -export async function fetchGraphSchema(): Promise { - if (schemaCache) return schemaCache; - if (schemaPending) return schemaPending; - - const opts = { timeoutMs: SCHEMA_FETCH_TIMEOUT_MS }; - const start = Date.now(); - - schemaPending = (async () => { - try { - type Comp = { name: string; versions: string[]; edition: string }; - type Stats = GraphSchema["stats"]; - type NodeProp = GraphSchema["nodeTypeProperties"][number]; - type RelProp = GraphSchema["relTypeProperties"][number]; - - const [components, stats, schemaRow, nodeProps, relProps, indexes, constraints] = - await Promise.all([ - runRead( - "CALL dbms.components() YIELD name, versions, edition RETURN name, versions, edition", - {}, - opts - ), - runRead( - "CALL apoc.meta.stats() YIELD labels, relTypes, relTypesCount, nodeCount, relCount RETURN labels, relTypes, relTypesCount, nodeCount, relCount", - {}, - opts - ), - runRead<{ value: Record }>( - "CALL apoc.meta.schema() YIELD value RETURN value", - {}, - opts - ), - runRead( - "CALL apoc.meta.nodeTypeProperties() YIELD nodeType, nodeLabels, propertyName, propertyTypes, mandatory RETURN nodeType, nodeLabels, propertyName, propertyTypes, mandatory", - {}, - opts - ), - runRead( - "CALL apoc.meta.relTypeProperties() YIELD relType, sourceNodeLabels, targetNodeLabels, propertyName, propertyTypes, mandatory RETURN relType, sourceNodeLabels, targetNodeLabels, propertyName, propertyTypes, mandatory", - {}, - opts - ).catch(() => [] as RelProp[]), - runRead( - "CALL db.indexes() YIELD name, state, type, entityType, labelsOrTypes, properties RETURN name, state, type, entityType, labelsOrTypes, properties", - {}, - opts - ).catch(() => [] as unknown[]), - runRead( - "CALL db.constraints() YIELD name, description RETURN name, description", - {}, - opts - ).catch(() => [] as unknown[]), - ]); - - const result: GraphSchema = { - fetchedAt: new Date().toISOString(), - dbComponents: components, - stats: stats[0] ?? { - nodeCount: 0, - relCount: 0, - labels: {}, - relTypes: {}, - relTypesCount: {}, - }, - schema: schemaRow[0]?.value ?? {}, - nodeTypeProperties: nodeProps, - relTypeProperties: relProps, - indexes, - constraints, - }; - - logger.info("graph schema fetched", { - durationMs: Date.now() - start, - nodeCount: result.stats.nodeCount, - relCount: result.stats.relCount, - labels: Object.keys(result.stats.labels ?? {}).length, - }); - - schemaCache = result; - 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/http-server.ts b/src/http-server.ts index e1ea0f2..17dd5fd 100644 --- a/src/http-server.ts +++ b/src/http-server.ts @@ -11,8 +11,7 @@ */ import { startHttpServer } from "./http.js"; import { logger } from "./logger.js"; -import { MCP_HTTP_PORT, MCP_HTTP_HOST, NEO4J_URI } from "./config.js"; -import { fetchGraphSchema } from "./graph/schema.js"; +import { MCP_HTTP_PORT, MCP_HTTP_HOST } from "./config.js"; const port = MCP_HTTP_PORT; @@ -23,21 +22,9 @@ if (!port) { process.exit(1); } -startHttpServer(port, MCP_HTTP_HOST) - .then(() => { - // Same warm-up the stdio entrypoint does: apoc.meta.schema() samples 3M - // nodes and takes 15-30s, so the first caller should not pay for it. - if (NEO4J_URI) { - fetchGraphSchema().catch((err: unknown) => { - logger.warn("graph schema prefetch failed; will retry on first use", { - error: err instanceof Error ? err.message : String(err), - }); - }); - } - }) - .catch((error: unknown) => { - logger.error("fatal error starting http server", { - error: error instanceof Error ? error.message : String(error), - }); - process.exit(1); +startHttpServer(port, MCP_HTTP_HOST).catch((error: unknown) => { + logger.error("fatal error starting http server", { + error: error instanceof Error ? error.message : String(error), }); + process.exit(1); +}); diff --git a/src/http.ts b/src/http.ts index 84277ec..b535c74 100644 --- a/src/http.ts +++ b/src/http.ts @@ -7,14 +7,12 @@ import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { createServer, SERVER_NAME, SERVER_VERSION } from "./server.js"; import { logger } from "./logger.js"; -import { isCypherEnabled } from "./clients/neo4j.js"; import { MCP_HTTP_HOST, MCP_MAX_SESSIONS, MCP_SESSION_TTL_MS, CONTENT_SERVICE_URL, ANALYSIS_SERVICE_URL, - NEO4J_URI, MAX_ANALYSIS_IDENTIFIERS, } from "./config.js"; @@ -220,8 +218,6 @@ export function startHttpServer(port: number, host: string = MCP_HTTP_HOST): Pro sessions: sessions.size, contentService: CONTENT_SERVICE_URL, analysisService: ANALYSIS_SERVICE_URL, - neo4jEnabled: Boolean(NEO4J_URI), - cypherEnabled: isCypherEnabled(), }); }); @@ -232,8 +228,6 @@ export function startHttpServer(port: number, host: string = MCP_HTTP_HOST): Pro port, contentService: CONTENT_SERVICE_URL, analysisService: ANALYSIS_SERVICE_URL, - neo4jEnabled: Boolean(NEO4J_URI), - cypherEnabled: isCypherEnabled(), }); resolve(http); }); diff --git a/src/index.ts b/src/index.ts index 8d99165..60cbf43 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,9 +3,7 @@ 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, NEO4J_URI } from "./config.js"; -import { fetchGraphSchema } from "./graph/schema.js"; -import { isCypherEnabled } from "./clients/neo4j.js"; +import { CONTENT_SERVICE_URL, ANALYSIS_SERVICE_URL } from "./config.js"; async function main() { // Built here rather than at module scope, so importing this file does not @@ -16,24 +14,7 @@ async function main() { logger.info("reactome mcp server started", { contentService: CONTENT_SERVICE_URL, analysisService: ANALYSIS_SERVICE_URL, - neo4jEnabled: Boolean(NEO4J_URI), - cypherEnabled: isCypherEnabled(), }); - - // Warm the schema cache in the background so the first - // reactome_cypher_schema call (or reactome://graph/schema read) doesn't - // wait 15–30s on apoc.meta.schema(). Failures are logged; the cache - // stays empty and the tool call will retry on demand. - // Gated on the opt-in, not the connection: without it there is no schema - // tool and no schema resource, so the prefetch would warm a cache nothing - // can read and open a Neo4j connection for nobody. - if (isCypherEnabled()) { - fetchGraphSchema().catch(err => { - logger.warn("graph schema prefetch failed; will retry on first use", { - error: err instanceof Error ? err.message : String(err), - }); - }); - } } main().catch(error => { diff --git a/src/instructions.ts b/src/instructions.ts index 1505d65..5cf3cc0 100644 --- a/src/instructions.ts +++ b/src/instructions.ts @@ -1,5 +1,3 @@ -import { isCypherEnabled } from "./clients/neo4j.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. @@ -33,28 +31,10 @@ This server exposes the Reactome pathway knowledgebase (https://reactome.org) to - \`reactome://pathway/{id}\`, \`reactome://entity/{id}\`, \`reactome://analysis/{token}\` — templated. `.trim(); -const CYPHER_INSTRUCTIONS = ` -# Graph database (Cypher) — enabled - -A local Neo4j Reactome graph is available. Use it when the user wants a query that the REST API does not expose — e.g. arbitrary graph traversals, complex relational joins, aggregate counts across labels. - -**Workflow for Cypher:** - -1. Call \`reactome_cypher_schema\` (or read the \`reactome://graph/schema\` resource) **before writing any query**. The schema tool returns labels with node counts, relationship cardinalities, per-label and per-rel property types (with mandatory flags), indexes, and constraints. Pulled live via APOC on first use and cached in-memory for the session (warm after the MCP's startup prefetch). Never guess the schema. -2. Use \`reactome_cypher_sample\` on a label to see a representative node's shape. -3. Write a Cypher query with \`reactome_cypher_query\`. Rules: - - Sessions run in READ mode; write clauses will be rejected. - - APOC procedures that can write (\`apoc.cypher.runWrite\`, \`apoc.periodic.*\`, \`apoc.create/merge/refactor.*\`, \`apoc.load/import/export.*\`, \`apoc.trigger.*\`, \`apoc.nodes.delete\`) are pre-rejected — don't try them. - - Always include a \`LIMIT\`. The tool caps rows, row width, and total response size, but short queries are faster and cheaper. - - Project specific fields (\`RETURN n.stId, n.displayName\`) rather than whole nodes when you don't need every property. - -**When NOT to use Cypher:** for lookups by ID or name, pathway hierarchies, or enrichment — the REST tools are faster, cached, and do the formatting for you. -`.trim(); - export function buildServerInstructions(): string { - const parts = [CORE_INSTRUCTIONS]; - // Not `isNeo4jConfigured()`: with a connection but no opt-in the tools this - // section tells the client to call do not exist. - if (isCypherEnabled()) parts.push(CYPHER_INSTRUCTIONS); - return parts.join("\n\n"); + // One section, unconditionally. There used to be a Cypher section appended + // when a graph connection was configured, and it was the surface that got + // left behind when the tools were gated -- a server describing tools it had + // not registered. Both are gone. + return CORE_INSTRUCTIONS; } diff --git a/src/resources/static.ts b/src/resources/static.ts index f0762db..0edbe1c 100644 --- a/src/resources/static.ts +++ b/src/resources/static.ts @@ -1,8 +1,6 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { contentClient } from "../clients/content.js"; import type { Species, Disease } from "../types/index.js"; -import { isCypherEnabled } from "../clients/neo4j.js"; -import { fetchGraphSchema } from "../graph/schema.js"; export function registerStaticResources(server: McpServer) { // All species @@ -69,23 +67,4 @@ export function registerStaticResources(server: McpServer) { ], }; }); - - // Graph schema — the same opt-in as the Cypher tools. It is not a query - // surface, but it runs apoc.meta.schema() for the caller and publishes the - // internal graph model, so a server told not to offer Cypher should not be - // handing this out either. - if (isCypherEnabled()) { - 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/tools/cypher-guard.ts b/src/tools/cypher-guard.ts deleted file mode 100644 index bb2400b..0000000 --- a/src/tools/cypher-guard.ts +++ /dev/null @@ -1,56 +0,0 @@ -// Guard against APOC procedures that bypass a READ-mode session. -// -// Neo4j's session access mode (READ) rejects native write clauses -// (CREATE/MERGE/DELETE/SET/REMOVE). But some APOC procedures — which are -// loaded in the reactome_neo4j_env image — open their own transactions and -// can write, load remote URLs, or touch the filesystem regardless of the -// calling session mode. Explicitly block the well-known offenders here. -// -// This is a guardrail, not a security boundary. A determined user can hide -// intent (string concatenation, dynamic CALL, obfuscation) and a real trust -// boundary has to live at the Neo4j RBAC / plugin-config layer. For the -// curator-facing use case this check is sufficient to prevent accidents. - -const WRITE_THROUGH_PATTERNS: Array<{ pattern: RegExp; label: string }> = [ - { - pattern: /\bapoc\.cypher\.(runWrite|doIt)\b/i, - label: "apoc.cypher.runWrite / apoc.cypher.doIt", - }, - { - pattern: /\bapoc\.periodic\.(iterate|commit|submit|countdown|repeat)\b/i, - label: "apoc.periodic.*", - }, - { - pattern: /\bapoc\.(create|merge|refactor)\.[a-z]\w*/i, - label: "apoc.create.* / apoc.merge.* / apoc.refactor.*", - }, - { pattern: /\bapoc\.nodes\.delete\b/i, label: "apoc.nodes.delete" }, - { - pattern: /\bapoc\.(load|import|export)\.[a-z]\w*/i, - label: "apoc.load.* / apoc.import.* / apoc.export.*", - }, - { pattern: /\bapoc\.trigger\.[a-z]\w*/i, label: "apoc.trigger.*" }, -]; - -function stripComments(query: string): string { - // // line comments, then /* */ block comments - return query.replace(/\/\/[^\n]*/g, " ").replace(/\/\*[\s\S]*?\*\//g, " "); -} - -export class WriteThroughRejected extends Error { - constructor(public readonly matchedLabel: string) { - super( - `Query rejected: calls to ${matchedLabel} can bypass READ-mode sessions and write to or load data from outside the graph. This MCP blocks those procedures. If you have a legitimate need, run the query outside this server against a read replica.` - ); - this.name = "WriteThroughRejected"; - } -} - -export function rejectWriteThroughCalls(query: string): void { - const cleaned = stripComments(query); - for (const { pattern, label } of WRITE_THROUGH_PATTERNS) { - if (pattern.test(cleaned)) { - throw new WriteThroughRejected(label); - } - } -} diff --git a/src/tools/cypher.ts b/src/tools/cypher.ts deleted file mode 100644 index 36f3ce2..0000000 --- a/src/tools/cypher.ts +++ /dev/null @@ -1,202 +0,0 @@ -import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { z } from "zod"; -import { runRead } from "../clients/neo4j.js"; -import { fetchGraphSchema } from "../graph/schema.js"; -import { formatGraphSchemaMarkdown } from "../graph/format-schema.js"; -import { logger } from "../logger.js"; -import { rejectWriteThroughCalls } from "./cypher-guard.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 }, - }; -} - -export function registerCypherTools(server: McpServer) { - server.tool( - "reactome_cypher_query", - "Run a Cypher query against the local Reactome Neo4j graph database. The session runs in READ mode, which rejects native write clauses (CREATE/MERGE/DELETE/SET/REMOVE). APOC procedures that can write through that guardrail (apoc.cypher.runWrite, apoc.periodic.*, apoc.create/merge/refactor.*, apoc.load/import/export.*, apoc.trigger.*, apoc.nodes.delete) are rejected before execution. Row count, per-row size, and total response size are capped; a query timeout terminates runaway queries. Use LIMIT and project specific fields in your query for large results.", - { - 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 }) => { - rejectWriteThroughCalls(query); - logger.info("cypher_query", { chars: query.length, max_rows }); - const rows = await runRead>(query, params ?? {}); - const { output, stats } = capCypherRows(rows, max_rows, max_row_chars, max_total_chars); - - const notes: string[] = []; - notes.push(`**Rows returned:** ${output.length} of ${rows.length}`); - if (stats.rowCountTruncated) notes.push(`(row-count cap: ${max_rows})`); - 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 with node counts, relationship types with cardinalities, per-label and per-rel property types (with mandatory flags), indexes, and constraints. Fetched live from the database via APOC on first call and cached in-memory for the rest of the session (~100–300 ms one-time). Call this before writing Cypher. For the full JSON (including the raw apoc.meta.schema() object), 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 2ff6f03..48da54f 100644 --- a/src/tools/index.ts +++ b/src/tools/index.ts @@ -11,9 +11,6 @@ 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 { isCypherEnabled, isNeo4jConfigured } from "../clients/neo4j.js"; -import { logger } from "../logger.js"; import { withNewRequestContext } from "../context.js"; import { capToolResult } from "../response-limits.js"; import { MAX_TOOL_RESPONSE_CHARS } from "../config.js"; @@ -64,21 +61,10 @@ export function registerAllTools(server: McpServer) { registerInteractorTools(server); registerGsaTools(server); - // Graph database tools — a connection AND an explicit opt-in. - // - // `NEO4J_URI` alone used to be enough, which made arbitrary query access a - // side effect of a connection string. A public instance that set it for any - // other reason would have published `reactome_cypher_query`. - if (isCypherEnabled()) { - registerCypherTools(server); - } else if (isNeo4jConfigured()) { - // Said out loud, because an operator who set NEO4J_URI expecting these - // tools needs to know why they are absent. The reverse -- silently - // present -- is the failure this guard exists for. - logger.warn("Cypher tools are OFF: MCP_ALLOW_CYPHER is not 1", { - hint: "Set MCP_ALLOW_CYPHER=1 on an instance that is not publicly reachable.", - }); - } + // 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. // Register utility tools directly here registerUtilityTools(server); diff --git a/tests/cypher-exposure.test.ts b/tests/cypher-exposure.test.ts deleted file mode 100644 index 145d549..0000000 --- a/tests/cypher-exposure.test.ts +++ /dev/null @@ -1,185 +0,0 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; - -/** - * Who can run arbitrary graph queries. - * - * Until 2026-09-21 the Cypher tools registered whenever `NEO4J_URI` was set, - * so "can the public run Cypher" was a side effect of a connection string. A - * public instance that set it for the graph-schema warm-up, or for a future - * non-Cypher graph tool, would have published `reactome_cypher_query` by - * doing so — and nothing would have looked wrong. - * - * These drive `registerAllTools` with a recording stub and assert on which - * tool names reach it, because that is the only place the answer is visible. - */ - -const TOOL_NAMES = () => { - const names: string[] = []; - const server = { - tool: (...args: unknown[]) => { - if (typeof args[0] === "string") names.push(args[0]); - return undefined; - }, - }; - return { server, names }; -}; - -async function registerWith(env: Record) { - vi.resetModules(); - const previous: Record = {}; - for (const [key, value] of Object.entries(env)) { - previous[key] = process.env[key]; - if (value === undefined) delete process.env[key]; - else process.env[key] = value; - } - try { - const { registerAllTools } = await import("../src/tools/index.js"); - const { server, names } = TOOL_NAMES(); - registerAllTools(server as never); - return names; - } finally { - for (const [key, value] of Object.entries(previous)) { - if (value === undefined) delete process.env[key]; - else process.env[key] = value; - } - } -} - -const isCypher = (name: string) => name.startsWith("reactome_cypher"); - -describe("Cypher tool exposure", () => { - beforeEach(() => vi.resetModules()); - afterEach(() => vi.resetModules()); - - it("registers no Cypher tools with neither variable set", async () => { - const names = await registerWith({ - NEO4J_URI: undefined, - MCP_ALLOW_CYPHER: undefined, - }); - expect(names.filter(isCypher)).toEqual([]); - expect(names.length).toBeGreaterThan(0); - }); - - it("registers no Cypher tools when NEO4J_URI alone is set", async () => { - // The case that mattered: a connection string is not consent. This is - // the configuration a public instance would most plausibly arrive at. - const names = await registerWith({ - NEO4J_URI: "bolt://localhost:7690", - MCP_ALLOW_CYPHER: undefined, - }); - expect(names.filter(isCypher)).toEqual([]); - }); - - it("registers no Cypher tools when the opt-in is set without a connection", async () => { - const names = await registerWith({ - NEO4J_URI: undefined, - MCP_ALLOW_CYPHER: "1", - }); - expect(names.filter(isCypher)).toEqual([]); - }); - - it("registers them only when both are set", async () => { - // Without this the three above would pass against a build that never - // registers Cypher at all, and prove nothing. - const names = await registerWith({ - NEO4J_URI: "bolt://localhost:7690", - MCP_ALLOW_CYPHER: "1", - }); - expect(names.filter(isCypher).length).toBeGreaterThan(0); - }); - - it("treats any value other than 1 as not opted in", async () => { - for (const value of ["", "0", "true", "yes", "TRUE"]) { - const names = await registerWith({ - NEO4J_URI: "bolt://localhost:7690", - MCP_ALLOW_CYPHER: value, - }); - expect(names.filter(isCypher), `MCP_ALLOW_CYPHER=${value}`).toEqual([]); - } - }); -}); - -/** - * The tools were the obvious surface. Two others answer the same question and - * were left behind by the first version of this guard: the server's own - * instructions, which tell a client Cypher is available and name the tools to - * call, and the `reactome://graph/schema` resource, which runs - * apoc.meta.schema() for the caller and returns the internal graph model. - * - * Each of these is asserted in both directions. The absent case alone would - * pass against a build that never offers the thing at all. - */ - -async function withEnv( - env: Record, - body: () => Promise -): Promise { - vi.resetModules(); - const previous: Record = {}; - for (const [key, value] of Object.entries(env)) { - previous[key] = process.env[key]; - if (value === undefined) delete process.env[key]; - else process.env[key] = value; - } - try { - return await body(); - } finally { - for (const [key, value] of Object.entries(previous)) { - if (value === undefined) delete process.env[key]; - else process.env[key] = value; - } - } -} - -const CONNECTED_ONLY = { NEO4J_URI: "bolt://localhost:7690", MCP_ALLOW_CYPHER: undefined }; -const OPTED_IN = { NEO4J_URI: "bolt://localhost:7690", MCP_ALLOW_CYPHER: "1" }; - -async function instructions(env: Record) { - return withEnv(env, async () => { - const { buildServerInstructions } = await import("../src/instructions.js"); - return buildServerInstructions(); - }); -} - -describe("server instructions", () => { - it("does not advertise Cypher on a connection alone", async () => { - const text = await instructions(CONNECTED_ONLY); - expect(text).not.toContain("reactome_cypher_query"); - expect(text).not.toContain("Graph database (Cypher)"); - // Still a usable server: the core instructions are there. - expect(text).toContain("reactome_search"); - }); - - it("advertises Cypher once opted in", async () => { - const text = await instructions(OPTED_IN); - expect(text).toContain("reactome_cypher_query"); - }); -}); - -async function resourceNames(env: Record) { - return withEnv(env, async () => { - const { registerStaticResources } = await import("../src/resources/static.js"); - const names: string[] = []; - const server = { - resource: (...args: unknown[]) => { - if (typeof args[0] === "string") names.push(args[0]); - return undefined; - }, - }; - registerStaticResources(server as never); - return names; - }); -} - -describe("graph schema resource", () => { - it("is not registered on a connection alone", async () => { - const names = await resourceNames(CONNECTED_ONLY); - expect(names).not.toContain("reactome://graph/schema"); - expect(names).toContain("reactome://species"); - }); - - it("is registered once opted in", async () => { - const names = await resourceNames(OPTED_IN); - expect(names).toContain("reactome://graph/schema"); - }); -}); diff --git a/tests/cypher-guard.test.ts b/tests/cypher-guard.test.ts deleted file mode 100644 index 483f63c..0000000 --- a/tests/cypher-guard.test.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { rejectWriteThroughCalls, WriteThroughRejected } from "../src/tools/cypher-guard.js"; - -describe("rejectWriteThroughCalls", () => { - it("accepts a plain read query", () => { - expect(() => rejectWriteThroughCalls("MATCH (n:Pathway) RETURN n LIMIT 10")).not.toThrow(); - }); - - it("accepts queries that mention apoc in a string literal if no procedure call", () => { - // This is a query that reads only — "apoc" appears inside a string and - // the regex uses word-boundary procedure shape, so it won't match. - expect(() => - rejectWriteThroughCalls(`MATCH (n) WHERE n.note = 'see apoc docs' RETURN n`) - ).not.toThrow(); - }); - - it("rejects apoc.cypher.runWrite", () => { - expect(() => - rejectWriteThroughCalls("CALL apoc.cypher.runWrite('CREATE (n) RETURN n', {})") - ).toThrow(WriteThroughRejected); - }); - - it("rejects apoc.cypher.doIt regardless of case", () => { - expect(() => rejectWriteThroughCalls("CALL APOC.CYPHER.DOIT('CREATE (n)', {})")).toThrow( - WriteThroughRejected - ); - }); - - it("rejects apoc.periodic.iterate", () => { - expect(() => - rejectWriteThroughCalls( - "CALL apoc.periodic.iterate('MATCH (n) RETURN n', 'SET n.flag = 1', {})" - ) - ).toThrow(WriteThroughRejected); - }); - - it("rejects apoc.create.node", () => { - expect(() => - rejectWriteThroughCalls("CALL apoc.create.node(['X'], {}) YIELD node RETURN node") - ).toThrow(WriteThroughRejected); - }); - - it("rejects apoc.refactor.mergeNodes", () => { - expect(() => rejectWriteThroughCalls("CALL apoc.refactor.mergeNodes([n1, n2])")).toThrow( - WriteThroughRejected - ); - }); - - it("rejects apoc.load.json (SSRF risk)", () => { - expect(() => - rejectWriteThroughCalls( - "CALL apoc.load.json('http://evil.example/data') YIELD value RETURN value" - ) - ).toThrow(WriteThroughRejected); - }); - - it("rejects apoc.export.csv.all", () => { - expect(() => rejectWriteThroughCalls("CALL apoc.export.csv.all('/tmp/x.csv', {})")).toThrow( - WriteThroughRejected - ); - }); - - it("rejects apoc.nodes.delete", () => { - expect(() => rejectWriteThroughCalls("CALL apoc.nodes.delete([1,2,3], 100)")).toThrow( - WriteThroughRejected - ); - }); - - it("strips line comments so commented-out bad calls are ignored", () => { - expect(() => - rejectWriteThroughCalls("// CALL apoc.cypher.runWrite('...')\nMATCH (n) RETURN n") - ).not.toThrow(); - }); - - it("does not strip comments when the bad call is in active code", () => { - expect(() => - rejectWriteThroughCalls("// harmless comment\nCALL apoc.periodic.commit('DELETE n', {})") - ).toThrow(WriteThroughRejected); - }); - - it("accepts read-only apoc procedures (apoc.meta.schema, apoc.path.*)", () => { - expect(() => - rejectWriteThroughCalls("CALL apoc.meta.schema() YIELD value RETURN value") - ).not.toThrow(); - expect(() => - rejectWriteThroughCalls( - "MATCH (n) CALL apoc.path.subgraphAll(n, {}) YIELD nodes RETURN nodes" - ) - ).not.toThrow(); - }); -}); diff --git a/tests/cypher.test.ts b/tests/cypher.test.ts deleted file mode 100644 index 175d895..0000000 --- a/tests/cypher.test.ts +++ /dev/null @@ -1,46 +0,0 @@ -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-schema.test.ts b/tests/graph-schema.test.ts deleted file mode 100644 index 53e8eea..0000000 --- a/tests/graph-schema.test.ts +++ /dev/null @@ -1,188 +0,0 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import type { GraphSchema } from "../src/graph/schema.js"; -import { formatGraphSchemaMarkdown } from "../src/graph/format-schema.js"; - -function minimalSchema(): GraphSchema { - return { - fetchedAt: "2026-04-24T15:00:00Z", - dbComponents: [{ name: "Neo4j Kernel", versions: ["4.3.6"], edition: "enterprise" }], - stats: { - nodeCount: 100, - relCount: 200, - labels: { Pathway: 50, Reaction: 30, Entity: 20 }, - relTypes: { "(:Pathway)-[:hasEvent]->(:Reaction)": 80 }, - relTypesCount: { hasEvent: 80, inputOf: 120 }, - }, - schema: { Pathway: { type: "node" } }, - nodeTypeProperties: [ - { - nodeType: ":`Pathway`", - nodeLabels: ["Pathway"], - propertyName: "stId", - propertyTypes: ["String"], - mandatory: true, - }, - { - nodeType: ":`Pathway`", - nodeLabels: ["Pathway"], - propertyName: "displayName", - propertyTypes: ["String"], - mandatory: false, - }, - ], - relTypeProperties: [ - { - relType: "hasEvent", - sourceNodeLabels: ["Pathway"], - targetNodeLabels: ["Reaction"], - propertyName: "stoichiometry", - propertyTypes: ["Long"], - mandatory: false, - }, - ], - indexes: [ - { - name: "pathway_stId", - labelsOrTypes: ["Pathway"], - properties: ["stId"], - type: "BTREE", - state: "ONLINE", - }, - ], - constraints: [ - { - name: "pathway_stId_unique", - description: "CONSTRAINT ON ( pathway:Pathway ) ASSERT (pathway.stId) IS UNIQUE", - }, - ], - }; -} - -describe("formatGraphSchemaMarkdown", () => { - it("covers totals, labels, rel types, properties, indexes, constraints", () => { - const md = formatGraphSchemaMarkdown(minimalSchema()); - expect(md).toContain("Neo4j:** 4.3.6"); - expect(md).toContain("100 nodes"); - expect(md).toContain("`Pathway` — 50"); - expect(md).toContain("`hasEvent` — 80"); - expect(md).toContain("`stId` _(String)_ **required**"); - expect(md).toContain("### Indexes (1)"); - expect(md).toContain("### Constraints (1)"); - expect(md).toContain("reactome://graph/schema"); - }); - - it("stays small enough for an LLM context window", () => { - expect(formatGraphSchemaMarkdown(minimalSchema()).length).toBeLessThan(10_000); - }); - - it("handles missing indexes/constraints gracefully", () => { - const empty = { ...minimalSchema(), indexes: [], constraints: [] }; - const md = formatGraphSchemaMarkdown(empty); - expect(md).not.toContain("### Indexes"); - expect(md).not.toContain("### Constraints"); - }); - - it("orders labels and rel types by count, descending", () => { - const md = formatGraphSchemaMarkdown(minimalSchema()); - const pathwayIdx = md.indexOf("`Pathway` — 50"); - const reactionIdx = md.indexOf("`Reaction` — 30"); - const entityIdx = md.indexOf("`Entity` — 20"); - expect(pathwayIdx).toBeLessThan(reactionIdx); - expect(reactionIdx).toBeLessThan(entityIdx); - }); -}); - -// Mock runRead from the neo4j client module so fetchGraphSchema (which -// imports from that module) sees the mock. -vi.mock("../src/clients/neo4j.js", () => ({ - runRead: vi.fn(), -})); - -import { runRead } from "../src/clients/neo4j.js"; -import { fetchGraphSchema, _resetGraphSchemaCache } from "../src/graph/schema.js"; - -function setupRunReadResponses() { - (runRead as ReturnType).mockImplementation(async (cypher: string) => { - if (cypher.includes("dbms.components")) - return [{ name: "Neo4j Kernel", versions: ["4.3.6"], edition: "enterprise" }]; - if (cypher.includes("apoc.meta.stats")) - return [{ nodeCount: 10, relCount: 5, labels: { X: 10 }, relTypes: {}, relTypesCount: {} }]; - if (cypher.includes("apoc.meta.schema")) return [{ value: {} }]; - return []; - }); -} - -describe("fetchGraphSchema caching", () => { - beforeEach(() => { - _resetGraphSchemaCache(); - (runRead as ReturnType).mockReset(); - }); - - afterEach(() => { - _resetGraphSchemaCache(); - }); - - it("caches the result; second call is a no-op on the driver", async () => { - setupRunReadResponses(); - const first = await fetchGraphSchema(); - const callsAfterFirst = (runRead as ReturnType).mock.calls.length; - const second = await fetchGraphSchema(); - const callsAfterSecond = (runRead as ReturnType).mock.calls.length; - - expect(first).toBe(second); - expect(callsAfterSecond).toBe(callsAfterFirst); - }); - - it("dedupes concurrent in-flight calls to a single fetch", async () => { - let resolveStats: (v: unknown) => void = () => {}; - const statsPromise = new Promise(r => { - resolveStats = r; - }); - - (runRead as ReturnType).mockImplementation(async (cypher: string) => { - if (cypher.includes("dbms.components")) - return [{ name: "Neo4j Kernel", versions: ["4.3.6"], edition: "enterprise" }]; - if (cypher.includes("apoc.meta.stats")) { - await statsPromise; - return [{ nodeCount: 1, relCount: 1, labels: {}, relTypes: {}, relTypesCount: {} }]; - } - if (cypher.includes("apoc.meta.schema")) return [{ value: {} }]; - return []; - }); - - const p1 = fetchGraphSchema(); - const p2 = fetchGraphSchema(); - const p3 = fetchGraphSchema(); - - resolveStats(undefined); - const [r1, r2, r3] = await Promise.all([p1, p2, p3]); - expect(r1).toBe(r2); - expect(r2).toBe(r3); - - const statsCalls = (runRead as ReturnType).mock.calls.filter(([q]) => - String(q).includes("apoc.meta.stats") - ).length; - expect(statsCalls).toBe(1); - }); - - it("recovers when optional calls fail (relTypeProperties / indexes / constraints)", async () => { - (runRead as ReturnType).mockImplementation(async (cypher: string) => { - if (cypher.includes("dbms.components")) - return [{ name: "Neo4j Kernel", versions: ["4.3.6"], edition: "enterprise" }]; - if (cypher.includes("apoc.meta.stats")) - return [{ nodeCount: 1, relCount: 1, labels: {}, relTypes: {}, relTypesCount: {} }]; - if (cypher.includes("apoc.meta.schema")) return [{ value: {} }]; - if (cypher.includes("apoc.meta.nodeTypeProperties")) return []; - // Simulate older Neo4j where these procs don't exist or return differently - if (cypher.includes("apoc.meta.relTypeProperties")) throw new Error("no such proc"); - if (cypher.includes("db.indexes")) throw new Error("no such proc"); - if (cypher.includes("db.constraints")) throw new Error("no such proc"); - return []; - }); - - const schema = await fetchGraphSchema(); - expect(schema.relTypeProperties).toEqual([]); - expect(schema.indexes).toEqual([]); - expect(schema.constraints).toEqual([]); - }); -}); diff --git a/tests/input-bounds.test.ts b/tests/input-bounds.test.ts index e8bb9fb..4eeb716 100644 --- a/tests/input-bounds.test.ts +++ b/tests/input-bounds.test.ts @@ -94,12 +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 in every configuration, including Cypher", async () => { + it("covers every tool, and the set does not change with the environment", async () => { const schemas = await allSchemas(); - // Without this the sweep would pass vacuously against an empty map, and - // without the Cypher names it would pass while missing three tools. - expect(schemas.size).toBe(62); - expect([...schemas.keys()].filter(n => n.startsWith("reactome_cypher"))).toHaveLength(3); + // 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([]); }); 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 new file mode 100644 index 0000000..08f53c9 --- /dev/null +++ b/tests/no-graph-access.test.ts @@ -0,0 +1,100 @@ +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 new file mode 100644 index 0000000..94f4cfd --- /dev/null +++ b/tests/readme-tools.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { registerAllTools } from "../src/tools/index.js"; + +/** + * The README says it lists every tool. It should be true. + * + * This list is about to be the public description of a publicly hosted + * server, so "which tools exist" stops being a documentation detail. When + * this was first written the README said "over 40 tools" and omitted the GSA + * section entirely — five tools that were registered, served, and documented + * nowhere. + * + * A prose count goes stale the first time someone adds a tool. A test does + * not. + */ +describe("README tool coverage", () => { + const readme = readFileSync(new URL("../README.md", import.meta.url), "utf8"); + + const registered = (): string[] => { + const names: string[] = []; + registerAllTools({ + tool: (...args: unknown[]) => { + if (typeof args[0] === "string") names.push(args[0]); + return undefined; + }, + } as never); + return names; + }; + + it("documents every registered tool", () => { + const missing = registered().filter(name => !readme.includes(name)); + expect(missing).toEqual([]); + }); + + it("states the tool count, and states it correctly", () => { + const names = registered(); + expect(names.length).toBeGreaterThan(0); + expect(readme).toContain(`${names.length} tools`); + }); +}); diff --git a/tests/server.test.ts b/tests/server.test.ts index 9677dd0..7b078eb 100644 --- a/tests/server.test.ts +++ b/tests/server.test.ts @@ -43,14 +43,23 @@ describe("createServer", () => { expect(toolNames).toContain("reactome_analyze_identifiers"); }); - it("keeps the graph tools behind the NEO4J_URI gate", () => { - // Principle IV: no deployment the team runs holds a Neo4j connection. - // NEO4J_URI is unset in the test environment, so these must be absent. - const server = createServer(); - const registered = server as unknown as { _registeredTools?: Record }; - const toolNames = Object.keys(registered._registeredTools ?? {}); + 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"; + try { + const server = createServer(); + const registered = server as unknown as { _registeredTools?: Record }; + const toolNames = Object.keys(registered._registeredTools ?? {}); - expect(process.env.NEO4J_URI).toBeFalsy(); - expect(toolNames).not.toContain("reactome_cypher_query"); + expect(toolNames.filter(n => n.includes("cypher"))).toEqual([]); + expect(toolNames.length).toBeGreaterThan(40); + } finally { + delete process.env.NEO4J_URI; + delete process.env.MCP_ALLOW_CYPHER; + } }); });