diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index 16bf2fbc6..4dddc1222 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -284,29 +284,50 @@ const mergeBaseWith = (base: string) => [`origin/${base}`, base].map(ref => git( const readerLine = (m: Manifest, r: PassReader) => `READER: ${r.reader} stage=${r.stage} state=${r.state} title="${r.title}"${r.sha256 ? ` sha256=${r.sha256} bytes=${r.bytes} chunks=${r.chunks}` : ''}${r.path ? ` path=${r.path}` : ''}`; /** Each selection is read once per process: the ceiling check and the outside voice's embedded diff share it. */ const selections = new Map(); -const selectionOf = (base: string, exclude: string | undefined, keep: readonly string[] = []) => { - const key = JSON.stringify([base, exclude ?? '', keep]); - if (!selections.has(key)) selections.set(key, readSelection(process.cwd(), base, parseTail(exclude), keep)); +const selectionOf = (base: string, exclude: string | undefined, keep: readonly string[] = [], excerpt: readonly string[] = []) => { + const key = JSON.stringify([base, exclude ?? '', keep, excerpt]); + if (!selections.has(key)) selections.set(key, readSelection(process.cwd(), base, parseTail(exclude), keep, excerpt)); return selections.get(key)!; }; -const branchDiff = (base: string, r?: Pick) => { - const d = selectionOf(base, r?.exclude, r?.keep); +const branchDiff = (base: string, r?: Pick) => { + const d = selectionOf(base, r?.exclude, r?.keep, r?.excerpt); if (!d.ok) throw new BriefError(`git diff ${base} failed`); return d.text; }; /** The ceiling over every reader about to render: the first one over or unreadable stops the step before it writes anything else. */ -function ceiling(context: string, base: string, outDir: string, readers: ReadonlyArray & { diff_base?: string }>, diffBase: string, known: PlanExclude | undefined): { exit: number; line: string } | null { +function ceiling(context: string, base: string, outDir: string, readers: ReadonlyArray & { diff_base?: string }>, diffBase: string, known: PlanExclude | undefined): { exit: number; line: string } | null { for (const r of readers) { if (r.prompt_file) continue; const at = r.diff_base ?? diffBase; - const d = selectionOf(at, r.exclude, r.keep); + const d = selectionOf(at, r.exclude, r.keep, r.excerpt); if (!d.ok) return { exit: EXIT.fail, line: unreadableLine(context, r.reader, d, at, base, outDir) }; if (d.bytes > REVIEW_DIFF_MAX_BYTES && r.reader !== 'outside-voice') return { exit: EXIT.refused, line: tooLargeLine(context, r.reader, d, ceilingContext(process.cwd(), base, known)).line }; } return null; } +/** + * Each reader's secret-line excerpt, written once beside the briefs under its sha256 (the brief names the file, its bytes and + * hash); a file the excerpt could not be built for moves to the reader's keep list, so its whole diff reaches the reader. + */ +function settleExcerpts(m: Manifest): void { + for (const r of m.readers) { + if (!r.excerpt?.length) continue; + const d = selectionOf(r.diff_base ?? m.diff_base, r.exclude, r.keep, r.excerpt); + if (!d.ok) throw new BriefError(`git diff ${r.diff_base ?? m.diff_base} failed for ${r.reader}'s secret-line excerpt`); + const whole = r.excerpt.filter(s => d.whole?.includes(s)); + if (whole.length) r.keep = [...(r.keep ?? []), ...whole]; + r.excerpt = r.excerpt.filter(s => !whole.includes(s)); + if (!r.excerpt.length || !d.excerpt) { delete r.excerpt; continue; } + const sha256 = createHash('sha256').update(d.excerpt).digest('hex'); + const file = path.join(m.out, 'briefs', `secret-excerpt-${sha256.slice(0, 16)}.md`); + fs.mkdirSync(path.dirname(file), { recursive: true, mode: 0o700 }); + fs.writeFileSync(file, d.excerpt, { mode: 0o600 }); + r.excerpt_file = { file, sha256, bytes: Buffer.byteLength(d.excerpt), files: r.excerpt.length }; + } +} + function pass(): number { const base = flag('--base'), out = flag('--out'), given = flag('--pass'), id = given?.toLowerCase(); if (!base || !out || !id) { process.stderr.write('gstack-review-dispatch pass: needs --base , --out and --pass \n'); return EXIT.usage; } @@ -371,6 +392,7 @@ function pass(): number { const over = ceiling('gstack-review-dispatch pass', base, m.out, m.readers, m.diff_base, (m.review_exclude as PlanExclude) ?? null); if (over) { process.stdout.write(planLines.join('\n') + '\n'); process.stderr.write(over.line + '\n'); return over.exit; } fs.mkdirSync(m.out, { recursive: true, mode: 0o700 }); + settleExcerpts(m); const t = sectionTexts(ROOT); renderStage(m, 'first', t, branchDiff); renderStage(m, 'specialists', t, branchDiff); diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index 43567624e..0638d0e1b 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -224,12 +224,14 @@ function main(): number { if (d.role === 'fix-check') continue; d.exclude.push(...forbidSpecs(readers).filter(s => !d.exclude.includes(s))); if (d.keep && forbidden.length) d.keep = d.keep.filter(k => !forbiddenPath(forbidden, k.replace(/^:\(top,literal\)/, ''))); + const excerpt = active?.excerpt.filter(k => d.keep?.includes(k)) ?? []; + if (excerpt.length) Object.assign(d, { excerpt }); } 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, readers, 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'); + ...(exclusion.state === 'head-only' ? { review_exclude: exclusion } : active ? { review_exclude: { state: 'active', ...active.record, specs: active.specs, keep: active.keep, excerpt: active.excerpt, kept_files: active.kept, content: active.content, references: active.references } } : {}) }) + '\n'); return EXIT.ok; } const out = [`REVIEW_CLASS: ${plan.class} (${plan.reasons.slice(0, 3).join('; ') || 'no signals'})`, reviewDepthLine(plan), `GATE_SCAN: ${gateTagged === null ? 'unavailable' : `tagged=${gateTagged}`} head=${head}`, `REVIEW_BASE: ${ref} ${baseSha.slice(0, 12)}`, diff --git a/changelog.d/capy-gst-1447-secret-line-excerpts.md b/changelog.d/capy-gst-1447-secret-line-excerpts.md new file mode 100644 index 000000000..08940e385 --- /dev/null +++ b/changelog.d/capy-gst-1447-secret-line-excerpts.md @@ -0,0 +1,3 @@ +### Fixed + +- **Results PRs that capture model requests no longer stop at the review ceiling just because their captured prompts mention credentials.** A file under `review_exclude_paths` stays in review when a changed line reads a credential, and until now every such file went to each reader whole. On harness-evals #269, 75 captured requests whose system prompts name `${…_API_KEY}` variables and keychain commands added 12.9 MB, and `/ship` refused with `REVIEW_DIFF_TOO_LARGE` against the 2 MB limit. When the secret line is the only reason a file is kept, `gstack-review-dispatch pass` now gives readers an excerpt instead. The excerpt holds every changed line the secret-line rule matches or the credential scanner finds a secret-shaped value on, with up to 3 lines of context, the path and the match counts. Only the excerpt counts toward the limit. A file kept for any other reason (a program names it, it is outside the applied patterns, the gate or name scan could not run) is still shown whole, and so is any file whose excerpt cannot be built. See [secret-line excerpts](docs/ship-policy.md#secret-line-excerpts). diff --git a/docs/review-policy.md b/docs/review-policy.md index d1fd82dce..058030698 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -517,7 +517,9 @@ every reader (specialists, the Red Team, the native and outside voices, the targeted reader) reads the same selection: its `DIFF_CMD:` with one `:(top,exclude,glob)` item per applied pattern, the fixture command with the same items, and the `DIFF_KEEP:` command for the kept files under those -patterns. The outside voice's embedded diff is that selection. Each brief says +patterns. A kept file whose only reason is a secret line reaches pass readers as +an excerpt of its matched lines in context instead ([secret-line +excerpts](ship-policy.md#secret-line-excerpts)). The outside voice's embedded diff is that selection. Each brief says how many files and bytes were left out and that the reader must not report on their absence. Excluded paths count as docs for the depth and never trigger the numeric-claim scan or a targeted class; kept files keep their own class. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 076c95b90..20a621f68 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1415,7 +1415,7 @@ Everything else stays in every reader's diff with one reason code: | `config` | it is a data-shaped config file (`package.json`, `tsconfig*.json`, `config/`, dotfiles, `scripts/harness-pins/`) | | `gate-tagged` | the gate scan tagged one of its hunks | | `code` | it is code or a test, not content; a binary file outside the content extensions above, or one with executable magic, counts as code | -| `secret-line` | an added line reads a credential (the targeted pass's secret rule); for a file git calls binary only because of its attributes, the added lines come from `git diff --text`, which `.gitattributes` cannot change (readers' own diffs still follow the attributes and show the kept file as binary) | +| `secret-line` | an added line reads a credential (the targeted pass's secret rule); for a file git calls binary only because of its attributes, the added lines come from `git diff --text`, which `.gitattributes` cannot change (readers' own diffs still follow the attributes and show the kept file as binary). When this is the only reason, pass readers get an excerpt, not the whole file ([secret-line excerpts](#secret-line-excerpts)) | | `named-by-code` | a code or test file that stays in review (not docs, a data file, a `.log` or binary, and not itself excluded) names its full repository path, at the head or the merge base | | `consumer-scan-unavailable` | the `git grep` for those names failed, so every candidate is kept; also a live binary file whose bytes could not be read | | `gate-scan-unavailable` | the gate scan failed or did not run at the head, so no hunk's tag is known and every candidate is kept | @@ -1446,6 +1446,25 @@ pass the bound is not applied, and the plan line says so as on this branch matching . They are not in your diff; do not report on their absence.` + +**Secret-line excerpts (GST-1447).** A kept file whose only reason is +`secret-line` (under an applied pattern, not named by code, with the gate scan +and the name scan both read) still appears whole in `DIFF_KEEP:`, but +`gstack-review-dispatch pass` gives its readers an excerpt instead: every changed +line the secret-line rule matches or the credential scanner finds a secret-shaped +value on, with up to 3 lines of context, the path and the match counts. A line +over 400 characters shows 160 characters on each side of each match. The excerpt +is read with `git diff --text`, written once beside the briefs, and each brief +names its file, bytes and sha256; the outside voice's supplied diff ends with it. +Only the excerpt counts toward the review ceiling. The plan prints +`REVIEW_EXCLUDE_EXCERPT: kept files …` when any file qualifies. Building +it fails closed per file: a section it cannot tie to its path, one with no +matched added line, and one whose hunks or scanner result cannot be read (a +single changed line over the scanner's 1 MiB cap, for example) go to the reader +whole through its keep command. Results PRs that capture model requests need +this: their system prompts name `${…_API_KEY}` variables and keychain commands +(harness-evals #269: 75 such files, 12.9 MB whole). + **Depth, numbers and the targeted pass.** Excluded paths count as docs for the review depth everywhere it is computed (`gstack-review-plan`, the receipt writer, carry and the queue), and their lines never trigger the numeric-claim diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 21b94bbb8..60dc35525 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3532,7 +3532,10 @@ the base policy commit. files need more than 24,000 argument bytes. Narrow the pattern, move the kept files out of it, or split the PR. - **Still over after exclusion:** add the printed folders to the base policy, or - split the PR. + split the PR. Files kept only for a secret line already count as their + excerpts ([secret-line excerpts](ship-policy.md#secret-line-excerpts)); one in + `DIFF_KEEP:` that still counts whole is kept for another reason or could not + be excerpted. There is no override flag. See [the review ceiling](ship-policy.md#review-diff-ceiling). diff --git a/lib/review-brief.ts b/lib/review-brief.ts index 6cb01ce7c..7e30942c7 100644 --- a/lib/review-brief.ts +++ b/lib/review-brief.ts @@ -17,7 +17,8 @@ * With the base policy's review_exclude_paths active (lib/review-exclude.ts) every role but fix-check also takes * policy_exclude (the `:(top,exclude,glob)` items, which the fixture-handling command of the security specialist, the * Red Team and the native adversarial pass gets), keep (the kept files' `:(top,literal)` items) and disclosure (the one - * line naming what was left out). Unset, every brief renders byte for byte as before. The base policy's readers.forbid_paths + * line naming what was left out), and excerpt_file, excerpt_bytes, excerpt_sha256 and excerpt_files (the secret-line excerpt + * gstack-review-dispatch pass wrote for the kept files whose only reason is a secret line). Unset, every brief renders byte for byte as before. The base policy's readers.forbid_paths * (lib/review-fetch.ts) arrive the same way as policy_exclude items, without a disclosure. */ import { spawnSync } from 'node:child_process'; @@ -131,7 +132,8 @@ export function renderBrief(role: BriefRole, slots: Slots, t: SectionTexts): str for (const k of keys) held[k] = `\u0000${nonce}:${k}\u0000`; const rendered = renderRole(role, held, t); const keep = slots.keep ? ` Read the files kept despite a match as well: \`git diff ${slots.diff_base} -- ${slots.keep}\`.` : ''; - const text = slots.disclosure && role !== 'fix-check' && role !== 'outside-voice' ? `${rendered}\n\n${slots.disclosure}${keep}` : rendered; + const excerpt = slots.excerpt_file ? ` ${need(slots, 'excerpt_files')} kept files have no keep reason but a changed line that reads a credential, so no git diff command above shows them: read every such line, with context, in the file ${slots.excerpt_file} (${need(slots, 'excerpt_bytes')} bytes, sha256 ${need(slots, 'excerpt_sha256')}).` : ''; + const text = slots.disclosure && role !== 'fix-check' && role !== 'outside-voice' ? `${rendered}\n\n${slots.disclosure}${keep}${excerpt}` : rendered; const left = role === 'fix-check' || role === 'outside-voice' ? [] : PLACEHOLDERS.filter(p => text.includes(p)); if (left.length) throw new BriefError(`${role}: unfilled placeholder ${left.join(', ')}`); return keys.reduce((out, k) => out.split(held[k]!).join(slots[k]!.trim()), text); diff --git a/lib/review-exclude.ts b/lib/review-exclude.ts index c5be689c3..707167f7e 100644 --- a/lib/review-exclude.ts +++ b/lib/review-exclude.ts @@ -13,6 +13,10 @@ * git exclude beats any positive pathspec. That second command is bounded by its argument * bytes: a pattern whose kept files would pass KEEP_ARG_MAX_BYTES is not applied. * + * A file kept only for a secret line (no other reason, under an applied pattern, gate scan and consumer scan both read) is + * also listed in `excerpt`: pass readers get its matched lines in context (lib/review-secret-excerpt.ts, GST-1447) instead + * of the whole file, and readSelection counts only that excerpt. + * * readSelection reads one reader's diff once, with a timeout, and counts its bytes; * gstack-review-dispatch stops with REVIEW_DIFF_TOO_LARGE above REVIEW_DIFF_MAX_BYTES and * with REVIEW_DIFF_UNREADABLE when git fails, before it writes a manifest or a brief. @@ -25,6 +29,7 @@ import * as path from 'node:path'; import { collectLive } from './review-collect'; import { DATA_PATH, classifyPath, isAgentProse, isDataConfig, isGatePath, type PathLines } from './review-depth'; import { secretPath } from './review-risk'; +import { secretExcerpt } from './review-secret-excerpt'; import { secretLine } from './review-targeted'; import { cmpVersion, parseVersion } from './version-source'; import { REVIEW_EXCLUDE_KEY_VERSION, SHIP_POLICY_PATH, isUnavailable, policyUnavailableLine } from './ship-policy'; @@ -49,6 +54,8 @@ export interface ExcludeRecord { } export interface ActiveExclusion { state: 'active'; record: ExcludeRecord; excluded: Set; specs: string[]; keep: string[]; + /** The kept files (a subset of keep, as `:(top,literal)` items) whose only reason is a secret line: pass readers get an excerpt. */ + excerpt: string[]; kept: Array<{ path: string; reason: KeptReason }>; content: Record; references: Array<{ prefix: string; files: string[] }>; } export type Exclusion = @@ -341,7 +348,8 @@ export function reviewExclude(i: ExcludeInput): Exclusion { const reasons = new Map(); for (const [p, m] of sel.files) reasons.set(p, unread.has(p) ? 'consumer-scan-unavailable' : pathReason(text, p, m, binary, native)); const candidates = new Set([...reasons].filter(([, r]) => r === null).map(([p]) => p)); - const named = grepFixed(i, [...candidates].filter(p => !p.includes('\n')), candidates, [i.mergeBase]); + const secretOnly = [...reasons].filter(([p, r]) => r === 'secret-line' && !p.includes('\n')).map(([p]) => p); + const named = grepFixed(i, [...candidates, ...secretOnly].filter(p => !p.includes('\n')), new Set([...candidates, ...secretOnly]), [i.mergeBase]); for (const p of candidates) { if (i.gateTagged === null) reasons.set(p, 'gate-scan-unavailable'); else if (named === null || p.includes('\n')) reasons.set(p, 'consumer-scan-unavailable'); @@ -360,6 +368,7 @@ export function reviewExclude(i: ExcludeInput): Exclusion { for (const f of kept) keep.add(f); } const excluded = new Set(applied.flatMap(p => (sel.byPattern.get(p) ?? []).filter(f => reasons.get(f) === null))); + const excerpt = i.gateTagged === null || named === null ? [] : secretOnly.filter(p => keep.has(p) && !named.has(p)).sort(); const keptList = [...reasons].filter(([, r]) => r !== null).map(([p, r]) => ({ path: p, reason: r! })).sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)); const sortedExcluded = [...excluded].sort(); const content: Record = { binary: 0, data: 0, docs: 0, log: 0 }; @@ -374,7 +383,7 @@ export function reviewExclude(i: ExcludeInput): Exclusion { gate_tagged: keptList.filter(k => k.reason === 'gate-tagged').map(k => k.path), patterns_sha256, list_sha256, digest: sha256(`${patterns_sha256}\n${list_sha256}`), }; return { - state: 'active', record, excluded, specs: applied.map(p => `:(top,exclude,glob)${p}`), keep: [...keep].sort().map(literal), kept: keptList, content, + state: 'active', record, excluded, specs: applied.map(p => `:(top,exclude,glob)${p}`), keep: [...keep].sort().map(literal), excerpt: excerpt.map(literal), kept: keptList, content, references: prefixes.filter(p => refs.has(p)).map(p => ({ prefix: p, files: [...refs.get(p)!].sort() })), }; } @@ -397,6 +406,7 @@ export function excludeLines(x: Exclusion, base: string): string[] { const out = [`REVIEW_EXCLUDE: patterns=${r.applied.length}/${r.patterns.length} excluded=${r.excluded} bytes=${r.excluded_bytes} kept=${r.kept} content=${Object.entries(x.content).filter(([, n]) => n).map(([k, n]) => `${k}:${n}`).join(',') || 'none'} source=${r.source} digest=${r.digest}${skipped}${r.unmatched.length ? ` unmatched=${r.unmatched.join(',')}` : ''}`]; for (const k of x.kept.slice(0, 20)) out.push(`REVIEW_EXCLUDE_KEPT: ${k.reason} ${k.path}`); if (x.kept.length > 20) out.push(`REVIEW_EXCLUDE_KEPT: +${x.kept.length - 20} more (gstack-review-plan --json lists every kept file)`); + if (x.excerpt.length) out.push(`REVIEW_EXCLUDE_EXCERPT: ${x.excerpt.length} kept files hold no keep reason but a secret line; gstack-review-dispatch pass gives readers their matched lines in context, not the whole files (DIFF_KEEP still lists them whole)`); for (const ref of x.references) out.push(`REVIEW_EXCLUDE_REFERENCE: ${ref.prefix} is named by ${ref.files.slice(0, 3).join(', ')}${ref.files.length > 3 ? ` (+${ref.files.length - 3} more)` : ''}; the files stay excluded, so readers see that code without the files it reads (REVIEW_EXCLUDE_REFERENCE)`); return out; } @@ -416,29 +426,46 @@ export function parseTail(tail: string | undefined): string[] { } export type ReaderDiff = - | { ok: true; text: string; bytes: number; byDir: Array<[string, number]> } + | { ok: true; text: string; bytes: number; byDir: Array<[string, number]>; excerpt?: string; whole?: string[] } | { ok: false; reason: 'timeout' | 'git-exit'; exit: number | null; stderr: string; timeoutMs: number }; -/** One reader's selection read once: `git diff [-- pathspecs]`, then the kept-file command, with its bytes per top directory. */ -export function readSelection(root: string, diffBase: string, pathspecs: readonly string[], keep: readonly string[] = [], timeoutMs = REVIEW_DIFF_TIMEOUT_MS): ReaderDiff { +/** The excerpt of one `git diff --text` over the excerpt files; any throw sends every file whole (fail closed). */ +function excerptOf(out: string, specs: readonly string[]): { text: string; whole: string[]; wholeText: string; files: Array<{ path: string; text: string }> } { + const paths = specs.map(s => s.replace(/^:\(top,literal\)/, '')); + try { + if (specs.some(s => !s.startsWith(':(top,literal)'))) throw new Error('an excerpt item is not a literal pathspec'); + const x = secretExcerpt(out, paths); + return { text: x.text, files: x.files, whole: x.whole.map(literal), wholeText: x.wholeText }; + } catch { return { text: '', files: [], whole: [...specs], wholeText: out }; } +} + +/** + * One reader's selection read once: `git diff [-- pathspecs]`, then the kept-file command, with its bytes per top + * directory. `excerpt` files (ActiveExclusion.excerpt) are read with `git diff --text` and count as their secret-line + * excerpt, appended to `text` and returned as `excerpt`; a file the excerpt could not be built for counts whole, its + * section joins `text` and its item is listed in `whole` for the caller's keep command. + */ +export function readSelection(root: string, diffBase: string, pathspecs: readonly string[], keep: readonly string[] = [], excerpt: readonly string[] = [], timeoutMs = REVIEW_DIFF_TIMEOUT_MS): ReaderDiff { let text = ''; - for (const specs of [pathspecs, ...(keep.length ? [keep] : [])]) { + let ex: ReturnType | undefined; + const reads: Array<[readonly string[], string[]]> = [[pathspecs, []], ...(keep.length ? [[keep, []] as [readonly string[], string[]]] : []), + ...(excerpt.length ? [[excerpt, ['--text', '--no-textconv', '--src-prefix=a/', '--dst-prefix=b/']] as [readonly string[], string[]]] : [])]; + for (const [specs, flags] of reads) { const tail = specs.length ? (specs[0] === '--' ? specs : ['--', ...specs]) : []; - const r = spawnSync('git', ['-c', 'core.quotePath=false', 'diff', '--no-color', '--no-ext-diff', diffBase, ...tail], { cwd: root, encoding: 'utf8', timeout: timeoutMs, maxBuffer: MAX_BUFFER }); + const r = spawnSync('git', ['-c', 'core.quotePath=false', 'diff', '--no-color', '--no-ext-diff', ...flags, diffBase, ...tail], { cwd: root, encoding: 'utf8', timeout: timeoutMs, maxBuffer: MAX_BUFFER }); const code = (r.error as NodeJS.ErrnoException | undefined)?.code; if (code === 'ENOBUFS') { text += r.stdout ?? ''; break; } if (code === 'ETIMEDOUT' || (r.signal && r.status === null)) return { ok: false, reason: 'timeout', exit: null, stderr: r.stderr ?? '', timeoutMs }; if (r.status !== 0) return { ok: false, reason: 'git-exit', exit: r.status, stderr: r.stderr ?? '', timeoutMs }; - text += r.stdout; + if (flags.length) { ex = excerptOf(r.stdout, specs); text += ex.wholeText; } else text += r.stdout; } const byDir = new Map(); + const add = (file: string, bytes: number) => { const dir = file.includes('/') ? file.split('/')[0]! : '(root)'; byDir.set(dir, (byDir.get(dir) ?? 0) + bytes); }; const starts = [...text.matchAll(/^diff --git a\/(\S+)/gm)]; - starts.forEach((m, k) => { - const end = k + 1 < starts.length ? starts[k + 1]!.index! : text.length; - const dir = m[1]!.includes('/') ? m[1]!.split('/')[0]! : '(root)'; - byDir.set(dir, (byDir.get(dir) ?? 0) + Buffer.byteLength(text.slice(m.index!, end))); - }); - return { ok: true, text, bytes: Buffer.byteLength(text), byDir: [...byDir].sort((a, b) => b[1] - a[1]) }; + starts.forEach((m, k) => add(m[1]!, Buffer.byteLength(text.slice(m.index!, k + 1 < starts.length ? starts[k + 1]!.index! : text.length)))); + for (const f of ex?.files ?? []) add(f.path, Buffer.byteLength(f.text)); + const all = ex?.text ? `${text}${text && !text.endsWith('\n') ? '\n' : ''}${ex.text}` : text; + return { ok: true, text: all, bytes: Buffer.byteLength(all), byDir: [...byDir].sort((a, b) => b[1] - a[1]), ...(ex ? { excerpt: ex.text, whole: ex.whole } : {}) }; } /** Credential-bearing remote URLs and scp-style remotes out of git's stderr. */ diff --git a/lib/review-pass.ts b/lib/review-pass.ts index 5c233d637..5bbf4b22f 100644 --- a/lib/review-pass.ts +++ b/lib/review-pass.ts @@ -41,6 +41,8 @@ export interface PassReader { file?: string; sha256?: string; bytes?: number; chunks?: number; findings_sha256?: string; /** The base policy's `:(top,exclude,glob)` items (review_exclude_paths and readers.forbid_paths), shell-quoted (the fixture command's slot), and the kept files' `:(top,literal)` items. */ policy_exclude?: string; keep?: string[]; + /** The kept files whose only reason is a secret line (lib/review-secret-excerpt.ts): readers get the excerpt gstack-review-dispatch pass writes once beside the briefs, not the whole files. */ + excerpt?: string[]; excerpt_file?: { file: string; sha256: string; bytes: number; files: number }; /** The fingerprint the reader's reply must report: the checkout's HEAD^{tree} and toplevel when its brief was rendered (GST-482). */ tree?: string | null; /** What the reader fetched, from its own tool calls (gstack-review-dispatch audit-fetch, lib/review-fetch.ts). */ @@ -128,7 +130,7 @@ const STAGE_OF: Record = { specialist: 'specialists', 'fix-check' /** The manifest for one pass from the plan's dispatches. A fix-check reader needs decision 3.5's files. `voices` keeps only Step 11's readers. */ export function buildManifest(o: { pass: string; out: string; ledger: string; base: string; diffBase: string; root: string; checklistRoot: string; - dispatches: Dispatch[]; next: string; planLines: string[]; learnings?: string | null; extras?: Extra[]; + dispatches: Array; next: string; planLines: string[]; learnings?: string | null; extras?: Extra[]; targetedClasses?: readonly string[]; mergeBase?: string; rangeBase?: string; fixCheck?: FixCheckFiles; reviewExclude?: Record; disclosure?: string; readers?: EffectiveReaders; voices?: boolean; }): Manifest { if (o.voices && o.extras?.length) throw new BriefError(`Step 11's voices pass sends no audit reader, so it takes no --extras: ${o.extras.map(e => e.name).join(', ')}`); @@ -143,8 +145,10 @@ export function buildManifest(o: { const reader = readerKey(role, name); const exclude = d.exclude.length ? diffCommand(d).slice('git diff '.length) : undefined; const policy = d.exclude.filter(s => s.startsWith(':(top,exclude,glob)')); + const excerpt = (d.excerpt ?? []).filter(s => d.keep?.includes(s)); + const keep = (d.keep ?? []).filter(s => !excerpt.includes(s)); readers.push({ reader, stage, title: `ship ${o.pass} ${reader}`, state: 'planned', role, name, effort: d.effort, exclude, checklists: d.checklists, - ...(policy.length ? { policy_exclude: policy.map(shellQuote).join(' ') } : {}), ...(d.keep?.length ? { keep: d.keep } : {}), + ...(policy.length ? { policy_exclude: policy.map(shellQuote).join(' ') } : {}), ...(keep.length ? { keep } : {}), ...(excerpt.length ? { excerpt } : {}), ...(role === 'targeted' ? { classes: [...(o.targetedClasses ?? [])], diff_base: o.mergeBase ?? o.diffBase } : {}), ...(role === 'adversarial-native' && d.classes?.length ? { classes: [...d.classes], diff_base: o.mergeBase ?? o.diffBase } : {}), ...(d.model === 'cross-family' ? { cross_family: true } : d.model && d.model !== 'lane' ? { model: { model_id: d.model } } : {}), @@ -211,7 +215,8 @@ export function renderStage(m: Manifest, stage: Stage, t: SectionTexts, diffFor: const role = r.role!; const slots: Slots = { name: r.name, diff_base: r.diff_base ?? m.diff_base, exclude: r.exclude, stack: m.stack, test_fw: m.test_fw, learnings: m.learnings ?? undefined, checklists: r.checklists?.map(n => path.join(m.checklist_root, 'review/specialists', `${n}.md`)).join(','), - policy_exclude: r.policy_exclude, ...(m.disclosure ? { disclosure: m.disclosure, keep: r.keep?.map(shellQuote).join(' ') } : {}) }; + policy_exclude: r.policy_exclude, ...(m.disclosure ? { disclosure: m.disclosure, keep: r.keep?.map(shellQuote).join(' ') } : {}), + ...(m.disclosure && r.excerpt_file ? { excerpt_file: r.excerpt_file.file, excerpt_bytes: String(r.excerpt_file.bytes), excerpt_sha256: r.excerpt_file.sha256, excerpt_files: String(r.excerpt_file.files) } : {}) }; if (role === 'specialist') slots.checklist = path.join(m.checklist_root, 'review/specialists', `${r.name}.md`); let findingsSha: string | undefined; if (role === 'red-team') { diff --git a/lib/review-secret-excerpt.ts b/lib/review-secret-excerpt.ts new file mode 100644 index 000000000..2d56b8bc5 --- /dev/null +++ b/lib/review-secret-excerpt.ts @@ -0,0 +1,153 @@ +/** + * review-secret-excerpt — what pass readers see of a file review_exclude_paths matches and keeps only because a changed + * line reads a credential (lib/review-exclude.ts reason secret-line, GST-1447). Results PRs that capture model requests hold + * system prompts naming `${…_API_KEY}` and keychain commands; inlined whole, 75 such files were 12.9 MB against the 2 MB + * reader ceiling (harness-evals #269). The excerpt holds every changed line SECRET_LINE matches or the credential scanner + * finds a secret-shaped value on, with EXCERPT_CONTEXT diff lines around each, the path and the counts, and only the excerpt + * counts toward REVIEW_DIFF_MAX_BYTES. A line longer than EXCERPT_LINE_MAX shows EXCERPT_WINDOW characters on each side of + * every match. It fails closed: a section it cannot tie to one expected path, one with no matched added line (the keep + * reason the excerpt stands in for), and one whose hunks or scanner result cannot be read come back whole, and readSelection + * inlines every file on any other throw. + */ +import { scan } from './redact-engine'; +import { SECRET_LINE } from './review-risk'; + +export const EXCERPT_CONTEXT = 3; +export const EXCERPT_LINE_MAX = 400; +export const EXCERPT_WINDOW = 160; +/** The most bytes one scanner call reads; a single changed line past the scanner's own 1 MiB cap sends its file whole. */ +const SCAN_CHUNK_BYTES = 512 * 1024; +/** Lines each chunk after the first repeats from the one before, so a label the scanner needs nearby is read with its value. */ +const SCAN_OVERLAP_BYTES = 1024; + +/** `text` is the excerpt readers get ('' when no file was excerpted); `whole` lists the paths it could not excerpt and `wholeText` their diff sections. */ +export interface SecretExcerpt { text: string; files: Array<{ path: string; text: string }>; whole: string[]; wholeText: string } + +interface Line { kind: string; text: string; old: number; new: number } +interface Hit { rule: boolean; shaped: boolean; spans: Array<[number, number]> } + +const GLOBAL = SECRET_LINE.map(r => new RegExp(r.source, r.flags.includes('g') ? r.flags : r.flags + 'g')); + +/** The path a section's ---/+++ header names under the a/ and b/ prefixes, or null when git quoted it or printed none. */ +function sectionPath(head: readonly string[]): string | null { + const plus = head.find(l => l.startsWith('+++ ')); + const minus = head.find(l => l.startsWith('--- ')); + const side = plus && plus !== '+++ /dev/null' ? plus.slice(4) : minus && minus !== '--- /dev/null' ? minus.slice(4) : ''; + const p = side.replace(/\t$/, ''); + return /^[ab]\//.test(p) ? p.slice(2) : null; +} + +function hunkLines(body: readonly string[]): Line[] { + const out: Line[] = []; + let old = 0, nu = 0; + for (const l of body) { + if (l.startsWith('@@')) { + const m = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(l); + if (!m) throw new Error(`unreadable hunk header: ${l.slice(0, 80)}`); + old = Number(m[1]); nu = Number(m[2]); + } else if (l[0] === '+') out.push({ kind: '+', text: l.slice(1), old, new: nu++ }); + else if (l[0] === '-') out.push({ kind: '-', text: l.slice(1), old: old++, new: nu }); + else if (l[0] === ' ') out.push({ kind: ' ', text: l.slice(1), old: old++, new: nu++ }); + } + return out; +} + +/** Each changed line SECRET_LINE matches or the scanner finds a secret-shaped value (not LOW) on, with the spans to show. */ +function hits(lines: readonly Line[], file: string): Map { + const out = new Map(); + const at = (k: number) => out.get(k) ?? out.set(k, { rule: false, shaped: false, spans: [] }).get(k)!; + const changed = lines.flatMap((l, k) => (l.kind === ' ' ? [] : [k])); + for (const k of changed) { + for (const r of GLOBAL) { + r.lastIndex = 0; + for (let m = r.exec(lines[k]!.text); m; m = r.exec(lines[k]!.text)) { + const h = at(k); + h.rule = true; + h.spans.push([m.index - EXCERPT_WINDOW, m.index + m[0].length + EXCERPT_WINDOW]); + if (!m[0]) r.lastIndex++; + } + } + } + const size = (k: number) => Buffer.byteLength(lines[k]!.text) + 1; + for (let from = 0; from < lines.length;) { + let to = from, bytes = 0; + while (to < lines.length && (to === from || bytes + size(to) <= SCAN_CHUNK_BYTES)) bytes += size(to++); + const chunk = lines.slice(from, to); + for (const f of scan(chunk.map(l => l.text).join('\n'), { processEnv: {}, sourcePath: file }).findings) { + if (f.id.startsWith('engine.')) throw new Error(`the credential scanner did not read ${file}: ${f.id}`); + if (f.category !== 'secret' || f.severity === 'LOW') continue; + if (!chunk[f.line - 1]) throw new Error(`a scanner finding in ${file} names line ${f.line} of ${chunk.length}`); + const k = from + f.line - 1; + if (lines[k]!.kind === ' ') continue; + const h = at(k); + h.shaped = true; + h.spans.push([f.col - 1 - EXCERPT_WINDOW, f.col - 1 + 2 * EXCERPT_WINDOW]); + } + if (to >= lines.length) break; + let back = to, overlap = 0; + while (back - 1 > from && overlap < SCAN_OVERLAP_BYTES) overlap += size(--back); + from = back; + } + return out; +} + +/** A line as shown: whole up to EXCERPT_LINE_MAX; past it, the windows around its matches, or its start when it has none. */ +function shown(text: string, h: Hit | undefined): string { + if (text.length <= EXCERPT_LINE_MAX) return text; + if (!h) return `${text.slice(0, EXCERPT_LINE_MAX)} … [${text.length - EXCERPT_LINE_MAX} more characters]`; + const spans: Array<[number, number]> = []; + for (const [a, b] of h.spans.map(([a, b]) => [Math.max(0, a), Math.min(text.length, b)] as [number, number]).sort((x, y) => x[0] - y[0])) { + const last = spans[spans.length - 1]; + if (last && a <= last[1]) last[1] = Math.max(last[1], b); else spans.push([a, b]); + } + const body = spans.map(([a, b]) => text.slice(a, b)).join(' … '); + return `${spans[0]![0] > 0 ? '… ' : ''}${body}${spans[spans.length - 1]![1] < text.length ? ' …' : ''} [a line of ${text.length} characters, shown around each match]`; +} + +/** One file's excerpt block, or null when no added line matches the secret-line rule (the reason the file was kept). */ +function block(file: string, lines: readonly Line[]): string | null { + const found = hits(lines, file); + if (![...found].some(([k, h]) => h.rule && lines[k]!.kind === '+')) return null; + const rows = [...found.keys()].sort((a, b) => a - b); + const windows: Array<[number, number]> = []; + for (const k of rows) { + const [a, b] = [Math.max(0, k - EXCERPT_CONTEXT), Math.min(lines.length - 1, k + EXCERPT_CONTEXT)]; + const last = windows[windows.length - 1]; + if (last && a <= last[1] + 1) last[1] = b; else windows.push([a, b]); + } + const rule = rows.filter(k => found.get(k)!.rule).length; + const changed = lines.filter(l => l.kind !== ' ').length; + const out = [`### ${file}`, `${rows.length} changed lines matched (${rule} by the secret-line rule, ${rows.length - rule} with a secret-shaped value) out of ${changed}; each is shown with up to ${EXCERPT_CONTEXT} lines of context.`]; + for (const [a, b] of windows) { + out.push(`@@ -${lines[a]!.old} +${lines[a]!.new} @@`); + for (let k = a; k <= b; k++) out.push(lines[k]!.kind + shown(lines[k]!.text, found.get(k))); + } + return out.join('\n') + '\n'; +} + +/** + * The excerpt of `diff` (`git diff --text` under the a/ and b/ prefixes) for the expected `paths`. A section whose path + * git quoted, that names no expected path or repeats one, that has no matched added line, or whose hunk header or scanner + * result cannot be read goes whole, and so does any expected path no section was excerpted for. + */ +export function secretExcerpt(diff: string, paths: readonly string[]): SecretExcerpt { + const want = new Set(paths); + const unattributed: string[] = []; + const byFile = new Map(); + for (const section of diff.split(/(?<=\n)(?=diff --git )/).filter(Boolean)) { + const lines = section.split('\n'); + const h = lines.findIndex(l => l.startsWith('@@')); + const file = section.startsWith('diff --git ') ? sectionPath(h < 0 ? lines : lines.slice(0, h)) : null; + if (file === null || !want.has(file)) { unattributed.push(section); continue; } + const prior = byFile.get(file); + if (prior) { prior.sections.push(section); prior.text = null; continue; } + let text: string | null = null; + try { text = h < 0 ? null : block(file, hunkLines(lines.slice(h))); } catch { text = null; } + byFile.set(file, { sections: [section], text }); + } + const files = paths.flatMap(p => { const text = byFile.get(p)?.text; return text ? [{ path: p, text }] : []; }); + const whole = paths.filter(p => !byFile.get(p)?.text); + const wholeParts = [...unattributed, ...whole.flatMap(p => byFile.get(p)?.sections ?? [])]; + const head = `## Secret-line excerpts (${files.length} files)\nThese files match review_exclude_paths and stay in review only because a changed line reads a credential; the rest of each file is left out like any excluded file. Each excerpt shows every changed line that matches the secret-line rule or holds a secret-shaped value, with up to ${EXCERPT_CONTEXT} lines of context; a line over ${EXCERPT_LINE_MAX} characters shows ${EXCERPT_WINDOW} characters on each side of each match. Review them for credentials that are leaked or handled unsafely.\n\n`; + return { text: files.length ? head + files.map(f => f.text).join('\n') : '', files, whole, wholeText: wholeParts.join('') }; +} diff --git a/test/review-secret-excerpt.test.ts b/test/review-secret-excerpt.test.ts new file mode 100644 index 000000000..2dab4c6bf --- /dev/null +++ b/test/review-secret-excerpt.test.ts @@ -0,0 +1,209 @@ +/** + * Secret-line excerpts (GST-1447, docs/ship-policy.md#review-exclude-paths): a file review_exclude_paths matches and keeps + * only because a changed line reads a credential goes to pass readers as every matched line with context, and only that + * excerpt counts toward REVIEW_DIFF_MAX_BYTES. Any other keep reason, and any excerpt that cannot be built, inlines the file. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT } from '../lib/headless-artifacts'; +import { REVIEW_DIFF_MAX_BYTES, readSelection, reviewExclude } from '../lib/review-exclude'; +import { collectCommit } from '../lib/review-collect'; +import { secretExcerpt } from '../lib/review-secret-excerpt'; +import { REVIEW_EXCLUDE_KEY_VERSION } from '../lib/ship-policy'; +import { cleanup, commit, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/pregate-fixture'; + +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); +const policyText = JSON.stringify({ review_exclude_paths: ['docs/benchmarks/**'], min_gstack_version: REVIEW_EXCLUDE_KEY_VERSION }) + '\n'; +const TURNS = 'docs/benchmarks/qm-s1-probe/results/turns'; +/** A credential-shaped value built at run time, so the source holds no scanner match. */ +const plantedKey = (seed: number) => 'gh' + 'p_' + `Zx9Qw3Er5Ty7Ui9Op1As3Df5Gh7Jk9Lz2Xc${seed}`.slice(0, 36); + +/** + * One captured model request in the harness-evals #269 shape: about 4,000 pretty-printed JSON lines (~350 KB) whose system + * prompt lines run past 20 KB and name \`\${…_API_KEY}\`, \`$…_TOKEN\` and keychain commands. Every match carries a marker + * unique to its file and line, so a test can ask for each one. + */ +function capture(k: number, plant: Record = {}): { text: string; markers: string[] } { + const lines = ['{', ' "requests": [']; + const markers: string[] = []; + for (let n = 0; n < 4000; n++) { + if (plant[n]) lines.push(plant[n]!); + else if (n % 800 === 30) { + const env = `\${OPENAI_API_KEY_F${k}_L${n}}`, svc = `svc-f${k}-l${n}`; + markers.push(env, svc); + lines.push(` "system": "${'You are a coding agent. '.repeat(450)}Read ${env} from the environment and never print it. ${'Follow the repo rules. '.repeat(450)}On macOS run security find-generic-password -s ${svc} for the keychain entry.",`); + } else if (n % 800 === 380) { + const env = `$GH_TOKEN_F${k}_L${n}`; + markers.push(env); + lines.push(` "note": "${'context '.repeat(150)}export ${env} before the push",`); + } else lines.push(` "line${n}": "${'x'.repeat(40)} ${n}",`); + } + lines.push(' ]', '}'); + return { text: lines.join('\n') + '\n', markers }; +} + +/** A results branch over the ceiling: ten captures and ten plain session files under docs/benchmarks/** (about 3.5 MB), plus \`extra\`. */ +function results(extra: Record = {}, plant: Record = {}) { + const repo = makeFixtureRepo({ prefix: 'gstack-excerpt-', baseFiles: { '.gstack/ship-policy.json': policyText } }); + repos.push(repo); + const files: Record = { 'lib/run.js': "exports.run = () => 'ok';\n", ...extra }; + const markers: Record = {}; + for (let k = 0; k < 10; k++) { + const c = capture(k, k === 3 ? plant : {}); + files[`${TURNS}/t${k}/llm-turn0.json`] = c.text; + files[`${TURNS}/t${k}/session.json`] = `{"turn": ${k}, "ok": true}\n`; + markers[`${TURNS}/t${k}/llm-turn0.json`] = c.markers; + } + commit(repo, files); + return { repo, markers }; +} +const pass = (repo: FixtureRepo) => { + const out = path.join(repo.root, 'pass'); + const r = runBin(repo, 'gstack-review-dispatch', ['pass', '--base', 'main', '--out', out, '--pass', 'p1', '--', '--gate-tagged', '0']); + const file = path.join(out, 'pass-manifest.json'); + return { r, out, m: fs.existsSync(file) ? JSON.parse(fs.readFileSync(file, 'utf8')) : null }; +}; +const exclusion = (repo: FixtureRepo, gateTagged: ReadonlySet | null = new Set()) => { + const head = repo.git('rev-parse', 'HEAD'); + const baseSha = repo.git('rev-parse', 'origin/main'); + const c = collectCommit(repo.work, baseSha, head); + const x = reviewExclude({ root: repo.work, baseSha, ref: 'origin/main', mergeBase: c.mergeBase!, head, perPath: c.perPath, added: c.added, classRoot: c.root ?? [], gateTagged, context: 'test' }); + if (x.state !== 'active') throw new Error(`expected an active exclusion, got ${x.state}`); + return { x, mergeBase: c.mergeBase! }; +}; +const lit = (p: string) => `:(top,literal)${p}`; +/** The kept files a reader still reads whole: DIFF_KEEP's list less the excerpt. */ +const wholeKeep = (x: { keep: string[]; excerpt: string[] }) => x.keep.filter(k => !x.excerpt.includes(k)); + +describe('files kept only for a secret line go to readers as excerpts', () => { + // Value: protects=a results PR whose captured prompts name credentials reaches review instead of stopping at the ceiling, + // and the secret review still sees every matched line; fails_when=a secret-only kept file is inlined whole (the pass + // refuses REVIEW_DIFF_TOO_LARGE, harness-evals #269: 75 files, 12.9 MB) or a matched line is missing from the excerpt; + // why_new=GST-1447; seam=gstack-review-dispatch pass on a real repository; no_claim=what a model does with the excerpt + test('the #269 shape fits under the ceiling and every match is in the excerpt each brief names', () => { + const { repo, markers } = results(); + const { r, m } = pass(repo); + expect(r.err).not.toContain('REVIEW_DIFF_TOO_LARGE'); + expect(r.status).toBe(EXIT.ok); + const readers = m.readers.filter((x: any) => !x.prompt_file); + expect(readers.length).toBeGreaterThan(0); + for (const reader of readers) { + expect(reader.excerpt).toEqual(Object.keys(markers).sort().map(lit)); + expect(reader.keep ?? []).toEqual([]); + expect(reader.excerpt_file.files).toBe(10); + expect(reader.excerpt_file.bytes).toBeLessThan(REVIEW_DIFF_MAX_BYTES / 4); + } + const excerpt = fs.readFileSync(readers[0].excerpt_file.file, 'utf8'); + expect(Buffer.byteLength(excerpt)).toBe(readers[0].excerpt_file.bytes); + for (const [file, ms] of Object.entries(markers)) { + expect(excerpt).toContain(`### ${file}`); + expect(excerpt).toContain('10 changed lines matched (10 by the secret-line rule, 0 with a secret-shaped value)'); + for (const mark of ms) expect({ file, mark, found: excerpt.includes(mark) }).toEqual({ file, mark, found: true }); + } + const rendered = readers.filter((x: any) => x.file && x.role !== 'outside-voice'); + expect(rendered.length).toBeGreaterThan(0); + for (const x of rendered) expect(fs.readFileSync(x.file, 'utf8')).toContain(`in the file ${x.excerpt_file.file} (${x.excerpt_file.bytes} bytes, sha256 ${x.excerpt_file.sha256})`); + const { x, mergeBase } = exclusion(repo); + const d = readSelection(repo.work, mergeBase, ['--', '.', ...x.specs], wholeKeep(x), x.excerpt); + expect(d.ok && d.bytes).toBeLessThan(REVIEW_DIFF_MAX_BYTES); + expect(d.ok && d.text).toContain('## Secret-line excerpts (10 files)'); + }, 240_000); + + // Value: protects=a real credential in a secret-only kept file still reaches the reviewer, wherever it sits in the file; + // fails_when=the excerpt keeps only SECRET_LINE matches, so a token far from any matched line (or inside a 20 KB line) is + // left out; why_new=GST-1447 (the excerpt replaced the whole file); seam=readSelection on a real repository; + // no_claim=pregate's gitleaks scan, which reads every added line on its own + test('a planted key far from any secret line, short or inside a long line, is in the excerpt', () => { + const short = plantedKey(1), long = plantedKey(2); + const plant = { 1500: ` "auth": "${short}",`, 2000: ` "blob": "${'y'.repeat(9000)} ${long} ${'y'.repeat(9000)}",` }; + const { repo } = results({}, plant); + const { x, mergeBase } = exclusion(repo); + const d = readSelection(repo.work, mergeBase, ['--', '.', ...x.specs], wholeKeep(x), x.excerpt); + if (!d.ok) throw new Error('unreadable'); + expect(d.whole).toEqual([]); + expect(d.excerpt).toContain(short); + expect(d.excerpt).toContain(long); + expect(d.excerpt).toContain(`### ${TURNS}/t3/llm-turn0.json`); + expect(d.excerpt).toContain('12 changed lines matched (10 by the secret-line rule, 2 with a secret-shaped value)'); + }, 240_000); + + // Value: protects=only the secret-line reason earns an excerpt: a file a program names, a file outside the applied patterns, + // and every file under an unknown gate scan stay whole; fails_when=named-by-code is never checked for a secret-line file + // (it is excerpted), a file outside the patterns is excerpted, or an unknown gate scan still excerpts; why_new=GST-1447; + // seam=reviewExclude and gstack-review-dispatch pass on a real repository + test('a secret-line file a program names, one outside the patterns and an unknown gate scan are inlined whole', () => { + const named = `${TURNS}/named/llm-turn0.json`, outside = 'data/llm-turn0.json'; + const secret = (k: number) => capture(k).text; + const { repo, markers } = results({ [named]: secret(20), [outside]: secret(21), 'lib/reader.js': `exports.p = '${named}';\n` }); + const { x } = exclusion(repo); + expect(x.kept.find(k => k.path === named)).toEqual({ path: named, reason: 'secret-line' }); + expect(wholeKeep(x)).toEqual([lit(named)]); + expect(x.keep).toContain(lit(named)); + expect(x.excerpt).toEqual(Object.keys(markers).sort().map(lit)); + expect(exclusion(repo, null).x.excerpt).toEqual([]); + const { r, m } = pass(repo); + expect(r.status).toBe(EXIT.ok); + const reader = m.readers.find((y: any) => !y.prompt_file); + expect(reader.keep).toEqual([lit(named)]); + expect(reader.excerpt).not.toContain(lit(named)); + expect(reader.excerpt).not.toContain(lit(outside)); + const { mergeBase } = exclusion(repo); + const d = readSelection(repo.work, mergeBase, ['--', '.', ...x.specs], wholeKeep(x), x.excerpt); + if (!d.ok) throw new Error('unreadable'); + expect(d.text).toContain(`diff --git a/${named} b/${named}`); + expect(d.text).toContain(`diff --git a/${outside} b/${outside}`); + expect(d.excerpt).not.toContain(`### ${named}`); + }, 240_000); + + // Value: protects=excerpt building fails closed per file: a file whose excerpt cannot be built goes to the reader whole and + // joins the reader's keep command while the others stay excerpted; fails_when=such a file drops out of review, is shown + // cut, or takes the other files' excerpts down with it; why_new=GST-1447; seam=readSelection and gstack-review-dispatch + // pass on a real repository (a minified capture whose one line passes the credential scanner's 1 MiB cap) + test('a file whose excerpt cannot be built falls back to the whole file', () => { + const big = `${TURNS}/big/llm-turn0.json`; + const line = `{"system": "${'You are a coding agent. '.repeat(48000)}Read \${OPENAI_API_KEY_BIG} and use the keychain.", "end": "tail-of-big"}\n`; + const { repo } = results({ [big]: line }); + const { x, mergeBase } = exclusion(repo); + expect(x.excerpt).toContain(lit(big)); + const d = readSelection(repo.work, mergeBase, ['--', '.', ...x.specs], wholeKeep(x), x.excerpt); + if (!d.ok) throw new Error('unreadable'); + expect(d.whole).toEqual([lit(big)]); + expect(d.text).toContain('"end": "tail-of-big"'); + expect(d.excerpt).not.toContain(`### ${big}`); + expect(d.excerpt).toContain('## Secret-line excerpts (10 files)'); + const { r, m } = pass(repo); + expect(r.status).toBe(EXIT.ok); + const reader = m.readers.find((y: any) => !y.prompt_file); + expect(reader.keep).toEqual([lit(big)]); + expect(reader.excerpt).not.toContain(lit(big)); + expect(reader.excerpt_file.files).toBe(10); + }, 240_000); + + // Value: protects=every matched line of an excerpted file reaches readers; fails_when=a CR, U+2028 or U+2029 followed by + // `diff --git ` inside a changed line splits the file's section, so the lines after it leave the excerpt while the file + // stays excluded from the reader's own diff; why_new=GST-1447 /ship red-team finding; seam=none + test('a line separator inside a changed line does not cut the file section short', () => { + for (const sep of ['\r', '\u2028', '\u2029']) { + const pad = Array.from({ length: 20 }, (_, i) => `+filler ${i}`); + const diff = ['diff --git a/r/x.json b/r/x.json', '--- a/r/x.json', '+++ b/r/x.json', '@@ -0,0 +1,23 @@', '+uses ${OPENAI_API_KEY}', ...pad, `+note${sep}diff --git a/zz b/zz`, '+run security find-generic-password -s after-split', ''].join('\n'); + const x = secretExcerpt(diff, ['r/x.json']); + expect(x.whole.includes('r/x.json') || x.text.includes('after-split')).toBe(true); + } + }); + + // Value: protects=a secret the scanner finds only with its nearby label (an AWS secret key) reaches readers; fails_when=the + // scanner reads changed lines alone or in chunks without overlap, so a label on a context line or across a chunk + // boundary hides the value; why_new=GST-1447 /ship outside-voice finding; seam=none + test('a secret whose label sits on a context line or across a scan chunk is in the excerpt', () => { + const value = 'wJ' + 'alrXUtnFEMI/K7MDENG/bPxRfiCYzQ9wQ3eR5t'; + const head = ['diff --git a/r/x.yml b/r/x.yml', '--- a/r/x.yml', '+++ b/r/x.yml']; + const pad = Array.from({ length: 20 }, (_, i) => `+filler ${i}`); + const context = [...head, '@@ -1,1 +1,23 @@', '+uses ${OPENAI_API_KEY}', ...pad, ' aws_secret_access_key:', `+ ${value}`, ''].join('\n'); + expect(secretExcerpt(context, ['r/x.yml']).text).toContain(value); + const fill = Array.from({ length: 511 }, () => `+${'f'.repeat(1023)}`); + const boundary = [...head, '@@ -0,0 +1,515 @@', '+uses ${OPENAI_API_KEY}', ...fill, `+${'x'.repeat(980)} aws_secret_access_key:`, `+${value}`, ''].join('\n'); + expect(secretExcerpt(boundary, ['r/x.yml']).text).toContain(value); + }); +});