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

feat: lavamoat #712

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

feat: lavamoat #712

wants to merge 29 commits into from

Conversation

andreabadesso
Copy link
Contributor

@andreabadesso andreabadesso commented Dec 23, 2024

Motivation

Add LavaMoat to protect against supply chain attacks and upgrade react-scripts to version 5.

Description

Added LavaMoat webpack plugin to production builds only. This required upgrading react-scripts from v4 to v5, which in turn required several changes:

  1. Added react-app-rewired to customize the webpack config (and add the lavamoat plugin)

  2. Added polyfills that were removed in react-scripts v5:

    • buffer
    • crypto-browserify
    • https-browserify
    • os-browserify
    • path-browserify
    • stream-browserify
    • stream-http
    • vm-browserify
  3. Created buffer-shim.js to handle Buffer compatibility issues in the elliptic package

  4. Updated imports in our code to use browserify versions (e.g. path -> path-browserify)

Acceptance Criteria

  • LavaMoat runs only in production builds as @lavamoat/webpack is missing a hook

  • Development builds work without LavaMoat

  • All functionality remains unchanged with lavamoat and react-scripts v5 activated

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@andreabadesso andreabadesso force-pushed the feat/lavamoat branch 2 times, most recently from c39da8d to 2ada178 Compare December 23, 2024 14:07
@@ -0,0 +1,1383 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is generated by lavamoat automatically (because we have generatePolicy: true in our plugin config). All manual changes are done in the policy-override.json

@andreabadesso andreabadesso self-assigned this Dec 23, 2024
@andreabadesso andreabadesso added the enhancement New feature or request label Dec 23, 2024
@andreabadesso andreabadesso changed the title feat: LavaMoat feat: lavamoat Dec 23, 2024
config-overrides.js Show resolved Hide resolved
lavamoat/webpack/policy-override.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
patches/elliptic++bn.js+4.12.0.patch Show resolved Hide resolved
src/components/ModalPin.js Outdated Show resolved Hide resolved
src/components/ModalPin.js Outdated Show resolved Hide resolved
src/components/ModalPin.js Outdated Show resolved Hide resolved
src/store/index.js Show resolved Hide resolved
src/utils/helpers.js Outdated Show resolved Hide resolved
}
});

config.module.rules.push({
Copy link
Member

Choose a reason for hiding this comment

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

Add comment for this rule also

"jquery": {
"globals": {
"document": true,
"window": true,
Copy link
Member

Choose a reason for hiding this comment

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

When you allow window, does it allow all window.*?

"document.title": true
}
},
"unleash-proxy-client": {
Copy link
Member

Choose a reason for hiding this comment

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

This is our package, right?

"console": true,
"Error": true,
"Object": true,
"XMLHttpRequest": true,
Copy link
Member

Choose a reason for hiding this comment

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

We send requests only through axios. Does it make sense to remove this and allow only in axios?

Copy link
Member

@pedroferreira1 pedroferreira1 left a comment

Choose a reason for hiding this comment

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

Waiting dependencies review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: In Review (WIP)
Development

Successfully merging this pull request may close these issues.

3 participants