diff --git a/src/rust/engine/process_execution/src/lib.rs b/src/rust/engine/process_execution/src/lib.rs index 49b2918a6cf..19bee39114c 100644 --- a/src/rust/engine/process_execution/src/lib.rs +++ b/src/rust/engine/process_execution/src/lib.rs @@ -1028,7 +1028,7 @@ fn maybe_make_wrapper_script( append_only_caches_base_path: Option<&str>, working_directory: Option<&str>, sandbox_root_token: Option<&str>, - env_vars_to_substitute: &[&str], + env_vars_to_substitute: Vec<&str>, ) -> Result, String> { let caches_fragment = match append_only_caches_base_path { Some(base_path) if !caches.is_empty() => { @@ -1097,7 +1097,8 @@ fn maybe_make_wrapper_script( token ); if !env_vars_to_substitute.is_empty() { - let env_vars_str = env_vars_to_substitute.join(" "); // TODO: shlex the strings + let env_vars_str = shlex::try_join(env_vars_to_substitute) + .map_err(|e| format!("Failed to shell quote environment names: {e}"))?; writeln!(&mut fragment, "for env_var in {env_vars_str}; do") .map_err(|err| format!("write! failed: {err}"))?; writeln!( @@ -1189,7 +1190,7 @@ pub async fn make_execute_request( append_only_caches_base_path, req.working_directory.as_ref().and_then(|p| p.to_str()), sandbox_root_token, - env_var_with_chroot_marker_refs.as_slice(), + env_var_with_chroot_marker_refs, )? }; let wrapper_script_digest_opt = if let Some(script) = wrapper_script_content_opt { diff --git a/src/rust/engine/process_execution/src/tests.rs b/src/rust/engine/process_execution/src/tests.rs index 2c294a5a697..ab82d61bf14 100644 --- a/src/rust/engine/process_execution/src/tests.rs +++ b/src/rust/engine/process_execution/src/tests.rs @@ -182,7 +182,7 @@ async fn wrapper_script_supports_append_only_caches() { dummy_caches_base_path.path().to_str(), Some(SUBDIR_NAME), None, - &[], + vec![], ) .unwrap() .unwrap(); @@ -243,7 +243,7 @@ async fn wrapper_script_supports_append_only_caches() { async fn wrapper_script_supports_sandbox_root_replacements_in_args() { let caches = BTreeMap::new(); - let script_content = maybe_make_wrapper_script(&caches, None, None, Some("__ROOT__"), &[]) + let script_content = maybe_make_wrapper_script(&caches, None, None, Some("__ROOT__"), vec![]) .unwrap() .unwrap(); @@ -287,10 +287,15 @@ async fn wrapper_script_supports_sandbox_root_replacements_in_args() { async fn wrapper_script_supports_sandbox_root_replacements_in_environmenbt() { let caches = BTreeMap::new(); - let script_content = - maybe_make_wrapper_script(&caches, None, None, Some("__ROOT__"), &["TEST_FILE_PATH"]) - .unwrap() - .unwrap(); + let script_content = maybe_make_wrapper_script( + &caches, + None, + None, + Some("__ROOT__"), + vec!["TEST_FILE_PATH"], + ) + .unwrap() + .unwrap(); let dummy_sandbox_path = TempDir::new().unwrap(); let script_path = dummy_sandbox_path.path().join("wrapper");