diff --git a/crates/brokerd/src/args.rs b/crates/brokerd/src/args.rs index 657b9c6..1c0a381 100644 --- a/crates/brokerd/src/args.rs +++ b/crates/brokerd/src/args.rs @@ -4,7 +4,7 @@ //! path, a host or a URL is well formed. The module is pure: no I/O, no clock. Everything it reads //! was written by the model, so it is treated as hostile. -use serde::{Deserialize, Serialize}; +use proto::tools::{HttpFetchArgs, ReadFileArgs, ShellArgs, WriteFileArgs}; use std::fmt; /// Maximum length, in bytes, of a path. @@ -142,34 +142,6 @@ impl std::error::Error for ArgsError { } } -/// The argument shape for one tool, decoded then re-serialised. -#[derive(Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -struct ReadFileArgs { - path: String, -} - -#[derive(Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -struct WriteFileArgs { - path: String, - content: String, -} - -#[derive(Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -struct ShellArgs { - command: String, - #[serde(default, skip_serializing_if = "Option::is_none")] - cwd: Option, -} - -#[derive(Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -struct HttpFetchArgs { - url: String, -} - /// Decode `arguments` for `tool` into a typed value. pub fn parse(tool: ToolName, arguments: &str) -> Result { match tool { @@ -257,64 +229,8 @@ pub fn inside(grant_path: &str, path: &str) -> bool { } } -/// A host label is 1 to 63 bytes of `a-z`, `0-9` or `-`, and neither starts nor ends with `-`. -fn valid_label(label: &str) -> bool { - if !(1..=63).contains(&label.len()) { - return false; - } - let bytes = label.as_bytes(); - if bytes.iter().next() == Some(&b'-') || bytes.iter().last() == Some(&b'-') { - return false; - } - bytes - .iter() - .all(|&byte| matches!(byte, b'a'..=b'z' | b'0'..=b'9' | b'-')) -} - -/// A host is 1 to 253 bytes of dot-separated labels, each a whole non-dot name, and the last label -/// starts with a letter. That last rule keeps out every spelling of an IPv4 address. -pub fn valid_host(host: &str) -> bool { - if !(1..=253).contains(&host.len()) { - return false; - } - let labels: Vec<&str> = host.split('.').collect(); - if labels.len() < 2 { - return false; - } - for label in &labels { - if !valid_label(label) { - return false; - } - } - match labels.iter().last() { - Some(last) => matches!(last.bytes().next(), Some(byte) if byte.is_ascii_lowercase()), - None => false, - } -} - -/// A host, or `*.` followed by a host. Nothing else. -pub fn valid_host_pattern(pattern: &str) -> bool { - match pattern.strip_prefix("*.") { - Some(base) => valid_host(base), - None => valid_host(pattern), - } -} - -/// Does `pattern` match `host`? Without `*.` the strings must be equal; with `*.base` the host must -/// end in `.base` with something before the dot. -pub fn host_matches(pattern: &str, host: &str) -> bool { - let base = match pattern.strip_prefix("*.") { - Some(base) => base, - None => return pattern == host, - }; - match host.strip_suffix(base) { - Some(before) => match before.strip_suffix('.') { - Some(prefix) => !prefix.is_empty(), - None => false, - }, - None => false, - } -} +/// Moved to `proto::hosts` in M3b, so `toolkit` checks hosts with the same rules. +pub use proto::hosts::{host_matches, valid_host, valid_host_pattern}; /// A character that may appear in a host name. fn host_char(c: char) -> bool { diff --git a/crates/proto/src/hosts.rs b/crates/proto/src/hosts.rs new file mode 100644 index 0000000..54f2ed8 --- /dev/null +++ b/crates/proto/src/hosts.rs @@ -0,0 +1,61 @@ +//! Host names and patterns, shared by `brokerd` and the egress proxy so both check hosts with the +//! same rules. + +/// A host label is 1 to 63 bytes of `a-z`, `0-9` or `-`, and neither starts nor ends with `-`. +fn valid_label(label: &str) -> bool { + if !(1..=63).contains(&label.len()) { + return false; + } + let bytes = label.as_bytes(); + if bytes.iter().next() == Some(&b'-') || bytes.iter().last() == Some(&b'-') { + return false; + } + bytes + .iter() + .all(|&byte| matches!(byte, b'a'..=b'z' | b'0'..=b'9' | b'-')) +} + +/// A host is 1 to 253 bytes of dot-separated labels, each a whole non-dot name, and the last label +/// starts with a letter. That last rule keeps out every spelling of an IPv4 address. +pub fn valid_host(host: &str) -> bool { + if !(1..=253).contains(&host.len()) { + return false; + } + let labels: Vec<&str> = host.split('.').collect(); + if labels.len() < 2 { + return false; + } + for label in &labels { + if !valid_label(label) { + return false; + } + } + match labels.iter().last() { + Some(last) => matches!(last.bytes().next(), Some(byte) if byte.is_ascii_lowercase()), + None => false, + } +} + +/// A host, or `*.` followed by a host. Nothing else. +pub fn valid_host_pattern(pattern: &str) -> bool { + match pattern.strip_prefix("*.") { + Some(base) => valid_host(base), + None => valid_host(pattern), + } +} + +/// Does `pattern` match `host`? Without `*.` the strings must be equal; with `*.base` the host must +/// end in `.base` with something before the dot. +pub fn host_matches(pattern: &str, host: &str) -> bool { + let base = match pattern.strip_prefix("*.") { + Some(base) => base, + None => return pattern == host, + }; + match host.strip_suffix(base) { + Some(before) => match before.strip_suffix('.') { + Some(prefix) => !prefix.is_empty(), + None => false, + }, + None => false, + } +} diff --git a/crates/proto/src/lib.rs b/crates/proto/src/lib.rs index c3ecc99..082b504 100644 --- a/crates/proto/src/lib.rs +++ b/crates/proto/src/lib.rs @@ -6,8 +6,10 @@ pub mod class; pub mod frame; pub mod grant; pub mod hash; +pub mod hosts; pub mod ids; pub mod log; +pub mod tools; pub mod wire; pub use audit::{ diff --git a/crates/proto/src/tools.rs b/crates/proto/src/tools.rs new file mode 100644 index 0000000..f7a7af3 --- /dev/null +++ b/crates/proto/src/tools.rs @@ -0,0 +1,36 @@ +//! The four tools' arguments, decoded from the model's JSON then re-serialised. Shared by +//! `brokerd` (which validates them) and `toolkit` (inside the container, which runs them), so the +//! two must agree on one definition. + +use serde::{Deserialize, Serialize}; + +/// The arguments for `read_file`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ReadFileArgs { + pub path: String, +} + +/// The arguments for `write_file`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct WriteFileArgs { + pub path: String, + pub content: String, +} + +/// The arguments for `shell`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ShellArgs { + pub command: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cwd: Option, +} + +/// The arguments for `http_fetch`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct HttpFetchArgs { + pub url: String, +} diff --git a/crates/proto/tests/hosts.rs b/crates/proto/tests/hosts.rs new file mode 100644 index 0000000..817472d --- /dev/null +++ b/crates/proto/tests/hosts.rs @@ -0,0 +1,50 @@ +//! Host names and patterns, shared by `brokerd` and `toolkit`'s egress proxy. The full tables are +//! in `crates/brokerd/tests/args.rs`, which reaches these through `brokerd::args`. Do not edit. + +use proto::hosts::{host_matches, valid_host, valid_host_pattern}; + +#[test] +fn hosts() { + for good in ["example.com", "a.b.example.com", "x-1.example.org", "a.b"] { + assert!(valid_host(good), "{good}"); + } + for bad in [ + "", + "example", + "Example.com", + "-a.com", + "a-.com", + "a..com", + ".a.com", + "a.com.", + "127.0.0.1", + "127.1", + "1.2.3.4x", + "[::1]", + "a_b.com", + "a.com:443", + "*.a.com", + ] { + assert!(!valid_host(bad), "{bad}"); + } +} + +#[test] +fn patterns() { + assert!(valid_host_pattern("example.com")); + assert!(valid_host_pattern("*.example.com")); + for bad in ["*", "*.", "*.*.a.com", "a.*.com", "**.a.com", "*a.com"] { + assert!(!valid_host_pattern(bad), "{bad}"); + } +} + +#[test] +fn matching() { + assert!(host_matches("example.com", "example.com")); + assert!(!host_matches("example.com", "www.example.com")); + assert!(host_matches("*.example.com", "www.example.com")); + assert!(host_matches("*.example.com", "a.b.example.com")); + assert!(!host_matches("*.example.com", "example.com")); + assert!(!host_matches("*.example.com", "badexample.com")); + assert!(!host_matches("*.example.com", ".example.com")); +} diff --git a/crates/proto/tests/tools.rs b/crates/proto/tests/tools.rs new file mode 100644 index 0000000..33ad148 --- /dev/null +++ b/crates/proto/tests/tools.rs @@ -0,0 +1,60 @@ +//! The four tools' arguments: `brokerd` writes them to a container's standard input, `toolkit` +//! reads them back, and both use these types. Do not edit. + +use proto::tools::{HttpFetchArgs, ReadFileArgs, ShellArgs, WriteFileArgs}; + +#[test] +fn each_type_round_trips_in_field_order() { + let read = ReadFileArgs { + path: "/n/a.md".to_string(), + }; + assert_eq!( + serde_json::to_string(&read).unwrap(), + r#"{"path":"/n/a.md"}"# + ); + let write = WriteFileArgs { + path: "/n/a.md".to_string(), + content: "hi\n".to_string(), + }; + let text = serde_json::to_string(&write).unwrap(); + assert_eq!(text, r#"{"path":"/n/a.md","content":"hi\n"}"#); + assert_eq!(serde_json::from_str::(&text).unwrap(), write); + let fetch = HttpFetchArgs { + url: "https://example.com/".to_string(), + }; + assert_eq!( + serde_json::to_string(&fetch).unwrap(), + r#"{"url":"https://example.com/"}"# + ); +} + +#[test] +fn a_shell_cwd_is_left_out_when_absent_and_may_be_null_or_missing() { + let bare = ShellArgs { + command: "ls".to_string(), + cwd: None, + }; + assert_eq!(serde_json::to_string(&bare).unwrap(), r#"{"command":"ls"}"#); + for text in [r#"{"command":"ls"}"#, r#"{"command":"ls","cwd":null}"#] { + assert_eq!(serde_json::from_str::(text).unwrap(), bare); + } + let with = ShellArgs { + command: "ls".to_string(), + cwd: Some("/n".to_string()), + }; + assert_eq!( + serde_json::to_string(&with).unwrap(), + r#"{"command":"ls","cwd":"/n"}"# + ); +} + +#[test] +fn unknown_and_missing_fields_are_refused() { + assert!(serde_json::from_str::(r#"{"path":"/a","mode":"x"}"#).is_err()); + assert!(serde_json::from_str::(r#"{}"#).is_err()); + assert!(serde_json::from_str::(r#"{"path":"/a"}"#).is_err()); + assert!(serde_json::from_str::(r#"{"command":"ls","env":{}}"#).is_err()); + assert!( + serde_json::from_str::(r#"{"url":"https://a.b/","method":"POST"}"#).is_err() + ); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 0d65628..376a2a8 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/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. | ? | | M3a/23-review-fixes | 2026-09-22 | done | 1 | pass | none | Done by the design model, not an implementer run: Ornith was under heavy contention. Findings 1 to 13 and 16 fixed. Tests that failed without their fix cover 1, 2, 3, 4, 7, 8, 9, 10 and 16; two more (a torn first record, an empty later file) passed before the fix and are regression guards; 6, 11 and 13 were checked by reading. 14 moves to M3b's first task; of 15, the log-name rule, `MAX_PATH`'s doc and the unused push are fixed, and the two that change the baseline wait for an epoch change. The server expectations in the device tests were updated to the new layout. | Claude Opus 5.5 | | M3a/22-end-to-end | 2026-09-22 | done | 1 | pass | none | Copied `end_to_end.rs`, `Makefile` (workspace build then ignored e2e with `BOXMAKER_BROKERD`) and `tools/check-m3a-device.sh`. Without the variable: 0 passed, 1 ignored. With it: 1 passed five runs. `--ignored` without the variable fails with "set BOXMAKER_BROKERD". `make gate` ran the e2e line (`1 passed`) and prints `gate: ok`. Did not run the straylight script. | Grok 4.6 | | M3a/21-runbook-check | 2026-09-22 | done | 1 | pass | none | Wrote `scripts/check-runbook.sh`: find `*.rs` under crates (prune `target/`), awk out every `docs/runbook.md#` pointer, empty anchors fail, each remaining anchor must match a whole `## ` line, every missing one is reported with its files, then one exit. Step 5: dropping `-x` from grep failed with "the entry is the whole line, at level two"; `exit 1` at a missing-anchor report failed with "both missing entries and their files are reported". Real tree exits 0. `make gate` prints `gate: ok`. | Grok 4.6 |