From cd36f220202351d8ca88c6ae31e49086078e7e1a Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 6 Nov 2026 11:05:24 +0000 Subject: [PATCH 1/4] fix: name no nodes in refused listings A refused location listing named the nodes the person could not view and read differently for a location that does not exist. Listing refusals now carry one fixed message with the decision ID, identical for unassigned and unknown locations. The audit event keeps the missing nodes. --- clients/typescript/tests/client.test.ts | 2 +- .../src/authorization/constants.rs | 4 + .../authorization/main/authorize_listing.rs | 13 +- .../src/authorization/mod.rs | 1 + .../tests/api/listing_denials/helpers.rs | 43 ++++++ .../tests/api/test_contracts.rs | 4 + .../tests/api/test_listing_denials.rs | 132 ++++++++++++++++++ .../tests/api/test_types.rs | 7 + .../tests/credentials/test_project_roles.rs | 8 +- .../publication/test_denial.py | 12 +- 10 files changed, 213 insertions(+), 13 deletions(-) create mode 100644 crates/sqlbuild-orchestration/src/authorization/constants.rs create mode 100644 crates/sqlbuild-orchestration/tests/api/listing_denials/helpers.rs create mode 100644 crates/sqlbuild-orchestration/tests/api/test_listing_denials.rs diff --git a/clients/typescript/tests/client.test.ts b/clients/typescript/tests/client.test.ts index bd937b2883..26781afc40 100644 --- a/clients/typescript/tests/client.test.ts +++ b/clients/typescript/tests/client.test.ts @@ -134,7 +134,7 @@ test("generated client preserves native scopes, replay, path encoding, SSE and b const decisionId: string | null | undefined = denied.error?.decision_id; assert.ok(decisionId); assert.deepEqual(denied.error, { - error: `missing permission in this scope; all requested nodes denied; decision ${decisionId}`, + error: `missing view on one or more nodes in this location; decision ${decisionId}`, decision_id: decisionId, }); const machine = await browser.GET(healthPath, { headers, params }); diff --git a/crates/sqlbuild-orchestration/src/authorization/constants.rs b/crates/sqlbuild-orchestration/src/authorization/constants.rs new file mode 100644 index 0000000000..8585acf319 --- /dev/null +++ b/crates/sqlbuild-orchestration/src/authorization/constants.rs @@ -0,0 +1,4 @@ +//! Fixed authorization texts that callers may see. + +/// A refused listing names no node, so a caller cannot learn nodes it cannot view or whether a location exists. +pub(crate) const LISTING_DENIED: &str = "missing view on one or more nodes in this location"; diff --git a/crates/sqlbuild-orchestration/src/authorization/main/authorize_listing.rs b/crates/sqlbuild-orchestration/src/authorization/main/authorize_listing.rs index 1801ec9f17..1dda1127d1 100644 --- a/crates/sqlbuild-orchestration/src/authorization/main/authorize_listing.rs +++ b/crates/sqlbuild-orchestration/src/authorization/main/authorize_listing.rs @@ -1,9 +1,10 @@ -//! A location-wide listing needs view coverage of every node in the current snapshot; it is never filtered. +//! A listing needs view of every node in the current snapshot; its refusal names no node and reads the same for unknown locations. use std::collections::BTreeSet; use uuid::Uuid; +use crate::authorization::constants::LISTING_DENIED; use crate::authorization::main::authorize_person::authorize_person; use crate::credentials::models::{Action, AuditContext, DefaultRole, PermissionScope}; use crate::store::errors::ControlError; @@ -30,5 +31,13 @@ pub(crate) async fn authorize_listing( target: Some(target.into()), nodes, }; - authorize_person(transaction, credentials, &scope, default_role).await + authorize_person(transaction, credentials, &scope, default_role) + .await + .map_err(|error| match error { + ControlError::Denied { decision_id, .. } => ControlError::Denied { + reason: LISTING_DENIED.into(), + decision_id, + }, + error => error, + }) } diff --git a/crates/sqlbuild-orchestration/src/authorization/mod.rs b/crates/sqlbuild-orchestration/src/authorization/mod.rs index d1ddfc554a..640a7b648c 100644 --- a/crates/sqlbuild-orchestration/src/authorization/mod.rs +++ b/crates/sqlbuild-orchestration/src/authorization/mod.rs @@ -1,4 +1,5 @@ //! Closed, scoped person permissions and serialized installation-administrator lifecycle. pub(crate) mod _helpers; +pub(crate) mod constants; pub mod main; diff --git a/crates/sqlbuild-orchestration/tests/api/listing_denials/helpers.rs b/crates/sqlbuild-orchestration/tests/api/listing_denials/helpers.rs new file mode 100644 index 0000000000..c33f11b046 --- /dev/null +++ b/crates/sqlbuild-orchestration/tests/api/listing_denials/helpers.rs @@ -0,0 +1,43 @@ +//! Masked denial responses and the audit detail behind them. + +use reqwest::header; +use serde_json::{Value, json}; +use uuid::Uuid; + +use crate::store::main::{begin::begin, commit::commit, query::query}; +use crate::store::models::{ControlStore, TransactionMode}; + +/// Status, the headers that could tell cases apart, and the body with its decision ID masked. +pub(crate) async fn masked(response: reqwest::Response) -> (Value, Uuid) { + let status = response.status().as_u16(); + let headers: Vec<(String, String)> = response + .headers() + .iter() + .filter(|(name, _)| *name != header::DATE) + .map(|(name, value)| (name.to_string(), value.to_str().unwrap_or("").to_owned())) + .collect(); + let text = response.text().await.expect("denial body"); + let body: Value = serde_json::from_str(&text).expect("JSON denial"); + let decision: Uuid = serde_json::from_value(body["decision_id"].clone()).expect("decision ID"); + ( + json!({"status": status, "headers": headers, + "body": text.replace(&decision.to_string(), "")}), + decision, + ) +} + +pub(crate) async fn audited_scope(store: &ControlStore, decision: Uuid) -> Value { + let transaction = begin(store, TransactionMode::Read).await.expect("audit"); + let (transaction, rows) = query( + transaction, + "SELECT scope_json, after_json FROM sqb_security_audit WHERE action='authorization.denied'", + vec![], + ) + .await + .expect("denials"); + commit(transaction).await.expect("close audit"); + rows.into_iter() + .find(|row| row["after_json"]["decision_id"] == json!(decision)) + .map(|row| json!([row["scope_json"]["nodes"], row["after_json"]["reason"]])) + .unwrap_or(Value::Null) +} diff --git a/crates/sqlbuild-orchestration/tests/api/test_contracts.rs b/crates/sqlbuild-orchestration/tests/api/test_contracts.rs index 25527b6ad0..ec5b9b4b9c 100644 --- a/crates/sqlbuild-orchestration/tests/api/test_contracts.rs +++ b/crates/sqlbuild-orchestration/tests/api/test_contracts.rs @@ -6,6 +6,8 @@ mod changes_helpers; mod fingerprint_reads_helpers; #[path = "helpers.rs"] pub(crate) mod helpers; +#[path = "listing_denials/helpers.rs"] +mod listing_denials_helpers; #[path = "node_sql/helpers.rs"] mod node_sql_helpers; #[path = "node_status/helpers.rs"] @@ -44,6 +46,8 @@ mod test_custom_roles; mod test_development; #[path = "test_external_sessions.rs"] mod test_external_sessions; +#[path = "test_listing_denials.rs"] +mod test_listing_denials; #[path = "test_location_health.rs"] mod test_location_health; #[path = "test_location_work.rs"] diff --git a/crates/sqlbuild-orchestration/tests/api/test_listing_denials.rs b/crates/sqlbuild-orchestration/tests/api/test_listing_denials.rs new file mode 100644 index 0000000000..75824910e4 --- /dev/null +++ b/crates/sqlbuild-orchestration/tests/api/test_listing_denials.rs @@ -0,0 +1,132 @@ +//! Refused listings name no node and read the same for an unassigned and an unknown location. + +use reqwest::header; +use serde_json::{Value, json}; +use uuid::Uuid; + +use crate::api::models::{ApiAuthentication, HumanLogin}; +use crate::api::tests::helpers::{start_api_with_authentication, stop_api}; +use crate::api::tests::listing_denials_helpers::{audited_scope, masked}; +use crate::api::tests::test_types::ListingDenialTestCase; +use crate::credentials::tests::location_helpers::create_location; +use crate::credentials::tests::role_helpers::role_fixture; +use crate::runs::tests::helpers::snapshot; +use crate::snapshots::main::publish_snapshot::publish_snapshot; +use crate::store::main::migrate_store::migrate_store; +use crate::store::tests::store_helpers::{close_test_store, open_test_store, run}; +use crate::store::tests::store_test_types::Backend; + +const LISTINGS: [&str; 6] = [ + "runs", + "catalog?schema=orders", + "statistics?schema=orders&name=orders", + "checks", + "checks/orders_not_null", + "maintenance", +]; + +const NODES: [&str; 4] = ["customers", "inventory", "orders", "products"]; + +#[test] +fn given_refused_listings_when_people_read_them_then_no_node_is_named_and_unknown_locations_look_unassigned() + { + let test_cases = [ + ListingDenialTestCase { + description: "SQLite listing denials", + backend: Backend::Sqlite, + expected_status: 403, + expected_error: "missing view on one or more nodes in this location; decision ", + }, + ListingDenialTestCase { + description: "PostgreSQL listing denials", + backend: Backend::Postgres, + expected_status: 403, + expected_error: "missing view on one or more nodes in this location; decision ", + }, + ]; + 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).await; + let inventory = create_location(store, "inventory", "orders-admin") + .await + .expect("unassigned location"); + publish_snapshot(store, &snapshot(inventory)) + .await + .expect("unassigned snapshot"); + let api = start_api_with_authentication( + store, + ApiAuthentication { + login: HumanLogin::Password("https://control.example.com".into()), + ..ApiAuthentication::default() + }, + ) + .await; + let client = reqwest::Client::new(); + let read = |location: Uuid, listing: &str| { + client + .get(format!( + "{}/api/control/locations/{location}/targets/development/{listing}", + api.url + )) + .header(header::HOST, "control.example.com") + .header( + header::COOKIE, + format!("__Host-sqb-session={}", roles.member.bearer), + ) + .send() + }; + for listing in LISTINGS { + let (partial, partial_decision) = + masked(read(roles.location_id, listing).await.expect("partial")).await; + let (unassigned, unassigned_decision) = + masked(read(inventory, listing).await.expect("unassigned")).await; + let (unknown, unknown_decision) = + masked(read(Uuid::now_v7(), listing).await.expect("unknown")).await; + let body: Value = + serde_json::from_str(partial["body"].as_str().expect("body")).expect("JSON"); + assert_eq!( + ( + partial["status"].clone(), + body["error"].clone(), + unassigned == unknown, + partial == unassigned, + ), + ( + json!(test_case.expected_status), + json!(test_case.expected_error), + true, + true + ), + "{}: {listing} names no node and hides whether the location exists", + test_case.description + ); + assert!( + NODES + .iter() + .all(|node| !partial["body"].as_str().unwrap_or_default().contains(node)), + "{}: {listing} body names a node", + test_case.description + ); + assert_eq!( + ( + audited_scope(store, unassigned_decision).await[0].clone(), + audited_scope(store, unknown_decision).await[0].clone(), + audited_scope(store, partial_decision).await[1] + .as_str() + .is_some_and(|reason| reason.contains("inventory")), + ), + (json!(NODES), json!([]), true), + "{}: {listing} audits every refusal with the full detail", + test_case.description + ); + } + stop_api(api).await; + close_test_store(fixture).await; + }); + } +} diff --git a/crates/sqlbuild-orchestration/tests/api/test_types.rs b/crates/sqlbuild-orchestration/tests/api/test_types.rs index 7d4430d6ec..be4ad753ec 100644 --- a/crates/sqlbuild-orchestration/tests/api/test_types.rs +++ b/crates/sqlbuild-orchestration/tests/api/test_types.rs @@ -119,6 +119,13 @@ pub(crate) struct CliLoginTestCase { pub(crate) expected_exchanges: usize, } +pub(crate) struct ListingDenialTestCase { + pub(crate) description: &'static str, + pub(crate) backend: Backend, + pub(crate) expected_status: u16, + pub(crate) expected_error: &'static str, +} + pub(crate) struct CustomRoleApiTestCase { pub(crate) description: &'static str, pub(crate) backend: Backend, diff --git a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs index 2182207428..db79fbbf9f 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs @@ -244,17 +244,13 @@ fn given_view_grant_on_part_of_the_location_when_listing_then_each_listing_is_de 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_before: Err("missing view on one or more nodes in this location"), 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_before: Err("missing view on one or more nodes in this location"), expected_after: Ok(SUPPORT_ROLE), }, ]; diff --git a/tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_denial.py b/tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_denial.py index 2c1f786811..70784861e5 100644 --- a/tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_denial.py +++ b/tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_denial.py @@ -53,7 +53,13 @@ def test_given_denied_browser_requests_when_returned_then_only_durable_decision_ ) unlisted: str = catalog.replace(api.location_id, str(uuid4())) decisions: dict[str, str] = {} - for path in [catalog, unlisted, "/api/control/audit"]: + listing_denied: str = "missing view on one or more nodes in this location" + scope_denied: str = "missing permission in this scope; all requested nodes denied" + for path, reason in [ + (catalog, listing_denied), + (unlisted, listing_denied), + ("/api/control/audit", scope_denied), + ]: with pytest.raises(HTTPError) as rejected: person_read(api, path, session=session) assert rejected.value.code == 403 @@ -61,9 +67,7 @@ def test_given_denied_browser_requests_when_returned_then_only_durable_decision_ body: dict[str, Any] = json.load(rejected.value) assert set(body) == {"error", "decision_id"} decision: str = str(UUID(body["decision_id"])) - assert body["error"] == ( - "missing permission in this scope; all requested nodes denied; decision " + decision - ) + assert body["error"] == reason + "; decision " + decision decisions[decision] = "authorization.denied" request: dict[str, Any] = { "run_id": str(uuid4()), From 1f53d591dcfaa4d47d39f101c69481b15c6a70b1 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 6 Nov 2026 11:34:49 +0000 Subject: [PATCH 2/4] fix: name no nodes in authorization denials Every refusal from person authorization now shows fixed text per action and its decision ID; the audit event keeps the missing nodes. Unknown jobs and triggers are refused only after the caller is authorized for the target, so an unknown object reads the same as a forbidden one. --- clients/typescript/tests/client.test.ts | 2 +- .../authorization/_helpers/custom_roles.rs | 24 +- .../src/authorization/constants.rs | 4 - .../authorization/main/authorize_listing.rs | 13 +- .../authorization/main/authorize_person.rs | 12 +- .../src/authorization/main/mod.rs | 1 + .../src/authorization/main/refuse_unknown.rs | 42 +++ .../src/authorization/mod.rs | 1 - .../declarations/_helpers/trigger_scope.rs | 27 +- .../main/change_person_trigger.rs | 2 +- .../src/runs/_helpers/requests/manual_job.rs | 30 +- .../{listing_denials => denials}/helpers.rs | 31 +- .../tests/api/test_contracts.rs | 8 +- .../tests/api/test_denials.rs | 268 ++++++++++++++++++ .../tests/api/test_listing_denials.rs | 132 --------- .../tests/api/test_types.rs | 6 +- .../tests/credentials/test_project_roles.rs | 36 +-- .../sqlbuild/references/docs/cli/server.md | 6 +- .../publication/test_denial.py | 8 +- .../publication/test_project_roles.py | 4 +- website/src/content/docs/docs/cli/server.mdx | 6 +- 21 files changed, 452 insertions(+), 211 deletions(-) delete mode 100644 crates/sqlbuild-orchestration/src/authorization/constants.rs create mode 100644 crates/sqlbuild-orchestration/src/authorization/main/refuse_unknown.rs rename crates/sqlbuild-orchestration/tests/api/{listing_denials => denials}/helpers.rs (57%) create mode 100644 crates/sqlbuild-orchestration/tests/api/test_denials.rs delete mode 100644 crates/sqlbuild-orchestration/tests/api/test_listing_denials.rs diff --git a/clients/typescript/tests/client.test.ts b/clients/typescript/tests/client.test.ts index 26781afc40..4c7c77a821 100644 --- a/clients/typescript/tests/client.test.ts +++ b/clients/typescript/tests/client.test.ts @@ -134,7 +134,7 @@ test("generated client preserves native scopes, replay, path encoding, SSE and b const decisionId: string | null | undefined = denied.error?.decision_id; assert.ok(decisionId); assert.deepEqual(denied.error, { - error: `missing view on one or more nodes in this location; decision ${decisionId}`, + error: `missing view permission for this target; decision ${decisionId}`, decision_id: decisionId, }); const machine = await browser.GET(healthPath, { headers, params }); diff --git a/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs b/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs index bfb34a400f..e62b97fb00 100644 --- a/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs +++ b/crates/sqlbuild-orchestration/src/authorization/_helpers/custom_roles.rs @@ -120,10 +120,18 @@ pub(crate) fn custom_coverage( (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 { +/// The audited reason naming up to twenty missing nodes, and node-free text for the caller. +pub(crate) fn denial_reason( + action: Action, + missing: &[String], +) -> Result<(String, String), ControlError> { + let action = action_name(action)?; + let shown = format!("missing {action} permission for this target"); if missing.is_empty() { - return Ok("missing permission in this scope; all requested nodes denied".into()); + return Ok(( + "missing permission in this scope; all requested nodes denied".into(), + shown, + )); } let listed: Vec<&str> = missing.iter().take(20).map(String::as_str).collect(); let more = missing.len() - listed.len(); @@ -132,10 +140,12 @@ pub(crate) fn denial_reason(action: Action, missing: &[String]) -> Result ControlError::Denied { - reason: LISTING_DENIED.into(), - decision_id, - }, - error => error, - }) + authorize_person(transaction, credentials, &scope, default_role).await } diff --git a/crates/sqlbuild-orchestration/src/authorization/main/authorize_person.rs b/crates/sqlbuild-orchestration/src/authorization/main/authorize_person.rs index 9489453bbf..a3b0d3cb82 100644 --- a/crates/sqlbuild-orchestration/src/authorization/main/authorize_person.rs +++ b/crates/sqlbuild-orchestration/src/authorization/main/authorize_person.rs @@ -96,6 +96,14 @@ pub(crate) async fn authorize_person( context.granting_role = "default:viewer".into(); return Ok((transaction, context)); } - let reason = denial_reason(scope.action, &missing)?; - deny(transaction, &context, json!(scope), &reason).await + let (audited, shown) = denial_reason(scope.action, &missing)?; + deny(transaction, &context, json!(scope), &audited) + .await + .map_err(|error| match error { + ControlError::Denied { decision_id, .. } => ControlError::Denied { + reason: shown, + decision_id, + }, + error => error, + }) } diff --git a/crates/sqlbuild-orchestration/src/authorization/main/mod.rs b/crates/sqlbuild-orchestration/src/authorization/main/mod.rs index 6ae84eb147..6c4ddfef68 100644 --- a/crates/sqlbuild-orchestration/src/authorization/main/mod.rs +++ b/crates/sqlbuild-orchestration/src/authorization/main/mod.rs @@ -11,3 +11,4 @@ pub(crate) mod publish_custom_roles; pub(crate) mod publish_group_roles; pub(crate) mod read_audit_events; pub mod read_security_audit; +pub(crate) mod refuse_unknown; diff --git a/crates/sqlbuild-orchestration/src/authorization/main/refuse_unknown.rs b/crates/sqlbuild-orchestration/src/authorization/main/refuse_unknown.rs new file mode 100644 index 0000000000..bafb0a9e6a --- /dev/null +++ b/crates/sqlbuild-orchestration/src/authorization/main/refuse_unknown.rs @@ -0,0 +1,42 @@ +//! An unknown object is refused like a forbidden one unless the person may act on the whole scope it was looked up in. + +use std::collections::BTreeSet; + +use uuid::Uuid; + +use crate::authorization::main::authorize_person::authorize_person; +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}; + +/// `location` is where the lookup happened, or `None` when the object alone named it. +pub(crate) async fn refuse_unknown( + transaction: ControlTransaction, + credentials: (&AuditContext, Uuid), + (action, location): (Action, Option<(Uuid, &str)>), + missing: ControlError, +) -> Result { + let (transaction, nodes) = match location { + Some((location_id, target)) => { + 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 nodes: BTreeSet = rows + .iter() + .filter_map(|row| row["node_key"].as_str().map(str::to_owned)) + .collect(); + (transaction, nodes) + } + None => (transaction, BTreeSet::new()), + }; + let scope = PermissionScope { + action, + location_id: location.map(|(location_id, _)| location_id), + target: location.map(|(_, target)| target.to_owned()), + nodes, + }; + authorize_person(transaction, credentials, &scope, DefaultRole::None).await?; + Err(missing) +} diff --git a/crates/sqlbuild-orchestration/src/authorization/mod.rs b/crates/sqlbuild-orchestration/src/authorization/mod.rs index 640a7b648c..d1ddfc554a 100644 --- a/crates/sqlbuild-orchestration/src/authorization/mod.rs +++ b/crates/sqlbuild-orchestration/src/authorization/mod.rs @@ -1,5 +1,4 @@ //! Closed, scoped person permissions and serialized installation-administrator lifecycle. pub(crate) mod _helpers; -pub(crate) mod constants; pub mod main; diff --git a/crates/sqlbuild-orchestration/src/declarations/_helpers/trigger_scope.rs b/crates/sqlbuild-orchestration/src/declarations/_helpers/trigger_scope.rs index 00b9691698..84e2af675a 100644 --- a/crates/sqlbuild-orchestration/src/declarations/_helpers/trigger_scope.rs +++ b/crates/sqlbuild-orchestration/src/declarations/_helpers/trigger_scope.rs @@ -2,6 +2,7 @@ use uuid::Uuid; +use crate::authorization::main::refuse_unknown::refuse_unknown; use crate::credentials::models::{Action, AuditContext, PermissionScope}; use crate::declarations::models::{TriggerChange, TriggerChangeRequest}; use crate::runs::main::frozen_job_request::frozen_job_request; @@ -14,15 +15,27 @@ 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, + (context, credentials): (&AuditContext, Uuid), request: &TriggerChangeRequest, ) -> Result<(ControlTransaction, PermissionScope), ControlError> { + let action = match request.change { + TriggerChange::Enable | TriggerChange::Pause | TriggerChange::ResetDefault => { + Action::TriggerToggle + } + TriggerChange::SetCursor { .. } | TriggerChange::ResetCursor => Action::SensorResetCursor, + }; 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 Some(row) = rows.first() else { + return refuse_unknown( + transaction, + (context, credentials), + (action, None), + ControlError::Invalid("current trigger not found".into()), + ) + .await; + }; let job = JobRunRequest { run_id: Uuid::nil(), location_id: serde_json::from_value(row["location_id"].clone()) @@ -36,12 +49,6 @@ pub(crate) async fn trigger_scope( 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 { 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 bab23514ed..25f8cc75ac 100644 --- a/crates/sqlbuild-orchestration/src/declarations/main/change_person_trigger.rs +++ b/crates/sqlbuild-orchestration/src/declarations/main/change_person_trigger.rs @@ -20,7 +20,7 @@ pub async fn change_person_trigger( ) -> Result { let (transaction, scope) = trigger_scope( begin(store, TransactionMode::Write).await?, - context, + (context, credentials), request, ) .await?; diff --git a/crates/sqlbuild-orchestration/src/runs/_helpers/requests/manual_job.rs b/crates/sqlbuild-orchestration/src/runs/_helpers/requests/manual_job.rs index a690f31d6d..531d0b8286 100644 --- a/crates/sqlbuild-orchestration/src/runs/_helpers/requests/manual_job.rs +++ b/crates/sqlbuild-orchestration/src/runs/_helpers/requests/manual_job.rs @@ -4,6 +4,7 @@ use serde_json::json; use uuid::Uuid; use crate::authorization::main::authorize_person::authorize_person; +use crate::authorization::main::refuse_unknown::refuse_unknown; use crate::credentials::main::audit::append_audit::append_audit; use crate::credentials::models::{Action, AuditContext, DefaultRole, PermissionScope}; use crate::runs::_helpers::requests::permissions::authorize_execution; @@ -14,7 +15,8 @@ use crate::runs::constants::MANUAL_JOB_AUDIT_TAG; use crate::runs::main::frozen_job_request::frozen_job_request; use crate::runs::models::{JobRunRequest, RunWindow}; use crate::store::errors::ControlError; -use crate::store::models::ControlTransaction; +use crate::store::main::query::query; +use crate::store::models::{ControlTransaction, Parameter}; pub(crate) async fn request_job_run_in( mut transaction: ControlTransaction, @@ -61,6 +63,10 @@ pub(crate) async fn request_job_run_in( if retry { return Ok((transaction, false)); } + let transaction = match credentials { + Some(credentials) => known_job(transaction, (&context, credentials), request).await?, + None => transaction, + }; let (mut transaction, frozen) = frozen_job_request(transaction, &context, request).await?; if let Some(credentials) = credentials { (transaction, context) = authorize_execution( @@ -78,3 +84,25 @@ pub(crate) async fn request_job_run_in( json!(null), json!({"request":request,"frozen":frozen}))).await?; Ok((transaction, true)) } + +/// A person's unknown job is refused like a forbidden one unless they may run every node of the target. +async fn known_job( + transaction: ControlTransaction, + credentials: (&AuditContext, Uuid), + request: &JobRunRequest, +) -> Result { + let (transaction, jobs) = query(transaction, + "SELECT j.job_key FROM sqb_jobs j JOIN sqb_current_snapshots c ON c.location_id=j.location_id \ + AND c.target=j.target AND c.snapshot_id=j.snapshot_id WHERE j.location_id=$1 AND j.target=$2 AND j.job_key=$3", + vec![Parameter::Id(request.location_id), Parameter::Text(request.target.clone()), Parameter::Text(request.job_key.clone())]).await?; + if jobs.is_empty() { + return refuse_unknown( + transaction, + credentials, + (Action::JobRun, Some((request.location_id, &request.target))), + ControlError::Invalid("published job not found in this location and target".into()), + ) + .await; + } + Ok(transaction) +} diff --git a/crates/sqlbuild-orchestration/tests/api/listing_denials/helpers.rs b/crates/sqlbuild-orchestration/tests/api/denials/helpers.rs similarity index 57% rename from crates/sqlbuild-orchestration/tests/api/listing_denials/helpers.rs rename to crates/sqlbuild-orchestration/tests/api/denials/helpers.rs index c33f11b046..548a653b95 100644 --- a/crates/sqlbuild-orchestration/tests/api/listing_denials/helpers.rs +++ b/crates/sqlbuild-orchestration/tests/api/denials/helpers.rs @@ -1,4 +1,4 @@ -//! Masked denial responses and the audit detail behind them. +//! Masked denial responses, person requests, and the audit detail behind them. use reqwest::header; use serde_json::{Value, json}; @@ -41,3 +41,32 @@ pub(crate) async fn audited_scope(store: &ControlStore, decision: Uuid) -> Value .map(|row| json!([row["scope_json"]["nodes"], row["after_json"]["reason"]])) .unwrap_or(Value::Null) } + +/// A person's browser request with session cookie, origin and CSRF proof. +pub(crate) async fn person_send( + client: &reqwest::Client, + (url, session, csrf): (&str, &str, &str), + body: Option, +) -> reqwest::Response { + let request = body.map_or_else(|| client.get(url), |body| client.post(url).json(&body)); + request + .header(header::HOST, "control.example.com") + .header(header::ORIGIN, "https://control.example.com") + .header(header::COOKIE, format!("__Host-sqb-session={session}")) + .header("x-sqb-csrf", csrf) + .send() + .await + .expect("person request") +} + +/// A job request body for a location and job key. +pub(crate) fn job(location_id: Uuid, job_key: &str) -> Value { + json!({"run_id": Uuid::now_v7(), "location_id": location_id, "target": "development", + "job_key": job_key, "reason": "Refresh orders", "priority": 0}) +} + +/// A trigger change body of the given kind. +pub(crate) fn trigger(trigger_id: Uuid, change: &str) -> Value { + json!({"event_id": Uuid::now_v7(), "trigger_id": trigger_id, "actor": "", + "reason": "Hold orders", "change": {"kind": change}}) +} diff --git a/crates/sqlbuild-orchestration/tests/api/test_contracts.rs b/crates/sqlbuild-orchestration/tests/api/test_contracts.rs index ec5b9b4b9c..c57b2e1c14 100644 --- a/crates/sqlbuild-orchestration/tests/api/test_contracts.rs +++ b/crates/sqlbuild-orchestration/tests/api/test_contracts.rs @@ -2,12 +2,12 @@ #[path = "changes/helpers.rs"] mod changes_helpers; +#[path = "denials/helpers.rs"] +mod denials_helpers; #[path = "fingerprint_reads/helpers.rs"] mod fingerprint_reads_helpers; #[path = "helpers.rs"] pub(crate) mod helpers; -#[path = "listing_denials/helpers.rs"] -mod listing_denials_helpers; #[path = "node_sql/helpers.rs"] mod node_sql_helpers; #[path = "node_status/helpers.rs"] @@ -42,12 +42,12 @@ mod test_cli_login; mod test_contract; #[path = "test_custom_roles.rs"] mod test_custom_roles; +#[path = "test_denials.rs"] +mod test_denials; #[path = "test_development.rs"] mod test_development; #[path = "test_external_sessions.rs"] mod test_external_sessions; -#[path = "test_listing_denials.rs"] -mod test_listing_denials; #[path = "test_location_health.rs"] mod test_location_health; #[path = "test_location_work.rs"] diff --git a/crates/sqlbuild-orchestration/tests/api/test_denials.rs b/crates/sqlbuild-orchestration/tests/api/test_denials.rs new file mode 100644 index 0000000000..838b41ebb7 --- /dev/null +++ b/crates/sqlbuild-orchestration/tests/api/test_denials.rs @@ -0,0 +1,268 @@ +//! Refusals name no node and read the same for a forbidden object and an unknown one. + +use serde_json::{Value, json}; +use uuid::Uuid; + +use crate::api::models::{ApiAuthentication, HumanLogin}; +use crate::api::tests::denials_helpers::{audited_scope, job, masked, person_send, trigger}; +use crate::api::tests::helpers::{start_api_with_authentication, stop_api}; +use crate::api::tests::test_types::{DenialRequest, DenialTestCase}; +use crate::credentials::tests::location_helpers::create_location; +use crate::credentials::tests::role_helpers::role_fixture; +use crate::declarations::main::list_triggers::list_triggers; +use crate::runs::main::request_job_run::request_job_run; +use crate::runs::models::JobRunRequest; +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::main::migrate_store::migrate_store; +use crate::store::tests::store_helpers::{close_test_store, open_test_store, run}; +use crate::store::tests::store_test_types::Backend; + +/// Location-scoped reads and the action each needs. +const READS: [(&str, &str); 8] = [ + ("runs", "view"), + ("catalog?schema=orders", "view"), + ("statistics?schema=orders&name=orders", "view"), + ("checks", "view"), + ("checks/orders_not_null", "view"), + ("maintenance", "view"), + ("checks/orders_not_null/sql", "view.sql"), + ( + "checks/orders_not_null/results/01a1b000-0000-7000-8000-000000000000/evidence", + "view.evidence", + ), +]; + +const NODES: [&str; 4] = ["customers", "inventory", "orders", "products"]; + +#[test] +fn given_refused_requests_when_people_send_them_then_no_node_is_named_and_unknown_objects_look_forbidden() + { + let test_cases = [ + DenialTestCase { + description: "SQLite denials", + backend: Backend::Sqlite, + expected_status: 403, + }, + DenialTestCase { + description: "PostgreSQL denials", + backend: Backend::Postgres, + expected_status: 403, + }, + ]; + 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).await; + let inventory = create_location(store, "inventory", "orders-admin") + .await + .expect("unassigned location"); + let mut published = snapshot(inventory); + published.documents.insert( + "jobs".into(), + json!({"orders": {"description": "Build orders", "selection": ["orders"]}}), + ); + published.documents.insert("triggers".into(), json!({"daily_orders": { + "description": "Refresh orders", "job_key": "orders", "starts_at": calendar_text(0), + "schedule": {"kind": "calendar", "times": [calendar_text(0), "2030-01-01T00:00:00Z"], + "refresh_at": "2029-01-01T00:00:00Z"}}})); + publish_snapshot(store, &published) + .await + .expect("unassigned snapshot"); + let trigger_id = list_triggers(store, inventory, "development") + .await + .expect("trigger")[0] + .trigger_id; + let queued = JobRunRequest { + run_id: Uuid::now_v7(), + location_id: inventory, + target: "development".into(), + job_key: "orders".into(), + reason: "Refresh orders".into(), + priority: 0, + window: None, + }; + assert!( + request_job_run(store, &roles.context, &queued) + .await + .expect("queued run") + ); + let api = start_api_with_authentication( + store, + ApiAuthentication { + login: HumanLogin::Password("https://control.example.com".into()), + ..ApiAuthentication::default() + }, + ) + .await; + let client = reqwest::Client::new(); + let member = (roles.member.bearer.as_str(), roles.member.csrf.as_str()); + let read = |location: Uuid, path: &str| { + format!( + "{}/api/control/locations/{location}/targets/development/{path}", + api.url + ) + }; + let runs = format!("{}/api/control/runs", api.url); + let triggers = format!("{}/api/control/triggers", api.url); + let cancel = format!("{}/api/control/runs/cancel", api.url); + let rerun = format!("{}/api/control/runs/rerun", api.url); + let unknown_run = Uuid::now_v7(); + let unknown_trigger = Uuid::now_v7(); + let mut cases: Vec<(String, &str, Vec)> = READS + .iter() + .map(|(path, action)| { + ( + (*path).to_owned(), + *action, + vec![ + (read(roles.location_id, path), None), + (read(inventory, path), None), + (read(Uuid::now_v7(), path), None), + ], + ) + }) + .collect(); + cases.extend([ + ( + "job run".to_owned(), + "job.run", + vec![ + (runs.clone(), Some(job(inventory, "orders"))), + (runs.clone(), Some(job(inventory, "returns"))), + (runs.clone(), Some(job(Uuid::now_v7(), "orders"))), + ], + ), + ( + "trigger pause".to_owned(), + "trigger.toggle", + vec![ + (triggers.clone(), Some(trigger(trigger_id, "pause"))), + (triggers.clone(), Some(trigger(unknown_trigger, "pause"))), + ], + ), + ( + "sensor cursor reset".to_owned(), + "sensor.reset_cursor", + vec![ + (triggers.clone(), Some(trigger(trigger_id, "reset_cursor"))), + ( + triggers.clone(), + Some(trigger(unknown_trigger, "reset_cursor")), + ), + ], + ), + ( + "run cancel".to_owned(), + "run.cancel", + vec![ + ( + cancel.clone(), + Some(json!({"run_id": queued.run_id, "event_id": Uuid::now_v7(), + "reason": "Stop orders"})), + ), + ( + cancel.clone(), + Some(json!({"run_id": unknown_run, "event_id": Uuid::now_v7(), + "reason": "Stop orders"})), + ), + ], + ), + ( + "rerun".to_owned(), + "run.rerun", + vec![ + ( + rerun.clone(), + Some( + json!({"run_id": Uuid::now_v7(), "source_run_id": queued.run_id, + "from_failure": false, "reason": "Retry orders", "priority": 0}), + ), + ), + ( + rerun.clone(), + Some( + json!({"run_id": Uuid::now_v7(), "source_run_id": unknown_run, + "from_failure": false, "reason": "Retry orders", "priority": 0}), + ), + ), + ], + ), + ]); + for (route, action, requests) in cases { + let mut responses: Vec<(Value, Uuid)> = Vec::new(); + for (url, body) in requests { + responses.push( + masked(person_send(&client, (&url, member.0, member.1), body).await).await, + ); + } + let first: Value = + serde_json::from_str(responses[0].0["body"].as_str().expect("body")) + .expect("JSON denial"); + assert_eq!( + ( + responses[0].0["status"].clone(), + first["error"].clone(), + responses + .iter() + .all(|(response, _)| *response == responses[0].0), + ), + ( + json!(test_case.expected_status), + json!(format!( + "missing {action} permission for this target; decision " + )), + true + ), + "{}: {route} reads the same for forbidden and unknown objects: {responses:?}", + test_case.description + ); + assert!( + NODES.iter().all(|node| !responses[0].0["body"] + .as_str() + .unwrap_or_default() + .contains(node)), + "{}: {route} names a node", + test_case.description + ); + for (_, decision) in &responses { + assert_ne!( + audited_scope(store, *decision).await, + Value::Null, + "{}: {route} audits every refusal", + test_case.description + ); + } + } + assert_eq!( + audited_scope( + store, + masked( + person_send( + &client, + (&read(inventory, "runs"), member.0, member.1), + None + ) + .await + ) + .await + .1 + ) + .await, + json!([ + NODES, + "missing view on nodes customers, inventory, orders, products; all requested nodes denied" + ]), + "{}: the audit keeps the missing nodes", + test_case.description + ); + stop_api(api).await; + close_test_store(fixture).await; + }); + } +} diff --git a/crates/sqlbuild-orchestration/tests/api/test_listing_denials.rs b/crates/sqlbuild-orchestration/tests/api/test_listing_denials.rs deleted file mode 100644 index 75824910e4..0000000000 --- a/crates/sqlbuild-orchestration/tests/api/test_listing_denials.rs +++ /dev/null @@ -1,132 +0,0 @@ -//! Refused listings name no node and read the same for an unassigned and an unknown location. - -use reqwest::header; -use serde_json::{Value, json}; -use uuid::Uuid; - -use crate::api::models::{ApiAuthentication, HumanLogin}; -use crate::api::tests::helpers::{start_api_with_authentication, stop_api}; -use crate::api::tests::listing_denials_helpers::{audited_scope, masked}; -use crate::api::tests::test_types::ListingDenialTestCase; -use crate::credentials::tests::location_helpers::create_location; -use crate::credentials::tests::role_helpers::role_fixture; -use crate::runs::tests::helpers::snapshot; -use crate::snapshots::main::publish_snapshot::publish_snapshot; -use crate::store::main::migrate_store::migrate_store; -use crate::store::tests::store_helpers::{close_test_store, open_test_store, run}; -use crate::store::tests::store_test_types::Backend; - -const LISTINGS: [&str; 6] = [ - "runs", - "catalog?schema=orders", - "statistics?schema=orders&name=orders", - "checks", - "checks/orders_not_null", - "maintenance", -]; - -const NODES: [&str; 4] = ["customers", "inventory", "orders", "products"]; - -#[test] -fn given_refused_listings_when_people_read_them_then_no_node_is_named_and_unknown_locations_look_unassigned() - { - let test_cases = [ - ListingDenialTestCase { - description: "SQLite listing denials", - backend: Backend::Sqlite, - expected_status: 403, - expected_error: "missing view on one or more nodes in this location; decision ", - }, - ListingDenialTestCase { - description: "PostgreSQL listing denials", - backend: Backend::Postgres, - expected_status: 403, - expected_error: "missing view on one or more nodes in this location; decision ", - }, - ]; - 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).await; - let inventory = create_location(store, "inventory", "orders-admin") - .await - .expect("unassigned location"); - publish_snapshot(store, &snapshot(inventory)) - .await - .expect("unassigned snapshot"); - let api = start_api_with_authentication( - store, - ApiAuthentication { - login: HumanLogin::Password("https://control.example.com".into()), - ..ApiAuthentication::default() - }, - ) - .await; - let client = reqwest::Client::new(); - let read = |location: Uuid, listing: &str| { - client - .get(format!( - "{}/api/control/locations/{location}/targets/development/{listing}", - api.url - )) - .header(header::HOST, "control.example.com") - .header( - header::COOKIE, - format!("__Host-sqb-session={}", roles.member.bearer), - ) - .send() - }; - for listing in LISTINGS { - let (partial, partial_decision) = - masked(read(roles.location_id, listing).await.expect("partial")).await; - let (unassigned, unassigned_decision) = - masked(read(inventory, listing).await.expect("unassigned")).await; - let (unknown, unknown_decision) = - masked(read(Uuid::now_v7(), listing).await.expect("unknown")).await; - let body: Value = - serde_json::from_str(partial["body"].as_str().expect("body")).expect("JSON"); - assert_eq!( - ( - partial["status"].clone(), - body["error"].clone(), - unassigned == unknown, - partial == unassigned, - ), - ( - json!(test_case.expected_status), - json!(test_case.expected_error), - true, - true - ), - "{}: {listing} names no node and hides whether the location exists", - test_case.description - ); - assert!( - NODES - .iter() - .all(|node| !partial["body"].as_str().unwrap_or_default().contains(node)), - "{}: {listing} body names a node", - test_case.description - ); - assert_eq!( - ( - audited_scope(store, unassigned_decision).await[0].clone(), - audited_scope(store, unknown_decision).await[0].clone(), - audited_scope(store, partial_decision).await[1] - .as_str() - .is_some_and(|reason| reason.contains("inventory")), - ), - (json!(NODES), json!([]), true), - "{}: {listing} audits every refusal with the full detail", - test_case.description - ); - } - stop_api(api).await; - close_test_store(fixture).await; - }); - } -} diff --git a/crates/sqlbuild-orchestration/tests/api/test_types.rs b/crates/sqlbuild-orchestration/tests/api/test_types.rs index be4ad753ec..b304469843 100644 --- a/crates/sqlbuild-orchestration/tests/api/test_types.rs +++ b/crates/sqlbuild-orchestration/tests/api/test_types.rs @@ -119,11 +119,13 @@ pub(crate) struct CliLoginTestCase { pub(crate) expected_exchanges: usize, } -pub(crate) struct ListingDenialTestCase { +/// A refused request's URL and, for writes, its JSON body. +pub(crate) type DenialRequest = (String, Option); + +pub(crate) struct DenialTestCase { pub(crate) description: &'static str, pub(crate) backend: Backend, pub(crate) expected_status: u16, - pub(crate) expected_error: &'static str, } pub(crate) struct CustomRoleApiTestCase { diff --git a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs index db79fbbf9f..387778bc48 100644 --- a/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs +++ b/crates/sqlbuild-orchestration/tests/credentials/test_project_roles.rs @@ -19,7 +19,7 @@ use crate::store::main::migrate_store::migrate_store; 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"; +const ADHOC_DENIED: &str = "missing run.adhoc permission for this target"; #[test] fn given_published_selector_when_it_matches_more_nodes_then_coverage_grows_on_publish() { @@ -103,13 +103,13 @@ fn given_selector_resolving_to_no_nodes_when_published_then_it_grants_nothing() 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"), + expected_after: Err("missing view permission for this target"), }, 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"), + expected_after: Err("missing view permission for this target"), }, ]; for test_case in test_cases { @@ -149,8 +149,8 @@ fn given_selector_resolving_to_no_nodes_when_published_then_it_grants_nothing() .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", + Err("missing run.adhoc permission for this target"), + "{}: the denial names no node", test_case.description ); close_test_store(fixture).await; @@ -165,22 +165,14 @@ fn given_grant_outside_a_triggers_job_when_the_member_overrides_it_then_both_tri 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", - ), + expected_before: Err("missing trigger.toggle permission for this target"), + expected_after: Err("missing sensor.reset_cursor permission for this target"), }, 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", - ), + expected_before: Err("missing trigger.toggle permission for this target"), + expected_after: Err("missing sensor.reset_cursor permission for this target"), }, ]; for test_case in test_cases { @@ -244,13 +236,13 @@ fn given_view_grant_on_part_of_the_location_when_listing_then_each_listing_is_de ProjectRoleTestCase { description: "SQLite location listings need view on every node", backend: Backend::Sqlite, - expected_before: Err("missing view on one or more nodes in this location"), + expected_before: Err("missing view permission for this target"), expected_after: Ok(SUPPORT_ROLE), }, ProjectRoleTestCase { description: "PostgreSQL location listings need view on every node", backend: Backend::Postgres, - expected_before: Err("missing view on one or more nodes in this location"), + expected_before: Err("missing view permission for this target"), expected_after: Ok(SUPPORT_ROLE), }, ]; @@ -298,13 +290,13 @@ fn given_role_removed_from_code_when_its_assignment_remains_then_it_grants_nothi 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"), + expected_after: Err("missing run.adhoc permission for this target"), }, 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"), + expected_after: Err("missing run.adhoc permission for this target"), }, ]; for test_case in test_cases { @@ -351,7 +343,7 @@ fn given_role_removed_from_code_when_its_assignment_remains_then_it_grants_nothi .await .as_deref() .map_err(String::as_str), - Err("missing permission in this scope; all requested nodes denied"), + Err("missing view permission for this target"), "{}: nor location-wide", test_case.description ); 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 dfa7f047ff..aee81e65cd 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/server.md @@ -172,5 +172,7 @@ permissions = ["view.evidence"] Assign a project role with `POST /api/admin/roles`, using the role key as `role`. Each publication replaces the location's project roles: new and changed grants apply at once, and removed roles stop applying. A selector covers newly matching nodes as soon as they are -published. Each change appears in the publication's change list as `role:`. A denial lists -the nodes the request was missing. +published. Each change appears in the publication's change list as `role:`. A denial names +only the missing permission, such as `missing view permission for this target`, and its decision +ID. It names no nodes, and an unknown location, job, trigger or run is refused with the same +response. The security audit event for that decision ID lists the nodes the request was missing. diff --git a/tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_denial.py b/tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_denial.py index 70784861e5..00906a3c9d 100644 --- a/tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_denial.py +++ b/tests/integration/src/sqlbuild/compiler/compiled_project/publication/test_denial.py @@ -53,12 +53,11 @@ def test_given_denied_browser_requests_when_returned_then_only_durable_decision_ ) unlisted: str = catalog.replace(api.location_id, str(uuid4())) decisions: dict[str, str] = {} - listing_denied: str = "missing view on one or more nodes in this location" - scope_denied: str = "missing permission in this scope; all requested nodes denied" + listing_denied: str = "missing view permission for this target" for path, reason in [ (catalog, listing_denied), (unlisted, listing_denied), - ("/api/control/audit", scope_denied), + ("/api/control/audit", "missing audit.view permission for this target"), ]: with pytest.raises(HTTPError) as rejected: person_read(api, path, session=session) @@ -97,8 +96,7 @@ def test_given_denied_browser_requests_when_returned_then_only_durable_decision_ body["error"] == { "authorization.denied": ( - "missing permission in this scope; all requested nodes denied; decision " - + decision + "missing job.run permission for this target; decision " + decision ), "authentication.denied": "authentication rejected", }[action] 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 index 61968b6f40..fc6221eb23 100644 --- 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 @@ -33,9 +33,7 @@ "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" - ), + expected_denial="missing view.sql permission for this target", ) ], ids=lambda case: case.description, diff --git a/website/src/content/docs/docs/cli/server.mdx b/website/src/content/docs/docs/cli/server.mdx index 91be26c0d9..37144b9e86 100644 --- a/website/src/content/docs/docs/cli/server.mdx +++ b/website/src/content/docs/docs/cli/server.mdx @@ -164,5 +164,7 @@ permissions = ["view.evidence"] Assign a project role with `POST /api/admin/roles`, using the role key as `role`. Each publication replaces the location's project roles: new and changed grants apply at once, and removed roles stop applying. A selector covers newly matching nodes as soon as they are -published. Each change appears in the publication's change list as `role:`. A denial lists -the nodes the request was missing. +published. Each change appears in the publication's change list as `role:`. A denial names +only the missing permission, such as `missing view permission for this target`, and its decision +ID. It names no nodes, and an unknown location, job, trigger or run is refused with the same +response. The security audit event for that decision ID lists the nodes the request was missing. From 66dcd1ec9608fcc9d567e6e8eb9ee36b9859f9c2 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 6 Nov 2026 16:02:31 +0000 Subject: [PATCH 3/4] wip: person reads for run nodes, triggers and ticks Person sessions can read a run's nodes, the target's triggers and a trigger's ticks. Each read authorizes like the runs list, with view on every current node of the target, before looking the object up, so an unknown run or trigger gets the same refusal as a forbidden one. A person who may view the whole target gets 404 for an unknown object. The machine reads share the same in-transaction queries. Validated locally with SQLite and PostgreSQL: orchestration API tests, including the forbidden-versus-unknown denial cases for the new reads (a lookup-before-authorize mutation fails them) and the per-role session matrix; clippy, fensu, and the regenerated TypeScript client check and test. Not yet reviewed; stacked on the listing-denial branch. --- clients/typescript/src/schema.d.ts | 178 ++++++++++++++++++ .../authentication/password_sessions.rs | 12 ++ .../src/api/_helpers/resources/mod.rs | 1 + .../api/_helpers/resources/person_reads.rs | 99 ++++++++++ .../sqlbuild-orchestration/src/api/models.rs | 7 +- .../src/declarations/_helpers/mod.rs | 1 + .../src/declarations/_helpers/reads.rs | 19 ++ .../declarations/main/list_person_triggers.rs | 29 +++ .../src/declarations/main/list_triggers.rs | 14 +- .../src/declarations/main/mod.rs | 1 + .../src/runs/_helpers/lifecycle/reads.rs | 32 +++- .../src/runs/main/list_person_run_nodes.rs | 35 ++++ .../src/runs/main/list_run_nodes.rs | 13 +- .../src/runs/main/mod.rs | 1 + .../src/scheduler/_helpers/mod.rs | 1 + .../src/scheduler/_helpers/reads.rs | 39 ++++ .../src/scheduler/main/list_person_ticks.rs | 36 ++++ .../src/scheduler/main/list_ticks.rs | 13 +- .../src/scheduler/main/mod.rs | 1 + .../tests/api/test_denials.rs | 39 +++- .../tests/api/test_person_sessions.rs | 53 ++++++ 21 files changed, 601 insertions(+), 23 deletions(-) create mode 100644 crates/sqlbuild-orchestration/src/api/_helpers/resources/person_reads.rs create mode 100644 crates/sqlbuild-orchestration/src/declarations/_helpers/reads.rs create mode 100644 crates/sqlbuild-orchestration/src/declarations/main/list_person_triggers.rs create mode 100644 crates/sqlbuild-orchestration/src/runs/main/list_person_run_nodes.rs create mode 100644 crates/sqlbuild-orchestration/src/scheduler/_helpers/reads.rs create mode 100644 crates/sqlbuild-orchestration/src/scheduler/main/list_person_ticks.rs diff --git a/clients/typescript/src/schema.d.ts b/clients/typescript/src/schema.d.ts index de824cbd63..91f70c1fef 100644 --- a/clients/typescript/src/schema.d.ts +++ b/clients/typescript/src/schema.d.ts @@ -402,6 +402,22 @@ export interface paths { patch?: never; trace?: never; }; + "/api/control/locations/{location_id}/targets/{target}/runs/{run_id}/nodes": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["listPersonRunNodes"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/control/locations/{location_id}/targets/{target}/statistics": { parameters: { query?: never; @@ -418,6 +434,38 @@ export interface paths { patch?: never; trace?: never; }; + "/api/control/locations/{location_id}/targets/{target}/triggers": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["listPersonTriggers"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/control/locations/{location_id}/targets/{target}/triggers/{trigger_id}/ticks": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["listPersonTicks"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/control/runs": { parameters: { query?: never; @@ -2896,6 +2944,51 @@ export interface operations { }; }; }; + listPersonRunNodes: { + parameters: { + query?: never; + header?: never; + path: { + location_id: string; + target: string; + run_id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["RunNodeRecord"][]; + }; + }; + /** @description Session rejected */ + 401: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 403: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ApiErrorBody"]; + }; + }; + /** @description Run not found for a person who may view the whole target */ + 404: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; readPersonStatistics: { parameters: { query: { @@ -2948,6 +3041,91 @@ export interface operations { }; }; }; + listPersonTriggers: { + parameters: { + query?: never; + header?: never; + path: { + location_id: string; + target: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["TriggerRecord"][]; + }; + }; + /** @description Session rejected */ + 401: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 403: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ApiErrorBody"]; + }; + }; + }; + }; + listPersonTicks: { + parameters: { + query?: { + /** @description Maximum recent ticks; defaults to 100, capped at 1000 */ + limit?: number; + }; + header?: never; + path: { + location_id: string; + target: string; + trigger_id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["TickRecord"][]; + }; + }; + /** @description Session rejected */ + 401: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 403: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ApiErrorBody"]; + }; + }; + /** @description Trigger not found for a person who may view the whole target */ + 404: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; requestJobRun: { parameters: { query?: never; diff --git a/crates/sqlbuild-orchestration/src/api/_helpers/authentication/password_sessions.rs b/crates/sqlbuild-orchestration/src/api/_helpers/authentication/password_sessions.rs index 0e14f0579e..dfd76e1526 100644 --- a/crates/sqlbuild-orchestration/src/api/_helpers/authentication/password_sessions.rs +++ b/crates/sqlbuild-orchestration/src/api/_helpers/authentication/password_sessions.rs @@ -89,6 +89,18 @@ pub(crate) fn person_routes() -> Router { "/api/control/locations/{location_id}/targets/{target}/runs", get(crate::api::_helpers::resources::person_operations::recent_runs), ) + .route( + "/api/control/locations/{location_id}/targets/{target}/runs/{run_id}/nodes", + get(crate::api::_helpers::resources::person_reads::run_nodes), + ) + .route( + "/api/control/locations/{location_id}/targets/{target}/triggers", + get(crate::api::_helpers::resources::person_reads::triggers), + ) + .route( + "/api/control/locations/{location_id}/targets/{target}/triggers/{trigger_id}/ticks", + get(crate::api::_helpers::resources::person_reads::ticks), + ) .route( "/api/control/locations/{location_id}/targets/{target}/catalog", get(crate::api::_helpers::resources::person_operations::catalog), diff --git a/crates/sqlbuild-orchestration/src/api/_helpers/resources/mod.rs b/crates/sqlbuild-orchestration/src/api/_helpers/resources/mod.rs index be262332c1..e637174ff0 100644 --- a/crates/sqlbuild-orchestration/src/api/_helpers/resources/mod.rs +++ b/crates/sqlbuild-orchestration/src/api/_helpers/resources/mod.rs @@ -10,6 +10,7 @@ pub(crate) mod person_location_work; pub(crate) mod person_maintenance; pub(crate) mod person_node_sql; pub(crate) mod person_operations; +pub(crate) mod person_reads; pub(crate) mod router; pub(crate) mod run_reads; pub(crate) mod runs; diff --git a/crates/sqlbuild-orchestration/src/api/_helpers/resources/person_reads.rs b/crates/sqlbuild-orchestration/src/api/_helpers/resources/person_reads.rs new file mode 100644 index 0000000000..cd4392dea9 --- /dev/null +++ b/crates/sqlbuild-orchestration/src/api/_helpers/resources/person_reads.rs @@ -0,0 +1,99 @@ +//! Person reads of run nodes, triggers and ticks; refusal precedes every lookup, so unknown objects look forbidden. + +use axum::extract::{Path, Query, State}; +use axum::http::StatusCode; +use axum::{Extension, Json}; +use uuid::Uuid; + +use crate::api::errors::ApiError; +use crate::api::models::ReadQuery; +use crate::credentials::models::{AuditContext, AuthenticatedPerson, DefaultRole}; +use crate::declarations::main::list_person_triggers::list_person_triggers; +use crate::declarations::models::TriggerRecord; +use crate::runs::main::list_person_run_nodes::list_person_run_nodes; +use crate::runs::models::RunNodeRecord; +use crate::scheduler::main::list_person_ticks::list_person_ticks; +use crate::scheduler::models::TickRecord; +use crate::store::models::ControlStore; + +#[utoipa::path(get, path = "/api/control/locations/{location_id}/targets/{target}/runs/{run_id}/nodes", operation_id = "listPersonRunNodes", + params(("location_id" = Uuid, Path), ("target" = String, Path), ("run_id" = Uuid, Path)), + responses((status = 200, body = Vec), (status = 401, description = "Session rejected"), + (status = 403, body = crate::api::models::ApiErrorBody), + (status = 404, description = "Run not found for a person who may view the whole target")), + security(("session" = []), ("cliSession" = [])))] +pub(crate) async fn run_nodes( + (State(store), Extension(context)): (State, Extension), + Extension(default_role): Extension, + Path((location_id, target, run_id)): Path<(Uuid, String, Uuid)>, + Extension(AuthenticatedPerson(principal, _)): Extension, +) -> Result>, ApiError> { + list_person_run_nodes( + &store, + (&context, principal), + (location_id, &target, run_id), + default_role, + ) + .await? + .map(Json) + .ok_or_else(|| not_found("run not found")) +} + +#[utoipa::path(get, path = "/api/control/locations/{location_id}/targets/{target}/triggers", operation_id = "listPersonTriggers", + params(("location_id" = Uuid, Path), ("target" = String, Path)), + responses((status = 200, body = Vec), (status = 401, description = "Session rejected"), + (status = 403, body = crate::api::models::ApiErrorBody)), security(("session" = []), ("cliSession" = [])))] +pub(crate) async fn triggers( + (State(store), Extension(context)): (State, Extension), + Extension(default_role): Extension, + Path((location_id, target)): Path<(Uuid, String)>, + Extension(AuthenticatedPerson(principal, _)): Extension, +) -> Result>, ApiError> { + Ok(Json( + list_person_triggers( + &store, + (&context, principal), + (location_id, &target), + default_role, + ) + .await?, + )) +} + +#[utoipa::path(get, path = "/api/control/locations/{location_id}/targets/{target}/triggers/{trigger_id}/ticks", operation_id = "listPersonTicks", + params(("location_id" = Uuid, Path), ("target" = String, Path), ("trigger_id" = Uuid, Path), + ("limit" = Option, Query, description = "Maximum recent ticks; defaults to 100, capped at 1000")), + responses((status = 200, body = Vec), (status = 401, description = "Session rejected"), + (status = 403, body = crate::api::models::ApiErrorBody), + (status = 404, description = "Trigger not found for a person who may view the whole target")), + security(("session" = []), ("cliSession" = [])))] +pub(crate) async fn ticks( + (State(store), Extension(context)): (State, Extension), + Extension(default_role): Extension, + (Path((location_id, target, trigger_id)), Query(options)): ( + Path<(Uuid, String, Uuid)>, + Query, + ), + Extension(AuthenticatedPerson(principal, _)): Extension, +) -> Result>, ApiError> { + list_person_ticks( + &store, + (&context, principal), + ( + (location_id, &target), + (trigger_id, options.limit.unwrap_or(100)), + ), + default_role, + ) + .await? + .map(Json) + .ok_or_else(|| not_found("trigger not found")) +} + +fn not_found(message: &str) -> ApiError { + ApiError { + status: StatusCode::NOT_FOUND, + message: message.into(), + decision_id: None, + } +} diff --git a/crates/sqlbuild-orchestration/src/api/models.rs b/crates/sqlbuild-orchestration/src/api/models.rs index 0b13d5a793..9eecba734a 100644 --- a/crates/sqlbuild-orchestration/src/api/models.rs +++ b/crates/sqlbuild-orchestration/src/api/models.rs @@ -6,8 +6,8 @@ use crate::api::_helpers::authentication::password_sessions; use crate::api::_helpers::authentication::{cli_login, oidc}; use crate::api::_helpers::resources::{ administrators, audit, changes, location_health, location_work, person_checks, - person_location_work, person_maintenance, person_node_sql, person_operations, run_reads, runs, - snapshots, trigger_reads, warehouse_metadata, + person_location_work, person_maintenance, person_node_sql, person_operations, person_reads, + run_reads, runs, snapshots, trigger_reads, warehouse_metadata, }; #[derive(Debug, utoipa::OpenApi)] @@ -20,6 +20,9 @@ use crate::api::_helpers::resources::{ person_operations::rerun, person_operations::change_trigger, person_operations::recent_runs, + person_reads::run_nodes, + person_reads::triggers, + person_reads::ticks, person_operations::catalog, person_operations::statistics, person_operations::fingerprint, diff --git a/crates/sqlbuild-orchestration/src/declarations/_helpers/mod.rs b/crates/sqlbuild-orchestration/src/declarations/_helpers/mod.rs index 90ce175a05..dd82f3f543 100644 --- a/crates/sqlbuild-orchestration/src/declarations/_helpers/mod.rs +++ b/crates/sqlbuild-orchestration/src/declarations/_helpers/mod.rs @@ -3,5 +3,6 @@ pub(crate) mod documents; pub(crate) mod operator_change; pub(crate) mod operator_state; pub(crate) mod publication; +pub(crate) mod reads; pub(crate) mod trigger_scope; pub(crate) mod trigger_state; diff --git a/crates/sqlbuild-orchestration/src/declarations/_helpers/reads.rs b/crates/sqlbuild-orchestration/src/declarations/_helpers/reads.rs new file mode 100644 index 0000000000..a69248fd01 --- /dev/null +++ b/crates/sqlbuild-orchestration/src/declarations/_helpers/reads.rs @@ -0,0 +1,19 @@ +//! Trigger reads inside a caller's transaction, so authorization and results share one view. + +use uuid::Uuid; + +use crate::declarations::models::TriggerRecord; +use crate::store::errors::ControlError; +use crate::store::main::query_records::query_records; +use crate::store::models::{ControlTransaction, Parameter}; + +pub(crate) async fn triggers_in( + transaction: ControlTransaction, + (location_id, target): (Uuid, &str), +) -> Result<(ControlTransaction, Vec), ControlError> { + query_records(transaction, + "SELECT t.trigger_id, t.name, t.job_key, t.definition_json, s.status, s.last_tick_for, s.next_due_at \ + FROM sqb_triggers t JOIN sqb_trigger_state s ON s.trigger_id=t.trigger_id \ + WHERE t.location_id=$1 AND t.target=$2 AND t.removed_in_snapshot_id IS NULL ORDER BY t.name", + vec![Parameter::Id(location_id), Parameter::Text(target.into())]).await +} diff --git a/crates/sqlbuild-orchestration/src/declarations/main/list_person_triggers.rs b/crates/sqlbuild-orchestration/src/declarations/main/list_person_triggers.rs new file mode 100644 index 0000000000..472327f34c --- /dev/null +++ b/crates/sqlbuild-orchestration/src/declarations/main/list_person_triggers.rs @@ -0,0 +1,29 @@ +//! A person lists triggers only with view on every current node of the target. + +use uuid::Uuid; + +use crate::authorization::main::authorize_listing::authorize_listing; +use crate::credentials::models::{AuditContext, DefaultRole}; +use crate::declarations::_helpers::reads::triggers_in; +use crate::declarations::models::TriggerRecord; +use crate::store::errors::ControlError; +use crate::store::main::{begin::begin, commit::commit}; +use crate::store::models::{ControlStore, TransactionMode}; + +pub async fn list_person_triggers( + store: &ControlStore, + (context, credentials): (&AuditContext, Uuid), + (location_id, target): (Uuid, &str), + default_role: DefaultRole, +) -> Result, ControlError> { + let (transaction, _) = authorize_listing( + begin(store, TransactionMode::Write).await?, + (context, credentials), + (location_id, target), + default_role, + ) + .await?; + let (transaction, triggers) = triggers_in(transaction, (location_id, target)).await?; + commit(transaction).await?; + Ok(triggers) +} diff --git a/crates/sqlbuild-orchestration/src/declarations/main/list_triggers.rs b/crates/sqlbuild-orchestration/src/declarations/main/list_triggers.rs index fbddbed582..f28763272e 100644 --- a/crates/sqlbuild-orchestration/src/declarations/main/list_triggers.rs +++ b/crates/sqlbuild-orchestration/src/declarations/main/list_triggers.rs @@ -2,19 +2,19 @@ use uuid::Uuid; +use crate::declarations::_helpers::reads::triggers_in; use crate::declarations::models::TriggerRecord; use crate::store::errors::ControlError; -use crate::store::main::read_records::read_records; -use crate::store::models::{ControlStore, Parameter}; +use crate::store::main::{begin::begin, commit::commit}; +use crate::store::models::{ControlStore, TransactionMode}; pub async fn list_triggers( store: &ControlStore, location_id: Uuid, target: &str, ) -> Result, ControlError> { - read_records(store, - "SELECT t.trigger_id, t.name, t.job_key, t.definition_json, s.status, s.last_tick_for, s.next_due_at \ - FROM sqb_triggers t JOIN sqb_trigger_state s ON s.trigger_id=t.trigger_id \ - WHERE t.location_id=$1 AND t.target=$2 AND t.removed_in_snapshot_id IS NULL ORDER BY t.name", - vec![Parameter::Id(location_id), Parameter::Text(target.into())]).await + let transaction = begin(store, TransactionMode::Read).await?; + let (transaction, triggers) = triggers_in(transaction, (location_id, target)).await?; + commit(transaction).await?; + Ok(triggers) } diff --git a/crates/sqlbuild-orchestration/src/declarations/main/mod.rs b/crates/sqlbuild-orchestration/src/declarations/main/mod.rs index 23cab85fba..1a14985c37 100644 --- a/crates/sqlbuild-orchestration/src/declarations/main/mod.rs +++ b/crates/sqlbuild-orchestration/src/declarations/main/mod.rs @@ -2,6 +2,7 @@ pub mod change_person_trigger; pub mod change_trigger; pub(crate) mod frozen_definitions; pub mod group_dependencies; +pub mod list_person_triggers; pub mod list_trigger_events; pub mod list_triggers; pub mod pause_development_triggers; diff --git a/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs b/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs index de9d1ba696..c89d116b57 100644 --- a/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs +++ b/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs @@ -2,8 +2,9 @@ use uuid::Uuid; -use crate::runs::models::RunRecord; +use crate::runs::models::{RunNodeRecord, RunRecord}; use crate::store::errors::ControlError; +use crate::store::main::query::query; use crate::store::main::query_records::query_records; use crate::store::models::{ControlTransaction, Parameter}; @@ -17,3 +18,32 @@ pub(crate) async fn list_runs_in( FROM sqb_runs WHERE location_id=$1 AND target=$2 ORDER BY queued_at DESC, run_id DESC LIMIT $3", vec![Parameter::Id(location_id), Parameter::Text(target.into()), Parameter::Integer(i64::from(limit.min(1000)))]).await } + +pub(crate) async fn run_nodes_in( + transaction: ControlTransaction, + run_id: Uuid, +) -> Result<(ControlTransaction, Vec), ControlError> { + query_records(transaction, + "SELECT node_key, status, started_at, ended_at, rows_affected, error_code, skipped_because_of \ + FROM sqb_run_nodes WHERE run_id=$1 AND kind='node' ORDER BY node_key", + vec![Parameter::Id(run_id)]).await +} + +/// Whether `run_id` belongs to this location and target. +pub(crate) async fn run_in_scope( + transaction: ControlTransaction, + (location_id, target): (Uuid, &str), + run_id: Uuid, +) -> Result<(ControlTransaction, bool), ControlError> { + let (transaction, rows) = query( + transaction, + "SELECT run_id FROM sqb_runs WHERE run_id=$1 AND location_id=$2 AND target=$3", + vec![ + Parameter::Id(run_id), + Parameter::Id(location_id), + Parameter::Text(target.into()), + ], + ) + .await?; + Ok((transaction, !rows.is_empty())) +} diff --git a/crates/sqlbuild-orchestration/src/runs/main/list_person_run_nodes.rs b/crates/sqlbuild-orchestration/src/runs/main/list_person_run_nodes.rs new file mode 100644 index 0000000000..fbaeebbfb9 --- /dev/null +++ b/crates/sqlbuild-orchestration/src/runs/main/list_person_run_nodes.rs @@ -0,0 +1,35 @@ +//! A person sees a run's nodes only with view on every current node; an unknown run is refused alike. + +use uuid::Uuid; + +use crate::authorization::main::authorize_listing::authorize_listing; +use crate::credentials::models::{AuditContext, DefaultRole}; +use crate::runs::_helpers::lifecycle::reads::{run_in_scope, run_nodes_in}; +use crate::runs::models::RunNodeRecord; +use crate::store::errors::ControlError; +use crate::store::main::{begin::begin, commit::commit}; +use crate::store::models::{ControlStore, TransactionMode}; + +/// `None` when the run is not in this location and target; authorization runs first either way. +pub async fn list_person_run_nodes( + store: &ControlStore, + (context, credentials): (&AuditContext, Uuid), + (location_id, target, run_id): (Uuid, &str, Uuid), + default_role: DefaultRole, +) -> Result>, ControlError> { + let (transaction, _) = authorize_listing( + begin(store, TransactionMode::Write).await?, + (context, credentials), + (location_id, target), + default_role, + ) + .await?; + let (transaction, known) = run_in_scope(transaction, (location_id, target), run_id).await?; + if !known { + commit(transaction).await?; + return Ok(None); + } + let (transaction, nodes) = run_nodes_in(transaction, run_id).await?; + commit(transaction).await?; + Ok(Some(nodes)) +} diff --git a/crates/sqlbuild-orchestration/src/runs/main/list_run_nodes.rs b/crates/sqlbuild-orchestration/src/runs/main/list_run_nodes.rs index 29b4c84060..c50964137a 100644 --- a/crates/sqlbuild-orchestration/src/runs/main/list_run_nodes.rs +++ b/crates/sqlbuild-orchestration/src/runs/main/list_run_nodes.rs @@ -2,18 +2,19 @@ use uuid::Uuid; +use crate::runs::_helpers::lifecycle::reads::run_nodes_in; use crate::runs::models::RunNodeRecord; use crate::store::errors::ControlError; -use crate::store::main::read_records::read_records; -use crate::store::models::{ControlStore, Parameter}; +use crate::store::main::{begin::begin, commit::commit}; +use crate::store::models::{ControlStore, TransactionMode}; /// The API authorizes visibility of the run before reading its nodes. pub async fn list_run_nodes( store: &ControlStore, run_id: Uuid, ) -> Result, ControlError> { - read_records(store, - "SELECT node_key, status, started_at, ended_at, rows_affected, error_code, skipped_because_of \ - FROM sqb_run_nodes WHERE run_id=$1 AND kind='node' ORDER BY node_key", - vec![Parameter::Id(run_id)]).await + let transaction = begin(store, TransactionMode::Read).await?; + let (transaction, nodes) = run_nodes_in(transaction, run_id).await?; + commit(transaction).await?; + Ok(nodes) } diff --git a/crates/sqlbuild-orchestration/src/runs/main/mod.rs b/crates/sqlbuild-orchestration/src/runs/main/mod.rs index bc36f01e6c..ce37545bfb 100644 --- a/crates/sqlbuild-orchestration/src/runs/main/mod.rs +++ b/crates/sqlbuild-orchestration/src/runs/main/mod.rs @@ -4,6 +4,7 @@ pub mod change_run; pub mod claim_next_run; pub(crate) mod frozen_job_request; pub mod heartbeat_run; +pub mod list_person_run_nodes; pub mod list_person_runs; pub mod list_run_claims; pub mod list_run_nodes; diff --git a/crates/sqlbuild-orchestration/src/scheduler/_helpers/mod.rs b/crates/sqlbuild-orchestration/src/scheduler/_helpers/mod.rs index bee69ea1d3..aa49638c2b 100644 --- a/crates/sqlbuild-orchestration/src/scheduler/_helpers/mod.rs +++ b/crates/sqlbuild-orchestration/src/scheduler/_helpers/mod.rs @@ -3,5 +3,6 @@ pub(crate) mod drain_due_triggers; pub(crate) mod drain_sensors; pub(crate) mod expansion; pub(crate) mod persistence; +pub(crate) mod reads; pub(crate) mod tick_retention; pub(crate) mod ticks; diff --git a/crates/sqlbuild-orchestration/src/scheduler/_helpers/reads.rs b/crates/sqlbuild-orchestration/src/scheduler/_helpers/reads.rs new file mode 100644 index 0000000000..6ae7e4d448 --- /dev/null +++ b/crates/sqlbuild-orchestration/src/scheduler/_helpers/reads.rs @@ -0,0 +1,39 @@ +//! Tick history inside a caller's transaction, so authorization and results share one view. + +use uuid::Uuid; + +use crate::scheduler::models::TickRecord; +use crate::store::errors::ControlError; +use crate::store::main::query::query; +use crate::store::main::query_records::query_records; +use crate::store::models::{ControlTransaction, Parameter}; + +pub(crate) async fn ticks_in( + transaction: ControlTransaction, + trigger_id: Uuid, + limit: u32, +) -> Result<(ControlTransaction, Vec), ControlError> { + query_records(transaction, + "SELECT tick_id, trigger_id, scheduled_for, evaluated_at, epoch, outcome, run_id, covered_by_run_id \ + FROM sqb_ticks WHERE trigger_id=$1 ORDER BY scheduled_for DESC LIMIT $2", + vec![Parameter::Id(trigger_id), Parameter::Integer(i64::from(limit.min(1000)))]).await +} + +/// Whether `trigger_id` was ever declared in this location and target, including removed triggers. +pub(crate) async fn trigger_in_scope( + transaction: ControlTransaction, + (location_id, target): (Uuid, &str), + trigger_id: Uuid, +) -> Result<(ControlTransaction, bool), ControlError> { + let (transaction, rows) = query( + transaction, + "SELECT trigger_id FROM sqb_triggers WHERE trigger_id=$1 AND location_id=$2 AND target=$3", + vec![ + Parameter::Id(trigger_id), + Parameter::Id(location_id), + Parameter::Text(target.into()), + ], + ) + .await?; + Ok((transaction, !rows.is_empty())) +} diff --git a/crates/sqlbuild-orchestration/src/scheduler/main/list_person_ticks.rs b/crates/sqlbuild-orchestration/src/scheduler/main/list_person_ticks.rs new file mode 100644 index 0000000000..1a15efa392 --- /dev/null +++ b/crates/sqlbuild-orchestration/src/scheduler/main/list_person_ticks.rs @@ -0,0 +1,36 @@ +//! A person sees a trigger's ticks only with view on every current node; an unknown trigger is refused alike. + +use uuid::Uuid; + +use crate::authorization::main::authorize_listing::authorize_listing; +use crate::credentials::models::{AuditContext, DefaultRole}; +use crate::scheduler::_helpers::reads::{ticks_in, trigger_in_scope}; +use crate::scheduler::models::TickRecord; +use crate::store::errors::ControlError; +use crate::store::main::{begin::begin, commit::commit}; +use crate::store::models::{ControlStore, TransactionMode}; + +/// `None` when the trigger is not in this location and target; authorization runs first either way. +pub async fn list_person_ticks( + store: &ControlStore, + (context, credentials): (&AuditContext, Uuid), + ((location_id, target), (trigger_id, limit)): ((Uuid, &str), (Uuid, u32)), + default_role: DefaultRole, +) -> Result>, ControlError> { + let (transaction, _) = authorize_listing( + begin(store, TransactionMode::Write).await?, + (context, credentials), + (location_id, target), + default_role, + ) + .await?; + let (transaction, known) = + trigger_in_scope(transaction, (location_id, target), trigger_id).await?; + if !known { + commit(transaction).await?; + return Ok(None); + } + let (transaction, ticks) = ticks_in(transaction, trigger_id, limit).await?; + commit(transaction).await?; + Ok(Some(ticks)) +} diff --git a/crates/sqlbuild-orchestration/src/scheduler/main/list_ticks.rs b/crates/sqlbuild-orchestration/src/scheduler/main/list_ticks.rs index 77699477a3..6ecd4d98bf 100644 --- a/crates/sqlbuild-orchestration/src/scheduler/main/list_ticks.rs +++ b/crates/sqlbuild-orchestration/src/scheduler/main/list_ticks.rs @@ -2,18 +2,19 @@ use uuid::Uuid; +use crate::scheduler::_helpers::reads::ticks_in; use crate::scheduler::models::TickRecord; use crate::store::errors::ControlError; -use crate::store::main::read_records::read_records; -use crate::store::models::{ControlStore, Parameter}; +use crate::store::main::{begin::begin, commit::commit}; +use crate::store::models::{ControlStore, TransactionMode}; pub async fn list_ticks( store: &ControlStore, trigger_id: Uuid, limit: u32, ) -> Result, ControlError> { - read_records(store, - "SELECT tick_id, trigger_id, scheduled_for, evaluated_at, epoch, outcome, run_id, covered_by_run_id \ - FROM sqb_ticks WHERE trigger_id=$1 ORDER BY scheduled_for DESC LIMIT $2", - vec![Parameter::Id(trigger_id), Parameter::Integer(i64::from(limit.min(1000)))]).await + let transaction = begin(store, TransactionMode::Read).await?; + let (transaction, ticks) = ticks_in(transaction, trigger_id, limit).await?; + commit(transaction).await?; + Ok(ticks) } diff --git a/crates/sqlbuild-orchestration/src/scheduler/main/mod.rs b/crates/sqlbuild-orchestration/src/scheduler/main/mod.rs index f91c3d1180..5875aa88b5 100644 --- a/crates/sqlbuild-orchestration/src/scheduler/main/mod.rs +++ b/crates/sqlbuild-orchestration/src/scheduler/main/mod.rs @@ -5,6 +5,7 @@ pub(in crate::scheduler) mod _plan_ticks; pub(in crate::scheduler) mod _previous_cron_time; pub(in crate::scheduler) mod _scheduler_cycle; pub(crate) mod check_lease; +pub mod list_person_ticks; pub mod list_ticks; pub(crate) mod next_schedule_time; pub mod parse_cron; diff --git a/crates/sqlbuild-orchestration/tests/api/test_denials.rs b/crates/sqlbuild-orchestration/tests/api/test_denials.rs index 838b41ebb7..fc976ae45c 100644 --- a/crates/sqlbuild-orchestration/tests/api/test_denials.rs +++ b/crates/sqlbuild-orchestration/tests/api/test_denials.rs @@ -20,8 +20,9 @@ use crate::store::tests::store_helpers::{close_test_store, open_test_store, run} use crate::store::tests::store_test_types::Backend; /// Location-scoped reads and the action each needs. -const READS: [(&str, &str); 8] = [ +const READS: [(&str, &str); 9] = [ ("runs", "view"), + ("triggers", "view"), ("catalog?schema=orders", "view"), ("statistics?schema=orders&name=orders", "view"), ("checks", "view"), @@ -129,6 +130,42 @@ fn given_refused_requests_when_people_send_them_then_no_node_is_named_and_unknow }) .collect(); cases.extend([ + ( + "run nodes".to_owned(), + "view", + vec![ + ( + read(inventory, &format!("runs/{}/nodes", queued.run_id)), + None, + ), + (read(inventory, &format!("runs/{unknown_run}/nodes")), None), + ( + read(roles.location_id, &format!("runs/{unknown_run}/nodes")), + None, + ), + ], + ), + ( + "trigger ticks".to_owned(), + "view", + vec![ + ( + read(inventory, &format!("triggers/{trigger_id}/ticks")), + None, + ), + ( + read(inventory, &format!("triggers/{unknown_trigger}/ticks")), + None, + ), + ( + read( + roles.location_id, + &format!("triggers/{unknown_trigger}/ticks"), + ), + None, + ), + ], + ), ( "job run".to_owned(), "job.run", diff --git a/crates/sqlbuild-orchestration/tests/api/test_person_sessions.rs b/crates/sqlbuild-orchestration/tests/api/test_person_sessions.rs index 911fc980b2..f776628da0 100644 --- a/crates/sqlbuild-orchestration/tests/api/test_person_sessions.rs +++ b/crates/sqlbuild-orchestration/tests/api/test_person_sessions.rs @@ -235,6 +235,59 @@ fn given_browser_session_roles_when_work_is_requested_then_scope_revocation_and_ role_case.expected_metadata == StatusCode::OK.as_u16() ); assert!(!page.to_string().contains("select 1 as id")); + let known_reads = [ + format!("{metadata}/{}/nodes", seed_run.run_id), + format!("/api/control/locations/{location}/targets/development/triggers"), + format!( + "/api/control/locations/{location}/targets/development/triggers/{}/ticks", + trigger.trigger_id + ), + ]; + let unknown_reads = [ + format!("{metadata}/{}/nodes", Uuid::now_v7()), + format!( + "/api/control/locations/{location}/targets/development/triggers/{}/ticks", + Uuid::now_v7() + ), + ]; + for read_path in &known_reads { + let detail = request( + &provider.client, + session, + (Method::GET, read_path), + (&Value::Null, &session.csrf), + ) + .await; + assert_eq!( + detail.status().as_u16(), + role_case.expected_metadata, + "{read_path}" + ); + let detail: Value = detail.json().await.expect("detail response"); + assert_eq!( + detail.is_array(), + role_case.expected_metadata == StatusCode::OK.as_u16(), + "{read_path}" + ); + } + for read_path in &unknown_reads { + let missing = request( + &provider.client, + session, + (Method::GET, read_path), + (&Value::Null, &session.csrf), + ) + .await; + assert_eq!( + missing.status().as_u16(), + if role_case.expected_metadata == StatusCode::OK.as_u16() { + StatusCode::NOT_FOUND.as_u16() + } else { + role_case.expected_metadata + }, + "{read_path}" + ); + } let sql = request( &provider.client, session, From f9a7b3998b0de8628e79c4adf334186b813ca429 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 6 Nov 2026 18:00:35 +0000 Subject: [PATCH 4/4] refactor: share the run and trigger target-scope check One store read decides whether a person's target-scoped read may name a run or a trigger. The handler and per-record read shapes join the existing duplicate allowlists. --- .../src/authorization/main/mod.rs | 1 + .../src/authorization/main/row_in_target.rs | 35 +++++++++++++++++++ .../src/runs/_helpers/lifecycle/reads.rs | 20 ----------- .../src/runs/main/list_person_run_nodes.rs | 8 +++-- .../src/scheduler/_helpers/reads.rs | 20 ----------- .../src/scheduler/main/list_person_ticks.rs | 13 ++++--- .../src/store/models.rs | 7 ++++ fensu.toml | 4 +-- 8 files changed, 59 insertions(+), 49 deletions(-) create mode 100644 crates/sqlbuild-orchestration/src/authorization/main/row_in_target.rs diff --git a/crates/sqlbuild-orchestration/src/authorization/main/mod.rs b/crates/sqlbuild-orchestration/src/authorization/main/mod.rs index 6c4ddfef68..584b8e5906 100644 --- a/crates/sqlbuild-orchestration/src/authorization/main/mod.rs +++ b/crates/sqlbuild-orchestration/src/authorization/main/mod.rs @@ -12,3 +12,4 @@ pub(crate) mod publish_group_roles; pub(crate) mod read_audit_events; pub mod read_security_audit; pub(crate) mod refuse_unknown; +pub(crate) mod row_in_target; diff --git a/crates/sqlbuild-orchestration/src/authorization/main/row_in_target.rs b/crates/sqlbuild-orchestration/src/authorization/main/row_in_target.rs new file mode 100644 index 0000000000..7b72aab83e --- /dev/null +++ b/crates/sqlbuild-orchestration/src/authorization/main/row_in_target.rs @@ -0,0 +1,35 @@ +//! Whether a run or trigger, including a removed trigger, belongs to one location and target. + +use uuid::Uuid; + +use crate::store::errors::ControlError; +use crate::store::main::query::query; +use crate::store::models::{ControlTransaction, Parameter, TargetRow}; + +pub(crate) async fn row_in_target( + transaction: ControlTransaction, + (location_id, target): (Uuid, &str), + row: TargetRow, +) -> Result<(ControlTransaction, bool), ControlError> { + let (sql, id) = match row { + TargetRow::Run(run_id) => ( + "SELECT run_id FROM sqb_runs WHERE run_id=$1 AND location_id=$2 AND target=$3", + run_id, + ), + TargetRow::Trigger(trigger_id) => ( + "SELECT trigger_id FROM sqb_triggers WHERE trigger_id=$1 AND location_id=$2 AND target=$3", + trigger_id, + ), + }; + let (transaction, rows) = query( + transaction, + sql, + vec![ + Parameter::Id(id), + Parameter::Id(location_id), + Parameter::Text(target.into()), + ], + ) + .await?; + Ok((transaction, !rows.is_empty())) +} diff --git a/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs b/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs index c89d116b57..e3f248bb67 100644 --- a/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs +++ b/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs @@ -4,7 +4,6 @@ use uuid::Uuid; use crate::runs::models::{RunNodeRecord, RunRecord}; use crate::store::errors::ControlError; -use crate::store::main::query::query; use crate::store::main::query_records::query_records; use crate::store::models::{ControlTransaction, Parameter}; @@ -28,22 +27,3 @@ pub(crate) async fn run_nodes_in( FROM sqb_run_nodes WHERE run_id=$1 AND kind='node' ORDER BY node_key", vec![Parameter::Id(run_id)]).await } - -/// Whether `run_id` belongs to this location and target. -pub(crate) async fn run_in_scope( - transaction: ControlTransaction, - (location_id, target): (Uuid, &str), - run_id: Uuid, -) -> Result<(ControlTransaction, bool), ControlError> { - let (transaction, rows) = query( - transaction, - "SELECT run_id FROM sqb_runs WHERE run_id=$1 AND location_id=$2 AND target=$3", - vec![ - Parameter::Id(run_id), - Parameter::Id(location_id), - Parameter::Text(target.into()), - ], - ) - .await?; - Ok((transaction, !rows.is_empty())) -} diff --git a/crates/sqlbuild-orchestration/src/runs/main/list_person_run_nodes.rs b/crates/sqlbuild-orchestration/src/runs/main/list_person_run_nodes.rs index fbaeebbfb9..4634d81121 100644 --- a/crates/sqlbuild-orchestration/src/runs/main/list_person_run_nodes.rs +++ b/crates/sqlbuild-orchestration/src/runs/main/list_person_run_nodes.rs @@ -3,12 +3,13 @@ use uuid::Uuid; use crate::authorization::main::authorize_listing::authorize_listing; +use crate::authorization::main::row_in_target::row_in_target; use crate::credentials::models::{AuditContext, DefaultRole}; -use crate::runs::_helpers::lifecycle::reads::{run_in_scope, run_nodes_in}; +use crate::runs::_helpers::lifecycle::reads::run_nodes_in; use crate::runs::models::RunNodeRecord; use crate::store::errors::ControlError; use crate::store::main::{begin::begin, commit::commit}; -use crate::store::models::{ControlStore, TransactionMode}; +use crate::store::models::{ControlStore, TargetRow, TransactionMode}; /// `None` when the run is not in this location and target; authorization runs first either way. pub async fn list_person_run_nodes( @@ -24,7 +25,8 @@ pub async fn list_person_run_nodes( default_role, ) .await?; - let (transaction, known) = run_in_scope(transaction, (location_id, target), run_id).await?; + let (transaction, known) = + row_in_target(transaction, (location_id, target), TargetRow::Run(run_id)).await?; if !known { commit(transaction).await?; return Ok(None); diff --git a/crates/sqlbuild-orchestration/src/scheduler/_helpers/reads.rs b/crates/sqlbuild-orchestration/src/scheduler/_helpers/reads.rs index 6ae7e4d448..ac1d3463e5 100644 --- a/crates/sqlbuild-orchestration/src/scheduler/_helpers/reads.rs +++ b/crates/sqlbuild-orchestration/src/scheduler/_helpers/reads.rs @@ -4,7 +4,6 @@ use uuid::Uuid; use crate::scheduler::models::TickRecord; use crate::store::errors::ControlError; -use crate::store::main::query::query; use crate::store::main::query_records::query_records; use crate::store::models::{ControlTransaction, Parameter}; @@ -18,22 +17,3 @@ pub(crate) async fn ticks_in( FROM sqb_ticks WHERE trigger_id=$1 ORDER BY scheduled_for DESC LIMIT $2", vec![Parameter::Id(trigger_id), Parameter::Integer(i64::from(limit.min(1000)))]).await } - -/// Whether `trigger_id` was ever declared in this location and target, including removed triggers. -pub(crate) async fn trigger_in_scope( - transaction: ControlTransaction, - (location_id, target): (Uuid, &str), - trigger_id: Uuid, -) -> Result<(ControlTransaction, bool), ControlError> { - let (transaction, rows) = query( - transaction, - "SELECT trigger_id FROM sqb_triggers WHERE trigger_id=$1 AND location_id=$2 AND target=$3", - vec![ - Parameter::Id(trigger_id), - Parameter::Id(location_id), - Parameter::Text(target.into()), - ], - ) - .await?; - Ok((transaction, !rows.is_empty())) -} diff --git a/crates/sqlbuild-orchestration/src/scheduler/main/list_person_ticks.rs b/crates/sqlbuild-orchestration/src/scheduler/main/list_person_ticks.rs index 1a15efa392..7062347159 100644 --- a/crates/sqlbuild-orchestration/src/scheduler/main/list_person_ticks.rs +++ b/crates/sqlbuild-orchestration/src/scheduler/main/list_person_ticks.rs @@ -3,12 +3,13 @@ use uuid::Uuid; use crate::authorization::main::authorize_listing::authorize_listing; +use crate::authorization::main::row_in_target::row_in_target; use crate::credentials::models::{AuditContext, DefaultRole}; -use crate::scheduler::_helpers::reads::{ticks_in, trigger_in_scope}; +use crate::scheduler::_helpers::reads::ticks_in; use crate::scheduler::models::TickRecord; use crate::store::errors::ControlError; use crate::store::main::{begin::begin, commit::commit}; -use crate::store::models::{ControlStore, TransactionMode}; +use crate::store::models::{ControlStore, TargetRow, TransactionMode}; /// `None` when the trigger is not in this location and target; authorization runs first either way. pub async fn list_person_ticks( @@ -24,8 +25,12 @@ pub async fn list_person_ticks( default_role, ) .await?; - let (transaction, known) = - trigger_in_scope(transaction, (location_id, target), trigger_id).await?; + let (transaction, known) = row_in_target( + transaction, + (location_id, target), + TargetRow::Trigger(trigger_id), + ) + .await?; if !known { commit(transaction).await?; return Ok(None); diff --git a/crates/sqlbuild-orchestration/src/store/models.rs b/crates/sqlbuild-orchestration/src/store/models.rs index 9366364a4b..0e93f7ed72 100644 --- a/crates/sqlbuild-orchestration/src/store/models.rs +++ b/crates/sqlbuild-orchestration/src/store/models.rs @@ -37,6 +37,13 @@ pub(crate) struct ControlEvent<'a> { pub(crate) payload: serde_json::Value, } +/// A row whose location and target decide whether a person's target-scoped read may name it. +#[derive(Clone, Copy, Debug)] +pub(crate) enum TargetRow { + Run(Uuid), + Trigger(Uuid), +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum TransactionMode { Read, diff --git a/fensu.toml b/fensu.toml index 54ddd88f0a..75e2277e0d 100644 --- a/fensu.toml +++ b/fensu.toml @@ -1367,11 +1367,11 @@ path = "src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py" reason = "The SQL Server adapter intentionally keeps overrideable backend behavior on one class." [[dupes.allowlist]] -paths = ["crates/sqlbuild-orchestration/src/api/_helpers/resources/administrators.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/audit.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/location_health.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/location_work.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/person_checks.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/person_location_work.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/person_maintenance.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/person_node_sql.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/person_operations.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/run_reads.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/runs.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/snapshots.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/trigger_reads.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/warehouse_metadata.rs"] +paths = ["crates/sqlbuild-orchestration/src/api/_helpers/resources/administrators.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/audit.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/location_health.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/location_work.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/person_checks.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/person_location_work.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/person_maintenance.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/person_node_sql.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/person_operations.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/person_reads.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/run_reads.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/runs.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/snapshots.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/trigger_reads.rs", "crates/sqlbuild-orchestration/src/api/_helpers/resources/warehouse_metadata.rs"] reason = "Each function is one utoipa-annotated axum route: OpenAPI needs one annotated handler per operation, and each body only checks the caller's scope and calls one store operation. Scope checks and store work live in their shared owners." [[dupes.allowlist]] -paths = ["crates/sqlbuild-orchestration/src/authorization/main/read_security_audit.rs", "crates/sqlbuild-orchestration/src/credentials/_helpers/active_session.rs", "crates/sqlbuild-orchestration/src/declarations/main/list_triggers.rs", "crates/sqlbuild-orchestration/src/platform/main/read_location_work.rs", "crates/sqlbuild-orchestration/src/runs/main/list_person_runs.rs", "crates/sqlbuild-orchestration/src/runs/main/list_runs.rs", "crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs", "crates/sqlbuild-orchestration/src/warehouse_history/_helpers/checks.rs", "crates/sqlbuild-orchestration/src/scheduler/main/list_ticks.rs", "crates/sqlbuild-orchestration/src/sensors/_helpers/attempts.rs", "crates/sqlbuild-orchestration/src/sensors/_helpers/facts.rs", "crates/sqlbuild-orchestration/src/snapshots/main/read_current_record.rs", "crates/sqlbuild-orchestration/src/snapshots/main/read_snapshot_documents.rs", "crates/sqlbuild-orchestration/src/snapshots/main/read_snapshot_nodes.rs", "crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_catalog.rs", "crates/sqlbuild-orchestration/src/warehouse_history/main/read_person_check.rs", "crates/sqlbuild-orchestration/src/warehouse_history/main/read_person_checks.rs", "crates/sqlbuild-orchestration/src/warehouse_history/main/read_person_maintenance.rs", "crates/sqlbuild-orchestration/src/warehouse_history/main/read_person_microbatch_coverage.rs", "crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_statistics.rs"] +paths = ["crates/sqlbuild-orchestration/src/authorization/main/read_security_audit.rs", "crates/sqlbuild-orchestration/src/credentials/_helpers/active_session.rs", "crates/sqlbuild-orchestration/src/declarations/_helpers/reads.rs", "crates/sqlbuild-orchestration/src/declarations/main/list_person_triggers.rs", "crates/sqlbuild-orchestration/src/declarations/main/list_triggers.rs", "crates/sqlbuild-orchestration/src/platform/main/read_location_work.rs", "crates/sqlbuild-orchestration/src/runs/main/list_person_runs.rs", "crates/sqlbuild-orchestration/src/runs/main/list_person_run_nodes.rs", "crates/sqlbuild-orchestration/src/runs/main/list_run_nodes.rs", "crates/sqlbuild-orchestration/src/runs/main/list_runs.rs", "crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs", "crates/sqlbuild-orchestration/src/warehouse_history/_helpers/checks.rs", "crates/sqlbuild-orchestration/src/scheduler/main/list_person_ticks.rs", "crates/sqlbuild-orchestration/src/store/main/read_records.rs", "crates/sqlbuild-orchestration/src/platform/main/location_work.rs", "crates/sqlbuild-orchestration/src/scheduler/main/list_ticks.rs", "crates/sqlbuild-orchestration/src/sensors/_helpers/attempts.rs", "crates/sqlbuild-orchestration/src/sensors/_helpers/facts.rs", "crates/sqlbuild-orchestration/src/snapshots/main/read_current_record.rs", "crates/sqlbuild-orchestration/src/snapshots/main/read_snapshot_documents.rs", "crates/sqlbuild-orchestration/src/snapshots/main/read_snapshot_nodes.rs", "crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_catalog.rs", "crates/sqlbuild-orchestration/src/warehouse_history/main/read_person_check.rs", "crates/sqlbuild-orchestration/src/warehouse_history/main/read_person_checks.rs", "crates/sqlbuild-orchestration/src/warehouse_history/main/read_person_maintenance.rs", "crates/sqlbuild-orchestration/src/warehouse_history/main/read_person_microbatch_coverage.rs", "crates/sqlbuild-orchestration/src/warehouse_metadata/main/read_person_statistics.rs"] reason = "Each is one control-store read with its own SQL and record type. Transactions, person authorization and decoding go through the shared begin_authorized, query_records, read_records and query_bodies helpers; what remains alike is the call shape." [[dupes.allowlist]]