robojerk/codeql-action

Merge pull request #2265 from github/koesie10/deprecate-merge

Browse source 
Add deprecation warning for merging SARIF files with non-unique categories
This commit is contained in:
Koen Vlaswinkel 2024-05-03 10:23:10 +02:00 • committed by GitHub
commit 93b8232a39
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 350 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

7
lib/feature-flags.js generated
View file

@ -51,6 +51,7 @@ var Feature;
(function (Feature) {
Feature["AutobuildDirectTracingEnabled"] = "autobuild_direct_tracing_enabled";
Feature["CliSarifMerge"] = "cli_sarif_merge_enabled";
Feature["CombineSarifFilesDeprecationWarning"] = "combine_sarif_files_deprecation_warning_enabled";
Feature["CppDependencyInstallation"] = "cpp_dependency_installation_enabled";
Feature["CppTrapCachingEnabled"] = "cpp_trap_caching_enabled";
Feature["DisableJavaBuildlessEnabled"] = "disable_java_buildless_enabled";
@ -71,6 +72,12 @@ exports.featureConfig = {
minimumVersion: undefined,
defaultValue: false,
},
[Feature.CombineSarifFilesDeprecationWarning]: {
envVar: "CODEQL_ACTION_COMBINE_SARIF_FILES_DEPRECATION_WARNING",
// Independent of the CLI version.
minimumVersion: undefined,
defaultValue: false,
},
[Feature.CppDependencyInstallation]: {
envVar: "CODEQL_EXTRACTOR_CPP_AUTOINSTALL_DEPENDENCIES",
minimumVersion: "2.15.0",

2
lib/feature-flags.js.map
View file

File diff suppressed because one or more lines are too long

72
lib/upload-lib.js generated
View file

@ -26,13 +26,14 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", { value: true });
exports.InvalidSarifUploadError = exports.validateUniqueCategory = exports.waitForProcessing = exports.buildPayload = exports.validateSarifFileSchema = exports.uploadFromActions = exports.findSarifFilesInDir = exports.populateRunAutomationDetails = void 0;
exports.InvalidSarifUploadError = exports.validateUniqueCategory = exports.waitForProcessing = exports.buildPayload = exports.validateSarifFileSchema = exports.uploadFromActions = exports.findSarifFilesInDir = exports.populateRunAutomationDetails = exports.shouldShowCombineSarifFilesDeprecationWarning = void 0;
const fs = __importStar(require("fs"));
const path = __importStar(require("path"));
const zlib_1 = __importDefault(require("zlib"));
const core = __importStar(require("@actions/core"));
const file_url_1 = __importDefault(require("file-url"));
const jsonschema = __importStar(require("jsonschema"));
const semver = __importStar(require("semver"));
const actionsUtil = __importStar(require("./actions-util"));
const actions_util_1 = require("./actions-util");
const api = __importStar(require("./api-client"));
@ -73,14 +74,58 @@ function combineSarifFiles(sarifFiles, logger) {
}
/**
* Checks whether all the runs in the given SARIF files were produced by CodeQL.
* @param sarifFiles The list of SARIF files to check.
* @param sarifObjects The list of SARIF objects to check.
*/
function areAllRunsProducedByCodeQL(sarifFiles) {
return sarifFiles.every((sarifFile) => {
const sarifObject = JSON.parse(fs.readFileSync(sarifFile, "utf8"));
function areAllRunsProducedByCodeQL(sarifObjects) {
return sarifObjects.every((sarifObject) => {
return sarifObject.runs?.every((run) => run.tool?.driver?.name === "CodeQL");
});
}
function createRunKey(run) {
return {
name: run.tool?.driver?.name,
fullName: run.tool?.driver?.fullName,
version: run.tool?.driver?.version,
semanticVersion: run.tool?.driver?.semanticVersion,
guid: run.tool?.driver?.guid,
automationId: run.automationDetails?.id,
};
}
/**
* Checks whether all runs in the given SARIF files are unique (based on the
* criteria used by Code Scanning to determine analysis categories).
* @param sarifObjects The list of SARIF objects to check.
*/
function areAllRunsUnique(sarifObjects) {
const keys = new Set();
for (const sarifObject of sarifObjects) {
for (const run of sarifObject.runs) {
const key = JSON.stringify(createRunKey(run));
// If the key already exists, the runs are not unique.
if (keys.has(key)) {
return false;
}
keys.add(key);
}
}
return true;
}
// Checks whether the deprecation warning for combining SARIF files should be shown.
async function shouldShowCombineSarifFilesDeprecationWarning(sarifObjects, features, githubVersion) {
if (!(await features.getValue(feature_flags_1.Feature.CombineSarifFilesDeprecationWarning))) {
return false;
}
// Do not show this warning on GHES versions before 3.14.0
if (githubVersion.type === util_1.GitHubVariant.GHES &&
semver.lt(githubVersion.version, "3.14.0")) {
return false;
}
// Only give a deprecation warning when not all runs are unique and
// we haven't already shown the warning.
return (!areAllRunsUnique(sarifObjects) &&
!process.env.CODEQL_MERGE_SARIF_DEPRECATION_WARNING);
}
exports.shouldShowCombineSarifFilesDeprecationWarning = shouldShowCombineSarifFilesDeprecationWarning;
// 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.
@ -90,8 +135,19 @@ async function combineSarifFilesUsingCLI(sarifFiles, gitHubVersion, features, lo
if (sarifFiles.length === 1) {
return JSON.parse(fs.readFileSync(sarifFiles[0], "utf8"));
}
if (!areAllRunsProducedByCodeQL(sarifFiles)) {
const sarifObjects = sarifFiles.map((sarifFile) => {
return JSON.parse(fs.readFileSync(sarifFile, "utf8"));
});
const deprecationWarningMessage = gitHubVersion.type === util_1.GitHubVariant.GHES
? "and will be removed in GitHub Enterprise Server 3.18"
: "and will be removed on June 4, 2025";
const deprecationMoreInformationMessage = "For more information, see https://github.blog/changelog/2024-05-06-code-scanning-will-stop-combining-runs-deprecation-notice";
if (!areAllRunsProducedByCodeQL(sarifObjects)) {
logger.debug("Not all SARIF files were produced by CodeQL. Merging files in the action.");
if (await shouldShowCombineSarifFilesDeprecationWarning(sarifObjects, features, gitHubVersion)) {
logger.warning(`Uploading multiple SARIF runs with the same category is deprecated ${deprecationWarningMessage}. Please update your workflow to upload a single run per category. ${deprecationMoreInformationMessage}`);
core.exportVariable("CODEQL_MERGE_SARIF_DEPRECATION_WARNING", "true");
}
// If not, use the naive method of combining the files.
return combineSarifFiles(sarifFiles, logger);
}
@ -119,6 +175,10 @@ async function combineSarifFilesUsingCLI(sarifFiles, gitHubVersion, features, lo
}
if (!(await codeQL.supportsFeature(tools_features_1.ToolsFeature.SarifMergeRunsFromEqualCategory))) {
logger.warning("The CodeQL CLI does not support merging SARIF files. Merging files in the action.");
if (await shouldShowCombineSarifFilesDeprecationWarning(sarifObjects, features, gitHubVersion)) {
logger.warning(`Uploading multiple CodeQL runs with the same category is deprecated ${deprecationWarningMessage} for CodeQL CLI 2.16.6 and earlier. Please update your CodeQL CLI version or update your workflow to set a distinct category for each CodeQL run. ${deprecationMoreInformationMessage}`);
core.exportVariable("CODEQL_MERGE_SARIF_DEPRECATION_WARNING", "true");
}
return combineSarifFiles(sarifFiles, logger);
}
const baseTempDir = path.resolve(tempDir, "combined-sarif");

2
lib/upload-lib.js.map
View file

File diff suppressed because one or more lines are too long

44
lib/upload-lib.test.js generated
View file

@ -29,6 +29,7 @@ Object.defineProperty(exports, "__esModule", { value: true });
const fs = __importStar(require("fs"));
const path = __importStar(require("path"));
const ava_1 = __importDefault(require("ava"));
const feature_flags_1 = require("./feature-flags");
const logging_1 = require("./logging");
const testing_utils_1 = require("./testing-utils");
const uploadLib = __importStar(require("./upload-lib"));
@ -195,6 +196,49 @@ ava_1.default.beforeEach(() => {
t.deepEqual(loggedMessages.length, 2);
t.deepEqual(loggedMessages[1], "Warning: 'not a valid URI' is not a valid URI in 'instance.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri'.");
});
(0, ava_1.default)("shouldShowCombineSarifFilesDeprecationWarning when on dotcom with feature flag", async (t) => {
t.true(await uploadLib.shouldShowCombineSarifFilesDeprecationWarning([createMockSarif("abc", "def"), createMockSarif("abc", "def")], (0, testing_utils_1.createFeatures)([feature_flags_1.Feature.CombineSarifFilesDeprecationWarning]), {
type: util_1.GitHubVariant.DOTCOM,
}));
});
(0, ava_1.default)("shouldShowCombineSarifFilesDeprecationWarning without feature flag", async (t) => {
t.false(await uploadLib.shouldShowCombineSarifFilesDeprecationWarning([createMockSarif("abc", "def"), createMockSarif("abc", "def")], (0, testing_utils_1.createFeatures)([]), {
type: util_1.GitHubVariant.DOTCOM,
}));
});
(0, ava_1.default)("shouldShowCombineSarifFilesDeprecationWarning when on GHES 3.13", async (t) => {
t.false(await uploadLib.shouldShowCombineSarifFilesDeprecationWarning([createMockSarif("abc", "def"), createMockSarif("abc", "def")], (0, testing_utils_1.createFeatures)([feature_flags_1.Feature.CombineSarifFilesDeprecationWarning]), {
type: util_1.GitHubVariant.GHES,
version: "3.13.2",
}));
});
(0, ava_1.default)("shouldShowCombineSarifFilesDeprecationWarning when on GHES 3.14", async (t) => {
t.true(await uploadLib.shouldShowCombineSarifFilesDeprecationWarning([createMockSarif("abc", "def"), createMockSarif("abc", "def")], (0, testing_utils_1.createFeatures)([feature_flags_1.Feature.CombineSarifFilesDeprecationWarning]), {
type: util_1.GitHubVariant.GHES,
version: "3.14.0",
}));
});
(0, ava_1.default)("shouldShowCombineSarifFilesDeprecationWarning with only 1 run", async (t) => {
t.false(await uploadLib.shouldShowCombineSarifFilesDeprecationWarning([createMockSarif("abc", "def")], (0, testing_utils_1.createFeatures)([feature_flags_1.Feature.CombineSarifFilesDeprecationWarning]), {
type: util_1.GitHubVariant.DOTCOM,
}));
});
(0, ava_1.default)("shouldShowCombineSarifFilesDeprecationWarning with distinct categories", async (t) => {
t.false(await uploadLib.shouldShowCombineSarifFilesDeprecationWarning([createMockSarif("abc", "def"), createMockSarif("def", "def")], (0, testing_utils_1.createFeatures)([feature_flags_1.Feature.CombineSarifFilesDeprecationWarning]), {
type: util_1.GitHubVariant.DOTCOM,
}));
});
(0, ava_1.default)("shouldShowCombineSarifFilesDeprecationWarning with distinct tools", async (t) => {
t.false(await uploadLib.shouldShowCombineSarifFilesDeprecationWarning([createMockSarif("abc", "abc"), createMockSarif("abc", "def")], (0, testing_utils_1.createFeatures)([feature_flags_1.Feature.CombineSarifFilesDeprecationWarning]), {
type: util_1.GitHubVariant.DOTCOM,
}));
});
(0, ava_1.default)("shouldShowCombineSarifFilesDeprecationWarning when environment variable is already set", async (t) => {
process.env["CODEQL_MERGE_SARIF_DEPRECATION_WARNING"] = "true";
t.false(await uploadLib.shouldShowCombineSarifFilesDeprecationWarning([createMockSarif("abc", "def"), createMockSarif("abc", "def")], (0, testing_utils_1.createFeatures)([feature_flags_1.Feature.CombineSarifFilesDeprecationWarning]), {
type: util_1.GitHubVariant.DOTCOM,
}));
});
function createMockSarif(id, tool) {
return {
runs: [

2
lib/upload-lib.test.js.map
View file

File diff suppressed because one or more lines are too long

2
lib/util.js.map
View file

File diff suppressed because one or more lines are too long

7
src/feature-flags.ts
View file

@ -47,6 +47,7 @@ export interface FeatureEnablement {
export enum Feature {
AutobuildDirectTracingEnabled = "autobuild_direct_tracing_enabled",
CliSarifMerge = "cli_sarif_merge_enabled",
CombineSarifFilesDeprecationWarning = "combine_sarif_files_deprecation_warning_enabled",
CppDependencyInstallation = "cpp_dependency_installation_enabled",
CppTrapCachingEnabled = "cpp_trap_caching_enabled",
DisableJavaBuildlessEnabled = "disable_java_buildless_enabled",
@ -91,6 +92,12 @@ export const featureConfig: Record<
minimumVersion: undefined,
defaultValue: false,
},
[Feature.CombineSarifFilesDeprecationWarning]: {
envVar: "CODEQL_ACTION_COMBINE_SARIF_FILES_DEPRECATION_WARNING",
// Independent of the CLI version.
minimumVersion: undefined,
defaultValue: false,
},
[Feature.CppDependencyInstallation]: {
envVar: "CODEQL_EXTRACTOR_CPP_AUTOINSTALL_DEPENDENCIES",
minimumVersion: "2.15.0",

105
src/upload-lib.test.ts
View file

@ -3,10 +3,11 @@ import * as path from "path";
import test from "ava";
import { Feature } from "./feature-flags";
import { getRunnerLogger, Logger } from "./logging";
import { setupTests } from "./testing-utils";
import { createFeatures, setupTests } from "./testing-utils";
import * as uploadLib from "./upload-lib";
import { initializeEnvironment, withTmpDir } from "./util";
import { GitHubVariant, initializeEnvironment, withTmpDir } from "./util";
setupTests(test);
@ -324,6 +325,106 @@ test("accept results with invalid artifactLocation.uri value", (t) => {
);
});
test("shouldShowCombineSarifFilesDeprecationWarning when on dotcom with feature flag", async (t) => {
t.true(
await uploadLib.shouldShowCombineSarifFilesDeprecationWarning(
[createMockSarif("abc", "def"), createMockSarif("abc", "def")],
createFeatures([Feature.CombineSarifFilesDeprecationWarning]),
{
type: GitHubVariant.DOTCOM,
},
),
);
});
test("shouldShowCombineSarifFilesDeprecationWarning without feature flag", async (t) => {
t.false(
await uploadLib.shouldShowCombineSarifFilesDeprecationWarning(
[createMockSarif("abc", "def"), createMockSarif("abc", "def")],
createFeatures([]),
{
type: GitHubVariant.DOTCOM,
},
),
);
});
test("shouldShowCombineSarifFilesDeprecationWarning when on GHES 3.13", async (t) => {
t.false(
await uploadLib.shouldShowCombineSarifFilesDeprecationWarning(
[createMockSarif("abc", "def"), createMockSarif("abc", "def")],
createFeatures([Feature.CombineSarifFilesDeprecationWarning]),
{
type: GitHubVariant.GHES,
version: "3.13.2",
},
),
);
});
test("shouldShowCombineSarifFilesDeprecationWarning when on GHES 3.14", async (t) => {
t.true(
await uploadLib.shouldShowCombineSarifFilesDeprecationWarning(
[createMockSarif("abc", "def"), createMockSarif("abc", "def")],
createFeatures([Feature.CombineSarifFilesDeprecationWarning]),
{
type: GitHubVariant.GHES,
version: "3.14.0",
},
),
);
});
test("shouldShowCombineSarifFilesDeprecationWarning with only 1 run", async (t) => {
t.false(
await uploadLib.shouldShowCombineSarifFilesDeprecationWarning(
[createMockSarif("abc", "def")],
createFeatures([Feature.CombineSarifFilesDeprecationWarning]),
{
type: GitHubVariant.DOTCOM,
},
),
);
});
test("shouldShowCombineSarifFilesDeprecationWarning with distinct categories", async (t) => {
t.false(
await uploadLib.shouldShowCombineSarifFilesDeprecationWarning(
[createMockSarif("abc", "def"), createMockSarif("def", "def")],
createFeatures([Feature.CombineSarifFilesDeprecationWarning]),
{
type: GitHubVariant.DOTCOM,
},
),
);
});
test("shouldShowCombineSarifFilesDeprecationWarning with distinct tools", async (t) => {
t.false(
await uploadLib.shouldShowCombineSarifFilesDeprecationWarning(
[createMockSarif("abc", "abc"), createMockSarif("abc", "def")],
createFeatures([Feature.CombineSarifFilesDeprecationWarning]),
{
type: GitHubVariant.DOTCOM,
},
),
);
});
test("shouldShowCombineSarifFilesDeprecationWarning when environment variable is already set", async (t) => {
process.env["CODEQL_MERGE_SARIF_DEPRECATION_WARNING"] = "true";
t.false(
await uploadLib.shouldShowCombineSarifFilesDeprecationWarning(
[createMockSarif("abc", "def"), createMockSarif("abc", "def")],
createFeatures([Feature.CombineSarifFilesDeprecationWarning]),
{
type: GitHubVariant.DOTCOM,
},
),
);
});
function createMockSarif(id?: string, tool?: string) {
return {
runs: [

126
src/upload-lib.ts
View file

@ -6,6 +6,7 @@ import * as core from "@actions/core";
import { OctokitResponse } from "@octokit/types";
import fileUrl from "file-url";
import * as jsonschema from "jsonschema";
import * as semver from "semver";
import * as actionsUtil from "./actions-util";
import { getOptionalInput, getRequiredInput } from "./actions-util";
@ -14,7 +15,7 @@ import { getGitHubVersion, wrapApiConfigurationError } from "./api-client";
import { CodeQL, getCodeQL } from "./codeql";
import { getConfig } from "./config-utils";
import { EnvVar } from "./environment";
import { Feature, Features } from "./feature-flags";
import { Feature, FeatureEnablement, Features } from "./feature-flags";
import * as fingerprints from "./fingerprints";
import { initCodeQL } from "./init";
import { Logger } from "./logging";
@ -24,8 +25,10 @@ import * as util from "./util";
import {
ConfigurationError,
getRequiredEnvParam,
GitHubVariant,
GitHubVersion,
SarifFile,
SarifRun,
wrapError,
} from "./util";
@ -65,20 +68,86 @@ function combineSarifFiles(sarifFiles: string[], logger: Logger): SarifFile {
/**
* Checks whether all the runs in the given SARIF files were produced by CodeQL.
* @param sarifFiles The list of SARIF files to check.
* @param sarifObjects The list of SARIF objects to check.
*/
function areAllRunsProducedByCodeQL(sarifFiles: string[]): boolean {
return sarifFiles.every((sarifFile) => {
const sarifObject = JSON.parse(
fs.readFileSync(sarifFile, "utf8"),
) as SarifFile;
function areAllRunsProducedByCodeQL(sarifObjects: SarifFile[]): boolean {
return sarifObjects.every((sarifObject) => {
return sarifObject.runs?.every(
(run) => run.tool?.driver?.name === "CodeQL",
);
});
}
type SarifRunKey = {
name: string | undefined;
fullName: string | undefined;
version: string | undefined;
semanticVersion: string | undefined;
guid: string | undefined;
automationId: string | undefined;
};
function createRunKey(run: SarifRun): SarifRunKey {
return {
name: run.tool?.driver?.name,
fullName: run.tool?.driver?.fullName,
version: run.tool?.driver?.version,
semanticVersion: run.tool?.driver?.semanticVersion,
guid: run.tool?.driver?.guid,
automationId: run.automationDetails?.id,
};
}
/**
* Checks whether all runs in the given SARIF files are unique (based on the
* criteria used by Code Scanning to determine analysis categories).
* @param sarifObjects The list of SARIF objects to check.
*/
function areAllRunsUnique(sarifObjects: SarifFile[]): boolean {
const keys = new Set<string>();
for (const sarifObject of sarifObjects) {
for (const run of sarifObject.runs) {
const key = JSON.stringify(createRunKey(run));
// If the key already exists, the runs are not unique.
if (keys.has(key)) {
return false;
}
keys.add(key);
}
}
return true;
}
// Checks whether the deprecation warning for combining SARIF files should be shown.
export async function shouldShowCombineSarifFilesDeprecationWarning(
sarifObjects: util.SarifFile[],
features: FeatureEnablement,
githubVersion: GitHubVersion,
) {
if (!(await features.getValue(Feature.CombineSarifFilesDeprecationWarning))) {
return false;
}
// Do not show this warning on GHES versions before 3.14.0
if (
githubVersion.type === GitHubVariant.GHES &&
semver.lt(githubVersion.version, "3.14.0")
) {
return false;
}
// Only give a deprecation warning when not all runs are unique and
// we haven't already shown the warning.
return (
!areAllRunsUnique(sarifObjects) &&
!process.env.CODEQL_MERGE_SARIF_DEPRECATION_WARNING
);
}
// 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.
@ -86,7 +155,7 @@ function areAllRunsProducedByCodeQL(sarifFiles: string[]): boolean {
async function combineSarifFilesUsingCLI(
sarifFiles: string[],
gitHubVersion: GitHubVersion,
features: Features,
features: FeatureEnablement,
logger: Logger,
): Promise<SarifFile> {
logger.info("Combining SARIF files using the CodeQL CLI");
@ -94,11 +163,35 @@ async function combineSarifFilesUsingCLI(
return JSON.parse(fs.readFileSync(sarifFiles[0], "utf8")) as SarifFile;
}
if (!areAllRunsProducedByCodeQL(sarifFiles)) {
const sarifObjects = sarifFiles.map((sarifFile): SarifFile => {
return JSON.parse(fs.readFileSync(sarifFile, "utf8")) as SarifFile;
});
const deprecationWarningMessage =
gitHubVersion.type === GitHubVariant.GHES
? "and will be removed in GitHub Enterprise Server 3.18"
: "and will be removed on June 4, 2025";
const deprecationMoreInformationMessage =
"For more information, see https://github.blog/changelog/2024-05-06-code-scanning-will-stop-combining-runs-deprecation-notice";
if (!areAllRunsProducedByCodeQL(sarifObjects)) {
logger.debug(
"Not all SARIF files were produced by CodeQL. Merging files in the action.",
);
if (
await shouldShowCombineSarifFilesDeprecationWarning(
sarifObjects,
features,
gitHubVersion,
)
) {
logger.warning(
`Uploading multiple SARIF runs with the same category is deprecated ${deprecationWarningMessage}. Please update your workflow to upload a single run per category. ${deprecationMoreInformationMessage}`,
);
core.exportVariable("CODEQL_MERGE_SARIF_DEPRECATION_WARNING", "true");
}
// If not, use the naive method of combining the files.
return combineSarifFiles(sarifFiles, logger);
}
@ -149,6 +242,19 @@ async function combineSarifFilesUsingCLI(
"The CodeQL CLI does not support merging SARIF files. Merging files in the action.",
);
if (
await shouldShowCombineSarifFilesDeprecationWarning(
sarifObjects,
features,
gitHubVersion,
)
) {
logger.warning(
`Uploading multiple CodeQL runs with the same category is deprecated ${deprecationWarningMessage} for CodeQL CLI 2.16.6 and earlier. Please update your CodeQL CLI version or update your workflow to set a distinct category for each CodeQL run. ${deprecationMoreInformationMessage}`,
);
core.exportVariable("CODEQL_MERGE_SARIF_DEPRECATION_WARNING", "true");
}
return combineSarifFiles(sarifFiles, logger);
}

3
src/util.ts
View file

@ -56,8 +56,11 @@ export interface SarifFile {
export interface SarifRun {
tool?: {
driver?: {
guid?: string;
name?: string;
fullName?: string;
semanticVersion?: string;
version?: string;
};
};
automationDetails?: {