ElevatedDatabaseToolsRunner: Process.Kill() ineffective from medium-IL to high-IL helper — wedged helper hangs runner indefinitely

[jschick04]

Summary

The ElevatedDatabaseToolsRunner design assumes Process.Kill() reliably terminates the elevated helper process and unblocks the IPC drain loop. In the actual medium-IL → high-IL deployment, this assumption is false: a medium-IL runner generally cannot obtain PROCESS_TERMINATE on a high-IL process started via ShellExecuteEx/runas. Process.Kill() throws Win32Exception (Access Denied), which MauiElevatedHelperProcess.Kill() swallows as a warning.

Consequence chain (when the elevated helper wedges):

1. MauiElevatedHelperProcess.Kill() swallows the Win32Exception (MauiElevatedHelperProcess.cs:48-51) and returns as if it succeeded.
2. The runner sets MarkKilled() unconditionally (ElevatedDatabaseToolsRunner.cs:231, :470), so internal liveness state lies.
3. The drain loop while (await channel.Reader.WaitToReadAsync(CancellationToken.None)) (ElevatedDatabaseToolsRunner.cs:419) only exits when the pipe closes; with the helper alive and silent, it never returns.
4. The post-terminal catch arm calls await process.WaitForExitAsync(CancellationToken.None) (ElevatedDatabaseToolsRunner.cs:473) with no timeout — unbounded wait on a process that won't exit.

Net effect: a wedged elevated helper hangs the runner indefinitely, with no user-visible escape.

Why this wasn't caught in testing

FakeElevatedHelperProcess.Kill() unconditionally sets the exit TaskCompletionSource (FakeElevatedHelperProcess.cs:28-35), so all force-kill / grace / exit-wait tests pass against a fake that always dies. The medium→high-IL scenario is structurally untestable with the current fakes.

Suggested fix

1. Don't trust Process.Kill() for an elevated child:
   • Have IElevatedHelperProcess.Kill() report success/failure; only MarkKilled() on confirmed termination.
   • Treat Win32Exception (Access Denied) as a real failure, not a warning.
2. Bound the post-terminal wait at ElevatedDatabaseToolsRunner.cs:473 with a timeout.
3. Give the drain loop at :419 an overall escape (watchdog that abandons the operation and disposes the pipe rather than waiting for process death).
4. Add a fake that models Kill() as a no-op and the process staying alive — this currently hangs the runner, demonstrating the defect; the fix must make this test pass.
5. Verify Process.Kill() behavior against a real runas helper before trusting any related path.

Related minor findings (same scope)

• MarkKilled liveness lie (ElevatedDatabaseToolsRunner.cs:231, :470): set unconditionally even when Kill() silently failed.
• Same-user pipe DoS (MauiElevatedHelperProcessHost.cs:46-90): pipe name is on the helper command line; same-user attacker can win the connect race (maxNumberOfServerInstances: 1), causing legitimate helper connection to fail. Availability-only; PID check still gates payload writes. XML doc covers confidentiality but not the DoS angle.
• RegexJsonConverter unvalidated options + InfiniteMatchTimeout (RegexJsonConverter.cs:57, :60, :73-75): a crafted request could hand the high-IL helper a catastrophic-backtracking pattern with Regex.InfiniteMatchTimeout, wedging it — combined with the unkillable-helper defect, becomes an unrecoverable hang.
• Early-return cleanup paths skip drain (ElevatedDatabaseToolsRunner.cs:503-512): Hello-timeout, protocol-mismatch, wrong-first-envelope, and request-write-cancel return straight into the finally which only DisposeAsync()s the pipe; a wedged helper is orphaned.
• Documentation accuracy (ElevatedDatabaseToolsRunner.cs:14-50, IElevatedHelperProcess cleanup contract :21-27): force-kill presented as reliable last resort; given the above, this is misleading for the actual medium→high-IL deployment.

Detection

Surfaced by a §2D-style 11-category code review (rubber-duck slot, claude-opus-4.8) run on commit 72436a23 as part of the PR #567 follow-up audit pass. Full audit report archived in the session log.

Severity

Blocking when triggered (user-visible app hang with no escape). Likelihood is gated on the helper actually wedging mid-operation, which is rare but not impossible — RegexJsonConverter-driven backtracking, helper deadlock, or any unhandled exception in the helper's event loop would all trip it.