Skip to content

CM-73654: Make the pre-push hook scan under the pre-commit framework - #553

Open
omer-roth wants to merge 1 commit into
mainfrom
CM-73654-cycode-cli-pre-push-hook-never-scans-silent-pass-since-3-13-0-hard-fail-before
Open

omer-roth wants to merge 1 commit into
mainfrom
CM-73654-cycode-cli-pre-push-hook-never-scans-silent-pass-since-3-13-0-hard-fail-before

Conversation

@omer-roth

Copy link
Copy Markdown
Collaborator

The cycode-pre-push hooks never scanned anything when installed through the pre-commit framework, which is the setup our docs describe. From 3.13.0 on they printed Passed and let every push through, secrets included.

The cause: pre-commit reads git's pre-push stdin itself and never forwards it. It passes the push range to hooks as PRE_COMMIT_FROM_REF / PRE_COMMIT_TO_REF instead. The CLI only read stdin, found it empty, and returned without scanning.

What changed

  • Push range from pre-commit (commit_range_documents.py, pre_push_command.py): the hook uses PRE_COMMIT_FROM_REF..PRE_COMMIT_TO_REF. When pre-commit omits them because the push includes the root commit (e.g. the first push to an empty remote), it scans all commits. Hand-written .git/hooks/pre-push hooks keep reading stdin.
  • Fail closed: if pre-commit is running the hook (PRE_COMMIT=1) but gives no push details, the hook exits 1 with a clear error instead of passing. Empty stdin from plain git is still a quiet pass: git sends it on "Everything up-to-date". Tag pushes and branch deletions also still pass (CM-62406).
  • Root commits: --all became first..HEAD, which leaves out the root commit, so a first push of a single commit scanned nothing. The root commit's diff was also reversed (R=True on top of diff-tree --root), so its secrets looked like removed lines and were dropped. This also affected commit-history and pre-receive scans of root commits.
  • Hook manifest: the pre-push hooks set always_run: true and pass_filenames: false.
    • Without always_run, pre-commit skipped the hook when the push's net change had no files (secret added, then deleted in a later commit).
    • Without pass_filenames: false, a large push could run the full scan once per filename batch.
  • Text printer crash (scan_result.py): commit-range detections come back as <sha>/<path> and never matched their document, so the text printer crashed with 'NoneType' object has no attribute 'path'. They now match. A detection whose document still can't be found is printed without a code snippet, never dropped.
  • pre-push is added to COMMIT_RANGE_BASED_COMMAND_SCAN_TYPES, like pre-receive. The printer shows the diff line, and secrets on removed lines are ignored.
  • README: the pinned rev: v3.5.0 is bumped, and the pre-push behavior and hook flags are documented.

Where to start reading

cycode/cli/apps/scan/pre_push/pre_push_command.py → _get_pre_push_commit_range, then get_pre_commit_framework_push_range in commit_range_documents.py.

Verification

Unit tests cover the env-var range, fail-closed, quiet-pass cases, root-commit polarity and document matching.

I also ran real git pushes through pre-commit to a local bare remote, against the real backend:

Scenario Result
First push of a secret in the root commit blocked, violation rendered in text
Secret on an existing branch blocked
Secret added then deleted in the next commit blocked
Clean commit, up-to-date push, tag push pass
Pre-commit without push details exit 1 with error
Hand-written git hook blocked

Not changed

  • HTTP/auth errors stay soft_fail, so a backend outage still lets the push through.
  • SCA/SAST commit-range scans still use first..HEAD for --all.
  • README rev is set to v3.25.0, assuming that's the release carrying this fix.

Fixes CM-73654

🤖 Generated with Claude Code

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.

1 participant