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

Modify module exports mode to production in webpack.config.js #20431

Merged

Conversation

krvikash
Copy link
Contributor

@krvikash krvikash commented Jan 20, 2024

Description

The generated webapp Javascript files has eval() function. Executing JavaScript from a string is an enormous security risk. It is far too easy for a bad actor to run arbitrary code when you use eval().

Please see: never_use_eval!

With mode: "development" and no mention of devtool in webpack config, the devtool defaults to eval and generates the javascript files with eval() function.

Modifying mode from "development" to "production" generates the JavaScript files without eval() function.

This change requires to use webpack-cli instead webpack-command, as webpack-command is deprecated (https://github.com/webpack-contrib/webpack-command).

Additional context and related issues

This example demonstrates various types of source-maps: https://github.com/webpack/webpack/tree/main/examples/source-map

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.

@cla-bot cla-bot bot added the cla-signed label Jan 20, 2024
@github-actions github-actions bot added the ui Web UI label Jan 20, 2024
@wendigo
Copy link
Contributor

wendigo commented Jan 20, 2024

it's still using eval

@wendigo wendigo closed this Jan 20, 2024
@wendigo wendigo reopened this Jan 20, 2024
@wendigo
Copy link
Contributor

wendigo commented Jan 20, 2024

Can we switch mode to production ?

@krvikash krvikash force-pushed the krvikash/add-devtool-in-webpack.config.js branch from a484e42 to fd60812 Compare January 20, 2024 15:22
@krvikash
Copy link
Contributor Author

Thanks for the review @wendigo,

it's still using eval

I could not find eval() in the generated file. Could you point me to file?

Can we switch mode to production ?

After switching to production mode, I was getting below error. Same as mentioned here.

$ yarn --cwd core/trino-main/src/main/resources/webapp/src install
yarn install v1.22.21
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
warning " > [email protected]" has incorrect peer dependency "webpack@2 || 3 || 4".
warning " > [email protected]" has incorrect peer dependency "webpack@^4.4.0".
warning "webpack-command > @webpack-contrib/[email protected]" has incorrect peer dependency "webpack@^4.3.0".
warning "webpack-command > @webpack-contrib/[email protected]" has incorrect peer dependency "webpack@^3.0.0 || ^4.0.0".
[4/4] Building fresh packages...
$ webpack --config webpack.config.js
⬡ webpack: Starting Build
⬡ webpack: Build Finished
/home/ubuntu/starburst/github/trino/core/trino-main/src/main/resources/webapp/src/node_modules/webpack-command/lib/reporters/parse.js:80
      const row = [module.size, module.id.toString(), modulePath, status(module)];
                                          ^

TypeError: Cannot read property 'toString' of null
    at Object.modules (/home/ubuntu/starburst/github/trino/core/trino-main/src/main/resources/webapp/src/node_modules/webpack-command/lib/reporters/parse.js:80:43)
    at Object.files (/home/ubuntu/starburst/github/trino/core/trino-main/src/main/resources/webapp/src/node_modules/webpack-command/lib/reporters/parse.js:27:36)
    at StylishReporter.render (/home/ubuntu/starburst/github/trino/core/trino-main/src/main/resources/webapp/src/node_modules/webpack-command/lib/reporters/StylishReporter.js:91:39)
    at /home/ubuntu/starburst/github/trino/core/trino-main/src/main/resources/webapp/src/node_modules/webpack-command/lib/compiler.js:27:29
    at finalCallback (/home/ubuntu/starburst/github/trino/core/trino-main/src/main/resources/webapp/src/node_modules/webpack/lib/Compiler.js:441:32)
    at /home/ubuntu/starburst/github/trino/core/trino-main/src/main/resources/webapp/src/node_modules/webpack/lib/Compiler.js:505:17
    at /home/ubuntu/starburst/github/trino/core/trino-main/src/main/resources/webapp/src/node_modules/webpack/lib/HookWebpackError.js:68:3
    at Hook.eval [as callAsync] (eval at create (/home/ubuntu/starburst/github/trino/core/trino-main/src/main/resources/webapp/src/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:4:1)
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (/home/ubuntu/starburst/github/trino/core/trino-main/src/main/resources/webapp/src/node_modules/tapable/lib/Hook.js:18:14)
    at Cache.storeBuildDependencies (/home/ubuntu/starburst/github/trino/core/trino-main/src/main/resources/webapp/src/node_modules/webpack/lib/Cache.js:122:37)
error Command failed with exit code 1.

https://stackoverflow.com/questions/68626172/on-building-babel-loader-outputs-a-module-object-with-id-null-webpack-command suggests to use webpack-cli instead webpack-command because webpack-command is deprecated (https://github.com/webpack-contrib/webpack-command). After using webpack-cli, The scripts are getting generated without eval() function and with license text file.

@krvikash krvikash changed the title Add devtool property in webpack.config.js Modify module exports mode to production in webpack.config.js Jan 20, 2024
The generated webapp Javascript files has `eval()` function.
Executing JavaScript from a string is an enormous security
risk. It is far too easy for a bad actor to run arbitrary
code when you use `eval()`.

Please see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval#never_use_eval!

With `mode: "development"` and no mention of `devtool` in webpack config,
the `devtool` defaults to `eval` and generates the javascript files with
`eval()` function.

Modifying mode from `"development"` to `"production"` generates the
JavaScript files without `eval()` function.

This change requires to use `webpack-cli` instead `webpack-command`,
as `webpack-command` is deprecated (https://github.com/webpack-contrib/webpack-command).
@krvikash krvikash force-pushed the krvikash/add-devtool-in-webpack.config.js branch from fd60812 to 064de86 Compare January 20, 2024 16:17
@wendigo
Copy link
Contributor

wendigo commented Jan 20, 2024

LGTM. I've checked that PR locally. Seems to work

@wendigo wendigo merged commit 37eb965 into trinodb:master Jan 20, 2024
86 of 88 checks passed
@github-actions github-actions bot added this to the 437 milestone Jan 20, 2024
@krvikash krvikash deleted the krvikash/add-devtool-in-webpack.config.js branch January 21, 2024 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants