feat(guardrails): Wave 9 — 15 hooks for 100% CC parity coverage#134
feat(guardrails): Wave 9 — 15 hooks for 100% CC parity coverage#134
Conversation
Wave 9: implement all 15 remaining hooks in guardrail.ts (1184→1453 lines). Medium priority (6): - block-manual-merge-ops: hard block cherry-pick/rebase/branch rename - pr-guard upgrade: hard block missing issue ref + --base main - enforce-post-merge-validation: high-risk change detection checklist - inject-claude-review-on-checks: CRITICAL/HIGH severity tracking + merge block - post-pr-create-review-trigger: auto /review suggestion after PR creation - verify-state-file-integrity upgrade: SHA-256 checksumming + TOCTOU detection Low priority (9): - enforce-review-reading upgrade: hard block stale review merge - enforce-deploy-verify-on-pr: deploy evidence advisory for infra changes - pre-merge tier-aware: EXEMPT/LIGHT/FULL review tiers - auto-init-permissions: project stack detection on session start - enforce-develop-base: block branch from main when develop exists - enforce-branch-workflow: branch warning on session start - audit-docker-build-args upgrade: full secret pattern scan + hard block - workflow-sync-guard: .github/workflows/ divergence advisory - enforce-seed-data-verification: block unverified seed data writes Closes #51 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
New PR opened -- automated review will run on the next push. To trigger a manual review, comment |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
Pull request overview
This PR extends the Guardrails plugin to reach full Claude Code hook parity by adding the remaining enforcement/advisory hooks in guardrail.ts, primarily around PR/merge workflow gating, secret detection, post-merge validation reminders, and state integrity tracking.
Changes:
- Add new session-start checks (stack detection + branch workflow warning) and multiple new bash/tool guardrails (tiered pre-merge gates, manual merge-op blocking, PR creation constraints, docker build-arg secret blocking).
- Add additional after-hook UX prompts (post-PR-create auto-review prompt, workflow divergence warning, post-merge risk checklist).
- Upgrade state file integrity checks to include SHA-256 tracking and TOCTOU detection.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Block: missing issue reference (Closes/Fixes/Resolves #XX) | ||
| if (!/\b(closes?|fixes?|resolves?)\s*#\d+/i.test(cmd)) { | ||
| await mark({ last_block: "bash", last_command: cmd, last_reason: "PR missing issue reference" }) | ||
| throw new Error(text("PR must reference an issue: include 'Closes #XX' in the body for auto-close on merge")) | ||
| } |
There was a problem hiding this comment.
The new PR-guard hard block checks for Closes/Fixes/Resolves #... by regexing the command string (cmd). For typical gh pr create interactive usage (no --body/-b in the command), this will always fail and block PR creation even if the user adds the issue reference in the prompted editor/template. Consider making this a non-blocking advisory, or enforce it after PR creation by querying the created PR body (e.g., gh pr view --json body) and validating that instead of inspecting cmd.
| // EXEMPT: CI green only (already checked by CI hard block above) | ||
| await seen("pre_merge.tier", { branch, tier, result: "pass" }) | ||
| } else if (tier === "LIGHT") { | ||
| // LIGHT: 3-pass OR (code-reviewer done OR C/H=0 OR manual verified) |
There was a problem hiding this comment.
In LIGHT-tier gating the comment mentions "3-pass" and "manual verified", but the implementation only checks review_state === "done" or CRITICAL/HIGH === 0 with checks timestamp. Either implement the additional allowed paths (3-pass/manual verification flag) or update the comment so it accurately reflects the enforced policy.
| // LIGHT: 3-pass OR (code-reviewer done OR C/H=0 OR manual verified) | |
| // LIGHT: code-reviewer done OR CRITICAL=0/HIGH=0 with review checks timestamp present |
| const { state_sha256: _prev, ...rest } = stateData | ||
| const contentForHash = JSON.stringify(rest, null, 2) | ||
| const hasher = new Bun.CryptoHasher("sha256") | ||
| hasher.update(contentForHash) | ||
| const currentSha = hasher.digest("hex") |
There was a problem hiding this comment.
state_sha256 is computed from rest (which includes volatile fields like updated_at written by mark()), but the subsequent save() call writes a different updated_at (now). This makes the stored SHA inconsistent with the actual file contents and will cause persistent false TOCTOU detections. Compute the hash over the exact object you’re about to persist (excluding only state_sha256), and/or exclude volatile fields such as updated_at/mode from the hash input.
| if (!stateData || typeof stateData !== "object") { | ||
| await seen("state_integrity.corrupted", { reason: "non-object state" }) | ||
| await mark({ last_event: "state_integrity_repair", repaired_at: now }) |
There was a problem hiding this comment.
The integrity check treats any typeof stateData === "object" as valid, which also includes arrays. If the state file is corrupted into an array, destructuring/spreading will behave unexpectedly. Consider explicitly rejecting Array.isArray(stateData) as corrupted and repairing the state.
| if (!stateData || typeof stateData !== "object") { | |
| await seen("state_integrity.corrupted", { reason: "non-object state" }) | |
| await mark({ last_event: "state_integrity_repair", repaired_at: now }) | |
| if (!stateData || typeof stateData !== "object" || Array.isArray(stateData)) { | |
| await seen("state_integrity.corrupted", { reason: "non-object or array state" }) | |
| await mark({ last_event: "state_integrity_repair", repaired_at: now }) | |
| await save(state, { mode, repaired_at: now, repair_reason: "invalid state shape" }) |
- pr-guard issue ref: downgrade from hard block to advisory (cmd may not contain --body content in editor-based flow) - LIGHT tier comment: fix "3-pass" → "2-pass" to match implementation - SHA-256 integrity: exclude updated_at from hash to avoid false TOCTOU, write SHA and timestamp atomically - Array corruption: explicitly reject Array.isArray(stateData) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eads - HIGH-1: consolidate 5 redundant stash() calls in bash before-hook into single bashData read at top of block - HIGH-2: fix LIGHT tier noSevere boolean logic (checksRan + C/H=0) - HIGH-3: move TOCTOU check to START of after-hook (before mark() calls), SHA update stays at END — eliminates false positives - WARN-5: replace console.warn with seen() for CI check failure logging - Remove all duplicate await stash(state) in bash guard chain Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add detailed test methodology for independent engineer evaluation: - Phase 0: full environment setup (CC install, OC build, shared model config) - Phase 1: state reset protocol between scenarios - Phase 2: 10 scenarios with exact prompts and expected hook names for both tools - Phase 3: YAML recording format with stderr/events.jsonl capture - Phase 5: behavioral diff analysis (timing, message quality, edge cases) - Phase 6: deliverable template with reviewer sign-off Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Medium priority (6 hooks)
--base main/reviewsuggestion after PR creationLow priority (9 hooks)
.github/workflows/divergence advisoryTest plan
bun -e "import(...)"— syntax validation passbun turbo typecheck— 13/13 tasks passopencode debug config --print-logs→ plugin load verificationCloses #51
🤖 Generated with Claude Code