diff --git a/crates/brokerd/src/broker.rs b/crates/brokerd/src/broker.rs index 468aa07..06f9a23 100644 --- a/crates/brokerd/src/broker.rs +++ b/crates/brokerd/src/broker.rs @@ -87,7 +87,7 @@ impl Broker { } } -/// The snake_case wire name of a message: all fourteen kinds, none with a leading underscore. +/// The snake_case wire name of a message: all eighteen kinds, none with a leading underscore. pub fn kind(msg: &Message) -> &'static str { match msg { Message::ToolRequest(_) => "tool_request", @@ -104,6 +104,10 @@ pub fn kind(msg: &Message) -> &'static str { Message::Ok(_) => "ok", Message::CheckGrants(_) => "check_grants", Message::GrantsReport(_) => "grants_report", + Message::ApprovalRequest(_) => "approval_request", + Message::ApprovalPosted(_) => "approval_posted", + Message::ApprovalReply(_) => "approval_reply", + Message::ApprovalOutcome(_) => "approval_outcome", } } diff --git a/crates/proto/src/gateway.rs b/crates/proto/src/gateway.rs new file mode 100644 index 0000000..21883ca --- /dev/null +++ b/crates/proto/src/gateway.rs @@ -0,0 +1,90 @@ +//! The messages on `gateway.sock`, between `brokerd` and `gatewayd`: one connection carries one +//! pending approval to Mattermost and its answer back (M4b spec, section 4). + +use serde::{Deserialize, Serialize}; + +use crate::wire::DenyReason; +use crate::wire::PendingApproval; + +/// The most characters a refusal's reason may have. +pub const MAX_REASON: usize = 500; + +/// `brokerd` → `gatewayd`, first: the approval to post. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ApprovalRequest { + pub approval: PendingApproval, +} + +/// `gatewayd` → `brokerd`: it is posted, in these posts, first part first. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ApprovalPosted { + pub posts: Vec, +} + +/// `gatewayd` → `brokerd`: an approver answered, with this post. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ApprovalReply { + pub approval: u64, + pub approved: bool, + pub by: String, + pub post: String, + pub reason: Option, +} + +/// `brokerd` → `gatewayd`, last: what came of the answer. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ApprovalOutcome { + pub outcome: GatewayOutcome, +} + +// JSON: {"outcome":"denied","reason":"no_grant"} ; the tag sits beside the fields +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "outcome", rename_all = "snake_case", deny_unknown_fields)] +pub enum GatewayOutcome { + /// Approved, and the grants as they are now still allow it. + Runs {}, + /// Approved, but denied on the re-check, or the answer could not be recorded. + Denied { reason: DenyReason }, + /// The refusal is recorded. + Refused {}, + /// It was no longer pending when the answer came. + Gone {}, +} + +/// A Mattermost id: 26 characters of `a-z0-9`. +pub fn is_mattermost_id(id: &str) -> bool { + // `id.len() == 26` and every byte `b.is_ascii_lowercase() || b.is_ascii_digit()`. + id.len() == 26 + && id + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit()) +} + +impl ApprovalReply { + /// `by` and `post` are Mattermost ids and the reason is at most `MAX_REASON` characters; the + /// first problem otherwise. + pub fn check(&self) -> Result<(), String> { + // In this order, each an Err with exactly this text: + // 1. `!is_mattermost_id(&self.by)`: "the answer's `by` is not a Mattermost id". + // 2. `!is_mattermost_id(&self.post)`: "the answer's `post` is not a Mattermost id". + // 3. a reason with more than `MAX_REASON` characters (`r.chars().count()`, not bytes): + // `format!("the reason is over {MAX_REASON} characters")`. + // 4. Ok(()). + if !is_mattermost_id(&self.by) { + return Err("the answer's `by` is not a Mattermost id".to_string()); + } + if !is_mattermost_id(&self.post) { + return Err("the answer's `post` is not a Mattermost id".to_string()); + } + if let Some(reason) = &self.reason + && reason.chars().count() > MAX_REASON + { + return Err(format!("the reason is over {MAX_REASON} characters")); + } + Ok(()) + } +} diff --git a/crates/proto/src/lib.rs b/crates/proto/src/lib.rs index 283b61f..ca0eab8 100644 --- a/crates/proto/src/lib.rs +++ b/crates/proto/src/lib.rs @@ -5,6 +5,7 @@ pub mod chain; pub mod class; pub mod escape; pub mod frame; +pub mod gateway; pub mod grant; pub mod hash; pub mod hosts; @@ -20,6 +21,10 @@ pub use audit::{ pub use chain::{ChainFailure, ChainReport, ChainVerifier, Location, TornTail}; pub use class::DataClass; pub use frame::{FrameError, MAX_FRAME, read_frame, write_frame}; +pub use gateway::{ + ApprovalOutcome, ApprovalPosted, ApprovalReply, ApprovalRequest, GatewayOutcome, MAX_REASON, + is_mattermost_id, +}; pub use grant::{Constraints, Grant, Mode}; pub use hash::{HashError, Sha256, sha256}; pub use ids::{CallId, Epoch, Hash32, SessionId, Timestamp, ValueError}; diff --git a/crates/proto/src/wire.rs b/crates/proto/src/wire.rs index 354d814..16f6f8b 100644 --- a/crates/proto/src/wire.rs +++ b/crates/proto/src/wire.rs @@ -1,5 +1,6 @@ use serde::{Deserialize, Serialize}; +use crate::gateway::{ApprovalOutcome, ApprovalPosted, ApprovalReply, ApprovalRequest}; use crate::{CallId, DataClass, DecisionRecord, SessionId, Timestamp, Usage}; pub const PROTOCOL_VERSION: u32 = 1; @@ -35,6 +36,10 @@ pub enum Message { Ok(Empty), CheckGrants(Empty), GrantsReport(GrantsReport), + ApprovalRequest(ApprovalRequest), + ApprovalPosted(ApprovalPosted), + ApprovalReply(ApprovalReply), + ApprovalOutcome(ApprovalOutcome), } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/crates/proto/tests/fixtures/wire/gateway_outcome_denied.json b/crates/proto/tests/fixtures/wire/gateway_outcome_denied.json new file mode 100644 index 0000000..c6fac58 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/gateway_outcome_denied.json @@ -0,0 +1 @@ +{"v":1,"id":2,"final":true,"msg":{"kind":"approval_outcome","body":{"outcome":{"outcome":"denied","reason":"no_grant"}}}} diff --git a/crates/proto/tests/fixtures/wire/gateway_outcome_gone.json b/crates/proto/tests/fixtures/wire/gateway_outcome_gone.json new file mode 100644 index 0000000..3274e15 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/gateway_outcome_gone.json @@ -0,0 +1 @@ +{"v":1,"id":2,"final":true,"msg":{"kind":"approval_outcome","body":{"outcome":{"outcome":"gone"}}}} diff --git a/crates/proto/tests/fixtures/wire/gateway_outcome_refused.json b/crates/proto/tests/fixtures/wire/gateway_outcome_refused.json new file mode 100644 index 0000000..52cab33 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/gateway_outcome_refused.json @@ -0,0 +1 @@ +{"v":1,"id":2,"final":true,"msg":{"kind":"approval_outcome","body":{"outcome":{"outcome":"refused"}}}} diff --git a/crates/proto/tests/fixtures/wire/gateway_outcome_runs.json b/crates/proto/tests/fixtures/wire/gateway_outcome_runs.json new file mode 100644 index 0000000..fc0147a --- /dev/null +++ b/crates/proto/tests/fixtures/wire/gateway_outcome_runs.json @@ -0,0 +1 @@ +{"v":1,"id":2,"final":true,"msg":{"kind":"approval_outcome","body":{"outcome":{"outcome":"runs"}}}} diff --git a/crates/proto/tests/fixtures/wire/gateway_posted.json b/crates/proto/tests/fixtures/wire/gateway_posted.json new file mode 100644 index 0000000..88e66b0 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/gateway_posted.json @@ -0,0 +1 @@ +{"v":1,"id":1,"final":false,"msg":{"kind":"approval_posted","body":{"posts":["p0000000000000000000000001","p0000000000000000000000002"]}}} diff --git a/crates/proto/tests/fixtures/wire/gateway_reply_approved.json b/crates/proto/tests/fixtures/wire/gateway_reply_approved.json new file mode 100644 index 0000000..ff42b64 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/gateway_reply_approved.json @@ -0,0 +1 @@ +{"v":1,"id":1,"final":true,"msg":{"kind":"approval_reply","body":{"approval":41,"approved":true,"by":"k0000000000000000000000000","post":"c0000000000000000000000001","reason":null}}} diff --git a/crates/proto/tests/fixtures/wire/gateway_reply_refused.json b/crates/proto/tests/fixtures/wire/gateway_reply_refused.json new file mode 100644 index 0000000..df3f197 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/gateway_reply_refused.json @@ -0,0 +1 @@ +{"v":1,"id":1,"final":true,"msg":{"kind":"approval_reply","body":{"approval":41,"approved":false,"by":"k0000000000000000000000000","post":"c0000000000000000000000001","reason":"not on a Friday"}}} diff --git a/crates/proto/tests/fixtures/wire/gateway_request.json b/crates/proto/tests/fixtures/wire/gateway_request.json new file mode 100644 index 0000000..e7d69e0 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/gateway_request.json @@ -0,0 +1 @@ +{"v":1,"id":1,"final":true,"msg":{"kind":"approval_request","body":{"approval":{"approval":41,"session":"mm-r0000000000000000000000001","call":3,"tool":"shell","arguments":"{\"command\":\"ls -la /srv\"}","grant":"shell-srv","taint":"private","created":"2026-09-24T08:05:00.000Z","expires":"2026-09-24T08:20:00.000Z"}}}} diff --git a/crates/proto/tests/gateway_wire.rs b/crates/proto/tests/gateway_wire.rs new file mode 100644 index 0000000..ce5886e --- /dev/null +++ b/crates/proto/tests/gateway_wire.rs @@ -0,0 +1,187 @@ +//! The four messages of `gateway.sock` against byte-exact fixtures, every object in them refusing +//! an unknown key, and the checks on an answer (M4b spec, section 4). Do not edit these or the +//! fixtures. + +use proto::{ + ApprovalOutcome, ApprovalPosted, ApprovalReply, ApprovalRequest, CallId, DataClass, DenyReason, + Envelope, GatewayOutcome, MAX_REASON, Message, PendingApproval, SessionId, Timestamp, + is_mattermost_id, +}; +use serde_json::Value; + +const BY: &str = "k0000000000000000000000000"; +const POST: &str = "c0000000000000000000000001"; + +fn fixture(name: &str) -> String { + let path = format!("{}/tests/fixtures/wire/{name}", env!("CARGO_MANIFEST_DIR")); + let text = std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("{path}: {e}")); + text.trim_end_matches('\n').to_string() +} + +/// The fixture must decode to `want`, and `want` must encode to exactly the fixture's bytes. +fn check(name: &str, id: u64, last: bool, msg: Message) { + let want = Envelope { + v: 1, + id, + r#final: last, + msg, + }; + let text = fixture(name); + let got: Envelope = serde_json::from_str(&text).unwrap_or_else(|e| panic!("{name}: {e}")); + assert_eq!(got, want, "{name}: decoded value"); + assert_eq!( + serde_json::to_string(&want).unwrap(), + text, + "{name}: encoded bytes" + ); +} + +fn pending() -> PendingApproval { + PendingApproval { + approval: 41, + session: SessionId::new("mm-r0000000000000000000000001").unwrap(), + call: CallId(3), + tool: "shell".to_string(), + arguments: r#"{"command":"ls -la /srv"}"#.to_string(), + grant: "shell-srv".to_string(), + taint: DataClass::Private, + created: Timestamp::parse("2026-09-24T08:05:00.000Z").unwrap(), + expires: Timestamp::parse("2026-09-24T08:20:00.000Z").unwrap(), + } +} + +fn reply(approved: bool, reason: Option<&str>) -> ApprovalReply { + ApprovalReply { + approval: 41, + approved, + by: BY.to_string(), + post: POST.to_string(), + reason: reason.map(str::to_string), + } +} + +#[test] +fn the_four_messages() { + check( + "gateway_request.json", + 1, + true, + Message::ApprovalRequest(ApprovalRequest { + approval: pending(), + }), + ); + let posts = vec![ + "p0000000000000000000000001".to_string(), + "p0000000000000000000000002".to_string(), + ]; + check( + "gateway_posted.json", + 1, + false, + Message::ApprovalPosted(ApprovalPosted { posts }), + ); + check( + "gateway_reply_approved.json", + 1, + true, + Message::ApprovalReply(reply(true, None)), + ); + check( + "gateway_reply_refused.json", + 1, + true, + Message::ApprovalReply(reply(false, Some("not on a Friday"))), + ); + for (name, outcome) in [ + ("gateway_outcome_runs.json", GatewayOutcome::Runs {}), + ( + "gateway_outcome_denied.json", + GatewayOutcome::Denied { + reason: DenyReason::NoGrant, + }, + ), + ("gateway_outcome_refused.json", GatewayOutcome::Refused {}), + ("gateway_outcome_gone.json", GatewayOutcome::Gone {}), + ] { + check( + name, + 2, + true, + Message::ApprovalOutcome(ApprovalOutcome { outcome }), + ); + } +} + +/// Every copy of `value` with one extra key in one object. +fn with_one_unknown_key(value: &Value) -> Vec { + let mut out = Vec::new(); + if let Value::Object(map) = value { + let mut extended = map.clone(); + extended.insert("zz_unknown".to_string(), Value::Bool(true)); + out.push(Value::Object(extended)); + for (key, child) in map { + for changed in with_one_unknown_key(child) { + let mut copy = map.clone(); + copy.insert(key.clone(), changed); + out.push(Value::Object(copy)); + } + } + } + out +} + +#[test] +fn every_object_refuses_an_unknown_key() { + for (name, objects) in [ + ("gateway_request.json", 4), + ("gateway_posted.json", 3), + ("gateway_reply_approved.json", 3), + ("gateway_reply_refused.json", 3), + ("gateway_outcome_runs.json", 4), + ("gateway_outcome_denied.json", 4), + ("gateway_outcome_refused.json", 4), + ("gateway_outcome_gone.json", 4), + ] { + let value: Value = serde_json::from_str(&fixture(name)).unwrap(); + let variations = with_one_unknown_key(&value); + assert_eq!(variations.len(), objects, "{name}: objects walked"); + for changed in variations { + assert!( + serde_json::from_value::(changed.clone()).is_err(), + "{name}: accepted {changed}" + ); + } + } +} + +#[test] +fn an_answer_is_checked() { + assert_eq!(reply(true, None).check(), Ok(())); + assert_eq!(reply(false, Some(&"x".repeat(MAX_REASON))).check(), Ok(())); + assert_eq!( + reply(false, Some(&"é".repeat(MAX_REASON))).check(), + Ok(()), + "characters, not bytes" + ); + assert!( + reply(false, Some(&"x".repeat(MAX_REASON + 1))) + .check() + .is_err() + ); + for bad in [ + "", + "K0000000000000000000000000", + "k000000000000000000000000", + "k00000000000000000000000000", + "../../../../../../../../etc", + ] { + let mut r = reply(true, None); + r.by = bad.to_string(); + assert!(r.check().is_err(), "by {bad:?}"); + let mut r = reply(true, None); + r.post = bad.to_string(); + assert!(r.check().is_err(), "post {bad:?}"); + assert!(!is_mattermost_id(bad), "{bad:?}"); + } + assert!(is_mattermost_id(BY) && is_mattermost_id(POST)); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 1cbe0b4..6f199fa 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/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. | ? | | M4a/16-gatewayd-secret-file-race | 2026-09-24 | done | 2 | pass | Reworded the template comment in `read_checked` to drop the literal `check_file` token, so step 5's grep prints only the credential branch's `std::fs::read(&path)` | Replaced `check_file` with `pub fn read_checked(path, between) -> Result>, String>`: check the path (absolute; `symlink_metadata` as `named`; not a symlink; a regular file), call `between()`, open the path, take `opened` metadata, refuse if `(opened.dev(), opened.ino()) != (named.dev(), named.ino())`, then the owner-uid and mode checks on `opened` (not `named`), then read the whole file into a `Zeroizing>` from that same handle. Added `use std::io::Read;`. In `load`, the File branch now calls `read_checked(path, &|| {})` and `value(std::mem::take(&mut *bytes))` so nothing unwiped remains. The three suites pass (5, 8, 5); the only remaining `std::fs::read(` is the credential branch and `check_file` is gone. `make gate` prints `gate: ok`. | ? | | M4a/15-gatewayd-main | 2026-09-24 | done | 1 | pass | none | Replaced the placeholder `src/main.rs` with the skeleton and copied `tests/main.rs`. `main` parses `args_os` like `loopd`'s; `serve` runs the five start checks in order — `Config::load` (`gatewayd: {e}\n{START_FAILED}`), `token_source` (`gatewayd: {}: {why}\n{START_FAILED}` with the path), `secrets::load` (`gatewayd: {e}`, its own `#secret-unavailable` pointer, then the file warning if any), then the state dir via `DirBuilder` recursive + `mode(0o700)` (`gatewayd: cannot prepare {}: {e}\n{START_FAILED}`), each returning exit 1 before anything is written. Then `run` with a `Log` that echoes to stderr and the returned `Stop` printed, exit 1. The todo comment matches the function and every test; the token never reaches output (it is only ever `expose`d inside `serve`, never here). `cargo test -p gatewayd` every suite passes; `make gate` prints `gate: ok` first run. | ? |