From 6429bb362d1cdc5789cc8f4fdf76594ee85a9cd0 Mon Sep 17 00:00:00 2001 From: Nils Lehnen <30603423+iderex@users.noreply.github.com> Date: Sat, 22 Aug 2026 09:30:36 +0200 Subject: [PATCH] Write the shell findings where the code-scanning tab reads them The shell leg refused in place and wrote nothing anywhere, so a finding in the language two of this gate's own checks are written in was readable in a job log and nowhere else. A job log is kept for a while and is found by whoever already knows to open that run, which is nobody after the pull request that produced it is merged. The script now writes its findings as SARIF when SHELL_ANALYSIS_SARIF names a path, and the workflow uploads that file under its own category. The file is produced by the same function the gate is, from shellcheck's json1 rather than from the line format, so the set that reaches the surface is the set that was refused, at the same severity and with the same rules excused. A run with the variable unset writes nothing, which is what a run on somebody's own machine does, and each run says which of the two it was on its own last lines. The upload runs after the gate rather than before it, so a refusal is already made when it starts and an upload cannot stand in front of one, and it carries always() so that the findings of a run that refused something are the ones that reach the surface. It is skipped on a pull request from a fork and on a Dependabot one, where the token cannot write security events, and the gate still refuses there. Three fixtures cover the written file and each was watched failing. Dropping --exclude from the one place shellcheck is invoked reddens the third with "A register excusing SC2086 still produced 1 finding(s) for the surface". Forcing the region's start to line 1 reddens the first with "It says SC2086 at .../refused.sh:1, and the fixture carries SC2086 at line 8". Appending an unquoted expansion to a tracked file reddens the gate and puts that finding in the written file: {"ruleId":"SC2086","level":"note", "uri":".github/shell-analysis/shell-analysis.sh","line":464} SECURITY.md named zizmor.yml as the only pull-request-triggered job here holding security-events: write. This change makes that false, so the paragraph derives the set from the tree instead of naming a file, which is the shape the paragraphs around it already use. Refs #81 Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com> --- .github/shell-analysis/shell-analysis.sh | 182 ++++++++++++++++++++++- .github/workflows/shell-analysis.yml | 58 ++++++-- .github/workflows/zizmor.yml | 9 +- SECURITY.md | 16 +- docs/gate-parity.md | 2 +- 5 files changed, 237 insertions(+), 30 deletions(-) diff --git a/.github/shell-analysis/shell-analysis.sh b/.github/shell-analysis/shell-analysis.sh index 5d07371..b503557 100644 --- a/.github/shell-analysis/shell-analysis.sh +++ b/.github/shell-analysis/shell-analysis.sh @@ -25,13 +25,26 @@ # on whitespace and refused valid work for it. Both scripts here read paths and # pull-request bodies, which are exactly the inputs that carry a space. # +# Where a finding is read: the job log, and the code-scanning surface. A run writes +# the findings as SARIF when `SHELL_ANALYSIS_SARIF` names a path, and the workflow +# is what uploads that file. A run with the variable unset writes nothing anywhere, +# which is what a run on somebody's own machine does, and it says which of the two +# it was on its own last lines rather than leaving a reader to assume. +# +# The surface and the gate read one setting. The SARIF is produced by the same +# function the gate is, with the same severity and the same exclusions, so an alert +# that is filed is a finding that was refused and a rule this register excuses +# reaches neither. +# # Verbs: # selftest prove the analyser refuses that defect, that the same fixture with -# the quotation marks added is not refused, and that a rule excluded -# with no reason after it is refused while the same line answered is -# not -# check run the fixtures, judge the exclusion register, print it, then -# analyse every tracked shell file and refuse +# the quotation marks added is not refused, that a rule excluded with +# no reason after it is refused while the same line answered is not, +# and that the SARIF a run writes carries the refused finding, carries +# nothing for the neighbour, and carries nothing for an excused rule +# check run the fixtures, judge the exclusion register, print it, analyse +# every tracked shell file, write the SARIF where one is asked for, +# and refuse # # `check` reads the repository through `git ls-files`, so the authority for what is # analysed is the tracked set. A file present on disk and not added is not a file @@ -50,6 +63,17 @@ SEVERITY=style # whether to add an exclusion edits a register instead of a script. EXCLUSIONS_FILE="$(dirname "$0")/excluded-rules" +# Where a run writes its findings for the code-scanning surface. Empty means it +# writes nothing anywhere. It is read from the environment rather than taken as an +# argument so that the verb a person runs by hand and the verb the workflow runs +# are the same verb, and the difference between them is one variable a reader can +# see in the workflow file. +SARIF_OUT="${SHELL_ANALYSIS_SARIF:-}" + +# The schema the written file declares. SARIF 2.1.0 is the version the +# code-scanning surface accepts. +SARIF_SCHEMA="https://json.schemastore.org/sarif-2.1.0.json" + # Every tracked shell file. shell_files() { git ls-files '*.sh' @@ -100,13 +124,95 @@ judge_register() { # file's shebang: forcing one here would read a file declaring another dialect as # if it had declared this one, which is the finding rather than a setting. analyse() { + analyse_as gcc "$@" +} + +# The one place shellcheck is invoked. The format is the only thing that varies +# between the gate and the file written for the code-scanning surface, so the +# severity and the exclusions cannot differ between what is refused and what is +# filed as an alert. +analyse_as() { + local format="$1" + shift local excluded excluded="$(excluded_ids "$EXCLUSIONS_FILE")" if [ -n "$excluded" ]; then - shellcheck --format=gcc --severity="$SEVERITY" --exclude="$excluded" "$@" + shellcheck --format="$format" --severity="$SEVERITY" --exclude="$excluded" "$@" else - shellcheck --format=gcc --severity="$SEVERITY" "$@" + shellcheck --format="$format" --severity="$SEVERITY" "$@" + fi +} + +# The build of the analyser that judged this run, so the written file says which +# one produced its findings rather than leaving that to the runner image. +analyser_version() { + shellcheck --version | awk -F': ' '/^version:/ { print $2; exit }' +} + +# Writes the findings for the files given, as SARIF, to the path in $1. +# +# The conversion is from shellcheck's own `json1`, which carries the rule number, +# the level, the message and both ends of the region, rather than from the line +# format the gate prints: reconstructing JSON out of a message that may itself hold +# a quotation mark is how a file that parses locally is refused by the surface. +# +# The level names differ between the two vocabularies and the mapping is written +# out rather than passed through. shellcheck says error, warning, info and style; +# SARIF has error, warning, note and none. info and style both become note, which +# is the level this surface shows without gating, and nothing becomes none, because +# a finding this gate refuses is not one to file as having no level at all. +# +# The region's end is written only where it is past the start. An end equal to the +# start is a zero-width region, which is a shape the surface rejects on some +# findings and renders as nothing on others. +write_sarif() { + local out="$1" + shift + local found status=0 + found="$(analyse_as json1 "$@")" || status=$? + if [ -z "$found" ]; then + echo "::error::The analyser wrote no JSON to convert, so there is nothing to hand the code-scanning surface. It exited ${status}." + return 1 fi + printf '%s\n' "$found" | jq \ + --arg schema "$SARIF_SCHEMA" \ + --arg analyser "$(analyser_version)" ' + def sarif_level: + { "error": "error", "warning": "warning", "info": "note", "style": "note" }[.] // "note"; + def region($c): + { startLine: $c.line, startColumn: $c.column } + + (if ($c.endLine > $c.line) or ($c.endColumn > $c.column) + then { endLine: $c.endLine, endColumn: $c.endColumn } + else {} end); + [ .comments[]? ] as $found + | { "$schema": $schema, + version: "2.1.0", + runs: [ { + tool: { driver: { + name: "shellcheck", + informationUri: "https://www.shellcheck.net/", + version: $analyser, + rules: ( $found + | map({ id: ("SC" + (.code | tostring)) }) + | unique_by(.id) + | map(. + { helpUri: ("https://www.shellcheck.net/wiki/" + .id) }) ) + } }, + results: ( $found | map({ + ruleId: ("SC" + (.code | tostring)), + level: (.level | sarif_level), + message: { text: .message }, + locations: [ { physicalLocation: { + artifactLocation: { uri: .file, uriBaseId: "%SRCROOT%" }, + region: region(.) + } } ] + }) ) + } ] } + ' > "$out" +} + +# How many findings the written file carries. +sarif_results() { + jq '.runs[0].results | length' "$1" } # -------------------------------------------------------------------------- @@ -230,6 +336,53 @@ selftest() { echo "ok not refused" echo + # The three below are about the file a run hands the code-scanning surface. The + # four above prove what is refused; these prove that what is refused is what is + # filed, that the neighbour files nothing, and that a rule the register excuses + # reaches the surface no more than it reaches the gate. + local wanted_line ruleid uri line results + wanted_line="$(awk '/wc -l