diff --git a/.github/scripts/pr-carry-attribution.cjs b/.github/scripts/pr-carry-attribution.cjs index 9e0ad1cabc..2886449102 100644 --- a/.github/scripts/pr-carry-attribution.cjs +++ b/.github/scripts/pr-carry-attribution.cjs @@ -56,7 +56,23 @@ const TRAILER_RE = /^[ \t]*co-authored-by:[ \t]*(.+)$/gim; const FENCED_CODE_RE = /^[ \t]*(\u0060{3,}|~{3,})[\s\S]*?^[ \t]*\1[ \t]*$/gm; const INLINE_CODE_RE = /\u0060[^\u0060\n]*\u0060/g; -const HTML_COMMENT_RE = //g; +/** + * HTML comments, which GitHub never renders. + * + * The `(?:-->|$)` alternative is load-bearing and matches `pr-quality.cjs`: an + * UNCLOSED comment runs to the end of the text, because that is what GitHub + * does with it. Without the alternative, `|$)/g; /** * Carry language inside a fenced block, an inline span, or an HTML comment is diff --git a/.github/scripts/pr-carry-attribution.test.cjs b/.github/scripts/pr-carry-attribution.test.cjs index 187469071e..08010a18d2 100644 --- a/.github/scripts/pr-carry-attribution.test.cjs +++ b/.github/scripts/pr-carry-attribution.test.cjs @@ -120,6 +120,35 @@ describe("assessCarryAttribution", () => { ); }); + it("ignores carry language after an unclosed HTML comment", () => { + // GitHub renders nothing after an unterminated ``, and a real claim after it + // is still a claim. + assert.equal( + assessCarryAttribution( + base({ + body: ["", "", "Supersedes #2797."].join("\n"), + }), + ).length, + 1, + ); + }); + it("passes an ordinary pull request with no carry language", () => { assert.deepEqual( assessCarryAttribution(base({ body: "Closes #2797." })), diff --git a/bun.lock b/bun.lock index 329084661e..c4619c2866 100644 --- a/bun.lock +++ b/bun.lock @@ -22,9 +22,10 @@ ], "overrides": { "@hono/node-server": "2.1.0", - "fast-uri": "^3.1.5", + "fast-uri": "^3.1.7", "hono": "4.13.1", "ip-address": "^10.4.0", + "qs": "^6.16.0", }, "packages": { "@bufbuild/protobuf": ["@bufbuild/protobuf@2.14.0", "", {}, "sha512-C3UGsiCwSprE2NKIIFA3hCDlpXTMCAXRZuEVp88L1GY36Y41+rYL5fryE+nOFhp4p4JPQvdV8PQ4DWgHgeTE+w=="], @@ -187,7 +188,7 @@ "fast-deep-equal": ["fast-deep-equal@3.1.3", "", {}, "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q=="], - "fast-uri": ["fast-uri@3.1.5", "", {}, "sha512-gHwA1O9LDIcKunMKhObS/HimwtehO1nPUECKAu5TpKgaO19fcWEl4bliWe1jWxVFvIXztJjjQ4L8XQ1EU9f7Jw=="], + "fast-uri": ["fast-uri@3.1.7", "", {}, "sha512-dOvZVzjdZdz7phd9v6jCbwxrBW3fK6n8Rc0CtdmM4bumzMnxywBYhuph6J819RRw/ku+rLbelwfMunktuzVVHg=="], "finalhandler": ["finalhandler@2.1.1", "", { "dependencies": { "debug": "^4.4.0", "encodeurl": "^2.0.0", "escape-html": "^1.0.3", "on-finished": "^2.4.1", "parseurl": "^1.3.3", "statuses": "^2.0.1" } }, "sha512-S8KoZgRZN+a5rNwqTxlZZePjT/4cnm0ROV70LedRHZ0p8u9fRID0hJUZQpkKLzro8LfmC8sx23bY6tVNxv8pQA=="], @@ -261,7 +262,7 @@ "proxy-addr": ["proxy-addr@2.0.7", "", { "dependencies": { "forwarded": "0.2.0", "ipaddr.js": "1.9.1" } }, "sha512-llQsMLSUDUPT44jdrU/O37qlnifitDP+ZwrmmZcoSKyLKvtZxpyV0n2/bD/N4tBAAZ/gJEdZU7KMraoK1+XYAg=="], - "qs": ["qs@6.15.3", "", { "dependencies": { "es-define-property": "^1.0.1", "side-channel": "^1.1.1" } }, "sha512-O9gl3zCl5h5blw1KGUzQKhA5oUXSl8rwUIM5o0S3nCXMliSvy5Dzx7/DJcI+SwgICv+IneSZwhBh1oSyEHA71A=="], + "qs": ["qs@6.16.0", "", { "dependencies": { "es-define-property": "^1.0.1", "side-channel": "^1.1.1" } }, "sha512-h6fhOIaRrID2CbEY2fqs+7t+UXZo+MLAnU5gRIq85uFtdiUPCdsApMlHhXogKVM4HM2DVbIjGNTTYH2OcmP1vA=="], "range-parser": ["range-parser@1.3.0", "", {}, "sha512-hek2mFQpPuI4E1BBKrSto+BU3e3x4xuarsbiwr3+lf7p44juvFMV0XFWQAP3xUyqXA4RrXLIoaSUGbSt056ZMw=="], diff --git a/package.json b/package.json index 78ad559e54..57f5f97d46 100644 --- a/package.json +++ b/package.json @@ -74,9 +74,10 @@ }, "overrides": { "@hono/node-server": "2.1.0", - "fast-uri": "^3.1.5", + "fast-uri": "^3.1.7", "hono": "4.13.1", - "ip-address": "^10.4.0" + "ip-address": "^10.4.0", + "qs": "^6.16.0" }, "keywords": [ "codex", diff --git a/tests/strict-semver.test.ts b/tests/strict-semver.test.ts index 934dabc048..e47d511f2b 100644 --- a/tests/strict-semver.test.ts +++ b/tests/strict-semver.test.ts @@ -10,35 +10,63 @@ import { parseStrictSemver } from "../src/lib/strict-semver"; * * The length ceiling did not help. It only chose where on the curve the input landed. */ +/** + * Every timing assertion here measures the BEST of several runs, not a single one. + * + * A first call carries one-time cost the parse itself does not: regex compilation, JIT + * warm-up, and whatever the shared CI runner was doing during that millisecond. On a + * loaded macOS runner that noise reached 53.77ms against a 50ms budget and failed a + * suite whose subject is three orders of magnitude away from the regression it guards + * (522ms). A gate that fires on runner weather rather than on the defect teaches + * everyone to re-run it, which is how a real ReDoS regression would get waved through. + * + * The minimum is the right statistic for this question. Superlinear backtracking is a + * property of the pattern, so it reproduces on EVERY iteration; scheduler noise does + * not. If the exponential path returns, no run is fast. + * + * That claim was measured rather than assumed. Running the semver.org prerelease + * pattern this module replaced against the same inputs, three runs each: + * + * reps=20 len=68 17.6ms 17.4ms 17.4ms + * reps=30 len=98 545.4ms 521.2ms 500.0ms + * reps=39 len=125 492.3ms 493.5ms 491.3ms + * reps=45 len=128 495.3ms 507.9ms 527.8ms + * + * The blowup is on every run, not the first, so a best-of-N below 50ms still fails + * loudly if it comes back. The spread across runs is under 10%, which is what a + * deterministic cost looks like next to the 4ms of scheduler jitter that broke the + * single-sample form. + */ +function fastestParseMs(input: string, runs = 5): number { + let best = Infinity; + for (let i = 0; i < runs; i++) { + const started = performance.now(); + parseStrictSemver(input); + const elapsed = performance.now() - started; + if (elapsed < best) best = elapsed; + } + return best; +} + describe("parseStrictSemver ReDoS resistance", () => { test("the flagged attack shape stays linear at the length ceiling", () => { // "0.0.0-0." followed by repetitions of "--." is the input CodeQL named. const attack = ("0.0.0-0." + "--.".repeat(45)).slice(0, 128); expect(attack.length).toBe(128); - const started = performance.now(); expect(parseStrictSemver(attack)).toBeNull(); - const elapsed = performance.now() - started; // The vulnerable pattern took ~522ms for this input. Anything in that region means the // superlinear path is back; a linear parse lands three orders of magnitude below it. - expect(elapsed).toBeLessThan(50); + expect(fastestParseMs(attack)).toBeLessThan(50); }); test("cost does not grow with the number of repetitions", () => { - const measure = (reps: number): number => { - const input = ("0.0.0-0." + "--.".repeat(reps)).slice(0, 128); - const started = performance.now(); - parseStrictSemver(input); - return performance.now() - started; - }; + const inputFor = (reps: number): string => ("0.0.0-0." + "--.".repeat(reps)).slice(0, 128); // Under the old pattern, going from 20 to 39 repetitions moved 16ms to 524ms. - measure(20); - const short = measure(20); - const long = measure(39); - expect(short).toBeLessThan(50); - expect(long).toBeLessThan(50); + expect(fastestParseMs(inputFor(20))).toBeLessThan(50); + expect(fastestParseMs(inputFor(39))).toBeLessThan(50); }); test("the length guard still rejects before any matching work", () => {