From 2b6b267342beea72c6b99ecf3baaf3930508fe8f Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sun, 18 Oct 2026 11:57:22 +0200 Subject: [PATCH 1/5] feat: probe nsjail with a real jail and add a gVisor compat mode Co-Authored-By: Claude Opus 5.5 --- .../windmill-worker/nsjail/probe.config.proto | 47 +++++ backend/windmill-worker/src/common.rs | 16 ++ backend/windmill-worker/src/duckdb_runner.rs | 2 + backend/windmill-worker/src/lib.rs | 2 + backend/windmill-worker/src/nsjail_compat.rs | 191 ++++++++++++++++++ backend/windmill-worker/src/worker.rs | 64 ++++-- 6 files changed, 309 insertions(+), 13 deletions(-) create mode 100644 backend/windmill-worker/nsjail/probe.config.proto create mode 100644 backend/windmill-worker/src/nsjail_compat.rs diff --git a/backend/windmill-worker/nsjail/probe.config.proto b/backend/windmill-worker/nsjail/probe.config.proto new file mode 100644 index 0000000000000..2fdc5abdcaebf --- /dev/null +++ b/backend/windmill-worker/nsjail/probe.config.proto @@ -0,0 +1,47 @@ +name: "nsjail startup probe" + +mode: ONCE +hostname: "probe" +log_level: WARNING +time_limit: 10 + +# Same namespaces and rlimit handling as the job profiles (nsjail's default rlimits, a cgroup +# namespace, the user namespace unless DISABLE_NUSER): a worker whose kernel or runtime refuses +# any of them cannot run jobs either. +clone_newnet: false +clone_newuser: {CLONE_NEWUSER} + +skip_setsid: true +keep_caps: false +mount_proc: true + +mount { + src: "/bin" + dst: "/bin" + is_bind: true +} +mount { + src: "/lib" + dst: "/lib" + is_bind: true +} +mount { + src: "/lib64" + dst: "/lib64" + is_bind: true + mandatory: false +} +mount { + src: "/usr" + dst: "/usr" + is_bind: true +} +mount { + dst: "/tmp" + fstype: "tmpfs" + rw: true +} + +exec_bin { + path: "/bin/true" +} diff --git a/backend/windmill-worker/src/common.rs b/backend/windmill-worker/src/common.rs index 32bd6dc7e9344..eda64c6d19c78 100644 --- a/backend/windmill-worker/src/common.rs +++ b/backend/windmill-worker/src/common.rs @@ -976,12 +976,28 @@ pub fn build_command_with_isolation(program: &str, args: &[&str]) -> Command { } } +/// Under `NSJAIL_GVISOR_COMPAT`, rewrites the config of an nsjail `cmd` for gVisor. A spawn of +/// nsjail that bypasses `start_child_process` must call this itself. +#[cfg(target_os = "linux")] +pub fn apply_nsjail_gvisor_compat(mut cmd: Command) -> Result { + if *crate::NSJAIL_GVISOR_COMPAT && cmd.as_std().get_program() == crate::NSJAIL_PATH.as_str() { + crate::nsjail_compat::apply(cmd.as_std_mut()).map_err(|e| { + Error::ExecutionErr(format!( + "NSJAIL_GVISOR_COMPAT could not prepare nsjail: {e}" + )) + })?; + } + Ok(cmd) +} + pub async fn start_child_process( cmd: Command, executable: &str, disable_process_group: bool, ) -> Result, Error> { use process_wrap::tokio::*; + #[cfg(target_os = "linux")] + let cmd = apply_nsjail_gvisor_compat(cmd)?; let mut cmd = TokioCommandWrap::from(cmd); // On Windows, put the child in its own console process group so a CTRL_BREAK_EVENT diff --git a/backend/windmill-worker/src/duckdb_runner.rs b/backend/windmill-worker/src/duckdb_runner.rs index 46f5c5cbde73b..c41847f1eb0c7 100644 --- a/backend/windmill-worker/src/duckdb_runner.rs +++ b/backend/windmill-worker/src/duckdb_runner.rs @@ -475,6 +475,8 @@ async fn spawn_runner( }); } } + #[cfg(target_os = "linux")] + let mut cmd = crate::common::apply_nsjail_gvisor_compat(cmd)?; let mut child = cmd.spawn().map_err(|e| { Error::InternalErr(format!("Could not start the DuckDB runner ({mode:?}): {e}")) })?; diff --git a/backend/windmill-worker/src/lib.rs b/backend/windmill-worker/src/lib.rs index e56334df67d72..a8fe610e2c7b2 100644 --- a/backend/windmill-worker/src/lib.rs +++ b/backend/windmill-worker/src/lib.rs @@ -62,6 +62,8 @@ pub mod memory_ee; pub mod memory_oss; #[cfg(feature = "mysql")] mod mysql_executor; +#[cfg(target_os = "linux")] +mod nsjail_compat; #[cfg(feature = "nu")] mod nu_executor; #[cfg(feature = "oracledb")] diff --git a/backend/windmill-worker/src/nsjail_compat.rs b/backend/windmill-worker/src/nsjail_compat.rs new file mode 100644 index 0000000000000..bcadb4b77aa9a --- /dev/null +++ b/backend/windmill-worker/src/nsjail_compat.rs @@ -0,0 +1,191 @@ +//! `NSJAIL_GVISOR_COMPAT=true`: run nsjail inside gVisor (runsc), which refuses +//! `clone(CLONE_NEWCGROUP)` and `setrlimit(RLIMIT_MSGQUEUE)`, both of which nsjail does by +//! default. Each config is rewritten right before nsjail starts: no cgroup namespace, and +//! `disable_rl` with the config's own rlimits set on the nsjail process, which the jail +//! inherits. A limit already at its value is not set again, so the inherited message +//! queue limit, which nsjail only reasserts, never reaches gVisor. + +use std::ffi::OsStr; +use std::io; +use std::path::PathBuf; +use std::process::Command; + +use nix::libc; + +/// Marks a config whose `disable_rl` was added here, so that a config spawned twice keeps its +/// limits: its `rlimit_*` lines are still in it, and nsjail ignores them under `disable_rl`. +const COMPAT_MARKER: &str = "# NSJAIL_GVISOR_COMPAT: the worker sets the rlimits below"; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RlimitValue { + Value(u64), + Soft, + Hard, + Inf, +} + +/// nsjail's rlimit options: name, resource, default, default type and unit (nsjail +/// config.proto). `Value(0)` stands for the type VALUE, whose value is `rlimit_`. +#[rustfmt::skip] +const RLIMITS: [(&str, Resource, u64, RlimitValue, u64); 10] = [ + ("as", libc::RLIMIT_AS, 4096, RlimitValue::Value(0), 1 << 20), + ("core", libc::RLIMIT_CORE, 0, RlimitValue::Value(0), 1 << 20), + ("cpu", libc::RLIMIT_CPU, 600, RlimitValue::Value(0), 1), + ("fsize", libc::RLIMIT_FSIZE, 1, RlimitValue::Value(0), 1 << 20), + ("nofile", libc::RLIMIT_NOFILE, 32, RlimitValue::Value(0), 1), + ("nproc", libc::RLIMIT_NPROC, 1024, RlimitValue::Soft, 1), + ("stack", libc::RLIMIT_STACK, 8, RlimitValue::Soft, 1 << 20), + ("memlock", libc::RLIMIT_MEMLOCK, 64, RlimitValue::Soft, 1 << 10), + ("rtprio", libc::RLIMIT_RTPRIO, 0, RlimitValue::Soft, 1), + ("msgqueue", libc::RLIMIT_MSGQUEUE, 1024, RlimitValue::Soft, 1), +]; + +#[cfg(target_env = "gnu")] +type Resource = libc::__rlimit_resource_t; +#[cfg(not(target_env = "gnu"))] +type Resource = libc::c_int; + +/// The config nsjail runs under gVisor, and the rlimits it would have set itself, or `None` +/// when the config already leaves them to the parent. +pub(crate) fn compat_config(config: &str) -> (String, Option>) { + let field = |name: &str| { + config.lines().rev().find_map(|l| { + let v = l + .trim() + .strip_prefix(name)? + .trim_start() + .strip_prefix(':')?; + v.split('#').next().map(str::trim) + }) + }; + let marked = config.lines().any(|l| l.trim() == COMPAT_MARKER); + let rlimits = (marked || field("disable_rl") != Some("true")).then(|| { + RLIMITS + .iter() + .map(|&(name, res, default, default_type, unit)| { + let value = field(&format!("rlimit_{name}")) + .and_then(|v| v.parse::().ok()) + .unwrap_or(default); + let kind = match field(&format!("rlimit_{name}_type")) { + Some("SOFT") => RlimitValue::Soft, + Some("HARD") => RlimitValue::Hard, + Some("INF") => RlimitValue::Inf, + Some("VALUE") => RlimitValue::Value(0), + _ => default_type, + }; + let kind = match kind { + RlimitValue::Value(_) => RlimitValue::Value(value.saturating_mul(unit)), + k => k, + }; + (res, kind) + }) + .collect() + }); + + let mut out = config + .lines() + .filter(|l| !l.trim_start().starts_with("clone_newcgroup")) + .collect::>() + .join("\n"); + out.push_str("\nclone_newcgroup: false\n"); + if rlimits.is_some() && !marked { + out.push_str(COMPAT_MARKER); + out.push_str("\ndisable_rl: true\n"); + } + (out, rlimits) +} + +/// Rewrites the config `cmd` (an nsjail command line) passes with `--config`, and makes the +/// child set that config's rlimits on itself before it execs nsjail. +pub(crate) fn apply(cmd: &mut Command) -> io::Result<()> { + let args = cmd.get_args().collect::>(); + let Some(config) = args.windows(2).find(|w| w[0] == "--config").map(|w| w[1]) else { + return Ok(()); + }; + let path = match cmd.get_current_dir() { + Some(dir) => dir.join(config), + None => PathBuf::from(config), + }; + let (rewritten, rlimits) = compat_config(&std::fs::read_to_string(&path)?); + std::fs::write(&path, rewritten)?; + let Some(rlimits) = rlimits else { + return Ok(()); + }; + let limits = rlimits + .into_iter() + .map(|(res, value)| { + let current = getrlimit(res)?; + let v = match value { + RlimitValue::Value(v) => v, + RlimitValue::Soft => current.rlim_cur, + RlimitValue::Hard => current.rlim_max, + RlimitValue::Inf => libc::RLIM_INFINITY, + }; + Ok((res, v)) + }) + .collect::>>()?; + use std::os::unix::process::CommandExt; + // Only raw syscalls here, between fork and exec. + unsafe { + cmd.pre_exec(move || { + for &(res, v) in &limits { + let current = getrlimit(res)?; + if current.rlim_cur == v && current.rlim_max == v { + continue; + } + let new = libc::rlimit { rlim_cur: v, rlim_max: v }; + if libc::setrlimit(res, &new) != 0 { + return Err(io::Error::last_os_error()); + } + } + Ok(()) + }); + } + Ok(()) +} + +fn getrlimit(res: Resource) -> io::Result { + let mut current = libc::rlimit { rlim_cur: 0, rlim_max: 0 }; + if unsafe { libc::getrlimit(res, &mut current) } != 0 { + return Err(io::Error::last_os_error()); + } + Ok(current) +} + +#[cfg(test)] +mod tests { + use super::*; + + // What nsjail itself would set for the python run profile, and the same again for a + // config the compat rewrite already went through. + #[test] + fn compat_config_keeps_the_profile_rlimits() { + let profile = include_str!("../nsjail/run.python3.config.proto") + .replace("{RLIMIT_AS}", "rlimit_as: 4096"); + let (rewritten, rlimits) = compat_config(&profile); + let mib = 1 << 20; + let expected = vec![ + (libc::RLIMIT_AS, RlimitValue::Value(4096 * mib)), + (libc::RLIMIT_CORE, RlimitValue::Value(0)), + (libc::RLIMIT_CPU, RlimitValue::Value(1000)), + (libc::RLIMIT_FSIZE, RlimitValue::Value(1000 * mib)), + (libc::RLIMIT_NOFILE, RlimitValue::Value(10000)), + (libc::RLIMIT_NPROC, RlimitValue::Soft), + (libc::RLIMIT_STACK, RlimitValue::Soft), + (libc::RLIMIT_MEMLOCK, RlimitValue::Soft), + (libc::RLIMIT_RTPRIO, RlimitValue::Soft), + (libc::RLIMIT_MSGQUEUE, RlimitValue::Soft), + ]; + assert_eq!(rlimits.as_ref(), Some(&expected)); + assert!(rewritten.contains("\nclone_newcgroup: false\n")); + assert!(rewritten.contains("\ndisable_rl: true\n")); + + let (again, rlimits) = compat_config(&rewritten); + assert_eq!(rlimits, Some(expected)); + assert_eq!(again.matches("disable_rl").count(), 1); + assert_eq!(again.matches("clone_newcgroup").count(), 1); + + let (_, rlimits) = compat_config(include_str!("../nsjail/run.bash.config.proto")); + assert_eq!(rlimits, None); + } +} diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index cef3b1bbfccff..b1db3c148c55c 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -261,6 +261,7 @@ const NUM_SECS_PING: u64 = 5; const NUM_SECS_READINGS: u64 = 60; const INCLUDE_DEPS_PY_SH_CONTENT: &str = include_str!("../nsjail/download_deps.py.sh"); +const NSJAIL_CONFIG_PROBE_CONTENT: &str = include_str!("../nsjail/probe.config.proto"); const WORKER_SHELL_NAP_TIME_DURATION: u64 = 15; const WORKER_SHELL_INITIAL_NAP_TIME_DURATION: u64 = 5; @@ -335,6 +336,14 @@ pub struct PowershellRepo { pub pat: String, } +/// Runs nsjail in a form gVisor (runsc) accepts, see `nsjail_compat`. +pub static NSJAIL_GVISOR_COMPAT: std::sync::LazyLock = std::sync::LazyLock::new(|| { + std::env::var("NSJAIL_GVISOR_COMPAT") + .ok() + .and_then(|x| x.parse::().ok()) + .unwrap_or(false) +}); + lazy_static::lazy_static! { static ref SLEEP_QUEUE_BASE: u64 = std::env::var("SLEEP_QUEUE") @@ -546,26 +555,31 @@ lazy_static::lazy_static! { pub static ref NSJAIL_AVAILABLE: Option = { let nsjail_path = NSJAIL_PATH.as_str(); - let test_result = std::process::Command::new(nsjail_path) - .arg("--help") - .output(); - - match test_result { + match probe_nsjail(nsjail_path) { Ok(output) if output.status.success() => { - tracing::info!("nsjail available at: {}", nsjail_path); + tracing::info!( + "nsjail available at: {}{}", + nsjail_path, + if *NSJAIL_GVISOR_COMPAT { " (NSJAIL_GVISOR_COMPAT)" } else { "" } + ); Some(nsjail_path.to_string()) }, Ok(output) => { - let stderr = String::from_utf8_lossy(&output.stderr); - tracing::error!( - "nsjail test failed (exit code: {}). stderr: '{}'. path: '{}'. \ - Nsjail sandboxing will NOT be available. \ - nsjail should be included in all standard windmill images. \ - If job_isolation is set to 'nsjail_sandboxing' in Instance Settings, jobs will fail.", + let msg = format!( + "nsjail could not run a test jail (exit code: {}). stderr: '{}'. path: '{}'. \ + Nsjail sandboxing will NOT be available: the worker reports its job isolation \ + without nsjail, and jobs that require nsjail fail. Under gVisor (runsc), see \ + NSJAIL_GVISOR_COMPAT.", output.status, - stderr.trim(), + String::from_utf8_lossy(&output.stderr).trim(), nsjail_path ); + // Most workers never jail anything, and most containers cannot. + if *DISABLE_NSJAIL { + tracing::info!("{msg}"); + } else { + tracing::error!("{msg}"); + } None }, Err(e) => { @@ -1021,6 +1035,30 @@ pub(crate) fn artifact_cache_name( format!("mod-{}", windmill_common::utils::calculate_hash(&keyed)) } +/// Runs `/bin/true` in a jail built like a job's, through the same spawn path, so a worker +/// whose runtime lets nsjail start but not jail (a container without the privileges, gVisor) +/// does not report nsjail isolation. +fn probe_nsjail(nsjail_path: &str) -> std::io::Result { + let dir = std::env::temp_dir().join(format!("windmill-nsjail-probe-{}", std::process::id())); + std::fs::create_dir_all(&dir)?; + std::fs::write( + dir.join("probe.config.proto"), + NSJAIL_CONFIG_PROBE_CONTENT.replace("{CLONE_NEWUSER}", &(!*DISABLE_NUSER).to_string()), + )?; + let mut cmd = std::process::Command::new(nsjail_path); + cmd.current_dir(&dir) + .env_clear() + .env("PATH", PATH_ENV.as_str()) + .args(["--config", "probe.config.proto"]); + #[cfg(target_os = "linux")] + if *NSJAIL_GVISOR_COMPAT { + crate::nsjail_compat::apply(&mut cmd)?; + } + let output = cmd.output(); + let _ = std::fs::remove_dir_all(&dir); + output +} + pub fn is_sandboxing_enabled() -> bool { if !*DISABLE_NSJAIL { return true; From 280ba9097ec1b16ecdcdfbb8d19d9acf6ceffa30 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sun, 18 Oct 2026 12:09:12 +0200 Subject: [PATCH 2/5] fix: route dbt captured phases through the gVisor compat step and harden the probe dir Co-Authored-By: Claude Opus 5.5 --- backend/windmill-worker/src/dbt_executor.rs | 4 ++ backend/windmill-worker/src/worker.rs | 57 ++++++++++++++------- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/backend/windmill-worker/src/dbt_executor.rs b/backend/windmill-worker/src/dbt_executor.rs index dbef02bec3b9c..a29f50f6b0079 100644 --- a/backend/windmill-worker/src/dbt_executor.rs +++ b/backend/windmill-worker/src/dbt_executor.rs @@ -4053,6 +4053,10 @@ pub(crate) async fn run_captured( ) -> error::Result { use tokio::io::AsyncReadExt; + #[cfg(target_os = "linux")] + { + cmd = crate::common::apply_nsjail_gvisor_compat(cmd)?; + } let mut child = cmd .stdout(Stdio::piped()) .stderr(Stdio::piped()) diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index b1db3c148c55c..bf59844ca6aaf 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -582,7 +582,14 @@ lazy_static::lazy_static! { } None }, - Err(e) => { + Err(NsjailProbeError::Setup(e)) => { + tracing::error!( + "Could not prepare the nsjail test jail: {}. Nsjail sandboxing will NOT be available.", + e + ); + None + }, + Err(NsjailProbeError::Start(e)) => { if e.kind() == std::io::ErrorKind::NotFound { tracing::error!( "nsjail not found at '{}'. Nsjail sandboxing will NOT be available. \ @@ -1038,23 +1045,37 @@ pub(crate) fn artifact_cache_name( /// Runs `/bin/true` in a jail built like a job's, through the same spawn path, so a worker /// whose runtime lets nsjail start but not jail (a container without the privileges, gVisor) /// does not report nsjail isolation. -fn probe_nsjail(nsjail_path: &str) -> std::io::Result { - let dir = std::env::temp_dir().join(format!("windmill-nsjail-probe-{}", std::process::id())); - std::fs::create_dir_all(&dir)?; - std::fs::write( - dir.join("probe.config.proto"), - NSJAIL_CONFIG_PROBE_CONTENT.replace("{CLONE_NEWUSER}", &(!*DISABLE_NUSER).to_string()), - )?; - let mut cmd = std::process::Command::new(nsjail_path); - cmd.current_dir(&dir) - .env_clear() - .env("PATH", PATH_ENV.as_str()) - .args(["--config", "probe.config.proto"]); - #[cfg(target_os = "linux")] - if *NSJAIL_GVISOR_COMPAT { - crate::nsjail_compat::apply(&mut cmd)?; - } - let output = cmd.output(); +enum NsjailProbeError { + /// The probe could not be prepared: nothing is known about nsjail. + Setup(std::io::Error), + /// nsjail itself could not be executed. + Start(std::io::Error), +} + +fn probe_nsjail(nsjail_path: &str) -> Result { + // A fresh private directory: the worker runs what this config says, as itself. + let dir = std::env::temp_dir().join(format!("windmill-nsjail-probe-{}", uuid::Uuid::new_v4())); + let mut builder = std::fs::DirBuilder::new(); + #[cfg(unix)] + std::os::unix::fs::DirBuilderExt::mode(&mut builder, 0o700); + builder.create(&dir).map_err(NsjailProbeError::Setup)?; + let output = (|| { + std::fs::write( + dir.join("probe.config.proto"), + NSJAIL_CONFIG_PROBE_CONTENT.replace("{CLONE_NEWUSER}", &(!*DISABLE_NUSER).to_string()), + ) + .map_err(NsjailProbeError::Setup)?; + let mut cmd = std::process::Command::new(nsjail_path); + cmd.current_dir(&dir) + .env_clear() + .env("PATH", PATH_ENV.as_str()) + .args(["--config", "probe.config.proto"]); + #[cfg(target_os = "linux")] + if *NSJAIL_GVISOR_COMPAT { + crate::nsjail_compat::apply(&mut cmd).map_err(NsjailProbeError::Setup)?; + } + cmd.output().map_err(NsjailProbeError::Start) + })(); let _ = std::fs::remove_dir_all(&dir); output } From bafcf5f1cac1626f2899960b70603c8caa901b43 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sun, 18 Oct 2026 12:23:43 +0200 Subject: [PATCH 3/5] feat: patch nsjail with a gvisor_compat option instead of applying rlimits from the worker Co-Authored-By: Claude Opus 5.5 --- Dockerfile | 4 +- backend/windmill-worker/src/common.rs | 10 +- backend/windmill-worker/src/dbt_executor.rs | 5 +- backend/windmill-worker/src/duckdb_runner.rs | 3 +- backend/windmill-worker/src/lib.rs | 1 - backend/windmill-worker/src/nsjail_compat.rs | 184 ++----------------- backend/windmill-worker/src/worker.rs | 3 +- docker/DockerfileSlim | 4 +- docker/DockerfileSlimEe | 4 +- docker/nsjail-patches/gvisor-compat.patch | 143 ++++++++++++++ 10 files changed, 173 insertions(+), 188 deletions(-) create mode 100644 docker/nsjail-patches/gvisor-compat.patch diff --git a/Dockerfile b/Dockerfile index e078c1bfef81f..79f9d05734641 100644 --- a/Dockerfile +++ b/Dockerfile @@ -18,7 +18,9 @@ RUN apt-get -y update \ pkg-config=1.8.* \ protobuf-compiler=3.21.* -RUN git clone -b master --single-branch https://github.com/google/nsjail.git . && git checkout dccf911fd2659e7b08ce9507c25b2b38ec2c5800 +COPY docker/nsjail-patches/ /nsjail-patches/ +RUN git clone -b master --single-branch https://github.com/google/nsjail.git . && git checkout dccf911fd2659e7b08ce9507c25b2b38ec2c5800 \ + && git apply /nsjail-patches/*.patch RUN make FROM ${RUST_IMAGE} AS rust_base diff --git a/backend/windmill-worker/src/common.rs b/backend/windmill-worker/src/common.rs index eda64c6d19c78..2ac205a29893e 100644 --- a/backend/windmill-worker/src/common.rs +++ b/backend/windmill-worker/src/common.rs @@ -978,16 +978,15 @@ pub fn build_command_with_isolation(program: &str, args: &[&str]) -> Command { /// Under `NSJAIL_GVISOR_COMPAT`, rewrites the config of an nsjail `cmd` for gVisor. A spawn of /// nsjail that bypasses `start_child_process` must call this itself. -#[cfg(target_os = "linux")] -pub fn apply_nsjail_gvisor_compat(mut cmd: Command) -> Result { +pub fn apply_nsjail_gvisor_compat(cmd: &Command) -> Result<(), Error> { if *crate::NSJAIL_GVISOR_COMPAT && cmd.as_std().get_program() == crate::NSJAIL_PATH.as_str() { - crate::nsjail_compat::apply(cmd.as_std_mut()).map_err(|e| { + crate::nsjail_compat::apply(cmd.as_std()).map_err(|e| { Error::ExecutionErr(format!( "NSJAIL_GVISOR_COMPAT could not prepare nsjail: {e}" )) })?; } - Ok(cmd) + Ok(()) } pub async fn start_child_process( @@ -996,8 +995,7 @@ pub async fn start_child_process( disable_process_group: bool, ) -> Result, Error> { use process_wrap::tokio::*; - #[cfg(target_os = "linux")] - let cmd = apply_nsjail_gvisor_compat(cmd)?; + apply_nsjail_gvisor_compat(&cmd)?; let mut cmd = TokioCommandWrap::from(cmd); // On Windows, put the child in its own console process group so a CTRL_BREAK_EVENT diff --git a/backend/windmill-worker/src/dbt_executor.rs b/backend/windmill-worker/src/dbt_executor.rs index a29f50f6b0079..28db2753e1bb0 100644 --- a/backend/windmill-worker/src/dbt_executor.rs +++ b/backend/windmill-worker/src/dbt_executor.rs @@ -4053,10 +4053,7 @@ pub(crate) async fn run_captured( ) -> error::Result { use tokio::io::AsyncReadExt; - #[cfg(target_os = "linux")] - { - cmd = crate::common::apply_nsjail_gvisor_compat(cmd)?; - } + crate::common::apply_nsjail_gvisor_compat(&cmd)?; let mut child = cmd .stdout(Stdio::piped()) .stderr(Stdio::piped()) diff --git a/backend/windmill-worker/src/duckdb_runner.rs b/backend/windmill-worker/src/duckdb_runner.rs index c41847f1eb0c7..41d2cdd3357a9 100644 --- a/backend/windmill-worker/src/duckdb_runner.rs +++ b/backend/windmill-worker/src/duckdb_runner.rs @@ -475,8 +475,7 @@ async fn spawn_runner( }); } } - #[cfg(target_os = "linux")] - let mut cmd = crate::common::apply_nsjail_gvisor_compat(cmd)?; + crate::common::apply_nsjail_gvisor_compat(&cmd)?; let mut child = cmd.spawn().map_err(|e| { Error::InternalErr(format!("Could not start the DuckDB runner ({mode:?}): {e}")) })?; diff --git a/backend/windmill-worker/src/lib.rs b/backend/windmill-worker/src/lib.rs index a8fe610e2c7b2..1945d163467de 100644 --- a/backend/windmill-worker/src/lib.rs +++ b/backend/windmill-worker/src/lib.rs @@ -62,7 +62,6 @@ pub mod memory_ee; pub mod memory_oss; #[cfg(feature = "mysql")] mod mysql_executor; -#[cfg(target_os = "linux")] mod nsjail_compat; #[cfg(feature = "nu")] mod nu_executor; diff --git a/backend/windmill-worker/src/nsjail_compat.rs b/backend/windmill-worker/src/nsjail_compat.rs index bcadb4b77aa9a..4fcd0bc9f60e7 100644 --- a/backend/windmill-worker/src/nsjail_compat.rs +++ b/backend/windmill-worker/src/nsjail_compat.rs @@ -1,103 +1,27 @@ -//! `NSJAIL_GVISOR_COMPAT=true`: run nsjail inside gVisor (runsc), which refuses -//! `clone(CLONE_NEWCGROUP)` and `setrlimit(RLIMIT_MSGQUEUE)`, both of which nsjail does by -//! default. Each config is rewritten right before nsjail starts: no cgroup namespace, and -//! `disable_rl` with the config's own rlimits set on the nsjail process, which the jail -//! inherits. A limit already at its value is not set again, so the inherited message -//! queue limit, which nsjail only reasserts, never reaches gVisor. +//! `NSJAIL_GVISOR_COMPAT=true`: run nsjail inside gVisor (runsc). Each config is rewritten +//! right before nsjail starts, with `gvisor_compat: true` (an option of the nsjail the images +//! build, from `docker/nsjail-patches/`) and no cgroup namespace, which gVisor refuses. use std::ffi::OsStr; use std::io; use std::path::PathBuf; use std::process::Command; -use nix::libc; - -/// Marks a config whose `disable_rl` was added here, so that a config spawned twice keeps its -/// limits: its `rlimit_*` lines are still in it, and nsjail ignores them under `disable_rl`. -const COMPAT_MARKER: &str = "# NSJAIL_GVISOR_COMPAT: the worker sets the rlimits below"; - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum RlimitValue { - Value(u64), - Soft, - Hard, - Inf, -} - -/// nsjail's rlimit options: name, resource, default, default type and unit (nsjail -/// config.proto). `Value(0)` stands for the type VALUE, whose value is `rlimit_`. -#[rustfmt::skip] -const RLIMITS: [(&str, Resource, u64, RlimitValue, u64); 10] = [ - ("as", libc::RLIMIT_AS, 4096, RlimitValue::Value(0), 1 << 20), - ("core", libc::RLIMIT_CORE, 0, RlimitValue::Value(0), 1 << 20), - ("cpu", libc::RLIMIT_CPU, 600, RlimitValue::Value(0), 1), - ("fsize", libc::RLIMIT_FSIZE, 1, RlimitValue::Value(0), 1 << 20), - ("nofile", libc::RLIMIT_NOFILE, 32, RlimitValue::Value(0), 1), - ("nproc", libc::RLIMIT_NPROC, 1024, RlimitValue::Soft, 1), - ("stack", libc::RLIMIT_STACK, 8, RlimitValue::Soft, 1 << 20), - ("memlock", libc::RLIMIT_MEMLOCK, 64, RlimitValue::Soft, 1 << 10), - ("rtprio", libc::RLIMIT_RTPRIO, 0, RlimitValue::Soft, 1), - ("msgqueue", libc::RLIMIT_MSGQUEUE, 1024, RlimitValue::Soft, 1), -]; - -#[cfg(target_env = "gnu")] -type Resource = libc::__rlimit_resource_t; -#[cfg(not(target_env = "gnu"))] -type Resource = libc::c_int; - -/// The config nsjail runs under gVisor, and the rlimits it would have set itself, or `None` -/// when the config already leaves them to the parent. -pub(crate) fn compat_config(config: &str) -> (String, Option>) { - let field = |name: &str| { - config.lines().rev().find_map(|l| { - let v = l - .trim() - .strip_prefix(name)? - .trim_start() - .strip_prefix(':')?; - v.split('#').next().map(str::trim) - }) - }; - let marked = config.lines().any(|l| l.trim() == COMPAT_MARKER); - let rlimits = (marked || field("disable_rl") != Some("true")).then(|| { - RLIMITS - .iter() - .map(|&(name, res, default, default_type, unit)| { - let value = field(&format!("rlimit_{name}")) - .and_then(|v| v.parse::().ok()) - .unwrap_or(default); - let kind = match field(&format!("rlimit_{name}_type")) { - Some("SOFT") => RlimitValue::Soft, - Some("HARD") => RlimitValue::Hard, - Some("INF") => RlimitValue::Inf, - Some("VALUE") => RlimitValue::Value(0), - _ => default_type, - }; - let kind = match kind { - RlimitValue::Value(_) => RlimitValue::Value(value.saturating_mul(unit)), - k => k, - }; - (res, kind) - }) - .collect() - }); - +pub(crate) fn compat_config(config: &str) -> String { let mut out = config .lines() - .filter(|l| !l.trim_start().starts_with("clone_newcgroup")) + .filter(|l| { + let l = l.trim_start(); + !l.starts_with("clone_newcgroup") && !l.starts_with("gvisor_compat") + }) .collect::>() .join("\n"); - out.push_str("\nclone_newcgroup: false\n"); - if rlimits.is_some() && !marked { - out.push_str(COMPAT_MARKER); - out.push_str("\ndisable_rl: true\n"); - } - (out, rlimits) + out.push_str("\nclone_newcgroup: false\ngvisor_compat: true\n"); + out } -/// Rewrites the config `cmd` (an nsjail command line) passes with `--config`, and makes the -/// child set that config's rlimits on itself before it execs nsjail. -pub(crate) fn apply(cmd: &mut Command) -> io::Result<()> { +/// Rewrites the config `cmd` (an nsjail command line) passes with `--config`. +pub(crate) fn apply(cmd: &Command) -> io::Result<()> { let args = cmd.get_args().collect::>(); let Some(config) = args.windows(2).find(|w| w[0] == "--config").map(|w| w[1]) else { return Ok(()); @@ -106,86 +30,6 @@ pub(crate) fn apply(cmd: &mut Command) -> io::Result<()> { Some(dir) => dir.join(config), None => PathBuf::from(config), }; - let (rewritten, rlimits) = compat_config(&std::fs::read_to_string(&path)?); - std::fs::write(&path, rewritten)?; - let Some(rlimits) = rlimits else { - return Ok(()); - }; - let limits = rlimits - .into_iter() - .map(|(res, value)| { - let current = getrlimit(res)?; - let v = match value { - RlimitValue::Value(v) => v, - RlimitValue::Soft => current.rlim_cur, - RlimitValue::Hard => current.rlim_max, - RlimitValue::Inf => libc::RLIM_INFINITY, - }; - Ok((res, v)) - }) - .collect::>>()?; - use std::os::unix::process::CommandExt; - // Only raw syscalls here, between fork and exec. - unsafe { - cmd.pre_exec(move || { - for &(res, v) in &limits { - let current = getrlimit(res)?; - if current.rlim_cur == v && current.rlim_max == v { - continue; - } - let new = libc::rlimit { rlim_cur: v, rlim_max: v }; - if libc::setrlimit(res, &new) != 0 { - return Err(io::Error::last_os_error()); - } - } - Ok(()) - }); - } - Ok(()) -} - -fn getrlimit(res: Resource) -> io::Result { - let mut current = libc::rlimit { rlim_cur: 0, rlim_max: 0 }; - if unsafe { libc::getrlimit(res, &mut current) } != 0 { - return Err(io::Error::last_os_error()); - } - Ok(current) -} - -#[cfg(test)] -mod tests { - use super::*; - - // What nsjail itself would set for the python run profile, and the same again for a - // config the compat rewrite already went through. - #[test] - fn compat_config_keeps_the_profile_rlimits() { - let profile = include_str!("../nsjail/run.python3.config.proto") - .replace("{RLIMIT_AS}", "rlimit_as: 4096"); - let (rewritten, rlimits) = compat_config(&profile); - let mib = 1 << 20; - let expected = vec![ - (libc::RLIMIT_AS, RlimitValue::Value(4096 * mib)), - (libc::RLIMIT_CORE, RlimitValue::Value(0)), - (libc::RLIMIT_CPU, RlimitValue::Value(1000)), - (libc::RLIMIT_FSIZE, RlimitValue::Value(1000 * mib)), - (libc::RLIMIT_NOFILE, RlimitValue::Value(10000)), - (libc::RLIMIT_NPROC, RlimitValue::Soft), - (libc::RLIMIT_STACK, RlimitValue::Soft), - (libc::RLIMIT_MEMLOCK, RlimitValue::Soft), - (libc::RLIMIT_RTPRIO, RlimitValue::Soft), - (libc::RLIMIT_MSGQUEUE, RlimitValue::Soft), - ]; - assert_eq!(rlimits.as_ref(), Some(&expected)); - assert!(rewritten.contains("\nclone_newcgroup: false\n")); - assert!(rewritten.contains("\ndisable_rl: true\n")); - - let (again, rlimits) = compat_config(&rewritten); - assert_eq!(rlimits, Some(expected)); - assert_eq!(again.matches("disable_rl").count(), 1); - assert_eq!(again.matches("clone_newcgroup").count(), 1); - - let (_, rlimits) = compat_config(include_str!("../nsjail/run.bash.config.proto")); - assert_eq!(rlimits, None); - } + let rewritten = compat_config(&std::fs::read_to_string(&path)?); + std::fs::write(&path, rewritten) } diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index bf59844ca6aaf..4048a3603c0bb 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -1070,9 +1070,8 @@ fn probe_nsjail(nsjail_path: &str) -> Result parseArgs(int argc, char* argv[]) { + nsjconf->rl_rtpr = parseRLimit(RLIMIT_RTPRIO, "soft", 1); + nsjconf->rl_msgq = parseRLimit(RLIMIT_MSGQUEUE, "soft", 1); + nsjconf->disable_rl = false; ++ nsjconf->gvisor_compat = false; + nsjconf->personality = 0; + nsjconf->clone_newnet = true; + nsjconf->clone_newuser = true; +@@ -603,6 +605,9 @@ std::unique_ptr parseArgs(int argc, char* argv[]) { + case 0x0208: + nsjconf->disable_rl = true; + break; ++ case 0x0510: ++ nsjconf->gvisor_compat = true; ++ break; + case 0x0301: + nsjconf->personality |= ADDR_COMPAT_LAYOUT; + break; +diff --git a/config.cc b/config.cc +index 620d08e..9427069 100644 +--- a/config.cc ++++ b/config.cc +@@ -166,6 +166,7 @@ static bool configParseInternal(nsjconf_t* nsjconf, const nsjail::NsJailConfig& + configRLimit(RLIMIT_MSGQUEUE, njc.rlimit_msgqueue_type(), njc.rlimit_msgqueue()); + + nsjconf->disable_rl = njc.disable_rl(); ++ nsjconf->gvisor_compat = njc.gvisor_compat(); + + if (njc.persona_addr_compat_layout()) { + nsjconf->personality |= ADDR_COMPAT_LAYOUT; +diff --git a/config.proto b/config.proto +index cd297ae..96b477c 100644 +--- a/config.proto ++++ b/config.proto +@@ -266,4 +266,10 @@ message NsJailConfig { + /* Binary path (with arguments) to be executed. If not specified here, it + can be specified with cmd-line as "-- /path/to/command arg1 arg2" */ + optional Exe exec_bin = 90; ++ ++ /* Tolerate what gVisor (runsc) does not implement: an EPERM from ++ setrlimit(RLIMIT_MSGQUEUE) or from PR_SET_SECUREBITS when the UID does ++ not change, and no rtnetlink link cache when no interface is moved ++ into the jail */ ++ optional bool gvisor_compat = 93 [default = false]; + } +diff --git a/contain.cc b/contain.cc +index b5120cc..41e69f8 100644 +--- a/contain.cc ++++ b/contain.cc +@@ -172,8 +172,12 @@ static bool containSetLimits(nsjconf_t* nsjconf) { + } + rl.rlim_cur = rl.rlim_max = nsjconf->rl_msgq; + if (setrlimit64(RLIMIT_MSGQUEUE, &rl) == -1) { +- PLOG_E("setrlimit64(0, RLIMIT_MSGQUEUE , %" PRIu64 ")", nsjconf->rl_msgq); +- return false; ++ if (!nsjconf->gvisor_compat || errno != EPERM) { ++ PLOG_E("setrlimit64(0, RLIMIT_MSGQUEUE , %" PRIu64 ")", nsjconf->rl_msgq); ++ return false; ++ } ++ PLOG_W("setrlimit64(0, RLIMIT_MSGQUEUE , %" PRIu64 "), ignored (gvisor_compat)", ++ nsjconf->rl_msgq); + } + return true; + } +diff --git a/net.cc b/net.cc +index 87c3df9..b4c64f4 100644 +--- a/net.cc ++++ b/net.cc +@@ -138,6 +138,10 @@ bool initNsFromParent(nsjconf_t* nsjconf, int pid) { + if (!nsjconf->clone_newnet) { + return true; + } ++ /* gVisor's rtnetlink replies are rejected by libnl's link cache, which is unused here */ ++ if (nsjconf->gvisor_compat && nsjconf->ifaces.empty() && nsjconf->iface_vs.empty()) { ++ return true; ++ } + struct nl_sock* sk = nl_socket_alloc(); + if (!sk) { + LOG_E("Could not allocate socket with nl_socket_alloc()"); +diff --git a/nsjail.h b/nsjail.h +index 5687ca7..fa2b9fa 100644 +--- a/nsjail.h ++++ b/nsjail.h +@@ -119,6 +119,7 @@ struct nsjconf_t { + uint64_t rl_rtpr; + uint64_t rl_msgq; + bool disable_rl; ++ bool gvisor_compat; + unsigned long personality; + bool clone_newnet; + bool clone_newuser; +diff --git a/user.cc b/user.cc +index a335e2d..f3e578b 100644 +--- a/user.cc ++++ b/user.cc +@@ -260,10 +260,24 @@ bool initNsFromChild(nsjconf_t* nsjconf) { + * Make sure all capabilities are retained after the subsequent setuid/setgid, as they will + * be needed for privileged operations: mounts, uts change etc. + */ ++ bool securebits_set = true; + if (prctl(PR_SET_SECUREBITS, SECBIT_KEEP_CAPS | SECBIT_NO_SETUID_FIXUP, 0UL, 0UL, 0UL) == + -1) { +- PLOG_E("prctl(PR_SET_SECUREBITS, SECBIT_KEEP_CAPS | SECBIT_NO_SETUID_FIXUP)"); +- return false; ++ /* ++ * The bits only matter when setresuid() below changes the UID: without them, ++ * leaving UID 0 clears the capabilities still needed by the jail setup. ++ */ ++ uid_t ruid, euid, suid; ++ const uid_t uid = nsjconf->uids[0].inside_id; ++ if (!nsjconf->gvisor_compat || errno != EPERM || ++ getresuid(&ruid, &euid, &suid) == -1 || ruid != uid || euid != uid || ++ suid != uid) { ++ PLOG_E("prctl(PR_SET_SECUREBITS, SECBIT_KEEP_CAPS | SECBIT_NO_SETUID_FIXUP)"); ++ return false; ++ } ++ PLOG_W("prctl(PR_SET_SECUREBITS, SECBIT_KEEP_CAPS | SECBIT_NO_SETUID_FIXUP), " ++ "ignored (gvisor_compat, UID unchanged)"); ++ securebits_set = false; + } + + /* +@@ -306,7 +320,7 @@ bool initNsFromChild(nsjconf_t* nsjconf) { + * unexpectedly retaining capabilities after a UID/GID + * change. + */ +- if (prctl(PR_SET_SECUREBITS, 0UL, 0UL, 0UL, 0UL) == -1) { ++ if (securebits_set && prctl(PR_SET_SECUREBITS, 0UL, 0UL, 0UL, 0UL) == -1) { + PLOG_E("prctl(PR_SET_SECUREBITS, 0)"); + return false; + } From da6749429d496b69a80080c1bda60f54b32d49b5 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sun, 18 Oct 2026 12:30:50 +0200 Subject: [PATCH 4/5] fix: keep DuckDB jailed on a sandboxed worker whose nsjail probe failed Co-Authored-By: Claude Opus 5.5 --- backend/windmill-worker/src/duckdb_runner.rs | 7 ++++--- backend/windmill-worker/src/nsjail_compat.rs | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/backend/windmill-worker/src/duckdb_runner.rs b/backend/windmill-worker/src/duckdb_runner.rs index 41d2cdd3357a9..b7c0bc6321794 100644 --- a/backend/windmill-worker/src/duckdb_runner.rs +++ b/backend/windmill-worker/src/duckdb_runner.rs @@ -23,8 +23,7 @@ use windmill_common::error::{Error, Result}; use crate::{ common::{build_command_with_isolation, resolve_nsjail_tmp_mount_block, DEV_CONF_NSJAIL}, - is_sandboxing_enabled, is_unshare_enabled, DISABLE_NUSER, HOME_ENV, NSJAIL_AVAILABLE, - NSJAIL_PATH, PATH_ENV, + is_sandboxing_enabled, is_unshare_enabled, DISABLE_NUSER, HOME_ENV, NSJAIL_PATH, PATH_ENV, }; const NSJAIL_CONFIG_RUN_DUCKDB_CONTENT: &str = include_str!("../nsjail/run.duckdb.config.proto"); @@ -42,7 +41,9 @@ pub(crate) enum RunnerMode { /// Decided once per job, so every connection a job opens runs the same way. pub(crate) fn runner_mode() -> RunnerMode { - if cfg!(unix) && is_sandboxing_enabled() && NSJAIL_AVAILABLE.is_some() { + // Not gated on `NSJAIL_AVAILABLE`: a sandboxed worker whose nsjail cannot jail must fail + // the job, as every other jailed executor does, not run it outside the jail. + if cfg!(unix) && is_sandboxing_enabled() { RunnerMode::Jailed } else if is_unshare_enabled() { // `build_command_with_isolation` logs and runs unisolated when `unshare` is missing, diff --git a/backend/windmill-worker/src/nsjail_compat.rs b/backend/windmill-worker/src/nsjail_compat.rs index 4fcd0bc9f60e7..3069dcee52b6a 100644 --- a/backend/windmill-worker/src/nsjail_compat.rs +++ b/backend/windmill-worker/src/nsjail_compat.rs @@ -33,3 +33,17 @@ pub(crate) fn apply(cmd: &Command) -> io::Result<()> { let rewritten = compat_config(&std::fs::read_to_string(&path)?); std::fs::write(&path, rewritten) } + +#[cfg(test)] +mod tests { + // nsjail rejects a config that sets a scalar field twice, which would fail every jailed job. + #[test] + fn compat_config_sets_each_field_once() { + let bun = include_str!("../nsjail/run.bun.config.proto"); + assert!(bun.contains("clone_newcgroup: false")); + let twice = super::compat_config(&super::compat_config(bun)); + assert_eq!(twice.matches("clone_newcgroup").count(), 1); + assert_eq!(twice.matches("gvisor_compat").count(), 1); + assert!(twice.ends_with("\nclone_newcgroup: false\ngvisor_compat: true\n")); + } +} From cf220c211a49186d6877f718b8879046f62c4e6b Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sun, 18 Oct 2026 12:33:01 +0200 Subject: [PATCH 5/5] docs: link the upstream gVisor issues from the nsjail patch Co-Authored-By: Claude Opus 5.5 --- docker/nsjail-patches/gvisor-compat.patch | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docker/nsjail-patches/gvisor-compat.patch b/docker/nsjail-patches/gvisor-compat.patch index 6ca78bc23f759..70176b7272c1c 100644 --- a/docker/nsjail-patches/gvisor-compat.patch +++ b/docker/nsjail-patches/gvisor-compat.patch @@ -1,3 +1,13 @@ +gvisor_compat: an opt-in nsjail option (config field and --gvisor_compat) for running inside +gVisor (runsc). With it false, nsjail behaves as upstream. With it true: +- no rtnetlink link cache when no interface is moved into the jail: gVisor acks NLM_F_DUMP + requests before NLMSG_DONE and libnl rejects that (google/gvisor#15945); +- an EPERM from setrlimit(RLIMIT_MSGQUEUE) is a warning: gVisor refuses that limit for any + value (google/gvisor#15944); +- an EPERM from PR_SET_SECUREBITS is tolerated when the UID does not change, where the bits + have no effect (SECBIT_NO_SETUID_FIXUP lands in gVisor with google/gvisor#15130). +Applied to google/nsjail dccf911fd2659e7b08ce9507c25b2b38ec2c5800 by the Dockerfiles. + diff --git a/cmdline.cc b/cmdline.cc index 1b52b33..b49ce16 100644 --- a/cmdline.cc