From 3664845356e689a5aa7703d1793cbc936dd15b21 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 10:48:57 -0700 Subject: [PATCH 1/8] fix(server): Stop and steering work on a daemon with no user-action key (SD-11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `POST /agent/cancel`, `POST /interrupt`, `POST /agent/continuation/abandon` and `POST /agent/continuation/recover` began with an unconditional `is_user_action` check. A daemon started with no user-action key — every `biorouter serve` daemon (SD-7), and a hand-run `biorouterd` — can never pass it, so the browser's Stop button, mid-turn steering and Stop-and-Send answered 403 on every chat, public ones included (measured 2026-09-11 from the page). On such a daemon the four routes now gate through `authorize_agent_control`, the same call `/agent/stop` makes there: the reach rule, then no subagent's chat. The refusal protected nothing — `/agent/stop` already cancels the same turn for the same caller, `workspace_close { scope: "turn" }` does it for a model, and `/reply` already puts the caller's text in front of the chat. - A keyless steer is recorded unstamped, never `UserDirect`: nothing on that daemon can establish that a person typed it, and `/reply` records the same caller's message the same way. - A daemon that holds a key (the desktop app's) is unchanged: proof only. - A subagent's chat stays refused on a keyless daemon, and `refuse_subagent_unless_user` now says the daemon, not the caller, is what cannot prove a person acted (SD-8). `tests/turn_control_no_user_key.rs` is its own binary because the digest is a process-global OnceLock. All seven of its tests fail on the previous code; one of them pins the premise (keyless `/agent/cancel` admits exactly what keyless `/agent/stop` admits). The session_reach ordering census gains rows for the three routes and new over-read controls in reply.rs, since `interrupt` is no longer ungated. --- crates/biorouter-server/src/routes/agent.rs | 41 +- crates/biorouter-server/src/routes/reply.rs | 145 ++++- .../src/routes/session_reach.rs | 88 ++- .../tests/turn_control_no_user_key.rs | 533 ++++++++++++++++++ .../biorouter/tests/privacy_guard_wiring.rs | 10 +- 5 files changed, 758 insertions(+), 59 deletions(-) create mode 100644 crates/biorouter-server/tests/turn_control_no_user_key.rs diff --git a/crates/biorouter-server/src/routes/agent.rs b/crates/biorouter-server/src/routes/agent.rs index 8dc1604f8..ff90e0260 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..764e7d011 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,66 @@ 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. +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 +1843,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 +1854,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 +1886,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 +1917,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 +2131,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 +2144,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 +2160,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 +2171,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 +2206,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 +2217,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, 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..f9c156cf4 --- /dev/null +++ b/crates/biorouter-server/tests/turn_control_no_user_key.rs @@ -0,0 +1,533 @@ +//! 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. + +// 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 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 ae20fc417..7f777c6a6 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", From 13b4c93cd0fafaaba46261b15f0a15735d9d2838 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 10:54:31 -0700 Subject: [PATCH 2/8] =?UTF-8?q?docs(serve):=20SD-11=20=E2=80=94=20Stop=20a?= =?UTF-8?q?nd=20steering=20work=20on=20a=20daemon=20with=20no=20key?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the ruling the previous commit implements: on a daemon that holds no user-action key, /agent/cancel, /interrupt and the two continuation routes admit exactly the callers /agent/stop already admits there, a subagent's chat excepted; a keyless steer is unstamped; a daemon that holds a key is unchanged. The record answers privacy-tiers §3.1's two questions in writing, and its Q2 table records one finding rather than a guard: POST /active_work/{id}/cancel cancels a subagent by registry id with no gate on any daemon (the residual session_reach.rs already records). - programmatic-session-access.md: the three routes honour the capability header on a keyless daemon; the "different instrument" row now says the proof applies on a daemon that holds a key; the troubleshooting entry no longer claims a capable caller never sees a "no user-action key" refusal (a subagent's session produces one). - browser-access.md: Stop, steering and Stop-and-Send work in an ordinary browser chat; a subagent's tab is read-only and does not yet say so. - CLAUDE.md: an SD-11 bullet in "Browser access". Numbered SD-11 because open PRs #226 and #229 both claim SD-9. --- CLAUDE.md | 8 ++ docs/deployment/browser-access.md | 2 + .../deployment/programmatic-session-access.md | 10 +- docs/deployment/serve-decisions.md | 128 +++++++++++++++++- 4 files changed, 144 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 17e8c1799..11de4edbc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1188,6 +1188,14 @@ 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`. - **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/docs/deployment/browser-access.md b/docs/deployment/browser-access.md index 299acf6b2..6e224e82a 100644 --- a/docs/deployment/browser-access.md +++ b/docs/deployment/browser-access.md @@ -186,6 +186,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 f413d2bef..d84e71c06 100644 --- a/docs/deployment/serve-decisions.md +++ b/docs/deployment/serve-decisions.md @@ -17,7 +17,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 @@ -227,6 +228,131 @@ can never half-believe a person is reachable. --- +## 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 attach` and `session cancel` → these routes | user at a terminal | the proof, which the CLI demands before it sends anything | `commands/session_watch.rs::build_user_action_post_request` | + + 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 CLI's +`session cancel` and `attach` steering still demand a user-action key from the terminal before they +send anything, so against a keyless daemon they refuse locally a request the daemon would now admit. + +--- + ## Related documentation - [Architecture of the serving path](serve-architecture.md) — how the decisions above are built. From ea656a112aa89665c44ce0c4d40691c909645ed7 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 10:54:31 -0700 Subject: [PATCH 3/8] chore(api): regenerate OpenAPI for the SD-11 403 descriptions --- ui/desktop/openapi.json | 8 ++++---- ui/desktop/src/api/types.gen.ts | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index 1a66a7468..626a4ebaf 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 68b657939..d658b5ac0 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -4413,7 +4413,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; /** @@ -4450,7 +4450,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; /** @@ -4483,7 +4483,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; /** @@ -5645,7 +5645,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; /** From 46d5f7e99c5da115fbb32eca2fe0b7f9a3e4d845 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 14:32:04 -0700 Subject: [PATCH 4/8] fix(cli): ask the daemon before asking for the user-action key (SD-11 follow-up) `session cancel`, `attach` and `send` demanded the user-action key from the terminal before they sent anything, so against a daemon that holds no key (`biorouter serve`, a hand-run `biorouterd agent`) they refused locally the stop and steer SD-11 now admits there, and asked every `serve` user for a key that does not exist. A terminal cannot tell the two kinds of daemon apart, but the daemon's answer can: on the turn-control routes a daemon that holds a key refuses an unproven request with an EMPTY 403, and one that holds none refuses only in its own words. So each command now sends its request without the proof (unless the key was supplied with --user-action-key-stdin), asks the person for the key only on that empty 403, and makes the request once more with it. A refusal that carries a sentence is printed, since no key would change it. - attach settles the key 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 the stdin reader then holds stdin's lock and a hidden prompt would block on it, or race the reader and hand the key to the session as a message. - send asks the same question after a refused /reply, whose own 403 for a subagent is empty on either kind of daemon. - A wrong key, typed or supplied, is reported as wrong and never re-prompted. Nothing is relaxed: the daemon stays the boundary, every refusal read here is given before the route touches the turn, subagent sessions still need the proof, and the raw key still comes only from the terminal or stdin. It is now sent only when supplied or asked for. --- crates/biorouter-cli/src/cli.rs | 6 +- .../src/commands/session_watch.rs | 1212 +++++++++++++++-- 2 files changed, 1108 insertions(+), 110 deletions(-) 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..5132c2301 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)] +pub(crate) 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)] +pub(crate) 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), +} + +pub(crate) 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. +pub(crate) 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?; @@ -1067,6 +1281,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 +1336,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 +1441,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 +1466,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 +1683,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 +1737,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 +1778,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 +2037,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 +2107,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 +2144,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 +2258,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 +2652,610 @@ 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() { + let refused = keyless_refusal_body(); + let admitted = (200u16, "{}".to_string()); + let wants_key = (403u16, String::new()); + let in_words = (403u16, refused.clone()); + // (daemon's answers in order, key supplied up front, key held per request, + // asked, final verdict) + let cases: Vec<(Vec<(u16, String)>, bool, Vec, u32, KeyVerdict)> = vec![ + // A daemon without a key admits it: one request, no key, nobody asked. + ( + vec![admitted.clone()], + false, + vec![false], + 0, + KeyVerdict::NotAsked, + ), + // A daemon with one wants it: asked once, and the second request carries it. + ( + vec![wants_key.clone(), admitted.clone()], + false, + vec![false, true], + 1, + KeyVerdict::NotAsked, + ), + // A daemon without a key refuses in words: shown, and nobody is asked. + ( + vec![in_words.clone()], + false, + vec![false], + 0, + KeyVerdict::Refused(KEYLESS_REFUSAL.to_string()), + ), + // A wrong key: refused again, and the person is not asked twice. + ( + vec![wants_key.clone(), wants_key.clone()], + false, + vec![false, true], + 1, + KeyVerdict::Wanted, + ), + // Supplied on stdin: sent at once, and never followed by a prompt — + // not when admitted, and not when refused either. + ( + vec![admitted.clone()], + true, + vec![true], + 0, + KeyVerdict::NotAsked, + ), + ( + vec![wants_key.clone()], + true, + vec![true], + 0, + KeyVerdict::Wanted, + ), + ]; + for (answers, supplied, expected_held, expected_asked, expected_verdict) in cases { + let held = std::cell::RefCell::new(Vec::::new()); + let asked = std::cell::Cell::new(0); + let auth = if supplied { + DaemonAuth::for_test_with_user_action("s3cret", "", "from-stdin") + } else { + DaemonAuth::for_test("s3cret", "") + }; + 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(), expected_held, "answers {answers:?}"); + assert_eq!(asked.get(), expected_asked, "answers {answers:?}"); + assert_eq!(verdict, expected_verdict, "answers {answers:?}"); + assert_eq!(auth.holds_key(), expected_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() + ) + } + + /// `authorize_turn_control`'s refusal on a daemon that holds a key. + fn empty_403() -> String { + "HTTP/1.1 403 Forbidden\r\ncontent-length: 0\r\n\r\n".to_string() + } + + /// 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() { + let daemon = FakeDaemon::start(vec![http("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 From 7cd22cf857242b37ecefb58819210930e4d83e51 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 14:32:14 -0700 Subject: [PATCH 5/8] test(server): pin the refusal shapes the terminal reads for the user-action key `biorouter session` now decides whether to ask a person for the user-action key from the shape of a turn-control refusal: an empty 403 means "this daemon holds a key", and a sentence means no key would help. Pin both halves from the daemon's side, so a change to either arm cannot silently stop the terminal prompting on the desktop's daemon, or make it prompt a `serve` user for a key that does not exist: - keyed (routes::reply integration_tests): an unproven stop or steer, empty text included, is a 403 with an EMPTY body, answered before the text is read; with the proof the empty steer is a 400; nothing touches the turn. - keyless (tests/turn_control_no_user_key.rs): the empty steer the terminal sends is a 400 where it may steer and otherwise a refusal carrying the daemon's sentence, never an empty 403, and it touches neither the turn nor the agent's queue. --- crates/biorouter-server/src/routes/reply.rs | 64 +++++++++++++++ .../tests/turn_control_no_user_key.rs | 78 +++++++++++++++++++ 2 files changed, 142 insertions(+) diff --git a/crates/biorouter-server/src/routes/reply.rs b/crates/biorouter-server/src/routes/reply.rs index 764e7d011..c905a5bed 100644 --- a/crates/biorouter-server/src/routes/reply.rs +++ b/crates/biorouter-server/src/routes/reply.rs @@ -1800,6 +1800,18 @@ enum TurnControlAuthority { /// ⚠ **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, @@ -4705,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/tests/turn_control_no_user_key.rs b/crates/biorouter-server/tests/turn_control_no_user_key.rs index f9c156cf4..1180d0d44 100644 --- a/crates/biorouter-server/tests/turn_control_no_user_key.rs +++ b/crates/biorouter-server/tests/turn_control_no_user_key.rs @@ -19,6 +19,15 @@ //! (`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`. @@ -481,6 +490,75 @@ async fn a_subagents_turn_is_still_refused_and_the_refusal_says_why() { 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 From f6347f894e993683827211ad9458bfced512d0ae Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 14:32:14 -0700 Subject: [PATCH 6/8] docs: the terminal asks the daemon before it asks for the user-action key Update workspace-control.md's user-action-key paragraph (it said attach and cancel prompt before steering, and that send needs no proof, while send prompted too), SD-11's CLI row and its "Not decided here" paragraph (now "The terminal, since"), the CLI command reference (which never mentioned the key or --user-action-key-stdin), the subagents page, and CLAUDE.md's SD-11 bullet with the refusal-shape invariant the CLI now depends on. --- CLAUDE.md | 7 +++++ docs/agent-loop/subagents.md | 2 +- docs/agent-loop/workspace-control.md | 41 +++++++++++++++++++--------- docs/cli/command-reference.md | 19 +++++++++++++ docs/deployment/serve-decisions.md | 32 +++++++++++++++++++--- 5 files changed, 83 insertions(+), 18 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 39fc1c89e..a4b03869b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1219,6 +1219,13 @@ replaced a standalone `biorouter-headless` binary and its Linux tarball, both de 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/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/serve-decisions.md b/docs/deployment/serve-decisions.md index 601bd9260..c555a253f 100644 --- a/docs/deployment/serve-decisions.md +++ b/docs/deployment/serve-decisions.md @@ -373,7 +373,7 @@ answers in writing ([privacy tiers §3.1](../security/privacy-tiers.md#31-the-re | `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 attach` and `session cancel` → these routes | user at a terminal | the proof, which the CLI demands before it sends anything | `commands/session_watch.rs::build_user_action_post_request` | + | `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 @@ -404,9 +404,33 @@ caller would. It is recorded rather than closed: on a daemon that cannot tell a 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 CLI's -`session cancel` and `attach` steering still demand a user-action key from the terminal before they -send anything, so against a keyless daemon they refuse locally a request the daemon would now admit. +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`. --- From ade905ea76113232ac5f8bda55bd8d286a21d67b Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 15:54:45 -0700 Subject: [PATCH 7/8] fix(cli): read a chunked empty refusal as empty, not as a body of "0" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured against a real daemon holding a user-action key: hyper answers an empty 403 with `transfer-encoding: chunked` and a lone `0\r\n\r\n` terminator, and `post_json` handed the framing back as the body. The terminal read "0" as the daemon's refusal sentence, so `session cancel` printed "the daemon would not stop the turn: 0" and never asked for the key the daemon was waiting for — the whole point of that empty refusal. `post_json` and `daemon_json_request` now share one response parser, which dechunks as the latter already did. The fake daemon in the tests frames its empty 403 the way the real one does, since a `content-length: 0` fixture passed every test while the real thing was misread. --- .../src/commands/session_watch.rs | 128 +++++++++++++----- 1 file changed, 95 insertions(+), 33 deletions(-) diff --git a/crates/biorouter-cli/src/commands/session_watch.rs b/crates/biorouter-cli/src/commands/session_watch.rs index 5132c2301..91030b221 100644 --- a/crates/biorouter-cli/src/commands/session_watch.rs +++ b/crates/biorouter-cli/src/commands/session_watch.rs @@ -1123,14 +1123,46 @@ async fn post_json_to( .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 @@ -1172,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 @@ -2968,9 +2976,60 @@ mod tests { ) } - /// `authorize_turn_control`'s refusal on a daemon that holds a key. + /// 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\ncontent-length: 0\r\n\r\n".to_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 @@ -3039,7 +3098,10 @@ mod tests { /// 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() { - let daemon = FakeDaemon::start(vec![http("403 Forbidden", &keyless_refusal_body())]).await; + // 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, From fbef097f7e98e5e9030cc3e7b06206f434a77929 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 16:05:26 -0700 Subject: [PATCH 8/8] refactor(cli): keep the key-verdict types to the module that reads them `KeyUse`, `KeyVerdict`, `key_verdict` and `refusal_sentence` have no reader outside `session_watch`, so they are private to it. The table in `the_person_is_asked_for_the_key_only_after_the_daemon_wants_it` becomes a named `Case` struct: its five-part tuple tripped `clippy::type_complexity`, and the field names say what each column is. --- .../src/commands/session_watch.rs | 125 ++++++++++-------- 1 file changed, 68 insertions(+), 57 deletions(-) diff --git a/crates/biorouter-cli/src/commands/session_watch.rs b/crates/biorouter-cli/src/commands/session_watch.rs index 91030b221..41295ea0e 100644 --- a/crates/biorouter-cli/src/commands/session_watch.rs +++ b/crates/biorouter-cli/src/commands/session_watch.rs @@ -169,7 +169,7 @@ fn non_empty_key(key: Zeroizing) -> Result> { /// What the key is wanted for, in the words its prompt and its refusals use. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum KeyUse { +enum KeyUse { /// `session cancel` — `POST /agent/cancel`. Stop, /// `session attach`, asked as it joins — `POST /interrupt`. @@ -245,7 +245,7 @@ impl KeyUse { /// 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)] -pub(crate) enum KeyVerdict { +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 @@ -256,7 +256,7 @@ pub(crate) enum KeyVerdict { Refused(String), } -pub(crate) fn key_verdict(code: u16, body: &str) -> KeyVerdict { +fn key_verdict(code: u16, body: &str) -> KeyVerdict { if code != 403 { return KeyVerdict::NotAsked; } @@ -271,7 +271,7 @@ pub(crate) fn key_verdict(code: u16, body: &str) -> KeyVerdict { /// `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. -pub(crate) fn refusal_sentence(body: &str) -> Option { +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()); @@ -2795,70 +2795,81 @@ mod tests { /// 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() { - let refused = keyless_refusal_body(); + /// 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, refused.clone()); - // (daemon's answers in order, key supplied up front, key held per request, - // asked, final verdict) - let cases: Vec<(Vec<(u16, String)>, bool, Vec, u32, KeyVerdict)> = vec![ + let in_words = (403u16, keyless_refusal_body()); + let cases = vec![ // A daemon without a key admits it: one request, no key, nobody asked. - ( - vec![admitted.clone()], - false, - vec![false], - 0, - KeyVerdict::NotAsked, - ), + 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. - ( - vec![wants_key.clone(), admitted.clone()], - false, - vec![false, true], - 1, - KeyVerdict::NotAsked, - ), + 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. - ( - vec![in_words.clone()], - false, - vec![false], - 0, - KeyVerdict::Refused(KEYLESS_REFUSAL.to_string()), - ), + 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. - ( - vec![wants_key.clone(), wants_key.clone()], - false, - vec![false, true], - 1, - KeyVerdict::Wanted, - ), + 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. - ( - vec![admitted.clone()], - true, - vec![true], - 0, - KeyVerdict::NotAsked, - ), - ( - vec![wants_key.clone()], - true, - vec![true], - 0, - KeyVerdict::Wanted, - ), + 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 (answers, supplied, expected_held, expected_asked, expected_verdict) in cases { + for case in cases { let held = std::cell::RefCell::new(Vec::::new()); let asked = std::cell::Cell::new(0); - let auth = if supplied { + 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| { @@ -2871,10 +2882,10 @@ mod tests { ) .await .unwrap(); - assert_eq!(*held.borrow(), expected_held, "answers {answers:?}"); - assert_eq!(asked.get(), expected_asked, "answers {answers:?}"); - assert_eq!(verdict, expected_verdict, "answers {answers:?}"); - assert_eq!(auth.holds_key(), expected_held.last() == Some(&true)); + 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)); } }