fix(providers): guard the statusline's unguarded array expansion

`parts` is seeded empty and all seven appends are conditional, so
"${parts[@]}" at the join loop can expand an empty array. install.sh
deploys this file to every user machine.

Two things had to both hold for the bare form to be safe: this file
enabling no `set -u`, and the shell being bash 4.4+, which stopped
treating an empty-array expansion as unbound. On bash 3.2 -- macOS's
system bash, an explicit repo target -- adding `set -u` aborts here.
That is also why the hazard is unreproducible on a modern dev box and
why the enforcement is a static scan rather than a runtime test.

Adds the `providers` glob to test-vale-wrap.sh's bash-3.2 scan, which
excluded it precisely because of this defect. Floor is 1 rather than
"count minus slack": the glob holds one file, so any slack at all
means a floor of 0, which passes vacuously on a renamed directory.

Also adds case 27, the regression test for the stale `shellcheck
source=` directives fixed in the next commit (#97 item 2). It lives in
this file because that is where the exemption it guards lives.

Closes #96
Refs #97

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
This commit is contained in:
2026-08-14 08:03:17 +00:00
parent 55d956b298
commit 49d21bcb4d
2 changed files with 124 additions and 13 deletions

View File

@@ -92,8 +92,17 @@ parts=()
[ -n "$vim_mode" ] && parts+=("${COLOR_MAGENTA}${vim_mode}${COLOR_RESET}") [ -n "$vim_mode" ] && parts+=("${COLOR_MAGENTA}${vim_mode}${COLOR_RESET}")
# --- Join with ' · ' separator and print --- # --- Join with ' · ' separator and print ---
# `parts` is seeded empty above and every append is conditional, so all seven can
# be false and the array can reach here with no elements. Two things had to both
# stay true for bare "${parts[@]}" to be safe: this file enabling no `set -u`
# (house style in every other script here is `set -euo pipefail`), and the shell
# being bash 4.4+, which stopped treating an empty-array expansion as unbound.
# On bash 3.2 -- macOS's system bash, and an explicit repo target -- adding
# `set -u` aborts here. The guarded form removes the tripwire instead of relying
# on both conditions holding. install.sh deploys this file to every user
# machine. See issue #96.
output="" output=""
for part in "${parts[@]}"; do for part in ${parts[@]+"${parts[@]}"}; do
[ -z "$output" ] && output="$part" || output="${output} · ${part}" [ -z "$output" ] && output="$part" || output="${output} · ${part}"
done done

View File

@@ -452,21 +452,28 @@ fi
# this branch introduced, whose own header (batch-run.sh:9-11) documents it as # this branch introduced, whose own header (batch-run.sh:9-11) documents it as
# bash-3.2-safe — along with four other scripts/*.sh. Deriving the list means a # bash-3.2-safe — along with four other scripts/*.sh. Deriving the list means a
# new script is covered the moment it lands. AGENTS.md names bash 3.2 as an # new script is covered the moment it lands. AGENTS.md names bash 3.2 as an
# explicit repo target, so the scope is three globs, each floor-asserted below: # explicit repo target, so the scope is four globs, each floor-asserted below:
# - scripts/**/*.sh — repo tooling and pre-commit hook scripts # - scripts/**/*.sh — repo tooling and pre-commit hook scripts
# - tests/*.sh — the runners and every regression test # - tests/*.sh — the runners and every regression test
# - plugins/*/.apm/**/*.sh — the scripts plugins ship to users # - plugins/*/.apm/**/*.sh — the scripts plugins ship to users
# - providers/**/*.sh — the scripts install.sh deploys to user machines
# plugins/*/skills/** is deliberately NOT scanned: it is the generated mirror of # plugins/*/skills/** is deliberately NOT scanned: it is the generated mirror of
# .apm/, so scanning both double-reports every finding, and mirror-vs-source # .apm/, so scanning both double-reports every finding, and mirror-vs-source
# drift is already sync-plugin-content.sh --check's job. Scanning .apm/ is what # drift is already sync-plugin-content.sh --check's job. Scanning .apm/ is what
# closed the gap where skill-audit's vale-wrap.sh was covered but agent-audit's # closed the gap where skill-audit's vale-wrap.sh was covered but agent-audit's
# byte-identical copy of it was not. # byte-identical copy of it was not.
# providers/**/*.sh is the one shipped script deliberately left out: # providers/**/*.sh was excluded until issue #96: statusline-command.sh seeded
# providers/claude-code/statusline-command.sh seeds `parts=()` empty at :83 and # `parts=()` empty and expanded it unguarded, a real latent hazard rather than a
# expands it unguarded at :96. That is a real latent hazard rather than a false # false positive, in a file this case did not own. That expansion is now guarded,
# positive — it just cannot abort today because the file enables no `set -u`. # so the glob is in the table below and the deployed provider scripts get the
# Fixing it is a change to a file this case does not own; once :96 uses the # same coverage as scripts/, tests/ and plugins/*/.apm/. This is the glob most
# guarded form, add a `providers` glob to the table below. # worth having: install.sh copies providers/ content onto every user machine, and
# it is also the glob most likely to look redundant to a future reader, because
# the hazard it catches is invisible on any modern bash — bash 4.4 stopped
# treating an empty-array expansion as unbound, so a bare "${a[@]}" under `set -u`
# runs clean on the maintainer's bash 5 and aborts only on macOS's bash 3.2.
# A runtime test cannot demonstrate that without a 3.2 binary; this static scan
# is the enforcement, which is why its floor must never be dropped to zero.
# #
# Four constructs are checked, because the expansion scan cannot see any of the # Four constructs are checked, because the expansion scan cannot see any of the
# other three: # other three:
@@ -588,19 +595,28 @@ bash32_glob() {
scripts) find "$REPO_ROOT/scripts" -name '*.sh' -type f ;; scripts) find "$REPO_ROOT/scripts" -name '*.sh' -type f ;;
tests) find "$REPO_ROOT/tests" -maxdepth 1 -name '*.sh' -type f ;; tests) find "$REPO_ROOT/tests" -maxdepth 1 -name '*.sh' -type f ;;
plugins) find "$REPO_ROOT/plugins" -path '*/.apm/*' -name '*.sh' -type f ;; plugins) find "$REPO_ROOT/plugins" -path '*/.apm/*' -name '*.sh' -type f ;;
providers) find "$REPO_ROOT/providers" -name '*.sh' -type f ;;
*) echo "bash32_glob: unknown glob '$1'" >&2; return 1 ;; *) echo "bash32_glob: unknown glob '$1'" >&2; return 1 ;;
esac esac
} }
# Floors are PER GLOB, not on the merged total. A single total floor cannot # Floors are PER GLOB, not on the merged total. A single total floor cannot
# detect the failure this assertion exists to name: with 12 + 17 + 13 files, # detect the failure this assertion exists to name: with 12 + 18 + 13 + 1 files,
# losing the whole `scripts` glob still leaves 30 and losing the whole `tests` # losing the whole `scripts` glob still leaves 32 and losing the whole `tests`
# glob still leaves 25, so any total floor low enough to survive normal churn # glob still leaves 26, so any total floor low enough to survive normal churn
# is too low to notice an entire glob silently resolving to nothing. Each floor # is too low to notice an entire glob silently resolving to nothing. Each floor
# sits a little under its current count so ordinary file removal does not trip # sits a little under its current count so ordinary file removal does not trip
# it, but a broken or renamed path does. Parallel arrays rather than an # it, but a broken or renamed path does. Parallel arrays rather than an
# associative one — `declare -A` is bash 4.0+, which this very case forbids. # associative one — `declare -A` is bash 4.0+, which this very case forbids.
BASH32_GLOB_NAMES=(scripts tests plugins) #
BASH32_GLOB_FLOORS=(10 14 10) # `providers` is floored at 1 rather than "current count minus slack" because it
# holds exactly one file: any slack at all would be a floor of 0, which passes on
# a renamed or deleted directory and is precisely the silent-zero failure the
# per-glob floors exist to catch. A floor of 1 means removing the last provider
# script trips this assertion — correct, because at that point the glob is dead
# weight and should be deleted from the table deliberately, not left to pass
# vacuously.
BASH32_GLOB_NAMES=(scripts tests plugins providers)
BASH32_GLOB_FLOORS=(10 14 10 1)
BASH32_SCRIPTS=() BASH32_SCRIPTS=()
BASH32_IDX=0 BASH32_IDX=0
while [[ $BASH32_IDX -lt ${#BASH32_GLOB_NAMES[@]} ]]; do while [[ $BASH32_IDX -lt ${#BASH32_GLOB_NAMES[@]} ]]; do
@@ -1091,6 +1107,92 @@ else
pass "all 6 never-empty shell arrays are exempt and all 5 sometimes-empty ones are still flagged" pass "all 6 never-empty shell arrays are exempt and all 5 sometimes-empty ones are still flagged"
fi fi
# Case 27 pins the `sourced_files()` exemption against the failure mode that
# actually happened (issue #97 item 2): a stale source-directive path.
#
# The exemption exists because array seeding often lives in the sourced file
# rather than the sourcing one -- install.sh's DEPLOY_* come from
# deploy-manifest.sh -- so without it those expansions read as hazards. It works
# by parsing each file's own source-directive comments. That makes it silently
# dependent on those paths resolving: a directive naming a file that is not
# there does not error, it just contributes no seed file, and the exemption
# quietly stops covering what it was written to cover.
#
# tests/run-tests.sh shipped exactly that for the length of PR #95 -- a directive
# resolving to neither the repo root nor the script's own directory. Nothing
# caught it: shellcheck's own SC1091 is `info`, and .pre-commit-config.yaml pins
# `--severity=warning`. The live consequence was nil only because both of
# run-tests.sh's expansions were independently guarded.
#
# Part C is the assertion that would have caught it, and it is the reason this
# case is not just fixture theatre: it holds every real directive in the scanned
# corpus to the resolution rule, so the next stale one fails here rather than
# lying dormant. Parts A and B pin the mechanism Part C depends on -- that
# resolution is what switches the exemption on, and non-resolution silently
# switches it off.
echo ""
echo "--- every shellcheck source directive resolves, so no seeding exemption is silently off ---"
FIXTURE27="$(mktemp -d)"
new_fixture "$FIXTURE27"
# The seeded array lives ONLY in the sourced file, never in the sourcing one --
# that separation is the whole point of the exemption.
{
echo "#!/usr/bin/env bash"
echo "SEEDED27=(a b c)"
} > "$FIXTURE27/seed-lib.sh"
# Both fixture bodies are emitted through printf with the closing `]` passed as
# an argument, never written literally. This file is itself inside the `tests`
# glob, so a literal bare expansion here would be scanned as a hazard in
# test-vale-wrap.sh's own source -- the trailing-comment/inside-a-string caveat
# documented above strip_comments(). Splitting the token means the emitted
# fixture holds the real text while this file holds no bare spelling, the same
# trick the `npro[c]` rule uses.
EXPANSION27="$(printf 'echo "${SEEDED27[@%s}"' ']')"
# Part A: a directive that resolves against the script's own directory exempts
# the expansion, so no hazard is reported.
{
echo "#!/usr/bin/env bash"
echo "# shellcheck source=seed-lib.sh"
echo 'source "$(dirname "$0")/seed-lib.sh"'
echo "$EXPANSION27"
} > "$FIXTURE27/resolves.sh"
# Part B: byte-identical except the directive names a file that is not there.
{
echo "#!/usr/bin/env bash"
echo "# shellcheck source=nonexistent/seed-lib.sh"
echo 'source "$(dirname "$0")/seed-lib.sh"'
echo "$EXPANSION27"
} > "$FIXTURE27/stale.sh"
RESOLVES27="$(unguarded_expansions "$FIXTURE27/resolves.sh")"
STALE27="$(unguarded_expansions "$FIXTURE27/stale.sh")"
# Part C: no real directive in the derived corpus may fail to resolve. Counted
# rather than diffed, because sourced_files() reports resolution only by
# omission -- an unresolvable directive produces no output line at all.
UNRESOLVED27=""
for BASH32_SCRIPT in ${BASH32_SCRIPTS[@]+"${BASH32_SCRIPTS[@]}"}; do
DECLARED27="$(grep -cE '^[[:space:]]*#[[:space:]]*shellcheck[[:space:]]+source=[^[:space:]]+' "$BASH32_SCRIPT" || true)"
[[ "$DECLARED27" -gt 0 ]] || continue
RESOLVED27="$(sourced_files "$BASH32_SCRIPT" | grep -c . || true)"
if [[ "$RESOLVED27" -lt "$DECLARED27" ]]; then
UNRESOLVED27+="${BASH32_SCRIPT#"$REPO_ROOT"/} ($RESOLVED27/$DECLARED27) "
fi
done
if [[ -n "$RESOLVES27" ]]; then
fail "an array seeded only in a resolvable sourced file was reported as a hazard, so the seeding exemption is not reading source directives at all"
elif [[ -z "$STALE27" ]]; then
fail "an array seeded only in an UNRESOLVABLE sourced file was still exempted, so this case cannot detect a stale directive and Part C proves nothing"
elif [[ -n "$UNRESOLVED27" ]]; then
fail "shellcheck source directive(s) resolve to nothing, silently disarming the seeding exemption for those files: $UNRESOLVED27"
else
pass "the seeding exemption switches on only for resolvable directives, and every directive in the scanned corpus resolves"
fi
echo "" echo ""
echo "Results: $PASS passed, $FAIL failed" echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]] [[ $FAIL -eq 0 ]]