fix: mv2 firefox csp header by itsyoboieltr · Pull Request #27770 · MetaMask/metamask-extension

itsyoboieltr · 2024-10-10T17:29:25Z

Description

This PR implements a workaround for a long-standing Firefox MV2 bug where the content-security-policy header is not bypassed, triggering an error.

The solution is simple: we check if the extension is MV2 running in Firefox. If yes, we override the header to prevent the error from raising.

Related issues

Fixes: #3133, https://github.com/MetaMask/MetaMask-planning/issues/3342

Manual testing steps

Opening github.com should not trigger the CSP error

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

github-actions · 2024-10-10T17:29:37Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

app/scripts/background.js

sonarqubecloud · 2024-10-15T18:50:37Z

Quality Gate failed

Failed conditions
1 Security Hotspot
18.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

app/scripts/background.js

shared/modules/add-nonce-to-csp.ts

shared/modules/content-security-policy.ts

app/scripts/background.js

app/_locales/en/messages.json

app/scripts/controllers/preferences-controller.ts

ui/pages/settings/advanced-tab/advanced-tab.component.js

app/scripts/background.js

danjm

This looks good to me overall. Has anyone manually tested with a prod-like build?

metamaskbot · 2024-11-06T22:24:03Z

Builds ready [cec02cb]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2095 ± 90 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	309	2475	2030	432	207
		domContentLoaded	1754	2464	2068	177	85
		load	1763	2477	2095	188	90
		domInteractive	22	90	45	19	9
		backgroundConnect	9	153	28	33	16
		firstReactRender	53	289	127	64	31
		getState	5	66	19	18	8
		initialActions	0	1	0	0	0
		loadScripts	1230	1777	1510	139	67
		setupStore	14	85	44	24	11
		uiStartup	2036	2862	2422	242	116

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 3 KiB (0.07%)
ui: 2.03 KiB (0.03%)
common: 604 Bytes (0.01%)

itsyoboieltr · 2024-11-06T22:35:01Z

@danjm I manually tested it today with a prod-like build using the command:

yarn webpack --env production --no-lavamoat --browser firefox

it worked for me locally.

DDDDDanica · 2024-11-07T11:28:09Z

Hey @itsyoboieltr I used the build from bot above and tested locally the zip in firefox. I noticed that in firefox page we won't receive any errors, while in extension log we still have it, is this expected?

itsyoboieltr · 2024-11-07T14:57:29Z

Hi @DDDDDanica, thank you for checking out and testing the PR! The error logs in the extension are unrelated to the issue. This PR is about fixing the CSP error messages for websites (not the extension itself). The screenshot you sent seems to be showing a pre-existing error. I could reproduce the same error messages in the console by running the current build from develop.

DDDDDanica · 2024-11-07T15:47:13Z

@itsyoboieltr thanks for the explanation, just to make sure it is not related, approve now !

fix: mv2 firefox csp header

f8014e2

itsyoboieltr requested a review from a team as a code owner October 10, 2024 17:29

github-actions bot added the team-extension-platform Extension Platform team label Oct 10, 2024

itsyoboieltr self-assigned this Oct 10, 2024

itsyoboieltr commented Oct 10, 2024

View reviewed changes

app/scripts/background.js Outdated Show resolved Hide resolved

itsyoboieltr requested review from davidmurdoch and weizman October 10, 2024 17:55

itsyoboieltr added 2 commits October 11, 2024 20:39

Merge branch 'develop' into fix-firefox-csp

51e7033

feat: nonce

c6bd1bd

itsyoboieltr requested a review from kumavis as a code owner October 14, 2024 20:45

itsyoboieltr added 5 commits October 15, 2024 18:29

fix: nonce

1905247

fix: random url

cd551fd

Merge branch 'develop' into fix-firefox-csp

a35b17e

fix: webpack tests

d1a84c2

fix: regex

885c320

davidmurdoch reviewed Oct 15, 2024

View reviewed changes

app/scripts/background.js Outdated Show resolved Hide resolved

fix: regex whitespace

ba383e4

davidmurdoch reviewed Oct 15, 2024

View reviewed changes

app/scripts/background.js Outdated Show resolved Hide resolved

feat: addNonceToCsp with tests

c048d41

itsyoboieltr requested a review from davidmurdoch October 15, 2024 18:47

davidmurdoch reviewed Oct 15, 2024

View reviewed changes

app/scripts/background.js Outdated Show resolved Hide resolved

davidmurdoch reviewed Oct 15, 2024

View reviewed changes

shared/modules/add-nonce-to-csp.ts Outdated Show resolved Hide resolved

itsyoboieltr added 4 commits October 17, 2024 18:48

Merge branch 'develop' into fix-firefox-csp

dc0d053

fix: limit types

95b3ad8

fix: getPlatform

ab2896d

feat: CSP

9a7cd35

davidmurdoch reviewed Oct 17, 2024

View reviewed changes

shared/modules/content-security-policy.ts Outdated Show resolved Hide resolved

davidmurdoch reviewed Oct 17, 2024

View reviewed changes

app/scripts/background.js Outdated Show resolved Hide resolved

itsyoboieltr dismissed stale reviews from DDDDDanica and davidmurdoch via 75e16be November 4, 2024 20:11

itsyoboieltr added 2 commits November 5, 2024 00:19

Update background.js

b482cac

Update settings-search.test.js

289dc50

davidmurdoch reviewed Nov 5, 2024

View reviewed changes

app/_locales/en/messages.json Outdated Show resolved Hide resolved

app/scripts/controllers/preferences-controller.ts Show resolved Hide resolved

ui/pages/settings/advanced-tab/advanced-tab.component.js Outdated Show resolved Hide resolved

fix: description

5163ec9

danjm reviewed Nov 5, 2024

View reviewed changes

app/scripts/background.js Show resolved Hide resolved

itsyoboieltr added 3 commits November 6, 2024 01:05

feat: only show on firefox

ab1f3df

Update settings-search.test.js

33508e7

Update settings-search.test.js

0fb3c3f

danjm reviewed Nov 5, 2024

View reviewed changes

itsyoboieltr added 5 commits November 6, 2024 20:12

Merge branch 'develop' into fix-firefox-csp

f50e960

Merge branch 'develop' into fix-firefox-csp

07d10be

fix

3bf6b9b

fixture

6e9ed23

Update fixture-builder.js

cec02cb

DDDDDanica approved these changes Nov 7, 2024

View reviewed changes

danjm approved these changes Nov 7, 2024

View reviewed changes

davidmurdoch approved these changes Nov 7, 2024

View reviewed changes

itsyoboieltr added this pull request to the merge queue Nov 7, 2024

Merged via the queue into develop with commit 54c563e Nov 7, 2024

itsyoboieltr deleted the fix-firefox-csp branch November 7, 2024 18:41

github-actions bot locked and limited conversation to collaborators Nov 7, 2024

metamaskbot added the release-12.8.0 Issue or pull request that will be included in release 12.8.0 label Nov 7, 2024

Conversation

itsyoboieltr commented Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Oct 10, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Oct 15, 2024

Quality Gate failed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danjm left a comment

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Nov 6, 2024

Uh oh!

itsyoboieltr commented Nov 6, 2024

Uh oh!

DDDDDanica commented Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

itsyoboieltr commented Nov 7, 2024

Uh oh!

DDDDDanica commented Nov 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

itsyoboieltr commented Oct 10, 2024 •

edited

Loading

DDDDDanica commented Nov 7, 2024 •

edited

Loading