XS✔ ◾ Fix Small Bugs and Remove Test Mock Artifact

dist/resources.resjson

@@ -138,6 +138,8 @@
   "loc.messages.repos.azureReposInvoker.insufficientAzureReposAccessTokenPermissions.comment": "The message to display when terminating as the Azure Repos Personal Access Token (PAT) does not have sufficient access.",
   "loc.messages.repos.azureReposInvoker.noAzureReposAccessToken": "Could not access the Workload Identity Federation or Personal Access Token (PAT). Add the 'WorkloadIdentityFederation' input or 'PR_Metrics_Access_Token' as a secret environment variable.",
   "loc.messages.repos.azureReposInvoker.noAzureReposAccessToken.comment": "The message to display when terminating as no access token is available.",
+  "loc.messages.repos.baseReposInvoker.resourceNotFound": "The resource could not be found. Verify the repository and pull request exist.",
+  "loc.messages.repos.baseReposInvoker.resourceNotFound.comment": "The message to display when an API call returns HTTP 404.",
   "loc.messages.repos.gitHubReposInvoker.insufficientGitHubAccessTokenPermissions": "Could not access the resources. Ensure the 'PR_Metrics_Access_Token' secret environment variable has Read and Write access to pull requests (or access to 'repos' if using a Classic PAT).",
   "loc.messages.repos.gitHubReposInvoker.insufficientGitHubAccessTokenPermissions.comment": "The message to display when terminating as the GitHub Personal Access Token (PAT) does not have sufficient access.",
   "loc.messages.repos.gitHubReposInvoker.noGitHubAccessToken": "Could not access the Personal Access Token (PAT). Add 'PR_Metrics_Access_Token' as a secret environment variable with Read and Write access to Pull Requests (or access to 'repos' if using a Classic PAT, or write access to 'pull-requests' and 'statuses' if specified within the workflow YAML).",

src/task/Strings/resources.resjson/en-US/resources.resjson

@@ -138,6 +138,8 @@
   "loc.messages.repos.azureReposInvoker.insufficientAzureReposAccessTokenPermissions.comment": "The message to display when terminating as the Azure Repos Personal Access Token (PAT) does not have sufficient access.",
   "loc.messages.repos.azureReposInvoker.noAzureReposAccessToken": "Could not access the Workload Identity Federation or Personal Access Token (PAT). Add the 'WorkloadIdentityFederation' input or 'PR_Metrics_Access_Token' as a secret environment variable.",
   "loc.messages.repos.azureReposInvoker.noAzureReposAccessToken.comment": "The message to display when terminating as no access token is available.",
+  "loc.messages.repos.baseReposInvoker.resourceNotFound": "The resource could not be found. Verify the repository and pull request exist.",
+  "loc.messages.repos.baseReposInvoker.resourceNotFound.comment": "The message to display when an API call returns HTTP 404.",
   "loc.messages.repos.gitHubReposInvoker.insufficientGitHubAccessTokenPermissions": "Could not access the resources. Ensure the 'PR_Metrics_Access_Token' secret environment variable has Read and Write access to pull requests (or access to 'repos' if using a Classic PAT).",
   "loc.messages.repos.gitHubReposInvoker.insufficientGitHubAccessTokenPermissions.comment": "The message to display when terminating as the GitHub Personal Access Token (PAT) does not have sufficient access.",
   "loc.messages.repos.gitHubReposInvoker.noGitHubAccessToken": "Could not access the Personal Access Token (PAT). Add 'PR_Metrics_Access_Token' as a secret environment variable with Read and Write access to Pull Requests (or access to 'repos' if using a Classic PAT, or write access to 'pull-requests' and 'statuses' if specified within the workflow YAML).",

src/task/src/git/octokitGitDiffParser.ts

@@ -111,12 +111,11 @@ export default class OctokitGitDiffParser {
      * For each diff, reinstate the "diff --git" prefix that was removed by the split. The first diff is excluded as it
      * will always be the empty string.
      */
-    const result: string[] = [];
-    for (const diffResponseLine of diffResponseLines.slice(1)) {
-      result.push(`diff --git ${diffResponseLine}`);
-    }
-
-    return result;
+    return diffResponseLines
+      .slice(1)
+      .map(
+        (diffResponseLine: string): string => `diff --git ${diffResponseLine}`,
+      );
   }
 
   private processDiffs(diffs: string[]): Map<string, number> {
@@ -152,10 +151,18 @@ export default class OctokitGitDiffParser {
           case "RenamedFile": {
             // For a renamed file, add the new file path and the first changed line.
             const fileCasted: RenamedFile = file;
-            if (fileCasted.chunks[0]) {
+            const [chunk]: AnyChunk[] = fileCasted.chunks;
+            if (chunk?.type === "BinaryFilesChunk") {
+              this._logger.logDebug(
+                `Skipping '${file.type}' '${fileCasted.pathAfter}' while performing diff parsing.`,
+              );
+              break;
+            }
+
+            if (chunk) {
               result.set(
                 fileCasted.pathAfter,
-                (fileCasted.chunks[0] as Chunk).toFileRange.start,
+                (chunk as Chunk).toFileRange.start,
               );
             }
 

src/task/src/metrics/codeMetrics.ts

@@ -357,17 +357,7 @@ export default class CodeMetrics {
   private createFileMetricsMap(input: string): CodeFileMetricInterface[] {
     this._logger.logDebug("* CodeMetrics.createFileMetricsMap()");
 
-    // Removing the ending that can be created by test mocks.
-    const endingToRemove = "\r\nrc:0\r\nsuccess:true";
-    let modifiedInput: string = input;
-    if (modifiedInput.endsWith(endingToRemove)) {
-      modifiedInput = modifiedInput.substring(
-        0,
-        input.length - endingToRemove.length,
-      );
-    }
-
-    const lines: string[] = modifiedInput.split("\n");
+    const lines: string[] = input.split("\n");
 
     const result: CodeFileMetricInterface[] = [];
     for (const line of lines) {
@@ -378,7 +368,7 @@ export default class CodeMetrics {
         typeof elements[2] === "undefined"
       ) {
         throw new RangeError(
-          `The number of elements '${String(elements.length)}' in '${line}' in input '${modifiedInput}' did not match the expected 3.`,
+          `The number of elements '${String(elements.length)}' in '${line}' in input '${input}' did not match the expected 3.`,
         );
       }
 

src/task/src/metrics/inputs.ts

@@ -341,7 +341,9 @@ export default class Inputs {
       const patterns = inputValue
         .replace(/\\/gu, "/")
         .replace(/\n$/gu, "")
-        .split("\n");
+        .split("\n")
+        .map((line: string): string => line.trim())
+        .filter((line: string): boolean => line !== "");
       if (patterns.length > maxPatternCount) {
         this._logger.logWarning(
           `The matching pattern count '${patterns.length.toLocaleString()}' exceeds the maximum '${maxPatternCount.toLocaleString()}'. Using only the first '${maxPatternCount.toLocaleString()}'.`,
@@ -370,7 +372,9 @@ export default class Inputs {
 
       const codeFileExtensionsArray: string[] = codeFileExtensions
         .replace(/\n$/gu, "")
-        .split("\n");
+        .split("\n")
+        .map((line: string): string => line.trim())
+        .filter((line: string): boolean => line !== "");
       for (const value of codeFileExtensionsArray) {
         let modifiedValue = value;
         if (modifiedValue.startsWith(wildcardStart)) {

src/task/src/pullRequestMetrics.ts

@@ -62,11 +62,14 @@ export default class PullRequestMetrics {
         this._runnerInvoker.loc("pullRequestMetrics.succeeded"),
       );
     } catch (error: unknown) {
-      const errorObject: Error = error as Error;
-      this._logger.logErrorObject(errorObject);
-      this._logger.replay();
-
-      this._runnerInvoker.setStatusFailed(errorObject.message);
+      if (error instanceof Error) {
+        this._logger.logErrorObject(error);
+        this._logger.replay();
+        this._runnerInvoker.setStatusFailed(error.message);
+      } else {
+        this._logger.replay();
+        this._runnerInvoker.setStatusFailed(String(error));
+      }
     }
   }
 }

src/task/src/pullRequests/pullRequest.ts

@@ -40,7 +40,8 @@ export default class PullRequest {
     this._logger.logDebug("* PullRequest.isPullRequest");
 
     return RunnerInvoker.isGitHub
-      ? process.env.GITHUB_BASE_REF !== ""
+      ? typeof process.env.GITHUB_BASE_REF !== "undefined" &&
+          process.env.GITHUB_BASE_REF !== ""
       : typeof process.env.SYSTEM_PULLREQUEST_PULLREQUESTID !== "undefined";
   }
 

src/task/src/repos/azureReposInvoker.ts

@@ -124,7 +124,10 @@ export default class AzureReposInvoker extends BaseReposInvoker {
       return tokenManagerResult;
     }
 
-    if (typeof process.env.PR_METRICS_ACCESS_TOKEN === "undefined") {
+    if (
+      typeof process.env.PR_METRICS_ACCESS_TOKEN === "undefined" ||
+      process.env.PR_METRICS_ACCESS_TOKEN.trim() === ""
+    ) {
       return this._runnerInvoker.loc(
         "repos.azureReposInvoker.noAzureReposAccessToken",
       );
@@ -329,6 +332,7 @@ export default class AzureReposInvoker extends BaseReposInvoker {
       this._runnerInvoker.loc(
         "repos.azureReposInvoker.insufficientAzureReposAccessTokenPermissions",
       ),
+      this._runnerInvoker.loc("repos.baseReposInvoker.resourceNotFound"),
     );
   }
 

src/task/src/repos/baseReposInvoker.ts

@@ -15,15 +15,17 @@ import { StatusCodes } from "http-status-codes";
  */
 export default abstract class BaseReposInvoker implements ReposInvokerInterface {
   /**
-   * Invokes an API call, augmenting any errors that may be thrown due to insufficient access.
+   * Invokes an API call, augmenting any errors that may be thrown due to insufficient access or missing resources.
    * @typeParam Response The type of the response from the API call.
    * @param action The action defining the API call to invoke.
    * @param accessErrorMessage The error message to insert if a caught error is due to insufficient access.
+   * @param notFoundErrorMessage The error message to insert if a caught error is due to a missing resource.
    * @returns A promise containing the response from the API call.
    */
   protected static async invokeApiCall<Response>(
     action: () => Promise<Response>,
     accessErrorMessage: string,
+    notFoundErrorMessage: string,
   ): Promise<Response> {
     try {
       return await action();
@@ -34,11 +36,13 @@ export default abstract class BaseReposInvoker implements ReposInvokerInterface
         castedError.status ?? castedError.statusCode;
       if (
         statusCode === StatusCodes.UNAUTHORIZED ||
-        statusCode === StatusCodes.FORBIDDEN ||
-        statusCode === StatusCodes.NOT_FOUND
+        statusCode === StatusCodes.FORBIDDEN
       ) {
         castedError.internalMessage = castedError.message;
         castedError.message = accessErrorMessage;
+      } else if (statusCode === StatusCodes.NOT_FOUND) {
+        castedError.internalMessage = castedError.message;
+        castedError.message = notFoundErrorMessage;
       }
 
       throw castedError;

src/task/src/repos/gitHubReposInvoker.ts

@@ -66,7 +66,10 @@ export default class GitHubReposInvoker extends BaseReposInvoker {
   public async isAccessTokenAvailable(): Promise<string | null> {
     this._logger.logDebug("* GitHubReposInvoker.isAccessTokenAvailable()");
 
-    if (typeof process.env.PR_METRICS_ACCESS_TOKEN === "undefined") {
+    if (
+      typeof process.env.PR_METRICS_ACCESS_TOKEN === "undefined" ||
+      process.env.PR_METRICS_ACCESS_TOKEN.trim() === ""
+    ) {
       return Promise.resolve(
         this._runnerInvoker.loc("repos.gitHubReposInvoker.noGitHubAccessToken"),
       );
@@ -257,6 +260,7 @@ export default class GitHubReposInvoker extends BaseReposInvoker {
       this._runnerInvoker.loc(
         "repos.gitHubReposInvoker.insufficientGitHubAccessTokenPermissions",
       ),
+      this._runnerInvoker.loc("repos.baseReposInvoker.resourceNotFound"),
     );
   }
 
@@ -411,9 +415,7 @@ export default class GitHubReposInvoker extends BaseReposInvoker {
     if (typeof result.headers.link !== "undefined") {
       const commitsLink: string = result.headers.link;
       const matches: RegExpMatchArray | null =
-        /<.+>; rel="next", <.+?page=(?<pageNumber>\d+)>; rel="last"/u.exec(
-          commitsLink,
-        );
+        /<.+?page=(?<pageNumber>\d+)>;\s*rel="last"/u.exec(commitsLink);
       if (typeof matches?.groups?.pageNumber === "undefined") {
         throw new Error(
           `The regular expression did not match '${commitsLink}'.`,

src/task/src/repos/tokenManager.ts

@@ -56,7 +56,6 @@ export default class TokenManager {
       return null;
     }
 
-    this._previouslyInvoked = true;
     const workloadIdentityFederation: string | null =
       this._runnerInvoker.getInput(["Workload", "Identity", "Federation"]);
     if (workloadIdentityFederation === null) {
@@ -84,6 +83,7 @@ export default class TokenManager {
     process.env.PR_METRICS_ACCESS_TOKEN = await this.getAccessToken(
       workloadIdentityFederation,
     );
+    this._previouslyInvoked = true;
     return null;
   }
 

src/task/src/utilities/constants.ts

@@ -13,6 +13,11 @@ export const decimalRadix = 10;
  */
 export const exitCodeForFailure = 1;
 
+/**
+ * The timeout in milliseconds for HTTP requests.
+ */
+export const httpTimeoutMs = 30_000;
+
 /**
  * The maximum number of matching patterns that can be specified for file or test matching.
  */

src/task/src/utilities/logger.ts

@@ -10,6 +10,14 @@ import type RunnerInvoker from "../runners/runnerInvoker.js";
  * A class for logging messages.
  */
 export default class Logger {
+  private static readonly _sensitiveProperties: Set<string> = new Set<string>([
+    "AUTHORIZATION",
+    "COOKIE",
+    "PASSWORD",
+    "SECRET",
+    "TOKEN",
+  ]);
+
   private readonly _consoleWrapper: ConsoleWrapper;
   private readonly _runnerInvoker: RunnerInvoker;
 
@@ -29,14 +37,25 @@ export default class Logger {
   }
 
   /**
-   * Filter messages so that control strings are not printed to `stdout`.
+   * Filter messages so that control strings and newlines are not printed to `stdout`.
    * @param message The message to filter.
-   * @returns The filtered message.
+   * @returns The filtered message with control prefixes removed and newlines replaced by spaces.
    */
   private static filterMessage(message: string): string {
-    return message.replace(/##(?:vso)?\[/giu, "");
+    return message.replace(/##(?:vso)?\[/giu, "").replace(/[\n\r]/gu, " ");
   }
 
+  private static readonly _redactReplacer = (
+    key: string,
+    value: unknown,
+  ): unknown => {
+    if (key !== "" && Logger._sensitiveProperties.has(key.toUpperCase())) {
+      return "[REDACTED]";
+    }
+
+    return value;
+  };
+
   /**
    * Logs a debug message.
    * @param message The message to log.
@@ -89,9 +108,17 @@ export default class Logger {
       unknown
     >;
     for (const property of properties) {
-      this.logInfo(
-        `${name} – ${property}: ${JSON.stringify(errorRecord[property])}`,
-      );
+      if (Logger._sensitiveProperties.has(property.toUpperCase())) {
+        this.logInfo(`${name} – ${property}: [REDACTED]`);
+      } else {
+        try {
+          this.logInfo(
+            `${name} – ${property}: ${JSON.stringify(errorRecord[property], Logger._redactReplacer)}`,
+          );
+        } catch {
+          this.logInfo(`${name} – ${property}: [COULD NOT SERIALIZE]`);
+        }
+      }
     }
   }
 

src/task/src/wrappers/consoleWrapper.ts

@@ -9,12 +9,14 @@
 
 export default class ConsoleWrapper {
   /**
-   * Logs a message to `stdout` suffixed with a new line character.
-   * @param message The optional message to log.
+   * Logs a sanitized message to `stdout` suffixed with a new line character. Newline and carriage-return characters in
+   * the message are replaced with spaces to prevent log injection.
+   * @param message The message to log.
    * @param optionalParams Optional parameters to insert into the message.
    */
   public log(message: string, ...optionalParams: string[]): void {
+    const sanitizedMessage: string = message.replace(/[\n\r]/gu, " ");
     /* eslint-disable-next-line no-console -- This is a wrapper around the console. */
-    console.log(message, ...optionalParams);
+    console.log(sanitizedMessage, ...optionalParams);
   }
 }

src/task/src/wrappers/httpWrapper.ts

@@ -3,6 +3,8 @@
  * Licensed under the MIT License.
  */
 
+import { httpTimeoutMs } from "../utilities/constants.js";
+
 /**
  * A wrapper around the Fetch API, to facilitate testability.
  */
@@ -13,7 +15,15 @@ export default class HttpWrapper {
    * @returns The contents of the URL.
    */
   public async getUrl(url: string): Promise<string> {
-    const response: Response = await fetch(url);
+    const response: Response = await fetch(url, {
+      signal: AbortSignal.timeout(httpTimeoutMs),
+    });
+    if (!response.ok) {
+      throw new Error(
+        `HTTP request to '${url}' failed with status ${String(response.status)} (${response.statusText}).`,
+      );
+    }
+
     return response.text();
   }
 }

src/task/task.json

@@ -125,6 +125,7 @@
     "pullRequests.pullRequestComments.testsSufficientComment": "✔ **Thanks for adding tests.**",
     "repos.azureReposInvoker.insufficientAzureReposAccessTokenPermissions": "Could not access the resources. Ensure the 'PR_Metrics_Access_Token' secret environment variable has access to 'Code' > 'Read & write' and 'Pull Request Threads' > 'Read & write'.",
     "repos.azureReposInvoker.noAzureReposAccessToken": "Could not access the Workload Identity Federation or Personal Access Token (PAT). Add the 'WorkloadIdentityFederation' input or 'PR_Metrics_Access_Token' as a secret environment variable.",
+    "repos.baseReposInvoker.resourceNotFound": "The resource could not be found. Verify the repository and pull request exist.",
     "repos.gitHubReposInvoker.insufficientGitHubAccessTokenPermissions": "Could not access the resources. Ensure the 'PR_Metrics_Access_Token' secret environment variable has Read and Write access to pull requests (or access to 'repos' if using a Classic PAT).",
     "repos.gitHubReposInvoker.noGitHubAccessToken": "Could not access the Personal Access Token (PAT). Add 'PR_Metrics_Access_Token' as a secret environment variable with Read and Write access to Pull Requests (or access to 'repos' if using a Classic PAT, or write access to 'pull-requests' and 'statuses' if specified within the workflow YAML).",
     "repos.tokenManager.incorrectAuthorizationScheme": "Authorization scheme of workload identity federation '%s' must be 'WorkloadIdentityFederation' instead of '%s'."

src/task/task.loc.json

@@ -125,6 +125,7 @@
     "pullRequests.pullRequestComments.testsSufficientComment": "ms-resource:loc.messages.pullRequests.pullRequestComments.testsSufficientComment",
     "repos.azureReposInvoker.insufficientAzureReposAccessTokenPermissions": "ms-resource:loc.messages.repos.azureReposInvoker.insufficientAzureReposAccessTokenPermissions",
     "repos.azureReposInvoker.noAzureReposAccessToken": "ms-resource:loc.messages.repos.azureReposInvoker.noAzureReposAccessToken",
+    "repos.baseReposInvoker.resourceNotFound": "ms-resource:loc.messages.repos.baseReposInvoker.resourceNotFound",
     "repos.gitHubReposInvoker.insufficientGitHubAccessTokenPermissions": "ms-resource:loc.messages.repos.gitHubReposInvoker.insufficientGitHubAccessTokenPermissions",
     "repos.gitHubReposInvoker.noGitHubAccessToken": "ms-resource:loc.messages.repos.gitHubReposInvoker.noGitHubAccessToken",
     "repos.tokenManager.incorrectAuthorizationScheme": "ms-resource:loc.messages.repos.tokenManager.incorrectAuthorizationScheme"