From 505322000c119096a1c3469102ec071fc7b7470d Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 19:00:14 +0000 Subject: [PATCH 1/9] review-depth: an exec bit or shebang keeps logs/ and .out paths code (GSTA-258 L7 Addendum 1, 7) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-review-plan | 2 +- lib/review-collect.ts | 57 +++++++++++++++++++++++++++++++--- lib/review-depth.ts | 6 ++-- lib/ship-preflight.ts | 2 +- test/review-collect.test.ts | 21 +++++++++++++ test/review-depth-lean.test.ts | 10 ++++++ 6 files changed, 89 insertions(+), 9 deletions(-) diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index ab4cd4eec..fa4a161be 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -216,7 +216,7 @@ function main(): number { const gateScan = { tagged: gateTagged, head }; if (has('--json')) { const { runtimeLines, stepDepth, depthReasons, ...rest } = plan; - process.stdout.write(JSON.stringify({ ...rest, review_policy: review, platform_review: platform ? (platform.ok ? platform.review : { none: platform.reason }) : null, targeted: { ...targeted, brief, targeted_patch_id: patch }, step_depth: stepDepth, runtime_lines: runtimeLines, depth_reasons: depthReasons, depth_floor: floor ?? null, gate_scan: gateScan, fix_range: fixRange ?? null, registry: { sha: registry.sha, outputs: registry.outputs }, per_path: perPath, diff_lines: diffLines, base: collected.base, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly, consumed: collected.consumed, drawings: collected.drawings, + process.stdout.write(JSON.stringify({ ...rest, review_policy: review, platform_review: platform ? (platform.ok ? platform.review : { none: platform.reason }) : null, targeted: { ...targeted, brief, targeted_patch_id: patch }, step_depth: stepDepth, runtime_lines: runtimeLines, depth_reasons: depthReasons, depth_floor: floor ?? null, gate_scan: gateScan, fix_range: fixRange ?? null, registry: { sha: registry.sha, outputs: registry.outputs }, per_path: perPath, diff_lines: diffLines, base: collected.base, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly, consumed: collected.consumed, drawings: collected.drawings, executables: collected.executables, ...(exclusion.state === 'head-only' ? { review_exclude: exclusion } : active ? { review_exclude: { state: 'active', ...active.record, specs: active.specs, keep: active.keep, kept_files: active.kept, content: active.content, references: active.references } } : {}) }) + '\n'); return EXIT.ok; } diff --git a/lib/review-collect.ts b/lib/review-collect.ts index f5e10709f..ab70cedac 100644 --- a/lib/review-collect.ts +++ b/lib/review-collect.ts @@ -13,7 +13,7 @@ import * as path from 'node:path'; import { resolveBase, type ResolvedBase } from './gate-diff/git'; import { GENERATED_PATH, parseRegistry } from './regen'; import { RESULT_CODES } from './result-codes'; -import { classifyPath, dataCandidate, svgCandidate, svgDrawing, type ClassInput, type DepthError, type PathLines, type RegistryInfo } from './review-depth'; +import { BINARY_DATA_PATH, classifyPath, dataCandidate, svgCandidate, svgDrawing, type ClassInput, type DepthError, type PathLines, type RegistryInfo } from './review-depth'; export interface Collected extends ClassInput { perPath: PathLines[]; @@ -32,6 +32,7 @@ export interface Collected extends ClassInput { untracked: string[]; consumed: string[] | 'all'; drawings: string[]; + executables: string[]; /** collectLive only: the ref resolveBase read for the base name (GST-1036), null when nothing resolved. */ base?: ResolvedBase | null; } @@ -232,9 +233,44 @@ function drawingsAmong(perPath: readonly PathLines[], classRoot: string | readon }).sort(); } -function result(diff: ReturnType, extra: Pick & { drawings?: string[] }): Collected { +/** + * The changed paths the data rules could move to docs (a data or run-log name) that are executables: the exec bit, 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(); +} + +/** The executables among the changed paths at `headSha`: tree mode 100755, or a text blob that starts with "#!"; an unreadable tree or blob counts as executable. */ +function executablesAt(root: string, headSha: string, perPath: readonly PathLines[], classRoot: readonly string[]): string[] { + const tree = git(root, ['ls-tree', '-r', '-z', headSha]); + const entries = new Map(tree.out.split('\0').flatMap(rec => { const m = /^(\d+) blob ([0-9a-f]+)\t(.*)$/s.exec(rec); return m ? [[m[3]!, { mode: m[1]!, sha: m[2]! }] as const] : []; })); + const firstBytes = new Map(); + const text = perPath.flatMap(e => { const t = entries.get(e.path); return !e.binary && t && t.mode !== '100755' ? [t.sha] : []; }); + if (text.length) { + const r = spawnSync('git', ['cat-file', '--batch'], { cwd: root, input: text.join('\n') + '\n', timeout: 60_000, maxBuffer: 256 * 1024 * 1024 }); + const out = r.status === 0 && !r.error ? r.stdout : null; + for (let at = 0; out && at < out.length;) { + const nl = out.indexOf(10, at); + const m = nl < 0 ? null : /^([0-9a-f]+) blob (\d+)$/.exec(out.subarray(at, nl).toString('latin1')); + if (!m) break; + firstBytes.set(m[1]!, out.subarray(nl + 1, nl + 3).toString('latin1')); + at = nl + 1 + Number(m[2]) + 1; + } + } + return executablesAmong(perPath, classRoot, e => { + const t = entries.get(e.path); + if (!tree.ok || !t) return true; + if (t.mode === '100755') return true; + if (e.binary) return false; + const head = firstBytes.get(t.sha); + return head === undefined || head === '#!'; + }); +} + +function result(diff: ReturnType, extra: Pick & { drawings?: string[]; executables?: string[] }): Collected { const paths = diff.perPath.map(e => e.path).sort(); - return { ...diff, drawings: [], ...extra, paths, gateTagged: null, diffLines: diff.perPath.reduce((n, e) => n + e.lines, 0) }; + return { ...diff, drawings: [], executables: [], ...extra, paths, gateTagged: null, diffLines: diff.perPath.reduce((n, e) => n + e.lines, 0) }; } /** The range `baseSha`...`headSha` from git objects alone; the registry is the base commit's, never the head's. */ @@ -255,7 +291,7 @@ export function collectCommit(root: string, baseSha: string, headSha: string): C const blob = spawnSync('git', ['cat-file', 'blob', `${headSha}:${p}`], { cwd: root, timeout: 60_000, maxBuffer: DRAWING_READ_CAP + 1 }); return blob.status === 0 && !blob.error ? blob.stdout : null; }); - return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [], consumed, drawings }); + return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [], consumed, drawings, executables: executablesAt(root, headSha, diff.perPath, skillDirs) }); } /** @@ -291,5 +327,16 @@ export function collectLive(root: string, base: string): Collected { 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; } }); - return { ...result(diff, { root, registry, errors, mergeBase, untracked, consumed, drawings }), base: resolved }; + const executables = executablesAmong(diff.perPath, root, e => { + try { + const f = path.join(root, e.path); + if (fs.statSync(f).mode & 0o111) return true; + if (e.binary) return false; + const fd = fs.openSync(f, 'r'); + const buf = Buffer.alloc(2); + try { fs.readSync(fd, buf, 0, 2, 0); } finally { fs.closeSync(fd); } + return buf.toString('latin1') === '#!'; + } catch { return true; } + }); + return { ...result(diff, { root, registry, errors, mergeBase, untracked, consumed, drawings, executables }), base: resolved }; } diff --git a/lib/review-depth.ts b/lib/review-depth.ts index 666768fd3..8ba09f109 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -237,6 +237,8 @@ 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. */ + 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; removed?: Record; @@ -252,8 +254,8 @@ export interface ClassResult { class: DiffClass; reasons: string[]; errors: Dept 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) => 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) => BINARY_DATA_PATH.test(p) && classifyPath(p, i.root) === 'code' && ![...AGENT_PROSE, ...GATE_PATH, ...DATA_CONFIG].some(r => r.test(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)); /** 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. */ diff --git a/lib/ship-preflight.ts b/lib/ship-preflight.ts index 3740e3fe1..f4711573c 100644 --- a/lib/ship-preflight.ts +++ b/lib/ship-preflight.ts @@ -148,7 +148,7 @@ function merge(cwd: string, base: string, r: Report): boolean { let runtime = 'unknown'; try { const j = JSON.parse(plan.stdout); - const c = classifyDiff({ paths: j.paths, perPath: j.per_path, stampOnly: j.stamp_only, registry: j.registry, consumed: j.consumed, drawings: j.drawings, root: cwd, gateTagged: 0 }); + const c = classifyDiff({ paths: j.paths, perPath: j.per_path, stampOnly: j.stamp_only, registry: j.registry, consumed: j.consumed, drawings: j.drawings, executables: j.executables, root: cwd, gateTagged: 0 }); runtime = String(c.runtimeLines ?? 'unknown'); } catch { /* unknown */ } r.add(`RUNTIME_LINES: ${runtime} (gstack-ship-budget check --runtime-lines at the Step 9 and 12 boundaries)`); diff --git a/test/review-collect.test.ts b/test/review-collect.test.ts index 3db4a3665..2613189b9 100644 --- a/test/review-collect.test.ts +++ b/test/review-collect.test.ts @@ -100,6 +100,27 @@ describe('collectCommit: git objects only', () => { }); }); +describe('GSTA-258 L7 Addendum 1 and 7: executables among data-named paths', () => { + // Value: protects=an exec bit or a shebang keeps a logs/ or .out path code in both collectors; fails_when=scripts/logs/prune + // or an executable .out reaches classifyDiff as data (docs, light); seam=collectCommit/collectLive -> depthOfCommitInput + test('the exec bit or a shebang lists the path as executable; a plain log does not', () => { + const r = repo({}); + r.write({ 'scripts/logs/prune': '#!/bin/sh\nrm -rf "$1"\n', 'bin/tool.out': 'ELF-ish\n', 'logs/run.txt': 'ok\n', 'docs/r/logs/ready': 'yes\n' }); + fs.chmodSync(path.join(r.dir, 'bin/tool.out'), 0o755); + const head = r.commit({}); + const c = collectCommit(r.dir, r.baseSha, head); + expect(c.executables).toEqual(['bin/tool.out', 'scripts/logs/prune']); + expect(depthOfCommitInput({ ...c, gateTagged: 0 }).class).toBe('code'); + r.write({ 'docs/r/logs/later': '#!/usr/bin/env bash\necho\n' }); + expect(collectLive(r.dir, 'main').executables).toEqual(['bin/tool.out', 'docs/r/logs/later', 'scripts/logs/prune']); + const plain = repo({}); + const ph = plain.commit({ 'logs/run.txt': 'ok\n', 'docs/r/logs/ready': 'yes\n', 'docs/r/agent.out': 'done\n' }); + const pc = collectCommit(plain.dir, plain.baseSha, ph); + expect(pc.executables).toEqual([]); + expect(depthOfCommitInput({ ...pc, gateTagged: 0 }).class).toBe('docs'); + }); +}); + describe('collectLive: the working tree', () => { test('tracked uncommitted edits and untracked files count per path; scratch under .gstack/tmp/ does not', () => { const r = repo({ 'src/a.ts': lines(1) }); diff --git a/test/review-depth-lean.test.ts b/test/review-depth-lean.test.ts index 6fd77c49d..760fb0971 100644 --- a/test/review-depth-lean.test.ts +++ b/test/review-depth-lean.test.ts @@ -28,6 +28,16 @@ describe('HARN-1 false triggers (harness-evals#231 on 1.93.7.0): data, config an expect([c.class, c.risk]).toEqual(['docs', []]); expect(classifyDiff({ paths: ['src/logs/rotate.ts'], gateTagged: 0 }).class).toBe('code'); }); + // Value: protects=a directory name or a data extension never downgrades an executable (GSTA-258 L7 Addendum 1 and 7); + // fails_when=scripts/logs/prune or an executable .out classes as docs; seam=collector executables -> classifyDiff + test('an executable (exec bit or shebang) under logs/ or named .out stays code; plain logs stay docs', () => { + const exe = ['scripts/logs/prune', 'tools/build/runner.out']; + const c = classifyDiff({ paths: [...exe, 'logs/run.txt'], gateTagged: 0, executables: exe }); + 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'); + expect(classifyDiff({ paths: ['logs/run.txt', 'docs/r/logs/ready'], gateTagged: 0, executables: [] }).class).toBe('docs'); + }); test('a sqlite file is data even when a script names it', () => { const c = classifyDiff({ paths: [sqlite, 'scripts/load.py'], gateTagged: 0, perPath: [{ path: sqlite, lines: 0, binary: true }, { path: 'scripts/load.py', lines: 4, binary: false }], consumed: [sqlite] }); expect([c.class, c.errors.map(e => e.code), c.runtimeLines]).toEqual(['code', [], 4]); From 9123ae2bb09a83c7abbd1f04fee6f360b9c5d9c4 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 19:02:11 +0000 Subject: [PATCH 2/9] review-brief: sliceDiff splits and names git-quoted and spaced filenames (GSTA-258 L7 Addendum 1.2) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/review-brief.ts | 11 +++++++++-- test/review-depth-lean.test.ts | 11 +++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/review-brief.ts b/lib/review-brief.ts index 698715c1d..aadc15e36 100644 --- a/lib/review-brief.ts +++ b/lib/review-brief.ts @@ -43,14 +43,21 @@ export class BriefError extends Error {} */ export function sliceDiff(text: string, cap: number): { text: string; sliced: boolean; files: number; kept: number; bytes: number; total: number; left: string[] } { const total = Buffer.byteLength(text); - const starts = [...text.matchAll(/^diff --git a\/(\S+)/gm)]; + const starts = [...text.matchAll(/^diff --git (.*)$/gm)]; + const name = (rest: string) => { + const quoted = /^"a\/((?:[^"\\]|\\.)*)"/.exec(rest); + if (quoted) return `"${quoted[1]}"`; + const half = (rest.length - 1) / 2; + if (rest.startsWith('a/') && rest.slice(half) === ` b/${rest.slice(2, half)}`) return rest.slice(2, half); + return rest.startsWith('a/') ? rest.slice(2).split(' b/')[0]! : rest; + }; if (total <= cap || !starts.length) return { text, sliced: false, files: starts.length, kept: starts.length, bytes: total, total, left: [] }; let out = '', bytes = 0; const left: string[] = []; starts.forEach((m, k) => { const section = text.slice(m.index!, k + 1 < starts.length ? starts[k + 1]!.index! : text.length); const n = Buffer.byteLength(section); - if (bytes + n <= cap) { out += section; bytes += n; } else left.push(m[1]!); + if (bytes + n <= cap) { out += section; bytes += n; } else left.push(name(m[1]!)); }); return { text: out, sliced: true, files: starts.length, kept: starts.length - left.length, bytes, total, left }; } diff --git a/test/review-depth-lean.test.ts b/test/review-depth-lean.test.ts index 760fb0971..2859f6738 100644 --- a/test/review-depth-lean.test.ts +++ b/test/review-depth-lean.test.ts @@ -232,6 +232,17 @@ describe('C7 merged reader brief and the outside voice reading its diff from dis expect(sliceNote(s, 100)).toMatch(/2 of 3 files.*big\.json/); expect(sliceDiff(sec('a.ts', 10), 100).sliced).toBe(false); }); + // Value: protects=no file leaves the outside voice's slice unnamed (GSTA-258 L7 Addendum 1.2); fails_when=a git-quoted header + // before the first plain one is dropped unlisted, or glued onto the file before it; seam=sliceDiff headers + test('a git-quoted or spaced filename is its own file in the slice and is named when left out', () => { + const quoted = (p: string, n: number) => `diff --git "a/${p}" "b/${p}"\n+${'x'.repeat(n)}\n`; + const sec = (p: string, n: number) => `diff --git a/${p} b/${p}\n+${'x'.repeat(n)}\n`; + const s = sliceDiff(quoted('t\\303\\251st.sql', 500) + sec('a.ts', 10) + quoted('tab\\there.ts', 10), 100); + expect([s.files, s.kept, s.left]).toEqual([3, 2, ['"t\\303\\251st.sql"']]); + expect(s.text).toContain('tab\\there.ts'); + const spaced = sliceDiff(sec('my notes/big data.csv', 500) + sec('a.ts', 10), 100); + expect(spaced.left).toEqual(['my notes/big data.csv']); + }); test('the outside voice gets a path and a hash, not the pasted diff', () => { const text = renderBrief('outside-voice', { diff_base: sha, diff_file: '/tmp/o/outside-voice.diff', diff_sha256: 'd'.repeat(64), diff_bytes: '1234' }, t); expect(text).toContain('/tmp/o/outside-voice.diff'); From 1aa9046981892434d62a0e9da6a229815167d401 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 19:03:46 +0000 Subject: [PATCH 3/9] review-risk: sealed custody, redaction and outside-model egress code class as security (GST-1177) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/review-risk.ts | 4 +++- test/review-depth-lean.test.ts | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/review-risk.ts b/lib/review-risk.ts index 522f6fcb3..5535a4243 100644 --- a/lib/review-risk.ts +++ b/lib/review-risk.ts @@ -51,7 +51,8 @@ export const SECRET_LINE = [ /\b(?:keychain|keytar|find-generic-password|find-internet-password|secret-tool|git-credential|credential\.helper)\b/i, /\bgh auth token\b/, /(?:^|[\s'"/])\.netrc\b/, ]; -const SECURITY_PATH_TOKENS = ['trust', 'redact', 'redaction', 'redactor', 'sanitize', 'sanitizer', 'neutralize', 'neutralizer', 'consent', 'escape']; +/** Sealed custody (harness-evals#237: scripts/sealed/seal.ts) and egress code are trust boundaries too (GST-1177). */ +const SECURITY_PATH_TOKENS = ['trust', 'redact', 'redaction', 'redactor', 'sanitize', 'sanitizer', 'neutralize', 'neutralizer', 'consent', 'escape', 'seal', 'sealed', 'custody', 'egress']; /** Coarse, linear-time rules (GST-850 B+): any changed line naming the child_process module, any outbound HTTP client call; quantifiers bounded. */ export const SECURITY_LINE = [ /\bshell\s*:\s*true\b/, /\bshell\s*=\s*True\b/, /(? { + const sec = (p: string, lines: string[] = ['export const x = 1;']) => matchRisk({ codePaths: [p], added: { [p]: lines }, scopeAuth: false }).has('security'); + expect(sec('scripts/sealed/seal.ts')).toBe(true); + expect(sec('lib/eval-custody.ts')).toBe(true); + expect(sec('lib/autoplan-lean.ts', ["import { redactFindingSpans } from './redact-engine';"])).toBe(true); + expect(sec('lib/autoplan-lean.ts', [' const redacted = (patch: string) => redactFindingSpans(patch) ?? "";'])).toBe(true); + expect(sec('lib/autoplan-lean.ts', ['const SECRET_NAME = /(^|\\/)(\\.env[^/]*)$/i;'])).toBe(true); + expect(sec('lib/review-x.ts', ["import { runOutsideVoice } from './outside-voice-runner';"])).toBe(true); + expect(sec('lib/autoplan-lean.ts', ['const INLINE_DRAFT_BYTES = 64_000;'])).toBe(false); + expect(sec('lib/seasonal.ts')).toBe(false); + expect(classifyDiff({ paths: ['scripts/sealed/seal.ts'], gateTagged: 0 }).risk).toContain('security'); + }); test('a sqlite file is data even when a script names it', () => { const c = classifyDiff({ paths: [sqlite, 'scripts/load.py'], gateTagged: 0, perPath: [{ path: sqlite, lines: 0, binary: true }, { path: 'scripts/load.py', lines: 4, binary: false }], consumed: [sqlite] }); expect([c.class, c.errors.map(e => e.code), c.runtimeLines]).toEqual(['code', [], 4]); From e449da89d3035edfa8cffd687e8a4b8f3b977d19 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 19:04:59 +0000 Subject: [PATCH 4/9] docs: review key row, review_depth receipt fields, executable and egress classing (GSTA-258 L7 Addendum 2, GST-1177) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- docs/review-policy.md | 8 ++++++-- docs/ship-policy.md | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/review-policy.md b/docs/review-policy.md index 31e24a4b9..335a54795 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -210,7 +210,9 @@ content rules: data and config files (JSON, YAML, TOML, CSV and the like) and an scripts under `docs/`, `results/`, `benchmarks/`, `analysis/` or `notebooks/` are judged by their path, so a `lease_usd` value or an `os.path.join(sys.argv[1], …)` in a stats script names no class (HARN-1, harness-evals#231), and binary data such as -`*.sqlite` is data wherever it sits; a docs line that can print a credential (an +`*.sqlite` is data wherever it sits, unless the file has the exec bit or starts with +`#!`: an executable stays code whatever its name or directory (`scripts/logs/prune`, +a runnable `*.out`); a docs line that can print a credential (an `Authorization:` header, a bearer or key-shaped value, an environment key set to a value, a get or show command for auth config such as `mcp get`) is security (gbrain#8417); code that reserves, binds, leases or settles spend against a ledger, budget or cap (`bindCheck(ledger, …)`, @@ -281,7 +283,9 @@ pairs: usage beside a token or cost word, proxy beside billing, and a client beside a cost or charge word, so a CLI's usage text and a plain proxy stay out (GST-850, from gbrain-evals #289, whose metering proxy and billing clients read `required=none`). **security** is a trust-boundary path (trust, redact, sanitize, neutralize, -consent, escape) or line (`shell: true`, `exec(` and `execSync(`, `eval`, `new Function(`, +consent, escape, and sealed custody or egress: seal, sealed, custody, egress) or line (a +redaction call, an import of a redaction or outside-voice module, a `SECRET` or `CREDENTIAL` +constant set to a regex, since that code decides what reaches an outside model, GST-1177; `shell: true`, `exec(` and `execSync(`, `eval`, `new Function(`, raw HTML sinks, a shell run through argv, recursive deletes), any call to an outbound HTTP client (`fetch`, `http(s).get` or `.request`, axios, got, undici, `requests`, `httpx`, urllib, with no exemption for a literal URL), `new URL` from diff --git a/docs/ship-policy.md b/docs/ship-policy.md index f611346eb..e89310487 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -60,6 +60,7 @@ refuses the file (`POLICY_GSTACK_TOO_OLD`). `validate` ends with | `full_e2e` | `true` \| `false` | `false` | `true`: every merge needs a passing full-gate result bound to a tree equal to the merging one modulo stamps; `gstack-ship-queue ready`, `stamp` and `verify` refuse without one (`QUEUE_GATE_MISSING`): a gate-ahead record, or the lane's `--gate-exit 0 --gate-log ` from its run on the gated head. Needs `min_gstack_version`. | | `review_scope` | `frozen` \| `diff-plus-blocking` \| `all` | `frozen` | The finding scope of `/ship`'s review loop; see [review_scope](#review-scope). `frozen` fixes the diff on pass 1, files blocking findings in unchanged code as issues, and later passes read only the range since the previous pass. `diff-plus-blocking` is the same but fixes a blocking finding in unchanged code in this PR instead of filing it. `all` is the earlier loop: every finding is fixed on every pass. Needs `min_gstack_version` when not `frozen`. | | `review_exclude_paths` | git glob pathspecs | `[]` | Content files `/ship`'s reviewers leave out: data, docs, `.log`, and binaries under a content extension (images, fonts, archives and the like) that do not start with executable magic. Read from the base only. Code (native libraries and every other binary outside the content extensions included), gate paths, agent prose, config, executables, links, `.gstack/` and `.github/` files, gate-tagged, secret-line and named-by-code files stay in review. See [review_exclude_paths](#review-exclude-paths). Needs `min_gstack_version`. | +| `review` | `{ panel?, reader_model?, platform?, risk_paths? }` | `{ all, lane, off, [] }` | Who does `/ship`'s review, read from the base only; see [Who reads](review-policy.md#who-reads-r65-the-base-policys-review-key). `panel`: `all` (today's panel on every class) \| `risk` \| `off` (the opt-in one-reader shapes); `reader_model`: `lane`, `cross-family` or a model id; `platform`: `off` \| `on` (a docs or tests diff with no numeric, results or prereg claims whose PR carries capy-ai[bot]'s review at the exact head skips the readers); `risk_paths`: globs of code paths that count as risk class `policy`. Introduced at 1.94.30.0; needs `min_gstack_version` when set to a non-default value. | | `min_gstack_version` | version or `null` | `null` | The oldest gstack that may read the file; see the compatibility rule above. | | `docs` | `{ caps: [{ path, max_bytes }] }` or `null` | `null` | Documentation files with a byte cap. `/ship` Step 14.5 runs `gstack-ship-receipt docs-caps --base ` (the caps come from `origin/`'s policy, never the branch's copy; `--policy ` is for a repository bootstrapping its policy) before the verification gate and prints one `DOCS_CAP: / ok\|over` line per cap; any `over` (an absent file counts) exits 1 and blocks the ship until the file is trimmed or the cap raised on the base branch (`DOCS_AUDIT_BLOCKED`). At most 64 caps, each path contained. See [the ship receipt](#ship-receipt). Needs `min_gstack_version`. | | `train` | `{ gate_workflow, free_workflow, full_gate_check, record_artifact, max_size, fill_hours, screen_selection, trial_reservation_usd, land? }`, `{ gates, land, max_size, fill_hours }` or `null` | `null` | Turns on [merge trains](#run-a-merge-train), read from the base revision only. Every field is required and an unknown field is `POLICY_INVALID`: `max_size` 1 to 6, `fill_hours` above 0 and at most 24, `screen_selection` `gstack-paid` \| `none`, `trial_reservation_usd` above 0 and at most 100. A block with `gates` ([any repository](#train-gates)) takes `max_size` 1 to 5 and needs `min_gstack_version` at least `TRAIN_GATES_KEY_VERSION`. `null` keeps the per-PR queue exactly as it is. Needs `min_gstack_version`. | @@ -1188,7 +1189,8 @@ repair is trimming the file, never raising the cap in the same PR. **`review_depth` and `gstack_version`.** `write` records the gstack that wrote the receipt and the review depth at the checked-out head ([review-policy.md#review-depth](review-policy.md#review-depth)): -`{ depth, class, runtime_lines, rules, base, registry, gate_scan: { tagged, head }, reasons }`, +`{ depth, class, runtime_lines, rules, base, registry, gate_scan: { tagged, head }, reasons, risk, mode }` +(`mode` is lean, panel or platform; `declared` and `detected` appear with `write --declare-risk`, and the platform depth adds `platform_review`, see [review-policy.md](review-policy.md#review-depth)), computed from git objects for the range against `origin/` (`gstack-diff-scope --head ` for the scope signals), with the gate scan run on that head. `--head` must be the checked-out HEAD From f72b423212b024d5e4b622a1cce12ab6c711bf03 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 19:32:23 +0000 Subject: [PATCH 5/9] review pins: GSTA-258-L7 planner no-loss replay admits the new depth rules; changelog fragment Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gsta258-l7-review-classing.md | 9 +++++++++ test/fixtures/review-pins/files/lib/review-depth.ts.json | 2 +- test/fixtures/review-pins/files/lib/review-risk.ts.json | 2 +- test/fixtures/review-pins/replays/GSTA-258-L7.json | 1 + 4 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 changelog.d/gsta258-l7-review-classing.md create mode 100644 test/fixtures/review-pins/replays/GSTA-258-L7.json diff --git a/changelog.d/gsta258-l7-review-classing.md b/changelog.d/gsta258-l7-review-classing.md new file mode 100644 index 000000000..132d369a2 --- /dev/null +++ b/changelog.d/gsta258-l7-review-classing.md @@ -0,0 +1,9 @@ +**/ship's review depth now follows what the code does, not what a file is named (GST-1177).** An executable file stays code wherever it sits, and code that seals eval custody or decides what reaches an outside model now gets full review. Over the last 200 commits on main, no commit's review depth went down; five went up, including the secret-egress fix in #3921 that had been planned light. + +### Changed +- **An executable is code whatever its name or folder.** A file with the exec bit or a `#!` first line, like `scripts/logs/prune` or a runnable `*.out`, no longer counts as a run log or data. If gstack can't read a file's mode or first bytes, it treats the file as executable. +- **Sealed custody and outside-model egress are security risk.** Paths naming seal, sealed, custody or egress, and code lines that call a redaction function, import a redaction or outside-voice module, or set a `SECRET` or `CREDENTIAL` constant to a regex now plan full depth (harness-evals#237, #3921). + +### Fixed +- **The outside voice's path-scoped slice no longer drops files with quoted or spaced names.** A file git prints with a quoted header, which can happen when the name has a tab or non-ASCII characters, was either glued onto the file before it or silently left out of a diff over 2 MB. Now each one is its own file, and the brief names it when it's left out. +- **docs/ship-policy.md describes the `review` policy key** and the `risk` and `mode` fields of the receipt's `review_depth` record. 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 1ed4cf629..664adda60 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":"fb21c0b80e98685ca8c5192aeaa217cfffe9fbdaa8890455c97e01be1b044a53","replay":"GSTA-258-C+GST-940"} +{"sha":"17617e1ecfc9cf91cd1df564363a67439bb1d451e07e98d38350b79905ec49ec","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 1528c69f7..73a2d5330 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":"e2a4d2aabe02e28140b3b8f3d5288bf0eac1872b5cb375eb011e4f0cde2ad67a","replay":"GSTA-258-C+GST-942"} +{"sha":"31601b6b07f1be146652ed308f130b4a9688bddc08500d94e03696c372c8f67d","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 new file mode 100644 index 000000000..ac34bac75 --- /dev/null +++ b/test/fixtures/review-pins/replays/GSTA-258-L7.json @@ -0,0 +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, redaction or outside-voice imports and SECRET or CREDENTIAL regex constants are security) 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 (GStack Minder drive handoffs/GSTA-258/L7-planner-noloss/)"} From f030b5324907c6701eeade4f569768f5774d9d60 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 19:48:41 +0000 Subject: [PATCH 6/9] fix: pre-landing review fixes (deleted paths are not executables, read two bytes of candidates only, wider egress line rules) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/review-collect.ts | 28 +++++++------------ lib/review-risk.ts | 3 +- .../review-pins/files/lib/review-risk.ts.json | 2 +- .../review-pins/replays/GSTA-258-L7.json | 2 +- test/review-collect.test.ts | 11 ++++++++ test/review-depth-lean.test.ts | 2 ++ 6 files changed, 27 insertions(+), 21 deletions(-) diff --git a/lib/review-collect.ts b/lib/review-collect.ts index ab70cedac..d0452b165 100644 --- a/lib/review-collect.ts +++ b/lib/review-collect.ts @@ -241,30 +241,22 @@ function executablesAmong(perPath: readonly PathLines[], classRoot: string | rea 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(); } -/** The executables among the changed paths at `headSha`: tree mode 100755, or a text blob that starts with "#!"; an unreadable tree or blob counts as executable. */ +/** + * 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. + */ function executablesAt(root: string, headSha: string, perPath: readonly PathLines[], classRoot: readonly string[]): string[] { const tree = git(root, ['ls-tree', '-r', '-z', headSha]); const entries = new Map(tree.out.split('\0').flatMap(rec => { const m = /^(\d+) blob ([0-9a-f]+)\t(.*)$/s.exec(rec); return m ? [[m[3]!, { mode: m[1]!, sha: m[2]! }] as const] : []; })); - const firstBytes = new Map(); - const text = perPath.flatMap(e => { const t = entries.get(e.path); return !e.binary && t && t.mode !== '100755' ? [t.sha] : []; }); - if (text.length) { - const r = spawnSync('git', ['cat-file', '--batch'], { cwd: root, input: text.join('\n') + '\n', timeout: 60_000, maxBuffer: 256 * 1024 * 1024 }); - const out = r.status === 0 && !r.error ? r.stdout : null; - for (let at = 0; out && at < out.length;) { - const nl = out.indexOf(10, at); - const m = nl < 0 ? null : /^([0-9a-f]+) blob (\d+)$/.exec(out.subarray(at, nl).toString('latin1')); - if (!m) break; - firstBytes.set(m[1]!, out.subarray(nl + 1, nl + 3).toString('latin1')); - at = nl + 1 + Number(m[2]) + 1; - } - } return executablesAmong(perPath, classRoot, e => { + if (!tree.ok) return true; const t = entries.get(e.path); - if (!tree.ok || !t) return true; + if (!t) return false; if (t.mode === '100755') return true; if (e.binary) return false; - const head = firstBytes.get(t.sha); - return head === undefined || head === '#!'; + 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'; + return !read || (r.stdout ?? Buffer.alloc(0)).subarray(0, 2).toString('latin1') === '#!'; }); } @@ -336,7 +328,7 @@ export function collectLive(root: string, base: string): Collected { const buf = Buffer.alloc(2); try { fs.readSync(fd, buf, 0, 2, 0); } finally { fs.closeSync(fd); } return buf.toString('latin1') === '#!'; - } catch { return true; } + } catch (err) { return (err as NodeJS.ErrnoException).code !== 'ENOENT'; } }); return { ...result(diff, { root, registry, errors, mergeBase, untracked, consumed, drawings, executables }), base: resolved }; } diff --git a/lib/review-risk.ts b/lib/review-risk.ts index 5535a4243..ef32d4512 100644 --- a/lib/review-risk.ts +++ b/lib/review-risk.ts @@ -68,7 +68,8 @@ export const SECURITY_LINE = [ /(? { + const r = repo({ 'logs/run': 'ok\n', 'r.log': 'ok\n', 'big.txt': 'a'.repeat(1024 * 1024) }); + const del = r.commit({ 'logs/run': null, 'r.log': 'ok2\n', 'big.txt': 'b' + 'a'.repeat(1024 * 1024) }); + const c = collectCommit(r.dir, r.baseSha, del); + expect(c.executables).toEqual([]); + expect(depthOfCommitInput({ ...c, gateTagged: 0 }).class).toBe('docs'); + r.write({ 'r.log': null }); + expect(collectLive(r.dir, 'main').executables).toEqual([]); + }); }); describe('collectLive: the working tree', () => { diff --git a/test/review-depth-lean.test.ts b/test/review-depth-lean.test.ts index 9badbbe60..a1429c6bf 100644 --- a/test/review-depth-lean.test.ts +++ b/test/review-depth-lean.test.ts @@ -48,7 +48,9 @@ describe('HARN-1 false triggers (harness-evals#231 on 1.93.7.0): data, config an expect(sec('lib/autoplan-lean.ts', [' const redacted = (patch: string) => redactFindingSpans(patch) ?? "";'])).toBe(true); expect(sec('lib/autoplan-lean.ts', ['const SECRET_NAME = /(^|\\/)(\\.env[^/]*)$/i;'])).toBe(true); expect(sec('lib/review-x.ts', ["import { runOutsideVoice } from './outside-voice-runner';"])).toBe(true); + for (const l of ['out = redact_secrets(text)', 'from gstack.redact import scrub', "import './outside-voice';", 'SECRET_PATTERNS = [/sk-[a-z]+/]', 'const CREDENTIAL_RE = new RegExp(src);']) expect(sec('lib/x.ts', [l])).toBe(true); expect(sec('lib/autoplan-lean.ts', ['const INLINE_DRAFT_BYTES = 64_000;'])).toBe(false); + expect(sec('lib/x.ts', ['const SECRET_COUNT = total / 2;', "import { readFile } from 'node:fs';"])).toBe(false); expect(sec('lib/seasonal.ts')).toBe(false); expect(classifyDiff({ paths: ['scripts/sealed/seal.ts'], gateTagged: 0 }).risk).toContain('security'); }); From b8d991bf7d74bb64d776cd6120b715910d86a3b8 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 20:13:26 +0000 Subject: [PATCH 7/9] fix: Step 11 review fixes (executables and symlinks are code with their lines read whatever their docs, data or config name; never open a symlink) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/review-collect.ts | 13 +++++++------ lib/review-depth.ts | 6 +++--- lib/review-risk.ts | 7 +++++-- .../review-pins/files/lib/review-depth.ts.json | 2 +- .../review-pins/files/lib/review-risk.ts.json | 2 +- .../review-pins/replays/GSTA-258-L7.json | 2 +- test/review-collect.test.ts | 17 +++++++++++++++++ 7 files changed, 35 insertions(+), 14 deletions(-) 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', () => { From c33df57a94e04745a6ba24ad66d835ac4dfb7196 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 20:35:19 +0000 Subject: [PATCH 8/9] fix: second review exception (docs credential check reads every docs path whatever its class; the live SVG reader never opens a symlink or non-regular file) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/review-collect.ts | 2 +- lib/review-depth.ts | 2 +- .../review-pins/files/lib/review-depth.ts.json | 2 +- test/fixtures/review-pins/replays/GSTA-258-L7.json | 2 +- test/review-collect.test.ts | 12 ++++++++++++ test/review-depth-lean.test.ts | 3 +++ 6 files changed, 19 insertions(+), 4 deletions(-) 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); From 6745d97b802931a68d4df3cd955bc7696513f58d Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 20:43:32 +0000 Subject: [PATCH 9/9] changelog: L7 fragment matches the final change Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gsta258-l7-review-classing.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/changelog.d/gsta258-l7-review-classing.md b/changelog.d/gsta258-l7-review-classing.md index 132d369a2..2e0c42753 100644 --- a/changelog.d/gsta258-l7-review-classing.md +++ b/changelog.d/gsta258-l7-review-classing.md @@ -1,9 +1,11 @@ -**/ship's review depth now follows what the code does, not what a file is named (GST-1177).** An executable file stays code wherever it sits, and code that seals eval custody or decides what reaches an outside model now gets full review. Over the last 200 commits on main, no commit's review depth went down; five went up, including the secret-egress fix in #3921 that had been planned light. +**/ship's review depth now follows what the code does, not what a file is named (GST-1177).** An executable file stays code wherever it sits and whatever its name, and code that seals eval custody or decides what reaches an outside model now gets full review. Over the last 200 commits on main, no commit's review depth went down. Six gained a risk class, and three of those went from light to full, including the secret-egress fix in #3921 that had been planned light. ### Changed -- **An executable is code whatever its name or folder.** A file with the exec bit or a `#!` first line, like `scripts/logs/prune` or a runnable `*.out`, no longer counts as a run log or data. If gstack can't read a file's mode or first bytes, it treats the file as executable. -- **Sealed custody and outside-model egress are security risk.** Paths naming seal, sealed, custody or egress, and code lines that call a redaction function, import a redaction or outside-voice module, or set a `SECRET` or `CREDENTIAL` constant to a regex now plan full depth (harness-evals#237, #3921). +- **An executable is code whatever its name or folder.** A file with the exec bit, a symlink, or a file whose first line is `#!` counts as code. Examples are `scripts/logs/prune`, a runnable `*.out`, or an executable `.txt` or `.json` script. It no longer counts as a run log, data, config or docs, and its changed lines are read for risk. If gstack can't read a file's mode or first bytes, it treats the file as executable. A deleted file is never counted as executable. +- **Sealed custody and outside-model egress are security risk.** Paths naming seal, sealed, custody or egress now plan full depth (harness-evals#237, #3921). So do code lines that call a redaction function, import a redaction or outside-voice module, or set a `SECRET` or `CREDENTIAL` constant to a regex. +- **The docs credential check reads every docs path**, including docs-named executables, so a script that prints an `Authorization:` header still plans full depth. ### Fixed -- **The outside voice's path-scoped slice no longer drops files with quoted or spaced names.** A file git prints with a quoted header, which can happen when the name has a tab or non-ASCII characters, was either glued onto the file before it or silently left out of a diff over 2 MB. Now each one is its own file, and the brief names it when it's left out. +- **The outside voice's path-scoped slice no longer drops files with quoted or spaced names.** Git prints a quoted header when a name has a tab or non-ASCII characters. Such a file was either glued onto the file before it or left out of a diff over 2 MB without being listed. Now each one is its own file, and the brief names it when it's left out. +- **`gstack-review-plan` never opens a tracked symlink** to check for `#!` or to read an SVG drawing, so a link to a FIFO can't hang it. - **docs/ship-policy.md describes the `review` policy key** and the `risk` and `mode` fields of the receipt's `review_depth` record.