fix: consolidated security and robustness improvements

This PR consolidates the following fixes:
- #74: Prevent shell injection in restore_script via path escaping
- #76: Replace unsafe unwrap() with expect() in init_client
- #78: Use secure random temp files in external editor to prevent symlink attacks
- #79: Add per-chunk streaming timeout to prevent indefinite hangs

Key changes:
- Added shell escaping for paths in shell-snapshot restore scripts
- Replaced unwrap() with expect() for better error context in exec runner
- Use secure random temp files instead of predictable names
- Added streaming chunk timeout to prevent hangs during LLM responses

Author: echobt

src/cortex-exec/src/runner.rs

@@ -32,6 +32,11 @@ const DEFAULT_TIMEOUT_SECS: u64 = 600;
 /// Default timeout for a single LLM request (2 minutes).
 const DEFAULT_REQUEST_TIMEOUT_SECS: u64 = 120;
 
+/// Per-chunk timeout during streaming responses.
+/// Prevents indefinite hangs when connections stall mid-stream.
+/// See cortex_common::http_client for timeout hierarchy documentation.
+const STREAMING_CHUNK_TIMEOUT_SECS: u64 = 30;
+
 /// Maximum retries for transient errors.
 const MAX_RETRIES: usize = 3;
 
@@ -187,7 +192,7 @@ impl ExecRunner {
             self.client = Some(client);
         }
 
-        Ok(self.client.as_ref().unwrap().as_ref())
+        Ok(self.client.as_ref().expect("Client should be initialized in init_client").as_ref())
     }
 
     /// Get filtered tool definitions based on options.
@@ -555,7 +560,28 @@ impl ExecRunner {
         let mut partial_tool_calls: std::collections::HashMap<String, (String, String)> =
             std::collections::HashMap::new();
 
-        while let Some(event) = stream.next().await {
+        loop {
+            // Apply per-chunk timeout to prevent indefinite hangs when connections stall
+            let event = match tokio::time::timeout(
+                Duration::from_secs(STREAMING_CHUNK_TIMEOUT_SECS),
+                stream.next(),
+            )
+            .await
+            {
+                Ok(Some(event)) => event,
+                Ok(None) => break, // Stream ended normally
+                Err(_) => {
+                    tracing::warn!(
+                        "Stream chunk timeout after {}s",
+                        STREAMING_CHUNK_TIMEOUT_SECS
+                    );
+                    return Err(CortexError::Provider(format!(
+                        "Streaming timeout: no response chunk received within {}s",
+                        STREAMING_CHUNK_TIMEOUT_SECS
+                    )));
+                }
+            };
+
             match event? {
                 ResponseEvent::Delta(delta) => {
                     if self.options.streaming {

src/cortex-shell-snapshot/src/snapshot.rs

@@ -115,12 +115,13 @@ impl ShellSnapshot {
     }
 
     /// Generate a restore script that sources this snapshot.
+    ///
+    /// The path is properly escaped to prevent shell injection attacks.
+    /// Paths containing single quotes are escaped using shell-safe quoting.
     pub fn restore_script(&self) -> String {
         let header = scripts::restore_header(self.metadata.shell_type);
-        format!(
-            "{header}\n# Source snapshot\nsource '{}'\n",
-            self.path.display()
-        )
+        let escaped_path = shell_escape_path(&self.path);
+        format!("{header}\n# Source snapshot\nsource {escaped_path}\n")
     }
 
     /// Save the snapshot to disk.
@@ -197,6 +198,27 @@ impl Drop for ShellSnapshot {
     }
 }
 
+/// Escape a path for safe use in shell commands.
+///
+/// This function handles paths containing single quotes by using the
+/// shell-safe escaping technique: 'path'"'"'with'"'"'quotes'
+///
+/// For paths without single quotes, simple single-quoting is used.
+fn shell_escape_path(path: &Path) -> String {
+    let path_str = path.display().to_string();
+
+    if !path_str.contains('\'') {
+        // Simple case: no single quotes, just wrap in single quotes
+        format!("'{}'", path_str)
+    } else {
+        // Complex case: escape single quotes using '"'"' technique
+        // This closes the single-quoted string, adds a double-quoted single quote,
+        // and reopens the single-quoted string
+        let escaped = path_str.replace('\'', "'\"'\"'");
+        format!("'{}'", escaped)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -221,4 +243,26 @@ mod tests {
             "snapshot_12345678-1234-1234-1234-123456789012.zsh"
         );
     }
+
+    #[test]
+    fn test_shell_escape_path_simple() {
+        let path = Path::new("/tmp/test/snapshot.sh");
+        let escaped = shell_escape_path(path);
+        assert_eq!(escaped, "'/tmp/test/snapshot.sh'");
+    }
+
+    #[test]
+    fn test_shell_escape_path_with_single_quotes() {
+        let path = Path::new("/tmp/test's/snap'shot.sh");
+        let escaped = shell_escape_path(path);
+        // Single quotes should be escaped using '"'"' technique
+        assert_eq!(escaped, "'/tmp/test'\"'\"'s/snap'\"'\"'shot.sh'");
+    }
+
+    #[test]
+    fn test_shell_escape_path_spaces() {
+        let path = Path::new("/tmp/test path/snapshot.sh");
+        let escaped = shell_escape_path(path);
+        assert_eq!(escaped, "'/tmp/test path/snapshot.sh'");
+    }
 }

src/cortex-tui/Cargo.toml

@@ -65,6 +65,7 @@ walkdir = { workspace = true }
 
 # External editor
 which = { workspace = true }
+tempfile = { workspace = true }
 
 # Audio notifications
 rodio = { workspace = true }

src/cortex-tui/src/external_editor.rs

@@ -162,17 +162,25 @@ pub async fn open_external_editor(initial_content: &str) -> Result<String, Edito
     // Get the editor command
     let editor_cmd = get_editor()?;
 
-    // Create a temporary file
-    let temp_dir = std::env::temp_dir();
-    let temp_file = temp_dir.join(format!("cortex_prompt_{}.md", std::process::id()));
-
-    // Write initial content
+    // Create a temporary file with a secure random name to prevent symlink attacks.
+    // Using tempfile crate ensures proper security (O_EXCL, restricted permissions).
+    let temp_file = tempfile::Builder::new()
+        .prefix("cortex_prompt_")
+        .suffix(".md")
+        .rand_bytes(16)
+        .tempfile()
+        .map_err(EditorError::Io)?;
+
+    // Write initial content using the secure file handle
     {
-        let mut file = std::fs::File::create(&temp_file)?;
+        let mut file = temp_file.reopen().map_err(EditorError::Io)?;
         file.write_all(initial_content.as_bytes())?;
         file.flush()?;
     }
 
+    // Keep the temp file alive (don't let it be deleted yet)
+    let temp_file = temp_file.into_temp_path();
+
     // Parse the editor command
     let parts: Vec<&str> = editor_cmd.split_whitespace().collect();
     let (editor, args) = match parts.split_first() {
@@ -219,17 +227,25 @@ pub fn open_external_editor_sync(initial_content: &str) -> Result<String, Editor
     // Get the editor command
     let editor_cmd = get_editor()?;
 
-    // Create a temporary file
-    let temp_dir = std::env::temp_dir();
-    let temp_file = temp_dir.join(format!("cortex_prompt_{}.md", std::process::id()));
+    // Create a temporary file with a secure random name to prevent symlink attacks.
+    // Using tempfile crate ensures proper security (O_EXCL, restricted permissions).
+    let temp_file = tempfile::Builder::new()
+        .prefix("cortex_prompt_")
+        .suffix(".md")
+        .rand_bytes(16)
+        .tempfile()
+        .map_err(EditorError::Io)?;
 
-    // Write initial content
+    // Write initial content using the secure file handle
     {
-        let mut file = std::fs::File::create(&temp_file)?;
+        let mut file = temp_file.reopen().map_err(EditorError::Io)?;
         file.write_all(initial_content.as_bytes())?;
         file.flush()?;
     }
 
+    // Keep the temp file alive (don't let it be deleted yet)
+    let temp_file = temp_file.into_temp_path();
+
     // Parse the editor command
     let parts: Vec<&str> = editor_cmd.split_whitespace().collect();
     let (editor, args) = match parts.split_first() {
@@ -264,12 +280,13 @@ pub fn open_external_editor_sync(initial_content: &str) -> Result<String, Editor
     Ok(content.trim().to_string())
 }
 
-/// Gets the path to the temporary file that would be used.
+/// Gets an example path pattern for temporary files.
 ///
-/// Useful for displaying to the user.
+/// Note: Actual temp files use random suffixes for security.
+/// This function returns a pattern showing the general location.
 pub fn get_temp_file_path() -> PathBuf {
     let temp_dir = std::env::temp_dir();
-    temp_dir.join(format!("cortex_prompt_{}.md", std::process::id()))
+    temp_dir.join("cortex_prompt_XXXXXXXXXXXXXXXX.md")
 }
 
 // ============================================================