diff --git a/.changeset/gate-absorb-entity-decode.md b/.changeset/gate-absorb-entity-decode.md new file mode 100644 index 00000000..e900bccd --- /dev/null +++ b/.changeset/gate-absorb-entity-decode.md @@ -0,0 +1,5 @@ +--- +"review": patch +--- + +The dispatch gate's rule 7 no longer false-blocks a review whose plan quotes angle brackets. Run 32758584548 (Khan/actions#371) staged a footer quoting `<STOP: ...>` (the renderer escapes angle brackets it quotes), gh-aw's ingest sanitizer decoded the entities and convertXmlTags parenthesised the result on the queued side, and the plan-vs-queue comparison went red on a fully conforming run, withholding the whole review. HTML entity decoding was a documented-not-absorbed residual of `sanitizer-normalize.ts`; it is now absorbed the way the `` placeholder shape was: `normalizeBody` mirrors the sanitizer's decodeHtmlEntities (named, decimal, hex, one level of `&` double-encoding, gh-aw v0.85.4) on both comparison sides, sequenced before the invisible-character strips exactly as hardenUnicodeText sequences it, so an entity-vs-literal splice that changes text still fails. diff --git a/workflows/review/lib/sanitizer-normalize.test.ts b/workflows/review/lib/sanitizer-normalize.test.ts index 587ef6f9..e1529c4c 100644 --- a/workflows/review/lib/sanitizer-normalize.test.ts +++ b/workflows/review/lib/sanitizer-normalize.test.ts @@ -131,3 +131,43 @@ describe("normalizeBody URL folds (run 31616001094 incident shapes)", () => { expect(normalizeBody(plan)).not.toBe(normalizeBody(spliced)); }); }); + +describe("normalizeBody HTML entity decoding (run 32758584548 incident shape)", () => { + it("matches a plan-side entity-escaped quote against its decoded queued form", () => { + // The incident: the renderer footer staged `<STOP: ...>`, the + // sanitizer decoded it and convertXmlTags parenthesised the result, + // and rule 7 blocked a fully conforming review. + const plan = "adds a `<STOP: run the merge>` pseudo-tag line"; + const queued = "adds a `(STOP: run the merge)` pseudo-tag line"; + expect(normalizeBody(plan)).toBe(normalizeBody(queued)); + }); + + it("decodes decimal, hex, and double-encoded forms like the sanitizer", () => { + expect(normalizeBody("at @user and @org")).toBe( + normalizeBody("at @user and @org"), + ); + expect(normalizeBody("a &gt; b &amp; c")).toBe( + normalizeBody("a > b & c"), + ); + }); + + it("folds entity-spelled invisible characters to nothing, matching the strip", () => { + // ­ decodes to U+00AD, which the very next fold deletes; the + // ordering mirrors hardenUnicodeText (decode step 2, strip step 3). + expect(normalizeBody("so­ft hy‌phen")).toBe( + normalizeBody("soft hyphen"), + ); + }); + + it("leaves an out-of-range numeric entity alone, like the sanitizer", () => { + expect(normalizeBody("bad � stays")).toBe( + normalizeBody("bad � stays"), + ); + }); + + it("still catches an entity-vs-literal splice that changes the text", () => { + expect(normalizeBody("size < 10")).not.toBe( + normalizeBody("size 100"), + ); + }); +}); diff --git a/workflows/review/lib/sanitizer-normalize.ts b/workflows/review/lib/sanitizer-normalize.ts index d4d40d17..a05d932b 100644 --- a/workflows/review/lib/sanitizer-normalize.ts +++ b/workflows/review/lib/sanitizer-normalize.ts @@ -12,9 +12,14 @@ * anything the sanitizer would not have done is still a mismatch (#244). * * Documented-not-absorbed residuals, each of which needs a pathological body - * and fails red rather than silently: HTML entity decoding, the + * and fails red rather than silently: the * percent-decode side effect, homoglyph folds, the 65k truncation, markdown - * link titles, and tilde fences. XML tag conversion is absorbed by + * link titles, and tilde fences. HTML entity decoding graduated out of this + * list the same way the `` placeholder did: run 32758584548 staged a + * footer quoting `<STOP: ...>` (the renderer escapes angle brackets it + * quotes), the sanitizer decoded it and convertXmlTags parenthesised the + * result, and rule 7 blocked a fully conforming review. Absorbed by + * {@link decodeHtmlEntities} below. XML tag conversion is absorbed by * {@link foldXmlTags} below; its remaining sub-residuals (dangerous-attribute * stripping inside a preserved allowed tag, CDATA marker rewriting) still * need a pathological body. @@ -137,6 +142,47 @@ const foldDomainName = (host: string): string => { : joined; }; +/** + * Mirror the sanitizer's decodeHtmlEntities (sanitize_content_core.cjs, + * gh-aw v0.85.4, hardenUnicodeText step 2): named entities for @ and the + * angle-bracket/ampersand trio, the invisible-character names, then decimal + * and hex forms, each tolerating one `&`-double-encoding. Decoding runs + * BEFORE the invisible strips so an entity-spelled zero-width character + * (`­`, `‌`) decodes to the code point the next fold deletes, + * exactly as the sanitizer sequences it. Applied to both comparison sides: + * the plan is composed pre-sanitizer (a renderer that quotes `` + * escapes it to `<STOP: ...>`), the queued side arrives decoded. + */ +const decodeHtmlEntities = (text: string): string => + text + .replace(/&(?:amp;)?commat;/gi, "@") + .replace(/&(?:amp;)?gt;/gi, ">") + .replace(/&(?:amp;)?lt;/gi, "<") + .replace(/&(?:amp;)?amp;/gi, "&") + .replace(/&(?:amp;)?shy;/gi, "\u00ad") + .replace(/&(?:amp;)?zwnj;/gi, "\u200c") + .replace(/&(?:amp;)?zwj;/gi, "\u200d") + .replace(/&(?:amp;)?lrm;/gi, "\u200e") + .replace(/&(?:amp;)?rlm;/gi, "\u200f") + .replace(/&(?:amp;)?ZeroWidthSpace;/gi, "\u200b") + .replace(/&(?:amp;)?NoBreak;/gi, "\u2060") + .replace(/&(?:amp;)?(?:af|ApplyFunction);/gi, "\u2061") + .replace(/&(?:amp;)?(?:it|InvisibleTimes);/gi, "\u2062") + .replace(/&(?:amp;)?(?:ic|InvisibleComma);/gi, "\u2063") + .replace(/&(?:amp;)?(?:ip|InvisiblePlus);/gi, "\u2064") + .replace(/&(?:amp;)?#(\d+);/g, (match, code: string) => { + const codePoint = parseInt(code, 10); + return codePoint >= 0 && codePoint <= 0x10ffff + ? String.fromCodePoint(codePoint) + : match; + }) + .replace(/&(?:amp;)?#[xX]([0-9a-fA-F]+);/g, (match, code: string) => { + const codePoint = parseInt(code, 16); + return codePoint >= 0 && codePoint <= 0x10ffff + ? String.fromCodePoint(codePoint) + : match; + }); + /** * Fold one body to its sanitizer-tolerant comparison form. Applied to the * plan and the queued text alike; never to text that gets posted. @@ -152,8 +198,7 @@ export const normalizeBody = (text: string): string => // hardened text on the queued side (run 31616001094: a stripped // U+034F turned `/\u034f/g` into `//g` before URL redaction saw it). foldXmlTags( - text - .normalize("NFKC") + decodeHtmlEntities(text.normalize("NFKC")) .replace(/\u034f/g, "") // Zero-width, bidi-control (sanitizer step 4), C0/DEL (its // control-strip): all deleted on the queued side only, so