Handling https-proxy-agent GitHub security issue.

[revanth0212]

Description

This fixes the https-proxy-agent security issue.

https-proxy-agent is a deep dependency of 3 of our dependencies:

1. Danger
2. Graphql CLI
3. Lerna

A GitHub security issue has been raised on https-proxy-agent@2.2.3 and lower versions.

Lerna is being used in the monorepo-introduction.js script. @zetlen made changes to the files so we don't need lerna any more. Hence deprecating it.

Also danger uses https-proxy-agent@2.2.1 which is vulnerable as well. It will be moved out of pwa-studio into the CICD repo where it belongs in (PWA-543)[https://jira.corp.magento.com/browse/PWA-543].

Coming to graphql-cli I have tried upgrading it to the latest version but, the deep dependency of https-proxy-agent is still 2.2.1. Not sure how to fix that.

Related Issue

Closes PWA-531

Verification Stakeholders

@zetlen

Verification Steps

The root package.json has a "prepare" script defined which runs scripts/monorepo-introduction.js. NPM runs the prepare script after every local yarn install. The purpose of scripts/monorepo-introduction.js is to set up the development environment correctly for brand new checkouts, and to verify that all packages are prepared to build. To do so, it runs the prepare script in all of the sub-packages that have one defined. It was using Lerna to do this, but now it doesn't need to.

To verify that scripts/monorepo-introduction.js is still working correctly:

1. Check out the branch and run yarn install.

2. Observe that after the install completes, it logs:

   Preparing packages...
   Ensuring valid environment...
   

   This is the expected output of scripts/monorepo-introduction.js.

3. Run rm packages/graphql-cli-validate-magento-pwa-queries/lib/magento-compatibility.js.

4. Run mv packages/venia-concept/.env packages/venia-concept/.env.temp

5. Run yarn install again. Observe that both of the removed files have been regenerated.

6. Run mv packages/venia-concept/.env.temp packages/venia-concept/.env to restore your old .env file.

To verify that scripts/monorepo-introduction.js will run any newly defined prepare step in a sub-package:

1. Edit packages/venia-concept/package.json.
2. Add "prepare": "[[ 1 == 2 ]]", to the scripts section. (That's designed to fail on purpose.)
3. Run yarn install. Observe that it fails and displays an error.
4. Remove the prepare step from packages/venia-concept/package.json.

Checklist

TODO

[PWAStudioBot]

	Warnings
⚠️	Found the word "TODO" in the PR description. Just letting you know incase you forgot :)

	Messages
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖	Associated JIRA tickets: PWA-543.

Generated by 🚫 dangerJS against 11a4bb7

[zetlen]

It works!

[sirugh]

Coming to graphql-cli I have tried upgrading it to the latest version but, the deep dependency of https-proxy-agent is still 2.2.1. Not sure how to fix that.

If graphql-cli has not fixed their dependency on the vulnerable package and we use graphql-cli through yarn we can utilize the "resolutions" in our root package.json. Be careful though because this forcefully requires all yarn dependencies to use that version if the package is listed as a dependency at any po9int in the tree. This means it is possible to introduce a breaking change to a package that was not upgraded, thus potentially breaking our use.

If you end up using resolutions make sure to test out all places we use graphql-cli.

[zetlen]

graphql-cli was unmaintained by Prisma for a long time, but it's been transferred to a new owner and there have been a lot of updates. We should schedule a story to consider whether we want to commit more to graphql-cli, or maybe switch our query checker to use the Buildpack CLI that now exists.

[revanth0212]

If graphql-cli has not fixed their dependency on the vulnerable package and we use graphql-cli through yarn we can utilize the "resolutions" in our root package.json.

Nice. I wasn't aware of this, thanks.

If you end up using resolutions make sure to test out all places we use graphql-cli.

I have checked this, everything looks fine. Can you run through this yourself once to make sure I haven't missed anything?

[zetlen]

We only use graphql-cli in one place, the validate-queries script in Venia. Confirmed that it still works. (But phew, it's slow and thread-locky. This seems to be ESLint's fault.)

I figure @sirugh just wants to know we tested, which we did.

[zetlen]

LGTM, thank you @revanth0212 and @supernova-at for writing and maintaining as well. I think we should consider deprecating this, however, and replacing it with something that understands which files are actually being included in the bundle.

In my tests I noticed that this looks through the filesystem, and not the dependency chain. This means that it's gonna read both .ee.js and .ce.js files. As these proliferate, that will probably mean that any CE-based projects won't validate properly, because the validator is seeing .ee.js files that aren't being included.

I recommend something like this instead:

• An extension! @magento/pwa-studio-validate-queries or something
  • It adds a command to buildpack, like buildpack validate-queries
  • Under the hood it runs a full build (with --mode development, for speed)
  • It adds the ESLint Webpack Plugin to the compiler, and configures it to use the ESLint GraphQL plugin we're currently using
• It needs some extension points opened up!
  • A Target for adding Buildpack commands
  • A Target for running a customized Webpack build with mode and plugin settings
• We'll have to decide how we download and maintain the schema. Should we continue using the GraphQLConfig ecosystem, or is it more trouble than it's worth?

@awilcoxa May I request that we backlog this issue? It would make the query validator script useful again. It would also reduce dependency bloat to modernize this feature, which would probably stabilize our project.

[dpatil-magento]

@revanth0212 Can you pls check the unit test failure (pwa-pr-test) packages/graphql-cli-validate-magento-pwa-queries/lib/tests/index.spec.js

[zetlen]

LGTM, thanks for sleuthing that test result, @revanth0212

[revanth0212]

Performance Test Results

The following fails have been reported by WebpageTest. These numbers indicates a possible performance issue with the PR which requires further manual testing to validate.

https://pr-2356.pwa-venia.com : WPT TTFB Expected 300 Actual 685.33333333333

This might be a false positive. We haven't changed anything related to UI. This is more of ES Lint and build stuff.

[dpatil-magento]

QA Pass.

[devops-pwa-codebuild]

Performance Test Results

The following fails have been reported by WebpageTest. These numbers indicates a possible performance issue with the PR which requires further manual testing to validate.

https://pr-2356.pwa-venia.com/venia-tops.html : LH Performance Expected 0.75 Actual 0.74