kyleandClaude Opus 5.5
a57a1305e7
14 moves the pipe handling out of container.rs (a pure move, replayed on its own); 15 starts threads with Builder and bounds output collection with a 2 s grace period; 16 escapes container errors in the log and fixes two texts; 17 fixes toolkit's thread start, casts and the egress-proxy form. Each checked against a reference, which is not kept. Tips T24 to T26 from this run. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
62 lines
2.9 KiB
Markdown
62 lines
2.9 KiB
Markdown
# M3b task 16: escape what the container writes before logging it, and three small texts
|
|
|
|
**Branch:** `m3b` (run `git switch m3b`; `git status --short` must be empty, otherwise stop)
|
|
**Commit subject:** `brokerd: escape container errors in the log; prefix and quote two messages`
|
|
|
|
## Goal
|
|
|
|
**M3b review finding 5.** What a container writes on standard error (and Podman's own errors, which
|
|
the tool can influence) goes into `brokerd`'s log as it is, newlines included. A tool can therefore
|
|
write a line that looks like one of `brokerd`'s own, for example a fake
|
|
`see docs/runbook.md#…` pointer. Log it with Rust's debug formatting (`{err:?}`), which quotes it and
|
|
writes every newline and control character as an escape: one event stays one entry.
|
|
|
|
And three small texts from the review (finding 7): the runtime notice lacks its `brokerd:` prefix,
|
|
and two `[runner]` errors should quote the bad value.
|
|
|
|
## Files
|
|
|
|
- Copy: `crates/brokerd/tests/container_log.rs`, `crates/brokerd/tests/notices.rs`
|
|
- Modify: `crates/brokerd/src/container.rs`, `crates/brokerd/src/main.rs`,
|
|
`crates/brokerd/src/config.rs`, `docs/implementer-log.md`
|
|
|
|
## The changes, exactly
|
|
|
|
In `container.rs`, `answer`, three log calls become:
|
|
|
|
| Case | Log call |
|
|
|---|---|
|
|
| exit 2 | `(self.log)(&format!("brokerd: {name}: the tool could not run: {err:?}"));` |
|
|
| exit 125 to 127 | `(self.log)(&format!("brokerd: podman could not start {name}: {err:?}\n{RUNBOOK}"));` |
|
|
| any other ending | `(self.log)(&format!("brokerd: container {name} exited {status}: {err:?}"));` |
|
|
|
|
In `start_egress`, where the proxy's `podman run -d` does not succeed, the standard error in the log
|
|
call becomes `{stderr:?}` in the same way (the rest of that line, and its `\n{RUNBOOK}`, stay).
|
|
|
|
The only raw `\n` left in any of these is the one before `RUNBOOK`, which is ours.
|
|
|
|
In `main.rs`: the line that prints the runtime notice becomes `eprintln!("brokerd: {runtime_notice}");`.
|
|
|
|
In `config.rs`: in the two messages `[runner] image is {}; …` and `[runner] memory is {}; …`, the
|
|
`{}` becomes `{:?}`, so the value is shown in quotes.
|
|
|
|
## Steps
|
|
|
|
- [ ] **1. Copy.** `git switch m3b`, then
|
|
`cp docs/plans/M3b/files/crates/brokerd/tests/container_log.rs docs/plans/M3b/files/crates/brokerd/tests/notices.rs crates/brokerd/tests/`
|
|
- [ ] **2. See them fail.** `cargo test -p brokerd --no-fail-fast --test container_log --test notices`.
|
|
Expected: 4 and 2 fail.
|
|
- [ ] **3. Make the changes.** Run `cargo fmt --all`.
|
|
- [ ] **4. See them pass.** `cargo test -p brokerd --test container_log --test notices --test serve_runner --test config_runner`.
|
|
Expected: 4, 2, 2 and 7 passed.
|
|
- [ ] **5. Run the gate.** `make gate`. Expected last line: `gate: ok`.
|
|
- [ ] **6. Log and commit.** `git add crates/brokerd docs/implementer-log.md Cargo.lock && git commit`
|
|
|
|
## Done when
|
|
|
|
- The four suites pass; `make gate` prints `gate: ok`.
|
|
|
|
## Stop and report if
|
|
|
|
- A test wants container output anywhere but the log.
|