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(worker/repository): add normalized match for pip alertPackageRules #28214

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

not7cd
Copy link
Contributor

@not7cd not7cd commented Apr 2, 2024

Changes

Adds additional normalized name in matchPackageName for alerts coming from vulnerability alerts. This should catch dependencies in managers using pep621 standards, poetry, and pip-compile lock files.

Context

Workaround for #27069, may be superseded by #27733.
There is an inconsistency how Renovate handles Python package names. When it comes to vulnerability alerts coming from GitHub dependabot, they are taken as is in their canonical form (ex.: Pillow). If a normalized name (pillow) is used in a file, the update will be ignored and not created.

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@not7cd not7cd changed the title fix(worker/repository): additional name match for alertPackageRules fix(worker/repository): add normalized match for pip alertPackageRules Apr 3, 2024
@not7cd
Copy link
Contributor Author

not7cd commented Apr 3, 2024

In rare instances it can result in two separate PRs, but it should be better to create two than none or one that addresses half of the problem.

@gzmarstone
Copy link

Anything I can do to help with getting traction on this PR? Would love to see this get released.

@viceice viceice added this pull request to the merge queue Apr 18, 2024
Merged via the queue into renovatebot:main with commit dfbb054 Apr 18, 2024
37 checks passed
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 37.306.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gzmarstone
Copy link

I am now seeing that the vulnerabilities are being found for Pillow, but the pull request is not being updated with either [security] in the title or the information about the vulnerabilities in the body of the pull request. I am assuming there is another match that needs to have the name normalized for the compare?

@not7cd
Copy link
Contributor Author

not7cd commented Apr 19, 2024

I guess that's good news. Do you have logs or the PR in a public repo @gzmarstone? I suspect that if PR was created before it was matched for vulnerability, it may not be updated. But I'm no expert in that matter.

I would also push more towards name normalization everywhere and not a workaround such as this one.

@not7cd not7cd deleted the fix/pypi-alerts-name-normalization branch April 19, 2024 16:20
@gzmarstone
Copy link

Its not a publc repo, I was wondering if it would re-create it or not, I had left the vulnerability around so it could be used as a test when this got fixed. Looking at the logs there are no differences in logging between the vulnerabilities that are marked [SECURITY] and those that are not. I tried modifying the config which made it close the existing pull request, then reverted the config and it still did not add [SECURITY]

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants