diff --git a/lib/review-collect.ts b/lib/review-collect.ts index d0452b165..06acd1f06 100644 --- a/lib/review-collect.ts +++ b/lib/review-collect.ts @@ -234,16 +234,16 @@ function drawingsAmong(perPath: readonly PathLines[], classRoot: string | readon } /** - * The changed paths the data rules could move to docs (a data or run-log name) that are executables: the exec bit, or text + * The changed paths a docs, data or run-log rule could move out of code that are executables: the exec bit, a symlink, or text * that starts with "#!" (GSTA-258 L7). `isExec` answers for one path and fails closed (true) when it cannot read it. */ function executablesAmong(perPath: readonly PathLines[], classRoot: string | readonly string[], isExec: (e: PathLines) => boolean): string[] { - return perPath.filter(e => (BINARY_DATA_PATH.test(e.path) && classifyPath(e.path, classRoot) === 'code') || dataCandidate(e.path, classRoot) || svgCandidate(e.path, classRoot)).filter(isExec).map(e => e.path).sort(); + return perPath.filter(e => (BINARY_DATA_PATH.test(e.path) && classifyPath(e.path, classRoot) === 'code') || dataCandidate(e.path, classRoot) || svgCandidate(e.path, classRoot) || classifyPath(e.path, classRoot) === 'docs').filter(isExec).map(e => e.path).sort(); } /** - * The executables among the changed paths at `headSha`: tree mode 100755, or a text blob whose first two bytes are "#!" (only - * those bytes are read). A path the head tree lacks is a deletion, never an executable; an unreadable tree or blob counts as one. + * The executables among the changed paths at `headSha`: tree mode 100755 or 120000 (a symlink, as collectLive sees it), or a text + * blob whose first two bytes are "#!". A path the head tree lacks is a deletion, never an executable; an unreadable tree or blob counts as one. */ function executablesAt(root: string, headSha: string, perPath: readonly PathLines[], classRoot: readonly string[]): string[] { const tree = git(root, ['ls-tree', '-r', '-z', headSha]); @@ -252,7 +252,7 @@ function executablesAt(root: string, headSha: string, perPath: readonly PathLine if (!tree.ok) return true; const t = entries.get(e.path); if (!t) return false; - if (t.mode === '100755') return true; + if (t.mode === '100755' || t.mode === '120000') return true; if (e.binary) return false; const r = spawnSync('git', ['cat-file', 'blob', t.sha], { cwd: root, timeout: 30_000, maxBuffer: 2 }); const read = r.status === 0 || (r.error as NodeJS.ErrnoException | undefined)?.code === 'ENOBUFS'; @@ -322,7 +322,8 @@ export function collectLive(root: string, base: string): Collected { const executables = executablesAmong(diff.perPath, root, e => { try { const f = path.join(root, e.path); - if (fs.statSync(f).mode & 0o111) return true; + const st = fs.lstatSync(f); + if (!st.isFile() || st.mode & 0o111) return true; if (e.binary) return false; const fd = fs.openSync(f, 'r'); const buf = Buffer.alloc(2); diff --git a/lib/review-depth.ts b/lib/review-depth.ts index 8ba09f109..bd95af017 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -237,7 +237,7 @@ export interface ClassInput { created?: readonly string[]; /** The SVG candidates (svgCandidate) whose whole text the collector read as a drawing (svgDrawing); any other SVG stays code. */ drawings?: readonly string[]; - /** Data- or log-named paths the collector found executable (exec bit or a "#!" first line): they stay code whatever their name or directory. */ + /** Docs-, data- or log-named paths the collector found executable (exec bit, a symlink or a "#!" first line): code whatever their name or directory, never judged by path alone. */ executables?: readonly string[]; /** Added and deleted lines per path (lib/review-collect.ts): the risk classes read them; without them no line signal applies. */ added?: Record; @@ -256,7 +256,7 @@ const sanitize = (s: string) => s.replace(/[\0-\x1f]+/g, ' ').slice(0, 200); const isOutput = (i: ClassInput, p: string) => !!i.registry?.outputs.length && matchesAny(i.registry.outputs, p); const unconsumedData = (i: ClassInput, p: string) => !i.executables?.includes(p) && Array.isArray(i.consumed) && !i.consumed.includes(p) && (dataCandidate(p, i.root) || (!!i.drawings?.includes(p) && svgCandidate(p, i.root))); const binaryData = (i: ClassInput, p: string) => !i.executables?.includes(p) && BINARY_DATA_PATH.test(p) && classifyPath(p, i.root) === 'code' && ![...AGENT_PROSE, ...GATE_PATH, ...DATA_CONFIG].some(r => r.test(p)); -const pathClass = (i: ClassInput, p: string): PathClass => (i.excluded?.has(p) || (i.stampOnly ?? []).includes(p) || unconsumedData(i, p) || binaryData(i, p) ? 'docs' : classifyPath(p, i.root)); +const pathClass = (i: ClassInput, p: string): PathClass => i.executables?.includes(p) && !i.excluded?.has(p) ? 'code' : (i.excluded?.has(p) || (i.stampOnly ?? []).includes(p) || unconsumedData(i, p) || binaryData(i, p) ? 'docs' : classifyPath(p, i.root)); /** The diff's class, raised to gate by any gate signal; `errors` lists every input that fails the depth closed; `runtimeLines` counts code-class lines outside the base registry's outputs. */ export function classifyDiff(i: ClassInput): ClassResult { @@ -287,7 +287,7 @@ export function classifyDiff(i: ClassInput): ClassResult { for (const p of i.paths) if (GATE_PATH.some(r => r.test(p))) { raise('gate', `gate path: ${p}`); break; } 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'), 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 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)))); 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); diff --git a/lib/review-risk.ts b/lib/review-risk.ts index ef32d4512..ba8cbb845 100644 --- a/lib/review-risk.ts +++ b/lib/review-risk.ts @@ -134,6 +134,8 @@ export interface RiskInput { removed?: Record; /** gstack-diff-scope's SCOPE_AUTH: the auth class needs it and an auth token in the path. */ scopeAuth: boolean; + /** Executables (lib/review-collect.ts): never a config file or results script, so their lines are always read. */ + executables?: readonly string[]; /** The base policy's review.risk_paths matcher: a code path it matches is risk class `policy`. */ isRiskPath?: (p: string) => boolean; } @@ -144,9 +146,10 @@ export function matchRisk(i: RiskInput): Map { const hit = (c: RiskClass, reason: string) => { if (!why.has(c)) why.set(c, reason); }; for (const p of i.codePaths) { if (i.isRiskPath?.(p)) hit('policy', 'risk_paths: ' + p); - const stats = STATS_SCRIPT.test(p); + const exe = !!i.executables?.includes(p); + const stats = !exe && STATS_SCRIPT.test(p); const t = stats ? [] : tokens(p); - const config = CONFIG_DATA_PATH.test(p); + const config = !exe && CONFIG_DATA_PATH.test(p); const pathOnly = config || stats; const prose = PROSE_PATH.test(p); if (i.scopeAuth && t.some(x => AUTH_PATH_TOKENS.includes(x) || AUTH_PATH_PREFIXES.some(a => x.startsWith(a)))) hit('auth', 'auth path: ' + p); 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 664adda60..8c010bd6a 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":"17617e1ecfc9cf91cd1df564363a67439bb1d451e07e98d38350b79905ec49ec","replay":"GSTA-258-L7"} +{"sha":"2c08a34c47094a808963e87e6a5eb95e8529d6f68b1896174b8dfa613562468e","replay":"GSTA-258-L7"} diff --git a/test/fixtures/review-pins/files/lib/review-risk.ts.json b/test/fixtures/review-pins/files/lib/review-risk.ts.json index 8c989d885..e160b17bb 100644 --- a/test/fixtures/review-pins/files/lib/review-risk.ts.json +++ b/test/fixtures/review-pins/files/lib/review-risk.ts.json @@ -1 +1 @@ -{"sha":"2b28089481c2209d319d8bd9e7a5feb4045a50aae6bccee1ca6c450804af87cc","replay":"GSTA-258-L7"} +{"sha":"1d03d5cd8f0cae5f01af85502004849c9fa2545009957f2c4c44a1359d9d0fc0","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 492fce682..3d6845fe6 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 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 ; rerun after the pass 1 review fixes gives the same 200 rows (GStack Minder drive handoffs/GSTA-258/L7-planner-noloss/, l7-200-v2.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; 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)"} diff --git a/test/review-collect.test.ts b/test/review-collect.test.ts index dac6833a8..2f945da25 100644 --- a/test/review-collect.test.ts +++ b/test/review-collect.test.ts @@ -130,6 +130,23 @@ describe('GSTA-258 L7 Addendum 1 and 7: executables among data-named paths', () r.write({ 'r.log': null }); expect(collectLive(r.dir, 'main').executables).toEqual([]); }); + // Value: protects=a docs or config name never hides an executable's lines, and a symlink is never opened (outside voice on #3972); + // fails_when=a 755 prune.txt or prune.json with rm -rf classes light with no security, or a logs/ symlink is read through; + // seam=collectors -> classifyDiff -> matchRisk + test('executable .txt and .json scripts are code with security lines; a symlink counts as executable without being read', () => { + const r = repo({}); + const script = '#!/bin/sh\nrm -rf "$1"\n'; + r.write({ 'scripts/logs/prune.txt': script, 'tools/prune.json': script }); + for (const p of ['scripts/logs/prune.txt', 'tools/prune.json']) fs.chmodSync(path.join(r.dir, p), 0o755); + fs.mkdirSync(path.join(r.dir, 'logs')); + fs.symlinkSync('/dev/null', path.join(r.dir, 'logs/run')); + const head = r.commit({}); + for (const c of [collectCommit(r.dir, r.baseSha, head), collectLive(r.dir, 'main')]) { + expect(c.executables).toEqual(['logs/run', 'scripts/logs/prune.txt', 'tools/prune.json']); + const d = depthOfCommitInput({ ...c, gateTagged: 0 }); + expect([d.class, d.risk.includes('security'), d.depth]).toEqual(['code', true, 'full']); + } + }); }); describe('collectLive: the working tree', () => {