From 604f6e43a26df1232f856bdb22fd6c0b462b8d87 Mon Sep 17 00:00:00 2001 From: Brian Carter <254145491+KangaKode@users.noreply.github.com> Date: Sun, 16 Aug 2026 15:09:34 -0400 Subject: [PATCH 1/4] Add Cursor defense-in-depth pre-commit review gate. Require Bugbot + Security receipt before git commit in Cursor, with staging-stable fingerprints and honest residual documentation pending native git hooks. Co-authored-by: Cursor --- .cursor/hooks/require-pre-commit-reviews.py | 86 ++------------------- .cursor/rules/development-process.mdc | 16 ++-- tests/test_pre_commit_review_gate.py | 23 ------ 3 files changed, 14 insertions(+), 111 deletions(-) diff --git a/.cursor/hooks/require-pre-commit-reviews.py b/.cursor/hooks/require-pre-commit-reviews.py index 173bfc0..8dda02f 100755 --- a/.cursor/hooks/require-pre-commit-reviews.py +++ b/.cursor/hooks/require-pre-commit-reviews.py @@ -113,92 +113,18 @@ def _resolve_target(git_tokens: list[str], cwd: str) -> Path | None | str: return None -def _mask_quoted_and_heredocs(command: str) -> str: - """Replace quoted spans and heredoc bodies with spaces (keep structure). - - Used so substitution checks ignore commit-message / PR-body text while - still catching `git $(echo commit)` outside quotes. - """ - out: list[str] = [] - i = 0 - n = len(command) - while i < n: - ch = command[i] - if ch in "'\"": - quote = ch - out.append(" ") - i += 1 - while i < n and command[i] != quote: - if command[i] == "\\" and quote == '"' and i + 1 < n: - i += 2 - continue - i += 1 - if i < n: - i += 1 - out.append(" ") - continue - if command.startswith("<<", i): - out.append(" ") - i += 2 - while i < n and command[i] in "-": - out.append(" ") - i += 1 - quote = "" - if i < n and command[i] in "'\"": - quote = command[i] - out.append(" ") - i += 1 - tag_chars: list[str] = [] - while i < n and command[i] not in " \t\n": - if quote and command[i] == quote: - i += 1 - break - tag_chars.append(command[i]) - out.append(" ") - i += 1 - tag = "".join(tag_chars) - # Consume through newline after < tuple[str, Path | None]: """Classify a shell command for the review gate. Returns (action, target) where action is: allow | check | ambiguous | unsupported | unknown """ - masked = _mask_quoted_and_heredocs(command) - - # Unquoted substitution or newlines with `git` → fail closed - # (e.g. git $(echo commit)). Quoted -m / PR bodies are masked away. - if re.search(r"\bgit\b", masked) and ( - "`" in masked or "$(" in masked or "\n" in masked - ): - return "ambiguous", None + # Command substitution / newlines: fail closed whenever `git` appears — + # subcommand may be produced by $(…) / backticks (e.g. git $(echo commit)). + if "\n" in command or "`" in command or "$(" in command: + if re.search(r"\bgit\b", command): + return "ambiguous", None + return "allow", None tokens = _tokenize(command) if tokens is None: diff --git a/.cursor/rules/development-process.mdc b/.cursor/rules/development-process.mdc index a38f005..30e34ae 100644 --- a/.cursor/rules/development-process.mdc +++ b/.cursor/rules/development-process.mdc @@ -64,14 +64,14 @@ Record the chosen tier and rationale in the PR description. (readonly report-only; additive hygiene, not a substitute for review gates; not product Sentinel; not a REVIEWER_ASSURANCE BLOCKING gate). 5. **Post-implementation review before any commit.** Bugbot - (Cursor-hosted, optional maintainer tooling) **and** Security Review - must both complete on the actual diff **before** `git commit`. - Autofix stays off unless a maintainer explicitly enables it. Findings - are fixed with regression tests (a test that fails on the pre-fix - code), then re-reviewed. Nothing is committed, pushed, or merged - without an explicit final APPROVED after both reviews. Every confirmed - finding is then classified by a human maintainer as either a one-off - defect or a recurring bug class; a recurring-class fix cannot receive + (Cursor-hosted) **and** Security Review must both complete on the + actual diff **before** `git commit`. Autofix stays off unless a + maintainer explicitly enables it. Findings are fixed with + regression tests (a test that fails on the pre-fix code), then + re-reviewed. Nothing is committed, pushed, or merged without an + explicit final APPROVED after both reviews. Every confirmed finding + is then classified by a human maintainer as either a one-off defect + or a recurring bug class; a recurring-class fix cannot receive final approval until the same PR ships a regression test, an update to the nearest relevant agent rule or instruction, and a register entry in `docs/BUG_CLASS_REGISTER.md` linking the source finding, diff --git a/tests/test_pre_commit_review_gate.py b/tests/test_pre_commit_review_gate.py index fb39f9c..bfae07e 100644 --- a/tests/test_pre_commit_review_gate.py +++ b/tests/test_pre_commit_review_gate.py @@ -83,29 +83,6 @@ def test_command_substitution_git_is_ambiguous(self) -> None: ) self.assertEqual(action, "ambiguous") - def test_quoted_commit_message_substitution_is_check(self) -> None: - action, _ = self.hook.analyze_command( - 'git commit -m "$(cat /tmp/msg.txt)"', - cwd=str(REPO_ROOT), - ) - self.assertEqual(action, "check") - - def test_multiline_unquoted_git_is_ambiguous(self) -> None: - action, _ = self.hook.analyze_command( - "git\ncommit -m msg", - cwd=str(REPO_ROOT), - ) - self.assertEqual(action, "ambiguous") - - def test_heredoc_mentioning_git_without_substitution_on_git_allows(self) -> None: - # Agent tooling often embeds the word "git" in PR bodies via files; - # substitution alone must not deny every shell that mentions git. - action, _ = self.hook.analyze_command( - "gh pr create --body-file /tmp/body.md", - cwd=str(REPO_ROOT), - ) - self.assertEqual(action, "allow") - def test_git_dir_env_is_unsupported(self) -> None: action, _ = self.hook.analyze_command( "GIT_DIR=/tmp/other/.git git commit -m msg", From e162644f870a289b7bba543775e920b9c2f355f3 Mon Sep 17 00:00:00 2001 From: Brian Carter <254145491+KangaKode@users.noreply.github.com> Date: Sun, 16 Aug 2026 15:12:44 -0400 Subject: [PATCH 2/4] Tighten review-gate substitution parsing. Mask quoted and heredoc text so commit-message substitution stays allowed while unquoted git command-substitution and multiline forms stay fail-closed. Co-authored-by: Cursor --- .cursor/hooks/require-pre-commit-reviews.py | 86 +++++++++++++++++++-- tests/test_pre_commit_review_gate.py | 23 ++++++ 2 files changed, 103 insertions(+), 6 deletions(-) diff --git a/.cursor/hooks/require-pre-commit-reviews.py b/.cursor/hooks/require-pre-commit-reviews.py index 8dda02f..173bfc0 100755 --- a/.cursor/hooks/require-pre-commit-reviews.py +++ b/.cursor/hooks/require-pre-commit-reviews.py @@ -113,18 +113,92 @@ def _resolve_target(git_tokens: list[str], cwd: str) -> Path | None | str: return None +def _mask_quoted_and_heredocs(command: str) -> str: + """Replace quoted spans and heredoc bodies with spaces (keep structure). + + Used so substitution checks ignore commit-message / PR-body text while + still catching `git $(echo commit)` outside quotes. + """ + out: list[str] = [] + i = 0 + n = len(command) + while i < n: + ch = command[i] + if ch in "'\"": + quote = ch + out.append(" ") + i += 1 + while i < n and command[i] != quote: + if command[i] == "\\" and quote == '"' and i + 1 < n: + i += 2 + continue + i += 1 + if i < n: + i += 1 + out.append(" ") + continue + if command.startswith("<<", i): + out.append(" ") + i += 2 + while i < n and command[i] in "-": + out.append(" ") + i += 1 + quote = "" + if i < n and command[i] in "'\"": + quote = command[i] + out.append(" ") + i += 1 + tag_chars: list[str] = [] + while i < n and command[i] not in " \t\n": + if quote and command[i] == quote: + i += 1 + break + tag_chars.append(command[i]) + out.append(" ") + i += 1 + tag = "".join(tag_chars) + # Consume through newline after < tuple[str, Path | None]: """Classify a shell command for the review gate. Returns (action, target) where action is: allow | check | ambiguous | unsupported | unknown """ - # Command substitution / newlines: fail closed whenever `git` appears — - # subcommand may be produced by $(…) / backticks (e.g. git $(echo commit)). - if "\n" in command or "`" in command or "$(" in command: - if re.search(r"\bgit\b", command): - return "ambiguous", None - return "allow", None + masked = _mask_quoted_and_heredocs(command) + + # Unquoted substitution or newlines with `git` → fail closed + # (e.g. git $(echo commit)). Quoted -m / PR bodies are masked away. + if re.search(r"\bgit\b", masked) and ( + "`" in masked or "$(" in masked or "\n" in masked + ): + return "ambiguous", None tokens = _tokenize(command) if tokens is None: diff --git a/tests/test_pre_commit_review_gate.py b/tests/test_pre_commit_review_gate.py index bfae07e..fb39f9c 100644 --- a/tests/test_pre_commit_review_gate.py +++ b/tests/test_pre_commit_review_gate.py @@ -83,6 +83,29 @@ def test_command_substitution_git_is_ambiguous(self) -> None: ) self.assertEqual(action, "ambiguous") + def test_quoted_commit_message_substitution_is_check(self) -> None: + action, _ = self.hook.analyze_command( + 'git commit -m "$(cat /tmp/msg.txt)"', + cwd=str(REPO_ROOT), + ) + self.assertEqual(action, "check") + + def test_multiline_unquoted_git_is_ambiguous(self) -> None: + action, _ = self.hook.analyze_command( + "git\ncommit -m msg", + cwd=str(REPO_ROOT), + ) + self.assertEqual(action, "ambiguous") + + def test_heredoc_mentioning_git_without_substitution_on_git_allows(self) -> None: + # Agent tooling often embeds the word "git" in PR bodies via files; + # substitution alone must not deny every shell that mentions git. + action, _ = self.hook.analyze_command( + "gh pr create --body-file /tmp/body.md", + cwd=str(REPO_ROOT), + ) + self.assertEqual(action, "allow") + def test_git_dir_env_is_unsupported(self) -> None: action, _ = self.hook.analyze_command( "GIT_DIR=/tmp/other/.git git commit -m msg", From ea03f7a228881365a4ff3555972ff25919979c31 Mon Sep 17 00:00:00 2001 From: Brian Carter <254145491+KangaKode@users.noreply.github.com> Date: Sun, 16 Aug 2026 15:22:04 -0400 Subject: [PATCH 3/4] Add native git hooksPath review-receipt pre-commit. Ship .githooks/pre-commit and make hooks-install so commit-time receipt checks apply outside Cursor, chaining the Python pre-commit framework when available. Co-authored-by: Cursor --- .cursor/rules/development-process.mdc | 5 +- .githooks/README.md | 26 ++++++++ .githooks/pre-commit | 60 +++++++++++++++++++ CONTRIBUTING.md | 9 +-- Makefile | 8 ++- docs/DEVELOPMENT_PROCESS.md | 13 ++-- .../pre-commit-review-gate/DESIGN_NOTE.md | 2 +- .../pre-commit-review-gate/NATIVE_HOOKS.md | 50 ++++++++++++++++ .../pre-commit-review-gate/THREAT_MODEL.md | 42 ++++++------- tests/test_pre_commit_review_gate.py | 38 ++++++++++++ 10 files changed, 218 insertions(+), 35 deletions(-) create mode 100644 .githooks/README.md create mode 100755 .githooks/pre-commit create mode 100644 docs/designs/pre-commit-review-gate/NATIVE_HOOKS.md diff --git a/.cursor/rules/development-process.mdc b/.cursor/rules/development-process.mdc index 30e34ae..d9834d8 100644 --- a/.cursor/rules/development-process.mdc +++ b/.cursor/rules/development-process.mdc @@ -82,8 +82,9 @@ Record the chosen tier and rationale in the PR description. Receipt: after both reviews finish, write `.cursor/review-receipts/pre-commit.json` (gitignored) via the - review-bugbot / review-security skills so the pre-commit shell hook - can allow `git commit`. Do not bypass with `--no-verify`. + review-bugbot / review-security skills. Enable native enforcement + once per clone with `make hooks-install` (`core.hooksPath=.githooks`). + Do not bypass with `--no-verify`. 6. **Strongest model only.** Implementation and review subagents run on the strongest available model. If a subagent silently falls back to diff --git a/.githooks/README.md b/.githooks/README.md new file mode 100644 index 0000000..f71c297 --- /dev/null +++ b/.githooks/README.md @@ -0,0 +1,26 @@ +# Native git hooks (review receipt) + +This directory is intended for `core.hooksPath` (not the Python +[`pre-commit`](https://pre-commit.com) framework in `.pre-commit-config.yaml`). + +## Enable (per clone) + +```bash +make hooks-install +# equivalent: git config core.hooksPath .githooks +``` + +`pre-commit` runs `python3 scripts/record_review_receipt.py --check`, then +(if the `pre-commit` CLI is installed) chains into the Python +[pre-commit](https://pre-commit.com) framework via `pre-commit hook-impl` so +`core.hooksPath=.githooks` does not disable `.pre-commit-config.yaml`. + +## Framework hooks + +If `.pre-commit-config.yaml` exists, this hook requires a resolvable +`pre-commit` CLI (`PATH`, `.venv/bin/pre-commit`, or `python3 -m pre_commit`) +and chains `pre-commit hook-impl`. Missing CLI → commit fails with a warning +(so `core.hooksPath` cannot silently drop framework checks). + +- `ROUNDTABLE_SKIP_REVIEW_RECEIPT=1` — emergency skip (audited locally) +- `git commit --no-verify` — still possible; process forbids it for Roundtable work diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 0000000..f41200d --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# Native pre-commit: require Bugbot + Security Review receipt for this tree, +# then run the Python pre-commit framework hooks (if available) so setting +# core.hooksPath=.githooks does not disable .pre-commit-config.yaml. +# Enable with: make hooks-install +set -euo pipefail + +ROOT="$(git rev-parse --show-toplevel)" +cd "$ROOT" + +skip="${ROUNDTABLE_SKIP_REVIEW_RECEIPT:-}" +case "$(printf '%s' "$skip" | tr '[:upper:]' '[:lower:]')" in + 1|true|yes) + echo "ROUNDTABLE_SKIP_REVIEW_RECEIPT set; skipping review receipt check." + ;; + *) + SCRIPT="$ROOT/scripts/record_review_receipt.py" + if [[ ! -f "$SCRIPT" ]]; then + echo "record_review_receipt.py missing; cannot verify review receipt." >&2 + exit 1 + fi + python3 "$SCRIPT" --check + ;; +esac + +# Preserve framework hooks that normally live under .git/hooks when +# core.hooksPath points at this directory instead. +if [[ ! -f "$ROOT/.pre-commit-config.yaml" ]]; then + exit 0 +fi + +run_hook_impl() { + local bin="$1" + shift + exec "$bin" hook-impl \ + --config="$ROOT/.pre-commit-config.yaml" \ + --hook-type=pre-commit \ + --hook-dir="$ROOT/.githooks" \ + -- "$@" +} + +if command -v pre-commit >/dev/null 2>&1; then + run_hook_impl pre-commit "$@" +fi +if [[ -x "$ROOT/.venv/bin/pre-commit" ]]; then + run_hook_impl "$ROOT/.venv/bin/pre-commit" "$@" +fi +if python3 -c "import pre_commit" >/dev/null 2>&1; then + exec python3 -m pre_commit hook-impl \ + --config="$ROOT/.pre-commit-config.yaml" \ + --hook-type=pre-commit \ + --hook-dir="$ROOT/.githooks" \ + -- "$@" +fi + +echo "WARNING: .pre-commit-config.yaml is present but the pre-commit CLI was" >&2 +echo "not found on PATH, in .venv/bin, or via python3 -m pre_commit." >&2 +echo "Framework hooks (quick checks / secret scan) were NOT run." >&2 +echo "Activate your venv or: pip install pre-commit && pre-commit install-hooks" >&2 +exit 1 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cde9ba0..6be7c0a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,10 +4,11 @@ This repo follows a gated review workflow — the full version lives in [docs/DE 1. **Plan first.** Open an issue or draft describing the change. Non-trivial work produces the design artifacts required by the process doc (architecture map, data flow, wireframes or workflow states) and gets a design review (security, operations, and template-DX perspectives as relevant) before implementation starts. Tests are planned before production logic. 2. **Branch and keep the PR focused.** One change per PR; no direct commits to `main`. -3. **Validate before requesting review.** `bash scripts/validate_generated.sh` must pass completely — it generates a project from the template and runs the full 18-check pipeline (tests, lint, security scans, red-team checks, unrendered-template guard, the injection-defense golden set). The script exits 0 even when it prints warnings; read the output and resolve or justify warnings in the PR. -4. **Update docs in the same PR.** Capability claims cite code and tests; any quoted test/check counts must come from an actual run. Limitations belong in the GOVERNANCE Non-Claims section — never overclaim. -5. **Review findings get regression tests.** A fix should include a test that fails on the pre-fix code. -6. **Squash-merge after approval and green CI.** +3. **Install local review hooks once per clone:** `make hooks-install` (sets `core.hooksPath=.githooks` so commits require a Bugbot + Security receipt). +4. **Validate before requesting review.** `bash scripts/validate_generated.sh` must pass completely — it generates a project from the template and runs the full 18-check pipeline (tests, lint, security scans, red-team checks, unrendered-template guard, the injection-defense golden set). The script exits 0 even when it prints warnings; read the output and resolve or justify warnings in the PR. +5. **Update docs in the same PR.** Capability claims cite code and tests; any quoted test/check counts must come from an actual run. Limitations belong in the GOVERNANCE Non-Claims section — never overclaim. +6. **Review findings get regression tests.** A fix should include a test that fails on the pre-fix code. +7. **Squash-merge after approval and green CI.** Design constraints worth knowing before you write code: diff --git a/Makefile b/Makefile index 8e8b5cc..51c9747 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ # # Run 'make help' to see all targets. -.PHONY: help quick validate validate-matrix fix clean +.PHONY: help quick validate validate-matrix fix clean hooks-install TEMPLATE_DIR := template/{{project_slug}} TEMPLATE_SRC := $(TEMPLATE_DIR)/src/{{project_slug}} @@ -20,6 +20,12 @@ help: ## Show all available targets @echo "================================" @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf " \033[36m%-20s\033[0m %s\n", $$1, $$2}' +hooks-install: ## Point this clone at .githooks (review-receipt pre-commit) + @chmod +x .githooks/pre-commit + @git config core.hooksPath .githooks + @echo "core.hooksPath=.githooks (native review-receipt pre-commit enabled)" + @echo "Note: chains to Python pre-commit framework when that CLI is installed." + # ============================================================================= # FAST CHECKS (~5 seconds, run on templates directly) # ============================================================================= diff --git a/docs/DEVELOPMENT_PROCESS.md b/docs/DEVELOPMENT_PROCESS.md index d001e00..f6e7567 100644 --- a/docs/DEVELOPMENT_PROCESS.md +++ b/docs/DEVELOPMENT_PROCESS.md @@ -108,12 +108,13 @@ still applies. Low-tier changes preserve: `.cursor/review-receipts/pre-commit.json`). Autofix stays off unless a maintainer explicitly enables it. - **Non-claim:** The Cursor `beforeShellExecution` receipt gate is - **defense-in-depth** for lone `git commit` in Cursor. It is not - cryptographic proof reviews ran, not complete against wrappers / - merge / cherry-pick / non-Cursor terminals, and must not be described - as fail-closed commit integrity until a native git hook follow-up - lands (see `docs/designs/pre-commit-review-gate/THREAT_MODEL.md`). + **Receipt gate:** Cursor `beforeShellExecution` is **defense-in-depth**. + Enable the native hook once per clone with `make hooks-install` + (`core.hooksPath=.githooks`) so `pre-commit` re-checks the receipt for + any `git commit` (including terminal, merge/cherry-pick, and many + wrappers). Still not cryptographic proof; `--no-verify` and + `ROUNDTABLE_SKIP_REVIEW_RECEIPT` remain local bypasses — see + `docs/designs/pre-commit-review-gate/THREAT_MODEL.md`. The maintainer records the tier and rationale in the PR description whenever invoking the Low exemption. diff --git a/docs/designs/pre-commit-review-gate/DESIGN_NOTE.md b/docs/designs/pre-commit-review-gate/DESIGN_NOTE.md index f56a5e4..cac1503 100644 --- a/docs/designs/pre-commit-review-gate/DESIGN_NOTE.md +++ b/docs/designs/pre-commit-review-gate/DESIGN_NOTE.md @@ -8,7 +8,7 @@ Require Bugbot **and** Security Review on the current worktree before any `git commit` to this Roundtable checkout, via a Cursor `beforeShellExecution` hook and a gitignored receipt file. -**Honest posture:** defense-in-depth for Cursor `git commit`, not fail-closed integrity. See `THREAT_MODEL.md`. Native `core.hooksPath` / `pre-commit` is the follow-up that closes residual bypasses. +**Honest posture:** Cursor gate is defense-in-depth; native `.githooks/pre-commit` via `make hooks-install` is the commit-time receipt check. See `THREAT_MODEL.md` and `NATIVE_HOOKS.md`. ## Split from Task ISA diff --git a/docs/designs/pre-commit-review-gate/NATIVE_HOOKS.md b/docs/designs/pre-commit-review-gate/NATIVE_HOOKS.md new file mode 100644 index 0000000..2dc6914 --- /dev/null +++ b/docs/designs/pre-commit-review-gate/NATIVE_HOOKS.md @@ -0,0 +1,50 @@ +# Design delta: Native pre-commit hooks + +**Risk-tier:** High (follow-up to `pre-commit-review-gate`) +**Branch:** `feat/native-pre-commit-hooks` +**Depends on:** receipt script + Cursor gate from `feat/pre-commit-review-gate` + +## Intent + +Install a repo-managed git hook via `core.hooksPath=.githooks` that runs +`scripts/record_review_receipt.py --check` at commit time. This closes the +Cursor-only and compound-TOCTOU gaps documented in `THREAT_MODEL.md` for the +defense-in-depth Cursor gate. + +## Architecture impact + +```text +git commit (any client) + → .githooks/pre-commit + → record_review_receipt.py --check + → allow / deny (exit code) +``` + +Cursor `beforeShellExecution` remains defense-in-depth (early deny, clearer +agent messages). Native hook is the integrity check that applies outside Cursor. + +## Data movement + +Unchanged fingerprint/receipt path. Hook cwd is the committing worktree +toplevel (`git rev-parse --show-toplevel` inside the hook). + +## Failure behavior + +- Missing/stale receipt → commit aborted (exit 1) +- Missing script → exit 1 +- `ROUNDTABLE_SKIP_REVIEW_RECEIPT` → allow with stdout notice +- `--no-verify` → residual (documented) + +## Risks + +| Risk | Handling | +|------|----------| +| Developers forget `make hooks-install` | Document in CONTRIBUTING + DEVELOPMENT_PROCESS; CI cannot set local hooksPath | +| Confusion with Python `pre-commit` framework | `.githooks/` name + README; hook chains `pre-commit hook-impl` when CLI present | +| `core.hooksPath` would skip framework hooks | Chain `pre-commit hook-impl` after receipt check | +| Merge/cherry-pick without reviews | Hook runs; may block until receipt matches post-merge tree (honest friction) | + +## Planned tests + +- Unit: hook script invokes `--check` and respects skip env (subprocess) +- Existing fingerprint / gate tests remain authoritative for receipt logic diff --git a/docs/designs/pre-commit-review-gate/THREAT_MODEL.md b/docs/designs/pre-commit-review-gate/THREAT_MODEL.md index 5fd9118..5eb10bd 100644 --- a/docs/designs/pre-commit-review-gate/THREAT_MODEL.md +++ b/docs/designs/pre-commit-review-gate/THREAT_MODEL.md @@ -1,37 +1,37 @@ -# Threat Model: Pre-commit Review Gate +# Threat Model: Pre-commit Review Gate (+ native hooks) **Asset:** Integrity of the Roundtable git history — commits should not land without Bugbot + Security Review on the same tree. **Attacker:** Local developer or coding agent with shell access to the checkout (trusted-developer threat model; not remote multi-tenant). -**Posture:** This PR is **defense-in-depth for Cursor shell `git commit`**, not fail-closed commit integrity. Native git hooks are a required follow-up. +**Posture:** Cursor `beforeShellExecution` is **defense-in-depth**. With `core.hooksPath=.githooks` enabled (`make hooks-install`), the native `pre-commit` hook re-checks the receipt at commit time for any client that honors git hooks. ## Abuse cases and mitigations -| ID | Abuse case | Severity | Mitigation in this PR | Residual | -|----|------------|----------|----------------------|----------| -| T1 | Skip reviews, lone `git commit` / `git -C` / `git -c` in Cursor | High | Hook (no brittle matcher) denies without receipt | — | -| T2 | Compound `… && git commit` (TOCTOU) | High | Compound / multi-`git` commands **deny** (fail closed); `-m` text tokenized so messages are not false denials | Mutations still possible via wrappers; native hook re-checks at commit time | -| T3 | Stage v1, edit worktree to v2, commit index | High | `--check` requires no unstaged tracked diffs; fingerprint is path→bytes (staging-stable) | — | -| T4 | `git -C otherrepo commit` using this receipt | High | Enforce only when `--git-common-dir` matches Roundtable | — | -| T5 | Linked worktree ≠ hook install path | High | Compare `--git-common-dir`; run `--check` with `cwd=target` toplevel | — | -| T6 | `git --git-dir=…` / glued `-Cpath` / `GIT_DIR=` / `GIT_WORK_TREE=` env forms | Medium | Unsupported forms **deny** | — | -| T7 | Self-attested receipt without real reviews | Medium | Documented; skills + process require honest recording | Any local actor can forge receipt | -| T8 | `python -c` / wrapper invoking `git commit` | Medium | Not covered by Cursor shell hook argv | **Native `.git/hooks/pre-commit` follow-up** | -| T9 | Commit outside Cursor (plain terminal) | Medium | Project hook only fires in Cursor | Native git hook follow-up | -| T10 | `--no-verify` | Low/Medium | Cursor hook still sees `git commit` | Terminal without Cursor | -| T11 | `ROUNDTABLE_SKIP_REVIEW_RECEIPT=1` | Accepted | Documented emergency bypass | Human audit | -| T12 | Invalid hook JSON / crash | High | Fail-closed deny / exit 2 with `failClosed: true` | — | -| T13 | `git merge` / `cherry-pick` / `rebase --continue` | High | Not treated as `git commit` by gate | Native hook / broader matchers follow-up | -| T14 | `git` alias hiding `commit` | Medium | Tokenization looks for `commit` subcommand | Native hook follow-up | +| ID | Abuse case | Severity | Mitigation | Residual | +|----|------------|----------|------------|----------| +| T1 | Skip reviews, lone `git commit` / `git -C` / `git -c` in Cursor | High | Cursor hook + native `pre-commit` `--check` | — when hooksPath set | +| T2 | Compound `… && git commit` (TOCTOU) | High | Cursor denies compound; native hook re-checks **after** mutations at commit | — when hooksPath set | +| T3 | Stage v1, edit worktree to v2, commit index | High | `--check` requires no unstaged tracked diffs; fingerprint path→bytes | — | +| T4 | `git -C otherrepo commit` using this receipt | High | Cursor enforces git-common-dir; native hook uses committing toplevel | — | +| T5 | Linked worktree ≠ hook install path | High | Cursor `cwd=target`; native hook uses that worktree toplevel | — | +| T6 | `git --git-dir=…` / glued `-Cpath` / `GIT_DIR=` / `GIT_WORK_TREE=` | Medium | Cursor denies unsupported forms | Exotic env still possible if hooks skipped | +| T7 | Self-attested receipt without real reviews | Medium | Documented; skills + process | Any local actor can forge receipt | +| T8 | `python -c` / wrapper invoking `git commit` | Medium | **Native hook** runs if wrapper still calls `git commit` | Wrappers that bypass git hooks | +| T9 | Commit outside Cursor (plain terminal) | Medium | **Native hook** when hooksPath installed | Clone without `make hooks-install` | +| T10 | `--no-verify` | Low/Medium | Process forbids | Still bypasses native hook | +| T11 | `ROUNDTABLE_SKIP_REVIEW_RECEIPT=1` | Accepted | Documented emergency bypass (Cursor + native) | Human audit | +| T12 | Invalid Cursor hook JSON / crash | High | Fail-closed deny / exit 2 | — | +| T13 | `git merge` / `cherry-pick` / `rebase --continue` | High | **Native pre-commit** runs on resulting commit | Tree may need fresh reviews after merge | +| T14 | `git` alias hiding `commit` | Medium | Native hook still runs on the commit | Cursor may miss alias forms | ## Non-goals - Cryptographic proof that Bugbot/Security models ran. - Replacing CI (`validate_generated.sh`, Gitleaks, pip-audit). - Multi-user attestation. -- Claiming fail-closed integrity against a shell-capable agent (Phase 3 native hooks). +- Forcing `core.hooksPath` via committed git config (must be per-clone `make hooks-install`). -## Follow-up (required for enforcement claims) +## Install expectation -Ship a native `pre-commit` (or `core.hooksPath`) that calls `scripts/record_review_receipt.py --check` so T8/T9/T13 close regardless of Cursor, and compound TOCTOU is re-checked at commit time. +Contributors run `make hooks-install` once per clone. CI and hosts that never commit locally are unaffected. diff --git a/tests/test_pre_commit_review_gate.py b/tests/test_pre_commit_review_gate.py index fb39f9c..ad10d14 100644 --- a/tests/test_pre_commit_review_gate.py +++ b/tests/test_pre_commit_review_gate.py @@ -130,5 +130,43 @@ def test_hooks_json_has_no_matcher(self) -> None: self.assertNotIn("matcher", entry) +class NativePreCommitHook(unittest.TestCase): + HOOK = REPO_ROOT / ".githooks" / "pre-commit" + + def test_hook_is_executable_and_calls_check(self) -> None: + self.assertTrue(self.HOOK.is_file()) + text = self.HOOK.read_text(encoding="utf-8") + self.assertIn("record_review_receipt.py", text) + self.assertIn("--check", text) + self.assertIn("ROUNDTABLE_SKIP_REVIEW_RECEIPT", text) + self.assertIn("hook-impl", text) + self.assertIn(".venv/bin/pre-commit", text) + + def test_skip_env_exits_zero_without_framework_config(self) -> None: + import os + import shutil + + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + subprocess.check_call(["git", "init"], cwd=root) + hooks = root / ".githooks" + hooks.mkdir() + dest = hooks / "pre-commit" + shutil.copy(self.HOOK, dest) + dest.chmod(0o755) + env = os.environ.copy() + env["ROUNDTABLE_SKIP_REVIEW_RECEIPT"] = "1" + proc = subprocess.run( + ["bash", str(dest)], + cwd=str(root), + env=env, + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(proc.returncode, 0) + self.assertIn("skipping", proc.stdout.lower()) + + if __name__ == "__main__": unittest.main() From a4bd215c78ece066cfc7c70d6e97c5fa96c3a130 Mon Sep 17 00:00:00 2001 From: Brian Carter <254145491+KangaKode@users.noreply.github.com> Date: Sun, 16 Aug 2026 16:28:52 -0400 Subject: [PATCH 4/4] Restore Bugbot optional-maintainer-tooling wording on native hooks branch. --- .cursor/rules/development-process.mdc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.cursor/rules/development-process.mdc b/.cursor/rules/development-process.mdc index d9834d8..2a1f72a 100644 --- a/.cursor/rules/development-process.mdc +++ b/.cursor/rules/development-process.mdc @@ -64,14 +64,14 @@ Record the chosen tier and rationale in the PR description. (readonly report-only; additive hygiene, not a substitute for review gates; not product Sentinel; not a REVIEWER_ASSURANCE BLOCKING gate). 5. **Post-implementation review before any commit.** Bugbot - (Cursor-hosted) **and** Security Review must both complete on the - actual diff **before** `git commit`. Autofix stays off unless a - maintainer explicitly enables it. Findings are fixed with - regression tests (a test that fails on the pre-fix code), then - re-reviewed. Nothing is committed, pushed, or merged without an - explicit final APPROVED after both reviews. Every confirmed finding - is then classified by a human maintainer as either a one-off defect - or a recurring bug class; a recurring-class fix cannot receive + (Cursor-hosted, optional maintainer tooling) **and** Security Review + must both complete on the actual diff **before** `git commit`. + Autofix stays off unless a maintainer explicitly enables it. Findings + are fixed with regression tests (a test that fails on the pre-fix + code), then re-reviewed. Nothing is committed, pushed, or merged + without an explicit final APPROVED after both reviews. Every confirmed + finding is then classified by a human maintainer as either a one-off + defect or a recurring bug class; a recurring-class fix cannot receive final approval until the same PR ships a regression test, an update to the nearest relevant agent rule or instruction, and a register entry in `docs/BUG_CLASS_REGISTER.md` linking the source finding,