Skip to content

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

Merged
arcaputo3 merged 1 commit into
agent/idiomatic-scalajs-mill-workflowsfrom
agent/bun-final-fixes
Aug 27, 2026
Merged

Final-review fixes: stale env toggles, zip-slip, Windows exe, test env#27
arcaputo3 merged 1 commit into
agent/idiomatic-scalajs-mill-workflowsfrom
agent/bun-final-fixes

Conversation

@arcaputo3

Copy link
Copy Markdown
Contributor

Final-review fixes for #13, from a four-reviewer adversarial pass over the complete
104-file diff against main. Targets agent/idiomatic-scalajs-mill-workflows.

Blocker

bunUseSystem and bunRequireLockfile read Task.env inside plain cached Task {} — the
first-seen value was served until a manual clean, so the documented workflows (toggling
MILL_BUN_USE_SYSTEM; MIGRATING-0.3's mid-migration MILL_BUN_REQUIRE_LOCKFILE=false) were
silently ignored on a warm out/. Both are Task.Input now — the same fix publishVersion and
bunDownloadCacheDir already had. New forked-eval integration test toggles the requirement
between runs on one warm out/ tree; confirmed failing with plain Task.

Security

Zip-slip in BunVendoredNodeModules.mergeFromJar. A dependency jar carrying the vendored
marker plus an entry like META-INF/bun/node_modules/../../../x produced an arbitrary file
write during bunInstall of any module with that jar on its classpath — no dependency code
execution required. Entries are now backslash-normalized and rejected when they climb out of
the bundle root. Unit test builds a hostile jar and asserts refusal plus zero leakage; confirmed
failing without the guard.

Majors

  • Windows compileExecutable recorded a PathRef to a file that doesn't exist (bun appends
    .exe; Win32 CreateProcess masked it by appending .exe at launch, so CI stayed green while
    downstream copies would fail and caching never invalidated). Both module kinds now record the
    .exe name on Windows, and both integration tests assert the recorded file exists.
  • TypeScript test modules had no env override pointoverride def forkEnv on a test
    object compiles and does nothing (only upstream's Node runners read it). New bunTestEnv on
    the test trait (default: outer bunRuntimeEnv), used by all six test-execution commands,
    mirroring the Scala.js bunTestJsEnv. The typescript-tests fixture overrides it and a bun
    test asserts the variable arrives.
  • bunWorkspacePackageName was ignored by generated member manifests — overriding it (the
    documented fix for colliding module names) satisfied Mill's duplicate guard while bun still
    saw the colliding defaults. Both module kinds now emit it as the manifest name (user
    packageJson.name still wins on TypeScript). The mixed-workspace fixture now renames a member
    and asserts directory, manifest name, and vendor staging all follow.
  • Undeclared workspace read in bunPublishedRuntimeInstall — the raw .npmrc read tripped
    Mill's filesystem checker the moment the file existed (exactly the private-registry case
    vendored publishing serves), and never invalidated on change. Declared as a Task.Source.
  • README's front-page Scala.js quick start did not compile (missing import mill.scalalib.*
    and the utest dependency); the workspace snippet was missing two imports.

Minors

  • Examples-CI version grep now survives prerelease pins (0.4.0-RC1).
  • RELEASING.md records that the release workflow's integration step deliberately omits
    PUBLISH_VERSION (fixtures pin 0.0.0-NIGHTLY) so nobody "fixes" it into breaking releases.
  • MIGRATING-0.3 documents that deterministic conflict failure applies to every install, not just
    publishing, with the npmOverrides escape hatch.

Reviewed and deliberately deferred (fast-follow candidates, not regressions)

System-mode Bun resolution caches PATH lookup and version verification until clean (needs a
design decision on re-probe cost); vendored publishing frozen-installs a reduced manifest
against the full lock (vendoring is opt-in and stub-covered — needs its own lock story); a
publish module's unmanagedDeps are invisible to local moduleDeps consumers; dev-server sync
thread resilience; error-message polish (raw ujson/HTTP/permission errors on exotic paths);
upstream runTypeCheck ignored; test-declared version of an outer package silently shadowed.

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 <noreply@anthropic.com>
@arcaputo3
arcaputo3 merged commit 6147ca7 into agent/idiomatic-scalajs-mill-workflows Aug 27, 2026
7 checks passed
@arcaputo3
arcaputo3 deleted the agent/bun-final-fixes branch August 27, 2026 15:44
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.

1 participant