-
Notifications
You must be signed in to change notification settings - Fork 651
wrappers/internal: Sanitize systemd user services #16686
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,13 +21,20 @@ package userd | |||||||||||||||||
|
|
||||||||||||||||||
| import ( | ||||||||||||||||||
| "fmt" | ||||||||||||||||||
| "io/fs" | ||||||||||||||||||
| "os" | ||||||||||||||||||
| "path" | ||||||||||||||||||
| "path/filepath" | ||||||||||||||||||
| "strings" | ||||||||||||||||||
|
|
||||||||||||||||||
| "github.com/godbus/dbus/v5" | ||||||||||||||||||
| "github.com/godbus/dbus/v5/introspect" | ||||||||||||||||||
| "gopkg.in/ini.v1" | ||||||||||||||||||
| "gopkg.in/tomb.v2" | ||||||||||||||||||
|
|
||||||||||||||||||
| "github.com/snapcore/snapd/dbusutil" | ||||||||||||||||||
| "github.com/snapcore/snapd/logger" | ||||||||||||||||||
| "github.com/snapcore/snapd/systemd" | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| type dbusInterface interface { | ||||||||||||||||||
|
|
@@ -50,6 +57,92 @@ var userdBusNames = []string{ | |||||||||||||||||
| "io.snapcraft.Settings", | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| func clearBrokenLink(targetPath string) error { | ||||||||||||||||||
| _, err := filepath.EvalSymlinks(targetPath) | ||||||||||||||||||
| if err != nil { | ||||||||||||||||||
| switch err.(type) { | ||||||||||||||||||
| case *fs.PathError: | ||||||||||||||||||
| logger.Noticef("deleting broken link in snap service %s", targetPath) | ||||||||||||||||||
| // it's a broken link; delete it | ||||||||||||||||||
| os.Remove(targetPath) | ||||||||||||||||||
| default: | ||||||||||||||||||
|
Comment on lines
+60
to
+68
|
||||||||||||||||||
| return err | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| return nil | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| func reenableUserService(serviceName string) error { | ||||||||||||||||||
| logger.Noticef("re-enabling user service %s", serviceName) | ||||||||||||||||||
| sysd := systemd.New(systemd.UserMode, nil) | ||||||||||||||||||
| return sysd.DaemonReEnable([]string{serviceName}) | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+75
to
+79
|
||||||||||||||||||
|
|
||||||||||||||||||
| func checkServicePlacement(targetName, snapTarget string) error { | ||||||||||||||||||
| snapTargetData, err := os.ReadFile(snapTarget) | ||||||||||||||||||
| if err != nil { | ||||||||||||||||||
| return err | ||||||||||||||||||
| } | ||||||||||||||||||
| targetIni, err := ini.Load(snapTargetData) | ||||||||||||||||||
| if err != nil { | ||||||||||||||||||
| return err | ||||||||||||||||||
| } | ||||||||||||||||||
| wantedBy := "" | ||||||||||||||||||
| if section := targetIni.Section("Install"); section != nil { | ||||||||||||||||||
| wantedBy = section.Key("WantedBy").String() | ||||||||||||||||||
| } | ||||||||||||||||||
| if wantedBy != targetName { | ||||||||||||||||||
| // the symlink is in the wrong folder. Re-enable the service | ||||||||||||||||||
| // to change it. | ||||||||||||||||||
| err := reenableUserService(path.Base(snapTarget)) | ||||||||||||||||||
| if err != nil { | ||||||||||||||||||
| return err | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+90
to
+101
|
||||||||||||||||||
| return nil | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| func sanitizeUserServices() error { | ||||||||||||||||||
| // ensure that the user services enabled in the $HOME folder are | ||||||||||||||||||
| // correctly placed in the right .target.wants folder. This placement | ||||||||||||||||||
| // will be wrong if a service is moved from default.target to | ||||||||||||||||||
| // graphical-session.target or vice-versa, so this clean up is required. | ||||||||||||||||||
| // | ||||||||||||||||||
| // The change can happen in two cases: | ||||||||||||||||||
| // | ||||||||||||||||||
| // * when migrating from an old version of snapd without "graphical-session.target" | ||||||||||||||||||
| // support, to a new version that supports it: all the user daemons' WantedBy entry | ||||||||||||||||||
| // will be updated, and so these entries will have to be updated too. | ||||||||||||||||||
| // * when a snap adds or removes the `desktop` plug in a daemon | ||||||||||||||||||
|
|
||||||||||||||||||
| userSystemdQuery := path.Join(os.Getenv("HOME"), ".config", "systemd", "user", "*.target.wants") | ||||||||||||||||||
|
||||||||||||||||||
| userSystemdQuery := path.Join(os.Getenv("HOME"), ".config", "systemd", "user", "*.target.wants") | |
| homeDir, err := os.UserHomeDir() | |
| if err != nil || homeDir == "" { | |
| logger.Noticef("cannot determine user home directory, skipping user service sanitization: %v", err) | |
| return nil | |
| } | |
| userSystemdQuery := path.Join(homeDir, ".config", "systemd", "user", "*.target.wants") |
Copilot
AI
Feb 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New startup behavior (sanitizeUserServices, checkServicePlacement, and the systemctl reenable path) isn’t covered by unit tests, but this package already has a substantial test suite. Adding tests for at least: (1) broken symlink cleanup, and (2) wrong .target.wants placement triggering a single systemctl --user reenable call (using systemd.MockSystemctl) would help prevent regressions.
Copilot
AI
Feb 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sanitizeUserServices() returns an error but Init() ignores it. If the intent is best-effort cleanup, at least log the returned error so failures are diagnosable; otherwise, propagate the error to avoid silently leaving incorrect symlinks behind.
| sanitizeUserServices() | |
| if err := sanitizeUserServices(); err != nil { | |
| logger.Noticef("cannot sanitize user services: %v", err) | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
userd.goimportsgopkg.in/ini.v1, so this dependency is direct. Ingo.modit’s currently listed in the indirect require block with// indirect, which will causego mod tidychurn. Move it to the main require block (or drop the// indirectmarker) so module metadata matches actual usage.