diff --git a/.github/workflows/__unsupported-macos.yml b/.github/workflows/__unsupported-macos.yml index 167473b48..f98bf6959 100644 --- a/.github/workflows/__unsupported-macos.yml +++ b/.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 diff --git a/lib/entry-points.js b/lib/entry-points.js index d6bf3f9f8..b8479554a 100644 --- a/lib/entry-points.js +++ b/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; }; @@ -188537,11 +188775,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) ); }); }); diff --git a/package-lock.json b/package-lock.json index f2c7f0bf4..2e17a0606 100644 --- a/package-lock.json +++ b/package-lock.json @@ -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" @@ -8199,9 +8199,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" diff --git a/package.json b/package.json index 16fc86dd0..2326dba02 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,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" diff --git a/pr-checks/checks/unsupported-macos.yml b/pr-checks/checks/unsupported-macos.yml index 5881b174a..f5388f1d6 100644 --- a/pr-checks/checks/unsupported-macos.yml +++ b/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 diff --git a/pr-checks/package.json b/pr-checks/package.json index 0cfda051c..3ad65382f 100644 --- a/pr-checks/package.json +++ b/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", diff --git a/pr-checks/sync-back.test.ts b/pr-checks/sync-back.test.ts index 52d4c98f3..142487449 100755 --- a/pr-checks/sync-back.test.ts +++ b/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"); diff --git a/pr-checks/sync-back.ts b/pr-checks/sync-back.ts index 233621ea7..702886467 100755 --- a/pr-checks/sync-back.ts +++ b/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; + +/** Mappings of workflow names to {@link ActionVersions}. */ +type ActionVersionsByWorkflow = Record; + +/** The result of {@link scanGeneratedWorkflows}. */ +type ScanWorkflowResult = { + /** A mapping of {@link ActionName}s to all encountered {@link ActionVersion}s. */ + actionVersions: Record>; + + /** 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 { - const actionVersions: Record = {}; +): ScanWorkflowResult { + const actionVersionsByWorkflow: ActionVersionsByWorkflow = {}; + const actionVersions: Record> = {}; 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, + actionVersions: Record>, ): 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, + 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) { diff --git a/src/upload-lib.test.ts b/src/upload-lib.test.ts index 4738c07c6..028cce504 100644 --- a/src/upload-lib.test.ts +++ b/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 { + 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, diffRanges: diffUtils.DiffThunkRange[], + logger: Logger = new RecordingLogger(), ): Partial { 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]); + }, +); diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 4e97c1d1d..52f6fdba6 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 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)), diff --git a/unreleased-change-notes/decode-diff-informed-sarif-uris.md b/unreleased-change-notes/decode-diff-informed-sarif-uris.md new file mode 100644 index 000000000..b233ef437 --- /dev/null +++ b/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)