feat(config): make config writes atomic with rollback-safe replacement (#190)

* feat(runtime): add Docker runtime MVP and runtime-aware command builder

* feat(security): add shell risk classification, approval gates, and action throttling

* feat(gateway): add per-endpoint rate limiting and webhook idempotency

* feat(config): make config writes atomic with rollback-safe replacement

---------

Co-authored-by: chumyin <chumyin@users.noreply.github.com>

src/config/schema.rs

@@ -2,8 +2,9 @@ use crate::security::AutonomyLevel;
 use anyhow::{Context, Result};
 use directories::UserDirs;
 use serde::{Deserialize, Serialize};
-use std::fs;
-use std::path::PathBuf;
+use std::fs::{self, File, OpenOptions};
+use std::io::Write;
+use std::path::{Path, PathBuf};
 
 // ── Top-level config ──────────────────────────────────────────────
 
@@ -112,6 +113,18 @@ pub struct GatewayConfig {
     /// Paired bearer tokens (managed automatically, not user-edited)
     #[serde(default)]
     pub paired_tokens: Vec<String>,
+
+    /// Max `/pair` requests per minute per client key.
+    #[serde(default = "default_pair_rate_limit")]
+    pub pair_rate_limit_per_minute: u32,
+
+    /// Max `/webhook` requests per minute per client key.
+    #[serde(default = "default_webhook_rate_limit")]
+    pub webhook_rate_limit_per_minute: u32,
+
+    /// TTL for webhook idempotency keys.
+    #[serde(default = "default_idempotency_ttl_secs")]
+    pub idempotency_ttl_secs: u64,
 }
 
 fn default_gateway_port() -> u16 {
@@ -122,6 +135,18 @@ fn default_gateway_host() -> String {
     "127.0.0.1".into()
 }
 
+fn default_pair_rate_limit() -> u32 {
+    10
+}
+
+fn default_webhook_rate_limit() -> u32 {
+    60
+}
+
+fn default_idempotency_ttl_secs() -> u64 {
+    300
+}
+
 fn default_true() -> bool {
     true
 }
@@ -134,6 +159,9 @@ impl Default for GatewayConfig {
             require_pairing: true,
             allow_public_bind: false,
             paired_tokens: Vec::new(),
+            pair_rate_limit_per_minute: default_pair_rate_limit(),
+            webhook_rate_limit_per_minute: default_webhook_rate_limit(),
+            idempotency_ttl_secs: default_idempotency_ttl_secs(),
         }
     }
 }
@@ -320,6 +348,14 @@ pub struct AutonomyConfig {
     pub forbidden_paths: Vec<String>,
     pub max_actions_per_hour: u32,
     pub max_cost_per_day_cents: u32,
+
+    /// Require explicit approval for medium-risk shell commands.
+    #[serde(default = "default_true")]
+    pub require_approval_for_medium_risk: bool,
+
+    /// Block high-risk shell commands even if allowlisted.
+    #[serde(default = "default_true")]
+    pub block_high_risk_commands: bool,
 }
 
 impl Default for AutonomyConfig {
@@ -363,6 +399,8 @@ impl Default for AutonomyConfig {
             ],
             max_actions_per_hour: 20,
             max_cost_per_day_cents: 500,
+            require_approval_for_medium_risk: true,
+            block_high_risk_commands: true,
         }
     }
 }
@@ -371,16 +409,85 @@ impl Default for AutonomyConfig {
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct RuntimeConfig {
-    /// Runtime kind (currently supported: "native").
-    ///
-    /// Reserved values (not implemented yet): "docker", "cloudflare".
+    /// Runtime kind (`native` | `docker`).
+    #[serde(default = "default_runtime_kind")]
     pub kind: String,
+
+    /// Docker runtime settings (used when `kind = "docker"`).
+    #[serde(default)]
+    pub docker: DockerRuntimeConfig,
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct DockerRuntimeConfig {
+    /// Runtime image used to execute shell commands.
+    #[serde(default = "default_docker_image")]
+    pub image: String,
+
+    /// Docker network mode (`none`, `bridge`, etc.).
+    #[serde(default = "default_docker_network")]
+    pub network: String,
+
+    /// Optional memory limit in MB (`None` = no explicit limit).
+    #[serde(default = "default_docker_memory_limit_mb")]
+    pub memory_limit_mb: Option<u64>,
+
+    /// Optional CPU limit (`None` = no explicit limit).
+    #[serde(default = "default_docker_cpu_limit")]
+    pub cpu_limit: Option<f64>,
+
+    /// Mount root filesystem as read-only.
+    #[serde(default = "default_true")]
+    pub read_only_rootfs: bool,
+
+    /// Mount configured workspace into `/workspace`.
+    #[serde(default = "default_true")]
+    pub mount_workspace: bool,
+
+    /// Optional workspace root allowlist for Docker mount validation.
+    #[serde(default)]
+    pub allowed_workspace_roots: Vec<String>,
+}
+
+fn default_runtime_kind() -> String {
+    "native".into()
+}
+
+fn default_docker_image() -> String {
+    "alpine:3.20".into()
+}
+
+fn default_docker_network() -> String {
+    "none".into()
+}
+
+fn default_docker_memory_limit_mb() -> Option<u64> {
+    Some(512)
+}
+
+fn default_docker_cpu_limit() -> Option<f64> {
+    Some(1.0)
+}
+
+impl Default for DockerRuntimeConfig {
+    fn default() -> Self {
+        Self {
+            image: default_docker_image(),
+            network: default_docker_network(),
+            memory_limit_mb: default_docker_memory_limit_mb(),
+            cpu_limit: default_docker_cpu_limit(),
+            read_only_rootfs: true,
+            mount_workspace: true,
+            allowed_workspace_roots: Vec::new(),
+        }
+    }
 }
 
 impl Default for RuntimeConfig {
     fn default() -> Self {
         Self {
-            kind: "native".into(),
+            kind: default_runtime_kind(),
+            docker: DockerRuntimeConfig::default(),
         }
     }
 }
@@ -811,11 +918,86 @@ impl Config {
 
     pub fn save(&self) -> Result<()> {
         let toml_str = toml::to_string_pretty(self).context("Failed to serialize config")?;
-        fs::write(&self.config_path, toml_str).context("Failed to write config file")?;
+
+        let parent_dir = self
+            .config_path
+            .parent()
+            .context("Config path must have a parent directory")?;
+        fs::create_dir_all(parent_dir).with_context(|| {
+            format!(
+                "Failed to create config directory: {}",
+                parent_dir.display()
+            )
+        })?;
+
+        let file_name = self
+            .config_path
+            .file_name()
+            .and_then(|v| v.to_str())
+            .unwrap_or("config.toml");
+        let temp_path = parent_dir.join(format!(".{file_name}.tmp-{}", uuid::Uuid::new_v4()));
+        let backup_path = parent_dir.join(format!("{file_name}.bak"));
+
+        let mut temp_file = OpenOptions::new()
+            .create_new(true)
+            .write(true)
+            .open(&temp_path)
+            .with_context(|| {
+                format!(
+                    "Failed to create temporary config file: {}",
+                    temp_path.display()
+                )
+            })?;
+        temp_file
+            .write_all(toml_str.as_bytes())
+            .context("Failed to write temporary config contents")?;
+        temp_file
+            .sync_all()
+            .context("Failed to fsync temporary config file")?;
+        drop(temp_file);
+
+        let had_existing_config = self.config_path.exists();
+        if had_existing_config {
+            fs::copy(&self.config_path, &backup_path).with_context(|| {
+                format!(
+                    "Failed to create config backup before atomic replace: {}",
+                    backup_path.display()
+                )
+            })?;
+        }
+
+        if let Err(e) = fs::rename(&temp_path, &self.config_path) {
+            let _ = fs::remove_file(&temp_path);
+            if had_existing_config && backup_path.exists() {
+                let _ = fs::copy(&backup_path, &self.config_path);
+            }
+            anyhow::bail!("Failed to atomically replace config file: {e}");
+        }
+
+        sync_directory(parent_dir)?;
+
+        if had_existing_config {
+            let _ = fs::remove_file(&backup_path);
+        }
+
         Ok(())
     }
 }
 
+#[cfg(unix)]
+fn sync_directory(path: &Path) -> Result<()> {
+    let dir = File::open(path)
+        .with_context(|| format!("Failed to open directory for fsync: {}", path.display()))?;
+    dir.sync_all()
+        .with_context(|| format!("Failed to fsync directory metadata: {}", path.display()))?;
+    Ok(())
+}
+
+#[cfg(not(unix))]
+fn sync_directory(_path: &Path) -> Result<()> {
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -850,12 +1032,20 @@ mod tests {
         assert!(a.forbidden_paths.contains(&"/etc".to_string()));
         assert_eq!(a.max_actions_per_hour, 20);
         assert_eq!(a.max_cost_per_day_cents, 500);
+        assert!(a.require_approval_for_medium_risk);
+        assert!(a.block_high_risk_commands);
     }
 
     #[test]
     fn runtime_config_default() {
         let r = RuntimeConfig::default();
         assert_eq!(r.kind, "native");
+        assert_eq!(r.docker.image, "alpine:3.20");
+        assert_eq!(r.docker.network, "none");
+        assert_eq!(r.docker.memory_limit_mb, Some(512));
+        assert_eq!(r.docker.cpu_limit, Some(1.0));
+        assert!(r.docker.read_only_rootfs);
+        assert!(r.docker.mount_workspace);
     }
 
     #[test]
@@ -905,9 +1095,12 @@ mod tests {
                 forbidden_paths: vec!["/secret".into()],
                 max_actions_per_hour: 50,
                 max_cost_per_day_cents: 1000,
+                require_approval_for_medium_risk: false,
+                block_high_risk_commands: true,
             },
             runtime: RuntimeConfig {
                 kind: "docker".into(),
+                ..RuntimeConfig::default()
             },
             reliability: ReliabilityConfig::default(),
             model_routes: Vec::new(),
@@ -1022,6 +1215,38 @@ default_temperature = 0.7
         let _ = fs::remove_dir_all(&dir);
     }
 
+
+    #[test]
+    fn config_save_atomic_cleanup() {
+        let dir =
+            std::env::temp_dir().join(format!("zeroclaw_test_config_{}", uuid::Uuid::new_v4()));
+        fs::create_dir_all(&dir).unwrap();
+
+        let config_path = dir.join("config.toml");
+        let mut config = Config::default();
+        config.workspace_dir = dir.join("workspace");
+        config.config_path = config_path.clone();
+        config.default_model = Some("model-a".into());
+
+        config.save().unwrap();
+        assert!(config_path.exists());
+
+        config.default_model = Some("model-b".into());
+        config.save().unwrap();
+
+        let contents = fs::read_to_string(&config_path).unwrap();
+        assert!(contents.contains("model-b"));
+
+        let names: Vec<String> = fs::read_dir(&dir)
+            .unwrap()
+            .map(|entry| entry.unwrap().file_name().to_string_lossy().to_string())
+            .collect();
+        assert!(!names.iter().any(|name| name.contains(".tmp-")));
+        assert!(!names.iter().any(|name| name.ends_with(".bak")));
+
+        let _ = fs::remove_dir_all(&dir);
+    }
+
     // ── Telegram / Discord config ────────────────────────────
 
     #[test]
@@ -1343,6 +1568,9 @@ channel_id = "C123"
             g.paired_tokens.is_empty(),
             "No pre-paired tokens by default"
         );
+        assert_eq!(g.pair_rate_limit_per_minute, 10);
+        assert_eq!(g.webhook_rate_limit_per_minute, 60);
+        assert_eq!(g.idempotency_ttl_secs, 300);
     }
 
     #[test]
@@ -1368,12 +1596,18 @@ channel_id = "C123"
             require_pairing: true,
             allow_public_bind: false,
             paired_tokens: vec!["zc_test_token".into()],
+            pair_rate_limit_per_minute: 12,
+            webhook_rate_limit_per_minute: 80,
+            idempotency_ttl_secs: 600,
         };
         let toml_str = toml::to_string(&g).unwrap();
         let parsed: GatewayConfig = toml::from_str(&toml_str).unwrap();
         assert!(parsed.require_pairing);
         assert!(!parsed.allow_public_bind);
         assert_eq!(parsed.paired_tokens, vec!["zc_test_token"]);
+        assert_eq!(parsed.pair_rate_limit_per_minute, 12);
+        assert_eq!(parsed.webhook_rate_limit_per_minute, 80);
+        assert_eq!(parsed.idempotency_ttl_secs, 600);
     }
 
     #[test]