Skip to content

CLI-298 git pre push#175

Merged
kirill-knize-sonarsource merged 2 commits intotask/kk/CLI-244-245-callback-infrastructurefrom
task/kk/CLI-298-git-pre-push
Apr 16, 2026
Merged

CLI-298 git pre push#175
kirill-knize-sonarsource merged 2 commits intotask/kk/CLI-244-245-callback-infrastructurefrom
task/kk/CLI-298-git-pre-push

Conversation

@kirill-knize-sonarsource
Copy link
Copy Markdown
Member

No description provided.

@kirill-knize-sonarsource kirill-knize-sonarsource changed the base branch from master to task/kk/CLI-244-245-callback-infrastructure April 14, 2026 07:27
@hashicorp-vault-sonar-prod
Copy link
Copy Markdown

hashicorp-vault-sonar-prod Bot commented Apr 14, 2026

CLI-298

@sonar-review-alpha
Copy link
Copy Markdown
Contributor

sonar-review-alpha Bot commented Apr 14, 2026

Summary

This PR moves the git pre-push secret scanning logic from shell scripts into TypeScript. Previously, the scan was implemented as inline shell code embedded in git hook scripts. Now it's a proper CLI command (sonar hook git-pre-push) that parses stdin refs, determines which files are being pushed, and scans them via the existing secrets binary.

The refactoring simplifies the hook scripts (both native .git/hooks/ and Husky) to a single line that invokes the new command, making the logic testable and maintainable. Three integration tests and 15 unit tests verify correct behavior for new branches, existing branch updates, secret detection, graceful skips (auth/binary not available), and edge cases like branch deletion and malformed input.

What reviewers should know

Where to start:

  • Read src/cli/commands/hook/git-pre-push.ts — the main logic that orchestrates the scan
  • Check src/cli/commands/hook/stdin.ts — how push refs are parsed from git's stdin format

Key details for reviewers:

  1. Ref parsing — Git sends refs as space-separated lines: <localRef> <localSha> <remoteRef> <remoteSha>. The code filters out malformed lines and handles the null OID (0000...0000) for branch deletions and new branches.

  2. File discovery differs by ref type:

    • Existing branch push → git diff remoteSha localSha to find changed files
    • New branch push (remoteSha is null OID) → git rev-list to find new commits, then diff each; falls back to empty-tree diff if no other remotes exist
  3. Graceful skip behavior is intentional — If auth fails, binary isn't installed, or git commands error out, the hook returns 0 (allow the push). Only CommandFailedError from the secrets scan itself fails the hook.

  4. Empty tree handling — Calls git mktree once per push to get the canonical empty tree; this correctly handles both SHA-1 and SHA-256 repos. Falls back to null OID if mktree fails.

  5. Integration with shell — The native and Husky hook scripts now just find SONAR_BIN and invoke "$SONAR_BIN" hook git-pre-push. This keeps the scripts simple and readable.

  6. Test coverage — Integration tests use real git repos; unit tests mock git/auth/binary calls. Watch for the test in git.test.ts that updated an error message from 'Secrets found' to 'Secrets detected'.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@kirill-knize-sonarsource kirill-knize-sonarsource changed the base branch from task/kk/CLI-244-245-callback-infrastructure to task/kk/CLI-248-git-hooks April 14, 2026 07:29
sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as resolved.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-298-git-pre-push branch 2 times, most recently from 4724e6a to ba8e2c6 Compare April 15, 2026 11:09
sonar-review-alpha[bot]

This comment was marked as outdated.

Base automatically changed from task/kk/CLI-248-git-hooks to task/kk/CLI-244-245-callback-infrastructure April 15, 2026 13:19
Replaces the embedded shell logic in the pre-push hook script with a
thin launcher that delegates to `sonar hook git-pre-push`. The TypeScript
handler reads pushed refs from stdin, resolves the file list via git
diff/rev-list, and scans with the secrets binary.
@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-298-git-pre-push branch from ba8e2c6 to 0fc66d4 Compare April 15, 2026 13:26
sonar-review-alpha[bot]

This comment was marked as outdated.

Comment thread src/lib/process.ts Outdated
Copy link
Copy Markdown
Contributor

@eray-felek-sonarsource eray-felek-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 comment otherwise tested and LGTM

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@sonar-review-alpha sonar-review-alpha Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! ✅

🗣️ Give feedback

@kirill-knize-sonarsource kirill-knize-sonarsource merged commit d27592a into task/kk/CLI-244-245-callback-infrastructure Apr 16, 2026
14 checks passed
@kirill-knize-sonarsource kirill-knize-sonarsource deleted the task/kk/CLI-298-git-pre-push branch April 16, 2026 10:42
kirill-knize-sonarsource added a commit that referenced this pull request Apr 16, 2026
kirill-knize-sonarsource added a commit that referenced this pull request Apr 16, 2026
kirill-knize-sonarsource added a commit that referenced this pull request Apr 16, 2026
kirill-knize-sonarsource added a commit that referenced this pull request Apr 16, 2026
* CLI-244 sonar callback — command infrastructure

* CLI-245 PreToolUse secrets scanner callback

* CLI-246 Move prompt submit hook from shell scripts (#158)

* CLI-247 PostToolUse SQAA analysis callback (#159)

* CLI-248 git hooks (#160)

* CLI-298 git pre push (#175)
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.

2 participants