fix(python): validate dependency names and drop unreachable PURL fallbacks - #1346
Merged
Conversation
Greptile SummaryThe PR validates normalized Python distribution names before constructing dependency PURLs while preserving invalid requirements as PURL-less dependency records. It also removes unreachable PURL fallbacks across Python, Poetry, uv, pylock, and Conan parsing paths.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/parsers/python/utils.rs | Adds PEP 508 distribution-name validation and propagates optional PURLs without dropping the dependency record. |
| src/parsers/python/setup_cfg.rs | Preserves setup.cfg requirements when PURL construction declines an invalid distribution name. |
| src/parsers/python/pyproject.rs | Retains Poetry and PEP 621 dependencies when PURL generation returns None. |
| src/parsers/python/scan_test.rs | Exercises the real scanner and assembly pipeline and verifies PURL-less dependency visibility and datafile attribution. |
| src/parsers/python/test.rs | Verifies invalid free-form distribution names omit PURLs while valid sibling requirements remain unaffected. |
| src/parsers/conan.rs | Removes an unreachable constructor fallback while preserving name-only PURLs for ranged and bare references. |
| src/parsers/poetry_lock.rs | Simplifies PackageUrl construction and cleanly propagates impossible constructor or version failures. |
| src/parsers/pylock_toml.rs | Removes unreachable manual PURL assembly in favor of the PackageUrl builder. |
| src/parsers/uv_lock.rs | Removes unreachable manual PURL fallback while retaining existing long-name handling. |
Reviews (3): Last reviewed commit: "test(python): cover the purl-less setup...." | Re-trigger Greptile
mstykow
force-pushed
the
investigate/purl-unknowns
branch
from
August 11, 2026 21:09
1dc704c to
43ea9db
Compare
`PackageUrl::new` validates only the type, and every one of these call sites passes a compile-time-constant type (`"conan"`, `PackageType::Pypi` via `PACKAGE_TYPE.as_str()`), so the `Err` arm cannot be taken. `PackageUrl::with_version` and `with_namespace` never fail for these types either -- `with_version` returns `Ok` unconditionally, and none of the affected types are in the namespace-prohibited list. The `format!`-based fallbacks behind those arms were therefore dead, and they built PURLs without percent-encoding, so if they ever had fired they would have emitted a non-round-tripping PURL. `build_python_dependency_purl` constructed a `PackageUrl` purely as a gate and discarded it; it is now infallible and returns `String`. Verified with a before/after scan over pypi and conan fixtures: output is byte-identical. Signed-off-by: Maxim Stykow <maxim.stykow@gmail.com>
`build_python_dependency_purl` guarded PURL construction with `PackageUrl::new(...).ok()`, which validates the *type* — a constant here — and never the name, then assembled the PURL with `format!` anyway. Names arriving from `setup.cfg` come from free-form text, so `install_requires = a/b==2.0` emitted `pkg:pypi/a/b@2.0`: a PURL no parser accepts, since pypi prohibits a namespace. Replace the gate with the PEP 508 distribution-name check. The dependency is still reported and `extracted_requirement` still carries the raw text, so declining the PURL loses nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Maxim Stykow <maxim.stykow@gmail.com>
Greptile review: the name-validation change was covered only at the parser level, so nothing verified that a dependency emitted with no PURL survives scanner and assembly processing — which is exactly where a purl-less entry is most likely to be dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Maxim Stykow <maxim.stykow@gmail.com>
mstykow
force-pushed
the
investigate/purl-unknowns
branch
from
August 11, 2026 21:36
43ea9db to
7d17798
Compare
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
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.
Summary
PackageUrl::new(type, name).ok()and fell back toformat!on failure. That call validates the type — always a compile-time constant at these sites — and never the name, so the guard could not fire and the fallback was unreachable.setup.cfgtakes names from free-form text:install_requires = a/b==2.0emittedpkg:pypi/a/b@2.0, which no PURL parser accepts because pypi prohibits a namespace. Replacing the dead gate with a real one is the point of this change; removing the dead fallbacks is the tidy-up that goes with it.Scope and exclusions
build_python_dependency_purl; removal of unreachableformat!fallbacks inconan,uv_lock,poetry_lock,pylock_toml,python/utils; removal of the sibling deadif …with_version(v).is_err() { return None }branches.pnpm_lock's equivalent fallback is also unreachable but is left alone — removing it cleanly means also removing three dead.ok()?exits inside the sharednpm_purl, whichyarn_lockandnpm_lockuse too..expect()was introduced. The pypi builders keep anOptionreturn, so an impossible arm degrades to no PURL rather than an unencoded one, and no new panic path enters a scanner that must survive hostile input.How to verify
Before:
pkg:pypi/a/b@2.0. After:purl: nullwithextracted_requirement: "a/b==2.0"intact, andrequestsunaffected.For the removals, the useful check is that nothing changed: scanning a fixture set covering
pyproject.toml,setup.cfg,poetry.lock,uv.lock,pylock.tomlandconanfile.txtis byte-identical before and after, once UIDs are normalised.Intentional differences from Python
extracted_requirement; this converges on that.Follow-up work
pkg:opam/conf gmpandpkg:opam/ocaml/evilintodependencies[].purl— the latter silently reinterpreted as namespace + name.pkg:conan/my pkgfor a ranged reference.pkg:nuget/My Packagewithformat!while the csproj parser buildspkg:nuget/My%20Packagethrough the crate, and the PURL is a join key — so a centralPackageVersionsilently fails to resolve andextracted_requirementcomes out null. A control fixture with the space removed resolves correctly.PackageUrl::newalso mutates names (lowercases for pypi/npm/github/hex/deb/bitbucket,_→-for pypi), so swappingformat!for the crate at those sites is a behaviour change, not a pure encoding fix.Expected-output fixture changes