Investigate Dependabot high and critical severity alerts in mobile-samples

[tcobbs-bentley]

The vulnerabilities in the following npm packages come from @bentley/react-scripts. I updated that from what we were using (4.0.5) to the latest (4.0.7), but they were still vulnerable:

• ansi-html
• browserslist
• ejs
• glob-parent
• immer
• node-forge
• nth-check
• postcss
• scss-tokenizer
• shell-quote

Regarding the scss-tokenizer vulnerability. This comes from node-sass@7.0.1. Since node-sass is totally deprecated, with instructions to use Dart sass instead (sass npm module), @bentley/react-scripts should really be updated to stop using node-sass.

A glob-parent vulnerability also comes from webpack@4.44.2.

[calebmshafer]

@tcobbs-bentley the latest version of @bentley/react-scripts does stop using node-sass if you remove it as a dependency of the app. It will only use it if you directly install it and if not will use dart sass instead.

Also, @aruniverse has been working on releasing react-scripts@5 which will fix many of the issues. That full release should be out this week so maybe try updating to that and see if there are any issues.

[tcobbs-bentley]

@calebmshafer when you say "latest version of @bentley/react-scripts, do you mean 5.0? Because I don't have node-sass in my list of dependencies, and yet I still get it from @bentley/react-scripts when using 4.0.7:

travis@NAOU49143 react-app % npm ls node-sass                        
react-app@0.10.31 /Users/travis/Dev/itwin/mobile-samples/cross-platform/react-app
└─┬ @bentley/react-scripts@4.0.7
  └─┬ sass-loader@10.3.1
    └── node-sass@7.0.1

[aruniverse]

@tcobbs-bentley , please try 5.0.0

[tcobbs-bentley]

@aruniverse Doing that and nothing else (still on iTwin 3.2.x) causes a build error:

travis@NAOU49143 react-app % npm run build

> react-app@0.10.31 build
> npm run build:frontend && npm run build:backend

> react-app@0.10.31 build:frontend
> cross-env NODE_OPTIONS=--max_old_space_size=8192 TRANSPILE_DEPS=false DISABLE_TERSER=true USE_FAST_SASS=true react-scripts build && npm run copy:assets

Creating an optimized production build...
(node:85053) [DEP_WEBPACK_COMPILATION_NORMAL_MODULE_LOADER_HOOK] DeprecationWarning: Compilation.hooks.normalModuleLoader was moved to NormalModule.getCompilationHooks(compilation).loader
(Use `node --trace-deprecation ...` to show where the warning was created)
Failed to compile.

Module not found: Error: Can't resolve './Arrow' in '/Users/travis/Dev/itwin/mobile-samples/cross-platform/react-app/node_modules/apache-arrow'
Did you mean 'Arrow.mjs'?
BREAKING CHANGE: The request './Arrow' failed to resolve only because it was resolved as fully specified
(probably because the origin is strict EcmaScript Module, e. g. a module with javascript mimetype, a '*.mjs' file, or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.

[aruniverse]

What is the status of this issue?

[tcobbs-bentley]

@aruniverse The last time I checked, React Scripts 5 was effectively unusable for mobile-samples. I had to remove a ton of eslint rule disabling comments due to those rules not working due to the plugins that produce the rules being incompatible with something deep down inside react-scripts 5. While I was able to get things to build, these now completely missing eslint rules were extremely useful ones that I don't feel it is appropriate to downgrade our software to live without.

Furthermore, there is a super scary sounding warning at build time (and react-scripts start time) due to no-longer-included node packages, and I was led to believe that react-scripts 5 made getting rid of this (I reiterate, super scary) warning impossible.

This all happened a while ago, and I haven't had the time to check again to see if there are resolutions.

[aruniverse]

Furthermore, there is a super scary sounding warning at build time (and react-scripts start time) due to no-longer-included node packages, and I was led to believe that react-scripts 5 made getting rid of this (I reiterate, super scary) warning impossible.

This is still present, and won't be completely removed til 3.6. Idk if its "super scary", especially since its just a warning and not a runtime error since its not in the critical path.

Re the eslint rules, cant comment on that. Version of eslint used by itwin/eslint-plugins and bentley/react-scripts differ and we will not be changing react-scripts to support an older version of eslint there

Sounds like this will stay open

[tcobbs-bentley]

As of today (2022-04-17), the only dependabot alerts are for xml2js, which is pulled in from iTwin. iTwin is in the process of resolving that problem.