CalvinBackup/gstack
1
0 Fork 0
mirror of https://github.com/garrytan/gstack.git synced 2026-11-11 03:00:26 +01:00
Code Issues Packages Projects Releases Wiki Activity

v1.94.2.0 fix(review-risk): a recursive rm or rmSync keeps the security class (GST-942) (#3864)

Browse Source 
* 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>

* fix: pre-landing review fixes (GST-942, pass 1)

- an imported rm( counts like fs.rm(;
- a recursive delete a formatter split across lines raises security,
  read over the file's changed lines joined (SECURITY_JOINED).

Repin lib/review-risk.ts under no-loss replay GST-942 (no tier change
over 100 main commits).

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

* fix: the joined delete rule keeps the over-500-character fallback (GST-942 scoped fix-check)

A split recursive delete padded past 500 characters planned light; the
joined rule now has the [^)]{501} fallback the line rules have. The
changelog and review policy say a one-line edit to an existing split
call is still read on its own.

Repin lib/review-risk.ts under no-loss replay GST-942 (no tier change
over 100 main commits).

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

* docs(changelog): GST-942 entry claims only what the rule covers

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

* test(review-packets-removed): finish the main merge's pin resolution (GST-967 depth pin, GST-942 risk pin)

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

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
Garry Tanandcapy-ai[bot] authored and GitHub committed 2026-11-03 04:21:22 +00:00
1 parent 466689bbbf
commit 7d475a4ea4
8 files changed
+41 -12
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

test/review-packets-removed.test.ts
+2 -1
View File
@@ -66,6 +66,7 @@ const NO_LOSS_REPLAYS = {
'GST-508+R51': "wave review-53cfb5bfe011 merge of #3744 into main: lib/review-depth.ts keeps both admitted changes, the GST-508 no-loss replay of review_exclude_paths (Arm V 33 of 41 blockers vs R 33 of 41, p_NI 0.0279; GStack Minder drive handoffs/GSTA-202/noloss-results.md) and the R51 risk-tier re-score with GST-850's dispatch-superset no-loss check: excluded paths sit beside the added and removed line maps, and the exclusion filter keeps the publishes-docs rule",
'GST-508+GST-464': "wave review-53cfb5bfe011 merge of #3744 into main: ship/sections/review-army.md and adversarial.md carry the GST-508 no-loss-replayed exclude pathspec placeholders inside the brief bodies (handoffs/GSTA-202/noloss-results.md) and the GST-464 lead-instruction edits outside the cut spans, which its brief-identity no-loss check admitted",
'GST-986': "GST-986 brief-identity no-loss check (the brief-body test below): every reviewer brief lib/review-brief.ts cuts from ship/sections/adversarial.md is byte-identical to the bodies at the GST-257 pin; only the lead's Step 11 instruction naming the targeted record's path changed, outside the cut spans",
'GST-942': "GST-942 dispatch-superset no-loss check: the rmSync rule no longer stops at the first ), a matching rm( rule is added, and both read a file's changed lines joined for a delete split across lines, so a recursive delete raises security on more diffs and never on fewer; over the last 100 first-parent main commits to 3d99c2249 (62,531 changed lines) the line 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",
'GST-508+R51+GST-911': "wave review-53cfb5bfe011 merge of main 3d99c2249 (#3848) into #3852: lib/review-depth.ts keeps GST-508's review_exclude_paths (no-loss replay, GStack Minder drive handoffs/GSTA-202/noloss-results.md), R51 with GST-850's dispatch-superset no-loss check, and GST-911's SVG drawings with GST-911-redos (no-loss replay: 0 of 100 main commits changed, reports/GSTA-249-911-decisions.tsv; test/review-planner-misreads.test.ts, test/review-risk-redos.test.ts); lib/review-risk.ts is main's GST-911 bytes. Merged-head checks (handoffs/GSTA-202/merge-3d99c22-checks.md): Arm P on unit 3593 differs only in the exclude pathspec and the disclosure line, the same brief diff as before the merge, and GST-911 panel unit f5cb6ec1e plans, dispatches and renders byte-identical briefs under 3d99c2249 and the merged head",
@@ -95,7 +96,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: 'd76a9afcfadfe0a95b15428c04e6f7a88c6b0fa75591201c45f023fec2bb90db', replay: 'GST-508+R51+GST-911+GST-967' },
'lib/review-risk.ts': { sha: '126eff0f1de6982e03dc24d8017c67e6807c91dfa518b18d2db3aa910aac911f', replay: 'GST-508+R51+GST-911' },
'lib/review-risk.ts': { sha: '08464b78a723ee463ca3a28d0abbc4eb8f4f512192061c703743707b66b5a311', replay: 'GST-942' },
};
/**
test/review-risk-redos.test.ts
+24 -5
View File
@@ -5,7 +5,7 @@
*/
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';
import { DATA_LINE, MONEY_LINE, SECRET_LINE, SECRET_PATH, SECURITY_JOINED, SECURITY_LINE, matchRisk } from '../lib/review-risk';
const L = 100_000;
const ms = (f: () => unknown) => { const t = performance.now(); f(); return performance.now() - t; };
@@ -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('', 'rm('), run('rm(', ' '), run('rmSync(', ' \n'),
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', ' '),
@@ -27,7 +27,7 @@ describe('linear time on hostile input', () => {
run('', 'a_'), run('', 'aU'), run('a', 'A'), run('spend', '_'), run('new Anthropic', ' '), run('.messages.create', ' '),
run('$', 'A_'), run('${', 'A_'), run('', '$A_'), run('process.env.', 'A_'), run('process.env[', ' '), run('os.environ[', ' '), run('os.getenv(', ' '), run('ENV[', ' '),
];
test.each(Object.entries({ SECURITY_LINE, DATA_LINE, SECRET_LINE, MONEY_LINE, SECRET_PATH: [SECRET_PATH] }))('every %s pattern finishes under 100 ms on each 100k-character hostile line', (_n, patterns) => {
test.each(Object.entries({ SECURITY_LINE, SECURITY_JOINED, DATA_LINE, SECRET_LINE, MONEY_LINE, SECRET_PATH: [SECRET_PATH] }))('every %s pattern finishes under 100 ms on each 100k-character hostile line', (_n, patterns) => {
const slow = patterns.flatMap(r => hostile.map(([label, s]) => [r.source.slice(0, 50), label, Math.round(ms(() => r.test(s)))] as const)).filter(([, , t]) => t >= 100);
expect(slow).toEqual([]);
}, 120_000);
@@ -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,25 @@ 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 })', "await rm(dir, { recursive: true, force: true }); // import { rm } from 'node:fs/promises'",
'fs.rm(' + 'a'.repeat(600) + ', { force: true })'];
expect(recursive.filter(l => !security(l))).toEqual([]);
expect(['fs.rm(path.join(dir, "x"), { force: true });', 'farm(x, { recursive: true });', 'confirm(x, { recursive: true });'].filter(security)).toEqual([]);
});
test('a recursive delete a formatter split across lines raises security, read over the joined lines (GST-942 review)', () => {
// Value: protects=a Prettier-formatted rmSync or fs.promises.rm keeps the security class; fails_when=the delete rules read
// only one line at a time; seam=matchRisk over a file's added lines
const split = (lines: string[]) => matchRisk({ codePaths: ['src/clean.ts'], added: { 'src/clean.ts': lines }, removed: {}, scopeAuth: false }).has('security');
expect(split([' fs.rmSync(path.join(root, "build-output-cache"), {', ' recursive: true,', ' force: true,', ' });'])).toBe(true);
expect(split([' await fs.promises.rm(', ' path.join(root, "cache"),', ' { recursive: true },', ' );'])).toBe(true);
expect(split([' fs.rmSync(', ' dir,', ' /*' + 'x'.repeat(510) + '*/', ' { recursive: true },', ' );'])).toBe(true);
expect(split([' fs.rmSync(path.join(root, "file"), {', ' force: true,', ' });'])).toBe(false);
});
});
describe('svgDrawing in linear time', () => {