Merge branch 'main' into dependabot/npm_and_yarn/npm-006da05bd8

src/analyze-action.ts

@@ -268,7 +268,7 @@ async function run() {
       pull_request &&
       (await setupDiffInformedQueryRun(
         pull_request.base.ref as string,
-        pull_request.head.ref as string,
+        pull_request.head.label as string,
         codeql,
         logger,
         features,

src/analyze.test.ts

@@ -4,7 +4,8 @@ import * as path from "path";
 import test from "ava";
 import * as sinon from "sinon";
 
-import { runQueries } from "./analyze";
+import * as actionsUtil from "./actions-util";
+import { exportedForTesting, runQueries } from "./analyze";
 import { setCodeQL } from "./codeql";
 import { Feature } from "./feature-flags";
 import { Language } from "./languages";
@@ -119,3 +120,190 @@ test("status report fields", async (t) => {
     }
   });
 });
+
+function runGetDiffRanges(changes: number, patch: string[] | undefined): any {
+  sinon
+    .stub(actionsUtil, "getRequiredInput")
+    .withArgs("checkout_path")
+    .returns("/checkout/path");
+  return exportedForTesting.getDiffRanges(
+    {
+      filename: "test.txt",
+      changes,
+      patch: patch?.join("\n"),
+    },
+    getRunnerLogger(true),
+  );
+}
+
+test("getDiffRanges: file unchanged", async (t) => {
+  const diffRanges = runGetDiffRanges(0, undefined);
+  t.deepEqual(diffRanges, []);
+});
+
+test("getDiffRanges: file diff too large", async (t) => {
+  const diffRanges = runGetDiffRanges(1000000, undefined);
+  t.deepEqual(diffRanges, undefined);
+});
+
+test("getDiffRanges: diff thunk with single addition range", async (t) => {
+  const diffRanges = runGetDiffRanges(2, [
+    "@@ -30,6 +50,8 @@",
+    " a",
+    " b",
+    " c",
+    "+1",
+    "+2",
+    " d",
+    " e",
+    " f",
+  ]);
+  t.deepEqual(diffRanges, [
+    {
+      path: "/checkout/path/test.txt",
+      startLine: 53,
+      endLine: 54,
+    },
+  ]);
+});
+
+test("getDiffRanges: diff thunk with single deletion range", async (t) => {
+  const diffRanges = runGetDiffRanges(2, [
+    "@@ -30,8 +50,6 @@",
+    " a",
+    " b",
+    " c",
+    "-1",
+    "-2",
+    " d",
+    " e",
+    " f",
+  ]);
+  t.deepEqual(diffRanges, []);
+});
+
+test("getDiffRanges: diff thunk with single update range", async (t) => {
+  const diffRanges = runGetDiffRanges(2, [
+    "@@ -30,7 +50,7 @@",
+    " a",
+    " b",
+    " c",
+    "-1",
+    "+2",
+    " d",
+    " e",
+    " f",
+  ]);
+  t.deepEqual(diffRanges, [
+    {
+      path: "/checkout/path/test.txt",
+      startLine: 53,
+      endLine: 53,
+    },
+  ]);
+});
+
+test("getDiffRanges: diff thunk with addition ranges", async (t) => {
+  const diffRanges = runGetDiffRanges(2, [
+    "@@ -30,7 +50,9 @@",
+    " a",
+    " b",
+    " c",
+    "+1",
+    " c",
+    "+2",
+    " d",
+    " e",
+    " f",
+  ]);
+  t.deepEqual(diffRanges, [
+    {
+      path: "/checkout/path/test.txt",
+      startLine: 53,
+      endLine: 53,
+    },
+    {
+      path: "/checkout/path/test.txt",
+      startLine: 55,
+      endLine: 55,
+    },
+  ]);
+});
+
+test("getDiffRanges: diff thunk with mixed ranges", async (t) => {
+  const diffRanges = runGetDiffRanges(2, [
+    "@@ -30,7 +50,7 @@",
+    " a",
+    " b",
+    " c",
+    "-1",
+    " d",
+    "-2",
+    "+3",
+    " e",
+    " f",
+    "+4",
+    "+5",
+    " g",
+    " h",
+    " i",
+  ]);
+  t.deepEqual(diffRanges, [
+    {
+      path: "/checkout/path/test.txt",
+      startLine: 54,
+      endLine: 54,
+    },
+    {
+      path: "/checkout/path/test.txt",
+      startLine: 57,
+      endLine: 58,
+    },
+  ]);
+});
+
+test("getDiffRanges: multiple diff thunks", async (t) => {
+  const diffRanges = runGetDiffRanges(2, [
+    "@@ -30,6 +50,8 @@",
+    " a",
+    " b",
+    " c",
+    "+1",
+    "+2",
+    " d",
+    " e",
+    " f",
+    "@@ -130,6 +150,8 @@",
+    " a",
+    " b",
+    " c",
+    "+1",
+    "+2",
+    " d",
+    " e",
+    " f",
+  ]);
+  t.deepEqual(diffRanges, [
+    {
+      path: "/checkout/path/test.txt",
+      startLine: 53,
+      endLine: 54,
+    },
+    {
+      path: "/checkout/path/test.txt",
+      startLine: 153,
+      endLine: 154,
+    },
+  ]);
+});
+
+test("getDiffRanges: no diff context lines", async (t) => {
+  const diffRanges = runGetDiffRanges(2, ["@@ -30 +50,2 @@", "+1", "+2"]);
+  t.deepEqual(diffRanges, [
+    {
+      path: "/checkout/path/test.txt",
+      startLine: 50,
+      endLine: 51,
+    },
+  ]);
+});

src/analyze.ts

@@ -7,6 +7,7 @@ import del from "del";
 import * as yaml from "js-yaml";
 
 import * as actionsUtil from "./actions-util";
+import { getApiClient } from "./api-client";
 import { setupCppAutobuild } from "./autobuild";
 import {
   CODEQL_VERSION_ANALYSIS_SUMMARY_V2,
@@ -17,7 +18,6 @@ import * as configUtils from "./config-utils";
 import { addDiagnostic, makeDiagnostic } from "./diagnostics";
 import { EnvVar } from "./environment";
 import { FeatureEnablement, Feature } from "./feature-flags";
-import * as gitUtils from "./git-utils";
 import { isScannedLanguage, Language } from "./languages";
 import { Logger, withGroupAsync } from "./logging";
 import { DatabaseCreationTimings, EventReport } from "./status-report";
@@ -240,7 +240,8 @@ async function finalizeDatabaseCreation(
  * Set up the diff-informed analysis feature.
  *
  * @param baseRef The base branch name, used for calculating the diff range.
- * @param headRef The head branch name, used for calculating the diff range.
+ * @param headLabel The label that uniquely identifies the head branch across
+ * repositories, used for calculating the diff range.
  * @param codeql
  * @param logger
  * @param features
@@ -249,7 +250,7 @@ async function finalizeDatabaseCreation(
  */
 export async function setupDiffInformedQueryRun(
   baseRef: string,
-  headRef: string,
+  headLabel: string,
   codeql: CodeQL,
   logger: Logger,
   features: FeatureEnablement,
@@ -262,10 +263,21 @@ export async function setupDiffInformedQueryRun(
     async () => {
       const diffRanges = await getPullRequestEditedDiffRanges(
         baseRef,
-        headRef,
+        headLabel,
         logger,
       );
-      return writeDiffRangeDataExtensionPack(logger, diffRanges);
+      const packDir = writeDiffRangeDataExtensionPack(logger, diffRanges);
+      if (packDir === undefined) {
+        logger.warning(
+          "Cannot create diff range extension pack for diff-informed queries; " +
+            "reverting to performing full analysis.",
+        );
+      } else {
+        logger.info(
+          `Successfully created diff range extension pack at ${packDir}.`,
+        );
+      }
+      return packDir;
     },
   );
 }
@@ -280,7 +292,8 @@ interface DiffThunkRange {
  * Return the file line ranges that were added or modified in the pull request.
  *
  * @param baseRef The base branch name, used for calculating the diff range.
- * @param headRef The head branch name, used for calculating the diff range.
+ * @param headLabel The label that uniquely identifies the head branch across
+ * repositories, used for calculating the diff range.
  * @param logger
  * @returns An array of tuples, where each tuple contains the absolute path of a
  * file, the start line and the end line (both 1-based and inclusive) of an
@@ -289,107 +302,167 @@ interface DiffThunkRange {
  */
 async function getPullRequestEditedDiffRanges(
   baseRef: string,
-  headRef: string,
+  headLabel: string,
   logger: Logger,
 ): Promise<DiffThunkRange[] | undefined> {
-  const checkoutPath = actionsUtil.getOptionalInput("checkout_path");
-  if (checkoutPath === undefined) {
+  const fileDiffs = await getFileDiffsWithBasehead(baseRef, headLabel, logger);
+  if (fileDiffs === undefined) {
     return undefined;
   }
+  if (fileDiffs.length >= 300) {
+    // The "compare two commits" API returns a maximum of 300 changed files. If
+    // we see that many changed files, it is possible that there could be more,
+    // with the rest being truncated. In this case, we should not attempt to
+    // compute the diff ranges, as the result would be incomplete.
+    logger.warning(
+      `Cannot retrieve the full diff because there are too many ` +
+        `(${fileDiffs.length}) changed files in the pull request.`,
+    );
+    return undefined;
+  }
+  const results: DiffThunkRange[] = [];
+  for (const filediff of fileDiffs) {
+    const diffRanges = getDiffRanges(filediff, logger);
+    if (diffRanges === undefined) {
+      return undefined;
+    }
+    results.push(...diffRanges);
+  }
+  return results;
+}
 
-  // To compute the merge bases between the base branch and the PR topic branch,
-  // we need to fetch the commit graph from the branch heads to those merge
-  // babes. The following 6-step procedure does so while limiting the amount of
-  // history fetched.
+/**
+ * This interface is an abbreviated version of the file diff object returned by
+ * the GitHub API.
+ */
+interface FileDiff {
+  filename: string;
+  changes: number;
+  // A patch may be absent if the file is binary, if the file diff is too large,
+  // or if the file is unchanged.
+  patch?: string | undefined;
+}
 
-  // Step 1: Deepen from the PR merge commit to the base branch head and the PR
-  // topic branch head, so that the PR merge commit is no longer considered a
-  // grafted commit.
-  await gitUtils.deepenGitHistory();
-  // Step 2: Fetch the base branch shallow history. This step ensures that the
-  // base branch name is present in the local repository. Normally the base
-  // branch name would be added by Step 4. However, if the base branch head is
-  // an ancestor of the PR topic branch head, Step 4 would fail without doing
-  // anything, so we need to fetch the base branch explicitly.
-  await gitUtils.gitFetch(baseRef, ["--depth=1"]);
-  // Step 3: Fetch the PR topic branch history, stopping when we reach commits
-  // that are reachable from the base branch head.
-  await gitUtils.gitFetch(headRef, [`--shallow-exclude=${baseRef}`]);
-  // Step 4: Fetch the base branch history, stopping when we reach commits that
-  // are reachable from the PR topic branch head.
-  await gitUtils.gitFetch(baseRef, [`--shallow-exclude=${headRef}`]);
-  // Step 5: Repack the history to remove the shallow grafts that were added by
-  // the previous fetches. This step works around a bug that causes subsequent
-  // deepening fetches to fail with "fatal: error in object: unshallow <SHA>".
-  // See https://stackoverflow.com/q/63878612
-  await gitUtils.gitRepack(["-d"]);
-  // Step 6: Deepen the history so that we have the merge bases between the base
-  // branch and the PR topic branch.
-  await gitUtils.deepenGitHistory();
+async function getFileDiffsWithBasehead(
+  baseRef: string,
+  headLabel: string,
+  logger: Logger,
+): Promise<FileDiff[] | undefined> {
+  const ownerRepo = util.getRequiredEnvParam("GITHUB_REPOSITORY").split("/");
+  const owner = ownerRepo[0];
+  const repo = ownerRepo[1];
+  const basehead = `${baseRef}...${headLabel}`;
+  try {
+    const response = await getApiClient().rest.repos.compareCommitsWithBasehead(
+      {
+        owner,
+        repo,
+        basehead,
+        per_page: 1,
+      },
+    );
+    logger.debug(
+      `Response from compareCommitsWithBasehead(${basehead}):` +
+        `\n${JSON.stringify(response, null, 2)}`,
+    );
+    return response.data.files;
+  } catch (error: any) {
+    if (error.status) {
+      logger.warning(`Error retrieving diff ${basehead}: ${error.message}`);
+      logger.debug(
+        `Error running compareCommitsWithBasehead(${basehead}):` +
+          `\nRequest: ${JSON.stringify(error.request, null, 2)}` +
+          `\nError Response: ${JSON.stringify(error.response, null, 2)}`,
+      );
+      return undefined;
+    } else {
+      throw error;
+    }
+  }
+}
 
-  // To compute the exact same diff as GitHub would compute for the PR, we need
-  // to use the same merge base as GitHub. That is easy to do if there is only
-  // one merge base, which is by far the most common case. If there are multiple
-  // merge bases, we stop without producing a diff range.
-  const mergeBases = await gitUtils.getAllGitMergeBases([baseRef, headRef]);
-  logger.info(`Merge bases: ${mergeBases.join(", ")}`);
-  if (mergeBases.length !== 1) {
-    logger.info(
-      "Cannot compute diff range because baseRef and headRef " +
-        `have ${mergeBases.length} merge bases (instead of exactly 1).`,
+function getDiffRanges(
+  fileDiff: FileDiff,
+  logger: Logger,
+): DiffThunkRange[] | undefined {
+  if (fileDiff.patch === undefined) {
+    if (fileDiff.changes === 0) {
+      // There are situations where a changed file legitimately has no diff.
+      // For example, the file may be a binary file, or that the file may have
+      // been renamed with no changes to its contents. In these cases, the
+      // file would be reported as having 0 changes, and we can return an empty
+      // array to indicate no diff range in this file.
+      return [];
+    }
+    // If a file is reported to have nonzero changes but no patch, that may be
+    // due to the file diff being too large. In this case, we should return
+    // undefined to indicate that we cannot process the diff.
+    logger.warning(
+      `No patch found for file ${fileDiff.filename} with ${fileDiff.changes} changes.`,
     );
     return undefined;
   }
 
-  const diffHunkHeaders = await gitUtils.getGitDiffHunkHeaders(
-    mergeBases[0],
-    headRef,
-  );
-  if (diffHunkHeaders === undefined) {
-    return undefined;
-  }
+  // Diff-informed queries expect the file path to be absolute. CodeQL always
+  // uses forward slashes as the path separator, so on Windows we need to
+  // replace any backslashes with forward slashes.
+  const filename = path
+    .join(actionsUtil.getRequiredInput("checkout_path"), fileDiff.filename)
+    .replaceAll(path.sep, "/");
 
-  const results = new Array<DiffThunkRange>();
+  // The 1-based file line number of the current line
+  let currentLine = 0;
+  // The 1-based file line number that starts the current range of added lines
+  let additionRangeStartLine: number | undefined = undefined;
+  const diffRanges: DiffThunkRange[] = [];
 
-  let changedFile = "";
-  for (const line of diffHunkHeaders) {
-    if (line.startsWith("+++ ")) {
-      const filePath = gitUtils.decodeGitFilePath(line.substring(4));
-      if (filePath.startsWith("b/")) {
-        // The file was edited: track all hunks in the file
-        changedFile = filePath.substring(2);
-      } else if (filePath === "/dev/null") {
-        // The file was deleted: skip all hunks in the file
-        changedFile = "";
-      } else {
-        logger.warning(`Failed to parse diff hunk header line: ${line}`);
-        return undefined;
-      }
+  const diffLines = fileDiff.patch.split("\n");
+  // Adding a fake context line at the end ensures that the following loop will
+  // always terminate the last range of added lines.
+  diffLines.push(" ");
+
+  for (const diffLine of diffLines) {
+    if (diffLine.startsWith("-")) {
+      // Ignore deletions completely -- we do not even want to consider them when
+      // calculating consecutive ranges of added lines.
       continue;
     }
-    if (line.startsWith("@@ ")) {
-      if (changedFile === "") continue;
-
-      const match = line.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/);
+    if (diffLine.startsWith("+")) {
+      if (additionRangeStartLine === undefined) {
+        additionRangeStartLine = currentLine;
+      }
+      currentLine++;
+      continue;
+    }
+    if (additionRangeStartLine !== undefined) {
+      // Any line that does not start with a "+" or "-" terminates the current
+      // range of added lines.
+      diffRanges.push({
+        path: filename,
+        startLine: additionRangeStartLine,
+        endLine: currentLine - 1,
+      });
+      additionRangeStartLine = undefined;
+    }
+    if (diffLine.startsWith("@@ ")) {
+      // A new hunk header line resets the current line number.
+      const match = diffLine.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/);
       if (match === null) {
-        logger.warning(`Failed to parse diff hunk header line: ${line}`);
+        logger.warning(
+          `Cannot parse diff hunk header for ${fileDiff.filename}: ${diffLine}`,
+        );
         return undefined;
       }
-      const startLine = parseInt(match[1], 10);
-      const numLines = parseInt(match[2], 10);
-      if (numLines === 0) {
-        // The hunk was a deletion: skip it
-        continue;
-      }
-      const endLine = startLine + (numLines || 1) - 1;
-      results.push({
-        path: path.join(checkoutPath, changedFile),
-        startLine,
-        endLine,
-      });
+      currentLine = parseInt(match[1], 10);
+      continue;
+    }
+    if (diffLine.startsWith(" ")) {
+      // An unchanged context line advances the current line number.
+      currentLine++;
+      continue;
     }
   }
-  return results;
+  return diffRanges;
 }
 
 /**
@@ -439,7 +512,11 @@ extensions:
   let data = ranges
     .map(
       (range) =>
-        `      - ["${range.path}", ${range.startLine}, ${range.endLine}]\n`,
+        // Using yaml.dump() with `forceQuotes: true` ensures that all special
+        // characters are escaped, and that the path is always rendered as a
+        // quoted string on a single line.
+        `      - [${yaml.dump(range.path, { forceQuotes: true }).trim()}, ` +
+        `${range.startLine}, ${range.endLine}]\n`,
     )
     .join("");
   if (!data) {
@@ -702,3 +779,7 @@ export async function runCleanup(
   }
   logger.endGroup();
 }
+
+export const exportedForTesting = {
+  getDiffRanges,
+};

src/languages.ts

@@ -19,6 +19,8 @@ export const LANGUAGE_ALIASES: { [lang: string]: Language } = {
   "c#": Language.csharp,
   kotlin: Language.java,
   typescript: Language.javascript,
+  "javascript-typescript": Language.javascript,
+  "java-kotlin": Language.java,
 };
 
 /**

src/start-proxy-action.ts

@@ -6,6 +6,7 @@ import * as toolcache from "@actions/tool-cache";
 import { pki } from "node-forge";
 
 import * as actionsUtil from "./actions-util";
+import { Language, parseLanguage } from "./languages";
 import { getActionsLogger, Logger } from "./logging";
 import * as util from "./util";
 
@@ -17,6 +18,20 @@ const PROXY_USER = "proxy_user";
 const KEY_SIZE = 2048;
 const KEY_EXPIRY_YEARS = 2;
 
+const LANGUAGE_TO_REGISTRY_TYPE: Record<Language, string> = {
+  java: "maven_repository",
+  csharp: "nuget_feed",
+  javascript: "npm_registry",
+  python: "python_index",
+  ruby: "rubygems_server",
+  rust: "cargo_registry",
+  // We do not have an established proxy type for these languages, thus leaving empty.
+  actions: "",
+  cpp: "",
+  go: "",
+  swift: "",
+} as const;
+
 type CertificateAuthority = {
   cert: string;
   key: string;
@@ -192,6 +207,11 @@ function getCredentials(logger: Logger): Credential[] {
     "registries_credentials",
   );
   const registrySecrets = actionsUtil.getOptionalInput("registry_secrets");
+  const languageString = actionsUtil.getOptionalInput("language");
+  const language = languageString ? parseLanguage(languageString) : undefined;
+  const registryTypeForLanguage = language
+    ? LANGUAGE_TO_REGISTRY_TYPE[language]
+    : undefined;
 
   let credentialsStr: string;
   if (registriesCredentials !== undefined) {
@@ -212,6 +232,13 @@ function getCredentials(logger: Logger): Credential[] {
     if (e.url === undefined && e.host === undefined) {
       throw new Error("Invalid credentials - must specify host or url");
     }
+
+    // Filter credentials based on language if specified. `type` is the registry type.
+    // E.g., "maven_feed" for Java/Kotlin, "nuget_repository" for C#.
+    if (e.type !== registryTypeForLanguage) {
+      continue;
+    }
+
     out.push({
       type: e.type,
       host: e.host,