jazzband / pip-tools

A set of tools to keep your pinned Python dependencies fresh.
https://pip-tools.rtfd.io
BSD 3-Clause "New" or "Revised" License
7.67k stars 608 forks source link

Skip constraint path check #2038

Closed honnix closed 5 months ago

honnix commented 8 months ago

pip can read constraint file via HTTP so pip-compile doesn't need to restrict that to a local file. Removing the option type makes it a plain string so users can pass in an URL. This of course skip local file validation, but the compilation would fail anyway if the file is not readable, no matter local or remote.

Closes #2040

Contributor checklist
Maintainer checklist
chrysle commented 7 months ago

I don't think removing the type altogether is the right solution. Failing early with a definite traceback is better, in my opinion.

honnix commented 7 months ago

I don't think removing the type altogether is the right solution. Failing early with a definite traceback is better, in my opinion.

It's not that early I guess but I can understand this.

honnix commented 7 months ago

I tried early validation in #2040 but I honestly feel it is overshooting.

honnix commented 7 months ago

If I understand it correctly, this command line option is more or less transparently passed down to pip internal, so it doesn't help much applying an additional layer of validation.

chrysle commented 7 months ago

Now I wasn't urging you to write an own enhanced parameter type! You must need this feature badly ;-).

I think I'm going to ask over at click whether they want to extend click.Path for URLs permanently. Until then, I'd like to hear other opinions.

honnix commented 7 months ago

@chrysle Haha, I never tried extending click type, so it was a fun exercise.

Until then, I'd like to hear other opinions.

Yeah that makes sense.

webknjaz commented 7 months ago

Honestly, this strikes me as dangerous and it removes the immutability/reproducibility property of the constraint files. This makes it possible to influence the resolved deptree by modifying an externally managed file on the internet which subsequently opens up a can of worms, allowing for rollback attacks.

If this is implemented, it shouldn't behave like this by default and should only be toggled by the end-user with a sufficiently cautious flag name like --allow-dangerous-insecure-and-unverified-constraints-for-rollback-attacks.

honnix commented 7 months ago

Honestly, this strikes me as dangerous and it removes the immutability/reproducibility property of the constraint files. This makes it possible to influence the resolved deptree by modifying an externally managed file on the internet which subsequently opens up a can of worms, allowing for rollback attacks.

If this is implemented, it shouldn't behave like this by default and should only be toggled by the end-user with a sufficiently cautious flag name like --allow-dangerous-insecure-and-unverified-constraints-for-rollback-attacks.

It is a valid concern, but this is not a feature of pip-compile but pip itself. pip always supports reading a file from HTTP(S) for constraints and requirements. This feature has been used by for example Airflow for some time: https://raw.githubusercontent.com/apache/airflow/constraints-2.4.3/constraints-3.10.txt. So there is no reason that pip-compile should block it. Today users of pip-compile can already use a constraints file from HTTP(S) in a *.in file header for example.

webknjaz commented 7 months ago

I think that's exactly the reason for doing it by default.

honnix commented 7 months ago

I think that's exactly the reason for doing it by default.

Not sure I understand this. What do you mean as "by default"? I think the original PR adding -c option to pip-compile might just have missed the fact that URL is supported by pip.

webknjaz commented 7 months ago

I'm not sure pip-tools should support everything that pip does the same way. Its purpose is to aid reproducibility and constraints that are mutable at random points in time aren't exactly that.

webknjaz commented 7 months ago

Whether it was overlooked is something that only the original PR author knows, I think.

honnix commented 7 months ago

I'm not sure pip-tools should support everything that pip does the same way. It's purpose is to aid reproducibility and constraints that are mutable at random points in time aren't exactly that.

The thing is, pip-compile already supports this.

In requirements.in:

-c https://example.com/some/constraints.txt

foo
bar

pip-compile ... requirements.in

There is no reason to not to support it via command line option I think.

honnix commented 7 months ago

In terms of reproducibility, I think it boils down to how the remote constraints file is served and digested. A local file does not ensure reproducibility either.

honnix commented 7 months ago

I'm not exactly sure why CI / Linters / linkcheck-docs (pull_request) failed but it doesn't seem to be related to the change.

honnix commented 7 months ago

Please let me know how I can move this forward together with #2040. Thank you.

webknjaz commented 6 months ago

I'm not exactly sure why CI / Linters / linkcheck-docs (pull_request) failed but it doesn't seem to be related to the change.

Try rebasing. It's probably fixed on main.

As for the change, I guess it's okay to make the CLI behave the same way as what's in the input files.

honnix commented 5 months ago

Please help merge this when suitable. Thank you all for the reviews and suggestions.

chrysle commented 5 months ago

Thank you!