diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f93d144c..daf8c116 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,6 +79,17 @@ jobs: run: bash testing/check-comment-refs.sh - name: Check no non-test code uses std 64-bit atomics run: python3 testing/check-portable-atomics.py + # The OpenWrt Package workflow runs this too, but only on trunk pushes, + # tags and pull requests; here a branch push sees a finding first. + # Kept in step with ci-local.sh's run_shellcheck by hand. + - name: Install shellcheck (if missing) + run: | + if ! command -v shellcheck >/dev/null 2>&1; then + sudo apt-get update && sudo apt-get install -y --no-install-recommends shellcheck + fi + shellcheck --version + - name: Check the OpenWrt package's shell scripts with shellcheck + run: bash testing/check-shellcheck.sh # Hermetic: synthetic ping functions, no containers, ~45s. Lives beside # the other two so both runners gate on it identically — putting it in # only one would create exactly the drift check-ci-parity.sh exists to diff --git a/.github/workflows/package-openwrt.yml b/.github/workflows/package-openwrt.yml index c9945dd6..880ca962 100644 --- a/.github/workflows/package-openwrt.yml +++ b/.github/workflows/package-openwrt.yml @@ -341,52 +341,12 @@ jobs: fi shellcheck --version - # Its own step, and its own shell dialect. The shipped-scripts lint below - # runs --shell=sh with the OpenWrt rc.common exclude set, which misfires - # on a bash script; install-nak.sh is also not shipped in the package. - - name: Lint install-nak.sh + # The scripts this package ships, as sh, and install-nak.sh, as bash. + # The guard is the one copy of the lint; ci.yml and ci-local.sh run it + # too, so a branch push sees a finding before it reaches a trunk. + - name: Lint shell scripts shell: bash - run: shellcheck --shell=bash .github/scripts/install-nak.sh - - - name: Lint shipped shell scripts - shell: bash - run: | - set -euo pipefail - FILES_DIR=packaging/openwrt-ipk/files - # Scripts shipped inside the .ipk. The init scripts use the OpenWrt - # `#!/bin/sh /etc/rc.common` shebang; tell shellcheck to treat them - # as POSIX sh and silence the unrecognized-shebang warning (SC1008). - # SC2317 (unreachable command) fires on rc.common's externally-invoked - # start_service/stop_service/reload_service hooks. - TARGETS=( - "$FILES_DIR/etc/init.d/fips" - "$FILES_DIR/etc/init.d/fips-gateway" - "$FILES_DIR/etc/fips/firewall.sh" - "$FILES_DIR/etc/hotplug.d/net/99-fips" - "$FILES_DIR/etc/uci-defaults/90-fips-setup" - "$FILES_DIR/usr/bin/fips-mesh-setup" - "$FILES_DIR/usr/bin/fips-ap-setup" - ) - fail=0 - for f in "${TARGETS[@]}"; do - if [ ! -f "$f" ]; then - echo "FAIL: missing $f" - fail=1 - continue - fi - echo "==> shellcheck $f" - if shellcheck --shell=sh --exclude=SC1008,SC2317,SC2034,SC3043,SC2086,SC2089,SC2090 "$f"; then - echo " PASS" - else - echo " FAIL" - fail=1 - fi - done - if [ "$fail" -ne 0 ]; then - echo "shellcheck FAILED" - exit 1 - fi - echo "shellcheck PASS (${#TARGETS[@]} scripts)" + run: bash testing/check-shellcheck.sh - name: Sysctl drop-in syntax check shell: bash diff --git a/testing/check-shellcheck.sh b/testing/check-shellcheck.sh new file mode 100755 index 00000000..2967a91b --- /dev/null +++ b/testing/check-shellcheck.sh @@ -0,0 +1,144 @@ +#!/bin/bash +# ── OpenWrt shell-script lint guard ───────────────────────────────────────── +# Runs shellcheck over the shell scripts the OpenWrt packages ship, and over +# .github/scripts/install-nak.sh, which the OpenWrt Package workflow runs to +# fetch its publishing tool. +# +# This is the one copy of that lint. The OpenWrt Package workflow calls it +# after building each .ipk, and ci.yml and ci-local.sh call it too, because +# that workflow runs only on trunk pushes, tags and pull requests: without the +# other two, a finding in a script edited on a topic branch first shows up +# after the branch has reached a trunk. +# +# What is checked, and how: +# * Every script under packaging/openwrt-ipk/files/ and the maintainer +# scripts under packaging/openwrt-ipk/scripts/, as POSIX sh. Both the .ipk +# and the .apk package take their payload and maintainer scripts from these +# two directories (build-apk.sh wraps the maintainer scripts with one +# header line, which is not linted separately); the SDK feed Makefile ships +# preinst as well. On a router they run under busybox ash. +# * install-nak.sh as bash, with no exclusions. It is a CI script, not a +# shipped one, and the sh exclusion set below misfires on bash. +# +# The sh exclusions, with reason: +# SC1008 the init scripts' `#!/bin/sh /etc/rc.common` shebang, an +# interpreter line the linter does not recognise. +# SC2317 rc.common's start_service/stop_service/reload_service hooks, +# which nothing in the file itself calls. +# SC2034 the init scripts' USE_PROCD, START, STOP, EXTRA_COMMANDS and +# EXTRA_HELP, which rc.common reads rather than the script. +# SC3043 `local`, which POSIX leaves undefined and ash supports. +# SC2086, SC2089, SC2090 firewall.sh builds an nft match, quotes included, +# in one variable and relies on word splitting to pass it as +# separate arguments; nft parses the quotes itself. +# +# Every sh-family script in those two directories must be on the list below. A +# new one that is not fails the guard, so a script added to the package is not +# silently left unlinted. +# +# Exit 0 = clean. Exit 1 = a finding, a listed script missing, or a shipped +# script not on the list. Exit 2 = the guard could not run (shellcheck or git +# missing, or shellcheck could not process a file); never treated as a pass. +# ───────────────────────────────────────────────────────────────────────────── +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +cd "$PROJECT_ROOT" || { echo "check-shellcheck: cannot cd to $PROJECT_ROOT" >&2; exit 2; } + +IPK=packaging/openwrt-ipk +SH_EXCLUDE=SC1008,SC2317,SC2034,SC3043,SC2086,SC2089,SC2090 + +SH_TARGETS=( + "$IPK/files/etc/init.d/fips" + "$IPK/files/etc/init.d/fips-gateway" + "$IPK/files/etc/fips/firewall.sh" + "$IPK/files/etc/hotplug.d/net/99-fips" + "$IPK/files/etc/uci-defaults/90-fips-setup" + "$IPK/files/usr/bin/fips-mesh-setup" + "$IPK/files/usr/bin/fips-ap-setup" + "$IPK/scripts/preinst" + "$IPK/scripts/postinst" + "$IPK/scripts/prerm" +) +BASH_TARGETS=( + ".github/scripts/install-nak.sh" +) + +if ! command -v shellcheck >/dev/null 2>&1; then + echo "check-shellcheck: shellcheck not found; cannot lint the shell scripts" >&2 + echo "check-shellcheck: install it with 'apt-get install shellcheck'" >&2 + exit 2 +fi +if ! command -v git >/dev/null 2>&1; then + echo "check-shellcheck: git not found; cannot list the shipped scripts" >&2 + exit 2 +fi + +shellcheck --version | sed -n 's/^version: /check-shellcheck: shellcheck /p' + +findings=0 +broken=0 + +# ── Completeness: every shipped sh-family script is on the list ───────────── +shipped=$(git ls-files -- "$IPK/files" "$IPK/scripts") || { + echo "check-shellcheck: git ls-files failed, refusing to pass" >&2 + exit 2 +} +if [[ -z "$shipped" ]]; then + echo "check-shellcheck: git ls-files found nothing under $IPK, refusing to pass" >&2 + exit 2 +fi +while IFS= read -r f; do + # A tracked file deleted from the working tree: if listed, the lint below + # reports it missing; if not, there is nothing to ship. + [[ -f "$f" ]] || continue + head -n 1 "$f" | grep -qE '^#![[:space:]]*[^[:space:]]*/(env[[:space:]]+)?(ba|a|da)?sh([[:space:]]|$)' || continue + listed=0 + for t in "${SH_TARGETS[@]}"; do + [[ "$t" == "$f" ]] && { listed=1; break; } + done + if [[ $listed -eq 0 ]]; then + echo "FAIL: $f is a shipped shell script missing from SH_TARGETS in $0" + findings=1 + fi +done <<< "$shipped" + +# ── Lint ───────────────────────────────────────────────────────────────────── +lint() { + # lint : one file; sets findings or broken. + local f="$1" rc=0 + shift + if [[ ! -f "$f" ]]; then + echo "FAIL: missing $f" + findings=1 + return 0 + fi + echo "==> shellcheck $* $f" + shellcheck "$@" "$f" || rc=$? + case $rc in + 0) echo " PASS" ;; + 1) echo " FAIL"; findings=1 ;; + *) echo " shellcheck exited $rc: could not check $f"; broken=1 ;; + esac + return 0 +} + +for f in "${SH_TARGETS[@]}"; do + lint "$f" --shell=sh --exclude="$SH_EXCLUDE" +done +for f in "${BASH_TARGETS[@]}"; do + lint "$f" --shell=bash +done + +total=$(( ${#SH_TARGETS[@]} + ${#BASH_TARGETS[@]} )) +if [[ $broken -ne 0 ]]; then + echo "shellcheck could not check every script; refusing to pass" + exit 2 +fi +if [[ $findings -ne 0 ]]; then + echo "shellcheck FAILED" + exit 1 +fi +echo "shellcheck PASS ($total scripts: ${#SH_TARGETS[@]} as sh, ${#BASH_TARGETS[@]} as bash)" +exit 0 diff --git a/testing/ci-local.sh b/testing/ci-local.sh index 7a0aec69..f0831825 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -1564,6 +1564,17 @@ run_portable_atomics() { record "portable-atomics" $rc } +# The shell scripts the OpenWrt packages ship, and the nak installer. The +# OpenWrt Package workflow lints them on GitHub, but only for trunk pushes, +# tags and pull requests, so this is where a branch first sees a finding. +# Mirrored in ci.yml's ci-parity job by hand. Static, about a second. +run_shellcheck() { + local rc=0 + info "[shellcheck] Linting the OpenWrt package's shell scripts" + bash "$SCRIPT_DIR/check-shellcheck.sh" || rc=$? + record "shellcheck" $rc +} + # Every daemon log string a test matches on must still be emitted by src/. # A stale one does not fail — it stops observing, and an expect-zero assertion # built on it then passes for the wrong reason. @@ -1647,6 +1658,7 @@ main() { run_action_pins run_comment_refs run_portable_atomics + run_shellcheck run_wait_converge run_deb_version run_nextest_flaky