From 77a7e8854b29344b3fc4658448100868db19a46b Mon Sep 17 00:00:00 2001 From: "K. Isom" Date: Mon, 28 Sep 2026 17:39:39 -0700 Subject: [PATCH] M5c-2 plan: task 02, brokerd knows ask_memory; the outline split to nine tasks Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/plans/M5c2/01-wire-ask-memory.md | 6 +- docs/plans/M5c2/02-brokerd-ask-memory.md | 462 +++++++++++++++++ docs/plans/M5c2/README.md | 21 +- .../crates/brokerd/tests/ask_memory_tool.rs | 340 ++++++++++++ .../M5c2/files/crates/brokerd/tests/grants.rs | 487 ++++++++++++++++++ .../crates/brokerd/tests/memory_tools.rs | 142 +++++ 6 files changed, 1446 insertions(+), 12 deletions(-) create mode 100644 docs/plans/M5c2/02-brokerd-ask-memory.md create mode 100644 docs/plans/M5c2/files/crates/brokerd/tests/ask_memory_tool.rs create mode 100644 docs/plans/M5c2/files/crates/brokerd/tests/grants.rs create mode 100644 docs/plans/M5c2/files/crates/brokerd/tests/memory_tools.rs diff --git a/docs/plans/M5c2/01-wire-ask-memory.md b/docs/plans/M5c2/01-wire-ask-memory.md index 5797150..73a78db 100644 --- a/docs/plans/M5c2/01-wire-ask-memory.md +++ b/docs/plans/M5c2/01-wire-ask-memory.md @@ -11,8 +11,8 @@ M5c-2 adds a core tool, `ask_memory`, that asks Honcho, a memory server shared b agents, a question about one or two of its workspaces (M5c spec, section 9). This task adds only the tool's arguments to `bxcore`, with their shape checks, the way `recall`'s are there (`RecallArgs`, its name constant `RECALL`, its limit `MAX_QUERY`, and `RecallArgs::check`). Later -tasks use them: task 02 (`brokerd` parses the arguments with them), task 03 (`loopd` declares the -tool), task 07 (`memoryd` answers it). +tasks use them: task 02 (`brokerd` parses the arguments with them), task 04 (`loopd` declares the +tool), tasks 08 and 09 (`memoryd` answers it). ```json {"question":"Who wrote the launch note?","workspaces":["fleet","boxmaker1"]} @@ -24,7 +24,7 @@ tool), task 07 (`memoryd` answers it). - **`workspaces`**: optional; at most two names, none empty, none repeated, each 1 to 512 characters of ASCII letters, digits, `_` and `-`. Missing, or `[]`, means the one workspace `own`, the agent's own trusted one. Whether a name is one this agent's `[honcho]` configuration - has is **not** checked here: `memoryd` checks that in task 07. + has is **not** checked here: `memoryd` checks that in task 08. - There is no other field, and an unknown field is refused, as in every argument type we define. (An earlier draft of the spec had a `reasoning` field; it was removed, and a given test sends it and expects a refusal.) diff --git a/docs/plans/M5c2/02-brokerd-ask-memory.md b/docs/plans/M5c2/02-brokerd-ask-memory.md new file mode 100644 index 0000000..444c4f5 --- /dev/null +++ b/docs/plans/M5c2/02-brokerd-ask-memory.md @@ -0,0 +1,462 @@ +# M5c-2 task 02: `brokerd` knows `ask_memory`, a built-in memory tool + +**Branch:** `m5c2`. Check it: `git branch --show-current` prints `m5c2` and `git status --short` +is empty; otherwise stop. Never switch branch. +**Commit subject:** `brokerd: ask_memory, a built-in memory tool that memoryd runs` +**Log row:** the Task cell is `M5c2/02-brokerd-ask-memory`. + +## Goal + +Task 01 gave `bxcore` the arguments of a new tool, `ask_memory` (`bxcore::AskMemoryArgs`, with +`check()`). This task makes `brokerd` accept it exactly as it accepts `recall` (M5c spec, +section 9): + +- **Its name.** `ToolName::AskMemory`, written `ask_memory`. It is a memory tool: + `is_memory()` is true, so `memoryd` runs it, never a container. +- **Its arguments.** `parse` decodes them into `ToolArgs::AskMemory(AskMemoryArgs)` and runs + `check()`; a refusal is `ArgsError::Shape` with `check`'s sentence, as for `recall`. The + canonical form is `serde_json::to_string` of the value, as for `recall`. +- **Its policy is built in.** A third built-in grant, `builtin-ask-memory`, allows it in every + session, `secret` and untrusted ones too: `mode = "auto"`, `max_taint = "secret"`, + `result_class = "private"`, `untrusted = false`. A question is not written to Honcho (spec, + section 8), and the answer's own labels, which `memoryd` gives, raise the session, as `recall`'s + do. An owner's grant file naming `ask_memory` is refused with the `BUILT_IN` sentence, which now + names the three tools. + +`brokerd`'s wait for `memoryd` stays 30 s for every memory tool in this task; task 03 gives +`ask_memory` its own. Nothing yet asks `brokerd` for `ask_memory`: `loopd` declares it in task 04 +and `memoryd` answers it from task 09. Until then, a call reaching `memoryd` is answered +`memoryd runs only recall and memory`, a failed result. + +## Every place that changes + +This is the complete list. + +| Place | What happens there | +|---|---| +| `crates/brokerd/src/args.rs` | `ToolName::AskMemory`, `ToolArgs::AskMemory`, and their arms in `parse`, `as_str`, `is_memory`, `tool`, `canonical_json` and `parse` (edits A1 to A9) | +| `crates/brokerd/src/builtin.rs` | the grant `ASK_MEMORY`, its id in `sources`, `BUILT_IN`'s sentence, two comments (edits B1, B2) | +| `crates/brokerd/src/config.rs`, `Runner::time_limit` | `AskMemory` beside `Recall` and `Memory` (edit C1) | +| `crates/brokerd/src/policy/matching.rs` | `ToolArgs::AskMemory` takes no constraints, as `Recall` (edit P1) | +| `crates/brokerd/src/runner.rs` | `ToolArgs::AskMemory` has no mounts and no egress, as `Recall` (edit R1) | +| `crates/brokerd/src/grants.rs` (rule 3), `container.rs` (`Podman::run`), `memory_port.rs` (`Routed`) | **no change**: each asks `tool.is_memory()`, which edit A5 makes true for `AskMemory` | +| `crates/brokerd/tests/ask_memory_tool.rs` | new, given | +| `crates/brokerd/tests/grants.rs`, `memory_tools.rs` | new versions, given: three built-in grants, not two | +| everything else | **no change**: not `bxcore`, not `loopd`, not `memoryd`, no `Cargo.toml` | + +## Files + +- Copy, from `docs/plans/M5c2/files/` (the same path under it): the new test + `crates/brokerd/tests/ask_memory_tool.rs`, and **new versions** of (they replace the files in + the tree) `crates/brokerd/tests/grants.rs` and `crates/brokerd/tests/memory_tools.rs`. +- Modify: `crates/brokerd/src/args.rs`, `crates/brokerd/src/builtin.rs`, + `crates/brokerd/src/config.rs`, `crates/brokerd/src/policy/matching.rs`, + `crates/brokerd/src/runner.rs`, `docs/implementer-log.md`. +- Do not touch: any other file. The test support files under `crates/brokerd/tests/support/` are + used as they are. + +## Facts you need + +- **`AskMemoryArgs`** (task 01, in `bxcore::memory`, exported as `bxcore::AskMemoryArgs`): + + ```rust + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] + #[serde(deny_unknown_fields)] + pub struct AskMemoryArgs { + pub question: String, + #[serde(default)] + pub workspaces: Vec, + } + impl AskMemoryArgs { + pub fn check(&self) -> Result<(), String>; + pub fn asked(&self) -> Vec<&str>; + } + ``` + + It is used here exactly as `RecallArgs` is: `serde_json::from_str`, then `check()`, each error + as `ArgsError::Shape`; `serde_json::to_string` for the canonical form. Its canonical form + writes every field: `{"question":"q","workspaces":[]}`. +- **The order of the built-in grants** is the order of `sources` in `builtin.rs`: `memory`, + `recall`, then the new `ask-memory` last. The given `grants.rs` lists them in that order. +- **Why the grant is `secret` and trusted.** `max_taint = "secret"` lets a `secret` session ask + (spec, section 8: a query is not stored by Honcho). `result_class = "private"` and `untrusted = + false` are the floor of the answer's label; `memoryd` labels each answer by the workspaces + asked, and `brokerd` keeps the stricter of the two (`runner::run`, unchanged). The given tests + send an `ask_memory` call through `broker::handle` to a fake `memoryd` that answers `untrusted` + or `secret`, and read the session's state afterwards. +- **The long lines.** The three grant constants are string literals longer than 100 characters; + `rustfmt` leaves them as they are, and the gate accepts them, as it does `RECALL` and `MEMORY` + today. Do not split them. +- The given tests: + - `ask_memory_tool.rs` (9 tests): the name, `is_memory`, parsing and the canonical form; each + refusal with `bxcore`'s sentence and its `detail()`; the built-in grant, its exact text and + its hash, allowing the call in a `private`, a `secret` and an untrusted session; an owner's + grant file refused with the new `BUILT_IN`; the call routed to `memoryd` with the session's + state and canonical arguments; a container runtime refusing it; and, through + `broker::handle`, an answer labelled untrusted making the session untrusted, one labelled + `secret` raising it to `secret`, and one labelled `public` leaving it `private`. + - `grants.rs`: one line changes, the list of built-in ids. `memory_tools.rs`: one line + changes, the number of grants `matchable` gives beside one owner grant (4, not 3). +- The items the edits use. This table is complete; nothing from a crate's source (the Cargo + registry, `~/.cargo/registry`) is needed or allowed. + + | Item | Where | + |---|---| + | `bxcore::AskMemoryArgs`, `AskMemoryArgs::check(&self) -> Result<(), String>` | A1, A9 | + | `serde_json::from_str`, `serde_json::to_string` | A8, A9, as the `Recall` arms beside them | + | `ArgsError::Shape(String)` | A9 | + +## The edits, exactly + +Nothing else in these files changes. Each quoted "before" block occurs exactly once in its file +when its turn comes (make them in order). Keep the variants in the order shown. + +### `crates/brokerd/src/args.rs` + +**A1.** Add one line. The line + +```rust +use bxcore::tools::{HttpFetchArgs, ReadFileArgs, ShellArgs, WriteFileArgs}; +``` + +becomes + +```rust +use bxcore::AskMemoryArgs; +use bxcore::tools::{HttpFetchArgs, ReadFileArgs, ShellArgs, WriteFileArgs}; +``` + +**A2.** The lines + +```rust +/// The tools the broker can dispatch to: four that run in a container, and the two memory +/// tools that `memoryd` runs (M5a spec, section 8). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ToolName { + ReadFile, + WriteFile, + Shell, + HttpFetch, + Recall, + Memory, +} +``` + +become + +```rust +/// The tools the broker can dispatch to: four that run in a container, and the three memory +/// tools that `memoryd` runs (M5a spec, section 8; M5c spec, section 9). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ToolName { + ReadFile, + WriteFile, + Shell, + HttpFetch, + Recall, + Memory, + AskMemory, +} +``` + +**A3.** In `ToolName::parse`, the lines (its spaces are checked: the name is `ask_memory`, with an +underscore) + +```rust + "memory" => Some(ToolName::Memory), + _ => None, +``` + +become + +```rust + "memory" => Some(ToolName::Memory), + "ask_memory" => Some(ToolName::AskMemory), + _ => None, +``` + +**A4.** In `ToolName::as_str`, the lines (its spaces are checked) + +```rust + ToolName::Memory => "memory", + } +``` + +become + +```rust + ToolName::Memory => "memory", + ToolName::AskMemory => "ask_memory", + } +``` + +**A5.** In `ToolName::is_memory`, the line + +```rust + matches!(self, ToolName::Recall | ToolName::Memory) +``` + +becomes + +```rust + matches!( + self, + ToolName::Recall | ToolName::Memory | ToolName::AskMemory + ) +``` + +**A6.** At the end of `enum ToolArgs`, the lines + +```rust + Recall(RecallArgs), + Memory(MemoryArgs), +} +``` + +become + +```rust + Recall(RecallArgs), + Memory(MemoryArgs), + AskMemory(AskMemoryArgs), +} +``` + +**A7.** In `ToolArgs::tool`, the line + +```rust + ToolArgs::Memory(_) => ToolName::Memory, +``` + +becomes + +```rust + ToolArgs::Memory(_) => ToolName::Memory, + ToolArgs::AskMemory(_) => ToolName::AskMemory, +``` + +**A8.** At the end of `ToolArgs::canonical_json`, the lines + +```rust + ToolArgs::Memory(value) => { + serde_json::to_string(value).unwrap_or_else(|_| "{}".to_string()) + } +``` + +become + +```rust + ToolArgs::Memory(value) => { + serde_json::to_string(value).unwrap_or_else(|_| "{}".to_string()) + } + ToolArgs::AskMemory(value) => { + serde_json::to_string(value).unwrap_or_else(|_| "{}".to_string()) + } +``` + +**A9.** At the end of `parse`, the lines + +```rust + Ok(ToolArgs::Memory(value)) + } +``` + +become + +```rust + Ok(ToolArgs::Memory(value)) + } + ToolName::AskMemory => { + let value: AskMemoryArgs = serde_json::from_str(arguments) + .map_err(|error| ArgsError::Shape(error.to_string()))?; + value.check().map_err(ArgsError::Shape)?; + Ok(ToolArgs::AskMemory(value)) + } +``` + +### `crates/brokerd/src/builtin.rs` + +**B1.** The module's doc comment, the lines + +```rust +//! The built-in grants of `recall` and `memory` (M5a spec, section 8). Reading and writing memory +//! is what an agent is, so no grant file can turn it off or put questions in front of it; the +//! prompt is protected by `memoryd`, which lets only clean sessions edit identity. +``` + +become + +```rust +//! The built-in grants of `recall`, `memory` and `ask_memory` (M5a spec, section 8; M5c spec, +//! section 9). Reading and writing memory is what an agent is, so no grant file can turn it off or +//! put questions in front of it; the prompt is protected by `memoryd`, which lets only clean +//! sessions edit identity. +``` + +**B2.** The lines (its spaces are checked: the grant's text is hashed into the audit log, and the +given test compares it and the sentence letter for letter; every `\n` and `\"` is as shown) + +```rust +pub const MEMORY: &str = "tool = \"memory\"\nmode = \"auto\"\nmax_taint = \"secret\"\nresult_class = \"private\"\nuntrusted = false\n"; + +/// The sentence for an owner's grant naming a memory tool. +pub const BUILT_IN: &str = "the policy of recall and memory is built in; remove this grant"; + +/// The two built-in grants, `builtin-memory` and `builtin-recall`. +pub fn grants() -> Result, GrantProblem> { + // For each (id, text) of [("builtin-memory", MEMORY), ("builtin-recall", RECALL)]: + // `toml::from_str::(text)` and `sha256(text.as_bytes())`, each error as a GrantProblem { + // file: id, line: None, problem: e.to_string() }; build LoadedGrant { id, grant, sha256 }. + // Collect into Result, GrantProblem> (the first error wins). + let sources: &[(&str, &str)] = &[("builtin-memory", MEMORY), ("builtin-recall", RECALL)]; +``` + +become + +```rust +pub const MEMORY: &str = "tool = \"memory\"\nmode = \"auto\"\nmax_taint = \"secret\"\nresult_class = \"private\"\nuntrusted = false\n"; +pub const ASK_MEMORY: &str = "tool = \"ask_memory\"\nmode = \"auto\"\nmax_taint = \"secret\"\nresult_class = \"private\"\nuntrusted = false\n"; + +/// The sentence for an owner's grant naming a memory tool. +pub const BUILT_IN: &str = + "the policy of recall, memory and ask_memory is built in; remove this grant"; + +/// The three built-in grants, `builtin-memory`, `builtin-recall` and `builtin-ask-memory`. +pub fn grants() -> Result, GrantProblem> { + // For each (id, text) of `sources`: + // `toml::from_str::(text)` and `sha256(text.as_bytes())`, each error as a GrantProblem { + // file: id, line: None, problem: e.to_string() }; build LoadedGrant { id, grant, sha256 }. + // Collect into Result, GrantProblem> (the first error wins). + let sources: &[(&str, &str)] = &[ + ("builtin-memory", MEMORY), + ("builtin-recall", RECALL), + ("builtin-ask-memory", ASK_MEMORY), + ]; +``` + +The id is `builtin-ask-memory`, with a hyphen; the tool in the grant's text is `ask_memory`, with +an underscore. + +### `crates/brokerd/src/config.rs` + +**C1.** In `Runner::time_limit`, the line + +```rust + crate::args::ToolName::Recall | crate::args::ToolName::Memory => self.read_file_ms, +``` + +becomes + +```rust + crate::args::ToolName::Recall + | crate::args::ToolName::Memory + | crate::args::ToolName::AskMemory => self.read_file_ms, +``` + +### `crates/brokerd/src/policy/matching.rs` + +**P1.** The line + +```rust + ToolArgs::Recall(_) | ToolArgs::Memory(_) => Some(None), +``` + +becomes + +```rust + ToolArgs::Recall(_) | ToolArgs::Memory(_) | ToolArgs::AskMemory(_) => Some(None), +``` + +### `crates/brokerd/src/runner.rs` + +**R1.** The line + +```rust + ToolArgs::Recall(_) | ToolArgs::Memory(_) => (Vec::new(), None), +``` + +becomes + +```rust + ToolArgs::Recall(_) | ToolArgs::Memory(_) | ToolArgs::AskMemory(_) => (Vec::new(), None), +``` + +## Steps + +- [ ] **1. Copy the given tests.** From the repository root, three commands: + + ```sh + cp docs/plans/M5c2/files/crates/brokerd/tests/ask_memory_tool.rs crates/brokerd/tests/ + cp docs/plans/M5c2/files/crates/brokerd/tests/grants.rs crates/brokerd/tests/ + cp docs/plans/M5c2/files/crates/brokerd/tests/memory_tools.rs crates/brokerd/tests/ + ``` + + Then `git status --short` lists `grants.rs` and `memory_tools.rs` as modified and + `ask_memory_tool.rs` as new, all under `crates/brokerd/tests/`. +- [ ] **2. See them fail.** Two commands, each with a shell timeout of 600000 ms: + - `cargo build --tests --keep-going -p brokerd`: one test target, `ask_memory_tool`, does not + compile, with 11 errors: one `E0432` (``unresolved import `brokerd::builtin::ASK_MEMORY` ``), + one `E0599` (``no variant, associated function, or constant named `AskMemory` found for enum + `ToolArgs` ``) and nine `E0599` (the same for enum `ToolName`). Every other target compiles. + - `cargo test --no-fail-fast -p brokerd --test grants --test memory_tools`: two tests fail, + each on its changed line: `an_empty_directory_is_a_valid_empty_set` in `grants` and + `grant_files_cannot_name_the_memory_tools_or_take_a_built_in_id` in `memory_tools`. +- [ ] **3. The edits.** Make edits A1 to A9, B1, B2, C1, P1 and R1, in order. `cargo check -p + brokerd` prints no error and no warning. +- [ ] **4. The tests.** With a shell timeout of 600000 ms, every test passes: 9 in + `ask_memory_tool`, 17 in `grants`, 4 in `memory_tools`, 5 in `memory_port`, 8 in `runner`: + + ```sh + cargo test -p brokerd --test ask_memory_tool --test grants --test memory_tools \ + --test memory_port --test runner + ``` +- [ ] **5. Check.** Two commands: + - `git diff --stat -- crates/brokerd/src` lists exactly the five files of "Modify" under + `crates/brokerd/src`: not `grants.rs`, `container.rs` or `memory_port.rs`. + - `git diff --stat -- crates/bxcore crates/loopd crates/memoryd Cargo.toml Cargo.lock` prints + nothing. +- [ ] **6. Run the gate.** `cargo fmt --all`, then `make gate`, with a shell timeout of 600000 ms. + Expected last line: `gate: ok`. The test count is the passed total of `cargo test --workspace` + plus one, for the ignored `end_to_end` test that the gate runs on its own: **1381** after this + task, 1372 before it; the 9 new ones are the tests of `ask_memory_tool.rs` (the new versions of + `grants.rs` and `memory_tools.rs` keep their counts). To count, add up the `N passed` of every + `test result:` line the gate's `cargo test --workspace` printed, and add 1. Never run the gate + again only to count. +- [ ] **7. Log and commit.** Add your row to `docs/implementer-log.md`; its Task cell is + `M5c2/02-brokerd-ask-memory`. `git status --short` lists the five source files, the two + replaced tests and your log as modified, and `ask_memory_tool.rs` as new. Stage with + `git add crates/brokerd docs/implementer-log.md Cargo.lock`, then commit as `AGENTS.md` says. + `Cargo.lock` does not change in this task; the `git add` is harmless. + +## Done when + +- `make gate` prints `gate: ok` with 1381 tests. +- `git diff HEAD~1 --stat` lists only the five source files, the three test files of step 1 and + `docs/implementer-log.md`. + +## Stop and report if + +- The branch check at the top fails. +- A "before" block is not in its file exactly as shown when its turn comes, or is there more + than once. +- Step 2 shows a different result: other targets not compiling, other errors, or other tests + failing. +- `cargo check` in step 3 prints an error or a warning. Quote it, and stop. +- A test fails in step 4. Do not change a test: stop, and give the failing tests' names and + their messages. +- The fix seems to need a change anywhere else: `grants.rs`, `container.rs`, `memory_port.rs`, + `bxcore`, a test support file, a per-tool wait. It does not; stop and say why you think so. +- A test outside the three given files fails in the gate: check once whether it fails without + your change (`git stash -u`, run it, `git stash pop`), then stop and report both results. +- The gate passes with a count other than 1381. +- A comment in an edit does not fit the code under it. Quote it and stop. + +## What the given tests pin + +Each of these changes to the finished code makes at least one given test fail (17 tried, all +caught): `ask_memory` not parsed as a name, or written another way; `AskMemory` not a memory +tool; `check` skipped; `tool()` giving another tool; the canonical form lost; the built-in grant +missing, under another id, asking instead of allowing, capped at `private`, or labelling its +results untrusted; `BUILT_IN`'s old sentence; `AskMemory` matching no grant; an owner's grant +naming it accepted; a container running it; `Routed` sending it to the container runtime; +`memoryd`'s label dropped, so its answer no longer raises the session. diff --git a/docs/plans/M5c2/README.md b/docs/plans/M5c2/README.md index 662ed72..e033040 100644 --- a/docs/plans/M5c2/README.md +++ b/docs/plans/M5c2/README.md @@ -73,16 +73,17 @@ Honcho sources and chats outside its state lock, and answers `ask_memory`. | # | Delivers | Crates | |---|---|---| | 00 | (done by the design model, `718e66d`) recordings from rift: workspace search with a limit and with no match, conclusion queries for two peers, and what a missing workspace answers | memoryd fixtures | -| 01 | Wire: `AskMemoryArgs { question, workspaces }`, its name and its checks (question at most 2,000 characters; one or two workspace names, no repeats); fixtures | bxcore | -| 02 | `brokerd`: `ask_memory` as a built-in memory tool (grant, parsing, routing to `memoryd`); a per-tool wait on `memoryd`, 170 s for `ask_memory` and 30 s for the others | brokerd | -| 03 | `loopd`: `ask_memory`'s schema as a core tool in a new registry; a per-tool wait on `brokerd`, 180 s for `ask_memory` and `[broker] timeout_ms` for the others; the system prompt's line on memory tools | loopd | -| 04 | `memoryd`: read clients, one per `[[honcho.read]]` workspace, the fleet and the agent's own two, with a 5 s limit for searches and 150 s for chat | memoryd | -| 05 | `memoryd`, pure: Honcho messages and conclusions as `recall` items (tag, source name, class, neutralised, capped), kept only when they share a term with the query, interleaved with the local hits to `k`, the local index first | memoryd | -| 06 | `memoryd`: `recall`'s Honcho sources, each on its own thread with its own limit, after the local search and outside the lock; `: unavailable`; a query holding a secret sends nothing | memoryd | -| 07 | `memoryd`: `ask_memory`: workspace names checked against `[honcho]`, both chats at once, each answer tagged and capped, labels, `: unavailable`; a question holding a secret sends nothing | memoryd | -| 08 | `memoryd` serves `ask_memory` beside `recall` and `memory`, outside the lock; `bxctl`'s usage and the runbook where a new message needs a pointer | memoryd, bxctl | +| 01 | Wire: `AskMemoryArgs { question, workspaces }`, its name and its checks (question at most 2,000 characters; one or two workspace names, no repeats, each 1 to 512 characters as `memoryd`'s name rule) | bxcore | +| 02 | `brokerd`: `ask_memory` as a built-in memory tool (name, parsing, the grant `builtin-ask-memory`, owner grants naming it refused, routing to `memoryd`); its answer's labels raise the session | brokerd | +| 03 | `brokerd`: a per-tool wait on `memoryd`, 170 s for `ask_memory` (`ASK_MEMORY_WAIT`) and 30 s for the others, chosen by `MemoryPort::wait_for` | brokerd | +| 04 | `loopd`: `ask_memory`'s schema as a core tool in a new registry; a per-tool wait on `brokerd`, 180 s for `ask_memory` and `[broker] timeout_ms` for the others; the system prompt's line on memory tools | loopd | +| 05 | `memoryd`: read clients, one per `[[honcho.read]]` workspace, the fleet and the agent's own two, with a 5 s limit for searches and 150 s for chat | memoryd | +| 06 | `memoryd`, pure: Honcho messages and conclusions as `recall` items (tag, source name, class, neutralised, capped), kept only when they share a term with the query, interleaved with the local hits to `k`, the local index first | memoryd | +| 07 | `memoryd`: `recall`'s Honcho sources, each on its own thread with its own limit, after the local search and outside the lock; `: unavailable`; a query holding a secret sends nothing | memoryd | +| 08 | `memoryd`: `ask_memory`: workspace names checked against `[honcho]`, both chats at once, each answer tagged and capped, labels, `: unavailable`; a question holding a secret sends nothing | memoryd | +| 09 | `memoryd` serves `ask_memory` beside `recall` and `memory`, outside the lock; `bxctl`'s usage and the runbook where a new message needs a pointer | memoryd, bxctl | -After task 08, the design model runs the M5c-2 device check (spec 13) on straylight, rift and the +After task 09, the design model runs the M5c-2 device check (spec 13) on straylight, rift and the owner's Mattermost: a second `memoryd` under its own name writes a fleet note that `recall` finds, tagged `memory-tainted`, turning the session untrusted; `ask_memory` answers from the agent's own trusted workspace within its limit; with rift down, `recall` answers from the local index with @@ -97,6 +98,8 @@ The gate count is the passed total of `cargo test --workspace` plus one, for the | Task | Gate count after | Mutations tried, caught | |---|---|---| | 01 | 1372 | 23, 23 | +| 02 | 1381 | 17, 17 | +| 03 | 1384 | 10, 10 | ## Before hand-over diff --git a/docs/plans/M5c2/files/crates/brokerd/tests/ask_memory_tool.rs b/docs/plans/M5c2/files/crates/brokerd/tests/ask_memory_tool.rs new file mode 100644 index 0000000..a719cb2 --- /dev/null +++ b/docs/plans/M5c2/files/crates/brokerd/tests/ask_memory_tool.rs @@ -0,0 +1,340 @@ +//! `ask_memory` in `brokerd`: its name and arguments, the built-in grant that allows it in every +//! session, grant files that name it refused, its calls routed to `memoryd` and never to a +//! container, and the labels of `memoryd`'s answer raising the session (M5c spec, section 9). +//! Do not edit. + +#[path = "support/build.rs"] +mod build; +#[path = "support/client.rs"] +mod client; +#[path = "support/fake_podman.rs"] +mod fake_podman; +#[path = "support/rig.rs"] +mod rig; +#[path = "support/runtime.rs"] +mod runtime; +#[path = "support/sink.rs"] +mod sink; +#[path = "support/tmp.rs"] +mod tmp; + +use std::os::unix::net::UnixListener; +use std::path::Path; +use std::sync::{Arc, mpsc}; +use std::time::Duration; + +use brokerd::args::{ArgsError, ToolArgs, ToolName, parse}; +use brokerd::broker::Broker; +use brokerd::builtin::{ASK_MEMORY, BUILT_IN}; +use brokerd::container::{IN_MEMORYD, Podman}; +use brokerd::grants::load; +use brokerd::memory_port::{MemoryPort, Routed}; +use brokerd::policy::{Label, SessionState, decide}; +use brokerd::runner::run; +use build::{allowed, now, private, secret, set}; +use bxcore::{ + AskMemoryArgs, DataClass, Envelope, MemoryCall, Message, SessionId, ToolResponse, read_frame, + sha256, write_frame, +}; +use fake_podman::{Fake, Lines, serial}; +use rig::Rig; +use runtime::{Recording, Shared}; +use tmp::TempDir; + +/// A fake `memoryd`: answers one call with `answer` and hands back what it was sent. A failed +/// write is ignored: the test has its answer or has already failed. +fn fake_memoryd(socket: &Path, answer: ToolResponse) -> mpsc::Receiver { + let listener = UnixListener::bind(socket).unwrap(); + let (tx, rx) = mpsc::channel(); + std::thread::spawn(move || { + let Ok((mut stream, _)) = listener.accept() else { + return; + }; + let Ok(env) = read_frame(&mut stream) else { + return; + }; + let Message::MemoryCall(call) = env.msg else { + return; + }; + let _ = tx.send(call); + let reply = Envelope { + v: 1, + id: env.id, + r#final: true, + msg: Message::ToolResponse(answer), + }; + let _ = write_frame(&mut stream, &reply); + }); + rx +} + +fn answer(class: DataClass, untrusted: bool) -> ToolResponse { + ToolResponse::Result { + content: "own: memory · The owner prefers UTC.".to_string(), + class, + untrusted, + truncated: false, + } +} + +fn routed(socket: &Path, other: &Arc) -> Routed { + Routed { + memory: MemoryPort::new(socket.to_path_buf(), Duration::from_secs(5)), + other: Box::new(Shared(Arc::clone(other))), + } +} + +const QUESTION: &str = r#"{"question":"What does the owner prefer?"}"#; + +#[test] +fn the_name_and_the_arguments() { + assert_eq!(ToolName::parse("ask_memory"), Some(ToolName::AskMemory)); + assert_eq!(ToolName::AskMemory.as_str(), "ask_memory"); + assert!(ToolName::AskMemory.is_memory()); + assert!( + !ToolName::ALL.contains(&ToolName::AskMemory), + "ALL is the container tools" + ); + let args = parse( + ToolName::AskMemory, + r#"{ "workspaces": ["fleet", "boxmaker1"], "question": "Who wrote it?" }"#, + ) + .unwrap(); + assert_eq!(args.tool(), ToolName::AskMemory); + assert_eq!( + args, + ToolArgs::AskMemory(AskMemoryArgs { + question: "Who wrote it?".to_string(), + workspaces: vec!["fleet".to_string(), "boxmaker1".to_string()], + }) + ); + assert_eq!( + args.canonical_json(), + r#"{"question":"Who wrote it?","workspaces":["fleet","boxmaker1"]}"# + ); + let default = parse(ToolName::AskMemory, QUESTION).unwrap(); + assert_eq!( + default.canonical_json(), + r#"{"question":"What does the owner prefer?","workspaces":[]}"#, + "every field is written, as recall's k is" + ); +} + +/// The refusals are `bxcore`'s sentences, shown to the model after `invalid argument shape: `. +#[test] +fn bad_arguments_are_refused_with_the_checks_sentences() { + let long = format!(r#"{{"question":"{}"}}"#, "é".repeat(2_001)); + for (json, sentence) in [ + ( + r#"{"question":" "}"#, + "question is empty; write the question to ask", + ), + ( + long.as_str(), + "question has 2001 characters, over the limit of 2000; ask a shorter question", + ), + ( + r#"{"question":"q","workspaces":["own","untrusted","fleet"]}"#, + "workspaces names 3, over the limit of 2; ask about the others in another call", + ), + ( + r#"{"question":"q","workspaces":["fleet","fleet"]}"#, + "workspaces names fleet twice; name each workspace once", + ), + ( + r#"{"question":"q","workspaces":[""]}"#, + "a workspace name is empty; name a workspace, or leave workspaces out to ask own", + ), + ( + r#"{"question":"q","workspaces":["a.b"]}"#, + "a workspace name has 1 to 512 letters, digits, _ or -; use own, untrusted, fleet or \ + another agent's name", + ), + ] { + let error = parse(ToolName::AskMemory, json).unwrap_err(); + assert_eq!(error, ArgsError::Shape(sentence.to_string()), "{json}"); + assert_eq!( + error.detail(), + format!("invalid argument shape: {sentence}") + ); + } + for json in [ + r#"{"question":"q","reasoning":"low"}"#, + r#"{"workspaces":["own"]}"#, + "not json", + ] { + let error = parse(ToolName::AskMemory, json).unwrap_err(); + assert!(matches!(error, ArgsError::Shape(_)), "{json}: {error:?}"); + } +} + +/// The built-in grant allows `ask_memory` in every session, `secret` and untrusted ones too: a +/// question is not written to Honcho (M5c spec, section 8). Its results are labelled `private` +/// and trusted; `memoryd`'s own labels can only make them stricter. +#[test] +fn the_built_in_grant_allows_it_in_every_session() { + assert_eq!( + ASK_MEMORY, + "tool = \"ask_memory\"\nmode = \"auto\"\nmax_taint = \"secret\"\nresult_class = \ + \"private\"\nuntrusted = false\n" + ); + let grants = set(vec![]); + let untrusted = SessionState { + taint: DataClass::Secret, + untrusted: true, + }; + for state in [private(), secret(), untrusted] { + let d = allowed(decide( + build::request("ask_memory", QUESTION), + &grants, + state, + now(), + )); + assert_eq!(d.grant(), "builtin-ask-memory"); + assert_eq!(d.grant_sha256(), sha256(ASK_MEMORY.as_bytes()).unwrap()); + assert_eq!( + d.label(), + Label { + class: DataClass::Private, + untrusted: false + } + ); + } + let ids: Vec<&str> = grants.matchable().map(|g| g.id.as_str()).collect(); + assert_eq!( + ids, + ["builtin-memory", "builtin-recall", "builtin-ask-memory"] + ); +} + +#[test] +fn a_grant_file_cannot_name_it() { + assert_eq!( + BUILT_IN, + "the policy of recall, memory and ask_memory is built in; remove this grant" + ); + let dir = TempDir::new("ask-grants"); + std::fs::write( + dir.path().join("mine.toml"), + "tool = \"ask_memory\"\nmode = \"deny\"\nmax_taint = \"secret\"\n", + ) + .unwrap(); + let problems = load(dir.path()).unwrap_err(); + let got: Vec<(String, String)> = problems.into_iter().map(|p| (p.file, p.problem)).collect(); + assert_eq!(got, [("mine.toml".to_string(), BUILT_IN.to_string())]); +} + +#[test] +fn it_goes_to_memoryd_with_its_canonical_arguments() { + let dir = TempDir::new("ask-route"); + let socket = dir.path().join("memory.sock"); + let seen = fake_memoryd(&socket, answer(DataClass::Private, false)); + let other = Recording::answering("container"); + let asked = r#"{"workspaces":["own","fleet"],"question":"Who wrote it?"}"#; + let got = run( + allowed(decide( + build::request("ask_memory", asked), + &set(vec![]), + secret(), + now(), + )), + &routed(&socket, &other), + ); + assert_eq!(got, answer(DataClass::Private, false)); + let call = seen.recv_timeout(Duration::from_secs(5)).unwrap(); + assert_eq!(call.request.tool, "ask_memory"); + assert_eq!( + call.request.arguments, + r#"{"question":"Who wrote it?","workspaces":["own","fleet"]}"# + ); + assert_eq!((call.taint, call.untrusted), (DataClass::Secret, false)); + assert_eq!(other.count(), 0, "nothing reached the other runtime"); +} + +#[test] +fn a_container_runtime_never_runs_it() { + let _s = serial(); + let fake = Fake::new("ask", "exit 0"); + let log = Lines::default(); + let podman = Podman::new(fake.runner(""), fake.dir.join("egress"), log.sink()); + let got = run( + allowed(decide( + build::request("ask_memory", QUESTION), + &set(vec![]), + private(), + now(), + )), + &podman, + ); + assert_eq!( + got, + ToolResponse::Failed { + message: IN_MEMORYD.to_string() + } + ); + assert!(fake.calls().is_empty(), "podman was never called"); +} + +/// One `ask_memory` call through `broker::handle`, whose runtime sends it to a fake `memoryd` +/// answering `reply`; the session's state afterwards. +fn state_after(tag: &str, reply: ToolResponse) -> SessionState { + let rig = Rig::new(tag); + let socket = rig.dir.path().join("memory.sock"); + let _seen = fake_memoryd(&socket, reply); + let other = Recording::answering("unused"); + let broker = Arc::new(Broker::new( + rig.cfg.clone(), + rig.ledger(), + Box::new(routed(&socket, &other)), + rig.lines.sink(), + )); + let frames = client::call(&broker, rig::request("s1", 1, "ask_memory", QUESTION)); + assert!( + matches!( + frames.as_slice(), + [Envelope { + msg: Message::ToolResponse(ToolResponse::Result { .. }), + .. + }] + ), + "{frames:?}" + ); + rig.state().read(&SessionId::new("s1").unwrap()).unwrap() +} + +/// T58: an answer from a workspace that is not trusted makes a clean session untrusted, as a +/// `memory-tainted` recall hit does. +#[test] +fn an_untrusted_answer_makes_the_session_untrusted() { + assert_eq!( + state_after("ask-untrusted", answer(DataClass::Private, true)), + SessionState { + taint: DataClass::Private, + untrusted: true + } + ); +} + +/// T58: an answer labelled `secret` raises the session to `secret`. +#[test] +fn a_secret_answer_raises_the_session_to_secret() { + assert_eq!( + state_after("ask-secret", answer(DataClass::Secret, false)), + SessionState { + taint: DataClass::Secret, + untrusted: false + } + ); +} + +/// The grant's label is the floor: a `public` answer still leaves the session `private`. +#[test] +fn a_public_answer_leaves_the_session_private() { + assert_eq!( + state_after("ask-public", answer(DataClass::Public, false)), + SessionState { + taint: DataClass::Private, + untrusted: false + } + ); +} diff --git a/docs/plans/M5c2/files/crates/brokerd/tests/grants.rs b/docs/plans/M5c2/files/crates/brokerd/tests/grants.rs new file mode 100644 index 0000000..bd1dfe2 --- /dev/null +++ b/docs/plans/M5c2/files/crates/brokerd/tests/grants.rs @@ -0,0 +1,487 @@ +//! Tests for loading grant files. Do not edit these or the fixtures. +//! +//! One case per loading rule in the M3a spec, each with words its problem text must contain, and +//! the rule that matters most: one invalid file makes the whole set invalid. + +#[path = "support/tmp.rs"] +mod tmp; + +use brokerd::grants::{GrantSet, LoadedGrant, RUNBOOK, load, render, valid_id}; +use bxcore::{Constraints, DataClass, Grant, GrantProblem, Hash32, Mode}; +use std::path::{Path, PathBuf}; +use tmp::TempDir; + +fn fixture(case: &str) -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/grants") + .join(case) +} + +const GOOD: &str = "tool = \"read_file\"\nmode = \"auto\"\nmax_taint = \"secret\"\n\n\ + [constraints]\npaths = [\"/home/kyle/notes\"]\n"; + +/// A directory holding one good grant and the given files; returns the problems of loading it. +fn problems_of(files: &[(&str, &str)]) -> Vec { + let dir = TempDir::new("grants"); + dir.write("good.toml", GOOD); + for (name, text) in files { + dir.write(name, text); + } + load(dir.path()).expect_err("the set should be invalid") +} + +/// Exactly one problem, in `file`, whose text contains every one of `words`. +fn one_problem(files: &[(&str, &str)], file: &str, words: &[&str]) -> GrantProblem { + let problems = problems_of(files); + assert_eq!(problems.len(), 1, "{problems:?}"); + let p = problems.into_iter().next().unwrap(); + assert_eq!(p.file, file); + for word in words { + assert!(p.problem.contains(word), "{:?} lacks {word:?}", p.problem); + } + p +} + +fn body(tool: &str, mode: &str, rest: &str) -> String { + format!("tool = \"{tool}\"\nmode = \"{mode}\"\nmax_taint = \"secret\"\n{rest}") +} + +#[test] +fn the_valid_fixture_loads_in_id_order_with_file_hashes() { + let set = load(&fixture("valid")).unwrap(); + let ids: Vec<&str> = set.grants().iter().map(|g| g.id.as_str()).collect(); + assert_eq!( + ids, + [ + "fetch-example", + "no-fetch-internal", + "notes-read", + "scratch-write", + "shell-bare" + ] + ); + let notes = &set.grants()[2]; + assert_eq!(notes.grant.tool, "read_file"); + assert_eq!(notes.grant.mode, Mode::Auto); + assert!(!notes.grant.untrusted); + assert_eq!(notes.grant.constraints.paths, ["/home/kyle/notes"]); + let bytes = std::fs::read(fixture("valid").join("notes-read.toml")).unwrap(); + assert_eq!(notes.sha256, bxcore::sha256(&bytes).unwrap()); + // Defaults from `bxcore::Grant`. + let scratch = &set.grants()[3]; + assert_eq!(scratch.grant.result_class, DataClass::Private); + assert!(scratch.grant.untrusted); + assert_eq!(scratch.grant.expires, None); +} + +#[test] +fn files_that_do_not_end_in_toml_are_ignored() { + let dir = TempDir::new("grants"); + dir.write("good.toml", GOOD); + dir.write("good.toml~", "not toml at all {{{"); + dir.write("good.toml.bak", "not toml at all {{{"); + dir.write("README.md", "# notes"); + dir.write("toml", "x"); + std::fs::create_dir(dir.path().join("archive")).unwrap(); + let set = load(dir.path()).unwrap(); + assert_eq!(set.grants().len(), 1); + assert_eq!(set.grants()[0].id, "good"); +} + +#[test] +fn an_empty_directory_is_a_valid_empty_set() { + let set = load(&fixture("empty")).unwrap(); + assert!(set.grants().is_empty()); + let builtin: Vec<&str> = set.matchable().map(|g| g.id.as_str()).collect(); + assert_eq!( + builtin, + ["builtin-memory", "builtin-recall", "builtin-ask-memory"], + "only the built-ins" + ); +} + +#[test] +fn a_missing_directory_is_a_problem_not_an_empty_set() { + let missing = fixture("does-not-exist"); + let problems = load(&missing).unwrap_err(); + assert_eq!(problems.len(), 1); + assert!(problems[0].file.contains("does-not-exist"), "{problems:?}"); + assert!(problems[0].problem.contains("cannot be read")); + // A file where the directory should be is the same. + let dir = TempDir::new("grants"); + let file = dir.write("grants", "x"); + assert!(load(&file).is_err()); +} + +/// The rule the whole design leans on. `fetch-example` alone would allow a fetch that the +/// mistyped `no-fetch-internal` was written to forbid, so nothing loads at all. +#[test] +fn one_invalid_file_makes_the_whole_set_invalid() { + let problems = load(&fixture("one-bad")).unwrap_err(); + assert_eq!(problems.len(), 1, "{problems:?}"); + assert_eq!(problems[0].file, "no-fetch-internal.toml"); + assert_eq!(problems[0].line, Some(3)); + assert!(problems[0].problem.contains("mdoe"), "{problems:?}"); + // The same directory without the bad file is fine. + let dir = TempDir::new("grants"); + for name in ["notes-read.toml", "fetch-example.toml"] { + let text = std::fs::read_to_string(fixture("one-bad").join(name)).unwrap(); + dir.write(name, &text); + } + assert_eq!(load(dir.path()).unwrap().grants().len(), 2); +} + +#[test] +fn every_problem_in_every_file_is_reported() { + let problems = load(&fixture("many-bad")).unwrap_err(); + let got: Vec<(&str, Option)> = + problems.iter().map(|p| (p.file.as_str(), p.line)).collect(); + assert_eq!( + got, + [ + ("a-secret.toml", None), + ("a-secret.toml", None), + ("b-paths.toml", None), + ("b-paths.toml", None), + ("b-paths.toml", None), + ("b-paths.toml", None), + ("c-syntax.toml", Some(3)), + ], + "{problems:?}" + ); + let all: String = problems + .iter() + .map(|p| format!("{}\n", p.problem)) + .collect(); + for words in [ + "secrets are not supported until M4", + "patterns are not supported", + "read_file does not take hosts", + "\"notes\" is not a valid absolute path", + "\"/home/kyle/../etc\" is not a valid absolute path", + "a grant of the whole file system is not supported", + ] { + assert!(all.contains(words), "missing {words:?} in:\n{all}"); + } +} + +#[test] +fn rule_1_unreadable_not_utf8_or_not_a_grant() { + one_problem(&[("bad.toml", "tool = ")], "bad.toml", &[]); + let p = one_problem( + &[("bad.toml", &body("shell", "auto", "colour = \"red\"\n"))], + "bad.toml", + &["colour"], + ); + assert_eq!(p.line, Some(4)); + one_problem( + &[("bad.toml", "tool = \"shell\"\nmode = \"auto\"\n")], + "bad.toml", + &["max_taint"], + ); + one_problem( + &[("bad.toml", &body("shell", "sometimes", ""))], + "bad.toml", + &["sometimes"], + ); + one_problem( + &[( + "bad.toml", + &body("shell", "auto", "[constraints]\ncwd = [\"/a\"]\n"), + )], + "bad.toml", + &["cwd"], + ); + + // Not UTF-8. + let dir = TempDir::new("grants"); + dir.write("good.toml", GOOD); + std::fs::write(dir.path().join("latin1.toml"), b"tool = \"caf\xe9\"\n").unwrap(); + let problems = load(dir.path()).unwrap_err(); + assert_eq!(problems.len(), 1); + assert_eq!(problems[0].file, "latin1.toml"); + assert!(problems[0].problem.contains("UTF-8"), "{problems:?}"); + + // Exists but cannot be read: a directory with a grant's name. + let dir = TempDir::new("grants"); + dir.write("good.toml", GOOD); + std::fs::create_dir(dir.path().join("folder.toml")).unwrap(); + let problems = load(dir.path()).unwrap_err(); + assert_eq!(problems.len(), 1); + assert_eq!(problems[0].file, "folder.toml"); + assert!( + problems[0].problem.contains("cannot be read"), + "{problems:?}" + ); +} + +#[test] +fn rule_1_a_file_without_read_permission_is_a_problem() { + use std::os::unix::fs::PermissionsExt; + if tmp::running_as_root("rule_1_a_file_without_read_permission_is_a_problem") { + return; + } + let dir = TempDir::new("grants"); + dir.write("good.toml", GOOD); + let locked = dir.write("locked.toml", GOOD); + std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o000)).unwrap(); + let problems = load(dir.path()).unwrap_err(); + assert_eq!(problems.len(), 1); + assert_eq!(problems[0].file, "locked.toml"); + assert!( + problems[0].problem.contains("cannot be read"), + "{problems:?}" + ); +} + +#[test] +fn rule_2_the_file_stem_is_the_id() { + for id in ["a", "notes-read", "0", "a-1-b", &"x".repeat(64)] { + assert!(valid_id(id), "{id:?}"); + } + for id in [ + "", + "Notes", + "notes_read", + "notes.read", + "notes read", + ".hidden", + &"x".repeat(65), + ] { + assert!(!valid_id(id), "{id:?}"); + } + one_problem( + &[("Bad_Name.toml", GOOD)], + "Bad_Name.toml", + &["not a valid grant id"], + ); + one_problem(&[(".toml", GOOD)], ".toml", &["not a valid grant id"]); + one_problem(&[("a.b.toml", GOOD)], "a.b.toml", &["not a valid grant id"]); +} + +#[test] +fn rule_3_the_tool_is_one_of_the_four() { + for tool in ["echo", "clock", "Read_File", ""] { + one_problem( + &[("bad.toml", &body(tool, "auto", ""))], + "bad.toml", + &[ + "unknown tool", + "read_file, write_file, shell and http_fetch", + ], + ); + } +} + +#[test] +fn rules_4_and_5_secrets_and_patterns_are_not_supported() { + one_problem( + &[("bad.toml", &body("shell", "ask", "secret = \"token\"\n"))], + "bad.toml", + &["secrets are not supported until M4"], + ); + one_problem( + &[( + "bad.toml", + &body("shell", "ask", "[constraints]\npatterns = [\"^ls\"]\n"), + )], + "bad.toml", + &["patterns are not supported"], + ); + // An empty list is the same as no list. + let dir = TempDir::new("grants"); + dir.write( + "ok.toml", + &body("shell", "ask", "[constraints]\npatterns = []\nhosts = []\n"), + ); + assert!(load(dir.path()).is_ok()); +} + +/// The table of rule 6, cell by cell. +#[test] +fn rule_6_each_tool_takes_its_own_constraints() { + let paths = "[constraints]\npaths = [\"/a\"]\n"; + let hosts = "[constraints]\nhosts = [\"example.com\"]\n"; + let both = "[constraints]\npaths = [\"/a\"]\nhosts = [\"example.com\"]\n"; + for tool in ["read_file", "write_file"] { + one_problem( + &[("bad.toml", &body(tool, "auto", ""))], + "bad.toml", + &[tool, "needs at least one path"], + ); + one_problem( + &[("bad.toml", &body(tool, "auto", both))], + "bad.toml", + &[tool, "does not take hosts"], + ); + } + one_problem( + &[("bad.toml", &body("shell", "auto", both))], + "bad.toml", + &["shell does not take hosts"], + ); + one_problem( + &[("bad.toml", &body("http_fetch", "auto", ""))], + "bad.toml", + &["http_fetch needs at least one host"], + ); + one_problem( + &[("bad.toml", &body("http_fetch", "auto", both))], + "bad.toml", + &["http_fetch does not take paths"], + ); + // The allowed cells. + let dir = TempDir::new("grants"); + dir.write("r.toml", &body("read_file", "auto", paths)); + dir.write("w.toml", &body("write_file", "auto", paths)); + dir.write("s1.toml", &body("shell", "auto", paths)); + dir.write("s2.toml", &body("shell", "auto", "")); + dir.write("h.toml", &body("http_fetch", "auto", hosts)); + assert_eq!(load(dir.path()).unwrap().grants().len(), 5); + // Two wrong cells in one file are two problems. + let wrong = body("http_fetch", "auto", paths); + assert_eq!(problems_of(&[("bad.toml", &wrong)]).len(), 2); +} + +#[test] +fn rule_7_paths_are_valid_absolute_paths_and_never_the_root() { + for bad in ["notes", "/a/../b", "/a//b", "/a/./b", "/a/", ""] { + let text = body( + "shell", + "auto", + &format!("[constraints]\npaths = [{bad:?}]\n"), + ); + one_problem( + &[("bad.toml", &text)], + "bad.toml", + &["is not a valid absolute path"], + ); + } + let root = body( + "shell", + "auto", + "[constraints]\npaths = [\"/home/kyle\", \"/\"]\n", + ); + one_problem( + &[("bad.toml", &root)], + "bad.toml", + &["a grant of the whole file system is not supported"], + ); +} + +#[test] +fn rule_8_hosts_are_valid_host_patterns() { + for bad in [ + "Example.com", + "example.com:443", + "127.0.0.1", + "localhost", + "*.com", + "https://example.com", + ] { + let text = body( + "http_fetch", + "auto", + &format!("[constraints]\nhosts = [{bad:?}]\n"), + ); + one_problem( + &[("bad.toml", &text)], + "bad.toml", + &[bad, "is not a valid host pattern"], + ); + } +} + +#[test] +fn rule_9_a_deny_grant_applies_at_every_taint() { + for taint in ["public", "private"] { + let text = format!("tool = \"shell\"\nmode = \"deny\"\nmax_taint = \"{taint}\"\n"); + one_problem( + &[("bad.toml", &text)], + "bad.toml", + &["a deny grant must apply at every taint"], + ); + } + let dir = TempDir::new("grants"); + dir.write("no-shell.toml", &body("shell", "deny", "")); + assert!(load(dir.path()).is_ok()); + // The rule is about deny only. + dir.write( + "ask.toml", + "tool = \"shell\"\nmode = \"ask\"\nmax_taint = \"public\"\n", + ); + assert!(load(dir.path()).is_ok()); +} + +fn loaded(id: &str, tool: &str, mode: Mode) -> LoadedGrant { + LoadedGrant { + id: id.to_string(), + grant: Grant { + tool: tool.to_string(), + mode, + max_taint: DataClass::Secret, + result_class: DataClass::Private, + untrusted: true, + expires: None, + secret: None, + constraints: Constraints::default(), + }, + sha256: Hash32::ZERO, + } +} + +/// `from_grants` is how tests and the property test build a set without files. It applies the +/// same value rules, sorts by id, and refuses two grants with one id. +#[test] +fn from_grants_applies_the_value_rules() { + let set = GrantSet::from_grants(vec![ + loaded("zz", "shell", Mode::Ask), + loaded("aa", "shell", Mode::Deny), + ]) + .unwrap(); + assert_eq!(set.grants()[0].id, "aa"); + assert_eq!(set.grants()[1].id, "zz"); + + let problems = GrantSet::from_grants(vec![ + loaded("ok", "shell", Mode::Auto), + loaded("no-paths", "read_file", Mode::Auto), + loaded("Bad", "shell", Mode::Auto), + ]) + .unwrap_err(); + let files: Vec<&str> = problems.iter().map(|p| p.file.as_str()).collect(); + assert_eq!(files, ["Bad.toml", "no-paths.toml"]); + + let twice = GrantSet::from_grants(vec![ + loaded("same", "shell", Mode::Auto), + loaded("same", "shell", Mode::Ask), + ]) + .unwrap_err(); + assert!( + twice[0].problem.contains("two grants have this id"), + "{twice:?}" + ); +} + +#[test] +fn render_prints_every_problem_and_then_the_runbook_pointer() { + let problems = [ + GrantProblem { + file: "a.toml".to_string(), + line: Some(3), + problem: "unknown field `mdoe`".to_string(), + }, + GrantProblem { + file: "Bad_Name.toml".to_string(), + line: None, + problem: "the file name is not a valid grant id".to_string(), + }, + ]; + assert_eq!( + render(&problems), + "a.toml:3: unknown field `mdoe`\n\ + Bad_Name.toml: the file name is not a valid grant id\n\ + see docs/runbook.md#grants-invalid\n" + ); + assert_eq!(RUNBOOK, "see docs/runbook.md#grants-invalid"); + assert!(render(&problems).trim_end().ends_with(RUNBOOK)); +} diff --git a/docs/plans/M5c2/files/crates/brokerd/tests/memory_tools.rs b/docs/plans/M5c2/files/crates/brokerd/tests/memory_tools.rs new file mode 100644 index 0000000..ed84278 --- /dev/null +++ b/docs/plans/M5c2/files/crates/brokerd/tests/memory_tools.rs @@ -0,0 +1,142 @@ +//! `recall` and `memory` in `brokerd`: their arguments, the built-in grants that allow them in +//! every session, and grant files that name them refused (M5a spec, section 8). Do not edit. + +#[path = "support/build.rs"] +mod build; +#[path = "support/tmp.rs"] +mod tmp; + +use brokerd::args::{ToolArgs, ToolName, parse}; +use brokerd::builtin::{BUILT_IN, MEMORY, RECALL}; +use brokerd::grants::{GrantSet, load}; +use brokerd::policy::{Label, Outcome, decide}; +use build::{allowed, grant, now, private, request, secret, set}; +use bxcore::{DataClass, DenyReason, Mode, sha256}; +use tmp::TempDir; + +#[test] +fn the_names_and_arguments_of_the_memory_tools() { + assert_eq!(ToolName::parse("recall"), Some(ToolName::Recall)); + assert_eq!(ToolName::parse("memory"), Some(ToolName::Memory)); + assert_eq!( + (ToolName::Recall.as_str(), ToolName::Memory.as_str()), + ("recall", "memory") + ); + assert!(ToolName::Recall.is_memory() && ToolName::Memory.is_memory()); + assert!( + ToolName::ALL.iter().all(|t| !t.is_memory()), + "ALL is the container tools" + ); + let recall = parse(ToolName::Recall, r#"{ "k": 3, "query": "launch" }"#).unwrap(); + assert_eq!(recall.tool(), ToolName::Recall); + assert_eq!(recall.canonical_json(), r#"{"query":"launch","k":3}"#); + let memory = parse(ToolName::Memory, r#"{"text":"x","action":"note"}"#).unwrap(); + assert!(matches!(memory, ToolArgs::Memory(_))); + assert_eq!(memory.canonical_json(), r#"{"action":"note","text":"x"}"#); + for (tool, bad) in [ + (ToolName::Recall, r#"{"query":""}"#), + (ToolName::Recall, r#"{"query":"x","k":11}"#), + ( + ToolName::Memory, + r#"{"action":"add","file":"soul","text":"x"}"#, + ), + ( + ToolName::Memory, + r#"{"action":"add","file":"memory","text":"a\n§\nb"}"#, + ), + ( + ToolName::Memory, + r#"{"action":"consolidate","file":"memory","entries":[]}"#, + ), + ] { + assert!(parse(tool, bad).is_err(), "{bad}"); + } +} + +#[test] +fn both_are_allowed_in_every_session_by_the_built_in_grants() { + let grants = set(vec![]); + for state in [private(), secret()] { + let d = allowed(decide( + request("recall", r#"{"query":"x"}"#), + &grants, + state, + now(), + )); + assert_eq!(d.grant(), "builtin-recall"); + assert_eq!(d.grant_sha256(), sha256(RECALL.as_bytes()).unwrap()); + assert_eq!( + d.label(), + Label { + class: DataClass::Private, + untrusted: false + } + ); + let d = allowed(decide( + request("memory", r#"{"action":"note","text":"x"}"#), + &grants, + state, + now(), + )); + assert_eq!(d.grant(), "builtin-memory"); + assert_eq!(d.grant_sha256(), sha256(MEMORY.as_bytes()).unwrap()); + } +} + +#[test] +fn a_set_without_built_ins_has_no_memory_tools() { + let outcome = decide( + request("recall", r#"{"query":"x"}"#), + &GrantSet::default(), + private(), + now(), + ); + assert!( + matches!(outcome, Outcome::Denied(ref d) if d.reason == DenyReason::NoGrant), + "{outcome:?}" + ); +} + +#[test] +fn grant_files_cannot_name_the_memory_tools_or_take_a_built_in_id() { + let dir = TempDir::new("memory-grants"); + std::fs::write( + dir.path().join("mine.toml"), + "tool = \"memory\"\nmode = \"ask\"\nmax_taint = \"secret\"\n", + ) + .unwrap(); + std::fs::write( + dir.path().join("off.toml"), + "tool = \"recall\"\nmode = \"deny\"\nmax_taint = \"secret\"\n", + ) + .unwrap(); + std::fs::write( + dir.path().join("builtin-x.toml"), + "tool = \"shell\"\nmode = \"ask\"\nmax_taint = \"private\"\n", + ) + .unwrap(); + let problems = load(dir.path()).unwrap_err(); + let got: Vec<(String, String)> = problems.into_iter().map(|p| (p.file, p.problem)).collect(); + assert!( + got.contains(&("mine.toml".to_string(), BUILT_IN.to_string())), + "{got:?}" + ); + assert!( + got.contains(&("off.toml".to_string(), BUILT_IN.to_string())), + "{got:?}" + ); + assert!( + got.contains(&( + "builtin-x.toml".to_string(), + "grant ids starting builtin- are reserved".to_string() + )), + "{got:?}" + ); + let with_shell = set(vec![grant("s", "shell", Mode::Ask)]); + assert_eq!( + with_shell.grants().len(), + 1, + "the built-ins are not listed as the owner's" + ); + assert_eq!(with_shell.matchable().count(), 4); +}