Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@
*.rs text eol=lf
*.ts text eol=lf
*.tsx text eol=lf
# Same drift, caught on the native helper's build file: an edit from Windows
# rewrote all 67 lines as CRLF and buried a 22-line change in a 156-line diff.
CMakeLists.txt text eol=lf
42 changes: 42 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
17 changes: 17 additions & 0 deletions crates/.cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,23 @@
FFMPEG_DIR = { value = "thirdparty/ffmpeg-n8.1.2-win64-lgpl-shared", relative = true }
LIBCLANG_PATH = "C:\\Program Files\\LLVM\\bin"

# CRT statique. Par défaut, un cdylib MSVC importe VCRUNTIME140.dll, qui ne fait PAS
# partie de Windows : elle vient du Redistribuable Visual C++. Toute machine ayant
# déjà compilé ce dépôt — ou installé à peu près n'importe quelle autre application
# de bureau — l'a dans System32, donc la dépendance est invisible en local comme en
# CI. Sur une image Windows propre, `require()` de compositor_view.node échoue et
# l'éditeur s'ouvre sans aperçu pendant que le son continue : le symptôme EXACT que
# la colocation avec les DLL ffmpeg (build-windows-compositor-addon.mjs) était
# censée corriger. Elle a bien corrigé sa cause à elle — MSIX qui ignore PATH — mais
# celle-ci restait, et aucune machine de dev ne peut la faire apparaître.
#
# Rien de partagé avec l'hôte ne traverse la frontière du CRT : napi-rs ne fait
# passer que des napi_value opaques, et les Buffer rendus à Node portent un
# finalizer qui libère côté addon ce que l'addon a alloué. Le CRT statique est donc
# sans effet de bord ici. scripts/before-pack.cjs échoue si l'import revient.
[target.x86_64-pc-windows-msvc]
rustflags = ["-C", "target-feature=+crt-static"]

# macOS : BtbN ne publie pas de build macOS (cf. scripts/fetch-ffmpeg.mjs), donc on
# laisse `crates/compositor/build.rs` chercher MAC_FFMPEG_DIR (env var explicite posée
# par la CI macOS ou par le dev local) ou un répertoire attendu sous `thirdparty/`.
Expand Down
22 changes: 22 additions & 0 deletions electron/native/wgc-capture/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,28 @@ set(CMAKE_CXX_STANDARD_LIBRARIES

project(openscreen-wgc-capture LANGUAGES CXX)

# Static CRT (/MT), not CMake's default /MD. With /MD both helpers import
# VCRUNTIME140.dll, VCRUNTIME140_1.dll and MSVCP140.dll, which are NOT part of
# Windows — they come from the Visual C++ Redistributable. Every machine that has
# ever built this repo, or installed almost any other desktop app, already has
# them in System32, so the dependency is invisible in local testing and in CI.
#
# On a clean Windows image it is fatal: the loader kills the process before main()
# and the parent only sees an exit code. Store certification runs on exactly such
# an image and rejected 1.9.1 for it, reporting
#
# Error: Native Windows capture exited before recording started (code=3221225781)
#
# 3221225781 is 0xC0000135, STATUS_DLL_NOT_FOUND. Recording was impossible on the
# test device while the app itself started fine, because Electron already links
# the CRT statically.
#
# These are standalone processes that share no CRT state with anything, so /MT is
# free: it costs ~100 KB each and removes the dependency instead of obliging us to
# redistribute Microsoft's DLLs beside them. scripts/before-pack.cjs fails the
# build if it ever comes back.
set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")

set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)
Expand Down
139 changes: 138 additions & 1 deletion scripts/before-pack.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,151 @@ const WIN_REQUIRED = [
})),
];

/**
* DLLs that come from the Visual C++ Redistributable rather than from Windows itself.
*
* The `api-ms-win-crt-*` api-sets are deliberately absent: that is the UCRT, which IS
* part of Windows 10 and later. These are not, and no machine is obliged to have them.
*/
const VC_REDIST_DLL = /^(msvcp|vcruntime|concrt)\d+/i;

/**
* The DLL names a PE binary imports — just enough of the format to walk the import
* directory, so this needs no dumpbin and therefore no Visual Studio on the runner.
*/
function importedDlls(file) {
const b = fs.readFileSync(file);
// Every read below is bounds-checked through this, so a truncated or non-PE file
// arrives at the message this function means to give rather than at a RangeError
// from readUInt32LE. The diagnostic is the whole product here: the caller's job is
// to explain an absence, and "offset is out of bounds" explains nothing.
const notPe = () => new Error(`${file} is not a PE binary, or is truncated`);
const u32 = (at) => {
if (at < 0 || at + 4 > b.length) throw notPe();
return b.readUInt32LE(at);
};
const u16 = (at) => {
if (at < 0 || at + 2 > b.length) throw notPe();
return b.readUInt16LE(at);
};

if (u16(0) !== 0x5a4d) throw notPe(); // "MZ"
const pe = u32(0x3c);
if (u32(pe) !== 0x00004550) throw notPe(); // "PE\0\0"
const opt = pe + 24;
// The optional header's fixed part is 96 bytes for PE32 and 112 for PE32+ (five
// fields widen to 8 bytes); the data directories follow it.
const dirs = opt + (u16(opt) === 0x20b ? 112 : 96);
// NumberOfRvaAndSizes is the last field before the directories, so it sits four
// bytes back whichever the format. Without it, a binary declaring fewer entries
// than we index would have unrelated header bytes read as an RVA.
const dirCount = u32(dirs - 4);

const sections = [];
for (let i = 0; i < u16(pe + 6); i++) {
const s = opt + u16(pe + 20) + i * 40;
sections.push({ va: u32(s + 12), size: u32(s + 16), ptr: u32(s + 20) });
}
const fileOffset = (rva) => {
const s = sections.find((s) => rva >= s.va && rva < s.va + s.size);
if (!s) throw new Error(`${file}: RVA 0x${rva.toString(16)} is in no section`);
return s.ptr + (rva - s.va);
};

const names = [];
// Both directories are arrays of fixed-size descriptors ending in an all-zero one,
// and both hold the DLL name as an RVA at a fixed offset. Reading only the first
// would miss a delay-loaded dependency entirely — the loader resolves those on
// first call rather than at load time, so the failure would arrive later and
// nowhere near the cause, which is worse than the one this guard was written for.
const walk = (index, stride, nameOffset) => {
if (dirCount <= index) return;
const rva = u32(dirs + index * 8);
if (!rva) return;
for (let entry = fileOffset(rva); ; entry += stride) {
const nameRva = u32(entry + nameOffset);
if (!nameRva) return;
const at = fileOffset(nameRva);
names.push(b.subarray(at, b.indexOf(0, at)).toString("latin1"));
}
};
walk(1, 20, 12); // IMAGE_IMPORT_DESCRIPTOR.Name
walk(13, 32, 4); // ImgDelayDescr.rvaDLLName
return names;
}

/**
* Nothing we ship may depend on the Visual C++ Redistributable.
*
* This is the one failure this whole hook could not see. Every machine that builds this
* repo, and most machines that have ever installed a desktop app, carry those DLLs in
* System32 — so a binary that needs them works in local testing, in CI, and in every
* package format, while being unloadable on a clean Windows image. There is no way to
* reproduce it here; only the import table tells the truth.
*
* Store certification rejected 1.9.1 for exactly this: the WGC helper was built against
* the dynamic CRT and died in the loader before main(), so the app reported
* `Native Windows capture exited before recording started (code=3221225781)`
* — 0xC0000135, STATUS_DLL_NOT_FOUND — and recording was impossible on the test device.
* `compositor_view.node` had the same defect and would have failed certification a
* second time, on the preview, right after the helper was fixed.
*
* The fix is per-toolchain, hence the two-part message: /MT for the CMake helpers
* (electron/native/wgc-capture/CMakeLists.txt), `+crt-static` for the Rust addon
* (crates/.cargo/config.toml).
*/
function checkWinNoRedistDependency(dir) {
const scanned = fs
.readdirSync(dir)
.filter((name) => /\.(exe|dll|node)$/i.test(name))
.map((name) => ({ name, imports: importedDlls(path.join(dir, name)) }));

// A guard that silently stops looking is worse than no guard: it reports "clean" for
// the rest of the project's life. Every native binary imports something — kernel32 at
// the very least — so an empty result means the parser broke, not that the file is
// self-contained. Asserted here against the real payload rather than a synthetic PE
// fixture, which would only ever prove this parser agrees with itself.
const unread = scanned.filter((entry) => entry.imports.length === 0);
if (unread.length > 0) {
throw new Error(
`Refusing to package: read no imports at all from ${unread.map((e) => e.name).join(", ")}.\n\n` +
"Every native binary imports at least kernel32, so this is a bug in importedDlls()\n" +
"(scripts/before-pack.cjs), not a self-contained binary. Fix the parser — leaving it\n" +
"is how the Visual C++ Redistributable dependency gets back into a shipped build.",
);
}

const offenders = scanned
.map((entry) => ({ name: entry.name, bad: entry.imports.filter((d) => VC_REDIST_DLL.test(d)) }))
.filter((entry) => entry.bad.length > 0);
if (offenders.length === 0) {
return;
}

throw new Error(
"Refusing to package binaries that need the Visual C++ Redistributable.\n\n" +
` looked in: ${path.relative(ROOT, dir)}\n\n` +
`${offenders.map((o) => ` - ${o.name} imports ${o.bad.join(", ")}`).join("\n")}\n\n` +
"Those DLLs are not part of Windows. On a clean image the loader kills the process\n" +
"before main() (0xC0000135) or fails require(), and the app can only report an exit\n" +
"code. It works on every developer machine, which is why this is checked here.\n\n" +
"Build against the static CRT instead:\n" +
" - CMake helpers: CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded (electron/native/wgc-capture)\n" +
" - Rust addon: -C target-feature=+crt-static (crates/.cargo/config.toml)\n\n" +
"For a third-party binary that cannot be rebuilt, ship the DLLs it needs beside it.",
);
}

function checkWinNativePayload() {
const dir = path.join(ROOT, "electron", "native", "bin", "win32-x64");
checkNativePayload({
dir: path.join(ROOT, "electron", "native", "bin", "win32-x64"),
dir,
required: WIN_REQUIRED,
osLabel: "Windows",
bundleNoun: "the installer",
emptyDirFix: `${FIX}\n\nThe STT helper and the capture helper are separate builds — see\ntechnical-documentation/engineering/build-and-packaging.md.`,
});
checkWinNoRedistDependency(dir);
}

function checkMacNativePayload(context) {
Expand Down
Loading
Loading