Merge pull request #2786 from github/aeisenberg/extensible-predicate-metadata

Copy more files into the synthetic variant analysis pack

Author: aeisenberg

extensions/ql-vscode/CHANGELOG.md

@@ -7,6 +7,7 @@
 - Update how variant analysis results are displayed. For queries with ["path-problem" or "problem" `@kind`](https://codeql.github.com/docs/writing-codeql-queries/metadata-for-codeql-queries/#metadata-properties), you can choose to display the results as rendered alerts or as a table of raw results. For queries with any other `@kind`, the results are displayed as a table. [#2745](https://github.com/github/vscode-codeql/pull/2745) & [#2749](https://github.com/github/vscode-codeql/pull/2749)
 - When running variant analyses, don't download artifacts for repositories with no results. [#2736](https://github.com/github/vscode-codeql/pull/2736)
 - Group the extension settings, so that they're easier to find in the Settings UI. [#2706](https://github.com/github/vscode-codeql/pull/2706)
+- Fix a bug where variant analysis queries would fail for queries in the `codeql/java-queries` query pack. [#2786](https://github.com/github/vscode-codeql/pull/2786)
 
 ## 1.8.10 - 15 August 2023
 

extensions/ql-vscode/src/codeql-cli/cli.ts

@@ -123,6 +123,15 @@ export type ResolveExtensionsResult = {
   };
 };
 
+export type GenerateExtensiblePredicateMetadataResult = {
+  // There are other properties in this object, but they are
+  // not relevant for its use in the extension, so we omit them.
+  extensible_predicates: Array<{
+    // pack relative path
+    path: string;
+  }>;
+};
+
 /**
  * The expected output of `codeql resolve qlref`.
  */
@@ -1458,6 +1467,17 @@ export class CodeQLCliServer implements Disposable {
     );
   }
 
+  async generateExtensiblePredicateMetadata(
+    packRoot: string,
+  ): Promise<GenerateExtensiblePredicateMetadataResult> {
+    return await this.runJsonCodeQlCliCommand(
+      ["generate", "extensible-predicate-metadata"],
+      [packRoot],
+      "Generating extensible predicate metadata",
+      { addFormat: false },
+    );
+  }
+
   public async getVersion() {
     if (!this._version) {
       try {
@@ -1830,6 +1850,14 @@ export class CliVersionConstraint {
    */
   public static CLI_VERSION_WITH_QUICK_EVAL_COUNT = new SemVer("2.13.3");
 
+  /**
+   * CLI version where the `generate extensible-predicate-metadata`
+   * command was implemented.
+   */
+  public static CLI_VERSION_WITH_EXTENSIBLE_PREDICATE_METADATA = new SemVer(
+    "2.14.3",
+  );
+
   /**
    * CLI version where the langauge server supports visisbility change notifications.
    */
@@ -1916,4 +1944,10 @@ export class CliVersionConstraint {
       CliVersionConstraint.CLI_VERSION_WITH_QUICK_EVAL_COUNT,
     );
   }
+
+  async supportsGenerateExtensiblePredicateMetadata() {
+    return this.isVersionAtLeast(
+      CliVersionConstraint.CLI_VERSION_WITH_EXTENSIBLE_PREDICATE_METADATA,
+    );
+  }
 }

extensions/ql-vscode/src/variant-analysis/run-remote-query.ts

@@ -189,6 +189,22 @@ async function copyExistingQueryPack(
 ) {
   const toCopy = await cliServer.packPacklist(originalPackRoot, false);
 
+  // Also include query files that contain extensible predicates. These query files are not
+  // needed for the query to run, but they are needed for the query pack to pass deep validation
+  // of data extensions.
+  if (
+    await cliServer.cliConstraints.supportsGenerateExtensiblePredicateMetadata()
+  ) {
+    const metadata = await cliServer.generateExtensiblePredicateMetadata(
+      originalPackRoot,
+    );
+    metadata.extensible_predicates.forEach((predicate) => {
+      if (predicate.path.endsWith(".ql")) {
+        toCopy.push(join(originalPackRoot, predicate.path));
+      }
+    });
+  }
+
   [
     // also copy the lock file (either new name or old name) and the query file itself. These are not included in the packlist.
     ...QLPACK_LOCK_FILENAMES.map((f) => join(originalPackRoot, f)),

extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts

@@ -2,7 +2,7 @@ import { CancellationTokenSource, commands, Uri, window } from "vscode";
 import { extLogger } from "../../../../src/common/logging/vscode";
 import { setRemoteControllerRepo } from "../../../../src/config";
 import * as ghApiClient from "../../../../src/variant-analysis/gh-api/gh-api-client";
-import { join } from "path";
+import { isAbsolute, join } from "path";
 
 import { VariantAnalysisManager } from "../../../../src/variant-analysis/variant-analysis-manager";
 import {
@@ -275,20 +275,63 @@ describe("Variant Analysis Manager", () => {
       });
     });
 
+    // Test running core java queries to ensure that we can compile queries in packs
+    // that contain queries with extensible predicates
+    it("should run a remote query that is part of the java pack", async () => {
+      if (
+        !(await cli.cliConstraints.supportsGenerateExtensiblePredicateMetadata())
+      ) {
+        console.log(
+          `Skipping test because generating extensible predicate metadata was only introduced in CLI version ${CliVersionConstraint.CLI_VERSION_WITH_EXTENSIBLE_PREDICATE_METADATA}.`,
+        );
+        return;
+      }
+
+      if (!process.env.TEST_CODEQL_PATH) {
+        fail(
+          "TEST_CODEQL_PATH environment variable not set. It should point to the absolute path to a checkout of the codeql repository.",
+        );
+      }
+
+      const queryToRun =
+        "Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql";
+      const extraQuery = "Telemetry/ExtractorInformation.ql";
+
+      await doVariantAnalysisTest({
+        queryPath: join(
+          process.env.TEST_CODEQL_PATH,
+          "java/ql/src",
+          queryToRun,
+        ),
+        expectedPackName: "codeql/java-queries",
+        filesThatExist: [queryToRun, extraQuery],
+        filesThatDoNotExist: [],
+        qlxFilesThatExist: [],
+        dependenciesToCheck: ["codeql/java-all"],
+        // Don't check the version since it will be the same version
+        checkVersion: false,
+      });
+    });
+
     async function doVariantAnalysisTest({
       queryPath,
       expectedPackName,
       filesThatExist,
       qlxFilesThatExist,
       filesThatDoNotExist,
+
+      // A subset of dependencies that we expect should be in the qlpack file.
+      // The first dependency is assumed to be the core library.
       dependenciesToCheck = ["codeql/javascript-all"],
+      checkVersion = true,
     }: {
       queryPath: string;
       expectedPackName: string;
       filesThatExist: string[];
       qlxFilesThatExist: string[];
       filesThatDoNotExist: string[];
       dependenciesToCheck?: string[];
+      checkVersion?: boolean;
     }) {
       const fileUri = getFile(queryPath);
       await variantAnalysisManager.runVariantAnalysis(
@@ -339,11 +382,16 @@ describe("Variant Analysis Manager", () => {
         packFS.fileContents(packFileName).toString("utf-8"),
       );
       expect(qlpackContents.name).toEqual(expectedPackName);
-      expect(qlpackContents.version).toEqual("0.0.0");
-      expect(qlpackContents.dependencies?.["codeql/javascript-all"]).toEqual(
-        "*",
-      );
+      if (checkVersion) {
+        expect(qlpackContents.version).toEqual("0.0.0");
+      }
 
+      // Assume the first dependency to check is the core library.
+      if (dependenciesToCheck.length > 0) {
+        expect(qlpackContents.dependencies?.[dependenciesToCheck[0]]).toEqual(
+          "*",
+        );
+      }
       const qlpackLockContents = load(
         packFS.fileContents("codeql-pack.lock.yml").toString("utf-8"),
       );
@@ -357,7 +405,11 @@ describe("Variant Analysis Manager", () => {
     }
 
     function getFile(file: string): Uri {
-      return Uri.file(join(baseDir, file));
+      if (isAbsolute(file)) {
+        return Uri.file(file);
+      } else {
+        return Uri.file(join(baseDir, file));
+      }
     }
   });
 });