Merge pull request #2072 from github/aeisenberg/mrva-extension-packs

Inject extension pack dependencies into MRVA packs

extensions/ql-vscode/src/cli.ts

@@ -1284,11 +1284,25 @@ export class CodeQLCliServer implements Disposable {
     );
   }
 
-  async packInstall(dir: string, forceUpdate = false) {
+  async packInstall(
+    dir: string,
+    { forceUpdate = false, workspaceFolders = [] as string[] } = {},
+  ) {
     const args = [dir];
     if (forceUpdate) {
       args.push("--mode", "update");
     }
+    if (workspaceFolders?.length > 0) {
+      if (await this.cliConstraints.supportsAdditionalPacksInstall()) {
+        args.push(
+          // Allow prerelease packs from the ql submodule.
+          "--allow-prerelease",
+          // Allow the use of --additional-packs argument without issueing a warning
+          "--no-strict-mode",
+          ...this.getAdditionalPacksArg(workspaceFolders),
+        );
+      }
+    }
     return this.runJsonCodeQlCliCommandWithAuthentication(
       ["pack", "install"],
       args,
@@ -1692,6 +1706,13 @@ export class CliVersionConstraint {
    */
   public static CLI_VERSION_WITH_QLPACKS_KIND = new SemVer("2.12.3");
 
+  /**
+   * CLI version that supports the `--additional-packs` option for the `pack install` command.
+   */
+  public static CLI_VERSION_WITH_ADDITIONAL_PACKS_INSTALL = new SemVer(
+    "2.12.4",
+  );
+
   constructor(private readonly cli: CodeQLCliServer) {
     /**/
   }
@@ -1755,4 +1776,10 @@ export class CliVersionConstraint {
       CliVersionConstraint.CLI_VERSION_WITH_QLPACKS_KIND,
     );
   }
+
+  async supportsAdditionalPacksInstall() {
+    return this.isVersionAtLeast(
+      CliVersionConstraint.CLI_VERSION_WITH_ADDITIONAL_PACKS_INSTALL,
+    );
+  }
 }

extensions/ql-vscode/src/pure/ql.ts

@@ -2,6 +2,10 @@ import { join } from "path";
 import { pathExists } from "fs-extra";
 
 export const QLPACK_FILENAMES = ["qlpack.yml", "codeql-pack.yml"];
+export const QLPACK_LOCK_FILENAMES = [
+  "qlpack.lock.yml",
+  "codeql-pack.lock.yml",
+];
 export const FALLBACK_QLPACK_FILENAME = QLPACK_FILENAMES[0];
 
 export async function getQlPackPath(

extensions/ql-vscode/src/quick-query.ts

@@ -143,7 +143,7 @@ export async function displayQuickQuery(
 
     if (shouldRewrite) {
       await cliServer.clearCache();
-      await cliServer.packInstall(queriesDir, true);
+      await cliServer.packInstall(queriesDir, { forceUpdate: true });
     }
 
     await Window.showTextDocument(await workspace.openTextDocument(qlFile));

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

@@ -34,6 +34,7 @@ import {
   getQlPackPath,
   FALLBACK_QLPACK_FILENAME,
   QLPACK_FILENAMES,
+  QLPACK_LOCK_FILENAMES,
 } from "../pure/ql";
 
 export interface QlPack {
@@ -70,42 +71,23 @@ async function generateQueryPack(
   const originalPackRoot = await findPackRoot(queryFile);
   const packRelativePath = relative(originalPackRoot, queryFile);
   const targetQueryFileName = join(queryPackDir, packRelativePath);
+  const workspaceFolders = getOnDiskWorkspaceFolders();
 
   let language: string | undefined;
+
+  // Check if the query is already in a query pack.
+  // If so, copy the entire query pack to the temporary directory.
+  // Otherwise, copy only the query file to the temporary directory
+  // and generate a synthetic query pack.
   if (await getQlPackPath(originalPackRoot)) {
     // don't include ql files. We only want the queryFile to be copied.
-    const toCopy = await cliServer.packPacklist(originalPackRoot, false);
-
-    // also copy the lock file (either new name or old name) and the query file itself. These are not included in the packlist.
-    [
-      join(originalPackRoot, "qlpack.lock.yml"),
-      join(originalPackRoot, "codeql-pack.lock.yml"),
+    await copyExistingQueryPack(
+      cliServer,
+      originalPackRoot,
       queryFile,
-    ].forEach((absolutePath) => {
-      if (absolutePath) {
-        toCopy.push(absolutePath);
-      }
-    });
-
-    let copiedCount = 0;
-    await copy(originalPackRoot, queryPackDir, {
-      filter: (file: string) =>
-        // copy file if it is in the packlist, or it is a parent directory of a file in the packlist
-        !!toCopy.find((f) => {
-          // Normalized paths ensure that Windows drive letters are capitalized consistently.
-          const normalizedPath = Uri.file(f).fsPath;
-          const matches =
-            normalizedPath === file || normalizedPath.startsWith(file + sep);
-          if (matches) {
-            copiedCount++;
-          }
-          return matches;
-        }),
-    });
-
-    void extLogger.log(`Copied ${copiedCount} files to ${queryPackDir}`);
-
-    await fixPackFile(queryPackDir, packRelativePath);
+      queryPackDir,
+      packRelativePath,
+    );
 
     language = await findLanguage(cliServer, Uri.file(targetQueryFileName));
   } else {
@@ -114,20 +96,12 @@ async function generateQueryPack(
 
     // copy only the query file to the query pack directory
     // and generate a synthetic query pack
-    void extLogger.log(`Copying ${queryFile} to ${queryPackDir}`);
-    await copy(queryFile, targetQueryFileName);
-    void extLogger.log("Generating synthetic query pack");
-    const syntheticQueryPack = {
-      name: QUERY_PACK_NAME,
-      version: "0.0.0",
-      dependencies: {
-        [`codeql/${language}-all`]: "*",
-      },
-      defaultSuite: generateDefaultSuite(packRelativePath),
-    };
-    await writeFile(
-      join(queryPackDir, FALLBACK_QLPACK_FILENAME),
-      dump(syntheticQueryPack),
+    await createNewQueryPack(
+      queryFile,
+      queryPackDir,
+      targetQueryFileName,
+      language,
+      packRelativePath,
     );
   }
   if (!language) {
@@ -149,12 +123,21 @@ async function generateQueryPack(
     precompilationOpts = ["--no-precompile"];
   }
 
+  if (await cliServer.useExtensionPacks()) {
+    await injectExtensionPacks(cliServer, queryPackDir, workspaceFolders);
+  }
+
+  await cliServer.packInstall(queryPackDir, {
+    workspaceFolders,
+  });
+
+  // Clear the CLI cache so that the most recent qlpack lock file is used.
+  await cliServer.clearCache();
+
   const bundlePath = await getPackedBundlePath(queryPackDir);
   void extLogger.log(
     `Compiling and bundling query pack from ${queryPackDir} to ${bundlePath}. (This may take a while.)`,
   );
-  await cliServer.packInstall(queryPackDir);
-  const workspaceFolders = getOnDiskWorkspaceFolders();
   await cliServer.packBundle(
     queryPackDir,
     workspaceFolders,
@@ -168,6 +151,70 @@ async function generateQueryPack(
   };
 }
 
+async function createNewQueryPack(
+  queryFile: string,
+  queryPackDir: string,
+  targetQueryFileName: string,
+  language: string | undefined,
+  packRelativePath: string,
+) {
+  void extLogger.log(`Copying ${queryFile} to ${queryPackDir}`);
+  await copy(queryFile, targetQueryFileName);
+  void extLogger.log("Generating synthetic query pack");
+  const syntheticQueryPack = {
+    name: QUERY_PACK_NAME,
+    version: "0.0.0",
+    dependencies: {
+      [`codeql/${language}-all`]: "*",
+    },
+    defaultSuite: generateDefaultSuite(packRelativePath),
+  };
+  await writeFile(
+    join(queryPackDir, FALLBACK_QLPACK_FILENAME),
+    dump(syntheticQueryPack),
+  );
+}
+
+async function copyExistingQueryPack(
+  cliServer: cli.CodeQLCliServer,
+  originalPackRoot: string,
+  queryFile: string,
+  queryPackDir: string,
+  packRelativePath: string,
+) {
+  const toCopy = await cliServer.packPacklist(originalPackRoot, false);
+
+  [
+    // 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)),
+    queryFile,
+  ].forEach((absolutePath) => {
+    if (absolutePath) {
+      toCopy.push(absolutePath);
+    }
+  });
+
+  let copiedCount = 0;
+  await copy(originalPackRoot, queryPackDir, {
+    filter: (file: string) =>
+      // copy file if it is in the packlist, or it is a parent directory of a file in the packlist
+      !!toCopy.find((f) => {
+        // Normalized paths ensure that Windows drive letters are capitalized consistently.
+        const normalizedPath = Uri.file(f).fsPath;
+        const matches =
+          normalizedPath === file || normalizedPath.startsWith(file + sep);
+        if (matches) {
+          copiedCount++;
+        }
+        return matches;
+      }),
+  });
+
+  void extLogger.log(`Copied ${copiedCount} files to ${queryPackDir}`);
+
+  await fixPackFile(queryPackDir, packRelativePath);
+}
+
 async function findPackRoot(queryFile: string): Promise<string> {
   // recursively find the directory containing qlpack.yml or codeql-pack.yml
   let dir = dirname(queryFile);
@@ -329,19 +376,54 @@ async function fixPackFile(
   }
   const qlpack = load(await readFile(packPath, "utf8")) as QlPack;
 
-  // update pack name
   qlpack.name = QUERY_PACK_NAME;
-
-  // update default suite
-  delete qlpack.defaultSuiteFile;
-  qlpack.defaultSuite = generateDefaultSuite(packRelativePath);
-
-  // remove any ${workspace} version references
+  updateDefaultSuite(qlpack, packRelativePath);
   removeWorkspaceRefs(qlpack);
 
   await writeFile(packPath, dump(qlpack));
 }
 
+async function injectExtensionPacks(
+  cliServer: cli.CodeQLCliServer,
+  queryPackDir: string,
+  workspaceFolders: string[],
+) {
+  const qlpackFile = await getQlPackPath(queryPackDir);
+  if (!qlpackFile) {
+    throw new Error(
+      `Could not find ${QLPACK_FILENAMES.join(
+        " or ",
+      )} file in '${queryPackDir}'`,
+    );
+  }
+  const syntheticQueryPack = load(await readFile(qlpackFile, "utf8")) as QlPack;
+
+  const extensionPacks = await cliServer.resolveQlpacks(workspaceFolders, true);
+  Object.entries(extensionPacks).forEach(([name, paths]) => {
+    // We are guaranteed that there is at least one path found for each extension pack.
+    // If there are multiple paths, then we have a problem. This means that there is
+    // ambiguity in which path to use. This is an error.
+    if (paths.length > 1) {
+      throw new Error(
+        `Multiple versions of extension pack '${name}' found: ${paths.join(
+          ", ",
+        )}`,
+      );
+    }
+    // Add this extension pack as a dependency. It doesn't matter which
+    // version we specify, since we are guaranteed that the extension pack
+    // is resolved from source at the given path.
+    syntheticQueryPack.dependencies[name] = "*";
+  });
+  await writeFile(qlpackFile, dump(syntheticQueryPack));
+  await cliServer.clearCache();
+}
+
+function updateDefaultSuite(qlpack: QlPack, packRelativePath: string) {
+  delete qlpack.defaultSuiteFile;
+  qlpack.defaultSuite = generateDefaultSuite(packRelativePath);
+}
+
 function generateDefaultSuite(packRelativePath: string) {
   return [
     {

extensions/ql-vscode/test/vscode-tests/cli-integration/data-extension-pack/extension-file.yml

@@ -0,0 +1,12 @@
+extensions:
+  - addsTo:
+      pack: codeql/javascript-all
+      extensible: sourceModel
+    data:
+      - [ "@example/read-write-user-data", "Member[readUserData].ReturnValue.Awaited", "remote" ]
+
+  - addsTo:
+      pack: codeql/javascript-all
+      extensible: sinkModel
+    data:
+      - [ "@example/read-write-user-data", "Member[writeUserData].Argument[0]", "command-line-injection" ]

extensions/ql-vscode/test/vscode-tests/cli-integration/data-extension-pack/qlpack.yml

@@ -0,0 +1,8 @@
+name: github/extension-pack-for-testing
+version: 0.0.0
+library: true
+extensionTargets:
+  github/remote-query-pack: "*"
+
+dataExtensions:
+  - extension-file.yml

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

@@ -12,7 +12,7 @@ import * as ghApiClient from "../../../../src/variant-analysis/gh-api/gh-api-cli
 import { join } from "path";
 
 import { VariantAnalysisManager } from "../../../../src/variant-analysis/variant-analysis-manager";
-import { CodeQLCliServer } from "../../../../src/cli";
+import { CliVersionConstraint, CodeQLCliServer } from "../../../../src/cli";
 import {
   fixWorkspaceReferences,
   restoreWorkspaceReferences,
@@ -255,6 +255,30 @@ describe("Variant Analysis Manager", () => {
           qlxFilesThatExist: ["subfolder/in-pack.qlx"],
         });
       });
+
+      it("should run a remote query with extension packs inside a qlpack", async () => {
+        if (!(await cli.cliConstraints.supportsQlpacksKind())) {
+          console.log(
+            `Skipping test because qlpacks kind is only suppported in CLI version ${CliVersionConstraint.CLI_VERSION_WITH_QLPACKS_KIND} or later.`,
+          );
+          return;
+        }
+        await cli.setUseExtensionPacks(true);
+        await doVariantAnalysisTest({
+          queryPath: "data-remote-qlpack-nested/subfolder/in-pack.ql",
+          filesThatExist: [
+            "subfolder/in-pack.ql",
+            "otherfolder/lib.qll",
+            ".codeql/libraries/semmle/targets-extension/0.0.0/ext/extension.yml",
+          ],
+          filesThatDoNotExist: ["subfolder/not-in-pack.ql"],
+          qlxFilesThatExist: ["subfolder/in-pack.qlx"],
+          dependenciesToCheck: [
+            "codeql/javascript-all",
+            "semmle/targets-extension",
+          ],
+        });
+      });
     });
 
     async function doVariantAnalysisTest({
@@ -262,11 +286,13 @@ describe("Variant Analysis Manager", () => {
       filesThatExist,
       qlxFilesThatExist,
       filesThatDoNotExist,
+      dependenciesToCheck = ["codeql/javascript-all"],
     }: {
       queryPath: string;
       filesThatExist: string[];
       qlxFilesThatExist: string[];
       filesThatDoNotExist: string[];
+      dependenciesToCheck?: string[];
     }) {
       const fileUri = getFile(queryPath);
       await variantAnalysisManager.runVariantAnalysis(
@@ -328,8 +354,10 @@ describe("Variant Analysis Manager", () => {
 
       const actualLockKeys = Object.keys(qlpackLockContents.dependencies);
 
-      // The lock file should contain at least codeql/javascript-all.
-      expect(actualLockKeys).toContain("codeql/javascript-all");
+      // The lock file should contain at least the specified dependencies.
+      dependenciesToCheck.forEach((dep) =>
+        expect(actualLockKeys).toContain(dep),
+      );
     }
 
     function getFile(file: string): Uri {