diff --git a/CLAUDE.md b/CLAUDE.md index d00981305..a4b03869b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1211,6 +1211,21 @@ replaced a standalone `biorouter-headless` binary and its Linux tarball, both de operator-pinned-off extension and its ordinary path needs none. ⚠ Not a security change: nothing that was refused becomes permitted. The availability flag is sampled ONCE per roster and threaded, so a roster can never half-believe a person is reachable. +- **Stop and steering answer to the reach gate on a keyless daemon** (SD-11). `/agent/cancel`, + `/interrupt` and the two `/agent/continuation/*` routes take the proof on a daemon that holds + a key, and on one that holds none gate through `authorize_agent_control` — the *same call* + `/agent/stop` makes — via `reply.rs::authorize_turn_control`. Tightening that gate tightens + who may press Stop in a browser. A keyless steer is recorded unstamped (never `UserDirect`), + and a subagent's tab stays refused. ⚠ Keyless behaviour can only be tested in its own binary + (the digest is a process-global `OnceLock`): `cargo test -p biorouter-server --test + turn_control_no_user_key`. + ⚠ **The CLI reads the refusal's shape, not its status.** `biorouter session cancel` / `attach` + / `send` cannot ask a daemon whether it holds a key, so they send without the proof and ask the + person for the key only on turn control's **empty** 403 — the keyed `Unproven` arm; every + keyless refusal carries a sentence and is printed instead (`key_verdict` in + `commands/session_watch.rs`). A sentence added to `Unproven`, or an empty keyless refusal, + breaks the terminal silently — one never prompts on the desktop's daemon, the other prompts a + `serve` user for a key that does not exist. Pinned from both sides. - **Proof of a person is checked at the resolution choke point, not at one route.** Every door that answers a parked decision — the HTTP route, an Agent Drafter app's WebSocket, ACP, the CLI prompt, the TUI modal, an ancestor agent's relay — passes a `DecisionAuthority` into diff --git a/crates/biorouter-cli/src/cli.rs b/crates/biorouter-cli/src/cli.rs index 8649acf2b..cc9ccc591 100644 --- a/crates/biorouter-cli/src/cli.rs +++ b/crates/biorouter-cli/src/cli.rs @@ -668,7 +668,7 @@ enum SessionCommand { no_wait: bool, #[arg( long, - help = "Read the daemon's raw user-action key from the first line of stdin instead of prompting on the controlling terminal" + help = "For a daemon started with a user-action key: read the raw key from the first line of stdin, instead of being asked for it on the terminal once the daemon wants it" )] user_action_key_stdin: bool, }, @@ -699,7 +699,7 @@ enum SessionCommand { read_only: bool, #[arg( long, - help = "Read the daemon's raw user-action key from the first line of stdin instead of prompting on the controlling terminal" + help = "For a daemon started with a user-action key: read the raw key from the first line of stdin, instead of being asked for it on the terminal once the daemon wants it" )] user_action_key_stdin: bool, }, @@ -709,7 +709,7 @@ enum SessionCommand { session_id: String, #[arg( long, - help = "Read the daemon's raw user-action key from the first line of stdin instead of prompting on the controlling terminal" + help = "For a daemon started with a user-action key: read the raw key from the first line of stdin, instead of being asked for it on the terminal once the daemon wants it" )] user_action_key_stdin: bool, }, diff --git a/crates/biorouter-cli/src/commands/session_watch.rs b/crates/biorouter-cli/src/commands/session_watch.rs index e923dc7f0..41295ea0e 100644 --- a/crates/biorouter-cli/src/commands/session_watch.rs +++ b/crates/biorouter-cli/src/commands/session_watch.rs @@ -64,10 +64,15 @@ pub(crate) struct DaemonAuth { /// the public tier — the same answer, stated by saying nothing rather than /// by saying something meaningless. caller_provider: String, - /// Present only for attach/cancel routes that require proof of a live - /// operator: the attached event stream, interrupt, cancel, and a - /// provenance-less reply to a subagent. - /// It is never sourced from argv, environment, config, or desktop settings. + /// The raw user-action key, when this terminal holds it: the person supplied + /// it on stdin (`--user-action-key-stdin`), or a daemon refused a request for + /// want of it and the person then typed it (see [`key_verdict`]). `None` + /// otherwise, and a protected request then goes out WITHOUT the header, for + /// the daemon to judge. + /// + /// Sent only on the requests the proof can change: the attached event + /// stream, `/interrupt`, `/agent/cancel` and `/reply`. It is never sourced + /// from argv, environment, config, or desktop settings. user_action: Option>>, } @@ -94,48 +99,234 @@ pub(crate) async fn daemon_auth() -> Result { }) } -async fn daemon_auth_with_user_action(from_stdin: bool) -> Result { - let auth = daemon_auth().await?; - auth_with_user_action(auth, from_stdin).await -} +// ────────────────────────────────────────────────────────────────────────────── +// The user-action key: the daemon is asked before the person is. +// +// A daemon started with a key (the desktop app's, or one launched with its +// digest on stdin) wants it before it lets anyone stop or steer a turn. One +// started without (`biorouter serve`, a hand-run `biorouterd agent`) holds +// nothing to check a key against, and admits those requests on the reach gate +// instead (serve decision SD-11). A terminal cannot tell the two apart, and the +// old answer to that — ask the person for a key before sending anything — asked +// every `serve` user for a key that does not exist. +// +// So the request goes out without the key and the daemon's answer says which +// kind it is (`key_verdict`). The person is asked only when the daemon wanted +// the key, and the request is made once more with it; a refusal the key cannot +// change is shown in the daemon's own words. None of this relaxes anything: the +// daemon is the boundary, and every refusal read here is given before the route +// touches the turn. A subagent's session still needs the proof, and a daemon +// without a key still refuses it. +// ────────────────────────────────────────────────────────────────────────────── -async fn auth_with_user_action(mut auth: DaemonAuth, from_stdin: bool) -> Result { - let key = tokio::task::spawn_blocking(move || read_user_action_key(from_stdin)) +/// `--user-action-key-stdin`: take the key from stdin's first line before +/// anything is sent. +/// +/// Read up front, unlike the terminal prompt, which waits for a daemon to ask: +/// on `attach` every later line of stdin is a message, so the key's line must +/// be taken before the reader that treats lines as messages starts. +async fn with_supplied_key(auth: DaemonAuth, key_from_stdin: bool) -> Result { + if !key_from_stdin { + return Ok(auth); + } + let key = tokio::task::spawn_blocking(read_key_from_stdin) .await .map_err(|join| anyhow!("could not read the user-action key: {join}"))??; - auth.user_action = Some(Arc::new(key)); - Ok(auth) + Ok(auth.with_user_action(key)) } -fn read_user_action_key(from_stdin: bool) -> Result> { - let key = if from_stdin { - let mut key = String::new(); - std::io::BufRead::read_line(&mut std::io::stdin().lock(), &mut key)?; - while key.ends_with('\n') || key.ends_with('\r') { - key.pop(); - } - Zeroizing::new(key) - } else { - if !std::io::stdin().is_terminal() || !std::io::stderr().is_terminal() { - return Err(anyhow!( - "steering and cancellation require the user-action key that was hashed into \ - biorouterd at startup. Run this from a controlling terminal, or pass \ - --user-action-key-stdin and pipe the raw key as the first line" - )); - } - eprintln!( - "Enter the user-action key supplied when this daemon was launched \ - (input is hidden):" - ); - Zeroizing::new(console::Term::stderr().read_secure_line()?) - }; +fn read_key_from_stdin() -> Result> { + let mut key = Zeroizing::new(String::new()); + std::io::BufRead::read_line(&mut std::io::stdin().lock(), &mut key)?; + while key.ends_with('\n') || key.ends_with('\r') { + key.pop(); + } + non_empty_key(key) +} + +/// Ask the person at the controlling terminal for the key, with echo off. Only +/// ever called once a daemon has refused a request for want of it. +async fn ask_terminal_for_key(key_use: KeyUse) -> Result> { + tokio::task::spawn_blocking(move || prompt_for_key(key_use)) + .await + .map_err(|join| anyhow!("could not read the user-action key: {join}"))? +} + +fn prompt_for_key(key_use: KeyUse) -> Result> { + if !std::io::stdin().is_terminal() || !std::io::stderr().is_terminal() { + return Err(anyhow!("{}", key_use.no_terminal())); + } + eprintln!("{}", key_use.prompt()); + non_empty_key(Zeroizing::new(console::Term::stderr().read_secure_line()?)) +} + +fn non_empty_key(key: Zeroizing) -> Result> { if key.is_empty() { return Err(anyhow!("the user-action key cannot be empty")); } Ok(key) } +/// What the key is wanted for, in the words its prompt and its refusals use. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum KeyUse { + /// `session cancel` — `POST /agent/cancel`. + Stop, + /// `session attach`, asked as it joins — `POST /interrupt`. + Steer, + /// `session send` — `POST /reply`. + Send, +} + +impl KeyUse { + /// What the daemon wants the key for. + fn act(self) -> &'static str { + match self { + KeyUse::Stop => "stopping a turn", + KeyUse::Steer => "steering a session", + KeyUse::Send => "sending to this session", + } + } + + /// What the refused request left undone. + fn undone(self) -> &'static str { + match self { + KeyUse::Stop => "The turn was not stopped.", + KeyUse::Steer => "The session was not steered.", + KeyUse::Send => "The message was not delivered.", + } + } + + fn prompt(self) -> String { + format!( + "This daemon was started with a user-action key and wants it for {}. \ + Enter the key (input is hidden):", + self.act() + ) + } + + fn no_terminal(self) -> String { + let read_only = match self { + KeyUse::Steer => " `--read-only` follows the session without it.", + KeyUse::Stop | KeyUse::Send => "", + }; + format!( + "This daemon was started with a user-action key and wants it for {}, but there is \ + no terminal to ask for it on. Run this from a terminal, or pass \ + --user-action-key-stdin and pipe the raw key as the first line of stdin. {}{read_only}", + self.act(), + self.undone() + ) + } + + fn wrong_key(self) -> String { + format!( + "the daemon refused the user-action key: it is not the key this daemon was started \ + with. {}", + self.undone() + ) + } +} + +/// What a daemon's answer to a stop or a steer says about the user-action key. +/// +/// ⚠ **Read only off the turn-control routes, `/agent/cancel` and +/// `/interrupt`.** There the two kinds of daemon refuse in shapes that cannot be +/// confused (serve decision SD-11). One that holds a key refuses a request that +/// lacks the proof, or carries a wrong one, with an EMPTY 403 +/// (`routes::reply::authorize_turn_control`). One that holds none gates through +/// `authorize_agent_control` instead, and every refusal of that gate carries a +/// sentence (`SESSION_REACH_NO_KEY`, `SUBAGENT_CONTROL_NO_KEY`). `/reply` +/// promises no such thing: its subagent refusal is an empty 403 on EITHER kind +/// of daemon, which is why `send` asks the steer gate instead of reading its own +/// 403. +/// +/// Both halves are pinned from the daemon's side, by `routes::reply`'s keyed +/// tests and by `tests/turn_control_no_user_key.rs`, because this reading +/// decides whether a person is asked for a key at all. +#[derive(Debug, Clone, PartialEq, Eq)] +enum KeyVerdict { + /// Not refused for want of the key; the status says what did happen. + NotAsked, + /// Refused for want of the key: this daemon holds one, and the request + /// carried none, or a wrong one. + Wanted, + /// Refused in the daemon's own words. No key changes this answer, so the + /// person is shown the words rather than asked for one. + Refused(String), +} + +fn key_verdict(code: u16, body: &str) -> KeyVerdict { + if code != 403 { + return KeyVerdict::NotAsked; + } + match refusal_sentence(body) { + Some(sentence) => KeyVerdict::Refused(sentence), + None => KeyVerdict::Wanted, + } +} + +/// The sentence a refusal carries, or `None` for an empty body. +/// +/// `ErrorResponse` answers `{"message": …}` and the reach gate answers plain +/// text where a route hands its refusal back directly. Both are the daemon's +/// own words and are shown as they are. +fn refusal_sentence(body: &str) -> Option { + let text = json_object(body) + .and_then(|value| value.get("message")?.as_str().map(str::to_string)) + .unwrap_or_else(|| body.to_string()); + let text = text.trim(); + (!text.is_empty()).then(|| text.to_string()) +} + +/// Make a request without the key and, only if the daemon refuses it for want +/// of one, ask the person for the key and make the request once more with it. +/// +/// ⚠ **The first attempt is the question, not a courtesy.** Asking the person +/// first would ask a `biorouter serve` user for a key that does not exist; the +/// request itself asks the daemon instead, and costs nothing when refused, +/// because every refusal [`key_verdict`] reads is given before the route touches +/// anything. A request that already carried a key (`--user-action-key-stdin`) +/// is never followed by a prompt: its refusal means the key is wrong, not +/// missing. +/// +/// Returns the last answer, what it said about the key, and the auth it was +/// made with — which holds the key exactly when it was supplied or wanted. +/// `attempt`, `verdict` and `ask` are arguments so this can be driven over +/// every answer order in a test, as `run_ladder` is. +async fn with_key_if_wanted( + auth: DaemonAuth, + mut attempt: Attempt, + verdict: impl Fn(&T) -> KeyVerdict, + ask: Ask, +) -> Result<(T, KeyVerdict, DaemonAuth)> +where + Attempt: FnMut(DaemonAuth) -> AttemptFut, + AttemptFut: std::future::Future>, + Ask: FnOnce() -> AskFut, + AskFut: std::future::Future>>, +{ + let answer = attempt(auth.clone()).await?; + let judged = verdict(&answer); + if judged != KeyVerdict::Wanted || auth.holds_key() { + return Ok((answer, judged, auth)); + } + let auth = auth.with_user_action(ask().await?); + let answer = attempt(auth.clone()).await?; + let judged = verdict(&answer); + Ok((answer, judged, auth)) +} + impl DaemonAuth { + fn with_user_action(mut self, key: Zeroizing) -> Self { + self.user_action = Some(Arc::new(key)); + self + } + + fn holds_key(&self) -> bool { + self.user_action.is_some() + } + /// The two headers every request carries, already CRLF-terminated. /// /// Composed in one place so a request cannot state its secret without also @@ -226,23 +417,33 @@ pub(crate) fn build_get_request(path: &str, host: &str, auth: &DaemonAuth) -> St ) } -fn build_user_action_get_request( - path: &str, - host: &str, - auth: &DaemonAuth, -) -> Result> { - let proof = auth.user_action.as_ref().ok_or_else(|| { - anyhow!("this action requires a user-action key from the controlling terminal") - })?; +/// Put the user-action proof on a request to a route it can change, exactly +/// when `auth` holds the key. +/// +/// Without a key the header is left off entirely, never sent empty, and the +/// daemon judges the request as it would any other caller's: that is how +/// `with_key_if_wanted` learns whether this daemon wants the key at all. The +/// buffer is zeroizing either way, since it may hold the raw key. +fn push_proof(request: &mut Zeroizing, auth: &DaemonAuth) { + if let Some(proof) = auth.user_action.as_ref() { + request.push_str(USER_ACTION_HEADER); + request.push_str(": "); + request.push_str(proof); + request.push_str("\r\n"); + } +} + +/// `build_get_request`, for the one GET the proof can change: attach's event +/// stream, where it lets a person reach a private chat on a daemon that holds a +/// key. See [`push_proof`]. +fn build_protected_get_request(path: &str, host: &str, auth: &DaemonAuth) -> Zeroizing { let mut request = Zeroizing::new(format!( "GET {path} HTTP/1.1\r\nHost: {host}\r\n{}", auth.headers() )); - request.push_str(USER_ACTION_HEADER); - request.push_str(": "); - request.push_str(proof); - request.push_str("\r\nAccept: text/event-stream\r\nConnection: close\r\n\r\n"); - Ok(request) + push_proof(&mut request, auth); + request.push_str("Accept: text/event-stream\r\nConnection: close\r\n\r\n"); + request } #[cfg(test)] @@ -256,27 +457,25 @@ pub(crate) fn build_post_request(path: &str, host: &str, auth: &DaemonAuth, body ) } -fn build_user_action_post_request( +/// A POST to a route the user-action proof can change — `/interrupt`, +/// `/agent/cancel`, `/reply` — carrying the proof exactly when `auth` holds the +/// key. See [`push_proof`]. +fn build_protected_post_request( path: &str, host: &str, auth: &DaemonAuth, body: &str, -) -> Result> { - let proof = auth.user_action.as_ref().ok_or_else(|| { - anyhow!("this action requires a user-action key from the controlling terminal") - })?; +) -> Zeroizing { let mut request = Zeroizing::new(format!( "POST {path} HTTP/1.1\r\nHost: {host}\r\n{}", auth.headers() )); - request.push_str(USER_ACTION_HEADER); - request.push_str(": "); - request.push_str(proof); - request.push_str("\r\nContent-Type: application/json\r\nContent-Length: "); + push_proof(&mut request, auth); + request.push_str("Content-Type: application/json\r\nContent-Length: "); request.push_str(&body.len().to_string()); request.push_str("\r\nAccept: application/json\r\nConnection: close\r\n\r\n"); request.push_str(body); - Ok(request) + request } /// Append `chunk` to `buffer` and drain every COMPLETE SSE frame into `out`. @@ -619,7 +818,13 @@ async fn stream_request( /// A connection to the configured daemon, or the actionable "no daemon" error. async fn connect_to_daemon() -> Result { - let port = configured_port(); + connect_to_daemon_at(configured_port()).await +} + +/// [`connect_to_daemon`] for a port the caller names — a test's stand-in +/// daemon, which must not be reached by setting `BIOROUTER_PORT` in a process +/// every test shares. +async fn connect_to_daemon_at(port: u16) -> Result { if !daemon_ok(DAEMON_HOST, port).await { return Err(anyhow!("{}", no_daemon_at(port))); } @@ -887,11 +1092,20 @@ fn json_object(body: &str) -> Option { /// request made from inside an interactive loop must not be able to hang it, /// hence the deadline (as in `running_session_ids`). async fn post_json(path: &str, body: &str, auth: &DaemonAuth) -> Result<(u16, String)> { - let port = configured_port(); + post_json_to(configured_port(), path, body, auth).await +} + +/// [`post_json`] to the daemon on `port`; see [`connect_to_daemon_at`]. +async fn post_json_to( + port: u16, + path: &str, + body: &str, + auth: &DaemonAuth, +) -> Result<(u16, String)> { if !daemon_ok(DAEMON_HOST, port).await { return Err(anyhow!("{}", no_daemon_at(port))); } - let request = build_user_action_post_request(path, DAEMON_HOST, auth, body)?; + let request = build_protected_post_request(path, DAEMON_HOST, auth, body); let raw = tokio::time::timeout(std::time::Duration::from_secs(10), async { let mut stream = tokio::net::TcpStream::connect(format!("{DAEMON_HOST}:{port}")).await?; stream.write_all(request.as_bytes()).await?; @@ -909,14 +1123,46 @@ async fn post_json(path: &str, body: &str, auth: &DaemonAuth) -> Result<(u16, St .await .map_err(|_| anyhow!("the daemon did not answer POST {path} within 10s"))??; - let text = String::from_utf8_lossy(&raw).to_string(); - let (head, body) = text - .split_once("\r\n\r\n") - .ok_or_else(|| anyhow!("daemon sent a malformed response to POST {path}"))?; + parse_http_response(&raw, &format!("POST {path}")) +} + +/// One HTTP response, as its status code and its body — dechunked when the head +/// says the body is chunked. +/// +/// ⚠ **Chunked framing is not part of the body, and reading it as one is not a +/// theoretical worry.** hyper answers an EMPTY body with `transfer-encoding: +/// chunked` and a lone `0\r\n\r\n` terminator, so a parser that hands the bytes +/// back as they came reports a body of `"0"`. Measured against a real daemon +/// that holds a user-action key on 2026-09-11: [`key_verdict`] read that `"0"` +/// as the daemon's refusal sentence, so `session cancel` printed *the daemon +/// would not stop the turn: 0* instead of asking for the key the daemon was +/// waiting for — the empty 403 is exactly the answer that must reach it intact. +fn parse_http_response(raw: &[u8], what: &str) -> Result<(u16, String)> { + // Split on BYTES: a chunk size counts bytes, and a lossy decode first could + // move them. + let end = raw + .windows(4) + .position(|w| w == b"\r\n\r\n") + .ok_or_else(|| { + anyhow!( + "the daemon closed the connection before sending a complete response to {what}, so \ + it is not known whether the request was carried out" + ) + })?; + let head = String::from_utf8_lossy(&raw[..end]).into_owned(); let status = head.lines().next().unwrap_or_default(); let code = status_code(status) .ok_or_else(|| anyhow!("daemon sent a response carrying no status code: {status}"))?; - Ok((code, body.to_string())) + let body = &raw[end + 4..]; + let body = if head + .to_ascii_lowercase() + .contains("transfer-encoding: chunked") + { + dechunk(body) + } else { + body.to_vec() + }; + Ok((code, String::from_utf8_lossy(&body).into_owned())) } /// One request to a JSON route that takes the secret key and nothing more — the @@ -958,31 +1204,7 @@ pub(crate) async fn daemon_json_request( None => exchange.await?, }; - // Split on BYTES: a chunk size counts bytes, and a lossy decode first could - // move them. - let end = raw - .windows(4) - .position(|w| w == b"\r\n\r\n") - .ok_or_else(|| { - anyhow!( - "the daemon closed the connection before sending a complete response to {method} \ - {path}, so it is not known whether the request was carried out" - ) - })?; - let head = String::from_utf8_lossy(&raw[..end]).into_owned(); - let status = head.lines().next().unwrap_or_default(); - let code = status_code(status) - .ok_or_else(|| anyhow!("daemon sent a response carrying no status code: {status}"))?; - let body = &raw[end + 4..]; - let body = if head - .to_ascii_lowercase() - .contains("transfer-encoding: chunked") - { - dechunk(body) - } else { - body.to_vec() - }; - Ok((code, String::from_utf8_lossy(&body).into_owned())) + parse_http_response(&raw, &format!("{method} {path}")) } /// An HTTP/1.1 chunked body, joined. Malformed framing ends the body where it @@ -1067,6 +1289,12 @@ pub(crate) enum SendOutcome { /// 202: the session is a subagent still starting, and the message was kept /// as steering for its first turn (`routes/reply.rs`). Queued, + /// 403: refused by the reach gate or the subagent rule, both of which + /// `/reply` asks before it takes the turn lock or writes anything — so + /// nothing happened, and the request can be made again. Whether the + /// user-action key would change the answer is [`send_to`]'s question; the + /// 403 alone cannot say (see [`key_verdict`]). + Forbidden, } /// Send one `POST /reply` over `stream` and read the answer as far as `wait` @@ -1116,11 +1344,94 @@ where turn_id: streamed.turn_id, }), 202 => Ok(SendOutcome::Queued), + 403 => Ok(SendOutcome::Forbidden), code => Err(anyhow!( "daemon refused the request: HTTP {code}\n\ - (401 usually means BIOROUTER_SERVER__SECRET_KEY does not match the daemon's; \ - 403 means the user-action key does not match the daemon's configured digest)" + (401 usually means BIOROUTER_SERVER__SECRET_KEY does not match the daemon's)" + )), + } +} + +/// The empty steer [`settle_steering_key`] and [`send_to`] ask a daemon with. +/// +/// ⚠ **A question, never a delivery.** `/interrupt` judges who may steer before +/// it reads the text, and refuses empty text with a 400 before it touches the +/// turn, the agent or a subagent's pending input (`routes::reply::interrupt`). +/// So the answer is the gate's verdict and nothing else: 400 when this terminal +/// may steer the session as it is, an empty 403 when the daemon wants the key, +/// and a 403 in the daemon's words when no key would help. The daemon's side +/// is pinned by `routes::reply`'s keyed tests and +/// `tests/turn_control_no_user_key.rs`. +fn steer_gate_question(session_id: &str) -> String { + serde_json::json!({ "session_id": session_id, "text": "" }).to_string() +} + +/// One `/reply` attempt, and — when it was refused — the steer gate's answer +/// to the same auth, which says whether the key would change that. +struct ReplyAttempt { + outcome: SendOutcome, + gate: Option<(u16, String)>, +} + +/// `session send` against the daemon on `port`, asking the person for the key +/// only if the daemon wants it (see [`with_key_if_wanted`]). +/// +/// A refused `/reply` wrote nothing (see [`SendOutcome::Forbidden`]), so making +/// it again with the key cannot deliver the text twice. +async fn send_to( + port: u16, + session_id: &str, + text: &str, + wait: bool, + auth: DaemonAuth, + ask: Ask, +) -> Result +where + Ask: FnOnce() -> AskFut, + AskFut: std::future::Future>>, +{ + let body = reply_body(session_id, text); + let question = steer_gate_question(session_id); + let (body, question) = (&body, &question); + let (attempt, verdict, _) = with_key_if_wanted( + auth, + |auth| async move { + let request = build_protected_post_request("/reply", DAEMON_HOST, &auth, body); + let mut stream = connect_to_daemon_at(port).await?; + let outcome = + send_turn(&mut stream, request.as_bytes(), wait, NO_WAIT_DEADLINE).await?; + let gate = match outcome { + SendOutcome::Forbidden => { + Some(post_json_to(port, "/interrupt", question, &auth).await?) + } + _ => None, + }; + Ok(ReplyAttempt { outcome, gate }) + }, + |attempt: &ReplyAttempt| { + attempt + .gate + .as_ref() + .map_or(KeyVerdict::NotAsked, |(code, answer)| { + key_verdict(*code, answer) + }) + }, + ask, + ) + .await?; + match (attempt.outcome, verdict) { + (SendOutcome::Forbidden, KeyVerdict::Wanted) => { + Err(anyhow!("{}", KeyUse::Send.wrong_key())) + } + (SendOutcome::Forbidden, KeyVerdict::Refused(sentence)) => Err(anyhow!( + "the daemon would not start a turn in session {session_id}: {sentence}" + )), + (SendOutcome::Forbidden, KeyVerdict::NotAsked) => Err(anyhow!( + "the daemon refused to start a turn in session {session_id} (HTTP 403) and gave no \ + reason. {}", + KeyUse::Send.undone() )), + (outcome, _) => Ok(outcome), } } @@ -1138,16 +1449,14 @@ pub async fn handle_session_send( wait: bool, user_action_key_stdin: bool, ) -> Result<()> { - let auth = daemon_auth_with_user_action(user_action_key_stdin).await?; - let request = build_user_action_post_request( - "/reply", - DAEMON_HOST, - &auth, - &reply_body(session_id, text), - )?; - // `/reply` streams the turn back, so a send that waits is one request. - let mut stream = connect_to_daemon().await?; - match send_turn(&mut stream, request.as_bytes(), wait, NO_WAIT_DEADLINE).await? { + let auth = with_supplied_key(daemon_auth().await?, user_action_key_stdin).await?; + // `/reply` streams the turn back, so a send that waits is one request — + // two, and a question between them, only when a daemon wants the key. + let outcome = send_to(configured_port(), session_id, text, wait, auth, || { + ask_terminal_for_key(KeyUse::Send) + }) + .await?; + match outcome { SendOutcome::Streamed => {} SendOutcome::Accepted { turn_id } => { match turn_id { @@ -1165,6 +1474,13 @@ pub async fn handle_session_send( "[queued] session {session_id} is a subagent that is still starting; the message \ will be part of its first turn" ), + // `send_to` turns a refusal into its reason before it gets here; this + // is only the wording it would fall back to. + SendOutcome::Forbidden => { + return Err(anyhow!( + "the daemon refused to start a turn in session {session_id} (HTTP 403)" + )) + } } Ok(()) } @@ -1375,12 +1691,31 @@ async fn post_interrupt(session_id: &str, text: &str, auth: &DaemonAuth) -> Resu .to_string(), }), (409, _) => Ok(SteerOutcome::Refused), - (code, _) => Err(anyhow!( + (code, body) => Err(steer_refusal(code, &body, auth)), + } +} + +/// Why a steer was refused, from the answer `/interrupt` gave. +/// +/// Attach settled the key as it joined ([`settle_steering_key`]), so a refusal +/// for want of it here means the daemon changed underneath the attach — most +/// likely restarted with a key it did not hold before. The prompt cannot be +/// offered now, because stdin is the steering channel, so the person is told to +/// attach again rather than asked. +fn steer_refusal(code: u16, body: &str, auth: &DaemonAuth) -> anyhow::Error { + match key_verdict(code, body) { + KeyVerdict::Refused(sentence) => anyhow!("the daemon refused the steer: {sentence}"), + KeyVerdict::Wanted if auth.holds_key() => anyhow!("{}", KeyUse::Steer.wrong_key()), + KeyVerdict::Wanted => anyhow!( + "the daemon now wants a user-action key for steering, which it did not when this \ + attach began; it may have been restarted with one. Detach with ctrl-c and attach \ + again to be asked for it." + ), + KeyVerdict::NotAsked => anyhow!( "the daemon refused the steer: HTTP {code}\n\ (400 means the message was empty; 401 means BIOROUTER_SERVER__SECRET_KEY \ - does not match the daemon's; 403 means the user-action key does not \ - match the daemon's configured digest)" - )), + does not match the daemon's)" + ), } } @@ -1410,7 +1745,7 @@ async fn post_reply_quiet( window: Arc, ) -> Result { let request = - build_user_action_post_request("/reply", DAEMON_HOST, auth, &reply_body(session_id, text))?; + build_protected_post_request("/reply", DAEMON_HOST, auth, &reply_body(session_id, text)); let (status_tx, status_rx) = tokio::sync::oneshot::channel(); // Opened from before the request rather than from the 200: erring towards // warning about a turn that does not exist is harmless, erring the other way @@ -1451,7 +1786,7 @@ async fn post_reply_quiet( Ok(code) => Err(anyhow!( "the daemon refused to start a turn: HTTP {code}\n\ (401 usually means BIOROUTER_SERVER__SECRET_KEY does not match the daemon's; \ - 403 means the user-action key does not match the daemon's configured digest)" + 403 means the daemon will not start a turn in this session for this terminal)" )), // The sender was dropped without a status: no status line was ever // read, so the request failed outright. The holder carries the reason. @@ -1710,6 +2045,57 @@ fn spawn_delivery_worker( send_tx } +/// Before stdin becomes the steering channel: will the daemon on `port` take a +/// steer from this terminal, and does it want the key for one? +/// +/// ⚠ **Asked here, once, and never at the first steer.** A daemon's refusal is +/// the only way to learn it wants the key ([`key_verdict`]), but by the first +/// steer the stdin reader owns stdin, holding its lock for the whole loop. A +/// hidden prompt then would block on that lock; without the lock, the typed key +/// would race the reader and could be delivered to the session as a message. +/// So attach asks as it joins, with the empty steer [`steer_gate_question`] +/// describes, and settles the key before anything reads a line. +/// +/// Returns the auth every later attach request carries: holding the key only +/// when the person supplied it or the daemon wanted it, because a daemon that +/// holds none has nothing to check a key against. +async fn settle_steering_key( + port: u16, + session_id: &str, + auth: DaemonAuth, + ask: Ask, +) -> Result +where + Ask: FnOnce() -> AskFut, + AskFut: std::future::Future>>, +{ + let question = steer_gate_question(session_id); + let question = &question; + let ((code, _), verdict, auth) = with_key_if_wanted( + auth, + |auth| async move { post_json_to(port, "/interrupt", question, &auth).await }, + |(code, answer): &(u16, String)| key_verdict(*code, answer), + ask, + ) + .await?; + match verdict { + KeyVerdict::Wanted => Err(anyhow!("{}", KeyUse::Steer.wrong_key())), + KeyVerdict::Refused(sentence) => Err(anyhow!( + "the daemon will not take a steer from this terminal in session {session_id}: \ + {sentence}\nTo follow the session without steering it: \ + biorouter session attach {session_id} --read-only" + )), + KeyVerdict::NotAsked if code == 401 => Err(anyhow!( + "the daemon refused this terminal: HTTP 401 \ + (BIOROUTER_SERVER__SECRET_KEY does not match the daemon's)" + )), + // 400 is the answer to the question: the gate let this terminal + // through, and only the empty text was refused. Anything else is left to + // the event stream, which reports it in its own terms. + KeyVerdict::NotAsked => Ok(auth), + } +} + /// `biorouter session attach ` — render where the session is, follow it /// live, and steer it from stdin. /// @@ -1729,10 +2115,16 @@ pub async fn handle_session_attach( // lookup. let auth = daemon_auth().await?; let session_id = resolve_attach_target(session_id, name, of).await?; + // Before the stdin reader starts — see `settle_steering_key` for why it + // cannot wait for the first steer. let auth = if read_only { auth } else { - auth_with_user_action(auth, user_action_key_stdin).await? + let auth = with_supplied_key(auth, user_action_key_stdin).await?; + settle_steering_key(configured_port(), &session_id, auth, || { + ask_terminal_for_key(KeyUse::Steer) + }) + .await? }; if read_only { @@ -1760,20 +2152,13 @@ pub async fn handle_session_attach( // The observer stream, exactly as `watch --follow`, except that its first // frame is rendered as a transcript. It is READ-ONLY: its task in the daemon // merely returns when the channel closes and cancels nothing, so detaching - // can never stop the session. - let observer_request = if read_only { - Zeroizing::new(build_get_request( - &format!("/sessions/{session_id}/events"), - DAEMON_HOST, - &auth, - )) - } else { - build_user_action_get_request( - &format!("/sessions/{session_id}/events"), - DAEMON_HOST, - &auth, - )? - }; + // can never stop the session. It carries the key when this attach holds one + // — never with `--read-only`. + let observer_request = build_protected_get_request( + &format!("/sessions/{session_id}/events"), + DAEMON_HOST, + &auth, + ); let observer = stream_request_bytes( observer_request.as_bytes(), Until::Closed, @@ -1881,21 +2266,58 @@ pub(crate) fn render_cancel(response: &serde_json::Value) -> Result { /// `workspace_close scope:"turn"` is the agent's version of the same act, and /// `POST /agent/cancel` is the route the GUI's Stop button already uses. pub async fn handle_session_cancel(session_id: &str, user_action_key_stdin: bool) -> Result<()> { - let auth = daemon_auth_with_user_action(user_action_key_stdin).await?; + let auth = with_supplied_key(daemon_auth().await?, user_action_key_stdin).await?; + let line = cancel_turn(configured_port(), session_id, auth, || { + ask_terminal_for_key(KeyUse::Stop) + }) + .await?; + println!("{line}"); + Ok(()) +} + +/// `session cancel` against the daemon on `port`: the line to print, or why the +/// turn was not stopped. The person is asked for the key only if the daemon +/// wants it (see [`with_key_if_wanted`]); a refused cancel stopped nothing, so +/// making it again with the key is safe. +async fn cancel_turn( + port: u16, + session_id: &str, + auth: DaemonAuth, + ask: Ask, +) -> Result +where + Ask: FnOnce() -> AskFut, + AskFut: std::future::Future>>, +{ let body = serde_json::json!({ "session_id": session_id }).to_string(); - let (code, body) = post_json("/agent/cancel", &body, &auth).await?; + let body = &body; + let ((code, answer), verdict, _) = with_key_if_wanted( + auth, + |auth| async move { post_json_to(port, "/agent/cancel", body, &auth).await }, + |(code, answer): &(u16, String)| key_verdict(*code, answer), + ask, + ) + .await?; + match verdict { + KeyVerdict::Wanted => return Err(anyhow!("{}", KeyUse::Stop.wrong_key())), + KeyVerdict::Refused(sentence) => { + return Err(anyhow!("the daemon would not stop the turn: {sentence}")) + } + KeyVerdict::NotAsked => {} + } if code != 200 { + let said = refusal_sentence(&answer) + .map(|sentence| format!(": {sentence}")) + .unwrap_or_default(); return Err(anyhow!( - "the daemon refused the cancel: HTTP {code}\n\ - (401 usually means BIOROUTER_SERVER__SECRET_KEY does not match the daemon's; \ - 403 means the user-action key does not match the daemon's configured digest)" + "the daemon refused the cancel: HTTP {code}{said}\n\ + (401 usually means BIOROUTER_SERVER__SECRET_KEY does not match the daemon's)" )); } - let response = json_object(&body).ok_or_else(|| { + let response = json_object(&answer).ok_or_else(|| { anyhow!("the daemon answered POST /agent/cancel with a body this client could not read") })?; - println!("{}", render_cancel(&response)?); - Ok(()) + render_cancel(&response) } #[cfg(test)] @@ -2238,26 +2660,675 @@ mod tests { assert!(!ordinary.contains("proof-known-only-to-the-operator")); for path in ["/interrupt", "/agent/cancel", "/reply"] { - let protected = build_user_action_post_request(path, "127.0.0.1", &auth, "{}").unwrap(); + let protected = build_protected_post_request(path, "127.0.0.1", &auth, "{}"); assert!(protected.contains("X-User-Action: proof-known-only-to-the-operator\r\n")); assert!(protected.contains("X-Secret-Key: s3cret\r\n")); assert!(protected.contains("X-Caller-Provider: versa_azure\r\n")); + assert!( + protected.ends_with("\r\n\r\n{}"), + "{path}: the body must follow" + ); } let protected_get = - build_user_action_get_request("/sessions/child/events", "127.0.0.1", &auth).unwrap(); + build_protected_get_request("/sessions/child/events", "127.0.0.1", &auth); assert!(protected_get.contains("X-User-Action: proof-known-only-to-the-operator\r\n")); let ordinary_get = build_get_request("/sessions/child/events", "127.0.0.1", &auth); assert!(!ordinary_get.contains(USER_ACTION_HEADER)); assert!(!ordinary_get.contains("proof-known-only-to-the-operator")); } + /// Without the key, a protected request goes out WITHOUT the header: not + /// refused here, and not sent with an empty one. + /// + /// This replaces a test that pinned the opposite — a builder that refused to + /// make the request at all, which is what made `session cancel` and attach's + /// steering refuse locally against a `biorouter serve` daemon that would have + /// admitted them (serve decision SD-11). The local refusal was never a + /// boundary: the daemon is, and its answer to this very request is how the + /// terminal learns whether it wants the key (`with_key_if_wanted`). #[test] - fn a_protected_post_without_live_operator_proof_fails_closed() { + fn a_protected_request_without_the_key_is_sent_without_the_header() { let auth = DaemonAuth::for_test("s3cret", "versa_azure"); - let error = - build_user_action_post_request("/agent/cancel", "127.0.0.1", &auth, "{}").unwrap_err(); - assert!(error.to_string().contains("user-action key")); + for path in ["/interrupt", "/agent/cancel", "/reply"] { + let request = build_protected_post_request(path, "127.0.0.1", &auth, "{\"a\":1}"); + assert!(!request.contains(USER_ACTION_HEADER), "{path}"); + // …and it is still a whole request. + assert!(request.starts_with(&format!("POST {path} HTTP/1.1\r\n"))); + assert!(request.contains("X-Secret-Key: s3cret\r\n")); + assert!(request.contains("X-Caller-Provider: versa_azure\r\n")); + assert!(request.contains("Content-Length: 7\r\n")); + assert!(request.ends_with("\r\n\r\n{\"a\":1}")); + } + let get = build_protected_get_request("/sessions/child/events", "127.0.0.1", &auth); + assert!(!get.contains(USER_ACTION_HEADER)); + assert!(get.ends_with("Connection: close\r\n\r\n")); + } + + /// A daemon that holds no key refusing in its own words — the shape of + /// `SUBAGENT_CONTROL_NO_KEY` inside `ErrorResponse` — for the tests below. + const KEYLESS_REFUSAL: &str = "This daemon was started without a user-action key, so it \ + cannot verify that a request came from the person at the \ + keyboard. Nothing was changed. This control is unavailable \ + on this daemon; use the desktop app."; + + fn keyless_refusal_body() -> String { + serde_json::json!({ "message": KEYLESS_REFUSAL }).to_string() + } + + /// The two kinds of daemon, told apart from the answer to a stop or a steer + /// sent without the key: the reading that decides whether a person is asked + /// for one at all. + #[test] + fn only_a_keyed_daemons_empty_refusal_asks_for_the_key() { + // A daemon that holds a key, refusing a request without the proof + // (`authorize_turn_control`'s `Unproven` arm). + assert_eq!(key_verdict(403, ""), KeyVerdict::Wanted); + assert_eq!(key_verdict(403, "\r\n"), KeyVerdict::Wanted); + + // A daemon that holds none, in its own words: no key would help. + assert_eq!( + key_verdict(403, &keyless_refusal_body()), + KeyVerdict::Refused(KEYLESS_REFUSAL.to_string()) + ); + // …in plain text too, as the reach gate answers where a route hands its + // refusal back directly. + assert_eq!( + key_verdict( + 403, + "That chat is private, or there is no chat with that id." + ), + KeyVerdict::Refused( + "That chat is private, or there is no chat with that id.".to_string() + ) + ); + + // Nothing that is not a 403 is about the key — including an admitted + // empty steer's 400, which is the answer attach asks for. + for (code, body) in [ + (200, "{\"cancelled\":false}"), + (202, "{\"turn_id\":\"t\"}"), + (400, ""), + (401, ""), + (409, "{}"), + (500, "{\"message\":\"Failed to get session\"}"), + ] { + assert_eq!(key_verdict(code, body), KeyVerdict::NotAsked, "HTTP {code}"); + } + } + + #[test] + fn a_refusal_is_shown_in_the_daemons_own_words() { + assert_eq!( + refusal_sentence("{\"message\":\" No. \"}"), + Some("No.".to_string()) + ); + assert_eq!( + refusal_sentence("No, in plain text.\n"), + Some("No, in plain text.".to_string()) + ); + assert_eq!(refusal_sentence(""), None); + assert_eq!(refusal_sentence(" \r\n"), None); + // An object with no message is shown as it came rather than dropped. + assert_eq!( + refusal_sentence("{\"error\":\"x\"}"), + Some("{\"error\":\"x\"}".to_string()) + ); + } + + /// The person at the terminal, typing `key` — and counting how often they + /// were asked. + fn person_types<'a>( + key: &'static str, + asked: &'a std::cell::Cell, + ) -> impl FnOnce() -> std::future::Ready>> + 'a { + move || { + asked.set(asked.get() + 1); + std::future::ready(Ok(Zeroizing::new(key.to_string()))) + } + } + + /// `with_key_if_wanted` over every answer order a daemon can give, driven + /// through the function itself (as `run_ladder`'s test is): the first + /// request never carries a key the person did not supply; the person is + /// asked only after a refusal for want of one, and at most once; and a key + /// supplied up front is never followed by a prompt. + #[tokio::test] + async fn the_person_is_asked_for_the_key_only_after_the_daemon_wants_it() { + /// One daemon's answers, and what the terminal must do with them. + struct Case { + /// What the daemon answers, one per request, in order. + answers: Vec<(u16, String)>, + /// `--user-action-key-stdin` supplied the key before anything was sent. + supplied: bool, + /// Whether each request in turn carried the key. + held: Vec, + /// How often the person was asked for it. + asked: u32, + verdict: KeyVerdict, + } + + let admitted = (200u16, "{}".to_string()); + let wants_key = (403u16, String::new()); + let in_words = (403u16, keyless_refusal_body()); + let cases = vec![ + // A daemon without a key admits it: one request, no key, nobody asked. + Case { + answers: vec![admitted.clone()], + supplied: false, + held: vec![false], + asked: 0, + verdict: KeyVerdict::NotAsked, + }, + // A daemon with one wants it: asked once, and the second request carries it. + Case { + answers: vec![wants_key.clone(), admitted.clone()], + supplied: false, + held: vec![false, true], + asked: 1, + verdict: KeyVerdict::NotAsked, + }, + // A daemon without a key refuses in words: shown, and nobody is asked. + Case { + answers: vec![in_words], + supplied: false, + held: vec![false], + asked: 0, + verdict: KeyVerdict::Refused(KEYLESS_REFUSAL.to_string()), + }, + // A wrong key: refused again, and the person is not asked twice. + Case { + answers: vec![wants_key.clone(), wants_key.clone()], + supplied: false, + held: vec![false, true], + asked: 1, + verdict: KeyVerdict::Wanted, + }, + // Supplied on stdin: sent at once, and never followed by a prompt — + // not when admitted, and not when refused either. + Case { + answers: vec![admitted], + supplied: true, + held: vec![true], + asked: 0, + verdict: KeyVerdict::NotAsked, + }, + Case { + answers: vec![wants_key], + supplied: true, + held: vec![true], + asked: 0, + verdict: KeyVerdict::Wanted, + }, + ]; + for case in cases { + let held = std::cell::RefCell::new(Vec::::new()); + let asked = std::cell::Cell::new(0); + let auth = if case.supplied { + DaemonAuth::for_test_with_user_action("s3cret", "", "from-stdin") + } else { + DaemonAuth::for_test("s3cret", "") + }; + let answers = &case.answers; + let (_, verdict, auth) = with_key_if_wanted( + auth, + |auth: DaemonAuth| { + let answer = answers[held.borrow().len()].clone(); + held.borrow_mut().push(auth.holds_key()); + async move { Ok(answer) } + }, + |(code, body): &(u16, String)| key_verdict(*code, body), + person_types("typed", &asked), + ) + .await + .unwrap(); + assert_eq!(*held.borrow(), case.held, "answers {answers:?}"); + assert_eq!(asked.get(), case.asked, "answers {answers:?}"); + assert_eq!(verdict, case.verdict, "answers {answers:?}"); + assert_eq!(auth.holds_key(), case.held.last() == Some(&true)); + } + } + + /// With no terminal to ask on, a daemon that wants the key gets no second + /// request, and the error says how to supply it. + #[tokio::test] + async fn with_no_terminal_to_ask_on_the_request_is_not_retried() { + let attempts = std::cell::Cell::new(0); + let err = with_key_if_wanted( + DaemonAuth::for_test("s3cret", ""), + |_auth: DaemonAuth| { + attempts.set(attempts.get() + 1); + async { Ok((403u16, String::new())) } + }, + |(code, body): &(u16, String)| key_verdict(*code, body), + || async { Err(anyhow!("{}", KeyUse::Stop.no_terminal())) }, + ) + .await + // `DaemonAuth` has no `Debug`, on purpose: it holds the secret and the key. + .map(|_| ()) + .unwrap_err() + .to_string(); + assert_eq!(attempts.get(), 1); + assert!(err.contains("--user-action-key-stdin"), "{err}"); + assert!(err.contains("The turn was not stopped."), "{err}"); + } + + /// A stand-in daemon on an ephemeral port. It answers the `GET /status` + /// every command probes first, then each further request, in order, with the + /// next scripted response, and records what it was sent, so a test reads + /// exactly what went over the wire, key header included. Reached by port + /// rather than through `BIOROUTER_PORT`, which every test in this process + /// shares. + struct FakeDaemon { + port: u16, + requests: Arc>>, + } + + impl FakeDaemon { + async fn start(script: Vec) -> Self { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let requests = Arc::new(std::sync::Mutex::new(Vec::new())); + let seen = requests.clone(); + let mut script = std::collections::VecDeque::from(script); + tokio::spawn(async move { + while let Ok((mut socket, _)) = listener.accept().await { + let request = read_one_request(&mut socket).await; + let response = if request.starts_with("GET /status ") { + "HTTP/1.1 200 OK\r\ncontent-length: 0\r\n\r\n".to_string() + } else { + seen.lock().unwrap().push(request); + // Past the script is a 500 no flow here treats as + // success, so an extra request shows up as a failure. + script.pop_front().unwrap_or_else(|| { + http("500 Internal Server Error", "{\"message\":\"unscripted\"}") + }) + }; + let _ = socket.write_all(response.as_bytes()).await; + } + }); + Self { port, requests } + } + + fn requests(&self) -> Vec { + self.requests.lock().unwrap().clone() + } + } + + /// One whole request: the head, then as many body bytes as it declares. + async fn read_one_request(socket: &mut tokio::net::TcpStream) -> String { + let mut raw = Vec::new(); + let mut chunk = [0u8; 4096]; + loop { + let read = socket.read(&mut chunk).await.unwrap_or(0); + if read == 0 { + break; + } + raw.extend_from_slice(&chunk[..read]); + if let Some(end) = raw.windows(4).position(|w| w == b"\r\n\r\n") { + let head = String::from_utf8_lossy(&raw[..end]).to_ascii_lowercase(); + let declared = head + .lines() + .find_map(|line| line.strip_prefix("content-length:")) + .and_then(|value| value.trim().parse::().ok()) + .unwrap_or(0); + if raw.len() >= end + 4 + declared { + break; + } + } + } + String::from_utf8_lossy(&raw).into_owned() + } + + fn http(status: &str, body: &str) -> String { + format!( + "HTTP/1.1 {status}\r\ncontent-type: application/json\r\ncontent-length: {}\r\n\r\n{body}", + body.len() + ) + } + + /// The same answer as a real daemon frames it: chunked, which is what hyper + /// does for these routes. See [`empty_403`]. + fn chunked(status: &str, body: &str) -> String { + format!( + "HTTP/1.1 {status}\r\ncontent-type: application/json\r\ntransfer-encoding: chunked\ + \r\n\r\n{:x}\r\n{body}\r\n0\r\n\r\n", + body.len() + ) + } + + /// `authorize_turn_control`'s refusal on a daemon that holds a key, framed + /// as a real one frames it. + /// + /// ⚠ **Chunked, with a lone `0\r\n\r\n` terminator, because that is what was + /// on the wire.** Captured from a keyed `biorouterd` on 2026-09-11: hyper + /// sends no `content-length` for an empty body. A fixture that used + /// `content-length: 0` passed every test here while the real thing was read + /// as a refusal whose sentence was "0" — so the terminal printed that + /// instead of asking for the key. [`parse_http_response`] is what keeps the + /// framing out of the body. + fn empty_403() -> String { + "HTTP/1.1 403 Forbidden\r\nconnection: close\r\ntransfer-encoding: chunked\r\n\r\n0\r\n\r\n" + .to_string() + } + + /// An empty refusal is an empty refusal however the daemon framed it, and + /// either way it is the answer that asks the person for the key. + #[tokio::test] + async fn an_empty_refusal_reads_as_wanted_however_it_is_framed() { + for empty in [ + empty_403(), + "HTTP/1.1 403 Forbidden\r\ncontent-length: 0\r\n\r\n".to_string(), + // Belt and braces: a chunked body that is genuinely empty, spelled + // with the terminator on its own read. + "HTTP/1.1 403 Forbidden\r\ntransfer-encoding: chunked\r\n\r\n0\r\n\r\n".to_string(), + ] { + let daemon = + FakeDaemon::start(vec![empty.clone(), http("200 OK", "{\"cancelled\":false}")]) + .await; + let asked = std::cell::Cell::new(0); + let line = cancel_turn( + daemon.port, + "20260911_4", + DaemonAuth::for_test("s3cret", ""), + person_types("the-typed-key", &asked), + ) + .await + .unwrap_or_else(|err| panic!("framing {empty:?} was not read as an empty 403: {err}")); + assert_eq!( + line, + "nothing to cancel: this session had no turn in flight" + ); + assert_eq!(asked.get(), 1, "framing {empty:?}"); + } + } + + /// The regression SD-11 left behind, end to end over a socket: on a daemon + /// that holds no key, `session cancel` goes out without one, the daemon + /// admits it, and the person is never asked for a key that does not exist. + #[tokio::test] + async fn cancel_on_a_daemon_without_a_key_never_asks_for_one() { + let daemon = FakeDaemon::start(vec![http( + "200 OK", + "{\"cancelled\":true,\"turn_id\":\"turn-3\"}", + )]) + .await; + let asked = std::cell::Cell::new(0); + let line = cancel_turn( + daemon.port, + "20260911_4", + DaemonAuth::for_test("s3cret", ""), + person_types("never-typed", &asked), + ) + .await + .unwrap(); + assert_eq!(line, "cancelled turn turn-3"); + assert_eq!( + asked.get(), + 0, + "a daemon that admitted the request was not asked about" + ); + let requests = daemon.requests(); + assert_eq!(requests.len(), 1, "{requests:?}"); + assert!(requests[0].starts_with("POST /agent/cancel HTTP/1.1\r\n")); + assert!(requests[0].ends_with("{\"session_id\":\"20260911_4\"}")); + assert!(!requests[0].contains(USER_ACTION_HEADER)); + } + + /// A daemon that holds a key: the first request asks it, the person is + /// asked once, and the key goes on the second request and nowhere else. + #[tokio::test] + async fn cancel_on_a_daemon_with_a_key_asks_once_and_sends_it() { + let daemon = FakeDaemon::start(vec![ + empty_403(), + http("200 OK", "{\"cancelled\":false,\"turn_id\":null}"), + ]) + .await; + let asked = std::cell::Cell::new(0); + let line = cancel_turn( + daemon.port, + "20260911_4", + DaemonAuth::for_test("s3cret", ""), + person_types("the-typed-key", &asked), + ) + .await + .unwrap(); + assert_eq!( + line, + "nothing to cancel: this session had no turn in flight" + ); + assert_eq!(asked.get(), 1); + let requests = daemon.requests(); + assert_eq!(requests.len(), 2, "{requests:?}"); + assert!(!requests[0].contains(USER_ACTION_HEADER)); + assert!(!requests[0].contains("the-typed-key")); + assert!(requests[1].contains("X-User-Action: the-typed-key\r\n")); + } + + /// A refusal no key can change — a subagent's session on a daemon that holds + /// none — is shown in the daemon's words, and nobody is asked for a key. + #[tokio::test] + async fn a_refusal_no_key_can_change_is_shown_rather_than_prompted_for() { + // Chunked, as a real daemon sends it: the sentence must survive the + // framing intact, with no chunk sizes in it. + let daemon = + FakeDaemon::start(vec![chunked("403 Forbidden", &keyless_refusal_body())]).await; + let asked = std::cell::Cell::new(0); + let err = cancel_turn( + daemon.port, + "20260911_5", + DaemonAuth::for_test("s3cret", ""), + person_types("never-typed", &asked), + ) + .await + .unwrap_err() + .to_string(); + assert!(err.contains(KEYLESS_REFUSAL), "{err}"); + assert!( + !err.contains("does not match"), + "a keyless refusal is not a key mismatch: {err}" + ); + assert_eq!(asked.get(), 0); + assert_eq!(daemon.requests().len(), 1); + } + + /// A key the daemon does not recognise is reported as wrong, whether it was + /// typed or supplied on stdin, and is never answered with another prompt. + #[tokio::test] + async fn a_wrong_key_is_reported_as_wrong_and_not_asked_for_again() { + let daemon = FakeDaemon::start(vec![empty_403(), empty_403()]).await; + let asked = std::cell::Cell::new(0); + let err = cancel_turn( + daemon.port, + "20260911_4", + DaemonAuth::for_test("s3cret", ""), + person_types("a-wrong-key", &asked), + ) + .await + .unwrap_err() + .to_string(); + assert!( + err.contains("not the key this daemon was started with"), + "{err}" + ); + assert!(err.contains("The turn was not stopped."), "{err}"); + assert_eq!(asked.get(), 1); + assert_eq!(daemon.requests().len(), 2); + + let daemon = FakeDaemon::start(vec![empty_403()]).await; + let err = cancel_turn( + daemon.port, + "20260911_4", + DaemonAuth::for_test_with_user_action("s3cret", "", "a-wrong-key"), + person_types("never-typed", &asked), + ) + .await + .unwrap_err() + .to_string(); + assert!( + err.contains("not the key this daemon was started with"), + "{err}" + ); + assert_eq!( + asked.get(), + 1, + "a supplied key is never followed by a prompt" + ); + let requests = daemon.requests(); + assert_eq!(requests.len(), 1); + assert!(requests[0].contains("X-User-Action: a-wrong-key\r\n")); + } + + /// Attach asks before it reads a single line, with an empty steer, and goes + /// on without a key where the daemon admits it; asks for the key once where + /// the daemon wants it, checking the key before anything is typed; and stops + /// with the daemon's words, and the way to watch instead, where no key would + /// help. + #[tokio::test] + async fn attach_settles_the_key_with_an_empty_steer_before_it_reads_stdin() { + // A daemon without a key: the gate lets this terminal through and only + // the empty text is refused. + let daemon = FakeDaemon::start(vec![http("400 Bad Request", "")]).await; + let asked = std::cell::Cell::new(0); + let auth = settle_steering_key( + daemon.port, + "20260911_6", + DaemonAuth::for_test("s3cret", ""), + person_types("never-typed", &asked), + ) + .await + .unwrap(); + assert!(!auth.holds_key()); + assert_eq!(asked.get(), 0); + let requests = daemon.requests(); + assert_eq!(requests.len(), 1, "{requests:?}"); + assert!(requests[0].starts_with("POST /interrupt HTTP/1.1\r\n")); + assert!( + requests[0].ends_with("{\"session_id\":\"20260911_6\",\"text\":\"\"}"), + "the question carries no text to deliver: {}", + requests[0] + ); + assert!(!requests[0].contains(USER_ACTION_HEADER)); + + // A daemon with a key: asked once, and the key is tried on the same + // question before attach reads anything. + let daemon = FakeDaemon::start(vec![empty_403(), http("400 Bad Request", "")]).await; + let auth = settle_steering_key( + daemon.port, + "20260911_6", + DaemonAuth::for_test("s3cret", ""), + person_types("the-typed-key", &asked), + ) + .await + .unwrap(); + assert!(auth.holds_key()); + assert_eq!(asked.get(), 1); + assert!(daemon.requests()[1].contains("X-User-Action: the-typed-key\r\n")); + + // A daemon without a key, and a session it will not let this terminal + // steer: no prompt, its words, and `--read-only`. + let daemon = FakeDaemon::start(vec![http("403 Forbidden", &keyless_refusal_body())]).await; + let err = settle_steering_key( + daemon.port, + "20260911_7", + DaemonAuth::for_test("s3cret", ""), + person_types("never-typed", &asked), + ) + .await + .map(|_| ()) + .unwrap_err() + .to_string(); + assert!(err.contains(KEYLESS_REFUSAL), "{err}"); + assert!( + err.contains("biorouter session attach 20260911_7 --read-only"), + "{err}" + ); + assert_eq!(asked.get(), 1, "not asked again"); + } + + /// A turn's whole stream, as `/reply` sends it when a `send` waits. + fn a_whole_turn() -> String { + "HTTP/1.1 200 OK\r\ncontent-type: text/event-stream\r\n\r\n\ + data: {\"type\":\"Finish\",\"reason\":\"stop\",\"seq\":1,\"turn_id\":\"turn-8\"}\n\n" + .to_string() + } + + /// `send` makes one proof-less `/reply`, and only its refusal leads anywhere + /// else: to the steer gate, which says whether the key would change it, and + /// on to the person only when it would. + #[tokio::test] + async fn send_asks_for_the_key_only_when_a_daemon_holding_one_refuses() { + let asked = std::cell::Cell::new(0); + + // An ordinary chat, on either kind of daemon: one request, no key. + let daemon = FakeDaemon::start(vec![a_whole_turn()]).await; + let outcome = send_to( + daemon.port, + "20260911_8", + "hello", + true, + DaemonAuth::for_test("s3cret", ""), + person_types("never-typed", &asked), + ) + .await + .unwrap(); + assert_eq!(outcome, SendOutcome::Streamed); + assert_eq!(asked.get(), 0); + let requests = daemon.requests(); + assert_eq!(requests.len(), 1, "{requests:?}"); + assert!(requests[0].starts_with("POST /reply HTTP/1.1\r\n")); + assert!(!requests[0].contains(USER_ACTION_HEADER)); + + // A subagent's session on a daemon that holds a key: the refusal, the + // question, the person, and the same text again with the key. + let daemon = FakeDaemon::start(vec![empty_403(), empty_403(), a_whole_turn()]).await; + let outcome = send_to( + daemon.port, + "20260911_9", + "hello", + true, + DaemonAuth::for_test("s3cret", ""), + person_types("the-typed-key", &asked), + ) + .await + .unwrap(); + assert_eq!(outcome, SendOutcome::Streamed); + assert_eq!(asked.get(), 1); + let requests = daemon.requests(); + assert_eq!(requests.len(), 3, "{requests:?}"); + assert!(requests[0].starts_with("POST /reply ")); + assert!(requests[1].starts_with("POST /interrupt ")); + assert!(requests[2].starts_with("POST /reply ")); + assert!(!requests[0].contains(USER_ACTION_HEADER)); + assert!(!requests[1].contains(USER_ACTION_HEADER)); + assert!(requests[2].contains("X-User-Action: the-typed-key\r\n")); + assert!(requests[2].contains("\"text\":\"hello\"")); + + // A subagent's session on a daemon that holds none: `/reply`'s empty 403 + // cannot say which kind of daemon this is, and the gate's words can. + let daemon = FakeDaemon::start(vec![ + empty_403(), + http("403 Forbidden", &keyless_refusal_body()), + ]) + .await; + let err = send_to( + daemon.port, + "20260911_9", + "hello", + true, + DaemonAuth::for_test("s3cret", ""), + person_types("never-typed", &asked), + ) + .await + .unwrap_err() + .to_string(); + assert!(err.contains(KEYLESS_REFUSAL), "{err}"); + assert_eq!( + asked.get(), + 1, + "not asked for a key no daemon here can check" + ); + assert_eq!(daemon.requests().len(), 2); } /// Issue #56 — **every** daemon request states the capability this terminal diff --git a/crates/biorouter-server/src/routes/agent.rs b/crates/biorouter-server/src/routes/agent.rs index e9f761dc9..172dde44b 100644 --- a/crates/biorouter-server/src/routes/agent.rs +++ b/crates/biorouter-server/src/routes/agent.rs @@ -70,17 +70,37 @@ pub struct UpdateFromSessionRequest { const SUBAGENT_USER_ACTION_REQUIRED: &str = "Changing or resuming a subagent from its tab requires proof that the request came from the person at the keyboard."; +/// …and when the daemon holds no user-action key at all. +/// +/// A separate sentence, per Task 18A's open question 23 and SD-8: telling a +/// person at a `biorouter serve` page that their request "requires proof" sends +/// them hunting for a permission this daemon can never grant anyone. It names +/// the daemon as the reason, in the register `CROSS_AFFILIATION_GRANT_NO_KEY` +/// and `session_reach::SESSION_REACH_NO_KEY` already use. Since SD-11 this is +/// also what a keyless daemon answers a Stop or a steer aimed at a subagent's +/// turn, because those routes gate through [`authorize_agent_control`] there. +const SUBAGENT_CONTROL_NO_KEY: &str = + "This daemon was started without a user-action key, so it cannot verify that a request came \ + from the person at the keyboard, and changing, resuming, stopping or steering a subagent from \ + its tab requires that proof. Nothing was changed. This control is unavailable on this \ + daemon; use the desktop app."; + fn refuse_subagent_unless_user( session: &Session, headers: &HeaderMap, ) -> Result<(), ErrorResponse> { - if session.session_type == SessionType::SubAgent && !is_user_action(headers) { - return Err(ErrorResponse { - message: SUBAGENT_USER_ACTION_REQUIRED.to_string(), - status: StatusCode::FORBIDDEN, - }); + if session.session_type != SessionType::SubAgent { + return Ok(()); } - Ok(()) + let message = match user_action_proof(headers) { + UserActionProof::Proven => return Ok(()), + UserActionProof::Unproven => SUBAGENT_USER_ACTION_REQUIRED, + UserActionProof::NoKeyInstalled => SUBAGENT_CONTROL_NO_KEY, + }; + Err(ErrorResponse { + message: message.to_string(), + status: StatusCode::FORBIDDEN, + }) } #[async_trait::async_trait] @@ -146,7 +166,14 @@ async fn read_update_session( /// Authorize an HTTP control-plane operation before it can touch an agent or a /// queued child handle. The daemon bearer proves only that the caller reached /// this process; it does not prove that a person chose to mutate a subagent. -async fn authorize_agent_control( +/// +/// ⚠ **Also the turn-control gate on a daemon with no user-action key** (SD-11): +/// `routes::reply`'s `authorize_turn_control` calls this for `/agent/cancel`, +/// `/interrupt` and the two continuation routes there, so that stopping a turn +/// admits exactly the callers `/agent/stop` admits. Tightening this therefore +/// tightens those four too, which is the point — but it is a change to who may +/// press Stop in a browser, and `tests/turn_control_no_user_key.rs` will say so. +pub(crate) async fn authorize_agent_control( state: &AppState, session_id: &str, headers: &HeaderMap, diff --git a/crates/biorouter-server/src/routes/reply.rs b/crates/biorouter-server/src/routes/reply.rs index ed2823699..c905a5bed 100644 --- a/crates/biorouter-server/src/routes/reply.rs +++ b/crates/biorouter-server/src/routes/reply.rs @@ -16,6 +16,10 @@ use biorouter::conversation::Conversation; use biorouter::privacy::SessionClassification; use biorouter::session::session_manager::ReplaceOutcome; use biorouter::session::SessionManager; +// Through the LIB path, as every route names the user-action digest: `src/routes/` +// is compiled into the `biorouterd` binary too, where `crate::auth` does not +// exist, and the digest must be the lib's one static. +use biorouter_server::auth::{user_action_proof, UserActionProof}; use bytes::Bytes; use futures::Stream; use rmcp::model::ServerNotification; @@ -1753,6 +1757,78 @@ pub struct InterruptAccepted { pub turn_id: String, } +/// On whose authority a turn-control request was admitted — the answer +/// [`authorize_turn_control`] gives its four routes. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum TurnControlAuthority { + /// The request carried the user-action proof: a person acted. The only + /// authority a daemon that holds a key accepts, and the only one under which + /// a steer is stamped as typed by a person. + Person, + /// A daemon that holds no key admitted the caller by the gate `/agent/stop` + /// already applies there (SD-11). Nothing establishes who the caller is, so + /// nothing it sends is attributed to a person. + Reach, +} + +/// May this request stop, steer or settle a turn in the chat it names? +/// +/// The gate of `POST /agent/cancel`, `POST /interrupt`, +/// `POST /agent/continuation/abandon` and `POST /agent/continuation/recover`, +/// asked before any of them touches the turn. +/// +/// * **A daemon that holds a user-action key** — the desktop application's — +/// takes the proof and nothing else, exactly as before; `Unproven` is the +/// empty 403 these routes have always answered. +/// * **A daemon that holds none** — the one `biorouter serve` starts (SD-7), or +/// a `biorouterd` started by hand — asks what `/agent/stop` asks there: +/// `routes::agent::authorize_agent_control`, the reach gate and then the +/// subagent rule. The same call, not a copy of it, so the two routes can never +/// disagree about who may stop a turn. +/// +/// ⚠ **Why the keyless arm no longer refuses** (serve decision SD-11). On such a +/// daemon the proof can only refuse everyone, the person at the browser +/// included: measured 2026-09-11, the Stop button answered 403 on every chat of +/// a `serve` host. And the refusal protected nothing. The proof is here so that +/// a model holding the daemon secret, which AR-11 found recoverable, cannot stop +/// another chat's turn or put words in a person's mouth; on a keyless daemon the +/// same caller already stops that turn through `/agent/stop` (this very gate) or, +/// as a model, `workspace_close { scope: "turn" }`, and already puts text in +/// front of that chat's model through `/reply`. Admitting it here gives no caller +/// a capability it lacked. +/// +/// ⚠ **Why a daemon that holds a key is not relaxed with it.** There the proof +/// costs the person nothing — the renderer attaches it to every request — and it +/// is what licenses the `UserDirect` stamp a steer carries. +/// +/// ⚠ **The two refusals' SHAPES are read by the terminal.** `biorouter session` +/// cannot ask a daemon whether it holds a key, so it sends a stop or a steer +/// without the proof and reads the answer (`commands/session_watch.rs`, +/// `key_verdict`): the `Unproven` arm's EMPTY 403 is the only thing that makes it +/// ask the person for the key, and a refusal carrying a sentence — every one the +/// keyless arm can give — is shown instead, because no key would change it. So +/// a sentence added to `Unproven` would stop the terminal asking for the key on +/// the desktop's daemon, and an empty refusal on the keyless arm would make it +/// ask a `serve` user for a key that does not exist. Both are pinned: the keyed +/// side here in `integration_tests`, the keyless side in +/// `tests/turn_control_no_user_key.rs`. +async fn authorize_turn_control( + state: &AppState, + session_id: &str, + headers: &HeaderMap, +) -> Result { + match user_action_proof(headers) { + UserActionProof::Proven => Ok(TurnControlAuthority::Person), + UserActionProof::Unproven => Err(StatusCode::FORBIDDEN.into_response()), + UserActionProof::NoKeyInstalled => { + crate::routes::agent::authorize_agent_control(state, session_id, headers) + .await + .map(|_| TurnControlAuthority::Reach) + .map_err(IntoResponse::into_response) + } + } +} + /// Soft interrupt: queue a user message to be injected into the session's /// running turn at the next safe loop boundary, instead of cancelling the turn /// and re-sending the whole context. Returns 202 Accepted; the message surfaces @@ -1779,7 +1855,9 @@ pub struct InterruptAccepted { responses( (status = 202, description = "Message queued for injection into the running turn", body = InterruptAccepted), (status = 400, description = "Empty message text"), - (status = 403, description = "The request was not proven to come from the user"), + (status = 403, description = "The request was not proven to come from the user; on a daemon \ + that holds no user-action key, the chat is out of the caller's \ + reach or is a subagent's (SD-11)"), (status = 409, description = "No turn is accepting interrupts for this session"), (status = 500, description = "Internal server error") ) @@ -1788,14 +1866,17 @@ pub async fn interrupt( State(state): State>, headers: HeaderMap, Json(req): Json, -) -> Result<(StatusCode, Json), StatusCode> { - if !biorouter_server::auth::is_user_action(&headers) { - return Err(StatusCode::FORBIDDEN); - } +) -> Result<(StatusCode, Json), axum::response::Response> { + let authority = authorize_turn_control(&state, &req.session_id, &headers).await?; if req.text.trim().is_empty() { - return Err(StatusCode::BAD_REQUEST); + return Err(StatusCode::BAD_REQUEST.into_response()); } - if let Some(turn_id) = req.turn_id.clone() { + // Only a person's steer is held for a delegated child that has not started: + // the queue stamps it `UserDirect`, which tells that child's parent a human + // intervened. A keyless daemon's caller never gets here — a child's chat is + // outside its reach (SD-11) — and the condition says so locally rather than + // leaving it to be re-derived from the gate. + if let (TurnControlAuthority::Person, Some(turn_id)) = (authority, req.turn_id.clone()) { let queued_message = crate::workspace::turn::stamp_user_direct_if_subagent( Message::user() .with_id(turn_id.clone()) @@ -1817,18 +1898,28 @@ pub async fn interrupt( // Cheap early-out only: it avoids constructing an agent for an idle session. // It is no longer the guard — see `try_queue_soft_interrupt` below. if !state.is_turn_active(&req.session_id) { - return Err(StatusCode::CONFLICT); + return Err(StatusCode::CONFLICT.into_response()); } - // User-action authentication above is the authority for this attribution. - // Keep it independent of the session store: a live agent can legitimately - // outlast or race its durable row, but an accepted human steer must never - // lose its provenance because that auxiliary lookup failed. - let provenance = Some(biorouter::conversation::message::MessageProvenance { - kind: biorouter::conversation::message::ProvenanceKind::UserDirect, - from_session_id: None, - from_session_name: None, - }); - let agent = state.get_agent_for_route(req.session_id).await?; + let provenance = match authority { + // User-action authentication above is the authority for this + // attribution. Keep it independent of the session store: a live agent + // can legitimately outlast or race its durable row, but an accepted human + // steer must never lose its provenance because that auxiliary lookup + // failed. + TurnControlAuthority::Person => Some(biorouter::conversation::message::MessageProvenance { + kind: biorouter::conversation::message::ProvenanceKind::UserDirect, + from_session_id: None, + from_session_name: None, + }), + // SD-11: nothing on a keyless daemon establishes that a person typed + // this, so it claims nothing — which is also how `/reply` records the + // same caller's message in the same chat. + TurnControlAuthority::Reach => None, + }; + let agent = state + .get_agent_for_route(req.session_id) + .await + .map_err(IntoResponse::into_response)?; match agent.try_queue_soft_interrupt(req.text, provenance) { Ok(turn_id) => Ok(( StatusCode::ACCEPTED, @@ -1838,7 +1929,7 @@ pub async fn interrupt( )), // #69: the turn the caller addressed has ended. Refusing is the honest // answer — queueing for whatever runs next is the bug this replaces. - Err(InterruptRefused::TurnEnded) => Err(StatusCode::CONFLICT), + Err(InterruptRefused::TurnEnded) => Err(StatusCode::CONFLICT.into_response()), } } @@ -2052,7 +2143,9 @@ const CANCEL_SETTLEMENT_TIMEOUT: Duration = Duration::from_secs(30); (status = 200, description = "Cancel processed; `cancelled` reports whether a turn was running", body = CancelTurnResponse), (status = 400, description = "Stop-and-Send requires an exact turn id and a valid stable continuation owner id"), (status = 401, description = "Unauthorized - invalid secret key"), - (status = 403, description = "The request was not proven to come from the user"), + (status = 403, description = "The request was not proven to come from the user; on a daemon \ + that holds no user-action key, the chat is out of the caller's \ + reach or is a subagent's (SD-11)"), (status = 409, description = "A different turn generation is active, another client owns the continuation, or its admission is still settling", body = CancelTurnConflict), (status = 504, description = "The cancelled turn did not release the session lock before the safety bound"), (status = 500, description = "Internal server error") @@ -2063,8 +2156,8 @@ pub async fn cancel_turn( headers: HeaderMap, Json(req): Json, ) -> axum::response::Response { - if !biorouter_server::auth::is_user_action(&headers) { - return StatusCode::FORBIDDEN.into_response(); + if let Err(refusal) = authorize_turn_control(&state, &req.session_id, &headers).await { + return refusal; } match cancel_turn_bounded(&state, &req, CANCEL_SETTLEMENT_TIMEOUT).await { Ok(response) => Json(response).into_response(), @@ -2079,7 +2172,9 @@ pub async fn cancel_turn( request_body = AbandonContinuationLeaseRequest, responses( (status = 200, description = "The continuation lease is abandoned or was already resolved", body = AbandonContinuationLeaseResponse), - (status = 403, description = "The request was not proven to come from the user"), + (status = 403, description = "The request was not proven to come from the user; on a daemon \ + that holds no user-action key, the chat is out of the caller's \ + reach or is a subagent's (SD-11)"), (status = 409, description = "The lease is invalid or belongs to another session", body = ContinuationLeaseErrorResponse) ) )] @@ -2088,8 +2183,8 @@ pub async fn abandon_continuation_lease( headers: HeaderMap, Json(req): Json, ) -> axum::response::Response { - if !biorouter_server::auth::is_user_action(&headers) { - return StatusCode::FORBIDDEN.into_response(); + if let Err(refusal) = authorize_turn_control(&state, &req.session_id, &headers).await { + return refusal; } match state.abandon_continuation_lease(&req.session_id, &req.continuation_lease) { Ok(resolution) => { @@ -2123,7 +2218,9 @@ fn valid_continuation_owner_id(owner_id: &str) -> bool { responses( (status = 200, description = "The exact pending continuation was taken over or its whole claim group was abandoned", body = RecoverContinuationResponse), (status = 400, description = "The continuation owner id is missing or invalid"), - (status = 403, description = "The session is out of reach or the request was not proven to come from the user"), + (status = 403, description = "The session is out of reach or the request was not proven to \ + come from the user; on a daemon that holds no user-action key, \ + the chat is out of the caller's reach or is a subagent's (SD-11)"), (status = 409, description = "The exact continuation generation was already resolved", body = ContinuationLeaseErrorResponse) ) )] @@ -2132,12 +2229,16 @@ pub async fn recover_continuation( headers: HeaderMap, Json(req): Json, ) -> axum::response::Response { - if !biorouter_server::auth::is_user_action(&headers) { - return StatusCode::FORBIDDEN.into_response(); + if let Err(refusal) = authorize_turn_control(&state, &req.session_id, &headers).await { + return refusal; } if !valid_continuation_owner_id(&req.continuation_owner_id) { return StatusCode::BAD_REQUEST.into_response(); } + // Asked again on a keyless daemon, where `authorize_turn_control` has already + // asked it: this is the route's own reach gate on EVERY daemon, and a second + // answer to the same question is the same answer. It stays where the + // ordering census (`session_reach::every_gated_route_resolves_…`) pins it. if let Err(refusal) = crate::routes::session_reach::session_reach( state.session_manager(), &req.session_id, @@ -4616,6 +4717,58 @@ mod tests { ); } + /// The keyed half of what `biorouter session` reads before it asks a + /// person for the key (`commands/session_watch.rs::key_verdict`). On a + /// daemon that holds one, turn control refuses a request without the + /// proof with an EMPTY 403 — the one refusal a daemon without a key + /// never gives (`tests/turn_control_no_user_key.rs`) — and refuses it + /// before it reads the text, so the empty steer the terminal asks with + /// is answered by the gate, not by the text check. With the proof, the + /// same empty steer is the 400 the terminal takes as "this key opens + /// the gate". None of it touches the turn or the queue. + #[tokio::test(flavor = "multi_thread")] + async fn an_unproven_stop_or_steer_is_refused_empty_before_its_text_is_read() { + install_test_user_action_key(); + let state = AppState::new().await.unwrap(); + let token = CancellationToken::new(); + let _guard = state + .try_begin_turn_idempotent("keyed-question", token.clone(), None) + .expect("turn lock acquired"); + let agent = state.get_agent("keyed-question".to_string()).await.unwrap(); + agent.open_for_turn(biorouter::agents::TurnId::new("agent-turn-keyed-question")); + + for mut request in [ + interrupt_request("keyed-question", ""), + interrupt_request("keyed-question", "pretend the user said this"), + cancel_request("keyed-question"), + ] { + let route = request.uri().path().to_string(); + request.headers_mut().remove("X-User-Action"); + let response = routes(Arc::clone(&state)).oneshot(request).await.unwrap(); + assert_eq!(response.status(), StatusCode::FORBIDDEN, "{route}"); + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + body.is_empty(), + "{route}: a sentence here reads to the terminal as a refusal no key can \ + change, so it would never ask for the key: {}", + String::from_utf8_lossy(&body) + ); + } + + let response = routes(Arc::clone(&state)) + .oneshot(interrupt_request("keyed-question", "")) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + assert!(!token.is_cancelled(), "a refused Stop reached the turn"); + assert!( + !agent.has_soft_interrupts(), + "the terminal's question reached the agent's queue" + ); + } + /// #69: the turn lock is still held — the reply task has not unwound yet — /// but the loop has performed its final drain and committed to exiting. /// The old route read only the lock and returned 202, and the text then diff --git a/crates/biorouter-server/src/routes/session_reach.rs b/crates/biorouter-server/src/routes/session_reach.rs index 09dbb875e..30667bb1e 100644 --- a/crates/biorouter-server/src/routes/session_reach.rs +++ b/crates/biorouter-server/src/routes/session_reach.rs @@ -24,7 +24,8 @@ //! inert there; //! * it still reaches every session-addressing route NOT on //! [the gated list](self#the-gated-list). `POST /interrupt` and `POST -//! /agent/cancel` now require user-action proof; `GET +//! /agent/cancel` require user-action proof on a daemon that holds a key, and +//! are on the list on one that does not (SD-11); `GET //! /sessions/{id}/extensions`, `GET /sessions/{id}/usage`, `PUT //! /sessions/{id}/name`, `PUT /sessions/{id}/user_workflow_values` and //! `DELETE /sessions/{id}` remain open, as do `GET /active_work` and `POST @@ -149,15 +150,17 @@ //! | `POST /agent/continuation/recover` | Resumes a parked continuation in the named session. Gates directly. | //! | `POST /agent/update_from_session` | Adopts another session's provider configuration. Gates directly. | //! | `POST /agent/update_provider` · `restart` · `stop` · `remove_extension` | Gate through [`authorize_agent_control`](../agent/fn.authorize_agent_control.html), which calls [`session_reach`] and then reads the row. | +//! | `POST /agent/cancel` · `/interrupt` · `/agent/continuation/abandon` | Stop, steer and settle the named session's turn. **On a daemon that holds no user-action key only** (serve decision SD-11): there `routes::reply::authorize_turn_control` gates them through the same `authorize_agent_control` as the row above, so a Stop admits exactly the callers `/agent/stop` does. A daemon that holds a key asks them for the proof instead, which reaches every chat. | //! -//! ⚠ **Two spellings, one list.** The last row reaches the gate through a helper -//! rather than by naming it, which is why a scan for the literal `session_reach(` -//! reports those four as ungated and why the ordering test below uses two of them -//! as over-read controls. They are NOT exempt — measured live, each answers 403 -//! without the capability header and proceeds with it. A future sweep that greps -//! for the call must follow `authorize_agent_control` too, or it will "discover" -//! four holes that are not there and, worse, trust the same grep when it reports -//! a real one. +//! ⚠ **Two spellings, one list.** The last two rows reach the gate through a +//! helper rather than by naming it, which is why a scan for the literal +//! `session_reach(` reports those seven as ungated and why the ordering test +//! below uses two of them as over-read controls. They are NOT exempt — measured +//! live, each of the first four answers 403 without the capability header and +//! proceeds with it, and `tests/turn_control_no_user_key.rs` measures the other +//! three on a keyless daemon. A future sweep that greps for the call must follow +//! `authorize_agent_control` too, or it will "discover" seven holes that are not +//! there and, worse, trust the same grep when it reports a real one. //! //! # Why `X-User-Action` and not a new mechanism, for the proof half //! @@ -1063,11 +1066,13 @@ mod tests { /// unit test — `AppState::new()` opens the developer's REAL session /// database. Every route on the list is also driven over HTTP by /// [`super::bypass_tests`] except `POST /agent/add_extension` (whose admitted - /// arm mints a real agent) and `GET|POST /knowledge/active` (a middleware, which + /// arm mints a real agent), `GET|POST /knowledge/active` (a middleware, which /// a body scan cannot see and /// [`super::bypass_tests::the_knowledge_active_gate_is_actually_wired`] - /// drives instead); this is what holds the ORDERING, which no status code - /// can show. + /// drives instead) and the three SD-11 turn-control routes, which are gated + /// only on a daemon with no user-action key and so are driven by their own + /// keyless binary, `tests/turn_control_no_user_key.rs`; this is what holds + /// the ORDERING, which no status code can show. /// /// ⚠ **Every route added to the gated list gets a row here.** `/export`, /// `/events` and `/diagnostics` each shipped a gate that this table did not @@ -1095,6 +1100,38 @@ mod tests { "recover_continuation_for_owner(", "the pending continuation ownership state", ), + // SD-11: the turn-control routes. Their gate is a helper, because on + // a daemon that holds a key the answer is the proof and on one that + // holds none it is `authorize_agent_control` — the ordering is the + // same either way, and it is what is asserted here. + ( + reply_rs, + "pub async fn recover_continuation", + "authorize_turn_control(", + "recover_continuation_for_owner(", + "the pending continuation ownership state", + ), + ( + reply_rs, + "pub async fn interrupt", + "authorize_turn_control(", + "queue_initializing_child_input(", + "a delegated child's pending-input queue, and the live agent's after it", + ), + ( + reply_rs, + "pub async fn cancel_turn(", + "authorize_turn_control(", + "cancel_turn_bounded(", + "the turn registry, whose answer says whether this chat is busy", + ), + ( + reply_rs, + "pub async fn abandon_continuation_lease", + "authorize_turn_control(", + "state.abandon_continuation_lease(", + "the continuation registry", + ), ( session_rs, "async fn get_session(", @@ -1164,17 +1201,19 @@ mod tests { // reads the row — and measured live against a private session each // answers 403 without the capability header and proceeds with it. They // are controls for the EXTRACTOR, not exemptions from the gate, and the - // comment here said otherwise until 2026-09-04. `interrupt` and - // `get_session_extensions` are the genuinely ungated pair: `interrupt` - // requires the user's proof instead, and `get_session_extensions` is on - // the module header's open residual. + // comment here said otherwise until 2026-09-04. `get_session_extensions` + // is genuinely ungated: it is on the module header's open residual. + // `interrupt` was this file's other control until SD-11 put it on the + // list above; `reply.rs`'s controls are now two functions that are not + // handlers at all, on either side of the six rows it contributes. // // BOTH sides in `agent.rs`: `agent_remove_extension` sits after the two // gated handlers' neighbourhood and `update_agent_provider` before it, // and a control on one side only passes against an extractor that // over-reads towards the other. for (src, control) in [ - (reply_rs, "pub async fn interrupt"), + (reply_rs, "fn attach_names_a_missing_turn("), + (reply_rs, "pub fn routes("), (session_rs, "async fn get_session_extensions"), (agent_rs, "async fn agent_remove_extension"), (agent_rs, "async fn update_agent_provider"), @@ -1187,11 +1226,16 @@ mod tests { (status_rs, "async fn system_info("), (status_rs, "pub fn routes("), ] { - assert!( - !body_of(src, control).contains("session_reach("), - "the body scan is over-reading: {control} is not on the gated list and \ - reported the gate" - ); + // Both spellings the rows above use, so a control is a control for + // every row it could be over-reading into. + let body = body_of(src, control); + for gate in ["session_reach(", "authorize_turn_control("] { + assert!( + !body.contains(gate), + "the body scan is over-reading: {control} is not on the gated list and \ + reported the gate (`{gate}`)" + ); + } } } diff --git a/crates/biorouter-server/tests/turn_control_no_user_key.rs b/crates/biorouter-server/tests/turn_control_no_user_key.rs new file mode 100644 index 000000000..1180d0d44 --- /dev/null +++ b/crates/biorouter-server/tests/turn_control_no_user_key.rs @@ -0,0 +1,611 @@ +//! SD-11: on a daemon that holds no proof-of-user key — the one `biorouter +//! serve` starts (SD-7), or a `biorouterd` started by hand — stopping, steering +//! and settling a turn admit exactly the callers `POST /agent/stop` already +//! admits there: a chat the caller can reach, and never a subagent's. +//! +//! Measured on 2026-09-11 from a browser page on a real `biorouter serve`: +//! `POST /agent/cancel` and `POST /interrupt` both answered 403 with an empty +//! body, capability header and all, because each began with an unconditional +//! `is_user_action` check that a keyless daemon can never pass. The browser's +//! Stop button and mid-turn steering could not work on any `serve` host, one +//! configured with a public model included. +//! +//! ⚠ **Its own test binary on purpose**, for the reason `approval_no_user_key.rs` +//! gives: the installed digest is a process-global `OnceLock`, the lib's tests +//! install one, and inside that binary the keyless state is unreachable once the +//! first of them wins. Nothing here installs a digest — which is exactly how +//! `biorouter serve` starts its daemon. What a daemon that DOES hold a key does +//! with the same requests is pinned by the lib's own tests +//! (`routes::reply`'s `cancel_without_user_action_proof_cannot_stop_another_turn` +//! and `interrupt_without_user_action_proof_cannot_forge_human_steering`), and +//! this change leaves it alone. +//! +//! ⚠ **The terminal reads what these refusals look like.** `biorouter session` +//! cannot ask a daemon whether it holds a key, so `session cancel`, `attach` and +//! `send` go out without one and read the answer (`commands/session_watch.rs`, +//! `key_verdict`): an EMPTY 403 means "this daemon holds a key", and only that +//! makes the terminal ask the person for it. Every refusal here must therefore +//! carry the daemon's sentence, or a `serve` user is asked for a key that does +//! not exist — `the_terminals_empty_steer_is_answered_by_the_gate_and_touches_nothing` +//! pins it for the question the terminal actually sends. + +// Redirects this binary's Biorouter data/config/state dirs at a throwaway root +// before `main`, so nothing here can open the developer's real `sessions.db`. +#[path = "../src/test_sandbox.rs"] +mod test_sandbox; + +use std::sync::Arc; +use std::time::Duration; + +use axum::body::Body; +use axum::http::{HeaderMap, Request, StatusCode}; +use axum::Router; +use biorouter::agents::{Drained, TurnId}; +use biorouter::model::ModelConfig; +use biorouter::privacy::SessionClassification; +use biorouter::session::session_manager::SessionType; +use biorouter_server::auth::{user_action_proof, UserActionProof}; +use biorouter_server::routes::session_reach::SESSION_REACH_NO_KEY; +use biorouter_server::state::AppState; +use serde_json::{json, Value}; +use serial_test::serial; +use tokio_util::sync::CancellationToken; +use tower::ServiceExt; + +/// Every test here stands on this: the daemon under test holds no key. +fn assert_the_daemon_is_keyless() { + assert_eq!( + user_action_proof(&HeaderMap::new()), + UserActionProof::NoKeyInstalled, + "something in this binary installed a user-action digest, so these tests would be \ + measuring a desktop daemon rather than a `biorouter serve` one" + ); + assert!( + biorouter::privacy::privacy_tiers_enabled(), + "privacy tiers are off, so the reach gate below would admit everything either way" + ); +} + +/// The chats these tests stop and steer. Every one is a real row, because the +/// keyless arm reads the row (reach, then the subagent rule) before it touches +/// the turn — unlike the lib's tests, which name ids that were never created. +#[derive(Clone, Copy, Debug)] +enum Chat { + /// An ordinary chat on a public model: what a browser on a `serve` host + /// configured with a public model opens. + Public, + /// An ordinary chat a private model's turn has ratcheted. + Private, + /// A delegated child's session. + Subagent, +} + +async fn seed(state: &Arc, chat: Chat) -> String { + let manager = state.session_manager(); + let session_type = match chat { + Chat::Subagent => SessionType::SubAgent, + Chat::Public | Chat::Private => SessionType::User, + }; + let session = manager + .create_session( + std::env::temp_dir(), + format!("SD-11 {chat:?} (test fixture)"), + session_type, + ) + .await + .unwrap(); + if matches!(chat, Chat::Private) { + // Raised the way a real chat gets there — a turn on a private provider — + // rather than by writing the column, so what is refused here is the + // state a user's own chat reaches. + manager + .update(&session.id) + .provider_name("versa_azure") + .model_config(ModelConfig::new("gpt-4o").unwrap()) + .raise_privacy(SessionClassification::Private, "turn:versa_azure") + .apply() + .await + .unwrap(); + } + session.id +} + +async fn discard(state: &Arc, session_id: &str) { + // The tests run serially, so every cached agent is this test's. + state.clear_cached_agents().await; + let _ = state.session_manager().delete_session(session_id).await; +} + +/// The headers a browser tab sends on a `serve` daemon: the page's shim answers +/// the user-action key with an empty string, and a caller that runs under a +/// private model states it in `X-Caller-Provider`, as `biorouter session` does +/// from a terminal. +fn post(uri: &str, body: Value, caller_provider: Option<&str>) -> Request { + let mut request = Request::builder() + .uri(uri) + .method("POST") + .header("content-type", "application/json") + .header("X-User-Action", ""); + if let Some(provider) = caller_provider { + request = request.header("X-Caller-Provider", provider); + } + request.body(Body::from(body.to_string())).unwrap() +} + +async fn send(app: Router, request: Request) -> (StatusCode, String) { + let response = app.oneshot(request).await.unwrap(); + let status = response.status(); + let bytes = tokio::time::timeout( + Duration::from_secs(60), + axum::body::to_bytes(response.into_body(), usize::MAX), + ) + .await + .expect("the response body did not finish within a minute") + .unwrap(); + (status, String::from_utf8_lossy(&bytes).into_owned()) +} + +fn reply_routes(state: &Arc) -> Router { + biorouter_server::routes::reply::routes(Arc::clone(state)) +} + +fn json_of(body: &str) -> Value { + serde_json::from_str(body).unwrap_or_else(|_| panic!("not JSON: {body}")) +} + +/// Begin a turn on `session_id` the way `/reply` does, holding its lock. +fn begin_turn( + state: &Arc, + session_id: &str, +) -> (biorouter_server::state::TurnGuard, CancellationToken) { + let token = CancellationToken::new(); + let guard = state + .try_begin_turn_idempotent(session_id, token.clone(), None) + .expect("the turn lock is free"); + (guard, token) +} + +/// A Stop, in the plain form a script sends. The browser's store +/// (`requestExactTurnSettlement` in `chatStreamStore.tsx`) also names the exact +/// generation and waits for it to settle; the Stop-and-Send test below sends +/// that fuller body. +fn stop_request(session_id: &str, caller_provider: Option<&str>) -> Request { + post( + "/agent/cancel", + json!({ "session_id": session_id }), + caller_provider, + ) +} + +fn steer_request(session_id: &str, text: &str, caller_provider: Option<&str>) -> Request { + post( + "/interrupt", + json!({ "session_id": session_id, "text": text, "turn_id": "browser-steer-1" }), + caller_provider, + ) +} + +/// The failure the QA run measured, as its own regression test: the browser's +/// Stop, on an ordinary public chat, on a daemon with no key. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_keyless_daemon_stops_a_turn_in_a_chat_the_caller_can_reach() { + assert_the_daemon_is_keyless(); + let state = AppState::new().await.unwrap(); + let id = seed(&state, Chat::Public).await; + let (guard, token) = begin_turn(&state, &id); + + let (status, body) = send(reply_routes(&state), stop_request(&id, None)).await; + + assert_eq!( + status, + StatusCode::OK, + "the browser's Stop was refused on a keyless daemon: {body}" + ); + let body = json_of(&body); + assert_eq!(body["cancelled"], json!(true), "{body}"); + assert_eq!(body["turn_id"], json!(guard.turn_id()), "{body}"); + assert!(token.is_cancelled(), "a 200 that did not trip the turn"); + + drop(guard); + discard(&state, &id).await; +} + +/// Mid-turn steering, and what it may not claim. The desktop stamps a steer +/// `UserDirect` because its proof establishes that a person typed it; nothing on +/// a keyless daemon can establish that, so the steer is recorded exactly as +/// `/reply` records the same caller's message — unstamped. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_keyless_daemon_steers_a_turn_without_claiming_a_person_typed_it() { + assert_the_daemon_is_keyless(); + let state = AppState::new().await.unwrap(); + let id = seed(&state, Chat::Public).await; + let (guard, _token) = begin_turn(&state, &id); + // #69: acceptance is the agent loop's to give, so the agent must be in the + // state a running loop puts it in; the turn lock alone is not enough. + let agent = state.get_agent(id.clone()).await.unwrap(); + agent.open_for_turn(TurnId::new("keyless-agent-turn")); + + let (status, body) = send( + reply_routes(&state), + steer_request(&id, "actually, use R", None), + ) + .await; + + assert_eq!( + status, + StatusCode::ACCEPTED, + "mid-turn steering was refused on a keyless daemon: {body}" + ); + assert_eq!(json_of(&body)["turn_id"], json!("keyless-agent-turn")); + match agent.close_and_drain() { + Drained::Some(queued) => { + assert_eq!(queued.len(), 1); + assert_eq!(queued[0].text, "actually, use R"); + assert_eq!( + queued[0].provenance, None, + "a keyless daemon stamped a steer as typed by a person, which only the proof \ + it does not hold can establish" + ); + } + Drained::Empty => panic!("the accepted steer is not on the agent's queue"), + } + + drop(guard); + discard(&state, &id).await; +} + +/// Stop-and-Send mints a continuation lease, and a live lease blocks every turn +/// that does not present it. So a keyless daemon that let the cancel through and +/// refused the abandon would wedge the chat the first time a user removed a +/// queued message — this is why the settle routes move with the cancel. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn stop_and_send_settles_and_its_lease_can_be_abandoned_on_a_keyless_daemon() { + assert_the_daemon_is_keyless(); + let state = AppState::new().await.unwrap(); + let id = seed(&state, Chat::Public).await; + let (guard, token) = begin_turn(&state, &id); + let turn_id = guard.turn_id().to_string(); + + let cancel = tokio::spawn(send( + reply_routes(&state), + post( + "/agent/cancel", + json!({ + "session_id": id, + "expected_turn_id": turn_id, + "wait_for_idle": true, + "continuation_pending": true, + "continuation_owner_id": "browser-window-a", + }), + None, + ), + )); + tokio::time::timeout(Duration::from_secs(10), async { + while !token.is_cancelled() && !cancel.is_finished() { + tokio::task::yield_now().await; + } + }) + .await + .expect("the Stop-and-Send request neither tripped the turn nor answered"); + // The turn unwinds: its guard retires, which is what settles the cancel. + drop(guard); + let (status, body) = cancel.await.unwrap(); + assert_eq!( + status, + StatusCode::OK, + "Stop-and-Send was refused on a keyless daemon: {body}" + ); + let body = json_of(&body); + assert_eq!(body["settled"], json!(true), "{body}"); + let lease = body["continuation_lease"] + .as_str() + .unwrap_or_else(|| panic!("Stop-and-Send returned no lease: {body}")) + .to_string(); + + assert!( + state + .try_begin_turn_idempotent(&id, CancellationToken::new(), None) + .is_err(), + "a live continuation lease must hold the chat for its replacement" + ); + + let (status, body) = send( + reply_routes(&state), + post( + "/agent/continuation/abandon", + json!({ "session_id": id, "continuation_lease": lease }), + None, + ), + ) + .await; + assert_eq!( + status, + StatusCode::OK, + "the lease a keyless cancel minted could not be abandoned, so the chat is wedged: {body}" + ); + assert_eq!(json_of(&body)["resolution"], json!("abandoned")); + let successor = state + .try_begin_turn_idempotent(&id, CancellationToken::new(), None) + .expect("an abandoned lease must release the chat"); + + drop(successor); + discard(&state, &id).await; +} + +/// A reload between the cancel and the replacement: the window asks for its +/// pending continuation back by its owner id, or gives the group up. Refusing +/// this on a keyless daemon strands the same live lease the abandon route +/// above releases. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_keyless_daemon_hands_a_pending_continuation_back_to_its_window() { + assert_the_daemon_is_keyless(); + let state = AppState::new().await.unwrap(); + let id = seed(&state, Chat::Public).await; + // A generation that has already retired mints its lease at once, with no + // settlement to wait for — the reload case, where the turn is long gone. + let (retired, _token) = begin_turn(&state, &id); + let retired_id = retired.turn_id().to_string(); + drop(retired); + let (status, body) = send( + reply_routes(&state), + post( + "/agent/cancel", + json!({ + "session_id": id, + "expected_turn_id": retired_id, + "wait_for_idle": true, + "continuation_pending": true, + "continuation_owner_id": "browser-window-b", + }), + None, + ), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert!(json_of(&body)["continuation_lease"].is_string(), "{body}"); + + let recover = |action: &str| { + post( + "/agent/continuation/recover", + json!({ + "session_id": id, + "superseded_turn_id": retired_id, + "continuation_owner_id": "browser-window-b", + "action": action, + }), + None, + ) + }; + let (status, body) = send(reply_routes(&state), recover("take_over")).await; + assert_eq!( + status, + StatusCode::OK, + "a reloaded window could not take its continuation back on a keyless daemon: {body}" + ); + let body = json_of(&body); + assert_eq!(body["resolution"], json!("taken_over"), "{body}"); + assert!(body["continuation_lease"].is_string(), "{body}"); + + let (status, body) = send(reply_routes(&state), recover("abandon")).await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(json_of(&body)["resolution"], json!("abandoned")); + let successor = state + .try_begin_turn_idempotent(&id, CancellationToken::new(), None) + .expect("an abandoned group must release the chat"); + + drop(successor); + discard(&state, &id).await; +} + +/// Reach decides a private chat, exactly as it decides reading one: a caller +/// whose stated capability covers it is admitted, one that states nothing is +/// refused with the keyless daemon's own sentence, and the refused request +/// touches neither the turn nor the agent's queue. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_private_chat_is_stopped_or_steered_only_by_a_caller_whose_capability_covers_it() { + assert_the_daemon_is_keyless(); + let state = AppState::new().await.unwrap(); + let id = seed(&state, Chat::Private).await; + let (guard, token) = begin_turn(&state, &id); + let agent = state.get_agent(id.clone()).await.unwrap(); + agent.open_for_turn(TurnId::new("private-agent-turn")); + + for request in [ + stop_request(&id, None), + steer_request(&id, "pretend the user said this", None), + // A tier is not a provider: the daemon resolves the NAME against its own + // registry, and a spelled-out tier resolves Public. + stop_request(&id, Some("private")), + ] { + let (status, body) = send(reply_routes(&state), request).await; + assert_eq!(status, StatusCode::FORBIDDEN, "{body}"); + assert!( + body.contains(SESSION_REACH_NO_KEY), + "refused, but not by the reach gate's keyless sentence: {body}" + ); + } + assert!(!token.is_cancelled(), "a refused Stop reached the turn"); + assert!( + !agent.has_soft_interrupts(), + "a refused steer reached the agent's queue" + ); + + let (status, body) = send( + reply_routes(&state), + steer_request(&id, "use the cohort table", Some("versa_azure")), + ) + .await; + assert_eq!(status, StatusCode::ACCEPTED, "{body}"); + let (status, body) = send(reply_routes(&state), stop_request(&id, Some("versa_azure"))).await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert!(token.is_cancelled()); + + drop(guard); + discard(&state, &id).await; +} + +/// A subagent's tab stays the person's, on this daemon as on every other: the +/// parent is told when a human intervened in its child, and only the proof can +/// establish that one did. `/reply` and `/agent/stop` refuse the same caller +/// there already. The refusal names this daemon's situation rather than +/// telling a person at the keyboard to go and prove they are one (SD-8). +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_subagents_turn_is_still_refused_and_the_refusal_says_why() { + assert_the_daemon_is_keyless(); + let state = AppState::new().await.unwrap(); + let id = seed(&state, Chat::Subagent).await; + let (guard, token) = begin_turn(&state, &id); + let agent = state.get_agent(id.clone()).await.unwrap(); + agent.open_for_turn(TurnId::new("child-agent-turn")); + + for request in [ + stop_request(&id, None), + steer_request(&id, "pretend the user said this", None), + stop_request(&id, Some("versa_azure")), + ] { + let (status, body) = send(reply_routes(&state), request).await; + assert_eq!(status, StatusCode::FORBIDDEN, "{body}"); + assert!( + body.contains("without a user-action key"), + "a keyless daemon's refusal must say it is this daemon, not the caller, that \ + cannot prove a person acted: {body}" + ); + } + assert!( + !token.is_cancelled(), + "a refused Stop reached a child's turn" + ); + assert!( + !agent.has_soft_interrupts(), + "a refused steer reached a child's queue" + ); + + drop(guard); + discard(&state, &id).await; +} + +/// The empty steer exactly as `biorouter session` sends it to ask whether it may +/// steer (`steer_gate_question` in `commands/session_watch.rs`): no +/// `X-User-Action` header at all — the browser's shim sends an empty one — and +/// no turn id. +fn terminal_question(session_id: &str, caller_provider: Option<&str>) -> Request { + let mut request = Request::builder() + .uri("/interrupt") + .method("POST") + .header("content-type", "application/json"); + if let Some(provider) = caller_provider { + request = request.header("X-Caller-Provider", provider); + } + request + .body(Body::from( + json!({ "session_id": session_id, "text": "" }).to_string(), + )) + .unwrap() +} + +/// The question `biorouter session attach` asks as it joins, and `session send` +/// after a refusal: would this daemon take a steer from this terminal, and does +/// it want the user-action key for one? On this daemon the gate answers it and +/// the text check refuses what the gate lets through, so the answer is the +/// gate's verdict and nothing more — a 400 where the terminal may steer, and +/// otherwise a refusal in the daemon's own words. Never the EMPTY 403 that means +/// "this daemon holds a key", which would send the terminal to ask the person +/// for one that does not exist. And the question touches neither the turn nor +/// the agent's queue. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn the_terminals_empty_steer_is_answered_by_the_gate_and_touches_nothing() { + assert_the_daemon_is_keyless(); + let state = AppState::new().await.unwrap(); + for (chat, caller, expected) in [ + (Chat::Public, None, StatusCode::BAD_REQUEST), + (Chat::Private, None, StatusCode::FORBIDDEN), + (Chat::Private, Some("versa_azure"), StatusCode::BAD_REQUEST), + (Chat::Subagent, None, StatusCode::FORBIDDEN), + (Chat::Subagent, Some("versa_azure"), StatusCode::FORBIDDEN), + ] { + let id = seed(&state, chat).await; + let (guard, token) = begin_turn(&state, &id); + let agent = state.get_agent(id.clone()).await.unwrap(); + agent.open_for_turn(TurnId::new("questioned-agent-turn")); + + let (status, body) = send(reply_routes(&state), terminal_question(&id, caller)).await; + + assert_eq!(status, expected, "{chat:?} asked by {caller:?}: {body}"); + if status == StatusCode::FORBIDDEN { + assert!( + body.contains("without a user-action key"), + "a keyless refusal without the daemon's sentence reads to the terminal as \ + 'this daemon wants a key': {body:?}" + ); + } + assert!( + !agent.has_soft_interrupts(), + "the terminal's question reached the agent's queue" + ); + assert!( + !token.is_cancelled(), + "the terminal's question reached the turn" + ); + + drop(guard); + discard(&state, &id).await; + } +} + +/// **The premise SD-11 stands on, pinned.** On a keyless daemon `/agent/stop` +/// already cancels a running turn for exactly the callers turn control now +/// admits — reach, and never a subagent's chat — so admitting them to +/// `/agent/cancel` gives nothing holding the daemon secret a capability it did +/// not have. If `/agent/stop` is ever tightened on such a daemon, this test +/// fails, and the ruling has to be re-argued rather than silently outlived. +#[tokio::test(flavor = "multi_thread")] +#[serial] +async fn a_keyless_cancel_admits_exactly_the_callers_agent_stop_already_admits() { + assert_the_daemon_is_keyless(); + let state = AppState::new().await.unwrap(); + for (chat, caller, admitted) in [ + (Chat::Public, None, true), + (Chat::Private, None, false), + (Chat::Private, Some("versa_azure"), true), + (Chat::Subagent, None, false), + (Chat::Subagent, Some("versa_azure"), false), + ] { + let mut verdicts = Vec::new(); + for route in ["/agent/stop", "/agent/cancel"] { + let id = seed(&state, chat).await; + // `/agent/stop` evicts the agent, and answers 404 when there is none + // to evict; give it one so its answer is about the gate alone. + state.get_agent(id.clone()).await.unwrap(); + let (guard, token) = begin_turn(&state, &id); + let app = if route == "/agent/stop" { + biorouter_server::routes::agent::routes(Arc::clone(&state)) + } else { + reply_routes(&state) + }; + let (status, body) = send(app, post(route, json!({ "session_id": id }), caller)).await; + assert_eq!( + status.is_success(), + token.is_cancelled(), + "{route} on a {chat:?} chat answered {status} but the turn's cancellation \ + disagrees: {body}" + ); + verdicts.push((route, status, token.is_cancelled())); + drop(guard); + discard(&state, &id).await; + } + for (route, status, cancelled) in &verdicts { + assert_eq!( + *cancelled, admitted, + "{route} on a {chat:?} chat with caller {caller:?} answered {status}; expected \ + admitted={admitted}. All verdicts: {verdicts:?}" + ); + } + } +} diff --git a/crates/biorouter/tests/privacy_guard_wiring.rs b/crates/biorouter/tests/privacy_guard_wiring.rs index bb42dea5c..857622a25 100644 --- a/crates/biorouter/tests/privacy_guard_wiring.rs +++ b/crates/biorouter/tests/privacy_guard_wiring.rs @@ -322,7 +322,11 @@ const REGISTRY: &[Guard] = &[ kind: SiteKind::Guard, what: "`POST /agent/resume`, `POST /agent/update_from_session`, and `POST \ /agent/update_working_dir`, plus the shared `authorize_agent_control` \ - gate used by provider, extension, stop, and restart mutations", + gate used by provider, extension, stop, and restart mutations. On a \ + daemon that holds no user-action key that same gate is also the \ + turn-control gate of `/agent/cancel`, `/interrupt` and the two \ + continuation routes (SD-11), reached from `routes::reply`'s \ + `authorize_turn_control` by name — so SD-11 added no call here", }, Site { file: "crates/biorouter-server/src/routes/mod.rs", @@ -337,7 +341,9 @@ const REGISTRY: &[Guard] = &[ kind: SiteKind::Guard, what: "`POST /reply`, which runs an agent turn with tools inside the named \ session, plus the explicit continuation takeover and group-abandon \ - recovery mutation", + recovery mutation. The other turn-control routes in this file reach the \ + gate only on a keyless daemon, and through `authorize_agent_control` \ + in `routes/agent.rs` (SD-11), which is why they add nothing to this count", }, Site { file: "crates/biorouter-server/src/routes/session.rs", diff --git a/docs/agent-loop/subagents.md b/docs/agent-loop/subagents.md index faf6ca9d4..ac31666ba 100644 --- a/docs/agent-loop/subagents.md +++ b/docs/agent-loop/subagents.md @@ -74,7 +74,7 @@ It is a cap on the burst, not a running total of open tabs. Each slot is release History hides subagent runs by default, so your session list stays a list of *your* conversations. Turn on **Show subagent runs** in History and each child appears nested under the conversation that spawned it, with a live marker while it is still running. -From the CLI, `biorouter session list --subagents` does the same, `biorouter session attach` joins a live child (`--of` to pick one by parent, `--read-only` to watch without steering), and `biorouter session cancel` stops it. +From the CLI, `biorouter session list --subagents` does the same, `biorouter session attach` joins a live child (`--of` to pick one by parent, `--read-only` to watch without steering), and `biorouter session cancel` stops it. Steering or stopping a child needs proof that a person acted: a daemon started with a user-action key asks the terminal for the key first, and one started without — `biorouter serve` — refuses, and says so; `--read-only` still follows the child there. See [the user-action key](workspace-control.md#as-subcommands-you-type). ## Internal subagents diff --git a/docs/agent-loop/workspace-control.md b/docs/agent-loop/workspace-control.md index 7ecb7cf0a..d654d7fc9 100644 --- a/docs/agent-loop/workspace-control.md +++ b/docs/agent-loop/workspace-control.md @@ -212,8 +212,28 @@ Give `session export` an identifier. With neither `--session-id` nor `--name` it `session list --subagents` reads the store for the rows but has to ask the daemon who is still live, so it marks each run `● live`, `○ done`, or — when it could not ask — `· state unknown`. It deliberately does *not* blame a missing daemon for that third state: a stripped `BIOROUTER_SERVER__SECRET_KEY` (which is what an agent-spawned shell gets) produces it with a daemon running perfectly well. The actual reason is printed once on stderr, so `--format json` on stdout stays clean. -The four daemon-bound commands need one running. Steering and cancellation also -require a user-action key whose SHA-256 digest is handed to the daemon on stdin: +The four daemon-bound commands need one running. It listens on `127.0.0.1:3000` unless +`BIOROUTER_PORT` says otherwise, and `send`, `watch`, `attach` and `cancel` authenticate with the +same `BIOROUTER_SERVER__SECRET_KEY`. `biorouterd` invents a random key when that variable is unset, +in which case no client can authenticate — so set it on both sides. A mismatch shows up as HTTP 401. + +Whether stopping and steering also need a **user-action key** depends on how the daemon was +started, and the command asks the daemon rather than you: + +- **A daemon started with a key** — its SHA-256 digest piped to `biorouterd` on stdin, as below, + which is how you start the daemon the desktop app shares with your terminal — wants the raw key + before it lets anyone stop or steer a turn, and before it takes a message into a subagent's + session, or into a private chat when your terminal runs a public model. When it refuses a + request for want of the key, the command asks you for it once, without echo, and makes the + request again with it. `attach` asks as it joins, before it reads anything you type, and + checks the key there rather than at your first steer. +- **A daemon started without one** — `biorouter serve`, or `biorouterd agent` with nothing piped in + — has nothing to check a key against, so you are never asked for one. It lets you stop and steer + any chat it lets you reach (a public one always, a private one when your terminal runs a private + model), except a subagent's: stopping, steering or sending to a subagent needs proof that a person + acted, which only a daemon holding a key can check, and the command prints that daemon's refusal + saying so ([SD-11](../deployment/serve-decisions.md#sd-11--stop-and-steering-work-on-a-daemon-with-no-key-a-subagents-tab-stays-the-persons)). + `attach --read-only` still follows such a session. ```bash read -r -s action_key @@ -222,17 +242,12 @@ printf '%s' "$action_key" | shasum -a 256 | cut -d ' ' -f 1 | \ BIOROUTER_SERVER__SECRET_KEY= biorouterd agent ``` -It listens on `127.0.0.1:3000` unless `BIOROUTER_PORT` says otherwise, and `send`, `watch`, -`attach` and `cancel` authenticate with the same `BIOROUTER_SERVER__SECRET_KEY`. `biorouterd` -invents a random key when that variable is unset, in which case no client can authenticate — so -set it on both sides. A mismatch shows up as HTTP 401. - -`session attach` prompts for the raw user-action key, without echo, before it permits steering; -`session cancel` does the same. `attach --read-only`, `watch`, and `send` do not need that proof. -Trusted automation can pipe the key as the first line with `--user-action-key-stdin`; never put it -in argv, an environment variable, config, or logs. The raw key is held in a zeroizing CLI buffer -and sent only on the attached event stream, `/interrupt`, `/agent/cancel`, or the attached -subagent's `/reply`, while the daemon retains only its digest. +`watch` and `attach --read-only` never send the key. Trusted automation can supply it as the first +line of stdin with `--user-action-key-stdin` (on `send`, `attach` and `cancel`), which sends it at +once instead of waiting for the daemon to ask; never put it in argv, an environment variable, +config, or logs. The raw key is held in a zeroizing CLI buffer and sent only on the attached event +stream, `/interrupt`, `/agent/cancel` and `/reply` — and only when you supplied it or the daemon +asked for it — while the daemon retains only its digest. Use `session attach` rather than `session --resume` on a session that is running right now: resuming opens a second agent on the same conversation, and the two do not share the daemon's turn lock. diff --git a/docs/cli/command-reference.md b/docs/cli/command-reference.md index 78c6e3592..b99f57d85 100644 --- a/docs/cli/command-reference.md +++ b/docs/cli/command-reference.md @@ -319,6 +319,19 @@ BIOROUTER_SERVER__SECRET_KEY= biorouter session watch A mismatched key surfaces as `HTTP 401` with that hint attached. +**The user-action key, when the daemon has one.** A daemon started with a user-action key (its +SHA-256 digest piped to `biorouterd agent` on stdin) wants the raw key before it lets anyone stop or +steer a turn, and before it takes a message into a subagent's session. `send`, `attach` and `cancel` +never ask for it up front: each makes its request without the key, and only if the daemon refuses +it for want of one asks you for it once, without echo, and tries again. `attach` asks as it joins, +before it reads anything you type. A daemon started without a key — `biorouter serve`, or +`biorouterd agent` with nothing piped in — is never asked about one. Such a daemon refuses every +request to stop, steer or send to a subagent's session, and the command prints its reason. To supply the key +without a prompt, pass `--user-action-key-stdin` and pipe the raw key as the first line of stdin. +Never put it in an argument, an environment variable or a config file. See +[Workspace control](../agent-loop/workspace-control.md#as-subcommands-you-type) for how to start a +daemon with a key. + ### session watch [options] Stream a session's live events into your terminal — the same frames biorouter Desktop renders, printed as lines. Watching is read-only: it never writes to the conversation, and stopping the watch never stops the session. @@ -357,6 +370,7 @@ Send a prompt into an existing session and stream the resulting turn, without op **Options:** - **`--no-wait`**: Return as soon as the daemon accepts the turn, printing `[started] turn in session `, instead of streaming it to completion +- **`--user-action-key-stdin`**: Read the raw user-action key from the first line of stdin instead of being asked for it when the daemon wants it. See [the user-action key](#live-session-commands-and-the-daemon) **Usage:** @@ -389,6 +403,7 @@ Join a session that is running *right now*. `attach` prints the conversation so - **`--name `**: Attach by session name instead of ID. Refuses, and lists the candidates, if several sessions share that name - **`--of `**: Attach to the running subagent of this parent session. Errors, listing what it found, if that parent has no subagent with a turn in flight or has more than one - **`--read-only`**: Observe only — do not read stdin and do not send anything +- **`--user-action-key-stdin`**: Read the raw user-action key from the first line of stdin, before the lines you steer with, instead of being asked for it when the daemon wants it. See [the user-action key](#live-session-commands-and-the-daemon) Give **exactly one** target: a session ID, `--name`, or `--of`. Passing none, or more than one, is an error. `biorouter session list --subagents` lists the sessions and subagent runs you can address. @@ -422,6 +437,10 @@ Stop the turn a session is currently running. This is the same action as the Sto - **``** (required): The session whose running turn should be stopped +**Options:** + +- **`--user-action-key-stdin`**: Read the raw user-action key from the first line of stdin instead of being asked for it when the daemon wants it. See [the user-action key](#live-session-commands-and-the-daemon) + Cancelling is idempotent: a session with no turn in flight is not an error, it reports `nothing to cancel: this session had no turn in flight`. **Usage:** diff --git a/docs/deployment/browser-access.md b/docs/deployment/browser-access.md index 44526d471..502e38c6e 100644 --- a/docs/deployment/browser-access.md +++ b/docs/deployment/browser-access.md @@ -214,6 +214,8 @@ differs: |---|---| | Chat, sessions, history, extensions, skills, knowledge bases, workflows | Work as they do in the desktop application. | | Workspace control, several conversations at once, live app agents | Work — these are WebSocket-backed daemon routes, reached on the same origin. | +| Stopping a response, steering it while it runs, Stop and send | Work in an ordinary chat ([SD-11](serve-decisions.md#sd-11--stop-and-steering-work-on-a-daemon-with-no-key-a-subagents-tab-stays-the-persons)). A message you steer with is recorded as an ordinary message rather than as one the desktop application marks as typed by you. | +| A delegated subagent's own tab | **Read-only.** Sending to a subagent, steering it and stopping it from its tab need proof that a person acted, which only the desktop application holds. The tab does not yet say so before you try. | | Model and provider selection | **Not available.** See [The model is fixed before you start](#the-model-is-fixed-before-you-start). | | File and folder pickers | No native dialog. You type a path, and it is a path **on the machine running the daemon**, not on the machine holding the browser. | | Artifacts and diagnostics bundles | The artifact side panel works as usual. Opening an artifact outside the panel opens a new tab; a diagnostics bundle downloads as a file. | diff --git a/docs/deployment/programmatic-session-access.md b/docs/deployment/programmatic-session-access.md index 7d976e883..f77267c89 100644 --- a/docs/deployment/programmatic-session-access.md +++ b/docs/deployment/programmatic-session-access.md @@ -175,6 +175,7 @@ one of them resolves the target's tier **before** it touches the session, so a r | `POST /agent/update_working_dir` | Repoints the session at a directory. | | `POST /agent/add_extension` · `remove_extension` | Attaches or detaches tools. | | `GET`/`POST /knowledge/active` | Reads or repoints the session's knowledge bases. | +| `POST /agent/cancel` · `POST /interrupt` · `POST /agent/continuation/abandon` | Stops, steers or settles the session's running turn — **on a daemon that holds no user-action key only**, such as `biorouter serve` or a hand-run `biorouterd` ([SD-11](serve-decisions.md#sd-11--stop-and-steering-work-on-a-daemon-with-no-key-a-subagents-tab-stays-the-persons)). There they admit exactly the callers `POST /agent/stop` admits, a subagent's session excepted. A steer sent this way is recorded as an ordinary message, not as one a person typed. | ## What the header does *not* cover @@ -186,7 +187,7 @@ it would be wrong: | Route | What guards it instead | |---|---| -| `POST /interrupt`, `POST /agent/cancel`, `POST /agent/continuation/abandon` | `X-User-Action` — steering a turn is the user's decision, not a capability. | +| `POST /interrupt`, `POST /agent/cancel`, `POST /agent/continuation/abandon` | On a daemon that holds a user-action key — the desktop application's — `X-User-Action` and nothing else: a steer there is stamped as typed by a person, and only the proof can back that. A daemon that holds no key cannot check the proof at all, so these routes honour the header there instead (the table above). | | `POST /sessions/{id}/declassify`, `POST /sessions/{id}/diverge`, `POST /sessions/{id}/edit_message` | `X-User-Action` — these change or copy a classification, which no model may decide. | | `POST /agent/cross_affiliation_grant`, `POST /action-required/tool-confirmation` | `X-User-Action`, plus a decision-authority check on the resolving surface. | | `POST /knowledge/bases/{id}/ingest-conversation` | Its own Gate G: capability is derived from the model named in the request body, and every selected conversation is checked against it before a transcript is rendered. | @@ -233,8 +234,11 @@ not evidence the session exists. **`403` saying the daemon was started without a user-action key.** A different state: this daemon holds no key with which to verify a human, which is normal for `just run-server`, a hand-run `biorouterd agent`, and `biorouter serve`. The capability header is unaffected and still admits a -private chat on that daemon — a capable caller never reaches this message, because capability is -checked before proof. If you are seeing it, your caller is public. +private chat on that daemon — a capable caller never reaches the reach gate's version of this +message, because capability is checked before proof. If you are seeing it, your caller is public, +or the session is a delegated subagent's: changing, stopping or steering a subagent from its tab +needs proof that a person acted, whatever the capability, and that refusal says so in its own +words. **The header worked yesterday and now does not.** The tier came from the daemon's own registry, so a provider removed from `config.yaml`, or renamed, now resolves public. diff --git a/docs/deployment/serve-decisions.md b/docs/deployment/serve-decisions.md index 24b74e3a5..c555a253f 100644 --- a/docs/deployment/serve-decisions.md +++ b/docs/deployment/serve-decisions.md @@ -18,7 +18,8 @@ This page records the decisions that replaced that arrangement. They were taken several of them only make sense as a set: the reason a browser session cannot switch models (SD-1) is also the reason it needs no proof-of-user mechanism, which is the reason the daemon can be spawned with a closed stdin (SD-7) — and the reason every control that needs that proof -must say so before the user reaches for it (SD-8). Read [the architecture](serve-architecture.md) +must say so before the user reaches for it (SD-8), and the reason the controls that stop and steer +a turn answer to the reach gate there instead (SD-11). Read [the architecture](serve-architecture.md) for how the result is built, and [browser access](browser-access.md) for how to use it. Records are identified `SD-n` — *serve decision*. The numbering is stable; a superseded record @@ -284,6 +285,155 @@ behaviour, so changing it means revisiting this record, not making a quiet fix. --- +## SD-11 — Stop and steering work on a daemon with no key; a subagent's tab stays the person's + +**Ruling.** On a daemon that holds no user-action key — the one `biorouter serve` starts (SD-7), or +a `biorouterd` started by hand — the four routes that control a running turn admit exactly the +callers `POST /agent/stop` already admits there: + +| Route | What it does | +|---|---| +| `POST /agent/cancel` | Stop, and the first half of Stop-and-Send. | +| `POST /interrupt` | Steers the turn that is running. | +| `POST /agent/continuation/abandon` | Gives up a Stop-and-Send replacement. | +| `POST /agent/continuation/recover` | Takes a Stop-and-Send replacement back after a reload, or gives it up. | + +That gate is `authorize_agent_control` in `routes/agent.rs`, called by name rather than written +again: the reach rule — every public chat, and a private one only for a caller whose stated +capability covers it — and then no subagent's chat. Three things hold beside it: + +- A steer admitted this way is recorded **unstamped**, exactly as `/reply` records the same caller's + message. It is never stamped `UserDirect`, the provenance that says a person typed it. +- A daemon that holds a key — the desktop application's — is unchanged. These routes take the proof + there and nothing else. +- A subagent's chat is refused on a keyless daemon, as `/reply` and `/agent/stop` already refuse it + there, and the refusal now names the daemon, not the caller, as what cannot prove a person acted + (SD-8). + +**Why.** The four routes asked for proof of a person before anything else, and on a daemon that +holds no key that proof can only refuse everyone, the person at the browser included. Measured on +2026-09-11 against a real `biorouter serve`, from the page, with the daemon secret and the host's +provider stated: `POST /agent/cancel` and `POST /interrupt` both answered `403` with an empty body. +The Stop button could stop nothing on any `serve` host, including one configured with a public +model. `crates/biorouter-server/tests/turn_control_no_user_key.rs` reproduces it on the code this +record changed: on a keyless daemon and an ordinary public chat, `/agent/stop` answered `200` and +cancelled the turn while `/agent/cancel` answered `403`. Stop is the control that ends a runaway +turn, and a chat that cannot be stopped from the only interface it has is less safe, not more. + +And the refusal protected nothing. The proof is on these routes so that a model holding the daemon +secret, which AR-11 found recoverable, cannot stop another chat's turn or put words in a person's +mouth. On a keyless daemon that caller already does each of those next door: + +| What the refusal withheld | Where the same caller already does it on a keyless daemon | Guarded by | +|---|---|---| +| Cancel the running turn of a chat it can reach | `POST /agent/stop`, which trips the same turn and evicts its agent besides | `authorize_agent_control` — the gate this record adopts | +| The same, as a model holding no secret at all | `workspace_close { scope: "turn" }` | `refuse_unless_writable`: the tier, and nothing else | +| Put text in front of the chat's model as the user | `POST /reply`, which a non-subagent chat accepts, unstamped, from any caller that reaches it | `session_reach` | +| Give up a chat's pending Stop-and-Send | `workspace_close { scope: "turn" }`, which abandons every pending continuation of the chat it names | `refuse_unless_writable` | + +So admitting a caller here gives nothing that holds the secret a capability it lacked. The person +gains the Stop button, mid-turn steering and Stop-and-Send, which the desktop application has +always had. + +**Why the four routes move together.** Stop-and-Send cancels with a continuation, and the +continuation mints a lease that holds the chat for its replacement: until the lease is used or +given up, every other turn in that chat is refused. A daemon that let the cancel through and +refused the abandon or the recover would wedge the chat the first time a person removed a queued +message or reloaded the page. The test binary above measures both. + +**Why a steer is not stamped.** `UserDirect` is a claim that a person typed the text, and the +subagent machinery acts on it: a child's parent is told a human intervened. Only the proof can back +that claim, so on a keyless daemon the steer carries no stamp — which is what `/reply` gives the +same caller's message, so the two ways of putting text in front of a chat's model agree about who +sent it. + +**Why a subagent's tab is not included.** That tab is where `UserDirect` means something, and +`/reply` and `/agent/stop` already refuse an unproven caller there, on every daemon. Admitting turn +control alone would give that tab a Stop that works beside a composer that cannot send, and the +rule for the tab is a decision about the whole tab rather than one of its buttons. + +**Why not on every daemon.** On a daemon that holds a key the proof costs the person nothing — the +renderer attaches it to every request — and it still refuses a caller that cannot present it. +Relaxing it there buys the person nothing, and would admit an unstamped steer where the desktop has +always stamped one. + +**Who can do this, and what else reaches the same place.** The two questions every privacy control +answers in writing ([privacy tiers §3.1](../security/privacy-tiers.md#31-the-review-checklist--two-questions-every-control-answers-in-writing)): + +- *Who can initiate it.* On a keyless daemon, anything holding the daemon secret: the person in the + browser and, indistinguishably, a model running in a chat on that daemon that has recovered the + secret. Both reach the chats the reach gate admits them to, less every subagent's. On a daemon + that holds a key, only a request carrying the proof. +- *What else reaches the same place:* + + | Other entry point to the same capability | Reachable by | Guarded by | Where that guard is called | + |---|---|---|---| + | `POST /agent/stop` — cancels the running turn | user; any holder of the daemon secret | `authorize_agent_control`: reach, then the subagent rule | `routes/agent.rs::stop_agent` | + | `workspace_close { scope: "turn" \| "agent" }` — cancels the turn, abandons its pending continuations | model | `refuse_unless_writable` (the tier) | `agents/workspace_extension.rs::handle_close` | + | `POST /active_work/{id}/cancel` — cancels a running subagent or background job by registry id | user; any holder of the daemon secret | **nothing**: the id is not a session id, so the reach gate cannot be applied | `routes/active_work.rs::cancel_active_work`; an open residual in `session_reach.rs` | + | `POST /reply` — puts the caller's text in front of the chat's model | user; any holder of the daemon secret | `session_reach`, then the subagent rule | `routes/reply.rs::reply` | + | `workspace_send_prompt { mode: "steer" \| "turn" }` — the same, from another chat | model | `refuse_unless_writable`; the text arrives framed as `AgentInjection`, and a private-to-public write raises a first-crossing approval | `agents/workspace_extension.rs::handle_send_prompt` | + | `biorouter session cancel`, `attach` and `send` → these routes, and `/reply` | user at a terminal | the routes' own gates: the CLI sends the proof only when the person supplied it or a daemon that holds a key refused without it (see *The terminal, since* below) | `commands/session_watch.rs::with_key_if_wanted` | + + The third row is a finding rather than a guard: a subagent's work can be cancelled on any daemon + by a caller that holds the secret and reads the id from `GET /active_work`. It is the residual + `session_reach.rs` already records, and this record does not widen it. + +**Displaced alternatives.** + +- *Keep the refusal, and explain it before the click (SD-8).* Rejected. SD-8 is for a control whose + absence is safe to explain; this one is how a person ends a turn they did not want. A Stop button + that says "unavailable here" is honest and still leaves the turn running. +- *Point the browser's Stop at `/agent/stop`.* Rejected. It cancels whatever turn is running rather + than the generation the person saw, so a late click can kill a successor; it does not wait for the + turn to settle and has no Stop-and-Send; and it evicts the agent. It would give the browser a + blunter Stop than the desktop's, to route around a refusal that guards nothing. +- *Relax the routes on every daemon.* Rejected; see *Why not on every daemon*. +- *Stamp the keyless steer `UserDirect`.* Rejected; see *Why a steer is not stamped*. +- *Admit plain Stop and refuse Stop-and-Send.* Rejected. The queue's "Stop and send" and a typed + "stop" in a busy composer both cancel with a continuation, so this would leave half of the Stop + controls refused, and a lease a person can mint must be a lease they can give up. + +**Consequence to accept.** On a keyless daemon, a model running in a chat on that daemon that has +recovered the daemon secret can stop or steer a turn in any chat the reach gate admits it to — a +public chat always, and a private one by stating a private provider, which the header does not +authenticate (`session_reach.rs` records as much). It could already stop those turns through +`/agent/stop` or `workspace_close`, and put text in front of those chats through `/reply`; what it +gains is steering a turn while it runs, with text that arrives unstamped, as a `/reply` from the same +caller would. It is recorded rather than closed: on a daemon that cannot tell a person from a model, +closing it means a Stop button nobody can press. + +**Not decided here.** A subagent's tab in a browser still offers a composer, a steer and a Stop +that all refuse; SD-8 requires them to say so before the click, and they do not yet. + +**The terminal, since.** `biorouter session cancel`, `attach` and `send` used to demand the +user-action key from the terminal before they sent anything, so against a keyless daemon they +refused locally the requests this record admits: the browser's defect, one client over. A terminal +cannot ask a daemon whether it holds a key, so each command now lets the daemon answer. It sends its +request without the proof, unless the person supplied the key on stdin (`--user-action-key-stdin`), +and asks the person for the key only when the answer is the empty 403 of a daemon that holds one; +then it sends once more, with the key. A refusal that carries a sentence, which is every refusal a +keyless daemon gives on these routes, is printed instead, because no key would change it. + +- `attach` asks as it joins, with an empty steer: the gate answers before `/interrupt` reads the + text, and empty text is refused before anything is touched. It cannot wait for the first real + steer, because by then stdin carries the person's messages, and a hidden prompt would have to share + it with them. +- `send` asks the same question after a refused `/reply`, because `/reply`'s own refusal for a + subagent's session is an empty 403 on either kind of daemon and cannot say which kind this is. + +Nothing is relaxed. The daemon stays the boundary, and every refusal the terminal reads is given +before the route touches the turn, so sending the request a second time cannot deliver anything +twice. A subagent's session still needs the proof. The raw key still comes only from the +terminal or from stdin, never from argv, the environment, config or logs, and it is now sent only +when the person supplied it or a daemon asked for it. Keeping the local refusal and rewording it to +say what to do was rejected, because it would ask the person whether the daemon holds a key, and +the daemon answers that itself. The shapes the terminal reads are pinned from the daemon's side, in +`routes::reply`'s keyed tests and in `tests/turn_control_no_user_key.rs`; the reader is +`key_verdict` in `commands/session_watch.rs`. + +--- + ## Related documentation - [Architecture of the serving path](serve-architecture.md) — how the decisions above are built. diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index cc6888530..ff33ce76d 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -308,7 +308,7 @@ "description": "Unauthorized - invalid secret key" }, "403": { - "description": "The request was not proven to come from the user" + "description": "The request was not proven to come from the user; on a daemon that holds no user-action key, the chat is out of the caller's reach or is a subagent's (SD-11)" }, "409": { "description": "A different turn generation is active, another client owns the continuation, or its admission is still settling", @@ -357,7 +357,7 @@ } }, "403": { - "description": "The request was not proven to come from the user" + "description": "The request was not proven to come from the user; on a daemon that holds no user-action key, the chat is out of the caller's reach or is a subagent's (SD-11)" }, "409": { "description": "The lease is invalid or belongs to another session", @@ -403,7 +403,7 @@ "description": "The continuation owner id is missing or invalid" }, "403": { - "description": "The session is out of reach or the request was not proven to come from the user" + "description": "The session is out of reach or the request was not proven to come from the user; on a daemon that holds no user-action key, the chat is out of the caller's reach or is a subagent's (SD-11)" }, "409": { "description": "The exact continuation generation was already resolved", @@ -1864,7 +1864,7 @@ "description": "Empty message text" }, "403": { - "description": "The request was not proven to come from the user" + "description": "The request was not proven to come from the user; on a daemon that holds no user-action key, the chat is out of the caller's reach or is a subagent's (SD-11)" }, "409": { "description": "No turn is accepting interrupts for this session" diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index b35605218..116f9d2fe 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -4420,7 +4420,7 @@ export type CancelTurnErrors = { */ 401: unknown; /** - * The request was not proven to come from the user + * The request was not proven to come from the user; on a daemon that holds no user-action key, the chat is out of the caller's reach or is a subagent's (SD-11) */ 403: unknown; /** @@ -4457,7 +4457,7 @@ export type AbandonContinuationLeaseData = { export type AbandonContinuationLeaseErrors = { /** - * The request was not proven to come from the user + * The request was not proven to come from the user; on a daemon that holds no user-action key, the chat is out of the caller's reach or is a subagent's (SD-11) */ 403: unknown; /** @@ -4490,7 +4490,7 @@ export type RecoverContinuationErrors = { */ 400: unknown; /** - * The session is out of reach or the request was not proven to come from the user + * The session is out of reach or the request was not proven to come from the user; on a daemon that holds no user-action key, the chat is out of the caller's reach or is a subagent's (SD-11) */ 403: unknown; /** @@ -5652,7 +5652,7 @@ export type InterruptErrors = { */ 400: unknown; /** - * The request was not proven to come from the user + * The request was not proven to come from the user; on a daemon that holds no user-action key, the chat is out of the caller's reach or is a subagent's (SD-11) */ 403: unknown; /**