Allow pack specifiers to include paths

Also, this cleans up our pack-related integration tests.
We are now testing with the most recent CLIs.

.github/workflows/__packaging-config-inputs-js.yml

@@ -26,9 +26,27 @@ jobs:
       matrix:
         include:
         - os: ubuntu-latest
-          version: nightly-20210831
+          version: latest
         - os: macos-latest
-          version: nightly-20210831
+          version: latest
+        - os: windows-2019
+          version: latest
+        - os: windows-2022
+          version: latest
+        - os: ubuntu-latest
+          version: cached
+        - os: macos-latest
+          version: cached
+        - os: windows-2019
+          version: cached
+        - os: ubuntu-latest
+          version: nightly-latest
+        - os: macos-latest
+          version: nightly-latest
+        - os: windows-2019
+          version: nightly-latest
+        - os: windows-2022
+          version: nightly-latest
     name: 'Packaging: Config and input'
     timeout-minutes: 45
     runs-on: ${{ matrix.os }}
@@ -43,7 +61,7 @@ jobs:
     - uses: ./../action/init
       with:
         config-file: .github/codeql/codeql-config-packaging3.yml
-        packs: +dsp-testing/codeql-pack1@0.1.0
+        packs: +dsp-testing/codeql-pack1@1.0.0
         languages: javascript
         tools: ${{ steps.prepare-test.outputs.tools-url }}
     - name: Build code
@@ -58,11 +76,11 @@ jobs:
       shell: bash
       run: |
         cd "$RUNNER_TEMP/results"
-        # We should have 3 hits from these rules
+        # We should have 4 hits from these rules
-        EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/two-block"
+        EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/other-query-block javascript/example/two-block"
 
         # use tr to replace newlines with spaces and xargs to trim leading and trailing whitespace
-        RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | xargs)"
+        RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | tr "\r" " " | xargs)"
         echo "Found matching rules '$RULES'"
         if [ "$RULES" != "$EXPECTED_RULES" ]; then
           echo "Did not match expected rules '$EXPECTED_RULES'."

.github/workflows/__packaging-config-js.yml

@@ -26,9 +26,27 @@ jobs:
       matrix:
         include:
         - os: ubuntu-latest
-          version: nightly-20210831
+          version: latest
         - os: macos-latest
-          version: nightly-20210831
+          version: latest
+        - os: windows-2019
+          version: latest
+        - os: windows-2022
+          version: latest
+        - os: ubuntu-latest
+          version: cached
+        - os: macos-latest
+          version: cached
+        - os: windows-2019
+          version: cached
+        - os: ubuntu-latest
+          version: nightly-latest
+        - os: macos-latest
+          version: nightly-latest
+        - os: windows-2019
+          version: nightly-latest
+        - os: windows-2022
+          version: nightly-latest
     name: 'Packaging: Config file'
     timeout-minutes: 45
     runs-on: ${{ matrix.os }}
@@ -57,11 +75,11 @@ jobs:
       shell: bash
       run: |
         cd "$RUNNER_TEMP/results"
-        # We should have 3 hits from these rules
+        # We should have 4 hits from these rules
-        EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/two-block"
+        EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/other-query-block javascript/example/two-block"
 
         # use tr to replace newlines with spaces and xargs to trim leading and trailing whitespace
-        RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | xargs)"
+        RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | tr "\r" " " | xargs)"
         echo "Found matching rules '$RULES'"
         if [ "$RULES" != "$EXPECTED_RULES" ]; then
           echo "Did not match expected rules '$EXPECTED_RULES'."

.github/workflows/__packaging-inputs-js.yml

@@ -26,9 +26,27 @@ jobs:
       matrix:
         include:
         - os: ubuntu-latest
-          version: nightly-20210831
+          version: latest
         - os: macos-latest
-          version: nightly-20210831
+          version: latest
+        - os: windows-2019
+          version: latest
+        - os: windows-2022
+          version: latest
+        - os: ubuntu-latest
+          version: cached
+        - os: macos-latest
+          version: cached
+        - os: windows-2019
+          version: cached
+        - os: ubuntu-latest
+          version: nightly-latest
+        - os: macos-latest
+          version: nightly-latest
+        - os: windows-2019
+          version: nightly-latest
+        - os: windows-2022
+          version: nightly-latest
     name: 'Packaging: Action input'
     timeout-minutes: 45
     runs-on: ${{ matrix.os }}
@@ -44,7 +62,7 @@ jobs:
       with:
         config-file: .github/codeql/codeql-config-packaging2.yml
         languages: javascript
-        packs: dsp-testing/codeql-pack1@0.1.0, dsp-testing/codeql-pack2
+        packs: dsp-testing/codeql-pack1@1.0.0, dsp-testing/codeql-pack2, dsp-testing/codeql-pack3:other-query.ql
         tools: ${{ steps.prepare-test.outputs.tools-url }}
     - name: Build code
       shell: bash
@@ -58,11 +76,11 @@ jobs:
       shell: bash
       run: |
         cd "$RUNNER_TEMP/results"
-        # We should have 3 hits from these rules
+        # We should have 4 hits from these rules
-        EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/two-block"
+        EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/other-query-block javascript/example/two-block"
 
         # use tr to replace newlines with spaces and xargs to trim leading and trailing whitespace
-        RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | xargs)"
+        RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | tr "\r" " " | xargs)"
         echo "Found matching rules '$RULES'"
         if [ "$RULES" != "$EXPECTED_RULES" ]; then
           echo "Did not match expected rules '$EXPECTED_RULES'."

.github/workflows/__split-workflow.yml

@@ -26,9 +26,17 @@ jobs:
       matrix:
         include:
         - os: ubuntu-latest
-          version: nightly-20210831
+          version: latest
         - os: macos-latest
-          version: nightly-20210831
+          version: latest
+        - os: ubuntu-latest
+          version: cached
+        - os: macos-latest
+          version: cached
+        - os: ubuntu-latest
+          version: nightly-latest
+        - os: macos-latest
+          version: nightly-latest
     name: Split workflow
     timeout-minutes: 45
     runs-on: ${{ matrix.os }}
@@ -43,7 +51,7 @@ jobs:
     - uses: ./../action/init
       with:
         config-file: .github/codeql/codeql-config-packaging3.yml
-        packs: +dsp-testing/codeql-pack1@0.1.0
+        packs: +dsp-testing/codeql-pack1@1.0.0
         languages: javascript
         tools: ${{ steps.prepare-test.outputs.tools-url }}
     - name: Build code
@@ -72,11 +80,11 @@ jobs:
       shell: bash
       run: |
         cd "$RUNNER_TEMP/results"
-        # We should have 3 hits from these rules
+        # We should have 4 hits from these rules
-        EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/two-block"
+        EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/other-query-block javascript/example/two-block"
 
         # use tr to replace newlines with spaces and xargs to trim leading and trailing whitespace
-        RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | xargs)"
+        RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | tr "\r" " " | xargs)"
         echo "Found matching rules '$RULES'"
         if [ "$RULES" != "$EXPECTED_RULES" ]; then
           echo "Did not match expected rules '$EXPECTED_RULES'."

lib/analyze.js

@@ -136,7 +136,7 @@ async function runQueries(sarifFolder, memoryFlag, addSnippetsFlag, threadsFlag,
             if (hasPackWithCustomQueries) {
                 logger.info("Performing analysis with custom CodeQL Packs.");
                 logger.startGroup(`Downloading custom packs for ${language}`);
-                const results = await codeql.packDownload(packsWithVersion);
+                const results = await codeql.packDownload(removePackPath(packsWithVersion));
                 logger.info(`Downloaded packs: ${results.packs
                     .map((r) => `${r.name}@${r.version || "latest"}`)
                     .join(", ")}`);
@@ -159,7 +159,7 @@ async function runQueries(sarifFolder, memoryFlag, addSnippetsFlag, threadsFlag,
                 }
             }
             if (packsWithVersion.length > 0) {
-                querySuitePaths.push(await runQueryGroup(language, "packs", createPackSuiteContents(packsWithVersion), undefined));
+                querySuitePaths.push(...(await runQueryPacks(language, "packs", packsWithVersion, undefined)));
                 ranCustom = true;
             }
             if (ranCustom) {
@@ -217,21 +217,23 @@ async function runQueries(sarifFolder, memoryFlag, addSnippetsFlag, threadsFlag,
         logger.debug(`BQRS results produced for ${language} (queries: ${type})"`);
         return querySuitePath;
     }
+    async function runQueryPacks(language, type, packs, searchPath) {
+        const databasePath = util.getCodeQLDatabasePath(config, language);
+        // Run the queries individually instead of all at once to avoid command
+        // line length restrictions, particularly on windows.
+        for (const pack of packs) {
+            logger.debug(`Running query pack for ${language}-${type}: ${pack}`);
+            const codeql = await (0, codeql_1.getCodeQL)(config.codeQLCmd);
+            await codeql.databaseRunQueries(databasePath, searchPath, pack, memoryFlag, threadsFlag);
+            logger.debug(`BQRS results produced for ${language} (queries: ${type})"`);
+        }
+        return packs;
+    }
 }
 exports.runQueries = runQueries;
 function createQuerySuiteContents(queries) {
     return queries.map((q) => `- query: ${q}`).join("\n");
 }
-function createPackSuiteContents(packsWithVersion) {
-    return packsWithVersion.map(packWithVersionToQuerySuiteEntry).join("\n");
-}
-function packWithVersionToQuerySuiteEntry(pack) {
-    let text = `- qlpack: ${pack.packName}`;
-    if (pack.version) {
-        text += `\n  version: ${pack.version}`;
-    }
-    return text;
-}
 async function runFinalize(outputDir, threadsFlag, memoryFlag, config, logger) {
     const codeql = await (0, codeql_1.getCodeQL)(config.codeQLCmd);
     if (await util.codeQlVersionAbove(codeql, codeql_1.CODEQL_VERSION_NEW_TRACING)) {
@@ -292,6 +294,15 @@ async function injectLinesOfCode(sarifFile, language, locPromise) {
         fs.writeFileSync(sarifFile, JSON.stringify(sarif));
     }
 }
+/**
+ * `codeql pack download` command does not support downloading pack specifiers with paths
+ * in them. This removes the path from the pack specifier.
+ * @param packsWithVersion array of pack specifiers, some of which may have paths in them
+ * @returns array of pack specifiers without paths
+ */
+function removePackPath(packsWithVersion) {
+    return packsWithVersion.map((pack) => pack.split(":")[0]);
+}
 function printLinesOfCodeSummary(logger, language, lineCounts) {
     if (language in lineCounts) {
         logger.info(`Counted a baseline of ${lineCounts[language]} lines of code for ${language}.`);

lib/analyze.test.js

@@ -26,7 +26,6 @@ const fs = __importStar(require("fs"));
 const path = __importStar(require("path"));
 const ava_1 = __importDefault(require("ava"));
 const yaml = __importStar(require("js-yaml"));
-const semver_1 = require("semver");
 const sinon = __importStar(require("sinon"));
 const analyze_1 = require("./analyze");
 const codeql_1 = require("./codeql");
@@ -53,18 +52,8 @@ const util = __importStar(require("./util"));
         const addSnippetsFlag = "";
         const threadsFlag = "";
         const packs = {
-            [languages_1.Language.cpp]: [
+            [languages_1.Language.cpp]: ["a/b@1.0.0"],
-                {
+            [languages_1.Language.java]: ["c/d@2.0.0"],
-                    packName: "a/b",
-                    version: (0, semver_1.clean)("1.0.0"),
-                },
-            ],
-            [languages_1.Language.java]: [
-                {
-                    packName: "c/d",
-                    version: (0, semver_1.clean)("2.0.0"),
-                },
-            ],
         };
         for (const language of Object.values(languages_1.Language)) {
             (0, codeql_1.setCodeQL)({
@@ -209,32 +198,10 @@ const util = __importStar(require("./util"));
                 query: "bar.ql",
             },
         ];
-        const qlsPackContentCpp = [
-            {
-                qlpack: "a/b",
-                version: "1.0.0",
-            },
-        ];
-        const qlsPackContentJava = [
-            {
-                qlpack: "c/d",
-                version: "2.0.0",
-            },
-        ];
         for (const lang of Object.values(languages_1.Language)) {
             t.deepEqual(readContents(`${lang}-queries-builtin.qls`), qlsContent);
             t.deepEqual(readContents(`${lang}-queries-custom-0.qls`), qlsContent);
             t.deepEqual(readContents(`${lang}-queries-custom-1.qls`), qlsContent2);
-            const packSuiteName = `${lang}-queries-packs.qls`;
-            if (lang === languages_1.Language.cpp) {
-                t.deepEqual(readContents(packSuiteName), qlsPackContentCpp);
-            }
-            else if (lang === languages_1.Language.java) {
-                t.deepEqual(readContents(packSuiteName), qlsPackContentJava);
-            }
-            else {
-                t.false(fs.existsSync(path.join(tmpDir, "codeql_databases", packSuiteName)));
-            }
         }
         function readContents(name) {
             const x = fs.readFileSync(path.join(tmpDir, "codeql_databases", name), "utf8");

lib/codeql.js

@@ -642,7 +642,7 @@ async function getCodeQLForCmd(cmd, checkVersion) {
                 "download",
                 "--format=json",
                 ...getExtraOptionsFromEnv(["pack", "download"]),
-                ...packs.map(packWithVersionToString),
+                ...packs,
             ];
             const output = await runTool(cmd, codeqlArgs);
             try {
@@ -698,9 +698,6 @@ async function getCodeQLForCmd(cmd, checkVersion) {
     }
     return codeql;
 }
-function packWithVersionToString(pack) {
-    return pack.version ? `${pack.packName}@${pack.version}` : pack.packName;
-}
 /**
  * Gets the options for `path` of `options` as an array of extra option strings.
  */

lib/config-utils.js

@@ -19,7 +19,7 @@ var __importStar = (this && this.__importStar) || function (mod) {
     return result;
 };
 Object.defineProperty(exports, "__esModule", { value: true });
-exports.getConfig = exports.getPathToParsedConfigFile = exports.initConfig = exports.parsePacks = exports.parsePacksFromConfig = exports.getDefaultConfig = exports.getUnknownLanguagesError = exports.getNoLanguagesError = exports.getConfigFileDirectoryGivenMessage = exports.getConfigFileFormatInvalidMessage = exports.getConfigFileRepoFormatInvalidMessage = exports.getConfigFileDoesNotExistErrorMessage = exports.getConfigFileOutsideWorkspaceErrorMessage = exports.getLocalPathDoesNotExist = exports.getLocalPathOutsideOfRepository = exports.getPacksStrInvalid = exports.getPacksInvalid = exports.getPacksInvalidSplit = exports.getPacksRequireLanguage = exports.getPathsInvalid = exports.getPathsIgnoreInvalid = exports.getQueryUsesInvalid = exports.getQueriesInvalid = exports.getDisableDefaultQueriesInvalid = exports.getNameInvalid = exports.validateAndSanitisePath = void 0;
+exports.getConfig = exports.getPathToParsedConfigFile = exports.initConfig = exports.parsePacks = exports.validatePacksSpecification = exports.parsePacksFromConfig = exports.getDefaultConfig = exports.getUnknownLanguagesError = exports.getNoLanguagesError = exports.getConfigFileDirectoryGivenMessage = exports.getConfigFileFormatInvalidMessage = exports.getConfigFileRepoFormatInvalidMessage = exports.getConfigFileDoesNotExistErrorMessage = exports.getConfigFileOutsideWorkspaceErrorMessage = exports.getLocalPathDoesNotExist = exports.getLocalPathOutsideOfRepository = exports.getPacksStrInvalid = exports.getPacksInvalid = exports.getPacksInvalidSplit = exports.getPacksRequireLanguage = exports.getPathsInvalid = exports.getPathsIgnoreInvalid = exports.getQueryUsesInvalid = exports.getQueriesInvalid = exports.getDisableDefaultQueriesInvalid = exports.getNameInvalid = exports.validateAndSanitisePath = void 0;
 const fs = __importStar(require("fs"));
 const path = __importStar(require("path"));
 const yaml = __importStar(require("js-yaml"));
@@ -135,7 +135,7 @@ async function addBuiltinSuiteQueries(languages, codeQL, resultMap, packs, suite
     process.platform !== "win32" &&
         languages.includes("javascript") &&
         (found === "security-extended" || found === "security-and-quality") &&
-        !((_a = packs.javascript) === null || _a === void 0 ? void 0 : _a.some((pack) => pack.packName === util_1.ML_POWERED_JS_QUERIES_PACK_NAME)) &&
+        !((_a = packs.javascript) === null || _a === void 0 ? void 0 : _a.some(isMlPoweredJsQueriesPack)) &&
         (await featureFlags.getValue(feature_flags_1.FeatureFlag.MlPoweredQueriesEnabled)) &&
         (await (0, util_1.codeQlVersionAbove)(codeQL, codeql_1.CODEQL_VERSION_ML_POWERED_QUERIES))) {
         if (!packs.javascript) {
@@ -148,6 +148,11 @@ async function addBuiltinSuiteQueries(languages, codeQL, resultMap, packs, suite
     await runResolveQueries(codeQL, resultMap, suites, undefined);
     return injectedMlQueries;
 }
+function isMlPoweredJsQueriesPack(pack) {
+    return (pack === util_1.ML_POWERED_JS_QUERIES_PACK_NAME ||
+        pack.startsWith(`${util_1.ML_POWERED_JS_QUERIES_PACK_NAME}@`) ||
+        pack.startsWith(`${util_1.ML_POWERED_JS_QUERIES_PACK_NAME}:`));
+}
 /**
  * Retrieve the set of queries at localQueryPath and add them to resultMap.
  */
@@ -634,7 +639,7 @@ function parsePacksFromConfig(packsByLanguage, languages, configFile) {
         }
         packs[lang] = [];
         for (const packStr of packsArr) {
-            packs[lang].push(toPackWithVersion(packStr, configFile));
+            packs[lang].push(validatePacksSpecification(packStr, configFile));
         }
     }
     return packs;
@@ -659,32 +664,74 @@ function parsePacksFromInput(packsInput, languages) {
     }
     return {
         [languages[0]]: packsInput.split(",").reduce((packs, pack) => {
-            packs.push(toPackWithVersion(pack, ""));
+            packs.push(validatePacksSpecification(pack, ""));
             return packs;
         }, []),
     };
 }
-function toPackWithVersion(packStr, configFile) {
+/**
+ * Validates that this package specification is syntactically correct.
+ * It may not point to any real package, but after this function returns
+ * without throwing, we are guaranteed that the package specification
+ * is roughly correct.
+ *
+ * The CLI itself will do a more thorough validation of the package
+ * specification.
+ *
+ * A package specification looks like this:
+ *
+ * `scope/name@version:path`
+ *
+ * Version and path are optional.
+ *
+ * @param packStr the package specification to verify.
+ * @param configFile Config file to use for error reporting
+ */
+function validatePacksSpecification(packStr, configFile) {
     if (typeof packStr !== "string") {
         throw new Error(getPacksStrInvalid(packStr, configFile));
     }
-    const nameWithVersion = packStr.trim().split("@");
+    packStr = packStr.trim();
-    let version;
+    const atIndex = packStr.indexOf("@");
-    if (nameWithVersion.length > 2 ||
+    const colonIndex = packStr.indexOf(":", atIndex);
-        !PACK_IDENTIFIER_PATTERN.test(nameWithVersion[0])) {
+    const packStart = 0;
+    const versionStart = atIndex + 1 || undefined;
+    const pathStart = colonIndex + 1 || undefined;
+    const packEnd = Math.min(atIndex > 0 ? atIndex : Infinity, colonIndex > 0 ? colonIndex : Infinity, packStr.length);
+    const versionEnd = versionStart
+        ? Math.min(colonIndex > 0 ? colonIndex : Infinity, packStr.length)
+        : undefined;
+    const pathEnd = pathStart ? packStr.length : undefined;
+    const packName = packStr.slice(packStart, packEnd).trim();
+    const version = versionStart
+        ? packStr.slice(versionStart, versionEnd).trim()
+        : undefined;
+    const packPath = pathStart
+        ? packStr.slice(pathStart, pathEnd).trim()
+        : undefined;
+    if (!PACK_IDENTIFIER_PATTERN.test(packName)) {
         throw new Error(getPacksStrInvalid(packStr, configFile));
     }
-    else if (nameWithVersion.length === 2) {
+    if (version) {
-        version = semver.clean(nameWithVersion[1]) || undefined;
+        try {
-        if (!version) {
+            new semver.Range(version);
+        }
+        catch (e) {
+            // The range string is invalid. OK to ignore the caught error
             throw new Error(getPacksStrInvalid(packStr, configFile));
         }
     }
-    return {
+    if (packPath &&
-        packName: nameWithVersion[0].trim(),
+        (path.isAbsolute(packPath) || path.normalize(packPath) !== packPath)) {
-        version,
+        throw new Error(getPacksStrInvalid(packStr, configFile));
-    };
+    }
+    if (!packPath && pathStart) {
+        // 0 length path
+        throw new Error(getPacksStrInvalid(packStr, configFile));
+    }
+    return (packName + (version ? `@${version}` : "") + (packPath ? `:${packPath}` : ""));
 }
+exports.validatePacksSpecification = validatePacksSpecification;
 // exported for testing
 function parsePacks(rawPacksFromConfig, rawPacksInput, languages, configFile) {
     const packsFromInput = parsePacksFromInput(rawPacksInput, languages);

lib/config-utils.test.js

@@ -26,7 +26,6 @@ const fs = __importStar(require("fs"));
 const path = __importStar(require("path"));
 const github = __importStar(require("@actions/github"));
 const ava_1 = __importDefault(require("ava"));
-const semver_1 = require("semver");
 const sinon = __importStar(require("sinon"));
 const api = __importStar(require("./api-client"));
 const codeql_1 = require("./codeql");
@@ -601,12 +600,7 @@ function queriesToResolvedQueryForm(queries) {
         const languages = "javascript";
         const { packs } = await configUtils.initConfig(languages, undefined, undefined, configFile, undefined, false, "", "", { owner: "github", repo: "example " }, tmpDir, tmpDir, codeQL, tmpDir, gitHubVersion, sampleApiDetails, (0, feature_flags_1.createFeatureFlags)([]), (0, logging_1.getRunnerLogger)(true));
         t.deepEqual(packs, {
-            [languages_1.Language.javascript]: [
+            [languages_1.Language.javascript]: ["a/b@1.2.3"],
-                {
-                    packName: "a/b",
-                    version: (0, semver_1.clean)("1.2.3"),
-                },
-            ],
         });
     });
 });
@@ -640,18 +634,8 @@ function queriesToResolvedQueryForm(queries) {
         const languages = "javascript,python,cpp";
         const { packs, queries } = await configUtils.initConfig(languages, undefined, undefined, configFile, undefined, false, "", "", { owner: "github", repo: "example" }, tmpDir, tmpDir, codeQL, tmpDir, gitHubVersion, sampleApiDetails, (0, feature_flags_1.createFeatureFlags)([]), (0, logging_1.getRunnerLogger)(true));
         t.deepEqual(packs, {
-            [languages_1.Language.javascript]: [
+            [languages_1.Language.javascript]: ["a/b@1.2.3"],
-                {
+            [languages_1.Language.python]: ["c/d@1.2.3"],
-                    packName: "a/b",
-                    version: (0, semver_1.clean)("1.2.3"),
-                },
-            ],
-            [languages_1.Language.python]: [
-                {
-                    packName: "c/d",
-                    version: (0, semver_1.clean)("1.2.3"),
-                },
-            ],
         });
         t.deepEqual(queries, {
             cpp: {
@@ -786,28 +770,47 @@ const invalidPackNameMacro = ava_1.default.macro({
 });
 (0, ava_1.default)("no packs", parsePacksMacro, {}, [], {});
 (0, ava_1.default)("two packs", parsePacksMacro, ["a/b", "c/d@1.2.3"], [languages_1.Language.cpp], {
-    [languages_1.Language.cpp]: [
+    [languages_1.Language.cpp]: ["a/b", "c/d@1.2.3"],
-        { packName: "a/b", version: undefined },
-        { packName: "c/d", version: (0, semver_1.clean)("1.2.3") },
-    ],
 });
 (0, ava_1.default)("two packs with spaces", parsePacksMacro, [" a/b ", " c/d@1.2.3 "], [languages_1.Language.cpp], {
-    [languages_1.Language.cpp]: [
+    [languages_1.Language.cpp]: ["a/b", "c/d@1.2.3"],
-        { packName: "a/b", version: undefined },
-        { packName: "c/d", version: (0, semver_1.clean)("1.2.3") },
-    ],
 });
 (0, ava_1.default)("two packs with language", parsePacksMacro, {
     [languages_1.Language.cpp]: ["a/b", "c/d@1.2.3"],
     [languages_1.Language.java]: ["d/e", "f/g@1.2.3"],
 }, [languages_1.Language.cpp, languages_1.Language.java, languages_1.Language.csharp], {
+    [languages_1.Language.cpp]: ["a/b", "c/d@1.2.3"],
+    [languages_1.Language.java]: ["d/e", "f/g@1.2.3"],
+});
+(0, ava_1.default)("packs with other valid names", parsePacksMacro, [
+    // ranges are ok
+    "c/d@1.0",
+    "c/d@~1.0.0",
+    "c/d@~1.0.0:a/b",
+    "c/d@~1.0.0+abc:a/b",
+    "c/d@~1.0.0-abc:a/b",
+    "c/d:a/b",
+    // whitespace is removed
+    " c/d      @     ~1.0.0    :    b.qls   ",
+    // and it is retained within a path
+    " c/d      @     ~1.0.0    :    b/a path with/spaces.qls   ",
+    // this is valid. the path is '@'. It will probably fail when passed to the CLI
+    "c/d@1.2.3:@",
+    // this is valid, too. It will fail if it doesn't match a path
+    // (globbing is not done)
+    "c/d@1.2.3:+*)_(",
+], [languages_1.Language.cpp], {
     [languages_1.Language.cpp]: [
-        { packName: "a/b", version: undefined },
+        "c/d@1.0",
-        { packName: "c/d", version: (0, semver_1.clean)("1.2.3") },
+        "c/d@~1.0.0",
-    ],
+        "c/d@~1.0.0:a/b",
-    [languages_1.Language.java]: [
+        "c/d@~1.0.0+abc:a/b",
-        { packName: "d/e", version: undefined },
+        "c/d@~1.0.0-abc:a/b",
-        { packName: "f/g", version: (0, semver_1.clean)("1.2.3") },
+        "c/d:a/b",
+        "c/d@~1.0.0:b.qls",
+        "c/d@~1.0.0:b/a path with/spaces.qls",
+        "c/d@1.2.3:@",
+        "c/d@1.2.3:+*)_(",
     ],
 });
 (0, ava_1.default)("no language", parsePacksErrorMacro, ["a/b@1.2.3"], [languages_1.Language.java, languages_1.Language.python], /The configuration file "\/a\/b" is invalid: property "packs" must split packages by language/);
@@ -817,7 +820,14 @@ const invalidPackNameMacro = ava_1.default.macro({
 (0, ava_1.default)(invalidPackNameMacro, "c-/d");
 (0, ava_1.default)(invalidPackNameMacro, "-c/d");
 (0, ava_1.default)(invalidPackNameMacro, "c/d_d");
-(0, ava_1.default)(invalidPackNameMacro, "c/d@x");
+(0, ava_1.default)(invalidPackNameMacro, "c/d@@");
+(0, ava_1.default)(invalidPackNameMacro, "c/d@1.0.0:");
+(0, ava_1.default)(invalidPackNameMacro, "c/d:");
+(0, ava_1.default)(invalidPackNameMacro, "c/d:/a");
+(0, ava_1.default)(invalidPackNameMacro, "@1.0.0:a");
+(0, ava_1.default)(invalidPackNameMacro, "c/d@../a");
+(0, ava_1.default)(invalidPackNameMacro, "c/d@b/../a");
+(0, ava_1.default)(invalidPackNameMacro, "c/d:z@1");
 /**
  * Test macro for testing the packs block and the packs input
  */
@@ -834,39 +844,22 @@ function parseInputAndConfigErrorMacro(t, packsFromConfig, packsFromInput, langu
 }
 parseInputAndConfigErrorMacro.title = (providedTitle) => `Parse Packs input and config Error: ${providedTitle}`;
 (0, ava_1.default)("input only", parseInputAndConfigMacro, {}, " c/d ", [languages_1.Language.cpp], {
-    [languages_1.Language.cpp]: [{ packName: "c/d", version: undefined }],
+    [languages_1.Language.cpp]: ["c/d"],
 });
 (0, ava_1.default)("input only with multiple", parseInputAndConfigMacro, {}, "a/b , c/d@1.2.3", [languages_1.Language.cpp], {
-    [languages_1.Language.cpp]: [
+    [languages_1.Language.cpp]: ["a/b", "c/d@1.2.3"],
-        { packName: "a/b", version: undefined },
-        { packName: "c/d", version: "1.2.3" },
-    ],
 });
 (0, ava_1.default)("input only with +", parseInputAndConfigMacro, {}, "  +  a/b , c/d@1.2.3 ", [languages_1.Language.cpp], {
-    [languages_1.Language.cpp]: [
+    [languages_1.Language.cpp]: ["a/b", "c/d@1.2.3"],
-        { packName: "a/b", version: undefined },
-        { packName: "c/d", version: "1.2.3" },
-    ],
 });
 (0, ava_1.default)("config only", parseInputAndConfigMacro, ["a/b", "c/d"], "  ", [languages_1.Language.cpp], {
-    [languages_1.Language.cpp]: [
+    [languages_1.Language.cpp]: ["a/b", "c/d"],
-        { packName: "a/b", version: undefined },
-        { packName: "c/d", version: undefined },
-    ],
 });
 (0, ava_1.default)("input overrides", parseInputAndConfigMacro, ["a/b", "c/d"], " e/f, g/h@1.2.3 ", [languages_1.Language.cpp], {
-    [languages_1.Language.cpp]: [
+    [languages_1.Language.cpp]: ["e/f", "g/h@1.2.3"],
-        { packName: "e/f", version: undefined },
-        { packName: "g/h", version: "1.2.3" },
-    ],
 });
 (0, ava_1.default)("input and config", parseInputAndConfigMacro, ["a/b", "c/d"], " +e/f, g/h@1.2.3 ", [languages_1.Language.cpp], {
-    [languages_1.Language.cpp]: [
+    [languages_1.Language.cpp]: ["e/f", "g/h@1.2.3", "a/b", "c/d"],
-        { packName: "e/f", version: undefined },
-        { packName: "g/h", version: "1.2.3" },
-        { packName: "a/b", version: undefined },
-        { packName: "c/d", version: undefined },
-    ],
 });
 (0, ava_1.default)("input with no language", parseInputAndConfigErrorMacro, {}, "c/d", [], /No languages specified/);
 (0, ava_1.default)("input with two languages", parseInputAndConfigErrorMacro, {}, "c/d", [languages_1.Language.cpp, languages_1.Language.csharp], /multi-language analysis/);
@@ -895,10 +888,7 @@ const mlPoweredQueriesMacro = ava_1.default.macro({
             if (expectedVersionString !== undefined) {
                 t.deepEqual(packs, {
                     [languages_1.Language.javascript]: [
-                        {
+                        `codeql/javascript-experimental-atm-queries@${expectedVersionString}`,
-                            packName: "codeql/javascript-experimental-atm-queries",
-                            version: expectedVersionString,
-                        },
                     ],
                 });
             }

lib/util.js

@@ -553,9 +553,9 @@ exports.ML_POWERED_JS_QUERIES_PACK_NAME = "codeql/javascript-experimental-atm-qu
  */
 async function getMlPoweredJsQueriesPack(codeQL) {
     if (await codeQlVersionAbove(codeQL, "2.8.4")) {
-        return { packName: exports.ML_POWERED_JS_QUERIES_PACK_NAME, version: "~0.2.0" };
+        return `${exports.ML_POWERED_JS_QUERIES_PACK_NAME}@~0.2.0`;
     }
-    return { packName: exports.ML_POWERED_JS_QUERIES_PACK_NAME, version: "~0.1.0" };
+    return `${exports.ML_POWERED_JS_QUERIES_PACK_NAME}@~0.1.0`;
 }
 exports.getMlPoweredJsQueriesPack = getMlPoweredJsQueriesPack;
 /**
@@ -580,7 +580,10 @@ exports.getMlPoweredJsQueriesPack = getMlPoweredJsQueriesPack;
  * explanation as to why this is.
  */
 function getMlPoweredJsQueriesStatus(config) {
-    const mlPoweredJsQueryPacks = (config.packs.javascript || []).filter((pack) => pack.packName === exports.ML_POWERED_JS_QUERIES_PACK_NAME);
+    const mlPoweredJsQueryPacks = (config.packs.javascript || [])
+        .map((pack) => pack.split("@"))
+        .filter((packNameVersion) => packNameVersion[0] === "codeql/javascript-experimental-atm-queries" &&
+        packNameVersion.length <= 2);
     switch (mlPoweredJsQueryPacks.length) {
         case 1:
             // We should always specify an explicit version string in `getMlPoweredJsQueriesPack`,
@@ -588,7 +591,7 @@ function getMlPoweredJsQueriesStatus(config) {
             // with each version of the CodeQL Action. Therefore in practice we should only hit the
             // `latest` case here when customers have explicitly added the ML-powered query pack to their
             // CodeQL config.
-            return mlPoweredJsQueryPacks[0].version || "latest";
+            return mlPoweredJsQueryPacks[0][1] || "latest";
         case 0:
             return "false";
         default:

lib/util.test.js

@@ -209,40 +209,28 @@ const ML_POWERED_JS_STATUS_TESTS = [
     // If no packs are loaded, status is false.
     [[], "false"],
     // If another pack is loaded but not the ML-powered query pack, status is false.
-    [[{ packName: "someOtherPack" }], "false"],
+    [["someOtherPack"], "false"],
     // If the ML-powered query pack is loaded with a specific version, status is that version.
-    [
+    [[`${util.ML_POWERED_JS_QUERIES_PACK_NAME}@~0.1.0`], "~0.1.0"],
-        [{ packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "~0.1.0" }],
-        "~0.1.0",
-    ],
     // If the ML-powered query pack is loaded with a specific version and another pack is loaded, the
     // status is the version of the ML-powered query pack.
     [
-        [
+        ["someOtherPack", `${util.ML_POWERED_JS_QUERIES_PACK_NAME}@~0.1.0`],
-            { packName: "someOtherPack" },
-            { packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "~0.1.0" },
-        ],
         "~0.1.0",
     ],
     // If the ML-powered query pack is loaded without a version, the status is "latest".
-    [[{ packName: util.ML_POWERED_JS_QUERIES_PACK_NAME }], "latest"],
+    [[util.ML_POWERED_JS_QUERIES_PACK_NAME], "latest"],
     // If the ML-powered query pack is loaded with two different versions, the status is "other".
     [
         [
-            { packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "0.0.1" },
+            `${util.ML_POWERED_JS_QUERIES_PACK_NAME}@~0.0.1`,
-            { packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "0.0.2" },
+            `${util.ML_POWERED_JS_QUERIES_PACK_NAME}@~0.0.2`,
         ],
         "other",
     ],
     // If the ML-powered query pack is loaded with no specific version, and another pack is loaded,
     // the status is "latest".
-    [
+    [["someOtherPack", util.ML_POWERED_JS_QUERIES_PACK_NAME], "latest"],
-        [
-            { packName: "someOtherPack" },
-            { packName: util.ML_POWERED_JS_QUERIES_PACK_NAME },
-        ],
-        "latest",
-    ],
 ];
 for (const [packs, expectedStatus] of ML_POWERED_JS_STATUS_TESTS) {
     const packDescriptions = `[${packs

pr-checks/checks/packaging-config-inputs-js.yml

@@ -1,12 +1,11 @@
 name: "Packaging: Config and input"
 description: "Checks that specifying packages using a combination of a config file and input to the Action works"
-versions: ["nightly-20210831"] # This CLI version is known to work with package used in this test
+versions: ["latest", "cached", "nightly-latest"] # This feature is not compatible with old CLIs
-os: ["ubuntu-latest", "macos-latest"]
 steps:
   - uses: ./../action/init
     with:
       config-file: ".github/codeql/codeql-config-packaging3.yml"
-      packs: +dsp-testing/codeql-pack1@0.1.0
+      packs: +dsp-testing/codeql-pack1@1.0.0
       languages: javascript
       tools: ${{ steps.prepare-test.outputs.tools-url }}
   - name: Build code
@@ -21,11 +20,11 @@ steps:
     shell: bash
     run: |
       cd "$RUNNER_TEMP/results"
-      # We should have 3 hits from these rules
+      # We should have 4 hits from these rules
-      EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/two-block"
+      EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/other-query-block javascript/example/two-block"
 
       # use tr to replace newlines with spaces and xargs to trim leading and trailing whitespace
-      RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | xargs)"
+      RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | tr "\r" " " | xargs)"
       echo "Found matching rules '$RULES'"
       if [ "$RULES" != "$EXPECTED_RULES" ]; then
         echo "Did not match expected rules '$EXPECTED_RULES'."

pr-checks/checks/packaging-config-js.yml

@@ -1,7 +1,6 @@
 name: "Packaging: Config file"
 description: "Checks that specifying packages using only a config file works"
-versions: ["nightly-20210831"] # This CLI version is known to work with package used in this test
+versions: ["latest", "cached", "nightly-latest"] # This feature is not compatible with old CLIs
-os: ["ubuntu-latest", "macos-latest"]
 steps:
   - uses: ./../action/init
     with:
@@ -20,11 +19,11 @@ steps:
     shell: bash
     run: |
       cd "$RUNNER_TEMP/results"
-      # We should have 3 hits from these rules
+      # We should have 4 hits from these rules
-      EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/two-block"
+      EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/other-query-block javascript/example/two-block"
 
       # use tr to replace newlines with spaces and xargs to trim leading and trailing whitespace
-      RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | xargs)"
+      RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | tr "\r" " " | xargs)"
       echo "Found matching rules '$RULES'"
       if [ "$RULES" != "$EXPECTED_RULES" ]; then
         echo "Did not match expected rules '$EXPECTED_RULES'."

pr-checks/checks/packaging-inputs-js.yml

@@ -1,13 +1,12 @@
 name: "Packaging: Action input"
 description: "Checks that specifying packages using the input to the Action works"
-versions: ["nightly-20210831"] # This CLI version is known to work with package used in this test
+versions: ["latest", "cached", "nightly-latest"] # This feature is not compatible with old CLIs
-os: ["ubuntu-latest", "macos-latest"]
 steps:
   - uses: ./../action/init
     with:
       config-file: ".github/codeql/codeql-config-packaging2.yml"
       languages: javascript
-      packs: dsp-testing/codeql-pack1@0.1.0, dsp-testing/codeql-pack2
+      packs: dsp-testing/codeql-pack1@1.0.0, dsp-testing/codeql-pack2, dsp-testing/codeql-pack3:other-query.ql
       tools: ${{ steps.prepare-test.outputs.tools-url }}
   - name: Build code
     shell: bash
@@ -21,11 +20,11 @@ steps:
     shell: bash
     run: |
       cd "$RUNNER_TEMP/results"
-      # We should have 3 hits from these rules
+      # We should have 4 hits from these rules
-      EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/two-block"
+      EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/other-query-block javascript/example/two-block"
 
       # use tr to replace newlines with spaces and xargs to trim leading and trailing whitespace
-      RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | xargs)"
+      RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | tr "\r" " " | xargs)"
       echo "Found matching rules '$RULES'"
       if [ "$RULES" != "$EXPECTED_RULES" ]; then
         echo "Did not match expected rules '$EXPECTED_RULES'."

pr-checks/checks/split-workflow.yml

@@ -1,12 +1,12 @@
 name: "Split workflow"
 description: "Tests a split-up workflow in which we first build a database and later analyze it"
-versions: ["nightly-20210831"] # This CLI version is known to work with package used in this test
 os: ["ubuntu-latest", "macos-latest"]
+versions: ["latest", "cached", "nightly-latest"] # This feature is not compatible with old CLIs
 steps:
   - uses: ./../action/init
     with:
       config-file: ".github/codeql/codeql-config-packaging3.yml"
-      packs: +dsp-testing/codeql-pack1@0.1.0
+      packs: +dsp-testing/codeql-pack1@1.0.0
       languages: javascript
       tools: ${{ steps.prepare-test.outputs.tools-url }}
   - name: Build code
@@ -35,11 +35,11 @@ steps:
     shell: bash
     run: |
       cd "$RUNNER_TEMP/results"
-      # We should have 3 hits from these rules
+      # We should have 4 hits from these rules
-      EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/two-block"
+      EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/other-query-block javascript/example/two-block"
 
       # use tr to replace newlines with spaces and xargs to trim leading and trailing whitespace
-      RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | xargs)"
+      RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | tr "\r" " " | xargs)"
       echo "Found matching rules '$RULES'"
       if [ "$RULES" != "$EXPECTED_RULES" ]; then
         echo "Did not match expected rules '$EXPECTED_RULES'."

src/analyze.test.ts

@@ -3,7 +3,6 @@ import * as path from "path";
 
 import test from "ava";
 import * as yaml from "js-yaml";
-import { clean } from "semver";
 import * as sinon from "sinon";
 
 import { runQueries } from "./analyze";
@@ -35,18 +34,8 @@ test("status report fields and search path setting", async (t) => {
     const addSnippetsFlag = "";
     const threadsFlag = "";
     const packs = {
-      [Language.cpp]: [
+      [Language.cpp]: ["a/b@1.0.0"],
-        {
+      [Language.java]: ["c/d@2.0.0"],
-          packName: "a/b",
-          version: clean("1.0.0")!,
-        },
-      ],
-      [Language.java]: [
-        {
-          packName: "c/d",
-          version: clean("2.0.0")!,
-        },
-      ],
     };
 
     for (const language of Object.values(Language)) {
@@ -241,32 +230,10 @@ test("status report fields and search path setting", async (t) => {
         query: "bar.ql",
       },
     ];
-    const qlsPackContentCpp = [
-      {
-        qlpack: "a/b",
-        version: "1.0.0",
-      },
-    ];
-    const qlsPackContentJava = [
-      {
-        qlpack: "c/d",
-        version: "2.0.0",
-      },
-    ];
     for (const lang of Object.values(Language)) {
       t.deepEqual(readContents(`${lang}-queries-builtin.qls`), qlsContent);
       t.deepEqual(readContents(`${lang}-queries-custom-0.qls`), qlsContent);
       t.deepEqual(readContents(`${lang}-queries-custom-1.qls`), qlsContent2);
-      const packSuiteName = `${lang}-queries-packs.qls`;
-      if (lang === Language.cpp) {
-        t.deepEqual(readContents(packSuiteName), qlsPackContentCpp);
-      } else if (lang === Language.java) {
-        t.deepEqual(readContents(packSuiteName), qlsPackContentJava);
-      } else {
-        t.false(
-          fs.existsSync(path.join(tmpDir, "codeql_databases", packSuiteName))
-        );
-      }
     }
 
     function readContents(name: string) {

src/analyze.ts

@@ -241,7 +241,9 @@ export async function runQueries(
         logger.info("Performing analysis with custom CodeQL Packs.");
         logger.startGroup(`Downloading custom packs for ${language}`);
 
-        const results = await codeql.packDownload(packsWithVersion);
+        const results = await codeql.packDownload(
+          removePackPath(packsWithVersion)
+        );
         logger.info(
           `Downloaded packs: ${results.packs
             .map((r) => `${r.name}@${r.version || "latest"}`)
@@ -283,12 +285,12 @@ export async function runQueries(
       }
       if (packsWithVersion.length > 0) {
         querySuitePaths.push(
-          await runQueryGroup(
+          ...(await runQueryPacks(
             language,
             "packs",
-            createPackSuiteContents(packsWithVersion),
+            packsWithVersion,
             undefined
-          )
+          ))
         );
         ranCustom = true;
       }
@@ -386,27 +388,38 @@ export async function runQueries(
     logger.debug(`BQRS results produced for ${language} (queries: ${type})"`);
     return querySuitePath;
   }
+  async function runQueryPacks(
+    language: Language,
+    type: string,
+    packs: string[],
+    searchPath: string | undefined
+  ): Promise<string[]> {
+    const databasePath = util.getCodeQLDatabasePath(config, language);
+    // Run the queries individually instead of all at once to avoid command
+    // line length restrictions, particularly on windows.
+
+    for (const pack of packs) {
+      logger.debug(`Running query pack for ${language}-${type}: ${pack}`);
+
+      const codeql = await getCodeQL(config.codeQLCmd);
+      await codeql.databaseRunQueries(
+        databasePath,
+        searchPath,
+        pack,
+        memoryFlag,
+        threadsFlag
+      );
+
+      logger.debug(`BQRS results produced for ${language} (queries: ${type})"`);
+    }
+    return packs;
+  }
 }
+
 function createQuerySuiteContents(queries: string[]) {
   return queries.map((q: string) => `- query: ${q}`).join("\n");
 }
 
-function createPackSuiteContents(
-  packsWithVersion: configUtils.PackWithVersion[]
-) {
-  return packsWithVersion.map(packWithVersionToQuerySuiteEntry).join("\n");
-}
-
-function packWithVersionToQuerySuiteEntry(
-  pack: configUtils.PackWithVersion
-): string {
-  let text = `- qlpack: ${pack.packName}`;
-  if (pack.version) {
-    text += `\n  version: ${pack.version}`;
-  }
-  return text;
-}
-
 export async function runFinalize(
   outputDir: string,
   threadsFlag: string,
@@ -486,6 +499,16 @@ async function injectLinesOfCode(
   }
 }
 
+/**
+ * `codeql pack download` command does not support downloading pack specifiers with paths
+ * in them. This removes the path from the pack specifier.
+ * @param packsWithVersion array of pack specifiers, some of which may have paths in them
+ * @returns array of pack specifiers without paths
+ */
+function removePackPath(packsWithVersion: string[]) {
+  return packsWithVersion.map((pack) => pack.split(":")[0]);
+}
+
 function printLinesOfCodeSummary(
   logger: Logger,
   language: Language,

src/codeql.ts

@@ -9,7 +9,7 @@ import * as semver from "semver";
 
 import { isRunningLocalAction, getRelativeScriptPath } from "./actions-util";
 import * as api from "./api-client";
-import { Config, PackWithVersion } from "./config-utils";
+import { Config } from "./config-utils";
 import * as defaults from "./defaults.json"; // Referenced from codeql-action-sync-tool!
 import { errorMatchers } from "./error-matcher";
 import { isTracedLanguage, Language } from "./languages";
@@ -117,7 +117,7 @@ export interface CodeQL {
   /**
    * Run 'codeql pack download'.
    */
-  packDownload(packs: PackWithVersion[]): Promise<PackDownloadOutput>;
+  packDownload(packs: string[]): Promise<PackDownloadOutput>;
 
   /**
    * Run 'codeql database cleanup'.
@@ -950,13 +950,13 @@ async function getCodeQLForCmd(
      * downloaded. The check to determine what the latest version is is done
      * each time this package is requested.
      */
-    async packDownload(packs: PackWithVersion[]): Promise<PackDownloadOutput> {
+    async packDownload(packs: string[]): Promise<PackDownloadOutput> {
       const codeqlArgs = [
         "pack",
         "download",
         "--format=json",
         ...getExtraOptionsFromEnv(["pack", "download"]),
-        ...packs.map(packWithVersionToString),
+        ...packs,
       ];
 
       const output = await runTool(cmd, codeqlArgs);
@@ -1028,9 +1028,6 @@ async function getCodeQLForCmd(
   return codeql;
 }
 
-function packWithVersionToString(pack: PackWithVersion): string {
-  return pack.version ? `${pack.packName}@${pack.version}` : pack.packName;
-}
 /**
  * Gets the options for `path` of `options` as an array of extra option strings.
  */

src/config-utils.test.ts

@@ -3,7 +3,6 @@ import * as path from "path";
 
 import * as github from "@actions/github";
 import test, { ExecutionContext } from "ava";
-import { clean } from "semver";
 import * as sinon from "sinon";
 
 import * as api from "./api-client";
@@ -1132,12 +1131,7 @@ test("Config specifies packages", async (t) => {
       getRunnerLogger(true)
     );
     t.deepEqual(packs as unknown, {
-      [Language.javascript]: [
+      [Language.javascript]: ["a/b@1.2.3"],
-        {
-          packName: "a/b",
-          version: clean("1.2.3"),
-        },
-      ],
     });
   });
 });
@@ -1194,18 +1188,8 @@ test("Config specifies packages for multiple languages", async (t) => {
       getRunnerLogger(true)
     );
     t.deepEqual(packs as unknown, {
-      [Language.javascript]: [
+      [Language.javascript]: ["a/b@1.2.3"],
-        {
+      [Language.python]: ["c/d@1.2.3"],
-          packName: "a/b",
-          version: clean("1.2.3"),
-        },
-      ],
-      [Language.python]: [
-        {
-          packName: "c/d",
-          version: clean("1.2.3"),
-        },
-      ],
     });
     t.deepEqual(queries, {
       cpp: {
@@ -1437,7 +1421,7 @@ const parsePacksMacro = test.macro({
     t: ExecutionContext<unknown>,
     packsByLanguage: string[] | Record<string, string[]>,
     languages: Language[],
-    expected: Partial<Record<Language, configUtils.PackWithVersion[]>>
+    expected: Partial<Record<Language, string[]>>
   ) =>
     t.deepEqual(
       configUtils.parsePacksFromConfig(packsByLanguage, languages, "/a/b"),
@@ -1490,10 +1474,7 @@ const invalidPackNameMacro = test.macro({
 
 test("no packs", parsePacksMacro, {}, [], {});
 test("two packs", parsePacksMacro, ["a/b", "c/d@1.2.3"], [Language.cpp], {
-  [Language.cpp]: [
+  [Language.cpp]: ["a/b", "c/d@1.2.3"],
-    { packName: "a/b", version: undefined },
-    { packName: "c/d", version: clean("1.2.3") as string },
-  ],
 });
 test(
   "two packs with spaces",
@@ -1501,10 +1482,7 @@ test(
   [" a/b ", " c/d@1.2.3 "],
   [Language.cpp],
   {
-    [Language.cpp]: [
+    [Language.cpp]: ["a/b", "c/d@1.2.3"],
-      { packName: "a/b", version: undefined },
-      { packName: "c/d", version: clean("1.2.3") as string },
-    ],
   }
 );
 test(
@@ -1515,14 +1493,46 @@ test(
     [Language.java]: ["d/e", "f/g@1.2.3"],
   },
   [Language.cpp, Language.java, Language.csharp],
+  {
+    [Language.cpp]: ["a/b", "c/d@1.2.3"],
+    [Language.java]: ["d/e", "f/g@1.2.3"],
+  }
+);
+
+test(
+  "packs with other valid names",
+  parsePacksMacro,
+  [
+    // ranges are ok
+    "c/d@1.0",
+    "c/d@~1.0.0",
+    "c/d@~1.0.0:a/b",
+    "c/d@~1.0.0+abc:a/b",
+    "c/d@~1.0.0-abc:a/b",
+    "c/d:a/b",
+    // whitespace is removed
+    " c/d      @     ~1.0.0    :    b.qls   ",
+    // and it is retained within a path
+    " c/d      @     ~1.0.0    :    b/a path with/spaces.qls   ",
+    // this is valid. the path is '@'. It will probably fail when passed to the CLI
+    "c/d@1.2.3:@",
+    // this is valid, too. It will fail if it doesn't match a path
+    // (globbing is not done)
+    "c/d@1.2.3:+*)_(",
+  ],
+  [Language.cpp],
   {
     [Language.cpp]: [
-      { packName: "a/b", version: undefined },
+      "c/d@1.0",
-      { packName: "c/d", version: clean("1.2.3") as string },
+      "c/d@~1.0.0",
-    ],
+      "c/d@~1.0.0:a/b",
-    [Language.java]: [
+      "c/d@~1.0.0+abc:a/b",
-      { packName: "d/e", version: undefined },
+      "c/d@~1.0.0-abc:a/b",
-      { packName: "f/g", version: clean("1.2.3") as string },
+      "c/d:a/b",
+      "c/d@~1.0.0:b.qls",
+      "c/d@~1.0.0:b/a path with/spaces.qls",
+      "c/d@1.2.3:@",
+      "c/d@1.2.3:+*)_(",
     ],
   }
 );
@@ -1553,7 +1563,14 @@ test(invalidPackNameMacro, "c"); // all packs require at least a scope and a nam
 test(invalidPackNameMacro, "c-/d");
 test(invalidPackNameMacro, "-c/d");
 test(invalidPackNameMacro, "c/d_d");
-test(invalidPackNameMacro, "c/d@x");
+test(invalidPackNameMacro, "c/d@@");
+test(invalidPackNameMacro, "c/d@1.0.0:");
+test(invalidPackNameMacro, "c/d:");
+test(invalidPackNameMacro, "c/d:/a");
+test(invalidPackNameMacro, "@1.0.0:a");
+test(invalidPackNameMacro, "c/d@../a");
+test(invalidPackNameMacro, "c/d@b/../a");
+test(invalidPackNameMacro, "c/d:z@1");
 
 /**
  * Test macro for testing the packs block and the packs input
@@ -1598,7 +1615,7 @@ parseInputAndConfigErrorMacro.title = (providedTitle: string) =>
   `Parse Packs input and config Error: ${providedTitle}`;
 
 test("input only", parseInputAndConfigMacro, {}, " c/d ", [Language.cpp], {
-  [Language.cpp]: [{ packName: "c/d", version: undefined }],
+  [Language.cpp]: ["c/d"],
 });
 
 test(
@@ -1608,10 +1625,7 @@ test(
   "a/b , c/d@1.2.3",
   [Language.cpp],
   {
-    [Language.cpp]: [
+    [Language.cpp]: ["a/b", "c/d@1.2.3"],
-      { packName: "a/b", version: undefined },
-      { packName: "c/d", version: "1.2.3" },
-    ],
   }
 );
 
@@ -1622,10 +1636,7 @@ test(
   "  +  a/b , c/d@1.2.3 ",
   [Language.cpp],
   {
-    [Language.cpp]: [
+    [Language.cpp]: ["a/b", "c/d@1.2.3"],
-      { packName: "a/b", version: undefined },
-      { packName: "c/d", version: "1.2.3" },
-    ],
   }
 );
 
@@ -1636,10 +1647,7 @@ test(
   "  ",
   [Language.cpp],
   {
-    [Language.cpp]: [
+    [Language.cpp]: ["a/b", "c/d"],
-      { packName: "a/b", version: undefined },
-      { packName: "c/d", version: undefined },
-    ],
   }
 );
 
@@ -1650,10 +1658,7 @@ test(
   " e/f, g/h@1.2.3 ",
   [Language.cpp],
   {
-    [Language.cpp]: [
+    [Language.cpp]: ["e/f", "g/h@1.2.3"],
-      { packName: "e/f", version: undefined },
-      { packName: "g/h", version: "1.2.3" },
-    ],
   }
 );
 
@@ -1664,12 +1669,7 @@ test(
   " +e/f, g/h@1.2.3 ",
   [Language.cpp],
   {
-    [Language.cpp]: [
+    [Language.cpp]: ["e/f", "g/h@1.2.3", "a/b", "c/d"],
-      { packName: "e/f", version: undefined },
-      { packName: "g/h", version: "1.2.3" },
-      { packName: "a/b", version: undefined },
-      { packName: "c/d", version: undefined },
-    ],
   }
 );
 
@@ -1760,10 +1760,7 @@ const mlPoweredQueriesMacro = test.macro({
       if (expectedVersionString !== undefined) {
         t.deepEqual(packs as unknown, {
           [Language.javascript]: [
-            {
+            `codeql/javascript-experimental-atm-queries@${expectedVersionString}`,
-              packName: "codeql/javascript-experimental-atm-queries",
-              version: expectedVersionString,
-            },
           ],
         });
       } else {

src/config-utils.ts

@@ -154,14 +154,7 @@ export interface Config {
   injectedMlQueries: boolean;
 }
 
-export type Packs = Partial<Record<Language, PackWithVersion[]>>;
+export type Packs = Partial<Record<Language, string[]>>;
-
-export interface PackWithVersion {
-  /** qualified name of a package reference */
-  packName: string;
-  /** version of the package, or undefined, which means latest version */
-  version?: string;
-}
 
 /**
  * A list of queries from https://github.com/github/codeql that
@@ -304,9 +297,7 @@ async function addBuiltinSuiteQueries(
     process.platform !== "win32" &&
     languages.includes("javascript") &&
     (found === "security-extended" || found === "security-and-quality") &&
-    !packs.javascript?.some(
+    !packs.javascript?.some(isMlPoweredJsQueriesPack) &&
-      (pack) => pack.packName === ML_POWERED_JS_QUERIES_PACK_NAME
-    ) &&
     (await featureFlags.getValue(FeatureFlag.MlPoweredQueriesEnabled)) &&
     (await codeQlVersionAbove(codeQL, CODEQL_VERSION_ML_POWERED_QUERIES))
   ) {
@@ -322,6 +313,14 @@ async function addBuiltinSuiteQueries(
   return injectedMlQueries;
 }
 
+function isMlPoweredJsQueriesPack(pack: string) {
+  return (
+    pack === ML_POWERED_JS_QUERIES_PACK_NAME ||
+    pack.startsWith(`${ML_POWERED_JS_QUERIES_PACK_NAME}@`) ||
+    pack.startsWith(`${ML_POWERED_JS_QUERIES_PACK_NAME}:`)
+  );
+}
+
 /**
  * Retrieve the set of queries at localQueryPath and add them to resultMap.
  */
@@ -1168,7 +1167,7 @@ export function parsePacksFromConfig(
     }
     packs[lang] = [];
     for (const packStr of packsArr) {
-      packs[lang].push(toPackWithVersion(packStr, configFile));
+      packs[lang].push(validatePacksSpecification(packStr, configFile));
     }
   }
   return packs;
@@ -1202,35 +1201,89 @@ function parsePacksFromInput(
 
   return {
     [languages[0]]: packsInput.split(",").reduce((packs, pack) => {
-      packs.push(toPackWithVersion(pack, ""));
+      packs.push(validatePacksSpecification(pack, ""));
       return packs;
-    }, [] as PackWithVersion[]),
+    }, [] as string[]),
   };
 }
 
-function toPackWithVersion(packStr, configFile?: string): PackWithVersion {
+/**
+ * Validates that this package specification is syntactically correct.
+ * It may not point to any real package, but after this function returns
+ * without throwing, we are guaranteed that the package specification
+ * is roughly correct.
+ *
+ * The CLI itself will do a more thorough validation of the package
+ * specification.
+ *
+ * A package specification looks like this:
+ *
+ * `scope/name@version:path`
+ *
+ * Version and path are optional.
+ *
+ * @param packStr the package specification to verify.
+ * @param configFile Config file to use for error reporting
+ */
+export function validatePacksSpecification(
+  packStr: string,
+  configFile?: string
+): string {
   if (typeof packStr !== "string") {
     throw new Error(getPacksStrInvalid(packStr, configFile));
   }
 
-  const nameWithVersion = packStr.trim().split("@");
+  packStr = packStr.trim();
-  let version: string | undefined;
+  const atIndex = packStr.indexOf("@");
-  if (
+  const colonIndex = packStr.indexOf(":", atIndex);
-    nameWithVersion.length > 2 ||
+  const packStart = 0;
-    !PACK_IDENTIFIER_PATTERN.test(nameWithVersion[0])
+  const versionStart = atIndex + 1 || undefined;
-  ) {
+  const pathStart = colonIndex + 1 || undefined;
+  const packEnd = Math.min(
+    atIndex > 0 ? atIndex : Infinity,
+    colonIndex > 0 ? colonIndex : Infinity,
+    packStr.length
+  );
+  const versionEnd = versionStart
+    ? Math.min(colonIndex > 0 ? colonIndex : Infinity, packStr.length)
+    : undefined;
+  const pathEnd = pathStart ? packStr.length : undefined;
+
+  const packName = packStr.slice(packStart, packEnd).trim();
+  const version = versionStart
+    ? packStr.slice(versionStart, versionEnd).trim()
+    : undefined;
+  const packPath = pathStart
+    ? packStr.slice(pathStart, pathEnd).trim()
+    : undefined;
+
+  if (!PACK_IDENTIFIER_PATTERN.test(packName)) {
     throw new Error(getPacksStrInvalid(packStr, configFile));
-  } else if (nameWithVersion.length === 2) {
+  }
-    version = semver.clean(nameWithVersion[1]) || undefined;
+  if (version) {
-    if (!version) {
+    try {
+      new semver.Range(version);
+    } catch (e) {
+      // The range string is invalid. OK to ignore the caught error
       throw new Error(getPacksStrInvalid(packStr, configFile));
     }
   }
 
-  return {
+  if (
-    packName: nameWithVersion[0].trim(),
+    packPath &&
-    version,
+    (path.isAbsolute(packPath) || path.normalize(packPath) !== packPath)
-  };
+  ) {
+    throw new Error(getPacksStrInvalid(packStr, configFile));
+  }
+
+  if (!packPath && pathStart) {
+    // 0 length path
+    throw new Error(getPacksStrInvalid(packStr, configFile));
+  }
+
+  return (
+    packName + (version ? `@${version}` : "") + (packPath ? `:${packPath}` : "")
+  );
 }
 
 // exported for testing

src/util.test.ts

@@ -8,7 +8,7 @@ import test, { ExecutionContext } from "ava";
 import * as sinon from "sinon";
 
 import * as api from "./api-client";
-import { Config, PackWithVersion } from "./config-utils";
+import { Config } from "./config-utils";
 import { getRunnerLogger, Logger } from "./logging";
 import { setupTests } from "./testing-utils";
 import * as util from "./util";
@@ -294,44 +294,32 @@ async function mockStdInForAuthExpectError(
   );
 }
 
-const ML_POWERED_JS_STATUS_TESTS: Array<[PackWithVersion[], string]> = [
+const ML_POWERED_JS_STATUS_TESTS: Array<[string[], string]> = [
   // If no packs are loaded, status is false.
   [[], "false"],
   // If another pack is loaded but not the ML-powered query pack, status is false.
-  [[{ packName: "someOtherPack" }], "false"],
+  [["someOtherPack"], "false"],
   // If the ML-powered query pack is loaded with a specific version, status is that version.
-  [
+  [[`${util.ML_POWERED_JS_QUERIES_PACK_NAME}@~0.1.0`], "~0.1.0"],
-    [{ packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "~0.1.0" }],
-    "~0.1.0",
-  ],
   // If the ML-powered query pack is loaded with a specific version and another pack is loaded, the
   // status is the version of the ML-powered query pack.
   [
-    [
+    ["someOtherPack", `${util.ML_POWERED_JS_QUERIES_PACK_NAME}@~0.1.0`],
-      { packName: "someOtherPack" },
-      { packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "~0.1.0" },
-    ],
     "~0.1.0",
   ],
   // If the ML-powered query pack is loaded without a version, the status is "latest".
-  [[{ packName: util.ML_POWERED_JS_QUERIES_PACK_NAME }], "latest"],
+  [[util.ML_POWERED_JS_QUERIES_PACK_NAME], "latest"],
   // If the ML-powered query pack is loaded with two different versions, the status is "other".
   [
     [
-      { packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "0.0.1" },
+      `${util.ML_POWERED_JS_QUERIES_PACK_NAME}@~0.0.1`,
-      { packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "0.0.2" },
+      `${util.ML_POWERED_JS_QUERIES_PACK_NAME}@~0.0.2`,
     ],
     "other",
   ],
   // If the ML-powered query pack is loaded with no specific version, and another pack is loaded,
   // the status is "latest".
-  [
+  [["someOtherPack", util.ML_POWERED_JS_QUERIES_PACK_NAME], "latest"],
-    [
-      { packName: "someOtherPack" },
-      { packName: util.ML_POWERED_JS_QUERIES_PACK_NAME },
-    ],
-    "latest",
-  ],
 ];
 
 for (const [packs, expectedStatus] of ML_POWERED_JS_STATUS_TESTS) {

src/util.ts

@@ -11,7 +11,7 @@ import * as api from "./api-client";
 import { getApiClient, GitHubApiDetails } from "./api-client";
 import * as apiCompatibility from "./api-compatibility.json";
 import { CodeQL, CODEQL_VERSION_NEW_TRACING } from "./codeql";
-import { Config, PackWithVersion } from "./config-utils";
+import { Config } from "./config-utils";
 import { Language } from "./languages";
 import { Logger } from "./logging";
 
@@ -663,11 +663,11 @@ export const ML_POWERED_JS_QUERIES_PACK_NAME =
  */
 export async function getMlPoweredJsQueriesPack(
   codeQL: CodeQL
-): Promise<PackWithVersion> {
+): Promise<string> {
   if (await codeQlVersionAbove(codeQL, "2.8.4")) {
-    return { packName: ML_POWERED_JS_QUERIES_PACK_NAME, version: "~0.2.0" };
+    return `${ML_POWERED_JS_QUERIES_PACK_NAME}@~0.2.0`;
   }
-  return { packName: ML_POWERED_JS_QUERIES_PACK_NAME, version: "~0.1.0" };
+  return `${ML_POWERED_JS_QUERIES_PACK_NAME}@~0.1.0`;
 }
 
 /**
@@ -692,9 +692,13 @@ export async function getMlPoweredJsQueriesPack(
  * explanation as to why this is.
  */
 export function getMlPoweredJsQueriesStatus(config: Config): string {
-  const mlPoweredJsQueryPacks = (config.packs.javascript || []).filter(
+  const mlPoweredJsQueryPacks = (config.packs.javascript || [])
-    (pack) => pack.packName === ML_POWERED_JS_QUERIES_PACK_NAME
+    .map((pack) => pack.split("@"))
-  );
+    .filter(
+      (packNameVersion) =>
+        packNameVersion[0] === "codeql/javascript-experimental-atm-queries" &&
+        packNameVersion.length <= 2
+    );
   switch (mlPoweredJsQueryPacks.length) {
     case 1:
       // We should always specify an explicit version string in `getMlPoweredJsQueriesPack`,
@@ -702,7 +706,7 @@ export function getMlPoweredJsQueriesStatus(config: Config): string {
       // with each version of the CodeQL Action. Therefore in practice we should only hit the
       // `latest` case here when customers have explicitly added the ML-powered query pack to their
       // CodeQL config.
-      return mlPoweredJsQueryPacks[0].version || "latest";
+      return mlPoweredJsQueryPacks[0][1] || "latest";
     case 0:
       return "false";
     default:

tests/multi-language-repo/.github/codeql/codeql-config-packaging.yml

@@ -3,8 +3,10 @@ name: Pack testing in the CodeQL Action
 disable-default-queries: true
 packs:
   javascript:
-    - dsp-testing/codeql-pack1@0.1.0
+    - dsp-testing/codeql-pack1@1.0.0
-    - dsp-testing/codeql-pack2 # latest
+    - dsp-testing/codeql-pack2
+    - dsp-testing/codeql-pack3:other-query.ql
+
 paths-ignore:
   - tests
   - lib

tests/multi-language-repo/.github/codeql/codeql-config-packaging3.yml

@@ -3,7 +3,8 @@ name: Pack testing in the CodeQL Action
 disable-default-queries: true
 packs:
   javascript:
-    - dsp-testing/codeql-pack2 # latest
+    - dsp-testing/codeql-pack2
+    - dsp-testing/codeql-pack3:other-query.ql
 paths-ignore:
   - tests
   - lib