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

fix #8414 follow poetry source constraint #8422

Conversation

lucemia
Copy link
Contributor

@lucemia lucemia commented Nov 17, 2023

Context

Package source constraints are strongly suggested for all packages that are expected to be provided only by one specific source to avoid dependency confusion attacks.

https://python-poetry.org/docs/repositories/#package-source-constraint

Summary

  • Update the index_finder.rb script to ensure that if a dependency has a package source configuration in pyproject.toml, it won't utilize any other sources as its :main source.
  • Please note that the behavior may not completely align with Poetry's behavior.
    • For example, if a package points to a source that is not defined in pyproject.toml, Poetry will raise an exception, but Dependabot will continue to check the default source.
    • Should we consider aligning dependabot behavior with Poetry's in this regard?

User-facing changes

  • dependabot bot won't check default source if a package defined its source correctly.

Testing Instructions

[dependabot-core-dev] ~ $ ./bin/dry-run.rb pip lucemia/dependabot-source-constraint --cache=files

Before modification

  • dependabot will check https://pypi.org/simple/requests/
=> reading cloned repo from /home/dependabot/tmp/lucemia/dependabot-source-constraint
=> parsing dependency files
=> updating 1 dependencies: requests

=== requests ()
 => checking for updates 1/1
🌍 --> GET https://pypi.org/simple/requests/
🌍 <-- 200 https://pypi.org/simple/requests/
🌍 --> GET https://some.internal.registry.com/pypi/requests/
 => handled error whilst updating requests: private_source_timed_out {:source=>"https://some.internal.registry.com/pypi/"}
🌍 Total requests made: '2'
🎈 Ecosystem Versions log: {:languages=>{:python=>{"raw"=>"^3.7", "max"=>"3.11"}}}

After modification

  • dependabot won't check https://pypi.org/simple/requests/
=> reading cloned repo from /home/dependabot/tmp/lucemia/dependabot-source-constraint
=> parsing dependency files
=> updating 1 dependencies: requests

=== requests ()
 => checking for updates 1/1
🌍 --> GET https://some.internal.registry.com/pypi/requests/
 => handled error whilst updating requests: private_source_timed_out {:source=>"https://some.internal.registry.com/pypi/"}
🌍 Total requests made: '1'
🎈 Ecosystem Versions log: {:languages=>{:python=>{"raw"=>"^3.7", "max"=>"3.11"}}}

@lucemia lucemia closed this Nov 24, 2023
@lucemia lucemia reopened this Nov 24, 2023
@lucemia lucemia force-pushed the fix-8414-follow-poetry-source-constraint branch 2 times, most recently from 9e5ac3e to d1b083e Compare November 24, 2023 04:44
@lucemia lucemia marked this pull request as ready for review November 24, 2023 04:58
@lucemia lucemia requested a review from a team as a code owner November 24, 2023 04:58
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me!

@deivid-rodriguez
Copy link
Contributor

Regarding your question, yes, it makes to me to align with poetry. If poetry fails to run that kind of project, since we run poetry ourselves, we're already running into an unknown error anyways. So it makes sense to raise a proper user error instead. Maybe DependencyFileNotEvaluatable or MisconfiguredTool error.

@lucemia lucemia force-pushed the fix-8414-follow-poetry-source-constraint branch from d1b083e to 047d29c Compare November 24, 2023 12:32
@lucemia
Copy link
Contributor Author

lucemia commented Nov 24, 2023

Regarding your question, yes, it makes to me to align with poetry. If poetry fails to run that kind of project, since we run poetry ourselves, we're already running into an unknown error anyways. So it makes sense to raise a proper user error instead. Maybe DependencyFileNotEvaluatable or MisconfiguredTool error.

I've squashed the commits, and I think the PR may be also to merged first.

To align with Poetry, it will require more time as I want to clarify Poetry's behavior. I have raised an issue there to discuss further: python-poetry/poetry#8704."

@deivid-rodriguez
Copy link
Contributor

Thank you! Yes, I will merge this PR independently of any other improvements. Hopefully next week!

@deivid-rodriguez deivid-rodriguez force-pushed the fix-8414-follow-poetry-source-constraint branch from 047d29c to 86c7998 Compare November 27, 2023 20:55
@deivid-rodriguez deivid-rodriguez enabled auto-merge (squash) November 27, 2023 20:55
@deivid-rodriguez deivid-rodriguez merged commit ef4ef5b into dependabot:main Nov 27, 2023
81 checks passed
@deivid-rodriguez
Copy link
Contributor

Thanks for the improvement @lucemia, it's now merged and deployed 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

not follow poetry's package source constraint
2 participants