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/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index ccbe4a6d2..5d2b1576f 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] @@ -145,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 }} @@ -209,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.actor != 'dependabot[bot]' && needs.other-checks.result == 'success' permissions: contents: read 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 diff --git a/init/action.yml b/init/action.yml index 7787a0a07..8d26999f7 100644 --- a/init/action.yml +++ b/init/action.yml @@ -153,7 +153,10 @@ 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. + 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: >- 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 9e674a2c1..f17d327c6 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -169945,8 +169945,15 @@ 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 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"); } function getTestingEnvironment() { const testingEnvironment = process.env["CODEQL_ACTION_TESTING_ENVIRONMENT" /* TESTING_ENVIRONMENT */] || ""; @@ -171805,8 +171812,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")); @@ -172098,11 +172105,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", @@ -174845,11 +174847,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; @@ -175052,7 +175054,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, @@ -178543,6 +178545,7 @@ __export(upload_lib_exports, { filterAlertsByDiffRange: () => filterAlertsByDiffRange, findSarifFilesInDir: () => findSarifFilesInDir, getGroupedSarifFilePaths: () => getGroupedSarifFilePaths, + getSavedPayloadPath: () => getSavedPayloadPath, populateRunAutomationDetails: () => populateRunAutomationDetails, postProcessSarifFiles: () => postProcessSarifFiles, readSarifFileOrThrow: () => readSarifFileOrThrow, @@ -180018,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); @@ -180039,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; @@ -180106,13 +180108,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}` ); @@ -180324,7 +180329,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; @@ -180338,7 +180343,8 @@ async function postProcessSarifFiles(logger2, features, checkoutPath, sarifPaths sarifPaths, gitHubVersion, features, - logger2 + logger2, + config ); } else { const sarifPath = sarifPaths[0]; @@ -180371,13 +180377,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, @@ -180385,10 +180392,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, @@ -180473,6 +180481,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") { @@ -180618,7 +180632,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 @@ -180631,6 +180645,7 @@ async function postProcessAndUploadSarif(logger2, features, uploadKind, checkout const postProcessingResults = await postProcessSarifFiles( logger2, features, + config, checkoutPath, sarifFiles, category, @@ -180861,6 +180876,7 @@ async function run(action) { uploadResults = await postProcessAndUploadSarif( logger2, features, + config, uploadKind, checkoutPath, outputDir, @@ -180913,8 +180929,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(), @@ -186914,7 +186932,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" }); @@ -186985,6 +187003,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, @@ -186998,6 +187017,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) { @@ -187006,19 +187028,25 @@ async function maybeUploadFailedSarifArtifact(config, features, logger2) { return failedSarifResult.value; } const failedSarif = failedSarifResult.value; - 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}`); - await client.uploadArtifact( - name, - [import_path8.default.normalize(failedSarif.sarifFile)], - import_path8.default.normalize("..") - ); - return { sarifID: name }; + return uploadSarifArtifact(logger2, "sarif-artifact", failedSarif); +} +async function uploadSarifArtifact(logger2, prefix, failedSarif) { + 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(import_path8.default.dirname(failedSarif.sarifFile)) + ); + 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") { @@ -187243,6 +187271,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(); @@ -188462,11 +188493,12 @@ async function run8(action) { 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( @@ -188483,9 +188515,11 @@ async function run8(action) { 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/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 diff --git a/src/analyze-action.ts b/src/analyze-action.ts index f7cad45f3..8ba10df0e 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, @@ -433,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" 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 5c2a89776..664853d68 100644 --- a/src/config-utils.ts +++ b/src/config-utils.ts @@ -940,10 +940,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"; @@ -951,12 +955,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; } @@ -1298,7 +1298,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/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" } diff --git a/src/environment.ts b/src/environment.ts index 7acff8e99..cc0f099f5 100644 --- a/src/environment.ts +++ b/src/environment.ts @@ -165,9 +165,21 @@ 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. + * + * 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", diff --git a/src/feature-flags.ts b/src/feature-flags.ts index e64b69eab..42dd866ad 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", 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 f9021b789..cfaab312f 100644 --- a/src/init-action-post-helper.ts +++ b/src/init-action-post-helper.ts @@ -44,6 +44,7 @@ import { getRequiredEnvParam, parseMatrixInput, Result, + shouldSkipGenerateFailedSarif, shouldSkipSarifUpload, Success, wrapError, @@ -115,7 +116,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", }); @@ -226,6 +227,7 @@ async function maybeUploadFailedSarif( logger.info(`Uploading failed SARIF file ${failedSarif.sarifFile}`); const uploadResult = await uploadLib.uploadFiles( + config, failedSarif.sarifFile, failedSarif.checkoutPath, failedSarif.category, @@ -239,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 } : {}; @@ -257,23 +267,34 @@ async function maybeUploadFailedSarifArtifact( } const failedSarif = failedSarifResult.value; + return uploadSarifArtifact(logger, "sarif-artifact", failedSarif); +} - logger.info( - `Uploading failed SARIF file ${failedSarif.sarifFile} as artifact`, - ); +/** Uploads a failed SARIF file as workflow artifact. */ +async function uploadSarifArtifact( + logger: Logger, + prefix: string, + failedSarif: FailedSarifInfo, +): Promise { + 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(`sarif-artifact-${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(path.dirname(failedSarif.sarifFile)), + ); - return { sarifID: name }; + return { sarifID: name }; + } finally { + logger.endGroup(); + } } /** 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); } diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 3c46c1519..4e97c1d1d 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; @@ -228,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. @@ -242,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}`, ); @@ -599,6 +607,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 +632,7 @@ export async function postProcessSarifFiles( gitHubVersion, features, logger, + config, ); } else { const sarifPath = sarifPaths[0]; @@ -683,6 +693,7 @@ export async function writePostProcessedFiles( * to. */ export async function uploadFiles( + config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined, inputSarifPath: string, checkoutPath: string, category: string | undefined, @@ -697,6 +708,7 @@ export async function uploadFiles( return uploadSpecifiedFiles( sarifPaths, + config, checkoutPath, category, features, @@ -710,6 +722,7 @@ export async function uploadFiles( */ async function uploadSpecifiedFiles( sarifPaths: string[], + config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined, checkoutPath: string, category: string | undefined, features: FeatureEnablement, @@ -719,6 +732,7 @@ async function uploadSpecifiedFiles( const processingResults: PostProcessingResults = await postProcessSarifFiles( logger, features, + config, checkoutPath, sarifPaths, category, @@ -857,6 +871,16 @@ export async function waitForProcessing( ): Promise { logger.startGroup("Waiting for processing to finish"); try { + // 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()) { + 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 diff --git a/src/upload-sarif-action.ts b/src/upload-sarif-action.ts index 7d67c5567..0efbcdb40 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"; @@ -67,11 +68,12 @@ async function run(action: ActionState<["Base", "Env", "Logger"]>) { // Make inputs accessible in the `post` step. actionsUtil.persistInputs(); + const tempDir = getTemporaryDirectory(); const repositoryNwo = getRepositoryNwo(); const features = initFeatures( gitHubVersion, repositoryNwo, - getTemporaryDirectory(), + tempDir, logger, ); @@ -92,9 +94,13 @@ async function run(action: ActionState<["Base", "Env", "Logger"]>) { const checkoutPath = actionsUtil.getRequiredInput("checkout_path"); const category = actionsUtil.getOptionalInput("category"); + // 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, 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, diff --git a/src/util.test.ts b/src/util.test.ts index bca2b452e..e7a7d3abd 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,70 @@ 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("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(); + + // 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 b9f51011e..533f40ed7 100644 --- a/src/util.ts +++ b/src/util.ts @@ -18,8 +18,9 @@ import type { Config } from "./config-utils"; import { EnvVar, getRequiredEnvParam, - isInTestMode, exportEnvVar, + getEnv, + type ReadOnlyEnv, } from "./environment"; import { ConfigurationError, getErrorMessage, wrapError } from "./errors"; import { GitHubVariant, type GitHubVersion } from "./github"; @@ -620,11 +621,30 @@ 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. */ -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") + ); } /**