diff --git a/cli/common/unityprocess/command_error.go b/cli/common/unityprocess/command_error.go index c259dfa36d..9ef78874c5 100644 --- a/cli/common/unityprocess/command_error.go +++ b/cli/common/unityprocess/command_error.go @@ -1,6 +1,8 @@ package unityprocess import ( + "context" + "errors" "fmt" "strings" ) @@ -15,3 +17,20 @@ func commandErrorWithStderr(err error, stderr string) error { } return fmt.Errorf("%w: %s", err, trimmedStderr) } + +// focusCommandError converts a focus script failure into an actionable error. +// Why: a timeout kills the script before it can write anything to stderr, so without this +// mapping the caller only sees a bare "exit status 1" with no hint about the stalled Editor. +func focusCommandError(contextErr error, runErr error, stderr string) error { + if runErr == nil { + return nil + } + if errors.Is(contextErr, context.DeadlineExceeded) { + return fmt.Errorf( + "focusing the Unity window timed out after %s; the Unity Editor may be busy (for example during a domain reload), retry once it is responsive: %w", + FocusCommandTimeout, + runErr, + ) + } + return commandErrorWithStderr(runErr, stderr) +} diff --git a/cli/common/unityprocess/command_error_test.go b/cli/common/unityprocess/command_error_test.go index 3f1c7c3b7c..fed786e542 100644 --- a/cli/common/unityprocess/command_error_test.go +++ b/cli/common/unityprocess/command_error_test.go @@ -1,6 +1,7 @@ package unityprocess import ( + "context" "errors" "strings" "testing" @@ -25,3 +26,31 @@ func TestCommandErrorWithStderrKeepsOriginalErrorWithoutStderr(t *testing.T) { t.Fatalf("expected original error, got %v", actual) } } + +// Verifies a timed-out focus script is reported as a busy-Editor timeout instead of a bare exit status. +func TestFocusCommandErrorReportsTimeoutWhenContextDeadlineExceeded(t *testing.T) { + err := focusCommandError(context.DeadlineExceeded, errors.New("exit status 1"), "") + + if err == nil || !strings.Contains(err.Error(), "timed out") || !strings.Contains(err.Error(), "domain reload") { + t.Fatalf("expected timeout explanation, got %v", err) + } +} + +// Verifies a focus script throw keeps the stderr text without the timeout explanation. +func TestFocusCommandErrorKeepsStderrForScriptFailures(t *testing.T) { + err := focusCommandError(nil, errors.New("exit status 1"), "Windows refused to bring the Unity window\r\n") + + if err == nil || !strings.Contains(err.Error(), "Windows refused to bring the Unity window") { + t.Fatalf("expected stderr in error, got %v", err) + } + if strings.Contains(err.Error(), "timed out") { + t.Fatalf("expected no timeout explanation, got %v", err) + } +} + +// Verifies a nil run error yields no focus error. +func TestFocusCommandErrorReturnsNilWithoutRunError(t *testing.T) { + if err := focusCommandError(context.DeadlineExceeded, nil, ""); err != nil { + t.Fatalf("expected nil, got %v", err) + } +} diff --git a/cli/common/unityprocess/focus_unity_process.ps1 b/cli/common/unityprocess/focus_unity_process.ps1 index 64d904e43f..89519c3141 100644 --- a/cli/common/unityprocess/focus_unity_process.ps1 +++ b/cli/common/unityprocess/focus_unity_process.ps1 @@ -3,18 +3,73 @@ Add-Type -TypeDefinition @" using System; using System.Runtime.InteropServices; public static class Win32Interop { + [DllImport("user32.dll")] public static extern IntPtr GetForegroundWindow(); [DllImport("user32.dll")] public static extern bool SetForegroundWindow(IntPtr hWnd); [DllImport("user32.dll")] public static extern bool ShowWindowAsync(IntPtr hWnd, int nCmdShow); + [DllImport("user32.dll")] public static extern bool IsIconic(IntPtr hWnd); + [DllImport("user32.dll")] public static extern bool BringWindowToTop(IntPtr hWnd); + [DllImport("user32.dll")] public static extern uint GetWindowThreadProcessId(IntPtr hWnd, IntPtr processIdPointer); + [DllImport("kernel32.dll")] public static extern uint GetCurrentThreadId(); + [DllImport("user32.dll")] public static extern bool AttachThreadInput(uint idAttach, uint idAttachTo, bool fAttach); + [DllImport("user32.dll")] public static extern void keybd_event(byte bVk, byte bScan, uint dwFlags, UIntPtr dwExtraInfo); + public static uint GetWindowProcessId(IntPtr hWnd) { + IntPtr buffer = Marshal.AllocHGlobal(4); + try { + GetWindowThreadProcessId(hWnd, buffer); + return (uint)Marshal.ReadInt32(buffer); + } finally { + Marshal.FreeHGlobal(buffer); + } + } } "@ +# Why: SetForegroundWindow can report the switch before the shell finishes it, so poll briefly instead of a single read. +function Test-TargetForeground { + for ($attempt = 0; $attempt -lt 10; $attempt++) { + if ([Win32Interop]::GetWindowProcessId([Win32Interop]::GetForegroundWindow()) -eq {{PID}}) { return $true } + Start-Sleep -Milliseconds 50 + } + return $false +} try { $process = Get-Process -Id {{PID}} -ErrorAction Stop } catch { throw 'Unity process was not found: {{PID}}' } $handle = $process.MainWindowHandle if ($handle -eq 0) { throw 'Unity process has no main window handle: {{PID}}' } -$shown = [Win32Interop]::ShowWindowAsync($handle, 9) -if (-not $shown) { throw 'Failed to show Unity window' } -$focused = [Win32Interop]::SetForegroundWindow($handle) -if (-not $focused) { - $shell = New-Object -ComObject WScript.Shell - $focused = $shell.AppActivate({{PID}}) +# Why: SW_RESTORE on a non-minimized window would shrink a maximized Unity window, so restore only when minimized. +if ([Win32Interop]::IsIconic($handle)) { + $shown = [Win32Interop]::ShowWindowAsync($handle, 9) + if (-not $shown) { throw 'Failed to show Unity window' } +} +[void][Win32Interop]::SetForegroundWindow($handle) +if (-not (Test-TargetForeground)) { + # Why: the Windows foreground lock rejects SetForegroundWindow from background processes; sharing the + # foreground thread's input queue via AttachThreadInput lifts that restriction. + $currentThreadId = [Win32Interop]::GetCurrentThreadId() + $foreground = [Win32Interop]::GetForegroundWindow() + $foregroundThreadId = [Win32Interop]::GetWindowThreadProcessId($foreground, [IntPtr]::Zero) + $targetThreadId = [Win32Interop]::GetWindowThreadProcessId($handle, [IntPtr]::Zero) + $attachedForeground = $false + $attachedTarget = $false + try { + if ($foregroundThreadId -ne 0 -and $foregroundThreadId -ne $currentThreadId) { + $attachedForeground = [Win32Interop]::AttachThreadInput($currentThreadId, $foregroundThreadId, $true) + } + if ($targetThreadId -ne 0 -and $targetThreadId -ne $currentThreadId) { + $attachedTarget = [Win32Interop]::AttachThreadInput($currentThreadId, $targetThreadId, $true) + } + [void][Win32Interop]::BringWindowToTop($handle) + [void][Win32Interop]::SetForegroundWindow($handle) + } finally { + if ($attachedTarget) { [void][Win32Interop]::AttachThreadInput($currentThreadId, $targetThreadId, $false) } + if ($attachedForeground) { [void][Win32Interop]::AttachThreadInput($currentThreadId, $foregroundThreadId, $false) } + } +} +if (-not (Test-TargetForeground)) { + # Why: a transient Alt keypress makes this process the last input source, a documented workaround + # that unlocks SetForegroundWindow when AttachThreadInput alone is not enough. + [Win32Interop]::keybd_event(0x12, 0, 0, [UIntPtr]::Zero) + [void][Win32Interop]::SetForegroundWindow($handle) + [Win32Interop]::keybd_event(0x12, 0, 2, [UIntPtr]::Zero) +} +if (-not (Test-TargetForeground)) { + throw 'Windows refused to bring the Unity window (PID: {{PID}}) to the foreground (foreground lock). Click the Unity window or its taskbar icon to focus it manually.' } -if (-not $focused) { throw 'Failed to focus Unity window' } diff --git a/cli/common/unityprocess/focus_unity_process_with_restore.ps1 b/cli/common/unityprocess/focus_unity_process_with_restore.ps1 index 8c1c396e6d..69175be53e 100644 --- a/cli/common/unityprocess/focus_unity_process_with_restore.ps1 +++ b/cli/common/unityprocess/focus_unity_process_with_restore.ps1 @@ -6,18 +6,72 @@ public static class Win32Interop { [DllImport("user32.dll")] public static extern IntPtr GetForegroundWindow(); [DllImport("user32.dll")] public static extern bool SetForegroundWindow(IntPtr hWnd); [DllImport("user32.dll")] public static extern bool ShowWindowAsync(IntPtr hWnd, int nCmdShow); + [DllImport("user32.dll")] public static extern bool IsIconic(IntPtr hWnd); + [DllImport("user32.dll")] public static extern bool BringWindowToTop(IntPtr hWnd); + [DllImport("user32.dll")] public static extern uint GetWindowThreadProcessId(IntPtr hWnd, IntPtr processIdPointer); + [DllImport("kernel32.dll")] public static extern uint GetCurrentThreadId(); + [DllImport("user32.dll")] public static extern bool AttachThreadInput(uint idAttach, uint idAttachTo, bool fAttach); + [DllImport("user32.dll")] public static extern void keybd_event(byte bVk, byte bScan, uint dwFlags, UIntPtr dwExtraInfo); + public static uint GetWindowProcessId(IntPtr hWnd) { + IntPtr buffer = Marshal.AllocHGlobal(4); + try { + GetWindowThreadProcessId(hWnd, buffer); + return (uint)Marshal.ReadInt32(buffer); + } finally { + Marshal.FreeHGlobal(buffer); + } + } } "@ +# Why: SetForegroundWindow can report the switch before the shell finishes it, so poll briefly instead of a single read. +function Test-TargetForeground { + for ($attempt = 0; $attempt -lt 10; $attempt++) { + if ([Win32Interop]::GetWindowProcessId([Win32Interop]::GetForegroundWindow()) -eq {{PID}}) { return $true } + Start-Sleep -Milliseconds 50 + } + return $false +} $previous = [Win32Interop]::GetForegroundWindow() try { $process = Get-Process -Id {{PID}} -ErrorAction Stop } catch { throw 'Unity process was not found: {{PID}}' } $handle = $process.MainWindowHandle if ($handle -eq 0) { throw 'Unity process has no main window handle: {{PID}}' } -$shown = [Win32Interop]::ShowWindowAsync($handle, 9) -if (-not $shown) { throw 'Failed to show Unity window' } -$focused = [Win32Interop]::SetForegroundWindow($handle) -if (-not $focused) { - $shell = New-Object -ComObject WScript.Shell - $focused = $shell.AppActivate({{PID}}) +# Why: SW_RESTORE on a non-minimized window would shrink a maximized Unity window, so restore only when minimized. +if ([Win32Interop]::IsIconic($handle)) { + $shown = [Win32Interop]::ShowWindowAsync($handle, 9) + if (-not $shown) { throw 'Failed to show Unity window' } +} +[void][Win32Interop]::SetForegroundWindow($handle) +if (-not (Test-TargetForeground)) { + # Why: the Windows foreground lock rejects SetForegroundWindow from background processes; sharing the + # foreground thread's input queue via AttachThreadInput lifts that restriction. + $currentThreadId = [Win32Interop]::GetCurrentThreadId() + $foreground = [Win32Interop]::GetForegroundWindow() + $foregroundThreadId = [Win32Interop]::GetWindowThreadProcessId($foreground, [IntPtr]::Zero) + $targetThreadId = [Win32Interop]::GetWindowThreadProcessId($handle, [IntPtr]::Zero) + $attachedForeground = $false + $attachedTarget = $false + try { + if ($foregroundThreadId -ne 0 -and $foregroundThreadId -ne $currentThreadId) { + $attachedForeground = [Win32Interop]::AttachThreadInput($currentThreadId, $foregroundThreadId, $true) + } + if ($targetThreadId -ne 0 -and $targetThreadId -ne $currentThreadId) { + $attachedTarget = [Win32Interop]::AttachThreadInput($currentThreadId, $targetThreadId, $true) + } + [void][Win32Interop]::BringWindowToTop($handle) + [void][Win32Interop]::SetForegroundWindow($handle) + } finally { + if ($attachedTarget) { [void][Win32Interop]::AttachThreadInput($currentThreadId, $targetThreadId, $false) } + if ($attachedForeground) { [void][Win32Interop]::AttachThreadInput($currentThreadId, $foregroundThreadId, $false) } + } +} +if (-not (Test-TargetForeground)) { + # Why: a transient Alt keypress makes this process the last input source, a documented workaround + # that unlocks SetForegroundWindow when AttachThreadInput alone is not enough. + [Win32Interop]::keybd_event(0x12, 0, 0, [UIntPtr]::Zero) + [void][Win32Interop]::SetForegroundWindow($handle) + [Win32Interop]::keybd_event(0x12, 0, 2, [UIntPtr]::Zero) +} +if (-not (Test-TargetForeground)) { + throw 'Windows refused to bring the Unity window (PID: {{PID}}) to the foreground (foreground lock). Click the Unity window or its taskbar icon to focus it manually.' } -if (-not $focused) { throw 'Failed to focus Unity window' } Write-Output $previous.ToInt64() diff --git a/cli/common/unityprocess/focus_windows.go b/cli/common/unityprocess/focus_windows.go index 7591fc6b03..07e72c4da8 100644 --- a/cli/common/unityprocess/focus_windows.go +++ b/cli/common/unityprocess/focus_windows.go @@ -16,7 +16,7 @@ func FocusUnityProcess(ctx context.Context, pid int) error { command := exec.CommandContext(commandContext, windowsPowerShellCommand, "-NoProfile", "-Command", script) command.Stderr = &stderr if err := command.Run(); err != nil { - return commandErrorWithStderr(err, stderr.String()) + return focusCommandError(commandContext.Err(), err, stderr.String()) } return nil } @@ -30,7 +30,7 @@ func FocusUnityProcessWithRestore(ctx context.Context, pid int) (RestoreFocusFun command.Stderr = &stderr output, err := command.Output() if err != nil { - return nil, commandErrorWithStderr(err, stderr.String()) + return nil, focusCommandError(commandContext.Err(), err, stderr.String()) } previousHandle := parseWindowsForegroundHandle(string(output)) if previousHandle == 0 { @@ -49,7 +49,7 @@ func restoreWindowsForegroundWindow(ctx context.Context, handle int64) error { command := exec.CommandContext(commandContext, windowsPowerShellCommand, "-NoProfile", "-Command", script) command.Stderr = &stderr if err := command.Run(); err != nil { - return commandErrorWithStderr(err, stderr.String()) + return focusCommandError(commandContext.Err(), err, stderr.String()) } return nil } diff --git a/cli/common/unityprocess/process_test.go b/cli/common/unityprocess/process_test.go index f29976f524..ed92051d46 100644 --- a/cli/common/unityprocess/process_test.go +++ b/cli/common/unityprocess/process_test.go @@ -63,40 +63,57 @@ func TestExtractProjectPathSupportsEqualsAndSpaces(t *testing.T) { } } -// Verifies the embedded Windows focus script throws instead of silently returning on failures. -func TestBuildFocusUnityProcessWindowsScriptThrowsOnFailures(t *testing.T) { +// Verifies the embedded Windows focus script verifies the foreground result and throws instead of trusting API return values. +func TestBuildFocusUnityProcessWindowsScriptVerifiesForegroundAndThrowsOnFailures(t *testing.T) { script := buildFocusUnityProcessWindowsScript(123) + assertWindowsFocusScriptContract(t, script) +} + +// Verifies the embedded Windows focus-with-restore script captures the previous foreground window and shares the focus contract. +func TestBuildFocusUnityProcessWindowsWithRestoreScriptCapturesForegroundWindow(t *testing.T) { + script := buildFocusUnityProcessWindowsWithRestoreScript(123) + + assertWindowsFocusScriptContract(t, script) for _, expected := range []string{ - "throw 'Unity process was not found: 123'", - "throw 'Unity process has no main window handle: 123'", - "throw 'Failed to show Unity window'", - "$focused = $shell.AppActivate(123)", - "throw 'Failed to focus Unity window'", + "$previous = [Win32Interop]::GetForegroundWindow()", + "Write-Output $previous.ToInt64()", } { if !strings.Contains(script, expected) { t.Fatalf("script missing %q: %s", expected, script) } } - if strings.Contains(script, "catch { return }") || strings.Contains(script, "{ return }") { - t.Fatalf("script should not silently return: %s", script) - } } -// Verifies the embedded Windows focus-with-restore script captures the previous foreground window. -func TestBuildFocusUnityProcessWindowsWithRestoreScriptCapturesForegroundWindow(t *testing.T) { - script := buildFocusUnityProcessWindowsWithRestoreScript(123) - +// Asserts the shared Windows focus contract: escalation techniques, foreground verification, and no trust in AppActivate. +func assertWindowsFocusScriptContract(t *testing.T, script string) { + t.Helper() for _, expected := range []string{ - "GetForegroundWindow", - "$previous = [Win32Interop]::GetForegroundWindow()", - "Write-Output $previous.ToInt64()", - "$focused = $shell.AppActivate(123)", + "throw 'Unity process was not found: 123'", + "throw 'Unity process has no main window handle: 123'", + "if ([Win32Interop]::IsIconic($handle)) {", + "throw 'Failed to show Unity window'", + "function Test-TargetForeground", + "[Win32Interop]::GetWindowProcessId([Win32Interop]::GetForegroundWindow()) -eq 123", + "AttachThreadInput($currentThreadId, $foregroundThreadId, $true)", + "AttachThreadInput($currentThreadId, $targetThreadId, $true)", + "AttachThreadInput($currentThreadId, $targetThreadId, $false)", + "AttachThreadInput($currentThreadId, $foregroundThreadId, $false)", + "BringWindowToTop", + "keybd_event(0x12, 0, 0, [UIntPtr]::Zero)", + "keybd_event(0x12, 0, 2, [UIntPtr]::Zero)", + "throw 'Windows refused to bring the Unity window (PID: 123) to the foreground (foreground lock). Click the Unity window or its taskbar icon to focus it manually.'", } { if !strings.Contains(script, expected) { t.Fatalf("script missing %q: %s", expected, script) } } + if strings.Contains(script, "AppActivate") { + t.Fatalf("script must not trust AppActivate return values: %s", script) + } + if strings.Contains(script, "catch { return }") || strings.Contains(script, "{ return }") { + t.Fatalf("script should not silently return: %s", script) + } } // Verifies the embedded Windows restore script fails when the saved foreground window cannot be restored.