From f9c4baa720a6f263d529db7f91b0ac72fb996598 Mon Sep 17 00:00:00 2001 From: Snider Date: Fri, 17 Apr 2026 17:38:21 +0100 Subject: [PATCH] Harden display IPC result handling --- pkg/display/api.go | 48 +++++++++++--- pkg/display/api_test.go | 61 ++++++++++++++--- stubs/wails/pkg/application/browser_window.go | 65 ++++++++++--------- 3 files changed, 122 insertions(+), 52 deletions(-) diff --git a/pkg/display/api.go b/pkg/display/api.go index ea5bcf39..0d0e77c4 100644 --- a/pkg/display/api.go +++ b/pkg/display/api.go @@ -78,6 +78,10 @@ type Theme struct { IsDark bool `json:"isDark"` } +func unexpectedResultType(method string) error { + return coreerr.E(method, "unexpected result type", nil) +} + func (s *Service) GetScreens() []*Screen { r := s.Core().QUERY(screen.QueryAll{}) if !r.OK { @@ -102,7 +106,10 @@ func (s *Service) GetScreen(id string) (*Screen, error) { } return nil, nil } - scr, _ := r.Value.(*screen.Screen) + scr, ok := r.Value.(*screen.Screen) + if !ok { + return nil, unexpectedResultType("display.GetScreen") + } return screenToDisplay(scr), nil } @@ -114,7 +121,10 @@ func (s *Service) GetPrimaryScreen() (*Screen, error) { } return nil, nil } - scr, _ := r.Value.(*screen.Screen) + scr, ok := r.Value.(*screen.Screen) + if !ok { + return nil, unexpectedResultType("display.GetPrimaryScreen") + } return screenToDisplay(scr), nil } @@ -126,7 +136,10 @@ func (s *Service) GetScreenAtPoint(x, y int) (*Screen, error) { } return nil, nil } - scr, _ := r.Value.(*screen.Screen) + scr, ok := r.Value.(*screen.Screen) + if !ok { + return nil, unexpectedResultType("display.GetScreenAtPoint") + } return screenToDisplay(scr), nil } @@ -173,7 +186,10 @@ func (s *Service) OpenFileDialog(opts OpenFileOptions) ([]string, error) { } return nil, coreerr.E("display.OpenFileDialog", "dialog.openFile action failed", nil) } - paths, _ := result.Value.([]string) + paths, ok := result.Value.([]string) + if !ok { + return nil, unexpectedResultType("display.OpenFileDialog") + } return paths, nil } @@ -187,7 +203,10 @@ func (s *Service) SaveFileDialog(opts SaveFileOptions) (string, error) { } return "", coreerr.E("display.SaveFileDialog", "dialog.saveFile action failed", nil) } - path, _ := result.Value.(string) + path, ok := result.Value.(string) + if !ok { + return "", unexpectedResultType("display.SaveFileDialog") + } return path, nil } @@ -201,7 +220,10 @@ func (s *Service) OpenDirectoryDialog(opts OpenDirectoryOptions) (string, error) } return "", coreerr.E("display.OpenDirectoryDialog", "dialog.openDirectory action failed", nil) } - path, _ := result.Value.(string) + path, ok := result.Value.(string) + if !ok { + return "", unexpectedResultType("display.OpenDirectoryDialog") + } return path, nil } @@ -219,7 +241,10 @@ func (s *Service) ConfirmDialog(title, message string) (bool, error) { } return false, coreerr.E("display.ConfirmDialog", "dialog.question action failed", nil) } - button, _ := result.Value.(string) + button, ok := result.Value.(string) + if !ok { + return false, unexpectedResultType("display.ConfirmDialog") + } return button == "Yes", nil } @@ -368,7 +393,7 @@ func (s *Service) ReadClipboardImage() ([]byte, error) { } content, ok := r.Value.(clipboard.ImageContent) if !ok { - return nil, coreerr.E("display.ReadClipboardImage", "unexpected result type", nil) + return nil, unexpectedResultType("display.ReadClipboardImage") } if !content.HasImage { return nil, nil @@ -432,7 +457,10 @@ func (s *Service) RequestNotificationPermission() (bool, error) { } return false, coreerr.E("display.RequestNotificationPermission", "notification.requestPermission action failed", nil) } - granted, _ := r.Value.(bool) + granted, ok := r.Value.(bool) + if !ok { + return false, unexpectedResultType("display.RequestNotificationPermission") + } return granted, nil } @@ -446,7 +474,7 @@ func (s *Service) CheckNotificationPermission() (bool, error) { } status, ok := r.Value.(notification.PermissionStatus) if !ok { - return false, coreerr.E("display.CheckNotificationPermission", "unexpected result type", nil) + return false, unexpectedResultType("display.CheckNotificationPermission") } return status.Granted, nil } diff --git a/pkg/display/api_test.go b/pkg/display/api_test.go index 2f517c7f..e42c71f6 100644 --- a/pkg/display/api_test.go +++ b/pkg/display/api_test.go @@ -19,11 +19,11 @@ func newTestDisplayAPIService(t *testing.T) (*Service, *core.Core) { func TestDisplayAPI_screenToDisplay_Good(t *testing.T) { got := screenToDisplay(&screen.Screen{ - ID: "screen-1", - Name: "Primary", + ID: "screen-1", + Name: "Primary", ScaleFactor: 2, - Bounds: screen.Rect{X: 10, Y: 20, Width: 1920, Height: 1080}, - IsPrimary: true, + Bounds: screen.Rect{X: 10, Y: 20, Width: 1920, Height: 1080}, + IsPrimary: true, }) require.NotNil(t, got) @@ -53,7 +53,7 @@ func TestDisplayAPI_screenToDisplay_Ugly(t *testing.T) { func TestDisplayAPI_toDialogOpenFileOptions_Good(t *testing.T) { got := toDialogOpenFileOptions(OpenFileOptions{ - Title: "Pick", + Title: "Pick", DefaultDirectory: "/tmp", DefaultFilename: "report.csv", AllowMultiple: true, @@ -131,11 +131,11 @@ func TestDisplayAPI_GetScreens_Good(t *testing.T) { case screen.QueryAll: return core.Result{Value: []screen.Screen{ { - ID: "screen-1", - Name: "Primary", - Bounds: screen.Rect{X: 10, Y: 20, Width: 1920, Height: 1080}, + ID: "screen-1", + Name: "Primary", + Bounds: screen.Rect{X: 10, Y: 20, Width: 1920, Height: 1080}, ScaleFactor: 2, - IsPrimary: true, + IsPrimary: true, }, }, OK: true} default: @@ -179,6 +179,23 @@ func TestDisplayAPI_GetScreens_Ugly(t *testing.T) { assert.Nil(t, svc.GetScreens()) } +func TestDisplayAPI_GetScreen_BadType(t *testing.T) { + svc, c := newTestDisplayAPIService(t) + c.RegisterQuery(func(_ *core.Core, q core.Query) core.Result { + switch q.(type) { + case screen.QueryByID: + return core.Result{Value: "unexpected", OK: true} + default: + return core.Result{} + } + }) + + got, err := svc.GetScreen("screen-1") + + require.Error(t, err) + assert.Nil(t, got) +} + func TestDisplayAPI_OpenFileDialog_Good(t *testing.T) { svc, c := newTestDisplayAPIService(t) c.Action("dialog.openFile", func(_ context.Context, opts core.Options) core.Result { @@ -197,6 +214,18 @@ func TestDisplayAPI_OpenFileDialog_Good(t *testing.T) { assert.Equal(t, []string{"/tmp/a.txt", "/tmp/b.txt"}, paths) } +func TestDisplayAPI_OpenFileDialog_BadType(t *testing.T) { + svc, c := newTestDisplayAPIService(t) + c.Action("dialog.openFile", func(_ context.Context, _ core.Options) core.Result { + return core.Result{Value: 42, OK: true} + }) + + paths, err := svc.OpenFileDialog(OpenFileOptions{}) + + require.Error(t, err) + assert.Nil(t, paths) +} + func TestDisplayAPI_OpenFileDialog_Bad(t *testing.T) { svc, c := newTestDisplayAPIService(t) c.Action("dialog.openFile", func(_ context.Context, _ core.Options) core.Result { @@ -217,10 +246,22 @@ func TestDisplayAPI_OpenFileDialog_Ugly(t *testing.T) { paths, err := svc.OpenFileDialog(OpenFileOptions{}) - require.NoError(t, err) + require.Error(t, err) assert.Nil(t, paths) } +func TestDisplayAPI_RequestNotificationPermission_BadType(t *testing.T) { + svc, c := newTestDisplayAPIService(t) + c.Action("notification.requestPermission", func(_ context.Context, _ core.Options) core.Result { + return core.Result{Value: "unexpected", OK: true} + }) + + granted, err := svc.RequestNotificationPermission() + + require.Error(t, err) + assert.False(t, granted) +} + func TestDisplayAPI_GetTheme_Good(t *testing.T) { svc, c := newTestDisplayAPIService(t) c.RegisterQuery(func(_ *core.Core, q core.Query) core.Result { diff --git a/stubs/wails/pkg/application/browser_window.go b/stubs/wails/pkg/application/browser_window.go index 8bef2a4e..545ebf26 100644 --- a/stubs/wails/pkg/application/browser_window.go +++ b/stubs/wails/pkg/application/browser_window.go @@ -1,6 +1,7 @@ package application import ( + "fmt" "sync" "unsafe" @@ -46,7 +47,7 @@ type BrowserWindow struct { func NewBrowserWindow(id uint, clientID string) *BrowserWindow { return &BrowserWindow{ id: id, - name: "browser-" + string(rune('0'+id%10)), + name: fmt.Sprintf("browser-%d", id), clientID: clientID, } } @@ -64,15 +65,15 @@ func (browserWindow *BrowserWindow) Error(message string, arguments ...any) {} func (browserWindow *BrowserWindow) Info(message string, arguments ...any) {} // No-op methods — browser windows are controlled via WebSocket, not native APIs. -func (browserWindow *BrowserWindow) Center() {} -func (browserWindow *BrowserWindow) Close() {} -func (browserWindow *BrowserWindow) DisableSizeConstraints() {} -func (browserWindow *BrowserWindow) EnableSizeConstraints() {} +func (browserWindow *BrowserWindow) Center() {} +func (browserWindow *BrowserWindow) Close() {} +func (browserWindow *BrowserWindow) DisableSizeConstraints() {} +func (browserWindow *BrowserWindow) EnableSizeConstraints() {} func (browserWindow *BrowserWindow) ExecJS(javascript string) {} -func (browserWindow *BrowserWindow) Focus() {} -func (browserWindow *BrowserWindow) ForceReload() {} -func (browserWindow *BrowserWindow) Fullscreen() Window { return browserWindow } -func (browserWindow *BrowserWindow) GetBorderSizes() *LRTB { return nil } +func (browserWindow *BrowserWindow) Focus() {} +func (browserWindow *BrowserWindow) ForceReload() {} +func (browserWindow *BrowserWindow) Fullscreen() Window { return browserWindow } +func (browserWindow *BrowserWindow) GetBorderSizes() *LRTB { return nil } func (browserWindow *BrowserWindow) GetScreen() (*Screen, error) { return nil, nil } @@ -95,7 +96,7 @@ func (browserWindow *BrowserWindow) Minimise() Window { return func (browserWindow *BrowserWindow) OnWindowEvent(eventType events.WindowEventType, callback func(event *WindowEvent)) func() { return func() {} } -func (browserWindow *BrowserWindow) OpenContextMenu(data *ContextMenuData) {} +func (browserWindow *BrowserWindow) OpenContextMenu(data *ContextMenuData) {} func (browserWindow *BrowserWindow) Position() (int, int) { return 0, 0 } func (browserWindow *BrowserWindow) RelativePosition() (int, int) { return 0, 0 } func (browserWindow *BrowserWindow) Reload() {} @@ -137,27 +138,27 @@ func (browserWindow *BrowserWindow) SetURL(url string) Window { return b func (browserWindow *BrowserWindow) SetZoom(magnification float64) Window { return browserWindow } -func (browserWindow *BrowserWindow) Show() Window { return browserWindow } -func (browserWindow *BrowserWindow) ShowMenuBar() {} -func (browserWindow *BrowserWindow) Size() (int, int) { return 0, 0 } -func (browserWindow *BrowserWindow) OpenDevTools() {} -func (browserWindow *BrowserWindow) ToggleFullscreen() {} -func (browserWindow *BrowserWindow) ToggleMaximise() {} -func (browserWindow *BrowserWindow) ToggleMenuBar() {} -func (browserWindow *BrowserWindow) ToggleFrameless() {} -func (browserWindow *BrowserWindow) UnFullscreen() {} -func (browserWindow *BrowserWindow) UnMaximise() {} -func (browserWindow *BrowserWindow) UnMinimise() {} -func (browserWindow *BrowserWindow) Width() int { return 0 } -func (browserWindow *BrowserWindow) IsVisible() bool { return true } -func (browserWindow *BrowserWindow) Bounds() Rect { return Rect{} } -func (browserWindow *BrowserWindow) SetBounds(bounds Rect) {} -func (browserWindow *BrowserWindow) Zoom() {} -func (browserWindow *BrowserWindow) ZoomIn() {} -func (browserWindow *BrowserWindow) ZoomOut() {} -func (browserWindow *BrowserWindow) ZoomReset() Window { return browserWindow } -func (browserWindow *BrowserWindow) SetMenu(menu *Menu) {} -func (browserWindow *BrowserWindow) SnapAssist() {} +func (browserWindow *BrowserWindow) Show() Window { return browserWindow } +func (browserWindow *BrowserWindow) ShowMenuBar() {} +func (browserWindow *BrowserWindow) Size() (int, int) { return 0, 0 } +func (browserWindow *BrowserWindow) OpenDevTools() {} +func (browserWindow *BrowserWindow) ToggleFullscreen() {} +func (browserWindow *BrowserWindow) ToggleMaximise() {} +func (browserWindow *BrowserWindow) ToggleMenuBar() {} +func (browserWindow *BrowserWindow) ToggleFrameless() {} +func (browserWindow *BrowserWindow) UnFullscreen() {} +func (browserWindow *BrowserWindow) UnMaximise() {} +func (browserWindow *BrowserWindow) UnMinimise() {} +func (browserWindow *BrowserWindow) Width() int { return 0 } +func (browserWindow *BrowserWindow) IsVisible() bool { return true } +func (browserWindow *BrowserWindow) Bounds() Rect { return Rect{} } +func (browserWindow *BrowserWindow) SetBounds(bounds Rect) {} +func (browserWindow *BrowserWindow) Zoom() {} +func (browserWindow *BrowserWindow) ZoomIn() {} +func (browserWindow *BrowserWindow) ZoomOut() {} +func (browserWindow *BrowserWindow) ZoomReset() Window { return browserWindow } +func (browserWindow *BrowserWindow) SetMenu(menu *Menu) {} +func (browserWindow *BrowserWindow) SnapAssist() {} func (browserWindow *BrowserWindow) SetContentProtection(protection bool) Window { return browserWindow } @@ -172,7 +173,7 @@ func (browserWindow *BrowserWindow) RegisterHook(eventType events.WindowEventTyp func (browserWindow *BrowserWindow) InitiateFrontendDropProcessing(filenames []string, x int, y int) { } -func (browserWindow *BrowserWindow) shouldUnconditionallyClose() bool { return false } +func (browserWindow *BrowserWindow) shouldUnconditionallyClose() bool { return true } func (browserWindow *BrowserWindow) cut() {} func (browserWindow *BrowserWindow) copy() {} func (browserWindow *BrowserWindow) paste() {}