Skip to content

Commit f74c6cf

Browse files
committed
Bound the remote default-branch lookup on every platform
GitPython's kill_after_timeout is rejected on Windows and relies on ps --ppid, which macOS lacks. It also leaves git-remote-https holding the output pipe after the parent dies, so a stalled remote blocked startup past the timeout. Run git ls-remote in its own process group and kill the whole group on timeout, with taskkill /T on Windows.
1 parent 8170ca1 commit f74c6cf

2 files changed

Lines changed: 126 additions & 6 deletions

File tree

‎socketsecurity/core/git_interface.py‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
import json
22
import os
33
import re
4+
import signal
5+
import subprocess
46
import time
57
import urllib.parse
68

79
from git import Repo
810

911
from socketsecurity.core import log
1012

13+
REMOTE_HEAD_TIMEOUT_SECONDS = 30
14+
IS_WINDOWS = os.name == "nt"
15+
1116

1217
class Git:
1318
repo: Repo
@@ -688,23 +693,66 @@ def _default_branch_from_github_event() -> str | None:
688693
return default_branch or None
689694

690695
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}
691702
try:
692-
output = self.repo.git.ls_remote(
693-
"--symref",
694-
"origin",
695-
"HEAD",
696-
env={"GIT_TERMINAL_PROMPT": "0"},
697-
kill_after_timeout=30,
703+
process = subprocess.Popen(
704+
["git", "ls-remote", "--symref", "origin", "HEAD"],
705+
cwd=self.repo.working_dir,
706+
env={**os.environ, "GIT_TERMINAL_PROMPT": "0"},
707+
stdin=subprocess.DEVNULL,
708+
stdout=subprocess.PIPE,
709+
stderr=subprocess.DEVNULL,
710+
text=True,
711+
**group_kwargs,
698712
)
699713
except Exception as error:
700714
log.debug(f"Could not query origin for its default branch: {error}")
701715
return None
716+
try:
717+
output, _ = process.communicate(timeout=REMOTE_HEAD_TIMEOUT_SECONDS)
718+
except subprocess.TimeoutExpired:
719+
self._kill_process_tree(process)
720+
log.debug(
721+
f"Querying origin for its default branch timed out after "
722+
f"{REMOTE_HEAD_TIMEOUT_SECONDS}s"
723+
)
724+
return None
725+
if process.returncode != 0:
726+
log.debug(f"Querying origin for its default branch exited with {process.returncode}")
727+
return None
702728
for line in output.splitlines():
703729
match = re.match(r"ref: refs/heads/(\S+)\tHEAD$", line)
704730
if match:
705731
log.debug(f"Default branch detected from origin: {match.group(1)}")
706732
return match.group(1)
707733
return None
734+
735+
@staticmethod
736+
def _kill_process_tree(process: subprocess.Popen) -> None:
737+
try:
738+
if IS_WINDOWS:
739+
subprocess.run(
740+
["taskkill", "/F", "/T", "/PID", str(process.pid)],
741+
stdout=subprocess.DEVNULL,
742+
stderr=subprocess.DEVNULL,
743+
timeout=10,
744+
)
745+
else:
746+
os.killpg(process.pid, signal.SIGKILL)
747+
except Exception as error:
748+
log.debug(f"Failed to stop git ls-remote process tree: {error}")
749+
process.kill()
750+
if process.stdout:
751+
process.stdout.close()
752+
try:
753+
process.wait(timeout=5)
754+
except subprocess.TimeoutExpired:
755+
log.debug("git ls-remote did not exit after being killed")
708756

709757
def is_commit_on_default_branch(self) -> bool:
710758
"""

‎tests/unit/test_git_interface.py‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
import logging
2+
import socket
23
import subprocess
4+
import threading
5+
import time
36
from types import SimpleNamespace
47
from unittest.mock import MagicMock
58

69
import pytest
10+
from git import Repo
711

12+
from socketsecurity.core import git_interface
813
from socketsecurity.core.git_interface import Git
914

1015
CI_ENVIRONMENT_VARIABLES = (
@@ -450,3 +455,70 @@ def test_feature_branch_in_single_branch_checkout_is_not_default(
450455
repository = Git(str(single_branch_checkout))
451456

452457
assert repository.is_default_branch is False
458+
459+
460+
@pytest.fixture
461+
def stalled_http_remote():
462+
"""An HTTP server that accepts connections and never responds."""
463+
server = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
464+
server.bind(("127.0.0.1", 0))
465+
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()
479+
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()
484+
server.close()
485+
486+
487+
def test_stalled_remote_lookup_stops_at_timeout(
488+
single_branch_checkout, stalled_http_remote, monkeypatch
489+
):
490+
_git(single_branch_checkout, "remote", "set-url", "origin", stalled_http_remote)
491+
monkeypatch.setattr(git_interface, "REMOTE_HEAD_TIMEOUT_SECONDS", 1)
492+
repository = Git.__new__(Git)
493+
repository.repo = Repo(str(single_branch_checkout))
494+
495+
started = time.monotonic()
496+
result = repository._default_branch_from_remote()
497+
498+
assert result is None
499+
assert time.monotonic() - started < 10
500+
501+
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)
505+
process = MagicMock(returncode=0)
506+
process.communicate.return_value = ("ref: refs/heads/release/stable\tHEAD\n", "")
507+
popen = mocker.patch.object(git_interface.subprocess, "Popen", return_value=process)
508+
repository = Git.__new__(Git)
509+
repository.repo = MagicMock(working_dir="/repo")
510+
511+
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
514+
515+
516+
def test_windows_timeout_kills_the_whole_process_tree(monkeypatch, mocker):
517+
monkeypatch.setattr(git_interface, "IS_WINDOWS", True)
518+
run = mocker.patch.object(git_interface.subprocess, "run")
519+
process = MagicMock(pid=4321)
520+
521+
Git._kill_process_tree(process)
522+
523+
assert run.call_args.args[0] == ["taskkill", "/F", "/T", "/PID", "4321"]
524+
process.wait.assert_called_once()

0 commit comments

Comments
 (0)