security: webpack-dev-server 5.2.4 resolution (#1237, #691, #692)#21420
security: webpack-dev-server 5.2.4 resolution (#1237, #691, #692)#21420charlesBochet wants to merge 1 commit into
Conversation
| "yeoman-environment": "6.0.1" | ||
| "yeoman-environment": "6.0.1", | ||
| "webpack-dev-server": "5.2.4" | ||
| }, |
There was a problem hiding this comment.
Bug: Forcing webpack-dev-server to v5 breaks @electron-forge/plugin-webpack, which uses the v4 constructor API. The swapped arguments will cause a crash when starting the dev server.
Severity: HIGH
Suggested Fix
The webpack-dev-server resolution should be removed. The incompatibility between @electron-forge/plugin-webpack and webpack-dev-server v5 is a fundamental breaking change. The dependency should not be forced to v5 until @electron-forge/plugin-webpack is updated to support it.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: package.json#L48
Potential issue: The PR forces the use of `webpack-dev-server` v5.2.4, but a dependency,
`@electron-forge/plugin-webpack`, is only compatible with v4. The constructor signature
for `WebpackDevServer` changed between v4 and v5, swapping the `compiler` and `options`
arguments. The plugin calls the constructor using the v4 argument order (`new
WebpackDevServer(options, compiler)`), which will be misinterpreted by the v5 library.
This will cause a runtime crash when attempting to start the development server via
`yarn start:electron`, as the server will receive a configuration object where it
expects a compiler instance.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Thanks — but this is a false positive. The constructor signature did not change between v4 and v5.
webpack-dev-server@5.2.4 defines constructor(options, compiler) (see node_modules/webpack-dev-server/lib/Server.js:331 and types/lib/Server.d.ts:1179), which is exactly what @electron-forge/plugin-webpack calls: new WebpackDevServer(this.devServerOptions(), compiler). The (compiler, options) → (options, compiler) swap happened at v3 → v4, not v4 → v5, so v4 and v5 are call-compatible here.
The plugin also only passes options that are unchanged in v5 (hot, devMiddleware.writeToDisk, historyApiFallback, port, setupExitSignals, static, headers) and uses none of the hooks v5 removed. The resolution is now scoped to @electron-forge/plugin-webpack/webpack-dev-server to limit blast radius.
There was a problem hiding this comment.
Pull request overview
This PR addresses dependency security alerts by forcing webpack-dev-server to resolve to 5.2.4 across the repository (primarily impacting @electron-forge/plugin-webpack used by twenty-companion tooling).
Changes:
- Add a root Yarn resolution pinning
webpack-dev-serverto5.2.4. - Update
yarn.lockto reflect the newwebpack-dev-server@5.2.4tree and its updated transitive dependencies.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Adds a root-level resolutions entry for webpack-dev-server@5.2.4. |
| yarn.lock | Updates the lockfile to remove webpack-dev-server@4.15.2 and record webpack-dev-server@5.2.4 plus updated transitive dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@angular-devkit/core": "19.2.24", | ||
| "yeoman-environment": "6.0.1" | ||
| "yeoman-environment": "6.0.1", | ||
| "webpack-dev-server": "5.2.4" |
| "@angular-devkit/core": "19.2.24", | ||
| "yeoman-environment": "6.0.1" | ||
| "yeoman-environment": "6.0.1", | ||
| "webpack-dev-server": "5.2.4" |
…v-server 5.2.4 (#1237, #691, #692) webpack-dev-server is pulled only by @electron-forge/plugin-webpack (twenty-companion), and no electron-forge release — including 8.0.0-alpha — uses webpack-dev-server 5, so there is no parent-upgrade path. A resolution is the only mechanism. Scoped to @electron-forge/plugin-webpack/webpack-dev-server (not a global override) to limit blast radius. Verified it's safe: webpack-dev-server's constructor is `constructor(options, compiler)` in BOTH v4 and v5 (the argument swap was v3->v4, not v4->v5), matching plugin-webpack's `new WebpackDevServer(this.devServerOptions(), compiler)` call. The options it passes (hot, devMiddleware.writeToDisk, historyApiFallback, port, setupExitSignals, static, headers) are all unchanged in v5, and it uses none of the hooks v5 removed. webpack-dev-server is only exercised by `electron-forge start` (dev HMR); production make/package builds don't use it, and twenty-companion has no CI workflow.
ca53abf to
01be837
Compare
Visual Regression Report
|
Visual Regression Report (twenty-new-ui)✅ No visual changes detected across 7 stories. |
🔍 Automated Pre-Review✅ No issues detected - This PR is ready for human review. Automated pre-review — human approval still required. |
| "@angular-devkit/core": "19.2.24", | ||
| "yeoman-environment": "6.0.1" | ||
| "yeoman-environment": "6.0.1", | ||
| "@electron-forge/plugin-webpack/webpack-dev-server": "5.2.4" |
There was a problem hiding this comment.
Bug: The resolution for webpack-dev-server uses incorrect syntax for Yarn Berry (v4). The intended security fix will not be applied, and the vulnerable package version will still be used.
Severity: CRITICAL
Suggested Fix
Update the resolutions field in package.json to use the correct syntax for Yarn Berry. Instead of the nested path, use a direct package name to version mapping: "resolutions": { "webpack-dev-server": "5.2.4" }. This will apply the resolution globally and correctly upgrade the package.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: package.json#L47
Potential issue: The resolution for `webpack-dev-server` in `package.json` uses Yarn v1
"selective resolution" syntax (`parent/package-name`). However, the project is
configured to use Yarn Berry (v4), which does not support this syntax. As a result, the
resolution is ignored, and the intended upgrade of `webpack-dev-server` to version
`5.2.4` does not occur. The `yarn.lock` file confirms this, as it lacks any entry for
the new version. This means the security vulnerabilities in the older version of
`webpack-dev-server` remain unfixed, and the vulnerable code will be used when running
the application.
Closes the webpack-dev-server alerts (#1237, #691, #692) with a scoped resolution:
@electron-forge/plugin-webpack/webpack-dev-server: 5.2.4.Why a resolution (the one place it's unavoidable)
webpack-dev-server is pulled only by
@electron-forge/plugin-webpack(twenty-companion's build tooling), and no electron-forge release uses webpack-dev-server 5 — not even8.0.0-alpha.9still pins^4. There is no parent-upgrade path, so a resolution is the only mechanism.Scoped, not global
Per review feedback, the resolution targets
@electron-forge/plugin-webpack/webpack-dev-serverrather than a globalwebpack-dev-serveroverride — it only forces v5 under electron-forge (the sole consumer), limiting blast radius and future-upgrade friction.Why it's safe (constructor did NOT change v4→v5)
@electron-forge/plugin-webpack@7callsnew WebpackDevServer(this.devServerOptions(), compiler). webpack-dev-server's constructor isconstructor(options, compiler)in both v4 and v5 — verified in the installed source (lib/Server.js:331,types/lib/Server.d.ts:1179). The(compiler, options)→(options, compiler)swap happened at v3 → v4, not v4 → v5. The plugin also passes only options unchanged in v5 (hot,devMiddleware.writeToDisk,historyApiFallback,port,setupExitSignals,static,headers) and uses none of the hooks v5 removed. (This addresses sentry's bug-prediction comment, which was a false positive.)Scope / verification
yarn install --immutable✓; webpack-dev-server now resolves to 5.2.4 only (was 4.15.2), no vulnerable copy remains.electron-forge start(dev HMR); productionmake/packagebuilds don't use it, and twenty-companion has no CI workflow, so this can't affect CI.yarn start:electronin twenty-companion still boots the dev server.