Move the tools' arguments and the host rules to proto
Implemented-By: OpenCode session (model recorded in docs/implementer-log.md)
This commit is contained in:
@@ -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
|
//! 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.
|
//! 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;
|
use std::fmt;
|
||||||
|
|
||||||
/// Maximum length, in bytes, of a path.
|
/// 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<String>,
|
|
||||||
}
|
|
||||||
|
|
||||||
#[derive(Serialize, Deserialize)]
|
|
||||||
#[serde(deny_unknown_fields)]
|
|
||||||
struct HttpFetchArgs {
|
|
||||||
url: String,
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Decode `arguments` for `tool` into a typed value.
|
/// Decode `arguments` for `tool` into a typed value.
|
||||||
pub fn parse(tool: ToolName, arguments: &str) -> Result<ToolArgs, ArgsError> {
|
pub fn parse(tool: ToolName, arguments: &str) -> Result<ToolArgs, ArgsError> {
|
||||||
match tool {
|
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 `-`.
|
/// Moved to `proto::hosts` in M3b, so `toolkit` checks hosts with the same rules.
|
||||||
fn valid_label(label: &str) -> bool {
|
pub use proto::hosts::{host_matches, valid_host, valid_host_pattern};
|
||||||
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,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// A character that may appear in a host name.
|
/// A character that may appear in a host name.
|
||||||
fn host_char(c: char) -> bool {
|
fn host_char(c: char) -> bool {
|
||||||
|
|||||||
@@ -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,
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -6,8 +6,10 @@ pub mod class;
|
|||||||
pub mod frame;
|
pub mod frame;
|
||||||
pub mod grant;
|
pub mod grant;
|
||||||
pub mod hash;
|
pub mod hash;
|
||||||
|
pub mod hosts;
|
||||||
pub mod ids;
|
pub mod ids;
|
||||||
pub mod log;
|
pub mod log;
|
||||||
|
pub mod tools;
|
||||||
pub mod wire;
|
pub mod wire;
|
||||||
|
|
||||||
pub use audit::{
|
pub use audit::{
|
||||||
|
|||||||
@@ -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<String>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The arguments for `http_fetch`.
|
||||||
|
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||||
|
#[serde(deny_unknown_fields)]
|
||||||
|
pub struct HttpFetchArgs {
|
||||||
|
pub url: String,
|
||||||
|
}
|
||||||
@@ -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"));
|
||||||
|
}
|
||||||
@@ -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::<WriteFileArgs>(&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::<ShellArgs>(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::<ReadFileArgs>(r#"{"path":"/a","mode":"x"}"#).is_err());
|
||||||
|
assert!(serde_json::from_str::<ReadFileArgs>(r#"{}"#).is_err());
|
||||||
|
assert!(serde_json::from_str::<WriteFileArgs>(r#"{"path":"/a"}"#).is_err());
|
||||||
|
assert!(serde_json::from_str::<ShellArgs>(r#"{"command":"ls","env":{}}"#).is_err());
|
||||||
|
assert!(
|
||||||
|
serde_json::from_str::<HttpFetchArgs>(r#"{"url":"https://a.b/","method":"POST"}"#).is_err()
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -6,6 +6,7 @@ reviewer adds findings under "Reviews" once per milestone.
|
|||||||
|
|
||||||
| Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model |
|
| 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/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/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 `## <anchor>` 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 |
|
| 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 `## <anchor>` 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 |
|
||||||
|
|||||||
Reference in New Issue
Block a user