Merge pull request #1345 from github/update-v2.1.31-a8cabafa

Merge main into releases/v2

CHANGELOG.md

@@ -1,5 +1,9 @@
 # CodeQL Action Changelog
 
+## 2.1.31 - 04 Nov 2022
+
+- The `rb/weak-cryptographic-algorithm` Ruby query has been updated to no longer report uses of hash functions such as `MD5` and `SHA1` even if they are known to be weak. These hash algorithms are used very often in non-sensitive contexts, making the query too imprecise in practice. For more information, see the corresponding change in the [github/codeql repository](https://github.com/github/codeql/pull/11129). [#1344](https://github.com/github/codeql-action/pull/1344)
+
 ## 2.1.30 - 02 Nov 2022
 
 - Improve the error message when using CodeQL bundle version 2.7.2 and earlier in a workflow that runs on a runner image such as `ubuntu-22.04` that uses glibc version 2.34 and later. [#1334](https://github.com/github/codeql-action/pull/1334)

lib/upload-lib.js

@@ -22,9 +22,10 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
     return (mod && mod.__esModule) ? mod : { "default": mod };
 };
 Object.defineProperty(exports, "__esModule", { value: true });
-exports.validateUniqueCategory = exports.waitForProcessing = exports.buildPayload = exports.validateSarifFileSchema = exports.countResultsInSarif = exports.uploadFromRunner = exports.uploadFromActions = exports.findSarifFilesInDir = exports.populateRunAutomationDetails = exports.combineSarifFiles = void 0;
+exports.pruneInvalidResults = exports.validateUniqueCategory = exports.waitForProcessing = exports.buildPayload = exports.validateSarifFileSchema = exports.countResultsInSarif = exports.uploadFromRunner = exports.uploadFromActions = exports.findSarifFilesInDir = exports.populateRunAutomationDetails = exports.combineSarifFiles = 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"));
@@ -269,6 +270,8 @@ 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);
@@ -376,4 +379,36 @@ exports.validateUniqueCategory = validateUniqueCategory;
 function sanitize(str) {
     return (str !== null && str !== void 0 ? str : "_").replace(/[^a-zA-Z0-9_]/g, "_").toLocaleUpperCase();
 }
+function pruneInvalidResults(sarif, logger) {
+    var _a, _b, _c, _d, _e, _f, _g, _h;
+    let pruned = 0;
+    const newRuns = [];
+    for (const run of sarif.runs || []) {
+        if (((_b = (_a = run.tool) === null || _a === void 0 ? void 0 : _a.driver) === null || _b === void 0 ? void 0 : _b.name) === "CodeQL" &&
+            ((_d = (_c = run.tool) === null || _c === void 0 ? void 0 : _c.driver) === null || _d === void 0 ? void 0 : _d.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" &&
+                    (((_f = (_e = result.message) === null || _e === void 0 ? void 0 : _e.text) === null || _f === void 0 ? void 0 : _f.includes(" MD5 ")) ||
+                        ((_h = (_g = result.message) === null || _g === void 0 ? void 0 : _g.text) === null || _h === void 0 ? void 0 : _h.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;
 //# sourceMappingURL=upload-lib.js.map

lib/upload-lib.test.js

@@ -28,6 +28,7 @@ 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(() => {
@@ -200,6 +201,105 @@ 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"));
+});
+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: [

node_modules/.package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "codeql",
-  "version": "2.1.30",
+  "version": "2.1.31",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "codeql",
-  "version": "2.1.30",
+  "version": "2.1.31",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "codeql",
-      "version": "2.1.30",
+      "version": "2.1.31",
       "license": "MIT",
       "dependencies": {
         "@actions/artifact": "^1.1.0",

package.json

@@ -1,6 +1,6 @@
 {
   "name": "codeql",
-  "version": "2.1.30",
+  "version": "2.1.31",
   "private": true,
   "description": "CodeQL action",
   "scripts": {

src/upload-lib.test.ts

@@ -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: [

src/upload-lib.ts

@@ -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 };
+}

src/util.ts

@@ -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: {