Skip to content

Add Windows MSI installer built in CI#29475

Open
robobun wants to merge 15 commits into
mainfrom
farm/4e017c28/windows-msi-installer
Open

Add Windows MSI installer built in CI#29475
robobun wants to merge 15 commits into
mainfrom
farm/4e017c28/windows-msi-installer

Conversation

@robobun

@robobun robobun commented Apr 19, 2026

Copy link
Copy Markdown
Collaborator

Adds a single universal bun-windows.msi to release assets.

One installer carries all three Windows payloads (x64, x64-baseline, arm64) and picks the right one at install time — same semantics as install.ps1, one product identity, no "which MSI do I need?" decision for users or deployment tooling.

What gets installed

  • bun.exe + bunx.exe to %ProgramFiles%\Bun\bin (per-machine). bunx.exe is created at install time via DuplicateFile from whichever bun.exe variant was selected, so each variant contributes one payload not two.
  • Runtime variant selection via a small native CustomAction DLL (packages/bun-msi/detect-cpu.c, compiled at MSI-build time — no binary checked in): IsWow64Process2() for ARM64 vs AMD64, IsProcessorFeaturePresent(PF_AVX2) for x64 vs x64-baseline. Same Win32 calls install.ps1 P/Invokes. Defaults to x64-baseline so a detection failure never leaves an illegal-instruction crash.
  • %ProgramFiles%\Bun\bin appended to the system PATH.
  • System env var BUN_INSTALL set to the install root.
  • HKLM\Software\Oven\Bun with InstallRoot / BinDir / Version / Variant for fleet inventory.
  • ARP entry with the Bun icon, https://bun.com, docs, and release-notes links.
  • Launch condition blocks anything older than Windows 10 1809 / Server 2019 (build 17763), matching src/cli/install.ps1's $MinBuild.

Package platform is x64; ARM64 Windows 11 runs x64 MSIs under emulation and IsWow64Process2 reports the native machine from there, so ARM64 hosts get the native arm64 payload.

Enterprise / silent deploy

msiexec /i bun-windows.msi /qn
msiexec /i bun-windows.msi /qn INSTALLFOLDER=D:\Tools\Bun ADDTOPATH=0
msiexec /i bun-windows.msi /qn BUNVARIANT=x64-baseline
msiexec /x bun-windows.msi /qn

Public properties (Secure="yes" so they survive UAC): INSTALLFOLDER, BUNVARIANT (override detection), ADDTOPATH (default 1), SETBUNINSTALL (default 1). Single UpgradeCode → running a newer MSI replaces an older one in place; running the same version repairs. Zero script CustomActions — works on VBScript-disabled / PowerShell-restricted enterprise images.

Branding

WixUI_InstallDir (Welcome → License → Destination → Install). Dialog/banner bitmaps are generated at build time by build-msi.ps1 from src/bun.ico onto Bun's cream→blush gradient (#fbf0df#f6dece) with a soft drop shadow, so no binary BMPs are checked into git. License page renders LICENSE.md wrapped in a minimal RTF envelope with \uN? Unicode escaping. Exit dialog says hi. 🐰

CI

Built by the msi job in .github/workflows/release.yml on a windows-latest runner, after the sign job. Downloads bun-windows-{x64,x64-baseline,aarch64}.zip from the GitHub release, compiles detect-cpu.c with MSVC (located via vswhere), runs packages/bun-msi/build-msi.ps1, and uploads bun-windows.msi back to the release + as a workflow artifact.

Fires automatically on release: published and the daily canary schedule; manually dispatchable via the Release workflow's "Should the Windows MSI installer be built?" input for testing.

The embedded bun.exe binaries are already Authenticode-signed (they come from the release zips Buildkite signed). The MSI wrapper itself is not signed here because the DigiCert smctl secrets live in Buildkite; the job has a comment noting where to add smctl sign if those secrets are mirrored to GHA.

Tests

test/cli/install/msi.test.ts — 18 checks across the .wxs structure (three-payload ComponentGroup with mutually-exclusive BUNVARIANT conditions, DetectCpu CA sequencing, single UpgradeCode, PATH/BUN_INSTALL env, min-OS, no script CA), detect-cpu.c (exports DetectCpu, uses IsWow64Process2 + IsProcessorFeaturePresent(40), stays in sync with install.ps1), build-msi.ps1 (cl.exe compile, vswhere fallback, bitmap sizes, brand FromArgb bytes, RTF Unicode escaping, -d preprocessor vars all wired, -arch x64), and the release.yml job shape. Runs on every platform since it only reads text sources; the actual wix build + cl.exe happen on the Windows runner.

Towards #8116 — this is the artifact winget would reference; the manifest itself is a follow-up.

@robobun

robobun commented Apr 19, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 3:01 AM PT - Apr 19th, 2026

@robobun, your commit 33a4274 has 6 failures in Build #46463 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 29475

That installs a local version of the PR into your bun-29475 executable, so you can run:

bun-29475 --bun

@github-actions

Copy link
Copy Markdown
Contributor

Found 2 issues this PR may fix:

  1. Make installer/package for bun (SNAP, deb/apt, rpm/dnf) #862 - Requests native OS installers/packages for Bun; the Windows MSI directly addresses the Windows portion of this request
  2. add winget to install docs #8116 - Requests winget support; an official MSI built in CI is the standard artifact format that winget packages reference, enabling proper winget distribution

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #862
Fixes #8116

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Apr 19, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds Windows MSI packaging (WiX v5) with CI orchestration, PowerShell build/sign scripts, installer sources, docs, and tests; plus internal socket wrapper lifecycle refactors in Zig and minor socket class metadata cleanup in TypeScript.

Changes

Cohort / File(s) Summary
CI pipeline & release upload
.buildkite/ci.mjs, .buildkite/scripts/upload-release.sh, .buildkite/scripts/build-windows-msi.ps1
Introduces getWindowsMsiStep() and windows-msi orchestration, parses buildMsi commit token, wires MSI step into release (WINDOWS_MSI_STEP), prefers .msi artifacts from windows-msi, and adds a PowerShell CI script to build/sign/upload MSIs.
MSI packaging sources & build tooling
packages/bun-msi/bun.wxs, packages/bun-msi/build-msi.ps1, packages/bun-msi/.gitignore, packages/bun-msi/README.md
Adds WiX installer source, PowerShell MSI builder (renders bitmaps, generates RTF license, ensures WiX tooling, produces per-arch MSIs), .gitignore for outputs, and README describing installer behavior and CI usage.
Release upload script changes
.buildkite/scripts/upload-release.sh
Updated artifact download logic to prefer windows-msi for bun-windows-*.msi and to include MSI filenames only when WINDOWS_MSI_STEP is provided.
Tests
test/cli/install/msi.test.ts
Adds static tests validating WiX source, build script behavior, RTF/bitmap generation, CI wiring, artifact routing, and installer metadata/registry/environment wiring.
Socket lifecycle refactor (Zig)
src/bun.js/api/bun/socket.zig, src/bun.js/api/bun/socket/Listener.zig
Replaces nullable jsc.JSValue with jsc.JSRef and explicit upgrade/downgrade/finalize/deinit semantics; removes has_pending_activity; adjusts close/upgrade/error flows and related initialization checks.
Socket class metadata (TS)
src/bun.js/api/sockets.classes.ts
Removes hasPendingActivity: true from generated socket class definitions.
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding Windows MSI installer support built in CI, which is the primary focus of this comprehensive changeset.
Description check ✅ Passed The PR description is comprehensive and detailed, covering objectives, implementation, testing, and deployment with all key aspects clearly explained.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/bun-msi/build-msi.ps1`:
- Around line 111-123: The current RTF build escapes backslashes/braces and line
endings but does not encode non-ASCII characters, causing symbols like © to
render incorrectly; update the license encoding pipeline around the licenseBody
variable so non-ASCII bytes are emitted as RTF hex escapes (\'HH). Specifically,
after creating licenseBody, convert it to bytes using a single-byte Windows code
page (e.g. CP1252) via System.Text.Encoding::GetEncoding(1252).GetBytes(...) and
then produce an RTF-safe string by mapping each byte: output the ASCII-safe
characters (already escaped for \ and { } and newlines) unchanged but replace
any byte >= 0x80 (and any remaining special bytes) with the sequence \'HH; then
continue building $rtf and call Set-Content as before so the MSI license dialog
displays non-ASCII characters correctly.

In `@packages/bun-msi/README.md`:
- Around line 17-21: Update the fenced code block containing the msiexec
examples in README.md to include a language specifier (e.g., add "batch" or
"powershell" after the opening ``` fence) so syntax-highlighting is enabled for
the commands; modify the existing triple-backtick fence that surrounds the
msiexec lines to read ```batch (or ```powershell) while keeping the same command
lines intact.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 38ca9c67-f7f0-4da4-b520-7b06a4fdf615

📥 Commits

Reviewing files that changed from the base of the PR and between f8d4254 and 44e89cfb938f5b1277cdf87b75bc650e133c6230.

📒 Files selected for processing (8)
  • .buildkite/ci.mjs
  • .buildkite/scripts/build-windows-msi.ps1
  • .buildkite/scripts/upload-release.sh
  • packages/bun-msi/.gitignore
  • packages/bun-msi/README.md
  • packages/bun-msi/build-msi.ps1
  • packages/bun-msi/bun.wxs
  • test/cli/install/msi.test.ts

Comment thread packages/bun-msi/build-msi.ps1 Outdated
Comment thread packages/bun-msi/README.md Outdated
Comment thread packages/bun-msi/bun.wxs
Comment thread packages/bun-msi/bun.wxs
Comment thread packages/bun-msi/build-msi.ps1
Comment thread .buildkite/scripts/build-windows-msi.ps1 Outdated
@robobun robobun force-pushed the farm/4e017c28/windows-msi-installer branch from ff5ec8b to aeb391e Compare April 19, 2026 02:11
Comment thread test/cli/install/msi.test.ts Outdated
Comment thread packages/bun-msi/bun.wxs Outdated
Comment thread packages/bun-msi/build-msi.ps1

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/bun-msi/bun.wxs`:
- Around line 83-90: Add INSTALLFOLDER as a Secure property in the MSI similar
to ADDTOPATH and SETBUNINSTALL: update the <Property Id="INSTALLFOLDER" ...
Secure="yes" /> entry in bun.wxs so the custom-folder parameter survives the UAC
boundary; then add a regression test in test/cli/install/msi.test.ts that
invokes msiexec /i ... INSTALLFOLDER=... (per-machine install) and asserts the
installer honors the provided INSTALLFOLDER (files/env-vars end up under the
specified folder), ensuring the test covers the previous failure mode.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 390c0ff0-716e-4ee6-8a4f-8dd9afd9e2a2

📥 Commits

Reviewing files that changed from the base of the PR and between aeb391ef8a53e163d1421e118d5f55eaaeb23c47 and 4d6693fa7d55837203dc499d6c662f0045e561af.

📒 Files selected for processing (3)
  • packages/bun-msi/build-msi.ps1
  • packages/bun-msi/bun.wxs
  • test/cli/install/msi.test.ts

Comment thread packages/bun-msi/bun.wxs
Comment thread packages/bun-msi/bun.wxs
Comment thread packages/bun-msi/build-msi.ps1
Comment thread test/cli/install/msi.test.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/cli/install/msi.test.ts`:
- Around line 17-25: The helpers elements, attr, and findBy parse raw bun.wxs
and currently treat XML comments as real elements; update elements (and/or
findBy) to first strip XML comments before scanning by removing all <!-- ... -->
blocks (including multi-line) from src (e.g., run a global regex to delete
comment ranges) and then run the existing RegExp matchAll on the cleaned string
so comment-only content no longer matches or affects attr/findBy results.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 14eae225-37ec-45bf-9ba9-3c0eeed79fa0

📥 Commits

Reviewing files that changed from the base of the PR and between 4d6693fa7d55837203dc499d6c662f0045e561af and b34acc6f3f7652d2d76c17c0d5bf1dd6a2eed5c3.

📒 Files selected for processing (5)
  • packages/bun-msi/bun.wxs
  • src/bun.js/api/bun/socket.zig
  • src/bun.js/api/bun/socket/Listener.zig
  • src/bun.js/api/sockets.classes.ts
  • test/cli/install/msi.test.ts
💤 Files with no reviewable changes (1)
  • src/bun.js/api/sockets.classes.ts

Comment thread test/cli/install/msi.test.ts
Comment thread packages/bun-msi/bun.wxs
@robobun robobun force-pushed the farm/4e017c28/windows-msi-installer branch from b34acc6 to c020666 Compare April 19, 2026 03:09
Comment thread test/cli/install/msi.test.ts
Comment thread .buildkite/scripts/build-windows-msi.ps1 Outdated
Comment thread .buildkite/ci.mjs Outdated
Comment thread packages/bun-msi/bun.wxs Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional findings (outside current diff — PR may have been updated during review):

  • 🔴 .buildkite/ci.mjs:841-843 — Both bun-windows-x64.msi and bun-windows-x64-baseline.msi receive the same UpgradeCode (5da3c2a1-3a8e-4c67-9e1c-0b6f1e7f9d9f) because ci.mjs:843 maps x64-baseline to BunArch=x64 and bun.wxs only branches the UpgradeCode on arm64 vs else. With MajorUpgrade AllowSameVersionUpgrades=yes, deploying bun-windows-x64.msi to a machine with x64-baseline installed silently uninstalls the baseline binary and replaces it with the AVX2-requiring build; on non-AVX2 CPUs this causes bun.exe to crash with an illegal instruction on every invocation.

    Extended reasoning...

    What the bug is

    In ci.mjs, getWindowsMsiStep() at line 843 maps the WiX arch value with the expression platform.arch === "aarch64" ? "arm64" : "x64". The x64-baseline platform has arch="x64" and baseline=true; the ternary only checks arch, so x64-baseline is passed to build-msi.ps1 as -d BunArch=x64 — identical to the regular x64 build. In bun.wxs lines 37-41, the UpgradeCode is selected with a simple if/else guard on BunArch=arm64, so both x64 and x64-baseline receive UpgradeCode 5da3c2a1-3a8e-4c67-9e1c-0b6f1e7f9d9f. The PR description states "MajorUpgrade with per-arch UpgradeCodes replaces older installs in place," but only two UpgradeCodes exist (arm64 and everything else); x64-baseline is silently folded into the x64 slot.

    The concrete breakage path

    1. A non-AVX2 user (Haswell, Piledriver, etc.) explicitly installs bun-windows-x64-baseline.msi. bun.exe works correctly.
    2. A GPO/SCCM/winget deployment later pushes bun-windows-x64.msi as the canonical x64 package for the fleet.
    3. Windows Installer finds an existing product with the same UpgradeCode already installed; MajorUpgrade fires, silently uninstalls the baseline binary, and installs the AVX2-requiring regular build.
    4. bun.exe now crashes with an illegal instruction (SIGILL / STATUS_ILLEGAL_INSTRUCTION) on every invocation. No warning was shown, no user action was taken.

    Why the existing code does not prevent it

    The code comment at lines 841-842 explicitly acknowledges the BunArch mapping: "The x64-baseline build is still x64 here — MSI cares about ProgramFiles64Folder, not AVX2." This correctly explains why the -arch WiX flag must be x64 for both (to select ProgramFiles64Folder), but it conflates the WiX architecture flag with the UpgradeCode identity. The flag controls binary directory selection; the UpgradeCode controls product identity and upgrade detection. These are orthogonal: one can pass -arch x64 to WiX while using a third UpgradeCode for x64-baseline.

    Addressing the refutation

    The refutation argues the shared UpgradeCode is intentional because both variants install to the same path, and separate UpgradeCodes would allow co-installation causing file-reference-count corruption. This is a real concern — but it does not make the current behavior safe. The real-world failure is that a working bun.exe on a non-AVX2 machine gets silently replaced with one that crashes on every invocation, with no administrator visibility into what changed. Even accepting that co-installation at the same path is problematic, the current design provides no protection against a deployment system mistakenly pushing the AVX2 variant to non-AVX2 machines. The baseline and regular x64 builds are architecturally distinct products (different instruction set requirements), and Windows Installer cannot reason about that distinction if they share an UpgradeCode. The fix is a third UpgradeCode for x64-baseline, making the two variants separately addressable by deployment tooling, with clear upgrade semantics: baseline replaces baseline, x64 replaces x64.

    The fix

    In ci.mjs, pass BunArch=x64-baseline for baseline platforms while keeping -arch x64 for the WiX arch flag (these are separate parameters and serve different purposes). In bun.wxs, add an elseif branch for BunArch=x64-baseline with a new GUID, analogous to the existing arm64 branch.

Comment thread packages/bun-msi/bun.wxs
Comment thread packages/bun-msi/bun.wxs Outdated
Comment thread test/cli/install/msi.test.ts Outdated
Comment thread packages/bun-msi/build-msi.ps1 Outdated
Comment thread packages/bun-msi/bun.wxs
Comment thread .github/workflows/release.yml
Comment thread test/cli/install/msi.test.ts
Comment thread .github/workflows/release.yml
Comment thread packages/bun-msi/detect-cpu.c Outdated
Comment thread test/cli/install/msi.test.ts Outdated
robobun added 8 commits April 19, 2026 07:43
packages/bun-msi/: WiX v5 source + build script. Per-machine install to
%ProgramFiles%\Bun\bin with bun.exe and bunx.exe, appends bin to system
PATH, sets BUN_INSTALL, writes HKLM\Software\Oven\Bun for inventory, and
registers an ARP entry with the Bun icon. Public msiexec properties
INSTALLFOLDER / ADDTOPATH / SETBUNINSTALL for unattended deployment.
MajorUpgrade with per-arch UpgradeCodes; min-OS check matches install.ps1.

Dialog/banner bitmaps are generated at build time from src/bun.ico onto
Bun's cream->blush gradient (#fbf0df -> #f6dece) via System.Drawing so no
binary assets are checked in.

.buildkite/: new windows-msi step runs on a Windows x64 agent after either
windows-sign (release) or the raw build-bun steps (canary), builds one MSI
per target, signs them when the sign step ran, and uploads as artifacts.
upload-release.sh publishes bun-windows-{x64,x64-baseline,aarch64}.msi
alongside the zips. [build msi] in a commit message exercises the step on
a branch.
- bun.wxs: remove the VBScript StripTrailingSlash custom action.
  VBScript is an optional component on Win11 24H2+ and commonly
  GPO-blocked on the enterprise fleets MSI deploy targets, so a script
  CA risks a 1603/1721 on exactly the machines we care about. Write
  BUN_INSTALL as [INSTALLFOLDER] directly; every consumer in the tree
  path-joins (Path.joinAbsStringBuf, ${BunRoot}\bin) so the trailing
  backslash Windows Installer appends is harmless. Tests now assert no
  script CA is present.
- bun.wxs: drop Secure="yes" from ARPNOREPAIR/ARPNOMODIFY (display-only
  properties, no reason to let msiexec override them).
- build-msi.ps1: emit non-ASCII license characters as RTF \uN? Unicode
  keywords so LICENSE.md's smart quotes render correctly in the
  RichEdit control regardless of the user's ANSI codepage.
- build-msi.ps1: Test-Path before Resolve-Path so the intended
  "BunExe not found" message is actually reachable.
- README.md: add batch language hint to the msiexec fence; document the
  trailing backslash on BUN_INSTALL.
…t, drop stale delete

- bun.wxs: drop Secure="yes" from WINDOWSBUILDNUMBER. It's read-only OS
  introspection feeding the min-build Launch condition; Secure would let
  a command-line override pre-empt the RegistrySearch and bypass the gate.
- build-msi.ps1: check $LASTEXITCODE after 'wix extension add' so a cold
  NuGet fetch failure surfaces here rather than as an opaque 'extension
  not found' from 'wix build'.
- msi.test.ts: remove stale referenced.delete("BunPlatform") — that
  preprocessor var no longer exists in the .wxs.
- bun.wxs: add <Property Id="INSTALLFOLDER" Secure="yes" /> so a
  command-line INSTALLFOLDER=D:\Bun survives the client -> elevated-
  service hop on a per-machine install, same as ADDTOPATH/SETBUNINSTALL.
- msi.test.ts: centralise comment/CDATA stripping in a stripXmlTrivia()
  helper applied inside elements(), so assertions can't accidentally
  pass against prose like '<Property Secure="yes">' that lives inside
  an explanatory <!-- --> block. Test asserts INSTALLFOLDER is Secure.
sign-windows-artifacts.ps1:Configure-KeyLocker notes these calls are
'sometimes flaky but signing still works', so don't throw on non-zero
exit — but do capture and log the output so a later Sign-Msi failure
is traceable to its root cause instead of an opaque 'keypair not
found'. Get-AuthenticodeSignature in Sign-Msi remains the authoritative
pass/fail.
- ci.mjs: gate shouldBuildMsi on !buildId like the binary-size step.
  [build msi][skip builds] together would otherwise emit a windows-msi
  step whose depends_on references *-build-bun keys that were never
  pushed (they're inside the if(!buildId) block), and Buildkite rejects
  pipelines with dangling step keys at upload time.
- bun.wxs: correct the per-arch UpgradeCode rationale. Both x64 and
  arm64 resolve ProgramFiles64Folder to C:\Program Files on ARM64
  Windows — there is no separate ARM64 Program Files root. Separate
  UpgradeCodes exist to stop cross-arch MajorUpgrade replacement, not
  for path separation.
Per maintainer direction: build the MSIs in the Release workflow
(.github/workflows/release.yml) instead of Buildkite, so they're
generated automatically on release: published and the daily canary
schedule, and can be dispatched manually via the new use-msi input.

The msi job runs on windows-latest after the sign job, one matrix
entry per Windows target (x64, x64-baseline, aarch64). Each entry
downloads the corresponding zip from the GH release, runs
packages/bun-msi/build-msi.ps1, and uploads the .msi back to the
release (plus as a workflow artifact for inspection). WiX cross-builds
the arm64 package from the x64 host — it only embeds the binary.

The embedded bun.exe is already Authenticode-signed (it comes from the
release zip, which Buildkite's windows-sign step signed). The MSI
wrapper itself is not signed here because the DigiCert smctl secrets
live in Buildkite; the job has a comment explaining where to add
'smctl sign' if those secrets are ever mirrored to GHA.

Reverted all Buildkite wiring: ci.mjs (getWindowsMsiStep, buildMsi
option, WINDOWS_MSI_STEP), upload-release.sh MSI handling, and deleted
build-windows-msi.ps1. Tests now assert the release.yml job shape
instead.
Per maintainer direction: ship a single MSI containing all three
bun.exe payloads (x64, x64-baseline, arm64) that picks the right one
at install time. Resolves the product-identity tangle the per-arch
MSIs had — one UpgradeCode, one product family, installing the MSI
always cleanly MajorUpgrades whatever was there before, same
semantics as install.ps1.

packages/bun-msi/detect-cpu.c: ~30-line immediate CustomAction DLL
that sets BUNVARIANT via IsWow64Process2 (sees the native machine
even when the x64 MSI runs under emulation on ARM64) and
IsProcessorFeaturePresent(PF_AVX2) — the same Win32 calls install.ps1
P/Invokes. Defaults to x64-baseline so a detection failure never
leaves the user with an illegal-instruction crash. Compiled at
MSI-build time by build-msi.ps1 via cl.exe on the windows-latest
runner, so no binary is checked in.

bun.wxs: single fixed UpgradeCode; three mutually-exclusive bun.exe
components conditioned on BUNVARIANT; bunx.exe via DuplicateFile so
each variant contributes one payload not two; DetectCpu CA sequenced
after AppSearch and before CostFinalize in both UI and execute,
guarded on NOT BUNVARIANT so msiexec ... BUNVARIANT=x64-baseline
overrides detection; Variant recorded under HKLM\Software\Oven\Bun.

build-msi.ps1: takes -BunExeX64/-BunExeX64Baseline/-BunExeArm64;
locates MSVC via vswhere if cl isn't on PATH; always -arch x64 (ARM64
Windows 11 runs x64 MSIs under emulation).

release.yml: single job, no matrix; downloads all three zips, builds
one bun-windows.msi, uploads to the release + as a workflow artifact.

msi.test.ts: assertions for the three-payload structure, DetectCpu
wiring, single UpgradeCode, and detect-cpu.c staying in sync with
install.ps1's feature index. Brand-colour check now matches the
FromArgb channel bytes rather than the inline hex comment.
robobun added 5 commits April 19, 2026 07:43
An explicit but typo'd BUNVARIANT (e.g. x64baseline, missing the
hyphen) would skip DetectCpu, match no payload component, and still
install PATH / BUN_INSTALL / HKLM\Software\Oven\Bun — a 'successful'
install with no bun.exe on disk. Add a Launch condition that requires
BUNVARIANT to be one of x64 / x64-baseline / arm64 by the time
LaunchConditions fires (it runs after AppSearch + DetectCpu, so an
unset value has already been auto-detected to something valid).
'Installed OR' lets repair/uninstall through. Test asserts the gate.
Without /MT, MSVC defaults to /MD and the CA DLL imports
VCRUNTIME140.dll — a redistributable, not an OS component, absent on
clean Server Core / LTSC / stripped enterprise images. LoadLibrary
would fail and the install abort with 1720/1723 on exactly the fleets
the MSI targets. detect-cpu.c calls zero CRT functions so /MT only
pulls in the DllMainCRTStartup stub; /GS- drops the stack-cookie init
which would otherwise be the one remaining CRT reference.

Hoist the cl flags into $ClFlags so both invocations (vcvars fallback
and direct) are guaranteed identical. Test asserts /MT, no /MD, and
both calls use $ClFlags.
BUN_VERSION comes from workflow_dispatch.inputs.tag or
release.tag_name — maintainer-controlled either way — but the Actions
hardening recommendation is to pass values into scripts via step
env: and reference them as process-environment variables rather than
template-interpolating into the run: body. Same for RELEASE_TAG and
REPO. No functional change; avoids the template-injection shape the
linter flags.
The previous rationale claimed LaunchConditions gates the CA on old
OSes, but DetectCpu is sequenced After=AppSearch (~51) which runs
*before* LaunchConditions (~100) — on a hypothetical pre-1709 box
the DLL load would fail with 1720/1723 before the friendly min-OS
message appeared. The actual safety is simply that our install
floor (1809) is higher than the API's availability floor (1709), so
kernel32 always exports it where the install can proceed. Reworded
to state that and explicitly note the sequencing order.
With Return="ignore" a DLL-load or entry-point failure would be
swallowed, BUNVARIANT would stay empty, and the BUNVARIANT Launch
gate would abort every clean install with a misleading 'BUNVARIANT
must be one of…' message instead of surfacing the real detection
failure. Asserting Return alongside BinaryRef/DllEntry/Execute
closes that regression path.
@robobun robobun force-pushed the farm/4e017c28/windows-msi-installer branch from c46db20 to 89c5e2e Compare April 19, 2026 08:00
Comment thread .github/workflows/release.yml
Comment thread packages/bun-msi/detect-cpu.c Outdated
Comment thread test/cli/install/msi.test.ts Outdated
…nce CA placement

- release.yml: check $LASTEXITCODE after gh release download so a
  failed fetch surfaces as 'gh release download failed for <target>'
  rather than an opaque 'Cannot find path ... .zip' from Expand-Archive.
- detect-cpu.c: propagate MsiSetPropertyW failure as
  ERROR_INSTALL_FAILURE so Return="check" surfaces a clear 1720
  instead of falling through to the BUNVARIANT Launch gate with an
  empty value and a misleading 'must be one of…' message. In practice
  MsiSetPropertyW only fails on an invalid handle — defensive.
- msi.test.ts: verify DetectCpu is sequenced inside *both*
  InstallUISequence and InstallExecuteSequence individually rather
  than as a flat count; two <Custom> elements both inside
  ExecuteSequence would otherwise pass while leaving BUNVARIANT empty
  at UI-sequence LaunchConditions.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a substantial new feature — a complete MSI packaging pipeline (WiX source, C custom-action DLL, PowerShell build script, CI job with contents: write that uploads to releases). Several important issues were caught and fixed during review (missing /MT on the CA DLL compile, template injection in the CI workflow, INSTALLFOLDER not in SecureCustomProperties, invalid BUNVARIANT not validated, etc.). The current state looks solid, but the overall complexity and the fact that it gates release artifact uploads warrants a human pass before merge.

Extended reasoning...

Overview

PR adds a single universal bun-windows.msi containing x64, x64-baseline, and arm64 payloads with runtime variant selection via a native DLL CustomAction. Touches: .github/workflows/release.yml (new msi job with contents: write), packages/bun-msi/bun.wxs, packages/bun-msi/build-msi.ps1, packages/bun-msi/detect-cpu.c, and test/cli/install/msi.test.ts.

Security risks

The msi job has contents: write and uploads directly to GitHub releases via gh release upload. A prior review identified template-injection sinks where ${{ env.BUN_VERSION }} and ${{ steps.tag.outputs.tag }} were interpolated directly into shell/PowerShell scripts — these were fixed by moving values through process-environment variables. No remaining injection sinks visible in the final diff. The detect-cpu.dll is compiled from source at build time (no checked-in binary) and links statically (/MT) to avoid a VCRUNTIME140.dll dependency on locked-down enterprise images.

Level of scrutiny

High. This is new infrastructure code responsible for producing release artifacts that end users will install on Windows machines. Multiple non-trivial correctness issues were found and fixed during review (missing /MT causing install failure on clean enterprise images; INSTALLFOLDER missing Secure="yes" causing silent UAC-boundary drop; invalid BUNVARIANT leaving PATH poisoned with no bun.exe; CI template injection). The fixes all look correct in the final diff, but the combination of new C code, new WiX installer logic, and new CI with release-upload permissions makes this a PR that deserves deliberate human sign-off.

Other factors

The PR author engaged thoroughly with every review round and addressed all identified issues promptly. Tests are comprehensive (static validation of .wxs structure, PowerShell compile flags, CA sequencing, etc.) and run on all platforms. No automated test failures are visible in the final commit range.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant