From ba79a4f0b940b7145973a1b844ab2c6332af8311 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 9 Oct 2026 09:55:02 +0100 Subject: [PATCH 01/26] Do not run `pr-checks` for all push events --- .github/workflows/pr-checks.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 2ae059440..aa8d15c10 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -2,6 +2,9 @@ name: PR Checks on: push: + branches: + - "main" + - "releases/v*" pull_request: merge_group: types: [checks_requested] From 2883b5b5db290dd67f1501acbde11ac5da9c1a01 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 9 Oct 2026 10:02:12 +0100 Subject: [PATCH 02/26] Fix conditions in `pr-checks` workflow --- .github/workflows/pr-checks.yml | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index aa8d15c10..1036cfa28 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -148,12 +148,11 @@ jobs: - name: Fetch base commit id: fetch-base - # Forks and Dependabot PRs don't have permission to write comments, so skip the repo size - # check in those cases. + # Skip this (and subsequent steps) if the PR originates from a fork, since this workflow + # won't have the necessary permissions to post comments on the PR. if: >- github.event_name == 'pull_request' && - github.event.pull_request.head.repo.full_name == github.repository && - github.event.pull_request.user.login != 'dependabot[bot]' + github.event.pull_request.head.repo.full_name == github.repository env: BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} @@ -212,7 +211,7 @@ jobs: if: >- github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository && - github.event.pull_request.user.login != 'dependabot[bot]' && + github.triggering_actor != 'dependabot[bot]' && needs.other-checks.result == 'success' permissions: contents: read From a6ff2ad52a40a9b23b69a8b73d442a64ac5d800f Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 9 Oct 2026 14:30:58 +0100 Subject: [PATCH 03/26] Use `github.actor` for `post-repo-size-comment` condition --- .github/workflows/pr-checks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 1036cfa28..476764219 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -211,7 +211,7 @@ jobs: if: >- github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository && - github.triggering_actor != 'dependabot[bot]' && + github.actor != 'dependabot[bot]' && needs.other-checks.result == 'success' permissions: contents: read From e038b67f1c7c67d313ac8fd9c3b36c8e1c9de0f4 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Thu, 15 Oct 2026 21:39:28 +0100 Subject: [PATCH 04/26] Graduate overlay TRAP caching defaults Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- init/action.yml | 2 +- lib/entry-points.js | 11 ++-- src/config-utils.test.ts | 112 ++++++++++++++++----------------------- src/config-utils.ts | 20 +++---- src/feature-flags.ts | 7 --- 5 files changed, 59 insertions(+), 93 deletions(-) diff --git a/init/action.yml b/init/action.yml index 7787a0a07..5b3ab0a1e 100644 --- a/init/action.yml +++ b/init/action.yml @@ -153,7 +153,7 @@ inputs: required: false trap-caching: description: >- - Explicitly enable or disable TRAP caching rather than respecting the feature flag for it. + Explicitly enable or disable TRAP caching instead of using the automatically determined default. required: false dependency-caching: description: >- diff --git a/lib/entry-points.js b/lib/entry-points.js index 1a99a4442..d5515b09c 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -172085,11 +172085,6 @@ var featureConfig = { minimumVersion: CODEQL_OVERLAY_MINIMUM_VERSION_RUBY }, // Other overlay-related feature flags - ["overlay_analysis_disable_trap_caching" /* OverlayAnalysisDisableTrapCaching */]: { - defaultValue: false, - envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS_DISABLE_TRAP_CACHING", - minimumVersion: void 0 - }, ["overlay_analysis_match_codeql_version" /* OverlayAnalysisMatchCodeqlVersion */]: { defaultValue: false, envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS_MATCH_CODEQL_VERSION", @@ -174834,11 +174829,11 @@ async function validateOverlayDatabaseMode(overlayDatabaseMode, useOverlayDataba overlayModeSetExplicitly }); } -async function isTrapCachingEnabled(features, overlayDatabaseMode) { +function isTrapCachingEnabled(overlayDatabaseMode) { const trapCaching = getOptionalInput("trap-caching"); if (trapCaching !== void 0) return trapCaching === "true"; if (!looksLikeHostedRunner()) return false; - if (overlayDatabaseMode !== "none" /* None */ && await features.getValue("overlay_analysis_disable_trap_caching" /* OverlayAnalysisDisableTrapCaching */)) { + if (overlayDatabaseMode !== "none" /* None */) { return false; } return true; @@ -175041,7 +175036,7 @@ async function initConfig(actionState, inputs) { inputs.codeql, logger2 ); - if (await isTrapCachingEnabled(features, config.overlayDatabaseMode)) { + if (isTrapCachingEnabled(config.overlayDatabaseMode)) { const { trapCaches, trapCacheDownloadTime } = await downloadCacheWithTime( inputs.codeql, config.languages, diff --git a/src/config-utils.test.ts b/src/config-utils.test.ts index 9ed0187e6..f31cf0206 100644 --- a/src/config-utils.test.ts +++ b/src/config-utils.test.ts @@ -2126,7 +2126,7 @@ test.serial("getPrimaryAnalysisConfig - Code Scanning + Code Quality", (t) => { }); test.serial( - "isTrapCachingEnabled: explicit input true is respected", + "isTrapCachingEnabled: explicit true input overrides self-hosted default", async (t) => { return await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); @@ -2134,12 +2134,40 @@ test.serial( .stub(actionsUtil, "getOptionalInput") .withArgs("trap-caching") .returns("true"); - t.true( - await configUtils.isTrapCachingEnabled( - createFeatures([]), - OverlayDatabaseMode.None, - ), - ); + t.true(configUtils.isTrapCachingEnabled(OverlayDatabaseMode.None)); + }); + }, +); + +test.serial( + "isTrapCachingEnabled: explicit true input overrides overlay defaults", + async (t) => { + return await withTmpDir(async (tmpDir) => { + setupActionsVars(tmpDir, path.join(tmpDir, "hostedtoolcache")); + sinon + .stub(actionsUtil, "getOptionalInput") + .withArgs("trap-caching") + .returns("true"); + for (const overlayDatabaseMode of [ + OverlayDatabaseMode.Overlay, + OverlayDatabaseMode.OverlayBase, + ]) { + t.true(configUtils.isTrapCachingEnabled(overlayDatabaseMode)); + } + }); + }, +); + +test.serial( + "isTrapCachingEnabled: explicit false input overrides hosted default", + async (t) => { + return await withTmpDir(async (tmpDir) => { + setupActionsVars(tmpDir, path.join(tmpDir, "hostedtoolcache")); + sinon + .stub(actionsUtil, "getOptionalInput") + .withArgs("trap-caching") + .returns("false"); + t.false(configUtils.isTrapCachingEnabled(OverlayDatabaseMode.None)); }); }, ); @@ -2153,18 +2181,13 @@ test.serial( .stub(actionsUtil, "getOptionalInput") .withArgs("trap-caching") .returns(undefined); - t.false( - await configUtils.isTrapCachingEnabled( - createFeatures([]), - OverlayDatabaseMode.None, - ), - ); + t.false(configUtils.isTrapCachingEnabled(OverlayDatabaseMode.None)); }); }, ); test.serial( - "isTrapCachingEnabled: enabled on hosted runner by default", + "isTrapCachingEnabled: enabled on hosted runner without overlay analysis", async (t) => { return await withTmpDir(async (tmpDir) => { const hostedToolCache = path.join(tmpDir, "hostedtoolcache"); @@ -2173,18 +2196,13 @@ test.serial( .stub(actionsUtil, "getOptionalInput") .withArgs("trap-caching") .returns(undefined); - t.true( - await configUtils.isTrapCachingEnabled( - createFeatures([]), - OverlayDatabaseMode.None, - ), - ); + t.true(configUtils.isTrapCachingEnabled(OverlayDatabaseMode.None)); }); }, ); test.serial( - "isTrapCachingEnabled: enabled on hosted runner when overlay enabled but feature flag off", + "isTrapCachingEnabled: disabled on hosted runner with overlay analysis", async (t) => { return await withTmpDir(async (tmpDir) => { const hostedToolCache = path.join(tmpDir, "hostedtoolcache"); @@ -2193,52 +2211,12 @@ test.serial( .stub(actionsUtil, "getOptionalInput") .withArgs("trap-caching") .returns(undefined); - t.true( - await configUtils.isTrapCachingEnabled( - createFeatures([]), - OverlayDatabaseMode.Overlay, - ), - ); - }); - }, -); - -test.serial( - "isTrapCachingEnabled: disabled on hosted runner when overlay enabled and feature flag on", - async (t) => { - return await withTmpDir(async (tmpDir) => { - const hostedToolCache = path.join(tmpDir, "hostedtoolcache"); - setupActionsVars(tmpDir, hostedToolCache); - sinon - .stub(actionsUtil, "getOptionalInput") - .withArgs("trap-caching") - .returns(undefined); - t.false( - await configUtils.isTrapCachingEnabled( - createFeatures([Feature.OverlayAnalysisDisableTrapCaching]), - OverlayDatabaseMode.Overlay, - ), - ); - }); - }, -); - -test.serial( - "isTrapCachingEnabled: enabled on hosted runner when overlay is None even with feature flag on", - async (t) => { - return await withTmpDir(async (tmpDir) => { - const hostedToolCache = path.join(tmpDir, "hostedtoolcache"); - setupActionsVars(tmpDir, hostedToolCache); - sinon - .stub(actionsUtil, "getOptionalInput") - .withArgs("trap-caching") - .returns(undefined); - t.true( - await configUtils.isTrapCachingEnabled( - createFeatures([Feature.OverlayAnalysisDisableTrapCaching]), - OverlayDatabaseMode.None, - ), - ); + for (const overlayDatabaseMode of [ + OverlayDatabaseMode.Overlay, + OverlayDatabaseMode.OverlayBase, + ]) { + t.false(configUtils.isTrapCachingEnabled(overlayDatabaseMode)); + } }); }, ); diff --git a/src/config-utils.ts b/src/config-utils.ts index de506aac0..7bb7a37c0 100644 --- a/src/config-utils.ts +++ b/src/config-utils.ts @@ -944,10 +944,14 @@ async function validateOverlayDatabaseMode( }); } -export async function isTrapCachingEnabled( - features: FeatureEnablement, +/** + * Determines whether TRAP caching is enabled for the current run. + * + * @returns Whether TRAP caches should be downloaded and uploaded. + */ +export function isTrapCachingEnabled( overlayDatabaseMode: OverlayDatabaseMode, -): Promise { +): boolean { // If the workflow specified something, always respect that. const trapCaching = getOptionalInput("trap-caching"); if (trapCaching !== undefined) return trapCaching === "true"; @@ -955,12 +959,8 @@ export async function isTrapCachingEnabled( // On self-hosted runners which may have slow network access, disable TRAP caching by default. if (!looksLikeHostedRunner()) return false; - // If overlay analysis is enabled, then disable TRAP caching since overlay analysis supersedes it. - // This change is gated behind a feature flag. - if ( - overlayDatabaseMode !== OverlayDatabaseMode.None && - (await features.getValue(Feature.OverlayAnalysisDisableTrapCaching)) - ) { + // Overlay analysis supersedes TRAP caching, so disable TRAP caching by default when it is active. + if (overlayDatabaseMode !== OverlayDatabaseMode.None) { return false; } @@ -1302,7 +1302,7 @@ export async function initConfig( logger, ); - if (await isTrapCachingEnabled(features, config.overlayDatabaseMode)) { + if (isTrapCachingEnabled(config.overlayDatabaseMode)) { const { trapCaches, trapCacheDownloadTime } = await downloadCacheWithTime( inputs.codeql, config.languages, diff --git a/src/feature-flags.ts b/src/feature-flags.ts index 5cf7c7414..a380c1a63 100644 --- a/src/feature-flags.ts +++ b/src/feature-flags.ts @@ -103,8 +103,6 @@ export enum Feature { OverlayAnalysisCodeScanningRuby = "overlay_analysis_code_scanning_ruby", OverlayAnalysisCpp = "overlay_analysis_cpp", OverlayAnalysisCsharp = "overlay_analysis_csharp", - /** Disable TRAP caching when overlay analysis is enabled. */ - OverlayAnalysisDisableTrapCaching = "overlay_analysis_disable_trap_caching", OverlayAnalysisGo = "overlay_analysis_go", OverlayAnalysisJava = "overlay_analysis_java", OverlayAnalysisJavascript = "overlay_analysis_javascript", @@ -369,11 +367,6 @@ export const featureConfig = { minimumVersion: CODEQL_OVERLAY_MINIMUM_VERSION_RUBY, }, // Other overlay-related feature flags - [Feature.OverlayAnalysisDisableTrapCaching]: { - defaultValue: false, - envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS_DISABLE_TRAP_CACHING", - minimumVersion: undefined, - }, [Feature.OverlayAnalysisMatchCodeqlVersion]: { defaultValue: false, envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS_MATCH_CODEQL_VERSION", From 0d2baf6822dc1d5e42b825ffc54fd1abef26fa22 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 16 Oct 2026 15:11:17 +0100 Subject: [PATCH 05/26] Update `copilot-instructions.md` to mention feature flags --- .github/copilot-instructions.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 345b7314d..a6c8176a4 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -22,6 +22,12 @@ The main codebase of the CodeQL Action is written in TypeScript and compiled to GitHub Actions workflows in the `.github/workflows` directory whose filenames start with two underscores (e.g. `__all-platform-bundle.yml`) are automatically generated using the `pr-checks/sync.sh` script from template files in the `pr-checks/checks` directory. Therefore, you do not need to review files in the `.github/workflows` directory that start with two underscores. However, you should review changes to the `pr-checks` directory as well as workflows in the `.github/workflows` directory that do not start with underscores. +## Feature flags + +Some functionality in the code may be gated behind feature flags, which are controlled server-side. If the functionality that is gated behind them is user-facing, then this should be documented in a change note at the time that the gated functionality is added. See `CHANGELOG.md` for existing notes that mention "experimental" for examples. These typically include information about when users may expect to see the feature rolled out. + +When a feature flag that gates some functionality is removed, this does not need to be documented in a change note since the feature is already rolled out and the notes for the release which introduced the feature already describes it. + ## Release process Changes that are merged to the `main` branch are not used in production straight away. Instead, changes from `main` must first be released using the release process, which has to be manually triggered by a staff member. Therefore, it is possible for further changes to be made to the code after some changes have been merged to `main` but before they are released. Once the release process automation is started, a series of PRs are automatically generated: From e344c793ad3665f7e18274b26fda7ccc0e837640 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Fri, 16 Oct 2026 15:28:39 +0100 Subject: [PATCH 06/26] Document TRAP caching default with improved incremental analysis Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- init/action.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/init/action.yml b/init/action.yml index 5b3ab0a1e..8d26999f7 100644 --- a/init/action.yml +++ b/init/action.yml @@ -154,6 +154,9 @@ inputs: trap-caching: description: >- Explicitly enable or disable TRAP caching instead of using the automatically determined default. + TRAP caching is disabled by default when improved incremental analysis is enabled, + because improved incremental analysis of a pull request only extracts changed files, + which rarely benefit from TRAP caching. required: false dependency-caching: description: >- From 2441e6e56e069df64af99d8a6365ca8e3fc4a9ed Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 16 Oct 2026 15:31:56 +0100 Subject: [PATCH 07/26] Update "Review instructions" based on recent reviews --- .github/copilot-instructions.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a6c8176a4..79b885bf0 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -11,7 +11,10 @@ The CodeQL Action is used in GitHub Actions workflows to run CodeQL scans using ## Review instructions -- When wording review comments, be helpful and friendly. Assume that the PR author has written the code with the best of intentions. Word your comments constructively as suggestions for improvements. Do not word suggestions as commands. +- Prioritise identifying critical issues, such as logic bugs, over comments on documentation or typos. +- When wording review comments, be helpful and friendly. Assume that the PR author has written the code with the best of intentions. Never word suggestions as commands, requests, or using the imperative. Examples of what you should never write: "Preserve that non-fatal behaviour here" or "Please preserve that non-fatal behaviour here". +- When writing feedback, assume that the PR author is competent and is able to understand whether the point you are making is valid or not, why you are raising it, and how to best address it. While you can suggest how to address a point you are making, don't phrase it as a requirement or assume that it is the only solution. +- Always word your comments constructively as suggestions for improvements, for example: "It would make sense to ..." or "Could you add ...?" or "Would it make sense to ...?" - If you want to comment on a change that you believe will fail a CI check, do not present the CI failure you expect as a fact. Instead, write that you think a change "may" lead to a failure in CI. Suggest that, if such a failure manifests, the changes you are commenting on may be the place responsible for the failure and are worth looking at. - If a suggestion you make is suitable for a follow-up, such as a refactoring that doesn't change the behaviour or fixing a typo in a comment, mention that it can be addressed in a later PR rather than blocking this one. - If a change is a net improvement, for example because it improves on an existing limitation of existing code, do not complain about pre-existing problems that remain. You may comment on them, but you should make it clear that the thing you are commenting on is not new by writing e.g. "Not new in this PR, but [..]" followed by your description of the issue and a suggestion that it could be improved at the same time with e.g. "Consider whether this is worth addressing as part of this PR as well." From f3c93e31065a7812e6fd3ae0e420902225773370 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 16 Oct 2026 15:32:50 +0100 Subject: [PATCH 08/26] Add some guidelines for change notes --- unreleased-change-notes/README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/unreleased-change-notes/README.md b/unreleased-change-notes/README.md index 60778c795..3030210a7 100644 --- a/unreleased-change-notes/README.md +++ b/unreleased-change-notes/README.md @@ -2,6 +2,16 @@ Change-notes are Markdown files used to document user-facing changes. When making a change that affects users, create a Markdown file here that describes the change. During the next release, the change-note files in `unreleased-change-notes/` will automatically be added to `CHANGELOG.md`. +### What do document + +Some guidelines to help you decide whether a change needs a change note: + +- New functionality that is gated behind a feature flag should be documented when it is added, highlighting that the feature will only be rolled out later. Ideally, this should also include an estimate for when the feature is expected to become available. +- When a feature flag is removed and the gated functionality becomes the default behaviour, this does not need to be documented. +- Changes that may require users to make changes to their workflows must be documented. + +If in doubt, see `CHANGELOG.md` for what we have included previously. + ### Change-note file format Change-note files must follow a certain format so that they can be automatically validated and processed. Failure to follow the format will result in a failed PR check. From 69921559019d093af81d6f06f29ca1565f9935cb Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 16 Oct 2026 16:02:01 +0100 Subject: [PATCH 09/26] Elaborate GHES details --- .github/copilot-instructions.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 79b885bf0..eacd39d5d 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -29,7 +29,11 @@ GitHub Actions workflows in the `.github/workflows` directory whose filenames st Some functionality in the code may be gated behind feature flags, which are controlled server-side. If the functionality that is gated behind them is user-facing, then this should be documented in a change note at the time that the gated functionality is added. See `CHANGELOG.md` for existing notes that mention "experimental" for examples. These typically include information about when users may expect to see the feature rolled out. -When a feature flag that gates some functionality is removed, this does not need to be documented in a change note since the feature is already rolled out and the notes for the release which introduced the feature already describes it. +When a feature flag that gates some functionality is removed, this does not need to be documented in a change note since the feature is already rolled out and the notes for the release which introduced the feature already describe it. The only exception is functionality which will start having an effect on GHES as a result of the feature flag removal: + +- Most functionality gated behind feature flags does not affect GHES. +- We can make functionality available on GHES by setting `defaultValue` to `true` first and waiting for this to have had an effect on GHES. No change note is needed if a feature flag is removed where `defaultValue` was `true`. +- When removing a feature flag where `defaultValue` was `false` and the functionality starts to affect GHES, a change note may be appropriate. ## Release process From e461913b384ad43fafb09d6d2a03f73293f1614c Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 16 Oct 2026 16:03:00 +0100 Subject: [PATCH 10/26] Drop "requests" again --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index eacd39d5d..18ce0cce3 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -12,7 +12,7 @@ The CodeQL Action is used in GitHub Actions workflows to run CodeQL scans using ## Review instructions - Prioritise identifying critical issues, such as logic bugs, over comments on documentation or typos. -- When wording review comments, be helpful and friendly. Assume that the PR author has written the code with the best of intentions. Never word suggestions as commands, requests, or using the imperative. Examples of what you should never write: "Preserve that non-fatal behaviour here" or "Please preserve that non-fatal behaviour here". +- When wording review comments, be helpful and friendly. Assume that the PR author has written the code with the best of intentions. Never word suggestions as commands or using the imperative. Examples of what you should never write: "Preserve that non-fatal behaviour here" or "Please preserve that non-fatal behaviour here". - When writing feedback, assume that the PR author is competent and is able to understand whether the point you are making is valid or not, why you are raising it, and how to best address it. While you can suggest how to address a point you are making, don't phrase it as a requirement or assume that it is the only solution. - Always word your comments constructively as suggestions for improvements, for example: "It would make sense to ..." or "Could you add ...?" or "Would it make sense to ...?" - If you want to comment on a change that you believe will fail a CI check, do not present the CI failure you expect as a fact. Instead, write that you think a change "may" lead to a failure in CI. Suggest that, if such a failure manifests, the changes you are commenting on may be the place responsible for the failure and are worth looking at. From 5ecf3b32e912123b6c137fb4cdc74df67d964199 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 16 Oct 2026 16:03:32 +0100 Subject: [PATCH 11/26] Suggest "Minor" prefix --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 18ce0cce3..9562cf6a4 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -16,7 +16,7 @@ The CodeQL Action is used in GitHub Actions workflows to run CodeQL scans using - When writing feedback, assume that the PR author is competent and is able to understand whether the point you are making is valid or not, why you are raising it, and how to best address it. While you can suggest how to address a point you are making, don't phrase it as a requirement or assume that it is the only solution. - Always word your comments constructively as suggestions for improvements, for example: "It would make sense to ..." or "Could you add ...?" or "Would it make sense to ...?" - If you want to comment on a change that you believe will fail a CI check, do not present the CI failure you expect as a fact. Instead, write that you think a change "may" lead to a failure in CI. Suggest that, if such a failure manifests, the changes you are commenting on may be the place responsible for the failure and are worth looking at. -- If a suggestion you make is suitable for a follow-up, such as a refactoring that doesn't change the behaviour or fixing a typo in a comment, mention that it can be addressed in a later PR rather than blocking this one. +- If a suggestion you make is suitable for a follow-up, such as a refactoring that doesn't change the behaviour or fixing a typo in a comment, mention that it can be addressed in a later PR rather than blocking this one or prefix your comment with "Minor: ". - If a change is a net improvement, for example because it improves on an existing limitation of existing code, do not complain about pre-existing problems that remain. You may comment on them, but you should make it clear that the thing you are commenting on is not new by writing e.g. "Not new in this PR, but [..]" followed by your description of the issue and a suggestion that it could be improved at the same time with e.g. "Consider whether this is worth addressing as part of this PR as well." ## Generated code From 604377fd2d9f2c700a097bf664530b76c3ecacb8 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 16 Oct 2026 14:00:25 +0100 Subject: [PATCH 12/26] Move inline script from `diagnostics-export.yml` to a `.ts` file --- .github/workflows/__diagnostics-export.yml | 60 ++++------------- pr-checks/check-diagnostics.ts | 77 ++++++++++++++++++++++ pr-checks/checks/diagnostics-export.yml | 52 ++------------- 3 files changed, 95 insertions(+), 94 deletions(-) create mode 100644 pr-checks/check-diagnostics.ts diff --git a/.github/workflows/__diagnostics-export.yml b/.github/workflows/__diagnostics-export.yml index 32361c922..930ff0eb4 100644 --- a/.github/workflows/__diagnostics-export.yml +++ b/.github/workflows/__diagnostics-export.yml @@ -48,6 +48,15 @@ jobs: steps: - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Install Node.js + uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version: 20.x + cache: npm + - name: Install newer npm + run: npm install -g npm@11.19.1 + - name: Install dependencies + run: npm ci - name: Prepare test id: prepare-test uses: ./.github/actions/prepare-test @@ -81,56 +90,13 @@ jobs: name: diagnostics-export-${{ matrix.os }}-${{ matrix.version }}.sarif.json path: '${{ runner.temp }}/results/javascript.sarif' retention-days: 7 + - name: Check diagnostics appear in SARIF - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 env: SARIF_PATH: '${{ runner.temp }}/results/javascript.sarif' - with: - script: | - const fs = require('fs'); - - function checkStatusPageNotification(n) { - const expectedMessage = 'Plaintext message'; - if (n.message.text !== expectedMessage) { - core.setFailed(`Expected the status page diagnostic to have the message '${expectedMessage}', but found '${n.message.text}'.`); - } - if (n.locations.length !== 1) { - core.setFailed(`Expected the status page diagnostic to have exactly 1 location, but found ${n.locations.length}.`); - } - } - - const sarif = JSON.parse(fs.readFileSync(process.env['SARIF_PATH'], 'utf8')); - const run = sarif.runs[0]; - - const toolExecutionNotifications = run.invocations[0].toolExecutionNotifications; - const statusPageNotifications = toolExecutionNotifications.filter(n => - n.descriptor.id === 'lang/diagnostics/example' && n.properties?.visibility?.statusPage - ); - if (statusPageNotifications.length !== 1) { - core.setFailed( - 'Expected exactly one status page reporting descriptor for this diagnostic in the ' + - `'runs[].invocations[].toolExecutionNotifications[]' SARIF property, but found ` + - `${statusPageNotifications.length}. All notification reporting descriptors: ` + - `${JSON.stringify(toolExecutionNotifications)}.` - ); - } - checkStatusPageNotification(statusPageNotifications[0]); - - const notifications = run.tool.driver.notifications; - const diagnosticNotification = notifications.filter(n => - n.id === 'lang/diagnostics/example' && n.name === 'lang/diagnostics/example' && - n.fullDescription.text === 'Diagnostic name' - ); - if (diagnosticNotification.length !== 1) { - core.setFailed( - 'Expected exactly one notification for this diagnostic in the ' + - `'runs[].tool.driver.notifications[]' SARIF property, but found ` + - `${diagnosticNotification.length}. All notifications: ` + - `${JSON.stringify(notifications)}.` - ); - } - - core.info('Finished diagnostic export test'); + run: | + cd ../action + npx tsx ./pr-checks/check-diagnostics.ts env: CODEQL_ACTION_EXPORT_DIAGNOSTICS: true CODEQL_ACTION_TEST_MODE: true diff --git a/pr-checks/check-diagnostics.ts b/pr-checks/check-diagnostics.ts new file mode 100644 index 000000000..0a35ac28b --- /dev/null +++ b/pr-checks/check-diagnostics.ts @@ -0,0 +1,77 @@ +import fs from "fs"; + +import * as core from "@actions/core"; + +import * as sarif from "../src/sarif"; + +function checkStatusPageNotification(n) { + const expectedMessage = "Plaintext message"; + if (n.message.text !== expectedMessage) { + core.setFailed( + `Expected the status page diagnostic to have the message '${expectedMessage}', but found '${n.message.text}'.`, + ); + } + if (n.locations.length !== 1) { + core.setFailed( + `Expected the status page diagnostic to have exactly 1 location, but found ${n.locations.length}.`, + ); + } +} + +const sarifPath = process.env["SARIF_PATH"]; + +if (sarifPath === undefined) { + throw new Error("The path to a SARIF file must be set in 'SARIF_PATH'."); +} + +const log = JSON.parse(fs.readFileSync(sarifPath, "utf8")) as sarif.Log; +const run = log.runs[0]; + +if (run.invocations === undefined) { + throw new Error("run.invocations is undefined"); +} + +const toolExecutionNotifications = + run.invocations[0].toolExecutionNotifications; + +if (toolExecutionNotifications === undefined) { + throw new Error("toolExecutionNotifications is undefined"); +} + +const statusPageNotifications = toolExecutionNotifications.filter( + (n) => + n.descriptor?.id === "lang/diagnostics/example" && + n.properties?.visibility?.statusPage, +); +if (statusPageNotifications.length !== 1) { + core.setFailed( + "Expected exactly one status page reporting descriptor for this diagnostic in the " + + `'runs[].invocations[].toolExecutionNotifications[]' SARIF property, but found ` + + `${statusPageNotifications.length}. All notification reporting descriptors: ` + + `${JSON.stringify(toolExecutionNotifications)}.`, + ); +} +checkStatusPageNotification(statusPageNotifications[0]); + +const notifications = run.tool.driver.notifications; + +if (notifications === undefined) { + throw new Error("notifications is undefined"); +} + +const diagnosticNotification = notifications.filter( + (n) => + n.id === "lang/diagnostics/example" && + n.name === "lang/diagnostics/example" && + n.fullDescription?.text === "Diagnostic name", +); +if (diagnosticNotification.length !== 1) { + core.setFailed( + "Expected exactly one notification for this diagnostic in the " + + `'runs[].tool.driver.notifications[]' SARIF property, but found ` + + `${diagnosticNotification.length}. All notifications: ` + + `${JSON.stringify(notifications)}.`, + ); +} + +core.info("Finished diagnostic export test"); diff --git a/pr-checks/checks/diagnostics-export.yml b/pr-checks/checks/diagnostics-export.yml index 93b3a8f60..0548531b9 100644 --- a/pr-checks/checks/diagnostics-export.yml +++ b/pr-checks/checks/diagnostics-export.yml @@ -5,6 +5,7 @@ versions: - nightly-latest env: CODEQL_ACTION_EXPORT_DIAGNOSTICS: true +installNode: true steps: - uses: ./../action/init id: init @@ -32,53 +33,10 @@ steps: name: diagnostics-export-${{ matrix.os }}-${{ matrix.version }}.sarif.json path: "${{ runner.temp }}/results/javascript.sarif" retention-days: 7 + - name: Check diagnostics appear in SARIF - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 env: SARIF_PATH: "${{ runner.temp }}/results/javascript.sarif" - with: - script: | - const fs = require('fs'); - - function checkStatusPageNotification(n) { - const expectedMessage = 'Plaintext message'; - if (n.message.text !== expectedMessage) { - core.setFailed(`Expected the status page diagnostic to have the message '${expectedMessage}', but found '${n.message.text}'.`); - } - if (n.locations.length !== 1) { - core.setFailed(`Expected the status page diagnostic to have exactly 1 location, but found ${n.locations.length}.`); - } - } - - const sarif = JSON.parse(fs.readFileSync(process.env['SARIF_PATH'], 'utf8')); - const run = sarif.runs[0]; - - const toolExecutionNotifications = run.invocations[0].toolExecutionNotifications; - const statusPageNotifications = toolExecutionNotifications.filter(n => - n.descriptor.id === 'lang/diagnostics/example' && n.properties?.visibility?.statusPage - ); - if (statusPageNotifications.length !== 1) { - core.setFailed( - 'Expected exactly one status page reporting descriptor for this diagnostic in the ' + - `'runs[].invocations[].toolExecutionNotifications[]' SARIF property, but found ` + - `${statusPageNotifications.length}. All notification reporting descriptors: ` + - `${JSON.stringify(toolExecutionNotifications)}.` - ); - } - checkStatusPageNotification(statusPageNotifications[0]); - - const notifications = run.tool.driver.notifications; - const diagnosticNotification = notifications.filter(n => - n.id === 'lang/diagnostics/example' && n.name === 'lang/diagnostics/example' && - n.fullDescription.text === 'Diagnostic name' - ); - if (diagnosticNotification.length !== 1) { - core.setFailed( - 'Expected exactly one notification for this diagnostic in the ' + - `'runs[].tool.driver.notifications[]' SARIF property, but found ` + - `${diagnosticNotification.length}. All notifications: ` + - `${JSON.stringify(notifications)}.` - ); - } - - core.info('Finished diagnostic export test'); + run: | + cd ../action + npx tsx ./pr-checks/check-diagnostics.ts From be1223f63ec6519618c55fb853646cfce4c5192c Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 12 Oct 2026 17:24:21 +0100 Subject: [PATCH 13/26] Log when we didn't get a normal configuration file --- lib/entry-points.js | 3 +++ src/init-action-post.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/lib/entry-points.js b/lib/entry-points.js index 1a99a4442..1300b53b0 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -187232,6 +187232,9 @@ async function run4(action) { const tmpDir = getTemporaryDirectory(); config = await getConfig(tmpDir, logger2); if (config === void 0) { + logger2.debug( + "Will try to load the configuration file from an aborted 'init' step, since there is no regular one." + ); config = getAbortedInitConfig(action, tmpDir); } const gitHubVersion = config?.gitHubVersion ?? await getGitHubVersion(); diff --git a/src/init-action-post.ts b/src/init-action-post.ts index 88aac4622..071137f54 100644 --- a/src/init-action-post.ts +++ b/src/init-action-post.ts @@ -76,6 +76,9 @@ async function run(action: ActionState<["Base", "Logger", "Env"]>) { // Try to load the "failed" configuration file instead which may have been written by the `init` step // with a minimal configuration for this post-action. if (config === undefined) { + logger.debug( + "Will try to load the configuration file from an aborted 'init' step, since there is no regular one.", + ); config = getAbortedInitConfig(action, tmpDir); } From a9cd2f9fa7d36f998863794b3c96ad9cec35db22 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Thu, 15 Oct 2026 14:35:40 +0100 Subject: [PATCH 14/26] Allow to explicitly not skip the SARIF upload --- lib/entry-points.js | 5 +++-- src/util.test.ts | 30 +++++++++++++++++++++++++++++- src/util.ts | 10 +++++++--- 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/lib/entry-points.js b/lib/entry-points.js index 1300b53b0..72559eea9 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -169889,8 +169889,9 @@ async function delay(milliseconds, opts) { function isGoodVersion(versionSpec) { return !BROKEN_VERSIONS.includes(versionSpec); } -function shouldSkipSarifUpload() { - return isInTestMode() || process.env["CODEQL_ACTION_SKIP_SARIF_UPLOAD" /* SKIP_SARIF_UPLOAD */] === "true"; +function shouldSkipSarifUpload(env = getEnv()) { + const skipSarifUpload = env.getOptional("CODEQL_ACTION_SKIP_SARIF_UPLOAD" /* SKIP_SARIF_UPLOAD */); + return skipSarifUpload !== "false" && (env.isSkippingUploadsInTests() || skipSarifUpload === "true"); } function getTestingEnvironment() { const testingEnvironment = process.env["CODEQL_ACTION_TESTING_ENVIRONMENT" /* TESTING_ENVIRONMENT */] || ""; diff --git a/src/util.test.ts b/src/util.test.ts index bca2b452e..1a1f7677e 100644 --- a/src/util.test.ts +++ b/src/util.test.ts @@ -11,7 +11,7 @@ import * as sinon from "sinon"; import * as api from "./api-client"; import { EnvVar } from "./environment"; import { getRunnerLogger } from "./logging"; -import { setupTests } from "./testing-utils"; +import { getTestEnv, setupTests } from "./testing-utils"; import * as util from "./util"; setupTests(test); @@ -493,3 +493,31 @@ test("Failure.orElse returns the default value for a failure result", (t) => { const result = new util.Failure(new Error("test error")); t.is(result.orElse("default value"), "default value"); }); + +test("shouldSkipSarifUpload", (t) => { + const env = getTestEnv(); + + // Don't skip by default + t.is(util.shouldSkipSarifUpload(env), false); + + // Skip in test mode + env.setAll({ + [EnvVar.TEST_MODE]: "true", + [EnvVar.SKIP_SARIF_UPLOAD]: undefined, + }); + t.is(util.shouldSkipSarifUpload(env), true); + + // Skip if explicitly requested + env.setAll({ + [EnvVar.TEST_MODE]: undefined, + [EnvVar.SKIP_SARIF_UPLOAD]: "true", + }); + t.is(util.shouldSkipSarifUpload(env), true); + + // Don't skip if explicitly requested + env.setAll({ + [EnvVar.TEST_MODE]: "true", + [EnvVar.SKIP_SARIF_UPLOAD]: "false", + }); + t.is(util.shouldSkipSarifUpload(env), false); +}); diff --git a/src/util.ts b/src/util.ts index 02e105d57..073ecf0a0 100644 --- a/src/util.ts +++ b/src/util.ts @@ -18,10 +18,10 @@ import type { Config } from "./config-utils"; import { EnvVar, getRequiredEnvParam, - isInTestMode, exportEnvVar, getEnv, type Env, + type ReadOnlyEnv, } from "./environment"; import * as json from "./json"; import { Language } from "./languages"; @@ -687,8 +687,12 @@ export function isGoodVersion(versionSpec: string) { /** * Returns whether we specifically want to skip uploading SARIF files. */ -export function shouldSkipSarifUpload(): boolean { - return isInTestMode() || process.env[EnvVar.SKIP_SARIF_UPLOAD] === "true"; +export function shouldSkipSarifUpload(env: ReadOnlyEnv = getEnv()): boolean { + const skipSarifUpload = env.getOptional(EnvVar.SKIP_SARIF_UPLOAD); + return ( + skipSarifUpload !== "false" && + (env.isSkippingUploadsInTests() || skipSarifUpload === "true") + ); } /** From 6e9aec682d5a2cb314aac734a431d598d774e438 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Thu, 15 Oct 2026 14:40:34 +0100 Subject: [PATCH 15/26] Add and use `shouldSkipGenerateFailedSarif` --- lib/entry-points.js | 8 ++++++- src/environment.ts | 7 ++++++ src/init-action-post-helper.ts | 4 ++-- src/util.test.ts | 39 ++++++++++++++++++++++++++++++++++ src/util.ts | 15 +++++++++++++ 5 files changed, 70 insertions(+), 3 deletions(-) diff --git a/lib/entry-points.js b/lib/entry-points.js index 72559eea9..6a1e50a4c 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -169889,6 +169889,12 @@ async function delay(milliseconds, opts) { function isGoodVersion(versionSpec) { return !BROKEN_VERSIONS.includes(versionSpec); } +function shouldSkipGenerateFailedSarif(env = getEnv()) { + const skipGenerateFailedSarif = env.getOptional( + "CODEQL_ACTION_SKIP_GENERATE_FAILED_SARIF" /* SKIP_GENERATE_FAILED_SARIF */ + ); + return skipGenerateFailedSarif !== "false" && (shouldSkipSarifUpload(env) || skipGenerateFailedSarif === "true"); +} function shouldSkipSarifUpload(env = getEnv()) { const skipSarifUpload = env.getOptional("CODEQL_ACTION_SKIP_SARIF_UPLOAD" /* SKIP_SARIF_UPLOAD */); return skipSarifUpload !== "false" && (env.isSkippingUploadsInTests() || skipSarifUpload === "true"); @@ -186904,7 +186910,7 @@ async function prepareFailedSarif(logger2, features, config) { } const jobName = getRequiredEnvParam("GITHUB_JOB"); const matrix = parseMatrixInput(getRequiredInput("matrix")); - if (shouldSkipSarifUpload()) { + if (shouldSkipGenerateFailedSarif()) { return new Failure({ upload_failed_run_skipped_because: "SARIF upload is disabled" }); diff --git a/src/environment.ts b/src/environment.ts index 9f6107025..32140baf0 100644 --- a/src/environment.ts +++ b/src/environment.ts @@ -165,6 +165,13 @@ export enum EnvVar { */ SARIF_DUMP_DIR = "CODEQL_ACTION_SARIF_DUMP_DIR", + /** + * Whether to skip generating a failed SARIF file. Intended for testing purposes. + * This setting is more specific than `CODEQL_ACTION_TEST_MODE` and + * `CODEQL_ACTION_SKIP_SARIF_UPLOAD`, which imply this option. + */ + SKIP_GENERATE_FAILED_SARIF = "CODEQL_ACTION_SKIP_GENERATE_FAILED_SARIF", + /** * Whether to skip uploading SARIF results to GitHub. Intended for testing purposes. * This setting is more specific than `CODEQL_ACTION_TEST_MODE`, which implies this option. diff --git a/src/init-action-post-helper.ts b/src/init-action-post-helper.ts index f9021b789..38ff07871 100644 --- a/src/init-action-post-helper.ts +++ b/src/init-action-post-helper.ts @@ -44,7 +44,7 @@ import { getRequiredEnvParam, parseMatrixInput, Result, - shouldSkipSarifUpload, + shouldSkipGenerateFailedSarif, Success, wrapError, } from "./util"; @@ -115,7 +115,7 @@ async function prepareFailedSarif( const jobName = getRequiredEnvParam("GITHUB_JOB"); const matrix = parseMatrixInput(actionsUtil.getRequiredInput("matrix")); - if (shouldSkipSarifUpload()) { + if (shouldSkipGenerateFailedSarif()) { return new Failure({ upload_failed_run_skipped_because: "SARIF upload is disabled", }); diff --git a/src/util.test.ts b/src/util.test.ts index 1a1f7677e..e7a7d3abd 100644 --- a/src/util.test.ts +++ b/src/util.test.ts @@ -494,6 +494,45 @@ test("Failure.orElse returns the default value for a failure result", (t) => { t.is(result.orElse("default value"), "default value"); }); +test("shouldSkipGenerateFailedSarif", (t) => { + const env = getTestEnv(); + + // Don't skip by default + t.is(util.shouldSkipGenerateFailedSarif(env), false); + + // Skip in test mode + env.setAll({ + [EnvVar.TEST_MODE]: "true", + [EnvVar.SKIP_SARIF_UPLOAD]: undefined, + [EnvVar.SKIP_GENERATE_FAILED_SARIF]: undefined, + }); + t.is(util.shouldSkipGenerateFailedSarif(env), true); + + // Skip if explicitly requested + env.setAll({ + [EnvVar.TEST_MODE]: undefined, + [EnvVar.SKIP_SARIF_UPLOAD]: undefined, + [EnvVar.SKIP_GENERATE_FAILED_SARIF]: "true", + }); + t.is(util.shouldSkipGenerateFailedSarif(env), true); + + // Skip if not uploading SARIF + env.setAll({ + [EnvVar.TEST_MODE]: undefined, + [EnvVar.SKIP_SARIF_UPLOAD]: "true", + [EnvVar.SKIP_GENERATE_FAILED_SARIF]: undefined, + }); + t.is(util.shouldSkipGenerateFailedSarif(env), true); + + // Don't skip if explicitly requested + env.setAll({ + [EnvVar.TEST_MODE]: "true", + [EnvVar.SKIP_SARIF_UPLOAD]: "true", + [EnvVar.SKIP_GENERATE_FAILED_SARIF]: "false", + }); + t.is(util.shouldSkipGenerateFailedSarif(env), false); +}); + test("shouldSkipSarifUpload", (t) => { const env = getTestEnv(); diff --git a/src/util.ts b/src/util.ts index 073ecf0a0..ba0c3c7d4 100644 --- a/src/util.ts +++ b/src/util.ts @@ -684,6 +684,21 @@ export function isGoodVersion(versionSpec: string) { return !BROKEN_VERSIONS.includes(versionSpec); } +/** + * Returns whether we specifically want to skip generating a failed SARIF. + */ +export function shouldSkipGenerateFailedSarif( + env: ReadOnlyEnv = getEnv(), +): boolean { + const skipGenerateFailedSarif = env.getOptional( + EnvVar.SKIP_GENERATE_FAILED_SARIF, + ); + return ( + skipGenerateFailedSarif !== "false" && + (shouldSkipSarifUpload(env) || skipGenerateFailedSarif === "true") + ); +} + /** * Returns whether we specifically want to skip uploading SARIF files. */ From 02fb8cc38a65e2d5fb2e750982c55ee388c723fb Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Thu, 15 Oct 2026 17:13:58 +0100 Subject: [PATCH 16/26] Propagate `Config` to `combineSarifFilesUsingCLI` --- lib/entry-points.js | 24 ++++++++++++++++-------- src/analyze-action.ts | 1 + src/init-action-post-helper.test.ts | 1 + src/init-action-post-helper.ts | 1 + src/upload-lib.ts | 21 +++++++++++++++------ src/upload-sarif-action.ts | 10 +++++++++- src/upload-sarif.test.ts | 6 ++++++ src/upload-sarif.ts | 3 +++ 8 files changed, 52 insertions(+), 15 deletions(-) diff --git a/lib/entry-points.js b/lib/entry-points.js index 6a1e50a4c..28d1694c1 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -180021,7 +180021,7 @@ async function shouldDisableCombineSarifFiles(sarifObjects, githubVersion) { } return true; } -async function combineSarifFilesUsingCLI(sarifFiles, gitHubVersion, features, logger2) { +async function combineSarifFilesUsingCLI(sarifFiles, gitHubVersion, features, logger2, config) { const env = getEnv(); logger2.info("Combining SARIF files using the CodeQL CLI"); const sarifObjects = sarifFiles.map(readSarifFile); @@ -180042,7 +180042,6 @@ async function combineSarifFilesUsingCLI(sarifFiles, gitHubVersion, features, lo } let codeQL; let tempDir = getTemporaryDirectory(); - const config = await getConfig(tempDir, logger2); if (config !== void 0) { codeQL = await getCodeQL(logger2, config.codeQLCmd); tempDir = config.tempDir; @@ -180327,7 +180326,7 @@ function buildPayload(commitOid, ref, analysisKey, analysisName, zippedSarif, wo } return payloadObj; } -async function postProcessSarifFiles(logger2, features, checkoutPath, sarifPaths, category, analysis) { +async function postProcessSarifFiles(logger2, features, config, checkoutPath, sarifPaths, category, analysis) { logger2.info(`Post-processing sarif files: ${JSON.stringify(sarifPaths)}`); const gitHubVersion = await getGitHubVersion(); let sarifLog; @@ -180341,7 +180340,8 @@ async function postProcessSarifFiles(logger2, features, checkoutPath, sarifPaths sarifPaths, gitHubVersion, features, - logger2 + logger2, + config ); } else { const sarifPath = sarifPaths[0]; @@ -180374,13 +180374,14 @@ async function writePostProcessedFiles(logger2, pathInput, uploadTarget, postPro logger2.debug(`Not writing post-processed SARIF files.`); } } -async function uploadFiles(inputSarifPath, checkoutPath, category, features, logger2, uploadTarget) { +async function uploadFiles(config, inputSarifPath, checkoutPath, category, features, logger2, uploadTarget) { const sarifPaths = getSarifFilePaths( inputSarifPath, uploadTarget.sarifPredicate ); return uploadSpecifiedFiles( sarifPaths, + config, checkoutPath, category, features, @@ -180388,10 +180389,11 @@ async function uploadFiles(inputSarifPath, checkoutPath, category, features, log uploadTarget ); } -async function uploadSpecifiedFiles(sarifPaths, checkoutPath, category, features, logger2, uploadTarget) { +async function uploadSpecifiedFiles(sarifPaths, config, checkoutPath, category, features, logger2, uploadTarget) { const processingResults = await postProcessSarifFiles( logger2, features, + config, checkoutPath, sarifPaths, category, @@ -180621,7 +180623,7 @@ function filterAlertsByDiffRange(logger2, sarifLog) { } // src/upload-sarif.ts -async function postProcessAndUploadSarif(logger2, features, uploadKind, checkoutPath, sarifPath, category, postProcessedOutputPath) { +async function postProcessAndUploadSarif(logger2, features, config, uploadKind, checkoutPath, sarifPath, category, postProcessedOutputPath) { const sarifGroups = await getGroupedSarifFilePaths( logger2, sarifPath @@ -180634,6 +180636,7 @@ async function postProcessAndUploadSarif(logger2, features, uploadKind, checkout const postProcessingResults = await postProcessSarifFiles( logger2, features, + config, checkoutPath, sarifFiles, category, @@ -180864,6 +180867,7 @@ async function run(action) { uploadResults = await postProcessAndUploadSarif( logger2, features, + config, uploadKind, checkoutPath, outputDir, @@ -186981,6 +186985,7 @@ async function maybeUploadFailedSarif(config, repositoryNwo, features, logger2) const failedSarif = failedSarifResult.value; logger2.info(`Uploading failed SARIF file ${failedSarif.sarifFile}`); const uploadResult = await uploadFiles( + config, failedSarif.sarifFile, failedSarif.checkoutPath, failedSarif.category, @@ -188460,11 +188465,12 @@ async function run8({ startedAt, logger: logger2 }) { const gitHubVersion = await getGitHubVersion(); checkActionVersion(getActionVersion(), gitHubVersion); persistInputs(); + const tempDir = getTemporaryDirectory(); const repositoryNwo = getRepositoryNwo(); const features = initFeatures( gitHubVersion, repositoryNwo, - getTemporaryDirectory(), + tempDir, logger2 ); const startingStatusReportBase = await createStatusReportBase( @@ -188481,9 +188487,11 @@ async function run8({ startedAt, logger: logger2 }) { const sarifPath = getRequiredInput("sarif_file"); const checkoutPath = getRequiredInput("checkout_path"); const category = getOptionalInput("category"); + const maybeConfig = await getConfig(tempDir, logger2); const uploadResults = await postProcessAndUploadSarif( logger2, features, + maybeConfig, "always", checkoutPath, sarifPath, diff --git a/src/analyze-action.ts b/src/analyze-action.ts index 9efbb529b..a11877b38 100644 --- a/src/analyze-action.ts +++ b/src/analyze-action.ts @@ -361,6 +361,7 @@ async function run(action: ActionState<["Base", "Logger", "Env", "Actions"]>) { uploadResults = await postProcessAndUploadSarif( logger, features, + config, uploadKind, checkoutPath, outputDir, diff --git a/src/init-action-post-helper.test.ts b/src/init-action-post-helper.test.ts index fd1f489f7..554add71f 100644 --- a/src/init-action-post-helper.test.ts +++ b/src/init-action-post-helper.test.ts @@ -793,6 +793,7 @@ async function testFailedSarifUpload( } t.true( uploadFiles.calledOnceWith( + config, sinon.match.string, sinon.match.string, category, diff --git a/src/init-action-post-helper.ts b/src/init-action-post-helper.ts index 38ff07871..b7b4da8e5 100644 --- a/src/init-action-post-helper.ts +++ b/src/init-action-post-helper.ts @@ -226,6 +226,7 @@ async function maybeUploadFailedSarif( logger.info(`Uploading failed SARIF file ${failedSarif.sarifFile}`); const uploadResult = await uploadLib.uploadFiles( + config, failedSarif.sarifFile, failedSarif.checkoutPath, failedSarif.category, diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 3c46c1519..7c6f2afea 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -12,7 +12,7 @@ import * as analyses from "./analyses"; import * as api from "./api-client"; import { getGitHubVersion, wrapApiConfigurationError } from "./api-client"; import { CodeQL, getCodeQL } from "./codeql"; -import { getConfig } from "./config-utils"; +import type { ActionConfig } from "./config/action-config"; import { readDiffRangesJsonFile } from "./diff-informed-analysis-utils"; import { ActionsEnvVars, EnvVar, exportEnvVar } from "./environment"; import { FeatureEnablement } from "./feature-flags"; @@ -94,15 +94,19 @@ async function shouldDisableCombineSarifFiles( return true; } -// Takes a list of paths to sarif files and combines them together using the -// CLI `github merge-results` command when all SARIF files are produced by -// CodeQL. Otherwise, it will fall back to combining the files in the action. -// Returns the contents of the combined sarif file. +/** + * Takes a list of paths to sarif files and combines them together using the + * CLI `github merge-results` command when all SARIF files are produced by + * CodeQL. Otherwise, it will fall back to combining the files in the action. + * + * @returns The contents of the combined sarif file. + */ async function combineSarifFilesUsingCLI( sarifFiles: string[], gitHubVersion: GitHubVersion, features: FeatureEnablement, logger: Logger, + config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined, ): Promise> { const env = util.getEnv(); @@ -140,7 +144,6 @@ async function combineSarifFilesUsingCLI( let codeQL: CodeQL; let tempDir: string = actionsUtil.getTemporaryDirectory(); - const config = await getConfig(tempDir, logger); if (config !== undefined) { codeQL = await getCodeQL(logger, config.codeQLCmd); tempDir = config.tempDir; @@ -599,6 +602,7 @@ export interface PostProcessingResults { export async function postProcessSarifFiles( logger: Logger, features: FeatureEnablement, + config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined, checkoutPath: string, sarifPaths: string[], category: string | undefined, @@ -623,6 +627,7 @@ export async function postProcessSarifFiles( gitHubVersion, features, logger, + config, ); } else { const sarifPath = sarifPaths[0]; @@ -683,6 +688,7 @@ export async function writePostProcessedFiles( * to. */ export async function uploadFiles( + config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined, inputSarifPath: string, checkoutPath: string, category: string | undefined, @@ -697,6 +703,7 @@ export async function uploadFiles( return uploadSpecifiedFiles( sarifPaths, + config, checkoutPath, category, features, @@ -710,6 +717,7 @@ export async function uploadFiles( */ async function uploadSpecifiedFiles( sarifPaths: string[], + config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined, checkoutPath: string, category: string | undefined, features: FeatureEnablement, @@ -719,6 +727,7 @@ async function uploadSpecifiedFiles( const processingResults: PostProcessingResults = await postProcessSarifFiles( logger, features, + config, checkoutPath, sarifPaths, category, diff --git a/src/upload-sarif-action.ts b/src/upload-sarif-action.ts index d3437510c..52dd12ec0 100644 --- a/src/upload-sarif-action.ts +++ b/src/upload-sarif-action.ts @@ -5,6 +5,7 @@ import * as actionsUtil from "./actions-util"; import { getActionVersion, getTemporaryDirectory } from "./actions-util"; import * as analyses from "./analyses"; import { getGitHubVersion } from "./api-client"; +import { getConfig } from "./config-utils"; import { initFeatures } from "./feature-flags"; import { Logger } from "./logging"; import { getRepositoryNwo } from "./repository"; @@ -66,11 +67,12 @@ async function run({ startedAt, logger }: ActionState<["Base", "Logger"]>) { // Make inputs accessible in the `post` step. actionsUtil.persistInputs(); + const tempDir = getTemporaryDirectory(); const repositoryNwo = getRepositoryNwo(); const features = initFeatures( gitHubVersion, repositoryNwo, - getTemporaryDirectory(), + tempDir, logger, ); @@ -91,9 +93,15 @@ async function run({ startedAt, logger }: ActionState<["Base", "Logger"]>) { const checkoutPath = actionsUtil.getRequiredInput("checkout_path"); const category = actionsUtil.getOptionalInput("category"); + // Although we wouldn't typically expect there to be a configuration since we don't expect another + // CodeQL Action step to have come before `upload-sarif`, this has previously been the behaviour + // when `combineSarifFilesUsingCLI` called `getConfig` directly. Since the only effect of this would + // be that an existing CLI that was installed gets re-used, we maintain this behaviour. + const maybeConfig = await getConfig(tempDir, logger); const uploadResults = await postProcessAndUploadSarif( logger, features, + maybeConfig, "always", checkoutPath, sarifPath, diff --git a/src/upload-sarif.test.ts b/src/upload-sarif.test.ts index d310bce92..033be70d0 100644 --- a/src/upload-sarif.test.ts +++ b/src/upload-sarif.test.ts @@ -26,6 +26,7 @@ function mockPostProcessSarifFiles() { const analysisConfig = getAnalysisConfig(analysisKind); postProcessSarifFiles .withArgs( + sinon.match.object, sinon.match.any, sinon.match.any, sinon.match.any, @@ -75,9 +76,11 @@ const postProcessAndUploadSarifMacro = makeMacro({ fs.writeFileSync(sarifFile, ""); } + const config = undefined; const actual = await postProcessAndUploadSarif( logger, features, + config, "always", "", testPath, @@ -94,6 +97,7 @@ const postProcessAndUploadSarifMacro = makeMacro({ postProcessSarifFiles.calledWith( logger, features, + config, sinon.match.any, analysisKindResult.expectedFiles?.map(toFullPath) ?? fullSarifPaths, @@ -223,6 +227,7 @@ test.serial( const actual = await postProcessAndUploadSarif( logger, features, + undefined, "never", "", tempDir, @@ -253,6 +258,7 @@ test.serial( const actual = await postProcessAndUploadSarif( logger, features, + undefined, "never", "", tempDir, diff --git a/src/upload-sarif.ts b/src/upload-sarif.ts index bc2c88698..0a8d8a14a 100644 --- a/src/upload-sarif.ts +++ b/src/upload-sarif.ts @@ -1,5 +1,6 @@ import { UploadKind } from "./actions-util"; import * as analyses from "./analyses"; +import type { ActionConfig } from "./config/action-config"; import { FeatureEnablement } from "./feature-flags"; import { Logger } from "./logging"; import * as upload_lib from "./upload-lib"; @@ -26,6 +27,7 @@ export type UploadSarifResults = Partial< export async function postProcessAndUploadSarif( logger: Logger, features: FeatureEnablement, + config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined, uploadKind: UploadKind, checkoutPath: string, sarifPath: string, @@ -45,6 +47,7 @@ export async function postProcessAndUploadSarif( const postProcessingResults = await upload_lib.postProcessSarifFiles( logger, features, + config, checkoutPath, sarifFiles, category, From e7340eea1b5895c30f5df0120ab216b178008119 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 16 Oct 2026 13:06:58 +0100 Subject: [PATCH 17/26] Skip waiting for SARIF if nothing was uploaded --- lib/entry-points.js | 6 ++++++ src/upload-lib.ts | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/lib/entry-points.js b/lib/entry-points.js index 28d1694c1..ea4f7c9be 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -180478,6 +180478,12 @@ async function waitForProcessing(repositoryNwo, sarifID, logger2, options = { }) { logger2.startGroup("Waiting for processing to finish"); try { + if (shouldSkipSarifUpload()) { + logger2.info( + "Nothing to wait for, because SARIF upload is disabled by an environment variable and the SARIF was saved to disk." + ); + return; + } const client = getApiClient(); let statusCheckBackoff = STATUS_CHECK_INITIAL_BACKOFF_MILLISECONDS; if (process.env["NODE_ENV"] !== "test") { diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 7c6f2afea..7b0a17b30 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -866,6 +866,16 @@ export async function waitForProcessing( ): Promise { logger.startGroup("Waiting for processing to finish"); try { + // If SARIF uplaod is disabled and we reach this point, then the file was written to disk and + // there is nothing to wait for. We still allow ourselves to get to this point so that we can + // see in the log that we would have waited for processing to finish if the upload had happened. + if (util.shouldSkipSarifUpload()) { + logger.info( + "Nothing to wait for, because SARIF upload is disabled by an environment variable and the SARIF was saved to disk.", + ); + return; + } + const client = api.getApiClient(); // Do an initial wait because processing will always take a minimum of 2-3 seconds From 1d373e07cd06e258526e54ed5e84a989e6efa9de Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 12 Oct 2026 17:23:44 +0100 Subject: [PATCH 18/26] Upload failed SARIF as workflow artifact in test mode --- lib/entry-points.js | 20 +++++++++++++++----- src/init-action-post-helper.ts | 19 ++++++++++++++++++- src/upload-lib.ts | 13 +++++++++---- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/lib/entry-points.js b/lib/entry-points.js index ea4f7c9be..05694644e 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -178546,6 +178546,7 @@ __export(upload_lib_exports, { filterAlertsByDiffRange: () => filterAlertsByDiffRange, findSarifFilesInDir: () => findSarifFilesInDir, getGroupedSarifFilePaths: () => getGroupedSarifFilePaths, + getSavedPayloadPath: () => getSavedPayloadPath, populateRunAutomationDetails: () => populateRunAutomationDetails, postProcessSarifFiles: () => postProcessSarifFiles, readSarifFileOrThrow: () => readSarifFileOrThrow, @@ -180108,13 +180109,16 @@ function getAutomationID2(category, analysis_key, environment) { } return computeAutomationID(analysis_key, environment); } +function getSavedPayloadPath(analysisKind) { + return path19.join( + getTemporaryDirectory(), + `payload-${analysisKind}.json` + ); +} async function uploadPayload(payload, repositoryNwo, logger2, analysis) { logger2.info("Uploading results"); if (shouldSkipSarifUpload()) { - const payloadSaveFile = path19.join( - getTemporaryDirectory(), - `payload-${analysis.kind}.json` - ); + const payloadSaveFile = getSavedPayloadPath(analysis.kind); logger2.info( `SARIF upload disabled by an environment variable. Saving to ${payloadSaveFile}` ); @@ -187005,6 +187009,9 @@ async function maybeUploadFailedSarif(config, repositoryNwo, features, logger2) logger2, { isUnsuccessfulExecution: true } ); + if (uploadResult.sarifID && shouldSkipSarifUpload()) { + await uploadSarifArtifact(logger2, "test-mode-sarif", failedSarif); + } return uploadResult ? { ...uploadResult.statusReport, sarifID: uploadResult.sarifID } : {}; } async function maybeUploadFailedSarifArtifact(config, features, logger2) { @@ -187013,13 +187020,16 @@ async function maybeUploadFailedSarifArtifact(config, features, logger2) { return failedSarifResult.value; } const failedSarif = failedSarifResult.value; + return uploadSarifArtifact(logger2, "sarif-artifact", failedSarif); +} +async function uploadSarifArtifact(logger2, prefix, failedSarif) { logger2.info( `Uploading failed SARIF file ${failedSarif.sarifFile} as artifact` ); const gitHubVersion = await getGitHubVersion(); const client = await getArtifactUploaderClient(logger2, gitHubVersion.type); const suffix = getArtifactSuffix(getOptionalInput("matrix")); - const name = sanitizeArtifactName(`sarif-artifact-${suffix}`); + const name = sanitizeArtifactName(`${prefix}-${suffix}`); await client.uploadArtifact( name, [import_path8.default.normalize(failedSarif.sarifFile)], diff --git a/src/init-action-post-helper.ts b/src/init-action-post-helper.ts index b7b4da8e5..0f0062ca7 100644 --- a/src/init-action-post-helper.ts +++ b/src/init-action-post-helper.ts @@ -45,6 +45,7 @@ import { parseMatrixInput, Result, shouldSkipGenerateFailedSarif, + shouldSkipSarifUpload, Success, wrapError, } from "./util"; @@ -240,6 +241,14 @@ async function maybeUploadFailedSarif( logger, { isUnsuccessfulExecution: true }, ); + + // If SARIF upload is disabled for testing, `uploadFiles` will not have uploaded the failed + // SARIF file and instead will have written it to disk. Upload it as a workflow artifact so that + // we can inspect it in validation jobs. + if (uploadResult.sarifID && shouldSkipSarifUpload()) { + await uploadSarifArtifact(logger, "test-mode-sarif", failedSarif); + } + return uploadResult ? { ...uploadResult.statusReport, sarifID: uploadResult.sarifID } : {}; @@ -258,7 +267,15 @@ async function maybeUploadFailedSarifArtifact( } const failedSarif = failedSarifResult.value; + return uploadSarifArtifact(logger, "sarif-artifact", failedSarif); +} +/** Uploads a failed SARIF file as workflow artifact. */ +async function uploadSarifArtifact( + logger: Logger, + prefix: string, + failedSarif: FailedSarifInfo, +): Promise { logger.info( `Uploading failed SARIF file ${failedSarif.sarifFile} as artifact`, ); @@ -267,7 +284,7 @@ async function maybeUploadFailedSarifArtifact( const client = await getArtifactUploaderClient(logger, gitHubVersion.type); const suffix = getArtifactSuffix(actionsUtil.getOptionalInput("matrix")); - const name = sanitizeArtifactName(`sarif-artifact-${suffix}`); + const name = sanitizeArtifactName(`${prefix}-${suffix}`); await client.uploadArtifact( name, [path.normalize(failedSarif.sarifFile)], diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 7b0a17b30..b327054fb 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -231,6 +231,14 @@ function getAutomationID( return api.computeAutomationID(analysis_key, environment); } +/** Gets the path to a saved payload file for {@link analysisKind}. */ +export function getSavedPayloadPath(analysisKind: analyses.AnalysisKind) { + return path.join( + actionsUtil.getTemporaryDirectory(), + `payload-${analysisKind}.json`, + ); +} + /** * Upload the given payload. * If the request fails then this will retry a small number of times. @@ -245,10 +253,7 @@ export async function uploadPayload( logger.info("Uploading results"); if (util.shouldSkipSarifUpload()) { - const payloadSaveFile = path.join( - actionsUtil.getTemporaryDirectory(), - `payload-${analysis.kind}.json`, - ); + const payloadSaveFile = getSavedPayloadPath(analysis.kind); logger.info( `SARIF upload disabled by an environment variable. Saving to ${payloadSaveFile}`, ); From 5b69b28796f47682d750572e7c4649b8edb1508f Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 16 Oct 2026 16:59:56 +0100 Subject: [PATCH 19/26] Improve logging in `uploadSarifArtifact` --- lib/entry-points.js | 29 ++++++++++++++++------------- src/init-action-post-helper.ts | 29 ++++++++++++++++------------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/lib/entry-points.js b/lib/entry-points.js index 05694644e..5ad162ce3 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -187023,19 +187023,22 @@ async function maybeUploadFailedSarifArtifact(config, features, logger2) { return uploadSarifArtifact(logger2, "sarif-artifact", failedSarif); } async function uploadSarifArtifact(logger2, prefix, failedSarif) { - logger2.info( - `Uploading failed SARIF file ${failedSarif.sarifFile} as artifact` - ); - const gitHubVersion = await getGitHubVersion(); - const client = await getArtifactUploaderClient(logger2, gitHubVersion.type); - const suffix = getArtifactSuffix(getOptionalInput("matrix")); - const name = sanitizeArtifactName(`${prefix}-${suffix}`); - await client.uploadArtifact( - name, - [import_path8.default.normalize(failedSarif.sarifFile)], - import_path8.default.normalize("..") - ); - return { sarifID: name }; + logger2.startGroup(`Uploading failed SARIF file as artifact`); + logger2.info(`Uploading '${failedSarif.sarifFile}' as artifact`); + try { + const gitHubVersion = await getGitHubVersion(); + const client = await getArtifactUploaderClient(logger2, gitHubVersion.type); + const suffix = getArtifactSuffix(getOptionalInput("matrix")); + const name = sanitizeArtifactName(`${prefix}-${suffix}`); + await client.uploadArtifact( + name, + [import_path8.default.normalize(failedSarif.sarifFile)], + import_path8.default.normalize("..") + ); + return { sarifID: name }; + } finally { + logger2.endGroup(); + } } async function tryUploadSarifIfRunFailed(config, repositoryNwo, features, logger2) { if (process.env["CODEQL_ACTION_ANALYZE_DID_COMPLETE_SUCCESSFULLY" /* ANALYZE_DID_COMPLETE_SUCCESSFULLY */] === "true") { diff --git a/src/init-action-post-helper.ts b/src/init-action-post-helper.ts index 0f0062ca7..30a3e0a3e 100644 --- a/src/init-action-post-helper.ts +++ b/src/init-action-post-helper.ts @@ -276,22 +276,25 @@ async function uploadSarifArtifact( prefix: string, failedSarif: FailedSarifInfo, ): Promise { - logger.info( - `Uploading failed SARIF file ${failedSarif.sarifFile} as artifact`, - ); + logger.startGroup(`Uploading failed SARIF file as artifact`); + logger.info(`Uploading '${failedSarif.sarifFile}' as artifact`); - const gitHubVersion = await getGitHubVersion(); - const client = await getArtifactUploaderClient(logger, gitHubVersion.type); + try { + const gitHubVersion = await getGitHubVersion(); + const client = await getArtifactUploaderClient(logger, gitHubVersion.type); - const suffix = getArtifactSuffix(actionsUtil.getOptionalInput("matrix")); - const name = sanitizeArtifactName(`${prefix}-${suffix}`); - await client.uploadArtifact( - name, - [path.normalize(failedSarif.sarifFile)], - path.normalize(".."), - ); + const suffix = getArtifactSuffix(actionsUtil.getOptionalInput("matrix")); + const name = sanitizeArtifactName(`${prefix}-${suffix}`); + await client.uploadArtifact( + name, + [path.normalize(failedSarif.sarifFile)], + path.normalize(".."), + ); - return { sarifID: name }; + return { sarifID: name }; + } finally { + logger.endGroup(); + } } /** From 950b1479a1b463f299172b24f365fa18998b5b5f Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Fri, 16 Oct 2026 13:09:16 +0100 Subject: [PATCH 20/26] Ensure that `rootDirectory` is suitable for `sarifFile` in `uploadSarifArtifact` --- lib/entry-points.js | 2 +- src/init-action-post-helper.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/entry-points.js b/lib/entry-points.js index 5ad162ce3..230d5b40f 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -187033,7 +187033,7 @@ async function uploadSarifArtifact(logger2, prefix, failedSarif) { await client.uploadArtifact( name, [import_path8.default.normalize(failedSarif.sarifFile)], - import_path8.default.normalize("..") + import_path8.default.normalize(import_path8.default.dirname(failedSarif.sarifFile)) ); return { sarifID: name }; } finally { diff --git a/src/init-action-post-helper.ts b/src/init-action-post-helper.ts index 30a3e0a3e..cfaab312f 100644 --- a/src/init-action-post-helper.ts +++ b/src/init-action-post-helper.ts @@ -288,7 +288,7 @@ async function uploadSarifArtifact( await client.uploadArtifact( name, [path.normalize(failedSarif.sarifFile)], - path.normalize(".."), + path.normalize(path.dirname(failedSarif.sarifFile)), ); return { sarifID: name }; From 72cb698832513ab0c830b12489d24e5193815b04 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:53:32 +0000 Subject: [PATCH 21/26] Update default bundle to codeql-bundle-v2.27.3 --- lib/defaults.json | 8 ++++---- lib/entry-points.js | 4 ++-- src/defaults.json | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/defaults.json b/lib/defaults.json index 71decd839..cde5136e2 100644 --- a/lib/defaults.json +++ b/lib/defaults.json @@ -1,6 +1,6 @@ { - "bundleVersion": "codeql-bundle-v2.27.2", - "cliVersion": "2.27.2", - "priorBundleVersion": "codeql-bundle-v2.27.1", - "priorCliVersion": "2.27.1" + "bundleVersion": "codeql-bundle-v2.27.3", + "cliVersion": "2.27.3", + "priorBundleVersion": "codeql-bundle-v2.27.2", + "priorCliVersion": "2.27.2" } diff --git a/lib/entry-points.js b/lib/entry-points.js index 1a99a4442..21193dec5 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -171792,8 +171792,8 @@ var path6 = __toESM(require("path")); var semver3 = __toESM(require_semver2()); // src/defaults.json -var bundleVersion = "codeql-bundle-v2.27.2"; -var cliVersion = "2.27.2"; +var bundleVersion = "codeql-bundle-v2.27.3"; +var cliVersion = "2.27.3"; // src/overlay/index.ts var fs5 = __toESM(require("fs")); diff --git a/src/defaults.json b/src/defaults.json index 71decd839..cde5136e2 100644 --- a/src/defaults.json +++ b/src/defaults.json @@ -1,6 +1,6 @@ { - "bundleVersion": "codeql-bundle-v2.27.2", - "cliVersion": "2.27.2", - "priorBundleVersion": "codeql-bundle-v2.27.1", - "priorCliVersion": "2.27.1" + "bundleVersion": "codeql-bundle-v2.27.3", + "cliVersion": "2.27.3", + "priorBundleVersion": "codeql-bundle-v2.27.2", + "priorCliVersion": "2.27.2" } From 260a53409e6044a52a54642d55495eb0a188a29f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:53:41 +0000 Subject: [PATCH 22/26] Add changelog note --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b63b79671..b88dff6aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ See the [releases page](https://github.com/github/codeql-action/releases) for th ## [UNRELEASED] -No user facing changes. +- Update default CodeQL bundle version to [2.27.3](https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.27.3). [#4256](https://github.com/github/codeql-action/pull/4256) ## 4.39.0 - 15 Oct 2026 From 74ddcd9f65683791ab07eccd074b4ba145d89ea6 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 20 Oct 2026 17:59:12 +0100 Subject: [PATCH 23/26] Fix typo in comment --- src/upload-lib.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/upload-lib.ts b/src/upload-lib.ts index b327054fb..4e97c1d1d 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -871,7 +871,7 @@ export async function waitForProcessing( ): Promise { logger.startGroup("Waiting for processing to finish"); try { - // If SARIF uplaod is disabled and we reach this point, then the file was written to disk and + // If SARIF upload is disabled and we reach this point, then the file was written to disk and // there is nothing to wait for. We still allow ourselves to get to this point so that we can // see in the log that we would have waited for processing to finish if the upload had happened. if (util.shouldSkipSarifUpload()) { From a69c7cb332d56f616fcadef002884523c71e8d1c Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 20 Oct 2026 18:02:30 +0100 Subject: [PATCH 24/26] Document that setting the env vars to `false` takes precedence --- src/environment.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/environment.ts b/src/environment.ts index 32140baf0..ffc3f305a 100644 --- a/src/environment.ts +++ b/src/environment.ts @@ -169,12 +169,17 @@ export enum EnvVar { * Whether to skip generating a failed SARIF file. Intended for testing purposes. * This setting is more specific than `CODEQL_ACTION_TEST_MODE` and * `CODEQL_ACTION_SKIP_SARIF_UPLOAD`, which imply this option. + * + * Setting this to `false` takes precedence over `CODEQL_ACTION_TEST_MODE: true` or + * `CODEQL_ACTION_SKIP_SARIF_UPLOAD: true`. */ SKIP_GENERATE_FAILED_SARIF = "CODEQL_ACTION_SKIP_GENERATE_FAILED_SARIF", /** * Whether to skip uploading SARIF results to GitHub. Intended for testing purposes. * This setting is more specific than `CODEQL_ACTION_TEST_MODE`, which implies this option. + * + * Setting this to `false` takes precedence over `CODEQL_ACTION_TEST_MODE: true`. */ SKIP_SARIF_UPLOAD = "CODEQL_ACTION_SKIP_SARIF_UPLOAD", From 7fa45180db8d657b23ded3636b5d8583aec6ddfe Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 20 Oct 2026 18:06:24 +0100 Subject: [PATCH 25/26] Use `shouldSkipSarifUpload` in `analyze-action.ts` --- lib/entry-points.js | 6 ++++-- src/analyze-action.ts | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/entry-points.js b/lib/entry-points.js index 230d5b40f..f70631437 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -180930,8 +180930,10 @@ async function run(action) { logger2 ); } - if (isInTestMode()) { - logger2.debug("In test mode. Waiting for processing is disabled."); + if (shouldSkipSarifUpload()) { + logger2.debug( + "SARIF upload is disabled and nothing was uploaded, so we do not need to wait for processing to finish." + ); } else if (uploadResults?.["code-scanning" /* CodeScanning */] !== void 0 && getRequiredInput("wait-for-processing") === "true") { await waitForProcessing( getRepositoryNwo(), diff --git a/src/analyze-action.ts b/src/analyze-action.ts index a11877b38..0bed377be 100644 --- a/src/analyze-action.ts +++ b/src/analyze-action.ts @@ -434,8 +434,10 @@ async function run(action: ActionState<["Base", "Logger", "Env", "Actions"]>) { } // We don't upload results in test mode, so don't wait for processing - if (util.isInTestMode()) { - logger.debug("In test mode. Waiting for processing is disabled."); + if (util.shouldSkipSarifUpload()) { + logger.debug( + "SARIF upload is disabled and nothing was uploaded, so we do not need to wait for processing to finish.", + ); } else if ( uploadResults?.[analyses.AnalysisKind.CodeScanning] !== undefined && actionsUtil.getRequiredInput("wait-for-processing") === "true" From b96e3390db93dc46c2c3753a94b87c0d734b0b61 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 20 Oct 2026 18:07:32 +0100 Subject: [PATCH 26/26] Change comment for `getConfig` call in `upload-sarif` --- src/upload-sarif-action.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/upload-sarif-action.ts b/src/upload-sarif-action.ts index 52dd12ec0..447314fb1 100644 --- a/src/upload-sarif-action.ts +++ b/src/upload-sarif-action.ts @@ -93,10 +93,8 @@ async function run({ startedAt, logger }: ActionState<["Base", "Logger"]>) { const checkoutPath = actionsUtil.getRequiredInput("checkout_path"); const category = actionsUtil.getOptionalInput("category"); - // Although we wouldn't typically expect there to be a configuration since we don't expect another - // CodeQL Action step to have come before `upload-sarif`, this has previously been the behaviour - // when `combineSarifFilesUsingCLI` called `getConfig` directly. Since the only effect of this would - // be that an existing CLI that was installed gets re-used, we maintain this behaviour. + // Load the configuration from a previous step if there is one. This allows us to reuse the CLI that + // may have been installed by a previous step. const maybeConfig = await getConfig(tempDir, logger); const uploadResults = await postProcessAndUploadSarif( logger,