Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions .github/workflows/__ml-powered-queries.yml

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

8 changes: 7 additions & 1 deletion lib/codeql.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/codeql.js.map

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions lib/config-utils.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/config-utils.js.map

Large diffs are not rendered by default.

22 changes: 15 additions & 7 deletions lib/config-utils.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/config-utils.test.js.map

Large diffs are not rendered by default.

19 changes: 14 additions & 5 deletions pr-checks/checks/ml-powered-queries.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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
7 changes: 7 additions & 0 deletions src/codeql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
52 changes: 37 additions & 15 deletions src/config-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1807,42 +1807,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",
process.platform === "win32" ? undefined : "0.0.1"
henrymercer marked this conversation as resolved.
Show resolved Hide resolved
);
9 changes: 7 additions & 2 deletions src/config-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -300,8 +301,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(
Expand Down