-
Notifications
You must be signed in to change notification settings - Fork 283
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(cmd-api-server): address CVE-2022-25881 #2899
fix(cmd-api-server): address CVE-2022-25881 #2899
Conversation
57f7751
to
1cd8a85
Compare
1c427ef
to
721dda5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zondervancalvez http-cache-semantics
is a transitive dependency. Making it (in addition to being a transitive dependency) a direct dependency of the cmd-api-server package would not have done anything to prevent the older version still being present in the sub-level node_modules dir (the transitive dependency).
With all that said, we've fixed this problem with the bulk CVE fix PR that was sent in a few months back where we added a forced resolution to the package.json in the root folder.
My guess is that the trivy scan was executed against an outdated version of the packages which still haven't had the forced resolution baked into it.
I'm pretty sure that this will be a duplicate based on the above but let's see with the latest release coming up soon and then you can confirm (or not) if it's a duplicate and close this one accordingly.
@zondervancalvez Are you still working on this? |
07367da
to
3d44bec
Compare
Hi @petermetz , I've just updated the PR and no more vulnerabilities were found. Thank you |
6a0099b
to
ffd3933
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zondervancalvez Alright, the fix with the hardcoded image version is good but the http-cache-semantics change is not doing anything to secure the container image build IMO. Let me quickly update that before we merge and then it's good to go from my side.
ffd3933
to
e8e85cb
Compare
Primary Changes: Updated the Dockerfile & https-cache-semantics inside the cmd-api-server package Fixes: hyperledger-cacti#2862 Signed-off-by: zondervancalvez <[email protected]> Signed-off-by: Peter Somogyvari <[email protected]>
e8e85cb
to
23d0bc5
Compare
Commit to be reviewed
fix(cmd-api-server): address GHSA-rc47-6667-2j5j
Primary Changes:
Updated the Dockerfile & https-cache-semantics inside the cmd-api-server package
Fixes: https://github.com/hyperledger/cacti/issues/2862
Signed-off-by: zondervancalvez [email protected]
Signed-off-by: Peter Somogyvari [email protected]
Pull Request Requirements
upstream/main
branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-s
flag when usinggit commit
command. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.