From 578709566ea0cdfe8cf1148f406b5e44f383a3d2 Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 19 Oct 2026 20:11:56 -0700 Subject: [PATCH 1/5] ci: ShellCheck tracked scripts and generated hook launchers scripts/ci/shellcheck.sh checks every tracked shell script (by .sh extension or sh/bash shebang) with shellcheck -x, then installs the hook launchers into a throwaway repository and checks those too, since they exist only inside an installed hooks directory. The quality job runs it. Verified locally: clean on the current tree; exits 1 when a violating script is added. ReplaceRange proof locks refreshed for the new ci.yml bytes (toolchain-lock file hash; build-lock toolchain hash, closure entry, and closure digest). --- .github/workflows/ci.yml | 3 ++ CHANGELOG.md | 5 +++ edict/replace-range/edict.build-lock.json | 8 ++-- edict/replace-range/edict.toolchain-lock.json | 2 +- scripts/ci/shellcheck.sh | 37 +++++++++++++++++++ 5 files changed, 50 insertions(+), 5 deletions(-) create mode 100755 scripts/ci/shellcheck.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f93cb306..004f78f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -147,6 +147,9 @@ jobs: - name: Run quality gate run: npm run quality + - name: ShellCheck scripts and generated hook launchers + run: scripts/ci/shellcheck.sh + release-gate: name: release-gate / jedit-echo needs: [ plan ] diff --git a/CHANGELOG.md b/CHANGELOG.md index f94171a7..0cf40081 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +- CI now runs ShellCheck (`scripts/ci/shellcheck.sh`, in the `quality` job) over + every tracked shell script and over the pre-commit and pre-push launchers that + `scripts/install-hooks.sh` generates, installed into a throwaway repository. + The ReplaceRange proof locks were refreshed for the changed workflow bytes. + - Replaced the leftover Husky hook path with plain shell hooks in `scripts/hooks/`: pre-commit runs the quality gate and a TypeScript typecheck, and pre-push runs the full test suite. Both share diff --git a/edict/replace-range/edict.build-lock.json b/edict/replace-range/edict.build-lock.json index 17cde352..04c2edbf 100644 --- a/edict/replace-range/edict.build-lock.json +++ b/edict/replace-range/edict.build-lock.json @@ -2,10 +2,10 @@ "schema": "jedit.edict-build-lock/v1", "applicationCoordinate": "jedit.text.replace_range@1", "operationCoordinate": "jedit.text.replace_range@1.replaceRange", - "toolchainLockSha256": "8872fe4bf8559e1ca9c96f9c984ff8a7c2ecee33bb357e15e5d42a3376be878c", + "toolchainLockSha256": "0a01119318d60a6dd10b17ffb68adab5e4162aa8a35828e18661fa39eedea5f6", "sourceClosure": { "schema": "jedit.edict-source-closure/v1", - "digest": "sha256:b6443e58cdb725fb09078d1709bb3e2ae0dcf994e6de233418b45c84a3f2b4b8", + "digest": "sha256:44b61622b6a109bb86cda184af31f56a7383a954d1b705189b1bd486ab7d2362", "files": [ { "path": "application/edict.application.json", @@ -17,7 +17,7 @@ }, { "path": "application/edict.toolchain-lock.json", - "sha256": "8872fe4bf8559e1ca9c96f9c984ff8a7c2ecee33bb357e15e5d42a3376be878c" + "sha256": "0a01119318d60a6dd10b17ffb68adab5e4162aa8a35828e18661fa39eedea5f6" }, { "path": "application/src/ReplaceRange.edict", @@ -81,7 +81,7 @@ }, { "path": "project/.github/workflows/ci.yml", - "sha256": "bc19286ef44ce7d8ba0fd8a3e39a8ec0313d07ed3084fbe15e25ef42d82bc8f1" + "sha256": "b85f2e43552dab43dd9d8f87daacc0c73b437fc2bde341eb3fb21fd6e2394c12" }, { "path": "project/package.json", diff --git a/edict/replace-range/edict.toolchain-lock.json b/edict/replace-range/edict.toolchain-lock.json index 84d78fd9..523a6976 100644 --- a/edict/replace-range/edict.toolchain-lock.json +++ b/edict/replace-range/edict.toolchain-lock.json @@ -24,7 +24,7 @@ }, { "path": ".github/workflows/ci.yml", - "sha256": "bc19286ef44ce7d8ba0fd8a3e39a8ec0313d07ed3084fbe15e25ef42d82bc8f1" + "sha256": "b85f2e43552dab43dd9d8f87daacc0c73b437fc2bde341eb3fb21fd6e2394c12" } ] }, diff --git a/scripts/ci/shellcheck.sh b/scripts/ci/shellcheck.sh new file mode 100755 index 00000000..2abc3e74 --- /dev/null +++ b/scripts/ci/shellcheck.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# ShellCheck every tracked shell script, plus the hook launchers that +# scripts/install-hooks.sh generates. The launchers exist only inside an +# installed hooks directory, so they are installed into a throwaway +# repository and checked there. +set -euo pipefail + +root="$(git rev-parse --show-toplevel)" +cd "$root" + +scripts=() +while IFS= read -r -d '' path; do + case "$path" in + *.sh) scripts+=("$path") ;; + *) + if [[ -f "$path" ]] && head -n 1 "$path" | grep -qE '^#!.*\b(bash|sh)\b'; then + scripts+=("$path") + fi + ;; + esac +done < <(git ls-files -z) + +if ((${#scripts[@]} == 0)); then + echo "shellcheck: no tracked shell scripts found" >&2 + exit 1 +fi +shellcheck -x "${scripts[@]}" + +scratch="$(mktemp -d)" +trap 'rm -rf "$scratch"' EXIT +git -C "$scratch" init -q +mkdir -p "$scratch/scripts" +cp scripts/install-hooks.sh "$scratch/scripts/" +(cd "$scratch" && GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 scripts/install-hooks.sh >/dev/null) +shellcheck "$scratch/.git/hooks/pre-commit" "$scratch/.git/hooks/pre-push" + +echo "shellcheck: ${#scripts[@]} tracked scripts and 2 generated launchers are clean" From ab0e939f18af24e3a589f418bc63c436e33049ca Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 19 Oct 2026 22:44:33 -0700 Subject: [PATCH 2/5] ci(shellcheck): match the shebang's interpreter, not any 'sh' on the line The discovery predicate matched bash or sh anywhere on the first line, so '#!/usr/bin/env python # sh' or '#!/opt/bash-tools/python' would have reached ShellCheck. shebang_interpreter now extracts the interpreter token, looking through env and its options (-S, NAME=value), and only bash or sh qualify. Checked against eleven shebang forms (python/node with a trailing 'sh' comment, a bash-named directory, /bin/sh, env bash, env -S bash -e, env FOO=1 sh, dash, '#! /bin/bash', no shebang, bare env); the tree still yields the same 10 scripts and 2 launchers. Reported by CodeRabbit on #373. --- scripts/ci/shellcheck.sh | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/scripts/ci/shellcheck.sh b/scripts/ci/shellcheck.sh index 2abc3e74..7f6cca27 100755 --- a/scripts/ci/shellcheck.sh +++ b/scripts/ci/shellcheck.sh @@ -8,13 +8,38 @@ set -euo pipefail root="$(git rev-parse --show-toplevel)" cd "$root" +# Prints the interpreter a file's shebang names, looking through `env` and its +# options (`env -S bash -e`, `env NAME=value sh`). Fails for a file with no +# shebang or no interpreter. +shebang_interpreter() { + local line word + local -a words + IFS= read -r line <"$1" || [[ -n "$line" ]] || return 1 + [[ "$line" == "#!"* ]] || return 1 + read -r -a words <<<"${line#\#!}" + ((${#words[@]})) || return 1 + if [[ "${words[0]##*/}" != env ]]; then + printf "%s\n" "${words[0]##*/}" + return 0 + fi + for word in "${words[@]:1}"; do + if [[ "$word" != -* && "$word" != *=* ]]; then + printf "%s\n" "${word##*/}" + return 0 + fi + done + return 1 +} + scripts=() while IFS= read -r -d '' path; do case "$path" in *.sh) scripts+=("$path") ;; *) - if [[ -f "$path" ]] && head -n 1 "$path" | grep -qE '^#!.*\b(bash|sh)\b'; then - scripts+=("$path") + if [[ -f "$path" ]] && interpreter="$(shebang_interpreter "$path")"; then + case "$interpreter" in + bash | sh) scripts+=("$path") ;; + esac fi ;; esac From 39abf9060137c4c7f81d22d3656bda66eed8faa7 Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 19 Oct 2026 22:54:41 -0700 Subject: [PATCH 3/5] ci(shellcheck): parse env's options and operands to find the interpreter Codex found that '#!/usr/bin/env -S -u FOO bash' and '#!/usr/bin/env -S -C /tmp bash' resolved to FOO and tmp, and '--split-string=bash' to nothing, so such scripts would have been skipped. env_command now consumes option operands (-u, -C, -P, attached or separate, and their long forms), unpacks a split string (-S, -Sxxx, --split-string=), skips NAME=value, and honours --. The functions move to scripts/ci/lib/shebang.sh, sourced by shellcheck.sh, so spec/ci-shellcheck-shebang.spec.mjs can pin 22 shebang forms through bash without ShellCheck installed. The spec fails against the previous implementation (env -u FOO sh -> FOO) and passes now. --- scripts/ci/lib/shebang.sh | 55 +++++++++++++++++++++++++++++ scripts/ci/shellcheck.sh | 25 ++----------- spec/ci-shellcheck-shebang.spec.mjs | 55 +++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 22 deletions(-) create mode 100644 scripts/ci/lib/shebang.sh create mode 100644 spec/ci-shellcheck-shebang.spec.mjs diff --git a/scripts/ci/lib/shebang.sh b/scripts/ci/lib/shebang.sh new file mode 100644 index 00000000..7938c379 --- /dev/null +++ b/scripts/ci/lib/shebang.sh @@ -0,0 +1,55 @@ +# shellcheck shell=bash +# Sourced by scripts/ci/shellcheck.sh and its spec. Not a script itself. + +# Prints the command `env` would run given its arguments: option operands +# (`-u NAME`, `-C DIR`, `-P PATH`, attached or separate) are consumed, a split +# string (`-S`, `--split-string`) is unpacked, `NAME=value` assignments are +# skipped, and `--` ends the options. Fails when no command remains. +env_command() { + local word + local -a parts + while (($#)); do + word="$1" + shift + case "$word" in + -u | -C | -P | --unset | --chdir) (($#)) && shift ;; + --unset=* | --chdir=* | -u?* | -C?* | -P?*) ;; + -S | --split-string) ;; + --split-string=*) + read -r -a parts <<<"${word#--split-string=}" + set -- "${parts[@]}" "$@" + ;; + -S?*) + read -r -a parts <<<"${word#-S}" + set -- "${parts[@]}" "$@" + ;; + --) + (($#)) || return 1 + printf '%s\n' "${1##*/}" + return 0 + ;; + -* | *=*) ;; + *) + printf '%s\n' "${word##*/}" + return 0 + ;; + esac + done + return 1 +} + +# Prints the interpreter a file's shebang names, looking through `env` (see +# env_command). Fails for a file with no shebang or no interpreter. +shebang_interpreter() { + local line + local -a words + IFS= read -r line <"$1" || [[ -n "$line" ]] || return 1 + [[ "$line" == "#!"* ]] || return 1 + read -r -a words <<<"${line#\#!}" + ((${#words[@]})) || return 1 + if [[ "${words[0]##*/}" == env ]]; then + env_command "${words[@]:1}" + return + fi + printf '%s\n' "${words[0]##*/}" +} diff --git a/scripts/ci/shellcheck.sh b/scripts/ci/shellcheck.sh index 7f6cca27..3b9b533d 100755 --- a/scripts/ci/shellcheck.sh +++ b/scripts/ci/shellcheck.sh @@ -3,33 +3,14 @@ # scripts/install-hooks.sh generates. The launchers exist only inside an # installed hooks directory, so they are installed into a throwaway # repository and checked there. +# shellcheck source-path=SCRIPTDIR set -euo pipefail root="$(git rev-parse --show-toplevel)" cd "$root" -# Prints the interpreter a file's shebang names, looking through `env` and its -# options (`env -S bash -e`, `env NAME=value sh`). Fails for a file with no -# shebang or no interpreter. -shebang_interpreter() { - local line word - local -a words - IFS= read -r line <"$1" || [[ -n "$line" ]] || return 1 - [[ "$line" == "#!"* ]] || return 1 - read -r -a words <<<"${line#\#!}" - ((${#words[@]})) || return 1 - if [[ "${words[0]##*/}" != env ]]; then - printf "%s\n" "${words[0]##*/}" - return 0 - fi - for word in "${words[@]:1}"; do - if [[ "$word" != -* && "$word" != *=* ]]; then - printf "%s\n" "${word##*/}" - return 0 - fi - done - return 1 -} +# shellcheck source=lib/shebang.sh +source "$root/scripts/ci/lib/shebang.sh" scripts=() while IFS= read -r -d '' path; do diff --git a/spec/ci-shellcheck-shebang.spec.mjs b/spec/ci-shellcheck-shebang.spec.mjs new file mode 100644 index 00000000..85c1ad86 --- /dev/null +++ b/spec/ci-shellcheck-shebang.spec.mjs @@ -0,0 +1,55 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import test from 'node:test'; + +// scripts/ci/shellcheck.sh checks an extensionless tracked file only when its +// shebang names bash or sh as the interpreter, looking through env and its +// options. shebang_interpreter decides that; these cases pin it. +const LIBRARY = path.join(process.cwd(), 'scripts', 'ci', 'lib', 'shebang.sh'); +const NO_INTERPRETER = ''; + +const CASES = Object.freeze([ + ['#!/bin/sh', 'sh'], + ['#!/usr/bin/env bash', 'bash'], + ['#! /bin/bash', 'bash'], + ['#!/usr/bin/env FOO=1 sh', 'sh'], + ['#!/usr/bin/env -i bash', 'bash'], + ['#!/usr/bin/env -- bash', 'bash'], + ['#!/usr/bin/env -u FOO sh', 'sh'], + ['#!/usr/bin/env -uFOO bash', 'bash'], + ['#!/usr/bin/env --unset=FOO bash', 'bash'], + ['#!/usr/bin/env -P /usr/local/bin bash', 'bash'], + ['#!/usr/bin/env -S bash -e', 'bash'], + ['#!/usr/bin/env -Sbash -e', 'bash'], + ['#!/usr/bin/env -S -u FOO bash', 'bash'], + ['#!/usr/bin/env -S -C /tmp bash', 'bash'], + ['#!/usr/bin/env --split-string=bash', 'bash'], + ['#!/usr/bin/env python # sh', 'python'], + ['#!/usr/bin/env node # sh', 'node'], + ['#!/opt/bash-tools/python', 'python'], + ['#!/usr/bin/env -S python3 -u', 'python3'], + ['#!/bin/dash', 'dash'], + ['#!/usr/bin/env', NO_INTERPRETER], + ['echo no shebang', NO_INTERPRETER], +]); + +function interpreterOf(directory, index, firstLine) { + const file = path.join(directory, `case-${index}`); + writeFileSync(file, `${firstLine}\necho body\n`); + const result = spawnSync('bash', ['-c', 'source "$1" && shebang_interpreter "$2"', 'shebang', LIBRARY, file], { + encoding: 'utf8', + }); + return result.status === 0 ? result.stdout.trim() : NO_INTERPRETER; +} + +test('shebang_interpreter names the interpreter a shebang runs, looking through env', (t) => { + const directory = realpathSync(mkdtempSync(path.join(tmpdir(), 'jedit-shebang-'))); + t.after(() => rmSync(directory, { recursive: true, force: true })); + + CASES.forEach(([firstLine, expected], index) => { + assert.equal(interpreterOf(directory, index, firstLine), expected, firstLine); + }); +}); From 61c742402dbba1f8c86d01d15783638fc7080c5d Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 19 Oct 2026 23:04:50 -0700 Subject: [PATCH 4/5] ci(shellcheck): split env strings with quoting; keep skipping assignments after -- Codex found two more gaps: '-S -u "FOO BAR" bash' split the quoted operand (giving 'BAR"'), '-S "bash" -e' kept the quotes, and '-S -- FOO=1 bash' returned 'FOO=1' because -- ends only the options, not the NAME=value assignments. split_words now splits like env -S (single quotes literal; double quotes and bare text honour backslash escapes), used for the shebang line and for attached split strings; after -- the parser keeps skipping assignments, and a bare - is treated as -i. The spec gains six cases and fails against the previous library ('BAR"'). --- scripts/ci/lib/shebang.sh | 116 ++++++++++++++++++++-------- spec/ci-shellcheck-shebang.spec.mjs | 6 ++ 2 files changed, 89 insertions(+), 33 deletions(-) diff --git a/scripts/ci/lib/shebang.sh b/scripts/ci/lib/shebang.sh index 7938c379..58086168 100644 --- a/scripts/ci/lib/shebang.sh +++ b/scripts/ci/lib/shebang.sh @@ -1,39 +1,90 @@ # shellcheck shell=bash # Sourced by scripts/ci/shellcheck.sh and its spec. Not a script itself. -# Prints the command `env` would run given its arguments: option operands +# Splits $1 into words the way `env -S` does: whitespace separates words, +# '...' is literal, and "..." or unquoted text honours backslash escapes. +# Sets the array SPLIT_WORDS. +split_words() { + local text="$1" word="" quote="" char + local -i i in_word=0 + SPLIT_WORDS=() + for ((i = 0; i < ${#text}; i++)); do + char="${text:i:1}" + if [[ "$quote" == "'" ]]; then + if [[ "$char" == "'" ]]; then quote=""; else word+="$char"; fi + continue + fi + if [[ "$char" == "\\" ]] && ((i + 1 < ${#text})); then + i+=1 + word+="${text:i:1}" + in_word=1 + continue + fi + if [[ "$quote" == '"' ]]; then + if [[ "$char" == '"' ]]; then quote=""; else word+="$char"; fi + continue + fi + case "$char" in + "'" | '"') + quote="$char" + in_word=1 + ;; + ' ' | $'\t') + if ((in_word)); then + SPLIT_WORDS+=("$word") + word="" + in_word=0 + fi + ;; + *) + word+="$char" + in_word=1 + ;; + esac + done + if ((in_word)); then + SPLIT_WORDS+=("$word") + fi +} + +# Prints the command `env` would run given its arguments. Option operands # (`-u NAME`, `-C DIR`, `-P PATH`, attached or separate) are consumed, a split -# string (`-S`, `--split-string`) is unpacked, `NAME=value` assignments are -# skipped, and `--` ends the options. Fails when no command remains. +# string (`-S`, `--split-string`) is unpacked, `-` is `-i`, and `NAME=value` +# assignments are skipped both before and after `--`, which ends only the +# options. Fails when no command remains. env_command() { local word - local -a parts + local -i options=1 while (($#)); do word="$1" shift - case "$word" in - -u | -C | -P | --unset | --chdir) (($#)) && shift ;; - --unset=* | --chdir=* | -u?* | -C?* | -P?*) ;; - -S | --split-string) ;; - --split-string=*) - read -r -a parts <<<"${word#--split-string=}" - set -- "${parts[@]}" "$@" - ;; - -S?*) - read -r -a parts <<<"${word#-S}" - set -- "${parts[@]}" "$@" - ;; - --) - (($#)) || return 1 - printf '%s\n' "${1##*/}" - return 0 - ;; - -* | *=*) ;; - *) - printf '%s\n' "${word##*/}" - return 0 - ;; - esac + if ((options)); then + case "$word" in + -u | -C | -P | --unset | --chdir) + (($#)) && shift + continue + ;; + --split-string=* | -S?*) + if [[ "$word" == -S?* ]]; then + split_words "${word#-S}" + else + split_words "${word#--split-string=}" + fi + set -- "${SPLIT_WORDS[@]}" "$@" + continue + ;; + --) + options=0 + continue + ;; + -*) continue ;; + esac + fi + if [[ "$word" == *=* ]]; then + continue + fi + printf '%s\n' "${word##*/}" + return 0 done return 1 } @@ -42,14 +93,13 @@ env_command() { # env_command). Fails for a file with no shebang or no interpreter. shebang_interpreter() { local line - local -a words IFS= read -r line <"$1" || [[ -n "$line" ]] || return 1 [[ "$line" == "#!"* ]] || return 1 - read -r -a words <<<"${line#\#!}" - ((${#words[@]})) || return 1 - if [[ "${words[0]##*/}" == env ]]; then - env_command "${words[@]:1}" + split_words "${line#\#!}" + ((${#SPLIT_WORDS[@]})) || return 1 + if [[ "${SPLIT_WORDS[0]##*/}" == env ]]; then + env_command "${SPLIT_WORDS[@]:1}" return fi - printf '%s\n' "${words[0]##*/}" + printf '%s\n' "${SPLIT_WORDS[0]##*/}" } diff --git a/spec/ci-shellcheck-shebang.spec.mjs b/spec/ci-shellcheck-shebang.spec.mjs index 85c1ad86..212c2631 100644 --- a/spec/ci-shellcheck-shebang.spec.mjs +++ b/spec/ci-shellcheck-shebang.spec.mjs @@ -27,6 +27,12 @@ const CASES = Object.freeze([ ['#!/usr/bin/env -S -u FOO bash', 'bash'], ['#!/usr/bin/env -S -C /tmp bash', 'bash'], ['#!/usr/bin/env --split-string=bash', 'bash'], + ['#!/usr/bin/env -S -u "FOO BAR" bash', 'bash'], + ['#!/usr/bin/env -S "bash" -e', 'bash'], + ["#!/usr/bin/env -S 'bash' -e", 'bash'], + ['#!/usr/bin/env -S -- FOO=1 bash', 'bash'], + ['#!/usr/bin/env - bash', 'bash'], + ['#!/usr/bin/env -S FOO="a b" sh', 'sh'], ['#!/usr/bin/env python # sh', 'python'], ['#!/usr/bin/env node # sh', 'node'], ['#!/opt/bash-tools/python', 'python'], From a4cf40950db96329d3f1d1bb4193fb0a7a4cdd26 Mon Sep 17 00:00:00 2001 From: James Ross Date: Mon, 19 Oct 2026 23:15:19 -0700 Subject: [PATCH 5/5] ci(shellcheck): refuse env shebangs it cannot classify instead of emulating env Three review rounds each found another GNU env parsing case (option operands, quoted and escaped split strings, clustered options) that made an extensionless shell script silently skip ShellCheck. Emulating env is the source of those silent skips, so the check now fails closed: shebang_interpreter classifies plain shebangs (#!/path/interp, #!/usr/bin/env [NAME=value]... interp) and returns status 2 for any env shebang that uses options, quotes, or escapes; the driver refuses such tracked files with a message naming them and the fix (a plain shebang or a .sh extension). No tracked file uses such a form. split_words and env_command are removed. The spec pins 19 shebang forms and adds a driver test in a fixture repository; both fail against the previous scripts and pass now. --- scripts/ci/lib/shebang.sh | 124 +++++++--------------------- scripts/ci/shellcheck.sh | 14 +++- spec/ci-shellcheck-shebang.spec.mjs | 75 ++++++++++------- 3 files changed, 88 insertions(+), 125 deletions(-) diff --git a/scripts/ci/lib/shebang.sh b/scripts/ci/lib/shebang.sh index 58086168..83b9a42e 100644 --- a/scripts/ci/lib/shebang.sh +++ b/scripts/ci/lib/shebang.sh @@ -1,105 +1,37 @@ # shellcheck shell=bash # Sourced by scripts/ci/shellcheck.sh and its spec. Not a script itself. -# Splits $1 into words the way `env -S` does: whitespace separates words, -# '...' is literal, and "..." or unquoted text honours backslash escapes. -# Sets the array SPLIT_WORDS. -split_words() { - local text="$1" word="" quote="" char - local -i i in_word=0 - SPLIT_WORDS=() - for ((i = 0; i < ${#text}; i++)); do - char="${text:i:1}" - if [[ "$quote" == "'" ]]; then - if [[ "$char" == "'" ]]; then quote=""; else word+="$char"; fi - continue - fi - if [[ "$char" == "\\" ]] && ((i + 1 < ${#text})); then - i+=1 - word+="${text:i:1}" - in_word=1 - continue - fi - if [[ "$quote" == '"' ]]; then - if [[ "$char" == '"' ]]; then quote=""; else word+="$char"; fi - continue - fi - case "$char" in - "'" | '"') - quote="$char" - in_word=1 - ;; - ' ' | $'\t') - if ((in_word)); then - SPLIT_WORDS+=("$word") - word="" - in_word=0 - fi - ;; +# Classifies a file by its shebang. For a plain shebang (`#!/path/interp ...` +# or `#!/usr/bin/env [NAME=value]... interp ...`) it prints the interpreter and +# returns 0. It returns 1 when the file has no shebang or no interpreter, and +# 2 for an `env` shebang it does not parse: one that uses env options (such as +# `-S` or `-u`), quotes, or backslash escapes. Emulating env's parser is how +# shell scripts get silently skipped, so callers must refuse status 2 rather +# than guess. +shebang_interpreter() { + local line word + local -a words + IFS= read -r line <"$1" || [[ -n "$line" ]] || return 1 + [[ "$line" == "#!"* ]] || return 1 + line="${line#\#!}" + read -r -a words <<<"$line" + ((${#words[@]})) || return 1 + if [[ "${words[0]##*/}" != env ]]; then + printf '%s\n' "${words[0]##*/}" + return 0 + fi + if [[ "$line" == *[\"\'\\]* ]]; then + return 2 + fi + for word in "${words[@]:1}"; do + case "$word" in + -*) return 2 ;; + *=*) ;; *) - word+="$char" - in_word=1 + printf '%s\n' "${word##*/}" + return 0 ;; esac done - if ((in_word)); then - SPLIT_WORDS+=("$word") - fi -} - -# Prints the command `env` would run given its arguments. Option operands -# (`-u NAME`, `-C DIR`, `-P PATH`, attached or separate) are consumed, a split -# string (`-S`, `--split-string`) is unpacked, `-` is `-i`, and `NAME=value` -# assignments are skipped both before and after `--`, which ends only the -# options. Fails when no command remains. -env_command() { - local word - local -i options=1 - while (($#)); do - word="$1" - shift - if ((options)); then - case "$word" in - -u | -C | -P | --unset | --chdir) - (($#)) && shift - continue - ;; - --split-string=* | -S?*) - if [[ "$word" == -S?* ]]; then - split_words "${word#-S}" - else - split_words "${word#--split-string=}" - fi - set -- "${SPLIT_WORDS[@]}" "$@" - continue - ;; - --) - options=0 - continue - ;; - -*) continue ;; - esac - fi - if [[ "$word" == *=* ]]; then - continue - fi - printf '%s\n' "${word##*/}" - return 0 - done return 1 } - -# Prints the interpreter a file's shebang names, looking through `env` (see -# env_command). Fails for a file with no shebang or no interpreter. -shebang_interpreter() { - local line - IFS= read -r line <"$1" || [[ -n "$line" ]] || return 1 - [[ "$line" == "#!"* ]] || return 1 - split_words "${line#\#!}" - ((${#SPLIT_WORDS[@]})) || return 1 - if [[ "${SPLIT_WORDS[0]##*/}" == env ]]; then - env_command "${SPLIT_WORDS[@]:1}" - return - fi - printf '%s\n' "${SPLIT_WORDS[0]##*/}" -} diff --git a/scripts/ci/shellcheck.sh b/scripts/ci/shellcheck.sh index 3b9b533d..b5e99597 100755 --- a/scripts/ci/shellcheck.sh +++ b/scripts/ci/shellcheck.sh @@ -13,11 +13,17 @@ cd "$root" source "$root/scripts/ci/lib/shebang.sh" scripts=() +unparsed=() while IFS= read -r -d '' path; do case "$path" in *.sh) scripts+=("$path") ;; *) - if [[ -f "$path" ]] && interpreter="$(shebang_interpreter "$path")"; then + [[ -f "$path" ]] || continue + status=0 + interpreter="$(shebang_interpreter "$path")" || status=$? + if ((status == 2)); then + unparsed+=("$path") + elif ((status == 0)); then case "$interpreter" in bash | sh) scripts+=("$path") ;; esac @@ -26,6 +32,12 @@ while IFS= read -r -d '' path; do esac done < <(git ls-files -z) +if ((${#unparsed[@]})); then + printf 'shellcheck: cannot tell whether %s is a shell script from its env shebang\n' "${unparsed[@]}" >&2 + echo "shellcheck: use a plain shebang such as '#!/usr/bin/env bash' or '#!/bin/sh' (env options, quotes, and escapes are not parsed), or give a shell script a .sh extension." >&2 + exit 1 +fi + if ((${#scripts[@]} == 0)); then echo "shellcheck: no tracked shell scripts found" >&2 exit 1 diff --git a/spec/ci-shellcheck-shebang.spec.mjs b/spec/ci-shellcheck-shebang.spec.mjs index 212c2631..a5bab5c1 100644 --- a/spec/ci-shellcheck-shebang.spec.mjs +++ b/spec/ci-shellcheck-shebang.spec.mjs @@ -1,61 +1,80 @@ import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; -import { mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { cpSync, mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; import test from 'node:test'; +import { cleanGitEnv, git } from './git-fixture-helpers.mjs'; -// scripts/ci/shellcheck.sh checks an extensionless tracked file only when its -// shebang names bash or sh as the interpreter, looking through env and its -// options. shebang_interpreter decides that; these cases pin it. +// scripts/ci/shellcheck.sh checks an extensionless tracked file when its +// shebang names bash or sh. shebang_interpreter classifies plain shebangs and +// reports env shebangs it does not parse (env options, quotes, escapes) so the +// driver refuses them instead of silently skipping a shell script. const LIBRARY = path.join(process.cwd(), 'scripts', 'ci', 'lib', 'shebang.sh'); -const NO_INTERPRETER = ''; +const NO_INTERPRETER = ''; +const UNPARSED = ''; +const UNPARSED_STATUS = 2; const CASES = Object.freeze([ ['#!/bin/sh', 'sh'], ['#!/usr/bin/env bash', 'bash'], ['#! /bin/bash', 'bash'], + ['#!/bin/bash -e', 'bash'], ['#!/usr/bin/env FOO=1 sh', 'sh'], - ['#!/usr/bin/env -i bash', 'bash'], - ['#!/usr/bin/env -- bash', 'bash'], - ['#!/usr/bin/env -u FOO sh', 'sh'], - ['#!/usr/bin/env -uFOO bash', 'bash'], - ['#!/usr/bin/env --unset=FOO bash', 'bash'], - ['#!/usr/bin/env -P /usr/local/bin bash', 'bash'], - ['#!/usr/bin/env -S bash -e', 'bash'], - ['#!/usr/bin/env -Sbash -e', 'bash'], - ['#!/usr/bin/env -S -u FOO bash', 'bash'], - ['#!/usr/bin/env -S -C /tmp bash', 'bash'], - ['#!/usr/bin/env --split-string=bash', 'bash'], - ['#!/usr/bin/env -S -u "FOO BAR" bash', 'bash'], - ['#!/usr/bin/env -S "bash" -e', 'bash'], - ["#!/usr/bin/env -S 'bash' -e", 'bash'], - ['#!/usr/bin/env -S -- FOO=1 bash', 'bash'], - ['#!/usr/bin/env - bash', 'bash'], - ['#!/usr/bin/env -S FOO="a b" sh', 'sh'], ['#!/usr/bin/env python # sh', 'python'], ['#!/usr/bin/env node # sh', 'node'], ['#!/opt/bash-tools/python', 'python'], - ['#!/usr/bin/env -S python3 -u', 'python3'], ['#!/bin/dash', 'dash'], ['#!/usr/bin/env', NO_INTERPRETER], ['echo no shebang', NO_INTERPRETER], + ['#!/usr/bin/env -S bash -e', UNPARSED], + ['#!/usr/bin/env -i bash', UNPARSED], + ['#!/usr/bin/env -u FOO sh', UNPARSED], + ['#!/usr/bin/env -S -iu FOO bash', UNPARSED], + ['#!/usr/bin/env --split-string=-u"FOO BAR" bash', UNPARSED], + ['#!/usr/bin/env -S bash\\_-e', UNPARSED], + ['#!/usr/bin/env "bash"', UNPARSED], + ['#!/usr/bin/env -S python3 -u', UNPARSED], ]); -function interpreterOf(directory, index, firstLine) { +function temporaryDirectory(t, prefix) { + const directory = realpathSync(mkdtempSync(path.join(tmpdir(), prefix))); + t.after(() => rmSync(directory, { recursive: true, force: true })); + return directory; +} + +function classify(directory, index, firstLine) { const file = path.join(directory, `case-${index}`); writeFileSync(file, `${firstLine}\necho body\n`); const result = spawnSync('bash', ['-c', 'source "$1" && shebang_interpreter "$2"', 'shebang', LIBRARY, file], { encoding: 'utf8', }); + if (result.status === UNPARSED_STATUS) return UNPARSED; return result.status === 0 ? result.stdout.trim() : NO_INTERPRETER; } -test('shebang_interpreter names the interpreter a shebang runs, looking through env', (t) => { - const directory = realpathSync(mkdtempSync(path.join(tmpdir(), 'jedit-shebang-'))); - t.after(() => rmSync(directory, { recursive: true, force: true })); +test('shebang_interpreter classifies plain shebangs and reports env forms it does not parse', (t) => { + const directory = temporaryDirectory(t, 'jedit-shebang-'); CASES.forEach(([firstLine, expected], index) => { - assert.equal(interpreterOf(directory, index, firstLine), expected, firstLine); + assert.equal(classify(directory, index, firstLine), expected, firstLine); + }); +}); + +test('the ShellCheck driver refuses a tracked file whose env shebang it cannot classify', (t) => { + const repo = temporaryDirectory(t, 'jedit-shellcheck-driver-'); + mkdirSync(path.join(repo, 'scripts', 'ci'), { recursive: true }); + cpSync(path.join(process.cwd(), 'scripts', 'ci'), path.join(repo, 'scripts', 'ci'), { recursive: true }); + writeFileSync(path.join(repo, 'run'), '#!/usr/bin/env -S bash -e\necho hi\n', { mode: 0o755 }); + git(repo, 'init', '-q'); + git(repo, 'add', '-A'); + + const result = spawnSync(path.join(repo, 'scripts', 'ci', 'shellcheck.sh'), [], { + cwd: repo, + encoding: 'utf8', + env: cleanGitEnv(), }); + + assert.equal(result.status, 1); + assert.match(result.stderr, /cannot tell whether run is a shell script/); });