From ac0fa9ec282c7a43ac99cc44e2da92cf4b1e5f90 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 13:08:52 +0000 Subject: [PATCH 1/8] test(review-depth): style-body character references and HTML comment ends keep an SVG code (GST-940, failing) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- test/review-planner-misreads.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/review-planner-misreads.test.ts b/test/review-planner-misreads.test.ts index 0c34036a0..40be1d1ae 100644 --- a/test/review-planner-misreads.test.ts +++ b/test/review-planner-misreads.test.ts @@ -125,7 +125,8 @@ describe('GST-911 misread 1: a chart SVG is data, an SVG that can run code is se // Value: protects=only an SVG made of drawing elements is data: anything that loads, links or declares outside that set stays code; // fails_when=an external image or stylesheet, an entity declaration, an external DTD, an animation or a CSS url() to another document reads as docs, // including through a CSS escape or a zero-padded character reference (review pass 1 findings) or behind a stray quote that - // pairs with a later one (review pass 2 finding), or any CSS beyond declarations and url(#id) (Step 11 finding: image-set()); + // pairs with a later one (review pass 2 finding), or any CSS beyond declarations and url(#id) (Step 11 finding: image-set()), + // including CSS behind a character-reference or comment, or an HTML-only comment end (GST-940); // seam=collectCommit + depthOfCommitInput test.each([ ['an external image', ' '], @@ -153,6 +154,11 @@ describe('GST-911 misread 1: a chart SVG is data, an SVG that can run code is se ['a style attribute cross-fade()', ' '], ['a style element whose hides in a comment', ' -->image-set("https://x.test/p.png" 1x)}'], ['a style element whose hides in CDATA', ' */ svg{fill:image-set("https://x.test/p.png" 1x)} ]]>'], + ['a character-reference in a style element', ' '], + ['a hex character-reference in a style element', ' '], + ['a character-reference comment in a style element', ' '], + ['CSS after an HTML empty comment in a style element', ' '], + ['CSS after an HTML --!> comment end in a style element', ' '], ])('%s keeps the SVG code', (_name, line) => { const fx = fixture(); expect(plan(fx, fx.commit({ 'results/chart.svg': chart(5, line) })).class).toBe('code'); From 11222c2673df625cecce437cbc8a4c5a6936b097 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 13:08:52 +0000 Subject: [PATCH 2/8] fix(review-depth): walk SVG style bodies as written and read their comments as CSS (GST-940) No-loss replay GST-940: 62,805 unique SVG files give the same verdict as main at 4bbad2422, and the planner over the last 100 first-parent main commits changes no depth, class or risk class. Repins lib/review-depth.ts on GST-940. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gst-940-style-charref.md | 3 +++ docs/review-policy.md | 2 +- lib/review-depth.ts | 29 ++++++++++++++-------------- test/review-packets-removed.test.ts | 3 ++- 4 files changed, 20 insertions(+), 17 deletions(-) create mode 100644 changelog.d/gst-940-style-charref.md diff --git a/changelog.d/gst-940-style-charref.md b/changelog.d/gst-940-style-charref.md new file mode 100644 index 000000000..2e543b25e --- /dev/null +++ b/changelog.d/gst-940-style-charref.md @@ -0,0 +1,3 @@ +### Fixed + +- **A character reference or an HTML comment end can no longer hide CSS in an SVG chart (GST-940).** `</style>` or `<!--` inside a `` nor a comment that only an HTML parser ends early can hide CSS (GST-940); anything else, a script or `foreignObject` included, keeps it code. More than 2,000 data paths keep every one code; a `git grep` failure is `consumer_scan_failed` and fails the depth closed. Only `classifyDiff` applies the rule (`lib/review-collect.ts` diff --git a/lib/review-depth.ts b/lib/review-depth.ts index 59340ac8e..b59862771 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -98,9 +98,10 @@ 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, 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. 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. + * stops at the next =, which keeps the scan linear. A style element's CSS is walked with indexOf to its closing tag over the raw + * text, so a character reference cannot fake a closing tag or a comment, and any & in it refuses (GST-940). Comments and CDATA are + * read as CSS, so neither can hide a closing tag and an HTML-only comment end ( or --!>) exposes nothing unread; 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); @@ -108,29 +109,27 @@ export function svgDrawing(text: string): boolean { const cssDrawing = (css: string) => !SVG_CSS_REFUSED.test(css.replace(SVG_CSS_REF, '')); for (let at = 0; ;) { SVG_STYLE_OPEN.lastIndex = at; - const open = SVG_STYLE_OPEN.exec(t); + const open = SVG_STYLE_OPEN.exec(text); if (!open) break; - let i = t.indexOf('>', open.index) + 1; + let i = text.indexOf('>', open.index) + 1; if (!i) return false; let css = ''; for (;;) { - const lt = t.indexOf('<', i); + const lt = text.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); + css += text.slice(i, lt); + const [start, close] = text.startsWith(''] : text.startsWith(''] : [-1, '']; + if (start !== -1) { + const end = text.indexOf(close, start); if (end === -1) return false; - css += t.slice(lt + 9, end); + css += ' ' + text.slice(start, end) + ' '; i = end + 3; - } else if (SVG_STYLE_CLOSE.test(t.slice(lt, lt + 256))) { + } else if (SVG_STYLE_CLOSE.test(text.slice(lt, lt + 256))) { at = lt + 1; break; } else return false; } - if (!cssDrawing(css)) return false; + if (css.includes('&') || !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; diff --git a/test/review-packets-removed.test.ts b/test/review-packets-removed.test.ts index 1ae42964e..ee49107c4 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -61,6 +61,7 @@ const NO_LOSS_REPLAYS = { 'GST-911': "GST-911 no-loss replay (GSTA-249/13): the planner over the last 100 first-parent main commits to f5cb6ec1e, origin/main's code against this branch, gate scan fixed at 0, changes no commit's depth, class, risk classes, targeted classes or SCOPE_AUTH; the three misreads (a drawing-only SVG, a session-named data file, a bare base) are proven on fixtures (test/review-planner-misreads.test.ts), where every SVG that can run script stays code with the security class; reviewer text is unchanged", 'R51': "R51 risk-tier routing no-loss re-score (GSTA-167): the saved panel findings (349 records over 24 of the 26 no-loss units) re-scored against the branch's plan lose no blocker whose only finders are roles it drops, and every gate, security, user-data and money fixture keeps main's reviewers plus the full panel (test/review-targeted.test.ts, test/review-risk-routing.test.ts); reviewer text is unchanged", 'GST-898': "GST-898 verdict-identity no-loss check: the linear rm rule, and the rmSync rule (which also raises security on any call with more than 500 characters before its closing parenthesis), give the same verdict as R51's on every changed line of the last 100 main commits (69,023 lines, a563f47 back 100 first-parent commits) and on 60,000 seeded lines per rule (test/review-risk-redos.test.ts), and no commit changes depth, class or risk class, so every diff dispatches main's reviewers unchanged; reviewer text is unchanged", + 'GST-940': "GST-940 verdict no-loss check: svgDrawing walks style bodies over the raw text, refuses any & in them and reads comments as CSS; on 62,805 unique SVG files (21,658 with a style element) it gives the same verdict as main at 4bbad2422 on every file, and the planner over the last 100 first-parent main commits to 4bbad2422 changes no commit's depth, class or risk classes; the only verdicts it changes are the five fail-open cases in test/review-planner-misreads.test.ts", } as const; type Replay = keyof typeof NO_LOSS_REPLAYS; const REVIEWER_BRIEFS: Record = { @@ -82,7 +83,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: '166dd7b1148a272b345dd9d2ceccd14348cc59c52ef9c88109436f6f61cf6b69', replay: 'GST-911-redos' }, + 'lib/review-depth.ts': { sha: 'e3b0be6299d408f881a0679672362463791dd3f4ba3cb325208f9b8eb0058720', replay: 'GST-940' }, 'lib/review-risk.ts': { sha: '126eff0f1de6982e03dc24d8017c67e6807c91dfa518b18d2db3aa910aac911f', replay: 'GST-911' }, }; From 8ad07db1278d8921ac8f5e87c02818a63fdb7474 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 13:20:20 +0000 Subject: [PATCH 3/8] test(review-depth): a quoted > on the style tag keeps an SVG code; plain style comments stay a drawing (GST-940, failing) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- test/review-planner-misreads.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/review-planner-misreads.test.ts b/test/review-planner-misreads.test.ts index 40be1d1ae..42059f822 100644 --- a/test/review-planner-misreads.test.ts +++ b/test/review-planner-misreads.test.ts @@ -159,11 +159,19 @@ describe('GST-911 misread 1: a chart SVG is data, an SVG that can run code is se ['a character-reference comment in a style element', ' '], ['CSS after an HTML empty comment in a style element', ' '], ['CSS after an HTML --!> comment end in a style element', ' '], + ['a inside a quoted style attribute', ' '], + ['an escaped ampersand in a style element', ' '], ])('%s keeps the SVG code', (_name, line) => { const fx = fixture(); expect(plan(fx, fx.commit({ 'results/chart.svg': chart(5, line) })).class).toBe('code'); }, 60_000); + // Value: protects=the GST-940 style walker still reads a plain chart style as a drawing: a comment, CDATA and a quoted attribute + // without markup; fails_when=the walker over-refuses them, so chart SVGs lose the light review; seam=svgDrawing + test('a style comment, CDATA and quoted attribute keep a chart a drawing (GST-940)', () => { + expect(svgDrawing(chart(5, ' '))).toBe(true); + }); + // Value: protects=a passive edit to an SVG that already carries a script is still code, because the whole file decides, not the // changed lines; and an SVG code names stays code like any consumed data file; fails_when=either plans as docs; seam=collectCommit test('an edit inside a scripted SVG, and an SVG code names, stay code', () => { From 2c8d13960e7204a9c91a221b60137adea8499259 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 13:20:20 +0000 Subject: [PATCH 4/8] fix(review-depth): read the SVG style tag quote by quote (GST-940 red-team#1) No-loss replay GST-940 rerun: 62,805 unique SVG files keep main's verdict and the planner over 100 main commits changes nothing. Repins lib/review-depth.ts on GST-940. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gst-940-style-charref.md | 2 +- docs/review-policy.md | 2 +- lib/review-depth.ts | 11 +++++++---- test/review-packets-removed.test.ts | 4 ++-- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/changelog.d/gst-940-style-charref.md b/changelog.d/gst-940-style-charref.md index 2e543b25e..3683a9ae1 100644 --- a/changelog.d/gst-940-style-charref.md +++ b/changelog.d/gst-940-style-charref.md @@ -1,3 +1,3 @@ ### Fixed -- **A character reference or an HTML comment end can no longer hide CSS in an SVG chart (GST-940).** `</style>` or `<!--` inside a `` nor a comment that only an HTML parser ends early can hide CSS (GST-940); anything else, a +entity declaration, and no DOCTYPE but a bare `` or W3C's own SVG DTD, read after character references are decoded. A ``, comment that only an HTML parser ends early, or quoted `>` can hide CSS (GST-940); anything else, a script or `foreignObject` included, keeps it code. More than 2,000 data paths keep every one code; a `git grep` failure is `consumer_scan_failed` and fails the depth closed. Only `classifyDiff` applies the rule (`lib/review-collect.ts` diff --git a/lib/review-depth.ts b/lib/review-depth.ts index b59862771..a99689a59 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -89,6 +89,8 @@ export const SVG_REFUSED = [/&(?!(?:amp|lt|gt|quot|apos);)/, /\\/, /@import/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_OPEN = /<(?:[A-Za-z_][\w.-]*:)?style\b/gi; const SVG_STYLE_CLOSE = /^<\/(?:[A-Za-z_][\w.-]*:)?style\s*>/i; +/** A style element's opening tag, read quote by quote: an attribute value holding < or > refuses, so it cannot hide a closing tag. */ +const SVG_STYLE_TAG = /^<(?:[A-Za-z_][\w.-]*:)?style(?:\s+[\w.:-]+\s*=\s*(?:"[^"<>]*"|'[^'<>]*'))*\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. */ @@ -98,7 +100,7 @@ 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, 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. A style element's CSS is walked with indexOf to its closing tag over the raw + * stops at the next =, which keeps the scan linear. A style element's opening tag is read quote by quote (SVG_STYLE_TAG), and its CSS is walked with indexOf to its closing tag over the raw * text, so a character reference cannot fake a closing tag or a comment, and any & in it refuses (GST-940). Comments and CDATA are * read as CSS, so neither can hide a closing tag and an HTML-only comment end ( or --!>) exposes nothing unread; any other * markup inside it, or a style element that never closes, refuses. Anything else, an SVG that can run code included, stays code. @@ -111,8 +113,9 @@ export function svgDrawing(text: string): boolean { SVG_STYLE_OPEN.lastIndex = at; const open = SVG_STYLE_OPEN.exec(text); if (!open) break; - let i = text.indexOf('>', open.index) + 1; - if (!i) return false; + const tag = SVG_STYLE_TAG.exec(text.slice(open.index, open.index + 4096)); + if (!tag) return false; + let i = open.index + tag[0].length; let css = ''; for (;;) { const lt = text.indexOf('<', i); @@ -147,7 +150,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_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}`)]), + 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_STYLE_TAG: [SVG_STYLE_TAG], 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 ee49107c4..d1871e7ef 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -61,7 +61,7 @@ const NO_LOSS_REPLAYS = { 'GST-911': "GST-911 no-loss replay (GSTA-249/13): the planner over the last 100 first-parent main commits to f5cb6ec1e, origin/main's code against this branch, gate scan fixed at 0, changes no commit's depth, class, risk classes, targeted classes or SCOPE_AUTH; the three misreads (a drawing-only SVG, a session-named data file, a bare base) are proven on fixtures (test/review-planner-misreads.test.ts), where every SVG that can run script stays code with the security class; reviewer text is unchanged", 'R51': "R51 risk-tier routing no-loss re-score (GSTA-167): the saved panel findings (349 records over 24 of the 26 no-loss units) re-scored against the branch's plan lose no blocker whose only finders are roles it drops, and every gate, security, user-data and money fixture keeps main's reviewers plus the full panel (test/review-targeted.test.ts, test/review-risk-routing.test.ts); reviewer text is unchanged", 'GST-898': "GST-898 verdict-identity no-loss check: the linear rm rule, and the rmSync rule (which also raises security on any call with more than 500 characters before its closing parenthesis), give the same verdict as R51's on every changed line of the last 100 main commits (69,023 lines, a563f47 back 100 first-parent commits) and on 60,000 seeded lines per rule (test/review-risk-redos.test.ts), and no commit changes depth, class or risk class, so every diff dispatches main's reviewers unchanged; reviewer text is unchanged", - 'GST-940': "GST-940 verdict no-loss check: svgDrawing walks style bodies over the raw text, refuses any & in them and reads comments as CSS; on 62,805 unique SVG files (21,658 with a style element) it gives the same verdict as main at 4bbad2422 on every file, and the planner over the last 100 first-parent main commits to 4bbad2422 changes no commit's depth, class or risk classes; the only verdicts it changes are the five fail-open cases in test/review-planner-misreads.test.ts", + 'GST-940': "GST-940 verdict no-loss check: svgDrawing reads a style element's opening tag quote by quote, walks its body over the raw text, refuses any & in it and reads comments as CSS; on 62,805 unique SVG files (21,658 with a style element) it gives the same verdict as main at 4bbad2422 on every file, and the planner over the last 100 first-parent main commits to 4bbad2422 changes no commit's depth, class or risk classes; the only verdicts it changes are the GST-940 cases in test/review-planner-misreads.test.ts", } as const; type Replay = keyof typeof NO_LOSS_REPLAYS; const REVIEWER_BRIEFS: Record = { @@ -83,7 +83,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: 'e3b0be6299d408f881a0679672362463791dd3f4ba3cb325208f9b8eb0058720', replay: 'GST-940' }, + 'lib/review-depth.ts': { sha: 'bd99c178da5fa1352ed4012a8e26c1f57ccef9179ec24d6bd178ed7ca10217cc', replay: 'GST-940' }, 'lib/review-risk.ts': { sha: '126eff0f1de6982e03dc24d8017c67e6807c91dfa518b18d2db3aa910aac911f', replay: 'GST-911' }, }; From 078e2bdaa941d5559e8ad29191ce0f3c44312f8d Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 13:20:31 +0000 Subject: [PATCH 5/8] docs(changelog): GST-940 lead names the three shapes Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gst-940-style-charref.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/gst-940-style-charref.md b/changelog.d/gst-940-style-charref.md index 3683a9ae1..fb3a46cc6 100644 --- a/changelog.d/gst-940-style-charref.md +++ b/changelog.d/gst-940-style-charref.md @@ -1,3 +1,3 @@ ### Fixed -- **A character reference or an HTML comment end can no longer hide CSS in an SVG chart (GST-940).** `</style>` or `<!--` inside a ` and non-ASCII name prefixes keep an SVG code (GST-940 fix-check, failing) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- test/review-planner-misreads.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/review-planner-misreads.test.ts b/test/review-planner-misreads.test.ts index 42059f822..2bbaea298 100644 --- a/test/review-planner-misreads.test.ts +++ b/test/review-planner-misreads.test.ts @@ -126,7 +126,8 @@ describe('GST-911 misread 1: a chart SVG is data, an SVG that can run code is se // fails_when=an external image or stylesheet, an entity declaration, an external DTD, an animation or a CSS url() to another document reads as docs, // including through a CSS escape or a zero-padded character reference (review pass 1 findings) or behind a stray quote that // pairs with a later one (review pass 2 finding), or any CSS beyond declarations and url(#id) (Step 11 finding: image-set()), - // including CSS behind a character-reference or comment, or an HTML-only comment end (GST-940); + // including CSS behind a character-reference or comment, an HTML-only comment end, a quoted > on the style tag, a + // non-ASCII space before > in , or a tag or attribute under a non-ASCII namespace prefix (GST-940); // seam=collectCommit + depthOfCommitInput test.each([ ['an external image', ' '], @@ -161,6 +162,11 @@ describe('GST-911 misread 1: a chart SVG is data, an SVG that can run code is se ['CSS after an HTML --!> comment end in a style element', ' '], ['a inside a quoted style attribute', ' '], ['an escaped ampersand in a style element', ' '], + ['a no-break space before > in ', ' '], + ['a vertical tab before > in ', ' '], + ['a script under a non-ASCII prefix', ' fetch(1)'], + ['a style under a non-ASCII prefix', ' rect{fill:image-set("https://x.test/p.png" 1x)}'], + ['an external href under a non-ASCII prefix', ' '], ])('%s keeps the SVG code', (_name, line) => { const fx = fixture(); expect(plan(fx, fx.commit({ 'results/chart.svg': chart(5, line) })).class).toBe('code'); From ee3908a18fb9fa0fb693ba35f231c6539075b1cf Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 13:33:04 +0000 Subject: [PATCH 7/8] fix(review-depth): ASCII whitespace in SVG style tags, refuse non-ASCII element and attribute names (GST-940 fix-check) Fixes round-2 red-team#1 and targeted#1 under the Captain's scoped fix-check grant. No-loss replay GST-940 rerun: 62,805 unique SVG files keep main's verdict; the planner over 100 main commits changes nothing. Repins lib/review-depth.ts on GST-940. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gst-940-style-charref.md | 2 +- docs/review-policy.md | 2 +- lib/review-depth.ts | 23 ++++++++++++++++------- test/review-packets-removed.test.ts | 4 ++-- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/changelog.d/gst-940-style-charref.md b/changelog.d/gst-940-style-charref.md index fb3a46cc6..79927bc62 100644 --- a/changelog.d/gst-940-style-charref.md +++ b/changelog.d/gst-940-style-charref.md @@ -1,3 +1,3 @@ ### Fixed -- **Three markup tricks no longer hide CSS in an SVG chart (GST-940).** `</style>` or `<!--` inside a `` let CSS that loads a remote image read as a plain drawing, and a non-ASCII namespace prefix such as `` hid a script. Any of these could get the SVG a light review. The style walker now reads the ``, comment that only an HTML parser ends early, or quoted `>` can hide CSS (GST-940); anything else, a +entity declaration, and no DOCTYPE but a bare `` or W3C's own SVG DTD, read after character references are decoded. A ``, comment that only an HTML parser ends early, quoted `>` or Unicode space can hide CSS. A non-ASCII character in an element or attribute name, such as a `gé:` prefix, keeps the SVG code too (GST-940); anything else, a script or `foreignObject` included, keeps it code. More than 2,000 data paths keep every one code; a `git grep` failure is `consumer_scan_failed` and fails the depth closed. Only `classifyDiff` applies the rule (`lib/review-collect.ts` diff --git a/lib/review-depth.ts b/lib/review-depth.ts index a99689a59..f656c0731 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -88,9 +88,16 @@ const SVG_MARKUP = /^<(?:!--|!\[CDATA\[|\?xml\s|!DOCTYPE\s+svg(?:\s+PUBLIC\s+([" export 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_OPEN = /<(?:[A-Za-z_][\w.-]*:)?style\b/gi; -const SVG_STYLE_CLOSE = /^<\/(?:[A-Za-z_][\w.-]*:)?style\s*>/i; -/** A style element's opening tag, read quote by quote: an attribute value holding < or > refuses, so it cannot hide a closing tag. */ -const SVG_STYLE_TAG = /^<(?:[A-Za-z_][\w.-]*:)?style(?:\s+[\w.:-]+\s*=\s*(?:"[^"<>]*"|'[^'<>]*'))*\s*>/i; +const SVG_STYLE_CLOSE = /^<\/(?:[A-Za-z_][\w.-]*:)?style[ \t\n\r]*>/i; +/** + * A style element's opening tag, read as quoted name="value" pairs separated by the ASCII whitespace XML and HTML share: any + * other shape (a value holding < or >, an unquoted or valueless attribute, />, another space) refuses, so it cannot hide a closing tag. + */ +const SVG_STYLE_TAG = /^<(?:[A-Za-z_][\w.-]*:)?style(?:[ \t\n\r]+[\w.:-]+[ \t\n\r]*=[ \t\n\r]*(?:"[^"<>]*"|'[^'<>]*'))*[ \t\n\r]*>/i; +/** An element name, ended by ASCII whitespace, / or >: a non-ASCII character in the name (a prefix such as gé:) refuses rather than reading as its ASCII start. */ +const SVG_TAG_NAME = /^<\/?(?:[A-Za-z_][\w.-]*:)?([A-Za-z_][\w.-]*)(?=[ \t\n\r/>]|$)/; +/** Every name= in the text, non-ASCII letters included so a prefix such as xé:href is read whole and refused. */ +const SVG_ATTR = /(?=]+)))/g; 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. */ @@ -99,7 +106,8 @@ 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, 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 + * decoded. Every name= in the text is read (SVG_ATTR), so a stray quote cannot hide a later attribute inside its value, and a name + * holding a non-ASCII character refuses, like an element name (SVG_TAG_NAME); an unquoted value * stops at the next =, which keeps the scan linear. A style element's opening tag is read quote by quote (SVG_STYLE_TAG), and its CSS is walked with indexOf to its closing tag over the raw * text, so a character reference cannot fake a closing tag or a comment, and any & in it refuses (GST-940). Comments and CDATA are * read as CSS, so neither can hide a closing tag and an HTML-only comment end ( or --!>) exposes nothing unread; any other @@ -136,10 +144,11 @@ export function svgDrawing(text: string): boolean { } 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)); + const tag = SVG_TAG_NAME.exec(t.slice(at, at + 256)); if (!tag || !SVG_ELEMENTS.has(tag[1]!.toLowerCase())) return false; } - for (const m of t.matchAll(/(?=]+)))/g)) { + for (const m of t.matchAll(SVG_ATTR)) { + if (/[^\x00-\x7f]/.test(m[1]!)) return false; const local = m[1]!.slice(m[1]!.lastIndexOf(':') + 1).toLowerCase(); const value = m[2] ?? m[3] ?? m[4] ?? ''; if (local.startsWith('on') || ((local === 'href' || local === 'src') && !SVG_HREF.test(value)) || (local === 'style' && !cssDrawing(value))) return false; @@ -150,7 +159,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_OPEN: [SVG_STYLE_OPEN], SVG_STYLE_CLOSE: [SVG_STYLE_CLOSE], SVG_STYLE_TAG: [SVG_STYLE_TAG], 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_STYLE_TAG: [SVG_STYLE_TAG], SVG_TAG_NAME: [SVG_TAG_NAME], SVG_ATTR: [SVG_ATTR], 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 d1871e7ef..2cadcc601 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -61,7 +61,7 @@ const NO_LOSS_REPLAYS = { 'GST-911': "GST-911 no-loss replay (GSTA-249/13): the planner over the last 100 first-parent main commits to f5cb6ec1e, origin/main's code against this branch, gate scan fixed at 0, changes no commit's depth, class, risk classes, targeted classes or SCOPE_AUTH; the three misreads (a drawing-only SVG, a session-named data file, a bare base) are proven on fixtures (test/review-planner-misreads.test.ts), where every SVG that can run script stays code with the security class; reviewer text is unchanged", 'R51': "R51 risk-tier routing no-loss re-score (GSTA-167): the saved panel findings (349 records over 24 of the 26 no-loss units) re-scored against the branch's plan lose no blocker whose only finders are roles it drops, and every gate, security, user-data and money fixture keeps main's reviewers plus the full panel (test/review-targeted.test.ts, test/review-risk-routing.test.ts); reviewer text is unchanged", 'GST-898': "GST-898 verdict-identity no-loss check: the linear rm rule, and the rmSync rule (which also raises security on any call with more than 500 characters before its closing parenthesis), give the same verdict as R51's on every changed line of the last 100 main commits (69,023 lines, a563f47 back 100 first-parent commits) and on 60,000 seeded lines per rule (test/review-risk-redos.test.ts), and no commit changes depth, class or risk class, so every diff dispatches main's reviewers unchanged; reviewer text is unchanged", - 'GST-940': "GST-940 verdict no-loss check: svgDrawing reads a style element's opening tag quote by quote, walks its body over the raw text, refuses any & in it and reads comments as CSS; on 62,805 unique SVG files (21,658 with a style element) it gives the same verdict as main at 4bbad2422 on every file, and the planner over the last 100 first-parent main commits to 4bbad2422 changes no commit's depth, class or risk classes; the only verdicts it changes are the GST-940 cases in test/review-planner-misreads.test.ts", + 'GST-940': "GST-940 verdict no-loss check: svgDrawing reads a style element's opening tag as quoted pairs with ASCII whitespace, walks its body over the raw text, refuses any & in it, reads comments as CSS and refuses a non-ASCII element or attribute name; on 62,805 unique SVG files (21,658 with a style element) it gives the same verdict as main at 4bbad2422 on every file, and the planner over the last 100 first-parent main commits to 4bbad2422 changes no commit's depth, class or risk classes; the only verdicts it changes are the GST-940 cases in test/review-planner-misreads.test.ts", } as const; type Replay = keyof typeof NO_LOSS_REPLAYS; const REVIEWER_BRIEFS: Record = { @@ -83,7 +83,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: 'bd99c178da5fa1352ed4012a8e26c1f57ccef9179ec24d6bd178ed7ca10217cc', replay: 'GST-940' }, + 'lib/review-depth.ts': { sha: '8b0671cfc0248e3ad9c1d20aef54f948d328f32441a5366af6a2d310de464333', replay: 'GST-940' }, 'lib/review-risk.ts': { sha: '126eff0f1de6982e03dc24d8017c67e6807c91dfa518b18d2db3aa910aac911f', replay: 'GST-911' }, }; From 8fdb64438baab591b0cee76e01fa8691448e518a Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 18:57:12 +0000 Subject: [PATCH 8/8] test(review-packets-removed): repin lib/review-depth.ts on GST-940 after the main merge Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- test/review-packets-removed.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/review-packets-removed.test.ts b/test/review-packets-removed.test.ts index 55af4fd68..ba902ea01 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -84,7 +84,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: '/bin/bash: -c: line 1: unexpected EOF while looking for matching `"'', replay: 'GST-940' }, + 'lib/review-depth.ts': { sha: '8c50527413edad1c9c14d4130a14b3ce1f95c3f0229b056f26d759b267b2428e', replay: 'GST-940' }, 'lib/review-risk.ts': { sha: '126eff0f1de6982e03dc24d8017c67e6807c91dfa518b18d2db3aa910aac911f', replay: 'GST-911' }, };