v1.94.14.0 fix(review): review_exclude_paths keeps native binaries, NUL-bearing code and attribute-binary secret lines; exact --review-exclude docs (GST-972) (#3902)

* fix(review): review_exclude_paths keeps native binaries, NUL-bearing code and attribute-binary secret lines; exact --review-exclude docs (GST-972)

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

* test(review): GST-972 key-file fixture holds no key header the credential scan flags

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

* fix: pre-landing review fixes

attributeBinaryLines reads attribute-binary files in one git diff per argument-byte chunk instead of one per file (red-team#2). The changelog and ship-policy row say readers' own diffs still show such a kept file as binary (red-team#1, deferred as #3905).

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

* docs: the native-magic check covers files git reports as binary; attribute-text files are #3910

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

* chore: changelog fragment named for the branch

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

* docs: ship-policy config row names content binaries and keeps native ones (Step 14.5 audit)

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

* docs: drop the post-review ship-policy sentence; #3910 stays in the changelog and PR notes

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

* test(review): kept reasons typed as literals for the test typecheck ratchet

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

* fix(review): sniff executable magic on every file git's own bytes call binary, whatever .gitattributes says; exact review_exclude docs (GST-972 round 2)

bytesBinary reads git diff --numstat with attributes from the empty tree, so a branch's '*.png diff -binary' cannot make an ELF read as text and skip the magic check (#3910). Adds 64-bit universal Mach-O magic. ship-policy's key row lists every kept class; troubleshooting and the write/queue refusal lines say a review_exclude mismatch compares the --review-exclude argument, not the review ledger.

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

* fix(ship-receipt): carry refusal names the --review-exclude digest, not what reviewers read

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-03 11:01:13 +00:00
1 parent d69ba2c6fb
commit 8d276f9e31
10 files changed
+210 -34

No files matched your search

+10
View File
@@ -1,5 +1,15 @@
# Changelog
## [1.94.14.0] - 2026-11-03
**`review_exclude_paths` keeps code it used to exclude, and the docs say exactly what the receipt check stops (GST-972).** Three ways to get code or a credential out of review under a base pattern are closed, each with a failing-first test in `test/review-exclude.test.ts`, and the receipt docs no longer over-claim.
### Fixed
- **Native binaries are code.** A `.node`, `.so`, `.dylib` or `.dll` file under a base pattern was excluded as binary content, and so was a compiled executable with a data extension. A binary file that starts with ELF, Mach-O (universal binaries included), PE, WebAssembly or `#!` magic is now kept as `code` whatever its extension. Binary here is git's own verdict on the bytes, read with no attributes from the tree as well as with the branch's, so a branch's `.gitattributes` (`*.png diff -binary`) cannot make an ELF read as text and skip the check (#3910).
- **A NUL byte no longer makes code excludable.** A binary file was content unless its extension was on a short source list, so `.gradle`, `.sql`, `.tf`, `.groovy`, a `.p12` key store or an extensionless private key with a NUL byte was excluded. A binary file is now content only under a known content extension (images, fonts, media, documents, compressed archives, `.bin`, `.dat` and data blobs); every other binary file is kept as `code`.
- **A branch's `.gitattributes` cannot get a file with a credential line excluded.** Marking `receipts/**` binary left the collector's added lines empty, so the secret-line keep rule saw nothing and the file was excluded. For a file git calls binary only because of its attributes, the rule now reads `git diff --text --no-textconv --no-ext-diff`, so the file stays in review and the plan lists it as `REVIEW_EXCLUDE_KEPT: secret-line`. Readers' own diffs still follow the branch's attributes and show that file as `Binary files differ`; reading such files as text in every reader's diff is a separate follow-up.
- **`gstack-ship-receipt write --review-exclude` docs are exact.** The help, `docs/troubleshooting.md`, `docs/ship-policy.md` and the queue's refusal line said a receipt write alone cannot clear `field=review_exclude`. Write checks the digest against the base policy and does not read the review ledger, so a write given the refusal's `expected=` digest passes without a review. A mismatch proves only that the `--review-exclude` argument (or the digest the receipt recorded from it) differs from the head's selection, not what the readers read. The text now says that, and that the review rerun is the fix.
## [1.94.13.0] - 2026-11-03
- **A bare `--base main` reads origin/main in every review and ship bin (GST-1036).** `gstack-review-plan` now prints `REVIEW_BASE: <ref> <sha>` and names a lagging local base on stderr. `gstack-banned-terms`, `gstack-prose-lint --base`, `gstack-docs-candidate`, `gstack-design-detect.ts scan --changed` and `gstack-pr-shots --base` no longer merge against a stale local main and report lines the branch only inherited. They all use the one resolver gstack-gate-diff already used.
+1 -1
View File
@@ -1 +1 @@
1.94.13.0
1.94.14.0
+1 -1
View File
@@ -1,4 +1,4 @@
# gstack digest v1.94.13.0 — regenerate/re-copy after upgrading gstack
# gstack digest v1.94.14.0 — regenerate/re-copy after upgrading gstack
Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed
for agent hosts without a full skill install. The full skills add workflows,
+5 -3
View File
@@ -99,7 +99,9 @@ write flags: --pr --head --base --version --gated-tree --predecessor --gate <jso
unreadable input records full).
--review-exclude <digest|none> (the digest= on gstack-review-dispatch verify's REVIEW_EXCLUDE_RECORD: line, which the reviewers' briefs
name; none without that line): must equal the digest HEAD has under the base policy's review_exclude_paths, else
RECEIPT_INVALID field=review_exclude and nothing is written (rerun the review; write alone cannot clear it); recorded as review_exclude. Carry and the queue compare that digest.
RECEIPT_INVALID field=review_exclude and nothing is written; recorded as review_exclude. Carry and the queue compare that digest.
The check binds the digest to the base policy, not to a review: write does not read the review ledger, so the expected=
digest a refusal prints passes too. After a refusal, rerun the review and pass the digest its verify prints.
--review-targeted <json|@file> ({classes, rules, brief_sha, status, findings, raised_by, head}: gstack-review-dispatch verify
--targeted-out); recorded as review_targeted bound to the receipt's product_patch_id, and refused (RECEIPT_INVALID) when the
commit the reader read (head) is not an ancestor or a change since it carries a targeted class (gate scan included). write and carry
@@ -236,7 +238,7 @@ function cmdWrite(rest: string[], json: boolean): number {
const excludeNow = review.exclude.state === 'active' ? review.exclude.record : null;
const excludeGiven = flag(rest, '--review-exclude') ?? 'none';
if ((excludeNow?.digest ?? 'none') !== excludeGiven) {
process.stderr.write(`gstack-ship-receipt write: field=review_exclude expected=${excludeNow?.digest ?? 'none'} recorded=${excludeGiven} source=${excludeNow?.source ?? `origin/${base} (no review_exclude_paths)`}: the review --review-exclude names read another selection than ${base}'s review_exclude_paths gives ${head.slice(0, 12)}; fix: rerun /ship's review on ${head.slice(0, 12)} (gstack-review-dispatch pass --base ${base}), run gstack-review-dispatch verify --ledger <ledger> and write with the --review-exclude <d> its REVIEW_EXCLUDE_RECORD: line prints (none when it prints no such line); nothing written (RECEIPT_INVALID)\n`);
process.stderr.write(`gstack-ship-receipt write: field=review_exclude expected=${excludeNow?.digest ?? 'none'} recorded=${excludeGiven} source=${excludeNow?.source ?? `origin/${base} (no review_exclude_paths)`}: the --review-exclude argument names another selection than ${base}'s review_exclude_paths gives ${head.slice(0, 12)} (write compares the argument, not the review ledger); fix: rerun /ship's review on ${head.slice(0, 12)} (gstack-review-dispatch pass --base ${base}), run gstack-review-dispatch verify --ledger <ledger> and write with the --review-exclude <d> its REVIEW_EXCLUDE_RECORD: line prints (none when it prints no such line); nothing written (RECEIPT_INVALID)\n`);
return EXIT.fail;
}
const product = reviewedProduct(repoRoot, base, head, pregateFile);
@@ -777,7 +779,7 @@ function cmdCarry(rest: string[], json: boolean): number {
const now = review.depth;
if (review.exclude.state === 'refused') return refused('RECEIPT_CARRY_REFUSED', `${review.exclude.lines.join('; ')}; run /ship on ${head.slice(0, 12)} once the base policy reads`);
const digest = review.exclude.state === 'active' ? review.exclude.record.digest : 'none';
if (digest !== (receipt.review_exclude?.digest ?? 'none')) return refused('RECEIPT_CARRY_REFUSED', `review_exclude changed: recorded=${receipt.review_exclude?.digest ?? 'none'} now=${digest} source=${review.exclude.state === 'active' ? review.exclude.record.source : `${base} (no review_exclude_paths)`}; the reviewers read another selection than HEAD ${head.slice(0, 12)} has under the base policy, so carry does not apply; run /ship on ${head.slice(0, 12)}`);
if (digest !== (receipt.review_exclude?.digest ?? 'none')) return refused('RECEIPT_CARRY_REFUSED', `review_exclude changed: recorded=${receipt.review_exclude?.digest ?? 'none'} now=${digest} source=${review.exclude.state === 'active' ? review.exclude.record.source : `${base} (no review_exclude_paths)`}; the receipt's digest is the --review-exclude argument write was given, and it names another selection than HEAD ${head.slice(0, 12)} has under the base policy (carry compares that digest, not the review ledger), so carry does not apply; run /ship on ${head.slice(0, 12)}`);
if (depthBelow(receipt.review_depth.depth, now.depth)) return refused('RECEIPT_CARRY_REFUSED', `HEAD ${head.slice(0, 12)} needs ${now.depth} review depth (${now.reasons.join(', ')}), above the receipt's ${receipt.review_depth.depth}; run /ship on ${head.slice(0, 12)}`);
depthField = { review_depth: { ...now, depth: deeper(receipt.review_depth.depth, now.depth) }, review_exclude: receiptExclude(review.exclude) };
}
+11 -7
View File
@@ -58,7 +58,7 @@ refuses the file (`POLICY_GSTACK_TOO_OLD`). `validate` ends with
| `gate_command` | command or `null` | `null` | The repo's full pre-merge gate. `gstack-ship-queue gate-ahead` and `ready` print it for the caller to run (in the background or in CI); gstack never executes it. Needs `min_gstack_version`. |
| `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 <file>` 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 (data, docs, binary, `.log`) `/ship`'s reviewers leave out; read from the base only, with code, gate, config, executable and named-by-code files kept. See [review_exclude_paths](#review-exclude-paths). Needs `min_gstack_version`. |
| `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`. |
| `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 <base>` (the caps come from `origin/<base>`'s policy, never the branch's copy; `--policy <file>` is for a repository bootstrapping its policy) before the verification gate and prints one `DOCS_CAP: <path> <bytes>/<max> 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`. |
@@ -1287,8 +1287,8 @@ A pattern that matches no changed file is listed as `unmatched=` on the plan lin
**What is excluded, and what is kept.** A matched file is excluded only when it
is content: a data file (JSON, JSONL, CSV, TSV, images), a docs-class file, a
`.log` file, or a file git reports as binary (`-\t-` in `--numstat`; readers would only see `Binary files differ`) whose bytes hold a NUL in their first 8,000, so a branch's `.gitattributes` cannot make a text file binary content. A binary file is content even under
`config/` or in a skill directory; only `policy-dir`, `gate-path`, `executable`, `link`, `gate-tagged`, or a source or script extension (`code`) keep it.
`.log` file, or a binary file: one git reports as binary (`-\t-` in `--numstat`; readers would only see `Binary files differ`) under the branch's attributes or with no attributes from the tree, so a branch's `.gitattributes` cannot make an executable read as text, and whose bytes hold a NUL in their first 8,000, so it cannot make a text file binary content either. A binary file is content only under a known content extension (images, fonts, media, documents, compressed archives, `.bin`, `.dat`, and data blobs such as `.parquet`, `.npy` or `.sqlite`) and only when its bytes do not start with an executable image's magic (ELF, Mach-O including universal binaries, PE, WebAssembly, `#!`). Under those rules it is content even under
`config/` or in a skill directory, and only `policy-dir`, `gate-path`, `executable`, `link` or `gate-tagged` keep it. Every other binary file is `code`: source with a NUL, a native library (`.node`, `.so`, `.dylib`, `.dll`), a compiled executable, a key store, a pickle, or a file with no extension.
Everything else stays in every reader's diff with one reason code:
| Reason | Kept because |
@@ -1300,8 +1300,8 @@ Everything else stays in every reader's diff with one reason code:
| `agent-prose` | it is a skill, section, resolver, template or rule file |
| `config` | it is a data-shaped config file (`package.json`, `tsconfig*.json`, `config/`, dotfiles) |
| `gate-tagged` | the gate scan tagged one of its hunks |
| `code` | it is code or a test, not content; a binary file with a source or script extension (`.js`, `.py`, `.sh`, `.wasm`) counts as code |
| `secret-line` | an added line reads a credential (the targeted pass's secret rule) |
| `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) |
| `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 |
@@ -1351,11 +1351,15 @@ paths and three hashes). The `digest` hashes the declared patterns and the
excluded and kept lists; `source` is provenance only. Write, carry and the queue
each recompute the digest at the current base and compare it, so a base merge
or a moved base that leaves the patterns and the lists unchanged still carries
and binds. A different digest means the reviewers read another selection:
and binds. A different digest means the digest write was given (or the receipt recorded) names another selection than the head has now:
write refuses with `RECEIPT_INVALID field=review_exclude`, carry with
`RECEIPT_CARRY_REFUSED review_exclude changed`, and the queue with
`RECEIPT_INVALID field=review_exclude expected=<digest> recorded=<digest>`. The
fix is to rerun the review, never a receipt write alone.
fix is to rerun the review and write with the digest its `verify` prints. Write
checks the digest against the base policy, so a stale selection cannot be
recorded by accident, but it does not read the review ledger: a write given the
`expected=` digest passes without a review, and only the review run shows that
the reviewers read that selection.
**Adopting it.** (1) Open a PR on the base that adds the patterns and
`min_gstack_version` at least `REVIEW_EXCLUDE_KEY_VERSION`, and ship it like any
+7 -4
View File
@@ -3340,10 +3340,13 @@ targeted refusal, run the targeted reader on HEAD, `gstack-review-dispatch verif
use this code when the receipt's review_exclude digest (or the
`--review-exclude <digest>` write was given) is not the digest HEAD has under the
base policy's [`review_exclude_paths`](ship-policy.md#review-exclude-paths). The line
names `expected=` (now), `recorded=` (what the review read, `none` when it read
without exclusion) and `source=` (the base commit). The reviewers read another
selection than the one the head now has, so a receipt write alone cannot clear
it: rerun `/ship`'s review on the head (`gstack-review-dispatch pass --base <base>`),
names `expected=` (now), `recorded=` (the `--review-exclude` argument, or the digest
the receipt recorded from it; `none` without one) and `source=` (the base commit).
A mismatch proves only that this argument differs from the selection the head has
now. Write compares the argument and does not read the review ledger, so the line
says nothing about what the readers read, and a write that passes the `expected=` digest clears
the field without a review. Do not do that. Rerun
`/ship`'s review on the head (`gstack-review-dispatch pass --base <base>`),
run `gstack-review-dispatch verify --ledger <ledger>`, which prints
`REVIEW_EXCLUDE_RECORD: digest=<d>` from the briefs the readers got, then
`gstack-ship-receipt write --pr <n> --review-exclude <d> --body <body>`.
+67 -14
View File
@@ -122,20 +122,38 @@ function select(i: ExcludeInput, patterns: readonly string[]): { byPattern: Map<
return { byPattern, files };
}
/** Executable image magic: ELF, Mach-O (thin, universal and 64-bit universal; 0xcafebabe is also a Java class), PE, WebAssembly and a script's #!. */
const NATIVE_MAGIC = [[0x7f, 0x45, 0x4c, 0x46], [0xfe, 0xed, 0xfa, 0xce], [0xfe, 0xed, 0xfa, 0xcf], [0xce, 0xfa, 0xed, 0xfe], [0xcf, 0xfa, 0xed, 0xfe], [0xca, 0xfe, 0xba, 0xbe], [0xca, 0xfe, 0xba, 0xbf], [0x4d, 0x5a], [0x00, 0x61, 0x73, 0x6d], [0x23, 0x21]];
/**
* The matched paths git calls binary whose bytes hold a NUL in their first 8,000 (git's own test), read from the head blob,
* the file on a live tree, or the old blob for a deletion. A branch's .gitattributes can make git call a text file binary;
* only real bytes make it binary content here.
* The matched paths git calls binary by their bytes alone: `git diff --numstat` with attributes read from the empty tree, so
* a branch's .gitattributes (`diff`, `-binary`, `text`) cannot make an executable image read as text and skip the
* magic check. null when git cannot run it (a git without --attr-source); the caller then reads every matched file's bytes.
*/
function binaryContent(i: ExcludeInput, files: ReadonlyMap<string, Matched>, claimed: ReadonlySet<string>, unread: Set<string>): Set<string> {
function bytesBinary(i: ExcludeInput, patterns: readonly string[]): string[] | null {
const empty = git(i.root, ['hash-object', '-t', 'tree', '--stdin'], '');
if (!empty.ok) return null;
const range = i.head ? [i.mergeBase, i.head] : [i.mergeBase];
const r = git(i.root, [`--attr-source=${empty.out.trim()}`, 'diff', '--numstat', '-z', '--no-renames', ...range, '--', ...patterns.map(p => `:(top,glob)${p}`)]);
return r.ok ? r.out.split('\0').filter(l => l.startsWith('-\t-\t')).map(l => l.slice(4)) : null;
}
const nativeImage = (b: Buffer) => NATIVE_MAGIC.some(m => m.every((x, k) => b[k] === x));
/**
* The claimed paths (git calls them binary under the branch's attributes or under none: bytesBinary) whose bytes hold a NUL
* in their first 8,000 (git's own test), read from the head blob, the file on a live tree, or the old blob for a deletion.
* A branch's .gitattributes can make git call a text file binary; only real bytes make it binary content here. A binary file that starts with executable image magic also goes in `native`.
*/
function binaryContent(i: ExcludeInput, files: ReadonlyMap<string, Matched>, claimed: ReadonlySet<string>, unread: Set<string>, native: Set<string>): Set<string> {
const out = new Set<string>();
const sniff = (p: string, b: Buffer) => { if (!b.includes(0)) return; out.add(p); if (nativeImage(b)) native.add(p); };
const real = (sha: string) => /^[0-9a-f]{40,64}$/.test(sha) && !/^0+$/.test(sha);
const bySha = new Map<string, string[]>();
for (const p of claimed) {
const m = files.get(p);
if (!m) continue;
if (!i.head && m.newMode !== '000000') {
try { const fd = fs.openSync(path.join(i.root, p), 'r'); const buf = Buffer.alloc(8000); const n = fs.readSync(fd, buf, 0, 8000, 0); fs.closeSync(fd); if (buf.subarray(0, n).includes(0)) out.add(p); } catch { unread.add(p); }
try { const fd = fs.openSync(path.join(i.root, p), 'r'); const buf = Buffer.alloc(8000); const n = fs.readSync(fd, buf, 0, 8000, 0); fs.closeSync(fd); sniff(p, buf.subarray(0, n)); } catch { unread.add(p); }
continue;
}
const sha = real(m.newSha) ? m.newSha : real(m.oldSha) ? m.oldSha : '';
@@ -144,8 +162,36 @@ function binaryContent(i: ExcludeInput, files: ReadonlyMap<string, Matched>, cla
for (const [sha, paths] of bySha) {
const r = spawnSync('git', ['cat-file', 'blob', sha], { cwd: i.root, timeout: 30_000, maxBuffer: 8000 });
const read = r.status === 0 || (r.error as NodeJS.ErrnoException | undefined)?.code === 'ENOBUFS';
const binary = read && (r.stdout ?? Buffer.alloc(0)).subarray(0, 8000).includes(0);
for (const p of paths) if (!read) unread.add(p); else if (binary) out.add(p);
const head = (r.stdout ?? Buffer.alloc(0)).subarray(0, 8000);
for (const p of paths) if (!read) unread.add(p); else sniff(p, head);
}
return out;
}
/**
* The added lines of matched files git calls binary only because of their attributes (the bytes hold no NUL): the
* collector's added lines are empty for them, so the secret-line rule reads `git diff --text` instead, which no
* .gitattributes, textconv or external diff driver changes. One git diff per KEEP_ARG_MAX_BYTES of literal pathspecs;
* git prints one section per path in byte order, so a chunk whose sections do not line up with its sorted paths, or a
* git failure, puts every path of that chunk in `unread`.
*/
function attributeBinaryLines(i: ExcludeInput, paths: readonly string[], unread: Set<string>): Record<string, string[]> {
const out: Record<string, string[]> = {};
const range = i.head ? [i.mergeBase, i.head] : [i.mergeBase];
const chunks: string[][] = [];
let bytes = Infinity;
for (const p of [...paths].sort((a, b) => Buffer.compare(Buffer.from(a), Buffer.from(b)))) {
const n = argBytes([p]);
if (bytes + n > KEEP_ARG_MAX_BYTES) { chunks.push([]); bytes = 0; }
chunks[chunks.length - 1]!.push(p);
bytes += n;
}
for (const chunk of chunks) {
const r = git(i.root, ['diff', '--text', '--no-textconv', '--no-ext-diff', '--no-renames', '--src-prefix=a/', '--dst-prefix=b/', '-U0', ...range, '--', ...chunk.map(literal)]);
const sections = r.ok ? r.out.split(/^(?=diff --git )/m).filter(s => s.startsWith('diff --git ')) : [];
const aligned = sections.length === chunk.length && chunk.every((p, k) => /["\\\x00-\x1f\x7f]/.test(p) || sections[k]!.startsWith(`diff --git a/${p} b/${p}\n`));
if (!aligned) { for (const p of chunk) unread.add(p); continue; }
chunk.forEach((p, k) => { let hunk = false; out[p] = sections[k]!.split('\n').filter(l => (hunk ||= l.startsWith('@@')) && l.startsWith('+')).map(l => l.slice(1)); });
}
return out;
}
@@ -159,17 +205,21 @@ function contentKind(i: ExcludeInput, p: string, binary: ReadonlySet<string>): C
return classifyPath(p, i.classRoot) === 'docs' ? 'docs' : null;
}
/** Source and script extensions: a NUL in one does not make it excludable binary content. */
const CODE_EXT = /\.(?:[cm]?[jt]sx?|py|rb|go|rs|java|kts?|scala|swift|exs?|php|cs|c|h|cc|cpp|hpp|mm?|dart|pl|pm|lua|sh|bash|zsh|fish|ps1|bat|cmd|vue|svelte|html?|wasm)$/i;
/**
* The only extensions a binary file may be excludable content under: images, fonts, media, documents, compressed archives
* and data blobs. Any other binary (source with a NUL, a native library or executable, a key store, a pickle, no
* extension) is code; so is one that starts with executable image magic whatever its extension.
*/
const BINARY_CONTENT_EXT = /\.(?:png|jpe?g|gif|webp|bmp|ico|avif|tiff?|heic|heif|woff2?|ttf|otf|eot|mp3|mp4|m4a|wav|ogg|oga|ogv|webm|mov|avi|mkv|flac|aac|opus|pdf|docx|xlsx|pptx|odt|ods|odp|gz|tgz|zip|tar|bz2|xz|zst|7z|bin|dat|parquet|arrow|feather|avro|orc|npy|npz|h5|hdf5|sqlite3?|db|safetensors)$/i;
/** Why a matched file is kept by its path, mode and lines alone; null for content the policy may exclude. A binary file readers would only see as "Binary files differ" is content unless its directory, mode or gate tags keep it. */
function pathReason(i: ExcludeInput, p: string, m: Matched, binary: ReadonlySet<string>): KeptReason | null {
/** Why a matched file is kept by its path, mode and lines alone; null for content the policy may exclude. A binary file readers would only see as "Binary files differ" is content unless its directory, mode, gate tags, extension or magic keep it. */
function pathReason(i: ExcludeInput, p: string, m: Matched, binary: ReadonlySet<string>, native: ReadonlySet<string>): KeptReason | null {
if (/^\.(gstack|github)\//.test(p)) return 'policy-dir';
if (isGatePath(p)) return 'gate-path';
if (m.oldMode === '100755' || m.newMode === '100755') return 'executable';
if ([m.oldMode, m.newMode].some(x => x === '120000' || x === '160000')) return 'link';
if (i.gateTagged?.has(p)) return 'gate-tagged';
if (binary.has(p)) return CODE_EXT.test(p) ? 'code' : null;
if (binary.has(p)) return BINARY_CONTENT_EXT.test(p) && !native.has(p) ? null : 'code';
if (isAgentProse(p)) return 'agent-prose';
if (isDataConfig(p)) return 'config';
if (contentKind(i, p, binary) === null) return 'code';
@@ -227,9 +277,12 @@ export function reviewExclude(i: ExcludeInput): Exclusion {
const sel = select(i, patterns);
if ('error' in sel) return { state: 'refused', code: 'REVIEW_DIFF_UNREADABLE', lines: [`${i.context}: git could not list the files review_exclude_paths matches (${sel.error}); fix: git fetch origin ${base}, then rerun once; nothing was planned (REVIEW_DIFF_UNREADABLE)`] };
const unread = new Set<string>();
const binary = binaryContent(i, sel.files, new Set(i.perPath.filter(e => e.binary && sel.files.has(e.path)).map(e => e.path)), unread);
const native = new Set<string>();
const claimed = i.perPath.filter(e => e.binary && sel.files.has(e.path)).map(e => e.path);
const binary = binaryContent(i, sel.files, new Set([...claimed, ...(bytesBinary(i, patterns) ?? sel.files.keys())]), unread, native);
const text = { ...i, added: { ...i.added, ...attributeBinaryLines(i, claimed.filter(p => !binary.has(p) && !unread.has(p)), unread) } };
const reasons = new Map<string, KeptReason | null>();
for (const [p, m] of sel.files) reasons.set(p, unread.has(p) ? 'consumer-scan-unavailable' : pathReason(i, p, m, binary));
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]);
for (const p of candidates) {
+3 -2
View File
@@ -224,7 +224,8 @@ function headInputs(root: string, baseRef: string, head: string, r?: ShipReceipt
/**
* The queue's exclusion check: the digest the receipt recorded (none when absent) against the one `head` has under the current
* base policy. Only the digest counts (the policy's patterns and the excluded list), so a base merge or a moved base that leaves
* both unchanged binds; a changed selection needs the review rerun, which receipt write alone cannot stand in for.
* both unchanged binds; a changed selection needs the review rerun (write checks the same digest and does not read the
* review ledger).
*/
export function excludeMismatch(x: Exclusion, r: ShipReceipt, head: string, base: string): string | null {
const expected = x.state === 'active' ? x.record.digest : 'none';
@@ -232,7 +233,7 @@ export function excludeMismatch(x: Exclusion, r: ShipReceipt, head: string, base
if (expected === recorded) return null;
const source = x.state === 'active' ? x.record.source : r.review_exclude?.source ?? base;
const h = head.slice(0, 12);
return `field=review_exclude expected=${expected} recorded=${recorded} source=${source}: the reviewers read another selection than ${base}'s review_exclude_paths gives ${h}; the receipt is unchanged; fix: rerun /ship's review on ${h} (gstack-review-dispatch pass --base ${base.replace(/^(refs\/remotes\/)?origin\//, '')}), then gstack-ship-receipt write --pr <n> --review-exclude <d> --body <body> with the digest gstack-review-dispatch verify --ledger <ledger> prints on REVIEW_EXCLUDE_RECORD:; a receipt write alone does not clear this`;
return `field=review_exclude expected=${expected} recorded=${recorded} source=${source}: the receipt records another selection than ${base}'s review_exclude_paths gives ${h}; the receipt is unchanged; fix: rerun /ship's review on ${h} (gstack-review-dispatch pass --base ${base.replace(/^(refs\/remotes\/)?origin\//, '')}), then gstack-ship-receipt write --pr <n> --review-exclude <d> --body <body> with the digest gstack-review-dispatch verify --ledger <ledger> prints on REVIEW_EXCLUDE_RECORD: (write checks the digest against the base policy and does not read the review ledger, so only the rerun shows the reviewers read this selection)`;
}
/** The receipt's recorded gate scan when it names this head; otherwise null (unavailable). */
const boundScan = (r: ShipReceipt, head: string): number | null => {
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "gstack",
"version": "1.94.13",
"version": "1.94.14",
"description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
"license": "MIT",
"type": "module",
+104 -1
View File
@@ -11,7 +11,7 @@ import { EXIT } from '../lib/headless-artifacts';
import { KEEP_ARG_MAX_BYTES, readSelection, reviewExclude, tooLargeLine, unreadableLine } from '../lib/review-exclude';
import { collectCommit, collectLive } from '../lib/review-collect';
import { extractReceipt } from '../lib/ship-receipt';
import { bindShippedReceipt, receiptReviewAtHead } from '../lib/ship-receipt-depth';
import { bindShippedReceipt, excludeMismatch, receiptReviewAtHead } from '../lib/ship-receipt-depth';
import { REVIEW_EXCLUDE_KEY_VERSION, isUnavailable } from '../lib/ship-policy';
import { loadPolicyAt } from '../lib/ship-policy-at';
import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture';
@@ -313,6 +313,31 @@ describe('the receipt binds the selection the reviewers read', () => {
expect(removed.status).toBe(EXIT.fail);
expect(removed.err).toContain(`field=review_exclude expected=none recorded=${digest}`);
}, 240_000);
// Value: protects=every place that describes write --review-exclude says what it checks: the digest HEAD has under the base
// policy, not that a review read that selection; fails_when=the help, troubleshooting, ship-policy or the queue's refusal
// line claims a receipt write alone cannot clear field=review_exclude while a write with the refusal's own expected= digest
// clears it with no review run; why_new=GST-972 item 2 (the #3852 cross-family trail review); seam=gstack-ship-receipt write
// and --help, excludeMismatch, the two docs; no_claim=binding the digest to the review ledger (write never reads it)
test('write binds --review-exclude to the base policy, not to a review, and no description claims more', () => {
const repo = census();
const body = path.join(repo.root, 'body.md');
const refused = writeReceipt(repo, body, []);
const expected = refused.err.match(/field=review_exclude expected=([0-9a-f]{64}) /)?.[1];
expect(expected).toBeDefined();
expect(writeReceipt(repo, body, ['--review-exclude', expected!, ...targetedAtHead(repo)]).status).toBe(EXIT.ok);
const doc = (rel: string) => fs.readFileSync(path.join(import.meta.dir, '..', rel), 'utf8').replace(/\s+/g, ' ');
const claims = {
help: runBin(repo, 'gstack-ship-receipt', ['--help']).out.replace(/\s+/g, ' '),
troubleshooting: doc('docs/troubleshooting.md'),
shipPolicy: doc('docs/ship-policy.md'),
queue: excludeMismatch({ state: 'none' }, { review_exclude: { digest: expected } } as any, repo.git('rev-parse', 'HEAD'), 'origin/main')!,
};
for (const [where, text] of Object.entries(claims)) {
expect({ where, overClaim: /write alone (cannot|does not) clear|never a receipt write alone/i.test(text) }).toEqual({ where, overClaim: false });
expect({ where, says: text.includes('does not read the review ledger') }).toEqual({ where, says: true });
}
}, 240_000);
});
describe('a branch cannot hide code under a base pattern', () => {
@@ -348,6 +373,84 @@ describe('a branch cannot hide code under a base pattern', () => {
expect(p).toMatchObject({ depth: 'full', errors: expect.arrayContaining([expect.objectContaining({ code: 'binary_code_path', input: 'assets/loader.js' })]) });
}, 120_000);
// Value: protects=a compiled binary under a base pattern stays in review, because it is code; fails_when=a .node, .so, .so.N,
// .dylib or .dll file, or an ELF, Mach-O or PE image under a content extension (.bin, .dat), is excluded as binary content;
// why_new=GST-972 item 3; seam=bin/gstack-review-plan; no_claim=what a reader can see inside a binary
test('native binaries are code: .node, .so, .dylib, .dll and executable images under a content extension stay kept', () => {
const repo = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['assets/**']) } }); repos.push(repo);
fs.mkdirSync(path.join(repo.work, 'assets'), { recursive: true });
const elf = Buffer.concat([Buffer.from([0x7f, 0x45, 0x4c, 0x46, 2, 1, 1, 0]), Buffer.alloc(24)]);
const macho = Buffer.concat([Buffer.from([0xcf, 0xfa, 0xed, 0xfe]), Buffer.alloc(28)]);
const pe = Buffer.concat([Buffer.from('MZ'), Buffer.alloc(30)]);
const files = { 'assets/addon.node': elf, 'assets/libx.dylib': macho, 'assets/libx.so': elf, 'assets/libx.so.1': elf, 'assets/setup.dat': pe, 'assets/tool.bin': elf, 'assets/x.dll': pe, 'assets/real.gz': GZ };
for (const [rel, bytes] of Object.entries(files)) fs.writeFileSync(path.join(repo.work, rel), bytes);
commit(repo, {});
const p = JSON.parse(plan(repo, ['--json']).out);
expect(p.review_exclude.kept_files).toEqual(Object.keys(files).filter(f => f !== 'assets/real.gz').map(f => ({ path: f, reason: 'code' })));
expect(p.review_exclude).toMatchObject({ excluded: 1, content: { binary: 1 } });
}, 120_000);
// Value: protects=a branch's .gitattributes cannot get an executable image excluded by making git read it as text, and a
// 64-bit universal Mach-O is an executable image; fails_when=only files git reports as binary under the branch's own
// attributes get the magic check, so an ELF named .png under '*.png diff -binary' is excluded as data, or 0xcafebabf
// is not executable magic; why_new=GST-972 item 3 follow-up (#3910, #3902 outside voice); seam=bin/gstack-review-plan and
// reviewExclude in head mode; no_claim=a NUL-free script under a content extension
test('a branch .gitattributes that makes git read an ELF as text cannot get it excluded; fat64 Mach-O is native', () => {
const repo = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['assets/**']) } }); repos.push(repo);
fs.mkdirSync(path.join(repo.work, 'assets'), { recursive: true });
fs.writeFileSync(path.join(repo.work, 'assets/native.png'), Buffer.concat([Buffer.from([0x7f, 0x45, 0x4c, 0x46, 2, 1, 1, 0]), Buffer.alloc(24)]));
fs.writeFileSync(path.join(repo.work, 'assets/fat64.dat'), Buffer.concat([Buffer.from([0xca, 0xfe, 0xba, 0xbf]), Buffer.alloc(28)]));
fs.writeFileSync(path.join(repo.work, 'assets/real.gz'), GZ);
commit(repo, { '.gitattributes': '*.png diff -binary\n' });
const p = JSON.parse(plan(repo, ['--json']).out);
const kept = [{ path: 'assets/fat64.dat', reason: 'code' as const }, { path: 'assets/native.png', reason: 'code' as const }];
expect(p.review_exclude.kept_files).toEqual(kept);
expect(p.review_exclude).toMatchObject({ excluded: 1, content: { binary: 1 } });
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' });
expect(x.state === 'active' && x.kept).toEqual(kept);
expect(x.state === 'active' && x.record.digest).toBe(p.review_exclude.digest);
}, 120_000);
// Value: protects=a NUL byte cannot turn code or a key file into excludable content; fails_when=a NUL-bearing file with a
// source extension outside the old CODE_EXT list (.gradle, .sql, .tf, .groovy), a key store (.p12) or an extensionless key
// is excluded as binary; why_new=GST-972 item 6 (#3852 targeted gate reader at 61803d9); seam=bin/gstack-review-plan;
// no_claim=the list of binary content extensions is complete (an unknown one is kept, never excluded)
test('a NUL-bearing file is binary content only under a known content extension; code and key files stay kept', () => {
const repo = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['assets/**']) } }); repos.push(repo);
fs.mkdirSync(path.join(repo.work, 'assets'), { recursive: true });
const nul = (code: string) => Buffer.from(`/* \0 */ ${code}\n`);
const kept = { 'assets/Build.groovy': nul('println "x"'), 'assets/build.gradle': nul('apply plugin: "java"'), 'assets/id_ed25519': nul('openssh-key-v1'), 'assets/main.tf': nul('resource "x" "y" {}'), 'assets/query.sql': nul('DROP TABLE users;'), 'assets/server.p12': Buffer.from([0x30, 0x82, 0, 1, 2, 3]) };
const content = { 'assets/report.pdf': GZ, 'assets/shot.png': GZ };
for (const [rel, bytes] of Object.entries({ ...kept, ...content })) fs.writeFileSync(path.join(repo.work, rel), bytes);
commit(repo, {});
const p = JSON.parse(plan(repo, ['--json']).out);
expect(p.review_exclude.kept_files).toEqual(Object.keys(kept).map(f => ({ path: f, reason: 'code' })));
expect(p.review_exclude).toMatchObject({ excluded: 2, content: { binary: 2 } });
}, 120_000);
// Value: protects=a branch's .gitattributes cannot get a matched file with a credential line excluded by making git call it
// binary, at the plan (live tree) and at the receipt (head blobs); fails_when=the secret-line rule reads only the
// collector's added lines, which git leaves empty for an attribute-binary file, so the file is excluded; why_new=GST-972
// 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';
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 }];
expect(p.review_exclude.kept_files).toEqual(kept);
expect(p.review_exclude).toMatchObject({ excluded: 2, content: { data: 2 } });
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' });
expect(x.state === 'active' && x.kept).toEqual(kept);
expect(x.state === 'active' && x.record.digest).toBe(p.review_exclude.digest);
}, 120_000);
test('an unavailable gate scan keeps every candidate instead of excluding it', () => {
const repo = census();
const head = repo.git('rev-parse', 'HEAD');