fix: propagate execution parameters more thoroughly (#374)

Address cases where command substitution environments don't correctly inherit the open files of the containing command.

brush-core/benches/shell.rs

@@ -26,7 +26,8 @@ mod unix {
     }
 
     async fn expand_string(shell: &mut brush_core::Shell, s: &str) {
-        let _ = shell.basic_expand_string(s).await.unwrap();
+        let params = shell.default_exec_params();
+        let _ = shell.basic_expand_string(&params, s).await.unwrap();
     }
 
     fn eval_arithmetic_expr(shell: &mut brush_core::Shell, expr: &str) {

brush-core/src/arithmetic.rs

@@ -1,6 +1,6 @@
 use std::borrow::Cow;
 
-use crate::{env, expansion, variables, Shell};
+use crate::{env, expansion, variables, ExecutionParameters, Shell};
 use brush_parser::ast;
 
 /// Represents an error that occurs during evaluation of an arithmetic expression.
@@ -47,12 +47,22 @@ pub trait ExpandAndEvaluate {
     ///
     /// * `shell` - The shell to use for evaluation.
     /// * `trace_if_needed` - Whether to trace the evaluation.
-    async fn eval(&self, shell: &mut Shell, trace_if_needed: bool) -> Result<i64, EvalError>;
+    async fn eval(
+        &self,
+        shell: &mut Shell,
+        params: &ExecutionParameters,
+        trace_if_needed: bool,
+    ) -> Result<i64, EvalError>;
 }
 
 impl ExpandAndEvaluate for ast::UnexpandedArithmeticExpr {
-    async fn eval(&self, shell: &mut Shell, trace_if_needed: bool) -> Result<i64, EvalError> {
-        expand_and_eval(shell, self.value.as_str(), trace_if_needed).await
+    async fn eval(
+        &self,
+        shell: &mut Shell,
+        params: &ExecutionParameters,
+        trace_if_needed: bool,
+    ) -> Result<i64, EvalError> {
+        expand_and_eval(shell, params, self.value.as_str(), trace_if_needed).await
     }
 }
 
@@ -65,11 +75,12 @@ impl ExpandAndEvaluate for ast::UnexpandedArithmeticExpr {
 /// * `trace_if_needed` - Whether to trace the evaluation.
 pub(crate) async fn expand_and_eval(
     shell: &mut Shell,
+    params: &ExecutionParameters,
     expr: &str,
     trace_if_needed: bool,
 ) -> Result<i64, EvalError> {
     // Per documentation, first shell-expand it.
-    let expanded_self = expansion::basic_expand_str_without_tilde(shell, expr)
+    let expanded_self = expansion::basic_expand_str_without_tilde(shell, params, expr)
         .await
         .map_err(|_e| EvalError::FailedToExpandExpression(expr.to_owned()))?;
 

brush-core/src/builtins/printf.rs

@@ -24,7 +24,13 @@ impl builtins::Command for PrintfCommand {
         let result = self.evaluate(&context)?;
 
         if let Some(variable_name) = &self.output_variable {
-            expansion::assign_to_named_parameter(context.shell, variable_name, result).await?;
+            expansion::assign_to_named_parameter(
+                context.shell,
+                &context.params,
+                variable_name,
+                result,
+            )
+            .await?;
         } else {
             write!(context.stdout(), "{result}")?;
             context.stdout().flush()?;

brush-core/src/builtins/test.rs

@@ -1,7 +1,7 @@
 use clap::Parser;
 use std::io::Write;
 
-use crate::{builtins, commands, error, tests, Shell};
+use crate::{builtins, commands, error, tests, ExecutionParameters, Shell};
 
 /// Evaluate test expression.
 #[derive(Parser)]
@@ -30,16 +30,20 @@ impl builtins::Command for TestCommand {
             args = &args[0..args.len() - 1];
         }
 
-        if execute_test(context.shell, args)? {
+        if execute_test(context.shell, &context.params, args)? {
             Ok(builtins::ExitCode::Success)
         } else {
             Ok(builtins::ExitCode::Custom(1))
         }
     }
 }
 
-fn execute_test(shell: &mut Shell, args: &[String]) -> Result<bool, error::Error> {
+fn execute_test(
+    shell: &mut Shell,
+    params: &ExecutionParameters,
+    args: &[String],
+) -> Result<bool, error::Error> {
     let test_command =
         brush_parser::test_command::parse(args).map_err(error::Error::TestCommandParseError)?;
-    tests::eval_test_expr(&test_command, shell)
+    tests::eval_test_expr(&test_command, shell, params)
 }

brush-core/src/commands.rs

@@ -521,7 +521,7 @@ pub(crate) async fn invoke_shell_function(
     // Apply any redirects specified at function definition-time.
     if let Some(redirects) = redirects {
         for redirect in &redirects.0 {
-            interp::setup_redirect(&mut context.params.open_files, context.shell, redirect).await?;
+            interp::setup_redirect(context.shell, &mut context.params, redirect).await?;
         }
     }
 
@@ -563,21 +563,23 @@ pub(crate) async fn invoke_shell_function(
 
 pub(crate) async fn invoke_command_in_subshell_and_get_output(
     shell: &mut Shell,
+    params: &ExecutionParameters,
     s: String,
 ) -> Result<String, error::Error> {
     // Instantiate a subshell to run the command in.
     let mut subshell = shell.clone();
 
+    // Get our own set of parameters we can customize and use.
+    let mut params = params.clone();
+    params.process_group_policy = ProcessGroupPolicy::SameProcessGroup;
+
     // Set up pipe so we can read the output.
     let (reader, writer) = sys::pipes::pipe()?;
-    subshell
+    params
         .open_files
         .files
         .insert(1, openfiles::OpenFile::PipeWriter(writer));
 
-    let mut params = subshell.default_exec_params();
-    params.process_group_policy = ProcessGroupPolicy::SameProcessGroup;
-
     // Run the command.
     let result = subshell.run_string(s, &params).await?;
 

brush-core/src/completion.rs

@@ -244,7 +244,9 @@ impl Spec {
         // Generate completions based on any provided actions (and on words).
         let mut candidates = self.generate_action_completions(shell, context).await?;
         if let Some(word_list) = &self.word_list {
-            let words = crate::expansion::full_expand_and_split_str(shell, word_list).await?;
+            let params = shell.default_exec_params();
+            let words =
+                crate::expansion::full_expand_and_split_str(shell, &params, word_list).await?;
             for word in words {
                 if word.starts_with(context.token_to_complete) {
                     candidates.insert(word);
@@ -607,8 +609,10 @@ impl Spec {
         }
 
         // Run the command.
+        let params = shell.default_exec_params();
         let output =
-            commands::invoke_command_in_subshell_and_get_output(&mut shell, command_line).await?;
+            commands::invoke_command_in_subshell_and_get_output(&mut shell, &params, command_line)
+                .await?;
 
         // Split results.
         let mut candidates = IndexSet::new();
@@ -1052,8 +1056,9 @@ async fn get_file_completions(
 ) -> IndexSet<String> {
     // Basic-expand the token-to-be-completed; it won't have been expanded to this point.
     let mut throwaway_shell = shell.clone();
+    let params = throwaway_shell.default_exec_params();
     let expanded_token = throwaway_shell
-        .basic_expand_string(token_to_complete)
+        .basic_expand_string(&params, token_to_complete)
         .await
         .unwrap_or_else(|_err| token_to_complete.to_owned());
 

brush-core/src/expansion.rs

@@ -20,6 +20,7 @@ use crate::trace_categories;
 use crate::variables::ShellValueUnsetType;
 use crate::variables::ShellVariable;
 use crate::variables::{self, ShellValue};
+use crate::ExecutionParameters;
 
 #[derive(Debug)]
 struct Expansion {
@@ -294,17 +295,19 @@ enum ParameterState {
 
 pub(crate) async fn basic_expand_pattern(
     shell: &mut Shell,
+    params: &ExecutionParameters,
     word: &ast::Word,
 ) -> Result<patterns::Pattern, error::Error> {
-    let mut expander = WordExpander::new(shell);
+    let mut expander = WordExpander::new(shell, params);
     expander.basic_expand_pattern(&word.flatten()).await
 }
 
 pub(crate) async fn basic_expand_regex(
     shell: &mut Shell,
+    params: &ExecutionParameters,
     word: &ast::Word,
 ) -> Result<crate::regex::Regex, error::Error> {
-    let mut expander = WordExpander::new(shell);
+    let mut expander = WordExpander::new(shell, params);
 
     // Brace expansion does not appear to be used in regexes.
     expander.force_disable_brace_expansion = true;
@@ -314,63 +317,73 @@ pub(crate) async fn basic_expand_regex(
 
 pub(crate) async fn basic_expand_word(
     shell: &mut Shell,
+    params: &ExecutionParameters,
     word: &ast::Word,
 ) -> Result<String, error::Error> {
-    basic_expand_str(shell, word.flatten().as_str()).await
+    basic_expand_str(shell, params, word.flatten().as_str()).await
 }
 
-pub(crate) async fn basic_expand_str(shell: &mut Shell, s: &str) -> Result<String, error::Error> {
-    let mut expander = WordExpander::new(shell);
+pub(crate) async fn basic_expand_str(
+    shell: &mut Shell,
+    params: &ExecutionParameters,
+    s: &str,
+) -> Result<String, error::Error> {
+    let mut expander = WordExpander::new(shell, params);
     expander.basic_expand_to_str(s).await
 }
 
 pub(crate) async fn basic_expand_str_without_tilde(
     shell: &mut Shell,
+    params: &ExecutionParameters,
     s: &str,
 ) -> Result<String, error::Error> {
-    let mut expander = WordExpander::new(shell);
+    let mut expander = WordExpander::new(shell, params);
     expander.parser_options.tilde_expansion = false;
     expander.basic_expand_to_str(s).await
 }
 
 pub(crate) async fn full_expand_and_split_word(
     shell: &mut Shell,
+    params: &ExecutionParameters,
     word: &ast::Word,
 ) -> Result<Vec<String>, error::Error> {
-    full_expand_and_split_str(shell, word.flatten().as_str()).await
+    full_expand_and_split_str(shell, params, word.flatten().as_str()).await
 }
 
 pub(crate) async fn full_expand_and_split_str(
     shell: &mut Shell,
+    params: &ExecutionParameters,
     s: &str,
 ) -> Result<Vec<String>, error::Error> {
-    let mut expander = WordExpander::new(shell);
+    let mut expander = WordExpander::new(shell, params);
     expander.full_expand_with_splitting(s).await
 }
 
 pub(crate) async fn assign_to_named_parameter(
     shell: &mut Shell,
+    params: &ExecutionParameters,
     name: &str,
     value: String,
 ) -> Result<(), error::Error> {
     let parser_options = shell.parser_options();
-    let mut expander = WordExpander::new(shell);
+    let mut expander = WordExpander::new(shell, params);
     let parameter = brush_parser::word::parse_parameter(name, &parser_options)?;
     expander.assign_to_parameter(&parameter, value).await
 }
 
 struct WordExpander<'a> {
     shell: &'a mut Shell,
+    params: &'a ExecutionParameters,
     parser_options: brush_parser::ParserOptions,
     force_disable_brace_expansion: bool,
 }
 
 impl<'a> WordExpander<'a> {
-    pub fn new(shell: &'a mut Shell) -> Self {
+    pub fn new(shell: &'a mut Shell, params: &'a ExecutionParameters) -> Self {
         let parser_options = shell.parser_options();
-
         Self {
             shell,
+            params,
             parser_options,
             force_disable_brace_expansion: false,
         }
@@ -710,7 +723,8 @@ impl<'a> WordExpander<'a> {
             brush_parser::word::WordPiece::BackquotedCommandSubstitution(s)
             | brush_parser::word::WordPiece::CommandSubstitution(s) => {
                 let output_str =
-                    commands::invoke_command_in_subshell_and_get_output(self.shell, s).await?;
+                    commands::invoke_command_in_subshell_and_get_output(self.shell, self.params, s)
+                        .await?;
 
                 // We trim trailing newlines, per spec.
                 let trimmed = output_str.trim_end_matches('\n');
@@ -923,7 +937,7 @@ impl<'a> WordExpander<'a> {
                     );
                 }
 
-                let expanded_offset = offset.eval(self.shell, false).await?;
+                let expanded_offset = offset.eval(self.shell, self.params, false).await?;
                 let expanded_offset = if expanded_offset < 0 {
                     0
                 } else {
@@ -934,7 +948,7 @@ impl<'a> WordExpander<'a> {
                 let expanded_offset = min(expanded_offset, expanded_parameter_len);
 
                 let end_offset = if let Some(length) = length {
-                    let mut expanded_length = length.eval(self.shell, false).await?;
+                    let mut expanded_length = length.eval(self.shell, self.params, false).await?;
                     if expanded_length < 0 {
                         let param_length: i64 = i64::try_from(expanded_parameter_len)?;
                         expanded_length += param_length;
@@ -1357,7 +1371,7 @@ impl<'a> WordExpander<'a> {
         let index_to_use = if for_set_associative_array {
             self.basic_expand_to_str(index).await?
         } else {
-            arithmetic::expand_and_eval(self.shell, index, false)
+            arithmetic::expand_and_eval(self.shell, self.params, index, false)
                 .await?
                 .to_string()
         };
@@ -1416,7 +1430,7 @@ impl<'a> WordExpander<'a> {
         &mut self,
         expr: brush_parser::ast::UnexpandedArithmeticExpr,
     ) -> Result<String, error::Error> {
-        let value = expr.eval(self.shell, false).await?;
+        let value = expr.eval(self.shell, self.params, false).await?;
         Ok(value.to_string())
     }
 
@@ -1691,33 +1705,34 @@ mod tests {
     async fn test_full_expansion() -> Result<()> {
         let options = crate::shell::CreateOptions::default();
         let mut shell = crate::shell::Shell::new(&options).await?;
+        let params = shell.default_exec_params();
 
         assert_eq!(
-            full_expand_and_split_str(&mut shell, "\"\"").await?,
+            full_expand_and_split_str(&mut shell, &params, "\"\"").await?,
             vec![""]
         );
         assert_eq!(
-            full_expand_and_split_str(&mut shell, "a b").await?,
+            full_expand_and_split_str(&mut shell, &params, "a b").await?,
             vec!["a", "b"]
         );
         assert_eq!(
-            full_expand_and_split_str(&mut shell, "ab").await?,
+            full_expand_and_split_str(&mut shell, &params, "ab").await?,
             vec!["ab"]
         );
         assert_eq!(
-            full_expand_and_split_str(&mut shell, r#""a b""#).await?,
+            full_expand_and_split_str(&mut shell, &params, r#""a b""#).await?,
             vec!["a b"]
         );
         assert_eq!(
-            full_expand_and_split_str(&mut shell, "").await?,
+            full_expand_and_split_str(&mut shell, &params, "").await?,
             Vec::<String>::new()
         );
         assert_eq!(
-            full_expand_and_split_str(&mut shell, "$@").await?,
+            full_expand_and_split_str(&mut shell, &params, "$@").await?,
             Vec::<String>::new()
         );
         assert_eq!(
-            full_expand_and_split_str(&mut shell, "$*").await?,
+            full_expand_and_split_str(&mut shell, &params, "$*").await?,
             Vec::<String>::new()
         );
 
@@ -1728,7 +1743,8 @@ mod tests {
     async fn test_brace_expansion() -> Result<()> {
         let options = crate::shell::CreateOptions::default();
         let mut shell = crate::shell::Shell::new(&options).await?;
-        let expander = WordExpander::new(&mut shell);
+        let params = shell.default_exec_params();
+        let expander = WordExpander::new(&mut shell, &params);
 
         assert_eq!(expander.brace_expand_if_needed("abc")?, ["abc"]);
         assert_eq!(expander.brace_expand_if_needed("a{,b}d")?, ["ad", "abd"]);
@@ -1751,7 +1767,8 @@ mod tests {
     async fn test_field_splitting() -> Result<()> {
         let options = crate::shell::CreateOptions::default();
         let mut shell = crate::shell::Shell::new(&options).await?;
-        let expander = WordExpander::new(&mut shell);
+        let params = shell.default_exec_params();
+        let expander = WordExpander::new(&mut shell, &params);
 
         let expansion = Expansion {
             fields: vec![