From 895cc0ebbd3b0186912ea52e90bd59906d477d02 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Wed, 30 Sep 2026 20:32:35 +1000 Subject: [PATCH] windows-vm.sh: review fixes for the headset key and input helper - frame-key remove deletes only the exact line add wrote, through a private temporary file, and leaves authorized_keys alone if it can't read or rewrite it. add checks the VM's key is a plain ed25519 key, keeps a last line without a newline intact, and doesn't duplicate a key the headset already trusts. - The input helper stops on any error before acknowledging, refuses a second click while one is in flight, and coordinates are bounded. - Host, container and user names are checked before they reach a remote shell; PowerShell errors come back as plain text. - docs/testing.md says what frame-key does and doesn't undo. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/testing.md | 16 ++++++++++----- scripts/windows-vm.sh | 47 ++++++++++++++++++++++++++++++------------- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index 5b120a7..8595f9e 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -205,8 +205,9 @@ scripts/windows-vm.sh down # shut Windows down - **Clicks go through a scheduled task.** Commands over SSH run in a session with no desktop, so `click` and `scroll` write the position to a file, and a scheduled task running as the signed-in user replays it. The - VM's screen must be signed in; it is after `up`. QEMU's own `mouse_move` is - relative and drifts, so the script doesn't use it. + VM's screen must be signed in; it is after `up`. One click or scroll at a + time: they share that task. QEMU's own `mouse_move` is relative and drifts, + so the script doesn't use it. - **Screenshots may not show the pointer.** Check the result of a click (a menu that opens, a button that changes) rather than the pointer's position. - **Windows' `ssh` waits for stdin.** The script closes it for every command. @@ -216,7 +217,10 @@ scripts/windows-vm.sh down # shut Windows down one `click` uses. **Testing against a real Frame.** The VM reaches the headset on the LAN like -any other computer. Set up Frame Control in the VM once, then: +any other computer. Run **Set Up Connection** in the VM's Frame Control once; +that makes the VM's key. So the VM doesn't keep access between test runs, +take that key's line out of the headset's `~/.ssh/authorized_keys` afterwards. +Then, around each run: ```sh scripts/windows-vm.sh frame-key add # let the VM's key into the headset @@ -224,8 +228,10 @@ scripts/windows-vm.sh frame-key add # let the VM's key into the headset scripts/windows-vm.sh frame-key remove # and take it out again ``` -`frame-key` tags the key `windows-vm-test` in the headset's -`authorized_keys`, and `remove` deletes only that line. Follow the shared-device +`add` appends the VM's key tagged `windows-vm-test`, unless the headset +already trusts that key through another line. `remove` deletes only the exact +line `add` wrote, so it doesn't undo what Set Up Connection did, and it leaves +the file alone if it can't rewrite it. Follow the shared-device procedure in [Headset smoke test](#headset-smoke-test) before installing or launching anything on the headset. diff --git a/scripts/windows-vm.sh b/scripts/windows-vm.sh index 441504d..d64aa47 100755 --- a/scripts/windows-vm.sh +++ b/scripts/windows-vm.sh @@ -30,14 +30,17 @@ opts=(-o ConnectTimeout=20 -o StrictHostKeyChecking=accept-new TAG=windows-vm-test # comment on the VM's key in the headset's authorized_keys die() { print -u2 "windows-vm: $*"; exit 1 } -int() { [[ $1 == (-|)<-> ]] || die "not a whole number: $1" } +int() { [[ $1 == (-|)<-99999> ]] || die "not a whole number (up to 99999): $1" } +# These go into commands run by a shell on the other machine, so keep them plain. +for v in $host $ctr $user; do [[ $v == [A-Za-z0-9_.]##[A-Za-z0-9_.-]# ]] || die "not a plain name: $v"; done +[[ $port == <1-65535> ]] || die "WINVM_PORT isn't a port: $port" # Windows' OpenSSH waits for stdin to close, so it always gets /dev/null. The # script travels UTF-16 base64-encoded, so no quoting survives two shells. vm_ps() { local b64=$(print -rn -- "\$ProgressPreference = 'SilentlyContinue'"$'\n'"$1" | iconv -f UTF-8 -t UTF-16LE | base64 | tr -d '\n') - ssh $opts -p $port $user@127.0.0.1 "powershell -NoProfile -NonInteractive -EncodedCommand $b64" /d # Input has to come from the signed-in desktop session, not SSH's session 0, so # a scheduled task running as the user replays one click or wheel turn written -# to input.txt, then deletes the file to say it's done. -INPUT_PS1='$a = (Get-Content "$PSScriptRoot\input.txt").Trim() -split "\s+" +# to input.txt, then deletes the file to say it's done. Any error stops it +# before that, so the caller times out instead of reporting a click. +INPUT_PS1='$ErrorActionPreference = "Stop" +$a = (Get-Content "$PSScriptRoot\input.txt").Trim() -split "\s+" Add-Type -Namespace WinVm -Name Input -MemberDefinition @" [DllImport("user32.dll")] public static extern bool SetProcessDPIAware(); [DllImport("user32.dll")] public static extern bool SetCursorPos(int x, int y); [DllImport("user32.dll")] public static extern void mouse_event(uint flags, int dx, int dy, int data, System.IntPtr extra); "@ [WinVm.Input]::SetProcessDPIAware() | Out-Null -[WinVm.Input]::SetCursorPos([int]$a[0], [int]$a[1]) | Out-Null +if (-not [WinVm.Input]::SetCursorPos([int]$a[0], [int]$a[1])) { throw "SetCursorPos failed" } Start-Sleep -Milliseconds 150 if ($a[2] -eq "click") { [WinVm.Input]::mouse_event(0x2, 0, 0, 0, [IntPtr]::Zero); Start-Sleep -Milliseconds 60 @@ -65,6 +70,8 @@ pointer() { # x y click|wheel [notches] local b64=$(print -rn -- $INPUT_PS1 | base64 | tr -d '\n') vm_ps '$d = Join-Path $env:LOCALAPPDATA "windows-vm" New-Item -ItemType Directory -Force $d | Out-Null +$busy = Get-Item "$d\input.txt" -ErrorAction SilentlyContinue +if ($busy -and $busy.LastWriteTime -gt (Get-Date).AddSeconds(-15)) { [Console]::Error.WriteLine("windows-vm: another click or scroll is in progress"); exit 1 } [IO.File]::WriteAllText("$d\input.ps1", [Text.Encoding]::UTF8.GetString([Convert]::FromBase64String("'$b64'"))) $act = New-ScheduledTaskAction -Execute powershell.exe -Argument "-NoProfile -WindowStyle Hidden -ExecutionPolicy Bypass -File `"$d\input.ps1`"" $who = New-ScheduledTaskPrincipal -UserId $env:USERNAME -LogonType Interactive @@ -73,7 +80,7 @@ Set-Content "$d\input.txt" "'"$*"'" Start-ScheduledTask -TaskName WindowsVmInput foreach ($i in 1..50) { if (-not (Test-Path "$d\input.txt")) { exit 0 }; Start-Sleep -Milliseconds 200 } Remove-Item "$d\input.txt" -ErrorAction SilentlyContinue -Write-Error "no input after 10 s: is $env:USERNAME signed in on the VM screen?"; exit 1' +[Console]::Error.WriteLine("windows-vm: no input after 10 s: is $env:USERNAME signed in on the VM screen, and is x,y on it?"); exit 1' } (( $# )) || die "usage: see the top of $0" @@ -115,14 +122,26 @@ open(sys.argv[1], "wb").write(b"\x89PNG\r\n\x1a\n" + chunk(b"IHDR", struct.pack( for k; do [[ $k == [a-z0-9_.,/=-]## ]] || die "not a QEMU key name: $k"; done for k; do print "sendkey $k"; done | monitor ;; frame-key) - pub=$(vm_ps 'Get-Content (Join-Path $env:USERPROFILE ".ssh\id_ed25519_frame.pub")' | tr -d '\r') - pub=${${(z)pub}[1,2]} - [[ $pub == ssh-ed25519\ * ]] || die "the VM has no Frame Control key yet: run Set Up Connection in the app first" - case ${1:-} in - add) ssh frame "grep -qxF '$pub $TAG' ~/.ssh/authorized_keys || echo '$pub $TAG' >> ~/.ssh/authorized_keys" \$f.tmp; chmod 600 \$f.tmp; mv \$f.tmp \$f" > "$f" # a last line without a newline would swallow ours + echo "$3" >> "$f" +else + t=$(mktemp "$f.XXXXXX") || exit 1 + grep -vxF "$3" "$f" > "$t" # 0: lines left, 1: none left, more: couldn't read or write + if [ $? -gt 1 ] || ! chmod 600 "$t" || ! mv "$t" "$f"; then + rm -f "$t"; echo "couldn't rewrite $f; it's unchanged" >&2; exit 1 + fi +fi +EOF print "frame-key $1: done" ;; *) die "unknown command: $cmd (see the top of $0)" ;; esac