fix(core): close-tag regex misses script/style end tags with attrs/whitespace

HomenShum · Sun-sunshine06 · commit 8ae1a4ddda31 · 2026-06-06T18:52:40.000+08:00
CodeQL js/bad-tag-filter (HIGH) on PR #241: the literal `<\/script>` / `<\/style>` patterns in stripTags() left bodies behind for HTML5-tolerated end-tag forms like `</script >` (trailing space) and `</script foo="bar">` (end-tag attributes, silently ignored by browsers per spec). A crafted source HTML could leak script/style body text into the visible-word vocabulary used for parity coverage scoring. Fix: mirror the opening-tag pattern's `\b[^>]*` on the close tag too. The `\b` after the tag name prevents over-matching `</scripts>` while the `[^>]*` consumes any tolerated end-tag content up to the closing `>`. Regression test covers all 4 previously-vulnerable forms: - `</script >` (trailing whitespace) - `</script foo="bar">` (end-tag attrs) - `</style >` (style branch) - `</SCRIPT>` (case) Asserts none of 4 leaked tokens appear in the parity report when the decomposition correctly omits the script/style content. Signed-off-by: homen <hshum2018@gmail.com>
diff --git a/packages/core/src/tools/verify-ui-kit-parity.test.ts b/packages/core/src/tools/verify-ui-kit-parity.test.ts
@@ -154,6 +154,56 @@ describe('makeVerifyUiKitParityTool', () => {
     expect(result.details.signals.visibleTextCoverage).toBe(1);
   });
 
+  // Regression for CodeQL js/bad-tag-filter (HIGH) flagged on PR #241.
+  // The pre-fix regex `<\/script>` literally missed end tags with trailing
+  // whitespace or attributes, both of which HTML5 parsers tolerate. A crafted
+  // source could leak script body text into the visible-word coverage check.
+  // This test asserts that a script body using the previously-vulnerable
+  // syntax does NOT contribute words to the source vocabulary, so a faithful
+  // decomposition that omits the script still scores OK.
+  it('strips script + style bodies even with attrs/whitespace in end tags', async () => {
+    const sourceWithCraftedScript = `
+      <html>
+        <body>
+          <header><h1>Acme Analytics</h1></header>
+          <main>
+            <div class="metric"><span>MRR</span><span>$12,400</span></div>
+          </main>
+          <script>secretLeakedTokenAaa()</script >
+          <script>anotherLeakedTokenBbb()</script foo="bar">
+          <style>.x{color:#f00}/*secretLeakedCssCcc*/</style >
+          <SCRIPT>upperCaseLeakedTokenDdd()</SCRIPT>
+        </body>
+      </html>
+    `;
+    const cleanDecomp = `
+      <html>
+        <body>
+          <header><h1>Acme Analytics</h1></header>
+          <main>
+            <div class="metric"><span>MRR</span><span>$12,400</span></div>
+          </main>
+        </body>
+      </html>
+    `;
+    const fs = makeFs({
+      'index.html': sourceWithCraftedScript,
+      'ui_kits/x/index.html': cleanDecomp,
+      'ui_kits/x/tokens.css': '',
+    });
+    const tool = makeVerifyUiKitParityTool(fs);
+    const result = await tool.execute('t', { slug: 'x' }, undefined);
+    const first = result.content[0];
+    if (first?.type !== 'text') throw new Error('expected text');
+    // None of the leaked tokens should appear in the parity report — if they
+    // did, it would mean stripTags failed to remove the script/style bodies
+    // and they leaked into the visible-word vocabulary.
+    expect(first.text.toLowerCase()).not.toContain('secretleakedtoken');
+    expect(first.text.toLowerCase()).not.toContain('anotherleakedtoken');
+    expect(first.text.toLowerCase()).not.toContain('secretleakedcss');
+    expect(first.text.toLowerCase()).not.toContain('uppercaseleakedtoken');
+  });
+
   it('summary text reflects pass/fail status', async () => {
     const fsOk = makeFs({
       'index.html': SOURCE_HTML,
diff --git a/packages/core/src/tools/verify-ui-kit-parity.ts b/packages/core/src/tools/verify-ui-kit-parity.ts
@@ -127,9 +127,15 @@ function elementParityScore(
 }
 
 function stripTags(html: string): string {
+  // Close-tag patterns mirror the opening pattern's `\b[^>]*` so we strip
+  // the full `<script>...</script foo="bar" >` form. HTML5 parsers tolerate
+  // attributes and trailing whitespace inside end tags (silently ignored)
+  // and CodeQL's "Bad HTML filtering regexp" rule flags the literal
+  // `</script>` form because it leaves bodies behind for `</script >` etc.
+  // The `\b` after the tag name prevents over-matching like `</scripts>`.
   return html
-    .replace(/<script\b[^>]*>[\s\S]*?<\/script>/gi, ' ')
-    .replace(/<style\b[^>]*>[\s\S]*?<\/style>/gi, ' ')
+    .replace(/<script\b[^>]*>[\s\S]*?<\/script\b[^>]*>/gi, ' ')
+    .replace(/<style\b[^>]*>[\s\S]*?<\/style\b[^>]*>/gi, ' ')
     .replace(/<[^>]+>/g, ' ');
 }
 
