Merge pull request #1051 from github/henrymercer/run-atm-on-windows

Run ML-powered queries on Windows with CodeQL CLI 2.9.0+

pr-checks/checks/ml-powered-queries.yml

@@ -32,10 +32,19 @@ steps:
       retention-days: 7
 
   - name: Check results
+    # Running ML-powered queries on Windows requires CodeQL CLI 2.9.0+. We don't run these checks
+    # against Windows and `cached` while CodeQL CLI 2.9.0 makes its way into `cached` to avoid the
+    # test starting to fail when the cached CodeQL Bundle gets updated. Once the CodeQL Bundle
+    # containing CodeQL CLI 2.9.0 has been fully released, we can drop this line and start running
+    # these checks on Windows and `cached`.
+    if: matrix.os != 'windows-latest' || matrix.version != 'cached'
     env:
-      IS_WINDOWS: ${{ matrix.os == 'windows-latest' }}
+      # Running on Windows requires CodeQL CLI 2.9.0+, which has so far only made it to 'latest'.
+      SHOULD_RUN_ML_POWERED_QUERIES: ${{ matrix.os != 'windows-latest' || matrix.version == 'latest' || matrix.version == 'nightly-latest' }}
     shell: bash
     run: |
+      echo "Expecting ML-powered queries to be run: ${SHOULD_RUN_ML_POWERED_QUERIES}"
+
       cd "$RUNNER_TEMP/results"
       # We should run at least the ML-powered queries in `expected_rules`.
       expected_rules="js/ml-powered/nosql-injection js/ml-powered/path-injection js/ml-powered/sql-injection js/ml-powered/xss"
@@ -44,10 +53,10 @@ steps:
         found_rule=$(jq --arg rule "${rule}" '[.runs[0].tool.extensions[].rules | select(. != null) |
           flatten | .[].id] | any(. == $rule)' javascript.sarif)
         echo "Did find rule '${rule}': ${found_rule}"
-        if [[ "${found_rule}" != "true" && "${IS_WINDOWS}" != "true" ]]; then
+        if [[ "${found_rule}" != "true" && "${SHOULD_RUN_ML_POWERED_QUERIES}" == "true" ]]; then
           echo "Expected SARIF output to contain rule '${rule}', but found no such rule."
           exit 1
-        elif [[ "${found_rule}" == "true" && "${IS_WINDOWS}" == "true" ]]; then
+        elif [[ "${found_rule}" == "true" && "${SHOULD_RUN_ML_POWERED_QUERIES}" != "true" ]]; then
           echo "Found rule '${rule}' in the SARIF output which shouldn't have been part of the analysis."
           exit 1
         fi
@@ -58,10 +67,10 @@ steps:
         select(.properties.score != null and (.rule.id | startswith("js/ml-powered/")))] | length' \
         javascript.sarif)
       echo "Found ${num_alerts} alerts from ML-powered queries.";
-      if [[ "${num_alerts}" -eq 0 && "${IS_WINDOWS}" != "true" ]]; then
+      if [[ "${num_alerts}" -eq 0 && "${SHOULD_RUN_ML_POWERED_QUERIES}" == "true" ]]; then
         echo "Expected to find at least one alert from an ML-powered query but found ${num_alerts}."
         exit 1
-      elif [[ "${num_alerts}" -ne 0 && "${IS_WINDOWS}" == "true" ]]; then
+      elif [[ "${num_alerts}" -ne 0 && "${SHOULD_RUN_ML_POWERED_QUERIES}" != "true" ]]; then
         echo "Expected not to find any alerts from an ML-powered query but found ${num_alerts}."
         exit 1
       fi

src/codeql.ts

@@ -232,6 +232,13 @@ export const CODEQL_VERSION_ML_POWERED_QUERIES = "2.7.5";
  */
 export const CODEQL_VERSION_NEW_TRACING = "2.7.0";
 
+/**
+ * Versions 2.9.0+ of the CodeQL CLI run machine learning models from a temporary directory, which
+ * resolves an issue on Windows where TensorFlow models are not correctly loaded due to the path of
+ * some of their files being greater than MAX_PATH (260 characters).
+ */
+export const CODEQL_VERSION_ML_POWERED_QUERIES_WINDOWS = "2.9.0";
+
 function getCodeQLBundleName(): string {
   let platform: string;
   if (process.platform === "win32") {

src/config-utils.test.ts

@@ -1804,42 +1804,64 @@ test(
   "security-extended",
   undefined
 );
+// Test that the ~0.1.0 version of ML-powered queries is run on v2.8.3 of the CLI.
+test(
+  mlPoweredQueriesMacro,
+  "2.8.3",
+  true,
+  undefined,
+  "security-extended",
+  process.platform === "win32" ? undefined : "~0.1.0"
+);
 // Test that ML-powered queries aren't run when the user hasn't specified that we should run the
 // `security-extended` or `security-and-quality` query suite.
 test(mlPoweredQueriesMacro, "2.7.5", true, undefined, undefined, undefined);
-// Test that ML-powered queries are run on non-Windows platforms running `security-extended`.
+// Test that ML-powered queries are run on non-Windows platforms running `security-extended` on
+// versions of the CodeQL CLI prior to 2.9.0.
 test(
   mlPoweredQueriesMacro,
-  "2.7.5",
+  "2.8.5",
   true,
   undefined,
   "security-extended",
-  process.platform === "win32" ? undefined : "~0.1.0"
+  process.platform === "win32" ? undefined : "~0.2.0"
 );
-// Test that ML-powered queries are run on non-Windows platforms running `security-and-quality`.
+// Test that ML-powered queries are run on non-Windows platforms running `security-and-quality` on
+// versions of the CodeQL CLI prior to 2.9.0.
 test(
   mlPoweredQueriesMacro,
-  "2.7.5",
+  "2.8.5",
   true,
   undefined,
   "security-and-quality",
-  process.platform === "win32" ? undefined : "~0.1.0"
+  process.platform === "win32" ? undefined : "~0.2.0"
 );
-// Test that we don't inject an ML-powered query pack if the user has already specified one.
+// Test that ML-powered queries are run on all platforms running `security-extended` on CodeQL CLI
+// 2.9.0+.
 test(
   mlPoweredQueriesMacro,
-  "2.7.5",
+  "2.9.0",
   true,
-  "codeql/javascript-experimental-atm-queries@0.0.1",
-  "security-and-quality",
-  process.platform === "win32" ? undefined : "0.0.1"
+  undefined,
+  "security-extended",
+  "~0.2.0"
 );
-// Test that the ~0.2.0 version of ML-powered queries is run on v2.8.4 of the CLI.
+// Test that ML-powered queries are run on all platforms running `security-and-quality` on CodeQL
+// CLI 2.9.0+.
 test(
   mlPoweredQueriesMacro,
-  "2.8.4",
+  "2.9.0",
   true,
   undefined,
-  "security-extended",
-  process.platform === "win32" ? undefined : "~0.2.0"
+  "security-and-quality",
+  "~0.2.0"
+);
+// Test that we don't inject an ML-powered query pack if the user has already specified one.
+test(
+  mlPoweredQueriesMacro,
+  "2.9.0",
+  true,
+  "codeql/javascript-experimental-atm-queries@0.0.1",
+  "security-and-quality",
+  "0.0.1"
 );

src/config-utils.ts

@@ -8,6 +8,7 @@ import * as api from "./api-client";
 import {
   CodeQL,
   CODEQL_VERSION_ML_POWERED_QUERIES,
+  CODEQL_VERSION_ML_POWERED_QUERIES_WINDOWS,
   ResolveQueriesOutput,
 } from "./codeql";
 import * as externalQueries from "./external-queries";
@@ -293,8 +294,12 @@ async function addBuiltinSuiteQueries(
   // opted into the ML-powered queries beta, and a user hasn't already added the ML-powered query
   // pack, then add the ML-powered query pack so that we run ML-powered queries.
   if (
-    // Disable ML-powered queries on Windows
-    process.platform !== "win32" &&
+    // Only run ML-powered queries on Windows if we have a CLI that supports it.
+    (process.platform !== "win32" ||
+      (await codeQlVersionAbove(
+        codeQL,
+        CODEQL_VERSION_ML_POWERED_QUERIES_WINDOWS
+      ))) &&
     languages.includes("javascript") &&
     (found === "security-extended" || found === "security-and-quality") &&
     !packs.javascript?.some(isMlPoweredJsQueriesPack) &&