Merge branch 'main' into dbartol/remove-actions-extractor

src/api-client.test.ts

@@ -149,4 +149,24 @@ test("wrapApiConfigurationError correctly wraps specific configuration errors",
     res,
     new util.ConfigurationError("API rate limit exceeded for installation"),
   );
+
+  const tokenSuggestionMessage =
+    "Please check that your token is valid and has the required permissions: contents: read, security-events: write";
+  const badCredentialsError = new util.HTTPError("Bad credentials", 401);
+  res = api.wrapApiConfigurationError(badCredentialsError);
+  t.deepEqual(res, new util.ConfigurationError(tokenSuggestionMessage));
+
+  const notFoundError = new util.HTTPError("Not Found", 404);
+  res = api.wrapApiConfigurationError(notFoundError);
+  t.deepEqual(res, new util.ConfigurationError(tokenSuggestionMessage));
+
+  const resourceNotAccessibleError = new util.HTTPError(
+    "Resource not accessible by integration",
+    403,
+  );
+  res = api.wrapApiConfigurationError(resourceNotAccessibleError);
+  t.deepEqual(
+    res,
+    new util.ConfigurationError("Resource not accessible by integration"),
+  );
 });

src/api-client.ts

@@ -245,9 +245,17 @@ export function wrapApiConfigurationError(e: unknown) {
     if (
       e.message.includes("API rate limit exceeded for installation") ||
       e.message.includes("commit not found") ||
+      e.message.includes("Resource not accessible by integration") ||
       /ref .* not found in this repository/.test(e.message)
     ) {
       return new ConfigurationError(e.message);
+    } else if (
+      e.message.includes("Bad credentials") ||
+      e.message.includes("Not Found")
+    ) {
+      return new ConfigurationError(
+        "Please check that your token is valid and has the required permissions: contents: read, security-events: write",
+      );
     }
   }
   return e;

src/status-report.test.ts

@@ -216,7 +216,7 @@ test("getActionStatus handling correctly various types of errors", (t) => {
       "multiple things went wrong",
     ),
     "user-error",
-    "getActionsStatus should return failure if passed a configuration error and an additional failure cause",
+    "getActionsStatus should return user-error if passed a configuration error and an additional failure cause",
   );
 
   t.is(

src/status-report.ts

@@ -55,6 +55,13 @@ export function isFirstPartyAnalysis(actionName: ActionName): boolean {
   return process.env[EnvVar.INIT_ACTION_HAS_RUN] === "true";
 }
 
+/**
+ * @returns true if the analysis is considered to be third party.
+ */
+export function isThirdPartyAnalysis(actionName: ActionName): boolean {
+  return !isFirstPartyAnalysis(actionName);
+}
+
 export type ActionStatus =
   | "aborted" // Only used in the init Action, if init failed before initializing the tracer due to something other than a configuration error.
   | "failure"

src/upload-lib.test.ts

@@ -405,6 +405,47 @@ test("shouldShowCombineSarifFilesDeprecationWarning when environment variable is
   );
 });
 
+test("shouldConsiderConfigurationError correctly detects configuration errors", (t) => {
+  const error1 = [
+    "CodeQL analyses from advanced configurations cannot be processed when the default setup is enabled",
+  ];
+  t.true(uploadLib.shouldConsiderConfigurationError(error1));
+
+  const error2 = [
+    "rejecting delivery as the repository has too many logical alerts",
+  ];
+  t.true(uploadLib.shouldConsiderConfigurationError(error2));
+
+  // We fail cases where we get > 1 error messages back
+  const error3 = [
+    "rejecting delivery as the repository has too many alerts",
+    "extra error message",
+  ];
+  t.false(uploadLib.shouldConsiderConfigurationError(error3));
+});
+
+test("shouldConsiderInvalidRequest returns correct recognises processing errors", (t) => {
+  const error1 = [
+    "rejecting SARIF",
+    "an invalid URI was provided as a SARIF location",
+  ];
+  t.true(uploadLib.shouldConsiderInvalidRequest(error1));
+
+  const error2 = [
+    "locationFromSarifResult: expected artifact location",
+    "an invalid URI was provided as a SARIF location",
+  ];
+  t.true(uploadLib.shouldConsiderInvalidRequest(error2));
+
+  // We expect ALL errors to be of processing errors, for the outcome to be classified as
+  // an invalid SARIF upload error.
+  const error3 = [
+    "could not convert rules: invalid security severity value, is not a number",
+    "an unknown error occurred",
+  ];
+  t.false(uploadLib.shouldConsiderInvalidRequest(error3));
+});
+
 function createMockSarif(id?: string, tool?: string) {
   return {
     runs: [

src/upload-lib.ts

@@ -568,9 +568,16 @@ export async function uploadFiles(
 
   const gitHubVersion = await getGitHubVersion();
 
-  // Validate that the files we were asked to upload are all valid SARIF files
-  for (const file of sarifFiles) {
-    validateSarifFileSchema(file, logger);
+  try {
+    // Validate that the files we were asked to upload are all valid SARIF files
+    for (const file of sarifFiles) {
+      validateSarifFileSchema(file, logger);
+    }
+  } catch (e) {
+    if (e instanceof SyntaxError) {
+      throw new InvalidSarifUploadError(e.message);
+    }
+    throw e;
   }
 
   let sarif = await combineSarifFilesUsingCLI(
@@ -734,18 +741,26 @@ export async function waitForProcessing(
 /**
  * Returns whether the provided processing errors are a configuration error.
  */
-function shouldConsiderConfigurationError(processingErrors: string[]): boolean {
+export function shouldConsiderConfigurationError(
+  processingErrors: string[],
+): boolean {
+  const expectedConfigErrors = [
+    "CodeQL analyses from advanced configurations cannot be processed when the default setup is enabled",
+    "rejecting delivery as the repository has too many logical alerts",
+  ];
+
   return (
     processingErrors.length === 1 &&
-    processingErrors[0] ===
-      "CodeQL analyses from advanced configurations cannot be processed when the default setup is enabled"
+    expectedConfigErrors.some((msg) => processingErrors[0].includes(msg))
   );
 }
 
 /**
  * Returns whether the provided processing errors are the result of an invalid SARIF upload request.
  */
-function shouldConsiderInvalidRequest(processingErrors: string[]): boolean {
+export function shouldConsiderInvalidRequest(
+  processingErrors: string[],
+): boolean {
   return processingErrors.every(
     (error) =>
       error.startsWith("rejecting SARIF") ||

src/upload-sarif-action.ts

@@ -12,7 +12,7 @@ import {
   StatusReportBase,
   getActionsStatus,
   ActionName,
-  isFirstPartyAnalysis,
+  isThirdPartyAnalysis,
 } from "./status-report";
 import * as upload_lib from "./upload-lib";
 import {
@@ -105,7 +105,7 @@ async function run() {
     await sendSuccessStatusReport(startedAt, uploadResult.statusReport, logger);
   } catch (unwrappedError) {
     const error =
-      !isFirstPartyAnalysis(ActionName.UploadSarif) &&
+      isThirdPartyAnalysis(ActionName.UploadSarif) &&
       unwrappedError instanceof upload_lib.InvalidSarifUploadError
         ? new ConfigurationError(unwrappedError.message)
         : wrapError(unwrappedError);