apply review comments

Author: rizsotto

bear/src/args.rs

@@ -112,12 +112,18 @@ impl TryFrom<&ArgMatches> for BuildCommand {
     type Error = ParseError;
 
     fn try_from(matches: &ArgMatches) -> Result<Self, Self::Error> {
-        let arguments = matches
+        let arguments: Vec<_> = matches
             .get_many("COMMAND")
             .ok_or(ParseError::MissingBuildCommand)?
             .cloned()
             .collect();
-        Ok(BuildCommand { arguments })
+
+        // TODO: write test to validate we need this check.
+        if arguments.is_empty() {
+            Err(ParseError::MissingBuildCommand)
+        } else {
+            Ok(BuildCommand { arguments })
+        }
     }
 }
 

bear/src/intercept/environment.rs

@@ -212,8 +212,15 @@ impl BuildEnvironment {
     ///
     /// Panics if the build command has no arguments (empty arguments vector).
     fn as_command(&self, val: BuildCommand) -> std::process::Command {
-        let mut command = std::process::Command::new(val.arguments.first().unwrap());
-        command.args(val.arguments.iter().skip(1));
+        let mut command = match val.arguments.as_slice() {
+            [] => panic!("BuildCommand arguments cannot be empty"),
+            [executable] => std::process::Command::new(executable),
+            [executable, arguments @ ..] => {
+                let mut cmd = std::process::Command::new(executable);
+                cmd.args(arguments);
+                cmd
+            }
+        };
         command.envs(self.environment.clone());
         command
     }

bear/src/intercept/mod.rs

@@ -26,6 +26,13 @@ use thiserror::Error;
 /// It does not contain information about the outcome of the execution,
 /// like the exit code or the duration of the execution. It only contains
 /// the information that is necessary to reproduce the execution.
+///
+/// # Fields
+/// - `executable`: The path to the executable that was run.
+/// - `arguments`: The command line arguments that were passed to the executable.
+///   Includes the executable itself as the first argument.
+/// - `working_dir`: The current working directory of the process.
+/// - `environment`: The environment variables that were set for the process.
 #[derive(Serialize, Deserialize, Debug, PartialEq, Clone)]
 pub struct Execution {
     pub executable: PathBuf,
@@ -39,6 +46,10 @@ impl Execution {
     ///
     /// This method retrieves the executable path, command-line arguments,
     /// current working directory, and environment variables of the process.
+    ///
+    /// **Security Note**: This method captures ALL environment variables from
+    /// the current process, which may include sensitive information. Consider
+    /// using the `trim()` method to filter to only relevant environment variables.
     pub fn capture() -> Result<Self, CaptureError> {
         let executable = std::env::current_exe().map_err(CaptureError::CurrentExecutable)?;
         let arguments = std::env::args().collect();

bear/src/intercept/reporter.rs

@@ -60,20 +60,26 @@ impl ReporterFactory {
     /// Creates a new reporter and returns it as an atomic pointer.
     ///
     /// This is useful for static/global usage where a stable pointer is required
-    /// for the program's lifetime. Caller is responsible for managing the memory.
+    /// for the program's lifetime.
     ///
-    /// Logs errors if reporter creation fails and returns a null pointer in that case.
-    /// Caller is responsible to check for null pointer before using it.
+    /// # Safety
+    ///
+    /// The caller is responsible for ensuring the returned pointer is not used after
+    /// the program terminates. The memory will be leaked intentionally to provide a
+    /// stable pointer for the lifetime of the program.
+    ///
+    /// Returns a null pointer if reporter creation fails. Caller must check for null
+    /// before dereferencing.
     pub fn create_as_ptr() -> AtomicPtr<tcp::ReporterOnTcp> {
         match Self::create() {
             Ok(reporter) => {
                 log::debug!("Reporter created successfully");
 
                 // Leak the reporter to get a stable pointer for the lifetime of the program
                 let boxed_reporter = Box::new(reporter);
-                let prt = Box::into_raw(boxed_reporter);
+                let ptr = Box::into_raw(boxed_reporter);
 
-                AtomicPtr::new(prt)
+                AtomicPtr::new(ptr)
             }
             Err(err) => {
                 log::error!("Failed to create reporter: {err}");

bear/src/intercept/supervise.rs

@@ -54,8 +54,16 @@ pub fn supervise_execution(execution: Execution) -> Result<ExitStatus, Supervise
 
 impl From<Execution> for std::process::Command {
     fn from(val: Execution) -> Self {
-        let mut command = std::process::Command::new(val.executable);
-        command.args(val.arguments.iter().skip(1));
+        let mut command = match val.arguments.as_slice() {
+            [] => panic!("Execution arguments cannot be empty"),
+            [_] => std::process::Command::new(val.executable),
+            [_, arguments @ ..] => {
+                let mut cmd = std::process::Command::new(val.executable);
+                cmd.args(arguments);
+                cmd
+            }
+        };
+
         command.envs(val.environment);
         command.current_dir(val.working_dir);
         command

bear/src/intercept/tcp.rs

@@ -11,13 +11,13 @@ use std::sync::Arc;
 
 /// The serializer for events to transmit over the network.
 ///
-/// The events are serialized using TLV (Type-Length-Value) format.
-/// The type is always 0, the length is a 4-byte big-endian integer,
-/// and the value is the JSON representation of the event.
+/// The events are serialized using LV (Length-Value) format.
+/// The length is a 4-byte big-endian integer, and the value is the JSON
+/// representation of the event.
 struct EventWireSerializer;
 
 impl EventWireSerializer {
-    /// Read an event from a reader using TLV format.
+    /// Read an event from a reader using LV format.
     fn read(reader: &mut impl Read) -> Result<Event, ReporterError> {
         let mut length_bytes = [0; 4];
         reader.read_exact(&mut length_bytes)?;
@@ -30,7 +30,7 @@ impl EventWireSerializer {
         Ok(event)
     }
 
-    /// Write an event to a writer using TLV format.
+    /// Write an event to a writer using LV format.
     fn write(writer: &mut impl Write, event: Event) -> Result<u32, ReporterError> {
         let serialized_event = serde_json::to_string(&event)?;
         let bytes = serialized_event.into_bytes();

bear/src/semantic/interpreters/combinators.rs

@@ -87,8 +87,8 @@ mod test {
 
     fn execution_fixture() -> Execution {
         Execution {
-            executable: PathBuf::new(),
-            arguments: vec![],
+            executable: PathBuf::from("/usr/bin/ls"),
+            arguments: vec!["ls".to_string()],
             working_dir: PathBuf::new(),
             environment: HashMap::new(),
         }