Skip to content
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

wp-now: Resolve npm audit warnings #224

Open
eliot-akira opened this issue Apr 9, 2024 · 0 comments
Open

wp-now: Resolve npm audit warnings #224

eliot-akira opened this issue Apr 9, 2024 · 0 comments
Labels
Bug Something isn't working

Comments

@eliot-akira
Copy link
Contributor

eliot-akira commented Apr 9, 2024

On installing @wp-now/wp-now, there's a warning:

4 moderate severity vulnerabilities

Running npm audit report shows:

express  <4.19.2
Severity: moderate
Express.js Open Redirect in malformed URLs - https://github.com/advisories/GHSA-rv95-896h-c2vc
No fix available
node_modules/express
  @php-wasm/node  >=0.1.18
  Depends on vulnerable versions of express
  node_modules/@php-wasm/node
  @wp-now/wp-now  *
  Depends on vulnerable versions of @php-wasm/node
  Depends on vulnerable versions of express
  Depends on vulnerable versions of follow-redirects
  node_modules/@wp-now/wp-now

follow-redirects  <=1.15.5
Severity: moderate
Follow Redirects improperly handles URLs in the url.parse() function - https://github.com/advisories/GHSA-jchw-25xp-jwwc
follow-redirects' Proxy-Authorization header kept across hosts - https://github.com/advisories/GHSA-cxjh-pqwp-8mfp
fix available via `npm audit fix`
node_modules/follow-redirects

Solution

While looking into this, I found that express is only used for tests in the entire Playground project.

./packages/php-wasm/node/src/test/php-networking.spec.ts:2:import express from 'express';

And there are unnecessary dependencies, express-fileupload and follow-redirects, which are only used for wp-now in the Playground Tools project.

./packages/wp-now/src/start-server.ts:4:import express from 'express';
./packages/wp-now/src/start-server.ts:7:import fileUpload from 'express-fileupload';

./packages/wp-now/src/download.ts:1:import followRedirects from 'follow-redirects';

So the solution is:

  • @php-wasm/node
    • Update express to newest 4.19.2
    • Move it to devDependencies
    • Remove unused express-fileupload and follow-redirects
  • @wp-now/wp-now
    • Add express to dependencies
    • Update follow-redirects to newest 1.15.6

I'll create two pull requests to address these.

@eliot-akira eliot-akira changed the title Resolve npm audit warnings wp-now: Resolve npm audit warnings Apr 9, 2024
@adamziel adamziel added the Bug Something isn't working label Apr 12, 2024
adamziel pushed a commit that referenced this issue Apr 12, 2024
#225)

This is the second part of solving:

- #224 

The first part is WordPress/wordpress-playground#1218. (php-wasm/node:
Update express to newest version, and move it to devDependencies)

## Why?

The first part of the solution is in the Playground project, which
updates `express` and moves it to `devDependencies` since it's only used
for tests in that project.

As a result, `express` will no longer be installed when `@php-wasm/node`
is installed. So `wp-now` needs to declare `express` as a dependency.

## How?

- Add `express` to `dependencies`
- Update `follow-redirects` to newest `1.15.6`

These together with WordPress/wordpress-playground#1218 will resolve the
original issue #224, which should eliminate all npm audit warnings.

## Testing Instructions

Currently `npm run test` fails with an unrelated issue, as can be seen
in another PR's CI test run.
https://github.com/WordPress/playground-tools/actions/runs/8616282863/job/23614052137?pr=223#step:4:49
("RuntimeError: memory access out of bounds") That seems have been
caused by commit
[133029c](133029c).
adamziel pushed a commit to WordPress/wordpress-playground that referenced this issue Apr 15, 2024
…pendencies (#1218)

This is the first part of solving WordPress/playground-tools#224
(wp-now: Resolve npm audit warnings).

- Update `express` to newest `4.19.2`
- Move it to `devDependencies`
- Remove unused `express-fileupload` and `follow-redirects`

## What problem is it solving?

It updates `express` to the newest version with security vulnerabilities
fixed.

## How is the problem addressed?

In addition to updating it, it's moved to `devDependencies` because
`express` is only used in tests.

```
./packages/php-wasm/node/src/test/php-networking.spec.ts:2:import express from 'express';
```

This consequently removes `express` from being installed when installing
`@php-wasm/node`.

The PR also removes unnecessary dependencies, `express-fileupload` and
`follow-redirects`, which are only used for `wp-now` in the Playground
Tools project.

## Testing Instructions

```sh
npm run test
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants