Prune results of Ruby query from SARIF

This commit is contained in:
Edoardo Pirovano
2022-11-04 13:58:27 +00:00
parent 71510779c2
commit 862a512899
8 changed files with 316 additions and 19 deletions

View File

@@ -3,14 +3,16 @@ import * as path from "path";
import test from "ava";
import { getRunnerLogger } from "./logging";
import { getRunnerLogger, Logger } from "./logging";
import { setupTests } from "./testing-utils";
import * as uploadLib from "./upload-lib";
import { pruneInvalidResults } from "./upload-lib";
import {
GitHubVariant,
GitHubVersion,
initializeEnvironment,
Mode,
GitHubVersion,
GitHubVariant,
SarifFile,
withTmpDir,
} from "./util";
@@ -344,6 +346,116 @@ 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"));
});
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 {
runs: [

View File

@@ -1,5 +1,6 @@
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 +16,7 @@ import { Logger } from "./logging";
import { parseRepositoryNwo, RepositoryNwo } from "./repository";
import * as sharedEnv from "./shared-environment";
import * as util from "./util";
import { SarifFile } from "./util";
import { SarifFile, SarifResult, SarifRun } from "./util";
// Takes a list of paths to sarif files and combines them together,
// returning the contents of the combined sarif file.
@@ -396,6 +397,9 @@ async function uploadFiles(
environment
);
if (env["CODEQL_DISABLE_SARIF_PRUNING"] !== "true")
sarif = pruneInvalidResults(sarif, logger);
const toolNames = util.getToolNames(sarif);
validateUniqueCategory(sarif);
@@ -546,3 +550,42 @@ export function validateUniqueCategory(sarif: SarifFile): void {
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 };
}

View File

@@ -52,21 +52,28 @@ export const DID_AUTOBUILD_GO_ENV_VAR_NAME =
export interface SarifFile {
version?: string | null;
runs: Array<{
tool?: {
driver?: {
name?: string;
};
runs: SarifRun[];
}
export interface SarifRun {
tool?: {
driver?: {
name?: string;
semanticVersion?: string;
};
automationDetails?: {
id?: string;
};
artifacts?: string[];
results?: SarifResult[];
}>;
};
automationDetails?: {
id?: string;
};
artifacts?: string[];
results?: SarifResult[];
}
export interface SarifResult {
ruleId?: string;
message?: {
text?: string;
};
locations: Array<{
physicalLocation: {
artifactLocation: {