The values stood in the argv of werkdock run and docker run, so every
ps showed the secrets of a build. A name the runtime or the host's
loader reads itself keeps KEY=VALUE on the command line, so a build
cannot steer the CLI through its environment.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-code
changed title from WIP: The values of a build's variables no longer stand on the command line of werkdock and docker to The values of a build's variables no longer stand on the command line of werkdock and docker (v1.43.6)2026-10-20 17:20:36 +02:00
hint writing rules violated in the changed docs: .claude/skills/architecture/SKILL.md:115: more than one sentence on a line; .claude/skills/architecture/SKILL.md:115: line longer than 400 characters (1881)
hint 5 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev
hint 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.01)
hint 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html
hint 3 finding(s) of low severity or rated minor by Jev: src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:342-352, src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:325-340, src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:380-403
hint model recommends changes before merge
hint model rates the risk as high (advisory: Jev judged every changed file)
Findings
Test Assertion Missing Variable at src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:342-352, severity unknown, counted as medium, hint, as a line comment
Weakened Test at src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:325-340, severity unknown, counted as medium, hint, as a line comment
Real Subprocess in Unit Test at src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:380-403, severity unknown, counted as medium, hint, as a line comment
9 of 11 units passed all questions (1 PR document(s)).
2 answers ignored because of a confidence below [classifier] min_confidence.
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 57167, calls 1, tokens in 54803 / out 32512 (reasoning included in out), duration 365.4 s, cost 0.0554 USD (reported), classifier `jev-1.13.0` calls 11, tokens in 9047, cost 0.0004 USD, finding judge `jev-1.13.0` calls 3, tokens in 8083, cost 0.0003 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32438 / out 263, cost 0.0042 USD (reported), build file judge `jev-1.13.0` calls 6, tokens in 9112, cost 0.0004 USD, phases: path_judge 0.5 s, classifier 0.7 s, context 4.6 s, model_review 358.4 s, finding_judge 0.4 s, config: defaults + host + repository (.werkrevisor.toml at main 512150c).
Reviewed head `319eb70da5071852a4f2d945fa5088622c1bd55a` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.54.1.
<!-- werkrevisor:319eb70da5071852a4f2d945fa5088622c1bd55a -->
<!-- werkrevisor-verdict:YELLOW -->
<!-- werkrevisor-blocks:afff7d01e991d4e9139b66351919b6e0,d4f7f764648242d618250fafe84f9bfd,e336a283dab975b343b4c6e924235378,0ab75f6af82b5a4b6255ebb22ba64f4c,3a0a51ead82263c727e33c8c2976b773,9b74126b5dfe3014243dc001a3c426d4,efe2a7d5fb21b63f8114dcc8a94424f7,0d84afc5a36d5a3bb8a5c1fd56f5a6ee,aea3626d44633ea8be6edd31b0158478,77a6e809410d790a65430f5d5ca348ef,ba4b1277c479c15e5ccd9b20f1487089,ec91dcef7c74a6fa2fbfed4f275a4e52,823a13932b5f720975c1ceb0d54b462f,b27df6c88279704ddb1c2aca78304da6,79ac6bd62d89a8506fd217b220f0730d,31b42dfda3c31d8a7753d7310f81a386,225232d74581cb74a649f805721f3e3e,6e6a16d44b45931997e8053d6fbf3edc,2927934c07c16a67c41c09d77512ca31,18c72334106ac2e9f73beb09aeb38178,deb9e60f8c6867471ed175b98e86b69b,ee246eba28172e8a5d13d59d6905fd8a,63c51e8ede7ddd461539fa036e99abdb,d506bb7ff9e1b4564906f87ffffea027,58b049ce5983a1ec51b2b485169fe70b,b0db02f18b38c0d10e54f20a32a848ea,4bbc4464e650ba4e5649c50c061e33ed,d1014ca3ccd40d55293e6403dbed3c9c,522539dd6acea0d864bdf8ec23aa8c6b,3daa4ffe3c65adcfa7202bc96e93b8a2,f331d5dd5aaff4f467778920cb802ab1,cfe8a72bc210abfcbc4371accc2ffe0b,8830817a413978e6d852be0cba13fa5a,bb2caf522616fc81410fe2b3d779ca38,d218aea685a0b9f8e5496fcdbb59f33b,4f2e8cea40360a8f9fa104728a10fb0d,1a1991587d8c3d491c40d64c3ad204b8,1601f25714e03e24f9ec90a7ae26e671,716ac70c5b271973a1fd9a31e00fd953,6c1a7788b5d1a10d8fc6d4b1ae944601,8ed0ee9c93edab14396e0f0bb4045217,3b442528f70c299527c2578cd9827a56 -->
<!-- werkrevisor-folded -->
<details><summary>🟡 werkrevisor YELLOW for <code>319eb70</code>, superseded by <a href="http://git.javagil.de/mi/werkator/pulls/166#issuecomment-6803">the result for <code>00f7462</code></a></summary>
## 🟡 werkrevisor: YELLOW / HUMAN_REVIEW
Change kind: `mixed`.
### Reasons
- **YELLOW** risky paths changed: src/main/kotlin/de/javagil/werkator/build/WerkdockBuildRunner.kt (**/*Werkdock*), src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt (**/*Werkdock*)
- **YELLOW** spec: docs/prs/2026-10-20-PR#166-build-env-off-argv.md Scenario#166.02 hard_to_read (0.52 > yellow_above = 0.5)
- **YELLOW** spec: docs/prs/2026-10-20-PR#166-build-env-off-argv.md Scenario#166.03 hard_to_read (0.53 > yellow_above = 0.5)
- **hint** writing rules violated in the changed docs: .claude/skills/architecture/SKILL.md:115: more than one sentence on a line; .claude/skills/architecture/SKILL.md:115: line longer than 400 characters (1881)
- **hint** 5 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev
- **hint** 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.01)
- **hint** 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html
- **hint** 3 finding(s) of low severity or rated minor by Jev: src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:342-352, src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:325-340, src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:380-403
- **hint** model recommends changes before merge
- **hint** model rates the risk as high (advisory: Jev judged every changed file)
### Findings
1. **Test Assertion Missing Variable** at `src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:342-352`, severity unknown, counted as medium, hint, as a line comment
2. **Weakened Test** at `src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:325-340`, severity unknown, counted as medium, hint, as a line comment
3. **Real Subprocess in Unit Test** at `src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:380-403`, severity unknown, counted as medium, hint, as a line comment
### Classifier
| Unit | Question | Problem | Confidence | Verdict |
|---|---|---|---|---|
| docs/prs/2026-10-20-PR#166-build-env-off-argv.md Scenario#166.02 | hard_to_read | 0.52 | 0.52 | YELLOW |
| docs/prs/2026-10-20-PR#166-build-env-off-argv.md Scenario#166.03 | hard_to_read | 0.53 | 0.54 | YELLOW |
9 of 11 units passed all questions (1 PR document(s)).
2 answers ignored because of a confidence below [classifier] min_confidence.
<sub>
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 57167, calls 1, tokens in 54803 / out 32512 (reasoning included in out), duration 365.4 s, cost 0.0554 USD (reported), classifier `jev-1.13.0` calls 11, tokens in 9047, cost 0.0004 USD, finding judge `jev-1.13.0` calls 3, tokens in 8083, cost 0.0003 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32438 / out 263, cost 0.0042 USD (reported), build file judge `jev-1.13.0` calls 6, tokens in 9112, cost 0.0004 USD, phases: path_judge 0.5 s, classifier 0.7 s, context 4.6 s, model_review 358.4 s, finding_judge 0.4 s, config: defaults + host + repository (.werkrevisor.toml at main 512150c).<br>
Reviewed head `319eb70da5071852a4f2d945fa5088622c1bd55a` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.54.1.
</sub>
</details>
werkrevisor: YELLOW / HUMAN_REVIEW for 319eb70da5071852a4f2d945fa5088622c1bd55a.
The result comment of werkrevisor holds the details.
werkrevisor: YELLOW / HUMAN_REVIEW for `319eb70da5071852a4f2d945fa5088622c1bd55a`.
The result comment of werkrevisor holds the details.
<!-- werkrevisor-review:319eb70da5071852a4f2d945fa5088622c1bd55a -->
On context close (e.g. systemd SIGTERM), a `ContextClosedEvent` listener in `BuildExecutor` terminates the process trees of all executing builds and waits (bounded) until their results are persisted as INTERRUPTED — a shutdown is never recorded as FAILED. Builds still queued stay PENDING and start no process. Both are re-enqueued by the watcher's startup recovery; INTERRUPTED therefore publishes as Gitea state `pending`, not `failure` (`GiteaStateMapping`).
The runtime is selected per build behind the `BuildRunner` interface: `DispatchingBuildRunner` (`@Primary`) routes to native `ProcessBuildRunner` (the default), to `DockerBuildRunner` when `docker.enabled`, or to `WerkdockBuildRunner` when `werkdock.enabled` — docker and werkdock are mutually exclusive per build and rejected in `buildSettings`, never picked silently. The Docker runner shells out to the `docker` CLI (no SDK): it (re)builds the configured image when the Dockerfile inputs changed (tracked via the `org.werkator.build-inputs-sha256` image label), maintains a per-repo Gradle cache volume, mounts the worktree and the Docker socket into a labelled (`org.hoennig.werkator`) `--rm --init` container (with `TESTCONTAINERS_HOST_OVERRIDE` from `docker.env` if it names one, else the child address a rootless daemon's rootlesskit reports for the `slirp4netns` port driver — `RootlessKitPorts`, asked over `dockerd-rootless/api.sock` on every build, PR#86 — else `localhost` under `network: host` and `host.docker.internal` otherwise), and repairs workspace ownership in-container after each command (under a rootless daemon the container runs as root, which is the host user, and the repair degenerates to `0:0`). Git works inside the container: the primary `.git` is mounted read-only with `.git/werkator/` masked by an empty tmpfs (credential isolation) and the worktree's admin dir mounted read-write (`gitMetadataMounts`). The returned `Process` is the attached `docker run` client, so log streaming and termination work exactly like native builds.
The runtime is selected per build behind the `BuildRunner` interface: `DispatchingBuildRunner` (`@Primary`) routes to native `ProcessBuildRunner` (the default), to `DockerBuildRunner` when `docker.enabled`, or to `WerkdockBuildRunner` when `werkdock.enabled` — docker and werkdock are mutually exclusive per build and rejected in `buildSettings`, never picked silently. The Docker runner shells out to the `docker` CLI (no SDK): it (re)builds the configured image when the Dockerfile inputs changed (tracked via the `org.werkator.build-inputs-sha256` image label), maintains a per-repo Gradle cache volume, mounts the worktree and the Docker socket into a labelled (`org.hoennig.werkator`) `--rm --init` container (with `TESTCONTAINERS_HOST_OVERRIDE` from `docker.env` if it names one, else the child address a rootless daemon's rootlesskit reports for the `slirp4netns` port driver — `RootlessKitPorts`, asked over `dockerd-rootless/api.sock` on every build, PR#86 — else `localhost` under `network: host` and `host.docker.internal` otherwise), and repairs workspace ownership in-container after each command (under a rootless daemon the container runs as root, which is the host user, and the repair degenerates to `0:0`). Git works inside the container: the primary `.git` is mounted read-only with `.git/werkator/` masked by an empty tmpfs (credential isolation) and the worktree's admin dir mounted read-write (`gitMetadataMounts`). The returned `Process` is the attached `docker run` client, so log streaming and termination work exactly like native builds. Both sandbox runners pass a build's variables as a bare `-e KEY`/`--env KEY` with the value in the CLI process's environment, never on its argv (`RuntimeEnvironment`, PR#166); that environment holds only the runtime's own variables from Werkator's, and a variable named like one of them keeps `KEY=VALUE` on the argv.
Weakened Test, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.77, severity 2.0 of 3
The "adds the sandbox env after the branch environment" test was changed from searching for the operands branch=main and FOO=bar (which incidentally verified the values "main" and "bar") to just the bare keys branch and FOO. The test no longer catches a wrong or missing value in the build's environment. The same strictness can be retained by additionally asserting capturedEnvironments.single()["branch"] shouldBe "main" and ["FOO"] shouldBe "bar".
**Weakened Test**, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.77, severity 2.0 of 3
The "adds the sandbox env after the branch environment" test was changed from searching for the operands `branch=main` and `FOO=bar` (which incidentally verified the values "main" and "bar") to just the bare keys `branch` and `FOO`. The test no longer catches a wrong or missing value in the build's environment. The same strictness can be retained by additionally asserting `capturedEnvironments.single()["branch"] shouldBe "main"` and `["FOO"] shouldBe "bar"`.
Test Assertion Missing Variable, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.44, severity 1.8 of 3
The new Scenario#166.01 test sets up two non-runtime build variables, GITEA_BOT_TOKEN and WERKREVISOR_CONFIG_TOML, but the expected environment HOST_RUNTIME_ENVIRONMENT + mapOf("branch" to "main", "GITEA_BOT_TOKEN" to "gitea-secret") only includes GITEA_BOT_TOKEN. Both variables are non-runtime names (WERKREVISOR_ is not in RUNTIME_PREFIXES, which only lists WERKDOCK_), so both travel in the processEnvironment, and the assertion will fail: the actual environment has six entries (PATH, HOME, WERKDOCK_HOME, branch, GITEA_BOT_TOKEN, WERKREVISOR_CONFIG_TOML) but the expected has five.
**Test Assertion Missing Variable**, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.44, severity 1.8 of 3
The new Scenario#166.01 test sets up two non-runtime build variables, GITEA_BOT_TOKEN and WERKREVISOR_CONFIG_TOML, but the expected environment `HOST_RUNTIME_ENVIRONMENT + mapOf("branch" to "main", "GITEA_BOT_TOKEN" to "gitea-secret")` only includes GITEA_BOT_TOKEN. Both variables are non-runtime names (WERKREVISOR_ is not in RUNTIME_PREFIXES, which only lists WERKDOCK_), so both travel in the processEnvironment, and the assertion will fail: the actual environment has six entries (PATH, HOME, WERKDOCK_HOME, branch, GITEA_BOT_TOKEN, WERKREVISOR_CONFIG_TOML) but the expected has five.
No change; the finding is wrong. The expected environment is HOST_RUNTIME_ENVIRONMENT + mapOf("branch" to "main") + secrets, and secrets holds both GITEA_BOT_TOKEN and WERKREVISOR_CONFIG_TOML. The test passes in CI. The map with only GITEA_BOT_TOKEN is the Docker test's, where the second entry, DOCKER_HOST, is a runtime name and stays on the command line.
No change; the finding is wrong. The expected environment is `HOST_RUNTIME_ENVIRONMENT + mapOf("branch" to "main") + secrets`, and `secrets` holds both GITEA_BOT_TOKEN and WERKREVISOR_CONFIG_TOML. The test passes in CI. The map with only GITEA_BOT_TOKEN is the Docker test's, where the second entry, DOCKER_HOST, is a runtime name and stays on the command line.
Real Subprocess in Unit Test, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.79, severity 2.0 of 3
The new Scenario#166.03 test "the build still sees the values its werkdock command line only names" starts the standInWerkdock shell script as a real subprocess, violating the *UnitTest convention (no real process). All other tests in the file use the mock processStarter; this test should either rely on an elaborated mock that simulates the werkdock operand resolution or be moved to an *IntegrationTest file.
**Real Subprocess in Unit Test**, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.79, severity 2.0 of 3
The new Scenario#166.03 test "the build still sees the values its werkdock command line only names" starts the standInWerkdock shell script as a real subprocess, violating the *UnitTest convention (no real process). All other tests in the file use the mock processStarter; this test should either rely on an elaborated mock that simulates the werkdock operand resolution or be moved to an *IntegrationTest file.
SSH_ASKPASS and SSH_ASKPASS_REQUIRE pass this filter and enter Docker's host-side environment. When the host uses an SSH Docker endpoint, Docker launches ssh with that environment. A branch can set SSH_ASKPASS to an executable in its worktree and SSH_ASKPASS_REQUIRE=force; if authentication requests a passphrase, SSH executes that file outside the sandbox as the Werkator user. The filter also removes the host's SSH_AUTH_SOCK, so an encrypted key normally unlocked through an agent can reach precisely this prompt, while ordinary agent-dependent builds fail authentication. These variables must remain host-controlled. The execution path follows Docker's SSH helper, its environment inheritance, and OpenSSH's documented environment handling.
Fix it, or reply why it does not hold, and resolve this comment.
**[high] Exclude SSH helper controls from branch-supplied host variables** (Codex review)
SSH_ASKPASS and SSH_ASKPASS_REQUIRE pass this filter and enter Docker's host-side environment. When the host uses an SSH Docker endpoint, Docker launches ssh with that environment. A branch can set SSH_ASKPASS to an executable in its worktree and SSH_ASKPASS_REQUIRE=force; if authentication requests a passphrase, SSH executes that file outside the sandbox as the Werkator user. The filter also removes the host's SSH_AUTH_SOCK, so an encrypted key normally unlocked through an agent can reach precisely this prompt, while ordinary agent-dependent builds fail authentication. These variables must remain host-controlled. The execution path follows [Docker's SSH helper](https://raw.githubusercontent.com/docker/cli/master/cli/connhelper/connhelper.go), its [environment inheritance](https://raw.githubusercontent.com/docker/cli/master/cli/connhelper/commandconn/commandconn.go), and [OpenSSH's documented environment handling](https://man.openbsd.org/ssh#ENVIRONMENT).
Fix it, or reply why it does not hold, and resolve this comment.
<!-- werkrevisor-codex -->
Fixed in d6e6683: every name starting with SSH_, SSL_ or OPENSSL_, and DISPLAY/WAYLAND_DISPLAY, belongs to the runtime now. A build value of such a name stays on the command line, and the docker client gets the host's SSH_AUTH_SOCK again. RuntimeEnvironmentUnitTest covers SSH_ASKPASS, SSH_AUTH_SOCK, DISPLAY, SSL_CERT_FILE and OPENSSL_CONF; docs/configuration.md names the reason.
Fixed in `d6e6683`: every name starting with `SSH_`, `SSL_` or `OPENSSL_`, and `DISPLAY`/`WAYLAND_DISPLAY`, belongs to the runtime now. A build value of such a name stays on the command line, and the docker client gets the host's `SSH_AUTH_SOCK` again. RuntimeEnvironmentUnitTest covers SSH_ASKPASS, SSH_AUTH_SOCK, DISPLAY, SSL_CERT_FILE and OPENSSL_CONF; docs/configuration.md names the reason.
This changes the security boundary between branch-controlled build variables and host-side runtime processes. An incomplete filter can execute branch-controlled code outside the sandbox, which the mocked runner tests do not detect.
Findings
[high] Exclude SSH helper controls from branch-supplied host variables at src/main/kotlin/de/javagil/werkator/build/RuntimeEnvironment.kt:74, as a line comment
The merge waits for the status ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.
Model `gpt-6-astra`, effort high, tokens in 1336433 (cached 1227904) / out 5492, 230.2 s, 16 commands; reviewed head `319eb70da5071852a4f2d945fa5088622c1bd55a`.
<!-- werkrevisor-codex:319eb70da5071852a4f2d945fa5088622c1bd55a -->
<!-- werkrevisor-codex-critical:true -->
<!-- werkrevisor-codex-unattached:0 -->
<!-- werkrevisor-codex -->
<!-- werkrevisor-folded -->
<details><summary>⚠️ werkrevisor ERROR for <code>?</code>, superseded by <a href="http://git.javagil.de/mi/werkator/pulls/166#issuecomment-6803">the result for <code>00f7462</code></a></summary>
## Codex review: critical
This changes the security boundary between branch-controlled build variables and host-side runtime processes. An incomplete filter can execute branch-controlled code outside the sandbox, which the mocked runner tests do not detect.
### Findings
1. **[high] Exclude SSH helper controls from branch-supplied host variables** at `src/main/kotlin/de/javagil/werkator/build/RuntimeEnvironment.kt:74`, as a line comment
The merge waits for the status `ai-review/codex`: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.
<sub>
Model `gpt-6-astra`, effort high, tokens in 1336433 (cached 1227904) / out 5492, 230.2 s, 16 commands; reviewed head `319eb70da5071852a4f2d945fa5088622c1bd55a`.
</sub>
</details>
YELLOW tests lost or disabled: src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:333 removes 5 test(s) or assertion(s) without a replacement
hint 5 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev
hint 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.01)
hint 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html
hint 1 finding(s) possibly severe, but Jev doubts that they are real: src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:325-383 (advisory: Jev judged every changed file)
hint model recommends merge with caution
Findings
Removed security-critical unit tests at src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:325-383, severity unknown, counted as medium, YELLOW, as a line comment
Classifier
11 of 11 units passed all questions (1 PR document(s)).
1 answer ignored because of a confidence below [classifier] min_confidence.
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 55898, calls 1, tokens in 53595 / out 48622 (reasoning included in out), duration 555.3 s, cost 0.0744 USD (reported), classifier `jev-1.13.0` calls 11, tokens in 8985, cost 0.0004 USD, finding judge `jev-1.13.0` calls 1, tokens in 3500, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32438 / out 181, cost 0.0004 USD (reported), build file judge `jev-1.13.0` calls 6, tokens in 9112, cost 0.0004 USD, phases: path_judge 0.5 s, classifier 0.8 s, context 3.5 s, model_review 549.6 s, finding_judge 0.3 s, config: defaults + host + repository (.werkrevisor.toml at main 512150c).
Reviewed head `00f746217af022b6311210cb48843b81d8523ffa` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.54.1.
<!-- werkrevisor:00f746217af022b6311210cb48843b81d8523ffa -->
<!-- werkrevisor-verdict:YELLOW -->
<!-- werkrevisor-blocks:e3cc49279ef455ea45b065a37bdc2a51,d4f7f764648242d618250fafe84f9bfd,e336a283dab975b343b4c6e924235378,f340fe7ba4772d99d69f55595a12816d,3a0a51ead82263c727e33c8c2976b773,9b74126b5dfe3014243dc001a3c426d4,efe2a7d5fb21b63f8114dcc8a94424f7,0d84afc5a36d5a3bb8a5c1fd56f5a6ee,aea3626d44633ea8be6edd31b0158478,77a6e809410d790a65430f5d5ca348ef,ba4b1277c479c15e5ccd9b20f1487089,ec91dcef7c74a6fa2fbfed4f275a4e52,823a13932b5f720975c1ceb0d54b462f,b27df6c88279704ddb1c2aca78304da6,79ac6bd62d89a8506fd217b220f0730d,31b42dfda3c31d8a7753d7310f81a386,225232d74581cb74a649f805721f3e3e,6e6a16d44b45931997e8053d6fbf3edc,2927934c07c16a67c41c09d77512ca31,18c72334106ac2e9f73beb09aeb38178,deb9e60f8c6867471ed175b98e86b69b,ee246eba28172e8a5d13d59d6905fd8a,63c51e8ede7ddd461539fa036e99abdb,d506bb7ff9e1b4564906f87ffffea027,58b049ce5983a1ec51b2b485169fe70b,b0db02f18b38c0d10e54f20a32a848ea,4bbc4464e650ba4e5649c50c061e33ed,d1014ca3ccd40d55293e6403dbed3c9c,522539dd6acea0d864bdf8ec23aa8c6b,3daa4ffe3c65adcfa7202bc96e93b8a2,f331d5dd5aaff4f467778920cb802ab1,cfe8a72bc210abfcbc4371accc2ffe0b,8830817a413978e6d852be0cba13fa5a,b4a4c778320ecb4dd0db7de8e1f703d2,bb2caf522616fc81410fe2b3d779ca38,d218aea685a0b9f8e5496fcdbb59f33b,4f2e8cea40360a8f9fa104728a10fb0d,1a1991587d8c3d491c40d64c3ad204b8,1601f25714e03e24f9ec90a7ae26e671,716ac70c5b271973a1fd9a31e00fd953,b05a5831af4b71e35a15849632d5d77a,e25f6366fd2959572fc48a3bda2e4579,34873e6f78d34a977a29d5f6af15e020,2f82dfd9031771a215787ffe01ad3d68,3c1a8b12955b205449c2ba869beb22ef,8ca3de8c306fd88c97a3aa062f575182,8c523e813adcba6ef029926e507b4cad,8b2530acd4227ac05d1962c80db855d2,3b442528f70c299527c2578cd9827a56 -->
<!-- werkrevisor-folded -->
<details><summary>🟡 werkrevisor YELLOW for <code>00f7462</code>, superseded by <a href="http://git.javagil.de/mi/werkator/pulls/166#issuecomment-6816">the result for <code>d6e6683</code></a></summary>
## 🟡 werkrevisor: YELLOW / HUMAN_REVIEW
Change kind: `mixed`.
### Reasons
- **YELLOW** tests lost or disabled: src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:333 removes 5 test(s) or assertion(s) without a replacement
- **YELLOW** risky paths changed: src/main/kotlin/de/javagil/werkator/build/WerkdockBuildRunner.kt (**/*Werkdock*), src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerIntegrationTest.kt (**/*Werkdock*), src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt (**/*Werkdock*)
- **hint** 5 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev
- **hint** 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.01)
- **hint** 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html
- **hint** 1 finding(s) possibly severe, but Jev doubts that they are real: src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:325-383 (advisory: Jev judged every changed file)
- **hint** model recommends merge with caution
### Findings
1. **Removed security-critical unit tests** at `src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:325-383`, severity unknown, counted as medium, YELLOW, as a line comment
### Classifier
11 of 11 units passed all questions (1 PR document(s)).
1 answer ignored because of a confidence below [classifier] min_confidence.
<sub>
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 55898, calls 1, tokens in 53595 / out 48622 (reasoning included in out), duration 555.3 s, cost 0.0744 USD (reported), classifier `jev-1.13.0` calls 11, tokens in 8985, cost 0.0004 USD, finding judge `jev-1.13.0` calls 1, tokens in 3500, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32438 / out 181, cost 0.0004 USD (reported), build file judge `jev-1.13.0` calls 6, tokens in 9112, cost 0.0004 USD, phases: path_judge 0.5 s, classifier 0.8 s, context 3.5 s, model_review 549.6 s, finding_judge 0.3 s, config: defaults + host + repository (.werkrevisor.toml at main 512150c).<br>
Reviewed head `00f746217af022b6311210cb48843b81d8523ffa` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.54.1.
</sub>
</details>
werkrevisor: YELLOW / HUMAN_REVIEW for 00f746217af022b6311210cb48843b81d8523ffa.
The result comment of werkrevisor holds the details.
werkrevisor: YELLOW / HUMAN_REVIEW for `00f746217af022b6311210cb48843b81d8523ffa`.
The result comment of werkrevisor holds the details.
<!-- werkrevisor-review:00f746217af022b6311210cb48843b81d8523ffa -->
Removed security-critical unit tests, severity unknown, counted as medium (werkrevisor)
YELLOW; Jev: real 0.51, severity 2.8 of 3
The PR removes five werkdock unit tests, and none of the scenarios they exercised is now covered by the surviving suite or by the new Scenario#166 cases:
"mounts the primary checkout read-only, the workspace read-write": asserts the committed primary checkout is mounted :ro, so a build cannot rewrite the .werkator.yml its successors' pinned settings are read from.
"exposes git metadata read-only with the werkator dir masked, in mount order": asserts .git is read-only and .git/werkator (machine config: git.token, control token, build state) is masked by an empty tmpfs.
"mounts no git metadata when the workspace is not a worktree": edge case for non-worktree workspaces.
"fails without a configured rootfs": input validation that werkdock.rootfs is set.
"downloads a URL rootfs once before importing it": URL-rootfs caching.
The first two are security invariants — a future regression that drops :ro on the primary checkout, or the .git/werkator tmpfs mask, would no longer be caught by any test. The other three cover edge cases (no-worktree workspaces, missing-rootfs validation, URL-rootfs download) that also have no replacement. A fix whose own security reasoning argues for keeping these invariants in place should not silently remove the tests that verify them.
**Removed security-critical unit tests**, severity unknown, counted as medium (werkrevisor)
YELLOW; Jev: real 0.51, severity 2.8 of 3
The PR removes five werkdock unit tests, and none of the scenarios they exercised is now covered by the surviving suite or by the new Scenario#166 cases:
- "mounts the primary checkout read-only, the workspace read-write": asserts the committed primary checkout is mounted `:ro`, so a build cannot rewrite the `.werkator.yml` its successors' pinned settings are read from.
- "exposes git metadata read-only with the werkator dir masked, in mount order": asserts `.git` is read-only and `.git/werkator` (machine config: `git.token`, control token, build state) is masked by an empty tmpfs.
- "mounts no git metadata when the workspace is not a worktree": edge case for non-worktree workspaces.
- "fails without a configured rootfs": input validation that `werkdock.rootfs` is set.
- "downloads a URL rootfs once before importing it": URL-rootfs caching.
The first two are security invariants — a future regression that drops `:ro` on the primary checkout, or the `.git/werkator` tmpfs mask, would no longer be caught by any test. The other three cover edge cases (no-worktree workspaces, missing-rootfs validation, URL-rootfs download) that also have no replacement. A fix whose own security reasoning argues for keeping these invariants in place should not silently remove the tests that verify them.
Right, and fixed in d6e6683: moving Scenario#166.03 into the integration test had cut the five tests after it by mistake. They are back unchanged; the spec has 23 tests, the 21 of main plus Scenario#166.01 and #166.02.
Right, and fixed in `d6e6683`: moving Scenario#166.03 into the integration test had cut the five tests after it by mistake. They are back unchanged; the spec has 23 tests, the 21 of main plus Scenario#166.01 and #166.02.
Moving Scenario#166.03 out had cut the tests after it. A docker client
that reaches its daemon over ssh:// starts ssh with its environment,
and SSH_ASKPASS makes ssh run a program on the host.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
[high] SSH control variables can execute branch code outside the sandbox (Codex review)
SSH_ASKPASS and SSH_ASKPASS_REQUIRE are not protected, so a branch's docker.env can place them in Docker's host-side environment. For an ssh:// Docker endpoint, Docker launches SSH with that environment. Setting SSH_ASKPASS to an executable in the build worktree and SSH_ASKPASS_REQUIRE=force executes it as the Werkator user whenever SSH requests a passphrase. Removing the host's SSH_AUTH_SOCK can itself trigger that prompt for an encrypted private key, and otherwise breaks agent-dependent authentication. These variables must retain host-controlled values rather than accepting build values. This follows from Docker's environment inheritance and OpenSSH's askpass execution.
Fix it, or reply why it does not hold, and resolve this comment.
**[high] SSH control variables can execute branch code outside the sandbox** (Codex review)
SSH_ASKPASS and SSH_ASKPASS_REQUIRE are not protected, so a branch's docker.env can place them in Docker's host-side environment. For an ssh:// Docker endpoint, Docker launches SSH with that environment. Setting SSH_ASKPASS to an executable in the build worktree and SSH_ASKPASS_REQUIRE=force executes it as the Werkator user whenever SSH requests a passphrase. Removing the host's SSH_AUTH_SOCK can itself trigger that prompt for an encrypted private key, and otherwise breaks agent-dependent authentication. These variables must retain host-controlled values rather than accepting build values. This follows from [Docker's environment inheritance](https://github.com/docker/cli/blob/v27.5.1/cli/connhelper/commandconn/commandconn.go) and [OpenSSH's askpass execution](https://github.com/openssh/openssh-portable/blob/master/readpass.c).
Fix it, or reply why it does not hold, and resolve this comment.
<!-- werkrevisor-codex -->
Fixed in d6e6683, the head after the reviewed 00f7462: every name starting with SSH_ belongs to the runtime, so SSH_ASKPASS and SSH_ASKPASS_REQUIRE of a build stay on the command line and never reach the docker client's environment, and the client gets the host's SSH_AUTH_SOCK again. DISPLAY, WAYLAND_DISPLAY, SSL_* and OPENSSL_* too. RuntimeEnvironmentUnitTest covers them.
Fixed in `d6e6683`, the head after the reviewed 00f7462: every name starting with `SSH_` belongs to the runtime, so SSH_ASKPASS and SSH_ASKPASS_REQUIRE of a build stay on the command line and never reach the docker client's environment, and the client gets the host's SSH_AUTH_SOCK again. DISPLAY, WAYLAND_DISPLAY, `SSL_*` and `OPENSSL_*` too. RuntimeEnvironmentUnitTest covers them.
[medium] Restore the removed git-metadata isolation tests (Codex review)
The replacement removes the test asserting read-only .git, the tmpfs mask over .git/werkator, and their ordering relative to writable mounts. No remaining werkdock test constructs the worktree metadata needed to exercise this path; the Docker tests cover a separate implementation. Consequently, removing the credential-hiding mask from WerkdockBuildRunner would now pass these tests. Preserve these security regression checks alongside the new environment tests.
Fix it, or reply why it does not hold, and resolve this comment.
**[medium] Restore the removed git-metadata isolation tests** (Codex review)
The replacement removes the test asserting read-only .git, the tmpfs mask over .git/werkator, and their ordering relative to writable mounts. No remaining werkdock test constructs the worktree metadata needed to exercise this path; the Docker tests cover a separate implementation. Consequently, removing the credential-hiding mask from WerkdockBuildRunner would now pass these tests. Preserve these security regression checks alongside the new environment tests.
Fix it, or reply why it does not hold, and resolve this comment.
<!-- werkrevisor-codex -->
Fixed in d6e6683: the five tests are back unchanged, among them "exposes git metadata read-only with the werkator dir masked, in mount order" and "mounts the primary checkout read-only, the workspace read-write".
Fixed in `d6e6683`: the five tests are back unchanged, among them "exposes git metadata read-only with the werkator dir masked, in mount order" and "mounts the primary checkout read-only, the workspace read-write".
This changes how branch-controlled variables reach host processes outside the sandbox; mistakes can permit host code execution or expose credentials. It also removes existing security-isolation tests.
Findings
[high] SSH control variables can execute branch code outside the sandbox at src/main/kotlin/de/javagil/werkator/build/RuntimeEnvironment.kt:68, as a line comment
[medium] Restore the removed git-metadata isolation tests at src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:356, as a line comment
The merge waits for the status ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.
Model `gpt-6-astra`, effort high, tokens in 682522 (cached 596736) / out 3782, 165.9 s, 13 commands; reviewed head `00f746217af022b6311210cb48843b81d8523ffa`.
<!-- werkrevisor-codex:00f746217af022b6311210cb48843b81d8523ffa -->
<!-- werkrevisor-codex-critical:true -->
<!-- werkrevisor-codex-unattached:0 -->
<!-- werkrevisor-codex -->
<!-- werkrevisor-folded -->
<details><summary>⚠️ werkrevisor ERROR for <code>?</code>, superseded by <a href="http://git.javagil.de/mi/werkator/pulls/166#issuecomment-6816">the result for <code>d6e6683</code></a></summary>
## Codex review: critical
This changes how branch-controlled variables reach host processes outside the sandbox; mistakes can permit host code execution or expose credentials. It also removes existing security-isolation tests.
### Findings
1. **[high] SSH control variables can execute branch code outside the sandbox** at `src/main/kotlin/de/javagil/werkator/build/RuntimeEnvironment.kt:68`, as a line comment
2. **[medium] Restore the removed git-metadata isolation tests** at `src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:356`, as a line comment
The merge waits for the status `ai-review/codex`: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.
<sub>
Model `gpt-6-astra`, effort high, tokens in 682522 (cached 596736) / out 3782, 165.9 s, 13 commands; reviewed head `00f746217af022b6311210cb48843b81d8523ffa`.
</sub>
</details>
hint 5 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev
hint 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.01)
hint 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html
hint the model review failed: TimeoutError: review exceeded 720 seconds (advisory: Jev judged every changed file)
Classifier
11 of 11 units passed all questions (1 PR document(s)).
2 answers ignored because of a confidence below [classifier] min_confidence.
Usage: model `openrouter/minimax/minimax-m3`, LLM failed (TimeoutError), tokens counted before the call 56695, duration 728.8 s, cost unknown, classifier `jev-1.13.0` calls 11, tokens in 9046, cost 0.0004 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32438 / out 207, cost 0.0004 USD (reported), build file judge `jev-1.13.0` calls 6, tokens in 9207, cost 0.0004 USD, phases: path_judge 0.5 s, classifier 0.7 s, context 4.0 s, model_review 723.1 s, config: defaults + host + repository (.werkrevisor.toml at main 512150c).
Reviewed head `d6e668319f0ac2e915ea78fb3eb5b230906a735b` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.54.1.
<!-- werkrevisor:d6e668319f0ac2e915ea78fb3eb5b230906a735b -->
<!-- werkrevisor-verdict:YELLOW -->
<!-- werkrevisor-blocks:e3cc49279ef455ea45b065a37bdc2a51,d4f7f764648242d618250fafe84f9bfd,ea543eb277b42c79a1e56034dc0043b1,6021eca34d104a66df78e9230521f10d,3a0a51ead82263c727e33c8c2976b773,9b74126b5dfe3014243dc001a3c426d4,efe2a7d5fb21b63f8114dcc8a94424f7,0d84afc5a36d5a3bb8a5c1fd56f5a6ee,aea3626d44633ea8be6edd31b0158478,77a6e809410d790a65430f5d5ca348ef,ba4b1277c479c15e5ccd9b20f1487089,ec91dcef7c74a6fa2fbfed4f275a4e52,8e876864aa669ea9071910f9853a8266,b27df6c88279704ddb1c2aca78304da6,79ac6bd62d89a8506fd217b220f0730d,31b42dfda3c31d8a7753d7310f81a386,225232d74581cb74a649f805721f3e3e,6e6a16d44b45931997e8053d6fbf3edc,2927934c07c16a67c41c09d77512ca31,18c72334106ac2e9f73beb09aeb38178,deb9e60f8c6867471ed175b98e86b69b,ee246eba28172e8a5d13d59d6905fd8a,63c51e8ede7ddd461539fa036e99abdb,d506bb7ff9e1b4564906f87ffffea027,58b049ce5983a1ec51b2b485169fe70b,b0db02f18b38c0d10e54f20a32a848ea,4bbc4464e650ba4e5649c50c061e33ed,d1014ca3ccd40d55293e6403dbed3c9c,522539dd6acea0d864bdf8ec23aa8c6b,3daa4ffe3c65adcfa7202bc96e93b8a2,f331d5dd5aaff4f467778920cb802ab1,cfe8a72bc210abfcbc4371accc2ffe0b,8e997a70d1f7c5180d741d6ffed97d03,b4a4c778320ecb4dd0db7de8e1f703d2,bb2caf522616fc81410fe2b3d779ca38,d218aea685a0b9f8e5496fcdbb59f33b,4f2e8cea40360a8f9fa104728a10fb0d,1a1991587d8c3d491c40d64c3ad204b8,1601f25714e03e24f9ec90a7ae26e671,716ac70c5b271973a1fd9a31e00fd953,b91a7ca1798ec2b105f6baeb5d35594b,3b442528f70c299527c2578cd9827a56 -->
<!-- werkrevisor-folded -->
<details><summary>🟡 werkrevisor YELLOW for <code>d6e6683</code>, superseded by <a href="http://git.javagil.de/mi/werkator/pulls/166#issuecomment-6937">the result for <code>425a99d</code></a></summary>
## 🟡 werkrevisor: YELLOW / HUMAN_REVIEW
Change kind: `mixed`.
### Reasons
- **YELLOW** risky paths changed: src/main/kotlin/de/javagil/werkator/build/WerkdockBuildRunner.kt (**/*Werkdock*), src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerIntegrationTest.kt (**/*Werkdock*), src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt (**/*Werkdock*)
- **hint** 5 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev
- **hint** 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.01)
- **hint** 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html
- **hint** the model review failed: TimeoutError: review exceeded 720 seconds (advisory: Jev judged every changed file)
### Classifier
11 of 11 units passed all questions (1 PR document(s)).
2 answers ignored because of a confidence below [classifier] min_confidence.
<sub>
Usage: model `openrouter/minimax/minimax-m3`, LLM failed (TimeoutError), tokens counted before the call 56695, duration 728.8 s, cost unknown, classifier `jev-1.13.0` calls 11, tokens in 9046, cost 0.0004 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32438 / out 207, cost 0.0004 USD (reported), build file judge `jev-1.13.0` calls 6, tokens in 9207, cost 0.0004 USD, phases: path_judge 0.5 s, classifier 0.7 s, context 4.0 s, model_review 723.1 s, config: defaults + host + repository (.werkrevisor.toml at main 512150c).<br>
Reviewed head `d6e668319f0ac2e915ea78fb3eb5b230906a735b` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.54.1.
</sub>
</details>
BASH_ENV passes this filter, so a branch can supply it through docker.env to Docker and its children. On a host using docker-credential-pass, the helper invokes the Bash-based pass program with that inherited environment, allowing branch-supplied startup code to execute as the Werkator user outside the container. This is reachable even with cached images when Docker 28 content trust is enabled: trusted-image resolution retrieves credentials, and the pass helper starts pass without clearing the environment. A harmless local check confirmed that BASH_ENV executes code when pass starts. Previously this value was only a container environment operand.
Fix it, or reply why it does not hold, and resolve this comment.
**[high] BASH_ENV permits code execution through host-side credential helpers** (Codex review)
BASH_ENV passes this filter, so a branch can supply it through docker.env to Docker and its children. On a host using docker-credential-pass, the helper invokes the Bash-based pass program with that inherited environment, allowing branch-supplied startup code to execute as the Werkator user outside the container. This is reachable even with cached images when Docker 28 content trust is enabled: [trusted-image resolution](https://github.com/docker/cli/blob/v28.5.1/cli/command/image/trust.go#L166-L172) retrieves credentials, and the [pass helper](https://github.com/docker/docker-credential-helpers/blob/master/pass/pass.go#L62-L68) starts pass without clearing the environment. A harmless local check confirmed that BASH_ENV executes code when pass starts. Previously this value was only a container environment operand.
Fix it, or reply why it does not hold, and resolve this comment.
<!-- werkrevisor-codex -->
Fixed in 425a99d, by a different route rather than a longer list: a Docker build's values no longer enter the docker client's environment at all. DockerEnvFile writes them to a 0600 file in .git/werkator/docker-env/ (0700, masked in the container) for docker run --env-file, removed when the client ends; the client keeps Werkator's own environment as before. A value with a line break, which docker's file format cannot carry, stays --env KEY=VALUE as before. werkdock keeps the environment route, since it starts no host program but itself and bwrap, and werkdock PR#18 starts bwrap with an empty environment. Scenario#166.04 covers it.
Fixed in `425a99d`, by a different route rather than a longer list: a Docker build's values no longer enter the docker client's environment at all. `DockerEnvFile` writes them to a 0600 file in `.git/werkator/docker-env/` (0700, masked in the container) for `docker run --env-file`, removed when the client ends; the client keeps Werkator's own environment as before. A value with a line break, which docker's file format cannot carry, stays `--env KEY=VALUE` as before. werkdock keeps the environment route, since it starts no host program but itself and bwrap, and werkdock PR#18 starts bwrap with an empty environment. Scenario#166.04 covers it.
This changes the security boundary between branch-controlled build variables and host processes that handle credentials. An incomplete environment filter can allow code execution outside the sandbox, which the current tests do not cover.
Findings
[high] BASH_ENV permits code execution through host-side credential helpers at src/main/kotlin/de/javagil/werkator/build/RuntimeEnvironment.kt:27, as a line comment
The merge waits for the status ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.
Model `gpt-6-astra`, effort high, tokens in 779535 (cached 691072) / out 3444, 165.9 s, 8 commands; reviewed head `d6e668319f0ac2e915ea78fb3eb5b230906a735b`.
<!-- werkrevisor-codex:d6e668319f0ac2e915ea78fb3eb5b230906a735b -->
<!-- werkrevisor-codex-critical:true -->
<!-- werkrevisor-codex-unattached:0 -->
<!-- werkrevisor-codex -->
<!-- werkrevisor-folded -->
<details><summary>⚠️ werkrevisor ERROR for <code>?</code>, superseded by <a href="http://git.javagil.de/mi/werkator/pulls/166#issuecomment-6937">the result for <code>425a99d</code></a></summary>
## Codex review: critical
This changes the security boundary between branch-controlled build variables and host processes that handle credentials. An incomplete environment filter can allow code execution outside the sandbox, which the current tests do not cover.
### Findings
1. **[high] BASH_ENV permits code execution through host-side credential helpers** at `src/main/kotlin/de/javagil/werkator/build/RuntimeEnvironment.kt:27`, as a line comment
The merge waits for the status `ai-review/codex`: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.
<sub>
Model `gpt-6-astra`, effort high, tokens in 779535 (cached 691072) / out 3444, 165.9 s, 8 commands; reviewed head `d6e668319f0ac2e915ea78fb3eb5b230906a735b`.
</sub>
</details>
The docker client starts ssh and credential helpers with its
environment, so a branch-chosen name such as BASH_ENV or SSH_ASKPASS
there runs code on the host; no list of names covers every helper.
werkdock keeps the environment route: it starts nothing but itself and
bwrap, which werkdock PR#18 starts with an empty environment.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hint 6 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev
hint 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.01)
hint 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html
[medium] Concurrent first builds can fail creating the shared directory (Codex review)
When two Docker builds first run for a repository, both can observe that docker-env is missing. SecretFiles.createDirectoriesOwnerOnly checks existence and then calls Files.createDirectory without handling FileAlreadyExistsException. One build therefore fails before launching its container. This is reachable through concurrent branches or the independent pull-request-target pool.
Fix it, or reply why it does not hold, and resolve this comment.
**[medium] Concurrent first builds can fail creating the shared directory** (Codex review)
When two Docker builds first run for a repository, both can observe that docker-env is missing. SecretFiles.createDirectoriesOwnerOnly checks existence and then calls Files.createDirectory without handling FileAlreadyExistsException. One build therefore fails before launching its container. This is reachable through concurrent branches or the independent pull-request-target pool.
Fix it, or reply why it does not hold, and resolve this comment.
<!-- werkrevisor-codex -->
Fixed in e9c77a3: SecretFiles.createDirectoriesOwnerOnly takes a directory that another thread created in the meantime and fails only if the path is no directory. DockerEnvFileUnitTest starts eight writes together on a fresh repository; without the fix it failed in three of three runs, with it it passes.
Fixed in `e9c77a3`: `SecretFiles.createDirectoriesOwnerOnly` takes a directory that another thread created in the meantime and fails only if the path is no directory. DockerEnvFileUnitTest starts eight writes together on a fresh repository; without the fix it failed in three of three runs, with it it passes.
[medium] Account for Docker’s env-file line-length limit (Codex review)
fits accepts arbitrarily long single-line values, but Docker’s parser uses a scanner with a 64 KiB token limit. A 70 KiB ASCII configuration value previously accepted through --env now makes docker run fail with 'token too long'. Check the encoded KEY=VALUE line length when selecting the file transport.
Fix it, or reply why it does not hold, and resolve this comment.
**[medium] Account for Docker’s env-file line-length limit** (Codex review)
fits accepts arbitrarily long single-line values, but [Docker’s parser](https://raw.githubusercontent.com/docker/cli/master/pkg/kvfile/kvfile.go) uses a scanner with a 64 KiB token limit. A 70 KiB ASCII configuration value previously accepted through --env now makes docker run fail with 'token too long'. Check the encoded KEY=VALUE line length when selecting the file transport.
Fix it, or reply why it does not hold, and resolve this comment.
<!-- werkrevisor-codex -->
Fixed in e9c77a3: fits also requires the UTF-8 line KEY=VALUE to stay below 64 KiB; a longer one stays --env KEY=VALUE on the command line, as before. DockerEnvFileUnitTest covers a 70 KiB value.
Fixed in `e9c77a3`: `fits` also requires the UTF-8 line `KEY=VALUE` to stay below 64 KiB; a longer one stays `--env KEY=VALUE` on the command line, as before. DockerEnvFileUnitTest covers a 70 KiB value.
The change handles build secrets and moves branch-controlled variables into a host process’s environment. Mistakes in filtering or file isolation could expose credentials or compromise execution outside the sandbox, so human review is required.
Findings
[medium] Concurrent first builds can fail creating the shared directory at src/main/kotlin/de/javagil/werkator/build/DockerEnvFile.kt:46, as a line comment
[medium] Account for Docker’s env-file line-length limit at src/main/kotlin/de/javagil/werkator/build/DockerEnvFile.kt:64, as a line comment
The merge waits for the status ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.
Model `gpt-6-astra`, effort high, tokens in 375117 (cached 265728) / out 2487, 89.2 s, 13 commands; reviewed head `425a99d8e602cbd0a148e681a2337e16d1cb5d70`.
<!-- werkrevisor-codex:425a99d8e602cbd0a148e681a2337e16d1cb5d70 -->
<!-- werkrevisor-codex-critical:true -->
<!-- werkrevisor-codex-unattached:0 -->
<!-- werkrevisor-codex -->
<!-- werkrevisor-folded -->
<details><summary>⚠️ werkrevisor ERROR for <code>?</code>, superseded by <a href="http://git.javagil.de/mi/werkator/pulls/166#issuecomment-6950">the result for <code>e9c77a3</code></a></summary>
## Codex review: critical
The change handles build secrets and moves branch-controlled variables into a host process’s environment. Mistakes in filtering or file isolation could expose credentials or compromise execution outside the sandbox, so human review is required.
### Findings
1. **[medium] Concurrent first builds can fail creating the shared directory** at `src/main/kotlin/de/javagil/werkator/build/DockerEnvFile.kt:46`, as a line comment
2. **[medium] Account for Docker’s env-file line-length limit** at `src/main/kotlin/de/javagil/werkator/build/DockerEnvFile.kt:64`, as a line comment
The merge waits for the status `ai-review/codex`: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.
<sub>
Model `gpt-6-astra`, effort high, tokens in 375117 (cached 265728) / out 2487, 89.2 s, 13 commands; reviewed head `425a99d8e602cbd0a148e681a2337e16d1cb5d70`.
</sub>
</details>
ai-review/assessment AI findings; a developer approval after checking them unlocks the merge; a human review request is pending; merge already scheduled
hint 8 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev
hint 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.01)
hint 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html
hint model recommends changes before merge
Findings
[medium] Value corruption at src/main/kotlin/de/javagil/werkator/build/DockerEnvFile.kt:64-68, ORANGE, as a line comment
Classifier
12 of 12 units passed all questions (1 PR document(s)).
2 answers ignored because of a confidence below [classifier] min_confidence.
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 62968, calls 1, tokens in 60394 / out 39980 (reasoning included in out), duration 283.4 s, cost 0.0661 USD (reported), classifier `jev-1.13.0` calls 12, tokens in 10442, cost 0.0004 USD, finding judge `jev-1.13.0` calls 1, tokens in 2131, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32506 / out 12, cost 0.0041 USD (reported), build file judge `jev-1.13.0` calls 9, tokens in 14684, cost 0.0006 USD, phases: path_judge 0.8 s, classifier 0.7 s, context 2.1 s, model_review 278.7 s, finding_judge 0.3 s, config: defaults + host + repository (.werkrevisor.toml at main 512150c).
Reviewed head `e9c77a3328838019ce16502b2a13ec9b35c8c39e` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.55.0.
<!-- werkrevisor:e9c77a3328838019ce16502b2a13ec9b35c8c39e -->
<!-- werkrevisor-verdict:ORANGE -->
<!-- werkrevisor-blocks:f154888fd6cf660f532962306b34525e,d4f7f764648242d618250fafe84f9bfd,7abc600c77d01859b9c1cec7f6f3102a,197aba4ad9e27e2a9f6d7087b361e775,3a0a51ead82263c727e33c8c2976b773,6cf1f7c22f3eeefc35a4ae2d9f61b6ab,30e0a1eac7577caa5d05a640e2f11259,61a0fc6c5b56281ca59c0562132a391b,af2d2afa5afcf682d5fb9249134b38f7,b65d1c7588d519b7122bb5973fab52a5,aea3626d44633ea8be6edd31b0158478,cf18f84404c6ee85ace34d87b0f5174c,f66797f40f09801482e1fa71f1505c18,5e0f74191143d55ed61f7a1fdca0c35c,d3a2bc67f3cfad662403786fc17e90b0,3c047ef85799d9bfb111fd0d7d26a3f6,9b2667e956bab12dab308cc547604851,b27df6c88279704ddb1c2aca78304da6,79ac6bd62d89a8506fd217b220f0730d,31b42dfda3c31d8a7753d7310f81a386,225232d74581cb74a649f805721f3e3e,6e6a16d44b45931997e8053d6fbf3edc,2927934c07c16a67c41c09d77512ca31,4ccc6eb4e30be2276c5d0f9c94465c77,f350df528b925b5f8e20b5714470c905,deb9e60f8c6867471ed175b98e86b69b,ee246eba28172e8a5d13d59d6905fd8a,0fcf9cb4aa8a089e2a426b36f8fbc7fe,a89d7ae85f89fba4871840b9e54b5eb3,a882e5385c6dc38c1a5149818328e4e4,80c18aa7560851d4634248c1ddb61b29,e8de4a3b7144c4abb832bf376384c7e8,805b7c5ac635a934aedadf0fc6007732,3043e9e48bc2288201a3fd81f20569fb,2350da2402872b3b92a45bc5e3d15e35,969e2ae8a466b0221fc5a86d3644e139,efceb8ef78c567520b0f511321fd2ebe,1e003b5cfbca400ff319107b06a7c889,5e0f68eb55aa512407c7ae38e35232ea,8e997a70d1f7c5180d741d6ffed97d03,b4a4c778320ecb4dd0db7de8e1f703d2,bb2caf522616fc81410fe2b3d779ca38,d218aea685a0b9f8e5496fcdbb59f33b,4f2e8cea40360a8f9fa104728a10fb0d,1a1991587d8c3d491c40d64c3ad204b8,1601f25714e03e24f9ec90a7ae26e671,716ac70c5b271973a1fd9a31e00fd953,b91a7ca1798ec2b105f6baeb5d35594b,3b442528f70c299527c2578cd9827a56 -->
## 🟠 werkrevisor: ORANGE / FINDINGS
Change kind: `mixed`.
### Reasons
- **ORANGE** 1 finding(s) that Jev rates as probably real and severe: src/main/kotlin/de/javagil/werkator/build/DockerEnvFile.kt:64-68
- **YELLOW** risky paths changed: src/main/kotlin/de/javagil/werkator/build/WerkdockBuildRunner.kt (**/*Werkdock*), src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerIntegrationTest.kt (**/*Werkdock*), src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt (**/*Werkdock*)
- **hint** 8 changed file(s) that neither weaken the checks, touch security nor show a risk category of jvm-spring-backend according to Jev
- **hint** 1 build change(s) of low risk according to Jev: build.gradle.kts (weakens checks 0.03, risk 0.00, security 0.01)
- **hint** 1 changed file was given to the AI as excerpt of the selected lines and 5 lines around each changed line, not in full (each larger than [review] max_full_file_chars = 80000): src/main/resources/templates/releases.html
- **hint** model recommends changes before merge
### Findings
1. **[medium] Value corruption** at `src/main/kotlin/de/javagil/werkator/build/DockerEnvFile.kt:64-68`, ORANGE, as a line comment
### Classifier
12 of 12 units passed all questions (1 PR document(s)).
2 answers ignored because of a confidence below [classifier] min_confidence.
<sub>
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 62968, calls 1, tokens in 60394 / out 39980 (reasoning included in out), duration 283.4 s, cost 0.0661 USD (reported), classifier `jev-1.13.0` calls 12, tokens in 10442, cost 0.0004 USD, finding judge `jev-1.13.0` calls 1, tokens in 2131, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32506 / out 12, cost 0.0041 USD (reported), build file judge `jev-1.13.0` calls 9, tokens in 14684, cost 0.0006 USD, phases: path_judge 0.8 s, classifier 0.7 s, context 2.1 s, model_review 278.7 s, finding_judge 0.3 s, config: defaults + host + repository (.werkrevisor.toml at main 512150c).<br>
Reviewed head `e9c77a3328838019ce16502b2a13ec9b35c8c39e` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.55.0.
</sub>
werkrevisor: ORANGE / FINDINGS for e9c77a3328838019ce16502b2a13ec9b35c8c39e.
The result comment of werkrevisor holds the details.
werkrevisor: ORANGE / FINDINGS for `e9c77a3328838019ce16502b2a13ec9b35c8c39e`.
The result comment of werkrevisor holds the details.
<!-- werkrevisor-review:e9c77a3328838019ce16502b2a13ec9b35c8c39e -->
fits() treats values without line breaks or NULs as round-trippable, but Docker's documented env-file parser interprets quotes and escape sequences. A literal value such as HOME="value" is therefore delivered as HOME=value, and an unterminated quoted value can make docker run reject the file; the previous HOME="value" argv form preserved it. Reject or encode values the parser cannot round-trip and add coverage for quoted inputs.
**[medium] Value corruption** (werkrevisor)
ORANGE; Jev: real 0.85, severity 3.0 of 3
`fits()` treats values without line breaks or NULs as round-trippable, but Docker's documented env-file parser interprets quotes and escape sequences. A literal value such as `HOME="value"` is therefore delivered as `HOME=value`, and an unterminated quoted value can make `docker run` reject the file; the previous `HOME="value"` argv form preserved it. Reject or encode values the parser cannot round-trip and add coverage for quoted inputs.
No change; the finding does not hold for docker run --env-file. docker/cli's parser (pkg/kvfile/kvfile.go, parseKeyValueFile) interprets neither quotes nor escapes: it trims leading whitespace of the line, skips empty lines and # comments, rejects a key with whitespace, and then appends the line as-is ("key/value pair is valid and has a value; add the line as-is"). So HOME=\"value\" arrives with its quotes, exactly as the former --env HOME=\"value\". Quote handling belongs to Compose's .env files, which this PR does not use. What the parser does change, a leading #, whitespace in the key, a line break and the 64 KiB line limit, fits keeps on the command line.
No change; the finding does not hold for `docker run --env-file`. docker/cli's parser (pkg/kvfile/kvfile.go, parseKeyValueFile) interprets neither quotes nor escapes: it trims leading whitespace of the line, skips empty lines and `#` comments, rejects a key with whitespace, and then appends the line as-is ("key/value pair is valid and has a value; add the line as-is"). So `HOME=\"value\"` arrives with its quotes, exactly as the former `--env HOME=\"value\"`. Quote handling belongs to Compose's .env files, which this PR does not use. What the parser does change, a leading `#`, whitespace in the key, a line break and the 64 KiB line limit, `fits` keeps on the command line.
This changes how build secrets and branch-controlled environment variables reach host-side sandbox processes. A mistake could expose credentials or allow execution outside the sandbox, and the mocked runtime tests do not establish that security boundary.
No findings.
The merge waits for the status ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.
Model `gpt-6-astra`, effort high, tokens in 692412 (cached 614784) / out 2581, 93.8 s, 16 commands; reviewed head `e9c77a3328838019ce16502b2a13ec9b35c8c39e`.
<!-- werkrevisor-codex:e9c77a3328838019ce16502b2a13ec9b35c8c39e -->
<!-- werkrevisor-codex-critical:true -->
<!-- werkrevisor-codex-unattached:0 -->
<!-- werkrevisor-codex -->
## Codex review: critical
This changes how build secrets and branch-controlled environment variables reach host-side sandbox processes. A mistake could expose credentials or allow execution outside the sandbox, and the mocked runtime tests do not establish that security boundary.
No findings.
The merge waits for the status `ai-review/codex`: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.
<sub>
Model `gpt-6-astra`, effort high, tokens in 692412 (cached 614784) / out 2581, 93.8 s, 16 commands; reviewed head `e9c77a3328838019ce16502b2a13ec9b35c8c39e`.
</sub>
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
See docs/prs/2026-10-20-PR#166-build-env-off-argv.md.
🤖 Generated with Claude Code
WIP: The values of a build's variables no longer stand on the command line of werkdock and dockerto The values of a build's variables no longer stand on the command line of werkdock and docker (v1.43.6)🟡 werkrevisor YELLOW for
319eb70, superseded by the result for00f7462🟡 werkrevisor: YELLOW / HUMAN_REVIEW
Change kind:
mixed.Reasons
Findings
Test Assertion Missing Variable at
src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:342-352, severity unknown, counted as medium, hint, as a line commentWeakened Test at
src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:325-340, severity unknown, counted as medium, hint, as a line commentReal Subprocess in Unit Test at
src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:380-403, severity unknown, counted as medium, hint, as a line commentClassifier
9 of 11 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 57167, calls 1, tokens in 54803 / out 32512 (reasoning included in out), duration 365.4 s, cost 0.0554 USD (reported), classifier `jev-1.13.0` calls 11, tokens in 9047, cost 0.0004 USD, finding judge `jev-1.13.0` calls 3, tokens in 8083, cost 0.0003 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32438 / out 263, cost 0.0042 USD (reported), build file judge `jev-1.13.0` calls 6, tokens in 9112, cost 0.0004 USD, phases: path_judge 0.5 s, classifier 0.7 s, context 4.6 s, model_review 358.4 s, finding_judge 0.4 s, config: defaults + host + repository (.werkrevisor.toml at main2 answers ignored because of a confidence below [classifier] min_confidence.
512150c).Reviewed head `319eb70da5071852a4f2d945fa5088622c1bd55a` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.54.1.
werkrevisor: YELLOW / HUMAN_REVIEW for
319eb70da5071852a4f2d945fa5088622c1bd55a.The result comment of werkrevisor holds the details.
@@ -113,3 +113,3 @@On context close (e.g. systemd SIGTERM), a `ContextClosedEvent` listener in `BuildExecutor` terminates the process trees of all executing builds and waits (bounded) until their results are persisted as INTERRUPTED — a shutdown is never recorded as FAILED. Builds still queued stay PENDING and start no process. Both are re-enqueued by the watcher's startup recovery; INTERRUPTED therefore publishes as Gitea state `pending`, not `failure` (`GiteaStateMapping`).The runtime is selected per build behind the `BuildRunner` interface: `DispatchingBuildRunner` (`@Primary`) routes to native `ProcessBuildRunner` (the default), to `DockerBuildRunner` when `docker.enabled`, or to `WerkdockBuildRunner` when `werkdock.enabled` — docker and werkdock are mutually exclusive per build and rejected in `buildSettings`, never picked silently. The Docker runner shells out to the `docker` CLI (no SDK): it (re)builds the configured image when the Dockerfile inputs changed (tracked via the `org.werkator.build-inputs-sha256` image label), maintains a per-repo Gradle cache volume, mounts the worktree and the Docker socket into a labelled (`org.hoennig.werkator`) `--rm --init` container (with `TESTCONTAINERS_HOST_OVERRIDE` from `docker.env` if it names one, else the child address a rootless daemon's rootlesskit reports for the `slirp4netns` port driver — `RootlessKitPorts`, asked over `dockerd-rootless/api.sock` on every build, PR#86 — else `localhost` under `network: host` and `host.docker.internal` otherwise), and repairs workspace ownership in-container after each command (under a rootless daemon the container runs as root, which is the host user, and the repair degenerates to `0:0`). Git works inside the container: the primary `.git` is mounted read-only with `.git/werkator/` masked by an empty tmpfs (credential isolation) and the worktree's admin dir mounted read-write (`gitMetadataMounts`). The returned `Process` is the attached `docker run` client, so log streaming and termination work exactly like native builds.The runtime is selected per build behind the `BuildRunner` interface: `DispatchingBuildRunner` (`@Primary`) routes to native `ProcessBuildRunner` (the default), to `DockerBuildRunner` when `docker.enabled`, or to `WerkdockBuildRunner` when `werkdock.enabled` — docker and werkdock are mutually exclusive per build and rejected in `buildSettings`, never picked silently. The Docker runner shells out to the `docker` CLI (no SDK): it (re)builds the configured image when the Dockerfile inputs changed (tracked via the `org.werkator.build-inputs-sha256` image label), maintains a per-repo Gradle cache volume, mounts the worktree and the Docker socket into a labelled (`org.hoennig.werkator`) `--rm --init` container (with `TESTCONTAINERS_HOST_OVERRIDE` from `docker.env` if it names one, else the child address a rootless daemon's rootlesskit reports for the `slirp4netns` port driver — `RootlessKitPorts`, asked over `dockerd-rootless/api.sock` on every build, PR#86 — else `localhost` under `network: host` and `host.docker.internal` otherwise), and repairs workspace ownership in-container after each command (under a rootless daemon the container runs as root, which is the host user, and the repair degenerates to `0:0`). Git works inside the container: the primary `.git` is mounted read-only with `.git/werkator/` masked by an empty tmpfs (credential isolation) and the worktree's admin dir mounted read-write (`gitMetadataMounts`). The returned `Process` is the attached `docker run` client, so log streaming and termination work exactly like native builds. Both sandbox runners pass a build's variables as a bare `-e KEY`/`--env KEY` with the value in the CLI process's environment, never on its argv (`RuntimeEnvironment`, PR#166); that environment holds only the runtime's own variables from Werkator's, and a variable named like one of them keeps `KEY=VALUE` on the argv.Writing rule: more than one sentence on a line (werkrevisor writing rule)
Fix the line, or reply why it stays, and resolve this comment.
Writing rule: line longer than 400 characters (1881) (werkrevisor writing rule)
Fix the line, or reply why it stays, and resolve this comment.
Fixed in
1e523bf: the new text is a paragraph of its own, one sentence per line. The long line 115 above it predates this PR and stays as it is.Same as above: the new sentences have their own lines now; line 115 is the existing paragraph and is not part of this change.
Weakened Test, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.77, severity 2.0 of 3
The "adds the sandbox env after the branch environment" test was changed from searching for the operands
branch=mainandFOO=bar(which incidentally verified the values "main" and "bar") to just the bare keysbranchandFOO. The test no longer catches a wrong or missing value in the build's environment. The same strictness can be retained by additionally assertingcapturedEnvironments.single()["branch"] shouldBe "main"and["FOO"] shouldBe "bar".Fixed in
1e523bf: the test asserts the values in the werkdock process's environment,branch= main andFOO= bar.@@ -336,0 +339,4 @@args.indexOf("branch") shouldBe args.indexOf("FOO") - 2}test("the werkdock command line names a build's variables, its environment holds their values (Scenario#166.01)") {Test Assertion Missing Variable, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.44, severity 1.8 of 3
The new Scenario#166.01 test sets up two non-runtime build variables, GITEA_BOT_TOKEN and WERKREVISOR_CONFIG_TOML, but the expected environment
HOST_RUNTIME_ENVIRONMENT + mapOf("branch" to "main", "GITEA_BOT_TOKEN" to "gitea-secret")only includes GITEA_BOT_TOKEN. Both variables are non-runtime names (WERKREVISOR_ is not in RUNTIME_PREFIXES, which only lists WERKDOCK_), so both travel in the processEnvironment, and the assertion will fail: the actual environment has six entries (PATH, HOME, WERKDOCK_HOME, branch, GITEA_BOT_TOKEN, WERKREVISOR_CONFIG_TOML) but the expected has five.No change; the finding is wrong. The expected environment is
HOST_RUNTIME_ENVIRONMENT + mapOf("branch" to "main") + secrets, andsecretsholds both GITEA_BOT_TOKEN and WERKREVISOR_CONFIG_TOML. The test passes in CI. The map with only GITEA_BOT_TOKEN is the Docker test's, where the second entry, DOCKER_HOST, is a runtime name and stays on the command line.@@ -336,0 +377,4 @@capturedEnvironments.single() shouldBe HOST_RUNTIME_ENVIRONMENT}test("the build still sees the values its werkdock command line only names (Scenario#166.03)") {Real Subprocess in Unit Test, severity unknown, counted as medium (werkrevisor)
hint; Jev: real 0.79, severity 2.0 of 3
The new Scenario#166.03 test "the build still sees the values its werkdock command line only names" starts the standInWerkdock shell script as a real subprocess, violating the *UnitTest convention (no real process). All other tests in the file use the mock processStarter; this test should either rely on an elaborated mock that simulates the werkdock operand resolution or be moved to an *IntegrationTest file.
Fixed in
1e523bf: the test moved to the newWerkdockBuildRunnerIntegrationTest.Codex review of
319eb70da5071852a4f2d945fa5088622c1bd55a@@ -0,0 +71,4 @@name.uppercase().let { upper -> upper in RUNTIME_NAMES || RUNTIME_PREFIXES.any { upper.startsWith(it) } }/** A name with `=` would read as `KEY=VALUE` on the command line, and no environment holds an empty one. */fun travelsInEnvironment(name: String): Boolean = name.isNotEmpty() && '=' !in name && '\u0000' !in name && !belongsToRuntime(name)[high] Exclude SSH helper controls from branch-supplied host variables (Codex review)
SSH_ASKPASS and SSH_ASKPASS_REQUIRE pass this filter and enter Docker's host-side environment. When the host uses an SSH Docker endpoint, Docker launches ssh with that environment. A branch can set SSH_ASKPASS to an executable in its worktree and SSH_ASKPASS_REQUIRE=force; if authentication requests a passphrase, SSH executes that file outside the sandbox as the Werkator user. The filter also removes the host's SSH_AUTH_SOCK, so an encrypted key normally unlocked through an agent can reach precisely this prompt, while ordinary agent-dependent builds fail authentication. These variables must remain host-controlled. The execution path follows Docker's SSH helper, its environment inheritance, and OpenSSH's documented environment handling.
Fix it, or reply why it does not hold, and resolve this comment.
Fixed in
d6e6683: every name starting withSSH_,SSL_orOPENSSL_, andDISPLAY/WAYLAND_DISPLAY, belongs to the runtime now. A build value of such a name stays on the command line, and the docker client gets the host'sSSH_AUTH_SOCKagain. RuntimeEnvironmentUnitTest covers SSH_ASKPASS, SSH_AUTH_SOCK, DISPLAY, SSL_CERT_FILE and OPENSSL_CONF; docs/configuration.md names the reason.⚠️ werkrevisor ERROR for
?, superseded by the result for00f7462Codex review: critical
This changes the security boundary between branch-controlled build variables and host-side runtime processes. An incomplete filter can execute branch-controlled code outside the sandbox, which the mocked runner tests do not detect.
Findings
src/main/kotlin/de/javagil/werkator/build/RuntimeEnvironment.kt:74, as a line commentThe merge waits for the status
Model `gpt-6-astra`, effort high, tokens in 1336433 (cached 1227904) / out 5492, 230.2 s, 16 commands; reviewed head `319eb70da5071852a4f2d945fa5088622c1bd55a`.ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.🟡 werkrevisor YELLOW for
00f7462, superseded by the result ford6e6683🟡 werkrevisor: YELLOW / HUMAN_REVIEW
Change kind:
mixed.Reasons
Findings
src/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:325-383, severity unknown, counted as medium, YELLOW, as a line commentClassifier
11 of 11 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 55898, calls 1, tokens in 53595 / out 48622 (reasoning included in out), duration 555.3 s, cost 0.0744 USD (reported), classifier `jev-1.13.0` calls 11, tokens in 8985, cost 0.0004 USD, finding judge `jev-1.13.0` calls 1, tokens in 3500, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32438 / out 181, cost 0.0004 USD (reported), build file judge `jev-1.13.0` calls 6, tokens in 9112, cost 0.0004 USD, phases: path_judge 0.5 s, classifier 0.8 s, context 3.5 s, model_review 549.6 s, finding_judge 0.3 s, config: defaults + host + repository (.werkrevisor.toml at main1 answer ignored because of a confidence below [classifier] min_confidence.
512150c).Reviewed head `00f746217af022b6311210cb48843b81d8523ffa` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.54.1.
werkrevisor: YELLOW / HUMAN_REVIEW for
00f746217af022b6311210cb48843b81d8523ffa.The result comment of werkrevisor holds the details.
Removed security-critical unit tests, severity unknown, counted as medium (werkrevisor)
YELLOW; Jev: real 0.51, severity 2.8 of 3
The PR removes five werkdock unit tests, and none of the scenarios they exercised is now covered by the surviving suite or by the new Scenario#166 cases:
:ro, so a build cannot rewrite the.werkator.ymlits successors' pinned settings are read from..gitis read-only and.git/werkator(machine config:git.token, control token, build state) is masked by an empty tmpfs.werkdock.rootfsis set.The first two are security invariants — a future regression that drops
:roon the primary checkout, or the.git/werkatortmpfs mask, would no longer be caught by any test. The other three cover edge cases (no-worktree workspaces, missing-rootfs validation, URL-rootfs download) that also have no replacement. A fix whose own security reasoning argues for keeping these invariants in place should not silently remove the tests that verify them.Right, and fixed in
d6e6683: moving Scenario#166.03 into the integration test had cut the five tests after it by mistake. They are back unchanged; the spec has 23 tests, the 21 of main plus Scenario#166.01 and #166.02.Codex review of
00f746217af022b6311210cb48843b81d8523ffa@@ -0,0 +65,4 @@"NO_PROXY","ALL_PROXY","DISPLAY","WAYLAND_DISPLAY",[high] SSH control variables can execute branch code outside the sandbox (Codex review)
SSH_ASKPASS and SSH_ASKPASS_REQUIRE are not protected, so a branch's docker.env can place them in Docker's host-side environment. For an ssh:// Docker endpoint, Docker launches SSH with that environment. Setting SSH_ASKPASS to an executable in the build worktree and SSH_ASKPASS_REQUIRE=force executes it as the Werkator user whenever SSH requests a passphrase. Removing the host's SSH_AUTH_SOCK can itself trigger that prompt for an encrypted private key, and otherwise breaks agent-dependent authentication. These variables must retain host-controlled values rather than accepting build values. This follows from Docker's environment inheritance and OpenSSH's askpass execution.
Fix it, or reply why it does not hold, and resolve this comment.
Fixed in
d6e6683, the head after the reviewed00f7462: every name starting withSSH_belongs to the runtime, so SSH_ASKPASS and SSH_ASKPASS_REQUIRE of a build stay on the command line and never reach the docker client's environment, and the client gets the host's SSH_AUTH_SOCK again. DISPLAY, WAYLAND_DISPLAY,SSL_*andOPENSSL_*too. RuntimeEnvironmentUnitTest covers them.@@ -336,0 +353,4 @@capturedEnvironments.single() shouldBe HOST_RUNTIME_ENVIRONMENT + mapOf("branch" to "main") + secrets}test("a variable werkdock or the host's loader reads itself stays on the command line (Scenario#166.02)") {[medium] Restore the removed git-metadata isolation tests (Codex review)
The replacement removes the test asserting read-only .git, the tmpfs mask over .git/werkator, and their ordering relative to writable mounts. No remaining werkdock test constructs the worktree metadata needed to exercise this path; the Docker tests cover a separate implementation. Consequently, removing the credential-hiding mask from WerkdockBuildRunner would now pass these tests. Preserve these security regression checks alongside the new environment tests.
Fix it, or reply why it does not hold, and resolve this comment.
Fixed in
d6e6683: the five tests are back unchanged, among them "exposes git metadata read-only with the werkator dir masked, in mount order" and "mounts the primary checkout read-only, the workspace read-write".⚠️ werkrevisor ERROR for
?, superseded by the result ford6e6683Codex review: critical
This changes how branch-controlled variables reach host processes outside the sandbox; mistakes can permit host code execution or expose credentials. It also removes existing security-isolation tests.
Findings
src/main/kotlin/de/javagil/werkator/build/RuntimeEnvironment.kt:68, as a line commentsrc/test/kotlin/de/javagil/werkator/build/WerkdockBuildRunnerUnitTest.kt:356, as a line commentThe merge waits for the status
Model `gpt-6-astra`, effort high, tokens in 682522 (cached 596736) / out 3782, 165.9 s, 13 commands; reviewed head `00f746217af022b6311210cb48843b81d8523ffa`.ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.🟡 werkrevisor YELLOW for
d6e6683, superseded by the result for425a99d🟡 werkrevisor: YELLOW / HUMAN_REVIEW
Change kind:
mixed.Reasons
Classifier
11 of 11 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM failed (TimeoutError), tokens counted before the call 56695, duration 728.8 s, cost unknown, classifier `jev-1.13.0` calls 11, tokens in 9046, cost 0.0004 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32438 / out 207, cost 0.0004 USD (reported), build file judge `jev-1.13.0` calls 6, tokens in 9207, cost 0.0004 USD, phases: path_judge 0.5 s, classifier 0.7 s, context 4.0 s, model_review 723.1 s, config: defaults + host + repository (.werkrevisor.toml at main2 answers ignored because of a confidence below [classifier] min_confidence.
512150c).Reviewed head `d6e668319f0ac2e915ea78fb3eb5b230906a735b` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.54.1.
Codex review of
d6e668319f0ac2e915ea78fb3eb5b230906a735b@@ -0,0 +24,4 @@/** The environment the CLI process starts with: the runtime's own variables from [host], then the build's values. */val processEnvironment: Map<String, String> =host.filterKeys { belongsToRuntime(it) } + variables.filterKeys { travelsInEnvironment(it) }[high] BASH_ENV permits code execution through host-side credential helpers (Codex review)
BASH_ENV passes this filter, so a branch can supply it through docker.env to Docker and its children. On a host using docker-credential-pass, the helper invokes the Bash-based pass program with that inherited environment, allowing branch-supplied startup code to execute as the Werkator user outside the container. This is reachable even with cached images when Docker 28 content trust is enabled: trusted-image resolution retrieves credentials, and the pass helper starts pass without clearing the environment. A harmless local check confirmed that BASH_ENV executes code when pass starts. Previously this value was only a container environment operand.
Fix it, or reply why it does not hold, and resolve this comment.
Fixed in
425a99d, by a different route rather than a longer list: a Docker build's values no longer enter the docker client's environment at all.DockerEnvFilewrites them to a 0600 file in.git/werkator/docker-env/(0700, masked in the container) fordocker run --env-file, removed when the client ends; the client keeps Werkator's own environment as before. A value with a line break, which docker's file format cannot carry, stays--env KEY=VALUEas before. werkdock keeps the environment route, since it starts no host program but itself and bwrap, and werkdock PR#18 starts bwrap with an empty environment. Scenario#166.04 covers it.⚠️ werkrevisor ERROR for
?, superseded by the result for425a99dCodex review: critical
This changes the security boundary between branch-controlled build variables and host processes that handle credentials. An incomplete environment filter can allow code execution outside the sandbox, which the current tests do not cover.
Findings
src/main/kotlin/de/javagil/werkator/build/RuntimeEnvironment.kt:27, as a line commentThe merge waits for the status
Model `gpt-6-astra`, effort high, tokens in 779535 (cached 691072) / out 3444, 165.9 s, 8 commands; reviewed head `d6e668319f0ac2e915ea78fb3eb5b230906a735b`.ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.New commits pushed, approval review dismissed automatically according to repository settings
🟡 werkrevisor YELLOW for
425a99d, superseded by the result fore9c77a3🟡 werkrevisor: YELLOW / HUMAN_REVIEW
Change kind:
mixed.Reasons
Classifier
10 of 12 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 60039, calls 1, tokens in 57561 / out 29982 (reasoning included in out), duration 224.5 s, cost 0.0532 USD (reported), classifier `jev-1.13.0` calls 12, tokens in 10397, cost 0.0004 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32506 / out 65, cost 0.0041 USD (reported), build file judge `jev-1.13.0` calls 7, tokens in 12004, cost 0.0005 USD, phases: path_judge 0.5 s, classifier 0.8 s, context 2.9 s, model_review 219.8 s, config: defaults + host + repository (.werkrevisor.toml at main2 answers ignored because of a confidence below [classifier] min_confidence.
512150c).Reviewed head `425a99d8e602cbd0a148e681a2337e16d1cb5d70` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.55.0.
Codex review of
425a99d8e602cbd0a148e681a2337e16d1cb5d70@@ -0,0 +43,4 @@return DockerEnvFile(null, variables)}val dir = repoDir.resolve(DIR)SecretFiles.createDirectoriesOwnerOnly(dir)[medium] Concurrent first builds can fail creating the shared directory (Codex review)
When two Docker builds first run for a repository, both can observe that docker-env is missing. SecretFiles.createDirectoriesOwnerOnly checks existence and then calls Files.createDirectory without handling FileAlreadyExistsException. One build therefore fails before launching its container. This is reachable through concurrent branches or the independent pull-request-target pool.
Fix it, or reply why it does not hold, and resolve this comment.
Fixed in
e9c77a3:SecretFiles.createDirectoriesOwnerOnlytakes a directory that another thread created in the meantime and fails only if the path is no directory. DockerEnvFileUnitTest starts eight writes together on a fresh repository; without the fix it failed in three of three runs, with it it passes.@@ -0,0 +61,4 @@key.isNotEmpty() &&!key.startsWith("#") &&key.none { it == '=' || it.isWhitespace() || it == '\u0000' } &&value.none { it == '\n' || it == '\r' || it == '\u0000' }[medium] Account for Docker’s env-file line-length limit (Codex review)
fits accepts arbitrarily long single-line values, but Docker’s parser uses a scanner with a 64 KiB token limit. A 70 KiB ASCII configuration value previously accepted through --env now makes docker run fail with 'token too long'. Check the encoded KEY=VALUE line length when selecting the file transport.
Fix it, or reply why it does not hold, and resolve this comment.
Fixed in
e9c77a3:fitsalso requires the UTF-8 lineKEY=VALUEto stay below 64 KiB; a longer one stays--env KEY=VALUEon the command line, as before. DockerEnvFileUnitTest covers a 70 KiB value.⚠️ werkrevisor ERROR for
?, superseded by the result fore9c77a3Codex review: critical
The change handles build secrets and moves branch-controlled variables into a host process’s environment. Mistakes in filtering or file isolation could expose credentials or compromise execution outside the sandbox, so human review is required.
Findings
src/main/kotlin/de/javagil/werkator/build/DockerEnvFile.kt:46, as a line commentsrc/main/kotlin/de/javagil/werkator/build/DockerEnvFile.kt:64, as a line commentThe merge waits for the status
Model `gpt-6-astra`, effort high, tokens in 375117 (cached 265728) / out 2487, 89.2 s, 13 commands; reviewed head `425a99d8e602cbd0a148e681a2337e16d1cb5d70`.ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.🟠 werkrevisor: ORANGE / FINDINGS
Change kind:
mixed.Reasons
Findings
src/main/kotlin/de/javagil/werkator/build/DockerEnvFile.kt:64-68, ORANGE, as a line commentClassifier
12 of 12 units passed all questions (1 PR document(s)).
Usage: model `openrouter/minimax/minimax-m3`, LLM done, tokens counted before the call 62968, calls 1, tokens in 60394 / out 39980 (reasoning included in out), duration 283.4 s, cost 0.0661 USD (reported), classifier `jev-1.13.0` calls 12, tokens in 10442, cost 0.0004 USD, finding judge `jev-1.13.0` calls 1, tokens in 2131, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 32506 / out 12, cost 0.0041 USD (reported), build file judge `jev-1.13.0` calls 9, tokens in 14684, cost 0.0006 USD, phases: path_judge 0.8 s, classifier 0.7 s, context 2.1 s, model_review 278.7 s, finding_judge 0.3 s, config: defaults + host + repository (.werkrevisor.toml at main2 answers ignored because of a confidence below [classifier] min_confidence.
512150c).Reviewed head `e9c77a3328838019ce16502b2a13ec9b35c8c39e` against base `512150c6ac367240d2ff699ad365aa75c8053940`, werkrevisor 0.55.0.
werkrevisor: ORANGE / FINDINGS for
e9c77a3328838019ce16502b2a13ec9b35c8c39e.The result comment of werkrevisor holds the details.
@@ -0,0 +61,4 @@key: String,value: String,): Boolean =key.isNotEmpty() &&[medium] Value corruption (werkrevisor)
ORANGE; Jev: real 0.85, severity 3.0 of 3
fits()treats values without line breaks or NULs as round-trippable, but Docker's documented env-file parser interprets quotes and escape sequences. A literal value such asHOME="value"is therefore delivered asHOME=value, and an unterminated quoted value can makedocker runreject the file; the previousHOME="value"argv form preserved it. Reject or encode values the parser cannot round-trip and add coverage for quoted inputs.No change; the finding does not hold for
docker run --env-file. docker/cli's parser (pkg/kvfile/kvfile.go, parseKeyValueFile) interprets neither quotes nor escapes: it trims leading whitespace of the line, skips empty lines and#comments, rejects a key with whitespace, and then appends the line as-is ("key/value pair is valid and has a value; add the line as-is"). SoHOME=\"value\"arrives with its quotes, exactly as the former--env HOME=\"value\". Quote handling belongs to Compose's .env files, which this PR does not use. What the parser does change, a leading#, whitespace in the key, a line break and the 64 KiB line limit,fitskeeps on the command line.Codex review: critical
This changes how build secrets and branch-controlled environment variables reach host-side sandbox processes. A mistake could expose credentials or allow execution outside the sandbox, and the mocked runtime tests do not establish that security boundary.
No findings.
The merge waits for the status
Model `gpt-6-astra`, effort high, tokens in 692412 (cached 614784) / out 2581, 93.8 s, 16 commands; reviewed head `e9c77a3328838019ce16502b2a13ec9b35c8c39e`.ai-review/codex: every line comment resolved, and the approval of a human for a critical PR or for a finding that is only here.