From 3f7865a8916246706c07f68bb214d6a08f42f68d Mon Sep 17 00:00:00 2001 From: temrjan Date: Thu, 9 Jul 2026 14:00:19 +0500 Subject: [PATCH 01/11] feat(protocol): approve/deny requests + resolve outcomes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit T1b protocol layer. Request::Approve/Deny (normal path — a high-risk approve's PIN is built in the transport layer to stay in Zeroizing, not through Serialize). ResolveOutcome covers every approve/deny reply: executed{tx_hash}, failed{reason}, denied, already_resolved{state}, and the error codes (unauthorized, pin_required, bad_pin, locked, pin_not_set, pin_unavailable, unknown_id). TerminalState mirrors already_resolved.state INCLUDING pending (I4 — the client must accept it or it panics on a real deny-vs-approve race). No expired error code (canon N1). 10 tests; red proven by dropping the pending arm. --- src/protocol.rs | 276 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 276 insertions(+) diff --git a/src/protocol.rs b/src/protocol.rs index 7dbe7f6..c9f29f8 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -42,6 +42,19 @@ pub enum Request<'a> { /// The item's preview-uuid, as received in a summary. id: &'a str, }, + /// Approve an item — the core signs and broadcasts. This is the normal path; + /// a **high-risk** item needs a per-request PIN, which is built separately in + /// the transport layer so the PIN stays in a `Zeroizing` buffer (never through + /// this general `Serialize` path). + Approve { + /// The item's preview-uuid. + id: &'a str, + }, + /// Deny an item — cheap, no PIN beyond the session `auth`. + Deny { + /// The item's preview-uuid. + id: &'a str, + }, } /// Serialize a request to a single JSON line (no trailing `\n`; the transport adds @@ -200,6 +213,67 @@ pub enum GetOutcome { UnknownId, } +/// The terminal state carried by an `already_resolved` reply (protocol §3.5). +/// Includes `Pending` (I4): another connection is executing this id right now. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum TerminalState { + /// Signed and broadcast. + Executed, + /// Rejected by the human. + Denied, + /// Expired before a decision. + Expired, + /// Approved, but signing/broadcast failed. + Failed, + /// Another connection is executing this id right now (retry / wait). + Pending, +} + +/// Outcome of `approve` or `deny`. Note there is **no** `expired` error code: +/// an item that expired resolves to `AlreadyResolved { Expired }` (or `UnknownId` +/// after retention), never a top-level `expired`. +#[derive(Debug, PartialEq, Eq)] +pub enum ResolveOutcome { + /// Approved: signed and broadcast; carries the tx hash. + Executed { + /// `0x`-hex transaction hash. + tx_hash: String, + }, + /// Approved, but signing/broadcast failed (still resolved — not retryable). + Failed { + /// Operator-masked failure reason. + reason: String, + }, + /// Denied. + Denied, + /// Already terminal (or in-flight) — carries the state. + AlreadyResolved { + /// The existing terminal/in-flight state. + state: TerminalState, + }, + /// No successful `auth` on this connection. + Unauthorized, + /// A high-risk item was approved without a `pin`. + PinRequired, + /// Wrong PIN; `attempts_left == 0` means the lockout is now armed. + BadPin { + /// Attempts before the lockout trips. + attempts_left: u32, + }, + /// Lockout active; retry after this many seconds. + Locked { + /// Seconds until the channel accepts a PIN again. + retry_after_s: u64, + }, + /// The wallet has no PIN record. + PinNotSet, + /// Transient Argon2 backend failure. + PinUnavailable, + /// The id is not a live item. + UnknownId, +} + /// A parse or encode failure in the protocol layer. #[derive(Debug, PartialEq, Eq)] pub enum ProtocolError { @@ -342,6 +416,99 @@ pub fn parse_get(line: &str) -> Result { } } +/// The fields an `approve` / `deny` reply may carry. `state` means the outcome +/// (`executed`/`failed`/`denied`) on an `ok` reply, or the `already_resolved` +/// state on an error reply. +#[derive(Deserialize)] +struct ResolveRaw { + ok: bool, + state: Option, + tx_hash: Option, + reason: Option, + error: Option, + attempts_left: Option, + retry_after_s: Option, +} + +/// Map an error reply (shared by `approve` and `deny`) to a [`ResolveOutcome`]. +fn resolve_error(raw: &ResolveRaw) -> Result { + match raw.error.as_deref() { + Some("unauthorized") => Ok(ResolveOutcome::Unauthorized), + Some("pin_required") => Ok(ResolveOutcome::PinRequired), + Some("bad_pin") => Ok(ResolveOutcome::BadPin { + attempts_left: raw.attempts_left.unwrap_or(0), + }), + Some("locked") => Ok(ResolveOutcome::Locked { + retry_after_s: raw.retry_after_s.unwrap_or(0), + }), + Some("pin_not_set") => Ok(ResolveOutcome::PinNotSet), + Some("pin_unavailable") => Ok(ResolveOutcome::PinUnavailable), + Some("unknown_id") => Ok(ResolveOutcome::UnknownId), + Some("already_resolved") => Ok(ResolveOutcome::AlreadyResolved { + state: parse_terminal_state(raw.state.as_deref())?, + }), + other => Err(ProtocolError::Unexpected( + other.unwrap_or("resolve without ok or error").to_owned(), + )), + } +} + +fn parse_terminal_state(s: Option<&str>) -> Result { + match s { + Some("executed") => Ok(TerminalState::Executed), + Some("denied") => Ok(TerminalState::Denied), + Some("expired") => Ok(TerminalState::Expired), + Some("pending") => Ok(TerminalState::Pending), + Some("failed") => Ok(TerminalState::Failed), + other => Err(ProtocolError::Unexpected(format!( + "already_resolved with unknown state {other:?}" + ))), + } +} + +/// Parse a response to `approve`. +/// +/// # Errors +/// [`ProtocolError::Malformed`] on non-JSON; [`ProtocolError::Unexpected`] on an +/// `ok` reply with an unexpected `state`, or an unmodeled error code. +pub fn parse_approve(line: &str) -> Result { + let raw: ResolveRaw = parse_line(line)?; + if raw.ok { + match raw.state.as_deref() { + Some("executed") => Ok(ResolveOutcome::Executed { + tx_hash: raw.tx_hash.unwrap_or_default(), + }), + Some("failed") => Ok(ResolveOutcome::Failed { + reason: raw.reason.unwrap_or_default(), + }), + other => Err(ProtocolError::Unexpected(format!( + "approve ok with unexpected state {other:?}" + ))), + } + } else { + resolve_error(&raw) + } +} + +/// Parse a response to `deny`. +/// +/// # Errors +/// [`ProtocolError::Malformed`] on non-JSON; [`ProtocolError::Unexpected`] on an +/// `ok` reply that is not `denied`, or an unmodeled error code. +pub fn parse_deny(line: &str) -> Result { + let raw: ResolveRaw = parse_line(line)?; + if raw.ok { + match raw.state.as_deref() { + Some("denied") => Ok(ResolveOutcome::Denied), + other => Err(ProtocolError::Unexpected(format!( + "deny ok with unexpected state {other:?}" + ))), + } + } else { + resolve_error(&raw) + } +} + #[cfg(test)] mod tests { use super::*; @@ -552,4 +719,113 @@ mod tests { Err(ProtocolError::Malformed(_)) )); } + + // ── approve / deny requests ── + + #[test] + fn encode_approve_and_deny_carry_the_id() { + assert_eq!( + encode_request(&Request::Approve { id: "a1" }).unwrap(), + r#"{"op":"approve","id":"a1"}"# + ); + assert_eq!( + encode_request(&Request::Deny { id: "a1" }).unwrap(), + r#"{"op":"deny","id":"a1"}"# + ); + } + + // ── approve outcomes ── + + #[test] + fn parse_approve_executed_carries_the_tx_hash() { + assert_eq!( + parse_approve(r#"{"ok":true,"state":"executed","tx_hash":"0xabc"}"#).unwrap(), + ResolveOutcome::Executed { + tx_hash: "0xabc".to_owned() + } + ); + } + + #[test] + fn parse_approve_failed_carries_the_reason() { + assert_eq!( + parse_approve(r#"{"ok":true,"state":"failed","reason":"broadcast error"}"#).unwrap(), + ResolveOutcome::Failed { + reason: "broadcast error".to_owned() + } + ); + } + + #[test] + fn parse_approve_error_codes() { + assert_eq!( + parse_approve(r#"{"ok":false,"error":"pin_required"}"#).unwrap(), + ResolveOutcome::PinRequired + ); + assert_eq!( + parse_approve(r#"{"ok":false,"error":"bad_pin","attempts_left":1}"#).unwrap(), + ResolveOutcome::BadPin { attempts_left: 1 } + ); + assert_eq!( + parse_approve(r#"{"ok":false,"error":"locked","retry_after_s":300}"#).unwrap(), + ResolveOutcome::Locked { retry_after_s: 300 } + ); + assert_eq!( + parse_approve(r#"{"ok":false,"error":"unauthorized"}"#).unwrap(), + ResolveOutcome::Unauthorized + ); + assert_eq!( + parse_approve(r#"{"ok":false,"error":"unknown_id"}"#).unwrap(), + ResolveOutcome::UnknownId + ); + } + + #[test] + fn parse_approve_already_resolved_accepts_every_state_including_pending() { + for (word, state) in [ + ("executed", TerminalState::Executed), + ("denied", TerminalState::Denied), + ("expired", TerminalState::Expired), + ("failed", TerminalState::Failed), + ("pending", TerminalState::Pending), // I4 — must not panic + ] { + let line = format!(r#"{{"ok":false,"error":"already_resolved","state":"{word}"}}"#); + assert_eq!( + parse_approve(&line).unwrap(), + ResolveOutcome::AlreadyResolved { state } + ); + } + } + + #[test] + fn parse_approve_rejects_an_unknown_already_resolved_state() { + assert!(matches!( + parse_approve(r#"{"ok":false,"error":"already_resolved","state":"weird"}"#), + Err(ProtocolError::Unexpected(_)) + )); + } + + // ── deny outcomes ── + + #[test] + fn parse_deny_denied() { + assert_eq!( + parse_deny(r#"{"ok":true,"state":"denied"}"#).unwrap(), + ResolveOutcome::Denied + ); + } + + #[test] + fn parse_deny_shares_the_error_codes() { + assert_eq!( + parse_deny(r#"{"ok":false,"error":"unauthorized"}"#).unwrap(), + ResolveOutcome::Unauthorized + ); + assert_eq!( + parse_deny(r#"{"ok":false,"error":"already_resolved","state":"executed"}"#).unwrap(), + ResolveOutcome::AlreadyResolved { + state: TerminalState::Executed + } + ); + } } From 3462fe5edda5b5050e11af6fd1656419320457e5 Mon Sep 17 00:00:00 2001 From: temrjan Date: Thu, 9 Jul 2026 14:03:24 +0500 Subject: [PATCH 02/11] feat(transport): approve/deny requests + resolve replies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Transport carries the T1b requests: Approve(id) (normal), ApprovePin(Zeroizing) (a high-risk approve's pre-serialized line with the PIN inside — never a plain String), and Deny(id). serve_one routes them and parses via parse_approve / parse_deny into Reply::Resolve(ResolveOutcome). MVU's on_reply gets a no-op Resolve arm for now (the confirmation flow that sends these requests lands in the next commit) — keeps every commit compiling. 3 fake-socket tests (executed, high-risk bad_pin, denied). --- src/app.rs | 3 ++ src/transport.rs | 80 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/app.rs b/src/app.rs index 2d611ef..e9432a4 100644 --- a/src/app.rs +++ b/src/app.rs @@ -306,6 +306,9 @@ impl Model { Reply::Auth(outcome) => self.apply_auth(outcome), Reply::List(items) => self.apply_list(items), Reply::Get(outcome) => self.apply_get(outcome), + // approve/deny outcomes are wired into the confirmation flow in the + // next T1b commit; the MVU does not send those requests yet. + Reply::Resolve(_) => {} Reply::Fatal(err) => { self.phase = Phase::Fatal(err); self.pending = None; diff --git a/src/transport.rs b/src/transport.rs index 2940aed..8b94e5a 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -24,8 +24,8 @@ use std::thread::JoinHandle; use zeroize::Zeroizing; use crate::protocol::{ - self, AuthOutcome, GetOutcome, HelloOutcome, PROTO_VERSION, Summary, encode_request, - parse_auth, parse_get, parse_hello, parse_list, + self, AuthOutcome, GetOutcome, HelloOutcome, PROTO_VERSION, ResolveOutcome, Summary, + encode_request, parse_approve, parse_auth, parse_deny, parse_get, parse_hello, parse_list, }; /// Informational client id sent in `hello` (the server does not validate it). @@ -44,6 +44,13 @@ pub enum Request { List, /// Ask for one item's card by id. Get(String), + /// Approve a normal (non-high-risk) item by id — no PIN. + Approve(String), + /// Approve a high-risk item: a pre-serialized `approve` line with the PIN + /// inside a [`Zeroizing`] buffer (built in the MVU layer, zeroized after send). + ApprovePin(Zeroizing), + /// Deny an item by id. + Deny(String), } /// A message from the worker to the MVU layer. @@ -60,6 +67,8 @@ pub enum Reply { List(Vec), /// Result of a `get`. Get(GetOutcome), + /// Result of an `approve` or `deny`. + Resolve(ResolveOutcome), /// The connection is finished and unusable — the worker has exited. Fatal(TransportError), } @@ -255,11 +264,24 @@ fn serve_one( .map_err(|e| TransportError::Protocol(e.to_string()))?; exchange(writer, reader, &line)? } + Request::Approve(id) => { + let line = encode_request(&protocol::Request::Approve { id }) + .map_err(|e| TransportError::Protocol(e.to_string()))?; + exchange(writer, reader, &line)? + } + Request::ApprovePin(line) => exchange(writer, reader, line)?, + Request::Deny(id) => { + let line = encode_request(&protocol::Request::Deny { id }) + .map_err(|e| TransportError::Protocol(e.to_string()))?; + exchange(writer, reader, &line)? + } }; let parsed = match req { Request::Auth(_) => parse_auth(&resp).map(Reply::Auth), Request::List => parse_list(&resp).map(Reply::List), Request::Get(_) => parse_get(&resp).map(Reply::Get), + Request::Approve(_) | Request::ApprovePin(_) => parse_approve(&resp).map(Reply::Resolve), + Request::Deny(_) => parse_deny(&resp).map(Reply::Resolve), }; parsed.map_err(|e| TransportError::Protocol(e.to_string())) } @@ -427,6 +449,60 @@ mod tests { ); } + const HELLO_OK: &str = r#"{"ok":true,"proto":1,"server":"core-server/0.1.0"}"#; + + #[test] + fn approve_normal_returns_the_resolve_outcome() { + let server = FakeServer::start( + "approve", + vec![ + Some(HELLO_OK), + Some(r#"{"ok":true,"state":"executed","tx_hash":"0xdead"}"#), + ], + ); + let t = Transport::connect(&server.path); + assert!(matches!(t.recv(), Some(Reply::Hello { .. }))); + assert!(t.send(Request::Approve("a1".to_owned()))); + assert_eq!( + t.recv(), + Some(Reply::Resolve(ResolveOutcome::Executed { + tx_hash: "0xdead".to_owned() + })) + ); + } + + #[test] + fn approve_pin_line_is_sent_for_a_high_risk_item() { + let server = FakeServer::start( + "approvepin", + vec![ + Some(HELLO_OK), + Some(r#"{"ok":false,"error":"bad_pin","attempts_left":1}"#), + ], + ); + let t = Transport::connect(&server.path); + assert!(matches!(t.recv(), Some(Reply::Hello { .. }))); + // The MVU layer builds this Zeroizing line (PIN inside). + let line = Zeroizing::new(r#"{"op":"approve","id":"a1","pin":"000000"}"#.to_owned()); + assert!(t.send(Request::ApprovePin(line))); + assert_eq!( + t.recv(), + Some(Reply::Resolve(ResolveOutcome::BadPin { attempts_left: 1 })) + ); + } + + #[test] + fn deny_returns_denied() { + let server = FakeServer::start( + "deny", + vec![Some(HELLO_OK), Some(r#"{"ok":true,"state":"denied"}"#)], + ); + let t = Transport::connect(&server.path); + assert!(matches!(t.recv(), Some(Reply::Hello { .. }))); + assert!(t.send(Request::Deny("a1".to_owned()))); + assert_eq!(t.recv(), Some(Reply::Resolve(ResolveOutcome::Denied))); + } + #[test] fn unsupported_proto_at_handshake_is_fatal() { let server = FakeServer::start( From 6db125effbc53a4cfd198d8c279d8959f8fe62e4 Mon Sep 17 00:00:00 2001 From: temrjan Date: Thu, 9 Jul 2026 14:54:40 +0500 Subject: [PATCH 03/11] =?UTF-8?q?feat(app):=20confirmation=20flow=20?= =?UTF-8?q?=E2=80=94=20approve/deny,=20default-deny,=20high-risk=20PIN?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An open card *is* the confirmation (AGENTS.md #5): it is left by deciding, never by closing. `y` approves, `n`/Esc/Ctrl-C deny, and the expiry deadline denies fail-closed rather than leaving an item pending for the next console. - Pin::approve_line(id): the id is escaped by serde so a card id cannot inject JSON, while the PIN is appended by hand — routing the whole request through serde_json would leave an un-zeroized copy of it behind. Reserved exactly, so no realloc frees the PIN without zeroizing it. - Phase::Watching.card -> .confirm (card + PIN prompt + error + resolving), and a terminal Phase::Resolved { outcome, exit }. - ExitOutcome reports what happened to the money, not which key was pressed: an item another connection executed while we denied it still exits Approved; only our own deadline-sent deny exits Expired. - A decision on the wire freezes the screen: a second key press is neither sent nor parked, so a stray `y` after `n` can never sign what the human refused. - Non-terminal answers keep the human in charge: pin_required opens the prompt, bad_pin clears it, already_resolved:pending is a retry, unknown_id closes the card. An unauthorized answer to an authed session fails closed. - PendingIntent no longer derives Debug — Zeroizing forwards Debug to its String, so a parked approve line would have printed the PIN through Model's Debug. ui.rs and main.rs get the minimum to stay honest: the confirmation renders its PIN dots and errors, keys map per phase, and a resolved item exits with its code. The countdown on the Reject button, the TTY gate, the timeout tick and the machine decision on stdout land in the next two commits. Tests: 95 (+32). Red proven by 10 mutations, incl. dropping either double-decision guard, un-escaping the id, and deriving Debug on the parked PIN. --- src/app.rs | 926 ++++++++++++++++++++++++++++++++++++++++++++++++++-- src/main.rs | 216 ++++++++++-- src/ui.rs | 79 ++++- 3 files changed, 1156 insertions(+), 65 deletions(-) diff --git a/src/app.rs b/src/app.rs index e9432a4..a48d764 100644 --- a/src/app.rs +++ b/src/app.rs @@ -7,10 +7,14 @@ //! **One request in flight** (protocol §1): while a request is outstanding the //! periodic `list` poll is suppressed (we do not pile up stale polls), and a user //! action taken meanwhile is parked in a single latest-wins slot so it is not lost. +//! +//! **Default-deny** (`AGENTS.md` #5): an open card *is* the confirmation. Only an +//! explicit `y` approves; `n`, Esc, Ctrl-C and the expiry deadline all send `deny`. +//! Leaving the confirmation without deciding is not offered. use zeroize::Zeroizing; -use crate::protocol::{AuthOutcome, Card, GetOutcome, Summary}; +use crate::protocol::{AuthOutcome, Card, GetOutcome, ResolveOutcome, Summary, TerminalState}; use crate::transport::{self, Reply, TransportError}; /// The approval PIN as it is typed. Zeroized on drop (via [`Zeroizing`]) and @@ -76,6 +80,36 @@ impl Pin { line.push_str(SUFFIX); line } + + /// Build the high-risk `approve` request line into a `Zeroizing` buffer. + /// + /// The `id` goes through serde (so it is quoted and escaped by the same code + /// that would encode it on the normal path — a card id can never break out of + /// its JSON string), while the PIN is appended by hand: routing the whole + /// request through `serde_json::to_string` would leave an **un-zeroized** + /// `String` copy of the PIN behind. Digits-only (see [`Self::push`]). + /// + /// Returns `None` if the id could not be serialized — a string always can, so + /// this is a fail-closed seam rather than a panic in a money path. + #[must_use] + pub fn approve_line(&self, id: &str) -> Option> { + const PREFIX: &str = r#"{"op":"approve","id":"#; + const MID: &str = r#","pin":""#; + const SUFFIX: &str = r#""}"#; + // Quoted and escaped by serde; carries no secret, so a plain String is fine. + let id_json = serde_json::to_string(id).ok()?; + // Reserve exactly — a realloc would free the old buffer (with the PIN in it) + // WITHOUT zeroizing it. + let mut line = Zeroizing::new(String::with_capacity( + PREFIX.len() + id_json.len() + MID.len() + self.0.len() + SUFFIX.len(), + )); + line.push_str(PREFIX); + line.push_str(&id_json); + line.push_str(MID); + line.push_str(&self.0); + line.push_str(SUFFIX); + Some(line) + } } /// Where the session is. @@ -96,15 +130,109 @@ pub enum Phase { items: Vec, /// Selected row (clamped into `items`). selected: usize, - /// The opened card, if one is being viewed. - card: Option>, + /// The opened card and its confirmation state, if one is being decided. + confirm: Option>, /// A transient note (e.g. the selected item vanished). note: Option, }, + /// The item reached a terminal state — render it, then exit with [`ExitOutcome`]. + Resolved { + /// The server's terminal answer, rendered as received. + outcome: ResolveOutcome, + /// What the process exits with. + exit: ExitOutcome, + }, /// The connection is finished — render the reason and exit. Fatal(TransportError), } +/// An open card **is** the confirmation dialog (`AGENTS.md` #5): it is left by +/// approving or denying, never by simply closing. +#[derive(Debug)] +pub struct Confirm { + card: Card, + /// `Some` while the high-risk PIN prompt is up. + pin: Option, + /// The last non-terminal failure to show. + error: Option, + /// A decision is on the wire — further key presses are ignored so a second + /// press cannot become a second decision. + resolving: bool, + /// The `deny` was sent by the expiry deadline, not by the human. + timed_out: bool, +} + +impl Confirm { + fn new(card: Card) -> Self { + Self { + card, + pin: None, + error: None, + resolving: false, + timed_out: false, + } + } + + /// The card being decided (rendered verbatim). + #[must_use] + pub fn card(&self) -> &Card { + &self.card + } + + /// Number of PIN digits entered, or `None` when the PIN prompt is not up. + #[must_use] + pub fn pin_len(&self) -> Option { + self.pin.as_ref().map(Pin::len) + } + + /// The last non-terminal failure, if any. + #[must_use] + pub fn error(&self) -> Option<&ResolveError> { + self.error.as_ref() + } + + /// Whether a decision is currently on the wire. + #[must_use] + pub fn is_resolving(&self) -> bool { + self.resolving + } +} + +/// What the console exits with once an item is terminal. It reports **what happened +/// to the money**, not which key the human pressed: an item another connection +/// executed while we were denying it is still [`Self::Approved`]. A `deny` sent by +/// the expiry deadline reports [`Self::Expired`], not [`Self::Rejected`] — the +/// deadline, not a human, said no. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExitOutcome { + /// Signed and broadcast. + Approved, + /// A human said no. + Rejected, + /// The deadline passed before a decision. + Expired, + /// Approved, but signing/broadcast failed — no money moved. + Failed, +} + +/// A **non-terminal** failure of `approve`/`deny`: the item is still live and the +/// human can act again. Terminal answers become [`Phase::Resolved`] instead. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ResolveError { + /// The item is high-risk; a per-request PIN is needed (the entry is untouched). + PinRequired, + /// Wrong PIN; attempts before lockout. + BadPin(u32), + /// Locked out; seconds to wait. + Locked(u64), + /// The wallet has no PIN set. + NotSet, + /// Transient verifier failure. + Unavailable, + /// Another connection is executing this id right now (`already_resolved:pending`). + Busy, +} + /// A human-facing auth failure shown on the PIN screen. #[derive(Debug, Clone, PartialEq, Eq)] pub enum AuthError { @@ -135,10 +263,14 @@ pub enum Msg { MoveUp, /// Move the queue selection down. MoveDown, - /// Open the selected item's card. + /// Open the selected item's card — which opens the confirmation. Open, - /// Close the open card. - Back, + /// Approve the open card (`y`). A high-risk card asks for the PIN first. + Approve, + /// Reject the open card — `n`, Esc or Ctrl-C (`AGENTS.md` #5). + Reject, + /// The open card's deadline passed: deny it fail-closed, and report `expired`. + Expire, /// Quit. Quit, } @@ -153,10 +285,29 @@ pub struct Model { } /// A parked user intent (no `List` — the poll is suppressed, not queued). -#[derive(Debug)] +/// +/// **Not `derive(Debug)`**: `ApprovePin` holds the serialized PIN, and `Zeroizing` +/// forwards `Debug` to the inner `String`. A derived `Debug` would print the PIN +/// through the `Model`'s own `Debug`. enum PendingIntent { Get(String), Auth, + Approve(String), + ApprovePin(Zeroizing), + Deny(String), +} + +impl std::fmt::Debug for PendingIntent { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Get(id) => write!(f, "Get({id})"), + Self::Auth => f.write_str("Auth"), + Self::Approve(id) => write!(f, "Approve({id})"), + // The line carries the PIN — never its contents. + Self::ApprovePin(_) => f.write_str("ApprovePin()"), + Self::Deny(id) => write!(f, "Deny({id})"), + } + } } impl Default for Model { @@ -199,18 +350,21 @@ impl Model { Msg::Tick => self.on_tick(), Msg::Reply(reply) => self.on_reply(reply), Msg::PinDigit(c) => { - if let Phase::Authing { pin, .. } = &mut self.phase { + if let Some(pin) = self.pin_mut() { pin.push(c); } None } Msg::PinBackspace => { - if let Phase::Authing { pin, .. } = &mut self.phase { + if let Some(pin) = self.pin_mut() { pin.pop(); } None } Msg::PinSubmit => self.on_pin_submit(), + Msg::Approve => self.on_approve(), + Msg::Reject => self.on_reject(false), + Msg::Expire => self.on_reject(true), Msg::MoveUp => { if let Phase::Watching { selected, .. } = &mut self.phase { *selected = selected.saturating_sub(1); @@ -227,12 +381,19 @@ impl Model { None } Msg::Open => self.on_open(), - Msg::Back => { - if let Phase::Watching { card, .. } = &mut self.phase { - *card = None; - } - None - } + } + } + + /// The PIN buffer the keyboard is currently feeding: the unlock screen, or the + /// high-risk prompt on an open confirmation. `None` while a decision is on the + /// wire — a late keystroke must not edit a PIN that has already been sent. + fn pin_mut(&mut self) -> Option<&mut Pin> { + match &mut self.phase { + Phase::Authing { pin, .. } => Some(pin), + Phase::Watching { + confirm: Some(c), .. + } if !c.resolving => c.pin.as_mut(), + _ => None, } } @@ -248,6 +409,15 @@ impl Model { } fn on_pin_submit(&mut self) -> Option { + if matches!( + self.phase, + Phase::Watching { + confirm: Some(_), + .. + } + ) { + return self.on_confirm_pin_submit(); + } let Phase::Authing { pin, .. } = &self.phase else { return None; }; @@ -258,18 +428,93 @@ impl Model { self.dispatch_user(PendingIntent::Auth, || transport::Request::Auth(line)) } + /// Submit the per-request PIN of a high-risk approval. + fn on_confirm_pin_submit(&mut self) -> Option { + let Phase::Watching { + confirm: Some(c), .. + } = &mut self.phase + else { + return None; + }; + if c.resolving { + return None; + } + let Some(pin) = &c.pin else { + return None; // the prompt is not up: `y` opens it + }; + if pin.is_empty() { + return None; + } + let Some(line) = pin.approve_line(&c.card.id) else { + // A card id that will not serialize cannot be approved safely. + self.phase = Phase::Fatal(TransportError::Protocol( + "card id could not be encoded".to_owned(), + )); + return None; + }; + c.resolving = true; + c.error = None; + self.dispatch_pin_approve(line) + } + + /// `y` on the confirmation: a high-risk card asks for the PIN first, a normal + /// one goes straight to `approve`. + fn on_approve(&mut self) -> Option { + let Phase::Watching { + confirm: Some(c), .. + } = &mut self.phase + else { + return None; + }; + if c.resolving || c.pin.is_some() { + return None; // decided already, or the PIN prompt owns Enter + } + if c.card.high_risk { + c.pin = Some(Pin::default()); + c.error = None; + return None; + } + c.resolving = true; + c.error = None; + let id = c.card.id.clone(); + self.dispatch_user(PendingIntent::Approve(id.clone()), || { + transport::Request::Approve(id) + }) + } + + /// Default-deny (`AGENTS.md` #5). `by_timeout` records that the deadline said + /// no, not the human — it changes only what we exit with, never the `deny`. + fn on_reject(&mut self, by_timeout: bool) -> Option { + let Phase::Watching { + confirm: Some(c), .. + } = &mut self.phase + else { + return None; + }; + if c.resolving { + return None; // a decision is already on the wire; the server decides + } + c.resolving = true; + c.timed_out = by_timeout; + c.error = None; + let id = c.card.id.clone(); + self.dispatch_user(PendingIntent::Deny(id.clone()), || { + transport::Request::Deny(id) + }) + } + fn on_open(&mut self) -> Option { let Phase::Watching { items, selected, - card, + confirm, .. } = &self.phase else { return None; }; - if card.is_some() { - return None; // already viewing one + if confirm.is_some() { + return None; // already deciding one } let Some(id) = items.get(*selected).map(|s| s.id.clone()) else { return None; // empty queue @@ -294,6 +539,18 @@ impl Model { } } + /// Same, for the high-risk approve line. It takes the `Zeroizing` buffer by + /// value so the PIN is never cloned into a second allocation. + fn dispatch_pin_approve(&mut self, line: Zeroizing) -> Option { + if self.in_flight { + self.pending = Some(PendingIntent::ApprovePin(line)); + None + } else { + self.in_flight = true; + Some(transport::Request::ApprovePin(line)) + } + } + fn on_reply(&mut self, reply: Reply) -> Option { self.in_flight = false; match reply { @@ -306,15 +563,18 @@ impl Model { Reply::Auth(outcome) => self.apply_auth(outcome), Reply::List(items) => self.apply_list(items), Reply::Get(outcome) => self.apply_get(outcome), - // approve/deny outcomes are wired into the confirmation flow in the - // next T1b commit; the MVU does not send those requests yet. - Reply::Resolve(_) => {} + Reply::Resolve(outcome) => self.apply_resolve(outcome), Reply::Fatal(err) => { self.phase = Phase::Fatal(err); self.pending = None; return None; } } + // A terminal answer ends the session: nothing parked may still be sent. + if matches!(self.phase, Phase::Resolved { .. } | Phase::Fatal(_)) { + self.pending = None; + return None; + } self.flush_pending() } @@ -324,7 +584,7 @@ impl Model { self.phase = Phase::Watching { items: Vec::new(), selected: 0, - card: None, + confirm: None, note: None, }; } @@ -360,22 +620,94 @@ impl Model { } fn apply_get(&mut self, outcome: GetOutcome) { - if let Phase::Watching { card, note, .. } = &mut self.phase { + if let Phase::Watching { confirm, note, .. } = &mut self.phase { match outcome { GetOutcome::Card(c) => { - *card = Some(c); + // An open card *is* the confirmation (AGENTS.md #5). + *confirm = Some(Box::new(Confirm::new(*c))); *note = None; } GetOutcome::UnknownId => { // The selected item vanished between list and get — drop any // stale card and show a transient note instead of a dead card. - *card = None; + *confirm = None; *note = Some("that request is no longer available".to_owned()); } } } } + /// Fold an `approve`/`deny` answer. Terminal answers end the session with the + /// matching [`ExitOutcome`]; the rest leave the item live and the human in + /// charge (`pin_required` opens the PIN prompt, `bad_pin` clears it, and so on). + fn apply_resolve(&mut self, outcome: ResolveOutcome) { + let timed_out = match &mut self.phase { + Phase::Watching { + confirm: Some(c), .. + } => { + c.resolving = false; + c.timed_out + } + // No confirmation is open — an answer to a decision we never made. + _ => return, + }; + + if let Some(exit) = terminal_exit(&outcome, timed_out) { + self.phase = Phase::Resolved { outcome, exit }; + return; + } + + if matches!(outcome, ResolveOutcome::Unauthorized) { + // We only send approve/deny after a successful auth on this very + // connection; the server disagreeing means the channel is not what we + // think it is. Fail closed rather than retry a money action. + self.phase = Phase::Fatal(TransportError::Protocol( + "server refused an authenticated session (unauthorized)".to_owned(), + )); + return; + } + + let Phase::Watching { confirm, note, .. } = &mut self.phase else { + return; + }; + if matches!(outcome, ResolveOutcome::UnknownId) { + *confirm = None; + *note = Some("that request is no longer available".to_owned()); + return; + } + let Some(c) = confirm else { + return; + }; + let error = match outcome { + // The entry is untouched and still approvable — with the PIN this time. + ResolveOutcome::PinRequired => { + c.pin.get_or_insert_with(Pin::default); + ResolveError::PinRequired + } + ResolveOutcome::BadPin { attempts_left } => { + c.pin.get_or_insert_with(Pin::default).clear(); + ResolveError::BadPin(attempts_left) + } + ResolveOutcome::Locked { retry_after_s } => { + if let Some(pin) = &mut c.pin { + pin.clear(); + } + ResolveError::Locked(retry_after_s) + } + ResolveOutcome::PinNotSet => ResolveError::NotSet, + ResolveOutcome::PinUnavailable => ResolveError::Unavailable, + // Only `already_resolved:pending` is non-terminal; the rest were taken + // by `terminal_exit` above. + ResolveOutcome::AlreadyResolved { .. } => ResolveError::Busy, + ResolveOutcome::Executed { .. } + | ResolveOutcome::Failed { .. } + | ResolveOutcome::Denied + | ResolveOutcome::Unauthorized + | ResolveOutcome::UnknownId => return, + }; + c.error = Some(error); + } + /// After a reply lands, send a parked user intent if one is waiting. fn flush_pending(&mut self) -> Option { match self.pending.take() { @@ -383,6 +715,18 @@ impl Model { self.in_flight = true; Some(transport::Request::Get(id)) } + Some(PendingIntent::Approve(id)) => { + self.in_flight = true; + Some(transport::Request::Approve(id)) + } + Some(PendingIntent::ApprovePin(line)) => { + self.in_flight = true; + Some(transport::Request::ApprovePin(line)) + } + Some(PendingIntent::Deny(id)) => { + self.in_flight = true; + Some(transport::Request::Deny(id)) + } Some(PendingIntent::Auth) => { // Re-derive the auth line from the (now cleared-on-failure) pin only // if we are still on the auth screen with digits; otherwise drop it. @@ -401,6 +745,45 @@ impl Model { } } +/// Is this answer the item's last word, and if so what do we exit with? +/// +/// The exit reports **what happened to the money**, not which key was pressed: an +/// `already_resolved:executed` answer to our `deny` means another connection got +/// there first and the transaction went out — that is [`ExitOutcome::Approved`]. +/// `timed_out` is the one place the cause matters: a `deny` the deadline sent +/// reports `expired`, so a caller can tell "the human said no" from "nobody did". +/// +/// `already_resolved:pending` is **not** terminal — another connection is executing +/// this id right now (protocol §3.5); the human may retry. +fn terminal_exit(outcome: &ResolveOutcome, timed_out: bool) -> Option { + // Only *our own* deny can have been sent by the deadline. An `already_resolved` + // deny was somebody else's decision, so it stays a rejection. + let our_deny = if timed_out { + ExitOutcome::Expired + } else { + ExitOutcome::Rejected + }; + match outcome { + ResolveOutcome::Executed { .. } => Some(ExitOutcome::Approved), + ResolveOutcome::Failed { .. } => Some(ExitOutcome::Failed), + ResolveOutcome::Denied => Some(our_deny), + ResolveOutcome::AlreadyResolved { state } => match state { + TerminalState::Executed => Some(ExitOutcome::Approved), + TerminalState::Failed => Some(ExitOutcome::Failed), + TerminalState::Denied => Some(ExitOutcome::Rejected), + TerminalState::Expired => Some(ExitOutcome::Expired), + TerminalState::Pending => None, + }, + ResolveOutcome::Unauthorized + | ResolveOutcome::PinRequired + | ResolveOutcome::BadPin { .. } + | ResolveOutcome::Locked { .. } + | ResolveOutcome::PinNotSet + | ResolveOutcome::PinUnavailable + | ResolveOutcome::UnknownId => None, + } +} + #[cfg(test)] mod tests { use super::*; @@ -420,19 +803,52 @@ mod tests { } fn card(id: &str) -> Box { + card_risk(id, false) + } + + fn card_risk(id: &str, high_risk: bool) -> Box { Box::new(Card { id: id.to_owned(), chain_id: 1, to: "0xabc".to_owned(), amount_wei: "0".to_owned(), decoded_call: None, - high_risk: false, + high_risk, high_risk_reasons: vec![], raw_data: "0x".to_owned(), not_after_unix: 1, }) } + /// Drive a model to an open confirmation on `id`. + fn confirming(id: &str, high_risk: bool) -> Model { + let mut m = watching(vec![summary(id)]); + assert!(matches!( + m.update(Msg::Open), + Some(transport::Request::Get(_)) + )); + m.update(Msg::Reply(Reply::Get(GetOutcome::Card(card_risk( + id, high_risk, + ))))); + m + } + + fn confirm_of(m: &Model) -> &Confirm { + let Phase::Watching { + confirm: Some(c), .. + } = m.phase() + else { + panic!("a confirmation must be open"); + }; + c + } + + fn type_pin(m: &mut Model, digits: &str) { + for c in digits.chars() { + m.update(Msg::PinDigit(c)); + } + } + /// Drive a model to the watch phase with the given items. fn watching(items: Vec) -> Model { let mut m = Model::new(); @@ -589,16 +1005,17 @@ mod tests { } #[test] - fn opening_shows_the_card_and_back_closes_it() { + fn opening_a_card_opens_the_confirmation() { let mut m = watching(vec![summary("a")]); assert!(matches!( m.update(Msg::Open), Some(transport::Request::Get(_)) )); m.update(Msg::Reply(Reply::Get(GetOutcome::Card(card("a"))))); - assert!(matches!(m.phase(), Phase::Watching { card: Some(_), .. })); - m.update(Msg::Back); - assert!(matches!(m.phase(), Phase::Watching { card: None, .. })); + let c = confirm_of(&m); + assert_eq!(c.card().id, "a"); + assert!(c.pin_len().is_none(), "a normal card asks for no PIN"); + assert!(!c.is_resolving()); } #[test] @@ -610,10 +1027,10 @@ mod tests { )); // the item vanished between list and get m.update(Msg::Reply(Reply::Get(GetOutcome::UnknownId))); - let Phase::Watching { card, note, .. } = m.phase() else { + let Phase::Watching { confirm, note, .. } = m.phase() else { panic!("watching"); }; - assert!(card.is_none()); + assert!(confirm.is_none()); assert!(note.is_some()); } @@ -657,6 +1074,451 @@ mod tests { assert!(pin.is_empty()); } + // ── confirmation: approve ── + + #[test] + fn approving_a_normal_card_sends_approve_without_a_pin() { + let mut m = confirming("a", false); + let req = m.update(Msg::Approve); + assert!(matches!(req, Some(transport::Request::Approve(id)) if id == "a")); + assert!(confirm_of(&m).is_resolving()); + } + + #[test] + fn approving_a_high_risk_card_asks_for_the_pin_before_sending_anything() { + let mut m = confirming("a", true); + assert!( + m.update(Msg::Approve).is_none(), + "a high-risk approve must not reach the wire without a PIN" + ); + let c = confirm_of(&m); + assert_eq!(c.pin_len(), Some(0), "the PIN prompt is up"); + assert!(!c.is_resolving()); + } + + #[test] + fn high_risk_pin_submit_sends_the_approve_line_with_the_pin() { + let mut m = confirming("a", true); + m.update(Msg::Approve); + type_pin(&mut m, "4839"); + let req = m.update(Msg::PinSubmit); + let Some(transport::Request::ApprovePin(line)) = req else { + panic!("expected a pin-carrying approve"); + }; + assert_eq!(&*line, r#"{"op":"approve","id":"a","pin":"4839"}"#); + assert!(confirm_of(&m).is_resolving()); + } + + #[test] + fn an_empty_pin_is_never_submitted() { + let mut m = confirming("a", true); + m.update(Msg::Approve); + assert!(m.update(Msg::PinSubmit).is_none()); + assert!(!confirm_of(&m).is_resolving()); + } + + #[test] + fn approve_line_reserves_exactly_so_the_buffer_never_reallocates() { + // A realloc frees the old buffer (with the PIN) WITHOUT zeroizing it. + for n in 1..=12 { + let mut pin = Pin::default(); + for _ in 0..n { + pin.push('9'); + } + let line = pin + .approve_line("2f1c9f3e-0000-4000-8000-0123456789ab") + .unwrap(); + assert_eq!( + line.capacity(), + line.len(), + "approve_line must reserve exactly (no realloc) for pin_len {n}" + ); + } + } + + #[test] + fn approve_line_escapes_the_id_so_it_cannot_inject_json() { + let mut pin = Pin::default(); + pin.push('1'); + let line = pin.approve_line(r#"a","pin":"0000"#).unwrap(); + // The hostile id stays one JSON string value: the real PIN is still the + // last `pin` member, and the injected quotes are escaped. + let parsed: serde_json::Value = serde_json::from_str(&line).unwrap(); + assert_eq!(parsed["id"], r#"a","pin":"0000"#); + assert_eq!(parsed["pin"], "1"); + } + + #[test] + fn a_parked_approve_pin_is_redacted_in_the_model_debug() { + // Park a money intent behind an in-flight poll, then Debug the whole model: + // `Zeroizing` forwards Debug to the String, so a derived Debug on + // the intent would print the PIN. + let mut m = confirming("a", true); + m.update(Msg::Approve); + type_pin(&mut m, "483920"); + assert!(matches!( + m.update(Msg::Tick), + Some(transport::Request::List) + )); + assert!(m.update(Msg::PinSubmit).is_none(), "parked behind the poll"); + let dbg = format!("{m:?}"); + assert!(!dbg.contains("483920"), "the PIN must not appear in Debug"); + assert!(dbg.contains("redacted")); + } + + #[test] + fn a_money_intent_parked_behind_a_poll_is_sent_after_the_reply() { + let mut m = confirming("a", false); + assert!(matches!( + m.update(Msg::Tick), + Some(transport::Request::List) + )); + assert!(m.update(Msg::Approve).is_none(), "parked, not lost"); + let flushed = m.update(Msg::Reply(Reply::List(vec![summary("a")]))); + assert!(matches!(flushed, Some(transport::Request::Approve(id)) if id == "a")); + } + + // ── confirmation: default-deny (AGENTS.md #5) ── + + #[test] + fn rejecting_sends_deny() { + let mut m = confirming("a", false); + let req = m.update(Msg::Reject); + assert!(matches!(req, Some(transport::Request::Deny(id)) if id == "a")); + } + + #[test] + fn rejecting_a_high_risk_card_needs_no_pin() { + // Saying no must always be cheap (protocol §3.6). + let mut m = confirming("a", true); + m.update(Msg::Approve); // the PIN prompt is up + let req = m.update(Msg::Reject); + assert!(matches!(req, Some(transport::Request::Deny(id)) if id == "a")); + } + + #[test] + fn a_second_decision_while_one_is_on_the_wire_is_ignored_and_not_parked() { + let mut m = confirming("a", true); + m.update(Msg::Approve); + type_pin(&mut m, "1234"); + assert!( + m.update(Msg::PinSubmit).is_some(), + "the approve is on the wire" + ); + + // Further keys send nothing… + assert!(m.update(Msg::Reject).is_none()); + assert!(m.update(Msg::Approve).is_none()); + assert!(m.update(Msg::Expire).is_none()); + + // …and are not *parked* either. A non-terminal answer flushes the parking + // slot, which would resurrect a deny for an item the human just approved. + let after = m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::BadPin { + attempts_left: 2, + }))); + assert!( + after.is_none(), + "an ignored keystroke must never be parked behind the decision" + ); + assert!(matches!( + m.phase(), + Phase::Watching { + confirm: Some(_), + .. + } + )); + } + + #[test] + fn an_approve_pressed_after_a_reject_is_never_parked_and_can_never_sign() { + // The scary direction: the human said no, a stray `y` must not survive in + // the parking slot and sign once the server answers something non-terminal. + let mut m = confirming("a", false); + assert!(m.update(Msg::Reject).is_some()); + assert!(m.update(Msg::Approve).is_none()); + let after = m.update(Msg::Reply(Reply::Resolve( + ResolveOutcome::AlreadyResolved { + state: TerminalState::Pending, + }, + ))); + assert!( + after.is_none(), + "a parked approve would sign what the human refused" + ); + assert_eq!(confirm_of(&m).error(), Some(&ResolveError::Busy)); + } + + #[test] + fn a_keystroke_cannot_edit_a_pin_that_is_already_on_the_wire() { + let mut m = confirming("a", true); + m.update(Msg::Approve); + type_pin(&mut m, "1234"); + m.update(Msg::PinSubmit); // sent + type_pin(&mut m, "9"); + assert_eq!(confirm_of(&m).pin_len(), Some(4), "the sent PIN is frozen"); + } + + // ── confirmation: terminal answers ── + + #[test] + fn an_executed_answer_exits_approved_and_keeps_the_tx_hash() { + let mut m = confirming("a", false); + m.update(Msg::Approve); + m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::Executed { + tx_hash: "0xfeed".to_owned(), + }))); + let Phase::Resolved { outcome, exit } = m.phase() else { + panic!("resolved"); + }; + assert_eq!(*exit, ExitOutcome::Approved); + assert!(matches!(outcome, ResolveOutcome::Executed { tx_hash } if tx_hash == "0xfeed")); + } + + #[test] + fn a_failed_broadcast_exits_failed_not_approved() { + let mut m = confirming("a", false); + m.update(Msg::Approve); + m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::Failed { + reason: "nonce too low".to_owned(), + }))); + let Phase::Resolved { exit, .. } = m.phase() else { + panic!("resolved"); + }; + assert_eq!(*exit, ExitOutcome::Failed, "no money moved"); + } + + #[test] + fn a_human_deny_exits_rejected() { + let mut m = confirming("a", false); + m.update(Msg::Reject); + m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::Denied))); + let Phase::Resolved { exit, .. } = m.phase() else { + panic!("resolved"); + }; + assert_eq!(*exit, ExitOutcome::Rejected); + } + + #[test] + fn the_deadline_denies_fail_closed_and_exits_expired_not_rejected() { + let mut m = confirming("a", false); + let req = m.update(Msg::Expire); + assert!( + matches!(req, Some(transport::Request::Deny(id)) if id == "a"), + "an expiring item is denied, never left pending" + ); + // The server has not observed the expiry yet and simply denies it. + m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::Denied))); + let Phase::Resolved { exit, .. } = m.phase() else { + panic!("resolved"); + }; + assert_eq!( + *exit, + ExitOutcome::Expired, + "the deadline said no, not the human" + ); + } + + #[test] + fn an_item_executed_by_another_connection_exits_approved_even_if_we_denied() { + // The exit reports what happened to the money, not which key was pressed. + let mut m = confirming("a", false); + m.update(Msg::Reject); + m.update(Msg::Reply(Reply::Resolve( + ResolveOutcome::AlreadyResolved { + state: TerminalState::Executed, + }, + ))); + let Phase::Resolved { exit, .. } = m.phase() else { + panic!("resolved"); + }; + assert_eq!(*exit, ExitOutcome::Approved); + } + + #[test] + fn an_already_expired_item_exits_expired() { + let mut m = confirming("a", false); + m.update(Msg::Approve); + m.update(Msg::Reply(Reply::Resolve( + ResolveOutcome::AlreadyResolved { + state: TerminalState::Expired, + }, + ))); + assert!(matches!( + m.phase(), + Phase::Resolved { + exit: ExitOutcome::Expired, + .. + } + )); + } + + #[test] + fn terminal_exit_maps_every_answer() { + use ResolveOutcome as R; + assert_eq!( + terminal_exit( + &R::Executed { + tx_hash: String::new() + }, + false + ), + Some(ExitOutcome::Approved) + ); + assert_eq!( + terminal_exit( + &R::Failed { + reason: String::new() + }, + true + ), + Some(ExitOutcome::Failed) + ); + assert_eq!( + terminal_exit(&R::Denied, false), + Some(ExitOutcome::Rejected) + ); + assert_eq!(terminal_exit(&R::Denied, true), Some(ExitOutcome::Expired)); + assert_eq!( + terminal_exit( + &R::AlreadyResolved { + state: TerminalState::Denied + }, + true + ), + Some(ExitOutcome::Rejected), + "somebody else's deny is a rejection, never our deadline" + ); + assert_eq!( + terminal_exit( + &R::AlreadyResolved { + state: TerminalState::Pending + }, + false + ), + None, + "pending means another connection is executing — retry, do not exit" + ); + for live in [ + R::PinRequired, + R::BadPin { attempts_left: 1 }, + R::Locked { retry_after_s: 1 }, + R::PinNotSet, + R::PinUnavailable, + R::UnknownId, + R::Unauthorized, + ] { + assert_eq!( + terminal_exit(&live, false), + None, + "{live:?} is not terminal" + ); + } + } + + // ── confirmation: non-terminal answers keep the human in charge ── + + #[test] + fn pin_required_opens_the_pin_prompt_and_keeps_the_item_live() { + // The server considers the item high-risk even though the summary did not. + let mut m = confirming("a", false); + m.update(Msg::Approve); + m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::PinRequired))); + let c = confirm_of(&m); + assert_eq!(c.pin_len(), Some(0)); + assert_eq!(c.error(), Some(&ResolveError::PinRequired)); + assert!(!c.is_resolving(), "the human may act again"); + } + + #[test] + fn a_bad_pin_clears_the_digits_and_shows_the_attempts_left() { + let mut m = confirming("a", true); + m.update(Msg::Approve); + type_pin(&mut m, "0000"); + m.update(Msg::PinSubmit); + m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::BadPin { + attempts_left: 2, + }))); + let c = confirm_of(&m); + assert_eq!( + c.pin_len(), + Some(0), + "the wrong PIN is cleared for re-entry" + ); + assert_eq!(c.error(), Some(&ResolveError::BadPin(2))); + assert!(!c.is_resolving()); + } + + #[test] + fn a_pending_race_is_transient_and_lets_the_human_retry() { + let mut m = confirming("a", false); + m.update(Msg::Reject); + m.update(Msg::Reply(Reply::Resolve( + ResolveOutcome::AlreadyResolved { + state: TerminalState::Pending, + }, + ))); + let c = confirm_of(&m); + assert_eq!(c.error(), Some(&ResolveError::Busy)); + assert!(!c.is_resolving()); + // and a retry is actually sent + assert!(matches!( + m.update(Msg::Reject), + Some(transport::Request::Deny(_)) + )); + } + + #[test] + fn a_locked_answer_shows_the_lockout_and_clears_the_pin() { + let mut m = confirming("a", true); + m.update(Msg::Approve); + type_pin(&mut m, "1111"); + m.update(Msg::PinSubmit); + m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::Locked { + retry_after_s: 300, + }))); + let c = confirm_of(&m); + assert_eq!(c.error(), Some(&ResolveError::Locked(300))); + assert_eq!(c.pin_len(), Some(0)); + } + + #[test] + fn a_vanished_item_closes_the_confirmation_with_a_note() { + let mut m = confirming("a", false); + m.update(Msg::Approve); + m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::UnknownId))); + let Phase::Watching { confirm, note, .. } = m.phase() else { + panic!("back to watching"); + }; + assert!(confirm.is_none()); + assert!(note.is_some()); + } + + #[test] + fn an_unauthorized_answer_to_an_authed_session_is_fatal() { + let mut m = confirming("a", false); + m.update(Msg::Approve); + m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::Unauthorized))); + assert!( + matches!(m.phase(), Phase::Fatal(TransportError::Protocol(_))), + "fail closed: never retry a money action on a channel we misread" + ); + } + + #[test] + fn a_resolve_answer_without_an_open_confirmation_is_ignored() { + let mut m = watching(vec![summary("a")]); + m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::Denied))); + assert!(matches!(m.phase(), Phase::Watching { .. })); + } + + #[test] + fn approve_and_reject_do_nothing_without_an_open_confirmation() { + let mut m = watching(vec![summary("a")]); + assert!(m.update(Msg::Approve).is_none()); + assert!(m.update(Msg::Reject).is_none()); + assert!(m.update(Msg::Expire).is_none()); + } + #[test] fn navigation_saturates_at_the_edges() { let mut m = watching(vec![summary("a"), summary("b")]); diff --git a/src/main.rs b/src/main.rs index 83eac64..d498131 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,7 +18,7 @@ use ratatui::crossterm::terminal::{ EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, enable_raw_mode, }; -use rustok_console::app::{Model, Msg, Phase}; +use rustok_console::app::{ExitOutcome, Model, Msg, Phase}; use rustok_console::transport::{Transport, TransportError}; use rustok_console::ui; @@ -32,12 +32,22 @@ const DEFAULT_SOCKET: &str = "/run/wallet/approve.sock"; /// How often the watch screen polls `list`. const POLL: Duration = Duration::from_millis(2500); -// Exit codes (AGENTS.md #7). C-PR-1a covers the read-only subset; approve/deny -// outcomes (approved/rejected/expired) arrive with C-PR-1b. -const EXIT_ABORTED: u8 = 0; +// Exit codes (AGENTS.md #7) — each decision outcome is distinguishable. +/// The transaction was signed and broadcast. +const EXIT_APPROVED: u8 = 0; +/// The connection broke, or an approved transaction failed to broadcast — no money +/// moved and the reason is on the screen. const EXIT_FATAL: u8 = 1; +/// The server speaks a protocol major this build does not. const EXIT_UPGRADE: u8 = 2; +/// No interactive terminal: an approval may never come from a pipe (invariant #4). const EXIT_NO_TTY: u8 = 3; +/// A human said no. +const EXIT_REJECTED: u8 = 4; +/// The deadline passed before a decision. +const EXIT_EXPIRED: u8 = 5; +/// Quit from the queue without deciding anything. +const EXIT_ABORTED: u8 = 6; fn main() -> ExitCode { let path = std::env::var("RUSTOK_APPROVE_SOCK").unwrap_or_else(|_| DEFAULT_SOCKET.to_owned()); @@ -96,10 +106,19 @@ fn run(mut terminal: Tui, transport: &Transport) -> u8 { return EXIT_FATAL; } - if let Phase::Fatal(err) = model.phase() { - let code = fatal_code(err); - wait_for_key(); - return code; + match model.phase() { + Phase::Fatal(err) => { + let code = fatal_code(err); + wait_for_key(); + return code; + } + // The item is terminal: show the answer, then carry it in the exit code. + Phase::Resolved { exit, .. } => { + let code = exit_code(*exit); + wait_for_key(); + return code; + } + _ => {} } let timeout = POLL.saturating_sub(last_tick.elapsed()); @@ -155,17 +174,37 @@ fn fatal_code(err: &TransportError) -> u8 { } } +/// The decision the process exits with. A `failed` execution is not an approval a +/// caller can act on — the transaction never made it out — so it exits fatal. +fn exit_code(exit: ExitOutcome) -> u8 { + match exit { + ExitOutcome::Approved => EXIT_APPROVED, + ExitOutcome::Rejected => EXIT_REJECTED, + ExitOutcome::Expired => EXIT_EXPIRED, + ExitOutcome::Failed => EXIT_FATAL, + } +} + /// Map a key press to a message, given the current phase. Returns `None` for keys /// with no meaning in that phase. fn map_key(key: &KeyEvent, phase: &Phase) -> Option { - // Ctrl-C always quits. + let confirm = match phase { + Phase::Watching { confirm, .. } => confirm.as_deref(), + _ => None, + }; + // Ctrl-C quits — but never walks away from an open confirmation without + // deciding: default-deny (AGENTS.md #5). if key.code == KeyCode::Char('c') && key.modifiers.contains(KeyModifiers::CONTROL) { - return Some(Msg::Quit); + return Some(if confirm.is_some() { + Msg::Reject + } else { + Msg::Quit + }); } match phase { - // Waiting for the handshake / showing a fatal reason: no interactive keys - // (fatal is handled by `wait_for_key`). - Phase::Connecting | Phase::Fatal(_) => None, + // Waiting for the handshake / showing a terminal screen: no interactive keys + // (both are handled by `wait_for_key`). + Phase::Connecting | Phase::Fatal(_) | Phase::Resolved { .. } => None, Phase::Authing { .. } => match key.code { KeyCode::Char(c) if c.is_ascii_digit() => Some(Msg::PinDigit(c)), KeyCode::Backspace => Some(Msg::PinBackspace), @@ -173,14 +212,35 @@ fn map_key(key: &KeyEvent, phase: &Phase) -> Option { KeyCode::Esc => Some(Msg::Quit), _ => None, }, - Phase::Watching { .. } => match key.code { + // The queue: no decision is pending, so quitting is free. + Phase::Watching { confirm: None, .. } => match key.code { KeyCode::Up | KeyCode::Char('k') => Some(Msg::MoveUp), KeyCode::Down | KeyCode::Char('j') => Some(Msg::MoveDown), KeyCode::Enter => Some(Msg::Open), - KeyCode::Esc | KeyCode::Backspace => Some(Msg::Back), KeyCode::Char('q') => Some(Msg::Quit), _ => None, }, + // An open card IS the confirmation: it is left by deciding, never by + // closing. A high-risk item takes the PIN on the same screen. + Phase::Watching { + confirm: Some(c), .. + } => { + if c.pin_len().is_some() { + match key.code { + KeyCode::Char(d) if d.is_ascii_digit() => Some(Msg::PinDigit(d)), + KeyCode::Backspace => Some(Msg::PinBackspace), + KeyCode::Enter => Some(Msg::PinSubmit), + KeyCode::Esc => Some(Msg::Reject), + _ => None, + } + } else { + match key.code { + KeyCode::Char('y' | 'Y') => Some(Msg::Approve), + KeyCode::Char('n' | 'N') | KeyCode::Esc => Some(Msg::Reject), + _ => None, + } + } + } } } @@ -188,6 +248,8 @@ fn map_key(key: &KeyEvent, phase: &Phase) -> Option { mod tests { use super::*; use rustok_console::app::Pin; + use rustok_console::protocol::{AuthOutcome, Card, GetOutcome, Kind, Risk, Summary}; + use rustok_console::transport::Reply; fn key(code: KeyCode) -> KeyEvent { KeyEvent::new(code, KeyModifiers::NONE) @@ -204,19 +266,122 @@ mod tests { Phase::Watching { items: vec![], selected: 0, - card: None, + confirm: None, note: None, } } + /// A model parked on an open confirmation — the only way to build one from + /// outside the crate, and the same path the real loop takes. + fn confirming(high_risk: bool) -> Model { + let mut m = Model::new(); + m.update(Msg::Reply(Reply::Hello { + server: "s".to_owned(), + })); + m.update(Msg::PinDigit('1')); + m.update(Msg::PinSubmit); + m.update(Msg::Reply(Reply::Auth(AuthOutcome::Ok))); + m.update(Msg::Tick); + m.update(Msg::Reply(Reply::List(vec![Summary { + id: "a".to_owned(), + kind: Kind::Send, + chain_id: 1, + to: "0xabc".to_owned(), + amount_wei: "0".to_owned(), + risk: Risk::Safe, + high_risk, + not_after_unix: 1, + }]))); + m.update(Msg::Open); + m.update(Msg::Reply(Reply::Get(GetOutcome::Card(Box::new(Card { + id: "a".to_owned(), + chain_id: 1, + to: "0xabc".to_owned(), + amount_wei: "0".to_owned(), + decoded_call: None, + high_risk, + high_risk_reasons: vec![], + raw_data: "0x".to_owned(), + not_after_unix: 1, + }))))); + m + } + #[test] - fn ctrl_c_quits_from_any_phase() { + fn ctrl_c_quits_when_no_decision_is_pending() { let k = KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL); assert!(matches!(map_key(&k, &Phase::Connecting), Some(Msg::Quit))); assert!(matches!(map_key(&k, &authing()), Some(Msg::Quit))); assert!(matches!(map_key(&k, &watching()), Some(Msg::Quit))); } + #[test] + fn ctrl_c_on_an_open_confirmation_rejects_it() { + // Default-deny (AGENTS.md #5): walking away must never leave an item + // pending for the next console to approve. + let k = KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL); + let m = confirming(false); + assert!(matches!(map_key(&k, m.phase()), Some(Msg::Reject))); + let m = confirming(true); + assert!(matches!(map_key(&k, m.phase()), Some(Msg::Reject))); + } + + #[test] + fn a_confirmation_maps_y_to_approve_and_n_or_esc_to_reject() { + let m = confirming(false); + assert!(matches!( + map_key(&key(KeyCode::Char('y')), m.phase()), + Some(Msg::Approve) + )); + assert!(matches!( + map_key(&key(KeyCode::Char('n')), m.phase()), + Some(Msg::Reject) + )); + assert!(matches!( + map_key(&key(KeyCode::Esc), m.phase()), + Some(Msg::Reject) + )); + // there is no "just close the card" key any more + assert!(map_key(&key(KeyCode::Backspace), m.phase()).is_none()); + assert!(map_key(&key(KeyCode::Char('q')), m.phase()).is_none()); + } + + #[test] + fn the_high_risk_pin_prompt_takes_digits_and_esc_still_rejects() { + let mut m = confirming(true); + m.update(Msg::Approve); // opens the PIN prompt + assert!(matches!( + map_key(&key(KeyCode::Char('7')), m.phase()), + Some(Msg::PinDigit('7')) + )); + assert!(matches!( + map_key(&key(KeyCode::Backspace), m.phase()), + Some(Msg::PinBackspace) + )); + assert!(matches!( + map_key(&key(KeyCode::Enter), m.phase()), + Some(Msg::PinSubmit) + )); + assert!(matches!( + map_key(&key(KeyCode::Esc), m.phase()), + Some(Msg::Reject) + )); + // 'y' is a no-op here: Enter submits the PIN + assert!(map_key(&key(KeyCode::Char('y')), m.phase()).is_none()); + } + + #[test] + fn exit_code_distinguishes_every_decision() { + assert_eq!(exit_code(ExitOutcome::Approved), EXIT_APPROVED); + assert_eq!(exit_code(ExitOutcome::Rejected), EXIT_REJECTED); + assert_eq!(exit_code(ExitOutcome::Expired), EXIT_EXPIRED); + assert_eq!(exit_code(ExitOutcome::Failed), EXIT_FATAL); + // aborted and no-tty are distinct from approved (invariant #7) + for code in [EXIT_ABORTED, EXIT_NO_TTY] { + assert_ne!(code, EXIT_APPROVED); + } + } + #[test] fn auth_phase_maps_digits_backspace_enter() { assert!(matches!( @@ -249,10 +414,6 @@ mod tests { map_key(&key(KeyCode::Enter), &watching()), Some(Msg::Open) )); - assert!(matches!( - map_key(&key(KeyCode::Esc), &watching()), - Some(Msg::Back) - )); assert!(matches!( map_key(&key(KeyCode::Char('q')), &watching()), Some(Msg::Quit) @@ -260,7 +421,7 @@ mod tests { } #[test] - fn connecting_and_fatal_ignore_ordinary_keys() { + fn connecting_fatal_and_resolved_ignore_ordinary_keys() { assert!(map_key(&key(KeyCode::Enter), &Phase::Connecting).is_none()); assert!(map_key(&key(KeyCode::Char('x')), &Phase::Connecting).is_none()); assert!( @@ -270,6 +431,17 @@ mod tests { ) .is_none() ); + assert!( + map_key( + &key(KeyCode::Char('y')), + &Phase::Resolved { + outcome: rustok_console::protocol::ResolveOutcome::Denied, + exit: ExitOutcome::Rejected, + } + ) + .is_none(), + "a resolved item can no longer be approved" + ); } #[test] diff --git a/src/ui.rs b/src/ui.rs index 7e6d15a..bf696ca 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -9,8 +9,8 @@ use ratatui::style::{Modifier, Style}; use ratatui::text::{Line, Span}; use ratatui::widgets::{Block, List, ListItem, ListState, Paragraph, Wrap}; -use crate::app::{AuthError, Model, Phase}; -use crate::protocol::{Card, Summary}; +use crate::app::{AuthError, Confirm, ExitOutcome, Model, Phase, ResolveError}; +use crate::protocol::{Card, ResolveOutcome, Summary}; /// Render the whole screen for the current model. pub fn render(frame: &mut Frame, model: &Model) { @@ -22,13 +22,34 @@ pub fn render(frame: &mut Frame, model: &Model) { Phase::Watching { items, selected, - card, + confirm, note, - } => render_watch(frame, items, *selected, card.as_deref(), note.as_deref()), + } => render_watch(frame, items, *selected, confirm.as_deref(), note.as_deref()), + Phase::Resolved { outcome, exit } => render_centered(frame, &resolved_text(outcome, *exit)), Phase::Fatal(err) => render_centered(frame, &err.to_string()), } } +/// The terminal answer, shown verbatim before the console exits. +fn resolved_text(outcome: &ResolveOutcome, exit: ExitOutcome) -> String { + let detail = match outcome { + ResolveOutcome::Executed { tx_hash } => format!("executed — {tx_hash}"), + ResolveOutcome::Failed { reason } => format!("execution failed — {reason}"), + ResolveOutcome::Denied => "denied".to_owned(), + ResolveOutcome::AlreadyResolved { state } => { + format!("already resolved by someone else ({state:?})") + } + other => format!("{other:?}"), + }; + let headline = match exit { + ExitOutcome::Approved => "APPROVED", + ExitOutcome::Rejected => "REJECTED", + ExitOutcome::Expired => "EXPIRED", + ExitOutcome::Failed => "FAILED", + }; + format!("{headline}\n\n{detail}\n\nPress any key to close.") +} + fn render_centered(frame: &mut Frame, message: &str) { let block = Block::bordered().title(" Rustok Console "); let paragraph = Paragraph::new(message).block(block); @@ -66,7 +87,7 @@ fn render_watch( frame: &mut Frame, items: &[Summary], selected: usize, - card: Option<&Card>, + confirm: Option<&Confirm>, note: Option<&str>, ) { let chunks = Layout::vertical([ @@ -83,9 +104,19 @@ fn render_watch( ); render_queue(frame, items, selected, chunks[1]); - render_detail(frame, card, chunks[2]); + render_detail(frame, confirm, chunks[2]); - let footer = note.unwrap_or("↑/↓ select · enter open · q quit (approve/deny in C-PR-1b)"); + let footer = note.unwrap_or_else(|| { + confirm.map_or("↑/↓ select · enter open · q quit", |c| { + if c.is_resolving() { + "sending your decision…" + } else if c.pin_len().is_some() { + "enter your PIN · enter approve · esc reject" + } else { + "y approve · n/esc reject" + } + }) + }); frame.render_widget(Paragraph::new(footer), chunks[3]); } @@ -129,16 +160,17 @@ fn kind_word(s: &Summary) -> &'static str { } } -/// Render the selected card **verbatim** — the core's fields as received, no -/// re-derivation. `None` shows a hint to open one. -fn render_detail(frame: &mut Frame, card: Option<&Card>, area: ratatui::layout::Rect) { +/// Render the open confirmation's card **verbatim** — the core's fields as +/// received, no re-derivation. `None` shows a hint to open one. +fn render_detail(frame: &mut Frame, confirm: Option<&Confirm>, area: ratatui::layout::Rect) { let block = Block::bordered().title(" Card "); - let Some(card) = card else { + let Some(confirm) = confirm else { let hint = Paragraph::new("Select a request and press enter to see the full card.").block(block); frame.render_widget(hint, area); return; }; + let card: &Card = confirm.card(); let mut lines = vec![ kv("to", &card.to), @@ -171,6 +203,20 @@ fn render_detail(frame: &mut Frame, card: Option<&Card>, area: ratatui::layout:: } } } + if let Some(pin_len) = confirm.pin_len() { + lines.push(Line::from("")); + lines.push(Line::from("High-risk approval — enter your PIN:")); + // Only the count is shown — never the digits. + lines.push(Line::from(Span::styled( + "●".repeat(pin_len), + Style::new().add_modifier(Modifier::BOLD), + ))); + } + if let Some(err) = confirm.error() { + lines.push(Line::from("")); + lines.push(Line::from(resolve_error_text(err))); + } + // Wrap (never truncate): a clear-signing card must show the whole value — // a silently clipped raw_data or address would be the one lie this screen // exists to prevent. trim: false keeps the exact bytes, including leading space. @@ -182,6 +228,17 @@ fn render_detail(frame: &mut Frame, card: Option<&Card>, area: ratatui::layout:: ); } +fn resolve_error_text(err: &ResolveError) -> String { + match err { + ResolveError::PinRequired => "This approval needs your PIN.".to_owned(), + ResolveError::BadPin(left) => format!("Wrong PIN — {left} attempt(s) left."), + ResolveError::Locked(secs) => format!("Locked out. Try again in {secs}s."), + ResolveError::NotSet => "This wallet has no PIN set (run set-pin).".to_owned(), + ResolveError::Unavailable => "PIN check unavailable — try again.".to_owned(), + ResolveError::Busy => "Another approval is executing this request — retry.".to_owned(), + } +} + fn kv(key: &str, value: &str) -> Line<'static> { Line::from(format!("{key}: {value}")) } From b06be712144201110576dabfd72e2d7698df64ba Mon Sep 17 00:00:00 2001 From: temrjan Date: Thu, 9 Jul 2026 16:08:46 +0500 Subject: [PATCH 04/11] =?UTF-8?q?feat(ui):=20confirmation=20row=20?= =?UTF-8?q?=E2=80=94=20countdown=20on=20Reject,=20PIN=20prompt,=20result?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The decision row is the screen's only moving part: the expiry countdown rides the Reject button and says what it will do on its own ("auto in 27s"), so the clock never draws the eye to the button that moves money (AGENTS.md #5). Reject is drawn as the focused button because it is what happens if the human does nothing; Approve is a quiet outline that has to be chosen. render() now takes the wall clock as a parameter instead of reading it, keeping Model a pure function of its messages and the countdown testable. main.rs reads it fail-closed: an unreadable clock reports u64::MAX — past every deadline — so a broken clock can never buy an approval more time. The note's row is claimed only when there is a note; an always-reserved row took its space from the card, silently clipping the last rendered field. --- src/main.rs | 18 +++- src/ui.rs | 244 +++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 240 insertions(+), 22 deletions(-) diff --git a/src/main.rs b/src/main.rs index d498131..c077b11 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,7 @@ use std::io::{self, Stderr}; use std::process::ExitCode; -use std::time::{Duration, Instant}; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use ratatui::Terminal; use ratatui::backend::CrosstermBackend; @@ -102,7 +102,10 @@ fn run(mut terminal: Tui, transport: &Transport) -> u8 { let mut last_tick = Instant::now(); loop { - if terminal.draw(|f| ui::render(f, &model)).is_err() { + if terminal + .draw(|f| ui::render(f, &model, now_unix())) + .is_err() + { return EXIT_FATAL; } @@ -156,6 +159,17 @@ fn run(mut terminal: Tui, transport: &Transport) -> u8 { } } +/// Wall-clock seconds since the Unix epoch, used to render the expiry countdown. +/// +/// A clock that cannot be read reports `u64::MAX` — *past every deadline* — so the +/// countdown floors at zero rather than granting the approval unbounded time. The +/// broken clock fails closed, like everything else on this path (`AGENTS.md` #5). +fn now_unix() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(u64::MAX, |d| d.as_secs()) +} + /// Block until the next key press (so a human can read a fatal message). fn wait_for_key() { loop { diff --git a/src/ui.rs b/src/ui.rs index bf696ca..e819b48 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -13,7 +13,10 @@ use crate::app::{AuthError, Confirm, ExitOutcome, Model, Phase, ResolveError}; use crate::protocol::{Card, ResolveOutcome, Summary}; /// Render the whole screen for the current model. -pub fn render(frame: &mut Frame, model: &Model) { +/// +/// `now_unix` is the wall clock, passed in rather than read here: the [`Model`] +/// stays a pure function of its messages, and the countdown stays testable. +pub fn render(frame: &mut Frame, model: &Model, now_unix: u64) { match model.phase() { Phase::Connecting => { render_centered(frame, "Connecting to the wallet…"); @@ -24,7 +27,14 @@ pub fn render(frame: &mut Frame, model: &Model) { selected, confirm, note, - } => render_watch(frame, items, *selected, confirm.as_deref(), note.as_deref()), + } => render_watch( + frame, + items, + *selected, + confirm.as_deref(), + note.as_deref(), + now_unix, + ), Phase::Resolved { outcome, exit } => render_centered(frame, &resolved_text(outcome, *exit)), Phase::Fatal(err) => render_centered(frame, &err.to_string()), } @@ -89,14 +99,21 @@ fn render_watch( selected: usize, confirm: Option<&Confirm>, note: Option<&str>, + now_unix: u64, ) { - let chunks = Layout::vertical([ + // The note's row is claimed only when there is a note. An always-reserved row + // would take its space from the card, and the card is the one thing on this + // screen that must never be silently clipped (`AGENTS.md` #1). + let mut constraints = vec![ Constraint::Length(1), // header Constraint::Min(3), // queue Constraint::Min(6), // card / hint - Constraint::Length(1), // note / footer - ]) - .split(frame.area()); + Constraint::Length(1), // decision row / navigation hint + ]; + if note.is_some() { + constraints.push(Constraint::Length(1)); // transient note + } + let chunks = Layout::vertical(constraints).split(frame.area()); frame.render_widget( Paragraph::new(format!(" Pending approvals: {}", items.len())), @@ -105,19 +122,73 @@ fn render_watch( render_queue(frame, items, selected, chunks[1]); render_detail(frame, confirm, chunks[2]); + render_actions(frame, confirm, now_unix, chunks[3]); - let footer = note.unwrap_or_else(|| { - confirm.map_or("↑/↓ select · enter open · q quit", |c| { - if c.is_resolving() { - "sending your decision…" - } else if c.pin_len().is_some() { - "enter your PIN · enter approve · esc reject" - } else { - "y approve · n/esc reject" - } - }) - }); - frame.render_widget(Paragraph::new(footer), chunks[3]); + if let Some(note) = note { + frame.render_widget(Paragraph::new(note), chunks[4]); + } +} + +/// Seconds left before the open card's deadline. +/// +/// Saturating on purpose: a deadline already in the past reads as `0`, never as a +/// wrapped-around eternity. An unreadable clock reaches us as `u64::MAX` (see +/// `main::now_unix`) and lands here as `0` too — a broken clock can never hand an +/// approval more time. +fn seconds_left(not_after_unix: u64, now_unix: u64) -> u64 { + not_after_unix.saturating_sub(now_unix) +} + +/// The decision row. +/// +/// The countdown rides the **Reject** button and nothing else on this screen moves +/// (`AGENTS.md` #5). Reject is drawn as the focused button — reversed and bold — +/// because it is what happens if the human does nothing; Approve is a quiet outline +/// that has to be chosen. The copy says so out loud: `auto in 27s`. +fn render_actions( + frame: &mut Frame, + confirm: Option<&Confirm>, + now_unix: u64, + area: ratatui::layout::Rect, +) { + let Some(confirm) = confirm else { + frame.render_widget(Paragraph::new(" ↑/↓ select · enter open · q quit"), area); + return; + }; + if confirm.is_resolving() { + // The decision is on the wire and the buttons are gone with it, so a second + // press cannot be mistaken for a second decision. + frame.render_widget(Paragraph::new(" Sending your decision…"), area); + return; + } + + // The PIN prompt owns Enter, so Enter — not `y` — is what approves while it is up. + let approve_key = if confirm.pin_len().is_some() { + "enter" + } else { + "y" + }; + let reject_key = if confirm.pin_len().is_some() { + "esc" + } else { + "n / esc" + }; + let left = seconds_left(confirm.card().not_after_unix, now_unix); + + frame.render_widget( + Paragraph::new(Line::from(vec![ + Span::raw(" "), + Span::raw(format!("[ {approve_key} Approve ]")), + Span::raw(" "), + Span::styled( + format!("[ {reject_key} Reject · auto in {left}s ]"), + Style::new() + .add_modifier(Modifier::BOLD) + .add_modifier(Modifier::REVERSED), + ), + ])), + area, + ); } fn render_queue( @@ -258,24 +329,39 @@ mod tests { use ratatui::Terminal; use ratatui::backend::TestBackend; + /// A fixed "now" for the countdown tests. Real time never enters the renderer. + const NOW: u64 = 1_000_000_000; + /// Render into a fixed grid, returning the screen as rows. Row-level checks /// catch a field rendered under the WRONG label (a swap) — which a /// whole-screen substring check would miss. - fn draw_rows(model: &Model, w: u16, h: u16) -> Vec { + fn draw_rows_at(model: &Model, w: u16, h: u16, now_unix: u64) -> Vec { let backend = TestBackend::new(w, h); let mut terminal = Terminal::new(backend).unwrap(); - terminal.draw(|f| render(f, model)).unwrap(); + terminal.draw(|f| render(f, model, now_unix)).unwrap(); let buffer = terminal.backend().buffer(); (0..h) .map(|y| (0..w).map(|x| buffer[(x, y)].symbol()).collect::()) .collect() } + fn draw_rows(model: &Model, w: u16, h: u16) -> Vec { + draw_rows_at(model, w, h, NOW) + } + /// Flatten to one String for checks that do not care about layout. fn draw(model: &Model, w: u16, h: u16) -> String { draw_rows(model, w, h).join("\n") } + /// The rendered decision row (the line carrying the buttons). + fn action_row(rows: &[String]) -> String { + rows.iter() + .find(|r| r.contains("Approve")) + .expect("the decision row must render") + .clone() + } + /// True if some rendered line contains all `fragments` — a label+value /// adjacency check, so a swapped field is caught. fn has_line_with(rows: &[String], fragments: &[&str]) -> bool { @@ -430,4 +516,122 @@ mod tests { let screen = draw(&m, 80, 10); assert!(screen.contains("wallet not running")); } + + fn card(id: &str, not_after_unix: u64, high_risk: bool) -> Box { + Box::new(Card { + id: id.to_owned(), + chain_id: 1, + to: "0xabc".to_owned(), + amount_wei: "0".to_owned(), + decoded_call: None, + high_risk, + high_risk_reasons: if high_risk { + vec!["unlimited_approval".to_owned()] + } else { + vec![] + }, + raw_data: "0x".to_owned(), + not_after_unix, + }) + } + + /// Drive the model to an open confirmation on a single queued item. + fn open_card(model: &mut Model, id: &str, not_after_unix: u64, high_risk: bool) { + to_watching(model, vec![summary(id, "0xabc", "0", high_risk)]); + model.update(Msg::Open); + model.update(Msg::Reply(Reply::Get(crate::protocol::GetOutcome::Card( + card(id, not_after_unix, high_risk), + )))); + } + + #[test] + fn the_countdown_rides_the_reject_button_never_the_approve_one() { + let mut m = Model::new(); + open_card(&mut m, "a1", NOW + 27, false); + + let row = action_row(&draw_rows(&m, 100, 24)); + // The Approve button closes at the first `]`; everything after it is Reject. + let (approve_side, reject_side) = row.split_once(']').expect("two buttons render"); + + assert!(approve_side.contains("Approve")); + assert!( + !approve_side.contains("27s"), + "the deadline must never count down on the button that moves money \ + (AGENTS.md #5); found: {row}" + ); + assert!( + reject_side.contains("Reject") && reject_side.contains("auto in 27s"), + "the countdown belongs to Reject, and says it will fire on its own: {row}" + ); + } + + #[test] + fn the_countdown_floors_at_zero_once_the_deadline_has_passed() { + let mut m = Model::new(); + open_card(&mut m, "a1", NOW + 27, false); + + let row = action_row(&draw_rows_at(&m, 100, 24, NOW + 99)); + assert!( + row.contains("auto in 0s"), + "an elapsed deadline reads as 0s, never as a wrapped-around eternity: {row}" + ); + + // An unreadable clock reaches the renderer as u64::MAX (`main::now_unix`). + // It must floor to 0s too — a broken clock never buys the approval more time. + let row = action_row(&draw_rows_at(&m, 100, 24, u64::MAX)); + assert!( + row.contains("auto in 0s"), + "an unreadable clock fails closed, it does not grant time: {row}" + ); + } + + #[test] + fn the_pin_prompt_moves_approve_onto_enter_and_masks_the_digits() { + let mut m = Model::new(); + open_card(&mut m, "a1", NOW + 27, true); + m.update(Msg::Approve); // high risk: `y` opens the PIN prompt, it does not approve + m.update(Msg::PinDigit('7')); + m.update(Msg::PinDigit('3')); + + let rows = draw_rows(&m, 100, 24); + let row = action_row(&rows); + let screen = rows.join("\n"); + + assert!( + row.contains("enter Approve") && row.contains("esc Reject"), + "while the PIN prompt is up, Enter approves and Esc rejects: {row}" + ); + assert!(screen.contains("●●"), "two dots for two digits"); + assert!(!screen.contains("73"), "the digits must never render"); + } + + #[test] + fn a_decision_on_the_wire_replaces_the_buttons() { + let mut m = Model::new(); + open_card(&mut m, "a1", NOW + 27, false); + m.update(Msg::Reject); + + let screen = draw(&m, 100, 24); + + assert!(screen.contains("Sending your decision")); + assert!( + !screen.contains("Approve"), + "with a decision on the wire there is no button left to press twice" + ); + } + + #[test] + fn the_resolved_screen_names_the_outcome_and_shows_the_tx_hash() { + let mut m = Model::new(); + open_card(&mut m, "a1", NOW + 27, false); + m.update(Msg::Approve); + m.update(Msg::Reply(Reply::Resolve(ResolveOutcome::Executed { + tx_hash: "0xfeed".to_owned(), + }))); + + let screen = draw(&m, 80, 12); + + assert!(screen.contains("APPROVED")); + assert!(screen.contains("0xfeed"), "the tx hash is shown verbatim"); + } } From d802056806f2737378ceaec27dc1f1fa03638b56 Mon Sep 17 00:00:00 2001 From: temrjan Date: Thu, 9 Jul 2026 16:29:20 +0500 Subject: [PATCH 05/11] feat(main): TTY gate, expiry clock, machine-readable decision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Invariant #4 becomes explicit. `enable_raw_mode` opens /dev/tty directly, so a successful terminal init proves nothing about stdin: a session with a controlling pty but a piped stdin used to enter the approval screen. stdin is now checked before any terminal or socket is touched, and a non-interactive one exits 3. The expiry clock is wired: once an open card's deadline passes, Msg::Expire denies it and reports `expired` — the deadline said no, not a human. A decision already on the wire outranks the clock, so we never deny what we just approved. The loop now wakes every 250ms, so the countdown's seconds actually tick. The decision leaves on stdout as one line of JSON, after the alternate screen is gone (invariant #7). It names what happened to the money and carries a detail only when we know it: an item another session executed reports `approved` with no hash, because the hash was never ours. serde_json escapes the server's `reason`, so a hostile string cannot forge a second decision line. --- src/main.rs | 220 +++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 194 insertions(+), 26 deletions(-) diff --git a/src/main.rs b/src/main.rs index c077b11..1144c75 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,15 +1,17 @@ //! Rustok Console — terminal approval screen (the human face of the wallet). //! -//! v0.1 (C-PR-1a): connect → hello → PIN → **watch** the queue read-only. Keys are +//! v0.1: connect → hello → PIN → watch the queue → approve or deny. Keys are //! mapped to [`Msg`] and folded into the [`Model`]; the socket worker runs on its -//! own thread so a slow core never freezes the UI. `approve`/`deny` land in -//! C-PR-1b. UI goes to stderr (ratatui's alternate screen); the exit code carries -//! the outcome. +//! own thread so a slow core never freezes the UI. UI goes to stderr (ratatui's +//! alternate screen), the machine-readable decision goes to stdout, and the exit +//! code carries the outcome (invariant #7). -use std::io::{self, Stderr}; +use std::io::{self, IsTerminal, Stderr}; use std::process::ExitCode; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +use serde_json::json; + use ratatui::Terminal; use ratatui::backend::CrosstermBackend; use ratatui::crossterm::event::{self, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; @@ -19,18 +21,22 @@ use ratatui::crossterm::terminal::{ }; use rustok_console::app::{ExitOutcome, Model, Msg, Phase}; +use rustok_console::protocol::ResolveOutcome; use rustok_console::transport::{Transport, TransportError}; use rustok_console::ui; -/// The console's terminal — rendered to **stderr**, so stdout stays clean for a -/// machine-readable decision (invariant #7; the approval verdict lands on stdout -/// in C-PR-1b). `ratatui::init` would use stdout, so the setup is done by hand. +/// The console's terminal — rendered to **stderr**, so stdout stays clean for the +/// machine-readable decision (invariant #7). `ratatui::init` would use stdout, so +/// the setup is done by hand. type Tui = Terminal>; /// Default approver socket path (overridable for tests / non-standard layouts). const DEFAULT_SOCKET: &str = "/run/wallet/approve.sock"; /// How often the watch screen polls `list`. const POLL: Duration = Duration::from_millis(2500); +/// Longest the loop sleeps between redraws — the countdown ticks in seconds, so it +/// must repaint far more often than the `list` poll. +const FRAME: Duration = Duration::from_millis(250); // Exit codes (AGENTS.md #7) — each decision outcome is distinguishable. /// The transaction was signed and broadcast. @@ -52,16 +58,30 @@ const EXIT_ABORTED: u8 = 6; fn main() -> ExitCode { let path = std::env::var("RUSTOK_APPROVE_SOCK").unwrap_or_else(|_| DEFAULT_SOCKET.to_owned()); - // No TTY → view-only is not possible for an interactive approver; fail clearly - // rather than half-render. (The full no-TTY view-only gate is C-PR-1b.) - let Ok(terminal) = try_init() else { + // Invariant #4, checked before anything is opened: an approval may never come + // from a pipe. `enable_raw_mode` opens `/dev/tty` directly, so a successful + // `try_init` proves nothing about stdin — only this does. Nothing is connected + // and no terminal is touched until stdin is known to be interactive. + if !io::stdin().is_terminal() { eprintln!("rustok-console needs an interactive terminal (a TTY)."); + eprintln!("Approval from a pipe is never accepted."); + return ExitCode::from(EXIT_NO_TTY); + } + + let Ok(terminal) = try_init() else { + eprintln!("rustok-console could not take the terminal."); return ExitCode::from(EXIT_NO_TTY); }; let transport = Transport::connect(&path); - let code = run(terminal, &transport); + let (code, decision) = run(terminal, &transport); let _ = restore(); + + // The decision lands on stdout only after the alternate screen is gone, so no + // escape sequence can ever reach a caller parsing it (invariant #7). + if let Some(decision) = decision { + println!("{decision}"); + } ExitCode::from(code) } @@ -96,8 +116,9 @@ fn set_panic_hook() { } /// The event loop: draw, read input, drain worker replies, tick the poll. Returns -/// the exit code. A `Fatal` phase is shown until a keypress, then exits. -fn run(mut terminal: Tui, transport: &Transport) -> u8 { +/// the exit code and, once an item is terminal, the machine-readable decision. +/// A `Fatal` phase is shown until a keypress, then exits. +fn run(mut terminal: Tui, transport: &Transport) -> (u8, Option) { let mut model = Model::new(); let mut last_tick = Instant::now(); @@ -106,25 +127,37 @@ fn run(mut terminal: Tui, transport: &Transport) -> u8 { .draw(|f| ui::render(f, &model, now_unix())) .is_err() { - return EXIT_FATAL; + return (EXIT_FATAL, None); } match model.phase() { Phase::Fatal(err) => { let code = fatal_code(err); wait_for_key(); - return code; + return (code, None); } - // The item is terminal: show the answer, then carry it in the exit code. - Phase::Resolved { exit, .. } => { + // The item is terminal: show the answer, then carry it out in both the + // exit code and the decision line. + Phase::Resolved { outcome, exit } => { let code = exit_code(*exit); + let decision = decision_line(outcome, *exit); wait_for_key(); - return code; + return (code, Some(decision)); } _ => {} } - let timeout = POLL.saturating_sub(last_tick.elapsed()); + // The deadline says no on its own (`AGENTS.md` #5). `Msg::Expire` denies the + // card and reports `expired` rather than `rejected` — no human pressed a key. + if deadline_passed(model.phase(), now_unix()) + && let Some(req) = model.update(Msg::Expire) + { + transport.send(req); + } + + // Wake up at least every FRAME so the countdown's seconds actually tick, + // and the deadline above is noticed within a frame of passing. + let timeout = POLL.saturating_sub(last_tick.elapsed()).min(FRAME); match event::poll(timeout) { Ok(true) => { if let Ok(Event::Key(key)) = event::read() @@ -136,7 +169,7 @@ fn run(mut terminal: Tui, transport: &Transport) -> u8 { } } Ok(false) => {} - Err(_) => return EXIT_FATAL, + Err(_) => return (EXIT_FATAL, None), } // Drain everything the worker has answered since the last pass. @@ -154,11 +187,43 @@ fn run(mut terminal: Tui, transport: &Transport) -> u8 { } if model.should_quit() { - return EXIT_ABORTED; + return (EXIT_ABORTED, None); } } } +/// Whether the open card's deadline has passed and nothing is on the wire yet. +/// +/// A decision already in flight is left alone: the server, not the clock, gets the +/// last word on an item we have already answered. +fn deadline_passed(phase: &Phase, now_unix: u64) -> bool { + matches!( + phase, + Phase::Watching { confirm: Some(c), .. } + if !c.is_resolving() && now_unix >= c.card().not_after_unix + ) +} + +/// The one line a caller parses (invariant #7). It names **what happened to the +/// money**, and carries a detail only when we actually know it: an item another +/// session executed comes back as `approved` with no hash, because the hash was +/// never ours to report. +fn decision_line(outcome: &ResolveOutcome, exit: ExitOutcome) -> String { + let value = match (exit, outcome) { + (ExitOutcome::Approved, ResolveOutcome::Executed { tx_hash }) => { + json!({ "decision": "approved", "tx_hash": tx_hash }) + } + (ExitOutcome::Approved, _) => json!({ "decision": "approved" }), + (ExitOutcome::Rejected, _) => json!({ "decision": "rejected" }), + (ExitOutcome::Expired, _) => json!({ "decision": "expired" }), + (ExitOutcome::Failed, ResolveOutcome::Failed { reason }) => { + json!({ "decision": "failed", "reason": reason }) + } + (ExitOutcome::Failed, _) => json!({ "decision": "failed" }), + }; + value.to_string() +} + /// Wall-clock seconds since the Unix epoch, used to render the expiry countdown. /// /// A clock that cannot be read reports `u64::MAX` — *past every deadline* — so the @@ -262,7 +327,9 @@ fn map_key(key: &KeyEvent, phase: &Phase) -> Option { mod tests { use super::*; use rustok_console::app::Pin; - use rustok_console::protocol::{AuthOutcome, Card, GetOutcome, Kind, Risk, Summary}; + use rustok_console::protocol::{ + AuthOutcome, Card, GetOutcome, Kind, Risk, Summary, TerminalState, + }; use rustok_console::transport::Reply; fn key(code: KeyCode) -> KeyEvent { @@ -285,9 +352,13 @@ mod tests { } } + fn confirming(high_risk: bool) -> Model { + confirming_at(high_risk, 1) + } + /// A model parked on an open confirmation — the only way to build one from /// outside the crate, and the same path the real loop takes. - fn confirming(high_risk: bool) -> Model { + fn confirming_at(high_risk: bool, not_after_unix: u64) -> Model { let mut m = Model::new(); m.update(Msg::Reply(Reply::Hello { server: "s".to_owned(), @@ -304,7 +375,7 @@ mod tests { amount_wei: "0".to_owned(), risk: Risk::Safe, high_risk, - not_after_unix: 1, + not_after_unix, }]))); m.update(Msg::Open); m.update(Msg::Reply(Reply::Get(GetOutcome::Card(Box::new(Card { @@ -316,7 +387,7 @@ mod tests { high_risk, high_risk_reasons: vec![], raw_data: "0x".to_owned(), - not_after_unix: 1, + not_after_unix, }))))); m } @@ -458,6 +529,103 @@ mod tests { ); } + #[test] + fn the_deadline_denies_an_open_card_and_nothing_else() { + // Nothing is open: the clock has nothing to say, however late it is. + assert!(!deadline_passed(&watching(), u64::MAX)); + assert!(!deadline_passed(&authing(), u64::MAX)); + + let m = confirming_at(false, 1_000); + assert!( + !deadline_passed(m.phase(), 999), + "a second before the deadline the human still owns the decision" + ); + assert!( + deadline_passed(m.phase(), 1_000), + "at the deadline the clock says no (AGENTS.md #5)" + ); + assert!(deadline_passed(m.phase(), 1_001)); + } + + #[test] + fn a_decision_on_the_wire_outranks_the_deadline() { + let mut m = confirming_at(false, 1_000); + m.update(Msg::Approve); + + assert!( + !deadline_passed(m.phase(), u64::MAX), + "an approve is already on the wire — the server, not the clock, has the \ + last word; expiring here would deny what we just approved" + ); + } + + #[test] + fn the_decision_line_names_what_happened_to_the_money() { + assert_eq!( + decision_line( + &ResolveOutcome::Executed { + tx_hash: "0xabc".to_owned() + }, + ExitOutcome::Approved + ), + r#"{"decision":"approved","tx_hash":"0xabc"}"# + ); + assert_eq!( + decision_line(&ResolveOutcome::Denied, ExitOutcome::Rejected), + r#"{"decision":"rejected"}"# + ); + // The same server answer, denied by the deadline rather than by a human. + assert_eq!( + decision_line(&ResolveOutcome::Denied, ExitOutcome::Expired), + r#"{"decision":"expired"}"# + ); + assert_eq!( + decision_line( + &ResolveOutcome::Failed { + reason: "nonce too low".to_owned() + }, + ExitOutcome::Failed + ), + r#"{"decision":"failed","reason":"nonce too low"}"# + ); + } + + #[test] + fn an_approval_by_another_session_reports_no_hash_we_never_saw() { + let line = decision_line( + &ResolveOutcome::AlreadyResolved { + state: TerminalState::Executed, + }, + ExitOutcome::Approved, + ); + + assert_eq!(line, r#"{"decision":"approved"}"#); + assert!( + !line.contains("tx_hash"), + "we never invent a hash the server did not give us" + ); + } + + #[test] + fn the_decision_line_survives_a_hostile_server_reason() { + // `reason` is server-controlled text landing on a caller's stdout. + let line = decision_line( + &ResolveOutcome::Failed { + reason: "\"}\n{\"decision\":\"approved\"".to_owned(), + }, + ExitOutcome::Failed, + ); + + assert!( + !line.contains('\n'), + "the decision is exactly one line — a reason cannot forge a second one" + ); + let parsed: serde_json::Value = + serde_json::from_str(&line).expect("one parsable JSON object"); + assert_eq!(parsed["decision"], "failed"); + assert_eq!(parsed["reason"], "\"}\n{\"decision\":\"approved\""); + } + #[test] fn fatal_code_distinguishes_upgrade_from_the_rest() { assert_eq!( From 2533b47f5765cd62355e4c4f0dbec91e61a3da98 Mon Sep 17 00:00:00 2001 From: temrjan Date: Thu, 9 Jul 2026 16:43:24 +0500 Subject: [PATCH 06/11] test(app): cover the parked high-risk approve and every already_resolved state Dropping flush_pending's ApprovePin arm would have silently lost an approval the human already confirmed with their PIN; nothing caught it. AlreadyResolved's Failed/Executed/Expired states never reached terminal_exit from a test either, so swapping Failed for Rejected would have changed the exit code and the decision line unnoticed. --- src/app.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/app.rs b/src/app.rs index a48d764..e60f3e0 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1166,6 +1166,26 @@ mod tests { assert!(dbg.contains("redacted")); } + #[test] + fn a_parked_high_risk_approve_is_sent_after_the_reply_pin_and_all() { + let mut m = confirming("a", true); + m.update(Msg::Approve); // high risk: opens the PIN prompt + type_pin(&mut m, "483920"); + assert!(matches!( + m.update(Msg::Tick), + Some(transport::Request::List) + )); + assert!(m.update(Msg::PinSubmit).is_none(), "parked behind the poll"); + + let flushed = m.update(Msg::Reply(Reply::List(vec![summary("a")]))); + + // Dropping this arm would silently lose an approval the human confirmed. + let Some(transport::Request::ApprovePin(line)) = flushed else { + panic!("the parked high-risk approve must be sent once the wire is free"); + }; + assert_eq!(&*line, r#"{"op":"approve","id":"a","pin":"483920"}"#); + } + #[test] fn a_money_intent_parked_behind_a_poll_is_sent_after_the_reply() { let mut m = confirming("a", false); @@ -1388,6 +1408,35 @@ mod tests { Some(ExitOutcome::Rejected), "somebody else's deny is a rejection, never our deadline" ); + assert_eq!( + terminal_exit( + &R::AlreadyResolved { + state: TerminalState::Failed + }, + false + ), + Some(ExitOutcome::Failed), + "somebody else's approval that failed to broadcast is a failure, not a rejection" + ); + assert_eq!( + terminal_exit( + &R::AlreadyResolved { + state: TerminalState::Executed + }, + true + ), + Some(ExitOutcome::Approved), + "somebody else executed it while our deadline denied — the money moved" + ); + assert_eq!( + terminal_exit( + &R::AlreadyResolved { + state: TerminalState::Expired + }, + false + ), + Some(ExitOutcome::Expired) + ); assert_eq!( terminal_exit( &R::AlreadyResolved { From a0c6d4da2a41c0e881a15a0fddfe592aa8f8d33b Mon Sep 17 00:00:00 2001 From: temrjan Date: Fri, 10 Jul 2026 09:45:21 +0500 Subject: [PATCH 07/11] fix(ui): risk warnings and PIN prompt can never leave the card The card rendered to/amount/chain_id/raw_data first and the warnings after, in one silently height-clipped Paragraph: a long calldata pushed HIGH RISK, UNLIMITED and the PIN prompt (dots included) off an 80x24 screen while Approve stayed live. Now every field except raw_data is priority and renders first, pre-wrapped to the inner width so the row budget is exact (unicode-width, the crate ratatui itself measures with). raw_data is the single elastic element: it takes the rows that remain and, when clipped, says so with an explicit marker (chars total / shown / not shown). If the priority fields alone overflow the card, raw_data degrades to a hidden-marker line - no panic. While a confirmation is open the queue collapses to its selected row so the card gets the height its warnings need on a 24-row terminal. --- Cargo.lock | 1 + Cargo.toml | 3 + src/ui.rs | 392 +++++++++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 356 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 61513fa..819080f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1101,6 +1101,7 @@ dependencies = [ "ratatui", "serde", "serde_json", + "unicode-width", "zeroize", ] diff --git a/Cargo.toml b/Cargo.toml index a586464..f185ba1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,9 @@ serde = { version = "1", features = ["derive"] } serde_json = "1" # PIN hygiene: the PIN and its serialized auth request live in Zeroizing buffers. zeroize = "1" +# Display-cell arithmetic for the card's height budget (ui::render_detail) — +# the same crate ratatui measures with, so our row math matches its renderer. +unicode-width = "0.2" [lints.rust] unsafe_code = "forbid" diff --git a/src/ui.rs b/src/ui.rs index e819b48..b96e9fc 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -103,10 +103,21 @@ fn render_watch( ) { // The note's row is claimed only when there is a note. An always-reserved row // would take its space from the card, and the card is the one thing on this - // screen that must never be silently clipped (`AGENTS.md` #1). + // screen whose priority fields must never leave the screen (`AGENTS.md` #1). + // + // While a confirmation is open the card is the decision surface: the queue + // collapses to a single-item strip (the List keeps the selection in view) + // and the card takes every remaining row. Splitting the height evenly would + // starve the card of the rows its risk warnings and PIN prompt need on a + // 24-row terminal. + let queue_rows = if confirm.is_some() { + Constraint::Length(3) // borders + the selected row + } else { + Constraint::Min(3) + }; let mut constraints = vec![ Constraint::Length(1), // header - Constraint::Min(3), // queue + queue_rows, // queue Constraint::Min(6), // card / hint Constraint::Length(1), // decision row / navigation hint ]; @@ -231,8 +242,17 @@ fn kind_word(s: &Summary) -> &'static str { } } -/// Render the open confirmation's card **verbatim** — the core's fields as -/// received, no re-derivation. `None` shows a hint to open one. +/// Render the open confirmation's card — the core's fields **verbatim**, no +/// re-derivation. `None` shows a hint to open one. +/// +/// Priority fields (everything except `raw_data`) render first, and +/// `raw_data` — the only elastic element — gets exactly the rows that remain, +/// truncated with an explicit marker when it cannot fit. A long calldata can +/// therefore never push a risk warning or the PIN prompt off the screen. That +/// guarantee is against `raw_data`: priority fields that alone overflow the +/// card (pathological server data) still clip at the bottom until scrolling +/// lands. Every line is pre-wrapped to the card's inner width so one logical +/// line is one visual row and the height budget is exact, not an estimate. fn render_detail(frame: &mut Frame, confirm: Option<&Confirm>, area: ratatui::layout::Rect) { let block = Block::bordered().title(" Card "); let Some(confirm) = confirm else { @@ -243,54 +263,78 @@ fn render_detail(frame: &mut Frame, confirm: Option<&Confirm>, area: ratatui::la }; let card: &Card = confirm.card(); - let mut lines = vec![ - kv("to", &card.to), - kv("amount_wei", &card.amount_wei), - kv("chain_id", &card.chain_id.to_string()), - kv("raw_data", &card.raw_data), - ]; + let inner = block.inner(area); + let width = usize::from(inner.width); + let height = usize::from(inner.height); + let bold = Style::new().add_modifier(Modifier::BOLD); + + let mut lines: Vec> = Vec::new(); + push_wrapped(&mut lines, width, format!("to: {}", card.to), Style::new()); + push_wrapped( + &mut lines, + width, + format!("amount_wei: {}", card.amount_wei), + Style::new(), + ); + push_wrapped( + &mut lines, + width, + format!("chain_id: {}", card.chain_id), + Style::new(), + ); if card.high_risk { - lines.push(Line::from(Span::styled( + push_wrapped( + &mut lines, + width, format!("HIGH RISK: {}", card.high_risk_reasons.join(", ")), - Style::new().add_modifier(Modifier::BOLD), - ))); + bold, + ); } match &card.decoded_call { None => lines.push(Line::from("decoded_call: (none)")), Some(dc) => { - lines.push(Line::from(format!("decoded_call.method: {}", dc.method))); - push_opt(&mut lines, "spender", dc.spender.as_deref()); - push_opt(&mut lines, "operator", dc.operator.as_deref()); - push_opt(&mut lines, "from", dc.from.as_deref()); - push_opt(&mut lines, "to", dc.to.as_deref()); - push_opt(&mut lines, "token", dc.token.as_deref()); - push_opt(&mut lines, "amount", dc.amount.as_deref()); - push_opt(&mut lines, "deadline", dc.deadline.as_deref()); + push_wrapped( + &mut lines, + width, + format!("decoded_call.method: {}", dc.method), + Style::new(), + ); + push_opt(&mut lines, width, "spender", dc.spender.as_deref()); + push_opt(&mut lines, width, "operator", dc.operator.as_deref()); + push_opt(&mut lines, width, "from", dc.from.as_deref()); + push_opt(&mut lines, width, "to", dc.to.as_deref()); + push_opt(&mut lines, width, "token", dc.token.as_deref()); + push_opt(&mut lines, width, "amount", dc.amount.as_deref()); + push_opt(&mut lines, width, "deadline", dc.deadline.as_deref()); if dc.is_unlimited == Some(true) { - lines.push(Line::from(Span::styled( - "amount: UNLIMITED", - Style::new().add_modifier(Modifier::BOLD), - ))); + push_wrapped(&mut lines, width, "amount: UNLIMITED".to_owned(), bold); } } } if let Some(pin_len) = confirm.pin_len() { lines.push(Line::from("")); - lines.push(Line::from("High-risk approval — enter your PIN:")); + push_wrapped( + &mut lines, + width, + "High-risk approval — enter your PIN:".to_owned(), + Style::new(), + ); // Only the count is shown — never the digits. - lines.push(Line::from(Span::styled( - "●".repeat(pin_len), - Style::new().add_modifier(Modifier::BOLD), - ))); + push_wrapped(&mut lines, width, "●".repeat(pin_len), bold); } if let Some(err) = confirm.error() { lines.push(Line::from("")); - lines.push(Line::from(resolve_error_text(err))); + push_wrapped(&mut lines, width, resolve_error_text(err), Style::new()); } - // Wrap (never truncate): a clear-signing card must show the whole value — - // a silently clipped raw_data or address would be the one lie this screen - // exists to prevent. trim: false keeps the exact bytes, including leading space. + // raw_data comes LAST and absorbs whatever rows the priority fields above + // left over. Never truncate priority fields; a truncated raw_data says so + // out loud — a silent clip would be the one lie this screen exists to + // prevent. Wrap stays on as a backstop only (every line already fits the + // width); trim: false keeps the exact bytes, including leading space. + let budget = height.saturating_sub(lines.len()); + push_raw_data(&mut lines, width, budget, &card.raw_data); + frame.render_widget( Paragraph::new(lines) .block(block) @@ -299,6 +343,106 @@ fn render_detail(frame: &mut Frame, confirm: Option<&Confirm>, area: ratatui::la ); } +/// Push `raw_data` into exactly `budget` visual rows. It fits → shown whole. +/// It does not → truncated with a marker naming how much is hidden; the marker +/// lives inside the same budget, so it can never spill onto the priority +/// fields. A zero budget (priority fields alone fill the card — an anomalously +/// long decoded_call) degrades to a marker-only line, clipped by ratatui if +/// even that row has no room; it never panics. +fn push_raw_data(lines: &mut Vec>, width: usize, budget: usize, raw: &str) { + use unicode_width::UnicodeWidthStr; + + let total = raw.chars().count(); + if budget == 0 { + push_wrapped( + lines, + width, + format!("raw_data: (hidden — card too small for {total} chars)"), + Style::new(), + ); + return; + } + let full_rows = chunk_display_width(&format!("raw_data: {raw}"), width); + if full_rows.len() <= budget { + lines.extend(full_rows.into_iter().map(Line::from)); + return; + } + + // Size the shown prefix by display cells, reserving room for the marker at + // its widest (both counters as wide as `total`). The digit widths change + // with `shown`, so verify by chunking and shrink a row's worth at a time — + // strictly downward, stopping at zero, where the marker alone is pushed. + let cells = budget.saturating_mul(width.max(1)); + let overhead = truncated_raw_line("", total, total).width(); + let mut shown = prefix_chars_for_cells(raw, cells.saturating_sub(overhead)); + loop { + let prefix: String = raw.chars().take(shown).collect(); + let candidate = truncated_raw_line(&prefix, shown, total); + if chunk_display_width(&candidate, width).len() <= budget || shown == 0 { + push_wrapped(lines, width, candidate, Style::new()); + return; + } + shown = shown.saturating_sub(width.max(1)); + } +} + +/// The truncated `raw_data` line: head of the value plus an explicit marker. +fn truncated_raw_line(prefix: &str, shown: usize, total: usize) -> String { + let hidden = total.saturating_sub(shown); + format!( + "raw_data: {prefix}… ({total} chars total, {shown} shown, {hidden} not shown — scroll not yet supported)" + ) +} + +/// How many leading `chars` of `s` fit within `cells` display cells. +fn prefix_chars_for_cells(s: &str, cells: usize) -> usize { + use unicode_width::UnicodeWidthChar; + + let mut used = 0; + let mut count = 0; + for ch in s.chars() { + used += ch.width().unwrap_or(0); + if used > cells { + break; + } + count += 1; + } + count +} + +/// Push `text` as one or more lines, each at most `width` display cells — the +/// pre-wrapping that keeps `render_detail`'s row arithmetic exact. +fn push_wrapped(lines: &mut Vec>, width: usize, text: String, style: Style) { + for chunk in chunk_display_width(&text, width) { + lines.push(Line::from(Span::styled(chunk, style))); + } +} + +/// Split `s` into chunks of at most `width` display cells, never inside a +/// `char`. Measured with the same unicode-width ratatui renders with, so a +/// chunk always fits one terminal row. +fn chunk_display_width(s: &str, width: usize) -> Vec { + use unicode_width::UnicodeWidthChar; + + let width = width.max(1); + let mut chunks = Vec::new(); + let mut current = String::new(); + let mut used = 0; + for ch in s.chars() { + let w = ch.width().unwrap_or(0); + if used + w > width && !current.is_empty() { + chunks.push(std::mem::take(&mut current)); + used = 0; + } + current.push(ch); + used += w; + } + if !current.is_empty() || chunks.is_empty() { + chunks.push(current); + } + chunks +} + fn resolve_error_text(err: &ResolveError) -> String { match err { ResolveError::PinRequired => "This approval needs your PIN.".to_owned(), @@ -310,13 +454,14 @@ fn resolve_error_text(err: &ResolveError) -> String { } } -fn kv(key: &str, value: &str) -> Line<'static> { - Line::from(format!("{key}: {value}")) -} - -fn push_opt(lines: &mut Vec>, key: &str, value: Option<&str>) { +fn push_opt(lines: &mut Vec>, width: usize, key: &str, value: Option<&str>) { if let Some(v) = value { - lines.push(Line::from(format!("decoded_call.{key}: {v}"))); + push_wrapped( + lines, + width, + format!("decoded_call.{key}: {v}"), + Style::new(), + ); } } @@ -620,6 +765,173 @@ mod tests { ); } + /// The Stage-1 repro card: high-risk unlimited `approve` whose calldata used + /// to push every warning off an 80×24 screen. + fn risk_card(id: &str, raw_data: String) -> Box { + Box::new(Card { + id: id.to_owned(), + chain_id: 1, + to: "0x742d35Cc6634C0532925a3b844Bc454e4438f44e".to_owned(), + amount_wei: "0".to_owned(), + decoded_call: Some(DecodedCall { + method: "approve".to_owned(), + spender: Some("0xdeadbeef".to_owned()), + operator: None, + from: None, + to: None, + token: None, + amount: Some("0xffffffffffffffff".to_owned()), + deadline: None, + approved: None, + is_unlimited: Some(true), + }), + high_risk: true, + high_risk_reasons: vec!["unlimited_approval".to_owned()], + raw_data, + not_after_unix: NOW + 27, + }) + } + + /// Drive the model to an open confirmation on `risk_card`. + fn open_risk_card(model: &mut Model, raw_data: String) { + to_watching(model, vec![summary("a1", "0xabc", "0", true)]); + model.update(Msg::Open); + model.update(Msg::Reply(Reply::Get(crate::protocol::GetOutcome::Card( + risk_card("a1", raw_data), + )))); + } + + /// The first row index containing `needle` — for order checks. + fn row_of(rows: &[String], needle: &str) -> usize { + rows.iter() + .position(|r| r.contains(needle)) + .unwrap_or_else(|| panic!("no row contains {needle:?}")) + } + + /// 328-char calldata, as measured in the Stage-1 repro. + fn stage1_raw_data() -> String { + let raw = format!("0x{}", "ab".repeat(163)); + assert_eq!(raw.chars().count(), 328); + raw + } + + #[test] + fn chunking_respects_display_width_and_never_splits_a_char() { + // Exact multiples: no empty trailing chunk. + assert_eq!(chunk_display_width("abcdef", 3), vec!["abc", "def"]); + // A 2-cell char that does not fit the remaining cell starts a new chunk. + assert_eq!(chunk_display_width("ab漢", 3), vec!["ab", "漢"]); + // Empty input still claims one (blank) row. + assert_eq!(chunk_display_width("", 5), vec![""]); + } + + #[test] + fn every_warning_and_the_pin_prompt_stay_on_screen_with_a_long_raw_data() { + let mut m = Model::new(); + open_risk_card(&mut m, stage1_raw_data()); + m.update(Msg::Approve); // high risk: opens the PIN prompt + m.update(Msg::PinDigit('7')); + + let rows = draw_rows(&m, 80, 24); + let screen = rows.join("\n"); + + assert!( + screen.contains("HIGH RISK"), + "the risk warning must never leave the screen" + ); + assert!( + screen.contains("UNLIMITED"), + "the unlimited-amount warning must never leave the screen" + ); + assert!( + screen.contains("enter your PIN"), + "the PIN prompt must never leave the screen" + ); + assert!( + screen.contains("●"), + "the PIN dots must be visible — a blind PIN entry is not an entry" + ); + // raw_data is the one elastic element, so it renders BELOW every warning. + assert!( + row_of(&rows, "HIGH RISK") < row_of(&rows, "raw_data"), + "raw_data must render below the risk warning, never above it" + ); + } + + #[test] + fn high_risk_and_unlimited_stay_on_screen_with_a_long_raw_data_without_pin() { + let mut m = Model::new(); + open_risk_card(&mut m, stage1_raw_data()); + + let screen = draw(&m, 80, 24); + + assert!(screen.contains("HIGH RISK")); + assert!(screen.contains("UNLIMITED")); + } + + #[test] + fn a_short_raw_data_still_renders_whole_with_no_truncation_marker() { + let mut m = Model::new(); + open_risk_card(&mut m, "0x095ea7b3deadbeef".to_owned()); + + let rows = draw_rows(&m, 80, 24); + + assert!( + has_line_with(&rows, &["raw_data: 0x095ea7b3deadbeef"]), + "a raw_data that fits renders whole, exactly as received" + ); + assert!( + !rows.join("\n").contains("not shown"), + "no truncation marker when nothing was truncated" + ); + } + + #[test] + fn an_overlong_raw_data_is_truncated_with_an_explicit_marker_not_silently() { + let mut m = Model::new(); + open_risk_card(&mut m, format!("0x{}", "ab".repeat(1000))); + + let rows = draw_rows(&m, 80, 24); + let screen = rows.join("\n"); + + assert!( + has_line_with(&rows, &["raw_data: 0xabab"]), + "the head of raw_data is still shown" + ); + assert!( + screen.contains("not shown"), + "a clipped raw_data must say so out loud, never trail off silently" + ); + + // The marker's numbers are the honesty of this screen: they must name + // the real payload and account for every char of it. + let marker_row = rows + .iter() + .find(|r| r.contains("not shown")) + .expect("the truncation marker renders"); + let nums: Vec = marker_row + .split(|c: char| !c.is_ascii_digit()) + .filter(|s| !s.is_empty()) + .map(|s| s.parse().unwrap()) + .collect(); + assert_eq!( + nums.len(), + 3, + "total, shown and not-shown counters: {marker_row}" + ); + assert_eq!(nums[0], 2002, "the total names the real payload size"); + assert_eq!( + nums[1] + nums[2], + nums[0], + "shown + not shown must account for every char: {marker_row}" + ); + assert!( + nums[1] < nums[2], + "a 2002-char payload on 24 rows is mostly hidden — the shown and \ + not-shown counters look swapped: {marker_row}" + ); + } + #[test] fn the_resolved_screen_names_the_outcome_and_shows_the_tx_hash() { let mut m = Model::new(); From 7183daf7827f97cad30f324844088d7b21d5648d Mon Sep 17 00:00:00 2001 From: temrjan Date: Fri, 10 Jul 2026 10:15:30 +0500 Subject: [PATCH 08/11] fix(app): a confirmation can never be rebound to another card apply_get replaced the open Confirm unconditionally, resolving flag and all: with one get on the wire and a second one parked, the late card landed after the human pressed approve, rebound the confirmation, reset resolving - and the sent decision now pointed at a card the human never opened, with the buttons live for a second decision. Deterministic repro via Model::update, no timing involved. Three layers, all fail-closed: - apply_get never touches an open confirmation (an open card IS the confirmation, AGENTS.md #5); a get outcome arriving while one is open is stale or unsolicited and is dropped whole. - awaiting_card: only one get may be on the wire; on_open refuses (does not park) while one is - both for a directly sent get and for one the reply flushes out of the park. Cleared when the get is answered, and on a handshake reset so the flag cannot brick opening. - The regression tests replay the incident key press by key press, plus the unsolicited-reply and flag-lifecycle edges. --- src/app.rs | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 151 insertions(+), 5 deletions(-) diff --git a/src/app.rs b/src/app.rs index e60f3e0..cd911b5 100644 --- a/src/app.rs +++ b/src/app.rs @@ -281,6 +281,10 @@ pub struct Model { phase: Phase, in_flight: bool, pending: Option, + /// A `get` has been **sent** and its answer is still pending. While set, + /// [`Self::on_open`] refuses new card requests: two card requests racing is + /// how a confirmation could be rebound to a card the human never opened. + awaiting_card: bool, quit: bool, } @@ -316,6 +320,7 @@ impl Default for Model { phase: Phase::Connecting, in_flight: false, pending: None, + awaiting_card: false, quit: false, } } @@ -516,12 +521,22 @@ impl Model { if confirm.is_some() { return None; // already deciding one } + if self.awaiting_card { + // A card request is already on the wire. Refused, NOT parked: with + // two gets racing, the late answer would land after the first card + // opened — and a confirmation must never be rebound (`apply_get`). + return None; + } let Some(id) = items.get(*selected).map(|s| s.id.clone()) else { return None; // empty queue }; - self.dispatch_user(PendingIntent::Get(id.clone()), || { + let request = self.dispatch_user(PendingIntent::Get(id.clone()), || { transport::Request::Get(id) - }) + }); + // Parked gets are covered too: `flush_pending` raises the flag when the + // park is sent, and latest-wins can only swap one park for another. + self.awaiting_card = request.is_some(); + request } /// Send a user intent now if idle, else park it (latest-wins) so it is not lost. @@ -559,6 +574,9 @@ impl Model { pin: Pin::default(), error: None, }; + // A handshake resets the protocol: an outstanding get will never + // be answered, and a stuck flag would refuse every future open. + self.awaiting_card = false; } Reply::Auth(outcome) => self.apply_auth(outcome), Reply::List(items) => self.apply_list(items), @@ -620,7 +638,18 @@ impl Model { } fn apply_get(&mut self, outcome: GetOutcome) { + // Whatever this outcome says, it answers the one get that was allowed + // on the wire — the console may ask for a card again. + self.awaiting_card = false; if let Phase::Watching { confirm, note, .. } = &mut self.phase { + if confirm.is_some() { + // An open card *is* the confirmation (`AGENTS.md` #5): no reply + // may replace it — least of all mid-resolve, where a swap would + // re-aim the human's decision at a card they never opened. No + // get is ever solicited while a card is open (`on_open`), so + // this outcome is stale or unsolicited: dropped, fail closed. + return; + } match outcome { GetOutcome::Card(c) => { // An open card *is* the confirmation (AGENTS.md #5). @@ -628,9 +657,8 @@ impl Model { *note = None; } GetOutcome::UnknownId => { - // The selected item vanished between list and get — drop any - // stale card and show a transient note instead of a dead card. - *confirm = None; + // The selected item vanished between list and get — show a + // transient note instead of a dead card. *note = Some("that request is no longer available".to_owned()); } } @@ -713,6 +741,7 @@ impl Model { match self.pending.take() { Some(PendingIntent::Get(id)) => { self.in_flight = true; + self.awaiting_card = true; Some(transport::Request::Get(id)) } Some(PendingIntent::Approve(id)) => { @@ -979,6 +1008,123 @@ mod tests { assert!(matches!(flushed, Some(transport::Request::Get(id)) if id == "b")); } + // ── one decision, one card: a confirmation can never be rebound ── + + #[test] + fn a_decision_can_never_be_rebound_to_a_card_the_human_did_not_open() { + // The incident, key press by key press — no timing involved. + let mut m = watching(vec![summary("a"), summary("b")]); + // Open(a): the get flies at once. + assert!(matches!( + m.update(Msg::Open), + Some(transport::Request::Get(id)) if id == "a" + )); + // Open(b) while that get is on the wire: refused outright, NOT parked — + // two card requests racing is how a decision gets rebound. + m.update(Msg::MoveDown); + assert!(m.update(Msg::Open).is_none()); + // Card a lands and binds the confirmation; nothing parked may chase it. + let flushed = m.update(Msg::Reply(Reply::Get(GetOutcome::Card(card("a"))))); + assert!( + flushed.is_none(), + "a second get must never chase an open card" + ); + // `y` — the approve goes out for a, the card on the screen. + assert!(matches!( + m.update(Msg::Approve), + Some(transport::Request::Approve(id)) if id == "a" + )); + // A stale card b arrives while the decision is on the wire. + assert!( + m.update(Msg::Reply(Reply::Get(GetOutcome::Card(card("b"))))) + .is_none() + ); + let c = confirm_of(&m); + assert_eq!( + c.card().id, + "a", + "the confirmation stays bound to the card the human saw" + ); + assert!( + c.is_resolving(), + "the in-flight decision survives a stale card reply" + ); + // A second `y` cannot become a second decision. + assert!( + m.update(Msg::Approve).is_none(), + "one session, one decision" + ); + } + + #[test] + fn an_unsolicited_card_reply_never_replaces_an_open_confirmation() { + let mut m = confirming("a", false); + assert!( + m.update(Msg::Reply(Reply::Get(GetOutcome::Card(card("b"))))) + .is_none() + ); + assert_eq!( + confirm_of(&m).card().id, + "a", + "an open card IS the confirmation — no reply may swap it out" + ); + } + + #[test] + fn an_unsolicited_unknown_id_never_clears_an_open_confirmation() { + let mut m = confirming("a", false); + m.update(Msg::Approve); // the decision goes on the wire + assert!( + m.update(Msg::Reply(Reply::Get(GetOutcome::UnknownId))) + .is_none() + ); + let c = confirm_of(&m); + assert_eq!(c.card().id, "a"); + assert!( + c.is_resolving(), + "the resolve answer must still find the confirmation it belongs to" + ); + } + + #[test] + fn a_get_sent_from_the_park_also_locks_out_further_opens() { + let mut m = watching(vec![summary("a"), summary("b")]); + assert!(matches!( + m.update(Msg::Tick), + Some(transport::Request::List) + )); + m.update(Msg::Open); // parked behind the poll + let flushed = m.update(Msg::Reply(Reply::List(vec![summary("a"), summary("b")]))); + assert!(matches!(flushed, Some(transport::Request::Get(id)) if id == "a")); + // The parked get is on the wire now — a second open is refused… + m.update(Msg::MoveDown); + assert!(m.update(Msg::Open).is_none()); + // …and nothing chases the card when it lands. + assert!( + m.update(Msg::Reply(Reply::Get(GetOutcome::Card(card("a"))))) + .is_none() + ); + assert_eq!(confirm_of(&m).card().id, "a"); + } + + #[test] + fn a_vanished_card_frees_the_console_to_open_another() { + let mut m = watching(vec![summary("a"), summary("b")]); + assert!(matches!( + m.update(Msg::Open), + Some(transport::Request::Get(id)) if id == "a" + )); + m.update(Msg::Reply(Reply::Get(GetOutcome::UnknownId))); + m.update(Msg::MoveDown); + assert!( + matches!( + m.update(Msg::Open), + Some(transport::Request::Get(id)) if id == "b" + ), + "once the get is answered the console can open the next card" + ); + } + // ── watch behaviour ── #[test] From aee86de9456e1a3899e806844e47e40e46a5905c Mon Sep 17 00:00:00 2001 From: temrjan Date: Fri, 10 Jul 2026 10:50:31 +0500 Subject: [PATCH 09/11] fix(protocol,app): a deny answered with a PIN ask kills the channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parse_deny piped every error reply through the resolve_error table it shares with approve, so a pin_required answer to a deny opened the PIN prompt over the human's rejection - and the prompt can only build an approve line: the no came back as a signature. One buggy or version-skewed core away, no hostile intent needed. The canon already forbade it (APPROVER-PROTOCOL §3.6: deny's only errors are unauthorized/unknown_id/already_resolved; deny never requires a PIN beyond session auth). Two layers, both fail-closed like the neighbouring Unauthorized arm: - parse_deny accepts exactly the documented surface; anything else, the PIN family first of all, is ProtocolError::Unexpected -> Fatal. - Confirm now records WHICH decision went on the wire (SentDecision replaces the resolving bool), and apply_resolve refuses the channel when a PIN-family answer arrives for anything but a sent approve - depth for replies that bypass parse_deny. Regression tests replay the incident (reject -> pin_required -> typed PIN -> Enter emits nothing) for every PIN-family member, and pin parse_deny's accepted/refused surfaces. --- src/app.rs | 93 ++++++++++++++++++++++++++++++++++++++++--------- src/protocol.rs | 40 +++++++++++++++++++-- 2 files changed, 113 insertions(+), 20 deletions(-) diff --git a/src/app.rs b/src/app.rs index cd911b5..50996c1 100644 --- a/src/app.rs +++ b/src/app.rs @@ -155,20 +155,30 @@ pub struct Confirm { pin: Option, /// The last non-terminal failure to show. error: Option, - /// A decision is on the wire — further key presses are ignored so a second - /// press cannot become a second decision. - resolving: bool, + /// The decision on the wire, while one is — further key presses are ignored + /// so a second press cannot become a second decision. *Which* decision it + /// was matters too: a PIN-family answer is only ever legal for an approve + /// (§3.6), and `apply_resolve` refuses the channel when it arrives for + /// anything else — a "no" must never grow a PIN prompt. + sent: Option, /// The `deny` was sent by the expiry deadline, not by the human. timed_out: bool, } +/// The decision the console put on the wire for the open card. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SentDecision { + Approve, + Deny, +} + impl Confirm { fn new(card: Card) -> Self { Self { card, pin: None, error: None, - resolving: false, + sent: None, timed_out: false, } } @@ -194,7 +204,7 @@ impl Confirm { /// Whether a decision is currently on the wire. #[must_use] pub fn is_resolving(&self) -> bool { - self.resolving + self.sent.is_some() } } @@ -397,7 +407,7 @@ impl Model { Phase::Authing { pin, .. } => Some(pin), Phase::Watching { confirm: Some(c), .. - } if !c.resolving => c.pin.as_mut(), + } if c.sent.is_none() => c.pin.as_mut(), _ => None, } } @@ -441,7 +451,7 @@ impl Model { else { return None; }; - if c.resolving { + if c.sent.is_some() { return None; } let Some(pin) = &c.pin else { @@ -457,7 +467,7 @@ impl Model { )); return None; }; - c.resolving = true; + c.sent = Some(SentDecision::Approve); c.error = None; self.dispatch_pin_approve(line) } @@ -471,7 +481,7 @@ impl Model { else { return None; }; - if c.resolving || c.pin.is_some() { + if c.sent.is_some() || c.pin.is_some() { return None; // decided already, or the PIN prompt owns Enter } if c.card.high_risk { @@ -479,7 +489,7 @@ impl Model { c.error = None; return None; } - c.resolving = true; + c.sent = Some(SentDecision::Approve); c.error = None; let id = c.card.id.clone(); self.dispatch_user(PendingIntent::Approve(id.clone()), || { @@ -496,10 +506,10 @@ impl Model { else { return None; }; - if c.resolving { + if c.sent.is_some() { return None; // a decision is already on the wire; the server decides } - c.resolving = true; + c.sent = Some(SentDecision::Deny); c.timed_out = by_timeout; c.error = None; let id = c.card.id.clone(); @@ -669,13 +679,10 @@ impl Model { /// matching [`ExitOutcome`]; the rest leave the item live and the human in /// charge (`pin_required` opens the PIN prompt, `bad_pin` clears it, and so on). fn apply_resolve(&mut self, outcome: ResolveOutcome) { - let timed_out = match &mut self.phase { + let (sent, timed_out) = match &mut self.phase { Phase::Watching { confirm: Some(c), .. - } => { - c.resolving = false; - c.timed_out - } + } => (c.sent.take(), c.timed_out), // No confirmation is open — an answer to a decision we never made. _ => return, }; @@ -695,6 +702,27 @@ impl Model { return; } + let pin_flavoured = matches!( + outcome, + ResolveOutcome::PinRequired + | ResolveOutcome::BadPin { .. } + | ResolveOutcome::Locked { .. } + | ResolveOutcome::PinNotSet + | ResolveOutcome::PinUnavailable + ); + if pin_flavoured && sent != Some(SentDecision::Approve) { + // A PIN-family answer exists only for `approve` (§3.6: "deny never + // requires a PIN beyond session auth"). Arriving for a deny — or for + // nothing we sent — it would open the PIN prompt over a rejection, + // and the prompt can only build an approve line: the human's "no" + // would come back as a signature. Same fail-closed as Unauthorized; + // `parse_deny` already refuses this at the wire, this is depth. + self.phase = Phase::Fatal(TransportError::Protocol( + "server asked for a PIN outside an approve (§3.6)".to_owned(), + )); + return; + } + let Phase::Watching { confirm, note, .. } = &mut self.phase else { return; }; @@ -1086,6 +1114,37 @@ mod tests { ); } + #[test] + fn a_pin_answer_to_a_deny_can_never_turn_the_no_into_a_signature() { + // The incident: the human said no, a skewed/hostile core answered the + // deny with a PIN-family error — the console must refuse the channel, + // never open the PIN prompt over a rejection. Every family member. + for outcome in [ + ResolveOutcome::PinRequired, + ResolveOutcome::BadPin { attempts_left: 2 }, + ResolveOutcome::Locked { retry_after_s: 30 }, + ResolveOutcome::PinNotSet, + ResolveOutcome::PinUnavailable, + ] { + let mut m = confirming("a", false); + assert!(matches!( + m.update(Msg::Reject), + Some(transport::Request::Deny(id)) if id == "a" + )); + assert!(m.update(Msg::Reply(Reply::Resolve(outcome))).is_none()); + assert!( + matches!(m.phase(), Phase::Fatal(_)), + "a PIN ask on a deny is not the channel we think it is (§3.6)" + ); + // The rest of the incident's key presses must be dead ends. + type_pin(&mut m, "42"); + assert!( + m.update(Msg::PinSubmit).is_none(), + "no approve line may ever follow a rejection" + ); + } + } + #[test] fn a_get_sent_from_the_park_also_locks_out_further_opens() { let mut m = watching(vec![summary("a"), summary("b")]); diff --git a/src/protocol.rs b/src/protocol.rs index c9f29f8..60b8799 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -494,7 +494,11 @@ pub fn parse_approve(line: &str) -> Result { /// /// # Errors /// [`ProtocolError::Malformed`] on non-JSON; [`ProtocolError::Unexpected`] on an -/// `ok` reply that is not `denied`, or an unmodeled error code. +/// `ok` reply that is not `denied`, or any error outside `deny`'s documented +/// surface (§3.6): `unauthorized` / `unknown_id` / `already_resolved`. The PIN +/// family in particular is refused — "deny never requires a PIN beyond session +/// auth" — because accepted, it would open the PIN prompt over a rejection and +/// the prompt can only build an approve line. pub fn parse_deny(line: &str) -> Result { let raw: ResolveRaw = parse_line(line)?; if raw.ok { @@ -505,7 +509,13 @@ pub fn parse_deny(line: &str) -> Result { ))), } } else { - resolve_error(&raw) + match raw.error.as_deref() { + Some("unauthorized" | "unknown_id" | "already_resolved") => resolve_error(&raw), + other => Err(ProtocolError::Unexpected(format!( + "deny answered with error {other:?} — its only errors are \ + unauthorized/unknown_id/already_resolved (§3.6)" + ))), + } } } @@ -816,11 +826,16 @@ mod tests { } #[test] - fn parse_deny_shares_the_error_codes() { + fn parse_deny_accepts_exactly_its_documented_errors() { + // §3.6: unauthorized / unknown_id / already_resolved — and nothing else. assert_eq!( parse_deny(r#"{"ok":false,"error":"unauthorized"}"#).unwrap(), ResolveOutcome::Unauthorized ); + assert_eq!( + parse_deny(r#"{"ok":false,"error":"unknown_id"}"#).unwrap(), + ResolveOutcome::UnknownId + ); assert_eq!( parse_deny(r#"{"ok":false,"error":"already_resolved","state":"executed"}"#).unwrap(), ResolveOutcome::AlreadyResolved { @@ -828,4 +843,23 @@ mod tests { } ); } + + #[test] + fn parse_deny_refuses_the_whole_pin_family() { + // §3.6: "deny never requires a PIN beyond session auth". A PIN-family + // answer to a deny would flow into the PIN prompt and turn the human's + // "no" into an approve line — it must kill the channel instead. + for line in [ + r#"{"ok":false,"error":"pin_required"}"#, + r#"{"ok":false,"error":"bad_pin","attempts_left":2}"#, + r#"{"ok":false,"error":"locked","retry_after_s":30}"#, + r#"{"ok":false,"error":"pin_not_set"}"#, + r#"{"ok":false,"error":"pin_unavailable"}"#, + ] { + assert!( + matches!(parse_deny(line), Err(ProtocolError::Unexpected(_))), + "deny must never accept: {line}" + ); + } + } } From c3e2da800fe1e98afc70f55dfa3795481238f1d7 Mon Sep 17 00:00:00 2001 From: temrjan Date: Fri, 10 Jul 2026 10:57:45 +0500 Subject: [PATCH 10/11] fix(ui,app,main): approve dies when the card cannot show its fields Below the card's needed height (an ordinary tmux pane, not just pathological data) the priority fields clipped silently from the bottom - UNLIMITED, the PIN prompt, the dots - while Approve stayed live and a PIN typed blind still signed. Measured at 80x14: HIGH RISK visible, everything after it gone, ApprovePin emitted. Now the gate follows the terminal, fail-closed at every seam: - ui::priority_fields_fit runs the SAME layout (watch_chunks) and the SAME pre-wrapped lines (priority_lines, extracted from render_detail) as the renderer, so the gate, the banner and the pulled button can never disagree. - The model learns the size via Msg::Resize (sent at startup and on every resize; never sent -> viewport stays zero and nothing fits) and refuses y and PIN submits while the card cannot show every priority field. Reject works at any size (AGENTS.md #5). - The card wears a bold TERMINAL TOO SMALL banner as its first row - the one row guaranteed visible - and the decision row drops the Approve button, naming the reason in its place. Tests: the 80x14 gate (y dead, no PIN prompt, deny alive), shrink mid-prompt kills the submit, regrowth re-arms, unknown size fails closed; the banner and pulled button are pinned at the render layer. --- src/app.rs | 165 ++++++++++++++++++++++++++++++++++++++ src/main.rs | 41 ++++++++-- src/ui.rs | 224 +++++++++++++++++++++++++++++++++++++++------------- 3 files changed, 366 insertions(+), 64 deletions(-) diff --git a/src/app.rs b/src/app.rs index 50996c1..6675767 100644 --- a/src/app.rs +++ b/src/app.rs @@ -16,6 +16,7 @@ use zeroize::Zeroizing; use crate::protocol::{AuthOutcome, Card, GetOutcome, ResolveOutcome, Summary, TerminalState}; use crate::transport::{self, Reply, TransportError}; +use crate::ui; /// The approval PIN as it is typed. Zeroized on drop (via [`Zeroizing`]) and /// **redacted in `Debug`** so it never lands in a log line, a panic message, or a @@ -281,6 +282,14 @@ pub enum Msg { Reject, /// The open card's deadline passed: deny it fail-closed, and report `expired`. Expire, + /// The terminal was resized (or its size first learned). The model gates + /// approval on the card being fully readable at this size. + Resize { + /// Terminal width, in columns. + width: u16, + /// Terminal height, in rows. + height: u16, + }, /// Quit. Quit, } @@ -295,6 +304,9 @@ pub struct Model { /// [`Self::on_open`] refuses new card requests: two card requests racing is /// how a confirmation could be rebound to a card the human never opened. awaiting_card: bool, + /// Last known terminal size, fed by [`Msg::Resize`]. Starts at zero — until + /// the size is known, nothing fits and approval is refused (fail closed). + viewport: (u16, u16), quit: bool, } @@ -331,6 +343,7 @@ impl Default for Model { in_flight: false, pending: None, awaiting_card: false, + viewport: (0, 0), quit: false, } } @@ -395,6 +408,10 @@ impl Model { } None } + Msg::Resize { width, height } => { + self.viewport = (width, height); + None + } Msg::Open => self.on_open(), } } @@ -454,6 +471,12 @@ impl Model { if c.sent.is_some() { return None; } + if !ui::priority_fields_fit(c, self.viewport.0, self.viewport.1) { + // The prompt (or a warning above it) is off-screen: a PIN typed + // into a card the human cannot read must not sign (the card shows + // the TOO SMALL banner). Esc still rejects. + return None; + } let Some(pin) = &c.pin else { return None; // the prompt is not up: `y` opens it }; @@ -484,6 +507,13 @@ impl Model { if c.sent.is_some() || c.pin.is_some() { return None; // decided already, or the PIN prompt owns Enter } + if !ui::priority_fields_fit(c, self.viewport.0, self.viewport.1) { + // A "yes" to a card the human could not read is not a decision: + // while the priority fields do not fit the terminal, `y` is dead — + // it neither approves nor opens the PIN prompt. The card shows the + // TOO SMALL banner; reject stays available (AGENTS.md #5). + return None; + } if c.card.high_risk { c.pin = Some(Pin::default()); c.error = None; @@ -909,6 +939,11 @@ mod tests { /// Drive a model to the watch phase with the given items. fn watching(items: Vec) -> Model { let mut m = Model::new(); + // The size report main sends at startup — a standard 80×24 terminal. + m.update(Msg::Resize { + width: 80, + height: 24, + }); assert!( m.update(Msg::Reply(Reply::Hello { server: "s".to_owned() @@ -1184,6 +1219,136 @@ mod tests { ); } + // ── the approve gate: no "yes" to a card the human cannot read ── + + /// A card whose priority fields need more rows than an 80×14 terminal's + /// card area can show (8 priority lines vs 7 inner rows). + fn tall_card(id: &str) -> Box { + Box::new(Card { + id: id.to_owned(), + chain_id: 1, + to: "0xabc".to_owned(), + amount_wei: "0".to_owned(), + decoded_call: Some(crate::protocol::DecodedCall { + method: "approve".to_owned(), + spender: Some("0xdeadbeef".to_owned()), + operator: None, + from: None, + to: None, + token: None, + amount: Some("0xffffffffffffffff".to_owned()), + deadline: None, + approved: None, + is_unlimited: Some(true), + }), + high_risk: true, + high_risk_reasons: vec!["unlimited_approval".to_owned()], + raw_data: "0x".to_owned(), + not_after_unix: 1, + }) + } + + fn confirming_tall(id: &str) -> Model { + let mut m = watching(vec![summary(id)]); + assert!(matches!( + m.update(Msg::Open), + Some(transport::Request::Get(_)) + )); + m.update(Msg::Reply(Reply::Get(GetOutcome::Card(tall_card(id))))); + m + } + + #[test] + fn approve_is_dead_while_the_priority_fields_cannot_fit_the_terminal() { + let mut m = confirming_tall("a"); + m.update(Msg::Resize { + width: 80, + height: 14, + }); + + assert!(m.update(Msg::Approve).is_none()); + let c = confirm_of(&m); + assert!( + !c.is_resolving(), + "no decision may leave a card the human cannot read" + ); + assert!( + c.pin_len().is_none(), + "the PIN prompt must not open over a mutilated card" + ); + // Default-deny survives at any size (AGENTS.md #5). + assert!(matches!( + m.update(Msg::Reject), + Some(transport::Request::Deny(id)) if id == "a" + )); + } + + #[test] + fn a_taller_terminal_re_arms_approve() { + let mut m = confirming_tall("a"); + m.update(Msg::Resize { + width: 80, + height: 14, + }); + assert!(m.update(Msg::Approve).is_none()); + assert!(confirm_of(&m).pin_len().is_none()); + + m.update(Msg::Resize { + width: 80, + height: 24, + }); + // High-risk: `y` now opens the PIN prompt again. + assert!(m.update(Msg::Approve).is_none()); + assert!(confirm_of(&m).pin_len().is_some()); + } + + #[test] + fn pin_submit_is_dead_while_the_prompt_is_off_screen() { + let mut m = confirming_tall("a"); + m.update(Msg::Approve); // 80×24: opens the PIN prompt + assert!(confirm_of(&m).pin_len().is_some()); + type_pin(&mut m, "42"); + + // The terminal shrinks mid-prompt: the submit must die with the view. + m.update(Msg::Resize { + width: 80, + height: 14, + }); + assert!( + m.update(Msg::PinSubmit).is_none(), + "a PIN typed into an unreadable card must not sign" + ); + assert!(!confirm_of(&m).is_resolving()); + // Esc still rejects (AGENTS.md #5). + assert!(matches!( + m.update(Msg::Reject), + Some(transport::Request::Deny(_)) + )); + } + + #[test] + fn an_unknown_terminal_size_fails_closed_for_approve_only() { + // No Msg::Resize ever arrived (terminal.size() failed at startup). + let mut m = Model::new(); + m.update(Msg::Reply(Reply::Hello { + server: "s".to_owned(), + })); + m.update(Msg::PinDigit('1')); + m.update(Msg::PinSubmit); + m.update(Msg::Reply(Reply::Auth(AuthOutcome::Ok))); + m.update(Msg::Tick); + m.update(Msg::Reply(Reply::List(vec![summary("a")]))); + m.update(Msg::Open); + m.update(Msg::Reply(Reply::Get(GetOutcome::Card(card("a"))))); + + assert!(m.update(Msg::Approve).is_none()); + assert!(!confirm_of(&m).is_resolving()); + assert!(matches!( + m.update(Msg::Reject), + Some(transport::Request::Deny(_)) + )); + } + // ── watch behaviour ── #[test] diff --git a/src/main.rs b/src/main.rs index 1144c75..067c295 100644 --- a/src/main.rs +++ b/src/main.rs @@ -122,6 +122,18 @@ fn run(mut terminal: Tui, transport: &Transport) -> (u8, Option) { let mut model = Model::new(); let mut last_tick = Instant::now(); + // Report the terminal size before anything else: the model approves only a + // card it knows to be fully readable, so an unknown size (this call + // failing) leaves approval gated off — fail closed. + if let Ok(size) = terminal.size() + && let Some(req) = model.update(Msg::Resize { + width: size.width, + height: size.height, + }) + { + transport.send(req); + } + loop { if terminal .draw(|f| ui::render(f, &model, now_unix())) @@ -159,15 +171,23 @@ fn run(mut terminal: Tui, transport: &Transport) -> (u8, Option) { // and the deadline above is noticed within a frame of passing. let timeout = POLL.saturating_sub(last_tick.elapsed()).min(FRAME); match event::poll(timeout) { - Ok(true) => { - if let Ok(Event::Key(key)) = event::read() - && key.kind == KeyEventKind::Press - && let Some(msg) = map_key(&key, model.phase()) - && let Some(req) = model.update(msg) - { - transport.send(req); + Ok(true) => match event::read() { + Ok(Event::Key(key)) if key.kind == KeyEventKind::Press => { + if let Some(msg) = map_key(&key, model.phase()) + && let Some(req) = model.update(msg) + { + transport.send(req); + } } - } + // The approve gate follows the terminal: shrink below the card's + // priority fields and `y` dies, grow back and it re-arms. + Ok(Event::Resize(width, height)) => { + if let Some(req) = model.update(Msg::Resize { width, height }) { + transport.send(req); + } + } + _ => {} + }, Ok(false) => {} Err(_) => return (EXIT_FATAL, None), } @@ -360,6 +380,11 @@ mod tests { /// outside the crate, and the same path the real loop takes. fn confirming_at(high_risk: bool, not_after_unix: u64) -> Model { let mut m = Model::new(); + // The size report main sends at startup — a standard 80×24 terminal. + m.update(Msg::Resize { + width: 80, + height: 24, + }); m.update(Msg::Reply(Reply::Hello { server: "s".to_owned(), })); diff --git a/src/ui.rs b/src/ui.rs index b96e9fc..dfff468 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -93,24 +93,25 @@ fn auth_error_text(err: &AuthError) -> String { } } -fn render_watch( - frame: &mut Frame, - items: &[Summary], - selected: usize, - confirm: Option<&Confirm>, - note: Option<&str>, - now_unix: u64, -) { - // The note's row is claimed only when there is a note. An always-reserved row - // would take its space from the card, and the card is the one thing on this - // screen whose priority fields must never leave the screen (`AGENTS.md` #1). - // - // While a confirmation is open the card is the decision surface: the queue - // collapses to a single-item strip (the List keeps the selection in view) - // and the card takes every remaining row. Splitting the height evenly would - // starve the card of the rows its risk warnings and PIN prompt need on a - // 24-row terminal. - let queue_rows = if confirm.is_some() { +/// Split the watch screen. One function for the renderer AND for +/// [`priority_fields_fit`], so the approve gate can never disagree with the +/// layout actually drawn. +/// +/// The note's row is claimed only when there is a note. An always-reserved row +/// would take its space from the card, and the card is the one thing on this +/// screen whose priority fields must never leave the screen (`AGENTS.md` #1). +/// +/// While a confirmation is open the card is the decision surface: the queue +/// collapses to a single-item strip (the List keeps the selection in view) +/// and the card takes every remaining row. Splitting the height evenly would +/// starve the card of the rows its risk warnings and PIN prompt need on a +/// 24-row terminal. +fn watch_chunks( + area: ratatui::layout::Rect, + confirm_open: bool, + has_note: bool, +) -> std::rc::Rc<[ratatui::layout::Rect]> { + let queue_rows = if confirm_open { Constraint::Length(3) // borders + the selected row } else { Constraint::Min(3) @@ -121,10 +122,21 @@ fn render_watch( Constraint::Min(6), // card / hint Constraint::Length(1), // decision row / navigation hint ]; - if note.is_some() { + if has_note { constraints.push(Constraint::Length(1)); // transient note } - let chunks = Layout::vertical(constraints).split(frame.area()); + Layout::vertical(constraints).split(area) +} + +fn render_watch( + frame: &mut Frame, + items: &[Summary], + selected: usize, + confirm: Option<&Confirm>, + note: Option<&str>, + now_unix: u64, +) { + let chunks = watch_chunks(frame.area(), confirm.is_some(), note.is_some()); frame.render_widget( Paragraph::new(format!(" Pending approvals: {}", items.len())), @@ -133,7 +145,10 @@ fn render_watch( render_queue(frame, items, selected, chunks[1]); render_detail(frame, confirm, chunks[2]); - render_actions(frame, confirm, now_unix, chunks[3]); + // The same fit the model gates approve on (`priority_fields_fit`), taken + // from the very chunk the card is drawn into. + let approve_ok = confirm.is_none_or(|c| card_priority_fits(c, chunks[2])); + render_actions(frame, confirm, approve_ok, now_unix, chunks[3]); if let Some(note) = note { frame.render_widget(Paragraph::new(note), chunks[4]); @@ -159,6 +174,7 @@ fn seconds_left(not_after_unix: u64, now_unix: u64) -> u64 { fn render_actions( frame: &mut Frame, confirm: Option<&Confirm>, + approve_ok: bool, now_unix: u64, area: ratatui::layout::Rect, ) { @@ -185,21 +201,30 @@ fn render_actions( "n / esc" }; let left = seconds_left(confirm.card().not_after_unix, now_unix); + let reject_button = Span::styled( + format!("[ {reject_key} Reject · auto in {left}s ]"), + Style::new() + .add_modifier(Modifier::BOLD) + .add_modifier(Modifier::REVERSED), + ); - frame.render_widget( - Paragraph::new(Line::from(vec![ + // No live Approve button on a card the human cannot read (the model refuses + // the key too — `priority_fields_fit`). Reject stays: default-deny may + // never depend on the terminal being big enough (`AGENTS.md` #5). + let row = if approve_ok { + Line::from(vec![ Span::raw(" "), Span::raw(format!("[ {approve_key} Approve ]")), Span::raw(" "), - Span::styled( - format!("[ {reject_key} Reject · auto in {left}s ]"), - Style::new() - .add_modifier(Modifier::BOLD) - .add_modifier(Modifier::REVERSED), - ), - ])), - area, - ); + reject_button, + ]) + } else { + Line::from(vec![ + Span::raw(" approve disabled — terminal too small "), + reject_button, + ]) + }; + frame.render_widget(Paragraph::new(row), area); } fn render_queue( @@ -242,30 +267,13 @@ fn kind_word(s: &Summary) -> &'static str { } } -/// Render the open confirmation's card — the core's fields **verbatim**, no -/// re-derivation. `None` shows a hint to open one. -/// -/// Priority fields (everything except `raw_data`) render first, and -/// `raw_data` — the only elastic element — gets exactly the rows that remain, -/// truncated with an explicit marker when it cannot fit. A long calldata can -/// therefore never push a risk warning or the PIN prompt off the screen. That -/// guarantee is against `raw_data`: priority fields that alone overflow the -/// card (pathological server data) still clip at the bottom until scrolling -/// lands. Every line is pre-wrapped to the card's inner width so one logical -/// line is one visual row and the height budget is exact, not an estimate. -fn render_detail(frame: &mut Frame, confirm: Option<&Confirm>, area: ratatui::layout::Rect) { - let block = Block::bordered().title(" Card "); - let Some(confirm) = confirm else { - let hint = - Paragraph::new("Select a request and press enter to see the full card.").block(block); - frame.render_widget(hint, area); - return; - }; +/// The card's priority lines — every field except `raw_data` — pre-wrapped to +/// `width` display cells, so one logical line is one visual row and the height +/// arithmetic downstream is exact. One source for the renderer AND for +/// [`priority_fields_fit`]: the approve gate can never disagree with what is +/// actually drawn. +fn priority_lines(confirm: &Confirm, width: usize) -> Vec> { let card: &Card = confirm.card(); - - let inner = block.inner(area); - let width = usize::from(inner.width); - let height = usize::from(inner.height); let bold = Style::new().add_modifier(Modifier::BOLD); let mut lines: Vec> = Vec::new(); @@ -326,6 +334,70 @@ fn render_detail(frame: &mut Frame, confirm: Option<&Confirm>, area: ratatui::la lines.push(Line::from("")); push_wrapped(&mut lines, width, resolve_error_text(err), Style::new()); } + lines +} + +/// Whether the card's priority lines fit its inner area. +fn card_priority_fits(confirm: &Confirm, area: ratatui::layout::Rect) -> bool { + let inner = Block::bordered().inner(area); + priority_lines(confirm, usize::from(inner.width)).len() <= usize::from(inner.height) +} + +/// The approve gate: can a `width`×`height` terminal show every priority field +/// of the open card? Runs the same layout ([`watch_chunks`]) and the same line +/// pre-wrap ([`priority_lines`]) as the renderer, so the gate, the banner and +/// the missing Approve button always agree. The [`Model`] consults this before +/// letting `y` or a PIN submit do anything — a "yes" to a card the human could +/// not read is not a decision (`AGENTS.md` #1). +/// +/// `has_note` is `false` by construction: a note and an open confirmation never +/// coexist (`apply_get`/`apply_resolve` set one while clearing the other). +#[must_use] +pub fn priority_fields_fit(confirm: &Confirm, width: u16, height: u16) -> bool { + let area = ratatui::layout::Rect::new(0, 0, width, height); + let chunks = watch_chunks(area, true, false); + card_priority_fits(confirm, chunks[2]) +} + +/// Render the open confirmation's card — the core's fields **verbatim**, no +/// re-derivation. `None` shows a hint to open one. +/// +/// Priority fields (everything except `raw_data`) render first, and +/// `raw_data` — the only elastic element — gets exactly the rows that remain, +/// truncated with an explicit marker when it cannot fit. A long calldata can +/// therefore never push a risk warning or the PIN prompt off the screen. When +/// the priority fields alone cannot fit (a terminal below ~24 rows, or +/// pathological server data), the card says so with a banner and the approve +/// path is gated off ([`priority_fields_fit`]) until the terminal grows. +fn render_detail(frame: &mut Frame, confirm: Option<&Confirm>, area: ratatui::layout::Rect) { + let block = Block::bordered().title(" Card "); + let Some(confirm) = confirm else { + let hint = + Paragraph::new("Select a request and press enter to see the full card.").block(block); + frame.render_widget(hint, area); + return; + }; + + let inner = block.inner(area); + let width = usize::from(inner.width); + let height = usize::from(inner.height); + + let mut lines = priority_lines(confirm, width); + if lines.len() > height { + // The card cannot show what the human must read; approve is gated off + // (`priority_fields_fit` — the model refuses `y` and PIN submits). The + // banner goes on top: the one row guaranteed visible when rows clip. + let mut banner = Vec::new(); + push_wrapped( + &mut banner, + width, + "TERMINAL TOO SMALL — approve disabled; resize to read the card (reject works)" + .to_owned(), + Style::new().add_modifier(Modifier::BOLD), + ); + banner.append(&mut lines); + lines = banner; + } // raw_data comes LAST and absorbs whatever rows the priority fields above // left over. Never truncate priority fields; a truncated raw_data says so @@ -333,7 +405,7 @@ fn render_detail(frame: &mut Frame, confirm: Option<&Confirm>, area: ratatui::la // prevent. Wrap stays on as a backstop only (every line already fits the // width); trim: false keeps the exact bytes, including leading space. let budget = height.saturating_sub(lines.len()); - push_raw_data(&mut lines, width, budget, &card.raw_data); + push_raw_data(&mut lines, width, budget, &confirm.card().raw_data); frame.render_widget( Paragraph::new(lines) @@ -528,6 +600,11 @@ mod tests { } fn to_watching(model: &mut Model, items: Vec) { + // The size report main sends at startup — a standard 80×24 terminal. + model.update(Msg::Resize { + width: 80, + height: 24, + }); model.update(Msg::Reply(Reply::Hello { server: "s".to_owned(), })); @@ -932,6 +1009,41 @@ mod tests { ); } + #[test] + fn a_cramped_terminal_pulls_approve_and_says_why() { + let mut m = Model::new(); + open_risk_card(&mut m, stage1_raw_data()); + + // 80×14: the stage-1 card's priority fields cannot all fit (B4). + let rows = draw_rows(&m, 80, 14); + let screen = rows.join("\n"); + + assert!( + screen.contains("TOO SMALL"), + "the human is told the card is cut, never left guessing" + ); + assert!( + !screen.contains("Approve"), + "no live Approve button on a card the human cannot read" + ); + assert!( + rows.iter() + .any(|r| r.contains("Reject") && r.contains("auto in")), + "reject and its countdown survive at any size (AGENTS.md #5)" + ); + } + + #[test] + fn the_gate_lifts_when_the_terminal_grows() { + let mut m = Model::new(); + open_risk_card(&mut m, stage1_raw_data()); + + let screen = draw(&m, 80, 24); + + assert!(screen.contains("Approve"), "a full card arms the button"); + assert!(!screen.contains("TOO SMALL"), "no banner on a full card"); + } + #[test] fn the_resolved_screen_names_the_outcome_and_shows_the_tx_hash() { let mut m = Model::new(); From 11d2f1853e2df2a8c377c02af16e60c4ffd4da0a Mon Sep 17 00:00:00 2001 From: temrjan Date: Fri, 10 Jul 2026 11:02:36 +0500 Subject: [PATCH 11/11] fix,test: review minors - reset hygiene, PIN buffer, invariant coverage - A mid-session hello now drops the parked intent too (a parked approve belonged to the session that parked it; flushed after the reset it would fire into the auth screen). Unreachable with today's worker, which sends hello exactly once - hygiene, not a live hole. - Pin's typing buffer is pre-reserved to a 64-digit cap and clear() re-reserves, so it never reallocates: a realloc frees the digits un-zeroized. The neighbouring auth_line/approve_line already did this; the typing buffer was the one secret holder that grew freely. - Invariant #4 gets its first end-to-end test: piped stdin -> exit 3, the refusal on stderr, stdout empty (tests/tty_gate.rs, via CARGO_BIN_EXE). Deleting the gate no longer leaves the suite green. - Invariant #7's restore-then-stdout order now lives in one function (restore_then_announce) with a test that fails if the write ever precedes the restore; the exit-code test asserts full pairwise distinctness instead of comparing two constants to a third. - apply_auth / apply_resolve error mappings are pinned arm by arm: swapping NotSet<->Unavailable (either table) now fails the suite. Every test red-proven against the unfixed code or a live mutation. --- src/app.rs | 129 +++++++++++++++++++++++++++++++++++++++++++--- src/main.rs | 78 +++++++++++++++++++++++++--- tests/tty_gate.rs | 33 ++++++++++++ 3 files changed, 226 insertions(+), 14 deletions(-) create mode 100644 tests/tty_gate.rs diff --git a/src/app.rs b/src/app.rs index 6675767..6c31c42 100644 --- a/src/app.rs +++ b/src/app.rs @@ -22,9 +22,23 @@ use crate::ui; /// **redacted in `Debug`** so it never lands in a log line, a panic message, or a /// derived `Debug` of the `Model`. Only ASCII digits are accepted, which also keeps /// the hand-built auth JSON injection-free. -#[derive(Default, Clone)] +/// Hard cap on typed PIN digits — far beyond any real PIN. The buffer is +/// pre-reserved to exactly this length so it can never reallocate: a realloc +/// frees the old allocation (with the digits in it) **without** zeroizing. +const MAX_PIN_DIGITS: usize = 64; + +// No `Clone`: a `String::clone` allocates `capacity == len`, so a cloned PIN +// would silently lose the reallocation-proof reserve below — the first push on +// it would free the digits un-zeroized. Nobody clones a PIN; nobody gets to. pub struct Pin(Zeroizing); +impl Default for Pin { + fn default() -> Self { + // Reserved to the cap up front — `push` never grows the buffer. + Self(Zeroizing::new(String::with_capacity(MAX_PIN_DIGITS))) + } +} + impl std::fmt::Debug for Pin { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { // Never the digits — length only, so a Debug of the Model cannot leak it. @@ -33,9 +47,11 @@ impl std::fmt::Debug for Pin { } impl Pin { - /// Append a digit; non-digits are ignored (keeps the auth JSON injection-free). + /// Append a digit; non-digits are ignored (keeps the auth JSON injection-free), + /// and so is anything past [`MAX_PIN_DIGITS`] — the pre-reserved buffer must + /// never reallocate (a realloc frees the digits un-zeroized). pub fn push(&mut self, c: char) { - if c.is_ascii_digit() { + if c.is_ascii_digit() && self.0.len() < MAX_PIN_DIGITS { self.0.push(c); } } @@ -57,9 +73,11 @@ impl Pin { self.0.is_empty() } - /// Clear the PIN, zeroizing the current buffer (the old `Zeroizing` is dropped). + /// Clear the PIN, zeroizing the current buffer (the old `Zeroizing` is + /// dropped). The fresh buffer is reserved to the cap again, like + /// [`Self::default`] — cleared-and-retyped digits must not reallocate either. pub fn clear(&mut self) { - self.0 = Zeroizing::new(String::new()); + self.0 = Zeroizing::new(String::with_capacity(MAX_PIN_DIGITS)); } /// Build the `auth` request line into a `Zeroizing` buffer. Assembled by hand @@ -615,8 +633,11 @@ impl Model { error: None, }; // A handshake resets the protocol: an outstanding get will never - // be answered, and a stuck flag would refuse every future open. + // be answered (a stuck flag would refuse every future open), and + // a parked intent — an approve first of all — belongs to the + // session that parked it, not to the one being born. self.awaiting_card = false; + self.pending = None; } Reply::Auth(outcome) => self.apply_auth(outcome), Reply::List(items) => self.apply_list(items), @@ -967,6 +988,102 @@ mod tests { // ── PIN hygiene ── + #[test] + fn the_pin_buffer_never_reallocates_however_much_is_typed() { + // A realloc frees the old buffer — with the digits in it — WITHOUT + // zeroizing. The buffer is pre-reserved and capped, so it never moves. + let mut pin = Pin::default(); + let capacity = pin.0.capacity(); + for _ in 0..100 { + pin.push('7'); + } + assert_eq!(pin.len(), MAX_PIN_DIGITS, "input is capped, not grown into"); + assert_eq!( + pin.0.capacity(), + capacity, + "the PIN buffer must never reallocate" + ); + + // clear() hands out a fresh buffer — it must be just as realloc-proof. + pin.clear(); + let capacity = pin.0.capacity(); + for _ in 0..100 { + pin.push('7'); + } + assert_eq!(pin.0.capacity(), capacity, "the cleared buffer too"); + } + + #[test] + fn a_handshake_reset_drops_any_parked_intent() { + // A parked money intent belongs to the session that parked it. If the + // server ever restarts the handshake, flushing that intent afterwards + // would fire an approve into a phase that cannot even show the card. + let mut m = confirming("a", false); + assert!(matches!( + m.update(Msg::Tick), + Some(transport::Request::List) + )); + m.update(Msg::Approve); // parked behind the poll + assert!( + m.update(Msg::Reply(Reply::Hello { + server: "s".to_owned(), + })) + .is_none(), + "a parked approve must die with the session, not follow the reset" + ); + } + + #[test] + fn auth_failures_map_each_code_to_its_own_message() { + // A swapped arm here would tell a locked-out human to set a PIN — the + // mappings must bite, not merely compile (несёт и retry-данные). + for (outcome, expected) in [ + ( + AuthOutcome::BadPin { attempts_left: 2 }, + AuthError::BadPin(2), + ), + ( + AuthOutcome::Locked { retry_after_s: 30 }, + AuthError::Locked(30), + ), + (AuthOutcome::PinNotSet, AuthError::NotSet), + (AuthOutcome::PinUnavailable, AuthError::Unavailable), + ] { + let mut m = Model::new(); + m.update(Msg::Reply(Reply::Hello { + server: "s".to_owned(), + })); + m.update(Msg::PinDigit('1')); + m.update(Msg::PinSubmit); + m.update(Msg::Reply(Reply::Auth(outcome))); + let Phase::Authing { error, .. } = m.phase() else { + panic!("an auth failure keeps the unlock screen"); + }; + assert_eq!(error.as_ref(), Some(&expected)); + } + } + + #[test] + fn resolve_failures_map_each_code_to_its_own_error() { + for (outcome, expected) in [ + ( + ResolveOutcome::BadPin { attempts_left: 1 }, + ResolveError::BadPin(1), + ), + ( + ResolveOutcome::Locked { retry_after_s: 60 }, + ResolveError::Locked(60), + ), + (ResolveOutcome::PinNotSet, ResolveError::NotSet), + (ResolveOutcome::PinUnavailable, ResolveError::Unavailable), + ] { + let mut m = confirming("a", false); + m.update(Msg::Approve); + m.update(Msg::Reply(Reply::Resolve(outcome))); + assert_eq!(confirm_of(&m).error(), Some(&expected)); + } + } + #[test] fn pin_debug_never_shows_the_digits() { let mut pin = Pin::default(); diff --git a/src/main.rs b/src/main.rs index 067c295..315406c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -75,14 +75,19 @@ fn main() -> ExitCode { let transport = Transport::connect(&path); let (code, decision) = run(terminal, &transport); - let _ = restore(); + restore_then_announce(|| drop(restore()), decision.as_deref(), &mut io::stdout()); + ExitCode::from(code) +} - // The decision lands on stdout only after the alternate screen is gone, so no - // escape sequence can ever reach a caller parsing it (invariant #7). +/// Leave the terminal, THEN speak on stdout — in that order and no other: the +/// decision line must never share stdout's airtime with alternate-screen escape +/// sequences a caller could capture (invariant #7). The order lives in this one +/// function so a test can pin it. +fn restore_then_announce(restore: impl FnOnce(), decision: Option<&str>, out: &mut impl io::Write) { + restore(); if let Some(decision) = decision { - println!("{decision}"); + let _ = writeln!(out, "{decision}"); } - ExitCode::from(code) } /// Set up the terminal on **stderr** (raw mode + alternate screen) and install a @@ -486,10 +491,67 @@ mod tests { assert_eq!(exit_code(ExitOutcome::Rejected), EXIT_REJECTED); assert_eq!(exit_code(ExitOutcome::Expired), EXIT_EXPIRED); assert_eq!(exit_code(ExitOutcome::Failed), EXIT_FATAL); - // aborted and no-tty are distinct from approved (invariant #7) - for code in [EXIT_ABORTED, EXIT_NO_TTY] { - assert_ne!(code, EXIT_APPROVED); + // Every process outcome a caller can see must be pairwise distinct + // (invariant #7) — reusing a code would let one outcome impersonate + // another, `approved` worst of all. + let codes = [ + EXIT_APPROVED, + EXIT_FATAL, + EXIT_UPGRADE, + EXIT_NO_TTY, + EXIT_REJECTED, + EXIT_EXPIRED, + EXIT_ABORTED, + ]; + for (i, a) in codes.iter().enumerate() { + for b in &codes[i + 1..] { + assert_ne!(a, b, "exit codes must never collide"); + } + } + } + + #[test] + fn the_decision_speaks_only_after_the_terminal_is_restored() { + use std::cell::Cell; + + struct OrderProbe<'a> { + restored: &'a Cell, + buf: Vec, } + impl io::Write for OrderProbe<'_> { + fn write(&mut self, bytes: &[u8]) -> io::Result { + assert!( + self.restored.get(), + "the decision line must never race the alternate-screen \ + escapes (invariant #7)" + ); + self.buf.extend_from_slice(bytes); + Ok(bytes.len()) + } + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + let restored = Cell::new(false); + let mut probe = OrderProbe { + restored: &restored, + buf: Vec::new(), + }; + restore_then_announce( + || restored.set(true), + Some(r#"{"decision":"approved"}"#), + &mut probe, + ); + assert_eq!(probe.buf, b"{\"decision\":\"approved\"}\n"); + + // And nothing is written at all when there is no decision. + let mut probe = OrderProbe { + restored: &restored, + buf: Vec::new(), + }; + restore_then_announce(|| restored.set(true), None, &mut probe); + assert!(probe.buf.is_empty()); } #[test] diff --git a/tests/tty_gate.rs b/tests/tty_gate.rs new file mode 100644 index 0000000..c671cbc --- /dev/null +++ b/tests/tty_gate.rs @@ -0,0 +1,33 @@ +//! Invariant #4, end to end: an approval may never come from a pipe. The gate +//! is the first thing `main` runs — before the terminal, before the socket — +//! so this is testable without either. + +use std::process::{Command, Stdio}; + +/// `EXIT_NO_TTY` in `main.rs` (the binary's constants are not importable). +const EXIT_NO_TTY: i32 = 3; + +#[test] +fn a_piped_stdin_is_refused_before_anything_opens() { + let out = Command::new(env!("CARGO_BIN_EXE_rustok-console")) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output() + .expect("the binary runs"); + + assert_eq!( + out.status.code(), + Some(EXIT_NO_TTY), + "a non-interactive stdin exits with the distinct no-tty code" + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("Approval from a pipe is never accepted."), + "the refusal names itself on stderr: {stderr}" + ); + assert!( + out.stdout.is_empty(), + "stdout carries decisions only (invariant #7) — a refusal is not one" + ); +}