Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support force resolving dependency versions a la yarn's "resolutions" support. #4530

Closed
chris-codaio opened this issue Nov 12, 2020 · 20 comments
Labels
Status: Awaiting Update ⏳ This issue requires more information before assistance can be provided. Type: Enhancement 💡 This is a feature or enhancement request. Type: Question ❔ This is a question or a request for support.

Comments

@chris-codaio
Copy link

Is your feature request related to a problem? Please describe.

GitHub is complaining that a transitive dependency has a vulnerability and I see no way in pipenv to force a resolution in the same way I currently can in the javascript ecosystem with yarn (https://classic.yarnpkg.com/en/docs/selective-version-resolutions/).

I'm using ScoutSuite which in turn relies on oci which relies on a version of cryptography that GitHub considers vulnerable. It looks like oci has no immediate plans to upgrade this bad dependency (oracle/oci-python-sdk#299). Since I don't use Oracle cloud, I'm happy if the oci dependency from ScoutSuite doesn't work. I just want to force update this dependency to resolve to version >= 3.2 where the issue is resolved.

Describe the solution you'd like

A resolutions section of the pipfile equivalent to yarn's resolutions section which behaves in the same way during install/resolution.

Describe alternatives you've considered

The only viable alternative I see is to fork one or both of the repos above and make the necessary changes myself - way too much effort for what should be a one-line change in my local pipfile.

Additional context

N/A

@frostming
Copy link
Contributor

frostming commented Nov 12, 2020

You can achieve this by updating the dependency version:

$ pipenv update

Or alternatively, you can pin the dependency version in your Pipfile:

[packages]
cryptography = ">=3.2"

Tell me if it solves your problem.

@frostming frostming added the Type: Question ❔ This is a question or a request for support. label Nov 12, 2020
@chris-codaio
Copy link
Author

No, this doesn't work as the ScoutSuite dependency relies on oci which has a dependency specifically to version 2.8 of the cryptography lib: https://github.com/oracle/oci-python-sdk/blob/master/requirements.txt#L5.

Attempting to override that locally using the suggestion above results in:

Locking [dev-packages] dependencies...
Locking [packages] dependencies...
Building requirements...
Resolving dependencies...
✘ Locking Failed!
[ResolutionFailure]:   File "/usr/local/Cellar/pipenv/2020.11.4/libexec/lib/python3.9/site-packages/pipenv/resolver.py", line 741, in _main
[ResolutionFailure]:       resolve_packages(pre, clear, verbose, system, write, requirements_dir, packages)
[ResolutionFailure]:   File "/usr/local/Cellar/pipenv/2020.11.4/libexec/lib/python3.9/site-packages/pipenv/resolver.py", line 702, in resolve_packages
[ResolutionFailure]:       results, resolver = resolve(
[ResolutionFailure]:   File "/usr/local/Cellar/pipenv/2020.11.4/libexec/lib/python3.9/site-packages/pipenv/resolver.py", line 684, in resolve
[ResolutionFailure]:       return resolve_deps(
[ResolutionFailure]:   File "/usr/local/Cellar/pipenv/2020.11.4/libexec/lib/python3.9/site-packages/pipenv/utils.py", line 1378, in resolve_deps
[ResolutionFailure]:       results, hashes, markers_lookup, resolver, skipped = actually_resolve_deps(
[ResolutionFailure]:   File "/usr/local/Cellar/pipenv/2020.11.4/libexec/lib/python3.9/site-packages/pipenv/utils.py", line 1093, in actually_resolve_deps
[ResolutionFailure]:       resolver.resolve()
[ResolutionFailure]:   File "/usr/local/Cellar/pipenv/2020.11.4/libexec/lib/python3.9/site-packages/pipenv/utils.py", line 818, in resolve
[ResolutionFailure]:       raise ResolutionFailure(message=str(e))
[pipenv.exceptions.ResolutionFailure]: Warning: Your dependencies could not be resolved. You likely have a mismatch in your sub-dependencies.
  First try clearing your dependency cache with $ pipenv lock --clear, then try the original command again.
 Alternatively, you can use $ pipenv install --skip-lock to bypass this mechanism, then run $ pipenv graph to inspect the situation.
  Hint: try $ pipenv lock --pre if it is a pre-release dependency.
ERROR: Could not find a version that matches cryptography==2.8,>=1.1.0,>=3.2 (from -r /var/folders/vd/r7ftxx9j2mx3gsyc80tsnskw0000gn/T/pipenv6f3i2losrequirements/pipenv-6wl5z1qj-constraints.txt (line 7))
Tried: 0.1, 0.2, 0.2.1, 0.2.2, 0.3, 0.4, 0.5, 0.5.1, 0.5.2, 0.5.3, 0.5.4, 0.6, 0.6.1, 0.7, 0.7.1, 0.7.2, 0.8, 0.8.1, 0.8.2, 0.9, 0.9.1, 0.9.2, 0.9.3, 1.0, 1.0.1, 1.0.2, 1.1, 1.1.1, 1.1.2, 1.2, 1.2.1, 1.2.2, 1.2.3, 1.3, 1.3.1, 1.3.2, 1.3.3, 1.3.4, 1.4, 1.5, 1.5.1, 1.5.2, 1.5.3, 1.6, 1.7, 1.7.1, 1.7.2, 1.8, 1.8.1, 1.8.2, 1.9, 2.0, 2.0.1, 2.0.2, 2.0.3, 2.1, 2.1.1, 2.1.2, 2.1.3, 2.1.4, 2.2, 2.2, 2.2.1, 2.2.1, 2.2.2, 2.2.2, 2.3, 2.3, 2.3.1, 2.3.1, 2.4, 2.4, 2.4.1, 2.4.1, 2.4.2, 2.4.2, 2.5, 2.5, 2.6, 2.6.1, 2.6.1, 2.7, 2.7, 2.8, 2.8, 2.9, 2.9, 2.9.1, 2.9.1, 2.9.2, 2.9.2, 3.0, 3.0, 3.1, 3.1, 3.1.1, 3.1.1, 3.2, 3.2, 3.2.1, 3.2.1
There are incompatible versions in the resolved dependencies:
  cryptography>=3.2 (from -r /var/folders/vd/r7ftxx9j2mx3gsyc80tsnskw0000gn/T/pipenv6f3i2losrequirements/pipenv-6wl5z1qj-constraints.txt (line 7))
  cryptography==2.8 (from oci==2.23.5->scoutsuite==5.10.1->-r /var/folders/vd/r7ftxx9j2mx3gsyc80tsnskw0000gn/T/pipenv6f3i2losrequirements/pipenv-6wl5z1qj-constraints.txt (line 10))
  cryptography>=1.1.0 (from adal==1.2.4->scoutsuite==5.10.1->-r /var/folders/vd/r7ftxx9j2mx3gsyc80tsnskw0000gn/T/pipenv6f3i2losrequirements/pipenv-6wl5z1qj-constraints.txt (line 10))

@frostming
Copy link
Contributor

frostming commented Nov 12, 2020

So if you have a package that has exactly pinned dependencies, it is impossible to have cryptography resolved to 3.2. It means oci only works under cryptography==2.8, any fancy commands or options can't help in this situation. You may have to ignore that security warning unless oci updates the dependency constraints.

@chris-codaio
Copy link
Author

I agree that this is how it works today, but I'm asking for a feature to override that pinned dependency locally.

@frostming
Copy link
Contributor

frostming commented Nov 12, 2020

No, it is not a flaw. You can't pin cryptography to 3.2 when 2.8 is pinned by oci and you can't override the dependency version without changing the code that can only work with 2.8, otherwise your app might get broken

@chris-codaio
Copy link
Author

I'm quite happy to take the risk in this case. That said, here are the reasons you might want to do this (from Yarn's site):

  1. You may be depending on a package that is not updated frequently, which depends on another package that got an important upgrade. In this case, if the version range specified by your direct dependency does not cover the new sub-dependency version, you are stuck waiting for the author.

  2. A sub-dependency of your project got an important security update and you don’t want to wait for your direct-dependency to issue a minimum version update.

  3. You are relying on an unmaintained but working package and one of its dependencies got upgraded. You know the upgrade would not break things and you also don’t want to fork the package you are relying on, just to update a minor dependency.

  4. Your dependency defines a broad version range and your sub-dependency just got a problematic update so you want to pin it to an earlier version.

@frostming
Copy link
Contributor

frostming commented Nov 12, 2020

Well, I am sold. In case the risk is known and acceptable.

I prefer to add an important flag to the entry in Pipfile, like what it does in CSS, meaning it should take precedence even incompatibility is found.

@frostming frostming added Status: Requires PEEP This issue requires an accompanying enhancement proposal Type: Enhancement 💡 This is a feature or enhancement request. labels Nov 12, 2020
@uranusjr
Copy link
Member

uranusjr commented Nov 12, 2020

I wrote a while ago about my thoughts on this for the pip equivalent to this pypa/pip#8076. TL;DR It is too much a footgun to allow users to override dependency information directly, and it would be better for all sides (the users, installer authors, maintainers of the package to be overriden) if the procedure can be done more transparently.

I proposed an alternative approach to the issue: pypa/pip#8076 (comment). This should not be too difficult to integrate this into pipenv (add a new section in Pipfile; perform the vendor+patch procedure automatically during sync), but someone needs to carry out the required design and implementation work. I don’t personally have a use for the feature at all and lack the interest on it.

@chris-codaio
Copy link
Author

I disagree that this is too much of a footgun. It seems to work quite well in the javascript ecosystem - unclear why the same pattern wouldn't work well here in python as well.

Here's a link to the RFC from javascript land (specifically the yarn tool): https://github.com/yarnpkg/rfcs/blob/master/implemented/0000-selective-versions-resolutions.md

@uranusjr
Copy link
Member

uranusjr commented Nov 12, 2020

You are free to disagree; this is what the PEEP procedure is for, to persuade pipenv maintainers. It is not nearly enough to say “JavaScript has this” since by the same measure we would have list.sort() work on heterogeneous lists and convert everything to str 🙂 It is not the way to design to decide on a solution first and reason it retrospectively; you need to describe the problem to solve, consider alternatives and their respective pros and cons, and only land on a solution if it offers the best trade-offs to the initial problem.

@mstrofbass

This comment has been minimized.

@balloob
Copy link

balloob commented Jan 20, 2022

(Edit: I thought this initially was the pip repo, not pipenv. Still think it's important feature)

I see mentions of it being a footgun. I think that it depends on how the feature is implemented. I would consider the implementation to be like constraints: set by passing a file. The user is aware of what they are doing because they are setting the CLI flag. Pip could also print warnings if the forced resolutions are incompatible to help a user that accidentally got to use a resolutions file.

I think that this is an important feature and it would solve all the problems big projects might have with the new resolver because it gives an escape hatch.

When we (Home Assistant) need to ship a security update, the only thing that matters is to ship it as soon as possible. By the time we are aware of a security incident, attackers often do too. Having to deal with backtracking issues and working with other packages to get it fixed delays release and keeps our users exposed.

I see that this issue has been tagged with "Requires PEEP" but I can't find any documentation about where to propose PEEPS besides a limited description in the Python wiki. Would love to write something up to see if we can get to an acceptable proposal?

@balloob
Copy link

balloob commented Jan 20, 2022

Another benefit of "resolutions" is that it could be used as a lock file speeding up CI builds.

(Edit: I thought this was the pip repo, not pipenv.)

@matteius
Copy link
Member

Another benefit of "resolutions" is that it could be used as a lock file speeding up CI builds.

Can you clarify what you mean by this? Pipenv already has the Pipfile.lock file that should be used by the CI for installing/syncing dependencies, so I am not sure that adding such a "resolutions" feature would have any speed benefit in builds. It makes sense otherwise that this could be a useful feature for the reasons mentioned earlier on in the thread.

I agree that the information about PEEPs are lacking, even the contributing.rst doc doesn't mention them. I found out the most about what we already have by reading through them here but from what I gather, it is basically a PR process that would create the PEEP following the template.

@mstrofbass

This comment has been minimized.

@matteius
Copy link
Member

@mstrofbass It is fine to suggest that pipenv can do similar to what other libraries have done, but keep in mind that this is an open source project where anyone can contribute, so if it is such a trivial thing to do I encourage you to look into making a pull request or adding constructive feedback that could lead to such an improvement. That being said, I should remind you that:

Pipenv has one very important rule governing all forms of contribution, including reporting bugs or requesting features. This golden rule is "be cordial or be on your way" All contributions are welcome, as long as everyone involved is treated with respect. More details here: https://github.com/pypa/pipenv/blob/main/docs/dev/contributing.rst#be-cordial

@mstrofbass
Copy link

@matteius What do you think ignoring existing solutions to real problems that impact your users and instead putting the onus on them to convince you that it's the right way to solve their problem (which they're not even subject matter experts on), while also not offering to solve the problem in any other way, is if not disrespectful?

But hey, at least that's in the documentation.

@matteius
Copy link
Member

@mstrofbass I've been there before feeling negatively about a different project, lets remain constructive. Correct me if I am wrong but yarn is a package manager for mostly javascript right? I have primarily used npm for that purpose so I may lack context, but my question is along the line of: how is it reasonable to think that because another open source software technology (developed in other languages using other supporting libraries) has an abstraction you see as viable in this tool, pipenv--how is that perceived as trivial to just have it rolled out here in this tool?

I have been helping out with this project's issues backlog for just over a month now and up until a few months ago, had never used pipenv. Nobody has closed this issue as not relevant, and even above one of the key contributors, @frostming has stated support for such a feature:

Well, I am sold. In case the risk is known and acceptable.

The issue is that its not trivial, and while 14 months may seem like a very long time in software, its honestly not in terms of open source software that is improved by volunteer efforts of mostly developers that have other fulltime jobs. While the syntax from yarn might make sense in this context, I am fairly confident that the supporting implementation that would have to be done from within the context of pipenv would look entirely different.

I want to be cordial to you as well, as nothing was deleted and you are still invited to help towards a path forward and solution on how to offer such an analogous abstraction in the land of pip and pipenv.

@balloob
Copy link

balloob commented Jan 21, 2022

I agree with @matteius. Please limit this conversation to try to solve this issue and not distract from it's topic.

Can you clarify what you mean by this? Pipenv already has the Pipfile.lock file

Right, my bad.

@matteius
Copy link
Member

Could this be rechecked on pipenv==2022.8.19?

@matteius matteius added Status: Awaiting Update ⏳ This issue requires more information before assistance can be provided. and removed Status: Requires PEEP This issue requires an accompanying enhancement proposal labels Aug 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting Update ⏳ This issue requires more information before assistance can be provided. Type: Enhancement 💡 This is a feature or enhancement request. Type: Question ❔ This is a question or a request for support.
Projects
None yet
Development

No branches or pull requests

6 participants