From feb50abc88207c62968cecc968eac6314fc646fe Mon Sep 17 00:00:00 2001 From: "K. Isom" Date: Wed, 23 Sep 2026 09:35:46 -0700 Subject: [PATCH] brokerd serve: run tools in containers when [runner] is set Implemented-By: OpenCode session (model recorded in docs/implementer-log.md) --- crates/brokerd/src/main.rs | 32 ++++-- crates/brokerd/tests/serve_runner.rs | 143 +++++++++++++++++++++++++++ docs/implementer-log.md | 1 + 3 files changed, 169 insertions(+), 7 deletions(-) create mode 100644 crates/brokerd/tests/serve_runner.rs diff --git a/crates/brokerd/src/main.rs b/crates/brokerd/src/main.rs index bedebd1..2938f31 100644 --- a/crates/brokerd/src/main.rs +++ b/crates/brokerd/src/main.rs @@ -7,7 +7,8 @@ use std::sync::Arc; use brokerd::audit::{AuditError, RECOVERED_NOTICE}; use brokerd::config::Config; -use brokerd::runner::Refusing; +use brokerd::container::Podman; +use brokerd::runner::{Refusing, Runtime}; use brokerd::serve::{self, ServeError}; const USAGE: &str = "usage: brokerd serve --config [--accept-break]"; @@ -44,12 +45,28 @@ fn main() -> ExitCode { let broker_path = cfg.broker_socket(); let admin_path = cfg.admin_socket(); - let started = match serve::start( - cfg, - accept_break, - Box::new(Refusing), - Arc::new(|line: &str| eprintln!("{line}")), - ) { + // The runtime and the line that says which. A `[runner]` runs calls in containers; without it, + // every call is refused, as in M3a. + let log: Arc = Arc::new(|line: &str| eprintln!("{line}")); + let runtime: Box; + let runtime_notice: String; + match &cfg.runner { + Some(runner) => { + let image = runner.image.clone(); + runtime = Box::new(Podman::new( + runner.clone(), + cfg.egress_dir(), + Arc::clone(&log), + )); + runtime_notice = format!("tools run in containers from {image}"); + } + None => { + runtime = Box::new(Refusing); + runtime_notice = "no [runner] section: every tool call is refused".to_string(); + } + } + + let started = match serve::start(cfg, accept_break, runtime, log) { Ok(started) => started, Err(ServeError::Audit(e @ AuditError::NothingToAccept)) => { eprintln!("brokerd: {e}"); @@ -80,6 +97,7 @@ fn main() -> ExitCode { broker_path.display(), admin_path.display() ); + eprintln!("{runtime_notice}"); match started.run() { Ok(()) => ExitCode::SUCCESS, diff --git a/crates/brokerd/tests/serve_runner.rs b/crates/brokerd/tests/serve_runner.rs new file mode 100644 index 0000000..3f58e64 --- /dev/null +++ b/crates/brokerd/tests/serve_runner.rs @@ -0,0 +1,143 @@ +//! `brokerd serve` with a `[runner]` section runs allowed calls through Podman (here a fake), and +//! says which runtime it uses. Without the section it refuses every call, as in M3a. Do not edit. + +#[path = "support/fake_podman.rs"] +mod fake_podman; + +use std::io::Read; +use std::os::unix::net::UnixStream; +use std::path::Path; +use std::process::{Child, Command, Stdio}; +use std::time::{Duration, Instant}; + +use fake_podman::{Fake, IMAGE, serial}; +use proto::{ + CallId, DataClass, Envelope, Message, PROTOCOL_VERSION, SessionId, ToolRequest, ToolResponse, + read_frame, write_frame, +}; + +struct Running(Child); + +impl Drop for Running { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } +} + +impl Running { + fn stop(mut self) -> String { + let _ = self.0.kill(); + let _ = self.0.wait(); + let mut err = String::new(); + if let Some(mut stderr) = self.0.stderr.take() { + let _ = stderr.read_to_string(&mut err); + } + err + } +} + +fn start(home: &Path, runner: &str) -> Running { + std::fs::create_dir_all(home.join("grants")).unwrap(); + std::fs::write( + home.join("grants/notes.toml"), + "tool = \"read_file\"\nmode = \"auto\"\nmax_taint = \"secret\"\n[constraints]\npaths = [\"/n\"]\n", + ) + .unwrap(); + let config = home.join("brokerd.toml"); + std::fs::write( + &config, + format!( + "[paths]\nhome = \"{0}\"\ngrants = \"{0}/grants\"\n{runner}\n", + home.display() + ), + ) + .unwrap(); + let child = Command::new(env!("CARGO_BIN_EXE_brokerd")) + .args(["serve", "--config"]) + .arg(&config) + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let running = Running(child); + let until = Instant::now() + Duration::from_secs(10); + while UnixStream::connect(home.join("run/loop-broker/broker.sock")).is_err() { + assert!(Instant::now() < until, "brokerd never listened"); + std::thread::sleep(Duration::from_millis(20)); + } + running +} + +fn read_call(home: &Path) -> ToolResponse { + let mut stream = UnixStream::connect(home.join("run/loop-broker/broker.sock")).unwrap(); + stream + .set_read_timeout(Some(Duration::from_secs(10))) + .unwrap(); + let request = ToolRequest { + session: SessionId::new("s1").unwrap(), + call: CallId(1), + tool: "read_file".to_string(), + arguments: r#"{"path":"/n/a.md"}"#.to_string(), + }; + let envelope = Envelope { + v: PROTOCOL_VERSION, + id: 1, + r#final: true, + msg: Message::ToolRequest(request), + }; + write_frame(&mut stream, &envelope).unwrap(); + match read_frame(&mut stream).unwrap().msg { + Message::ToolResponse(response) => response, + other => panic!("{other:?}"), + } +} + +#[test] +fn with_a_runner_an_allowed_call_runs_in_a_container() { + let _s = serial(); + let fake = Fake::new( + "serve", + "cat > /dev/null; printf 'from the container'; exit 0", + ); + let home = fake.dir.join("home"); + let running = start( + &home, + &format!( + "[runner]\npodman = \"{}\"\nimage = \"{IMAGE}\"\n", + fake.script.display() + ), + ); + assert_eq!( + read_call(&home), + ToolResponse::Result { + content: "from the container".to_string(), + class: DataClass::Private, + untrusted: true, + truncated: false, + } + ); + let printed = running.stop(); + assert!( + printed.contains(&format!("tools run in containers from {IMAGE}")), + "{printed}" + ); + assert_eq!(fake.calls().len(), 1); +} + +#[test] +fn without_a_runner_every_call_is_refused_and_it_says_so() { + let _s = serial(); + let fake = Fake::new("serve-none", "exit 0"); + let home = fake.dir.join("home"); + let running = start(&home, ""); + assert_eq!( + read_call(&home), + ToolResponse::Failed { + message: brokerd::runner::REFUSING.to_string() + } + ); + let printed = running.stop(); + assert!(printed.contains("no [runner] section"), "{printed}"); + assert!(fake.calls().is_empty()); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index d924aad..5c6dd35 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -6,6 +6,7 @@ reviewer adds findings under "Reviews" once per milestone. | Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model | |---|---|---|---|---|---|---|---| +| M3b/13-brokerd-serve-runner | 2026-09-23 | done | 1 | pass | none | The `Podman` runtime was already written in task 12 (`crates/brokerd/src/container.rs`), so this task only wired it into `serve`. In `main.rs`: after the config loads, added `let log: Arc = Arc::new(|line| eprintln!("{line}"))`; chose the runtime and its notice with a match on `&cfg.runner` — `Some(runner)` captures `runner.image` first then `Box::new(Podman::new(runner.clone(), cfg.egress_dir(), Arc::clone(&log)))` with notice `tools run in containers from {image}`; `None` → `Box::new(Refusing)` with notice `no [runner] section: every tool call is refused`. Passed `runtime` and `log` to `serve::start` (its signature needed no change — it already took `Box` and `Arc`), and printed the notice with `eprintln!` after the `serving tools on … and approvals on …` line. Added `use brokerd::container::Podman` and `Runtime` to the imports. Copied `tests/serve_runner.rs`; the 2 new tests plus the 9 serve tests pass; `make gate` prints `gate: ok` on the first run. | ? | | M3b/12-brokerd-egress | 2026-09-23 | done | 1 | pass | none | Wrote `crates/brokerd/src/container.rs` (432 lines). Added consts `EGRESS_WAIT` (5s) and `EGRESS_POLL` (20ms); `Podman` gained private `egress_wait` (set to `EGRESS_WAIT` by `new`) and the public `with_egress_wait`. Added private `cannot_launch(podman, e)` (the task-11 step 3 log `brokerd: cannot start {path}: {e}\n{RUNBOOK}`), which `spawn` now calls, and `cannot_make(dir, e)` returning `Err(Unavailable(CANNOT_START))` with `brokerd: cannot make {dir}: {e}\n{RUNBOOK}`. `Runtime::run` now branches on `spec.egress()`: `None` runs `tool_args(spec, &runner, &name, None)` as before; `Some(hosts)` computes `dir = egress_dir.join(&name)`, calls `start_egress`, then `tool_args(spec, &runner, &name, Some(&dir))`. `start_egress` creates the `EgressGuard` first (so every return cleans up), then step 1: `DirBuilder::new().recursive(true).mode(0o700).create(egress_dir)`, `set_permissions(egress_dir, 0o700)` anyway, `remove_dir_all(dir)` if it exists (NotFound ok), `DirBuilder::new().mode(0o700).create(dir)` non-recursively, each failure via `cannot_make`; step 2: `Command::new(&runner.podman).args(egress_args).stdin(null).stdout(null).stderr(piped()).output()`, non-success logs `brokerd: podman could not start {name}-egress: {stderr}\n{RUNBOOK}` and returns Unavailable, launch error uses `cannot_launch`; step 3: wait for `dir.join("egress.sock")` every 20 ms until `egress_wait`, then log `brokerd: {name}-egress did not make its socket within {ms} ms\n{RUNBOOK}` and return Unavailable; step 4 `Ok(_guard)`. `EgressGuard<'a>` holds `&'a Podman`, the container name `-egress` and the directory; its `Drop` runs `podman rm -f -egress` (the task-11 helper) then `remove_dir_all(dir)` (NotFound ok, else log `brokerd: cannot remove {dir}: {e}`). Added `use std::os::unix::fs::{DirBuilderExt, PermissionsExt};`. `cargo fmt` put the new `impl Podman` block after `impl Runtime`. 6 egress + 11 container tests pass ten runs in a row. First gate failed on `start_egress` being inside `impl Runtime` (not a trait member) and missing `DirBuilderExt`/`PermissionsExt`; then on the guard being created at the end instead of the start (the two "nothing left" tests need the `rm -f -egress` call on a failed proxy), fixed by moving `EgressGuard::new` to the top and returning it; then on `unused_variable` for the drop guard and `mismatched_lifetime_syntaxes` on the return type (fixed to `EgressGuard<'_>`), the guard renamed `_guard` (1.98 still lints drop-only bindings). `make gate` prints `gate: ok`. | ? | | M3b/11-brokerd-container | 2026-09-23 | done | 2 | fail | none | Wrote `crates/brokerd/src/container.rs`: the fixed-sentence constants (`COULD_NOT_RUN`, `CANNOT_START`, `KILLED`, `TIMED_OUT`, `UNEXPECTED`), `RUNBOOK`, `POLL`, `STDERR_KEPT` and the `Log` type; `Podman { runner, egress_dir, log, next: AtomicU64 }` with `new` (`next` starts at 0) and the public `egress_dir`. `Runtime::run` = `next.fetch_add` for the container number, `podman::container_name`, `podman::tool_args(spec, &runner, &name, None)`, `spec.arguments().canonical_json()`, `runner.time_limit(tool)`, then `run_container` (already written). `spawn` = `Command::new(&runner.podman)` with all three streams piped, on failure log `brokerd: cannot start {path}: {e}\n{RUNBOOK}` and return None. `Io::start` takes the three pipes and starts one thread each: write `input` then drop stdin, `read_capped` stdout with `cap`, `read_capped` stderr with `STDERR_KEPT`; `finish` joins all three (a missing or panicked thread counts as empty via `join().ok()`/`unwrap_or_default()`). `read_capped` reads past the cap with an 8 KiB buffer, keeping the first `cap` bytes and setting `truncated`, taking `remaining.min(n)` so it never indexes past what it kept. `wait` loops `child.try_wait()`, and once `limit` has elapsed runs `podman kill ` then `podman rm -f ` (each via the private `podman` helper — `.status()` with the three streams null, logging a line on non-success), then `child.kill()`/`child.wait()`, returning None. `podman` helper uses `&self.runner.podman`, not a `podman` field. `answer` follows the table: None→TIMED_OUT (log a line naming the container); exit 0/1→Ok with content `from_utf8_lossy(out)`; exit 2→COULD_NOT_RUN (log stderr); 125/126/127→CANNOT_START (log stderr then `\n{RUNBOOK}`); 137→KILLED; anything else incl. signal→UNEXPECTED (log status and stderr). The tool's output never enters a `RunError`; only the six constants do. Deleted the skeleton paragraph. Added `pub mod container;` to lib.rs after `config`. Copied `tests/support/fake_podman.rs` and `tests/container.rs`. `container` 11 passed ten runs; first gate failed on clippy `manual_range_patterns` (`125|126|127` → `125..=127`), fixed; `make gate` prints `gate: ok`. | ? | | M3b/10-brokerd-podman-args | 2026-09-23 | done | 2 | fail | none | Wrote `crates/brokerd/src/podman.rs`: the `EGRESS_MOUNT`/`EGRESS_SOCKET`/`TOOLKIT` constants; `container_name` = `boxmaker---`; the private `hardening(pids, memory)` emitting the six shared flags (`--read-only` to `--memory=…`) once; the private `volume(host, container, mode)` built with `push` so a directory need not be UTF-8; `tool_args` = `run --rm -i --name= --label=boxmaker=tool --network=none`, then hardening with `runner.pids`/`runner.memory`, `--tmpfs=/tmp:rw,size=64m,mode=1777`, one `--volume=::ro|rw` per `spec.mounts()` in order, the egress volume when `egress` is Some, then ` /bin/toolkit `; `egress_args` = `run -d --rm --name=-egress --label=boxmaker=egress --network=`, hardening 64/128m, the egress volume, then ` /bin/toolkit egress-proxy --socket /run/egress/egress.sock --allow `. `RunSpec` gained private `session: SessionId` and `call: CallId` as its first two fields with `session()`/`call()` getters; `run` fills them from `decision.request()`, and the `compile_fail` doctest struct literal gains the two fields first. Copied `tests/podman_args.rs` and the six `podman/*.args` golden files. `podman_args` 7 passed, `runner` 8 passed, all doctests pass. First gate failed on clippy `redundant_closure` (`.map(|f| OsString::from(f))` → `.map(OsString::from)`). `make gate` prints `gate: ok`. | ? |