Repository navigation
ci(windows): turn off Smart App Control on the Windows 11 test runners - #41550
Conversation
Smart App Control ships in evaluation mode on Windows 11. It hashes every unsigned executable on its first launch and asks the cloud for its reputation. That costs 2 to 3.5 seconds per bun build --compile output. bundler_compile.test.ts launches about 85 of those, so the file takes 220 to 290 seconds against the runner's 300 second per-file cap, and goes over it in the slower Azure regions. The runner turns the policy off at job start (it takes effect at once, no reboot). bootstrap.ps1 does the same for future image bakes.
|
Warning Review limit reached
On-demand reviews are free for the next 14 days. After that, they cost $0.25 per reviewed file. Or wait 2 minutes for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Comment |
|
Updated 5:19 AM PT - Sep 6th, 2026
❌ @robobun, your commit 651369f has 7 failures in
Add 🧪 To try this PR locally: bunx bun-pr 41550That installs a local version of the PR into your bun-41550 --bun |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Status: ready for review. The diff is green on the lane it targets. The remaining red lanes are unrelated. Reproduced on a Windows 11 aarch64 VM built from the CI image. Confirmed in CI twice (builds 111010 and 111037): the Windows 11 aarch64 agent logs The change is CI machinery only (runner and bootstrap), no bun or test code. 651369f gates the runner step on Buildkite so a developer machine with Failures on build 111037 do not touch this diff and are reported for main: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
CI=true alone also satisfies isCI, and the policy cannot be turned on again without a reinstall.
There was a problem hiding this comment.
LGTM — the isBuildkite guard in disableSmartAppControl() addresses the dev-machine concern from the earlier review.
What was reviewed:
- Confirmed
isBuildkiteis imported (runner.node.mjs:59) and the guard now short-circuits before any registry write on a non-BuildkiteCI=truerun. spawnSafereturn shape hasok/error(line 1845–1846); the 60s timeout and warn-on-failure match the best-effort pattern used elsewhere in the runner.Disable-Smart-App-Controlmirrors the siblingDisable-Windows-Threat-ProtectionTest-Path/Set-ItemProperty pattern in bootstrap.ps1; no-op on Server SKUs where the key is absent.
Extended reasoning...
Overview
This PR touches two CI-infrastructure scripts only: scripts/bootstrap.ps1 gains a Disable-Smart-App-Control function invoked from Optimize-System at image-bake time, and scripts/runner.node.mjs gains an async disableSmartAppControl() helper invoked from main() inside the existing isWindows && isCI block. Both write HKLM:\SYSTEM\CurrentControlSet\Control\CI\Policy\VerifiedAndReputablePolicyState = 0 and run CiTool --refresh to disable Windows 11 Smart App Control, eliminating a per-binary cloud reputation check that was pushing bundler_compile.test.ts toward the 300-second file cap. No runtime, bundler, or user-facing code is touched.
Security risks
The only meaningful risk was the one flagged in the earlier review: the SAC disable is irreversible without an OS reinstall, so it must never run on a developer machine that happens to have CI=true set. Commit 651369f addressed this by adding if (!isBuildkite) return; at the top of disableSmartAppControl() with an explanatory comment; isBuildkite is derived from Buildkite-specific env vars in utils.mjs and is not set by a bare CI=true. The bootstrap.ps1 path only runs during CI image baking, where disabling security features is already the explicit purpose of Optimize-System (it sits alongside Disable-Windows-Defender and Disable-Windows-Threat-Protection). No injection, auth, or data-exposure surface is introduced — the pwsh script string is a static literal with no interpolation of external input.
Level of scrutiny
Low-to-moderate. This is a small, self-contained infra change following established sibling patterns in both files, with no CODEOWNERS coverage for scripts/. The one non-obvious safety concern (irreversible modification of non-ephemeral machines) was raised, fixed, and re-verified against the current diff. Per REVIEW.md, best-effort auxiliary steps degrading quietly (warn-and-continue on !ok) is the correct behavior here since failing to disable SAC should not fail the test run.
Other factors
The bug-hunting run exited on dry_streak with no findings and no ruled-out candidates. There are no outstanding third-party CHANGES_REQUESTED reviews in the timeline. The spawnSafe call shape matches other call sites in the file (e.g. line 280, 1944), and the destructured { ok, error } matches the return object at line 1845. No automated test is expected for CI provisioning scripts; the PR description documents manual verification on a Windows 11 aarch64 VM (205s → 28s).
… time and assert stderr (#42358) ### Problem - `test/bundler/bundler_compile_prelinked.test.ts` takes 81 s on the Windows 11 aarch64 lane (build 114078). It links 26 executables one after the other and runs each one 3 times. - About 62 s of that is Smart App Control: the first `CreateProcess` of a new executable blocks about 2.4 s. `Bun.spawn` makes that call on the JS thread, so `describe.concurrent` alone cannot overlap it (74.5 s to 69 s). ### Fix - The block is `describe.concurrent`. A `beforeEach`/`afterEach` pair lets 3 cases link at once on release builds and 1 on ASAN and debug builds (the split #39649 measured). - On Windows the executables start through `cmd.exe /d /c`. The wait happens in `cmd.exe`, and the cases overlap it. - Every run asserts an empty stderr (75 new checks). `GeneratedGraph+splitting` pins each loader hook count: `resolve` is 5 with the graph, 252 without (was `<= 6`, `> 63`). The module-load `Bun.spawnSync` probe is gone. - Verified, main to branch: Windows 11 arm64 74.5 s to 27 s, Windows Server 2019 x64 11.9 s to 5.8 s, Linux x64 release 6.5 s to 2.9 s. `bun bd test` is unchanged (1 slot). All 27 tests and 3 loader modes still run. ### Background - Smart App Control is a Windows 11 code integrity policy, in evaluation mode by default. #41550 (open) turns it off on the runners. - Bun runs concurrent tests 20 at a time with no per-describe limit. 20 links at once ran CI out of memory before. - A loader mode is one environment for the same executable: graph in use, graph cross-checked, graph off. <details><summary>Notes</summary> **Where the time goes.** Probe on a Windows 11 arm64 VM with Smart App Control in evaluation mode (`Get-MpComputerStatus` reports `SmartAppControlState: Eval`, the same as the CI image): `bun build --compile --bytecode` 260 to 330 ms, first launch of each new executable 2.4 to 2.7 s, second and third launch 16 ms. The `Bun.spawn()` call itself returns after 2.3 to 4.0 s for a new executable and after 2 to 3 ms for a known one. Eight cases started at once take 22.0 s with a direct spawn (the same as serial) and 5.8 s through `cmd.exe /d /c`, where the `Bun.spawn()` call returns in 7 ms. During one first launch `MsMpEng` uses about 1.7 s of CPU. Pinning the test process tree to 4 or to 2 cores does not change the file's time (28.0 s and 27.2 s), so the 4 vCPU runners should see a similar gain. **Whole file, `bun test` with a release build, main to this branch.** | machine | main | branch | | --- | ---: | ---: | | Windows 11 arm64, 16 vCPU, Smart App Control in evaluation mode | 74.5 s | 26.2 to 35.2 s (7 runs, median 28.0 s) | | the same, without the `cmd.exe` launcher | | 69.0 s | | Windows Server 2019 x64, 16 vCPU | 11.9 s | 5.8 s (6.4 s without the launcher) | | Linux x64, release, 5 interleaved pairs | 6.3 to 6.9 s | 2.4 to 3.4 s | | Linux x64, `bun bd test` (debug + ASAN, 1 slot) | 107.5 s, 127.8 s | 114.0 s, 99.4 s, 99.0 s | Slot count on the Windows 11 VM: 2 slots 38.3 s, 3 slots 27 s, 4 slots 22.9 s, 6 slots 17.0 s. The file uses 3 on every release build, the number #39649 checked on the 4 vCPU, 8 GB Linux lanes. **Per lane in build 114078** (all in the parallel bucket): windows 11 aarch64 80.55 s, darwin x64 44.52 s, debian x64-asan 37.19 s, windows 2019 x64 17.27 s, debian x64 9.68 s, ubuntu x64 9.60 s, ubuntu aarch64 7.20 s, alpine x64 7.18 s, debian aarch64 7.08 s, alpine aarch64 5.00 s, darwin aarch64 4.31 s. In that batch of 189 files (91 s) this file was the critical path on Windows 11. **ASAN and debug stay serial.** Locally the link is 2.5 to 2.9 s of a 3.5 s case with the 812 MB debug binary, and each run is about 0.3 s. #39649 tried 3 links at once on the x64-asan lane: the file got slower there (170 s against 107 s), because the link is three passes over the binary and the lane is bound by writeback. One slot under `describe.concurrent` behaves like today. **The three runs of one executable do not overlap.** After the first launch a run takes 5 ms on Linux and 16 ms on Windows, so there is nothing to gain on release builds. On debug builds it would save about 0.6 s per case, and it needs a change to the run loop in `expectBundled.ts`, which every bundler test file shares. **`bunArgs`.** `expectBundled` builds the command as `[...bunArgs, file, ...args]` for a compiled executable, so the launcher needs no change to the helper. `cmd.exe /d /c <path>` passes stdio and the exit code through. It works for paths with spaces and `+` (probed). It does not work for a temp directory whose path contains `&`, `(`, `)`, `^` or `%VAR%`. **Loader counts.** With `BUN_JSC_dumpModuleLoadingState=1` JSC prints one `Loader [hook] key` line per host hook call. Linux x64 release, Linux x64 debug, Windows 11 arm64 and Windows Server 2019 x64 all print the same table: `resolve` 5 (bun:main twice, the entry, m24 twice) with the graph, 252 without it, `fetch` 3, `evaluate` 63, `import` 1, and no other line on stderr. **The probe.** `hasPrelinkOptions` let an older bun fail only in the loader log check. That was for the fail-before run of #42002. A bun without the options now fails the second and third run of every case with `invalid JSC environment variable`, which is also a clear failure. **Slots.** 45 concurrent tests with 3 slots, one test that throws and one that times out: 45 ran, at most 3 held a slot, no slot leaked. `afterEach` runs after a failed or timed out test. The per-test timeout starts when the test callback starts (`on_entry_started` in `src/runtime/test_runner/Execution.rs`), so the wait in `beforeEach` does not count against it. The reported duration of a test does include the wait. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/bundler_compile_prelinked.test.ts <!-- robobun:evidence:end -->
Problem
test/bundler/bundler_compile.test.tsreportstimeout on :windows: 11 aarch64(builds 110724, 110766, 110870). The output stops after a different test each time and no per-test timeout fires. The file is not hung. It takes 286 to 293 s in the passing runs and the runner caps it at 300 s (integrationTimeout,scripts/runner.node.mjs:90).bun build --compileoutput pays 2.3 s for that on the aarch64 VM, more in canadaeast and switzerlandnorth. The file launches about 85 such binaries.scripts/bootstrap.ps1tries to disable Defender, but tamper protection blocksDisableRealtimeMonitoringon Windows 11,Uninstall-WindowsFeaturedoes not exist there, and theC:\exclusion does not cover Smart App Control.Fix
scripts/runner.node.mjs: at job start on Windows, setHKLM\SYSTEM\CurrentControlSet\Control\CI\Policy\VerifiedAndReputablePolicyStateto 0 and runCiTool --refresh. This takes effect at once, with no reboot. It runs next to the existingSet-DnsClientServerAddressstep, which already needs the same rights.scripts/bootstrap.ps1:Disable-Smart-App-ControlinOptimize-System, so the next image bake has it off from the start. No# Versionbump and no[publish images]: the runner step covers the current v23 images.Smart App Control: state 2 -> 0and is a no-op once the value is 0.Background
bundler_compileis on it). A file that passes the cap is reported astimeoutand retried once.Notes
There is no single culprit commit. The file has grown to 85 tests (#41263 and #40816 added the latest ones) and the per-launch cost depends on the Azure region the spot VM lands in. The three failures ran in canadaeast and switzerlandnorth.
How the cause was found:
EPERMfrom the runner'srmSyncof the temp dir: the killed test's child was still running. Nobun testper-test timeout fired, so nothing was stuck, the file was cut off mid-run.where.exewith 77 MB of random bytes appended (not mapped by the loader) pays 2600 ms. A copy of the signednode.exe(89 MB) pays 800 ms. Defender exclusions forC:\andD:\were already in place.Get-MpComputerStatusshowsSmartAppControlState: Evaland the Code Integrity log lists theVerifiedAndReputableDesktopEvaluationpolicy as active. AfterVerifiedAndReputablePolicyState = 0andCiTool --refresh: stateOff, first launch 154 ms, full file 28 s.Suites run on the dev VM after the change:
bundler_compile.test.ts(85 pass),bundler_compile_autoload.test.ts,bundler_compile_splitting.test.ts,transpiler/property.test.tsthroughrunner.node.mjs.no test proof · iteration 1 · build/CI scripts only; test-proof not applicable