Merge pull request #3123 from github/mbg/fix/upload-sarif-cq-only

Author: mbg

.github/workflows/__upload-quality-sarif.yml

@@ -6,9 +6,8 @@ steps:
   - uses: ./../action/init
     with:
       tools: ${{ steps.prepare-test.outputs.tools-url }}
-      languages: cpp,csharp,java,javascript,python
-      config-file: ${{ github.repository }}/tests/multi-language-repo/.github/codeql/custom-queries.yml@${{ github.sha }}
-      analysis-kinds: code-scanning,code-quality
+      languages: csharp,java,javascript,python
+      analysis-kinds: code-quality
   - name: Build code
     run: ./build.sh
   # Generate some SARIF we can upload with the upload-sarif step
@@ -18,6 +17,10 @@ steps:
       sha: '5e235361806c361d4d3f8859e3c897658025a9a2'
       upload: never
   - uses: ./../action/upload-sarif
+    id: upload-sarif
     with:
       ref: 'refs/heads/main'
       sha: '5e235361806c361d4d3f8859e3c897658025a9a2'
+  - name: "Check output from `upload-sarif` step"
+    if: fromJSON(steps.upload-sarif.outputs.sarif-ids)[0].analysis != 'code-quality'
+    run: exit 1

lib/upload-sarif-action.js

@@ -32,6 +32,55 @@ interface UploadSarifStatusReport
   extends StatusReportBase,
     upload_lib.UploadStatusReport {}
 
+/**
+ * Searches for SARIF files for the given `analysis` in the given `sarifPath`.
+ * If any are found, then they are uploaded to the appropriate endpoint for the given `analysis`.
+ *
+ * @param logger The logger to use.
+ * @param features Information about FFs.
+ * @param sarifPath The path to a SARIF file or directory containing SARIF files.
+ * @param pathStats Information about `sarifPath`.
+ * @param checkoutPath The checkout path.
+ * @param analysis The configuration of the analysis we should upload SARIF files for.
+ * @param category The SARIF category to use for the upload.
+ * @returns The result of uploading the SARIF file(s) or `undefined` if there are none.
+ */
+async function findAndUpload(
+  logger: Logger,
+  features: Features,
+  sarifPath: string,
+  pathStats: fs.Stats,
+  checkoutPath: string,
+  analysis: analyses.AnalysisConfig,
+  category?: string,
+): Promise<upload_lib.UploadResult | undefined> {
+  let sarifFiles: string[] | undefined;
+
+  if (pathStats.isDirectory()) {
+    sarifFiles = upload_lib.findSarifFilesInDir(
+      sarifPath,
+      analysis.sarifPredicate,
+    );
+  } else if (pathStats.isFile() && analysis.sarifPredicate(sarifPath)) {
+    sarifFiles = [sarifPath];
+  } else {
+    return undefined;
+  }
+
+  if (sarifFiles.length !== 0) {
+    return await upload_lib.uploadSpecifiedFiles(
+      sarifFiles,
+      checkoutPath,
+      category,
+      features,
+      logger,
+      analysis,
+    );
+  }
+
+  return undefined;
+}
+
 async function sendSuccessStatusReport(
   startedAt: Date,
   uploadStats: upload_lib.UploadStatusReport,
@@ -86,54 +135,71 @@ async function run() {
   }
 
   try {
+    // `sarifPath` can either be a path to a single file, or a path to a directory.
     const sarifPath = actionsUtil.getRequiredInput("sarif_file");
     const checkoutPath = actionsUtil.getRequiredInput("checkout_path");
     const category = actionsUtil.getOptionalInput("category");
+    const pathStats = fs.lstatSync(sarifPath, { throwIfNoEntry: false });
 
-    const uploadResult = await upload_lib.uploadFiles(
+    if (pathStats === undefined) {
+      throw new ConfigurationError(`Path does not exist: ${sarifPath}.`);
+    }
+
+    const sarifIds: Array<{ analysis: string; id: string }> = [];
+    const uploadResult = await findAndUpload(
+      logger,
+      features,
       sarifPath,
+      pathStats,
       checkoutPath,
-      category,
-      features,
-      logger,
       analyses.CodeScanning,
+      category,
     );
-    core.setOutput("sarif-id", uploadResult.sarifID);
+    if (uploadResult !== undefined) {
+      core.setOutput("sarif-id", uploadResult.sarifID);
+      sarifIds.push({
+        analysis: analyses.AnalysisKind.CodeScanning,
+        id: uploadResult.sarifID,
+      });
+    }
 
     // If there are `.quality.sarif` files in `sarifPath`, then upload those to the code quality service.
-    // Code quality can currently only be enabled on top of security, so we'd currently always expect to
-    // have a directory for the results here.
-    if (fs.lstatSync(sarifPath).isDirectory()) {
-      const qualitySarifFiles = upload_lib.findSarifFilesInDir(
-        sarifPath,
-        analyses.CodeQuality.sarifPredicate,
-      );
-
-      if (qualitySarifFiles.length !== 0) {
-        await upload_lib.uploadSpecifiedFiles(
-          qualitySarifFiles,
-          checkoutPath,
-          actionsUtil.fixCodeQualityCategory(logger, category),
-          features,
-          logger,
-          analyses.CodeQuality,
-        );
-      }
+    const qualityUploadResult = await findAndUpload(
+      logger,
+      features,
+      sarifPath,
+      pathStats,
+      checkoutPath,
+      analyses.CodeQuality,
+      actionsUtil.fixCodeQualityCategory(logger, category),
+    );
+    if (qualityUploadResult !== undefined) {
+      sarifIds.push({
+        analysis: analyses.AnalysisKind.CodeQuality,
+        id: qualityUploadResult.sarifID,
+      });
     }
+    core.setOutput("sarif-ids", JSON.stringify(sarifIds));
 
     // We don't upload results in test mode, so don't wait for processing
     if (isInTestMode()) {
       core.debug("In test mode. Waiting for processing is disabled.");
     } else if (actionsUtil.getRequiredInput("wait-for-processing") === "true") {
-      await upload_lib.waitForProcessing(
-        getRepositoryNwo(),
-        uploadResult.sarifID,
-        logger,
-      );
+      if (uploadResult !== undefined) {
+        await upload_lib.waitForProcessing(
+          getRepositoryNwo(),
+          uploadResult.sarifID,
+          logger,
+        );
+      }
       // The code quality service does not currently have an endpoint to wait for SARIF processing,
       // so we can't wait for that here.
     }
-    await sendSuccessStatusReport(startedAt, uploadResult.statusReport, logger);
+    await sendSuccessStatusReport(
+      startedAt,
+      uploadResult?.statusReport || {},
+      logger,
+    );
   } catch (unwrappedError) {
     const error =
       isThirdPartyAnalysis(ActionName.UploadSarif) &&

pr-checks/checks/upload-quality-sarif.yml

@@ -34,7 +34,12 @@ inputs:
     default: "true"
 outputs:
   sarif-id:
-    description: The ID of the uploaded SARIF file.
+    description: The ID of the uploaded Code Scanning SARIF file, if any.
+  sarif-ids:
+    description: |
+      A stringified JSON object containing the SARIF ID for each kind of analysis. For example:
+
+      { "code-scanning": "some-id", "code-quality": "some-other-id" }
 runs:
   using: node20
   main: '../lib/upload-sarif-action.js'