Merge pull request #2765 from github/cklin/alert-diff-filtering

Perform consistent diff-informed alert filtering in the action

Author: cklin

Diff for: lib/analyze.js

@@ -12,6 +12,10 @@ import { setupCppAutobuild } from "./autobuild";
 import { CodeQL, getCodeQL } from "./codeql";
 import * as configUtils from "./config-utils";
 import { addDiagnostic, makeDiagnostic } from "./diagnostics";
+import {
+  DiffThunkRange,
+  writeDiffRangesJsonFile,
+} from "./diff-filtering-utils";
 import { EnvVar } from "./environment";
 import { FeatureEnablement, Feature } from "./feature-flags";
 import { isScannedLanguage, Language } from "./languages";
@@ -284,12 +288,6 @@ export async function setupDiffInformedQueryRun(
   );
 }
 
-interface DiffThunkRange {
-  path: string;
-  startLine: number;
-  endLine: number;
-}
-
 /**
  * Return the file line ranges that were added or modified in the pull request.
  *
@@ -537,6 +535,10 @@ extensions:
     `Wrote pr-diff-range extension pack to ${extensionFilePath}:\n${extensionContents}`,
   );
 
+  // Write the diff ranges to a JSON file, for action-side alert filtering by the
+  // upload-lib module.
+  writeDiffRangesJsonFile(logger, ranges);
+
   return diffRangeDir;
 }
 

Diff for: lib/analyze.js.map

@@ -0,0 +1,42 @@
+import * as fs from "fs";
+import * as path from "path";
+
+import * as actionsUtil from "./actions-util";
+import { Logger } from "./logging";
+
+export interface DiffThunkRange {
+  path: string;
+  startLine: number;
+  endLine: number;
+}
+
+function getDiffRangesJsonFilePath(): string {
+  return path.join(actionsUtil.getTemporaryDirectory(), "pr-diff-range.json");
+}
+
+export function writeDiffRangesJsonFile(
+  logger: Logger,
+  ranges: DiffThunkRange[],
+): void {
+  const jsonContents = JSON.stringify(ranges, null, 2);
+  const jsonFilePath = getDiffRangesJsonFilePath();
+  fs.writeFileSync(jsonFilePath, jsonContents);
+  logger.debug(
+    `Wrote pr-diff-range JSON file to ${jsonFilePath}:\n${jsonContents}`,
+  );
+}
+
+export function readDiffRangesJsonFile(
+  logger: Logger,
+): DiffThunkRange[] | undefined {
+  const jsonFilePath = getDiffRangesJsonFilePath();
+  if (!fs.existsSync(jsonFilePath)) {
+    logger.debug(`Diff ranges JSON file does not exist at ${jsonFilePath}`);
+    return undefined;
+  }
+  const jsonContents = fs.readFileSync(jsonFilePath, "utf8");
+  logger.debug(
+    `Read pr-diff-range JSON file from ${jsonFilePath}:\n${jsonContents}`,
+  );
+  return JSON.parse(jsonContents) as DiffThunkRange[];
+}

Diff for: lib/diff-filtering-utils.js

@@ -14,6 +14,7 @@ import * as api from "./api-client";
 import { getGitHubVersion, wrapApiConfigurationError } from "./api-client";
 import { CodeQL, getCodeQL } from "./codeql";
 import { getConfig } from "./config-utils";
+import { readDiffRangesJsonFile } from "./diff-filtering-utils";
 import { EnvVar } from "./environment";
 import { FeatureEnablement } from "./feature-flags";
 import * as fingerprints from "./fingerprints";
@@ -578,6 +579,7 @@ export async function uploadFiles(
     features,
     logger,
   );
+  sarif = filterAlertsByDiffRange(logger, sarif);
   sarif = await fingerprints.addFingerprints(sarif, checkoutPath, logger);
 
   const analysisKey = await api.getAnalysisKey();
@@ -848,3 +850,50 @@ export class InvalidSarifUploadError extends Error {
     super(message);
   }
 }
+
+function filterAlertsByDiffRange(logger: Logger, sarif: SarifFile): SarifFile {
+  const diffRanges = readDiffRangesJsonFile(logger);
+  if (!diffRanges?.length) {
+    return sarif;
+  }
+
+  const checkoutPath = actionsUtil.getRequiredInput("checkout_path");
+
+  for (const run of sarif.runs) {
+    if (run.results) {
+      run.results = run.results.filter((result) => {
+        const locations = [
+          ...(result.locations || []).map((loc) => loc.physicalLocation),
+          ...(result.relatedLocations || []).map((loc) => loc.physicalLocation),
+        ];
+
+        return locations.some((physicalLocation) => {
+          const locationUri = physicalLocation?.artifactLocation?.uri;
+          const locationStartLine = physicalLocation?.region?.startLine;
+          if (!locationUri || locationStartLine === undefined) {
+            return false;
+          }
+          // CodeQL always uses forward slashes as the path separator, so on Windows we
+          // need to replace any backslashes with forward slashes.
+          const locationPath = path
+            .join(checkoutPath, locationUri)
+            .replaceAll(path.sep, "/");
+          // Alert filtering here replicates the same behavior as the restrictAlertsTo
+          // extensible predicate in CodeQL. See the restrictAlertsTo documentation
+          // https://codeql.github.com/codeql-standard-libraries/csharp/codeql/util/AlertFiltering.qll/predicate.AlertFiltering$restrictAlertsTo.3.html
+          // for more details, such as why the filtering applies only to the first line
+          // of an alert location.
+          return diffRanges.some(
+            (range) =>
+              range.path === locationPath &&
+              ((range.startLine <= locationStartLine &&
+                range.endLine >= locationStartLine) ||
+                (range.startLine === 0 && range.endLine === 0)),
+          );
+        });
+      });
+    }
+  }
+
+  return sarif;
+}

Diff for: lib/diff-filtering-utils.js.map

@@ -96,6 +96,16 @@ export interface SarifResult {
       };
     };
   }>;
+  relatedLocations?: Array<{
+    physicalLocation: {
+      artifactLocation: {
+        uri: string;
+      };
+      region?: {
+        startLine?: number;
+      };
+    };
+  }>;
   partialFingerprints: {
     primaryLocationLineHash?: string;
   };