From 6cebce8b56fcf48a27a04323c56647ce8890dbc9 Mon Sep 17 00:00:00 2001 From: Richie Caputo Date: Thu, 27 Aug 2026 11:15:13 -0400 Subject: [PATCH] Final-review fixes: stale env toggles, zip-slip, Windows exe, test env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bunUseSystem and bunRequireLockfile read Task.env inside plain cached Tasks, so the documented workflows — toggling MILL_BUN_USE_SYSTEM, or MIGRATING-0.3's mid-migration MILL_BUN_REQUIRE_LOCKFILE=false — were silently ignored on a warm out/ until a manual clean. Both are Task.Input now, with a forked-eval integration test that toggles the requirement between runs on one warm out/ tree (confirmed failing with plain Task). mergeFromJar accepted `..` segments from jar entries: a dependency jar carrying the vendored marker could write arbitrary files during bunInstall without executing any code (zip-slip). Entries are backslash-normalized and refused when they climb out of the bundle root; a unit test builds a hostile jar and asserts refusal plus zero leakage. compileExecutable on Windows recorded a PathRef without the .exe bun actually appends — CreateProcess masked it in CI while downstream copies would fail and a missing path's constant signature broke invalidation. Both module kinds record the .exe name on Windows, and both integration tests now assert the recorded file exists. The TypeScript test trait gains bunTestEnv (default: outer bunRuntimeEnv), the test-side env lever mirroring bunTestJsEnv — overriding forkEnv on a test object compiles and does nothing. All six test-execution commands use it; the typescript-tests fixture asserts the variable reaches bun test. Generated member manifests now carry bunWorkspacePackageName (user packageJson.name still wins on TypeScript): overriding the name — the documented fix for colliding module names — previously satisfied Mill's duplicate guard while bun still saw the colliding defaults. The mixed-workspace fixture renames a member and asserts directory, manifest name, and vendor staging all follow. bunPublishedRuntimeInstall declared its .npmrc read as a Task.Source; the raw workspace read tripped Mill's filesystem checker the moment the file existed and never invalidated on change. Docs and CI: the README Scala.js quick start compiles now (missing scalalib import and utest dependency); the workspace snippet's imports are complete; the examples-CI version grep survives prerelease pins; RELEASING.md records why the release integration step deliberately omits PUBLISH_VERSION; MIGRATING-0.3 documents that deterministic conflict failure applies to every install, with npmOverrides as the escape hatch. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 2 +- README.md | 8 ++++- docs/MIGRATING-0.3.md | 10 +++++++ docs/RELEASING.md | 2 +- .../resources/mixed-workspace/build.mill | 3 ++ .../resources/typescript-tests/build.mill | 6 +++- .../typescript-tests/test/env.test.ts | 5 ++++ .../mill/bun/BunScalaJSIntegrationTests.scala | 3 ++ .../bun/BunTypeScriptIntegrationTests.scala | 21 ++++++++++++++ .../bun/BunWorkspaceIntegrationTests.scala | 9 ++++-- millbun/src/mill/bun/BunToolchainModule.scala | 8 +++-- .../src/mill/bun/BunVendoredNodeModules.scala | 9 +++++- .../bun/BunTypeScriptModule.scala | 29 ++++++++++++++----- .../scalajslib/bun/BunPublishModule.scala | 7 ++++- .../scalajslib/bun/BunScalaJSModule.scala | 10 +++++-- .../bun/BunVendoredNodeModulesTests.scala | 23 ++++++++++++++- 16 files changed, 133 insertions(+), 22 deletions(-) create mode 100644 millbun/integration/resources/typescript-tests/test/env.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1ef38bb..950aa79 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/README.md b/README.md index 99e2b46..f2ddb4e 100644 --- a/README.md +++ b/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" diff --git a/docs/MIGRATING-0.3.md b/docs/MIGRATING-0.3.md index 6dcd494..b6cb44d 100644 --- a/docs/MIGRATING-0.3.md +++ b/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 diff --git a/docs/RELEASING.md b/docs/RELEASING.md index 4d47403..cbd88c0 100644 --- a/docs/RELEASING.md +++ b/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 diff --git a/millbun/integration/resources/mixed-workspace/build.mill b/millbun/integration/resources/mixed-workspace/build.mill index d158b78..d85e2e4 100644 --- a/millbun/integration/resources/mixed-workspace/build.mill +++ b/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 } diff --git a/millbun/integration/resources/typescript-tests/build.mill b/millbun/integration/resources/typescript-tests/build.mill index 3504a2a..3e3731f 100644 --- a/millbun/integration/resources/typescript-tests/build.mill +++ b/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") } + } } diff --git a/millbun/integration/resources/typescript-tests/test/env.test.ts b/millbun/integration/resources/typescript-tests/test/env.test.ts new file mode 100644 index 0000000..09468b6 --- /dev/null +++ b/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"); +}); diff --git a/millbun/integration/src/mill/bun/BunScalaJSIntegrationTests.scala b/millbun/integration/src/mill/bun/BunScalaJSIntegrationTests.scala index 9219895..1335e37 100644 --- a/millbun/integration/src/mill/bun/BunScalaJSIntegrationTests.scala +++ b/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") } diff --git a/millbun/integration/src/mill/bun/BunTypeScriptIntegrationTests.scala b/millbun/integration/src/mill/bun/BunTypeScriptIntegrationTests.scala index 269aafc..a06b2ad 100644 --- a/millbun/integration/src/mill/bun/BunTypeScriptIntegrationTests.scala +++ b/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 diff --git a/millbun/integration/src/mill/bun/BunWorkspaceIntegrationTests.scala b/millbun/integration/src/mill/bun/BunWorkspaceIntegrationTests.scala index 1bb1d3e..023cce6 100644 --- a/millbun/integration/src/mill/bun/BunWorkspaceIntegrationTests.scala +++ b/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") diff --git a/millbun/src/mill/bun/BunToolchainModule.scala b/millbun/src/mill/bun/BunToolchainModule.scala index 83491bd..6728a70 100644 --- a/millbun/src/mill/bun/BunToolchainModule.scala +++ b/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")) } diff --git a/millbun/src/mill/bun/BunVendoredNodeModules.scala b/millbun/src/mill/bun/BunVendoredNodeModules.scala index 23050c0..061008e 100644 --- a/millbun/src/mill/bun/BunVendoredNodeModules.scala +++ b/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}" diff --git a/millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala b/millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala index caed8b2..2e33a8e 100644 --- a/millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala +++ b/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) diff --git a/millbun/src/mill/scalajslib/bun/BunPublishModule.scala b/millbun/src/mill/scalajslib/bun/BunPublishModule.scala index 8956b06..c290066 100644 --- a/millbun/src/mill/scalajslib/bun/BunPublishModule.scala +++ b/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) diff --git a/millbun/src/mill/scalajslib/bun/BunScalaJSModule.scala b/millbun/src/mill/scalajslib/bun/BunScalaJSModule.scala index 39e7642..0c03165 100644 --- a/millbun/src/mill/scalajslib/bun/BunScalaJSModule.scala +++ b/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") diff --git a/millbun/test/src/mill/bun/BunVendoredNodeModulesTests.scala b/millbun/test/src/mill/bun/BunVendoredNodeModulesTests.scala index ac34238..949afd3 100644 --- a/millbun/test/src/mill/bun/BunVendoredNodeModulesTests.scala +++ b/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() }