diff --git a/backend/windmill-worker/nsjail/download.py.config.proto b/backend/windmill-worker/nsjail/download.py.config.proto index 18957bb4a5..a68d96eb92 100644 --- a/backend/windmill-worker/nsjail/download.py.config.proto +++ b/backend/windmill-worker/nsjail/download.py.config.proto @@ -30,7 +30,8 @@ clone_newnet: false clone_newuser: {CLONE_NEWUSER} skip_setsid: true -keep_caps: true +# No capability needed: writes go to the rw-mounted dirs as the jail's own uid, like the run.* profiles. +keep_caps: false keep_env: true mount_proc: true diff --git a/backend/windmill-worker/nsjail/download.ruby.config.proto b/backend/windmill-worker/nsjail/download.ruby.config.proto index 31eefae766..3a3a6f3b74 100644 --- a/backend/windmill-worker/nsjail/download.ruby.config.proto +++ b/backend/windmill-worker/nsjail/download.ruby.config.proto @@ -23,7 +23,8 @@ clone_newnet: false clone_newuser: {CLONE_NEWUSER} skip_setsid: true -keep_caps: true +# No capability needed: writes go to the rw-mounted dirs as the jail's own uid, like the run.* profiles. +keep_caps: false keep_env: true mount_proc: true diff --git a/backend/windmill-worker/nsjail/download_deps.py.sh b/backend/windmill-worker/nsjail/download_deps.py.sh index 6b4cd8cfd8..8f0ebd1cd0 100755 --- a/backend/windmill-worker/nsjail/download_deps.py.sh +++ b/backend/windmill-worker/nsjail/download_deps.py.sh @@ -20,20 +20,22 @@ then echo "\$TRUSTED_HOST is set to $TRUSTED_HOST" fi -CMD="/usr/local/bin/uv pip install -\"$REQ\" ---target \"$TARGET\" ---no-cache ---no-config ---no-color ---no-deps ---link-mode=copy -$PY_PATH -$INDEX_URL_ARG $EXTRA_INDEX_URL_ARG $TRUSTED_HOST_ARG ---system ---reinstall ---compile-bytecode -" +# Build the argv directly so each value reaches uv as a single argument, quoting +# intact (e.g. PEP 508 markers). The unquoted expansions are worker configuration +# and split into separate arguments on purpose. +set -- /usr/local/bin/uv pip install \ + "$REQ" \ + --target "$TARGET" \ + --no-cache \ + --no-config \ + --no-color \ + --no-deps \ + --link-mode=copy \ + $PY_PATH \ + $INDEX_URL_ARG $EXTRA_INDEX_URL_ARG $TRUSTED_HOST_ARG \ + --system \ + --reinstall \ + --compile-bytecode -echo $CMD -eval $CMD +echo "$@" +exec "$@" diff --git a/backend/windmill-worker/nsjail/install.r.config.proto b/backend/windmill-worker/nsjail/install.r.config.proto index dff9acde15..e844b2e720 100644 --- a/backend/windmill-worker/nsjail/install.r.config.proto +++ b/backend/windmill-worker/nsjail/install.r.config.proto @@ -16,7 +16,8 @@ clone_newnet: false clone_newuser: {CLONE_NEWUSER} skip_setsid: true -keep_caps: true +# No capability needed: writes go to the rw-mounted dirs as the jail's own uid, like the run.* profiles. +keep_caps: false keep_env: true mount_proc: true diff --git a/backend/windmill-worker/nsjail/lock.py.config.proto b/backend/windmill-worker/nsjail/lock.py.config.proto index 14119c4089..60c5573f40 100644 --- a/backend/windmill-worker/nsjail/lock.py.config.proto +++ b/backend/windmill-worker/nsjail/lock.py.config.proto @@ -23,7 +23,8 @@ clone_newnet: false clone_newuser: {CLONE_NEWUSER} skip_setsid: true -keep_caps: true +# No capability needed: writes go to the rw-mounted dirs as the jail's own uid, like the run.* profiles. +keep_caps: false keep_env: true mount_proc: true diff --git a/backend/windmill-worker/nsjail/lock.ruby.config.proto b/backend/windmill-worker/nsjail/lock.ruby.config.proto index 6f82f3b748..bcca74e77e 100644 --- a/backend/windmill-worker/nsjail/lock.ruby.config.proto +++ b/backend/windmill-worker/nsjail/lock.ruby.config.proto @@ -19,7 +19,8 @@ clone_newnet: false clone_newuser: {CLONE_NEWUSER} skip_setsid: true -keep_caps: true +# No capability needed: writes go to the rw-mounted dirs as the jail's own uid, like the run.* profiles. +keep_caps: false keep_env: true mount_proc: true diff --git a/backend/windmill-worker/src/nsjail_compat.rs b/backend/windmill-worker/src/nsjail_compat.rs index 3069dcee52..e3b1f7656f 100644 --- a/backend/windmill-worker/src/nsjail_compat.rs +++ b/backend/windmill-worker/src/nsjail_compat.rs @@ -46,4 +46,90 @@ mod tests { assert_eq!(twice.matches("gvisor_compat").count(), 1); assert!(twice.ends_with("\nclone_newcgroup: false\ngvisor_compat: true\n")); } + + /// Every nsjail profile drops its capabilities. + #[test] + fn every_nsjail_config_drops_capabilities() { + let dir = concat!(env!("CARGO_MANIFEST_DIR"), "/nsjail"); + let mut checked = 0; + for entry in std::fs::read_dir(dir).expect("nsjail config dir") { + let path = entry.expect("dir entry").path(); + if path.extension().and_then(|e| e.to_str()) != Some("proto") { + continue; + } + let content = std::fs::read_to_string(&path).expect("read config"); + let settings = content + .lines() + .map(str::trim) + .filter(|l| l.starts_with("keep_caps")) + .collect::>(); + assert_eq!( + settings, + vec!["keep_caps: false"], + "{} must set keep_caps: false exactly once", + path.display() + ); + checked += 1; + } + assert!( + checked >= 20, + "only {checked} nsjail configs found in {dir}, expected all of them" + ); + } + + /// `REQ` and `TARGET` reach uv as single, unaltered argv words. + #[cfg(unix)] + #[test] + fn download_deps_passes_the_requirement_as_a_literal_argv_word() { + use std::os::unix::fs::PermissionsExt; + + let dir = std::env::temp_dir().join(format!("wm_download_deps_{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("scratch dir"); + + // Stand-in for uv that prints each argument it receives on its own line. + let uv = dir.join("uv_argv_dump"); + std::fs::write( + &uv, + "#!/bin/sh\nfor a in \"$@\"; do printf '<%s>\\n' \"$a\"; done\n", + ) + .expect("write stub"); + std::fs::set_permissions(&uv, std::fs::Permissions::from_mode(0o755)).expect("chmod stub"); + + let script = dir.join("download_deps.sh"); + std::fs::write( + &script, + include_str!("../nsjail/download_deps.py.sh") + .replace("/usr/local/bin/uv", uv.to_str().unwrap()), + ) + .expect("write script"); + + let marker = dir.join("marker"); + let req = format!("x==$(touch {})", marker.display()); + let target = dir.join("target dir with spaces"); + let out = std::process::Command::new("/bin/sh") + .arg(&script) + .env_clear() + .env("PATH", "/usr/bin:/bin") + .env("REQ", &req) + .env("TARGET", &target) + .output() + .expect("run download_deps.sh"); + let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); + + assert!( + !marker.exists(), + "REQ was not passed through verbatim; stdout was:\n{stdout}" + ); + assert!( + stdout.contains(&format!("<{req}>\n")), + "the requirement must be one literal argv word; stdout was:\n{stdout}" + ); + assert!( + stdout.contains(&format!("<--target>\n<{}>\n", target.display())), + "--target must be followed by the target dir as one argv word; stdout was:\n{stdout}" + ); + + std::fs::remove_dir_all(&dir).expect("cleanup scratch dir"); + } }