Add safeguards against corrupt repodata - #909
Draft
maresb wants to merge 11 commits into
Draft
Conversation
Capture a real ``mamba 2.6.0 create --dry-run --json`` output so later commits in this series have a concrete fixture to test against. The notable shape is that mamba 2.6.0 emits a rich ``LINK`` action (with ``url`` / ``md5`` / ``sha256`` / ``depends`` / ... already populated) and an empty ``FETCH`` list whenever the target packages are already present in ``CONDA_PKGS_DIRS``. This is the failure surface for conda#906: conda-lock's package-plan extraction reads from ``FETCH``, so a LINK-only dryrun produces an empty plan. The next commit adds the LINK->FETCH normalization that recovers the plan from rich LINK metadata.
Mamba/micromamba 2.6.0 emits LINK actions that already carry every field conda-lock reads from a FETCH: ``url``, ``fn``, ``md5``, ``sha256``, ``depends``, ``constrains``, ``subdir``, ``timestamp`` ... When that is the case there is no need to crack open ``repodata_record.json`` on disk -- and when the package is already present in ``CONDA_PKGS_DIRS`` mamba 2.6.0 emits an empty FETCH list, so the previous "always go to disk for any LINK without a matching FETCH" path produced an empty package plan. Add ``conda_lock.solver.dry_run.link_action_as_fetch`` which reuses a rich LINK action as a FETCH when every required field is present. Synthesis is rejected unless ``depends`` is present *and* a list (an absent or null value would silently erase the package's runtime dependencies) and unless every identity field is set. Wire ``_reconstruct_fetch_actions`` in ``conda_lock.conda_solver`` to try the fast path before falling through to the existing disk lookup, so older mamba/conda solvers that emit sparse LINKs are unaffected. ``LinkAction`` becomes ``total=False`` to model that the field set varies by solver. Resolves conda#906.
✅ Deploy Preview for conda-lock ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Mamba/micromamba 2.6.0 reorganized the package cache from the flat layout (``<pkgs_dir>/<dist_name>/info/...``) to a hierarchical one keyed on channel and subdir (``<pkgs_dir>/<scheme>/<host>/<channel>/<subdir>/<dist_name>/info/...``). See mamba-org/mamba#4163. Without matching that layout, the disk-fallback path hit by sparse-LINK dryruns silently misses records that mamba just wrote. Add ``conda_lock.solver.repodata_cache`` with: - ``hierarchical_cache_subpath``: derive the per-channel subpath from the LINK action's ``url`` (preferred) or ``base_url`` + ``platform`` (fallback). - ``candidate_record_paths``: hierarchical path first, legacy flat path second. Computed from LINK metadata rather than walking the cache, so the lookup is bounded. - ``get_repodata_record``: open the first viable candidate, with the existing 10-attempt + 0.1s retry loop preserved for the CI race condition the inline implementation handled. Move the orchestration ``reconstruct_fetch_actions`` from ``conda_solver.py`` into ``conda_lock.solver.dry_run``: the fast path was already there, and folding the disk fallback alongside it gives ``conda_solver.py`` a single import for dryrun normalization. Promote ``extract_json_object`` and ``get_pkgs_dirs`` from private helpers in ``conda_solver.py`` to public functions in ``conda_lock.invoke_conda``. They are CLI-probe / JSON-parse concerns, not cache-record concerns; ``repodata_cache`` and ``dry_run`` consume them via that boundary so the layering stays ``models`` -> ``invoke_conda`` -> ``solver/*``. Completes the conda#906 cache-layout fix.
The bare cache-layout fix gets us close but is not safe alone: ``<pkgs_dir>/<dist_name>/info/repodata_record.json`` is keyed on ``dist_name`` only, so the legacy flat layout still admits a cross-channel ``dist_name`` collision. Without identity checks the lookup can silently return a stale or impostor record. The hierarchical layout is meant to disambiguate collisions, but for that to work we have to walk it the way libmamba walks it -- and libmamba normalizes URLs before mapping to a cache path. Add to ``conda_lock.solver.repodata_cache``: - ``libmamba_strip_url_secrets`` -- libmamba-compat removal of user:pass@ credentials and ``/t/<token>`` path segments. The ``/t/<chars>`` regex is intentionally as overbroad as libmamba's; the cache lookup must derive bit-for-bit the same path mamba 2.6.0 wrote. - ``normalize_url_for_compare`` -- libmamba's ``compare_cleaned_url`` semantics (force https, strip credentials, strip trailing slash). Two URLs differing only by scheme, slash, or credentials must compare equal. - ``record_matches_link`` -- positive name/version equality plus contradiction checks on subdir / fn / md5 / sha256 / url. URL comparison goes through ``normalize_url_for_compare``; falls back to channel-string compare when only one side has a URL. Wire ``hierarchical_cache_subpath`` to feed ``libmamba_strip_url_secrets`` before normalizing the path separators, and ``get_repodata_record`` to call ``record_matches_link`` on every loaded candidate. Distinct failure reasons (missing file, JSON corruption, identity mismatch) are tracked per-retry so the final ``not found`` log surfaces the most actionable diagnostic. Continues conda#906: with the cache layout in place and now identity-checked, conda-lock cannot silently grab a wrong-channel record sharing a ``dist_name``.
Capture the failure-mode data from PR conda#862 so subsequent behavioral commits can run against a known-bad cache without needing live conda-forge or Docker. Three pinned-metadata archives represent the three relevant mamba versions: - ``2.1.0-pkgs.tar.gz``: clean baseline. - ``2.1.1-pkgs.tar.gz``: corrupt (the bug's first appearance). - ``2.3.3-pkgs.tar.gz``: corrupt (the bug's last appearance before mamba 2.6.0's fix). Each archive contains ``info/index.json`` plus ``info/repodata_record.json`` exactly as that mamba version wrote them when installing the same explicit lockfile, so overlaying the metadata on a freshly warmed cache produces a "hybrid" cache: real package files + bug-faithful metadata. That is what later end-to-end tests will drive ``conda-lock lock`` against. The ``05-render-explicit-lockfiles.py`` harness plus the per-version ``lockfile-...yml`` outputs lock the reproduction inputs. Add ``tests/support/corrupt_repodata.py`` with the helpers a component test needs to drive the harness: warmed-cache fixture factory, corrupt-metadata overlay, ``conda-lock lock`` / ``conda-lock render --kind=explicit`` driver wrappers, and explicit-lockfile-URL extraction. None of this calls into new production code yet -- it is pure test scaffolding so the next commits can focus on behavior. Exclude ``tests/test-corrupt-repodata/`` from mypy / ty / pre-commit since the stage scripts are Docker-based one-shot data generators, not runtime code. The committed fixtures (.tar.gz / .lock / .yml) are what tests consume. References conda#896, mamba-org/mamba#4052, mamba-org/mamba#4110.
Mamba/micromamba 2.1.1 through 2.3.3 wrote URL-stub metadata directly to ``repodata_record.json`` instead of merging with ``info/index.json`` (mamba-org/mamba#4052, fixed in 2.6.0 via mamba-org/mamba#4110). The fingerprint is ``timestamp == 0`` and empty ``license``; ``depends``, ``constrains``, ``build_number`` are zeroed too. A conda-lock dryrun against such a cache produces empty-deps lockfile entries that silently drop dependencies from later solves and installs. conda#896. This commit adds the cache-side mitigation. Subsequent commits add the lockfile-side carry-forward heal and the strict graph-integrity check that catches the silent-vanishing variant. Changes: - ``is_mamba_2_1_to_2_3_stub_record`` in ``solver/repodata_cache``: pin the corruption fingerprint as a function so callers across the dryrun pipeline can recognize a stub record consistently. - ``heal_corrupt_record``: overlay the corrupt record with the sibling ``info/index.json``. ``info/index.json`` is extracted from the package tarball at install time and is never affected by the bug, so it carries the canonical ``depends`` / ``license`` / ``timestamp``. Cache-derived fields (``url``, ``md5``, ``sha256``, ``size``) are kept from the record where non-empty. - ``RepodataLookup`` dataclass: ``get_repodata_record`` now returns a structured outcome -- ``found`` / ``healed`` / ``unhealable_corrupt`` / ``not_found`` -- so the cache layer reports facts and the orchestration layer (``solver.dry_run.reconstruct_fetch_actions``) translates each outcome to a user-facing WARNING with the right remediation text. Eliminates the previous in-cache ``logger.warning`` calls that mixed policy with I/O. - Two-stage identity gate before classifying a stub as ``unhealable_corrupt``: 1. ``record_matches_link`` rejects on any contradiction. 2. ``_stub_has_strong_identity_match`` requires positive match on at least one of url/md5/sha256/fn (the artifact-identity fields). ``subdir`` is excluded -- it narrows platform, not artifact, and would license a "no contradictions" overreach. An impostor stub at a flat-fallback path falls through to ``not_found`` rather than misclaiming the operator's cache is corrupt. - ``link_action_as_fetch`` rejects a LINK that itself carries the stub-record fingerprint, so the rich-LINK fast path can't smuggle a corrupt record into the synthesized FETCH on a solver that emits stubs in LINK without going through cache heal. - Outcome priority: ``unhealable_corrupt`` always wins over ``not_found``. ``last_unhealable`` is set only after both identity gates pass, so a real corrupt-cache claim is never demoted by trivia from later candidate noise. - ``warn_on_pkgs_dirs_leak`` in ``solver.dry_run``: surface when the user's ``.condarc`` leaks extra ``pkgs_dirs`` past the ``CONDA_PKGS_DIRS`` isolation conda-lock requests, since stale or corrupt entries in those leaked dirs are step three of the conda#896 chain. Tests in ``tests/unit/test_repodata_cache.py`` and ``tests/unit/test_dry_run_actions.py`` cover: the corruption fingerprint, ``info/index.json`` heal, the strong-identity gate (both directions: weak-identity stubs fall through; one matching strong field is enough), outcome priority (unhealable beats identity-rejection on a later candidate), and the orchestration boundary's WARNING-translation contract for each outcome.
Cache-side heal alone is not enough to close conda#896: once a lockfile has been *generated* against a corrupt mamba 2.1.1-2.3.3 cache, the entries it carries already have empty ``dependencies: {}``. Every subsequent ``conda-lock lock --update`` then reads that lockfile back via ``fake_conda_environment``, ``to_fetch_action()`` emits a FETCH with empty ``depends``, ``apply_categories`` cannot reach the package via the dependency graph, and the freshly-emitted lockfile inherits the empty deps -- forever, even after the user upgrades to mamba 2.6.0+. Add ``conda_lock.solver.lockfile_heal`` to break the carry-forward. The strategy is "consult the cache on the way in." For each lockfile entry with empty ``dependencies``: - Project the entry into a LinkAction-shaped dict and reuse the existing ``candidate_record_paths`` machinery to look up the package in the local cache. Hierarchical layout (mamba 2.6.0+) is preferred, legacy flat layout is the fallback. - Read ``info/index.json`` (not ``repodata_record.json``): ``info/index.json`` is extracted directly from the package tarball at install time and is unaffected by the cache corruption bug, so it carries the canonical ``depends``. - Identity-check the ``info/index.json`` against the LINK before trusting it. The legacy flat path is keyed on ``dist_name`` alone and can hold same-dist-name packages from different channels; an unchecked match would silently heal the lockfile with metadata from the wrong artifact. Some packages legitimately have no runtime dependencies (``tzdata``, ``python_abi``, ``_libgcc_mutex``, ...) so empty ``dependencies`` alone is not proof of corruption. ``LockfileHealReport`` exposes the per-entry classification as three explicit tuples: - ``healed``: cache contradicted the lockfile -> proof of corruption; depends recovered in place. - ``confirmed_legit_empty``: cache agreed empty deps are correct. - ``ambiguous``: cache has no entry to adjudicate. Mutually-exclusive tuples make it structurally impossible for the orchestration layer to accidentally collapse "legit-empty" with "corrupt-but-unprovable evidence," which an earlier integer return value did. Wire ``update_specs_for_arch`` in ``conda_lock.conda_solver`` to call the heal before ``fake_conda_environment``, log a WARNING when entries were healed (with the list and the conda-lock#896 reference), and log a separate prescriptive WARNING when ambiguous entries remain. The orchestration layer owns log policy; the heal layer is silent at WARNING level by contract. Tests in ``tests/component/test_lockfile_heal.py`` cover the three classification states (healed, confirmed_legit_empty, ambiguous), the partial-cache case where one matched entry must NOT promote ambiguous entries, the legacy-flat cross-channel rejection, the noarch / concrete-subdir cross-validation, and the end-to-end ``update_specs_for_arch`` WARNING translations.
After ``apply_categories`` walks forward from each requested input spec, every planned package should have at least one category. A package without any category is an orphan: the lockfile dependency graph cannot explain why this package is in the plan. Orphans are not tolerated. They would silently vanish from the v1 lockfile output (which emits one entry per category, so a package with no category produces zero entries) and the resulting environment would install fewer packages than the solver actually planned. This is the silent-vanishing variant of the conda#896 chain that cache-side and lockfile-side heal do not catch -- it surfaces only after categorization. Add ``conda_lock.solver.graph_integrity.assert_no_orphaned_conda_packages`` which owns the entire orphan policy: - Definition of an orphaned planned package. - No reverse-propagation of categories. ``apply_categories``' forward walk is the only categorization path. Reverse-prop hides the broken-graph signal in single-category projects (every orphan with any categorized dependency would silently inherit ``main``, and the hard-fail would never fire on real conda#896-style breakage), and ``"P depends on X (main)"`` does not prove ``"main needs P"``; a permissive variant would launder dev-only solver artifacts into main installs. - Why ``pip`` is not normally an orphan even though nothing the user explicitly requests transitively depends on it: both conda's ``add_pip_as_python_dependency`` and libmamba's repo-load injection mutate ``python``'s declared dependencies to include ``pip`` at metadata-load time. So a healthy forward walk from ``python`` reaches ``pip`` in normal operation, and an orphaned ``pip`` indicates broken solver metadata rather than the expected shape. - ``CONDA_LOCK_ALLOW_ORPHANED_LOCKFILE=1`` escape hatch: intentionally ugly, demotes the hard-fail to a loud WARNING and assigns orphans to ``main`` so they survive v1 serialization. This is for implementation-bug emergencies (a buggy custom solver, missing ``add_pip_as_python_dependency`` injection on an exotic channel) not a configuration knob. The WARNING spells out the over-install consequence: every ``conda-lock install`` invocation -- even ones without ``--dev-dependencies`` or ``-e <category>`` -- will install the orphans, potentially over-installing dev-only solver artifacts into production environments. Add ``OrphanLockedDependencyError`` in ``conda_lock.errors``; ``solve_conda`` calls ``assert_no_orphaned_conda_packages(planned, platform)`` after ``apply_categories``. The module docstring on ``conda_lock.conda_solver`` introduces the ``solver/*`` split as a whole so a new reader sees the layering on first opening the file. Six component tests in ``tests/component/test_graph_integrity.py`` pin the policy contract: - ``test_solve_conda_accepts_pip_via_python_add_pip_dependency``: the *normal* shape. ``python.depends == ["pip"]`` makes ``pip`` forward-reachable; ``pip`` inherits ``python``'s requested category without rescue. - ``test_solve_conda_hard_fails_when_python_metadata_omits_pip``: the abnormal-metadata shape. ``python.depends == []`` makes ``pip`` unreachable; the orphan check hard-fails. The test docstring names this as broken solver metadata, not normal solver auto-install behavior. - ``test_solve_conda_envvar_demotes_orphan_to_warning``: pins ``CONDA_LOCK_ALLOW_ORPHANED_LOCKFILE=1`` semantics. WARNING fires (explicitly mentioning ``main``, ``--dev-dependencies`` / ``-e <category>``, and the envvar name so the message can't silently weaken), orphans are assigned to ``main``, ``solve_conda`` returns instead of raising. - ``test_solve_conda_hard_fails_on_unrecoverable_orphan``: the classic conda#896-corrupt-cache shape -- ``zlib`` planned with empty depends, no categorized package transitively requires it -- hard-fails with regenerate-from-sources guidance and the envvar escape named in the error message. - ``test_solve_conda_orphan_via_dep_breakage_still_hard_fails``: multi-category corrupt-cache variant. - ``test_solve_conda_passes_when_dependency_graph_is_intact``: positive sanity check.
The unit + component tests exercise the cache-side heal, lockfile-side heal, and graph-integrity invariants in isolation with hand-crafted fixtures. They cannot reach the silent-vanishing path that conda#896 actually reports, because that path is only reachable through the full pipeline: ``LINK``-only dryrun -> ``reconstruct_fetch_actions`` -> ``get_repodata_record`` -> cache-side heal -> ``apply_categories`` forward walk -> ``assert_no_orphaned_conda_packages`` -> v1 serialization. Add ``tests/e2e/test_corrupt_repodata_repro.py`` to drive that whole pipeline against the corrupt-metadata fixtures committed in ``tests/test-corrupt-repodata/``. The test parametrizes over ``mamba`` and ``conda-standalone`` (both solvers were affected by conda#896 because both read leaked ``pkgs_dirs``) and asserts: 1. *Structural lockfile equivalence*: parsing both unified lockfiles via ``parse_conda_lock_file`` produces the same set of ``(name, version, category)`` tuples. Catches silent drops and any "rescue into main" miscategorization. 2. *Explicit-URL equivalence*: ``conda-lock render --kind=explicit`` against both unified lockfiles produces identical URL sets. PR conda#862 stage 05's whole point is that "packages are missing from the resulting environment if and only if they are missing from the explicit lockfile," so this is the user-visible bug surface. 3. *Empty-deps regression*: corrupt-cache run does not produce significantly more empty-``dependencies`` entries than the clean-cache run, proving the heal pipeline recovered them. The supporting fixture factory and harness wrappers live in ``tests.support.corrupt_repodata`` (added with the fixture data), so the test body stays focused on the assertions. The test is marked ``corrupt_cache_repro`` for CI isolation in the next commit; locally it runs in ~2-5 minutes per parametrization because it warms a real conda-forge cache and drives full ``conda-lock lock`` and ``conda-lock render`` invocations.
The ``corrupt_cache_repro`` end-to-end test drives live conda-forge in two independent solves (corrupt-cache vs. clean-control) and runs ~2-5 minutes per parametrization. Putting it in the default unit-test collection would dominate every contributor's local pytest run and inflate every PR's CI wall clock for negligible additional confidence on most changes. Three pieces of CI plumbing: 1. ``pyproject.toml`` declares the ``corrupt_cache_repro`` marker and adds ``-m "not corrupt_cache_repro"`` to the default ``addopts``. ``pytest`` invocations from contributors, editors, and the unit-test CI shards exclude the e2e by default. Local opt-in is ``pytest -m corrupt_cache_repro``; bypass-everything is ``pytest -m ""``. 2. ``.github/workflows/test.yml`` adds a dedicated ``run-corrupt-cache-repro`` step that runs once per ``(os, py-version)`` on shard 1 / linux-64 only -- the fixture is a linux-64 cache snapshot from the PR conda#862 harness, so other platforms have nothing to test against. The step uses ``-o "addopts=-rsx --verbose"`` to clear the project-default exclusion and ``-m corrupt_cache_repro`` to opt back into just the e2e parametrizations. 3. The CI step targets ``tests/e2e/test_corrupt_repodata_repro.py`` directly so a future test author who marks an unrelated test with ``corrupt_cache_repro`` doesn't accidentally end up in the heavy CI lane. Behavior of the unit + component tests is unchanged.
Pull in ``conda-standalone`` in the dev environment so contributors can run the dual-solver e2e parametrization locally without standing up a separate conda installation, and refresh ``pixi.lock`` to match. - ``pixi.toml`` and ``environments/dev-environment.yaml``: add ``conda-standalone``. - ``environments/conda-lock-python-3.10.yaml``, ``environments/conda-lock-python-3.14.yaml``: regenerated from the updated source. - ``pixi.lock``: re-resolved to match. Pure environment / lockfile churn; no source code changes.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes #896