diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8ce503db..b296b47a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -138,6 +138,48 @@ jobs: } Write-Output "$($expected.Count) tile assets present in $($appx.Name), byte-identical to build/appx/" + # Twice running, a release shipped a dependency that could not be resolved on + # the target machine, and both times someone outside the project found it: + # 1.9.0's compositor addon was reached through PATH, which MSIX ignores, and + # 1.9.1's capture helper needed the Visual C++ Redistributable, which is not + # part of Windows. Each fix arrived with a guard aimed at the failure already + # understood, and neither guard would have caught the other. + # + # This step is the one that generalises: it registers the package and asks the + # Windows loader to resolve every shipped binary for real. It deliberately does + # not record anything — a runner has no useful GPU or desktop session, and a + # flaky gate gets switched off. The loader is what broke both times. + # + # `powershell`, not `pwsh`: the Appx module is not loaded natively in + # PowerShell 7 and needs -UseWindowsPowerShell to work at all. + - name: Verify native binaries load under package identity + shell: powershell + run: | + $ErrorActionPreference = "Stop" + + # Loose registration of an unsigned package needs Developer Mode. The runner + # is discarded after the job, so enabling it here costs nothing; the script + # itself refuses to touch this, because on a real machine it is the owner's + # setting to make. + $key = "HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\AppModelUnlock" + New-Item -Path $key -Force | Out-Null + Set-ItemProperty -Path $key -Name AllowDevelopmentWithoutDevLicense -Value 1 -Type DWord + + $appx = Get-ChildItem release -Recurse -Filter *.appx | Select-Object -First 1 + if (-not $appx) { throw "no .appx found under release/" } + + # Explicit exit rather than letting the error surface on its own: a + # terminating error does not set $LASTEXITCODE, and the runner's epilogue + # only inspects that. Trusting it would let a failed verification report a + # green step, which is the exact shape of bug this job exists to stop. + try { + & "$env:GITHUB_WORKSPACE\scripts\verify-appx-native.ps1" -Appx $appx.FullName + } + catch { + Write-Output "::error::$($_.Exception.Message)" + exit 1 + } + - name: Upload Windows Store package uses: actions/upload-artifact@v7 with: diff --git a/scripts/verify-appx-native.ps1 b/scripts/verify-appx-native.ps1 new file mode 100644 index 00000000..01ae79fc --- /dev/null +++ b/scripts/verify-appx-native.ps1 @@ -0,0 +1,241 @@ +<# +.SYNOPSIS +Proves every native binary in a built .appx can actually load, from inside a +registered MSIX package. + +.DESCRIPTION +Twice in a row a Windows release shipped a dependency that could not be resolved +on the target machine, and both times someone else found it: + + 1.9.0 compositor_view.node sat one directory away from its ffmpeg DLLs and was + reached via PATH. MSIX resolves dependent DLLs through the package graph + and ignores PATH, so the Store build loaded no compositor: the editor + opened with a permanently blank preview while audio kept playing. + + 1.9.1 wgc-capture.exe, cursor-sampler.exe and compositor_view.node imported + VCRUNTIME140/MSVCP140 from the Visual C++ Redistributable, which is not + part of Windows. Store certification rejected the submission: recording + died with 0xC0000135, STATUS_DLL_NOT_FOUND, before main(). + +Each fix came with a guard aimed at the failure already understood, and neither +guard would have caught the other one. That is the gap this script closes. Rather +than asserting a known-bad pattern, it registers the package and asks the Windows +loader to resolve every shipped binary for real. Whatever the next unresolvable +dependency turns out to be, this fails on it. + +It deliberately does NOT record anything. A real capture needs a GPU and a desktop +session, which a CI runner does not usefully have, and a flaky gate gets switched +off. The loader is the part that broke both times, and it can be tested with +neither. + +ASCII only, on purpose: Windows PowerShell 5.1 reads a .ps1 as ANSI unless it +carries a BOM, so a stray em dash turns into a parser error rather than a typo. + +.PARAMETER Appx +The .appx to verify. + +.PARAMETER KeepRegistered +Leave the package registered afterwards, to click through the app by hand. + +.EXAMPLE +powershell -File scripts/verify-appx-native.ps1 -Appx release/1.9.1/Openscreen.Setup.1.9.1.appx + +.NOTES +Loose registration needs Developer Mode (Settings > System > For developers). +This script will not enable it: that is a machine-wide setting and its owner +should be the one turning it on. CI sets AllowDevelopmentWithoutDevLicense itself, +on a runner that is thrown away afterwards. + +Run it with Windows PowerShell, not pwsh: the Appx module is not loaded natively +in PowerShell 7 and needs -UseWindowsPowerShell to work at all. +#> +[CmdletBinding(DefaultParameterSetName = "Verify")] +param( + [Parameter(Mandatory, ParameterSetName = "Verify")] + [string]$Appx, + + [Parameter(ParameterSetName = "Verify")] + [switch]$KeepRegistered, + + # Set when the script re-invokes itself inside the package container. Not for + # direct use: outside the container it proves nothing, because a developer + # machine resolves through PATH and System32 exactly the way the Store does not. + [Parameter(Mandatory, ParameterSetName = "InPackage")] + [switch]$InPackage, + + [Parameter(Mandatory, ParameterSetName = "InPackage")] + [string]$PackageRoot, + + [Parameter(Mandatory, ParameterSetName = "InPackage")] + [string]$ReportPath +) + +$ErrorActionPreference = "Stop" + +# 0xC0000135. Surfaces as a negative Int32 through Process.ExitCode. +$STATUS_DLL_NOT_FOUND = -1073741515 + +function Get-NativeDir { + param([string]$Root) + return (Join-Path $Root "app\resources\electron\native\bin\win32-x64") +} + +# ---------------------------------------------------------------- in-package -- + +if ($InPackage) { + # LOAD_WITH_ALTERED_SEARCH_PATH is the flag Node passes for a .node addon, so + # the module's own directory is searched for its dependencies. Using the same + # flag is what makes this a test of require() rather than of something adjacent. + Add-Type -Namespace OpenScreen -Name Loader -MemberDefinition @' +[System.Runtime.InteropServices.DllImport("kernel32.dll", SetLastError = true, CharSet = System.Runtime.InteropServices.CharSet.Unicode)] +public static extern System.IntPtr LoadLibraryExW(string path, System.IntPtr file, uint flags); +'@ + + $results = @() + $dir = Get-NativeDir -Root $PackageRoot + + foreach ($file in Get-ChildItem -Path $dir -File | Where-Object { $_.Extension -match '^\.(dll|node)$' }) { + $handle = [OpenScreen.Loader]::LoadLibraryExW($file.FullName, [IntPtr]::Zero, 0x00000008) + $lastError = [System.Runtime.InteropServices.Marshal]::GetLastWin32Error() + $loaded = ($handle -ne [IntPtr]::Zero) + # 126 is ERROR_MOD_NOT_FOUND: a dependency of this binary is missing, which + # is the same story 0xC0000135 tells about an executable. + $detail = "loaded" + if (-not $loaded) { $detail = "LoadLibraryEx failed, GetLastError=$lastError" } + $results += [pscustomobject]@{ name = $file.Name; kind = "load"; ok = $loaded; detail = $detail } + } + + # The helpers are separate processes, so their imports resolve at CreateProcess + # time and a failure never reaches their own code. Started with no arguments on + # purpose: each one prints its usage and exits, which needs no GPU, no desktop + # session and no capture, while still proving the loader let it start. A binary + # the loader rejects produces NO output at all and exits STATUS_DLL_NOT_FOUND. + foreach ($name in @("wgc-capture.exe", "cursor-sampler.exe", "whisper-stt-server.exe")) { + $exe = Join-Path $dir $name + if (-not (Test-Path $exe)) { + $results += [pscustomobject]@{ name = $name; kind = "start"; ok = $false; detail = "not present in the package" } + continue + } + + $psi = New-Object System.Diagnostics.ProcessStartInfo + $psi.FileName = $exe + $psi.UseShellExecute = $false + $psi.RedirectStandardOutput = $true + $psi.RedirectStandardError = $true + $psi.CreateNoWindow = $true + $proc = [System.Diagnostics.Process]::Start($psi) + $stdout = $proc.StandardOutput.ReadToEnd() + $stderr = $proc.StandardError.ReadToEnd() + $proc.WaitForExit(20000) | Out-Null + + $exitCode = "still running" + if ($proc.HasExited) { $exitCode = $proc.ExitCode } else { $proc.Kill() } + + $said = ($stdout + $stderr).Trim() + $firstLine = "" + if ($said.Length -gt 0) { $firstLine = @($said -split "`r?`n")[0] } + + # "Said something" is the assertion, not "exited non-zero": these all exit 1 + # when told nothing to do. Only a process that reached its own main() prints. + $ok = ($said.Length -gt 0) -and ($exitCode -ne $STATUS_DLL_NOT_FOUND) + $detail = "exit=$exitCode with NO output, killed by the loader before main()" + if ($said.Length -gt 0) { $detail = "exit=$exitCode, said: $firstLine" } + $results += [pscustomobject]@{ name = $name; kind = "start"; ok = $ok; detail = $detail } + } + + $results | ConvertTo-Json -Depth 4 | Set-Content -Path $ReportPath -Encoding utf8 + return +} + +# ------------------------------------------------------------- orchestration -- + +$appxPath = (Resolve-Path $Appx).Path +$work = Join-Path ([System.IO.Path]::GetTempPath()) ("openscreen-appx-verify-" + [System.IO.Path]::GetRandomFileName()) +$extracted = Join-Path $work "package" +$report = Join-Path $work "report.json" +$registered = $null + +try { + Write-Host "Extracting $([System.IO.Path]::GetFileName($appxPath))" + New-Item -ItemType Directory -Path $work -Force | Out-Null + # Expand-Archive insists on the extension even though an .appx is a plain zip. + $zip = Join-Path $work "package.zip" + Copy-Item $appxPath $zip + Expand-Archive -Path $zip -DestinationPath $extracted -Force + Remove-Item $zip -Force + + # A loose registration is unsigned by definition, and these two describe a + # signed layout. Leaving them makes Add-AppxPackage reject the directory. + Remove-Item (Join-Path $extracted "AppxSignature.p7x"), (Join-Path $extracted "AppxBlockMap.xml") -Force -ErrorAction SilentlyContinue + + $manifest = Join-Path $extracted "AppxManifest.xml" + if (-not (Test-Path $manifest)) { throw "no AppxManifest.xml in $appxPath, is it really an appx?" } + + Write-Host "Registering the package" + try { + Add-AppxPackage -Register $manifest -ErrorAction Stop + } + catch { + throw "Add-AppxPackage -Register failed: $($_.Exception.Message)`n`nLoose registration needs Developer Mode (Settings > System > For developers)." + } + + $pkg = Get-AppxPackage -Name "EtienneLescot.OpenScreen" + if (-not $pkg) { throw "the package registered but cannot be found by name" } + $registered = $pkg.PackageFullName + Write-Host "Registered $($pkg.PackageFullName)" + + # Invoke-CommandInDesktopPackage gives the child package identity, which is the + # entire point: outside it, PATH and System32 paper over exactly the failures + # being looked for. It returns no output, so the child reports through a file. + $childArgs = "-NoProfile -ExecutionPolicy Bypass -File `"$PSCommandPath`" -InPackage -PackageRoot `"$extracted`" -ReportPath `"$report`"" + Invoke-CommandInDesktopPackage ` + -PackageFamilyName $pkg.PackageFamilyName ` + -AppId "Openscreen" ` + -Command "powershell.exe" ` + -Args $childArgs ` + -ErrorAction Stop + + $deadline = (Get-Date).AddSeconds(180) + while (-not (Test-Path $report) -and (Get-Date) -lt $deadline) { Start-Sleep -Milliseconds 500 } + if (-not (Test-Path $report)) { throw "the in-package probe never wrote its report" } + + # `@(ConvertFrom-Json ...)` looks like it produces an array and does not: Windows + # PowerShell 5.1 writes the whole deserialized array to the pipeline as ONE + # object, so @() wraps it into a single-element array holding an array. The first + # version of this script printed "All 1 native binaries load" for seventeen of + # them, and `Where-Object { -not $_.ok }` evaluated `-not` against an array of + # booleans, which is always false. It would have reported success no matter what + # the probe found. `foreach` over the variable enumerates properly. + $results = @() + foreach ($item in (Get-Content $report -Raw | ConvertFrom-Json)) { $results += $item } + if ($results.Count -eq 0) { throw "the in-package probe found no native binaries to check" } + + # The package ships fourteen libraries and three helpers. An exact count is too + # brittle to assert, but a probe that suddenly checks two things has stopped + # looking at the payload, and silence is the one outcome this must never have. + if ($results.Count -lt 10) { + throw "the in-package probe only checked $($results.Count) binaries, which is too few to be the real payload" + } + + foreach ($r in $results) { + $mark = "ok " + if (-not $r.ok) { $mark = "FAIL" } + Write-Host " $mark $($r.kind)`t$($r.name)`t$($r.detail)" + } + + $failed = @($results | Where-Object { -not $_.ok }) + if ($failed.Count -gt 0) { + foreach ($r in $failed) { Write-Output "::error::$($r.name): $($r.detail)" } + throw "$($failed.Count) of $($results.Count) native binaries do not load under package identity" + } + + Write-Host "All $($results.Count) native binaries load under package identity." +} +finally { + if ($registered -and -not $KeepRegistered) { + Remove-AppxPackage -Package $registered -ErrorAction SilentlyContinue + } + if (-not $KeepRegistered) { + Remove-Item $work -Recurse -Force -ErrorAction SilentlyContinue + } +} diff --git a/technical-documentation/engineering/build-and-packaging.md b/technical-documentation/engineering/build-and-packaging.md index 119eb840..e5e8918a 100644 --- a/technical-documentation/engineering/build-and-packaging.md +++ b/technical-documentation/engineering/build-and-packaging.md @@ -95,6 +95,41 @@ The fix is to link the CRT statically, which removes the dependency instead of o **Local testing cannot confirm this class of fix.** This machine has the redistributable and always will, so a successful run here proves the build is not broken — it says nothing about the clean-machine behaviour. The import table is the only evidence for that half, which is why the guard reads it rather than running anything. +### Verifying a package actually loads + +The two failures above were found by the Store, not by us, and each was fixed with a guard aimed at the failure already understood — the colocation check would never have caught the redistributable, and the import-table check would never have caught the `PATH` bug. Both are worth keeping, and neither generalises. + +`scripts/verify-appx-native.ps1` is the check that does. It registers a built `.appx` and asks the Windows loader to resolve every shipped binary from inside the package: `LoadLibraryEx` with `LOAD_WITH_ALTERED_SEARCH_PATH` for each `.dll`/`.node` — the same call Node makes for an addon — and, for each helper executable, a start with no arguments. Whatever the next unresolvable dependency turns out to be, this fails on it. + +```bash +powershell -File scripts/verify-appx-native.ps1 -Appx release/1.9.1/Openscreen.Setup.1.9.1.appx +``` + +Add `-KeepRegistered` to leave the package installed and click through the app afterwards. Loose registration needs Developer Mode; the script will not enable it for you, because that is a machine-wide setting. The `Windows Store package` job runs the same script on every build, enabling Developer Mode on the runner it is about to discard. + +Two things it deliberately does not do. It never records: a real capture needs a GPU and a desktop session that a CI runner does not usefully have, and a flaky gate gets switched off — the loader is the part that broke both times, and it can be tested without either. And it proves nothing about a machine that lacks a runtime, because every runner and every developer machine has the Visual C++ Redistributable; that half is held by the import-table check in `before-pack.cjs`. + +### Testing without the build machine's advantages + +Every failure in this section shares one shape: **the machines we test on have more installed than the machines we ship to.** A developer box carries the Visual C++ Redistributable because Visual Studio put it there; a CI runner carries a newer glibc than the distros the README claims. Nothing run on either can reveal an absence, so "it works here" is not evidence about anything, however many times it is repeated. + +Three layers address it, and the order matters: + +1. **Remove the dependency at the source.** A statically linked CRT cannot be missing; a runner pinned to the oldest supported distro cannot bind symbols the user does not have. This is the only layer that needs no testing at all, so prefer it whenever there is a choice. +2. **Prove it automatically.** Static analysis for absences that are known and enumerable (`before-pack.cjs` reads import tables and ELF symbol-version needs), and a real load for everything else (`verify-appx-native.ps1` under package identity). +3. **A pristine machine before a Store submission.** The only layer that catches a failure nobody has thought of yet. + +For layer 3, keep a virtual machine whose entire value is what is *not* installed in it. VirtualBox and VMware Workstation both run on Windows 11 Home, which has neither Windows Sandbox nor Hyper-V; Microsoft publishes Windows 11 ISOs at no cost, and an unactivated install is fine for this. + +**Snapshot it the moment Windows finishes installing, before anything else touches it.** That snapshot is the asset — the VM itself is disposable. Restore it after every test, and never install Visual Studio, the Rust toolchain, or anything that carries a redistributable, or it quietly becomes another machine that cannot tell you anything. + +What to run inside it, cheapest first: + +- **The `.exe` installer.** No Developer Mode, no ceremony, and it carries the same native binaries as the appx. Record something and open the editor: if recording starts and the preview renders, the missing-runtime class is clear for both Windows channels at once. +- **The appx**, for what is specific to MSIX — package-graph DLL resolution, which the `.exe` cannot exercise. Enable Developer Mode and run `verify-appx-native.ps1`, then launch the app by hand. Developer Mode installs no runtime, so it does not compromise what the VM is for. + +Expect the compositor to report a software backend: a VM has no real GPU, and `probeBackend()` returning `"software"` there is correct, not a regression. Enable the hypervisor's 3D acceleration so the preview renders at all. + ### Stale native artifacts **On Windows, always package with `npm run build:win`, not `npm run build`.**