Let review exclusions handle big results PRs (GST-1967) (#4184)

* Let review exclusions handle big results PRs (GST-1967)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* Keep the value rule in review-exclude so the pinned review-risk rules stay at their replayed bytes

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* Reword the changelog entry so its credential sentence reads plainly

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
Garry Tanandcapy-ai[bot] authored and GitHub committed 2026-11-09 03:43:59 +00:00
1 parent e077053c3c
commit a229e25806
10 files changed
+193 -36

No files matched your search

+1 -1
View File
@@ -289,7 +289,7 @@ function main(): number {
const key = d.name.split(' ')[0];
out.push(`DISPATCH: ${d.role} ${d.name} effort=${d.effort}${d.model ? ` model=${d.model}` : ''}${d.checklists ? ` checklists=${d.checklists.join(',')}` : ''}${d.classes ? ` classes=${d.classes.join(',')}` : ''}${d.reason ? ` reason=${d.reason}` : ''}`, `DIFF_CMD: ${key} ${diffCommand(d)}`);
if (!d.exclude.length) out.push(`DIFF_EXCLUDE: ${key} none`);
const keepCmd = keepCommand(d);
const keepCmd = keepCommand({ ...d, keep: d.keep?.filter(k => !d.excerpt?.includes(k)) });
if (keepCmd) out.push(`DIFF_KEEP: ${key} ${keepCmd}`);
}
for (const s of plan.skipped) out.push(`SKIP: ${s}`);
@@ -0,0 +1 @@
- **Big results PRs now fit under /ship's review ceiling when their transcripts only name a credential.** Under `review_exclude_paths`, a data file used to stay in review for any line that named a credential, so captured model prompts that mention `${OPENAI_API_KEY}`, `$AGENT_API_TOKEN` or a keychain kept every such file (harness-evals#333: 2,610 files). That many kept files pushed the pattern past the 24,000-byte keep-command limit, so it wasn't applied at all, and readers got over 5 MB against the 2 MB ceiling. A line now keeps its file only when it names a credential and carries a value, meaning the credential scan finds a secret on it or a base64 or token run of 20+ characters follows the name. A key name given a value, such as `OPENAI_API_KEY=sk-…` or `"GH_TOKEN": "…"`, now keeps its file too. Files kept only for a secret line are excerpted, so they no longer count toward the keep-command limit, gstack reads them in git calls under that limit, and `DIFF_KEEP:` leaves them out. On harness-evals#333 the reader's diff drops from 5,458,045 bytes to 1,061,721.
+6 -3
View File
@@ -1445,7 +1445,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). When this is the only reason, pass readers get an excerpt, not the whole file ([secret-line excerpts](#secret-line-excerpts)) |
| `secret-line` | an added line names a credential (the targeted pass's secret rule, or a key name given a value such as `OPENAI_API_KEY=`) and carries a value: the credential scan finds a secret on the line, or a run of 20 or more base64 or token characters mixing upper case, lower case and digits follows a name within 200 characters. A line that only names a variable or a credential store (`${OPENAI_API_KEY}`, `$GH_TOKEN`, `process.env.X`, `os.environ["X"]`, a keychain command) keeps nothing (GST-1967: harness-evals #333's captured prompts kept 2,610 files that way). 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 |
@@ -1471,7 +1471,10 @@ kept files under an applied pattern come back through a second command,
brief names. That command is bounded by its argument bytes (24,000 across all
patterns, under Windows' command-line limit): a pattern whose kept files would
pass the bound is not applied, and the plan line says so as
`skipped=<pattern>:kept=<n>:bytes=<b>`. Every brief also gets one line:
`skipped=<pattern>:kept=<n>:bytes=<b>`. Files that reach readers as
[secret-line excerpts](#secret-line-excerpts) are not on that command, so they
do not count toward the bound, and gstack reads them in git calls of at most
24,000 argument bytes each (GST-1967). Every brief also gets one line:
`Excluded by base policy <ref>@<sha12> (selection <digest>): <n> files, <bytes> bytes
on this branch matching <patterns>. They are not in your diff; do not report on
their absence.`
@@ -1479,7 +1482,7 @@ their absence.`
<a id="secret-line-excerpts"></a>
**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
and the name scan both read) is left out of `DIFF_KEEP:`, and
`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
+5 -4
View File
@@ -3752,13 +3752,14 @@ the base policy commit.
- **The branch declares the key but the base does not:** only the base copy
applies; land that hunk on the base first, as above.
- **A pattern was not applied** (`skipped=` on the `REVIEW_EXCLUDE:` line): its kept
files need more than 24,000 argument bytes. Narrow the pattern, move the kept
files out of it, or split the PR.
files need more than 24,000 argument bytes on the reader's keep command (files
kept only for a secret line are excerpted and do not count). 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. 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.
`DIFF_KEEP:` is kept for another reason, and one the excerpt could not be
built for moves to the reader's keep command and counts whole.
There is no override flag. See [the review ceiling](ship-policy.md#review-diff-ceiling).
+47 -20
View File
@@ -11,11 +11,12 @@
* stable reason. Applied patterns become `:(top,exclude,glob)` pathspecs on every reader,
* and kept files under them come back through a second `:(top,literal)` command, because a
* 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.
* bytes: a pattern whose kept files would pass KEEP_ARG_MAX_BYTES on it 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.
* A file kept only for a secret line (a line that names a credential and carries a value, secretValueLine;
* 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, readSelection counts
* only that excerpt, and no reader's command names it, so it does not count toward the bound (GST-1967).
*
* 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
@@ -28,16 +29,17 @@ import * as os from 'node:os';
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 { scan } from './redact-engine';
import { shannonEntropy } from './redact-patterns';
import { KEY_NAME, SECRET_LINE, 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';
import { loadPolicyAt } from './ship-policy-at';
/** The most diff bytes one reader is handed (about 500K tokens); above it the pass stops instead of truncating. */
export const REVIEW_DIFF_MAX_BYTES = 2_000_000;
/** The most argument bytes the kept-file command may carry across all applied patterns (under Windows' 32,767-character command line). */
/** The most argument bytes one git command, or the kept-file command a reader runs, may carry (under Windows' 32,767-character command line). */
export const KEEP_ARG_MAX_BYTES = 24_000;
/** How long one reader's `git diff` may run. */
export const REVIEW_DIFF_TIMEOUT_MS = 120_000;
@@ -81,6 +83,24 @@ export interface ExcludeInput {
}
const sha256 = (s: string) => createHash('sha256').update(s).digest('hex');
/** Every credential name on a line: SECRET_LINE, and a key name given a value (`OPENAI_API_KEY=`, `"GH_TOKEN": `). */
const SECRET_NAMES = [...SECRET_LINE, new RegExp(String.raw`\b${KEY_NAME}['"]?\s*[=:]`)].map(r => new RegExp(r.source, `${r.flags}g`));
/** A variable reference (`$X`, `${X}`): a name, never a value. */
const VARIABLE_REFERENCE = /\$\{?[A-Za-z_]\w*\}?/g;
/** An unprefixed credential-shaped run (base64, base64url, a random token): 20+ characters mixing upper case, lower case and digits. */
const VALUE_TOKEN = /[A-Za-z0-9+/_=-]{20,}/g;
const valueToken = (text: string) => [...text.replace(VARIABLE_REFERENCE, ' ').matchAll(VALUE_TOKEN)].some(([t]) => /[A-Z]/.test(t) && /[a-z]/.test(t) && /\d/.test(t) && shannonEntropy(t) >= 3.5);
/**
* Whether one changed line of excludable content holds a credential value, so its file is kept (GST-1967): it names a
* credential, and the credential scan (the secrets check, with #4157's env-read rule) finds a secret anywhere on the line,
* or a credential-shaped run follows a name within 200 characters. A line that only names one (`${OPENAI_API_KEY}`,
* `process.env.X`, `os.environ["X"]`, a keychain) holds no value.
*/
export function secretValueLine(p: string, line: string): boolean {
const ends = SECRET_NAMES.flatMap(r => [...line.matchAll(r)].map(m => m.index + m[0].length));
return ends.length > 0 && (ends.some(e => valueToken(line.slice(e, e + 200))) || scan(line, { processEnv: {}, sourcePath: p }).findings.some(f => f.category === 'secret' && f.severity !== 'LOW'));
}
/** The list_sha256 of a selection that holds no file: no pattern matched the range, so applied, excluded and kept are all empty. */
export const EMPTY_LIST_SHA256 = sha256(JSON.stringify({ applied: [], excluded: [], kept: [] }));
/** A recorded selection's list hash (applied patterns, excluded files, kept files with their reasons) when its digest is built from it; null otherwise. */
@@ -88,12 +108,12 @@ export const recordedList = (r: Partial<Pick<ExcludeRecord, 'patterns_sha256' |
const shellQuote = (s: string) => `'${s.replace(/'/g, "'\\''")}'`;
const literal = (p: string) => `:(top,literal)${p}`;
const argBytes = (paths: Iterable<string>) => { let n = 0; for (const p of paths) n += Buffer.byteLength(shellQuote(literal(p))) + 1; return n; };
/** `paths` in order, split so each chunk's literal pathspecs stay within KEEP_ARG_MAX_BYTES (one path past it is its own chunk). */
function argChunks(paths: readonly string[]): string[][] {
/** `paths` in order, split so each chunk's literal pathspecs (`size` bytes each) stay within KEEP_ARG_MAX_BYTES (one path past it is its own chunk). */
function argChunks(paths: readonly string[], size = (p: string) => argBytes([p])): string[][] {
const chunks: string[][] = [];
let bytes = Infinity;
for (const p of paths) {
const n = argBytes([p]);
const n = size(p);
if (bytes + n > KEEP_ARG_MAX_BYTES) { chunks.push([]); bytes = 0; }
chunks[chunks.length - 1]!.push(p);
bytes += n;
@@ -262,7 +282,7 @@ function pathReason(i: ExcludeInput, p: string, m: Matched, binary: ReadonlySet<
if (isAgentProse(p)) return 'agent-prose';
if (isDataConfig(p)) return 'config';
if (contentKind(i, p, binary) === null) return 'code';
if ((i.added[p] ?? []).some(secretLine)) return 'secret-line';
if ((i.added[p] ?? []).some(l => secretValueLine(p, l))) return 'secret-line';
return null;
}
@@ -355,6 +375,7 @@ export function reviewExclude(i: ExcludeInput): Exclusion {
else if (named === null || p.includes('\n')) reasons.set(p, 'consumer-scan-unavailable');
else if (named.has(p)) reasons.set(p, 'named-by-code');
}
const excerptable = new Set(i.gateTagged === null || named === null ? [] : secretOnly.filter(p => !named.has(p)));
const applied: string[] = [];
const skipped: ExcludeRecord['skipped'] = [];
const keep = new Set<string>();
@@ -362,13 +383,13 @@ export function reviewExclude(i: ExcludeInput): Exclusion {
const hit = sel.byPattern.get(p) ?? [];
if (!hit.some(f => reasons.get(f) === null)) continue;
const kept = hit.filter(f => reasons.get(f) !== null);
const bytes = argBytes(new Set([...keep, ...kept]));
const bytes = argBytes([...new Set([...keep, ...kept])].filter(f => !excerptable.has(f)));
if (bytes > KEEP_ARG_MAX_BYTES) { skipped.push({ pattern: p, kept: kept.length, bytes }); continue; }
applied.push(p);
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 excerpt = [...excerptable].filter(p => keep.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<ContentKind, number> = { binary: 0, data: 0, docs: 0, log: 0 };
@@ -406,7 +427,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)`);
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, and DIFF_KEEP leaves them out`);
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;
}
@@ -441,24 +462,30 @@ function excerptOf(out: string, specs: readonly string[]): { text: string; whole
/**
* One reader's selection read once: `git diff <base> [-- 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
* directory. The kept and excerpt files are read in git calls of at most KEEP_ARG_MAX_BYTES of pathspecs each, so however many
* there are no command passes Windows' limit. `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 = '';
let ex: ReturnType<typeof excerptOf> | 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[]]] : [])];
let raw = '';
let full = false;
const size = (s: string) => Buffer.byteLength(shellQuote(s)) + 1;
const textFlags = ['--text', '--no-textconv', '--src-prefix=a/', '--dst-prefix=b/'];
const reads: Array<[readonly string[], string[]]> = [[pathspecs, []], ...argChunks(keep, size).map(c => [c, []] as [readonly string[], string[]]),
...argChunks(excerpt, size).map(c => [c, textFlags] 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', ...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 === 'ENOBUFS') { text += raw + (r.stdout ?? ''); full = true; 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 };
if (flags.length) { ex = excerptOf(r.stdout, specs); text += ex.wholeText; } else text += r.stdout;
if (flags.length) raw += r.stdout; else text += r.stdout;
}
const ex = excerpt.length && !full ? excerptOf(raw, excerpt) : undefined;
if (ex) text += ex.wholeText;
const byDir = new Map<string, number>();
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)];
-3
View File
@@ -27,9 +27,6 @@ import { AUTH_PATH_PREFIXES, AUTH_PATH_TOKENS, KEY_NAME, MONEY_LINE, MONEY_LINE_
export const TARGETED_CLASSES = ['money', 'gate', 'auth', 'secret'] as const;
export type TargetedClass = (typeof TARGETED_CLASSES)[number];
/** Whether one added or deleted line reads a credential (SECRET_LINE): lib/review-exclude.ts keeps a matched file that has one. */
export const secretLine = (line: string) => SECRET_LINE.some(r => r.test(line));
/** A hash of the patterns that decide the classes, not of this file's bytes: a comment edit keeps it, a pattern edit changes it. */
export const TARGETED_RULES = createHash('sha256').update(JSON.stringify({
TARGETED_CLASSES, MONEY_PATH_TOKENS, MONEY_LINE_TOKENS, MONEY_PAIRS, SPEND_PAIRS, REPORTED_COST_WORDS, MONEY_USD_WORDS, AUTH_PATH_TOKENS, AUTH_PATH_PREFIXES, KEY_NAME, SECRET_SCAN,
+122
View File
@@ -0,0 +1,122 @@
/**
* Which secret-like lines keep a file under review_exclude_paths (GST-1967, harness-evals#333), and the argument-byte bound
* counting only the kept files a reader's diff command carries. A line that only names a credential (an environment
* variable, a keychain) holds no value and keeps nothing; a line that names one and carries a value still keeps its file.
*/
import { afterEach, describe, expect, test } from 'bun:test';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { collectCommit } from '../lib/review-collect';
import { KEEP_ARG_MAX_BYTES, readSelection, reviewExclude, secretValueLine } from '../lib/review-exclude';
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: ['receipts/**'], min_gstack_version: REVIEW_EXCLUDE_KEY_VERSION }) + '\n';
/** Credential-shaped values built at run time, so this source holds no scanner match. */
const OPENAI = ['sk', 'proj', 'Ab3dEf6hIj9kLm2nOp5qRs8tUv1wXy4zAb7cDe0fGh3iJk6lMn9o'].join('-');
const GITHUB = 'gh' + 'p_' + 'Zx9Qw3Er5Ty7Ui9Op1As3Df5Gh7Jk9Lz2Xc4';
const BLOB = Buffer.from('a private signing key that must never reach a results tree 0x5f3759df').toString('base64');
/** Each keeps its file: it names a credential and carries a value. */
const VALUE_LINES: Record<string, string> = {
'real-value': `{"OPENAI_API_KEY": "${OPENAI}"}`,
assignment: `OPENAI_API_KEY=${OPENAI}`,
'json-string': `{"cmd": "curl -H $OPENAI_API_KEY", "value": "${OPENAI}"}`,
'after-name': `{"cmd": "echo \${OPENAI_API_KEY} ${GITHUB} | gh auth login --with-token"}`,
'far-after-name': `{"cmd": "export GH_TOKEN=$GH_TOKEN; ${'step '.repeat(200)}then push with ${GITHUB}"}`,
base64: `{"cmd": "curl -H $OPENAI_API_KEY", "auth": "${BLOB}"}`,
};
/** None keeps its file: each only names a credential. The last two are harness-evals#333's captured prompt lines. */
const NAME_LINES: Record<string, string> = {
brace: '{"cmd": "curl -H \\"Authorization: Bearer ${OPENAI_API_KEY}\\""}',
bare: '{"cmd": "curl -H $OPENAI_API_KEY"}',
'process-env': '{"code": "const key = process.env.OPENAI_API_KEY;"}',
'os-environ': '{"code": "key = os.environ[\\"OPENAI_API_KEY\\"]"}',
'harn-333-curl': '{"system": "Here you go.\\n\\n`curl -H \\"x-agent-capability: $AGENT_API_TOKEN\\" \\"$AGENT_API_URL/v1/apis\\"` lists what you can call."}',
'harn-333-keychain': '{"system": "Requires managed model access and a keychain grant for the browser provider key; follow the repo rules."}',
};
describe('secret-like lines under review_exclude_paths', () => {
// Value: protects=narrowing the keep rule opens no bypass: a key value, a key-name assignment, a key inside a JSON string, a
// key after the name (near or far on the line) and a base64 blob after the name all still keep their file; fails_when=the
// rule drops every line that names a variable, or reads only the name's own span; why_new=GST-1967; seam=secretValueLine
// and gstack-review-plan; no_claim=a bare key with no credential name on its line (pregate's credential scan owns it)
test('a line that names a credential and carries a value keeps its file', () => {
for (const [name, line] of Object.entries(VALUE_LINES)) expect({ name, kept: secretValueLine(`receipts/${name}.json`, line) }).toEqual({ name, kept: true });
});
// Value: protects=a results tree whose captured prompts only name an environment variable or a keychain is excluded like
// any data (harness-evals#333: 2,610 such files kept, 9.5 MB to readers against the 2 MB ceiling); fails_when=a name with no
// value counts as secret-like; why_new=GST-1967; seam=secretValueLine; no_claim=code files, which are never excludable
test('a line that only names an environment variable or a credential store keeps nothing', () => {
for (const [name, line] of Object.entries(NAME_LINES)) expect({ name, kept: secretValueLine(`receipts/${name}.json`, line) }).toEqual({ name, kept: false });
});
test('the plan keeps the value files and excludes the name-only files', () => {
const repo = makeFixtureRepo({ prefix: 'gstack-secret-value-', baseFiles: { '.gstack/ship-policy.json': policyText } }); repos.push(repo);
const files: Record<string, string> = { 'receipts/plain.json': '{"ok": true}\n' };
for (const [name, line] of Object.entries(VALUE_LINES)) files[`receipts/value/${name}.json`] = line + '\n';
for (const [name, line] of Object.entries(NAME_LINES)) files[`receipts/name/${name}.json`] = line + '\n';
commit(repo, files);
const p = JSON.parse(runBin(repo, 'gstack-review-plan', ['--base', 'main', '--gate-tagged', '0', '--json']).out);
expect(p.review_exclude.kept_files).toEqual(Object.keys(VALUE_LINES).sort().map(n => ({ path: `receipts/value/${n}.json`, reason: 'secret-line' })));
expect(p.review_exclude.excluded).toBe(1 + Object.keys(NAME_LINES).length);
expect(p.review_exclude.skipped).toEqual([]);
}, 120_000);
});
describe('the argument-byte bound', () => {
const DIR = `receipts/${'captured-model-requests-'.repeat(4)}`;
const COUNT = 260;
function excerptOnly(namedByCode: boolean) {
const repo = makeFixtureRepo({ prefix: 'gstack-secret-value-', baseFiles: { '.gstack/ship-policy.json': policyText } }); repos.push(repo);
const files: Record<string, string> = { 'receipts/plain.json': '{"ok": true}\n' };
const paths = Array.from({ length: COUNT }, (_, k) => `${DIR}t${String(k).padStart(3, '0')}/llm-turn0.json`);
for (const p of paths) files[p] = `{"n": "${p}"}\n${VALUE_LINES.base64}\n`;
if (namedByCode) files['lib/reader.js'] = paths.map(p => `require('../${p}');`).join('\n') + '\n';
commit(repo, files);
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: new Set(), context: 'test' });
if (x.state !== 'active') throw new Error(`exclusion is ${x.state}`);
return { repo, x, paths, mergeBase: c.mergeBase! };
}
// Value: protects=kept files that reach readers as a secret-line excerpt file never count toward the bound, so a pattern
// over thousands of them still applies, and every git call that reads them stays under the bound (Windows' command line);
// fails_when=the bound counts the excerpt files (harness-evals#333: main/trials/** needed 251,063 bytes and was dropped
// whole) or readSelection passes them to one git diff; why_new=GST-1967; seam=reviewExclude and readSelection under
// GIT_TRACE; no_claim=what an agent's shell does with a brief's keep command (those files still count)
test('excerpt-only kept files past the bound still apply the pattern, and their read is chunked under it', () => {
const { repo, x, paths, mergeBase } = excerptOnly(false);
expect(x.record.skipped).toEqual([]);
expect(x.record.applied).toEqual(['receipts/**']);
expect(x.excerpt.length).toBe(COUNT);
expect(paths.reduce((n, p) => n + Buffer.byteLength(`':(top,literal)${p}' `), 0)).toBeGreaterThan(KEEP_ARG_MAX_BYTES);
const trace = path.join(repo.root, 'git-trace.log');
const prior = process.env.GIT_TRACE;
process.env.GIT_TRACE = trace;
let d: ReturnType<typeof readSelection>;
try { d = readSelection(repo.work, mergeBase, ['.', ...x.specs], x.keep.filter(k => !x.excerpt.includes(k)), x.excerpt); }
finally { if (prior === undefined) delete process.env.GIT_TRACE; else process.env.GIT_TRACE = prior; }
expect(d.ok).toBe(true);
if (!d.ok) return;
expect(d.whole ?? []).toEqual([]);
for (const p of paths) expect(d.excerpt).toContain(p);
const diffs = fs.readFileSync(trace, 'utf8').split('\n').filter(l => /built-in: git( -c \S+)* diff /.test(l));
expect(diffs.length).toBeGreaterThan(2);
for (const l of diffs) expect(Buffer.byteLength(l)).toBeLessThan(KEEP_ARG_MAX_BYTES + 1_000);
}, 120_000);
// Value: protects=kept files a reader's diff command names in full still count, so no brief carries a command line past the
// bound; fails_when=the bound stops counting kept files that are not excerpted; why_new=GST-1967; seam=reviewExclude
test('kept files a reader runs as a command still count toward the bound', () => {
const { x } = excerptOnly(true);
expect(x.record.applied).toEqual([]);
expect(x.record.skipped).toMatchObject([{ pattern: 'receipts/**', kept: COUNT }]);
}, 120_000);
});
+4 -2
View File
@@ -19,6 +19,8 @@ import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } fro
const repos: FixtureRepo[] = [];
afterEach(() => { while (repos.length) cleanup(repos.pop()!); });
const policyText = (patterns: string[]) => JSON.stringify({ review_exclude_paths: patterns, min_gstack_version: REVIEW_EXCLUDE_KEY_VERSION }) + '\n';
/** A credential-shaped base64 value (no known key prefix) for the lines that must keep their file: a name alone keeps nothing (GST-1967). */
const VALUE = Buffer.from('a private signing key that must never reach a results tree 0x5f3759df').toString('base64');
const GZ = Buffer.from([0x1f, 0x8b, 0, 1, 2, 3, 0, 0]);
const EXCLUDED = ['receipts/a/result.json', 'receipts/a/trial.gz', 'receipts/config/blob.bin', 'receipts/c/result.json', 'receipts/e/result.json', 'receipts/notes.md'];
const KEPT = ['receipts/b/result.json', 'receipts/d/fixture.json', 'receipts/run.sh'];
@@ -145,7 +147,7 @@ describe('what review_exclude_paths excludes and keeps', () => {
// and a credential line inside a matched file keeps it; fails_when=scanNumbers or targetedClasses read excluded lines, or
// a secret-bearing data file is excluded; why_new=task 9; seam=bin/gstack-review-plan; no_claim=pregate's credential scan
test('excluded lines feed neither the numeric scan nor the targeted classes; a credential line keeps its file', () => {
const files = { 'receipts/r.json': '{"cost_usd": 12.5, "price_usd": 3, "pass_rate": "97%", "latency_ms": 180}\n', 'docs/bench.md': '# bench\n', 'receipts/s.json': '{"cmd": "curl -H $OPENAI_API_KEY"}\n' };
const files = { 'receipts/r.json': '{"cost_usd": 12.5, "price_usd": 3, "pass_rate": "97%", "latency_ms": 180}\n', 'docs/bench.md': '# bench\n', 'receipts/s.json': `{"cmd": "curl -H $OPENAI_API_KEY", "auth": "${VALUE}"}\n` };
const control = makeFixtureRepo({ prefix: 'gstack-exclude-' }); repos.push(control);
commit(control, files);
const with_ = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['receipts/**']) } }); repos.push(with_);
@@ -963,7 +965,7 @@ describe('a branch cannot hide code under a base pattern', () => {
// item 5 (#3852 round 3, outside-voice#2); seam=bin/gstack-review-plan and reviewExclude in head mode; no_claim=pregate's gitleaks scan, what readers' own diffs show (they still follow the attributes)
test('a branch .gitattributes marking receipts binary cannot get a credential-line file excluded: the keep rule reads the text', () => {
const repo = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['receipts/**']) } }); repos.push(repo);
const secret = '{"cmd": "curl -H $OPENAI_API_KEY"}\n';
const secret = `{"cmd": "curl -H $OPENAI_API_KEY", "auth": "${VALUE}"}\n`;
commit(repo, { '.gitattributes': 'receipts/** binary\n', 'receipts/s.json': secret, 'receipts/a.b/c.json': secret, 'receipts/a.json': '{"ok": true}\n', 'receipts/a/t.json': '{"ok": true}\n' });
const p = JSON.parse(plan(repo, ['--json']).out);
const kept = [{ path: 'receipts/a.b/c.json', reason: 'secret-line' as const }, { path: 'receipts/s.json', reason: 'secret-line' as const }];
+3 -1
View File
@@ -18,6 +18,8 @@ import { cleanup, commit, makeFixtureRepo, runBin, type FixtureRepo } from './he
const repos: FixtureRepo[] = [];
afterEach(() => { while (repos.length) cleanup(repos.pop()!); });
/** A credential-shaped base64 value (no known key prefix) for the lines that must keep their file: a name alone keeps nothing (GST-1967). */
const VALUE = Buffer.from('a private signing key that must never reach a results tree 0x5f3759df').toString('base64');
const SPEC = ':(top,exclude,glob)docs/benchmarks/**';
const QUOTED = "'" + SPEC + "'";
@@ -29,7 +31,7 @@ function fixture() {
const files: Record<string, string> = { 'lib/ship-queue-x.ts': Array.from({ length: 260 }, (_, i) => 'export const f' + i + ' = (a: number) => a + ' + i + ';').join('\n') + '\n' };
for (let k = 0; k < 6; k++) files['docs/benchmarks/r/t' + k + '.json'] = ('{"x": "' + 'y'.repeat(200) + '"}\n').repeat(2000);
for (let k = 0; k < 2; k++) files['docs/benchmarks/r/cap' + k + '.json'] = Array.from({ length: 400 }, (_, n) => n % 100 === 7
? ' "system": "Read ${OPENAI_API_KEY_F' + k + '_L' + n + '} from the environment and never print it.",'
? ' "system": "Read ${OPENAI_API_KEY_F' + k + '_L' + n + '} (value ' + VALUE + ') from the environment and never print it.",'
: ' "line' + n + '": "' + 'x'.repeat(40) + '",').join('\n') + '\n';
commit(repo, files);
const mb = repo.git('merge-base', 'origin/main', 'HEAD');
+4 -2
View File
@@ -17,6 +17,8 @@ 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 base64 value (no known key prefix) for the lines that must keep their file: a name alone keeps nothing (GST-1967). */
const VALUE = Buffer.from('a private signing key that must never reach a results tree 0x5f3759df').toString('base64');
/** 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);
@@ -33,11 +35,11 @@ function capture(k: number, plant: Record<number, string> = {}): { text: string;
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.",`);
lines.push(` "system": "${'You are a coding agent. '.repeat(450)}Read ${env} (value ${VALUE}) 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",`);
lines.push(` "note": "${'context '.repeat(150)}export ${env} (value ${VALUE}) before the push",`);
} else lines.push(` "line${n}": "${'x'.repeat(40)} ${n}",`);
}
lines.push(' ]', '}');