Skip to content

fix: add --allow-scripts for bun/deno in npm v11 - #182

Merged
vltbaudbot merged 1 commit into
mainfrom
evert/fix-bun-ci-setup
Sep 30, 2026
Merged

vltbaudbot merged 1 commit into
mainfrom
evert/fix-bun-ci-setup

Conversation

@vltbaudbot

@vltbaudbot vltbaudbot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Problem

npm v11 blocks dependency lifecycle scripts by default. bun and deno both use postinstall scripts to download their platform-specific binaries, so npm install -g bun@latest and npm install -g deno@latest silently fail to install the actual binaries — causing exit code 1 in all ci+setup benchmark fixtures.

Fix

scripts/variations/ci+setup.sh

  • Add --allow-scripts=bun to SETUP_INSTALL_BUN
  • Add --allow-scripts=deno to SETUP_INSTALL_DENO

scripts/setup.sh

  • Add --allow-scripts=bun,deno,nx to the main npm install -g line (latent bug masked by the CI runner image having these tools pre-installed)

Context

This matches the pattern already used for pacquet (--allow-scripts=pnpm) and aube (--allow-scripts=@endevco/aube) in the same files.

Summary by CodeRabbit

  • Chores
    • Updated applicable global installation steps to permit lifecycle scripts for Bun, Deno, and Nx. Bun and Deno setup steps now use package-specific script permissions.
    • Other CI and preparation commands remain unchanged. These setup adjustments do not change application features or behavior.

npm v11 blocks dependency lifecycle scripts by default. bun and deno
both use postinstall scripts to download platform-specific binaries,
so they need --allow-scripts to work.

Changes:
- scripts/variations/ci+setup.sh: Add --allow-scripts=bun and
  --allow-scripts=deno to SETUP_INSTALL_BUN and SETUP_INSTALL_DENO
- scripts/setup.sh: Add --allow-scripts=bun,deno,nx to the main
  npm install -g line (latent bug masked by runner image having
  these pre-installed)
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 780f66dc-7d80-4cc6-bbbe-7b402ae91bc1

📥 Commits

Reviewing files that changed from the base of the PR and between 3c9e07e and dc4f71c.

📒 Files selected for processing (2)
  • scripts/setup.sh
  • scripts/variations/ci+setup.sh

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Global tool installation commands now allow lifecycle scripts for Bun, Deno, and Nx. The Bun and Deno variation commands allow scripts for their respective packages.

Changes

Install script permissions

Layer / File(s) Summary
Global install script permissions
scripts/setup.sh, scripts/variations/ci+setup.sh
The setup command allows scripts for Bun, Deno, and Nx. The variation commands allow scripts for Bun and Deno, respectively.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested reviewers: jdx

Merge Risk: ⚪ Minimal · up to dc4f7

The selected install scripts are permitted while benchmark fixture installs remain script-disabled; no actionable merge-blocking risk is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to dc4f7

The change requests script permission for named tools rather than enabling all installation scripts. Fixture-script restrictions remain intact. Installer execution occurs within an existing third-party tool trust boundary, but exact npm enforcement and interrupted-install recovery are not fully established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — CI setup installers inherit the benchmark process environment, including any populated VLT, Cloudsmith, GitHub, and JFrog registry credentials injected into that step. The launcher adds environment values without clearing inherited values. Credential population and downstream permissions are unknown. Access to these credentials by benchmark tool execution predates this PR.

Security Findings and Attack Paths

  • inferred — Compromise of an allowed upstream package could make its installer execute malicious code with ambient runner authority where npm honors the permission. This adds an execution phase, but the same benchmark already executes third-party tool binaries and other authorized installers with inherited credentials. No compromised package, additional privilege entitlement, or materially larger attack scope was established.

Trust Boundaries and Controls

  • observed — The commands request package-name-specific permission rather than unrestricted lifecycle execution. Fixed names limit direct package-identity input, but the arguments do not demonstrate process sandboxing or credential isolation. Exact npm version support and direct-versus-transitive identity matching remain unverified.

Resilience and Maintainability Implications

  • observed — Measured installation is fail-fast, but preparation uses semicolon-separated operations, tolerates fixture-install errors, and tolerates global-uninstall errors. These pre-existing transitions provide normal cleanup attempts rather than proof that every failure or cancellation leaves a clean global prefix. The new arguments do not change their ordering or error handling.

Hardening Proposals

  • proposed — Restrict registry credentials to the variations that need them and consider a credential-free environment for tool provisioning. This would reduce the existing third-party execution exposure; it is not evidence of a newly introduced credential leak.
  • proposed — Establish a supported bootstrap npm version and document its lifecycle-permission semantics. This would make the intended authorization boundary reproducible without requiring the benchmarked tool versions themselves to stop tracking latest releases.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding npm v11 --allow-scripts permissions for Bun and Deno installations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@evert
evert enabled auto-merge (squash) September 30, 2026 17:50
@evert
evert disabled auto-merge September 30, 2026 17:52
@vltbaudbot
vltbaudbot merged commit 928880f into main Sep 30, 2026
36 of 58 checks passed
@vltbaudbot
vltbaudbot deleted the evert/fix-bun-ci-setup branch September 30, 2026 17:52

This branch was previously deployed

1 inactive deployment
Benchmark — dc4f71c3 Deployed Sep 30, 2026 by vltbaudbot via Run Benchmarks (run, run) #1322
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants