Update upload input values and logic (#1598)

- The `upload` input to the `analyze` Action now accepts the following values:
    - `always` is the default value, which uploads the SARIF file to Code Scanning for successful and failed runs.
    - `failure-only` is recommended for customers post-processing the SARIF file before uploading it to Code Scanning. This option uploads debugging information to Code Scanning for failed runs to improve the debugging experience.
    - `never` avoids uploading the SARIF file to Code Scanning even if the code scanning run fails. This is not recommended for external users since it complicates debugging.
    - The legacy `true` and `false` options will be interpreted as `always` and `failure-only` respectively.

---------

Co-authored-by: Henry Mercer <henry.mercer@me.com>

src/actions-util.ts

@@ -680,3 +680,25 @@ export async function printDebugLogs(config: Config) {
     walkLogFiles(logsDirectory);
   }
 }
+
+export type UploadKind = "always" | "failure-only" | "never";
+
+// Parses the `upload` input into an `UploadKind`, converting unspecified and deprecated upload inputs appropriately.
+export function getUploadValue(input: string | undefined): UploadKind {
+  switch (input) {
+    case undefined:
+    case "true":
+    case "always":
+      return "always";
+    case "false":
+    case "failure-only":
+      return "failure-only";
+    case "never":
+      return "never";
+    default:
+      core.warning(
+        `Unrecognized 'upload' input to 'analyze' Action: ${input}. Defaulting to 'always'.`
+      );
+      return "always";
+  }
+}

src/analyze-action.ts

@@ -268,8 +268,8 @@ async function run() {
       dbLocations[language] = util.getCodeQLDatabasePath(config, language);
     }
     core.setOutput("db-locations", dbLocations);
-
-    if (runStats && actionsUtil.getRequiredInput("upload") === "true") {
+    const uploadInput = actionsUtil.getOptionalInput("upload");
+    if (runStats && actionsUtil.getUploadValue(uploadInput) === "always") {
       uploadResult = await upload_lib.uploadFromActions(
         outputDir,
         actionsUtil.getRequiredInput("checkout_path"),

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

@@ -161,33 +161,70 @@ test("uploads failed SARIF run with database export-diagnostics if the database
   });
 });
 
-test("doesn't upload failed SARIF for workflow with upload: false", async (t) => {
-  const actionsWorkflow = createTestWorkflow([
-    {
-      name: "Checkout repository",
-      uses: "actions/checkout@v3",
-    },
-    {
-      name: "Initialize CodeQL",
-      uses: "github/codeql-action/init@v2",
-      with: {
-        languages: "javascript",
+const UPLOAD_INPUT_TEST_CASES = [
+  {
+    uploadInput: "true",
+    shouldUpload: true,
+  },
+  {
+    uploadInput: "false",
+    shouldUpload: true,
+  },
+  {
+    uploadInput: "always",
+    shouldUpload: true,
+  },
+  {
+    uploadInput: "failure-only",
+    shouldUpload: true,
+  },
+  {
+    uploadInput: "never",
+    shouldUpload: false,
+  },
+  {
+    uploadInput: "unrecognized-value",
+    shouldUpload: true,
+  },
+];
+
+for (const { uploadInput, shouldUpload } of UPLOAD_INPUT_TEST_CASES) {
+  test(`does ${
+    shouldUpload ? "" : "not "
+  }upload failed SARIF run for workflow with upload: ${uploadInput}`, async (t) => {
+    const actionsWorkflow = createTestWorkflow([
+      {
+        name: "Checkout repository",
+        uses: "actions/checkout@v3",
       },
-    },
-    {
-      name: "Perform CodeQL Analysis",
-      uses: "github/codeql-action/analyze@v2",
-      with: {
-        category: "my-category",
-        upload: false,
+      {
+        name: "Initialize CodeQL",
+        uses: "github/codeql-action/init@v2",
+        with: {
+          languages: "javascript",
+        },
       },
-    },
-  ]);
-  const result = await testFailedSarifUpload(t, actionsWorkflow, {
-    expectUpload: false,
+      {
+        name: "Perform CodeQL Analysis",
+        uses: "github/codeql-action/analyze@v2",
+        with: {
+          category: "my-category",
+          upload: uploadInput,
+        },
+      },
+    ]);
+    const result = await testFailedSarifUpload(t, actionsWorkflow, {
+      category: "my-category",
+      expectUpload: shouldUpload,
+    });
+    if (!shouldUpload) {
+      t.is(
+        result.upload_failed_run_skipped_because,
+        "SARIF upload is disabled"
+      );
+    }
   });
-  t.is(result.upload_failed_run_skipped_because, "SARIF upload is disabled");
-});
+}
 
 test("uploading failed SARIF run succeeds when workflow uses an input with a matrix var", async (t) => {
   const actionsWorkflow = createTestWorkflow([
@@ -294,14 +331,14 @@ async function testFailedSarifUpload(
   {
     category,
     databaseExists = true,
-    exportDiagnosticsEnabled = false,
     expectUpload = true,
+    exportDiagnosticsEnabled = false,
     matrix = {},
   }: {
     category?: string;
     databaseExists?: boolean;
-    exportDiagnosticsEnabled?: boolean;
     expectUpload?: boolean;
+    exportDiagnosticsEnabled?: boolean;
     matrix?: { [key: string]: string };
   } = {}
 ): Promise<initActionPostHelper.UploadFailedSarifResult> {
@@ -356,8 +393,6 @@ async function testFailedSarifUpload(
       raw_upload_size_bytes: 20,
       zipped_upload_size_bytes: 10,
     });
-  }
-  if (expectUpload) {
     if (databaseExists && exportDiagnosticsEnabled) {
       t.true(
         databaseExportDiagnosticsStub.calledOnceWith(

src/init-action-post-helper.ts

@@ -56,8 +56,11 @@ async function maybeUploadFailedSarif(
   const workflow = await getWorkflow();
   const jobName = getRequiredEnvParam("GITHUB_JOB");
   const matrix = parseMatrixInput(actionsUtil.getRequiredInput("matrix"));
+  const shouldUpload = getUploadInputOrThrow(workflow, jobName, matrix);
   if (
-    getUploadInputOrThrow(workflow, jobName, matrix) !== "true" ||
+    !["always", "failure-only"].includes(
+      actionsUtil.getUploadValue(shouldUpload)
+    ) ||
     isInTestMode()
   ) {
     return { upload_failed_run_skipped_because: "SARIF upload is disabled" };

src/workflow.ts

@@ -426,22 +426,20 @@ export function getCategoryInputOrThrow(
  *
  * Typically you'll want to wrap this function in a try/catch block and handle the error.
  *
- * @returns the upload input
+ * @returns the user input to upload, or undefined if input was unspecified
  * @throws an error if the upload input could not be determined
  */
 export function getUploadInputOrThrow(
   workflow: Workflow,
   jobName: string,
   matrixVars: { [key: string]: string } | undefined
-): string {
-  return (
-    getInputOrThrow(
-      workflow,
-      jobName,
-      getAnalyzeActionName(),
-      "upload",
-      matrixVars
-    ) || "true" // if unspecified, upload defaults to true
+): string | undefined {
+  return getInputOrThrow(
+    workflow,
+    jobName,
+    getAnalyzeActionName(),
+    "upload",
+    matrixVars
   );
 }