From c0a7655aabfb04e5a9d7467e2e9cc67ea5584230 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Mon, 10 Aug 2026 09:03:10 -0400 Subject: [PATCH] geenforce: kiosk watchdog must see a WINDOW, not just a process Reported from a real display: Edge closed on the desktop but still listed in Task Manager, and the watchdog never relaunched it. That is this watchdog's own bug. It asked only whether a --kiosk process existed. After an Edge update the window can be gone while the process lingers, so the check said "kiosk is up" and returned - every cycle, forever. Matching on the command line was chosen to stop a stray renderer masking a dead kiosk; it does not help when the orphan is the parent. A kiosk now counts as up only when a --kiosk process still owns a visible window (MainWindowHandle). Windowless ones are killed BEFORE relaunching: leaving them would satisfy the next cycle's check again, and a second browser would fight the first for the display. Also refuses to run as SYSTEM. MainWindowHandle is session-scoped, so a SYSTEM caller reads 0 for a perfectly healthy kiosk and would kill and relaunch it on every cycle. The task uses an interactive Users principal so this cannot normally happen; the guard makes a mis-registered task fail loudly instead of thrashing a display in a hallway. Verified on Windows this time by letting the SCHEDULED TASK do the work rather than invoking the script by hand - which is what the first version was missing: - task fired unattended (rc=0) and launched the kiosk into session 1 - the next cycle saw the healthy kiosk and did nothing, no relaunch loop - a windowless --kiosk process was killed and replaced - run as SYSTEM, it refused and the healthy kiosk survived --- plugins/geenforce/seed_display_scope.py | 55 ++++++++++++++++--- .../test_geenforce_display_seed.py | 23 ++++++++ 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/plugins/geenforce/seed_display_scope.py b/plugins/geenforce/seed_display_scope.py index 4bdf779..31970c7 100644 --- a/plugins/geenforce/seed_display_scope.py +++ b/plugins/geenforce/seed_display_scope.py @@ -352,6 +352,17 @@ function Log { -Value ('[{0}] {1}' -f (Get-Date -Format 'yyyy-MM-dd HH:mm:ss'), $Message) } +# REFUSE to run as SYSTEM. MainWindowHandle is session-scoped: a SYSTEM caller +# reads 0 for a perfectly healthy kiosk in the user's session, so this script +# would judge it dead, kill it and relaunch - every single cycle. The scheduled +# task uses an interactive Users principal precisely so this cannot happen, and +# this guard makes a mis-registered task fail loudly instead of thrashing the +# display. +if ([Security.Principal.WindowsIdentity]::GetCurrent().IsSystem) { + Log 'refusing to run as SYSTEM: window state is not readable across sessions.' + return +} + # The kiosk shortcut is the source of truth for target + arguments. $startup = Join-Path $env:ProgramData 'Microsoft\Windows\Start Menu\Programs\StartUp' $lnkPath = Join-Path $startup 'ShopDB Kiosk.lnk' @@ -375,19 +386,47 @@ if (-not $target -or -not (Test-Path -LiteralPath $target)) { return } -# Match on the COMMAND LINE, not just the image name. Edge spawns a crowd of -# msedge.exe children (renderers, GPU, crashpad) that outlive nothing useful; -# testing for "is msedge running" would leave a stray child masking a dead -# kiosk forever. -$running = $false +# A kiosk counts as UP only when a --kiosk process still owns a VISIBLE WINDOW. +# +# Two traps, and the obvious checks fall into one or the other. Testing "is +# msedge running" lets a stray renderer or crashpad child mask a dead kiosk. +# Testing the command line alone is what shipped first, and it is not enough +# either: after an Edge update the window can be gone while a --kiosk process +# lingers, so the watchdog saw "kiosk is up" and never relaunched - observed on +# a real display, Edge closed on the desktop but still listed in Task Manager. +# +# MainWindowHandle is the discriminator. A process with no window cannot be +# showing anything to the floor, whatever its command line says. +$kioskProcs = @() try { - $running = [bool](Get-CimInstance Win32_Process -Filter "Name = 'msedge.exe'" -EA SilentlyContinue | - Where-Object { $_.CommandLine -match '--kiosk' }) + $kioskProcs = @(Get-CimInstance Win32_Process -Filter "Name = 'msedge.exe'" -EA SilentlyContinue | + Where-Object { $_.CommandLine -match '--kiosk' }) } catch { Log "process query failed, assuming kiosk is up: $_" return } -if ($running) { return } + +$visible = @() +foreach ($p in $kioskProcs) { + $proc = Get-Process -Id $p.ProcessId -EA SilentlyContinue + if ($proc -and $proc.MainWindowHandle -ne 0) { $visible += $p } +} +if ($visible.Count -gt 0) { return } + +# Windowless --kiosk processes are orphans. They must be killed BEFORE +# relaunching: leaving them would keep the next cycle's check satisfied again, +# and a second browser would fight the first for the display. +if ($kioskProcs.Count -gt 0) { + Log ("found {0} windowless --kiosk process(es) - killing before relaunch" -f $kioskProcs.Count) + foreach ($p in $kioskProcs) { + try { + Stop-Process -Id $p.ProcessId -Force -EA Stop + Log (" killed pid {0}" -f $p.ProcessId) + } catch { + Log (" could not kill pid {0}: {1}" -f $p.ProcessId, $_) + } + } +} # Debounce. Edge takes a few seconds to present a --kiosk process, and a display # that fails to start would otherwise get a new browser every couple of minutes. diff --git a/tests/test_plugins/test_geenforce_display_seed.py b/tests/test_plugins/test_geenforce_display_seed.py index 5786adb..d253bce 100644 --- a/tests/test_plugins/test_geenforce_display_seed.py +++ b/tests/test_plugins/test_geenforce_display_seed.py @@ -180,3 +180,26 @@ def test_watchdog_does_nothing_without_a_kiosk_shortcut(): script = build_watchdog_script() assert 'ShopDB Kiosk.lnk' in script assert 'shopdb-kiosk-lastlaunch' in script # debounce stamp + + +def test_watchdog_requires_a_visible_window_not_just_a_process(): + """Edge can update and leave a --kiosk process behind with no window. The + first version tested the command line alone, so it saw 'kiosk is up' and + never relaunched - observed on a real display.""" + script = build_watchdog_script() + assert 'MainWindowHandle' in script + + +def test_watchdog_kills_windowless_kiosk_before_relaunching(): + """Leaving the orphan would satisfy the next cycle's check again, and two + browsers would fight over the display.""" + script = build_watchdog_script() + assert 'Stop-Process' in script + assert 'windowless' in script + + +def test_watchdog_refuses_to_run_as_system(): + """MainWindowHandle is session-scoped: SYSTEM reads 0 for a healthy kiosk, + so a mis-registered task would kill and relaunch the display every cycle.""" + script = build_watchdog_script() + assert 'IsSystem' in script