From 7a8cb62fa135bff04522f4bbd93aba194e33b121 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sat, 17 Oct 2026 11:26:21 +0200 Subject: [PATCH] fix: mount python script modules into the nsjail sandbox (#11846) Co-authored-by: Claude Opus 5.5 --- .../nsjail/run.python3.config.proto | 2 + .../windmill-worker/src/python_executor.rs | 47 ++++++++++++++++++- backend/windmill-worker/src/worker.rs | 2 +- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/backend/windmill-worker/nsjail/run.python3.config.proto b/backend/windmill-worker/nsjail/run.python3.config.proto index 6e457207f5..6b18f6d7fa 100644 --- a/backend/windmill-worker/nsjail/run.python3.config.proto +++ b/backend/windmill-worker/nsjail/run.python3.config.proto @@ -62,6 +62,8 @@ mount { is_bind: true } +{MODULE_MOUNTS} + mount { src: "{JOB_DIR}/loader.py" dst: "/tmp/loader.py" diff --git a/backend/windmill-worker/src/python_executor.rs b/backend/windmill-worker/src/python_executor.rs index 77874653a0..430d6e82ef 100644 --- a/backend/windmill-worker/src/python_executor.rs +++ b/backend/windmill-worker/src/python_executor.rs @@ -153,8 +153,8 @@ use crate::{ common::{ build_command_with_isolation, create_args_and_out_file, get_reserved_variables, read_file, read_result, render_nsjail_rlimit_as, resolve_nsjail_timeout, - resolve_nsjail_tmp_mount_block, start_child_process, OccupancyMetrics, StreamNotifier, - DEV_CONF_NSJAIL, + proto_str, resolve_nsjail_tmp_mount_block, start_child_process, OccupancyMetrics, + StreamNotifier, DEV_CONF_NSJAIL, }, get_proxy_envs_for_lang, get_run_proxy_envs, handle_child::handle_child, @@ -1148,6 +1148,7 @@ mount {{ ) }) .join("\n"); + let module_mounts = python_module_mounts(job_dir, &dirs, modules.as_ref()); let nsjail_timeout = resolve_nsjail_timeout(conn, &job.workspace_id, job.id, job.timeout).await; let _ = write_file( @@ -1167,6 +1168,7 @@ mount {{ ) .replace("{SHARED_MOUNT}", shared_mount) .replace("{SHARED_DEPENDENCIES}", shared_deps.as_str()) + .replace("{MODULE_MOUNTS}", &module_mounts) .replace("{GLOBAL_SITE_PACKAGES}", &global_site_packages_path) .replace("{MAIN}", format!("{dirs}/{last}").as_str()) .replace( @@ -1327,6 +1329,28 @@ mount {{ Ok(result) } +/// nsjail mounts for a job's module files. They are written next to the main script, and the +/// jail sees only what it mounts: without these, a module import falls through to the +/// loader's API fetch. Keys come from the request, hence the containment check and quoting. +fn python_module_mounts( + job_dir: &str, + dirs: &str, + modules: Option<&HashMap>, +) -> String { + modules + .into_iter() + .flat_map(|m| m.keys()) + .filter(|rel| crate::worker::is_contained_relative_path(rel)) + .map(|rel| { + format!( + "mount {{\n src: {}\n dst: {}\n is_bind: true\n mandatory: false\n}}\n", + proto_str(&format!("{job_dir}/{dirs}/{rel}")), + proto_str(&format!("/tmp/{dirs}/{rel}")), + ) + }) + .join("\n") +} + /// Generate Python code to spread preprocessor args from `kwargs` into `pre_args`. /// The `indent` parameter controls the join separator for multi-arg spreads. fn python_preprocessor_spread(sig: &windmill_parser::MainArgSignature, indent: &str) -> String { @@ -3741,6 +3765,25 @@ pub async fn start_worker( #[cfg(test)] mod tests { + #[test] + fn module_files_are_mounted_next_to_the_main_script() { + let module = |content: &str| windmill_common::scripts::ScriptModule { + content: content.to_string(), + language: windmill_common::scripts::ScriptLang::Python3, + lock: None, + }; + let modules = HashMap::from([ + ("lib/util.py".to_string(), module("")), + ("../escape.py".to_string(), module("")), + ("q\"uote.py".to_string(), module("")), + ]); + let mounts = super::python_module_mounts("/jobs/1", "u/admin", Some(&modules)); + assert!(mounts.contains("src: \"/jobs/1/u/admin/lib/util.py\""), "{mounts}"); + assert!(mounts.contains("dst: \"/tmp/u/admin/lib/util.py\""), "{mounts}"); + assert!(!mounts.contains("escape"), "{mounts}"); + assert!(mounts.contains("q\\\"uote.py"), "{mounts}"); + } + use super::*; #[test] diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index feb5502801..26c83881c8 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -5962,7 +5962,7 @@ async fn record_declared_warehouse_write( /// True when `path` contains only `Normal`/`CurDir` components, i.e. it cannot /// escape the directory it is joined onto (no `..`, no absolute root, no Windows /// drive prefix). -fn is_contained_relative_path(path: &str) -> bool { +pub(crate) fn is_contained_relative_path(path: &str) -> bool { use std::path::Component; std::path::Path::new(path) .components()