fix(review-risk): a recursive rm or rmSync keeps the security class whatever its arguments hold (GST-942)

The rmSync rule stopped at the first ), so path.join(...) as the first
argument or a ) in a comment hid recursive: true, and async fs.rm had no
rule. Both now read up to 500 characters of the line, in linear time.

Repin lib/review-risk.ts under no-loss replay GST-942 (dispatch superset:
170 of 170 old matches kept, 19 added, no tier change over 100 main
commits).

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

changelog.d/gst-942-rm-recursive.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- **A recursive delete keeps the security class however its arguments are written (GST-942).** `fs.rmSync(path.join(dir, "x"), { recursive: true })`, a `)` inside a comment before `recursive: true`, and the async `fs.rm`/`fs.promises.rm` all slipped past the rule, because it stopped reading at the first `)`. The review depth could then drop to light on real recursive-delete code. The rule now reads up to 500 characters of the line after `rmSync(` or `.rm(`, still in linear time.

docs/review-policy.md

@@ -228,7 +228,7 @@ line that can run code (a script or `foreignObject` element, an event handler, a
 rules are coarse on purpose and parse no import clause or call arguments;
 `test/review-risk-linear.test.ts` holds each of them under 100 ms on
 200,000-character hostile lines, its anchor followed by a long run of the
-characters it repeats included. `test/review-risk-redos.test.ts` (GST-898) holds every risk pattern, R51's `rm` and `rmSync` rules included, under 100 ms on 100,000-character hostile lines. An `rmSync(` call with more than 500 characters before the first `)` after it counts as security on its own. **data** is user data: a path naming pii, gdpr, ccpa, privacy,
+characters it repeats included. `test/review-risk-redos.test.ts` (GST-898) holds every risk pattern, R51's `rm` and `rmSync` rules included, under 100 ms on 100,000-character hostile lines. An `rmSync(` or `.rm(` call counts as security when `recursive: true` follows within 500 characters on the same line, whatever the arguments hold (a `path.join(...)` first argument or a `)` in a comment included, GST-942), and on its own when more than 500 characters come before the first `)` after it. **data** is user data: a path naming pii, gdpr, ccpa, privacy,
 personal, account, customer, cookie, persistence, storage or database, or a line
 that drops a table or column, deletes rows or bulk-deletes records. Auth scope,
 migrations and deleted code paths were already gate class. `RISK_RULES` hashes

lib/review-risk.ts

@@ -55,7 +55,7 @@ export const SECURITY_LINE = [
   /\b(?:includes|startsWith|endsWith|indexOf)\s*\(\s*['"`]\.\.[/\\]{0,2}['"`]|[!=]==?\s*['"`]\.\.['"`]|['"`]\.\.['"`]\s*[!=]==?|\\\.\\\./,
   /(?<![.\w])eval\s*\(/, /(?:^|[;&|])\s*eval\s+["'$]/, /\bnew\s+Function\s*\(/,
   /\b(?:spawn|spawnSync|execFile|execFileSync)\s*\(\s*['"](?:\/bin\/)?(?:ba|z)?sh['"]/, /\bBun\.spawn(?:Sync)?\s*\(\s*\[\s*['"](?:\/bin\/)?(?:ba|z)?sh['"]/,
-  /\brmSync\s*\((?:[^)]{0,500}recursive\s*:\s*true|[^)]{501})/, /\brm\s+-(?=[a-z]*r)(?=[a-z]*f)[a-z]*[rf]\b/,
+  /\brmSync\s*\((?:.{0,500}?recursive\s*:\s*true|[^)]{501})/, /\.rm\s*\((?:.{0,500}?recursive\s*:\s*true|[^)]{501})/, /\brm\s+-(?=[a-z]*r)(?=[a-z]*f)[a-z]*[rf]\b/,
   /\b(?:innerHTML|outerHTML)\s*\+?=/, /\bdangerouslySetInnerHTML\b/, /\bv-html\b/, /\{@html\b/, /\binsertAdjacentHTML\s*\(/, /\bdocument\.write\s*\(/,
 ];
 export const SVG_PATH = /\.svg$/i;

test/review-packets-removed.test.ts

@@ -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-942': "GST-942 dispatch-superset no-loss check: the rmSync rule no longer stops at the first ) and a matching .rm( rule is added, so a recursive delete raises security on more lines and never on fewer; over the last 100 first-parent main commits to 3d99c2249 (62,531 changed lines) the rules match all 170 lines the old rule matched plus 19 more, and no commit changes depth, class or risk class; 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); SVG_MARKUP now refuses a DOCTYPE naming any document but W3C's SVG DTD (test/review-planner-misreads.test.ts), which only keeps more SVGs code and so drops no reviewer; 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",
@@ -83,7 +84,7 @@ const REVIEWER_BRIEFS: Record<string, { sha: string; replay: Replay }> = {
     '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-risk.ts': { sha: '126eff0f1de6982e03dc24d8017c67e6807c91dfa518b18d2db3aa910aac911f', replay: 'GST-911' },
+    'lib/review-risk.ts': { sha: '262ae8a14cda820c3ea0e508045229c2e36993c0fdfd3a4d51a8834a915de94f', replay: 'GST-942' },
 };
 
 describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => {

test/review-risk-redos.test.ts

@@ -18,7 +18,7 @@ describe('linear time on hostile input', () => {
   //   or on a line of repeated anchors; seam=every pattern in SECURITY_LINE, DATA_LINE, SECRET_LINE, MONEY_LINE and SECRET_PATH
   const hostile = [
     run('rm -', 'r'), run('rm -', 'f'), run('rm -r', 'r'), run('rm -f', 'a'), run('rm\t-', 'r'), run('rm ', ' '), run('rm -', 'r', '1'), run('rm -', 'rf', '_'), run('', 'rm -r '),
-    run('', 'rmSync('), run('rmSync(', ' '), run('rmSync(', 'recursive '), run('rmSync(', 'recursive: '),
+    run('', 'rmSync('), run('rmSync(', ' '), run('rmSync(', 'recursive '), run('rmSync(', 'recursive: '), run('', '.rm('), run('.rm(', ' '), run('.rm(', 'recursive: '),
     run('shell', ' '), run('shell:', ' '), run('shell=', ' '), run('', '; '), run(';', ' '), run('; eval', ' '), run('eval', ' '), run('', ';eval '),
     run('execSync', ' '), run('child_process.exec', ' '), run('os.system', ' '), run('new', ' '), run('new Function', ' '),
     run('spawn(', ' '), run('Bun.spawn(', ' '), run('Bun.spawn([', ' '), run('innerHTML', ' '),
@@ -57,13 +57,13 @@ describe('the rm rule matches what R51 matched', () => {
     expect(lines.filter(s => r51.test(s)).length).toBeGreaterThan(1000);
     expect(edges.filter(s => rm.test(s))).toEqual(['rm -rf /', 'rm -fr x', 'rm -rvf x', 'rm -rf-x', 'rm -rrf', 'rm -ffr', 'rm -rfr', 'rm\t\t-rf', 'sudo rm -rf "$DIR"']);
   });
-  test('the rmSync rule matches what R51 matched on 60,000 seeded short lines, and padding past its window still raises security', () => {
+  test('the rmSync rule matches every line R51 matched on 60,000 seeded short lines, and padding past its window still raises security', () => {
     x = 898;
     const r51Sync = /\brmSync\s*\([^)]*recursive\s*:\s*true/;
     const rmSync = SECURITY_LINE.find(r => r.source.startsWith(String.raw`\brmSync`))!;
     const parts = ['rmSync(', 'rmSync (', 'xrmSync(', 'recursive', ':', ' ', 'true', ')', 'a', 'recursive: true', 'rmSync', '(', '\t'];
     const lines = Array.from({ length: 60_000 }, () => Array.from({ length: next(9) + 1 }, () => parts[next(parts.length)]).join(''));
-    expect(lines.filter(s => rmSync.test(s) !== r51Sync.test(s))).toEqual([]);
+    expect(lines.filter(s => r51Sync.test(s) && !rmSync.test(s))).toEqual([]);
     expect(lines.filter(s => r51Sync.test(s)).length).toBeGreaterThan(1000);
     expect(security('fs.rmSync(dir, { recursive: true, force: true });')).toBe(true);
     expect(security('fs.rmSync(' + 'a'.repeat(480) + ', { recursive: true })')).toBe(true);
@@ -71,6 +71,15 @@ describe('the rm rule matches what R51 matched', () => {
     expect(security('fs.rmSync(dir, /*' + 'x'.repeat(5_000_000) + '*/ { recursive: true });')).toBe(true);
     expect(security('fs.rmSync(dir, { force: true });')).toBe(false);
   });
+  test('a recursive rm or rmSync whose first argument is a call, or whose arguments hold a ), raises security (GST-942)', () => {
+    // Value: protects=a recursive delete written the common way keeps the security class; fails_when=the rule stops at the first
+    //   ) inside the first argument or a comment, or skips the async fs.rm; seam=matchRisk on one added line
+    const recursive = ['fs.rmSync(path.join(dir, "x"), { recursive: true, force: true });', 'fs.rmSync(dir, /* ) */ { recursive: true });',
+      'fs.rmSync(path.resolve(root, name), { recursive: true });', 'await fs.promises.rm(path.join(dir, "x"), { recursive: true });',
+      'await fs.rm(dir, { recursive: true })'];
+    expect(recursive.filter(l => !security(l))).toEqual([]);
+    expect(['fs.rm(path.join(dir, "x"), { force: true });', 'const rm = (a) => a; rm(x, { recursive: true });', 'farm(x, { recursive: true });'].filter(security)).toEqual([]);
+  });
 });
 
 describe('svgDrawing in linear time', () => {