Skip to content

Do not auto-approve dependabot PRs, and add cooldown to dependabot - #1564

Open
ludfjig wants to merge 1 commit into
hyperlight-dev:mainfrom
ludfjig:no_auto_approves
Open

Do not auto-approve dependabot PRs, and add cooldown to dependabot#1564
ludfjig wants to merge 1 commit into
hyperlight-dev:mainfrom
ludfjig:no_auto_approves

Conversation

@ludfjig

@ludfjig ludfjig commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Automatically merging PRs is not a good idea security-wise, especially given the many supply chain attacks these days. Having a human review the diff of an updated crate is always good to make sure it's not been compromised. This PR removes auto-approving PRs by our bot and also adds a cooldown to dependabot which is good practice in general.

I also considered just adding a dependabot cooldown, and also only auto-merging patch/minor bumps, but ultimately settle on this which I think is the better approach.

Note: The bot is still used in dep_update_guest_locks.yml

Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 19, 2026 16:51
@ludfjig ludfjig added area/security Involves security-related changes or fixes kind/dependencies For PRs that update dependencies or related components labels Jun 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the repository automation that auto-approved/auto-merged Dependabot PRs (to require human review) and adds a Dependabot “cooldown” period to reduce update churn while still allowing Dependabot to operate normally.

Changes:

  • Deleted the scheduled GitHub Actions workflow that auto-approved/auto-merged Dependabot PRs.
  • Removed the dev/auto-approve-dependabot.sh script used by that workflow.
  • Added a 7-day Dependabot cooldown across configured ecosystems/directories.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
dev/auto-approve-dependabot.sh Removed the Dependabot auto-approval/auto-merge script to require human review.
.github/workflows/auto-merge-dependabot.yml Removed the scheduled workflow that used the script to auto-approve/auto-merge Dependabot PRs.
.github/workflows/dep_update_guest_locks.yml Removed a comment referencing the deleted auto-merge workflow (no functional change).
.github/dependabot.yml Added cooldown: default-days: 7 to reduce update noise and avoid immediate adoption of freshly-released versions.

@jsturtevant jsturtevant left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems this is a wise choice in the current environment, but it will make a little bit more work for maintainers. I would leave it for others to look at too and give some comments or approve

@simongdavies

Copy link
Copy Markdown
Member

I am in 2 minds about this, generally agree with the cooldown changes (although we should probably think about if we want different settings for major/minor/patch).

I don't know about having manual review, besides the overhead, what is the expectation on a reviewer? How likely is it that one of us will find a supply chain attack in a dependency that has presumably evaded the maintainers of the dependency? If a dependency has been compromised then I guess it will be yanked (and dependabot will behave appropriately?

@syntactically

Copy link
Copy Markdown
Member

I think this is generally a good direction to move in, although a bit of extra work. If we do want to actually vet crates and keep track of changes in case of malicious maintainers or attacks on maintainers, we might want to use something like Mozilla's cargo vet as well.

If the idea is partly to avoid attacks on our own infrastructure, rather than on downstream users of hyperlight (since they control their own Cargo.locks anyway, and we can't do anything about semver-compatible minor version bumps of our dependencies auto-updating there), do we also need to avoid running CI on PRs (both dependant and manual) that include dependency or lock file upgrades, at least if they have not already appeared in trusted cargo-vet feeds or something like that?

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

Labels

area/security Involves security-related changes or fixes kind/dependencies For PRs that update dependencies or related components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants