From 691ca08a0e6d05556e07fc94f923dc577c634a81 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Thu, 3 Sep 2026 15:30:49 -0400 Subject: [PATCH 01/14] fix(gitlab): stabilize report data --- socketsecurity/core/__init__.py | 29 +++++++++++++-- socketsecurity/core/classes.py | 16 ++++----- socketsecurity/core/messages.py | 45 +++++++++++++---------- tests/core/test_package_and_alerts.py | 25 +++++++++++-- tests/core/test_sdk_methods.py | 52 +++++++++++++++++++++++++++ tests/unit/test_gitlab_format.py | 33 +++++++++++++---- 6 files changed, 162 insertions(+), 38 deletions(-) diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index a5305bee..eb21d8c7 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -1431,17 +1431,38 @@ def get_repo_info(self, repo_slug: str, default_branch: str = "socket-default-br return response.data - def get_head_scan_for_repo(self, repo_slug: str) -> str: + def get_head_scan_for_repo( + self, + repo_slug: str, + workspace: Optional[str] = None, + scan_type: Optional[str] = None, + ) -> Optional[str]: """ Gets the head scan ID for a repository. Args: repo_slug: Repository slug to get head scan for + workspace: Socket workspace the scan belongs to, if any + scan_type: Socket scan type to match, if any Returns: Head scan ID if it exists, None otherwise """ repo_info = self.get_repo_info(repo_slug) + if workspace: + query_params = { + "repo": repo_slug, + "workspace": workspace, + "branch": repo_info.default_branch, + "sort": "created_at", + "direction": "desc", + "per_page": 1, + } + if scan_type: + query_params["scan_type"] = scan_type + response = self.sdk.fullscans.get(self.config.org_slug, query_params) + results = response.get("results") if isinstance(response, dict) else None + return results[0].get("id") if results else None return repo_info.head_full_scan_id if repo_info.head_full_scan_id else None def get_full_scan_id_by_commit( @@ -1528,7 +1549,11 @@ def resolve_base_full_scan_id(self, params: FullScanParams) -> Optional[str]: return scan_id try: - return self.get_head_scan_for_repo(params.repo) + return self.get_head_scan_for_repo( + params.repo, + workspace=params.workspace, + scan_type=params.scan_type, + ) except APIResourceNotFound: return None diff --git a/socketsecurity/core/classes.py b/socketsecurity/core/classes.py index db145221..46d8ffc9 100644 --- a/socketsecurity/core/classes.py +++ b/socketsecurity/core/classes.py @@ -153,18 +153,16 @@ def from_socket_artifact(cls, data: dict) -> "Package": Returns: New Package instance """ - purl = f"{data['type']}/" - namespace = data.get("namespace") - if namespace: - purl += f"{namespace}@" - purl += f"{data['name']}@{data['version']}" - base_url = "https://socket.dev" - url = f"{base_url}/{data['type']}/package/{namespace or ''}{data['name']}/overview/{data['version']}" + package_type = getattr(data["type"], "value", data["type"]) + namespace = (data.get("namespace") or "").strip("/") + package_path = "/".join(part for part in (namespace, data["name"]) if part) + purl = f"{package_type}/{package_path}@{data['version']}" + url = f"https://socket.dev/{package_type}/package/{package_path}/overview/{data['version']}" return cls( id=data["id"], name=data["name"], version=data["version"], - type=data["type"], + type=package_type, release=data.get("release"), diffType=data.get("diffType"), score=data["score"], @@ -179,7 +177,7 @@ def from_socket_artifact(cls, data: dict) -> "Package": artifact=data.get("artifact"), purl=purl, url=url, - namespace=namespace + namespace=namespace or None ) @classmethod diff --git a/socketsecurity/core/messages.py b/socketsecurity/core/messages.py index 673dde5c..18f56fbf 100644 --- a/socketsecurity/core/messages.py +++ b/socketsecurity/core/messages.py @@ -5,6 +5,7 @@ import uuid from datetime import datetime, timezone from pathlib import Path + from mdutils import MdUtils from prettytable import PrettyTable @@ -655,25 +656,31 @@ def extract_identifiers_gitlab(alert: Issue) -> list: "url": alert.url if hasattr(alert, 'url') and alert.url else None }) - # Extract CVE identifiers from props - if hasattr(alert, 'props') and alert.props: - if 'cve' in alert.props: - cves = alert.props['cve'] - if isinstance(cves, list): - for cve in cves: - identifiers.append({ - "type": "cve", - "name": cve, - "value": cve, - "url": f"https://cve.mitre.org/cgi-bin/cvename.cgi?name={cve}" - }) - elif isinstance(cves, str): - identifiers.append({ - "type": "cve", - "name": cves, - "value": cves, - "url": f"https://cve.mitre.org/cgi-bin/cvename.cgi?name={cves}" - }) + props = getattr(alert, "props", None) or {} + identifier_fields = ( + ("cveId", "cve", "https://nvd.nist.gov/vuln/detail/"), + ("cve", "cve", "https://nvd.nist.gov/vuln/detail/"), + ("ghsaId", "ghsa", "https://github.com/advisories/"), + ) + seen = set() + for field, identifier_type, url_prefix in identifier_fields: + values = props.get(field, []) + if isinstance(values, str): + values = [values] + for value in values or []: + if not isinstance(value, str) or not value.strip(): + continue + value = value.strip() + identifier_key = (identifier_type, value.upper()) + if identifier_key in seen: + continue + seen.add(identifier_key) + identifiers.append({ + "type": identifier_type, + "name": value, + "value": value, + "url": f"{url_prefix}{value}" + }) return identifiers diff --git a/tests/core/test_package_and_alerts.py b/tests/core/test_package_and_alerts.py index 171eae77..4d1fa3b1 100644 --- a/tests/core/test_package_and_alerts.py +++ b/tests/core/test_package_and_alerts.py @@ -1,8 +1,9 @@ -from dataclasses import dataclass +from dataclasses import asdict, dataclass from unittest.mock import Mock import pytest from socketdev import socketdev +from socketdev.fullscans import SocketArtifact from socketsecurity.core import Core, _humanize_alert_type from socketsecurity.core.classes import Issue, Package @@ -104,6 +105,27 @@ def test_create_packages_dict_basic(self, core): assert pkg.version == "1.0.0" assert pkg.transitives == 0 + def test_full_scan_package_normalizes_enum_type_and_namespace_url(self): + artifact = SocketArtifact.from_dict({ + "id": "pkg:maven/com.arenko/trading-core@1.2.3", + "type": "maven", + "namespace": "com.arenko", + "name": "trading-core", + "version": "1.2.3", + "direct": True, + "topLevelAncestors": [], + "manifestFiles": [{"file": "pom.xml"}], + "alerts": [], + }) + + package = Package.from_socket_artifact(asdict(artifact)) + + assert package.type == "maven" + assert package.purl == "maven/com.arenko/trading-core@1.2.3" + assert package.url == ( + "https://socket.dev/maven/package/com.arenko/trading-core/overview/1.2.3" + ) + def test_create_packages_dict_with_transitives(self, core): """Test package dictionary creation with transitive dependencies""" mock_artifacts = [ @@ -340,4 +362,3 @@ def test_empty_input_returns_empty_string(self): def test_handles_acronyms_conservatively(self): """Adjacent capitals are kept together: SQLInjection -> 'SQL Injection'.""" assert _humanize_alert_type("SQLInjection") == "SQL Injection" - diff --git a/tests/core/test_sdk_methods.py b/tests/core/test_sdk_methods.py index da0efc62..e04573dd 100644 --- a/tests/core/test_sdk_methods.py +++ b/tests/core/test_sdk_methods.py @@ -63,6 +63,35 @@ def test_get_head_scan_for_repo_no_head(core, mock_sdk_with_responses): head_scan_id = core.get_head_scan_for_repo("no-head") assert head_scan_id is None + +def test_get_head_scan_for_repo_scopes_workspace_to_default_branch( + core, mock_sdk_with_responses +): + mock_sdk_with_responses.fullscans.get.return_value = { + "results": [{"id": "workspace-head"}], + "nextPage": None, + } + + head_scan_id = core.get_head_scan_for_repo( + "test", + workspace="customer-a", + scan_type="socket_tier1", + ) + + assert head_scan_id == "workspace-head" + mock_sdk_with_responses.fullscans.get.assert_called_once_with( + core.config.org_slug, + { + "repo": "test", + "workspace": "customer-a", + "branch": "main", + "sort": "created_at", + "direction": "desc", + "per_page": 1, + "scan_type": "socket_tier1", + }, + ) + def test_get_full_scan_id_by_commit(core, mock_sdk_with_responses): """Looks up the newest full scan for a repo + commit via the list endpoint""" mock_sdk_with_responses.fullscans.get.return_value = { @@ -126,6 +155,29 @@ def test_resolve_base_full_scan_id_defaults_to_head_scan(core): """Without base overrides the repository head scan is the baseline""" assert core.resolve_base_full_scan_id(make_full_scan_params()) == "head" + +def test_resolve_base_full_scan_id_scopes_head_to_workspace(core): + core.sdk.fullscans.get.return_value = { + "results": [{"id": "workspace-head"}], + "nextPage": None, + } + + params = make_full_scan_params(workspace="customer-a", scan_type="socket_tier1") + + assert core.resolve_base_full_scan_id(params) == "workspace-head" + core.sdk.fullscans.get.assert_called_once_with( + core.config.org_slug, + { + "repo": "test", + "workspace": "customer-a", + "branch": "main", + "sort": "created_at", + "direction": "desc", + "per_page": 1, + "scan_type": "socket_tier1", + }, + ) + def test_resolve_base_full_scan_id_uses_base_scan_id(core): """--base-scan-id is used verbatim, without touching the repo endpoint""" core.cli_config = make_cli_config("--base-scan-id", "explicit-base") diff --git a/tests/unit/test_gitlab_format.py b/tests/unit/test_gitlab_format.py index 96218e4e..a8126c70 100644 --- a/tests/unit/test_gitlab_format.py +++ b/tests/unit/test_gitlab_format.py @@ -1,8 +1,7 @@ import re -import pytest -from socketsecurity.core.messages import Messages from socketsecurity.core.classes import Diff, Issue +from socketsecurity.core.messages import Messages class TestGitLabFormat: @@ -87,7 +86,10 @@ def test_identifier_extraction_with_cve(self): type="vulnerability", severity="critical", title="Known CVE", - props={"cve": ["CVE-2024-5678", "CVE-2024-9012"]}, + props={ + "cveId": ["CVE-2024-5678", "CVE-2024-9012"], + "ghsaId": "GHSA-1234-5678-9012", + }, pkg_type="npm", key="test-key", purl="pkg:npm/vulnerable-pkg@2.0.0" @@ -97,15 +99,17 @@ def test_identifier_extraction_with_cve(self): report = Messages.create_security_comment_gitlab(diff) vuln = report["vulnerabilities"][0] - # Should have socket_alert identifier + 2 CVE identifiers - assert len(vuln["identifiers"]) >= 3 + # Should have socket_alert identifier + CVE and GHSA identifiers + assert len(vuln["identifiers"]) == 4 cve_identifiers = [i for i in vuln["identifiers"] if i["type"] == "cve"] assert len(cve_identifiers) == 2 assert any(i["value"] == "CVE-2024-5678" for i in cve_identifiers) assert any(i["value"] == "CVE-2024-9012" for i in cve_identifiers) + ghsa_identifiers = [i for i in vuln["identifiers"] if i["type"] == "ghsa"] + assert ghsa_identifiers[0]["value"] == "GHSA-1234-5678-9012" def test_identifier_extraction_with_single_cve_string(self): - """Test single CVE identifier as string""" + """Legacy CVE property remains supported""" diff = Diff() diff.id = "test-scan-id" diff.diff_url = "https://socket.dev/test" @@ -130,6 +134,23 @@ def test_identifier_extraction_with_single_cve_string(self): assert len(cve_identifiers) == 1 assert cve_identifiers[0]["value"] == "CVE-2024-1111" + def test_identifier_extraction_deduplicates_legacy_and_current_cve_fields(self): + issue = Issue( + pkg_name="vulnerable-pkg", + pkg_version="2.0.0", + type="vulnerability", + severity="high", + title="Duplicate CVE", + props={"cve": "CVE-2024-1111", "cveId": "CVE-2024-1111"}, + pkg_type="npm", + key="test-key", + purl="pkg:npm/vulnerable-pkg@2.0.0", + ) + + identifiers = Messages.extract_identifiers_gitlab(issue) + + assert [item["value"] for item in identifiers].count("CVE-2024-1111") == 1 + def test_dependency_chain_handling_transitive(self): """Test transitive dependency path is captured""" diff = Diff() From 2bb42d4ac08c1d62733f40d21bfa54976bfc93e5 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Thu, 3 Sep 2026 15:33:43 -0400 Subject: [PATCH 02/14] chore: bump version to 2.8.1 --- CHANGELOG.md | 11 +++++++++++ pyproject.toml | 2 +- socketsecurity/__init__.py | 2 +- uv.lock | 2 +- 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1007a3b9..e9987f9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## 2.8.1 + +### Fixed: GitLab report serialization and workspace baselines + +- Full-scan package identities and Socket links now preserve namespaced packages + when the SDK returns enum-backed ecosystem values. +- GitLab dependency-scanning reports emit CVE and GHSA identifiers from current + API fields while remaining compatible with legacy CVE data. +- Implicit diff baselines are selected from the same workspace, scan type, + repository, and default branch. + ## 2.7.1 ### Changed: bump pinned @coana-tech/cli to 15.10.36 diff --git a/pyproject.toml b/pyproject.toml index 3dcd5718..ec8f544d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" [project] name = "socketsecurity" -version = "2.7.1" +version = "2.8.1" requires-python = ">= 3.11" license = {"file" = "LICENSE"} dependencies = [ diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index 78220a1f..6cf31cd7 100644 --- a/socketsecurity/__init__.py +++ b/socketsecurity/__init__.py @@ -1,3 +1,3 @@ __author__ = 'socket.dev' -__version__ = '2.7.1' +__version__ = '2.8.1' USER_AGENT = f'SocketPythonCLI/{__version__}' diff --git a/uv.lock b/uv.lock index d0338009..d722ee6c 100644 --- a/uv.lock +++ b/uv.lock @@ -1282,7 +1282,7 @@ wheels = [ [[package]] name = "socketsecurity" -version = "2.7.1" +version = "2.8.1" source = { editable = "." } dependencies = [ { name = "beautifulsoup4" }, From f6d5aa7e54943e3da2727e53022066a1f5e7df1a Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:07:00 -0400 Subject: [PATCH 03/14] fix(gitlab): harden implicit diff baseline resolution The workspace-scoped head scan lookup treated any failed request as "no baseline". The SDK logs and returns {} for every non-200, so a transient API error resolved to None, and create_new_diff answers None by creating an empty baseline scan -- reporting every dependency in the repository as newly added. An absent "results" key now raises APIFailure, and resolve_base_full_scan_id surfaces it the same way a missing --base-commit-sha baseline is surfaced. Selecting the newest scan on the default branch also reintroduced temporary scans, which the repository head pointer had excluded. The empty baseline scan that create_new_diff creates inherits the branch and commit of the run that created it, so a default-branch run whose real scan fails leaves that empty scan as the newest one. Both baseline lookups now skip tmp scans. Also unwrap scan_type before it is URL encoded. FullScanParams types it as a ScanType enum, and urlencode renders a (str, Enum) member as its repr-style name, which would filter on a scan type that does not exist. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 +- socketsecurity/core/__init__.py | 85 +++++++++++++++++++++++-- tests/core/test_sdk_methods.py | 108 +++++++++++++++++++++++++++++--- 3 files changed, 183 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9987f9d..88f99d31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,9 @@ - GitLab dependency-scanning reports emit CVE and GHSA identifiers from current API fields while remaining compatible with legacy CVE data. - Implicit diff baselines are selected from the same workspace, scan type, - repository, and default branch. + repository, and default branch. A baseline lookup that fails is reported as an + API error instead of resolving to an empty baseline, and temporary scans are + skipped when selecting one. ## 2.7.1 diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index eb21d8c7..f12ff0eb 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -51,6 +51,11 @@ _HUMANIZE_BOUNDARY = re.compile(r"(?<=[a-z0-9])(?=[A-Z])|(?<=[A-Z])(?=[A-Z][a-z])") +# How many full scans to request when resolving a diff baseline. The newest scan is +# usually the one we want, but temporary scans have to be skipped (see +# Core.newest_persisted_scan_id), so a single result is not enough. +SCAN_LOOKUP_PAGE_SIZE = 10 + # Reachability facts-file upload compression. # # The Socket full-scan endpoint transparently brotli-decompresses any multipart part @@ -1440,6 +1445,11 @@ def get_head_scan_for_repo( """ Gets the head scan ID for a repository. + Without a workspace this is the repository's head scan pointer. That pointer + tracks a single scan for the whole repository rather than one per workspace, + so workspace-scoped runs instead take the newest matching scan on the default + branch. + Args: repo_slug: Repository slug to get head scan for workspace: Socket workspace the scan belongs to, if any @@ -1447,6 +1457,12 @@ def get_head_scan_for_repo( Returns: Head scan ID if it exists, None otherwise + + Raises: + APIFailure: If the workspace scan lookup fails. A failed lookup must not + be reported as "no baseline": the caller answers that by creating an + empty baseline scan, which reports every dependency in the repository + as newly added. """ repo_info = self.get_repo_info(repo_slug) if workspace: @@ -1456,15 +1472,58 @@ def get_head_scan_for_repo( "branch": repo_info.default_branch, "sort": "created_at", "direction": "desc", - "per_page": 1, + "per_page": SCAN_LOOKUP_PAGE_SIZE, } if scan_type: - query_params["scan_type"] = scan_type + query_params["scan_type"] = Core.query_param_value(scan_type) response = self.sdk.fullscans.get(self.config.org_slug, query_params) results = response.get("results") if isinstance(response, dict) else None - return results[0].get("id") if results else None + if results is None: + # The SDK logs and returns {} for any non-200, so an empty "results" + # key is the only signal that the request itself succeeded. + raise APIFailure( + f"Failed to list full scans for repo {repo_slug} in workspace {workspace}" + ) + return Core.newest_persisted_scan_id(results) return repo_info.head_full_scan_id if repo_info.head_full_scan_id else None + @staticmethod + def query_param_value(value): + """ + Unwraps an enum member so it survives URL encoding. + + The SDK types several params as str-backed enums (ScanType, IntegrationType). + urlencode calls str() on values, and a (str, Enum) mixin renders as + "ScanType.SOCKET_TIER1" rather than "socket_tier1", which would silently + filter on a scan type that does not exist. + """ + return getattr(value, "value", value) + + @staticmethod + def newest_persisted_scan_id(results: List[dict]) -> Optional[str]: + """ + Returns the newest scan ID from a full scan listing, skipping temporary scans. + + create_new_diff creates an empty ``tmp`` scan when a repository has no baseline + yet, and that scan inherits the branch and commit of the run that created it. + If the real scan then fails, the empty scan is left behind as the newest scan + for that branch/commit; selecting it as a baseline would report every + dependency as newly added. + + Args: + results: Full scan listing results, newest first + + Returns: + Newest non-temporary scan ID, or None if the listing has none + """ + for result in results or []: + if not isinstance(result, dict) or result.get("tmp"): + continue + scan_id = result.get("id") + if scan_id: + return scan_id + return None + def get_full_scan_id_by_commit( self, repo_slug: str, @@ -1493,12 +1552,12 @@ def get_full_scan_id_by_commit( "commit_hash": commit_sha, "sort": "created_at", "direction": "desc", - "per_page": 1, + "per_page": SCAN_LOOKUP_PAGE_SIZE, } if workspace: query_params["workspace"] = workspace if scan_type: - query_params["scan_type"] = scan_type + query_params["scan_type"] = Core.query_param_value(scan_type) response = self.sdk.fullscans.get( self.config.org_slug, @@ -1507,7 +1566,7 @@ def get_full_scan_id_by_commit( results = response.get("results") if isinstance(response, dict) else None if not results: return None - return results[0].get("id") + return Core.newest_persisted_scan_id(results) def resolve_base_full_scan_id(self, params: FullScanParams) -> Optional[str]: """ @@ -1556,6 +1615,20 @@ def resolve_base_full_scan_id(self, params: FullScanParams) -> Optional[str]: ) except APIResourceNotFound: return None + except APIFailure as error: + # Only workspace-scoped lookups raise here. Returning None instead would + # make the caller create an empty baseline scan, reporting every + # dependency as newly added, so fail loudly like the --base-commit-sha + # path above. + log.error( + f"Failed to resolve the head scan for repo {params.repo} in workspace " + f"{params.workspace}: {error}" + ) + if self.cli_config is None: + raise + if self.cli_config.disable_blocking: + sys.exit(0) + sys.exit(self.cli_config.exit_code_on_api_error) @staticmethod def update_package_values(pkg: Package) -> Package: diff --git a/tests/core/test_sdk_methods.py b/tests/core/test_sdk_methods.py index e04573dd..07b84459 100644 --- a/tests/core/test_sdk_methods.py +++ b/tests/core/test_sdk_methods.py @@ -1,9 +1,9 @@ import pytest from socketdev.exceptions import APIFailure -from socketdev.fullscans import FullScanParams, FullScanStreamResponse +from socketdev.fullscans import FullScanParams, FullScanStreamResponse, ScanType from socketsecurity.config import CliConfig -from socketsecurity.core import Core +from socketsecurity.core import SCAN_LOOKUP_PAGE_SIZE, Core from socketsecurity.core.socket_config import SocketConfig @@ -87,11 +87,57 @@ def test_get_head_scan_for_repo_scopes_workspace_to_default_branch( "branch": "main", "sort": "created_at", "direction": "desc", - "per_page": 1, + "per_page": SCAN_LOOKUP_PAGE_SIZE, "scan_type": "socket_tier1", }, ) + +def test_get_head_scan_for_repo_workspace_lookup_failure_raises(core, mock_sdk_with_responses): + """A failed listing is not the same as an empty one and must not resolve to None""" + mock_sdk_with_responses.fullscans.get.return_value = {} + + with pytest.raises(APIFailure): + core.get_head_scan_for_repo("test", workspace="customer-a") + + +def test_get_head_scan_for_repo_workspace_no_scans_yet(core, mock_sdk_with_responses): + """An empty listing is a real answer: the workspace has no baseline yet""" + mock_sdk_with_responses.fullscans.get.return_value = {"results": [], "nextPage": None} + + assert core.get_head_scan_for_repo("test", workspace="customer-a") is None + + +def test_get_head_scan_for_repo_skips_temporary_scans(core, mock_sdk_with_responses): + """A leftover empty tmp scan must not be picked up as the baseline""" + mock_sdk_with_responses.fullscans.get.return_value = { + "results": [ + {"id": "leftover-tmp-scan", "tmp": True}, + {"id": "workspace-head", "tmp": False}, + ], + "nextPage": None, + } + + assert core.get_head_scan_for_repo("test", workspace="customer-a") == "workspace-head" + + +def test_get_head_scan_for_repo_normalizes_enum_scan_type(core, mock_sdk_with_responses): + """ScanType members must be sent as their value, not their repr-style name""" + mock_sdk_with_responses.fullscans.get.return_value = { + "results": [{"id": "workspace-head"}], + "nextPage": None, + } + + core.get_head_scan_for_repo( + "test", + workspace="customer-a", + scan_type=ScanType.SOCKET_TIER1, + ) + + query_params = mock_sdk_with_responses.fullscans.get.call_args.args[1] + assert query_params["scan_type"] == "socket_tier1" + + def test_get_full_scan_id_by_commit(core, mock_sdk_with_responses): """Looks up the newest full scan for a repo + commit via the list endpoint""" mock_sdk_with_responses.fullscans.get.return_value = { @@ -109,7 +155,7 @@ def test_get_full_scan_id_by_commit(core, mock_sdk_with_responses): "commit_hash": "abc123", "sort": "created_at", "direction": "desc", - "per_page": 1, + "per_page": SCAN_LOOKUP_PAGE_SIZE, }, ) @@ -136,13 +182,26 @@ def test_get_full_scan_id_by_commit_scopes_to_workspace_and_scan_type(core, mock "commit_hash": "abc123", "sort": "created_at", "direction": "desc", - "per_page": 1, + "per_page": SCAN_LOOKUP_PAGE_SIZE, "workspace": "customer-a", "scan_type": "socket_tier1", }, ) +def test_get_full_scan_id_by_commit_skips_temporary_scans(core, mock_sdk_with_responses): + """A tmp scan carries the commit hash of the run that created it, so skip it too""" + mock_sdk_with_responses.fullscans.get.return_value = { + "results": [ + {"id": "leftover-tmp-scan", "commit_hash": "abc123", "tmp": True}, + {"id": "base-scan-id", "commit_hash": "abc123"}, + ], + "nextPage": None, + } + + assert core.get_full_scan_id_by_commit("test", "abc123") == "base-scan-id" + + def test_get_full_scan_id_by_commit_not_found(core, mock_sdk_with_responses): """No scan for the commit returns None (empty results and SDK error dict)""" mock_sdk_with_responses.fullscans.get.return_value = {"results": [], "nextPage": None} @@ -173,11 +232,46 @@ def test_resolve_base_full_scan_id_scopes_head_to_workspace(core): "branch": "main", "sort": "created_at", "direction": "desc", - "per_page": 1, + "per_page": SCAN_LOOKUP_PAGE_SIZE, "scan_type": "socket_tier1", }, ) +def test_resolve_base_full_scan_id_workspace_lookup_failure_exits(core): + """A failed workspace lookup fails the run instead of diffing against an empty scan""" + core.cli_config = make_cli_config() + core.sdk.fullscans.get.return_value = {} + + params = make_full_scan_params(workspace="customer-a") + + with pytest.raises(SystemExit) as exc_info: + core.resolve_base_full_scan_id(params) + assert exc_info.value.code == core.cli_config.exit_code_on_api_error + + +def test_resolve_base_full_scan_id_workspace_lookup_failure_disable_blocking(core): + """--disable-blocking keeps the failed lookup from failing the build""" + core.cli_config = make_cli_config("--disable-blocking") + core.sdk.fullscans.get.return_value = {} + + params = make_full_scan_params(workspace="customer-a") + + with pytest.raises(SystemExit) as exc_info: + core.resolve_base_full_scan_id(params) + assert exc_info.value.code == 0 + + +def test_resolve_base_full_scan_id_workspace_lookup_failure_without_cli_config(core): + """Library callers with no CliConfig see the APIFailure rather than a process exit""" + core.cli_config = None + core.sdk.fullscans.get.return_value = {} + + params = make_full_scan_params(workspace="customer-a") + + with pytest.raises(APIFailure): + core.resolve_base_full_scan_id(params) + + def test_resolve_base_full_scan_id_uses_base_scan_id(core): """--base-scan-id is used verbatim, without touching the repo endpoint""" core.cli_config = make_cli_config("--base-scan-id", "explicit-base") @@ -204,7 +298,7 @@ def test_resolve_base_full_scan_id_uses_base_commit_sha(core): "commit_hash": "abc123", "sort": "created_at", "direction": "desc", - "per_page": 1, + "per_page": SCAN_LOOKUP_PAGE_SIZE, "workspace": "customer-a", "scan_type": "socket_tier1", }, From de06e3bb97e8bcedf6a7e4ffcf953760e7d95237 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:07:09 -0400 Subject: [PATCH 04/14] fix(gitlab): match snake_case vulnerability ids in report identifiers Issue.props reaches the GitLab formatter from several sources, and core.alert_selection already matches both ghsaId/ghsa_id and cveId/cve_id when deciding reachability. The identifier extractor only read the camelCase spellings, so an alert carrying ghsa_id was selected for the report but emitted with only its socket_alert identifier -- the CVE and GHSA values GitLab dedupes and links on were dropped. Values that are neither a string nor a sequence are now skipped rather than iterated, so a malformed prop cannot raise out of the whole report. Co-Authored-By: Claude Opus 5 (1M context) --- socketsecurity/core/messages.py | 45 ++++++++++++++++++-------------- tests/unit/test_gitlab_format.py | 38 +++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 20 deletions(-) diff --git a/socketsecurity/core/messages.py b/socketsecurity/core/messages.py index 18f56fbf..d5774cce 100644 --- a/socketsecurity/core/messages.py +++ b/socketsecurity/core/messages.py @@ -657,30 +657,35 @@ def extract_identifiers_gitlab(alert: Issue) -> list: }) props = getattr(alert, "props", None) or {} + # Both spellings of each field are read because alerts reach Issue.props from + # several sources; core.alert_selection matches on the same pair. "cve" is the + # legacy property, kept for alerts produced by older API responses. identifier_fields = ( - ("cveId", "cve", "https://nvd.nist.gov/vuln/detail/"), - ("cve", "cve", "https://nvd.nist.gov/vuln/detail/"), - ("ghsaId", "ghsa", "https://github.com/advisories/"), + (("cveId", "cve_id", "cve"), "cve", "https://nvd.nist.gov/vuln/detail/"), + (("ghsaId", "ghsa_id"), "ghsa", "https://github.com/advisories/"), ) seen = set() - for field, identifier_type, url_prefix in identifier_fields: - values = props.get(field, []) - if isinstance(values, str): - values = [values] - for value in values or []: - if not isinstance(value, str) or not value.strip(): + for fields, identifier_type, url_prefix in identifier_fields: + for field in fields: + values = props.get(field) + if isinstance(values, str): + values = [values] + elif not isinstance(values, (list, tuple)): continue - value = value.strip() - identifier_key = (identifier_type, value.upper()) - if identifier_key in seen: - continue - seen.add(identifier_key) - identifiers.append({ - "type": identifier_type, - "name": value, - "value": value, - "url": f"{url_prefix}{value}" - }) + for value in values: + if not isinstance(value, str) or not value.strip(): + continue + value = value.strip() + identifier_key = (identifier_type, value.upper()) + if identifier_key in seen: + continue + seen.add(identifier_key) + identifiers.append({ + "type": identifier_type, + "name": value, + "value": value, + "url": f"{url_prefix}{value}" + }) return identifiers diff --git a/tests/unit/test_gitlab_format.py b/tests/unit/test_gitlab_format.py index a8126c70..7b064db9 100644 --- a/tests/unit/test_gitlab_format.py +++ b/tests/unit/test_gitlab_format.py @@ -151,6 +151,44 @@ def test_identifier_extraction_deduplicates_legacy_and_current_cve_fields(self): assert [item["value"] for item in identifiers].count("CVE-2024-1111") == 1 + def test_identifier_extraction_supports_snake_case_props(self): + """Alerts can reach Issue.props with snake_case vulnerability ids""" + issue = Issue( + pkg_name="vulnerable-pkg", + pkg_version="2.0.0", + type="vulnerability", + severity="high", + title="Snake case ids", + props={"cve_id": "CVE-2024-2222", "ghsa_id": "GHSA-2222-3333-4444"}, + pkg_type="npm", + key="test-key", + purl="pkg:npm/vulnerable-pkg@2.0.0", + ) + + identifiers = Messages.extract_identifiers_gitlab(issue) + + by_type = {item["type"]: item for item in identifiers} + assert by_type["cve"]["value"] == "CVE-2024-2222" + assert by_type["ghsa"]["value"] == "GHSA-2222-3333-4444" + + def test_identifier_extraction_ignores_unusable_prop_values(self): + """Malformed props must not take down the whole report""" + issue = Issue( + pkg_name="vulnerable-pkg", + pkg_version="2.0.0", + type="vulnerability", + severity="high", + title="Malformed props", + props={"cveId": 1234, "ghsaId": None}, + pkg_type="npm", + key="test-key", + purl="pkg:npm/vulnerable-pkg@2.0.0", + ) + + identifiers = Messages.extract_identifiers_gitlab(issue) + + assert [item["type"] for item in identifiers] == ["socket_alert"] + def test_dependency_chain_handling_transitive(self): """Test transitive dependency path is captured""" diff = Diff() From 162aa907982417d688a65102c495ed93edf9ac9b Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:07:09 -0400 Subject: [PATCH 05/14] test: use a generic package name in the namespace normalization fixture The fixture named a real organization. Public test data should not, so use the reserved com.example namespace instead. Co-Authored-By: Claude Opus 5 (1M context) --- tests/core/test_package_and_alerts.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/core/test_package_and_alerts.py b/tests/core/test_package_and_alerts.py index 4d1fa3b1..c9e6e115 100644 --- a/tests/core/test_package_and_alerts.py +++ b/tests/core/test_package_and_alerts.py @@ -107,10 +107,10 @@ def test_create_packages_dict_basic(self, core): def test_full_scan_package_normalizes_enum_type_and_namespace_url(self): artifact = SocketArtifact.from_dict({ - "id": "pkg:maven/com.arenko/trading-core@1.2.3", + "id": "pkg:maven/com.example/example-core@1.2.3", "type": "maven", - "namespace": "com.arenko", - "name": "trading-core", + "namespace": "com.example", + "name": "example-core", "version": "1.2.3", "direct": True, "topLevelAncestors": [], @@ -121,9 +121,9 @@ def test_full_scan_package_normalizes_enum_type_and_namespace_url(self): package = Package.from_socket_artifact(asdict(artifact)) assert package.type == "maven" - assert package.purl == "maven/com.arenko/trading-core@1.2.3" + assert package.purl == "maven/com.example/example-core@1.2.3" assert package.url == ( - "https://socket.dev/maven/package/com.arenko/trading-core/overview/1.2.3" + "https://socket.dev/maven/package/com.example/example-core/overview/1.2.3" ) def test_create_packages_dict_with_transitives(self, core): From 1abb63649c4967bea2d9bfa3be42189ca2968cbe Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Tue, 8 Sep 2026 16:39:23 -0400 Subject: [PATCH 06/14] fix(gitlab): use colon-separated Maven coordinates in package links Socket addresses Maven package pages as groupId:artifactId. The CLI emitted the slash-separated form, so every Maven package link 404'd -- the dashboard's Maven handler rejects the slash form outright with "Maven package must have a colon". Removing the enum leak from these URLs fixed how they looked without fixing where they pointed. The separator now follows the ecosystem, via Package.socket_url, which both the full-scan and diff construction paths call. Previously each built its URL inline and they disagreed on namespace handling, so the same package could produce different links depending on which path ran. Purl strings are deliberately left on the slash form for every ecosystem: that is what the purl spec defines and what Socket's purl API consumes. Only the dashboard URL is ecosystem-dependent. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 +++ socketsecurity/core/__init__.py | 7 ++-- socketsecurity/core/classes.py | 49 +++++++++++++++++++++++-- tests/core/test_package_and_alerts.py | 52 ++++++++++++++++++++++++++- 4 files changed, 106 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88f99d31..de8cdff2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Full-scan package identities and Socket links now preserve namespaced packages when the SDK returns enum-backed ecosystem values. +- Maven package links use the `groupId:artifactId` form the Socket dashboard + expects. The slash-separated form returned a 404 for every Maven package, on + both the full-scan and diff code paths. Purl strings are unchanged and keep the + slash form the purl spec defines. - GitLab dependency-scanning reports emit CVE and GHSA identifiers from current API fields while remaining compatible with legacy CVE data. - Implicit diff baselines are selected from the same workspace, scan type, diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index f12ff0eb..884f60bb 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -1632,12 +1632,13 @@ def resolve_base_full_scan_id(self, params: FullScanParams) -> Optional[str]: @staticmethod def update_package_values(pkg: Package) -> Package: + # The purl keeps the "/" form for every ecosystem; only the dashboard URL + # varies, so it is built by Package.socket_url rather than inline here. + pkg.type = Package.normalize_type(pkg.type) pkg.purl = f"{pkg.name}@{pkg.version}" - pkg.url = f"https://socket.dev/{pkg.type}/package" if pkg.namespace: pkg.purl = f"{pkg.namespace}/{pkg.purl}" - pkg.url += f"/{pkg.namespace}" - pkg.url += f"/{pkg.name}/overview/{pkg.version}" + pkg.url = Package.socket_url(pkg.type, pkg.namespace, pkg.name, pkg.version) return pkg def get_license_text_via_purl(self, packages: dict[str, Package], batch_size: int = 5000) -> dict: diff --git a/socketsecurity/core/classes.py b/socketsecurity/core/classes.py index 46d8ffc9..dd742a80 100644 --- a/socketsecurity/core/classes.py +++ b/socketsecurity/core/classes.py @@ -11,6 +11,14 @@ SocketScore, ) +# Separator between namespace and name in a socket.dev package URL. Socket addresses +# Maven artifacts as "groupId:artifactId" -- the slash form 404s, and the dashboard's +# Maven handler raises "Maven package must have a colon" on it. Every other ecosystem +# uses a path segment per component (npm "@scope/name", golang "github.com/org/repo"). +URL_NAMESPACE_SEPARATORS = { + "maven": ":", +} + __all__ = [ "Report", "Score", @@ -142,6 +150,43 @@ class Package(): licenseAttrib: Optional[List] = None + @staticmethod + def normalize_type(package_type) -> str: + """ + Unwraps the SDK's str-backed SocketPURL_Type enum to its value. + + str(SocketPURL_Type.MAVEN) is "SocketPURL_Type.MAVEN", not "maven", so any + enum member reaching an f-string leaks the class name into user-facing output. + """ + return getattr(package_type, "value", package_type) + + @staticmethod + def socket_url(package_type, namespace: Optional[str], name: str, version: str) -> str: + """ + Builds the socket.dev package overview URL for a package. + + Maven package pages are addressed as ``groupId:artifactId``; every other + ecosystem gives the namespace its own path segment. The slash form 404s for + Maven, so the separator has to follow the ecosystem. + + Purl strings keep the "/" form for both, which is what the purl spec and + Socket's purl API expect -- only the dashboard URL differs. + + Args: + package_type: Ecosystem, as a string or SocketPURL_Type member + namespace: Package namespace (Maven groupId, npm scope), if any + name: Package name + version: Package version + + Returns: + Package overview URL on socket.dev + """ + package_type = Package.normalize_type(package_type) + namespace = (namespace or "").strip("/") + separator = URL_NAMESPACE_SEPARATORS.get(package_type, "/") + package_path = f"{namespace}{separator}{name}" if namespace else name + return f"https://socket.dev/{package_type}/package/{package_path}/overview/{version}" + @classmethod def from_socket_artifact(cls, data: dict) -> "Package": """ @@ -153,11 +198,11 @@ def from_socket_artifact(cls, data: dict) -> "Package": Returns: New Package instance """ - package_type = getattr(data["type"], "value", data["type"]) + package_type = Package.normalize_type(data["type"]) namespace = (data.get("namespace") or "").strip("/") package_path = "/".join(part for part in (namespace, data["name"]) if part) purl = f"{package_type}/{package_path}@{data['version']}" - url = f"https://socket.dev/{package_type}/package/{package_path}/overview/{data['version']}" + url = Package.socket_url(package_type, namespace, data["name"], data["version"]) return cls( id=data["id"], name=data["name"], diff --git a/tests/core/test_package_and_alerts.py b/tests/core/test_package_and_alerts.py index c9e6e115..2c6a1424 100644 --- a/tests/core/test_package_and_alerts.py +++ b/tests/core/test_package_and_alerts.py @@ -123,7 +123,57 @@ def test_full_scan_package_normalizes_enum_type_and_namespace_url(self): assert package.type == "maven" assert package.purl == "maven/com.example/example-core@1.2.3" assert package.url == ( - "https://socket.dev/maven/package/com.example/example-core/overview/1.2.3" + "https://socket.dev/maven/package/com.example:example-core/overview/1.2.3" + ) + + def test_maven_package_url_uses_colon_between_group_and_artifact(self): + """Socket addresses Maven artifacts as groupId:artifactId; the slash form 404s""" + artifact = SocketArtifact.from_dict({ + "id": "pkg:maven/org.apache.logging.log4j/log4j-api@2.17.2", + "type": "maven", + "namespace": "org.apache.logging.log4j", + "name": "log4j-api", + "version": "2.17.2", + "direct": True, + "topLevelAncestors": [], + "manifestFiles": [{"file": "pom.xml"}], + "alerts": [], + }) + + package = Package.from_socket_artifact(asdict(artifact)) + + assert package.url == ( + "https://socket.dev/maven/package/org.apache.logging.log4j:log4j-api" + "/overview/2.17.2" + ) + # The purl keeps the "/" form, which is what the purl spec and the purl API want. + assert package.purl == "maven/org.apache.logging.log4j/log4j-api@2.17.2" + + def test_non_maven_package_url_keeps_slash_separator(self): + """npm scopes and Go module paths stay slash-delimited""" + scoped_npm = Package.socket_url("npm", "@babel", "core", "7.0.0") + assert scoped_npm == "https://socket.dev/npm/package/@babel/core/overview/7.0.0" + + unscoped = Package.socket_url("nuget", None, "newtonsoft.json", "6.0.8") + assert unscoped == "https://socket.dev/nuget/package/newtonsoft.json/overview/6.0.8" + + def test_diff_path_builds_the_same_maven_url_as_the_full_scan_path(self): + """Both package construction paths must agree, or links break on only some runs""" + package = Package( + id="pkg:maven/com.google.code.gson/gson@2.8.6", + type="maven", + name="gson", + version="2.8.6", + namespace="com.google.code.gson", + score={}, + alerts=[], + topLevelAncestors=[], + ) + + package = Core.update_package_values(package) + + assert package.url == ( + "https://socket.dev/maven/package/com.google.code.gson:gson/overview/2.8.6" ) def test_create_packages_dict_with_transitives(self, core): From c5fcbc8045651ffcdbf9448337b5b160ce5886ab Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:59:06 -0400 Subject: [PATCH 07/14] docs: tighten comments added by this branch The Maven separator rule was explained in three places and the enum-unwrapping rule in two. Each now has one home: the separator at URL_NAMESPACE_SEPARATORS where it is defined, the enum behavior at each helper that depends on it, stated once rather than narrated. Co-Authored-By: Claude Opus 5 (1M context) --- socketsecurity/core/__init__.py | 9 +++------ socketsecurity/core/classes.py | 8 ++------ socketsecurity/core/messages.py | 6 +++--- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index 884f60bb..6619110f 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -1492,10 +1492,9 @@ def query_param_value(value): """ Unwraps an enum member so it survives URL encoding. - The SDK types several params as str-backed enums (ScanType, IntegrationType). - urlencode calls str() on values, and a (str, Enum) mixin renders as - "ScanType.SOCKET_TIER1" rather than "socket_tier1", which would silently - filter on a scan type that does not exist. + The SDK types several query params as str-backed enums (ScanType, + IntegrationType). urlencode calls str(), which renders a (str, Enum) member + as "ScanType.SOCKET_TIER1" -- a filter value the API does not recognize. """ return getattr(value, "value", value) @@ -1632,8 +1631,6 @@ def resolve_base_full_scan_id(self, params: FullScanParams) -> Optional[str]: @staticmethod def update_package_values(pkg: Package) -> Package: - # The purl keeps the "/" form for every ecosystem; only the dashboard URL - # varies, so it is built by Package.socket_url rather than inline here. pkg.type = Package.normalize_type(pkg.type) pkg.purl = f"{pkg.name}@{pkg.version}" if pkg.namespace: diff --git a/socketsecurity/core/classes.py b/socketsecurity/core/classes.py index dd742a80..b07cddc2 100644 --- a/socketsecurity/core/classes.py +++ b/socketsecurity/core/classes.py @@ -165,12 +165,8 @@ def socket_url(package_type, namespace: Optional[str], name: str, version: str) """ Builds the socket.dev package overview URL for a package. - Maven package pages are addressed as ``groupId:artifactId``; every other - ecosystem gives the namespace its own path segment. The slash form 404s for - Maven, so the separator has to follow the ecosystem. - - Purl strings keep the "/" form for both, which is what the purl spec and - Socket's purl API expect -- only the dashboard URL differs. + The namespace separator is ecosystem-dependent; see URL_NAMESPACE_SEPARATORS. + Purl strings are not, and keep the "/" form everywhere. Args: package_type: Ecosystem, as a string or SocketPURL_Type member diff --git a/socketsecurity/core/messages.py b/socketsecurity/core/messages.py index d5774cce..eec22faa 100644 --- a/socketsecurity/core/messages.py +++ b/socketsecurity/core/messages.py @@ -657,9 +657,9 @@ def extract_identifiers_gitlab(alert: Issue) -> list: }) props = getattr(alert, "props", None) or {} - # Both spellings of each field are read because alerts reach Issue.props from - # several sources; core.alert_selection matches on the same pair. "cve" is the - # legacy property, kept for alerts produced by older API responses. + # Alerts reach Issue.props from several sources, so both spellings of each + # field are in play; core.alert_selection matches on the same pair. "cve" is + # the older spelling and still appears in some payloads. identifier_fields = ( (("cveId", "cve_id", "cve"), "cve", "https://nvd.nist.gov/vuln/detail/"), (("ghsaId", "ghsa_id"), "ghsa", "https://github.com/advisories/"), From ff7ae5a11f6af0263333249cc53b78b924887c65 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:03:58 -0400 Subject: [PATCH 08/14] fix(gitlab): warn when a Maven coordinate has no namespace An ecosystem with its own URL separator cannot be addressed without both halves of the coordinate. A Maven artifact that arrives with no groupId still gets a link so the finding reports, but that link cannot resolve, and previously it was emitted silently. It now logs a warning naming the package. Co-Authored-By: Claude Opus 5 (1M context) --- socketsecurity/core/classes.py | 12 ++++++++++++ tests/core/test_package_and_alerts.py | 16 ++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/socketsecurity/core/classes.py b/socketsecurity/core/classes.py index b07cddc2..73af8f9a 100644 --- a/socketsecurity/core/classes.py +++ b/socketsecurity/core/classes.py @@ -1,4 +1,5 @@ import json +import logging from dataclasses import dataclass, field from typing import Dict, List, Optional, TypedDict @@ -11,6 +12,8 @@ SocketScore, ) +log = logging.getLogger("socketdev") + # Separator between namespace and name in a socket.dev package URL. Socket addresses # Maven artifacts as "groupId:artifactId" -- the slash form 404s, and the dashboard's # Maven handler raises "Maven package must have a colon" on it. Every other ecosystem @@ -180,6 +183,15 @@ def socket_url(package_type, namespace: Optional[str], name: str, version: str) package_type = Package.normalize_type(package_type) namespace = (namespace or "").strip("/") separator = URL_NAMESPACE_SEPARATORS.get(package_type, "/") + if separator != "/" and not namespace: + # An ecosystem with its own separator cannot be addressed without the + # namespace half of the coordinate. The link is emitted anyway so the + # finding still reports, but it will not resolve. + log.warning( + f"{package_type} package {name}@{version} has no namespace, so its " + f"Socket link cannot use the '{separator}' separator the dashboard " + "requires and will not resolve" + ) package_path = f"{namespace}{separator}{name}" if namespace else name return f"https://socket.dev/{package_type}/package/{package_path}/overview/{version}" diff --git a/tests/core/test_package_and_alerts.py b/tests/core/test_package_and_alerts.py index 2c6a1424..4f816b44 100644 --- a/tests/core/test_package_and_alerts.py +++ b/tests/core/test_package_and_alerts.py @@ -157,6 +157,22 @@ def test_non_maven_package_url_keeps_slash_separator(self): unscoped = Package.socket_url("nuget", None, "newtonsoft.json", "6.0.8") assert unscoped == "https://socket.dev/nuget/package/newtonsoft.json/overview/6.0.8" + def test_maven_package_without_namespace_warns(self, caplog): + """A Maven coordinate missing its groupId cannot produce a resolvable link""" + with caplog.at_level("WARNING", logger="socketdev"): + url = Package.socket_url("maven", None, "orphan-artifact", "1.0.0") + + assert url == "https://socket.dev/maven/package/orphan-artifact/overview/1.0.0" + assert "orphan-artifact@1.0.0" in caplog.text + assert "no namespace" in caplog.text + + def test_namespaced_maven_package_does_not_warn(self, caplog): + """The warning is for missing data, not for every Maven package""" + with caplog.at_level("WARNING", logger="socketdev"): + Package.socket_url("maven", "com.example", "artifact", "1.0.0") + + assert caplog.text == "" + def test_diff_path_builds_the_same_maven_url_as_the_full_scan_path(self): """Both package construction paths must agree, or links break on only some runs""" package = Package( From 212c82518b2463165c7c86ee8dfea9ebe570bebb Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:18:23 -0400 Subject: [PATCH 09/14] fix(gitlab): separate namespace and name with a slash, not a colon Reverts the separator introduced two commits ago. It rested on a report that the slash form does not resolve, which has since failed to reproduce: every affected link in that report loads, and the report's own screenshots show a working slash-form link. The defect those links actually exhibit is a namespace and name fused with no separator at all, which yields one path segment that cannot be split back into two. A slash fixes that and matches what the other package construction path has always emitted. The missing-namespace warning is kept and re-aimed: an absent namespace is what produces the unsplittable single segment, so that is the case worth surfacing. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 8 ++++---- socketsecurity/core/classes.py | 26 +++++++++----------------- tests/core/test_package_and_alerts.py | 10 +++++----- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de8cdff2..8a335537 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,10 @@ - Full-scan package identities and Socket links now preserve namespaced packages when the SDK returns enum-backed ecosystem values. -- Maven package links use the `groupId:artifactId` form the Socket dashboard - expects. The slash-separated form returned a 404 for every Maven package, on - both the full-scan and diff code paths. Purl strings are unchanged and keep the - slash form the purl spec defines. +- Namespaced package links separate the namespace from the name instead of + concatenating them, so Maven links no longer fuse groupId and artifactId into a + single unresolvable path segment. A namespaced package whose namespace is + missing now logs a warning rather than emitting a broken link silently. - GitLab dependency-scanning reports emit CVE and GHSA identifiers from current API fields while remaining compatible with legacy CVE data. - Implicit diff baselines are selected from the same workspace, scan type, diff --git a/socketsecurity/core/classes.py b/socketsecurity/core/classes.py index 73af8f9a..87e76c18 100644 --- a/socketsecurity/core/classes.py +++ b/socketsecurity/core/classes.py @@ -14,13 +14,10 @@ log = logging.getLogger("socketdev") -# Separator between namespace and name in a socket.dev package URL. Socket addresses -# Maven artifacts as "groupId:artifactId" -- the slash form 404s, and the dashboard's -# Maven handler raises "Maven package must have a colon" on it. Every other ecosystem -# uses a path segment per component (npm "@scope/name", golang "github.com/org/repo"). -URL_NAMESPACE_SEPARATORS = { - "maven": ":", -} +# Ecosystems whose package pages cannot be addressed by name alone. A Maven +# coordinate is a groupId plus an artifactId; with no namespace the URL collapses to +# one path segment that cannot be split back into two, and the page does not resolve. +NAMESPACE_REQUIRED_TYPES = frozenset({"maven"}) __all__ = [ "Report", @@ -168,8 +165,7 @@ def socket_url(package_type, namespace: Optional[str], name: str, version: str) """ Builds the socket.dev package overview URL for a package. - The namespace separator is ecosystem-dependent; see URL_NAMESPACE_SEPARATORS. - Purl strings are not, and keep the "/" form everywhere. + Namespace and name are separate path segments, the same form purl strings use. Args: package_type: Ecosystem, as a string or SocketPURL_Type member @@ -182,17 +178,13 @@ def socket_url(package_type, namespace: Optional[str], name: str, version: str) """ package_type = Package.normalize_type(package_type) namespace = (namespace or "").strip("/") - separator = URL_NAMESPACE_SEPARATORS.get(package_type, "/") - if separator != "/" and not namespace: - # An ecosystem with its own separator cannot be addressed without the - # namespace half of the coordinate. The link is emitted anyway so the - # finding still reports, but it will not resolve. + if not namespace and package_type in NAMESPACE_REQUIRED_TYPES: + # The link is still emitted so the finding reports, but it cannot resolve. log.warning( f"{package_type} package {name}@{version} has no namespace, so its " - f"Socket link cannot use the '{separator}' separator the dashboard " - "requires and will not resolve" + "Socket link collapses to a single path segment and will not resolve" ) - package_path = f"{namespace}{separator}{name}" if namespace else name + package_path = "/".join(part for part in (namespace, name) if part) return f"https://socket.dev/{package_type}/package/{package_path}/overview/{version}" @classmethod diff --git a/tests/core/test_package_and_alerts.py b/tests/core/test_package_and_alerts.py index 4f816b44..0e096621 100644 --- a/tests/core/test_package_and_alerts.py +++ b/tests/core/test_package_and_alerts.py @@ -123,11 +123,11 @@ def test_full_scan_package_normalizes_enum_type_and_namespace_url(self): assert package.type == "maven" assert package.purl == "maven/com.example/example-core@1.2.3" assert package.url == ( - "https://socket.dev/maven/package/com.example:example-core/overview/1.2.3" + "https://socket.dev/maven/package/com.example/example-core/overview/1.2.3" ) - def test_maven_package_url_uses_colon_between_group_and_artifact(self): - """Socket addresses Maven artifacts as groupId:artifactId; the slash form 404s""" + def test_maven_package_url_separates_group_and_artifact(self): + """groupId and artifactId are distinct path segments, not one fused string""" artifact = SocketArtifact.from_dict({ "id": "pkg:maven/org.apache.logging.log4j/log4j-api@2.17.2", "type": "maven", @@ -143,7 +143,7 @@ def test_maven_package_url_uses_colon_between_group_and_artifact(self): package = Package.from_socket_artifact(asdict(artifact)) assert package.url == ( - "https://socket.dev/maven/package/org.apache.logging.log4j:log4j-api" + "https://socket.dev/maven/package/org.apache.logging.log4j/log4j-api" "/overview/2.17.2" ) # The purl keeps the "/" form, which is what the purl spec and the purl API want. @@ -189,7 +189,7 @@ def test_diff_path_builds_the_same_maven_url_as_the_full_scan_path(self): package = Core.update_package_values(package) assert package.url == ( - "https://socket.dev/maven/package/com.google.code.gson:gson/overview/2.8.6" + "https://socket.dev/maven/package/com.google.code.gson/gson/overview/2.8.6" ) def test_create_packages_dict_with_transitives(self, core): From 860ce6f87cb740af2059f39508f7fca4fea2f085 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:31:03 -0400 Subject: [PATCH 10/14] chore: bump version to 2.8.2 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 2 +- pyproject.toml | 2 +- socketsecurity/__init__.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a335537..da1f9cf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 2.8.1 +## 2.8.2 ### Fixed: GitLab report serialization and workspace baselines diff --git a/pyproject.toml b/pyproject.toml index ec8f544d..bcb1195c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" [project] name = "socketsecurity" -version = "2.8.1" +version = "2.8.2" requires-python = ">= 3.11" license = {"file" = "LICENSE"} dependencies = [ diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index 6cf31cd7..29fd46e0 100644 --- a/socketsecurity/__init__.py +++ b/socketsecurity/__init__.py @@ -1,3 +1,3 @@ __author__ = 'socket.dev' -__version__ = '2.8.1' +__version__ = '2.8.2' USER_AGENT = f'SocketPythonCLI/{__version__}' From 52fd1dcfb164c1b55d9bccb4551f10eed5c05f64 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:53:32 -0400 Subject: [PATCH 11/14] fix(gitlab): report a real manifest and real directness in report locations Two defects in the same location block. The manifest path fell back to "unknown" whenever a package had no introducing chain. That happens routinely for a transitive package whose top-level ancestors are absent from the scan's package set, which a diff-scoped run causes by construction. The package records its own manifest files regardless, so those are now used before giving up. Directness was inferred by looking for " > " in the introducing entry, but no producer emits that separator -- get_source_data yields either ("direct", files) or (ancestor_purl, files). Every finding was therefore reported as direct, including transitive ones. It now comes from the package record. The dependency chain was also parsed into a local that was never read, and the docstring advertised a dependency_path key the function never returned. Both are removed rather than wired up, since the GitLab schema expects dependency references rather than a name path. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 +++ socketsecurity/core/__init__.py | 2 ++ socketsecurity/core/classes.py | 9 +++++++ socketsecurity/core/messages.py | 44 +++++++++++++++----------------- tests/unit/test_gitlab_format.py | 41 ++++++++++++++++++++++++++++- 5 files changed, 76 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 326591dd..9d3f6da3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ missing now logs a warning rather than emitting a broken link silently. - GitLab dependency-scanning reports emit CVE and GHSA identifiers from current API fields while remaining compatible with legacy CVE data. +- GitLab report findings record the manifest they came from when the package's + introducing chain is unavailable, instead of reporting the location as + `unknown`, and report whether a dependency is direct from the package record + rather than inferring it from a dependency-path string that is never produced. - Implicit diff baselines are selected from the same workspace, scan type, repository, and default branch. A baseline lookup that fails is reported as an API error instead of resolving to an empty baseline, and temporary scans are diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index b57d5631..65fbabc6 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -2503,6 +2503,8 @@ def add_package_alerts_to_collection(self, package: Package, alerts_collection: suggestion=props.suggestion, next_step_title=props.nextStepTitle, introduced_by=introduced_by, + manifest_files=package.manifestFiles or [], + direct=bool(package.direct), purl=package.purl, url=package.url ) diff --git a/socketsecurity/core/classes.py b/socketsecurity/core/classes.py index 87e76c18..2a19c2bb 100644 --- a/socketsecurity/core/classes.py +++ b/socketsecurity/core/classes.py @@ -317,6 +317,11 @@ class Issue: manifests: str url: str purl: str + # The package's own manifest files, independent of how it was introduced. A + # transitive package whose ancestors are absent from the scan has no + # introduced_by chain, but its manifest is still known. + manifest_files: list + direct: bool def __init__(self, **kwargs): if kwargs: @@ -325,6 +330,10 @@ def __init__(self, **kwargs): if hasattr(self, "created_at"): self.created_at = self.created_at.strip(" (Coordinated Universal Time)") + if not hasattr(self, "manifest_files"): + self.manifest_files = [] + if not hasattr(self, "direct"): + self.direct = False if not hasattr(self, "manifests"): self.manifests = "" if not hasattr(self, "suggestion"): diff --git a/socketsecurity/core/messages.py b/socketsecurity/core/messages.py index eec22faa..91fa7e55 100644 --- a/socketsecurity/core/messages.py +++ b/socketsecurity/core/messages.py @@ -697,37 +697,35 @@ def extract_location_gitlab(alert: Issue) -> dict: GitLab location requires: - file: path to manifest file - dependency: package name and version - - dependency_path (optional): dependency chain """ - # Get manifest file from introduced_by or manifests attribute - manifest_file = "unknown" - dependency_path = [] - is_direct = True - - if hasattr(alert, 'introduced_by') and alert.introduced_by: - if isinstance(alert.introduced_by, list) and len(alert.introduced_by) > 0: - first_entry = alert.introduced_by[0] - if isinstance(first_entry, (list, tuple)) and len(first_entry) >= 2: - dependency_path_str = first_entry[0] - manifest_file = first_entry[1].split(';')[0] if ';' in first_entry[1] else first_entry[1] - - # Parse dependency path - if ' > ' in dependency_path_str: - dependency_path = dependency_path_str.split(' > ') - # If there's a chain, it's transitive (not direct) - is_direct = len(dependency_path) <= 1 - - elif hasattr(alert, 'manifests') and alert.manifests: - manifest_file = alert.manifests.split(';')[0] + manifest_file = "" + + introduced_by = getattr(alert, "introduced_by", None) + if isinstance(introduced_by, list) and introduced_by: + first_entry = introduced_by[0] + if isinstance(first_entry, (list, tuple)) and len(first_entry) >= 2: + manifest_file = (first_entry[1] or "").split(";")[0] + + if not manifest_file: + manifest_file = (getattr(alert, "manifests", "") or "").split(";")[0] + + if not manifest_file: + # A transitive package whose ancestors are not in this scan has no + # introduced_by chain, but the package still records its own manifest. + for entry in getattr(alert, "manifest_files", None) or []: + candidate = entry.get("file") if isinstance(entry, dict) else None + if candidate: + manifest_file = candidate + break location = { - "file": manifest_file, + "file": manifest_file or "unknown", "dependency": { "package": { "name": alert.pkg_name }, "version": alert.pkg_version, - "direct": is_direct + "direct": bool(getattr(alert, "direct", False)) } } diff --git a/tests/unit/test_gitlab_format.py b/tests/unit/test_gitlab_format.py index 7b064db9..92cacb62 100644 --- a/tests/unit/test_gitlab_format.py +++ b/tests/unit/test_gitlab_format.py @@ -190,7 +190,7 @@ def test_identifier_extraction_ignores_unusable_prop_values(self): assert [item["type"] for item in identifiers] == ["socket_alert"] def test_dependency_chain_handling_transitive(self): - """Test transitive dependency path is captured""" + """Directness comes from the package record, not from parsing a path string""" diff = Diff() diff.id = "test-scan-id" diff.diff_url = "https://socket.dev/test" @@ -204,6 +204,7 @@ def test_dependency_chain_handling_transitive(self): introduced_by=[ ["top-level > intermediate > transitive-dep", "package.json"] ], + direct=False, pkg_type="npm", key="test-key", purl="pkg:npm/transitive-dep@1.5.0" @@ -231,6 +232,7 @@ def test_dependency_chain_handling_direct(self): introduced_by=[ ["direct-dep", "package.json"] ], + direct=True, pkg_type="npm", key="test-key", purl="pkg:npm/direct-dep@3.0.0" @@ -242,6 +244,43 @@ def test_dependency_chain_handling_direct(self): assert vuln["location"]["dependency"]["direct"] is True + def test_location_file_falls_back_to_the_package_manifest(self): + """A package with no introduced_by chain still knows its own manifest""" + issue = Issue( + pkg_name="transitive-dep", + pkg_version="1.5.0", + type="malware", + severity="critical", + title="Malware Found", + introduced_by=[], + manifest_files=[{"file": "services/api/pom.xml"}], + direct=False, + pkg_type="maven", + key="test-key", + purl="pkg:maven/org.example/transitive-dep@1.5.0", + ) + + location = Messages.extract_location_gitlab(issue) + + assert location["file"] == "services/api/pom.xml" + assert location["dependency"]["direct"] is False + + def test_location_file_is_unknown_only_when_nothing_is_known(self): + """The unknown placeholder is a last resort, not the first answer""" + issue = Issue( + pkg_name="orphan", + pkg_version="1.0.0", + type="malware", + severity="critical", + title="Malware Found", + introduced_by=[], + pkg_type="npm", + key="test-key", + purl="pkg:npm/orphan@1.0.0", + ) + + assert Messages.extract_location_gitlab(issue)["file"] == "unknown" + def test_severity_mapping(self): """Test all Socket severities map to GitLab severities""" severity_tests = [ From 9bb07006991ee53c69d937d1ea640dc194459c73 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:59:46 -0400 Subject: [PATCH 12/14] fix(gitlab): omit an absent identifier url instead of sending null The GitLab dependency-scanning schema types an identifier's url as a string matching ^(https?|ftp)://, so a null fails validation. The socket_alert identifier emitted null whenever an alert carried no url, which invalidates that finding for every consumer that validates the report. Verified against the published schema: a report containing an alert with no url now produces zero validation errors. Co-Authored-By: Claude Opus 5 (1M context) --- socketsecurity/core/messages.py | 13 +++++++++---- tests/unit/test_gitlab_format.py | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/socketsecurity/core/messages.py b/socketsecurity/core/messages.py index 91fa7e55..14a6f829 100644 --- a/socketsecurity/core/messages.py +++ b/socketsecurity/core/messages.py @@ -648,13 +648,18 @@ def extract_identifiers_gitlab(alert: Issue) -> list: """ identifiers = [] - # Primary identifier: Socket alert type - identifiers.append({ + # Primary identifier: Socket alert type. The GitLab schema types identifier + # url as a string matching ^(https?|ftp)://, so an absent url is omitted + # rather than sent as null, which fails validation for the whole finding. + socket_identifier = { "type": "socket_alert", "name": f"Socket {alert.type}", "value": alert.type, - "url": alert.url if hasattr(alert, 'url') and alert.url else None - }) + } + alert_url = getattr(alert, "url", None) + if alert_url: + socket_identifier["url"] = alert_url + identifiers.append(socket_identifier) props = getattr(alert, "props", None) or {} # Alerts reach Issue.props from several sources, so both spellings of each diff --git a/tests/unit/test_gitlab_format.py b/tests/unit/test_gitlab_format.py index 92cacb62..b817e341 100644 --- a/tests/unit/test_gitlab_format.py +++ b/tests/unit/test_gitlab_format.py @@ -281,6 +281,25 @@ def test_location_file_is_unknown_only_when_nothing_is_known(self): assert Messages.extract_location_gitlab(issue)["file"] == "unknown" + def test_identifier_url_is_omitted_rather_than_null(self): + """GitLab types identifier url as a string; null fails schema validation""" + issue = Issue( + pkg_name="nourl-pkg", + pkg_version="1.0.0", + type="malware", + severity="critical", + title="Malware", + pkg_type="npm", + key="test-key", + purl="pkg:npm/nourl-pkg@1.0.0", + ) + + identifiers = Messages.extract_identifiers_gitlab(issue) + + # An absent key is correct; a present-but-null value is what breaks validation. + assert all("url" not in i or i["url"] for i in identifiers) + assert "url" not in identifiers[0] + def test_severity_mapping(self): """Test all Socket severities map to GitLab severities""" severity_tests = [ From f69bf4eec734764d8b1d5ba16d6ba2364c3779f8 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Sat, 12 Sep 2026 01:02:46 -0400 Subject: [PATCH 13/14] feat(gitlab): fall back to the nearest scanned ancestor for --base-commit-sha A merge base can have no full scan even when default-branch scanning is configured and running: squash merges and rebases rewrite commits, and a multi-commit push produces one scan for the tip while leaving the commits in between unscanned. Any of those turned every open merge request into a failed pipeline, because a missing baseline was a hard stop with no degraded mode. The requested commit is still preferred. When it has no scan, one listing of recent scans is matched against local first-parent history and the nearest scanned ancestor is used instead, logged at warning with the commit chosen and its distance. Only an unreachable ancestor now fails the run. Both bounds are fixed and neither costs an extra request: the listing is fetched once, and the walk stops at a set depth. Following first parents keeps a merge commit from contributing everything merged into it, and a shallow checkout simply narrows the search rather than breaking it. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 5 ++ socketsecurity/core/__init__.py | 142 +++++++++++++++++++++++++++++--- tests/core/test_sdk_methods.py | 48 +++++++++++ 3 files changed, 185 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d3f6da3..1f047dad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ introducing chain is unavailable, instead of reporting the location as `unknown`, and report whether a dependency is direct from the package record rather than inferring it from a dependency-path string that is never produced. +- `--base-commit-sha` degrades to the nearest scanned ancestor of the requested + commit instead of failing the run, and logs which commit was used and how far + back it is. Squash merges, rebases, and multi-commit pushes all leave a merge + base unscanned even when default-branch scanning is configured correctly. The + run still fails when no scanned ancestor is reachable. - Implicit diff baselines are selected from the same workspace, scan type, repository, and default branch. A baseline lookup that fails is reported as an API error instead of resolving to an empty baseline, and temporary scans are diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index 65fbabc6..34f63f78 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -18,6 +18,7 @@ if TYPE_CHECKING: from socketsecurity.config import CliConfig +from git import Repo from socketdev import socketdev from socketdev.exceptions import APIFailure from socketdev.fullscans import DiffArtifacts, FullScanParams, SocketArtifact @@ -56,6 +57,12 @@ # Core.newest_persisted_scan_id), so a single result is not enough. SCAN_LOOKUP_PAGE_SIZE = 10 +# Bounds on the search for a scanned ancestor when the requested baseline commit has +# no full scan of its own. The scan listing is fetched once and matched against local +# history, so neither bound costs an extra request. +ANCESTOR_SCAN_LOOKUP_LIMIT = 100 +ANCESTOR_WALK_MAX_DEPTH = 100 + # Reachability facts-file upload compression. # # The Socket full-scan endpoint transparently brotli-decompresses any multipart part @@ -1555,6 +1562,102 @@ def newest_persisted_scan_id(results: List[dict]) -> Optional[str]: return scan_id return None + def first_parent_commits(self, start_commit_sha: str, max_count: int) -> List[str]: + """ + Lists a commit and its first-parent ancestors, newest first. + + Follows only first parents so a merge commit contributes the branch's own + history rather than everything merged into it. A shallow checkout simply + yields fewer commits, which narrows the search rather than failing it. + + Args: + start_commit_sha: Commit to walk back from, included in the result + max_count: Maximum number of commits to return + + Returns: + Commit SHAs, newest first. Empty when the repository or commit is + unavailable locally. + """ + target_path = self.cli_config.target_path if self.cli_config else None + if not target_path: + return [] + try: + repo = Repo(target_path) + output = repo.git.rev_list( + "--first-parent", + f"--max-count={max_count}", + start_commit_sha, + ) + except Exception as error: + log.debug(f"Unable to walk history back from {start_commit_sha}: {error}") + return [] + return [line.strip() for line in output.splitlines() if line.strip()] + + def find_baseline_scan_for_ancestor( + self, + repo_slug: str, + commit_sha: str, + workspace: Optional[str] = None, + scan_type: Optional[str] = None, + ) -> Tuple[Optional[str], Optional[str], int]: + """ + Finds the nearest ancestor of a commit that does have a full scan. + + Used when --base-commit-sha names a commit that was never scanned. Squash + merges and rebases rewrite commits, and a multi-commit push produces one scan + for the tip, so a merge base can be unscanned even when default-branch + scanning is configured correctly. Diffing against a slightly older ancestor + is a wider diff; failing outright is no diff at all. + + One scan listing is fetched and matched against local first-parent history, + so the walk costs no additional requests. + + Args: + repo_slug: Repository slug the scan belongs to + commit_sha: Commit that has no full scan of its own + workspace: Socket workspace the scan belongs to, if any + scan_type: Socket scan type to match, if any + + Returns: + (scan_id, ancestor_commit_sha, commits_back), or (None, None, 0) when no + scanned ancestor is reachable. + """ + query_params = { + "repo": repo_slug, + "sort": "created_at", + "direction": "desc", + "per_page": ANCESTOR_SCAN_LOOKUP_LIMIT, + } + if workspace: + query_params["workspace"] = workspace + if scan_type: + query_params["scan_type"] = Core.query_param_value(scan_type) + + response = self.sdk.fullscans.get(self.config.org_slug, query_params) + results = response.get("results") if isinstance(response, dict) else None + if not results: + return None, None, 0 + + scans_by_commit = {} + for result in results: + if not isinstance(result, dict) or result.get("tmp"): + continue + result_commit = result.get("commit_hash") + scan_id = result.get("id") + # Newest first, so the first scan seen for a commit is the one to keep. + if result_commit and scan_id and result_commit not in scans_by_commit: + scans_by_commit[result_commit] = scan_id + + if not scans_by_commit: + return None, None, 0 + + ancestors = self.first_parent_commits(commit_sha, ANCESTOR_WALK_MAX_DEPTH) + for distance, ancestor in enumerate(ancestors): + scan_id = scans_by_commit.get(ancestor) + if scan_id: + return scan_id, ancestor, distance + return None, None, 0 + def get_full_scan_id_by_commit( self, repo_slug: str, @@ -1628,19 +1731,38 @@ def resolve_base_full_scan_id(self, params: FullScanParams) -> Optional[str]: workspace=params.workspace, scan_type=params.scan_type, ) + baseline_source = "explicit-commit" + baseline_commit = commit_sha if scan_id is None: - log.error( - f"No full scan found for commit {commit_sha} in repo {params.repo} " - "(--base-commit-sha). Ensure a scan was created for that commit " - "(e.g. the CLI runs on default-branch pushes), or pass " - "--base-scan-id instead." + scan_id, ancestor_sha, commits_back = self.find_baseline_scan_for_ancestor( + params.repo, + commit_sha, + workspace=params.workspace, + scan_type=params.scan_type, ) - if self.cli_config.disable_blocking: - sys.exit(0) - sys.exit(self.cli_config.exit_code_on_api_error) + if scan_id: + baseline_source = "explicit-commit-ancestor" + baseline_commit = ancestor_sha + log.warning( + f"No full scan for commit {commit_sha} (--base-commit-sha). " + f"Diffing against its nearest scanned ancestor {ancestor_sha}, " + f"{commits_back} commit(s) earlier, so the diff is wider than " + "the merge base." + ) + else: + log.error( + f"No full scan found for commit {commit_sha} in repo {params.repo} " + "(--base-commit-sha), and no scanned ancestor within " + f"{ANCESTOR_WALK_MAX_DEPTH} commits of it. Ensure a scan was " + "created for that commit (e.g. the CLI runs on default-branch " + "pushes), or pass --base-scan-id instead." + ) + if self.cli_config.disable_blocking: + sys.exit(0) + sys.exit(self.cli_config.exit_code_on_api_error) log.info( - "Baseline selected: source=explicit-commit " - f"scan_id={json.dumps(scan_id)} commit={json.dumps(commit_sha)}" + f"Baseline selected: source={baseline_source} " + f"scan_id={json.dumps(scan_id)} commit={json.dumps(baseline_commit)}" ) return scan_id diff --git a/tests/core/test_sdk_methods.py b/tests/core/test_sdk_methods.py index b2ffb354..92b9a17b 100644 --- a/tests/core/test_sdk_methods.py +++ b/tests/core/test_sdk_methods.py @@ -304,6 +304,54 @@ def test_resolve_base_full_scan_id_uses_base_commit_sha(core): }, ) +def test_resolve_base_full_scan_id_falls_back_to_scanned_ancestor(core, monkeypatch): + """An unscanned merge base degrades to the nearest scanned ancestor""" + core.cli_config = make_cli_config("--base-commit-sha", "unscanned-sha") + core.sdk.fullscans.get.side_effect = [ + {"results": [], "nextPage": None}, # exact commit + {"results": [ # recent scans + {"id": "tmp-scan", "commit_hash": "ancestor-1", "tmp": True}, + {"id": "ancestor-scan", "commit_hash": "ancestor-2"}, + ], "nextPage": None}, + ] + monkeypatch.setattr( + Core, "first_parent_commits", + lambda self, sha, depth: ["unscanned-sha", "ancestor-1", "ancestor-2"], + ) + + params = make_full_scan_params() + assert core.resolve_base_full_scan_id(params) == "ancestor-scan" + + +def test_resolve_base_full_scan_id_ancestor_fallback_skips_temporary_scans(core, monkeypatch): + """A tmp scan on an ancestor is not a usable baseline either""" + core.cli_config = make_cli_config("--base-commit-sha", "unscanned-sha") + core.sdk.fullscans.get.side_effect = [ + {"results": [], "nextPage": None}, + {"results": [{"id": "tmp-scan", "commit_hash": "ancestor-1", "tmp": True}], "nextPage": None}, + ] + monkeypatch.setattr( + Core, "first_parent_commits", + lambda self, sha, depth: ["unscanned-sha", "ancestor-1"], + ) + + with pytest.raises(SystemExit): + core.resolve_base_full_scan_id(make_full_scan_params()) + + +def test_resolve_base_full_scan_id_ancestor_fallback_needs_local_history(core, monkeypatch): + """Without local history there is nothing to match scans against""" + core.cli_config = make_cli_config("--base-commit-sha", "unscanned-sha") + core.sdk.fullscans.get.side_effect = [ + {"results": [], "nextPage": None}, + {"results": [{"id": "ancestor-scan", "commit_hash": "ancestor-2"}], "nextPage": None}, + ] + monkeypatch.setattr(Core, "first_parent_commits", lambda self, sha, depth: []) + + with pytest.raises(SystemExit): + core.resolve_base_full_scan_id(make_full_scan_params()) + + def test_resolve_base_full_scan_id_commit_sha_not_found_exits(core): """A --base-commit-sha with no scan is a hard error (exit_code_on_api_error)""" core.cli_config = make_cli_config("--base-commit-sha", "abc123") From 26f32d2179934248afd79e3ea7dbe59c6952673a Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Sat, 12 Sep 2026 02:02:04 -0400 Subject: [PATCH 14/14] fix: harden diff baseline resolution --- CHANGELOG.md | 9 +- README.md | 17 ++-- docs/ci-cd.md | 22 +++-- docs/cli-reference.md | 16 ++-- socketsecurity/config.py | 6 +- socketsecurity/core/__init__.py | 145 ++++++++++++++++++++------------ tests/core/test_sdk_methods.py | 106 +++++++++++++++++++++-- 7 files changed, 228 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f047dad..8716b1c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,11 +20,12 @@ commit instead of failing the run, and logs which commit was used and how far back it is. Squash merges, rebases, and multi-commit pushes all leave a merge base unscanned even when default-branch scanning is configured correctly. The - run still fails when no scanned ancestor is reachable. + lookup follows paginated scan history and the run still fails when no scanned + ancestor is reachable or the exact-commit lookup itself fails. - Implicit diff baselines are selected from the same workspace, scan type, - repository, and default branch. A baseline lookup that fails is reported as an - API error instead of resolving to an empty baseline, and temporary scans are - skipped when selecting one. + repository, and default branch, including when no workspace is supplied. A + baseline lookup that fails is reported as an API error instead of resolving to + an empty baseline, and temporary scans are skipped when selecting one. ## 2.8.1 ### Changed: bump pinned @coana-tech/cli to 15.10.40 diff --git a/README.md b/README.md index 5f978d59..5221daf1 100644 --- a/README.md +++ b/README.md @@ -44,21 +44,22 @@ socketcli --enable-gitlab-security --gitlab-security-file gl-dependency-scanning ### PR scan diffed against the merge base -By default, PR scans are diffed against the repository's latest head scan. To diff against -the exact commit your PR branched from instead, pass the merge base as the baseline: +By default, PR scans are diffed against the repository's latest matching head scan. To +prefer the commit your PR branched from as the baseline, pass the merge base: ```bash BASE_SHA=$(git merge-base origin/main HEAD) socketcli --pr-number 123 --base-commit-sha "$BASE_SHA" ``` -> **Requirement:** `--base-commit-sha` only works if Socket already has a full scan for that -> exact commit. In practice this means your CI must run `socketcli` on **every commit that -> lands on your default branch** — not just some of them. If merges can land without a scan -> (skipped/canceled builds, `[skip ci]`, path-filtered pipelines), the PR scan will fail with -> exit code 3 rather than silently diff against the wrong baseline. See +> The CLI uses the exact commit's newest matching full scan when one exists. Otherwise, it +> searches up to 100 first-parent commits in the local checkout and uses the nearest scanned +> ancestor, with a warning that the diff is wider than the merge base. Run `socketcli` +> regularly on your default branch and ensure PR checkouts contain enough history for that +> walk. The run fails with the configured API-error exit code only when no scanned ancestor +> is reachable (or when the scan lookup itself fails). See > [`docs/cli-reference.md`](https://github.com/SocketDev/socket-python-cli/blob/main/docs/cli-reference.md) -> for the full requirements and a backfill pattern that makes PR jobs self-sufficient. +> for the full behavior and an optional exact-baseline backfill pattern. A specific full scan ID also works: `--base-scan-id `. diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 968799b5..2051a691 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -347,11 +347,14 @@ generator rather than a static YAML file: BASE_SHA=$(git merge-base "origin/${TARGET}" HEAD) ``` -- **Emit the backfill step conditionally from the generator.** The generator is the - natural place for the "does a baseline scan exist?" check +- **If an exact baseline is required, emit a backfill step conditionally from the + generator.** The generator is the natural place for the "does an exact baseline + scan exist?" check (`GET /orgs/{org}/full-scans?repo=&commit_hash=$BASE_SHA&per_page=1`): only emit the baseline-scan step when it returns nothing. The emitted pipeline then shows - in the UI whether a backfill will run. + in the UI whether a backfill will run. Without a backfill, the CLI automatically + uses the nearest scanned first-parent ancestor within 100 commits and warns that + the diff is wider. - **Keep the backfill inside one command step.** The checkout-base → scan → checkout-PR sequence must not be split across steps — steps can land on different @@ -360,17 +363,18 @@ generator rather than a static YAML file: checkout: `git worktree add /tmp/socket-base "$BASE_SHA"` then `socketcli --target-path /tmp/socket-base --branch "$TARGET" --disable-blocking`. -- **Soft-fail infra errors, not findings.** A missing baseline (or any API error) - exits with code 3 (`--exit-code-on-api-error` to change it); real findings exit 1. +- **Soft-fail infra errors, not findings.** No reachable scanned ancestor (or any API + error) exits with code 3 (`--exit-code-on-api-error` to change it); real findings exit 1. [`soft_fail: [{exit_status: 3}]`](https://buildkite.com/docs/pipelines/configure/step-types/command-step) on the PR scan step keeps infra errors from blocking merges while security findings still do. - **["Cancel intermediate builds"](https://buildkite.com/docs/pipelines/configure/canceling-builds#cancel-running-intermediate-builds) - on the default branch is the main source of baseline gaps.** Canceled builds never - scan their commit, so merge-base lookups for PRs based on those commits fail. The - conditional backfill step above is the remedy; there is no per-step exemption from - build cancellation in Buildkite. If you need strict scan-once semantics for + on the default branch is a common source of exact-baseline gaps.** Canceled builds + never scan their commit, so these PRs fall back to an older scanned ancestor. Use + the conditional backfill step above when an exact merge-base comparison is required; + there is no per-step exemption from build cancellation in Buildkite. If you need + strict scan-once semantics for concurrent backfills of the same merge base, serialize the backfill step with a [concurrency group](https://buildkite.com/docs/pipelines/configure/workflows/controlling-concurrency) keyed on the merge-base SHA. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index f64de267..d0b61ce5 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -256,24 +256,22 @@ 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 diff against, overriding the repository's head scan as the baseline. The most recent full scan for that commit is used; the CLI errors (exit code 3, or `--exit-code-on-api-error`) if no scan exists for it. 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. Mutually exclusive with `--base-scan-id` | -> **Diffing against the merge base** — by default, PR scans are diffed against the repository's *latest* head scan, which may include newer default-branch commits than your PR branched from. To diff against the exact commit your PR is based on, compute the merge base and pass it as the baseline: +> **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: > > ```shell > BASE_SHA=$(git merge-base origin/main HEAD) > socketcli --pr-number 123 --base-commit-sha "$BASE_SHA" > ``` > -> **Requirement: a full scan must already exist for the merge-base commit.** `--base-commit-sha` does not create a scan of that commit; it looks up an existing one. That lookup only succeeds if your CI runs `socketcli` on **every commit that lands on your default branch** — every merge and direct push, not just periodic or latest-only scans. Common ways commits slip through without a scan: +> `--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. > -> - CI settings that cancel or skip intermediate builds when newer commits land (e.g. Buildkite's ["cancel intermediate builds"](https://buildkite.com/docs/pipelines/configure/canceling-builds#cancel-running-intermediate-builds)) -> - `[skip ci]` commits, path-filtered pipelines, or failed/canceled scan steps -> - merge-base commits that predate your Socket rollout +> 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 scan exists for the commit, 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 head scan — a wrong baseline would misreport which alerts the PR introduces. Don't adopt this flag without default-branch scan coverage in place; you'll fail PR builds on lookup misses. +> 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. > -> **Backfill pattern** — if your default-branch coverage has gaps, the PR job can create the missing baseline itself before scanning: +> **Optional exact-baseline backfill** — if the wider ancestor fallback is not acceptable, the PR job can create the missing exact baseline before scanning: > > ```shell > BASE_SHA=$(git merge-base origin/main HEAD) @@ -285,7 +283,7 @@ If you don't want to provide the Socket API Token every time then you can use th > socketcli --pr-number 123 --base-commit-sha "$BASE_SHA" > ``` > -> Run the baseline step with `--disable-blocking` (findings on the default branch must not fail the PR job) and an explicit `--branch`, since branch auto-detection is unreliable at a detached HEAD. +> Run the baseline step with `--disable-blocking` (findings on the default branch must not fail the PR job) and an explicit `--branch`, since branch auto-detection is unreliable at a detached HEAD. Without this step, the CLI automatically uses the nearest scanned ancestor. > > Buildkite users with dynamically generated pipelines: see [Merge-base baselines in Buildkite](ci-cd.md#merge-base-baselines-in-buildkite-dynamic-pipelines) for generation-time vs. step-time guidance. diff --git a/socketsecurity/config.py b/socketsecurity/config.py index 35904976..ecd2f287 100644 --- a/socketsecurity/config.py +++ b/socketsecurity/config.py @@ -587,9 +587,9 @@ def create_argument_parser() -> argparse.ArgumentParser: metavar="", default=None, help="Commit SHA to diff the new scan against, overriding the repository's head " - "scan as the baseline. The most recent full scan matching this commit (e.g. " - "the merge base from 'git merge-base origin/main HEAD') is used; the CLI " - "errors if no scan exists for it. Mutually exclusive with --base-scan-id." + "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." ) # Path and File options diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index 34f63f78..c5f3f52c 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -14,7 +14,7 @@ import time from dataclasses import asdict from pathlib import PurePath -from typing import TYPE_CHECKING, Dict, List, NamedTuple, Optional, Set, Tuple +from typing import TYPE_CHECKING, Dict, Iterator, List, NamedTuple, Optional, Set, Tuple if TYPE_CHECKING: from socketsecurity.config import CliConfig @@ -58,8 +58,8 @@ SCAN_LOOKUP_PAGE_SIZE = 10 # Bounds on the search for a scanned ancestor when the requested baseline commit has -# no full scan of its own. The scan listing is fetched once and matched against local -# history, so neither bound costs an extra request. +# no full scan of its own. Full-scan listing pages are matched against bounded local +# history, so the local walk cannot grow without limit. ANCESTOR_SCAN_LOOKUP_LIMIT = 100 ANCESTOR_WALK_MAX_DEPTH = 100 @@ -1484,10 +1484,10 @@ def get_head_scan_for_repo( """ Gets the head scan ID for a repository. - Without a workspace this is the repository's head scan pointer. That pointer - tracks a single scan for the whole repository rather than one per workspace, - so workspace-scoped runs instead take the newest matching scan on the default - branch. + Without a workspace or scan type this is the repository's head scan pointer. + That pointer tracks a single scan for the whole repository rather than one per + workspace or scan type, so scoped runs instead take the newest matching scan + on the default branch. Args: repo_slug: Repository slug to get head scan for @@ -1498,32 +1498,32 @@ def get_head_scan_for_repo( Head scan ID if it exists, None otherwise Raises: - APIFailure: If the workspace scan lookup fails. A failed lookup must not + APIFailure: If the scoped scan lookup fails. A failed lookup must not be reported as "no baseline": the caller answers that by creating an empty baseline scan, which reports every dependency in the repository as newly added. """ repo_info = self.get_repo_info(repo_slug) - if workspace: + if workspace or scan_type: query_params = { "repo": repo_slug, - "workspace": workspace, "branch": repo_info.default_branch, "sort": "created_at", "direction": "desc", "per_page": SCAN_LOOKUP_PAGE_SIZE, } + if workspace: + query_params["workspace"] = workspace if scan_type: query_params["scan_type"] = Core.query_param_value(scan_type) - response = self.sdk.fullscans.get(self.config.org_slug, query_params) - results = response.get("results") if isinstance(response, dict) else None - if results is None: - # The SDK logs and returns {} for any non-200, so an empty "results" - # key is the only signal that the request itself succeeded. - raise APIFailure( - f"Failed to list full scans for repo {repo_slug} in workspace {workspace}" - ) - return Core.newest_persisted_scan_id(results) + for results in self._full_scan_result_pages( + query_params, + f"Failed to list matching full scans for repo {repo_slug}", + ): + scan_id = Core.newest_persisted_scan_id(results) + if scan_id: + return scan_id + return None return repo_info.head_full_scan_id if repo_info.head_full_scan_id else None @staticmethod @@ -1562,6 +1562,38 @@ def newest_persisted_scan_id(results: List[dict]) -> Optional[str]: return scan_id return None + def _full_scan_result_pages( + self, + query_params: dict, + failure_message: str, + ) -> Iterator[List[dict]]: + """Yields successful full-scan listing pages and rejects failed lookups.""" + request_params = dict(query_params) + seen_pages = {str(request_params.get("page", 1))} + per_page = int(request_params.get("per_page", 30)) + + while True: + response = self.sdk.fullscans.get(self.config.org_slug, request_params) + results = response.get("results") if isinstance(response, dict) else None + if results is None: + # The SDK logs and returns {} for any non-200, so a present results + # key is the only signal that the request itself succeeded. + raise APIFailure(failure_message) + yield results + + next_page = response.get("nextPage") + # The API has historically returned nextPage=1 for a short final page. + if len(results) < per_page or next_page in (None, 0, "0", False, ""): + return + + page_key = str(next_page) + if page_key in seen_pages: + raise APIFailure( + f"{failure_message}: full-scan pagination repeated page {next_page}" + ) + seen_pages.add(page_key) + request_params = {**query_params, "page": next_page} + def first_parent_commits(self, start_commit_sha: str, max_count: int) -> List[str]: """ Lists a commit and its first-parent ancestors, newest first. @@ -1609,8 +1641,9 @@ def find_baseline_scan_for_ancestor( scanning is configured correctly. Diffing against a slightly older ancestor is a wider diff; failing outright is no diff at all. - One scan listing is fetched and matched against local first-parent history, - so the walk costs no additional requests. + Scan listing pages are matched against local first-parent history. All pages + are considered because scans from other branches and reruns can fill newer + pages without covering the nearest candidate ancestors. Args: repo_slug: Repository slug the scan belongs to @@ -1622,6 +1655,10 @@ def find_baseline_scan_for_ancestor( (scan_id, ancestor_commit_sha, commits_back), or (None, None, 0) when no scanned ancestor is reachable. """ + ancestors = self.first_parent_commits(commit_sha, ANCESTOR_WALK_MAX_DEPTH) + if not ancestors: + return None, None, 0 + query_params = { "repo": repo_slug, "sort": "created_at", @@ -1633,25 +1670,28 @@ def find_baseline_scan_for_ancestor( if scan_type: query_params["scan_type"] = Core.query_param_value(scan_type) - response = self.sdk.fullscans.get(self.config.org_slug, query_params) - results = response.get("results") if isinstance(response, dict) else None - if not results: - return None, None, 0 - scans_by_commit = {} - for result in results: - if not isinstance(result, dict) or result.get("tmp"): - continue - result_commit = result.get("commit_hash") - scan_id = result.get("id") - # Newest first, so the first scan seen for a commit is the one to keep. - if result_commit and scan_id and result_commit not in scans_by_commit: - scans_by_commit[result_commit] = scan_id + ancestor_set = set(ancestors) + for results in self._full_scan_result_pages( + query_params, + f"Failed to list ancestor full scans for repo {repo_slug}", + ): + for result in results: + if not isinstance(result, dict) or result.get("tmp"): + continue + result_commit = result.get("commit_hash") + scan_id = result.get("id") + # Newest first, so the first scan seen for a commit is the one to keep. + if ( + result_commit in ancestor_set + and scan_id + and result_commit not in scans_by_commit + ): + scans_by_commit[result_commit] = scan_id if not scans_by_commit: return None, None, 0 - ancestors = self.first_parent_commits(commit_sha, ANCESTOR_WALK_MAX_DEPTH) for distance, ancestor in enumerate(ancestors): scan_id = scans_by_commit.get(ancestor) if scan_id: @@ -1693,24 +1733,25 @@ def get_full_scan_id_by_commit( if scan_type: query_params["scan_type"] = Core.query_param_value(scan_type) - response = self.sdk.fullscans.get( - self.config.org_slug, - query_params, - ) - results = response.get("results") if isinstance(response, dict) else None - if not results: - return None - return Core.newest_persisted_scan_id(results) + for results in self._full_scan_result_pages( + query_params, + f"Failed to list full scans for commit {commit_sha} in repo {repo_slug}", + ): + scan_id = Core.newest_persisted_scan_id(results) + if scan_id: + return scan_id + return None def resolve_base_full_scan_id(self, params: FullScanParams) -> Optional[str]: """ Resolves the baseline full scan ID to diff a new scan against. Priority: --base-scan-id (used verbatim), then --base-commit-sha (newest - full scan for that commit), then the repository's current head scan. A - --base-commit-sha with no matching full scan is a hard error rather than a - silent fallback to the head scan, because diffing against the wrong - baseline silently misreports which alerts a PR introduces. + full scan for that commit, or its nearest scanned first-parent ancestor), + then the repository's current matching head scan. A --base-commit-sha with + no reachable scanned ancestor is a hard error rather than a silent fallback + to the head scan, because diffing against the wrong baseline silently + misreports which alerts a PR introduces. Returns: Full scan ID to use as the diff baseline, or None when the repository @@ -1775,13 +1816,11 @@ def resolve_base_full_scan_id(self, params: FullScanParams) -> Optional[str]: except APIResourceNotFound: return None except APIFailure as error: - # Only workspace-scoped lookups raise here. Returning None instead would - # make the caller create an empty baseline scan, reporting every - # dependency as newly added, so fail loudly like the --base-commit-sha - # path above. + # Returning None would make the caller create an empty baseline scan, + # reporting every dependency as newly added, so fail loudly like the + # --base-commit-sha path above. log.error( - f"Failed to resolve the head scan for repo {params.repo} in workspace " - f"{params.workspace}: {error}" + f"Failed to resolve the matching head scan for repo {params.repo}: {error}" ) if self.cli_config is None: raise diff --git a/tests/core/test_sdk_methods.py b/tests/core/test_sdk_methods.py index 92b9a17b..bf439a3b 100644 --- a/tests/core/test_sdk_methods.py +++ b/tests/core/test_sdk_methods.py @@ -3,7 +3,7 @@ from socketdev.fullscans import FullScanParams, FullScanStreamResponse, ScanType from socketsecurity.config import CliConfig -from socketsecurity.core import SCAN_LOOKUP_PAGE_SIZE, Core +from socketsecurity.core import ANCESTOR_SCAN_LOOKUP_LIMIT, SCAN_LOOKUP_PAGE_SIZE, Core from socketsecurity.core.socket_config import SocketConfig @@ -93,6 +93,31 @@ def test_get_head_scan_for_repo_scopes_workspace_to_default_branch( ) +def test_get_head_scan_for_repo_scopes_scan_type_without_workspace( + core, mock_sdk_with_responses +): + """Reachability and standard scans must not share one unscoped head pointer""" + mock_sdk_with_responses.fullscans.get.return_value = { + "results": [{"id": "standard-head"}], + "nextPage": None, + } + + head_scan_id = core.get_head_scan_for_repo("test", scan_type="socket") + + assert head_scan_id == "standard-head" + mock_sdk_with_responses.fullscans.get.assert_called_once_with( + core.config.org_slug, + { + "repo": "test", + "branch": "main", + "sort": "created_at", + "direction": "desc", + "per_page": SCAN_LOOKUP_PAGE_SIZE, + "scan_type": "socket", + }, + ) + + def test_get_head_scan_for_repo_workspace_lookup_failure_raises(core, mock_sdk_with_responses): """A failed listing is not the same as an empty one and must not resolve to None""" mock_sdk_with_responses.fullscans.get.return_value = {} @@ -203,16 +228,26 @@ def test_get_full_scan_id_by_commit_skips_temporary_scans(core, mock_sdk_with_re def test_get_full_scan_id_by_commit_not_found(core, mock_sdk_with_responses): - """No scan for the commit returns None (empty results and SDK error dict)""" + """A successful empty listing means the commit has no scan""" mock_sdk_with_responses.fullscans.get.return_value = {"results": [], "nextPage": None} assert core.get_full_scan_id_by_commit("test", "abc123") is None + +def test_get_full_scan_id_by_commit_lookup_failure_raises(core, mock_sdk_with_responses): + """An SDK error dict must not trigger fallback to an older ancestor""" mock_sdk_with_responses.fullscans.get.return_value = {} - assert core.get_full_scan_id_by_commit("test", "abc123") is None + with pytest.raises(APIFailure): + core.get_full_scan_id_by_commit("test", "abc123") + def test_resolve_base_full_scan_id_defaults_to_head_scan(core): - """Without base overrides the repository head scan is the baseline""" - assert core.resolve_base_full_scan_id(make_full_scan_params()) == "head" + """Without base overrides the matching scan type's head scan is the baseline""" + core.sdk.fullscans.get.return_value = { + "results": [{"id": "standard-head"}], + "nextPage": None, + } + + assert core.resolve_base_full_scan_id(make_full_scan_params()) == "standard-head" def test_resolve_base_full_scan_id_scopes_head_to_workspace(core): @@ -323,6 +358,55 @@ def test_resolve_base_full_scan_id_falls_back_to_scanned_ancestor(core, monkeypa assert core.resolve_base_full_scan_id(params) == "ancestor-scan" +def test_find_baseline_scan_for_ancestor_paginates_and_selects_nearest( + core, monkeypatch +): + """Reruns can fill page one while a closer scanned ancestor is on page two""" + first_page = [ + {"id": "farther-scan", "commit_hash": "ancestor-2"}, + *[ + {"id": f"unrelated-{index}", "commit_hash": f"other-{index}"} + for index in range(ANCESTOR_SCAN_LOOKUP_LIMIT - 1) + ], + ] + core.sdk.fullscans.get.side_effect = [ + {"results": first_page, "nextPage": 2}, + { + "results": [{"id": "nearest-scan", "commit_hash": "ancestor-1"}], + "nextPage": 0, + }, + ] + monkeypatch.setattr( + Core, + "first_parent_commits", + lambda self, sha, depth: ["unscanned-sha", "ancestor-1", "ancestor-2"], + ) + + assert core.find_baseline_scan_for_ancestor( + "test", + "unscanned-sha", + scan_type="socket", + ) == ("nearest-scan", "ancestor-1", 1) + assert core.sdk.fullscans.get.call_args_list[1].args[1]["page"] == 2 + + +def test_resolve_base_full_scan_id_exact_lookup_failure_does_not_fallback( + core, monkeypatch +): + core.cli_config = make_cli_config("--base-commit-sha", "abc123") + core.sdk.fullscans.get.return_value = {} + fallback_calls = [] + monkeypatch.setattr( + Core, + "find_baseline_scan_for_ancestor", + lambda *args, **kwargs: fallback_calls.append((args, kwargs)), + ) + + with pytest.raises(APIFailure): + core.resolve_base_full_scan_id(make_full_scan_params()) + assert fallback_calls == [] + + def test_resolve_base_full_scan_id_ancestor_fallback_skips_temporary_scans(core, monkeypatch): """A tmp scan on an ancestor is not a usable baseline either""" core.cli_config = make_cli_config("--base-commit-sha", "unscanned-sha") @@ -503,10 +587,18 @@ def test_empty_alerts_preserved(core): def test_repository_head_baseline_log(core, caplog): + core.sdk.fullscans.get.return_value = { + "results": [{"id": "standard-head"}], + "nextPage": None, + } + with caplog.at_level("INFO", logger="socketdev"): - assert core.resolve_base_full_scan_id(make_full_scan_params()) == "head" + assert core.resolve_base_full_scan_id(make_full_scan_params()) == "standard-head" - assert 'Baseline selected: source=repository-head scan_id="head"' in caplog.messages + assert ( + 'Baseline selected: source=repository-head scan_id="standard-head"' + in caplog.messages + ) def test_explicit_scan_baseline_log(core, caplog):