toolkit: shell

Implemented-By: OpenCode session (model recorded in docs/implementer-log.md)

crates/toolkit/src/lib.rs

@@ -1,9 +1,10 @@
 //! The programs that run inside tool containers.
 
-use proto::tools::{ReadFileArgs, WriteFileArgs};
+use proto::tools::{ReadFileArgs, ShellArgs, WriteFileArgs};
 
 pub mod files;
 pub mod input;
+pub mod shell;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Exit {
@@ -77,6 +78,13 @@ pub fn run(name: &str, stdin: &mut dyn std::io::Read) -> Outcome {
             };
             files::write_file(&args)
         }
+        "shell" => {
+            let args = match input::parse::<ShellArgs>(name, &text) {
+                Ok(a) => a,
+                Err(e) => return Outcome::misuse(e),
+            };
+            shell::shell(&args)
+        }
         _ => Outcome::misuse(format!("toolkit: unknown tool {name:?}")),
     }
 }

crates/toolkit/src/shell.rs

@@ -0,0 +1,93 @@
+//! The third tool, `shell`: run one command under `/bin/sh -c` in a directory and report its
+//! output (stdout and stderr together, in order) followed by how the command ended. The command's
+//! own exit status is part of the result, not `toolkit`'s.
+
+use std::io::{self, Read};
+use std::os::unix::process::ExitStatusExt;
+use std::path::Path;
+use std::process::{Command, Stdio};
+
+use proto::tools::ShellArgs;
+
+use crate::Outcome;
+
+pub const SHELL: &str = "/bin/sh";
+pub const DEFAULT_CWD: &str = "/tmp";
+pub const MAX_OUTPUT: usize = 1024 * 1024;
+
+pub fn shell(args: &ShellArgs) -> Outcome {
+    let cwd = match args.cwd.as_deref() {
+        Some(c) => c,
+        None => DEFAULT_CWD,
+    };
+    if !Path::new(cwd).is_dir() {
+        return Outcome::tool_error(format!("shell: {cwd}: no such directory"));
+    }
+
+    let (mut reader, writer) = match io::pipe() {
+        Ok(p) => p,
+        Err(e) => return Outcome::tool_error(format!("shell: cannot make a pipe: {e}")),
+    };
+    let writer2 = match writer.try_clone() {
+        Ok(w) => w,
+        Err(e) => return Outcome::tool_error(format!("shell: cannot make a pipe: {e}")),
+    };
+
+    let mut command = Command::new(SHELL);
+    command.arg("-c").arg(&args.command).current_dir(cwd);
+    command.stdin(Stdio::null());
+    command.stdout(writer);
+    command.stderr(writer2);
+    let mut child = match command.spawn() {
+        Ok(c) => c,
+        Err(e) => return Outcome::tool_error(format!("shell: cannot start {SHELL}: {e}")),
+    };
+    // The Command still owns the write ends; while they are open reading never reaches EOF.
+    drop(command);
+
+    let mut buf = [0u8; 8 * 1024];
+    let mut out: Vec<u8> = Vec::new();
+    let mut dropped = false;
+    loop {
+        let n = match reader.read(&mut buf) {
+            Ok(0) => break,
+            Ok(n) => n,
+            Err(e) if e.kind() == io::ErrorKind::Interrupted => continue,
+            Err(e) => return Outcome::tool_error(format!("shell: cannot read the command: {e}")),
+        };
+        if out.len() >= MAX_OUTPUT {
+            // Past the limit: keep draining so the command never blocks on a full pipe.
+            dropped = true;
+            continue;
+        }
+        let chunk = match buf.get(..n) {
+            Some(c) => c,
+            None => continue,
+        };
+        let room = MAX_OUTPUT - out.len();
+        if chunk.len() <= room {
+            out.extend_from_slice(chunk);
+        } else {
+            out.extend_from_slice(chunk.get(..room).unwrap_or_default());
+            dropped = true;
+        }
+    }
+
+    let status = match child.wait() {
+        Ok(s) => s,
+        Err(e) => return Outcome::tool_error(format!("shell: cannot wait for the command: {e}")),
+    };
+
+    let mut text = String::from_utf8_lossy(&out).into_owned();
+    if dropped {
+        text.push_str(&format!("\n[output after {MAX_OUTPUT} bytes dropped]"));
+    }
+    match status.code() {
+        Some(code) => text.push_str(&format!("\n[exit {code}]")),
+        None => match status.signal() {
+            Some(sig) => text.push_str(&format!("\n[killed by signal {sig}]")),
+            None => text.push_str("\n[ended without a status]"),
+        },
+    }
+    Outcome::done(text)
+}

crates/toolkit/tests/shell.rs

@@ -0,0 +1,89 @@
+//! `toolkit shell`, run as `brokerd` runs it, with the host's `/bin/sh`. Do not edit.
+
+mod support;
+
+use proto::tools::ShellArgs;
+use support::{TempDir, json, toolkit};
+use toolkit::shell::MAX_OUTPUT;
+
+fn sh(command: &str, cwd: Option<&str>) -> support::Ran {
+    let args = ShellArgs {
+        command: command.to_string(),
+        cwd: cwd.map(str::to_string),
+    };
+    toolkit(&["shell"], &json(&args))
+}
+
+#[test]
+fn output_and_errors_come_back_in_order_then_the_exit_status() {
+    let ran = sh("echo one; echo two >&2; echo three", None);
+    assert_eq!(ran.code, 0);
+    assert_eq!(ran.stdout, "one\ntwo\nthree\n\n[exit 0]");
+}
+
+#[test]
+fn a_failing_command_is_still_a_result_with_its_status() {
+    let ran = sh("echo nope; exit 3", None);
+    assert_eq!((ran.code, ran.stdout.as_str()), (0, "nope\n\n[exit 3]"));
+    let ran = sh("true", None);
+    assert_eq!(ran.stdout, "\n[exit 0]");
+}
+
+#[test]
+fn a_command_killed_by_a_signal_says_so() {
+    let ran = sh("kill -9 $$", None);
+    assert_eq!(
+        (ran.code, ran.stdout.as_str()),
+        (0, "\n[killed by signal 9]")
+    );
+}
+
+#[test]
+fn the_command_runs_in_cwd_or_in_tmp() {
+    let dir = TempDir::new("cwd");
+    let here = dir.path().to_str().unwrap();
+    let ran = sh("pwd", Some(here));
+    assert_eq!(ran.stdout, format!("{here}\n\n[exit 0]"));
+    let ran = sh("pwd", None);
+    assert_eq!(ran.stdout, "/tmp\n\n[exit 0]");
+}
+
+#[test]
+fn a_missing_cwd_is_exit_1() {
+    let ran = sh("pwd", Some("/no/such/dir"));
+    assert_eq!(
+        (ran.code, ran.stdout.as_str()),
+        (1, "shell: /no/such/dir: no such directory")
+    );
+}
+
+#[test]
+fn standard_input_is_empty() {
+    let ran = sh("cat; echo done", None);
+    assert_eq!(ran.stdout, "done\n\n[exit 0]");
+}
+
+#[test]
+fn output_past_the_limit_is_dropped_and_the_command_still_finishes() {
+    let ran = sh(
+        &format!(
+            "head -c {} /dev/zero | tr '\\0' a; echo; echo end >&2; exit 4",
+            MAX_OUTPUT + 5000
+        ),
+        None,
+    );
+    assert_eq!(ran.code, 0);
+    let expected_tail = format!("\n[output after {MAX_OUTPUT} bytes dropped]\n[exit 4]");
+    assert!(
+        ran.stdout.ends_with(&expected_tail),
+        "{}",
+        &ran.stdout[ran.stdout.len() - 80..]
+    );
+    assert_eq!(ran.stdout.len(), MAX_OUTPUT + expected_tail.len());
+}
+
+#[test]
+fn output_that_is_not_utf8_is_replaced_not_refused() {
+    let ran = sh("printf 'a\\377b'", None);
+    assert_eq!(ran.stdout, "a\u{fffd}b\n[exit 0]");
+}

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/05-toolkit-shell | 2026-09-23 | done | 2 | fail | none | Wrote `crates/toolkit/src/shell.rs` (`SHELL=/bin/sh`, `DEFAULT_CWD=/tmp`, `MAX_OUTPUT=1048576`). `shell()`: `cwd` defaults to `/tmp`, `Path::is_dir` check returns `"shell: {cwd}: no such directory"`; one `io::pipe` with a `try_clone`'d second write end (stdout gets `writer`, stderr gets `writer2`); `Command::new(SHELL).arg("-c").arg(command).current_dir(cwd)` with null stdin, `spawn`, then `drop(command)` so the parent holds no write end and reading reaches EOF (the hang fix); an 8 KiB-buffer read loop keeping the first `MAX_OUTPUT` bytes and draining+discarding the rest with a `dropped` flag, retrying `Interrupted`, decoding with `from_utf8_lossy`; `wait()` then appending the dropped line and the exit/signal/none status in that order via a `match` on `code()`/`signal()` → `Outcome::done`. `lib.rs` gained `pub mod shell` (after `input`) and a `"shell"` arm parsing `ShellArgs` like the other two tools. One logic bug caught by the limit test: when the buffer fills `MAX_OUTPUT` exactly the drain branch never set `dropped`, fixed by marking `dropped` in the drain branch. First gate failed on an unused `Write` import (removed); 8 shell tests pass five runs, `make gate` prints `gate: ok`. | ? |
 | M3b/04-toolkit-files | 2026-09-22 | done | 1 | pass | none | Wrote `crates/toolkit/src/input.rs` (`MAX_INPUT`, `InputError` with hand-written `Display` + `std::error::Error` + `From<io::Error>`, `read_input` reading at most `MAX_INPUT+1` bytes via `Read::take` and flagging `TooLarge`, `parse` returning the whole stderr line) and `crates/toolkit/src/files.rs` (`MAX_READ`, `read_file` and `write_file`). `read_file` walks the exits in order: metadata NotFound→"no such file", other metadata error→Display text, directory→"is a directory", open failure→text, `take(MAX_READ+1)` read failure→text, over-limit→"larger than {MAX_READ} bytes", non-UTF-8→"not UTF-8 text", else `done`. `write_file` checks the parent is an existing directory before writing so no directory is ever created, then the path-is-directory case, then `fs::write`. Replaced the `lib.rs`/`main.rs` stubs: `Exit` via a `match` (no `as`), `Outcome` with `done`/`tool_error`/`misuse` (misuse stdout empty), `run` reading input before the name lookup for every name. `Cargo.toml` gained `serde` and `serde_json` workspace deps; `dependencies.md` lists `toolkit` under both. Copied the two given test files. `an_unreadable_file_names_the_error` ran for real (UID 1000, not root). 7 `files.rs` tests pass; `make gate` prints `gate: ok` on the first run. | ? |
 | M3b/02-brokerd-fetch-url | 2026-09-22 | done | 1 | pass | none | Sealed the fetch target: replaced the two-field `ToolArgs::HttpFetch { url, host }` variant with a tuple variant `HttpFetch(FetchUrl)` holding a new `FetchUrl { url, host }` struct whose fields are private and exposed only through `url()`/`host()`; `parse` is the only constructor. Added the two doctests word for word (a `compile_fail` proving the struct cannot be built outside `args`, and a passing one reading the parsed value through getters). Changed the four match sites that built or read the old variant: `tool` (`HttpFetch(_)`), `canonical_json` (`HttpFetch(target)` serialising `HttpFetchArgs { url: target.url.clone() }`, still only the URL), `parse` (`HttpFetch(FetchUrl { url: value.url, host })`), and `policy::covers` (`HttpFetch(target)` matching with `host_matches(pattern, target.host())`); `runner.rs` `HttpFetch { .. }` became `HttpFetch(_)`. Copied the new `tests/args.rs` (one test now reads the parsed value through getters). `grep "FetchUrl {"` shows the struct def, impl opener, the `parse` line and the doctest only; `grep "HttpFetch {"` shows nothing. All brokerd tests and doctests pass; `make gate` prints `gate: ok` on the first run. | ? |
 | M3b/01-proto-tools-hosts | 2026-09-22 | done | 1 | pass | none | Copied `tools.rs` and `hosts.rs` from the plan's `files/` into `crates/proto/tests/`; they fail to compile until the modules exist. Created `crates/proto/src/tools.rs` with the four public arg structs (ReadFile/Write/Shell/HttpFetch), each `#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]` and `#[serde(deny_unknown_fields)]`, `ShellArgs` keeping its `#[serde(default, skip_serializing_if = "Option::is_none")]` cwd; and `crates/proto/src/hosts.rs` with private `valid_label` plus `valid_host`, `valid_host_pattern`, `host_matches`, bodies and doc comments unchanged. Added `pub mod hosts;` and `pub mod tools;` to `proto/src/lib.rs` in alphabetical order (no re-exports). Deleted the private structs from `brokerd/src/args.rs`, added `use proto::tools::{...}` at the top and the `pub use proto::hosts::{...}` re-export where the functions were, and dropped the now-unused `use serde::{Deserialize, Serialize}`; `url_host`, `valid_path`, `inside` and `MAX_URL` stay. New suites 3 and 3 pass; brokerd args/grants/policy pass unchanged. `grep "fn valid_host\|struct ShellArgs"` shows one extra line, `brokerd/tests/args.rs:91`, the pre-existing test `valid_hosts_and_patterns` (substring match, not a duplicate definition). Gate passed on the first run. | ? |