List each finding once under --strict-blocking

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

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 = [

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__}'

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],

socketsecurity/core/alert_selection.py

@@ -15,14 +15,29 @@ 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 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:
-        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, str]]:
+    purl = getattr(alert, "purl", "") or ""
+    if not purl:
+        return None
+    return (getattr(alert, "pkg_type", "") or "", 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(

tests/core/test_strict_blocking_duplicates.py

@@ -0,0 +1,213 @@
+"""`--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 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 FullScanParams, 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):
+    """Run Core.create_new_diff with the upload mocked out and `response` as the diff."""
+    core.sdk.fullscans.stream_diff.return_value = response
+    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):
+    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."""
+    diff = _build_diff(core, _diff_response(unchanged=[
+        _artifact("unchanged", "top", "parent"),
+        _artifact(
+            "unchanged", "dep", "child", [_blocking_alert("k5")],
+            direct=False, ancestors=["top"],
+        ),
+    ]))
+
+    assert diff.unchanged_alerts[0].introduced_by == [("npm/parent@1.0.0", "package.json")]
+
+
+def _issue(name, alert_type="malware", pkg_type="npm"):
+    return Issue(
+        pkg_type=pkg_type,
+        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"),
+    ]
+
+
+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"]