diff --git a/crates/bxctl/src/main.rs b/crates/bxctl/src/main.rs index 53f87ac..e5ed98b 100644 --- a/crates/bxctl/src/main.rs +++ b/crates/bxctl/src/main.rs @@ -11,7 +11,19 @@ use bxctl::escape::escape_model_text; use proto::{ErrorCode, SessionId, Timestamp, TurnDone}; fn main() -> ExitCode { - let args: Vec = std::env::args().skip(1).collect(); + // `args_os`, because `args` panics on an argument that is not UTF-8. bxctl's arguments are + // text (ids, messages, paths it prints back), so such an argument is a usage error. + let args: Vec = match std::env::args_os() + .skip(1) + .map(|a| a.into_string()) + .collect() + { + Ok(args) => args, + Err(_) => { + eprintln!("bxctl: an argument is not valid UTF-8\n{USAGE}"); + return ExitCode::from(2); + } + }; // $BOXMAKER_HOME defaults to /var/lib/boxmaker; defaults for the sockets are read from it. let home = std::env::var_os("BOXMAKER_HOME") .map(PathBuf::from) diff --git a/crates/bxctl/tests/args_os.rs b/crates/bxctl/tests/args_os.rs new file mode 100644 index 0000000..cb5e09b --- /dev/null +++ b/crates/bxctl/tests/args_os.rs @@ -0,0 +1,19 @@ +//! An argument that is not UTF-8 is a usage error, not a panic (M3a review finding 7). + +use std::ffi::OsStr; +use std::os::unix::ffi::OsStrExt; +use std::process::Command; + +#[test] +fn an_argument_that_is_not_utf8_is_a_usage_error() { + let out = Command::new(env!("CARGO_BIN_EXE_bxctl")) + .args([OsStr::new("approve"), OsStr::from_bytes(b"4\xff")]) + .output() + .unwrap(); + assert_eq!(out.status.code(), Some(2)); + let err = String::from_utf8_lossy(&out.stderr); + assert!( + err.starts_with("bxctl: an argument is not valid UTF-8"), + "{err}" + ); +} diff --git a/crates/inferproxy/src/main.rs b/crates/inferproxy/src/main.rs index b591430..2b44951 100644 --- a/crates/inferproxy/src/main.rs +++ b/crates/inferproxy/src/main.rs @@ -7,7 +7,11 @@ use std::process; use inferproxy::{Limits, serve}; fn main() { - let args: Vec = env::args().collect(); + // `args_os`, because `args` panics on an argument that is not UTF-8. + let args: Vec = match env::args_os().map(|a| a.into_string()).collect() { + Ok(args) => args, + Err(_) => usage(), + }; let mut listen: Option = None; let mut upstream: Option = None; let mut i = 1; diff --git a/crates/inferproxy/tests/args_os.rs b/crates/inferproxy/tests/args_os.rs new file mode 100644 index 0000000..38bf3c3 --- /dev/null +++ b/crates/inferproxy/tests/args_os.rs @@ -0,0 +1,14 @@ +//! An argument that is not UTF-8 is a usage error, not a panic (M3a review finding 7). + +use std::ffi::OsStr; +use std::os::unix::ffi::OsStrExt; +use std::process::Command; + +#[test] +fn an_argument_that_is_not_utf8_is_a_usage_error() { + let out = Command::new(env!("CARGO_BIN_EXE_inferproxy")) + .args([OsStr::new("--listen"), OsStr::from_bytes(b"/tmp/\xff.sock")]) + .output() + .unwrap(); + assert_eq!(out.status.code(), Some(2)); +} diff --git a/crates/loopd/src/main.rs b/crates/loopd/src/main.rs index bfb367a..f8e5c14 100644 --- a/crates/loopd/src/main.rs +++ b/crates/loopd/src/main.rs @@ -16,11 +16,12 @@ use loopd::selftest::{SelfTestError, run}; use loopd::tools::{Registry, ToolPort}; fn main() -> ExitCode { - let args: Vec = std::env::args().skip(1).collect(); - let args: Vec<&str> = args.iter().map(String::as_str).collect(); - match args.as_slice() { - ["selftest", "--config", path] => run_selftest(path), - ["serve", "--config", path] => run_serve(path), + // `args_os`: the config path need not be UTF-8, and `args` would panic on one that is not. + let args: Vec = std::env::args_os().skip(1).collect(); + let words: Vec> = args.iter().map(|a| a.to_str()).collect(); + match (words.as_slice(), args.get(2)) { + ([Some("selftest"), Some("--config"), _], Some(path)) => run_selftest(Path::new(path)), + ([Some("serve"), Some("--config"), _], Some(path)) => run_serve(Path::new(path)), _ => { eprintln!("usage: loopd selftest --config "); eprintln!("usage: loopd serve --config "); @@ -46,8 +47,8 @@ fn run_selftest_check(client: &Client) -> Result<(), SelfTestError> { } } -fn run_selftest(path: &str) -> ExitCode { - let cfg = match Config::load(Path::new(path)) { +fn run_selftest(path: &Path) -> ExitCode { + let cfg = match Config::load(path) { Ok(cfg) => cfg, Err(e) => { eprintln!("loopd: {e}"); @@ -62,8 +63,8 @@ fn run_selftest(path: &str) -> ExitCode { } } -fn run_serve(path: &str) -> ExitCode { - let cfg = match Config::load(Path::new(path)) { +fn run_serve(path: &Path) -> ExitCode { + let cfg = match Config::load(path) { Ok(cfg) => cfg, Err(e) => { eprintln!("loopd: {e}"); diff --git a/crates/loopd/tests/args_os.rs b/crates/loopd/tests/args_os.rs new file mode 100644 index 0000000..974093c --- /dev/null +++ b/crates/loopd/tests/args_os.rs @@ -0,0 +1,36 @@ +//! A config path that is not UTF-8 is read as a path, not a panic (M3a review finding 7). + +use std::ffi::OsStr; +use std::os::unix::ffi::OsStrExt; +use std::process::Command; + +#[test] +fn a_config_path_that_is_not_utf8_is_read_as_a_path() { + let path = std::env::temp_dir().join(OsStr::from_bytes(b"loopd-missing-\xff.toml")); + let out = Command::new(env!("CARGO_BIN_EXE_loopd")) + .args([ + OsStr::new("selftest"), + OsStr::new("--config"), + path.as_os_str(), + ]) + .output() + .unwrap(); + assert_eq!( + out.status.code(), + Some(1), + "a config error, not a panic (101)" + ); +} + +#[test] +fn a_flag_that_is_not_utf8_is_a_usage_error() { + let out = Command::new(env!("CARGO_BIN_EXE_loopd")) + .args([ + OsStr::from_bytes(b"serve\xff"), + OsStr::new("--config"), + OsStr::new("x"), + ]) + .output() + .unwrap(); + assert_eq!(out.status.code(), Some(2)); +}