Upload the findings of a Dependabot head, whose token can write them [#311]

[iderex]

The issue this belongs to

#311, and it does not close here. The class stands at zero only once Scorecard re-reads a window in which every merged proposal carries an analysis check run, and the first Dependabot proposal after this lands is the earliest that can be true.

What changed

Both SARIF uploads on this board - .github/workflows/codeql.yml and .github/workflows/shell-analysis.yml - skipped a pull request Dependabot opened. The condition now excludes a fork's pull request and nothing else. The paragraph above each condition is corrected in the same change and carries the reading that refutes what it used to say.

What failure it prevents

A surface that reports less than it examined, and it has already happened.

The analysis runs on a Dependabot head and gates it: the Analyze (rust) check run on fe2fcb98 - the head of #256 - succeeded, and .github/codeql/codeql.sh check judged its SARIF in that run. Nothing it found reached the code-scanning tab, because the upload step was the one thing the condition skipped. So a reader of that tab saw a repository whose dependency proposals were never analysed, while the job log said they were.

The reason both files gave for the skip is refuted by this repository's own run log. That is how it was found: the Scorecard alert was read first, the one unchecked head in the window was identified, and the token that head's run held was read from the log rather than recalled.

gh run view 33360249639 --repo Flowfin/core --log | grep -oE 'SecurityEvents: write|Contents: read|Metadata: read|GITHUB_TOKEN Permissions'
GITHUB_TOKEN Permissions
Contents: read
Metadata: read
SecurityEvents: write

Evidence

The alert, and the number it rests on:

gh api repos/Flowfin/core/code-scanning/alerts/2 --jq .most_recent_instance.message.text
score is 9: SAST tool detected but not run on all commits:
Warn: 29 commits out of 30 are checked with a SAST tool

The one head in thirty that carries no analysis check run is #256's, and #256 is the only Dependabot proposal in that window. Walking the thirty most recently merged pull requests and counting successful github-advanced-security check runs on each head:

gh api "repos/Flowfin/core/pulls?state=closed&sort=updated&direction=desc&per_page=100" --paginate \
  --jq '.[] | select(.merged_at != null) | "\(.number) \(.merged_at) \(.user.login) \(.head.sha)"' \
  | sort -k2 -r | head -30 > prs.txt
while read -r n d u hs; do
  echo "PR#$n $u head=$(gh api "repos/Flowfin/core/commits/$hs/check-runs?per_page=100" \
    --jq '[.check_runs[]|select(.app.slug=="github-advanced-security" and .conclusion=="success")]|length')"
done < prs.txt

PR#256 dependabot[bot] head=0
PR#308 iderex head=3
PR#307 iderex head=3
... twenty-seven further rows, every one head=3 ...

Twenty-nine heads at three, one at zero, which is the alert's twenty-nine in thirty reproduced from the other side.

That the analysis itself ran on that head, and that the missing thing is the upload rather than the reading:

gh api repos/Flowfin/core/commits/fe2fcb98bac0710820a40a247f2b47bb4aa19ec1/check-runs --jq '.check_runs[]|select(.name=="Analyze (rust)")|"\(.name) \(.conclusion)"'
Analyze (rust) success

Both files still parse as workflows after the edit, and the tree's own rule register refuses nothing:

python -c "import yaml;[print(p,list(yaml.safe_load(open(p,encoding='utf-8'))['jobs'])) for p in ['.github/workflows/codeql.yml','.github/workflows/shell-analysis.yml']]"
.github/workflows/codeql.yml ['analyze']
.github/workflows/shell-analysis.yml ['shell']

bash .github/invariants/invariants.sh check; echo "EXIT=$?"
Every rule above was applied to its subject and refused nothing.
EXIT=0

What this does not cover

Whether the upload succeeds on a Dependabot head is not measured. What the log proves is the grant, SecurityEvents: write. The call itself has never been made on this board, and the next proposal Dependabot opens is where it is read. If it fails, the cost is bounded by an ordering both files already keep and which this change leaves alone: the upload step runs after the judging step, so a red upload names an upload that did not happen, stands behind a verdict already made, and hides no finding. Nothing here makes that a positive assurance.

The fork half is untouched and unmeasured. Both files still say a fork's pull request runs with a token that cannot write security events, and no run on this board has been read for it. It stays a claim, and the corrected paragraphs say so.

#311 does not close on this. Scorecard's window is the thirty most recently merged pull requests, and the alert clears only once one of them is a Dependabot proposal whose head carries the check run this change is meant to produce. Nothing here forces that; it arrives with the next weekly proposal.

No local run of the workflow auditor. .github/workflows/zizmor.yml reads these two files and was not run on this workstation. Its verdict on this change is the one the checks post here, and it has not been read at the time this body was written.

No fixture proves this bites. The change removes a condition rather than adding a guard, and the thing it enables is a network call to the code-scanning API that no fixture on this board can make. What stands in place of a fixture is the log quotation above and the check runs this pull request itself 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.