Merge pull request #86 from vsilent/fix/security-owasp

OWASP-10 security fixes

Author: vsilent

src/agent/daemon.rs

@@ -11,6 +11,7 @@ use tracing::{error, info, warn};
 use crate::agent::config::Config;
 use crate::commands::executor::CommandExecutor;
 use crate::commands::firewall::FirewallPolicy;
+use crate::commands::validator::CommandValidator;
 use crate::commands::TimeoutStrategy;
 use crate::monitoring::{spawn_heartbeat, MetricsCollector, MetricsSnapshot, MetricsStore};
 use crate::transport::{http_polling, CommandResult};
@@ -216,36 +217,54 @@ async fn execute_and_report(
             }
         }
         Ok(None) => {
-            // Not a stacker command, fall back to shell execution
-            info!(
-                command_id = %cmd.command_id,
-                command_name = %cmd.name,
-                "executing as shell command"
-            );
-            let strategy = TimeoutStrategy::backup_strategy(ctx.command_timeout);
-            let exec_result = executor.execute(&cmd, strategy).await;
-
-            match exec_result {
-                Ok(output) => CommandResult {
-                    command_id: cmd.command_id.clone(),
-                    status: "success".to_string(),
-                    result: Some(json!({
-                        "stdout": output.stdout,
-                        "stderr": output.stderr,
-                        "exit_code": output.exit_code,
-                    })),
-                    error: None,
-                    completed_at: Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true),
-                    ..CommandResult::default()
-                },
-                Err(e) => CommandResult {
+            // Not a stacker command — validate before shell execution
+            let validator = CommandValidator::default_secure();
+            if let Err(e) = validator.validate(&cmd) {
+                error!(
+                    command_id = %cmd.command_id,
+                    command_name = %cmd.name,
+                    error = %e,
+                    "shell command rejected by validator"
+                );
+                CommandResult {
                     command_id: cmd.command_id.clone(),
                     status: "failed".to_string(),
                     result: None,
-                    error: Some(e.to_string()),
+                    error: Some(format!("Command validation failed: {}", e)),
                     completed_at: Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true),
                     ..CommandResult::default()
-                },
+                }
+            } else {
+                info!(
+                    command_id = %cmd.command_id,
+                    command_name = %cmd.name,
+                    "executing validated shell command"
+                );
+                let strategy = TimeoutStrategy::backup_strategy(ctx.command_timeout);
+                let exec_result = executor.execute(&cmd, strategy).await;
+
+                match exec_result {
+                    Ok(output) => CommandResult {
+                        command_id: cmd.command_id.clone(),
+                        status: "success".to_string(),
+                        result: Some(json!({
+                            "stdout": output.stdout,
+                            "stderr": output.stderr,
+                            "exit_code": output.exit_code,
+                        })),
+                        error: None,
+                        completed_at: Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true),
+                        ..CommandResult::default()
+                    },
+                    Err(e) => CommandResult {
+                        command_id: cmd.command_id.clone(),
+                        status: "failed".to_string(),
+                        result: None,
+                        error: Some(e.to_string()),
+                        completed_at: Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true),
+                        ..CommandResult::default()
+                    },
+                }
             }
         }
         Err(e) => {

src/agent/docker.rs

@@ -699,6 +699,65 @@ pub async fn exec_in_container(name: &str, cmd: &str) -> Result<()> {
     }
 }
 
+/// Execute a command inside a container using an explicit argv (no shell interpretation).
+/// Each element in `argv` is passed directly to execve, preventing shell injection.
+pub async fn exec_in_container_argv(name: &str, argv: Vec<String>) -> Result<()> {
+    use bollard::exec::StartExecResults;
+    use futures_util::StreamExt;
+
+    let docker = docker_client()?;
+    let resolved_name = resolve_container_name(name)
+        .await
+        .unwrap_or_else(|_| name.to_string());
+    let argv_display = argv.join(" ");
+    let exec = docker
+        .create_exec(
+            &resolved_name,
+            CreateExecOptions {
+                attach_stdout: Some(true),
+                attach_stderr: Some(true),
+                tty: Some(false),
+                cmd: Some(argv),
+                ..Default::default()
+            },
+        )
+        .await
+        .context("create exec")?;
+
+    let start = docker
+        .start_exec(&exec.id, None)
+        .await
+        .context("start exec")?;
+
+    let mut combined = String::new();
+    match start {
+        StartExecResults::Detached => {
+            debug!(container = name, command = %argv_display, "exec_argv detached");
+        }
+        StartExecResults::Attached { mut output, .. } => {
+            while let Some(item) = output.next().await {
+                match item {
+                    Ok(log) => combined.push_str(&format!("{}", log)),
+                    Err(e) => error!("exec output stream error: {}", e),
+                }
+            }
+        }
+    }
+
+    let info = docker
+        .inspect_exec(&exec.id)
+        .await
+        .context("inspect exec")?;
+    let exit_code = info.exit_code.unwrap_or_default();
+    if exit_code == 0 {
+        debug!(container = name, command = %argv_display, "exec_argv completed");
+        Ok(())
+    } else {
+        error!(container = name, command = %argv_display, exit_code, output = combined, "exec_argv failed");
+        Err(anyhow::anyhow!("exec failed with code {}", exit_code))
+    }
+}
+
 /// Execute a shell command inside a running container and return output.
 /// Returns (exit_code, stdout, stderr) tuple.
 pub async fn exec_in_container_with_output(name: &str, cmd: &str) -> Result<(i64, String, String)> {

src/commands/self_update.rs

@@ -74,6 +74,16 @@ pub async fn start_update_job(jobs: UpdateJobs, target_version: Option<String>)
             return;
         };
 
+        // Enforce HTTPS for update URLs using shared validation policy
+        if !crate::security::validation::is_safe_update_url(&url) {
+            let mut w = jobs_clone.write().await;
+            if let Some(st) = w.get_mut(&id_clone) {
+                st.phase =
+                    UpdatePhase::Failed("Update URL must use HTTPS for integrity".to_string());
+            }
+            return;
+        }
+
         {
             let mut w = jobs_clone.write().await;
             if let Some(st) = w.get_mut(&id_clone) {
@@ -114,29 +124,38 @@ pub async fn start_update_job(jobs: UpdateJobs, target_version: Option<String>)
             }
         }
 
-        // Optional SHA256 verification
-        if let Some(expected) = expected_sha {
-            let verify_res = async {
-                let data = tokio::fs::read(&tmp_path)
-                    .await
-                    .context("reading temp binary for sha256")?;
-                let mut hasher = Sha256::new();
-                hasher.update(&data);
-                let got = format!("{:x}", hasher.finalize());
+        // SHA256 verification — mandatory for update integrity
+        let verify_res = async {
+            let data = tokio::fs::read(&tmp_path)
+                .await
+                .context("reading temp binary for sha256")?;
+            let mut hasher = Sha256::new();
+            hasher.update(&data);
+            let got = format!("{:x}", hasher.finalize());
+            if let Some(expected) = &expected_sha {
                 if got != expected.to_lowercase() {
                     anyhow::bail!("sha256 mismatch: got {} expected {}", got, expected);
                 }
-                Result::<()>::Ok(())
+            } else {
+                tracing::error!(
+                    sha256 = %got,
+                    "UPDATE_EXPECTED_SHA256 not set — refusing update because integrity \
+                     verification is mandatory"
+                );
+                anyhow::bail!(
+                    "UPDATE_EXPECTED_SHA256 not set — integrity verification is mandatory"
+                );
             }
-            .await;
+            Result::<()>::Ok(())
+        }
+        .await;
 
-            if let Err(e) = verify_res {
-                let mut w = jobs_clone.write().await;
-                if let Some(st) = w.get_mut(&id_clone) {
-                    st.phase = UpdatePhase::Failed(e.to_string());
-                }
-                return;
+        if let Err(e) = verify_res {
+            let mut w = jobs_clone.write().await;
+            if let Some(st) = w.get_mut(&id_clone) {
+                st.phase = UpdatePhase::Failed(e.to_string());
             }
+            return;
         }
 
         // Completed preparation (download + verify). Deployment handled in a later phase.