From ea550eced4e8e60f2fa6d103bf1b6c21c5e6940f Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Fri, 9 Oct 2026 09:40:55 +0400 Subject: [PATCH 01/15] fix(core): report when failed key setup may leave password login off --- crates/omnyssh-core/src/ssh/key_setup.rs | 220 ++++++++++++++++++++--- 1 file changed, 194 insertions(+), 26 deletions(-) diff --git a/crates/omnyssh-core/src/ssh/key_setup.rs b/crates/omnyssh-core/src/ssh/key_setup.rs index 9a24ed33..6069d592 100644 --- a/crates/omnyssh-core/src/ssh/key_setup.rs +++ b/crates/omnyssh-core/src/ssh/key_setup.rs @@ -125,6 +125,9 @@ pub struct KeySetupMachine { current_step: Option, has_sudo: bool, password_disabled: bool, + /// The step-4 command was sent and has not answered. Kept on the machine + /// so it survives the total timeout dropping the running future. + disable_unanswered: bool, } impl KeySetupMachine { @@ -135,6 +138,7 @@ impl KeySetupMachine { current_step: None, has_sudo: true, password_disabled: false, + disable_unanswered: false, } } @@ -208,6 +212,35 @@ impl KeySetupMachine { self.state = KeySetupState::RolledBack; self.password_disabled = false; } + + /// Records that the step-4 command is about to run. + fn disable_sent(&mut self) { + self.disable_unanswered = true; + } + + /// Records that the step-4 command answered, so its effect is known. + fn disable_answered(&mut self) { + self.disable_unanswered = false; + } + + /// Fails the run on the total timeout. + fn timed_out(&mut self) { + // Running out of time is only safe before the point of no return. + // Past it the server has password auth disabled, so the run needs + // the same rollback a failed final check would get. + let step = if self.password_disabled { + KeySetupStep::FinalCheck + } else { + KeySetupStep::VerifyKeyAuth + }; + self.step_result(step, Err(anyhow!("timed out"))); + } + + /// Whether a failed run, once any rollback was tried, may have left + /// password login off: the rollback failed, or step 4 never answered. + fn password_may_be_off(&self) -> bool { + self.disable_unanswered || self.state == KeySetupState::NeedsRollback + } } impl Default for KeySetupMachine { @@ -488,13 +521,15 @@ pub fn build_reload_sshd_command() -> String { /// Builds the emergency rollback command. /// /// Restores the most recent OmnySSH backup of sshd_config and reloads the daemon. +/// A missing backup exits 0 so its marker reaches the caller: a checked run drops +/// the output of a command that fails. pub fn build_rollback_command() -> String { r#"BACKUP=$(find /etc/ssh -maxdepth 1 -name 'sshd_config.omnyssh_backup.*' 2>/dev/null | sort | tail -1); \ if [ -n "$BACKUP" ]; then \ sudo cp "$BACKUP" /etc/ssh/sshd_config && \ (sudo systemctl reload sshd 2>/dev/null || sudo systemctl reload ssh 2>/dev/null || sudo service sshd reload 2>/dev/null || sudo service ssh reload); \ else \ - echo "OMNYSSH_NO_BACKUP"; exit 1; \ + echo "OMNYSSH_NO_BACKUP"; \ fi"# .to_string() } @@ -517,11 +552,39 @@ pub async fn setup_key_for_host( key_type: KeyType, progress_tx: Option>, ) -> Result { + match run_key_setup(host, password_session, key_type, progress_tx).await { + (result, None) => Ok(result), + (_, Some(error)) => Err(error), + } +} + +/// Runs the same process as [`setup_key_for_host`], but every outcome comes back +/// as the result: a failed run ends `FailedSafe`, `RolledBack` or `NeedsRollback` +/// (the rollback failed), with `error_message` and `password_may_be_off` set. +pub async fn setup_key_for_host_detailed( + host: &Host, + password_session: &SshSession, + key_type: KeyType, + progress_tx: Option>, +) -> KeySetupResult { + run_key_setup(host, password_session, key_type, progress_tx) + .await + .0 +} + +/// Shared runner: the result, plus the error a failed run ends with. +async fn run_key_setup( + host: &Host, + password_session: &SshSession, + key_type: KeyType, + progress_tx: Option>, +) -> (KeySetupResult, Option) { let mut machine = KeySetupMachine::new(); let mut result = KeySetupResult { key_path: PathBuf::new(), state: KeySetupState::NotStarted, error_message: None, + password_may_be_off: false, }; // The two verification steps reconnect, so they walk the host's ProxyJump @@ -538,30 +601,22 @@ pub async fn setup_key_for_host( key_type, verify_timeout, &mut machine, + &mut result.key_path, progress_tx, ), ) .await { - Ok(Ok(key_path)) => { - result.key_path = key_path; + Ok(Ok(())) => { result.state = machine.state().clone(); - return Ok(result); + return (result, None); } Ok(Err(e)) => { error!("Key setup failed for {}: {}", host.name, e); e } Err(_) => { - // Running out of time is only safe before the point of no return. - // Past it the server has password auth disabled, so the run needs - // the same rollback a failed final check would get. - let step = if machine.password_disabled { - KeySetupStep::FinalCheck - } else { - KeySetupStep::VerifyKeyAuth - }; - machine.step_result(step, Err(anyhow!("timed out"))); + machine.timed_out(); anyhow!( "Key setup timed out after {} seconds", total_timeout.as_secs() @@ -585,19 +640,22 @@ pub async fn setup_key_for_host( result.state = KeySetupState::RolledBack; } } + result.password_may_be_off = machine.password_may_be_off(); - Err(error) + (result, Some(error)) } -/// Internal implementation of the key setup process. +/// Internal implementation of the key setup process. `key_path` is filled as soon +/// as step 1 produced the key, so a failed or timed-out run still reports it. async fn setup_key_internal( host: &Host, password_session: &SshSession, key_type: KeyType, verify_timeout: Duration, machine: &mut KeySetupMachine, + key_path: &mut PathBuf, progress_tx: Option>, -) -> Result { +) -> Result<()> { // Step 1: Generate key pair. info!("Step 1/6: Generating key pair for {}", host.name); if let Some(ref tx) = progress_tx { @@ -606,6 +664,7 @@ async fn setup_key_internal( let (private_key_path, public_key_path) = match generate_key_pair(&host.name, key_type).await { Ok(paths) => { machine.step_result(KeySetupStep::GenerateKey, Ok(())); + *key_path = paths.0.clone(); paths } Err(e) => { @@ -699,7 +758,7 @@ async fn setup_key_internal( warn!("No sudo access — password authentication will NOT be disabled"); machine.set_has_sudo(false); machine.step_result(KeySetupStep::VerifyKeyAuth, Ok(())); // Trigger PartialSuccess. - return Ok(private_key_path); + return Ok(()); } } @@ -709,11 +768,17 @@ async fn setup_key_internal( let _ = tx.send(KeySetupStep::DisablePassword).await; } let disable_cmd = build_disable_password_command(); - match time::timeout(STEP_TIMEOUT, password_session.run_command(&disable_cmd)).await { + machine.disable_sent(); + let disabled = time::timeout(STEP_TIMEOUT, password_session.run_command(&disable_cmd)).await; + // Only the command's own output says what it did to the server. + if matches!(disabled, Ok(Ok(_))) { + machine.disable_answered(); + } + match disabled { Ok(Ok(output)) if output.contains("OMNYSSH_NO_SUDO") => { machine.set_has_sudo(false); machine.step_result(KeySetupStep::VerifyKeyAuth, Ok(())); - return Ok(private_key_path); + return Ok(()); } Ok(Ok(output)) if output.contains("OMNYSSH_CONFIG_ERROR") => { machine.step_result(KeySetupStep::DisablePassword, Err(anyhow!("Config error"))); @@ -770,7 +835,7 @@ async fn setup_key_internal( info!("Final verification passed! Key setup complete."); final_session.disconnect().await; machine.step_result(KeySetupStep::FinalCheck, Ok(())); - Ok(private_key_path) + Ok(()) } Ok(Err(e)) => { error!( @@ -802,16 +867,24 @@ async fn emergency_rollback(session: &SshSession) -> Result<()> { warn!("Attempting emergency rollback of sshd_config"); let rollback_cmd = build_rollback_command(); - match time::timeout(STEP_TIMEOUT, session.run_command(&rollback_cmd)).await { - Ok(Ok(output)) if output.contains("OMNYSSH_NO_BACKUP") => { + // Checked: a failed restore or reload must not count as a rollback. + let output = time::timeout(STEP_TIMEOUT, session.run_command_checked(&rollback_cmd)) + .await + .unwrap_or_else(|_| Err(anyhow!("timeout"))); + rollback_outcome(output) +} + +/// Reads the rollback command's checked run. +fn rollback_outcome(output: Result) -> Result<()> { + match output { + Ok(output) if output.contains("OMNYSSH_NO_BACKUP") => { Err(anyhow!("No backup file found for rollback")) } - Ok(Ok(_)) => { + Ok(_) => { info!("Rollback successful — password authentication restored"); Ok(()) } - Ok(Err(e)) => Err(anyhow!("Rollback failed: {}", e)), - Err(_) => Err(anyhow!("Rollback failed: timeout")), + Err(e) => Err(anyhow!("Rollback failed: {}", e)), } } @@ -822,12 +895,16 @@ async fn emergency_rollback(session: &SshSession) -> Result<()> { /// Result of the key setup process. #[derive(Debug, Clone)] pub struct KeySetupResult { - /// Path to the generated private key file. + /// Path to the generated private key file; set once step 1 produced it, so a + /// failed run reports it too (empty when no key was made). pub key_path: PathBuf, /// Final state of the setup process. pub state: KeySetupState, /// Error message if the setup failed. pub error_message: Option, + /// A failed run may have left the server's password login off: the + /// rollback failed, or the step-4 command's outcome is unknown. + pub password_may_be_off: bool, } // --------------------------------------------------------------------------- @@ -948,6 +1025,97 @@ mod tests { assert!(machine.password_disabled); } + #[test] + fn test_unanswered_disable_may_leave_password_off() { + let mut machine = KeySetupMachine::new(); + machine.step_result(KeySetupStep::VerifyKeyAuth, Ok(())); + machine.disable_sent(); + // Transport error or step timeout: the command never answered. + machine.step_result(KeySetupStep::DisablePassword, Err(anyhow!("timeout"))); + + assert_eq!(machine.state(), &KeySetupState::FailedSafe); + assert!(machine.password_may_be_off()); + } + + #[test] + fn test_total_timeout_during_disable_may_leave_password_off() { + let mut machine = KeySetupMachine::new(); + machine.step_result(KeySetupStep::VerifyKeyAuth, Ok(())); + machine.disable_sent(); + machine.timed_out(); + + // No rollback runs, but the server may already have been changed. + assert_eq!(machine.state(), &KeySetupState::FailedSafe); + assert!(machine.password_may_be_off()); + } + + #[test] + fn test_answered_disable_failure_is_known() { + // OMNYSSH_CONFIG_ERROR: the command restored its backup itself. + let mut machine = KeySetupMachine::new(); + machine.disable_sent(); + machine.disable_answered(); + machine.step_result(KeySetupStep::DisablePassword, Err(anyhow!("config error"))); + + assert_eq!(machine.state(), &KeySetupState::FailedSafe); + assert!(!machine.password_may_be_off()); + } + + #[test] + fn test_failure_before_disable_is_known() { + let mut machine = KeySetupMachine::new(); + machine.step_result(KeySetupStep::VerifyKeyAuth, Err(anyhow!("refused"))); + assert!(!machine.password_may_be_off()); + + let mut machine = KeySetupMachine::new(); + machine.timed_out(); + assert_eq!(machine.state(), &KeySetupState::FailedSafe); + assert!(!machine.password_may_be_off()); + } + + #[test] + fn test_rollback_outcome_decides_password_may_be_off() { + let mut machine = KeySetupMachine::new(); + machine.disable_sent(); + machine.disable_answered(); + machine.step_result(KeySetupStep::DisablePassword, Ok(())); + machine.timed_out(); + + // Past step 4 a timeout needs the rollback; until it succeeds the + // password may be off. + assert_eq!(machine.state(), &KeySetupState::NeedsRollback); + assert!(machine.password_may_be_off()); + + machine.rollback_complete(); + assert!(!machine.password_may_be_off()); + } + + #[test] + fn test_failed_rollback_command_is_a_failed_rollback() { + // A non-zero exit from `sudo cp` or the reload. + let failed = rollback_outcome(Err(anyhow!("remote command exited with status 1"))); + assert!(failed + .unwrap_err() + .to_string() + .starts_with("Rollback failed")); + + let none = rollback_outcome(Ok("OMNYSSH_NO_BACKUP\n".to_string())); + assert_eq!( + none.unwrap_err().to_string(), + "No backup file found for rollback" + ); + + assert!(rollback_outcome(Ok(String::new())).is_ok()); + } + + #[test] + fn test_rollback_command_reports_missing_backup_with_exit_zero() { + // A checked run drops the output of a failing command, marker included. + let cmd = build_rollback_command(); + assert!(cmd.contains("echo \"OMNYSSH_NO_BACKUP\";")); + assert!(!cmd.contains("exit 1")); + } + #[test] fn test_authorized_keys_command_escapes_quotes() { let pubkey = "ssh-ed25519 AAAA... user's key"; From ac374ebc9f6533ca74d25567c8c5e704636e81c3 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Fri, 9 Oct 2026 09:40:55 +0400 Subject: [PATCH 02/15] fix(tui): keep the password when key setup leaves password login on --- crates/omnyssh-core/src/event.rs | 6 +++++ crates/omnyssh/src/app/actions.rs | 10 +++++++- crates/omnyssh/src/app/mod.rs | 38 +++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/crates/omnyssh-core/src/event.rs b/crates/omnyssh-core/src/event.rs index b193d472..db0923cc 100644 --- a/crates/omnyssh-core/src/event.rs +++ b/crates/omnyssh-core/src/event.rs @@ -149,6 +149,12 @@ pub enum CoreEvent { KeySetupFailed(HostId, String), /// Emergency rollback was triggered (host_id, rollback_result). KeySetupRollback(HostId, String), + /// Key login works, but password login stayed on: changing the server's + /// settings needs sudo without a password (host_id, private_key_path). + KeySetupPartial(HostId, std::path::PathBuf), + /// Key setup failed and the server's password login may be off: the + /// rollback failed or the disable step never answered (host_id, error_message). + KeySetupFailedUnsafe(HostId, String), /// A private key is encrypted and no passphrase is cached for it yet. /// Frontends prompt once per key path and call [`crate::ssh::identity::unlock`]. diff --git a/crates/omnyssh/src/app/actions.rs b/crates/omnyssh/src/app/actions.rs index 72693eac..aaba9397 100644 --- a/crates/omnyssh/src/app/actions.rs +++ b/crates/omnyssh/src/app/actions.rs @@ -204,7 +204,7 @@ impl App { Ok(result) => { use omnyssh_core::ssh::key_setup::KeySetupState; match result.state { - KeySetupState::Success | KeySetupState::PartialSuccess => { + KeySetupState::Success => { let _ = tx .send(CoreEvent::KeySetupComplete( host_clone.name.clone(), @@ -212,6 +212,14 @@ impl App { )) .await; } + KeySetupState::PartialSuccess => { + let _ = tx + .send(CoreEvent::KeySetupPartial( + host_clone.name.clone(), + result.key_path, + )) + .await; + } KeySetupState::RolledBack => { let msg = result .error_message diff --git a/crates/omnyssh/src/app/mod.rs b/crates/omnyssh/src/app/mod.rs index 2ed6db1f..9aa7ffd2 100644 --- a/crates/omnyssh/src/app/mod.rs +++ b/crates/omnyssh/src/app/mod.rs @@ -845,6 +845,44 @@ impl App { )); } + CoreEvent::KeySetupPartial(host_name, key_path) => { + tracing::info!( + host = %host_name, + key = %key_path.display(), + "key setup partial: password login still on" + ); + + // The key works, but the server still takes the password, so + // it stays saved. + { + let mut state = self.state.write().await; + if let Some(host) = state.hosts.iter_mut().find(|h| h.name == host_name) { + host.identity_file = Some(key_path.to_string_lossy().to_string()); + host.key_setup_date = Some(chrono::Utc::now().to_rfc3339()); + host.password_auth_disabled = Some(false); + } + if let Err(e) = config::save_hosts(&state.hosts) { + tracing::warn!("Failed to save hosts after key setup: {}", e); + } + } + + self.view.host_list.popup = None; + self.view.status_message = Some(format!( + "⚠ Key login works for '{}', but password login is still on (needs sudo without a password). Key: {}", + host_name, + key_path.display() + )); + } + + CoreEvent::KeySetupFailedUnsafe(host_name, error) => { + tracing::error!(host = %host_name, error = %error, "key setup failed, password login may be off"); + self.view.host_list.popup = None; + self.view.status_message = Some(format!( + "✗ Key setup failed for '{}' and password login may be off: {}", + host_name, error + )); + } + CoreEvent::KeySetupFailed(host_name, error) => { tracing::error!(host = %host_name, error = %error, "key setup failed"); // Close popup and show error. From 01a0abc23395792d501fc4032866df9b7d427335 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Fri, 9 Oct 2026 09:50:17 +0400 Subject: [PATCH 03/15] fix(gui): say whether key setup turned password login off --- crates/omnyssh-gui/src/bridge.rs | 121 ++++++++-- crates/omnyssh-gui/src/commands/keysetup.rs | 226 ++++++++++++------ crates/omnyssh-gui/src/events.rs | 14 +- crates/omnyssh-gui/ui/e2e/keysetup.spec.ts | 151 +++++++++--- crates/omnyssh-gui/ui/src/lib/bindings.ts | 16 +- .../omnyssh-gui/ui/src/lib/ipc/router.test.ts | 15 +- crates/omnyssh-gui/ui/src/lib/ipc/router.ts | 4 +- .../ui/src/lib/screens/Dashboard.svelte | 21 +- .../src/lib/screens/KeySetupProgress.svelte | 87 +++++-- .../ui/src/lib/stores/keySetup.test.ts | 28 ++- .../omnyssh-gui/ui/src/lib/stores/keySetup.ts | 19 +- 11 files changed, 520 insertions(+), 182 deletions(-) diff --git a/crates/omnyssh-gui/src/bridge.rs b/crates/omnyssh-gui/src/bridge.rs index 73542655..db9f1f91 100644 --- a/crates/omnyssh-gui/src/bridge.rs +++ b/crates/omnyssh-gui/src/bridge.rs @@ -95,25 +95,15 @@ pub async fn forward_core_events( } // Auto key-setup (§4.2): `start_key_setup` drives the core and reports these // on the shared engine channel; the host name identifies the run. - CoreEvent::KeySetupProgress(host_name, step) => { - let _ = events::KeySetupProgress { - host_name, - step: step.into(), - } - .emit(&app); - } - CoreEvent::KeySetupComplete(host_name, key_path) => { - let _ = events::KeySetupComplete { - host_name, - key_path: key_path.to_string_lossy().to_string(), + event @ (CoreEvent::KeySetupProgress(..) + | CoreEvent::KeySetupComplete(..) + | CoreEvent::KeySetupPartial(..) + | CoreEvent::KeySetupFailed(..) + | CoreEvent::KeySetupFailedUnsafe(..) + | CoreEvent::KeySetupRollback(..)) => { + if let Some(outbound) = map_key_setup_event(event) { + outbound.emit(&app); } - .emit(&app); - } - CoreEvent::KeySetupFailed(host_name, error) => { - let _ = events::KeySetupFailed { host_name, error }.emit(&app); - } - CoreEvent::KeySetupRollback(host_name, result) => { - let _ = events::KeySetupRollback { host_name, result }.emit(&app); } // A newer release found by the startup check (§4.3) → the update banner. CoreEvent::UpdateAvailable(info) => { @@ -130,6 +120,61 @@ pub async fn forward_core_events( } } +/// A typed key-setup IPC event, ready to emit. +#[derive(Debug)] +enum KeySetupOutbound { + Progress(events::KeySetupProgress), + Complete(events::KeySetupComplete), + Failed(events::KeySetupFailed), + Rollback(events::KeySetupRollback), +} + +impl KeySetupOutbound { + fn emit(self, app: &AppHandle) { + let _ = match self { + KeySetupOutbound::Progress(e) => e.emit(app), + KeySetupOutbound::Complete(e) => e.emit(app), + KeySetupOutbound::Failed(e) => e.emit(app), + KeySetupOutbound::Rollback(e) => e.emit(app), + }; + } +} + +/// Map a core key-setup event to its IPC event (§4.3): the partial and unsafe +/// variants share the complete / failed events, told apart by their flag. +fn map_key_setup_event(event: CoreEvent) -> Option { + let complete = |host_name, key_path: std::path::PathBuf, password_off| { + KeySetupOutbound::Complete(events::KeySetupComplete { + host_name, + key_path: key_path.to_string_lossy().to_string(), + password_off, + }) + }; + let failed = |host_name, error, password_may_be_off| { + KeySetupOutbound::Failed(events::KeySetupFailed { + host_name, + error, + password_may_be_off, + }) + }; + Some(match event { + CoreEvent::KeySetupProgress(host_name, step) => { + KeySetupOutbound::Progress(events::KeySetupProgress { + host_name, + step: step.into(), + }) + } + CoreEvent::KeySetupComplete(host_name, key_path) => complete(host_name, key_path, true), + CoreEvent::KeySetupPartial(host_name, key_path) => complete(host_name, key_path, false), + CoreEvent::KeySetupFailed(host_name, error) => failed(host_name, error, false), + CoreEvent::KeySetupFailedUnsafe(host_name, error) => failed(host_name, error, true), + CoreEvent::KeySetupRollback(host_name, result) => { + KeySetupOutbound::Rollback(events::KeySetupRollback { host_name, result }) + } + _ => return None, + }) +} + /// Written under a session's last output when it ends: dim, then the cursor hidden /// and any mouse reporting the dead app left on turned off, so a drag selects text. const END_LINE: &[u8] = b"\r\n\x1b[2m[Connection closed. Press Enter to close this tab.]\x1b[0m\ @@ -546,4 +591,44 @@ mod tests { // forwarder robust and the match exhaustive without inventing an event (§3.4). assert!(map_sftp_event(1, CoreEvent::PtyOutput(3)).is_none()); } + + /// The wire payload of a key-setup event as JSON. + fn key_setup_json(event: CoreEvent) -> serde_json::Value { + match map_key_setup_event(event).expect("a key-setup event") { + KeySetupOutbound::Progress(e) => serde_json::to_value(e), + KeySetupOutbound::Complete(e) => serde_json::to_value(e), + KeySetupOutbound::Failed(e) => serde_json::to_value(e), + KeySetupOutbound::Rollback(e) => serde_json::to_value(e), + } + .expect("serialise") + } + + #[test] + fn key_setup_outcomes_carry_the_password_state() { + let key = std::path::PathBuf::from("/k/omnyssh_web_ed25519"); + assert_eq!( + key_setup_json(CoreEvent::KeySetupComplete("web".into(), key.clone())), + serde_json::json!({ + "hostName": "web", "keyPath": "/k/omnyssh_web_ed25519", "passwordOff": true + }) + ); + assert_eq!( + key_setup_json(CoreEvent::KeySetupPartial("web".into(), key))["passwordOff"], + false + ); + assert_eq!( + key_setup_json(CoreEvent::KeySetupFailed("web".into(), "boom".into())), + serde_json::json!({ "hostName": "web", "error": "boom", "passwordMayBeOff": false }) + ); + assert_eq!( + key_setup_json(CoreEvent::KeySetupFailedUnsafe("web".into(), "boom".into())) + ["passwordMayBeOff"], + true + ); + assert_eq!( + key_setup_json(CoreEvent::KeySetupRollback("web".into(), "restored".into())), + serde_json::json!({ "hostName": "web", "result": "restored" }) + ); + assert!(map_key_setup_event(CoreEvent::Error("x".into())).is_none()); + } } diff --git a/crates/omnyssh-gui/src/commands/keysetup.rs b/crates/omnyssh-gui/src/commands/keysetup.rs index 01923f88..a50e3f0d 100644 --- a/crates/omnyssh-gui/src/commands/keysetup.rs +++ b/crates/omnyssh-gui/src/commands/keysetup.rs @@ -1,10 +1,13 @@ //! Auto SSH key-setup command (tech-gui.md §4.2). `start_key_setup` mirrors the TUI's //! orchestration (crates/omnyssh `ConfirmKeySetup`): connect a password session, drive -//! `setup_key_for_host`, and report progress + the terminal outcome as +//! `setup_key_for_host_detailed`, and report progress + the terminal outcome as //! `CoreEvent::KeySetup*` on the shared engine channel — the bridge maps those to -//! `key-setup-*` IPC events (§3.4). On success it writes the generated key back onto the +//! `key-setup-*` IPC events (§3.4). Once the key works it writes it back onto the //! manual host in `hosts.toml`, the same completion write the TUI performs, so a -//! `reload_hosts` afterwards refreshes `hasKey`/`passwordAuthDisabled` on the card. +//! `reload_hosts` afterwards refreshes `hasKey`/`passwordAuthDisabled` on the card. A +//! failure that may have left password login off saves the tested key alone. + +use std::path::Path; use tauri::{AppHandle, Manager, State}; use tokio::sync::mpsc; @@ -12,7 +15,9 @@ use tokio::sync::mpsc; use omnyssh_core::config::{load_hosts, save_hosts}; use omnyssh_core::event::CoreEvent; use omnyssh_core::ssh::client::Host; -use omnyssh_core::ssh::key_setup::{setup_key_for_host, KeySetupState, KeySetupStep, KeyType}; +use omnyssh_core::ssh::key_setup::{ + setup_key_for_host_detailed, KeySetupResult, KeySetupState, KeySetupStep, KeyType, +}; use omnyssh_core::ssh::session::SshSession; use crate::error::CommandError; @@ -52,8 +57,9 @@ impl Drop for KeySetupSlot { } /// The background flow: forward each step as `KeySetupProgress`, connect with the -/// password, run `setup_key_for_host`, then map the final state to exactly one terminal -/// event (`KeySetupComplete` / `KeySetupRollback` / `KeySetupFailed`). +/// password, run `setup_key_for_host_detailed`, then map the final state to exactly one +/// terminal event (`KeySetupComplete` / `KeySetupPartial` / `KeySetupRollback` / +/// `KeySetupFailed` / `KeySetupFailedUnsafe`). async fn run_key_setup(app: AppHandle, host: Host, engine_tx: mpsc::Sender) { // Frees the slot on any return path, including an unwind (see `KeySetupSlot`). let _slot = KeySetupSlot(app); @@ -63,7 +69,7 @@ async fn run_key_setup(app: AppHandle, host: Host, engine_tx: mpsc::Sender(8); let step_tx = engine_tx.clone(); let step_host = host.name.clone(); - tokio::spawn(async move { + let drainer = tokio::spawn(async move { while let Some(step) = progress_rx.recv().await { let _ = step_tx .send(CoreEvent::KeySetupProgress(step_host.clone(), step)) @@ -84,83 +90,161 @@ async fn run_key_setup(app: AppHandle, host: Host, engine_tx: mpsc::Sender - { - let disabled = matches!(result.state, KeySetupState::Success); - persist_key(&host.name, &result.key_path.to_string_lossy(), disabled).await; - let _ = engine_tx - .send(CoreEvent::KeySetupComplete( - host.name.clone(), - result.key_path, - )) - .await; - } - // The two arms below are the correct mapping for a rolled-back / non-terminal - // `Ok`, kept symmetric with the `KeySetupRollback` event the bridge must map - // (§3.4) and mirroring the TUI reference. They are currently unreachable: the - // stable core's `setup_key_for_host` returns `Err` on every rollback/failure path - // and discards the `RolledBack` state (§12), so a rolled-back setup surfaces via - // the `Err` arm as `KeySetupFailed` with the core's message. Distinguishing - // rollback from failure would need the core to surface that outcome — out of scope - // for this slice, not an edit we are authorised to make (§3.6). - Ok(result) if result.state == KeySetupState::RolledBack => { - let reason = result - .error_message - .unwrap_or_else(|| "Rolled back.".to_string()); - let _ = engine_tx - .send(CoreEvent::KeySetupRollback(host.name.clone(), reason)) - .await; - } - Ok(result) => { - let reason = result - .error_message - .unwrap_or_else(|| "Key setup failed.".to_string()); - let _ = engine_tx - .send(CoreEvent::KeySetupFailed(host.name.clone(), reason)) - .await; - } - Err(e) => { - let _ = engine_tx - .send(CoreEvent::KeySetupFailed( - host.name.clone(), - format!("{e:#}"), - )) - .await; + // Persist BEFORE emitting so the frontend's reload sees it. + if let Some(write) = key_write(&result) { + persist_key(&host.name, &result.key_path, write).await; + } + let _ = engine_tx.send(outcome_event(host.name, result)).await; +} + +/// What a finished run writes onto the manual host. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum KeyWrite { + /// The key works: identity, setup date and the password-login state. + Setup { password_disabled: bool }, + /// A failed run that may have turned password login off: only the tested key, so + /// the next connect can still get in. Password and setup state stay as they were. + IdentityOnly, +} + +fn key_write(result: &KeySetupResult) -> Option { + match result.state { + KeySetupState::Success => Some(KeyWrite::Setup { + password_disabled: true, + }), + KeySetupState::PartialSuccess => Some(KeyWrite::Setup { + password_disabled: false, + }), + KeySetupState::RolledBack => None, + // The core only reports may-be-off after the key passed its verify step. + _ if result.password_may_be_off && !result.key_path.as_os_str().is_empty() => { + Some(KeyWrite::IdentityOnly) } + _ => None, } } -/// Write the generated key onto the manual host in `hosts.toml`, mirroring the TUI's -/// completion write: set `identity_file` + `key_setup_date`, and — only when password -/// auth was actually disabled — mark `password_auth_disabled` and drop the stored -/// password (key auth supersedes it). Only manual hosts live in `hosts.toml`, so an -/// SSH-config name is a no-op. Best-effort: the completion event fires regardless, and -/// this is awaited so a following `reload_hosts` observes the write (no read/write race). -async fn persist_key(host_name: &str, key_path: &str, password_disabled: bool) { +/// The one terminal event for a finished run. +fn outcome_event(host_name: String, result: KeySetupResult) -> CoreEvent { + let error = result + .error_message + .unwrap_or_else(|| "Key setup failed.".to_string()); + match result.state { + KeySetupState::Success => CoreEvent::KeySetupComplete(host_name, result.key_path), + KeySetupState::PartialSuccess => CoreEvent::KeySetupPartial(host_name, result.key_path), + KeySetupState::RolledBack => CoreEvent::KeySetupRollback(host_name, error), + _ if result.password_may_be_off => CoreEvent::KeySetupFailedUnsafe(host_name, error), + _ => CoreEvent::KeySetupFailed(host_name, error), + } +} + +/// Write the generated key onto the manual host in `hosts.toml`. `Setup` mirrors the +/// TUI's completion write: set `identity_file` + `key_setup_date` and record whether +/// password auth was disabled; only when it was, drop the stored password (key auth +/// supersedes it, and the server would refuse it anyway). `IdentityOnly` sets just +/// `identity_file`. Only manual hosts live in `hosts.toml`, so an SSH-config name is a +/// no-op. Best-effort: the outcome event fires regardless, and this is awaited so a +/// following `reload_hosts` observes the write (no read/write race). +async fn persist_key(host_name: &str, key_path: &Path, write: KeyWrite) { let name = host_name.to_string(); - let key_path = key_path.to_string(); + let key_path = key_path.to_string_lossy().to_string(); let _ = tauri::async_runtime::spawn_blocking(move || -> Option<()> { let mut hosts = load_hosts().ok()?; let host = hosts.iter_mut().find(|h| h.name == name)?; host.identity_file = Some(key_path); - host.key_setup_date = Some(chrono::Utc::now().to_rfc3339()); - if password_disabled { - host.password_auth_disabled = Some(true); - host.password = None; + if let KeyWrite::Setup { password_disabled } = write { + host.key_setup_date = Some(chrono::Utc::now().to_rfc3339()); + host.password_auth_disabled = Some(password_disabled); + if password_disabled { + host.password = None; + } } save_hosts(&hosts).ok() }) .await; } + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + fn result(state: KeySetupState, password_may_be_off: bool) -> KeySetupResult { + KeySetupResult { + key_path: PathBuf::from("/k/omnyssh_web_ed25519"), + state, + error_message: Some("boom".to_string()), + password_may_be_off, + } + } + + #[test] + fn each_outcome_maps_to_its_terminal_event() { + let ev = |state, off| outcome_event("web".to_string(), result(state, off)); + assert!(matches!( + ev(KeySetupState::Success, false), + CoreEvent::KeySetupComplete(h, p) if h == "web" && p.ends_with("omnyssh_web_ed25519") + )); + assert!(matches!( + ev(KeySetupState::PartialSuccess, false), + CoreEvent::KeySetupPartial(h, _) if h == "web" + )); + assert!(matches!( + ev(KeySetupState::RolledBack, false), + CoreEvent::KeySetupRollback(_, r) if r == "boom" + )); + assert!(matches!( + ev(KeySetupState::FailedSafe, false), + CoreEvent::KeySetupFailed(_, e) if e == "boom" + )); + // A failed rollback, or a disable step that never answered. + assert!(matches!( + ev(KeySetupState::NeedsRollback, true), + CoreEvent::KeySetupFailedUnsafe(_, e) if e == "boom" + )); + assert!(matches!( + ev(KeySetupState::FailedSafe, true), + CoreEvent::KeySetupFailedUnsafe(..) + )); + } + + #[test] + fn each_outcome_writes_what_it_proved() { + let write = |state, off| key_write(&result(state, off)); + assert_eq!( + write(KeySetupState::Success, false), + Some(KeyWrite::Setup { + password_disabled: true + }) + ); + assert_eq!( + write(KeySetupState::PartialSuccess, false), + Some(KeyWrite::Setup { + password_disabled: false + }) + ); + // May be off: save the tested key so "Open terminal" can use it, nothing else. + assert_eq!( + write(KeySetupState::NeedsRollback, true), + Some(KeyWrite::IdentityOnly) + ); + assert_eq!( + write(KeySetupState::FailedSafe, true), + Some(KeyWrite::IdentityOnly) + ); + assert_eq!(write(KeySetupState::FailedSafe, false), None); + assert_eq!(write(KeySetupState::RolledBack, false), None); + + // No key made: nothing to save. + let mut keyless = result(KeySetupState::FailedSafe, true); + keyless.key_path = PathBuf::new(); + assert_eq!(key_write(&keyless), None); + } +} diff --git a/crates/omnyssh-gui/src/events.rs b/crates/omnyssh-gui/src/events.rs index 8ced3da3..fee7fb3b 100644 --- a/crates/omnyssh-gui/src/events.rs +++ b/crates/omnyssh-gui/src/events.rs @@ -145,26 +145,30 @@ pub struct KeySetupProgress { pub step: KeySetupStepDto, } -/// Key setup finished successfully — key auth is configured (tech-gui.md §4.3). -/// `keyPath` is the generated private-key path (a path, never key material, §3.4). +/// Key setup finished — key auth is configured (tech-gui.md §4.3). `keyPath` is the +/// generated private-key path (a path, never key material, §3.4). `passwordOff` is +/// false when the server's password login stayed on (no passwordless sudo). #[derive(Debug, Clone, Serialize, Deserialize, specta::Type, tauri_specta::Event)] #[serde(rename_all = "camelCase")] pub struct KeySetupComplete { pub host_name: String, pub key_path: String, + pub password_off: bool, } -/// Key setup failed before touching the server's auth config (tech-gui.md §4.3). -/// Password authentication is never disabled unless a key was verified first. +/// Key setup failed (tech-gui.md §4.3). `passwordMayBeOff` is true when OmnySSH could +/// not confirm the server's password login is on: the rollback failed, or the disable +/// step never answered. Otherwise password login was not changed, or was restored. #[derive(Debug, Clone, Serialize, Deserialize, specta::Type, tauri_specta::Event)] #[serde(rename_all = "camelCase")] pub struct KeySetupFailed { pub host_name: String, pub error: String, + pub password_may_be_off: bool, } /// Key setup rolled the server's sshd config back after a late failure (tech-gui.md -/// §4.3). `result` is the human-readable rollback outcome. +/// §4.3). `result` is the failure that triggered the rollback. #[derive(Debug, Clone, Serialize, Deserialize, specta::Type, tauri_specta::Event)] #[serde(rename_all = "camelCase")] pub struct KeySetupRollback { diff --git a/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts b/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts index febf1d29..a89a8424 100644 --- a/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts +++ b/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts @@ -2,20 +2,25 @@ import { expect, test, type Page } from '@playwright/test'; // Auto SSH-key setup (tech-gui.md §4.2). e2e runs against the static SPA with Tauri // absent, so we stub `__TAURI_INTERNALS__` at the boundary (§6.4). `start_key_setup` is -// faked: it streams `key-setup-progress` events and then `key-setup-complete`, flipping -// the host's hasKey/passwordAuthDisabled so the follow-up `reload_hosts` replays a keyed -// host — exactly how the real backend drives the panel and refreshes the card. +// faked per outcome: it streams the `key-setup-progress` steps the real run would reach, +// persists what the backend persists (full success: key + password off; partial: key + +// password on; may-be-off failure: the key alone), then emits the terminal event, so the +// follow-up `reload_hosts` replays +// the host as the real backend would leave it. const HOSTS = [ { name: 'pw-host', hostname: 'pw.example.com', user: 'root', port: 22, tags: [], source: 'manual', hasKey: false, localForwards: [], tunnelAutostart: false, forwardAgent: false } ]; -async function boot(page: Page): Promise { +type Outcome = 'success' | 'partial' | 'unsafe' | 'rollback'; + +async function boot(page: Page, outcome: Outcome = 'success'): Promise { await page.addInitScript( - ({ hosts }) => { + ({ hosts, outcome }) => { let cbid = 0; const listeners: Record = {}; const state: { hosts: Array> } = { hosts: hosts.map((h) => ({ ...h })) }; const win = window as unknown as Record; + win.__startCalls = 0; function fire(event: string, payload: unknown): void { for (const id of listeners[event] ?? []) { @@ -24,6 +29,53 @@ async function boot(page: Page): Promise { } } + const LABELS = [ + 'Generating Ed25519 key pair', + 'Copying public key to server', + 'Verifying key authentication', + 'Disabling password authentication', + 'Reloading SSH service', + 'Final verification' + ]; + // The last step each outcome reaches: partial stops after the verify step (no + // passwordless sudo), the unsafe failure in the disable step, and a rollback + // after the final check. + const LAST: Record = { success: 6, partial: 3, unsafe: 4, rollback: 6 }; + + function run(name: string): void { + const last = LAST[outcome]; + for (let i = 1; i <= last; i++) { + setTimeout( + () => fire('key-setup-progress', { hostName: name, step: { index: i, total: 6, description: LABELS[i - 1] } }), + 40 * i + ); + } + setTimeout(() => { + const h = state.hosts.find((x) => (x as { name: string }).name === name); + const keyPath = `/home/me/.ssh/omnyssh_${name}_ed25519`; + if (outcome === 'success' || outcome === 'partial') { + // The real backend persists before emitting complete. + if (h) { + h.hasKey = true; + h.passwordAuthDisabled = outcome === 'success'; + } + fire('key-setup-complete', { hostName: name, keyPath, passwordOff: outcome === 'success' }); + } else if (outcome === 'unsafe') { + if (h) h.hasKey = true; + fire('key-setup-failed', { + hostName: name, + error: 'Failed to disable password authentication: timeout', + passwordMayBeOff: true + }); + } else { + fire('key-setup-rollback', { + hostName: name, + result: 'Final verification failed after disabling password. Attempting rollback.' + }); + } + }, 40 * last + 200); + } + (win as { __TAURI_INTERNALS__: unknown }).__TAURI_INTERNALS__ = { invoke: (cmd: string, args: Record) => { switch (cmd) { @@ -32,24 +84,10 @@ async function boot(page: Page): Promise { case 'reload_hosts': setTimeout(() => fire('hosts-loaded', [...state.hosts]), 0); return Promise.resolve(null); - case 'start_key_setup': { - const name = args.hostName as string; - const step = (index: number, description: string) => - fire('key-setup-progress', { hostName: name, step: { index, total: 6, description } }); - setTimeout(() => step(1, 'Generating Ed25519 key pair'), 40); - setTimeout(() => step(3, 'Verifying key authentication'), 120); - setTimeout(() => { - // The real backend persists the key before emitting complete; mirror - // that so the panel's reload shows a keyed host. - const h = state.hosts.find((x) => (x as { name: string }).name === name); - if (h) { - h.hasKey = true; - h.passwordAuthDisabled = true; - } - fire('key-setup-complete', { hostName: name, keyPath: `/home/me/.ssh/omnyssh_${name}_ed25519` }); - }, 400); + case 'start_key_setup': + win.__startCalls = (win.__startCalls as number) + 1; + run(args.hostName as string); return Promise.resolve(null); - } case 'plugin:event|listen': { const { event, handler } = args as { event: string; handler: number }; (listeners[event] ||= []).push(handler); @@ -66,33 +104,78 @@ async function boot(page: Page): Promise { } }; }, - { hosts: HOSTS } + { hosts: HOSTS, outcome } ); await page.goto('/'); await expect(page.getByText('pw-host', { exact: true })).toBeVisible(); } -test('prompts key setup, streams progress, then reflects key auth on the card', async ({ page }) => { - await boot(page); +const startCalls = (page: Page) => page.evaluate(() => (window as unknown as { __startCalls: number }).__startCalls); - // A password host with no key offers the host-first "Set up key" action (§4.2). +test('a full success says password login is off and the card shows the shield', async ({ page }) => { + await boot(page, 'success'); await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); const dialog = page.getByRole('dialog', { name: 'Key setup' }); await expect(dialog).toBeVisible(); - - // A step streams in while the flow runs. - await expect(dialog.getByText('Verifying key authentication')).toBeVisible(); - - // Completion shows the generated key path. - await expect(dialog.getByText('Key authentication configured')).toBeVisible(); + await expect(dialog.getByText('Key login is set up and password login is off')).toBeVisible(); await expect(dialog.getByText(/omnyssh_pw-host_ed25519/)).toBeVisible(); + await expect(dialog.getByText('The password saved in OmnySSH was removed.')).toBeVisible(); + expect(await startCalls(page)).toBe(1); await dialog.getByRole('button', { name: 'Done' }).click(); await expect(page.getByRole('dialog')).toHaveCount(0); - // The reload flipped hasKey/passwordAuthDisabled: the card now reads key-only and no + // The reload flipped hasKey/passwordAuthDisabled: the card shows the shield and no // longer offers key setup. - await expect(page.getByText('key-only')).toBeVisible(); + await expect(page.getByRole('img', { name: 'Key login only' })).toBeVisible(); await expect(page.getByRole('button', { name: 'Set up an SSH key for pw-host' })).toHaveCount(0); }); + +test('a partial run says password login is still on', async ({ page }) => { + await boot(page, 'partial'); + await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + + const dialog = page.getByRole('dialog', { name: 'Key setup' }); + await expect(dialog.getByText(/Key login works\. Password login is still on/)).toBeVisible(); + await expect(dialog.getByText('Your saved password was kept.')).toBeVisible(); + await expect(dialog.getByText(/password login is off/)).toHaveCount(0); + await dialog.getByRole('button', { name: 'Done' }).click(); + + await expect(page.getByText('Password on')).toBeVisible(); + await expect(page.getByRole('img', { name: 'Key login only' })).toHaveCount(0); +}); + +test('a failure that may leave password login off says so', async ({ page }) => { + await boot(page, 'unsafe'); + await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + + const dialog = page.getByRole('dialog', { name: 'Key setup' }); + await expect(dialog.getByText('Key setup failed — password login may be off')).toBeVisible(); + await expect(dialog.getByText(/off\s+now, or after the SSH service next restarts/)).toBeVisible(); + await expect(dialog.getByText(/The new key was saved for this\s+host/)).toBeVisible(); + await expect(dialog.getByText('sudo sshd -T | grep -i passwordauthentication')).toBeVisible(); + await expect(dialog.getByText(/from the newest/)).toBeVisible(); + await expect(dialog.getByText('Failed to disable password authentication: timeout')).toBeVisible(); + await expect(dialog.getByText('Password login was not changed.')).toHaveCount(0); + await expect(dialog.getByRole('button', { name: 'Open terminal' })).toBeVisible(); + await expect(dialog.getByRole('button', { name: 'Close' })).toBeVisible(); + + // The reload picked up the saved key, so the card stops offering key setup. + await expect(page.getByTitle('Key authentication configured')).toBeVisible(); + await expect(page.getByRole('button', { name: 'Set up an SSH key for pw-host' })).toHaveCount(0); +}); + +test('a rollback says password login was turned back on', async ({ page }) => { + await boot(page, 'rollback'); + await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + + const dialog = page.getByRole('dialog', { name: 'Key setup' }); + await expect(dialog.getByText('Password login was turned back on')).toBeVisible(); + await expect(dialog.getByText(/A step after password login was turned off failed/)).toBeVisible(); + await expect(dialog.getByText(/restored the\s+server's SSH settings from its backup/)).toBeVisible(); + await expect(dialog.getByText(/stopped working/)).toHaveCount(0); + await expect( + dialog.getByText('Cause: Final verification failed after disabling password. Attempting rollback.') + ).toBeVisible(); +}); diff --git a/crates/omnyssh-gui/ui/src/lib/bindings.ts b/crates/omnyssh-gui/ui/src/lib/bindings.ts index 3fab408b..7ec40999 100644 --- a/crates/omnyssh-gui/ui/src/lib/bindings.ts +++ b/crates/omnyssh-gui/ui/src/lib/bindings.ts @@ -572,15 +572,17 @@ export type HostsLoaded = HostDto[] */ export type KeyPassphraseRequired = { hostName: string; keyPath: string } /** - * Key setup finished successfully — key auth is configured (tech-gui.md §4.3). - * `keyPath` is the generated private-key path (a path, never key material, §3.4). + * Key setup finished — key auth is configured (tech-gui.md §4.3). `keyPath` is the + * generated private-key path (a path, never key material, §3.4). `passwordOff` is + * false when the server's password login stayed on (no passwordless sudo). */ -export type KeySetupComplete = { hostName: string; keyPath: string } +export type KeySetupComplete = { hostName: string; keyPath: string; passwordOff: boolean } /** - * Key setup failed before touching the server's auth config (tech-gui.md §4.3). - * Password authentication is never disabled unless a key was verified first. + * Key setup failed (tech-gui.md §4.3). `passwordMayBeOff` is true when OmnySSH could + * not confirm the server's password login is on: the rollback failed, or the disable + * step never answered. Otherwise password login was not changed, or was restored. */ -export type KeySetupFailed = { hostName: string; error: string } +export type KeySetupFailed = { hostName: string; error: string; passwordMayBeOff: boolean } /** * A progress step of an auto key-setup run (tech-gui.md §4.3). Mapped by the shared * engine bridge from `CoreEvent::KeySetupProgress`; the host name identifies the run. @@ -588,7 +590,7 @@ export type KeySetupFailed = { hostName: string; error: string } export type KeySetupProgress = { hostName: string; step: KeySetupStepDto } /** * Key setup rolled the server's sshd config back after a late failure (tech-gui.md - * §4.3). `result` is the human-readable rollback outcome. + * §4.3). `result` is the failure that triggered the rollback. */ export type KeySetupRollback = { hostName: string; result: string } /** diff --git a/crates/omnyssh-gui/ui/src/lib/ipc/router.test.ts b/crates/omnyssh-gui/ui/src/lib/ipc/router.test.ts index d07ff02d..c336706a 100644 --- a/crates/omnyssh-gui/ui/src/lib/ipc/router.test.ts +++ b/crates/omnyssh-gui/ui/src/lib/ipc/router.test.ts @@ -216,18 +216,25 @@ describe('ipc event router', () => { phase: { kind: 'running', step: { index: 3, total: 6, description: 'Verifying key authentication' } } }); - applyKeySetupComplete({ hostName: 'web-1', keyPath: '/k/id_ed25519' }); + applyKeySetupComplete({ hostName: 'web-1', keyPath: '/k/id_ed25519', passwordOff: false }); expect(get(keySetup)).toEqual({ hostName: 'web-1', - phase: { kind: 'complete', keyPath: '/k/id_ed25519' } + phase: { kind: 'complete', keyPath: '/k/id_ed25519', passwordOff: false } }); + + // A step still queued behind the outcome must not reopen the running view. + applyKeySetupProgress({ hostName: 'web-1', step: { index: 6, total: 6, description: 'Final verification' } }); + expect(get(keySetup)?.phase.kind).toBe('complete'); dismissKeySetup(); }); it('a key-setup failure/rollback shows for its host even with no open run', () => { dismissKeySetup(); // nothing open - applyKeySetupFailed({ hostName: 'db-1', error: 'Connection failed' }); - expect(get(keySetup)).toEqual({ hostName: 'db-1', phase: { kind: 'failed', error: 'Connection failed' } }); + applyKeySetupFailed({ hostName: 'db-1', error: 'Connection failed', passwordMayBeOff: false }); + expect(get(keySetup)).toEqual({ + hostName: 'db-1', + phase: { kind: 'failed', error: 'Connection failed', passwordMayBeOff: false } + }); applyKeySetupRollback({ hostName: 'db-1', result: 'Restored.' }); expect(get(keySetup)).toEqual({ hostName: 'db-1', phase: { kind: 'rolledBack', result: 'Restored.' } }); diff --git a/crates/omnyssh-gui/ui/src/lib/ipc/router.ts b/crates/omnyssh-gui/ui/src/lib/ipc/router.ts index db765e2f..d9c4d5cd 100644 --- a/crates/omnyssh-gui/ui/src/lib/ipc/router.ts +++ b/crates/omnyssh-gui/ui/src/lib/ipc/router.ts @@ -157,11 +157,11 @@ export function applyKeySetupProgress(payload: KeySetupProgress): void { } export function applyKeySetupComplete(payload: KeySetupComplete): void { - keySetup.set(reduceComplete(payload.hostName, payload.keyPath)); + keySetup.set(reduceComplete(payload.hostName, payload.keyPath, payload.passwordOff)); } export function applyKeySetupFailed(payload: KeySetupFailed): void { - keySetup.set(reduceFailed(payload.hostName, payload.error)); + keySetup.set(reduceFailed(payload.hostName, payload.error, payload.passwordMayBeOff)); } export function applyKeySetupRollback(payload: KeySetupRollback): void { diff --git a/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte b/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte index 6d618914..19403651 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte @@ -620,15 +620,24 @@ ssh config {/if} - - {#if card.host.passwordAuthDisabled} + + {#if card.host.passwordAuthDisabled === true} - key-only + + {:else if card.host.passwordAuthDisabled === false} + + Password on {:else if card.host.hasKey} // Auto SSH-key setup progress panel (tech-gui.md §4.2). Renders the active run from // the keySetup store: a stepped progress bar while running, then the terminal - // outcome (success / failure / rollback). On the first 'complete' it reloads the - // host list once — the backend already wrote hosts.toml, so the card must refresh to - // reflect the new hasKey / passwordAuthDisabled. Mounted globally (AppShell) so it - // survives navigating away from the Dashboard mid-run. + // outcome (full or partial success / failure / rollback), each saying what happened + // to the server's password login. On the first 'complete', or a failure that may + // have left password login off, it reloads the host list once — the backend already + // wrote hosts.toml, so the card must refresh to reflect the new hasKey / + // passwordAuthDisabled, and "Open terminal" must use the saved key. Mounted globally + // (AppShell) so it survives navigating away from the Dashboard mid-run. import Modal from '$lib/components/Modal.svelte'; import { Button, Icon, StatusDot } from '$lib/theme'; import { keySetup, dismissKeySetup } from '$lib/stores/keySetup'; import { reloadHosts } from '$lib/ipc/commands'; + import { spawnSession } from '$lib/stores/navigation'; import { lastError } from '$lib/stores/notifications'; const message = (e: unknown): string => (e instanceof Error ? e.message : String(e)); - // Reload hosts once per completed run. Cleared while a run is not complete, so + // Reload hosts once per run that may have saved a key. Cleared while a run has not, so // re-running setup on the same host reloads again. let reloadedFor = $state(null); + // The terminal waits for it, so it connects with the key the backend just saved. + let reloaded: Promise = Promise.resolve(); + const BACKUP = '/etc/ssh/sshd_config.omnyssh_backup.*'; + + async function openTerminal(hostName: string): Promise { + await reloaded; + spawnSession('terminal', hostName); + dismissKeySetup(); + } + $effect(() => { const run = $keySetup; if (!run) return; - if (run.phase.kind === 'complete') { + const phase = run.phase; + if (phase.kind === 'complete' || (phase.kind === 'failed' && phase.passwordMayBeOff)) { if (reloadedFor !== run.hostName) { reloadedFor = run.hostName; - reloadHosts().catch((e) => lastError.set(message(e))); + reloaded = reloadHosts().catch((e) => lastError.set(message(e))); } } else if (reloadedFor === run.hostName) { reloadedFor = null; @@ -61,10 +75,24 @@

{:else if phase.kind === 'complete'}
- +
-

Key authentication configured

-

{phase.keyPath}

+ {#if phase.passwordOff} +

Key login is set up and password login is off

+

{phase.keyPath}

+

+ The server's SSH settings were changed; a backup is kept at + {BACKUP}. +

+

The password saved in OmnySSH was removed.

+ {:else} +

+ Key login works. Password login is still on — OmnySSH needs sudo without a + password prompt to change server settings. +

+

{phase.keyPath}

+

Your saved password was kept.

+ {/if}
@@ -74,21 +102,48 @@
-

Key setup failed

+ {#if phase.passwordMayBeOff} +

Key setup failed — password login may be off

+

+ The server's SSH settings may have been changed. Password login may be off + now, or after the SSH service next restarts. The new key was saved for this + host. +

+

+ To see, open a terminal and run + sudo sshd -T | grep -i passwordauthentication. +

+

+ To turn passwords back on, restore + /etc/ssh/sshd_config from the newest + {BACKUP} and reload sshd. If you + can't log in, use your hosting provider's console. +

+ {:else} +

Key setup failed

+ {/if}

{phase.error}

-

Password authentication was not changed.

+ {#if !phase.passwordMayBeOff} +

Password login was not changed.

+ {/if}
-
+
+ {#if phase.passwordMayBeOff} + + {/if}
{:else}
-

Rolled back

-

{phase.result}

-

The server's password authentication was restored.

+

Password login was turned back on

+

+ A step after password login was turned off failed, so OmnySSH restored the + server's SSH settings from its backup. Password login works as before. +

+

Cause: {phase.result}

diff --git a/crates/omnyssh-gui/ui/src/lib/stores/keySetup.test.ts b/crates/omnyssh-gui/ui/src/lib/stores/keySetup.test.ts index 8ddae19f..755d7ab8 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/keySetup.test.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/keySetup.test.ts @@ -51,26 +51,34 @@ describe('reduceProgress', () => { expect(reduceProgress(null, 'web-1', step(1))).toBeNull(); }); - it('overwrites a terminal phase only for its own host', () => { - const done: KeySetupRun = { hostName: 'web-1', phase: { kind: 'complete', keyPath: '/k' } }; - // A late progress for the same host would re-open running (harmless); a different - // host's progress is ignored so the completed panel stays put. + it('never turns an outcome back into a running panel', () => { + const done: KeySetupRun = { + hostName: 'web-1', + phase: { kind: 'complete', keyPath: '/k', passwordOff: true } + }; + const failed: KeySetupRun = { + hostName: 'web-1', + phase: { kind: 'failed', error: 'x', passwordMayBeOff: true } + }; + // A late step must not hide the outcome, least of all a may-be-off failure. + expect(reduceProgress(done, 'web-1', step(4))).toBe(done); + expect(reduceProgress(failed, 'web-1', step(5))).toBe(failed); expect(reduceProgress(done, 'db-9', step(4))).toBe(done); }); }); describe('terminal reducers', () => { - it('reduceComplete carries the key path', () => { - expect(reduceComplete('web-1', '/home/me/.ssh/omnyssh_web-1_ed25519')).toEqual({ + it('reduceComplete carries the key path and whether password login is off', () => { + expect(reduceComplete('web-1', '/home/me/.ssh/omnyssh_web-1_ed25519', false)).toEqual({ hostName: 'web-1', - phase: { kind: 'complete', keyPath: '/home/me/.ssh/omnyssh_web-1_ed25519' } + phase: { kind: 'complete', keyPath: '/home/me/.ssh/omnyssh_web-1_ed25519', passwordOff: false } }); }); - it('reduceFailed carries the error', () => { - expect(reduceFailed('web-1', 'Connection failed')).toEqual({ + it('reduceFailed carries the error and whether password login may be off', () => { + expect(reduceFailed('web-1', 'Connection failed', true)).toEqual({ hostName: 'web-1', - phase: { kind: 'failed', error: 'Connection failed' } + phase: { kind: 'failed', error: 'Connection failed', passwordMayBeOff: true } }); }); diff --git a/crates/omnyssh-gui/ui/src/lib/stores/keySetup.ts b/crates/omnyssh-gui/ui/src/lib/stores/keySetup.ts index 8573c469..2e9d032a 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/keySetup.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/keySetup.ts @@ -9,8 +9,8 @@ import type { KeySetupStepDto } from '$lib/bindings'; /** The phase a run is in: streaming steps, or one of the three terminal outcomes. */ export type KeySetupPhase = | { kind: 'running'; step: KeySetupStepDto | null } - | { kind: 'complete'; keyPath: string } - | { kind: 'failed'; error: string } + | { kind: 'complete'; keyPath: string; passwordOff: boolean } + | { kind: 'failed'; error: string; passwordMayBeOff: boolean } | { kind: 'rolledBack'; result: string }; export interface KeySetupRun { @@ -34,26 +34,27 @@ export function dismissKeySetup(): void { keySetup.set(null); } -/** Fold a `key-setup-progress` event into the active run. Only the active host's run - * advances — a stray progress for another host is ignored (single active run). Pure. */ +/** Fold a `key-setup-progress` event into the active run. Only the active host's + * running run advances — a stray progress for another host is ignored (single active + * run), and an outcome is never turned back into a running panel. Pure. */ export function reduceProgress( run: KeySetupRun | null, hostName: string, step: KeySetupStepDto ): KeySetupRun | null { - if (run?.hostName !== hostName) return run; + if (run?.hostName !== hostName || run.phase.kind !== 'running') return run; return { hostName, phase: { kind: 'running', step } }; } /** A terminal outcome always shows for its host, even if the running panel was * dismissed — so the result (and, for `complete`, the card refresh) is never missed. * Pure. */ -export function reduceComplete(hostName: string, keyPath: string): KeySetupRun { - return { hostName, phase: { kind: 'complete', keyPath } }; +export function reduceComplete(hostName: string, keyPath: string, passwordOff: boolean): KeySetupRun { + return { hostName, phase: { kind: 'complete', keyPath, passwordOff } }; } -export function reduceFailed(hostName: string, error: string): KeySetupRun { - return { hostName, phase: { kind: 'failed', error } }; +export function reduceFailed(hostName: string, error: string, passwordMayBeOff: boolean): KeySetupRun { + return { hostName, phase: { kind: 'failed', error, passwordMayBeOff } }; } export function reduceRollback(hostName: string, result: string): KeySetupRun { From 31672b363826f22143be74991048183259f64375 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Fri, 9 Oct 2026 09:51:01 +0400 Subject: [PATCH 04/15] fix(gui): ask before setting up a key --- crates/omnyssh-gui/ui/e2e/keysetup.spec.ts | 52 +++++++++++++-- .../ui/src/lib/screens/Dashboard.svelte | 38 ++++++++--- .../ui/src/lib/screens/KeySetupConfirm.svelte | 66 +++++++++++++++++++ .../ui/src/lib/stores/keySetup.test.ts | 27 ++++++++ .../omnyssh-gui/ui/src/lib/stores/keySetup.ts | 10 +++ 5 files changed, 178 insertions(+), 15 deletions(-) create mode 100644 crates/omnyssh-gui/ui/src/lib/screens/KeySetupConfirm.svelte diff --git a/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts b/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts index a89a8424..02605dad 100644 --- a/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts +++ b/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts @@ -112,9 +112,39 @@ async function boot(page: Page, outcome: Outcome = 'success'): Promise { const startCalls = (page: Page) => page.evaluate(() => (window as unknown as { __startCalls: number }).__startCalls); +/** Open the confirm dialog from the card and accept it. */ +async function confirmSetup(page: Page): Promise { + await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + await page.getByRole('dialog', { name: 'Set up key login' }).getByRole('button', { name: 'Set up key login' }).click(); +} + +test('asks before key setup, with Cancel focused, and Cancel starts nothing', async ({ page }) => { + await boot(page); + + await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + const confirm = page.getByRole('dialog', { name: 'Set up key login' }); + await expect(confirm.getByText('Set up key login for pw-host?')).toBeVisible(); + // Desktop Chrome's user agent says Windows, so the path is the Windows one. + await expect(confirm.getByText('%USERPROFILE%\\.ssh\\omnyssh_pw-host_ed25519')).toBeVisible(); + await expect(confirm.getByText('root@pw.example.com')).toBeVisible(); + await expect(confirm.getByText(/for every account and device/)).toBeVisible(); + await expect(confirm.getByRole('button', { name: 'Cancel' })).toBeFocused(); + + await confirm.getByRole('button', { name: 'Cancel' }).click(); + await expect(page.getByRole('dialog')).toHaveCount(0); + expect(await startCalls(page)).toBe(0); + + // Escape cancels too. + await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + await expect(confirm).toBeVisible(); + await page.keyboard.press('Escape'); + await expect(page.getByRole('dialog')).toHaveCount(0); + expect(await startCalls(page)).toBe(0); +}); + test('a full success says password login is off and the card shows the shield', async ({ page }) => { await boot(page, 'success'); - await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + await confirmSetup(page); const dialog = page.getByRole('dialog', { name: 'Key setup' }); await expect(dialog).toBeVisible(); @@ -132,9 +162,9 @@ test('a full success says password login is off and the card shows the shield', await expect(page.getByRole('button', { name: 'Set up an SSH key for pw-host' })).toHaveCount(0); }); -test('a partial run says password login is still on', async ({ page }) => { +test('a partial run says password login is still on and offers to turn it off', async ({ page }) => { await boot(page, 'partial'); - await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + await confirmSetup(page); const dialog = page.getByRole('dialog', { name: 'Key setup' }); await expect(dialog.getByText(/Key login works\. Password login is still on/)).toBeVisible(); @@ -142,13 +172,23 @@ test('a partial run says password login is still on', async ({ page }) => { await expect(dialog.getByText(/password login is off/)).toHaveCount(0); await dialog.getByRole('button', { name: 'Done' }).click(); - await expect(page.getByText('Password on')).toBeVisible(); + const badge = page.getByRole('button', { name: 'Password on' }); + await expect(badge).toBeVisible(); await expect(page.getByRole('img', { name: 'Key login only' })).toHaveCount(0); + + await badge.click(); + const rerun = page.getByRole('dialog', { name: 'Turn off password login' }); + await expect(rerun.getByText('Turn off password login on pw-host?')).toBeVisible(); + await expect(rerun.getByText(/Reuse the login key OmnySSH made before/)).toBeVisible(); + await expect(rerun.getByRole('button', { name: 'Cancel' })).toBeFocused(); + await rerun.getByRole('button', { name: 'Turn off password login' }).click(); + await expect(page.getByRole('dialog', { name: 'Key setup' })).toBeVisible(); + expect(await startCalls(page)).toBe(2); }); test('a failure that may leave password login off says so', async ({ page }) => { await boot(page, 'unsafe'); - await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + await confirmSetup(page); const dialog = page.getByRole('dialog', { name: 'Key setup' }); await expect(dialog.getByText('Key setup failed — password login may be off')).toBeVisible(); @@ -168,7 +208,7 @@ test('a failure that may leave password login off says so', async ({ page }) => test('a rollback says password login was turned back on', async ({ page }) => { await boot(page, 'rollback'); - await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + await confirmSetup(page); const dialog = page.getByRole('dialog', { name: 'Key setup' }); await expect(dialog.getByText('Password login was turned back on')).toBeVisible(); diff --git a/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte b/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte index 19403651..2712c5c8 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte @@ -41,9 +41,14 @@ import { emptyForm, formFromHost } from './hostForm'; import { clampToScroller, dropTarget, scrollParent, scrollStep, swallowClick } from './cardDrag'; import HostEditor from './HostEditor.svelte'; + import KeySetupConfirm from './KeySetupConfirm.svelte'; import Modal from '$lib/components/Modal.svelte'; - type Dialog = { kind: 'add' } | { kind: 'edit'; host: HostDto } | { kind: 'delete'; host: HostDto }; + type Dialog = + | { kind: 'add' } + | { kind: 'edit'; host: HostDto } + | { kind: 'delete'; host: HostDto } + | { kind: 'keySetup'; host: HostDto; rerun: boolean }; let dialog = $state(null); @@ -334,8 +339,8 @@ dialog = null; } - // Host-first auto key-setup (tech-gui.md §4.2). Open the progress panel immediately, - // then kick the backend flow; its progress/outcome arrive as `key-setup-*` events. + // Host-first auto key-setup (tech-gui.md §4.2), once the confirm dialog is accepted. + // Open the progress panel immediately, then kick the backend flow; its progress/outcome arrive as `key-setup-*` events. // A synchronous reject (unknown host) closes the panel and surfaces the error. async function setupKey(host: HostDto): Promise { beginKeySetup(host.name); @@ -347,6 +352,11 @@ } } + function confirmKeySetup(host: HostDto): void { + dialog = null; + void setupKey(host); + } + // The outcome arrives as `tunnel-status-changed`; only a rejected command lands here. async function toggleTunnel(name: string, running: boolean): Promise { try { @@ -622,7 +632,7 @@ {/if} + takes passwords (offers the re-run); else a plain key badge. --> {#if card.host.passwordAuthDisabled === true} {:else if card.host.passwordAuthDisabled === false} - (dialog = { kind: 'keySetup', host: card.host, rerun: true })} > Password on - + {:else if card.host.hasKey} setupKey(card.host)} + onclick={() => (dialog = { kind: 'keySetup', host: card.host, rerun: false })} > @@ -859,6 +871,14 @@ onSubmit={submit} onCancel={() => (dialog = null)} /> +{:else if dialog?.kind === 'keySetup'} + {@const host = dialog.host} + confirmKeySetup(host)} + onCancel={() => (dialog = null)} + /> {:else if dialog?.kind === 'delete'} {@const host = dialog.host} (dialog = null)}> diff --git a/crates/omnyssh-gui/ui/src/lib/screens/KeySetupConfirm.svelte b/crates/omnyssh-gui/ui/src/lib/screens/KeySetupConfirm.svelte new file mode 100644 index 00000000..3c918ad1 --- /dev/null +++ b/crates/omnyssh-gui/ui/src/lib/screens/KeySetupConfirm.svelte @@ -0,0 +1,66 @@ + + + +
+

+ {rerun ? `Turn off password login on ${host.name}?` : `Set up key login for ${host.name}?`} +

+
    + {#if rerun} +
  • + Reuse the login key OmnySSH made before, or create it again if it is gone: + {keyPath} +
    • + {:else} +
  • + Create a login key on this computer: + {keyPath} (no passphrase) +
    • + {/if} +
  • + Add it to + {host.user}@{displayHostname(host.hostname, $streamerMode)}, + so this computer logs in without a password +
    • +
  • + Turn off password login on the server for every account and device, if your account + can use sudo without a password. A backup of the server's SSH settings is kept. +
    • +
+

+ Password login is turned off only after the new key is tested. Keep the key file: if + you lose it, you'll need your hosting provider's console to get back in. +

+
+ + +
+
+ diff --git a/crates/omnyssh-gui/ui/src/lib/stores/keySetup.test.ts b/crates/omnyssh-gui/ui/src/lib/stores/keySetup.test.ts index 755d7ab8..6286a536 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/keySetup.test.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/keySetup.test.ts @@ -4,6 +4,7 @@ import type { KeySetupStepDto } from '$lib/bindings'; import { beginKeySetup, dismissKeySetup, + keyFilePath, keySetup, reduceComplete, reduceFailed, @@ -89,3 +90,29 @@ describe('terminal reducers', () => { }); }); }); + +describe('keyFilePath', () => { + // Pinned to the core's `sanitize_hostname` on the same inputs. + it.each([ + ['web-prod_1', 'web-prod_1'], + ['web.1', 'web_1'], + ['Ελλάδα.prod-1', 'Ελλάδα_prod-1'], + ['日本語', '日本語'], + ['db²½Ⅻ', 'db²½Ⅻ'], + ['my server (prod)', 'my_server__prod_'], + ['../../etc/passwd', '______etc_passwd'], + ['😀db', '_db'], + ['e\u0301x', 'e_x'], + ['', 'unnamed_host'] + ])('sanitizes %j to %s', (name, sanitized) => { + expect(keyFilePath(name, false)).toBe(`~/.ssh/omnyssh_${sanitized}_ed25519`); + }); + + it('cuts the name to 64 code points', () => { + expect(keyFilePath('λ'.repeat(70), false)).toBe(`~/.ssh/omnyssh_${'λ'.repeat(64)}_ed25519`); + }); + + it('shows the Windows profile folder on Windows', () => { + expect(keyFilePath('web.1', true)).toBe('%USERPROFILE%\\.ssh\\omnyssh_web_1_ed25519'); + }); +}); diff --git a/crates/omnyssh-gui/ui/src/lib/stores/keySetup.ts b/crates/omnyssh-gui/ui/src/lib/stores/keySetup.ts index 2e9d032a..38912a53 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/keySetup.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/keySetup.ts @@ -60,3 +60,13 @@ export function reduceFailed(hostName: string, error: string, passwordMayBeOff: export function reduceRollback(hostName: string, result: string): KeySetupRun { return { hostName, phase: { kind: 'rolledBack', result } }; } + +/** The key file the core creates (or reuses) for `hostName`, as the user would find it: + * `omnyssh__ed25519` in ~/.ssh. Mirrors the core's `sanitize_hostname`: each + * code point outside Unicode Alphabetic / Numeric, `-` and `_` becomes `_`, cut to 64 + * code points, and an empty name is `unnamed_host`. Pure. */ +export function keyFilePath(hostName: string, windows: boolean): string { + const kept = [...hostName].map((c) => (/^[\p{Alphabetic}\p{N}_-]$/u.test(c) ? c : '_')); + const file = `omnyssh_${kept.slice(0, 64).join('') || 'unnamed_host'}_ed25519`; + return windows ? `%USERPROFILE%\\.ssh\\${file}` : `~/.ssh/${file}`; +} From ca144f50b342aa000b290d9c4ace408ac471b2b9 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Fri, 9 Oct 2026 09:45:30 +0400 Subject: [PATCH 05/15] fix(gui): ask before quitting while terminals, transfers or tunnels are live --- crates/omnyssh-gui/capabilities/default.json | 3 + crates/omnyssh-gui/src/main.rs | 7 + crates/omnyssh-gui/src/menu.rs | 84 ++++++++ crates/omnyssh-gui/src/tray.rs | 80 +++++++- crates/omnyssh-gui/ui/e2e/quit.spec.ts | 193 ++++++++++++++++++ .../ui/src/lib/components/QuitConfirm.svelte | 80 ++++++++ .../ui/src/lib/screens/Settings.svelte | 8 + .../ui/src/lib/screens/SnippetResults.svelte | 4 +- .../ui/src/lib/stores/quit.test.ts | 174 ++++++++++++++++ crates/omnyssh-gui/ui/src/lib/stores/quit.ts | 154 ++++++++++++++ .../ui/src/lib/stores/snippets.test.ts | 29 ++- .../omnyssh-gui/ui/src/lib/stores/snippets.ts | 22 +- .../omnyssh-gui/ui/src/routes/+layout.svelte | 22 ++ 13 files changed, 843 insertions(+), 17 deletions(-) create mode 100644 crates/omnyssh-gui/src/menu.rs create mode 100644 crates/omnyssh-gui/ui/e2e/quit.spec.ts create mode 100644 crates/omnyssh-gui/ui/src/lib/components/QuitConfirm.svelte create mode 100644 crates/omnyssh-gui/ui/src/lib/stores/quit.test.ts create mode 100644 crates/omnyssh-gui/ui/src/lib/stores/quit.ts diff --git a/crates/omnyssh-gui/capabilities/default.json b/crates/omnyssh-gui/capabilities/default.json index 2cf8c266..9260e47c 100644 --- a/crates/omnyssh-gui/capabilities/default.json +++ b/crates/omnyssh-gui/capabilities/default.json @@ -7,6 +7,9 @@ "core:default", "core:window:allow-set-theme", "core:window:allow-start-dragging", + "core:window:allow-destroy", + "core:window:allow-unminimize", + "core:window:allow-set-focus", "store:default", { "identifier": "opener:allow-open-url", diff --git a/crates/omnyssh-gui/src/main.rs b/crates/omnyssh-gui/src/main.rs index e9ad0c92..674ba116 100644 --- a/crates/omnyssh-gui/src/main.rs +++ b/crates/omnyssh-gui/src/main.rs @@ -11,6 +11,8 @@ mod commands; mod dto; mod error; mod events; +#[cfg(target_os = "macos")] +mod menu; mod state; mod tray; @@ -246,6 +248,11 @@ fn main() { tray::reveal(app) })); } + // Replaces Tauri's default, whose ⌘W and ⌘Q close and quit without asking. + #[cfg(target_os = "macos")] + { + app = app.menu(menu::build); + } app // Persists UI prefs (theme, sidebar collapse, refresh interval) from the // frontend JS API — no bespoke command (tech-gui.md §4.2, §5.1). diff --git a/crates/omnyssh-gui/src/menu.rs b/crates/omnyssh-gui/src/menu.rs new file mode 100644 index 00000000..628e6d85 --- /dev/null +++ b/crates/omnyssh-gui/src/menu.rs @@ -0,0 +1,84 @@ +//! The macOS app menu. Tauri's default binds ⌘W and ⌘Q to `performClose:` and +//! `terminate:`, which close the window and end the app without asking; this one keeps +//! everything else and routes both through the page instead. + +use tauri::menu::{AboutMetadata, Menu, MenuItem, PredefinedMenuItem, Submenu}; +use tauri::{AppHandle, Wry}; + +pub const QUIT_ID: &str = "app-quit"; +pub const CLOSE_WINDOW_ID: &str = "app-close-window"; + +/// No item claims ⌘W, so the page gets it and closes the active tab. +pub fn build(app: &AppHandle) -> tauri::Result> { + let info = app.package_info(); + let about = AboutMetadata { + name: Some(info.name.clone()), + version: Some(info.version.to_string()), + copyright: app.config().bundle.copyright.clone(), + authors: app.config().bundle.publisher.clone().map(|p| vec![p]), + ..Default::default() + }; + let separator = || PredefinedMenuItem::separator(app); + + let app_menu = Submenu::with_items( + app, + info.name.clone(), + true, + &[ + &PredefinedMenuItem::about(app, None, Some(about))?, + &separator()?, + &PredefinedMenuItem::services(app, None)?, + &separator()?, + &PredefinedMenuItem::hide(app, None)?, + &PredefinedMenuItem::hide_others(app, None)?, + &PredefinedMenuItem::show_all(app, None)?, + &separator()?, + &MenuItem::with_id(app, QUIT_ID, "Quit OmnySSH", true, Some("CmdOrCtrl+Q"))?, + ], + )?; + // Closing goes through `CloseRequested`, so the tray or the page's question still apply. + let file = Submenu::with_items( + app, + "File", + true, + &[&MenuItem::with_id( + app, + CLOSE_WINDOW_ID, + "Close Window", + true, + Some("CmdOrCtrl+Shift+W"), + )?], + )?; + // WKWebView's clipboard and undo shortcuts work only through these items. + let edit = Submenu::with_items( + app, + "Edit", + true, + &[ + &PredefinedMenuItem::undo(app, None)?, + &PredefinedMenuItem::redo(app, None)?, + &separator()?, + &PredefinedMenuItem::cut(app, None)?, + &PredefinedMenuItem::copy(app, None)?, + &PredefinedMenuItem::paste(app, None)?, + &PredefinedMenuItem::select_all(app, None)?, + ], + )?; + let view = Submenu::with_items( + app, + "View", + true, + &[&PredefinedMenuItem::fullscreen(app, None)?], + )?; + let window = Submenu::with_items( + app, + "Window", + true, + &[ + &PredefinedMenuItem::minimize(app, None)?, + // "Zoom" on macOS. + &PredefinedMenuItem::maximize(app, None)?, + ], + )?; + Menu::with_items(app, &[&app_menu, &file, &edit, &view, &window]) +} diff --git a/crates/omnyssh-gui/src/tray.rs b/crates/omnyssh-gui/src/tray.rs index a4cbaa6b..e970dfa3 100644 --- a/crates/omnyssh-gui/src/tray.rs +++ b/crates/omnyssh-gui/src/tray.rs @@ -4,7 +4,8 @@ //! a tray it would have nowhere to come back from. use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::{Mutex, PoisonError}; +use std::sync::{mpsc, Mutex, PoisonError}; +use std::time::Duration; use tauri::menu::{Menu, MenuEvent, MenuItem}; use tauri::tray::{MouseButton, MouseButtonState, TrayIconBuilder, TrayIconEvent}; @@ -55,6 +56,13 @@ pub static REVEALED: AtomicBool = AtomicBool::new(false); /// does not send it straight back. static MINIMIZED: AtomicBool = AtomicBool::new(false); +/// Set by the tray's and the app menu's Quit, taken by the close it starts: that close +/// ends the app instead of hiding into the tray. +static QUITTING: AtomicBool = AtomicBool::new(false); + +/// How long a page has to answer before a close it holds up goes through anyway. +const PAGE_ANSWER: Duration = Duration::from_secs(5); + fn behavior() -> Behavior { *BEHAVIOR.lock().unwrap_or_else(PoisonError::into_inner) } @@ -265,11 +273,66 @@ pub fn reveal(app: &AppHandle) { } } +/// The tray's and the app menu's Quit, as the window's own close: a page listening for +/// it asks first when terminals, transfers, tunnels or snippet runs are live, and with +/// no listener the window closes, which ends the app. +fn request_quit(app: &AppHandle) { + // Never `prevent_exit` anywhere: it would stop these exits too. + let Some(window) = app.get_webview_window("main") else { + return app.exit(0); + }; + // Still coming up, so nothing is live yet, and the page takes a hidden window's + // close for one into the tray. + if !REVEALED.load(Ordering::Acquire) { + return app.exit(0); + } + QUITTING.store(true, Ordering::Release); + // Out of the tray or the Dock: the question has to be seen. + reveal(app); + if window.close().is_err() { + QUITTING.store(false, Ordering::Release); + app.exit(0); + } +} + +/// Set by the page while its close handler is registered (`routes/+layout.svelte`). +const CLOSE_GUARD: &str = "window.__omnysshAsksOnClose === true ? 1 : 0"; + +/// Once the page listens for closes Tauri leaves every close to it, so a hung page +/// would hold the window open for good. `destroy` skips `CloseRequested`. +fn close_unless_page_answers(window: &Window) { + let Some(webview) = window.get_webview_window(window.label()) else { + return; + }; + let (answered, answer) = mpsc::channel(); + // Only the page's own flag counts: a dead web process answers every script at once, + // with an empty result, while its stale listener still holds the close. + let _ = webview.eval_with_callback(CLOSE_GUARD, move |result| { + if result == "1" { + let _ = answered.send(()); + } + }); + let window = window.clone(); + // Never on the main thread: the answer arrives there. + std::thread::spawn(move || { + if answer.recv_timeout(PAGE_ANSWER).is_err() { + let _ = window.destroy(); + } + }); +} + pub fn on_menu_event(app: &AppHandle, event: MenuEvent) { match event.id().as_ref() { SHOW_ID => reveal(app), - // Never `prevent_exit` anywhere: it would stop this too. - QUIT_ID => app.exit(0), + QUIT_ID => request_quit(app), + #[cfg(target_os = "macos")] + crate::menu::QUIT_ID => request_quit(app), + #[cfg(target_os = "macos")] + crate::menu::CLOSE_WINDOW_ID => { + if let Some(window) = app.get_webview_window("main") { + let _ = window.close(); + } + } _ => {} } } @@ -277,9 +340,14 @@ pub fn on_menu_event(app: &AppHandle, event: MenuEvent) { pub fn on_window_event(window: &Window, event: &WindowEvent) { let behavior = behavior(); match event { - WindowEvent::CloseRequested { api, .. } if behavior.closes_to_tray() => { - api.prevent_close(); - let _ = window.hide(); + WindowEvent::CloseRequested { api, .. } => { + let quitting = QUITTING.swap(false, Ordering::AcqRel); + if behavior.closes_to_tray() && !quitting { + api.prevent_close(); + let _ = window.hide(); + } else { + close_unless_page_answers(window); + } } // Minimizing arrives as a resize, on GTK with the size unchanged. WindowEvent::Resized(_) => { diff --git a/crates/omnyssh-gui/ui/e2e/quit.spec.ts b/crates/omnyssh-gui/ui/e2e/quit.spec.ts new file mode 100644 index 00000000..013e47d5 --- /dev/null +++ b/crates/omnyssh-gui/ui/e2e/quit.spec.ts @@ -0,0 +1,193 @@ +import { expect, test, type Page } from '@playwright/test'; + +// Quitting with live work asks first (tech-gui.md §4.3). Tauri is stubbed at +// `__TAURI_INTERNALS__` (§6.4): the window's close, and the tray's or app menu's Quit, +// arrive as `tauri://close-requested`, and the stub records every command so a test can +// tell whether the window was destroyed. +const HOSTS = [ + { name: 'web-1', hostname: 'web-1.example.com', user: 'deploy', port: 22, tags: [], source: 'manual', hasKey: true, localForwards: [], tunnelAutostart: false, forwardAgent: false } +]; + +type Call = { cmd: string; args: Record }; + +async function boot(page: Page, options: { tunnel?: boolean; visible?: boolean } = {}): Promise { + await page.addInitScript( + ({ hosts, tunnel, visible }) => { + let cbid = 0; + const listeners: Record = {}; + const win = window as unknown as Record; + const calls: Call[] = []; + win.__calls = calls; + + function fire(event: string, payload: unknown): void { + for (const id of listeners[event] ?? []) { + const cb = win[`__cb${id}`] as ((e: unknown) => void) | undefined; + cb?.({ event, id, payload }); + } + } + win.__fire = fire; + + (win as { __TAURI_INTERNALS__: unknown }).__TAURI_INTERNALS__ = { + metadata: { + currentWindow: { label: 'main' }, + currentWebview: { windowLabel: 'main', label: 'main' } + }, + invoke: (cmd: string, args: Record) => { + calls.push({ cmd, args }); + switch (cmd) { + case 'list_hosts': + return Promise.resolve(hosts); + case 'reload_hosts': + if (tunnel) { + setTimeout(() => fire('tunnel-status-changed', { hostName: 'web-1', status: { kind: 'up' } }), 0); + } + return Promise.resolve(null); + case 'set_tray_behavior': + return Promise.resolve({ available: true, minimize: true }); + case 'terminal_open': { + const chId = (args.onOutput as { id: number }).id; + setTimeout(() => { + const cb = win[`__cb${chId}`] as ((m: unknown) => void) | undefined; + cb?.({ message: new TextEncoder().encode('ready> ').buffer, index: 0 }); + }, 0); + return Promise.resolve(1); + } + case 'plugin:window|is_visible': + return Promise.resolve(visible); + case 'plugin:event|listen': { + const { event, handler } = args as { event: string; handler: number }; + (listeners[event] ||= []).push(handler); + return Promise.resolve(handler); + } + default: + return Promise.resolve(null); + } + }, + transformCallback: (cb: unknown) => { + const id = ++cbid; + win[`__cb${id}`] = cb; + return id; + }, + unregisterCallback: (id: number) => { + delete win[`__cb${id}`]; + } + }; + }, + { hosts: HOSTS, tunnel: options.tunnel ?? false, visible: options.visible ?? true } + ); + await page.goto('/'); + await expect(page.getByText('web-1', { exact: true })).toBeVisible(); +} + +const calls = (page: Page) => + page.evaluate(() => ((window as unknown as { __calls: Call[] }).__calls).map((c) => c.cmd)); + +const fire = (page: Page, event: string, payload: unknown = null) => + page.evaluate( + ([e, p]) => (window as unknown as { __fire: (e: string, p: unknown) => void }).__fire(e as string, p), + [event, payload] + ); + +const closeWindow = (page: Page) => fire(page, 'tauri://close-requested'); + +async function openTerminal(page: Page): Promise { + await page.getByTitle('sh on web-1').click(); + await expect(page.locator('.xterm-rows')).toContainText('ready>'); +} + +const dialog = (page: Page) => page.getByRole('dialog', { name: 'Quit OmnySSH?' }); + +test('closing with a live terminal and tunnel asks first, and Cancel keeps the app', async ({ page }) => { + await boot(page, { tunnel: true }); + await openTerminal(page); + + await closeWindow(page); + await expect(dialog(page)).toBeVisible(); + await expect(dialog(page).getByText('1 terminal and 1 tunnel will be closed.')).toBeVisible(); + // Not the terminal, which had the keyboard: a stray Enter must not quit. + await expect(dialog(page).getByRole('button', { name: 'Cancel' })).toBeFocused(); + + await page.keyboard.press('Enter'); + await expect(dialog(page)).toHaveCount(0); + expect(await calls(page)).not.toContain('plugin:window|destroy'); + await expect(page.locator('.xterm-rows')).toContainText('ready>'); +}); + +test('Quit destroys the window', async ({ page }) => { + await boot(page, { tunnel: true }); + await closeWindow(page); + await expect(dialog(page).getByText('1 tunnel will be closed.')).toBeVisible(); + await dialog(page).getByRole('button', { name: 'Quit' }).click(); + await expect.poll(() => calls(page)).toContain('plugin:window|destroy'); +}); + +test("Don't ask again sticks, and the next close quits without asking", async ({ page }) => { + await boot(page, { tunnel: true }); + await closeWindow(page); + await dialog(page).getByLabel("Don't ask again").check(); + await dialog(page).getByRole('button', { name: 'Quit' }).click(); + await expect.poll(() => calls(page)).toContain('plugin:window|destroy'); + expect(await page.evaluate(() => localStorage.getItem('omnyssh-confirm-quit'))).toBe('false'); + const stored = await page.evaluate(() => + (window as unknown as { __calls: Call[] }).__calls.find((c) => c.cmd === 'plugin:store|set')?.args + ); + expect(stored).toMatchObject({ key: 'confirmQuit', value: false }); + + // The next launch. + await page.reload(); + await expect(page.getByText('web-1', { exact: true })).toBeVisible(); + await closeWindow(page); + await expect.poll(() => calls(page)).toContain('plugin:window|destroy'); + await expect(dialog(page)).toHaveCount(0); + + // Settings turns the question back on. + await page.getByRole('button', { name: 'Settings' }).click(); + const ask = page.getByRole('switch', { name: 'Ask before quitting' }); + await expect(ask).toHaveAttribute('aria-checked', 'false'); + await ask.click(); + await expect(ask).toHaveAttribute('aria-checked', 'true'); + expect(await page.evaluate(() => localStorage.getItem('omnyssh-confirm-quit'))).toBe('true'); +}); + +test('with nothing live the window closes at once', async ({ page }) => { + await boot(page); + await closeWindow(page); + await expect.poll(() => calls(page)).toContain('plugin:window|destroy'); + await expect(dialog(page)).toHaveCount(0); +}); + +test('a window the tray already hid is left alone', async ({ page }) => { + await boot(page, { tunnel: true, visible: false }); + await closeWindow(page); + await expect.poll(() => calls(page)).toContain('plugin:window|is_visible'); + await page.waitForTimeout(100); + expect(await calls(page)).not.toContain('plugin:window|destroy'); + await expect(dialog(page)).toHaveCount(0); +}); + +test('the question brings a minimized window forward, once', async ({ page }) => { + await boot(page, { tunnel: true }); + await closeWindow(page); + await expect(dialog(page).getByText('1 tunnel will be closed.')).toBeVisible(); + await expect + .poll(() => calls(page)) + .toEqual(expect.arrayContaining(['plugin:window|unminimize', 'plugin:window|set_focus'])); + + // A second close while it is open does not stack another. + await closeWindow(page); + await expect(dialog(page)).toHaveCount(1); + + // Keeping it running in the tray instead goes to the setting. + await dialog(page).getByRole('button', { name: 'Keep running in the tray instead' }).click(); + await expect(dialog(page)).toHaveCount(0); + await expect(page.getByRole('switch', { name: 'Close to tray' })).toBeVisible(); + expect(await calls(page)).not.toContain('plugin:window|destroy'); +}); + +test('with nothing live a minimized window closes where it is', async ({ page }) => { + await boot(page); + await closeWindow(page); + await expect.poll(() => calls(page)).toContain('plugin:window|destroy'); + await expect(dialog(page)).toHaveCount(0); + expect(await calls(page)).not.toContain('plugin:window|unminimize'); +}); diff --git a/crates/omnyssh-gui/ui/src/lib/components/QuitConfirm.svelte b/crates/omnyssh-gui/ui/src/lib/components/QuitConfirm.svelte new file mode 100644 index 00000000..dfbec65f --- /dev/null +++ b/crates/omnyssh-gui/ui/src/lib/components/QuitConfirm.svelte @@ -0,0 +1,80 @@ + + + +
+

Quit OmnySSH?

+

{summary} will be closed.

+ + {#if $traySupport.available} + + {/if} +
+ + +
+
+ diff --git a/crates/omnyssh-gui/ui/src/lib/screens/Settings.svelte b/crates/omnyssh-gui/ui/src/lib/screens/Settings.svelte index b7cb61fd..eeb20fe2 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/Settings.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/Settings.svelte @@ -10,6 +10,7 @@ import { streamerMode } from '$lib/stores/streamer'; import { refreshInterval, REFRESH_OPTIONS } from '$lib/stores/settings'; import { traySupport, trayBehavior } from '$lib/stores/tray'; + import { confirmQuit } from '$lib/stores/quit'; import { isMac } from '$lib/platform'; import { offerUpdate } from '$lib/stores/update'; import { lastError } from '$lib/stores/notifications'; @@ -199,6 +200,13 @@ No icon in sight? Opening OmnySSH again brings the window back.

{/if} + {@render traySwitch( + 'Ask before quitting', + 'When terminals, transfers, tunnels or snippet runs are still open.', + $confirmQuit, + true, + () => confirmQuit.toggle() + )}
diff --git a/crates/omnyssh-gui/ui/src/lib/screens/SnippetResults.svelte b/crates/omnyssh-gui/ui/src/lib/screens/SnippetResults.svelte index 2d317076..2256a094 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/SnippetResults.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/SnippetResults.svelte @@ -1,7 +1,7 @@ -{#if $snippetRun} +{#if $snippetRun && !$snippetRun.dismissed} {@const run = $snippetRun}
diff --git a/crates/omnyssh-gui/ui/src/lib/stores/quit.test.ts b/crates/omnyssh-gui/ui/src/lib/stores/quit.test.ts new file mode 100644 index 00000000..bfe5abb7 --- /dev/null +++ b/crates/omnyssh-gui/ui/src/lib/stores/quit.test.ts @@ -0,0 +1,174 @@ +// @vitest-environment jsdom +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { get } from 'svelte/store'; +import type { CloseRequestedEvent, Window } from '@tauri-apps/api/window'; +import type { TunnelStatusDto } from '$lib/bindings'; +import type { Session } from './sessions'; +import { newSession, type SftpSession } from './sftp'; +import { tunnels } from './tunnels'; +import { beginRun, clearRun, snippetRun } from './snippets'; +import { countLiveWork, describeLiveWork, handleCloseRequested, quitPrompt, type LiveWork } from './quit'; + +const NONE: LiveWork = { terminals: 0, transfers: 0, tunnels: 0, snippetRuns: 0 }; + +const terminal = (status: Session['status']): Session => ({ + id: Math.random(), + kind: 'terminal', + hostName: 'web-1', + status +}); + +describe('countLiveWork', () => { + it('counts only what a quit would cut off', () => { + const list = [ + terminal('connecting'), + terminal('connected'), + // An ended tab is only its last screen; a failed one never connected. + terminal('closed'), + terminal('failed'), + { ...terminal('connected'), kind: 'sftp' } as Session + ]; + const idle = newSession('web-1'); + const running: SftpSession = { + ...newSession('db-1'), + transfer: { kind: 'download', name: 'a.log', done: 1, total: 2 } + }; + // Queued behind a mkdir: not running yet, but it would be lost all the same. + const queued: SftpSession = { + ...newSession('db-2'), + pending: [ + { kind: 'mkdir', refresh: 'remote' }, + { kind: 'upload', name: 'b.txt', refresh: 'remote' } + ] + }; + const busyOnly: SftpSession = { ...newSession('db-3'), pending: [{ kind: 'delete', refresh: 'remote' }] }; + const tunnels = new Map([ + ['a', { kind: 'connecting' }], + ['b', { kind: 'up' }], + ['c', { kind: 'retrying', message: 'refused' }], + ['d', { kind: 'failed', message: 'gone' }] + ]); + const run = { + snippetName: 'deploy', + entries: [ + { hostName: 'a', pending: false, ok: true, output: '' }, + { hostName: 'b', pending: true, ok: false, output: '' } + ] + }; + + expect( + countLiveWork(list, new Map([[1, idle], [2, running], [3, queued], [4, busyOnly]]), tunnels, run) + ).toEqual({ terminals: 2, transfers: 2, tunnels: 3, snippetRuns: 1 }); + }); + + it('finds nothing live in an idle app or a finished snippet run', () => { + const done = { snippetName: 'x', entries: [{ hostName: 'a', pending: false, ok: true, output: '' }] }; + expect(countLiveWork([], new Map(), new Map(), null)).toEqual(NONE); + expect(countLiveWork([], new Map(), new Map(), done)).toEqual(NONE); + }); + + it('still counts a snippet run whose results panel was closed', () => { + beginRun('deploy', ['a']); + clearRun(); + expect(countLiveWork([], new Map(), new Map(), get(snippetRun)).snippetRuns).toBe(1); + snippetRun.set(null); + }); +}); + +describe('handleCloseRequested', () => { + const order: string[] = []; + const event = { preventDefault: vi.fn(() => order.push('preventDefault')) }; + const win = { + isVisible: vi.fn(async () => true), + unminimize: vi.fn(async () => void order.push('unminimize')), + setFocus: vi.fn(async () => void order.push('setFocus')) + }; + const close = () => + handleCloseRequested(event as unknown as CloseRequestedEvent, win as unknown as Window); + + beforeEach(() => { + order.length = 0; + vi.clearAllMocks(); + win.isVisible.mockResolvedValue(true); + quitPrompt.set(null); + tunnels.set(new Map()); + }); + + it('asks in a window brought out of the taskbar or dock, after holding the close', async () => { + tunnels.set(new Map([['web-1', { kind: 'up' }]])); + await close(); + expect(get(quitPrompt)).toBe('1 tunnel'); + expect(order).toEqual(['preventDefault', 'unminimize', 'setFocus']); + }); + + it('lets the window close when nothing is live', async () => { + await close(); + expect(order).toEqual([]); + expect(get(quitPrompt)).toBeNull(); + }); + + it('keeps a window the tray hid, without asking or showing it', async () => { + tunnels.set(new Map([['web-1', { kind: 'up' }]])); + win.isVisible.mockResolvedValue(false); + await close(); + expect(order).toEqual(['preventDefault']); + expect(get(quitPrompt)).toBeNull(); + }); +}); + +describe('describeLiveWork', () => { + it('lists the counts, singular or plural, with "and" before the last', () => { + expect(describeLiveWork({ terminals: 2, transfers: 1, tunnels: 1, snippetRuns: 0 })).toBe( + '2 terminals, 1 transfer and 1 tunnel' + ); + expect(describeLiveWork({ ...NONE, terminals: 1 })).toBe('1 terminal'); + expect(describeLiveWork({ ...NONE, tunnels: 3, snippetRuns: 1 })).toBe('3 tunnels and 1 snippet run'); + expect(describeLiveWork({ terminals: 1, transfers: 2, tunnels: 2, snippetRuns: 1 })).toBe( + '1 terminal, 2 transfers, 2 tunnels and 1 snippet run' + ); + }); + + it('is empty when nothing is live', () => { + expect(describeLiveWork(NONE)).toBe(''); + }); +}); + +// Persisted like the other UI prefs: a fake Tauri store, a fresh module per test. +const backend = { get: vi.fn(), set: vi.fn(), save: vi.fn() }; +vi.mock('@tauri-apps/plugin-store', () => ({ load: vi.fn(async () => backend) })); + +async function fresh() { + vi.resetModules(); + return (await import('./quit')).confirmQuit; +} + +describe('confirmQuit persistence', () => { + beforeEach(() => { + localStorage.clear(); + backend.get.mockReset(); + backend.set.mockReset().mockResolvedValue(undefined); + backend.save.mockReset().mockResolvedValue(undefined); + }); + + it('defaults to asking and reads the localStorage mirror', async () => { + expect(get(await fresh())).toBe(true); + localStorage.setItem('omnyssh-confirm-quit', 'false'); + expect(get(await fresh())).toBe(false); + }); + + it('has the store written by the time set resolves, as a quit follows at once', async () => { + const confirmQuit = await fresh(); + await confirmQuit.set(false); + expect(backend.set).toHaveBeenCalledWith('confirmQuit', false); + expect(backend.save).toHaveBeenCalled(); + expect(localStorage.getItem('omnyssh-confirm-quit')).toBe('false'); + }); + + it('hydrate applies the stored value without clobbering a fresh user flip', async () => { + backend.get.mockResolvedValue(false); + const confirmQuit = await fresh(); + void confirmQuit.set(true); + await confirmQuit.hydrate(); + expect(get(confirmQuit)).toBe(true); + }); +}); diff --git a/crates/omnyssh-gui/ui/src/lib/stores/quit.ts b/crates/omnyssh-gui/ui/src/lib/stores/quit.ts new file mode 100644 index 00000000..89667a74 --- /dev/null +++ b/crates/omnyssh-gui/ui/src/lib/stores/quit.ts @@ -0,0 +1,154 @@ +import { get, writable } from 'svelte/store'; +import type { CloseRequestedEvent, Window } from '@tauri-apps/api/window'; +import type { TunnelStatusDto } from '$lib/bindings'; +import { sessions, type Session } from './sessions'; +import { sftp, type SftpSession } from './sftp'; +import { tunnels } from './tunnels'; +import { snippetRun, type SnippetRun } from './snippets'; + +// Quitting ends every terminal, transfer, tunnel and snippet run at once, so the app +// asks first while any is live (tech-gui.md §4.3). `confirmQuit` persists +// like the other UI prefs (tauri-plugin-store + a localStorage mirror). +const LOCAL_KEY = 'omnyssh-confirm-quit'; +const STORE_FILE = 'settings.json'; +const STORE_KEY = 'confirmQuit'; + +function mirrored(): boolean { + try { + return localStorage.getItem(LOCAL_KEY) !== 'false'; + } catch { + return true; // localStorage unavailable: default to asking. + } +} + +function mirrorLocal(on: boolean): void { + try { + localStorage.setItem(LOCAL_KEY, String(on)); + } catch { + // localStorage unavailable (hardened webview): the store copy is canonical. + } +} + +async function persistStore(on: boolean): Promise { + try { + const { load } = await import('@tauri-apps/plugin-store'); + const store = await load(STORE_FILE); + await store.set(STORE_KEY, on); + await store.save(); + } catch { + // Not under Tauri (tests, vite preview): the localStorage mirror suffices. + } +} + +function createConfirmQuit() { + const initial = mirrored(); + const { subscribe, set: setStore } = writable(initial); + let current = initial; + let interacted = false; + + function apply(on: boolean, user: boolean): Promise { + current = on; + setStore(on); + mirrorLocal(on); + if (!user) return Promise.resolve(); + interacted = true; + return persistStore(on); + } + + return { + subscribe, + /** Resolves once saved: "Don't ask again" quits right after, and must not lose it. */ + set: (on: boolean) => apply(on, true), + toggle: () => void apply(!current, true), + /** Reconcile with the canonical tauri-plugin-store value once Tauri is reachable. */ + async hydrate(): Promise { + try { + const { load } = await import('@tauri-apps/plugin-store'); + const store = await load(STORE_FILE); + const saved = await store.get(STORE_KEY); + if (!interacted && typeof saved === 'boolean') void apply(saved, false); + } catch { + // Store unreachable: keep the mirrored value. + } + } + }; +} + +export const confirmQuit = createConfirmQuit(); + +export interface LiveWork { + terminals: number; + transfers: number; + tunnels: number; + snippetRuns: number; +} + +const LIVE_TUNNEL: TunnelStatusDto['kind'][] = ['connecting', 'up', 'retrying']; + +/** What a quit would cut off. An ended terminal tab is only a last screen. */ +export function countLiveWork( + list: Session[], + sftpSessions: Map, + tunnelStatuses: Map, + run: SnippetRun | null +): LiveWork { + return { + terminals: list.filter( + (s) => s.kind === 'terminal' && (s.status === 'connecting' || s.status === 'connected') + ).length, + // A queued upload or download is as good as running: the core works through them in turn. + transfers: [...sftpSessions.values()].filter( + (s) => s.transfer || s.pending.some((op) => op.kind === 'upload' || op.kind === 'download') + ).length, + tunnels: [...tunnelStatuses.values()].filter((t) => LIVE_TUNNEL.includes(t.kind)).length, + snippetRuns: run?.entries.some((e) => e.pending) ? 1 : 0 + }; +} + +/** "2 terminals, 1 transfer and 1 tunnel"; empty when nothing is live. */ +export function describeLiveWork(work: LiveWork): string { + const parts = ( + [ + [work.terminals, 'terminal'], + [work.transfers, 'transfer'], + [work.tunnels, 'tunnel'], + [work.snippetRuns, 'snippet run'] + ] as const + ) + .filter(([n]) => n > 0) + .map(([n, noun]) => `${n} ${noun}${n === 1 ? '' : 's'}`); + if (parts.length < 2) return parts.join(''); + return `${parts.slice(0, -1).join(', ')} and ${parts.at(-1)}`; +} + +/** The open question's summary, or null when none is open. */ +export const quitPrompt = writable(null); + +/** Opens the question if quitting would cut anything off and the user wants to be asked; + * false means quit now. One question at a time: a second request finds it open. */ +function askToQuit(): boolean { + if (get(quitPrompt) !== null) return true; + if (!get(confirmQuit)) return false; + const summary = describeLiveWork( + countLiveWork(get(sessions), get(sftp), get(tunnels), get(snippetRun)) + ); + if (!summary) return false; + quitPrompt.set(summary); + return true; +} + +/** The window's close button, ⌘⇧W, and the tray's or app menu's Quit, which brings the + * window out first. Not prevented, Tauri destroys the window and the app exits. */ +export async function handleCloseRequested(event: CloseRequestedEvent, win: Window): Promise { + try { + // The tray already hid it: that close keeps the app running. + if (!(await win.isVisible())) return event.preventDefault(); + if (!askToQuit()) return; + event.preventDefault(); + // A taskbar or dock close reaches a minimized window, and the question has to be seen. + await win.unminimize(); + await win.setFocus(); + } catch { + // Never leave a window that cannot be closed: let this one go. + } +} diff --git a/crates/omnyssh-gui/ui/src/lib/stores/snippets.test.ts b/crates/omnyssh-gui/ui/src/lib/stores/snippets.test.ts index 2da877ad..f7ff9884 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/snippets.test.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/snippets.test.ts @@ -15,7 +15,7 @@ function result(partial: Partial): SnippetResult { } describe('snippet run lifecycle', () => { - beforeEach(() => clearRun()); + beforeEach(() => snippetRun.set(null)); it('beginRun seeds one pending entry per host', () => { beginRun('deploy', ['web-1', 'web-2']); @@ -27,12 +27,37 @@ describe('snippet run lifecycle', () => { ]); }); - it('clearRun dismisses the panel', () => { + it('clearRun dismisses a finished run', () => { beginRun('deploy', ['web-1']); + reduceRunResultInto(result({})); clearRun(); expect(get(snippetRun)).toBeNull(); }); + it('clearRun keeps a run with hosts pending out of sight until the last one reports', () => { + beginRun('deploy', ['web-1', 'web-2']); + reduceRunResultInto(result({ hostName: 'web-1' })); + clearRun(); + expect(get(snippetRun)).toMatchObject({ dismissed: true }); + expect(get(snippetRun)?.entries[1].pending).toBe(true); + + reduceRunResultInto(result({ hostName: 'web-2' })); + expect(get(snippetRun)).toBeNull(); + }); + + it('a dismissed run goes once the execute command fails, and a new run starts fresh', () => { + beginRun('deploy', ['web-1']); + clearRun(); + failPendingRun('boom'); + expect(get(snippetRun)).toBeNull(); + + beginRun('deploy', ['web-1']); + clearRun(); + beginRun('other', ['web-2']); + expect(get(snippetRun)?.dismissed).toBeUndefined(); + expect(get(snippetRun)?.snippetName).toBe('other'); + }); + it('failPendingRun fails only the still-pending entries', () => { beginRun('deploy', ['web-1', 'web-2']); reduceRunResultInto({ hostName: 'web-1', snippetName: 'deploy', ok: true, output: 'done' }); diff --git a/crates/omnyssh-gui/ui/src/lib/stores/snippets.ts b/crates/omnyssh-gui/ui/src/lib/stores/snippets.ts index f5c172f1..2d86026b 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/snippets.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/snippets.ts @@ -19,11 +19,19 @@ export interface SnippetResultEntry { export interface SnippetRun { snippetName: string; entries: SnippetResultEntry[]; + /** The panel was closed with hosts still running: kept, out of sight, so a quit + * still asks about it, and dropped once the last host reports. */ + dismissed?: boolean; } -// The active execution, or null when no results panel is open (tech-gui.md §2.2). +// The active execution, or null when there is none (tech-gui.md §2.2). export const snippetRun = writable(null); +// A dismissed run has nothing left to show once no host is pending. +function settle(run: SnippetRun): SnippetRun | null { + return run.dismissed && !run.entries.some((e) => e.pending) ? null : run; +} + /** Seed a run with one pending entry per target host — called the moment execute * fires, so every host shows as pending before its result streams back. */ export function beginRun(snippetName: string, hostNames: string[]): void { @@ -33,9 +41,9 @@ export function beginRun(snippetName: string, hostNames: string[]): void { }); } -/** Dismiss the results panel. */ +/** Dismiss the results panel. The core keeps running whatever is still pending. */ export function clearRun(): void { - snippetRun.set(null); + snippetRun.update((run) => (run ? settle({ ...run, dismissed: true }) : run)); } /** Mark every still-pending entry of the active run as failed. Used when the execute @@ -44,12 +52,12 @@ export function clearRun(): void { export function failPendingRun(errorMessage: string): void { snippetRun.update((run) => run - ? { + ? settle({ ...run, entries: run.entries.map((e) => e.pending ? { hostName: e.hostName, pending: false, ok: false, output: errorMessage } : e ) - } + }) : run ); } @@ -61,12 +69,12 @@ export function failPendingRun(errorMessage: string): void { * on the same host. Pure, so the router and the tests share one definition. */ export function reduceRunResult(run: SnippetRun | null, payload: SnippetResult): SnippetRun | null { if (!run || run.snippetName !== payload.snippetName) return run; - return { + return settle({ ...run, entries: run.entries.map((e) => e.hostName === payload.hostName ? { hostName: e.hostName, pending: false, ok: payload.ok, output: payload.output } : e ) - }; + }); } diff --git a/crates/omnyssh-gui/ui/src/routes/+layout.svelte b/crates/omnyssh-gui/ui/src/routes/+layout.svelte index cb3c3a33..12a8789c 100644 --- a/crates/omnyssh-gui/ui/src/routes/+layout.svelte +++ b/crates/omnyssh-gui/ui/src/routes/+layout.svelte @@ -10,6 +10,8 @@ import { trayBehavior, driveTray } from '$lib/stores/tray'; import { dashboardView } from '$lib/stores/dashboardView'; import { lastError } from '$lib/stores/notifications'; + import { confirmQuit, handleCloseRequested, quitPrompt } from '$lib/stores/quit'; + import QuitConfirm from '$lib/components/QuitConfirm.svelte'; let { children } = $props(); @@ -24,6 +26,7 @@ void refreshInterval.hydrate(); void trayBehavior.hydrate(); void dashboardView.hydrate(); + void confirmQuit.hydrate(); // Force a metric refresh on the user's interval; re-arms when the interval changes. const stopRefresh = driveMetricsRefresh(() => { void refreshMetrics().catch(() => {}); @@ -44,9 +47,25 @@ reloadHosts().catch((err) => lastError.set(err instanceof Error ? err.message : String(err))); }) .catch(() => {}); + // Asks before the window's close ends live work. Off Tauri there is no window. + let stopClose: (() => void) | undefined; + import('@tauri-apps/api/window') + .then(({ getCurrentWindow }) => { + const win = getCurrentWindow(); + return win.onCloseRequested((e) => handleCloseRequested(e, win)); + }) + .then((off) => { + if (disposed) return off(); + stopClose = off; + // The backend's liveness probe reads this before it trusts the page with a close. + (window as { __omnysshAsksOnClose?: boolean }).__omnysshAsksOnClose = true; + }) + .catch(() => {}); return () => { disposed = true; stop?.(); + stopClose?.(); + delete (window as { __omnysshAsksOnClose?: boolean }).__omnysshAsksOnClose; stopRefresh(); stopTray(); }; @@ -54,3 +73,6 @@ {@render children()} +{#if $quitPrompt !== null} + +{/if} From 01abb2697a075ac0b8f1049f7230907c8ab6ef8c Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Fri, 9 Oct 2026 09:45:41 +0400 Subject: [PATCH 06/15] fix(gui): close the active tab with cmd+w on macos --- crates/omnyssh-gui/ui/e2e/quit.spec.ts | 50 +++++++++++++++++++ .../ui/src/lib/components/AppShell.svelte | 15 +++++- .../omnyssh-gui/ui/src/lib/stores/ui.test.ts | 34 ++++++++++++- crates/omnyssh-gui/ui/src/lib/stores/ui.ts | 8 +++ 4 files changed, 105 insertions(+), 2 deletions(-) diff --git a/crates/omnyssh-gui/ui/e2e/quit.spec.ts b/crates/omnyssh-gui/ui/e2e/quit.spec.ts index 013e47d5..8a2910b0 100644 --- a/crates/omnyssh-gui/ui/e2e/quit.spec.ts +++ b/crates/omnyssh-gui/ui/e2e/quit.spec.ts @@ -191,3 +191,53 @@ test('with nothing live a minimized window closes where it is', async ({ page }) await expect(dialog(page)).toHaveCount(0); expect(await calls(page)).not.toContain('plugin:window|unminimize'); }); + +// The Desktop Chrome device reports a Windows user agent: Ctrl+W is the shell's. +test('off macOS Ctrl+W goes to the terminal', async ({ page }) => { + await boot(page); + await openTerminal(page); + await page.locator('.xterm-helper-textarea').focus(); + await page.keyboard.press('Control+w'); + await expect(page.getByRole('button', { name: 'web-1 · terminal', exact: true })).toBeVisible(); +}); + +test.describe('on macOS', () => { + test.use({ + userAgent: + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko)' + }); + + test('⌘W closes the active tab, not the window', async ({ page }) => { + await boot(page); + await openTerminal(page); + await page.locator('.xterm-helper-textarea').focus(); + await page.keyboard.press('Meta+w'); + await expect(page.getByRole('button', { name: 'web-1 · terminal', exact: true })).toHaveCount(0); + await expect(page.locator('.xterm')).toHaveCount(0); + expect(await calls(page)).not.toContain('plugin:window|destroy'); + + // With no tab left it does nothing. + await page.keyboard.press('Meta+w'); + await expect(page.getByText('web-1', { exact: true })).toBeVisible(); + expect(await calls(page)).not.toContain('plugin:window|destroy'); + }); + + test('⌘W leaves the tab alone while a dialog is open', async ({ page }) => { + await boot(page); + await openTerminal(page); + await closeWindow(page); + await expect(dialog(page)).toBeVisible(); + await page.keyboard.press('Meta+w'); + await expect(page.getByRole('button', { name: 'web-1 · terminal', exact: true })).toBeVisible(); + await expect(dialog(page).getByRole('button', { name: 'Keep running in the menu bar instead' })).toBeVisible(); + }); + + test('⌘W leaves the tab alone while the command palette is open', async ({ page }) => { + await boot(page); + await openTerminal(page); + await page.keyboard.press('Meta+k'); + await expect(page.getByRole('dialog', { name: 'Command palette' })).toBeVisible(); + await page.keyboard.press('Meta+w'); + await expect(page.getByRole('button', { name: 'web-1 · terminal', exact: true })).toBeVisible(); + }); +}); diff --git a/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte b/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte index 486500e9..7dd9eb25 100644 --- a/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte +++ b/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte @@ -13,7 +13,13 @@ import PasswordPrompt from '$lib/screens/PasswordPrompt.svelte'; import UpdateBanner from './UpdateBanner.svelte'; import { support } from '$lib/stores/support'; - import { sidebarCollapsed, isCollapseChord } from '$lib/stores/ui'; + import { get } from 'svelte/store'; + import { sidebarCollapsed, isCollapseChord, isCloseTabChord } from '$lib/stores/ui'; + import { activeEntity } from '$lib/stores/activeEntity'; + import { palette } from '$lib/stores/palette'; + import { dialogs } from '$lib/stores/dialogs'; + import { closeSession } from '$lib/stores/navigation'; + import { isMac } from '$lib/platform'; let { children }: { children: Snippet } = $props(); @@ -21,6 +27,13 @@ if (isCollapseChord(e)) { e.preventDefault(); sidebarCollapsed.toggle(); + } else if (isMac && isCloseTabChord(e)) { + // As the tab's own close button; never the window, which has ⌘⇧W. + e.preventDefault(); + // The palette is not a dialog, but it covers the session all the same. + if (get(palette).open) return; + const active = get(activeEntity); + if (get(dialogs).length === 0 && active.kind === 'session') closeSession(active.id); } } diff --git a/crates/omnyssh-gui/ui/src/lib/stores/ui.test.ts b/crates/omnyssh-gui/ui/src/lib/stores/ui.test.ts index 98b45e12..6058becc 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/ui.test.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/ui.test.ts @@ -1,7 +1,13 @@ // @vitest-environment jsdom import { beforeEach, describe, expect, it, vi } from 'vitest'; import { get } from 'svelte/store'; -import { isCollapseChord, isGroupHotkey, isPaletteChord, isRefreshHotkey } from './ui'; +import { + isCloseTabChord, + isCollapseChord, + isGroupHotkey, + isPaletteChord, + isRefreshHotkey +} from './ui'; import { dialogs } from './dialogs'; import { palette } from './palette'; @@ -123,6 +129,32 @@ describe('palette chord (⌘K / Ctrl+K)', () => { }); }); +describe('close-tab chord (⌘W)', () => { + const chord = (init: KeyboardEventInit) => isCloseTabChord(new KeyboardEvent('keydown', init)); + + it('matches ⌘W, by key or by physical key', () => { + expect(chord({ key: 'w', metaKey: true })).toBe(true); + expect(chord({ key: 'W', metaKey: true })).toBe(true); + // AZERTY puts W where QWERTY has Z. + expect(chord({ key: 'w', code: 'KeyZ', metaKey: true })).toBe(true); + // A Greek layout names the key ς. + expect(chord({ key: '\u03c2', code: 'KeyW', metaKey: true })).toBe(true); + }); + + it('leaves the W key to a Latin layout that puts another character there', () => { + expect(chord({ key: 'z', code: 'KeyW', metaKey: true })).toBe(false); // AZERTY ⌘Z + expect(chord({ key: ',', code: 'KeyW', metaKey: true })).toBe(false); // Dvorak ⌘, + }); + + it('leaves Ctrl+W to the shell, ⌘⇧W to the window, and a held chord at one tab', () => { + expect(chord({ key: 'w', ctrlKey: true })).toBe(false); + expect(chord({ key: 'w', metaKey: true, shiftKey: true })).toBe(false); + expect(chord({ key: 'w', metaKey: true, altKey: true })).toBe(false); + expect(chord({ key: 'w', metaKey: true, repeat: true })).toBe(false); + expect(chord({ key: 'w' })).toBe(false); + }); +}); + describe('refresh hotkey (r)', () => { const hot = (init: KeyboardEventInit) => isRefreshHotkey(new KeyboardEvent('keydown', init)); diff --git a/crates/omnyssh-gui/ui/src/lib/stores/ui.ts b/crates/omnyssh-gui/ui/src/lib/stores/ui.ts index 46b6b13a..9ccbbaf4 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/ui.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/ui.ts @@ -98,6 +98,14 @@ export function isPaletteChord(e: KeyboardEvent): boolean { return (e.metaKey || e.ctrlKey) && (e.key === 'k' || e.key === 'K'); } +/** The macOS close-tab chord: ⌘W. The app menu leaves it free; off macOS Ctrl+W stays + * the shell's delete-word. The physical key only when the layout names no ASCII + * character there: on AZERTY it is ⌘Z, on Dvorak ⌘, — but on Greek it is ⌘W. */ +export function isCloseTabChord(e: KeyboardEvent): boolean { + if (e.repeat || e.altKey || e.ctrlKey || e.shiftKey || e.isComposing || !e.metaKey) return false; + return e.key === 'w' || e.key === 'W' || (e.code === 'KeyW' && !/^[\x21-\x7e]$/.test(e.key)); +} + /** A bare-letter dashboard hotkey (mirrors the TUI): no modifier, ignored while typing in * an editable surface so it never eats a keystroke, and while any modal or the command * palette is open so it never changes the view behind it. Terminal input is never From 7c3b8e1fd38bd221ab93de794ebad0adaae81f49 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Fri, 9 Oct 2026 09:44:29 +0400 Subject: [PATCH 07/15] fix(gui): hide host addresses in error text in streamer mode --- crates/omnyssh-gui/ui/e2e/streamer.spec.ts | 91 +++++++++ .../ui/src/lib/components/StatusBar.svelte | 4 +- .../ui/src/lib/screens/HostEditor.svelte | 3 +- .../src/lib/screens/PassphrasePrompt.svelte | 3 +- .../ui/src/lib/screens/SftpPane.svelte | 3 +- .../ui/src/lib/screens/SftpView.svelte | 5 +- .../ui/src/lib/screens/SnippetResults.svelte | 5 +- .../ui/src/lib/screens/serverCard.ts | 5 +- .../ui/src/lib/stores/streamer.test.ts | 172 +++++++++++++++++- .../omnyssh-gui/ui/src/lib/stores/streamer.ts | 90 ++++++++- 10 files changed, 368 insertions(+), 13 deletions(-) create mode 100644 crates/omnyssh-gui/ui/e2e/streamer.spec.ts diff --git a/crates/omnyssh-gui/ui/e2e/streamer.spec.ts b/crates/omnyssh-gui/ui/e2e/streamer.spec.ts new file mode 100644 index 00000000..b1dd101b --- /dev/null +++ b/crates/omnyssh-gui/ui/e2e/streamer.spec.ts @@ -0,0 +1,91 @@ +import { expect, test, type Page } from '@playwright/test'; + +// Streamer mode (tech-gui.md §4.1, 2026-10-09 note). Tauri is stubbed at +// `__TAURI_INTERNALS__` (§6.4); `__fire` plays a core event, so an error carrying a +// real address can be sent the way the backend would send it. +const HOSTS = [ + { + name: 'db', + hostname: 'db.example.com', + user: 'admin', + port: 22, + tags: [], + source: 'manual', + hasKey: true, + monitoring: 'ssh', + localForwards: [{ bindPort: 8080, remoteHost: '10.20.3.7', remotePort: 5432 }], + tunnelAutostart: false, + forwardAgent: false + } +]; + +async function boot(page: Page, streamer: boolean): Promise { + await page.addInitScript( + ({ hosts, streamer }) => { + localStorage.setItem('omnyssh-streamer-mode', String(streamer)); + let cbid = 0; + const listeners: Record = {}; + const win = window as unknown as Record; + win.__fire = (event: string, payload: unknown): void => { + for (const id of listeners[event] ?? []) { + const cb = win[`__cb${id}`] as ((e: unknown) => void) | undefined; + cb?.({ event, id, payload }); + } + }; + (win as { __TAURI_INTERNALS__: unknown }).__TAURI_INTERNALS__ = { + invoke: (cmd: string, args: Record) => { + switch (cmd) { + case 'list_hosts': + return Promise.resolve(hosts); + case 'plugin:event|listen': { + const { event, handler } = args as { event: string; handler: number }; + (listeners[event] ||= []).push(handler); + return Promise.resolve(cbid); + } + default: + return Promise.resolve(null); + } + }, + transformCallback: (cb: unknown) => { + const id = ++cbid; + win[`__cb${id}`] = cb; + return id; + }, + unregisterCallback: (id: number) => { + delete win[`__cb${id}`]; + } + }; + }, + { hosts: HOSTS, streamer } + ); + await page.goto('/'); + await expect(page.getByText('1 host', { exact: true })).toBeVisible(); +} + +const TUNNEL_ERROR = "Tunnel to 'db': 8080:10.20.3.7:5432 could not be opened: db.example.com refused"; + +async function fireError(page: Page, message: string): Promise { + await page.evaluate( + (message) => (window as unknown as { __fire: (e: string, p: unknown) => void }).__fire('error', { message }), + message + ); +} + +test('streamer mode keeps real addresses out of the status bar and its tooltip', async ({ page }) => { + await boot(page, true); + await fireError(page, TUNNEL_ERROR); + + const status = page.locator('footer').getByText("Tunnel to 'db'"); + await expect(status).toBeVisible(); + for (const real of ['10.20.3.7', 'db.example.com']) { + await expect(status).not.toContainText(real); + await expect(status).not.toHaveAttribute('title', new RegExp(real.replaceAll('.', '\\.'))); + } + await expect(status).toContainText(':5432 could not be opened'); +}); + +test('with streamer mode off the status bar shows the error as sent', async ({ page }) => { + await boot(page, false); + await fireError(page, TUNNEL_ERROR); + await expect(page.locator('footer').getByText(TUNNEL_ERROR)).toHaveAttribute('title', TUNNEL_ERROR); +}); diff --git a/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte b/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte index cc71a69e..ada65c7e 100644 --- a/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte +++ b/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte @@ -4,6 +4,7 @@ // (§4.1); colour lives only in the status dots, per the brandbook. import { lastError } from '$lib/stores/notifications'; import { hostSummary } from '$lib/stores/hostSummary'; + import { maskText } from '$lib/stores/streamer'; import { StatusDot } from '$lib/theme'; @@ -11,8 +12,9 @@ class="col-span-2 col-start-1 row-start-2 flex items-center justify-between gap-4 border-t border-default bg-surface px-5 py-2 text-xs text-muted" > {#if $lastError} + {@const text = $maskText($lastError)} - {$lastError} + {text} {:else} Ready {/if} diff --git a/crates/omnyssh-gui/ui/src/lib/screens/HostEditor.svelte b/crates/omnyssh-gui/ui/src/lib/screens/HostEditor.svelte index 2954162b..b5cbeaca 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/HostEditor.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/HostEditor.svelte @@ -9,6 +9,7 @@ import Modal from '$lib/components/Modal.svelte'; import Select from '$lib/components/Select.svelte'; import { isWindows } from '$lib/platform'; + import { maskText } from '$lib/stores/streamer'; import { emptyForwardRow, formToInput, type HostFormFields } from './hostForm'; let { @@ -302,7 +303,7 @@
{#if error} -

{error}

+

{$maskText(error)}

{/if}
diff --git a/crates/omnyssh-gui/ui/src/lib/screens/PassphrasePrompt.svelte b/crates/omnyssh-gui/ui/src/lib/screens/PassphrasePrompt.svelte index 0d5da328..6abfcad7 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/PassphrasePrompt.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/PassphrasePrompt.svelte @@ -4,6 +4,7 @@ import Modal from '$lib/components/Modal.svelte'; import { Button, Icon } from '$lib/theme'; import { passphrasePrompt, settlePassphrase } from '$lib/stores/passphrase'; + import { maskText } from '$lib/stores/streamer'; import { unlockIdentity } from '$lib/ipc/commands'; const message = (e: unknown): string => (e instanceof Error ? e.message : String(e)); @@ -103,7 +104,7 @@ session that stopped on this key has to be opened again.

{#if error} -

{error}

+

{$maskText(error)}

{/if}
diff --git a/crates/omnyssh-gui/ui/src/lib/screens/SftpPane.svelte b/crates/omnyssh-gui/ui/src/lib/screens/SftpPane.svelte index 9a20fd8a..32eac919 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/SftpPane.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/SftpPane.svelte @@ -11,6 +11,7 @@ import { Icon } from '$lib/theme'; import type { FileEntryDto } from '$lib/bindings'; import { formatBytes, formatDate, type Pane, type PaneSide } from '$lib/stores/sftp'; + import { maskText } from '$lib/stores/streamer'; let { title, @@ -75,7 +76,7 @@
{#if pane.error} -

{pane.error}

+

{$maskText(pane.error)}

{:else if pane.loading && pane.entries.length === 0}

Loading…

{:else if pane.entries.length === 0} diff --git a/crates/omnyssh-gui/ui/src/lib/screens/SftpView.svelte b/crates/omnyssh-gui/ui/src/lib/screens/SftpView.svelte index 3dd256c4..7858d605 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/SftpView.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/SftpView.svelte @@ -29,6 +29,7 @@ type PaneSide } from '$lib/stores/sftp'; import { lastError } from '$lib/stores/notifications'; + import { maskText } from '$lib/stores/streamer'; import { dropPoint, isMac, isWindows } from '$lib/platform'; import { sftpOpen, @@ -593,7 +594,7 @@ {#if openError}

Could not open SFTP on {session.hostName}

-

{openError}

+

{$maskText(openError)}

{:else if !view}
@@ -762,7 +763,7 @@ class="shrink-0 border-t border-default px-4 py-2 text-xs {view.error === CANCELLED ? 'text-muted' : 'text-status-crit'}" > - {view.error} + {$maskText(view.error)}
{/if} {/if} diff --git a/crates/omnyssh-gui/ui/src/lib/screens/SnippetResults.svelte b/crates/omnyssh-gui/ui/src/lib/screens/SnippetResults.svelte index 2256a094..63c1b329 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/SnippetResults.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/SnippetResults.svelte @@ -5,6 +5,7 @@ import { StatusDot, Button, type Status } from '$lib/theme'; import Modal from '$lib/components/Modal.svelte'; import { snippetRun, clearRun, type SnippetResultEntry } from '$lib/stores/snippets'; + import { maskText } from '$lib/stores/streamer'; function dot(entry: SnippetResultEntry): Status { if (entry.pending) return 'unknown'; @@ -35,10 +36,12 @@ {#if entry.pending}

Running…

{:else if entry.output.trim()} + 
{entry.output}
+ : 'text-status-crit'}">{entry.ok ? entry.output : $maskText(entry.output)} {:else}

(no output)

{/if} diff --git a/crates/omnyssh-gui/ui/src/lib/screens/serverCard.ts b/crates/omnyssh-gui/ui/src/lib/screens/serverCard.ts index eb3d6528..36f14b19 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/serverCard.ts +++ b/crates/omnyssh-gui/ui/src/lib/screens/serverCard.ts @@ -20,7 +20,7 @@ import { statuses } from '$lib/stores/statuses'; import { metrics } from '$lib/stores/metrics'; import { services, type HostServices } from '$lib/stores/services'; import { tunnels } from '$lib/stores/tunnels'; -import { displayHostname } from '$lib/stores/streamer'; +import { displayHostname, REVEALS_NOTHING } from '$lib/stores/streamer'; import type { CardSort } from '$lib/stores/dashboardView'; import { bracketed } from './hostForm'; @@ -88,9 +88,6 @@ export function deriveTunnel(host: HostDto, status: TunnelStatusDto | undefined) } } -// Loopback and wildcard addresses reveal nothing, and read wrong disguised. -const REVEALS_NOTHING = /^(localhost|127(\.\d{1,3}){3}|::1|\*|0\.0\.0\.0|::)$/i; - /** An address as a forward shows it: masked in streamer mode like any host address. */ function shown(address: string, streamerOn: boolean): string { return REVEALS_NOTHING.test(address) ? address : displayHostname(address, streamerOn); diff --git a/crates/omnyssh-gui/ui/src/lib/stores/streamer.test.ts b/crates/omnyssh-gui/ui/src/lib/stores/streamer.test.ts index cfe4af34..97e6867e 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/streamer.test.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/streamer.test.ts @@ -1,7 +1,8 @@ // @vitest-environment jsdom import { beforeEach, describe, expect, it, vi } from 'vitest'; import { get } from 'svelte/store'; -import { maskHostname, displayHostname } from './streamer'; +import type { HostDto } from '$lib/bindings'; +import { addressMasker, displayHostname, knownAddresses, maskHostname } from './streamer'; // Streamer mode disguises host addresses on screen (tech-gui.md §4.3). The mask must be // deterministic (same host → same disguise all recording), never leak the real value, @@ -44,6 +45,175 @@ describe('address masking', () => { }); }); +function host(hostname: string, localForwards: HostDto['localForwards'] = []): HostDto { + return { + name: hostname, + hostname, + user: 'deploy', + port: 22, + tags: [], + source: 'manual', + hasKey: false, + monitoring: 'ssh', + localForwards, + tunnelAutostart: false, + forwardAgent: false + }; +} + +// Error text reaches the status bar verbatim from the core, so the masker has to find +// addresses in the core's own message shapes and leave everything else readable. +describe('masking addresses in text', () => { + const HOSTS = [ + host('db.example.com', [{ bindPort: 8080, remoteHost: '10.20.3.7', remotePort: 5432 }]), + host('2001:db8::1'), + host('nas'), + host('203.0.113.7') + ]; + const mask = addressMasker(knownAddresses(HOSTS)); + const m = maskHostname; + + it('collects hostnames and forward ends, deduped, longest first, without loopback', () => { + const list = knownAddresses([ + ...HOSTS, + host('DB.Example.com', [{ bindAddress: '0.0.0.0', bindPort: 1, remoteHost: 'localhost', remotePort: 2 }]), + host(' db.example.com '), + host(' ') + ]); + expect(list).toEqual(['db.example.com', 'DB.Example.com', '2001:db8::1', '203.0.113.7', '10.20.3.7', 'nas']); + }); + + it('gives each case spelling of a dotted name its own card disguise', () => { + const list = [host('db.example.com'), host('DB.Example.COM')]; + expect(knownAddresses(list)).toEqual(['db.example.com', 'DB.Example.COM']); + const words = addressMasker(knownAddresses(list)); + for (const h of list) expect(words(`${h.hostname} refused`)).toBe(`${m(h.hostname)} refused`); + // A spelling no card has takes the first one seen. + expect(words('Db.Example.Com refused')).toBe(`${m('db.example.com')} refused`); + }); + + it('masks a dotted ProxyJump hop no host knows, in the core wordings', () => { + const hop = 'jump.corp.example.com'; + const cases = [ + `ProxyJump via '${hop}' failed: SSH connection timed out (10 s)`, + `connecting via '${hop}' failed: refused`, + `unusable ProxyJump hop '${hop}:abc': invalid port`, + `ProxyJump cycle detected at '${hop}'`, + `ProxyJump via 'bastion' failed: open tunnel to ${hop}:22: refused` + ]; + for (const text of cases) expect(mask(text)).toBe(text.replace(hop, m(hop))); + }); + + it('keeps a single-label hop readable and a known hop or literal as its card shows it', () => { + const plain = "ProxyJump via 'bastion' failed: refused"; + expect(mask(plain)).toBe(plain); + expect(mask("ProxyJump via 'DB.EXAMPLE.COM' failed")).toBe(`ProxyJump via '${m('db.example.com')}' failed`); + expect(mask("ProxyJump via 'nas' failed")).toBe(`ProxyJump via '${m('nas')}' failed`); + expect(mask('open tunnel to 10.20.3.7:22: refused')).toBe(`open tunnel to ${m('10.20.3.7')}:22: refused`); + }); + + it('masks the whole of an IPv6 literal with an embedded IPv4 tail', () => { + for (const ip of ['2001:db8::192.0.2.1', '64:ff9b::198.51.100.7', '1:2:3:4:5:6:1.2.3.4', '::ffff:203.0.113.9']) { + expect(mask(`via ${ip} refused`)).toBe(`via ${m(ip)} refused`); + expect(mask(`[${ip}]:2222`)).toBe(`[${m(ip)}]:2222`); + } + expect(mask('fe80::1 and 12:30:45 and 10.0.0.1.5')).toBe(`${m('fe80::1')} and 12:30:45 and 10.0.0.1.5`); + }); + + it('masks a changed host key message, on port 22 and off it', () => { + const fp = 'SHA256:uNiVztksCsDhcc0u9e8BujQXVUpKZIDTMczCvj3tD2s'; + const file = '/home/me/.ssh/known_hosts'; + const on22 = + `Host key of db.example.com has changed: its ED25519 key ${fp} does not match the one ` + + `saved in ${file}. Remove it with ssh-keygen -R "db.example.com" -f "${file}".`; + expect(mask(on22)).toBe(on22.replaceAll('db.example.com', m('db.example.com'))); + const off22 = `Host key of 203.0.113.7 port 2222 has changed. ssh-keygen -R "[203.0.113.7]:2222" -f "${file}"`; + expect(mask(off22)).toBe( + `Host key of ${m('203.0.113.7')} port 2222 has changed. ssh-keygen -R "[${m('203.0.113.7')}]:2222" -f "${file}"` + ); + }); + + it('masks the connect and tunnel messages, keeping ports and brackets', () => { + expect(mask('open tunnel to db.example.com:22: refused')).toBe(`open tunnel to ${m('db.example.com')}:22: refused`); + expect(mask("Tunnel to 'db': 8080:10.20.3.7:5432 could not be opened: refused")).toBe( + `Tunnel to 'db': 8080:${m('10.20.3.7')}:5432 could not be opened: refused` + ); + expect(mask('0.0.0.0:8080:[2001:db8::1]:443')).toBe(`0.0.0.0:8080:[${m('2001:db8::1')}]:443`); + expect(mask('no answer from 198.51.100.4:22')).toBe(`no answer from ${m('198.51.100.4')}:22`); + }); + + it('masks unknown IP literals and the domain of user@domain', () => { + expect(mask('via 192.168.1.10, fe80::1 and 2001:0db8:0:0:0:0:0:2')).toBe( + `via ${m('192.168.1.10')}, ${m('fe80::1')} and ${m('2001:0db8:0:0:0:0:0:2')}` + ); + expect(mask("unusable ProxyJump hop 'ops@bastion.corp.io:2200'")).toBe( + `unusable ProxyJump hop 'ops@${m('bastion.corp.io')}:2200'` + ); + expect(mask('admin@db.example.com')).toBe(`admin@${m('db.example.com')}`); + }); + + it('gives a known name exactly the disguise its card shows, whatever its case', () => { + for (const h of HOSTS) expect(mask(h.hostname)).toBe(m(h.hostname)); + expect(mask('DB.EXAMPLE.COM refused')).toBe(`${m('db.example.com')} refused`); + }); + + it('never masks a disguise again', () => { + const once = mask('db.example.com 10.20.3.7 2001:db8::1'); + expect(once).toBe(`${m('db.example.com')} ${m('10.20.3.7')} ${m('2001:db8::1')}`); + expect(once).not.toContain('10.20.3.7'); + }); + + it('leaves loopback, wildcards, fingerprints, versions, times, paths and MACs alone', () => { + const plain = [ + 'listen on 127.0.0.1:8080, [::1]:22, 0.0.0.0:80, :: and *:443 failed', + 'key SHA256:nas+Zm9vYmFy/nas and ssh-keygen -lf /etc/ssh/ssh_host_ed25519_key.pub', + 'OmnySSH 1.1.4 at 2026-10-09 12:30:45, file id_ed25519.pub in /srv/10.0.0.5/logs', + 'link aa:bb:cc:dd:ee:ff up; std::io error; github.com is fine' + ]; + for (const text of plain) expect(mask(text)).toBe(text); + }); + + it('masks a single-label name only as a whole token', () => { + expect(mask('nas: refused')).toBe(`${m('nas')}: refused`); + expect(mask('nasty nas.local nas-2 synas')).toBe('nasty nas.local nas-2 synas'); + }); + + it('matches a single-label name only in its stored spelling, dotted names and IPv6 in any', () => { + const words = addressMasker(knownAddresses([host('backup'), host('server'), host('Vault'), ...HOSTS])); + const plain = ['sshd config validation failed. Backup restored.', 'copy public key to Server', 'vault']; + for (const text of plain) expect(words(text)).toBe(text); + expect(words('pwsudo@backup:22 and copy to server')).toBe(`pwsudo@${m('backup')}:22 and copy to ${m('server')}`); + expect(words('Vault: refused')).toBe(`${m('Vault')}: refused`); + expect(words('2001:DB8::1 refused')).toBe(`${m('2001:db8::1')} refused`); + }); + + it('keeps each spelling of a single-label name, so each gets its own card disguise', () => { + const list = [host('backup'), host('Backup')]; + expect(knownAddresses(list)).toEqual(['backup', 'Backup']); + const words = addressMasker(knownAddresses(list)); + for (const h of list) expect(words(h.hostname)).toBe(m(h.hostname)); + }); + + it('masks only literals when no host is known', () => { + expect(addressMasker([])('10.0.0.9 and nas')).toBe(`${m('10.0.0.9')} and nas`); + }); +}); + +describe('maskText store', () => { + it('hands text back as is while streamer mode is off, and masks once it is on', async () => { + vi.resetModules(); + const { maskText, streamerMode } = await import('./streamer'); + const { hosts } = await import('./hosts'); + hosts.set([host('db.example.com')]); + streamerMode.set(false); + expect(get(maskText)('db.example.com 10.0.0.9')).toBe('db.example.com 10.0.0.9'); + streamerMode.set(true); + expect(get(maskText)('db.example.com 10.0.0.9')).toBe( + `${maskHostname('db.example.com')} ${maskHostname('10.0.0.9')}` + ); + }); +}); + // Persistence mirrors the sidebar-collapse pref: a fake Tauri store, a fresh module per // test to reset the singleton, and the localStorage mirror seeding the initial value. const backend = { get: vi.fn(), set: vi.fn(), save: vi.fn() }; diff --git a/crates/omnyssh-gui/ui/src/lib/stores/streamer.ts b/crates/omnyssh-gui/ui/src/lib/stores/streamer.ts index bd51c3e1..eeb8057c 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/streamer.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/streamer.ts @@ -1,4 +1,6 @@ -import { writable } from 'svelte/store'; +import { derived, writable } from 'svelte/store'; +import type { HostDto } from '$lib/bindings'; +import { hosts } from './hosts'; // Streamer mode: a privacy pref that swaps real host addresses in the UI for // deterministic fake-but-realistic ones, so IPs never appear on screen while recording. @@ -137,3 +139,89 @@ export function maskHostname(hostname: string): string { export function displayHostname(hostname: string, streamerOn: boolean): string { return streamerOn ? maskHostname(hostname) : hostname; } + +// Loopback and wildcard addresses reveal nothing, and read wrong disguised. +export const REVEALS_NOTHING = /^(localhost|127(\.\d{1,3}){3}|::1|\*|0\.0\.0\.0|::)$/i; + +/** `address` disguised like a host address, unless it reveals nothing. */ +export function maskAddress(address: string): string { + return REVEALS_NOTHING.test(address.trim()) ? address : maskHostname(address); +} + +// --- Masking free text ------------------------------------------------------------ + +// A name with no dot or colon (`backup`) is also an ordinary word, so it is matched only +// in its exact stored spelling: "Backup restored." stays readable. +const singleLabel = (address: string): boolean => !/[.:]/.test(address); + +/** Every address the host list knows (hostnames and forward ends), trimmed, deduped + * exactly so each spelling keeps its own card's disguise, in first-seen order, and + * longest first so a name is never cut short by a shorter one it starts with. */ +export function knownAddresses(list: HostDto[]): string[] { + const seen = new Set(); + for (const h of list) { + const ends = h.localForwards.flatMap((f) => [f.remoteHost, f.bindAddress ?? '']); + for (const raw of [h.hostname, ...ends]) { + const a = raw.trim(); + if (a && !REVEALS_NOTHING.test(a)) seen.add(a); + } + } + return [...seen].sort((a, b) => b.length - a.length); +} + +const OCTET = String.raw`(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)`; +const IPV4 = String.raw`${OCTET}(?:\.${OCTET}){3}`; +const HEX = '[0-9a-fA-F]{1,4}'; +const HEXES = `${HEX}(?::${HEX}){0,6}`; +// All eight groups, or compressed with `::`: a time (12:30:45), a MAC or a forward's +// `port:host:port` never reads as one. An embedded IPv4 tail is tried first, or the match +// would stop short of it and leave the real prefix. +const IPV6 = + `(?:${HEX}:){6}${IPV4}|(?:${HEXES})?::(?:${HEX}:){0,5}${IPV4}|` + + `(?:${HEX}:){7}${HEX}|(?:${HEXES})?::(?:${HEXES})?`; +// A token stands alone: not inside a word, a path or a base64 fingerprint. A dot ends it +// only when no word follows, so `10.0.0.1.5` is not `10.0.0.1` and `nas.local` not `nas`. +const START = String.raw`(^|[^\w.+/\\-])`; +const END = String.raw`(?=$|[^\w.+-]|\.(?!\w))`; +const DOMAIN = String.raw`[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)+`; +// Where the core names a ProxyJump hop or a tunnelled target, which no host entry may +// know: a dotted name there is masked, a single-label alias stays readable. +const HOP = `ProxyJump (?:via|hop|cycle detected at) '|connecting via '|open tunnel to `; + +const escapeRegExp = (s: string): string => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +// Either case without the `i` flag, which would fold single-label names too. +const caseless = (s: string): string => s.replace(/[a-z]/gi, (c) => `[${c.toLowerCase()}${c.toUpperCase()}]`); + +/** Disguises every address in `text`: the known names (each to the exact disguise its + * card shows; single-label ones only as stored, the rest in any case, an unknown case + * variant as the first spelling seen), IPv4 and IPv6 literals, the domain of a + * `user@domain`, and a dotted name where the core names a ProxyJump hop. Ports, + * brackets and quotes around an address stay, and so do loopback and wildcards, + * fingerprints, versions and paths. One pass, so no disguise is masked again. No + * lookbehind, which older WKWebView rejects: the text before a token is captured and + * written back. */ +export function addressMasker(known: readonly string[]): (text: string) => string { + const exact = new Set(known); + const firstSeen = new Map(); + for (const k of known) if (!singleLabel(k) && !firstSeen.has(k.toLowerCase())) firstSeen.set(k.toLowerCase(), k); + // One caseless pattern per dotted name; which spelling it was is settled after the match. + const names = known + .filter((k) => singleLabel(k) || firstSeen.get(k.toLowerCase()) === k) + .map((k) => (singleLabel(k) ? escapeRegExp(k) : caseless(escapeRegExp(k)))); + const token = [...names, IPV4, IPV6].join('|'); + // Known names and literals come before DOMAIN after a hop, so they keep their disguise. + const re = new RegExp(`${START}(${token})${END}|(${HOP})(${token}|${DOMAIN})${END}|@(${DOMAIN})${END}`, 'g'); + const disguise = (a: string): string => maskAddress(exact.has(a) ? a : (firstSeen.get(a.toLowerCase()) ?? a)); + return (text) => + text.replace(re, (_, pre?: string, address?: string, hop?: string, hopAddress?: string, domain?: string) => { + if (address !== undefined) return `${pre}${disguise(address)}`; + if (hopAddress !== undefined) return `${hop}${disguise(hopAddress)}`; + return `@${maskAddress(domain ?? '')}`; + }); +} + +/** Masks the addresses in error text while streamer mode is on, else returns it as is. + * Applied at render, so flipping the mode re-renders what is already on screen. */ +export const maskText = derived([streamerMode, hosts], ([on, list]) => + on ? addressMasker(knownAddresses(list)) : (text: string) => text +); From 88c7e26bc5ebb905249e58c143840b4178fcbc90 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Fri, 9 Oct 2026 09:46:46 +0400 Subject: [PATCH 08/15] feat(gui): toggle streamer mode with cmd+shift+s or ctrl+shift+s --- crates/omnyssh-gui/ui/e2e/streamer.spec.ts | 21 +++++++++- crates/omnyssh-gui/ui/e2e/terminal.spec.ts | 11 ++++++ .../ui/src/lib/components/AppShell.svelte | 6 ++- .../ui/src/lib/screens/TerminalView.svelte | 4 ++ .../omnyssh-gui/ui/src/lib/stores/ui.test.ts | 39 ++++++++++++++++++- crates/omnyssh-gui/ui/src/lib/stores/ui.ts | 8 ++++ 6 files changed, 85 insertions(+), 4 deletions(-) diff --git a/crates/omnyssh-gui/ui/e2e/streamer.spec.ts b/crates/omnyssh-gui/ui/e2e/streamer.spec.ts index b1dd101b..4cf99fd2 100644 --- a/crates/omnyssh-gui/ui/e2e/streamer.spec.ts +++ b/crates/omnyssh-gui/ui/e2e/streamer.spec.ts @@ -75,7 +75,7 @@ test('streamer mode keeps real addresses out of the status bar and its tooltip', await boot(page, true); await fireError(page, TUNNEL_ERROR); - const status = page.locator('footer').getByText("Tunnel to 'db'"); + const status = page.getByRole('contentinfo').getByText("Tunnel to 'db'"); await expect(status).toBeVisible(); for (const real of ['10.20.3.7', 'db.example.com']) { await expect(status).not.toContainText(real); @@ -87,5 +87,22 @@ test('streamer mode keeps real addresses out of the status bar and its tooltip', test('with streamer mode off the status bar shows the error as sent', async ({ page }) => { await boot(page, false); await fireError(page, TUNNEL_ERROR); - await expect(page.locator('footer').getByText(TUNNEL_ERROR)).toHaveAttribute('title', TUNNEL_ERROR); + await expect(page.getByRole('contentinfo').getByText(TUNNEL_ERROR)).toHaveAttribute('title', TUNNEL_ERROR); +}); + +test('Ctrl+Shift+S toggles streamer mode and re-masks what is already on screen', async ({ page }) => { + await boot(page, false); + await fireError(page, TUNNEL_ERROR); + const statusBar = page.getByRole('contentinfo'); + await expect(statusBar).toContainText('10.20.3.7'); + + await page.keyboard.press('Control+Shift+S'); + await expect(statusBar).toContainText("Tunnel to 'db'"); + await expect(statusBar).not.toContainText('10.20.3.7'); + + // Typing in a field does not hold it back. + await page.getByRole('button', { name: 'Search hosts' }).click(); + await page.getByRole('textbox', { name: 'Search hosts' }).press('Control+Shift+S'); + await expect(statusBar).toContainText('10.20.3.7'); + await expect(page.getByRole('textbox', { name: 'Search hosts' })).toHaveValue(''); }); diff --git a/crates/omnyssh-gui/ui/e2e/terminal.spec.ts b/crates/omnyssh-gui/ui/e2e/terminal.spec.ts index fc49245a..40c8517a 100644 --- a/crates/omnyssh-gui/ui/e2e/terminal.spec.ts +++ b/crates/omnyssh-gui/ui/e2e/terminal.spec.ts @@ -379,6 +379,17 @@ test('Ctrl+Shift+C with nothing selected copies nothing', async ({ page }) => { expect(await writes(page)).toEqual([]); }); +test('Ctrl+Shift+S toggles streamer mode and sends the shell no ^S', async ({ page }) => { + await bootWithClipboard(page); + await page.locator('.xterm-helper-textarea').focus(); + + await page.keyboard.press('Control+Shift+S'); + await expect + .poll(() => page.evaluate(() => localStorage.getItem('omnyssh-streamer-mode'))) + .toBe('true'); + expect(await writes(page)).toEqual([]); +}); + // WebKitGTK under a Russian layout reports keyCode 0 for letter keys, which is also // what a synthetic keydown carries unless told otherwise — so this is the key event // xterm gets there: without the fallback, Ctrl+C would send nothing at all. diff --git a/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte b/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte index 7dd9eb25..22e17c55 100644 --- a/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte +++ b/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte @@ -14,12 +14,13 @@ import UpdateBanner from './UpdateBanner.svelte'; import { support } from '$lib/stores/support'; import { get } from 'svelte/store'; - import { sidebarCollapsed, isCollapseChord, isCloseTabChord } from '$lib/stores/ui'; + import { sidebarCollapsed, isCollapseChord, isCloseTabChord, isStreamerChord } from '$lib/stores/ui'; import { activeEntity } from '$lib/stores/activeEntity'; import { palette } from '$lib/stores/palette'; import { dialogs } from '$lib/stores/dialogs'; import { closeSession } from '$lib/stores/navigation'; import { isMac } from '$lib/platform'; + import { streamerMode } from '$lib/stores/streamer'; let { children }: { children: Snippet } = $props(); @@ -34,6 +35,9 @@ if (get(palette).open) return; const active = get(activeEntity); if (get(dialogs).length === 0 && active.kind === 'session') closeSession(active.id); + } else if (isStreamerChord(e)) { + e.preventDefault(); + streamerMode.toggle(); } } diff --git a/crates/omnyssh-gui/ui/src/lib/screens/TerminalView.svelte b/crates/omnyssh-gui/ui/src/lib/screens/TerminalView.svelte index 8a09e600..91a59232 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/TerminalView.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/TerminalView.svelte @@ -18,6 +18,7 @@ import { lastError } from '$lib/stores/notifications'; import { dialogs } from '$lib/stores/dialogs'; import { palette } from '$lib/stores/palette'; + import { isStreamerChord } from '$lib/stores/ui'; import { activeEntity } from '$lib/stores/activeEntity'; import { terminalOpen, @@ -183,6 +184,9 @@ // reaches the shell. Returning false only keeps xterm out of it; the default is // ours to stop. The write happens inside the keydown, which WebKit requires. term.attachCustomKeyEventHandler((e) => { + // The streamer chord belongs to the window (AppShell), never to the shell, where + // a ^S would stop its output. + if (isStreamerChord(e)) return false; if (isCopyShortcut(e, isMac)) { e.preventDefault(); if (term?.hasSelection()) { diff --git a/crates/omnyssh-gui/ui/src/lib/stores/ui.test.ts b/crates/omnyssh-gui/ui/src/lib/stores/ui.test.ts index 6058becc..b7b1d1ed 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/ui.test.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/ui.test.ts @@ -6,7 +6,8 @@ import { isCollapseChord, isGroupHotkey, isPaletteChord, - isRefreshHotkey + isRefreshHotkey, + isStreamerChord } from './ui'; import { dialogs } from './dialogs'; import { palette } from './palette'; @@ -155,6 +156,42 @@ describe('close-tab chord (⌘W)', () => { }); }); +describe('streamer chord (⌘⇧S / Ctrl+Shift+S)', () => { + const chord = (init: KeyboardEventInit) => isStreamerChord(new KeyboardEvent('keydown', init)); + + it('matches ⌘⇧S and Ctrl+Shift+S, either case', () => { + expect(chord({ key: 'S', code: 'KeyS', metaKey: true, shiftKey: true })).toBe(true); + expect(chord({ key: 's', code: 'KeyS', ctrlKey: true, shiftKey: true })).toBe(true); + }); + + it('goes by the physical key under a non-Latin layout, but not for another Latin letter', () => { + expect(chord({ key: 'Σ', code: 'KeyS', ctrlKey: true, shiftKey: true })).toBe(true); + expect(chord({ key: 'Unidentified', code: 'KeyS', ctrlKey: true, shiftKey: true })).toBe(true); + expect(chord({ key: 'O', code: 'KeyS', ctrlKey: true, shiftKey: true })).toBe(false); // Dvorak + }); + + it('rejects auto-repeat, Alt, IME composing, a missing Shift or modifier, and other keys', () => { + expect(chord({ key: 'S', metaKey: true, shiftKey: true, repeat: true })).toBe(false); + expect(chord({ key: 'S', metaKey: true, shiftKey: true, altKey: true })).toBe(false); + expect(chord({ key: 'S', metaKey: true, shiftKey: true, isComposing: true })).toBe(false); + expect(chord({ key: 's', metaKey: true })).toBe(false); + expect(chord({ key: 'S', shiftKey: true })).toBe(false); + expect(chord({ key: 'K', metaKey: true, shiftKey: true })).toBe(false); + }); + + it('fires even while typing in an input', () => { + const input = document.createElement('input'); + document.body.append(input); + let matched = false; + input.addEventListener('keydown', (e) => (matched = isStreamerChord(e))); + input.dispatchEvent( + new KeyboardEvent('keydown', { key: 'S', ctrlKey: true, shiftKey: true, bubbles: true }) + ); + input.remove(); + expect(matched).toBe(true); + }); +}); + describe('refresh hotkey (r)', () => { const hot = (init: KeyboardEventInit) => isRefreshHotkey(new KeyboardEvent('keydown', init)); diff --git a/crates/omnyssh-gui/ui/src/lib/stores/ui.ts b/crates/omnyssh-gui/ui/src/lib/stores/ui.ts index 9ccbbaf4..9bcda621 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/ui.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/ui.ts @@ -106,6 +106,14 @@ export function isCloseTabChord(e: KeyboardEvent): boolean { return e.key === 'w' || e.key === 'W' || (e.code === 'KeyW' && !/^[\x21-\x7e]$/.test(e.key)); } +/** The streamer-mode chord: ⌘⇧S / Ctrl+Shift+S. It fires regardless of focus, like the + * palette's, so it hides addresses at once from wherever the user is. A non-Latin + * layout (or WebKitGTK naming no key) goes by the physical key. */ +export function isStreamerChord(e: KeyboardEvent): boolean { + if (e.repeat || e.altKey || e.isComposing || !e.shiftKey || !(e.metaKey || e.ctrlKey)) return false; + return e.key === 's' || e.key === 'S' || (e.code === 'KeyS' && !/^[a-z]$/i.test(e.key)); +} + /** A bare-letter dashboard hotkey (mirrors the TUI): no modifier, ignored while typing in * an editable surface so it never eats a keystroke, and while any modal or the command * palette is open so it never changes the view behind it. Terminal input is never From 1b9245fa5a03f422850cd898fd9ef4cf64601daf Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Fri, 9 Oct 2026 09:48:05 +0400 Subject: [PATCH 09/15] feat(gui): show a streamer mode chip in the status bar --- crates/omnyssh-gui/ui/e2e/streamer.spec.ts | 16 +++++++++++++++ .../ui/src/lib/components/StatusBar.svelte | 20 +++++++++++++++++-- crates/omnyssh-gui/ui/src/lib/stores/ui.ts | 3 +++ .../omnyssh-gui/ui/src/lib/theme/Icon.svelte | 3 +++ crates/omnyssh-gui/ui/src/lib/theme/icons.ts | 1 + 5 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/omnyssh-gui/ui/e2e/streamer.spec.ts b/crates/omnyssh-gui/ui/e2e/streamer.spec.ts index 4cf99fd2..2bd679e6 100644 --- a/crates/omnyssh-gui/ui/e2e/streamer.spec.ts +++ b/crates/omnyssh-gui/ui/e2e/streamer.spec.ts @@ -90,6 +90,20 @@ test('with streamer mode off the status bar shows the error as sent', async ({ p await expect(page.getByRole('contentinfo').getByText(TUNNEL_ERROR)).toHaveAttribute('title', TUNNEL_ERROR); }); +test('a status-bar chip says streamer mode is on and turns it off', async ({ page }) => { + await boot(page, true); + await fireError(page, TUNNEL_ERROR); + const statusBar = page.getByRole('contentinfo'); + const chip = statusBar.getByRole('button', { name: 'Streamer mode is on — click to turn it off' }); + await expect(chip).toHaveText('Streamer mode'); + await expect(chip).toHaveAttribute('title', /\(Ctrl\+Shift\+S\)$/); + + await chip.click(); + await expect(chip).toHaveCount(0); + await expect(statusBar).toContainText(TUNNEL_ERROR); + expect(await page.evaluate(() => localStorage.getItem('omnyssh-streamer-mode'))).toBe('false'); +}); + test('Ctrl+Shift+S toggles streamer mode and re-masks what is already on screen', async ({ page }) => { await boot(page, false); await fireError(page, TUNNEL_ERROR); @@ -99,10 +113,12 @@ test('Ctrl+Shift+S toggles streamer mode and re-masks what is already on screen' await page.keyboard.press('Control+Shift+S'); await expect(statusBar).toContainText("Tunnel to 'db'"); await expect(statusBar).not.toContainText('10.20.3.7'); + await expect(statusBar.getByRole('button', { name: /^Streamer mode is on/ })).toBeVisible(); // Typing in a field does not hold it back. await page.getByRole('button', { name: 'Search hosts' }).click(); await page.getByRole('textbox', { name: 'Search hosts' }).press('Control+Shift+S'); await expect(statusBar).toContainText('10.20.3.7'); + await expect(statusBar.getByRole('button', { name: /^Streamer mode is on/ })).toHaveCount(0); await expect(page.getByRole('textbox', { name: 'Search hosts' })).toHaveValue(''); }); diff --git a/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte b/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte index ada65c7e..2a12e20d 100644 --- a/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte +++ b/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte @@ -4,8 +4,11 @@ // (§4.1); colour lives only in the status dots, per the brandbook. import { lastError } from '$lib/stores/notifications'; import { hostSummary } from '$lib/stores/hostSummary'; - import { maskText } from '$lib/stores/streamer'; - import { StatusDot } from '$lib/theme'; + import { maskText, streamerMode } from '$lib/stores/streamer'; + import { streamerChordLabel } from '$lib/stores/ui'; + import { Icon, StatusDot } from '$lib/theme'; + + const chipLabel = 'Streamer mode is on — click to turn it off';