Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Move conversion of PR diff-range paths to absolute paths
  • Loading branch information
kaspersv committed Nov 12, 2025
commit 4eb247591f3ebb27df9032f5a6d73fff582b5f8d
23 changes: 8 additions & 15 deletions lib/analyze-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 5 additions & 7 deletions lib/init-action-post.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions lib/upload-lib.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions lib/upload-sarif-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/analyze.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as path from "path";
import test from "ava";
import * as sinon from "sinon";

import * as actionsUtil from "./actions-util";
import { CodeQuality, CodeScanning } from "./analyses";
import {
runQueries,
Expand Down Expand Up @@ -161,9 +162,13 @@ test("addSarifExtension", (t) => {
});

test("diffRangeExtensionPackContents", (t) => {
sinon
.stub(actionsUtil, "getRequiredInput")
.withArgs("checkout_path")
.returns("/checkout/path");
const output = diffRangeExtensionPackContents([
{
path: "/checkout/path/main.js",
path: "main.js",
startLine: 10,
endLine: 20,
},
Expand Down
30 changes: 21 additions & 9 deletions src/analyze.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import { performance } from "perf_hooks";
import * as io from "@actions/io";
import * as yaml from "js-yaml";

import { getTemporaryDirectory, PullRequestBranches } from "./actions-util";
import {
getTemporaryDirectory,
getRequiredInput,
PullRequestBranches,
} from "./actions-util";
import * as analyses from "./analyses";
import { setupCppAutobuild } from "./autobuild";
import { type CodeQL } from "./codeql";
Expand Down Expand Up @@ -257,14 +261,22 @@ extensions:
`;

let data = ranges
.map(
(range) =>
// 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`,
)
.map((range) => {
// 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(getRequiredInput("checkout_path"), range.path)
.replaceAll(path.sep, "/");

// 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.
return (
` - [${yaml.dump(filename, { forceQuotes: true }).trim()}, ` +
`${range.startLine}, ${range.endLine}]\n`
);
})
.join("");
if (!data) {
// Ensure that the data extension is not empty, so that a pull request with
Expand Down
24 changes: 10 additions & 14 deletions src/diff-informed-analysis-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,6 @@ test(
);

function runGetDiffRanges(changes: number, patch: string[] | undefined): any {
sinon
.stub(actionsUtil, "getRequiredInput")
.withArgs("checkout_path")
.returns("/checkout/path");
return exportedForTesting.getDiffRanges(
{
filename: "test.txt",
Expand All @@ -211,7 +207,7 @@ test("getDiffRanges: file diff too large", async (t) => {
const diffRanges = runGetDiffRanges(1000000, undefined);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
path: "test.txt",
startLine: 0,
endLine: 0,
},
Expand All @@ -232,7 +228,7 @@ test("getDiffRanges: diff thunk with single addition range", async (t) => {
]);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
path: "test.txt",
startLine: 53,
endLine: 54,
},
Expand Down Expand Up @@ -268,7 +264,7 @@ test("getDiffRanges: diff thunk with single update range", async (t) => {
]);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
path: "test.txt",
startLine: 53,
endLine: 53,
},
Expand All @@ -290,12 +286,12 @@ test("getDiffRanges: diff thunk with addition ranges", async (t) => {
]);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
path: "test.txt",
startLine: 53,
endLine: 53,
},
{
path: "/checkout/path/test.txt",
path: "test.txt",
startLine: 55,
endLine: 55,
},
Expand All @@ -322,12 +318,12 @@ test("getDiffRanges: diff thunk with mixed ranges", async (t) => {
]);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
path: "test.txt",
startLine: 54,
endLine: 54,
},
{
path: "/checkout/path/test.txt",
path: "test.txt",
startLine: 57,
endLine: 58,
},
Expand Down Expand Up @@ -357,12 +353,12 @@ test("getDiffRanges: multiple diff thunks", async (t) => {
]);
t.deepEqual(diffRanges, [
{
path: "/checkout/path/test.txt",
path: "test.txt",
startLine: 53,
endLine: 54,
},
{
path: "/checkout/path/test.txt",
path: "test.txt",
startLine: 153,
endLine: 154,
},
Expand All @@ -373,7 +369,7 @@ 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",
path: "test.txt",
startLine: 50,
endLine: 51,
},
Expand Down
11 changes: 2 additions & 9 deletions src/diff-informed-analysis-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,6 @@ function getDiffRanges(
fileDiff: FileDiff,
logger: Logger,
): DiffThunkRange[] | 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, "/");

if (fileDiff.patch === undefined) {
if (fileDiff.changes === 0) {
// There are situations where a changed file legitimately has no diff.
Expand All @@ -212,7 +205,7 @@ function getDiffRanges(
// to a special diff range that covers the entire file.
return [
{
path: filename,
path: fileDiff.filename,
startLine: 0,
endLine: 0,
},
Expand Down Expand Up @@ -247,7 +240,7 @@ function getDiffRanges(
// Any line that does not start with a "+" or "-" terminates the current
// range of added lines.
diffRanges.push({
path: filename,
path: fileDiff.filename,
startLine: additionRangeStartLine,
endLine: currentLine - 1,
});
Expand Down
7 changes: 1 addition & 6 deletions src/upload-lib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { HTTPError } from "@actions/tool-cache";
import test from "ava";
import * as sinon from "sinon";

import * as actionsUtil from "./actions-util";
import * as analyses from "./analyses";
import { AnalysisKind, CodeQuality, CodeScanning } from "./analyses";
import * as api from "./api-client";
Expand Down Expand Up @@ -972,10 +971,6 @@ function runFilterAlertsByDiffRange(
input: SarifFile,
diffRanges: diffUtils.DiffThunkRange[],
): SarifFile {
sinon
.stub(actionsUtil, "getRequiredInput")
.withArgs("checkout_path")
.returns("/checkout/path");
sinon.stub(diffUtils, "readDiffRangesJsonFile").returns(diffRanges);
return uploadLib.filterAlertsByDiffRange(getRunnerLogger(true), input);
}
Expand All @@ -986,7 +981,7 @@ test("filterAlertsByDiffRange filters out alerts outside diff-range", (t) => {
);
const actualOutput = runFilterAlertsByDiffRange(input, [
{
path: "/checkout/path/main.js",
path: "main.js",
startLine: 1,
endLine: 3,
},
Expand Down
9 changes: 1 addition & 8 deletions src/upload-lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1143,8 +1143,6 @@ export function filterAlertsByDiffRange(
return sarif;
}

const checkoutPath = actionsUtil.getRequiredInput("checkout_path");

for (const run of sarif.runs) {
if (run.results) {
run.results = run.results.filter((result) => {
Expand All @@ -1159,19 +1157,14 @@ export function filterAlertsByDiffRange(
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.path === locationUri &&
((range.startLine <= locationStartLine &&
range.endLine >= locationStartLine) ||
(range.startLine === 0 && range.endLine === 0)),
Expand Down
Loading