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

Update react-scripts to address immer alert (SCP-3893) #1273

Merged
merged 1 commit into from Nov 23, 2021

Conversation

eweitz
Copy link
Member

@eweitz eweitz commented Nov 22, 2021

This fixes a false-positive critical security vulnerability in immer. That was a dependency of an old version of react-scripts, which we use in development -- and not production. Nevertheless, the dependency can't easily be moved from dependencies to devDependencies. Details: facebook/create-react-app#10411.

I drafted a more comprehensive set of dependency updates, but those caused deeply cryptic problems. This is a minimal change that fixes the most-reportedly-severe error.

To test:

  • Pull branch
  • yarn install --force
  • Restart rails s, verify no errors upon startup
  • In another terminal, restart bin/webpack-dev-server, verify no errors upon startup
  • In local SCP, navigate to a study that has visualizations
  • Save a change to a JavaScript file, e.g. open ScatterPlot.js, add console.log('ok') atop RawScatterPlot
  • In webpack-dev-server terminal, verify your change triggered a recompilation
  • In local SCP, open DevTools console, verify "ok" was printed to console
  • Revert change to ScatterPlot.js

This relates to SCP-3893.

@codecov
Copy link

codecov bot commented Nov 22, 2021

Codecov Report

Merging #1273 (4e840f3) into development (8829e83) will increase coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #1273      +/-   ##
===============================================
+ Coverage        68.90%   69.01%   +0.11%     
===============================================
  Files              241      242       +1     
  Lines            18816    18905      +89     
  Branches           952      954       +2     
===============================================
+ Hits             12965    13048      +83     
- Misses            5682     5688       +6     
  Partials           169      169              
Impacted Files Coverage Δ
app/uploaders/branding_group_image_uploader.rb 58.82% <0.00%> (-35.30%) ⬇️
app/models/genome_assembly.rb 90.47% <0.00%> (-9.53%) ⬇️
app/lib/synthetic_branding_group_populator.rb 92.15% <0.00%> (-3.68%) ⬇️
app/lib/user_asset_service.rb 94.59% <0.00%> (-2.71%) ⬇️
...search/controls/download/DownloadSelectionTable.js 91.11% <0.00%> (-1.84%) ⬇️
app/models/study.rb 82.97% <0.00%> (-0.38%) ⬇️
app/models/search_facet.rb 97.99% <0.00%> (-0.34%) ⬇️
app/lib/azul_search_service.rb 94.17% <0.00%> (-0.12%) ⬇️
app/models/download_request.rb 100.00% <0.00%> (ø)
app/controllers/studies_controller.rb 21.78% <0.00%> (ø)
... and 11 more

Copy link
Contributor

@ehanna4 ehanna4 left a comment

Choose a reason for hiding this comment

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

Tested following the outlined steps and got the results expected.

@eweitz
Copy link
Member Author

eweitz commented Nov 23, 2021

The SCP team investigated the build failure, and determined it was a false positive with some newly-introduced log noise.

@eweitz eweitz merged commit 5558d97 into development Nov 23, 2021
@eweitz eweitz deleted the ew-secure-immer branch November 23, 2021 17:06
@eweitz
Copy link
Member Author

eweitz commented Nov 23, 2021

After merge, Dependabot decreased reported vulnerabilities from 11-12 to 9, but retained its basic alert about immer.

The previous alert had a clause that suggested updating to immer 8.0.1 (via updating react-scripts) would suffice, but the new message clarifies that’s not so.

As noted above, though, this vulnerability is a false positive. It's used only in development, and not production, and so there are no known vulnerabilities in our use case.

@eweitz eweitz changed the title Update react-scripts to fix alert about immer (SCP-3893) Update react-scripts to address immer alert (SCP-3893) Nov 23, 2021
@eweitz eweitz mentioned this pull request Nov 23, 2021
@eweitz eweitz added the build failure: false positive Build error confirmed as false positive. E.g. upstream service has a problem. label Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build failure: false positive Build error confirmed as false positive. E.g. upstream service has a problem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants