diff --git a/crates/brokerd/src/admin.rs b/crates/brokerd/src/admin.rs index e13b842..3a9b113 100644 --- a/crates/brokerd/src/admin.rs +++ b/crates/brokerd/src/admin.rs @@ -80,6 +80,7 @@ pub fn handle(mut stream: UnixStream, broker: &Broker) { entry, Answer::Approved { by: Some(BY.to_string()), + post: None, }, now, ); @@ -96,6 +97,7 @@ pub fn handle(mut stream: UnixStream, broker: &Broker) { entry, Answer::Refused { by: Some(BY.to_string()), + post: None, reason, }, now, diff --git a/crates/brokerd/src/approvals.rs b/crates/brokerd/src/approvals.rs index 92ff589..7e4cc3f 100644 --- a/crates/brokerd/src/approvals.rs +++ b/crates/brokerd/src/approvals.rs @@ -2,7 +2,7 @@ //! gone. In memory only. use crate::policy::{Ask, Decision}; -use proto::{DenyReason, PendingApproval, Timestamp}; +use proto::{DecisionRecord, DenyReason, PendingApproval, Timestamp}; use std::collections::BTreeMap; use std::sync::mpsc::{Receiver, Sender, channel}; use std::sync::{Mutex, MutexGuard}; @@ -14,6 +14,29 @@ pub enum Verdict { Denied(DenyReason), } +/// An owner's answer to a pending call. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Answer { + /// `by` is "bxctl" or a Mattermost user id; `post` the Mattermost post that answered. + Approved { + by: Option, + post: Option, + }, + Refused { + by: Option, + post: Option, + reason: Option, + }, + Expired, +} + +/// What an approval leaves: the verdict (run or deny) and the record written. +#[derive(Debug)] +pub struct Answered { + pub verdict: Verdict, + pub outcome: DecisionRecord, +} + #[derive(Debug)] pub struct Entry { pub info: PendingApproval, @@ -52,6 +75,11 @@ impl Table { receiver } + /// Is approval `id` still pending? Only a hint: it may be taken the moment after. + pub fn contains(&self, id: u64) -> bool { + self.lock().contains_key(&id) + } + pub fn take(&self, id: u64) -> Option { self.lock().remove(&id) } diff --git a/crates/brokerd/src/ledger.rs b/crates/brokerd/src/ledger.rs index bb35b1a..cc87658 100644 --- a/crates/brokerd/src/ledger.rs +++ b/crates/brokerd/src/ledger.rs @@ -10,6 +10,7 @@ use proto::{ }; use crate::approvals::Verdict; +pub use crate::approvals::{Answer, Answered}; use crate::audit::{AuditError, Writer}; use crate::grants::GrantSet; use crate::policy::{Ask, Denial, Label, SessionState}; @@ -82,26 +83,6 @@ enum RecordPayload { Simple(DenyReason), } -/// An owner's answer to a pending call. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum Answer { - Approved { - by: Option, - }, - Refused { - by: Option, - reason: Option, - }, - Expired, -} - -/// What an approval leaves: the verdict (run or deny) and the record written. -#[derive(Debug)] -pub struct Answered { - pub verdict: Verdict, - pub outcome: DecisionRecord, -} - struct Inner { audit: Box, state: StateStore, @@ -339,10 +320,17 @@ impl Ledger { }; // 3. The answer and its fields. - let (answer, by, reason) = match &answer { - Answer::Approved { by } => (ApprovalAnswer::Approved, by.clone(), None), - Answer::Refused { by, reason } => (ApprovalAnswer::Refused, by.clone(), reason.clone()), - Answer::Expired => (ApprovalAnswer::Expired, None, None), + let (answer, by, post, reason) = match &answer { + Answer::Approved { by, post } => { + (ApprovalAnswer::Approved, by.clone(), post.clone(), None) + } + Answer::Refused { by, post, reason } => ( + ApprovalAnswer::Refused, + by.clone(), + post.clone(), + reason.clone(), + ), + Answer::Expired => (ApprovalAnswer::Expired, None, None, None), }; // 4. The re-decision (approved only); every other answer is a denial. @@ -382,7 +370,7 @@ impl Ledger { decision, answer, by, - post: None, + post, reason, outcome: outcome.clone(), grant, diff --git a/crates/brokerd/tests/broker_pending.rs b/crates/brokerd/tests/broker_pending.rs index 54e8093..89402f2 100644 --- a/crates/brokerd/tests/broker_pending.rs +++ b/crates/brokerd/tests/broker_pending.rs @@ -73,6 +73,7 @@ fn answer_entry( fn approved() -> Answer { Answer::Approved { by: Some("bxctl".to_string()), + post: None, } } @@ -166,6 +167,7 @@ fn a_denied_verdict_reaches_the_requester_and_nothing_runs() { let (mut stream, approval, _) = start(&broker, r#"{"path":"/n/a"}"#); let refused = Answer::Refused { by: Some("bxctl".to_string()), + post: None, reason: None, }; answer_by_hand(&broker, approval, refused); diff --git a/crates/brokerd/tests/broker_sequence.rs b/crates/brokerd/tests/broker_sequence.rs index 9976f1d..021e6f0 100644 --- a/crates/brokerd/tests/broker_sequence.rs +++ b/crates/brokerd/tests/broker_sequence.rs @@ -177,10 +177,14 @@ fn approved_and_refused_calls_run_only_after_an_approval() { continue; }; let answer = if item.approval % 2 == 0 { - Answer::Approved { by: None } + Answer::Approved { + by: None, + post: None, + } } else { Answer::Refused { by: None, + post: None, reason: None, } }; diff --git a/crates/brokerd/tests/ledger_answer.rs b/crates/brokerd/tests/ledger_answer.rs index cd543d9..89c8913 100644 --- a/crates/brokerd/tests/ledger_answer.rs +++ b/crates/brokerd/tests/ledger_answer.rs @@ -33,6 +33,7 @@ fn pending(ledger: &Ledger) -> Ask { fn bxctl() -> Answer { Answer::Approved { by: Some("bxctl".to_string()), + post: None, } } @@ -206,6 +207,7 @@ fn a_refusal_is_recorded_with_the_owners_reason() { let ask = pending(&ledger); let refused = Answer::Refused { by: Some("bxctl".to_string()), + post: None, reason: Some("not now".to_string()), }; let answered = ledger.answer(ask, 0, refused, &asking(), now()); diff --git a/crates/brokerd/tests/ledger_post.rs b/crates/brokerd/tests/ledger_post.rs new file mode 100644 index 0000000..48b2614 --- /dev/null +++ b/crates/brokerd/tests/ledger_post.rs @@ -0,0 +1,118 @@ +//! An answer from Mattermost records who answered and in which post, approved or refused; the +//! table says whether an approval is still pending (M4b spec, section 5). Do not edit. + +#[path = "support/build.rs"] +mod build; +#[path = "support/rig.rs"] +mod rig; +#[path = "support/sink.rs"] +mod sink; +#[path = "support/tmp.rs"] +mod tmp; + +use brokerd::approvals::Table; +use brokerd::ledger::{Answer, Decided, Grants, Ledger}; +use brokerd::policy::Ask; +use build::{grant, now, read, set}; +use proto::{ApprovalAnswer, AuditEvent, CallId, DataClass, Mode, PendingApproval, SessionId}; +use rig::Rig; + +const KYLE: &str = "k0000000000000000000000000"; +const POST: &str = "c0000000000000000000000001"; + +fn asking() -> Grants { + Ok(set(vec![grant("n", "read_file", Mode::Ask).paths(&["/n"])])) +} + +fn pending(ledger: &Ledger) -> Ask { + match ledger.decide(read("/n/deep/a"), &asking(), now()) { + Decided::Ask { ask, seq: 0, .. } => ask, + other => panic!("{other:?}"), + } +} + +/// The one `Approval` record's (answer, by, post, reason). +fn recorded( + rig: &Rig, +) -> ( + ApprovalAnswer, + Option, + Option, + Option, +) { + let events = rig.events(); + match &events[1] { + AuditEvent::Approval { + answer, + by, + post, + reason, + .. + } => (*answer, by.clone(), post.clone(), reason.clone()), + other => panic!("{other:?}"), + } +} + +#[test] +fn an_approval_from_mattermost_names_the_user_and_the_post() { + let rig = Rig::new("post-approve"); + let ledger = rig.ledger(); + let answer = Answer::Approved { + by: Some(KYLE.to_string()), + post: Some(POST.to_string()), + }; + ledger.answer(pending(&ledger), 0, answer, &asking(), now()); + assert_eq!( + recorded(&rig), + ( + ApprovalAnswer::Approved, + Some(KYLE.to_string()), + Some(POST.to_string()), + None + ) + ); +} + +#[test] +fn a_refusal_from_mattermost_names_them_and_keeps_the_reason() { + let rig = Rig::new("post-refuse"); + let ledger = rig.ledger(); + let answer = Answer::Refused { + by: Some(KYLE.to_string()), + post: Some(POST.to_string()), + reason: Some("not on a Friday".to_string()), + }; + ledger.answer(pending(&ledger), 0, answer, &asking(), now()); + assert_eq!( + recorded(&rig), + ( + ApprovalAnswer::Refused, + Some(KYLE.to_string()), + Some(POST.to_string()), + Some("not on a Friday".to_string()) + ) + ); +} + +#[test] +fn the_table_says_what_is_pending() { + let rig = Rig::new("post-table"); + let ledger = rig.ledger(); + let table = Table::new(); + assert!(!table.contains(0)); + let info = PendingApproval { + approval: 0, + session: SessionId::new("s1").unwrap(), + call: CallId(1), + tool: "read_file".to_string(), + arguments: "{}".to_string(), + grant: "n".to_string(), + taint: DataClass::Private, + created: now(), + expires: now(), + }; + let _verdict = table.insert(info, pending(&ledger)); + assert!(table.contains(0) && !table.contains(1)); + assert!(table.take(0).is_some()); + assert!(!table.contains(0)); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index b57c0ba..8e32ad9 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -6,6 +6,7 @@ reviewer adds findings under "Reviews" once per milestone. | Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model | |---|---|---|---|---|---|---|---| +| M4b/04-brokerd-answer-post | 2026-09-24 | done | 1 | pass | none | Moved `enum Answer` and `struct Answered` from `ledger.rs` into `approvals.rs` before `pub struct Entry` (with doc comments), added `pub use crate::approvals::{Answer, Answered};` next to the `Verdict` use and `DecisionRecord` to the `use proto` line in `approvals.rs`. Added the `post` field to `Answer::Approved`/`Answer::Refused`, threaded it through `ledger::answer` (step 3 binds `post`, step 6 writes it into `AuditEvent::Approval` instead of `None`), and gave the two `bxctl` answers in `admin.rs` `post: None`. Added `Table::contains` before `take`. Copied `ledger_post.rs`, `broker_pending.rs`, `broker_sequence.rs` and `ledger_answer.rs` into `crates/brokerd/tests/`; every suite passes, `ledger_post` 3 passed, `ledger.rs` is 487 lines, and `make gate` prints `gate: ok` on the first run. | ? | | M4b/03-bxctl-deny | 2026-09-24 | done | 1 | pass | none | Copied `docs/plans/M4b/files/crates/bxctl/tests/cli_deny.rs` into `crates/bxctl/tests/`. In `parse`, `"refuse" => parse_refuse(rest, home)` became `"refuse" | "deny" => parse_refuse(rest, home)`, so `deny` produces the same `Command::Refuse` as `refuse` for every argument list including the error cases. Added a blank line and the two-line `bxctl deny [--reason TEXT] [--admin-socket PATH]` / `Another name for refuse.` block to `USAGE` after the `refuse` block, keeping the two-space and six-space indents. The copied test's `deny_parses_as_refuse` and `usage_names_deny` both failed before the changes and pass after; `cargo test -p bxctl` is green and `make gate` prints `gate: ok` on the first run. | ? | | M4b/02-proto-gateway-wire | 2026-09-24 | done | 2 | fail | none | Copied `gateway.rs`, `gateway_wire.rs` and the eight `gateway_*.json` fixtures into place; added `pub mod gateway;` after `pub mod frame;` and the re-export to `lib.rs`, the four `Message` variants plus the `use` to `wire.rs`, and the four `kind` arms (changing "fourteen" to "eighteen") in `broker.rs`. Filled `is_mattermost_id` (`id.len() == 26 && every byte a-z0-9`) and `ApprovalReply::check` in the given order: `by`, `post`, then a reason over `MAX_REASON` chars by `chars().count()`. All 3 `gateway_wire` tests pass; the first `make gate` failed on clippy `collapsible_if`, fixed by collapsing the nested `if let` into a let-chain guard, and the second prints `gate: ok`. | ? | | M4b/01-proto-escape | 2026-09-24 | done | 1 | pass | none | Copied `docs/plans/M4b/files/crates/proto/src/escape.rs`, `bxctl/src/escape.rs` and `proto/tests/escape.rs` into place, and added `pub mod escape;` to `proto/src/lib.rs` after `pub mod class;`. The bxctl file is now the one-line re-export `pub use proto::escape::{escape_json_text, escape_model_text};`, so no bxctl source or test changed and behaviour is identical. `cargo test -p proto --test escape` reports 8 passed; every bxctl suite passes unchanged (the moved escape test now runs against `proto::escape`); `cargo fmt --all` then `make gate` prints `gate: ok` on the first run. | ? |