fix: mv3 sw lavamoat background wrapping - #44187
Merged
Merged
Conversation
itsyoboieltr
requested review from
FrederikBolding,
Gudahtt,
davidmurdoch and
naugtur
July 6, 2026 00:20
itsyoboieltr
added
the
no-changelog
Contributor
|
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. |
Contributor
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/extension-platform (2 files, +315 -83)
📜 @MetaMask/policy-reviewers (12 files, +8044 -160)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. 👨🔧 @itsyoboieltr (2 files, +315 -83)
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
itsyoboieltr
commented
Jul 6, 2026
Contributor
Builds ready [b84b701]
⚡ Performance Benchmarks (Total: 🟢 20 pass · 🟡 5 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
seaona
reviewed
Jul 6, 2026
seaona
left a comment
seaona
left a comment
Member
There was a problem hiding this comment.
Could we add an e2e test to verify this? Maybe something similar as we had for lockdown-more?
Related conversation
https://consensys.slack.com/archives/CTQAGKY5V/p1781256822162049
Contributor
Builds ready [4c9d190]
⚡ Performance Benchmarks (Total: 🟢 14 pass · 🟡 9 warn · 🔴 1 fail)
Bundle sizes
|
Contributor
|
I've built this branch and read some relevant files in dist. the layer works as advertised and makes unwrapped copies of modules for service-worker. One consequence of that is all of bootstrap goes into that layer too and there's a surprising depth of dependencies involved there. All of sentry makes sense, but some of the items I found there are not what we'd want duplicated Here's a few references I found (ignore webpack specific underscores) 👉 full list, somewhat messyincludes all selectors from the service-worker chunk, so these are package names but also file names ``` _address_mjs__ _api_js__ _applyScopeDataToEvent_js__ _assert_mjs__ _asyncContext_index_js__ _backgroundtab_js__ _baggage_js__ _BaseController_mjs__ _breadcrumbs_js__ _browser_js__ _button_mjs__ _carrier_js__ _classes_mjs__ _client_js__ _cls_js__ _component_mjs__ _components_index_mjs__ _constants_js__ _conversions_js__ _copyable_mjs__ _currentScopes_js__ _datetime_js__ _debug_build_js__ _defaultScopes_js__ _digits_js__ _divider_mjs__ _dsn_js__ _duration_js__ _dynamicSamplingContext_js__ _english_js__ _envelope_js__ _env_js__ _error_constants_mjs__ _error_js__ _error_mjs__ _errors_js__ _errors_mjs__ _eventbuilder_js__ _eventProcessors_js__ _exports_js__ _extractEthjsErrorMessage__ _feedbackAsync_js__ _feedbackSync_js__ _fetch_js__ _formats_js__ _formatter_js__ _form_mjs__ _generateUniqueID_js__ _getActivationStart_js__ _getNavigationEntry_js__ _handlers_index_mjs__ _handlers_js__ _heading_mjs__ _helpers_js__ _image_mjs__ _impl_conversions_js__ _impl_diff_js__ _impl_digits_js__ _impl_formats_js__ _impl_formatter_js__ _impl_invalid_js__ _impl_locale_js__ _impl_regexParser_js__ _impl_tokenParser_js__ _impl_util_js__ _impl_zoneUtil_js__ _info_js__ _input_mjs__ _install_type__ _instance_js__ _instrument_js__ _integration_js__ _integrations_breadcrumbs_js__ _integrations_browserapierrors_js__ _integrations_contextlines_js__ _integrations_globalhandlers_js__ _integrations_httpclient_js__ _integrations_httpcontext_js__ _integrations_linkederrors_js__ _integrations_reportingobserver_js__ _internals_index_mjs__ _interval_js__ _invalid_js__ _is_js__ _jsx_index_mjs__ _lib_bindReporter_js__ _lib_getActivationStart_js__ _lib_getNavigationEntry_js__ _lib_getVisibilityWatcher_js__ _lib_initMetric_js__ _lib_observe_js__ _lib_onHidden_js__ _lib_polyfills_interactionCountPolyfill_js__ _lib_runOnce_js__ _lib_setupSentry__ _lib_whenActivated_js__ _logger_js__ _logSpans_js__ _measurement_js__ _memo_js__ _metadata_js__ _metamask_base_controller__ _metamask_permission_controller__ _metamask_rpc_errors__ _metamask_snaps_rpc_methods__ _metamask_snaps_sdk__ _metamask_superstruct__ _metamask_utils__ _metrics_js__ _metrics_metric_summary_js__ _misc_js__ _misc_mjs__ _noble_hashes_sha3__ _node_js__ _nodes_mjs__ _normalize_js__ _object_js__ _observe_js__ _onFCP_js__ _permissions__ _polyfill__ _profiling_integration_js__ _regex_js__ _request_js__ _rng_js__ _row_mjs__ _safe_stringify__ _sampling_js__ _scope_js__ _scripts_lib_extension_lazy_listener_extension_lazy_listener__ _scripts_load_bootstrap__ _sdk_js__ _semanticAttributes_js__ _sentry_browser__ _sentry_core__ _sentry_get_state__ _sentry_internal_browser_utils__ _sentry_internal_feedback__ _sentry_internal_replay__ _sentry_internal_replay_canvas__ _sentry_make_transport__ _sentry_metametrics__ _sentryNonRecordingSpan_js__ _sentrySpan_js__ _sentry_trace_propagation__ _sentry_utils__ _session_js__ _settings_js__ _shared_constants_app__ _shared_constants_ui_initialization__ _shared_lib_manifestFlags__ _shared_lib_mv3_utils__ _shared_lib_sentry__ _shared_lib_sentry_release__ _snaps_permissions__ _spanOnScope_js__ _spanstatus_js__ _spanUtils_js__ _spinner_mjs__ _stack_parsers_js__ _stackStrategy_js__ _stacktrace_js__ _startProfileForSpan_js__ _stringify_js__ _string_js__ _struct_mjs__ _structs_mjs__ _superstruct_mjs__ _supports_js__ _syncpromise_js__ _text_mjs__ _time_js__ _trace_js__ _tracing_browserTracingIntegration_js__ _tracing_dynamicSamplingContext_js__ _tracing_request_js__ _tracing_sentryNonRecordingSpan_js__ _tracing_spanstatus_js__ _tracing_trace_js__ _transports_fetch_js__ _transports_offline_js__ _types_index_mjs__ _types_js__ _types_mjs__ _u64_js__ _ui_index_mjs__ _userfeedback_js__ _utilities_mjs__ _util_js__ _utils_handleCallbackErrors_js__ _utils_hasTracingEnabled_js__ _utils_js__ _utils_lazyLoadIntegration_js__ _utils_mjs__ _utils_parseSampleRate_js__ _utils_prepareEvent_js__ _utils_spanOnScope_js__ _utils_spanUtils_js__ _validate_js__ _version_js__ _web_vitals_getCLS_js__ _web_vitals_getFID_js__ _web_vitals_getINP_js__ _web_vitals_getLCP_js__ _web_vitals_lib_getActivationStart_js__ _web_vitals_lib_getNavigationEntry_js__ _web_vitals_lib_getVisibilityWatcher_js__ _web_vitals_lib_observe_js__ _web_vitals_lib_onHidden_js__ _web_vitals_onTTFB_js__ _worldwide_js__ _zone_js__ _zones_fixedOffsetZone_js__ _zones_IANAZone_js__ _zones_invalidZone_js__ _zones_systemZone_js__one possible culprit is that I have no idea what pulls snaps stuff in. potential solutions1 - rule + exclude loaderInstead of creating a layer, we could use the excludeLoader in rules and target the modules we want to work outside LavaMoat protections, but let them use the same protected dependencies.
2 - manual treeshakingTweak the bootstrap to drop some heavy dependencies
Note: it's likely we pulled the same packages in the browserify build. Doesn't mean it's good :) |
davidmurdoch
previously approved these changes
Jul 10, 2026
davidmurdoch
left a comment
davidmurdoch
left a comment
Contributor
There was a problem hiding this comment.
good for now. i think we can improve things later.
github-project-automation
Bot
moved this from Needs dev review
to Review finalised - Ready to be merged
in PR review queue
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 10, 2026
Contributor
Builds ready [9031555]
⚡ Performance Benchmarks (Total: 🟢 10 pass · 🟡 8 warn · 🔴 1 fail)
Bundle sizes
|
Member
|
Just pushed a merge commit to resolve some policy conflicts. The conflicts were just with the entries for our own packages, here is the diff: |
|
Contributor
Builds ready [eb17c5c]
⚡ Performance Benchmarks (Total: 🟢 14 pass · 🟡 8 warn · 🔴 2 fail)
Bundle sizes
|
davidmurdoch
approved these changes
Jul 10, 2026
HowardBraham
approved these changes
Jul 10, 2026
HowardBraham
added
the
retry-ci
davidmurdoch
approved these changes
Jul 10, 2026
Contributor
|
Missing release label release-13.40.0 on PR. Adding release label release-13.40.0 on PR and removing other release labels(release-13.41.0), as PR was added to branch 13.40.0 when release was cut. |
Contributor
🧪 Validation RunVerdict: ✅ the wrapping is real and adds no capability mv2 did not already grant — two hand-written entries are the exception — Claim: wrapping the mv3 background under LavaMoat brings it into containment. head Note Trial run of the MetaMask evidence skills,
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
nullUnsafeEntries(scripts/inpage.js,bootstrap) run fully outside LavaMoat with no runtime added.service-worker.tsstays in theunsafelayer (so its module + static bootstrap imports are excluded from wrapping) but its chunk now runs insafemode so it carries the LavaMoat runtime.runtimechunk, soservice-worker.jsis added toinlineLockdown— SES is inlined into its own bundle so the wrapped background has hardened intrinsics +Compartment/hardenavailable.lavamoatBackgroundLayerRulere-layersbackground.js(and, by inheritance, its whole graph) out ofunsafeinto abackgroundlayer so it gets wrapped, andlavamoatUnsafeLayerRulenowexcludesbackground.jsso the boundary module itself is wrapped rather than excluded.runtimeandscripts/contentscript.jschunks:scuttleGlobalThisis enabled for the SW with an exception list for the globals the unwrapped bootstrap and webpack'simportScriptschunk loader touch live after scuttling (importScripts,console,performance, timers, and the Sentry cluster). Dependency containment does not depend on scuttling — it comes from the Compartments — so wrapped deps still can't reach the real global regardless.lavamoat/webpack/mv3/{main,flask,beta,experimental}/policy.jsonwere regenerated to include the now-wrapped background dependency graph (~1198 → ~1594 resources). MV2 policies are untouched.Changelog
CHANGELOG entry: null
Related issues
Fixes:
Manual testing steps
Build the MV3 Chrome bundle with LavaMoat:
yarn webpack:lavamoat:build.Load
dist/chromeunpacked in Chrome, open the service worker inspectorConfirm the service worker installs and reaches
MetaMask initialization completewith noinaccessible under scuttling mode/ missing-global / policy errors.Containment check: temporarily patch a background dependency (e.g.
node_modules/loglevel/lib/loglevel.js) with:Rebuild, reload, and confirm in the SW console that
clients/registration/serviceWorkerare"undefined"andkeyCountis small (the dep sees its policy-scoped Compartment global).Screenshots/Recordings
N/A — no UI changes. Behavioral verification is the service-worker console output described in Manual testing steps.
Before
A background dependency could see globals outside of its intended scope.
After
A wrapped background dependency sees only its Compartment global.
Pre-merge author checklist
Pre-merge reviewer checklist
Note
High Risk
Changes core extension security boundaries for the MV3 service worker and background; mis-layering could break the SW or leave background code unwrapped.
Overview
Fixes MV3 so
background.jsand its dependency graph run inside LavaMoat instead of inheriting the unwrapped service worker “unsafe” layer.LavamoatPluginnow treats onlyinpage/bootstrapasnull_unsafe, keepsservice-worker.tsin the unsafe webpack layer but runs that chunk insafemode (with inlined SES onservice-worker.js,scuttleGlobalThis, andimportScriptsin the exception list).lavamoatBackgroundLayerRuleplus anexcludeonbackground.jsin the unsafe exclude-loader pull the background entry and its graph into a dedicatedbackgroundlayer so they get wrapped.Adds
LavamoatPlugin.test.tsregression tests for the layer rules and per-chunk runtime config. LavaMoat policies are updated (notably a large MV3policy.jsonregen for the newly wrapped background graph; smallpolicy-override.jsontweaks including zod global write).Reviewed by Cursor Bugbot for commit eb17c5c. Bugbot is set up for automated code reviews on this repo. Configure here.