Skip to content

Commit fa1fd7b

Browse files
committed
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.
1 parent fd2501d commit fa1fd7b

2 files changed

Lines changed: 32 additions & 24 deletions

File tree

‎socketsecurity/core/alert_selection.py‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ def select_diff_alerts(diff: Diff, strict_blocking: bool = False) -> List[Issue]
1616
1717
In strict blocking mode, include unchanged alerts so rendered output aligns
1818
with pass/fail policy evaluation. An unchanged alert that is already listed
19-
as new (same purl and alert type, the key the diff consolidates on) is
20-
skipped so each finding is rendered once.
19+
as new (same ecosystem, purl and alert type) is skipped so each finding is
20+
rendered once. The ecosystem is part of the key because ``Issue.purl`` holds
21+
only name and version, so an npm and a PyPI package can share it.
2122
"""
2223
selected = list(getattr(diff, "new_alerts", []) or [])
2324
if strict_blocking:
@@ -30,11 +31,11 @@ def select_diff_alerts(diff: Diff, strict_blocking: bool = False) -> List[Issue]
3031
return selected
3132

3233

33-
def _alert_identity(alert: Issue) -> Optional[Tuple[str, str]]:
34+
def _alert_identity(alert: Issue) -> Optional[Tuple[str, str, str]]:
3435
purl = getattr(alert, "purl", "") or ""
3536
if not purl:
3637
return None
37-
return (purl, getattr(alert, "type", "") or "")
38+
return (getattr(alert, "pkg_type", "") or "", purl, getattr(alert, "type", "") or "")
3839

3940

4041
def clone_diff_with_selected_alerts(diff: Diff, selected_alerts: List[Issue]) -> Diff:

‎tests/core/test_strict_blocking_duplicates.py‎

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,16 @@
55
new and once as existing, doubling the JSON, SARIF, Slack and GitLab findings and
66
inflating the "EXISTING blocking issues" count in the summary.
77
8-
These tests feed a mocked diff response through the same steps `create_new_diff`
9-
uses, then check what the output layer renders.
8+
These tests run `Core.create_new_diff` against a mocked diff response, with the
9+
scan upload mocked out, then check what the output layer renders.
1010
"""
1111
import json
1212
from pathlib import Path
13+
from types import SimpleNamespace
1314
from unittest.mock import Mock
1415

1516
import pytest
16-
from socketdev.fullscans import StreamDiffResponse
17+
from socketdev.fullscans import FullScanParams, StreamDiffResponse
1718

1819
from socketsecurity.config import CliConfig
1920
from socketsecurity.core import Core
@@ -79,16 +80,15 @@ def _diff_response(**buckets):
7980

8081

8182
def _build_diff(core, response):
82-
"""Mirror the package and diff assembly in Core.create_new_diff."""
83+
"""Run Core.create_new_diff with the upload mocked out and `response` as the diff."""
8384
core.sdk.fullscans.stream_diff.return_value = response
84-
added, removed, packages = core.get_added_and_removed_packages("head", "new")
85-
unchanged = Core.get_unchanged_packages(packages, added, removed)
86-
diff = core.create_diff_report(added, removed, unchanged)
87-
diff.packages = packages
88-
diff.id = "new"
89-
diff.diff_url = "https://socket.dev/diff"
90-
diff.report_url = "https://socket.dev/report"
91-
return diff
85+
core.resolve_base_full_scan_id = Mock(return_value="head")
86+
core.create_full_scan = Mock(return_value=SimpleNamespace(id="new"))
87+
params = FullScanParams(repo="repo", branch="feature")
88+
params.include_license_details = True
89+
return core.create_new_diff(
90+
["/repo"], params, explicit_files=["/repo/package.json"],
91+
)
9292

9393

9494
def _handler(strict_blocking=True):
@@ -166,24 +166,20 @@ def test_without_strict_blocking_output_is_unchanged(core):
166166

167167
def test_unchanged_packages_are_keyed_by_artifact_id(core):
168168
"""Transitive unchanged alerts resolve their introducer through the ancestor ID."""
169-
core.sdk.fullscans.stream_diff.return_value = _diff_response(unchanged=[
169+
diff = _build_diff(core, _diff_response(unchanged=[
170170
_artifact("unchanged", "top", "parent"),
171171
_artifact(
172172
"unchanged", "dep", "child", [_blocking_alert("k5")],
173173
direct=False, ancestors=["top"],
174174
),
175-
])
176-
added, removed, packages = core.get_added_and_removed_packages("head", "new")
177-
unchanged = Core.get_unchanged_packages(packages, added, removed)
175+
]))
178176

179-
assert set(unchanged) == {"top", "dep"}
180-
diff = core.create_diff_report(added, removed, unchanged)
181177
assert diff.unchanged_alerts[0].introduced_by == [("npm/parent@1.0.0", "package.json")]
182178

183179

184-
def _issue(name, alert_type="malware"):
180+
def _issue(name, alert_type="malware", pkg_type="npm"):
185181
return Issue(
186-
pkg_type="npm",
182+
pkg_type=pkg_type,
187183
pkg_name=name,
188184
pkg_version="1.0.0",
189185
type=alert_type,
@@ -204,3 +200,14 @@ def test_select_diff_alerts_skips_unchanged_alert_already_listed_as_new():
204200
("alpha", "typosquat"),
205201
("gamma", "malware"),
206202
]
203+
204+
205+
def test_select_diff_alerts_keeps_same_purl_from_another_ecosystem():
206+
"""`Issue.purl` has no ecosystem, so npm and PyPI packages can share one."""
207+
diff = Diff()
208+
diff.new_alerts = [_issue("alpha", pkg_type="npm")]
209+
diff.unchanged_alerts = [_issue("alpha", pkg_type="pypi")]
210+
211+
selected = select_diff_alerts(diff, strict_blocking=True)
212+
213+
assert [a.pkg_type for a in selected] == ["npm", "pypi"]

0 commit comments

Comments
 (0)