From 36840a68687d065027c05ac5154a0f6aa10f8595 Mon Sep 17 00:00:00 2001 From: gotbadger Date: Tue, 7 Apr 2026 16:05:13 +0100 Subject: [PATCH] CM-62406: handle more pre-push types gracefully --- .../apps/scan/pre_push/pre_push_command.py | 4 ++ .../files_collector/commit_range_documents.py | 21 ++++--- .../test_commit_range_documents.py | 56 +++++++++---------- 3 files changed, 44 insertions(+), 37 deletions(-) diff --git a/cycode/cli/apps/scan/pre_push/pre_push_command.py b/cycode/cli/apps/scan/pre_push/pre_push_command.py index d3339ea9..729f3571 100644 --- a/cycode/cli/apps/scan/pre_push/pre_push_command.py +++ b/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( diff --git a/cycode/cli/files_collector/commit_range_documents.py b/cycode/cli/files_collector/commit_range_documents.py index a4a1a784..daa5c432 100644 --- a/cycode/cli/files_collector/commit_range_documents.py +++ b/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}' diff --git a/tests/cli/files_collector/test_commit_range_documents.py b/tests/cli/files_collector/test_commit_range_documents.py index 501c1811..0b96a0e2 100644 --- a/tests/cli/files_collector/test_commit_range_documents.py +++ b/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