Skip to content
This repository has been archived by the owner on Mar 1, 2024. It is now read-only.

mardizzone/pos-944 Snyk integration #440

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open

Conversation

marcello33
Copy link

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)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist

  • I have added at least 2 reviewer or the whole pos-v1 team
  • I have added sufficient documentation in code
  • 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
  • I have added tests to CI
  • 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.

atvanguard and others added 30 commits April 13, 2020 17:36
Merge develop (May 6 - May 27) to master
Merge develop (May 25 - May 30) into master
- update hdwallet-provider
- set gas fee for bor
- enable deposit ERC20 in script/deposit.js
…ssues for 3rd party libraries and report in snyk policy file
@marcello33
Copy link
Author

marcello33 commented Nov 28, 2022

@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

@ZeroEkkusu
Copy link
Member

ZeroEkkusu commented Nov 29, 2022

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.

@marcello33
Copy link
Author

marcello33 commented Nov 29, 2022

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

@ZeroEkkusu
Copy link
Member

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

@marcello33
Copy link
Author

marcello33 commented Nov 29, 2022

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!

Copy link

@jonesk7734 jonesk7734 left a comment

Choose a reason for hiding this comment

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

😀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants