From 900028d0e47e7a292d2024ecd5df6dd67c51feb9 Mon Sep 17 00:00:00 2001 From: "L. Elaine Dazzio" Date: Thu, 26 Feb 2026 19:34:37 -0500 Subject: [PATCH 1/3] Replace .unwrap() with proper error handling in run() Replace panic-inducing .unwrap() calls in the `run()` function with proper error handling using `anyhow::Context`, so users get actionable error messages instead of panics when the environment is not set up correctly. Changes: - Add `anyhow::Context` import - Replace `.unwrap()` on `std::path::absolute()` with `.with_context()` - Replace `.unwrap()` on `path.metadata()` with `.with_context()` - Replace `.unwrap()` on `Mode::from_bits()` with `.ok_or_else()` - Update docstring from "Panics" to "Errors" section Fixes #650 --- litebox_runner_linux_userland/src/lib.rs | 28 +++++++++++++----------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/litebox_runner_linux_userland/src/lib.rs b/litebox_runner_linux_userland/src/lib.rs index 6fa68a5c3..084da1a36 100644 --- a/litebox_runner_linux_userland/src/lib.rs +++ b/litebox_runner_linux_userland/src/lib.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -use anyhow::{Result, anyhow}; +use anyhow::{Context, Result, anyhow}; use clap::Parser; use litebox::fs::{FileSystem as _, Mode}; use litebox_platform_multiplex::Platform; @@ -92,11 +92,10 @@ fn mmapped_file_data(path: impl AsRef) -> Result<&'static [u8]> { /// Run Linux programs with LiteBox on unmodified Linux /// -/// # Panics +/// # Errors /// -/// Can panic if any particulars of the environment are not set up as expected. Ideally, would not -/// panic. If it does actually panic, then ping the authors of LiteBox, and likely a better error -/// message could be thrown instead. +/// Returns an error if the program path cannot be resolved, directories along the path are +/// inaccessible, or the initial filesystem cannot be set up. pub fn run(cli_args: CliArgs) -> Result<()> { if !cli_args.insert_files.is_empty() { unimplemented!( @@ -108,20 +107,22 @@ pub fn run(cli_args: CliArgs) -> Result<()> { Vec<(litebox::fs::Mode, u32)>, alloc::borrow::Cow<'static, [u8]>, ) = { - let prog = std::path::absolute(Path::new(&cli_args.program_and_arguments[0])).unwrap(); + let prog = std::path::absolute(Path::new(&cli_args.program_and_arguments[0])) + .with_context(|| format!("Could not resolve absolute path for program '{}'", &cli_args.program_and_arguments[0]))?; let ancestors: Vec<_> = prog.ancestors().collect(); - let modes: Vec<_> = ancestors + let modes: Result> = ancestors .into_iter() .rev() .skip(1) .map(|path| { - let metadata = path.metadata().unwrap(); - ( - litebox::fs::Mode::from_bits(metadata.st_mode()).unwrap(), - metadata.st_uid(), - ) + let metadata = path.metadata() + .with_context(|| format!("Could not read metadata for '{}'. Ensure the path exists and is accessible.", path.display()))?; + let mode = litebox::fs::Mode::from_bits(metadata.st_mode()) + .ok_or_else(|| anyhow!("Invalid file mode bits {:#o} for '{}'", metadata.st_mode(), path.display()))?; + Ok((mode, metadata.st_uid())) }) .collect(); + let modes = modes?; let data = mmapped_file_data(prog)?; let data = if cli_args.rewrite_syscalls { litebox_syscall_rewriter::hook_syscalls_in_elf(data, None) @@ -152,7 +153,8 @@ pub fn run(cli_args: CliArgs) -> Result<()> { let litebox = shim_builder.litebox(); let initial_file_system = { let mut in_mem = litebox::fs::in_mem::FileSystem::new(litebox); - let prog = std::path::absolute(Path::new(&cli_args.program_and_arguments[0])).unwrap(); + let prog = std::path::absolute(Path::new(&cli_args.program_and_arguments[0])) + .with_context(|| format!("Could not resolve absolute path for program '{}'", &cli_args.program_and_arguments[0]))?; let ancestors: Vec<_> = prog.ancestors().collect(); let mut prev_user = 0; for (path, &mode_and_user) in ancestors From f2f38c773db825de3edbe6e26746eeae2ef4f842 Mon Sep 17 00:00:00 2001 From: "L. Elaine Dazzio" Date: Thu, 26 Feb 2026 19:42:07 -0500 Subject: [PATCH 2/3] Address Copilot review feedback - Use `from_bits_truncate` instead of `ok_or_else` for Mode (comment 1) - Add regression test for nonexistent program path (comment 2) - Update docstring to mention remaining panic conditions (comment 3) - Fix third missed `.unwrap()` on `std::path::absolute` (comment 4) --- litebox_runner_linux_userland/src/lib.rs | 11 ++++-- .../tests/error_handling.rs | 39 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 litebox_runner_linux_userland/tests/error_handling.rs diff --git a/litebox_runner_linux_userland/src/lib.rs b/litebox_runner_linux_userland/src/lib.rs index 084da1a36..b7046336a 100644 --- a/litebox_runner_linux_userland/src/lib.rs +++ b/litebox_runner_linux_userland/src/lib.rs @@ -96,6 +96,11 @@ fn mmapped_file_data(path: impl AsRef) -> Result<&'static [u8]> { /// /// Returns an error if the program path cannot be resolved, directories along the path are /// inaccessible, or the initial filesystem cannot be set up. +/// +/// # Panics +/// +/// May still panic if internal filesystem operations (e.g., in-memory mkdir/chown/open) or +/// syscall rewriting fail unexpectedly. pub fn run(cli_args: CliArgs) -> Result<()> { if !cli_args.insert_files.is_empty() { unimplemented!( @@ -117,8 +122,7 @@ pub fn run(cli_args: CliArgs) -> Result<()> { .map(|path| { let metadata = path.metadata() .with_context(|| format!("Could not read metadata for '{}'. Ensure the path exists and is accessible.", path.display()))?; - let mode = litebox::fs::Mode::from_bits(metadata.st_mode()) - .ok_or_else(|| anyhow!("Invalid file mode bits {:#o} for '{}'", metadata.st_mode(), path.display()))?; + let mode = litebox::fs::Mode::from_bits_truncate(metadata.st_mode()); Ok((mode, metadata.st_uid())) }) .collect(); @@ -256,7 +260,8 @@ pub fn run(cli_args: CliArgs) -> Result<()> { }; // We need to get the file path before enabling seccomp - let prog = std::path::absolute(Path::new(&cli_args.program_and_arguments[0])).unwrap(); + let prog = std::path::absolute(Path::new(&cli_args.program_and_arguments[0])) + .with_context(|| format!("Could not resolve absolute path for program '{}'", &cli_args.program_and_arguments[0]))?; let prog_path = prog.to_str().ok_or_else(|| { anyhow!( "Could not convert program path {:?} to a string", diff --git a/litebox_runner_linux_userland/tests/error_handling.rs b/litebox_runner_linux_userland/tests/error_handling.rs new file mode 100644 index 000000000..2d7db69b8 --- /dev/null +++ b/litebox_runner_linux_userland/tests/error_handling.rs @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +//! Regression tests for error handling in the runner (see #650). +//! +//! These tests verify that the runner produces clear error messages instead +//! of panicking when given invalid inputs. + +use std::process::Command; + +/// Get the path to the litebox_runner_linux_userland binary. +fn runner_binary() -> String { + std::env::var("NEXTEST_BIN_EXE_litebox_runner_linux_userland") + .unwrap_or_else(|_| env!("CARGO_BIN_EXE_litebox_runner_linux_userland").to_string()) +} + +/// Regression test for #650: running with a nonexistent program path should +/// produce a clear error message instead of panicking. +#[test] +fn test_nonexistent_program_returns_error_not_panic() { + let output = Command::new(runner_binary()) + .arg("/nonexistent/path/to/program") + .output() + .expect("Failed to execute runner binary"); + + // The process should fail (non-zero exit code) + assert!( + !output.status.success(), + "Expected runner to fail for nonexistent program, but it succeeded" + ); + + let stderr = String::from_utf8_lossy(&output.stderr); + + // It should NOT contain a panic message + assert!( + !stderr.contains("panicked at"), + "Runner panicked instead of returning an error.\nStderr: {stderr}" + ); +} From 7a4ea7a9ac09d4d689967a6baaae21623e300d41 Mon Sep 17 00:00:00 2001 From: "L. Elaine Dazzio" Date: Fri, 27 Feb 2026 20:08:25 -0500 Subject: [PATCH 3/3] Strengthen regression test to verify contextual error message The test now also asserts that stderr contains a contextual error message (e.g., "Could not resolve absolute path" or "Could not read metadata") rather than just checking for the absence of a panic. --- litebox_runner_linux_userland/tests/error_handling.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/litebox_runner_linux_userland/tests/error_handling.rs b/litebox_runner_linux_userland/tests/error_handling.rs index 2d7db69b8..81e9bed1c 100644 --- a/litebox_runner_linux_userland/tests/error_handling.rs +++ b/litebox_runner_linux_userland/tests/error_handling.rs @@ -15,7 +15,7 @@ fn runner_binary() -> String { } /// Regression test for #650: running with a nonexistent program path should -/// produce a clear error message instead of panicking. +/// produce a clear, contextual error message instead of panicking. #[test] fn test_nonexistent_program_returns_error_not_panic() { let output = Command::new(runner_binary()) @@ -36,4 +36,12 @@ fn test_nonexistent_program_returns_error_not_panic() { !stderr.contains("panicked at"), "Runner panicked instead of returning an error.\nStderr: {stderr}" ); + + // It SHOULD contain a contextual error message about the path + assert!( + stderr.contains("Could not resolve absolute path") + || stderr.contains("Could not read metadata"), + "Expected a contextual error message about path resolution or metadata, \ + but got:\nStderr: {stderr}" + ); }