contracts icon indicating copy to clipboard operation
contracts copied to clipboard

mardizzone/pos-944 Snyk integration

Open marcello33 opened this issue 3 years ago • 5 comments

Description

This PR integrates snyk security CI into bor GH pipeline. It executes snyk vulnerabilities check and snyk static code analysis and publish the results into the relative action. Licenses check has been removed from snyk UI as ours is a open source organization.

Changes

  • [ ] Bugfix (non-breaking change that solves an issue)
  • [ ] Hotfix (change that solves an urgent issue, and requires immediate attention)
  • [x] New feature (non-breaking change that adds functionality)
  • [ ] Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist

  • [x] I have added at least 2 reviewer or the whole pos-v1 team
  • [x] I have added sufficient documentation in code
  • [x] I will be resolving comments - if any - by pushing each fix in a separate commit and linking the commit hash in the comment reply

Testing

  • [ ] I have added unit tests
  • [x] I have added tests to CI
  • [x] I have tested this code manually on local environment
  • [ ] I have tested this code manually on remote devnet using express-cli
  • [ ] I have tested this code manually on mumbai
  • [ ] I have created new e2e tests into express-cli

Manual tests

Used snyk CLI for tests, and embedded security-ci on PR.

marcello33 avatar Nov 10 '22 13:11 marcello33

@ZeroEkkusu just committed one more change to solve a vulnerability issue. Please notify me when you are done here.

I see only CI/build is failing

marcello33 avatar Nov 28 '22 13:11 marcello33

Confirmed that the build was failing because of a babel dependency. The build error is now because of a wrong nonce (as expected), but sometimes it's an out-of-gas error.

ZeroEkkusu avatar Nov 29 '22 13:11 ZeroEkkusu

Confirmed that the build was failing because of a babel dependency. The build error is now because of a wrong nonce (as expected), but sometimes it's an out-of-gas error.

@ZeroEkkusu so, shall we leave it as it is for the moment, and we can get back to it once you have all the knowledge about it and feel confident to solve (based on our call planned for tomorrow) ? Thanks

marcello33 avatar Nov 29 '22 14:11 marcello33

@marcello33 Sure. Feel free to revert it if you want.

ZeroEkkusu avatar Nov 29 '22 14:11 ZeroEkkusu

No problems @ZeroEkkusu. Our prio is to get the whole topic solved around node upgrade (and thus make it work with all devnets deployed via matic-cli). I'll leave this PR pending until then. Thanks!

marcello33 avatar Nov 29 '22 14:11 marcello33