From 7072973ed46487f4cc2d379caa55fef3a8444411 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:09:17 +0000 Subject: [PATCH 1/5] wip: project-defined roles with node-scoped grants Work in progress, not yet ready for review. roles.toml at the project root defines project roles. Each grant's node selectors resolve per snapshot when the location publishes. Effective grants are the approved (selector, permission) pairs that the new definition still contains: removals apply at once, and additions wait for an administrator's approval. Runs need the permission on every node they write, view.sql is checked per node, and authorization takes the union across a person's roles. Validated so far: the native role tests (13, on SQLite and PostgreSQL), the full orchestration native suite (104), the installed test_project_roles.py, Fensu and the hygiene scan. Not yet run: make check-ci. The full publication suite has failures that are still being attributed. --- Cargo.lock | 1 + .../orchestration/custom_role_approval.json | 10 +- .../orchestration/project_roles.toml | 11 + crates/sqlbuild-orchestration/Cargo.toml | 1 + .../authorization/_helpers/custom_roles.rs | 165 +++++- .../authorization/_helpers/role_approvals.rs | 4 +- .../authorization/main/authorize_session.rs | 45 +- .../src/authorization/main/compile_roles.rs | 40 ++ .../src/authorization/main/mod.rs | 1 + .../main/publish_custom_roles.rs | 28 +- .../main/request_role_activation.rs | 4 +- .../src/credentials/models.rs | 36 +- .../tests/credentials/roles/helpers.rs | 81 ++- .../tests/credentials/test_contracts.rs | 2 + .../tests/credentials/test_custom_roles.rs | 13 +- .../tests/credentials/test_project_roles.rs | 483 ++++++++++++++++++ .../tests/credentials/test_types.rs | 13 + .../bindings/_helpers/compiled_project/mod.rs | 1 + .../compiled_project/snapshot_roles.rs | 52 ++ .../_helpers/location_command/snapshot.rs | 4 +- .../sqlbuild/references/docs/cli/server.md | 37 +- .../publication/_test_types.py | 8 + .../compiled_project/publication/helpers.py | 52 ++ .../publication/test_project_roles.py | 138 +++++ website/src/content/docs/docs/cli/server.mdx | 36 +- 25 files changed, 1173 insertions(+), 93 deletions(-) create mode 100644 benchmarks/duckshop/edge_cases/orchestration/project_roles.toml create mode 100644 crates/sqlbuild-orchestration/src/authorization/main/compile_roles.rs create mode 100644 crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs create mode 100644 crates/sqlbuild-python/src/bindings/_helpers/compiled_project/snapshot_roles.rs create mode 100644 tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_project_roles.py diff --git a/Cargo.lock b/Cargo.lock index 462a1de8f..0336d4e91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4310,6 +4310,7 @@ dependencies = [ "tempfile", "thiserror 2.0.21", "tokio", + "toml", "utoipa", "uuid", ] diff --git a/benchmarks/duckshop/edge_cases/orchestration/custom_role_approval.json b/benchmarks/duckshop/edge_cases/orchestration/custom_role_approval.json index 6ac20d433..f7b385b76 100644 --- a/benchmarks/duckshop/edge_cases/orchestration/custom_role_approval.json +++ b/benchmarks/duckshop/edge_cases/orchestration/custom_role_approval.json @@ -1,10 +1,10 @@ { - "description": "A support role is published, approved by another administrator, widened, republished in a different action order, approved, then narrowed. Widening waits for approval; reordering keeps the same definition; narrowing applies on publish.", + "description": "A support role is published, approved by another administrator, widened, republished in a different permission order, approved, then narrowed. Widening waits for approval; reordering keeps the same definition; narrowing applies on publish.", "publications": [ - ["view"], - ["view", "job.run"], - ["job.run", "view"], - ["job.run"] + [{"nodes": ["tag:orders"], "permissions": ["view"], "resolved_nodes": {"tag:orders": ["orders"]}}], + [{"nodes": ["tag:orders"], "permissions": ["view", "job.run"], "resolved_nodes": {"tag:orders": ["orders"]}}], + [{"nodes": ["tag:orders"], "permissions": ["job.run", "view"], "resolved_nodes": {"tag:orders": ["orders"]}}], + [{"nodes": ["tag:orders"], "permissions": ["job.run"], "resolved_nodes": {"tag:orders": ["orders"]}}] ], "expected_after_narrowing": {"view": null, "job.run": "orders_support"} } diff --git a/benchmarks/duckshop/edge_cases/orchestration/project_roles.toml b/benchmarks/duckshop/edge_cases/orchestration/project_roles.toml new file mode 100644 index 000000000..be64548f9 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/orchestration/project_roles.toml @@ -0,0 +1,11 @@ +# Project roles: one grant follows tag coverage across publications; one intersection selects nothing yet. +[roles.orders_support] +description = "Answer order questions" + +[[roles.orders_support.grants]] +nodes = ["tag:support"] +permissions = ["view.sql"] + +[[roles.orders_support.grants]] +nodes = ["tag:support,tag:pii"] +permissions = ["view.evidence"] diff --git a/crates/sqlbuild-orchestration/Cargo.toml b/crates/sqlbuild-orchestration/Cargo.toml index 6c2b73328..f663cbb89 100644 --- a/crates/sqlbuild-orchestration/Cargo.toml +++ b/crates/sqlbuild-orchestration/Cargo.toml @@ -31,6 +31,7 @@ sqlx-postgres.workspace = true subtle.workspace = true thiserror.workspace = true tokio.workspace = true +toml.workspace = true utoipa.workspace = true uuid.workspace = true diff --git a/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs b/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs index c8ade0b02..c92c57b16 100644 --- a/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs +++ b/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs @@ -1,4 +1,4 @@ -//! Published custom roles narrow at once; a widened definition waits for an administrator approval. +//! Reviewed role selectors and permissions narrow at once and widen only by approval; coverage follows snapshots. use std::collections::{BTreeMap, BTreeSet}; @@ -6,11 +6,14 @@ use serde_json::{Value, json}; use sqlbuild_recall::records::main::sha256_hex::sha256_hex; use uuid::Uuid; -use crate::credentials::models::{Action, PublishedRole, RoleDefinition}; +use crate::credentials::models::{ + Action, PermissionScope, PublishedRole, RoleDefinition, RoleGrant, +}; use crate::snapshots::main::read_snapshot_documents::read_snapshot_documents; use crate::store::errors::ControlError; use crate::store::main::query::query; use crate::store::models::{Change, ControlTransaction, Parameter}; +use crate::store::types::Row; const ADMINISTRATIVE: [Action; 5] = [ Action::LocationManage, @@ -29,6 +32,18 @@ fn action_name(action: Action) -> Result { } } +fn valid_grant(grant: &RoleGrant) -> bool { + let blank_selector = grant + .nodes + .iter() + .any(|selector| selector.trim().is_empty()); + let administrative = grant + .permissions + .iter() + .any(|action| ADMINISTRATIVE.contains(action)); + !grant.nodes.is_empty() && !blank_selector && !grant.permissions.is_empty() && !administrative +} + pub(crate) fn decode_roles( document: Option<&Value>, ) -> Result, ControlError> { @@ -44,20 +59,29 @@ pub(crate) fn decode_roles( if key.trim().is_empty() || BUILTIN.contains(&key.to_lowercase().as_str()) || definition.description.trim().is_empty() - || definition - .actions - .iter() - .any(|action| ADMINISTRATIVE.contains(action)) + || definition.grants.is_empty() + || !definition.grants.iter().all(valid_grant) { return Err(ControlError::Invalid(format!( - "role {key} needs a description, a key that is not a built-in role, and only non-administrative actions" + "role {key} needs a description, a key that is not a built-in role, and grants of node selectors with only non-administrative permissions" ))); } - let mut grants: BTreeSet = BTreeSet::new(); - for action in definition.actions { - grants.insert(action_name(action)?); + let mut grants: Vec = definition.grants; + grants.sort(); + grants.dedup(); + let mut selectors: BTreeSet<(Vec, Vec)> = BTreeSet::new(); + for grant in &grants { + let permissions = grant + .permissions + .iter() + .map(|action| action_name(*action)) + .collect::, _>>()?; + selectors.insert((grant.nodes.iter().cloned().collect(), permissions)); } - let definition = json!({"description": definition.description, "actions": grants}); + let definition = json!({ + "description": definition.description, + "grants": selectors.iter().map(|(nodes, permissions)| json!({"nodes": nodes, "permissions": permissions})).collect::>(), + }); let digest = sha256_hex(definition.to_string().as_bytes()); published.insert( key, @@ -71,16 +95,121 @@ pub(crate) fn decode_roles( Ok(published) } -pub(crate) fn decode_grants(value: &Value) -> Result, ControlError> { +pub(crate) fn decode_grants(value: &Value) -> Result, ControlError> { serde_json::from_str(value.as_str().unwrap_or("[]")) .map_err(|error| ControlError::Schema(format!("effective role grants: {error}"))) } -pub(crate) fn grants_allow(value: &Value, action: Action) -> bool { - match (decode_grants(value), action_name(action)) { - (Ok(grants), Ok(name)) => grants.contains(&name), - _ => false, +/// Nodes on which these effective grants allow the action; a grant that resolved to none gives nothing. +pub(crate) fn granted_nodes(value: &Value, action: Action) -> BTreeSet { + let mut nodes: BTreeSet = BTreeSet::new(); + for grant in decode_grants(value).unwrap_or_default() { + if !grant.permissions.contains(&action) { + continue; + } + for selector in &grant.nodes { + nodes.extend( + grant + .resolved_nodes + .get(selector) + .cloned() + .unwrap_or_default(), + ); + } + } + nodes +} + +fn pairs(grants: &[RoleGrant]) -> BTreeSet<(&str, Action)> { + let mut pairs: BTreeSet<(&str, Action)> = BTreeSet::new(); + for grant in grants { + for selector in &grant.nodes { + for action in &grant.permissions { + pairs.insert((selector.as_str(), *action)); + } + } + } + pairs +} + +/// Custom roles whose effective grants add coverage, and requested nodes none covers; built-ins have no grants. +pub(crate) fn custom_coverage( + rows: &[&Row], + scope: &PermissionScope, +) -> (Vec, Vec) { + let mut covered: BTreeSet = BTreeSet::new(); + let mut granting: Vec = Vec::new(); + for row in rows { + let role_key = row["role_key"].as_str().unwrap_or_default(); + let nodes = granted_nodes(&row["resolved_grants_json"], scope.action); + let adds = if scope.nodes.is_empty() { + !nodes.is_empty() && granting.is_empty() + } else { + scope + .nodes + .iter() + .any(|node| nodes.contains(node) && !covered.contains(node)) + }; + if !adds { + continue; + } + if !granting.iter().any(|key| key == role_key) { + granting.push(role_key.into()); + } + covered.extend(nodes.into_iter().filter(|node| scope.nodes.contains(node))); + } + let missing: Vec = scope.nodes.difference(&covered).cloned().collect(); + (granting, missing) +} + +/// A denial names at most twenty uncovered nodes so the audited reason stays bounded. +pub(crate) fn denial_reason(action: Action, missing: &[String]) -> Result { + if missing.is_empty() { + return Ok("missing permission in this scope; all requested nodes denied".into()); + } + let listed: Vec<&str> = missing.iter().take(20).map(String::as_str).collect(); + let more = missing.len() - listed.len(); + let suffix = if more > 0 { + format!(" and {more} more") + } else { + String::new() + }; + Ok(format!( + "missing {} on nodes {}{suffix}; all requested nodes denied", + action_name(action)?, + listed.join(", ") + )) +} + +/// Provably narrowing: every published (selector, permission) pair is already effective. +pub(crate) fn narrows(published: &[RoleGrant], effective: &[RoleGrant]) -> bool { + pairs(published).is_subset(&pairs(effective)) +} + +/// Effective pairs the publication still contains, one grant per selector with its resolution now. +pub(crate) fn retained_grants(published: &[RoleGrant], effective: &[RoleGrant]) -> Vec { + let kept: BTreeSet<(&str, Action)> = pairs(published) + .intersection(&pairs(effective)) + .copied() + .collect(); + let mut grants: BTreeMap<&str, RoleGrant> = BTreeMap::new(); + for (selector, action) in kept { + let resolved = published + .iter() + .find_map(|grant| grant.resolved_nodes.get(selector)) + .cloned() + .unwrap_or_default(); + grants + .entry(selector) + .or_insert_with(|| RoleGrant { + nodes: BTreeSet::from([selector.to_owned()]), + permissions: BTreeSet::new(), + resolved_nodes: BTreeMap::from([(selector.to_owned(), resolved)]), + }) + .permissions + .insert(action); } + grants.into_values().collect() } /// The effective digest and grants of one custom role, if any definition is in effect. @@ -88,7 +217,7 @@ pub(crate) async fn effective_role( transaction: ControlTransaction, (location_id, target): (Uuid, &str), role_key: &str, -) -> Result<(ControlTransaction, Option<(String, BTreeSet)>), ControlError> { +) -> Result<(ControlTransaction, Option<(String, Vec)>), ControlError> { let (transaction, rows) = query(transaction, "SELECT definition_digest,resolved_grants_json FROM sqb_effective_roles WHERE location_id=$1 AND target=$2 AND role_key=$3", vec![Parameter::Id(location_id), Parameter::Text(target.into()), Parameter::Text(role_key.into())]).await?; @@ -136,7 +265,7 @@ pub(crate) async fn current_role( pub(crate) async fn set_effective_role( mut transaction: ControlTransaction, (location_id, target, role_key): (Uuid, &str, &str), - (digest, grants): (&str, &BTreeSet), + (digest, grants): (&str, &[RoleGrant]), ) -> Result { let grants = serde_json::to_string(grants).map_err(|error| ControlError::Invalid(error.to_string()))?; diff --git a/crates/sqlbuild-orchestration/src/authorization/_helpers/role_approvals.rs b/crates/sqlbuild-orchestration/src/authorization/_helpers/role_approvals.rs index 5d6e8ea00..e064e1937 100644 --- a/crates/sqlbuild-orchestration/src/authorization/_helpers/role_approvals.rs +++ b/crates/sqlbuild-orchestration/src/authorization/_helpers/role_approvals.rs @@ -4,7 +4,7 @@ use serde_json::json; use uuid::Uuid; use crate::authorization::_helpers::custom_roles::{ - current_role, effective_role, set_effective_role, + current_role, effective_role, retained_grants, set_effective_role, }; use crate::credentials::main::audit::append_audit::append_audit; use crate::credentials::models::{AuditContext, RoleApproval, RoleDecision}; @@ -81,7 +81,7 @@ pub(crate) async fn record_decision( transaction = set_effective_role( transaction, (location_id, target, role_key), - (digest, &role.grants), + (digest, &retained_grants(&role.grants, &role.grants)), ) .await?; } diff --git a/crates/sqlbuild-orchestration/src/authorization/main/authorize_session.rs b/crates/sqlbuild-orchestration/src/authorization/main/authorize_session.rs index c1f490194..3d7d93add 100644 --- a/crates/sqlbuild-orchestration/src/authorization/main/authorize_session.rs +++ b/crates/sqlbuild-orchestration/src/authorization/main/authorize_session.rs @@ -1,8 +1,8 @@ -//! Live identity and additive built-in grants are evaluated under the operation's store lock. +//! Live identity and the union of role grants over every requested node decide under the store lock. use serde_json::json; -use crate::authorization::_helpers::custom_roles::grants_allow; +use crate::authorization::_helpers::custom_roles::{custom_coverage, denial_reason}; use crate::authorization::main::deny_authorization::deny; use crate::credentials::main::sessions::authenticate_session_in::authenticate_session_in; use crate::credentials::models::{ @@ -11,6 +11,7 @@ use crate::credentials::models::{ use crate::store::errors::ControlError; use crate::store::main::query::query; use crate::store::models::{ControlTransaction, Parameter}; +use crate::store::types::Row; fn role_allows(role: BuiltinRole, action: Action) -> bool { use Action::{ @@ -75,28 +76,30 @@ pub(crate) async fn authorize_session( LEFT JOIN sqb_effective_roles e ON e.location_id=a.location_id AND e.role_key=a.role_key AND e.target=$2 \ WHERE a.principal_id=$1 AND a.revoked_at IS NULL ORDER BY a.role_key", vec![Parameter::Id(identity.principal_id), Parameter::OptionalText(scope.target.clone())]).await?; - for row in rows { - let allowed = serde_json::from_value::(row["role_key"].clone()).map_or_else( - |_| grants_allow(&row["resolved_grants_json"], scope.action), - |role| role_allows(role, scope.action), - ); - if allowed - && (row["location_id"].is_null() || row["location_id"] == json!(scope.location_id)) - && (row["target"].is_null() || row["target"] == json!(scope.target)) - { - context.granting_role = row["role_key"].as_str().unwrap_or_default().into(); - return Ok((transaction, context)); - } + let rows: Vec<&Row> = rows + .iter() + .filter(|row| { + (row["location_id"].is_null() || row["location_id"] == json!(scope.location_id)) + && (row["target"].is_null() || row["target"] == json!(scope.target)) + }) + .collect(); + let builtin = rows.iter().find(|row| { + serde_json::from_value::(row["role_key"].clone()) + .is_ok_and(|role| role_allows(role, scope.action)) + }); + if let Some(row) = builtin { + context.granting_role = row["role_key"].as_str().unwrap_or_default().into(); + return Ok((transaction, context)); + } + let (granting, missing) = custom_coverage(&rows, scope); + if !granting.is_empty() && missing.is_empty() { + context.granting_role = granting.join("+"); + return Ok((transaction, context)); } if default_role == DefaultRole::Viewer && role_allows(BuiltinRole::Viewer, scope.action) { context.granting_role = "default:viewer".into(); return Ok((transaction, context)); } - deny( - transaction, - &context, - json!(scope), - "missing permission in this scope; all requested nodes denied", - ) - .await + let reason = denial_reason(scope.action, &missing)?; + deny(transaction, &context, json!(scope), &reason).await } diff --git a/crates/sqlbuild-orchestration/src/authorization/main/compile_roles.rs b/crates/sqlbuild-orchestration/src/authorization/main/compile_roles.rs new file mode 100644 index 000000000..b42306f39 --- /dev/null +++ b/crates/sqlbuild-orchestration/src/authorization/main/compile_roles.rs @@ -0,0 +1,40 @@ +//! A project's `roles.toml` becomes the snapshot `roles` document, its selectors resolved per snapshot. + +use std::collections::{BTreeMap, BTreeSet}; + +use serde_json::Value; + +use crate::authorization::_helpers::custom_roles::decode_roles; +use crate::credentials::models::RolesFile; + +/// The `roles` document and one warning per grant whose selectors match no node in this snapshot. +pub fn compile_roles( + text: &str, + resolve: impl Fn(&str) -> Result, String>, +) -> Result<(Value, Vec), String> { + let file: RolesFile = toml::from_str(text).map_err(|error| format!("roles.toml: {error}"))?; + let mut roles = file.roles; + let mut warnings: Vec = Vec::new(); + for (key, role) in &mut roles { + for (index, grant) in role.grants.iter_mut().enumerate() { + let mut resolved: BTreeMap> = BTreeMap::new(); + for selector in &grant.nodes { + let nodes = resolve(selector).map_err(|error| { + format!("roles.toml: role {key} selector '{selector}': {error}") + })?; + resolved.insert(selector.clone(), nodes); + } + if resolved.values().all(BTreeSet::is_empty) { + warnings.push(format!( + "role {key} grant {} selects no nodes ({}); it grants nothing in this snapshot", + index + 1, + grant.nodes.iter().cloned().collect::>().join(" ") + )); + } + grant.resolved_nodes = resolved; + } + } + let document = serde_json::to_value(&roles).map_err(|error| error.to_string())?; + decode_roles(Some(&document)).map_err(|error| format!("roles.toml: {error}"))?; + Ok((document, warnings)) +} diff --git a/crates/sqlbuild-orchestration/src/authorization/main/mod.rs b/crates/sqlbuild-orchestration/src/authorization/main/mod.rs index 0043c0b01..95c0738e9 100644 --- a/crates/sqlbuild-orchestration/src/authorization/main/mod.rs +++ b/crates/sqlbuild-orchestration/src/authorization/main/mod.rs @@ -2,6 +2,7 @@ pub mod assign_builtin_role; pub(crate) mod authorize_session; pub mod bootstrap_user; pub mod change_administrator; +pub mod compile_roles; pub mod decide_role_approval; pub(crate) mod deny_authorization; pub(crate) mod publish_custom_roles; diff --git a/crates/sqlbuild-orchestration/src/authorization/main/publish_custom_roles.rs b/crates/sqlbuild-orchestration/src/authorization/main/publish_custom_roles.rs index 8c001a1f6..468691f37 100644 --- a/crates/sqlbuild-orchestration/src/authorization/main/publish_custom_roles.rs +++ b/crates/sqlbuild-orchestration/src/authorization/main/publish_custom_roles.rs @@ -1,11 +1,9 @@ -//! Record each published custom role; narrowing and removal apply now, widening waits for approval. - -use std::collections::BTreeSet; +//! Record each published custom role; removed pairs and selector coverage apply now, additions wait. use serde_json::Value; use crate::authorization::_helpers::custom_roles::{ - decode_roles, effective_role, set_effective_role, + decode_roles, effective_role, narrows, retained_grants, set_effective_role, }; use crate::snapshots::models::SnapshotRecord; use crate::store::errors::ControlError; @@ -55,21 +53,15 @@ pub(crate) async fn publish_custom_roles( let Some((digest, grants)) = effective else { continue; }; - if role.grants.is_subset(&grants) { - if digest != role.digest || grants != role.grants { - transaction = set_effective_role( - transaction, - (scope.0, scope.1, key), - (&role.digest, &role.grants), - ) - .await?; - } - continue; - } - let narrowed: BTreeSet = grants.intersection(&role.grants).cloned().collect(); - if narrowed != grants { + let next = retained_grants(&role.grants, &grants); + let next_digest = if narrows(&role.grants, &grants) { + role.digest.as_str() + } else { + digest.as_str() + }; + if next != grants || next_digest != digest { transaction = - set_effective_role(transaction, (scope.0, scope.1, key), (&digest, &narrowed)) + set_effective_role(transaction, (scope.0, scope.1, key), (next_digest, &next)) .await?; } } diff --git a/crates/sqlbuild-orchestration/src/authorization/main/request_role_activation.rs b/crates/sqlbuild-orchestration/src/authorization/main/request_role_activation.rs index 990452cea..1dccea4ba 100644 --- a/crates/sqlbuild-orchestration/src/authorization/main/request_role_activation.rs +++ b/crates/sqlbuild-orchestration/src/authorization/main/request_role_activation.rs @@ -3,7 +3,7 @@ use serde_json::json; use uuid::Uuid; -use crate::authorization::_helpers::custom_roles::{current_role, effective_role}; +use crate::authorization::_helpers::custom_roles::{current_role, effective_role, narrows}; use crate::authorization::main::authorize_session::authorize_session; use crate::credentials::main::audit::append_audit::append_audit; use crate::credentials::models::{ @@ -46,7 +46,7 @@ pub async fn request_role_activation( let (transaction, effective) = effective_role(transaction, scope, &request.role_key).await?; if effective .as_ref() - .is_some_and(|(_, grants)| role.grants.is_subset(grants)) + .is_some_and(|(_, grants)| narrows(&role.grants, grants)) { return Err(ControlError::Invalid(format!( "role {} does not widen its effective definition; narrowing applies on publish", diff --git a/crates/sqlbuild-orchestration/src/credentials/models.rs b/crates/sqlbuild-orchestration/src/credentials/models.rs index 8bc943421..cdb5aaef6 100644 --- a/crates/sqlbuild-orchestration/src/credentials/models.rs +++ b/crates/sqlbuild-orchestration/src/credentials/models.rs @@ -87,7 +87,9 @@ impl Default for AuditRetention { } /// Closed permission vocabulary; unknown actions fail deserialization. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] +#[derive( + Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, utoipa::ToSchema, +)] pub enum Action { #[serde(rename = "view")] View, @@ -150,19 +152,37 @@ impl From for RoleKey { } } -/// One custom role entry of a published `roles` document; only non-administrative actions. -#[derive(Clone, Debug, Deserialize)] +/// One grant of a project role: node selectors, their permissions, and each selector's nodes. +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct RoleGrant { + pub nodes: std::collections::BTreeSet, + pub permissions: std::collections::BTreeSet, + #[serde(default)] + pub resolved_nodes: std::collections::BTreeMap>, +} + +/// One project role of a `roles.toml` file or a published `roles` document. +#[derive(Clone, Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] -pub(crate) struct RoleDefinition { - pub(crate) description: String, - pub(crate) actions: Vec, +pub struct RoleDefinition { + pub description: String, + pub grants: Vec, +} + +/// The reviewed project-root `roles.toml` file. +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +pub(crate) struct RolesFile { + #[serde(default)] + pub(crate) roles: std::collections::BTreeMap, } -/// A validated custom role definition with its content digest and granted action names. +/// A validated custom role definition with its selector digest and resolved grants. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct PublishedRole { pub(crate) digest: String, - pub(crate) grants: std::collections::BTreeSet, + pub(crate) grants: Vec, pub(crate) definition: serde_json::Value, } diff --git a/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs b/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs index 75989abd3..47bfe415c 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs @@ -59,7 +59,7 @@ pub(crate) fn credentials(session: &IssuedSession) -> (&str, &str) { (session.bearer.as_str(), session.csrf.as_str()) } -async fn assign( +pub(crate) async fn assign( store: &ControlStore, fixture: (&AuditContext, &IssuedSession), request: RoleAssignmentRequest, @@ -76,17 +76,27 @@ async fn assign( ); } -/// Publish one generation whose `roles` document grants the support role these actions. +/// One grant over the `tag:orders` selector, resolved to these snapshot nodes. +pub(crate) fn orders_grant(permissions: Value, resolved_nodes: &[&str]) -> Value { + json!({"nodes": ["tag:orders"], "permissions": permissions, "resolved_nodes": {"tag:orders": resolved_nodes}}) +} + +/// Ad-hoc and SQL permissions over one selector, resolved to these snapshot nodes. +pub(crate) fn developer_grants(selector: &str, resolved_nodes: &[&str]) -> Value { + json!([{"nodes": [selector], "permissions": ["run.adhoc", "view.sql"], "resolved_nodes": {selector: resolved_nodes}}]) +} + +/// Publish one generation whose `roles` document gives the support role these grants. pub(crate) async fn publish_support_role( store: &ControlStore, location_id: Uuid, - (generation, actions): (i64, Value), + (generation, grants): (i64, Value), ) { let mut input = snapshot(location_id); input.generation = generation; input.documents.insert( "roles".into(), - json!({SUPPORT_ROLE: {"description": "Answer order questions", "actions": actions}}), + json!({SUPPORT_ROLE: {"description": "Answer order questions", "grants": grants}}), ); publish_snapshot(store, &input) .await @@ -132,7 +142,12 @@ pub(crate) async fn role_fixture(store: &ControlStore, administrators: usize) -> let location_id = create_location(store, "orders", "orders-admin") .await .expect("location"); - publish_support_role(store, location_id, (1, json!(["view"]))).await; + publish_support_role( + store, + location_id, + (1, json!([orders_grant(json!(["view"]), &["orders"])])), + ) + .await; assign( store, (&context, &sessions[0]), @@ -154,17 +169,17 @@ pub(crate) async fn role_fixture(store: &ControlStore, administrators: usize) -> } } -/// The role that grants the member this action in the fixture location, or `None` when denied. -pub(crate) async fn granting_role( +/// The role that grants the member this action on these nodes, or the denial reason. +pub(crate) async fn authorize_member( store: &ControlStore, fixture: &RoleFixture, - action: Action, -) -> Option { + (action, nodes): (Action, &[&str]), +) -> Result { let scope = PermissionScope { action, location_id: Some(fixture.location_id), target: Some("development".into()), - nodes: Default::default(), + nodes: nodes.iter().map(|node| (*node).into()).collect(), }; let decision = authorize_session( begin(store, TransactionMode::Write).await.expect("begin"), @@ -180,7 +195,26 @@ pub(crate) async fn granting_role( .is_none_or(|error| matches!(error, ControlError::Denied { .. })), "authorization either grants or denies" ); - decision.ok().map(|(_, context)| context.granting_role) + decision + .map(|(_, context)| context.granting_role) + .map_err(|error| { + let reason = error.to_string(); + let reason = reason.trim_start_matches("control-plane operation denied: "); + reason + .split("; decision ") + .next() + .unwrap_or_default() + .to_owned() + }) +} + +/// The role that grants the member this location-scoped action, or `None` when denied. +pub(crate) async fn granting_role( + store: &ControlStore, + fixture: &RoleFixture, + action: Action, +) -> Option { + authorize_member(store, fixture, (action, &[])).await.ok() } /// Role approval audit facts in commit order: action and recorded outcome. @@ -205,11 +239,11 @@ pub(crate) async fn role_audit(store: &ControlStore) -> Vec<(String, Value)> { .collect() } -pub(crate) async fn request( +pub(crate) async fn try_request( store: &ControlStore, fixture: &RoleFixture, requester: usize, -) -> RoleActivation { +) -> Result { request_role_activation( store, ( @@ -224,7 +258,26 @@ pub(crate) async fn request( }, ) .await - .expect("activation request") +} + +pub(crate) async fn request( + store: &ControlStore, + fixture: &RoleFixture, + requester: usize, +) -> RoleActivation { + try_request(store, fixture, requester) + .await + .expect("activation request") +} + +/// Request and approve the support role's current definition through a second administrator. +pub(crate) async fn activate(store: &ControlStore, fixture: &RoleFixture) { + let activation = request(store, fixture, 0).await; + assert!( + approve(store, fixture, (1, &activation, false)) + .await + .expect("approve") + ); } pub(crate) async fn approve( diff --git a/crates/sqlbuild-orchestration/tests/credentials/test_contracts.rs b/crates/sqlbuild-orchestration/tests/credentials/test_contracts.rs index d8c431a13..982ae4233 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/test_contracts.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/test_contracts.rs @@ -18,6 +18,8 @@ mod test_contract; mod test_custom_roles; #[path = "passwords/test_contract.rs"] mod test_passwords; +#[path = "test_project_roles.rs"] +mod test_project_roles; #[path = "test_recovery.rs"] mod test_recovery; #[path = "sessions/test_contract.rs"] diff --git a/crates/sqlbuild-orchestration/tests/credentials/test_custom_roles.rs b/crates/sqlbuild-orchestration/tests/credentials/test_custom_roles.rs index aa7f96c55..d7de4eaf7 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/test_custom_roles.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/test_custom_roles.rs @@ -4,7 +4,8 @@ use serde_json::json; use crate::credentials::models::Action; use crate::credentials::tests::role_helpers::{ - SUPPORT_ROLE, approve, granting_role, publish_support_role, request, role_audit, role_fixture, + SUPPORT_ROLE, approve, granting_role, orders_grant, publish_support_role, request, role_audit, + role_fixture, }; use crate::credentials::tests::test_types::CustomRoleTestCase; use crate::store::errors::ControlError; @@ -44,7 +45,15 @@ fn given_widened_custom_role_when_unapproved_then_new_grants_stay_refused() { approve(store, &roles, (1, &activation, false)) .await .expect("first approval"); - publish_support_role(store, roles.location_id, (2, json!(["view", "job.run"]))).await; + publish_support_role( + store, + roles.location_id, + ( + 2, + json!([orders_grant(json!(["view", "job.run"]), &["orders"])]), + ), + ) + .await; assert_eq!( granting_role(store, &roles, Action::JobRun) .await diff --git a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs new file mode 100644 index 000000000..9035183b0 --- /dev/null +++ b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs @@ -0,0 +1,483 @@ +//! Project role grants follow selector coverage per snapshot; a rewritten selector waits for approval. + +use std::collections::{BTreeMap, BTreeSet}; + +use serde_json::{Value, json}; + +use crate::authorization::_helpers::custom_roles::set_effective_role; +use crate::authorization::main::compile_roles::compile_roles; +use crate::credentials::models::{Action, RoleAssignmentRequest, RoleGrant, RoleKey}; +use crate::credentials::tests::role_helpers::{ + SUPPORT_ROLE, activate, assign, authorize_member, developer_grants, orders_grant, + publish_support_role, role_fixture, try_request, +}; +use crate::credentials::tests::test_types::{ProjectRoleTestCase, RolesFileTestCase}; +use crate::runs::tests::helpers::snapshot; +use crate::snapshots::main::publish_snapshot::publish_snapshot; +use crate::store::errors::ControlError; +use crate::store::main::{begin::begin, commit::commit, migrate_store::migrate_store}; +use crate::store::models::TransactionMode; +use crate::store::tests::store_helpers::{close_test_store, open_test_store, run}; +use crate::store::tests::store_test_types::Backend; + +const ADHOC_DENIED: &str = "missing run.adhoc on nodes customers; all requested nodes denied"; + +#[test] +fn given_approved_selector_when_it_matches_more_nodes_then_coverage_grows_without_approval() { + let test_cases = [ + ProjectRoleTestCase { + description: "SQLite widens coverage of an unchanged selector on publish", + backend: Backend::Sqlite, + expected_before: Err(ADHOC_DENIED), + expected_after: Ok(SUPPORT_ROLE), + }, + ProjectRoleTestCase { + description: "PostgreSQL widens coverage of an unchanged selector on publish", + backend: Backend::Postgres, + expected_before: Err(ADHOC_DENIED), + expected_after: Ok(SUPPORT_ROLE), + }, + ]; + for test_case in test_cases { + run(async { + let Some(fixture) = open_test_store(test_case.backend).await else { + return; + }; + let store = &fixture.store; + migrate_store(store).await.expect("migrate"); + let roles = role_fixture(store, 2).await; + activate(store, &roles).await; + let grants = |nodes: &[&str]| developer_grants("tag:orders", nodes); + publish_support_role(store, roles.location_id, (2, grants(&["orders"]))).await; + activate(store, &roles).await; + let adhoc = (Action::RunAdhoc, ["customers", "orders"].as_slice()); + assert_eq!( + authorize_member(store, &roles, adhoc) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_before, + "{}: an ad-hoc run needs every written node", + test_case.description + ); + publish_support_role( + store, + roles.location_id, + (3, grants(&["customers", "orders"])), + ) + .await; + assert_eq!( + authorize_member(store, &roles, adhoc) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_after, + "{}", + test_case.description + ); + assert_eq!( + authorize_member(store, &roles, (Action::ViewSql, &["customers"])) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_after, + "{}: view.sql follows the same coverage", + test_case.description + ); + assert!( + matches!( + try_request(store, &roles, 0).await, + Err(ControlError::Invalid(_)) + ), + "{}: coverage growth is not a widening to approve", + test_case.description + ); + close_test_store(fixture).await; + }); + } +} + +#[test] +fn given_rewritten_selector_when_published_then_its_coverage_waits_for_approval() { + let test_cases = [ + ProjectRoleTestCase { + description: "SQLite holds a rewritten selector for approval", + backend: Backend::Sqlite, + expected_before: Err( + "missing run.adhoc on nodes customers, orders; all requested nodes denied", + ), + expected_after: Ok(SUPPORT_ROLE), + }, + ProjectRoleTestCase { + description: "PostgreSQL holds a rewritten selector for approval", + backend: Backend::Postgres, + expected_before: Err( + "missing run.adhoc on nodes customers, orders; all requested nodes denied", + ), + expected_after: Ok(SUPPORT_ROLE), + }, + ]; + for test_case in test_cases { + run(async { + let Some(fixture) = open_test_store(test_case.backend).await else { + return; + }; + let store = &fixture.store; + migrate_store(store).await.expect("migrate"); + let roles = role_fixture(store, 2).await; + activate(store, &roles).await; + publish_support_role( + store, + roles.location_id, + (2, developer_grants("tag:orders", &["orders"])), + ) + .await; + activate(store, &roles).await; + publish_support_role( + store, + roles.location_id, + (3, developer_grants("tag:support", &["customers", "orders"])), + ) + .await; + let adhoc = (Action::RunAdhoc, ["customers", "orders"].as_slice()); + assert_eq!( + authorize_member(store, &roles, adhoc) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_before, + "{}: the rewritten selector grants nothing before approval", + test_case.description + ); + assert_eq!( + authorize_member(store, &roles, (Action::RunAdhoc, &["orders"])) + .await + .as_deref() + .map_err(String::as_str), + Err("missing run.adhoc on nodes orders; all requested nodes denied"), + "{}: the replaced selector stops applying at once", + test_case.description + ); + activate(store, &roles).await; + assert_eq!( + authorize_member(store, &roles, adhoc) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_after, + "{}", + test_case.description + ); + close_test_store(fixture).await; + }); + } +} + +#[test] +fn given_selector_resolving_to_no_nodes_when_approved_then_it_grants_nothing() { + let test_cases = [ + ProjectRoleTestCase { + description: "SQLite grants nothing for an empty resolution", + backend: Backend::Sqlite, + expected_before: Ok(SUPPORT_ROLE), + expected_after: Err("missing permission in this scope; all requested nodes denied"), + }, + ProjectRoleTestCase { + description: "PostgreSQL grants nothing for an empty resolution", + backend: Backend::Postgres, + expected_before: Ok(SUPPORT_ROLE), + expected_after: Err("missing permission in this scope; all requested nodes denied"), + }, + ]; + for test_case in test_cases { + run(async { + let Some(fixture) = open_test_store(test_case.backend).await else { + return; + }; + let store = &fixture.store; + migrate_store(store).await.expect("migrate"); + let roles = role_fixture(store, 2).await; + activate(store, &roles).await; + assert_eq!( + authorize_member(store, &roles, (Action::View, &[])) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_before, + "{}: the resolved grant allows view", + test_case.description + ); + publish_support_role( + store, + roles.location_id, + (2, json!([orders_grant(json!(["view", "run.adhoc"]), &[])])), + ) + .await; + activate(store, &roles).await; + assert_eq!( + authorize_member(store, &roles, (Action::View, &[])) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_after, + "{}: a location-scoped action needs a grant that resolved to a node", + test_case.description + ); + assert_eq!( + authorize_member(store, &roles, (Action::RunAdhoc, &["orders"])) + .await + .as_deref() + .map_err(String::as_str), + Err("missing run.adhoc on nodes orders; all requested nodes denied"), + "{}: the denial names the uncovered node", + test_case.description + ); + close_test_store(fixture).await; + }); + } +} + +#[test] +fn given_removal_and_rewrite_in_one_publish_when_widening_waits_then_the_removal_applies_at_once() { + let test_cases = [ + ProjectRoleTestCase { + description: "SQLite removes a permission while a new selector waits", + backend: Backend::Sqlite, + expected_before: Err("missing run.adhoc on nodes orders; all requested nodes denied"), + expected_after: Err("missing view.sql on nodes customers; all requested nodes denied"), + }, + ProjectRoleTestCase { + description: "PostgreSQL removes a permission while a new selector waits", + backend: Backend::Postgres, + expected_before: Err("missing run.adhoc on nodes orders; all requested nodes denied"), + expected_after: Err("missing view.sql on nodes customers; all requested nodes denied"), + }, + ]; + for test_case in test_cases { + run(async { + let Some(fixture) = open_test_store(test_case.backend).await else { + return; + }; + let store = &fixture.store; + migrate_store(store).await.expect("migrate"); + let roles = role_fixture(store, 2).await; + activate(store, &roles).await; + publish_support_role( + store, + roles.location_id, + (2, developer_grants("tag:orders", &["orders"])), + ) + .await; + activate(store, &roles).await; + publish_support_role( + store, + roles.location_id, + ( + 3, + json!([ + orders_grant(json!(["view.sql"]), &["orders"]), + {"nodes": ["tag:support"], "permissions": ["view.sql"], + "resolved_nodes": {"tag:support": ["customers"]}}, + ]), + ), + ) + .await; + assert_eq!( + authorize_member(store, &roles, (Action::RunAdhoc, &["orders"])) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_before, + "{}: the removed permission is denied at once", + test_case.description + ); + assert_eq!( + authorize_member(store, &roles, (Action::ViewSql, &["customers"])) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_after, + "{}: the rewritten selector's coverage waits for approval", + test_case.description + ); + assert_eq!( + authorize_member(store, &roles, (Action::ViewSql, &["orders"])) + .await + .as_deref() + .map_err(String::as_str), + Ok(SUPPORT_ROLE), + "{}: the kept pair stays effective", + test_case.description + ); + activate(store, &roles).await; + assert_eq!( + authorize_member(store, &roles, (Action::ViewSql, &["customers", "orders"])) + .await + .as_deref() + .map_err(String::as_str), + Ok(SUPPORT_ROLE), + "{}: approval adds the new coverage", + test_case.description + ); + close_test_store(fixture).await; + }); + } +} + +#[test] +fn given_two_custom_roles_when_each_covers_part_of_a_run_then_their_union_authorizes_it() { + let test_cases = [ + ProjectRoleTestCase { + description: "SQLite authorizes the union of two custom roles", + backend: Backend::Sqlite, + expected_before: Err(ADHOC_DENIED), + expected_after: Ok("orders_returns+orders_support"), + }, + ProjectRoleTestCase { + description: "PostgreSQL authorizes the union of two custom roles", + backend: Backend::Postgres, + expected_before: Err(ADHOC_DENIED), + expected_after: Ok("orders_returns+orders_support"), + }, + ]; + for test_case in test_cases { + run(async { + let Some(fixture) = open_test_store(test_case.backend).await else { + return; + }; + let store = &fixture.store; + migrate_store(store).await.expect("migrate"); + let roles = role_fixture(store, 2).await; + activate(store, &roles).await; + let mut input = snapshot(roles.location_id); + input.generation = 2; + input.documents.insert( + "roles".into(), + json!({ + SUPPORT_ROLE: {"description": "Answer order questions", "grants": developer_grants("tag:orders", &["orders"])}, + "orders_returns": {"description": "Handle returns", "grants": developer_grants("tag:returns", &["customers"])}, + }), + ); + publish_snapshot(store, &input) + .await + .expect("publish roles"); + activate(store, &roles).await; + let adhoc = (Action::RunAdhoc, ["customers", "orders"].as_slice()); + assert_eq!( + authorize_member(store, &roles, adhoc) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_before, + "{}: one role alone does not cover the run", + test_case.description + ); + let returns = serde_json::from_value::>(developer_grants( + "tag:returns", + &["customers"], + )) + .expect("returns grants"); + let transaction = set_effective_role( + begin(store, TransactionMode::Write).await.expect("begin"), + (roles.location_id, "development", "orders_returns"), + ("synthetic-returns-digest", &returns), + ) + .await + .expect("effective returns role"); + commit(transaction).await.expect("commit"); + assign( + store, + (&roles.context, &roles.administrators[0]), + RoleAssignmentRequest { + principal_id: roles.member_id, + role: RoleKey::Custom("orders_returns".into()), + location_id: Some(roles.location_id), + target: None, + revoke: false, + }, + ) + .await; + assert_eq!( + authorize_member(store, &roles, adhoc) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_after, + "{}", + test_case.description + ); + close_test_store(fixture).await; + }); + } +} + +#[test] +fn given_roles_file_when_compiled_then_selectors_resolve_and_empty_grants_warn() { + let roles = r#" +[roles.orders_support] +description = "Answer order questions" +[[roles.orders_support.grants]] +nodes = ["tag:orders"] +permissions = ["view.sql", "run.adhoc"] +[[roles.orders_support.grants]] +nodes = ["tag:orders,tag:refunds"] +permissions = ["view.evidence"] +"#; + let test_cases = [ + RolesFileTestCase { + description: "resolved grants and a warning for the empty intersection", + text: roles, + expected_compiled: Ok(( + json!({"orders_support": {"description": "Answer order questions", "grants": [ + {"nodes": ["tag:orders"], "permissions": ["view.sql", "run.adhoc"], + "resolved_nodes": {"tag:orders": ["model:orders", "model:returns"]}}, + {"nodes": ["tag:orders,tag:refunds"], "permissions": ["view.evidence"], + "resolved_nodes": {"tag:orders,tag:refunds": []}}, + ]}}), + vec![ + "role orders_support grant 2 selects no nodes (tag:orders,tag:refunds); it grants nothing in this snapshot", + ], + )), + }, + RolesFileTestCase { + description: "an administrative permission is never granted from code", + text: "[roles.orders_support]\ndescription = \"Answer order questions\"\n[[roles.orders_support.grants]]\nnodes = [\"tag:orders\"]\npermissions = [\"role.assign\"]\n", + expected_compiled: Err( + "roles.toml: invalid control-plane data: role orders_support needs a description, a key that is not a built-in role, and grants of node selectors with only non-administrative permissions", + ), + }, + RolesFileTestCase { + description: "a selector the project rejects fails publication", + text: "[roles.orders_support]\ndescription = \"Answer order questions\"\n[[roles.orders_support.grants]]\nnodes = [\"tag:unknown\"]\npermissions = [\"view.sql\"]\n", + expected_compiled: Err( + "roles.toml: role orders_support selector 'tag:unknown': no models found with tag 'unknown'", + ), + }, + ]; + let catalog: BTreeMap<&str, Result, &str>> = BTreeMap::from([ + ("tag:orders", Ok(vec!["model:orders", "model:returns"])), + ("tag:orders,tag:refunds", Ok(vec![])), + ("tag:unknown", Err("no models found with tag 'unknown'")), + ]); + let resolve = |selector: &str| { + catalog[selector] + .clone() + .map(|keys| keys.into_iter().map(str::to_owned).collect::>()) + .map_err(str::to_owned) + }; + for test_case in test_cases { + let compiled = compile_roles(test_case.text, resolve); + assert_eq!( + compiled + .as_ref() + .map(|(document, warnings)| ( + document.clone(), + warnings.iter().map(String::as_str).collect::>() + )) + .map_err(String::as_str), + test_case.expected_compiled, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-orchestration/tests/credentials/test_types.rs b/crates/sqlbuild-orchestration/tests/credentials/test_types.rs index 7eb14ccc5..955833e33 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/test_types.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/test_types.rs @@ -55,3 +55,16 @@ pub(crate) struct RoleFixture { pub(crate) member: crate::credentials::models::IssuedSession, pub(crate) member_id: uuid::Uuid, } + +pub(crate) struct ProjectRoleTestCase { + pub(crate) description: &'static str, + pub(crate) backend: Backend, + pub(crate) expected_before: Result<&'static str, &'static str>, + pub(crate) expected_after: Result<&'static str, &'static str>, +} + +pub(crate) struct RolesFileTestCase { + pub(crate) description: &'static str, + pub(crate) text: &'static str, + pub(crate) expected_compiled: Result<(Value, Vec<&'static str>), &'static str>, +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/mod.rs b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/mod.rs index 2101d9ab7..5949e9aa3 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/mod.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/mod.rs @@ -10,5 +10,6 @@ pub(crate) mod snapshot_declarations; pub(crate) mod snapshot_execution; pub(crate) mod snapshot_models; pub(crate) mod snapshot_resources; +pub(crate) mod snapshot_roles; pub(crate) mod snapshot_worker; pub(crate) mod values; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/snapshot_roles.rs b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/snapshot_roles.rs new file mode 100644 index 000000000..4867f861a --- /dev/null +++ b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/snapshot_roles.rs @@ -0,0 +1,52 @@ +//! Project roles from the reviewed `roles.toml` file, their selectors resolved against this snapshot. + +use std::collections::BTreeSet; +use std::path::Path; + +use pyo3::exceptions::PyValueError; +use pyo3::prelude::{PyResult, Python}; +use sqlbuild_analysis::graph::main::resolve_selectors::resolve_selectors; +use sqlbuild_analysis::graph::models::ProjectGraph; +use sqlbuild_orchestration::authorization::main::compile_roles::compile_roles; +use sqlbuild_orchestration::snapshots::models::SnapshotInput; + +use crate::bindings::_helpers::native_host::host::write_console; + +/// The project-root file that defines project roles. +pub(crate) const ROLES_FILE: &str = "roles.toml"; + +/// Node keys of one grant selector, in the job selection grammar. +fn selector_nodes(graph: &ProjectGraph, selector: &str) -> Result, String> { + let tokens: Vec = selector.split_whitespace().map(str::to_owned).collect(); + let keys = resolve_selectors(graph, &tokens, &[]).map_err(|error| error.message)?; + let mut nodes: BTreeSet = BTreeSet::new(); + for (kind, name) in keys { + nodes.insert(format!("{kind}:{name}")); + } + Ok(nodes) +} + +pub(crate) fn compile_project_roles( + py: Python<'_>, + mut input: SnapshotInput, + (project_dir, graph): (&Path, &ProjectGraph), +) -> PyResult { + let path = project_dir.join(ROLES_FILE); + if !path.is_file() { + return Ok(input); + } + if input.documents.contains_key("roles") { + return Err(PyValueError::new_err( + "roles.toml cannot overwrite a snapshot roles document", + )); + } + let text = std::fs::read_to_string(&path) + .map_err(|error| PyValueError::new_err(format!("roles.toml: {error}")))?; + let (document, warnings) = compile_roles(&text, |selector| selector_nodes(graph, selector)) + .map_err(PyValueError::new_err)?; + for warning in warnings { + write_console(py, "stderr", &format!("Warning: {warning}\n"))?; + } + input.documents.insert("roles".into(), document); + Ok(input) +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/location_command/snapshot.rs b/crates/sqlbuild-python/src/bindings/_helpers/location_command/snapshot.rs index f975aa24e..e5e702896 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/location_command/snapshot.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/location_command/snapshot.rs @@ -17,6 +17,7 @@ use crate::bindings::_helpers::compiled_project::snapshot_declarations::{ }; use crate::bindings::_helpers::compiled_project::snapshot_models::model_nodes; use crate::bindings::_helpers::compiled_project::snapshot_resources::resource_nodes; +use crate::bindings::_helpers::compiled_project::snapshot_roles::compile_project_roles; use crate::bindings::_helpers::compiled_views::project_view::NativeProjectView; use crate::bindings::_helpers::native_host::orchestration_declarations::collect_declarations; use crate::bindings::_helpers::native_host::orchestration_environment::environment_hash; @@ -67,9 +68,9 @@ pub(crate) fn build_orchestration_snapshot( Some(declarations) => declarations, None => discover_declarations(py, &project_dir)?, }; + let graph = project.lineage_graph(py)?; if !declarations.is_empty() { let authored = collect_declarations(py, declarations)?; - let graph = project.lineage_graph(py)?; input = compile_declarations( py, input, @@ -77,6 +78,7 @@ pub(crate) fn build_orchestration_snapshot( authored, )?; } + input = compile_project_roles(py, input, (&project_dir, graph.bind(py).get().graph()))?; input.nodes = identity_nodes(project.into_pyobject(py)?.as_any(), input.nodes)?; Ok(NativeOrchestrationSnapshot { input }) } diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md index e161ddaf8..0efe61895 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md @@ -10,6 +10,7 @@ Online: https://sqlbuild.com/docs/cli/server/ - Set retention - Inspect published SQL +- Define project roles - Repair control views - Handle a refused repair @@ -84,7 +85,8 @@ stderr; a failed request prints no SQL to stdout. Permission denials include the The equivalent person-authenticated API is a `GET` request to `/api/control/locations/{location_id}/targets/{target}/nodes/{node_key}/sql`. -Developer or Admin access must cover the requested location and target. Viewer, Operator and +Developer or Admin access must cover the requested location and target, or a +[project role](#define-project-roles) must grant `view.sql` on the node. Viewer, Operator and the configurable default Viewer role do not grant SQL inspection. The response contains the snapshot ID, exact node key, raw SQL and compiled SQL. Missing SQL fields are `null`; an absent current publication or node returns JSON `null` after authorization. @@ -94,6 +96,39 @@ data. Every successful inspection records an audit entry without copying SQL int audit cannot be saved, the request fails without returning SQL. Revocation takes effect on the next request, including sessions created by [`sqb login`](login.md). +## Define project roles + +Put project roles in `roles.toml` at the project root. `sqb location publish` resolves each +grant's node selectors against the published project: + +```toml +[roles.orders_support] +description = "Answer order questions" + +[[roles.orders_support.grants]] +nodes = ["tag:orders"] +permissions = ["view.sql", "run.adhoc"] + +[[roles.orders_support.grants]] +nodes = ["tag:orders,tag:pii"] +permissions = ["view.evidence"] +``` + +| Rule | Behaviour | +| --- | --- | +| Selectors | Same grammar as a job's `select`; an unknown tag or name fails publication | +| No matching nodes | Publication succeeds with a warning; the grant gives nothing | +| Permissions | Any non-administrative action; `location.manage`, `user.manage`, `role.assign`, `token.manage` and `audit.view` are rejected | +| Runs | `job.run`, `run.adhoc`, `run.rerun` and `run.rewrite_history` need the permission on every node the run writes | +| Several roles | Grants add up across a person's roles | + +Assign a project role with `POST /api/admin/roles`, using the role key as `role`. A first +definition, and any publication that adds a selector and permission pair, takes effect only after +an administrator requests it with `POST /api/admin/role-activations` and another administrator +approves it with `POST /api/admin/role-approvals`. A sole administrator must confirm a +self-approval. Removed pairs stop applying on publish. An unchanged selector covers new +matching nodes on publish without approval. A denial lists the nodes the request was missing. + ## Repair control views Repair snapshot selection, jobs, triggers, runs, node status and the structural catalog from diff --git a/tests/integration/src/sqlbuild/compiler/compiled_project/publication/_test_types.py b/tests/integration/src/sqlbuild/compiler/compiled_project/publication/_test_types.py index c9866865b..0f0eef726 100644 --- a/tests/integration/src/sqlbuild/compiler/compiled_project/publication/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/compiled_project/publication/_test_types.py @@ -325,3 +325,11 @@ class SchedulerCrashTestCase: fixture: str expected_anomalies: dict[str, int] expected_minimum_kills: int + + +@dataclass(frozen=True) +class ProjectRolesTestCase: + description: str + roles_file: str + expected_warning: str + expected_denial: str diff --git a/tests/integration/src/sqlbuild/compiler/compiled_project/publication/helpers.py b/tests/integration/src/sqlbuild/compiler/compiled_project/publication/helpers.py index 28fb6f479..701cb3acc 100644 --- a/tests/integration/src/sqlbuild/compiler/compiled_project/publication/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/compiled_project/publication/helpers.py @@ -1221,3 +1221,55 @@ def scheduler_crash_history(directory: Path, fixture: dict[str, Any]) -> dict[st "ticks_before_survivor": interrupted, "ticks": len(ticks), } + + +def publish_generation( + project: Path, api: SnapshotApi, generation: int +) -> subprocess.CompletedProcess[str]: + """Publish one generation of a project through the installed location command.""" + return subprocess.run( + [ + str(Path(sys.executable).with_name("sqb")), + "location", + "publish", + "--project-dir", + str(project), + "--location-id", + api.location_id, + "--target", + "development", + "--server", + api.url, + "--generation", + str(generation), + ], + env=os.environ | {"SQLBUILD_LOCATION_TOKEN": api.bearer}, + capture_output=True, + text=True, + timeout=60, + check=False, + ) + + +def read_person_sql(api: SnapshotApi, session: tuple[str, str], node: str) -> tuple[int, str]: + """A person's view.sql read of one node: the node key on success, else the denial.""" + path: str = f"/api/control/locations/{api.location_id}/targets/development/nodes/{node}/sql" + headers: dict[str, str] = { + "Host": "control.example.com", + "Origin": "https://control.example.com", + "Cookie": session[0], + "x-sqb-csrf": session[1], + } + try: + with urlopen(Request(api.url + path, headers=headers), timeout=5) as response: + return 200, json.load(response)["node_key"] + except HTTPError as denied: + return denied.code, json.load(denied)["error"].split("; decision ")[0] + + +def tagged_model(name: str, tags: str) -> str: + """A table model with these tags.""" + return ( + f"MODEL (description 'Synthetic {name}.', materialized table, tags [{tags}]);\n" + "select 1 as id\n" + ) diff --git a/tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_project_roles.py b/tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_project_roles.py new file mode 100644 index 000000000..60ceebe19 --- /dev/null +++ b/tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_project_roles.py @@ -0,0 +1,138 @@ +"""Installed publication compiles roles.toml; person grants follow selector coverage per snapshot.""" + +import subprocess +from pathlib import Path +from typing import Any +from urllib.error import HTTPError + +import pytest + +from tests.integration.src.sqlbuild.compiler.compiled_project.publication._test_types import ( + ProjectRolesTestCase, +) +from tests.integration.src.sqlbuild.compiler.compiled_project.publication.helpers import ( + publish_generation, + read_person_sql, + snapshot_api, + tagged_model, +) +from tests.integration.src.sqlbuild.compiler.compiled_project.publication.person.helpers import ( + person_login, + person_request, + provision_person, +) +from tests.integration.src.sqlbuild.compiler.graph.helpers import write_files + + +@pytest.mark.parametrize( + "test_case", + [ + ProjectRolesTestCase( + description="roles.toml grants follow tags, rewrites wait, removals apply at once", + roles_file="benchmarks/duckshop/edge_cases/orchestration/project_roles.toml", + expected_warning=( + "Warning: role orders_support grant 2 selects no nodes (tag:support,tag:pii); " + "it grants nothing in this snapshot" + ), + expected_denial=( + "missing view.sql on nodes model:customers; all requested nodes denied" + ), + ) + ], + ids=lambda case: case.description, +) +def test_given_project_roles_when_published_and_approved_then_person_sql_follows_selector_coverage( + test_case: ProjectRolesTestCase, tmp_path: Path +) -> None: + project: Path = tmp_path / "project" + roles: str = Path(test_case.roles_file).read_text() + write_files( + project_dir=project, + files={ + "sqlbuild_project.toml": ( + 'name = "orders"\nadapter = "duckdb"\n' + '[connections.local]\ndatabase = "warehouse.duckdb"\n' + '[targets.development]\nschema = "main"\n' + ), + "models/orders.sql": tagged_model("orders", "support"), + "models/customers.sql": tagged_model("customers", "pii"), + "roles.toml": roles, + }, + ) + with snapshot_api( + tmp_path / "control", + server_arguments=("--password-origin", "https://control.example.com"), + ) as api: + first: subprocess.CompletedProcess[str] = publish_generation(project, api, 1) + assert first.returncode == 0, first.stderr + assert test_case.expected_warning in first.stderr + admin: dict[str, Any] = provision_person( + tmp_path / "control", "create-user", "orders-admin" + ) + person: dict[str, Any] = provision_person( + tmp_path / "control", "create-principal", "orders-support" + ) + admin_session: tuple[str, str] = person_login(api, admin["name"]) + session: tuple[str, str] = person_login(api, person["name"]) + assert person_request( + api, + "/api/admin/roles", + body={ + "principal_id": person["principal_id"], + "role": "orders_support", + "location_id": api.location_id, + "target": "development", + }, + session=admin_session, + ) == {"applied": True} + activation: dict[str, Any] = { + "location_id": api.location_id, + "target": "development", + "role_key": "orders_support", + "reason": "Support answers order questions", + } + + def approve() -> Any: + requested: dict[str, Any] = person_request( + api, "/api/admin/role-activations", body=activation, session=admin_session + ) + return person_request( + api, + "/api/admin/role-approvals", + body={ + "request_id": requested["request_id"], + "decision": "approved", + "reason": "Reviewed the support grants", + "confirm_self_approval": True, + }, + session=admin_session, + ) + + assert read_person_sql(api, session, "model:orders")[0] == 403 + assert approve() == {"applied": True} + assert read_person_sql(api, session, "model:orders") == (200, "model:orders") + assert read_person_sql(api, session, "model:customers") == (403, test_case.expected_denial) + write_files( + project_dir=project, + files={"models/customers.sql": tagged_model("customers", "pii, support")}, + ) + grown: subprocess.CompletedProcess[str] = publish_generation(project, api, 2) + assert grown.returncode == 0, grown.stderr + assert "Warning:" not in grown.stderr + assert read_person_sql(api, session, "model:customers") == (200, "model:customers") + with pytest.raises(HTTPError) as unchanged: + person_request( + api, "/api/admin/role-activations", body=activation, session=admin_session + ) + assert unchanged.value.code == 400 + write_files( + project_dir=project, + files={"roles.toml": roles.replace('nodes = ["tag:support"]', 'nodes = ["tag:pii"]')}, + ) + rewritten: subprocess.CompletedProcess[str] = publish_generation(project, api, 3) + assert rewritten.returncode == 0, rewritten.stderr + assert read_person_sql(api, session, "model:orders")[0] == 403 + assert read_person_sql(api, session, "model:customers")[0] == 403 + assert approve() == {"applied": True} + assert read_person_sql(api, session, "model:customers") == (200, "model:customers") + assert read_person_sql(api, session, "model:orders")[0] == 403 diff --git a/website/src/content/docs/docs/cli/server.mdx b/website/src/content/docs/docs/cli/server.mdx index b9b15fdcc..7a73689eb 100644 --- a/website/src/content/docs/docs/cli/server.mdx +++ b/website/src/content/docs/docs/cli/server.mdx @@ -76,7 +76,8 @@ stderr; a failed request prints no SQL to stdout. Permission denials include the The equivalent person-authenticated API is a `GET` request to `/api/control/locations/{location_id}/targets/{target}/nodes/{node_key}/sql`. -Developer or Admin access must cover the requested location and target. Viewer, Operator and +Developer or Admin access must cover the requested location and target, or a +[project role](#define-project-roles) must grant `view.sql` on the node. Viewer, Operator and the configurable default Viewer role do not grant SQL inspection. The response contains the snapshot ID, exact node key, raw SQL and compiled SQL. Missing SQL fields are `null`; an absent current publication or node returns JSON `null` after authorization. @@ -86,6 +87,39 @@ data. Every successful inspection records an audit entry without copying SQL int audit cannot be saved, the request fails without returning SQL. Revocation takes effect on the next request, including sessions created by [`sqb login`](/docs/cli/login/). +## Define project roles + +Put project roles in `roles.toml` at the project root. `sqb location publish` resolves each +grant's node selectors against the published project: + +```toml +[roles.orders_support] +description = "Answer order questions" + +[[roles.orders_support.grants]] +nodes = ["tag:orders"] +permissions = ["view.sql", "run.adhoc"] + +[[roles.orders_support.grants]] +nodes = ["tag:orders,tag:pii"] +permissions = ["view.evidence"] +``` + +| Rule | Behaviour | +| --- | --- | +| Selectors | Same grammar as a job's `select`; an unknown tag or name fails publication | +| No matching nodes | Publication succeeds with a warning; the grant gives nothing | +| Permissions | Any non-administrative action; `location.manage`, `user.manage`, `role.assign`, `token.manage` and `audit.view` are rejected | +| Runs | `job.run`, `run.adhoc`, `run.rerun` and `run.rewrite_history` need the permission on every node the run writes | +| Several roles | Grants add up across a person's roles | + +Assign a project role with `POST /api/admin/roles`, using the role key as `role`. A first +definition, and any publication that adds a selector and permission pair, takes effect only after +an administrator requests it with `POST /api/admin/role-activations` and another administrator +approves it with `POST /api/admin/role-approvals`. A sole administrator must confirm a +self-approval. Removed pairs stop applying on publish. An unchanged selector covers new +matching nodes on publish without approval. A denial lists the nodes the request was missing. + ## Repair control views Repair snapshot selection, jobs, triggers, runs, node status and the structural catalog from From dd19d9d58a43f260727698073d77e4e465781648 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:31:26 +0000 Subject: [PATCH 2/5] test: drop an unused import from the project role tests make check-ci now passes on this branch: clippy with -D warnings, ruff and Fensu. --- .../tests/credentials/test_project_roles.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs index 9035183b0..82f09e3ed 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs @@ -2,7 +2,7 @@ use std::collections::{BTreeMap, BTreeSet}; -use serde_json::{Value, json}; +use serde_json::json; use crate::authorization::_helpers::custom_roles::set_effective_role; use crate::authorization::main::compile_roles::compile_roles; From 234d192ea73ac2e27fe263be0fb9809a1d9f3850 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:41:40 +0000 Subject: [PATCH 3/5] feat: record project role coverage changes in the snapshot change feed When a publication changes a custom role's effective grants, either by removing pairs or by changing selector coverage, the change is appended to that snapshot's change feed as role:, as CHI-907 section 13 requires. Validated: the native role tests (13 on SQLite and PostgreSQL), the full orchestration native suite (104) and make check-ci. --- .../authorization/_helpers/custom_roles.rs | 39 +++++++++++++++++++ .../main/publish_custom_roles.rs | 28 ++++++++++--- .../tests/credentials/roles/helpers.rs | 22 +++++++++++ .../tests/credentials/test_project_roles.rs | 8 +++- .../sqlbuild/references/docs/cli/server.md | 3 +- website/src/content/docs/docs/cli/server.mdx | 3 +- 6 files changed, 94 insertions(+), 9 deletions(-) diff --git a/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs b/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs index c92c57b16..9ca98831e 100644 --- a/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs +++ b/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs @@ -262,6 +262,45 @@ pub(crate) async fn current_role( )) } +fn grants_digest(grants: Option<&[RoleGrant]>) -> Result, ControlError> { + let Some(grants) = grants else { + return Ok(None); + }; + let text = + serde_json::to_string(grants).map_err(|error| ControlError::Invalid(error.to_string()))?; + Ok(Some(sha256_hex(text.as_bytes()))) +} + +/// Append an effective role's coverage change to the snapshot's change feed, after its node changes. +pub(crate) async fn record_role_change( + transaction: ControlTransaction, + (snapshot_id, role_key): (Uuid, &str), + (before, after): (Option<&[RoleGrant]>, Option<&[RoleGrant]>), +) -> Result { + let kind = if after.is_some() { + "changed" + } else { + "removed" + }; + let (transaction, rows) = query( + transaction, + "SELECT COALESCE(MAX(change_seq) + 1, 0) AS next FROM sqb_snapshot_changes WHERE snapshot_id=$1", + vec![Parameter::Id(snapshot_id)], + ) + .await?; + let sequence = rows + .first() + .and_then(|row| row["next"].as_i64()) + .unwrap_or_default(); + let (transaction, _) = query(transaction, + "INSERT INTO sqb_snapshot_changes (snapshot_id, change_seq, change_kind, subject_key, before_hash, after_hash) \ + VALUES ($1,$2,$3,$4,$5,$6)", + vec![Parameter::Id(snapshot_id), Parameter::Integer(sequence), Parameter::Text(kind.into()), + Parameter::Text(format!("role:{role_key}")), Parameter::OptionalText(grants_digest(before)?), + Parameter::OptionalText(grants_digest(after)?)]).await?; + Ok(transaction) +} + pub(crate) async fn set_effective_role( mut transaction: ControlTransaction, (location_id, target, role_key): (Uuid, &str, &str), diff --git a/crates/sqlbuild-orchestration/src/authorization/main/publish_custom_roles.rs b/crates/sqlbuild-orchestration/src/authorization/main/publish_custom_roles.rs index 468691f37..7c06c86ef 100644 --- a/crates/sqlbuild-orchestration/src/authorization/main/publish_custom_roles.rs +++ b/crates/sqlbuild-orchestration/src/authorization/main/publish_custom_roles.rs @@ -1,9 +1,10 @@ -//! Record each published custom role; removed pairs and selector coverage apply now, additions wait. +//! Record each published custom role; removals and coverage apply now and enter the change feed; additions wait. use serde_json::Value; use crate::authorization::_helpers::custom_roles::{ - decode_roles, effective_role, narrows, retained_grants, set_effective_role, + decode_grants, decode_roles, effective_role, narrows, record_role_change, retained_grants, + set_effective_role, }; use crate::snapshots::models::SnapshotRecord; use crate::store::errors::ControlError; @@ -20,15 +21,22 @@ pub(crate) async fn publish_custom_roles( let rows; (transaction, rows) = query( transaction, - "SELECT role_key FROM sqb_effective_roles WHERE location_id=$1 AND target=$2", + "SELECT role_key,resolved_grants_json FROM sqb_effective_roles WHERE location_id=$1 AND target=$2", vec![Parameter::Id(scope.0), Parameter::Text(scope.1.into())], ) .await?; - for key in rows + for row in rows .iter() - .filter_map(|row| row["role_key"].as_str()) - .filter(|key| !roles.contains_key(*key)) + .filter(|row| !roles.contains_key(row["role_key"].as_str().unwrap_or_default())) { + let key = row["role_key"].as_str().unwrap_or_default(); + let before = decode_grants(&row["resolved_grants_json"])?; + transaction = record_role_change( + transaction, + (record.snapshot_id, key), + (Some(&before), None), + ) + .await?; (transaction, _) = query( transaction, "DELETE FROM sqb_effective_roles WHERE location_id=$1 AND target=$2 AND role_key=$3", @@ -59,6 +67,14 @@ pub(crate) async fn publish_custom_roles( } else { digest.as_str() }; + if next != grants { + transaction = record_role_change( + transaction, + (record.snapshot_id, key), + (Some(&grants), Some(&next)), + ) + .await?; + } if next != grants || next_digest != digest { transaction = set_effective_role(transaction, (scope.0, scope.1, key), (next_digest, &next)) diff --git a/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs b/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs index 47bfe415c..c49a39192 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs @@ -217,6 +217,28 @@ pub(crate) async fn granting_role( authorize_member(store, fixture, (action, &[])).await.ok() } +/// Custom role entries of the snapshot change feed, by publication generation. +pub(crate) async fn role_changes(store: &ControlStore) -> Vec<(i64, String)> { + let transaction = begin(store, TransactionMode::Read).await.expect("read"); + let (transaction, rows) = query( + transaction, + "SELECT s.generation, c.change_kind FROM sqb_snapshot_changes c JOIN sqb_snapshots s ON s.snapshot_id=c.snapshot_id \ + WHERE c.subject_key='role:orders_support' ORDER BY s.generation, c.change_seq", + vec![], + ) + .await + .expect("role changes"); + commit(transaction).await.expect("commit"); + rows.iter() + .map(|row| { + ( + row["generation"].as_i64().unwrap_or_default(), + row["change_kind"].as_str().unwrap_or_default().into(), + ) + }) + .collect() +} + /// Role approval audit facts in commit order: action and recorded outcome. pub(crate) async fn role_audit(store: &ControlStore) -> Vec<(String, Value)> { let transaction = begin(store, TransactionMode::Read).await.expect("read"); diff --git a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs index 82f09e3ed..c5072175b 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs @@ -9,7 +9,7 @@ use crate::authorization::main::compile_roles::compile_roles; use crate::credentials::models::{Action, RoleAssignmentRequest, RoleGrant, RoleKey}; use crate::credentials::tests::role_helpers::{ SUPPORT_ROLE, activate, assign, authorize_member, developer_grants, orders_grant, - publish_support_role, role_fixture, try_request, + publish_support_role, role_changes, role_fixture, try_request, }; use crate::credentials::tests::test_types::{ProjectRoleTestCase, RolesFileTestCase}; use crate::runs::tests::helpers::snapshot; @@ -92,6 +92,12 @@ fn given_approved_selector_when_it_matches_more_nodes_then_coverage_grows_withou "{}: coverage growth is not a widening to approve", test_case.description ); + assert_eq!( + role_changes(store).await, + [(2, "changed".to_owned()), (3, "changed".to_owned())], + "{}: the dropped view grant and the grown coverage enter the change feed", + test_case.description + ); close_test_store(fixture).await; }); } diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md index 0efe61895..d7aeb8bbb 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md @@ -127,7 +127,8 @@ definition, and any publication that adds a selector and permission pair, takes an administrator requests it with `POST /api/admin/role-activations` and another administrator approves it with `POST /api/admin/role-approvals`. A sole administrator must confirm a self-approval. Removed pairs stop applying on publish. An unchanged selector covers new -matching nodes on publish without approval. A denial lists the nodes the request was missing. +matching nodes on publish without approval. Either change appears in the publication's change +list as `role:`. A denial lists the nodes the request was missing. ## Repair control views diff --git a/website/src/content/docs/docs/cli/server.mdx b/website/src/content/docs/docs/cli/server.mdx index 7a73689eb..8462d5a88 100644 --- a/website/src/content/docs/docs/cli/server.mdx +++ b/website/src/content/docs/docs/cli/server.mdx @@ -118,7 +118,8 @@ definition, and any publication that adds a selector and permission pair, takes an administrator requests it with `POST /api/admin/role-activations` and another administrator approves it with `POST /api/admin/role-approvals`. A sole administrator must confirm a self-approval. Removed pairs stop applying on publish. An unchanged selector covers new -matching nodes on publish without approval. A denial lists the nodes the request was missing. +matching nodes on publish without approval. Either change appears in the publication's change +list as `role:`. A denial lists the nodes the request was missing. ## Repair control views From 81fd3a7cf5385ce021bf1e011bb2b0c7293dd951 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:14:18 +0000 Subject: [PATCH 4/5] fix: scope trigger overrides and location listings to node coverage trigger.toggle and sensor.reset_cursor are now authorized on every node the trigger's current job or group writes, all or nothing, as job.run is. A role covering only some of those nodes can no longer pause or reset the trigger. The catalog, statistics and run listings span the whole location, so they now need view coverage of every node in the current publication. Partial coverage is denied and names the missing nodes; results are never filtered. Validated: the native role tests (15, on SQLite and PostgreSQL), deliberate breaks of both fixes, the full orchestration native suite (106), 13 installed publication tests and make check-ci. --- .../authorization/main/authorize_listing.rs | 34 ++++ .../src/authorization/main/mod.rs | 1 + .../src/declarations/_helpers/mod.rs | 1 + .../declarations/_helpers/trigger_scope.rs | 54 +++++++ .../main/change_person_trigger.rs | 38 ++--- .../src/runs/main/list_person_runs.rs | 13 +- .../main/read_person_catalog.rs | 19 +-- .../main/read_person_statistics.rs | 19 +-- .../tests/credentials/roles/helpers.rs | 127 +++++++++++++-- .../tests/credentials/test_project_roles.rs | 146 +++++++++++++++++- .../sqlbuild/references/docs/cli/server.md | 2 + website/src/content/docs/docs/cli/server.mdx | 2 + 12 files changed, 386 insertions(+), 70 deletions(-) create mode 100644 crates/sqlbuild-orchestration/src/authorization/main/authorize_listing.rs create mode 100644 crates/sqlbuild-orchestration/src/declarations/_helpers/trigger_scope.rs diff --git a/crates/sqlbuild-orchestration/src/authorization/main/authorize_listing.rs b/crates/sqlbuild-orchestration/src/authorization/main/authorize_listing.rs new file mode 100644 index 000000000..6e4f66114 --- /dev/null +++ b/crates/sqlbuild-orchestration/src/authorization/main/authorize_listing.rs @@ -0,0 +1,34 @@ +//! A location-wide listing needs view coverage of every node in the current snapshot; it is never filtered. + +use std::collections::BTreeSet; + +use uuid::Uuid; + +use crate::authorization::main::authorize_session::authorize_session; +use crate::credentials::models::{Action, AuditContext, DefaultRole, PermissionScope}; +use crate::store::errors::ControlError; +use crate::store::main::query::query; +use crate::store::models::{ControlTransaction, Parameter}; + +pub(crate) async fn authorize_listing( + transaction: ControlTransaction, + credentials: (&AuditContext, (&str, &str)), + (location_id, target): (Uuid, &str), + default_role: DefaultRole, +) -> Result<(ControlTransaction, AuditContext), ControlError> { + let (transaction, rows) = query(transaction, + "SELECT n.node_key FROM sqb_snapshot_nodes n JOIN sqb_current_snapshots c ON c.snapshot_id=n.snapshot_id \ + WHERE c.location_id=$1 AND c.target=$2", + vec![Parameter::Id(location_id), Parameter::Text(target.into())]).await?; + let mut nodes: BTreeSet = BTreeSet::new(); + for row in &rows { + nodes.insert(row["node_key"].as_str().unwrap_or_default().into()); + } + let scope = PermissionScope { + action: Action::View, + location_id: Some(location_id), + target: Some(target.into()), + nodes, + }; + authorize_session(transaction, credentials, &scope, default_role).await +} diff --git a/crates/sqlbuild-orchestration/src/authorization/main/mod.rs b/crates/sqlbuild-orchestration/src/authorization/main/mod.rs index 95c0738e9..c163f17af 100644 --- a/crates/sqlbuild-orchestration/src/authorization/main/mod.rs +++ b/crates/sqlbuild-orchestration/src/authorization/main/mod.rs @@ -1,4 +1,5 @@ pub mod assign_builtin_role; +pub(crate) mod authorize_listing; pub(crate) mod authorize_session; pub mod bootstrap_user; pub mod change_administrator; diff --git a/crates/sqlbuild-orchestration/src/declarations/_helpers/mod.rs b/crates/sqlbuild-orchestration/src/declarations/_helpers/mod.rs index 1e3bd2943..90ce175a0 100644 --- a/crates/sqlbuild-orchestration/src/declarations/_helpers/mod.rs +++ b/crates/sqlbuild-orchestration/src/declarations/_helpers/mod.rs @@ -3,4 +3,5 @@ pub(crate) mod documents; pub(crate) mod operator_change; pub(crate) mod operator_state; pub(crate) mod publication; +pub(crate) mod trigger_scope; pub(crate) mod trigger_state; diff --git a/crates/sqlbuild-orchestration/src/declarations/_helpers/trigger_scope.rs b/crates/sqlbuild-orchestration/src/declarations/_helpers/trigger_scope.rs new file mode 100644 index 000000000..00b969169 --- /dev/null +++ b/crates/sqlbuild-orchestration/src/declarations/_helpers/trigger_scope.rs @@ -0,0 +1,54 @@ +//! A trigger override's permission scope is every node the trigger's current job or group writes. + +use uuid::Uuid; + +use crate::credentials::models::{Action, AuditContext, PermissionScope}; +use crate::declarations::models::{TriggerChange, TriggerChangeRequest}; +use crate::runs::main::frozen_job_request::frozen_job_request; +use crate::runs::models::JobRunRequest; +use crate::snapshots::main::job_write_set::job_write_set; +use crate::store::errors::ControlError; +use crate::store::main::query::query; +use crate::store::models::{ControlTransaction, Parameter}; + +/// A group's scope is its frozen selection's write set, which covers every child's write set. +pub(crate) async fn trigger_scope( + transaction: ControlTransaction, + context: &AuditContext, + request: &TriggerChangeRequest, +) -> Result<(ControlTransaction, PermissionScope), ControlError> { + let (transaction, rows) = query(transaction, + "SELECT location_id,target,job_key FROM sqb_triggers WHERE trigger_id=$1 AND removed_in_snapshot_id IS NULL", + vec![Parameter::Id(request.trigger_id)]).await?; + let row = rows + .first() + .ok_or_else(|| ControlError::Invalid("current trigger not found".into()))?; + let job = JobRunRequest { + run_id: Uuid::nil(), + location_id: serde_json::from_value(row["location_id"].clone()) + .map_err(|error| ControlError::Invalid(error.to_string()))?, + target: row["target"].as_str().unwrap_or_default().into(), + job_key: row["job_key"].as_str().unwrap_or_default().into(), + reason: request.reason.clone(), + priority: 0, + window: None, + }; + let (transaction, frozen) = frozen_job_request(transaction, context, &job).await?; + let (transaction, nodes) = + job_write_set(transaction, frozen.snapshot_id, &frozen.selection).await?; + let action = match request.change { + TriggerChange::Enable | TriggerChange::Pause | TriggerChange::ResetDefault => { + Action::TriggerToggle + } + TriggerChange::SetCursor { .. } | TriggerChange::ResetCursor => Action::SensorResetCursor, + }; + Ok(( + transaction, + PermissionScope { + action, + location_id: Some(job.location_id), + target: Some(job.target), + nodes, + }, + )) +} diff --git a/crates/sqlbuild-orchestration/src/declarations/main/change_person_trigger.rs b/crates/sqlbuild-orchestration/src/declarations/main/change_person_trigger.rs index c9e0a2948..0dfe59869 100644 --- a/crates/sqlbuild-orchestration/src/declarations/main/change_person_trigger.rs +++ b/crates/sqlbuild-orchestration/src/declarations/main/change_person_trigger.rs @@ -1,42 +1,28 @@ -//! Resolve a trigger's real location before authorizing an authenticated runtime override. +//! A trigger override needs its permission on every node the trigger's current job or group writes. use serde_json::json; use crate::authorization::main::authorize_session::authorize_session; use crate::authorization::main::deny_authorization::deny; -use crate::credentials::models::{Action, AuditContext, DefaultRole, PermissionScope}; +use crate::credentials::models::{AuditContext, DefaultRole}; use crate::declarations::_helpers::operator_change::change_trigger_in; -use crate::declarations::models::{TriggerChange, TriggerChangeRequest}; +use crate::declarations::_helpers::trigger_scope::trigger_scope; +use crate::declarations::models::TriggerChangeRequest; use crate::store::errors::ControlError; -use crate::store::main::{begin::begin, commit::commit, query::query}; -use crate::store::models::{ControlStore, Parameter, TransactionMode}; +use crate::store::main::{begin::begin, commit::commit}; +use crate::store::models::{ControlStore, TransactionMode}; pub async fn change_person_trigger( store: &ControlStore, (context, credentials): (&AuditContext, (&str, &str)), request: &TriggerChangeRequest, ) -> Result { - let (transaction, rows) = query(begin(store, TransactionMode::Write).await?, - "SELECT location_id,target FROM sqb_triggers WHERE trigger_id=$1 AND removed_in_snapshot_id IS NULL", - vec![Parameter::Id(request.trigger_id)]).await?; - let row = rows - .first() - .ok_or_else(|| ControlError::Invalid("current trigger not found".into()))?; - let scope = PermissionScope { - action: match request.change { - TriggerChange::Enable | TriggerChange::Pause | TriggerChange::ResetDefault => { - Action::TriggerToggle - } - TriggerChange::SetCursor { .. } | TriggerChange::ResetCursor => { - Action::SensorResetCursor - } - }, - location_id: serde_json::from_value(row["location_id"].clone()) - .map_err(|error| ControlError::Invalid(error.to_string()))?, - target: serde_json::from_value(row["target"].clone()) - .map_err(|error| ControlError::Invalid(error.to_string()))?, - nodes: Default::default(), - }; + let (transaction, scope) = trigger_scope( + begin(store, TransactionMode::Write).await?, + context, + request, + ) + .await?; let (transaction, context) = authorize_session( transaction, (context, credentials), diff --git a/crates/sqlbuild-orchestration/src/runs/main/list_person_runs.rs b/crates/sqlbuild-orchestration/src/runs/main/list_person_runs.rs index 9d21d68a2..aaed1ca82 100644 --- a/crates/sqlbuild-orchestration/src/runs/main/list_person_runs.rs +++ b/crates/sqlbuild-orchestration/src/runs/main/list_person_runs.rs @@ -2,8 +2,8 @@ use uuid::Uuid; -use crate::authorization::main::authorize_session::authorize_session; -use crate::credentials::models::{Action, AuditContext, DefaultRole, PermissionScope}; +use crate::authorization::main::authorize_listing::authorize_listing; +use crate::credentials::models::{AuditContext, DefaultRole}; use crate::runs::_helpers::lifecycle::reads::list_runs_in; use crate::runs::models::RunRecord; use crate::store::errors::ControlError; @@ -16,15 +16,10 @@ pub async fn list_person_runs( (location_id, target, limit): (Uuid, &str, u32), default_role: DefaultRole, ) -> Result, ControlError> { - let (transaction, _) = authorize_session( + let (transaction, _) = authorize_listing( begin(store, TransactionMode::Write).await?, (context, credentials), - &PermissionScope { - action: Action::View, - location_id: Some(location_id), - target: Some(target.into()), - nodes: Default::default(), - }, + (location_id, target), default_role, ) .await?; diff --git a/crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_catalog.rs b/crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_catalog.rs index 25a7e38a4..ef3dce553 100644 --- a/crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_catalog.rs +++ b/crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_catalog.rs @@ -2,12 +2,12 @@ use uuid::Uuid; -use crate::credentials::models::{Action, AuditContext, DefaultRole, PermissionScope}; +use crate::authorization::main::authorize_listing::authorize_listing; +use crate::credentials::models::{AuditContext, DefaultRole}; use crate::store::errors::ControlError; -use crate::store::main::commit::commit; -use crate::store::models::ControlStore; +use crate::store::main::{begin::begin, commit::commit}; +use crate::store::models::{ControlStore, TransactionMode}; use crate::warehouse_metadata::_helpers::catalog::read_catalog_page; -use crate::warehouse_metadata::_helpers::person_reads::begin_view; use crate::warehouse_metadata::models::{CatalogPage, CatalogQuery}; pub async fn read_person_catalog( @@ -16,15 +16,10 @@ pub async fn read_person_catalog( (location_id, target, options): (Uuid, &str, &CatalogQuery), default_role: DefaultRole, ) -> Result { - let transaction = begin_view( - store, + let (transaction, _) = authorize_listing( + begin(store, TransactionMode::Write).await?, (context, credentials), - &PermissionScope { - action: Action::View, - location_id: Some(location_id), - target: Some(target.into()), - nodes: Default::default(), - }, + (location_id, target), default_role, ) .await?; diff --git a/crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_statistics.rs b/crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_statistics.rs index c1fab9451..770cf91f2 100644 --- a/crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_statistics.rs +++ b/crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_statistics.rs @@ -2,11 +2,11 @@ use uuid::Uuid; -use crate::credentials::models::{Action, AuditContext, DefaultRole, PermissionScope}; +use crate::authorization::main::authorize_listing::authorize_listing; +use crate::credentials::models::{AuditContext, DefaultRole}; use crate::store::errors::ControlError; -use crate::store::main::commit::commit; -use crate::store::models::ControlStore; -use crate::warehouse_metadata::_helpers::person_reads::begin_view; +use crate::store::main::{begin::begin, commit::commit}; +use crate::store::models::{ControlStore, TransactionMode}; use crate::warehouse_metadata::_helpers::statistics::read_statistics_page; use crate::warehouse_metadata::models::{StatisticsPage, StatisticsQuery, StatisticsRetention}; @@ -16,15 +16,10 @@ pub async fn read_person_statistics( (location_id, target, options): (Uuid, &str, &StatisticsQuery), (default_role, retention): (DefaultRole, StatisticsRetention), ) -> Result { - let transaction = begin_view( - store, + let (transaction, _) = authorize_listing( + begin(store, TransactionMode::Write).await?, (context, credentials), - &PermissionScope { - action: Action::View, - location_id: Some(location_id), - target: Some(target.into()), - nodes: Default::default(), - }, + (location_id, target), default_role, ) .await?; diff --git a/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs b/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs index c49a39192..0c35b066f 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/roles/helpers.rs @@ -24,11 +24,19 @@ use crate::credentials::models::{ }; use crate::credentials::tests::location_helpers::create_location; use crate::credentials::tests::test_types::RoleFixture; +use crate::declarations::main::change_person_trigger::change_person_trigger; +use crate::declarations::main::list_triggers::list_triggers; +use crate::declarations::models::{TriggerChange, TriggerChangeRequest}; +use crate::runs::main::list_person_runs::list_person_runs; use crate::runs::tests::helpers::snapshot; +use crate::scheduler::tests::contracts::ticks::helpers::calendar_text; use crate::snapshots::main::publish_snapshot::publish_snapshot; use crate::store::errors::ControlError; use crate::store::main::{begin::begin, commit::commit, query::query}; use crate::store::models::{ControlStore, TransactionMode}; +use crate::warehouse_metadata::main::read_person_catalog::read_person_catalog; +use crate::warehouse_metadata::main::read_person_statistics::read_person_statistics; +use crate::warehouse_metadata::models::{CatalogQuery, StatisticsQuery, StatisticsRetention}; pub(crate) const SUPPORT_ROLE: &str = "orders_support"; @@ -197,15 +205,116 @@ pub(crate) async fn authorize_member( ); decision .map(|(_, context)| context.granting_role) - .map_err(|error| { - let reason = error.to_string(); - let reason = reason.trim_start_matches("control-plane operation denied: "); - reason - .split("; decision ") - .next() - .unwrap_or_default() - .to_owned() - }) + .map_err(denial) +} + +/// A denial's reason without its decision ID, or the whole message for any other error. +fn denial(error: ControlError) -> String { + let reason = error.to_string(); + let reason = reason.trim_start_matches("control-plane operation denied: "); + reason + .split("; decision ") + .next() + .unwrap_or_default() + .to_owned() +} + +/// Publish support role grants with a job that writes only customers and its daily trigger. +pub(crate) async fn publish_customers_trigger( + store: &ControlStore, + location_id: Uuid, + (generation, grants): (i64, Value), +) { + let mut input = snapshot(location_id); + input.generation = generation; + input.documents.insert( + "roles".into(), + json!({SUPPORT_ROLE: {"description": "Answer order questions", "grants": grants}}), + ); + input.documents.insert( + "jobs".into(), + json!({"customers_job": {"description": "Build customers", "selection": ["customers"]}}), + ); + input.documents.insert("triggers".into(), json!({"customers_daily": {"description": "Refresh customers", + "job_key": "customers_job", "starts_at": calendar_text(0), + "schedule": {"kind": "calendar", "times": [calendar_text(0)], "refresh_at": "2030-01-01T00:00:00Z"}}})); + publish_snapshot(store, &input) + .await + .expect("publish trigger"); +} + +/// The member's override of the location's only trigger, or the denial reason. +pub(crate) async fn member_trigger_change( + store: &ControlStore, + fixture: &RoleFixture, + change: TriggerChange, +) -> Result { + let trigger_id = list_triggers(store, fixture.location_id, "development") + .await + .expect("triggers")[0] + .trigger_id; + change_person_trigger( + store, + (&fixture.context, credentials(&fixture.member)), + &TriggerChangeRequest { + event_id: Uuid::now_v7(), + trigger_id, + actor: String::new(), + reason: "Hold customer refreshes".into(), + change, + }, + ) + .await + .map_err(denial) +} + +/// The member's catalog, statistics and run listings of the fixture location: allowed or the denial. +pub(crate) async fn member_listings( + store: &ControlStore, + fixture: &RoleFixture, +) -> Vec> { + let member = (&fixture.context, credentials(&fixture.member)); + let scope = (fixture.location_id, "development"); + let catalog = CatalogQuery { + database: None, + schema: "main".into(), + after_observed_at: None, + after_event_id: None, + limit: None, + }; + let statistics = StatisticsQuery { + database: None, + schema: "main".into(), + name: "orders".into(), + after_observed_at: None, + after_event_id: None, + limit: None, + }; + let retention = StatisticsRetention::days(30).expect("retention"); + vec![ + read_person_catalog( + store, + member, + (scope.0, scope.1, &catalog), + DefaultRole::None, + ) + .await + .map(|_| ()) + .map_err(denial), + read_person_statistics( + store, + member, + (scope.0, scope.1, &statistics), + (DefaultRole::None, retention), + ) + .await + .map(|_| ()) + .map_err(denial), + list_person_runs(store, member, (scope.0, scope.1, 10), DefaultRole::None) + .await + .map(|_| ()) + .map_err(denial), + ] } /// The role that grants the member this location-scoped action, or `None` when denied. diff --git a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs index c5072175b..8ee45be65 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs @@ -8,10 +8,12 @@ use crate::authorization::_helpers::custom_roles::set_effective_role; use crate::authorization::main::compile_roles::compile_roles; use crate::credentials::models::{Action, RoleAssignmentRequest, RoleGrant, RoleKey}; use crate::credentials::tests::role_helpers::{ - SUPPORT_ROLE, activate, assign, authorize_member, developer_grants, orders_grant, - publish_support_role, role_changes, role_fixture, try_request, + SUPPORT_ROLE, activate, assign, authorize_member, developer_grants, member_listings, + member_trigger_change, orders_grant, publish_customers_trigger, publish_support_role, + role_changes, role_fixture, try_request, }; use crate::credentials::tests::test_types::{ProjectRoleTestCase, RolesFileTestCase}; +use crate::declarations::models::TriggerChange; use crate::runs::tests::helpers::snapshot; use crate::snapshots::main::publish_snapshot::publish_snapshot; use crate::store::errors::ControlError; @@ -330,6 +332,146 @@ fn given_removal_and_rewrite_in_one_publish_when_widening_waits_then_the_removal } } +#[test] +fn given_grant_outside_a_triggers_job_when_the_member_overrides_it_then_both_trigger_actions_are_denied() + { + let test_cases = [ + ProjectRoleTestCase { + description: "SQLite authorizes trigger overrides on the job's write set", + backend: Backend::Sqlite, + expected_before: Err( + "missing trigger.toggle on nodes customers; all requested nodes denied", + ), + expected_after: Err( + "missing sensor.reset_cursor on nodes customers; all requested nodes denied", + ), + }, + ProjectRoleTestCase { + description: "PostgreSQL authorizes trigger overrides on the job's write set", + backend: Backend::Postgres, + expected_before: Err( + "missing trigger.toggle on nodes customers; all requested nodes denied", + ), + expected_after: Err( + "missing sensor.reset_cursor on nodes customers; all requested nodes denied", + ), + }, + ]; + for test_case in test_cases { + run(async { + let Some(fixture) = open_test_store(test_case.backend).await else { + return; + }; + let store = &fixture.store; + migrate_store(store).await.expect("migrate"); + let roles = role_fixture(store, 2).await; + activate(store, &roles).await; + let grants = |nodes: &[&str]| { + json!([orders_grant( + json!(["view", "trigger.toggle", "sensor.reset_cursor"]), + nodes + )]) + }; + publish_customers_trigger(store, roles.location_id, (2, grants(&["orders"]))).await; + activate(store, &roles).await; + assert_eq!( + member_trigger_change(store, &roles, TriggerChange::Pause) + .await + .map(|_| SUPPORT_ROLE) + .as_ref() + .map(|role| *role) + .map_err(String::as_str), + test_case.expected_before, + "{}: an orders-only grant cannot pause a customers-only trigger", + test_case.description + ); + assert_eq!( + member_trigger_change(store, &roles, TriggerChange::ResetCursor) + .await + .map(|_| SUPPORT_ROLE) + .as_ref() + .map(|role| *role) + .map_err(String::as_str), + test_case.expected_after, + "{}: nor reset its cursor", + test_case.description + ); + publish_customers_trigger( + store, + roles.location_id, + (3, grants(&["customers", "orders"])), + ) + .await; + assert_eq!( + member_trigger_change(store, &roles, TriggerChange::Pause).await, + Ok(true), + "{}: coverage of the job's write set allows the pause", + test_case.description + ); + close_test_store(fixture).await; + }); + } +} + +#[test] +fn given_view_grant_on_part_of_the_location_when_listing_then_each_listing_is_denied_until_full_coverage() + { + let test_cases = [ + ProjectRoleTestCase { + description: "SQLite location listings need view on every node", + backend: Backend::Sqlite, + expected_before: Err( + "missing view on nodes customers, inventory, products; all requested nodes denied", + ), + expected_after: Ok(SUPPORT_ROLE), + }, + ProjectRoleTestCase { + description: "PostgreSQL location listings need view on every node", + backend: Backend::Postgres, + expected_before: Err( + "missing view on nodes customers, inventory, products; all requested nodes denied", + ), + expected_after: Ok(SUPPORT_ROLE), + }, + ]; + for test_case in test_cases { + run(async { + let Some(fixture) = open_test_store(test_case.backend).await else { + return; + }; + let store = &fixture.store; + migrate_store(store).await.expect("migrate"); + let roles = role_fixture(store, 2).await; + activate(store, &roles).await; + assert_eq!( + member_listings(store, &roles).await, + vec![test_case.expected_before.map(|_| ()).map_err(str::to_owned); 3], + "{}: catalog, statistics and runs are denied, not filtered", + test_case.description + ); + publish_support_role( + store, + roles.location_id, + ( + 2, + json!([orders_grant( + json!(["view"]), + &["customers", "inventory", "orders", "products"] + )]), + ), + ) + .await; + assert_eq!( + member_listings(store, &roles).await, + vec![test_case.expected_after.map(|_| ()).map_err(str::to_owned); 3], + "{}: full coverage allows every listing", + test_case.description + ); + close_test_store(fixture).await; + }); + } +} + #[test] fn given_two_custom_roles_when_each_covers_part_of_a_run_then_their_union_authorizes_it() { let test_cases = [ diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md index d7aeb8bbb..4b08a1fe0 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md @@ -120,6 +120,8 @@ permissions = ["view.evidence"] | No matching nodes | Publication succeeds with a warning; the grant gives nothing | | Permissions | Any non-administrative action; `location.manage`, `user.manage`, `role.assign`, `token.manage` and `audit.view` are rejected | | Runs | `job.run`, `run.adhoc`, `run.rerun` and `run.rewrite_history` need the permission on every node the run writes | +| Triggers | `trigger.toggle` and `sensor.reset_cursor` need the permission on every node the trigger's job writes | +| Listings | The catalog, statistics and run list need `view` on every node of the current publication; partial coverage is denied, not filtered | | Several roles | Grants add up across a person's roles | Assign a project role with `POST /api/admin/roles`, using the role key as `role`. A first diff --git a/website/src/content/docs/docs/cli/server.mdx b/website/src/content/docs/docs/cli/server.mdx index 8462d5a88..b613ce18d 100644 --- a/website/src/content/docs/docs/cli/server.mdx +++ b/website/src/content/docs/docs/cli/server.mdx @@ -111,6 +111,8 @@ permissions = ["view.evidence"] | No matching nodes | Publication succeeds with a warning; the grant gives nothing | | Permissions | Any non-administrative action; `location.manage`, `user.manage`, `role.assign`, `token.manage` and `audit.view` are rejected | | Runs | `job.run`, `run.adhoc`, `run.rerun` and `run.rewrite_history` need the permission on every node the run writes | +| Triggers | `trigger.toggle` and `sensor.reset_cursor` need the permission on every node the trigger's job writes | +| Listings | The catalog, statistics and run list need `view` on every node of the current publication; partial coverage is denied, not filtered | | Several roles | Grants add up across a person's roles | Assign a project role with `POST /api/admin/roles`, using the role key as `role`. A first From 77faacc966f3719912252a780a6ccbecc28ac23f Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:19:01 +0000 Subject: [PATCH 5/5] test: stale role assignments grant nothing after the role is removed A member keeps an assignment to a role that a later snapshot no longer defines. The assignment must grant nothing, on nodes or location-wide, and the removal must appear in the change feed. Covered on SQLite and PostgreSQL. --- .../tests/credentials/test_project_roles.rs | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs index 8ee45be65..3c3ea5070 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs @@ -472,6 +472,83 @@ fn given_view_grant_on_part_of_the_location_when_listing_then_each_listing_is_de } } +#[test] +fn given_role_removed_from_code_when_its_assignment_remains_then_it_grants_nothing() { + let test_cases = [ + ProjectRoleTestCase { + description: "SQLite stale assignment grants nothing", + backend: Backend::Sqlite, + expected_before: Ok(SUPPORT_ROLE), + expected_after: Err("missing run.adhoc on nodes orders; all requested nodes denied"), + }, + ProjectRoleTestCase { + description: "PostgreSQL stale assignment grants nothing", + backend: Backend::Postgres, + expected_before: Ok(SUPPORT_ROLE), + expected_after: Err("missing run.adhoc on nodes orders; all requested nodes denied"), + }, + ]; + for test_case in test_cases { + run(async { + let Some(fixture) = open_test_store(test_case.backend).await else { + return; + }; + let store = &fixture.store; + migrate_store(store).await.expect("migrate"); + let roles = role_fixture(store, 2).await; + activate(store, &roles).await; + publish_support_role( + store, + roles.location_id, + (2, developer_grants("tag:orders", &["orders"])), + ) + .await; + activate(store, &roles).await; + let adhoc = (Action::RunAdhoc, ["orders"].as_slice()); + assert_eq!( + authorize_member(store, &roles, adhoc) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_before, + "{}: the approved role grants ad-hoc runs", + test_case.description + ); + let mut input = snapshot(roles.location_id); + input.generation = 3; + input.documents.insert("roles".into(), json!({})); + publish_snapshot(store, &input) + .await + .expect("publish without roles"); + assert_eq!( + authorize_member(store, &roles, adhoc) + .await + .as_deref() + .map_err(String::as_str), + test_case.expected_after, + "{}: the removed role's assignment grants nothing on nodes", + test_case.description + ); + assert_eq!( + authorize_member(store, &roles, (Action::View, &[])) + .await + .as_deref() + .map_err(String::as_str), + Err("missing permission in this scope; all requested nodes denied"), + "{}: nor location-wide", + test_case.description + ); + assert_eq!( + role_changes(store).await.last(), + Some(&(3, "removed".to_owned())), + "{}: the removal enters the change feed", + test_case.description + ); + close_test_store(fixture).await; + }); + } +} + #[test] fn given_two_custom_roles_when_each_covers_part_of_a_run_then_their_union_authorizes_it() { let test_cases = [