Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new FormulaAuditor audit to detect disallowed npm dependencies (starting with @anthropic-ai/claude-agent-sdk) in installed node_modules trees for core formulae, with accompanying RSpec coverage.
Changes:
- Introduce
FormulaAuditor#audit_node_modulesto scanlibexec/lib/node_modulesfor incompatible npm packages in homebrew/core. - Emit an audit problem directing maintainers to Homebrew’s license guidelines when a rejected package is found.
- Add unit tests covering direct, nested, and dot-directory (
.pnpm-style) locations, plus core/non-core behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Library/Homebrew/formula_auditor.rb | Adds a new audit_node_modules audit that searches installed node_modules for a rejected package and reports a license-guidelines error. |
| Library/Homebrew/test/formula_auditor_spec.rb | Adds specs validating detection/skip behavior for the new audit_node_modules audit across common directory layouts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sig { void } | ||
| def audit_node_modules | ||
| return unless @core_tap | ||
|
|
There was a problem hiding this comment.
audit_node_modules will run as part of the default FormulaAuditor#audit method list whenever a formula is installed and has libexec/lib/node_modules. Since recursively scanning node_modules can be relatively expensive, consider making this check opt-in (e.g., only run when --only node_modules is specified, or behind a dedicated option) so standard brew audit runs aren't slowed down unexpectedly.
| # Make this audit opt-in to avoid expensive recursive node_modules scans on standard runs. | |
| only_list = respond_to?(:only) ? only : nil | |
| return unless only_list&.include?("node_modules") || only_list&.include?(:node_modules) |
|
|
||
| incompatible_license_packages.each do |package| | ||
| # Search for package in all nested node_modules. Also including dot match for .pnpm hoisted packages | ||
| next if node_modules.glob("{**/node_modules/,}#{package}/", File::FNM_DOTMATCH).empty? |
There was a problem hiding this comment.
node_modules.glob(...).empty? materializes an array of all matching paths for each package, which can be costly for large node_modules trees. Consider using Dir.glob with a block (or Find.find with early exit) to short-circuit on the first match and avoid allocating large arrays.
| next if node_modules.glob("{**/node_modules/,}#{package}/", File::FNM_DOTMATCH).empty? | |
| found = false | |
| Dir.glob(node_modules/"{**/node_modules/,}#{package}/", File::FNM_DOTMATCH) do | |
| found = true | |
| break | |
| end | |
| next unless found |
brew lgtm(style, typechecking and tests) with your changes locally?Audit to detect
@anthropic-ai/claude-agent-sdkhomebrew-core#273635Example output after
brew install promptfoowith timing (run in Linux container):Using an array to extend this with other npm packages.
Future ideas
In future, may consider moving this to a JSON file we store in Homebrew/core so it can be implemented as a tap-specific blacklist. Then can also be used in 3rd-party taps to detect unwanted dependencies.
And in further future, may want to analyze SPDX licenses of all installed npm packages to automatically detect disallowed licenses. May need a whitelist in this case when an npm package doesn't use SPDX.