diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index dc9e2a8..80eeb9c 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -50,7 +50,7 @@ use subtle::ConstantTimeEq; use tokio::net::TcpListener; use tokio::sync::mpsc; use tower_http::trace::TraceLayer; -use tracing::{error, info}; +use tracing::{error, info, warn}; use tracing_subscriber::EnvFilter; use utoipa::OpenApi; use utoipa::openapi::security::{Http, HttpAuthScheme, SecurityScheme}; @@ -114,6 +114,15 @@ pub struct ServerConfig { pub uri: String, pub bind: String, pub policy_file: Option, + /// Operator opt-in for fully-unauthenticated dev mode (MR-723). + /// When neither bearer tokens nor a policy file are configured, + /// `serve()` refuses to start unless this is true (set via + /// `--unauthenticated` or `OMNIGRAPH_UNAUTHENTICATED=1`). The + /// motivation is that "no tokens + no policy" looks like protection + /// (no Cedar errors at boot) but is actually fully open — operators + /// who set up auth and forgot the policy file would otherwise ship + /// the illusion of protection. + pub allow_unauthenticated: bool, } #[derive(Clone)] @@ -246,6 +255,17 @@ impl AppState { } } + /// Install a `PolicyEngine` post-construction (MR-723). Used by + /// integration tests that need to thread custom workload limits + /// alongside a permit-all policy — the existing `new_with_*` and + /// `new_with_workload` constructors don't compose. Production + /// callers should use `open_with_bearer_tokens_and_policy` which + /// installs the policy on both the HTTP state and the engine. + pub fn with_policy_engine(mut self, engine: PolicyEngine) -> Self { + self.policy_engine = Some(Arc::new(engine)); + self + } + pub async fn open(uri: impl Into) -> Result { Self::open_with_bearer_token(uri, None).await } @@ -535,20 +555,77 @@ pub fn load_server_settings( cli_uri: Option, cli_target: Option, cli_bind: Option, + cli_allow_unauthenticated: bool, ) -> Result { let config = load_config(config_path)?; let uri = config.resolve_target_uri(cli_uri, cli_target.as_deref(), config.server_graph_name())?; let bind = cli_bind.unwrap_or_else(|| config.server_bind().to_string()); let policy_file = config.resolve_policy_file(); + // Either `--unauthenticated` or `OMNIGRAPH_UNAUTHENTICATED=1` flips + // this. Treat any non-empty, non-"0"/"false" string as truthy — + // standard 12-factor "any value is true" reading of the env var. + let env_unauth = std::env::var("OMNIGRAPH_UNAUTHENTICATED") + .ok() + .map(|v| { + let trimmed = v.trim(); + !trimmed.is_empty() && trimmed != "0" && !trimmed.eq_ignore_ascii_case("false") + }) + .unwrap_or(false); + let allow_unauthenticated = cli_allow_unauthenticated || env_unauth; Ok(ServerConfig { uri, bind, policy_file, + allow_unauthenticated, }) } +/// MR-723 server runtime state, classified from the three-state matrix +/// of (bearer tokens configured) × (policy file configured) at startup. +/// +/// * **Open** — neither tokens nor policy; requires explicit +/// `allow_unauthenticated`. Effectively a "trust the network" dev +/// mode. `serve()` refuses to start in this shape without the flag, +/// so the only way to reach this state at runtime is via deliberate +/// operator opt-in. +/// * **DefaultDeny** — tokens configured but no policy file. The +/// server requires a valid bearer token; once authenticated, every +/// action except `Read` is denied with 403. Closes the "tokens but +/// forgot the policy file" trap. +/// * **PolicyEnabled** — policy file configured. Cedar evaluates every +/// authenticated request. Tokens may also be configured (typical) or +/// not (unusual but valid — every request fails 401 without a +/// bearer, which is effectively "locked"). +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub enum ServerRuntimeState { + Open, + DefaultDeny, + PolicyEnabled, +} + +/// Compute the [`ServerRuntimeState`] from the configured inputs. +/// Pulled out as a pure function so the 3-state matrix is unit-testable +/// without standing up the full server. +pub fn classify_server_runtime_state( + has_tokens: bool, + has_policy: bool, + allow_unauthenticated: bool, +) -> Result { + match (has_tokens, has_policy, allow_unauthenticated) { + (false, false, false) => bail!( + "server has no bearer tokens and no policy file configured. This is a fully \ + open server — pass `--unauthenticated` (or set OMNIGRAPH_UNAUTHENTICATED=1) \ + if you actually want that, otherwise configure bearer tokens (see \ + docs/user/server.md) and/or `policy.file` in omnigraph.yaml." + ), + (false, false, true) => Ok(ServerRuntimeState::Open), + (true, false, _) => Ok(ServerRuntimeState::DefaultDeny), + (_, true, _) => Ok(ServerRuntimeState::PolicyEnabled), + } +} + pub fn build_app(state: AppState) -> Router { let protected = Router::new() .route("/snapshot", get(server_snapshot)) @@ -586,9 +663,28 @@ pub fn build_app(state: AppState) -> Router { pub async fn serve(config: ServerConfig) -> Result<()> { let token_source = resolve_token_source().await?; info!(source = token_source.name(), "loaded bearer token source"); + let tokens = token_source.load().await?; + let runtime_state = classify_server_runtime_state( + !tokens.is_empty(), + config.policy_file.is_some(), + config.allow_unauthenticated, + )?; + match runtime_state { + ServerRuntimeState::Open => warn!( + "running with --unauthenticated: no bearer tokens, no policy file, all \ + requests permitted. This is for local dev only — do not expose to a \ + network you don't fully trust." + ), + ServerRuntimeState::DefaultDeny => warn!( + "bearer tokens are configured but no policy file is set — running in \ + default-deny mode (only `read` actions are permitted for authenticated \ + actors). Configure `policy.file` in omnigraph.yaml to enable Cedar rules." + ), + ServerRuntimeState::PolicyEnabled => {} + } let state = AppState::open_with_bearer_tokens_and_policy( config.uri.clone(), - token_source.load().await?, + tokens, config.policy_file.as_ref(), ) .await?; @@ -708,6 +804,27 @@ fn authorize_request( mut request: PolicyRequest, ) -> std::result::Result<(), ApiError> { let Some(engine) = state.policy_engine() else { + // MR-723 default-deny path. We're here when no PolicyEngine is + // installed. Two startup-validated shapes can reach this: + // + // * **Open mode** (`--unauthenticated`): no tokens, no policy. + // `require_bearer_auth` short-circuits before this is called, + // but defense in depth — if a future change makes the + // middleware call here for an unauthenticated request, we + // want every action to remain Ok rather than 403. The + // operator opted in. + // * **DefaultDeny mode**: tokens configured but no policy. The + // request went through bearer auth, so `actor` is Some and + // identifies a known actor. Only `Read` is permitted; every + // other action returns 403. This closes the "configured auth + // but forgot the policy file" trap from MR-723. + if actor.is_some() && request.action != PolicyAction::Read { + return Err(ApiError::forbidden( + "server runs in default-deny mode (bearer tokens configured but no \ + policy file). Only `read` actions are permitted; configure \ + `policy.file` in omnigraph.yaml to enable other actions.", + )); + } return Ok(()); }; let Some(actor) = actor else { @@ -1641,8 +1758,8 @@ fn server_bearer_tokens_from_env() -> Result> { #[cfg(test)] mod tests { use super::{ - hash_bearer_token, load_server_settings, normalize_bearer_token, parse_bearer_tokens_json, - server_bearer_tokens_from_env, + ServerRuntimeState, classify_server_runtime_state, hash_bearer_token, load_server_settings, + normalize_bearer_token, parse_bearer_tokens_json, server_bearer_tokens_from_env, }; use std::env; use std::fs; @@ -1695,7 +1812,7 @@ server: ) .unwrap(); - let settings = load_server_settings(Some(&config), None, None, None).unwrap(); + let settings = load_server_settings(Some(&config), None, None, None, false).unwrap(); assert_eq!(settings.uri, "/tmp/demo.omni"); assert_eq!(settings.bind, "0.0.0.0:9090"); } @@ -1722,6 +1839,7 @@ server: Some("/tmp/override.omni".to_string()), None, Some("0.0.0.0:9999".to_string()), + false, ) .unwrap(); assert_eq!(settings.uri, "/tmp/override.omni"); @@ -1748,16 +1866,69 @@ server: .unwrap(); let settings = - load_server_settings(Some(&config), None, Some("dev".to_string()), None).unwrap(); + load_server_settings(Some(&config), None, Some("dev".to_string()), None, false) + .unwrap(); assert_eq!(settings.uri, "http://127.0.0.1:8080"); } #[test] fn server_settings_require_uri_from_cli_or_config() { - let error = load_server_settings(None, None, None, None).unwrap_err(); + let error = load_server_settings(None, None, None, None, false).unwrap_err(); assert!(error.to_string().contains("URI must be provided")); } + #[test] + fn classify_open_requires_explicit_unauthenticated_flag() { + // State 1: no tokens, no policy, no flag → refuse to start. + let error = classify_server_runtime_state(false, false, false).unwrap_err(); + let msg = error.to_string(); + assert!( + msg.contains("--unauthenticated"), + "expected refusal message mentioning --unauthenticated, got: {msg}" + ); + + // Same matrix cell but with the flag set → Open mode permitted. + assert_eq!( + classify_server_runtime_state(false, false, true).unwrap(), + ServerRuntimeState::Open + ); + } + + #[test] + fn classify_tokens_without_policy_is_default_deny() { + // State 2: tokens configured, no policy → DefaultDeny regardless + // of the flag (the flag opts into the fully-open dev mode; it + // doesn't downgrade default-deny back to open). + assert_eq!( + classify_server_runtime_state(true, false, false).unwrap(), + ServerRuntimeState::DefaultDeny + ); + assert_eq!( + classify_server_runtime_state(true, false, true).unwrap(), + ServerRuntimeState::DefaultDeny + ); + } + + #[test] + fn classify_policy_enabled_always_wins() { + // State 3: any setup with a policy file → PolicyEnabled. The + // flag doesn't matter and tokens-or-not doesn't matter (no + // tokens + policy is unusual but valid — every request fails + // 401 without a bearer, which is effectively "locked"). + assert_eq!( + classify_server_runtime_state(true, true, false).unwrap(), + ServerRuntimeState::PolicyEnabled + ); + assert_eq!( + classify_server_runtime_state(false, true, false).unwrap(), + ServerRuntimeState::PolicyEnabled + ); + assert_eq!( + classify_server_runtime_state(true, true, true).unwrap(), + ServerRuntimeState::PolicyEnabled + ); + } + #[test] fn normalize_bearer_token_trims_and_filters_blank_values() { assert_eq!(normalize_bearer_token(None), None); diff --git a/crates/omnigraph-server/src/main.rs b/crates/omnigraph-server/src/main.rs index 0b43105..54af1ed 100644 --- a/crates/omnigraph-server/src/main.rs +++ b/crates/omnigraph-server/src/main.rs @@ -16,6 +16,12 @@ struct Cli { config: Option, #[arg(long)] bind: Option, + /// Run without bearer tokens and without a policy file (MR-723). + /// Required when neither is configured — otherwise the server + /// refuses to start to prevent shipping the illusion of protection. + /// Equivalent to setting `OMNIGRAPH_UNAUTHENTICATED=1`. + #[arg(long)] + unauthenticated: bool, } #[tokio::main] @@ -24,7 +30,12 @@ async fn main() -> Result<()> { init_tracing(); let cli = Cli::parse(); - let settings: ServerConfig = - load_server_settings(cli.config.as_ref(), cli.uri, cli.target, cli.bind)?; + let settings: ServerConfig = load_server_settings( + cli.config.as_ref(), + cli.uri, + cli.target, + cli.bind, + cli.unauthenticated, + )?; serve(settings).await } diff --git a/crates/omnigraph-server/tests/server.rs b/crates/omnigraph-server/tests/server.rs index 998da23..1688765 100644 --- a/crates/omnigraph-server/tests/server.rs +++ b/crates/omnigraph-server/tests/server.rs @@ -5,6 +5,7 @@ use std::sync::Arc; use axum::Router; use axum::body::{Body, to_bytes}; +use axum::http::header::AUTHORIZATION; use axum::http::{Method, Request, StatusCode}; use lance_index::traits::DatasetIndexExt; use omnigraph::db::{Omnigraph, ReadTarget}; @@ -176,15 +177,55 @@ async fn app_for_loaded_repo() -> (tempfile::TempDir, Router) { (temp, build_app(state)) } +/// Build a permit-all policy YAML that grants every action used by the +/// HTTP-layer tests to the listed actor names. MR-723 default-deny +/// closed the "tokens but no policy" loophole; helpers that used to +/// represent "auth without policy" now install this permit-all policy +/// so test cases retain their pre-MR-723 semantics ("auth required, +/// every action permitted") without conflicting with the new state +/// matrix. Tests that specifically need the State-2 deny path use +/// `app_for_repo_with_auth_tokens_only` instead. +fn permit_all_policy_yaml(actors: &[&str]) -> String { + let members = actors + .iter() + .map(|a| format!("\"{a}\"")) + .collect::>() + .join(", "); + format!( + r#" +version: 1 +groups: + permitted: [{members}] +protected_branches: [main] +rules: + - id: permit-data + allow: + actors: {{ group: permitted }} + actions: [read, change, export] + branch_scope: any + - id: permit-protected-target-actions + allow: + actors: {{ group: permitted }} + actions: [schema_apply, branch_create, branch_delete, branch_merge] + target_branch_scope: any +"# + ) +} + async fn app_for_loaded_repo_with_auth(token: &str) -> (tempfile::TempDir, Router) { + // `AppState::new_with_bearer_token(token)` maps the token to actor "default"; + // permit-all policy needs to include that actor. let temp = init_loaded_repo().await; let repo = repo_path(temp.path()); - let db = Omnigraph::open(repo.to_str().unwrap()).await.unwrap(); - let state = AppState::new_with_bearer_token( + let policy_path = temp.path().join("policy.yaml"); + fs::write(&policy_path, permit_all_policy_yaml(&["default"])).unwrap(); + let state = AppState::open_with_bearer_tokens_and_policy( repo.to_string_lossy().to_string(), - db, - Some(token.to_string()), - ); + vec![("default".to_string(), token.to_string())], + Some(&policy_path), + ) + .await + .unwrap(); (temp, build_app(state)) } @@ -193,15 +234,19 @@ async fn app_for_loaded_repo_with_auth_tokens( ) -> (tempfile::TempDir, Router) { let temp = init_loaded_repo().await; let repo = repo_path(temp.path()); - let db = Omnigraph::open(repo.to_str().unwrap()).await.unwrap(); - let state = AppState::new_with_bearer_tokens( + let policy_path = temp.path().join("policy.yaml"); + let actors: Vec<&str> = tokens.iter().map(|(actor, _)| *actor).collect(); + fs::write(&policy_path, permit_all_policy_yaml(&actors)).unwrap(); + let state = AppState::open_with_bearer_tokens_and_policy( repo.to_string_lossy().to_string(), - db, tokens .iter() .map(|(actor, token)| ((*actor).to_string(), (*token).to_string())) .collect(), - ); + Some(&policy_path), + ) + .await + .unwrap(); (temp, build_app(state)) } @@ -248,6 +293,29 @@ async fn app_for_repo_with_auth_tokens_and_policy( (temp, build_app(state)) } +/// MR-723 default-deny mode: bearer tokens configured, no policy file. +/// Exercises ServerRuntimeState::DefaultDeny — authenticated requests +/// for Read succeed, every other action is rejected with 403 from +/// `authorize_request`'s state-2 branch. +async fn app_for_repo_with_auth_tokens_only( + schema: &str, + tokens: &[(&str, &str)], +) -> (tempfile::TempDir, Router) { + let temp = init_repo_with_schema(schema).await; + let repo = repo_path(temp.path()); + let state = AppState::open_with_bearer_tokens_and_policy( + repo.to_string_lossy().to_string(), + tokens + .iter() + .map(|(actor, token)| ((*actor).to_string(), (*token).to_string())) + .collect(), + None, + ) + .await + .unwrap(); + (temp, build_app(state)) +} + fn additive_schema_with_nickname() -> String { fs::read_to_string(fixture("test.pg")).unwrap().replace( " age: I32?\n}", @@ -831,11 +899,20 @@ async fn export_route_returns_jsonl_for_branch_snapshot() { .unwrap(); drop(db); - let state = AppState::new_with_bearer_token( + // MR-723: tokens-without-policy is now default-deny. Install a + // permit-all policy alongside the bearer token so /export + // (action=Export) passes Cedar evaluation. The test is exercising + // export semantics, not policy — the policy is just enough to clear + // the State 3 path. + let policy_path = temp.path().join("policy.yaml"); + fs::write(&policy_path, permit_all_policy_yaml(&["default"])).unwrap(); + let state = AppState::open_with_bearer_tokens_and_policy( repo.to_string_lossy().to_string(), - Omnigraph::open(repo.to_str().unwrap()).await.unwrap(), - Some(token.to_string()), - ); + vec![("default".to_string(), token.to_string())], + Some(&policy_path), + ) + .await + .unwrap(); let app = build_app(state); let response = app @@ -3487,12 +3564,24 @@ async fn ingest_per_actor_admission_cap_returns_429() { 1, // per-actor in-flight cap (the fixture under test) 1_000_000_000, // per-actor byte budget — large so it never bottlenecks ); + // MR-723: install a permit-all policy alongside the bearer token so + // /ingest (action=Change) passes Cedar evaluation. The test is + // exercising the admission cap, not policy — the policy is just + // enough to clear the State 3 path so the test reaches workload. + let policy_path = temp.path().join("policy.yaml"); + fs::write(&policy_path, permit_all_policy_yaml(&["act-flooder"])).unwrap(); + let policy_engine = omnigraph_server::PolicyEngine::load( + &policy_path, + repo.to_string_lossy().as_ref(), + ) + .unwrap(); let state = AppState::new_with_workload( repo.to_string_lossy().to_string(), db, vec![("act-flooder".to_string(), "flooder-token".to_string())], workload, - ); + ) + .with_policy_engine(policy_engine); let app = build_app(state); let _temp = temp; @@ -3604,3 +3693,97 @@ async fn oversized_request_body_returns_payload_too_large() { assert_eq!(response.status(), StatusCode::PAYLOAD_TOO_LARGE); } + +// ─── MR-723 default-deny mode (State 2: tokens without policy) ────────── +// +// `authorize_request` returns 403 for every action except `Read` when a +// PolicyEngine is not installed but bearer tokens are configured. Pinned +// by the three tests below — Read allowed, Change/SchemaApply denied — +// to prevent regressing back to the pre-MR-723 "tokens configured but +// no policy = fully open" trap. + +#[tokio::test(flavor = "multi_thread")] +async fn default_deny_mode_allows_read_for_authenticated_actor() { + let (_temp, app) = app_for_repo_with_auth_tokens_only( + &fs::read_to_string(fixture("test.pg")).unwrap(), + &[("act-andrew", "demo-token")], + ) + .await; + + let (status, _body) = json_response( + &app, + Request::builder() + .uri("/snapshot") + .method(Method::GET) + .header(AUTHORIZATION, "Bearer demo-token") + .body(Body::empty()) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::OK); +} + +#[tokio::test(flavor = "multi_thread")] +async fn default_deny_mode_rejects_change_with_forbidden() { + let (_temp, app) = app_for_repo_with_auth_tokens_only( + &fs::read_to_string(fixture("test.pg")).unwrap(), + &[("act-andrew", "demo-token")], + ) + .await; + + let change = ChangeRequest { + query_source: MUTATION_QUERIES.to_string(), + query_name: Some("insert_person".to_string()), + params: Some(json!({ "name": "DefaultDeny", "age": 1 })), + branch: Some("main".to_string()), + }; + let (status, body) = json_response( + &app, + Request::builder() + .uri("/change") + .method(Method::POST) + .header(AUTHORIZATION, "Bearer demo-token") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&change).unwrap())) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::FORBIDDEN); + let error: ErrorOutput = serde_json::from_value(body).unwrap(); + assert!( + error.error.contains("default-deny"), + "expected default-deny in error message, got: {}", + error.error + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn default_deny_mode_rejects_schema_apply_with_forbidden() { + let (_temp, app) = app_for_repo_with_auth_tokens_only( + &fs::read_to_string(fixture("test.pg")).unwrap(), + &[("act-andrew", "demo-token")], + ) + .await; + + let req = SchemaApplyRequest { + schema_source: additive_schema_with_nickname(), + }; + let (status, body) = json_response( + &app, + Request::builder() + .uri("/schema/apply") + .method(Method::POST) + .header(AUTHORIZATION, "Bearer demo-token") + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&req).unwrap())) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::FORBIDDEN); + let error: ErrorOutput = serde_json::from_value(body).unwrap(); + assert!( + error.error.contains("default-deny"), + "expected default-deny in error message, got: {}", + error.error + ); +} diff --git a/docs/user/policy.md b/docs/user/policy.md index 8550fb8..b121213 100644 --- a/docs/user/policy.md +++ b/docs/user/policy.md @@ -63,6 +63,25 @@ is a strict no-op; when one is installed and the call site forgets to thread an actor through, the gate fails closed rather than silently bypassing. +## Server runtime states (MR-723) + +The HTTP server classifies its startup configuration into one of three +states based on whether bearer tokens are configured and whether a +policy file is set. The state determines what happens to a request that +reaches `authorize_request()` without a matching policy permit. + +| State | Tokens | Policy file | Behavior | +|---|---|---|---| +| **Open** | no | no | Every request is permitted. Refuses to start unless `--unauthenticated` or `OMNIGRAPH_UNAUTHENTICATED=1` is set — the operator must explicitly opt in. | +| **DefaultDeny** | yes | no | Every authenticated request for an action other than `read` is rejected with HTTP 403. Closes the "tokens but forgot the policy file" trap — an operator who sets up auth and forgot to point at a policy file used to ship the illusion of protection. | +| **PolicyEnabled** | any | yes | Every request is evaluated by Cedar against the configured policy. | + +The classifier is `classify_server_runtime_state` in +`crates/omnigraph-server/src/lib.rs`; it returns `Err` for the "no +tokens, no policy, no flag" cell so the server refuses to start instead +of silently shipping an open instance. Tests pin every cell of the +matrix and the State-2 deny path. + Server-side, `authorize_request()` still runs at the HTTP boundary — that's where actor identity is resolved from the bearer token and where admission control / per-actor rate limits live. Engine-layer enforcement