fix(installer+wpf): REVERT runas /trustlevel demotion (it was the bug, not the fix)
Some checks failed
CI / build-and-test (push) Failing after 26s
Some checks failed
CI / build-and-test (push) Failing after 26s
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.
This commit is contained in:
parent
ea940ffac4
commit
1cdd4ebd04
2 changed files with 41 additions and 171 deletions
|
|
@ -113,39 +113,34 @@
|
|||
</ComponentGroup>
|
||||
|
||||
<!--
|
||||
Start Menu and Desktop shortcuts. The Target is intentionally
|
||||
runas.exe (NOT TeamsISO.exe directly) with the /trustlevel:0x20000
|
||||
argument. This drops the spawned TeamsISO to MEDIUM integrity even
|
||||
when the shortcut is invoked from an elevated File Explorer.
|
||||
Start Menu and Desktop shortcuts — direct .exe targets.
|
||||
|
||||
Why: on admin-user boxes with UAC effectively disabled, double-
|
||||
clicking the .lnk has explorer.exe as the spawner — and when the
|
||||
spawned TeamsISO inherits explorer's elevated token, NDI Find
|
||||
returns zero discovered sources (suspected window-station / desktop
|
||||
handle inheritance quirk in NDI's mDNS layer; not reproducible from
|
||||
any other parent). Wrapping with /trustlevel:0x20000 forces the
|
||||
child to a restricted medium-integrity token regardless of the
|
||||
invoking shell's level. Empirically verified to make discovery
|
||||
succeed on the dev box where the bug first surfaced.
|
||||
History note: an earlier revision wrapped the Target in
|
||||
runas.exe /trustlevel:0x20000 to drop the spawned TeamsISO to
|
||||
medium integrity, on the theory that elevated TeamsISO couldn't
|
||||
discover NDI sources. THAT THEORY WAS WRONG. Verified empirically
|
||||
2026-05-16: elevated TeamsISO discovers NDI sources fine
|
||||
(vm.Participants.Count=2 at +5s with the keep-elevation flag
|
||||
forcing OnStartup past the de-elevation check). The actual bug was the
|
||||
SAFER-restricted token produced by runas /trustlevel (the demotion) breaks
|
||||
.NET 8 WPF apphost startup in a way that the process appears alive
|
||||
with a window but executes zero managed code past the very first
|
||||
BAML-parse for MainWindow.xaml. No logs, no port binds, no
|
||||
controller subscription. The runas wrapper was actively causing
|
||||
every "shortcut launch shows no participants" report.
|
||||
|
||||
The /trustlevel:0x20000 magic number is the Windows "Basic User"
|
||||
SAFER trust level — documented at
|
||||
https://learn.microsoft.com/windows/security/identity-protection/windows-credential-theft-mitigation-guide-appendix#runas
|
||||
|
||||
Icon points at TeamsISO.exe (not runas) so the shortcut still shows
|
||||
the app's icon, and WindowStyle=Minimized hides the brief runas
|
||||
console flash.
|
||||
Direct .exe target. The in-app `ShouldDeElevate` check (App.xaml.cs)
|
||||
has also been removed for the same reason — letting TeamsISO run
|
||||
elevated is strictly better than re-spawning it through runas.
|
||||
-->
|
||||
<ComponentGroup Id="Shortcuts" Directory="WildDragonStartMenuFolder">
|
||||
<Component Id="StartMenuShortcut" Guid="*">
|
||||
<Shortcut Id="StartMenuTeamsISO"
|
||||
Name="TeamsISO"
|
||||
Description="Per-Participant NDI ISO Controller for Microsoft Teams"
|
||||
Target="[SystemFolder]runas.exe"
|
||||
Arguments="/trustlevel:0x20000 "[INSTALLFOLDER]TeamsISO.exe""
|
||||
Target="[INSTALLFOLDER]TeamsISO.exe"
|
||||
WorkingDirectory="INSTALLFOLDER"
|
||||
Icon="TeamsISOIcon"
|
||||
Show="minimized" />
|
||||
Icon="TeamsISOIcon" />
|
||||
<!-- Required by ICE64: Start Menu folder must be cleaned on uninstall. -->
|
||||
<RemoveFolder Id="RemoveWildDragonStartMenuFolder"
|
||||
Directory="WildDragonStartMenuFolder"
|
||||
|
|
@ -165,11 +160,9 @@
|
|||
<Shortcut Id="DesktopTeamsISO"
|
||||
Name="TeamsISO"
|
||||
Description="Per-Participant NDI ISO Controller for Microsoft Teams"
|
||||
Target="[SystemFolder]runas.exe"
|
||||
Arguments="/trustlevel:0x20000 "[INSTALLFOLDER]TeamsISO.exe""
|
||||
Target="[INSTALLFOLDER]TeamsISO.exe"
|
||||
WorkingDirectory="INSTALLFOLDER"
|
||||
Icon="TeamsISOIcon"
|
||||
Show="minimized" />
|
||||
Icon="TeamsISOIcon" />
|
||||
<RegistryValue Root="HKCU"
|
||||
Key="Software\Wild Dragon\TeamsISO"
|
||||
Name="DesktopShortcut"
|
||||
|
|
|
|||
|
|
@ -100,24 +100,18 @@ public partial class App : Application
|
|||
base.OnStartup(e);
|
||||
StartupTrace.Write("base.OnStartup returned");
|
||||
|
||||
// De-elevation check — see ShouldDeElevate doc. Trace records the decision.
|
||||
bool deElev = false;
|
||||
string[] relaunchArgs = e.Args;
|
||||
try { deElev = ShouldDeElevate(e.Args, out relaunchArgs); } catch (Exception ex) { StartupTrace.Write($"ShouldDeElevate THREW: {ex}"); }
|
||||
StartupTrace.Write($"ShouldDeElevate decision: {deElev}");
|
||||
if (deElev)
|
||||
{
|
||||
var didExit = TryDeElevateAndExit(relaunchArgs);
|
||||
if (didExit)
|
||||
{
|
||||
// Shutdown(0) was issued; let WPF tear us down. No more code runs.
|
||||
return;
|
||||
}
|
||||
// Spawn failed — fall through to normal startup as a fallback so the
|
||||
// operator at least sees a window. They may hit the elevated-launch
|
||||
// bug (no participants) but that's better than nothing.
|
||||
StartupTrace.Write("de-elevate spawn failed — falling through to normal startup as fallback");
|
||||
}
|
||||
// De-elevation via runas /trustlevel:0x20000 was tried (commits 191b2c5,
|
||||
// 54ee578) on the theory that elevated TeamsISO can't discover NDI
|
||||
// sources. THAT THEORY WAS WRONG — verified 2026-05-16 that elevated
|
||||
// TeamsISO discovers NDI sources fine. The SAFER-restricted token
|
||||
// produced by runas /trustlevel was the ACTUAL cause of every "no
|
||||
// participants" report: it breaks .NET 8 WPF startup such that the
|
||||
// process appears alive with a window but the managed code never gets
|
||||
// past BAML parsing. No logs, no port binds. We now skip the check
|
||||
// entirely. The --keep-elevation arg, originally an opt-out, is now
|
||||
// accepted but no-op'd (kept to avoid breaking any operator scripts).
|
||||
if (Array.IndexOf(e.Args, "--keep-elevation") >= 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
|
|||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns true when we need to re-spawn ourselves with a non-elevated
|
||||
/// medium-integrity token. Rule:
|
||||
/// <list type="number">
|
||||
/// <item>If we've already been relaunched once (<c>--relaunched</c>
|
||||
/// marker present in args), DO NOT demote again. Strip the
|
||||
/// marker from forwardArgs so it doesn't leak further.</item>
|
||||
/// <item>If our token is elevated (Administrators group active),
|
||||
/// demote — full stop, regardless of parent.</item>
|
||||
/// </list>
|
||||
/// <para>
|
||||
/// 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.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// If you ever need to run TeamsISO elevated on purpose (debugging some
|
||||
/// admin-only API path), pass <c>--keep-elevation</c> on the command line
|
||||
/// to bypass this check.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
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.
|
||||
|
||||
/// <summary>
|
||||
/// Look up our parent process's image name (without extension). Returns
|
||||
|
|
@ -289,74 +233,7 @@ public partial class App : Application
|
|||
return null;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Re-launch TeamsISO via <c>runas.exe /trustlevel:0x20000</c>. 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, <see cref="Application.Shutdown(int)"/>
|
||||
/// so only the medium-integrity child remains.
|
||||
/// </summary>
|
||||
/// <returns>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.</returns>
|
||||
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).
|
||||
|
||||
/// <summary>
|
||||
/// Parse the supported CLI flags. Currently:
|
||||
|
|
|
|||
Loading…
Reference in a new issue