Prevent combineSarifFilesUsingCLI initialising CodeQL instance more than once

Author: mbg

lib/analyze-action.js

@@ -363,7 +363,9 @@ async function run(startedAt: Date) {
 
       uploadResults = await postProcessAndUploadSarif(
         logger,
+        config.tempDir,
         features,
+        async () => codeql,
         uploadKind,
         checkoutPath,
         outputDir,

lib/init-action-post.js

@@ -424,6 +424,8 @@ async function testFailedSarifUpload(
     }
     t.true(
       uploadFiles.calledOnceWith(
+        sinon.match.string,
+        codeqlObject,
         sinon.match.string,
         sinon.match.string,
         category,

lib/upload-lib.js

@@ -104,6 +104,8 @@ async function maybeUploadFailedSarif(
 
   logger.info(`Uploading failed SARIF file ${sarifFile}`);
   const uploadResult = await uploadLib.uploadFiles(
+    config.tempDir,
+    codeql,
     sarifFile,
     checkoutPath,
     category,

lib/upload-sarif-action.js

@@ -11,8 +11,7 @@ import * as actionsUtil from "./actions-util";
 import * as analyses from "./analyses";
 import * as api from "./api-client";
 import { getGitHubVersion, wrapApiConfigurationError } from "./api-client";
-import { getCodeQL, type CodeQL } from "./codeql";
-import { getConfig } from "./config-utils";
+import { type CodeQL } from "./codeql";
 import { readDiffRangesJsonFile } from "./diff-informed-analysis-utils";
 import { EnvVar } from "./environment";
 import { FeatureEnablement } from "./feature-flags";
@@ -219,15 +218,19 @@ export async function minimalInitCodeQL(
   return initCodeQLResult.codeql;
 }
 
+export type CodeQLGetter = () => Promise<CodeQL>;
+
 // Takes a list of paths to sarif files and combines them together using the
 // CLI `github merge-results` command when all SARIF files are produced by
 // CodeQL. Otherwise, it will fall back to combining the files in the action.
 // Returns the contents of the combined sarif file.
 async function combineSarifFilesUsingCLI(
   sarifFiles: string[],
   gitHubVersion: GitHubVersion,
-  features: FeatureEnablement,
+  _features: FeatureEnablement,
   logger: Logger,
+  getCodeQL: CodeQLGetter,
+  tempDir: string,
 ): Promise<SarifFile> {
   logger.info("Combining SARIF files using the CodeQL CLI");
 
@@ -265,18 +268,10 @@ async function combineSarifFilesUsingCLI(
     return combineSarifFiles(sarifFiles, logger);
   }
 
-  // Initialize CodeQL, either by using the config file from the 'init' step,
-  // or by initializing it here.
-  let codeQL: CodeQL;
-  let tempDir: string = actionsUtil.getTemporaryDirectory();
-
-  const config = await getConfig(tempDir, logger);
-  if (config !== undefined) {
-    codeQL = await getCodeQL(config.codeQLCmd);
-    tempDir = config.tempDir;
-  } else {
-    codeQL = await minimalInitCodeQL(logger, gitHubVersion, features);
-  }
+  // Obtain a `CodeQL` instance. For `analyze`, this is typically the instance that was used for running the queries.
+  // For `upload-sarif`, this either initialises a new instance or returns a previously initialised one if `getCodeQL`
+  // is called more than once.
+  const codeQL: CodeQL = await getCodeQL();
 
   const baseTempDir = path.resolve(tempDir, "combined-sarif");
   fs.mkdirSync(baseTempDir, { recursive: true });
@@ -682,6 +677,8 @@ export interface PostProcessingResults {
  *
  * @param logger The logger to use.
  * @param features Information about enabled features.
+ * @param getCodeQL A function to retrieve a `CodeQL` instance.
+ * @param tempPath A path to a temporary directory.
  * @param checkoutPath The path where the repo was checked out at.
  * @param sarifPaths The paths of the SARIF files to post-process.
  * @param category The analysis category.
@@ -693,6 +690,8 @@ export interface PostProcessingResults {
 export async function postProcessSarifFiles(
   logger: Logger,
   features: FeatureEnablement,
+  getCodeQL: CodeQLGetter,
+  tempPath: string,
   checkoutPath: string,
   sarifPaths: string[],
   category: string | undefined,
@@ -717,6 +716,8 @@ export async function postProcessSarifFiles(
       gitHubVersion,
       features,
       logger,
+      getCodeQL,
+      tempPath,
     );
   } else {
     const sarifPath = sarifPaths[0];
@@ -777,6 +778,8 @@ export async function writePostProcessedFiles(
  * to.
  */
 export async function uploadFiles(
+  tempDir: string,
+  codeql: CodeQL,
   inputSarifPath: string,
   checkoutPath: string,
   category: string | undefined,
@@ -790,6 +793,8 @@ export async function uploadFiles(
   );
 
   return uploadSpecifiedFiles(
+    tempDir,
+    codeql,
     sarifPaths,
     checkoutPath,
     category,
@@ -803,6 +808,8 @@ export async function uploadFiles(
  * Uploads the given array of SARIF files.
  */
 async function uploadSpecifiedFiles(
+  tempDir: string,
+  codeql: CodeQL,
   sarifPaths: string[],
   checkoutPath: string,
   category: string | undefined,
@@ -813,6 +820,8 @@ async function uploadSpecifiedFiles(
   const processingResults: PostProcessingResults = await postProcessSarifFiles(
     logger,
     features,
+    async () => codeql,
+    tempDir,
     checkoutPath,
     sarifPaths,
     category,

src/analyze-action.ts

@@ -4,7 +4,9 @@ import * as actionsUtil from "./actions-util";
 import { getActionVersion, getTemporaryDirectory } from "./actions-util";
 import * as analyses from "./analyses";
 import { getGitHubVersion } from "./api-client";
-import { initFeatures } from "./feature-flags";
+import { CodeQL, getCodeQL } from "./codeql";
+import { Config, getConfig } from "./config-utils";
+import { FeatureEnablement, initFeatures } from "./feature-flags";
 import { Logger, getActionsLogger } from "./logging";
 import { getRepositoryNwo } from "./repository";
 import {
@@ -20,6 +22,7 @@ import * as upload_lib from "./upload-lib";
 import { postProcessAndUploadSarif } from "./upload-sarif";
 import {
   ConfigurationError,
+  GitHubVersion,
   checkActionVersion,
   checkDiskUsage,
   getErrorMessage,
@@ -54,6 +57,35 @@ async function sendSuccessStatusReport(
   }
 }
 
+/** The cached `CodeQL` instance, if any. */
+let codeql: CodeQL | undefined;
+
+/** Get or initialise a `CodeQL` instance for use by the `upload-sarif` action. */
+async function getOrInitCodeQL(
+  logger: Logger,
+  gitHubVersion: GitHubVersion,
+  features: FeatureEnablement,
+  config: Config | undefined,
+): Promise<CodeQL> {
+  // Return the cached instance, if we have one.
+  if (codeql !== undefined) return codeql;
+
+  // If we have been able to load a `Config` from an earlier CodeQL Action step in the job,
+  // then use the CodeQL executable that we have used previously. Otherwise, initialise the
+  // CLI specifically for `upload-sarif`. Either way, we cache the instance.
+  if (config !== undefined) {
+    codeql = await getCodeQL(config.codeQLCmd);
+  } else {
+    codeql = await upload_lib.minimalInitCodeQL(
+      logger,
+      gitHubVersion,
+      features,
+    );
+  }
+
+  return codeql;
+}
+
 async function run(startedAt: Date) {
   // To capture errors appropriately, keep as much code within the try-catch as
   // possible, and only use safe functions outside.
@@ -94,9 +126,20 @@ async function run(startedAt: Date) {
     const checkoutPath = actionsUtil.getRequiredInput("checkout_path");
     const category = actionsUtil.getOptionalInput("category");
 
+    // Determine the temporary directory to use. If we are able to read a `Config` from a previous CodeQL Action
+    // step in the job, then use the temporary directory configured there. Otherwise, use our default.
+    let tempDir: string = actionsUtil.getTemporaryDirectory();
+
+    const config = await getConfig(tempDir, logger);
+    if (config !== undefined) {
+      tempDir = config.tempDir;
+    }
+
     const uploadResults = await postProcessAndUploadSarif(
       logger,
+      tempDir,
       features,
+      () => getOrInitCodeQL(logger, gitHubVersion, features, config),
       "always",
       checkoutPath,
       sarifPath,

src/init-action-post-helper.test.ts

@@ -5,6 +5,7 @@ import test, { ExecutionContext } from "ava";
 import * as sinon from "sinon";
 
 import { AnalysisKind, getAnalysisConfig } from "./analyses";
+import { getCodeQLForTesting } from "./codeql";
 import { getRunnerLogger } from "./logging";
 import { createFeatures, setupTests } from "./testing-utils";
 import { UploadResult } from "./upload-lib";
@@ -31,6 +32,8 @@ function mockPostProcessSarifFiles() {
         sinon.match.any,
         sinon.match.any,
         sinon.match.any,
+        sinon.match.any,
+        sinon.match.any,
         analysisConfig,
       )
       .resolves({ sarif: { runs: [] }, analysisKey: "", environment: "" });
@@ -73,7 +76,9 @@ const postProcessAndUploadSarifMacro = test.macro({
 
       const actual = await postProcessAndUploadSarif(
         logger,
+        tempDir,
         features,
+        async () => getCodeQLForTesting(),
         "always",
         "",
         testPath,
@@ -90,6 +95,8 @@ const postProcessAndUploadSarifMacro = test.macro({
             postProcessSarifFiles.calledWith(
               logger,
               features,
+              sinon.match.func,
+              tempDir,
               sinon.match.any,
               analysisKindResult.expectedFiles?.map(toFullPath) ??
                 fullSarifPaths,
@@ -221,7 +228,9 @@ test("postProcessAndUploadSarif doesn't upload if upload is disabled", async (t)
 
     const actual = await postProcessAndUploadSarif(
       logger,
+      tempDir,
       features,
+      () => getCodeQLForTesting(),
       "never",
       "",
       tempDir,
@@ -248,7 +257,9 @@ test("postProcessAndUploadSarif writes post-processed SARIF files if output dire
     const postProcessedOutPath = path.join(tempDir, "post-processed");
     const actual = await postProcessAndUploadSarif(
       logger,
+      tempDir,
       features,
+      () => getCodeQLForTesting(),
       "never",
       "",
       tempDir,