feat(groom): extract CI-privileged patch path policy into tested patch_policy.py (BE-4404)

[mattmillerai]

ELI-5

The groom auto-builder lets a (prompt-injectable, credential-free) AI agent write a code patch, and a later job opens a review-gated PR from it. But review only gates the merge — pushing the branch already runs the repo's CI, with secrets, before any human looks. So if that patch edits a file CI executes, it's arbitrary code execution in credentialed CI. There was already a deny-list that downgrades such a patch from an auto-PR to a plain filed issue — but it was a one-line grep nobody could test, and it missed the scariest case: dependency lockfiles. Rewrite a lockfile's resolved URL + integrity hash and the next npm ci downloads and runs an attacker's install script. This PR moves the deny-list into a small, unit-tested Python file and plugs that gap (plus .husky/, action.yml, and build files for many ecosystems).

What changed

• .github/groom/patch_policy.py (new) — denied_paths(paths) returns the CI-privileged subset of changed paths; main() reads NUL-delimited paths from stdin (matching git diff --cached --name-only -z) and prints matches, exit 0 always (the caller tests non-emptiness). Ports the pre-existing patterns verbatim, then adds: lockfiles (package-lock.json, npm-shrinkwrap.json, yarn.lock, pnpm-lock.yaml, bun.lock/bun.lockb, poetry.lock, uv.lock, Pipfile.lock, Cargo.lock, Gemfile.lock), .husky/ (any segment), action.yml/action.yaml, .gitmodules, requirements*.txt, Pipfile, Cargo.toml, build.rs, Package.swift, *.pbxproj, *.gradle/*.gradle.kts/settings.gradle, gradle/wrapper/, Gemfile, Rakefile, *.gemspec, CMakeLists.txt/*.cmake, Bazel WORKSPACE(.bazel)/BUILD(.bazel)/*.bzl, Jenkinsfile, Taskfile.yml, just(J)file.
• .github/groom/tests/test_patch_policy.py (new) — 19 tests: every ported + added pattern family at root and nested depth, anchored negatives (src/foo.py, README.md, packages.json, Dockerfile.md, … must NOT match), and the git raw-byte regressions (embedded " and embedded newline still caught from -z bytes; both newline halves tested).
• groom.yml Capture patch step — the inline grep -E becomes git -C repo diff --cached --name-only -z | python3 "$GROOM_ASSETS/patch_policy.py". GROOM_ASSETS is the existing workflow-env var pointing into the _groom_assets checkout the build job already does (Load groom assets (builder brief) runs before this step). The stale comment block is trimmed to a pointer keeping the two load-bearing notes (conservative default; NUL-delimited comparison mandatory), and the bail reason string now names the policy file.
• README.md + groom.yml header — mention the policy file and its structural limit: the deny-list guards privileged-config surfaces, but any patch's source still executes when caller CI runs its tests — callers should avoid exposing secrets to test steps and consider npm ci --ignore-scripts where viable.

Verification

• actionlint .github/workflows/groom.yml — clean (includes shellcheck on the run: block).
• python3 -m unittest discover -s .github/groom/tests -p 'test_*.py' — 78 tests OK (ledger 59 + patch_policy 19); test_patch_policy and test_ledger both auto-discovered by the existing test-groom-scripts.yml pattern, no workflow change needed.
• End-to-end pipeline simulated: printf '.github/workflows/ci.yml\0package-lock.json\0src/app.py\0.husky/pre-commit\0tools/action.yml\0README.md\0' | python3 patch_policy.py prints exactly the four privileged paths; clean-only input prints nothing; empty stdin → exit 0.

Acceptance criteria

• ✅ actionlint clean; ledger + new policy tests green locally.
• ✅ Behavior identical for previously-covered patterns (PortedPatternsTest pins the full ported set at root + nested).
• ✅ New coverage: package-lock.json, .husky/pre-commit, tools/action.yml each bail.
• ✅ Bail semantics unchanged — a denied patch still bails → lands in the ledger and files a groom issue; nothing dropped.
• ✅ No change to the builder: false default; no caller touched.

Judgment calls

• || true kept on the assignment. The original wrapped the grep in || true (grep exits 1 on no-match, the common case). patch_policy.py exits 0 always, so || true now only masks a genuine git/python3 failure — preserving the original posture that this line never aborts the step under set -euo pipefail. Dropping it would instead abort the matrix leg on any error, which (per the step's own secret-scan comment) drops the finding entirely rather than filing it. The script is unit-tested and SHA-pinned via workflows_ref, so a broken script is caught by test-groom-scripts.yml before merge. Flagged rather than silently changed.
• .github/(workflows|actions)/ stays root-anchored (verbatim port), while newly-added .husky/ and gradle/wrapper/ match at any depth per the ticket — documented inline. Over-blocking is the safe direction, but the ported pattern is kept byte-for-byte to guarantee no regression on the previously-covered set.

Negative-claim falsification

Not applicable in the #581 sense: this diff does not deny a product capability or add a "not supported"/STOP dead-end. "Denied" here means a builder patch is downgraded from an auto-PR to a filed issue — the finding is still delivered and the human authors the privileged change. Nothing is dropped. The deny behavior is empirically validated by the end-to-end pipeline simulation above, not merely asserted by self-authored tests.

BE-4404

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 980e16fc-2629-4d28-94a3-ff80a94a166c

📥 Commits

Reviewing files that changed from the base of the PR and between 29a81ca and ef119a8.

📒 Files selected for processing (4)

• .github/groom/README.md
• .github/groom/patch_policy.py
• .github/groom/tests/test_patch_policy.py
• .github/workflows/groom.yml

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-4404-groom-patch-policy

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-4404-groom-patch-policy

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 9 finding(s).

Severity	Count
🟠 High	4
🟡 Medium	4
🟢 Low	1

Panel: 8/8 reviewers contributed findings.