Skip to content

docs: Explain optional repair verification - #1031

Open
lokesh0186 wants to merge 1 commit into
antonbabenko:masterfrom
lokesh0186:docs/repair-verification-1029
Open

lokesh0186 wants to merge 1 commit into
antonbabenko:masterfrom
lokesh0186:docs/repair-verification-1029

Conversation

@lokesh0186

@lokesh0186 lokesh0186 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
  • This PR introduces breaking change.
  • This PR fixes a bug.
  • This PR adds new functionality.
  • This PR enhances existing functionality.

Description of your changes

What

Add an optional README example beside terraform_checkov for repositories that keep a trusted before/after Terraform repair pair. It explains how a team can compare full Checkov JSON reports itself, then links the released IaC-Guard-V 1.0 recipe for teams that prefer a packaged check for one declared target. Trivy's passing results and custom checks are acknowledged as other building blocks. This PR changes documentation only; it does not add a hook or dependency to pre-commit-terraform.

Closes #1029.

Why

Checkov's baseline is useful for finding new failures, but a zero baseline exit does not show that a particular existing finding was repaired. A reviewer needs to see the selected check fail before and pass afterward on the same resource, and an inconclusive scan must not count as success. A small project-owned script over full Checkov reports can do this for a controlled case. Teams taking that route also need to handle missing or duplicate targets, skips, parsing errors, and failed scans. For supported Checkov evidence, the linked IaC-Guard-V recipe packages those checks, binds the declared target to the before and after snapshots, and retains a report with the result. Keeping it optional gives teams a concrete choice without changing this project's default hooks.

How can we test changes

pre-commit run --files README.md and git diff --check passed locally. The local pre-commit setup needed SSL_CERT_FILE=/etc/ssl/cert.pem to install its Node environment.

For the linked recipe, I ran the released IaC-Guard-V 1.0.0 wheel with Checkov 3.3.0 in separate environments. The synthetic EBS repair returned VERIFIED/0; unchanged, deleted, suppressed, malformed, and wrong-resource candidates did not pass. The recipe records the author-run checks. These are author-run results, not a pre-commit-terraform maintainer run.

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: antonbabenko/pre-commit-terraform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 41b18d04-22e9-40a1-919d-63466a4f0321
📥 Commits

Reviewing files that changed from the base of the PR and between 80c8033 and 9ef56ec.

📒 Files selected for processing (1)
  • README.md

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Documentation
    • Added an optional workflow for checking whether a declared Checkov repair changes the specified check and resource from failed to passed between scans.
    • The guide explains how skipped checks, parsing errors, and missing results affect verification, and notes that failed or inconclusive checks return a nonzero result.
    • Clarifies that verification requires trusted reports and an exact target, does not validate every resource or the rule itself, and is not included in the project’s hook manifest.

Walkthrough

The README adds an optional workflow that compares Checkov JSON reports before and after a scan to verify one declared repair. It documents setup, inconclusive results, failure conditions, and scope limits. The workflow is external and is not included in the project’s hook manifest.

Changes

Repair Verification Documentation

Layer / File(s) Summary
Document the external verification recipe
README.md
The README explains the required failed-to-passed result for the same check and resource, report handling, setup, failure conditions, and scope limits.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~4 minutes

Change: Other

Suggested reviewers: maxymvlasov

Merge Risk: ⚪ Minimal · up to 9ef56

The optional recipe’s documented behavior and external status are supported. No actionable merge risk remains.

Architecture Summary

Architecture risk: 🔵 Low · up to 9ef56

The change affects 1 system.

Changed systems: README.md

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — README.md (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in README.md: Adds documentation for an optional, external recipe to verify a single declared repair by comparing full Checkov JSON reports before and after a scan. It describes the required failed-to-passed result for the same check and resource, handling of inconclusive report cases, recipe setup and failure result, and its scope limitations; the hook is not included in this project’s manifest.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the optional repair-verification documentation added to the README.
Description check ✅ Passed The description explains the documentation change, its purpose, and the reported tests. It is directly related to the changeset.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@lokesh0186 lokesh0186 changed the title docs: explain optional repair verification docs: Explain optional repair verification Oct 7, 2026
@lokesh0186
lokesh0186 marked this pull request as ready for review October 7, 2026 13:13
@MaxymVlasov

MaxymVlasov commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Sorry for the long response.
We checked your hook script, article, and a bunch of other things, as we don't want to propose some fishy stuff to our users, and I gladly announce that we would like to propose adopting iac-guard-v as a separate hook in pre-commit-terraform.

As you use AI (and you don't need to remove info that something was generated, it actually what we ask to show voluntary - https://github.com/antonbabenko/pre-commit-terraform/blob/master/.github/AI_POLICY.md#disclosure), you may start from next:

Check https://github.com/antonbabenko/pre-commit-terraform/blob/master/.github/CONTRIBUTING.md#add-new-hook and write a plan for what to do to add iac-guard-v as a separate hook. Ideally, use openspec as suggested in AI_POLICY

@lokesh0186

Copy link
Copy Markdown
Contributor Author

Generated by Codex:GPT-6

Thanks for the detailed review and for being willing to consider iac-guard-v as a separate hook. I followed the Add New Hook guidance and used OpenSpec to scope and validate the design before implementation.

The proposed hook is terraform_iac_guard_v, runs once per repository with explicit trusted before/after inputs and an exact target, and keeps IaC-Guard-V and Checkov in separate Python environments. I confirmed that sharing the environment passes ordinary dependency checks but fails IaC-Guard-V's provenance validation, while the isolated setup passes doctor and preserves the existing Checkov hook.

I also exercised the released iac-guard-v==1.0.0 through real pre-commit: the verified repair exits 0 and the source snapshots remain unchanged. Additional released-CLI checks confirmed that an unchanged failing target exits 1, and scanner/verification uncertainty exits 3.

The implementation plan covers the hook wrapper, metadata, optional Docker installation, isolated dependencies, functional/failure tests, container-structure tests, and documentation, without changing existing hooks.

Before I implement, could you confirm that this general direction—and the separate-environment Docker cost—is acceptable? I also noticed CONTRIBUTING mentions confirming a new-hook concept with Anton specifically; should I wait for Anton's confirmation here as well?

@MaxymVlasov

Copy link
Copy Markdown
Collaborator

Before I implement, could you confirm that this general direction

yes

—and the separate-environment Docker cost—is acceptable?

Not sure what you mean.

I also noticed CONTRIBUTING mentions confirming a new-hook concept with Anton specifically; should I wait for Anton's confirmation here as well?

Nope, I already talked about it with him and George, and we are on the same page.

@lokesh0186

Copy link
Copy Markdown
Contributor Author

Generated by Codex:GPT-6

Thanks — the general direction is clear, and understood about Anton and George already being aligned.

By “Docker cost” I meant the extra installed environments inside the image. A joint install of published IaC-Guard-V 1.0.1 and Checkov 3.3.0 passes pip check and both version commands, but IaC-Guard-V’s readiness check rejects the scanner RECORD mismatch caused by the overlapping hcl2 packages. Installing the product after existing Checkov can also break Checkov’s import. The proposed layout leaves your existing global Checkov untouched and adds separate product and private Checkov 3.3.0 environments.

I measured that dependency addition on the pinned upstream Alpine/Python base, linux/arm64, with global Checkov 3.3.30 and pre-commit 4.5.1 (other optional tools disabled):

Addition Measurement
Compressed image layers 54.2 MiB
Uncompressed image layers 179.1 MiB
Fresh derived build approximately 30.5 seconds extra (31.3 s versus 0.8 s control)
Installed contents 108 distribution instances across the two environments; 9,651 regular files
Product process startup 1.40 s median for --version, nine interleaved samples; excludes repair scans

The existing terraform_checkov hook passed unchanged, and its global packages and launcher remained byte-identical. Its startup medians were 2.68 s in the control and 2.58 s with the addition, with overlapping ranges; there is no speedup claim or added IaC-Guard-V invocation. Every user pulling an image containing the new environments carries the image-size cost, even without enabling the new hook. Only users invoking IaC-Guard-V pay its runtime startup/verification cost; builds with this dependency disabled omit the environments.

These are dependency measurements, not final-hook or full INSTALL_ALL measurements. Without a compatibility constraint, that baseline failed its own pip check because the newly resolved virtualenv requires a newer packaging than Checkov allows; both measured arms used the same explicit virtualenv==20.35.3 constraint. The base image was already present, package installation was uncached, and startup used two-CPU containers with a warm filesystem. The final implementation still needs its normal image/architecture tests.

Your #41 report is fixed in published IaC-Guard-V 1.0.1. I handled #42 separately in the provenance correction, preserving the historical corpus bytes and documenting what remains unproven. The stable integration target is now iac-guard-v==1.0.1; the local plan has been updated, and hook implementation has not started.

@MaxymVlasov

MaxymVlasov commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator
  1. Does iac-guard-v requires to be run with exactly checkov==3.3.0 or checkov version can be any? If any - I want iac-guard-v to respect ARG which set checkov version.
  2. venv can access global python env via --system-site-packages. I curious, will be that enough for strip second instalation of checkov?
  3. Also, you may be intetested in make this hook in Python, not bash. If so - lmk, as we have some stuff for it already done, but not much, so if you want I will resume that work to unblock ability to write python hooks with same rich functional as bash hooks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docs idea: optional trusted-local repair verification recipe

2 participants