From e11196a5845eb07ed7f2833811cf9b0d4a33371e Mon Sep 17 00:00:00 2001 From: Nils Lehnen <30603423+iderex@users.noreply.github.com> Date: Mon, 7 Sep 2026 05:12:32 +0200 Subject: [PATCH] Upload the findings of a Dependabot head, whose token can write them [#311] Both SARIF uploads on this board skipped a pull request Dependabot opened, and the reason each of them gave for skipping it was that such a run holds a token that cannot write security events. The run log says otherwise. The proposal Dependabot opened as #256 ran the codeql workflow, and the job printed its own grant: 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 So the exclusion rested on a premise this repository's own logs refute, and the condition now excludes a fork's pull request and nothing else. What the fork half rests on is not measured here and stays a claim in both files. What it prevents is a surface that reports less than it examined. The analysis already runs on a Dependabot head and already gates it - the `Analyze (rust)` check run on `fe2fcb98` succeeded - but nothing it found reached the code-scanning tab, so a reader of that tab saw a repository whose dependency proposals were never analysed. The Scorecard SAST check reads the same absence and reports it as one commit in thirty going unchecked: 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 carrying no analysis check run is #256's, which is the only Dependabot proposal in the window: gh api repos/Flowfin/core/commits/fe2fcb98bac0710820a40a247f2b47bb4aa19ec1/check-runs --jq '[.check_runs[]|select(.app.slug=="github-advanced-security" and .conclusion=="success")]|length' 0 gh api repos/Flowfin/core/commits/40d38fbc1d33de91ebc4a364bcea93a305e7383d/check-runs --jq '[.check_runs[]|select(.app.slug=="github-advanced-security" and .conclusion=="success")]|length' 3 Whether an upload to a Dependabot head succeeds is NOT measured here. The grant is read from the log; the call itself has never been made on this board, and the next proposal Dependabot opens is where it is read. The cost of it failing is bounded by the ordering both files already keep: the upload step runs after the judging step, so a red upload names an upload that did not happen and stands behind a verdict that was already made. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com> --- .github/workflows/codeql.yml | 27 +++++++++++++++++++++------ .github/workflows/shell-analysis.yml | 22 ++++++++++++++++++---- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index f269e08..8db0d28 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -94,10 +94,22 @@ jobs: # `upload: never` here, and a separate upload step below, for the reason # .github/workflows/shell-analysis.yml already gives: a pull request from a - # fork and a Dependabot one run with a token that cannot write security - # events, and the gate has to refuse on those runs whatever the surface can - # accept. Splitting the two lets the upload be skipped without the verdict - # being skipped with it. + # fork runs with a token that cannot write security events, and the gate has + # to refuse on that run whatever the surface can accept. Splitting the two + # lets the upload be skipped without the verdict being skipped with it. + # + # THIS PARAGRAPH NAMED A DEPENDABOT PULL REQUEST BESIDE THE FORK AND THE RUN + # LOG REFUTES IT (#311). The proposal Dependabot opened as #256 ran this + # workflow with the write it was said not to have: + # + # 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 + # + # So the condition below excludes the fork and nothing else. What the fork + # half rests on is not measured here and stays a claim. - name: Analyse uses: github/codeql-action/analyze@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4.37.8 with: @@ -123,9 +135,12 @@ jobs: # After the judging step rather than before it, so the verdict is already # made when this runs and an upload cannot stand in front of a refusal. # always() is what carries the findings of a run that refused something, - # which is the run whose findings are worth having on the surface. + # which is the run whose findings are worth having on the surface. That + # ordering is also what bounds the cost of the widening above: an upload + # that fails on a head this condition now reaches leaves a red step behind + # a verdict that was already made, and hides no finding. - name: Upload the findings to the code-scanning tab - if: always() && ((github.event_name == 'push' && github.ref == 'refs/heads/main') || (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.user.login != 'dependabot[bot]')) + if: always() && ((github.event_name == 'push' && github.ref == 'refs/heads/main') || github.event.pull_request.head.repo.full_name == github.repository) uses: github/codeql-action/upload-sarif@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4.37.8 with: sarif_file: ${{ env.SARIF_DIR }}/rust.sarif diff --git a/.github/workflows/shell-analysis.yml b/.github/workflows/shell-analysis.yml index 08e65aa..0297be3 100644 --- a/.github/workflows/shell-analysis.yml +++ b/.github/workflows/shell-analysis.yml @@ -18,7 +18,9 @@ # writes them as SARIF and the step below uploads that file, so an alert carries # the same set the gate refused, at the same severity and with the same rules # excused. A pull request from a fork runs with a token that cannot write there, so -# the upload is skipped on one and the gate still refuses. +# the upload is skipped on one and the gate still refuses. A Dependabot proposal is +# NOT one of those: the paragraph at the upload step carries the run log that +# refutes the sentence that said it was. # # This covers the shell and nothing else. The analysis over the core's own language # is the other half of #81 and waits on #11, because there is no code in this tree @@ -79,8 +81,20 @@ jobs: # whose findings are worth having on the surface. # # Only where the token can write security events: a push to main and a pull - # request from a branch on this repository. A fork's pull request and a - # Dependabot one run read-only, and the upload is skipped rather than failed. + # request from a branch on this repository. A fork's pull request runs + # read-only, and the upload is skipped rather than failed. + # + # THIS PARAGRAPH NAMED A DEPENDABOT PULL REQUEST BESIDE THE FORK AND THE RUN + # LOG REFUTES IT (#311). The proposal Dependabot opened as #256 ran a + # workflow of this repository with the write it was said not to have, and the + # same reading is quoted in full in .github/workflows/codeql.yml beside the + # sentence it corrects there: + # + # gh run view 33360249639 --repo Flowfin/core --log | grep -oE 'SecurityEvents: write' + # SecurityEvents: write + # + # So the condition below excludes the fork and nothing else. What the fork + # half rests on is not measured here and stays a claim. # # No continue-on-error here, unlike .github/workflows/zizmor.yml, and the # reason is the ordering above: there the upload runs before the gating step @@ -88,7 +102,7 @@ jobs: # so a red upload step names an upload that did not happen and hides no # finding. - name: Upload the findings to the code-scanning tab - if: always() && ((github.event_name == 'push' && github.ref == 'refs/heads/main') || (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.user.login != 'dependabot[bot]')) + if: always() && ((github.event_name == 'push' && github.ref == 'refs/heads/main') || github.event.pull_request.head.repo.full_name == github.repository) uses: github/codeql-action/upload-sarif@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4.37.8 with: sarif_file: ${{ env.SARIF }}