From 389a4f0f7a600ed2fab264eabe1190214cb29410 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Mon, 10 Aug 2026 07:37:55 +0000 Subject: [PATCH] fix(lint): avoid bash 4 associative arrays in check-vale-style-sync.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check-vale-style-sync.sh used `declare -A` for a per-skill regex cache. Associative arrays are bash 4.0+; this script runs as an always-run pre-push hook with `language: system`, so it inherits whatever bash is first on the invoking user's PATH. On macOS's stock bash 3.2, `declare -A` at top level aborts immediately under `set -euo pipefail` — every push would hard-fail before the sync check ran anything. Replaced with two parallel indexed arrays (HOOK_REGEX_CACHE_KEYS/_VALS), linear-scanned by index — same caching behavior (avoids re-parsing both pre-commit manifests when agent-audit is probed twice), but only ever uses ${#arr[@]} and index access, never a bare ${arr[@]} expansion. Extended test-vale-wrap.sh's existing bash-3.2 hazard sweep to scan this file too, and added a check for `declare -A` itself — it previously only caught unguarded ${arr[@]} expansions and mapfile/readarray, so this exact regression had no test that would have caught it. Refs: #85 --- scripts/check-vale-style-sync.sh | 37 ++++++++++++++++++-------------- tests/test-vale-wrap.sh | 15 ++++++++++++- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/scripts/check-vale-style-sync.sh b/scripts/check-vale-style-sync.sh index 49e6a9d..cd4a0d6 100755 --- a/scripts/check-vale-style-sync.sh +++ b/scripts/check-vale-style-sync.sh @@ -81,21 +81,26 @@ done # $1's vale-wrap.sh. Records are delimited by their `- id:` line, so the check # does not depend on `entry:` preceding `files:` within a record. # -# Cached per skill (in HOOK_REGEX_CACHE, populated lazily) because the final -# validation loop below probes agent-audit twice — once for its CC agent-file -# shape, once for its Copilot .agent.md shape — and both probes need the same -# regex set. Without the cache, that pair of calls would each re-parse both -# manifest files from scratch for no new information. HOOK_REGEX_CACHE_SEEN is -# a separate array so a skill with no matching hooks (empty result) is still -# recognized as already computed, rather than re-parsed every call. -declare -A HOOK_REGEX_CACHE=() -declare -A HOOK_REGEX_CACHE_SEEN=() +# Cached per skill (parallel HOOK_REGEX_CACHE_KEYS/_VALS arrays, populated +# lazily) because the final validation loop below probes agent-audit twice — +# once for its CC agent-file shape, once for its Copilot .agent.md shape — and +# both probes need the same regex set. Without the cache, that pair of calls +# would each re-parse both manifest files from scratch for no new information. +# Plain indexed arrays, not `declare -A`: associative arrays are bash 4.0+ and +# this script must run on macOS's stock bash 3.2. Only ${#arr[@]} (always safe +# on an empty/unset array under `set -u`) and index access are used below — +# never a bare `${arr[@]}` expansion, which aborts on bash < 4.4 under nounset. +HOOK_REGEX_CACHE_KEYS=() +HOOK_REGEX_CACHE_VALS=() hook_file_regexes() { - local skill="$1" manifest raw result - if [[ -n "${HOOK_REGEX_CACHE_SEEN[$skill]:-}" ]]; then - printf '%s' "${HOOK_REGEX_CACHE[$skill]}" - return - fi + local skill="$1" manifest raw result idx=0 + while [[ $idx -lt ${#HOOK_REGEX_CACHE_KEYS[@]} ]]; do + if [[ "${HOOK_REGEX_CACHE_KEYS[$idx]}" == "$skill" ]]; then + printf '%s' "${HOOK_REGEX_CACHE_VALS[$idx]}" + return + fi + idx=$((idx + 1)) + done result="$( for manifest in "$REPO_ROOT/.pre-commit-hooks.yaml" "$REPO_ROOT/.pre-commit-config.yaml"; do [[ -f "$manifest" ]] || continue @@ -116,8 +121,8 @@ hook_file_regexes() { printf '%s\n' "$raw" done )" - HOOK_REGEX_CACHE[$skill]="$result" - HOOK_REGEX_CACHE_SEEN[$skill]=1 + HOOK_REGEX_CACHE_KEYS[${#HOOK_REGEX_CACHE_KEYS[@]}]="$skill" + HOOK_REGEX_CACHE_VALS[${#HOOK_REGEX_CACHE_VALS[@]}]="$result" printf '%s' "$result" } diff --git a/tests/test-vale-wrap.sh b/tests/test-vale-wrap.sh index c823a38..f883df6 100755 --- a/tests/test-vale-wrap.sh +++ b/tests/test-vale-wrap.sh @@ -447,7 +447,10 @@ fi # scripts, and the test runner AGENTS.md tells contributors to run by hand. # `mapfile` is checked alongside, because it is bash 4.0+ and the expansion scan # cannot see it — run-tests.sh carried one until it was replaced with a -# `while read` loop, and nothing would have caught its return. +# `while read` loop, and nothing would have caught its return. `declare -A` +# (bash 4.0+ associative arrays) is checked for the same reason — the +# expansion scan cannot see it, and check-vale-style-sync.sh carried a pair of +# them until they were replaced with index-scanned plain arrays. echo "" echo "--- no unguarded array expansion remains in the macOS-facing scripts ---" unguarded_expansions() { @@ -474,6 +477,7 @@ for BASH32_SCRIPT in \ "$SCRIPT" \ "$REPO_ROOT/scripts/skill-size-check.sh" \ "$REPO_ROOT/scripts/check-release-needed.sh" \ + "$REPO_ROOT/scripts/check-vale-style-sync.sh" \ "$REPO_ROOT/tests/run-tests.sh"; do FOUND16="$(unguarded_expansions "$BASH32_SCRIPT")" if [[ -n "$FOUND16" ]]; then @@ -486,6 +490,15 @@ for BASH32_SCRIPT in \ if [[ -n "$FOUND16B" ]]; then HAZARDS16+="${BASH32_SCRIPT##*/}:$FOUND16B " fi + # `declare -A` (associative arrays) is bash 4.0+ with no 3.2 fallback. The + # flag cluster can carry other letters in any order (-Ag, -rA, ...); what + # matters is a literal uppercase A appearing in it, so match on that rather + # than the exact string "-A". + FOUND16C="$(awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$BASH32_SCRIPT" \ + | grep -nE '(^|[^[:alnum:]_])declare[[:space:]]+-[a-zA-Z]*A[a-zA-Z]*([[:space:]]|$)' || true)" + if [[ -n "$FOUND16C" ]]; then + HAZARDS16+="${BASH32_SCRIPT##*/}:$FOUND16C " + fi done if [[ -n "$HAZARDS16" ]]; then fail "unguarded array expansion(s) abort on bash < 4.4 under set -u: $(echo "$HAZARDS16" | tr '\n' ' ')"