fix(security): address CVE-2019-5413 #1796
Conversation
charellesandig
requested review from
jonathan-m-hamilton and
petermetz
as code owners
There was a problem hiding this comment.
@charellesandig We do not use package-lock.json files since the migration to yarn from npm, it can (and need to) be deleted.
There was a problem hiding this comment.
@charellesandig Just a small nit, please lock down the version of the dependency to the exact one that you specified (without allowing a range) so that we have reproducible builds (or at least as close to it as we can get.
petermetz
requested review from
izuru0 and
takeutak
and removed request for
jonathan-m-hamilton
petermetz
added
dependencies
There was a problem hiding this comment.
@charellesandig The file you added in the diff that's called cactus: Is that needed for something specific or could it be removed as some leftover that's not meant to be there anymore?
charellesandig
force-pushed
the
issue1777
branch
from
b088681
to
8349e73
Compare
There was a problem hiding this comment.
Hi @petermetz, I deleted the 2 files you mentioned and just keep the child package.json and yarn.lock files. And is now ready for review again, thank you.
@charellesandig For the package.json file I meant that the diff in it is not needed. We do need the file itself in the state it is in on the main as of now. Sorry for the confusion! :-)
Deleting the cactus file was the right move, that's what I meant, thank you for removing that one.
charellesandig
force-pushed
the
issue1777
branch
from
8349e73
to
245111a
Compare
|
Hi @petermetz , I added back the package.json file, this time I removed the changes I made from the previous commits and now back to how it is. Thank you so much. |
There was a problem hiding this comment.
@charellesandig The diff is LGTM. Please resolve the conflicts, squash the commits into a single one and then we should be good to go!
charellesandig
force-pushed
the
issue1777
branch
from
245111a
to
6d3b174
Compare
There was a problem hiding this comment.
@charellesandig Sorry my bad, conflicts are still there, please fix those first because we cannot merge until then.
charellesandig
force-pushed
the
issue1777
branch
7 times, most recently
from
1877f95
to
c539810
Compare
|
@petermetz I am still working on this one. Thank you. |
charellesandig
force-pushed
the
issue1777
branch
6 times, most recently
from
3e3a39d
to
d565438
Compare
charellesandig
force-pushed
the
issue1777
branch
2 times, most recently
from
d21eb7b
to
7421fcb
Compare
There was a problem hiding this comment.
@charellesandig You need to update the yarn lock file because currently there are inconsistencies between what's in the package.json files and the lock file itself. (Did you manually edit the yarn.lock file by any chance?)
Most likely you can just do a yarn install and then commit the changes made by yarn itself to the yarn.lock file, but if the build still doesn't work after that just let me know and we can take a closer look together.
charellesandig
force-pushed
the
issue1777
branch
3 times, most recently
from
a71038b
to
5eb563c
Compare
charellesandig
force-pushed
the
issue1777
branch
from
33af76b
to
00a628f
Compare
charellesandig
force-pushed
the
issue1777
branch
from
23e69f1
to
836eddb
Compare
fixes: hyperledger-cacti#1777 Signed-off-by: charellesandig <charelle.wrk@gmail.com>
charellesandig
force-pushed
the
issue1777
branch
from
836eddb
to
34078ee
Compare
|
Hi @petermetz, I had an issue with my main branch and after fixing it I think it resulted for conflicts to occur in this ticket so I fixed and pushed the updates again. Thank you so much! |
@charellesandig My pleasure, thank you! |
fixes #1777