Skip to content

ci(codeql): a failed SARIF upload is retried once, in a job of its own (#1499) - #1504

Merged
DrunkOnJava merged 5 commits into
mainfrom
lane/1499-codeql-upload-retry
Nov 6, 2026
Merged

DrunkOnJava merged 5 commits into
mainfrom
lane/1499-codeql-upload-retry

Conversation

@DrunkOnJava

@DrunkOnJava DrunkOnJava commented Nov 5, 2026 •

Copy link
Copy Markdown
Owner

Summary

CodeQL's analyze job retries a failed SARIF upload once, in a job of its own, with the same shape #1461 gave Scorecard and zizmor. A real rejection of the file still fails the run.

Motivation

#1499: Analyze (rust) failed on #1468 (CodeQL run 42128776536) at github/codeql-action/analyze with the log ending at "Uploading results" and no finding. arm.sh refuses a head with a red check, so the landing waited for a hand rerun.

Changes

.github/workflows/codeql.yml and docs/maintaining.md; the matrix stays the four languages and the check names Analyze (<language>) do not change.

  • analyze gets upload: failure-only and output: ${{ runner.temp }}/codeql-sarif: it writes the SARIF, with the category already in it (analyze passes category to the CLI as the SARIF category), and does not upload it. At the pinned commit (v4.39.1, analyze-action.ts) analyze post-processes and writes the files for every value of upload, and sends them only for always; the one-category-per-job sentinel is set only by an upload (validateUniqueCategory, inside uploadPostProcessedFiles), so failure-only leaves the later upload step free of it. It is failure-only and not never because never makes init-action-post-helper.ts (prepareFailedSarif) skip the failed-run SARIF when the analysis itself dies, and analyze's action.yml calls never not recommended. It still saves the overlay base database on main.
  • A step of its own, Upload the results to code scanning (upload-sarif, same category, continue-on-error: true, waits for processing as analyze did), uploads that directory. Its failure does not fail the leg.
  • Keep the SARIF for the retry runs only when that step failed and uploads the directory as the artifact codeql-sarif-<language> (7 days, overwrite: true so that "Re-run all jobs" can replace the one an earlier attempt left; an artifact name is taken once per run). A healthy leg leaves no artifact.
  • analyze-retry (new job, needs: analyze, runs when analyze succeeded; skipped on a draft and when a leg really failed) downloads codeql-sarif-* into one directory (merge-multiple, since a single match is otherwise not put in a directory of its own) and, for each language whose SARIF is there, uploads it once more with that language's category, the matrix input the leg has ({"language": "<language>"}, as toJson(matrix) writes it) and CODEQL_ACTION_ANALYSIS_KEY: .github/workflows/codeql.yml:analyze, so the retry is the same configuration in code scanning and leaves no second one holding alerts. The steps are one per language, each if: ${{ !cancelled() && hashFiles('retry/<codeql language>.sarif') != '' }} (javascript-typescript's file is javascript.sarif): a rejected file fails its step and the job, and the next language is still tried.
  • The job's last step, Fail on a SARIF no step above uploads, fails on a kept retry/<name>.sarif that no sarif_file: of the workflow names, so a language added to analyze's matrix and not to the retry cannot pass with its kept results unsent. It reads the steps from the job's own checkout, not from a second list of names.
  • The retry job holds contents: read and security-events: write. It does not hold actions: read: with the analysis key set, the only call upload-sarif makes about the run (GET /repos/:owner/:repo/actions/runs/:run_id, getWorkflowRelativePath in api-client.ts) is not made, and the other actions API call in the action's source is the cache listing analyze uses.
  • Why a job of its own: codeql-action's upload-lib.ts exports a sentinel into the job's GITHUB_ENV before it sends anything and refuses a second upload of one category in a job ("only one run of the codeql/analyze or codeql/upload-sarif actions is allowed per job per tool/category"), so a second step would fail at once whatever the first died of. Shown by ci: wheels without LTO, CodeQL on ready pull requests, one RE1 pin list, a retried SARIF upload (#1457) #1461 (run 42096477400).
  • Why the artifacts say which leg failed: a matrix job's outputs are one set shared by its legs (the last to finish sets them), so a leg cannot report its own failure through them. The cost is one job on each non-draft run that does a checkout and an empty download when nothing failed.
  • docs/maintaining.md's CodeQL bullet names the retry job.
  • The SHA pins are the ones the file and ci: wheels without LTO, CodeQL on ready pull requests, one RE1 pin list, a retried SARIF upload (#1457) #1461 already use (upload-sarif at the pinned v4.39.1, upload-artifact v7.0.2, download-artifact v8, as in scorecard.yml).

Left alone

Testing

Unit tests are banned in this repository: do not add them. Show the change working end to end on real files instead.

  • tools/check-local.sh passes. N/A: CI-only change; no local compute on this lane.
  • Verified end to end on real Revit files. N/A: no Rust or reader change.
  • If this touches the IFC exporter. N/A.
  • If this is a performance change. N/A.

On this head (4837ed23):

  • actionlint exits 0. python3 tools/ci/check-workflows.py: 0 errors, 0 warnings (66 jobs run steps in 17 workflows, the new job has its timeout). zizmor 1.32.0 offline over .github: 45 findings (2 ignored, 37 suppressed, 6 medium: the Dependabot 3-day cooldown), the same count as origin/main; at the pedantic persona the retry job has no finding. The first version of the retry steps passed the matrix input as toJson(fromJSON('{...}')), which zizmor reports as obfuscation, so it is written out.
  • First proof, on the first version (upload: never, actions: read held, a list of file names in the last step; run 42130620963, scratch branch, a copy of this workflow with the trigger push on that branch, the matrix cut to actions and python, and in the first upload of both legs CODEQL_ACTION_ANALYSIS_KEY set, so that the action needs no API call before it sends the file, and token set to a refused one for the actions leg only): the actions leg's upload logged Post-processing sarif files, Uploading results, then Bad credentials, failed, and the job stayed green; codeql-sarif-actions was kept; python's upload succeeded and left no artifact; the retry job warned, uploaded retry/actions.sarif (Successfully uploaded results, Analysis upload status is complete) and skipped the other languages; code scanning listed the retried analysis with category /language:actions, analysis key .github/workflows/codeql.yml:analyze and environment {"language":"actions"}, the same three values main's actions analysis has.
  • Second proof, on the changes of the second read (upload: failure-only, no actions: read, !cancelled(); run 42133787781, scratch branch, the same copy with the matrix actions, javascript-typescript and python, every first upload given the refused token, the retry of actions given a refused token as well, and the python retry step removed): all three legs' uploads failed with Bad credentials after Uploading results (so analyze under failure-only had not uploaded, and set no sentinel) and the jobs stayed green; the three artifacts were kept; the retry job (permissions contents: read and security-events: write only) downloaded the three, warned for each, failed the actions retry step on the refused token, still ran the javascript-typescript retry step, which uploaded (Successfully uploaded results, Analysis upload status is complete), skipped rust, and the job ended failure, so the run did. That is a real rejection staying red with the next language still tried, and the upload working without actions: read.
  • That run's last step was the first version of it, which listed the file names and so passed for the python SARIF whose upload step I had removed; it now reads the steps from the workflow. Its script, taken from the workflow, run with bash on fake kept files: no file exits 0; actions, javascript, python and rust exits 0; a kept go.sarif exits 1 with the error naming it; the workflow with the python step's sarif_file renamed and a kept python.sarif exits 1. Not run on a runner.
  • Both scratch runs' analyses on their refs, their artifacts and the two scratch branches were deleted afterwards (their commits were never on this branch).
  • Not exercised: the real workflow's own run. CodeQL runs on this pull request once it is ready, so the first run of upload: failure-only followed by the upload step, the retry job finding nothing, and all four languages on the real file is the CI run of this head; a retry of rust (the shape is the others', with rust.sarif's name taken from a run's log); overwrite on a re-run.

Audit-honesty checklist

  • README / docs reflect the current state after this PR (docs/maintaining.md, and the workflow's header comment says what the retry is and why)
  • A user-visible change has its entry as a fragment under changelog.d/<section>/. N/A: CI only, as agreed for CI: where the minutes go, and what to change #1457.
  • If a limitation was resolved, the "Known limitations" section is updated. N/A.
  • If a new limitation was introduced, it is documented in the PR body (Left alone)
  • Commit messages describe WHAT changed and WHY
  • No emojis in code, docs, or commit messages
  • No private paths, personal email addresses, or internal jargon in committed files
  • Reverse-engineering finding. N/A.

Legal and contribution hygiene

  • I agree this work is licensable under Apache-2.0
  • This PR contains no Autodesk proprietary source, decompiled internals, or NDA'd SDK content
  • Any new test fixtures use synthetic values

Breaking changes

None.

Screenshots or output samples

See Testing.

Related issues

Closes #1499

#1499)

analyze uploaded its own SARIF and Analyze (rust) failed on #1468 with the log ending at Uploading results and no finding, the failure class #1461 gave Scorecard and zizmor a retry for. analyze now writes the SARIF without uploading it (upload: never), a step of its own uploads it and may fail without failing the job, and a failure leaves the SARIF as an artifact; analyze-retry, a job of its own because codeql-action refuses a second upload of one category in a job, uploads what the artifacts hold with the same category, analysis key and matrix. A real rejection fails the retry and the run. Closes #1499.
@DrunkOnJava DrunkOnJava added the ready-when-green Draft whose code is complete: marked ready for review once its fast CI passes label Nov 5, 2026
@DrunkOnJava
DrunkOnJava marked this pull request as ready for review November 5, 2026 23:44
@DrunkOnJava DrunkOnJava removed the ready-when-green Draft whose code is complete: marked ready for review once its fast CI passes label Nov 5, 2026
…language, and fail on a SARIF no step uploads (#1499)

Second-read findings on the retry. analyze takes upload: failure-only, not never: both leave the SARIF in output and upload nothing of it (the upload is gated on always), but never makes codeql-action skip the failed-run SARIF when the analysis itself dies. The kept artifact overwrites an earlier attempt's. Each language's retry step runs whatever an earlier one did, a last step fails on a kept SARIF no step covers, the retry job drops actions: read (with the analysis key set upload-sarif never asks about the run), and docs/maintaining.md's CodeQL bullet names the retry job. Closes #1499.
…ob's own upload steps (#1499)

The first version listed the four file names beside the four steps, which a step removed or a name added would leave out of step. It now fails on a kept SARIF whose retry/<name>.sarif no step of the workflow names, checked on fake kept files and on the workflow with a step's file renamed. Closes #1499.
@DrunkOnJava
DrunkOnJava enabled auto-merge (squash) November 6, 2026 00:29
@DrunkOnJava
DrunkOnJava merged commit 7da2fb4 into main Nov 6, 2026
39 checks passed
@DrunkOnJava
DrunkOnJava deleted the lane/1499-codeql-upload-retry branch November 6, 2026 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CodeQL analyze: a transient SARIF upload failure turns the check red with no finding

1 participant