Skip to content

fix(cli): resolve vm2 security vulnerability#5070

Merged
sean-perkins merged 6 commits intodevelopfrom
sp/vm2-security
Dec 19, 2023
Merged

fix(cli): resolve vm2 security vulnerability#5070
sean-perkins merged 6 commits intodevelopfrom
sp/vm2-security

Conversation

@sean-perkins
Copy link
Copy Markdown
Contributor

@sean-perkins sean-perkins commented Dec 14, 2023 •
edited
Loading

Ionic CLI currently reports a security vulnerability with the vm2 dependency as a result of superagent-proxy → proxy-agent.

The superagent-proxy package is a smaller wrapper around superagent and proxy-agent that is not maintained.

This PR removes the dependency for superagent-proxy and upgrades to proxy-agent@6 which no longer has the vm2 dependency requirement. The proxy-agent package uses ESM & as a result a lot of other usages had to be migrated once esmoduleInterop was enabled. The proxy behavior that superagent-proxy was responsible for has been added here: https://github.com/ionic-team/ionic-cli/blob/4332a24af67b98424ff19083a6f9e2e09a66c0a8/packages/%40ionic/cli/src/lib/utils/superagent-proxy.ts

Here is the error message without esmoduleInterop:

Details 
node_modules/get-uri/dist/http.d.ts:5:8 - error TS1192: Module '"http"' has no default export.

5 import http_ from 'http';

~~~~~


node_modules/get-uri/dist/http.d.ts:6:8 - error TS1192: Module '"https"' has no default export.


6 import https from 'https';

~~~~~


node_modules/proxy-agent/dist/index.d.ts:3:8 - error TS1259: Module '"/Users/sean/Documents/ionic/ionic-cli/packages/@ionic/cli/node_modules/proxy-agent/node_modules/lru-cache/index"' can only be default-imported using the 'esModuleInterop' flag


3 import LRUCache from 'lru-cache';

~~~~~~~~


node_modules/proxy-agent/node_modules/lru-cache/index.d.ts:869:1

869 export = LRUCache

~~~~~~~~~~~~~~~~~

This module is declared with 'export =', and can only be used with a default import when using the 'esModuleInterop' flag.


Found 3 errors in 2 files.


Errors  Files

2  node_modules/get-uri/dist/http.d.ts:5

1  node_modules/proxy-agent/dist/index.d.ts:3

@sean-perkins sean-perkins marked this pull request as ready for review December 14, 2023 20:45
@sean-perkins sean-perkins requested review from a team and brandyscarney and removed request for a team December 14, 2023 20:45
thetaPC
thetaPC requested changes Dec 14, 2023
View reviewed changes
@sean-perkins sean-perkins requested a review from thetaPC December 18, 2023 17:44
thetaPC
thetaPC requested changes Dec 18, 2023
View reviewed changes
@sean-perkins sean-perkins requested a review from thetaPC December 18, 2023 18:39
thetaPC
thetaPC approved these changes Dec 18, 2023
View reviewed changes
Copy link
Copy Markdown

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sean-perkins sean-perkins merged commit 4050419 into develop Dec 19, 2023
@sean-perkins sean-perkins deleted the sp/vm2-security branch December 19, 2023 16:31
@sean-perkins sean-perkins mentioned this pull request Jan 15, 2024
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thetaPC thetaPC thetaPC approved these changes

@brandyscarney brandyscarney Awaiting requested review from brandyscarney brandyscarney was automatically assigned from ionic-team/framework

+1 more reviewer

@ltm ltm ltm approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sean-perkins @ltm @thetaPC