-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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(cli): patch security vulnerability in https-proxy-agent (npm advisory 1184) #4603
Conversation
Package `https-proxy-agent` has a security vulnerability which has required a major version bump to address: https://hackerone.com/reports/541502 We depend on this package via 2 other packages (`proxy-agent` and `pac-proxy-agent`), both of which have had a GitHub PR applied to bump their dependencies on the vulnerable package, but both of which have not released that change. Vendor in both packages with updated dependencies to eliminate the dependency on the vulnerable version of `https-proxy-agent`. Internal reference: t/P29683837
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
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.
I believe this way can work in the mono-repo, but I suspect it'll fail once the packages are consumed from npm pack
output (e.g. when consuming from the registry). I believe npm install
will attempt to resolve the dependency closure before it goes to install anything, which in this case will fail (because the nested vendor
directory won't be there yet.
My recommendation would be to:
- Vendor-in tarballs for the fixed packages (instead of their source - it'll be leaner)
- Make sure the
vendor
directory is.npmignore
d
- Make sure the
- Refer to the tarballs from the
dependencies
section - Add the fixed up packages to
bundledDependencies
, so they are not resolved from registry but simply inherited from the CLI's tarball itself
Obviously, strongly recommend running integration tests on the resulting packages, to make sure they're installable (and have the correct (aka fixed) versions of those two libraries.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@RomainMuller wrote:
Why wouldn’t the vendor directory be there? We want to include it in our tarball. |
Okay @RomainMuller seems to be right (a lot):
|
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
8f2017c
to
96d6624
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request is now being automatically merged. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Package
https-proxy-agent
has a security vulnerability which hasrequired a major version bump to address: https://hackerone.com/reports/541502, https://www.npmjs.com/advisories/1184
We depend on this package via 2 other packages (
proxy-agent
andpac-proxy-agent
), both of which have had a GitHub PR applied to bumptheir dependencies on the vulnerable package, but both of which have
not released that change.
Vendor in both packages with updated dependencies to eliminate the
dependency on the vulnerable version of
https-proxy-agent
.Internal reference: t/P29683837
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license