From 16c038b1783ad235853198eea6092db28a5de0de Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Sun, 9 Aug 2026 13:32:30 +0000 Subject: [PATCH] fix(lint): guard empty array expansions in vale-wrap.sh for bash 3.2 Under set -u, "${arr[@]}" on an empty array aborts on bash before 4.4, which is what macOS ships as /bin/bash. Three expansion sites now use ${arr[@]+"${arr[@]}"} consistently. The hazard is not currently reachable: verified on a bash 3.2.57 built from source that all seven invocation shapes succeed against the previous code, including zero args, flags-only and an empty directory. vale_args is provably non-empty at every site because the default --config branch always appends first. The guard is kept because that invariant is non-local and untested, so an edit to the default-config branch would reintroduce a macOS-only crash silently. Test fidelity is deliberately mixed. Case 16 is static and is the only one that fails against the previous code, since no bash 5 host can reproduce the abort at runtime. Case 17 runs the emptiest invocations under the oldest bash it can find and names that shell in its output so it cannot overclaim. Case 18 guards against the tempting wrong fix of dropping the quotes, which also silences the abort but word-splits a path containing a space. No other bash 4.x construct is present; swept for mapfile, declare -A, case modification, negative indices, globstar, wait -n and namerefs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58 --- plugins/kyberforge/.claude-plugin/plugin.json | 2 +- plugins/kyberforge/plugin.json | 2 +- .../skills/agent-audit/scripts/vale-wrap.sh | 11 ++- .../skills/skill-audit/scripts/vale-wrap.sh | 11 ++- tests/test-vale-wrap.sh | 75 +++++++++++++++++++ 5 files changed, 93 insertions(+), 8 deletions(-) diff --git a/plugins/kyberforge/.claude-plugin/plugin.json b/plugins/kyberforge/.claude-plugin/plugin.json index 2a76c69..2c67fcb 100644 --- a/plugins/kyberforge/.claude-plugin/plugin.json +++ b/plugins/kyberforge/.claude-plugin/plugin.json @@ -8,5 +8,5 @@ "keywords": [], "license": "MIT", "name": "kyberforge", - "version": "1.2.6" + "version": "1.2.7" } diff --git a/plugins/kyberforge/plugin.json b/plugins/kyberforge/plugin.json index 367ddb4..7609ef2 100644 --- a/plugins/kyberforge/plugin.json +++ b/plugins/kyberforge/plugin.json @@ -13,5 +13,5 @@ "skills": [ "skills/" ], - "version": "1.2.6" + "version": "1.2.7" } diff --git a/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh b/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh index 9e22cb8..f3d891b 100755 --- a/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh +++ b/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh @@ -34,6 +34,11 @@ set -euo pipefail cwd="$(pwd -P)" +# Every array below is expanded as `${arr[@]+"${arr[@]}"}`: bash before 4.4 — +# including the 3.2 that macOS still ships as /bin/bash — treats `"${arr[@]}"` +# on an empty array as an unbound variable under `set -u`. No expansion site is +# reachable while empty on today's control flow, so this is insurance against a +# later edit breaking that invariant, not a live fix. vale_args=() path_args=() config_next=false @@ -93,7 +98,7 @@ fi if [[ ${#path_args[@]} -eq 0 ]]; then # Nothing to flatten. Hand off directly, with stdin closed so vale doesn't # block waiting on a pipe that will never carry content. - exec vale "${vale_args[@]}" < /dev/null + exec vale ${vale_args[@]+"${vale_args[@]}"} < /dev/null fi # `realpath -m` would be the obvious normalizer, but `-m` (canonicalize-missing) @@ -178,7 +183,7 @@ mirror="$tmpdir$cwd" mkdir -p "$mirror" argv_paths=() -for arg in "${path_args[@]}"; do +for arg in ${path_args[@]+"${path_args[@]}"}; do if [[ "$arg" == /* ]]; then dest="$tmpdir$arg" else @@ -220,4 +225,4 @@ for arg in "${path_args[@]}"; do done cd "$mirror" -vale "${vale_args[@]}" "${argv_paths[@]}" +vale ${vale_args[@]+"${vale_args[@]}"} ${argv_paths[@]+"${argv_paths[@]}"} diff --git a/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh b/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh index 9e22cb8..f3d891b 100755 --- a/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh +++ b/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh @@ -34,6 +34,11 @@ set -euo pipefail cwd="$(pwd -P)" +# Every array below is expanded as `${arr[@]+"${arr[@]}"}`: bash before 4.4 — +# including the 3.2 that macOS still ships as /bin/bash — treats `"${arr[@]}"` +# on an empty array as an unbound variable under `set -u`. No expansion site is +# reachable while empty on today's control flow, so this is insurance against a +# later edit breaking that invariant, not a live fix. vale_args=() path_args=() config_next=false @@ -93,7 +98,7 @@ fi if [[ ${#path_args[@]} -eq 0 ]]; then # Nothing to flatten. Hand off directly, with stdin closed so vale doesn't # block waiting on a pipe that will never carry content. - exec vale "${vale_args[@]}" < /dev/null + exec vale ${vale_args[@]+"${vale_args[@]}"} < /dev/null fi # `realpath -m` would be the obvious normalizer, but `-m` (canonicalize-missing) @@ -178,7 +183,7 @@ mirror="$tmpdir$cwd" mkdir -p "$mirror" argv_paths=() -for arg in "${path_args[@]}"; do +for arg in ${path_args[@]+"${path_args[@]}"}; do if [[ "$arg" == /* ]]; then dest="$tmpdir$arg" else @@ -220,4 +225,4 @@ for arg in "${path_args[@]}"; do done cd "$mirror" -vale "${vale_args[@]}" "${argv_paths[@]}" +vale ${vale_args[@]+"${vale_args[@]}"} ${argv_paths[@]+"${argv_paths[@]}"} diff --git a/tests/test-vale-wrap.sh b/tests/test-vale-wrap.sh index 9816125..3fc4d0a 100755 --- a/tests/test-vale-wrap.sh +++ b/tests/test-vale-wrap.sh @@ -425,6 +425,81 @@ else fail "a path with a space was dropped from the directory walk" fi +# --- 16. No unguarded `"${arr[@]}"` expansion survives in the wrapper. bash +# before 4.4 — including the 3.2 that macOS still ships as /bin/bash — treats +# that form on an empty array as an unbound variable under `set -u` and aborts. +# The portable form is `${arr[@]+"${arr[@]}"}`. This is a static check because +# no bash 5 host can reproduce the abort at runtime: the construct is only fatal +# on the older shell, so absence of the construct is the property to assert. +# `${#arr[@]}` is deliberately not flagged — the count form is safe on 3.2. +echo "" +echo "--- no unguarded array expansion remains in vale-wrap.sh ---" +unguarded_expansions() { + # Blank out whole-line comments (keeping line numbers), delete every correctly + # guarded expansion, then anything still matching is a real hazard. + awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$1" \ + | sed -E 's/\$\{([A-Za-z_][A-Za-z0-9_]*)\[@\]\+"\$\{\1\[@\]\}"\}//g' \ + | grep -nE '\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}' || true +} +HAZARDS16="$(unguarded_expansions "$SCRIPT")" +if [[ -n "$HAZARDS16" ]]; then + fail "unguarded array expansion(s) abort on bash < 4.4 under set -u: $(echo "$HAZARDS16" | tr '\n' ' ')" +else + pass "every array expansion uses the bash-3.2-safe \${arr[@]+\"\${arr[@]}\"} form" +fi + +# --- 17. The invocations whose arrays are closest to empty actually run. Under +# a bash older than 4.4 this is genuine macOS-shell coverage; on a modern bash it +# degrades to a smoke test, so the pass message names the shell that really ran. +# Point VALE_WRAP_TEST_BASH at a 3.2 build to get the real thing in CI. +echo "" +echo "--- degenerate invocations survive on the oldest available bash ---" +OLD_BASH="bash" +OLD_BASH_VER="$(bash -c 'echo "${BASH_VERSINFO[0]}.${BASH_VERSINFO[1]}"')" +for CAND in "${VALE_WRAP_TEST_BASH:-}" bash-3.2 bash3 /bin/bash /usr/local/bin/bash; do + [[ -n "$CAND" ]] && command -v "$CAND" >/dev/null 2>&1 || continue + CAND_VER="$("$CAND" -c 'echo "${BASH_VERSINFO[0]}.${BASH_VERSINFO[1]}"' 2>/dev/null)" || continue + [[ -n "$CAND_VER" ]] || continue + if (( ${CAND_VER%.*} * 100 + ${CAND_VER#*.} < ${OLD_BASH_VER%.*} * 100 + ${OLD_BASH_VER#*.} )); then + OLD_BASH="$CAND" + OLD_BASH_VER="$CAND_VER" + fi +done +FIXTURE17="$(make_fixture 2)" +mkdir -p "$FIXTURE17/emptydir" +trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12" "$STUB13" "$FIXTURE14" "$FIXTURE17"' EXIT +# Zero args, flags with no path, and a directory that walks to nothing are the +# three shapes that leave vale_args/path_args/argv_paths at their emptiest. +OUT17="" +for ARGS17 in "" "--config $VALE_CONFIG" "--config $VALE_CONFIG emptydir"; do + # shellcheck disable=SC2086 # deliberate word splitting of the argv fixture + OUT17+="$( (cd "$FIXTURE17" && "$OLD_BASH" "$SCRIPT" $ARGS17 &1) || true)" +done +if echo "$OUT17" | grep -q "unbound variable"; then + fail "aborted with 'unbound variable' on bash $OLD_BASH_VER — the bug this test guards against" +else + pass "degenerate invocations run clean under bash $OLD_BASH_VER ($OLD_BASH)" +fi + +# --- 18. The guarded expansion must keep argv word boundaries intact. Dropping +# the quotes (`${arr[@]}`) also silences the unbound-variable abort, so it is the +# tempting wrong fix — and it splits any path containing a space into two bogus +# arguments. Case 15 covers spaces found by the directory walk; this covers a +# space in the path argument itself, which is what argv_paths expands. +echo "" +echo "--- a path argument containing a space survives the guarded expansion ---" +FIXTURE18="$(make_fixture 2)" +trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12" "$STUB13" "$FIXTURE14" "$FIXTURE17" "$FIXTURE18"' EXIT +SPACED18="$FIXTURE18/plugins/testplugin/skills/zzz skill dir" +mkdir -p "$SPACED18" +mv "$FIXTURE18/plugins/testplugin/skills/zzzskill/SKILL.md" "$SPACED18/SKILL.md" +OUT18=$(run_wrap "$FIXTURE18" --config "$VALE_CONFIG" "plugins/testplugin/skills/zzz skill dir/SKILL.md") +if echo "$OUT18" | grep -q "zzz skill dir/SKILL.md" && echo "$OUT18" | grep -q "VagueWording"; then + pass "a path argument with a space is passed to vale as one word" +else + fail "a path argument with a space was split by the array expansion: $OUT18" +fi + echo "" echo "Results: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]]