Task files, the files they copy in (byte-identical to the reference on m3a-ref), each area's check record, and a README with the per-task table of what each check exposed. The handoff note is done with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
139 lines
7.0 KiB
Markdown
139 lines
7.0 KiB
Markdown
# M3a task 06: loading grant files
|
||
|
||
**Branch:** `m3a` (run `git switch m3a`; `git status --short` must be empty, otherwise stop)
|
||
**Commit subject:** `Load grant files, failing closed on any invalid file`
|
||
|
||
## Goal
|
||
|
||
`brokerd::grants` reads `grants/*.toml` into a `GrantSet`. **If any file is invalid the whole set
|
||
is invalid.** The reason, from the spec: a mistyped `deny` grant that was skipped would silently
|
||
turn into an allow wherever another grant matches. So `load` returns either a complete valid set
|
||
or the full list of problems, never a partial set. It reports **every** problem in every file, not
|
||
only the first.
|
||
|
||
`brokerd` calls `load` at the start of every decision (a later task). There is no cache here.
|
||
|
||
## Files
|
||
|
||
- Copy: `crates/brokerd/tests/grants.rs`, `crates/brokerd/tests/support/tmp.rs`, and the directory
|
||
`crates/brokerd/tests/fixtures/grants/` (four sub-directories)
|
||
- Create: `crates/brokerd/src/grants.rs`
|
||
- Modify: `crates/brokerd/src/lib.rs` (add `pub mod grants;`), `docs/implementer-log.md`
|
||
|
||
## Interfaces
|
||
|
||
```rust
|
||
pub const RUNBOOK: &str = "see docs/runbook.md#grants-invalid";
|
||
|
||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||
pub struct LoadedGrant {
|
||
pub id: String, // the file stem
|
||
pub grant: proto::Grant,
|
||
pub sha256: proto::Hash32, // of the file's bytes as read
|
||
}
|
||
|
||
/// A set that passed every rule, in id order. Its field is private: `from_grants` and `load` are
|
||
/// the only ways to make one that is not empty.
|
||
#[derive(Debug, Clone, PartialEq, Eq, Default)]
|
||
pub struct GrantSet { /* grants: Vec<LoadedGrant> */ }
|
||
|
||
impl GrantSet {
|
||
pub fn from_grants(grants: Vec<LoadedGrant>) -> Result<GrantSet, Vec<GrantProblem>>;
|
||
pub fn grants(&self) -> &[LoadedGrant];
|
||
}
|
||
|
||
pub fn valid_id(id: &str) -> bool; // 1 to 64 characters of a-z, 0-9 and -
|
||
pub fn load(dir: &Path) -> Result<GrantSet, Vec<GrantProblem>>;
|
||
pub fn render(problems: &[GrantProblem]) -> String;
|
||
```
|
||
|
||
`GrantProblem { file, line, problem }` is `proto::GrantProblem`. `file` is the file's name with
|
||
`.toml`, without the directory.
|
||
|
||
## `from_grants`: the rules about a grant's values
|
||
|
||
Sort by `id` (byte order). Then check every grant and collect every problem; `file` is
|
||
`"<id>.toml"`, `line` is `None`. The problem text must contain the words in quotes.
|
||
|
||
| # | A problem when | Text |
|
||
|---|---|---|
|
||
| 2 | `valid_id(id)` is false | "the file name is not a valid grant id" |
|
||
| – | two grants have the same id | "two grants have this id" |
|
||
| 3 | `ToolName::parse(&grant.tool)` is `None` | "unknown tool", and "read_file, write_file, shell and http_fetch" |
|
||
| 4 | `grant.secret` is `Some` | "secrets are not supported until M4" |
|
||
| 5 | `constraints.patterns` is not empty | "patterns are not supported" |
|
||
| 6 | `read_file` or `write_file` with no paths | "`<tool>` needs at least one path" (the tool's name, no backquotes) |
|
||
| 6 | `read_file`, `write_file` or `shell` with hosts | "`<tool>` does not take hosts" |
|
||
| 6 | `http_fetch` with no hosts | "http_fetch needs at least one host" |
|
||
| 6 | `http_fetch` with paths | "http_fetch does not take paths" |
|
||
| 7 | a path is `/` | "a grant of the whole file system is not supported" |
|
||
| 7 | a path fails `args::valid_path` | the path with `{:?}`, then "is not a valid absolute path" |
|
||
| 8 | a host fails `args::valid_host_pattern` | the host with `{:?}`, then "is not a valid host pattern" |
|
||
| 9 | `mode` is `Deny` and `max_taint` is not `Secret` | "a deny grant must apply at every taint" |
|
||
|
||
`shell` may have paths or not. An empty list is the same as no list. One grant can have several
|
||
problems (rule 6 can fire twice for one file; rule 7 once per bad path): **push every one**. If the
|
||
tool is unknown, skip rule 6 for that grant; the other rules still apply.
|
||
|
||
Return `Ok(GrantSet)` only if there is no problem at all.
|
||
|
||
## `load`: the rules about files (rule 1), then `from_grants`
|
||
|
||
1. `std::fs::read_dir(dir)` fails (missing, not a directory, no permission): return one problem,
|
||
`file` = the directory's path as text, text "the grants directory cannot be read: <error>".
|
||
**A missing directory is not an empty set.** An entry that cannot be read is the same problem;
|
||
keep going.
|
||
2. Collect the file names and **sort them**, so problems come out in the same order every time.
|
||
3. For each name: if it does not end in `.toml`, skip it silently (`README.md`, `x.toml~`,
|
||
`x.toml.bak`, sub-directories). Otherwise the id is the name without `.toml`.
|
||
4. `std::fs::read` fails (no permission, or it is a directory): problem "cannot be read: <error>".
|
||
Go on to the next file. **Never treat an unreadable file as absent.**
|
||
5. `std::str::from_utf8` fails: problem "is not UTF-8". Next file.
|
||
6. `toml::from_str::<proto::Grant>` fails (bad syntax, unknown field, missing field, bad mode):
|
||
problem text = `error.message()`, line = the 1-based line of `error.span()`'s start. Next file.
|
||
7. Otherwise `sha256 = proto::sha256(&bytes)` (an error here is a problem too), and keep the grant.
|
||
8. Run `from_grants` on what was kept. **Even if step 4, 5 or 6 found a problem**, so the owner
|
||
sees every problem at once. Join both lists, sort by `file` (a stable sort), and return `Err`
|
||
if the joined list is not empty.
|
||
|
||
Verified in `toml` 1.1.6 (`src/de/error.rs`):
|
||
|
||
```rust
|
||
impl toml::de::Error {
|
||
pub fn message(&self) -> &str;
|
||
/// The start/end index into the original document where the error occurred
|
||
pub fn span(&self) -> Option<core::ops::Range<usize>>;
|
||
}
|
||
```
|
||
|
||
The line of a byte offset: count the `\n` bytes in `text.get(..offset)` and add 1. Use `get`, not
|
||
`[..offset]`; if `get` returns `None`, use the whole text.
|
||
|
||
## `render`
|
||
|
||
One line per problem, `<file>:<line>: <problem>` when there is a line and `<file>: <problem>` when
|
||
not; then `RUNBOOK` on a line of its own; every line ends with `\n`.
|
||
|
||
## Steps
|
||
|
||
- [ ] **1. Copy.** `git switch m3a`, then
|
||
`mkdir -p crates/brokerd/tests/support crates/brokerd/tests/fixtures && cp docs/plans/M3a/files/crates/brokerd/tests/grants.rs crates/brokerd/tests/ && cp docs/plans/M3a/files/crates/brokerd/tests/support/tmp.rs crates/brokerd/tests/support/ && cp -r docs/plans/M3a/files/crates/brokerd/tests/fixtures/grants crates/brokerd/tests/fixtures/`
|
||
- [ ] **2. See the test fail.** `cargo test -p brokerd --test grants`. Expected: it does not
|
||
compile.
|
||
- [ ] **3. Write `grants.rs`** and add `pub mod grants;` to `lib.rs`. Run `cargo fmt --all`.
|
||
- [ ] **4. See the tests pass.** `cargo test -p brokerd --test grants`. Expected: `17 passed`.
|
||
- [ ] **5. Walk the exits of `load`.** For each of steps 1, 4, 5, 6 and 7 above, find the line in
|
||
your code and check that it records a problem and does not return early with a partial set.
|
||
- [ ] **6. Run the gate.** `make gate`. Expected last line: `gate: ok`.
|
||
- [ ] **7. Log and commit.** `git add crates/brokerd docs/implementer-log.md && git commit`
|
||
|
||
## Done when
|
||
|
||
- `cargo test -p brokerd --test grants` reports 17 passed; `make gate` prints `gate: ok`.
|
||
|
||
## Stop and report if
|
||
|
||
- A test expects a set to load although one of its files is invalid.
|
||
- `git status` does not show `crates/brokerd/tests/fixtures/grants/empty/README.md` after the
|
||
copy (the empty-directory fixture needs a file in it for git to keep it).
|