allowScripts: root-listed bundleDependencies packages hidden from install-scripts ls — inBundle vs inDepBundle?

[vbjay]

Trying to understand inBundle vs inDepBundle in the allowScripts policy — am I missing something?

Hi! I've been reading through the allowScripts / install-scripts code and
I'm trying to understand the difference between node.inBundle and
node.inDepBundle, and whether the right one is being used in a couple of
places. I might be missing something obvious here, so I wanted to ask before
assuming it's a bug.

What I think the two properties mean

Looking at workspaces/arborist/lib/node.js:

get inBundle () {
  return !!this.getBundler()  // bundled by ANYONE — root or a dep
}

// "when reifying, if a package is technically in a bundleDependencies list,
//  but that list is the root project, we still have to install it."
get inDepBundle () {
  const bundler = this.getBundler()
  return !!bundler && bundler !== this.root  // bundled by a DEP, not root
}

So my understanding is:

Case A — a published dependency bundles a transitive dep in its tarball

// node_modules/some-lib/package.json
{ "bundleDependencies": ["some-internal"] }

some-internal came packed inside some-lib's tarball. npm never fetches it
from the registry. Its scripts already ran on the publisher's machine at publish
time — not on the consumer's machine.

inBundle    = true
inDepBundle = true  (bundler is some-lib, not root)

→ Makes sense to exclude from the allowScripts review. Nothing to review.

---

Case B — the root project lists a dep in bundleDependencies for publishing

// my project's package.json
{
  "dependencies":       { "canvas": "^3.0.0" },
  "bundleDependencies": ["canvas"]
}

I'm listing canvas in bundleDependencies so npm pack bundles it into my
published tarball (e.g. for distributing pre-compiled binaries). But during a
normal npm install in development, canvas **is still fetched from the
registry** and its native build scripts do run at install time.

inBundle    = true   (root listed it in bundleDependencies)
inDepBundle = false  (the bundler IS the root, not a dep)

→ This is where I'm confused — shouldn't this be included in the review?

What I'm seeing

isScriptAllowed in script-allowed.js and collectUnreviewedScripts in
unreviewed-scripts.js both use node.inBundle to gate out bundled packages:

if (node.inBundle) {
  return null   // excluded from allowScripts policy entirely
}

In Case B above, canvas.inBundle is true, so it gets excluded.

Reproduction

I added a TAP smoke test (smoke-tests/test/install-scripts-root-bundle.js)
0001-Bundle-question-smoke-test.patch
using
the existing smoke-test harness. It has three subtests covering each scenario
with pre-populated node_modules fixtures — no registry calls needed.

Running the test on the current latest branch 86416a626e4599791c5d8115ed08aa4369774844:

no-bundle: normal dep listed and approveable
  √ ls lists dep with install script
  √ approve succeeds
  √ allowScripts entry written after approve

root-bundle: root-bundled dep listed and approveable (inDepBundle fix)
  1) ls lists root-bundled dep (inDepBundle=false → scripts run → must be reviewable)
  2) approve failed — upstream inBundle bug (ENOMATCH): command failed

dep-bundle: dep-bundled package correctly hidden from ls
  √ ls correctly hides dep-bundled package (inDepBundle=true → scripts ran at publish)
  √ approve correctly rejects dep-bundled package with ENOMATCH

5 passing
2 failing

The root-bundle subtest fails because install-scripts ls returns
No packages with unreviewed install scripts. even though has-script's
install script runs during npm install. The dep-bundle subtest passes —
that case is correctly hidden because its scripts ran at publish time.

The problem

Cases B and C produce identical output, but their security posture is
completely different:

	inBundle	inDepBundle	Scripts run at install?	Visible in ls?
Normal dep	false	false	yes	✅ yes
Root bundle	true	false	yes	❌ no — bug?
Dep bundle	true	true	no	✅ no

If the intent of the inBundle check is to exclude packages whose scripts
already ran at publish time (Case C only), then inDepBundle seems like the
right guard — it is already false for root-bundled packages. The inDepBundle
comment in node.js already notes this distinction:

"if a package is technically in a bundleDependencies list, but that list is
the root project, we still have to install it."

Questions

1. Is there a reason inBundle (rather than inDepBundle) is used here? Is
   there a scenario I'm not thinking of where a root-bundled package's scripts
   genuinely don't run at install time?

2. If the intent is only to exclude packages pre-built inside a dep's tarball,
   would inDepBundle be the right check?

3. Same question for the candidate list in runPrune in allow-scripts-cmd.js
   — it also uses inBundle to skip nodes, which I think would prevent stale
   allowScripts entries for root-bundled packages from ever being pruned.

Happy to be wrong here — I just wanted to make sure I wasn't misreading the
intent before assuming it's a gap.

References

• inDepBundle defined: workspaces/arborist/lib/node.js lines 573–579
• isScriptAllowed guard: workspaces/arborist/lib/script-allowed.js
• collectUnreviewedScripts: workspaces/arborist/lib/unreviewed-scripts.js
• Related PRs: feat: namespace install-script approval commands under npm install-scripts npm/cli#9629 (install-scripts namespace), fix(allowScripts): close three enforcement gaps npm/cli#9652 (enforcement gaps)
• issue created [BUG] InBundle vs InDepBundle in approve scripts functionality npm/cli#9679

[rdpsawada]

Your reasoning seems sound to me. The distinction appears to be that inBundle means “bundled by anyone” while inDepBundle means “bundled by a dependency, not the root.”

For allowScripts, the behavior that seems security-relevant is whether install scripts execute on the consumer machine. In the root bundleDependencies case, the package is still installed/reified locally, so its install scripts can still run. That makes me wonder whether inDepBundle would be a better guard than inBundle for isScriptAllowed, collectUnreviewedScripts, and possibly runPrune.

Your smoke test is useful here because it demonstrates the behavioral difference rather than only arguing semantics. Curious whether the current behavior is intentional or just inherited from broader bundle handling.

[kartik99917]

Confirm the technical read is right (because [Likely] it is, based on the code excerpts and test output shown) — state plainly that inBundle conflates two cases with different security postures, and inDepBundle is the correct guard if the intent is "exclude packages whose scripts already ran at publish time."
Point to the exact fix locations he already found — script-allowed.js's isScriptAllowed and unreviewed-scripts.js's collectUnreviewedScripts, swapping the inBundle check for !inDepBundle (or inBundle && !inDepBundle if you want to keep excluding dep-bundled packages but stop excluding root-bundled ones).
Flag the third spot he found — runPrune in allow-scripts-cmd.js has the same inBundle check, which means root-bundled packages could get silently pruned from the allowScripts ledger too. That's worth calling out explicitly because it's easy for maintainers to fix the two obvious spots and miss this one.

Rest!!

[rehuux]

You've done excellent analysis here — this is definitely a bug.

You're exactly right about the distinction:

Case inBundle inDepBundle Scripts run? Current behavior
Normal dep false false ✅ Yes ✅ Visible
Root-bundled true false ✅ Yes ❌ Hidden (bug)
Dep-bundled true true ❌ No ✅ Hidden (correct)

The guard if (node.inBundle) return null is too broad — it treats root-bundled packages (where the root project lists something in bundleDependencies) the same as dependency-bundled packages (where a published tarball already ran scripts at publish time).

The fix should be inDepBundle instead of inBundle — your test patch confirms this. The comment in node.js even says root-bundled packages "still have to install it," so the current behavior contradicts that.

Some additional context:

· In a root-bundled scenario, npm pack includes the dep, but npm install still fetches it from the registry during development — scripts run.
· The inBundle check was likely a copy-paste oversight when the allowScripts feature was originally implemented.