fix: replace panics reachable from user input with proper error handling

src/filetransfer/params.rs

@@ -25,10 +25,10 @@ pub enum HostBridgeParams {
 }
 
 impl HostBridgeParams {
-    pub fn unwrap_protocol_params(&self) -> &ProtocolParams {
+    pub fn protocol_params(&self) -> Option<&ProtocolParams> {
         match self {
-            HostBridgeParams::Localhost(_) => panic!("Localhost has no protocol params"),
-            HostBridgeParams::Remote(_, params) => params,
+            HostBridgeParams::Localhost(_) => None,
+            HostBridgeParams::Remote(_, params) => Some(params),
         }
     }
 
@@ -278,6 +278,19 @@ mod test {
 
     use super::*;
 
+    #[test]
+    fn test_protocol_params_on_localhost_returns_none() {
+        let params = HostBridgeParams::Localhost(PathBuf::from("/tmp"));
+        assert!(params.protocol_params().is_none());
+    }
+
+    #[test]
+    fn test_protocol_params_on_remote_returns_some() {
+        let params =
+            HostBridgeParams::Remote(FileTransferProtocol::Sftp, ProtocolParams::default());
+        assert!(params.protocol_params().is_some());
+    }
+
     #[test]
     fn test_filetransfer_params() {
         let params: FileTransferParams =

src/filetransfer/remotefs_builder.rs

@@ -44,13 +44,13 @@ impl RemoteFsBuilder {
     ) -> Result<Box<dyn RemoteFs>, String> {
         match (protocol, params) {
             (FileTransferProtocol::AwsS3, ProtocolParams::AwsS3(params)) => {
-                Ok(Box::new(Self::aws_s3_client(params)))
+                Ok(Box::new(Self::aws_s3_client(params)?))
             }
             (FileTransferProtocol::Ftp(secure), ProtocolParams::Generic(params)) => {
                 Ok(Box::new(Self::ftp_client(params, secure)))
             }
             (FileTransferProtocol::Kube, ProtocolParams::Kube(params)) => {
-                Ok(Box::new(Self::kube_client(params)))
+                Ok(Box::new(Self::kube_client(params)?))
             }
             (FileTransferProtocol::Scp, ProtocolParams::Generic(params)) => {
                 Ok(Box::new(Self::scp_client(params, config_client)))
@@ -60,7 +60,7 @@ impl RemoteFsBuilder {
             }
             #[cfg(smb)]
             (FileTransferProtocol::Smb, ProtocolParams::Smb(params)) => {
-                Ok(Box::new(Self::smb_client(params)))
+                Ok(Box::new(Self::smb_client(params)?))
             }
             (FileTransferProtocol::WebDAV, ProtocolParams::WebDAV(params)) => {
                 Ok(Box::new(Self::webdav_client(params)))
@@ -75,13 +75,13 @@ impl RemoteFsBuilder {
     }
 
     /// Build aws s3 client from parameters
-    fn aws_s3_client(params: AwsS3Params) -> AwsS3Fs {
+    fn aws_s3_client(params: AwsS3Params) -> Result<AwsS3Fs, String> {
         let rt = Arc::new(
             tokio::runtime::Builder::new_current_thread()
                 .worker_threads(1)
                 .enable_all()
                 .build()
-                .expect("Unable to create tokio runtime"),
+                .map_err(|e| format!("Unable to create tokio runtime: {e}"))?,
         );
         let mut client =
             AwsS3Fs::new(params.bucket_name, &rt).new_path_style(params.new_path_style);
@@ -106,7 +106,7 @@ impl RemoteFsBuilder {
         if let Some(session_token) = params.session_token {
             client = client.session_token(session_token);
         }
-        client
+        Ok(client)
     }
 
     /// Build ftp client from parameters
@@ -125,19 +125,19 @@ impl RemoteFsBuilder {
     }
 
     /// Build kube client
-    fn kube_client(params: KubeProtocolParams) -> KubeFs {
+    fn kube_client(params: KubeProtocolParams) -> Result<KubeFs, String> {
         let rt = Arc::new(
             tokio::runtime::Builder::new_current_thread()
                 .worker_threads(1)
                 .enable_all()
                 .build()
-                .expect("Unable to create tokio runtime"),
+                .map_err(|e| format!("Unable to create tokio runtime: {e}"))?,
         );
         let kube_fs = KubeFs::new(&rt);
         if let Some(config) = params.config() {
-            kube_fs.config(config)
+            Ok(kube_fs.config(config))
         } else {
-            kube_fs
+            Ok(kube_fs)
         }
     }
 
@@ -158,7 +158,7 @@ impl RemoteFsBuilder {
     }
 
     #[cfg(smb_unix)]
-    fn smb_client(params: SmbParams) -> SmbFs {
+    fn smb_client(params: SmbParams) -> Result<SmbFs, String> {
         let mut credentials = SmbCredentials::default()
             .server(format!("smb://{}:{}", params.address, params.port))
             .share(params.share);
@@ -179,14 +179,14 @@ impl RemoteFsBuilder {
                 .one_share_per_server(true)
                 .case_sensitive(false),
         )
-        .unwrap_or_else(|e| {
+        .map_err(|e| {
             error!("Invalid params for protocol SMB: {e}");
-            panic!("Invalid params for protocol SMB: {e}")
+            format!("Invalid params for protocol SMB: {e}")
         })
     }
 
     #[cfg(smb_windows)]
-    fn smb_client(params: SmbParams) -> SmbFs {
+    fn smb_client(params: SmbParams) -> Result<SmbFs, String> {
         let mut credentials = SmbCredentials::new(params.address, params.share);
 
         if let Some(username) = params.username {
@@ -196,7 +196,7 @@ impl RemoteFsBuilder {
             credentials = credentials.password(password);
         }
 
-        SmbFs::new(credentials)
+        Ok(SmbFs::new(credentials))
     }
 
     fn webdav_client(params: WebDAVProtocolParams) -> WebDAVFs {

src/host/localhost.rs

@@ -395,7 +395,17 @@ impl HostBridge for Localhost {
     fn exec(&mut self, cmd: &str) -> HostResult<String> {
         // Make command
         let args: Vec<&str> = cmd.split(' ').collect();
-        let cmd: &str = args.first().unwrap();
+        let cmd: &str = match args.first() {
+            Some(cmd) => cmd,
+            None => {
+                error!("Empty command provided to exec");
+                return Err(HostError::new(
+                    HostErrorType::ExecutionFailed,
+                    None,
+                    self.wrkdir.as_path(),
+                ));
+            }
+        };
         let argv: &[&str] = &args[1..];
         info!("Executing command: {} {:?}", cmd, argv);
         match std::process::Command::new(cmd).args(argv).output() {
@@ -993,6 +1003,14 @@ mod tests {
         assert!(host.exec("echo 5").ok().unwrap().as_str().contains("5"));
     }
 
+    #[test]
+    fn test_host_exec_empty_command() {
+        let tmpdir: tempfile::TempDir = tempfile::TempDir::new().unwrap();
+        let mut host: Localhost = Localhost::new(PathBuf::from(tmpdir.path())).ok().unwrap();
+        // Execute empty command should return error
+        assert!(host.exec("").is_err());
+    }
+
     #[cfg(posix)]
     #[test]
     fn should_create_symlink() {

src/system/bookmarks_client.rs

@@ -190,8 +190,8 @@ impl BookmarksClient {
     ) {
         let name: String = name.as_ref().to_string();
         if name.is_empty() {
-            error!("Fatal error; bookmark name is empty");
-            panic!("Bookmark name can't be empty");
+            error!("Bookmark name is empty; ignoring add_bookmark request");
+            return;
         }
         // Make bookmark
         info!("Added bookmark {}", name);
@@ -557,15 +557,13 @@ mod tests {
     }
 
     #[test]
-    #[should_panic]
-
     fn test_system_bookmarks_bad_bookmark_name() {
         let tmp_dir: tempfile::TempDir = TempDir::new().ok().unwrap();
         let (cfg_path, key_path): (PathBuf, PathBuf) = get_paths(tmp_dir.path());
         // Initialize a new bookmarks client
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
-        // Add bookmark
+        // Add bookmark with empty name should be silently ignored
         client.add_bookmark(
             "",
             make_generic_ftparams(
@@ -577,6 +575,8 @@ mod tests {
             ),
             true,
         );
+        // No bookmark should have been added
+        assert_eq!(client.iter_bookmarks().count(), 0);
     }
 
     #[test]
@@ -738,14 +738,13 @@ mod tests {
     }
 
     #[test]
-    #[should_panic]
     fn test_system_bookmarks_add_bookmark_empty() {
         let tmp_dir: tempfile::TempDir = TempDir::new().ok().unwrap();
         let (cfg_path, key_path): (PathBuf, PathBuf) = get_paths(tmp_dir.path());
         // Initialize a new bookmarks client
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
-        // Add bookmark
+        // Add bookmark with empty name should be silently ignored
         client.add_bookmark(
             "",
             make_generic_ftparams(
@@ -757,6 +756,8 @@ mod tests {
             ),
             true,
         );
+        // No bookmark should have been added
+        assert_eq!(client.iter_bookmarks().count(), 0);
     }
 
     #[test]

src/system/config_client.rs

@@ -309,7 +309,7 @@ impl ConfigClient {
             None => None,
             Some(key_path) => {
                 // Get host and username
-                let (host, username): (String, String) = Self::get_ssh_tokens(mkey);
+                let (host, username) = Self::get_ssh_tokens(mkey)?;
                 // Return key
                 Some((host, username, PathBuf::from(key_path)))
             }
@@ -389,12 +389,18 @@ impl ConfigClient {
     }
 
     /// Get ssh tokens starting from ssh host key
-    /// Panics if key has invalid syntax
-    /// Returns: (host, username)
-    fn get_ssh_tokens(host_key: &str) -> (String, String) {
+    /// Returns: (host, username) or None if key has invalid syntax
+    fn get_ssh_tokens(host_key: &str) -> Option<(String, String)> {
         let tokens: Vec<&str> = host_key.split('@').collect();
-        assert!(tokens.len() >= 2);
-        (String::from(tokens[1]), String::from(tokens[0]))
+        if tokens.len() >= 2 {
+            Some((String::from(tokens[1]), String::from(tokens[0])))
+        } else {
+            error!(
+                "Invalid SSH host key format: '{}' (expected 'username@host')",
+                host_key
+            );
+            None
+        }
     }
 
     /// Make serializer error from `std::io::Error`
@@ -711,10 +717,16 @@ mod tests {
         );
         assert_eq!(
             ConfigClient::get_ssh_tokens("pi@192.168.1.31"),
-            (String::from("192.168.1.31"), String::from("pi"))
+            Some((String::from("192.168.1.31"), String::from("pi")))
         );
     }
 
+    #[test]
+    fn test_system_config_get_ssh_tokens_invalid() {
+        assert!(ConfigClient::get_ssh_tokens("invalid").is_none());
+        assert!(ConfigClient::get_ssh_tokens("").is_none());
+    }
+
     #[test]
     fn test_system_config_make_io_err() {
         let err: SerializerError =

src/system/keys/filestorage.rs

@@ -71,7 +71,10 @@ impl KeyStorage for FileStorage {
                     return Err(KeyStorageError::ProviderError);
                 }
                 // Set file to readonly
-                let mut permissions: Permissions = file.metadata().unwrap().permissions();
+                let mut permissions: Permissions = file
+                    .metadata()
+                    .map_err(|_| KeyStorageError::ProviderError)?
+                    .permissions();
                 permissions.set_readonly(true);
                 let _ = file.set_permissions(permissions);
                 Ok(())

src/ui/activities/filetransfer/mod.rs

@@ -478,9 +478,10 @@ impl Activity for FileTransferActivity {
             && !self.browser.local_pane().fs.is_localhost()
         {
             let host_bridge_params = self.context().host_bridge_params().unwrap();
-            let ft_params = host_bridge_params.unwrap_protocol_params();
-            // print params
-            let msg: String = Self::get_connection_msg(ft_params);
+            let msg: String = match host_bridge_params.protocol_params() {
+                Some(ft_params) => Self::get_connection_msg(ft_params),
+                None => String::from("Connecting..."),
+            };
             // Set init state to connecting popup
             self.mount_blocking_wait(msg.as_str());
             // Connect to remote