From 29f7ff64de3af9c6876929f1f4961084a5cc4cde Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Fri, 27 Mar 2026 22:08:19 +0000 Subject: [PATCH] fix(interpreter): expand assoc array keys with command subs, fix set -e in compound commands Fix #872: Add async expand_assoc_key() that uses full word expansion (parse_word_string + expand_word) for associative array keys containing $() or backtick substitutions, replacing the sync-only expand_variable_or_literal() in process_command_assignments(). Fix #873: Check result.errexit_suppressed in execute_script_body() so compound commands (for/while/until) whose body ends with an AND-OR chain failure do not incorrectly trigger set -e. --- crates/bashkit/src/interpreter/mod.rs | 22 ++++++-- crates/bashkit/tests/issue_872_test.rs | 66 ++++++++++++++++++++++++ crates/bashkit/tests/issue_873_test.rs | 70 ++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 3 deletions(-) create mode 100644 crates/bashkit/tests/issue_872_test.rs create mode 100644 crates/bashkit/tests/issue_873_test.rs diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index aca72bdb..bc2d9f38 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -1142,7 +1142,8 @@ impl Interpreter { // Run ERR trap on non-zero exit (unless in conditional chain) if exit_code != 0 { let suppressed = matches!(command, Command::List(_)) - || matches!(command, Command::Pipeline(p) if p.negated); + || matches!(command, Command::Pipeline(p) if p.negated) + || result.errexit_suppressed; if !suppressed { self.run_err_trap(&mut stdout, &mut stderr).await; } @@ -1151,9 +1152,12 @@ impl Interpreter { // errexit (set -e): stop on non-zero exit for top-level simple commands. // List commands handle errexit internally (with && / || chain awareness). // Negated pipelines (! cmd) explicitly handle the exit code. + // Compound commands (for/while/until) propagate errexit_suppressed when + // their body ends with an AND-OR chain failure. if self.is_errexit_enabled() && exit_code != 0 { let suppressed = matches!(command, Command::List(_)) - || matches!(command, Command::Pipeline(p) if p.negated); + || matches!(command, Command::Pipeline(p) if p.negated) + || result.errexit_suppressed; if !suppressed { break; } @@ -3129,7 +3133,7 @@ impl Interpreter { if let Some(index_str) = &assignment.index { let resolved_name = self.resolve_nameref(&assignment.name).to_string(); if self.assoc_arrays.contains_key(&resolved_name) { - let key = self.expand_variable_or_literal(index_str); + let key = self.expand_assoc_key(index_str).await?; let is_new_entry = self .assoc_arrays .get(&resolved_name) @@ -7791,6 +7795,18 @@ impl Interpreter { s.to_string() } + /// Expand an associative array key with full word expansion. + /// Unlike `expand_variable_or_literal`, this handles command substitutions + /// (`$(...)`, backticks) and all other expansion types. (Issue #872) + async fn expand_assoc_key(&mut self, s: &str) -> Result { + if s.contains('$') || s.contains('`') { + let word = crate::parser::Parser::parse_word_string(s); + self.expand_word(&word).await + } else { + Ok(s.to_string()) + } + } + /// THREAT[TM-INJ-009]: Check if a variable name is an internal marker. fn is_internal_variable(name: &str) -> bool { is_internal_variable(name) diff --git a/crates/bashkit/tests/issue_872_test.rs b/crates/bashkit/tests/issue_872_test.rs new file mode 100644 index 00000000..76a55203 --- /dev/null +++ b/crates/bashkit/tests/issue_872_test.rs @@ -0,0 +1,66 @@ +//! Test for issue #872: Associative array keys with command substitutions +//! expand to empty string. + +use bashkit::Bash; + +#[tokio::test] +async fn assoc_key_command_substitution() { + let mut bash = Bash::new(); + let result = bash + .exec( + r#" +declare -A m=() +m["$(echo hello)"]="world" +echo "count: ${#m[@]}" +for k in "${!m[@]}"; do echo "key=[$k] val=[${m[$k]}]"; done +"#, + ) + .await + .unwrap(); + assert!( + result.stdout.contains("key=[hello] val=[world]"), + "expected key=[hello], got: {}", + result.stdout + ); +} + +#[tokio::test] +async fn assoc_key_variable_expansion() { + let mut bash = Bash::new(); + let result = bash + .exec( + r#" +declare -A m=() +key="mykey" +m[$key]="myval" +echo "${m[mykey]}" +"#, + ) + .await + .unwrap(); + assert!( + result.stdout.contains("myval"), + "expected myval, got: {}", + result.stdout + ); +} + +#[tokio::test] +async fn assoc_key_literal_unchanged() { + let mut bash = Bash::new(); + let result = bash + .exec( + r#" +declare -A m=() +m[literal]="val" +echo "${m[literal]}" +"#, + ) + .await + .unwrap(); + assert!( + result.stdout.contains("val"), + "expected val, got: {}", + result.stdout + ); +} diff --git a/crates/bashkit/tests/issue_873_test.rs b/crates/bashkit/tests/issue_873_test.rs new file mode 100644 index 00000000..48579b6e --- /dev/null +++ b/crates/bashkit/tests/issue_873_test.rs @@ -0,0 +1,70 @@ +//! Test for issue #873: set -e incorrectly triggers on compound commands +//! whose body ends with && chain failure. + +use bashkit::Bash; + +#[tokio::test] +async fn set_e_for_loop_and_chain_no_exit() { + let mut bash = Bash::new(); + let result = bash + .exec( + r#" +set -euo pipefail +result="" +for src in yes no; do + [[ "${src}" == "yes" ]] && result="${src}" +done +echo "result: ${result}" +"#, + ) + .await + .unwrap(); + assert!( + result.stdout.contains("result: yes"), + "expected 'result: yes', got: {}", + result.stdout + ); +} + +#[tokio::test] +async fn set_e_while_loop_and_chain_no_exit() { + let mut bash = Bash::new(); + let result = bash + .exec( + r#" +set -e +i=0 +while [[ $i -lt 3 ]]; do + [[ $i -eq 1 ]] && echo "found one" + ((i++)) || true +done +echo "done" +"#, + ) + .await + .unwrap(); + assert!( + result.stdout.contains("found one"), + "stdout: {}", + result.stdout + ); + assert!(result.stdout.contains("done"), "stdout: {}", result.stdout); +} + +#[tokio::test] +async fn set_e_plain_failure_in_loop_still_exits() { + let mut bash = Bash::new(); + let result = bash + .exec( + r#" +set -e +for x in a b; do + false +done +echo "SHOULD NOT APPEAR" +"#, + ) + .await + .unwrap(); + assert!(!result.stdout.contains("SHOULD NOT APPEAR")); +}