diff --git a/changelog.d/gst-508-review-exclude-paths.md b/changelog.d/gst-508-review-exclude-paths.md index 221061f8e..4859bfa95 100644 --- a/changelog.d/gst-508-review-exclude-paths.md +++ b/changelog.d/gst-508-review-exclude-paths.md @@ -2,7 +2,7 @@ ### Added - **`review_exclude_paths` in `.gstack/ship-policy.json`** (`["receipts/**"]`, with `min_gstack_version` at least the release that adds it). It is read from the base branch only, so a branch cannot exclude its own files; a branch that adds the key gets a normal review and `REVIEW_EXCLUDE: none (head sets review_exclude_paths; …)`. Patterns are git glob pathspecs, root-anchored, and git itself lists the matches. -- **Only content is excluded**: data files, docs, `.log` files and files whose bytes are binary (a branch's `.gitattributes` cannot make a text file count as binary). Untracked files are never part of the selection. Code, tests, executables, symlinks, submodule commits, gate paths, agent prose, config, `.gstack/` and `.github/` files, gate-tagged files and files with a credential line stay in every reader's diff, each with a reason code. A data file a code or test file names by its full path stays too. A basename, a directory reference or a data manifest that lists receipts does not keep anything. +- **Only content is excluded**: data files, docs, `.log` files and files whose bytes are binary and whose extension is not source or script (a branch's `.gitattributes` cannot make a text file count as binary, and a NUL byte cannot hide a `.js` file). Untracked files are never part of the selection. Code, tests, executables, symlinks, submodule commits, gate paths, agent prose, config, `.gstack/` and `.github/` files, gate-tagged files and files with a credential line stay in every reader's diff, each with a reason code. A data file a code or test file names by its full path stays too. A basename, a directory reference or a data manifest that lists receipts does not keep anything. - **`gstack-review-plan` prints `REVIEW_EXCLUDE:`** with the counts, the excluded files by kind, the kept files and a digest. Every reader's `DIFF_CMD:`, the fixture command the security specialist and the Red Team use, and the outside voice's diff leave the files out; kept files come back through `DIFF_KEEP:`. Every brief says what was left out. The kept-file command is bounded at 24,000 argument bytes, and a pattern over that bound is not applied and the plan says why. - **Excluded lines no longer trigger the numeric-claim scan or the targeted money, gate and auth classes**, and they count as docs for the review depth. - **The receipt records `review_exclude`.** `gstack-review-dispatch verify` prints `REVIEW_EXCLUDE_RECORD: digest=` from the briefs the readers got, and `gstack-ship-receipt write --review-exclude ` must match the digest HEAD has under the base policy (`RECEIPT_INVALID field=review_exclude`, nothing written). Carry and the queue compare the digest, not the base commit, so a base merge that leaves the policy alone still carries. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index b4b9fc7e3..3dec13db0 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1130,7 +1130,7 @@ 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 the first four reasons below keep it. +`config/` or in a skill directory; only the first five reasons below, or a source or script extension (`code`), keep it. Everything else stays in every reader's diff with one reason code: | Reason | Kept because | @@ -1142,7 +1142,7 @@ 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 | +| `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) | | `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 | diff --git a/lib/review-exclude.ts b/lib/review-exclude.ts index 8466bb08e..ea5e071bc 100644 --- a/lib/review-exclude.ts +++ b/lib/review-exclude.ts @@ -165,6 +165,9 @@ function contentKind(i: ExcludeInput, p: string, binary: ReadonlySet): 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; + /** 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): KeptReason | null { if (/^\.(gstack|github)\//.test(p)) return 'policy-dir'; @@ -172,7 +175,7 @@ function pathReason(i: ExcludeInput, p: string, m: Matched, binary: ReadonlySet< 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 null; + if (binary.has(p)) return CODE_EXT.test(p) ? 'code' : null; if (isAgentProse(p)) return 'agent-prose'; if (isDataConfig(p)) return 'config'; if (contentKind(i, p, binary) === null) return 'code'; diff --git a/test/review-exclude.test.ts b/test/review-exclude.test.ts index 3a90f72bd..8884239e9 100644 --- a/test/review-exclude.test.ts +++ b/test/review-exclude.test.ts @@ -279,7 +279,7 @@ describe('the receipt binds the selection the reviewers read', () => { describe('a branch cannot hide code under a base pattern', () => { // Value: protects=a branch cannot hide code or a link under a base-declared pattern, and untracked files never move the // selection away from the committed head the receipt reads; fails_when=the branch's .gitattributes makes a code file - // binary content, a symlink is excluded as data, or an untracked file under a pattern changes the plan's digest; + // binary content, a NUL-bearing code file is excluded as binary, a symlink is excluded as data, or an untracked file under a pattern changes the plan's digest; // why_new=/ship pass-1 security and Red Team findings on GST-508, each reproduced before the fix; seam=bin/gstack-review-plan; // no_claim=gitlinks (the same mode rule, no fixture) test('a branch .gitattributes cannot make code binary, a symlink is kept, and an untracked file leaves the digest alone', () => { @@ -297,5 +297,16 @@ describe('a branch cannot hide code under a base pattern', () => { write(repo.work, 'receipts/stray/result.json', '{"ok": true}\n'); expect(JSON.parse(plan(repo, ['--json']).out).review_exclude.digest).toBe(p.review_exclude.digest); }, 120_000); + test('a binary file at a code path stays kept and still forces full depth; a .gz stays binary content', () => { + 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/loader.js'), Buffer.concat([Buffer.from('/* \0 */ require("child_process").execSync("id");\n')])); + fs.writeFileSync(path.join(repo.work, 'assets/real.gz'), GZ); + commit(repo, {}); + const p = JSON.parse(plan(repo, ['--json']).out); + expect(p.review_exclude.kept_files).toEqual([{ path: 'assets/loader.js', reason: 'code' }]); + expect(p.review_exclude.content).toMatchObject({ binary: 1 }); + expect(p).toMatchObject({ depth: 'full', errors: expect.arrayContaining([expect.objectContaining({ code: 'binary_code_path', input: 'assets/loader.js' })]) }); + }, 120_000); });