Skip to content

feat: probe nsjail with a real jail and add a gVisor compat mode - #11865

Merged
rubenfiszel merged 5 commits into
mainfrom
nsjail-gvisor-compat
Oct 18, 2026
Merged

rubenfiszel merged 5 commits into
mainfrom
nsjail-gvisor-compat

Conversation

@rubenfiszel

@rubenfiszel rubenfiszel commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two related changes to how workers use nsjail.

  1. A real startup probe. The worker used to decide that nsjail was available by running nsjail --help. That succeeds in any container that ships the binary, even when nsjail cannot build a jail there: an unprivileged container, a host that restricts unprivileged user namespaces, or a gVisor (runsc) sandbox. The worker then reported job_isolation=nsjail while every jailed job failed. The probe now runs /bin/true in a jail with the same namespaces, rlimit handling and user-namespace switch as the job profiles (nsjail/probe.config.proto).

    • If the probe fails, NSJAIL_AVAILABLE is None. The worker reports the isolation it really has (unshare or none) and logs nsjail's own error. That is the same fallback as when nsjail is missing: jobs that require nsjail still fail closed.
    • The probe costs one short jail at startup, about 5 ms on runc and 30 ms under gVisor.
    • Its failure is logged at info when DISABLE_NSJAIL is set (the default), since most such workers never jail anything, and at error otherwise.
    • The probe config is written to a fresh 0700 temp directory, and errors setting the probe up are reported separately from failures to execute nsjail.
  2. NSJAIL_GVISOR_COMPAT=true, opt-in, for workers under gVisor such as GKE Sandbox (RuntimeClass gvisor). Stock nsjail cannot create a jail on released gVisor (release-20261012.0), for four reasons:

    nsjail patch (docker/nsjail-patches/gvisor-compat.patch, applied with git apply in the nsjail stage of Dockerfile, docker/DockerfileSlim and docker/DockerfileSlimEe). It adds one config field and command-line flag, gvisor_compat (default false). When it is false, nsjail behaves exactly as upstream. When it is true:

    • The rtnetlink link cache is not built when no interface is moved into the jail (no iface_own, no MACVLAN). The cache is only used for that.
    • setrlimit(RLIMIT_MSGQUEUE) EPERM is a warning. Every other rlimit stays fatal.
    • PR_SET_SECUREBITS EPERM is tolerated only when the real, effective and saved UIDs already equal the jail's UID. Then setresuid() changes nothing, so the bits would have had no effect. A jail that changes UID still fails.

    Windmill side. In compat mode the worker rewrites each nsjail config right before nsjail starts, adding clone_newcgroup: false and gvisor_compat: true. The rewrite happens at spawn time rather than in the 24 profiles, because those are rendered by about 36 separate .replace chains with no shared renderer. The spawn is the one place they all pass through: start_child_process, plus the two spawns that bypass it, the DuckDB runner and dbt's run_captured. The probe takes the same path. A spawn that missed the rewrite would fail under gVisor rather than run with less isolation.

Changes

  • nsjail/probe.config.proto, probe_nsjail and NSJAIL_AVAILABLE in worker.rs: the startup probe.
  • nsjail_compat.rs, plus apply_nsjail_gvisor_compat in common.rs, called from start_child_process, duckdb_runner::spawn_runner and dbt_executor::run_captured.
  • NSJAIL_GVISOR_COMPAT is a standalone LazyLock, because worker.rs's lazy_static! block is at its macro recursion limit.
  • docker/nsjail-patches/gvisor-compat.patch, applied in the three Dockerfiles that build nsjail.

Test plan

Verified live on a k3s 1.35 VM: server under runc, worker pod under runc or under RuntimeClass gvisor with runsc release-20261012.0, the current release. The worker binary was built from this branch. nsjail was built from the pinned commit with the patch applied, in a debian:trixie-slim stage identical to the Dockerfiles'. Jobs were bash, python3, bun, python with a pip dependency (lock + download profiles), and no_network bash.

  • released runsc, compat on, uid 1000, all capabilities dropped: 5/5. The worker reports nsjail; no_network leaves only lo and egress fails.
  • released runsc, compat on, root with DISABLE_NUSER=true + SYS_ADMIN/SETPCAP: bash, python3 and no_network pass.
  • Capabilities inside the jail, all of the above and both runc runs: CapInh/CapPrm/CapEff/CapBnd/CapAmb are all 0000000000000000 and NoNewPrivs: 1 in every case. Verbose nsjail on released runsc shows all three tolerances firing, with the full bounding set still dropped.
  • Securebits stays fatal on a UID change: root with --user 1000 and no user namespace fails on PR_SET_SECUREBITS under compat.
  • Limits: the python jail's /proc/self/limits match runc (AS 4 GiB, CPU 1000, FSIZE 1000 MiB, NOFILE 10000, CORE 0, msgqueue 819200). MEMLOCK follows the pod's own soft limit, which nsjail pins, and gVisor's pod default is lower.
  • runc, compat off, patched nsjail: unchanged, 5/5 (non-root user-namespace jail and privileged root).
  • runc, compat on: 5/5, identical capabilities and limits.
  • released runsc, compat off: the probe logs the clone(...CLONE_NEWCGROUP...) EINVAL. The worker reports none (unshare with ENABLE_UNSHARE_PID=true), and jailed jobs fail closed.
  • Ubuntu 24.04 node with kernel.apparmor_restrict_unprivileged_userns=1, non-root runc worker: the probe reports none instead of nsjail. This is the existing silent failure the probe now surfaces.
  • Dev backend on a plain host: the probe succeeds and its directory is removed.
  • DuckDB and dbt, released runsc, compat on, uid 1000, no caps. Checked DuckDB SELECT 42, and a DuckDB query globbing /tmp/windmill/*, /proc/[0-9]* and /home/* from inside the jail, which returns 0 each, as on runc. Also checked dbt (dbt-core 1.12 + dbt-duckdb, installed at runtime) build with two models (PASS=2), and show, which goes through run_captured. The same set passes on runc with compat off.
  • DuckDB and dbt fail closed: on released runsc with compat off (probe failed, worker reports none, DISABLE_NSJAIL=false), every DuckDB and dbt job fails with nsjail's launch error and none returns rows. DuckDB is no longer run outside the jail.
  • Not exercised: a full image build. The nsjail stage was built standalone with the same commands.

Docs: windmill-labs/windmilldocs PR to follow (a gVisor section on the security/isolation page, plus the env var).

Upstream: the gVisor gaps are filed as google/gvisor#15944 and google/gvisor#15945. The Linux-neutral form of the netlink and msgqueue changes is proposed upstream in google/nsjail#365; the patch can be dropped once that lands and gVisor ships #15130.

🤖 Generated with Claude Code

rubenfiszel and others added 2 commits October 18, 2026 11:57
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…den the probe dir

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: cf220c2
Status: ✅  Deploy successful!
Preview URL: https://ff3cc6e0.windmill.pages.dev
Branch Preview URL: https://nsjail-gvisor-compat.windmill.pages.dev

View logs

…imits from the worker

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rubenfiszel

Copy link
Copy Markdown
Contributor Author

/review

@claude

claude Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @rubenfiszel's task in 1m 17s —— View job


Good to merge

Review: probe nsjail with a real jail, plus a gVisor compat mode

  • Read diff and AGENTS.md
  • Checked the spawn paths, the config rewrite and the probe
  • Posted review

I found no P0, P1 or P2 issues. These are the points I checked:

  • Rewrite coverage. Every --config call site (bash, python lock/download/run, bun, go, rust, csharp, java, php, ruby, r, nu, pwsh, ansible, docker_v2, duckdb, dbt) builds Command::new(NSJAIL_PATH). Each one goes through start_child_process, duckdb_runner::spawn_runner or dbt_executor::run_captured, and all three now call apply_nsjail_gvisor_compat. Since get_program() is compared with NSJAIL_PATH, non-nsjail spawns are left untouched.
  • Config path resolution. nsjail_compat::apply joins the config path to cmd.get_current_dir(), which is the same lookup nsjail does. Configs are written per job or per requirement into job_dir (for example {req}.config.proto), so jobs never rewrite a config another job is using. The rewrite is idempotent: it removes any existing clone_newcgroup or gvisor_compat line before appending.
  • Probe strictness. The probe uses nsjail's default rlimits. Profiles like run.bash set disable_rl: true, so the probe is the stricter of the two: whenever it passes, both kinds of profile can run. When it fails, NSJAIL_AVAILABLE = None, and every consumer (#sandbox annotations, docker_v2, worker_utils job isolation, duckdb_runner) fails closed with a clear message. The server process only evaluates the probe when the setting is nsjail_sandboxing, because monitor.rs:3329 short-circuits.
  • Patch. With gvisor_compat=false the patch leaves nsjail's behavior unchanged. The securebits tolerance only applies when real, effective and saved UIDs all equal the jail UID, so a jail that changes UID still fails. The second PR_SET_SECUREBITS(0) is skipped only when the first call never set the bits.
  • Probe directory. It is a fresh 0700 directory with a UUID name and is removed after the probe runs. Setup errors are reported separately from start errors.

Optional, non-blocking: .devcontainer/Dockerfile still builds unpatched nsjail. If someone sets NSJAIL_GVISOR_COMPAT=true there, nsjail will reject the unknown config field. The probe then fails closed, so nothing breaks unsafely.

AGENTS.md compliance

Comments state constraints rather than narrating the change, and the new module has a //! doc that matches what it does. No issues.

Test coverage

  • Backend. No unit tests were added. nsjail_compat::compat_config is a pure function, and a small test would pin its idempotency and the removal of existing clone_newcgroup/gvisor_compat lines. The probe and the compat spawn path need a real nsjail and runtime, so the live k3s/runc/runsc matrix in the PR description is the right verification for them.
  • Docker/CI. No automated tests are expected.
  • Manual testing still open, as listed in the PR. With NSJAIL_GVISOR_COMPAT=true and nsjail enabled, run a DuckDB script and a dbt job under runsc; both should finish without a CLONE_NEWCGROUP error in the job log. Also build one of the three images in full, to confirm that COPY docker/nsjail-patches/ resolves from the build context and that git apply succeeds.

@github-actions

Copy link
Copy Markdown
Contributor

Codex Review

cc @rubenfiszel

Should address issues before merging: DuckDB fails open after a failed nsjail probe

  • [P1] DuckDB fails open after a failed nsjail probe — backend/windmill-worker/src/worker.rs:583

    With nsjail installed but unable to create namespaces, the new probe sets NSJAIL_AVAILABLE to None. Even when DISABLE_NSJAIL=false or instance isolation requires nsjail_sandboxing, duckdb_runner::runner_mode() then selects Plain or Unshare, and do_duckdb() executes the query without filesystem confinement. Previously, --help succeeded, so DuckDB attempted a jailed launch and failed.

    This turns a sandbox failure into unconfined execution: query SQL can read and write files accessible to the worker user. Make DuckDB reject execution when nsjail is required but unavailable, rather than selecting a weaker mode.

Test coverage

No committed Rust unit or worker integration tests cover the new probe/config rewriting. Add a regression covering DuckDB’s response to a failed probe when sandboxing is required.

I built the Dockerfile’s nsjail stage successfully, ran the probe successfully under privileged runc, confirmed it fails in a restricted container, and passed two isolated Rust checks covering config rewriting, idempotence, relative paths, and missing files. I did not run full worker integration. The PR author reports the broader gVisor job, capability, and limit checks.

No automated tests are expected for the Docker/config wiring itself. Full final-image builds remain unverified.

Exercise dbt’s captured phases and DuckDB jobs under gVisor with compatibility enabled; both should complete using their jail profiles. With compatibility disabled and the probe failing, a worker explicitly configured for nsjail must reject DuckDB execution.

rubenfiszel and others added 2 commits October 18, 2026 12:30
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rubenfiszel

Copy link
Copy Markdown
Contributor Author

/review

@claude

claude Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @rubenfiszel's task in 1m 11s —— View job


Good to merge

Re-review after da67494 and cf220c2

  • Read the new commits and compared them with the previous Claude and Codex reviews
  • Checked the DuckDB fail-closed fix against the other jailed executors
  • Checked the patch preamble and the Dockerfile wiring
  • Posted review

The Codex P1 (DuckDB fails open after a failed probe) is fixed. duckdb_runner::runner_mode() (backend/windmill-worker/src/duckdb_runner.rs:44) no longer checks NSJAIL_AVAILABLE. A sandboxed worker now always picks RunnerMode::Jailed. If nsjail can't build a jail, it exits with an error, so the job fails and never runs outside the jail. If the binary is missing, spawn_runner returns Could not start the DuckDB runner (Jailed). The other executors already worked this way: bash, python, bun and the rest decide to jail from is_sandboxing_enabled() alone, and use NSJAIL_AVAILABLE only for the #sandbox annotation and no_network checks. Workers whose sandboxing is off still pick Unshare or Plain as before.

One side effect of the fix, which matches the intent and isn't a bug: on a worker with DISABLE_NSJAIL=false and no nsjail binary, DuckDB jobs now fail instead of running unconfined. The worker already warns about this setup at startup (worker.rs:2899).

New unit test. compat_config_sets_each_field_once covers the case the previous review asked for. It runs run.bun.config.proto, which already contains clone_newcgroup: false, through compat_config twice, then checks that each field appears once and that the output ends with the two compat lines. That is the regression that would break every jailed job, since nsjail rejects a config that sets a scalar field twice.

Patch preamble (cf220c2). git apply skips any text before the first diff --git line. None of the preamble lines starts with --- , +++ or diff , so the patch still applies. .dockerignore doesn't exclude docker/, so COPY docker/nsjail-patches/ resolves from the repo-root build context.

I found no new P0, P1 or P2 issues. The earlier non-blocking note still applies: .devcontainer/Dockerfile builds nsjail without the patch. If someone sets NSJAIL_GVISOR_COMPAT=true there, nsjail rejects the gvisor_compat field and the probe fails closed, so nothing becomes unsafe.

AGENTS.md compliance

The new comment in runner_mode explains why the gate is absent, which is a constraint and not narration. The test's comment explains what would break. No issues.

Test coverage

  • Backend. The pure config rewrite now has a test. The probe, the DuckDB fail-closed path and the compat spawn path need a real nsjail and a real runtime, so no unit test can reasonably cover them. The live k3s matrix in the PR description is the right check for those.
  • Docker/CI. No automated tests are expected.
  • Manual checks still open before merge.
    • gVisor runs. With NSJAIL_GVISOR_COMPAT=true on a worker under runsc that has nsjail enabled, run a DuckDB script and a dbt job. Both should finish, with no CLONE_NEWCGROUP error in the job logs.
    • DuckDB fail-closed. Use a worker with DISABLE_NSJAIL=false whose probe fails, for example runsc with compat off. A DuckDB job there should fail with nsjail's error and not return query results.
    • Image build. Build one of the three images in full and confirm that git apply succeeds in the nsjail stage.

@github-actions

Copy link
Copy Markdown
Contributor

Codex Review

Good to merge

The prior DuckDB fail-open finding is resolved: runner_mode() now selects Jailed whenever sandboxing is required, independently of the probe result. I found no further actionable issues in the exact base-to-head range.

Test coverage

The added Rust unit test covers config-rewrite idempotence and duplicate-field removal. I compiled and ran it independently with rustc --test: 1 passed. There is no new automated coverage for probe execution or DuckDB’s fail-closed selection, and no worker integration tests were run in this review.

No automated tests are expected for the Docker/config wiring itself. git apply --check passed against the pinned nsjail commit. Full image builds and the author’s reported runtime matrix were not repeated.

Remaining manual verification: under gVisor with compatibility enabled, run DuckDB and dbt jobs, including dbt’s captured phases, and confirm they complete inside their jails. With compatibility disabled and the probe failing, confirm a worker configured for nsjail rejects DuckDB execution.

Build a full affected image to verify the final packaging and startup probe.

@github-actions

Copy link
Copy Markdown
Contributor

Pi Review

Good to merge.

I reviewed the cumulative diff (2b6b26734…cf220c211) against AGENTS.md and the shared policy, focusing on what changed since the earlier Claude/Codex passes. The one substantive finding already raised — DuckDB falling back to a weaker isolation mode after a failed nsjail probe — is correctly fixed in da6749429, and I found no new P0/P1/P2 issues.

Verified points:

  • DuckDB now fails closed. runner_mode() (duckdb_runner.rs:43) no longer gates Jailed on NSJAIL_AVAILABLE.is_some(). A worker with is_sandboxing_enabled() true but a failed probe now selects Jailed, and spawn_runner's Command::new(NSJAIL_PATH) spawn fails rather than running unconfined — matching bash/python, which never consulted NSJAIL_AVAILABLE for the general sandboxed path. The only other NSJAIL_AVAILABLE consumers (#sandbox checks, docker_v2, check_no_network_jailed) all fail closed.
  • Rewrite coverage is complete. The only .spawn() sites on nsjail commands are start_child_process (common.rs:1043), duckdb_runner::spawn_runner (duckdb_runner.rs:480) and dbt_executor::run_captured (dbt_executor.rs:4061); all three now call apply_nsjail_gvisor_compat. dbt_engine::run_tool (dbt_engine.rs:729) and ssh_executor_ee.rs:128 spawn non-nsjail provisioning/SSH commands. The guard get_program() == NSJAIL_PATH correctly skips non-nsjail commands.
  • Config path resolution matches nsjail. nsjail_compat::apply joins --config to cmd.get_current_dir(), which is exactly how nsjail resolves a relative config; every executor that passes a relative name (e.g. run.config.proto) sets current_dir(job_dir). Absolute paths (python {req}.config.proto, the dbt SandboxProfile) resolve through Path::join's absolute-path replacement. Configs are per-job (duckdb uses a per-call_id name; dbt's lives in a job-sibling dir), so no cross-job rewrite.
  • Rewrite is idempotent and safe. compat_config strips any existing clone_newcgroup/gvisor_compat line before appending, and the committed test pins single-emission. A spawn that missed the rewrite fails under gVisor rather than running with less isolation, so the safety property is fail-closed even in the missed-path case.
  • Probe. Runs /bin/true in a 0700 UUID-named temp dir that is removed on every exit path; Builder::create fails on pre-existing dirs so there is no symlink/TOCTOU exposure. setup vs start errors are separated, and DISABLE_NSJAIL gates info vs error logging, with the separate worker.rs:2899 error covering the "configured for nsjail but unavailable" case.
  • nsjail patch. With gvisor_compat=false behavior is upstream-identical; the securebits tolerance only fires when ruid == euid == suid == inside_id (no UID change), and the live test matrix confirms the UID-change case stays fatal.

Considered and dismissed

  • .devcontainer/Dockerfile builds unpatched nsjail (the three main Dockerfiles got the git apply, this one didn't). Dismissed: a devcontainer never runs under gVisor, and if NSJAIL_GVISOR_COMPAT=true were set there, unpatched nsjail rejects the unknown gvisor_compat field and the probe fails closed — non-functional, not unsafe. Already noted by a prior reviewer.
  • Probe hardcodes clone_newnet: false while no_network jobs use clone_newnet: true. Dismissed: on non-compat gVisor the probe already fails on CLONE_NEWCGROUP, and under compat the patch skips the link cache when no interface is moved, so no_network works; the probe never produces a false "available" that leads to a silent no-network escape.
  • probe_nsjail sets current_dir(&dir), which would misresolve a relative NSJAIL_PATH (e.g. ./nsjail). Dismissed: NSJAIL_PATH is expected to be absolute or a bare name on PATH, and the probe is the only place that changes cwd — speculative edge case.
  • Job-log header isolation_label (worker.rs:5085) still derives from is_sandboxing_enabled() alone, not NSJAIL_AVAILABLE. Dismissed: pre-existing code untouched by this diff; the server-facing job_isolation ping (worker_utils.rs:215/297) already gates on NSJAIL_AVAILABLE.is_some() and now reports accurately.
  • Blocking std::fs read/write in apply on async spawn paths. Dismissed: configs are tiny and this matches the existing write_file pattern throughout the executors; not an observable regression.
  • apply silently returns Ok when no --config arg is present, and only matches the separate --config <value> form. Dismissed: every nsjail spawn in the tree uses the separate form, and a missed rewrite fails closed under gVisor.
  • Patch dereferences nsjconf->uids[0].inside_id. Dismissed: upstream initNsFromChild already dereferences uids[0] for the subsequent setresuid, so the assumption predates the patch.

Test coverage

  • Backend (Rust). One committed unit test (nsjail_compat.rs) pins compat_config idempotency and single-field emission. The probe and compat spawn paths genuinely require a real nsjail and runtime, so the PR's live k3s/runc/runsc matrix is the right verification for those. The DuckDB fail-closed regression has no committed test; one would need a real (or shimmed) nsjail, which is why I don't block on it, but it is the highest-value guard the PR leaves unwritten.
  • Docker / config. No automated tests expected; the nsjail stage was built standalone with the same commands as the Dockerfiles.
  • Manual verification still open, per the PR body. Run a DuckDB script and a dbt job under released runsc with NSJAIL_GVISOR_COMPAT=true and nsjail enabled — both should complete with no CLONE_NEWCGROUP/netlink error in the job log, and a no_network dbt/DuckDB run should leave only lo. Also build one of the three images end-to-end to confirm the COPY docker/nsjail-patches/ build-context path and git apply succeed from a clean clone.

@rubenfiszel
rubenfiszel marked this pull request as ready for review October 18, 2026 23:33
@rubenfiszel
rubenfiszel merged commit 69d36f7 into main Oct 18, 2026
28 of 30 checks passed
@rubenfiszel
rubenfiszel deleted the nsjail-gvisor-compat branch October 18, 2026 23:33
@claude

claude Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@github-actions github-actions Bot locked and limited conversation to collaborators Oct 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant