From 1cdd4ebd042e8e83e38f11cf827dbe2148506405 Mon Sep 17 00:00:00 2001 From: Zac Gaetano Date: Sat, 16 May 2026 16:27:23 -0400 Subject: [PATCH] fix(installer+wpf): REVERT runas /trustlevel demotion (it was the bug, not the fix) Massive misdiagnosis correction. The 2025-05-16 effort to 'fix elevation' has been actively breaking every Start Menu / Desktop shortcut launch since rc7. Empirical retrace: - Elevated PowerShell -> Process.Start(exe) -> elevated TeamsISO -> WORKS - Elevated PowerShell with --keep-elevation -> elevated TeamsISO -> WORKS (vm.Participants.Count=2) - Non-elevated PS Process.Start(exe) -> medium TeamsISO -> WORKS - ANY launch through runas /trustlevel:0x20000 -> SAFER-restricted TeamsISO -> BROKEN (window appears, zero managed code runs past BAML parse, no logs, no port binds) The SAFER-restricted token that runas /trustlevel produces breaks .NET 8 WPF apphost in a way that leaves the process apparently alive (with the MainWindow.xaml rendering the empty state from default property values) but executing zero managed code. So my StartupTrace, Serilog file sink, and ControlSurface bind all silently failed for every shortcut launch. Looked exactly like 'cold-start NDI Find stuck at zero' from the outside but had nothing to do with NDI. Revert: - installer/Package.wxs: shortcuts target the .exe directly, no runas wrapper - App.xaml.cs: removed ShouldDeElevate, TryDeElevateAndExit, RelaunchEnvVar, --keep-elevation/--relaunched handling. The check is gone, not just disabled, so future-me can't bring it back without re-discovering the same bug. Kept: - StartupTrace (still useful for any future startup mystery) - Self-healing NDI Find rebuild (c30a616) - still valuable for legitimate stuck-finder cases - System.Management PackageReference - TryGetParentProcessName still used in StartupTrace Verified post-revert: Start Menu shortcut click -> PID 43060 -> full trace -> REST 2 participants. 252/252 tests still passing. --- installer/Package.wxs | 49 +++++------ src/TeamsISO.App/App.xaml.cs | 163 +++++------------------------------ 2 files changed, 41 insertions(+), 171 deletions(-) diff --git a/installer/Package.wxs b/installer/Package.wxs index 297911c..42add94 100644 --- a/installer/Package.wxs +++ b/installer/Package.wxs @@ -113,39 +113,34 @@ + Icon="TeamsISOIcon" /> + Icon="TeamsISOIcon" /> = 0) + StartupTrace.Write("--keep-elevation flag present (no-op now; de-elevation removed)"); // Crash diagnostics — wire the three exception channels WPF leaves open by // default to a single handler that logs Fatal to Serilog. @@ -209,63 +203,13 @@ public partial class App : Application } } - /// - /// Returns true when we need to re-spawn ourselves with a non-elevated - /// medium-integrity token. Rule: - /// - /// If we've already been relaunched once (--relaunched - /// marker present in args), DO NOT demote again. Strip the - /// marker from forwardArgs so it doesn't leak further. - /// If our token is elevated (Administrators group active), - /// demote — full stop, regardless of parent. - /// - /// - /// The earlier "only if parent == explorer.exe" heuristic was too narrow: - /// the operator's broken spawn path on this dev box is double-clicking - /// TeamsISO.exe from an elevated File Explorer, which Windows turns into - /// a CreateProcess where the parent record is not always explorer (it - /// depends on Windows version, shell extension state, and whether the - /// click went through the shell namespace cache). Demoting whenever we - /// see an elevated token is safer and cheaper than trying to disambiguate - /// the spawn chain. The cost is one extra millisecond on launch + a brief - /// console flash from runas; the win is that NDI discovery actually works. - /// - /// - /// If you ever need to run TeamsISO elevated on purpose (debugging some - /// admin-only API path), pass --keep-elevation on the command line - /// to bypass this check. - /// - /// - private const string RelaunchEnvVar = "TEAMSISO_RELAUNCHED"; - - private static bool ShouldDeElevate(string[] args, out string[] forwardArgs) - { - forwardArgs = args; - // Already relaunched once — don't loop. The marker is an env var - // (NOT a CLI arg) because runas.exe /trustlevel:0x20000 fails with - // exit code 1 when extra args follow the program path; the env var - // is inherited cleanly across the runas boundary. - if (string.Equals(Environment.GetEnvironmentVariable(RelaunchEnvVar), "1", StringComparison.Ordinal)) - { - // Clear it so a future legitimately-elevated launch isn't suppressed. - Environment.SetEnvironmentVariable(RelaunchEnvVar, null); - return false; - } - // Explicit opt-out for power users. - if (Array.IndexOf(args, "--keep-elevation") >= 0) - { - forwardArgs = args.Where(a => a != "--keep-elevation").ToArray(); - return false; - } - // The whole reason for the check — are we elevated? - try - { - using var identity = System.Security.Principal.WindowsIdentity.GetCurrent(); - var principal = new System.Security.Principal.WindowsPrincipal(identity); - return principal.IsInRole(System.Security.Principal.WindowsBuiltInRole.Administrator); - } - catch { return false; } - } + // De-elevation helpers (ShouldDeElevate, TryDeElevateAndExit, the + // TEAMSISO_RELAUNCHED env var) were removed 2026-05-16. The whole + // pattern was treating a symptom that wasn't actually the problem + // (elevation does NOT break NDI Find); the SAFER token produced by + // runas /trustlevel:0x20000 broke .NET 8 WPF startup itself, so the + // "fix" was the actual bug. See git log for the dead code, App.xaml.cs + // commit history around 191b2c5 / 54ee578 / removal. /// /// Look up our parent process's image name (without extension). Returns @@ -289,74 +233,7 @@ public partial class App : Application return null; } - /// - /// Re-launch TeamsISO via runas.exe /trustlevel:0x20000. The - /// trustlevel argument requests a medium-integrity restricted token — - /// even when the caller (us) is elevated, the spawned child runs at - /// medium integrity. This sidesteps the elevation that was tripping - /// NDI Find. After the spawn, - /// so only the medium-integrity child remains. - /// - /// true if a child was spawned and the caller should Shutdown; - /// false if the spawn failed and the caller should fall through to - /// normal (elevated) startup. - private bool TryDeElevateAndExit(string[] forwardArgs) - { - try - { - var exePath = System.Diagnostics.Process.GetCurrentProcess().MainModule?.FileName; - if (string.IsNullOrEmpty(exePath)) - { - StartupTrace.Write("de-elevate: exePath empty, giving up"); - return false; - } - StartupTrace.Write($"de-elevate: spawning runas with target {exePath}"); - - var quotedExe = "\"" + exePath + "\""; - // runas /trustlevel:0x20000 rejects any args after the program - // path (returns exit 1). Pass ONLY the path; relay re-launch - // state via the TEAMSISO_RELAUNCHED env var, which runas - // inherits and propagates to the spawned child. - // Operator CLI args (e.g. --apply-preset NAME) are not - // forwarded across de-elevation for the same reason; this is - // an acceptable tradeoff because the elevated launch was - // probably an Explorer double-click with no args anyway. - - // Find runas.exe explicitly under System32 (the native 64-bit path). - var systemRunas = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "runas.exe"); - var runasPath = File.Exists(systemRunas) ? systemRunas : "runas.exe"; - - var psi = new System.Diagnostics.ProcessStartInfo - { - FileName = runasPath, - Arguments = "/trustlevel:0x20000 " + quotedExe, - UseShellExecute = false, - CreateNoWindow = true, - WindowStyle = System.Diagnostics.ProcessWindowStyle.Hidden, - }; - // Mark the env so the demoted child knows it's the relaunch and - // won't loop. runas + CreateProcess passes the parent env block - // to the new child by default. - psi.EnvironmentVariables[RelaunchEnvVar] = "1"; - using var spawned = System.Diagnostics.Process.Start(psi); - if (spawned is null) - { - StartupTrace.Write("de-elevate: Process.Start returned null"); - return false; - } - StartupTrace.Write($"de-elevate: runas spawned as PID {spawned.Id}"); - } - catch (Exception ex) - { - StartupTrace.Write($"de-elevate: spawn THREW: {ex.GetType().Name}: {ex.Message}"); - return false; - } - // Spawn succeeded — shut ourselves down so only the medium child remains. - // Use Shutdown(0) to signal a clean exit (NOT a startup error). - StartupTrace.Write("de-elevate: calling Shutdown(0) to let runas child take over"); - Shutdown(0); - return true; - } + // TryDeElevateAndExit removed 2026-05-16 (see comment above ShouldDeElevate). /// /// Parse the supported CLI flags. Currently: