diff --git a/lib/review-collect.ts b/lib/review-collect.ts index 06acd1f06..087059a65 100644 --- a/lib/review-collect.ts +++ b/lib/review-collect.ts @@ -317,7 +317,7 @@ export function collectLive(root: string, base: string): Collected { } const consumed = scanConsumers(root, [...diff.perPath.map(e => e.path), ...diff.deleted], [mergeBase], true, root, errors); const drawings = drawingsAmong(diff.perPath, root, p => { - try { return fs.statSync(path.join(root, p)).size <= DRAWING_READ_CAP ? fs.readFileSync(path.join(root, p)) : null; } catch { return null; } + try { const st = fs.lstatSync(path.join(root, p)); return st.isFile() && st.size <= DRAWING_READ_CAP ? fs.readFileSync(path.join(root, p)) : null; } catch { return null; } }); const executables = executablesAmong(diff.perPath, root, e => { try { diff --git a/lib/review-depth.ts b/lib/review-depth.ts index bd95af017..11c568501 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -288,7 +288,7 @@ export function classifyDiff(i: ClassInput): ClassResult { if (i.registry?.error) errors.push(i.registry.error); } const riskHits = CLASS_RANK[cls] >= CLASS_RANK.code ? matchRisk({ codePaths: i.paths.filter(p => pathClass(i, p) === 'code'), executables: i.executables, added: i.added, removed: i.removed, scopeAuth: s.SCOPE_AUTH === 'true', ...(i.riskPaths?.length ? { isRiskPath: (p: string) => matchesAny(i.riskPaths!, p) && !DATA_PATH.test(p) } : {}) }) : new Map(); - const docSecret = i.paths.find(p => pathClass(i, p) === 'docs' && DOC_PATH.test(p) && (i.added?.[p] ?? []).some(l => DOC_SECRET_LINE.some(r => r.test(l)))); + const docSecret = i.paths.find(p => DOC_PATH.test(p) && (i.added?.[p] ?? []).some(l => DOC_SECRET_LINE.some(r => r.test(l)))); if (docSecret && !riskHits.has('security')) riskHits.set('security', `docs that can print a credential: ${docSecret}`); const byRank = (a: RiskClass, b: RiskClass) => RISK_CLASSES.indexOf(a) - RISK_CLASSES.indexOf(b); const detected = [...riskHits.keys()].sort(byRank); diff --git a/test/fixtures/review-pins/files/lib/review-depth.ts.json b/test/fixtures/review-pins/files/lib/review-depth.ts.json index 8c010bd6a..a77572084 100644 --- a/test/fixtures/review-pins/files/lib/review-depth.ts.json +++ b/test/fixtures/review-pins/files/lib/review-depth.ts.json @@ -1 +1 @@ -{"sha":"2c08a34c47094a808963e87e6a5eb95e8529d6f68b1896174b8dfa613562468e","replay":"GSTA-258-L7"} +{"sha":"6688963750c14f03cdbb249dcaa27ebd9e9c2c62170f4e142ec8ded28af010a1","replay":"GSTA-258-L7"} diff --git a/test/fixtures/review-pins/replays/GSTA-258-L7.json b/test/fixtures/review-pins/replays/GSTA-258-L7.json index 3d6845fe6..0066b5951 100644 --- a/test/fixtures/review-pins/replays/GSTA-258-L7.json +++ b/test/fixtures/review-pins/replays/GSTA-258-L7.json @@ -1 +1 @@ -{"replay":"GSTA-258-L7","justification":"GSTA-258 L7 planner no-loss check (GST-1177, Addenda 1 and 7): the change only adds triggers (executables stay code; seal, sealed, custody and egress path tokens; redaction calls (camelCase or snake_case), redaction or outside-voice imports (from, side-effect, require, dynamic and Python forms) and SECRET or CREDENTIAL constants set to a regex literal, a regex list or new RegExp are security; a deleted file is never an executable, and an executable or symlink is code with its lines read whatever its docs, data or config name) and removes none; over the last 200 first-parent main commits to bf2cab34c5, planReview and collectCommit from origin/main and from this branch give the same depth, class, risk classes, next step and dispatches on 195, and the other 5 only raise: ed5c948eb6 (#3921, the GST-1164 secret-egress fix) and c197a139b7 (#3918, approval text in public PR bodies) and 514daebf5f (#3569, lib/sealed-guard.ts) go light to full on security, while 90a170c941 and 71acd83aa6 gain security at unchanged full depth and reviewers; no commit's depth drops ; reruns after the pass 1 and Step 11 review fixes give the same 200 rows (GStack Minder drive handoffs/GSTA-258/L7-planner-noloss/, l7-200-v2.tsv, l7-200-v3.tsv)"} +{"replay":"GSTA-258-L7","justification":"GSTA-258 L7 planner no-loss check (GST-1177, Addenda 1 and 7): the change only adds triggers (executables stay code; seal, sealed, custody and egress path tokens; redaction calls (camelCase or snake_case), redaction or outside-voice imports (from, side-effect, require, dynamic and Python forms) and SECRET or CREDENTIAL constants set to a regex literal, a regex list or new RegExp are security; the docs credential check reads every docs path whatever its class; a deleted file is never an executable, and an executable or symlink is code with its lines read whatever its docs, data or config name) and removes none; over the last 200 first-parent main commits to bf2cab34c5, planReview and collectCommit from origin/main and from this branch give the same depth, class, risk classes, next step and dispatches on 194, and the other 6 only raise: ed5c948eb6 (#3921, the GST-1164 secret-egress fix) and c197a139b7 (#3918, approval text in public PR bodies) and 514daebf5f (#3569, lib/sealed-guard.ts) go light to full on security, while 90a170c941, 71acd83aa6 and 216493fb32 gain security at unchanged full depth and reviewers; no commit's depth drops ; reruns after the pass 1 and Step 11 fixes give the same rows, and after the second-exception fixes 216493fb32 also gains security at unchanged depth and readers (GStack Minder drive handoffs/GSTA-258/L7-planner-noloss/, l7-200-v2.tsv, l7-200-v3.tsv, l7-200-v4.tsv)"} diff --git a/test/review-collect.test.ts b/test/review-collect.test.ts index 2f945da25..74440e333 100644 --- a/test/review-collect.test.ts +++ b/test/review-collect.test.ts @@ -147,6 +147,18 @@ describe('GSTA-258 L7 Addendum 1 and 7: executables among data-named paths', () expect([d.class, d.risk.includes('security'), d.depth]).toEqual(['code', true, 'full']); } }); + // Value: protects=collectLive never opens a symlinked SVG (GStack Minder second exception on #3972); fails_when=a tracked + // art.svg linked to a FIFO blocks the drawing read; seam=drawingsAmong's live reader + test('a symlinked SVG is not read as a drawing in collectLive', () => { + const r = repo({}); + const fifo = path.join(r.dir, '.git', 'fifo'); + spawnSync('mkfifo', [fifo], { timeout: 5_000 }); + fs.symlinkSync(fifo, path.join(r.dir, 'art.svg')); + r.commit({}); + const t0 = Date.now(); + expect(collectLive(r.dir, 'main').drawings).toEqual([]); + expect(Date.now() - t0).toBeLessThan(20_000); + }, 30_000); }); describe('collectLive: the working tree', () => { diff --git a/test/review-depth-lean.test.ts b/test/review-depth-lean.test.ts index a1429c6bf..885269fd8 100644 --- a/test/review-depth-lean.test.ts +++ b/test/review-depth-lean.test.ts @@ -36,6 +36,9 @@ describe('HARN-1 false triggers (harness-evals#231 on 1.93.7.0): data, config an expect(c.class).toBe('code'); expect(c.reasons).toEqual(expect.arrayContaining(['code: scripts/logs/prune'])); for (const p of exe) expect(classifyDiff({ paths: [p], gateTagged: 0, executables: [p] }).class).toBe('code'); + const txt = 'scripts/logs/prune.txt'; + const printsKey = classifyDiff({ paths: [txt], gateTagged: 0, executables: [txt], added: { [txt]: ['curl -H "Authorization: Bearer abcdefgh12345678"'] } }); + expect([printsKey.class, printsKey.risk]).toEqual(['code', ['security']]); expect(classifyDiff({ paths: ['logs/run.txt', 'docs/r/logs/ready'], gateTagged: 0, executables: [] }).class).toBe('docs'); }); // Value: protects=sealed custody and outside-model prompt egress plan full depth (GST-1177: harness-evals#237, gstack #3921);