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

Support configuring allowed updates for security updates using config file #2521

Open
kyeotic opened this issue Sep 11, 2020 · 32 comments
Open
Labels

Comments

@kyeotic
Copy link

kyeotic commented Sep 11, 2020

I have configured a dependabot.yml that should ignore devDependencies

version: 2
updates:
  # Maintain dependencies for npm
  - package-ecosystem: "npm"
    directory: "/"
    schedule:
      interval: "daily"
    allow:
      - dependency-name: "*"
        dependency-type: "production"
    ignore:
      - dependency-name: "*"
        dependency-type: "development"

Despite this I am still getting PR's for devDependency warnings

What do I need to do to stop these from happening?

@kyeotic kyeotic added the T: bug 🐞 Something isn't working label Sep 11, 2020
@jurre
Copy link
Member

jurre commented Sep 11, 2020

Hi, that config file looks to be invalid: https://github.com/kyeotic/raviger/runs/1097204025

You cannot specify dependency-type in the ignore condition, but since you already have an allow rule set up to match only production dependency types, I think you can get rid of that ignore field entirely and it should work.

Lmk if that works 👍

@kyeotic
Copy link
Author

kyeotic commented Sep 11, 2020

I added the ignore field after the allow failed to stop a PR yesterday. I will remove ignore again and give it another day I guess.

@kyeotic kyeotic closed this as completed Sep 14, 2020
@kyeotic
Copy link
Author

kyeotic commented Nov 17, 2020

It's happening again

with this config

version: 2
updates:
  # Maintain dependencies for npm
  - package-ecosystem: "npm"
    directory: "/"
    schedule:
      interval: "daily"
    allow:
      - dependency-name: "*"
        dependency-type: "production"

@kyeotic kyeotic reopened this Nov 17, 2020
@kyeotic
Copy link
Author

kyeotic commented Nov 19, 2020

@jurre I realized you probably wouldn't get notified from an issue re-opening... so :bump:

@jurre
Copy link
Member

jurre commented Nov 20, 2020

@feelepxyz do you have an idea what's going on here?

@feelepxyz
Copy link
Contributor

@kyeotic pretty sure that's because it's a security update fixing a vulnerability in lodash: GHSA-p6mc-m468-83gw

You currently can't configure allowed updates for security updates using the config file. Currently only the following config file options are used for security updates:

  • assignees
  • commit-message
  • ignore
  • labels
  • milestone
  • pull-request-branch-name.separator
  • rebase-strategy
  • reviewers
  • vendor
  • versioning-strategy

You'd have to ignore the dependencies specifically as we don't support ignoring dependency-type yet.

@kyeotic
Copy link
Author

kyeotic commented Nov 20, 2020

@feelepxyz That's surprising. Is this explained in the docs anywhere?

I think wanting to ignore dependabot warnings for devDependencies is a pretty common request. I don't see why this separate and additional category of "security updates" is even in the equation. If I say I don't want devDependency warnings I mean I don't want any warnings for any devDependencies. Frankly, this behavior is wrong.

@kyeotic
Copy link
Author

kyeotic commented Nov 20, 2020

I guess I'm just going to have to disable dependabot entirely.

@feelepxyz
Copy link
Contributor

Good feedback and legitimate use-case! Will add this to our backlog to fix, we haven't done much to make both products configurable using the same config file but are looking at changing this.

I guess I'm just going to have to disable dependabot entirely.

You can also only disable security updates from the GitHub UI:

Screenshot 2020-11-20 at 17 46 11

@ronjouch
Copy link

ronjouch commented Nov 27, 2020

@kyeotic for findability, can you pls rename this issue to something like Dependabot *security* updates should accept a "production dependencies only" mode ?

Hope this lands someday.

@ronjouch
Copy link

Also, @feelepxyz @jurre reading dependabot-security docs, it's not clear to me how yml-configurable this "security" product is. Before this issue I hadn't understood it was configurable, at all. Does dependabot-security read dependabot.yml? Will both dependabot and dependabot-security run? Since having a dependabot.yml is how you enable dependabot, how do I for example configure reviewers for my security alerts, but without enabling the full dependabot? Am I missing documentation?

@feelepxyz feelepxyz changed the title Still getting devDependency PR's despite use of allow and ignore Support configuring allowed updates for security updates using config file Dec 3, 2020
@feelepxyz
Copy link
Contributor

Also, @feelepxyz @jurre reading dependabot-security docs, it's not clear to me how yml-configurable this "security" product is. Before this issue I hadn't understood it was configurable, at all. Does dependabot-security read dependabot.yml? Will both dependabot and dependabot-security run? Since having a dependabot.yml is how you enable dependabot, how do I for example configure reviewers for my security alerts, but without enabling the full dependabot? Am I missing documentation?

Yeah it's currently very patchy but we have a plan to fix it! You currently can't configure security updates using the config file without also enabling version updates, if you do want to enable version updates the following config file options will also apply to security updates: #2521 (comment)

@kyeotic
Copy link
Author

kyeotic commented Dec 3, 2020

@feelepxyz Sounds good, I will disable dependabot-security until thats fixed.

As a user, I have to say that this distinction is surprising and frustrating. I understand that they are internally considered two independent products, possibly developed by different teams, but they serve similar functions, have similar names, with similar operation and similar configuration so as a user I don't see them as independent, but as parts of a whole. I expect the difference in security and regular (?) to be a kind of alert, not a completely separate application with separate configuration. I would like to see them use a single configuration, possibly with a threshold or minimumAlertLevel for each kind.

@ronjouch
Copy link

ronjouch commented Jun 16, 2021

@feelepxyz @jurre any news on making Dependabot security updates configurable? (Either through a dependabot.yml, or through checkboxes in https://github.com/<org>/<project>/settings/security_analysis )

For my desired use case (1. security vulns only, 2. npm prod deps only, 3. automatic assignment to a list of reviewers) I'm still torn between two unsatisfying options:

  1. Use a dependabot.yml to get above points [2, 3], but miss point [1] and be overwhelmed with non-security upgrades.
  2. Use the barebones Dependabot security updates feature integrated into GitHub and get above point [1], but miss points [2, 3] and have PRs for all deps (as opposed to only prod deps) and lacking assignment to reviewers.
    • Note: point 2. "prod deps only" isn't the biggest deal for me. What matters most to me is point 3. "automatically assignment to a list of reviewers", else these PRs are just floating here and nobody looks at them.

Has there been progress around this? Can we hope to see progress soon? Thanks.


(Issue subscribers: sorry for the lame "bump" comment, I never write those in open source projects because I hate causing noise to issue subscribers, but I'm paying for dependabot/GitHub and it's been six months since @feelepxyz wrote "we have a plan to fix it", so here's me doing an exception to this rule 🤷)

@feelepxyz
Copy link
Contributor

feelepxyz commented Jun 17, 2021

@ronjouch yeah it's kinda possible to configure security updates through dependabot.yml, this is a bit of a hack atm but should work:

version: 2
updates:
  - package-ecosystem: npm
    directory: "/"
    schedule:
      interval: "monthly"
    open-pull-requests-limit: 0 # in case you don't want to enable version updates
    allow:
      - dependency-type: "production"
    reviewers:
      - username

Security update PR should inherit this config as the config object is shared between version and security updates as long as the ecosystem, directory matches on the default branch.

We're planning to make this a lot nicer.

@ronjouch
Copy link

It's kinda possible to configure security updates through dependabot.yml, this is a bit of a hack atm but should work [...]. Security update PR should inherit this config as the config object is shared between version and security updates as long as the ecosystem, directory matches on the default branch.

@feelepxyz oooooh neat, trying this now. Thanks!

We're planning to make this a lot nicer.

Cool. I'm staying subscribed to this issue, will try the nicer thing if/when it ships and undo the dependabot.yml hack. Until then, the hack, if it works, will do very well for my use case. Thanks again!

@ronjouch
Copy link

ronjouch commented Jun 17, 2021

Security update PR should inherit this config as the config object is shared between version and security updates as long as the ecosystem, directory matches on the default branch

@feelepxyz one more thing about this detail: does this mean that I should not have a second package-ecosystem in the dependabot.yml ?

Said differently, will a second package-ecosystem break your ecosystem/directory matching logic and this hack of extending dependabot/security config from dependabot.yml?

@feelepxyz
Copy link
Contributor

will a second package-ecosystem break your ecosystem/directory matching

Nope, you can have any number and they should match on ecosystem + directory so you can have version updates enabled for some and security updates for all (not yet possible to disable security updates on some projects).

alfjjacob pushed a commit to hpcc-systems/Tombolo that referenced this issue Nov 23, 2021
…vDependencies

a lot of the vulnerabilities are from react-scripts package which is just a build tool and should not affect the product facebook/create-react-app#11174. dependabot config dependabot/dependabot-core#2521
sverchdotgov added a commit to usds/justice40-tool that referenced this issue Dec 13, 2021
Showing obscure vulnerabilities that only exist in the dev setup creates
more noise and means that they just get ignored (because they are
probably low priority). Silencing them means when we get a vulnerable
dependency alert we know to pay attention to it.

Comes from dependabot/dependabot-core#2521 and
hpcc-systems/Tombolo@501bbef.
sverchdotgov added a commit to usds/justice40-tool that referenced this issue Dec 13, 2021
Showing obscure vulnerabilities that only exist in the dev setup creates
more noise and means that they just get ignored (because they are
probably low priority). Silencing them means when we get a vulnerable
dependency alert we know to pay attention to it.

Comes from dependabot/dependabot-core#2521 and
hpcc-systems/Tombolo@501bbef.
nicks added a commit to tilt-dev/tilt that referenced this issue Dec 17, 2021
nicks added a commit to tilt-dev/tilt that referenced this issue Dec 17, 2021
…endency vulnerabilities

I have no idea why this isn't the default, here are some threads on the issues:
dependabot/dependabot-core#4146
dependabot/dependabot-core#2521
facebook/create-react-app#11174
nicks added a commit to tilt-dev/tilt that referenced this issue Dec 17, 2021
…endency vulnerabilities (#5304)

I have no idea why this isn't the default, here are some threads on the issues:
dependabot/dependabot-core#4146
dependabot/dependabot-core#2521
facebook/create-react-app#11174
@ls-valentinas-bakaitis
Copy link

@jurre @feelepxyz any update on this issue? Being able to ignore devDependencies would help to massively reduce the number of false positives we get.

@GuillaumeBenini
Copy link

Will we be able to get support for a private registry config with the security updates? We have our own artifactory for npm dependencies and the PRs dependabot creates for security updates doesn't match the registry we setup in the dependabot.yml file.

@carogalvin
Copy link
Contributor

👋 Hello! Product Manager for Dependabot here. I’m currently doing research into adding/improving configuration for security updates, and am looking for user input. This issue is similar to things I’m thinking about, so if you’re subscribed to this and you’re open to a short conversation with me, please feel free to select a time in my calendar that fits your schedule here: https://calendar.app.google/7RSxjJJo9FdvRHNz7

@abdulapopoola
Copy link
Member

Closing out as stale

@abdulapopoola abdulapopoola closed this as not planned Won't fix, can't repro, duplicate, stale Mar 4, 2024
@kyeotic
Copy link
Author

kyeotic commented Mar 5, 2024

Closing out issues as stale encourages "me too" comments that keep the issue active.

Do you want "me too" comments @abdulapopoola ?

@ronjouch
Copy link

ronjouch commented Mar 5, 2024

Closing out issues as stale encourages "me too" comments that keep the issue active.

To this, adding that I did answer @carogalvin 's request for feedback above. We did meet, and when we did, Caro acknowledged that #2521 (comment) remained unaddressed.

Not throwing @carogalvin under the bus, as no promises were made. Still, this issue is clearly not "stale", it's simply unaddressed but should remain open as acknowledgement of the configurability gap expressed by many users/customers through this issue.

(And as kyeotic highlighted above, stale isn't a reason to close an issue, especially with many upvotes, this argument has been made a million times in a million repos)

@abdulapopoola
Copy link
Member

Thanks @ronjouch , @kyeotic and @ChiriVulpes for the feedback.

I'm sorry about that and I take full responsibility for this. As a first step, I'm re-opening the issue.

I closed it since the last seen engagement was a long time ago and the crew is actually making good progress in this area. The goal is to engage the community more.

Update:
The crew is starting to work in this area and we should have some simple configuration for grouped security updates coming out this week or next. This builds upon this release.

What would be a better way to carry you along going forward?

@abdulapopoola abdulapopoola reopened this Mar 5, 2024
@ls-valentinas-bakaitis
Copy link

While it might not completely address your issues, I've been using the recently introduced auto-triage rules (that can be set both on the Organisation and Repository level.

https://docs.github.com/en/code-security/dependabot/dependabot-auto-triage-rules/about-dependabot-auto-triage-rules

It allows you to automatically dismiss dependabot alerts that meet specified criteria (so you can ignore updates for a particular library, particular issue type, dev dependencies, etc).

@kyeotic
Copy link
Author

kyeotic commented Mar 5, 2024

What would be a better way to carry you along going forward?

Updates provided here in the issue would be ideal, since it already has people subscribed to it.

Is there an organizational blocker to this?

@abdulapopoola
Copy link
Member

abdulapopoola commented Mar 5, 2024

No blockers to that.

It's a bit challenging to keep the multiple issues about the same feature updated though. I'll see how it goes though. We're trying to engage the community more and create clarity about our in-progress work and plans.

One option being explored is pinning in-progress items to the top of issues; another potential approach will be to use roadmaps (which require some more work).

@kyeotic
Copy link
Author

kyeotic commented Mar 6, 2024

If there is another issue that should be canonical a reference to it while closing this should be sufficient. All subscribers and future visitors will see the new place.

@ronjouch
Copy link

ronjouch commented Mar 7, 2024

Thanks for the feedback. I'm sorry about that and I take full responsibility for this. As a first step, I'm re-opening the issue.

@abdulapopoola 👍, thanks for the ack and honest comeback 🙂.

I closed it since the last seen engagement was a long time ago and the crew is actually making good progress in this area. The goal is to engage the community more.

Update: The crew is starting to work in this area and we should have some simple configuration for grouped security updates coming out this week or next. This builds upon this release.

@abdulapopoola 👍, eager to see what you come up with. On my side, my problem was that #2521 (comment) simply didn't work (my config was exactly this comment, in several repos where vulns do happen). We've had it in several repos, it failed to produce any PR in a year, without visibility on what's wrong, and thus we abandoned it. Since then, we manage these manually with npm audit and without Dependabot.

But maybe this is fixed. I re-enabled my attempt in 2 repos, let’s see if this works now.

At any rate, if this is something you’d like, I’d be happy to have a new live chat with you or another Dependabot PM / tech, for "business analysis" clarity or support.

EDIT WHOA HEY, this seems to work now 🙂, I immedia-got a PR after enabling it in a repo. Cool, keeping it. Am still interested in the "We're planning to make this a lot nicer" that @feelepxyz was mentioning in #2521 (comment) . Thx for your work!

@ronjouch
Copy link

ronjouch commented Mar 7, 2024

@ronjouch yeah it's kinda possible to configure security updates through dependabot.yml, this is a bit of a hack atm but should work:

@feelepxyz (or anyone familiar): follow-up to #2521 (comment) : is the open-pull-requests-limit: 0 # in case you don't want to enable version updates and just want security updates hack deprecated now that we have applies-to: security-updates, or is it still needed?

Said differently, what's the best way since the latest release to enable security updates (configured by dependabot.yml, e.g. with specific assignees) without enabling version updates?

@carogalvin
Copy link
Contributor

@ronjouch it's still needed because applies-to only works in the context of groups.

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

No branches or pull requests

9 participants