diff --git a/Cargo.lock b/Cargo.lock index 272f74930..7ae1dd014 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -574,6 +574,7 @@ dependencies = [ "comfy-table", "dialoguer", "dirs 6.0.0", + "dotenvy", "futures", "gtmpl", "gtmpl_value", @@ -1425,6 +1426,12 @@ dependencies = [ "litrs", ] +[[package]] +name = "dotenvy" +version = "0.15.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1aaf95b3e5c8f23aa320147307562d361db0ae0d51242340f558153b4eb2439b" + [[package]] name = "dunce" version = "1.0.5" diff --git a/docs/reference/cli/README.md b/docs/reference/cli/README.md index 7b60ca7bd..73eeabf8f 100644 --- a/docs/reference/cli/README.md +++ b/docs/reference/cli/README.md @@ -256,13 +256,14 @@ takes the command's exit code; `boxlite ps` shows it stopped and - Default (foreground): streams stdout/stderr to the terminal, exits with the box command's exit code. If the command was killed by signal *N*, exits with `128 + N` (Unix convention, see [Exit Codes](#exit-codes)). - `-d`/`--detach`: prints the box ID to stdout and exits `0` immediately; `auto_remove` is force-disabled in this mode so the box outlives the CLI process. -- `--tty` with non-TTY stdin: fails with `the input device is not a TTY.` +- Foreground `--tty` with non-TTY stdin: fails with `the input device is not a TTY.` Detached `-d -t` is allowed because the command does not attach to local stdin. **Examples:** ```bash boxlite run alpine:latest echo "Hello" boxlite run -it --rm alpine:latest /bin/sh +boxlite run --env-file .env -e DEBUG=1 python:slim python app.py boxlite run -d --name web -p 8080:80 nginx:alpine boxlite run -v $(pwd):/work -w /work alpine:latest ls -la boxlite run --cpus 4 --memory 4096 python:slim python -c "print(2+2)" @@ -291,6 +292,7 @@ Run a command in a *running* box. The `--` separator is required (`src/cli/src/c boxlite exec mybox -- echo "hello" boxlite exec -it mybox -- /bin/sh boxlite exec -e DEBUG=1 -w /app mybox -- pytest tests/ +boxlite exec --env-file test.env mybox -- pytest tests/ ``` --- @@ -319,16 +321,20 @@ implicitly, because starting it runs that command. Start it deliberately with |------|-------|-------------| | `--rootfs PATH` | — | Use a prepared rootfs path instead of pulling/resolving an image | | `--env KEY=VALUE` | `-e` | Set environment variables (repeatable) | +| `--env-file FILE` | — | Read environment variables from a file (repeatable) | | `--workdir PATH` | `-w` | Working directory inside the box | Also uses [`ResourceFlags`](#resourceflags) + [`PublishFlags`](#publishflags) + [`VolumeFlags`](#volumeflags) + [`ManagementFlags`](#managementflags). -> Note: `create` accepts `--env` and `--workdir` directly rather than via `ProcessFlags` (no `-i`/`-t`/`-u` here, since no command is being executed). +> Note: `create` flattens `EnvironmentFlags` directly and defines `--workdir` +> itself rather than using `ProcessFlags`; it therefore does not expose +> `-i`/`-t`/`-u`, since no command is executed. **Examples:** ```bash boxlite create --name mybox alpine:latest +boxlite create --env-file .env --name configured alpine:latest boxlite create -p 8080:80 -v /data:/app/data --name web nginx:alpine boxlite create --rootfs /path/to/rootfs --name local-rootfs ``` @@ -599,19 +605,48 @@ boxlite completion fish > ~/.config/fish/completions/boxlite.fish Several commands flatten shared `clap` `Args` structs. Each is documented here once. +### `EnvironmentFlags` + +Used by `run`, `create`, and `exec`. + +| Flag | Short | Description | +|------|-------|-------------| +| `--env KEY=VALUE` | `-e` | Set an environment variable; a bare key inherits from the host | +| `--env-file FILE` | — | Read environment variables from a file; repeatable | + +Environment behavior: + +- **Syntax:** Environment files use dotenv syntax, including quoted values, + `export` prefixes, comments, escapes, and variable substitution. A UTF-8 BOM + at the start of a file is ignored. +- **Precedence:** Files are applied in command-line order. Later files override + earlier files, and explicit `-e` values have the highest precedence. During + substitution, host values take precedence over values defined earlier in the + same file. If the host variable is unset, an earlier same-file value is used. +- **Substitution scope:** Each file is evaluated independently, so references + cannot use values defined by an earlier `--env-file`. +- **Undefined substitutions:** An undefined reference expands to an empty string + without a warning. Escape `$` or use single quotes to preserve it literally. +- **Bare `-e` keys:** A bare key inherits its value from the host. If absent + there, it remains unset and removes any value loaded from an earlier + `--env-file`. +- **Secrets:** Environment-file values enter the guest. Use BoxLite secret + injection for credentials that must remain outside the VM. + ### `ProcessFlags` -Used by `run` and `exec` (defined at `src/cli/src/cli.rs:208-281`). +Used by `run` and `exec` and includes [`EnvironmentFlags`](#environmentflags). | Flag | Short | Description | |------|-------|-------------| | `--interactive` | `-i` | Keep STDIN open even if not attached | | `--tty` | `-t` | Allocate a pseudo-TTY (stdout and stderr are merged in TTY mode) | -| `--env KEY=VALUE` | `-e` | Set environment variables (repeatable; if value omitted, inherits from host) | | `--workdir PATH` | `-w` | Working directory inside the box | | `--user NAME[:GROUP]` | `-u` | Run as `name`/`uid`[:`group`/`gid`] | -`--tty` implies `--interactive` when stdin is a TTY. `--tty` without a TTY-attached stdin is a hard error. +In foreground mode, `--tty` implies `--interactive` when stdin is a TTY and +fails when stdin is not a TTY. Detached `-d -t` is allowed because the command +does not attach to local stdin. ### `ResourceFlags` diff --git a/src/cli/Cargo.toml b/src/cli/Cargo.toml index f13f84a4c..de6ceabce 100644 --- a/src/cli/Cargo.toml +++ b/src/cli/Cargo.toml @@ -26,6 +26,7 @@ futures = "0.3" term_size = "0.3" nix = { version = "0.30.1", features = ["term", "signal"] } anyhow = "1.0" +dotenvy = "0.15.7" tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["fmt", "env-filter", "json"] } tracing-appender = "0.2" diff --git a/src/cli/README.md b/src/cli/README.md index a7d28d50a..854059d4c 100644 --- a/src/cli/README.md +++ b/src/cli/README.md @@ -317,6 +317,7 @@ silently restarting it, because restarting would run the command a second time. | `--interactive` | `-i` | Keep STDIN open | | `--tty` | `-t` | Allocate a pseudo-TTY | | `--env KEY=VALUE` | `-e` | Set environment variables (repeatable) | +| `--env-file FILE` | | Read environment variables from a file (repeatable) | | `--workdir PATH` | `-w` | Working directory in the box | | `--publish PORT` | `-p` | Publish box port to host (e.g. `8080:80`, `8080:80/tcp`) | | `--volume VOLUME` | `-v` | Mount a volume (e.g. `hostPath:boxPath`, `boxPath` for anonymous) | @@ -331,6 +332,7 @@ silently restarting it, because restarting would run the command a second time. ```bash boxlite run alpine:latest echo "Hello" boxlite run -it --rm alpine:latest /bin/sh +boxlite run --env-file .env -e DEBUG=1 python:slim python app.py boxlite run -d --name openclaw -p 18789:18789 ghcr.io/openclaw/openclaw:main boxlite run -v /host/data:/app/data alpine:latest cat /app/data/hello.txt boxlite run --rootfs /path/to/rootfs /bin/sh @@ -356,6 +358,7 @@ default, and `exec` still starts it on demand. | `--rootfs PATH` | | Use a prepared rootfs path instead of pulling/resolving an image | | `--name NAME` | | Name the box | | `--env KEY=VALUE` | `-e` | Environment variables | +| `--env-file FILE` | | Read environment variables from a file (repeatable) | | `--workdir PATH` | `-w` | Working directory | | `--publish PORT` | `-p` | Publish box port to host (e.g. `8080:80`) | | `--volume VOLUME` | `-v` | Mount a volume (e.g. `hostPath:boxPath`, or box path for anonymous) | @@ -368,6 +371,7 @@ default, and `exec` still starts it on demand. ```bash boxlite create --name mybox alpine:latest +boxlite create --env-file .env --name configured alpine:latest boxlite create -p 18789:18789 -v /data:/app/data --name openclaw ghcr.io/openclaw/openclaw:main boxlite create --rootfs /path/to/rootfs --name local-rootfs boxlite start mybox @@ -385,15 +389,36 @@ Run a command in a running box. | `--interactive` | `-i` | Keep STDIN open | | `--tty` | `-t` | Allocate a TTY | | `--env KEY=VALUE` | `-e` | Environment variables | +| `--env-file FILE` | | Read environment variables from a file (repeatable) | | `--workdir PATH` | `-w` | Working directory | | `--detach` | `-d` | Run in background (don’t wait) | **Example:** ```bash -boxlite exec -it mybox /bin/sh +boxlite exec -it mybox -- /bin/sh +boxlite exec --env-file test.env mybox -- pytest ``` +#### Environment behavior for `run`, `create`, and `exec` + +- **Syntax:** Environment files use dotenv syntax, including quoted values, + `export` prefixes, comments, escapes, and variable substitution. A UTF-8 BOM + at the start of a file is ignored. +- **Precedence:** Later files override earlier files, and explicit `-e` values + override all files. During substitution, host values take precedence over + values defined earlier in the same file. If the host variable is unset, an + earlier same-file value is used. +- **Substitution scope:** Each file is evaluated independently, so references + cannot use values defined by an earlier `--env-file`. +- **Undefined substitutions:** An undefined reference expands to an empty string + without a warning. Escape `$` or use single quotes to preserve it literally. +- **Bare `-e` keys:** A bare key inherits its value from the host. If absent + there, it remains unset and removes any value loaded from an earlier + `--env-file`. +- **Secrets:** Environment-file values enter the box and should not replace + BoxLite secret injection. + ### `boxlite list` (alias: `ls`, `ps`) List boxes. diff --git a/src/cli/src/cli.rs b/src/cli/src/cli.rs index b75d85e8b..d97e27970 100644 --- a/src/cli/src/cli.rs +++ b/src/cli/src/cli.rs @@ -2,6 +2,7 @@ //! This module contains all CLI-related code including the main CLI structure, //! subcommands, and flag definitions. +pub use crate::environment::EnvironmentFlags; use boxlite::runtime::options::{NetworkConfig, NetworkMode, PortProtocol, PortSpec, VolumeSpec}; use boxlite::{ BoxCommand, BoxOptions, BoxliteOptions, BoxliteRestOptions, BoxliteRuntime, ImageRegistry, @@ -12,30 +13,6 @@ use clap_complete::shells::{Bash, Fish, Zsh}; use std::io::{IsTerminal, Write}; use std::path::Path; -/// Helper to parse CLI environment variables and apply them to BoxOptions -pub fn apply_env_vars(env: &[String], opts: &mut BoxOptions) { - apply_env_vars_with_lookup(env, opts, |k| std::env::var(k).ok()) -} - -/// Helper to parse CLI environment variables with custom lookup for host variables -pub fn apply_env_vars_with_lookup(env: &[String], opts: &mut BoxOptions, lookup: F) -where - F: Fn(&str) -> Option, -{ - for env_str in env { - if let Some((k, v)) = env_str.split_once('=') { - opts.env.push((k.to_string(), v.to_string())); - } else if let Some(val) = lookup(env_str) { - opts.env.push((env_str.to_string(), val)); - } else { - tracing::warn!( - "Environment variable '{}' not found on host, skipping", - env_str - ); - } - } -} - // ============================================================================ // CLI Definition // ============================================================================ @@ -311,6 +288,34 @@ impl GlobalFlags { // PROCESS FLAGS // ============================================================================ +pub(crate) struct BoxProcessOptions<'a> { + working_dir: Option<&'a str>, + environment: &'a [(String, String)], + entrypoint: Option<&'a str>, +} + +impl<'a> BoxProcessOptions<'a> { + pub(crate) fn new( + working_dir: Option<&'a str>, + environment: &'a [(String, String)], + entrypoint: Option<&'a str>, + ) -> Self { + Self { + working_dir, + environment, + entrypoint, + } + } + + pub(crate) fn apply_to(&self, opts: &mut BoxOptions) { + opts.working_dir = self.working_dir.map(str::to_string); + opts.env.extend(self.environment.iter().cloned()); + if let Some(exec) = self.entrypoint { + opts.entrypoint = Some(vec![exec.to_string()]); + } + } +} + #[derive(Args, Debug, Clone)] pub struct ProcessFlags { /// Keep STDIN open even if not attached @@ -321,9 +326,8 @@ pub struct ProcessFlags { #[arg(short, long)] pub tty: bool, - /// Set environment variables - #[arg(short = 'e', long = "env")] - pub env: Vec, + #[command(flatten)] + pub environment: EnvironmentFlags, /// Working directory inside the box #[arg(short = 'w', long = "workdir")] @@ -343,27 +347,20 @@ pub struct ProcessFlags { impl ProcessFlags { /// Apply process configuration to BoxOptions - pub fn apply_to(&self, opts: &mut BoxOptions) -> anyhow::Result<()> { - self.apply_to_with_lookup(opts, |k| std::env::var(k).ok()) - } + pub fn apply_to(&self, opts: &mut BoxOptions, environment: &[(String, String)]) { + BoxProcessOptions::new( + self.workdir.as_deref(), + environment, + self.entrypoint.as_deref(), + ) + .apply_to(opts); - /// Internal helper for dependency injection of environment variables - fn apply_to_with_lookup(&self, opts: &mut BoxOptions, lookup: F) -> anyhow::Result<()> - where - F: Fn(&str) -> Option, - { - opts.working_dir = self.workdir.clone(); - apply_env_vars_with_lookup(&self.env, opts, lookup); - if let Some(ref exec) = self.entrypoint { - opts.entrypoint = Some(vec![exec.clone()]); - } if let Some(ref user) = self.user { opts.user = Some(user.clone()); } // `-t` is a property of the container's init, which COMMAND now is, so // it has to be decided here at create time rather than at attach. opts.tty = self.tty; - Ok(()) } /// Validate process flags @@ -377,13 +374,13 @@ impl ProcessFlags { } /// Configures a BoxCommand with process flags (env, workdir, tty) - pub fn configure_command(&self, mut cmd: BoxCommand) -> BoxCommand { - for env_str in &self.env { - if let Some((k, v)) = env_str.split_once('=') { - cmd = cmd.env(k, v); - } else if let Ok(val) = std::env::var(env_str) { - cmd = cmd.env(env_str, val); - } + pub fn configure_command( + &self, + mut cmd: BoxCommand, + environment: &[(String, String)], + ) -> BoxCommand { + for (key, value) in environment { + cmd = cmd.env(key, value); } if let Some(ref w) = self.workdir { @@ -801,36 +798,54 @@ impl ManagementFlags { mod tests { use super::*; use std::fs; + use std::path::PathBuf; use tempfile::TempDir; #[test] - fn test_apply_env_vars_with_lookup() { - let mut opts = BoxOptions::default(); - let current_env = vec![ - "TEST_VAR=test_value".to_string(), - "TEST_HOST_VAR".to_string(), - "NON_EXISTENT_VAR".to_string(), - ]; - - apply_env_vars_with_lookup(¤t_env, &mut opts, |k| { - if k == "TEST_HOST_VAR" { - Some("host_value".to_string()) - } else { - None - } - }); - - assert!( - opts.env - .contains(&("TEST_VAR".to_string(), "test_value".to_string())) + fn env_file_flag_is_available_on_run_create_and_exec() { + let run = Cli::try_parse_from(["boxlite", "run", "--env-file", "run.env", "alpine:latest"]) + .unwrap(); + let Commands::Run(run_args) = run.command else { + panic!("expected run command"); + }; + assert_eq!( + run_args.process.environment.env_files, + vec![PathBuf::from("run.env")] ); - assert!( - opts.env - .contains(&("TEST_HOST_VAR".to_string(), "host_value".to_string())) + let create = Cli::try_parse_from([ + "boxlite", + "create", + "--env-file", + "create.env", + "alpine:latest", + ]) + .unwrap(); + let Commands::Create(create_args) = create.command else { + panic!("expected create command"); + }; + assert_eq!( + create_args.environment.env_files, + vec![PathBuf::from("create.env")] ); - assert!(!opts.env.iter().any(|(k, _)| k == "NON_EXISTENT_VAR")); + let exec = Cli::try_parse_from([ + "boxlite", + "exec", + "--env-file", + "exec.env", + "box-id", + "--", + "env", + ]) + .unwrap(); + let Commands::Exec(exec_args) = exec.command else { + panic!("expected exec command"); + }; + assert_eq!( + exec_args.process.environment.env_files, + vec![PathBuf::from("exec.env")] + ); } #[test] @@ -1087,7 +1102,7 @@ mod tests { ProcessFlags { interactive: false, tty: false, - env: Vec::new(), + environment: EnvironmentFlags::default(), workdir: None, user: None, entrypoint: entrypoint.map(str::to_string), @@ -1099,9 +1114,7 @@ mod tests { // --entrypoint reaches BoxOptions.entrypoint as a single-token // argv, which container_rootfs applies as config.entrypoint. let mut opts = BoxOptions::default(); - process_flags_with_entrypoint(Some("/bin/bash")) - .apply_to(&mut opts) - .expect("entrypoint apply"); + process_flags_with_entrypoint(Some("/bin/bash")).apply_to(&mut opts, &[]); assert_eq!(opts.entrypoint, Some(vec!["/bin/bash".to_string()])); } @@ -1118,7 +1131,7 @@ mod tests { let mut flags = process_flags_with_entrypoint(None); flags.tty = true; - flags.apply_to(&mut opts).expect("tty apply"); + flags.apply_to(&mut opts, &[]); assert!(opts.tty, "-t must make the main command a terminal"); } @@ -1128,13 +1141,24 @@ mod tests { // No --entrypoint leaves BoxOptions.entrypoint None so the image's // own entrypoint is used unchanged. let mut opts = BoxOptions::default(); - process_flags_with_entrypoint(None) - .apply_to(&mut opts) - .expect("no-op apply"); + process_flags_with_entrypoint(None).apply_to(&mut opts, &[]); assert_eq!(opts.entrypoint, None); } + #[test] + fn box_process_options_apply_box_level_process_configuration() { + let environment = vec![("KEY".to_string(), "value".to_string())]; + let mut opts = BoxOptions::default(); + + BoxProcessOptions::new(Some("/workspace"), &environment, Some("/bin/sh")) + .apply_to(&mut opts); + + assert_eq!(opts.working_dir.as_deref(), Some("/workspace")); + assert_eq!(opts.env, environment); + assert_eq!(opts.entrypoint, Some(vec!["/bin/sh".to_string()])); + } + #[test] fn test_parse_publish_spec_host_box() { let spec = super::parse_publish_spec("18789:18789").unwrap(); diff --git a/src/cli/src/commands/create.rs b/src/cli/src/commands/create.rs index a70c1e057..6dba130aa 100644 --- a/src/cli/src/commands/create.rs +++ b/src/cli/src/commands/create.rs @@ -1,4 +1,7 @@ -use crate::cli::{GlobalFlags, NetworkFlags, PublishFlags, ResourceFlags, VolumeFlags}; +use crate::cli::{ + BoxProcessOptions, EnvironmentFlags, GlobalFlags, NetworkFlags, PublishFlags, ResourceFlags, + VolumeFlags, +}; use boxlite::{BoxOptions, RootfsSpec}; use clap::Args; @@ -16,9 +19,8 @@ pub struct CreateArgs { #[command(flatten)] pub management: crate::cli::ManagementFlags, - /// Set environment variables - #[arg(short = 'e', long = "env")] - pub env: Vec, + #[command(flatten)] + pub environment: EnvironmentFlags, /// Working directory inside the box #[arg(short = 'w', long = "workdir")] @@ -59,13 +61,13 @@ pub async fn execute(args: CreateArgs, global: &GlobalFlags) -> anyhow::Result<( impl CreateArgs { fn to_box_options(&self, global: &GlobalFlags) -> anyhow::Result { + let environment = self.environment.resolve()?; let mut options = BoxOptions::default(); self.resource.apply_to(&mut options); self.management.apply_to(&mut options)?; self.publish.apply_to(&mut options)?; self.volume.apply_to(&mut options, global.home.as_deref())?; self.network.apply_to(&mut options)?; - // A `create`d box is a background box: `create` then `start`/`exec` runs // its main command detached (docker's create → start), so the launching // CLI's exit must not tear it down. Foreground/interactive lifecycles go @@ -78,14 +80,15 @@ impl CreateArgs { // rejected at sanitize). options.detach = true; options.auto_delete = Some(0); - options.working_dir = self.workdir.clone(); - if let Some(ref exec) = self.entrypoint { - options.entrypoint = Some(vec![exec.clone()]); - } + BoxProcessOptions::new( + self.workdir.as_deref(), + &environment, + self.entrypoint.as_deref(), + ) + .apply_to(&mut options); if !self.command.is_empty() { options.cmd = Some(self.command.clone()); } - crate::cli::apply_env_vars(&self.env, &mut options); options.rootfs = self.rootfs_spec()?; Ok(options) } @@ -152,4 +155,39 @@ mod tests { assert!(err.to_string().contains("either IMAGE or --rootfs")); } + + #[test] + fn create_env_file_error_does_not_create_anonymous_volume() { + let temp_dir = tempfile::tempdir().unwrap(); + let home = temp_dir.path().join("home"); + let missing_env_file = temp_dir.path().join("missing.env"); + let args: Vec = vec![ + "boxlite".into(), + "--home".into(), + home.as_os_str().to_owned(), + "create".into(), + "--env-file".into(), + missing_env_file.as_os_str().to_owned(), + "--volume".into(), + "/data".into(), + "alpine".into(), + ]; + let cli = Cli::try_parse_from(args).expect("create arguments should parse"); + let Commands::Create(args) = cli.command else { + panic!("expected create command"); + }; + + let err = args + .to_box_options(&cli.global) + .expect_err("missing environment file must be rejected"); + + assert!( + format!("{err:#}").contains(&missing_env_file.display().to_string()), + "error should identify the missing environment file" + ); + assert!( + !home.join("volumes/anonymous").exists(), + "environment errors must not create anonymous volumes" + ); + } } diff --git a/src/cli/src/commands/exec.rs b/src/cli/src/commands/exec.rs index 47f535ee6..623c283b0 100644 --- a/src/cli/src/commands/exec.rs +++ b/src/cli/src/commands/exec.rs @@ -31,8 +31,10 @@ pub struct ExecArgs { /// below is the graceful path (Guest.Shutdown RPC); Drop is the backstop. /// Mirrors the RAII fix in [`super::run::execute`] (#622). pub async fn execute(args: ExecArgs, global: &GlobalFlags) -> anyhow::Result { + args.process.validate(args.detach)?; + let environment = args.process.environment.resolve()?; let mut executor = BoxExecutor::new(args, global)?; - executor.execute().await + executor.execute(environment).await } struct BoxExecutor { @@ -46,10 +48,9 @@ impl BoxExecutor { Ok(Self { args, rt }) } - async fn execute(&mut self) -> anyhow::Result { - self.args.process.validate(self.args.detach)?; + async fn execute(&mut self, environment: Vec<(String, String)>) -> anyhow::Result { let litebox = self.get_box().await?; - let cmd = self.prepare_command(); + let cmd = self.prepare_command(&environment); let mut execution = litebox.exec(cmd).await?; // Detach mode: Exit immediately without waiting @@ -88,8 +89,8 @@ impl BoxExecutor { .ok_or_else(|| anyhow::anyhow!("No such box: {}", self.args.target_box)) } - fn prepare_command(&self) -> BoxCommand { + fn prepare_command(&self, environment: &[(String, String)]) -> BoxCommand { let cmd = BoxCommand::new(&self.args.command[0]).args(&self.args.command[1..]); - self.args.process.configure_command(cmd) + self.args.process.configure_command(cmd, environment) } } diff --git a/src/cli/src/commands/run.rs b/src/cli/src/commands/run.rs index d3504daf6..34c84c0c4 100644 --- a/src/cli/src/commands/run.rs +++ b/src/cli/src/commands/run.rs @@ -6,7 +6,6 @@ use crate::terminal::StreamManager; use crate::util::to_shell_exit_code; use boxlite::{BoxOptions, BoxliteRuntime, LiteBox, RootfsSpec}; use clap::Args; -use std::io::{self, IsTerminal}; #[derive(Args, Debug)] pub struct RunArgs { @@ -48,8 +47,10 @@ pub struct RunArgs { pub async fn execute(args: RunArgs, global: &GlobalFlags) -> anyhow::Result { let (rootfs, command_args) = args.rootfs_and_command()?; let command_args = command_args.to_vec(); + args.process.validate(args.management.detach)?; + let environment = args.process.environment.resolve()?; let mut runner = BoxRunner::new(args, global)?; - runner.run(rootfs, command_args).await + runner.run(rootfs, command_args, environment).await } struct BoxRunner { @@ -66,14 +67,16 @@ impl BoxRunner { Ok(Self { args, rt, home }) } - async fn run(&mut self, rootfs: RootfsSpec, command_args: Vec) -> anyhow::Result { - // Validate flags and environment - self.validate_flags()?; - + async fn run( + &mut self, + rootfs: RootfsSpec, + command_args: Vec, + environment: Vec<(String, String)>, + ) -> anyhow::Result { // COMMAND becomes the container's init (docker semantics — it replaces // the image CMD via options.cmd in create_box), so there is nothing to // spawn here: attach to the init session instead. - let litebox = self.create_box(rootfs, &command_args).await?; + let litebox = self.create_box(rootfs, &command_args, &environment).await?; // Detach mode: start it and get out of the way. Nobody is reading the // output, so there is nothing to be attached for. @@ -91,7 +94,7 @@ impl BoxRunner { litebox.start().await?; // --tty implies --interactive when stdin is a terminal - // (validate_flags already ensures stdin is a terminal when --tty is set) + // (ProcessFlags::validate already checked stdin before runtime creation) if self.args.process.tty { self.args.process.interactive = true; } @@ -118,6 +121,7 @@ impl BoxRunner { &self, rootfs: RootfsSpec, command_args: &[String], + environment: &[(String, String)], ) -> anyhow::Result { let mut options = BoxOptions::default(); self.args.resource.apply_to(&mut options); @@ -127,7 +131,7 @@ impl BoxRunner { .volume .apply_to(&mut options, self.home.as_deref())?; self.args.network.apply_to(&mut options)?; - self.args.process.apply_to(&mut options)?; + self.args.process.apply_to(&mut options, environment); // Detached boxes keep manual lifecycle control: detach silently // overrides --rm (historical CLI behavior). @@ -151,15 +155,6 @@ impl BoxRunner { Ok(litebox) } - - fn validate_flags(&self) -> anyhow::Result<()> { - // Check TTY availability if requested - if self.args.process.tty && !io::stdin().is_terminal() { - anyhow::bail!("the input device is not a TTY."); - } - - Ok(()) - } } impl RunArgs { diff --git a/src/cli/src/environment.rs b/src/cli/src/environment.rs new file mode 100644 index 000000000..d0762937a --- /dev/null +++ b/src/cli/src/environment.rs @@ -0,0 +1,310 @@ +//! Environment variable flags and resolution for CLI commands. + +use anyhow::Context; +use clap::Args; +use std::fs; +use std::path::PathBuf; + +/// Environment variables shared by `run`, `create`, and `exec`. +#[derive(Args, Debug, Clone, Default)] +pub struct EnvironmentFlags { + /// Read environment variables from a file (repeatable) + #[arg(long = "env-file", value_name = "FILE")] + pub env_files: Vec, + + /// Set environment variables + #[arg(short = 'e', long = "env")] + pub env: Vec, +} + +impl EnvironmentFlags { + /// Resolve files first, then explicit `-e` values. + /// + /// Later definitions replace earlier ones while retaining deterministic + /// output order. A bare `--env` key inherits its value from the host + /// environment. + pub fn resolve(&self) -> anyhow::Result> { + self.resolve_with_lookup(|key| std::env::var(key).ok()) + } + + fn resolve_with_lookup(&self, lookup: F) -> anyhow::Result> + where + F: Fn(&str) -> Option, + { + let mut resolved = Vec::new(); + + for path in &self.env_files { + let contents = fs::read(path) + .with_context(|| format!("failed to read environment file '{}'", path.display()))?; + let contents = contents.strip_prefix(b"\xEF\xBB\xBF").unwrap_or(&contents); + let entries = dotenvy::from_read_iter(contents); + for entry in entries { + // dotenvy errors can include the raw line, so do not preserve + // the original error in the user-visible error chain. + let (key, value) = entry.map_err(|_| { + anyhow::anyhow!("failed to process environment file '{}'", path.display()) + })?; + set_environment_value(&mut resolved, key, value); + } + } + + for entry in &self.env { + apply_explicit_environment_entry(&mut resolved, entry, &lookup); + } + + Ok(resolved) + } +} + +fn apply_explicit_environment_entry( + resolved: &mut Vec<(String, String)>, + entry: &str, + lookup: &F, +) where + F: Fn(&str) -> Option, +{ + let (key, explicit_value) = match entry.split_once('=') { + Some((key, value)) => (key, Some(value.to_string())), + None => (entry, None), + }; + + let value = explicit_value.or_else(|| lookup(key)); + let Some(value) = value else { + resolved.retain(|(name, _)| name != key); + tracing::warn!( + env_key = key, + source = "--env", + "Environment variable not found on host, leaving unset" + ); + return; + }; + + set_environment_value(resolved, key.to_string(), value); +} + +fn set_environment_value(resolved: &mut Vec<(String, String)>, key: String, value: String) { + if let Some((_, existing_value)) = resolved.iter_mut().find(|(name, _)| name == &key) { + *existing_value = value; + } else { + resolved.push((key, value)); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use tempfile::TempDir; + + #[test] + fn resolve_files_then_explicit_values() { + let temp_dir = TempDir::new().unwrap(); + let base_file = temp_dir.path().join("base.env"); + let override_file = temp_dir.path().join("override.env"); + fs::write( + &base_file, + "# comment\nBOXLITE_DOTENV_SOURCE=one\nSHARED=base\nQUOTED='two words'\nEXPANDED=${BOXLITE_DOTENV_SOURCE}-suffix\nexport EXPORTED=yes\n", + ) + .unwrap(); + fs::write(&override_file, "\nSHARED=override\nEMPTY=\n").unwrap(); + + let flags = EnvironmentFlags { + env_files: vec![base_file, override_file], + env: vec!["SHARED=explicit".to_string(), "HOST_VALUE".to_string()], + }; + + let resolved = flags + .resolve_with_lookup(|key| (key == "HOST_VALUE").then(|| "host".to_string())) + .unwrap(); + + assert_eq!( + resolved, + vec![ + ("BOXLITE_DOTENV_SOURCE".to_string(), "one".to_string()), + ("SHARED".to_string(), "explicit".to_string()), + ("QUOTED".to_string(), "two words".to_string()), + ("EXPANDED".to_string(), "one-suffix".to_string()), + ("EXPORTED".to_string(), "yes".to_string()), + ("EMPTY".to_string(), String::new()), + ("HOST_VALUE".to_string(), "host".to_string()), + ] + ); + } + + #[test] + fn later_env_file_overrides_earlier_file() { + let temp_dir = TempDir::new().unwrap(); + let base_file = temp_dir.path().join("base.env"); + let override_file = temp_dir.path().join("override.env"); + fs::write(&base_file, "FILE_VALUE=base\n").unwrap(); + fs::write(&override_file, "FILE_VALUE=override\n").unwrap(); + let flags = EnvironmentFlags { + env_files: vec![base_file, override_file], + env: Vec::new(), + }; + + let resolved = flags.resolve_with_lookup(|_| None).unwrap(); + + assert_eq!( + resolved, + vec![("FILE_VALUE".to_string(), "override".to_string())] + ); + } + + #[test] + fn preserves_existing_explicit_environment_entry_behavior() { + let flags = EnvironmentFlags { + env_files: Vec::new(), + env: vec![String::new(), "=value".to_string(), "A B=value".to_string()], + }; + + let resolved = flags.resolve_with_lookup(|_| None).unwrap(); + + assert_eq!( + resolved, + vec![ + (String::new(), "value".to_string()), + ("A B".to_string(), "value".to_string()), + ] + ); + } + + #[test] + fn bare_explicit_key_without_host_value_removes_file_value() { + let temp_dir = TempDir::new().unwrap(); + let env_file = temp_dir.path().join("base.env"); + fs::write(&env_file, "TOKEN=file-value\n").unwrap(); + let flags = EnvironmentFlags { + env_files: vec![env_file], + env: vec!["TOKEN".to_string()], + }; + + let resolved = flags.resolve_with_lookup(|_| None).unwrap(); + + assert!(resolved.is_empty()); + } + + #[test] + fn bare_explicit_key_without_host_or_prior_value_stays_unset() { + let flags = EnvironmentFlags { + env_files: Vec::new(), + env: vec!["MISSING_TOKEN".to_string()], + }; + + let resolved = flags.resolve_with_lookup(|_| None).unwrap(); + + assert!(resolved.is_empty()); + } + + #[test] + fn each_env_file_has_independent_substitution_scope() { + let temp_dir = TempDir::new().unwrap(); + let base_file = temp_dir.path().join("base.env"); + let service_file = temp_dir.path().join("service.env"); + let source_key = format!("BOXLITE_TEST_{}", ulid::Ulid::new()); + let service_key = format!("{source_key}_SERVICE"); + fs::write(&base_file, format!("{source_key}=demo\n")).unwrap(); + fs::write( + &service_file, + format!("{service_key}=${{{source_key}}}-api\n"), + ) + .unwrap(); + + let flags = EnvironmentFlags { + env_files: vec![base_file, service_file], + env: Vec::new(), + }; + + let resolved = flags.resolve_with_lookup(|_| None).unwrap(); + + assert_eq!( + resolved, + vec![ + (source_key, "demo".to_string()), + (service_key, "-api".to_string()), + ] + ); + } + + #[test] + fn ignores_utf8_bom_at_start_of_env_file() { + let temp_dir = TempDir::new().unwrap(); + let env_file = temp_dir.path().join("bom.env"); + fs::write(&env_file, b"\xEF\xBB\xBFBOXLITE_BOM_VALUE=present\n").unwrap(); + + let flags = EnvironmentFlags { + env_files: vec![env_file], + env: Vec::new(), + }; + + let resolved = flags.resolve_with_lookup(|_| None).unwrap(); + + assert_eq!( + resolved, + vec![("BOXLITE_BOM_VALUE".to_string(), "present".to_string())] + ); + } + + #[test] + fn redacts_invalid_line_from_processing_error() { + const SECRET: &str = "p@ss'w0rd"; + + let temp_dir = TempDir::new().unwrap(); + let env_file = temp_dir.path().join("invalid.env"); + fs::write(&env_file, format!("DB_PASS={SECRET}\n")).unwrap(); + let flags = EnvironmentFlags { + env_files: vec![env_file.clone()], + env: Vec::new(), + }; + + let error = flags + .resolve_with_lookup(|_| None) + .expect_err("invalid dotenv syntax must fail"); + + let message = format!("{error:#}"); + assert!(message.contains(&env_file.display().to_string())); + assert!(!message.contains(SECRET)); + assert!(!message.contains("DB_PASS=")); + assert!(message.contains("failed to process environment file")); + } + + #[test] + fn reports_directory_as_read_error() { + let temp_dir = TempDir::new().unwrap(); + let env_dir = temp_dir.path().join("env-dir"); + fs::create_dir(&env_dir).unwrap(); + let flags = EnvironmentFlags { + env_files: vec![env_dir.clone()], + env: Vec::new(), + }; + + let error = flags + .resolve_with_lookup(|_| None) + .expect_err("directory must fail as an environment file"); + + let message = format!("{error:#}"); + assert!(message.contains(&env_dir.display().to_string())); + assert!(message.contains("failed to read environment file")); + assert!(!message.contains("failed to parse environment file")); + } + + #[test] + fn reports_missing_file_path() { + let temp_dir = TempDir::new().unwrap(); + let missing_file = temp_dir.path().join("missing.env"); + let flags = EnvironmentFlags { + env_files: vec![missing_file.clone()], + env: Vec::new(), + }; + + let error = flags + .resolve_with_lookup(|_| None) + .expect_err("missing environment file must fail"); + + assert!( + error + .to_string() + .contains(&missing_file.display().to_string()) + ); + } +} diff --git a/src/cli/src/main.rs b/src/cli/src/main.rs index 24701bad7..95fbe8fe1 100644 --- a/src/cli/src/main.rs +++ b/src/cli/src/main.rs @@ -3,6 +3,7 @@ mod commands; mod config; mod credentials; mod defaults; +mod environment; mod formatter; pub mod terminal; pub mod util; diff --git a/src/cli/tests/common/mod.rs b/src/cli/tests/common/mod.rs index e18cb0c0b..820d18acd 100644 --- a/src/cli/tests/common/mod.rs +++ b/src/cli/tests/common/mod.rs @@ -4,6 +4,7 @@ use assert_cmd::Command; use boxlite_test_utils::TEST_REGISTRIES; use boxlite_test_utils::home::PerTestBoxHome; use std::fs; +use std::io::Write; use std::path::PathBuf; use std::time::Duration; @@ -110,6 +111,13 @@ pub fn boxlite() -> TestContext { } } +pub fn temp_env_file(contents: &str) -> tempfile::NamedTempFile { + let mut file = tempfile::NamedTempFile::new().expect("create temporary environment file"); + file.write_all(contents.as_bytes()) + .expect("write temporary environment file"); + file +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/cli/tests/create.rs b/src/cli/tests/create.rs index c7ef9346d..6fa37f05c 100644 --- a/src/cli/tests/create.rs +++ b/src/cli/tests/create.rs @@ -62,6 +62,38 @@ fn test_create_resources() { ctx.cleanup_box(name); } +#[test] +fn test_create_env_file_persists_environment() { + let env_file = common::temp_env_file("CREATED_FROM=file\nOVERRIDE=file\n"); + + let mut ctx = common::boxlite(); + let name = "create-env-file"; + ctx.cmd + .arg("create") + .arg("--name") + .arg(name) + .arg("--env-file") + .arg(env_file.path()) + .args(["-e", "OVERRIDE=cli", "alpine:latest"]) + .assert() + .success(); + + ctx.new_cmd() + .args([ + "exec", + name, + "--", + "sh", + "-c", + "printf '%s|%s' \"$CREATED_FROM\" \"$OVERRIDE\"", + ]) + .assert() + .success() + .stdout("file|cli"); + + ctx.cleanup_box(name); +} + // ============================================================================ // Publish (-p / --publish) Tests // ============================================================================ diff --git a/src/cli/tests/exec.rs b/src/cli/tests/exec.rs index b2eb10767..40859fd31 100644 --- a/src/cli/tests/exec.rs +++ b/src/cli/tests/exec.rs @@ -133,6 +133,35 @@ fn test_exec_env_override() { cleanup(&ctx, &box_id); } +#[test] +fn test_exec_env_file_applies_to_command() { + let env_file = common::temp_env_file("EXEC_FROM=file\nOVERRIDE=file\n"); + + let mut ctx = common::boxlite(); + ctx.cmd.args(["run", "-d", "alpine:latest", "sleep", "300"]); + let output = ctx.cmd.assert().success().get_output().clone(); + let box_id = String::from_utf8_lossy(&output.stdout).trim().to_string(); + + ctx.new_cmd() + .arg("exec") + .arg("--env-file") + .arg(env_file.path()) + .args([ + "-e", + "OVERRIDE=cli", + &box_id, + "--", + "sh", + "-c", + "printf '%s|%s' \"$EXEC_FROM\" \"$OVERRIDE\"", + ]) + .assert() + .success() + .stdout("file|cli"); + + cleanup(&ctx, &box_id); +} + #[test] fn test_exec_inherits_box_workdir() { let mut ctx = common::boxlite(); diff --git a/src/cli/tests/run.rs b/src/cli/tests/run.rs index 955b79955..ce037311c 100644 --- a/src/cli/tests/run.rs +++ b/src/cli/tests/run.rs @@ -236,6 +236,34 @@ fn test_run_env_var_empty_value() { ctx.cmd.assert().success().stdout("xx\n"); } +#[test] +fn test_run_env_files_follow_precedence_and_host_substitution() { + let base_env = common::temp_env_file( + "# Base configuration\nBOXLITE_ENV_FILE_FIRST=one\nSHARED=from-base\nHOST_ONLY=${HOST_ONLY}\n", + ); + let override_env = common::temp_env_file("\nSHARED=from-override\nEMPTY=\n"); + + let mut ctx = common::boxlite(); + ctx.cmd.env("HOST_ONLY", "from-host"); + ctx.cmd + .arg("run") + .arg("--rm") + .arg("--env-file") + .arg(base_env.path()) + .arg("--env-file") + .arg(override_env.path()) + .args([ + "-e", + "SHARED=from-cli", + "alpine:latest", + "sh", + "-c", + "printf '%s|%s|%s|%s' \"$BOXLITE_ENV_FILE_FIRST\" \"$SHARED\" \"$HOST_ONLY\" \"$EMPTY\"", + ]); + + ctx.cmd.assert().success().stdout("one|from-cli|from-host|"); +} + // ============================================================================ // Working Directory Tests // ============================================================================ @@ -873,7 +901,7 @@ fn test_run_invalid_image() { } #[test] -fn test_run_tty_error_in_pipe() { +fn test_run_foreground_tty_error_in_pipe() { let mut ctx = common::boxlite(); ctx.cmd.args(["run", "--tty", "alpine:latest"]); // Simulate non-TTY input by writing to stdin @@ -883,3 +911,22 @@ fn test_run_tty_error_in_pipe() { .failure() .stderr(predicate::str::contains("input device is not a TTY")); } + +#[test] +fn test_run_detach_with_tty_allowed() { + let mut ctx = common::boxlite(); + let name = "run-detach-tty"; + ctx.cmd.args([ + "run", + "--detach", + "--tty", + "--name", + name, + "alpine:latest", + "sleep", + "300", + ]); + ctx.cmd.assert().success(); + + ctx.cleanup_box(name); +}