Say publicly why a finding that is not fixed does not apply (#89) - #286
Merged
Conversation
Every scanner pointed at this repository will eventually report something that is not exploitable here. The choices are to fix it, to suppress it silently, or to say publicly why it does not apply, and the third is the only one a downstream consumer can act on and the only one that survives the person who made the judgement leaving. `security/statements.json` is that document. It carries two kinds of entry, because there are two ways a finding stops reaching the gate and they do not have the same shape. A finding a scanner raised here and somebody dismissed has an identifier, so it carries a per-finding disposition. A class taken out of what a scanner is pointed at has no identifier and no list of what would have been found, so it carries a scope and, in a field of its own, what is consequently NOT being looked for. That last field is the one a consumer needs and the one an author will otherwise leave out. Both kinds live in one document because a consumer reads one thing and needs to see both what was examined and found not to apply and what was never examined. A document holding only dismissals lets a reader conclude that everything else came back clean, and where a class was excluded that conclusion is false and nothing says so. The failure it prevents is the cheaper path being the quieter one. Dismissing a finding costs a written statement; narrowing the scanner so the finding never fires costs nothing, and the configuration change looks like tuning. A rule whose honest route is more expensive than its evasion selects for the evasion without anybody deciding to evade, so the check reaches an exclusion and not only a dismissal. `.github/statements/statements.sh` refuses a document that does not parse before any rule reads it, an entry missing a field or carrying one blank, a status outside the vocabulary the document declares, one identifier used twice, a statement naming a configuration that is not tracked, a directive a register excludes with no statement, and a statement for a directive no register excludes any more. The two directions are what stop the pointer rotting from either end. The registers are DERIVED rather than listed, in the script and in the document both: every tracked file under .github/ whose name begins with `excluded` or is `suppressions`, excluding the scripts that read them. A register added tomorrow under that shape is read on the day it lands, where a list written here would drift against the thing it describes. Twenty-one fixtures run before the gate judges anything, and both Done-when proofs were watched on the real tree rather than only on a fixture: a trailing comma in the document reddens the run before any rule reads it, and a rule added to .github/shell-analysis/excluded-rules with no statement reddens it by name. What it cannot reach is printed on every run rather than only written down. Whether a statement is TRUE is a judgement no reading of the tree makes. An exclusion that is not a directive - the persona a workflow does not select, the codepoint a pattern does not list - is stated in the document and derived by nothing, so one added tomorrow with no statement is silent. A dismissal on the code-scanning surface is a state on the repository rather than a byte in the tree, so the four entries for those are written by hand and nothing compares them against the live alerts. The comment at the head of .github/workflows/scorecard.yml said the reasoning for a dismissal lived only in the dismissal, where a reader who cannot list the alerts cannot reach it. That stops being true with this change, so the comment says what is true now and states the bound rather than dropping it. The means is a JSON document read with jq, which is on the runner image, beside bash and awk for the registers, matching the other checks in this gate. JSON rather than this repository's own block-register shape because the audience is a downstream consumer rather than this gate: a bespoke format is not something a consumer acts on. The document says of itself that it is NOT an OpenVEX document and does not claim to be - no advisory against a dependency here has been raised, every entry is a policy check rather than a vulnerability, and the fields are named after OpenVEX's so that projecting it later is a mapping rather than a translation. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.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.
The issue this belongs to
Closes #89
What changed
security/statements.jsonsays publicly, per finding, why something a scannerraised here is not going to be fixed. It carries two kinds of entry, because there
are two ways a finding stops reaching the gate and they do not have the same
shape.
A
findingsentry is a finding a scanner raised against this repository andsomebody dismissed. It has an identifier, so it carries a per-finding disposition:
status, justification, impact, and the condition that retires it.
An
exclusionsentry is a class taken out of what a scanner is pointed at. It hasno identifier and no list of what would have been found, so it carries a scope,
and it carries in a field of its own what is consequently NOT being looked for.
That last field is the one a consumer needs and the one an author will otherwise
leave out - "the pedantic persona is not run" says less than the same sentence
ending "so no finding from that persona will appear in this document or in the
code-scanning tab at all".
.github/statements/statements.shand.github/workflows/statements.ymlare thecheck. The comment at the head of
.github/workflows/scorecard.ymlsaid thereasoning for a dismissal lived only in the dismissal; that stops being true with
this change, so it says what is true now and keeps the bound rather than dropping
it.
What failure it prevents
The cheaper path being the quieter one. Dismissing a finding costs a written
statement; narrowing the scanner so the finding never fires costs nothing, and the
configuration change looks like tuning. A rule whose honest route is more
expensive than its evasion selects for the evasion without anybody deciding to
evade. That is why the check reaches an exclusion and not only a dismissal, which
is the scope question this issue's comments raised three times.
It has already happened here in the weaker form. Four exclusions stand in this
tree today with their reasoning in a comment beside a pattern or a line in a
register, and four dismissed alerts stand on the code-scanning surface with their
reasoning in the dismissal itself, where a reader who cannot list the alerts
cannot reach it. Three of those four dismissals point at a repository a reader of
this board cannot open.
One of the four is worse than the others and the document is what fixes it. The
fuzzing check is dismissed permanently on the ground that there is no untrusted
input surface here for a fuzzer to reach. That is true of the tree today because
no parser is in it, and 0101, 0055 and open issue #86 say the opposite about the
surface that is coming. A dismissal comment carries no condition and its state does
not expire; a
retires_whenfield does, and that entry now carries one.Evidence
The commit these were run at:
The rule set and the fixtures behind it:
Twelve violations the rules must refuse, five near misses they must not, and four
readings of what a register line is. Every one runs before the gate judges the
document, so a rule cannot pass its fixture and refuse something else.
The tree it judges:
Two directives, both in
.github/shell-analysis/excluded-rules; the other threeregisters are empty of entries today. Four exclusions rather than two, because the
two that are not directives - the persona
.github/workflows/zizmor.ymldoes notselect and the codepoint
.github/workflows/unicode-guard.ymldoes not list -have entries with a null rule.
The four dismissed alerts the
findingsentries are written for, read live ratherthan recalled:
THAT READING NEEDS A PERMISSION A READER OF THIS BOARD DOES NOT HOLD, so it is a
reading made here rather than one a reader can reproduce from the command beside
it. The dismissal comment text is counted rather than pasted, for the reason the
document gives: three of the four send a reader outside this repository, and
pasting the reference would put a pointer of exactly the shape this issue exists
to replace into the thing that exists to replace it. The document's entries are
written in this repository's own words instead.
No dependency advisory exists to state. Every finding in the document is a policy
check from one scanner:
and the document a consumer reads is the one this change adds:
The rest of the gate, run locally at this commit:
What a guard here refuses, and the proof it bites
The check refuses a document that does not parse before any rule reads it, an
entry missing a field or carrying one blank, a status outside the vocabulary the
document declares, one identifier used twice, a statement naming a configuration
that is not tracked, a directive a register excludes with no statement, and a
statement for a directive no register excludes any more.
Both conditions this issue's Done-when names as proofs were watched on the real
tree rather than only on a fixture.
A malformed edit. One comma added after
"exclusions": [:A suppression with no statement. One line added to
.github/shell-analysis/excluded-rules:Both were then reverted and the run returns to the
okline quoted underEvidence.
The five near misses are the other half of the proof and they are the mistakes
somebody actually makes: both arrays present and empty, which is a tree with
nothing to state; a null
rule, which is an exclusion with no directive to name,in both directions of the comparison; a register comment line, which is where
every one of these registers explains itself; and a blank line.
A blank field is refused as hard as an absent key, and that is a fixture of its
own. A field somebody emptied to get past a required-fields check is the silent
suppression this document exists to replace, wearing the shape of a statement.
The means, and why it fits
A JSON document read with
jq, which is on the runner image, beside bash and awkfor the registers - the same means as
doc-paths,invariantsandshell-analysis, and no language, runtime or dependency this tree does notalready carry.
JSON rather than this repository's own block-register shape, which is what
.github/fixture-scrub/valuesand.github/invariants/rulesuse. Those are readby this gate and by nothing else; this document's audience is a downstream
consumer, and a bespoke format is not something a consumer acts on.
The document says of itself that it is NOT an OpenVEX document and does not claim
to be. No advisory against a dependency here has been raised, so there is nothing
for OpenVEX's vocabulary to describe and every entry is a policy check rather than
a vulnerability.
status,justificationandimpactare named after OpenVEX'sfields so that projecting this document later is a mapping rather than a
translation. No such projection has been produced, and the shape a dependency
advisory takes here is decided when the first one arrives.
What this does not cover
Whether a statement is TRUE. Every field either carries text or it does not;
whether the reasoning is sound, and whether what an entry says is not looked for is
what is actually not looked for, is a judgement no reading of the tree makes. The
review is where a wrong one is caught.
An exclusion that is not a directive in a register. The persona a workflow does
not select and the codepoint a pattern does not list are stated in the document
and derived by nothing, so their entries carry a null rule and only their
configuration path is checked. An exclusion of that kind added tomorrow with no
statement is silent to every rule here. That is the larger residual and it is
printed on every run rather than only written down.
A finding dismissed on the code-scanning surface. A dismissal is a state on the
repository rather than a byte in the tree, no commit records one, and the listing
needs a permission a reader of this board does not hold. The four
findingsentries are written by hand and nothing compares them against the live alerts, so
a fifth dismissal tomorrow adds no entry and reddens nothing.
The one open alert is not stated.
MaintainedIDis open rather than dismissed, soit is a finding nobody has disposed of and this document is for the ones somebody
has. Nothing here dismisses it.
.github/workflows/zizmor.ymland.github/workflows/unicode-guard.ymlare notedited. Their comments still carry their reasoning, the document now carries it
too, and nothing keeps the two in step. Pointing each comment at the document
would be the next step and it is not taken here.
No run on GitHub had happened when this body was written. Every command above ran
on a Windows machine in Git Bash, and the gate runs on
ubuntu-latest. One thingwas found that way and is worth naming: a
jqbuilt for Windows cannot open theprocess substitution a
--slurpfileis handed, and the failure is a message onstandard error with an empty verdict, which reads exactly like a run that found
nothing. Every input is a real file for that reason.
Who has read it
Nobody other than the author. There was no second reader available for it, and the
evidence above stands in place of one rather than the question being left open.