mirror of
https://github.com/jmcorgan/fips.git
synced 2026-10-05 11:08:25 +00:00
Lint the OpenWrt package's shell scripts on every branch, not only on trunk pushes
The OpenWrt Package workflow was the only place the package's shell scripts were linted, and it runs on trunk pushes, tags and pull requests but never on a branch push. A shellcheck finding introduced on a topic branch therefore first failed after the branch had landed, which is how all three trunks went red on the gateway init script. testing/check-shellcheck.sh now runs that pass: the scripts the package ships as POSIX sh with the workflow's exclusions, and the nak installer as bash. It also covers the maintainer scripts the workflow's list left out: the postinst and prerm that both the .ipk and the .apk package ship, and the preinst that the SDK feed Makefile ships. It fails when a shell script under the package's files or scripts directory is not on its list, so a new one cannot go unlinted. It exits 2 when shellcheck is missing or cannot read a file, 1 on a finding or a missing script, and 0 when clean. The OpenWrt Package workflow calls the guard in place of its two inline steps, keeping its install-if-missing step, so the runners cannot drift apart. ci-local.sh runs it as a static stage beside the other repository guards, and ci.yml's ci-parity job runs it on every branch push.
This commit is contained in:
Johnathan Corgan
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Executable
+144
@@ -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 <file> <shellcheck args...>: 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
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user