From c5a6a7125af1c1d95da953e6a35917582995da21 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Thu, 28 May 2026 11:20:34 +0200 Subject: [PATCH] mr-668: regression test for GraphList open-mode bypass (red) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursor bot's review at commit 4120448 surfaced that `server_graphs_list` returns 200 in Open mode (`--unauthenticated`, no tokens, no policy), exposing the full graph registry — graph IDs and URIs that may contain S3 bucket paths or internal hostnames — to any unauthenticated caller. Root cause: `authorize_request`'s no-policy fallback only denies when `actor.is_some()`. In Open mode `actor: None`, so the denial branch never fires and the call returns `Ok(())`. The docstring on `server_graphs_list` claims the endpoint is "Cedar-gated" and that we "don't leak the registry until the operator explicitly authorizes it" — but Open mode has no Cedar at all, so the docstring intent and the code disagree. This commit renames the existing `get_graphs_lists_registered_graphs_in_multi_mode` test to `get_graphs_denied_in_open_mode_without_server_policy` and flips the assertion from 200 → 403. Today this fails (server returns 200) — exactly the symptom the bot named. The fix in the next commit tightens the no-policy fallback to deny server-scoped actions unconditionally, regardless of mode. Per AGENTS.md rule 12, the red test commit lands just before the fix so the red → green pair is visible in `git log` and a reviewer can check out this commit alone to reproduce. Sort-order coverage that previously lived in the renamed test moves to `get_graphs_with_server_policy_authorizes_per_cedar` in the next commit, where the admin-200 response is operator- authorized and a non-empty body is asserted. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/omnigraph-server/tests/server.rs | 40 ++++++++++++++++++------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/crates/omnigraph-server/tests/server.rs b/crates/omnigraph-server/tests/server.rs index 95c9537..73223b3 100644 --- a/crates/omnigraph-server/tests/server.rs +++ b/crates/omnigraph-server/tests/server.rs @@ -4915,10 +4915,28 @@ graphs: } } - /// `GET /graphs` lists the registered graphs alphabetically in - /// multi mode. No auth or server policy = open mode (allowed). + /// `GET /graphs` must NOT leak the registry in Open mode without + /// an explicit server policy. Operators who pass `--unauthenticated` + /// opted into trusting the network for graph DATA, not for leaking + /// server topology (graph IDs + URIs, which may contain S3 bucket + /// paths or internal hostnames). Cedar gating the management + /// surface is the documented contract for `server_graphs_list` + /// ("don't leak the registry until the operator explicitly + /// authorizes it"); enforcing that contract in every runtime + /// state — not just `PolicyEnabled` — is the correct-by-design + /// closure of the open-mode hole the bot-review pass surfaced. + /// + /// Today (pre-fix) this returns 200 because `authorize_request`'s + /// no-policy fallback only denies when `actor.is_some()`, so Open + /// mode (`actor: None`) falls through to `Ok(())`. The fix in the + /// next commit tightens the fallback so server-scoped actions + /// always require explicit policy. + /// + /// Sort-order coverage previously lived here; it has moved to + /// `get_graphs_with_server_policy_authorizes_per_cedar` where + /// the response body is now non-empty and operator-authorized. #[tokio::test(flavor = "multi_thread")] - async fn get_graphs_lists_registered_graphs_in_multi_mode() { + async fn get_graphs_denied_in_open_mode_without_server_policy() { let (_dirs, app) = build_multi_mode_app(&["beta", "alpha"]).await; let resp = app .oneshot( @@ -4930,14 +4948,16 @@ graphs: ) .await .unwrap(); - assert_eq!(resp.status(), StatusCode::OK); + let status = resp.status(); let body = to_bytes(resp.into_body(), usize::MAX).await.unwrap(); - let json: Value = serde_json::from_slice(&body).unwrap(); - let graphs = json["graphs"].as_array().unwrap(); - assert_eq!(graphs.len(), 2); - // Server-sorted alphabetically. - assert_eq!(graphs[0]["graph_id"].as_str().unwrap(), "alpha"); - assert_eq!(graphs[1]["graph_id"].as_str().unwrap(), "beta"); + let body_str = String::from_utf8_lossy(&body); + assert_eq!( + status, + StatusCode::FORBIDDEN, + "GET /graphs must require an explicit server policy in every \ + runtime state; Open-mode bypass would leak server topology. \ + Body: {body_str}", + ); } /// `GET /graphs` returns 405 in single mode (resource exists in the