Repository navigation
guard: the commit scan covers every commit form and checkout the guard allows [PROTECTED: owner merges] - #979
Conversation
…d allows The commit secret scan now covers every commit form and checkout the guard allows (#971). Before an agent's command commits, the guard models the command line (src/guard/commit.ts): which checkout the commit lands in and what it will record. It scans that, across the whole checkout. A command line whose commit it can't model is refused with the reason, never scanned narrowly. Plain commits keep their behaviour: their staged changes are scanned, and people's own sessions are not. The model is a strict reading of the shell line: only syntax whose meaning is fully known is accepted, and it must agree with the guard's general matcher on every commit in the line. The git queries it needs are run by src/hook.ts, so the new guard module imports only node built-ins and src/guardrails (#785); test/review.test.ts pins it in the guard's tree. Test (fails without): test/adversarial/commit-scan.test.ts, 8 of its 10 tests fail at 9f9533a (the other 2 keep today's behaviour); test/commit-plan.test.ts covers the model itself. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Second adversarial review: CHANGES NEEDED (head a40dafc)I tried to get a secret committed through an agent's command without a refusal, using scratch checkouts, the test token, a scratch git config with no system config, and the hook entry ( Findings
Checked and holding (measured)Option abbreviations and negations, combined short options, path-limited commits, and a physical-path move through a plain link all scanned in the right checkout or were refused. Lines that aren't commits were unaffected. Known residual I confirmed but don't countSomething git runs during the commit, set up in an earlier call, can add content after the scan (measured). That falls under the PR's "anything git itself would run there". I'd still name it in "What this can never do", because the "superset" bullet is false while it exists. Required changes
Verdict: CHANGES NEEDED (one regression against main, three working route classes, two false claims). |
Evaluator verdict: CHANGES NEEDED (round 1/2)Head checked: I checked that the commit scan covers every commit form and checkout the guard allows. It covers the forms the PR models, and those are tested well. It doesn't yet cover all of them. I measured further commit lines that the guard still lets through at this head without scanning what they record. Under the #707 rule the details have gone privately to the orchestrator, not here. CI is also red, and this PR is the cause. Acceptance criteria (#971)
Design checks
Fail-without (method b)
No regression (measured: ordinary lines through the hook in a clean scratch checkout, base vs head)These are still allowed at the head:
New refusals (allowed at base, refused at head) and whether ordinary work hits them:
Measured, no loosening: over a 229-line corpus taken from the five related test files, main's Weakening gate and locks
Protection
Checks
Merge-tree (measured,
|
Review round 2 for #971 (#979). The commit scan now covers every commit form and checkout the guard allows in these further ways, each failing closed: - Every place a commit may run must resolve to a checkout the scan reads; a lookup that fails or finds none refuses the line, and the hook never treats an empty plan as clean. - A directory change is followed only where the shell's answer and the model's can't differ; any other is refused. - No shell evaluation inside a git command's words: a subcommand that isn't a literal word, a word the shell could turn into others, or git run by another program without a literal subcommand, is refused. - The scan reads what the commit records (objects by id, files from disk), with the changed paths from the change reader (src/coordinator/change-files.ts), never a diff git renders. SizeFile moves next to that reader so the guard's tree grows by that one module. Ordinary work keeps going: read-only git before a commit, other git commands in a line that changes directory, and here-document bodies that only hold text. Test (fails without): test/adversarial/commit-scan-routes.test.ts, 6 of its 7 tests fail at a40dafc (the 7th keeps plain commits as they were). Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Evaluator verdict: CHANGES NEEDED (round 2/2, delta final check)Head checked: Every finding from both reviews of Under the #707 rule, this comment gives verdicts by finding ID only. The orchestrator has the details and the probe harness. I ran each probe through the hook entry with real gitleaks 8.24.2. I used scratch checkouts, a test token, a temporary HOME, state and TMPDIR, and a scratch git config with no system config. Each line ran against this head and against a build of main at Findings from the previous round
The builder's claims against the code
New findings
Scanner suppressions (#980), measured. The head is the same as main for inline allow comments and for the checkout's The exempt program list (D5). Inferred from the tools' documented options: the other listed programs don't run their arguments as commands. The list is not closed while D5's entries stay on it. Fail-without (method b)
Weakening gateThese are the changed existing test lines in round 2. None loosens what is refused (inferred):
Checks
Required changes
|
Review round 3 for #971 (#979). The commit scan covers every commit form and checkout the guard allows; this round makes what it reads match what the commit records, and closes the class of lines that run a program the model can't name: - Added lines come from a line diff computed in process (a shortest edit script), so no config or attribute of the checkout can change it, and a line counts where it is added even if the file had it elsewhere. - Nothing in the content decides whether the scanner reads a file. - A working-tree file that git would store as other bytes refuses the commit; the scan never runs a filter. - The commit's own files never configure the scanner. - Every word in command position must be literal, after wrappers too; no wrapper may build a command from its input; no shell may read its script from a pipe, a here-document or an expanded string. - The list of programs treated as never running another program keeps only programs with no option that runs one. Test (fails without): test/adversarial/commit-scan-content.test.ts, all 7 of its tests fail at 9c6761f. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Second review (verification-scoped, after a classifier stop): CHANGES NEEDED on 1a731dcScope. Under the owner's standing rule, this rerun did three things only:
It built no new payloads. Setup. Main is b3176d3. Every run used scratch checkouts and the test token only, with a temp HOME and state, a scratch git config and real gitleaks 8.24.2. Probes are named by group and finding ID only (#707). Documented probes (measured, head and main)
Notes on the table:
Fail-without (measured)
Claims against the codeWhat this can never do
Compromised component
What remains
Required changes
Settling by the decision guide. Both changes are body corrections (principle 5). If the orchestrator treats main's refusal as a closure under principle 3, the fix is code, not words. The commit scan would refuse to proceed in a checkout whose config names a file-system-monitor program, failing closed as main effectively does. That follows principles 1 and 3, so it needs no owner decision. The round-2 final check called this "not a regression" (D7). That call rests on an end-to-end commit on main that was inferred and never run. A verification-scoped rerun may not build that payload, so this review leaves the call to the orchestrator. Everything else checks out.
|
Evaluator verdict: CHANGES NEEDED (round 3, delta final check)Head checked: Round 3 closes every route the round-2 check named. D2 holds. D7 is fixed. But three new ways appeared in which main refuses a commit and this head lets it through with the secret recorded (decision guide 3). The D4 and D5 claims are also still false. Under the #707 rule, this comment gives verdicts by finding ID only. The orchestrator has the details and the probe harness. How I tested:
Round-2 findings
New findings
Cost to ordinary work (D4's report). The intended refusals are: an expanded program name (an editor variable, a tool under an expanded path, a substituted path), piping a script into a shell, a here-document fed to a shell, an expanded D1's work budget (measured).
Fail-without (method b)
Weakening gateMeasured: round 3 changes no existing test line. The only test change is the new file. Checks
Required changes
|
Review round 4 for #971 (#979). The commit scan covers every commit form and checkout the guard allows; this round: - Scans every placement a line diff could give each run of added lines: a run is widened across the unchanged lines it could slide over. - Writes the commit's files under a fixed folder below the scan's root, so no name of theirs can be a file the scanner reads at its root. - Refuses a working-tree file with an ident attribute, as it already did for filter and encoding attributes. - Refuses a commit in a checkout whose effective config names a program git runs while reading changes, in the reads allowed before a commit or in the scan's own listing (fail closed). - Keeps on the exempt list only programs that can't run another one. - Reads the test command and xargs's own options as what they are, so ordinary test commands and xargs runs are not refused. Test (fails without): test/adversarial/commit-scan-reads.test.ts, all 6 of its tests fail at 1a731dc. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Second review (verification-scoped, after a classifier stop)Verdict: CHANGES NEEDED on 1e210a9. Scope, per the owner's standing rule: the documented attacks and the PR's fail-without tests were re-run, and the claims were checked against the code by reading only. No new payloads and no new attack forms. Refusals: none. 1. Documented attacks: head vs origin/main cec056a (measured)Scratch checkouts, a test token, a fake key, temp HOME and temp git config. The groups were run directly and again through the adapted wrapper, with identical results.
Lines the head allows that main refused: none (measured). Still open at both: H1, H2, T1–T5, TR, SC, FL, DS, PS, SO, FX, U9, U11. These are the shell-evaluation class the PR holds for #983. 2. Fail-without (measured)
3. Claims vs code (all inferred, by reading
|
| Claim | Label |
|---|---|
Never: nothing scanned (every place resolves to a top; the hook denies an empty plan, hook.ts:168) |
true |
Never: display settings hide content (reads objects and files; change list with --no-textconv/--no-ext-diff, attributes file off) |
true |
| Never: changes the index or files | true for content. git diff may rewrite stat info in the index; harmless |
| Never: reopens a closed route, as far as measured | true for the documented probes (above); see (a) |
| Never: can't promise program-added content | true |
| Compromised: agent user can't pass by "a git display setting the guard allows" | unscoped: (b), (d), and (c) through reads |
Compromised: agent can't widen the model (src/guard/** and src/coordinator/** locked, review.ts:378, :432) |
true |
| Compromised: coordinator, dashboard, push access | true |
| Compromised: scanned checkout runs no configured program except for the change listing and object reads | true as worded; the listing reaches submodule config (c) |
Remains: programs the line runs; programs git runs during a commit; race; other git commands; docs label "inferred" (guardrails.md:187) |
true |
| Remains: config-run read programs refused since round 4 | unscoped: (b), (c), (d) |
| Remains: shell features that turn data into a command, same as main | true for T1–T5, TR, SC, FL, DS, PS, SO, FX, U11 (measured same as main). H1/H2 (a builtin that remaps a command name) fit none of the listed kinds: unscoped, add it to the list |
| Remains: "decides what is scanned, not how the scanner judges it" | unscoped: see (a); the folder changes how the checkout's own path rules judge |
Required changes
-
(a) Make the checkout's own path rules see repo-relative paths again, or fail closed when its
.gitleaks.tomlhas any rulepathor allowlistpaths. Over-refusal is acceptable; a miss is not. Keep the CL1 and scan-folder protections. Add a test with an anchored path-scoped rule that is found on main and fails without the fix. -
(b)–(d) Either refuse them (preferred, principle 1):
- a signature-display option or config on an allowed read;
-Con a read before a commit, or check those directories' config too;- submodule configs, read or refused when a submodule is checked out.
Or correct the round-4 "What remains" bullet and the Agent-user line so they name these as open. Each one closed gets a fail-without test.
-
Add the command-remapping builtin (H1/H2) to the "What remains" list of shell features.
The fixes follow the decision guide (fail closed, no regression, correct the claim), so no owner decision is needed unless a fix adds a new over-refusal cost beyond these.
Review round 5 for #971 (#979). The commit scan covers every commit form and checkout the guard allows; this round: - Scans each changed file whole, after and before the commit, and counts a secret as new when the file holds it more often after. No placement a diff could choose changes what is found; the in-process line diff and its widening are gone. - Writes each file at its own path, so the checkout's own path rules see the paths main's scan saw; a file named like the scanner's own settings is scanned apart, as content. - Reads xargs's options and bracket patterns as the shell does in the remaining spellings. - Refuses config programs exactly where git runs them: a monitor program always, diff programs only when the line shows diffs, a signature program when it shows signatures; git's own monitor daemon is allowed. Submodules' own config and other checkouts the line reads are checked. - Says why, and what to do, when a filter its files use refuses a commit. Test (fails without): test/adversarial/commit-scan-findings.test.ts, 8 of its 10 tests fail at 1e210a9 (the others keep a fixture-holding file committable and guard round 4's exempt list). Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Round 5b for #971 (#979), by the owner's decision (2026-10-21): a checkout whose large-file filter is exactly the standard git-lfs one, as its installer writes it, may commit. The scan reads each large file's own bytes: from the working tree, or from the checkout's large-file store for a stored pointer (a pointer whose object is missing refuses). Any other value for those keys, any other filter, or a git-lfs on the path that is not a regular file still refuses. This is a narrow, owner-approved loosening of the filter refusal added in round 4. Test (fails without): test/adversarial/commit-scan-lfs.test.ts, the two tests of the loosening fail at 1d1966f; the three refusal tests pass there and guard it. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Second review (verification-scoped, after a classifier stop)Verdict: CHANGES NEEDED on ce7d4ab. Scope: this run repeated the documented attacks and the PR's fail-without tests, and checked the claims by reading the code. It built no new payloads. Where it says "measured", that comes from a command run in this review; "inferred" means read from the code, not run. Probe lines are referred to by group and ID only (#707). 1. Documented attacks, run at the head and at origin/main 1808043 (measured)
Result (measured): across all of r2, r3 and r4, the head allows no line that main refused. Every line the head allows and that leaks (r3 progs and unread) is allowed and leaks on main too. 2. Fail-without (measured)
3. Required changes
4. Claims checked by reading (src/scan/secrets.ts, src/guard/commit.ts, src/hook.ts)(a) The unredacted report: true, with two edges that are out of scope.
(b) The "before" side of the count: true.
(c) Exact standard config: true for the values, the store location and the program check; false for the filter's name (finding 2). All four keys must be present, and every value must equal the standard string (:388). What this can never do
Compromised component
What remains
Round 5 and 5b sections
Refusals during this run: none. |
Evaluator verdict: CHANGES NEEDED (round 5 and 5b, delta final check)Head checked: Round 5 fixes N1–N7 and the second review's (a)–(d) as their probes measure them, and count-based findings hold up. But round 5b's large-file reading lets through a secret that main refuses, and CI is red on this PR's own new test. The narrowed config-program check also misses some spellings. Main allows all of those too, so they are not regressions, but they make round-5 claims false. Under the #707 rule, this comment gives verdicts by finding ID only. The orchestrator has the shapes and the probe harness. How I tested:
Round-4 findings and the second review
Count-based findings (measured)
New findings
The LFS checks that hold (measured):
A
The monitor's boolean handling.
Test lines changed (weakening gate)Three existing lines changed, all in test files this PR adds, none on main. I judge all three acceptable (inferred):
Fail-without (method b)
Checks
Required changes
R5-1 is a regression against main, so it can't be held for the owner. R5-3 to R5-6 follow from principle 1 or need a claim corrected, so no owner decision is needed. No new trade-off goes to the owner. Everything else checks out:
|
…fig as git does Review round 6 for #971 (#979). The commit scan covers every commit form and checkout the guard allows; this round: - Under the standard large-file filter, the bytes git stores are always scanned, and a side that is exactly one valid pointer is scanned with the object it names (checked against the pointer); working-tree and untracked pointers too. - Config keys keep their subsection's case, so only the exactly named large-file driver gets its allowance; any large-file setting outside a short safe list takes the allowance away. - With a diff program configured, any diff or show, and any log or status with an option outside a short list, refuses the commit; with a signature program configured, any log or show does; the file-system monitor setting is read as a boolean the way git reads it. - A nested checkout the line stages is checked like a submodule. - The commit-scan tests no longer read the host's system or user git config. Test (fails without): test/adversarial/commit-scan-stored.test.ts, all 7 of its tests fail at ce7d4ab. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Round 6 for #971 (#979), found while re-running the round-5 probes: an interactive staging step shows each change through any diff filter the config names, so with one configured, a line that stages interactively before it commits is refused, as other diff-showing reads are. Test (fails without): test/adversarial/commit-scan-stored.test.ts, its interactive staging case fails at ce7d4ab. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Second review (verification-scoped)Verdict: PASS on 3e63c5e for the guardrail. I found no route that main had refused and this head lets through, and no false safety claim. Not mergeable as it stands: CI is red on this head, and that needs a builder fix (see Required changes). This review is verification-scoped, per the owner's decision of 2026-10-21. It re-ran the documented probe groups and the PR's fail-without tests, and checked the claims against the code. It built no new payloads. The comment names groups and IDs only (#707). 1. Documented probe groups: head 3e63c5e vs origin/main 1cee9bd (measured)All of these ran unchanged except for paths, on scratch checkouts with a temp HOME.
Still committed at head, the same as on main (measured):
These are the shell-feature residuals named in "What remains" bullet 4 (#983), so they are disclosed, not new. 2. Fail-without (measured)Each file was written byte-identical to the head copy (sha256 matched) as a new file into a fresh worktree, then built and run there.
3. Claims vs
|
#971 (#979) grows the package: with the dashboard UI built, npm pack --dry-run --json --ignore-scripts measures 6,402,689 bytes unpacked, over the 6,400,000 budget (the tarball, 2,045,128, is still under its 2,050,000). The unpacked budget becomes the measured size plus 5%, rounded up to the next 10,000: 6,730,000. The tarball budget is unchanged. This is a loosening of a check, named in the PR body. Test: test/package-files.test.ts's size budget test passes with the measured package; it failed in CI at 3e63c5e. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Second review (verification-scoped)Verdict: PASS on dd6b301. I found no route that main refuses and this head lets through, and no false safety claim. CI's This review is verification-scoped, per the owner's decision of 2026-10-21. It re-ran the documented probe groups and the fail-without test, checked the claims against the code, and built no new payloads. The comment names groups and IDs only (#707). 1. What changed since 3e63c5e (measured)
2. Documented probe groups: head dd6b301 vs origin/main 1cee9bd (measured)All groups ran unchanged on scratch checkouts with a temp HOME and TMPDIR.
Across all four groups, the comparison found no ID that the head allows and main refuses. Still committed at head, the same as on main:
These are exactly the residuals of the last round. They are the shell-feature cases disclosed in "What remains" bullet 4 (#983). 3. Fail-without and tests (measured)
4. Package budget (measured)
5. Claims vs codeThe guard code is byte-identical to 3e63c5e, so last round's claim checks and line references still hold (measured: no
Required changesNone. Digest
Refusals: none. |
Delta final check (round 6 + package budget): CHANGES NEEDED on dd6b301Head checked: Summary. Nothing that main refuses is let through at this head (measured). Every round-5 finding is fixed as written. The budget raise is correct and named as a loosening. CI is green. Two gaps of the same class as R5-1/R5-5 and DF1 remain, and both are the same as on main: R6-1 and R6-2 below. 1. Round-5 findings and the second review's points
2. New findingsR6-1: pointer spellings git-lfs accepts are not read as pointers.
R6-2: an interactive staging step runs the editor after the scan.
3. Weakening gate (changed existing test lines, ce7d4ab to dd6b301)
4. Package budget (principle 9 loosening, independent review)
5. Other checks
Required changes
Neither is a regression against main, and both follow from principles 1 and 5, so neither needs an owner decision. The only owner item is the 2%-vs-5% budget comment in section 4, which doesn't block this PR. |
Review round 7 for #971 (#979). The commit scan covers every commit form and checkout the guard allows; this round: - Under the standard large-file filter, a small large file that names a pointer version in any spelling but is not the exact pointer form refuses the commit, since the large-file program may still read it as a pointer to an object the scan can't see. - A large-file object that is not a regular file refuses the commit with its reason, before anything reads it. - Interactive staging, and an interactive commit, in the line are refused whatever the config, as edited staging already was: either can change what is committed after the scan. - The large-file test keeps its case with the required key left out, next to the case with it set to another value. Test (fails without): test/adversarial/commit-scan-pointers.test.ts, all 3 of its tests fail at dd6b301; test/adversarial/commit-scan-fifo.test.ts is head-only (on a build without the check its read blocks the process). Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Second review (verification-scoped): PASS on 85a5bbdScope (owner, 2026-10-21): this review re-ran the documented probe groups and the round-7 fail-without test, and checked the claims against 1. Documented probe groups, head vs main (all measured)Each group was run unchanged except for the worktree path, in scratch checkouts with a temporary HOME and isolated git config.
The routes still open at head are open on main too (measured). They fall in the classes the PR body discloses under "What remains", bullet 4: builtins that evaluate their arguments, a builtin that re-points a command name, traps, programs that run a command string, a script path that reads the command's input, and process substitution (r3 H/T/TR/SC/FL/DS/PS/SO/FX/U9/U11). 2. Fail-without (measured)
3. Claims against the codeRound 7:
What this can never do:
Compromised component:
What remains:
VerdictPASS on 85a5bbd.
|
Delta final check (round 7): CHANGES NEEDED (PR body only) on 85a5bbdHead checked: Summary.
1. Round-6 findings
2. The pointer-marker refusal: what it costs (measured, scratch checkouts, a git-lfs stand-in that cleans to the exact pointer)
3. Weakening gate: changed existing test lines, dd6b301 to 85a5bbd
4. The named-pipe test and CI
5. Other checks
Required changes
This is a body edit. It doesn't need a new head, a fail-without or a CI run. It follows from principle 5, so it isn't an owner decision. Recommended, not required:
|
Delta final check (round 7), body re-check: PASS on 85a5bbdThe head is unchanged:
Nothing remains from my earlier comment. |
The package budget line conflicted: main's values (2,170,000 / 6,750,000) are kept, as the merged package measures 2,085,053 / 6,541,354 with the web build, within them. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Delta check (merge-only head c935e7e): CHANGES NEEDED (PR body only)Head c935e7e = merge of origin/main 8df2304 (#918) into 85a5bbd. The code is fine; one body section is no longer true. Checks (all measured):
REQUIRED (c), overclaim in the body:
Follow-ups (not required): none. Note (inferred): the body's adversarial pass already lists a round 7, and DELTA's round cap says no PR goes past round 6 without the owner's rethink. The orchestrator should decide whether this body-only fix needs the owner. |
Delta check, body re-check (head c935e7e): PASSThe head is unchanged at c935e7e; only the body was edited. I read the body through the raw API. The required change from issuecomment-6332886090 is fixed (measured):
The code checks and CI from the previous comment still stand: run 39722718259 succeeded at this head, and that run includes the package-size test. |
Second review (verification-scoped): PASS on c935e7eHead checked: Scope (owner decision): this review re-ran the documented probe groups (rounds 2 to 7) and the round-7 fail-without, and checked the body against the merged code. It built no new payloads. Probe lines stay with the orchestrator (#707), so this comment uses IDs only. 1. What the merge changed (measured)
2. Documented probe groups, head vs main 8df2304 (measured)Each group ran unchanged on both builds, with a temporary HOME, TMPDIR and state dirs, isolated git config, and recording stand-ins (exit 1) first on PATH for gh, sudo, systemctl, launchctl, loginctl, docker, claude, tailscale, useradd, usermod, setfacl and id.
Details:
3. Fail-without and tests (measured)
4. Body claims after the merge
VerdictPASS on c935e7e.
Required changes: none. |
Main 7b13c8b (#746, #979). The one conflict was test/review.test.ts's locked GUARD_TREE list, resolved as the union of both sides: this branch's src/confusables.ts and src/confusables-data.ts, and main's src/coordinator/change-files.ts and src/guard/commit.ts. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Conflicts, each resolved to keep both sides: - .worklane/review.yaml: the engine-guards list keeps test/git-runner.test.ts and #903's vacuity tests. - src/coordinator/change-files.ts: the runner's import; main's local SizeFile replaces the pr-rules import. - src/coordinator/merge-flow.ts: the runner's imports with main's randomBytes; isAncestor keeps main's arguments through gitSpawnSync. - src/scan/secrets.ts: main's new content-based scanCommit (#979) whole; the runner's imports for scanRange. - src/worktrees.ts: main's removed() split, with the runner's calls. Every raw git start the merge brought now goes through the runner, with the same arguments and options: merge-flow.ts (the tip fetch, sync and async; merge-base; rev-list --merges; vacuityFor's git; branch -D; the changed-lines spawn), scan/secrets.ts (gitOut, headId, the core.fsmonitor read), worktrees.ts (removeWorktreeAsync), hook.ts (gitQuery) and vacuity.ts (baseTextReader). No entry is added to the scan's exception list. Its src/scan/secrets.ts entry shrinks from 2 starts to 1, since #979 took the commit scan's gitleaks `git` call out; the pin on that call is replaced by one on scanRange's `--log-opts` snapshot range. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Intent: the commit scan now covers every commit form and checkout the guard allows. Before an agent's command commits, the guard models the command line (new
src/guard/commit.ts): which checkout the commit lands in and what it will record. It scans that content itself (objects by id and files from disk, with the changed paths from the change readersrc/coordinator/change-files.ts), never a diff git renders, across the whole checkout. A command line whose commit it can't model, a directory change it can't resolve exactly, a git command whose words the shell could change, or a place the commit may run that isn't a checkout it can read, is refused with a reason, never scanned narrowly or skipped. Plain commits keep their behaviour: their new staged lines are scanned; people's own sessions are not scanned (they opt in with the git pre-commit hook).Risk class: protected. It changes the agents' tool guard (
src/hook.ts, a newsrc/guard/module,src/scan/secrets.ts), the locked guard-tree pin intest/review.test.ts(two modules added:src/guard/commit.tsandsrc/coordinator/change-files.ts; nothing removed), and moves theSizeFiletype fromsrc/pr-rules.tsintosrc/coordinator/change-files.ts(a type only, re-exported fromsrc/pr-rules.ts) so the guard's tree grows by that one module.Closes #971
How it was verified
test/adversarial/commit-scan.test.ts, written as a new file into a worktree at 9f9533a and run there:# pass 2,# fail 8, each failureactual: 'allow'.test/adversarial/commit-scan-routes.test.ts, written as a new file (byte-identical, sha256 checked) into a worktree at a40dafc (this PR's previous head) and run there:# pass 1,# fail 6. The passing one keeps plain commits as they were. A per-line run of the same lines at a40dafc let through every route the second review and the final check named (scratch file, not committed).commit-scan.test10/10,commit-scan-routes.test7/7,commit-plan.test6/6.change-files-renames(the change-reader lint CI flagged: now clean),review(guard-tree pin),pr-rules,install,hook(the recorded guard-answer corpus: unchanged),push-hook-guard,docs-guardrails,changelog,merge-policy: 228/228 in one run.adversarial.test: 96 pass, 0 fail.test/install.test.tsalready does; they skip where it isn't installed.npm run typecheckclean;npm run check:denylist1442 files clean;check-dco.mjs origin/main HEADall signed off; push gate onorigin/main..HEADclean.npm pack --dry-run --json, no web build on either side, before the merge of main): tarball 1,484,470 → 1,497,197 (+12,727); unpacked 4,971,214 → 5,019,503 (+48,289). The delta is the new guard module and the content scan (inferred: test files are not in the package). Correction (round 6): these numbers had no web build, and the budget counts it; with the web build the package went over the unpacked budget, see "Package budget" below.New dependencies
None.
What ordinary work this now refuses
Each is refused with its reason, and a way to do it instead (measured with the final check's ordinary-work probe and the unit tests):
env -S).git add -p/-iin any spelling before a commit in the same line, andgit commit -p/--interactive. Way around: stage in a call of its own, then commit.What this can never do
git-lfsmay store objects in the checkout's large-file store during that listing. Neither changes what is committed..gitleaksignoreis honoured, not one on disk and not one staged in the commit, and no file of the commit can stand in for one (corrected in rounds 3 and 4). The checkout's.gitleaks.tomland inline allow comments are honoured as on main; they belong to secret scans: ignore suppressions an agent can write #980.Compromised component
src/guard/**is locked with the guard, andsrc/coordinator/**is locked as the coordinator.src/guard/commit.tsor the change reader, but both are locked, so the change waits for the owner (inferred fromsrc/review.ts).What remains
git-lfscomes first on its path. The guard reads the hook's PATH and checks that the firstgit-lfsthere is a regular file. It can't verify the program itself, and the agent's shell may have a different PATH; a replacedgit-lfscould change what is stored after the scan has read the file.docs/guardrails.mdlabels the new guarantee "inferred" until CI on main has run its tests.Round 3
Changes (head after the round-3 commit and a signed-off merge of origin/main):
src/scan/secrets.ts#addedLines), so no repo or user config or attribute can change it. Over a fixed work budget it counts the whole new file as added: more is scanned, never less.filterorworking-tree-encodingattribute, from any attributes source, is refused; the scan never runs a filter. A staged change is still read from the index, as stored.[, ls, cat, head, tail, wc, grep, egrep, fgrep, cd, pushd, mkdir, rmdir, rm, cp, mv, touch, stat, file, du, realpath, readlink, basename, dirname. Each prints or tests text, lists, moves or reports on files, or is a shell builtin with no option that runs a program (inferred from their documented options). gh, man, rg, info, whatis and apropos are off it.Verification:
test/adversarial/commit-scan-content.test.ts, written as a new file (sha256 checked byte-identical) into a worktree at 9c6761f and run there:# pass 0,# fail 7..gitleaks.toml(secret scans: ignore suppressions an agent can write #980), and the config-run programs in What remains.-cscript; xargs with a replace string.-execwith a literal program, xargs without a replace string, loops, pipes into ordinary programs, python here-documents,bash -cwith literal text, quoted expansions in arguments.origin/main..HEADclean.Round 4
Changes (head 1e210a9; origin/main hadn't moved, so no merge):
Every placement of added lines (E1): each run the line diff marks as added is widened across the unchanged lines it could slide over: up while the line before the run equals the run's last line, down while the line after it equals its first. So every placement any diff could choose is scanned (
src/scan/secrets.ts#slideWidened). Measured: the three multi-line shapes the reviewer sent (two of them the same on main) are found, staged and from the working tree.No file stands in for the scanner's own (E2): the commit's files are written under a fixed folder below the scan's root; the round-3 rename is gone. Measured: an ignore-list name and a near name staged together, and a file named like that folder, are found.
Ident attribute (E3): a working-tree file with an
identattribute refuses the commit, asfilterandworking-tree-encodingalready did. Tracked and untracked forms tested.Config-run programs (second review): before it reads anything, the scan reads the checkout's effective config (
git config --list, which runs no program). It refuses the commit when the config names:Fail closed. Measured: the second review's F1–F3s and O3, and the final check's O1 and O2, are refused. A monitor set to off still commits.
Exempt list (E5): hash, test,
[and printf are off it. It is now: echo, which, type, whereis, help, ls, cat, head, tail, wc, grep, egrep, fgrep, cd, pushd, mkdir, rmdir, rm, cp, mv, touch, stat, file, du, realpath, readlink, basename, dirname. Each prints text, lists, moves or reports on files, or is a lookup builtin, with no option that runs a program (inferred from their documented options).Over-refusals fixed (E6): a
[is a pattern only with a]after it in the same word, and[/[[as a command name is a name. After a shell keyword, a command's own variable assignments are skipped too. For xargs, only its own options are read: they stop at the command it runs. Tested: test commands,if/whilewith tests, and xargs runs without a replace string are allowed again; xargs with a replace string is still refused.Read-only programs before a commit (E7, a loosening named under principle 9): round 3 let head, tail, wc, grep and ls run before a commit in the same line, next to true,
:, pwd, echo and cat. Why it's safe: they read and print only, with no option that writes a file or runs a program. A redirect or tee that writes a file is still refused (measured with the final check's read-only probe). The check this exempts is "nothing but reads, directory changes and staging before a commit"; it wasn't wrong, but its list was too short for ordinarygit log | head && git commitlines.Weakening gate: two existing test lines in this PR changed. Both still require a refusal:
commit-scan-content.test.ts, the staged case with a configured filter now expects a refusal ("could not run") instead of "allow".commit-scan-routes.test.ts, the text-conversion and external-diff display shapes now expect the config refusal instead of a finding.Verification:
test/adversarial/commit-scan-reads.test.ts, written as a new file (sha256 5419da3e…, byte-identical) into a worktree at 1a731dc and run there:# pass 0,# fail 6.Round 5
Changes (head 1d1966f, after a signed-off merge of origin/main 1808043):
src/scan/secrets.ts#introducedholds the rule.[counts as a pattern when a]follows it anywhere in the word, across escaped or quoted separators.gpg.*program) is refused when the line shows commits and asks for signatures, by option, by a signature format, or by the log config.-C.Verification:
test/adversarial/commit-scan-findings.test.ts, written as a new file (sha256 72db0afb…, byte-identical) into a worktree at 1e210a9 and run there:# pass 2,# fail 8. The two that pass are:Round 5b (owner's decision on large files, 2026-10-21)
A narrow, owner-approved loosening of the filter refusal added in round 4 (principle 9).
filter.lfs.clean = git-lfs clean -- %f,filter.lfs.smudge = git-lfs smudge -- %f,filter.lfs.process = git-lfs filter-processandfilter.lfs.required = true. Every value the effective config gives for those keys, from any config source, must equal that string, and the large-file store must be at its default place.git-lfsfirst on the path that isn't a regular file. What the guard can't verify about that program is in What remains.Verification:
test/adversarial/commit-scan-lfs.test.ts, written as a new file (sha256 e664f9cd…, byte-identical) into a worktree at 1d1966f and run there:# pass 3,# fail 2.git-lfsthat isn't a regular file.git-lfsin the scan's own change listing; with git-lfs installed, as on CI runners, that is the program the owner allowed.Round 6
Changes (head 3e63c5e, after a signed-off merge of origin/main 1cee9bd):
lfsdriver gets the allowance.lfs.*key outside a short safe list (remote URLs, lock checking, fetch filters, timeouts, per-remote access) takes the allowance away. That includes extension programs, a moved store and custom transfers.difforshow;logwith an option outside a short list of options that never take the next word;statuswith an option outside a short list;logorshowrefuses it.git config --type=bool); a value git doesn't read as a boolean is a program.GIT_CONFIG_NOSYSTEMand a scratchGIT_CONFIG_GLOBAL, so the runner's own git config can't change them. The large-file "not required" case now setsrequiredto a non-standard value rather than leaving it out.logbefore a commit to be refused when a signature program is set. It used to expect "allowed", so this is a tightening.Verification:
test/adversarial/commit-scan-stored.test.ts, written as a new file (sha256 3dd52ad6…, byte-identical) into a worktree at ce7d4ab and run there:# pass 0,# fail 8.Round 7
Changes (head 85a5bbd; origin/main hadn't moved):
add -p,-i,--patch,--interactive, any prefix) and an interactive commit (commit -p,--patch,--interactive) are refused in the line whatever the config, as edited staging (add -e) already was. Either can open an editor, from config or from the environment the guard can't see, after the scan has read.false.commit -pamong commits scanned with working-tree content; it is now refused.Verification:
test/adversarial/commit-scan-pointers.test.ts, written as a new file (sha256 1d094c07…, byte-identical) into a worktree at dd6b301 and run there:# pass 0,# fail 3.test/adversarial/commit-scan-fifo.test.ts. On a build without the check its read blocks the whole test process, so it can't be part of a fail-without run; it passes at this head.commit-plan.test.ts) and the related set: 378 pass, 0 fail, 12 todo. Typecheck clean; denylist 1506 files clean; DCO all signed off; push gate clean.npm pack --dry-run --json --ignore-scripts): at 85a5bbd, tarball 2,045,483 and unpacked 6,404,343, within that head's raised budget (2,050,000 / 6,730,000). Superseded by the merge of main: see "Package budget".Package budget (superseded by the merge of main)
Now (head c935e7e, measured):
test/package-files.test.ts; the merge took main's line, and the PR's own raise below is gone.npm pack --dry-run --json --ignore-scriptsmeasures tarball 2,085,053 and unpacked 6,541,354, within main's budget. The package-size test passes.History (heads dd6b301 to 85a5bbd):
PACKAGE_BUDGET.unpackedSizeintest/package-files.test.tsgoes from 6,400,000 to 6,730,000. The tarball budget (2,050,000) is unchanged; that dimension isn't over.npm run buildandnpm run build:web,npm pack --dry-run --json --ignore-scripts): tarball 2,045,128 (budget 2,050,000); unpacked 6,402,689, over the 6,400,000 budget by 2,689. CI shard 1/4 of run 39685972735 at 3e63c5e measured the same 6,402,689.src/scan/secrets.ts, on top of what main has added since the budget was set.Adversarial pass
Every attack below was run against this branch with scratch checkouts and the test token only (measured), grouped generically under the #707 rule; the exact command lines are with the orchestrator.