From 0cf0f740bc391a3549653ffa7d431d98533bfe31 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Wed, 23 Sep 2026 12:34:13 -0400 Subject: [PATCH 1/5] 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. --- socketsecurity/core/git_interface.py | 96 ++++++++++++++++++---------- socketsecurity/socketcli.py | 2 +- tests/unit/test_git_interface.py | 95 +++++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 33 deletions(-) diff --git a/socketsecurity/core/git_interface.py b/socketsecurity/core/git_interface.py index b3c53bdc..72a526bb 100644 --- a/socketsecurity/core/git_interface.py +++ b/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') diff --git a/socketsecurity/socketcli.py b/socketsecurity/socketcli.py index 2be68507..704f1fc2 100644 --- a/socketsecurity/socketcli.py +++ b/socketsecurity/socketcli.py @@ -217,7 +217,7 @@ def apply_git_context(config: CliConfig) -> Tuple[bool, Optional[Git]]: Returns whether target_path is a git repository, along with the Git handle when it is. """ try: - git_repo = Git(config.target_path) + git_repo = Git(config.target_path, base_commit_sha=config.base_commit_sha) except InvalidGitRepositoryError: log.debug("Not a git repository, setting ignore_commit_files=True") config.ignore_commit_files = True diff --git a/tests/unit/test_git_interface.py b/tests/unit/test_git_interface.py index a22cf634..0b03e9c9 100644 --- a/tests/unit/test_git_interface.py +++ b/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("\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() From bd9c5b3355555aa86ae316766049d98073417c7f Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Wed, 23 Sep 2026 12:35:42 -0400 Subject: [PATCH 2/5] Report full-scan findings as repository findings, not new ones A run with no baseline creates a full scan and suppresses blocking, because there is nothing to compare against and so nothing can be attributed to the change. When an alert-bearing output format is enabled the scan still carries every finding in the repository, and the console summary labeled those `NEW` and their link `Diff Url`. Both are wrong for a full scan, and the first contradicts the exit code: the summary reported blocking issues while the run exited 0, which reads as gating that silently failed rather than gating that correctly did not apply. Label the counts and the link by what the run produced, and say why the counts do not gate. The alert list itself is left alone, since SARIF and JSON output read it and renaming their fields would break consumers. --- socketsecurity/output.py | 25 +++++++---- tests/unit/test_summary_text.py | 74 +++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 tests/unit/test_summary_text.py diff --git a/socketsecurity/output.py b/socketsecurity/output.py index 30717360..566e4d3e 100644 --- a/socketsecurity/output.py +++ b/socketsecurity/output.py @@ -229,10 +229,8 @@ def save_sbom_file(self, diff_report: Diff, sbom_file_name: Optional[str] = None def build_summary_text(self, diff_report: Diff) -> str: """Render the console summary text for stdout and file output.""" - if ( - getattr(diff_report, "is_full_scan", False) - and not getattr(diff_report, "alerts_fetched", False) - ): + is_full_scan = getattr(diff_report, "is_full_scan", False) + if is_full_scan and not getattr(diff_report, "alerts_fetched", False): lines = ["Full scan completed. Findings were not fetched for console output."] report_link = getattr(diff_report, "report_url", "") or getattr( diff_report, "diff_url", "" @@ -264,20 +262,33 @@ def build_summary_text(self, diff_report: Diff) -> str: selected_diff = clone_diff_with_selected_alerts(diff_report, selected_alerts) console_security_comment = Messages.create_console_security_alert_table(selected_diff) + # A full scan has no baseline, so everything it carries is a finding in the + # repository rather than one the change introduced. Calling those NEW would + # also contradict the exit code, which stays 0 because no finding can be + # attributed to the change. + blocking_label = "Blocking issues" if is_full_scan else "NEW blocking issues" + warning_label = "Warning issues" if is_full_scan else "NEW warning issues" + link_label = "Report Url" if is_full_scan else "Diff Url" + lines = ["Security issues detected by Socket Security:"] if new_blocking > 0: - lines.append(f" - NEW blocking issues: {new_blocking}") + lines.append(f" - {blocking_label}: {new_blocking}") if new_warning > 0: - lines.append(f" - NEW warning issues: {new_warning}") + lines.append(f" - {warning_label}: {new_warning}") if unchanged_blocking > 0: lines.append( f" - EXISTING blocking issues: {unchanged_blocking} (causing failure due to --strict-blocking)" ) if unchanged_warning > 0: lines.append(f" - EXISTING warning issues: {unchanged_warning}") + if is_full_scan: + lines.append( + " Reported against the whole repository, with no baseline to compare " + "against, so these do not affect the exit code." + ) report_link = getattr(diff_report, "report_url", "") or getattr(diff_report, "diff_url", "") - lines.append(f"Diff Url: {report_link}") + lines.append(f"{link_label}: {report_link}") lines.append("") lines.append(str(console_security_comment)) return "\n".join(lines) diff --git a/tests/unit/test_summary_text.py b/tests/unit/test_summary_text.py new file mode 100644 index 00000000..d75c1d7e --- /dev/null +++ b/tests/unit/test_summary_text.py @@ -0,0 +1,74 @@ +"""How the console summary distinguishes a comparison from a full scan. + +A full scan carries every finding in the repository, not the ones a change +introduced, and blocking is suppressed for exactly that reason. The summary has +to say so, or the reported counts read as gating failures that returned 0. +""" +from unittest.mock import MagicMock + +from socketsecurity.config import CliConfig +from socketsecurity.core.classes import Diff, Issue +from socketsecurity.output import OutputHandler + + +def _issue(name: str, error: bool = False, warn: bool = False) -> Issue: + return Issue( + pkg_name=name, + pkg_version="1.0.0", + severity="high", + title=f"Vuln in {name}", + description="test", + type="vulnerability", + manifests="pom.xml", + pkg_type="maven", + key=f"key-{name}", + purl=f"pkg:maven/{name}@1.0.0", + url=f"https://socket.dev/maven/package/{name}/overview/1.0.0", + error=error, + warn=warn, + ) + + +def _handler() -> OutputHandler: + return OutputHandler(CliConfig.from_args(["--api-token", "test"]), MagicMock()) + + +def _diff(is_full_scan: bool) -> Diff: + diff = Diff() + diff.new_alerts = [_issue("alpha", error=True), _issue("beta", warn=True)] + diff.id = "scan-id" + diff.report_url = "https://socket.dev/dashboard/org/test/sbom/scan-id" + diff.diff_url = diff.report_url + diff.is_full_scan = is_full_scan + diff.alerts_fetched = is_full_scan + return diff + + +def test_comparison_reports_findings_as_new(): + summary = _handler().build_summary_text(_diff(is_full_scan=False)) + + assert "NEW blocking issues: 1" in summary + assert "NEW warning issues: 1" in summary + assert "Diff Url:" in summary + + +def test_full_scan_does_not_report_findings_as_new(): + summary = _handler().build_summary_text(_diff(is_full_scan=True)) + + assert "NEW" not in summary + assert "Blocking issues: 1" in summary + assert "Warning issues: 1" in summary + + +def test_full_scan_labels_its_link_as_a_report(): + summary = _handler().build_summary_text(_diff(is_full_scan=True)) + + assert "Report Url:" in summary + assert "Diff Url:" not in summary + + +def test_full_scan_explains_why_the_counts_do_not_gate(): + summary = _handler().build_summary_text(_diff(is_full_scan=True)) + + assert "no baseline to compare against" in summary + assert "do not affect the exit code" in summary From 7912791f6f83729332347c4b24f202b6020733b6 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Wed, 23 Sep 2026 12:36:40 -0400 Subject: [PATCH 3/5] Stop doubling the namespace in a removed package's purl update_package_values already prefixes a namespaced package's purl with its namespace, so prefixing it again while collecting removed artifacts produced `com.example/widget@1.0.0com.example/com.example/widget@1.0.0`. The purl reaches the dependency overview comment verbatim, so every removed or replaced row for a namespaced package rendered with an unreadable name. The loop collecting added artifacts calls the same function and never did this. --- socketsecurity/core/__init__.py | 2 -- tests/core/test_diff_generation.py | 47 +++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index 811bf650..06c037e6 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -2290,8 +2290,6 @@ def get_added_and_removed_packages( try: pkg = Package.from_diff_artifact(asdict(artifact)) pkg = Core.update_package_values(pkg) - if pkg.namespace: - pkg.purl += f"{pkg.namespace}/{pkg.purl}" removed_packages[artifact.id] = pkg except KeyError: log.error(f"KeyError: Could not create package from removed artifact {artifact.id}") diff --git a/tests/core/test_diff_generation.py b/tests/core/test_diff_generation.py index 5150e3fe..b48fabe4 100644 --- a/tests/core/test_diff_generation.py +++ b/tests/core/test_diff_generation.py @@ -3,7 +3,7 @@ from pathlib import Path import pytest -from socketdev.fullscans import DiffArtifact +from socketdev.fullscans import DiffArtifact, StreamDiffResponse from socketsecurity.core import Core from socketsecurity.core.classes import Package @@ -312,3 +312,48 @@ def print_added_and_removed(added, removed): # pkg1_purl = next(p for p in diff.new_packages if p.id == "pkg1") # assert hasattr(pkg1_purl, "capabilities") # assert set(pkg1_purl.capabilities) == {"File System Access", "Network Access"} + + +def _namespaced_diff_response(namespace: str = "com.example"): + """One namespaced artifact, delivered as both an addition and a removal.""" + raw = json.loads( + (Path(__file__).parent.parent / "data/fullscans/diff/stream_diff.json").read_text() + ) + template = raw["data"]["artifacts"]["added"][0] + artifacts = {bucket: [] for bucket in ("added", "removed", "unchanged", "replaced", "updated")} + for bucket in ("added", "removed"): + artifacts[bucket].append( + dict( + template, + diffType=bucket, + head=None, + base=None, + id=f"namespaced-{bucket}", + namespace=namespace, + name="widget", + version="1.0.0", + type="maven", + ) + ) + return StreamDiffResponse.from_dict({ + "success": raw["success"], + "status": raw["status"], + "data": {**raw["data"], "artifacts": artifacts}, + }) + + +def test_removed_package_purl_matches_the_added_form(core): + """A namespace belongs in the purl once, whichever bucket the artifact arrives in. + + The purl reaches the dependency overview comment verbatim, so a second copy of + the namespace renders as an unreadable package name on every removed or + replaced row. + """ + core.sdk.fullscans.stream_diff.side_effect = None + core.sdk.fullscans.stream_diff.return_value = _namespaced_diff_response() + core.sdk.diffscans.create_from_ids.side_effect = Exception("diff-scans unavailable") + + added, removed, _ = core.get_added_and_removed_packages("head", "new") + + assert added["namespaced-added"].purl == "com.example/widget@1.0.0" + assert removed["namespaced-removed"].purl == added["namespaced-added"].purl From 62a2c0393a1933936d8b17231503cd1375d91132 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Wed, 23 Sep 2026 12:38:19 -0400 Subject: [PATCH 4/5] Describe --ignore-commit-files by what it does, and release 2.10.0 The CLI reference described `--ignore-commit-files` as forcing a full scan in four places, and `--help` said only "Ignore commit files". The flag forces a comparison. The confusion is that "full scan" carries two meanings here: the set of files scanned, where the documentation was right, and the scan mode, where it stated the opposite of the behavior. Anyone looking for a way to run a comparison when the changed-file check would skip one would rule out the only flag that does it. Also documents the range that changed-file detection reads, and that supplying a base commit widens it. --- CHANGELOG.md | 21 +++++++++++++++++++-- docs/cli-reference.md | 18 ++++++++++-------- pyproject.toml | 2 +- socketsecurity/__init__.py | 2 +- socketsecurity/config.py | 6 ++++-- uv.lock | 2 +- 6 files changed, 36 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3909587a..cadb5da4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,20 @@ # Changelog -## 2.9.8 +## 2.10.0 -### Fixed: oversized commit messages no longer fail the scan +### Fixed + +- `--base-commit-sha` now sets the range changed-file detection reads, so a manifest + changed anywhere between that commit and HEAD triggers a comparison. Previously only + a recognized GitHub, GitLab, Bitbucket, or Buildkite pull request read a full range, + and every other run saw the current commit alone. +- A base commit that cannot be resolved in the checkout now warns instead of narrowing + the comparison to a single commit. +- A full scan reports its findings as repository findings rather than new ones, and + labels its link `Report Url`. The summary states why the counts do not affect the + exit code. +- Removed and replaced packages with a namespace no longer render a duplicated purl in + the dependency overview comment. - The 200-character cap on the commit message now applies to the value read from the repository, not only to `--commit-message`. A truncated message ends in `...` and the @@ -10,6 +22,11 @@ - A full scan refused for its size (HTTP 413, 414 or 431) now distinguishes possible upload-size and request-metadata causes and reports what to shorten. +### Changed + +- `--ignore-commit-files` is documented as forcing a comparison, which is what it does. + The CLI reference and `--help` described it as forcing a full scan. + ## 2.9.7 ### Changed: bump pinned @coana-tech/cli to 15.10.51 diff --git a/docs/cli-reference.md b/docs/cli-reference.md index d0d85e2c..b02cf7c0 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -256,7 +256,7 @@ If you don't want to provide the Socket API Token every time then you can use th | `--commit-message` | False | *auto* | Commit message (auto-detected from git) | | `--commit-sha` | False | *auto* | Commit SHA (auto-detected from git) | | `--base-scan-id` | False | | Full scan ID to diff against, overriding the repository's head scan as the baseline. Mutually exclusive with `--base-commit-sha` | -| `--base-commit-sha`| False | | Commit SHA to prefer as the diff baseline, overriding the repository's head scan. The CLI uses its most recent matching full scan or the nearest scanned first-parent ancestor within 100 local commits. It errors (exit code 3, or `--exit-code-on-api-error`) if no scanned ancestor is reachable. Mutually exclusive with `--base-scan-id` | +| `--base-commit-sha`| False | | Commit SHA to prefer as the diff baseline, overriding the repository's head scan. The CLI uses its most recent matching full scan or the nearest scanned first-parent ancestor within 100 local commits. It errors (exit code 3, or `--exit-code-on-api-error`) if no scanned ancestor is reachable. Also sets the range changed-file detection reads, so a manifest changed anywhere between this commit and HEAD is seen. Mutually exclusive with `--base-scan-id` | > **Diffing against the merge base** — by default, PR scans are diffed against the repository's latest matching head scan, which may include newer default-branch commits than your PR branched from. To prefer the commit your PR is based on, compute the merge base and pass it as the baseline: > @@ -267,6 +267,8 @@ If you don't want to provide the Socket API Token every time then you can use th > > `--base-commit-sha` does not create a scan of that commit. The CLI first looks for the newest non-temporary scan matching the repository, workspace, scan type, and exact commit. If the exact commit was not scanned, it walks up to 100 first-parent commits from that SHA in the local checkout and uses the nearest matching scanned ancestor. It logs a warning with the selected commit and distance because this produces a wider diff than the merge base. > +> Supplying `--base-commit-sha` also widens the range the CLI reads when deciding whether any manifest changed. Without it, and outside a recognized CI pull request or merge request, the CLI sees only the current commit, so a pull request whose manifest changed in an earlier commit is treated as a source-only change and the comparison is skipped. If the base commit cannot be resolved in the local checkout, the CLI warns and falls back to the current commit alone; deepen the clone or fetch the base commit to compare the full range. +> > Run `socketcli` regularly on the default branch so recent ancestors have scans. PR checkouts must also retain the merge base and enough first-parent history; shallow clones can shorten the search. Gaps are expected when CI cancels intermediate builds, commits use `[skip ci]`, pipelines are path-filtered, or the merge base predates your Socket rollout. > > If no scanned ancestor is reachable within the local 100-commit walk, the CLI **fails** (exit code 3, or your `--exit-code-on-api-error` value; exit 0 with `--disable-blocking`) instead of silently falling back to the repository head. API or permission failures also fail rather than being treated as a missing exact scan. @@ -427,7 +429,7 @@ The launcher can be tuned via the `SOCKET_CLI_COANA_LAUNCHER` environment variab #### Advanced Configuration | Parameter | Required | Default | Description | |:-------------------------|:---------|:--------|:----------------------------------------------------------------------| -| `--ignore-commit-files` | False | False | Ignore commit files | +| `--ignore-commit-files` | False | False | Compare regardless of which files changed, scanning every manifest | | `--disable-blocking` | False | False | Non-blocking CI mode: the CLI always exits **0**, even when blocking alerts are present (including with `--strict-blocking`). Also exits 0 on uncaught runtime errors and Socket API failures, so the job is treated as successful while findings and errors are still logged. Takes precedence over `--strict-blocking`. | | `--disable-ignore` | False | False | Disable support for `@SocketSecurity ignore` commands in PR comments. When set, alerts cannot be suppressed via comments and ignore instructions are hidden from comment output. See [Who can ignore an alert](#who-can-ignore-an-alert). | | `--ignore-authorization` | False | enforce | Who may suppress alerts with `@SocketSecurity ignore`. `enforce` requires write access and honors the command with a warning when the provider cannot report it; `strict` rejects it in that case; `off` honors any commenter. See [Who can ignore an alert](#who-can-ignore-an-alert). | @@ -674,10 +676,10 @@ The CLI now automatically detects repository information from your git environme - **Commit message**: Latest commit message - **Committer information**: Git commit author details - **Default branch status**: Determined from git repository and CI environment -- **Changed files**: Files modified in the current commit (for differential scanning) +- **Changed files**: Files modified in the current commit, or across the whole `--base-commit-sha`..HEAD range when a base commit is supplied (for differential scanning) > **Note on merge commits**: > Standard merges (two parents) are supported. -> For *octopus merges* (three or more parents), Git only reports changes relative to the first parent. This can lead to incomplete or empty file lists if changes only exist relative to other parents. In these cases, differential scanning may be skipped. To ensure coverage, use `--ignore-commit-files` to force a full scan or specify files explicitly with `--files`. +> For *octopus merges* (three or more parents), Git only reports changes relative to the first parent. This can lead to incomplete or empty file lists if changes only exist relative to other parents. In these cases, differential scanning may be skipped. To ensure coverage, use `--ignore-commit-files` to compare regardless of the detected changes, or specify files explicitly with `--files`. ### Default Branch Detection The CLI uses intelligent default branch detection with the following priority: @@ -728,11 +730,11 @@ GitLab token/auth behavior and CI examples are documented in [`ci-cd.md`](ci-cd. The CLI determines which files to scan based on the following logic: -1. **Git Commit Files (Default)**: The CLI automatically checks files changed in the current git commit. If any of these files match supported manifest patterns (like package.json, requirements.txt, etc.), a scan is triggered. +1. **Git Commit Files (Default)**: The CLI automatically checks which files changed. In a recognized CI pull request or merge request, and whenever `--base-commit-sha` is supplied, that is every file changed across the range; otherwise it is the current commit alone. If any of them match supported manifest patterns (like package.json, requirements.txt, etc.), a comparison is run. 2. **`--files` Parameter Override**: When specified, this parameter takes precedence over git commit detection. It accepts a JSON array of file paths to check for manifest files. -3. **`--ignore-commit-files` Flag**: When set, git commit files are ignored completely, and the CLI will scan all manifest files in the target directory regardless of what changed. +3. **`--ignore-commit-files` Flag**: When set, the changed-file check is skipped entirely. The CLI runs the comparison regardless of what changed, and scans every manifest file in the target directory. 4. **Automatic Fallback**: If no manifest files are found in git commit changes and no `--files` are specified, the CLI automatically switches to "API mode" and performs a full repository scan. @@ -742,7 +744,7 @@ The CLI determines which files to scan based on the following logic: - **Differential Mode**: When manifest files are detected in changes, performs a diff scan with PR/MR comment integration - **API Mode**: When no manifest files are in changes, creates a full scan report without PR comments but still scans the entire repository -- **Force Mode**: With `--ignore-commit-files`, always performs a full scan regardless of changes +- **Force Mode**: With `--ignore-commit-files`, always runs a comparison regardless of which files changed, over every manifest in the target path - **Forced Diff Mode**: With `--enable-diff`, forces differential mode even when using `--integration api` (without SCM integration) ### Examples @@ -750,7 +752,7 @@ The CLI determines which files to scan based on the following logic: - **Commit with manifest file**: If your commit includes changes to `package.json`, a differential scan will be triggered automatically with PR comment integration. - **Commit without manifest files**: If your commit only changes non-manifest files (like `.github/workflows/socket.yaml`), the CLI automatically switches to API mode and performs a full repository scan. - **Using `--files`**: If you specify `--files '["package.json"]'`, the CLI will check if this file exists and is a manifest file before determining scan type. -- **Using `--ignore-commit-files`**: This forces a full scan of all manifest files in the target path, regardless of what's in your commit. +- **Using `--ignore-commit-files`**: This runs the comparison regardless of what's in your commit, over all manifest files in the target path. Use it when the changed-file check would otherwise skip a comparison you need. - **Using `--enable-diff`**: Forces diff mode without SCM integration - useful when you want differential scanning but are using `--integration api`. For example: `socketcli --integration api --enable-diff --target-path /path/to/repo` - **Auto-detection**: Most CI/CD scenarios now work with just `socketcli --target-path /path/to/repo --scm github --pr-number $PR_NUM` diff --git a/pyproject.toml b/pyproject.toml index db09130d..d98e04a2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" [project] name = "socketsecurity" -version = "2.9.8" +version = "2.10.0" requires-python = ">= 3.11" license = {"file" = "LICENSE"} dependencies = [ diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index 5730d9be..bbe367f0 100644 --- a/socketsecurity/__init__.py +++ b/socketsecurity/__init__.py @@ -1,3 +1,3 @@ __author__ = 'socket.dev' -__version__ = '2.9.8' +__version__ = '2.10.0' USER_AGENT = f'SocketPythonCLI/{__version__}' diff --git a/socketsecurity/config.py b/socketsecurity/config.py index d497f8ea..1f107cf8 100644 --- a/socketsecurity/config.py +++ b/socketsecurity/config.py @@ -631,7 +631,9 @@ def create_argument_parser() -> argparse.ArgumentParser: help="Commit SHA to diff the new scan against, overriding the repository's head " "scan as the baseline. The CLI uses the most recent matching full scan, or " "the nearest scanned first-parent ancestor within 100 local commits when " - "the commit itself was not scanned. Mutually exclusive with --base-scan-id." + "the commit itself was not scanned. Also sets the range changed-file " + "detection reads, so a manifest changed anywhere between this commit and " + "HEAD is seen. Mutually exclusive with --base-scan-id." ) # Path and File options @@ -932,7 +934,7 @@ def create_argument_parser() -> argparse.ArgumentParser: "--ignore-commit-files", dest="ignore_commit_files", action="store_true", - help="Ignore commit files" + help="Compare against the baseline regardless of which files changed, scanning every manifest in the target path" ) advanced_group.add_argument( "--ignore_commit_files", diff --git a/uv.lock b/uv.lock index 030af62b..c453682f 100644 --- a/uv.lock +++ b/uv.lock @@ -1293,7 +1293,7 @@ wheels = [ [[package]] name = "socketsecurity" -version = "2.9.8" +version = "2.10.0" source = { editable = "." } dependencies = [ { name = "beautifulsoup4" }, From 5839d100f0e65e82731353140d645b5526ac523c Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Wed, 23 Sep 2026 19:29:44 -0400 Subject: [PATCH 5/5] Handle explicit base ranges in shallow clones --- socketsecurity/core/git_interface.py | 9 +++++--- tests/unit/test_git_interface.py | 31 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/socketsecurity/core/git_interface.py b/socketsecurity/core/git_interface.py index 72a526bb..81001f4b 100644 --- a/socketsecurity/core/git_interface.py +++ b/socketsecurity/core/git_interface.py @@ -183,6 +183,7 @@ def __init__(self, path: str, base_commit_sha: str | None = None): provider="explicit base commit", base_ref=self.base_commit_sha, head_ref=None, + use_merge_base=False, ) if detected: detection_source = "explicit-base-commit" @@ -348,8 +349,9 @@ def _detect_pull_request_changes( provider: str, base_ref: str, head_ref: str | None, + use_merge_base: bool = True, ) -> bool: - """Detect a full PR range locally, fetching only refs needed to complete it.""" + """Detect a base-to-head range locally, fetching only refs needed to complete it.""" base_commit = self._resolve_ref(base_ref) if base_commit is None: base_commit = self._fetch_ref(base_ref, f"{provider} pull-request base ref missing") @@ -358,7 +360,8 @@ def _detect_pull_request_changes( return False head_commit = self.commit.hexsha - diff_range = f"{base_commit}...{head_commit}" + range_separator = "..." if use_merge_base else ".." + diff_range = f"{base_commit}{range_separator}{head_commit}" try: diff_files = self.repo.git.diff("--name-only", diff_range) self.show_files = diff_files.splitlines() @@ -384,7 +387,7 @@ def _detect_pull_request_changes( try: diff_files = self.repo.git.diff( "--name-only", - f"{base_commit}...{head_commit}", + f"{base_commit}{range_separator}{head_commit}", ) self.show_files = diff_files.splitlines() log.debug( diff --git a/tests/unit/test_git_interface.py b/tests/unit/test_git_interface.py index 0b03e9c9..ea2d9414 100644 --- a/tests/unit/test_git_interface.py +++ b/tests/unit/test_git_interface.py @@ -305,6 +305,37 @@ def test_explicit_base_commit_covers_the_whole_range( ) +def test_explicit_base_commit_does_not_require_merge_base( + commit_range_repo, tmp_path, mocker, +): + shallow_path = tmp_path / "shallow-range-repo" + _git( + tmp_path, + "clone", + "--depth=1", + "--branch=feature", + commit_range_repo.path.as_uri(), + str(shallow_path), + ) + mocker.patch.object(Git, "ensure_safe_directory") + + repository = Git( + str(shallow_path), + base_commit_sha=commit_range_repo.base_sha, + ) + + # Fetching the base supplies both endpoint trees but does not deepen the + # feature history enough to calculate a merge base. + merge_base = subprocess.run( + ["git", "merge-base", commit_range_repo.base_sha, "HEAD"], + cwd=shallow_path, + capture_output=True, + text=True, + ) + assert merge_base.returncode != 0 + assert sorted(repository.changed_files) == ["App.java", "pom.xml"] + + def test_explicit_base_commit_takes_precedence_over_ci_environment( commit_range_repo, monkeypatch, mocker, caplog, ):