diff --git a/crates/toolkit/src/lib.rs b/crates/toolkit/src/lib.rs index 65a221e..32ca4e2 100644 --- a/crates/toolkit/src/lib.rs +++ b/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::(name, &text) { + Ok(a) => a, + Err(e) => return Outcome::misuse(e), + }; + shell::shell(&args) + } _ => Outcome::misuse(format!("toolkit: unknown tool {name:?}")), } } diff --git a/crates/toolkit/src/shell.rs b/crates/toolkit/src/shell.rs new file mode 100644 index 0000000..454f99c --- /dev/null +++ b/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 = 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) +} diff --git a/crates/toolkit/tests/shell.rs b/crates/toolkit/tests/shell.rs new file mode 100644 index 0000000..7f821eb --- /dev/null +++ b/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]"); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 0d897c0..8c73d31 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/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`, `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. | ? |