Merge pull request #4283 from github/sam-robson/decode-diff-informed-sarif-uris

fix: decode SARIF location URIs in diff-informed alert filtering

.github/workflows/__unsupported-macos.yml

@@ -114,7 +114,7 @@ jobs:
       - name: Install dependencies
         run: npm ci
       - name: Download workflow artifact
-        uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
+        uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2
         with:
           pattern: test-mode-sarif-*
           path: sarif

.github/workflows/update-bundle.yml

@@ -113,11 +113,12 @@ jobs:
           echo "CLI_VERSION=$cli_version" | tee -a "$GITHUB_ENV"
           echo "PR_URL=$pr_url" | tee -a "$GITHUB_ENV"
 
-      - name: Create changelog note
+      - name: Create changenote
         run: |
           npx tsx pr-checks/bundle-changelog.ts
 
-      - name: Push changelog note
+      - name: Push changenote
         run: |
-          git commit -am "Add changelog note"
+          git add unreleased-change-notes
+          git commit -m "Add changenote"
           git push

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 See the [releases page](https://github.com/github/codeql-action/releases) for the relevant changes to the CodeQL CLI and language packs.
 
+## [UNRELEASED]
+
+No user facing changes.
+
 ## 4.39.2 - 03 Nov 2026
 
 - Update default CodeQL bundle version to [2.27.4](https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.27.4). [#4284](https://github.com/github/codeql-action/pull/4284)

lib/entry-points.js

@@ -157611,10 +157611,18 @@ var require_cipherModes = __commonJS({
         return false;
       }
       var len = output.length();
+      if (len === 0) {
+        return true;
+      }
       var count = output.at(len - 1);
-      if (count > this.blockSize << 2) {
+      if (count === 0 || count > this.blockSize || count > len) {
         return false;
       }
+      for (var i = 2; i <= count; ++i) {
+        if (output.at(len - i) !== count) {
+          return false;
+        }
+      }
       output.truncate(count);
       return true;
     };
@@ -157676,10 +157684,18 @@ var require_cipherModes = __commonJS({
         return false;
       }
       var len = output.length();
+      if (len === 0) {
+        return true;
+      }
       var count = output.at(len - 1);
-      if (count > this.blockSize << 2) {
+      if (count === 0 || count > this.blockSize || count > len) {
         return false;
       }
+      for (var i = 2; i <= count; ++i) {
+        if (output.at(len - i) !== count) {
+          return false;
+        }
+      }
       output.truncate(count);
       return true;
     };
@@ -157937,11 +157953,14 @@ var require_cipherModes = __commonJS({
       } else {
         additionalData = forge.util.createBuffer();
       }
-      if ("tagLength" in options) {
-        this._tagLength = options.tagLength;
+      if (options.tagLength !== void 0 && options.tagLength !== null) {
+        this._tagLength = Number(options.tagLength);
       } else {
         this._tagLength = 128;
       }
+      if ([128, 120, 112, 104, 96, 64, 32].indexOf(this._tagLength) === -1) {
+        throw new Error("Invalid tag length.");
+      }
       this._tag = null;
       if (options.decrypt) {
         this._tag = forge.util.createBuffer(options.tag).getBytes();
@@ -158892,7 +158911,10 @@ var require_asn1 = __commonJS({
             var subOptions = {
               // enforce strict mode to avoid parsing ASN.1 from plain data
               strict: true,
-              decodeBitStrings: true
+              // only decode one level of BIT STRINGs, nested decoding has
+              // quadratic cost due to copies for change detection
+              decodeBitStrings: false,
+              maxDepth: options.maxDepth
             };
             var composed = _fromDer(bytes, remaining, depth + 1, subOptions);
             var used = start - bytes.length();
@@ -158996,12 +159018,16 @@ var require_asn1 = __commonJS({
     asn1.oidToDer = function(oid) {
       var values = oid.split(".");
       var bytes = forge.util.createBuffer();
-      bytes.putByte(40 * parseInt(values[0], 10) + parseInt(values[1], 10));
+      var first = parseInt(values[0], 10);
+      var second = parseInt(values[1], 10);
+      if (!(first >= 0 && first <= 2 && second >= 0) || first < 2 && second > 39) {
+        throw new Error("Invalid OID; the first two arcs are out of range.");
+      }
       var last, valueBytes, value, b;
-      for (var i = 2; i < values.length; ++i) {
+      for (var i = 1; i < values.length; ++i) {
         last = true;
         valueBytes = [];
-        value = parseInt(values[i], 10);
+        value = i === 1 ? 40 * first + second : parseInt(values[i], 10);
         if (value > 4294967295) {
           throw new Error("OID value too large; max is 32-bits.");
         }
@@ -159025,9 +159051,9 @@ var require_asn1 = __commonJS({
       if (typeof bytes === "string") {
         bytes = forge.util.createBuffer(bytes);
       }
-      var b = bytes.getByte();
-      oid = Math.floor(b / 40) + "." + b % 40;
+      var b;
       var value = 0;
+      var first = true;
       while (bytes.length() > 0) {
         if (value > 70368744177663) {
           throw new Error("OID value too large; max is 53-bits.");
@@ -159037,7 +159063,13 @@ var require_asn1 = __commonJS({
         if (b & 128) {
           value += b & 127;
         } else {
-          oid += "." + (value + b);
+          value += b;
+          if (first) {
+            oid = value < 80 ? Math.floor(value / 40) + "." + value % 40 : "2." + (value - 80);
+            first = false;
+          } else {
+            oid += "." + value;
+          }
           value = 0;
         }
       }
@@ -159873,12 +159905,12 @@ var require_pem = __commonJS({
     };
     pem.decode = function(str) {
       var rval = [];
-      var rMessage = /\s*-----BEGIN ([A-Z0-9- ]+)-----\r?\n?([\x21-\x7e\s]+?(?:\r?\n\r?\n))?([:A-Za-z0-9+\/=\s]+?)-----END \1-----/g;
-      var rHeader = /([\x21-\x7e]+):\s*([\x21-\x7e\s^:]+)/;
+      var messages = _pemMessageScanner(str);
+      var rHeader = /(?:^|[^\x21-\x7e])([\x21-\x7e]+):\s*([\x21-\x7e\s^:]+)/;
       var rCRLF = /\r?\n/;
       var match2;
       while (true) {
-        match2 = rMessage.exec(str);
+        match2 = messages.next();
         if (!match2) {
           break;
         }
@@ -159901,7 +159933,7 @@ var require_pem = __commonJS({
         var lines = match2[2].split(rCRLF);
         var li = 0;
         while (match2 && li < lines.length) {
-          var line = lines[li].replace(/\s+$/, "");
+          var line = rtrim(lines[li]);
           for (var nl = li + 1; nl < lines.length; ++nl) {
             var next = lines[nl];
             if (!/\s/.test(next[0])) {
@@ -159979,6 +160011,140 @@ var require_pem = __commonJS({
     function ltrim(str) {
       return str.replace(/^\s+/, "");
     }
+    function rtrim(str) {
+      var end = str.length;
+      while (end > 0 && /\s/.test(str.charAt(end - 1))) {
+        --end;
+      }
+      return str.substring(0, end);
+    }
+    function _pemMessageScanner(str) {
+      var rBody = /[:A-Za-z0-9+\/=\s]/;
+      var rNotBody = /[^:A-Za-z0-9+\/=\s]/g;
+      var rNotHeader = /[^\x21-\x7e\s]/g;
+      var rLabel = /[A-Z0-9- ]/;
+      function indexOfRegex(regex, i) {
+        regex.lastIndex = i;
+        var match2 = regex.exec(str);
+        return match2 ? match2.index : str.length;
+      }
+      function labelAt(i) {
+        for (var j = i; j < str.length && rLabel.test(str.charAt(j)); ++j) {
+          if (j > i && str.substr(j, 5) === "-----") {
+            return str.substring(i, j);
+          }
+        }
+        return null;
+      }
+      function blankLineEnd(i) {
+        if (str.charAt(i) === "\r") {
+          ++i;
+        }
+        if (str.charAt(i) !== "\n") {
+          return -1;
+        }
+        if (str.charAt(++i) === "\r") {
+          ++i;
+        }
+        return str.charAt(i) === "\n" ? i + 1 : -1;
+      }
+      function firstBlankLine(start2, end2) {
+        for (var i = start2; i < end2; ++i) {
+          var v = blankLineEnd(i);
+          if (v !== -1 && v < end2) {
+            return i;
+          }
+        }
+        return -1;
+      }
+      var ends = {};
+      for (var end = str.indexOf("-----END "); end !== -1; end = str.indexOf("-----END ", end + 1)) {
+        var endLabel = labelAt(end + 9);
+        var start = end;
+        while (start > 0 && rBody.test(str.charAt(start - 1))) {
+          --start;
+        }
+        var blank = firstBlankLine(start, end);
+        if (endLabel !== null && blank !== -1) {
+          var key = "$" + endLabel;
+          (ends[key] = ends[key] || []).push({ end, blank });
+        }
+      }
+      var pos = 0;
+      var headerEnd = -1;
+      return {
+        next: function() {
+          var begin;
+          while ((begin = str.indexOf("-----BEGIN ", pos)) !== -1) {
+            pos = begin + 1;
+            var label = labelAt(begin + 11);
+            if (label === null) {
+              continue;
+            }
+            var q = begin + 16 + label.length;
+            var bodyEnd = indexOfRegex(rNotBody, q);
+            var bodyEndOk = str.substr(bodyEnd, 9) === "-----END " && labelAt(bodyEnd + 9) === label;
+            if (q > headerEnd) {
+              headerEnd = indexOfRegex(rNotHeader, q);
+            }
+            var later = null;
+            var list = ends["$" + label];
+            if (list) {
+              var lo = 0;
+              var hi = list.length;
+              while (lo < hi) {
+                var mid = lo + hi >> 1;
+                if (list[mid].end > bodyEnd) {
+                  hi = mid;
+                } else {
+                  lo = mid + 1;
+                }
+              }
+              later = list[lo] || null;
+            }
+            var starts = [];
+            if (str.charAt(q) === "\r") {
+              if (str.charAt(q + 1) === "\n") {
+                starts.push(q + 2);
+              }
+              starts.push(q + 1);
+            } else if (str.charAt(q) === "\n") {
+              starts.push(q + 1);
+            }
+            starts.push(q);
+            for (var i = 0; i < starts.length; ++i) {
+              var t = starts[i];
+              var blank2 = bodyEndOk ? firstBlankLine(t + 1, bodyEnd) : -1;
+              var end2 = bodyEnd;
+              if (blank2 === -1 && later) {
+                blank2 = later.blank;
+                end2 = later.end;
+              }
+              if (blank2 !== -1 && blank2 <= headerEnd) {
+                var v = blankLineEnd(blank2);
+                pos = end2 + 14 + label.length;
+                return [
+                  str.substring(begin, pos),
+                  label,
+                  str.substring(t, v),
+                  str.substring(v, end2)
+                ];
+              }
+              if (bodyEndOk && t < bodyEnd) {
+                pos = bodyEnd + 14 + label.length;
+                return [
+                  str.substring(begin, pos),
+                  label,
+                  void 0,
+                  str.substring(t, bodyEnd)
+                ];
+              }
+            }
+          }
+          return null;
+        }
+      };
+    }
   }
 });
 
@@ -162416,7 +162582,7 @@ var require_jsbn = __commonJS({
       return r;
     }
     function bnModInverse(m) {
-      if (this.signum() == 0) {
+      if (this.signum() <= 0 || m.signum() <= 0) {
         return BigInteger.ZERO;
       }
       var ac = m.isEven();
@@ -163295,6 +163461,41 @@ var require_rsa = __commonJS({
         capture: "digest"
       }]
     };
+    var _encodeDigestInfo = function(oid, digest, withParameters) {
+      var digestAlgorithm = [asn1.create(
+        asn1.Class.UNIVERSAL,
+        asn1.Type.OID,
+        false,
+        asn1.oidToDer(oid).getBytes()
+      )];
+      if (withParameters) {
+        digestAlgorithm.push(asn1.create(
+          asn1.Class.UNIVERSAL,
+          asn1.Type.NULL,
+          false,
+          ""
+        ));
+      }
+      return asn1.toDer(asn1.create(
+        asn1.Class.UNIVERSAL,
+        asn1.Type.SEQUENCE,
+        true,
+        [
+          asn1.create(
+            asn1.Class.UNIVERSAL,
+            asn1.Type.SEQUENCE,
+            true,
+            digestAlgorithm
+          ),
+          asn1.create(
+            asn1.Class.UNIVERSAL,
+            asn1.Type.OCTETSTRING,
+            false,
+            digest
+          )
+        ]
+      )).getBytes();
+    };
     var emsaPkcs1v15encode = function(md2) {
       var oid;
       if (md2.algorithm in pki2.oids) {
@@ -163798,6 +163999,13 @@ var require_rsa = __commonJS({
                   );
                 }
               }
+              var withParameters = _encodeDigestInfo(oid, capture.digest, true);
+              var withoutParameters = _encodeDigestInfo(oid, capture.digest, false);
+              if (d2.substring(0, withParameters.length) !== withParameters && d2.substring(0, withoutParameters.length) !== withoutParameters) {
+                throw new Error(
+                  "ASN.1 object does not contain a valid RSASSA-PKCS1-v1_5 DigestInfo value."
+                );
+              }
               return digest2 === capture.digest;
             }
           };
@@ -164758,6 +164966,9 @@ var require_pbe = __commonJS({
     };
     pki2.pbe.generatePkcs12Key = function(password, salt, id, iter, n, md2) {
       var j, l;
+      if (!(iter >= 1)) {
+        throw new Error("PKCS#12 key derivation iteration count must be >= 1.");
+      }
       if (typeof md2 === "undefined" || md2 === null) {
         if (!("sha1" in forge.md)) {
           throw new Error('"sha1" hash algorithm unavailable.');
@@ -166043,6 +166254,7 @@ var require_x509 = __commonJS({
     };
     var _verifySignature = function(options) {
       var cert = options.certificate;
+      var publicKey = options.publicKey || cert.publicKey;
       var scheme;
       switch (cert.signatureOid) {
         case oids.sha1WithRSAEncryption:
@@ -166080,7 +166292,7 @@ var require_x509 = __commonJS({
           );
           break;
       }
-      return cert.publicKey.verify(
+      return publicKey.verify(
         options.md.digest().getBytes(),
         options.signature,
         scheme
@@ -166300,7 +166512,8 @@ var require_x509 = __commonJS({
         }
         if (md2 !== null) {
           rval = _verifySignature({
-            certificate: cert,
+            certificate: child,
+            publicKey: cert.publicKey,
             md: md2,
             signature: child.signature
           });
@@ -167535,10 +167748,7 @@ var require_x509 = __commonJS({
             }
           }
           if (parent) {
-            var parents = parent;
-            if (!forge.util.isArray(parents)) {
-              parents = [parents];
-            }
+            var parents = forge.util.isArray(parent) ? parent.slice() : [parent];
             var verified = false;
             while (!verified && parents.length > 0) {
               parent = parents.shift();
@@ -167583,7 +167793,7 @@ var require_x509 = __commonJS({
           }
         }
         if (error3 === null && (!first || chain.length === 0 && (!parent || selfSigned))) {
-          var bcExt = cert.getExtension("basicConstraints");
+          var bcExt = cert.getExtension({ id: "2.5.29.19" });
           var keyUsageExt = cert.getExtension("keyUsage");
           if (keyUsageExt !== null) {
             if (!keyUsageExt.keyCertSign || bcExt === null) {
@@ -167605,7 +167815,7 @@ var require_x509 = __commonJS({
               error: pki2.certificateError.bad_certificate
             };
           }
-          if (error3 === null && keyUsageExt !== null && "pathLenConstraint" in bcExt) {
+          if (error3 === null && bcExt !== null && "pathLenConstraint" in bcExt) {
             var pathLen = depth - 1;
             if (pathLen > bcExt.pathLenConstraint) {
               error3 = {
@@ -167989,6 +168199,9 @@ var require_pkcs12 = __commonJS({
         }
         var macSalt = new forge.util.ByteBuffer(capture.macSalt);
         var macIterations = "macIterations" in capture ? parseInt(forge.util.bytesToHex(capture.macIterations), 16) : 1;
+        if (!(macIterations >= 1)) {
+          throw new Error("PKCS#12 MAC iteration count must be >= 1.");
+        }
         var macKey = p12.generateKey(
           password,
           macSalt,
@@ -169250,7 +169463,7 @@ var require_tls = __commonJS({
           }
         });
       }
-      if (c.serverCertificate === null) {
+      if (c.session.serverCertificate === null) {
         var error3 = {
           message: "No server certificate provided. Not enough security.",
           send: true,
@@ -169274,8 +169487,8 @@ var require_tls = __commonJS({
               error3.alert.description = ret;
             }
           }
-          return c.error(c, error3);
         }
+        return c.error(c, error3);
       }
       if (c.session.certificateRequest !== null) {
         record = tls.createRecord(c, {
@@ -169531,7 +169744,7 @@ var require_tls = __commonJS({
       var length = b.getInt16();
       var payload = b.getBytes(length);
       if (type === tls.HeartbeatMessageType.heartbeat_request) {
-        if (c.handshaking || length > payload.length) {
+        if (c.handshakes === 0 || c.handshaking || length > payload.length) {
           return c.process();
         }
         tls.queue(c, tls.createRecord(c, {
@@ -170446,6 +170659,7 @@ var require_tls = __commonJS({
                     description: tls.Alert.Description.unexpected_message
                   }
                 });
+                return rval;
               }
             }
             c2.record.ready = true;
@@ -170510,24 +170724,48 @@ var require_tls = __commonJS({
           tls.flush(c);
         }
       };
+      var _processing = false;
+      var _processNext = false;
+      var _processRecord = function(c2) {
+        var rval = 0;
+        if (!c2.fail) {
+          if (c2.record !== null && c2.record.ready && c2.record.fragment.isEmpty()) {
+            c2.record = null;
+          }
+          if (c2.record === null) {
+            rval = _readRecordHeader(c2);
+          }
+          if (!c2.fail && c2.record !== null && !c2.record.ready) {
+            rval = _readRecord(c2);
+          }
+          if (!c2.fail && c2.record !== null && c2.record.ready) {
+            _update(c2, c2.record);
+          }
+        }
+        return rval;
+      };
       c.process = function(data) {
         var rval = 0;
         if (data) {
           c.input.putBytes(data);
         }
-        if (!c.fail) {
-          if (c.record !== null && c.record.ready && c.record.fragment.isEmpty()) {
-            c.record = null;
-          }
-          if (c.record === null) {
-            rval = _readRecordHeader(c);
-          }
-          if (!c.fail && c.record !== null && !c.record.ready) {
-            rval = _readRecord(c);
-          }
-          if (!c.fail && c.record !== null && c.record.ready) {
-            _update(c, c.record);
+        if (_processing && typeof data === "undefined") {
+          _processNext = true;
+          return rval;
+        }
+        var processing = _processing;
+        var processNext = _processNext;
+        _processing = true;
+        _processNext = false;
+        try {
+          rval = _processRecord(c);
+          while (_processNext) {
+            _processNext = false;
+            _processRecord(c);
           }
+        } finally {
+          _processing = processing;
+          _processNext = processNext;
         }
         return rval;
       };
@@ -177993,7 +178231,7 @@ function getDiffRangesJsonFilePath(env = getEnv()) {
   return path2.join(getTemporaryDirectory(env), PR_DIFF_RANGE_JSON_FILENAME);
 }
 function getActionVersion() {
-  return "4.39.2";
+  return "4.39.3";
 }
 function getWorkflowEventName(env = getEnv()) {
   return env.getRequired("GITHUB_EVENT_NAME" /* GITHUB_EVENT_NAME */);
@@ -188538,11 +188776,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 decodedLocationUri = locationUri;
+          try {
+            decodedLocationUri = 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 === decodedLocationUri || range2.path === locationUri) && (range2.startLine <= locationStartLine && range2.endLine >= locationStartLine || range2.startLine === 0 && range2.endLine === 0)
           );
         });
       });

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "codeql",
-  "version": "4.39.2",
+  "version": "4.39.3",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "codeql",
-      "version": "4.39.2",
+      "version": "4.39.3",
       "license": "MIT",
       "workspaces": [
         "pr-checks"
@@ -33,7 +33,7 @@
         "js-yaml": "^5.5.0",
         "jsonschema": "1.5.0",
         "long": "^5.3.2",
-        "node-forge": "^1.4.0",
+        "node-forge": "^1.4.1",
         "semver": "^7.8.5",
         "undici": "^7.30.1",
         "uuid": "^14.0.3"
@@ -8198,9 +8198,9 @@
       }
     },
     "node_modules/node-forge": {
-      "version": "1.4.0",
-      "resolved": "https://registry.npmjs.org/node-forge/-/node-forge-1.4.0.tgz",
-      "integrity": "sha512-LarFH0+6VfriEhqMMcLX2F7SwSXeWwnEAJEsYm5QKWchiVYVvJyV9v7UDvUv+w5HO23ZpQTXDv/GxdDdMyOuoQ==",
+      "version": "1.4.1",
+      "resolved": "https://registry.npmjs.org/node-forge/-/node-forge-1.4.1.tgz",
+      "integrity": "sha512-w5/MfWaeMrpR/5ba6fZyAUTLSuE6aYz7H/3cYrqwZEmf5eQS3IaaLgJt4MBv7HglGFWmV22K8h4U18RJQmw/ow==",
       "license": "(BSD-3-Clause OR GPL-2.0)",
       "engines": {
         "node": ">= 6.13.0"

package.json

@@ -1,6 +1,6 @@
 {
   "name": "codeql",
-  "version": "4.39.2",
+  "version": "4.39.3",
   "private": true,
   "description": "CodeQL action",
   "scripts": {
@@ -42,7 +42,7 @@
     "js-yaml": "^5.5.0",
     "jsonschema": "1.5.0",
     "long": "^5.3.2",
-    "node-forge": "^1.4.0",
+    "node-forge": "^1.4.1",
     "semver": "^7.8.5",
     "undici": "^7.30.1",
     "uuid": "^14.0.3"

pr-checks/bundle-changelog.test.ts

@@ -10,17 +10,13 @@ import { afterEach, beforeEach, describe, it } from "node:test";
 
 import {
   CLI_VERSION_ENV_VAR,
+  getChangenote,
   getCLIVersion,
   getPRNumber,
   getPRUrl,
   PR_URL_ENV_VAR,
-  updateChangelog,
 } from "./bundle-changelog";
-import {
-  EMPTY_CHANGELOG,
-  NO_CHANGES_STR,
-  UNRELEASED_PLACEHOLDER,
-} from "./changelog";
+import { isValidChangenoteContent } from "./changelog/validate";
 
 let testDir: string;
 
@@ -85,58 +81,15 @@ describe("getPRNumber", async () => {
   });
 });
 
-const testChangelog = `${EMPTY_CHANGELOG.trimEnd()}
-
-## 4.23.7
-
-- Other change
-
-## 4.23.6
-
-${NO_CHANGES_STR}`;
-
-const expectedChangelog = `# CodeQL Action Changelog
-
-## ${UNRELEASED_PLACEHOLDER}
-
-- Update default CodeQL bundle version to
-
-## 4.23.7
-
-- Other change
-
-## 4.23.6
-
-${NO_CHANGES_STR}`;
-
-describe("updateChangelog", async () => {
-  await it("removes `NO_CHANGES_STR` if present in [UNRELEASED] section", async () => {
-    const result = updateChangelog(EMPTY_CHANGELOG, "");
-    assert.ok(!result.includes(NO_CHANGES_STR));
-  });
-
-  await it("doesn't remove `NO_CHANGES_STR` if present in versioned section", async () => {
-    const result = updateChangelog(
-      EMPTY_CHANGELOG.replace(UNRELEASED_PLACEHOLDER, "1.2.3"),
-      "",
+describe("getChangenote", async () => {
+  await it("returns a note linking to the bundle release and the PR", async () => {
+    assert.equal(
+      getChangenote("1.2.3", testPrUrl),
+      "- Update default CodeQL bundle version to [1.2.3](https://github.com/github/codeql-action/releases/tag/codeql-bundle-v1.2.3). [#42](https://github.com/github/codeql-action/pulls/42)\n",
     );
-    assert.ok(result.includes(NO_CHANGES_STR));
   });
 
-  await it("throws if there are no sections", async () => {
-    assert.throws(() => {
-      updateChangelog(
-        "# CodeQL Action Changelog",
-        "- Update default CodeQL bundle version to",
-      );
-    });
-  });
-
-  await it("adds note at the end of the first section", async () => {
-    const result = updateChangelog(
-      testChangelog,
-      "- Update default CodeQL bundle version to",
-    );
-    assert.deepEqual(result, expectedChangelog);
+  await it("returns valid changenote content", async () => {
+    assert.ok(isValidChangenoteContent(getChangenote("1.2.3", testPrUrl)));
   });
 });

pr-checks/bundle-changelog.ts

@@ -1,17 +1,13 @@
 #!/usr/bin/env npx tsx
 
 /**
- * Updates the changelog with a change note for an updated CodeQL CLI bundle.
+ * Creates a changenote file for an update to the default CodeQL CLI bundle.
  */
 
 import * as fs from "node:fs";
+import * as path from "node:path";
 
-import {
-  parseChangelog,
-  renderChangelog,
-  UNRELEASED_PLACEHOLDER,
-} from "./changelog";
-import { CHANGELOG_FILE, CLI_BUNDLE_RELEASE_URL_PREFIX } from "./config";
+import { CHANGENOTES_DIR, CLI_BUNDLE_RELEASE_URL_PREFIX } from "./config";
 import { getErrorMessage } from "./util";
 
 export const CLI_VERSION_ENV_VAR = "CLI_VERSION";
@@ -58,42 +54,19 @@ export function getPRNumber(prUrl: string) {
 }
 
 /**
- * Updates `changelog` by adding `changelogNote` to the first section.
+ * Returns the contents of the changenote file for an update to the default
+ * CodeQL CLI bundle.
  *
- * @param contents The existing changelog contents.
- * @param changelogNote The note to add to the first section.
+ * @param cliVersion The CodeQL CLI version of the new default bundle.
+ * @param prUrl The URL of the PR that updates the default bundle.
+ * @throws If the PR number can't be determined from `prUrl`.
  */
-export function updateChangelog(contents: string, changelogNote: string) {
-  // If the "[UNRELEASED]" section starts with "no user facing changes", remove that line.
-  contents = contents.replace(
-    `## ${UNRELEASED_PLACEHOLDER}\n\nNo user facing changes.`,
-    `## ${UNRELEASED_PLACEHOLDER}\n`,
-  );
+export function getChangenote(cliVersion: string, prUrl: string): string {
+  // The GitHub Release for the new bundle version.
+  const bundleReleaseUrl = `${CLI_BUNDLE_RELEASE_URL_PREFIX}${cliVersion}`;
 
-  const changelog = parseChangelog(contents);
-
-  if (changelog.sections.length === 0) {
-    throw new Error("The changelog contains no existing sections.");
-  }
-
-  // Add the changelog note to the bottom of the first section.
-  const firstSection = changelog.sections[0];
-  const lastLine = firstSection.bodyLines.pop();
-
-  if (lastLine !== undefined && lastLine.trim() !== "") {
-    // We expect the last line to be empty. If it isn't for some reason,
-    // add it back.
-    firstSection.bodyLines.push(lastLine);
-  }
-
-  firstSection.bodyLines.push(changelogNote);
-
-  // If the last line is empty as expected, then add it back in after the new note.
-  if (lastLine?.trim() === "") {
-    firstSection.bodyLines.push(lastLine);
-  }
-
-  return renderChangelog(changelog);
+  const prNumber = getPRNumber(prUrl);
+  return `- Update default CodeQL bundle version to [${cliVersion}](${bundleReleaseUrl}). [#${prNumber}](${prUrl})\n`;
 }
 
 function main() {
@@ -101,22 +74,19 @@ function main() {
     const cliVersion = getCLIVersion();
     const prUrl = getPRUrl();
 
-    // The GitHub Release for the new bundle version.
-    const bundleReleaseUrl = `${CLI_BUNDLE_RELEASE_URL_PREFIX}${cliVersion}`;
-
-    // Get the PR number from the PR URL.
-    const prNumber = getPRNumber(prUrl);
-    const changelogNote = `- Update default CodeQL bundle version to [${cliVersion}](${bundleReleaseUrl}). [#${prNumber}](${prUrl})`;
-
-    let changelog = fs.readFileSync(CHANGELOG_FILE, "utf-8");
-
-    changelog = updateChangelog(changelog, changelogNote);
-
-    fs.writeFileSync(CHANGELOG_FILE, changelog);
+    // Include the CLI version in the file name so that changenotes for
+    // different bundle updates don't conflict.
+    const changenotePath = path.join(
+      CHANGENOTES_DIR,
+      `update-bundle-${cliVersion}.md`,
+    );
+    fs.writeFileSync(changenotePath, getChangenote(cliVersion, prUrl));
 
     return 0;
   } catch (err) {
-    console.error(`Failed to bundle changelog: ${getErrorMessage(err)}`);
+    console.error(
+      `Failed to create changenote for bundle update: ${getErrorMessage(err)}`,
+    );
     return -1;
   }
 }

pr-checks/checks/unsupported-macos.yml

@@ -48,7 +48,7 @@ validationJobs:
     installNode: true
     steps:
       - name: Download workflow artifact
-        uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
+        uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2
         with:
           pattern: test-mode-sarif-*
           path: sarif

pr-checks/package.json

@@ -14,7 +14,7 @@
     "@octokit/core": "^7.0.8",
     "@octokit/plugin-paginate-rest": ">=16.0.0",
     "@octokit/plugin-rest-endpoint-methods": "^18.0.0",
-    "glob": "^13.0.6",
+    "glob": "^14.0.3",
     "lite-matter": "^0.1.2",
     "mdast-util-from-markdown": "^2.1.0",
     "semver": "^7.8.5",

pr-checks/sync-back.test.ts

@@ -56,9 +56,18 @@ jobs:
 
     const result = scanGeneratedWorkflows(workflowDir);
 
-    assert.equal(result["actions/checkout"], "v4");
-    assert.equal(result["actions/setup-node"], "v5");
-    assert.equal(result["actions/setup-go"], "v6");
+    assert.deepEqual(
+      result.actionVersions["actions/checkout"],
+      new Set(["v4"]),
+    );
+    assert.deepEqual(
+      result.actionVersions["actions/setup-node"],
+      new Set(["v5"]),
+    );
+    assert.deepEqual(
+      result.actionVersions["actions/setup-go"],
+      new Set(["v6"]),
+    );
   });
 
   await it("scanning workflows with version comments", () => {
@@ -78,12 +87,18 @@ jobs:
 
     const result = scanGeneratedWorkflows(workflowDir);
 
-    assert.equal(result["actions/checkout"], "v4");
-    assert.equal(
-      result["ruby/setup-ruby"],
-      "44511735964dcb71245e7e55f72539531f7bc0eb # v1.257.0",
+    assert.deepEqual(
+      result.actionVersions["actions/checkout"],
+      new Set(["v4"]),
+    );
+    assert.deepEqual(
+      result.actionVersions["ruby/setup-ruby"],
+      new Set(["44511735964dcb71245e7e55f72539531f7bc0eb # v1.257.0"]),
+    );
+    assert.deepEqual(
+      result.actionVersions["actions/setup-python"],
+      new Set(["v6 # Latest Python"]),
     );
-    assert.equal(result["actions/setup-python"], "v6 # Latest Python");
   });
 
   await it("ignores local actions", () => {
@@ -103,9 +118,15 @@ jobs:
 
     const result = scanGeneratedWorkflows(workflowDir);
 
-    assert.equal(result["actions/checkout"], "v4");
-    assert.equal("./.github/actions/local-action" in result, false);
-    assert.equal("./another-local-action" in result, false);
+    assert.deepEqual(
+      result.actionVersions["actions/checkout"],
+      new Set(["v4"]),
+    );
+    assert.equal(
+      "./.github/actions/local-action" in result.actionVersions,
+      false,
+    );
+    assert.equal("./another-local-action" in result.actionVersions, false);
   });
 });
 
@@ -128,8 +149,8 @@ const steps = [
     fs.writeFileSync(syncTsPath, syncTsContent);
 
     const actionVersions = {
-      "actions/setup-node": "v5",
-      "actions/setup-go": "v6",
+      "actions/setup-node": new Set(["v5"]),
+      "actions/setup-go": new Set(["v6"]),
     };
 
     const result = updateSyncTs(syncTsPath, actionVersions);
@@ -155,7 +176,7 @@ const steps = [
     fs.writeFileSync(syncTsPath, syncTsContent);
 
     const actionVersions = {
-      "actions/setup-node": "v5 # Latest version",
+      "actions/setup-node": new Set(["v5 # Latest version"]),
     };
 
     const result = updateSyncTs(syncTsPath, actionVersions);
@@ -182,7 +203,7 @@ const steps = [
     fs.writeFileSync(syncTsPath, syncTsContent);
 
     const actionVersions = {
-      "actions/setup-node": "v5",
+      "actions/setup-node": new Set(["v5"]),
     };
 
     const result = updateSyncTs(syncTsPath, actionVersions);
@@ -206,7 +227,9 @@ const steps = [
     fs.writeFileSync(syncTsPath, syncTsContent);
 
     const actionVersions = {
-      "actions/setup-node": "48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0",
+      "actions/setup-node": new Set([
+        "48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0",
+      ]),
     };
 
     const result = updateSyncTs(syncTsPath, actionVersions);
@@ -244,8 +267,9 @@ steps:
       "actions/checkout": "v4",
       "actions/setup-node": "v5 # Latest",
     };
+    const byWorkflow = { ["test.yml"]: actionVersions };
 
-    const result = updateTemplateFiles(checksDir, actionVersions);
+    const result = updateTemplateFiles(checksDir, byWorkflow);
     assert.equal(result.length, 1);
     assert.ok(result.includes(templatePath));
 
@@ -269,8 +293,9 @@ steps:
     const actionVersions = {
       "ruby/setup-ruby": "55511735964dcb71245e7e55f72539531f7bc0eb # v1.257.0",
     };
+    const byWorkflow = { ["test.yml"]: actionVersions };
 
-    const result = updateTemplateFiles(checksDir, actionVersions);
+    const result = updateTemplateFiles(checksDir, byWorkflow);
     assert.equal(result.length, 1);
 
     const updatedContent = fs.readFileSync(templatePath, "utf8");

pr-checks/sync-back.ts

@@ -27,16 +27,39 @@ const CHECKS_DIR = path.join(PR_CHECKS_DIR, "checks");
 const WORKFLOW_DIR = path.join(REPO_ROOT, ".github", "workflows");
 const SYNC_TS_PATH = path.join(PR_CHECKS_DIR, "sync.ts");
 
+/** Type alias for action names. */
+type ActionName = string;
+
+/** Type alias for action versions. May include a comment. */
+type ActionVersion = string;
+
+/** Mappings of {@link ActionName}s to {@link ActionVersion}s. */
+type ActionVersions = Record<ActionName, ActionVersion>;
+
+/** Mappings of workflow names to {@link ActionVersions}. */
+type ActionVersionsByWorkflow = Record<string, ActionVersions>;
+
+/** The result of {@link scanGeneratedWorkflows}. */
+type ScanWorkflowResult = {
+  /** A mapping of {@link ActionName}s to all encountered {@link ActionVersion}s. */
+  actionVersions: Record<ActionName, Set<ActionVersion>>;
+
+  /** A mapping of workflow names to {@link ActionVersions}. */
+  actionVersionsByWorkflow: ActionVersionsByWorkflow;
+};
+
 /**
  * Scan generated workflow files to extract the latest action versions.
  *
  * @param workflowDir - Path to .github/workflows directory
- * @returns Map from action names to their latest versions (including comments)
+ *
+ * @returns A {@link ScanWorkflowResult} object containing information about the extracted action versions.
  */
 export function scanGeneratedWorkflows(
   workflowDir: string,
-): Record<string, string> {
-  const actionVersions: Record<string, string> = {};
+): ScanWorkflowResult {
+  const actionVersionsByWorkflow: ActionVersionsByWorkflow = {};
+  const actionVersions: Record<ActionName, Set<ActionVersion>> = {};
 
   const generatedFiles = fs
     .readdirSync(workflowDir)
@@ -46,6 +69,10 @@ export function scanGeneratedWorkflows(
   for (const filePath of generatedFiles) {
     const content = fs.readFileSync(filePath, "utf8");
 
+    // Get the basename of the workflow and drop the leading `__` from it.
+    const workflowName = path.basename(filePath).slice(2);
+    actionVersionsByWorkflow[workflowName] = {};
+
     // Find all action uses in the file, including potential comments
     // This pattern captures: action_name@version_with_possible_comment
     const pattern = /uses:\s+([^/\s]+\/[^@\s]+)@([^@\n]+)/g;
@@ -57,13 +84,26 @@ export function scanGeneratedWorkflows(
 
       // Only track non-local actions (those with / but not starting with ./)
       if (!actionName.startsWith("./")) {
-        // Assume that version numbers are consistent (this should be the case on a Dependabot update PR)
-        actionVersions[actionName] = versionWithComment;
+        // Record the version of the action for this workflow.
+        actionVersionsByWorkflow[workflowName][actionName] = versionWithComment;
+
+        // Record this version for the action so that we can write it to `sync.ts` if needed.
+        // Usually we'd expect there to be exactly one version per action across all workflows,
+        // but sometimes these can go out-of-sync if e.g. a workflow is added in a branch and a
+        // Dependabot PR is merged in the meantime that changes the version of an action that is
+        // used in the new workflow. Different versions of the same action across different
+        // workflows is not fundamentally a problem, but we may need to pick one for `sync.ts`
+        // so we warn there if we have multiple versions of an action.
+        if (actionVersions[actionName] === undefined) {
+          actionVersions[actionName] = new Set([versionWithComment]);
+        } else if (!actionVersions[actionName].has(versionWithComment)) {
+          actionVersions[actionName].add(versionWithComment);
+        }
       }
     }
   }
 
-  return actionVersions;
+  return { actionVersions, actionVersionsByWorkflow };
 }
 
 /**
@@ -79,7 +119,7 @@ export function scanGeneratedWorkflows(
  */
 export function updateSyncTs(
   syncTsPath: string,
-  actionVersions: Record<string, string>,
+  actionVersions: Record<ActionName, Set<ActionVersion>>,
 ): boolean {
   if (!fs.existsSync(syncTsPath)) {
     throw new Error(`Could not find ${syncTsPath}`);
@@ -89,9 +129,18 @@ export function updateSyncTs(
   const originalContent = content;
 
   // Update hardcoded action versions
-  for (const [actionName, versionWithComment] of Object.entries(
+  for (const [actionName, versionsWithComments] of Object.entries(
     actionVersions,
   )) {
+    // We usually expect `versionsWithComments` to contain one version. If there are multiple,
+    // we arbitrarily pick one and warn about it.
+    const versionsWithCommentsArray = Array.from(versionsWithComments);
+    const versionWithComment = versionsWithCommentsArray[0];
+    if (versionsWithComments.size > 1) {
+      console.warn(
+        `Inconsistent versions for ${actionName}. Arbitrarily picking '${versionWithComment}'. All found: ${versionsWithCommentsArray.join(", ")}`,
+      );
+    }
     // Split the scanned value into the ref (e.g. a commit SHA) and the optional
     // trailing version comment (e.g. `v6.0.3`).
     const ref = versionWithComment.includes("#")
@@ -138,12 +187,12 @@ export function updateSyncTs(
  * Update action versions in template files in pr-checks/checks/
  *
  * @param checksDir - Path to pr-checks/checks directory
- * @param actionVersions - Map of action names to versions (may include comments)
+ * @param actionVersionsByWorkflow - Map of workflows to maps of action names to versions (may include comments)
  * @returns List of files that were modified
  */
 export function updateTemplateFiles(
   checksDir: string,
-  actionVersions: Record<string, string>,
+  actionVersionsByWorkflow: ActionVersionsByWorkflow,
 ): string[] {
   const modifiedFiles: string[] = [];
 
@@ -156,6 +205,16 @@ export function updateTemplateFiles(
     let content = fs.readFileSync(filePath, "utf8");
     const originalContent = content;
 
+    const workflowName = path.basename(filePath);
+    const actionVersions = actionVersionsByWorkflow[workflowName];
+
+    if (actionVersions === undefined) {
+      console.info(
+        `Found no action versions for workflow template '${filePath}'.`,
+      );
+      continue;
+    }
+
     // Update action versions
     for (const [actionName, versionWithComment] of Object.entries(
       actionVersions,
@@ -191,12 +250,15 @@ function main(): number {
   const verbose = values.verbose ?? false;
 
   console.info("Scanning generated workflows for latest action versions...");
-  const actionVersions = scanGeneratedWorkflows(WORKFLOW_DIR);
+  const { actionVersions, actionVersionsByWorkflow } =
+    scanGeneratedWorkflows(WORKFLOW_DIR);
 
   if (verbose) {
     console.info("Found action versions:");
-    for (const [action, version] of Object.entries(actionVersions)) {
-      console.info(`  ${action}@${version}`);
+    for (const [action, versions] of Object.entries(actionVersions)) {
+      for (const version of versions) {
+        console.info(`  ${action}@${version}`);
+      }
     }
   }
 
@@ -215,7 +277,10 @@ function main(): number {
   }
 
   // Update template files
-  const templateModified = updateTemplateFiles(CHECKS_DIR, actionVersions);
+  const templateModified = updateTemplateFiles(
+    CHECKS_DIR,
+    actionVersionsByWorkflow,
+  );
   modifiedFiles.push(...templateModified);
 
   if (modifiedFiles.length > 0) {

src/upload-lib.test.ts

@@ -13,7 +13,11 @@ import * as diffUtils from "./diff-informed-analysis-utils";
 import { getEnv } from "./environment";
 import { Logger } from "./logging";
 import * as sarif from "./sarif";
-import { RecordingLogger, setupTests } from "./testing-utils";
+import {
+  checkExpectedLogMessages,
+  RecordingLogger,
+  setupTests,
+} from "./testing-utils";
 import * as uploadLib from "./upload-lib";
 import { UploadPayload } from "./upload-lib/types";
 import { GitHubVariant, withTmpDir } from "./util";
@@ -1015,12 +1019,62 @@ for (const analysisKind of analyses.supportedAnalysisKinds) {
   );
 }
 
+function createMockSarifWithResults(
+  results: sarif.Result[],
+): Partial<sarif.Log> {
+  return {
+    version: "2.1.0",
+    runs: [
+      {
+        tool: {
+          driver: {
+            name: "test tool",
+          },
+        },
+        results,
+      },
+    ],
+  };
+}
+
+function createMockResultWithUri(
+  uri: string,
+  startLine = 2,
+  relatedUri?: string,
+): sarif.Result {
+  return {
+    ruleId: "test-rule",
+    message: { text: "test message" },
+    locations: [
+      {
+        physicalLocation: {
+          artifactLocation: { uri },
+          region: { startLine },
+        },
+      },
+    ],
+    ...(relatedUri
+      ? {
+          relatedLocations: [
+            {
+              physicalLocation: {
+                artifactLocation: { uri: relatedUri },
+                region: { startLine },
+              },
+            },
+          ],
+        }
+      : {}),
+  };
+}
+
 function runFilterAlertsByDiffRange(
   input: Partial<sarif.Log>,
   diffRanges: diffUtils.DiffThunkRange[],
+  logger: Logger = new RecordingLogger(),
 ): Partial<sarif.Log> {
   sinon.stub(diffUtils, "readDiffRangesJsonFile").returns(diffRanges);
-  return uploadLib.filterAlertsByDiffRange(new RecordingLogger(), input);
+  return uploadLib.filterAlertsByDiffRange(logger, input);
 }
 
 test.serial(
@@ -1044,3 +1098,139 @@ test.serial(
     t.deepEqual(actualOutput, expectedOutput);
   },
 );
+
+for (const [filePath, uri] of [
+  ["with space/board.js", "with%20space/board.js"],
+  ["src/[id]/board.js", "src/%5Bid%5D/board.js"],
+  ["src/file#1.js", "src/file%231.js"],
+  ["src/100%.js", "src/100%25.js"],
+  ["src/literal%20.js", "src/literal%2520.js"],
+  ["src/foo:bar.js", "src/foo%3Abar.js"],
+  ["src/caf\u00e9.js", "src/caf%C3%A9.js"],
+  ["src/caf\u00e9.js", "src/caf%c3%a9.js"],
+]) {
+  test.serial(
+    `filterAlertsByDiffRange matches percent-encoded URI ${uri}`,
+    (t) => {
+      const matchingResult = createMockResultWithUri(uri);
+      const nonMatchingResult = createMockResultWithUri("other.js");
+      const input = createMockSarifWithResults([
+        matchingResult,
+        nonMatchingResult,
+      ]);
+
+      const actualOutput = runFilterAlertsByDiffRange(input, [
+        { path: filePath, startLine: 1, endLine: 3 },
+      ]);
+
+      t.deepEqual(actualOutput.runs?.[0].results, [matchingResult]);
+    },
+  );
+}
+
+test.serial(
+  "filterAlertsByDiffRange filters out percent-encoded URI when outside diff line range",
+  (t) => {
+    const outOfRangeResult = createMockResultWithUri(
+      "with%20space/board.js",
+      2,
+    );
+    const inRangeResult = createMockResultWithUri("with%20space/board.js", 15);
+    const input = createMockSarifWithResults([outOfRangeResult, inRangeResult]);
+    const actualOutput = runFilterAlertsByDiffRange(input, [
+      { path: "with space/board.js", startLine: 10, endLine: 20 },
+    ]);
+
+    t.deepEqual(actualOutput.runs?.[0].results, [inRangeResult]);
+  },
+);
+
+test.serial(
+  "filterAlertsByDiffRange filters out percent-encoded URI when path does not match",
+  (t) => {
+    const input = createMockSarifWithResults([
+      createMockResultWithUri("with%20space/board.js"),
+    ]);
+    const actualOutput = runFilterAlertsByDiffRange(input, [
+      { path: "other/board.js", startLine: 1, endLine: 3 },
+    ]);
+
+    t.deepEqual(actualOutput.runs?.[0].results, []);
+  },
+);
+
+test.serial(
+  "filterAlertsByDiffRange keeps alert when relatedLocations has a matching percent-encoded URI",
+  (t) => {
+    const matchingRelatedResult = createMockResultWithUri(
+      "src/promiseUtils.js",
+      2,
+      "with%20space/ParseObject.js",
+    );
+    const nonMatchingResult = createMockResultWithUri("other.js");
+    const input = createMockSarifWithResults([
+      matchingRelatedResult,
+      nonMatchingResult,
+    ]);
+
+    const actualOutput = runFilterAlertsByDiffRange(input, [
+      { path: "with space/ParseObject.js", startLine: 1, endLine: 3 },
+    ]);
+
+    t.deepEqual(actualOutput.runs?.[0].results, [matchingRelatedResult]);
+  },
+);
+
+for (const uri of ["dir%ZZ/board.js", "src/bad%.js", "src/bad%C3%28.js"]) {
+  test.serial(
+    `filterAlertsByDiffRange falls back to raw path for malformed URI ${uri}`,
+    (t) => {
+      const logger = new RecordingLogger();
+      const matchingResult = createMockResultWithUri(uri);
+      const input = createMockSarifWithResults([matchingResult]);
+      const actualOutput = runFilterAlertsByDiffRange(
+        input,
+        [{ path: uri, startLine: 1, endLine: 3 }],
+        logger,
+      );
+
+      t.deepEqual(actualOutput.runs?.[0].results, [matchingResult]);
+      checkExpectedLogMessages(t, logger.messages, [
+        `Failed to decode location URI "${uri}", comparing it as a raw path`,
+      ]);
+    },
+  );
+}
+
+test.serial(
+  "filterAlertsByDiffRange filters out malformed URI when raw path does not match",
+  (t) => {
+    const logger = new RecordingLogger();
+    const input = createMockSarifWithResults([
+      createMockResultWithUri("dir%ZZ/board.js"),
+    ]);
+    const actualOutput = runFilterAlertsByDiffRange(
+      input,
+      [{ path: "src/different.js", startLine: 1, endLine: 3 }],
+      logger,
+    );
+
+    t.deepEqual(actualOutput.runs?.[0].results, []);
+    checkExpectedLogMessages(t, logger.messages, [
+      'Failed to decode location URI "dir%ZZ/board.js", comparing it as a raw path',
+    ]);
+  },
+);
+
+test.serial(
+  "filterAlertsByDiffRange matches raw unencoded path containing valid percent-encoding sequence",
+  (t) => {
+    const matchingResult = createMockResultWithUri("dir/100%20done.js");
+    const input = createMockSarifWithResults([matchingResult]);
+    const actualOutput = runFilterAlertsByDiffRange(input, [
+      { path: "dir/100%20done.js", startLine: 1, endLine: 3 },
+    ]);
+
+    t.deepEqual(actualOutput.runs?.[0].results, [matchingResult]);
+  },
+);

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 decodedLocationUri = locationUri;
+          try {
+            decodedLocationUri = 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,8 @@ export function filterAlertsByDiffRange(
           // of an alert location.
           return diffRanges.some(
             (range) =>
-              range.path === locationUri &&
+              (range.path === decodedLocationUri ||
+                range.path === locationUri) &&
               ((range.startLine <= locationStartLine &&
                 range.endLine >= locationStartLine) ||
                 (range.startLine === 0 && range.endLine === 0)),

unreleased-change-notes/decode-diff-informed-sarif-uris.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. [#4283](https://github.com/github/codeql-action/pull/4283)