fix(lint): avoid bash 4 associative arrays in check-vale-style-sync.sh
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
This commit is contained in:
@@ -81,21 +81,26 @@ done
|
|||||||
# $1's vale-wrap.sh. Records are delimited by their `- id:` line, so the check
|
# $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.
|
# does not depend on `entry:` preceding `files:` within a record.
|
||||||
#
|
#
|
||||||
# Cached per skill (in HOOK_REGEX_CACHE, populated lazily) because the final
|
# Cached per skill (parallel HOOK_REGEX_CACHE_KEYS/_VALS arrays, populated
|
||||||
# validation loop below probes agent-audit twice — once for its CC agent-file
|
# lazily) because the final validation loop below probes agent-audit twice —
|
||||||
# shape, once for its Copilot .agent.md shape — and both probes need the same
|
# once for its CC agent-file shape, once for its Copilot .agent.md shape — and
|
||||||
# regex set. Without the cache, that pair of calls would each re-parse both
|
# both probes need the same regex set. Without the cache, that pair of calls
|
||||||
# manifest files from scratch for no new information. HOOK_REGEX_CACHE_SEEN is
|
# would each re-parse both manifest files from scratch for no new information.
|
||||||
# a separate array so a skill with no matching hooks (empty result) is still
|
# Plain indexed arrays, not `declare -A`: associative arrays are bash 4.0+ and
|
||||||
# recognized as already computed, rather than re-parsed every call.
|
# this script must run on macOS's stock bash 3.2. Only ${#arr[@]} (always safe
|
||||||
declare -A HOOK_REGEX_CACHE=()
|
# on an empty/unset array under `set -u`) and index access are used below —
|
||||||
declare -A HOOK_REGEX_CACHE_SEEN=()
|
# never a bare `${arr[@]}` expansion, which aborts on bash < 4.4 under nounset.
|
||||||
|
HOOK_REGEX_CACHE_KEYS=()
|
||||||
|
HOOK_REGEX_CACHE_VALS=()
|
||||||
hook_file_regexes() {
|
hook_file_regexes() {
|
||||||
local skill="$1" manifest raw result
|
local skill="$1" manifest raw result idx=0
|
||||||
if [[ -n "${HOOK_REGEX_CACHE_SEEN[$skill]:-}" ]]; then
|
while [[ $idx -lt ${#HOOK_REGEX_CACHE_KEYS[@]} ]]; do
|
||||||
printf '%s' "${HOOK_REGEX_CACHE[$skill]}"
|
if [[ "${HOOK_REGEX_CACHE_KEYS[$idx]}" == "$skill" ]]; then
|
||||||
|
printf '%s' "${HOOK_REGEX_CACHE_VALS[$idx]}"
|
||||||
return
|
return
|
||||||
fi
|
fi
|
||||||
|
idx=$((idx + 1))
|
||||||
|
done
|
||||||
result="$(
|
result="$(
|
||||||
for manifest in "$REPO_ROOT/.pre-commit-hooks.yaml" "$REPO_ROOT/.pre-commit-config.yaml"; do
|
for manifest in "$REPO_ROOT/.pre-commit-hooks.yaml" "$REPO_ROOT/.pre-commit-config.yaml"; do
|
||||||
[[ -f "$manifest" ]] || continue
|
[[ -f "$manifest" ]] || continue
|
||||||
@@ -116,8 +121,8 @@ hook_file_regexes() {
|
|||||||
printf '%s\n' "$raw"
|
printf '%s\n' "$raw"
|
||||||
done
|
done
|
||||||
)"
|
)"
|
||||||
HOOK_REGEX_CACHE[$skill]="$result"
|
HOOK_REGEX_CACHE_KEYS[${#HOOK_REGEX_CACHE_KEYS[@]}]="$skill"
|
||||||
HOOK_REGEX_CACHE_SEEN[$skill]=1
|
HOOK_REGEX_CACHE_VALS[${#HOOK_REGEX_CACHE_VALS[@]}]="$result"
|
||||||
printf '%s' "$result"
|
printf '%s' "$result"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -447,7 +447,10 @@ fi
|
|||||||
# scripts, and the test runner AGENTS.md tells contributors to run by hand.
|
# 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
|
# `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
|
# 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 ""
|
||||||
echo "--- no unguarded array expansion remains in the macOS-facing scripts ---"
|
echo "--- no unguarded array expansion remains in the macOS-facing scripts ---"
|
||||||
unguarded_expansions() {
|
unguarded_expansions() {
|
||||||
@@ -474,6 +477,7 @@ for BASH32_SCRIPT in \
|
|||||||
"$SCRIPT" \
|
"$SCRIPT" \
|
||||||
"$REPO_ROOT/scripts/skill-size-check.sh" \
|
"$REPO_ROOT/scripts/skill-size-check.sh" \
|
||||||
"$REPO_ROOT/scripts/check-release-needed.sh" \
|
"$REPO_ROOT/scripts/check-release-needed.sh" \
|
||||||
|
"$REPO_ROOT/scripts/check-vale-style-sync.sh" \
|
||||||
"$REPO_ROOT/tests/run-tests.sh"; do
|
"$REPO_ROOT/tests/run-tests.sh"; do
|
||||||
FOUND16="$(unguarded_expansions "$BASH32_SCRIPT")"
|
FOUND16="$(unguarded_expansions "$BASH32_SCRIPT")"
|
||||||
if [[ -n "$FOUND16" ]]; then
|
if [[ -n "$FOUND16" ]]; then
|
||||||
@@ -486,6 +490,15 @@ for BASH32_SCRIPT in \
|
|||||||
if [[ -n "$FOUND16B" ]]; then
|
if [[ -n "$FOUND16B" ]]; then
|
||||||
HAZARDS16+="${BASH32_SCRIPT##*/}:$FOUND16B "
|
HAZARDS16+="${BASH32_SCRIPT##*/}:$FOUND16B "
|
||||||
fi
|
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
|
done
|
||||||
if [[ -n "$HAZARDS16" ]]; then
|
if [[ -n "$HAZARDS16" ]]; then
|
||||||
fail "unguarded array expansion(s) abort on bash < 4.4 under set -u: $(echo "$HAZARDS16" | tr '\n' ' ')"
|
fail "unguarded array expansion(s) abort on bash < 4.4 under set -u: $(echo "$HAZARDS16" | tr '\n' ' ')"
|
||||||
|
|||||||
Reference in New Issue
Block a user