Skip to content
18 changes: 13 additions & 5 deletions lib/analyze-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 18 additions & 6 deletions lib/init-action-post.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 13 additions & 5 deletions lib/upload-lib.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 16 additions & 7 deletions lib/upload-sarif-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,10 @@ export enum EnvVar {
* whether the upload is disabled. This is intended for testing and debugging purposes.
*/
SARIF_DUMP_DIR = "CODEQL_ACTION_SARIF_DUMP_DIR",

/**
* Whether to skip uploading SARIF results to GitHub. Intended for testing purposes.
* This setting is implied by `CODEQL_ACTION_TEST_MODE`, but is more specific.
*/
SKIP_SARIF_UPLOAD = "CODEQL_ACTION_SKIP_SARIF_UPLOAD",
}
9 changes: 6 additions & 3 deletions src/init-action-post-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import {
delay,
getErrorMessage,
getRequiredEnvParam,
isInTestMode,
parseMatrixInput,
getSarifUploadSkipReason,
wrapError,
} from "./util";
import {
Expand Down Expand Up @@ -80,11 +80,14 @@ async function maybeUploadFailedSarif(
if (
!["always", "failure-only"].includes(
actionsUtil.getUploadValue(shouldUpload),
) ||
isInTestMode()
)
) {
return { upload_failed_run_skipped_because: "SARIF upload is disabled" };
}
const skipReason = getSarifUploadSkipReason();
if (skipReason) {
return { upload_failed_run_skipped_because: skipReason };
}
const category = getCategoryInputOrThrow(workflow, jobName, matrix);
const checkoutPath = getCheckoutPathInputOrThrow(workflow, jobName, matrix);
const databasePath = config.dbLocation;
Expand Down
11 changes: 5 additions & 6 deletions src/upload-lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,18 +356,17 @@ async function uploadPayload(
): Promise<string> {
logger.info("Uploading results");

// If in test mode we don't want to upload the results
if (util.isInTestMode()) {
// If in test mode we don't want to upload the results,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment is a bit stale now, ditto at the other isInTestMode/shouldSkipSarifUpload switch.

const skipReason = util.getSarifUploadSkipReason();
if (skipReason) {
const payloadSaveFile = path.join(
actionsUtil.getTemporaryDirectory(),
"payload.json",
);
logger.info(
`In test mode. Results are not uploaded. Saving to ${payloadSaveFile}`,
);
logger.info(`${skipReason}. Saving to ${payloadSaveFile}`);
logger.info(`Payload: ${JSON.stringify(payload, null, 2)}`);
fs.writeFileSync(payloadSaveFile, JSON.stringify(payload, null, 2));
return "test-mode-sarif-id";
return "dummy-sarif-id";
}

const client = api.getApiClient();
Expand Down
7 changes: 4 additions & 3 deletions src/upload-sarif-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
checkDiskUsage,
getErrorMessage,
initializeEnvironment,
isInTestMode,
getSarifUploadSkipReason,
wrapError,
} from "./util";

Expand Down Expand Up @@ -113,8 +113,9 @@ async function run() {
core.setOutput("sarif-ids", JSON.stringify(uploadResults));

// We don't upload results in test mode, so don't wait for processing
if (isInTestMode()) {
core.debug("In test mode. Waiting for processing is disabled.");
const skipReason = getSarifUploadSkipReason();
if (skipReason) {
core.debug(`${skipReason}. Waiting for processing is disabled.`);
} else if (actionsUtil.getRequiredInput("wait-for-processing") === "true") {
if (codeScanningResult !== undefined) {
await upload_lib.waitForProcessing(
Expand Down
15 changes: 14 additions & 1 deletion src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -764,12 +764,25 @@ export function isGoodVersion(versionSpec: string) {
/**
* Returns whether we are in test mode. This is used by CodeQL Action PR checks.
*
* In test mode, we don't upload SARIF results or status reports to the GitHub API.
* In test mode, we skip several uploads (SARIF results, status reports, DBs, ...).
*/
export function isInTestMode(): boolean {
return process.env[EnvVar.TEST_MODE] === "true";
}

/**
* Returns whether we specifically want to skip uploading SARIF files, and if so, why.
*/
export function getSarifUploadSkipReason(): string | null {
if (isInTestMode()) {
return `SARIF upload is disabled via ${EnvVar.TEST_MODE}`;
}
if (process.env[EnvVar.SKIP_SARIF_UPLOAD] === "true") {
return `SARIF upload is disabled via ${EnvVar.SKIP_SARIF_UPLOAD}`;
}
return null;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had considered something like this when I wrote my comment, but wasn't too keen on it. I also don't love using the human-readable reason phrase to mean true for enablement purposes. The previous version with the boolean function was preferable.

Also, in general, we don't use null in the codeql-action codebase (unless it comes to us from outside the codebase) and would go for undefined here.

Copy link
Contributor Author

@redsun82 redsun82 Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to avoid something like

    logger.info(`SARIF upload disabled by `${ isInTestMode() ? EnvVar.TEST_MODE : EnvVar.SKIP_SARIF_UPLOAD }`. Saving to ${payloadSaveFile}`);

as I didn't want to expose that util implementation detail. So I think I'll just go with your first proposal for a generic with an environment variable. The users of those variables (us) probably are savvy enough for that to be enough for them.


/**
* Get the testing environment.
*
Expand Down
Loading