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.

Sign up for GitHub

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

Jump to bottom

feat: add osv-vulnerabilities and vulnerability service #14567

Merged
merged 3 commits into from
Apr 12, 2022
Merged

feat: add osv-vulnerabilities and vulnerability service #14567

merged 3 commits into from
Apr 12, 2022

Conversation

JamieMagee
Copy link
Contributor

Changes

Adds a Vulnerabilities class which uses @jamiemagee/osv-offline to fetch OSV1 databases and parse them offline. This is the first part of #6562, and still needs much work

Context

Initial part of #6562

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

Footnotes

  1. https://osv.dev/ ↩

viceice
viceice reviewed Mar 8, 2022
View reviewed changes
lib/workers/repository/process/vulnerabilities.ts Outdated Show resolved Hide resolved
lib/workers/repository/process/vulnerabilities.ts Outdated Show resolved Hide resolved
lib/workers/repository/process/vulnerabilities.ts Outdated Show resolved Hide resolved
lib/workers/repository/process/vulnerabilities.ts Outdated Show resolved Hide resolved
astellingwerf
astellingwerf reviewed Mar 9, 2022
View reviewed changes
Copy link
Collaborator

@astellingwerf astellingwerf left a comment

Choose a reason for hiding this comment

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

You're deliberately not (yet) adding invocation to this service?

lib/workers/repository/process/vulnerabilities.ts Outdated Show resolved Hide resolved
lib/workers/repository/process/vulnerabilities.ts Show resolved Hide resolved
@JamieMagee
Copy link
Contributor Author

You're deliberately not (yet) adding invocation to this service?

Yep! I need to do some more testing and nail down a rollout plan. I think it will likely be something like:

  1. Undocumented config flag to enable the feature
  2. Documented config flag to enable the feature
  3. Enabled by default for all platforms except GitHub (Renovate already provides security advisories via GitHub's API)
  4. Enabled by default for all platforms

@JamieMagee JamieMagee force-pushed the feat/osv branch 3 times, most recently from 564501d to acda78b Compare March 16, 2022 03:52
@JamieMagee
Copy link
Contributor Author

@viceice I'm seeing null check errors for files I haven't edited. Any ideas why?

@viceice
Copy link
Member

viceice commented Mar 16, 2022

@viceice I'm seeing null check errors for files I haven't edited. Any ideas why?

You need to exclude lib/workers/repository/process/vulnerabilities.spec.ts from strict checks, the transitive imports are causing the issue. 😉

@JamieMagee
Copy link
Contributor Author

All tests are already excluded?

renovate/tsconfig.strict.json

Line 19 in ab8e1c6

"**/*.spec.ts",

@viceice
Copy link
Member

viceice commented Mar 21, 2022

All tests are already excluded?

renovate/tsconfig.strict.json

Line 19 in ab8e1c6

"**/*.spec.ts",

Currently yes

@JamieMagee JamieMagee marked this pull request as ready for review March 25, 2022 03:52
@JamieMagee JamieMagee force-pushed the feat/osv branch 2 times, most recently from eb21192 to c27b6ae Compare March 29, 2022 03:21
@JamieMagee
Copy link
Contributor Author

@viceice Can you review this please? Thanks!

viceice
viceice reviewed Apr 3, 2022
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

lib/workers/repository/process/vulnerabilities.ts Show resolved Hide resolved
lib/workers/repository/process/vulnerabilities.ts Show resolved Hide resolved
lib/workers/repository/process/vulnerabilities.ts Show resolved Hide resolved
@JamieMagee JamieMagee force-pushed the feat/osv branch 2 times, most recently from 02725be to 97324be Compare April 11, 2022 01:27
@JamieMagee JamieMagee enabled auto-merge (squash) April 11, 2022 01:28
@JamieMagee JamieMagee removed the request for review from astellingwerf April 12, 2022 03:21
@JamieMagee
Copy link
Contributor Author

@viceice Can I get an approval please 🙏

viceice
viceice approved these changes Apr 12, 2022
View reviewed changes
tsconfig.strict.json Show resolved Hide resolved
@JamieMagee JamieMagee merged commit 48cb88e into main Apr 12, 2022
@JamieMagee JamieMagee deleted the feat/osv branch April 12, 2022 16:13
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 32.20.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@astellingwerf astellingwerf astellingwerf left review comments

@viceice viceice viceice approved these changes

@rarkins rarkins rarkins left review comments

@HonkingGoose HonkingGoose HonkingGoose left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@JamieMagee @viceice @renovate-release @astellingwerf @rarkins @HonkingGoose