Skip to content

Add dependency review PR check#4769

Merged
quis merged 1 commit intomainfrom
add-dependency-review-pr-check
Mar 10, 2026
Merged

Add dependency review PR check#4769
quis merged 1 commit intomainfrom
add-dependency-review-pr-check

Conversation

@quis
Copy link
Member

@quis quis commented Mar 9, 2026

We have a tool called Pyup which is supposed to check for PRs which introduce known vulnerabilities.

I think we should move away from it for 2 reasons:

  1. It doesn’t seem to be working
  2. We use Dependabot for other dependency-related automations, so consolidating on one tool seems better

The ‘it doesn’t seem to be working’ bit is probably something to do with this:

image

As an example, we introduced an older version of wheel here: https://github.com/alphagov/notifications-api/pull/4764/changes#diff-4d7c51b1efe9043e44439a949dfd92e5827321b34082903477fd04876edb7552R264-R265

This version had a vulnerability, but we were not warned about it. Pyup did not catch it.

It’s only after merging to main that Dependabot showed the vulnerability in the ‘security’ tab. Even then, it was unable to raise a PR to fix the dependency (see https://github.com/alphagov/notifications-api/security/dependabot/99)

So all in all, it would have been much better if the original PR hadn’t been merged (at least without resolving the issue first).


This PR sets up Dependabot to check PRs for any new or updated dependencies.

It copies the basic config from https://docs.github.com/en/code-security/how-tos/secure-your-supply-chain/manage-your-dependency-security/configuring-the-dependency-review-action

We could set it up to do more sophisticated stuff in the future, like block merging of PRs which have vulnerabilities, but let’s just see what it does for now.


You can see the check has run against this PR below, or directly at https://github.com/alphagov/notifications-api/actions/runs/22849613073/job/66274601963?pr=4769

We have a tool called Pyup which is supposed to check our PRs for known
vulnerabilities.

I think we should move away from it for 2 reasons:
1. It doesn’t seem to be working
2. We use dependabot for other dependency-related automations, so
   consolidating on one tool seems better

***

As an example, we introduced an older version of `wheel` here:
https://github.com/alphagov/notifications-api/pull/4764/changes#diff-4d7c51b1efe9043e44439a949dfd92e5827321b34082903477fd04876edb7552R264-R265

This version had a vulnerability, but we were not warned about it. Pyup
did not catch it.

It’s only after merging to `main` that Dependabot showed the vulnerability
in the ‘security’ tab.

Furthermore, it was unable to raise a PR to fix the dependency
https://github.com/alphagov/notifications-api/security/dependabot/99

So all in all, it would have been much better if the original PR hadn’t
been merged (at least without resolving the issue).

***

This copies the basic config from https://docs.github.com/en/code-security/how-tos/secure-your-supply-chain/manage-your-dependency-security/configuring-the-dependency-review-action

We could set it up to do more sophisticated stuff in the future, like
fail PRs which have vulnerabilities, but let’s just see what it does for
now.
@quis quis merged commit 66453ae into main Mar 10, 2026
9 checks passed
@quis quis deleted the add-dependency-review-pr-check branch March 10, 2026 15:31
quis added a commit to alphagov/notifications-template-preview that referenced this pull request Mar 10, 2026
quis added a commit to alphagov/notifications-antivirus that referenced this pull request Mar 10, 2026
quis added a commit to alphagov/notifications-template-preview that referenced this pull request Mar 10, 2026
quis added a commit to alphagov/notifications-template-preview that referenced this pull request Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants