Disable hashes for requirements.txt if a VCS dependency is encountered

[ghost]

Pull Request Check List

Resolves: python-poetry/poetry-plugin-export#69

• Added tests for changed code.
• Updated documentation for changed code.

[ghost]

Something is wrong, it's not actually outputting the requirements.txt file.

[zyv]

This would improve the situation w.r.t. moneymeets/python-poetry-buildpack#19, but is disabling hashes completely actually a good idea? Why to omit hashes only for VCS dependencies?

[ghost]

Because pip is dumb. You either have hashes for all packages or none of them, you can't mix it.

[ghost]

I forgot to unmark this as a draft. I've rebased the PR to fix the merge conflict, should be ready for review/merging now.

[AlexChalk]

Hi @nyuszika7h, is this still a live issue/PR?

[ghost]

Yes, it is still relevant. Sorry for the late reply.

[ghost]

I've rebased the branch to be compatible with the latest changes.

[AlexChalk]

@nyuszika7h It's probably worth @ing a maintainer to ask for a review, do you know who the right person would be?

[zyv]

AFAIK, maintainers are @finswimmer and @abn , so maybe the could voice their opinion?

I’m personally -1 on automatically enabling —without-hashes, because it has security implications and in my opinion should be an explicit and conscious decision. Too bad that hashes apparently cannot be omitted for VCS dependencies only. This would habe been a good idea in my view.

There have been some discussions in the related pip bug however in the last few weeks:

pypa/pip#6469

It seems that the maintainers would be happy to accept a per-requirement opt out as the first step towards solving this problem. Maybe if you have time @nyuszika7h you could try to solve the problem at the pip side first?

[ghost]

Trust me, it's not going to get fixed in pip anytime soon. pypa/pip#4995 has been open since 2018 and pypa/pip#6469 since 2019. Their stance basically seems to be "Poetry or whoever needs this should fix it themselves".

I specifically coded this PR to explicitly warn about hashes being disabled in such cases, so it should not reduce security in most cases, unless it's run as part of an automated script and log output is not reviewed. I'd be slightly less happier but also OK with the alternative of simply refusing to generate a requirements.txt in such cases unless --without-hashes is explicitly passed. Or maybe something like --without-hashes-if-vcs (with some better name). Anything is better than the status quo, where Poetry happily generates an invalid requirements.txt file that pip will refuse to install from.

[zyv]

@nyuszika7h maybe I’m being too optimistic since I didn’t ever have to work with pip maintainers, but they explicitly said on multiple occasions the PRs addressing the problem will be reviewed and accepted, and no such PRs seemed to happen… so yeah, the issues have been open for a very long time, but I didn’t get an impression that they are blocking people, rather nobody seems to be interested enough to try to fix it.

[ghost]

The problem is that there isn't a clear-cut solution to it. Should pip support hashing VCS dependencies somehow, or ignore the lack of hashes unless --require-hashes is used (which follows the principle of least surprise, but some may argue it's a downgrade in security from the current behavior)? I could try, but I don't expect it to be resolved quickly.

It seems to me like this behavior isn't necessarily considered a bug by pip maintainers, even though they'd be potentially open to changing it.

I would still argue that until it is resolved, Poetry should stop producing invalid requirements.txt files one way or another (whether that is the current proposed patch or aborting if VCS dependencies are encountered and --without-hashes is not used). Pushing the responsibility back and forth isn't going to achieve anything. This has been the behavior of pip for years, and a conservative approach in Poetry would not break existing uses if pip decided to be more permissive in the future, it would just have to be updated to allow it on pip >= X.Y, and meanwhile people could still use --without-hashes.

[zyv]

The problem is that there isn't a clear-cut solution to it. Should pip support hashing VCS dependencies somehow, or ignore the lack of hashes unless --require-hashes is used (which follows the principle of least surprise, but some may argue it's a downgrade in security from the current behavior)? I could try, but I don't expect it to be resolved quickly.

Yes, the 100% solution is to teach pip how to hash VCS requirements, but this is complicated and I think there is no consensus how to proceed with that.

However, in pypa/pip#6469 it was proposed to first add --hash=skip so that hashes can be used for non-VCS requirements and disabled for VCS requirements. If this is done, then poetry could add --hash=skip and warn the used instead of hard-fail. This I personally would consider "safe enough", because pip can't do better, only worse.

I would still argue that until it is resolved, Poetry should stop producing invalid requirements.txt files one way or another (whether that is the current proposed patch or aborting if VCS dependencies are encountered and --without-hashes is not used). Pushing the responsibility back and forth isn't going to achieve anything.

I agree with you that producing broken requirements.txt is simply wrong. I would prefer poetry to fail and require --without-hashes explicitly instead of silently enabling it - but I'm not the one who makes decisions here - this is just my opinion :-)

[neersighted]

Closing this as the code has moved into https://github.com/python-poetry/poetry-plugin-export -- please discuss/PR this over there.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.