Repository navigation
Keep a namespace box's environment off every command line - #163
Conversation
Namespace boxes passed each variable to bwrap as --setenv KEY VALUE on its argv, which /proc/<pid>/cmdline shows to every process of the user, and which an SSH host also carries inside the remote shell command. The --setenv list now reaches bwrap on its stdin (bwrap --args 0), which every host forwards and namespace exec otherwise leaves empty. A NUL in a key or value is refused before anything runs.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Tiny Sweeper reviewReviewed head: FindingsNo active actionable findings. Resolved this pass
Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): Keep a namespace box's environment off every command line. The change stops `NamespaceSandbox` from passing environment variables to bwrap as `--setenv KEY VALUE` on its argv, which `/proc/<pid>/cmdline` exposes to every process of the same user and which, on an SSH host, also travels inside the remote shell command. The argv instead carries `--clearenv` plus `--args 0` only when there is something to set, and bwrap reads NUL-terminated `--setenv` records from stdin, box environment first and the request's on top, matching other backends. A key or value containing a NUL is refused with `Error::Backend` before anything spawns, since a NUL would end an entry early and let the remainder be parsed as further bwrap options. Because the environment payload occupies the command's stdin, `exec` branches on the environment payload and the caller's `request.stdin`: both present is refused with `Error::Backend` rather than dropping one, a caller stdin with no environment is forwarded through, and neither case leaves the request untouched. The commits lane answered with no sensitive content in the committed files. The latest-push-delta section is separately labeled and unavailable: review state has the prior reviewed head and rendered evidence but no trusted base-to-head diff comparison, so no push delta is described. Review snapshot
Completeness: Complete What changedEnvironment variables no longer travel to bwrap on its argv: `bwrap` emits `--clearenv` and, only when the box or request holds variables, `--args 0`, while the new `args::environment` helper serializes the entries as NUL-terminated `--setenv` records with the box's environment first, returning `None` when there is nothing to set and rejecting NUL-containing keys or values with `Error::Backend` before the host is invoked. `NamespaceSandbox`'s `exec` builds the fresh `ExecRequest` from the argv and matches on the pair of the environment payload and `request.stdin`: both present is refused with `Error::Backend`, an environment alone is attached via `with_stdin`, a caller stdin alone is forwarded via `with_stdin`, and neither leaves the request unchanged. The helper's rustdoc documents that the stdin slot is consumed, that caller stdin is only forwarded to a command with no environment, and that a command with both is refused. No API change is introduced; a NUL entry now fails at validation rather than at spawn. Features
TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Before mergeNone. Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
tinysweeper
Bot
added
the
priority: p3
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
tinysweeper
Bot
added
priority: p1
…onment Namespace exec dropped the caller's stdin (it always had), and with the environment now on bwrap's stdin it could not carry both. A command with no environment now gets the caller's stdin; one with both is refused before it runs, rather than either being silently dropped.
tinysweeper
Bot
added
priority: p3
Summary
A namespace box (
tinybox-linuxNamespaceSandbox) passed each environment variable to bwrap as--setenv KEY VALUEon its argv./proc/<pid>/cmdlineshows that to every process of the same user. On an SSH host, the whole argv also travels inside the remote shell command, so the values sit on the localsshcommand line and on the remote shell's.The
--setenvlist now reaches bwrap on its stdin, NUL-separated, throughbwrap --args 0.This is the namespace-box counterpart of #155, which does the same for the 2.1 native resource jail.
Related issue
None filed. Found while reviewing #155.
API or behavior changes
args::environment.Error::Backend. Before, it failed at spawn.Validation
On our Linux CI runner (bubblewrap 0.9.0), at 7a3e783ca8. The head differs only by applying
cargo fmt's patch:cargo clippy --all-targets --all-features -- -D warnings: 0 errors, same as main.cargo buildandcargo doc: ok.cargo test -p tinybox-linux sandbox: 42 passed.TINYBOX_LIVE_NAMESPACES=1 cargo test -p tinybox-linux --test live_namespaces: 15 passed, all of them run, none returned early.cargo test --workspace --no-fail-fast: 1578 passed, 0 failed, with no failures unique to this branch.--setenvloop back on the argv fails both the unit test (the argv carries--setenv) and the live test (the value appears on thebwrapcommand line).Tests
the_environment_is_cleared_and_then_rebuilt: the argv has--clearenvand--args 0and no value; the stdin holds--setenv FROM_BOX 1 --setenv FROM_COMMAND 2, the box's first.without_an_environment_bwrap_reads_no_arguments_from_stdin.a_nul_in_an_environment_value_is_refused_before_anything_runs.live_an_environment_value_is_on_no_command_line_while_the_box_runsscans every/proc/*/cmdlinewhile a box command runs, then checks that the command still received the value. The existinglive_the_callers_environment_does_not_leak_instill passes.Documentation
Rustdoc on
args::environmentand the comment inbwrap().Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the description