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

mardizzone/pos-944 Snyk integration #440

wants to merge 36 commits into from

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
@atvanguard
Merge pull request #223 from maticnetwork/develop
265df12 
Merge Develop to Master
@atvanguard
Merge pull request #270 from maticnetwork/develop
e55b2d6 
Merge develop (May 6 - May 27) to master
@atvanguard
Merge pull request #275 from maticnetwork/develop
2659701 
Merge develop (May 25 - May 30) into master
@temaniarpit27
remove docker for compiling
a1bdb02
@temaniarpit27
revert changes
1553570
@cffls
Improvement for integration test
7f1148d 
- update hdwallet-provider
- set gas fee for bor
- enable deposit ERC20 in script/deposit.js
@gatsbyz
hello
e596c4b
@gatsbyz
gana
1ffd184
@gatsbyz
hooo
854ac98
@gatsbyz
fix
6dc208d
@gatsbyz
ci use node 16
b385f98
@gatsbyz
testrpc script upgrade
8cb8d72
@gatsbyz
wait for several block confirmations between each deployment
5383918
@gatsbyz
fix parameters
ba3b59d
@gatsbyz
change
06f10a7
@gatsbyz
go
c43f8ab
@gatsbyz
test
49e7b55
@gatsbyz
safemath fix
233f2d2
@gatsbyz
fix
a1da472
@gatsbyz
add
50db5e7
@marcello33
dev: add: pos-944 security-ci, readme and sol related files
fd020cf
@marcello33
dev: add: merge node upgrade branch / add \idea in gitignore
8e95fa4
@marcello33
dev: add: implement security-ci for snyk
eb61673
@marcello33
Merge branch 'mardizzone/node-upgrade' into mardizzone/pos-944
5f7dfe4
@marcello33
dev: add: pos-944 ignore licenses vulnerabilities
f6978bd
@marcello33
dev: add: pos-944 upgrade dependensies
228ab78
@marcello33
dev: fix: pos-944 remove duplicate
4146fb1
@marcello33
dev: add: pos-944 solve SAST vulnerabilities / add snyk cache file to…
6cf8644 
… gitignore
@marcello33
dev: fix: pos-944 solve syntax issues with reasons and use comment
ce5d012
@marcello33
dev: fix: pos-944 upgrade some deps to solve vulnerabilities / open i…
ecfcf6d 
…ssues for 3rd party libraries and report in snyk policy file
@ZeroEkkusu ZeroEkkusu mentioned this pull request Nov 23, 2022
Open
13 tasks
@marcello33
Copy link
Author

marcello33 commented Nov 28, 2022 •
edited

@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 •
edited

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 •
edited

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 •
edited

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!

jonesk7734
jonesk7734 approved these changes Sep 7, 2023
View reviewed changes
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.
Reviewers

@jonesk7734 jonesk7734 jonesk7734 approved these changes

At least 1 approving review is required to merge this pull request.

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
7 participants
@marcello33 @ZeroEkkusu @jonesk7734 @atvanguard @temaniarpit27 @cffls @gatsbyz