Fix: scan_packages.py --fix crash on download_packages() tuple return (#6413)

* Fix scan_packages.py --fix crash on download_packages() tuple return

`download_packages()` returns `(results, download_errors)`, but the two
`--fix`-path call sites still treated the return value as the bare results
list. `find_safe_version` did `downloaded = download_packages(...)` followed
by `if not downloaded:` (always false: a 2-tuple is truthy) and
`for _, archive_path in downloaded:`, which unpacked the results list into
two variables -> ValueError in the normal single-archive `--no-deps` case.
`_run_fix` indexed `downloaded[0][1]`, i.e. the second archive of the results
list instead of the first archive's path -> IndexError. So `--fix` crashed
exactly when a CRITICAL finding needed remediation. The main scan path already
unpacks the tuple; this aligns the two `--fix` sites with it.

Adds CPU-only regression tests for both sites.

Closes #6412

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Update scripts/scan_packages.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update scripts/scan_packages.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This commit is contained in:
Parvesh Saini 2026-06-18 18:30:07 +05:30 committed by GitHub
commit 892d2983b0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 65 additions and 2 deletions

View file

@ -2154,8 +2154,10 @@ def find_safe_version(
scan_dir = os.path.join(tmpdir, f"{name}_{ver}")
os.makedirs(scan_dir, exist_ok = True)
downloaded = download_packages([spec], scan_dir)
downloaded, download_errors = download_packages([spec], scan_dir)
if not downloaded:
for err in download_errors:
print(f" [WARN] {err}", file = sys.stderr)
continue
clean = True
@ -2288,9 +2290,12 @@ def _run_fix(critical_pkgs: set[str], entries: list[dict], max_search: int) -> N
# If no pinned version, download to find what pip resolves
dl_dir = os.path.join(tmpdir, f"resolve_{pkg_name}")
os.makedirs(dl_dir, exist_ok = True)
downloaded = download_packages([pkg_name], dl_dir)
downloaded, download_errors = download_packages([pkg_name], dl_dir)
if downloaded:
current_ver = get_downloaded_version(downloaded[0][1])
else:
for err in download_errors:
print(f" [WARN] {err}", file = sys.stderr)
shutil.rmtree(dl_dir, ignore_errors = True)
if not current_ver:

View file

@ -548,3 +548,61 @@ def test_per_spec_sdist_only_is_not_error(tmp_path, monkeypatch):
sp._resolve_per_spec_with_deps(["x==1.0.0"], str(tmp_path), {}, errors)
assert errors == [] # sdist-only handled, not an exit-2 failure
assert any(p.name.endswith(".tar.gz") for p in tmp_path.iterdir())
# ---------------------------------------------------------------------------
# --fix path: download_packages() returns (results, download_errors); both
# --fix call sites must unpack the tuple, not treat it as the results list.
# ---------------------------------------------------------------------------
def test_find_safe_version_handles_download_tuple(monkeypatch):
# One downloaded archive, returned as the real (results, download_errors) tuple.
monkeypatch.setattr(sp, "fetch_pypi_versions", lambda name: ["0.9.0", "1.0.0"])
monkeypatch.setattr(
sp,
"download_packages",
lambda specs, dest, **kw: ([("foo==0.9.0", "/tmp/foo-0.9.0.whl")], []),
)
monkeypatch.setattr(sp, "scan_archive", lambda archive_path, name: []) # clean
monkeypatch.setattr(sp.os, "makedirs", lambda *a, **k: None)
monkeypatch.setattr(sp.os, "remove", lambda *a, **k: None)
monkeypatch.setattr(sp.shutil, "rmtree", lambda *a, **k: None)
# bad_ver 1.0.0 -> the only older candidate is 0.9.0, which is clean.
result = sp.find_safe_version("foo", "1.0.0", "/tmp/ignored", max_search = 10)
assert result == "0.9.0"
def test_run_fix_uses_first_archive_path(monkeypatch):
monkeypatch.setattr(
sp,
"download_packages",
lambda specs, dest, **kw: ([("foo", "/tmp/foo-1.2.3.whl")], []),
)
seen = {}
def fake_get_downloaded_version(path):
seen["path"] = path
return "1.2.3"
monkeypatch.setattr(sp, "get_downloaded_version", fake_get_downloaded_version)
monkeypatch.setattr(sp, "find_safe_version", lambda *a, **k: None)
monkeypatch.setattr(sp.os, "makedirs", lambda *a, **k: None)
monkeypatch.setattr(sp.shutil, "rmtree", lambda *a, **k: None)
# CRITICAL package with no pinned version -> must download to resolve it,
# reaching downloaded[0][1] (the first archive's path).
entries = [
{
"name": "foo",
"is_git": False,
"spec": "foo",
"source_file": None,
"raw_line": "foo",
"line_num": 1,
}
]
sp._run_fix({"foo"}, entries, max_search = 10) # must not raise
assert seen.get("path") == "/tmp/foo-1.2.3.whl"