Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v3] Start At Login #3910

Open
wants to merge 7 commits into
base: v3-alpha
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions mkdocs-website/docs/en/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]

### Added
- Added Support for StartAtLogin by [ansxuman](https://github.com/ansxuman) in [#3910](https://github.com/wailsapp/wails/pull/3910)
- Support of linux packaging of deb,rpm, and arch linux packager builds by @atterpac in [#3909](https://github.com/wailsapp/wails/3909)
- Added Support for darwin universal builds and packages by [ansxuman](https://github.com/ansxuman) in [#3902](https://github.com/wailsapp/wails/pull/3902)
- Events documentation to the mkdocs webite by [atterpac](https://github.com/atterpac) in [#3867](https://github.com/wailsapp/wails/pull/3867)
Expand Down
19 changes: 19 additions & 0 deletions v3/pkg/application/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ func New(appOptions Options) *App {
result.OnShutdown(appOptions.OnShutdown)
}

result.startAtLoginOnRun = appOptions.StartAtLogin

return result
}

Expand Down Expand Up @@ -202,6 +204,8 @@ type (
GetFlags(options Options) map[string]any
isOnMainThread() bool
isDarkMode() bool
setStartAtLogin(enabled bool) error
canStartAtLogin() bool
}

runnable interface {
Expand Down Expand Up @@ -352,6 +356,8 @@ type App struct {
// Wails ApplicationEvent Listener related
wailsEventListenerLock sync.Mutex
wailsEventListeners []WailsEventListener

startAtLoginOnRun bool
}

func (a *App) handleWarning(msg string) {
Expand Down Expand Up @@ -639,6 +645,11 @@ func (a *App) Run() error {
a.impl.setIcon(a.options.Icon)
}

if err := a.SetStartAtLogin(a.startAtLoginOnRun); err != nil {
a.Logger.Error("SetStartAtLogin() failed:",
"error", err.Error())
}

err = a.impl.run()
if err != nil {
return err
Expand Down Expand Up @@ -1044,3 +1055,11 @@ func (a *App) Path(selector Path) string {
func (a *App) Paths(selector Paths) []string {
return pathdirs[selector]
}

func (a *App) SetStartAtLogin(enabled bool) error {
if !a.impl.canStartAtLogin() {
a.Logger.Warn("SetStartAtLogin: Not supported in current configuration")
return nil
}
return a.impl.setStartAtLogin(enabled)
}
46 changes: 46 additions & 0 deletions v3/pkg/application/application_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,19 @@ static const char* serializationNSDictionary(void *dict) {
import "C"
import (
"encoding/json"
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
"unsafe"

"github.com/pkg/errors"
"github.com/wailsapp/wails/v3/internal/operatingsystem"

"github.com/wailsapp/wails/v3/internal/assetserver/webview"
"github.com/wailsapp/wails/v3/pkg/events"
"github.com/wailsapp/wails/v3/pkg/mac"
)

type macosApp struct {
Expand Down Expand Up @@ -377,3 +384,42 @@ func HandleOpenFile(filePath *C.char) {
ctx: eventContext,
}
}

func (m *macosApp) setStartAtLogin(enabled bool) error {
exe, err := os.Executable()
if err != nil {
return errors.Wrap(err, "Error running os.Executable:")
}

binName := filepath.Base(exe)
if !strings.HasSuffix(exe, "/Contents/MacOS/"+binName) {
return fmt.Errorf("app needs to be running as package.app file to start at login")
}

appPath := strings.TrimSuffix(exe, "/Contents/MacOS/"+binName)
var command string
if enabled {
command = fmt.Sprintf("tell application \"System Events\" to make login item at end with properties {name: \"%s\",path:\"%s\", hidden:false}", binName, appPath)
} else {
command = fmt.Sprintf("tell application \"System Events\" to delete login item \"%s\"", binName)
}

cmd := exec.Command("osascript", "-e", command)
_, err = cmd.CombinedOutput()
return err
}
Comment on lines +388 to +410
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance error handling and add input validation

While the implementation is generally sound, there are a few areas that could be improved:

  1. The output from cmd.CombinedOutput() might contain valuable error information that's currently being discarded
  2. The binName should be validated/escaped to prevent AppleScript injection
  3. Consider checking if the login item already exists before adding it

Here's a suggested improvement:

 func (m *macosApp) setStartAtLogin(enabled bool) error {
     exe, err := os.Executable()
     if err != nil {
         return errors.Wrap(err, "Error running os.Executable:")
     }

     binName := filepath.Base(exe)
+    // Escape special characters in binName to prevent AppleScript injection
+    binName = strings.ReplaceAll(binName, "\"", "\\\"")
+
     if !strings.HasSuffix(exe, "/Contents/MacOS/"+binName) {
         return fmt.Errorf("app needs to be running as package.app file to start at login")
     }

     appPath := strings.TrimSuffix(exe, "/Contents/MacOS/"+binName)
     var command string
     if enabled {
         command = fmt.Sprintf("tell application \"System Events\" to make login item at end with properties {name: \"%s\",path:\"%s\", hidden:false}", binName, appPath)
     } else {
         command = fmt.Sprintf("tell application \"System Events\" to delete login item \"%s\"", binName)
     }

     cmd := exec.Command("osascript", "-e", command)
-    _, err = cmd.CombinedOutput()
-    return err
+    output, err := cmd.CombinedOutput()
+    if err != nil {
+        return errors.Wrapf(err, "failed to execute AppleScript command: %s", string(output))
+    }
+    return nil
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (m *macosApp) setStartAtLogin(enabled bool) error {
exe, err := os.Executable()
if err != nil {
return errors.Wrap(err, "Error running os.Executable:")
}
binName := filepath.Base(exe)
if !strings.HasSuffix(exe, "/Contents/MacOS/"+binName) {
return fmt.Errorf("app needs to be running as package.app file to start at login")
}
appPath := strings.TrimSuffix(exe, "/Contents/MacOS/"+binName)
var command string
if enabled {
command = fmt.Sprintf("tell application \"System Events\" to make login item at end with properties {name: \"%s\",path:\"%s\", hidden:false}", binName, appPath)
} else {
command = fmt.Sprintf("tell application \"System Events\" to delete login item \"%s\"", binName)
}
cmd := exec.Command("osascript", "-e", command)
_, err = cmd.CombinedOutput()
return err
}
func (m *macosApp) setStartAtLogin(enabled bool) error {
exe, err := os.Executable()
if err != nil {
return errors.Wrap(err, "Error running os.Executable:")
}
binName := filepath.Base(exe)
// Escape special characters in binName to prevent AppleScript injection
binName = strings.ReplaceAll(binName, "\"", "\\\"")
if !strings.HasSuffix(exe, "/Contents/MacOS/"+binName) {
return fmt.Errorf("app needs to be running as package.app file to start at login")
}
appPath := strings.TrimSuffix(exe, "/Contents/MacOS/"+binName)
var command string
if enabled {
command = fmt.Sprintf("tell application \"System Events\" to make login item at end with properties {name: \"%s\",path:\"%s\", hidden:false}", binName, appPath)
} else {
command = fmt.Sprintf("tell application \"System Events\" to delete login item \"%s\"", binName)
}
cmd := exec.Command("osascript", "-e", command)
output, err := cmd.CombinedOutput()
if err != nil {
return errors.Wrapf(err, "failed to execute AppleScript command: %s", string(output))
}
return nil
}


func (m *macosApp) canStartAtLogin() bool {
bundleID := mac.GetBundleID()
if bundleID == "" {
return false
}

exe, err := os.Executable()
if err != nil {
return false
}

binName := filepath.Base(exe)
return strings.HasSuffix(exe, "/Contents/MacOS/"+binName)
}
Comment on lines +388 to +425
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Missing required permissions for AppleScript automation

The implementation lacks the required NSAppleEventsUsageDescription permission in the Info.plist file, which is necessary for AppleScript automation on macOS. This may cause permission issues when trying to modify login items.

  • Add NSAppleEventsUsageDescription to Info.plist to request System Events automation permission
  • Add error handling to guide users when permissions are denied
  • Consider documenting the required permissions in the README
🔗 Analysis chain

Verify security implications and permissions

The implementation uses AppleScript to modify login items, which requires appropriate permissions. We should verify:

  1. Required permissions are documented
  2. Error messages guide users when permissions are missing
  3. No sensitive information is exposed in logs

Let's check for similar implementations and security considerations:


Let me gather more information about error handling and permission requirements.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for similar login item implementations and permission handling
rg -g '*.go' -A 5 'System Events.*login item'

# Look for permission-related code
rg -g '*.go' 'NSAppleEventsUsageDescription'

# Check for security-related comments or documentation
rg -g '*.md' -i 'login item|start at login|permission|security'

Length of output: 4515


Script:

#!/bin/bash
# Check Info.plist for permission requirements
fd -e plist -x cat {}

# Look for error handling patterns around osascript
rg -g '*.go' -B 3 -A 3 'osascript.*err'

# Search for documentation about permissions
fd -g 'README*.md' -x cat {}

Length of output: 109485

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ansxuman this is important, if true...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leaanthony Most likely it's not needed. I had tested it on my machine. I'll do one thing to be double sure, I'll build and sign it using GitHub Actions and then will check how it behaves on my machine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be needed on later versions of macOS. The rabbit may be on to something.....
https://developer.apple.com/documentation/bundleresources/information-property-list/nsappleeventsusagedescription

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leaanthony I build the package on github action and tested on my machine, it seems to be working without NSAppleEventsUsageDescription. Do you still want me to add it ?

- Macbook Air M1 running on 15.1.1 (24B91)

Relevant Screenshot

Screenshot 2024-11-28 at 9 41 01 PM Screenshot 2024-11-28 at 9 41 07 PM

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it works, then that's all good

67 changes: 67 additions & 0 deletions v3/pkg/application/application_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ import "C"
import (
"fmt"
"os"
"path/filepath"
"strings"
"sync"
"text/template"

"github.com/godbus/dbus/v5"
"github.com/wailsapp/wails/v3/internal/operatingsystem"
Expand Down Expand Up @@ -260,3 +262,68 @@ func fatalHandler(errFunc func(error)) {
// Stub for windows function
return
}

func (l *linuxApp) setStartAtLogin(enabled bool) error {
homedir, err := os.UserHomeDir()
if err != nil {
return err
}

bin, err := os.Executable()
if err != nil {
return err
}
Comment on lines +272 to +275
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Validate executable path

The executable path should be validated to ensure it exists and is executable.

 	bin, err := os.Executable()
 	if err != nil {
 		return err
 	}
+
+	// Validate executable
+	if info, err := os.Stat(bin); err != nil {
+		return fmt.Errorf("invalid executable path: %w", err)
+	} else if info.Mode()&0111 == 0 {
+		return fmt.Errorf("file is not executable: %s", bin)
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bin, err := os.Executable()
if err != nil {
return err
}
bin, err := os.Executable()
if err != nil {
return err
}
// Validate executable
if info, err := os.Stat(bin); err != nil {
return fmt.Errorf("invalid executable path: %w", err)
} else if info.Mode()&0111 == 0 {
return fmt.Errorf("file is not executable: %s", bin)
}


name := filepath.Base(bin)
autostartFile := fmt.Sprintf("%s-autostart.desktop", name)
autostartPath := filepath.Join(homedir, ".config", "autostart", autostartFile)

if !enabled {
err := os.Remove(autostartPath)
if os.IsNotExist(err) {
return nil
}
return err
}

const tpl = `[Desktop Entry]
Name={{.Name}}
Comment=Autostart service for {{.Name}}
Type=Application
Exec={{.Cmd}}
Hidden=true
X-GNOME-Autostart-enabled=true
`
if err := os.MkdirAll(filepath.Dir(autostartPath), 0755); err != nil {
return err
}

file, err := os.OpenFile(autostartPath, os.O_RDWR|os.O_CREATE, 0600)
if err != nil {
return err
}
defer file.Close()

t := template.Must(template.New("autostart").Parse(tpl))
return t.Execute(file, struct {
Name string
Cmd string
}{
Name: name,
Cmd: bin,
})
}
Comment on lines +266 to +315
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add security measures against path manipulation

The implementation should include additional security measures:

  1. Resolve any symlinks in the executable path
  2. Validate the final path is within expected boundaries
  3. Sanitize the application name used in the file path
 	bin, err := os.Executable()
 	if err != nil {
 		return err
 	}
+
+	// Resolve symlinks and validate path
+	bin, err = filepath.EvalSymlinks(bin)
+	if err != nil {
+		return fmt.Errorf("failed to resolve executable path: %w", err)
+	}
+
+	// Sanitize name
+	name := filepath.Base(bin)
+	name = strings.Map(func(r rune) rune {
+		if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '-' || r == '_' {
+			return r
+		}
+		return '_'
+	}, name)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (l *linuxApp) setStartAtLogin(enabled bool) error {
homedir, err := os.UserHomeDir()
if err != nil {
return err
}
bin, err := os.Executable()
if err != nil {
return err
}
name := filepath.Base(bin)
autostartFile := fmt.Sprintf("%s-autostart.desktop", name)
autostartPath := filepath.Join(homedir, ".config", "autostart", autostartFile)
if !enabled {
err := os.Remove(autostartPath)
if os.IsNotExist(err) {
return nil
}
return err
}
const tpl = `[Desktop Entry]
Name={{.Name}}
Comment=Autostart service for {{.Name}}
Type=Application
Exec={{.Cmd}}
Hidden=true
X-GNOME-Autostart-enabled=true
`
if err := os.MkdirAll(filepath.Dir(autostartPath), 0755); err != nil {
return err
}
file, err := os.OpenFile(autostartPath, os.O_RDWR|os.O_CREATE, 0600)
if err != nil {
return err
}
defer file.Close()
t := template.Must(template.New("autostart").Parse(tpl))
return t.Execute(file, struct {
Name string
Cmd string
}{
Name: name,
Cmd: bin,
})
}
func (l *linuxApp) setStartAtLogin(enabled bool) error {
homedir, err := os.UserHomeDir()
if err != nil {
return err
}
bin, err := os.Executable()
if err != nil {
return err
}
// Resolve symlinks and validate path
bin, err = filepath.EvalSymlinks(bin)
if err != nil {
return fmt.Errorf("failed to resolve executable path: %w", err)
}
// Sanitize name
name := filepath.Base(bin)
name = strings.Map(func(r rune) rune {
if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '-' || r == '_' {
return r
}
return '_'
}, name)
autostartFile := fmt.Sprintf("%s-autostart.desktop", name)
autostartPath := filepath.Join(homedir, ".config", "autostart", autostartFile)
if !enabled {
err := os.Remove(autostartPath)
if os.IsNotExist(err) {
return nil
}
return err
}
const tpl = `[Desktop Entry]
Name={{.Name}}
Comment=Autostart service for {{.Name}}
Type=Application
Exec={{.Cmd}}
Hidden=true
X-GNOME-Autostart-enabled=true
`
if err := os.MkdirAll(filepath.Dir(autostartPath), 0755); err != nil {
return err
}
file, err := os.OpenFile(autostartPath, os.O_RDWR|os.O_CREATE, 0600)
if err != nil {
return err
}
defer file.Close()
t := template.Must(template.New("autostart").Parse(tpl))
return t.Execute(file, struct {
Name string
Cmd string
}{
Name: name,
Cmd: bin,
})
}


func (l *linuxApp) canStartAtLogin() bool {
homedir, err := os.UserHomeDir()
if err != nil {
return false
}

autostartDir := filepath.Join(homedir, ".config", "autostart")
if err := os.MkdirAll(autostartDir, 0755); err != nil {
return false
}

return true
}
Comment on lines +317 to +329
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor capability check to be non-mutating

The canStartAtLogin method currently creates directories, which is unexpected behavior for a capability check. This should be split into separate "check" and "ensure" operations.

 func (l *linuxApp) canStartAtLogin() bool {
 	homedir, err := os.UserHomeDir()
 	if err != nil {
+		l.parent.error("Failed to get user home directory: %v", err)
 		return false
 	}
 
 	autostartDir := filepath.Join(homedir, ".config", "autostart")
-	if err := os.MkdirAll(autostartDir, 0755); err != nil {
+	_, err = os.Stat(autostartDir)
+	if os.IsNotExist(err) {
+		return true
+	} else if err != nil {
+		l.parent.error("Failed to check autostart directory: %v", err)
 		return false
 	}
 
 	return true
 }
+
+// ensureAutostartDir ensures the autostart directory exists
+func (l *linuxApp) ensureAutostartDir() error {
+	homedir, err := os.UserHomeDir()
+	if err != nil {
+		return fmt.Errorf("failed to get user home directory: %w", err)
+	}
+
+	autostartDir := filepath.Join(homedir, ".config", "autostart")
+	return os.MkdirAll(autostartDir, 0755)
+}

Then update setStartAtLogin to use this:

 func (l *linuxApp) setStartAtLogin(enabled bool) error {
+	if err := l.ensureAutostartDir(); err != nil {
+		return err
+	}

Committable suggestion skipped: line range outside the PR's diff.

3 changes: 3 additions & 0 deletions v3/pkg/application/application_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ type Options struct {
// The '.' is required
FileAssociations []string

// StartAtLogin determines if the application should start when the user logs in
StartAtLogin bool

// This blank field ensures types from other packages
// are never convertible to Options.
// This property, in turn, improves the accuracy of the binding generator.
Expand Down
32 changes: 32 additions & 0 deletions v3/pkg/application/application_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"slices"
"strings"
"sync"
"syscall"
"unsafe"
Expand All @@ -16,6 +17,7 @@ import (

"github.com/wailsapp/wails/v3/pkg/events"
"github.com/wailsapp/wails/v3/pkg/w32"
"golang.org/x/sys/windows/registry"
)

type windowsApp struct {
Expand Down Expand Up @@ -369,3 +371,33 @@ func fatalHandler(errFunc func(error)) {
w32.Fatal = errFunc
return
}

func (m *windowsApp) setStartAtLogin(enabled bool) error {
exePath, err := os.Executable()
if err != nil {
return fmt.Errorf("failed to get executable path: %s", err)
}

registryKey := strings.Split(filepath.Base(exePath), ".")[0]

key, err := registry.OpenKey(registry.CURRENT_USER, `Software\Microsoft\Windows\CurrentVersion\Run`, registry.ALL_ACCESS)
Comment on lines +381 to +383
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve registry key name handling and access rights

The current implementation has potential issues with registry key handling:

  1. Registry key name derivation using strings.Split is fragile
  2. ALL_ACCESS rights might be more permissive than necessary

Consider these improvements:

-registryKey := strings.Split(filepath.Base(exePath), ".")[0]
+registryKey := strings.TrimSuffix(filepath.Base(exePath), filepath.Ext(exePath))

-key, err := registry.OpenKey(registry.CURRENT_USER, `Software\Microsoft\Windows\CurrentVersion\Run`, registry.ALL_ACCESS)
+key, err := registry.OpenKey(registry.CURRENT_USER, `Software\Microsoft\Windows\CurrentVersion\Run`, registry.SET_VALUE|registry.QUERY_VALUE)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
registryKey := strings.Split(filepath.Base(exePath), ".")[0]
key, err := registry.OpenKey(registry.CURRENT_USER, `Software\Microsoft\Windows\CurrentVersion\Run`, registry.ALL_ACCESS)
registryKey := strings.TrimSuffix(filepath.Base(exePath), filepath.Ext(exePath))
key, err := registry.OpenKey(registry.CURRENT_USER, `Software\Microsoft\Windows\CurrentVersion\Run`, registry.SET_VALUE|registry.QUERY_VALUE)

if err != nil {
return fmt.Errorf("failed to open registry key: %s", err)
}
defer key.Close()

if enabled {
err = key.SetStringValue(registryKey, exePath)
} else {
err = key.DeleteValue(registryKey)
if err == registry.ErrNotExist {
return nil
}
}
return err
}

func (m *windowsApp) canStartAtLogin() bool {
// Windows generally supports start at login via registry
return true
}