Remove result pruning for CodeQL 2.11.2

This commit is contained in:
Henry Mercer 2023-11-27 12:54:14 +00:00
parent a36fc67ec3
commit fdea2a523d
6 changed files with 5 additions and 292 deletions

36
lib/upload-lib.js generated
View file

@ -26,10 +26,9 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", { value: true });
exports.pruneInvalidResults = exports.validateUniqueCategory = exports.waitForProcessing = exports.buildPayload = exports.validateSarifFileSchema = exports.uploadFromActions = exports.findSarifFilesInDir = exports.populateRunAutomationDetails = void 0;
exports.validateUniqueCategory = exports.waitForProcessing = exports.buildPayload = exports.validateSarifFileSchema = exports.uploadFromActions = exports.findSarifFilesInDir = exports.populateRunAutomationDetails = void 0;
const fs = __importStar(require("fs"));
const path = __importStar(require("path"));
const process_1 = require("process");
const zlib_1 = __importDefault(require("zlib"));
const core = __importStar(require("@actions/core"));
const file_url_1 = __importDefault(require("file-url"));
@ -264,8 +263,6 @@ async function uploadFiles(sarifFiles, repositoryNwo, commitOid, ref, analysisKe
let sarif = combineSarifFiles(sarifFiles);
sarif = await fingerprints.addFingerprints(sarif, sourceRoot, logger);
sarif = populateRunAutomationDetails(sarif, category, analysisKey, environment);
if (process_1.env["CODEQL_DISABLE_SARIF_PRUNING"] !== "true")
sarif = pruneInvalidResults(sarif, logger);
const toolNames = util.getToolNames(sarif);
validateUniqueCategory(sarif);
const sarifPayload = JSON.stringify(sarif);
@ -432,37 +429,6 @@ exports.validateUniqueCategory = validateUniqueCategory;
function sanitize(str) {
return (str ?? "_").replace(/[^a-zA-Z0-9_]/g, "_").toLocaleUpperCase();
}
function pruneInvalidResults(sarif, logger) {
let pruned = 0;
const newRuns = [];
for (const run of sarif.runs || []) {
if (run.tool?.driver?.name === "CodeQL" &&
run.tool?.driver?.semanticVersion === "2.11.2") {
// Version 2.11.2 of the CodeQL CLI had many false positives in the
// rb/weak-cryptographic-algorithm query which we prune here. The
// issue is tracked in https://github.com/github/codeql/issues/11107.
const newResults = [];
for (const result of run.results || []) {
if (result.ruleId === "rb/weak-cryptographic-algorithm" &&
(result.message?.text?.includes(" MD5 ") ||
result.message?.text?.includes(" SHA1 "))) {
pruned += 1;
continue;
}
newResults.push(result);
}
newRuns.push({ ...run, results: newResults });
}
else {
newRuns.push(run);
}
}
if (pruned > 0) {
logger.info(`Pruned ${pruned} results believed to be invalid from SARIF file.`);
}
return { ...sarif, runs: newRuns };
}
exports.pruneInvalidResults = pruneInvalidResults;
/**
* An error that occurred due to an invalid SARIF upload request.
*/

File diff suppressed because one or more lines are too long

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

@ -32,7 +32,6 @@ const ava_1 = __importDefault(require("ava"));
const logging_1 = require("./logging");
const testing_utils_1 = require("./testing-utils");
const uploadLib = __importStar(require("./upload-lib"));
const upload_lib_1 = require("./upload-lib");
const util_1 = require("./util");
(0, testing_utils_1.setupTests)(ava_1.default);
ava_1.default.beforeEach(() => {
@ -184,55 +183,6 @@ ava_1.default.beforeEach(() => {
t.throws(() => uploadLib.validateUniqueCategory(sarif1));
t.throws(() => uploadLib.validateUniqueCategory(sarif2));
});
(0, ava_1.default)("pruneInvalidResults", (t) => {
const loggedMessages = [];
const mockLogger = {
info: (message) => {
loggedMessages.push(message);
},
};
const sarif = {
runs: [
{
tool: otherTool,
results: [resultWithBadMessage1, resultWithGoodMessage],
},
{
tool: affectedCodeQLVersion,
results: [
resultWithOtherRuleId,
resultWithBadMessage1,
resultWithBadMessage2,
resultWithGoodMessage,
],
},
{
tool: unaffectedCodeQLVersion,
results: [resultWithBadMessage1, resultWithGoodMessage],
},
],
};
const result = (0, upload_lib_1.pruneInvalidResults)(sarif, mockLogger);
const expected = {
runs: [
{
tool: otherTool,
results: [resultWithBadMessage1, resultWithGoodMessage],
},
{
tool: affectedCodeQLVersion,
results: [resultWithOtherRuleId, resultWithGoodMessage],
},
{
tool: unaffectedCodeQLVersion,
results: [resultWithBadMessage1, resultWithGoodMessage],
},
],
};
t.deepEqual(result, expected);
t.deepEqual(loggedMessages.length, 1);
t.assert(loggedMessages[0].includes("Pruned 2 results"));
});
(0, ava_1.default)("accept results with invalid artifactLocation.uri value", (t) => {
const loggedMessages = [];
const mockLogger = {
@ -245,56 +195,6 @@ ava_1.default.beforeEach(() => {
t.deepEqual(loggedMessages.length, 1);
t.deepEqual(loggedMessages[0], "Warning: 'not a valid URI' is not a valid URI in 'instance.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri'.");
});
const affectedCodeQLVersion = {
driver: {
name: "CodeQL",
semanticVersion: "2.11.2",
},
};
const unaffectedCodeQLVersion = {
driver: {
name: "CodeQL",
semanticVersion: "2.11.3",
},
};
const otherTool = {
driver: {
name: "Some other tool",
semanticVersion: "2.11.2",
},
};
const resultWithOtherRuleId = {
ruleId: "doNotPrune",
message: {
text: "should not be pruned even though it says MD5 in it",
},
locations: [],
partialFingerprints: {},
};
const resultWithGoodMessage = {
ruleId: "rb/weak-cryptographic-algorithm",
message: {
text: "should not be pruned SHA128 is not a FP",
},
locations: [],
partialFingerprints: {},
};
const resultWithBadMessage1 = {
ruleId: "rb/weak-cryptographic-algorithm",
message: {
text: "should be pruned MD5 is a FP",
},
locations: [],
partialFingerprints: {},
};
const resultWithBadMessage2 = {
ruleId: "rb/weak-cryptographic-algorithm",
message: {
text: "should be pruned SHA1 is a FP",
},
locations: [],
partialFingerprints: {},
};
function createMockSarif(id, tool) {
return {
runs: [

File diff suppressed because one or more lines are too long

View file

@ -6,8 +6,7 @@ import test from "ava";
import { getRunnerLogger, Logger } from "./logging";
import { setupTests } from "./testing-utils";
import * as uploadLib from "./upload-lib";
import { pruneInvalidResults } from "./upload-lib";
import { initializeEnvironment, SarifFile, withTmpDir } from "./util";
import { initializeEnvironment, withTmpDir } from "./util";
setupTests(test);
@ -307,59 +306,6 @@ test("validateUniqueCategory for multiple runs", (t) => {
t.throws(() => uploadLib.validateUniqueCategory(sarif2));
});
test("pruneInvalidResults", (t) => {
const loggedMessages: string[] = [];
const mockLogger = {
info: (message: string) => {
loggedMessages.push(message);
},
} as Logger;
const sarif: SarifFile = {
runs: [
{
tool: otherTool,
results: [resultWithBadMessage1, resultWithGoodMessage],
},
{
tool: affectedCodeQLVersion,
results: [
resultWithOtherRuleId,
resultWithBadMessage1,
resultWithBadMessage2,
resultWithGoodMessage,
],
},
{
tool: unaffectedCodeQLVersion,
results: [resultWithBadMessage1, resultWithGoodMessage],
},
],
};
const result = pruneInvalidResults(sarif, mockLogger);
const expected: SarifFile = {
runs: [
{
tool: otherTool,
results: [resultWithBadMessage1, resultWithGoodMessage],
},
{
tool: affectedCodeQLVersion,
results: [resultWithOtherRuleId, resultWithGoodMessage],
},
{
tool: unaffectedCodeQLVersion,
results: [resultWithBadMessage1, resultWithGoodMessage],
},
],
};
t.deepEqual(result, expected);
t.deepEqual(loggedMessages.length, 1);
t.assert(loggedMessages[0].includes("Pruned 2 results"));
});
test("accept results with invalid artifactLocation.uri value", (t) => {
const loggedMessages: string[] = [];
const mockLogger = {
@ -377,62 +323,6 @@ test("accept results with invalid artifactLocation.uri value", (t) => {
"Warning: 'not a valid URI' is not a valid URI in 'instance.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri'.",
);
});
const affectedCodeQLVersion = {
driver: {
name: "CodeQL",
semanticVersion: "2.11.2",
},
};
const unaffectedCodeQLVersion = {
driver: {
name: "CodeQL",
semanticVersion: "2.11.3",
},
};
const otherTool = {
driver: {
name: "Some other tool",
semanticVersion: "2.11.2",
},
};
const resultWithOtherRuleId = {
ruleId: "doNotPrune",
message: {
text: "should not be pruned even though it says MD5 in it",
},
locations: [],
partialFingerprints: {},
};
const resultWithGoodMessage = {
ruleId: "rb/weak-cryptographic-algorithm",
message: {
text: "should not be pruned SHA128 is not a FP",
},
locations: [],
partialFingerprints: {},
};
const resultWithBadMessage1 = {
ruleId: "rb/weak-cryptographic-algorithm",
message: {
text: "should be pruned MD5 is a FP",
},
locations: [],
partialFingerprints: {},
};
const resultWithBadMessage2 = {
ruleId: "rb/weak-cryptographic-algorithm",
message: {
text: "should be pruned SHA1 is a FP",
},
locations: [],
partialFingerprints: {},
};
function createMockSarif(id?: string, tool?: string) {
return {

View file

@ -1,6 +1,5 @@
import * as fs from "fs";
import * as path from "path";
import { env } from "process";
import zlib from "zlib";
import * as core from "@actions/core";
@ -15,7 +14,7 @@ import * as fingerprints from "./fingerprints";
import { Logger } from "./logging";
import { parseRepositoryNwo, RepositoryNwo } from "./repository";
import * as util from "./util";
import { SarifFile, SarifResult, SarifRun, UserError, wrapError } from "./util";
import { SarifFile, UserError, wrapError } from "./util";
// Takes a list of paths to sarif files and combines them together,
// returning the contents of the combined sarif file.
@ -372,9 +371,6 @@ async function uploadFiles(
environment,
);
if (env["CODEQL_DISABLE_SARIF_PRUNING"] !== "true")
sarif = pruneInvalidResults(sarif, logger);
const toolNames = util.getToolNames(sarif);
validateUniqueCategory(sarif);
@ -596,45 +592,6 @@ function sanitize(str?: string) {
return (str ?? "_").replace(/[^a-zA-Z0-9_]/g, "_").toLocaleUpperCase();
}
export function pruneInvalidResults(
sarif: SarifFile,
logger: Logger,
): SarifFile {
let pruned = 0;
const newRuns: SarifRun[] = [];
for (const run of sarif.runs || []) {
if (
run.tool?.driver?.name === "CodeQL" &&
run.tool?.driver?.semanticVersion === "2.11.2"
) {
// Version 2.11.2 of the CodeQL CLI had many false positives in the
// rb/weak-cryptographic-algorithm query which we prune here. The
// issue is tracked in https://github.com/github/codeql/issues/11107.
const newResults: SarifResult[] = [];
for (const result of run.results || []) {
if (
result.ruleId === "rb/weak-cryptographic-algorithm" &&
(result.message?.text?.includes(" MD5 ") ||
result.message?.text?.includes(" SHA1 "))
) {
pruned += 1;
continue;
}
newResults.push(result);
}
newRuns.push({ ...run, results: newResults });
} else {
newRuns.push(run);
}
}
if (pruned > 0) {
logger.info(
`Pruned ${pruned} results believed to be invalid from SARIF file.`,
);
}
return { ...sarif, runs: newRuns };
}
/**
* An error that occurred due to an invalid SARIF upload request.
*/