Write the shell findings where the code-scanning tab reads them

[iderex]

The issue this belongs to

Refs #81. It is not finished by this change and stays open: the analysis over
the core's own language waits on #11, and this covers the other thing that issue
still holds, which is the shell findings reaching the code-scanning surface.

What changed

The shell leg writes its findings as SARIF when SHELL_ANALYSIS_SARIF names a
path, and .github/workflows/shell-analysis.yml uploads that file under the
category shellcheck.

The file is produced through the same function the gate is, from shellcheck's
own json1 rather than from the line format the gate prints, so the set that
reaches the surface is the set that was refused, at the same severity and with
the same rules excused. Reconstructing JSON out of a message that can itself
hold a quotation mark is how a file that parses on one machine is refused by the
surface, which is why the conversion starts from JSON.

A run with the variable unset writes nothing anywhere, which is what a run on
somebody's own machine does, and every run says which of the two it was on its
own last lines instead of leaving a reader to assume.

The upload step runs after the gate rather than before it, and carries
always(). The ordering means a refusal is already made when the upload starts,
so an upload cannot stand in front of one; always() means the findings of a
run that refused something are the ones that reach the surface, which is the run
whose findings are worth having there. 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.

The job now asks for security-events: write, and the workflow-level grant is
{} so a job added later starts with nothing.

What failure it prevents

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 found by whoever already
knows to open that run, which is nobody once the pull request that produced it
is merged. This repository already puts the workflow audit's findings and the
Scorecard findings on the code-scanning surface, so the one body of executable
code whose findings did not arrive there was the shell.

This has not happened here yet, because the tree carries no finding today. It is
the shape rather than an instance.

Evidence

Every fixture, at this commit, with shellcheck 0.10.0 and jq 1.7.1:

bash .github/shell-analysis/shell-analysis.sh selftest
-- the analyser refuses an unquoted expansion
ok    refused, and the finding names SC2086
-- the same fixture with the expansion quoted is not refused
ok    not refused
-- the register refuses a rule identifier with no reason
ok    refused, and the message names SC2129
-- the same register with that one line answered is not refused
ok    not refused
-- the refused finding is written for the code-scanning surface
ok    one finding, SC2086, at refused.sh line 8
-- the same fixture with the expansion quoted files nothing
ok    no finding written
-- a rule the register excuses is not written either
ok    no finding written, from the same fixture the first of these three refused
Every fixture behaved as this check claims, at severity style.

The tree passes and the file it writes carries nothing, which is the state it
should be in today:

SHELL_ANALYSIS_SARIF=/tmp/tree.sarif bash .github/shell-analysis/shell-analysis.sh check
Written for the code-scanning surface: /tmp/tree.sarif, carrying 0 finding(s).
Every tracked shell file passed at severity style, with the rules named above not refused and every other rule in force.

jq -c '{version, tool: .runs[0].tool.driver.name, analyser: .runs[0].tool.driver.version, results: (.runs[0].results|length)}' /tmp/tree.sarif
{"version":"2.1.0","tool":"shellcheck","analyser":"0.10.0","results":0}

The document check passes at this commit as well:

bash .github/doc-paths/doc-paths.sh check ; echo "exit=$?"
Every path these documents name resolves against the tracked set.
exit=0

What a guard here refuses, and the proof it bites

Three fixtures are added and each was watched failing, by making the mistake it
exists for.

The written file must carry the finding the analyser refused, naming the rule,
the file and the line. Forcing the region's start to line 1 in the conversion:

-            region: region(.)
+            region: (region(.) + { startLine: 1 })

bash .github/shell-analysis/shell-analysis.sh selftest
-- the refused finding is written for the code-scanning surface
::error::The written finding does not name the rule, the file and the line the analyser refused. It says SC2086 at /tmp/tmp.WIkwMmnJuA/refused.sh:1, and the fixture carries SC2086 at line 8.
exit=1

The neighbouring fixture, two quotation marks away, must file nothing. It is the
same pair the first two fixtures already use, so the near miss is the one a
person actually writes.

A rule the register excuses must not reach the surface either. Dropping
--exclude from the one place shellcheck is invoked, which is exactly how the
gate and the surface would come apart:

-    shellcheck --format="$format" --severity="$SEVERITY" --exclude="$excluded" "$@"
+    shellcheck --format="$format" --severity="$SEVERITY" "$@"

bash .github/shell-analysis/shell-analysis.sh selftest
-- a rule the register excuses is not written either
::error::A register excusing SC2086 still produced 1 finding(s) for the surface. The gate and the surface are reading different settings, so an alert could name a rule this repository has argued is not a defect here.
exit=1

It also bit on a real subject with nothing arranged. Appending an unquoted
expansion to a tracked file reddens the gate and puts that finding in the
written file, with the path relative to the tree:

SHELL_ANALYSIS_SARIF=/tmp/broken.sarif bash .github/shell-analysis/shell-analysis.sh check
.github/shell-analysis/shell-analysis.sh:464:11: note: Double quote to prevent globbing and word splitting. [SC2086]
Written for the code-scanning surface: /tmp/broken.sarif, carrying 1 finding(s).
::error::shellcheck refused at least one tracked shell file. Each finding is printed above with its file, its line and the rule that raised it.
exit=1

jq -c '.runs[0].results[] | {ruleId, level, uri: .locations[0].physicalLocation.artifactLocation.uri, line: .locations[0].physicalLocation.region.startLine}' /tmp/broken.sarif
{"ruleId":"SC2086","level":"note","uri":".github/shell-analysis/shell-analysis.sh","line":464}

That file was restored afterwards and the appended function is not in this
branch.

What this does not cover

The analysis over the core's own language. No language is chosen, no code is in
this tree, and #11 is where that is decided. #81 stays open on it.

The upload itself was not observed before this pull request existed, and it
cannot be: the token that can write security events is the one a run on this
repository holds. What is proven above is the file, its contents and the
conditions under which the step runs. Whether the surface accepted it is read
from this pull request's own run and from the code-scanning tab afterwards, not
from anything above.

No alert is filed by this change today. The tree carries no finding, so the
first upload writes an empty run, which is what closes a stale alert rather than
what opens one.

Nothing here says which findings are worth an alert and which are not. The gate
already refuses every one of them at severity style, so the surface receives
that same set, and a written position on what is not fixed is #89 rather than
this change.

.github/pr-hygiene/hygiene.sh was not run on this machine: it reaches the
tracker for each issue a body names and did not finish inside the time it was
given. It runs on this pull request, which is where its verdict is read.

The sentence about a job log being nobody's after a merge is a claim about how
this repository is read, not a measurement, and no command here backs it.

Who has read it

Nobody but the author. There is no second reader on this board tonight, and the
evidence above stands in place of one rather than the question being left open.

[github-advanced-security]

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.