chore: update browserslist #39054
chore: update browserslist #39054
Conversation
davidmurdoch
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
|
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. |
|
@metamaskbot update-policies |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
✨ Files requiring CODEOWNER review ✨📜 @MetaMask/policy-reviewers (1 files, +3 -1)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. |
Builds ready [4286dd8]
UI Startup Metrics (1291 ± 117 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
davidmurdoch
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
davidmurdoch
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
davidmurdoch
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| "console.log": true, | ||
| "console.warn": true, | ||
| "process.exit": true | ||
| "process": true |
There was a problem hiding this comment.
Broadened process permissions may expose environment variables
Medium Severity
The policy change for browserslist>baseline-browser-mapping from process.exit to process significantly broadens permissions. The full process object includes process.env (environment variables often containing CI secrets, API keys), process.argv, and other sensitive capabilities. Other packages in this policy file use granular permissions like process.exit, process.cwd, or process.stderr.write. If baseline-browser-mapping only needs to exit the process, the broader process permission grants unnecessary access that could be exploited if the package is compromised.
There was a problem hiding this comment.
Good catch. Granting unnecessary process.env access to a build process dependency isn't ideal, and the fix looks simple:
The process entry can be removed by adding process.argv as an override (the following passes yarn lavamoat:auto locally):
diff --git a/lavamoat/build-system/policy-override.json b/lavamoat/build-system/policy-override.json
index 512734d809..1f81ec6b2d 100644
--- a/lavamoat/build-system/policy-override.json
+++ b/lavamoat/build-system/policy-override.json
@@ -593,6 +593,12 @@
"packages": {
"@babel/core>@babel/types": true
}
+ },
+ "browserslist>baseline-browser-mapping": {
+ "globals": {
+ "process.exit": true,
+ "process.argv": true
+ }
}
}
}
diff --git a/lavamoat/build-system/policy.json b/lavamoat/build-system/policy.json
index 36f6a625de..79b637c254 100644
--- a/lavamoat/build-system/policy.json
+++ b/lavamoat/build-system/policy.json
@@ -1620,6 +1620,7 @@
"globals": {
"console.log": true,
"console.warn": true,
+ "process.argv": true,
"process.exit": true
}
},That said, the packages don't seem to be used in any way that could directly expose vulnerabilities, and they're both well maintained by reputable orgs. @davidmurdoch What do you think -- worth the trouble?
@naugtur It looks like policy generation came up with wider permissions than necessary here (top-level global object instead of nested capabilities). Is this expected behavior?
There was a problem hiding this comment.
re "worth the trouble" - sha1-hulud has caught very reputable organizations off-guard. It's not whether we trust the org, it's whether we trust the contents of a package on npm ;)
Narrowing down these policies is a very responsible thing to do.
Policy generation is the best-effort estimation of what seems necessary for the package to work. Depending on how it accesses the globals we sometimes can figure out a subset of fields from the AST, but some AST shapes will cause us to defer to the whole global field. We can't generate a policy that's too narrow because it'd force devs to have to add overrides for many common and harmless globals.
The globals that should be preferred for narrowing down are things like process, document, or chrome - the ones that are bags of mixed capabilities. Once we get past the basics of providing a modern version of lavamoat for node, we are planning to focus on the capabilities that need special treatment (at runtime and in policy review)
Please continue to be vigilant about narrowing down policies for the most powerful capabilities. This is great work! 👏 👏 👏
Also, I'm impressed that AI can now understand implications of policy diffs.
Any ideas how to make it better?
There was a problem hiding this comment.
Looks like this can't be narrowed after all due to a typeof process !== undefined in the dependency. :(
Any ideas how to make it better?
We could add relevant information or instructions to AGENTS.md, but it seems like cursorbot and other models already have a pretty good understanding of what lavamoat does just from having docs/lavamoat-policy-review-process.md in context.
I asked which AST shapes would cause deferral to wider permissions here and got this response:
AST Shapes That Cause LavaMoat to Defer to Wider Permissions
LavaMoat's policy generator (lavamoat-tofu) tries to determine the most specific global access path by walking the AST's member expression chain. When it can't follow the chain, it defers to the parent/wider permission.
How the Analysis Works
For an identifier like process, lavamoat-tofu:
- Finds all references to global
process - For each reference, walks up the parent chain looking for an unbroken sequence of non-computed
MemberExpressionnodes - Builds a path like
['process', 'env', 'NODE_ENV']from that chain - If the chain is empty or broken, returns just
['process']
The key function is isNonComputedMemberLikeExpression:
function isNonComputedMemberLikeExpression(node) {
return !node.computed && isMemberLikeExpression(node);
}AST Shapes That Cause Deferral
1. typeof Check on Bare Identifier
typeof process !== 'undefined';AST:
UnaryExpression
├── operator: "typeof"
└── argument: Identifier("process") ← parent is UnaryExpression, NOT MemberExpression
Why it defers: The process identifier's parent is UnaryExpression, which fails isNonComputedMemberLikeExpression. Result: memberExpressions = [], so path = ['process'].
2. Bare Identifier in Logical/Binary Expressions
process && process.env;AST:
LogicalExpression
├── operator: "&&"
├── left: Identifier("process") ← parent is LogicalExpression, NOT MemberExpression
└── right: MemberExpression(process.env)
Why it defers: The left process has LogicalExpression as parent. The right process.env would produce ['process', 'env'], but the left one produces ['process']. LavaMoat then reduces to the topmost (widest) path, keeping just process.
3. Computed Property Access
process['env'];
// or
process[someVariable];AST:
MemberExpression
├── object: Identifier("process")
├── property: StringLiteral("env") or Identifier(someVariable)
└── computed: true ← fails isNonComputedMemberLikeExpression!
Why it defers: computed: true fails the check. The chain breaks, so path = ['process'].
4. Identifier Passed as Argument
someFunction(process);AST:
CallExpression
├── callee: Identifier("someFunction")
└── arguments: [Identifier("process")] ← parent is CallExpression, NOT MemberExpression
Why it defers: Parent is CallExpression, not MemberExpression. Path = ['process'].
5. Assignment to Variable (Aliasing)
const p = process;
p.env.NODE_ENV; // later usageAST for const p = process:
VariableDeclarator
├── id: Identifier("p")
└── init: Identifier("process") ← parent is VariableDeclarator, NOT MemberExpression
Why it defers: The process reference is assigned to p. LavaMoat sees process with parent VariableDeclarator. Path = ['process'].
6. Destructuring
const { env } = process;AST:
VariableDeclarator
├── id: ObjectPattern({ env })
└── init: Identifier("process") ← bare reference to get the object to destructure
Why it defers: Same as aliasing - process is referenced without property access. The destructuring analysis in lavamoat-tofu tries to expand this, but a RestElement or computed property in the pattern causes fallback to ['process'].
The Reduction Rule
After analyzing all references, lavamoat-tofu calls reduceToTopmostApiCalls:
// If array contains 'process' and 'process.env' just keep 'process'So even if some references produce process.env and others produce process, the final policy contains just process since it's the topmost (widest, most permissive) path that covers all accesses.
Real-World Example: baseline-browser-mapping v2.9.11
The code contains:
"undefined"!=typeof process && process.env && ...This creates:
typeof process→ path['process'](Pattern 1)process && ...→ left side produces['process'](Pattern 2)process.env→ path['process', 'env']
After reduction: ['process'] wins because it's the topmost path.
Summary Table
| Pattern | Parent Node Type | Can Follow Chain? | Result |
|---|---|---|---|
process.env |
MemberExpression |
✅ Yes | process.env |
typeof process |
UnaryExpression |
❌ No | process |
process && x |
LogicalExpression |
❌ No | process |
process["env"] |
MemberExpression (computed) |
❌ No | process |
fn(process) |
CallExpression |
❌ No | process |
const p = process |
VariableDeclarator |
❌ No | process |
const {env} = process |
VariableDeclarator |
❌ No | process |
Builds ready [99f6224]
UI Startup Metrics (1232 ± 89 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
davidmurdoch
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
davidmurdoch
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
Builds ready [2781195]
UI Startup Metrics (1271 ± 120 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
metamaskbot
added
the
release-13.14.0
CHANGELOG entry: null
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Updates dependency targets and LavaMoat policy to align with newer Browserslist data.
browserslistto^4.28.1and refreshes lockfile, updatingbaseline-browser-mapping,caniuse-lite,electron-to-chromium,node-releases, andupdate-browserslist-dblavamoat/build-system/policy.jsonglobals: replaceprocess.exitwithprocess, addconsole.infoandconsole.traceWritten by Cursor Bugbot for commit 2781195. This will update automatically on new commits. Configure here.