Sandbox untrusted environment builds - #495
Open
tylerpotts wants to merge 26 commits into
Open
Conversation
Design for the core isolation fix of #445: scrubbed subprocess environments plus Landlock-based per-job filesystem and network confinement for untrusted pixi builds.
11 tasks covering config, the sandbox package, the Landlock shim, executor and pkgmgr integration, worker timeouts, the #445 containment acceptance test, and operator docs.
Add Confine(), which applies a Landlock ruleset to the calling process so that it is inherited across execve by the build command. Also narrows the read-only path set: /etc is no longer granted wholesale. config.go searches /etc/nebi/ for config.yaml, and both database.dsn and auth.jwt_secret are file-loadable, so a readable /etc handed build code the credentials the environment allowlist exists to strip. Only the TLS trust stores and a short list of resolver/user-lookup files are granted now, emitted as a new --allow-ro-file flag. Filesystem confinement is attempted at ABI v2 before falling back to v1 so the workspace can carry the "refer" right; without it Landlock denies renaming a file between two workspace subdirectories, which package managers do when staging a download into their cache.
The server rewrites build argv to "nebi sandbox-exec ... -- <real argv>"; this makes that subcommand exist. It confines itself with Landlock and then execve's the real command, exiting 125 when the sandbox cannot be established so the parent can tell a broken sandbox from a failed build. syscall.Exec does not exist on Windows and cmd/nebi builds for windows/amd64, so the exec step is split into _unix and _windows files.
The uint16 narrowing in Confine relied on config validation, but the shim parses --allow-port itself, so 70000 would have wrapped to 4464 and opened a port nobody named.
RestrictNet was skipped for an empty port list, so "allowed_ports: []" silently granted unrestricted network access instead of the fully offline build it reads as. The network rules are now always applied; zero rules under a v4 ruleset that handles bind and connect denies all TCP. Doing that exposed a second bug. The filesystem and network restrictions were two stacked Landlock domains, and Landlock denies reparenting a file across directories by default in every domain, with "refer" grantable only inside the domain that handles it. The stacked network domain therefore re-denied refer and cross-subdirectory renames in the workspace failed with EXDEV, which is exactly what package managers do when moving a staged download into their cache. Both halves now go into a single ruleset, tried at ABI v4, then v2 (filesystem plus refer, network unrestricted), then v1. Missing TCP support is reported as ErrNetworkUnrestricted and warns rather than failing, including in strict mode, since the filesystem is confined. The setup-failure hint no longer asserts the kernel is at fault. Verified on kernel 6.17: combined ruleset preserves renames and truncation, denies /etc/nebi/config.yaml, denies writes outside the workspace, allows connect only to listed ports, and denies all TCP when the list is empty.
Command rewrote HOME to <workspace>/.nebi-home and created both job-scoped directories in every mode, including off. That gained nothing there, since an unconfined build can already reach the real HOME, and cost two real things. For source == "local" workspaces GetWorkspacePath returns the user's own project directory, so the desktop app would have started dropping a multi-GB conda cache into people's project folders, untracked by any gitignore. And pixi and rattler read $HOME/.rattler/credentials.json and $HOME/.pixi, so the rewrite broke private-channel authentication. Local and desktop mode default to off, so both landed on exactly the users who opted out of sandboxing. Off mode now passes the parent's HOME and TMPDIR through the allowlist untouched and creates neither directory. Strict and permissive keep the job-scoped behaviour, since per-workspace cache isolation is the point.
…ve modes Runner.IsSetupFailure only reports exit code 125 as a sandbox failure when the sandbox is actually active. In off mode the shim never runs, so 125 can only be the build tool's own exit code, and reporting it as "build sandbox setup failed" sent operators after a sandbox that was never involved. The executor call sites now go through the method; the package-level function stays for callers with no runner in scope. NewRunner also skips the os.Executable lookup in off mode. selfPath is only read when building the shim argv, which off mode never does, so a resolution failure there stopped the executor from constructing, and the server from booting, over a value nothing would have used.
✅ Deploy Preview for nebi-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
NewRunner trusted os.Executable() blindly. When the running program is not the nebi binary the failure was catastrophic and silent: every build re-execs that program with "sandbox-exec ..." prepended, and Go's flag package stops parsing at the first non-flag argument, so a Go test binary does not reject the subcommand. It ignores it and runs its whole suite again, starting another server that does the same. That is a fork bomb, and it is what has been killing Backend / Test at the five minute mark with no output. An active-mode Runner now proves its target implements the shim before accepting it, by running "sandbox-exec --check": a fast path that exits 0 without applying a ruleset or exec'ing anything. Once at construction, not per command, and the error names the resolved path. Measured on the e2e binary with team-mode defaults: 0.31s to a legible startup failure naming /tmp/e2e_nofix2.test, no leftover processes. Two details the probe needs to be safe. It sets NEBI_SANDBOX_SHIM_PROBE, and any process that reaches the probe with that set refuses to probe further, which caps an accidental chain at depth one; a real shim exits at --check long before it could build a Runner. And it sets WaitDelay, because killing the target on timeout does not kill what the target already spawned, and those grandchildren hold the output pipe open. Without it a 30 second target blocked the probe for the full 30 seconds and the timeout bought nothing. A non-empty selfPath stays trusted: it is an in-package escape hatch for tests that never re-exec, and production passes "".
TestMain never set NEBI_MODE, so config.Load defaulted to team mode and resolved sandbox.mode to strict. The e2e binary is not the nebi binary, so it cannot re-exec itself as the shim. Set the mode explicitly instead of relying on a default that means something else here. Survey of the other places a test builds an executor from config: only cmd/nebi/e2e_test.go and cmd/nebi/bundle_api_e2e_test.go reach config.Load, both via server.Run. The latter sets NEBI_MODE=local, which resolves the sandbox to off, and now also inherits this setting. Every other site (internal/api, internal/service, internal/executor) builds a config.Config literal, where Sandbox.Mode is "" and NewLocalExecutor maps that to off, so none of them can construct an active Runner.
The local-mode bundle tests pointed NEBI_PACKAGE_MANAGER_PIXI_PATH at
/usr/bin/true, so `pixi lock` in the create job produced no pixi.lock.
TestE2E_BundlePublishViaAPI_LocalMode nevertheless passed, because the
package-listing path did not honor the configured pixi path: it called
pkgmgr.New("pixi"), which resolves the real pixi from PATH (installing
one if absent). Real `pixi list` writes pixi.lock as a side effect of
resolving the manifest, and that accidental file is what let the create
job's version snapshot succeed and the workspace be publishable.
Routing package listing through the executor made the configured path
authoritative, so the stub is now actually used, no lockfile is ever
produced, the snapshot fails with "failed to read pixi.lock" and publish
returns "Workspace has no versions to publish".
Honoring the configured binary is the correct behavior, so fix the test
rather than the product: give it a pixi stand-in that reproduces the one
side effect the server depends on, `pixi lock` writing pixi.lock. It
writes the file only when absent so a workspace seeded from an imported
bundle keeps the lockfile it shipped with, and only for `lock` so that
the `--version` probe, which runs in the server's working directory,
stays inert. This also removes the suite's hidden dependency on a real
pixi being installed.
These are local agent workflow artifacts, not project documentation. The package comment now points at the issue instead of the removed spec.
18 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference Issues or PRs
Part of #445
Companion work: #465 fixed the pixi argument injection, and nebari-dev/nebi-pack#45 hardens the Helm chart. This PR addresses the core finding those two did not: untrusted build code executing next to credentials and other tenants' files.
What does this implement/fix?
Summary
Environment builds execute untrusted code: a
pixi.tomlcan declare source or build backends that run arbitrary commands during a solve or install. Today those commands inherit the full server environment and can reach every tenant's files. This PR closes both halves at the process level.Environment scrubbing, in every mode on every platform. Build subprocesses receive only an allowlist (
PATH,HOME,TMPDIR, locale, TLS trust, proxy variables).NEBI_DATABASE_DSN,NEBI_AUTH_JWT_SECRET, queue addresses, and registry credentials never reach build code. Previously noexec.Cmdininternal/executororinternal/pkgmgrsetcmd.Envat all.Landlock confinement, on Linux. A hidden
nebi sandbox-execsubcommand applies a Landlock ruleset to itself and thensyscall.Execs the real build command, so pixi and everything it spawns inherit the confinement. A build can read and write only its own workspace directory, read a specific set of system paths, and open TCP connections only to configured ports. The database and queue become unreachable from build code even though they share the pod network.Confinement needs no extra privileges. It works with
runAsNonRoot, all capabilities dropped, and no user namespaces, which is what makes it compatible with the chart hardening in nebari-dev/nebi-pack#45.Configuration
NEBI_SANDBOX_MODE,NEBI_SANDBOX_ALLOWED_PORTS,NEBI_SANDBOX_BUILD_TIMEOUT. Mode defaults tostrictin team mode andoffin local/desktop mode.strictNEBI_SANDBOX_MODEpermissiveoffKernel requirements
EXDEVrefer, which package managers need to stage downloads. Practical minimumBelow v4 the network is unrestricted with a warning rather than a build failure, even in strict mode.
Two operator traps, both documented
allowed_ports: []denies all TCP, it does not mean unrestricted. Note that the environment variable cannot express this: viper treats an empty env var as unset, soNEBI_SANDBOX_ALLOWED_PORTS=""silently yields the default. Use the YAML form.permissiveoroff.Notable things found while building this
Three of these were caught only because the implementation was verified against a real kernel rather than reasoned about:
referis granted, andrefercan only be granted in the domain that handles it. A filesystem ruleset followed by a stacked network ruleset therefore re-denies renames regardless of what the filesystem domain allowed. Package managers rename staged downloads into their cache constantly, so this would have broken every Linux build.Confineis now a single combined ruleset with a v4 → v2 → v1 ladder. Note for anyone testing this: GNUmvfalls back to copy-and-unlink onEXDEVand reports success, so only a rawrename(2)exposes it.pixi listcalls bypassed the executor entirely (syncPackagesFromDisk,SyncPackagesFromWorkspace,CreateVersionSnapshot), running against user-controlled workspaces with the full server environment. Closed by addingListPackagesto theExecutorinterface, which also fixes a pre-existing bug where those paths ignoredconfig.PackageManager.PixiPath./etcwould have re-exposed the secrets the allowlist removes, since nebi searches/etc/nebi/forconfig.yamland bothdatabase.dsnandauth.jwt_secretare file-loadable. Only the TLS trust stores and specific resolver files are granted.config.package_manager.pixi_pathwas being ignored by package listing and version snapshots, which resolved pixi from PATH instead and would silently auto-install one if absent. Routing those through the executor fixed it. This surfaced as an e2e test failure: that test only passed on main because the real pixi it accidentally invoked writespixi.lockas a side effect ofpixi list, which the version snapshot then read. The test was asserting on a bug.uint16(port), so70000would have opened port 4464 while the intended port stayed closed. Validated at both the config and ruleset layers.Testing
Unit tests cover the environment allowlist, argv construction, mode resolution and validation, HOME scoping on both sides of the active/off branch, setup-failure detection, and the Landlock rule assembly.
internal/sandbox/confine_test.gois the acceptance test issue 445 asks for. It runs a deliberately malicious build backend (testdata/probe) confined and asserts it cannot read the planted DSN from the environment, cannot read or write a sibling workspace, cannot read/etcoutside the allowlist, and cannot connect to a stand-in database port, while a legitimate build in its own workspace succeeds and cross-subdirectory renames still work. Negative cases assert on the specific errno, so a missing file or broken binary cannot masquerade as containment.The test is
//go:build linuxand skips when the kernel reports no Landlock support. It is confirmed enforcing in CI, not skipping: the run logskernel Landlock ABI version 7and all eight subtests pass, including the positive controls (a legitimate write and a cross-subdirectory rename succeed, and an allowed port connects) that stop the negative cases from passing vacuously.Two bugs this work surfaced in existing code
Neither is fixed here, and both predate this branch.
A real race in the create-job API contract.
internal/worker/worker.gosets the workspace status toreadybefore callingCreateVersionSnapshot. A client that polls untilreadyand immediately publishes can legitimately get400 Workspace has no versions to publish. The 250ms poll interval in the e2e test hides it. Moving the snapshot above the status flip looks like a three-line fix.Snapshot failures are swallowed. When
CreateVersionSnapshotfails it only logs. The workspace still goesreadywith zero versions, and the user learns nothing until a later publish fails with a message that does not name the real cause.Separately,
cmd/nebi's e2e suite still shells out to the real pixi in most tests, which makes it slow and dependent on PATH and the network.Not in scope
These remain open under issue 445 and should keep it open after this merges:
runningforever, which is pre-existing.exec.CommandContextsignals only the direct child, so pixi's grandchildren can outlive a timeout. They stay Landlock-confined, so this is resource cleanup rather than containment, and the fix needs a platform split for the Windows desktop build.HOMEis redirected when the sandbox is active,$HOME/.rattler/credentials.jsondoes not reach confined builds. Teams using authenticated channels needofftoday. The right fix is a per-job credential file scoped to the workspace's declared channels, never environment inheritance.Documentation
config.yaml.exampledocuments every key, anddocs/docs/server-setup.mdgains a "Build Sandbox" section covering the two layers, the mode and ABI tables, both operator traps, the per-workspace cache trade-off, and the known limitations.Access-centered content checklist
Text styling
H1or#in markdown).Non-text content