Task 04 added dependencies to toolkit and its git add line left out the lock file, so the driver stopped on an unclean tree. The lock change is folded into task 04's commit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
4.3 KiB
M3b task 02: seal the fetch target
Branch: m3b (run git switch m3b; git status --short must be empty, otherwise stop)
Commit subject: Seal the fetch target: one value holds the URL and its host
Goal
Today ToolArgs::HttpFetch { url, host } has two public fields. Policy matches host against the
grant; from M3b on, the runtime fetches url. Any code could build one where the two disagree
(url of evil.example, host of example.com), and policy would allow a fetch of the wrong
host. Make that impossible: one sealed value, built only by args::parse after the URL checks.
This is M3a review finding 14.
Files
- Copy:
crates/brokerd/tests/args.rs(replaces the old one; one test now reads the parsed value through getters, because it can no longer build one) - Modify:
crates/brokerd/src/args.rs,crates/brokerd/src/policy.rs,crates/brokerd/src/runner.rs,docs/implementer-log.md
Interfaces
In args.rs, the variant becomes a tuple variant holding a new struct:
pub enum ToolArgs {
ReadFile { path: String },
WriteFile { path: String, content: String },
Shell { command: String, cwd: Option<String> },
HttpFetch(FetchUrl),
}
/// A URL that passed the checks, and its host. Only `parse` makes one, so the host policy matched
/// is always the host of the URL the tool fetches.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct FetchUrl {
url: String, // private
host: String, // private
}
impl FetchUrl {
pub fn url(&self) -> &str;
pub fn host(&self) -> &str;
}
No pub fn new, no Default, no Deserialize, no public field: parse (in the same module) is
the only code that writes FetchUrl { url, host }.
Put these two doctests in FetchUrl's doc comment, word for word. The first proves the struct
cannot be built outside args; the second proves the first fails for that reason and not a typo:
/// ```compile_fail
/// let _ = brokerd::args::FetchUrl {
/// url: "https://evil.example/".to_string(),
/// host: "example.com".to_string(),
/// };
/// ```
///
/// ```
/// let args = brokerd::args::parse(brokerd::args::ToolName::HttpFetch, r#"{"url":"https://example.com/a"}"#);
/// let Ok(brokerd::args::ToolArgs::HttpFetch(target)) = args else { panic!("{args:?}") };
/// assert_eq!((target.url(), target.host()), ("https://example.com/a", "example.com"));
/// ```
Every place that matched the old variant changes. There are four:
ToolArgs::tool:ToolArgs::HttpFetch(_) => ToolName::HttpFetch.ToolArgs::canonical_json:ToolArgs::HttpFetch(target) =>serialiseHttpFetchArgs { url: target.url.clone() }(still only the URL: the host is not an argument).parse'sToolName::HttpFetcharm:Ok(ToolArgs::HttpFetch(FetchUrl { url: value.url, host })).policy.rs, incovers:ToolArgs::HttpFetch(target) =>and match the grant's patterns withhost_matches(pattern, target.host()).
In runner.rs: ToolArgs::HttpFetch { .. } becomes ToolArgs::HttpFetch(_).
Search for any other: grep -rn "HttpFetch {" crates/ must show nothing when you are done.
Steps
- 1. Copy.
git switch m3b, thencp docs/plans/M3b/files/crates/brokerd/tests/args.rs crates/brokerd/tests/ - 2. See it fail.
cargo test -p brokerd --test args. Expected: it does not compile (ToolArgs::HttpFetch(target)is not a tuple variant yet). - 3. Make the change. Run
cargo fmt --all. - 4. See it pass.
cargo test -p brokerd --test args --test policy --test runner. Expected:args13 passed; the others pass unchanged.cargo test -p brokerd --doc: every doctest passes, including your two new ones (compile_failcounts as passing when it fails to compile). - 5. Check the seal.
grep -rn "FetchUrl {" crates/brokerd/src/shows the struct definition, theparseline, and the doctest only. - 6. Run the gate.
make gate. Expected last line:gate: ok. - 7. Log and commit.
git add crates/brokerd docs/implementer-log.md Cargo.lock && git commit
Done when
cargo test -p brokerdandcargo test -p brokerd --docpass;make gateprintsgate: ok.
Stop and report if
- Some code outside
args.rsneeds to build aFetchUrl: that would break the seal. - A test other than
args.rsneeds a change.