From 14207c4d70164615d05d025bcc6bd63684f4f525 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Sat, 12 Sep 2026 19:30:05 -0700 Subject: [PATCH] fix(privacy): /sessions/running names a chat only to a caller that could open it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GET /sessions/running` returned `state.active_turn_session_ids()` verbatim: no `HeaderMap`, no reach gate, no tier filter. Every chat holding a turn — private ones included — was named to any caller holding the daemon secret, and because an id appears while the turn runs and vanishes when it ends, POLLING the route timed a private chat's turns. Measured on main (e7002972) against a live `biorouter serve`, X-Secret-Key and no other header, while private chat 20260913_1 (versa_azure, tier private) held a turn: GET /sessions/running -> {"session_ids":["20260913_1"]} samples 4-10, then {"session_ids":[]} when the turn ended GET /sessions/20260913_1 -> 403, 284 bytes GET /sessions/zzz_missing -> 403, 284 bytes, byte-identical body GET /sessions -> 200, 4751 rows, that row absent GET /active_work -> {"items":[]} throughout So the sibling that refuses to confirm the chat EXISTS sat beside one publishing its id and its timing. The route now resolves one `session_reach::http_caller` for the whole list and keeps an id only when `HttpCaller::lists_work` admits it — the `GET /active_work` decision (PR #257), not `GET /sessions`' (PR #237), because this route holds IDS rather than rows: each chat has to be resolved, resolution can fail, and a turn held on a chat this daemon cannot read is `TargetTier::Unreadable`, answered as a private one. FILTER and not refuse, for `GET /sessions`' reason — a 403 would break `biorouter session list`'s liveness on the public chats this gate is deliberately inert on. An unproven caller now gets `{"session_ids":[]}`, byte-identical to the body it gets when nothing is running. Re-measured on the same store with the same private chat: secret only -> {"session_ids":[]} 45/45 samples x-caller-provider -> {"session_ids":["20260913_2"]} while the turn ran served cookie (SD-10)-> {"session_ids":["20260913_2"]} while the turn ran status 200, so it is a filter and not a refusal Callers. The renderer never calls this route — only the generated SDK declares it. The one real caller is the CLI (`biorouter session list`, `session watch`), and it is unaffected: `session_watch::DaemonAuth::headers` states this terminal's configured provider on every request, so a private-provider install still sees everything, and a public one sees exactly the chats `GET /sessions` already handed it — every row it can display keeps its liveness. Verified live: twelve consecutive `session list --subagents` runs with no "liveness is unknown" note, and the note does appear on a wrong secret, so the check can fail. The module header carried `/sessions/running` as deliberately open. Both halves of that reason were wrong and the doc now says so: "ids only" treated a chat's id as metadata when `GET /sessions/{id}` refuses to confirm the same id exists, and the claim that the CLI "needs it whole" had the dependency backwards — it needed exactly the filtered set. Tests. A fail-before HTTP test in `session_reach`: it fails on main's handler with `{"session_ids":[""]}`, and fails the other way too — stubbing the filter to drop every row trips the phase-3 assertion that the PUBLIC id must survive, so it cannot be satisfied by a route that refuses everyone. The census rows are extended, never duplicated: `http_caller` at session.rs 10 -> 11 refs with 8 calls unchanged (a listing, not a ninth reach decision), and a new `lists_work` site for session.rs. `running_sessions` is removed from the ordering test's ungated over-read controls, since it is gated now; `pub fn routes(` takes the after-side. The module's liveness test fabricates ids that are in no store, so it now states the user-action proof and says why. Gates: cargo fmt, clippy-lint.sh, 668/668 biorouter-server --lib, privacy guard-wiring and capability censuses, openapi schema + TS client regenerated, format:check and lint:check. --- crates/biorouter-server/src/routes/session.rs | 76 ++++++-- .../src/routes/session_reach.rs | 180 +++++++++++++++++- .../biorouter/tests/privacy_guard_wiring.rs | 28 ++- ui/desktop/openapi.json | 2 +- ui/desktop/src/api/types.gen.ts | 2 +- 5 files changed, 258 insertions(+), 30 deletions(-) diff --git a/crates/biorouter-server/src/routes/session.rs b/crates/biorouter-server/src/routes/session.rs index 3ddbba71..ec5284f6 100644 --- a/crates/biorouter-server/src/routes/session.rs +++ b/crates/biorouter-server/src/routes/session.rs @@ -1738,14 +1738,48 @@ pub struct RunningSessionsResponse { // is invisible to it. tag = "workspace", responses( - (status = 200, description = "Sessions with a turn in flight", body = RunningSessionsResponse), + (status = 200, description = "Sessions with a turn in flight, holding only the chats this \ + caller could open: a private chat's id, and one this daemon \ + cannot read, are omitted — never redacted — for a caller \ + with neither the user-action proof nor a private \ + capability, exactly as `GET /active_work` omits that chat's \ + running work. An omitted row is indistinguishable from \ + nothing running", body = RunningSessionsResponse), (status = 401, description = "Unauthorized - invalid secret key") ) )] -async fn running_sessions(State(state): State>) -> Json { - Json(RunningSessionsResponse { - session_ids: state.active_turn_session_ids(), - }) +async fn running_sessions( + State(state): State>, + headers: axum::http::HeaderMap, +) -> Json { + // Issue #56: this published the id of EVERY chat holding a turn — and, to + // anything polling it, when each one started and stopped — to a caller + // holding nothing but the daemon secret, while `GET /sessions/{id}` on that + // same id answered 403 and `GET /sessions` omitted the row. A running + // private chat is exactly what `GET /active_work` was closed for on + // 2026-09-11; this route is that same enumeration with the content stripped + // off, so it takes the same decision. + // + // FILTER, not refuse, for `GET /sessions`' reason: a 403 would break + // `biorouter session list`'s liveness on the public chats this gate is + // deliberately inert on. A caller that can prove nothing is answered + // `{"session_ids":[]}` — byte-for-byte the body it gets when nothing is + // running, so an omission is not an oracle. + let caller = crate::routes::session_reach::http_caller(&headers).await; + let manager = state.session_manager(); + let mut session_ids = Vec::new(); + for session_id in state.active_turn_session_ids() { + // `lists_work` and not `lists_session`: this route holds IDS, not rows, + // so each chat has to be resolved and resolution can fail. A turn held + // on a chat this daemon cannot read is `TargetTier::Unreadable` and so + // is answered as a private one — `work_reach`'s rule for work that names + // no chat, for the same reason. ONE resolved caller for the whole list, + // so the rows cannot half-believe two answers. + if caller.lists_work(manager, Some(&session_id)).await { + session_ids.push(session_id); + } + } + Json(RunningSessionsResponse { session_ids }) } pub fn routes(state: Arc) -> Router { @@ -2852,14 +2886,26 @@ pub(crate) mod diverge_tests { manager.delete_session(&original.id).await.unwrap(); } - async fn get_running(state: Arc) -> Vec { + /// ⚠ **The proof header is load-bearing, and it is why this helper takes + /// one.** Since the 2026-09-12 serve sweep `GET /sessions/running` filters + /// each id through `session_reach::HttpCaller::lists_work`, and the ids this + /// module's liveness test fabricates are in NO store — `TargetTier:: + /// Unreadable`, which an unproven caller is refused exactly as it is refused + /// a private chat. So an unproven request here would answer `[]` and the + /// liveness assertions below would pass while measuring the gate rather than + /// the turn map. The person at the keyboard is the right caller for a test + /// about bookkeeping; whose ids the route hands out is measured in + /// `session_reach`'s own HTTP tests, against chats that really exist. + async fn get_running(state: Arc, headers: &[(&str, &str)]) -> Vec { let app = routes(state); - let req = Request::builder() - .method("GET") - .uri("/sessions/running") - .body(Body::empty()) + let mut builder = Request::builder().method("GET").uri("/sessions/running"); + for (name, value) in headers { + builder = builder.header(*name, *value); + } + let res = app + .oneshot(builder.body(Body::empty()).unwrap()) + .await .unwrap(); - let res = app.oneshot(req).await.unwrap(); let bytes = to_bytes(res.into_body(), usize::MAX).await.unwrap(); let json: serde_json::Value = serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null); @@ -2895,6 +2941,8 @@ pub(crate) mod diverge_tests { #[tokio::test(flavor = "multi_thread")] #[serial] async fn running_sessions_reports_exactly_the_sessions_holding_a_turn() { + install_test_user_action_key(); + let proof: &[(&str, &str)] = &[("X-User-Action", TEST_USER_ACTION_KEY)]; let state = AppState::new().await.unwrap(); // ⚠ NOT `uuid::Uuid::new_v4()`: `uuid` is not a dependency of // `biorouter-server`, so that would be an unresolved-crate error. A @@ -2908,14 +2956,14 @@ pub(crate) mod diverge_tests { // A cheap precondition, not a strong one — this map starts empty. Kept // so the failure message names the offender if that ever stops holding. - let before = get_running(state.clone()).await; + let before = get_running(state.clone(), proof).await; assert!(!before.contains(&busy), "precondition: {before:?}"); let guard = state .try_begin_turn_idempotent(&busy, CancellationToken::new(), None) .expect("nothing holds this fabricated session"); - let during = get_running(state.clone()).await; + let during = get_running(state.clone(), proof).await; assert!(during.contains(&busy), "a held turn must be reported"); assert!( !during.contains(&idle), @@ -2924,7 +2972,7 @@ pub(crate) mod diverge_tests { drop(guard); assert!( - !get_running(state.clone()).await.contains(&busy), + !get_running(state.clone(), proof).await.contains(&busy), "TurnGuard::drop clears the slot, so the route must read LIVE state: \ a snapshot taken at construction passes every assertion above and \ fails this one" diff --git a/crates/biorouter-server/src/routes/session_reach.rs b/crates/biorouter-server/src/routes/session_reach.rs index 02450a9f..29dc460b 100644 --- a/crates/biorouter-server/src/routes/session_reach.rs +++ b/crates/biorouter-server/src/routes/session_reach.rs @@ -45,11 +45,30 @@ //! one-word change of URL. It and `GET /schedule/{id}/inspect` now ask //! [`work_reach`]; `GET /schedule/list` keeps every row and redacts the //! chat-naming FIELDS, because a schedule names chats rather than being one -//! and an idle schedule names none. `GET /sessions/running` (ids -//! only, and `biorouter session list` needs it whole to report liveness -//! truthfully), `GET /sessions/changes` (a watched row's provider, model and -//! tier columns), `GET /sessions/insights` and `GET /sessions/activity` -//! (aggregates) remain open too. +//! and an idle schedule names none. ⚠ **`GET /sessions/running` sat on this +//! list as deliberately open until 2026-09-12, and BOTH halves of the reason +//! given for it were wrong.** "Ids only" treated a chat's id as metadata when +//! the sibling `GET /sessions/{id}` refuses to confirm that same id EXISTS — +//! it answers an unknown id and a private one with one byte-identical +//! sentence, precisely so that nothing says which chats are there. And the +//! clause about `biorouter session list` had its dependency backwards: that +//! client sends its capability header on this request like any other +//! (`session_watch::DaemonAuth::headers`) and renders liveness only for the +//! rows `GET /sessions` has already handed it — rows that listing filters by +//! this same decision — so it never needed the route whole. It needed exactly +//! the filtered set, which is what it now gets. Driving `biorouter serve` on +//! 2026-09-12 measured what the sentence cost: holding X-Secret-Key alone, the +//! route returned a private chat's id for as long as that chat held a turn and +//! dropped it when the turn ended, so polling it timed a private chat's turns +//! — while `GET /active_work` beside it answered `{"items":[]}` throughout and +//! `GET /sessions/{that id}` answered 403. It now filters through +//! [`HttpCaller::lists_work`], the `/active_work` decision, for the reason that +//! route takes it: this one holds ids rather than rows, so each chat is +//! resolved, and a turn on a chat this daemon cannot read is answered as a +//! private chat's. An omitted id is indistinguishable from nothing running. +//! `GET /sessions/changes` (a watched row's provider, model and tier columns), +//! `GET /sessions/insights` and `GET /sessions/activity` (aggregates) remain +//! open. //! ⚠ **This bullet listed `POST /agent/resume` as open until 2026-09-04, and //! it was wrong** — measured against a live private session, `/agent/resume` //! answers 403 without the capability header and 200 with it, because @@ -1759,9 +1778,13 @@ mod tests { // `pub fn routes(` sits on the far side of the rows reply.rs // contributes. `get_session_extensions` was this file's other ungated // control until QA's 2026-09-10 sweep gated it, which is why it can no - // longer serve as one; `get_session_insights` and `running_sessions` - // replace it — machine-wide aggregates that name no chat — on the two - // sides of this file's gated handlers. + // longer serve as one. `running_sessions` replaced it and has now gone + // the same way: the 2026-09-12 serve sweep measured it naming every + // private chat holding a turn to a secret-only caller, so it filters + // through `http_caller`/`lists_work` and is no longer ungated either. + // `get_session_insights` — a machine-wide aggregate that names no chat + // — and `pub fn routes(` replace the pair, on the two sides of this + // file's gated handlers. // // BOTH sides in `agent.rs`: `agent_remove_extension` sits after the two // gated handlers' neighbourhood and `update_agent_provider` before it, @@ -1772,7 +1795,7 @@ mod tests { (reply_rs, "pub async fn interrupt"), (reply_rs, "pub fn routes("), (session_rs, "async fn get_session_insights("), - (session_rs, "async fn running_sessions("), + (session_rs, "pub fn routes("), (agent_rs, "async fn agent_remove_extension"), (agent_rs, "async fn update_agent_provider"), // BOTH sides in the two files this sweep added, for the same reason: @@ -3668,6 +3691,145 @@ mod bypass_tests { } } + /// Pull the id set out of a `GET /sessions/running` body. + async fn running_ids(state: Arc, headers: &[(&str, &str)]) -> (String, Vec) { + let (status, body) = call(state, "GET", "/sessions/running", None, headers).await; + assert_eq!(status, StatusCode::OK, "{headers:?}: {body}"); + let json: serde_json::Value = serde_json::from_str(&body).unwrap(); + let ids = json["session_ids"] + .as_array() + .expect("session_ids is an array") + .iter() + .map(|id| id.as_str().unwrap().to_string()) + .collect(); + (body, ids) + } + + /// `GET /sessions/running` named every chat holding a turn — private ones + /// included — to a caller holding nothing but the daemon secret, and because + /// the id appears while the turn runs and vanishes when it ends, POLLING it + /// timed a private chat's turns. + /// + /// Measured on `main` (e7002972) against a live `biorouter serve`, with + /// X-Secret-Key and no other header: `{"session_ids":["20260913_1"]}` for as + /// long as that private chat held a turn, while `GET /sessions/20260913_1` + /// answered 403 with the byte-identical body an id that never existed gets, + /// `GET /sessions` omitted the row, and `GET /active_work` — the sibling + /// closed on 2026-09-11 — answered `{"items":[]}` throughout. + /// + /// ⚠ **Phase 2's leak assertion is the cheap half; phases 2 and 3 together + /// are what make this test worth having.** A route that refused the request + /// outright, or answered `[]` to every unproven caller, satisfies "the + /// secret-only caller does not see the private id" perfectly while breaking + /// `biorouter session list`'s liveness on every public chat — a leak traded + /// for a broken client. So phase 3 requires the PUBLIC id to survive for + /// every caller, and phase 2 requires the secret-only body to be + /// byte-identical to the body that caller gets when nothing is running, + /// because an omission a caller can detect is the oracle the refusal is + /// worded to withhold. The final `drop` pins that the turn map is read live, + /// so phase 2 measured a running turn and not a row that is always absent. + #[tokio::test(flavor = "multi_thread")] + #[serial] + async fn the_running_list_names_a_chat_only_to_a_caller_that_could_open_it() { + install_test_user_action_key(); + let state = AppState::new().await.unwrap(); + let private = seed_private_chat(&state, "Running list private (test fixture)").await; + let public = seed_chat( + &state, + "Running list public (test fixture)", + SessionClassification::Public, + ) + .await; + + // PHASE 1 — nothing running. The body an omission has to be + // indistinguishable from, observed rather than assumed. + let (idle_body, idle_ids) = running_ids(state.clone(), &[]).await; + assert!( + !idle_ids.contains(&private.id().to_string()) + && !idle_ids.contains(&public.id().to_string()), + "precondition: neither seeded chat holds a turn yet, got {idle_ids:?}" + ); + + // PHASE 2 — the PRIVATE chat alone holds a turn, which is the leak in + // its purest form: on `main` this answered `{"session_ids":[""]}`. + // `try_begin_turn_idempotent` writes the in-memory turn map and never + // consults the store, so this is the whole route path without running a + // model. + let private_turn = state + .try_begin_turn_idempotent( + private.id(), + tokio_util::sync::CancellationToken::new(), + None, + ) + .expect("nothing holds the seeded private chat"); + let (secret_only_body, secret_only_ids) = running_ids(state.clone(), &[]).await; + assert!( + !secret_only_ids.contains(&private.id().to_string()), + "a caller holding only the daemon secret was handed the id of a PRIVATE chat while \ + it held a turn: {secret_only_body}" + ); + // ⚠ Byte-identical, not merely "the id is absent". A body that differed + // at all — a `has_more`, a count, a different ordering — would let a + // caller tell a running private chat from no private chat, which is the + // oracle `SESSION_OUT_OF_REACH` is one sentence for two answers to + // avoid, and which is how the sidebar's continuation value leaked a + // count of the rows it hid. + assert_eq!( + secret_only_body, idle_body, + "a secret-only caller can tell a running private chat from nothing running at all" + ); + for (headers, label) in [ + (&[PROOF][..], "the person at the keyboard"), + (&[PRIVATE_CAPABILITY][..], "a program on a private model"), + ] { + let (body, ids) = running_ids(state.clone(), headers).await; + assert!( + ids.contains(&private.id().to_string()), + "{label} lost the private chat's running turn: {body}" + ); + } + + // PHASE 3 — the PUBLIC chat holds one too. This is the half that says + // the fix is a filter and not a refusal: a route that answered `[]` to + // every unproven caller satisfies phase 2 perfectly while breaking + // `biorouter session list`'s liveness on every public chat. + let _public_turn = state + .try_begin_turn_idempotent( + public.id(), + tokio_util::sync::CancellationToken::new(), + None, + ) + .expect("nothing holds the seeded public chat"); + for (headers, sees_private) in [ + (&[][..], false), + (&[PROOF][..], true), + (&[PRIVATE_CAPABILITY][..], true), + ] { + let (body, ids) = running_ids(state.clone(), headers).await; + assert!( + ids.contains(&public.id().to_string()), + "{headers:?} lost the PUBLIC chat's running turn — a leak traded for a broken \ + client: {body}" + ); + assert_eq!( + ids.contains(&private.id().to_string()), + sees_private, + "{headers:?}: the private chat's id was {} the running list — {body}", + if sees_private { "missing from" } else { "in" } + ); + } + + // The turn map is read LIVE: drop the private turn and the callers that + // COULD see it stop seeing it, so phase 2 measured a running turn rather + // than a row that is always there. + drop(private_turn); + let (body, ids) = running_ids(state.clone(), &[PROOF]).await; + assert!( + !ids.contains(&private.id().to_string()) && ids.contains(&public.id().to_string()), + "the route must read the live turn map: {body}" + ); + } + /// Paging a FILTERED sidebar must still walk every visible row exactly /// once: a filter applied after `LIMIT` would hand back short, ragged pages /// and let `has_more` count the rows it hid. diff --git a/crates/biorouter/tests/privacy_guard_wiring.rs b/crates/biorouter/tests/privacy_guard_wiring.rs index 576fd6fb..f65f0497 100644 --- a/crates/biorouter/tests/privacy_guard_wiring.rs +++ b/crates/biorouter/tests/privacy_guard_wiring.rs @@ -421,7 +421,7 @@ const REGISTRY: &[Guard] = &[ }, Site { file: "crates/biorouter-server/src/routes/session.rs", - counts: c(8, 10, 0), + counts: c(8, 11, 0), kind: SiteKind::Guard, what: "`GET /sessions/{id}` (the transcript) and `GET /sessions/{id}/export` \ (the same transcript, `to_string_pretty`), and — QA 2026-09-10 F0 and \ @@ -429,8 +429,13 @@ const REGISTRY: &[Guard] = &[ /sessions/{id}` (measured deleting a private chat the read refused, four \ of four), `PUT …/name`, `PUT …/user_workflow_values`, the in-place arm \ of `POST …/edit_message` (it truncates), `GET …/extensions` and `GET \ - …/usage`. Ten refs: the module qualifier on each of the eight calls, \ - and on `http_caller` for the two listings", + …/usage`. ELEVEN refs and still EIGHT calls: the module qualifier on \ + each of the eight calls, and on `http_caller` for the THREE listings — \ + `GET /sessions`, `GET /sessions/sidebar`, and, since the 2026-09-12 serve \ + sweep, `GET /sessions/running`. ⚠ The eleventh ref is a LISTING and not \ + a ninth reach decision, which is why `calls` did not move: a listing asks \ + `HttpCaller` whether to SHOW a row and drops it silently, where a call \ + here refuses the request outright", }, Site { file: "crates/biorouter-server/src/routes/skills.rs", @@ -570,10 +575,12 @@ const REGISTRY: &[Guard] = &[ }, Site { file: "crates/biorouter-server/src/routes/session.rs", - counts: c(2, 0, 0), + counts: c(3, 0, 0), kind: SiteKind::Guard, what: "`GET /sessions` and `GET /sessions/sidebar` — QA 2026-09-10 M1, every \ - chat on the machine, titled, to a secret-only caller", + chat on the machine, titled, to a secret-only caller — and, since the \ + 2026-09-12 serve sweep, `GET /sessions/running`, which named every chat \ + holding a turn to that same caller and, polled, timed each one", }, Site { file: SESSION_REACH, @@ -634,6 +641,17 @@ const REGISTRY: &[Guard] = &[ background jobs, foreground commands, subagents, detached turns and scheduled \ runs alike — after one `http_caller` for the whole list", }, + Site { + file: "crates/biorouter-server/src/routes/session.rs", + counts: c(1, 0, 0), + kind: SiteKind::Guard, + what: "`GET /sessions/running`, whose every row IS a chat id — so the id is \ + shown exactly when `GET /active_work` would show that chat's work. It \ + holds ids and not rows, which is why it takes this predicate rather than \ + `lists_session`: the chat must be resolved, resolution can fail, and a \ + turn held on a chat this daemon cannot read is answered as a private \ + chat's", + }, Site { file: "crates/biorouter-server/src/routes/schedule.rs", counts: c(2, 0, 0), diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index 2472aa41..20721906 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -4083,7 +4083,7 @@ "operationId": "running_sessions", "responses": { "200": { - "description": "Sessions with a turn in flight", + "description": "Sessions with a turn in flight, holding only the chats this caller could open: a private chat's id, and one this daemon cannot read, are omitted — never redacted — for a caller with neither the user-action proof nor a private capability, exactly as `GET /active_work` omits that chat's running work. An omitted row is indistinguishable from nothing running", "content": { "application/json": { "schema": { diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index 1840d2f8..4ec4346a 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -7408,7 +7408,7 @@ export type RunningSessionsErrors = { export type RunningSessionsResponses = { /** - * Sessions with a turn in flight + * Sessions with a turn in flight, holding only the chats this caller could open: a private chat's id, and one this daemon cannot read, are omitted — never redacted — for a caller with neither the user-action proof nor a private capability, exactly as `GET /active_work` omits that chat's running work. An omitted row is indistinguishable from nothing running */ 200: RunningSessionsResponse; };