Close the remaining M3a spec review findings
brokerd internals: the ledger lock over the audit writer and session state, the pending table's take-it-and-answer rule, sticky audit_unavailable after a failed write, pure policy functions with an Ask type that only redecide turns into a Decision, how a waiting thread detects a lost connection without peek, fixed RunError text. Approvals: bxctl chat fetches the block from brokerd by id, shows the parsed arguments with invisible and bidi characters escaped, asks for the approval id instead of y, and escapes model text. A tool_denied event carries the reason to the owner. Also: BrokerPort timeouts, tests for runbook pointers and concurrency, threat-model notes, P14 for broker/sessions in the brief's State list. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
@@ -6,6 +6,8 @@ Newest first. A decision that changes `docs/design.md` lands in the same commit
|
||||
|
||||
| Date | Decision | Reason |
|
||||
|---|---|---|
|
||||
| 2026-09-18 | Spec review of M3a, approvals. The approval block in `bxctl chat` is fetched from `brokerd` by id, never taken from `loopd`'s event. It shows the parsed arguments serialised again, with control, invisible and direction-changing characters as `\uXXXX`. The owner types the approval id, not `y`. `bxctl chat` escapes model text the same way. A `tool_denied` turn event carries the reason to the owner. | A compromised `loopd` must not choose what the owner approves; `/etc` and `/etc` must look alike; U+202E can make a path read as another; a line already waiting in stdin must never approve; `chat` wrote model text raw, so a conceal sequence could hide the block. |
|
||||
| 2026-09-18 | Spec review of M3a, `brokerd` internals. One lock (the ledger) covers the audit writer and the session state files; the pending table has its own, and whoever removes an entry answers it. After any failed audit write, `brokerd` denies with `audit_unavailable` until restarted. `policy` is pure, and `decide` never returns a `Decision` for an `ask` grant: only `redecide` turns an `Ask` into one. A `RunError`'s text is fixed text, never tool output, and a failure does not raise taint. `BrokerPort` waits `[broker] timeout_ms` for a first frame, and until `expires` plus that after a pending one. | Without the lock two results could lower a session's taint. A partial line followed by more records is a broken chain. `loopd` logs failures as `public`, so tool output in one would be unlabelled. |
|
||||
| 2026-09-18 | Spec review of M3a, policy. A `deny` grant must have `max_taint = "secret"` or the grant set is invalid. A result's `result_class` and `untrusted` are combined over every matching grant (highest class; untrusted if any says so), and within a mode the grant with the longest matched path wins before the id tie-break. A grant path of `/` is invalid. | A `deny` grant with a lower `max_taint` stopped denying once the session read a secret. With overlapping `auto` grants the id tie-break chose the label, so a secret could come back `private`. M3b mounts grant paths at the same path, where `/` would replace the tool image. |
|
||||
| 2026-09-18 | Spec review of M3a, audit. A line followed by a matching `Recovery` is recovered whether or not it parses; an unparseable last line is recovered like a torn one. The writer never moves to an earlier day's file, and `Recovery` and `AcceptedBreak` go in the latest file. `--accept-break` verifies the whole log; the verifier checks an `AcceptedBreak`'s `file`, `line`, `last_good`, `prev` and `seq`, and the `seq` is counted from lines, not read from them. The `Approval` record carries the re-decision's grant and session state. The report lists allowed calls with no `Result`. | As first written, a record cut exactly before its newline made `brokerd` break its own chain; so did a clock stepped back over midnight; a break in an older file could never be accepted; and a damaged line could choose the next `seq`. |
|
||||
| 2026-09-18 | M3 is split. M3a is the decision path: grants and matching, session taint, the hash-chained audit log, approvals through `bxctl`, the `broker.sock` and `admin.sock` protocol, and `loopd`'s `BrokerPort`; tools do not run (the runner is a trait with a refusing implementation). M3b is the Podman runner, the four tools and the egress proxy. Review happens after each. | The security logic is reviewed before real tools run on it, as with M2a and M2b. Spec `docs/specs/2026-09-18-m3a-decision-path.md`. |
|
||||
@@ -76,3 +78,4 @@ Later changes to the brief:
|
||||
| # | Change | Reason |
|
||||
|---|---|---|
|
||||
| P13 | Authority contract 4: add "Tool containers run from one image built from source by Nix and named by digest; nothing is pulled at call time." | The brief does not say where tool images come from; a pull at call time would be unlisted egress. Decided 2026-09-18 above. |
|
||||
| P14 | State: add "`broker/sessions/<id>.json` — each session's taint and untrusted flag, written only by `brokerd`". | The M3a spec keeps this state in files the brief's list does not name. It can be rebuilt from the audit log's `result` records, so the audit log stays the record of truth. |
|
||||
|
||||
Reference in New Issue
Block a user