feat(runtime): switch shim config to file path and harden env delivery

boxlite/src/bin/shim/main.rs

@@ -14,7 +14,7 @@
 
 mod crash_capture;
 
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::thread;
 use std::time::Duration;
 
@@ -49,8 +49,20 @@ struct ShimArgs {
     ///
     /// This contains the full InstanceSpec including rootfs path, volumes,
     /// networking, guest entrypoint, and other runtime configuration.
-    #[arg(long)]
-    config: String,
+    #[arg(
+        long,
+        conflicts_with = "config_file",
+        required_unless_present = "config_file"
+    )]
+    config: Option<String>,
+
+    /// Path to Box configuration JSON file
+    #[arg(
+        long = "config-file",
+        conflicts_with = "config",
+        required_unless_present = "config"
+    )]
+    config_file: Option<PathBuf>,
 }
 
 /// Initialize tracing with file logging.
@@ -85,9 +97,8 @@ fn main() -> BoxliteResult<()> {
     // VmmKind parsed via FromStr trait automatically
     let args = ShimArgs::parse();
 
-    // Parse InstanceSpec from JSON
-    let config: InstanceSpec = serde_json::from_str(&args.config)
-        .map_err(|e| BoxliteError::Engine(format!("Failed to parse config JSON: {}", e)))?;
+    // Parse InstanceSpec from --config-file (preferred) or --config.
+    let config = load_instance_spec(&args)?;
 
     // Initialize logging using box_dir derived from exit_file path.
     // Logs go to box_dir/logs/ so the sandbox only needs write access to box_dir.
@@ -125,6 +136,35 @@ fn main() -> BoxliteResult<()> {
     })
 }
 
+fn load_instance_spec(args: &ShimArgs) -> BoxliteResult<InstanceSpec> {
+    if let Some(config_file) = &args.config_file {
+        let config_json = std::fs::read_to_string(config_file).map_err(|e| {
+            BoxliteError::Engine(format!(
+                "Failed to read config file {}: {}",
+                config_file.display(),
+                e
+            ))
+        })?;
+
+        return serde_json::from_str(&config_json).map_err(|e| {
+            BoxliteError::Engine(format!(
+                "Failed to parse config JSON from {}: {}",
+                config_file.display(),
+                e
+            ))
+        });
+    }
+
+    if let Some(config_json) = &args.config {
+        return serde_json::from_str(config_json)
+            .map_err(|e| BoxliteError::Engine(format!("Failed to parse config JSON: {}", e)));
+    }
+
+    Err(BoxliteError::Engine(
+        "Either --config-file or --config must be provided".to_string(),
+    ))
+}
+
 fn run_shim(args: ShimArgs, mut config: InstanceSpec) -> BoxliteResult<()> {
     tracing::debug!(
         shares = ?config.fs_shares.shares(),
@@ -394,3 +434,117 @@ fn start_parent_watchdog() {
         std::process::exit(137); // 128 + 9 (SIGKILL)
     });
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use serde_json::Value;
+    use std::io::Write;
+
+    fn test_instance_spec_json() -> String {
+        serde_json::json!({
+            "box_id": "box-test",
+            "cpus": null,
+            "memory_mib": null,
+            "fs_shares": { "shares": [] },
+            "block_devices": { "devices": [] },
+            "guest_entrypoint": {
+                "executable": "/boxlite/bin/boxlite-guest",
+                "args": ["--listen", "vsock://2695"],
+                "env": []
+            },
+            "transport": { "Unix": { "socket_path": "/tmp/box.sock" } },
+            "ready_transport": { "Unix": { "socket_path": "/tmp/ready.sock" } },
+            "guest_rootfs": {
+                "path": "/tmp/rootfs",
+                "strategy": "Direct",
+                "kernel": null,
+                "initrd": null,
+                "env": []
+            },
+            "network_config": null,
+            "home_dir": "/tmp/home",
+            "console_output": "/tmp/console.log",
+            "exit_file": "/tmp/exit",
+            "detach": false,
+            "parent_pid": 1
+        })
+        .to_string()
+    }
+
+    #[test]
+    fn shim_args_accepts_legacy_config_flag() {
+        let args =
+            ShimArgs::try_parse_from(["boxlite-shim", "--engine", "libkrun", "--config", "{}"])
+                .unwrap();
+        assert!(args.config.is_some());
+        assert!(args.config_file.is_none());
+    }
+
+    #[test]
+    fn shim_args_accepts_config_file_flag() {
+        let args = ShimArgs::try_parse_from([
+            "boxlite-shim",
+            "--engine",
+            "libkrun",
+            "--config-file",
+            "/tmp/shim-config.json",
+        ])
+        .unwrap();
+        assert!(args.config.is_none());
+        assert_eq!(
+            args.config_file.as_deref(),
+            Some(Path::new("/tmp/shim-config.json"))
+        );
+    }
+
+    #[test]
+    fn load_instance_spec_from_legacy_config() {
+        let args = ShimArgs {
+            engine: VmmKind::Libkrun,
+            config: Some(test_instance_spec_json()),
+            config_file: None,
+        };
+
+        let parsed = load_instance_spec(&args).unwrap();
+        assert_eq!(parsed.box_id, "box-test");
+    }
+
+    #[test]
+    fn load_instance_spec_from_config_file() {
+        let mut file = tempfile::NamedTempFile::new().unwrap();
+        file.write_all(test_instance_spec_json().as_bytes())
+            .unwrap();
+
+        let args = ShimArgs {
+            engine: VmmKind::Libkrun,
+            config: None,
+            config_file: Some(file.path().to_path_buf()),
+        };
+
+        let parsed = load_instance_spec(&args).unwrap();
+        assert_eq!(parsed.box_id, "box-test");
+    }
+
+    #[test]
+    fn load_instance_spec_from_config_file_with_large_payload() {
+        let mut json: Value = serde_json::from_str(&test_instance_spec_json()).unwrap();
+        let large_env: Vec<(String, String)> = (0..2000)
+            .map(|i| (format!("KEY_{i}"), "x".repeat(128)))
+            .collect();
+        json["guest_entrypoint"]["env"] = serde_json::json!(large_env);
+
+        let mut file = tempfile::NamedTempFile::new().unwrap();
+        file.write_all(json.to_string().as_bytes()).unwrap();
+
+        let args = ShimArgs {
+            engine: VmmKind::Libkrun,
+            config: None,
+            config_file: Some(file.path().to_path_buf()),
+        };
+
+        let parsed = load_instance_spec(&args).unwrap();
+        assert_eq!(parsed.box_id, "box-test");
+        assert_eq!(parsed.guest_entrypoint.env.len(), 2000);
+    }
+}

boxlite/src/litebox/box_impl.rs

@@ -203,20 +203,23 @@ impl BoxImpl {
 
         let live = self.live_state().await?;
 
-        // Inject container ID into environment if not already set
-        let command = if command
-            .env
-            .as_ref()
-            .map(|env| env.iter().any(|(k, _)| k == executor_const::ENV_VAR))
-            .unwrap_or(false)
-        {
-            command
-        } else {
-            command.env(
+        // Inject container ID into environment if not already set.
+        let mut command = command;
+        if effective_env_value(&command, executor_const::ENV_VAR).is_none() {
+            command = command.env(
                 executor_const::ENV_VAR,
                 format!("{}={}", executor_const::CONTAINER_KEY, self.container_id()),
-            )
-        };
+            );
+        }
+
+        // For explicit guest execution, merge box-level defaults at exec-time.
+        // Command-level env entries must take precedence.
+        if matches!(
+            effective_env_value(&command, executor_const::ENV_VAR),
+            Some(v) if v == executor_const::GUEST
+        ) {
+            command = merge_box_env_for_guest_exec(command, &self.config.options.env);
+        }
 
         // Set working directory from BoxOptions if not set in command
         let command = match (&command.working_dir, &self.config.options.working_dir) {
@@ -647,6 +650,31 @@ impl crate::runtime::backend::BoxBackend for BoxImpl {
     }
 }
 
+fn effective_env_value<'a>(command: &'a BoxCommand, key: &str) -> Option<&'a str> {
+    command.env.as_ref().and_then(|env| {
+        env.iter()
+            .rev()
+            .find(|(k, _)| k == key)
+            .map(|(_, v)| v.as_str())
+    })
+}
+
+fn merge_box_env_for_guest_exec(
+    mut command: BoxCommand,
+    box_env: &[(String, String)],
+) -> BoxCommand {
+    if box_env.is_empty() {
+        return command;
+    }
+
+    let mut merged_env = box_env.to_vec();
+    if let Some(command_env) = command.env.take() {
+        merged_env.extend(command_env);
+    }
+    command.env = Some(merged_env);
+    command
+}
+
 fn build_tar_from_host(
     src: &std::path::Path,
     tar_path: &std::path::Path,

boxlite/src/litebox/init/tasks/guest_entrypoint.rs

@@ -92,20 +92,11 @@ impl GuestEntrypointBuilder {
 
     /// Add an env var, using FILO semantics (later calls override earlier ones).
     ///
-    /// If a var with the same key exists, it's removed and its space is reclaimed
-    /// before adding the new value.
+    /// If a var with the same key exists, replacement is atomic: when the new value
+    /// is invalid or doesn't fit, the old value is preserved.
     ///
     /// Returns `true` if added, `false` if skipped (logged as warning).
     pub fn with_env(&mut self, key: &str, value: &str) -> bool {
-        // FILO: Remove existing key if present, reclaim space
-        if let Some(pos) = self.env.iter().position(|(k, _)| k == key) {
-            let (old_key, old_value) = &self.env[pos];
-            let old_size = old_key.len() + old_value.len() + Self::ENV_VAR_OVERHEAD;
-            self.total_size = self.total_size.saturating_sub(old_size);
-            self.env.remove(pos);
-            tracing::trace!(env_key = %key, "Overriding existing env var");
-        }
-
         // Check ASCII
         if !is_printable_ascii(key) || !is_printable_ascii(value) {
             tracing::warn!(
@@ -118,18 +109,37 @@ impl GuestEntrypointBuilder {
 
         // Check size limit
         let var_size = key.len() + value.len() + Self::ENV_VAR_OVERHEAD;
-        if self.total_size + var_size > self.limit {
+        let existing = self.env.iter().position(|(k, _)| k == key).map(|pos| {
+            let old_size = {
+                let (old_key, old_value) = &self.env[pos];
+                old_key.len() + old_value.len() + Self::ENV_VAR_OVERHEAD
+            };
+            (pos, old_size)
+        });
+        let projected_total = match existing {
+            Some((_, old_size)) => self.total_size.saturating_sub(old_size) + var_size,
+            None => self.total_size + var_size,
+        };
+
+        if projected_total > self.limit {
             tracing::warn!(
                 env_key = %key,
                 env_value = %Self::redact_for_log(value),
                 total_size = self.total_size,
+                projected_total = projected_total,
                 var_size,
                 limit = self.limit,
                 "Skipping env var: kernel cmdline size limit reached"
             );
             return false;
         }
 
+        if let Some((pos, old_size)) = existing {
+            self.total_size = self.total_size.saturating_sub(old_size);
+            self.env.remove(pos);
+            tracing::trace!(env_key = %key, "Overriding existing env var");
+        }
+
         self.total_size += var_size;
         self.env.push((key.to_string(), value.to_string()));
         tracing::trace!(env_key = %key, var_size, "Added env var");
@@ -184,3 +194,47 @@ impl GuestEntrypointBuilder {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::GuestEntrypointBuilder;
+
+    #[test]
+    fn with_env_oversized_override_preserves_existing_value() {
+        let mut builder = GuestEntrypointBuilder::new("/boxlite/bin/boxlite-guest".to_string());
+        assert!(builder.with_env("PATH", "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin"));
+
+        let long_path = "x".repeat(10_000);
+        assert!(!builder.with_env("PATH", &long_path));
+
+        let entrypoint = builder.build();
+        let path = entrypoint
+            .env
+            .iter()
+            .find(|(k, _)| k == "PATH")
+            .map(|(_, v)| v.as_str());
+        assert_eq!(
+            path,
+            Some("/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin")
+        );
+    }
+
+    #[test]
+    fn with_env_invalid_override_preserves_existing_value() {
+        let mut builder = GuestEntrypointBuilder::new("/boxlite/bin/boxlite-guest".to_string());
+        assert!(builder.with_env("PATH", "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin"));
+
+        assert!(!builder.with_env("PATH", "ok\u{0080}bad"));
+
+        let entrypoint = builder.build();
+        let path = entrypoint
+            .env
+            .iter()
+            .find(|(k, _)| k == "PATH")
+            .map(|(_, v)| v.as_str());
+        assert_eq!(
+            path,
+            Some("/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin")
+        );
+    }
+}