From fd2501db175579b9a5fb193f38528b8ae79bbb7d Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Fri, 13 Nov 2026 20:56:55 -0500 Subject: [PATCH 1/2] List each finding once under --strict-blocking create_new_diff built the unchanged package set by filtering packages keyed by type/purl against added and removed packages keyed by artifact ID. The keys never matched, so every added package was also treated as unchanged, and under --strict-blocking its alerts were reported twice: once as new and once as existing. Against an empty baseline that doubled every finding in JSON, SARIF, Slack and GitLab output, and inflated the "EXISTING blocking issues" count in the summary and GitLab commit status. The exit code was not affected. Unchanged packages are now selected and keyed by artifact ID, which also lets transitive unchanged alerts resolve their introducing package. select_diff_alerts additionally skips an unchanged alert already listed as new, matching on purl and alert type like the rest of the diff. Bumps the version to 2.12.1. --- CHANGELOG.md | 10 + pyproject.toml | 2 +- socketsecurity/__init__.py | 2 +- socketsecurity/core/__init__.py | 24 +- socketsecurity/core/alert_selection.py | 18 +- tests/core/test_strict_blocking_duplicates.py | 206 ++++++++++++++++++ uv.lock | 2 +- 7 files changed, 254 insertions(+), 10 deletions(-) create mode 100644 tests/core/test_strict_blocking_duplicates.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 810aabd4..f16c39ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## 2.12.1 + +### Fixed: duplicate findings with `--strict-blocking` + +- Alerts on added packages are no longer also reported as existing alerts. Under + `--strict-blocking`, each finding on an added package was listed twice in JSON, + SARIF, Slack and GitLab output, and counted again in the summary's + "EXISTING blocking issues" line and the GitLab commit status. The exit code + was not affected. + ## 2.12.0 ### Added diff --git a/pyproject.toml b/pyproject.toml index b62bd60f..3ee6508e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" [project] name = "socketsecurity" -version = "2.12.0" +version = "2.12.1" requires-python = ">= 3.11" license = {"file" = "LICENSE"} dependencies = [ diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index 203f24cf..01e27579 100644 --- a/socketsecurity/__init__.py +++ b/socketsecurity/__init__.py @@ -1,4 +1,4 @@ __author__ = 'socket.dev' -__version__ = '2.12.0' +__version__ = '2.12.1' USER_AGENT_NAME = 'socket-python-cli' USER_AGENT = f'{USER_AGENT_NAME}/{__version__}' diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index f78f6c19..305d6309 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -2480,11 +2480,7 @@ def create_new_diff( external_href=external_href, ) - # Separate unchanged packages from added/removed for --strict-blocking support - unchanged_packages = { - pkg_id: pkg for pkg_id, pkg in packages.items() - if pkg_id not in added_packages and pkg_id not in removed_packages - } + unchanged_packages = Core.get_unchanged_packages(packages, added_packages, removed_packages) diff = self.create_diff_report(added_packages, removed_packages, unchanged_packages) diff.packages = packages @@ -2509,6 +2505,24 @@ def create_new_diff( return diff + @staticmethod + def get_unchanged_packages( + packages: Dict[str, Package], + added_packages: Dict[str, Package], + removed_packages: Dict[str, Package], + ) -> Dict[str, Package]: + """Return the packages in ``packages`` that were neither added nor removed. + + ``packages`` is keyed by ``type/purl`` while the added and removed dicts are + keyed by artifact ID, so membership is checked on ``Package.id``. The result + is keyed by artifact ID to match the other two, which is what + ``get_source_data`` expects when it resolves ``topLevelAncestors``. + """ + return { + pkg.id: pkg for pkg in packages.values() + if pkg.id not in added_packages and pkg.id not in removed_packages + } + def create_diff_report( self, added_packages: Dict[str, Package], diff --git a/socketsecurity/core/alert_selection.py b/socketsecurity/core/alert_selection.py index 132be294..83cddd40 100644 --- a/socketsecurity/core/alert_selection.py +++ b/socketsecurity/core/alert_selection.py @@ -15,14 +15,28 @@ def select_diff_alerts(diff: Diff, strict_blocking: bool = False) -> List[Issue] """Select diff alerts for output rendering. In strict blocking mode, include unchanged alerts so rendered output aligns - with pass/fail policy evaluation. + with pass/fail policy evaluation. An unchanged alert that is already listed + as new (same purl and alert type, the key the diff consolidates on) is + skipped so each finding is rendered once. """ selected = list(getattr(diff, "new_alerts", []) or []) if strict_blocking: - selected.extend(getattr(diff, "unchanged_alerts", []) or []) + seen = {_alert_identity(alert) for alert in selected} - {None} + for alert in getattr(diff, "unchanged_alerts", []) or []: + identity = _alert_identity(alert) + if identity is None or identity not in seen: + selected.append(alert) + seen.add(identity) return selected +def _alert_identity(alert: Issue) -> Optional[Tuple[str, str]]: + purl = getattr(alert, "purl", "") or "" + if not purl: + return None + return (purl, getattr(alert, "type", "") or "") + + def clone_diff_with_selected_alerts(diff: Diff, selected_alerts: List[Issue]) -> Diff: """Clone a diff object while replacing new_alerts with selected alerts.""" selected_diff = Diff( diff --git a/tests/core/test_strict_blocking_duplicates.py b/tests/core/test_strict_blocking_duplicates.py new file mode 100644 index 00000000..e4a352ff --- /dev/null +++ b/tests/core/test_strict_blocking_duplicates.py @@ -0,0 +1,206 @@ +"""`--strict-blocking` lists each finding once, whatever the baseline. + +Added packages must not also count as unchanged. An added package that leaks into +the unchanged set has its alerts reported twice under `--strict-blocking`, once as +new and once as existing, doubling the JSON, SARIF, Slack and GitLab findings and +inflating the "EXISTING blocking issues" count in the summary. + +These tests feed a mocked diff response through the same steps `create_new_diff` +uses, then check what the output layer renders. +""" +import json +from pathlib import Path +from unittest.mock import Mock + +import pytest +from socketdev.fullscans import StreamDiffResponse + +from socketsecurity.config import CliConfig +from socketsecurity.core import Core +from socketsecurity.core.alert_selection import select_diff_alerts +from socketsecurity.core.classes import Diff, Issue +from socketsecurity.core.socket_config import SocketConfig +from socketsecurity.output import OutputHandler + +BUCKETS = ("added", "removed", "unchanged", "replaced", "updated") + + +@pytest.fixture +def core(mock_sdk_with_responses): + core = Core(config=SocketConfig(api_key="test_key"), sdk=mock_sdk_with_responses) + # Force the streaming diff so the mocked response below is the artifact source. + core.sdk.diffscans.create_from_ids.side_effect = Exception("diff-scans unavailable") + core.sdk.fullscans.stream_diff.side_effect = None + return core + + +def _artifact(diff_type, artifact_id, name, alerts=None, direct=True, ancestors=None): + ref = { + "direct": direct, + "manifestFiles": [{"file": "package.json", "start": 0, "end": 0}] if direct else [], + "topLevelAncestors": ancestors or [], + } + return { + "diffType": diff_type, + "type": "npm", + "name": name, + "version": "1.0.0", + "id": artifact_id, + "head": [] if diff_type in ("removed", "replaced") else [ref], + "base": [ref] if diff_type in ("removed", "replaced") else [], + "license": "MIT", + "licenseDetails": [], + "author": [], + "size": 1, + "score": {}, + "alerts": alerts or [], + } + + +def _blocking_alert(key, alert_type="malware"): + return { + "key": key, + "type": alert_type, + "severity": "critical", + "category": "supplyChainRisk", + "props": {}, + "action": "error", + } + + +def _diff_response(**buckets): + raw = json.loads((Path(__file__).parent.parent / "data/fullscans/diff/stream_diff.json").read_text()) + artifacts = {name: buckets.get(name, []) for name in BUCKETS} + return StreamDiffResponse.from_dict({ + "success": raw["success"], + "status": raw["status"], + "data": {**raw["data"], "artifacts": artifacts}, + }) + + +def _build_diff(core, response): + """Mirror the package and diff assembly in Core.create_new_diff.""" + core.sdk.fullscans.stream_diff.return_value = response + added, removed, packages = core.get_added_and_removed_packages("head", "new") + unchanged = Core.get_unchanged_packages(packages, added, removed) + diff = core.create_diff_report(added, removed, unchanged) + diff.packages = packages + diff.id = "new" + diff.diff_url = "https://socket.dev/diff" + diff.report_url = "https://socket.dev/report" + return diff + + +def _handler(strict_blocking=True): + config = Mock(spec=CliConfig) + config.disable_blocking = False + config.strict_blocking = strict_blocking + config.legal_format = "socket" + config.legal = False + config.repo = None + config.branch = None + config.commit_sha = None + return OutputHandler(config, Mock()) + + +def test_empty_baseline_lists_added_alerts_once(core): + """Against an empty baseline every package is added and none is unchanged.""" + diff = _build_diff(core, _diff_response(added=[ + _artifact("added", "a1", "alpha", [_blocking_alert("k1")]), + _artifact("added", "a2", "beta", [_blocking_alert("k2")]), + ])) + + assert len(diff.new_alerts) == 2 + assert diff.unchanged_alerts == [] + + handler = _handler() + report = handler.build_json_report(diff) + assert len(report["new_alerts"]) == 2 + assert not handler.report_pass(diff) + + summary = handler.build_summary_text(diff) + assert "NEW blocking issues: 2" in summary + assert "EXISTING blocking issues" not in summary + + +def test_real_baseline_lists_new_and_existing_alerts_once_each(core): + """With a baseline, added alerts are new and unchanged alerts are existing.""" + diff = _build_diff(core, _diff_response( + added=[_artifact("added", "a1", "alpha", [_blocking_alert("k1")])], + unchanged=[_artifact("unchanged", "u1", "gamma", [_blocking_alert("k3")])], + )) + + assert [a.pkg_name for a in diff.new_alerts] == ["alpha"] + assert [a.pkg_name for a in diff.unchanged_alerts] == ["gamma"] + + handler = _handler() + report = handler.build_json_report(diff) + assert sorted(a["pkg_name"] for a in report["new_alerts"]) == ["alpha", "gamma"] + assert not handler.report_pass(diff) + + summary = handler.build_summary_text(diff) + assert "NEW blocking issues: 1" in summary + assert "EXISTING blocking issues: 1" in summary + + +def test_real_baseline_fails_on_existing_alerts_alone(core): + """An unchanged blocking alert fails the run when the API returns it.""" + diff = _build_diff(core, _diff_response( + unchanged=[_artifact("unchanged", "u1", "gamma", [_blocking_alert("k3")])], + )) + + assert diff.new_alerts == [] + assert not _handler(strict_blocking=True).report_pass(diff) + assert _handler(strict_blocking=False).report_pass(diff) + + +def test_without_strict_blocking_output_is_unchanged(core): + diff = _build_diff(core, _diff_response( + added=[_artifact("added", "a1", "alpha", [_blocking_alert("k1")])], + unchanged=[_artifact("unchanged", "u1", "gamma", [_blocking_alert("k3")])], + )) + + report = _handler(strict_blocking=False).build_json_report(diff) + assert [a["pkg_name"] for a in report["new_alerts"]] == ["alpha"] + + +def test_unchanged_packages_are_keyed_by_artifact_id(core): + """Transitive unchanged alerts resolve their introducer through the ancestor ID.""" + core.sdk.fullscans.stream_diff.return_value = _diff_response(unchanged=[ + _artifact("unchanged", "top", "parent"), + _artifact( + "unchanged", "dep", "child", [_blocking_alert("k5")], + direct=False, ancestors=["top"], + ), + ]) + added, removed, packages = core.get_added_and_removed_packages("head", "new") + unchanged = Core.get_unchanged_packages(packages, added, removed) + + assert set(unchanged) == {"top", "dep"} + diff = core.create_diff_report(added, removed, unchanged) + assert diff.unchanged_alerts[0].introduced_by == [("npm/parent@1.0.0", "package.json")] + + +def _issue(name, alert_type="malware"): + return Issue( + pkg_type="npm", + pkg_name=name, + pkg_version="1.0.0", + type=alert_type, + purl=f"{name}@1.0.0", + error=True, + ) + + +def test_select_diff_alerts_skips_unchanged_alert_already_listed_as_new(): + diff = Diff() + diff.new_alerts = [_issue("alpha")] + diff.unchanged_alerts = [_issue("alpha"), _issue("alpha", "typosquat"), _issue("gamma")] + + selected = select_diff_alerts(diff, strict_blocking=True) + + assert [(a.pkg_name, a.type) for a in selected] == [ + ("alpha", "malware"), + ("alpha", "typosquat"), + ("gamma", "malware"), + ] diff --git a/uv.lock b/uv.lock index dc276bb5..6077c132 100644 --- a/uv.lock +++ b/uv.lock @@ -1334,7 +1334,7 @@ wheels = [ [[package]] name = "socketsecurity" -version = "2.12.0" +version = "2.12.1" source = { editable = "." } dependencies = [ { name = "beautifulsoup4" }, From fa1fd7baca00126d3a3e53fbed56ee396e843338 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Fri, 13 Nov 2026 22:45:11 -0500 Subject: [PATCH 2/2] Include the ecosystem in the strict-blocking dedupe key Issue.purl holds only name and version, so an npm and a PyPI package can share it. Keying the dedupe on purl and alert type alone would hide an existing PyPI alert behind a new npm alert with the same name, version and type. The regression tests now run Core.create_new_diff with the scan upload mocked out, instead of repeating its package assembly, so they fail if create_new_diff goes back to filtering unchanged packages by purl key. --- socketsecurity/core/alert_selection.py | 9 ++-- tests/core/test_strict_blocking_duplicates.py | 47 +++++++++++-------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/socketsecurity/core/alert_selection.py b/socketsecurity/core/alert_selection.py index 83cddd40..8c89c8b5 100644 --- a/socketsecurity/core/alert_selection.py +++ b/socketsecurity/core/alert_selection.py @@ -16,8 +16,9 @@ def select_diff_alerts(diff: Diff, strict_blocking: bool = False) -> List[Issue] In strict blocking mode, include unchanged alerts so rendered output aligns with pass/fail policy evaluation. An unchanged alert that is already listed - as new (same purl and alert type, the key the diff consolidates on) is - skipped so each finding is rendered once. + as new (same ecosystem, purl and alert type) is skipped so each finding is + rendered once. The ecosystem is part of the key because ``Issue.purl`` holds + only name and version, so an npm and a PyPI package can share it. """ selected = list(getattr(diff, "new_alerts", []) or []) if strict_blocking: @@ -30,11 +31,11 @@ def select_diff_alerts(diff: Diff, strict_blocking: bool = False) -> List[Issue] return selected -def _alert_identity(alert: Issue) -> Optional[Tuple[str, str]]: +def _alert_identity(alert: Issue) -> Optional[Tuple[str, str, str]]: purl = getattr(alert, "purl", "") or "" if not purl: return None - return (purl, getattr(alert, "type", "") or "") + return (getattr(alert, "pkg_type", "") or "", purl, getattr(alert, "type", "") or "") def clone_diff_with_selected_alerts(diff: Diff, selected_alerts: List[Issue]) -> Diff: diff --git a/tests/core/test_strict_blocking_duplicates.py b/tests/core/test_strict_blocking_duplicates.py index e4a352ff..a37f67cc 100644 --- a/tests/core/test_strict_blocking_duplicates.py +++ b/tests/core/test_strict_blocking_duplicates.py @@ -5,15 +5,16 @@ new and once as existing, doubling the JSON, SARIF, Slack and GitLab findings and inflating the "EXISTING blocking issues" count in the summary. -These tests feed a mocked diff response through the same steps `create_new_diff` -uses, then check what the output layer renders. +These tests run `Core.create_new_diff` against a mocked diff response, with the +scan upload mocked out, then check what the output layer renders. """ import json from pathlib import Path +from types import SimpleNamespace from unittest.mock import Mock import pytest -from socketdev.fullscans import StreamDiffResponse +from socketdev.fullscans import FullScanParams, StreamDiffResponse from socketsecurity.config import CliConfig from socketsecurity.core import Core @@ -79,16 +80,15 @@ def _diff_response(**buckets): def _build_diff(core, response): - """Mirror the package and diff assembly in Core.create_new_diff.""" + """Run Core.create_new_diff with the upload mocked out and `response` as the diff.""" core.sdk.fullscans.stream_diff.return_value = response - added, removed, packages = core.get_added_and_removed_packages("head", "new") - unchanged = Core.get_unchanged_packages(packages, added, removed) - diff = core.create_diff_report(added, removed, unchanged) - diff.packages = packages - diff.id = "new" - diff.diff_url = "https://socket.dev/diff" - diff.report_url = "https://socket.dev/report" - return diff + core.resolve_base_full_scan_id = Mock(return_value="head") + core.create_full_scan = Mock(return_value=SimpleNamespace(id="new")) + params = FullScanParams(repo="repo", branch="feature") + params.include_license_details = True + return core.create_new_diff( + ["/repo"], params, explicit_files=["/repo/package.json"], + ) def _handler(strict_blocking=True): @@ -166,24 +166,20 @@ def test_without_strict_blocking_output_is_unchanged(core): def test_unchanged_packages_are_keyed_by_artifact_id(core): """Transitive unchanged alerts resolve their introducer through the ancestor ID.""" - core.sdk.fullscans.stream_diff.return_value = _diff_response(unchanged=[ + diff = _build_diff(core, _diff_response(unchanged=[ _artifact("unchanged", "top", "parent"), _artifact( "unchanged", "dep", "child", [_blocking_alert("k5")], direct=False, ancestors=["top"], ), - ]) - added, removed, packages = core.get_added_and_removed_packages("head", "new") - unchanged = Core.get_unchanged_packages(packages, added, removed) + ])) - assert set(unchanged) == {"top", "dep"} - diff = core.create_diff_report(added, removed, unchanged) assert diff.unchanged_alerts[0].introduced_by == [("npm/parent@1.0.0", "package.json")] -def _issue(name, alert_type="malware"): +def _issue(name, alert_type="malware", pkg_type="npm"): return Issue( - pkg_type="npm", + pkg_type=pkg_type, pkg_name=name, pkg_version="1.0.0", type=alert_type, @@ -204,3 +200,14 @@ def test_select_diff_alerts_skips_unchanged_alert_already_listed_as_new(): ("alpha", "typosquat"), ("gamma", "malware"), ] + + +def test_select_diff_alerts_keeps_same_purl_from_another_ecosystem(): + """`Issue.purl` has no ecosystem, so npm and PyPI packages can share one.""" + diff = Diff() + diff.new_alerts = [_issue("alpha", pkg_type="npm")] + diff.unchanged_alerts = [_issue("alpha", pkg_type="pypi")] + + selected = select_diff_alerts(diff, strict_blocking=True) + + assert [a.pkg_type for a in selected] == ["npm", "pypi"]