From b1e87dc11cd70b750748e7a00e3981b01d39cef6 Mon Sep 17 00:00:00 2001 From: hellno Date: Mon, 13 Jul 2026 11:43:02 +0200 Subject: [PATCH 1/4] =?UTF-8?q?feat(connections):=20#199=20=C2=B7=20list?= =?UTF-8?q?=20live=20dapp=20sessions=20in=20the=20app,=20revocable=20per?= =?UTF-8?q?=20site?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ending beat of the browser demo: connect → attribute → revoke → the site is locked out. Revoke is a session-scoped DISCONNECT, not a ban (ADR 0006): the revoked site's next request fails 4100 Unauthorized and it may reconnect via eth_requestAccounts for a fresh session. Bridge: sessions stay in bridge RAM (DappSessionStore gains list/revoke_all; revoke() wakes from dead code). New POST /admin on the existing loopback server (list | revoke | clear — every op returns the post-op live list). Gate: /admin requires the x-deckard-admin header, which is deliberately NEVER CORS-allowed — a web page can't clear the preflight, so only local processes (the app) can enumerate or disconnect sessions. No daemon/wire/contract changes at all — the daemon-relay alternative would pay the #31 wire tax for zero alpha benefit. App: hand-rolled blocking loopback client (bridge_admin.rs, no new deps), a 5s whole-session connections poll mirroring the #200 pending poll (epoch- fenced + in-flight-guarded, plus a connections_epoch so a stale poll reply can't resurrect a just-revoked row), sidebar Connections rows (Globe + host + ghost ✕), the rail's Connections count goes live, and ⌘K gains revoke-site with a live label (1 site → its host; N → "all N") whose action always matches the label. Lock and Lockdown best-effort-clear all bridge sessions. Tests: revoke→4100→reconnect-fresh round-trip, admin dispatch semantics, the admin gate, a const-level CORS invariant, response-parser + origin_host units, registry order, and a new reflective registry↔handler test (every ⌘K id must have a run_palette_command arm). Closes #199 --- crates/deckard-app/src/bridge_admin.rs | 192 +++++++++++++ crates/deckard-app/src/main.rs | 1 + crates/deckard-app/src/palette.rs | 11 + crates/deckard-app/src/palette_commands.rs | 56 ++++ crates/deckard-app/src/shell.rs | 181 ++++++++++++ crates/deckard-app/src/shell_chrome.rs | 99 ++++++- crates/deckard-app/src/shell_rail.rs | 15 +- crates/deckard-browser-bridge/src/lib.rs | 309 ++++++++++++++++++++- docs/browser-bridge.md | 32 +++ 9 files changed, 872 insertions(+), 24 deletions(-) create mode 100644 crates/deckard-app/src/bridge_admin.rs diff --git a/crates/deckard-app/src/bridge_admin.rs b/crates/deckard-app/src/bridge_admin.rs new file mode 100644 index 0000000..4cf244a --- /dev/null +++ b/crates/deckard-app/src/bridge_admin.rs @@ -0,0 +1,192 @@ +//! A tiny blocking HTTP client to the browser bridge's `/admin` endpoint (#199). +//! +//! The app lists and disconnects dapp sessions by POSTing to the bridge's loopback HTTP server — +//! the same server that already serves `/rpc` — so there is ZERO daemon / wire / contract change +//! and no new dependency (this is a hand-rolled [`std::net::TcpStream`] client, not an HTTP crate). +//! +//! **Blocking, background-only.** Every function here does synchronous socket IO, so it MUST only +//! ever be called from inside `cx.background_spawn(...)`, never on the GPUI UI thread — a dead or +//! slow bridge would otherwise stall the frame. The connect/read/write timeouts below bound a hung +//! listener; a bridge that isn't running fails instantly with `ECONNREFUSED`. +//! +//! Revoke is a session-scoped DISCONNECT, not a ban (ADR 0006): a revoked site's next request +//! fails `4100 Unauthorized`, and it may reconnect for a fresh session. Sessions live only in the +//! bridge's RAM, so an unreachable bridge genuinely means no live sessions — an error maps to an +//! empty list, which is the honest answer, never a stale phantom row. + +use std::io::{Read, Write}; +use std::net::{TcpStream, ToSocketAddrs}; +use std::time::Duration; + +use serde::Deserialize; + +/// Environment override for the bridge's loopback address; defaults to the bridge/justfile default +/// (`deckard-browser-bridge --bind`, `just qa-bridge`/`demo-bridge` both use `127.0.0.1:8765`). +const BRIDGE_ADDR_ENV: &str = "DECKARD_BRIDGE_ADDR"; +const DEFAULT_BRIDGE_ADDR: &str = "127.0.0.1:8765"; + +/// How long to wait for the loopback TCP connect. A live bridge accepts instantly; a dead port +/// refuses instantly (`ECONNREFUSED`). This only bounds a wedged listener. +const CONNECT_TIMEOUT: Duration = Duration::from_millis(300); +/// Read/write timeouts once connected, so a hung bridge can't stall the background thread forever. +const IO_TIMEOUT: Duration = Duration::from_secs(2); + +/// One connected dapp site, as the Connections sidebar renders it (#199). Mirrors the bridge's live +/// `DappSession`, minus the fields the app doesn't display; serde ignores the extra `revoked` / +/// `permissions` keys the bridge still serializes (no `deny_unknown_fields`). +#[derive(Clone, Debug, PartialEq, Eq, Deserialize)] +pub struct ConnectedSite { + pub origin: String, + pub account: String, + pub chain_id: u64, + pub created_at: u64, + pub last_seen: u64, +} + +/// The bridge address to dial: `DECKARD_BRIDGE_ADDR` or the default loopback port. +fn bridge_addr() -> String { + std::env::var(BRIDGE_ADDR_ENV).unwrap_or_else(|_| DEFAULT_BRIDGE_ADDR.to_string()) +} + +/// POST one admin op body to `/admin` and parse the returned live session list. Blocking; +/// background-only. Writes a minimal HTTP/1.1 request with the `x-deckard-admin` marker header the +/// bridge gates on (a web page can't set it — the CORS preflight forbids it), then reads to EOF +/// (the bridge answers `connection: close`). +fn admin_request(body: &str) -> anyhow::Result> { + let addr = bridge_addr(); + // Resolve to a concrete `SocketAddr` so `connect_timeout` can bound the connect. The default is + // a literal IP:port (no DNS); a hostname override may resolve here, which is the caller's choice. + let socket_addr = addr + .to_socket_addrs()? + .next() + .ok_or_else(|| anyhow::anyhow!("bridge address {addr} did not resolve"))?; + let mut stream = TcpStream::connect_timeout(&socket_addr, CONNECT_TIMEOUT)?; + stream.set_read_timeout(Some(IO_TIMEOUT))?; + stream.set_write_timeout(Some(IO_TIMEOUT))?; + let request = format!( + "POST /admin HTTP/1.1\r\nhost: {addr}\r\nx-deckard-admin: 1\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{body}", + body.len() + ); + stream.write_all(request.as_bytes())?; + stream.flush()?; + let mut raw = Vec::new(); + stream.read_to_end(&mut raw)?; + parse_admin_response(&raw) +} + +/// Parse a bridge `/admin` HTTP response into the live session list. PURE (no IO) so it's unit- +/// testable with canned bytes: require `200 OK` on the status line, split headers from body on the +/// blank line, and read `{"sessions":[…]}`. Any non-200 is an error the caller maps to "no live +/// sessions" — the honest answer when the bridge refuses or is unhealthy. +pub fn parse_admin_response(raw: &[u8]) -> anyhow::Result> { + let text = std::str::from_utf8(raw)?; + let (head, body) = text + .split_once("\r\n\r\n") + .ok_or_else(|| anyhow::anyhow!("bridge response has no header/body separator"))?; + let status_line = head + .lines() + .next() + .ok_or_else(|| anyhow::anyhow!("bridge response has no status line"))?; + // `HTTP/1.1 200 OK` — the status code is the second whitespace-separated token. + let ok = status_line + .split_whitespace() + .nth(1) + .is_some_and(|code| code == "200"); + if !ok { + anyhow::bail!("bridge admin request failed: {status_line}"); + } + + #[derive(Deserialize)] + struct AdminResponse { + sessions: Vec, + } + let parsed: AdminResponse = serde_json::from_str(body.trim())?; + Ok(parsed.sessions) +} + +/// List the bridge's live (non-revoked) dapp sessions. Blocking; background-only. +pub fn list_sites() -> anyhow::Result> { + admin_request(r#"{"op":"list"}"#) +} + +/// Disconnect one site (session-scoped, NOT a ban — ADR 0006) and return the remaining live list. +/// Blocking; background-only. Revoking an unknown/already-gone origin is a no-op on the bridge. +pub fn revoke_site(origin: &str) -> anyhow::Result> { + let body = serde_json::json!({ "op": "revoke", "origin": origin }).to_string(); + admin_request(&body) +} + +/// Disconnect every live site (Lock / Lockdown reduce all authority) and return the now-empty list. +/// Blocking; background-only. +pub fn clear_sites() -> anyhow::Result> { + admin_request(r#"{"op":"clear"}"#) +} + +/// The host portion of a dapp origin for sidebar display: strips a leading `http://` / `https://` +/// so `https://app.uniswap.org` reads as `app.uniswap.org`. Anything without one of those schemes +/// (the bridge's `unknown-origin` fallback, or an unusual scheme) passes through unchanged — we +/// never fabricate or hide what the origin actually is. +pub fn origin_host(origin: &str) -> &str { + origin + .strip_prefix("https://") + .or_else(|| origin.strip_prefix("http://")) + .unwrap_or(origin) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Build a canned HTTP response so the pure parser can be exercised without a socket. + fn http(status_line: &str, body: &str) -> Vec { + format!( + "{status_line}\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{body}", + body.len() + ) + .into_bytes() + } + + #[test] + fn parse_admin_response_reads_sessions_on_200() { + let body = r#"{"sessions":[{"origin":"https://app.example","chain_id":11155111,"account":"0xabc","permissions":["eth_accounts"],"created_at":1,"last_seen":2,"revoked":false}]}"#; + let raw = http("HTTP/1.1 200 OK", body); + let sites = parse_admin_response(&raw).expect("200 parses"); + assert_eq!(sites.len(), 1); + assert_eq!(sites[0].origin, "https://app.example"); + assert_eq!(sites[0].account, "0xabc"); + assert_eq!(sites[0].chain_id, 11155111); + assert_eq!(sites[0].created_at, 1); + assert_eq!(sites[0].last_seen, 2); + } + + #[test] + fn parse_admin_response_reads_empty_session_list() { + let raw = http("HTTP/1.1 200 OK", r#"{"sessions":[]}"#); + assert!(parse_admin_response(&raw).expect("empty parses").is_empty()); + } + + #[test] + fn parse_admin_response_rejects_non_200() { + // A 400/403 must NOT masquerade as an empty session list — it's an error the caller maps to + // "no live sessions" itself, distinctly from a real empty 200. + let raw = http("HTTP/1.1 400 Bad Request", "unknown admin op"); + assert!(parse_admin_response(&raw).is_err()); + } + + #[test] + fn parse_admin_response_rejects_garbage() { + assert!(parse_admin_response(b"not an http response at all").is_err()); + // A 200 with a non-JSON body is still an error. + let raw = http("HTTP/1.1 200 OK", "nope"); + assert!(parse_admin_response(&raw).is_err()); + } + + #[test] + fn origin_host_strips_scheme_and_passes_unknown_through() { + assert_eq!(origin_host("https://app.uniswap.org"), "app.uniswap.org"); + assert_eq!(origin_host("http://127.0.0.1:8777"), "127.0.0.1:8777"); + // The bridge's fallback and any non-web scheme pass through unchanged — never disguised. + assert_eq!(origin_host("unknown-origin"), "unknown-origin"); + assert_eq!(origin_host("app.example.org"), "app.example.org"); + } +} diff --git a/crates/deckard-app/src/main.rs b/crates/deckard-app/src/main.rs index 39ab00e..c5b25a4 100644 --- a/crates/deckard-app/src/main.rs +++ b/crates/deckard-app/src/main.rs @@ -7,6 +7,7 @@ mod activity_view; mod agent_view; +mod bridge_admin; mod capture; mod commit_flow; mod commit_view; diff --git a/crates/deckard-app/src/palette.rs b/crates/deckard-app/src/palette.rs index ae76661..3b876f3 100644 --- a/crates/deckard-app/src/palette.rs +++ b/crates/deckard-app/src/palette.rs @@ -37,6 +37,17 @@ fn display_label(this: &Shell, id: &str, title: &str) -> String { "Mask balances".to_string() } } + // The Connections revoke command (#199) names exactly what Enter will disconnect: nothing + // (static title) with no sites, the single site's host with one, or "all N" with several — + // so the palette label and `run_palette_command`'s action never disagree. + "revoke-site" => match this.connections.as_slice() { + [] => title.to_string(), + [only] => format!( + "Revoke connected site — {}", + crate::bridge_admin::origin_host(&only.origin) + ), + many => format!("Revoke all {} connected sites", many.len()), + }, _ => title.to_string(), } } diff --git a/crates/deckard-app/src/palette_commands.rs b/crates/deckard-app/src/palette_commands.rs index d15911b..3d96d9a 100644 --- a/crates/deckard-app/src/palette_commands.rs +++ b/crates/deckard-app/src/palette_commands.rs @@ -171,6 +171,24 @@ pub const COMMANDS: &[Command] = &[ shortcut: None, icon: None, }, + Command { + // Disconnect a connected dapp site (#199). Session-scoped disconnect, not a ban (ADR 0006). + // The live label (palette.rs `display_label`) names the single site's host, or "all N", so + // the operator always sees exactly what Enter will disconnect. Per-site precision at N>1 is + // on the sidebar row's ✕. No bundled unlink glyph, so no icon. Kept BEFORE `revoke-all` so + // the panic brake stays the registry's last entry. + id: "revoke-site", + title: "Revoke connected site", + aliases: &[ + "disconnect", + "revoke site", + "unlink", + "connections", + "sites", + ], + shortcut: None, + icon: None, + }, Command { // The panic brake: zeroize the key, lock the daemon, deny every in-flight request. Its // OWN id — never overloads the demo `agent` toggle (whose "stop" means "stop the demo @@ -444,4 +462,42 @@ mod tests { let results = rank("zzzz", COMMANDS, &usage, 0, &mut m); assert!(results.is_empty()); } + + /// The Connections revoke command (#199) sits immediately BEFORE the STOP `revoke-all` panic + /// brake, which must stay the registry's last entry (`frecency_orders_empty_query` asserts it). + #[test] + fn revoke_site_precedes_revoke_all() { + let site = COMMANDS + .iter() + .position(|c| c.id == "revoke-site") + .expect("revoke-site must be in the registry"); + let all = COMMANDS + .iter() + .position(|c| c.id == "revoke-all") + .expect("revoke-all must be in the registry"); + assert_eq!( + site + 1, + all, + "revoke-site must sit right before revoke-all" + ); + assert_eq!(all, COMMANDS.len() - 1, "revoke-all must stay last"); + } + + /// Reflective registry↔handler check: EVERY registry `id` must have a handler arm in + /// `run_palette_command` (shell.rs). A command that ranks + displays but does nothing on Enter + /// is invisible-failure slop. Asserts the EXACT quoted match token `"" =>` is in shell.rs's + /// source (not a substring of some identifier), so a new registry entry can never ship without + /// its handler. + #[test] + fn every_command_id_has_a_handler_arm() { + let shell_src = include_str!("shell.rs"); + for cmd in COMMANDS { + let token = format!("\"{}\" =>", cmd.id); + assert!( + shell_src.contains(&token), + "command id {:?} has no handler arm ({token}) in shell.rs's run_palette_command", + cmd.id + ); + } + } } diff --git a/crates/deckard-app/src/shell.rs b/crates/deckard-app/src/shell.rs index 3dde1ad..6fe3605 100644 --- a/crates/deckard-app/src/shell.rs +++ b/crates/deckard-app/src/shell.rs @@ -29,6 +29,7 @@ use zeroize::Zeroizing; use deckard_signerd::SignerClient; +use crate::bridge_admin; use crate::commit_flow::CommitFlow; use crate::errors::{humanize_deny, humanize_swap_deny, is_session_ended, short_err}; use crate::settings::{Settings, ThemeModePref}; @@ -50,6 +51,12 @@ const BALANCE_POLL_SECS: u64 = 20; /// silently expires. const PENDING_POLL_SECS: u64 = 5; +/// Background Connections poll cadence (#199): how often the app asks the browser bridge's `/admin` +/// endpoint for its live dapp sessions, so the Connections sidebar stays current while the operator +/// works. Unlike the pending poll there is NO surface skip — the sidebar is visible everywhere. A +/// dead bridge fails instantly (`ECONNREFUSED`), so this loop is cheap when no bridge is running. +const CONNECTIONS_POLL_SECS: u64 = 5; + /// The confirm arm-delay: a clear-signing review must be on screen this long before ⌘↵ / a /// click can confirm, so a keypress or click carried over from the previous screen can't approve /// a money move (DESIGN §confirm pattern). The ⌘↵ chord plus this delay replace the old hold. @@ -394,6 +401,21 @@ pub struct Shell { /// True while a background `PendingList` round-trip is in flight, so a slow daemon can't /// stack fetches (the same role `portfolio_loading` plays for the balance poll). pending_poll_inflight: bool, + /// The browser bridge's live dapp sessions from the last `/admin` list poll (#199) — the + /// Connections sidebar's data. Empty when the bridge is unreachable (sessions live in the + /// bridge's RAM, so no bridge means no live sessions — the honest answer, never a phantom row). + /// `pub(crate)` so the sidebar (`shell_chrome`) and the palette label (`palette`) can read it; + /// refreshed by [`Shell::start_connections_poll`]. + pub(crate) connections: Vec, + /// Handle to the background Connections poll loop (#199); dropped on lock to cancel it. + connections_poll_task: Option>, + /// True while a `/admin` list round-trip is in flight, so a slow bridge can't stack fetches. + connections_inflight: bool, + /// Bumped by every disconnect (per-site revoke, ⌘K revoke-all-sites, Lock, Lockdown) so a list + /// poll reply that was already in flight — a snapshot from BEFORE the disconnect — is dropped + /// instead of folded in. Without this fence the stale reply could land last and resurrect the + /// just-revoked row for one poll interval, and "the row is gone" is the trust surface's promise. + connections_epoch: u64, /// Bumped on every `retarget`; a slow ENS resolution checks it before applying so a /// stale reply for a since-changed target can't clobber the current view. view_epoch: u64, @@ -769,6 +791,10 @@ impl Shell { pending_poll_task: None, polled_pending: None, pending_poll_inflight: false, + connections: Vec::new(), + connections_poll_task: None, + connections_inflight: false, + connections_epoch: 0, view_epoch: 0, current_rpc, chain_id, @@ -975,6 +1001,18 @@ impl Shell { self.pending_poll_task = None; self.polled_pending = None; self.pending_poll_inflight = false; + // Dapp sessions are authority too, and a Lock reduces ALL of it (#199): cancel the + // Connections poll, drop the local list, and best-effort-disconnect every live bridge + // session (detached, result ignored — the bridge is a separate process that may not even be + // running). Session-scoped disconnect, not a ban (ADR 0006). + self.connections_poll_task = None; + self.connections_inflight = false; + self.connections.clear(); + self.connections_epoch = self.connections_epoch.wrapping_add(1); + cx.background_spawn(async move { + let _ = bridge_admin::clear_sites(); + }) + .detach(); // Supersede any in-flight `refresh_activity` reply: without this, a fetch started just // before the lock could land afterwards and re-install a dead session's snapshot into // `self.activity` / `self.inbox` (#216) — and off-surface `waiting_count` falls back to the @@ -1330,6 +1368,7 @@ impl Shell { self.kick_agent_policy(cx); self.start_balance_poll(cx); self.start_pending_poll(cx); + self.start_connections_poll(cx); } /// Fetch the daemon's live policy for the wallet home's "what the agent may do" fence (off @@ -2880,6 +2919,15 @@ impl Shell { /// kill is visible — #60 acceptance 3. pub fn stop_revoke_all(&mut self, cx: &mut Context) { self.activity_stop_arming = false; + // Lockdown reduces ALL authority, dapp sessions included (#199): best-effort-disconnect + // every live bridge session and clear the local list. Detached + ignored — the bridge is a + // separate process that may not be running, and the key-zeroize below is the real brake. + self.connections.clear(); + self.connections_epoch = self.connections_epoch.wrapping_add(1); + cx.background_spawn(async move { + let _ = bridge_admin::clear_sites(); + }) + .detach(); let client = self.signer.client(); self.activity_loading = true; cx.notify(); @@ -3070,6 +3118,123 @@ impl Shell { .detach(); } + /// Start the background Connections poll (#199): a low-cadence, whole-session loop that keeps + /// the Connections sidebar current with the browser bridge's live dapp sessions. Mirrors + /// [`start_pending_poll`], minus the Activity-surface skip — the sidebar is visible on every + /// surface, so this poll never pauses. Stored in `connections_poll_task` (dropped on lock + /// cancels it) and epoch-fenced against a fast re-unlock. A first fetch fires immediately so a + /// site connected before this unlock shows up the moment the shell renders. + fn start_connections_poll(&mut self, cx: &mut Context) { + let epoch = self.auth_epoch; + self.kick_connections_refresh(cx); + self.connections_poll_task = Some(cx.spawn(async move |this, cx| loop { + cx.background_executor() + .timer(Duration::from_secs(CONNECTIONS_POLL_SECS)) + .await; + let keep = this.update(cx, |this, cx| { + // End the loop once this unlocked session is over (lock / re-unlock bumps the epoch). + if this.auth != AuthStep::Ready || this.auth_epoch != epoch { + return false; + } + this.kick_connections_refresh(cx); + true + }); + if !matches!(keep, Ok(true)) { + break; + } + })); + } + + /// Fetch the bridge's live dapp sessions off the UI thread and fold them into `self.connections` + /// (#199). In-flight-guarded so a slow bridge can't stack round-trips; epoch-fenced so a reply + /// landing after lock/re-unlock is dropped. Notifies only when the list actually changed (the + /// poll is ambient — no needless repaints). On ERROR it sets the list empty: bridge sessions + /// live in the bridge's RAM, so an unreachable bridge genuinely means no live sessions — the + /// honest answer, never a stale phantom row. + fn kick_connections_refresh(&mut self, cx: &mut Context) { + if self.auth != AuthStep::Ready || self.connections_inflight { + return; + } + self.connections_inflight = true; + let epoch = self.auth_epoch; + let cepoch = self.connections_epoch; + let task = cx.background_spawn(async move { bridge_admin::list_sites() }); + cx.spawn(async move |this, cx| { + let res = task.await; + this.update(cx, |this, cx| { + if this.auth_epoch != epoch { + // Lock/re-unlock already reset the in-flight guard — this reply belongs to a + // dead session; clearing the NEW session's guard here could stack fetches. + return; + } + this.connections_inflight = false; + // A disconnect fired while this list fetch was in flight: the snapshot predates it, + // so folding it in could resurrect the just-revoked row for one poll interval. Drop + // it — the disconnect's own reply (or the next tick) carries the current truth. The + // guard above IS cleared: same live session, and the next tick must be able to fetch. + if this.connections_epoch != cepoch { + return; + } + let next = res.unwrap_or_default(); + if this.connections != next { + this.connections = next; + cx.notify(); + } + }) + .ok(); + }) + .detach(); + } + + /// Disconnect one connected site (#199): a session-scoped revoke, NOT a persistent ban + /// (ADR 0006) — the site's next request fails `4100 Unauthorized` and it may reconnect fresh. + /// The bridge returns the remaining live list, which we fold in immediately (epoch-fenced) so + /// the row disappears without waiting for the next poll tick. A bridge error clears the list + /// (no bridge = no live sessions — the honest, fail-safe direction). `pub(crate)` so the + /// Connections sidebar row's ✕ (`shell_chrome`) can call it. + pub(crate) fn revoke_connected_site(&mut self, origin: String, cx: &mut Context) { + let epoch = self.auth_epoch; + // Bump the connections epoch so an in-flight list poll (a pre-revoke snapshot) is dropped + // instead of resurrecting this row; capture AFTER the bump so our own reply still folds. + self.connections_epoch = self.connections_epoch.wrapping_add(1); + let cepoch = self.connections_epoch; + let task = cx.background_spawn(async move { bridge_admin::revoke_site(&origin) }); + cx.spawn(async move |this, cx| { + let res = task.await; + this.update(cx, |this, cx| { + if this.auth_epoch != epoch || this.connections_epoch != cepoch { + return; + } + this.connections = res.unwrap_or_default(); + cx.notify(); + }) + .ok(); + }) + .detach(); + } + + /// Disconnect every connected site (#199): the ⌘K "revoke all connected sites" path. Session- + /// scoped like [`revoke_connected_site`]; folds the (now-empty) returned list in immediately. + fn disconnect_all_sites(&mut self, cx: &mut Context) { + let epoch = self.auth_epoch; + // Same stale-poll fence as `revoke_connected_site`: bump, then capture. + self.connections_epoch = self.connections_epoch.wrapping_add(1); + let cepoch = self.connections_epoch; + let task = cx.background_spawn(async move { bridge_admin::clear_sites() }); + cx.spawn(async move |this, cx| { + let res = task.await; + this.update(cx, |this, cx| { + if this.auth_epoch != epoch || this.connections_epoch != cepoch { + return; + } + this.connections = res.unwrap_or_default(); + cx.notify(); + }) + .ok(); + }) + .detach(); + } + /// The "waiting on you" count behind the everywhere-cues — the home waiting strip and the /// sidebar Activity badge (#200). Dispatch is the pure [`waiting_count`] (unit-tested): on the /// Activity surface the UNCAPPED inbox is the source (#216), so the badge always equals the @@ -3239,6 +3404,22 @@ impl Shell { self.open_activity(window, cx); self.stop_revoke_all(cx); } + // Disconnect connected dapp site(s) (#199). The action ALWAYS matches the live label + // `display_label` shows: 0 → nothing to do; exactly 1 → disconnect that one site; N>1 → + // disconnect all. Per-site precision at N>1 lives on the sidebar row's ✕; from the + // palette, revoking MORE than one when the label reads "all N" is the fail-safe direction + // (revoking too much only reduces authority). Session-scoped disconnect, not a ban. + "revoke-site" => { + // Snapshot origins into an owned Vec first so the match doesn't hold an immutable + // borrow of `self.connections` across the &mut-self revoke calls below. + let origins: Vec = + self.connections.iter().map(|s| s.origin.clone()).collect(); + match origins.as_slice() { + [] => {} + [only] => self.revoke_connected_site(only.clone(), cx), + _ => self.disconnect_all_sites(cx), + } + } "settings" => self.open(Surface::Settings, cx), // Rename the wallet / agent (E2, #182): open Settings and focus the field so the ⌘K // command lands the operator directly on the input it named. Drop the captured diff --git a/crates/deckard-app/src/shell_chrome.rs b/crates/deckard-app/src/shell_chrome.rs index 7f551cf..4552123 100644 --- a/crates/deckard-app/src/shell_chrome.rs +++ b/crates/deckard-app/src/shell_chrome.rs @@ -11,8 +11,8 @@ use gpui::prelude::FluentBuilder; use gpui::{ - div, px, Context, InteractiveElement, IntoElement, ParentElement, StatefulInteractiveElement, - Styled, + div, px, Context, InteractiveElement, IntoElement, ParentElement, SharedString, + StatefulInteractiveElement, Styled, }; use gpui_component::{ button::{Button, ButtonVariants}, @@ -184,19 +184,16 @@ impl Shell { ) .on_click(cx.listener(|this, _, _, cx| this.select(Selection::Agent, cx))), ) - // Connections group — dapp origins (DESIGN §IA). A reserved, list-only slot: the - // request-origin model exists, but deep connection management is deferred (ADR-0001 / - // #44), so the group announces itself with a quiet empty state until a bridged dapp - // origin appears. Non-interactive on purpose — nothing to select here yet. + // Connections group — dapp origins (DESIGN §IA). One row per live browser-bridge + // session: a neutral Globe + the origin host + a ghost ✕ that disconnects it (#199). + // Now interactive (the old "nothing to select" note is stale). Revoke is a session- + // scoped disconnect, not a ban (ADR 0006) — a disconnected site can reconnect. DESIGN + // names "favicon + domain + a trust dot" here, but favicons and the trust model are + // #48's; we render the NEUTRAL Globe and NO trust dot rather than fabricate a trust + // signal we don't have (DESIGN §the request-origin model). A quiet empty state until a + // dapp connects. .child(group_label("Connections")) - .child( - div().mx_2().px_2().py_1p5().child( - div() - .text_sm() - .text_color(muted) - .child("No connected sites yet"), - ), - ) + .child(self.render_connections(fg, muted, cx)) // Spacer pushes the footer rows to the bottom. .child(div().flex_1()) // Activity ledger — a sibling of Settings (bottom): the full cross-agent record AND @@ -254,6 +251,80 @@ impl Shell { ) } + /// The Connections group body (#199): the quiet empty-state line when no dapp is connected, + /// else one [`Shell::connection_row`] per live browser-bridge session. Split out of + /// `render_sidebar` so the per-row loop (each row carries its own revoke listener) reads cleanly. + fn render_connections( + &self, + fg: gpui::Hsla, + muted: gpui::Hsla, + cx: &mut Context, + ) -> impl IntoElement { + if self.connections.is_empty() { + return div().mx_2().px_2().py_1p5().child( + div() + .text_sm() + .text_color(muted) + .child("No connected sites yet"), + ); + } + let mut list = v_flex().w_full(); + for site in self.connections.iter() { + list = list.child(self.connection_row(site, fg, muted, cx)); + } + list + } + + /// One Connections row (#199): a neutral Globe, the origin host (truncating), and a right- + /// aligned ghost ✕ that disconnects THIS origin (session-scoped, not a ban — ADR 0006). Matches + /// the sibling nav rows' geometry (`mx_2 px_2 py_1p5 rounded_md`). NO trust dot — the trust + /// model is #48's, and we never fabricate a trust signal (DESIGN §the request-origin model). + /// Dynamic ids off the origin so each row + button keeps a stable identity across poll churn. + fn connection_row( + &self, + site: &crate::bridge_admin::ConnectedSite, + fg: gpui::Hsla, + muted: gpui::Hsla, + cx: &mut Context, + ) -> impl IntoElement { + let origin = site.origin.clone(); + let host = crate::bridge_admin::origin_host(&origin).to_string(); + let row_id = SharedString::from(format!("conn-{origin}")); + let btn_id = SharedString::from(format!("conn-x-{origin}")); + div().id(row_id).mx_2().px_2().py_1p5().rounded_md().child( + h_flex() + .items_center() + .justify_between() + .gap_2() + .child( + // Left cluster clamps (`min_w_0` + truncate) so a long origin never runs + // off the sidebar edge (DESIGN §cockpit row anatomy). + h_flex() + .items_center() + .gap_2() + .min_w_0() + .child(Icon::new(IconName::Globe).text_color(muted).small()) + .child( + div() + .min_w_0() + .truncate() + .text_sm() + .text_color(fg) + .child(host), + ), + ) + .child( + Button::new(btn_id) + .ghost() + .icon(IconName::CircleX) + .small() + .on_click(cx.listener(move |this, _, _, cx| { + this.revoke_connected_site(origin.clone(), cx); + })), + ), + ) + } + /// The 44px breadcrumb bar: `[mark] ` on the left — the entity the current view is /// about (the wallet `Meridian` or the agent `Kyoto`), plus `› ` on an action surface /// (`Meridian › Send`). Identity is named (E2, #182): no `Personal ›` prefix, no literal Wallet. diff --git a/crates/deckard-app/src/shell_rail.rs b/crates/deckard-app/src/shell_rail.rs index ab050fa..f9666b2 100644 --- a/crates/deckard-app/src/shell_rail.rs +++ b/crates/deckard-app/src/shell_rail.rs @@ -132,10 +132,15 @@ impl Shell { } // The golden ref's trailing composition metasec — ALWAYS present (independent of whether the - // policy has landed). Connections: the reserved/empty state — the browser bridge (dapp - // connections) is deferred (ADR-0001 / #44), so we render the honest empty "none", NEVER a - // fabricated count. Agents: the app models one agent on this wallet; its state follows the - // live policy (idle until it lands, active unless a STOP revoked it). + // policy has landed). Connections: the LIVE browser-bridge session count (#199) — the same + // `self.connections` the sidebar renders, so the rail and the Connections group can never + // disagree; "none" is the honest empty state, never a fabricated count. Agents: the app + // models one agent on this wallet; its state follows the live policy (idle until it lands, + // active unless a STOP revoked it). + let connections = match self.connections.len() { + 0 => "none".to_string(), + n => format!("{n} connected"), + }; let agents = match self.agent_policy.as_ref() { Some(p) if p.revoked => "1 stopped", Some(_) => "1 active", @@ -144,7 +149,7 @@ impl Shell { let summary = v_flex() .w_full() .gap_2() - .child(kv("Connections", KvValue::Sans("none"))) + .child(kv("Connections", KvValue::Sans(&connections))) .child(kv("Agents", KvValue::Sans(agents))) .into_any_element(); col = col.child(meta_section(None, summary, theme)); diff --git a/crates/deckard-browser-bridge/src/lib.rs b/crates/deckard-browser-bridge/src/lib.rs index 881bb22..c9ba9db 100644 --- a/crates/deckard-browser-bridge/src/lib.rs +++ b/crates/deckard-browser-bridge/src/lib.rs @@ -88,7 +88,6 @@ impl DappSessionStore { session.clone() } - #[allow(dead_code)] pub fn revoke(&mut self, origin: &str) -> bool { match self.sessions.get_mut(origin) { Some(session) => { @@ -99,6 +98,33 @@ impl DappSessionStore { None => false, } } + + /// The live (non-revoked) sessions in stable origin order (`BTreeMap` iterates its keys + /// sorted). Feeds the app's Connections list over `/admin` (#199) — the app renders exactly + /// what the bridge still honors, so a revoked session never lingers on screen. + pub fn list(&self) -> Vec { + self.sessions + .values() + .filter(|session| !session.revoked) + .cloned() + .collect() + } + + /// Tombstone every live session and report how many were live. The authority-reducing side of + /// the app's Lock / Lockdown (#199): each site must reconnect fresh afterward (disconnect, not + /// ban — ADR 0006). Idempotent — an already-revoked session isn't re-counted. + pub fn revoke_all(&mut self) -> usize { + let now = unix_now(); + let mut revoked = 0; + for session in self.sessions.values_mut() { + if !session.revoked { + session.revoked = true; + session.last_seen = now; + revoked += 1; + } + } + revoked + } } #[derive(Clone)] @@ -373,6 +399,39 @@ impl BrowserBridge { message: "Deckard requires eth_requestAccounts before signing".into(), }) } + + /// Handle one app→bridge `/admin` request (#199). `list` returns the live sessions; `revoke` + /// disconnects one origin; `clear` disconnects all. Every op replies with the SAME shape — the + /// post-op live list — so the app refreshes its whole Connections view in one round-trip. + /// Revoke is a session-scoped DISCONNECT, not a ban (ADR 0006): a revoked site's next signing + /// request fails `4100` and it may reconnect fresh. Revoking an unknown/already-revoked origin + /// is a silent no-op — disconnect is idempotent and fail-safe. A `revoke` missing `origin`, or + /// an unknown `op`, is a client error (`Err`) the HTTP layer answers with `400`. + pub fn handle_admin(&self, request: AdminRequest) -> Result { + let mut store = self + .sessions + .lock() + .map_err(|_| "Deckard browser bridge session store is unavailable".to_string())?; + match request.op.as_str() { + "list" => {} + "revoke" => { + let origin = request + .origin + .as_deref() + .ok_or_else(|| "admin revoke requires an origin".to_string())?; + // Idempotent no-op if the origin has no live session — it simply won't appear in + // the returned list. Never an error: disconnect is fail-safe. + store.revoke(origin); + } + "clear" => { + store.revoke_all(); + } + other => return Err(format!("unknown admin op {other}")), + } + Ok(AdminResponse { + sessions: store.list(), + }) + } } #[derive(Clone, Debug, Deserialize)] @@ -419,6 +478,23 @@ pub struct BridgeError { pub message: String, } +/// An app→bridge admin request over `/admin` (#199). The local Deckard app is the only caller — a +/// web page cannot reach `/admin` (see [`admin_authorized`]). `op` is `list` | `revoke` | `clear`; +/// `origin` names the site to disconnect for a `revoke` (ignored by `list`/`clear`). +#[derive(Clone, Debug, Deserialize)] +pub struct AdminRequest { + pub op: String, + #[serde(default)] + pub origin: Option, +} + +/// The uniform `/admin` reply: the live session list AFTER the op ran, so the app refreshes its +/// Connections view in a single round-trip regardless of which op it sent. +#[derive(Clone, Debug, Serialize)] +pub struct AdminResponse { + pub sessions: Vec, +} + async fn sign_message_with_daemon( wallet: &WalletClient, message: SignMessage, @@ -1189,12 +1265,30 @@ async fn handle_http_connection( if method == "OPTIONS" { return write_http(&mut stream, 204, "text/plain", "").await; } - if method != "POST" || path != "/rpc" { + // Two POST-only, loopback-only endpoints share this connection handler: `/rpc` is the EIP-1193 + // dapp path; `/admin` is the local Deckard app's session-management path (#199 — list + revoke + // dapp sessions). The Host check and the body read below are shared by both. + let is_rpc = method == "POST" && path == "/rpc"; + let is_admin = method == "POST" && path == "/admin"; + if !is_rpc && !is_admin { return write_http(&mut stream, 404, "text/plain", "not found").await; } if !host_is_loopback(&headers) { return write_http(&mut stream, 403, "text/plain", "host must be localhost").await; } + // `/admin` additionally requires the `x-deckard-admin` marker header (see `admin_authorized`): + // a web page cannot set it — the CORS preflight does not allow it — so only a local process + // (the Deckard app) reaches admin. This is the ADR-0006 "minimal loopback check" posture; we + // deliberately do NOT add token auth on a wire we intend to retire. + if is_admin && !admin_authorized(&headers) { + return write_http( + &mut stream, + 403, + "text/plain", + "admin requires x-deckard-admin", + ) + .await; + } let content_length = header_value(&headers, "content-length") .and_then(|s| s.parse::().ok()) @@ -1211,14 +1305,37 @@ async fn handle_http_connection( } read += n; } + let body = &buf[body_start..body_start + content_length]; + + if is_admin { + // The app owns both ends of `/admin`, so a bad request is a client bug, not hostile input: + // a valid op maps to its 200 JSON reply (the post-op live list); a client error (missing + // origin / unknown op) maps to 400. A body that won't even parse propagates like `/rpc`'s. + let request: AdminRequest = serde_json::from_slice(body)?; + return match bridge.handle_admin(request) { + Ok(response) => { + let response_body = serde_json::to_string(&response)?; + write_http(&mut stream, 200, "application/json", &response_body).await + } + Err(message) => write_http(&mut stream, 400, "text/plain", &message).await, + }; + } - let request: BridgeRequest = - serde_json::from_slice(&buf[body_start..body_start + content_length])?; + let request: BridgeRequest = serde_json::from_slice(body)?; let response = bridge.handle_request(&origin, request).await; let response_body = serde_json::to_string(&response)?; write_http(&mut stream, 200, "application/json", &response_body).await } +/// The exact `access-control-allow-headers` value the bridge advertises. Broken out as a const so a +/// unit test can pin the load-bearing invariant (#199): `x-deckard-admin` MUST NEVER appear here. +/// That custom header gates `/admin` (see [`admin_authorized`]); because a browser must clear a +/// CORS preflight before it may send ANY custom request header, and this allow-list omits +/// `x-deckard-admin`, no web page can call `/admin` — only a local, preflight-free process (the +/// Deckard app) can. Adding it here would hand a malicious page the power to enumerate connected +/// origins or disconnect sessions. Do not add it; do not add token auth either (ADR 0006). +const CORS_ALLOW_HEADERS: &str = "content-type,x-deckard-origin"; + async fn write_http( stream: &mut TcpStream, status: u16, @@ -1228,13 +1345,14 @@ async fn write_http( let reason = match status { 200 => "OK", 204 => "No Content", + 400 => "Bad Request", 403 => "Forbidden", 404 => "Not Found", 413 => "Payload Too Large", _ => "Error", }; let response = format!( - "HTTP/1.1 {status} {reason}\r\ncontent-type: {content_type}\r\ncontent-length: {}\r\naccess-control-allow-origin: *\r\naccess-control-allow-headers: content-type,x-deckard-origin\r\naccess-control-allow-methods: POST,OPTIONS\r\nconnection: close\r\n\r\n{body}", + "HTTP/1.1 {status} {reason}\r\ncontent-type: {content_type}\r\ncontent-length: {}\r\naccess-control-allow-origin: *\r\naccess-control-allow-headers: {CORS_ALLOW_HEADERS}\r\naccess-control-allow-methods: POST,OPTIONS\r\nconnection: close\r\n\r\n{body}", body.len() ); stream.write_all(response.as_bytes()).await?; @@ -1311,6 +1429,16 @@ fn host_is_loopback(headers: &str) -> bool { .unwrap_or(false) } +/// Whether an `/admin` request carries the loopback admin marker header (#199). The gate is +/// deliberately minimal (ADR 0006 accepts the localhost wire as-is): the header's PRESENCE — any +/// value — is the whole check. Its security rests entirely on the CORS invariant documented on +/// [`CORS_ALLOW_HEADERS`]: a web page cannot set `x-deckard-admin` (the preflight will not allow +/// it), so only a local process can reach `/admin`. This is header-shape hygiene, NOT authentication +/// — extracted as a pure function so it is unit-testable without a TCP round-trip. +fn admin_authorized(headers: &str) -> bool { + header_value(headers, "x-deckard-admin").is_some() +} + #[cfg(test)] mod tests { use super::*; @@ -1971,4 +2099,175 @@ mod tests { .await; assert_eq!(response.error.expect("wrong-account error").code, 4100); } + + /// #199 disconnect-not-ban (ADR 0006): revoking a session via the admin dispatch drops it from + /// the live list, its next `eth_sendTransaction` fails 4100 and `eth_accounts` goes empty — but + /// the site can `eth_requestAccounts` again for a FRESH session. Revoke is a session-scoped + /// disconnect, never a persistent block. + #[tokio::test] + async fn admin_revoke_disconnects_but_allows_reconnect() { + let bridge = bridge(); + let _ = bridge + .handle_request( + ORIGIN, + BridgeRequest { + id: json!(1), + method: "eth_requestAccounts".into(), + params: Value::Null, + }, + ) + .await; + assert_eq!( + bridge.accounts_for_origin(ORIGIN), + vec![DEFAULT_DEV_ACCOUNT] + ); + + // Revoke via the admin dispatch — the returned list no longer carries this origin. + let after = bridge + .handle_admin(AdminRequest { + op: "revoke".into(), + origin: Some(ORIGIN.to_string()), + }) + .expect("revoke ok"); + assert!( + after.sessions.is_empty(), + "a revoked session must drop out of the live list" + ); + + // A revoked origin is unauthorized again: the next signing request fails 4100 … + let send = bridge + .handle_request( + ORIGIN, + BridgeRequest { + id: json!(2), + method: "eth_sendTransaction".into(), + params: json!([{ "from": DEFAULT_DEV_ACCOUNT, "to": "0x0000000000000000000000000000000000000001", "value": "0x1" }]), + }, + ) + .await; + assert_eq!(send.error.expect("unauthorized after revoke").code, 4100); + // … and `eth_accounts` discloses nothing. + assert!(bridge.accounts_for_origin(ORIGIN).is_empty()); + + // Disconnect, not ban: the site may reconnect for a fresh session. + let reconnect = bridge + .handle_request( + ORIGIN, + BridgeRequest { + id: json!(3), + method: "eth_requestAccounts".into(), + params: Value::Null, + }, + ) + .await; + assert_eq!(reconnect.result, Some(json!([DEFAULT_DEV_ACCOUNT]))); + assert_eq!( + bridge.accounts_for_origin(ORIGIN), + vec![DEFAULT_DEV_ACCOUNT] + ); + } + + /// #199 admin dispatch semantics: `list` excludes revoked and keeps stable origin order, + /// `revoke` of an unknown origin is a no-op returning the unchanged list, revoking a live origin + /// removes it, `clear` empties everything, and a `revoke` without `origin` (or an unknown op) is + /// a client error. + #[tokio::test] + async fn admin_dispatch_list_revoke_and_clear() { + let bridge = bridge(); + for origin in ["https://a.example", "https://b.example"] { + let _ = bridge + .handle_request( + origin, + BridgeRequest { + id: json!(1), + method: "eth_requestAccounts".into(), + params: Value::Null, + }, + ) + .await; + } + + // list: both, in stable (BTreeMap) origin order. + let listed = bridge + .handle_admin(AdminRequest { + op: "list".into(), + origin: None, + }) + .expect("list ok"); + let origins: Vec<&str> = listed.sessions.iter().map(|s| s.origin.as_str()).collect(); + assert_eq!(origins, vec!["https://a.example", "https://b.example"]); + + // revoke of an unknown origin is a no-op that returns the unchanged list. + let after_unknown = bridge + .handle_admin(AdminRequest { + op: "revoke".into(), + origin: Some("https://not-connected.example".into()), + }) + .expect("revoke unknown ok"); + assert_eq!(after_unknown.sessions.len(), 2); + + // revoke one → list excludes it. + let after_one = bridge + .handle_admin(AdminRequest { + op: "revoke".into(), + origin: Some("https://a.example".into()), + }) + .expect("revoke ok"); + let origins: Vec<&str> = after_one + .sessions + .iter() + .map(|s| s.origin.as_str()) + .collect(); + assert_eq!(origins, vec!["https://b.example"]); + + // clear empties everything. + let after_clear = bridge + .handle_admin(AdminRequest { + op: "clear".into(), + origin: None, + }) + .expect("clear ok"); + assert!(after_clear.sessions.is_empty()); + + // a revoke without an origin, or an unknown op, is a client error. + assert!(bridge + .handle_admin(AdminRequest { + op: "revoke".into(), + origin: None, + }) + .is_err()); + assert!(bridge + .handle_admin(AdminRequest { + op: "explode".into(), + origin: None, + }) + .is_err()); + } + + /// #199 admin gate: the `x-deckard-admin` marker header must be PRESENT (any value); its absence + /// forbids the request. Presence is the whole check — its safety comes from the CORS invariant + /// below, not from the value. + #[test] + fn admin_authorized_requires_the_marker_header() { + assert!(!admin_authorized( + "POST /admin HTTP/1.1\r\nhost: 127.0.0.1:8765\r\ncontent-type: application/json" + )); + assert!(admin_authorized( + "POST /admin HTTP/1.1\r\nhost: 127.0.0.1:8765\r\nx-deckard-admin: 1" + )); + // Any value counts — the header's presence is the check, not its contents. + assert!(admin_authorized("x-deckard-admin: anything-here")); + } + + /// #199 load-bearing CORS invariant: the admin gate header must NEVER be in the allow-list, or a + /// web page could clear a preflight and call `/admin` to enumerate or disconnect dapp sessions. + /// Enforced at the const level so a careless edit to the allow-list fails this test. + #[test] + fn cors_allow_headers_never_exposes_the_admin_gate() { + assert!( + !CORS_ALLOW_HEADERS.contains("x-deckard-admin"), + "the /admin gate header must never be CORS-allowed" + ); + assert!(CORS_ALLOW_HEADERS.contains("content-type")); + } } diff --git a/docs/browser-bridge.md b/docs/browser-bridge.md index 056bc99..e278810 100644 --- a/docs/browser-bridge.md +++ b/docs/browser-bridge.md @@ -108,6 +108,38 @@ The bridge stores sessions in memory only, keyed by origin: This is deliberately minimal. Restarting the bridge clears sessions. Future work should move this into a reviewed permissions registry with explicit approval UI, anti-phishing copy, revocation UX, and persistence. +## Connections in the app, and per-site revoke (#199) + +The Deckard desktop app lists the bridge's live sessions under **Connections** in the sidebar and lets +you disconnect a site with the ✕ on its row (or the ⌘K command "Revoke connected site"). This talks to +a second endpoint on the same loopback HTTP server: + +- `POST /admin` with a JSON body: `{"op":"list"}`, `{"op":"revoke","origin":"https://…"}`, or + `{"op":"clear"}`. **Every op replies with the same shape** — the live session list *after* the op ran + (`{"sessions":[…]}`) — so the app refreshes its whole view in one round-trip. +- **Revoke is a session-scoped disconnect, not a ban** (ADR 0006 / #202). A revoked site's next request + fails `4100 Unauthorized`; it may reconnect with `eth_requestAccounts` for a fresh session. There is no + blocklist and nothing persisted — sessions live only in the bridge's RAM, so a bridge restart clears + them all too. The richer per-origin permission model (and any persistence) is #48's territory. +- **`clear` is the authority-reducing side of the app's Lock / Lockdown**: locking the wallet, or the + Activity STOP brake, best-effort-disconnects every live dapp session too. + +### Why a web page can't call `/admin` + +`/admin` requires the request header `x-deckard-admin` (any value) — else `403`. That single check is +load-bearing, and it works *because* `x-deckard-admin` is a **custom** header that is deliberately **never** +listed in the bridge's `access-control-allow-headers`. A browser must win a CORS preflight before it may +send any custom request header, and the preflight does not allow `x-deckard-admin`, so the browser blocks +a malicious page's request before it is ever sent. A local process (the Deckard app) makes a plain, +preflight-free socket request and sets the header freely. Net: only local processes can enumerate connected +origins or disconnect sessions. This is the same minimal-loopback posture ADR 0006 accepts for the alpha — +there is no token auth, on purpose (it is a wire we intend to retire). + +The app finds the bridge at `127.0.0.1:8765` by default; set `DECKARD_BRIDGE_ADDR` to override (it must +stay on loopback — the bridge rejects a non-loopback Host). If no bridge is running, the connect fails +instantly and Connections simply shows nothing — an unreachable bridge holds no sessions, so an empty list +is the honest answer. + ## Run in dev/mock mode This is the smallest way to test the browser bridge without an unlocked wallet: From 1cc6eb1a54ffe03a74d777271494f82915489c6b Mon Sep 17 00:00:00 2001 From: hellno Date: Mon, 13 Jul 2026 12:01:45 +0200 Subject: [PATCH 2/4] =?UTF-8?q?fix(connections):=20#199=20=C2=B7=20sequenc?= =?UTF-8?q?e=20daemon=20lock/revoke=20BEFORE=20the=20bridge=20session=20cl?= =?UTF-8?q?ear?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex adversarial review (gpt-5.5 xhigh) finding: Lock and Lockdown fired the daemon key-zeroize and the bridge clear_sites as two unordered detached tasks. If the bridge clear won the race, a dapp reconnecting via eth_requestAccounts in the window before the daemon locked would be granted a fresh session that survives the lock. Sequenced in one background task — daemon authority down first, then the bridge disconnect — that reconnect now hits a locked daemon and grants nothing. Also documents the accepted low-severity edge codex flagged: two OVERLAPPING per-site revokes whose loopback replies reorder can leave the slower row on screen until the next 5s poll tick reconciles (every revoke IS applied on the bridge; only the interim display can lag). --- crates/deckard-app/src/shell.rs | 43 ++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/crates/deckard-app/src/shell.rs b/crates/deckard-app/src/shell.rs index 6fe3605..0acecd0 100644 --- a/crates/deckard-app/src/shell.rs +++ b/crates/deckard-app/src/shell.rs @@ -985,7 +985,13 @@ impl Shell { pub fn lock(&mut self, cx: &mut Context) { let client = self.signer.client(); cx.background_spawn(async move { + // ORDER MATTERS (#199, codex finding): zeroize the daemon key FIRST, then disconnect + // the bridge sessions. Clearing first would open a race — a dapp reconnecting via + // `eth_requestAccounts` in the window before the daemon locks would be granted a fresh + // session that survives the lock. Locked-first, that reconnect fails at the daemon + // (no unlocked account to read) and grants nothing. let _ = client.lock_blocking(); + let _ = bridge_admin::clear_sites(); }) .detach(); self.wallet_address = None; @@ -1002,17 +1008,14 @@ impl Shell { self.polled_pending = None; self.pending_poll_inflight = false; // Dapp sessions are authority too, and a Lock reduces ALL of it (#199): cancel the - // Connections poll, drop the local list, and best-effort-disconnect every live bridge - // session (detached, result ignored — the bridge is a separate process that may not even be - // running). Session-scoped disconnect, not a ban (ADR 0006). + // Connections poll and drop the local list here; the bridge-side disconnect runs in the + // task above, sequenced AFTER the daemon lock (best-effort, result ignored — the bridge is + // a separate process that may not even be running). Session-scoped disconnect, not a ban + // (ADR 0006). self.connections_poll_task = None; self.connections_inflight = false; self.connections.clear(); self.connections_epoch = self.connections_epoch.wrapping_add(1); - cx.background_spawn(async move { - let _ = bridge_admin::clear_sites(); - }) - .detach(); // Supersede any in-flight `refresh_activity` reply: without this, a fetch started just // before the lock could land afterwards and re-install a dead session's snapshot into // `self.activity` / `self.inbox` (#216) — and off-surface `waiting_count` falls back to the @@ -2919,19 +2922,22 @@ impl Shell { /// kill is visible — #60 acceptance 3. pub fn stop_revoke_all(&mut self, cx: &mut Context) { self.activity_stop_arming = false; - // Lockdown reduces ALL authority, dapp sessions included (#199): best-effort-disconnect - // every live bridge session and clear the local list. Detached + ignored — the bridge is a - // separate process that may not be running, and the key-zeroize below is the real brake. + // Lockdown reduces ALL authority, dapp sessions included (#199): clear the local list + // optimistically; the bridge-side disconnect runs in the task below, sequenced AFTER the + // daemon revoke (best-effort, ignored — the bridge may not be running, and the key-zeroize + // is the real brake). Clearing the bridge FIRST would open a race: a dapp reconnecting in + // the window before the daemon revokes would be granted a fresh session that survives the + // Lockdown (codex finding); revoked-first, that reconnect gets nothing from the daemon. self.connections.clear(); self.connections_epoch = self.connections_epoch.wrapping_add(1); - cx.background_spawn(async move { - let _ = bridge_admin::clear_sites(); - }) - .detach(); let client = self.signer.client(); self.activity_loading = true; cx.notify(); - let task = cx.background_spawn(async move { client.revoke_all_blocking() }); + let task = cx.background_spawn(async move { + let res = client.revoke_all_blocking(); + let _ = bridge_admin::clear_sites(); + res + }); cx.spawn(async move |this, cx| { let res = task.await; this.update(cx, |this, cx| match res { @@ -3190,8 +3196,11 @@ impl Shell { /// (ADR 0006) — the site's next request fails `4100 Unauthorized` and it may reconnect fresh. /// The bridge returns the remaining live list, which we fold in immediately (epoch-fenced) so /// the row disappears without waiting for the next poll tick. A bridge error clears the list - /// (no bridge = no live sessions — the honest, fail-safe direction). `pub(crate)` so the - /// Connections sidebar row's ✕ (`shell_chrome`) can call it. + /// (no bridge = no live sessions — the honest, fail-safe direction). Known, accepted edge: two + /// OVERLAPPING revokes whose loopback replies reorder can leave the slower site's row on + /// screen until the next poll tick reconciles (≤5s) — every revoke IS applied on the bridge; + /// only the interim display can lag. `pub(crate)` so the Connections sidebar row's ✕ + /// (`shell_chrome`) can call it. pub(crate) fn revoke_connected_site(&mut self, origin: String, cx: &mut Context) { let epoch = self.auth_epoch; // Bump the connections epoch so an in-flight list poll (a pre-revoke snapshot) is dropped From 9d27ef694ca213e0bc9ac7be48d001ef0229c982 Mon Sep 17 00:00:00 2001 From: hellno Date: Mon, 13 Jul 2026 12:01:45 +0200 Subject: [PATCH 3/4] =?UTF-8?q?chore(ci):=20pin=20cargo-deny-action=20to?= =?UTF-8?q?=20v2.0.20=20=E2=80=94=20v2.1.0=20breaks=20every=20invocation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cargo-deny-action v2.1.0 (released 2026-07-13 09:32 UTC, between main's last green run and this PR's first run) removed the use-git-cli positional from action.yml without updating the entrypoint's positional indexing, so the log-level value lands in a command slot and every run dies with `unrecognized subcommand 'warn'` — on both the advisories and supply-chain jobs, and it would take the nightly audit down too. Pin all three uses to the last-good v2.0.20; unpin to @v2 once upstream ships a fix. --- .github/workflows/audit.yml | 5 ++++- .github/workflows/ci.yml | 10 ++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index d87b139..989279c 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -30,7 +30,10 @@ jobs: steps: - uses: actions/checkout@v4 - id: deny - uses: EmbarkStudios/cargo-deny-action@v2 + # Pinned to the last-good release: v2.1.0 (2026-07-13) drops a positional arg in action.yml + # without updating its entrypoint, shifting --log-level into a command slot — every run dies + # with `unrecognized subcommand 'warn'`. Unpin to @v2 once upstream ships a fix. + uses: EmbarkStudios/cargo-deny-action@v2.0.20 with: command: check advisories # Make the daily watch LOUD (#82): if the ADVISORY SCAN itself failed (the `steps.deny.outcome` diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 29391b7..bf35994 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -131,7 +131,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: EmbarkStudios/cargo-deny-action@v2 + # Pinned to the last-good release: v2.1.0 (2026-07-13) drops a positional arg in action.yml + # without updating its entrypoint, shifting --log-level into a command slot — every run dies + # with `unrecognized subcommand 'warn'`. Unpin to @v2 once upstream ships a fix. + - uses: EmbarkStudios/cargo-deny-action@v2.0.20 with: command: check advisories # PR-only yank downgrade (warn, not block). Empty elsewhere → deny.toml's `yanked = "deny"` @@ -148,6 +151,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: EmbarkStudios/cargo-deny-action@v2 + # Pinned to the last-good release: v2.1.0 (2026-07-13) drops a positional arg in action.yml + # without updating its entrypoint, shifting --log-level into a command slot — every run dies + # with `unrecognized subcommand 'warn'`. Unpin to @v2 once upstream ships a fix. + - uses: EmbarkStudios/cargo-deny-action@v2.0.20 with: command: check bans sources licenses From 4efd1d082ba6bdc3a270f6dd145916d3ba1d922c Mon Sep 17 00:00:00 2001 From: hellno Date: Mon, 13 Jul 2026 15:54:04 +0200 Subject: [PATCH 4/4] =?UTF-8?q?fix(connections):=20#199=20=C2=B7=20/code-r?= =?UTF-8?q?eview=20hardening=20=E2=80=94=20poll=20lifecycle,=20honest=20er?= =?UTF-8?q?ror=20semantics,=20loopback=20guard,=20shared=20revoke=20plan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Findings from the 10-angle xhigh review (3 angles independently confirmed the first one): - Lockdown (stop_revoke_all) now cancels the connections poll exactly like lock() does: with the bridge clear sequenced AFTER the daemon revoke, a still-running tick could refetch the not-yet-cleared sessions, pass the post-bump epoch fence, and flash "N connected" onto a locked-down wallet. - Honest error semantics: only ECONNREFUSED proves no bridge (→ empty list); every other failure — connect timeout, read timeout, non-200 — keeps the current list instead of falsely blanking live sessions. A failed single-site revoke can no longer wipe sibling rows or read as "all disconnected". - resolve_loopback_addr refuses a non-loopback DECKARD_BRIDGE_ADDR: the x-deckard-admin marker and disconnect ops must never leave this machine, and a remote endpoint must never feed the Connections trust surface (+ tests). - apply_disconnect: one shared helper for per-site revoke and revoke-all — the epoch fence, the NEW in-flight guard (a poll can no longer start under the post-bump epoch and land a pre-revoke snapshot), and the error handling can no longer drift between the two paths. - RevokeTarget: the ⌘K revoke-site label and its Enter action now read ONE computed target (Nothing/One/All) instead of hand-maintaining the same 0/1/N partition in two files — the label structurally cannot promise a different disconnect than Enter performs. Accepted, documented: DESIGN's favicon+trust-dot stays #48's (never fabricate a trust signal); admin DTO sharing via deckard-contract deferred (that crate is the frozen daemon wire, this channel is the alpha's swappable one); the per-op session-list clone (tiny by design). --- crates/deckard-app/src/bridge_admin.rs | 62 ++++++++-- crates/deckard-app/src/palette.rs | 16 +-- crates/deckard-app/src/shell.rs | 154 ++++++++++++++++--------- 3 files changed, 161 insertions(+), 71 deletions(-) diff --git a/crates/deckard-app/src/bridge_admin.rs b/crates/deckard-app/src/bridge_admin.rs index 4cf244a..55a2d3c 100644 --- a/crates/deckard-app/src/bridge_admin.rs +++ b/crates/deckard-app/src/bridge_admin.rs @@ -10,9 +10,13 @@ //! listener; a bridge that isn't running fails instantly with `ECONNREFUSED`. //! //! Revoke is a session-scoped DISCONNECT, not a ban (ADR 0006): a revoked site's next request -//! fails `4100 Unauthorized`, and it may reconnect for a fresh session. Sessions live only in the -//! bridge's RAM, so an unreachable bridge genuinely means no live sessions — an error maps to an -//! empty list, which is the honest answer, never a stale phantom row. +//! fails `4100 Unauthorized`, and it may reconnect for a fresh session. +//! +//! **Error semantics.** Sessions live only in the bridge's RAM, so a bridge that isn't listening +//! genuinely means no live sessions: a failed CONNECT resolves to an honest empty list. But a +//! reachable-but-unhealthy bridge (a read/write timeout, a partial response, a non-200) resolves to +//! `Err` — NOT an empty list — so the caller keeps its current view rather than falsely blanking +//! live sessions on a momentary hiccup. A transient error is not proof a dapp disconnected. use std::io::{Read, Write}; use std::net::{TcpStream, ToSocketAddrs}; @@ -52,15 +56,39 @@ fn bridge_addr() -> String { /// background-only. Writes a minimal HTTP/1.1 request with the `x-deckard-admin` marker header the /// bridge gates on (a web page can't set it — the CORS preflight forbids it), then reads to EOF /// (the bridge answers `connection: close`). -fn admin_request(body: &str) -> anyhow::Result> { - let addr = bridge_addr(); - // Resolve to a concrete `SocketAddr` so `connect_timeout` can bound the connect. The default is - // a literal IP:port (no DNS); a hostname override may resolve here, which is the caller's choice. +/// Resolve the bridge address to a concrete LOOPBACK socket, or an error. Refuses a non-loopback +/// target: the bridge only ever binds loopback (`serve` bails otherwise), so a non-loopback +/// `DECKARD_BRIDGE_ADDR` is a misconfiguration — we must never write the `x-deckard-admin` marker + +/// a disconnect command to a remote host, nor trust its session list. Mirrors the server's own +/// loopback guard; the marker header must not leave this machine. Pure w.r.t. env (takes the addr +/// string) so it's unit-testable without a socket. +fn resolve_loopback_addr(addr: &str) -> anyhow::Result { let socket_addr = addr .to_socket_addrs()? .next() .ok_or_else(|| anyhow::anyhow!("bridge address {addr} did not resolve"))?; - let mut stream = TcpStream::connect_timeout(&socket_addr, CONNECT_TIMEOUT)?; + if !socket_addr.ip().is_loopback() { + anyhow::bail!("refusing admin request to non-loopback bridge address {addr}"); + } + Ok(socket_addr) +} + +fn admin_request(body: &str) -> anyhow::Result> { + let addr = bridge_addr(); + let socket_addr = resolve_loopback_addr(&addr)?; + let mut stream = match TcpStream::connect_timeout(&socket_addr, CONNECT_TIMEOUT) { + Ok(stream) => stream, + // REFUSED is the one connect failure that proves nothing is listening: there is no bridge, + // so there are no live sessions — they live in the bridge's RAM. Report an honest empty + // list. Every OTHER failure (a connect timeout on a momentarily-stalled accept loop, an + // unreachable error) is NOT proof the bridge is gone, so it propagates as `Err` — same as + // a post-connect failure — and the caller keeps its current list instead of falsely + // blanking still-live sessions. + Err(error) if error.kind() == std::io::ErrorKind::ConnectionRefused => { + return Ok(Vec::new()) + } + Err(error) => return Err(error.into()), + }; stream.set_read_timeout(Some(IO_TIMEOUT))?; stream.set_write_timeout(Some(IO_TIMEOUT))?; let request = format!( @@ -181,6 +209,24 @@ mod tests { assert!(parse_admin_response(&raw).is_err()); } + #[test] + fn resolve_loopback_addr_accepts_loopback_and_refuses_the_rest() { + // Loopback literals resolve without DNS and pass. Note [::1] passes THIS guard (its job is + // only "the admin marker never leaves this machine") but today's bridge can't bind or serve + // it (`serve`/`host_is_loopback` accept 127.0.0.1/localhost only), so an IPv6 override + // fails CLOSED downstream with a 403 — accepted here for the guard's own honesty, not as + // an end-to-end capability claim. + assert!(resolve_loopback_addr("127.0.0.1:8765").is_ok()); + assert!(resolve_loopback_addr("[::1]:8765").is_ok()); + // A non-loopback literal is refused — the admin marker must never leave this machine, even + // if DECKARD_BRIDGE_ADDR is misconfigured to a routable host. Literal IPs, so no DNS. + assert!(resolve_loopback_addr("8.8.8.8:80").is_err()); + assert!(resolve_loopback_addr("93.184.216.34:80").is_err()); + // Malformed / unresolvable input errors rather than dialing anything. + assert!(resolve_loopback_addr("not-an-address").is_err()); + assert!(resolve_loopback_addr("127.0.0.1").is_err()); + } + #[test] fn origin_host_strips_scheme_and_passes_unknown_through() { assert_eq!(origin_host("https://app.uniswap.org"), "app.uniswap.org"); diff --git a/crates/deckard-app/src/palette.rs b/crates/deckard-app/src/palette.rs index 3b876f3..4a32ad0 100644 --- a/crates/deckard-app/src/palette.rs +++ b/crates/deckard-app/src/palette.rs @@ -37,16 +37,16 @@ fn display_label(this: &Shell, id: &str, title: &str) -> String { "Mask balances".to_string() } } - // The Connections revoke command (#199) names exactly what Enter will disconnect: nothing - // (static title) with no sites, the single site's host with one, or "all N" with several — - // so the palette label and `run_palette_command`'s action never disagree. - "revoke-site" => match this.connections.as_slice() { - [] => title.to_string(), - [only] => format!( + // The Connections revoke command (#199) names exactly what Enter will disconnect. It reads + // the SAME `connection_revoke_target` the action reads (shell.rs), so the label and the + // action can't disagree: nothing (static title) / the single site's host / "all N". + "revoke-site" => match this.connection_revoke_target() { + crate::shell::RevokeTarget::Nothing => title.to_string(), + crate::shell::RevokeTarget::One(origin) => format!( "Revoke connected site — {}", - crate::bridge_admin::origin_host(&only.origin) + crate::bridge_admin::origin_host(&origin) ), - many => format!("Revoke all {} connected sites", many.len()), + crate::shell::RevokeTarget::All(n) => format!("Revoke all {n} connected sites"), }, _ => title.to_string(), } diff --git a/crates/deckard-app/src/shell.rs b/crates/deckard-app/src/shell.rs index 0acecd0..3fa42b7 100644 --- a/crates/deckard-app/src/shell.rs +++ b/crates/deckard-app/src/shell.rs @@ -57,6 +57,20 @@ const PENDING_POLL_SECS: u64 = 5; /// dead bridge fails instantly (`ECONNREFUSED`), so this loop is cheap when no bridge is running. const CONNECTIONS_POLL_SECS: u64 = 5; +/// What the ⌘K "Revoke connected site" command will disconnect (#199). Computed ONCE by +/// [`Shell::connection_revoke_target`] and consumed by BOTH the palette label +/// (`palette::display_label`) and the action (`Shell::run_palette_command`), so the label can never +/// promise a different disconnect than Enter performs — a trust-surface invariant that was +/// previously a hand-maintained partition duplicated across two files. +pub(crate) enum RevokeTarget { + /// No connected sites — the command is a no-op and shows its static title. + Nothing, + /// Exactly one site: disconnect it; the label names its host. + One(String), + /// Several sites: disconnect all; the label reads "all N". + All(usize), +} + /// The confirm arm-delay: a clear-signing review must be on screen this long before ⌘↵ / a /// click can confirm, so a keypress or click carried over from the previous screen can't approve /// a money move (DESIGN §confirm pattern). The ⌘↵ chord plus this delay replace the old hold. @@ -2928,6 +2942,14 @@ impl Shell { // is the real brake). Clearing the bridge FIRST would open a race: a dapp reconnecting in // the window before the daemon revokes would be granted a fresh session that survives the // Lockdown (codex finding); revoked-first, that reconnect gets nothing from the daemon. + // + // CANCEL the connections poll here, exactly as `lock` does. Because the bridge clear is + // sequenced after the daemon revoke round-trip, a still-running poll tick could fetch the + // not-yet-cleared sessions in that window and — capturing the post-bump epoch — fold them + // back in, flashing "N connected" onto a locked-down wallet (the panic brake's promise is + // the row is gone). Dropping the task stops any tick; the next unlock restarts the poll. + self.connections_poll_task = None; + self.connections_inflight = false; self.connections.clear(); self.connections_epoch = self.connections_epoch.wrapping_add(1); let client = self.signer.client(); @@ -3154,9 +3176,10 @@ impl Shell { /// Fetch the bridge's live dapp sessions off the UI thread and fold them into `self.connections` /// (#199). In-flight-guarded so a slow bridge can't stack round-trips; epoch-fenced so a reply /// landing after lock/re-unlock is dropped. Notifies only when the list actually changed (the - /// poll is ambient — no needless repaints). On ERROR it sets the list empty: bridge sessions - /// live in the bridge's RAM, so an unreachable bridge genuinely means no live sessions — the - /// honest answer, never a stale phantom row. + /// poll is ambient — no needless repaints). On ERROR it KEEPS the current list: `bridge_admin` + /// already maps "no bridge listening" to an honest empty list, so a genuinely-down bridge + /// clears; an error here is a reachable-but-unhealthy bridge (a momentary timeout), where + /// blanking would falsely show "no connected sites" for still-live sessions until the next tick. fn kick_connections_refresh(&mut self, cx: &mut Context) { if self.auth != AuthStep::Ready || self.connections_inflight { return; @@ -3181,10 +3204,11 @@ impl Shell { if this.connections_epoch != cepoch { return; } - let next = res.unwrap_or_default(); - if this.connections != next { - this.connections = next; - cx.notify(); + if let Ok(next) = res { + if this.connections != next { + this.connections = next; + cx.notify(); + } } }) .ok(); @@ -3192,52 +3216,77 @@ impl Shell { .detach(); } + /// What the ⌘K "Revoke connected site" command targets right now (#199) — the single source + /// both the palette label and its action read, so they can't disagree about what Enter does. + /// `pub(crate)` for `palette::display_label`. + pub(crate) fn connection_revoke_target(&self) -> RevokeTarget { + match self.connections.as_slice() { + [] => RevokeTarget::Nothing, + [only] => RevokeTarget::One(only.origin.clone()), + many => RevokeTarget::All(many.len()), + } + } + /// Disconnect one connected site (#199): a session-scoped revoke, NOT a persistent ban /// (ADR 0006) — the site's next request fails `4100 Unauthorized` and it may reconnect fresh. - /// The bridge returns the remaining live list, which we fold in immediately (epoch-fenced) so - /// the row disappears without waiting for the next poll tick. A bridge error clears the list - /// (no bridge = no live sessions — the honest, fail-safe direction). Known, accepted edge: two - /// OVERLAPPING revokes whose loopback replies reorder can leave the slower site's row on - /// screen until the next poll tick reconciles (≤5s) — every revoke IS applied on the bridge; - /// only the interim display can lag. `pub(crate)` so the Connections sidebar row's ✕ - /// (`shell_chrome`) can call it. + /// `pub(crate)` so the Connections sidebar row's ✕ (`shell_chrome`) can call it. pub(crate) fn revoke_connected_site(&mut self, origin: String, cx: &mut Context) { - let epoch = self.auth_epoch; - // Bump the connections epoch so an in-flight list poll (a pre-revoke snapshot) is dropped - // instead of resurrecting this row; capture AFTER the bump so our own reply still folds. - self.connections_epoch = self.connections_epoch.wrapping_add(1); - let cepoch = self.connections_epoch; - let task = cx.background_spawn(async move { bridge_admin::revoke_site(&origin) }); - cx.spawn(async move |this, cx| { - let res = task.await; - this.update(cx, |this, cx| { - if this.auth_epoch != epoch || this.connections_epoch != cepoch { - return; - } - this.connections = res.unwrap_or_default(); - cx.notify(); - }) - .ok(); - }) - .detach(); + self.apply_disconnect(move || bridge_admin::revoke_site(&origin), cx); } - /// Disconnect every connected site (#199): the ⌘K "revoke all connected sites" path. Session- - /// scoped like [`revoke_connected_site`]; folds the (now-empty) returned list in immediately. + /// Disconnect every connected site (#199): the ⌘K "revoke all connected sites" path. fn disconnect_all_sites(&mut self, cx: &mut Context) { + self.apply_disconnect(bridge_admin::clear_sites, cx); + } + + /// Run a disconnect op against the bridge and fold its returned live list into + /// `self.connections` (#199). Shared by per-site revoke and revoke-all so the epoch fence, the + /// in-flight guard, and the error handling can never drift between the two paths. + /// + /// - Bumps `connections_epoch` so an in-flight LIST poll carrying a pre-op snapshot is dropped + /// instead of resurrecting a just-revoked row (capture the value AFTER the bump so this op's + /// own reply still folds). + /// - Sets `connections_inflight` so the poll won't START a fresh list fetch under the same + /// post-bump epoch while this op round-trips — closing the poll-vs-revoke fence gap the bare + /// epoch bump left open. + /// - On SUCCESS the returned list replaces `self.connections`; on ERROR the current list is + /// KEPT and the next poll reconciles. A failed revoke must NOT blank sibling rows or claim + /// "all disconnected" — a bridge error is not proof the sessions are gone. (`bridge_admin` + /// already turns "no bridge listening" into an honest empty list, so a genuinely-down bridge + /// still clears; only a reachable-but-unhealthy bridge yields the error handled here.) + /// + /// Accepted edge: two OVERLAPPING revokes whose loopback replies reorder can leave the slower + /// site's row on screen until the next poll tick reconciles (≤5s) — every revoke IS applied on + /// the bridge; only the interim display can lag. + fn apply_disconnect( + &mut self, + op: impl FnOnce() -> anyhow::Result> + Send + 'static, + cx: &mut Context, + ) { let epoch = self.auth_epoch; - // Same stale-poll fence as `revoke_connected_site`: bump, then capture. self.connections_epoch = self.connections_epoch.wrapping_add(1); let cepoch = self.connections_epoch; - let task = cx.background_spawn(async move { bridge_admin::clear_sites() }); + self.connections_inflight = true; + let task = cx.background_spawn(async move { op() }); cx.spawn(async move |this, cx| { let res = task.await; this.update(cx, |this, cx| { - if this.auth_epoch != epoch || this.connections_epoch != cepoch { + if this.auth_epoch != epoch { + // Reply belongs to a dead session (lock/re-unlock already reset the guard). return; } - this.connections = res.unwrap_or_default(); - cx.notify(); + this.connections_inflight = false; + if this.connections_epoch != cepoch { + // A newer disconnect superseded this one; let its reply carry the truth. + return; + } + // Keep the current list on error (see the doc): only a successful op reshapes it. + if let Ok(next) = res { + if this.connections != next { + this.connections = next; + cx.notify(); + } + } }) .ok(); }) @@ -3413,22 +3462,17 @@ impl Shell { self.open_activity(window, cx); self.stop_revoke_all(cx); } - // Disconnect connected dapp site(s) (#199). The action ALWAYS matches the live label - // `display_label` shows: 0 → nothing to do; exactly 1 → disconnect that one site; N>1 → - // disconnect all. Per-site precision at N>1 lives on the sidebar row's ✕; from the - // palette, revoking MORE than one when the label reads "all N" is the fail-safe direction - // (revoking too much only reduces authority). Session-scoped disconnect, not a ban. - "revoke-site" => { - // Snapshot origins into an owned Vec first so the match doesn't hold an immutable - // borrow of `self.connections` across the &mut-self revoke calls below. - let origins: Vec = - self.connections.iter().map(|s| s.origin.clone()).collect(); - match origins.as_slice() { - [] => {} - [only] => self.revoke_connected_site(only.clone(), cx), - _ => self.disconnect_all_sites(cx), - } - } + // Disconnect connected dapp site(s) (#199). The action reads the SAME + // `connection_revoke_target` the palette label rendered, so Enter always disconnects + // exactly what the label named: nothing / that one site / all of them. Per-site + // precision at N>1 lives on the sidebar row's ✕; from the palette, revoking all when the + // label reads "all N" is the fail-safe direction (revoking too much only reduces + // authority). Session-scoped disconnect, not a ban. + "revoke-site" => match self.connection_revoke_target() { + RevokeTarget::Nothing => {} + RevokeTarget::One(origin) => self.revoke_connected_site(origin, cx), + RevokeTarget::All(_) => self.disconnect_all_sites(cx), + }, "settings" => self.open(Surface::Settings, cx), // Rename the wallet / agent (E2, #182): open Settings and focus the field so the ⌘K // command lands the operator directly on the input it named. Drop the captured