Restore graph database access, off unless NEO4J_URI and MCP_ALLOW_CYPHER=1

.specify/memory/constitution.md

@@ -1,18 +1,22 @@
 <!--
 Sync Impact Report
 ==================
-Version: none -> 1.0.0  (initial ratification)
+Version: 1.0.0 -> 2.0.0  (2026-10-14)
 
-Added:    Principles I-VII, Scope boundaries, Development workflow, Governance
-Modified: none
+Modified: Principle IV redefined -- "no Neo4j connection in any deployment"
+          becomes "no graph access on a public instance; internal instances
+          may opt in behind one gate that the driver itself enforces".
+          Scope boundaries updated to match.
+Added:    none
 Removed:  none
 
-Rationale: first constitution for this repository. The principles are not
-aspirational -- each one is written from a failure this repo actually had, or a
-boundary the Reactome team has already decided. Sources are named inline so a
-future reader can check whether the reason still holds.
+Rationale: Adam asked for the Cypher tools back for internal instances pointed
+at a dedicated graph database container, switched off by default so a public
+instance can never run Cypher. MAJOR because a principle was redefined, not
+reworded. The reason for the 2026-09-21 removal (a gate asked in four places,
+one of them wrong) is kept below, with what is different this time.
 
-Follow-up TODOs: none.
+Previous: 1.0.0 (initial ratification, 2026-09-14).
 -->
 
 # 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

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<T>` 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`.

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`.