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/9] 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/9] 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/9] 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/9] 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]] From 9dd2f3cab0b9b812d49c274d3c46bc1dbf3b188d Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 6 Nov 2026 23:28:29 +0000 Subject: [PATCH 5/9] feat(orchestration): list run kind, origin, lineage and requester The person and machine run listings now carry the run metadata the store already keeps: kind, origin, parent and root run, trigger and tick, rerun source, priority, history rewrite, status reason and cancel request. They also name the requester: the stored actor, plus the principal's display name when a person requested the run. Validated locally: API, runs, sensors and scheduler tests on SQLite and PostgreSQL; regenerated TypeScript client. --- clients/typescript/src/schema.d.ts | 23 ++++++++++ .../src/runs/_helpers/lifecycle/reads.rs | 9 +++- .../sqlbuild-orchestration/src/runs/models.rs | 16 +++++++ .../tests/api/test_person_sessions.rs | 44 +++++++++++++++++++ .../tests/sensors/test_custom.rs | 29 ++++++++++-- .../tests/sensors/test_runs.rs | 36 ++++++++++++++- 6 files changed, 150 insertions(+), 7 deletions(-) diff --git a/clients/typescript/src/schema.d.ts b/clients/typescript/src/schema.d.ts index 57810a34ff..7c24125d11 100644 --- a/clients/typescript/src/schema.d.ts +++ b/clients/typescript/src/schema.d.ts @@ -1576,9 +1576,12 @@ export interface components { */ RunOutcome: "success" | "warn" | "partial" | "failed" | "cancelled"; RunRecord: { + /** Format: date-time */ + cancel_requested_at?: string | null; /** Format: date-time */ ended_at?: string | null; job_key?: string | null; + kind: components["schemas"]["RunKind"]; /** Format: int64 */ last_event_storage_id: number; /** Format: date-time */ @@ -1588,8 +1591,22 @@ export interface components { node_counts_json: { [key: string]: number; }; + origin: components["schemas"]["RunOrigin"]; + /** Format: uuid */ + parent_run_id?: string | null; + /** Format: int64 */ + priority: number; /** Format: date-time */ queued_at: string; + /** @description The stored actor: `principal:` for a person, otherwise a system actor such as `system:scheduler`. */ + requested_by: string; + /** @description The display name of the requesting principal; never set for system actors. */ + requester_name?: string | null; + /** Format: uuid */ + rerun_of_run_id?: string | null; + rewrite_history: boolean; + /** Format: uuid */ + root_run_id: string; /** Format: uuid */ run_id: string; selection_json: string[]; @@ -1598,7 +1615,13 @@ export interface components { /** Format: date-time */ started_at?: string | null; status: components["schemas"]["RunStatus"]; + status_reason?: string | null; target: string; + /** Format: uuid */ + tick_id?: string | null; + /** Format: uuid */ + trigger_id?: string | null; + trigger_name?: string | null; }; /** @description The authorized selection and pinned execution inputs are immutable once requested. */ RunRequest: { diff --git a/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs b/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs index e3f248bb67..bddbdfa2df 100644 --- a/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs +++ b/crates/sqlbuild-orchestration/src/runs/_helpers/lifecycle/reads.rs @@ -13,8 +13,13 @@ pub(crate) async fn list_runs_in( limit: u32, ) -> Result<(ControlTransaction, Vec), ControlError> { query_records(transaction, - "SELECT run_id, location_id, target, snapshot_id, job_key, status, queued_at, launched_at, started_at, ended_at, selection_json, node_counts_json, last_event_storage_id \ - FROM sqb_runs WHERE location_id=$1 AND target=$2 ORDER BY queued_at DESC, run_id DESC LIMIT $3", + "SELECT r.run_id, r.location_id, r.target, r.snapshot_id, r.job_key, r.kind, r.origin, r.parent_run_id, r.root_run_id, \ + r.trigger_id, t.name AS trigger_name, r.tick_id, r.rerun_of_run_id, r.requested_by, p.name AS requester_name, r.priority, \ + r.rewrite_history, r.status, r.status_reason, r.queued_at, r.launched_at, r.started_at, r.ended_at, r.cancel_requested_at, \ + r.selection_json, r.node_counts_json, r.last_event_storage_id \ + FROM sqb_runs r LEFT JOIN sqb_triggers t ON t.trigger_id=r.trigger_id \ + LEFT JOIN sqb_principals p ON 'principal:' || CAST(p.principal_id AS TEXT)=r.requested_by \ + WHERE r.location_id=$1 AND r.target=$2 ORDER BY r.queued_at DESC, r.run_id DESC LIMIT $3", vec![Parameter::Id(location_id), Parameter::Text(target.into()), Parameter::Integer(i64::from(limit.min(1000)))]).await } diff --git a/crates/sqlbuild-orchestration/src/runs/models.rs b/crates/sqlbuild-orchestration/src/runs/models.rs index 7b6f9c7c2d..545660e12d 100644 --- a/crates/sqlbuild-orchestration/src/runs/models.rs +++ b/crates/sqlbuild-orchestration/src/runs/models.rs @@ -153,11 +153,27 @@ pub struct RunRecord { pub target: String, pub snapshot_id: Uuid, pub job_key: Option, + pub kind: RunKind, + pub origin: RunOrigin, + pub parent_run_id: Option, + pub root_run_id: Uuid, + pub trigger_id: Option, + pub trigger_name: Option, + pub tick_id: Option, + pub rerun_of_run_id: Option, + /// The stored actor: `principal:` for a person, otherwise a system actor such as `system:scheduler`. + pub requested_by: String, + /// The display name of the requesting principal; never set for system actors. + pub requester_name: Option, + pub priority: i64, + pub rewrite_history: bool, pub status: RunStatus, + pub status_reason: Option, pub queued_at: DateTime, pub launched_at: Option>, pub started_at: Option>, pub ended_at: Option>, + pub cancel_requested_at: Option>, pub selection_json: BTreeSet, pub node_counts_json: BTreeMap, pub last_event_storage_id: i64, diff --git a/crates/sqlbuild-orchestration/tests/api/test_person_sessions.rs b/crates/sqlbuild-orchestration/tests/api/test_person_sessions.rs index f776628da0..777ff64a75 100644 --- a/crates/sqlbuild-orchestration/tests/api/test_person_sessions.rs +++ b/crates/sqlbuild-orchestration/tests/api/test_person_sessions.rs @@ -343,6 +343,50 @@ fn given_browser_session_roles_when_work_is_requested_then_scope_revocation_and_ .as_u16(), role_case.expected_job ); + if role_case.expected_job == 200 && role_case.expected_metadata == 200 { + let listed: Value = request( + &provider.client, + session, + (Method::GET, &metadata), + (&Value::Null, &session.csrf), + ) + .await + .json() + .await + .expect("runs after launch"); + let rows = listed.as_array().expect("run rows"); + let launched = rows + .iter() + .find(|row| row["run_id"] == job["run_id"]) + .expect("launched job is listed"); + for (key, expected) in [ + ("kind", json!("job")), + ("origin", json!("manual")), + ("job_key", json!("orders")), + ("requested_by", json!(format!("principal:{person}"))), + ("requester_name", json!("orders-person")), + ("root_run_id", job["run_id"].clone()), + ("parent_run_id", Value::Null), + ("trigger_name", Value::Null), + ("rerun_of_run_id", Value::Null), + ("priority", json!(0)), + ("rewrite_history", json!(false)), + ("cancel_requested_at", Value::Null), + ] { + assert_eq!(launched[key], expected, "launched run {key}"); + } + let seeded = rows + .iter() + .find(|row| row["run_id"] == json!(seed_run.run_id)) + .expect("seed run is listed"); + for (key, expected) in [ + ("kind", json!("adhoc")), + ("requested_by", json!("test-operator")), + ("requester_name", Value::Null), + ] { + assert_eq!(seeded[key], expected, "system-requested run {key}"); + } + } for (endpoint, request_body) in [(&path[..], &body), ("/api/control/runs", &job)] { diff --git a/crates/sqlbuild-orchestration/tests/sensors/test_custom.rs b/crates/sqlbuild-orchestration/tests/sensors/test_custom.rs index ae19f87689..e0fde5327c 100644 --- a/crates/sqlbuild-orchestration/tests/sensors/test_custom.rs +++ b/crates/sqlbuild-orchestration/tests/sensors/test_custom.rs @@ -18,6 +18,7 @@ use crate::platform::models::{ LocationWorkResult, }; use crate::runs::main::list_runs::list_runs; +use crate::runs::models::RunOrigin; use crate::scheduler::tests::contracts::helpers::acquire_lease; use crate::sensors::main::begin_sensor_evaluation::begin_sensor_evaluation; use crate::sensors::main::complete_custom_sensor::complete_custom_sensor; @@ -260,15 +261,35 @@ fn given_custom_sensor_work_when_results_retry_then_cursor_runs_and_outcome_comm .expect("retry commit"), 0 ); + let sensor_runs = list_runs(store, (location, "development"), 100) + .await + .expect("dedup runs"); assert_eq!( - list_runs(store, (location, "development"), 100) - .await - .expect("dedup runs") - .len(), + sensor_runs.len(), test_case.expected_runs, "{}", test_case.description ); + for sensor_run in &sensor_runs { + assert_eq!( + ( + sensor_run.origin, + sensor_run.trigger_id, + sensor_run.trigger_name.as_deref(), + sensor_run.requested_by.as_str(), + sensor_run.requester_name.as_deref(), + ), + ( + RunOrigin::Sensor, + Some(trigger.trigger_id), + Some(trigger.name.as_str()), + "system:sensor", + None, + ), + "{}", + test_case.description + ); + } let facts = evaluations(store).await; let SensorEvaluationOutcome::Completed { cursor, run_ids } = &facts.last().expect("completed").outcome diff --git a/crates/sqlbuild-orchestration/tests/sensors/test_runs.rs b/crates/sqlbuild-orchestration/tests/sensors/test_runs.rs index a7d19db4f8..1b88eb8d3b 100644 --- a/crates/sqlbuild-orchestration/tests/sensors/test_runs.rs +++ b/crates/sqlbuild-orchestration/tests/sensors/test_runs.rs @@ -12,7 +12,7 @@ use crate::declarations::main::list_triggers::list_triggers; use crate::declarations::models::TriggerChange; use crate::runs::main::list_runs::list_runs; use crate::runs::main::request_rerun::request_rerun; -use crate::runs::models::{NodeOutcome, RerunRequest, RunOutcome}; +use crate::runs::models::{NodeOutcome, RerunRequest, RunKind, RunOrigin, RunOutcome}; use crate::scheduler::tests::contracts::helpers::{acquire_lease, hold_lease}; use crate::sensors::main::{ begin_sensor_evaluation::begin_sensor_evaluation, @@ -383,6 +383,40 @@ fn given_terminal_jobs_and_groups_when_selectors_retry_then_outcomes_scope_and_r let sources = list_runs(store, (location, "development"), 500) .await .expect("all source executions"); + let rerun = sources + .iter() + .find(|record| record.run_id == retry.run_id) + .expect("rerun listed"); + assert_eq!( + ( + rerun.kind, + rerun.origin, + rerun.rerun_of_run_id, + rerun.priority, + rerun.root_run_id, + rerun.parent_run_id, + rerun.requested_by.as_str(), + rerun.requester_name.as_deref(), + ), + ( + RunKind::Group, + RunOrigin::Rerun, + Some(retry.source_run_id), + 10, + retry.run_id, + None, + "test-admin", + None, + ) + ); + let rerun_children: Vec<_> = sources + .iter() + .filter(|record| record.parent_run_id == Some(retry.run_id)) + .collect(); + assert!(!rerun_children.is_empty(), "rerun children listed"); + for child in rerun_children { + assert_eq!(child.root_run_id, retry.run_id, "child root"); + } // Same key in another target/location and a different job in this scope must not match. let mut other_input = input.clone(); other_input.location_id = other; From 52a2003a0148b0e21567443b07ed126121d601e6 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sat, 7 Nov 2026 00:54:12 +0000 Subject: [PATCH 6/9] feat(ui): wire the runs list and run detail to the control API Map listed runs, nodes, reruns and child runs to the runs pages, with cancel and rerun (all or from failure) through the CSRF-guarded control routes. Show the requester's display name for people and the trigger or system actor otherwise. Widgets without a server endpoint (queue, timeline, new run, bulk cancel, events, statements, output, rerun of selected nodes) are hidden instead of showing placeholders. Short run IDs show the first 8 digits of a UUID. Validated: svelte-check clean, 50 UI unit tests, production build, and a live sqb server run (launch, live status, cancel, rerun from failure). --- ui/src/lib/api/client.ts | 4 +- ui/src/lib/api/http-client.ts | 65 +++++- ui/src/lib/api/http-runs.test.ts | 122 +++++++++++ ui/src/lib/api/http-runs.ts | 193 ++++++++++++++++++ ui/src/lib/api/mock/mock-client.ts | 9 +- ui/src/lib/api/mock/operate-store.ts | 3 +- ui/src/lib/api/types/operate.ts | 13 +- ui/src/lib/operate/labels.ts | 5 +- ui/src/lib/status.ts | 3 +- ui/src/routes/runs/+page.svelte | 15 +- ui/src/routes/runs/[id]/failures-panel.svelte | 3 +- ui/src/routes/runs/[id]/related-runs.svelte | 4 + ui/src/routes/runs/[id]/run-header.svelte | 2 +- ui/src/routes/runs/[id]/run-tabs.svelte | 11 +- ui/src/routes/runs/[id]/run-view.svelte | 12 +- ui/src/routes/runs/[id]/state.svelte.ts | 10 +- ui/src/routes/runs/[id]/utils/index.ts | 1 + ui/src/routes/runs/[id]/utils/tabs.ts | 14 ++ ui/src/routes/runs/bulk-bar.svelte | 2 +- ui/src/routes/runs/runs-toolbar.svelte | 17 +- 20 files changed, 468 insertions(+), 40 deletions(-) create mode 100644 ui/src/lib/api/http-runs.test.ts create mode 100644 ui/src/lib/api/http-runs.ts create mode 100644 ui/src/routes/runs/[id]/utils/tabs.ts diff --git a/ui/src/lib/api/client.ts b/ui/src/lib/api/client.ts index 0944f8eb5f..fd900528bf 100644 --- a/ui/src/lib/api/client.ts +++ b/ui/src/lib/api/client.ts @@ -97,7 +97,9 @@ export interface ApiClient extends PlatformApi { /** Writes a `cancel_requested` fact; the scheduler then signals the worker. */ cancelRun(scope: Scope, runId: string): Promise; /** `nodes` is required for `selected` and ignored otherwise. */ - rerunRun(scope: Scope, runId: string, mode: RerunMode, nodes?: string[]): Promise; + rerunRun(scope: Scope, runId: string, mode: Exclude): Promise; + /** Reruns exactly `nodes` of the run. */ + rerunSelectedNodes(scope: Scope, runId: string, nodes: string[]): Promise; listSnapshotNodes(scope: Scope): Promise; /** The server's pre-launch check: resolution, write set and authorization. */ diff --git a/ui/src/lib/api/http-client.ts b/ui/src/lib/api/http-client.ts index 7c908c7c95..52d835ca88 100644 --- a/ui/src/lib/api/http-client.ts +++ b/ui/src/lib/api/http-client.ts @@ -1,13 +1,15 @@ import createFetch from 'openapi-fetch'; import type { components, paths } from '$control/schema.d.ts'; import { ApiError, type ApiClient } from './client'; +import { RUN_WINDOW, runDetailOf, runRowPage, runTabCountsOf, toRunNode, toRuns } from './http-runs'; import type { ChangeSubscription, ChangedChannel, ChangedSignal, ConnectionState } from './signals'; import type { AuthMode, Bootstrap, Capability, Location, Scope } from './types'; +import type { RerunMode, Run, RunDetail, RunRowPage, RunRowQuery, RunTabCounts } from './types/operate'; type Wire = components['schemas']; /** The client methods the server has endpoints for; the shell hides pages that need others. */ -const SUPPORTED: ReadonlySet = new Set(['supports', 'bootstrap', 'changes']); +const SUPPORTED: ReadonlySet = new Set(['supports', 'bootstrap', 'changes', 'listRunRows', 'runTabCounts', 'getRun', 'getRunDetail', 'cancelRun', 'rerunRun']); /** A UI location is one server location and target; its id carries both. */ export function locationKey(locationId: string, target: string): string { @@ -162,8 +164,69 @@ export function createHttpClient({ baseUrl = '', onUnauthorized }: HttpClientOpt if (response.status === 401) onUnauthorized(); if (!data) throw new ApiError(errorText(error, response), response.status); return toBootstrap(data); + }, + listRunRows: async (scope: Scope, query: RunRowQuery = {}): Promise => runRowPage(await listedRuns(scope), scope.locationId, query), + runTabCounts: async (scope: Scope, query: RunRowQuery = {}): Promise => runTabCountsOf(await listedRuns(scope), scope.locationId, query), + getRun: async (scope: Scope, runId: string): Promise => (await findRun(scope, runId))?.run ?? null, + async getRunDetail(scope: Scope, runId: string): Promise { + const found = await findRun(scope, runId); + if (!found) return null; + const { run, wire, listed } = found; + const owners = [run, ...listed.filter((r) => r.parentRunId === run.id)]; + const nodes = (await Promise.all(owners.map(async (owner) => (await runNodes(scope, owner.id)).map((n) => toRunNode(n, owner))))).flat(); + return runDetailOf(run, wire, listed, nodes); + }, + async cancelRun(scope: Scope, runId: string): Promise { + const body = { run_id: runId, event_id: crypto.randomUUID(), reason: 'Cancelled from the web UI' }; + const { error, response } = await http.POST('/api/control/runs/cancel', { body, params: { header: { 'x-sqb-csrf': csrfToken(document.cookie) } } }); + if (response.status === 401) onUnauthorized(); + if (!response.ok) throw new ApiError(errorText(error, response), response.status); + return requireRun(scope, runId); + }, + async rerunRun(scope: Scope, runId: string, mode: Exclude): Promise { + const source = await requireRun(scope, runId); + const body = { run_id: crypto.randomUUID(), source_run_id: runId, from_failure: mode === 'from_failure', priority: source.priority, reason: 'Rerun from the web UI' }; + const { error, response } = await http.POST('/api/control/runs/rerun', { body, params: { header: { 'x-sqb-csrf': csrfToken(document.cookie) } } }); + if (response.status === 401) onUnauthorized(); + if (!response.ok) throw new ApiError(errorText(error, response), response.status); + return requireRun(scope, body.run_id); } }; + + /** The newest runs of the location, up to the server's cap. */ + async function listedRuns(scope: Scope) { + const { locationId, target } = parseLocationKey(scope.locationId); + const { data, error, response } = await http.GET('/api/control/locations/{location_id}/targets/{target}/runs', { + params: { path: { location_id: locationId, target }, query: { limit: RUN_WINDOW } } + }); + if (response.status === 401) onUnauthorized(); + if (!data) throw new ApiError(errorText(error, response), response.status); + return data; + } + + async function findRun(scope: Scope, runId: string) { + const wires = await listedRuns(scope); + const listed = toRuns(wires, scope.locationId); + const index = listed.findIndex((r) => r.id === runId); + return index < 0 ? null : { run: listed[index], wire: wires[index], listed }; + } + + async function requireRun(scope: Scope, runId: string): Promise { + const found = await findRun(scope, runId); + if (!found) throw new ApiError('The run is not in the recent runs of this location', 404); + return found.run; + } + + async function runNodes(scope: Scope, runId: string) { + const { locationId, target } = parseLocationKey(scope.locationId); + const { data, error, response } = await http.GET('/api/control/locations/{location_id}/targets/{target}/runs/{run_id}/nodes', { + params: { path: { location_id: locationId, target, run_id: runId } } + }); + if (response.status === 401) onUnauthorized(); + if (!data) throw new ApiError(errorText(error, response), response.status); + return data; + } + return new Proxy(implemented, { get(target, property, receiver) { if (property in target) return Reflect.get(target, property, receiver); diff --git a/ui/src/lib/api/http-runs.test.ts b/ui/src/lib/api/http-runs.test.ts new file mode 100644 index 0000000000..c87355e084 --- /dev/null +++ b/ui/src/lib/api/http-runs.test.ts @@ -0,0 +1,122 @@ +import { describe, expect, it } from 'vitest'; +import type { components } from '$control/schema.d.ts'; +import { runDetailOf, runRowPage, runTabCountsOf, toRunNode, toRuns } from './http-runs'; + +type RunRecord = components['schemas']['RunRecord']; + +const LOCATION = '0191f0aa-orders:development'; +const PERSON_ID = '0191f0cc-0000-7000-8000-000000000001'; + +function wire(id: string, overrides: Partial = {}): RunRecord { + return { + run_id: id, + root_run_id: id, + kind: 'job', + origin: 'manual', + status: 'success', + job_key: 'orders_daily', + last_event_storage_id: 1, + location_id: '0191f0aa-0000-7000-8000-000000000000', + node_counts_json: {}, + priority: 0, + queued_at: '2026-11-06T10:00:00Z', + requested_by: `principal:${PERSON_ID}`, + requester_name: 'Ada Admin', + rewrite_history: false, + selection_json: ['model.orders'], + snapshot_id: '0191f0dd-0000-7000-8000-000000000000', + target: 'development', + ...overrides + }; +} + +describe('run mapping', () => { + it('shows a person by display name and a machine origin by its trigger', () => { + const [person, sensor, scheduler] = toRuns( + [ + wire('a'), + wire('b', { origin: 'sensor', requested_by: 'system:sensor', requester_name: null, trigger_name: 'new_inventory_file' }), + wire('c', { origin: 'schedule', requested_by: 'system:scheduler', requester_name: null }) + ], + LOCATION + ); + expect(person.requestedBy).toEqual({ kind: 'user', name: PERSON_ID, displayName: 'Ada Admin' }); + expect(sensor.requestedBy).toEqual({ kind: 'sensor', name: 'new_inventory_file', displayName: 'new_inventory_file' }); + expect(scheduler.requestedBy).toEqual({ kind: 'scheduler', name: 'system:scheduler', displayName: 'system:scheduler' }); + }); + + it('counts cancelled nodes as failed and counts each run’s direct children', () => { + const runs = toRuns( + [ + wire('group', { kind: 'group', node_counts_json: { success: 3, cancelled: 2, failed: 1, pending: 4 } }), + wire('child-1', { parent_run_id: 'group', root_run_id: 'group' }), + wire('child-2', { parent_run_id: 'group', root_run_id: 'group' }), + wire('grandchild', { parent_run_id: 'child-1', root_run_id: 'group' }) + ], + LOCATION + ); + expect(runs[0].nodeCounts).toEqual({ success: 3, warn: 0, failed: 3, skipped: 0, running: 0, pending: 4 }); + expect(runs.map((r) => r.childRunCount)).toEqual([2, 1, 0, 0]); + }); +}); + +describe('runs list', () => { + const listing = [ + wire('r1', { status: 'running', cancel_requested_at: '2026-11-06T10:05:00Z' }), + wire('r2', { status: 'cancelled', cancel_requested_at: '2026-11-06T09:05:00Z' }), + wire('r3', { status: 'queued' }), + wire('r4', { status: 'failed', node_counts_json: { success: 1, failed: 2, skipped: 3 } }), + wire('r5', { status: 'partial' }), + wire('r5-child', { status: 'queued', parent_run_id: 'r5', root_run_id: 'r5' }), + wire('r6', { status: 'success' }) + ]; + + it('pages one tab with an offset cursor and hides children unless asked', () => { + const first = runRowPage(listing, LOCATION, { tab: 'all', limit: 4 }); + expect(first.rows.map((r) => r.run.id)).toEqual(['r1', 'r2', 'r3', 'r4']); + expect(first.nextCursor).toBe('4'); + const second = runRowPage(listing, LOCATION, { tab: 'all', limit: 4, cursor: first.nextCursor }); + expect(second.rows.map((r) => r.run.id)).toEqual(['r5', 'r6']); + expect(second.nextCursor).toBeNull(); + expect(runRowPage(listing, LOCATION, { tab: 'failed' }).rows.map((r) => r.run.id)).toEqual(['r4', 'r5']); + expect(runRowPage(listing, LOCATION, { parentRunId: 'r5' }).rows.map((r) => r.run.id)).toEqual(['r5-child']); + }); + + it('keeps a cancel request only while the run is active', () => { + const rows = runRowPage(listing, LOCATION, { tab: 'all' }).rows; + expect(rows.find((r) => r.run.id === 'r1')?.cancelRequestedAt).toBe('2026-11-06T10:05:00Z'); + expect(rows.find((r) => r.run.id === 'r2')?.cancelRequestedAt).toBeNull(); + expect(rows.find((r) => r.run.id === 'r4')?.rerunFromFailureCount).toBe(5); + }); + + it('counts active and queued runs under the filter, ignoring the tab', () => { + expect(runTabCountsOf(listing, LOCATION, { tab: 'failed' })).toEqual({ active: 2, queued: 1 }); + expect(runTabCountsOf(listing, LOCATION, { includeChildren: true })).toEqual({ active: 3, queued: 2 }); + expect(runTabCountsOf(listing, LOCATION, { filter: { requestedBy: ['ada admin'] }, includeChildren: true })).toEqual({ active: 3, queued: 2 }); + expect(runTabCountsOf(listing, LOCATION, { filter: { requestedBy: ['someone else'] } })).toEqual({ active: 0, queued: 0 }); + }); +}); + +describe('run detail', () => { + it('links parent, children and reruns, and reruns from failed, skipped and cancelled nodes', () => { + const wires = [ + wire('parent', { kind: 'group' }), + wire('run', { parent_run_id: 'parent', root_run_id: 'parent', status: 'failed' }), + wire('child-late', { parent_run_id: 'run', root_run_id: 'parent', queued_at: '2026-11-06T10:02:00Z' }), + wire('child-early', { parent_run_id: 'run', root_run_id: 'parent', queued_at: '2026-11-06T10:01:00Z' }), + wire('retry', { rerun_of_run_id: 'run' }) + ]; + const runs = toRuns(wires, LOCATION); + const run = runs[1]; + const nodes = (['success', 'failed', 'skipped', 'cancelled', 'bogus'] as const).map((status, i) => + toRunNode({ node_key: `model.n${i}`, status, skipped_because_of: status === 'skipped' ? 'model.n1' : null } as components['schemas']['RunNodeRecord'], run) + ); + const detail = runDetailOf(run, wires[1], runs, nodes); + expect(detail.parent?.id).toBe('parent'); + expect(detail.children.map((r) => r.id)).toEqual(['child-early', 'child-late']); + expect(detail.reruns.map((r) => r.id)).toEqual(['retry']); + expect(detail.rerunFromFailureNodes).toEqual(['model.n1', 'model.n2', 'model.n3']); + expect(nodes[4].status).toBe('not_run'); + expect(nodes[2].skippedBecauseOf).toEqual({ nodeKey: 'model.n1', runId: 'run', jobKey: 'orders_daily' }); + }); +}); diff --git a/ui/src/lib/api/http-runs.ts b/ui/src/lib/api/http-runs.ts new file mode 100644 index 0000000000..6a82a65035 --- /dev/null +++ b/ui/src/lib/api/http-runs.ts @@ -0,0 +1,193 @@ +import type { components } from '$control/schema.d.ts'; +import { isActiveRun } from '$lib/status'; +import type { Principal } from './types'; +import type { NodeCounts, Run, RunDetail, RunNode, RunNodeStatus, RunRow, RunRowPage, RunRowQuery, RunTabCounts } from './types/operate'; + +type Wire = components['schemas']; + +/** The server lists at most this many runs per request; tabs, filters and pages apply to them. */ +export const RUN_WINDOW = 1000; + +const PERSON = 'principal:'; + +/** Who requested a run: a person by display name and principal ID, otherwise the trigger or system actor. */ +export function requesterOf(wire: Wire['RunRecord']): Principal { + if (wire.requested_by.startsWith(PERSON)) { + const id = wire.requested_by.slice(PERSON.length); + return { kind: 'user', name: id, displayName: wire.requester_name ?? id }; + } + const actor = wire.requested_by; + const kind = actor === 'system:scheduler' ? 'scheduler' : actor === 'system:sensor' ? 'sensor' : 'system'; + const name = wire.trigger_name ?? actor; + return { kind, name, displayName: name }; +} + +function countsOf(wire: Wire['RunRecord']): NodeCounts { + const c = wire.node_counts_json; + const n = (key: string) => c[key] ?? 0; + return { + success: n('success'), + warn: n('warn'), + failed: n('failed') + n('cancelled'), + skipped: n('skipped'), + running: n('running'), + pending: n('pending') + }; +} + +export function toRun(wire: Wire['RunRecord'], locationId: string, childRunCount: number): Run { + return { + id: wire.run_id, + kind: wire.kind, + origin: wire.origin, + status: wire.status, + statusReason: wire.status_reason ?? null, + jobKey: wire.job_key ?? null, + parentRunId: wire.parent_run_id ?? null, + rootRunId: wire.root_run_id, + childRunCount, + triggerId: wire.trigger_id ?? null, + triggerName: wire.trigger_name ?? null, + tickId: wire.tick_id ?? null, + tickScheduledFor: null, + runKey: null, + rerunOfRunId: wire.rerun_of_run_id ?? null, + requestedBy: requesterOf(wire), + priority: wire.priority, + rewriteHistory: wire.rewrite_history, + locationId, + target: wire.target, + snapshotId: wire.snapshot_id, + gitRevision: null, + queuedAt: wire.queued_at, + launchedAt: wire.launched_at ?? null, + startedAt: wire.started_at ?? null, + endedAt: wire.ended_at ?? null, + nodeCounts: countsOf(wire), + cost: null + }; +} + +/** Runs keyed by id, each with its number of child runs in the listing. */ +export function toRuns(wires: Wire['RunRecord'][], locationId: string): Run[] { + const children = new Map(); + for (const w of wires) if (w.parent_run_id) children.set(w.parent_run_id, (children.get(w.parent_run_id) ?? 0) + 1); + return wires.map((w) => toRun(w, locationId, children.get(w.run_id) ?? 0)); +} + +function rowOf(run: Run, wire: Wire['RunRecord']): RunRow { + const c = run.nodeCounts; + const total = c.success + c.warn + c.failed + c.skipped + c.running + c.pending; + return { + run, + progress: { done: total - c.running - c.pending, total }, + waits: [], + lastContactAt: null, + lostContactLimitSeconds: 0, + cancelRequestedAt: isActiveRun(run.status) ? (wire.cancel_requested_at ?? null) : null, + selection: run.kind === 'adhoc' ? wire.selection_json : [], + rerunFromFailureCount: c.failed + c.skipped + }; +} + +function inTab(run: Run, tab: RunRowQuery['tab']): boolean { + if (tab === 'active') return isActiveRun(run.status); + if (tab === 'queued') return run.status === 'queued'; + if (tab === 'failed') return run.status === 'failed' || run.status === 'partial' || run.status === 'presumed_failed'; + if (tab === 'cancelled') return run.status === 'cancelled'; + return true; +} + +/** Whether a run passes a runs-list query, apart from its tab. Node-key filters need nodes and match nothing here. */ +function matches(run: Run, wire: Wire['RunRecord'], query: RunRowQuery): boolean { + const f = query.filter ?? {}; + const lower = (xs?: string[]) => (xs?.length ? xs.map((x) => x.toLowerCase()) : null); + if (query.parentRunId) { + if (run.parentRunId !== query.parentRunId) return false; + } else if (!query.includeChildren && run.parentRunId) return false; + if (query.since && run.queuedAt < query.since) return false; + if (query.until && run.queuedAt >= query.until) return false; + if (f.kinds?.length && !f.kinds.includes(run.kind)) return false; + if (f.origins?.length && !f.origins.includes(run.origin)) return false; + if (f.statuses?.length && !f.statuses.includes(run.status)) return false; + const ids = lower(f.ids)?.map((id) => id.replace(/^run-/, '')); + if (ids && !ids.some((id) => run.id.toLowerCase().startsWith(id))) return false; + const jobs = lower(f.jobKeys); + if (jobs && !jobs.includes((run.jobKey ?? '').toLowerCase())) return false; + const trigger = (run.triggerName ?? '').toLowerCase(); + const schedules = lower(f.schedules); + if (schedules && !(run.origin === 'schedule' && schedules.includes(trigger))) return false; + const sensors = lower(f.sensors); + if (sensors && !(run.origin === 'sensor' && sensors.includes(trigger))) return false; + const by = lower(f.requestedBy); + if (by && !by.includes(run.requestedBy.name.toLowerCase()) && !by.includes(run.requestedBy.displayName.toLowerCase())) return false; + if (f.nodeKeys?.length && !f.nodeKeys.some((k) => wire.selection_json.includes(k))) return false; + const text = f.text?.trim().toLowerCase() ?? ''; + if (text && !`${run.id} ${run.jobKey ?? ''} ${run.triggerName ?? ''} ${run.requestedBy.name} ${run.requestedBy.displayName}`.toLowerCase().includes(text)) return false; + return true; +} + +/** One page of the listing under a runs-list query; the cursor is an offset into the newest runs. */ +export function runRowPage(wires: Wire['RunRecord'][], locationId: string, query: RunRowQuery): RunRowPage { + const runs = toRuns(wires, locationId); + const matching = runs.flatMap((run, i) => (matches(run, wires[i], query) && inTab(run, query.tab) ? [rowOf(run, wires[i])] : [])); + const offset = Number(query.cursor ?? 0) || 0; + const limit = query.limit ?? 50; + return { + rows: matching.slice(offset, offset + limit), + nextCursor: offset + limit < matching.length ? String(offset + limit) : null + }; +} + +export function runTabCountsOf(wires: Wire['RunRecord'][], locationId: string, query: RunRowQuery): RunTabCounts { + const runs = toRuns(wires, locationId).filter((run, i) => matches(run, wires[i], query)); + return { + active: runs.filter((r) => isActiveRun(r.status)).length, + queued: runs.filter((r) => r.status === 'queued').length + }; +} + +function nodeStatus(status: string): RunNodeStatus { + return (['pending', 'running', 'success', 'warn', 'failed', 'skipped', 'cancelled'] as const).find((s) => s === status) ?? 'not_run'; +} + +export function toRunNode(wire: Wire['RunNodeRecord'], run: Run): RunNode { + return { + nodeKey: wire.node_key, + kind: null, + runId: run.id, + jobKey: run.jobKey, + status: nodeStatus(wire.status), + attempt: 1, + startedAt: wire.started_at ?? null, + endedAt: wire.ended_at ?? null, + rowsAffected: wire.rows_affected ?? null, + errorCode: wire.error_code ?? null, + errorMessage: null, + errorLocation: null, + errorDetail: null, + skippedBecauseOf: wire.skipped_because_of ? { nodeKey: wire.skipped_because_of, runId: run.id, jobKey: run.jobKey } : null, + critical: false + }; +} + +/** A run's detail from the listing and the nodes of the run and its child runs; parts with no server source stay empty. */ +export function runDetailOf(run: Run, wire: Wire['RunRecord'], listed: Run[], nodes: RunNode[]): RunDetail { + const byId = new Map(listed.map((r) => [r.id, r])); + return { + run, + parent: run.parentRunId ? (byId.get(run.parentRunId) ?? null) : null, + children: listed.filter((r) => r.parentRunId === run.id).sort((a, b) => a.queuedAt.localeCompare(b.queuedAt)), + nodes, + edges: [], + invocations: [], + liveness: null, + cancel: null, + provenance: null, + selection: wire.selection_json, + waits: [], + rerunFromFailureNodes: nodes.filter((n) => n.status === 'failed' || n.status === 'skipped' || n.status === 'cancelled').map((n) => n.nodeKey), + rerunOf: run.rerunOfRunId ? (byId.get(run.rerunOfRunId) ?? null) : null, + reruns: listed.filter((r) => r.rerunOfRunId === run.id) + }; +} diff --git a/ui/src/lib/api/mock/mock-client.ts b/ui/src/lib/api/mock/mock-client.ts index dde29a1485..0e632aba05 100644 --- a/ui/src/lib/api/mock/mock-client.ts +++ b/ui/src/lib/api/mock/mock-client.ts @@ -229,9 +229,14 @@ export class MockApiClient implements ApiClient { return this.#operate.cancelRun(scope, runId); } - async rerunRun(scope: Scope, runId: string, mode: RerunMode, nodes?: string[]): Promise { + async rerunRun(scope: Scope, runId: string, mode: Exclude): Promise { this.#location(scope); - return this.#operate.rerunRun(scope, runId, mode, nodes); + return this.#operate.rerunRun(scope, runId, mode); + } + + async rerunSelectedNodes(scope: Scope, runId: string, nodes: string[]): Promise { + this.#location(scope); + return this.#operate.rerunRun(scope, runId, 'selected', nodes); } getStatementSql(...args: Parameters) { diff --git a/ui/src/lib/api/mock/operate-store.ts b/ui/src/lib/api/mock/operate-store.ts index 89037a9737..9765fa7a3a 100644 --- a/ui/src/lib/api/mock/operate-store.ts +++ b/ui/src/lib/api/mock/operate-store.ts @@ -270,7 +270,7 @@ export class MockOperate { provenance: { snapshotId: rec.run.snapshotId, snapshotGeneration: rec.script.snapshotGeneration, - gitRevision: rec.run.gitRevision, + gitRevision: rec.run.gitRevision ?? '', gitDirty: false, imageId: rec.script.imageId, sqbVersion: rec.script.snapshotGeneration === SNAPSHOT_GENERATION ? '0.185.0' : '0.184.2', @@ -278,6 +278,7 @@ export class MockOperate { exclude: rec.script.exclude, command: rec.script.nodes.length ? commandOf(rec) : `group ${rec.run.jobKey}: ${this.#children(rec).length} child runs` }, + selection: rec.script.select, waits: rec.run.status === 'queued' ? this.#queue().get(rec.run.id) ?? [] : [], rerunFromFailureNodes: nodes.filter((n) => n.status === 'failed' || n.status === 'skipped').map((n) => n.nodeKey), rerunOf: rec.run.rerunOfRunId ? ((r) => (r ? this.#view(r, scope) : null))(this.#byId(rec.run.rerunOfRunId)) : null, diff --git a/ui/src/lib/api/types/operate.ts b/ui/src/lib/api/types/operate.ts index 32f616ccd6..40bbd8a81a 100644 --- a/ui/src/lib/api/types/operate.ts +++ b/ui/src/lib/api/types/operate.ts @@ -27,7 +27,7 @@ export const RUN_KINDS = ['job', 'group', 'adhoc'] as const; export type RunKind = (typeof RUN_KINDS)[number]; /** Why the run was requested. Group children inherit their parent's origin. */ -export const RUN_ORIGINS = ['manual', 'schedule', 'sensor', 'rerun'] as const; +export const RUN_ORIGINS = ['manual', 'schedule', 'sensor', 'rerun', 'backfill'] as const; export type RunOrigin = (typeof RUN_ORIGINS)[number]; export type NodeCounts = { @@ -74,7 +74,8 @@ export type Run = { locationId: string; target: string; snapshotId: string; - gitRevision: string; + /** Null where the server does not report the commit. */ + gitRevision: string | null; queuedAt: Iso; launchedAt: Iso | null; startedAt: Iso | null; @@ -197,7 +198,8 @@ export type SkippedBecauseOf = { export type RunNode = { nodeKey: string; - kind: NodeKind; + /** Null where the server does not report the node kind. */ + kind: NodeKind | null; /** The (child) run that executed this node. */ runId: string; jobKey: string | null; @@ -359,7 +361,10 @@ export type RunDetail = { invocations: RunInvocation[]; liveness: RunLiveness | null; cancel: CancelProgress | null; - provenance: RunProvenance; + /** Null where the server does not report how the run was built. */ + provenance: RunProvenance | null; + /** The frozen selection the run was authorized for. */ + selection: string[]; /** Why the run is still queued; empty once claimed. */ waits: QueueWait[]; /** Nodes a rerun from failure would run: the failed ones plus exactly those skipped. */ diff --git a/ui/src/lib/operate/labels.ts b/ui/src/lib/operate/labels.ts index 414f2b9f7c..8715ef3349 100644 --- a/ui/src/lib/operate/labels.ts +++ b/ui/src/lib/operate/labels.ts @@ -100,9 +100,10 @@ export const onFailurePlain: Record = { export type WaitPart = { text: string; href?: string; code?: boolean }; -/** `run-4f2a` → `4f2a`: the short form shown in lists; the full ID stays in links and copies. */ +/** `run-4f2a` → `4f2a`, a UUID → its first 8 digits: the short form shown in lists; the full ID stays in links and copies. */ export function shortRunId(id: string): string { - return id.replace(/^run-/, ''); + const short = id.replace(/^run-/, ''); + return /^[0-9a-f]{8}-[0-9a-f]{4}-/i.test(short) ? short.slice(0, 8) : short; } /** A concurrency group's slots in words, e.g. `warehouse_heavy` → `warehouse heavy`, `target:prod` → `prod target`. */ diff --git a/ui/src/lib/status.ts b/ui/src/lib/status.ts index 76e24d8449..dc7db40e72 100644 --- a/ui/src/lib/status.ts +++ b/ui/src/lib/status.ts @@ -141,7 +141,8 @@ export const runOriginLabel: Record = { manual: 'Manual', schedule: 'Schedule', sensor: 'Sensor', - rerun: 'Rerun' + rerun: 'Rerun', + backfill: 'Backfill' }; /** One-line "what · why" label, e.g. "Group · Schedule". */ diff --git a/ui/src/routes/runs/+page.svelte b/ui/src/routes/runs/+page.svelte index ca212214a2..16060abb69 100644 --- a/ui/src/routes/runs/+page.svelte +++ b/ui/src/routes/runs/+page.svelte @@ -1,5 +1,6 @@ - (panelOpen = true)}>▷ New run + {#if app.client.supports('launchAdhocRun')} + (panelOpen = true)}>▷ New run + {/if} -{#if p.view === 'timeline'} +{#if shown === 'timeline'}
-{:else if p.view === 'queue'} +{:else if shown === 'queue'} {:else} {/if} - +{#if app.client.supports('launchAdhocRun')}{/if}