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

chore: Dependency Review Action #1974

Merged

Conversation

naveensrinivasan
Copy link
Contributor

Dependency review is a tool that helps you identify and fix vulnerabilities in your dependencies. By checking the dependency reviews in a pull request and changing any dependencies that are flagged as vulnerable, the project can avoid vulnerabilities being added to your project. https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/about-dependency-review#dependency-review-enforcement

Dependency review is a tool that helps you identify and fix vulnerabilities in your dependencies. By checking the dependency reviews in a pull request and changing any dependencies that are flagged as vulnerable, the project can avoid vulnerabilities being added to your project. https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/about-dependency-review#dependency-review-enforcement

Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com>
@jku
Copy link
Member

jku commented Apr 26, 2022

So

  • Dependency Review seems to be a brand new action from GitHub.
  • It warns about dependency changes that have vulnerabilities -- so the exact same thing Dependabot does, but on PRs instead of a cron job?
  • Dependency review includes details of changes to indirect dependencies -- (this should not affect us as we try to keep everything listed in requirements files). It seems to do this by using the dependency graph api however, so uses a different mechanism: we'd be using a Github service to get our dependency graph instead of just running software developed by github...

What it actually changes in the workflow is buried three links deep in the docs but the result seems to be this in the review UI:
image

@coveralls
Copy link

coveralls commented May 12, 2022

Pull Request Test Coverage Report for Build 2428073168

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 7 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.03%) to 98.357%

Files with Coverage Reduction New Missed Lines %
tuf/api/metadata.py 2 98.96%
tuf/ngclient/fetcher.py 2 94.12%
tuf/ngclient/updater.py 3 97.98%
Totals Coverage Status
Change from base Build 2201397029: 0.03%
Covered Lines: 1204
Relevant Lines: 1220

💛 - Coveralls

@lukpueh
Copy link
Member

lukpueh commented Jun 1, 2022

What it actually changes in the workflow is buried three links deep in the docs but the result seems to be this in the review UI:
...

The feature referenced here does not come with actions/dependency-review-action, but is already enabled by default for all public repos (see e.g. the rich diff for requirements-test.txt in a recent Dependabot PR).

The /=dependency-review-action basically makes any vulnerabilities shown in that diff more visible by displaying them in the checks box on the front page of a PR, with the option to block merge if the action fails.

I agree with @jku that it probably does not add much value since we already use Dependabot alerts, which are sent whenever:

  • A new vulnerability is added to the GitHub Advisory Database.
  • The dependency graph for a repository changes.

OTOH, there is probably not much harm in enabling the action (it is hosted by GitHub, only needs read permission), other than probably making builds take slightly longer and adding another config file that we need to maintain. Furthermore, the Dependabot alerts docs cited above further say:

Additionally [besides Dependabot alerts], GitHub can review any dependencies added, updated, or removed [...] This allows you to spot and deal with vulnerable dependencies before, rather than after, they reach your codebase.

I have no strong preference about this, but lean a bit towards merging and see how it goes.

Copy link
Member

@jku jku left a comment

Choose a reason for hiding this comment

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

I have no strong preference about this, but lean a bit towards merging and see how it goes.

Works for me, let's try it out. I have one change request, will leave comment about that.

.github/workflows/dependency-review.yml Outdated Show resolved Hide resolved
Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com>
@naveensrinivasan naveensrinivasan requested a review from jku June 2, 2022 12:02
- name: 'Dependency Review'
uses: actions/dependency-review-action@v1
Copy link
Member

Choose a reason for hiding this comment

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

wow: I think your changes are correct and I'll approve but... look at where "v1" actually points to: the initial commit event though there are several 1.0.x releases after that! Further proof that actions versioning is really not well thought out.

Copy link
Member

Choose a reason for hiding this comment

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

they fixed this now... apparently the project had 1600 users and no-one had noticed they were all running the initial commit

@jku jku merged commit cfcc0c3 into theupdateframework:develop Jun 6, 2022
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.

None yet

4 participants