Repository navigation
release: 13.39.2 - #44395
release: 13.39.2#44395
Conversation
|
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. |
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
… leak by avoiding `DocumentPictureInPicture` instantiation in Snow hook cp-13.39.2 (#44401) - fix: Fix Firefox detatched-window memory leak by avoiding `DocumentPictureInPicture` instantiation in Snow hook cp-13.39.2 (#44352) ## **Motivation** On Firefox, every closed MetaMask window (popup, notification, home) is retained in memory ([#42891](#42891) and recent internal reports). - **Rate:** ~70 detached window entries (~105MB, fresh test wallet) per popup open/close, linear with use. 30 cycles leave **3.56GB** (13.37.0) / **3.31GB** (13.38.1) of detached documents, surviving forced GC. State-heavy wallets multiply every retained document. - **Mechanism** (heap-graph traced, intervention-tested): Snow's picture-in-picture hook reads `win?.documentPictureInPicture?.requestWindow` in every window. The read lazily instantiates a per-window `DocumentPictureInPicture`, and Firefox's cycle collector fails to break the instance's preserved-wrapper cycle after window close, pinning the entire realm. - **Why now:** Firefox shipped the API in 151 (May 2026). The April-2023 hook was dormant until users auto-updated. The collection failure itself is a Gecko defect (upstream filing in progress; prior art: [Bugzilla 1902438](https://bugzilla.mozilla.org/show_bug.cgi?id=1902438)), but this fix does not depend on Mozilla. ## **Description** Applies the upstream Snow fix ([LavaMoat/snow#171](LavaMoat/snow#171)) as a Yarn patch: the hook moves from the per-window instance to `DocumentPictureInPicture.prototype`, reached through the interface object, which does not invoke the instance-creating getter. - Call semantics unchanged: instance lookup falls through to the patched prototype, `this` stays the instance, same wrapped native. Snow's PiP realm monitoring is preserved. - The patch covers `snow.prod.js` and `snow.js` in addition to `src/request.js`: the extension consumes Snow's prebuilt bundle as a LavaMoat static shim (`LavamoatPlugin`), and webpack never processes `@lavamoat/snow/**`. - **Interim:** remove this patch when `@lavamoat/snow` is bumped to a release containing the upstream fix. The bump is the removal trigger. ## **Cherry-pick candidacy** - Affected population: every Firefox user on 151+ (effectively the whole Firefox base via auto-update). Reproduces from a fresh wallet in five popup opens. No workaround short of a browser restart. - Proposed: next RC, with a hotfix on the current production line worth considering. - Snow is security-team-owned. Their sign-off on shipping a patched Snow is part of this review. ## **Verification** | Check | Result | |---|---| | Intervention: official 13.37.0 artifact, only this hook changed, 30 cycles | **+2,100 docs (3.35GB) → -13 (1.6MB total)**. Hook is the sole delta, so this is also the causal proof | | This PR's CI artifact (`d4dd55f`), probed directly | Patched hook present; 5 cycles: +7 KB-scale entries (1.1MB) vs +345 docs (+534MB) stock | | Field | Original reporter confirms the leak is gone on this PR's CI build | | Independent repro | Manual steps + automated probe: https://github.com/MajorLift/snow/tree/verification/pip-realm-leak/verification | ## **Related issues** Fixes: #42891 ## **Manual testing steps** 1. Install this PR's Firefox build (metamaskbot comment) in a fresh profile via `about:debugging` → Load Temporary Add-on. Onboard with a test SRP. 2. Confirm the fix is live: extension console → `DocumentPictureInPicture.prototype.requestWindow.name` → `"open"` (stock: `"requestWindow"`). 3. Open/close the popup five times, visiting activity or a swap view on a few opens. 4. `about:memory` → Minimize memory usage → Measure → filter `detached`: no `moz-extension` UI-document cohorts (stock: ~70 entries / ~105MB per cycle). ## **Screenshots/Recordings** Heap-graph traces, memory reports, and retaining-path analysis: see the verification bundle above. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. CHANGELOG entry: Fixed a memory leak on Firefox where closed MetaMask windows were retained in memory, growing extension memory use with every popup open [942441b](942441b) Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>
## **Description**
1. **Split the entry sets.** `nullUnsafeEntries` (`scripts/inpage.js`,
`bootstrap`) run fully outside LavaMoat with no runtime added.
`service-worker.ts` stays in the `unsafe` layer (so its module + static
bootstrap imports are excluded from wrapping) **but** its chunk now runs
in `safe` mode so it carries the LavaMoat runtime.
2. **SES in the SW bundle.** The MV3 SW is self-contained and loads no
shared `runtime` chunk, so `service-worker.js` is added to
`inlineLockdown` — SES is inlined into its own bundle so the wrapped
background has hardened intrinsics + `Compartment`/`harden` available.
3. **Layer boundary at the dynamic import.**
`lavamoatBackgroundLayerRule` re-layers `background.js` (and, by
inheritance, its whole graph) out of `unsafe` into a `background` layer
so it gets wrapped, and `lavamoatUnsafeLayerRule` now `exclude`s
`background.js` so the boundary module itself is wrapped rather than
excluded.
4. **Scuttling on the SW global.** Defense-in-depth parity with the UI
`runtime` and `scripts/contentscript.js` chunks: `scuttleGlobalThis` is
enabled for the SW with an exception list for the globals the
*unwrapped* bootstrap and webpack's `importScripts` chunk 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.
5. **Policies regenerated.**
`lavamoat/webpack/mv3/{main,flask,beta,experimental}/policy.json` were
regenerated to include the now-wrapped background dependency graph
(~1198 → ~1594 resources). MV2 policies are untouched.
## **Changelog**
CHANGELOG entry: null
## **Related issues**
Fixes: <!-- add the sev1 security ticket link -->
## **Manual testing steps**
1. Build the MV3 Chrome bundle with LavaMoat: `yarn
webpack:lavamoat:build`.
2. Load `dist/chrome` unpacked in Chrome, open the **service worker**
inspector
3. Confirm the service worker installs and reaches `MetaMask
initialization complete` with **no** `inaccessible under scuttling mode`
/ missing-global / policy errors.
4. **Containment check**: temporarily patch a background dependency
(e.g.`node_modules/loglevel/lib/loglevel.js`) with:
```js
console.log('probe', {
clients: typeof globalThis.clients,
registration: typeof globalThis.registration,
serviceWorker: typeof globalThis.serviceWorker,
keyCount: Object.keys(globalThis).length,
});
```
Rebuild, reload, and confirm in the SW console that
`clients`/`registration`/`serviceWorker` are `"undefined"` and
`keyCount` is 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**
- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable <!-- follow-up: regression guard
asserting background.js resolves out of the `unsafe` layer / SW chunks
contain the LavaMoat wrapper -->
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
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.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **High Risk**
> Changes MV3 extension sandboxing for the service worker and background
(LavaMoat layers, SES lockdown, global scuttling); misconfiguration
could break SW startup or weaken containment.
>
> **Overview**
> Fixes MV3 so **`background.js` and its dependency graph run inside
LavaMoat** instead of inheriting the service worker’s unwrapped `unsafe`
layer.
>
> **Webpack / LavaMoat plugin:** `service-worker.ts` is removed from
fully unwrapped entries but stays on the `unsafe` layer for bootstrap
imports; its chunk now uses **`safe` mode** with an embedded LavaMoat
runtime. **`service-worker.js`** is added to **`inlineLockdown`** (SES
inlined in the SW bundle). A new **`background` webpack layer**
re-layers `app/scripts/background.js` out of `unsafe`, and the unsafe
exclude-loader **`exclude`s** that path so the boundary module is
wrapped. Shared **`scuttleGlobalThis`** exceptions are centralized; the
SW chunk enables scuttling with **`importScripts`** (and shared
exceptions) allowed.
>
> **Tests:** New `plugins.LavamoatPlugin.test.ts` guards layer regexes,
SW `safe` mode, and plugin rule registration.
>
> **Policies:** MV3 `policy.json` (e.g. beta) is regenerated for the
wrapped background graph; **`policy-override.json`** updates across
flavors add entries such as
`@metamask/assets-controllers>@tanstack/query-core`, `centrifuge`, and
gridplus `zod`.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
c251e94. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Co-authored-by: Norbert Elter <72046715+itsyoboieltr@users.noreply.github.com>
Co-authored-by: Howard Braham <howrad@gmail.com>
This PR updates the change log for 13.39.2. (Hotfix - no test plan generated.) --------- Co-authored-by: metamaskbot <metamaskbot@users.noreply.github.com> Co-authored-by: Mark Stacey <mark.stacey@consensys.net>
The merge-base changed after approval.
Builds ready [311d329]
⚡ Performance Benchmarks (Total: 🟢 15 pass · 🟡 6 warn · 🔴 3 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
AI Test Plan
Cherry-Pick Scenarios (3)High Risk Scenarios (1)1. Firefox Detached Window Stability and MemoryRisk Level: HIGH Why This Matters: Cherry-pick 44401 fixes a Firefox detached-window memory leak by avoiding DocumentPictureInPicture usage; regressions could reintroduce leaks or break window initialization. Test Steps:
Medium Risk Scenarios (2)1. Firefox Popup/Detached Window Initialization on First PromptRisk Level: MEDIUM Why This Matters: Cherry-pick 44401 fixes a leak tied to window instantiation; it may affect how the first or subsequent notification windows initialize in Firefox. Test Steps:
2. Firefox Long-Running Session Memory Behavior (Detached Window)Risk Level: MEDIUM Why This Matters: Cherry-pick 44401 fixes persistent memory growth in Firefox; long-running sessions should no longer leak and must remain stable. Test Steps:
Release Scenarios (7)High Risk Scenarios (4)1. Dapp Connection and Provider InjectionRisk Level: HIGH Why This Matters: LavaMoat policy/plugin changes can block or delay inpage provider injection, breaking basic dapp connectivity. Test Steps:
2. Transaction Confirmation and Sending (ETH transfer)Risk Level: HIGH Why This Matters: Runtime sandbox/instrumentation adjustments can disrupt the tx approval UI or background-controller message flow, preventing sends. Test Steps:
3. EIP-712 Typed Data Signing (e.g., SIWE)Risk Level: HIGH Why This Matters: Policy changes can block content-script <-> background messaging, causing signature prompts to break or return invalid results. Test Steps:
4. Concurrent Notifications Handling (Connect/Add Chain/Sign)Risk Level: HIGH Why This Matters: Multiple parallel UI surfaces rely on secure bridges; policy or hook updates can cause race conditions and blank modals. Test Steps:
Medium Risk Scenarios (3)1. Unlock Flow and Popup State PersistenceRisk Level: MEDIUM Why This Matters: Hooking/policy changes can disrupt post-unlock routing and state handoff, causing users to lose in-flight actions. Test Steps:
2. Network Add/Switch via dapp (addEthereumChain/switchEthereumChain)Risk Level: MEDIUM Why This Matters: Provider and background message changes can block network prompts or event dispatch, breaking add/switch flows. Test Steps:
3. Account Switching and Events PropagationRisk Level: MEDIUM Why This Matters: Instrumentation changes may prevent accountsChanged from propagating, leading to signing/sending from the wrong account. Test Steps:
Teams Sign-off StatusSigned off: None yet Awaiting sign-off (6): Generated by AI Test Plan Analyzer (gpt-5) at 2026-07-13T16:28:29.208Z AI generated test plan (JSON): test-plan-13.39.2.json |
Builds ready [311d329]
⚡ Performance Benchmarks (Total: 🟢 15 pass · 🟡 6 warn · 🔴 3 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
AI Test Plan
Cherry-Pick Scenarios (3)High Risk Scenarios (1)1. Firefox notification and popup lifecycle (Connect, Sign, Approve)Risk Level: HIGH Why This Matters: Cherry-pick 44401 fixes a Firefox detached-window memory leak by altering the Snow hook; popup lifecycles (connect, sign, approve) are directly impacted and could regress into stuck or phantom windows, broken close behavior, or runtime errors. Test Steps:
Medium Risk Scenarios (2)1. Transaction confirmation with detached opener (Firefox)Risk Level: MEDIUM Why This Matters: Cherry-pick 44401 fixes a Firefox detached-window memory leak; flows where the opener is closed are most susceptible to lifecycle bugs, so we must ensure confirmations still complete and windows close correctly. Test Steps:
2. Cross-browser regression check for popups (Chromium-based)Risk Level: MEDIUM Why This Matters: Cherry-pick 44401 fixes a Firefox-specific issue but changes shared Snow hook behavior; we must verify it introduces no regressions in Chromium-based browsers for popup creation/teardown. Test Steps:
Teams Sign-off StatusSigned off: None yet Awaiting sign-off (3): Generated by AI Test Plan Analyzer (gpt-5) at 2026-07-13T21:09:55.627Z AI generated test plan (JSON): test-plan-13.39.2.json |
|
Merge my PR |
🚀 v13.39.2 Testing & Release Quality Process
Hi Team,
As part of our new MetaMask Release Quality Process, here’s a quick overview of the key processes, testing strategies, and milestones to ensure a smooth and high-quality deployment.
📋 Key Processes
Testing Strategy
Conduct regression and exploratory testing for your functional areas, including automated and manual tests for critical workflows.
Focus on exploratory testing across the wallet, prioritize high-impact areas, and triage any Sentry errors found during testing.
Validate new functionalities and provide feedback to support release monitoring.
GitHub Signoff
Issue Resolution
Cherry-Picking Criteria
🗓️ Timeline and Milestones
✅ Signoff Checklist
Each team is responsible for signing off via GitHub. Use the checkbox below to track signoff completion:
Team sign-off checklist
This process is a major step forward in ensuring release stability and quality. Let’s stay aligned and make this release a success! 🚀
Feel free to reach out if you have questions or need clarification.
Many thanks in advance
Reference