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

(security): update webpack-dev-server to address cves #1473

Merged
merged 6 commits into from
Dec 12, 2024

Conversation

d-buckner
Copy link
Member

@d-buckner d-buckner commented Dec 9, 2024

Description

This addresses 11 CVEs associated with webpack-dev-server v3.

Resolved CVE Vulnerable Library
CVE-2024-37890 ws-6.2.2
CVE-2024-43796 express-4.19.2
CVE-2024-45590 body-parser-1.20.2
CVE-2024-21536 http-proxy-middleware-0.19.1
CVE-2024-47764 cookie-0.6.0
CVE-2024-29415 ip-1.1.9
CVE-2024-29180 webpack-dev-middleware-3.7.2
CVE-2024-52798 path-to-regexp-0.1.7
CVE-2024-45296 path-to-regexp-0.1.7
CVE-2024-43800 serve-static-1.15.0
CVE-2024-43799 send-0.18.0

Issues Resolved

fixes #389

Verification

I was able to diff the dist artifacts with and without these changes. The only change across all of the generated artifacts is in the charts_theme module.

/******/ 	// Load entry module and return exports
- /******/ 	return __webpack_require__(__webpack_require__.s = "./themes/charts/themes.ts");
+ /******/ 	return __webpack_require__(__webpack_require__.s = 0);
/******/ })

I've validated that the imported chart theme object is identical as is the type definition file.

I've also manually validated the docs pages.

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • All tests pass
    • yarn lint
    • yarn test-unit
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@d-buckner d-buckner marked this pull request as ready for review December 10, 2024 18:02
Copy link
Collaborator

@virajsanghvi virajsanghvi left a comment

Choose a reason for hiding this comment

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

How have we tested the impact of this change? Can we validate it doesn't have impact on generated artifacts?

CHANGELOG.md Outdated
@@ -40,6 +40,7 @@

### 🛡 Security
- Update cross-spawn to address CVE ([#1469](https://github.com/opensearch-project/oui/pull/1469))
- Update webpack and webpack-dev-server to address CVEs ([#1473](https://github.com/opensearch-project/oui/pull/1473))
Copy link
Collaborator

Choose a reason for hiding this comment

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

We'd want to to move the existing unreleased items to 1.18 and then add this to the unreleased section

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense, I'll update this

@d-buckner
Copy link
Member Author

How have we tested the impact of this change? Can we validate it doesn't have impact on generated artifacts?

I've done cursory testing with the integration tests and by running the dev server. I'm planning on also doing the following

  1. update webpack-cli since I realize it's mismatched with webpack itself. One breaking change I know I'll have to address is the deprecation of the --output-library-target override we use to compile the charts to commonjs.
  2. diff the dist folder with a build with and without this change.

@virajsanghvi Let me know if this doesn't properly address your comment.

@d-buckner
Copy link
Member Author

I've updated with the webpack-cli update and I've added the verification process in the PR description.

Comment on lines +84 to +85
--env filename=${outputFilename} \
--env library-target=commonjs`,
Copy link
Member Author

Choose a reason for hiding this comment

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

These are no longer directly configurable from the cli after the update. I've updated these to get passed into the webpack config.

Copy link
Collaborator

@AMoo-Miki AMoo-Miki Dec 11, 2024

Choose a reason for hiding this comment

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

nit: Anything you see around /* OUI -> EUI Aliases */ was semi-automatically added when we were adding aliases and are meant to be semi-automatically removable. Pulling this into a util function, while the originals code blocks will be removed with the next major release, the util will be left over.

PS: noticed that it would still be used by the OUI version after the next major version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Gotcha. The util is also used for the non-aliased build.

@d-buckner d-buckner changed the title (security): update webpack & webpack-dev-server (security): update webpack-dev-server to address cves Dec 12, 2024
@virajsanghvi virajsanghvi merged commit b2005bd into opensearch-project:main Dec 12, 2024
16 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Dec 12, 2024
* (security): update webpack & webpack-dev-server

Signed-off-by: Daniel Rowe <[email protected]>

* update changelog, fix types

Signed-off-by: Daniel Rowe <[email protected]>

* revert webpack-cli upgrade

Signed-off-by: Daniel Rowe <[email protected]>

* address breaking changes with webpack-cli upgrade

Signed-off-by: Daniel Rowe <[email protected]>

* update changelog

Signed-off-by: Daniel Rowe <[email protected]>

* revert change to main webpack release

Signed-off-by: Daniel Rowe <[email protected]>

---------

Signed-off-by: Daniel Rowe <[email protected]>
(cherry picked from commit b2005bd)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
virajsanghvi pushed a commit that referenced this pull request Jan 9, 2025
* (security): update webpack & webpack-dev-server

Signed-off-by: Daniel Rowe <[email protected]>

* update changelog, fix types

Signed-off-by: Daniel Rowe <[email protected]>

* revert webpack-cli upgrade

Signed-off-by: Daniel Rowe <[email protected]>

* address breaking changes with webpack-cli upgrade

Signed-off-by: Daniel Rowe <[email protected]>

* update changelog

Signed-off-by: Daniel Rowe <[email protected]>

* revert change to main webpack release

Signed-off-by: Daniel Rowe <[email protected]>

---------

Signed-off-by: Daniel Rowe <[email protected]>
(cherry picked from commit b2005bd)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

webpack-dev-server-3.11.3.tgz: 11 vulnerabilities (highest severity is: 9.1)
3 participants