From dca7e16532c90feb03f5f7ce1ed76ca096337365 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Mon, 12 Jan 2026 06:21:05 +0100 Subject: [PATCH] fix: tighten preview path (#7541) --- backend/ee-repo-ref.txt | 2 +- backend/windmill-api/src/jobs.rs | 5 ++- backend/windmill-api/src/users.rs | 53 +++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index d03b2537b8..1bbd214709 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -c8e8a6df19203acc2cef1aebd1bd4157f2439cbf \ No newline at end of file +0c02f5aba882b2e16361836835545cbf9e9ab275 \ No newline at end of file diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 7f47676de1..81a6055d22 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -64,7 +64,7 @@ use crate::{ concurrency_groups::join_concurrency_key, db::{ApiAuthed, DB}, triggers::trigger_helpers::RunnableId, - users::{get_scope_tags, require_owner_of_path, OptAuthed}, + users::{get_scope_tags, require_owner_of_path, require_path_read_access_for_preview, OptAuthed}, utils::{check_scopes, content_plain, require_super_admin}, }; use anyhow::Context; @@ -6018,6 +6018,7 @@ async fn run_preview_script( "Operators cannot run preview jobs for security reasons".to_string(), )); } + require_path_read_access_for_preview(&authed, &preview.path)?; let scheduled_for = run_query.get_scheduled_for(&db).await?; let tag = run_query.tag.clone().or(preview.tag.clone()); check_tag_available_for_workspace(&db, &w_id, &tag, &authed).await?; @@ -6160,6 +6161,7 @@ async fn run_bundle_preview_script( let data = data.map_err(to_anyhow)?; if name == "preview" { let preview: Preview = serde_json::from_slice(&data).map_err(to_anyhow)?; + require_path_read_access_for_preview(&authed, &preview.path)?; format = preview .format .and_then(|s| BundleFormat::from_string(&s)) @@ -6771,6 +6773,7 @@ async fn run_preview_flow_job( "Operators cannot run preview jobs for security reasons".to_string(), )); } + require_path_read_access_for_preview(&authed, &raw_flow.path)?; let scheduled_for = run_query.get_scheduled_for(&db).await?; let tag = run_query.tag.clone().or(raw_flow.tag.clone()); check_tag_available_for_workspace(&db, &w_id, &tag, &authed).await?; diff --git a/backend/windmill-api/src/users.rs b/backend/windmill-api/src/users.rs index 8dbf7efa9e..5c03d54646 100644 --- a/backend/windmill-api/src/users.rs +++ b/backend/windmill-api/src/users.rs @@ -929,6 +929,59 @@ pub fn require_owner_of_path(authed: &ApiAuthed, path: &str) -> Result<()> { } } +/// Checks that a user has at least read access to the path for preview jobs. +/// This prevents privilege escalation where a user could run preview code +/// under a path they don't have access to. +pub fn require_path_read_access_for_preview(authed: &ApiAuthed, path: &Option) -> Result<()> { + let Some(path) = path else { + return Ok(()); + }; + + if authed.is_admin { + return Ok(()); + } + + if path.is_empty() { + return Ok(()); + } + + let splitted: Vec<&str> = path.split('/').collect(); + if splitted.len() < 2 { + return Err(Error::BadRequest(format!( + "Invalid path format for preview job: {}", + path + ))); + } + + match splitted[0] { + "u" => { + if splitted[1] == authed.username { + Ok(()) + } else { + Err(Error::BadRequest(format!( + "You can only run preview jobs in your own namespace (u/{}) or in folders you have read access to", + authed.username + ))) + } + } + "f" => { + let folder = splitted[1]; + if authed.folders.iter().any(|(f, _, _)| f == folder) { + Ok(()) + } else { + Err(Error::BadRequest(format!( + "You do not have read access to folder '{}'. Preview jobs require at least read access to the target folder.", + folder + ))) + } + } + _ => Err(Error::BadRequest(format!( + "Invalid path format for preview job: {}. Path must start with 'u/' or 'f/'", + path + ))), + } +} + pub fn get_perm_in_extra_perms_for_authed( v: serde_json::Value, authed: &ApiAuthed,