Skip to content

Commit c2f9af2

Browse files
arpitjain099radoering
authored andcommitted
Fail closed when a locked hash cannot be checked
Two layers bind an installed artifact to the hashes in poetry.lock, and an index could skip both. The chooser accepted any link that advertised no hash at all, without comparing anything to the locked hashes, so 'no hash offered' was treated as 'accept' rather than 'compute and compare'. Removing that early accept lets the link fall through to the existing calculate_sha256 path, which is what the surrounding code already does for links advertising a different hash type. A link whose computed hash matches the lock is still chosen. The executor's post-download check only ran when the downloaded archive's filename appeared in the lock's files list, so an artifact served under an unlisted filename was never validated and its hash was never recorded. The filename condition is gone; if the lock pins hashes, the archive is checked. An archive the lock does not list now raises with that stated plainly, rather than falling through the empty-known-hashes case into the less specific 'no usable hash type' message. Both new tests fail without the corresponding change. Signed-off-by: Arpit Jain <arpitjain099@gmail.com> (cherry picked from commit a165d51)
1 parent 3a194dd commit c2f9af2

4 files changed

Lines changed: 99 additions & 5 deletions

File tree

‎src/poetry/installation/chooser.py‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,8 @@ def _get_links(self, package: Package) -> list[Link]:
215215
skipped = []
216216
locked_hash_names = {h.split(":")[0] for h in locked_hashes}
217217
for link in links:
218-
if not link.hashes:
219-
selected_links.append(link)
220-
continue
221-
218+
# A link that advertises no hash is not trusted: the lock file requires
219+
# a hash for this package, so fall through and compute one to compare.
222220
link_hash: str | None = None
223221
if (candidates := locked_hash_names.intersection(link.hashes.keys())) and (
224222
hash_name := get_highest_priority_hash_type(candidates, link.filename)

‎src/poetry/installation/executor.py‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -787,7 +787,7 @@ def _download_link(self, operation: Install | Update, link: Link) -> Path:
787787
return archive
788788

789789
def _populate_hashes_dict(self, archive: Path, package: Package) -> None:
790-
if package.files and archive.name in {f["file"] for f in package.files}:
790+
if package.files:
791791
archive_hash = self._validate_archive_hash(archive, package)
792792
self._hashes[package.name] = archive_hash
793793

@@ -797,6 +797,14 @@ def _validate_archive_hash(archive: Path, package: Package) -> str:
797797
hash_types = {t.split(":")[0] for t in known_hashes}
798798
hash_type = get_highest_priority_hash_type(hash_types, archive.name)
799799

800+
if not known_hashes:
801+
locked_files = sorted(f["file"] for f in package.files)
802+
raise RuntimeError(
803+
f"Downloaded archive {archive.name} for {package} is not listed in"
804+
f" the lock file, so its hash cannot be verified"
805+
f" (locked files: {locked_files})"
806+
)
807+
800808
if hash_type is None:
801809
raise RuntimeError(
802810
f"No usable hash type(s) for {package} from archive"

‎tests/installation/test_chooser.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,16 @@
1010

1111
from poetry.console.exceptions import PoetryRuntimeError
1212
from poetry.installation.chooser import Chooser
13+
from poetry.core.packages.utils.link import Link
1314
from poetry.repositories.legacy_repository import LegacyRepository
1415
from poetry.utils.env import MockEnv
1516

1617

1718
if TYPE_CHECKING:
1819
from poetry.core.packages.package import PackageFile
1920

21+
from pytest_mock import MockerFixture
22+
2023
from poetry.repositories.repository_pool import RepositoryPool
2124
from tests.conftest import Config
2225
from tests.types import DistributionHashGetter
@@ -323,6 +326,60 @@ def test_chooser_does_not_choose_yanked_if_others(
323326
assert link_partial_yank.filename == "futures-3.2.0.tar.gz"
324327

325328

329+
LOCKED_SHA = "sha256:" + "1" * 64
330+
OTHER_SHA = "sha256:" + "2" * 64
331+
332+
333+
def _hashless_package_and_pool(
334+
pool: RepositoryPool, mocker: MockerFixture, computed: str
335+
) -> Package:
336+
"""Point the pool at a single link that advertises no hash at all, and make
337+
the repository compute `computed` for it.
338+
"""
339+
package = Package(
340+
"isort",
341+
"4.3.4",
342+
source_type="legacy",
343+
source_reference="foo",
344+
source_url="https://legacy.foo.bar/simple/",
345+
)
346+
package.files = [{"file": "isort-4.3.4.tar.gz", "hash": LOCKED_SHA}]
347+
348+
hashless = Link("https://legacy.foo.bar/isort-4.3.4.tar.gz")
349+
assert not hashless.hashes
350+
351+
mocker.patch.object(
352+
LegacyRepository, "find_links_for_package", return_value=[hashless]
353+
)
354+
mocker.patch.object(LegacyRepository, "calculate_sha256", return_value=computed)
355+
return package
356+
357+
358+
def test_chooser_rejects_a_hashless_link_whose_computed_hash_does_not_match(
359+
env: MockEnv, pool: RepositoryPool, mocker: MockerFixture
360+
) -> None:
361+
"""An index that omits the hash fragment must not bypass the locked hashes."""
362+
package = _hashless_package_and_pool(pool, mocker, OTHER_SHA)
363+
364+
with pytest.raises(PoetryRuntimeError) as e:
365+
Chooser(pool, env).choose_for(package)
366+
367+
assert "did not match any known checksums" in str(e.value)
368+
369+
370+
def test_chooser_accepts_a_hashless_link_whose_computed_hash_matches(
371+
env: MockEnv, pool: RepositoryPool, mocker: MockerFixture
372+
) -> None:
373+
"""Computing the hash is the point, so a hashless link that hashes to the
374+
locked value is still installable.
375+
"""
376+
package = _hashless_package_and_pool(pool, mocker, LOCKED_SHA)
377+
378+
link = Chooser(pool, env).choose_for(package)
379+
380+
assert link.filename == "isort-4.3.4.tar.gz"
381+
382+
326383
@pytest.mark.parametrize("source_type", ["", "legacy"])
327384
def test_chooser_throws_an_error_if_package_hashes_do_not_match(
328385
env: MockEnv,

‎tests/installation/test_executor.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1929,6 +1929,37 @@ def test_executor_known_hashes(
19291929
verify_installed_distribution(tmp_venv, package, expected_url_reference)
19301930

19311931

1932+
def test_executor_rejects_an_archive_not_listed_in_the_lock_file(
1933+
tmp_venv: VirtualEnv,
1934+
pool: RepositoryPool,
1935+
config: Config,
1936+
io: BufferedIO,
1937+
fixture_dir: FixtureDirGetter,
1938+
) -> None:
1939+
"""The served filename must not decide whether the hash is checked. A lock
1940+
file that pins hashes has to reject an archive it does not list.
1941+
"""
1942+
url = (fixture_dir("distributions") / "demo-0.1.0.tar.gz").resolve()
1943+
package = Package("demo", "0.1.0", source_type="file", source_url=url.as_posix())
1944+
# The lock pins a hash, but under a filename the served archive does not use.
1945+
package.files = [
1946+
{
1947+
"file": "demo-0.1.0-py2.py3-none-any.whl",
1948+
"hash": "sha256:" + "0" * 64,
1949+
},
1950+
]
1951+
1952+
executor = Executor(tmp_venv, pool, config, io)
1953+
return_code = executor.execute([Install(package)])
1954+
1955+
distributions = list(tmp_venv.site_packages.distributions(name=package.name))
1956+
assert len(distributions) == 0
1957+
1958+
output = io.fetch_output()
1959+
assert return_code == 1, f"\noutput: {output}\n"
1960+
assert "is not listed in the lock file" in output
1961+
1962+
19321963
def test_executor_no_supported_hash_types(
19331964
tmp_venv: VirtualEnv,
19341965
pool: RepositoryPool,

0 commit comments

Comments
 (0)