Guard unguarded array expansion in statusline-command.sh (latent, ships to users) #96

New Issue

Claude · 2026-08-14T07:00:05Z

Claude commented

2026-08-14 07:00:05 +00:00

Found during PR #95's review round, in a file that PR does not touch. Filed separately rather than piggybacked — that PR already carries one unrelated fix and the review flagged undisclosed scope as a problem.

The defect

providers/claude-code/statusline-command.sh:

:85   parts=()
:86   [ -n "$dir_name" ]         && parts+=(...)
:87   [ -n "$git_branch" ]       && parts+=(...)
      ...   # all 7 appends conditional, every one can be false
:96   for part in "${parts[@]}"; do

parts is seeded empty and every append is guarded by a condition that can be false, so "${parts[@]}" at :96 can expand an empty array.

Why it is latent, not live

The file sets no set -u (grep -n '^set ' providers/claude-code/statusline-command.sh → no output), so an empty array expands to nothing and the loop simply does not run. Output is an empty string, which is correct behaviour.

It becomes a hard failure the moment anyone adds set -u — which is the house style in this repo's other scripts.

Why it matters

install.sh:16 deploys this file to every user machine:

"providers/claude-code/statusline-command.sh:.claude/statusline-command.sh"

Fix

Guard the expansion with the idiom already used throughout the repo:

for part in ${parts[@]+"${parts[@]}"}; do

or the short-circuit form used in the cleanup traps:

[[ ${#parts[@]} -eq 0 ]] || for part in "${parts[@]}"; do ...

Unblocks

tests/test-vale-wrap.sh's derived bash-3.2 scan currently excludes providers/** specifically because of this hazard, with the exclusion and its reason recorded in-file. Fixing this allows adding a providers glob so deployed provider scripts get the same bash-3.2 coverage as scripts/, tests/ and plugins/*/.apm/.

Verification note: an earlier review pass misread :85 as seeding the array non-empty. It does not — the appends at :86+ are all conditional. Confirmed by reading the file directly.

Found during PR #95's review round, in a file that PR does not touch. Filed separately rather than piggybacked — that PR already carries one unrelated fix and the review flagged undisclosed scope as a problem. ## The defect `providers/claude-code/statusline-command.sh`: ```bash :85 parts=() :86 [ -n "$dir_name" ] && parts+=(...) :87 [ -n "$git_branch" ] && parts+=(...) ... # all 7 appends conditional, every one can be false :96 for part in "${parts[@]}"; do ``` `parts` is seeded empty and every append is guarded by a condition that can be false, so `"${parts[@]}"` at `:96` can expand an empty array. ## Why it is latent, not live The file sets no `set -u` (`grep -n '^set ' providers/claude-code/statusline-command.sh` → no output), so an empty array expands to nothing and the loop simply does not run. Output is an empty string, which is correct behaviour. It becomes a hard failure the moment anyone adds `set -u` — which is the house style in this repo's other scripts. ## Why it matters `install.sh:16` deploys this file to every user machine: ``` "providers/claude-code/statusline-command.sh:.claude/statusline-command.sh" ``` ## Fix Guard the expansion with the idiom already used throughout the repo: ```bash for part in ${parts[@]+"${parts[@]}"}; do ``` or the short-circuit form used in the cleanup traps: ```bash [[ ${#parts[@]} -eq 0 ]] || for part in "${parts[@]}"; do ... ``` ## Unblocks `tests/test-vale-wrap.sh`'s derived bash-3.2 scan currently excludes `providers/**` specifically because of this hazard, with the exclusion and its reason recorded in-file. Fixing this allows adding a `providers` glob so deployed provider scripts get the same bash-3.2 coverage as `scripts/`, `tests/` and `plugins/*/.apm/`. Verification note: an earlier review pass misread `:85` as seeding the array non-empty. It does not — the appends at `:86`+ are all conditional. Confirmed by reading the file directly.

Claude added the Kind/Bug

labels 2026-08-14 07:00:37 +00:00

Defame1297 referenced this issue from a commit

2026-08-14 09:27:00 +00:00

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

Defame1297 closed this issue

2026-08-14 09:27:00 +00:00

Claude referenced a pull request that will close this issue

2026-08-14 09:28:03 +00:00

feat(kyberforge): execute the plugin→APM conversion #95

Claude commented

2026-08-14 09:28:20 +00:00

Fixed in 49d21bc on feat/90-execute-apm-conversion, folded into PR #95 (disclosed there, since this widens that PR into providers/ — an area it did not previously touch).

What landed

The join loop now uses the repo idiom:

for part in ${parts[@]+"${parts[@]}"}; do

and providers/**/*.sh is in test-vale-wrap.sh's bash-3.2 scan — the exclusion this issue's "Unblocks" section describes is gone. 44 files now scanned.

Floor for the new glob is 1, not "current count minus slack". The glob holds exactly one file, so any slack at all means a floor of 0, which passes vacuously on a renamed or deleted directory — precisely the silent-zero failure the per-glob floors exist to catch.

Correction to the issue's framing

it just cannot abort today because the file enables no set -u

That is one of two conditions, and the issue names only the first. bash 4.4 stopped treating an empty-array expansion as unbound. So the pre-fix code is safe if either no set -u is enabled or the shell is bash 4.4+. On bash 3.2 — macOS's system bash, and the reason this scan exists — adding set -u aborts.

The practical consequence is worth recording, because it will mislead the next person who tries to reproduce this: the hazard cannot be demonstrated at runtime on a modern dev box. Measured here on bash 5.2:

$ echo '{}' | bash -u statusline-command.sh   # pre-fix
rc=0

No abort. It looks like a false positive. It is not — it is a shell-version artifact. There is no bash 3.2 binary in this environment to demonstrate it with, which is exactly why the enforcement is a static scan rather than a runtime test, and why the providers floor must never be dropped to zero. Both facts are now recorded in the two files themselves so the glob is not later deleted as redundant.

Verification

Behaviour unchanged: normal input renders identically; all-conditions-false input emits an empty string; bash -u now exits 0 either way.
Mutation: restoring the bare "${parts[@]}" produces FAIL: bash-4-only construct(s) found in 44 scanned script(s): statusline-command.sh:105.
Mutation: pointing the providers glob at a wrong path produces FAIL: … derived 0 file(s), under its floor of 1.
shellcheck --severity=warning clean; full suite 16/16; all 14 pre-push hooks pass.

The verification note in the issue was correct — :85 does seed the array empty and every append is conditional.

Fixed in `49d21bc` on `feat/90-execute-apm-conversion`, folded into PR #95 (disclosed there, since this widens that PR into `providers/` — an area it did not previously touch). ## What landed The join loop now uses the repo idiom: ```bash for part in ${parts[@]+"${parts[@]}"}; do ``` and `providers/**/*.sh` is in `test-vale-wrap.sh`'s bash-3.2 scan — the exclusion this issue's "Unblocks" section describes is gone. 44 files now scanned. Floor for the new glob is **1**, not "current count minus slack". The glob holds exactly one file, so any slack at all means a floor of 0, which passes vacuously on a renamed or deleted directory — precisely the silent-zero failure the per-glob floors exist to catch. ## Correction to the issue's framing > it just cannot abort today because the file enables no `set -u` That is one of **two** conditions, and the issue names only the first. bash 4.4 stopped treating an empty-array expansion as unbound. So the pre-fix code is safe if *either* no `set -u` is enabled *or* the shell is bash 4.4+. On bash 3.2 — macOS's system bash, and the reason this scan exists — adding `set -u` aborts. The practical consequence is worth recording, because it will mislead the next person who tries to reproduce this: **the hazard cannot be demonstrated at runtime on a modern dev box.** Measured here on bash 5.2: ``` $ echo '{}' | bash -u statusline-command.sh # pre-fix rc=0 ``` No abort. It looks like a false positive. It is not — it is a shell-version artifact. There is no bash 3.2 binary in this environment to demonstrate it with, which is exactly why the enforcement is a **static scan** rather than a runtime test, and why the `providers` floor must never be dropped to zero. Both facts are now recorded in the two files themselves so the glob is not later deleted as redundant. ## Verification - Behaviour unchanged: normal input renders identically; all-conditions-false input emits an empty string; `bash -u` now exits 0 either way. - Mutation: restoring the bare `"${parts[@]}"` produces `FAIL: bash-4-only construct(s) found in 44 scanned script(s): statusline-command.sh:105`. - Mutation: pointing the `providers` glob at a wrong path produces `FAIL: … derived 0 file(s), under its floor of 1`. - `shellcheck --severity=warning` clean; full suite 16/16; all 14 pre-push hooks pass. The verification note in the issue was correct — `:85` does seed the array empty and every append is conditional.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Defame1297/holocron#96