Skip to content

Commit

Permalink
fix: Fix LavaMoat build failures and restore RegExp OOM mitigation (#…
Browse files Browse the repository at this point in the history
…29637)

## **Description**

LavaMoat was updated recently, but the patch we had of `lavamoat-core`
was not updated with it. This restores the changes made in that patch.

Note that the old patch still exists because we're still using the older
version of `lavamoat-core` for the `lavamoat-viz` tool.

The patch had three changes; the third was upstreamed already so it was
not required, but the first two (RegExp cache and skipping policy write)
were still required.
[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29637?quickstart=1)

## **Related issues**

Fixes #29482

## **Manual testing steps**

There is no consistent way to test the intermittent build failure issue,
though we could try running this over and over.

For the RegExp cache, we never had a clear reproduction for the OOM
error that was meant to mitigate either.

## **Screenshots/Recordings**

N/A

## **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.

## **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.

---------

Co-authored-by: MetaMask Bot <[email protected]>
  • Loading branch information
Gudahtt and metamaskbot authored Jan 13, 2025
1 parent a55c0ce commit 9344d0a
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 4 deletions.
48 changes: 48 additions & 0 deletions .yarn/patches/lavamoat-core-npm-16.2.2-e361ff1f8a.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
diff --git a/src/loadPolicy.js b/src/loadPolicy.js
index b3053356c739a5f351fd4e271b67e31ee00bb4dc..7daebe4104ce5d799f90068516b1d0aaa58546c0 100644
--- a/src/loadPolicy.js
+++ b/src/loadPolicy.js
@@ -101,10 +101,9 @@ async function loadPolicyAndApplyOverrides({

const finalPolicy = mergePolicy(policy, policyOverride)

- // TODO: Only write if merge results in changes.
- // Would have to make a deep equal check on whole policy, which is a waste of time.
- // mergePolicy() should be able to do it in one pass.
- await fs.writeFile(policyPath, jsonStringifySortedPolicy(finalPolicy))
+ // Skip policy write step to prevent intermittent build failures
+ // The extension validates the policy in a separate step, we don't need it
+ // to be written to disk here.

return finalPolicy
}
diff --git a/src/scuttle.js b/src/scuttle.js
index c096a1fbf0bfe8a8f22290852881598f74fff4b1..b7438881be5e25b48ea18919a4b642a0b14cc317 100644
--- a/src/scuttle.js
+++ b/src/scuttle.js
@@ -77,6 +77,8 @@ function generateScuttleOpts(globalRef, originalOpts = create(null)) {
exceptions: [],
scuttlerName: '',
}
+ // cache regular expressions to work around https://github.com/MetaMask/metamask-extension/issues/21006
+ const regexCache = new Map()
const opts = assign(
create(null),
originalOpts === true ? defaultOpts : originalOpts,
@@ -109,10 +111,15 @@ function generateScuttleOpts(globalRef, originalOpts = create(null)) {
if (!except.startsWith('/')) {
return except
}
+ if (regexCache.has(except)) {
+ return regexCache.get(except)
+ }
const parts = except.split('/')
const pattern = parts.slice(1, -1).join('/')
const flags = parts[parts.length - 1]
- return new RegExp(pattern, flags)
+ const re = new RegExp(pattern, flags)
+ regexCache.set(except, re)
+ return re
}
}

2 changes: 1 addition & 1 deletion lavamoat/build-system/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -4327,7 +4327,7 @@
"builtin": {
"node:events": true,
"node:fs.readFileSync": true,
"node:fs/promises.writeFile": true,
"node:fs/promises": true,
"node:path.extname": true,
"node:path.join": true
},
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@
"[email protected]": "^7.5.4",
"@trezor/schema-utils@npm:1.0.2": "patch:@trezor/schema-utils@npm%3A1.0.2#~/.yarn/patches/@trezor-schema-utils-npm-1.0.2-7dd48689b2.patch",
"lavamoat-core@npm:^15.1.1": "patch:lavamoat-core@npm%3A15.1.1#~/.yarn/patches/lavamoat-core-npm-15.1.1-51fbe39988.patch",
"lavamoat-core@npm:^16.2.2": "patch:lavamoat-core@npm%3A16.2.2#~/.yarn/patches/lavamoat-core-npm-16.2.2-e361ff1f8a.patch",
"@metamask/snaps-sdk": "^6.14.0",
"@swc/[email protected]": "^0.1.6",
"@babel/core": "patch:@babel/core@npm%3A7.25.9#~/.yarn/patches/@babel-core-npm-7.25.9-4ae3bff7f3.patch",
Expand Down
22 changes: 19 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -25258,6 +25258,22 @@ __metadata:
languageName: node
linkType: hard

"lavamoat-core@npm:16.2.2":
version: 16.2.2
resolution: "lavamoat-core@npm:16.2.2"
dependencies:
"@babel/types": "npm:7.26.0"
json-stable-stringify: "npm:1.1.1"
lavamoat-tofu: "npm:^8.0.4"
merge-deep: "npm:3.0.3"
ses: "npm:1.9.0"
type-fest: "npm:4.30.0"
bin:
lavamoat-sort-policy: src/policy-sort-cli.js
checksum: 10/4509a560ec6f35aabf3f2915beeaefb526db06121b0bea40864c8f75d0b3e55df089bedd719392a3224dab040fa03a2593c998a66ea23ca820c29de038aa448b
languageName: node
linkType: hard

"lavamoat-core@npm:^15.2.1":
version: 15.3.0
resolution: "lavamoat-core@npm:15.3.0"
Expand All @@ -25271,9 +25287,9 @@ __metadata:
languageName: node
linkType: hard

"lavamoat-core@npm:^16.2.2":
"lavamoat-core@patch:lavamoat-core@npm%3A16.2.2#~/.yarn/patches/lavamoat-core-npm-16.2.2-e361ff1f8a.patch":
version: 16.2.2
resolution: "lavamoat-core@npm:16.2.2"
resolution: "lavamoat-core@patch:lavamoat-core@npm%3A16.2.2#~/.yarn/patches/lavamoat-core-npm-16.2.2-e361ff1f8a.patch::version=16.2.2&hash=c09704"
dependencies:
"@babel/types": "npm:7.26.0"
json-stable-stringify: "npm:1.1.1"
Expand All @@ -25283,7 +25299,7 @@ __metadata:
type-fest: "npm:4.30.0"
bin:
lavamoat-sort-policy: src/policy-sort-cli.js
checksum: 10/4509a560ec6f35aabf3f2915beeaefb526db06121b0bea40864c8f75d0b3e55df089bedd719392a3224dab040fa03a2593c998a66ea23ca820c29de038aa448b
checksum: 10/d0feaa9598f50ba4fa268c227e81cf6235d12f6272f9b06d9e3e350367081ce3060c67caff9855211ce5f05f995cd342361e4cfa2bc108a7d525c0a630301efc
languageName: node
linkType: hard

Expand Down

0 comments on commit 9344d0a

Please sign in to comment.