From 9dbd8da430fb95263f22f35f4f23bb617596576d Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Thu, 24 Sep 2026 10:57:11 +0800 Subject: [PATCH] feat(ssh): switch a port forward off without losing its rule A forward could only be removed, so pointing one local port at another remote target meant deleting the rule and retyping the other one (#439). Forward rules gain an `enabled` flag (serde default on, so saved profiles and older peers keep every rule live) and the daemon a SetForwardEnabled op for panes and workspaces. Off releases the listener and keeps the entry; on rebinds it from the rule it was made from, and is refused by name when another switched-on forward of the same owner holds the port. The Ports panel and the Settings rules editor each get a switch; a switch flipped in the panel on a rule from a saved host is written back to that host. Closes #439 --- CHANGELOG.md | 16 + assets/icons/power.svg | 1 + crates/tty7-core/src/core/ssh_profile.rs | 31 ++ crates/tty7-core/src/daemon/protocol.rs | 91 +++++ crates/tty7-core/src/daemon/server.rs | 25 ++ crates/tty7-core/src/daemon/ssh/forward.rs | 402 ++++++++++++++++++- crates/tty7-core/src/daemon/ssh/mod.rs | 13 + crates/tty7-core/src/daemon/ssh/workspace.rs | 7 + docs/remote/port-forwarding.mdx | 18 + src/core/ssh_config.rs | 1 + src/terminal/remote.rs | 23 ++ src/ui/app.rs | 122 +++++- src/ui/assets.rs | 1 + src/ui/forwards.rs | 111 ++++- src/ui/i18n/en.rs | 4 + src/ui/i18n/ja.rs | 4 + src/ui/i18n/mod.rs | 4 + src/ui/i18n/zh.rs | 4 + src/ui/right_panel.rs | 35 +- src/ui/settings.rs | 32 +- src/ui/ssh_connect.rs | 13 +- 21 files changed, 933 insertions(+), 25 deletions(-) create mode 100644 assets/icons/power.svg diff --git a/CHANGELOG.md b/CHANGELOG.md index f77640ff..6e98a6bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,22 @@ All notable changes to tty7 are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added + +- **A port forward can be switched off without losing its rule** (#439). A + forward could only be removed, so pointing one local port at a different + remote target meant deleting the rule and typing the other one in again. Each + forward in the Ports section, and each rule under Settings → SSH → Port + forwarding, now has a switch: off releases the listener and keeps the rule in + the list, on binds it again from the rule it was made from. Switching one on + while another switched-on forward holds its port is refused with a notice + naming that forward, rather than failing at bind time. A switch flipped in the + panel on a rule that came from a saved host is written back to that host, so + the next connection opens the same set; a rule saved switched off is listed + but not opened. Profiles saved before this load with every rule on. + ## [26.9.3] - 2026-09-23 ### Added diff --git a/assets/icons/power.svg b/assets/icons/power.svg new file mode 100644 index 00000000..16fc83f7 --- /dev/null +++ b/assets/icons/power.svg @@ -0,0 +1 @@ + diff --git a/crates/tty7-core/src/core/ssh_profile.rs b/crates/tty7-core/src/core/ssh_profile.rs index 97a43d5c..651aaea0 100644 --- a/crates/tty7-core/src/core/ssh_profile.rs +++ b/crates/tty7-core/src/core/ssh_profile.rs @@ -153,6 +153,16 @@ pub struct ForwardRule { pub bind: HostPort, pub target: HostPort, pub description: String, + /// Whether the rule is opened with the connection. A switched-off rule + /// keeps its whole configuration and simply binds nothing, so one local + /// port can be pointed at different targets by switching rules rather + /// than retyping them. + /// + /// Missing from every profile saved before the switch existed, and those + /// rules were all live: the container's `#[serde(default)]` fills it from + /// `Default`, which says `true`, or upgrading would silently turn every + /// saved forward off. + pub enabled: bool, } impl Default for ForwardRule { @@ -162,6 +172,7 @@ impl Default for ForwardRule { bind: HostPort::default(), target: HostPort::default(), description: String::new(), + enabled: true, } } } @@ -569,6 +580,7 @@ mod tests { bind: HostPort::new("127.0.0.1", 8080), target: HostPort::new("10.0.0.1", 80), description: "web".to_string(), + enabled: false, }]; original.socks_proxy = Some(HostPort::new("proxy", 1080)); original.algorithms.kex = vec!["curve25519-sha256".to_string()]; @@ -597,6 +609,25 @@ mod tests { let m: AuthMode = serde_json::from_str("\"agent\"").unwrap(); assert_eq!(m, AuthMode::Agent); } + + #[test] + fn a_forward_rule_saved_before_the_switch_existed_loads_enabled() { + let old = r#"{"name":"box","host":"h","forwards":[ + {"kind":"local","bind":{"host":"127.0.0.1","port":8080}, + "target":{"host":"srv","port":80},"description":"web"} + ]}"#; + let p: SshProfile = serde_json::from_str(old).unwrap(); + assert_eq!(p.forwards.len(), 1); + assert!(p.forwards[0].enabled, "an old rule must stay live"); + assert!(ForwardRule::default().enabled); + + let mut off = p.forwards[0].clone(); + off.enabled = false; + let back: ForwardRule = + serde_json::from_str(&serde_json::to_string(&off).unwrap()).unwrap(); + assert!(!back.enabled, "a switched-off rule stays off across a save"); + assert_eq!(back, off); + } } fn new_id() -> Uuid { diff --git a/crates/tty7-core/src/daemon/protocol.rs b/crates/tty7-core/src/daemon/protocol.rs index 3f9632f4..cc89de7e 100644 --- a/crates/tty7-core/src/daemon/protocol.rs +++ b/crates/tty7-core/src/daemon/protocol.rs @@ -181,6 +181,10 @@ pub enum WorkspaceOp { RemoveForward { forward_id: u64, }, + SetForwardEnabled { + forward_id: u64, + enabled: bool, + }, ListForwards, TeardownForwards, SftpList { @@ -261,6 +265,13 @@ pub struct SshForwardRule { pub target_port: u16, #[serde(default)] pub description: Option, + /// A switched-off rule is registered but binds nothing. Defaults to on, + /// and is not a `PROTOCOL_VERSION` bump: a peer that predates it never + /// sends it and every rule it knows is live, while one that receives it + /// ignores it — the worst case is an older far end opening a rule the + /// user switched off, which is what it always did. + #[serde(default = "default_true")] + pub enabled: bool, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -420,6 +431,13 @@ pub struct ManagedForward { #[serde(default)] pub description: Option, pub status: ForwardStatus, + /// Whether the forward is switched on. A field rather than a + /// `ForwardStatus` variant for the reason `loop_exit_status` gives: an + /// older peer fails the whole frame on a variant it has never heard of, + /// but skips a field it does not know. A switched-off forward's status is + /// an `Error` saying so, which is what such a peer then draws. + #[serde(default = "default_true")] + pub enabled: bool, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -892,6 +910,11 @@ pub enum ClientMsg { pane_id: u64, forward_id: u64, }, + SetForwardEnabled { + pane_id: u64, + forward_id: u64, + enabled: bool, + }, ListForwards { pane_id: u64, }, @@ -987,6 +1010,7 @@ mod kind { pub const ADD_FORWARD: u8 = 20; pub const REMOVE_FORWARD: u8 = 21; pub const LIST_FORWARDS: u8 = 22; + pub const SET_FORWARD_ENABLED: u8 = 23; pub const VERSION: u8 = 40; pub const QUERY_PROCS: u8 = 50; pub const ON_WORKSPACE: u8 = 52; @@ -1260,6 +1284,15 @@ impl ClientMsg { pane_id, forward_id, } => write_frame(w, kind::REMOVE_FORWARD, &to_json(&(pane_id, forward_id))?), + ClientMsg::SetForwardEnabled { + pane_id, + forward_id, + enabled, + } => write_frame( + w, + kind::SET_FORWARD_ENABLED, + &to_json(&(pane_id, forward_id, enabled))?, + ), ClientMsg::QueryProcs { pane_id } => { write_frame(w, kind::QUERY_PROCS, &to_json(pane_id)?) } @@ -1390,6 +1423,14 @@ impl ClientMsg { forward_id, } } + kind::SET_FORWARD_ENABLED => { + let (pane_id, forward_id, enabled) = from_json(&payload)?; + ClientMsg::SetForwardEnabled { + pane_id, + forward_id, + enabled, + } + } kind::LIST_FORWARDS => ClientMsg::ListForwards { pane_id: from_json(&payload)?, }, @@ -1758,6 +1799,7 @@ mod tests { target_host: "10.0.0.5".into(), target_port: 80, description: Some("web".into()), + enabled: true, }, }, ClientMsg::AddForward { @@ -1769,12 +1811,18 @@ mod tests { target_host: String::new(), target_port: 0, description: None, + enabled: false, }, }, ClientMsg::RemoveForward { pane_id: 7, forward_id: 3, }, + ClientMsg::SetForwardEnabled { + pane_id: 7, + forward_id: 3, + enabled: false, + }, ClientMsg::ListForwards { pane_id: 7 }, ClientMsg::Version, ]; @@ -1934,6 +1982,7 @@ mod tests { target_port: 80, description: Some("web".into()), status: ForwardStatus::Listening, + enabled: true, }, ManagedForward { id: 2, @@ -1945,6 +1994,7 @@ mod tests { target_port: 3000, description: None, status: ForwardStatus::Error("bind refused".into()), + enabled: false, }, ]), DaemonMsg::Version(DaemonVersion { @@ -2385,6 +2435,7 @@ mod tests { target_host: "127.0.0.1".into(), target_port: 80, description: Some("web".into()), + enabled: true, }], keepalive_interval_s: Some(30), keepalive_count_max: Some(3), @@ -2471,9 +2522,14 @@ mod tests { target_host: "127.0.0.1".into(), target_port: 5432, description: Some("db".into()), + enabled: true, }, }, WorkspaceOp::RemoveForward { forward_id: 4 }, + WorkspaceOp::SetForwardEnabled { + forward_id: 4, + enabled: true, + }, WorkspaceOp::ListForwards, WorkspaceOp::TeardownForwards, WorkspaceOp::SftpList { @@ -2702,4 +2758,39 @@ mod tests { "the session daemon must not advertise a dialect it cannot serve" ); } + + /// `enabled` crosses the same peers as `rejected` above and gets the same + /// treatment: a rule or a forward from a peer that predates the switch is + /// a live one, and a peer that predates it reads a frame carrying it. + #[test] + fn a_forward_enabled_flag_decodes_from_a_peer_that_never_sends_it() { + let rule: SshForwardRule = serde_json::from_str( + r#"{"kind":"local","bind_host":"127.0.0.1","bind_port":8080, + "target_host":"srv","target_port":80}"#, + ) + .unwrap(); + assert!(rule.enabled); + + let managed: ManagedForward = serde_json::from_str( + r#"{"id":1,"pane_id":7,"kind":"local","bind_host":"127.0.0.1", + "bind_port":8080,"status":"listening"}"#, + ) + .unwrap(); + assert!(managed.enabled); + + #[derive(Deserialize)] + struct LegacyManagedForward { + id: u64, + status: ForwardStatus, + } + let off = serde_json::to_string(&ManagedForward { + enabled: false, + status: ForwardStatus::Error("disabled".into()), + ..managed + }) + .unwrap(); + let legacy: LegacyManagedForward = serde_json::from_str(&off).unwrap(); + assert_eq!(legacy.id, 1); + assert_eq!(legacy.status, ForwardStatus::Error("disabled".into())); + } } diff --git a/crates/tty7-core/src/daemon/server.rs b/crates/tty7-core/src/daemon/server.rs index d2a43c73..f0da41de 100644 --- a/crates/tty7-core/src/daemon/server.rs +++ b/crates/tty7-core/src/daemon/server.rs @@ -1035,6 +1035,31 @@ fn handle_conn(stream: Stream, registry: Arc) -> anyhow::Result<()> { Ok(()) } + ClientMsg::SetForwardEnabled { + pane_id, + forward_id, + enabled, + } => { + let mut w = write_stream; + // Only switching on needs the connection; switching off has to + // work on a pane whose connection is already gone. + let conn = match forward_pane_connection(®istry, pane_id) { + Ok(conn) => Some(conn), + Err(e) if enabled => { + DaemonMsg::Error(e).encode(&mut w)?; + return Ok(()); + } + Err(_) => None, + }; + match crate::daemon::ssh::SshManager::global() + .set_forward_enabled(pane_id, conn, forward_id, enabled) + { + Ok(list) => DaemonMsg::ForwardList(list).encode(&mut w)?, + Err(e) => DaemonMsg::Error(e).encode(&mut w)?, + } + Ok(()) + } + ClientMsg::QueryProcs { pane_id } => { let mut w = write_stream; let procs = registry.get(pane_id).map(|p| p.procs()).unwrap_or_default(); diff --git a/crates/tty7-core/src/daemon/ssh/forward.rs b/crates/tty7-core/src/daemon/ssh/forward.rs index 726ff9f1..e6d055ef 100644 --- a/crates/tty7-core/src/daemon/ssh/forward.rs +++ b/crates/tty7-core/src/daemon/ssh/forward.rs @@ -257,17 +257,39 @@ fn bind_failed(rule: &SshForwardRule, e: io::Error) -> SharedStatus { )))) } +/// What a switched-off forward reports. An `Error` for the same reason as +/// `loop_exit_status`, so an older peer reading it draws a stopped forward +/// with a word of explanation; a current one reads `enabled` instead. +fn disabled_status() -> SharedStatus { + Arc::new(Mutex::new(ForwardStatus::Error("disabled".to_string()))) +} + +/// Whether two forwards would want the same listening socket: the same port, +/// on the same side of the connection. Local and dynamic forwards both listen +/// here; a remote forward listens on the far host. +fn same_socket(a: SshForwardKind, a_port: u16, b: SshForwardKind, b_port: u16) -> bool { + let far = |k| k == SshForwardKind::Remote; + a_port != 0 && a_port == b_port && far(a) == far(b) +} + struct ForwardEntry { id: u64, kind: SshForwardKind, bind_host: String, bind_port: u16, + /// The port the rule asked for, which `bind_port` is not when that was 0 + /// and the OS picked one. Switching the forward back on asks again for + /// this, not for whatever it happened to get last time. + requested_port: u16, target_host: String, target_port: u16, description: Option, status: SharedStatus, cancel: ForwardCancel, auto_local: bool, + /// Switched off: the entry, and the rule in it, stay listed, but nothing + /// is bound and `cancel` is `None`. + enabled: bool, } #[derive(Clone, PartialEq, Eq, Hash, Debug)] @@ -288,6 +310,19 @@ impl ForwardEntry { target_port: self.target_port, description: self.description.clone(), status: self.status.lock().unwrap().clone(), + enabled: self.enabled, + } + } + + fn rule(&self) -> SshForwardRule { + SshForwardRule { + kind: self.kind, + bind_host: self.bind_host.clone(), + bind_port: self.requested_port, + target_host: self.target_host.clone(), + target_port: self.target_port, + description: self.description.clone(), + enabled: self.enabled, } } } @@ -318,6 +353,23 @@ impl SshForwardRegistry { .await } + pub async fn set_enabled( + &self, + pane_id: u64, + conn: Option>, + forward_id: u64, + enabled: bool, + ) -> Result, String> { + self.set_enabled_owned( + &ForwardOwner::Pane(pane_id), + pane_id, + conn, + forward_id, + enabled, + ) + .await + } + pub async fn teardown_pane(&self, pane_id: u64) { self.teardown_owned(&ForwardOwner::Pane(pane_id)).await; } @@ -347,6 +399,24 @@ impl SshForwardRegistry { .await } + pub async fn set_enabled_workspace( + &self, + workspace: WorkspaceId, + view_pane: u64, + conn: Arc, + forward_id: u64, + enabled: bool, + ) -> Result, String> { + self.set_enabled_owned( + &ForwardOwner::Workspace(workspace), + view_pane, + Some(conn), + forward_id, + enabled, + ) + .await + } + pub async fn teardown_workspace(&self, workspace: WorkspaceId) { self.teardown_owned(&ForwardOwner::Workspace(workspace)) .await; @@ -360,22 +430,24 @@ impl SshForwardRegistry { rule: &SshForwardRule, ) -> ManagedForward { let id = self.next_id.fetch_add(1, Ordering::Relaxed); - let (bind_port, status, cancel) = match rule.kind { - SshForwardKind::Local => self.start_local(&conn, rule).await, - SshForwardKind::Dynamic => self.start_dynamic(&conn, rule).await, - SshForwardKind::Remote => self.start_remote(&conn, rule).await, + let (bind_port, status, cancel) = if rule.enabled { + self.start(&conn, rule).await + } else { + (rule.bind_port, disabled_status(), ForwardCancel::None) }; let entry = ForwardEntry { id, kind: rule.kind, bind_host: rule.bind_host.clone(), bind_port, + requested_port: rule.bind_port, target_host: rule.target_host.clone(), target_port: rule.target_port, description: rule.description.clone(), status, cancel, auto_local: false, + enabled: rule.enabled, }; let managed = entry.to_managed(view_pane); self.owners @@ -413,20 +485,145 @@ impl SshForwardRegistry { }) }; if let Some(entry) = removed { - Self::cancel_entry(entry).await; + Self::cancel(entry.cancel).await; } self.list_owned(owner, view_pane) } + /// Switch one forward off or back on, keeping its entry — and so its + /// place in the list and the rule it was made from — either way. + /// + /// Switching on refuses, rather than fails at bind time with whatever the + /// OS says, when another switched-on forward of the same owner already + /// holds the port: the answer then names that forward, which is the one + /// the user has to switch off first. A forward that cannot be started for + /// any other reason stays off, with the reason as the error. + async fn set_enabled_owned( + &self, + owner: &ForwardOwner, + view_pane: u64, + conn: Option>, + forward_id: u64, + enabled: bool, + ) -> Result, String> { + if !enabled { + let cancel = { + let mut owners = self.owners.lock().unwrap(); + let entry = Self::entry_mut(&mut owners, owner, forward_id)?; + entry.enabled = false; + entry.status = disabled_status(); + std::mem::replace(&mut entry.cancel, ForwardCancel::None) + }; + Self::cancel(cancel).await; + return Ok(self.list_owned(owner, view_pane)); + } + + let Some(rule) = self.enable_plan(owner, forward_id)? else { + return Ok(self.list_owned(owner, view_pane)); + }; + let Some(conn) = conn else { + return Err("the SSH connection is gone — reconnect and try again".to_string()); + }; + let (bound, status, cancel) = self.start(&conn, &rule).await; + if let Some(e) = bind_error(&status) { + Self::cancel(cancel).await; + return Err(e); + } + if let Some(cancel) = self.finish_enable(owner, forward_id, bound, status, cancel) { + // Removed while it was being started: nobody wants what was bound. + Self::cancel(cancel).await; + } + Ok(self.list_owned(owner, view_pane)) + } + + fn entry_mut<'a>( + owners: &'a mut HashMap>, + owner: &ForwardOwner, + forward_id: u64, + ) -> Result<&'a mut ForwardEntry, String> { + owners + .get_mut(owner) + .and_then(|entries| entries.iter_mut().find(|e| e.id == forward_id)) + .ok_or_else(|| format!("no forward {forward_id}")) + } + + /// The rule to start for switching `forward_id` on, `None` when it is on + /// already, or why it may not be switched on. + fn enable_plan( + &self, + owner: &ForwardOwner, + forward_id: u64, + ) -> Result, String> { + let mut owners = self.owners.lock().unwrap(); + let entry = Self::entry_mut(&mut owners, owner, forward_id)?; + if entry.enabled { + return Ok(None); + } + let rule = SshForwardRule { + enabled: true, + ..entry.rule() + }; + let holder = owners.get(owner).into_iter().flatten().find(|e| { + e.id != forward_id + && e.enabled + && same_socket(e.kind, e.bind_port, rule.kind, rule.bind_port) + }); + if let Some(h) = holder { + let what = match h.kind { + SshForwardKind::Dynamic => "a SOCKS forward".to_string(), + _ => format!("the forward to {}:{}", h.target_host, h.target_port), + }; + return Err(format!( + "port {} is already used by {what}; switch that one off first", + rule.bind_port + )); + } + Ok(Some(rule)) + } + + /// Record a started forward as switched on. Hands `cancel` back when the + /// entry is no longer there to take it. + fn finish_enable( + &self, + owner: &ForwardOwner, + forward_id: u64, + bound: u16, + status: SharedStatus, + cancel: ForwardCancel, + ) -> Option { + let mut owners = self.owners.lock().unwrap(); + let Ok(entry) = Self::entry_mut(&mut owners, owner, forward_id) else { + return Some(cancel); + }; + entry.enabled = true; + entry.bind_port = bound; + entry.status = status; + // Anything already in the slot is a second switch-on that raced this + // one; it is replaced, and has to be stopped rather than leaked. + Some(std::mem::replace(&mut entry.cancel, cancel)) + } + + async fn start( + &self, + conn: &Arc, + rule: &SshForwardRule, + ) -> (u16, SharedStatus, ForwardCancel) { + match rule.kind { + SshForwardKind::Local => self.start_local(conn, rule).await, + SshForwardKind::Dynamic => self.start_dynamic(conn, rule).await, + SshForwardKind::Remote => self.start_remote(conn, rule).await, + } + } + async fn teardown_owned(&self, owner: &ForwardOwner) { let entries = self.owners.lock().unwrap().remove(owner); for entry in entries.into_iter().flatten() { - Self::cancel_entry(entry).await; + Self::cancel(entry.cancel).await; } } - async fn cancel_entry(entry: ForwardEntry) { - match entry.cancel { + async fn cancel(cancel: ForwardCancel) { + match cancel { ForwardCancel::Task(handle) => { handle.abort(); let _ = handle.await; @@ -634,6 +831,7 @@ impl SshForwardRegistry { target_host: remote_host.to_string(), target_port: remote_port, description: Some(format!("localhost link → :{remote_port}")), + enabled: true, }; let id = self.next_id.fetch_add(1, Ordering::Relaxed); let (mut bind_port, mut status, mut cancel) = self.start_local(&conn, &rule).await; @@ -658,12 +856,14 @@ impl SshForwardRegistry { kind: SshForwardKind::Local, bind_host: rule.bind_host.clone(), bind_port, + requested_port: rule.bind_port, target_host: rule.target_host.clone(), target_port: rule.target_port, description: rule.description.clone(), status, cancel, auto_local: true, + enabled: true, }; self.owners .lock() @@ -735,6 +935,20 @@ impl SshManager { ) } + pub fn set_workspace_forward_enabled( + &self, + workspace: WorkspaceId, + view_pane: u64, + conn: Arc, + forward_id: u64, + enabled: bool, + ) -> Result, String> { + self.runtime.block_on( + self.forwards + .set_enabled_workspace(workspace, view_pane, conn, forward_id, enabled), + ) + } + pub fn list_workspace_forwards( &self, workspace: WorkspaceId, @@ -941,12 +1155,14 @@ mod tests { kind: SshForwardKind::Local, bind_host: "127.0.0.1".into(), bind_port: port, + requested_port: port, target_host: "h".into(), target_port: 80, description: None, status: Arc::new(Mutex::new(ForwardStatus::Listening)), cancel: ForwardCancel::Task(task), auto_local: false, + enabled: true, } }; { @@ -986,12 +1202,14 @@ mod tests { kind: SshForwardKind::Local, bind_host: "127.0.0.1".into(), bind_port: port, + requested_port: port, target_host: "h".into(), target_port: 80, description: None, status: Arc::new(Mutex::new(ForwardStatus::Listening)), cancel: ForwardCancel::Task(handle), auto_local: false, + enabled: true, } } @@ -1047,12 +1265,14 @@ mod tests { kind: SshForwardKind::Local, bind_host: "127.0.0.1".into(), bind_port: port, + requested_port: port, target_host: "127.0.0.1".into(), target_port: 3000, description: None, status: Arc::new(Mutex::new(ForwardStatus::Listening)), cancel: ForwardCancel::Task(handle), auto_local: true, + enabled: true, }; reg.owners .lock() @@ -1143,4 +1363,170 @@ mod tests { ); assert_eq!(table.lookup("", 0), None); } + + fn push_disabled( + reg: &SshForwardRegistry, + owner: ForwardOwner, + id: u64, + kind: SshForwardKind, + port: u16, + ) { + reg.owners + .lock() + .unwrap() + .entry(owner) + .or_default() + .push(ForwardEntry { + id, + kind, + bind_host: "127.0.0.1".into(), + bind_port: port, + requested_port: port, + target_host: "srv-b".into(), + target_port: 80, + description: None, + status: disabled_status(), + cancel: ForwardCancel::None, + auto_local: false, + enabled: false, + }); + } + + /// Switching a forward off has to release what it bound — that is the + /// point, so the port can be taken by another rule — while leaving the + /// entry, and the rule in it, exactly where it was. + #[tokio::test] + async fn switching_a_forward_off_frees_its_socket_and_keeps_its_rule() { + let reg = SshForwardRegistry::default(); + let guard = push_listener(®, ForwardOwner::Pane(7), 3).await; + let port = reg.list(7)[0].bind_port; + + let list = reg.set_enabled(7, None, 3, false).await.unwrap(); + assert_eq!(list.len(), 1, "the forward is still listed"); + assert_eq!(list[0].id, 3); + assert!(!list[0].enabled); + assert_eq!(list[0].bind_port, port); + assert_eq!( + (list[0].target_host.as_str(), list[0].target_port), + ("127.0.0.1", 3000) + ); + assert!(matches!(list[0].status, ForwardStatus::Error(_))); + assert_eq!(Arc::strong_count(&guard), 1, "…and its listener is gone"); + TcpListener::bind(("127.0.0.1", port)) + .await + .expect("the port is free for someone else"); + + // Twice is the same as once, and a forward nobody has is an error. + assert_eq!(reg.set_enabled(7, None, 3, false).await.unwrap(), list); + assert!(reg.set_enabled(7, None, 99, false).await.is_err()); + + reg.teardown_pane(7).await; + assert!(reg.list(7).is_empty()); + } + + /// Two rules on one local port, pointed at different targets, is the + /// whole use of the switch. Turning the second on while the first still + /// holds the port is refused by name, not left to fail at bind time. + #[tokio::test] + async fn switching_on_a_forward_whose_port_is_held_is_refused_by_name() { + let reg = SshForwardRegistry::default(); + push_listener(®, ForwardOwner::Pane(7), 0).await; + let port = reg.list(7)[0].bind_port; + push_disabled(®, ForwardOwner::Pane(7), 1, SshForwardKind::Local, port); + + let err = reg.set_enabled(7, None, 1, true).await.unwrap_err(); + assert!(err.contains(&port.to_string()), "{err}"); + assert!(err.contains("127.0.0.1:3000"), "names the holder: {err}"); + assert!(!reg.list(7)[1].enabled, "the refused forward stays off"); + + // A SOCKS forward listens here too, so it collides the same way. + push_disabled( + ®, + ForwardOwner::Pane(7), + 2, + SshForwardKind::Dynamic, + port, + ); + assert!(reg.set_enabled(7, None, 2, true).await.is_err()); + + // Once the holder is off, the only thing left in the way is the + // missing connection — which is said, and changes nothing. + reg.set_enabled(7, None, 0, false).await.unwrap(); + let err = reg.set_enabled(7, None, 1, true).await.unwrap_err(); + assert!(err.contains("connection"), "{err}"); + assert!(reg.list(7).iter().all(|m| !m.enabled)); + + // Another owner's forwards are not this one's business. + push_disabled(®, ForwardOwner::Pane(8), 0, SshForwardKind::Local, port); + assert!( + reg.enable_plan(&ForwardOwner::Pane(8), 0) + .unwrap() + .is_some() + ); + } + + #[test] + fn only_the_same_port_on_the_same_side_collides() { + use SshForwardKind::{Dynamic, Local, Remote}; + assert!(same_socket(Local, 8080, Local, 8080)); + assert!(same_socket(Local, 8080, Dynamic, 8080)); + assert!(same_socket(Remote, 8080, Remote, 8080)); + assert!(!same_socket(Local, 8080, Remote, 8080), "different hosts"); + assert!(!same_socket(Local, 8080, Local, 8081)); + assert!( + !same_socket(Local, 0, Local, 0), + "0 asks the OS for a fresh one" + ); + } + + /// Switching back on asks for the port the rule asked for, not the one + /// the OS last picked for it, and records what it got this time. + #[tokio::test] + async fn switching_on_starts_from_the_rule_and_records_the_new_listener() { + let reg = SshForwardRegistry::default(); + push_disabled(®, ForwardOwner::Pane(7), 0, SshForwardKind::Local, 0); + reg.owners + .lock() + .unwrap() + .get_mut(&ForwardOwner::Pane(7)) + .unwrap()[0] + .bind_port = 41000; + + let rule = reg + .enable_plan(&ForwardOwner::Pane(7), 0) + .unwrap() + .expect("an off forward has something to start"); + assert_eq!(rule.bind_port, 0); + assert!(rule.enabled); + assert_eq!((rule.target_host.as_str(), rule.target_port), ("srv-b", 80)); + + let guard = Arc::new(()); + let held = guard.clone(); + let task = tokio::spawn(async move { + let _held = held; + std::future::pending::<()>().await + }); + let status: SharedStatus = Arc::new(Mutex::new(ForwardStatus::Listening)); + let leftover = reg.finish_enable( + &ForwardOwner::Pane(7), + 0, + 42000, + status, + ForwardCancel::Task(task), + ); + assert!(matches!(leftover, Some(ForwardCancel::None))); + let now = ®.list(7)[0]; + assert!(now.enabled); + assert_eq!(now.bind_port, 42000); + assert_eq!(now.status, ForwardStatus::Listening); + assert!( + reg.enable_plan(&ForwardOwner::Pane(7), 0) + .unwrap() + .is_none(), + "an on forward has nothing to start" + ); + + reg.teardown_pane(7).await; + assert_eq!(Arc::strong_count(&guard), 1, "the new listener is owned"); + } } diff --git a/crates/tty7-core/src/daemon/ssh/mod.rs b/crates/tty7-core/src/daemon/ssh/mod.rs index 6cfddade..937c06ef 100644 --- a/crates/tty7-core/src/daemon/ssh/mod.rs +++ b/crates/tty7-core/src/daemon/ssh/mod.rs @@ -115,6 +115,19 @@ impl SshManager { .block_on(self.forwards.remove(pane_id, forward_id)) } + pub fn set_forward_enabled( + &self, + pane_id: u64, + conn: Option>, + forward_id: u64, + enabled: bool, + ) -> Result, String> { + self.runtime.block_on( + self.forwards + .set_enabled(pane_id, conn, forward_id, enabled), + ) + } + pub fn list_forwards(&self, pane_id: u64) -> Vec { self.forwards.list(pane_id) } diff --git a/crates/tty7-core/src/daemon/ssh/workspace.rs b/crates/tty7-core/src/daemon/ssh/workspace.rs index e022de4e..657faffb 100644 --- a/crates/tty7-core/src/daemon/ssh/workspace.rs +++ b/crates/tty7-core/src/daemon/ssh/workspace.rs @@ -33,6 +33,13 @@ pub fn handle(req: &WorkspaceRequest) -> DaemonMsg { WorkspaceOp::RemoveForward { forward_id } => { DaemonMsg::ForwardList(mgr.remove_workspace_forward(ws, view, *forward_id)) } + WorkspaceOp::SetForwardEnabled { + forward_id, + enabled, + } => match mgr.set_workspace_forward_enabled(ws, view, conn, *forward_id, *enabled) { + Ok(list) => DaemonMsg::ForwardList(list), + Err(e) => DaemonMsg::Error(e), + }, WorkspaceOp::ListForwards => DaemonMsg::ForwardList(mgr.list_workspace_forwards(ws, view)), WorkspaceOp::TeardownForwards => { mgr.teardown_workspace_forwards(ws); diff --git a/docs/remote/port-forwarding.mdx b/docs/remote/port-forwarding.mdx index 4002d9cb..1b4c424f 100644 --- a/docs/remote/port-forwarding.mdx +++ b/docs/remote/port-forwarding.mdx @@ -67,6 +67,24 @@ need it about as often as you need `ssh -R`. These live only as long as the session unless you save them into the profile. +## Switching a rule off + +Every forward in the Ports section has a switch. Off, the forward lets go of +its port but keeps its whole rule — kind, ports, target, description — so it +can be switched back on without retyping anything. That is how one local port +is pointed at different places: keep one rule per target on the same port, and +switch on the one you want. + +Only one of them can hold the port at a time. Switching one on while another +still has its port is refused, and the notice names the forward to switch off +first. + +Rules from a profile carry their switch into it, so the next connection opens +the same set. The same switch sits beside each rule under **Settings → SSH →** +a profile **→ Port forwarding**; a rule saved switched off is listed but not +opened. Rules added mid-session are still gone when the session ends, on or +off. + The ports section diff --git a/src/core/ssh_config.rs b/src/core/ssh_config.rs index c98b452f..101e5ea2 100644 --- a/src/core/ssh_config.rs +++ b/src/core/ssh_config.rs @@ -883,6 +883,7 @@ fn parse_forward_rule(kind: ForwardKind, value: &str) -> Option { bind, target, description: String::new(), + enabled: true, }) } diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index b319c5c2..340596d7 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -2296,6 +2296,29 @@ impl RemoteTerminal { .ok() } + /// The list after switching one forward on or off, or why it was not + /// switched — the daemon's own refusal, which names what is in the way. + pub fn set_forward_enabled( + pane_id: u64, + forward_id: u64, + enabled: bool, + ) -> anyhow::Result> { + let mut stream = connect()?; + ClientMsg::SetForwardEnabled { + pane_id, + forward_id, + enabled, + } + .encode(&mut stream)?; + match DaemonMsg::read(&mut stream)? { + DaemonMsg::ForwardList(list) => Ok(list), + DaemonMsg::Error(msg) => Err(anyhow::anyhow!(msg)), + other => Err(anyhow::anyhow!( + "unexpected reply to SetForwardEnabled: {other:?}" + )), + } + } + pub fn list_forwards(pane_id: u64) -> Vec { fn query(pane_id: u64) -> anyhow::Result> { let mut stream = connect()?; diff --git a/src/ui/app.rs b/src/ui/app.rs index 51e5b132..706f365b 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -3049,7 +3049,7 @@ impl Tty7App { cx: &mut Context, ) { let fields = self.managed_forward_fields(cx); - let Some(rule) = fields.collect() else { + let Some(mut rule) = fields.collect() else { // Add is disabled while the fields do not make a rule and the form // already says what is missing, so there is nothing to do here and // nothing left to explain. @@ -3057,6 +3057,10 @@ impl Tty7App { }; let route = self.forward_route(pane_id, cx); let previous = self.loopback_panel.mf_editing.clone(); + // Editing a switched-off rule edits it; it does not switch it on. + if let Some(old) = &previous { + rule.enabled = old.enabled; + } // A saved edit is a replace, and the rule being replaced has to come // out first: the ordinary edit keeps the bind port, and the far side // really does bind it, so adding first would collide with the very @@ -3160,10 +3164,14 @@ impl Tty7App { let Some(list) = route.add(rule) else { return PlaceOutcome::Unreachable(t(L10nKey::ForwardRequestFailed).to_string()); }; - let broken = added_forward(&before, &list).and_then(|added| match &added.status { - ForwardStatus::Error(msg) => Some((added.id, msg.clone())), - ForwardStatus::Listening => None, - }); + // A switched-off rule reports an error status because it binds + // nothing, and that is what it was asked to do. + let broken = added_forward(&before, &list) + .filter(|added| added.enabled) + .and_then(|added| match &added.status { + ForwardStatus::Error(msg) => Some((added.id, msg.clone())), + ForwardStatus::Listening => None, + }); self.loopback_panel.managed = list; let Some((id, msg)) = broken else { return PlaceOutcome::Placed; @@ -3251,6 +3259,84 @@ impl Tty7App { cx.notify(); } + /// Switch one forward on or off, and carry the choice into the saved host + /// the rule came from, if it came from one — so the next connection opens + /// the same set of rules instead of the ones that were saved before. + pub(crate) fn set_managed_forward_enabled( + &mut self, + pane_id: u64, + forward_id: u64, + enabled: bool, + window: &mut Window, + cx: &mut Context, + ) { + let route = self.forward_route(pane_id, cx); + let forward = self + .loopback_panel + .managed + .iter() + .find(|m| m.id == forward_id) + .cloned(); + match route.set_enabled(forward_id, enabled) { + Ok(list) => { + self.loopback_panel.managed = list; + if let Some(forward) = forward { + self.remember_forward_enabled(pane_id, &forward, enabled, cx); + } + } + Err(e) => { + window.push_notification( + t_fmt(L10nKey::ForwardSwitchFailed, &[("error", &e.to_string())]), + cx, + ); + // The refusal changed nothing, but the panel may be behind + // whatever did change — ask rather than guess. + self.loopback_panel.managed = route.list(); + } + } + cx.notify(); + } + + fn remember_forward_enabled( + &mut self, + pane_id: u64, + forward: &crate::daemon::protocol::ManagedForward, + enabled: bool, + cx: &mut Context, + ) { + let profile_id = self + .tabs + .iter() + .flat_map(|tab| tab.pane.terminals()) + .find_map(|leaf| { + let view = leaf.read(cx); + (view.pane_id == pane_id).then(|| view.ssh_spec()?.profile_id.clone())? + }) + .and_then(|id| uuid::Uuid::parse_str(&id).ok()); + let Some(profile_id) = profile_id else { + return; + }; + let saved = cx + .global::() + .ssh_profiles + .iter() + .find(|p| p.id == profile_id) + .and_then(|p| crate::ui::forwards::saved_rule_index(&p.forwards, forward, enabled)); + let Some(idx) = saved else { + return; + }; + self.update_config(cx, |cfg| { + if let Some(rule) = cfg + .ssh_profiles + .iter_mut() + .find(|p| p.id == profile_id) + .and_then(|p| p.forwards.get_mut(idx)) + { + rule.enabled = enabled; + } + }); + } + pub(crate) fn show_ssh_forwards(&mut self, window: &mut Window, cx: &mut Context) { // Whatever the panel would let this pane forward, which is a wider set // than "a connected native-ssh pane": a pane in a remote workspace @@ -8221,6 +8307,31 @@ impl ForwardRoute { } } + pub(crate) fn set_enabled( + &self, + forward_id: u64, + enabled: bool, + ) -> anyhow::Result> { + let Some(req) = + self.workspace_op(crate::daemon::protocol::WorkspaceOp::SetForwardEnabled { + forward_id, + enabled, + }) + else { + return crate::terminal::RemoteTerminal::set_forward_enabled( + self.pane_id, + forward_id, + enabled, + ); + }; + match crate::terminal::RemoteTerminal::on_workspace(req)? { + crate::daemon::protocol::DaemonMsg::ForwardList(list) => Ok(list), + other => Err(anyhow::anyhow!( + "unexpected reply to SetForwardEnabled: {other:?}" + )), + } + } + pub(crate) fn remove( &self, forward_id: u64, @@ -11837,6 +11948,7 @@ mod managed_forward_gpui_tests { target_port: 80, description: None, status: ForwardStatus::Listening, + enabled: true, } } diff --git a/src/ui/assets.rs b/src/ui/assets.rs index 2bf9e9ab..2dcb216a 100644 --- a/src/ui/assets.rs +++ b/src/ui/assets.rs @@ -47,6 +47,7 @@ fn agent_icon(path: &str) -> Option<&'static [u8]> { "icons/machine-local.svg" => include_bytes!("../../assets/icons/machine-local.svg"), "icons/machine-remote.svg" => include_bytes!("../../assets/icons/machine-remote.svg"), "icons/refresh.svg" => include_bytes!("../../assets/icons/refresh.svg"), + "icons/power.svg" => include_bytes!("../../assets/icons/power.svg"), "icons/agents/claude.svg" => include_bytes!("../../assets/icons/agents/claude.svg"), "icons/agents/codex.svg" => include_bytes!("../../assets/icons/agents/codex.svg"), "icons/agents/traecli.svg" => include_bytes!("../../assets/icons/agents/traecli.svg"), diff --git a/src/ui/forwards.rs b/src/ui/forwards.rs index 3257016d..0b00cb10 100644 --- a/src/ui/forwards.rs +++ b/src/ui/forwards.rs @@ -3,6 +3,7 @@ use gpui_component::button::{Button, ButtonVariants as _}; use gpui_component::input::Input; use gpui_component::{ActiveTheme as _, Disableable as _, IconName, Sizable as _, h_flex, v_flex}; +use crate::core::ssh_profile::ForwardRule; use crate::daemon::protocol::{ForwardStatus, ManagedForward, SshForwardKind, SshForwardRule}; use crate::terminal::view::TerminalView; use crate::ui::app::{CONTENT_INSET, Tty7App}; @@ -70,6 +71,7 @@ impl ForwardFields { target_host, target_port, description: (!description.is_empty()).then(|| description.to_string()), + enabled: true, }) } @@ -93,6 +95,7 @@ impl ForwardFields { // left over from a trip through the advanced form is not something // this rule was given. description: None, + enabled: true, }) } @@ -140,9 +143,34 @@ pub(crate) fn rule_of(forward: &ManagedForward) -> SshForwardRule { target_host: forward.target_host.clone(), target_port: forward.target_port, description: forward.description.clone(), + enabled: forward.enabled, } } +/// Which of a saved host's rules a live forward was opened from, among +/// those not already switched to `enabled` — the one a switch in the panel +/// has to be written back to. +/// +/// Matched on the whole mapping, since a live forward carries no reference to +/// the rule it came from. A saved rule always names its port, so the port the +/// forward bound is the one the rule asked for. A forward added in the panel +/// matches nothing and stays as temporary as it always was. +pub(crate) fn saved_rule_index( + rules: &[ForwardRule], + forward: &ManagedForward, + enabled: bool, +) -> Option { + rules.iter().position(|r| { + let saved = crate::ui::ssh_connect::map_forward(r); + saved.enabled != enabled + && saved.kind == forward.kind + && saved.bind_host == forward.bind_host + && saved.bind_port == forward.bind_port + && saved.target_host == forward.target_host + && saved.target_port == forward.target_port + }) +} + impl Tty7App { pub(crate) fn render_ssh_status_strip( &self, @@ -254,18 +282,23 @@ impl Tty7App { SshForwardKind::Remote => "R", SshForwardKind::Dynamic => "D", }; - let errored = matches!(forward.status, ForwardStatus::Error(_)); + // A switched-off forward has an error status only because it binds + // nothing; it is drawn faded, not red. + let off = !forward.enabled; + let errored = !off && matches!(forward.status, ForwardStatus::Error(_)); let bind = if matches!(forward.bind_host.as_str(), "127.0.0.1" | "localhost" | "") { forward.bind_port.to_string() } else { format!("{}:{}", forward.bind_host, forward.bind_port) }; + let mapping = match forward.kind { + SshForwardKind::Dynamic => "SOCKS".to_string(), + _ => format!("→ {}:{}", forward.target_host, forward.target_port), + }; let tail = match &forward.status { + _ if off => mapping, ForwardStatus::Error(msg) => msg.clone(), - ForwardStatus::Listening => match forward.kind { - SshForwardKind::Dynamic => "SOCKS".to_string(), - _ => format!("→ {}:{}", forward.target_host, forward.target_port), - }, + ForwardStatus::Listening => mapping, }; let pane_id = forward.pane_id; let forward_id = forward.id; @@ -295,6 +328,7 @@ impl Tty7App { .flex_1() .min_w_0() .gap(px(1.)) + .when(off, |col| col.opacity(0.5)) .child( h_flex() .items_center() @@ -326,6 +360,33 @@ impl Tty7App { ) }), ) + // Always shown, unlike Remove beside it: whether a forward is on is + // state to read at a glance, not an action to go looking for. + .child( + div() + .flex_shrink_0() + .on_mouse_down(gpui::MouseButton::Left, |_, _, cx| cx.stop_propagation()) + .child( + crate::ui::theme::switch( + ("panel-forward-enabled", forward_id as usize), + cx, + ) + .checked(forward.enabled) + .xsmall() + .tooltip(if off { + t(L10nKey::ForwardTooltipTurnOn) + } else { + t(L10nKey::ForwardTooltipTurnOff) + }) + .on_click(cx.listener( + move |this, on: &bool, window, cx| { + this.set_managed_forward_enabled( + pane_id, forward_id, *on, window, cx, + ) + }, + )), + ), + ) .child( div() .flex_shrink_0() @@ -586,6 +647,7 @@ mod tests { target_port: 80, description: Some("the staging box".to_string()), status: ForwardStatus::Listening, + enabled: true, } } @@ -718,4 +780,43 @@ mod tests { "nothing was added, so there is nothing to point at" ); } + + #[test] + fn a_switched_off_forward_keeps_its_switch_through_an_edit() { + let mut off = managed(3, 8080); + off.enabled = false; + assert!(!rule_of(&off).enabled); + assert!(rule_of(&managed(3, 8080)).enabled); + } + + /// The panel's switch is written back to the saved host only for the rule + /// the forward was opened from, and a rule added in the panel — which no + /// saved rule describes — is left as temporary as it always was. + #[test] + fn a_switch_is_written_back_to_the_saved_rule_it_came_from() { + use crate::core::ssh_profile::{ForwardKind, HostPort}; + let saved = |target: &str, enabled: bool| ForwardRule { + kind: ForwardKind::Local, + bind: HostPort::new("127.0.0.1", 8080), + target: HostPort::new(target, 80), + description: "the staging box".to_string(), + enabled, + }; + let rules = vec![saved("10.0.0.9", true), saved("10.0.0.5", true)]; + let live = managed(3, 8080); + assert_eq!(saved_rule_index(&rules, &live, false), Some(1)); + assert_eq!( + saved_rule_index(&rules, &live, true), + None, + "already on: nothing to write" + ); + + let mut elsewhere = managed(3, 8081); + elsewhere.description = None; + assert_eq!(saved_rule_index(&rules, &elsewhere, false), None); + + let mut remote = managed(3, 8080); + remote.kind = SshForwardKind::Remote; + assert_eq!(saved_rule_index(&rules, &remote, false), None); + } } diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index d0e8621a..183dac4c 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -1022,6 +1022,10 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::SshEditProfile => "Edit connection…", L10nKey::ForwardTooltipAdd => "Add forward", L10nKey::ForwardTooltipRemove => "Remove", + L10nKey::ForwardTooltipTurnOn => "Turn on", + L10nKey::ForwardTooltipTurnOff => "Turn off — the rule is kept", + L10nKey::ForwardSwitchFailed => "Couldn't switch the forward — {error}", + L10nKey::SettingsFwdEnabled => "Open this rule with the connection", L10nKey::ForwardLocal => "Local", L10nKey::ForwardRemote => "Remote", L10nKey::ForwardDynamic => "Dynamic", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index 56470abf..82cf92c1 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -1088,6 +1088,10 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::SshEditProfile => "接続を編集…", L10nKey::ForwardTooltipAdd => "フォワードを追加", L10nKey::ForwardTooltipRemove => "削除", + L10nKey::ForwardTooltipTurnOn => "オンにする", + L10nKey::ForwardTooltipTurnOff => "オフにする — ルールは残ります", + L10nKey::ForwardSwitchFailed => "フォワードを切り替えられませんでした — {error}", + L10nKey::SettingsFwdEnabled => "接続時にこのルールを開く", L10nKey::ForwardLocal => "ローカル", L10nKey::ForwardRemote => "リモート", L10nKey::ForwardDynamic => "ダイナミック", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index c3b97bb1..b6325c4f 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -739,6 +739,10 @@ l10n_keys! { SshEditProfile, ForwardTooltipAdd, ForwardTooltipRemove, + ForwardTooltipTurnOn, + ForwardTooltipTurnOff, + ForwardSwitchFailed, + SettingsFwdEnabled, ForwardLocal, ForwardRemote, ForwardDynamic, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index 82d96fb2..1448659e 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -977,6 +977,10 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::SshEditProfile => "编辑连接…", L10nKey::ForwardTooltipAdd => "添加转发", L10nKey::ForwardTooltipRemove => "移除", + L10nKey::ForwardTooltipTurnOn => "开启", + L10nKey::ForwardTooltipTurnOff => "关闭——规则会保留", + L10nKey::ForwardSwitchFailed => "无法切换这条转发——{error}", + L10nKey::SettingsFwdEnabled => "连接时开启这条规则", L10nKey::ForwardLocal => "本地", L10nKey::ForwardRemote => "远程", L10nKey::ForwardDynamic => "动态", diff --git a/src/ui/right_panel.rs b/src/ui/right_panel.rs index 994f83c0..b3f86f98 100644 --- a/src/ui/right_panel.rs +++ b/src/ui/right_panel.rs @@ -112,8 +112,11 @@ pub(crate) const ROW_INSET: f32 = 6.; /// Local forwards only, and only those aimed at the far host's own loopback: /// a forward to some third machine happens to carry the same number, and /// pairing it with the port row would claim it leads somewhere it does not. +/// Nor does a switched-off forward: it leads nowhere until it is switched back +/// on, and its own row is where that switch is. pub(crate) fn forwards_port(m: &ManagedForward, port: u16) -> bool { - m.kind == crate::daemon::protocol::SshForwardKind::Local + m.enabled + && m.kind == crate::daemon::protocol::SshForwardKind::Local && m.target_port == port && crate::daemon::protocol::PortEntry::reaches_loopback(&m.target_host) } @@ -1339,8 +1342,26 @@ impl Tty7App { }), ); if let Some(f) = forward { - tiles_wide += 1; + tiles_wide += 2; let forward_id = f.id; + // Switched off, the forward stops speaking for this row and + // comes out as a row of its own with the switch to turn it + // back on. + actions = actions.child( + crate::ui::tab_strip::chrome_tile_sized( + Button::new(("panel-port-switch-off", i)) + .icon(Icon::empty().path("icons/power.svg")), + TILE_SIZE_XS, + TILE_GLYPH_XS, + false, + cx, + ) + .rounded(px(4.)) + .tooltip(t(L10nKey::ForwardTooltipTurnOff)) + .on_click(cx.listener(move |this, _, window, cx| { + this.set_managed_forward_enabled(pane_id, forward_id, false, window, cx) + })), + ); actions = actions.child( self.info_tile( ("panel-port-unforward", i), @@ -1956,6 +1977,7 @@ mod tests { target_port, description: None, status: ForwardStatus::Listening, + enabled: true, } } @@ -1987,6 +2009,15 @@ mod tests { ); } + /// A switched-off forward reaches nothing, so it must not take over the + /// port row — its own row is the only place its switch is. + #[test] + fn a_switched_off_forward_is_not_the_way_to_a_port() { + let mut off = forward(SshForwardKind::Local, "localhost", 3000); + off.enabled = false; + assert!(!forwards_port(&off, 3000)); + } + fn diff(added: u32, removed: u32, open: bool) -> InfoRow { InfoRow { label: "changes", diff --git a/src/ui/settings.rs b/src/ui/settings.rs index 6e66e41e..ff8bd623 100644 --- a/src/ui/settings.rs +++ b/src/ui/settings.rs @@ -1554,6 +1554,7 @@ pub(crate) struct ForwardRuleForm { pub(crate) target_host: Entity, pub(crate) target_port: Entity, pub(crate) description: Entity, + pub(crate) enabled: bool, } impl ForwardRuleForm { @@ -1576,6 +1577,7 @@ impl ForwardRuleForm { bind, target, description: val(&self.description), + enabled: self.enabled, }) } @@ -2019,6 +2021,7 @@ fn seed_forward_row( &rule.description, t(L10nKey::ForwardDescriptionPlaceholder), ), + enabled: rule.enabled, } } @@ -6227,7 +6230,7 @@ impl Tty7App { let count = form .forwards .iter() - .filter(|r| r.collect(cx).is_some()) + .filter(|r| r.enabled && r.collect(cx).is_some()) .count(); let summary = match count { 0 => t(L10nKey::SettingsNoneSummary).to_string(), @@ -6372,6 +6375,20 @@ impl Tty7App { .flex_1() .min_w(px(80.)) .child(Input::new(&row.description).xsmall()); + let enabled = div().flex_shrink_0().child( + crate::ui::theme::switch(("ssh-fwd-enabled", idx), cx) + .checked(row.enabled) + .xsmall() + .tooltip(t(L10nKey::SettingsFwdEnabled)) + .on_click(cx.listener(move |this, on: &bool, _w, cx| { + if let Some(f) = this.ssh_form_mut() + && let Some(r) = f.forwards.get_mut(idx) + { + r.enabled = *on; + cx.notify(); + } + })), + ); let remove = crate::ui::tab_strip::hit_target( Button::new(("ssh-fwd-remove", idx)) .icon(Icon::new(IconName::Close)) @@ -6388,6 +6405,7 @@ impl Tty7App { h_flex() .gap_2() .items_center() + .child(enabled) .child(kind_switch) .child(description) .child(remove), @@ -6396,9 +6414,15 @@ impl Tty7App { true => mapping(v_flex().gap_1().items_start()), false => mapping(h_flex().gap_2().items_center()), }), - false => mapping(h_flex().gap_2().items_center().child(kind_switch)) - .child(description) - .child(remove), + false => mapping( + h_flex() + .gap_2() + .items_center() + .child(enabled) + .child(kind_switch), + ) + .child(description) + .child(remove), }; v_flex() diff --git a/src/ui/ssh_connect.rs b/src/ui/ssh_connect.rs index c780c7bd..90711b3a 100644 --- a/src/ui/ssh_connect.rs +++ b/src/ui/ssh_connect.rs @@ -574,6 +574,7 @@ fn unmap_forward(rule: &SshForwardRule) -> ForwardRule { bind: HostPort::new(rule.bind_host.clone(), rule.bind_port), target: HostPort::new(rule.target_host.clone(), rule.target_port), description: rule.description.clone().unwrap_or_default(), + enabled: rule.enabled, } } @@ -617,7 +618,7 @@ fn map_proxy(profile: &SshProfile) -> SshProxy { SshProxy::None } -fn map_forward(rule: &ForwardRule) -> SshForwardRule { +pub(crate) fn map_forward(rule: &ForwardRule) -> SshForwardRule { SshForwardRule { kind: match rule.kind { ForwardKind::Local => SshForwardKind::Local, @@ -629,6 +630,7 @@ fn map_forward(rule: &ForwardRule) -> SshForwardRule { target_host: rule.target.host.clone(), target_port: rule.target.port, description: (!rule.description.is_empty()).then(|| rule.description.clone()), + enabled: rule.enabled, } } @@ -823,7 +825,16 @@ mod tests { bind: HostPort::new("localhost", 8080), target: HostPort::new("127.0.0.1", 80), description: "web".into(), + enabled: true, }]; + // A switched-off rule survives the trip through a live spec switched + // off, or reopening a host from its connection would turn it back on. + p.forwards.push(ForwardRule { + bind: HostPort::new("localhost", 8080), + target: HostPort::new("10.0.0.7", 80), + enabled: false, + ..ForwardRule::default() + }); let spec = build_native_ssh_spec(&p, &[], &store, true); let back = profile_from_live_spec(&spec);