Upload the zizmor findings of a Dependabot head, whose token can write them [#318] - #319
Merged
Merged
Conversation
…e them [#318] The workflow auditor's upload step skipped a pull request Dependabot opened, by author name, and the comment beside it said such a run holds a token that cannot write security events. #312 took that same exclusion out of the other two uploads on this board on the reading of #256's run log; this file was not in that count, and the run log refutes it here too. The proposal Dependabot opened as #317 ran this workflow with the write it was said not to have, and the upload was the one step skipped: gh run view 34809323213 --repo Flowfin/core --log | grep -oE 'SecurityEvents: (write|read)|Contents: read|Metadata: read' | sort -u Contents: read Metadata: read SecurityEvents: write gh api repos/Flowfin/core/actions/runs/34809323213/jobs --jq '.jobs[] | .steps[] | select(.name=="Upload SARIF") | .conclusion' skipped So the condition now excludes a fork's pull request and nothing else, and the paragraph beside it carries the reading rather than the sentence it refutes. What the fork half rests on is not measured here and stays a claim. What it prevents is a surface that reports less than it examined: the auditor judged #317's head and passed it, and nothing it examined reached the code-scanning tab, so the only check-run name absent on that head against a person's head is the code-scanning application's zizmor context. It was found while comparing those two heads for #108. Whether the upload succeeds on a Dependabot head is not measured here; the next proposal Dependabot opens is where it is read. A failed upload on such a head is bounded by continue-on-error on the step, which this change leaves alone, because in this file the upload runs before the gating step. Closes #318 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 #318
What changed
The upload step of
.github/workflows/zizmor.ymlno longer excludes a pull request Dependabot opened. Its condition excludes a fork's pull request and nothing else, and the paragraph above it carries the run-log reading that refutes what it used to say. Thecontinue-on-erroron the step is untouched.What failure it prevents
A surface that reports less than it examined, and it has already happened on this board three times over: #312 corrected the other two uploads for exactly this, and counted two uploads where there are three.
On the head of #317, the newest proposal Dependabot opened here, the auditor ran, its token carried the write the comment said it lacked, and the upload was the one step skipped. So the gate passed a head whose examination never reached the code-scanning tab, and the only check-run name absent on that head against a person's head is the code-scanning application's
zizmorcontext. That comparison, taken for #108, is how it was found.Evidence
The run on #317's head, its grant, and its steps:
The check-run names on that head against the head of #312, one line of difference:
The file still parses as a workflow after the edit, with the one job it had, and the tree's own rule register refuses nothing:
What this does not cover
Whether the upload succeeds on a Dependabot head is not measured. The log proves the grant; the call has never been made on such a head by this workflow, and the next proposal Dependabot opens is where it is read. A failed upload there is bounded by
continue-on-error: trueon the step, which this change leaves alone: in this file the upload runs BEFORE the gating step, so the bound is that line and not the ordering the other two files rely on. Nothing here makes that a positive assurance.The fork half is untouched and unmeasured. The file still says a fork's pull request runs with a token that cannot write security events, no run on this board has been read for it, and the corrected paragraph says so.
No local run of the workflow auditor.
uvxis not on this workstation, sozizmorwas not run here on this file. Its verdict on this change is the one theAudit workflows (zizmor)check posts on this pull request, and it had not been read when this body was written.No fixture proves this bites. The change removes a condition rather than adding a guard, and what it enables is a network call to the code-scanning API that no fixture on this board can make. What stands in place of one is the log quotation above and the check runs this pull request produces.
Who has read it
Nobody but the author. This board had no second reader at the time this was opened, and the evidence above stands in place of one.