From adf67138664645f5fbfc21804c1e299e5d460137 Mon Sep 17 00:00:00 2001 From: "K. Isom" Date: Wed, 23 Sep 2026 00:14:16 -0700 Subject: [PATCH] toolkit: http_fetch through curl Implemented-By: OpenCode session (model recorded in docs/implementer-log.md) --- crates/toolkit/src/fetch.rs | 146 ++++++++++++++++++++++++++++++++++ crates/toolkit/src/lib.rs | 10 ++- crates/toolkit/tests/fetch.rs | 146 ++++++++++++++++++++++++++++++++++ docs/implementer-log.md | 1 + 4 files changed, 302 insertions(+), 1 deletion(-) create mode 100644 crates/toolkit/src/fetch.rs create mode 100644 crates/toolkit/tests/fetch.rs diff --git a/crates/toolkit/src/fetch.rs b/crates/toolkit/src/fetch.rs new file mode 100644 index 0000000..7d50772 --- /dev/null +++ b/crates/toolkit/src/fetch.rs @@ -0,0 +1,146 @@ +//! The fourth tool, `http_fetch`: run `/bin/curl` with a fixed argument list against `args.url`, +//! through the egress proxy's socket `brokerd` mounts for this call, and report what curl printed +//! and how it ended. toolkit writes no HTTP or TLS client of its own; curl does that inside the +//! container. Spec sections 4 and 6. + +use std::io::{self, Read}; +use std::path::Path; +use std::process::{Command, Stdio}; + +use proto::tools::HttpFetchArgs; + +use crate::Outcome; + +pub const CURL: &str = "/bin/curl"; +pub const PROXY: &str = "socks5h://localhost/run/egress/egress.sock"; +pub const CA_BUNDLE: &str = "/etc/ssl/certs/ca-certificates.crt"; + +/// The most to keep from curl's standard error. curl can still run past it; the rest is drained +/// and discarded, never shown. +const MAX_STDERR: usize = 64 * 1024; + +/// `curl`'s arguments for `url`, in order, without the program name. +pub fn curl_args(url: &str) -> Vec { + vec![ + "--silent".to_string(), + "--show-error".to_string(), + "--proto".to_string(), + "=https".to_string(), + "--proto-redir".to_string(), + "=https".to_string(), + "--location".to_string(), + "--max-redirs".to_string(), + "5".to_string(), + "--max-time".to_string(), + "50".to_string(), + "--max-filesize".to_string(), + "8388608".to_string(), + "--cacert".to_string(), + CA_BUNDLE.to_string(), + "--proxy".to_string(), + PROXY.to_string(), + "--write-out".to_string(), + "\n[http %{response_code}]".to_string(), + "--url".to_string(), + url.to_string(), + ] +} + +pub fn fetch(args: &HttpFetchArgs) -> Outcome { + fetch_with(Path::new(CURL), args) +} + +pub fn fetch_with(curl: &Path, args: &HttpFetchArgs) -> Outcome { + let url = &args.url; + let args = curl_args(url); + + let mut command = Command::new(curl); + command + .args(args) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let mut child = match command.spawn() { + Ok(c) => c, + Err(e) => { + return Outcome::tool_error(format!( + "http_fetch: cannot start {}: {e}", + curl.display() + )); + } + }; + + // Read standard error on its own thread while the main thread reads standard output to the + // end: two full pipes block each other, and the given test fills both. + let stderr_reader = match child.stderr.take() { + Some(r) => r, + None => return Outcome::tool_error("http_fetch: curl has no standard error".to_string()), + }; + let stderr_handle = std::thread::spawn(move || read_capped(stderr_reader)); + + let mut body = Vec::new(); + let mut stdout = match child.stdout.take() { + Some(r) => r, + None => return Outcome::tool_error("http_fetch: curl has no standard output".to_string()), + }; + if let Err(e) = stdout.read_to_end(&mut body) { + return Outcome::tool_error(format!("http_fetch: cannot read curl output: {e}")); + } + + let status = match child.wait() { + Ok(s) => s, + Err(e) => return Outcome::tool_error(format!("http_fetch: cannot wait for curl: {e}")), + }; + // A panicked reader lost its data; the body and status are still what curl gave us. + let stderr_bytes = stderr_handle.join().unwrap_or_default(); + + if status.success() { + return Outcome::done(String::from_utf8_lossy(&body).into_owned()); + } + + let why = match first_non_blank_line(&stderr_bytes) { + Some(line) => line, + None => match status.code() { + Some(code) => format!("curl exited {code}"), + None => "curl was killed".to_string(), + }, + }; + Outcome::tool_error(format!("http_fetch: {url}: {why}")) +} + +/// Read `reader` to the end, keeping at most `MAX_STDERR` bytes; past the cap it keeps draining so +/// curl never blocks on a full pipe, but the rest is discarded. +fn read_capped(mut reader: impl Read) -> Vec { + let mut buf = [0u8; 8 * 1024]; + let mut out: Vec = Vec::new(); + loop { + let n = match reader.read(&mut buf) { + Ok(0) => break, + Ok(n) => n, + Err(e) if e.kind() == io::ErrorKind::Interrupted => continue, + Err(_) => break, + }; + if out.len() >= MAX_STDERR { + continue; + } + let take = n.min(MAX_STDERR - out.len()); + match buf.get(..take) { + Some(chunk) => out.extend_from_slice(chunk), + None => break, + } + } + out +} + +/// The first line of `bytes` that is not blank after trimming, or `None` if every line is blank or +/// the bytes are not UTF-8 text. +fn first_non_blank_line(bytes: &[u8]) -> Option { + let text = String::from_utf8_lossy(bytes); + for line in text.lines() { + let trimmed = line.trim(); + if !trimmed.is_empty() { + return Some(trimmed.to_string()); + } + } + None +} diff --git a/crates/toolkit/src/lib.rs b/crates/toolkit/src/lib.rs index 32ca4e2..f5ce16e 100644 --- a/crates/toolkit/src/lib.rs +++ b/crates/toolkit/src/lib.rs @@ -1,7 +1,8 @@ //! The programs that run inside tool containers. -use proto::tools::{ReadFileArgs, ShellArgs, WriteFileArgs}; +use proto::tools::{HttpFetchArgs, ReadFileArgs, ShellArgs, WriteFileArgs}; +pub mod fetch; pub mod files; pub mod input; pub mod shell; @@ -85,6 +86,13 @@ pub fn run(name: &str, stdin: &mut dyn std::io::Read) -> Outcome { }; shell::shell(&args) } + "http_fetch" => { + let args = match input::parse::(name, &text) { + Ok(a) => a, + Err(e) => return Outcome::misuse(e), + }; + fetch::fetch(&args) + } _ => Outcome::misuse(format!("toolkit: unknown tool {name:?}")), } } diff --git a/crates/toolkit/tests/fetch.rs b/crates/toolkit/tests/fetch.rs new file mode 100644 index 0000000..a95978f --- /dev/null +++ b/crates/toolkit/tests/fetch.rs @@ -0,0 +1,146 @@ +//! `http_fetch`: the fixed `curl` arguments, and what `toolkit` makes of `curl`'s answer, against +//! fake `curl` scripts. Do not edit. + +mod support; + +use std::os::unix::fs::PermissionsExt; +use std::path::PathBuf; +use std::sync::{Mutex, MutexGuard}; + +use proto::tools::HttpFetchArgs; +use support::TempDir; +use toolkit::Exit; +use toolkit::fetch::{curl_args, fetch_with}; + +const URL: &str = "https://example.com/a?b=c"; + +/// Every test that starts a process takes its turn. Otherwise another test's fork can hold +/// the script open for writing at the moment it is run, and running it fails with "text file +/// busy" (ETXTBSY), which has nothing to do with the code under test. +static SERIAL: Mutex<()> = Mutex::new(()); + +fn serial() -> MutexGuard<'static, ()> { + SERIAL.lock().unwrap_or_else(|p| p.into_inner()) +} + +fn fake_curl(dir: &TempDir, body: &str) -> PathBuf { + let path = dir.path().join("curl"); + std::fs::write(&path, format!("#!/bin/sh\n{body}\n")).unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap(); + path +} + +fn args() -> HttpFetchArgs { + HttpFetchArgs { + url: URL.to_string(), + } +} + +#[test] +fn the_argument_list_is_fixed_and_ends_with_the_url() { + let expected: Vec<&str> = vec![ + "--silent", + "--show-error", + "--proto", + "=https", + "--proto-redir", + "=https", + "--location", + "--max-redirs", + "5", + "--max-time", + "50", + "--max-filesize", + "8388608", + "--cacert", + "/etc/ssl/certs/ca-certificates.crt", + "--proxy", + "socks5h://localhost/run/egress/egress.sock", + "--write-out", + "\n[http %{response_code}]", + "--url", + URL, + ]; + assert_eq!(curl_args(URL), expected); +} + +#[test] +fn curl_is_given_exactly_those_arguments() { + let _serial = serial(); + let dir = TempDir::new("fetch-args"); + let curl = fake_curl(&dir, r#"for a in "$@"; do printf '%s|' "$a"; done"#); + let got = fetch_with(&curl, &args()); + assert_eq!(got.exit, Exit::Done); + let expected: String = curl_args(URL).iter().map(|a| format!("{a}|")).collect(); + assert_eq!(got.stdout, expected); +} + +#[test] +fn a_successful_fetch_is_the_body_and_the_status_line() { + let _serial = serial(); + let dir = TempDir::new("fetch-ok"); + let curl = fake_curl(&dir, r#"printf 'hello\n\n[http 404]'"#); + let got = fetch_with(&curl, &args()); + assert_eq!( + (got.exit, got.stdout.as_str()), + (Exit::Done, "hello\n\n[http 404]") + ); +} + +#[test] +fn a_failed_fetch_is_exit_1_with_curls_first_error_line() { + let _serial = serial(); + let dir = TempDir::new("fetch-fail"); + let curl = fake_curl( + &dir, + "printf 'partial'; printf '\\ncurl: (97) cannot complete SOCKS5 connection to evil.test. (2)\\nmore\\n' >&2; exit 97", + ); + let got = fetch_with(&curl, &args()); + assert_eq!(got.exit, Exit::ToolError); + assert_eq!( + got.stdout, + format!( + "http_fetch: {URL}: curl: (97) cannot complete SOCKS5 connection to evil.test. (2)" + ) + ); +} + +#[test] +fn a_failure_without_a_message_names_the_exit() { + let _serial = serial(); + let dir = TempDir::new("fetch-quiet"); + let curl = fake_curl(&dir, "exit 28"); + let got = fetch_with(&curl, &args()); + assert_eq!( + (got.exit, got.stdout.as_str()), + ( + Exit::ToolError, + "http_fetch: https://example.com/a?b=c: curl exited 28" + ) + ); +} + +#[test] +fn a_curl_that_cannot_start_is_exit_1() { + let _serial = serial(); + let got = fetch_with(&PathBuf::from("/no/such/curl"), &args()); + assert_eq!(got.exit, Exit::ToolError); + assert!( + got.stdout + .starts_with("http_fetch: cannot start /no/such/curl: "), + "{}", + got.stdout + ); +} + +#[test] +fn much_output_on_both_streams_does_not_stop_curl() { + let _serial = serial(); + let dir = TempDir::new("fetch-both"); + let curl = fake_curl( + &dir, + "head -c 300000 /dev/zero | tr '\\0' e >&2; head -c 300000 /dev/zero | tr '\\0' o; exit 0", + ); + let got = fetch_with(&curl, &args()); + assert_eq!((got.exit, got.stdout.len()), (Exit::Done, 300000)); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 8c73d31..768341c 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/06-toolkit-fetch | 2026-09-23 | done | 3 | fail | none | Wrote `crates/toolkit/src/fetch.rs`: `CURL=/bin/curl`, `PROXY=socks5h://localhost/run/egress/egress.sock`, `CA_BUNDLE=/etc/ssl/certs/ca-certificates.crt`, `MAX_STDERR=65536`. `curl_args(url)` returns the 21 fixed strings in order ending in `--url `. `fetch` calls `fetch_with(Path::new(CURL), args)`. `fetch_with`: spawn curl with `curl_args(&args.url)`, null stdin, stdout/stderr piped (spawn failure -> `"http_fetch: cannot start {}: {e}"`); read stderr on its own thread via the `read_capped` helper (8 KiB buffer, keeps the first 64 KiB and drains/discards the rest so curl never blocks on a full pipe), main thread reads stdout to the end; `wait()` then join the thread (a panicked reader falls back to empty stderr); exit 0 -> `done(from_utf8_lossy(body))` with the `--write-out` status line already in it, else the first non-blank trimmed stderr line as `why` or `"curl exited {code}"` or `"curl was killed"` when there is no code, wrapped as `"http_fetch: {url}: {why}"`, and a wait failure -> `"http_fetch: cannot wait for curl: {e}"`. `lib.rs` gained `pub mod fetch` (between `files` and `input`) and an `"http_fetch"` arm parsing `HttpFetchArgs` like the other three tools. First gate run failed on a rustfmt line-wrap of the spawn-error return; second on clippy `manual_unwrap_or_default` for the stderr-join match, switched to `unwrap_or_default()`. 7 fetch tests pass ten runs; `make gate` prints `gate: ok`. | ? | | 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. | ? |