util/resolve: sort versions in matchRequirement

[miraeti]

Fixes#385

Problem

The v3alpha API returns package versions sorted lexicographically by version string (e.g. for setuptools, "9.1.0" sorts after "84.0.0"). The default branch of MatchRequirement (used by PyPI and other non-NPM systems) passed the list through unsorted, but resolvers assume matches come back in ascending order, the PyPI resolver iterates candidates in reverse (attemptToPinCriterion) and pins the first one that works. Unpinned transitive requirements therefore resolve to the lexicographic maximum (e.g. setuptools 9.1.0 instead of 84.0.0), producing false-positive vulnerability reports in osv-scanner with --data-source deps.dev.

Fix

Call SortVersions at the top of matchRequirement, mirroring what the NPM branch already does with sortNPMVersions (util/resolve/match.go:163). Covers all systems on the default branch, not just PyPI.

Test

Adds TestMatchRequirement: feeds the lexicographic order returned by the API and asserts matches come back in ascending semver order; verified the test fails without the fix.

[cuixq]

Thanks for digging into this and identifying the root cause of the issue!

I am a bit concerned about sorting in matchRequirement: it should ideally be a pure filtering function - its only responsibility is to evaluate constraints against candidate versions and filtering matches. In addition, having the sorting in matchRequirement may introduce redundant sorting overhead considering it will be called many time during dependency resolution.

The root discrepancy is actually at the API client:

• LocalClient already sorts versions in ascending semver order via SortVersions when they are added.
• osv-scalibr's native PyPIRegistryClient.Versions also explicitly sorts versions with semver.PyPI.Compare before returning.
• APIClient.Versions() was the only place that returns raw lexicographical order from the backend API.

Therefore, what do you think about:

1. Sorting in APIClient.Versions()util/resolve/api.go?
2. Documenting on Client.Versions in util/resolve/client.go to make it clear that the versions should be sorted?

[miraeti]

Thanks for the review @cuixq

Good point about keeping matchRequirement a pure filter. I've reworked the fix per your suggestion. The sort now happens in APIClient.Versions, and the ordering contract is documented on Client.Versions.

Heads up: this PR was automatically closed when I renamed the head branch. The reworked change is now in #387 .

[miraeti]

Fixes#385

Problem

The v3alpha API returns package versions sorted lexicographically by version string (e.g. for setuptools, "9.1.0" sorts after "84.0.0"). The default branch of MatchRequirement (used by PyPI and other non-NPM systems) passed the list through unsorted, but resolvers assume matches come back in ascending order, the PyPI resolver iterates candidates in reverse (attemptToPinCriterion) and pins the first one that works. Unpinned transitive requirements therefore resolve to the lexicographic maximum (e.g. setuptools 9.1.0 instead of 84.0.0), producing false-positive vulnerability reports in osv-scanner with --data-source deps.dev.

Fix

Call SortVersions at the top of matchRequirement, mirroring what the NPM branch already does with sortNPMVersions (util/resolve/match.go:163). Covers all systems on the default branch, not just PyPI.

Test

Adds TestMatchRequirement: feeds the lexicographic order returned by the API and asserts matches come back in ascending semver order; verified the test fails without the fix.

[cuixq]

Thanks for digging into this and identifying the root cause of the issue!

I am a bit concerned about sorting in matchRequirement: it should ideally be a pure filtering function - its only responsibility is to evaluate constraints against candidate versions and filtering matches. In addition, having the sorting in matchRequirement may introduce redundant sorting overhead considering it will be called many time during dependency resolution.

The root discrepancy is actually at the API client:

• LocalClient already sorts versions in ascending semver order via SortVersions when they are added.
• osv-scalibr's native PyPIRegistryClient.Versions also explicitly sorts versions with semver.PyPI.Compare before returning.
• APIClient.Versions() was the only place that returns raw lexicographical order from the backend API.

Therefore, what do you think about:

1. Sorting in APIClient.Versions()util/resolve/api.go?
2. Documenting on Client.Versions in util/resolve/client.go to make it clear that the versions should be sorted?

[miraeti]

Thanks for the review @cuixq

Good point about keeping matchRequirement a pure filter. I've reworked the fix per your suggestion. The sort now happens in APIClient.Versions, and the ordering contract is documented on Client.Versions.

Heads up: this PR was automatically closed when I renamed the head branch. The reworked change is now in #387 .

[miraeti]

Fixes#385

Problem

The v3alpha API returns package versions sorted lexicographically by version string (e.g. for setuptools, "9.1.0" sorts after "84.0.0"). The default branch of MatchRequirement (used by PyPI and other non-NPM systems) passed the list through unsorted, but resolvers assume matches come back in ascending order, the PyPI resolver iterates candidates in reverse (attemptToPinCriterion) and pins the first one that works. Unpinned transitive requirements therefore resolve to the lexicographic maximum (e.g. setuptools 9.1.0 instead of 84.0.0), producing false-positive vulnerability reports in osv-scanner with --data-source deps.dev.

Fix

Call SortVersions at the top of matchRequirement, mirroring what the NPM branch already does with sortNPMVersions (util/resolve/match.go:163). Covers all systems on the default branch, not just PyPI.

Test

Adds TestMatchRequirement: feeds the lexicographic order returned by the API and asserts matches come back in ascending semver order; verified the test fails without the fix.

[cuixq]

Thanks for digging into this and identifying the root cause of the issue!

I am a bit concerned about sorting in matchRequirement: it should ideally be a pure filtering function - its only responsibility is to evaluate constraints against candidate versions and filtering matches. In addition, having the sorting in matchRequirement may introduce redundant sorting overhead considering it will be called many time during dependency resolution.

The root discrepancy is actually at the API client:

• LocalClient already sorts versions in ascending semver order via SortVersions when they are added.
• osv-scalibr's native PyPIRegistryClient.Versions also explicitly sorts versions with semver.PyPI.Compare before returning.
• APIClient.Versions() was the only place that returns raw lexicographical order from the backend API.

Therefore, what do you think about:

1. Sorting in APIClient.Versions()util/resolve/api.go?
2. Documenting on Client.Versions in util/resolve/client.go to make it clear that the versions should be sorted?

[miraeti]

Thanks for the review @cuixq

Good point about keeping matchRequirement a pure filter. I've reworked the fix per your suggestion. The sort now happens in APIClient.Versions, and the ordering contract is documented on Client.Versions.

Heads up: this PR was automatically closed when I renamed the head branch. The reworked change is now in #387 .

[miraeti]

Fixes#385

Problem

The v3alpha API returns package versions sorted lexicographically by version string (e.g. for setuptools, "9.1.0" sorts after "84.0.0"). The default branch of MatchRequirement (used by PyPI and other non-NPM systems) passed the list through unsorted, but resolvers assume matches come back in ascending order, the PyPI resolver iterates candidates in reverse (attemptToPinCriterion) and pins the first one that works. Unpinned transitive requirements therefore resolve to the lexicographic maximum (e.g. setuptools 9.1.0 instead of 84.0.0), producing false-positive vulnerability reports in osv-scanner with --data-source deps.dev.

Fix

Call SortVersions at the top of matchRequirement, mirroring what the NPM branch already does with sortNPMVersions (util/resolve/match.go:163). Covers all systems on the default branch, not just PyPI.

Test

Adds TestMatchRequirement: feeds the lexicographic order returned by the API and asserts matches come back in ascending semver order; verified the test fails without the fix.

[cuixq]

Thanks for digging into this and identifying the root cause of the issue!

I am a bit concerned about sorting in matchRequirement: it should ideally be a pure filtering function - its only responsibility is to evaluate constraints against candidate versions and filtering matches. In addition, having the sorting in matchRequirement may introduce redundant sorting overhead considering it will be called many time during dependency resolution.

The root discrepancy is actually at the API client:

• LocalClient already sorts versions in ascending semver order via SortVersions when they are added.
• osv-scalibr's native PyPIRegistryClient.Versions also explicitly sorts versions with semver.PyPI.Compare before returning.
• APIClient.Versions() was the only place that returns raw lexicographical order from the backend API.

Therefore, what do you think about:

1. Sorting in APIClient.Versions()util/resolve/api.go?
2. Documenting on Client.Versions in util/resolve/client.go to make it clear that the versions should be sorted?

[miraeti]

Thanks for the review @cuixq

Good point about keeping matchRequirement a pure filter. I've reworked the fix per your suggestion. The sort now happens in APIClient.Versions, and the ordering contract is documented on Client.Versions.

Heads up: this PR was automatically closed when I renamed the head branch. The reworked change is now in #387 .

[miraeti]

Fixes#385

Problem

The v3alpha API returns package versions sorted lexicographically by version string (e.g. for setuptools, "9.1.0" sorts after "84.0.0"). The default branch of MatchRequirement (used by PyPI and other non-NPM systems) passed the list through unsorted, but resolvers assume matches come back in ascending order, the PyPI resolver iterates candidates in reverse (attemptToPinCriterion) and pins the first one that works. Unpinned transitive requirements therefore resolve to the lexicographic maximum (e.g. setuptools 9.1.0 instead of 84.0.0), producing false-positive vulnerability reports in osv-scanner with --data-source deps.dev.

Fix

Call SortVersions at the top of matchRequirement, mirroring what the NPM branch already does with sortNPMVersions (util/resolve/match.go:163). Covers all systems on the default branch, not just PyPI.

Test

Adds TestMatchRequirement: feeds the lexicographic order returned by the API and asserts matches come back in ascending semver order; verified the test fails without the fix.

[cuixq]

Thanks for digging into this and identifying the root cause of the issue!

I am a bit concerned about sorting in matchRequirement: it should ideally be a pure filtering function - its only responsibility is to evaluate constraints against candidate versions and filtering matches. In addition, having the sorting in matchRequirement may introduce redundant sorting overhead considering it will be called many time during dependency resolution.

The root discrepancy is actually at the API client:

• LocalClient already sorts versions in ascending semver order via SortVersions when they are added.
• osv-scalibr's native PyPIRegistryClient.Versions also explicitly sorts versions with semver.PyPI.Compare before returning.
• APIClient.Versions() was the only place that returns raw lexicographical order from the backend API.

Therefore, what do you think about:

1. Sorting in APIClient.Versions()util/resolve/api.go?
2. Documenting on Client.Versions in util/resolve/client.go to make it clear that the versions should be sorted?

[miraeti]

Thanks for the review @cuixq

Good point about keeping matchRequirement a pure filter. I've reworked the fix per your suggestion. The sort now happens in APIClient.Versions, and the ordering contract is documented on Client.Versions.

Heads up: this PR was automatically closed when I renamed the head branch. The reworked change is now in #387 .

[miraeti]

Fixes#385

Problem

The v3alpha API returns package versions sorted lexicographically by version string (e.g. for setuptools, "9.1.0" sorts after "84.0.0"). The default branch of MatchRequirement (used by PyPI and other non-NPM systems) passed the list through unsorted, but resolvers assume matches come back in ascending order, the PyPI resolver iterates candidates in reverse (attemptToPinCriterion) and pins the first one that works. Unpinned transitive requirements therefore resolve to the lexicographic maximum (e.g. setuptools 9.1.0 instead of 84.0.0), producing false-positive vulnerability reports in osv-scanner with --data-source deps.dev.

Fix

Call SortVersions at the top of matchRequirement, mirroring what the NPM branch already does with sortNPMVersions (util/resolve/match.go:163). Covers all systems on the default branch, not just PyPI.

Test

Adds TestMatchRequirement: feeds the lexicographic order returned by the API and asserts matches come back in ascending semver order; verified the test fails without the fix.

[cuixq]

Thanks for digging into this and identifying the root cause of the issue!

I am a bit concerned about sorting in matchRequirement: it should ideally be a pure filtering function - its only responsibility is to evaluate constraints against candidate versions and filtering matches. In addition, having the sorting in matchRequirement may introduce redundant sorting overhead considering it will be called many time during dependency resolution.

The root discrepancy is actually at the API client:

• LocalClient already sorts versions in ascending semver order via SortVersions when they are added.
• osv-scalibr's native PyPIRegistryClient.Versions also explicitly sorts versions with semver.PyPI.Compare before returning.
• APIClient.Versions() was the only place that returns raw lexicographical order from the backend API.

Therefore, what do you think about:

1. Sorting in APIClient.Versions()util/resolve/api.go?
2. Documenting on Client.Versions in util/resolve/client.go to make it clear that the versions should be sorted?

[miraeti]

Thanks for the review @cuixq

Good point about keeping matchRequirement a pure filter. I've reworked the fix per your suggestion. The sort now happens in APIClient.Versions, and the ordering contract is documented on Client.Versions.

Heads up: this PR was automatically closed when I renamed the head branch. The reworked change is now in #387 .

[miraeti]

Fixes#385

Problem

The v3alpha API returns package versions sorted lexicographically by version string (e.g. for setuptools, "9.1.0" sorts after "84.0.0"). The default branch of MatchRequirement (used by PyPI and other non-NPM systems) passed the list through unsorted, but resolvers assume matches come back in ascending order, the PyPI resolver iterates candidates in reverse (attemptToPinCriterion) and pins the first one that works. Unpinned transitive requirements therefore resolve to the lexicographic maximum (e.g. setuptools 9.1.0 instead of 84.0.0), producing false-positive vulnerability reports in osv-scanner with --data-source deps.dev.

Fix

Call SortVersions at the top of matchRequirement, mirroring what the NPM branch already does with sortNPMVersions (util/resolve/match.go:163). Covers all systems on the default branch, not just PyPI.

Test

Adds TestMatchRequirement: feeds the lexicographic order returned by the API and asserts matches come back in ascending semver order; verified the test fails without the fix.

[cuixq]

Thanks for digging into this and identifying the root cause of the issue!

I am a bit concerned about sorting in matchRequirement: it should ideally be a pure filtering function - its only responsibility is to evaluate constraints against candidate versions and filtering matches. In addition, having the sorting in matchRequirement may introduce redundant sorting overhead considering it will be called many time during dependency resolution.

The root discrepancy is actually at the API client:

• LocalClient already sorts versions in ascending semver order via SortVersions when they are added.
• osv-scalibr's native PyPIRegistryClient.Versions also explicitly sorts versions with semver.PyPI.Compare before returning.
• APIClient.Versions() was the only place that returns raw lexicographical order from the backend API.

Therefore, what do you think about:

1. Sorting in APIClient.Versions()util/resolve/api.go?
2. Documenting on Client.Versions in util/resolve/client.go to make it clear that the versions should be sorted?

[miraeti]

Thanks for the review @cuixq

Good point about keeping matchRequirement a pure filter. I've reworked the fix per your suggestion. The sort now happens in APIClient.Versions, and the ordering contract is documented on Client.Versions.

Heads up: this PR was automatically closed when I renamed the head branch. The reworked change is now in #387 .

[miraeti]

Fixes#385

Problem

The v3alpha API returns package versions sorted lexicographically by version string (e.g. for setuptools, "9.1.0" sorts after "84.0.0"). The default branch of MatchRequirement (used by PyPI and other non-NPM systems) passed the list through unsorted, but resolvers assume matches come back in ascending order, the PyPI resolver iterates candidates in reverse (attemptToPinCriterion) and pins the first one that works. Unpinned transitive requirements therefore resolve to the lexicographic maximum (e.g. setuptools 9.1.0 instead of 84.0.0), producing false-positive vulnerability reports in osv-scanner with --data-source deps.dev.

Fix

Call SortVersions at the top of matchRequirement, mirroring what the NPM branch already does with sortNPMVersions (util/resolve/match.go:163). Covers all systems on the default branch, not just PyPI.

Test

Adds TestMatchRequirement: feeds the lexicographic order returned by the API and asserts matches come back in ascending semver order; verified the test fails without the fix.

[cuixq]

Thanks for digging into this and identifying the root cause of the issue!

I am a bit concerned about sorting in matchRequirement: it should ideally be a pure filtering function - its only responsibility is to evaluate constraints against candidate versions and filtering matches. In addition, having the sorting in matchRequirement may introduce redundant sorting overhead considering it will be called many time during dependency resolution.

The root discrepancy is actually at the API client:

• LocalClient already sorts versions in ascending semver order via SortVersions when they are added.
• osv-scalibr's native PyPIRegistryClient.Versions also explicitly sorts versions with semver.PyPI.Compare before returning.
• APIClient.Versions() was the only place that returns raw lexicographical order from the backend API.

Therefore, what do you think about:

1. Sorting in APIClient.Versions()util/resolve/api.go?
2. Documenting on Client.Versions in util/resolve/client.go to make it clear that the versions should be sorted?

[miraeti]

Thanks for the review @cuixq

Good point about keeping matchRequirement a pure filter. I've reworked the fix per your suggestion. The sort now happens in APIClient.Versions, and the ordering contract is documented on Client.Versions.

Heads up: this PR was automatically closed when I renamed the head branch. The reworked change is now in #387 .