Merge branch 'main' into robertbrignull/init_status_queries

.github/workflows/codeql.yml

@@ -1,6 +1,9 @@
 name: "CodeQL action"
 
-on: [push, pull_request]
+on:
+  push:
+    branches: [main, v1]
+  pull_request:
 
 jobs:
   build:

.github/workflows/integration-testing.yml

@@ -1,6 +1,9 @@
 name: "Integration Testing"
 
-on: [push, pull_request]
+on:
+  push:
+    branches: [main, v1]
+  pull_request:
 
 jobs:
   multi-language-repo_test-autodetect-languages:
@@ -150,7 +153,7 @@ jobs:
     - uses: ./../action/analyze
       env:
         TEST_MODE: true
-  
+
   runner-analyze-javascript-ubuntu:
     runs-on: ubuntu-latest
 
@@ -176,7 +179,7 @@ jobs:
         runner/dist/codeql-runner-linux analyze --repository $GITHUB_REPOSITORY --commit $GITHUB_SHA --ref $GITHUB_REF --github-url $GITHUB_SERVER_URL --github-auth ${{ github.token }}
       env:
         TEST_MODE: true
-  
+
   runner-analyze-javascript-windows:
     runs-on: windows-latest
 
@@ -198,7 +201,7 @@ jobs:
         runner/dist/codeql-runner-win.exe analyze --repository $Env:GITHUB_REPOSITORY --commit $Env:GITHUB_SHA --ref $Env:GITHUB_REF --github-url $Env:GITHUB_SERVER_URL --github-auth ${{ github.token }}
       env:
         TEST_MODE: true
-  
+
   runner-analyze-javascript-macos:
     runs-on: macos-latest
 
@@ -220,7 +223,7 @@ jobs:
         runner/dist/codeql-runner-macos analyze --repository $GITHUB_REPOSITORY --commit $GITHUB_SHA --ref $GITHUB_REF --github-url $GITHUB_SERVER_URL --github-auth ${{ github.token }}
       env:
         TEST_MODE: true
-  
+
   runner-analyze-csharp-ubuntu:
     runs-on: ubuntu-latest
 
@@ -254,7 +257,7 @@ jobs:
         ../action/runner/dist/codeql-runner-linux analyze --repository $GITHUB_REPOSITORY --commit $GITHUB_SHA --ref $GITHUB_REF --github-url $GITHUB_SERVER_URL --github-auth ${{ github.token }}
       env:
         TEST_MODE: true
-  
+
   runner-analyze-csharp-windows:
     runs-on: windows-latest
 
@@ -325,7 +328,7 @@ jobs:
       env:
         TEST_MODE: true
 
-  
+
   runner-analyze-csharp-autobuild-ubuntu:
     runs-on: ubuntu-latest
 
@@ -358,7 +361,7 @@ jobs:
         ../action/runner/dist/codeql-runner-linux analyze --repository $GITHUB_REPOSITORY --commit $GITHUB_SHA --ref $GITHUB_REF --github-url $GITHUB_SERVER_URL --github-auth ${{ github.token }}
       env:
         TEST_MODE: true
-  
+
   runner-analyze-csharp-autobuild-windows:
     runs-on: windows-latest
 

.github/workflows/pr-checks.yml

@@ -1,6 +1,9 @@
 name: "PR checks"
 
-on: [push, pull_request]
+on:
+  push:
+    branches: [main, v1]
+  pull_request:
 
 jobs:
   lint-js:
@@ -62,7 +65,7 @@ jobs:
           exit 1
         fi
         echo "Success: node_modules are up to date"
-        
+
   npm-test:
     strategy:
       matrix:

src/analyze.test.ts

@@ -0,0 +1,76 @@
+import test from "ava";
+import * as fs from "fs";
+
+import { runQueries } from "./analyze";
+import { setCodeQL } from "./codeql";
+import { Config } from "./config-utils";
+import { Language } from "./languages";
+import { getRunnerLogger } from "./logging";
+import { setupTests } from "./testing-utils";
+import * as util from "./util";
+
+setupTests(test);
+
+// Checks that the duration fields are populated for the correct language
+// and correct case of builtin or custom.
+test("status report fields", async (t) => {
+  return await util.withTmpDir(async (tmpDir) => {
+    setCodeQL({
+      databaseAnalyze: async () => undefined,
+    });
+
+    const memoryFlag = "";
+    const addSnippetsFlag = "";
+    const threadsFlag = "";
+
+    for (const language of Object.values(Language)) {
+      const config: Config = {
+        languages: [language],
+        queries: {},
+        pathsIgnore: [],
+        paths: [],
+        originalUserInput: {},
+        tempDir: tmpDir,
+        toolCacheDir: tmpDir,
+        codeQLCmd: "",
+      };
+      fs.mkdirSync(util.getCodeQLDatabasePath(config.tempDir, language), {
+        recursive: true,
+      });
+
+      config.queries[language] = {
+        builtin: ["foo.ql"],
+        custom: [],
+      };
+      const builtinStatusReport = await runQueries(
+        tmpDir,
+        memoryFlag,
+        addSnippetsFlag,
+        threadsFlag,
+        config,
+        getRunnerLogger(true)
+      );
+      t.deepEqual(Object.keys(builtinStatusReport).length, 1);
+      t.true(
+        `analyze_builtin_queries_${language}_duration_ms` in builtinStatusReport
+      );
+
+      config.queries[language] = {
+        builtin: [],
+        custom: ["foo.ql"],
+      };
+      const customStatusReport = await runQueries(
+        tmpDir,
+        memoryFlag,
+        addSnippetsFlag,
+        threadsFlag,
+        config,
+        getRunnerLogger(true)
+      );
+      t.deepEqual(Object.keys(customStatusReport).length, 1);
+      t.true(
+        `analyze_custom_queries_${language}_duration_ms` in customStatusReport
+      );
+    }
+  });
+});

src/analyze.ts

@@ -82,60 +82,78 @@ async function finalizeDatabaseCreation(
 }
 
 // Runs queries and creates sarif files in the given folder
-async function runQueries(
+export async function runQueries(
   sarifFolder: string,
   memoryFlag: string,
   addSnippetsFlag: string,
   threadsFlag: string,
   config: configUtils.Config,
   logger: Logger
 ): Promise<QueriesStatusReport> {
-  const codeql = getCodeQL(config.codeQLCmd);
+  const statusReport: QueriesStatusReport = {};
+
   for (const language of config.languages) {
     logger.startGroup(`Analyzing ${language}`);
 
-    const queries = config.queries[language] || [];
-    if (queries.length === 0) {
+    const queries = config.queries[language];
+    if (queries.builtin.length === 0 && queries.custom.length === 0) {
       throw new Error(
         `Unable to analyse ${language} as no queries were selected for this language`
       );
     }
 
     try {
-      const databasePath = util.getCodeQLDatabasePath(config.tempDir, language);
-      // Pass the queries to codeql using a file instead of using the command
-      // line to avoid command line length restrictions, particularly on windows.
-      const querySuite = `${databasePath}-queries.qls`;
-      const querySuiteContents = queries.map((q) => `- query: ${q}`).join("\n");
-      fs.writeFileSync(querySuite, querySuiteContents);
-      logger.debug(
-        `Query suite file for ${language}...\n${querySuiteContents}`
-      );
-
-      const sarifFile = path.join(sarifFolder, `${language}.sarif`);
-
-      await codeql.databaseAnalyze(
-        databasePath,
-        sarifFile,
-        querySuite,
-        memoryFlag,
-        addSnippetsFlag,
-        threadsFlag
-      );
-
-      logger.debug(
-        `SARIF results for database ${language} created at "${sarifFile}"`
-      );
-      logger.endGroup();
+      for (const type of ["builtin", "custom"]) {
+        if (queries[type].length > 0) {
+          const startTime = new Date().getTime();
+
+          const databasePath = util.getCodeQLDatabasePath(
+            config.tempDir,
+            language
+          );
+          // Pass the queries to codeql using a file instead of using the command
+          // line to avoid command line length restrictions, particularly on windows.
+          const querySuitePath = `${databasePath}-queries-${type}.qls`;
+          const querySuiteContents = queries[type]
+            .map((q: string) => `- query: ${q}`)
+            .join("\n");
+          fs.writeFileSync(querySuitePath, querySuiteContents);
+          logger.debug(
+            `Query suite file for ${language}...\n${querySuiteContents}`
+          );
+
+          const sarifFile = path.join(sarifFolder, `${language}-${type}.sarif`);
+
+          const codeql = getCodeQL(config.codeQLCmd);
+          await codeql.databaseAnalyze(
+            databasePath,
+            sarifFile,
+            querySuitePath,
+            memoryFlag,
+            addSnippetsFlag,
+            threadsFlag
+          );
+
+          logger.debug(
+            `SARIF results for database ${language} created at "${sarifFile}"`
+          );
+          logger.endGroup();
+
+          // Record the performance
+          const endTime = new Date().getTime();
+          statusReport[`analyze_${type}_queries_${language}_duration_ms`] =
+            endTime - startTime;
+        }
+      }
     } catch (e) {
-      // For now the fields about query performance are not populated
-      return {
-        analyze_failure_language: language,
-      };
+      logger.error(`Error running analysis for ${language}: ${e}`);
+      logger.info(e);
+      statusReport.analyze_failure_language = language;
+      return statusReport;
     }
   }
 
-  return {};
+  return statusReport;
 }
 
 export async function runAnalyze(

src/config-utils.test.ts

@@ -272,7 +272,12 @@ test("load non-empty input", async (t) => {
     // And the config we expect it to parse to
     const expectedConfig: configUtils.Config = {
       languages: [Language.javascript],
-      queries: { javascript: ["/foo/a.ql", "/bar/b.ql"] },
+      queries: {
+        javascript: {
+          builtin: [],
+          custom: ["/foo/a.ql", "/bar/b.ql"],
+        },
+      },
       pathsIgnore: ["a", "b"],
       paths: ["c/d"],
       originalUserInput: {
@@ -442,9 +447,13 @@ test("Queries can be specified in config file", async (t) => {
     t.regex(resolveQueriesArgs[1].queries[0], /.*\/foo$/);
 
     // Now check that the end result contains the default queries and the query from config
-    t.deepEqual(config.queries["javascript"].length, 2);
-    t.regex(config.queries["javascript"][0], /javascript-code-scanning.qls$/);
-    t.regex(config.queries["javascript"][1], /.*\/foo$/);
+    t.deepEqual(config.queries["javascript"].builtin.length, 1);
+    t.deepEqual(config.queries["javascript"].custom.length, 1);
+    t.regex(
+      config.queries["javascript"].builtin[0],
+      /javascript-code-scanning.qls$/
+    );
+    t.regex(config.queries["javascript"].custom[0], /.*\/foo$/);
   });
 });
 
@@ -501,9 +510,13 @@ test("Queries from config file can be overridden in workflow file", async (t) =>
     t.regex(resolveQueriesArgs[1].queries[0], /.*\/override$/);
 
     // Now check that the end result contains only the default queries and the override query
-    t.deepEqual(config.queries["javascript"].length, 2);
-    t.regex(config.queries["javascript"][0], /javascript-code-scanning.qls$/);
-    t.regex(config.queries["javascript"][1], /.*\/override$/);
+    t.deepEqual(config.queries["javascript"].builtin.length, 1);
+    t.deepEqual(config.queries["javascript"].custom.length, 1);
+    t.regex(
+      config.queries["javascript"].builtin[0],
+      /javascript-code-scanning.qls$/
+    );
+    t.regex(config.queries["javascript"].custom[0], /.*\/override$/);
   });
 });
 
@@ -558,8 +571,9 @@ test("Queries in workflow file can be used in tandem with the 'disable default q
     t.regex(resolveQueriesArgs[0].queries[0], /.*\/workflow-query$/);
 
     // Now check that the end result contains only the workflow query, and not the default one
-    t.deepEqual(config.queries["javascript"].length, 1);
-    t.regex(config.queries["javascript"][0], /.*\/workflow-query$/);
+    t.deepEqual(config.queries["javascript"].builtin.length, 0);
+    t.deepEqual(config.queries["javascript"].custom.length, 1);
+    t.regex(config.queries["javascript"].custom[0], /.*\/workflow-query$/);
   });
 });
 
@@ -610,10 +624,14 @@ test("Multiple queries can be specified in workflow file, no config file require
     t.regex(resolveQueriesArgs[2].queries[0], /.*\/override2$/);
 
     // Now check that the end result contains both the queries from the workflow, as well as the defaults
-    t.deepEqual(config.queries["javascript"].length, 3);
-    t.regex(config.queries["javascript"][0], /javascript-code-scanning.qls$/);
-    t.regex(config.queries["javascript"][1], /.*\/override1$/);
-    t.regex(config.queries["javascript"][2], /.*\/override2$/);
+    t.deepEqual(config.queries["javascript"].builtin.length, 1);
+    t.deepEqual(config.queries["javascript"].custom.length, 2);
+    t.regex(
+      config.queries["javascript"].builtin[0],
+      /javascript-code-scanning.qls$/
+    );
+    t.regex(config.queries["javascript"].custom[0], /.*\/override1$/);
+    t.regex(config.queries["javascript"].custom[1], /.*\/override2$/);
   });
 });
 
@@ -678,11 +696,15 @@ test("Queries in workflow file can be added to the set of queries without overri
     t.regex(resolveQueriesArgs[3].queries[0], /.*\/foo$/);
 
     // Now check that the end result contains all the queries
-    t.deepEqual(config.queries["javascript"].length, 4);
-    t.regex(config.queries["javascript"][0], /javascript-code-scanning.qls$/);
-    t.regex(config.queries["javascript"][1], /.*\/additional1$/);
-    t.regex(config.queries["javascript"][2], /.*\/additional2$/);
-    t.regex(config.queries["javascript"][3], /.*\/foo$/);
+    t.deepEqual(config.queries["javascript"].builtin.length, 1);
+    t.deepEqual(config.queries["javascript"].custom.length, 3);
+    t.regex(
+      config.queries["javascript"].builtin[0],
+      /javascript-code-scanning.qls$/
+    );
+    t.regex(config.queries["javascript"].custom[0], /.*\/additional1$/);
+    t.regex(config.queries["javascript"].custom[1], /.*\/additional2$/);
+    t.regex(config.queries["javascript"].custom[2], /.*\/foo$/);
   });
 });
 

src/config-utils.ts

@@ -31,6 +31,24 @@ export interface UserConfig {
   paths?: string[];
 }
 
+/**
+ * Lists of query files for each language.
+ * Will only contain .ql files and not other kinds of files,
+ * and all file paths will be absolute.
+ *
+ * The queries are split between ones from a builtin suite
+ * and custom queries from unknown locations. This allows us to treat
+ * them separately if we want to, for example to measure performance.
+ */
+type Queries = {
+  [language: string]: {
+    /** Queries from one of the builtin suites */
+    builtin: string[];
+    /** Custom queries, from a non-standard location */
+    custom: string[];
+  };
+};
+
 /**
  * Format of the parsed config file.
  */
@@ -41,10 +59,8 @@ export interface Config {
   languages: Language[];
   /**
    * Map from language to query files.
-   * Will only contain .ql files and not other kinds of files,
-   * and all file paths will be absolute.
    */
-  queries: { [language: string]: string[] };
+  queries: Queries;
   /**
    * List of paths to ignore from analysis.
    */
@@ -131,32 +147,44 @@ function validateQueries(resolvedQueries: ResolveQueriesOutput) {
 
 /**
  * Run 'codeql resolve queries' and add the results to resultMap
+ *
+ * If a checkout path is given then the queries are assumed to be custom queries
+ * and an error will be thrown if there is anything invalid about the queries.
+ * If a checkout path is not given then the queries are assumed to be builtin
+ * queries, and error checking will be suppressed.
  */
 async function runResolveQueries(
   codeQL: CodeQL,
-  resultMap: { [language: string]: string[] },
+  resultMap: Queries,
   toResolve: string[],
-  extraSearchPath: string | undefined,
-  errorOnInvalidQueries: boolean
+  extraSearchPath: string | undefined
 ) {
   const resolvedQueries = await codeQL.resolveQueries(
     toResolve,
     extraSearchPath
   );
 
-  for (const [language, queries] of Object.entries(
+  if (extraSearchPath !== undefined) {
+    validateQueries(resolvedQueries);
+  }
+
+  for (const [language, queryPaths] of Object.entries(
     resolvedQueries.byLanguage
   )) {
     if (resultMap[language] === undefined) {
-      resultMap[language] = [];
+      resultMap[language] = {
+        builtin: [],
+        custom: [],
+      };
     }
-    resultMap[language].push(
-      ...Object.keys(queries).filter((q) => !queryIsDisabled(language, q))
+    const queries = Object.keys(queryPaths).filter(
+      (q) => !queryIsDisabled(language, q)
     );
-  }
-
-  if (errorOnInvalidQueries) {
-    validateQueries(resolvedQueries);
+    if (extraSearchPath !== undefined) {
+      resultMap[language].custom.push(...queries);
+    } else {
+      resultMap[language].builtin.push(...queries);
+    }
   }
 }
 
@@ -166,10 +194,10 @@ async function runResolveQueries(
 async function addDefaultQueries(
   codeQL: CodeQL,
   languages: string[],
-  resultMap: { [language: string]: string[] }
+  resultMap: Queries
 ) {
   const suites = languages.map((l) => `${l}-code-scanning.qls`);
-  await runResolveQueries(codeQL, resultMap, suites, undefined, false);
+  await runResolveQueries(codeQL, resultMap, suites, undefined);
 }
 
 // The set of acceptable values for built-in suites from the codeql bundle
@@ -182,7 +210,7 @@ const builtinSuites = ["security-extended", "security-and-quality"] as const;
 async function addBuiltinSuiteQueries(
   languages: string[],
   codeQL: CodeQL,
-  resultMap: { [language: string]: string[] },
+  resultMap: Queries,
   suiteName: string,
   configFile?: string
 ) {
@@ -192,15 +220,15 @@ async function addBuiltinSuiteQueries(
   }
 
   const suites = languages.map((l) => `${l}-${suiteName}.qls`);
-  await runResolveQueries(codeQL, resultMap, suites, undefined, false);
+  await runResolveQueries(codeQL, resultMap, suites, undefined);
 }
 
 /**
  * Retrieve the set of queries at localQueryPath and add them to resultMap.
  */
 async function addLocalQueries(
   codeQL: CodeQL,
-  resultMap: { [language: string]: string[] },
+  resultMap: Queries,
   localQueryPath: string,
   checkoutPath: string,
   configFile?: string
@@ -228,21 +256,15 @@ async function addLocalQueries(
     );
   }
 
-  await runResolveQueries(
-    codeQL,
-    resultMap,
-    [absoluteQueryPath],
-    checkoutPath,
-    true
-  );
+  await runResolveQueries(codeQL, resultMap, [absoluteQueryPath], checkoutPath);
 }
 
 /**
  * Retrieve the set of queries at the referenced remote repo and add them to resultMap.
  */
 async function addRemoteQueries(
   codeQL: CodeQL,
-  resultMap: { [language: string]: string[] },
+  resultMap: Queries,
   queryUses: string,
   tempDir: string,
   githubUrl: string,
@@ -283,7 +305,7 @@ async function addRemoteQueries(
       ? path.join(checkoutPath, tok.slice(2).join("/"))
       : checkoutPath;
 
-  await runResolveQueries(codeQL, resultMap, [queryPath], checkoutPath, true);
+  await runResolveQueries(codeQL, resultMap, [queryPath], checkoutPath);
 }
 
 /**
@@ -297,7 +319,7 @@ async function addRemoteQueries(
 async function parseQueryUses(
   languages: string[],
   codeQL: CodeQL,
-  resultMap: { [language: string]: string[] },
+  resultMap: Queries,
   queryUses: string,
   tempDir: string,
   checkoutPath: string,
@@ -660,7 +682,7 @@ async function addQueriesFromWorkflow(
   codeQL: CodeQL,
   queriesInput: string,
   languages: string[],
-  resultMap: { [language: string]: string[] },
+  resultMap: Queries,
   tempDir: string,
   checkoutPath: string,
   githubUrl: string,
@@ -718,7 +740,7 @@ export async function getDefaultConfig(
     githubUrl,
     logger
   );
-  const queries = {};
+  const queries: Queries = {};
   await addDefaultQueries(codeQL, languages, queries);
   if (queriesInput) {
     await addQueriesFromWorkflow(
@@ -790,7 +812,7 @@ async function loadConfig(
     logger
   );
 
-  const queries = {};
+  const queries: Queries = {};
   const pathsIgnore: string[] = [];
   const paths: string[] = [];
 
@@ -880,7 +902,11 @@ async function loadConfig(
   // The list of queries should not be empty for any language. If it is then
   // it is a user configuration error.
   for (const language of languages) {
-    if (queries[language] === undefined || queries[language].length === 0) {
+    if (
+      queries[language] === undefined ||
+      (queries[language].builtin.length === 0 &&
+        queries[language].custom.length === 0)
+    ) {
       throw new Error(
         `Did not detect any queries to run for ${language}. ` +
           "Please make sure that the default queries are enabled, or you are specifying queries to run."