Skip to content

Commit 2dba87d

Browse files
committed
Fix diff-scan poll so omit_license_details is honored
cached=true ignores omit_license_details per the Socket API, so poll for readiness with the cached endpoint and re-fetch the lean payload without cached when license details should be omitted (CE-224).
1 parent 8a0e2c8 commit 2dba87d

3 files changed

Lines changed: 86 additions & 18 deletions

File tree

socketsecurity/core/__init__.py

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,6 +1337,11 @@ def get_diff_scan_artifacts(
13371337
the backend computes, so the comparison survives network idle timeouts
13381338
(CE-354). See the DIFF_SCAN_POLL_* constants for the polling policy.
13391339
1340+
When ``include_license_details`` is False (the default), a final fetch
1341+
without ``cached`` requests ``omit_license_details=true``. The API
1342+
ignores that flag on cached responses, so the lean payload has to come
1343+
from a separate non-cached get (CE-224).
1344+
13401345
Requires an org token with the ``diff-scans:create``, ``diff-scans:list``
13411346
and ``full-scans:list`` scopes; callers are expected to catch failures and
13421347
fall back to the legacy streaming comparison.
@@ -1368,10 +1373,16 @@ def get_diff_scan_artifacts(
13681373
# which case the create response already carries the artifacts.
13691374
artifacts_dict = diff_scan.get("artifacts")
13701375

1371-
poll_params = {
1372-
"cached": "true",
1373-
"omit_license_details": "false" if include_license_details else "true",
1374-
}
1376+
# Poll with cached=true for short bounded 202/200 responses (CE-354).
1377+
# The API ignores omit_license_details whenever cached=true — cached
1378+
# payloads always embed full license data — so readiness polling never
1379+
# requests it. When license details should be omitted (the default;
1380+
# CE-224), the lean payload is fetched separately below without cached.
1381+
poll_params = {"cached": "true"}
1382+
if not include_license_details:
1383+
# Keep the readiness response small so the ignored omit doesn't
1384+
# reintroduce large-response truncation while we wait for 200.
1385+
poll_params["omit_unchanged"] = "true"
13751386
deadline = time.monotonic() + DIFF_SCAN_POLL_TIMEOUT_SECONDS
13761387
interval = DIFF_SCAN_POLL_INITIAL_INTERVAL_SECONDS
13771388
while artifacts_dict is None:
@@ -1404,6 +1415,21 @@ def get_diff_scan_artifacts(
14041415
time.sleep(interval)
14051416
interval = min(interval * DIFF_SCAN_POLL_BACKOFF_MULTIPLIER, DIFF_SCAN_POLL_MAX_INTERVAL_SECONDS)
14061417

1418+
# Cached results always include license details. Re-fetch once without
1419+
# cached so omit_license_details is honored and the diff stays lean.
1420+
if not include_license_details:
1421+
response = self.sdk.diffscans.get(
1422+
self.config.org_slug,
1423+
diff_scan_id,
1424+
params={"omit_license_details": "true"},
1425+
)
1426+
scan = response.get("diff_scan") or {}
1427+
if scan.get("artifacts") is None:
1428+
raise Exception(
1429+
f"Error fetching diff scan {diff_scan_id}: unexpected response: {str(response)[:500]}"
1430+
)
1431+
artifacts_dict = scan["artifacts"]
1432+
14071433
return DiffArtifacts.from_dict({
14081434
key: artifacts_dict.get(key) or []
14091435
for key in ("added", "removed", "unchanged", "replaced", "updated")

tests/core/test_diff_scan_polling.py

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,14 @@ def no_sleep(mocker):
2626
def test_polls_until_diff_scan_ready(core, diff_scan_get_response, no_sleep):
2727
"""202 processing responses are polled through until the 200 result arrives."""
2828
processing = {"status": "processing", "id": "diff-scan-123"}
29-
core.sdk.diffscans.get.side_effect = [processing, processing, diff_scan_get_response]
29+
# Final cached poll + lean omit_license_details re-fetch.
30+
core.sdk.diffscans.get.side_effect = [
31+
processing, processing, diff_scan_get_response, diff_scan_get_response
32+
]
3033

3134
artifacts = core.get_diff_scan_artifacts("head", "new")
3235

33-
assert core.sdk.diffscans.get.call_count == 3
36+
assert core.sdk.diffscans.get.call_count == 4
3437
assert no_sleep.call_count == 2 # slept between polls, never during them
3538
assert len(artifacts.added) > 0
3639

@@ -40,7 +43,9 @@ def test_poll_interval_backs_off(core, diff_scan_get_response, no_sleep, monkeyp
4043
monkeypatch.setattr(core_module, "DIFF_SCAN_POLL_INITIAL_INTERVAL_SECONDS", 4.0)
4144
monkeypatch.setattr(core_module, "DIFF_SCAN_POLL_MAX_INTERVAL_SECONDS", 10.0)
4245
processing = {"status": "processing", "id": "diff-scan-123"}
43-
core.sdk.diffscans.get.side_effect = [processing] * 4 + [diff_scan_get_response]
46+
core.sdk.diffscans.get.side_effect = (
47+
[processing] * 4 + [diff_scan_get_response, diff_scan_get_response]
48+
)
4449

4550
core.get_diff_scan_artifacts("head", "new")
4651

@@ -50,11 +55,13 @@ def test_poll_interval_backs_off(core, diff_scan_get_response, no_sleep, monkeyp
5055

5156
def test_transient_poll_error_is_retried(core, diff_scan_get_response, no_sleep):
5257
"""A dropped poll doesn't abandon the flow - the diff keeps computing server-side."""
53-
core.sdk.diffscans.get.side_effect = [APIConnectionError("reset"), diff_scan_get_response]
58+
core.sdk.diffscans.get.side_effect = [
59+
APIConnectionError("reset"), diff_scan_get_response, diff_scan_get_response
60+
]
5461

5562
artifacts = core.get_diff_scan_artifacts("head", "new")
5663

57-
assert core.sdk.diffscans.get.call_count == 2
64+
assert core.sdk.diffscans.get.call_count == 3
5865
assert len(artifacts.added) > 0
5966

6067

@@ -76,15 +83,45 @@ def test_poll_timeout_raises(core, no_sleep, monkeypatch):
7683

7784

7885
def test_duplicate_redirect_uses_embedded_artifacts(core, diff_scan_get_response):
79-
"""An on_duplicate redirect can return the computed diff scan straight away."""
86+
"""An on_duplicate redirect skips readiness polling; lean re-fetch still runs."""
8087
core.sdk.diffscans.create_from_ids.return_value = diff_scan_get_response
8188

8289
artifacts = core.get_diff_scan_artifacts("head", "new")
8390

84-
core.sdk.diffscans.get.assert_not_called()
91+
# Create already carried artifacts, so cached readiness polling is skipped,
92+
# but omit_license_details still needs a non-cached get (cached ignores it).
93+
core.sdk.diffscans.get.assert_called_once_with(
94+
core.config.org_slug,
95+
"diff-scan-123",
96+
params={"omit_license_details": "true"},
97+
)
8598
assert len(artifacts.added) > 0
8699

87100

101+
def test_lean_refetch_omits_license_details_without_cached(
102+
core, diff_scan_get_response, no_sleep
103+
):
104+
"""omit_license_details is fetched without cached=true (API ignores it otherwise)."""
105+
processing = {"status": "processing", "id": "diff-scan-123"}
106+
core.sdk.diffscans.get.side_effect = [
107+
processing, diff_scan_get_response, diff_scan_get_response
108+
]
109+
110+
core.get_diff_scan_artifacts("head", "new")
111+
112+
assert core.sdk.diffscans.get.call_args_list[0].kwargs["params"] == {
113+
"cached": "true",
114+
"omit_unchanged": "true",
115+
}
116+
assert core.sdk.diffscans.get.call_args_list[1].kwargs["params"] == {
117+
"cached": "true",
118+
"omit_unchanged": "true",
119+
}
120+
assert core.sdk.diffscans.get.call_args_list[2].kwargs["params"] == {
121+
"omit_license_details": "true",
122+
}
123+
124+
88125
def test_fallback_to_streaming_diff_on_failure(core):
89126
"""If the diff-scans flow fails (e.g. token missing the diff-scans scopes),
90127
the comparison falls back to the legacy streaming diff transparently."""

tests/core/test_sdk_methods.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -239,12 +239,15 @@ def test_get_added_and_removed_packages(core):
239239
# include_license_details defaults to False: the diff path never consumes
240240
# embedded license data (license artifacts come from the PURL endpoint), so
241241
# requesting it only bloats the response and risks the truncation
242-
# crash on large repos.
243-
core.sdk.diffscans.get.assert_called_once_with(
244-
core.config.org_slug,
245-
"diff-scan-123",
246-
params={"cached": "true", "omit_license_details": "true"},
247-
)
242+
# crash on large repos. cached=true ignores omit_license_details, so the
243+
# poll checks readiness (optionally omitting unchanged to stay small) and
244+
# a separate non-cached get fetches the lean payload.
245+
get_calls = core.sdk.diffscans.get.call_args_list
246+
assert len(get_calls) == 2
247+
assert get_calls[0].args == (core.config.org_slug, "diff-scan-123")
248+
assert get_calls[0].kwargs["params"] == {"cached": "true", "omit_unchanged": "true"}
249+
assert get_calls[1].args == (core.config.org_slug, "diff-scan-123")
250+
assert get_calls[1].kwargs["params"] == {"omit_license_details": "true"}
248251
core.sdk.fullscans.stream_diff.assert_not_called()
249252

250253
# Verify the results
@@ -263,10 +266,12 @@ def test_get_added_and_removed_packages_license_override(core):
263266
"""The include_license_details override seam still works when explicitly requested."""
264267
core.get_added_and_removed_packages("head", "new", include_license_details=True)
265268

269+
# When license details are wanted, the cached poll response is used directly
270+
# — no lean re-fetch, and omit_license_details is not sent.
266271
core.sdk.diffscans.get.assert_called_once_with(
267272
core.config.org_slug,
268273
"diff-scan-123",
269-
params={"cached": "true", "omit_license_details": "false"},
274+
params={"cached": "true"},
270275
)
271276

272277
def test_empty_alerts_preserved(core):

0 commit comments

Comments
 (0)