From a786e7d5dfa730b44be402dafd1b2306a4b6e391 Mon Sep 17 00:00:00 2001 From: pitoi Date: Fri, 28 Aug 2026 11:24:39 +0000 Subject: [PATCH] Enforce read-only Hive queries at the Bolt transaction level; map DB write rejection to 403 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GraphOps::execute_raw_cypher now runs through neo4rs execute_read() (Bolt autocommit mode "r"), so Neo4j itself refuses any write — including write procedures invoked via CALL, which the keyword denylist cannot generally catch. Database-level rejections (Neo.ClientError.Statement.AccessMode, or ProcedureCallFailed wrapping an access-mode violation) surface as shared::Error::ReadOnlyViolation and map to HTTP 403 in the Hive handler with the same body as the denylist rejection; warn! logs carry the Neo4j error code and query length (never the query body) at both boundaries. The denylist, forced LIMIT, and 4096-char cap stay as defense-in-depth, and the previously false 'read-mode transaction' comments now describe the real implementation. A test-only hive_query_handler_denylist_bypassed entry point (not wired into the router) lets the integration test prove the guarantee comes from the database: with the denylist disabled, CREATE and CALL apoc.create.node probes both return 403 and leave no trace in the graph, while MATCH and read-only procedures keep working (200). Verified against live Neo4j 5.19. --- Cargo.lock | 1 + ast/src/lang/graphs/graph_ops.rs | 77 ++++++-- shared/src/error.rs | 3 + standalone/Cargo.toml | 5 + standalone/src/handlers/hive_query.rs | 151 ++++++++++++-- standalone/src/types.rs | 7 +- standalone/tests/hive_read_only.rs | 273 ++++++++++++++++++++++++++ 7 files changed, 483 insertions(+), 34 deletions(-) create mode 100644 standalone/tests/hive_read_only.rs diff --git a/Cargo.lock b/Cargo.lock index d8034d25f..1e1265097 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4573,6 +4573,7 @@ dependencies = [ "hex", "hmac", "lsp", + "neo4rs", "once_cell", "regex", "reqwest", diff --git a/ast/src/lang/graphs/graph_ops.rs b/ast/src/lang/graphs/graph_ops.rs index a32ee4ac2..4d2a54b6b 100644 --- a/ast/src/lang/graphs/graph_ops.rs +++ b/ast/src/lang/graphs/graph_ops.rs @@ -589,15 +589,19 @@ impl GraphOps { /// Execute an arbitrary read-only Cypher query and return a flat `(columns, rows)` result. /// /// ## Write protection - /// The bolt transaction is opened in **read mode** so Neo4j itself rejects any write - /// operation — including write procedures invoked via `CALL` that bypass the - /// application-level keyword denylist. The caller's denylist check is defense-in-depth. + /// The query runs through `connection.execute_read()`, which opens the bolt + /// transaction in **read mode** (`mode: "r"` autocommit metadata). Neo4j itself + /// rejects any write attempted inside it — including write procedures invoked + /// via `CALL` (e.g. APOC) — so write protection does not depend on the caller's + /// keyword denylist, which stays as defense-in-depth. Such database-level + /// rejections are surfaced as [`Error::ReadOnlyViolation`] carrying the Neo4j + /// status code; every other failure keeps the generic dependency-error mapping. /// /// ## Column names - /// Column names are derived from the first row returned by the query. neo4rs 0.8.x does - /// not expose a `keys()` method on `DetachedRowStream` (the internal `fields` BoltList - /// from the protocol response is not publicly accessible). As a consequence, empty - /// result sets will return `columns: []` — this is an API limitation of neo4rs 0.8.x. + /// Column names are derived from the first row returned by the query. The + /// `DetachedRowStream` does not expose the response's `fields` BoltList before + /// row consumption. As a consequence, empty result sets will return + /// `columns: []`. /// /// ## Graph object serialization /// Each row is deserialized via `row.to::()` which uses the neo4rs @@ -619,14 +623,16 @@ impl GraphOps { let connection = self.graph.ensure_connected().await?; let query_obj = query(cypher); + let query_len = cypher.len(); + // execute_read() sends the bolt RUN with `mode: "r"` autocommit metadata, so the + // server enforces read-only access for the whole result stream. let mut stream = connection - .execute(query_obj) + .execute_read(query_obj) .await - .map_err(|e| Error::dependency(format!("Neo4j execute error: {e}")))?; + .map_err(|e| map_read_mode_error("Neo4j execute error", e, query_len))?; - // neo4rs 0.8.x does not expose column names from DetachedRowStream before - // row consumption. Columns are initialized from the first row's keys. + // neo4rs initializes column names from the first row's keys. let mut columns: Vec = Vec::new(); let mut rows: Vec> = Vec::new(); let mut columns_initialized = false; @@ -634,7 +640,7 @@ impl GraphOps { while let Some(row) = stream .next() .await - .map_err(|e| Error::dependency(format!("Neo4j stream error: {e}")))? + .map_err(|e| map_read_mode_error("Neo4j stream error", e, query_len))? { // Deserialize the row as a serde_json::Value::Object to extract both column // names (keys) and typed values in a single pass. @@ -660,3 +666,50 @@ impl GraphOps { Ok((columns, rows)) } } + +/// Map a neo4rs error raised by the read-mode execution path to a `shared::Error`. +/// +/// A rejection with status code `Neo.ClientError.Statement.AccessMode` is Neo4j +/// refusing a write inside the read-mode bolt transaction — surfaced as +/// [`Error::ReadOnlyViolation`] carrying the status code. Write-mode procedures +/// invoked via `CALL` (e.g. APOC writes) can surface as +/// `Neo.ClientError.Procedure.ProcedureCallFailed` with the same access-mode +/// violation in the server message; those are classified here too. Every other +/// error keeps the generic dependency-error mapping. +/// +/// On a database-level rejection this logs a `warn!` with the Neo4j error code and +/// the query length only — never the query body. This is the signal separating +/// "the denylist caught it" from "the denylist had a hole and the database +/// saved us". +fn map_read_mode_error(context: &str, e: neo4rs::Error, query_len: usize) -> Error { + if let neo4rs::Error::Neo4j(ne) = &e { + let code = ne.code(); + if is_write_in_read_mode(code, ne.message()) { + tracing::warn!( + neo4j_error_code = code, + query_len, + "Neo4j rejected a write attempt inside the read-mode bolt transaction" + ); + return Error::ReadOnlyViolation(code.to_string()); + } + } + Error::dependency(format!("{context}: {e}")) +} + +/// True when a Neo4j server failure indicates a write attempted under read access +/// mode. Matches the direct access-mode status code plus the procedure-call-wrapped +/// form, whose message names the offending write while its cause chain carries the +/// access-mode violation. +fn is_write_in_read_mode(code: &str, message: &str) -> bool { + if code == "Neo.ClientError.Statement.AccessMode" { + return true; + } + if !code.starts_with("Neo.ClientError.Procedure") { + return false; + } + let m = message.to_lowercase(); + m.contains("read access mode") + || m.contains("writing in read") + || m.contains("write in read") + || m.contains("not allowed unless") +} diff --git a/shared/src/error.rs b/shared/src/error.rs index e9fa55228..bd233e5c8 100644 --- a/shared/src/error.rs +++ b/shared/src/error.rs @@ -50,6 +50,9 @@ pub enum Error { #[error("Dependency error: {0}")] Dependency(String), + #[error("Read-only violation: {0}")] + ReadOnlyViolation(String), + #[error("Internal error: {0}")] Internal(String), diff --git a/standalone/Cargo.toml b/standalone/Cargo.toml index 7b6fe0df7..e79d186ca 100644 --- a/standalone/Cargo.toml +++ b/standalone/Cargo.toml @@ -34,3 +34,8 @@ once_cell = "1" [features] neo4j = ["ast/neo4j"] fulltest = ["ast/fulltest"] + +[dev-dependencies] +# Direct (write-mode) bolt connection for the hive read-only integration test: +# it verifies the database itself rejected the write and left no trace. +neo4rs = "0.9.0-rc.10" diff --git a/standalone/src/handlers/hive_query.rs b/standalone/src/handlers/hive_query.rs index 7e0815de5..7815abfc6 100644 --- a/standalone/src/handlers/hive_query.rs +++ b/standalone/src/handlers/hive_query.rs @@ -10,8 +10,12 @@ use serde_json::json; /// Write-keyword denylist — case-insensitive word-boundary patterns. /// /// `CALL` is intentionally omitted: read-only procedures (e.g. `CALL db.labels()`) are -/// legitimate Graph Explorer queries. Write procedures invoked via `CALL` are blocked at -/// the Neo4j transaction layer by read-mode enforcement in `execute_raw_cypher`. +/// legitimate Graph Explorer queries. Write procedures invoked via `CALL` (e.g. APOC +/// writes) are rejected by Neo4j itself: `execute_raw_cypher` runs every query through a +/// read-mode bolt transaction, and the server refuses any write inside it. This denylist +/// stays as defense-in-depth underneath that database-level guarantee, and it remains +/// the only guard for `LOAD` (an SSRF vector that performs no database write, so the +/// read-mode transaction does not block it). /// /// `FOREACH` and `LOAD` are included: /// - `FOREACH` is a native Cypher write clause. @@ -87,13 +91,58 @@ pub struct HiveQueryResponse { pub rows: Vec>, } +/// 403 response for a query rejected by the write-keyword denylist. +/// +/// Deliberately independent from [`read_only_violation_response`]: the unit test +/// asserting both bodies are byte-identical only has drift-detection value if the +/// two literals are not trivially the same constant. +fn denylist_rejection_response() -> Response { + ( + StatusCode::FORBIDDEN, + Json(json!({"error": "write operations not permitted"})), + ) + .into_response() +} + +/// 403 response for a query rejected by the database inside the read-mode bolt +/// transaction (`Error::ReadOnlyViolation`). +fn read_only_violation_response() -> Response { + ( + StatusCode::FORBIDDEN, + Json(json!({"error": "write operations not permitted"})), + ) + .into_response() +} + +/// Which pre-execution write guards to apply. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum DenylistMode { + /// Production behavior: keyword denylist, then database-level read mode. + Enforced, + /// Skip the keyword denylist so tests can exercise the database-level + /// read-mode guard in isolation. Not reachable from any HTTP route. + Bypassed, +} + /// `POST /api/hive/query` /// /// Validates the request, applies the write-keyword denylist, enforces a server-controlled -/// LIMIT, and proxies the query to Neo4j via a read-mode bolt transaction. -pub async fn hive_query_handler( - Json(body): Json, -) -> Response { +/// LIMIT, and proxies the query to Neo4j through a read-mode bolt transaction (the +/// database itself refuses any write; this denylist is defense-in-depth underneath). +pub async fn hive_query_handler(Json(body): Json) -> Response { + execute_hive_query(body, DenylistMode::Enforced).await +} + +/// TEST-ONLY variant of [`hive_query_handler`] with the write-keyword denylist +/// disabled, so integration tests can prove that the database-level read-mode +/// transaction rejects writes on its own. This function is intentionally **not** +/// wired into the router — no HTTP route can reach it. +#[doc(hidden)] +pub async fn hive_query_handler_denylist_bypassed(Json(body): Json) -> Response { + execute_hive_query(body, DenylistMode::Bypassed).await +} + +async fn execute_hive_query(body: HiveQueryBody, denylist_mode: DenylistMode) -> Response { // Language validation — 400 (not 422) even when the field is missing. if body.language.as_deref() != Some("cypher") { return ( @@ -120,21 +169,20 @@ pub async fn hive_query_handler( "hive_query: received request" ); - // Write-keyword denylist (defense-in-depth — primary guard is read-mode transaction). - // Strip string literals first so values like `n.creator = 'MERGE request author'` - // do not cause false positives. - let query_no_literals = strip_string_literals(&body.query); - for (keyword, pattern) in WRITE_PATTERNS.iter() { - if pattern.is_match(&query_no_literals) { - tracing::warn!( - matched_keyword = keyword, - "hive_query: write keyword detected in query" - ); - return ( - StatusCode::FORBIDDEN, - Json(json!({"error": "write operations not permitted"})), - ) - .into_response(); + // Write-keyword denylist (defense-in-depth — the primary guard is the read-mode + // bolt transaction enforced by `execute_raw_cypher`). Strip string literals first + // so values like `n.creator = 'MERGE request author'` do not cause false positives. + // Bypassed only by the test-only entry point above. + if denylist_mode == DenylistMode::Enforced { + let query_no_literals = strip_string_literals(&body.query); + for (keyword, pattern) in WRITE_PATTERNS.iter() { + if pattern.is_match(&query_no_literals) { + tracing::warn!( + matched_keyword = keyword, + "hive_query: write keyword detected in query" + ); + return denylist_rejection_response(); + } } } @@ -159,6 +207,18 @@ pub async fn hive_query_handler( Json(json!(HiveQueryResponse { columns, rows })), ) .into_response(), + // The database itself refused a write inside the read-mode bolt transaction — + // the denylist had a hole and the DB guard saved us. Surface it as the same + // 403 body the denylist uses. Log the Neo4j error code and query length only; + // never the query body. + Err(shared::Error::ReadOnlyViolation(neo4j_code)) => { + tracing::warn!( + neo4j_error_code = %neo4j_code, + query_len = modified_query.len(), + "hive_query: database rejected a write attempt inside the read-mode transaction" + ); + read_only_violation_response() + } Err(e) => { tracing::error!(error = %e, "hive_query: Neo4j execution failed"); ( @@ -343,4 +403,53 @@ mod tests { let ok_query = "A".repeat(4096); assert!(ok_query.len() <= 4096); } + + // ── Read-mode transaction rejection (Error::ReadOnlyViolation → 403) ────── + + async fn response_parts(resp: Response) -> (axum::http::StatusCode, Vec) { + let status = resp.status(); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .expect("read response body") + .to_vec(); + (status, bytes) + } + + /// The database-level read-mode rejection must be indistinguishable on the + /// wire from the denylist rejection: same status, byte-identical JSON body. + /// The two literals are kept independent on purpose so this test fails if + /// either path drifts. + #[tokio::test] + async fn test_read_only_violation_body_matches_denylist_body() { + let (denylist_status, denylist_body) = + response_parts(denylist_rejection_response()).await; + let (db_status, db_body) = response_parts(read_only_violation_response()).await; + + assert_eq!(denylist_status, StatusCode::FORBIDDEN); + assert_eq!(db_status, StatusCode::FORBIDDEN); + assert_eq!( + denylist_body, db_body, + "ReadOnlyViolation body diverged from the denylist body" + ); + assert_eq!( + String::from_utf8(db_body).unwrap(), + r#"{"error":"write operations not permitted"}"# + ); + } + + /// `apoc.create.node` is caught by the denylist today (word boundaries exist + /// around `create` inside `apoc.create.node`), but write procedures whose + /// names contain no denylist keyword (e.g. `apoc.atomic.add`) are not — those + /// are exactly the queries the database-level read-mode guard must reject, + /// which the integration test exercises via + /// `hive_query_handler_denylist_bypassed`. + #[test] + fn test_denylist_catches_apoc_create_but_not_atomic_add() { + assert!(check_write_blocked( + "CALL apoc.create.node(['_ReadOnlyProbe'], {id: 'x'}) YIELD node RETURN node" + )); + assert!(!check_write_blocked( + "MATCH (n) CALL apoc.atomic.add(n, 'count', 1) YIELD oldValue RETURN oldValue" + )); + } } diff --git a/standalone/src/types.rs b/standalone/src/types.rs index 270174852..6bab035b3 100644 --- a/standalone/src/types.rs +++ b/standalone/src/types.rs @@ -261,7 +261,12 @@ impl IntoResponse for WebError { | shared::Error::Walkdir(_) | shared::Error::Other(_) | shared::Error::TreeSitterLanguage(_) => StatusCode::INTERNAL_SERVER_ERROR, - shared::Error::Custom(_) => StatusCode::INTERNAL_SERVER_ERROR, + // Note: the Hive query handler maps Error::ReadOnlyViolation to 403 itself; + // this generic wrapper never sees it because execute_raw_cypher's only + // caller is that handler. Default to 500 if it ever leaks here. + shared::Error::Custom(_) | shared::Error::ReadOnlyViolation(_) => { + StatusCode::INTERNAL_SERVER_ERROR + } }; tracing::error!("Handler error: {:?}", self.0); let resp = ErrorResponse { diff --git a/standalone/tests/hive_read_only.rs b/standalone/tests/hive_read_only.rs new file mode 100644 index 000000000..a320a7cba --- /dev/null +++ b/standalone/tests/hive_read_only.rs @@ -0,0 +1,273 @@ +//! Live-Neo4j integration tests for `POST /api/hive/query`. +//! +//! Proves that the read-mode bolt transaction (`GraphOps::execute_raw_cypher` → +//! `neo4rs::Graph::execute_read`, which sends `mode: "r"` autocommit metadata) +//! makes the **database itself** refuse writes — including write procedures +//! invoked via `CALL`, which the keyword denylist does not generally catch — +//! and that those rejections surface as HTTP 403 with the same body the +//! denylist uses, leaving no trace in the graph. +//! +//! The denylist is bypassed via the test-only entry point +//! `hive_query_handler_denylist_bypassed` (never routed), so every rejection +//! observed here comes from Neo4j, not from string matching. +//! +//! Every probe query is terminated with a `RETURN` clause: the handler appends +//! a server-controlled `LIMIT`, and a bare `CREATE .../ CALL ... LIMIT` is a +//! Cypher syntax error (which would fail for the wrong reason). +//! +//! Skips gracefully when Neo4j is unreachable (e.g. plain `cargo test` without +//! a database); run with `cargo test --features neo4j` against a live instance. + +#![cfg(feature = "neo4j")] + +use axum::Json; +use neo4rs::query; +use standalone::handlers::hive_query::{ + hive_query_handler, hive_query_handler_denylist_bypassed, HiveQueryBody, +}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; + +/// Label used for probe nodes; deleted before and after each run. +const PROBE_LABEL: &str = "_ReadOnlyProbe"; + +/// Env-configured direct (write-mode) bolt connection, mirroring +/// `Neo4jConfig::default()` (NEO4J_URI / NEO4J_USERNAME / NEO4J_PASSWORD / +/// NEO4J_DATABASE). +/// +/// Returns `None` (after printing a skip notice) when the server cannot be +/// reached, so the test skips gracefully instead of failing. +fn direct_connection() -> Option { + let uri = std::env::var("NEO4J_URI").unwrap_or_else(|_| "bolt://localhost:7687".to_string()); + let user = std::env::var("NEO4J_USERNAME").unwrap_or_else(|_| "neo4j".to_string()); + let pass = std::env::var("NEO4J_PASSWORD").unwrap_or_else(|_| "testtest".to_string()); + let db = std::env::var("NEO4J_DATABASE").unwrap_or_else(|_| "neo4j".to_string()); + let config = match neo4rs::ConfigBuilder::default() + .uri(uri) + .user(user) + .password(pass) + .db(db) + .build() + { + Ok(config) => config, + Err(e) => { + eprintln!("skipping hive read-only test: invalid Neo4j config ({e})"); + return None; + } + }; + // Building the pool is synchronous; the first query below is what actually + // dials the server, so reachability is checked separately. + Some(neo4rs::Graph::connect(config).expect("build neo4j pool")) +} + +/// Cheap reachability probe: `RETURN 1` under a timeout. The bolt pool is +/// built synchronously, but the first query is what dials the server. +async fn reachable(conn: &neo4rs::Graph) -> bool { + match tokio::time::timeout(Duration::from_secs(10), run_direct(conn, "RETURN 1")).await { + Ok(Ok(())) => true, + Ok(Err(e)) => { + eprintln!("skipping hive read-only test: Neo4j not reachable ({e})"); + false + } + Err(_) => { + eprintln!("skipping hive read-only test: Neo4j connection timed out"); + false + } + } +} + +/// Run a write-mode statement directly against Neo4j, draining the stream so +/// the query actually executes (neo4rs executes lazily on stream consumption). +async fn run_direct(conn: &neo4rs::Graph, cypher: &str) -> neo4rs::Result<()> { + let mut stream = conn.execute(query(cypher)).await?; + while stream.next().await?.is_some() {} + Ok(()) +} + +/// Count probe nodes with the given id, via the direct write-mode connection. +async fn count_probe(conn: &neo4rs::Graph, id: &str) -> i64 { + let mut stream = conn + .execute(query(&format!( + "MATCH (n:{label} {{id: $id}}) RETURN count(n) AS c", + label = PROBE_LABEL + )) + .param("id", id)) + .await + .expect("probe count query"); + match stream.next().await.expect("probe count row") { + Some(row) => row.get::("c").expect("count column"), + None => 0, + } +} + +fn body(query: impl Into) -> Json { + Json(HiveQueryBody { + language: Some("cypher".to_string()), + query: query.into(), + limit: None, + }) +} + +/// (status, parsed JSON body) of a handler response. +async fn status_and_json( + resp: axum::response::Response, +) -> (axum::http::StatusCode, serde_json::Value) { + let status = resp.status(); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .expect("read response body"); + (status, serde_json::from_slice(&bytes).expect("json body")) +} + +/// True when `apoc.create.node` is registered on the connected server. +async fn apoc_create_node_installed(conn: &neo4rs::Graph) -> bool { + let mut stream = match conn + .execute(query( + "SHOW PROCEDURES YIELD name WHERE name = 'apoc.create.node' RETURN count(*) AS c", + )) + .await + { + Ok(s) => s, + Err(_) => return false, + }; + match stream.next().await { + Ok(Some(row)) => row.get::("c").unwrap_or(0) > 0, + _ => false, + } +} + +/// Unique probe id per test run. +fn probe_id() -> String { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("clock") + .as_nanos(); + format!("ro-probe-{}", nanos) +} + +/// End-to-end: with the denylist bypassed, both a plain `CREATE` and a write +/// procedure invoked via `CALL` are refused by the database inside the +/// read-mode bolt transaction (403, denylist-identical body), and leave no +/// node behind. A direct write-mode connection first proves the database is +/// writable at all, so the absence assertions are not vacuous. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_hive_write_rejected_at_database_level() { + let conn = match direct_connection() { + Some(conn) => conn, + None => return, + }; + if !reachable(&conn).await { + return; + } + + let id = probe_id(); + let cleanup = format!("MATCH (n:{}) DETACH DELETE n", PROBE_LABEL); + run_direct(&conn, &cleanup).await.expect("cleanup before"); + + // Sanity: the database accepts writes through a normal (write-mode) + // connection. If it did not (e.g. a read-only instance), every 403 below + // would be trivially true and prove nothing. + run_direct( + &conn, + &format!( + "CREATE (n:{} {{id: '{}'}})", + PROBE_LABEL, id + ), + ) + .await + .expect("direct write-mode CREATE"); + assert_eq!(count_probe(&conn, &id).await, 1, "sanity write landed"); + run_direct(&conn, &cleanup).await.expect("cleanup sanity node"); + + // 1. Plain CREATE through the handler, denylist bypassed — the database + // must refuse it. (RETURN keeps the appended LIMIT valid.) + let resp = hive_query_handler_denylist_bypassed(body(format!( + "CREATE (n:{label} {{id: '{id}'}}) RETURN n.id", + label = PROBE_LABEL, + id = id + ))) + .await; + let (status, json) = status_and_json(resp).await; + assert_eq!(status, axum::http::StatusCode::FORBIDDEN, "json: {json}"); + assert_eq!( + json, + serde_json::json!({"error": "write operations not permitted"}) + ); + assert_eq!( + count_probe(&conn, &id).await, + 0, + "database must not have executed the CREATE" + ); + + // 2. Write procedure via CALL — the case the denylist's CALL allowance + // would normally leave to the transaction layer — also refused. + let resp = hive_query_handler_denylist_bypassed(body(format!( + "CALL apoc.create.node(['{label}'], {{id: '{id}'}}) YIELD node RETURN node.id", + label = PROBE_LABEL, + id = id + ))) + .await; + let (status, json) = status_and_json(resp).await; + if status == axum::http::StatusCode::INTERNAL_SERVER_ERROR { + // Distinguish "apoc not installed here" from a real failure. + assert!( + !apoc_create_node_installed(&conn).await, + "apoc.create.node is installed but the write-procedure call did not return 403: {json}" + ); + eprintln!("skipping apoc assertion: apoc.create.node not installed on this server"); + } else { + assert_eq!(status, axum::http::StatusCode::FORBIDDEN, "json: {json}"); + assert_eq!( + json, + serde_json::json!({"error": "write operations not permitted"}) + ); + } + assert_eq!( + count_probe(&conn, &id).await, + 0, + "write procedure must leave no trace in the graph" + ); + + run_direct(&conn, &cleanup).await.expect("cleanup after"); +} + +/// End-to-end: legitimate reads — plain `MATCH` and a read-only procedure +/// (`CALL db.labels()`) — still work through the full handler on the read-mode +/// transaction, and the production entry point still enforces the denylist. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_hive_reads_still_work() { + let conn = match direct_connection() { + Some(conn) => conn, + None => return, + }; + if !reachable(&conn).await { + return; + } + + // Plain MATCH returns 200 with columns/rows. + let (status, json) = + status_and_json(hive_query_handler(body("MATCH (n) RETURN n LIMIT 5")).await).await; + assert_eq!(status, axum::http::StatusCode::OK, "json: {json}"); + assert!(json.get("columns").is_some(), "expected columns: {json}"); + + // Read-only procedure returns 200 — CALL usage must not be collateral damage + // of the read-mode transaction. (The query uses the `YIELD ... RETURN` form + // because the handler appends a server-controlled LIMIT, and bare + // `CALL db.labels() LIMIT n` is a Cypher syntax error — pre-existing + // forced-LIMIT behavior, independent of read mode.) + let (status, json) = status_and_json( + hive_query_handler(body("CALL db.labels() YIELD label RETURN label")).await, + ) + .await; + assert_eq!(status, axum::http::StatusCode::OK, "json: {json}"); + assert!(json.get("rows").is_some(), "expected rows: {json}"); + + // Production entry point still enforces the denylist (CREATE → 403 from the + // keyword check, before the query ever reaches Neo4j). + let (status, json) = + status_and_json(hive_query_handler(body("CREATE (n:Denied)")).await).await; + assert_eq!(status, axum::http::StatusCode::FORBIDDEN, "json: {json}"); + assert_eq!( + json, + serde_json::json!({"error": "write operations not permitted"}) + ); +}