std_npm_args: ignore scripts optionally, by default. #21136
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances security for NPM installations by adding an ignore_scripts parameter (defaulting to true) to std_npm_args and related functions. This change aligns with pnpm 10+ behavior and mitigates NPM supply-side attacks by preventing automatic execution of install scripts unless explicitly allowed.
- Added
ignore_scriptsparameter withtruedefault tostd_npm_args,std_npm_install_args, andlocal_npm_install_args - Updated function signatures and type annotations to support the new parameter
- Modified implementation to conditionally append
--ignore-scriptsflag based on parameter value
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Library/Homebrew/utils.rb | Contains an unintended comment modification (likely accidental edit) |
| Library/Homebrew/language/node.rb | Adds ignore_scripts parameter to std_npm_install_args and local_npm_install_args, appends --ignore-scripts flag when enabled |
| Library/Homebrew/formula.rb | Adds ignore_scripts parameter to public std_npm_args API and forwards it to underlying Node language functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5a46a18 to
f73f11d
Compare
bevanjkay
left a comment
There was a problem hiding this comment.
Makes sense to me, I think if we were to add pnpm as an option/default separating the code in the language class would be more suitable and maintainable anyway.
p-linnane
left a comment
There was a problem hiding this comment.
Much better than a full cutover to pnpm. Thanks!
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This emulates what `pnpm` 10+ does by default. In response to current NPM supply-side attacks. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
355b34a to
bc7621c
Compare
As `std_npm_args` now ignores scripts by default (Homebrew/brew#21136), let's start accepting NodeJS formula updates Signed-off-by: botantony <antonsm21@gmail.com>
appwrite: remove universal binaries Co-authored-by: Patrick Linnane <patrick@linnane.io> Signed-off-by: botantony <antonsm21@gmail.com>
hf-mcp-server: sleep longer Signed-off-by: Rui Chen <rui@chenrui.dev>
|
now, all the fsevent.node stuff is coming back with this change |
|
@chenrui333 Can you elaborate on that and open some relevant issue(s)? |
|
I will have to compile later this weekend and see what exactly going on across the board. |
This emulates what
pnpm10+ does by default.In response to current NPM supply-side attacks.