metamask-extension icon indicating copy to clipboard operation
metamask-extension copied to clipboard

Published 20 hours ago • verified publisher icon MetaMask

Restore frame-ancestors: none

Open brad-decker opened this issue 1 year ago • 5 comments

Description

In #21765 I updated the CSP to allow frame-ancestors: self. I did this so that html documents hosted in the extension could be loaded in an iframe in the offscreen document. However this has unintended consequences that @weizman brought to my attention as he reviewed the whole of this work for MV3.

Frame-ancestors: none still allows for externally hosted pages to be added as iframes but disallows pages hosted internally to be loaded in iframes. So loading the ledger keyring iframe from the ledger library is still doable, as is loading all of the snaps iframes. The major change required was not to encapsulate each of the hardware wallets listeners in its own iframe. Considering that we are still just using the chrome runtime api to listen for updates in those iframes there was functionally no requirement to have each of these isolated. This new solution maintains all functionality and restores the frame-ancestors: none CSP.

Related issues

Fixes:

Manual testing steps

  1. run yarn start:mv3
  2. Go to the test dapp
  3. Use the various methods with the hardware wallets at your disposal.

Screenshots/Recordings

Before

After

Pre-merge author checklist

  • [x] I’ve followed MetaMask Coding Standards.
  • [x] I've clearly explained what problem this PR is solving and how it is solved.
  • [x] I've linked related issues
  • [x] I've included manual testing steps
  • [x] I've included screenshots/recordings if applicable
  • [x] I’ve included tests if applicable
  • [x] 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.
  • [x] I’ve properly set the pull request status:
    • [ ] In case it's not yet "ready for review", I've set it to "draft".
    • [x] In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

brad-decker avatar Feb 13 '24 14:02 brad-decker

Builds ready [6251b57]
Page Load Metrics (720 ± 12 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint761149284
domContentLoaded9221432
load6907667202412
domInteractive9221432
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 13 '24 15:02 metamaskbot

Builds ready [b6156e2]
Page Load Metrics (1045 ± 41 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1302221882512
domContentLoaded116927199
load970127610458541
domInteractive116927199
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 13 '24 18:02 metamaskbot

Builds ready [e22c965]
Page Load Metrics (1118 ± 59 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1213042024220
domContentLoaded12112332713
load8971337111812259
domInteractive12112332713
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 13 '24 18:02 metamaskbot

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (030786a) 68.47% compared to head (042d3c5) 68.47%. Report is 3 commits behind head on develop.

Additional details and impacted files 
@@           Coverage Diff            @@
##           develop   #22914   +/-   ##
========================================
  Coverage    68.47%   68.47%           
========================================
  Files         1089     1089           
  Lines        43026    43026           
  Branches     11459    11459           
========================================
  Hits         29459    29459           
  Misses       13567    13567

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 13 '24 18:02 codecov[bot]

Builds ready [836a946]
Page Load Metrics (1066 ± 45 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1192861804924
domContentLoaded12192574220
load934125910669545
domInteractive12192574220
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 13 '24 22:02 metamaskbot

Builds ready [f4805ad]
Page Load Metrics (990 ± 26 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1332721913215
domContentLoaded983362512
load84810969905426
domInteractive983362512
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 19 '24 21:02 metamaskbot

Builds ready [042d3c5]
Page Load Metrics (1038 ± 70 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1035092069043
domContentLoaded9135383215
load7471523103814670
domInteractive9135383215
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

metamaskbot avatar Feb 20 '24 16:02 metamaskbot