From 26d71ad689dbf9d9e740919f81cb7d2826b99fe9 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 05:27:20 +0000 Subject: [PATCH] fix(review): walk a style element's CSS with indexOf, dropping comments and reading CDATA, so no hidden ends it early and unclosed style tags refuse in linear time (GST-911) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/review-depth.ts | 38 +++++++++++++++++++++++++---- test/review-packets-removed.test.ts | 2 +- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/lib/review-depth.ts b/lib/review-depth.ts index 16ff3a3b2..e1c2b00ac 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -87,7 +87,8 @@ const SVG_MARKUP = /^<(?:!--|!\[CDATA\[|\?xml\s|!DOCTYPE\s+svg\b[^>[]*>)/i; */ const SVG_REFUSED = [/&(?!(?:amp|lt|gt|quot|apos);)/, /\\/, /@import/i, /url\(\s*(?!['"]?\s*#)/i, /expression\s*\(/i, /-moz-binding|\bbehavior\s*:/i]; /** A drawing's CSS, a style element's text or a style attribute's value, holds declarations only: no @ rule and no function but url(#id). */ -const SVG_STYLE = /<(?:[A-Za-z_][\w.-]*:)?style\b[^>]*>([\s\S]*?)(?:<\/(?:[A-Za-z_][\w.-]*:)?style\s*>|$)/gi; +const SVG_STYLE_OPEN = /<(?:[A-Za-z_][\w.-]*:)?style\b/gi; +const SVG_STYLE_CLOSE = /^<\/(?:[A-Za-z_][\w.-]*:)?style\s*>/i; const SVG_CSS_REF = /url\(\s*(['"]?)#[\w.:-]*\1\s*\)/gi; const SVG_CSS_REFUSED = /[@(]/; /** An href or src value a drawing may hold: a same-document reference, or a raster image inline. */ @@ -95,15 +96,42 @@ const SVG_HREF = /^\s*(?:#|data:image\/(?:png|jpe?g|gif|webp);base64,[A-Za-z0-9+ /** * Whether an SVG's whole text is a drawing (GST-911): every element in SVG_ELEMENTS, no event handler, every href or src a - * same-document reference or an inline raster image, CSS of declarations only (SVG_STYLE), and nothing SVG_REFUSED names, read after character references are + * same-document reference or an inline raster image, CSS of declarations only, and nothing SVG_REFUSED names, read after character references are * decoded. Every name= in the text is read, so a stray quote cannot hide a later attribute inside its value; an unquoted value - * stops at the next =, which keeps the scan linear. Anything else, an SVG that can run code included, stays code. + * stops at the next =, which keeps the scan linear. A style element's CSS is walked with indexOf to its closing tag, dropping + * comments and reading CDATA as CSS, so neither can hide a closing tag; any other markup inside it, or a style element that never + * closes, refuses. Anything else, an SVG that can run code included, stays code. */ export function svgDrawing(text: string): boolean { const t = decodeCharRefs(text); if (SVG_REFUSED.some(r => r.test(t)) || svgActive(t)) return false; const cssDrawing = (css: string) => !SVG_CSS_REFUSED.test(css.replace(SVG_CSS_REF, '')); - for (const m of t.matchAll(SVG_STYLE)) if (!cssDrawing(m[1]!)) return false; + for (let at = 0; ;) { + SVG_STYLE_OPEN.lastIndex = at; + const open = SVG_STYLE_OPEN.exec(t); + if (!open) break; + let i = t.indexOf('>', open.index) + 1; + if (!i) return false; + let css = ''; + for (;;) { + const lt = t.indexOf('<', i); + if (lt === -1) return false; + css += t.slice(i, lt); + if (t.startsWith('', lt + 4) + 3; + if (i === 2) return false; + } else if (t.startsWith('', lt + 9); + if (end === -1) return false; + css += t.slice(lt + 9, end); + i = end + 3; + } else if (SVG_STYLE_CLOSE.test(t.slice(lt, lt + 256))) { + at = lt + 1; + break; + } else return false; + } + if (!cssDrawing(css)) return false; + } for (let at = t.indexOf('<'); at !== -1; at = t.indexOf('<', at + 1)) { if (SVG_MARKUP.test(t.slice(at, at + 512))) continue; const tag = /^<\/?(?:[A-Za-z_][\w.-]*:)?([A-Za-z_][\w.-]*)/.exec(t.slice(at, at + 256)); @@ -120,7 +148,7 @@ export function svgDrawing(text: string): boolean { /** A hash of the constants and patterns that decide a depth, not of this file's bytes: a comment edit keeps it, a threshold edit changes it. */ export const DEPTH_RULES = createHash('sha256').update(JSON.stringify({ RISK_RULES, SMALL_DIFF_LINES, RED_TEAM_LINES, LARGE_DIFF_LINES, FULL_PASS_CAP, LIGHT_FIXED, BATCH_SCOPED, SCOPED_RANGE_LINES, ROLE_EFFORT, SVG_ELEMENTS: [...SVG_ELEMENTS], - patterns: Object.entries({ GATE_PATH, AGENT_PROSE, TEST_PATH, RELEASE_METADATA, DOC_PATH: [DOC_PATH], DATA_PATH: [DATA_PATH], DATA_CONFIG, SVG_MARKUP: [SVG_MARKUP], SVG_REFUSED, SVG_HREF: [SVG_HREF], SVG_STYLE: [SVG_STYLE], SVG_CSS_REF: [SVG_CSS_REF], SVG_CSS_REFUSED: [SVG_CSS_REFUSED] }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]), + patterns: Object.entries({ GATE_PATH, AGENT_PROSE, TEST_PATH, RELEASE_METADATA, DOC_PATH: [DOC_PATH], DATA_PATH: [DATA_PATH], DATA_CONFIG, SVG_MARKUP: [SVG_MARKUP], SVG_REFUSED, SVG_HREF: [SVG_HREF], SVG_STYLE_OPEN: [SVG_STYLE_OPEN], SVG_STYLE_CLOSE: [SVG_STYLE_CLOSE], SVG_CSS_REF: [SVG_CSS_REF], SVG_CSS_REFUSED: [SVG_CSS_REFUSED] }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]), })).digest('hex').slice(0, 16); /** diff --git a/test/review-packets-removed.test.ts b/test/review-packets-removed.test.ts index b02be6c3b..e16cb6361 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -79,7 +79,7 @@ const REVIEWER_BRIEFS: Record = { 'review/specialists/testing.md': { sha: 'd6cc2bef7be562387d0c0a22fefe646dff9c295f1d34d8453a6cb496584150e4', replay: '#3638' }, 'review/specialists/targeted.md': { sha: '7f92a0d0a8319c3c0e076fb3b1a4689c45120b2be2c14dfc7d4cb03aafa9d6a5', replay: 'GST-257' }, 'scripts/resolvers/fix-check.ts': { sha: '712bf5b0c439e96bac32f19cbe25394dabb4c94aa66600100bfde9b100e15a08', replay: '#3638' }, - 'lib/review-depth.ts': { sha: '7c4e2673d6d7613a94a493665bd566e6c34958bb0b72374e1552f2223a1da2bd', replay: 'GST-911' }, + 'lib/review-depth.ts': { sha: 'b0c86b19ed13959c1cda8c1d2a2066b3b899473b855f06c0d9680e66660fb698', replay: 'GST-911' }, 'lib/review-risk.ts': { sha: 'df618e8aef3d641e8bf2073d8dca170740434acbb69c04555c4166df047f566c', replay: 'GST-911' }, };