CLI-279 Fail soft on hook crash outside CI

[kirill-knize-sonarsource]

No description provided.

[sonar-review-alpha]

Summary

This PR changes git hooks (pre-commit and pre-push) to gracefully handle scan failures differently depending on the environment. In CI (CI=true): failures block the operation (existing behavior). Outside CI: failures log a warning and allow the operation to proceed, so a broken secrets scan doesn't interrupt local development workflows.

The implementation adds a handleScanError() function that checks process.env.CI and either throws or warns. Both hook handlers now call this function instead of unconditionally throwing. The pre-push logic is refactored into a scanRef() function to support early exit when scanning fails outside CI.

Test coverage includes new cases for fail-soft behavior alongside updated assertions that the fail-hard behavior still works in CI.

What reviewers should know

Start with the core logic: Check hook-dependencies.ts:handleScanError() — this is the decision point for the entire feature. Then look at how it's called in git-pre-commit.ts (simple) and git-pre-push.ts (more involved due to the ref-scanning loop).

Non-obvious changes:

• The git-pre-push.ts refactor introduces a scanRef() function that returns boolean to signal success/failure, enabling the caller to break after the first failure. This is necessary because you can't throw in a loop and continue to the next iteration outside CI.
• Both hooks now return without throwing after calling handleScanError() outside CI, allowing the hook to exit 0.
• Existing unit tests were updated to explicitly set process.env.CI = 'true' to preserve their original intent, with cleanup in finally blocks.

What to watch:

• The ref-scanning loop in pre-push now has early-break behavior only when outside CI — verify this doesn't mask legitimate failures in CI (it shouldn't, but trace through the control flow).
• The warning message text is user-facing — check if it's appropriate for the audience.
• Tests mock readGitPushRefs() — make sure the new test for "stops scanning after first crash" (runSecretsBinarySpy called once) correctly reflects the expected behavior.

---

• Generate Walkthrough
• Generate Diagram

🗣️ Give feedback

[hashicorp-vault-sonar-prod]

CLI-279

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues
0 New dependency risks

Measures
0 Security Hotspots
96.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[sonar-review-alpha]

Three previously flagged issues remain open; none have been fixed in this revision. Until all three are addressed, the PR cannot be approved.

🗣️ Give feedback

[sonar-review-alpha]

Bug: process.env.CI === 'true' is still an exact-string match. The previous review comment on this line flagged that several common CI platforms (Azure DevOps, custom runners with CI=1 or CI=yes) will not match this check and will silently skip enforcement. A truthy check is safer and matches the convention used by Jest, create-react-app, and most JS tooling.

Suggested change

	export function handleScanError(context: 'Commit' | 'Push'): void {
	if (process.env.CI) {

• Mark as noise

[sonar-review-alpha]

Bug: The original error is still not surfaced in the fail-soft path. handleScanError() accepts no error parameter, so the debug log at the call site already printed the message, but the warn() shown to the user is always the same generic string regardless of what actually went wrong (binary not found, SIGSEGV, network timeout, etc.). Accepting the error here and including at least its message in the warning would make local debugging meaningful without changing the fail-soft contract.

• Mark as noise

[sonar-review-alpha]

Bug: return false is still reached after handleScanError('Push') returns normally (outside CI). This breaks the loop and silently skips all remaining refs — the opposite of the fail-soft contract, which should warn but keep scanning. The return false / break pattern is only correct for the CI throw path; it should not apply when the soft path falls through.

Fix: return true after handleScanError so remaining refs are still scanned:

Suggested change

	throw new CommandFailedError('Secrets detected in pushed commits');
	}
	handleScanError('Push'); // throws in CI, warns outside CI
	return true;

• Mark as noise