diff --git a/.github/actions/shellcheck/action.yml b/.github/actions/shellcheck/action.yml index b1a25a0a..5b67dfb8 100644 --- a/.github/actions/shellcheck/action.yml +++ b/.github/actions/shellcheck/action.yml @@ -12,6 +12,21 @@ inputs: that does NOT consult .gitignore — checking exactly what is found under those roots, including untracked files. default: '' + extra-globs: + description: >- + Additional tracked shell inputs, one Git pathspec per line. Use this for + extensionless sourced files that the default *.sh/*.bash discovery cannot + see (for example, `dot_bash*`). Blank by default. Matches are always read + from Git's index, even when `paths` opts the primary discovery into a raw + filesystem walk, so ignored or generated files are not pulled in silently. + default: '' + extra-exclude-codes: + description: >- + Optional comma-separated ShellCheck codes to suppress only while checking + files selected by `extra-globs` (for example, SC1090,SC1091). This keeps a + sourced extensionless-file exception from weakening the ordinary + *.sh/*.bash lane. Requires at least one non-blank `extra-globs` entry. + default: '' rcfile: description: Path to the .shellcheckrc ruleset in the caller repo. default: .shellcheckrc @@ -52,47 +67,9 @@ runs: shell: bash env: PATHS: ${{ inputs.paths }} + EXTRA_GLOBS: ${{ inputs.extra-globs }} + EXTRA_EXCLUDE_CODES: ${{ inputs.extra-exclude-codes }} RCFILE: ${{ inputs.rcfile }} EXCLUDE: ${{ inputs.exclude }} SEVERITY: ${{ inputs.severity }} - run: | - set -euo pipefail - if [[ ! -f "$RCFILE" ]]; then - echo "::error::shellcheck: rcfile not found: $RCFILE" - exit 2 - fi - if [[ -z "${PATHS// }" ]]; then - # Git-tracked discovery (default): tracked *.sh/*.bash only, so ignored - # or generated scripts in a dirty tree are never gated. NUL-delimited so - # any path is safe; ls-files output is already sorted. - mapfile -d '' -t files < <(git ls-files -z -- '*.sh' '*.bash') - else - # $PATHS unquoted so multiple roots word-split. Explicit roots opt into - # a raw filesystem walk that does not consult .gitignore. - mapfile -t files < <(find $PATHS -type f \( -name '*.sh' -o -name '*.bash' \) -not -path '*/.git/*' | sort) - fi - # Keep only on-disk files: a sparse checkout leaves tracked-but-absent - # (skip-worktree) entries that ShellCheck cannot open. - present=() - for f in ${files[@]+"${files[@]}"}; do [[ -f "$f" ]] && present+=("$f"); done - files=(${present[@]+"${present[@]}"}) - # Drop excluded substrings (space-separated, fixed-string match). The - # outer guard plus the per-iteration break keep an emptied list from - # feeding printf a lone newline, which grep -vF would pass back as a - # spurious empty-string element (length 1, not 0). - if [[ ${#files[@]} -gt 0 ]]; then - for sub in $EXCLUDE; do - mapfile -t files < <(printf '%s\n' "${files[@]}" | grep -vF -- "$sub" || true) - [[ ${#files[@]} -eq 0 ]] && break - done - fi - if [[ ${#files[@]} -eq 0 ]]; then - echo 'No shell scripts to check.' - exit 0 - fi - printf 'Checking %d file(s):\n' "${#files[@]}" - printf ' %s\n' "${files[@]}" - args=(--rcfile="$RCFILE") - # Empty severity omits the flag, leaving ShellCheck's own default (style). - if [[ -n "${SEVERITY// }" ]]; then args+=(--severity="$SEVERITY"); fi - shellcheck "${args[@]}" "${files[@]}" + run: bash "$GITHUB_ACTION_PATH/run.sh" diff --git a/.github/actions/shellcheck/run.sh b/.github/actions/shellcheck/run.sh new file mode 100755 index 00000000..bee71d7b --- /dev/null +++ b/.github/actions/shellcheck/run.sh @@ -0,0 +1,143 @@ +#!/usr/bin/env bash +# shellcheck shell=bash +set -euo pipefail + +paths="${PATHS:-}" +extra_globs="${EXTRA_GLOBS:-}" +extra_exclude_codes="${EXTRA_EXCLUDE_CODES:-}" +rcfile="${RCFILE:-.shellcheckrc}" +exclude="${EXCLUDE:-}" +severity="${SEVERITY:-}" + +if [[ ! -f "$rcfile" ]]; then + echo "::error::shellcheck: rcfile not found: $rcfile" + exit 2 +fi + +# Parse one pathspec per line instead of word-splitting. Git pathspecs can +# contain spaces, and keeping each caller-supplied line as one argv entry also +# prevents shell metacharacters from being evaluated by this action. +extra_pathspecs=() +while IFS= read -r pathspec || [[ -n "$pathspec" ]]; do + pathspec="${pathspec%$'\r'}" + [[ -z "${pathspec//[[:space:]]/}" ]] || extra_pathspecs+=("$pathspec") +done <<<"$extra_globs" + +if [[ -n "${extra_exclude_codes//[[:space:]]/}" && ${#extra_pathspecs[@]} -eq 0 ]]; then + echo '::error::shellcheck: extra-exclude-codes requires at least one extra-globs entry.' + exit 2 +fi +if [[ -n "${extra_exclude_codes//[[:space:]]/}" && ! "$extra_exclude_codes" =~ ^SC[0-9]{4}(,SC[0-9]{4})*$ ]]; then + echo '::error::shellcheck: extra-exclude-codes must be comma-separated SC codes (for example, SC1090,SC1091).' + exit 2 +fi + +discover_tracked_files() { + local label="$1" output + shift + output="$(mktemp)" + if ! git ls-files -z -- "$@" >"$output"; then + rm -f -- "$output" + echo "::error::shellcheck: Git-tracked $label discovery failed." + return 2 + fi + git_files=() + mapfile -d '' -t git_files <"$output" + rm -f -- "$output" +} + +normal_files=() +if [[ -z "${paths//[[:space:]]/}" ]]; then + # Git-tracked discovery (default): tracked *.sh/*.bash only, so ignored or + # generated scripts in a dirty tree are never gated. NUL-delimited so any + # path is safe; ls-files output is already sorted. + discover_tracked_files primary '*.sh' '*.bash' + normal_files=("${git_files[@]}") +else + # Space-separated roots preserve the existing input contract. Explicit roots + # opt into a raw filesystem walk that does not consult .gitignore. + read -r -a path_roots <<<"$paths" + mapfile -d '' -t normal_files < <( + find "${path_roots[@]}" -type f \( -name '*.sh' -o -name '*.bash' \) \ + -not -path '*/.git/*' -print0 | sort -z + ) +fi + +extra_files=() +if [[ ${#extra_pathspecs[@]} -gt 0 ]]; then + # Extra inputs deliberately remain Git-tracked even when primary discovery + # uses raw roots. `--` keeps a leading dash in a pathspec from becoming an + # option; the quoted array prevents shell expansion or code execution. + discover_tracked_files extra "${extra_pathspecs[@]}" + extra_files=("${git_files[@]}") +fi + +filter_files() { + local array_name="$1" file substring + local -a kept=() + local -n candidates="$array_name" + + for file in ${candidates[@]+"${candidates[@]}"}; do + # Sparse checkouts can leave tracked, skip-worktree entries absent on disk. + [[ -f "$file" ]] || continue + for substring in $exclude; do + [[ "$file" == *"$substring"* ]] && continue 2 + done + kept+=("$file") + done + candidates=(${kept[@]+"${kept[@]}"}) +} + +filter_files normal_files +filter_files extra_files + +# A path selected by both lanes stays in the ordinary lane. That preserves the +# existing strict result instead of weakening a normal *.sh/*.bash file with an +# exception intended only for extensionless extras. Also deduplicate repeated +# or overlapping extra pathspecs. +declare -A normal_seen=() extra_seen=() +for file in ${normal_files[@]+"${normal_files[@]}"}; do + normal_seen["$file"]=1 +done +deduplicated_extra_files=() +for file in ${extra_files[@]+"${extra_files[@]}"}; do + [[ -n "${normal_seen[$file]+present}" || -n "${extra_seen[$file]+present}" ]] && continue + extra_seen["$file"]=1 + deduplicated_extra_files+=("$file") +done +extra_files=(${deduplicated_extra_files[@]+"${deduplicated_extra_files[@]}"}) + +if [[ ${#normal_files[@]} -eq 0 && ${#extra_files[@]} -eq 0 ]]; then + echo 'No shell scripts to check.' + exit 0 +fi + +args=(--rcfile="$rcfile") +# Empty severity omits the flag, leaving ShellCheck's own default (style). +if [[ -n "${severity//[[:space:]]/}" ]]; then + args+=(--severity="$severity") +fi + +status=0 +if [[ ${#normal_files[@]} -gt 0 ]]; then + printf 'Checking %d standard shell file(s):\n' "${#normal_files[@]}" + printf ' %s\n' "${normal_files[@]}" + shellcheck "${args[@]}" "${normal_files[@]}" || status=$? +fi + +if [[ ${#extra_files[@]} -gt 0 ]]; then + printf 'Checking %d extra shell file(s):\n' "${#extra_files[@]}" + printf ' %s\n' "${extra_files[@]}" + extra_args=("${args[@]}") + if [[ -n "${extra_exclude_codes//[[:space:]]/}" ]]; then + extra_args+=(--exclude="$extra_exclude_codes") + fi + extra_status=0 + shellcheck "${extra_args[@]}" "${extra_files[@]}" || extra_status=$? + # ShellCheck reserves 1 for completed scans with findings and 2-4 for + # processing/invocation errors. Keep the more severe result if the two lanes + # differ instead of masking an operational failure behind a finding code. + ((extra_status <= status)) || status=$extra_status +fi + +exit "$status" diff --git a/.github/actions/shellcheck/run.test.sh b/.github/actions/shellcheck/run.test.sh new file mode 100755 index 00000000..097781b5 --- /dev/null +++ b/.github/actions/shellcheck/run.test.sh @@ -0,0 +1,184 @@ +#!/usr/bin/env bash +# shellcheck shell=bash +set -euo pipefail + +action_directory="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +temporary_directory="$(mktemp -d)" +trap 'rm -rf -- "$temporary_directory"' EXIT +repository="$temporary_directory/repository with spaces" +fake_bin="$temporary_directory/bin" +captures="$temporary_directory/captures" +mkdir -p -- "$repository/empty" "$repository/nested" "$repository/raw" "$fake_bin" "$captures" + +cat >"$fake_bin/shellcheck" <<'FAKE' +#!/usr/bin/env bash +set -euo pipefail +count=0 +[[ ! -f "$CAPTURE_DIR/count" ]] || read -r count <"$CAPTURE_DIR/count" +count=$((count + 1)) +printf '%s\n' "$count" >"$CAPTURE_DIR/count" +printf '%s\0' "$@" >"$CAPTURE_DIR/$count.args" +exit "${FAKE_STATUS:-0}" +FAKE +chmod +x "$fake_bin/shellcheck" + +printf 'shell=bash\n' >"$repository/.shellcheckrc" +printf '#!/usr/bin/env bash\ntrue\n' >"$repository/script.sh" +printf '#!/usr/bin/env bash\ntrue\n' >"$repository/nested/tool.bash" +printf 'source ~/.bashrc.local\n' >"$repository/dot_bashrc" +printf 'source ~/.bash_profile.local\n' >"$repository/dot bash profile" +printf '#!/usr/bin/env bash\ntrue\n' >"$repository/raw/untracked.sh" +printf 'ignored*\nraw/\n' >"$repository/.gitignore" +printf 'source ignored\n' >"$repository/ignored_extensionless" + +git -C "$repository" init -q +git -C "$repository" -c core.autocrlf=false add \ + .shellcheckrc script.sh nested/tool.bash dot_bashrc 'dot bash profile' .gitignore + +reset_captures() { + rm -f -- "$captures"/* +} + +run_action() { + local expected_status="$1" + shift + local actual_status output + reset_captures + set +e + output="$( + cd "$repository" + env \ + CAPTURE_DIR="$captures" \ + EXCLUDE='' \ + EXTRA_EXCLUDE_CODES='' \ + EXTRA_GLOBS='' \ + FAKE_STATUS=0 \ + PATH="$fake_bin:$PATH" \ + PATHS='' \ + RCFILE=.shellcheckrc \ + SEVERITY='' \ + "$@" \ + bash "$action_directory/run.sh" 2>&1 + )" + actual_status=$? + set -e + if [[ "$actual_status" != "$expected_status" ]]; then + printf 'FAIL: expected status %s, got %s\n%s\n' "$expected_status" "$actual_status" "$output" >&2 + return 1 + fi + ACTION_OUTPUT="$output" +} + +load_args() { + local invocation="$1" + captured_args=() + mapfile -d '' -t captured_args <"$captures/$invocation.args" +} + +assert_contains() { + local name="$1" expected="$2" + shift 2 + local actual + for actual in "$@"; do + [[ "$actual" != "$expected" ]] || { + printf 'PASS: %s\n' "$name" + return 0 + } + done + printf 'FAIL: %s did not contain %q\n' "$name" "$expected" >&2 + return 1 +} + +assert_not_contains() { + local name="$1" unexpected="$2" + shift 2 + local actual + for actual in "$@"; do + [[ "$actual" != "$unexpected" ]] || { + printf 'FAIL: %s unexpectedly contained %q\n' "$name" "$unexpected" >&2 + return 1 + } + done + printf 'PASS: %s\n' "$name" +} + +run_action 0 +load_args 1 +[[ ! -e "$captures/2.args" ]] +assert_contains 'default discovery keeps tracked .sh' script.sh "${captured_args[@]}" +assert_contains 'default discovery keeps tracked .bash' nested/tool.bash "${captured_args[@]}" +assert_not_contains 'default discovery skips extensionless files' dot_bashrc "${captured_args[@]}" +assert_not_contains 'default discovery skips raw untracked files' raw/untracked.sh "${captured_args[@]}" + +run_action 0 \ + EXTRA_GLOBS=$'dot_bash*\ndot bash*' \ + EXTRA_EXCLUDE_CODES=SC1090,SC1091 \ + SEVERITY=warning +load_args 1 +assert_contains 'standard lane retains configured severity' --severity=warning "${captured_args[@]}" +assert_not_contains 'standard lane does not inherit extra suppressions' --exclude=SC1090,SC1091 "${captured_args[@]}" +load_args 2 +assert_contains 'extra glob selects extensionless file' dot_bashrc "${captured_args[@]}" +assert_contains 'one pathspec line preserves spaces' 'dot bash profile' "${captured_args[@]}" +assert_contains 'extra lane receives scoped suppressions' --exclude=SC1090,SC1091 "${captured_args[@]}" + +run_action 0 \ + PATHS=empty \ + EXTRA_GLOBS=dot_bashrc \ + EXTRA_EXCLUDE_CODES=SC1090,SC1091 +load_args 1 +[[ ! -e "$captures/2.args" ]] +assert_contains 'extra-only discovery checks an extensionless file' dot_bashrc "${captured_args[@]}" +assert_contains 'extra-only discovery keeps scoped suppressions' --exclude=SC1090,SC1091 "${captured_args[@]}" + +run_action 0 PATHS=empty EXTRA_GLOBS=does-not-match +[[ ! -e "$captures/1.args" ]] +grep -F 'No shell scripts to check.' <<<"$ACTION_OUTPUT" >/dev/null +printf 'PASS: empty primary and extra discovery exits cleanly\n' + +run_action 0 EXTRA_GLOBS=$'*.sh\ndot_bash*' +load_args 1 +assert_contains 'overlap keeps normal file in strict lane' script.sh "${captured_args[@]}" +load_args 2 +assert_not_contains 'overlap removes normal file from extra lane' script.sh "${captured_args[@]}" +assert_contains 'overlap still keeps extensionless extra' dot_bashrc "${captured_args[@]}" + +run_action 0 EXTRA_GLOBS='ignored*' +load_args 1 +[[ ! -e "$captures/2.args" ]] +assert_not_contains 'extra discovery does not include ignored untracked files' ignored_extensionless "${captured_args[@]}" + +run_action 0 PATHS=raw +load_args 1 +assert_contains 'explicit roots preserve raw untracked discovery' raw/untracked.sh "${captured_args[@]}" + +run_action 0 \ + EXCLUDE=profile \ + EXTRA_GLOBS=$'dot_bash*\ndot bash*' +load_args 2 +assert_contains 'path exclusion retains other extras' dot_bashrc "${captured_args[@]}" +assert_not_contains 'path exclusion also filters extras' 'dot bash profile' "${captured_args[@]}" + +run_action 2 EXTRA_EXCLUDE_CODES=SC1090,SC1091 +[[ ! -e "$captures/1.args" ]] +grep -F 'extra-exclude-codes requires at least one extra-globs entry' <<<"$ACTION_OUTPUT" >/dev/null +printf 'PASS: orphaned extra suppressions fail closed\n' + +run_action 2 EXTRA_GLOBS=dot_bashrc EXTRA_EXCLUDE_CODES='SC1090, SC1091' +[[ ! -e "$captures/1.args" ]] +grep -F 'must be comma-separated SC codes' <<<"$ACTION_OUTPUT" >/dev/null +printf 'PASS: malformed suppression list fails closed\n' + +run_action 2 EXTRA_GLOBS=':(attr' +[[ ! -e "$captures/1.args" ]] +grep -F 'Git-tracked extra discovery failed' <<<"$ACTION_OUTPUT" >/dev/null +printf 'PASS: invalid extra pathspec fails closed before ShellCheck\n' + +run_action 1 EXTRA_GLOBS=dot_bashrc FAKE_STATUS=1 +[[ -e "$captures/1.args" && -e "$captures/2.args" ]] +printf 'PASS: ShellCheck findings propagate after both lanes run\n' + +grep -F "EXTRA_GLOBS: \${{ inputs.extra-globs }}" "$action_directory/action.yml" >/dev/null +grep -F "EXTRA_EXCLUDE_CODES: \${{ inputs.extra-exclude-codes }}" "$action_directory/action.yml" >/dev/null +grep -F "run: bash \"\$GITHUB_ACTION_PATH/run.sh\"" "$action_directory/action.yml" >/dev/null +printf 'PASS: action metadata forwards the new inputs to the tested runner\n' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cec64a2b..bd6baa02 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -313,6 +313,8 @@ jobs: persist-credentials: false - name: Lint shell scripts uses: ./.github/actions/shellcheck + - name: Test ShellCheck discovery and scoped exclusions + run: bash .github/actions/shellcheck/run.test.sh shfmt: runs-on: ubuntu-24.04 diff --git a/README.md b/README.md index 22fd15db..02cd49b3 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,10 @@ checkout of this repo. (Public is required because a public consumer such as - `.github/actions/markdown` — markdownlint-cli2 over the repo's markdown. - `.github/actions/shellcheck` — ShellCheck over the repo's shell scripts - (installs a pinned, checksum-verified binary). + (installs a pinned, checksum-verified binary). Its default discovery remains + tracked `*.sh`/`*.bash`; `extra-globs` adds tracked extensionless inputs as + newline-delimited Git pathspecs, with optional `extra-exclude-codes` scoped + only to that extra lane so ordinary scripts keep the stricter result. - `.github/actions/shfmt` — shfmt formatting check over the repo's shell scripts, driven by the caller's `.editorconfig` (installs a pinned, checksum-verified binary).