Detect changed files across the whole base..HEAD range

Changed-file detection reads a full comparison range only when it recognizes
the CI environment: a GitHub pull request, a GitLab merge request, a Bitbucket
pull request, or a Buildkite pull request. Every other run falls through to
`git show HEAD`, which sees the tip commit alone.

That makes dependency gating depend on commit ordering. A pull request whose
manifest changed in an earlier commit, followed by a source-only commit, looks
like a source-only change: the supported-manifest check fails, the comparison
is abandoned for a full scan, and blocking is suppressed, so the run reports no
new issues and exits 0.

A caller that supplies a base commit has stated the range outright, so honor it
ahead of any inference from the environment, reusing the same range detection
the recognized providers already use. An unresolvable base commit warns rather
than degrading quietly, because the fallback silently narrows the comparison to
one commit.

Author: lelia

socketsecurity/core/git_interface.py

@@ -11,10 +11,21 @@
 class Git:
     repo: Repo
     path: str
+    base_commit_sha: str | None
 
-    def __init__(self, path: str):
+    def __init__(self, path: str, base_commit_sha: str | None = None):
+        """
+        Reads the repository state a scan is built from.
+
+        Args:
+            path: Path to the repository working tree
+            base_commit_sha: Commit the comparison should start from. Supplied when
+                the caller knows the range and no CI environment describes it, which
+                is the only way changed-file detection can see commits behind HEAD.
+        """
         initialization_start = time.perf_counter()
         self.path = path
+        self.base_commit_sha = base_commit_sha
         self._fetched_ref_commits = {}
         self.ensure_safe_directory(path)
         self.repo = Repo(path)
@@ -164,40 +175,61 @@ def __init__(self, path: str):
         buildkite_pr = os.getenv('BUILDKITE_PULL_REQUEST')
         buildkite_base_ref = os.getenv('BUILDKITE_PULL_REQUEST_BASE_BRANCH')
         buildkite_head_ref = os.getenv('BUILDKITE_BRANCH')
-        if self._is_buildkite_pull_request(buildkite_pr) and buildkite_base_ref:
-            detected = self._detect_pull_request_changes(
-                provider="Buildkite",
-                base_ref=buildkite_base_ref,
-                head_ref=buildkite_head_ref,
-            )
-            if detected:
-                detection_source = "buildkite-pr"
-        elif github_event_name == 'pull_request' and github_base_ref:
+
+        # An explicitly supplied base commit states the comparison range outright,
+        # so it is honored before any inference from CI environment variables.
+        if self.base_commit_sha:
             detected = self._detect_pull_request_changes(
-                provider="GitHub",
-                base_ref=github_base_ref,
-                head_ref=github_head_ref,
+                provider="explicit base commit",
+                base_ref=self.base_commit_sha,
+                head_ref=None,
             )
             if detected:
-                detection_source = "github-pr"
-        # Commits to default branch (push events)
-        elif github_event_name == 'push' and github_before_sha and github_sha:
-            try:
-                diff_files = self.repo.git.diff('--name-only', f'{github_before_sha}..{github_sha}')
-                self.show_files = diff_files.splitlines()
-                log.debug(f"Changed files detected via git diff (GitHub push): {self.show_files}")
-                detected = True
-                detection_source = "github-push"
-            except Exception as error:
-                log.debug(f"Failed to get changed files via git diff (GitHub push): {error}")
-        elif github_event_name == 'push':
-            try:
-                self.show_files = self.repo.git.show(self.commit, name_only=True, format="%n").splitlines()
-                log.debug(f"Changed files detected via git show (GitHub push fallback): {self.show_files}")
-                detected = True
-                detection_source = "github-push-fallback"
-            except Exception as error:
-                log.debug(f"Failed to get changed files via git show (GitHub push fallback): {error}")
+                detection_source = "explicit-base-commit"
+            else:
+                log.warning(
+                    f"Could not resolve base commit {self.base_commit_sha} in this "
+                    "checkout, so changed-file detection falls back to the current "
+                    "commit alone. A manifest changed earlier in the range will not "
+                    "be seen, which can skip the comparison entirely. Deepen the "
+                    "clone or fetch the base commit to compare the full range."
+                )
+
+        if not detected:
+            if self._is_buildkite_pull_request(buildkite_pr) and buildkite_base_ref:
+                detected = self._detect_pull_request_changes(
+                    provider="Buildkite",
+                    base_ref=buildkite_base_ref,
+                    head_ref=buildkite_head_ref,
+                )
+                if detected:
+                    detection_source = "buildkite-pr"
+            elif github_event_name == 'pull_request' and github_base_ref:
+                detected = self._detect_pull_request_changes(
+                    provider="GitHub",
+                    base_ref=github_base_ref,
+                    head_ref=github_head_ref,
+                )
+                if detected:
+                    detection_source = "github-pr"
+            # Commits to default branch (push events)
+            elif github_event_name == 'push' and github_before_sha and github_sha:
+                try:
+                    diff_files = self.repo.git.diff('--name-only', f'{github_before_sha}..{github_sha}')
+                    self.show_files = diff_files.splitlines()
+                    log.debug(f"Changed files detected via git diff (GitHub push): {self.show_files}")
+                    detected = True
+                    detection_source = "github-push"
+                except Exception as error:
+                    log.debug(f"Failed to get changed files via git diff (GitHub push): {error}")
+            elif github_event_name == 'push':
+                try:
+                    self.show_files = self.repo.git.show(self.commit, name_only=True, format="%n").splitlines()
+                    log.debug(f"Changed files detected via git show (GitHub push fallback): {self.show_files}")
+                    detected = True
+                    detection_source = "github-push-fallback"
+                except Exception as error:
+                    log.debug(f"Failed to get changed files via git show (GitHub push fallback): {error}")
         # GitLab CI Merge Request context
         if not detected:
             gitlab_target = os.getenv('CI_MERGE_REQUEST_TARGET_BRANCH_NAME')

socketsecurity/socketcli.py

@@ -405,7 +405,7 @@ def main_code():
         is_repo = False
         git_repo: Git
         try:
-            git_repo = Git(config.target_path)
+            git_repo = Git(config.target_path, base_commit_sha=config.base_commit_sha)
             is_repo = True
             if not config.repo:
                 config.repo = git_repo.repo_name

tests/unit/test_git_interface.py

@@ -249,3 +249,98 @@ def test_targeted_fetch_never_uses_all():
 )
 def test_buildkite_pull_request_detection(value, expected):
     assert Git._is_buildkite_pull_request(value) is expected
+
+
+@pytest.fixture
+def commit_range_repo(tmp_path):
+    """A manifest changes mid-range, then a source-only commit lands on top of it."""
+    path = tmp_path / "range-repo"
+    path.mkdir()
+    _git(path, "init", "-b", "main")
+    _git(path, "config", "user.name", "Socket Test")
+    _git(path, "config", "user.email", "socket@example.com")
+    (path / "README.md").write_text("base\n", encoding="utf-8")
+    _git(path, "add", "README.md")
+    _git(path, "commit", "-m", "base")
+    base_sha = _git(path, "rev-parse", "HEAD")
+
+    _git(path, "checkout", "-b", "feature")
+    (path / "pom.xml").write_text("<project/>\n", encoding="utf-8")
+    _git(path, "add", "pom.xml")
+    _git(path, "commit", "-m", "add dependency")
+    manifest_sha = _git(path, "rev-parse", "HEAD")
+
+    (path / "App.java").write_text("class App {}\n", encoding="utf-8")
+    _git(path, "add", "App.java")
+    _git(path, "commit", "-m", "source only")
+    return SimpleNamespace(path=path, base_sha=base_sha, manifest_sha=manifest_sha)
+
+
+def test_head_commit_alone_misses_a_manifest_changed_earlier_in_the_range(
+        commit_range_repo, mocker,
+):
+    mocker.patch.object(Git, "ensure_safe_directory")
+
+    repository = Git(str(commit_range_repo.path))
+
+    # Without a stated base the range is unknown, so only the tip commit is read.
+    assert repository.changed_files == ["App.java"]
+
+
+def test_explicit_base_commit_covers_the_whole_range(
+        commit_range_repo, mocker, caplog,
+):
+    mocker.patch.object(Git, "ensure_safe_directory")
+
+    with caplog.at_level(logging.INFO, logger="socketdev"):
+        repository = Git(
+            str(commit_range_repo.path),
+            base_commit_sha=commit_range_repo.base_sha,
+        )
+
+    assert sorted(repository.changed_files) == ["App.java", "pom.xml"]
+    assert any(
+        "source=explicit-base-commit" in record.message
+        for record in caplog.records
+    )
+
+
+def test_explicit_base_commit_takes_precedence_over_ci_environment(
+        commit_range_repo, monkeypatch, mocker, caplog,
+):
+    # The CI variables describe the whole branch; the explicit base describes only
+    # the last commit. They disagree, so the winner is unambiguous in the result.
+    monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request")
+    monkeypatch.setenv("GITHUB_BASE_REF", "main")
+    monkeypatch.setenv("GITHUB_HEAD_REF", "feature")
+    mocker.patch.object(Git, "ensure_safe_directory")
+
+    with caplog.at_level(logging.INFO, logger="socketdev"):
+        repository = Git(
+            str(commit_range_repo.path),
+            base_commit_sha=commit_range_repo.manifest_sha,
+        )
+
+    assert repository.changed_files == ["App.java"]
+    assert any(
+        "source=explicit-base-commit" in record.message
+        for record in caplog.records
+    )
+
+
+def test_unresolvable_base_commit_warns_and_falls_back(
+        commit_range_repo, mocker, caplog,
+):
+    mocker.patch.object(Git, "ensure_safe_directory")
+    fetch = mocker.patch.object(Git, "_fetch_ref", return_value=None)
+
+    with caplog.at_level(logging.WARNING, logger="socketdev"):
+        repository = Git(str(commit_range_repo.path), base_commit_sha="0" * 40)
+
+    # Falling back silently would hide that the comparison lost most of its range.
+    assert repository.changed_files == ["App.java"]
+    assert any(
+        "Could not resolve base commit" in record.message
+        for record in caplog.records
+    )
+    fetch.assert_called_once()