diff --git a/crates/omnigraph-cli/tests/system_local.rs b/crates/omnigraph-cli/tests/system_local.rs index fd9441d..3d3e9bf 100644 --- a/crates/omnigraph-cli/tests/system_local.rs +++ b/crates/omnigraph-cli/tests/system_local.rs @@ -35,6 +35,16 @@ rules: actors: { group: admins } actions: [change] branch_scope: any + - id: admins-branch-ops + allow: + actors: { group: admins } + actions: [branch_create, branch_delete] + target_branch_scope: any + - id: admins-schema-apply + allow: + actors: { group: admins } + actions: [schema_apply] + target_branch_scope: any "#; const POLICY_E2E_TESTS_YAML: &str = r#" @@ -1090,3 +1100,389 @@ fn local_cli_change_enforces_engine_layer_policy() { assert_eq!(verify["row_count"], 1); assert_eq!(verify["rows"][0]["p.name"], "RagnorOnMain"); } + +// ─── MR-722 PR A: CLI×writer matrix ─────────────────────────────────────── +// +// The change writer is covered above by `local_cli_change_enforces_engine_layer_policy`. +// These tests extend the engine-layer-policy assertion to the other 6 +// writers, asserting each `omnigraph --as ` invocation +// reaches the corresponding `_as` method and Cedar evaluates correctly. +// One denied case (`--as act-bruno`) + one allowed case (`--as act-ragnor` +// via the `admins-*` rules) per writer; the no-actor footgun is already +// proved by the change-writer test and applies identically to every +// other `_as` variant. + +#[test] +fn local_cli_load_enforces_engine_layer_policy() { + let repo = SystemRepo::loaded(); + let config = repo.write_config("omnigraph-policy.yaml", &local_policy_config(&repo)); + repo.write_config("policy.yaml", POLICY_E2E_YAML); + let data = repo.write_jsonl( + "system-local-policy-load.jsonl", + r#"{"type":"Person","data":{"name":"LoadPolicy","age":11}}"#, + ); + + // act-bruno: change-on-protected is denied (team-write-unprotected only). + let denied = output_failure( + cli() + .arg("--as") + .arg("act-bruno") + .arg("load") + .arg("--config") + .arg(&config) + .arg("--data") + .arg(&data) + .arg("--json"), + ); + let stderr = String::from_utf8_lossy(&denied.stderr); + assert!( + stderr.contains("denied"), + "expected 'denied' for bruno/main load, got: {stderr}" + ); + + // act-ragnor: admins-write rule permits change anywhere. + let allowed = parse_stdout_json(&output_success( + cli() + .arg("--as") + .arg("act-ragnor") + .arg("load") + .arg("--config") + .arg(&config) + .arg("--data") + .arg(&data) + .arg("--json"), + )); + assert_eq!(allowed["branch"], "main"); + assert!(allowed["nodes_loaded"].as_u64().unwrap() >= 1); +} + +#[test] +fn local_cli_ingest_enforces_engine_layer_policy() { + let repo = SystemRepo::loaded(); + let config = repo.write_config("omnigraph-policy.yaml", &local_policy_config(&repo)); + repo.write_config("policy.yaml", POLICY_E2E_YAML); + let data = repo.write_jsonl( + "system-local-policy-ingest.jsonl", + r#"{"type":"Person","data":{"name":"IngestPolicy","age":12}}"#, + ); + + // act-bruno: ingest into a new branch requires both BranchCreate and + // Change. Bruno has change-unprotected only, and the implicit + // branch_create fires first when the target branch doesn't exist. + // Either gate is enough to deny — assert denial without pinning + // which one fires first. + let denied = output_failure( + cli() + .arg("--as") + .arg("act-bruno") + .arg("ingest") + .arg("--config") + .arg(&config) + .arg("--data") + .arg(&data) + .arg("--branch") + .arg("policy-ingest-feature") + .arg("--json"), + ); + let stderr = String::from_utf8_lossy(&denied.stderr); + assert!( + stderr.contains("denied"), + "expected 'denied' for bruno ingest, got: {stderr}" + ); + + // act-ragnor: admins-write covers Change, admins-branch-ops covers + // BranchCreate. Both fire as ingest creates the branch + loads. + let allowed = parse_stdout_json(&output_success( + cli() + .arg("--as") + .arg("act-ragnor") + .arg("ingest") + .arg("--config") + .arg(&config) + .arg("--data") + .arg(&data) + .arg("--branch") + .arg("policy-ingest-feature") + .arg("--json"), + )); + assert_eq!(allowed["branch"], "policy-ingest-feature"); + assert_eq!(allowed["branch_created"], true); +} + +#[test] +fn local_cli_schema_apply_enforces_engine_layer_policy() { + let repo = SystemRepo::loaded(); + let config = repo.write_config("omnigraph-policy.yaml", &local_policy_config(&repo)); + repo.write_config("policy.yaml", POLICY_E2E_YAML); + + // Additive: add a nullable property; SDK-compatible with the fixture + // schema. Uses the schema-apply scope (TargetBranch("main")). + let new_schema = std::fs::read_to_string(fixture("test.pg")) + .unwrap() + .replace(" age: I32?\n}", " age: I32?\n nickname: String?\n}"); + let schema_path = repo.path().join("policy-additive.pg"); + std::fs::write(&schema_path, &new_schema).unwrap(); + + let denied = output_failure( + cli() + .arg("--as") + .arg("act-bruno") + .arg("schema") + .arg("apply") + .arg("--config") + .arg(&config) + .arg("--schema") + .arg(&schema_path) + .arg("--json"), + ); + let stderr = String::from_utf8_lossy(&denied.stderr); + assert!( + stderr.contains("denied"), + "expected 'denied' for bruno schema apply, got: {stderr}" + ); + + let allowed = parse_stdout_json(&output_success( + cli() + .arg("--as") + .arg("act-ragnor") + .arg("schema") + .arg("apply") + .arg("--config") + .arg(&config) + .arg("--schema") + .arg(&schema_path) + .arg("--json"), + )); + assert_eq!(allowed["applied"], true); +} + +#[test] +fn local_cli_branch_create_enforces_engine_layer_policy() { + let repo = SystemRepo::loaded(); + let config = repo.write_config("omnigraph-policy.yaml", &local_policy_config(&repo)); + repo.write_config("policy.yaml", POLICY_E2E_YAML); + + let denied = output_failure( + cli() + .arg("--as") + .arg("act-bruno") + .arg("branch") + .arg("create") + .arg("--config") + .arg(&config) + .arg("--from") + .arg("main") + .arg("bruno-feature"), + ); + let stderr = String::from_utf8_lossy(&denied.stderr); + assert!( + stderr.contains("denied"), + "expected 'denied' for bruno branch create, got: {stderr}" + ); + + output_success( + cli() + .arg("--as") + .arg("act-ragnor") + .arg("branch") + .arg("create") + .arg("--config") + .arg(&config) + .arg("--from") + .arg("main") + .arg("ragnor-feature"), + ); +} + +#[test] +fn local_cli_branch_delete_enforces_engine_layer_policy() { + let repo = SystemRepo::loaded(); + let config = repo.write_config("omnigraph-policy.yaml", &local_policy_config(&repo)); + repo.write_config("policy.yaml", POLICY_E2E_YAML); + + // Pre-create the branch as ragnor so there's something to delete. + output_success( + cli() + .arg("--as") + .arg("act-ragnor") + .arg("branch") + .arg("create") + .arg("--config") + .arg(&config) + .arg("--from") + .arg("main") + .arg("doomed"), + ); + + let denied = output_failure( + cli() + .arg("--as") + .arg("act-bruno") + .arg("branch") + .arg("delete") + .arg("--config") + .arg(&config) + .arg("doomed"), + ); + let stderr = String::from_utf8_lossy(&denied.stderr); + assert!( + stderr.contains("denied"), + "expected 'denied' for bruno branch delete, got: {stderr}" + ); + + output_success( + cli() + .arg("--as") + .arg("act-ragnor") + .arg("branch") + .arg("delete") + .arg("--config") + .arg(&config) + .arg("doomed"), + ); +} + +#[test] +fn local_cli_branch_merge_enforces_engine_layer_policy() { + let repo = SystemRepo::loaded(); + let config = repo.write_config("omnigraph-policy.yaml", &local_policy_config(&repo)); + repo.write_config("policy.yaml", POLICY_E2E_YAML); + + // Pre-create a feature branch as ragnor (admins-branch-ops covers it). + output_success( + cli() + .arg("--as") + .arg("act-ragnor") + .arg("branch") + .arg("create") + .arg("--config") + .arg(&config) + .arg("--from") + .arg("main") + .arg("merge-feature"), + ); + + let denied = output_failure( + cli() + .arg("--as") + .arg("act-bruno") + .arg("branch") + .arg("merge") + .arg("--config") + .arg(&config) + .arg("merge-feature") + .arg("--into") + .arg("main"), + ); + let stderr = String::from_utf8_lossy(&denied.stderr); + assert!( + stderr.contains("denied"), + "expected 'denied' for bruno branch merge, got: {stderr}" + ); + + output_success( + cli() + .arg("--as") + .arg("act-ragnor") + .arg("branch") + .arg("merge") + .arg("--config") + .arg(&config) + .arg("merge-feature") + .arg("--into") + .arg("main"), + ); +} + +// ─── MR-722 PR A: cli.actor config-only precedence ──────────────────────── +// +// The change-writer test above uses `--as` directly. These two tests +// pin the precedence rule that `main.rs::resolve_cli_actor` implements: +// `--as` flag > `cli.actor` from `omnigraph.yaml` > None. + +fn local_policy_config_with_actor(repo: &SystemRepo, actor: &str) -> String { + // Mirrors `local_policy_config` but adds `cli.actor` so the + // config-only precedence path is exercised. The `cli:` block + // already has `graph` and `branch`; appending `actor` here. + format!( + "\ +project: + name: policy-e2e-local +graphs: + local: + uri: {} +cli: + graph: local + branch: main + actor: {} +query: + roots: + - . +policy: + file: ./policy.yaml +", + yaml_string(&repo.path().to_string_lossy()), + actor, + ) +} + +#[test] +fn local_cli_actor_from_config_used_when_no_flag() { + // cli.actor: act-ragnor in omnigraph.yaml, no --as flag → change + // permitted via admins-write rule. Proves the config-only path + // works; previously the only proof was structural. + let repo = SystemRepo::loaded(); + let config = repo.write_config( + "omnigraph-policy.yaml", + &local_policy_config_with_actor(&repo, "act-ragnor"), + ); + repo.write_config("policy.yaml", POLICY_E2E_YAML); + let mutation_file = insert_person_query(&repo, "system-local-cli-actor.gq"); + + let allowed = parse_stdout_json(&output_success( + cli() + .arg("change") + .arg("--config") + .arg(&config) + .arg("--query") + .arg(&mutation_file) + .arg("--params") + .arg(r#"{"name":"ConfigActorEve","age":18}"#) + .arg("--json"), + )); + assert_eq!(allowed["affected_nodes"], 1); + assert_eq!(allowed["actor_id"], "act-ragnor"); +} + +#[test] +fn local_cli_actor_flag_overrides_config_actor() { + // cli.actor: act-ragnor in config + --as act-bruno on CLI → change + // denied. Flag wins per the precedence rule. Without this test, a + // future change that reverses precedence would ride through silently. + let repo = SystemRepo::loaded(); + let config = repo.write_config( + "omnigraph-policy.yaml", + &local_policy_config_with_actor(&repo, "act-ragnor"), + ); + repo.write_config("policy.yaml", POLICY_E2E_YAML); + let mutation_file = insert_person_query(&repo, "system-local-cli-actor-override.gq"); + + let denied = output_failure( + cli() + .arg("--as") + .arg("act-bruno") + .arg("change") + .arg("--config") + .arg(&config) + .arg("--query") + .arg(&mutation_file) + .arg("--params") + .arg(r#"{"name":"OverrideEve","age":19}"#) + .arg("--json"), + ); + let stderr = String::from_utf8_lossy(&denied.stderr); + assert!( + stderr.contains("denied"), + "expected 'denied' when --as overrides config to bruno, got: {stderr}" + ); +} diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index 80eeb9c..bef91e2 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -1758,9 +1758,11 @@ fn server_bearer_tokens_from_env() -> Result> { #[cfg(test)] mod tests { use super::{ - ServerRuntimeState, classify_server_runtime_state, hash_bearer_token, load_server_settings, - normalize_bearer_token, parse_bearer_tokens_json, server_bearer_tokens_from_env, + ServerConfig, ServerRuntimeState, classify_server_runtime_state, hash_bearer_token, + load_server_settings, normalize_bearer_token, parse_bearer_tokens_json, serve, + server_bearer_tokens_from_env, }; + use serial_test::serial; use std::env; use std::fs; use tempfile::tempdir; @@ -1909,6 +1911,117 @@ server: ); } + #[tokio::test] + #[serial] + async fn serve_refuses_to_start_in_state_1_without_unauthenticated() { + // MR-723 PR A: pin the integration boundary that the classifier + // is actually called by `serve()` before any side-effecting + // work (Lance dataset open, TcpListener::bind). The classifier + // itself is unit-tested above; this test guards the propagation + // path from `classify_server_runtime_state` through serve's + // `?` so a future refactor that drops the call returns red. + // + // Marked `#[serial]` because we have to clear all bearer-token + // env vars, and another test in this module setting any of them + // concurrently would corrupt the read inside `resolve_token_source`. + let _guard = EnvGuard::set(&[ + ("OMNIGRAPH_SERVER_BEARER_TOKEN", None), + ("OMNIGRAPH_SERVER_BEARER_TOKENS_FILE", None), + ("OMNIGRAPH_SERVER_BEARER_TOKENS_JSON", None), + ("OMNIGRAPH_SERVER_BEARER_TOKENS_AWS_SECRET", None), + ("OMNIGRAPH_UNAUTHENTICATED", None), + ]); + let temp = tempdir().unwrap(); + // Repo path doesn't need to exist — classifier fires before + // `AppState::open_with_bearer_tokens_and_policy`. + let config = ServerConfig { + uri: temp + .path() + .join("repo.omni") + .to_string_lossy() + .into_owned(), + bind: "127.0.0.1:0".to_string(), + policy_file: None, + allow_unauthenticated: false, + }; + let result = serve(config).await; + let err = result.expect_err("serve should refuse to start in State 1 without --unauthenticated"); + let msg = format!("{:?}", err); + assert!( + msg.contains("no bearer tokens") || msg.contains("policy file"), + "expected refusal message naming the misconfiguration, got: {msg}", + ); + } + + #[test] + #[serial] + fn unauthenticated_env_var_classification() { + // MR-723 PR A: closes the gap where the env-var read path inside + // `load_server_settings` was structurally implemented but not + // exercised by any test. Three properties to pin, all in one + // sequential test because `cargo test` runs the mod test suite + // in parallel and `OMNIGRAPH_UNAUTHENTICATED` is process-global + // — interleaving with another test that sets the same env var + // (concurrent classifier tests, even the bearer-token suite + // sharing `EnvGuard`) corrupts the read. Sequential within one + // test fn is the simplest race-free shape. + let temp = tempdir().unwrap(); + let config_path = temp.path().join("omnigraph.yaml"); + fs::write( + &config_path, + r#" +graphs: + local: + uri: /tmp/demo-unauth.omni +server: + graph: local +"#, + ) + .unwrap(); + + // Truthy values flip Open mode on, even with CLI flag off. + for value in ["1", "true", "yes", "TRUE", "anything"] { + let _guard = EnvGuard::set(&[("OMNIGRAPH_UNAUTHENTICATED", Some(value))]); + let settings = load_server_settings(Some(&config_path), None, None, None, false) + .expect("settings load should succeed"); + assert!( + settings.allow_unauthenticated, + "OMNIGRAPH_UNAUTHENTICATED={value:?} should enable Open mode", + ); + } + + // Falsy values keep refusal behavior, even with CLI flag off. + for value in ["0", "false", "FALSE", ""] { + let _guard = EnvGuard::set(&[("OMNIGRAPH_UNAUTHENTICATED", Some(value))]); + let settings = load_server_settings(Some(&config_path), None, None, None, false) + .expect("settings load should succeed"); + assert!( + !settings.allow_unauthenticated, + "OMNIGRAPH_UNAUTHENTICATED={value:?} should NOT enable Open mode", + ); + } + + // Unset env var: also false. + let _guard = EnvGuard::set(&[("OMNIGRAPH_UNAUTHENTICATED", None)]); + let settings = load_server_settings(Some(&config_path), None, None, None, false) + .expect("settings load should succeed"); + assert!( + !settings.allow_unauthenticated, + "OMNIGRAPH_UNAUTHENTICATED unset should NOT enable Open mode", + ); + drop(_guard); + + // CLI flag wins even when env is falsy — `serve()` honors the + // OR of both inputs. + let _guard = EnvGuard::set(&[("OMNIGRAPH_UNAUTHENTICATED", Some("0"))]); + let settings = load_server_settings(Some(&config_path), None, None, None, true) + .expect("settings load should succeed"); + assert!( + settings.allow_unauthenticated, + "--unauthenticated CLI flag should win even when env is falsy", + ); + } + #[test] fn classify_policy_enabled_always_wins() { // State 3: any setup with a policy file → PolicyEnabled. The @@ -1983,6 +2096,7 @@ server: } #[test] + #[serial] fn server_bearer_tokens_from_env_reads_legacy_token_and_token_file() { let temp = tempdir().unwrap(); let tokens_path = temp.path().join("tokens.json"); diff --git a/crates/omnigraph-server/tests/server.rs b/crates/omnigraph-server/tests/server.rs index 1688765..69fa8a0 100644 --- a/crates/omnigraph-server/tests/server.rs +++ b/crates/omnigraph-server/tests/server.rs @@ -8,8 +8,10 @@ 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}; +use omnigraph::db::{Omnigraph, ReadTarget, SchemaApplyOptions}; +use omnigraph::error::OmniError; use omnigraph::loader::{LoadMode, load_jsonl}; +use omnigraph_policy::{PolicyChecker, PolicyEngine}; use omnigraph_server::api::{ BranchCreateRequest, BranchMergeRequest, ChangeRequest, ErrorOutput, ExportRequest, IngestRequest, ReadRequest, SchemaApplyRequest, SchemaOutput, @@ -3787,3 +3789,234 @@ async fn default_deny_mode_rejects_schema_apply_with_forbidden() { error.error ); } + +// ─── SDK ↔ HTTP decision parity (MR-722 PR A) ───────────────────────────── +// +// Engine and HTTP both consult Cedar via `PolicyChecker::check()`; by +// construction they cannot disagree on a decision. These tests pin that +// property explicitly so a future refactor that introduces a separate +// auth path (or copy-pastes Cedar evaluation logic) turns red. +// +// Four cases cover the per-action scope shapes: +// * Change on a protected branch via `mutate_as` / POST /change +// * Change with an actor that has no permit +// * BranchMerge to a protected target via `branch_merge_as` / POST /branches/merge +// * BranchMerge with an actor that has no permit + +const PARITY_POLICY_YAML: &str = r#" +version: 1 +groups: + team: [act-bruno] + admins: [act-ragnor] +protected_branches: [main] +rules: + - id: admins-change-anywhere + allow: + actors: { group: admins } + actions: [change] + branch_scope: any + - id: admins-merge-to-protected + allow: + actors: { group: admins } + actions: [branch_merge] + target_branch_scope: protected +"#; + +#[derive(Clone, Copy, Debug)] +enum ParityDecision { + Allow, + Deny, +} + +async fn build_parity_repo() -> (tempfile::TempDir, PathBuf, PathBuf) { + // Build a repo with `main` loaded and a `feature` branch ready for + // merge. Returns the repo path and a written policy.yaml path. + let temp = init_loaded_repo().await; + let repo = repo_path(temp.path()); + { + let db = Omnigraph::open(repo.to_str().unwrap()).await.unwrap(); + db.branch_create_from(ReadTarget::branch("main"), "feature") + .await + .unwrap(); + db.load_as( + "feature", + r#"{"type":"Person","data":{"name":"ParityEve","age":29}}"#, + LoadMode::Append, + None, + ) + .await + .unwrap(); + } + let policy_path = temp.path().join("policy.yaml"); + fs::write(&policy_path, PARITY_POLICY_YAML).unwrap(); + (temp, repo, policy_path) +} + +async fn sdk_change_decision(repo: &Path, policy_path: &Path, actor: &str) -> ParityDecision { + let policy = PolicyEngine::load(policy_path, repo.to_string_lossy().as_ref()).unwrap(); + let db = Omnigraph::open(repo.to_str().unwrap()) + .await + .unwrap() + .with_policy(Arc::new(policy) as Arc); + let mut params: omnigraph_compiler::ParamMap = Default::default(); + // Parameter keys are bare names (no `$` prefix); the runtime resolves + // `$name` references in the query body to `params["name"]`. + params.insert( + "name".to_string(), + omnigraph_compiler::Literal::String("ParityCharlie".to_string()), + ); + params.insert("age".to_string(), omnigraph_compiler::Literal::Integer(30)); + let result = db + .mutate_as("main", MUTATION_QUERIES, "insert_person", ¶ms, Some(actor)) + .await; + match result { + Ok(_) => ParityDecision::Allow, + Err(OmniError::Policy(_)) => ParityDecision::Deny, + Err(other) => panic!("unexpected SDK error for change: {other:?}"), + } +} + +async fn http_change_decision( + repo: &Path, + policy_path: &PathBuf, + actor: &str, + token: &str, +) -> ParityDecision { + let state = AppState::open_with_bearer_tokens_and_policy( + repo.to_string_lossy().to_string(), + vec![(actor.to_string(), token.to_string())], + Some(policy_path), + ) + .await + .unwrap(); + let app = build_app(state); + let req = ChangeRequest { + query_source: MUTATION_QUERIES.to_string(), + query_name: Some("insert_person".to_string()), + params: Some(json!({ "name": "ParityCharlie", "age": 30 })), + branch: Some("main".to_string()), + }; + let (status, _body) = json_response( + &app, + Request::builder() + .uri("/change") + .method(Method::POST) + .header(AUTHORIZATION, format!("Bearer {token}")) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&req).unwrap())) + .unwrap(), + ) + .await; + match status { + StatusCode::OK => ParityDecision::Allow, + StatusCode::FORBIDDEN => ParityDecision::Deny, + other => panic!("unexpected HTTP status for change: {other}"), + } +} + +async fn sdk_merge_decision(repo: &Path, policy_path: &Path, actor: &str) -> ParityDecision { + let policy = PolicyEngine::load(policy_path, repo.to_string_lossy().as_ref()).unwrap(); + let db = Omnigraph::open(repo.to_str().unwrap()) + .await + .unwrap() + .with_policy(Arc::new(policy) as Arc); + let result = db.branch_merge_as("feature", "main", Some(actor)).await; + match result { + Ok(_) => ParityDecision::Allow, + Err(OmniError::Policy(_)) => ParityDecision::Deny, + Err(other) => panic!("unexpected SDK error for branch_merge: {other:?}"), + } +} + +async fn http_merge_decision( + repo: &Path, + policy_path: &PathBuf, + actor: &str, + token: &str, +) -> ParityDecision { + let state = AppState::open_with_bearer_tokens_and_policy( + repo.to_string_lossy().to_string(), + vec![(actor.to_string(), token.to_string())], + Some(policy_path), + ) + .await + .unwrap(); + let app = build_app(state); + let req = BranchMergeRequest { + source: "feature".to_string(), + target: Some("main".to_string()), + }; + let (status, _body) = json_response( + &app, + Request::builder() + .uri("/branches/merge") + .method(Method::POST) + .header(AUTHORIZATION, format!("Bearer {token}")) + .header("content-type", "application/json") + .body(Body::from(serde_json::to_vec(&req).unwrap())) + .unwrap(), + ) + .await; + match status { + StatusCode::OK => ParityDecision::Allow, + StatusCode::FORBIDDEN => ParityDecision::Deny, + other => panic!("unexpected HTTP status for branch_merge: {other}"), + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn policy_decision_parity_change_admin_on_main_allowed() { + // (act-ragnor, change, main) — admins-change-anywhere rule applies. + // Both SDK and HTTP must allow. Each path uses its own fresh repo + // because allow→side-effects. + let (_t1, repo1, policy1) = build_parity_repo().await; + let sdk = sdk_change_decision(&repo1, &policy1, "act-ragnor").await; + let (_t2, repo2, policy2) = build_parity_repo().await; + let http = http_change_decision(&repo2, &policy2, "act-ragnor", "ragnor-token").await; + assert!( + matches!(sdk, ParityDecision::Allow) && matches!(http, ParityDecision::Allow), + "SDK={sdk:?} HTTP={http:?} — should both Allow", + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn policy_decision_parity_change_team_on_main_denied() { + // (act-bruno, change, main) — no rule grants bruno change on + // protected. Both SDK and HTTP must deny. Same repo is reusable + // because deny→no side-effects. + let (_temp, repo, policy) = build_parity_repo().await; + let sdk = sdk_change_decision(&repo, &policy, "act-bruno").await; + let http = http_change_decision(&repo, &policy, "act-bruno", "bruno-token").await; + assert!( + matches!(sdk, ParityDecision::Deny) && matches!(http, ParityDecision::Deny), + "SDK={sdk:?} HTTP={http:?} — should both Deny", + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn policy_decision_parity_branch_merge_admin_allowed() { + // (act-ragnor, branch_merge, feature→main) — admins-merge-to-protected + // rule applies. Both Allow. Each path uses its own fresh repo — + // a successful merge consumes the feature branch's commit on main. + let (_t1, repo1, policy1) = build_parity_repo().await; + let sdk = sdk_merge_decision(&repo1, &policy1, "act-ragnor").await; + let (_t2, repo2, policy2) = build_parity_repo().await; + let http = http_merge_decision(&repo2, &policy2, "act-ragnor", "ragnor-token").await; + assert!( + matches!(sdk, ParityDecision::Allow) && matches!(http, ParityDecision::Allow), + "SDK={sdk:?} HTTP={http:?} — should both Allow", + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn policy_decision_parity_branch_merge_team_denied() { + // (act-bruno, branch_merge, feature→main) — no rule grants bruno + // branch_merge. Both Deny. + let (_temp, repo, policy) = build_parity_repo().await; + let sdk = sdk_merge_decision(&repo, &policy, "act-bruno").await; + let http = http_merge_decision(&repo, &policy, "act-bruno", "bruno-token").await; + assert!( + matches!(sdk, ParityDecision::Deny) && matches!(http, ParityDecision::Deny), + "SDK={sdk:?} HTTP={http:?} — should both Deny", + ); +} diff --git a/crates/omnigraph/tests/policy_engine_chassis.rs b/crates/omnigraph/tests/policy_engine_chassis.rs index 5eb221a..b1f43d9 100644 --- a/crates/omnigraph/tests/policy_engine_chassis.rs +++ b/crates/omnigraph/tests/policy_engine_chassis.rs @@ -258,6 +258,46 @@ async fn load_as_allows_when_policy_permits_actor() { .expect("act-allowed should be able to load on main"); } +#[tokio::test] +async fn load_file_as_denies_when_policy_rejects_actor() { + // `load_file_as` was added in PR #104 as the actor-aware mirror of + // `load_file`, used by the CLI's `omnigraph load`. Tested + // indirectly via CLI integration; this test closes the direct-SDK + // gap so a regression in the file-read path doesn't ride through + // unnoticed. + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + let data_path = dir.path().join("one-person.jsonl"); + fs::write(&data_path, ONE_PERSON_JSONL).unwrap(); + + let result = db + .load_file_as( + "main", + data_path.to_str().unwrap(), + LoadMode::Merge, + Some("act-denied"), + ) + .await; + assert_denied(result, "load_file_as"); +} + +#[tokio::test] +async fn load_file_as_allows_when_policy_permits_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + let data_path = dir.path().join("one-person.jsonl"); + fs::write(&data_path, ONE_PERSON_JSONL).unwrap(); + + db.load_file_as( + "main", + data_path.to_str().unwrap(), + LoadMode::Merge, + Some("act-allowed"), + ) + .await + .expect("act-allowed should be able to load_file_as on main"); +} + #[tokio::test] async fn ingest_as_denies_when_policy_rejects_actor() { let dir = tempfile::tempdir().unwrap();