Skip to content

Conan download_urls properly checks channel

Steve Abrams requested to merge 235653-conan-dependency-bug into master

What does this MR do?

What the bug was doing 🐛

When downloading a Conan package, the Conan client makes an API call to

GET /api/v4/packages/conan/v1/conans/:package_name/:package_version/:package_username/:package_channel/download_urls

Which returns a set of urls that the Conan client will request to download the various files:

{
    "conanfile.py": "https://gitlab.com/api/v4/packages/conan/v1/files/:package_name/:package_version/:package_username/:package_channel/0/export/conanfile.py",
    "conanmanifest.txt": "https://gitlab.com/api/v4/packages/conan/v1/files/:package_name/:package_version/:package_username/:package_channel/0/export/conanmanifest.txt"
}

The problem was that if you had two packages with the same :package_name, :package_version, and :package_username, but different :package_channels, a request for /download_urls to either package_channel would return the same response, always returning whichever package was newest.

So if I publish package foo/1.0/bar/stable, then publish foo/1.0/bar/beta, a request to foo/1.0/bar/stable/download_urls would return the urls for the beta files.

How it is being fixed 💡

The Packages::Conan::PackagePresenter is responsible for generating the urls returned in the response. In the presenter, the code that fetches the package was not filtering by channel, so it would find both packages and just use the newest one.

The code fetching the package in the presenter is essentially a duplicate of https://gitlab.com/gitlab-org/gitlab/-/blob/2b90e26af664be3cd556a6c315c906c694f9c882/lib/api/helpers/packages/conan/api_helpers.rb#L94, which does filter by channel. So instead of fixing the code in the presenter, we remove it, and pass the package to the presenter directly using the other helper.

Screenshots

I uploaded two packages: mypackage/1.0@sabrams+mypackage/stable and mypackage/1.0@sabrams+mypackage/beta

Before:

curl -H "Authorization: Bearer <token>" http://localhost:3000/api/v4/packages/conan/v1/conans/mypackage/1.0/sabrams+mypackage/stable/download_urls
{
    "conanfile.py": "http://localhost:3000/api/v4/packages/conan/v1/files/mypackage/1.0/sabrams+mypackage/beta/0/export/conanfile.py",
    "conanmanifest.txt": "http://localhost:3000/api/v4/packages/conan/v1/files/mypackage/1.0/sabrams+mypackage/beta/0/export/conanmanifest.txt"
}

After

curl -H "Authorization: Bearer <token>" http://localhost:3000/api/v4/packages/conan/v1/conans/mypackage/1.0/sabrams+mypackage/stable/download_urls
{
    "conanfile.py": "http://localhost:3000/api/v4/packages/conan/v1/files/mypackage/1.0/sabrams+mypackage/stable/0/export/conanfile.py",
    "conanmanifest.txt": "http://localhost:3000/api/v4/packages/conan/v1/files/mypackage/1.0/sabrams+mypackage/stable/0/export/conanmanifest.txt"
}

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team

Related #235653 (closed)

Edited by Steve Abrams

Merge request reports