ci(shell): gate every tracked shell script on bash -n + ShellCheck - #76
Merged
Conversation
Closes the v0.2 audit's "no shellcheck / bash -n anywhere" item.
This repo runs on shell guards — check-platform-floors.sh,
check-review-notes.sh, check-release-commit.sh, check-demo-account.sh,
verify-floor-runtime.sh, build-tailscalekit.sh, validate-xcframework.sh
— and none of them was ever linted, nor was anything checking that they
parse. CI builds the app; it does not execute the ship path or source
the guards, so a PR that broke one outright merged green.
Seven warning-level findings on the first run, all fixed:
- SC2164 x6, in six guards: `cd "$(dirname "$0")/.."` with no
`|| exit`. This is the fail-open mode — cd fails, the script keeps
running in the wrong directory, and then checks and PASSES files that
are not the ones it exists to check.
- SC2046 x1 in local-release-check.sh's macOS archive invocation.
Rewritten to the ${ARR[@]+"${ARR[@]}"} idiom the two lines above it
already use, so the "pass nothing" branch is zero arguments instead of
one empty one.
Severity capped at warning. The 21 info/style findings are printed each
run but never fail; the count differs between a local run and the runner
because the ShellCheck versions differ, which is exactly why only
warning is gated.
Authored upstream (apple-shipkit#285, merged 0a47db1) and cherry-picked:
ci/check-shell.sh is scaffolding every fork needs, and ci/lib/ is pinned
byte-for-byte across repos by ci/lib/SHA256SUMS — covered via
`shellcheck -s bash` rather than exempted, so no downstream re-pin.
Mutation-verified upstream: a syntax break fails it, reintroducing the
cd bug fails it, and removing shellcheck from PATH fails it rather than
skipping.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the v0.2 audit's "no shellcheck / bash -n anywhere" item.
The hole
This repo runs on shell guards —
check-platform-floors.sh,check-review-notes.sh,check-release-commit.sh,check-demo-account.sh,verify-floor-runtime.sh,build-tailscalekit.sh,validate-xcframework.sh. None of them was ever linted, and nothing checked that any of them parses.CI builds the app; it does not execute the ship path or source the guards. A PR that broke one outright would merge green and surface later, on a release.
What it found
Seven warning-level findings on the first run, all fixed here:
check-app-icon.sh,check-demo-account.sh,check-platform-floors.sh,check-release-commit.sh,check-review-notes.sh,verify-floor-runtime.shcd "$(dirname "$0")/.."with no|| exitci/local-release-check.sh$($PATCH_MACOS_PLIST && echo "" || echo …)in the macOS archive callAll six SC2164 are in guards, which is what makes them worth fixing rather than silencing. If
cdfails the script does not stop — it keeps running in the wrong directory and then checks, and passes, files that are not the ones it exists to check. That is the fail-open mode this repo has already been bitten by: an inertcheck-platform-floors.shis exactly how the macOS floor stayed wrong in two of three files.The SC2046 fix uses the
${ARR[@]+"${ARR[@]}"}idiom the two lines directly above it already use, so the "pass nothing" branch is genuinely zero arguments instead of one empty one thatxcodebuildwould see as a stray empty setting.Deliberate calls
Capped at
warning. The 21 remaininginfo/stylefindings print every run for visibility but never fail — dominated by SC2012 and SC1091, neither a defect here. The count differs between a local run (21) and the runner (22 upstream) because the ShellCheck versions differ; that variance is precisely why onlywarningis gated and the rest is advisory.ci/lib/is covered, not exempted — viashellcheck -s bash. Those files are pinned byte-for-byte byci/lib/SHA256SUMSacross this repo and the template, so adding a shell directive to them would force a re-pin here and in every other consumer for the sake of a comment.Provenance
Authored upstream in apple-shipkit (#285, merged
0a47db1) and cherry-picked, sinceci/check-shell.shis scaffolding every fork needs. Upstream carried three findings this fork does not have (SC1087 ×3 in theIdentity.xcconfigcode, absent here) and one this fork does not carry (SC2088).Mutation-verified upstream, because a gate that cannot fail is not evidence:
cdbugshellcheckfromPATHIt also caught a defect in its own header on first run: prose wrapped so a line began
# shellcheck, which ShellCheck parses as a malformed directive.Wired as pr.yml's
shell scriptsjob (ubuntu, seconds, no paths filter) and as the first preflight inci/local-check.sh— ahead of the other guards, since it is the one that checks they can run at all.shellcheckadded to the Brewfile.Local: gate passes 31/31; parser 9/9,
Sh.stream10/10, build number 11/11, demo-account 14/14.