From dcbb38c247b3c0c5c5744103a42e78703989a51f Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Wed, 21 Oct 2026 09:45:56 -0300 Subject: [PATCH] nvx-rs: lay out execution-helper configurations with one function The managed agent hands a workload's working directory and environment to its execution helper in a sealed configuration, which exec_config::encode lays out: the little-endian flags, entry count, and working-directory length, then the working directory, then each entry after its little-endian length. The tests of exec_config.rs and helper.rs spelled that layout out again, byte by byte, for six configurations that encode never produces, with unknown flags, malformed entries, a relative working directory, and conflicting working-directory flags, and both test modules defined the same file_with helper. encode now computes the flags and leaves the layout to encode_fields, which takes the flags, the working directory, empty without one, and the entries. The six fixtures call encode_fields with the fields that they vary: the four configurations that exec_config's rejection test reads, and the relative and conflicting working directories of helper's. file_with moves beside read, under cfg(test), and serves both test modules. The two files lose 25 lines. Behavior is unchanged: encode_fields writes the bytes that encode wrote, in the same order, and checks the entry count, the working-directory length, and each entry's length for EOVERFLOW in the same order; a configuration without a working directory has length 0 and no working-directory bytes, as before. Each fixture encodes the bytes of its hand-written version, whose count and length were those of the entries and working directory that followed them. Only the managed agent changes, and only in this file's private layout code. The managed agent's unit tests cover the change: exec_config's round-trip and sealing tests, its rejection test, and the helper tests that read these configurations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/guest/managed_agent/exec_config.rs | 83 ++++++++----------- nvx-rs/src/guest/managed_agent/helper.rs | 32 ++----- 2 files changed, 45 insertions(+), 70 deletions(-) diff --git a/nvx-rs/src/guest/managed_agent/exec_config.rs b/nvx-rs/src/guest/managed_agent/exec_config.rs index 8472b3ec2..f9500b826 100644 --- a/nvx-rs/src/guest/managed_agent/exec_config.rs +++ b/nvx-rs/src/guest/managed_agent/exec_config.rs @@ -50,16 +50,29 @@ fn encode(execution: &GuestExecPayload, cwd_entered: bool) -> Result, Er if execution.inherit_default_env { flags |= EXEC_INHERIT_DEFAULT_ENV; } - let environment = execution.environment.as_deref().unwrap_or_default(); + encode_fields( + flags, + cwd.unwrap_or_default(), + execution.environment.as_deref().unwrap_or_default(), + ) +} + +/// Lays out the private helper configuration format with `flags`, the working directory `cwd`, +/// empty without one, and the `environment` entries: the little-endian flags, entry count, and +/// working-directory length, then the working directory, then each entry after its +/// little-endian length. Tests also lay out configurations that [`encode`] never produces. +pub(super) fn encode_fields( + flags: u16, + cwd: &[u8], + environment: &[Vec], +) -> Result, Errno> { let count = u16::try_from(environment.len()).map_err(|_| Errno::EOVERFLOW)?; - let cwd_len = u32::try_from(cwd.map_or(0, <[u8]>::len)).map_err(|_| Errno::EOVERFLOW)?; + let cwd_len = u32::try_from(cwd.len()).map_err(|_| Errno::EOVERFLOW)?; let mut bytes = Vec::new(); bytes.extend_from_slice(&flags.to_le_bytes()); bytes.extend_from_slice(&count.to_le_bytes()); bytes.extend_from_slice(&cwd_len.to_le_bytes()); - if let Some(cwd) = cwd { - bytes.extend_from_slice(cwd); - } + bytes.extend_from_slice(cwd); for entry in environment { let length = u32::try_from(entry.len()).map_err(|_| Errno::EOVERFLOW)?; bytes.extend_from_slice(&length.to_le_bytes()); @@ -119,10 +132,20 @@ pub(crate) fn read(descriptor: &FileDescriptor) -> Result { }) } +/// Returns a descriptor of a file that holds `bytes`, from which [`read`] reads a configuration +/// as it reads a sealed one. #[cfg(test)] -mod tests { +pub(super) fn file_with(bytes: &[u8]) -> FileDescriptor { use std::io::{Seek, Write}; + let mut file = tempfile::tempfile().unwrap(); + file.write_all(bytes).unwrap(); + file.rewind().unwrap(); + FileDescriptor::from_owned(file.into()) +} + +#[cfg(test)] +mod tests { use super::*; fn execution() -> GuestExecPayload { @@ -135,13 +158,6 @@ mod tests { } } - fn file_with(bytes: &[u8]) -> FileDescriptor { - let mut file = tempfile::tempfile().unwrap(); - file.write_all(bytes).unwrap(); - file.rewind().unwrap(); - FileDescriptor::from_owned(file.into()) - } - #[test] fn helper_configuration_round_trips_empty_exact_and_large_environments() { for environment in [ @@ -178,40 +194,13 @@ mod tests { #[test] fn helper_defensively_rejects_unknown_flags_and_malformed_environment() { for bytes in [ - { - let mut bytes = Vec::new(); - bytes.extend_from_slice(&0x4000u16.to_le_bytes()); - bytes.extend_from_slice(&0u16.to_le_bytes()); - bytes.extend_from_slice(&0u32.to_le_bytes()); - bytes - }, - { - let mut bytes = Vec::new(); - bytes.extend_from_slice(&EXEC_ENVIRONMENT_PRESENT.to_le_bytes()); - bytes.extend_from_slice(&1u16.to_le_bytes()); - bytes.extend_from_slice(&0u32.to_le_bytes()); - bytes.extend_from_slice(&9u32.to_le_bytes()); - bytes.extend_from_slice(b"NO_EQUALS"); - bytes - }, - { - let mut bytes = Vec::new(); - bytes.extend_from_slice(&EXEC_ENVIRONMENT_PRESENT.to_le_bytes()); - bytes.extend_from_slice(&1u16.to_le_bytes()); - bytes.extend_from_slice(&0u32.to_le_bytes()); - bytes.extend_from_slice(&6u32.to_le_bytes()); - bytes.extend_from_slice(b"=value"); - bytes - }, - { - let mut bytes = Vec::new(); - bytes.extend_from_slice(&EXEC_CWD_ENTERED.to_le_bytes()); - bytes.extend_from_slice(&0u16.to_le_bytes()); - bytes.extend_from_slice(&1u32.to_le_bytes()); - bytes.push(b'/'); - bytes - }, - ] { + encode_fields(0x4000, b"", &[]), + encode_fields(EXEC_ENVIRONMENT_PRESENT, b"", &[b"NO_EQUALS".to_vec()]), + encode_fields(EXEC_ENVIRONMENT_PRESENT, b"", &[b"=value".to_vec()]), + encode_fields(EXEC_CWD_ENTERED, b"/", &[]), + ] + .map(Result::unwrap) + { let descriptor = file_with(&bytes); assert!(read(&descriptor).is_err(), "{bytes:?}"); } diff --git a/nvx-rs/src/guest/managed_agent/helper.rs b/nvx-rs/src/guest/managed_agent/helper.rs index f066ab261..75e2d8b26 100644 --- a/nvx-rs/src/guest/managed_agent/helper.rs +++ b/nvx-rs/src/guest/managed_agent/helper.rs @@ -133,8 +133,6 @@ pub(crate) fn safe_path(path: &[u8]) -> bool { #[cfg(test)] mod tests { - use std::io::{Seek, Write}; - use crate::guest::sys::FileDescriptor; use crate::protocol::{self, GuestExecPayload}; @@ -166,13 +164,6 @@ mod tests { arguments } - fn file_with(bytes: &[u8]) -> FileDescriptor { - let mut file = tempfile::tempfile().unwrap(); - file.write_all(bytes).unwrap(); - file.rewind().unwrap(); - FileDescriptor::from_owned(file.into()) - } - /// Returns the inherited environment without the helper's descriptor variable. fn inherited() -> Vec> { let mut environment = environment::inherited().unwrap(); @@ -224,26 +215,21 @@ mod tests { ); assert!(system.execs.is_empty()); - let mut relative = Vec::new(); - relative.extend_from_slice(&protocol::EXEC_CWD_PRESENT.to_le_bytes()); - relative.extend_from_slice(&0u16.to_le_bytes()); - relative.extend_from_slice(&1u32.to_le_bytes()); - relative.push(b'.'); - let fd = file_with(&relative); + let relative = exec_config::encode_fields(protocol::EXEC_CWD_PRESENT, b".", &[]).unwrap(); + let fd = exec_config::file_with(&relative); let helper_arguments = arguments(&mut system, fd, &[b"/bin/pwd"]); assert_eq!( run(&mut system, &helper_arguments), Err(CommandError::Agent) ); - let mut conflicting = Vec::new(); - conflicting.extend_from_slice( - &(protocol::EXEC_CWD_PRESENT | exec_config::EXEC_CWD_ENTERED).to_le_bytes(), - ); - conflicting.extend_from_slice(&0u16.to_le_bytes()); - conflicting.extend_from_slice(&1u32.to_le_bytes()); - conflicting.push(b'/'); - let fd = file_with(&conflicting); + let conflicting = exec_config::encode_fields( + protocol::EXEC_CWD_PRESENT | exec_config::EXEC_CWD_ENTERED, + b"/", + &[], + ) + .unwrap(); + let fd = exec_config::file_with(&conflicting); let helper_arguments = arguments(&mut system, fd, &[b"/bin/pwd"]); assert_eq!( run(&mut system, &helper_arguments),