Closed stekern closed 2 years ago
Hi there,
We have found that there's a problem with the logs. Depending on which situation applies follow one, some or all of these instructions.
If there's no log posted yet, we need you to find and copy/paste the log into the issue template.
This bug report is missing some necessary information. To start with, you didn't mention it, but is the problem related to lock file maintenance?
@rarkins Sorry, forgot to mention that. Lock file maintenance is not enabled in the affected repository. It's related to the automatic update of a dependency ansi-regex
. All of the commits have the following message:
chore(deps): update dependency ansi-regex to 5.0.1 [security]
In that case, please locate the logs for one branch on the last two times it was updated. You're looking for the runs where you see a docker run
command in the logs (this is how lock files are updated).
I've briefly checked the logs, but nothing really pops out. The are no errors or warnings. From what I can tell, this is the most relevant part.
If you take a look at commit https://github.com/capralifecycle/webapp-deploy-lambda/commit/26f51446024257fe575c188d7c10ed438ef65931 and https://github.com/capralifecycle/webapp-deploy-lambda/commit/78223adbf61ac51a46f0ba326d2a93d20d78d91e, you can see the change Renovate keeps cycling between.
That log is missing a little bit (it's truncated, does not include any npm command) and we need it for both commits.
Seems like another key piece of info about this is that it's a vulnerability remediation PR?
Yep, this is an automerged vulnerability remediation PR. (On that note, may it make things a bit easier for the maintainers by adding an optional field to the bug template for describing the type of update? Lock file maintenance, direct dependency update, vulnerability remediation, ...?)
Is there a way to temporarily disable updates for this specific vulnerability to avoid getting these changes? The commit history on our main branch is becoming a bit of a mess with all of these updates. I assume we can just manually remove the Dependabot alert from the GitHub repository, but we are waiting for an upstream library to make the required changes on their side, so we don't really want to close it before it's fixed. Alternatively we could temporarily turn off automerge.
Since the logs are quite long, I initially only wanted to share the relevant snippets to make troubleshooting easier and to avoid potentially leaking anything that can be considered sensitive. It's not immediately apparent to me what is and isn't relevant in this case, so I've sifted through the full logs and I'm sharing them below (with some minor redactions).
Yep, this is an automerged vulnerability remediation PR. (On that note, may it make things a bit easier for the maintainers by adding an optional field to the bug template for describing the type of update? Lock file maintenance, direct dependency update, vulnerability remediation, ...?)
There is a limit to how many fields we can add to the bug template before it becomes too difficult to use. There are a LOT of different types of use, not all relate to an update. Thankfully most people voluntarily describe their problem in detail.
Hi there,
Help us by making a minimal reproduction repository.
Before we can start work on your issue we first need to know exactly what's causing the current behavior. A minimal reproduction helps us with this.
To get started, please read our guide on creating a minimal reproduction to understand what is needed.
We may close the issue if you (or someone else) have not provided a minimal reproduction within two weeks. If you need more time, or are stuck, please ask for help or more time in a comment.
Good luck,
The Renovate team
In both the logs you gave, the branch already existed, whereas we need to see a job when the branch/commit was first created to understand why. A public reproduction would allow us to debug/troubleshoot it directly.
In terms of making the problem go away, you could try setting "transitiveRemediation": false
in your config to stop this type of nested vulnerability fixing (but it will be for every transitive vulnerability).
@rarkins First of, thanks for the quick response and help.
I understand that you can't include all types of optional parameters to cover all types of different use-cases to the bug template, but I figured it was worth suggesting at least in the case it could be used as a common discriminator. I do try to add the necessary amount of detail to the bug reports I submit, but apologies for the lack in this case. Especially for projects one might not be very familiar with, there are nuances that can make it easy to forget to include something of importance (e.g., it was not immediately clear to me which logs are actually useful to share in this situation).
The issue has occurred across a wide range of our repositories, so it's not an isolated incident. I figured the public repository I mentioned in this bug report could serve as the reproduction repository, but it unfortunately seems like the logs you requested from when the branch was first created has expired (?), unfortunately.
I've set up a minimal repository https://github.com/stekern/repro-renovate-13977 with the same Dependabot alert, and I'm waiting for Renovate to take action.
Perfect! Thank you for taking the time for the reproduction - I'll take a look
Alright, I've managed to reproduce the issue in my new repository.
Renovate first creates a commit that performs the only automatically fixable change for the Dependabot alert in question (through npm audit fix
I assume). This is exhibited in https://github.com/stekern/repro-renovate-13977/commit/c6e95adad28fd00c87e926346c6e5baf4ae76c39.
This commit is then merged in, and after this I get a series of commits from Renovate that just seem to move dependencies up and down the dependency tree. It seems like every time Renovate scans the repository, it creates a commit that moves a dependency up the dependency tree (https://github.com/stekern/repro-renovate-13977/commit/344624e9f8ac646dceb4fbabeb4b5dfaa8a3f9da) followed by a new commit that reverses this change (https://github.com/stekern/repro-renovate-13977/commit/194c08fe2985f94450c241e092e2df6811c8653b).
FYI the problem is essentially because the npm
package itself "bundles" its dependencies, which is a really weird way to handle dependencies, and it's fooling our remediation. Our remediation makes a change but then running npm install
puts the vulnerable package back in. The reason why it's such a problem for you is because of the combination of ignoreTests
and automerge
.
We should probably stop sending any remediation for npm
children, as they always embed ?
I'm planning to abort any time we find that the locked dependency we need to update is part of a bundle
ignoreTests
was only enabled because I wanted to enable automerge
in a repository without any checks. The issue still occurs in other repositories where we have checks that Renovate needs to wait for (i.e., where ignoreTests
is disabled).
Is this a bug that should be handled by Renovate, or is it something that end-users should handle themselves in their configuration?
This is the first time I've had issue with the automerge
feature. It's a really valuable feature, and it would be a shame if the only way to mitigate the issue is to disable it. As a hotfix, we could close the Dependabot alert on our side to avoid this specific issue, but I assume it can crop up again when another vulnerability appears.
I'm hoping to handle this ASAP, or at least be able to tell you if a hotfix is unlikely quickly. If you'd like, I can turn off transitive remediation for your org while you wait? I think it's only turned on by default for public repos right now
We've determined that the CVE in question is not relevant for the affected repositories and have closed the Dependabot alert. Renovate hasn't made any similar commits after that, so we should be fine until a permanent fix is in place unless you believe there's a high probability the bug may appear in the next couple of days... (there would need to be a new Dependabot alert where npm
is involved for this to happen again, no?)
Won't be necessary to make any temporary, organization-specific changes on your end, but thanks for the offer. We're using an org-wide config, so we'd effectively be able to globally disable it ourselves.
Sounds resolved for now but I'm aiming for a fix too
:tada: This issue has been resolved in version 31.67.1 :tada:
The release is available on:
31.67.1
Your semantic-release bot :package::rocket:
How are you running Renovate?
WhiteSource Renovate hosted app on github.com
If you're self-hosting Renovate, tell us what version of Renovate you run.
No response
Please select which platform you are using if self-hosting.
No response
If you're self-hosting Renovate, tell us what version of the platform you run.
No response
Was this something which used to work for you, and then stopped?
It used to work, and then stopped
Describe the bug
Renovate is continuously making changes to our NPM lockfile by 1) moving a specific dependency up and then later 2) moving it down the dependency tree again, and then moving it up again and continuing in a loop.
We have now accumulated over 10 of these commits, and I expect there are more to come.
See repository https://github.com/capralifecycle/webapp-deploy-lambda for an example.
Relevant debug logs
No response
Have you created a minimal reproduction repository?
No reproduction, but I have linked to a public repo where it occurs