diff --git a/CHANGELOG.md b/CHANGELOG.md index a830385..efbfb10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # Changelog +## 2.10.6 + +### Fixed: default-branch detection in single-branch CI checkouts + +- Repositories whose default branch isn't `main` or `master` are detected as the + default branch again when the checkout has no `origin/HEAD`, as with + `actions/checkout`. The CLI reads the default branch from the GitHub event + payload or asks the remote, before falling back to `main`/`master`. Scans on + those branches become the branch head again. +- GitLab's `CI_DEFAULT_BRANCH` and Buildkite's `BUILDKITE_PIPELINE_DEFAULT_BRANCH` + now apply to every default-branch check, including whether the commit is on + the default branch. + ## 2.10.5 ### Changed: bump pinned @coana-tech/cli to 15.11.4 diff --git a/pyproject.toml b/pyproject.toml index 1d77630..cc8aa90 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" [project] name = "socketsecurity" -version = "2.10.5" +version = "2.10.6" requires-python = ">= 3.11" license = {"file" = "LICENSE"} dependencies = [ diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index c1d9595..015a271 100644 --- a/socketsecurity/__init__.py +++ b/socketsecurity/__init__.py @@ -1,3 +1,3 @@ __author__ = 'socket.dev' -__version__ = '2.10.5' +__version__ = '2.10.6' USER_AGENT = f'SocketPythonCLI/{__version__}' diff --git a/socketsecurity/core/git_interface.py b/socketsecurity/core/git_interface.py index 81001f4..563b41a 100644 --- a/socketsecurity/core/git_interface.py +++ b/socketsecurity/core/git_interface.py @@ -1,5 +1,8 @@ +import json import os import re +import signal +import subprocess import time import urllib.parse @@ -7,6 +10,9 @@ from socketsecurity.core import log +REMOTE_HEAD_TIMEOUT_SECONDS = 30 +IS_WINDOWS = os.name == "nt" + class Git: repo: Repo @@ -27,6 +33,7 @@ def __init__(self, path: str, base_commit_sha: str | None = None): self.path = path self.base_commit_sha = base_commit_sha self._fetched_ref_commits = {} + self._default_branch_name: str | None = None self.ensure_safe_directory(path) self.repo = Repo(path) assert self.repo @@ -407,23 +414,16 @@ def _is_commit_and_branch_default(self) -> bool: True if commit is on default branch and we're processing the default branch """ try: - # First check if the commit is reachable from the default branch - if not self.is_commit_on_default_branch(): - log.debug("Commit is not on default branch") - return False - - # Check if we're processing the default branch via CI environment variables github_ref = os.getenv('GITHUB_REF') # e.g., 'refs/heads/main' or 'refs/pull/123/merge' gitlab_branch = os.getenv('CI_COMMIT_BRANCH') gitlab_mr_branch = os.getenv('CI_MERGE_REQUEST_SOURCE_BRANCH_NAME') - gitlab_default_branch = os.getenv('CI_DEFAULT_BRANCH', '') bitbucket_branch = os.getenv('BITBUCKET_BRANCH') + bitbucket_pr = os.getenv('BITBUCKET_PR_ID') buildkite_branch = os.getenv('BUILDKITE_BRANCH') buildkite_pr = os.getenv('BUILDKITE_PULL_REQUEST') - buildkite_default_branch = os.getenv('BUILDKITE_PIPELINE_DEFAULT_BRANCH') - - # Handle Buildkite before GitHub because some Buildkite pipelines - # intentionally provide GitHub-compatible environment variables. + + # PR and non-branch builds cannot become the default branch head. + # Decide that locally before default-branch lookup contacts origin. if buildkite_branch: if self._is_buildkite_pull_request(buildkite_pr): log.debug( @@ -431,7 +431,29 @@ def _is_commit_and_branch_default(self) -> bool: "not default branch" ) return False - default_branch_name = buildkite_default_branch or self.get_default_branch_name() + elif github_ref: + if github_ref.startswith('refs/pull/'): + log.debug("Processing a pull request, not default branch") + return False + if not github_ref.startswith('refs/heads/'): + log.debug(f"Non-branch ref: {github_ref}, not default branch") + return False + elif gitlab_branch or gitlab_mr_branch: + if gitlab_mr_branch: + log.debug(f"Processing GitLab MR from branch: {gitlab_mr_branch}, not default branch") + return False + elif bitbucket_branch and bitbucket_pr: + log.debug(f"Processing Bitbucket pull request from branch: {bitbucket_branch}, not default branch") + return False + + if not self.is_commit_on_default_branch(): + log.debug("Commit is not on default branch") + return False + + # Handle Buildkite before GitHub because some Buildkite pipelines + # intentionally provide GitHub-compatible environment variables. + if buildkite_branch: + default_branch_name = self.get_default_branch_name() is_default = buildkite_branch == default_branch_name log.debug( f"Buildkite branch: {buildkite_branch}, Default: {default_branch_name}, " @@ -442,35 +464,16 @@ def _is_commit_and_branch_default(self) -> bool: # Handle GitHub Actions elif github_ref: log.debug(f"GitHub ref: {github_ref}") - - # Handle pull requests - they're not on the default branch - if github_ref.startswith('refs/pull/'): - log.debug("Processing a pull request, not default branch") - return False - - # Handle regular branch pushes - if github_ref.startswith('refs/heads/'): - branch_from_ref = github_ref.replace('refs/heads/', '') - default_branch_name = self.get_default_branch_name() - is_default = branch_from_ref == default_branch_name - log.debug(f"Branch from GITHUB_REF: {branch_from_ref}, Default: {default_branch_name}, Is default: {is_default}") - return is_default - - # Handle tags or other refs - not default branch - log.debug(f"Non-branch ref: {github_ref}, not default branch") - return False + branch_from_ref = github_ref.removeprefix('refs/heads/') + default_branch_name = self.get_default_branch_name() + is_default = branch_from_ref == default_branch_name + log.debug(f"Branch from GITHUB_REF: {branch_from_ref}, Default: {default_branch_name}, Is default: {is_default}") + return is_default # Handle GitLab CI elif gitlab_branch or gitlab_mr_branch: - # If this is a merge request, use the source branch current_branch = gitlab_mr_branch or gitlab_branch - default_branch_name = gitlab_default_branch or self.get_default_branch_name() - - # For merge requests, they're typically not considered "default branch" - if gitlab_mr_branch: - log.debug(f"Processing GitLab MR from branch: {gitlab_mr_branch}, not default branch") - return False - + default_branch_name = self.get_default_branch_name() is_default = current_branch == default_branch_name log.debug(f"GitLab branch: {current_branch}, Default: {default_branch_name}, Is default: {is_default}") return is_default @@ -638,27 +641,116 @@ def get_default_branch_name(self) -> str: Returns: Default branch name (e.g., 'main', 'master') """ + if self._default_branch_name is None: + self._default_branch_name = self._detect_default_branch_name() + return self._default_branch_name + + def _detect_default_branch_name(self) -> str: + for variable in ('CI_DEFAULT_BRANCH', 'BUILDKITE_PIPELINE_DEFAULT_BRANCH'): + default_branch = os.getenv(variable) + if default_branch: + log.debug(f"Default branch detected from {variable}: {default_branch}") + return default_branch + try: - # Try to get the default branch from remote HEAD - remote_head = self.repo.remotes.origin.refs.HEAD - # Extract branch name from refs/remotes/origin/HEAD -> refs/remotes/origin/main - default_branch = str(remote_head.reference).split('/')[-1] - log.debug(f"Default branch detected: {default_branch}") + default_branch = self.repo.remotes.origin.refs.HEAD.reference.remote_head + log.debug(f"Default branch detected from origin/HEAD: {default_branch}") return default_branch except Exception as error: - log.debug(f"Could not determine default branch from remote: {error}") - # Fallback: check common default branch names - for branch_name in ['main', 'master']: - try: - if f'origin/{branch_name}' in [str(ref) for ref in self.repo.remotes.origin.refs]: - log.debug(f"Using fallback default branch: {branch_name}") - return branch_name - except Exception: - continue - - # Last fallback: assume 'main' - log.debug("Using final fallback default branch: main") - return 'main' + log.debug(f"Could not determine default branch from origin/HEAD: {error}") + + # CI checkouts such as actions/checkout fetch a single branch and leave no origin/HEAD. + default_branch = ( + self._default_branch_from_github_event() + or self._default_branch_from_remote() + ) + if default_branch: + return default_branch + + try: + remote_refs = {str(ref) for ref in self.repo.remotes.origin.refs} + except Exception: + remote_refs = set() + for branch_name in ['main', 'master']: + if f'origin/{branch_name}' in remote_refs: + log.debug(f"Using fallback default branch: {branch_name}") + return branch_name + + log.debug("Using final fallback default branch: main") + return 'main' + + @staticmethod + def _default_branch_from_github_event() -> str | None: + event_path = os.getenv('GITHUB_EVENT_PATH') + if not event_path: + return None + try: + with open(event_path, encoding="utf-8") as event_file: + default_branch = json.load(event_file).get("repository", {}).get("default_branch") + except Exception as error: + log.debug(f"Could not read default branch from GitHub event payload: {error}") + return None + if default_branch: + log.debug(f"Default branch detected from GitHub event payload: {default_branch}") + return default_branch or None + + def _default_branch_from_remote(self) -> str | None: + # A new session lets a timeout also kill the remote helpers, which otherwise + # hold stdout open and keep communicate() blocked. Windows ignores it. + try: + process = subprocess.Popen( + ["git", "ls-remote", "--symref", "origin", "HEAD"], + cwd=self.repo.working_dir, + env={**os.environ, "GIT_TERMINAL_PROMPT": "0"}, + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + text=True, + start_new_session=True, + ) + except Exception as error: + log.debug(f"Could not query origin for its default branch: {error}") + return None + try: + output, _ = process.communicate(timeout=REMOTE_HEAD_TIMEOUT_SECONDS) + except subprocess.TimeoutExpired: + self._kill_process_tree(process) + log.debug( + f"Querying origin for its default branch timed out after " + f"{REMOTE_HEAD_TIMEOUT_SECONDS}s" + ) + return None + if process.returncode != 0: + log.debug(f"Querying origin for its default branch exited with {process.returncode}") + return None + for line in output.splitlines(): + match = re.match(r"ref: refs/heads/(\S+)\tHEAD$", line) + if match: + log.debug(f"Default branch detected from origin: {match.group(1)}") + return match.group(1) + return None + + @staticmethod + def _kill_process_tree(process: subprocess.Popen) -> None: + try: + if IS_WINDOWS: + subprocess.run( + ["taskkill", "/F", "/T", "/PID", str(process.pid)], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + timeout=10, + ) + else: + os.killpg(process.pid, signal.SIGKILL) + except Exception as error: + log.debug(f"Failed to stop git ls-remote process tree: {error}") + process.kill() + if process.stdout: + process.stdout.close() + try: + process.wait(timeout=5) + except subprocess.TimeoutExpired: + log.debug("git ls-remote did not exit after being killed") def is_commit_on_default_branch(self) -> bool: """ diff --git a/tests/unit/test_git_interface.py b/tests/unit/test_git_interface.py index ea2d941..b86a4db 100644 --- a/tests/unit/test_git_interface.py +++ b/tests/unit/test_git_interface.py @@ -1,10 +1,14 @@ import logging +import socket import subprocess +import time from types import SimpleNamespace from unittest.mock import MagicMock import pytest +from git import Repo +from socketsecurity.core import git_interface from socketsecurity.core.git_interface import Git CI_ENVIRONMENT_VARIABLES = ( @@ -17,6 +21,7 @@ "GITHUB_BASE_REF", "GITHUB_EVENT_BEFORE", "GITHUB_EVENT_NAME", + "GITHUB_EVENT_PATH", "GITHUB_HEAD_REF", "GITHUB_REF", "GITHUB_SHA", @@ -127,6 +132,11 @@ def test_pull_request_context_uses_local_refs_without_fetch( "_fetch_ref", side_effect=AssertionError("unexpected fetch"), ) + remote_lookup = mocker.patch.object( + Git, + "_default_branch_from_remote", + side_effect=AssertionError("unexpected remote default-branch lookup"), + ) mocker.patch.object(Git, "ensure_safe_directory") with caplog.at_level(logging.INFO, logger="socketdev"): @@ -136,6 +146,7 @@ def test_pull_request_context_uses_local_refs_without_fetch( assert repository.changed_files == ["package.json"] assert repository.is_default_branch is False fetch.assert_not_called() + remote_lookup.assert_not_called() assert any( f"source={expected_source}" in record.message for record in caplog.records @@ -375,3 +386,146 @@ def test_unresolvable_base_commit_warns_and_falls_back( for record in caplog.records ) fetch.assert_called_once() + + +@pytest.fixture +def single_branch_checkout(tmp_path): + """A repo whose default branch is dev, checked out the way actions/checkout does.""" + source = tmp_path / "source" + source.mkdir() + _git(source, "init", "-b", "dev") + _git(source, "config", "user.name", "Socket Test") + _git(source, "config", "user.email", "socket@example.com") + (source / "package.json").write_text("{}\n", encoding="utf-8") + _git(source, "add", "package.json") + _git(source, "commit", "-m", "base") + origin = tmp_path / "origin.git" + _git(tmp_path, "clone", "--bare", str(source), str(origin)) + + checkout = tmp_path / "checkout" + checkout.mkdir() + _git(checkout, "init") + _git(checkout, "remote", "add", "origin", str(origin)) + _git(checkout, "fetch", "--no-tags", "--depth=1", "origin", "+refs/heads/dev:refs/remotes/origin/dev") + _git(checkout, "checkout", "-B", "dev", "refs/remotes/origin/dev") + return checkout + + +def test_single_branch_checkout_detects_non_main_default_branch( + single_branch_checkout, monkeypatch, mocker +): + monkeypatch.setenv("GITHUB_REF", "refs/heads/dev") + mocker.patch.object(Git, "ensure_safe_directory") + + repository = Git(str(single_branch_checkout)) + + assert repository.get_default_branch_name() == "dev" + assert repository.is_default_branch is True + + +def test_github_event_payload_supplies_default_branch( + single_branch_checkout, tmp_path, monkeypatch, mocker +): + event_path = tmp_path / "event.json" + event_path.write_text('{"repository": {"default_branch": "dev"}}', encoding="utf-8") + monkeypatch.setenv("GITHUB_REF", "refs/heads/dev") + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_path)) + mocker.patch.object(Git, "ensure_safe_directory") + remote_lookup = mocker.patch.object(Git, "_default_branch_from_remote") + + repository = Git(str(single_branch_checkout)) + + assert repository.is_default_branch is True + remote_lookup.assert_not_called() + + +@pytest.mark.parametrize( + ("branch_variable", "default_variable"), + [ + ("CI_COMMIT_BRANCH", "CI_DEFAULT_BRANCH"), + ("BUILDKITE_BRANCH", "BUILDKITE_PIPELINE_DEFAULT_BRANCH"), + ], +) +def test_ci_default_branch_variable_skips_remote_lookup( + single_branch_checkout, monkeypatch, mocker, branch_variable, default_variable +): + monkeypatch.setenv(branch_variable, "dev") + monkeypatch.setenv(default_variable, "dev") + mocker.patch.object(Git, "ensure_safe_directory") + remote_lookup = mocker.patch.object(Git, "_default_branch_from_remote") + + repository = Git(str(single_branch_checkout)) + + assert repository.is_default_branch is True + remote_lookup.assert_not_called() + + +def test_origin_head_wins_without_remote_lookup(single_branch_checkout, monkeypatch, mocker): + _git(single_branch_checkout, "symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/dev") + monkeypatch.setenv("GITHUB_REF", "refs/heads/dev") + mocker.patch.object(Git, "ensure_safe_directory") + remote_lookup = mocker.patch.object(Git, "_default_branch_from_remote") + + repository = Git(str(single_branch_checkout)) + + assert repository.is_default_branch is True + remote_lookup.assert_not_called() + + +def test_feature_branch_in_single_branch_checkout_is_not_default( + single_branch_checkout, monkeypatch, mocker +): + monkeypatch.setenv("GITHUB_REF", "refs/heads/feature") + mocker.patch.object(Git, "ensure_safe_directory") + + repository = Git(str(single_branch_checkout)) + + assert repository.is_default_branch is False + + +@pytest.fixture +def stalled_http_remote(): + """An HTTP remote that completes the TCP handshake and never responds.""" + server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + server.bind(("127.0.0.1", 0)) + server.listen() + yield f"http://127.0.0.1:{server.getsockname()[1]}/repo.git" + server.close() + + +def test_stalled_remote_lookup_stops_at_timeout( + single_branch_checkout, stalled_http_remote, monkeypatch +): + _git(single_branch_checkout, "remote", "set-url", "origin", stalled_http_remote) + monkeypatch.setattr(git_interface, "REMOTE_HEAD_TIMEOUT_SECONDS", 1) + for variable in ("HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "http_proxy", "https_proxy", "all_proxy"): + monkeypatch.delenv(variable, raising=False) + repository = Git.__new__(Git) + repository.repo = Repo(str(single_branch_checkout)) + + started = time.monotonic() + result = repository._default_branch_from_remote() + + assert result is None + assert 1 <= time.monotonic() - started < 10 + + +def test_remote_lookup_parses_branch_with_slash(mocker): + process = MagicMock(returncode=0) + process.communicate.return_value = ("ref: refs/heads/release/stable\tHEAD\n", "") + mocker.patch.object(git_interface.subprocess, "Popen", return_value=process) + repository = Git.__new__(Git) + repository.repo = MagicMock(working_dir="/repo") + + assert repository._default_branch_from_remote() == "release/stable" + + +def test_windows_timeout_kills_the_whole_process_tree(monkeypatch, mocker): + monkeypatch.setattr(git_interface, "IS_WINDOWS", True) + run = mocker.patch.object(git_interface.subprocess, "run") + process = MagicMock(pid=4321) + + Git._kill_process_tree(process) + + assert run.call_args.args[0] == ["taskkill", "/F", "/T", "/PID", "4321"] + process.wait.assert_called_once() diff --git a/uv.lock b/uv.lock index bcf3699..8f03888 100644 --- a/uv.lock +++ b/uv.lock @@ -1293,7 +1293,7 @@ wheels = [ [[package]] name = "socketsecurity" -version = "2.10.5" +version = "2.10.6" source = { editable = "." } dependencies = [ { name = "beautifulsoup4" },