Six closed tasks for consult: registry, CONSULT_WAIT, [consult]/[secrets], ToolName and grants, Decision body, loopback HTTP. Reference is the git bundle ~/boxmaker-refs/m6b-ref.bundle at 5fb05f0. Gate after task 06 is 1611. Co-Authored-By: Grok 4.6 <noreply@x.ai>
409 lines
15 KiB
Markdown
409 lines
15 KiB
Markdown
# M6b task 05: Decision `body`; denials still record it
|
||
|
||
**Branch:** `m6b`. Check it: `git branch --show-current` prints `m6b` and `git status --short`
|
||
is empty; otherwise stop. Never switch branch.
|
||
**Commit subject:** `brokerd: consult decisions record the body`
|
||
**Log row:** the Task cell is `M6b/05-brokerd-consult-audit-body`.
|
||
|
||
**First action.** Run this before any Read of crate source. Do not `ls`
|
||
`crates/brokerd/tests/`. Do not Read `main.rs`, `consult_http`, `docs/implementer-log.md`
|
||
(except the first 12 lines at the log step). Do not edit `docs/runbook.md` (task 03
|
||
already appended `## consult-not-configured`). The signatures below are complete.
|
||
|
||
```sh
|
||
cp docs/plans/M6b/files/crates/brokerd/tests/consult_deny.rs crates/brokerd/tests/
|
||
```
|
||
|
||
## Goal
|
||
|
||
Every `consult` decision records the JSON body that would be POSTed, including a denial
|
||
(spec 4.4). `AuditEvent::Decision` gains optional `body` with
|
||
`#[serde(default, skip_serializing_if = "Option::is_none")]`. Other tools leave it unset.
|
||
No `[consult]` is `decide_forced` with `InvalidArguments` and `NOT_CONFIGURED` (the
|
||
runbook pointer). A grant that does not cover the session's taint is `TaintTooHigh` and
|
||
still records the body. Unknown keys are invalid-arguments. Nothing is POSTed on a
|
||
denial (`Recording` stays at count 0).
|
||
|
||
## Every place that changes
|
||
|
||
Adding `body` to `AuditEvent::Decision` makes every struct literal of that variant fail
|
||
to compile until it names the field. List each constructor. Matches that use `..` need
|
||
no edit.
|
||
|
||
| Place | What happens there |
|
||
|---|---|
|
||
| `crates/loopd` / other crates | **no change** except the constructors below |
|
||
| `crates/bxcore/src/audit.rs` | `body: Option<String>` on `Decision` only |
|
||
| `crates/bxcore/tests/records.rs` | three `Decision { }` literals: `body: None` |
|
||
| `crates/brokerd/tests/ledger.rs` | one `Decision { }` literal: `body: None` |
|
||
| `crates/brokerd/tests/support/audit_dir.rs` | `denied()`: `body: None` |
|
||
| `crates/brokerd/src/ledger/mod.rs` | `decide_with_body`, `decide_forced`, `decide_inner`; event includes `body` |
|
||
| `crates/brokerd/src/consult.rs` | `NOT_CONFIGURED` |
|
||
| `crates/brokerd/src/broker.rs` | consult uses `wire_body` then `decide_forced` / `decide_with_body` |
|
||
| `crates/brokerd/tests/consult_deny.rs` | new, given |
|
||
| everything else | **no change**: not `main.rs`, not HTTP, not `docs/runbook.md` |
|
||
|
||
`loopd/tests/end_to_end.rs` matches `Decision` with `..` and needs no edit. Do not add
|
||
`body` to `AuditEvent::Approval`.
|
||
|
||
## Files
|
||
|
||
- Copy, from `docs/plans/M6b/files/` (the same path under it): the new test
|
||
`crates/brokerd/tests/consult_deny.rs`.
|
||
- Modify: `crates/bxcore/src/audit.rs`, `crates/bxcore/tests/records.rs`,
|
||
`crates/brokerd/src/ledger/mod.rs`, `crates/brokerd/src/consult.rs`,
|
||
`crates/brokerd/src/broker.rs`, `crates/brokerd/tests/ledger.rs`,
|
||
`crates/brokerd/tests/support/audit_dir.rs`, `docs/implementer-log.md`.
|
||
- Do not touch: any other file. Do not edit `docs/runbook.md`.
|
||
|
||
## Facts you need
|
||
|
||
- **Serde.** `body` is last on `Decision`. `#[serde(default, skip_serializing_if = "Option::is_none")]`.
|
||
Existing JSONL fixtures without `body` still decode. A `None` still equals a decoded
|
||
record that omitted the field.
|
||
- **`decide` stays.** Existing callers keep `decide(...)`. Consult goes through
|
||
`decide_with_body` or `decide_forced`. `decide` is `decide_inner(..., None, None)`.
|
||
- **`decide_forced` does not match grants.** The outcome is the given `Denial`. The
|
||
noconfig test's rig has no consult grant; matching would be `NoGrant`, not
|
||
`InvalidArguments`.
|
||
- **`NOT_CONFIGURED`** is exactly
|
||
`consult is not configured; see docs/runbook.md#consult-not-configured` (no trailing
|
||
period). `scripts/check-runbook.sh` reads this pointer; the heading already exists.
|
||
- **`Denial`** is `crate::policy::Denial { reason, grant, grant_sha256, detail }`.
|
||
- **Wire body on noconfig.** `wire_body(None, arguments)`: empty model, temperature 0.6,
|
||
one user message.
|
||
- The items the edits use. This table is complete.
|
||
|
||
| Item | Signature |
|
||
|---|---|
|
||
| `consult::wire_body` | `wire_body(consult: Option<&Consult>, arguments: &str) -> String` |
|
||
| `consult::CONSULT` | `&str`, `"consult"` |
|
||
| `Ledger::decide` | `decide(&self, request: ToolRequest, grants: &Grants, now: Timestamp) -> Decided` |
|
||
| `policy::Denial` | `{ reason: DenyReason, grant: Option<String>, grant_sha256: Option<Hash32>, detail: Option<String> }` |
|
||
| `DenyReason::InvalidArguments` | in `bxcore` |
|
||
| `DenyReason::TaintTooHigh` | in `bxcore` |
|
||
| `Broker::cfg` | `cfg(&self) -> &Config` |
|
||
| `Broker::ledger` | `ledger(&self) -> &Ledger` |
|
||
| `Broker::grants` | as used in `handle` already |
|
||
|
||
## The edits, exactly
|
||
|
||
Each quoted "before" block occurs exactly once after task 04.
|
||
|
||
### `crates/bxcore/src/audit.rs`
|
||
|
||
**E1.** The `Decision` fields ending
|
||
|
||
```rust
|
||
taint: DataClass,
|
||
untrusted: bool,
|
||
},
|
||
Approval {
|
||
```
|
||
|
||
become
|
||
|
||
```rust
|
||
taint: DataClass,
|
||
untrusted: bool,
|
||
/// The JSON body `consult` would send (or sent). Other tools leave this unset.
|
||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||
body: Option<String>,
|
||
},
|
||
Approval {
|
||
```
|
||
|
||
Do not add `body` to `Approval`.
|
||
|
||
### `crates/bxcore/tests/records.rs`
|
||
|
||
**E2, E3, E4.** Each of the three `AuditEvent::Decision {` literals gains `body: None,`
|
||
after `untrusted: …,`. They are:
|
||
|
||
1. `tool: "read_file"` / `untrusted: false,` then `},` of that event
|
||
2. `tool: "shell"` / `untrusted: true,` then `},` of that event
|
||
3. `tool: "rm_rf"` / `untrusted: false,` then `},` of that event
|
||
|
||
Each `untrusted: false,` / `untrusted: true,` that is the last field of a `Decision`
|
||
becomes that line plus `body: None,`. Do not add it to `Approval` or `Result`.
|
||
|
||
### `crates/brokerd/tests/ledger.rs`
|
||
|
||
**E5.** In `an_allowed_call_is_recorded_in_full`, after `untrusted: false,` of the
|
||
`AuditEvent::Decision` add `body: None,`.
|
||
|
||
### `crates/brokerd/tests/support/audit_dir.rs`
|
||
|
||
**E6.** In `denied()`, after `untrusted: false,` add `body: None,`.
|
||
|
||
### `crates/brokerd/src/consult.rs`
|
||
|
||
**N1.** Immediately before `pub const SECRET_POINTER: &str = "see docs/runbook.md#consult-secret-unreadable";` insert:
|
||
|
||
```rust
|
||
/// Denied when `brokerd.toml` has no `[consult]` table.
|
||
pub const NOT_CONFIGURED: &str =
|
||
"consult is not configured; see docs/runbook.md#consult-not-configured";
|
||
```
|
||
|
||
### `crates/brokerd/src/ledger/mod.rs`
|
||
|
||
**D1.** Replace the start of `decide`
|
||
|
||
```rust
|
||
/// Decide and record a call.
|
||
pub fn decide(&self, request: ToolRequest, grants: &Grants, now: Timestamp) -> Decided {
|
||
let session = request.session.clone();
|
||
```
|
||
|
||
with
|
||
|
||
```rust
|
||
/// Decide and record a call.
|
||
pub fn decide(&self, request: ToolRequest, grants: &Grants, now: Timestamp) -> Decided {
|
||
self.decide_inner(request, grants, now, None, None)
|
||
}
|
||
|
||
/// Decide and record a call, storing `body` on the decision (consult).
|
||
pub fn decide_with_body(
|
||
&self,
|
||
request: ToolRequest,
|
||
grants: &Grants,
|
||
now: Timestamp,
|
||
body: Option<String>,
|
||
) -> Decided {
|
||
self.decide_inner(request, grants, now, body, None)
|
||
}
|
||
|
||
/// Record `forced` as the denial, with `body`, without matching grants.
|
||
pub fn decide_forced(
|
||
&self,
|
||
request: ToolRequest,
|
||
grants: &Grants,
|
||
now: Timestamp,
|
||
body: Option<String>,
|
||
forced: crate::policy::Denial,
|
||
) -> Decided {
|
||
self.decide_inner(request, grants, now, body, Some(forced))
|
||
}
|
||
|
||
fn decide_inner(
|
||
&self,
|
||
request: ToolRequest,
|
||
grants: &Grants,
|
||
now: Timestamp,
|
||
body: Option<String>,
|
||
forced: Option<crate::policy::Denial>,
|
||
) -> Decided {
|
||
let session = request.session.clone();
|
||
```
|
||
|
||
**D2.** The outcome assignment. The lines
|
||
|
||
```rust
|
||
let (outcome, grant, grant_sha256, payload) = match grants {
|
||
Err(_) => (
|
||
DecisionRecord::Denied {
|
||
reason: DenyReason::GrantsInvalid,
|
||
},
|
||
None,
|
||
None,
|
||
RecordPayload::Simple(DenyReason::GrantsInvalid),
|
||
),
|
||
Ok(set) => match state {
|
||
Err(()) => (
|
||
DecisionRecord::Denied {
|
||
reason: DenyReason::StateUnreadable,
|
||
},
|
||
None,
|
||
None,
|
||
RecordPayload::Simple(DenyReason::StateUnreadable),
|
||
),
|
||
Ok(state) => match crate::policy::decide(request, set, state, now) {
|
||
crate::policy::Outcome::Allowed(decision) => (
|
||
DecisionRecord::Allowed {},
|
||
Some(decision.grant().to_string()),
|
||
Some(decision.grant_sha256()),
|
||
RecordPayload::Allowed(decision),
|
||
),
|
||
crate::policy::Outcome::Ask(ask) => (
|
||
DecisionRecord::Ask {},
|
||
Some(ask.grant().to_string()),
|
||
Some(ask.grant_sha256()),
|
||
RecordPayload::Ask(ask),
|
||
),
|
||
crate::policy::Outcome::Denied(denial) => (
|
||
DecisionRecord::Denied {
|
||
reason: denial.reason,
|
||
},
|
||
denial.grant.clone(),
|
||
denial.grant_sha256,
|
||
RecordPayload::Denial(denial),
|
||
),
|
||
},
|
||
},
|
||
};
|
||
```
|
||
|
||
become
|
||
|
||
```rust
|
||
let (outcome, grant, grant_sha256, payload) = if let Some(denial) = forced {
|
||
(
|
||
DecisionRecord::Denied {
|
||
reason: denial.reason,
|
||
},
|
||
denial.grant.clone(),
|
||
denial.grant_sha256,
|
||
RecordPayload::Denial(denial),
|
||
)
|
||
} else {
|
||
match grants {
|
||
Err(_) => (
|
||
DecisionRecord::Denied {
|
||
reason: DenyReason::GrantsInvalid,
|
||
},
|
||
None,
|
||
None,
|
||
RecordPayload::Simple(DenyReason::GrantsInvalid),
|
||
),
|
||
Ok(set) => match state {
|
||
Err(()) => (
|
||
DecisionRecord::Denied {
|
||
reason: DenyReason::StateUnreadable,
|
||
},
|
||
None,
|
||
None,
|
||
RecordPayload::Simple(DenyReason::StateUnreadable),
|
||
),
|
||
Ok(state) => match crate::policy::decide(request, set, state, now) {
|
||
crate::policy::Outcome::Allowed(decision) => (
|
||
DecisionRecord::Allowed {},
|
||
Some(decision.grant().to_string()),
|
||
Some(decision.grant_sha256()),
|
||
RecordPayload::Allowed(decision),
|
||
),
|
||
crate::policy::Outcome::Ask(ask) => (
|
||
DecisionRecord::Ask {},
|
||
Some(ask.grant().to_string()),
|
||
Some(ask.grant_sha256()),
|
||
RecordPayload::Ask(ask),
|
||
),
|
||
crate::policy::Outcome::Denied(denial) => (
|
||
DecisionRecord::Denied {
|
||
reason: denial.reason,
|
||
},
|
||
denial.grant.clone(),
|
||
denial.grant_sha256,
|
||
RecordPayload::Denial(denial),
|
||
),
|
||
},
|
||
},
|
||
}
|
||
};
|
||
```
|
||
|
||
**D3.** The event fields ending
|
||
|
||
```rust
|
||
taint: recorded.taint,
|
||
untrusted: recorded.untrusted,
|
||
};
|
||
```
|
||
|
||
become
|
||
|
||
```rust
|
||
taint: recorded.taint,
|
||
untrusted: recorded.untrusted,
|
||
body,
|
||
};
|
||
```
|
||
|
||
### `crates/brokerd/src/broker.rs`
|
||
|
||
**H1.** The lines
|
||
|
||
```rust
|
||
let now = Timestamp::now();
|
||
let grants = broker.grants();
|
||
let answer = match broker.ledger().decide(request, &grants, now) {
|
||
```
|
||
|
||
become
|
||
|
||
```rust
|
||
let now = Timestamp::now();
|
||
let grants = broker.grants();
|
||
let decided = if request.tool == crate::consult::CONSULT {
|
||
let body = crate::consult::wire_body(broker.cfg().consult.as_ref(), &request.arguments);
|
||
if broker.cfg().consult.is_none() {
|
||
broker.ledger().decide_forced(
|
||
request,
|
||
&grants,
|
||
now,
|
||
Some(body),
|
||
crate::policy::Denial {
|
||
reason: bxcore::DenyReason::InvalidArguments,
|
||
grant: None,
|
||
grant_sha256: None,
|
||
detail: Some(crate::consult::NOT_CONFIGURED.to_string()),
|
||
},
|
||
)
|
||
} else {
|
||
broker
|
||
.ledger()
|
||
.decide_with_body(request, &grants, now, Some(body))
|
||
}
|
||
} else {
|
||
broker.ledger().decide(request, &grants, now)
|
||
};
|
||
let answer = match decided {
|
||
```
|
||
|
||
## Steps
|
||
|
||
- [ ] **1. Copy the given test.**
|
||
|
||
```sh
|
||
cp docs/plans/M6b/files/crates/brokerd/tests/consult_deny.rs crates/brokerd/tests/
|
||
```
|
||
|
||
- [ ] **2. See it fail.** `cargo test -p brokerd --test consult_deny`, timeout 600000 ms:
|
||
the target does not compile (`E0560` / no field `body` on `AuditEvent::Decision`, or
|
||
unresolved `NOT_CONFIGURED`).
|
||
- [ ] **3. The edits.** E1–E6, N1, D1–D3, H1. `cargo check -p bxcore -p brokerd` prints no
|
||
error and no warning. `wc -l crates/brokerd/src/ledger/mod.rs` and
|
||
`wc -l crates/brokerd/src/broker.rs` each ≤ 500.
|
||
- [ ] **4. The tests.** `cargo test -p brokerd --test consult_deny --test ledger --test
|
||
audit` and `cargo test -p bxcore --test records`: every test passes, 3 in
|
||
`consult_deny`. Existing Decision equality tests still pass.
|
||
- [ ] **5. Run the gate.** `cargo fmt --all`, then `make gate`, timeout 600000 ms. Last
|
||
line `gate: ok`. Test count **1609** (1606 before, plus 3). After `gate: ok`, do not add
|
||
up `test result:` lines.
|
||
- [ ] **6. Log and commit.** Task cell `M6b/05-brokerd-consult-audit-body`. Read only the
|
||
first 12 lines of `docs/implementer-log.md`; insert the row under the header. Stage with
|
||
`git add crates/bxcore crates/brokerd docs/implementer-log.md Cargo.lock`, then commit
|
||
as `AGENTS.md` says.
|
||
|
||
## Done when
|
||
|
||
- `make gate` prints `gate: ok` with 1609 tests.
|
||
- `git diff HEAD~1 --stat` lists only the files in the Files section plus the log.
|
||
|
||
## Stop and report if
|
||
|
||
- The branch check fails; a "before" block is missing or duplicated (in particular E1
|
||
matching `Approval` instead of `Decision`); step 2 compiles; you edit `docs/runbook.md`
|
||
or `main.rs`; you add `body` to `Approval`; `cargo check` warns; an existing ledger or
|
||
records test fails; a test outside the named files fails in the gate (`git stash -u`,
|
||
rerun, `git stash pop`, report both).
|
||
|
||
## What the given tests pin
|
||
|
||
No `[consult]` is `InvalidArguments` naming `consult-not-configured` and `docs/runbook.md`;
|
||
the audit `body` equals `wire_body(None, question)` and has no `tools`. A secret session
|
||
against a private consult grant is `TaintTooHigh` with no detail; the body has
|
||
`"model":"test-model"` and one message. An unknown `history` key is `InvalidArguments`.
|
||
Nothing reaches the runtime (`count() == 0`).
|