mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 23:14:33 +01:00
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>
This commit is contained in:
1 parent
2df9dad441
commit
ba95b1d050
5 files changed
+5
-4
No files matched your search
@@ -1,3 +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`, `fs.rm`/`fs.promises.rm`/an imported `rm`, and a call a formatter split across lines all slipped past the rule, because it stopped reading at the first `)` and read one line at a time. The review depth could then drop to light on real recursive-delete code. The rule now reads up to 500 characters after `rmSync(` or `rm(`, across the file's changed lines, still in linear time.
|
||||
- **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`, `fs.rm`/`fs.promises.rm`/an imported `rm`, and a new call a formatter split across lines all slipped past the rule, because it stopped reading at the first `)` and read one line at a time. The review depth could then drop to light on real recursive-delete code. The rule now reads up to 500 characters after `rmSync(` or `rm(` across the file's changed lines, still in linear time. An edit to one line of an existing split call is still read on its own.
|
||||
@@ -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(` or `rm(` call (`fs.rm`, `fs.promises.rm` or an imported `rm`) counts as security when `recursive: true` follows within 500 characters, whatever the arguments hold (a `path.join(...)` first argument or a `)` in a comment included) and also when a formatter split the call across lines, which is read over the file's changed lines joined (GST-942); a call with more than 500 characters before the first `)` after it counts 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 (`fs.rm`, `fs.promises.rm` or an imported `rm`) counts as security when `recursive: true` follows within 500 characters, whatever the arguments hold (a `path.join(...)` first argument or a `)` in a comment included) and also when a formatter split a new call across lines, which is read over the file's changed lines joined (GST-942; an edit to one line of an existing split call is read on its own); a call with more than 500 characters before the first `)` after it counts on its own. **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
|
||||
|
||||
+1
-1
@@ -59,7 +59,7 @@ export const SECURITY_LINE = [
|
||||
/\b(?:innerHTML|outerHTML)\s*\+?=/, /\bdangerouslySetInnerHTML\b/, /\bv-html\b/, /\{@html\b/, /\binsertAdjacentHTML\s*\(/, /\bdocument\.write\s*\(/,
|
||||
];
|
||||
/** A recursive delete split across lines (a formatter puts `recursive: true` on its own line), read over a file's changed lines joined. */
|
||||
export const SECURITY_JOINED = [/\brm(?:Sync)?\s*\([\s\S]{0,500}?recursive\s*:\s*true/];
|
||||
export const SECURITY_JOINED = [/\brm(?:Sync)?\s*\((?:[\s\S]{0,500}?recursive\s*:\s*true|[^)]{501})/];
|
||||
export const SVG_PATH = /\.svg$/i;
|
||||
/** SVG text that can run code, read decoded and again with whitespace removed (java	script: is javascript:); a file's changed lines are read joined, so a split attribute still counts. */
|
||||
export const SVG_ACTIVE = [/<(?:[\w.-]+:)?(?:script|foreignObject)\b/i, /(?:^|[\s"'/])on[a-z]+\s*=/i, /(?:java|vb)script:|data:text\/html/i];
|
||||
|
||||
@@ -84,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: 'd8845fdb7b4cffe73d400b367c1fdf7df03ca155f2e579ed5c9833d4e9d09094', replay: 'GST-942' },
|
||||
'lib/review-risk.ts': { sha: '08464b78a723ee463ca3a28d0abbc4eb8f4f512192061c703743707b66b5a311', replay: 'GST-942' },
|
||||
};
|
||||
|
||||
describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => {
|
||||
|
||||
@@ -87,6 +87,7 @@ describe('the rm rule matches what R51 matched', () => {
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user