Skip to content

Commit a8523f8

Browse files
committed
Read CI default-branch variables in the shared lookup
GitLab's CI_DEFAULT_BRANCH and Buildkite's BUILDKITE_PIPELINE_DEFAULT_BRANCH only fed the final branch comparison. The commit-on-default check still went to the remote, so single-branch checkouts on those CIs paid for a git ls-remote call even when CI already knew the answer. Both variables now come first in get_default_branch_name. Also simplifies the remote lookup. start_new_session works on every platform because Windows ignores it and taskkill walks the tree by PID. The stalled-remote fixture is now a listener that never accepts, and the test clears proxy variables so it really waits for the timeout.
1 parent f74c6cf commit a8523f8

3 files changed

Lines changed: 45 additions & 48 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
`actions/checkout`. The CLI reads the default branch from the GitHub event
1010
payload or asks the remote, before falling back to `main`/`master`. Scans on
1111
those branches become the branch head again.
12+
- GitLab's `CI_DEFAULT_BRANCH` and Buildkite's `BUILDKITE_PIPELINE_DEFAULT_BRANCH`
13+
now apply to every default-branch check, including whether the commit is on
14+
the default branch.
1215

1316
## 2.10.5
1417

‎socketsecurity/core/git_interface.py‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ def __init__(self, path: str, base_commit_sha: str | None = None):
3333
self.path = path
3434
self.base_commit_sha = base_commit_sha
3535
self._fetched_ref_commits = {}
36+
self._default_branch_name: str | None = None
3637
self.ensure_safe_directory(path)
3738
self.repo = Repo(path)
3839
assert self.repo
@@ -422,12 +423,10 @@ def _is_commit_and_branch_default(self) -> bool:
422423
github_ref = os.getenv('GITHUB_REF') # e.g., 'refs/heads/main' or 'refs/pull/123/merge'
423424
gitlab_branch = os.getenv('CI_COMMIT_BRANCH')
424425
gitlab_mr_branch = os.getenv('CI_MERGE_REQUEST_SOURCE_BRANCH_NAME')
425-
gitlab_default_branch = os.getenv('CI_DEFAULT_BRANCH', '')
426426
bitbucket_branch = os.getenv('BITBUCKET_BRANCH')
427427
buildkite_branch = os.getenv('BUILDKITE_BRANCH')
428428
buildkite_pr = os.getenv('BUILDKITE_PULL_REQUEST')
429-
buildkite_default_branch = os.getenv('BUILDKITE_PIPELINE_DEFAULT_BRANCH')
430-
429+
431430
# Handle Buildkite before GitHub because some Buildkite pipelines
432431
# intentionally provide GitHub-compatible environment variables.
433432
if buildkite_branch:
@@ -437,7 +436,7 @@ def _is_commit_and_branch_default(self) -> bool:
437436
"not default branch"
438437
)
439438
return False
440-
default_branch_name = buildkite_default_branch or self.get_default_branch_name()
439+
default_branch_name = self.get_default_branch_name()
441440
is_default = buildkite_branch == default_branch_name
442441
log.debug(
443442
f"Buildkite branch: {buildkite_branch}, Default: {default_branch_name}, "
@@ -470,7 +469,7 @@ def _is_commit_and_branch_default(self) -> bool:
470469
elif gitlab_branch or gitlab_mr_branch:
471470
# If this is a merge request, use the source branch
472471
current_branch = gitlab_mr_branch or gitlab_branch
473-
default_branch_name = gitlab_default_branch or self.get_default_branch_name()
472+
default_branch_name = self.get_default_branch_name()
474473

475474
# For merge requests, they're typically not considered "default branch"
476475
if gitlab_mr_branch:
@@ -644,13 +643,17 @@ def get_default_branch_name(self) -> str:
644643
Returns:
645644
Default branch name (e.g., 'main', 'master')
646645
"""
647-
cached = getattr(self, "_default_branch_name", None)
648-
if cached:
649-
return cached
650-
self._default_branch_name = self._detect_default_branch_name()
646+
if self._default_branch_name is None:
647+
self._default_branch_name = self._detect_default_branch_name()
651648
return self._default_branch_name
652649

653650
def _detect_default_branch_name(self) -> str:
651+
for variable in ('CI_DEFAULT_BRANCH', 'BUILDKITE_PIPELINE_DEFAULT_BRANCH'):
652+
default_branch = os.getenv(variable)
653+
if default_branch:
654+
log.debug(f"Default branch detected from {variable}: {default_branch}")
655+
return default_branch
656+
654657
try:
655658
default_branch = self.repo.remotes.origin.refs.HEAD.reference.remote_head
656659
log.debug(f"Default branch detected from origin/HEAD: {default_branch}")
@@ -666,13 +669,14 @@ def _detect_default_branch_name(self) -> str:
666669
if default_branch:
667670
return default_branch
668671

672+
try:
673+
remote_refs = {str(ref) for ref in self.repo.remotes.origin.refs}
674+
except Exception:
675+
remote_refs = set()
669676
for branch_name in ['main', 'master']:
670-
try:
671-
if f'origin/{branch_name}' in [str(ref) for ref in self.repo.remotes.origin.refs]:
672-
log.debug(f"Using fallback default branch: {branch_name}")
673-
return branch_name
674-
except Exception:
675-
continue
677+
if f'origin/{branch_name}' in remote_refs:
678+
log.debug(f"Using fallback default branch: {branch_name}")
679+
return branch_name
676680

677681
log.debug("Using final fallback default branch: main")
678682
return 'main'
@@ -693,12 +697,8 @@ def _default_branch_from_github_event() -> str | None:
693697
return default_branch or None
694698

695699
def _default_branch_from_remote(self) -> str | None:
696-
# A new process group lets a timeout also kill the remote helpers, which
697-
# otherwise hold stdout open and keep communicate() blocked.
698-
if IS_WINDOWS:
699-
group_kwargs = {"creationflags": subprocess.CREATE_NEW_PROCESS_GROUP}
700-
else:
701-
group_kwargs = {"start_new_session": True}
700+
# A new session lets a timeout also kill the remote helpers, which otherwise
701+
# hold stdout open and keep communicate() blocked. Windows ignores it.
702702
try:
703703
process = subprocess.Popen(
704704
["git", "ls-remote", "--symref", "origin", "HEAD"],
@@ -708,7 +708,7 @@ def _default_branch_from_remote(self) -> str | None:
708708
stdout=subprocess.PIPE,
709709
stderr=subprocess.DEVNULL,
710710
text=True,
711-
**group_kwargs,
711+
start_new_session=True,
712712
)
713713
except Exception as error:
714714
log.debug(f"Could not query origin for its default branch: {error}")

‎tests/unit/test_git_interface.py‎

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import logging
22
import socket
33
import subprocess
4-
import threading
54
import time
65
from types import SimpleNamespace
76
from unittest.mock import MagicMock
@@ -434,6 +433,20 @@ def test_github_event_payload_supplies_default_branch(
434433
remote_lookup.assert_not_called()
435434

436435

436+
def test_ci_default_branch_variable_skips_remote_lookup(
437+
single_branch_checkout, monkeypatch, mocker
438+
):
439+
monkeypatch.setenv("CI_COMMIT_BRANCH", "dev")
440+
monkeypatch.setenv("CI_DEFAULT_BRANCH", "dev")
441+
mocker.patch.object(Git, "ensure_safe_directory")
442+
remote_lookup = mocker.patch.object(Git, "_default_branch_from_remote")
443+
444+
repository = Git(str(single_branch_checkout))
445+
446+
assert repository.is_default_branch is True
447+
remote_lookup.assert_not_called()
448+
449+
437450
def test_origin_head_wins_without_remote_lookup(single_branch_checkout, monkeypatch, mocker):
438451
_git(single_branch_checkout, "symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/dev")
439452
monkeypatch.setenv("GITHUB_REF", "refs/heads/dev")
@@ -459,28 +472,11 @@ def test_feature_branch_in_single_branch_checkout_is_not_default(
459472

460473
@pytest.fixture
461474
def stalled_http_remote():
462-
"""An HTTP server that accepts connections and never responds."""
475+
"""An HTTP remote that completes the TCP handshake and never responds."""
463476
server = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
464477
server.bind(("127.0.0.1", 0))
465478
server.listen()
466-
connections = []
467-
stop = threading.Event()
468-
469-
def accept():
470-
server.settimeout(0.2)
471-
while not stop.is_set():
472-
try:
473-
connections.append(server.accept()[0])
474-
except OSError:
475-
continue
476-
477-
thread = threading.Thread(target=accept, daemon=True)
478-
thread.start()
479479
yield f"http://127.0.0.1:{server.getsockname()[1]}/repo.git"
480-
stop.set()
481-
thread.join()
482-
for connection in connections:
483-
connection.close()
484480
server.close()
485481

486482

@@ -489,28 +485,26 @@ def test_stalled_remote_lookup_stops_at_timeout(
489485
):
490486
_git(single_branch_checkout, "remote", "set-url", "origin", stalled_http_remote)
491487
monkeypatch.setattr(git_interface, "REMOTE_HEAD_TIMEOUT_SECONDS", 1)
488+
for variable in ("HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "http_proxy", "https_proxy", "all_proxy"):
489+
monkeypatch.delenv(variable, raising=False)
492490
repository = Git.__new__(Git)
493491
repository.repo = Repo(str(single_branch_checkout))
494492

495493
started = time.monotonic()
496494
result = repository._default_branch_from_remote()
497495

498496
assert result is None
499-
assert time.monotonic() - started < 10
497+
assert 1 <= time.monotonic() - started < 10
500498

501499

502-
def test_windows_remote_lookup_uses_new_process_group(monkeypatch, mocker):
503-
monkeypatch.setattr(git_interface, "IS_WINDOWS", True)
504-
monkeypatch.setattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0x200, raising=False)
500+
def test_remote_lookup_parses_branch_with_slash(mocker):
505501
process = MagicMock(returncode=0)
506502
process.communicate.return_value = ("ref: refs/heads/release/stable\tHEAD\n", "")
507-
popen = mocker.patch.object(git_interface.subprocess, "Popen", return_value=process)
503+
mocker.patch.object(git_interface.subprocess, "Popen", return_value=process)
508504
repository = Git.__new__(Git)
509505
repository.repo = MagicMock(working_dir="/repo")
510506

511507
assert repository._default_branch_from_remote() == "release/stable"
512-
assert popen.call_args.kwargs["creationflags"] == 0x200
513-
assert "start_new_session" not in popen.call_args.kwargs
514508

515509

516510
def test_windows_timeout_kills_the_whole_process_tree(monkeypatch, mocker):

0 commit comments

Comments
 (0)