From d433dabfd06559f6ef170033c94dcfbd48fd877f Mon Sep 17 00:00:00 2001 From: akream Date: Wed, 28 Oct 2026 12:06:28 +0000 Subject: [PATCH] decode SARIF URIs when filtering diff-informed alerts Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b1744ae3-8204-4901-8dc7-16368fd6d865 --- lib/entry-points.js | 12 +++- src/upload-lib.test.ts | 58 +++++++++++++++++++ src/upload-lib.ts | 16 ++++- .../fix-diff-informed-sarif-paths.md | 1 + 4 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 unreleased-change-notes/fix-diff-informed-sarif-paths.md diff --git a/lib/entry-points.js b/lib/entry-points.js index 872366a314..996e6b392e 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -188340,11 +188340,19 @@ function filterAlertsByDiffRange(logger2, sarifLog) { return locations.some((physicalLocation) => { const locationUri = physicalLocation?.artifactLocation?.uri; const locationStartLine = physicalLocation?.region?.startLine; - if (!locationUri || locationStartLine === void 0) { + if (typeof locationUri !== "string" || !locationUri || locationStartLine === void 0) { return false; } + let locationPath = locationUri; + try { + locationPath = decodeURIComponent(locationUri); + } catch { + logger2.debug( + `Failed to decode location URI "${locationUri}", comparing it as a raw path` + ); + } return diffRanges.some( - (range2) => range2.path === locationUri && (range2.startLine <= locationStartLine && range2.endLine >= locationStartLine || range2.startLine === 0 && range2.endLine === 0) + (range2) => range2.path === locationPath && (range2.startLine <= locationStartLine && range2.endLine >= locationStartLine || range2.startLine === 0 && range2.endLine === 0) ); }); }); diff --git a/src/upload-lib.test.ts b/src/upload-lib.test.ts index ebfc5cd404..047f115ede 100644 --- a/src/upload-lib.test.ts +++ b/src/upload-lib.test.ts @@ -1043,3 +1043,61 @@ test.serial( t.deepEqual(actualOutput, expectedOutput); }, ); + +for (const [filePath, uri] of [ + ["src/with space.js", "src/with%20space.js"], + ["src/caf\u00e9.js", "src/caf%C3%A9.js"], + ["src/caf\u00e9.js", "src/caf%c3%a9.js"], + ["src/!()'*.js", "src/%21%28%29%27%2A.js"], + ["src/literal%20.js", "src/literal%2520.js"], +]) { + test.serial(`filterAlertsByDiffRange decodes URI ${uri}`, (t) => { + const input = sarif.readSarifFile( + `${__dirname}/../src/testdata/valid-sarif.sarif`, + ); + input.runs![0].results![0].locations![0].physicalLocation!.artifactLocation!.uri = + uri; + + const actualOutput = runFilterAlertsByDiffRange(input, [ + { path: filePath, startLine: 1, endLine: 3 }, + ]); + + t.is(actualOutput.runs?.[0].results?.length, 1); + }); +} + +for (const uri of ["src/bad%.js", "src/bad%GG.js", "src/bad%C3%28.js"]) { + test.serial( + `filterAlertsByDiffRange falls back for malformed URI ${uri}`, + (t) => { + const input = sarif.readSarifFile( + `${__dirname}/../src/testdata/valid-sarif.sarif`, + ); + input.runs![0].results![0].locations![0].physicalLocation!.artifactLocation!.uri = + uri; + + const actualOutput = runFilterAlertsByDiffRange(input, [ + { path: uri, startLine: 0, endLine: 0 }, + ]); + + t.is(actualOutput.runs?.[0].results?.length, 1); + }, + ); +} + +test.serial( + "filterAlertsByDiffRange filters malformed URI without a matching raw path", + (t) => { + const input = sarif.readSarifFile( + `${__dirname}/../src/testdata/valid-sarif.sarif`, + ); + input.runs![0].results![0].locations![0].physicalLocation!.artifactLocation!.uri = + "src/bad%GG.js"; + + const actualOutput = runFilterAlertsByDiffRange(input, [ + { path: "src/different.js", startLine: 0, endLine: 0 }, + ]); + + t.is(actualOutput.runs?.[0].results?.length, 0); + }, +); diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 4e97c1d1db..bf5ab5fab2 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -1099,9 +1099,21 @@ export function filterAlertsByDiffRange( return locations.some((physicalLocation) => { const locationUri = physicalLocation?.artifactLocation?.uri; const locationStartLine = physicalLocation?.region?.startLine; - if (!locationUri || locationStartLine === undefined) { + if ( + typeof locationUri !== "string" || + !locationUri || + locationStartLine === undefined + ) { return false; } + let locationPath = locationUri; + try { + locationPath = decodeURIComponent(locationUri); + } catch { + logger.debug( + `Failed to decode location URI "${locationUri}", comparing it as a raw path`, + ); + } // 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 @@ -1109,7 +1121,7 @@ export function filterAlertsByDiffRange( // of an alert location. return diffRanges.some( (range) => - range.path === locationUri && + range.path === locationPath && ((range.startLine <= locationStartLine && range.endLine >= locationStartLine) || (range.startLine === 0 && range.endLine === 0)), diff --git a/unreleased-change-notes/fix-diff-informed-sarif-paths.md b/unreleased-change-notes/fix-diff-informed-sarif-paths.md new file mode 100644 index 0000000000..f899078284 --- /dev/null +++ b/unreleased-change-notes/fix-diff-informed-sarif-paths.md @@ -0,0 +1 @@ +- Fixed diff-informed alert filtering incorrectly removing alerts in changed files whose paths contain spaces, special characters, or non-ASCII characters.