mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 07:18:03 +01:00
fix(review-depth): linear url() refusal in svgDrawing (wave review-2, red-team CRITICAL)
SVG_REFUSED's url() entry backtracked quadratically on url( followed by a long whitespace run (80k spaces: 820 ms; 1 MB: over 230 s), and svgDrawing runs in collectLive and in collectCommit, which ship-queue ready, train build and land call with no timeout. The rewritten entry refuses exactly what #3838's pattern refused. Repin lib/review-depth.ts under no-loss replay GST-911-redos (verdict identity: 60,000 seeded strings, the edges and every SVG blob in history, zero differences). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
61cd2158a5
commit
b3d0eb8ad8
3 files changed
+32
-2
No files matched your search
+1
-1
@@ -85,7 +85,7 @@ const SVG_MARKUP = /^<(?:!--|!\[CDATA\[|\?xml\s|!DOCTYPE\s+svg\b[^>[]*>)/i;
|
||||
* What a drawing never holds: a named entity beyond the five XML ones, a backslash (a CSS escape such as @\69mport), a CSS
|
||||
* import, a url() to another document, a CSS expression or binding.
|
||||
*/
|
||||
const SVG_REFUSED = [/&(?!(?:amp|lt|gt|quot|apos);)/, /\\/, /@import/i, /url\(\s*(?!['"]?\s*#)/i, /expression\s*\(/i, /-moz-binding|\bbehavior\s*:/i];
|
||||
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;
|
||||
|
||||
@@ -57,6 +57,7 @@ const NO_LOSS_REPLAYS = {
|
||||
'L': "GSTA-167 variant G Arm L: live no-loss replay of one batched reader (testing and security plus scoped checklists) and the Red Team on its findings, 2 passes, scored L∘G1 against G1 on the paired rule (part 2)",
|
||||
'GST-257': "GST-257 PR 1 dispatch-superset no-loss check (test/review-targeted.test.ts): on every fixture diff the branch dispatches main's reviewers at 8be7f21 unchanged plus only the targeted reader, and over the last 100 non-release commits on main it lost none; adds the targeted role and brief to the G1 and L bytes, whose existing reviewer text is unchanged",
|
||||
'GST-850': "GST-850 dispatch-superset no-loss check: on the 26 GSTA-167 no-loss units the branch plan (wider security and money classes, write-up text in the batched diff) drops no reviewer role main dispatches, so no saved blocker can be lost, and over the last 100 main commits no commit with a gate or risk class drops depth (test/review-risk-routing.test.ts, test/review-risk-linear.test.ts); reviewer text is unchanged",
|
||||
'GST-911-redos': "GST-911 ReDoS fix verdict-identity no-loss check (wave review-e7b551ebd865): SVG_REFUSED's url() entry, rewritten to run in linear time, refuses exactly the strings #3838's pattern refused (test/review-risk-redos.test.ts: 60,000 seeded strings and the hand-picked edges, and every SVG blob in the repository's history, with zero differences), so GST-911's no-loss replay holds unchanged; reviewer text is unchanged",
|
||||
'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",
|
||||
@@ -81,7 +82,7 @@ const REVIEWER_BRIEFS: Record<string, { sha: string; replay: Replay }> = {
|
||||
'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: '17a78ec841a62e86e772dcb13e84ef10102db7a24cf4cf8af618c41436d368d5', replay: 'GST-911' },
|
||||
'lib/review-depth.ts': { sha: 'b16f28d6eecc162928d58fcfc6209f703b9692bfe5e901d381972939d2e16f7e', replay: 'GST-911-redos' },
|
||||
'lib/review-risk.ts': { sha: '126eff0f1de6982e03dc24d8017c67e6807c91dfa518b18d2db3aa910aac911f', replay: 'GST-911' },
|
||||
};
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
* class it repeats, and the rewritten rm rule matches exactly the lines R51's pattern matched.
|
||||
*/
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { SVG_REFUSED, svgDrawing } from '../lib/review-depth';
|
||||
import { DATA_LINE, MONEY_LINE, SECRET_LINE, SECRET_PATH, SECURITY_LINE, matchRisk } from '../lib/review-risk';
|
||||
|
||||
const L = 100_000;
|
||||
@@ -71,3 +72,31 @@ describe('the rm rule matches what R51 matched', () => {
|
||||
expect(security('fs.rmSync(dir, { force: true });')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('svgDrawing in linear time', () => {
|
||||
// Value: protects=gstack-review-plan and the commit collector behind ship-queue ready, train build and land never stall on one
|
||||
// changed SVG; fails_when=a whole-text SVG pattern backtracks quadratically on a long whitespace run, or the linear url()
|
||||
// refusal reads any url( differently from #3838's pattern; seam=SVG_REFUSED's url() entry and svgDrawing
|
||||
const old = /url\(\s*(?!['"]?\s*#)/i;
|
||||
test('a 1 MB whitespace run in url(), an attribute, a style element and an href returns in under 250 ms each', () => {
|
||||
const pad = ' '.repeat(1_000_000);
|
||||
const cases = [
|
||||
['<svg><rect fill="url(' + pad + '#a)"/></svg>', true], ['<svg><rect fill="url(' + pad + 'x)"/></svg>', false],
|
||||
['<svg><rect fill' + pad + '="red"/></svg>', true], ['<svg><style>rect{fill:url(' + pad + '#a)}</style></svg>', true],
|
||||
['<svg><use href="' + pad + '#a"/></svg>', true], ['<svg>' + pad + '</svg>', true],
|
||||
] as const;
|
||||
const wrong = cases.map(([s, want], i) => { let got = !want; const t = ms(() => { got = svgDrawing(s); }); return [i, got === want, Math.round(t)] as const; }).filter(([, ok, t]) => !ok || t >= 250);
|
||||
expect(wrong).toEqual([]);
|
||||
}, 60_000);
|
||||
test("the url() refusal matches #3838's pattern on 60,000 seeded short strings and the hand-picked edges", () => {
|
||||
const url = SVG_REFUSED.find(r => r.source.startsWith('url'))!;
|
||||
let x = 911;
|
||||
const next = (n: number) => { x ^= x << 13; x ^= x >>> 17; x ^= x << 5; return (x >>> 0) % n; };
|
||||
const parts = ['url(', 'URL(', ' ', '\t', '\n', '#', "'", '"', 'a', '#a', ')', 'http:', '(', 'u'];
|
||||
const lines = Array.from({ length: 60_000 }, () => Array.from({ length: next(8) + 1 }, () => parts[next(parts.length)]).join(''));
|
||||
const edges = ['url(#a)', 'url( #a)', "url('#a')", "url( '#a')", 'url( "#a")', 'url(" #a")', 'url(\t#a)', 'url(x)', 'url( x)', "url('http://x')", 'url(', 'url( ', "url('')"];
|
||||
expect([...lines, ...edges].filter(s => url.test(s) !== old.test(s))).toEqual([]);
|
||||
expect(lines.filter(s => old.test(s)).length).toBeGreaterThan(1000);
|
||||
expect(lines.filter(s => /url\(/i.test(s) && !old.test(s)).length).toBeGreaterThan(100);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user