Skip to content

Commit 60183b1

Browse files
committed
Keep namespaced package changes distinct
Retain npm scopes and Maven groups when deduplicating package changes. Keep existing filtering, order, and true-duplicate behavior. Assisted-by: Codex:GPT-6
1 parent c397151 commit 60183b1

2 files changed

Lines changed: 319 additions & 9 deletions

File tree

‎socketsecurity/core/__init__.py‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2599,7 +2599,7 @@ def create_diff_report(
25992599
alerts_in_removed_packages: Dict[str, List[Issue]] = {}
26002600
alerts_in_unchanged_packages: Dict[str, List[Issue]] = {}
26012601

2602-
seen_packages = {
2602+
seen_packages: Dict[str, Set[Tuple[str, str, str, str]]] = {
26032603
"added": set(),
26042604
"updated": set(),
26052605
"removed": set(),
@@ -2608,13 +2608,21 @@ def create_diff_report(
26082608

26092609
for package_id, package in added_packages.items():
26102610
purl = self.create_purl(package_id, added_packages)
2611-
base_purl = f"{purl.ecosystem}/{purl.name}@{purl.version}"
2611+
package_identity = (
2612+
purl.ecosystem,
2613+
package.namespace or "",
2614+
purl.name,
2615+
purl.version,
2616+
)
26122617

26132618
change_type = "updated" if package.diffType == "updated" else "added"
26142619
target = diff.updated_packages if change_type == "updated" else diff.new_packages
2615-
if (not direct_only or package.direct) and base_purl not in seen_packages[change_type]:
2620+
if (
2621+
(not direct_only or package.direct)
2622+
and package_identity not in seen_packages[change_type]
2623+
):
26162624
target.append(purl)
2617-
seen_packages[change_type].add(base_purl)
2625+
seen_packages[change_type].add(package_identity)
26182626

26192627
self.add_package_alerts_to_collection(
26202628
package=package,
@@ -2624,13 +2632,21 @@ def create_diff_report(
26242632

26252633
for package_id, package in removed_packages.items():
26262634
purl = self.create_purl(package_id, removed_packages)
2627-
base_purl = f"{purl.ecosystem}/{purl.name}@{purl.version}"
2635+
package_identity = (
2636+
purl.ecosystem,
2637+
package.namespace or "",
2638+
purl.name,
2639+
purl.version,
2640+
)
26282641

26292642
change_type = "replaced" if package.diffType == "replaced" else "removed"
26302643
target = diff.replaced_packages if change_type == "replaced" else diff.removed_packages
2631-
if (not direct_only or package.direct) and base_purl not in seen_packages[change_type]:
2644+
if (
2645+
(not direct_only or package.direct)
2646+
and package_identity not in seen_packages[change_type]
2647+
):
26322648
target.append(purl)
2633-
seen_packages[change_type].add(base_purl)
2649+
seen_packages[change_type].add(package_identity)
26342650

26352651
self.add_package_alerts_to_collection(
26362652
package=package,

‎tests/core/test_json_package_changes_transport.py‎

Lines changed: 296 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@
33
from __future__ import annotations
44

55
from collections.abc import Iterator
6+
from dataclasses import replace
67

78
import pytest
89
from socketdev import socketdev
910
from socketdev.fullscans import FullScanParams
1011

1112
from socketsecurity.config import CliConfig
1213
from socketsecurity.core import Core
14+
from socketsecurity.core.classes import Package
1315
from socketsecurity.core.report_output import build_socket_json_report
1416
from socketsecurity.core.socket_config import SocketConfig
1517
from tests.support.socket_api_fixture import (
@@ -57,6 +59,7 @@ def _artifact(
5759
*,
5860
direct: bool = True,
5961
namespace: str | None = None,
62+
ecosystem: str | None = None,
6063
score: dict[str, int | float | None] | None = None,
6164
parent_id: str | None = None,
6265
) -> dict[str, object]:
@@ -71,7 +74,7 @@ def _artifact(
7174
artifact: dict[str, object] = {
7275
"diffType": change_type,
7376
"id": artifact_id,
74-
"type": "npm" if namespace else "pypi",
77+
"type": ecosystem or ("npm" if namespace else "pypi"),
7578
"name": name,
7679
"version": version,
7780
"alerts": [],
@@ -81,7 +84,7 @@ def _artifact(
8184
reference
8285
],
8386
}
84-
if namespace:
87+
if namespace is not None:
8588
artifact["namespace"] = namespace
8689
if score is not None:
8790
artifact["score"] = score
@@ -135,6 +138,297 @@ def _stream_response(
135138
}
136139

137140

141+
def _namespace_identity_artifacts() -> dict[str, list[dict[str, object]]]:
142+
def artifact(
143+
artifact_id: str,
144+
change_type: str,
145+
ecosystem: str,
146+
namespace: str | None,
147+
name: str,
148+
version: str,
149+
quality: float,
150+
*,
151+
direct: bool = True,
152+
parent_id: str | None = None,
153+
) -> dict[str, object]:
154+
score = _score()
155+
score["quality"] = quality
156+
return _artifact(
157+
artifact_id,
158+
change_type,
159+
name,
160+
version,
161+
direct=direct,
162+
namespace=namespace,
163+
ecosystem=ecosystem,
164+
score=score,
165+
parent_id=parent_id,
166+
)
167+
168+
return {
169+
"added": [
170+
artifact("added-one", "added", "npm", "@one", "widget", "1.0.0", 0.1),
171+
artifact("added-two", "added", "npm", "@two", "widget", "1.0.0", 0.2),
172+
artifact("added-duplicate", "added", "npm", "@one", "widget", "1.0.0", 0.3),
173+
artifact("added-unscoped", "added", "npm", None, "widget", "1.0.0", 0.4),
174+
artifact("added-pypi", "added", "pypi", None, "widget", "1.0.0", 0.5),
175+
artifact(
176+
"child-transitive",
177+
"added",
178+
"npm",
179+
"@child",
180+
"leaf",
181+
"2.0.0",
182+
0.6,
183+
direct=False,
184+
parent_id="added-one",
185+
),
186+
artifact("child-direct", "added", "npm", "@child", "leaf", "2.0.0", 0.7),
187+
],
188+
"updated": [
189+
artifact(
190+
"updated-one", "updated", "maven", "com.one", "widget", "3.0.0", 0.1
191+
),
192+
artifact(
193+
"updated-two", "updated", "maven", "com.two", "widget", "3.0.0", 0.2
194+
),
195+
artifact(
196+
"updated-duplicate",
197+
"updated",
198+
"maven",
199+
"com.one",
200+
"widget",
201+
"3.0.0",
202+
0.3,
203+
),
204+
],
205+
"removed": [
206+
artifact(
207+
"removed-cross-bucket", "removed", "npm", "@one", "widget", "1.0.0", 0.8
208+
),
209+
artifact("removed-one", "removed", "npm", "@one", "retired", "4.0.0", 0.1),
210+
artifact("removed-two", "removed", "npm", "@two", "retired", "4.0.0", 0.2),
211+
artifact(
212+
"removed-duplicate", "removed", "npm", "@one", "retired", "4.0.0", 0.3
213+
),
214+
],
215+
"replaced": [
216+
artifact(
217+
"replaced-one", "replaced", "maven", "com.one", "retired", "5.0.0", 0.1
218+
),
219+
artifact(
220+
"replaced-two", "replaced", "maven", "com.two", "retired", "5.0.0", 0.2
221+
),
222+
artifact(
223+
"replaced-duplicate",
224+
"replaced",
225+
"maven",
226+
"com.one",
227+
"retired",
228+
"5.0.0",
229+
0.3,
230+
),
231+
],
232+
"unchanged": [],
233+
}
234+
235+
236+
@pytest.mark.parametrize("streaming_fallback", [False, True])
237+
def test_package_change_identity_keeps_namespace_and_bucket_boundaries(
238+
socket_api: SocketApiFixture,
239+
streaming_fallback: bool,
240+
) -> None:
241+
core = _core(socket_api)
242+
artifacts = _namespace_identity_artifacts()
243+
if streaming_fallback:
244+
socket_api.route(
245+
"POST",
246+
"orgs/fixture-org/diff-scans/from-ids",
247+
SocketResponse(403, {"error": {"message": "forbidden"}}),
248+
)
249+
socket_api.route(
250+
"GET",
251+
"orgs/fixture-org/full-scans/diff",
252+
SocketResponse(200, _stream_response(artifacts)),
253+
)
254+
else:
255+
socket_api.route(
256+
"POST",
257+
"orgs/fixture-org/diff-scans/from-ids",
258+
SocketResponse(201, {"diff_scan": {"id": "diff-namespace-fixture"}}),
259+
)
260+
socket_api.route(
261+
"GET",
262+
"orgs/fixture-org/diff-scans/diff-namespace-fixture",
263+
SocketResponse(
264+
200,
265+
{
266+
"diff_scan": {
267+
"artifacts": artifacts,
268+
"before_full_scan": {"id": "baseline-id"},
269+
"after_full_scan": {"id": "new-scan-id"},
270+
}
271+
},
272+
),
273+
)
274+
275+
comparison = core._get_package_comparison(
276+
"baseline-id", "new-scan-id", include_license_details=False
277+
)
278+
request_count = len(socket_api.requests)
279+
diff = core.create_diff_report(
280+
comparison.added_packages,
281+
comparison.removed_packages,
282+
{},
283+
)
284+
report = build_socket_json_report(
285+
diff,
286+
CliConfig(api_token="fictional-local-only", repo="fixture-repo"),
287+
)
288+
289+
expected_ids = {
290+
"new_packages": [
291+
"added-one",
292+
"added-two",
293+
"added-unscoped",
294+
"added-pypi",
295+
"child-direct",
296+
],
297+
"updated_packages": ["updated-one", "updated-two"],
298+
"removed_packages": [
299+
"removed-cross-bucket",
300+
"removed-one",
301+
"removed-two",
302+
],
303+
"replaced_packages": ["replaced-one", "replaced-two"],
304+
}
305+
assert {
306+
"new_packages": [package.id for package in diff.new_packages],
307+
"updated_packages": [package.id for package in diff.updated_packages],
308+
"removed_packages": [package.id for package in diff.removed_packages],
309+
"replaced_packages": [package.id for package in diff.replaced_packages],
310+
} == expected_ids
311+
assert {
312+
key: [row["id"] for row in report[key]] for key in expected_ids
313+
} == expected_ids
314+
315+
new_rows = {row["id"]: row for row in report["new_packages"]}
316+
assert new_rows["added-one"]["purl"] == "@one/widget@1.0.0"
317+
assert new_rows["added-two"]["purl"] == "@two/widget@1.0.0"
318+
assert new_rows["added-unscoped"]["purl"] == "widget@1.0.0"
319+
assert new_rows["added-pypi"]["ecosystem"] == "pypi"
320+
assert new_rows["added-one"]["url"] == (
321+
"https://socket.dev/npm/package/@one/widget/overview/1.0.0"
322+
)
323+
assert new_rows["added-two"]["url"] == (
324+
"https://socket.dev/npm/package/@two/widget/overview/1.0.0"
325+
)
326+
assert new_rows["added-one"]["scores"]["quality"] == 0.1
327+
assert new_rows["added-two"]["scores"]["quality"] == 0.2
328+
assert "previousVersion" not in new_rows["added-one"]
329+
330+
updated_rows = {row["id"]: row for row in report["updated_packages"]}
331+
assert updated_rows["updated-one"]["purl"] == "com.one/widget@3.0.0"
332+
assert updated_rows["updated-two"]["purl"] == "com.two/widget@3.0.0"
333+
assert updated_rows["updated-one"]["url"] == (
334+
"https://socket.dev/maven/package/com.one/widget/overview/3.0.0"
335+
)
336+
assert updated_rows["updated-two"]["url"] == (
337+
"https://socket.dev/maven/package/com.two/widget/overview/3.0.0"
338+
)
339+
assert updated_rows["updated-one"]["scores"]["quality"] == 0.1
340+
assert updated_rows["updated-two"]["scores"]["quality"] == 0.2
341+
342+
removed_rows = {row["id"]: row for row in report["removed_packages"]}
343+
assert removed_rows["removed-one"]["purl"] == "@one/retired@4.0.0"
344+
assert removed_rows["removed-two"]["purl"] == "@two/retired@4.0.0"
345+
assert removed_rows["removed-one"]["url"] == (
346+
"https://socket.dev/npm/package/@one/retired/overview/4.0.0"
347+
)
348+
assert removed_rows["removed-two"]["url"] == (
349+
"https://socket.dev/npm/package/@two/retired/overview/4.0.0"
350+
)
351+
assert removed_rows["removed-one"]["scores"]["quality"] == 0.1
352+
assert removed_rows["removed-two"]["scores"]["quality"] == 0.2
353+
354+
replaced_rows = {row["id"]: row for row in report["replaced_packages"]}
355+
assert replaced_rows["replaced-one"]["purl"] == "com.one/retired@5.0.0"
356+
assert replaced_rows["replaced-two"]["purl"] == "com.two/retired@5.0.0"
357+
assert replaced_rows["replaced-one"]["url"] == (
358+
"https://socket.dev/maven/package/com.one/retired/overview/5.0.0"
359+
)
360+
assert replaced_rows["replaced-two"]["url"] == (
361+
"https://socket.dev/maven/package/com.two/retired/overview/5.0.0"
362+
)
363+
assert replaced_rows["replaced-one"]["scores"]["quality"] == 0.1
364+
assert replaced_rows["replaced-two"]["scores"]["quality"] == 0.2
365+
366+
assert report["new_alerts"] == []
367+
assert set(report) == {
368+
"scan_failed",
369+
"new_alerts",
370+
"full_scan_id",
371+
"diff_url",
372+
"report_url",
373+
"diff_scan_id",
374+
"old_scan_id",
375+
"new_scan_id",
376+
"old_scan_url",
377+
"new_scan_url",
378+
"repo",
379+
"branch",
380+
"commit_sha",
381+
"legal_mode",
382+
"new_packages",
383+
"updated_packages",
384+
"removed_packages",
385+
"replaced_packages",
386+
}
387+
assert len(socket_api.requests) == request_count
388+
389+
direct_only_false = core.create_diff_report(
390+
comparison.added_packages,
391+
comparison.removed_packages,
392+
{},
393+
direct_only=False,
394+
)
395+
assert [package.id for package in direct_only_false.new_packages] == [
396+
"added-one",
397+
"added-two",
398+
"added-unscoped",
399+
"added-pypi",
400+
"child-transitive",
401+
]
402+
403+
404+
def test_package_change_identity_treats_missing_and_empty_namespace_equally(
405+
socket_api: SocketApiFixture,
406+
) -> None:
407+
core = _core(socket_api)
408+
package = Package(
409+
id="source",
410+
name="widget",
411+
version="1.0.0",
412+
type="npm",
413+
score={},
414+
alerts=[],
415+
direct=True,
416+
manifestFiles=[{"file": "package.json"}],
417+
)
418+
missing_namespace = replace(package, id="missing-namespace", namespace=None)
419+
empty_namespace = replace(package, id="empty-namespace", namespace="")
420+
421+
diff = core.create_diff_report(
422+
{
423+
missing_namespace.id: missing_namespace,
424+
empty_namespace.id: empty_namespace,
425+
},
426+
{},
427+
)
428+
429+
assert [package.id for package in diff.new_packages] == ["missing-namespace"]
430+
431+
138432
@pytest.mark.parametrize("streaming_fallback", [False, True])
139433
def test_json_contains_real_classified_packages_without_extra_transport(
140434
socket_api: SocketApiFixture,

0 commit comments

Comments
 (0)