Skip to content

Commit

Permalink
Remove result pruning for CodeQL 2.11.2
Browse files Browse the repository at this point in the history
  • Loading branch information
Henry Mercer committed Nov 27, 2023
1 parent a36fc67 commit fdea2a5
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 292 deletions.
36 changes: 1 addition & 35 deletions lib/upload-lib.js

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

2 changes: 1 addition & 1 deletion lib/upload-lib.js.map

Large diffs are not rendered by default.

100 changes: 0 additions & 100 deletions lib/upload-lib.test.js

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

2 changes: 1 addition & 1 deletion lib/upload-lib.test.js.map

Large diffs are not rendered by default.

112 changes: 1 addition & 111 deletions src/upload-lib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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 = {
Expand All @@ -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 {
Expand Down
45 changes: 1 addition & 44 deletions src/upload-lib.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
*/
Expand Down

0 comments on commit fdea2a5

Please sign in to comment.