CLI-248 git hooks

[kirill-knize-sonarsource]

No description provided.

[sonar-review-alpha]

Summary

This PR migrates git pre-commit hook logic from shell scripts into a new TypeScript handler (sonar hook git-pre-commit).

What changed: The shell scripts (getPreCommitHookScript(), getHuskyPreCommitSnippet()) are now thin wrappers that delegate to gitPreCommit(), which handles staged file detection, authentication checks, and secret scanning. The old inline shell logic (git diff parsing, file filtering, binary invocation) is removed.

Why: Decouples shell scripting from business logic. Makes the code type-safe, testable via unit tests with mocks, and easier to maintain. Graceful degradation (skipping when auth/binary unavailable) is clearer in TypeScript.

Test changes: Added comprehensive unit tests for gitPreCommit() with mocked dependencies, new integration tests using real git repos, and a test helper module. Restructured claude-pre-tool-use tests to front-load graceful skip scenarios. Added hook migration scenario tests (fresh install, old→new conversion, idempotency, hook preservation).

What reviewers should know

Start here:

• Read src/cli/commands/hook/git-pre-commit.ts first — it's the core handler. Note the guard clause ordering (no files → auth → binary → scan).
• Compare git-shell-fragments.ts before/after — shows how thin the wrapper scripts became.

Key decisions:

• Guard order matters: Staged files checked first (cheapest), auth/binary checked before calling runSecretsBinary(). This keeps the hook fast in common cases.
• Graceful skips: All guards return silently instead of erroring. The hook should never break the user's commit flow; it should only block if a secret is actually found.
• Test structure shift: Skip scenarios now tested first with simple assertions (absence of "deny"), then happy path with detailed checks. This reflects real-world priority (preventing false positives matters more than detailed error reporting).

Non-obvious details:

• getStagedFiles() catches all errors and returns [] — protects against missing git binary, missing repo, etc.
• resolveSecretsBinaryPath() is synchronous, not async — binary location is resolved at startup.
• Integration tests use Bun.which('git') to avoid PATH-based git lookup (security check S4036).
• Hook migration scenarios test that old embedded-logic scripts are replaced, and that re-running integrate is idempotent.

Gotchas:

• Pre-commit now exits 1 if secrets found, blocking the commit. Pre-push wasn't touched; verify pre-push logic if you change it.
• "$SONAR_BIN" hook git-pre-commit in shell scripts assumes $SONAR_BIN is set by the bin block (check nativeBinBlock() and huskyBinBlock()).

---

• Generate Walkthrough
• Generate Diagram

🗣️ Give feedback

[hashicorp-vault-sonar-prod]

CLI-280

[sonar-review-alpha]

Conclusion: Solid PR — the migration from inline shell logic to TypeScript callback handlers is clean and well-tested. One logic duplication worth tidying up before merge.

🗣️ Give feedback

[sonar-review-alpha]

Logic duplication: This try-catch block — scanFiles → check EXIT_CODE_SECRETS_FOUND → re-throw as CommandFailedError, with a catch that re-throws CommandFailedError and wraps everything else — is duplicated verbatim in git-pre-push.ts lines 52–61. The only differences are the error message strings ('staged files' vs 'pushed commits') and the debug log prefix.

If the exit-code check or error-wrapping logic ever needs to change (e.g. new exit codes, changed error format, added telemetry), both files must be updated in sync.

Extract a shared helper into secrets-scan.ts (or a new run-secrets.ts), e.g.:

export async function runSecretsOrFail(
  binaryPath: string,
  files: string[],
  auth: ResolvedAuth,
  context: string,
): Promise<void> {
  try {
    const exitCode = await scanFiles(binaryPath, files, auth);
    if (exitCode === EXIT_CODE_SECRETS_FOUND) {
      throw new CommandFailedError(`Secrets detected in ${context}`);
    }
  } catch (err) {
    if (err instanceof CommandFailedError) throw err;
    logger.debug(`${context} secrets scan failed: ${(err as Error).message}`);
    throw new CommandFailedError('Secrets scan failed');
  }
}

Then call await runSecretsOrFail(binaryPath, stagedFiles, auth, 'staged files') and await runSecretsOrFail(binaryPath, files, auth, 'pushed commits') respectively.

• Mark as noise

[eray-felek-sonarsource]

As a user I only see this generic message, maybe we could return more comprehensive error message that includes what caused the failure

[eray-felek-sonarsource]

Maybe comment could be slightly more descriptive about why the regex pattern matters

[eray-felek-sonarsource]

Small comments, otherwise tested and LGTM

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues
2 New dependency risks

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

See analysis details on SonarQube Cloud

[sonar-review-alpha]

LGTM! ✅

The previously flagged logic duplication between git-pre-commit.ts and git-pre-push.ts remains open, but git-pre-push.ts does not exist yet in this codebase — so there is no actual current duplication to fix. The concern is valid for when that handler is eventually added.

🗣️ Give feedback