Skip to content

Commit fd2501d

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

7 files changed

Lines changed: 254 additions & 10 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
# Changelog
22

3+
## 2.12.1
4+
5+
### Fixed: duplicate findings with `--strict-blocking`
6+
7+
- Alerts on added packages are no longer also reported as existing alerts. Under
8+
`--strict-blocking`, each finding on an added package was listed twice in JSON,
9+
SARIF, Slack and GitLab output, and counted again in the summary's
10+
"EXISTING blocking issues" line and the GitLab commit status. The exit code
11+
was not affected.
12+
313
## 2.12.0
414

515
### Added

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ build-backend = "hatchling.build"
66

77
[project]
88
name = "socketsecurity"
9-
version = "2.12.0"
9+
version = "2.12.1"
1010
requires-python = ">= 3.11"
1111
license = {"file" = "LICENSE"}
1212
dependencies = [

‎socketsecurity/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
__author__ = 'socket.dev'
2-
__version__ = '2.12.0'
2+
__version__ = '2.12.1'
33
USER_AGENT_NAME = 'socket-python-cli'
44
USER_AGENT = f'{USER_AGENT_NAME}/{__version__}'

‎socketsecurity/core/__init__.py‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2480,11 +2480,7 @@ def create_new_diff(
24802480
external_href=external_href,
24812481
)
24822482

2483-
# Separate unchanged packages from added/removed for --strict-blocking support
2484-
unchanged_packages = {
2485-
pkg_id: pkg for pkg_id, pkg in packages.items()
2486-
if pkg_id not in added_packages and pkg_id not in removed_packages
2487-
}
2483+
unchanged_packages = Core.get_unchanged_packages(packages, added_packages, removed_packages)
24882484

24892485
diff = self.create_diff_report(added_packages, removed_packages, unchanged_packages)
24902486
diff.packages = packages
@@ -2509,6 +2505,24 @@ def create_new_diff(
25092505

25102506
return diff
25112507

2508+
@staticmethod
2509+
def get_unchanged_packages(
2510+
packages: Dict[str, Package],
2511+
added_packages: Dict[str, Package],
2512+
removed_packages: Dict[str, Package],
2513+
) -> Dict[str, Package]:
2514+
"""Return the packages in ``packages`` that were neither added nor removed.
2515+
2516+
``packages`` is keyed by ``type/purl`` while the added and removed dicts are
2517+
keyed by artifact ID, so membership is checked on ``Package.id``. The result
2518+
is keyed by artifact ID to match the other two, which is what
2519+
``get_source_data`` expects when it resolves ``topLevelAncestors``.
2520+
"""
2521+
return {
2522+
pkg.id: pkg for pkg in packages.values()
2523+
if pkg.id not in added_packages and pkg.id not in removed_packages
2524+
}
2525+
25122526
def create_diff_report(
25132527
self,
25142528
added_packages: Dict[str, Package],

‎socketsecurity/core/alert_selection.py‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,28 @@ def select_diff_alerts(diff: Diff, strict_blocking: bool = False) -> List[Issue]
1515
"""Select diff alerts for output rendering.
1616
1717
In strict blocking mode, include unchanged alerts so rendered output aligns
18-
with pass/fail policy evaluation.
18+
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.
1921
"""
2022
selected = list(getattr(diff, "new_alerts", []) or [])
2123
if strict_blocking:
22-
selected.extend(getattr(diff, "unchanged_alerts", []) or [])
24+
seen = {_alert_identity(alert) for alert in selected} - {None}
25+
for alert in getattr(diff, "unchanged_alerts", []) or []:
26+
identity = _alert_identity(alert)
27+
if identity is None or identity not in seen:
28+
selected.append(alert)
29+
seen.add(identity)
2330
return selected
2431

2532

33+
def _alert_identity(alert: Issue) -> Optional[Tuple[str, str]]:
34+
purl = getattr(alert, "purl", "") or ""
35+
if not purl:
36+
return None
37+
return (purl, getattr(alert, "type", "") or "")
38+
39+
2640
def clone_diff_with_selected_alerts(diff: Diff, selected_alerts: List[Issue]) -> Diff:
2741
"""Clone a diff object while replacing new_alerts with selected alerts."""
2842
selected_diff = Diff(
Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
"""`--strict-blocking` lists each finding once, whatever the baseline.
2+
3+
Added packages must not also count as unchanged. An added package that leaks into
4+
the unchanged set has its alerts reported twice under `--strict-blocking`, once as
5+
new and once as existing, doubling the JSON, SARIF, Slack and GitLab findings and
6+
inflating the "EXISTING blocking issues" count in the summary.
7+
8+
These tests feed a mocked diff response through the same steps `create_new_diff`
9+
uses, then check what the output layer renders.
10+
"""
11+
import json
12+
from pathlib import Path
13+
from unittest.mock import Mock
14+
15+
import pytest
16+
from socketdev.fullscans import StreamDiffResponse
17+
18+
from socketsecurity.config import CliConfig
19+
from socketsecurity.core import Core
20+
from socketsecurity.core.alert_selection import select_diff_alerts
21+
from socketsecurity.core.classes import Diff, Issue
22+
from socketsecurity.core.socket_config import SocketConfig
23+
from socketsecurity.output import OutputHandler
24+
25+
BUCKETS = ("added", "removed", "unchanged", "replaced", "updated")
26+
27+
28+
@pytest.fixture
29+
def core(mock_sdk_with_responses):
30+
core = Core(config=SocketConfig(api_key="test_key"), sdk=mock_sdk_with_responses)
31+
# Force the streaming diff so the mocked response below is the artifact source.
32+
core.sdk.diffscans.create_from_ids.side_effect = Exception("diff-scans unavailable")
33+
core.sdk.fullscans.stream_diff.side_effect = None
34+
return core
35+
36+
37+
def _artifact(diff_type, artifact_id, name, alerts=None, direct=True, ancestors=None):
38+
ref = {
39+
"direct": direct,
40+
"manifestFiles": [{"file": "package.json", "start": 0, "end": 0}] if direct else [],
41+
"topLevelAncestors": ancestors or [],
42+
}
43+
return {
44+
"diffType": diff_type,
45+
"type": "npm",
46+
"name": name,
47+
"version": "1.0.0",
48+
"id": artifact_id,
49+
"head": [] if diff_type in ("removed", "replaced") else [ref],
50+
"base": [ref] if diff_type in ("removed", "replaced") else [],
51+
"license": "MIT",
52+
"licenseDetails": [],
53+
"author": [],
54+
"size": 1,
55+
"score": {},
56+
"alerts": alerts or [],
57+
}
58+
59+
60+
def _blocking_alert(key, alert_type="malware"):
61+
return {
62+
"key": key,
63+
"type": alert_type,
64+
"severity": "critical",
65+
"category": "supplyChainRisk",
66+
"props": {},
67+
"action": "error",
68+
}
69+
70+
71+
def _diff_response(**buckets):
72+
raw = json.loads((Path(__file__).parent.parent / "data/fullscans/diff/stream_diff.json").read_text())
73+
artifacts = {name: buckets.get(name, []) for name in BUCKETS}
74+
return StreamDiffResponse.from_dict({
75+
"success": raw["success"],
76+
"status": raw["status"],
77+
"data": {**raw["data"], "artifacts": artifacts},
78+
})
79+
80+
81+
def _build_diff(core, response):
82+
"""Mirror the package and diff assembly in Core.create_new_diff."""
83+
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
92+
93+
94+
def _handler(strict_blocking=True):
95+
config = Mock(spec=CliConfig)
96+
config.disable_blocking = False
97+
config.strict_blocking = strict_blocking
98+
config.legal_format = "socket"
99+
config.legal = False
100+
config.repo = None
101+
config.branch = None
102+
config.commit_sha = None
103+
return OutputHandler(config, Mock())
104+
105+
106+
def test_empty_baseline_lists_added_alerts_once(core):
107+
"""Against an empty baseline every package is added and none is unchanged."""
108+
diff = _build_diff(core, _diff_response(added=[
109+
_artifact("added", "a1", "alpha", [_blocking_alert("k1")]),
110+
_artifact("added", "a2", "beta", [_blocking_alert("k2")]),
111+
]))
112+
113+
assert len(diff.new_alerts) == 2
114+
assert diff.unchanged_alerts == []
115+
116+
handler = _handler()
117+
report = handler.build_json_report(diff)
118+
assert len(report["new_alerts"]) == 2
119+
assert not handler.report_pass(diff)
120+
121+
summary = handler.build_summary_text(diff)
122+
assert "NEW blocking issues: 2" in summary
123+
assert "EXISTING blocking issues" not in summary
124+
125+
126+
def test_real_baseline_lists_new_and_existing_alerts_once_each(core):
127+
"""With a baseline, added alerts are new and unchanged alerts are existing."""
128+
diff = _build_diff(core, _diff_response(
129+
added=[_artifact("added", "a1", "alpha", [_blocking_alert("k1")])],
130+
unchanged=[_artifact("unchanged", "u1", "gamma", [_blocking_alert("k3")])],
131+
))
132+
133+
assert [a.pkg_name for a in diff.new_alerts] == ["alpha"]
134+
assert [a.pkg_name for a in diff.unchanged_alerts] == ["gamma"]
135+
136+
handler = _handler()
137+
report = handler.build_json_report(diff)
138+
assert sorted(a["pkg_name"] for a in report["new_alerts"]) == ["alpha", "gamma"]
139+
assert not handler.report_pass(diff)
140+
141+
summary = handler.build_summary_text(diff)
142+
assert "NEW blocking issues: 1" in summary
143+
assert "EXISTING blocking issues: 1" in summary
144+
145+
146+
def test_real_baseline_fails_on_existing_alerts_alone(core):
147+
"""An unchanged blocking alert fails the run when the API returns it."""
148+
diff = _build_diff(core, _diff_response(
149+
unchanged=[_artifact("unchanged", "u1", "gamma", [_blocking_alert("k3")])],
150+
))
151+
152+
assert diff.new_alerts == []
153+
assert not _handler(strict_blocking=True).report_pass(diff)
154+
assert _handler(strict_blocking=False).report_pass(diff)
155+
156+
157+
def test_without_strict_blocking_output_is_unchanged(core):
158+
diff = _build_diff(core, _diff_response(
159+
added=[_artifact("added", "a1", "alpha", [_blocking_alert("k1")])],
160+
unchanged=[_artifact("unchanged", "u1", "gamma", [_blocking_alert("k3")])],
161+
))
162+
163+
report = _handler(strict_blocking=False).build_json_report(diff)
164+
assert [a["pkg_name"] for a in report["new_alerts"]] == ["alpha"]
165+
166+
167+
def test_unchanged_packages_are_keyed_by_artifact_id(core):
168+
"""Transitive unchanged alerts resolve their introducer through the ancestor ID."""
169+
core.sdk.fullscans.stream_diff.return_value = _diff_response(unchanged=[
170+
_artifact("unchanged", "top", "parent"),
171+
_artifact(
172+
"unchanged", "dep", "child", [_blocking_alert("k5")],
173+
direct=False, ancestors=["top"],
174+
),
175+
])
176+
added, removed, packages = core.get_added_and_removed_packages("head", "new")
177+
unchanged = Core.get_unchanged_packages(packages, added, removed)
178+
179+
assert set(unchanged) == {"top", "dep"}
180+
diff = core.create_diff_report(added, removed, unchanged)
181+
assert diff.unchanged_alerts[0].introduced_by == [("npm/parent@1.0.0", "package.json")]
182+
183+
184+
def _issue(name, alert_type="malware"):
185+
return Issue(
186+
pkg_type="npm",
187+
pkg_name=name,
188+
pkg_version="1.0.0",
189+
type=alert_type,
190+
purl=f"{name}@1.0.0",
191+
error=True,
192+
)
193+
194+
195+
def test_select_diff_alerts_skips_unchanged_alert_already_listed_as_new():
196+
diff = Diff()
197+
diff.new_alerts = [_issue("alpha")]
198+
diff.unchanged_alerts = [_issue("alpha"), _issue("alpha", "typosquat"), _issue("gamma")]
199+
200+
selected = select_diff_alerts(diff, strict_blocking=True)
201+
202+
assert [(a.pkg_name, a.type) for a in selected] == [
203+
("alpha", "malware"),
204+
("alpha", "typosquat"),
205+
("gamma", "malware"),
206+
]

‎uv.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)