CM-62406: handle more pre-push types gracefully (#433)

Author: gotbadger

cycode/cli/apps/scan/pre_push/pre_push_command.py

@@ -44,6 +44,10 @@ def pre_push_command(
         timeout = configuration_manager.get_pre_push_command_timeout(command_scan_type)
         with TimeoutAfter(timeout):
             push_update_details = parse_pre_push_input()
+            if not push_update_details:
+                logger.info('No pre-push input found, nothing to scan')
+                return
+
             commit_range = calculate_pre_push_commit_range(push_update_details)
             if not commit_range:
                 logger.info(

cycode/cli/files_collector/commit_range_documents.py

@@ -228,7 +228,7 @@ def parse_pre_receive_input() -> str:
     return pre_receive_input.splitlines()[0]
 
 
-def parse_pre_push_input() -> str:
+def parse_pre_push_input() -> Optional[str]:
     """Parse input to pre-push hook details.
 
     Example input:
@@ -237,13 +237,11 @@ def parse_pre_push_input() -> str:
     refs/heads/main 9cf90954ef26e7c58284f8ebf7dcd0fcf711152a refs/heads/main 973a96d3e925b65941f7c47fa16129f1577d499f
     refs/heads/feature-branch 3378e52dcfa47fb11ce3a4a520bea5f85d5d0bf3 refs/heads/feature-branch 59564ef68745bca38c42fc57a7822efd519a6bd9
 
-    :return: First, push update details (input's first line)
+    :return: First push update details (input's first line), or None if no input was provided
     """  # noqa: E501
     pre_push_input = _read_hook_input_from_stdin()
     if not pre_push_input:
-        raise ValueError(
-            'Pre push input was not found. Make sure that you are using this command only in pre-push hook'
-        )
+        return None
 
     # each line represents a branch push request, handle the first one only
     return pre_push_input.splitlines()[0]
@@ -332,6 +330,15 @@ def calculate_pre_push_commit_range(push_update_details: str) -> Optional[str]:
     """
     local_ref, local_object_name, remote_ref, remote_object_name = push_update_details.split()
 
+    # Tag pushes don't contain file diffs that need scanning
+    if local_ref.startswith('refs/tags/') or remote_ref.startswith('refs/tags/'):
+        logger.info('Skipping scan for tag push: %s -> %s', local_ref, remote_ref)
+        return None
+
+    # If deleting a ref (local_object_name is all zeros), no need to scan
+    if local_object_name == consts.EMPTY_COMMIT_SHA:
+        return None
+
     if remote_object_name == consts.EMPTY_COMMIT_SHA:
         try:
             repo = git_proxy.get_repo(os.getcwd())
@@ -356,10 +363,6 @@ def calculate_pre_push_commit_range(push_update_details: str) -> Optional[str]:
             logger.debug('Failed to get repo for pre-push commit range calculation: %s', exc_info=e)
             return consts.COMMIT_RANGE_ALL_COMMITS
 
-    # If deleting a branch (local_object_name is all zeros), no need to scan
-    if local_object_name == consts.EMPTY_COMMIT_SHA:
-        return None
-
     # For updates to existing branches, scan from remote to local
     return f'{remote_object_name}..{local_object_name}'
 

tests/cli/files_collector/test_commit_range_documents.py

@@ -392,15 +392,15 @@ def test_parse_branch_deletion_input(self) -> None:
             result = parse_pre_push_input()
             assert result == pre_push_input
 
-    def test_parse_empty_input_raises_error(self) -> None:
-        """Test that empty input raises ValueError."""
-        with patch('sys.stdin', StringIO('')), pytest.raises(ValueError, match='Pre push input was not found'):
-            parse_pre_push_input()
+    def test_parse_empty_input_returns_none(self) -> None:
+        """Test that empty input returns None instead of raising."""
+        with patch('sys.stdin', StringIO('')):
+            assert parse_pre_push_input() is None
 
-    def test_parse_whitespace_only_input_raises_error(self) -> None:
-        """Test that whitespace-only input raises ValueError."""
-        with patch('sys.stdin', StringIO('   \n\t  ')), pytest.raises(ValueError, match='Pre push input was not found'):
-            parse_pre_push_input()
+    def test_parse_whitespace_only_input_returns_none(self) -> None:
+        """Test that whitespace-only input returns None instead of raising."""
+        with patch('sys.stdin', StringIO('   \n\t  ')):
+            assert parse_pre_push_input() is None
 
 
 class TestGetDefaultBranchesForMergeBase:
@@ -758,26 +758,23 @@ def test_calculate_range_parsing_push_details(self) -> None:
         result = calculate_pre_push_commit_range(push_details)
         assert result == '789xyz456abc..abc123def456'
 
-    def test_calculate_range_with_tags(self) -> None:
-        """Test calculating commit range when pushing tags."""
+    def test_calculate_range_with_new_tag_push_returns_none(self) -> None:
+        """Test that pushing a new tag returns None (no scanning needed)."""
         push_details = f'refs/tags/v1.0.0 1234567890abcdef refs/tags/v1.0.0 {consts.EMPTY_COMMIT_SHA}'
+        result = calculate_pre_push_commit_range(push_details)
+        assert result is None
 
-        with temporary_git_repository() as (temp_dir, repo):
-            # Create a commit
-            test_file = os.path.join(temp_dir, 'test.py')
-            with open(test_file, 'w') as f:
-                f.write("print('test')")
-
-            repo.index.add(['test.py'])
-            commit = repo.index.commit('Test commit')
-
-            # Create tag
-            repo.create_tag('v1.0.0', commit)
+    def test_calculate_range_with_tag_deletion_returns_none(self) -> None:
+        """Test that deleting a tag returns None (no scanning needed)."""
+        push_details = f'refs/tags/v1.0.0 {consts.EMPTY_COMMIT_SHA} refs/tags/v1.0.0 1234567890abcdef'
+        result = calculate_pre_push_commit_range(push_details)
+        assert result is None
 
-            with patch('os.getcwd', return_value=temp_dir):
-                result = calculate_pre_push_commit_range(push_details)
-                # For new tags, should try to find a merge base or fall back to --all
-                assert result in [f'{commit.hexsha}..{commit.hexsha}', '--all']
+    def test_calculate_range_with_tag_update_returns_none(self) -> None:
+        """Test that updating a tag returns None (no scanning needed)."""
+        push_details = 'refs/tags/v1.0.0 1234567890abcdef refs/tags/v1.0.0 0987654321fedcba'
+        result = calculate_pre_push_commit_range(push_details)
+        assert result is None
 
 
 class TestPrePushHookIntegration:
@@ -805,12 +802,15 @@ def test_simulate_pre_push_hook_input_format(self) -> None:
                 # Test that we can calculate the commit range for each case
                 commit_range = calculate_pre_push_commit_range(parsed)
 
-                if consts.EMPTY_COMMIT_SHA in push_input:
-                    if push_input.startswith('refs/heads/') and push_input.split()[1] == consts.EMPTY_COMMIT_SHA:
+                if push_input.startswith('refs/tags/'):
+                    # Tag pushes - should return None (no scanning needed)
+                    assert commit_range is None
+                elif consts.EMPTY_COMMIT_SHA in push_input:
+                    if push_input.split()[1] == consts.EMPTY_COMMIT_SHA:
                         # Branch deletion - should return None
                         assert commit_range is None
                     else:
-                        # New branch/tag - should return a range or --all
+                        # New branch - should return a range or --all
                         assert commit_range is not None
                 else:
                     # Regular update - should return proper range