diff --git a/docs/remote/ssh.mdx b/docs/remote/ssh.mdx index a565b6fa..8c84afa0 100644 --- a/docs/remote/ssh.mdx +++ b/docs/remote/ssh.mdx @@ -71,7 +71,9 @@ Right-clicking an SSH tab opens that connection's host form — **Edit Host…** for a saved one, **Save as SSH Host…** for an address typed by hand. It is the same row the workspace switcher's machine menu carries, so a hostname or password typed wrong is corrected from the tab you noticed it on. The menu acts -on the tab it was opened on, not on whichever pane is focused. +on the tab it was opened on, not on whichever pane is focused. Saving one opens +on the whole live connection — its proxy, keys and forwards as well as its +address — so the host that lands is the one you were already on. Passwords and key passphrases go in the **OS keychain**, never in `config.json` and never on disk in plain text. **Forget Password** in a diff --git a/src/terminal/view.rs b/src/terminal/view.rs index f6c0b300..7a55ec51 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -8892,14 +8892,25 @@ pub(crate) fn quiet_test_ssh_pane_of( profile_id: Option, window: &mut Window, cx: &mut gpui::App, +) -> (gpui::Entity, crate::daemon::transport::Stream) { + let mut spec: crate::daemon::protocol::NativeSshSpec = + serde_json::from_str(r#"{"host":"build-box","port":22,"user":"me","auth_mode":"auto"}"#) + .expect("a minimal NativeSshSpec decodes"); + spec.profile_id = profile_id.map(|id| id.to_string()); + quiet_test_ssh_pane_with(pane_id, spec, window, cx) +} + +/// The same again, over a spec the caller shaped — for everything a live +/// connection carries beyond its address. +#[cfg(test)] +pub(crate) fn quiet_test_ssh_pane_with( + pane_id: u64, + spec: crate::daemon::protocol::NativeSshSpec, + window: &mut Window, + cx: &mut gpui::App, ) -> (gpui::Entity, crate::daemon::transport::Stream) { let (view, stream) = quiet_test_pane(pane_id, window, cx); view.update(cx, |view, _| { - let mut spec: crate::daemon::protocol::NativeSshSpec = serde_json::from_str( - r#"{"host":"build-box","port":22,"user":"me","auth_mode":"auto"}"#, - ) - .expect("a minimal NativeSshSpec decodes"); - spec.profile_id = profile_id.map(|id| id.to_string()); view.ssh_spec = Some(Box::new(spec)); }); (view, stream) diff --git a/src/ui/ssh_connect.rs b/src/ui/ssh_connect.rs index 73719f92..167543fe 100644 --- a/src/ui/ssh_connect.rs +++ b/src/ui/ssh_connect.rs @@ -186,7 +186,18 @@ impl Tty7App { let Some(spec) = self.unsaved_ssh_session(window, cx) else { return; }; - let profile = profile_from_live_spec(&spec); + self.save_ssh_spec_as_host(&spec, window, cx); + } + + /// The same form, for a connection named by the caller rather than by the + /// focus — the tab menu's row offers it for the tab it was opened on. + pub(crate) fn save_ssh_spec_as_host( + &mut self, + spec: &NativeSshSpec, + window: &mut gpui::Window, + cx: &mut gpui::Context, + ) { + let profile = profile_from_live_spec(spec); let jumped = spec.jump.is_some(); self.open_settings_section(crate::ui::settings::SettingsSection::Ssh, window, cx); self.ssh_form_load(&profile, window, cx); @@ -261,12 +272,33 @@ impl Tty7App { index: usize, window: &gpui::Window, cx: &gpui::App, - ) -> Option<(crate::core::session::RemoteTarget, &'static str)> { + ) -> Option<(TabHostForm, &'static str)> { let leaf = self.tabs.get(index)?.pane.focused_or_first(window, cx)?; let spec = leaf.read(cx).ssh_spec()?; let target = ssh_host_target_of_spec(&spec, &cx.global::().ssh_profiles); let label = crate::ui::switcher::host_form_label(&target)?; - Some((target, label)) + let form = match target { + crate::core::session::RemoteTarget::Profile { .. } => TabHostForm::Saved(target), + _ => TabHostForm::Unsaved(spec), + }; + Some((form, label)) + } + + /// Open what the row offered. A saved host goes to its own record; an + /// unsaved one goes through the same "save this connection" path the + /// command already uses, so the proxy, the identity files and the forwards + /// the session was dialled with land in the draft rather than being + /// thrown away with everything that does not fit in `user@host:port`. + pub(crate) fn open_tab_ssh_host_form( + &mut self, + form: &TabHostForm, + window: &mut gpui::Window, + cx: &mut gpui::Context, + ) { + match form { + TabHostForm::Saved(target) => self.edit_ssh_host_of_target(target, window, cx), + TabHostForm::Unsaved(spec) => self.save_ssh_spec_as_host(spec, window, cx), + } } fn bump_ssh_frecency(&mut self, profile_id: uuid::Uuid, cx: &mut gpui::Context) { @@ -434,13 +466,33 @@ fn build_spec_inner( } } +/// What a tab's host row opens when it is taken. +/// +/// The two halves are not the same form. A saved host is already a record, so +/// it is addressed by the target that names it and nothing about the live +/// session is needed. An unsaved one is only ever the session, and it goes to +/// the form whole: an address dialled by hand carries a proxy, a jump host, +/// identity files and forwards, and a draft built from `user@host:port` alone +/// would save fine and then not connect. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum TabHostForm { + Saved(crate::core::session::RemoteTarget), + Unsaved(Box), +} + /// Which host form a live connection belongs to: the saved host it was opened /// from, or the address it was dialled by. /// /// A transient profile is handed a fresh uuid on its way to the daemon, so an /// id alone does not mean a host was saved — only one that still resolves -/// against the saved list does. Anything else is an address worth keeping, and -/// [`Tty7App::edit_ssh_host_of_target`] opens a new host prefilled from it. +/// against the saved list does. Anything else is an address worth keeping. +/// +/// The `Direct` this hands back is the gate and the label, not the draft: +/// [`host_form_label`] reads it to decide the row exists and what it says, +/// while the form itself opens on the whole live spec, which carries far more +/// than an address does. +/// +/// [`host_form_label`]: crate::ui::switcher::host_form_label pub(crate) fn ssh_host_target_of_spec( spec: &NativeSshSpec, profiles: &[SshProfile], diff --git a/src/ui/tab_strip.rs b/src/ui/tab_strip.rs index 9e17e163..33982dd7 100644 --- a/src/ui/tab_strip.rs +++ b/src/ui/tab_strip.rs @@ -1433,13 +1433,13 @@ impl Tty7App { // is the switcher machine menu's, word for word: the saved host when // there is one, an offer to keep the address when it was dialled by // hand, and nothing at all for a tab with no host form behind it. - if let Some((target, label)) = this.tab_ssh_host_form(index, window, cx) { + if let Some((form, label)) = this.tab_ssh_host_form(index, window, cx) { menu = menu.separator().item(PopupMenuItem::new(label).on_click({ let app = app.clone(); move |_, window, cx| { - let target = target.clone(); + let form = form.clone(); let _ = app.update(cx, |this, cx| { - this.edit_ssh_host_of_target(&target, window, cx) + this.open_tab_ssh_host_form(&form, window, cx) }); } })); @@ -1939,10 +1939,14 @@ mod ssh_host_row_tests { use crate::core::config::Config; use crate::core::session::RemoteTarget; use crate::core::ssh_profile::SshProfile; - use crate::terminal::view::{quiet_test_pane, quiet_test_ssh_pane, quiet_test_ssh_pane_of}; + use crate::daemon::protocol::SshProxy; + use crate::terminal::view::{ + quiet_test_pane, quiet_test_ssh_pane, quiet_test_ssh_pane_of, quiet_test_ssh_pane_with, + }; use crate::ui::app::{Tab, test_window::harness}; use crate::ui::i18n::{L10nKey, set_locale, t}; use crate::ui::pane::{Pane, PaneSlot}; + use crate::ui::ssh_connect::TabHostForm; use gpui::TestAppContext; #[gpui::test] @@ -1984,19 +1988,29 @@ mod ssh_host_row_tests { ); // An address typed by hand is worth keeping, not editing: there is - // no saved host behind it yet. - let (target, label) = app + // no saved host behind it yet, so the live session itself is what + // the form opens on. + let (form, label) = app .tab_ssh_host_form(1, window, cx) .expect("a tab dialled by hand offers to save the host"); - assert_eq!(target, RemoteTarget::direct("me", "build-box", 22)); + let TabHostForm::Unsaved(spec) = form else { + panic!("a hand-dialled tab must offer its own session, not a bare address"); + }; + assert_eq!( + (spec.user.as_str(), spec.host.as_str(), spec.port), + ("me", "build-box", 22) + ); assert_eq!(label, t(L10nKey::SwitcherSaveAsHost)); // One opened from a saved host edits that host — by its id, so the // form lands on the record the connection actually came from. - let (target, label) = app + let (form, label) = app .tab_ssh_host_form(2, window, cx) .expect("a tab on a saved host offers to edit it"); - assert_eq!(target, RemoteTarget::Profile { id: saved }); + assert_eq!( + form, + TabHostForm::Saved(RemoteTarget::Profile { id: saved }) + ); assert_eq!(label, t(L10nKey::SwitcherEditHost)); }); } @@ -2018,13 +2032,67 @@ mod ssh_host_row_tests { vcx.background_executor.run_until_parked(); app.update_in(&mut vcx, |app, window, cx| { - let (target, label) = app + let (form, label) = app .tab_ssh_host_form(0, window, cx) .expect("an unresolvable profile id still names a host worth keeping"); - assert_eq!(target, RemoteTarget::direct("me", "build-box", 22)); + let TabHostForm::Unsaved(spec) = form else { + panic!("a dangling profile id must not open a form on a host that is gone"); + }; + assert_eq!( + (spec.user.as_str(), spec.host.as_str(), spec.port), + ("me", "build-box", 22) + ); assert_eq!(label, t(L10nKey::SwitcherSaveAsHost)); }); } + + /// The row says "Save as SSH Host", and a host saved without the proxy it + /// was reached through is a host that will not connect. What the session + /// was dialled with has to reach the form whole — an address is only the + /// part of it that fits in `user@host:port`. + #[gpui::test] + fn saving_a_hand_dialled_tab_keeps_what_it_was_dialled_with(cx: &mut TestAppContext) { + set_locale("en"); + let (app, mut vcx) = harness(cx); + let _end = app.update_in(&mut vcx, |app, window, cx| { + let mut spec: crate::daemon::protocol::NativeSshSpec = serde_json::from_str( + r#"{"host":"build-box","port":2222,"user":"me","auth_mode":"auto"}"#, + ) + .expect("a minimal NativeSshSpec decodes"); + spec.proxy = SshProxy::Socks { + host: "127.0.0.1".to_string(), + port: 1080, + }; + spec.identity_files = vec!["/keys/id_ed25519".to_string()]; + spec.login_script = vec!["tmux attach".to_string()]; + let (view, end) = quiet_test_ssh_pane_with(1, spec, window, cx); + app.tabs.push(Tab::new(Pane::leaf(PaneSlot::Ready(view)))); + app.active = 0; + cx.notify(); + end + }); + vcx.background_executor.run_until_parked(); + + app.update_in(&mut vcx, |app, window, cx| { + let (form, _) = app + .tab_ssh_host_form(0, window, cx) + .expect("a hand-dialled tab offers to save the host"); + let TabHostForm::Unsaved(spec) = form else { + panic!("nothing here is saved, so nothing here is an edit"); + }; + assert_eq!( + spec.proxy, + SshProxy::Socks { + host: "127.0.0.1".to_string(), + port: 1080, + }, + "the proxy the session was reached through was dropped on the way to the form" + ); + assert_eq!(spec.identity_files, vec!["/keys/id_ed25519".to_string()]); + assert_eq!(spec.login_script, vec!["tmux attach".to_string()]); + assert_eq!(spec.port, 2222, "a non-default port is part of the address"); + }); + } } #[cfg(test)]