From 9e744d701f1426a94570e7fd04bdbd49cf198773 Mon Sep 17 00:00:00 2001 From: Joakim Persson Date: Tue, 25 Aug 2026 21:11:55 +0200 Subject: [PATCH] lint: shellcheck the repo's own shell scripts, not just workflow run: steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lint.yml has shellchecked every workflow `run:` step since the dash-vs-bash incidents, but nothing had ever pointed shellcheck at entrypoint.sh, scripts/*.sh or the extensionless tools under rootfs/usr/local/bin/. That gap is not hypothetical: the skillset repo's ci-release-watcher template shipped `echo "$json" | python3 <<'EOF' ... json.load(sys.stdin)` for two months, where the heredoc IS python's stdin (no script arg) so the load hit EOF and the function silently returned nothing. shellcheck names exactly that at severity ERROR — SC2259, "This redirection overrides piped input" — and could have named it the whole time. New step in the existing actionlint job, so no second container pull: shellcheck -S error plus bash -n over every shell file, discovered as *.sh UNION a shebang scan (the glob alone misses pi-devbox-version, devbox-skill-reconcile, dot-watch and studio-expose; a shebang scan alone would miss a sourced fragment without one). Fails loudly on a zero-file match, because a green tick over an empty set is not a check. Severity chosen by measurement, not taste: -S error is 0 findings across all 11 shell files today, so the gate is green on arrival with no cleanup, while -S warning is NOT free (19x SC2088 tilde-in-quotes in recreate-sanity-check.sh plus assorted SC2016, all intentional) and would train everyone to ignore the job — the same reasoning as the SHELLCHECK_OPTS exclusions already on the actionlint step. --- .gitea/workflows/lint.yml | 49 +++++++++++++++++++++++++++++++++++++++ CHANGELOG.md | 4 ++++ 2 files changed, 53 insertions(+) diff --git a/.gitea/workflows/lint.yml b/.gitea/workflows/lint.yml index af2242a..6a4eb1a 100644 --- a/.gitea/workflows/lint.yml +++ b/.gitea/workflows/lint.yml @@ -52,6 +52,55 @@ jobs: apt-get update apt-get install -y --no-install-recommends shellcheck python3-yaml + - name: "Shellcheck + syntax-check repository scripts (severity: error)" + # Gap being closed: everything else in this job shellchecks workflow + # `run:` steps ONLY, via actionlint. The repo's own shell scripts — + # entrypoint.sh, scripts/*.sh, and the extensionless tools under + # rootfs/usr/local/bin/ — have never been shellchecked. That exact gap + # (a sibling repo with no shell-script lint at all) is how a defect + # shipped invisibly for two months: `echo "$json" | python3 <<'EOF' + # ... json.load(sys.stdin)` cannot work — with no script argument + # python reads its SCRIPT from stdin, so the heredoc IS stdin and the + # json.load call hits EOF. shellcheck flags exactly this at severity + # ERROR (SC2259, "This redirection overrides piped input"); nothing + # ever ran it. Measured before adding this gate: `-S error` is 0 + # findings across every shell file in THIS repo today, so it is free + # to add. `-S warning` is NOT free here (19x SC2088 tilde-in-quotes in + # scripts/recreate-sanity-check.sh, plus assorted SC2016 — both + # intentional), so warning-level would train people to ignore the job; + # hence error-only, matching the SHELLCHECK_OPTS philosophy below. + # + # Discovery is *.sh UNION a shebang scan, because rootfs/usr/local/ + # bin/{pi-devbox-version,devbox-skill-reconcile,dot-watch,studio-expose} + # are shell scripts with no extension. -print0/mapfile -d '' so a path + # with a space cannot silently split, and the file count is asserted + # non-zero — a green tick over an empty file set is not a check. + run: | + # Union of two signals, because either alone misses a real case: + # a shebang scan misses a sourced fragment with no shebang, and a + # *.sh glob misses the extensionless tools in rootfs/usr/local/bin/. + # Silent skipping is precisely the failure mode this gate exists to + # prevent, so err toward over-collecting. + mapfile -d '' -t all_files < <(find . -not -path './.git/*' -type f -print0) + sh_files=() + for f in "${all_files[@]}"; do + case "$f" in *.sh) sh_files+=("$f"); continue;; esac + if head -n1 "$f" 2>/dev/null | grep -qE '^#!.*\b(bash|sh)\b'; then + sh_files+=("$f") + fi + done + echo "Checking ${#sh_files[@]} shell file(s)" + if [ "${#sh_files[@]}" -eq 0 ]; then + echo "::error::no shell files found — the shebang scan or the checkout is wrong" + exit 1 + fi + shellcheck -S error -f gcc "${sh_files[@]}" + rc=0 + for f in "${sh_files[@]}"; do + bash -n "$f" || { echo "::error file=$f::bash -n failed"; rc=1; } + done + exit "$rc" + - name: Gitea shell guard (catches the actionlint blind spot) # actionlint models GitHub Actions, where the default run shell is # bash, so it does NOT flag bash syntax in a step that merely OMITS diff --git a/CHANGELOG.md b/CHANGELOG.md index d0841d5..b948d82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ Pre-v1.0.0 tags followed the pi npm version (`v{pi_version}[letter]`). ### Added +- **CI now shellchecks the repo's own shell scripts, not just workflow `run:` steps.** `.gitea/workflows/lint.yml`'s `actionlint` job already shellchecks every workflow step, but nothing had ever pointed shellcheck at `entrypoint.sh`, `scripts/*.sh`, or the extensionless tools under `rootfs/usr/local/bin/` (`pi-devbox-version`, `devbox-skill-reconcile`, `dot-watch`, `studio-expose`). The gap is not hypothetical: a sibling repo (skillset's `ci-release-watcher` templates) shipped `echo "$json" | python3 <<'EOF' ... json.load(sys.stdin)` for two months without anyone noticing it silently returned nothing — with no script argument python reads its *script* from stdin, so the heredoc is stdin and the JSON load hits EOF. shellcheck flags exactly this at severity **error** (`SC2259`, "This redirection overrides piped input"); it had been available to catch it the whole time, just never run. + + New step in the `actionlint` job, `Shellcheck + syntax-check repository scripts`, runs `shellcheck -S error` plus `bash -n` over every shell file in the repo, **discovered by `*.sh` union a shebang scan** (neither alone suffices) so the extensionless `rootfs/usr/local/bin/*` tools are covered too. Measured before adding it: `-S error` is 0 findings across all 11 shell files today, so the gate is green on arrival with no cleanup. `-S warning` is *not* free (19× `SC2088` tilde-in-quotes in `scripts/recreate-sanity-check.sh`, plus assorted `SC2016`, both intentional here) — a warning-level gate would train people to ignore it, so it stays error-only, same reasoning as the existing `SHELLCHECK_OPTS` exclusions on the actionlint step. File-count guard included: the step fails loudly if the shebang scan matches zero files, since a green check over an empty set is not a check. + - **`MEMPALACE_VERSION` now gets the same CI audit as `PI_VERSION`** — closing the item v1.8.6 (and v1.8.5 before it) listed as "Still open". The pin was a literal string in `Dockerfile.base` with **zero** references anywhere in