Final-review fixes: stale env toggles, zip-slip, Windows exe, test env

.github/workflows/ci.yml

@@ -107,7 +107,7 @@ jobs:
       # silently desynchronize this leg.
       - name: Publish the plugin locally
         run: |
-          V=$(grep -o 'mill-bun_mill1:[0-9][0-9.]*' example-typescript/build.mill | cut -d: -f2)
+          V=$(grep -o 'mill-bun_mill1:[0-9][-0-9A-Za-z.]*' example-typescript/build.mill | cut -d: -f2)
           PUBLISH_VERSION=$V ./mill --no-server millbun.publishLocal
 
       - name: example-typescript

README.md

@@ -22,6 +22,7 @@ Bun does not need to be installed. The plugin downloads and checksum-verifies Bu
 package build
 
 import mill.*
+import mill.scalalib.*
 import mill.bun.bun
 import mill.scalajslib.api.*
 import mill.scalajslib.bun.*
@@ -34,7 +35,10 @@ object app extends BunScalaJSModule {
   override def moduleKind = Task { ModuleKind.ESModule }
   override def npmDeps = Task { Seq(bun"lodash@^4.17.21") }
 
-  object test extends BunScalaJSTests, TestModule.Utest
+  object test extends BunScalaJSTests, TestModule.Utest {
+    def mvnDeps = Seq(mvn"com.lihaoyi::utest::0.8.5")
+    def testFramework = "utest.runner.Framework"
+  }
 }
 ```
 
@@ -148,6 +152,8 @@ Set `bunArchiveUrl` as well to download from a mirror; a mirror always requires
 
 ```scala
 import mill.bun.*
+import mill.javascriptlib.bun.*
+import mill.scalajslib.bun.*
 
 object scalaApp extends BunScalaJSModule {
   def scalaVersion = "3.8.2"

docs/MIGRATING-0.3.md

@@ -42,6 +42,16 @@ override def bunBundleFormat = Task { "esm" }
 override def bunBundleFormat = Task { Some("esm") }
 ```
 
+### Contradictory npm declarations fail every install, not just publishing
+
+0.2 resolved the same package declared with different specifiers last-wins. 0.3 fails
+deterministically for **all** generated package.json files — direct declarations, module-graph
+aggregation, and classpath manifests alike. Resolve with `npmOverrides`:
+
+```scala
+override def npmOverrides = Task { Map("react" -> "^19.0.0") }
+```
+
 ### Local packages via `unmanagedDeps` must be directories with a `package.json`
 
 Entries are staged into `vendor/` beside the generated package.json and declared as

docs/RELEASING.md

@@ -5,7 +5,7 @@ This document describes how to publish `mill-bun-plugin` to Maven Central and cr
 ## Overview
 
 - Releases are triggered by pushing a semver tag like `v0.1.0` or `v0.1.0-RC1`.
-- The release workflow injects `PUBLISH_VERSION` from the tag, then runs `millbun.compile`, `millbun.test`, `millbun.integration`, and the Maven Central publish step.
+- The release workflow injects `PUBLISH_VERSION` from the tag for the compile, unit test, and publish steps. The integration step deliberately runs **without** `PUBLISH_VERSION`: fixtures pin `0.0.0-NIGHTLY` against `publishLocalTestRepo`, and injecting the release version there would republish the test repo at the wrong version and break every fixture resolution. Do not "fix" that inconsistency.
 - If the tag is annotated, the tag message is used as the GitHub release body. Otherwise GitHub generates release notes automatically.
 
 ## Prerequisites

millbun/integration/resources/mixed-workspace/build.mill

@@ -12,6 +12,9 @@ import mill.scalajslib.api.*
 import mill.scalajslib.bun.*
 
 object scalaApp extends BunScalaJSModule {
+  // Renamed deliberately: the layout's directory naming, duplicate guard, AND the generated
+  // member manifest must all follow this name, or bun sees a different identity than Mill.
+  override def bunWorkspacePackageName = Task { "scala-app-renamed" }
   def scalaVersion = "3.8.2"
   def scalaJSVersion = "1.22.0"
   override def moduleKind = Task { ModuleKind.ESModule }

millbun/integration/resources/typescript-tests/build.mill

@@ -11,5 +11,9 @@ import mill.javascriptlib.bun.*
 object app extends BunTypeScriptModule {
   override def moduleDir = build.moduleDir
 
-  object test extends BunTypeScriptTests
+  object test extends BunTypeScriptTests {
+    // The test-side env lever: overriding forkEnv here compiles but is ignored (only upstream's
+    // Node runners read it), and outer bunRuntimeEnv would also change `run`.
+    override def bunTestEnv = Task { super.bunTestEnv() + ("BUN_TEST_MARKER" -> "set") }
+  }
 }

millbun/integration/resources/typescript-tests/test/env.test.ts

@@ -0,0 +1,5 @@
+import { expect, test } from "bun:test";
+
+test("bunTestEnv reaches the bun test process", () => {
+  expect(process.env.BUN_TEST_MARKER).toBe("set");
+});

millbun/integration/src/mill/bun/BunScalaJSIntegrationTests.scala

@@ -88,6 +88,9 @@ object BunScalaJSIntegrationTests extends BunIntegrationSuite {
       assert(res.isSuccess)
 
       val executable = outputPath(tester, "app.compileExecutable")
+      // The recorded PathRef must name the file bun actually wrote — on Windows bun appends
+      // .exe, and CreateProcess would happily run an extensionless path that does not exist.
+      assert(os.isFile(executable))
       assert(runExecutable(executable) == "Hello from scala.js with lodash on bun")
     }
 

millbun/integration/src/mill/bun/BunTypeScriptIntegrationTests.scala

@@ -55,6 +55,7 @@ object BunTypeScriptIntegrationTests extends BunIntegrationSuite {
       assert(res.isSuccess)
 
       val executable = outputPath(tester, "app.compileExecutable")
+      assert(os.isFile(executable))
       val run = os.call(
         Seq(executable.toString),
         cwd = executable / os.up
@@ -94,6 +95,26 @@ object BunTypeScriptIntegrationTests extends BunIntegrationSuite {
       assert(args.contains("--frozen-lockfile"))
     }
 
+    test("lockfile requirement toggles between runs on a warm out directory") {
+      // bunRequireLockfile reads MILL_BUN_REQUIRE_LOCKFILE inside Task.Input: a plain Task
+      // caches the first-seen value, so the documented mid-migration escape hatch
+      // (MILL_BUN_REQUIRE_LOCKFILE=false) was silently ignored after one strict run.
+      // Forked evals, because only a subprocess sees a per-eval env.
+      val tester = new IntegrationTester(
+        daemonMode = false,
+        workspaceSourcePath = resourceDir / "typescript-simple",
+        millExecutable = millExe,
+        useInMemory = false
+      )
+      // typescript-simple ships a committed lock; remove it from the copy so the strict default
+      // has something to reject. (typescript-lock is unsuitable here: it pins the requirement.)
+      os.remove(tester.workspacePath / "bun.lock")
+      val strict = tester.eval("app.bunInstall", env = Map("MILL_BUN_REQUIRE_LOCKFILE" -> "true"))
+      assert(!strict.isSuccess)
+      val relaxed = tester.eval("app.bunInstall", env = Map("MILL_BUN_REQUIRE_LOCKFILE" -> "false"))
+      assert(relaxed.isSuccess)
+    }
+
     test("a lockfile from a newer Bun fails with regeneration guidance") {
       // bun.lock is forward- but not backward-compatible; bun's own failure is a raw
       // UnknownLockfileVersion that never mentions bunLock. The fixture's stub proves the

millbun/integration/src/mill/bun/BunWorkspaceIntegrationTests.scala

@@ -19,7 +19,7 @@ object BunWorkspaceIntegrationTests extends BunIntegrationSuite:
       assert(os.read(workspaceInstall / ".workspace-installed").contains("--frozen-lockfile"))
       val rootJson = ujson.read(os.read(workspaceInstall / "package.json"))
       assert(rootJson("workspaces").arr.map(_.str).toSet == Set(
-        "packages/scalaApp",
+        "packages/scala-app-renamed",
         "packages/typescriptApp"
       ))
       assert(os.exists(workspaceInstall / "node_modules" / "is-even" / "package.json"))
@@ -28,9 +28,12 @@ object BunWorkspaceIntegrationTests extends BunIntegrationSuite:
       // Unmanaged local packages arrive as file: specifiers with vendor trees staged beside the
       // member's package.json — never as positional install args, which turn `bun install` into
       // `bun add` and are unconditionally rejected by --frozen-lockfile.
-      val scalaJson = ujson.read(os.read(workspaceInstall / "packages" / "scalaApp" / "package.json"))
+      val scalaJson = ujson.read(os.read(workspaceInstall / "packages" / "scala-app-renamed" / "package.json"))
+      // The manifest identity must be the workspace package name, not the module's default —
+      // otherwise a rename satisfies Mill's duplicate guard while bun still sees collisions.
+      assert(scalaJson("name").str == "scala-app-renamed")
       assert(scalaJson("dependencies").obj("shared-local").str == "file:./vendor/shared-local")
-      assert(os.exists(workspaceInstall / "packages" / "scalaApp" / "vendor" / "shared-local" / "package.json"))
+      assert(os.exists(workspaceInstall / "packages" / "scala-app-renamed" / "vendor" / "shared-local" / "package.json"))
       assert(!os.read(workspaceInstall / ".workspace-installed").contains("shared-local"))
 
       val scalaResult = tester.eval("scalaApp.bunInstall")

millbun/src/mill/bun/BunToolchainModule.scala

@@ -515,7 +515,9 @@ trait BunToolchainModule extends Module {
   def bunUseMusl: T[Boolean] = Task.Input { BunToolchainModule.detectMusl() }
 
   /** Resolve Bun from PATH instead of using the managed distribution. */
-  def bunUseSystem: T[Boolean] = Task {
+  // Task.Input: a plain Task caches the first-seen env value until a manual clean, so toggling
+  // MILL_BUN_USE_SYSTEM between runs — the documented workflow — would be silently ignored.
+  def bunUseSystem: T[Boolean] = Task.Input {
     Task.env.get("MILL_BUN_USE_SYSTEM").exists(_.equalsIgnoreCase("true"))
   }
 
@@ -578,7 +580,9 @@ trait BunToolchainModule extends Module {
   }
 
   /** Require dependency-bearing modules to provide [[bunLockfile]]. */
-  def bunRequireLockfile: T[Boolean] = Task {
+  // Task.Input for the same reason as bunUseSystem: MIGRATING-0.3 tells users mid-migration to
+  // set MILL_BUN_REQUIRE_LOCKFILE=false, which must take effect on a warm out/ directory.
+  def bunRequireLockfile: T[Boolean] = Task.Input {
     !Task.env.get("MILL_BUN_REQUIRE_LOCKFILE").exists(_.equalsIgnoreCase("false"))
   }
 

millbun/src/mill/bun/BunVendoredNodeModules.scala

@@ -84,9 +84,16 @@ object BunVendoredNodeModules:
       if entries.isEmpty then return false
 
       entries.sortBy(_.getName).foreach { entry =>
-        val relString = entry.getName.stripPrefix(prefix)
+        // Normalize the separators some zip writers use, then refuse entries that climb out of
+        // the bundle root: a hostile jar with `META-INF/bun/node_modules/../../x` must never
+        // become a write outside the destination (zip-slip).
+        val relString = entry.getName.replace('\\', '/').stripPrefix(prefix)
         if relString.nonEmpty then
           val rel = os.RelPath(relString)
+          if rel.ups != 0 then
+            throw new RuntimeException(
+              s"Vendored Bun bundle entry escapes its root in $jarPath: ${entry.getName}"
+            )
           if !shouldSkip(rel.toNIO) then
             val dest = destNodeModules / rel
             val sourceLabel = s"$jarPath!/${entry.getName}"

millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala

@@ -92,7 +92,7 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with
 
     val resolved = ujson.Obj.from(
       user.copy(
-        name = if (user.name.nonEmpty) user.name else moduleName,
+        name = if (user.name.nonEmpty) user.name else bunWorkspacePackageName(),
         version = if (user.version.nonEmpty) user.version else "1.0.0",
         `type` = if (enableEsm()) "module" else user.`type`,
         dependencies = ujson.Obj.from(BunToolchainModule.dependencyPairsWithUnmanaged(
@@ -329,7 +329,10 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with
     val compileDir = compile().path
     val buildDir = Task.dest / "workspace"
     val mainFile = resolvedEntrypoint(mainFilePath(), compileDir).relativeTo(compileDir).toString
-    val outFile = Task.dest / bunBinaryName()
+    // bun appends .exe to extensionless --compile outputs on Windows; the recorded PathRef
+    // must name the file bun actually writes, or downstream copies fail and caching never
+    // invalidates (a missing path's signature is constant).
+    val outFile = Task.dest / (bunBinaryName() + (if (scala.util.Properties.isWin) ".exe" else ""))
 
     // Declared explicitly: the staged tree carries a node_modules symlink into this
     // install, and Mill's filesystem checker only permits reading a dest we depend on.
@@ -430,6 +433,16 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with
      */
     override def tsDeps: T[Seq[String]] = Task { outer.tsDeps() }
 
+    /**
+     * Runtime environment for `bun test` processes, overridable per test module.
+     *
+     * `override def forkEnv` on a test object compiles but is silently ignored here (only
+     * upstream's Node-based runners read it), and `bunRuntimeEnv` lives on the outer module
+     * where overriding it also changes `run`. This is the test-side lever, mirroring the
+     * Scala.js `bunTestJsEnv`.
+     */
+    def bunTestEnv: T[Map[String, String]] = Task { outer.bunRuntimeEnv() }
+
     /** Test timeout in milliseconds. 0 means no timeout. */
     def bunTestTimeout: T[Int] = Task { 0 }
 
@@ -615,7 +628,7 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with
       os.call(
         Seq(bunExecutable(), "test") ++ resolvedTestFlags() ++ args.value,
         cwd = preparedTestWorkspace().path,
-        env = outer.bunRuntimeEnv(),
+        env = bunTestEnv(),
         stdout = os.Inherit,
         stderr = os.Inherit
       )
@@ -626,7 +639,7 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with
       os.call(
         Seq(bunExecutable(), "test") ++ resolvedTestFlags() ++ args.value,
         cwd = preparedTestWorkspace().path,
-        env = outer.bunRuntimeEnv(),
+        env = bunTestEnv(),
         stdout = os.Inherit,
         stderr = os.Inherit
       )
@@ -637,7 +650,7 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with
       os.call(
         Seq(bunExecutable(), "test", "--watch") ++ resolvedTestFlags() ++ args.value,
         cwd = preparedTestWorkspace().path,
-        env = outer.bunRuntimeEnv(),
+        env = bunTestEnv(),
         stdout = os.Inherit,
         stderr = os.Inherit
       )
@@ -648,7 +661,7 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with
       os.call(
         Seq(bunExecutable(), "test", "--update-snapshots") ++ resolvedTestFlags() ++ args.value,
         cwd = preparedTestWorkspace().path,
-        env = outer.bunRuntimeEnv(),
+        env = bunTestEnv(),
         stdout = os.Inherit,
         stderr = os.Inherit
       )
@@ -669,7 +682,7 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with
           coverageDir.toString
         ) ++ coverageReporterArgs ++ resolvedTestFlags() ++ args.value,
         cwd = preparedTestWorkspace().path,
-        env = outer.bunRuntimeEnv(),
+        env = bunTestEnv(),
         stdout = os.Inherit,
         stderr = os.Inherit
       )
@@ -691,7 +704,7 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with
           coverageDir.toString
         ) ++ coverageReporterArgs ++ resolvedTestFlags(),
         cwd = preparedTestWorkspace().path,
-        env = outer.bunRuntimeEnv()
+        env = bunTestEnv()
       )
 
       PathRef(coverageDir)

millbun/src/mill/scalajslib/bun/BunPublishModule.scala

@@ -20,6 +20,11 @@ import mill.scalajslib.api.ModuleKind
   */
 trait BunPublishModule extends BunScalaJSModule {
 
+  // Declared as a source: a plain read of workspaceRoot/.npmrc trips Mill's filesystem checker
+  // the moment the file exists (exactly the private-registry case vendoring needs), and an
+  // undeclared read would never invalidate this task when the file changes.
+  private def publishNpmRc = Task.Source(BuildCtx.workspaceRoot / ".npmrc")
+
   /** Embed resolved `node_modules` into published artifacts.
     *
     * Disabled by default because published JARs are cross-platform, while
@@ -56,7 +61,7 @@ trait BunPublishModule extends BunScalaJSModule {
     val dest = Task.dest
     os.makeDir.all(dest)
 
-    val npmRc = BuildCtx.workspaceRoot / ".npmrc"
+    val npmRc = publishNpmRc().path
     if (os.exists(npmRc)) os.copy.over(npmRc, dest / ".npmrc", createFolders = true)
     bunfigFiles().foreach { cfg =>
       os.copy.over(cfg.path, dest / cfg.path.last, createFolders = true)

millbun/src/mill/scalajslib/bun/BunScalaJSModule.scala

@@ -203,7 +203,10 @@ trait BunScalaJSModule extends ScalaJSModule with BunToolchainModule with BunPac
     val allOptional = BunToolchainModule.dependencyPairs(transitiveNpmOptionalDeps(), overrides)
     val allPeers = BunToolchainModule.dependencyPairs(transitiveNpmPeerDeps(), overrides)
     val base = ujson.Obj(
-      "name" -> defaultPackageName,
+      // bunWorkspacePackageName, not the raw default: the workspace layout names directories
+      // and detects duplicates by it, so the manifest must carry the same identity or an
+      // override satisfies the guard while bun still sees the colliding default names.
+      "name" -> bunWorkspacePackageName(),
       "private" -> true,
       "version" -> "0.0.0",
       "dependencies" -> ujson.Obj.from(BunToolchainModule.dependencyPairsWithUnmanaged(
@@ -486,7 +489,10 @@ trait BunScalaJSModule extends ScalaJSModule with BunToolchainModule with BunPac
     resolvedBunConfigs().foreach(cfg => os.copy.over(cfg.path, buildDir / cfg.path.last, createFolders = true))
     copyCompileResources(bunCompileResources(), buildDir)
 
-    val outFile = Task.dest / bunBinaryName()
+    // bun appends .exe to extensionless --compile outputs on Windows; the recorded PathRef
+    // must name the file bun actually writes, or downstream copies fail and caching never
+    // invalidates (a missing path's signature is constant).
+    val outFile = Task.dest / (bunBinaryName() + (if (scala.util.Properties.isWin) ".exe" else ""))
     val entry = primaryEntrypoint(linked).relativeTo(linked.dest.path).toString
     val formatArgs = bunBundleFormat().toSeq.flatMap(fmt => Seq("--format", fmt))
     val sourcemapArgs = bunBundleSourcemap().toSeq.map(mode => s"--sourcemap=$mode")

millbun/test/src/mill/bun/BunVendoredNodeModulesTests.scala

@@ -101,6 +101,26 @@ object BunVendoredNodeModulesTests extends TestSuite {
       assert(err.getMessage.contains("Vendored Bun bundle conflict"))
     }
 
+    test("jar entries that climb out of the bundle root are refused") {
+      // Zip-slip: a hostile dependency jar carrying the vendored marker plus a `..` entry must
+      // never become a file write outside the destination — this runs during bunInstall for any
+      // module whose classpath contains the jar, without executing any dependency code.
+      val jarPath = tempJar(
+        Map(
+          s"${BunVendoredNodeModules.BundleRoot}/react/package.json" -> """{"name":"react"}""",
+          s"${BunVendoredNodeModules.BundleRoot}/../../../escaped.txt" -> "outside"
+        )
+      )
+      val destRoot = os.temp.dir()
+
+      val err = intercept[RuntimeException] {
+        BunVendoredNodeModules.mergeFromClasspathEntry(jarPath, destRoot / "node_modules")
+      }
+      assert(err.getMessage.contains("escapes its root"))
+      assert(!os.walk(destRoot).exists(_.last == "escaped.txt"))
+      assert(!os.exists(destRoot / os.up / "escaped.txt"))
+    }
+
     test("merging through a symlinked destination is refused") {
       // In workspace mode node_modules links into the shared install's Task.dest. Merging
       // through it would silently mutate a directory every workspace member depends on.
@@ -130,10 +150,11 @@ object BunVendoredNodeModulesTests extends TestSuite {
   private def tempJar(entries: Map[String, String]): os.Path = {
     val jarPath = os.temp.dir() / "bundle.jar"
     val jarOut = new JarOutputStream(new FileOutputStream(jarPath.toIO))
+    val written = scala.collection.mutable.Set.empty[String]
     try
       entries.toSeq.sortBy(_._1).foreach { case (path, content) =>
         val parentDirs = parentDirectories(path)
-        parentDirs.foreach { dir =>
+        parentDirs.filter(written.add).foreach { dir =>
           jarOut.putNextEntry(new JarEntry(dir))
           jarOut.closeEntry()
         }