diff --git a/backend/.sqlx/query-35f960e9de880fdcaf81615933c828c4a877709c3c39a530d629cc91f9dd2d29.json b/backend/.sqlx/query-35f960e9de880fdcaf81615933c828c4a877709c3c39a530d629cc91f9dd2d29.json new file mode 100644 index 0000000000..db2af83b47 --- /dev/null +++ b/backend/.sqlx/query-35f960e9de880fdcaf81615933c828c4a877709c3c39a530d629cc91f9dd2d29.json @@ -0,0 +1,29 @@ +{ + "db_name": "PostgreSQL", + "query": "WITH RECURSIVE chain(origin, id, parent_job) AS (\n SELECT id, id, parent_job FROM v2_job WHERE id = ANY($1) AND workspace_id = $2\n UNION ALL\n SELECT c.origin, j.id, j.parent_job FROM v2_job j\n JOIN chain c ON j.id = c.parent_job AND j.workspace_id = $2\n )\n SELECT c.origin AS \"origin!\",\n CASE WHEN a.agent THEN regexp_replace(j.runnable_path, '\\.chat$', '')\n ELSE j.runnable_path END AS \"runnable_path!\"\n FROM chain c JOIN v2_job j ON j.id = c.id,\n LATERAL (SELECT j.kind = 'flowpreview'\n AND j.raw_flow->'modules'->1 IS NULL\n AND j.raw_flow->'modules'->0->>'id' = '__wm_agent_root' AS agent) a\n WHERE j.workspace_id = $2 AND j.runnable_path IS NOT NULL\n AND (a.agent OR j.kind IN ('script', 'script_hub', 'unassigned_script', 'flow',\n 'unassigned_flow', 'singlestepflow', 'unassigned_singlestepflow'))", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "origin!", + "type_info": "Uuid" + }, + { + "ordinal": 1, + "name": "runnable_path!", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "UuidArray", + "Text" + ] + }, + "nullable": [ + null, + null + ] + }, + "hash": "35f960e9de880fdcaf81615933c828c4a877709c3c39a530d629cc91f9dd2d29" +} diff --git a/backend/windmill-api-auth/src/lib.rs b/backend/windmill-api-auth/src/lib.rs index 12ad5f2654..5b6a618e5f 100644 --- a/backend/windmill-api-auth/src/lib.rs +++ b/backend/windmill-api-auth/src/lib.rs @@ -625,6 +625,7 @@ fn scope_contains(caller: &ScopeDefinition, requested: &ScopeDefinition) -> bool // Apps only: `write` covers `run` (see `ScopeDefinition::includes`), so an // app-editor token can mint the narrower run-only credential. ("write", "run") if caller.domain == "apps" => {} + ("write", "cancel") if caller.domain == "jobs" => {} _ => return false, } diff --git a/backend/windmill-api-auth/src/scopes.rs b/backend/windmill-api-auth/src/scopes.rs index e100cf88c6..f2a6612e42 100644 --- a/backend/windmill-api-auth/src/scopes.rs +++ b/backend/windmill-api-auth/src/scopes.rs @@ -124,6 +124,7 @@ impl ScopeDefinition { // running its components. Not general — `jobs:write` must not grant // `jobs:run`. The resource check below still confines it to the same app. ("write", "run") if self.domain == "apps" => {} + ("write", "cancel") if self.domain == "jobs" => {} _ => return false, } @@ -430,9 +431,10 @@ impl ScopeDomain { /// Available scope actions #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum ScopeAction { - Read, // GET operations, list, view - Write, // POST, PUT, PATCH, DELETE operations, create, update, delete - Run, // Special action for running (scripts, flows, etc.) + Read, // GET operations, list, view + Write, // POST, PUT, PATCH, DELETE operations, create, update, delete + Run, // Special action for running (scripts, flows, etc.) + Cancel, // Cancelling jobs (`CANCEL_PATH_ACTIONS`); covered by `jobs:write` } impl ScopeAction { @@ -441,6 +443,7 @@ impl ScopeAction { Self::Read => "read", Self::Write => "write", Self::Run => "run", + Self::Cancel => "cancel", } } @@ -450,6 +453,7 @@ impl ScopeAction { "write" => Some(Self::Write), "delete" => Some(Self::Write), "run" => Some(Self::Run), + "cancel" => Some(Self::Cancel), _ => None, } } @@ -460,6 +464,7 @@ impl ScopeAction { match (self, other) { (ScopeAction::Write, ScopeAction::Read) => true, (ScopeAction::Run, ScopeAction::Read) => true, + (ScopeAction::Write, ScopeAction::Cancel) => true, (a, b) => a == b, } } @@ -636,7 +641,38 @@ lazy_static::lazy_static! { }; } +/// The job routes a `jobs:cancel` scope reaches, as workspaced route suffixes. A +/// path-scoped `jobs:cancel:` is resource-blind here like every scope; the +/// handlers confine it through `job_cancel_path_confinement`, so a cancel route added +/// here without that check would serve a path-scoped token every job. +const CANCEL_PATH_ACTIONS: [&'static str; 4] = [ + "jobs_u/queue/cancel/", + "jobs_u/queue/force_cancel/", + "jobs_u/queue/cancel_persistent/", + "jobs/queue/cancel_selection", +]; + +fn is_cancel_route(method: &str, route_path: &str) -> bool { + if !method.eq_ignore_ascii_case("POST") { + return false; + } + let mut parts = route_path.splitn(5, '/'); + let (Some(""), Some("api"), Some("w"), Some(_), Some(suffix)) = ( + parts.next(), + parts.next(), + parts.next(), + parts.next(), + parts.next(), + ) else { + return false; + }; + CANCEL_PATH_ACTIONS.iter().any(|p| suffix.starts_with(p)) +} + fn map_http_method_to_action(method: &str, route_path: &str) -> ScopeAction { + if is_cancel_route(method, route_path) { + return ScopeAction::Cancel; + } if RUN_PATH_ACTIONS .iter() .any(|run_path| route_path.contains(run_path)) @@ -974,6 +1010,9 @@ pub fn job_read_run_confinement(scopes: Option<&[String]>) -> Option { confinement.push(scope) } + // Grants no reads (see `scope_grants_access`), so it neither confines nor + // frees them. + Some(ScopeAction::Cancel) => continue, Some(_) => return None, None => continue, } @@ -997,6 +1036,58 @@ pub fn run_confinement_admits( confinement.iter().any(|scope| scope.includes(&required)) } +/// The `jobs:cancel:` scopes a token's cancels are confined to, or `None` when +/// they are not confined: an unscoped token, or one whose cancel grant is not +/// path-scoped (`jobs:cancel`, or `jobs:write`, which stays resource-blind here as on +/// every other job write). +/// +/// A job is within the confinement when its own `runnable_path` or that of any of its +/// `parent_job` ancestors matches (see `cancel_confinement_admits`), so a token scoped +/// to a flow can cancel the flow's steps. +pub fn job_cancel_path_confinement(scopes: Option<&[String]>) -> Option> { + let mut confinement = Vec::new(); + for scope in scopes? + .iter() + .filter(|s| !s.starts_with("if_jobs:filter_tags:")) + { + let Ok(scope) = ScopeDefinition::from_scope_string(scope) else { + continue; + }; + if ScopeDomain::from_str(&scope.domain) != Some(ScopeDomain::Jobs) { + continue; + } + match ScopeAction::from_str(&scope.action) { + Some(ScopeAction::Cancel) if scope.resource.is_some() => confinement.push(scope), + Some(ScopeAction::Cancel | ScopeAction::Write) => return None, + _ => continue, + } + } + (!confinement.is_empty()).then_some(confinement) +} + +/// Whether the token holds a `jobs:cancel` scope, path-scoped or not. +pub fn has_job_cancel_grant(scopes: Option<&[String]>) -> bool { + scopes.is_some_and(|scopes| { + scopes.iter().any(|s| { + ScopeDefinition::from_scope_string(s).is_ok_and(|s| { + ScopeDomain::from_str(&s.domain) == Some(ScopeDomain::Jobs) + && ScopeAction::from_str(&s.action) == Some(ScopeAction::Cancel) + }) + }) + }) +} + +/// Whether a job of `runnable_path` is inside a [`job_cancel_path_confinement`] set. +pub fn cancel_confinement_admits(confinement: &[ScopeDefinition], runnable_path: &str) -> bool { + let required = ScopeDefinition::new( + ScopeDomain::Jobs.as_str(), + ScopeAction::Cancel.as_str(), + None, + Some(vec![runnable_path.to_string()]), + ); + confinement.iter().any(|scope| scope.includes(&required)) +} + fn scope_grants_access( scope: &ScopeDefinition, required_domain: ScopeDomain, @@ -1781,4 +1872,68 @@ mod tests { // Unknown route -> None so the caller fails closed. assert!(scope_for_route("GET", "/healthz").is_none()); } + + #[test] + fn jobs_cancel_grants_only_the_cancel_routes() { + let cancel = vec!["jobs:cancel:f/served/*".to_string()]; + let id = "0190f4c2-0000-7000-8000-000000000000"; + for route in [ + format!("/api/w/ws/jobs_u/queue/cancel/{id}"), + format!("/api/w/ws/jobs_u/queue/force_cancel/{id}"), + "/api/w/ws/jobs_u/queue/cancel_persistent/f/served/s".to_string(), + "/api/w/ws/jobs/queue/cancel_selection".to_string(), + ] { + assert!( + check_route_access(&cancel, &route, "POST").is_ok(), + "{route}" + ); + assert!( + check_route_access(&["jobs:write".to_string()], &route, "POST").is_ok(), + "{route}" + ); + assert!( + check_route_access(&["jobs:read".to_string()], &route, "POST").is_err(), + "{route}" + ); + } + for (route, method) in [ + (format!("/api/w/ws/jobs_u/get/{id}"), "GET"), + ("/api/w/ws/jobs/list".to_string(), "GET"), + (format!("/api/w/ws/jobs/flow/resume_suspended/{id}"), "POST"), + (format!("/api/w/ws/jobs/queue/run_now/{id}"), "POST"), + ("/api/w/ws/jobs/run/p/f/served/s".to_string(), "POST"), + ] { + assert!( + check_route_access(&cancel, &route, method).is_err(), + "{route}" + ); + } + } + + #[test] + fn jobs_cancel_path_confinement() { + let scopes = |s: &[&str]| s.iter().map(|s| s.to_string()).collect::>(); + let conf = + job_cancel_path_confinement(Some(&scopes(&["jobs:cancel:f/served/*,u/svc/kill_me"]))) + .unwrap(); + assert!(cancel_confinement_admits(&conf, "f/served/etl")); + assert!(cancel_confinement_admits(&conf, "u/svc/kill_me")); + assert!(!cancel_confinement_admits(&conf, "f/other/etl")); + assert!(!cancel_confinement_admits(&conf, "f/served_not/etl")); + + // A cancel grant that is not path-scoped leaves cancels unconfined. + for s in [ + &["jobs:cancel"][..], + &["jobs:cancel:f/a/*", "jobs:write"], + &[], + ] { + assert!( + job_cancel_path_confinement(Some(&scopes(s))).is_none(), + "{s:?}" + ); + } + // A cancel scope neither confines nor frees a run token's reads. + let run = scopes(&["jobs:run:flows:f/a/b", "jobs:cancel"]); + assert!(job_read_run_confinement(Some(&run)).is_some()); + } } diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 785c6ff935..00139215c4 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -19,7 +19,7 @@ use serde_json::value::RawValue; use serde_json::Value; use sha2::{Digest, Sha256}; use std::borrow::Cow; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::str::FromStr; use std::sync::Arc; use std::time::Instant; @@ -620,7 +620,7 @@ async fn cancel_job_api( // right to kill someone else's run. Anonymous callers are instead confined to // anonymous-created jobs by `cancel_job`'s `require_anonymous`. if let Some(authed) = opt_authed.as_ref() { - require_job_update_read_access(&db, &user_db, authed, &w_id, &id, None).await?; + require_job_cancel_access(&db, &user_db, authed, &w_id, &id).await?; } let tx = db.begin().await?; @@ -689,7 +689,15 @@ async fn cancel_persistent_script_api( Json(CancelJob { reason }): Json, ) -> error::Result<()> { let audit_author: AuditAuthor = match opt_authed { - Some(authed) => (&authed).into(), + Some(authed) => { + let path = script_path.to_path(); + if !windmill_api_auth::scopes::job_cancel_path_confinement(authed.scopes.as_deref()) + .is_none_or(|c| windmill_api_auth::scopes::cancel_confinement_admits(&c, path)) + { + return Err(Error::NotFound(format!("Script {path} not found"))); + } + (&authed).into() + } None => { return Err(Error::BadRequest(format!( "Cancelling persistent script require to be logged in and member of {w_id}" @@ -782,7 +790,7 @@ async fn force_cancel( // caller who can only see an inner step kill a root flow hidden from them. if let Some(authed) = opt_authed.as_ref() { let target = force_cancel_target(&db, &w_id, id).await?; - require_job_update_read_access(&db, &user_db, authed, &w_id, &target, None).await?; + require_job_cancel_access(&db, &user_db, authed, &w_id, &target).await?; } let tx = db.begin().await?; @@ -1705,6 +1713,22 @@ pub(crate) async fn require_job_read_access( job_id: &Uuid, created_by: &str, view_token: Option<&str>, +) -> error::Result<()> { + require_job_access( + db, user_db, authed, w_id, job_id, created_by, view_token, true, + ) + .await +} + +async fn require_job_access( + db: &DB, + user_db: &UserDB, + authed: &ApiAuthed, + w_id: &str, + job_id: &Uuid, + created_by: &str, + view_token: Option<&str>, + run_confined: bool, ) -> error::Result<()> { // Tag scope (`if_jobs:filter_tags:`) is an orthogonal hard restriction on a // scoped token: it must never read a job outside its allowed tags, regardless of @@ -1732,7 +1756,9 @@ pub(crate) async fn require_job_read_access( // A path-scoped `jobs:run` token is likewise hard-restricted to the runnables it // may start, ahead of every grant below — the token is handed out to run one thing, // so it must not read jobs of anything else merely because its owner could. - require_job_within_run_scope(db, authed, w_id, job_id).await?; + if run_confined { + require_job_within_run_scope(db, authed, w_id, job_id).await?; + } // Fast path: you can always read a job you launched. This is also load-bearing // for apps — a component job runs as the app policy's `permissioned_as`, but its @@ -1961,6 +1987,73 @@ async fn require_job_within_run_scope( } } +/// The `ids` a path-scoped `jobs:cancel:` token may cancel; all of them for +/// every caller whose cancels are not path-confined (see `job_cancel_path_confinement`). +/// +/// A job is admitted when it, or any of its `parent_job` ancestors, is a run of a +/// deployed script, flow or agent whose path the scope names: cancelling a flow's step +/// is within a scope on the flow. Only those kinds count, because a preview's +/// `runnable_path` is whatever its caller sent and would otherwise let any preview +/// impersonate an in-scope runnable. Agent runs are previews filed under the agent's +/// path, recognized the same way `require_job_within_run_scope` does. +async fn filter_jobs_within_cancel_scope( + db: &DB, + authed: &ApiAuthed, + w_id: &str, + ids: Vec, +) -> error::Result> { + let Some(confinement) = + windmill_api_auth::scopes::job_cancel_path_confinement(authed.scopes.as_deref()) + else { + return Ok(ids); + }; + let chain = sqlx::query!( + r#"WITH RECURSIVE chain(origin, id, parent_job) AS ( + SELECT id, id, parent_job FROM v2_job WHERE id = ANY($1) AND workspace_id = $2 + UNION ALL + SELECT c.origin, j.id, j.parent_job FROM v2_job j + JOIN chain c ON j.id = c.parent_job AND j.workspace_id = $2 + ) + SELECT c.origin AS "origin!", + CASE WHEN a.agent THEN regexp_replace(j.runnable_path, '\.chat$', '') + ELSE j.runnable_path END AS "runnable_path!" + FROM chain c JOIN v2_job j ON j.id = c.id, + LATERAL (SELECT j.kind = 'flowpreview' + AND j.raw_flow->'modules'->1 IS NULL + AND j.raw_flow->'modules'->0->>'id' = '__wm_agent_root' AS agent) a + WHERE j.workspace_id = $2 AND j.runnable_path IS NOT NULL + AND (a.agent OR j.kind IN ('script', 'script_hub', 'unassigned_script', 'flow', + 'unassigned_flow', 'singlestepflow', 'unassigned_singlestepflow'))"#, + &ids, + w_id, + ) + .fetch_all(db) + .await?; + let admitted: HashSet = chain + .into_iter() + .filter(|r| { + windmill_api_auth::scopes::cancel_confinement_admits(&confinement, &r.runnable_path) + }) + .map(|r| r.origin) + .collect(); + Ok(ids.into_iter().filter(|id| admitted.contains(id)).collect()) +} + +async fn require_job_within_cancel_scope( + db: &DB, + authed: &ApiAuthed, + w_id: &str, + job_id: Uuid, +) -> error::Result<()> { + if filter_jobs_within_cancel_scope(db, authed, w_id, vec![job_id]) + .await? + .is_empty() + { + return Err(Error::NotFound(format!("Job {job_id} not found"))); + } + Ok(()) +} + /// Self + every `parent_job` ancestor (intermediate sub-flows up to the top-level /// root) of `job_id`, resolved via the root DB (flow lineage is not sensitive). /// Falls back to `[job_id]` if the row is absent so callers still run their probe. @@ -2156,6 +2249,41 @@ async fn require_job_update_read_access( require_job_read_access(db, user_db, authed, w_id, job_id, &created_by, view_token).await } +/// The per-job check of the cancel routes. A `jobs:cancel` grant stands on its own, like +/// `jobs:write`: its paths confine it (`filter_jobs_within_cancel_scope`), not the +/// token's `jobs:run` scopes, which bound what a run token may read. Intersecting the two +/// would not hold anyway, since the token can mint itself a child holding only the +/// cancel scope. +async fn require_job_cancel_access( + db: &DB, + user_db: &UserDB, + authed: &ApiAuthed, + w_id: &str, + job_id: &Uuid, +) -> error::Result<()> { + let created_by = sqlx::query_scalar!( + "SELECT created_by FROM v2_job WHERE id = $1 AND workspace_id = $2", + job_id, + w_id, + ) + .fetch_optional(db) + .await? + .ok_or_else(|| Error::NotFound(format!("Job {job_id} not found")))?; + let run_confined = !windmill_api_auth::scopes::has_job_cancel_grant(authed.scopes.as_deref()); + require_job_access( + db, + user_db, + authed, + w_id, + job_id, + &created_by, + None, + run_confined, + ) + .await?; + require_job_within_cancel_scope(db, authed, w_id, *job_id).await +} + /// Whether a validated approval token should grant the job-read bypass. The token alone /// is sufficient unless the current approval step has `user_auth_required`, in which case /// only an authorized approver may read the job (and thus its args/flow inputs). @@ -4372,6 +4500,38 @@ async fn cancel_selection( let force_cancel = query.force_cancel.unwrap_or(false); let mut cancelled = Vec::new(); for (workspace_id, ids) in jobs_by_workspace { + let ids = + if windmill_api_auth::scopes::job_cancel_path_confinement(authed.scopes.as_deref()) + .is_some() + { + // Checked on the job the cancel actually kills: a force cancel reaches the + // highest queued ancestor. + let mut targets = Vec::with_capacity(ids.len()); + for id in ids { + let target = if force_cancel { + force_cancel_target(&db, &workspace_id, id).await? + } else { + id + }; + targets.push((id, target)); + } + let admitted: HashSet = filter_jobs_within_cancel_scope( + &db, + &authed, + &workspace_id, + targets.iter().map(|(_, t)| *t).collect(), + ) + .await? + .into_iter() + .collect(); + targets + .into_iter() + .filter(|(_, t)| admitted.contains(t)) + .map(|(id, _)| id) + .collect() + } else { + ids + }; let Json(mut w_cancelled) = cancel_jobs( ids, &db, diff --git a/backend/windmill-api/src/token.rs b/backend/windmill-api/src/token.rs index a1a9206196..53dfb2a214 100644 --- a/backend/windmill-api/src/token.rs +++ b/backend/windmill-api/src/token.rs @@ -200,6 +200,11 @@ lazy_static! { label: "Write".to_string(), requires_resource_path: false, }, + ScopeOption { + value: "jobs:cancel".to_string(), + label: "Cancel".to_string(), + requires_resource_path: true, + }, ScopeOption { value: "jobs:run:scripts".to_string(), label: "Run scripts".to_string(),