Skip to content

Commit 25ea9db

Browse files
committed
CM-51935 - Fix pre-receive hook for bare repositories
1 parent bc03b57 commit 25ea9db

File tree

4 files changed

+245
-29
lines changed

4 files changed

+245
-29
lines changed

cycode/cli/files_collector/commit_range_documents.py

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ def collect_commit_range_diff_documents(
107107
def calculate_pre_receive_commit_range(branch_update_details: str) -> Optional[str]:
108108
end_commit = _get_end_commit_from_branch_update_details(branch_update_details)
109109

110-
# branch is deleted, no need to perform scan
110+
# the branch is deleted, no need to perform a scan
111111
if end_commit == consts.EMPTY_COMMIT_SHA:
112112
return None
113113

@@ -127,10 +127,21 @@ def _get_end_commit_from_branch_update_details(update_details: str) -> str:
127127

128128

129129
def _get_oldest_unupdated_commit_for_branch(commit: str) -> Optional[str]:
130-
# get a list of commits by chronological order that are not in the remote repository yet
130+
# get a list of commits by chronological order that is not in the remote repository yet
131131
# more info about rev-list command: https://git-scm.com/docs/git-rev-list
132132
repo = git_proxy.get_repo(os.getcwd())
133-
not_updated_commits = repo.git.rev_list(commit, '--topo-order', '--reverse', '--not', '--all')
133+
134+
try:
135+
not_updated_commits = repo.git.rev_list(commit, '--topo-order', '--reverse', '--not', '--all')
136+
except git_proxy.get_git_command_error() as e:
137+
# Handle case where the commit doesn't exist in the repository yet (e.g., first push to bare repo)
138+
if 'bad object' in str(e) or 'unknown revision' in str(e):
139+
logger.debug(
140+
'Commit does not exist in repository yet (likely first push), %s', {'commit': commit, 'error': str(e)}
141+
)
142+
return None
143+
# Re-raise other git errors
144+
raise
134145

135146
commits = not_updated_commits.splitlines()
136147
if not commits:
@@ -311,9 +322,25 @@ def parse_commit_range_sast(commit_range: str, path: str) -> tuple[Optional[str]
311322
to_spec = consts.GIT_HEAD_COMMIT_REV
312323

313324
try:
314-
# Use rev_parse to resolve each specifier to its full commit SHA
315-
from_commit_rev = repo.rev_parse(from_spec).hexsha
316-
to_commit_rev = repo.rev_parse(to_spec).hexsha
325+
# Use safe head reference resolution for HEAD references in bare repositories
326+
if from_spec == consts.GIT_HEAD_COMMIT_REV:
327+
safe_from_spec = get_safe_head_reference_for_diff(repo)
328+
if safe_from_spec == consts.GIT_EMPTY_TREE_OBJECT:
329+
logger.debug("Cannot resolve HEAD reference in bare repository for commit range '%s'", commit_range)
330+
return None, None
331+
from_commit_rev = repo.rev_parse(safe_from_spec).hexsha
332+
else:
333+
from_commit_rev = repo.rev_parse(from_spec).hexsha
334+
335+
if to_spec == consts.GIT_HEAD_COMMIT_REV:
336+
safe_to_spec = get_safe_head_reference_for_diff(repo)
337+
if safe_to_spec == consts.GIT_EMPTY_TREE_OBJECT:
338+
logger.debug("Cannot resolve HEAD reference in bare repository for commit range '%s'", commit_range)
339+
return None, None
340+
to_commit_rev = repo.rev_parse(safe_to_spec).hexsha
341+
else:
342+
to_commit_rev = repo.rev_parse(to_spec).hexsha
343+
317344
return from_commit_rev, to_commit_rev
318345
except git_proxy.get_git_command_error() as e:
319346
logger.warning("Failed to parse commit range '%s'", commit_range, exc_info=e)
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import os
2+
import tempfile
3+
from collections.abc import Generator
4+
from contextlib import contextmanager
5+
6+
from git import Repo
7+
8+
9+
@contextmanager
10+
def git_repository(path: str) -> Generator[Repo, None, None]:
11+
"""Context manager for Git repositories that ensures proper cleanup on Windows."""
12+
repo = Repo.init(path)
13+
try:
14+
yield repo
15+
finally:
16+
# Properly close the repository to release file handles
17+
repo.close()
18+
19+
20+
@contextmanager
21+
def temporary_git_repository() -> Generator[tuple[str, Repo], None, None]:
22+
"""Combined context manager for temporary directory with Git repository."""
23+
with tempfile.TemporaryDirectory() as temp_dir, git_repository(temp_dir) as repo:
24+
yield temp_dir, repo
25+
26+
27+
def create_multiple_commits(repo: Repo, temp_dir: str, num_commits: int = 3) -> list:
28+
"""Helper function to create multiple commits in the repository."""
29+
commits = []
30+
for i in range(num_commits):
31+
test_file = os.path.join(temp_dir, f'file{i}.py')
32+
with open(test_file, 'w') as f:
33+
f.write(f"print('file {i}')")
34+
35+
repo.index.add([f'file{i}.py'])
36+
commit = repo.index.commit(f'Commit {i + 1}')
37+
commits.append(commit)
38+
return commits

tests/cli/files_collector/test_commit_range_documents.py

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,8 @@
11
import os
2-
import tempfile
3-
from collections.abc import Generator
4-
from contextlib import contextmanager
5-
6-
from git import Repo
72

83
from cycode.cli import consts
94
from cycode.cli.files_collector.commit_range_documents import get_safe_head_reference_for_diff
10-
11-
12-
@contextmanager
13-
def git_repository(path: str) -> Generator[Repo, None, None]:
14-
"""Context manager for Git repositories that ensures proper cleanup on Windows."""
15-
repo = Repo.init(path)
16-
try:
17-
yield repo
18-
finally:
19-
# Properly close the repository to release file handles
20-
repo.close()
21-
22-
23-
@contextmanager
24-
def temporary_git_repository() -> Generator[tuple[str, Repo], None, None]:
25-
"""Combined context manager for temporary directory with Git repository."""
26-
with tempfile.TemporaryDirectory() as temp_dir, git_repository(temp_dir) as repo:
27-
yield temp_dir, repo
5+
from tests.cli.files_collector.common import temporary_git_repository
286

297

308
class TestGetSafeHeadReferenceForDiff:
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
import os
2+
3+
import pytest
4+
5+
from cycode.cli import consts
6+
from cycode.cli.files_collector.commit_range_documents import (
7+
_get_oldest_unupdated_commit_for_branch,
8+
calculate_pre_receive_commit_range,
9+
parse_commit_range_sast,
10+
)
11+
from tests.cli.files_collector.common import create_multiple_commits, temporary_git_repository
12+
13+
14+
class TestParseCommitRangeSast:
15+
"""Test the SAST commit range parsing with bare repository support."""
16+
17+
@pytest.mark.parametrize(
18+
('commit_range', 'description'),
19+
[
20+
('HEAD', 'single HEAD reference'),
21+
('..HEAD', 'range ending with HEAD'),
22+
('HEAD..', 'range starting with HEAD'),
23+
('HEAD..HEAD', 'range with both HEAD references'),
24+
],
25+
)
26+
def test_returns_none_for_head_references_in_bare_repository(self, commit_range: str, description: str) -> None:
27+
"""Test that HEAD references return None in bare repositories."""
28+
with temporary_git_repository() as (temp_dir, repo):
29+
from_commit, to_commit = parse_commit_range_sast(commit_range, temp_dir)
30+
31+
# Should return None for bare repositories since HEAD doesn't exist
32+
assert from_commit is None
33+
assert to_commit is None
34+
35+
def test_works_correctly_with_head_references_when_commits_exist(self) -> None:
36+
"""Test that HEAD references work correctly when the repository has commits."""
37+
with temporary_git_repository() as (temp_dir, repo):
38+
test_file = os.path.join(temp_dir, 'test.py')
39+
with open(test_file, 'w') as f:
40+
f.write("print('initial')")
41+
42+
repo.index.add(['test.py'])
43+
initial_commit = repo.index.commit('Initial commit')
44+
45+
commit_range = initial_commit.hexsha # This gets interpreted as 'commit..HEAD'
46+
from_commit, to_commit = parse_commit_range_sast(commit_range, temp_dir)
47+
48+
# Should successfully resolve both commits
49+
assert from_commit is not None
50+
assert to_commit is not None
51+
assert to_commit == initial_commit.hexsha # HEAD should resolve to the latest commit
52+
53+
def test_handles_explicit_commit_ranges_correctly(self) -> None:
54+
"""Test that explicit commit ranges (no HEAD) work correctly."""
55+
with temporary_git_repository() as (temp_dir, repo):
56+
commits = create_multiple_commits(repo, temp_dir)
57+
58+
commit_range = f'{commits[0].hexsha}..{commits[2].hexsha}'
59+
from_commit, to_commit = parse_commit_range_sast(commit_range, temp_dir)
60+
61+
assert from_commit == commits[0].hexsha
62+
assert to_commit == commits[2].hexsha
63+
64+
def test_handles_three_dot_ranges_correctly(self) -> None:
65+
"""Test that three-dot commit ranges work correctly."""
66+
with temporary_git_repository() as (temp_dir, repo):
67+
test_file = os.path.join(temp_dir, 'test.py')
68+
with open(test_file, 'w') as f:
69+
f.write("print('first')")
70+
71+
repo.index.add(['test.py'])
72+
first_commit = repo.index.commit('First commit')
73+
74+
with open(test_file, 'w') as f:
75+
f.write("print('second')")
76+
77+
repo.index.add(['test.py'])
78+
second_commit = repo.index.commit('Second commit')
79+
80+
# Test three-dot range
81+
commit_range = f'{first_commit.hexsha}...{second_commit.hexsha}'
82+
83+
from_commit, to_commit = parse_commit_range_sast(commit_range, temp_dir)
84+
85+
assert from_commit == first_commit.hexsha
86+
assert to_commit == second_commit.hexsha
87+
88+
89+
class TestGetOldestUnupdatedCommitForBranch:
90+
"""Test the oldest unupdated commit function with bare repository support."""
91+
92+
def test_returns_none_for_nonexistent_commit_in_bare_repository(self) -> None:
93+
"""Test that function returns None for non-existent commits in bare repositories."""
94+
with temporary_git_repository() as (temp_dir, repo):
95+
# Use a fake commit SHA that doesn't exist
96+
fake_commit_sha = '9cf90954ef26e7c58284f8ebf7dcd0fcf711152a'
97+
98+
original_cwd = os.getcwd()
99+
os.chdir(temp_dir)
100+
101+
try:
102+
# Should handle missing commit gracefully
103+
result = _get_oldest_unupdated_commit_for_branch(fake_commit_sha)
104+
assert result is None
105+
finally:
106+
os.chdir(original_cwd)
107+
108+
def test_works_correctly_with_existing_commits(self) -> None:
109+
"""Test that the function works correctly when commits exist."""
110+
with temporary_git_repository() as (temp_dir, repo):
111+
commits = create_multiple_commits(repo, temp_dir)
112+
113+
original_cwd = os.getcwd()
114+
os.chdir(temp_dir)
115+
116+
try:
117+
# Test with an existing commit
118+
result = _get_oldest_unupdated_commit_for_branch(commits[-1].hexsha)
119+
# Result depends on repository state, but should not crash
120+
assert isinstance(result, (str, type(None)))
121+
finally:
122+
os.chdir(original_cwd)
123+
124+
125+
class TestCalculatePreReceiveCommitRange:
126+
"""Test the pre-receive commit range calculation with bare repository support."""
127+
128+
def test_handles_first_push_to_bare_repository(self) -> None:
129+
"""Test the first push scenario (old commit is all zeros)."""
130+
with temporary_git_repository() as (temp_dir, repo):
131+
# Simulate the first push: old_commit=zeros, new_commit=actual_sha
132+
zero_commit = consts.EMPTY_COMMIT_SHA
133+
new_commit_sha = '9cf90954ef26e7c58284f8ebf7dcd0fcf711152a'
134+
branch_update_details = f'{zero_commit} {new_commit_sha} refs/heads/main'
135+
136+
original_cwd = os.getcwd()
137+
os.chdir(temp_dir)
138+
139+
try:
140+
# Should handle the first push gracefully
141+
commit_range = calculate_pre_receive_commit_range(branch_update_details)
142+
# For the first push to bare repo, this typically returns None
143+
assert commit_range is None or isinstance(commit_range, str)
144+
finally:
145+
os.chdir(original_cwd)
146+
147+
def test_handles_branch_deletion(self) -> None:
148+
"""Test branch deletion scenario (new commit is all zeros)."""
149+
old_commit_sha = '9cf90954ef26e7c58284f8ebf7dcd0fcf711152a'
150+
zero_commit = consts.EMPTY_COMMIT_SHA
151+
branch_update_details = f'{old_commit_sha} {zero_commit} refs/heads/feature'
152+
153+
# Should return None for branch deletion
154+
commit_range = calculate_pre_receive_commit_range(branch_update_details)
155+
assert commit_range is None
156+
157+
def test_normal_push_scenario(self) -> None:
158+
"""Test a normal push scenario with existing commits."""
159+
with temporary_git_repository() as (temp_dir, repo):
160+
commits = create_multiple_commits(repo, temp_dir, num_commits=2)
161+
162+
# Simulate normal push
163+
branch_update_details = f'{commits[0].hexsha} {commits[1].hexsha} refs/heads/main'
164+
165+
original_cwd = os.getcwd()
166+
os.chdir(temp_dir)
167+
168+
try:
169+
commit_range = calculate_pre_receive_commit_range(branch_update_details)
170+
# Should work without errors
171+
assert commit_range is None or isinstance(commit_range, str)
172+
finally:
173+
os.chdir(original_cwd)

0 commit comments

Comments
 (0)