103 lines
4.6 KiB
Markdown
103 lines
4.6 KiB
Markdown
# PR#17: The sandbox's environment no longer stands on the command line of bwrap, nor in a file others can read
|
|
|
|
> **WARNING:** This document describes only the change applied in this PR.
|
|
> It may already be outdated once the next PR is merged.
|
|
> Historic PR documentation is not maintained along with new PRs; treat it as a snapshot.
|
|
|
|
## Related Links
|
|
|
|
- [werkator PR#166](https://git.javagil.de/mi/werkator/pulls/166):
|
|
Werkator passes the values of a build's variables to werkdock through its environment.
|
|
|
|
## Problem
|
|
|
|
werkdock passes each variable of the sandbox as `bwrap --setenv KEY VALUE`.
|
|
So every secret of a build stands in the argv of bwrap, e.g. the tokens of werkrevisor's ai-review build on mih09.
|
|
Each process that can read `/proc/<pid>/cmdline` sees them, and so does each `ps`, crash report or log of an argv.
|
|
On 2026-10-20 they leaked from `pgrep -af` on mih09 into a transcript.
|
|
|
|
Werkator passes the values to `werkdock run` through its environment since werkator PR#166, as a bare `-e KEY`;
|
|
bwrap's argv is the last place where they stand.
|
|
|
|
A second leak, found on mih09 the same day: each instance, also of an attached `run --rm`,
|
|
writes its configuration with the whole environment to `instances/<id>/config.json`, mode 0644, in a directory of mode 0755.
|
|
The other accounts of the group `mih09` could read the four secrets of a running ai-review build there.
|
|
|
|
## Non-Goals
|
|
|
|
- No change of what the sandbox sees:
|
|
the image's variables, the `-e` values and werkdock's `HOME` and `PATH`, nothing of the caller's environment.
|
|
- No change of the instance layout; the files only change their mode.
|
|
`config.json` keeps the environment in plain text, since `start` of a stopped instance needs it.
|
|
With mode 0600 only the account reads it, and that account holds the secrets anyway, e.g. in Werkator's configuration.
|
|
|
|
## Specification
|
|
|
|
### Feature: the environment of a sandbox reaches it off the command line
|
|
|
|
#### Scenario#17.01: bwrap's command line holds no value of the sandbox's environment
|
|
|
|
… so that no `ps` and no log of an argv shows a secret of a build.
|
|
|
|
- **Given** a sandbox with the variable `API_KEY` and a secret value
|
|
- **When** werkdock starts it
|
|
- **Then** `/proc/<pid>/cmdline` of bwrap holds neither the value nor a `--setenv`
|
|
- **and** the command in the sandbox sees the value
|
|
|
|
##### Verified by
|
|
|
|
- [TestArgvHoldsNoValueOfTheEnvironment](../../internal/engine/bwrap_test.go)
|
|
- [TestStartSetsTheEnvironmentOffTheCommandLine](../../internal/engine/bwrap_test.go)
|
|
|
|
#### Scenario#17.02: a large value reaches the sandbox whole
|
|
|
|
… so that a whole configuration file in a variable still works.
|
|
|
|
- **Given** a variable of 100,000 bytes
|
|
- **When** werkdock starts the sandbox
|
|
- **Then** the command sees all 100,000 bytes
|
|
- **and** holds no open descriptor of the pipe
|
|
|
|
##### Verified by
|
|
|
|
- [TestStartSetsTheEnvironmentOffTheCommandLine](../../internal/engine/bwrap_test.go)
|
|
|
|
#### Scenario#17.03: an instance is readable by its owner only
|
|
|
|
… so that no other account reads the secrets of a build from its configuration or its logs.
|
|
|
|
- **Given** an instance whose environment holds a secret
|
|
- **When** werkdock creates and runs it
|
|
- **Then** its directory has mode 0700
|
|
- **and** `config.json`, `state.json` and the logs have mode 0600
|
|
|
|
##### Verified by
|
|
|
|
- [TestAnInstanceIsReadableByItsOwnerOnly](../../internal/instance/instance_test.go)
|
|
|
|
## Solution
|
|
|
|
- `Argv` ends the environment with `--clearenv --args 4`; bwrap reads the `--setenv` arguments, NUL-separated, from fd 4.
|
|
`EnvArgs` writes them; `Start` passes a pipe as fd 4 and writes into it while bwrap reads.
|
|
bwrap has had `--args FD` since 0.1.x; mih09 and the workstation have 0.11.
|
|
- A NUL byte in a key or a value is refused: it would split the value into further bwrap arguments.
|
|
- Not chosen: bwrap's own environment set to the sandbox's variables, as the task proposed.
|
|
bwrap runs on the host, so an `LD_PRELOAD`, `LD_LIBRARY_PATH` or `GCONV_PATH` meant for the sandbox
|
|
would load a library of the build into bwrap, outside the sandbox.
|
|
`--args` keeps the semantics of `--clearenv` and `--setenv` exactly.
|
|
- An instance directory is created with mode 0700, its JSON files and logs with 0600.
|
|
The `instances/` directory itself stays as it is; each instance below it is closed.
|
|
An instance created before 0.13.2 keeps its modes until it is removed; Werkator's builds remove theirs (`--rm`).
|
|
- Red/green: with `--setenv` back in `Argv`, both tests of Scenario#17.01 fail;
|
|
with the old modes, Scenario#17.03 fails for each of its five paths.
|
|
- Version 0.13.2 with a section in `CHANGELOG.md`.
|
|
- `docs/risk-analysis.md` names the leak and its measure.
|
|
|
|
## Open Questions
|
|
|
|
None.
|
|
|
|
## Additional Changes
|
|
|
|
None.
|