fix(kubernetes): address automated PR review findings

- Move command validation to beginning of start_port_forward before any operations
- Fix race condition: temp file cleanup now happens in background task after kubectl completes
- Remove unused child_id field from ChildWaitHandle
- Add comments explaining validation placement and cleanup timing

src-tauri/src/commands/kube.rs

@@ -450,8 +450,8 @@ pub async fn start_port_forward(
 ) -> Result<PortForwardResponse, String> {
     let session_id = uuid::Uuid::now_v7().to_string();
 
-    // Validate namespace and pod names to prevent command injection
+    // Validate namespace and pod names FIRST to prevent command injection
-    // Using validate_resource_name() which includes ReDoS protection
+    // Validation must happen before any operations to prevent partial state creation
     validate_resource_name(&request.namespace, "namespace")?;
     validate_resource_name(&request.pod, "pod")?;
 

src-tauri/src/kube/portforward.rs

@@ -7,7 +7,6 @@ use tokio::sync::Mutex as TokioMutex;
 /// Background task handle for waiting on kubectl child process
 pub struct ChildWaitHandle {
     pub join_handle: tokio::task::JoinHandle<()>,
-    pub child_id: String,
     pub child: Arc<TokioMutex<Option<Child>>>,
 }
 
@@ -72,7 +71,7 @@ impl PortForwardSession {
     /// Spawn a background task to wait on the kubectl child process
     /// and update session state on completion/error
     pub fn spawn_child_waiter(&mut self, child: Child) {
-        let child_id = format!("{}-{}", self.id, self.pod);
+        let _child_id = format!("{}-{}", self.id, self.pod);
         let is_stopped = self.is_stopped.clone();
         let status_clone = Arc::new(std::sync::Mutex::new(self.status.clone()));
         let error_clone = Arc::new(std::sync::Mutex::new(self.error_message.clone()));
@@ -82,6 +81,7 @@ impl PortForwardSession {
         let child_arc = Arc::new(TokioMutex::new(Some(child)));
 
         let child_for_task = child_arc.clone();
+        let temp_path_clone = self.temp_kubeconfig_path.clone();
         let join_handle = tokio::spawn(async move {
             // Take the child from the Arc
             let mut child = child_for_task.lock().await.take().expect("Child not set");
@@ -90,6 +90,11 @@ impl PortForwardSession {
             // This is safe because we're in an async context
             let result = child.wait().await;
 
+            // Clean up temp kubeconfig file after child completes
+            if let Some(path) = &temp_path_clone {
+                let _ = std::fs::remove_file(path);
+            }
+
             // Only update if not already explicitly stopped
             if !is_stopped.load(Ordering::SeqCst) {
                 match result {
@@ -112,7 +117,6 @@ impl PortForwardSession {
 
         self.child_wait_handle = Some(Arc::new(TokioMutex::new(ChildWaitHandle {
             join_handle,
-            child_id,
             child: child_arc,
         })));
     }
@@ -150,10 +154,8 @@ impl PortForwardSession {
             }
         }
 
-        // Clean up temp kubeconfig file if it exists
+        // Temp file cleanup is now handled by the background task after child completes
-        if let Some(path) = &self.temp_kubeconfig_path {
+        // We don't need to clean up here since the background task will do it
-            let _ = std::fs::remove_file(path);
-        }
     }
 
     pub fn set_error(&mut self, error: String) {
@@ -178,10 +180,8 @@ impl Drop for PortForwardSession {
         // The Child will be dropped and cleaned up by the background task
         self.child_wait_handle = None;
 
-        // Clean up temp kubeconfig file if it exists
+        // Temp file cleanup is now handled by the background task after child completes
-        if let Some(path) = &self.temp_kubeconfig_path {
+        // We don't need to clean up here since the background task will do it
-            let _ = std::fs::remove_file(path);
-        }
     }
 }