LedgerHQ / ledger-live

Mono-repository for packages related to Ledger Live and its JavaScript ecosystem.
MIT License
414 stars 316 forks source link

This project should be auditing it's code automatically for vulnerabilities #409

Closed kdenhartog closed 1 year ago

kdenhartog commented 2 years ago

Context

CI Pipeline / monorepo base package

Is your feature request related to a problem? Please describe.

Given many of the applications and code in this monorepo is strongly related to the security of cryptoassets this repo should be automatically failing CI checks if vulnerable packages are in use or automatically updating vulnerable code.

Describe the solution you'd like

Use dependabot or something like it to automatically open PRs to fix vulnerable packages.

Describe alternatives you've considered

  1. This can be done with pnpm audit --fix to leverage the overrides capability of pnpm. However there's two downsides I've noticed by taking this approach. First off, this makes a modification to the package.json at the base of the repo which probably isn't ideal for long term maintainability. Second, this tool opts to relist the same package if there's multiple security advisories listed for the same package rather than setting the newest package version as the minimum. This ends up leading to issues where these things need to be manually updated instead.

  2. Alternatively this could be used as an alert to a developer on the team with only pnpm audit so that it can be manually updated when a CI pipeline fails running. This is probably the least favorable solution long term, but should work decently well for now.

Additional context

Add any other context or screenshots about the feature request here.

elbywan commented 2 years ago

Hey @kdenhartog,

Use dependabot or something like it to automatically open PRs to fix vulnerable packages.

Thanks for the feedback, this is a valid concern!

We are currently investigating the best way to achieve that, but before putting any automated tool in place we will need to audit and update our thousands of dependencies which is a long process.

kdenhartog commented 2 years ago

Is there anything I could help with for this? Brave is interested in sticking with directly using this repository for our wallet to support Ledger HW wallets rather than forking but we are concerned about the risk of supply chain attacks that we become susceptible to if this takes awhile to complete.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment, or consider closing it.

kdenhartog commented 2 years ago

bump to keep this ticket open as we still would like to see this completed.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment, or consider closing it.

kdenhartog commented 2 years ago

bump to keep this ticket open as we still would like to see this completed.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment, or consider closing it.

punithbm commented 1 year ago

Same here when running the yarn audit getting the following vulnerabilities in the package dependencies.

Screenshot 2022-11-03 at 11 50 17 PM
github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment, or consider closing it.

kdenhartog commented 1 year ago

can't remove label, so bumping via a comment again.

elbywan commented 1 year ago

Hey @kdenhartog, sorry for the lack of response here but I just wanted to let you know that we have not forgotten this issue and that it is definitely on our roadmap 😉.

kdenhartog commented 1 year ago

That's great to hear! I know these things can take time so thanks for the update that this is still in the works.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment, or consider closing it.

kdenhartog commented 1 year ago

bump so the bot doesn't close it. I know the team still intends to get to this when they get the time.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment, or consider closing it.

kdenhartog commented 1 year ago

bump

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment, or consider closing it.

wholroyd commented 1 year ago

bump

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment, or consider closing it.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment, or consider closing it.

kdenhartog commented 1 year ago

Is there any progress on moving this one up on the backlog at this point?

valpinkman commented 1 year ago

Yes we are looking into it in the next few weeks. Sorry for the long waiting time. Bandwidth was not on our side but it has been prioritized.

valpinkman commented 1 year ago

Since the issue is about the setup, i'm closing it as we have setup renovate bot to help us with our dependencies update (this plus the security feature in Github allows us to tackle the important upgrade first, but this is a long term process)

Thanks again for the reports.

kdenhartog commented 1 year ago

Awesome, that's great to hear! Thanks for setting that up which should be fine to make this work and make it easier for us to manage the transient dependencies we pick up from this.