From e36a1df745e9e558d047f798ef769a84f869b628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Wed, 4 Nov 2026 06:48:34 -0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix(actions):=20guard=20empty=20?= =?UTF-8?q?arrays=20for=20bash=203.2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit macOS runners run `shell: bash` steps with /bin/bash 3.2. Under `set -u` that shell aborts on `"${array[@]}"` when the array is empty, so `install-locked-tools` failed on macOS when no targets were given. The other composite Actions and the check workflow expand arrays the same way. Expand arrays that can be empty with `${array[@]+"${array[@]}"}`, which bash 3.2 accepts. Arrays that are always filled where they expand keep the plain form. A test scans the run blocks of every Action, the tools scripts, and the workflow jobs that take their runner from an expression, and fails on any unguarded expansion that is not on its list of provably non-empty arrays. --- .github/workflows/check.yaml | 6 +-- changelog.d/+bash32-empty-arrays.bugfix.md | 3 ++ cooled-prek/action.yml | 2 +- install-locked-tools/action.yml | 6 +-- publish-post-release-pr/action.yml | 5 ++- refresh-release-lock/action.yml | 22 +++++----- select-previous-tag/action.yml | 2 +- tests/bash32_arrays/test_empty_arrays.py | 50 ++++++++++++++++++++++ tests/check_workflow/test_check.py | 7 +++ validate-release-patch/action.yml | 2 +- 10 files changed, 84 insertions(+), 21 deletions(-) create mode 100644 changelog.d/+bash32-empty-arrays.bugfix.md create mode 100644 tests/bash32_arrays/test_empty_arrays.py diff --git a/.github/workflows/check.yaml b/.github/workflows/check.yaml index 5e89d02..10c743f 100644 --- a/.github/workflows/check.yaml +++ b/.github/workflows/check.yaml @@ -69,7 +69,7 @@ jobs: fi read -ra packages <<<"$APT_PACKAGES" sudo apt-get update - sudo apt-get install -y "${packages[@]}" + sudo apt-get install -y ${packages[@]+"${packages[@]}"} - name: 🐍 Install uv and Python uses: astral-sh/setup-uv@07a047e608db84b3d8c06cd9a7c7dcc9c355c18e # v10.3.2 with: @@ -91,8 +91,8 @@ jobs: while IFS= read -r line; do if [[ -n "$line" ]]; then options+=(--exclude-newer-package "$line"); fi done <<<"$EXCLUDE_NEWER_PACKAGE" - echo "Installing ${spec[*]} with uploads older than $EXCLUDE_NEWER" - uv tool install --python-preference only-managed --python 3.15 "${options[@]}" "${spec[@]}" + echo "Installing ${spec[*]+"${spec[*]}"} with uploads older than $EXCLUDE_NEWER" + uv tool install --python-preference only-managed --python 3.15 "${options[@]}" ${spec[@]+"${spec[@]}"} - name: 🐍 Install Python id: python background: true diff --git a/changelog.d/+bash32-empty-arrays.bugfix.md b/changelog.d/+bash32-empty-arrays.bugfix.md new file mode 100644 index 0000000..bdba86d --- /dev/null +++ b/changelog.d/+bash32-empty-arrays.bugfix.md @@ -0,0 +1,3 @@ +Actions and the `check` workflow no longer fail with `unbound variable` on macOS runners when an input leaves an array +empty, such as the `targets` input of `install-locked-tools`. macOS runs `shell: bash` steps with Bash 3.2, which +rejects `"${array[@]}"` on an empty array under `set -u`. diff --git a/cooled-prek/action.yml b/cooled-prek/action.yml index 809f822..3f94b3d 100644 --- a/cooled-prek/action.yml +++ b/cooled-prek/action.yml @@ -14,4 +14,4 @@ runs: run: | set -euo pipefail read -ra arguments <<<"$ARGS" - "$GITHUB_ACTION_PATH/../tools/cooled_prek.sh" "${arguments[@]}" + "$GITHUB_ACTION_PATH/../tools/cooled_prek.sh" ${arguments[@]+"${arguments[@]}"} diff --git a/install-locked-tools/action.yml b/install-locked-tools/action.yml index f7a27c3..b34f14c 100644 --- a/install-locked-tools/action.yml +++ b/install-locked-tools/action.yml @@ -50,7 +50,7 @@ runs: groups=() while IFS= read -r line; do read -ra selected <<<"$line" - groups+=("${selected[@]}") + groups+=(${selected[@]+"${selected[@]}"}) done <<<"$DEPENDENCY_GROUPS" if ((${#groups[@]})); then echo "Selected lock groups: ${groups[*]} (from the groups input), installing into $VENV from pylock.toml" @@ -82,7 +82,7 @@ runs: fi digest=$( sha256sum "$BUILD_LOCK" - printf '%s\n' "$TARGET_PLATFORM" "$TARGETS" "${args[@]}" + printf '%s\n' "$TARGET_PLATFORM" "$TARGETS" ${args[@]+"${args[@]}"} ) digest=$(sha256sum <<<"$digest") output="$RUNNER_TEMP/build-constraints-${digest%% *}.txt" @@ -90,7 +90,7 @@ runs: echo "Reusing build constraints from an earlier step (same lock and targets): $output" else uv run --no-project --python "$RUNNER_TEMP/lock-reader" python \ - "$GITHUB_ACTION_PATH/../tools/build_constraints.py" "${args[@]}" "$output" "$BUILD_LOCK" + "$GITHUB_ACTION_PATH/../tools/build_constraints.py" ${args[@]+"${args[@]}"} "$output" "$BUILD_LOCK" fi echo "path=$output" >>"$GITHUB_OUTPUT" - name: 📦 Install selected groups diff --git a/publish-post-release-pr/action.yml b/publish-post-release-pr/action.yml index 1f752cb..3ccd94f 100644 --- a/publish-post-release-pr/action.yml +++ b/publish-post-release-pr/action.yml @@ -41,7 +41,7 @@ runs: while IFS= read -r path || [[ -n "$path" ]]; do trusted_paths+=("$path") done < <(printf '%s' "$ALLOWED_PATHS") - printf '%s\n' "${trusted_paths[@]}" | LC_ALL=C sort -u >"$RUNNER_TEMP/allowed-paths" + printf '%s\n' ${trusted_paths[@]+"${trusted_paths[@]}"} | LC_ALL=C sort -u >"$RUNNER_TEMP/allowed-paths" paths=() while IFS=$'\t' read -r path blob; do case "$path" in @@ -51,7 +51,8 @@ runs: ;; esac if ! grep -Fqx -- "$path" "$RUNNER_TEMP/allowed-paths"; then - echo "::error::prepared path $path is not in the writer's allowed-paths: ${trusted_paths[*]}" >&2 + echo "::error::prepared path $path is not in the writer's allowed-paths:" \ + "${trusted_paths[*]+"${trusted_paths[*]}"}" >&2 exit 1 fi case "$blob" in diff --git a/refresh-release-lock/action.yml b/refresh-release-lock/action.yml index 17bc3f7..496aa7e 100644 --- a/refresh-release-lock/action.yml +++ b/refresh-release-lock/action.yml @@ -83,7 +83,8 @@ runs: ;; esac cp pylock.toml "$RUNNER_TEMP/lock-before.toml" - uv run --no-project --python "$RUNNER_TEMP/lock-tools" nab lock --all-groups --all-extras --no-cache "${refresh[@]}" + uv run --no-project --python "$RUNNER_TEMP/lock-tools" nab lock --all-groups --all-extras --no-cache \ + ${refresh[@]+"${refresh[@]}"} if [[ "$MODE" == check ]]; then python3 "$GITHUB_ACTION_PATH/../tools/check_nab_lock.py" "$RUNNER_TEMP/lock-before.toml" pylock.toml fi @@ -110,15 +111,15 @@ runs: selected_groups=() while IFS= read -r line; do read -ra words <<<"$line" - platforms+=("${words[@]}") + platforms+=(${words[@]+"${words[@]}"}) done <<<"$MATRIX_PLATFORMS" while IFS= read -r line; do read -ra words <<<"$line" - implementations+=("${words[@]}") + implementations+=(${words[@]+"${words[@]}"}) done <<<"$MATRIX_IMPLEMENTATIONS" while IFS= read -r line; do read -ra words <<<"$line" - selected_groups+=("${words[@]}") + selected_groups+=(${words[@]+"${words[@]}"}) done <<<"$ADDITIONAL_GROUPS" case "$ADDITIONAL_SELECTION" in project) @@ -134,14 +135,15 @@ runs: fi ;; esac - echo "Secondary lock $ADDITIONAL_LOCK: $description for Python $MATRIX_PYTHON on ${platforms[*]}" \ - "with ${implementations[*]}, ${MODE} mode" + echo "Secondary lock $ADDITIONAL_LOCK: $description for Python $MATRIX_PYTHON" \ + "on ${platforms[*]+"${platforms[*]}"} with ${implementations[*]+"${implementations[*]}"}, ${MODE} mode" cp "$ADDITIONAL_LOCK" "$RUNNER_TEMP/additional-lock-before.toml" - uv run --no-project --python "$RUNNER_TEMP/lock-tools" nab lock --no-cache pyproject.toml "${selection[@]}" \ + uv run --no-project --python "$RUNNER_TEMP/lock-tools" nab lock --no-cache pyproject.toml \ + ${selection[@]+"${selection[@]}"} \ --project-matrix-python "$MATRIX_PYTHON" \ - --project-matrix-platforms "${platforms[@]}" \ - --project-matrix-implementations "${implementations[@]}" \ - --output "$ADDITIONAL_LOCK" "${refresh[@]}" + --project-matrix-platforms ${platforms[@]+"${platforms[@]}"} \ + --project-matrix-implementations ${implementations[@]+"${implementations[@]}"} \ + --output "$ADDITIONAL_LOCK" ${refresh[@]+"${refresh[@]}"} if [[ "$MODE" == check ]]; then python3 "$GITHUB_ACTION_PATH/../tools/check_nab_lock.py" \ "$RUNNER_TEMP/additional-lock-before.toml" "$ADDITIONAL_LOCK" diff --git a/select-previous-tag/action.yml b/select-previous-tag/action.yml index a855e06..2fb8910 100644 --- a/select-previous-tag/action.yml +++ b/select-previous-tag/action.yml @@ -20,7 +20,7 @@ runs: set -euo pipefail args=() if [[ "$TAG" == */* ]]; then args+=(--package "${TAG%%/*}"); fi - previous=$(git tag --list | release-creator tags "$TAG" --previous "${args[@]}") + previous=$(git tag --list | release-creator tags "$TAG" --previous ${args[@]+"${args[@]}"}) if [[ -n "$previous" ]]; then echo "Previous release tag: $previous (newest release tag before $TAG${args[*]:+ for package ${TAG%%/*}})" else diff --git a/tests/bash32_arrays/test_empty_arrays.py b/tests/bash32_arrays/test_empty_arrays.py new file mode 100644 index 0000000..ea6c2a8 --- /dev/null +++ b/tests/bash32_arrays/test_empty_arrays.py @@ -0,0 +1,50 @@ +import re +from collections.abc import Iterator +from pathlib import Path + +from conftest import WORKFLOWS, at, load_yaml, mapping, step_list, text + + +def test_unguarded_array_expansions_are_provably_non_empty() -> None: + # Bash 3.2, the /bin/bash of macOS runners, aborts on "${arr[@]}" of an empty array under set -u. Bash 5 does + # not, so CI only sees this through the scan. Each allowed name is an array that cannot be empty where it expands. + expected = { + (".github/workflows/check.yaml", "options"): "starts with --exclude-newer", + ("check-nab-exceptions/action.yml", "files"): "starts with pyproject.toml", + ("install-locked-tools/action.yml", "groups"): "filled only when the selection found a group", + ("prepare-towncrier/action.yml", "args"): "set to three options before the call", + ("publish-github-release/action.yml", "request"): "starts with two -f pairs", + ("publish-post-release-pr/action.yml", "paths"): "checked non-empty before any other use", + ("refresh-release-lock/action.yml", "paths"): "starts with pylock.toml", + ("refresh-release-lock/action.yml", "requirements"): "set to one requirement in every mode", + ("refresh-release-lock/action.yml", "selected_groups"): "used only inside the non-empty check", + ("render-release-notes/action.yml", "args"): "starts with three options", + ("stage-jetbrains-marketplace/action.yml", "args"): "starts with the marketplace subcommand", + ("tools/install_live_tools.sh", "options"): "starts with --exclude-newer", + ("tools/install_live_tools.sh", "overrides"): "expanded only inside the :+ alternative", + } + found = {(path, name) for path, script in _run_scripts() for name in _unguarded_names(script)} + + assert found == set(expected) + + +def _run_scripts() -> Iterator[tuple[str, str]]: + root = Path(__file__).resolve().parents[2] + for action in sorted(root.glob("*/action.yml")): + for step in step_list(at(load_yaml(action), "runs", "steps")): + if "run" in step: + yield action.relative_to(root).as_posix(), text(step["run"]) + for script in sorted((root / "tools").glob("*.sh")): + yield script.relative_to(root).as_posix(), script.read_text() + for workflow in sorted(WORKFLOWS.glob("*.y*ml")): + for job_value in mapping(at(load_yaml(workflow), "jobs")).values(): + job = mapping(job_value) + if "${{" in text(job.get("runs-on", "")) and "steps" in job: + for step in step_list(job["steps"]): + if "run" in step: + yield workflow.relative_to(root).as_posix(), text(step["run"]) + + +def _unguarded_names(script: str) -> set[str]: + without_guards = re.sub(r'\$\{(\w+)\[[@*]\]\+"\$\{\1\[[@*]\]\}"\}', "", script) + return {match.group(1) for match in re.finditer(r"\$\{(\w+)\[[@*]\]\}", without_guards)} diff --git a/tests/check_workflow/test_check.py b/tests/check_workflow/test_check.py index bd24653..786f26e 100644 --- a/tests/check_workflow/test_check.py +++ b/tests/check_workflow/test_check.py @@ -179,6 +179,13 @@ def test_gate_requires_test_through_required_checks() -> None: + ["--exclude-newer", "7 days", "tox", "--with", "tox-uv"], id="default", ), + pytest.param( + "", + "", + ["tool", "install", "--python-preference", "only-managed", "--python", "3.15"] + + ["--exclude-newer", "7 days"], + id="empty-tox-install", + ), pytest.param( "tox --with .", "", diff --git a/validate-release-patch/action.yml b/validate-release-patch/action.yml index 9d31826..f259d17 100644 --- a/validate-release-patch/action.yml +++ b/validate-release-patch/action.yml @@ -39,7 +39,7 @@ runs: if [[ -n "$ALLOWED_PATHS" ]]; then echo "Release patch path: $status $path" matched=false - for pattern in "${patterns[@]}"; do + for pattern in ${patterns[@]+"${patterns[@]}"}; do # shellcheck disable=SC2053 # The patterns are globs, matched on purpose. if [[ "$path" == $pattern ]]; then matched=true