From 8957573fc589061ba93f525920775c25f9d33ea6 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Fri, 11 Sep 2026 19:41:44 -0400 Subject: [PATCH] fix(agentd): print only printable characters from a server's scopes A shared `printable` escapes everything outside printable ASCII, so a scope atom or an origin cannot repaint the screen an agent approves from. Co-Authored-By: Claude Opus 5 (1M context) --- crates/didbot-agentd/src/bin/didbot-oauth.rs | 35 ++++++++++--- crates/didbot-agentd/src/cli.rs | 53 ++++++++++++++++++-- 2 files changed, 77 insertions(+), 11 deletions(-) diff --git a/crates/didbot-agentd/src/bin/didbot-oauth.rs b/crates/didbot-agentd/src/bin/didbot-oauth.rs index 834c9694..850e3376 100644 --- a/crates/didbot-agentd/src/bin/didbot-oauth.rs +++ b/crates/didbot-agentd/src/bin/didbot-oauth.rs @@ -15,7 +15,7 @@ use std::process::ExitCode; -use didbot_agentd::cli::{ask, choose, flag, one_line, positional, socket_path, Mode}; +use didbot_agentd::cli::{ask, choose, flag, one_line, positional, printable, socket_path, Mode}; use didbot_agentd::decisions::Record; use didbot_agentd::direct::{Direct, Named}; use didbot_agentd::protocol::{ @@ -48,18 +48,25 @@ account. --direct insists on it rather than trying the socket first. /// came through: the scopes requested, then the scopes current policies /// allow. fn print_scopes(requested: &[String], granted: &[String]) { - println!("This application requested these scopes:"); + print!("{}", scope_block(requested, granted)); +} + +/// The same, as text, so what an agent is shown at the moment it approves can +/// be read back by a test. +fn scope_block(requested: &[String], granted: &[String]) -> String { + let mut block = String::from("This application requested these scopes:\n"); for scope in requested { - println!("* {scope}"); + block.push_str(&format!("* {}\n", printable(scope))); } if granted == requested { - println!("Current policies allow these scopes:"); + block.push_str("Current policies allow these scopes:\n"); } else { - println!("Current policies only allow these scopes:"); + block.push_str("Current policies only allow these scopes:\n"); } for scope in granted { - println!("* {scope}"); + block.push_str(&format!("* {}\n", printable(scope))); } + block } fn main() -> ExitCode { @@ -328,6 +335,22 @@ mod tests { raw.iter().map(|arg| (*arg).to_owned()).collect() } + #[test] + fn a_scope_carrying_an_escape_sequence_reaches_the_terminal_as_text() { + let requested = vec!["repo:generic\u{1b}[2K\u{1b}[Aadmin".to_owned()]; + let granted = vec!["repo:generic".to_owned()]; + let block = scope_block(&requested, &granted); + + // No ESC anywhere in what is written, so the two lines an agent reads + // to decide cannot be erased or overwritten by the atom itself. + assert!(!block.contains('\u{1b}'), "{block}"); + assert!( + block.contains(r"* repo:generic\u{1b}[2K\u{1b}[Aadmin"), + "{block}" + ); + assert!(block.contains("Current policies only allow these scopes:")); + } + #[test] fn every_command_this_binary_takes_is_in_its_usage() { for command in ["pending", "approve", "decline"] { diff --git a/crates/didbot-agentd/src/cli.rs b/crates/didbot-agentd/src/cli.rs index 7bcf9709..94b329c9 100644 --- a/crates/didbot-agentd/src/cli.rs +++ b/crates/didbot-agentd/src/cli.rs @@ -122,12 +122,35 @@ pub fn flag<'a>(args: &'a [String], name: &str) -> Option<&'a str> { None } +/// One string a server chose, ready to put on a terminal. +/// +/// Everything outside printable ASCII is written as its escape rather than +/// sent through, so a scope atom or an origin carrying `ESC[` cannot move the +/// cursor, repaint the line above it, or hide what an approval covers. An +/// agent reads these at the moment it decides, which is the worst moment for +/// the screen to be lying. +pub fn printable(text: &str) -> String { + text.chars() + .map(|c| { + if c.is_ascii_graphic() || c == ' ' { + c.to_string() + } else { + c.escape_debug().to_string() + } + }) + .collect() +} + /// A scope set, as one field of a line. pub fn list(scopes: &[String]) -> String { if scopes.is_empty() { "none".to_owned() } else { - scopes.join(",") + scopes + .iter() + .map(|scope| printable(scope)) + .collect::>() + .join(",") } } @@ -149,14 +172,14 @@ const NARROW_APPROVES: &str = "approves=asked ceiling-checked=each-use"; pub fn one_line(decision: &DecisionForAgent) -> String { let mut line = format!( "{} {} asked={} verdict={}", - decision.client_origin, + printable(&decision.client_origin), if decision.first_time { "first-time" } else { "seen-before" }, list(&decision.requested), - decision.verdict, + printable(&decision.verdict), ); if decision.granted != decision.requested { line.push_str(&format!(" granted={}", list(&decision.granted))); @@ -176,9 +199,9 @@ pub fn one_line(decision: &DecisionForAgent) -> String { line.push(' '); line.push_str(NARROW_APPROVES); } - line.push_str(&format!(" expires={}", decision.expires_at)); + line.push_str(&format!(" expires={}", printable(&decision.expires_at))); match &decision.token { - Some(token) => line.push_str(&format!(" token={token}")), + Some(token) => line.push_str(&format!(" token={}", printable(token))), None => line.push_str(" token=none"), } line @@ -318,6 +341,26 @@ mod tests { } } + #[test] + fn a_line_stays_one_line_however_the_server_wrote_its_fields() { + // Every field here is chosen by whoever answered, so each is a place + // an escape sequence could be handed to the terminal. + let mut decision = decision(); + decision.client_origin = "http://one.example\u{1b}[2K".into(); + decision.requested = vec!["atproto\u{1b}[A".into()]; + decision.granted = vec!["atproto".into()]; + decision.cut = vec![]; + decision.verdict = "allow\r".into(); + decision.token = Some("k7f3\nrogue".into()); + + let line = one_line(&decision); + assert!(!line.contains('\u{1b}'), "{line}"); + assert!(!line.contains('\n'), "{line}"); + assert!(!line.contains('\r'), "{line}"); + assert!(line.contains(r"asked=atproto\u{1b}[A"), "{line}"); + assert!(line.ends_with(r"token=k7f3\nrogue"), "{line}"); + } + #[test] fn a_decision_is_one_line_and_the_token_is_the_end_of_it() { let line = one_line(&decision()); -- 2.51.2