diff --git a/AGENTS.md b/AGENTS.md index 827efbb..d1c1bde 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,7 @@ OmniGraph is a typed property-graph engine built as a coordination layer over ma - **Multi-modal querying**: vector ANN (`nearest`), full-text (`search`/`fuzzy`/`match_text`/`bm25`), Reciprocal Rank Fusion (`rrf`), and graph traversal (`Expand`, anti-join `not { … }`) in one runtime. - **Branches and commits across the whole graph**: Git-style — every successful publish appends to a commit DAG; merges are three-way at the row level. - **Atomic per-query writes**: `mutate_as` and `load` accumulate insert/update batches into an in-memory `MutationStaging.pending` per touched table; one `stage_*` + `commit_staged` per table runs at end-of-query, then `ManifestBatchPublisher::publish` commits the manifest atomically with per-table `expected_table_versions` CAS. A mid-query failure leaves Lance HEAD untouched on staged tables — no drift, no run state machine, no staging branches. Deletes still inline-commit; D₂ at parse time prevents inserts/updates and deletes from coexisting in one query. -- **HTTP server**: Axum + utoipa OpenAPI, bearer auth (SHA-256 hashed, optional AWS Secrets Manager), Cedar policy gating. +- **HTTP server**: Axum + utoipa OpenAPI, bearer auth (SHA-256 hashed, optional AWS Secrets Manager). Cedar policy enforcement is engine-wide — every `_as` writer calls `Omnigraph::enforce(action, scope, actor)`, so HTTP, CLI, and embedded SDK consumers all hit the same gate. - **CLI** driven by a single `omnigraph.yaml`; multi-format output (json/jsonl/csv/kv/table). Throughout the docs, capabilities are split into **L1 — Inherited from Lance** vs **L2 — Added by OmniGraph**. @@ -226,8 +226,8 @@ omnigraph policy explain --actor act-alice --action change --branch main | Per-query atomic writes | — | In-memory `MutationStaging.pending` accumulator + `stage_*` / `commit_staged` per touched table at end-of-query + publisher CAS via `commit_with_expected` (single manifest commit per `mutate_as` / `load`); D₂ parse-time rule keeps inserts/updates and deletes from mixing | | Three-way row-level merge | — | `OrderedTableCursor` + `StagedTableWriter`, structured `MergeConflictKind` | | Change feeds | — | `diff_between` / `diff_commits` with manifest fast path + ID streaming | -| Cedar policy | — | 8 actions, branch / target_branch / protected scopes, validate/test/explain CLI | -| HTTP server | — | Axum, OpenAPI via utoipa, bearer auth (SHA-256, AWS Secrets Manager option), policy gating, NDJSON streaming export | +| Cedar policy | — | 8 actions, branch / target_branch / protected scopes, validate/test/explain CLI. **Engine-wide enforcement** (MR-722): every `_as` writer (`apply_schema_as`, `mutate_as`, `load_as`, `ingest_as`, `branch_create_as` / `branch_create_from_as`, `branch_delete_as`, `branch_merge_as`) calls `Omnigraph::enforce(action, scope, actor)` — HTTP, CLI, embedded SDK all hit the same gate. | +| HTTP server | — | Axum, OpenAPI via utoipa, bearer auth (SHA-256, AWS Secrets Manager option), `authorize_request` at the HTTP boundary (resolves bearer→actor, applies admission control), NDJSON streaming export | | CLI with config | — | `omnigraph.yaml`, aliases, multi-format output (json/jsonl/csv/kv/table) | | Audit / actor tracking | — | `_as` write APIs + actor map in commit graph | | Local RustFS bootstrap | — | `scripts/local-rustfs-bootstrap.sh` one-shot S3-backed dev environment | diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index a3bfed1..dc9e2a8 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -1298,9 +1298,13 @@ async fn server_branch_create( .map_err(ApiError::from_workload_reject)?; { let db = &state.engine; - db.branch_create_from(ReadTarget::branch(&from), &request.name) - .await - .map_err(ApiError::from_omni)?; + db.branch_create_from_as( + ReadTarget::branch(&from), + &request.name, + actor.as_ref().map(|Extension(a)| a.as_str()), + ) + .await + .map_err(ApiError::from_omni)?; } Ok(Json(BranchCreateOutput { uri: state.uri().to_string(), @@ -1359,7 +1363,7 @@ async fn server_branch_delete( .map_err(ApiError::from_workload_reject)?; { let db = &state.engine; - db.branch_delete(&branch) + db.branch_delete_as(&branch, actor_id) .await .map_err(ApiError::from_omni)?; } diff --git a/crates/omnigraph/src/db/omnigraph.rs b/crates/omnigraph/src/db/omnigraph.rs index 9519abe..610be62 100644 --- a/crates/omnigraph/src/db/omnigraph.rs +++ b/crates/omnigraph/src/db/omnigraph.rs @@ -991,6 +991,22 @@ impl Omnigraph { } pub async fn branch_create(&self, name: &str) -> Result<()> { + self.branch_create_as(name, None).await + } + + /// Create a branch from the coordinator's currently-open snapshot, + /// with an explicit actor for engine-layer policy enforcement + /// (MR-722 fan-out). Scope is `TargetBranch(name)` — symmetric with + /// `branch_delete_as`: the branch being acted upon is the target. + /// Cedar rules using `target_branch_scope: protected` therefore see + /// the new-branch name and can deny e.g. creating any branch named + /// `main` from a non-privileged actor. + pub async fn branch_create_as(&self, name: &str, actor: Option<&str>) -> Result<()> { + self.enforce( + omnigraph_policy::PolicyAction::BranchCreate, + &omnigraph_policy::ResourceScope::TargetBranch(name.to_string()), + actor, + )?; self.ensure_schema_state_valid().await?; self.ensure_schema_apply_idle("branch_create").await?; ensure_public_branch_ref(name, "branch_create")?; @@ -1002,8 +1018,41 @@ impl Omnigraph { from: impl Into, name: &str, ) -> Result<()> { + self.branch_create_from_as(from, name, None).await + } + + /// Create a branch from a specific source branch with an explicit + /// actor for engine-layer policy enforcement (MR-722 fan-out). + /// + /// Scope is `BranchTransition { source, target }` — matches the + /// HTTP-layer convention at `server_branch_create` + /// (branch=Some(from), target_branch=Some(name)), so engine and + /// HTTP fire the same Cedar decision. Pinned-snapshot sources + /// (which aren't a branch ref) materialize as the sentinel + /// `` for the policy check; Cedar rules using + /// `branch_scope: any` still match, rules pinning a specific + /// source branch correctly do not. + pub async fn branch_create_from_as( + &self, + from: impl Into, + name: &str, + actor: Option<&str>, + ) -> Result<()> { + let target = from.into(); + let source_branch = match &target { + ReadTarget::Branch(b) => b.clone(), + _ => "".to_string(), + }; + self.enforce( + omnigraph_policy::PolicyAction::BranchCreate, + &omnigraph_policy::ResourceScope::BranchTransition { + source: source_branch, + target: name.to_string(), + }, + actor, + )?; self.ensure_schema_apply_idle("branch_create_from").await?; - self.branch_create_from_impl(from, name, false).await + self.branch_create_from_impl(target, name, false).await } async fn branch_create_from_impl( @@ -1049,6 +1098,22 @@ impl Omnigraph { } pub async fn branch_delete(&self, name: &str) -> Result<()> { + self.branch_delete_as(name, None).await + } + + /// Delete a branch with an explicit actor for engine-layer policy + /// enforcement (MR-722 fan-out). Scope is `TargetBranch(name)` — + /// matches the HTTP-layer convention at `server_branch_delete` + /// (branch=None, target_branch=Some(name)). Cedar rules using + /// `target_branch_scope: protected` therefore correctly gate + /// deletion of protected branches (e.g. deny BranchDelete against + /// `main`). + pub async fn branch_delete_as(&self, name: &str, actor: Option<&str>) -> Result<()> { + self.enforce( + omnigraph_policy::PolicyAction::BranchDelete, + &omnigraph_policy::ResourceScope::TargetBranch(name.to_string()), + actor, + )?; self.ensure_schema_state_valid().await?; self.ensure_schema_apply_idle("branch_delete").await?; ensure_public_branch_ref(name, "branch_delete")?; diff --git a/crates/omnigraph/src/exec/merge.rs b/crates/omnigraph/src/exec/merge.rs index 7250881..2e5f32e 100644 --- a/crates/omnigraph/src/exec/merge.rs +++ b/crates/omnigraph/src/exec/merge.rs @@ -1062,6 +1062,21 @@ impl Omnigraph { target: &str, actor_id: Option<&str>, ) -> Result { + // Engine-layer policy gate (MR-722 fan-out / PR #3). Scope is + // `BranchTransition { source, target }` — matches the HTTP-layer + // convention at `server_branch_merge` (branch=Some(source), + // target_branch=Some(target)). Cedar rules using + // `target_branch_scope: protected` therefore correctly gate + // merges INTO protected branches without forbidding the + // (symmetric) source-side reference. + self.enforce( + omnigraph_policy::PolicyAction::BranchMerge, + &omnigraph_policy::ResourceScope::BranchTransition { + source: source.to_string(), + target: target.to_string(), + }, + actor_id, + )?; self.ensure_schema_apply_idle("branch_merge").await?; self.branch_merge_impl(source, target, actor_id).await } diff --git a/crates/omnigraph/src/exec/mutation.rs b/crates/omnigraph/src/exec/mutation.rs index e58b718..a5fc6c7 100644 --- a/crates/omnigraph/src/exec/mutation.rs +++ b/crates/omnigraph/src/exec/mutation.rs @@ -692,6 +692,16 @@ impl Omnigraph { params: &ParamMap, actor_id: Option<&str>, ) -> Result { + // Engine-layer policy gate (MR-722 fan-out / PR #3). Scope is + // `Branch(branch)` to match the HTTP-layer convention at + // `server_change` (branch=Some(branch), target_branch=None). When no + // PolicyChecker is installed this is a no-op; with policy installed + // and actor=None this fails hard (forget-the-actor footgun guard). + self.enforce( + omnigraph_policy::PolicyAction::Change, + &omnigraph_policy::ResourceScope::Branch(branch.to_string()), + actor_id, + )?; self.mutate_with_current_actor(branch, query_source, query_name, params, actor_id) .await } diff --git a/crates/omnigraph/src/loader/mod.rs b/crates/omnigraph/src/loader/mod.rs index a795f28..50abfe3 100644 --- a/crates/omnigraph/src/loader/mod.rs +++ b/crates/omnigraph/src/loader/mod.rs @@ -90,6 +90,18 @@ impl Omnigraph { mode: LoadMode, actor_id: Option<&str>, ) -> Result { + // Engine-layer policy gate (MR-722 fan-out / PR #3). Scope is + // `Branch(branch)` for the data-write portion. If ingest creates + // a new branch as a side-effect (target branch doesn't exist), + // the inner `branch_create_from_as` call below additionally + // checks `BranchCreate` — both authorities are genuinely needed + // for "ingest into a fresh branch", so the layered check is + // correct, not redundant. + self.enforce( + omnigraph_policy::PolicyAction::Change, + &omnigraph_policy::ResourceScope::Branch(branch.to_string()), + actor_id, + )?; self.ingest_with_current_actor(branch, from, data, mode, actor_id) .await } @@ -135,8 +147,18 @@ impl Omnigraph { .iter() .any(|name| name == &target_branch); if branch_created { - self.branch_create_from(crate::db::ReadTarget::branch(&base_branch), &target_branch) - .await?; + // Thread the actor through to the implicit BranchCreate so + // policy decisions match what an explicit `branch_create_from_as` + // call would see. Calling the no-actor variant here would + // bypass BranchCreate enforcement when policy is installed — + // the footgun guard catches that case too, but threading is + // the correct fix. + self.branch_create_from_as( + crate::db::ReadTarget::branch(&base_branch), + &target_branch, + actor_id, + ) + .await?; } let result = self.load_as(&target_branch, data, mode, actor_id).await?; @@ -160,6 +182,17 @@ impl Omnigraph { mode: LoadMode, actor_id: Option<&str>, ) -> Result { + // Engine-layer policy gate (MR-722 fan-out / PR #3). Scope is + // `Branch(branch)` to match the HTTP-layer Change convention. + // `ingest_as` also calls `load_as` after enforcing its own + // Change gate — that double-check is fine because both gates + // resolve to identical Cedar decisions for the same actor + + // branch (the second check is a structurally-correct no-op). + self.enforce( + omnigraph_policy::PolicyAction::Change, + &omnigraph_policy::ResourceScope::Branch(branch.to_string()), + actor_id, + )?; self.ensure_schema_state_valid().await?; // Reject internal `__run__*` / system-prefixed branches at the // public write boundary. Direct-publish paths assert this diff --git a/crates/omnigraph/tests/policy_engine_chassis.rs b/crates/omnigraph/tests/policy_engine_chassis.rs index 83775a8..5eb221a 100644 --- a/crates/omnigraph/tests/policy_engine_chassis.rs +++ b/crates/omnigraph/tests/policy_engine_chassis.rs @@ -1,39 +1,59 @@ -//! Engine-layer policy enforcement (MR-722 chassis core, PR #2). +//! Engine-layer policy enforcement (MR-722 chassis core, PR #2 + PR #3). //! -//! These tests exercise `Omnigraph::with_policy()` + `apply_schema_as()` +//! These tests exercise `Omnigraph::with_policy()` + every `_as` writer //! via the SDK directly — *no HTTP layer involved*. They're the proof //! that engine-layer enforcement works for embedded callers and CLI //! direct-engine writes, not just server requests. //! -//! `apply_schema_as` is the only writer wired in PR #2; PR #3 fans the -//! `enforce()` call out to the other six (`mutate_as`, `load`, -//! `ingest_as`, `branch_create_from`, `branch_delete`, `branch_merge`). +//! PR #2 wired `apply_schema_as`. PR #3 fans the same `enforce()` call +//! out to the remaining six writers — `mutate_as`, `load_as`, +//! `ingest_as`, `branch_create_as` / `branch_create_from_as`, +//! `branch_delete_as`, `branch_merge_as`. Each writer pair below +//! covers allow + deny via the engine-side gate; the allow case proves +//! the enforce call is correctly scoped (i.e. doesn't reject a legit +//! actor), the deny case proves it actually denies an unauthorized +//! actor — and both together pin the action × scope shape to match the +//! HTTP-layer authorize_request convention so engine and HTTP fire the +//! same Cedar decision. mod helpers; use std::fs; +use std::path::Path; use std::sync::Arc; -use omnigraph::db::{Omnigraph, SchemaApplyOptions}; +use omnigraph::db::{Omnigraph, ReadTarget, SchemaApplyOptions}; +use omnigraph::loader::LoadMode; use omnigraph::error::OmniError; use omnigraph_policy::{PolicyChecker, PolicyEngine}; use helpers::*; -/// Cedar policy: `act-allowed` may SchemaApply; `act-denied` is in the -/// known-actors set (so Cedar evaluates the policy, doesn't reject as -/// unknown) but has no permit rule. +/// Cedar policy: `act-allowed` may do every write; `act-denied` is in +/// the known-actors set (so Cedar evaluates the policy and doesn't +/// reject as unknown) but has no permit rule and is therefore implicitly +/// denied for every action. +/// +/// The rule split mirrors the per-action scope convention: Change uses +/// `branch_scope`; SchemaApply, BranchCreate, BranchDelete, BranchMerge +/// use `target_branch_scope` (see `PolicyAction::uses_branch_scope` and +/// `uses_target_branch_scope` in `omnigraph-policy`). const POLICY_YAML: &str = r#" version: 1 groups: - schema-writers: [act-allowed] + writers: [act-allowed] readers: [act-denied] protected_branches: [main] rules: - - id: writers-schema-apply + - id: writers-data allow: - actors: { group: schema-writers } - actions: [schema_apply] + actors: { group: writers } + actions: [change] + branch_scope: any + - id: writers-branches-schema + allow: + actors: { group: writers } + actions: [schema_apply, branch_create, branch_delete, branch_merge] target_branch_scope: any "#; @@ -41,16 +61,54 @@ fn additive_schema() -> String { helpers::TEST_SCHEMA.replace(" age: I32?\n}", " age: I32?\n nickname: String?\n}") } -async fn init_with_policy(dir: &tempfile::TempDir) -> (Omnigraph, Arc) { - let db = init_and_load(dir).await; - let policy_path = dir.path().join("policy.yaml"); +fn install_policy(db: Omnigraph, dir_path: &Path) -> (Omnigraph, Arc) { + let policy_path = dir_path.join("policy.yaml"); fs::write(&policy_path, POLICY_YAML).unwrap(); - let engine = PolicyEngine::load(&policy_path, dir.path().to_str().unwrap()).unwrap(); + let engine = PolicyEngine::load(&policy_path, dir_path.to_str().unwrap()).unwrap(); let engine = Arc::new(engine); let db = db.with_policy(Arc::clone(&engine) as Arc); (db, engine) } +async fn init_with_policy(dir: &tempfile::TempDir) -> (Omnigraph, Arc) { + let db = init_and_load(dir).await; + install_policy(db, dir.path()) +} + +/// Variant for tests that need a pre-created feature branch (branch_delete / +/// branch_merge setup). Create the branch BEFORE wrapping with policy so the +/// setup itself doesn't need to satisfy BranchCreate. +async fn init_with_policy_and_feature_branch( + dir: &tempfile::TempDir, + branch: &str, +) -> (Omnigraph, Arc) { + let db = init_and_load(dir).await; + db.branch_create_from(ReadTarget::branch("main"), branch) + .await + .expect("setup: create feature branch before installing policy"); + install_policy(db, dir.path()) +} + +// `MUTATION_QUERIES` from helpers/mod.rs already defines `insert_person($name, $age)` +// — reuse it rather than redefining one here, so this test exercises the +// same surface the engine integration tests do. + +/// One JSONL record for `load_as` / `ingest_as` exercises. +const ONE_PERSON_JSONL: &str = r#"{"type": "Person", "data": {"name": "Eve"}}"#; + +fn assert_denied(result: Result, what: &str) { + match result { + Err(OmniError::Policy(msg)) => { + assert!( + msg.contains("denied"), + "{what}: expected denial message, got: {msg}" + ); + } + Err(other) => panic!("{what}: expected OmniError::Policy, got: {other:?}"), + Ok(value) => panic!("{what}: expected denial, got Ok({value:?})"), + } +} + #[tokio::test] async fn apply_schema_as_denies_when_policy_rejects_actor() { let dir = tempfile::tempdir().unwrap(); @@ -127,3 +185,191 @@ async fn apply_schema_without_policy_still_works() { .await .expect("no policy → no enforcement → apply succeeds"); } + +// ─── PR #3 writer fan-out ───────────────────────────────────────────────── +// +// One allow + one deny test per newly-wired writer. The allow case +// proves the enforce scope is correctly shaped (i.e. doesn't reject a +// legit actor whose policy permit matches the engine-side scope). The +// deny case proves the gate actually fires for an unauthorized actor. +// Footgun-guard (no-actor + policy-installed) is already proved by +// `apply_schema_without_actor_when_policy_is_installed_denies` and +// applies identically to every `_as` variant — duplicating it per +// writer would be redundant. + +#[tokio::test] +async fn mutate_as_denies_when_policy_rejects_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + + let params = mixed_params(&[("$name", "Eve")], &[("$age", 22)]); + let result = db + .mutate_as( + "main", + MUTATION_QUERIES, + "insert_person", + ¶ms, + Some("act-denied"), + ) + .await; + assert_denied(result, "mutate_as"); +} + +#[tokio::test] +async fn mutate_as_allows_when_policy_permits_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + + let params = mixed_params(&[("$name", "Eve")], &[("$age", 22)]); + db.mutate_as( + "main", + MUTATION_QUERIES, + "insert_person", + ¶ms, + Some("act-allowed"), + ) + .await + .expect("act-allowed should be able to Change on main"); +} + +#[tokio::test] +async fn load_as_denies_when_policy_rejects_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + + let result = db + .load_as("main", ONE_PERSON_JSONL, LoadMode::Merge, Some("act-denied")) + .await; + assert_denied(result, "load_as"); +} + +#[tokio::test] +async fn load_as_allows_when_policy_permits_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + + db.load_as( + "main", + ONE_PERSON_JSONL, + LoadMode::Merge, + Some("act-allowed"), + ) + .await + .expect("act-allowed should be able to load on main"); +} + +#[tokio::test] +async fn ingest_as_denies_when_policy_rejects_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + + let result = db + .ingest_as( + "main", + Some("main"), + ONE_PERSON_JSONL, + LoadMode::Merge, + Some("act-denied"), + ) + .await; + assert_denied(result, "ingest_as"); +} + +#[tokio::test] +async fn ingest_as_allows_when_policy_permits_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + + db.ingest_as( + "main", + Some("main"), + ONE_PERSON_JSONL, + LoadMode::Merge, + Some("act-allowed"), + ) + .await + .expect("act-allowed should be able to ingest on main"); +} + +#[tokio::test] +async fn branch_create_as_denies_when_policy_rejects_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + + let result = db.branch_create_as("feature", Some("act-denied")).await; + assert_denied(result, "branch_create_as"); +} + +#[tokio::test] +async fn branch_create_as_allows_when_policy_permits_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + + db.branch_create_as("feature", Some("act-allowed")) + .await + .expect("act-allowed should be able to BranchCreate"); +} + +#[tokio::test] +async fn branch_create_from_as_denies_when_policy_rejects_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + + let result = db + .branch_create_from_as(ReadTarget::branch("main"), "feature", Some("act-denied")) + .await; + assert_denied(result, "branch_create_from_as"); +} + +#[tokio::test] +async fn branch_create_from_as_allows_when_policy_permits_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy(&dir).await; + + db.branch_create_from_as(ReadTarget::branch("main"), "feature", Some("act-allowed")) + .await + .expect("act-allowed should be able to BranchCreate from main"); +} + +#[tokio::test] +async fn branch_delete_as_denies_when_policy_rejects_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy_and_feature_branch(&dir, "feature").await; + + let result = db.branch_delete_as("feature", Some("act-denied")).await; + assert_denied(result, "branch_delete_as"); +} + +#[tokio::test] +async fn branch_delete_as_allows_when_policy_permits_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy_and_feature_branch(&dir, "feature").await; + + db.branch_delete_as("feature", Some("act-allowed")) + .await + .expect("act-allowed should be able to BranchDelete"); +} + +#[tokio::test] +async fn branch_merge_as_denies_when_policy_rejects_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy_and_feature_branch(&dir, "feature").await; + + let result = db + .branch_merge_as("feature", "main", Some("act-denied")) + .await; + assert_denied(result, "branch_merge_as"); +} + +#[tokio::test] +async fn branch_merge_as_allows_when_policy_permits_actor() { + let dir = tempfile::tempdir().unwrap(); + let (db, _engine) = init_with_policy_and_feature_branch(&dir, "feature").await; + + // No diverging writes on feature → merge is a no-op fast-forward, + // but it still goes through enforce(BranchMerge, ...). That's the + // path under test; the actual merge outcome is incidental. + db.branch_merge_as("feature", "main", Some("act-allowed")) + .await + .expect("act-allowed should be able to BranchMerge"); +} diff --git a/docs/user/policy.md b/docs/user/policy.md index e0f1fda..974323c 100644 --- a/docs/user/policy.md +++ b/docs/user/policy.md @@ -11,9 +11,7 @@ OmniGraph integrates AWS Cedar (`cedar-policy = 4.9`) for ABAC. 5. `branch_create` 6. `branch_delete` 7. `branch_merge` -8. `run_publish` -9. `run_abort` -10. `admin` — reserved +8. `admin` — reserved for policy-management surfaces (hot reload, audit log, approvals). No call site today; see MR-724 for the reservation rationale. ## Scope kinds @@ -39,9 +37,43 @@ Each rule must use exactly one of `branch_scope` or `target_branch_scope`. - `omnigraph policy test` — run cases in `policy.tests.yaml`, exit 1 on any expectation mismatch. - `omnigraph policy explain --actor … --action … [--branch …] [--target-branch …]` — show decision and matched rule. -## Server enforcement +## Enforcement -Every mutating endpoint calls `authorize_request()` *before* the handler runs; decisions are logged with actor / action / branch / outcome / matched rule. +Policy is a property of the **engine**, not the transport. Every mutating +write — `mutate_as`, `load_as`, `ingest_as`, `apply_schema_as`, +`branch_create_as`, `branch_create_from_as`, `branch_delete_as`, +`branch_merge_as` — calls `Omnigraph::enforce(action, scope, actor)` at +the head of the method. The gate fires identically whether the call +originates from the HTTP server, the CLI, or an embedded SDK consumer. +When no `PolicyChecker` is installed (the dev/embedded default) the gate +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-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 +is the **defense in depth** layer: it catches CLI direct-engine writes, +embedded SDK consumers, and any future transport that hasn't (or won't) +re-implement HTTP's authorize_request. Both layers consult the same +Cedar policy via the same `PolicyChecker` trait, so decisions cannot +disagree. + +## Coarse vs. fine enforcement + +There are two enforcement points, each with non-overlapping +responsibilities: + +| Layer | Question it answers | Where it fires | +|---|---|---| +| **Engine-layer (coarse)** | Can this actor invoke this action against this branch / branch-transition? | `Omnigraph::enforce(action, scope, actor)` at the head of every `_as` writer; one Cedar decision per call. | +| **Query-layer (fine)** | For the rows / types this action actually touches, which can the actor see or modify? | Per-row predicates pushed into DataFusion at plan time. **Not yet implemented — see MR-725.** | + +The engine-layer gate keeps `ResourceScope` deliberately at branch +granularity (`Graph`, `Branch`, `TargetBranch`, `BranchTransition`). +Per-type and per-row authority is the query-layer's job; conflating them +in `ResourceScope` would create two places per-type policy could be +evaluated and a drift surface between them. ## Actor identity (signed-claim-only)