From 4fda6a242ea2383856d48452f53dc8e78dcc04e3 Mon Sep 17 00:00:00 2001 From: Aly Raffauf Date: Fri, 19 Jun 2026 13:00:44 -0400 Subject: [PATCH] harden systemd units --- cmd/appherder/cli.go | 16 ++-- cmd/appherder/systemd.go | 82 +++++++++++++++++-- cmd/appherder/systemd/appherder-sync.path | 7 +- cmd/appherder/systemd/appherder-sync.service | 9 +- .../systemd/appherder-upgrade.service | 7 ++ cmd/appherder/systemd_test.go | 50 +++++++++++ internal/appherder/app.go | 11 +++ 7 files changed, 161 insertions(+), 21 deletions(-) create mode 100644 cmd/appherder/systemd_test.go diff --git a/cmd/appherder/cli.go b/cmd/appherder/cli.go index 93d419f..05f3ea6 100644 --- a/cmd/appherder/cli.go +++ b/cmd/appherder/cli.go @@ -33,8 +33,8 @@ func newRootCommand(a appherder.App, stdout io.Writer, stderr io.Writer) *cobra. newMigrateCommand(a), newUpgradeCommand(a), newRollbackCommand(a), - newAutosyncCommand(), - newAutoupgradeCommand(), + newAutosyncCommand(a), + newAutoupgradeCommand(a), ) return cmd } @@ -253,26 +253,26 @@ func newUnlinkCommand(a appherder.App) *cobra.Command { } } -func newAutosyncCommand() *cobra.Command { +func newAutosyncCommand(a appherder.App) *cobra.Command { return newAutoCommand( "autosync", "Enable or disable automatic sync when AppImages change", - "Installs a systemd user unit that watches ~/AppImages and runs sync whenever a\n"+ - "file is added or removed. No root required.", + "Installs a systemd user unit that watches the configured AppImages directory\n"+ + "and runs sync whenever a file is added or removed. No root required.", "Disable and remove the autosync watcher", - func() error { return enableUnits(syncUnits) }, + func() error { return enableUnits(a, syncUnits) }, func() error { return disableUnits(syncUnits) }, ) } -func newAutoupgradeCommand() *cobra.Command { +func newAutoupgradeCommand(a appherder.App) *cobra.Command { return newAutoCommand( "autoupgrade", "Enable or disable daily automatic upgrades", "Installs a systemd user timer that checks for and installs AppImage updates\n"+ "once a day. No root required.", "Disable and remove the upgrade timer", - func() error { return enableUnits(upgradeUnits) }, + func() error { return enableUnits(a, upgradeUnits) }, func() error { return disableUnits(upgradeUnits) }, ) } diff --git a/cmd/appherder/systemd.go b/cmd/appherder/systemd.go index c554c75..0b970f1 100644 --- a/cmd/appherder/systemd.go +++ b/cmd/appherder/systemd.go @@ -6,7 +6,10 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" + + "github.com/alyraffauf/appherder/internal/appherder" ) //go:embed systemd/* @@ -22,8 +25,11 @@ var upgradeUnits = []string{ "appherder-upgrade.service", } -func enableUnits(units []string) error { - if err := writeUnitFiles(units); err != nil { +func enableUnits(a appherder.App, units []string) error { + if err := ensureServiceWritePaths(a); err != nil { + return err + } + if err := writeUnitFiles(a, units); err != nil { return err } if err := runSystemctl("daemon-reload"); err != nil { @@ -32,6 +38,15 @@ func enableUnits(units []string) error { return runSystemctl("enable", "--now", units[0]) } +func ensureServiceWritePaths(a appherder.App) error { + for _, path := range a.ServiceWritePaths() { + if err := os.MkdirAll(path, 0o755); err != nil { + return fmt.Errorf("create service write directory %s: %w", path, err) + } + } + return nil +} + func disableUnits(units []string) error { if err := runSystemctl("disable", "--now", units[0]); err != nil { return err @@ -56,9 +71,9 @@ func binaryPath() (string, error) { return bin, nil } -// writeUnitFiles renders the named templates with the appherder binary path -// and writes them to the systemd user directory. -func writeUnitFiles(names []string) error { +// writeUnitFiles renders the named templates with the appherder binary and +// configured directories, then writes them to the systemd user directory. +func writeUnitFiles(a appherder.App, names []string) error { bin, err := binaryPath() if err != nil { return err @@ -75,8 +90,10 @@ func writeUnitFiles(names []string) error { if err != nil { return fmt.Errorf("read %s: %w", name, err) } - escaped := strings.ReplaceAll(bin, "%", "%%") - rendered := strings.ReplaceAll(string(data), "{{BIN}}", escaped) + rendered, err := renderUnitTemplate(string(data), bin, a) + if err != nil { + return fmt.Errorf("render %s: %w", name, err) + } dest := filepath.Join(userDir, name) if err := os.WriteFile(dest, []byte(rendered), 0o644); err != nil { return fmt.Errorf("write %s: %w", dest, err) @@ -85,6 +102,57 @@ func writeUnitFiles(names []string) error { return nil } +func renderUnitTemplate(template, bin string, a appherder.App) (string, error) { + appimagesDir, err := systemdPathValue(a.AppImagesDir()) + if err != nil { + return "", err + } + writePaths, err := systemdPathList(a.ServiceWritePaths()) + if err != nil { + return "", err + } + + rendered := strings.ReplaceAll(template, "{{BIN}}", systemdSpecifierEscape(bin)) + rendered = strings.ReplaceAll(rendered, "{{APPIMAGES_DIR}}", appimagesDir) + rendered = strings.ReplaceAll(rendered, "{{READ_WRITE_PATHS}}", writePaths) + return rendered, nil +} + +func systemdPathList(paths []string) (string, error) { + seen := make(map[string]bool, len(paths)) + values := make([]string, 0, len(paths)) + for _, path := range paths { + if seen[path] { + continue + } + seen[path] = true + value, err := systemdPathValue(path) + if err != nil { + return "", err + } + values = append(values, value) + } + return strings.Join(values, " "), nil +} + +func systemdPathValue(path string) (string, error) { + if path == "" { + return "", fmt.Errorf("empty path") + } + if strings.ContainsAny(path, "\n\r") { + return "", fmt.Errorf("path contains a newline: %q", path) + } + escaped := systemdSpecifierEscape(path) + if strings.ContainsAny(escaped, " \t\"'\\") { + return strconv.Quote(escaped), nil + } + return escaped, nil +} + +func systemdSpecifierEscape(value string) string { + return strings.ReplaceAll(value, "%", "%%") +} + func removeUnitFiles(names []string) error { userDir, err := systemdUserDir() if err != nil { diff --git a/cmd/appherder/systemd/appherder-sync.path b/cmd/appherder/systemd/appherder-sync.path index 6bdb449..f41fff9 100644 --- a/cmd/appherder/systemd/appherder-sync.path +++ b/cmd/appherder/systemd/appherder-sync.path @@ -1,11 +1,8 @@ [Unit] -Description=Watch ~/AppImages for appherder sync +Description=Watch AppImages for appherder sync [Path] -# PathChanged watches the directory and everything under it via inotify. If -# ~/AppImages does not yet exist, systemd watches its parent and picks the -# directory up when it is created. -PathChanged=%h/AppImages +PathChanged={{APPIMAGES_DIR}} Unit=appherder-sync.service [Install] diff --git a/cmd/appherder/systemd/appherder-sync.service b/cmd/appherder/systemd/appherder-sync.service index 18d137b..ac205d3 100644 --- a/cmd/appherder/systemd/appherder-sync.service +++ b/cmd/appherder/systemd/appherder-sync.service @@ -1,6 +1,13 @@ [Unit] -Description=Reconcile ~/AppImages with installed applications +Description=Reconcile AppImages with installed applications [Service] Type=oneshot ExecStart={{BIN}} sync +NoNewPrivileges=true +PrivateTmp=true +ProtectSystem=strict +RestrictSUIDSGID=true +LockPersonality=true +SystemCallArchitectures=native +ReadWritePaths={{READ_WRITE_PATHS}} diff --git a/cmd/appherder/systemd/appherder-upgrade.service b/cmd/appherder/systemd/appherder-upgrade.service index fdc9e24..7f992b4 100644 --- a/cmd/appherder/systemd/appherder-upgrade.service +++ b/cmd/appherder/systemd/appherder-upgrade.service @@ -4,3 +4,10 @@ Description=Download and install available AppImage updates [Service] Type=oneshot ExecStart={{BIN}} upgrade +NoNewPrivileges=true +PrivateTmp=true +ProtectSystem=strict +RestrictSUIDSGID=true +LockPersonality=true +SystemCallArchitectures=native +ReadWritePaths={{READ_WRITE_PATHS}} diff --git a/cmd/appherder/systemd_test.go b/cmd/appherder/systemd_test.go new file mode 100644 index 0000000..a0d9f47 --- /dev/null +++ b/cmd/appherder/systemd_test.go @@ -0,0 +1,50 @@ +package main + +import ( + "strings" + "testing" + + "github.com/alyraffauf/appherder/internal/appherder" +) + +func TestRenderUnitTemplateUsesConfiguredPaths(t *testing.T) { + app := appherder.NewAppWithDirs( + "/tmp/App Images/%apps", + "/tmp/data/applications", + "/tmp/App Images/%apps/.icons", + "/tmp/bin dir", + ) + template := strings.Join([]string{ + "ExecStart={{BIN}} sync", + "PathChanged={{APPIMAGES_DIR}}", + "ReadWritePaths={{READ_WRITE_PATHS}}", + }, "\n") + + got, err := renderUnitTemplate(template, "/opt/app%herder", app) + if err != nil { + t.Fatalf("renderUnitTemplate: %v", err) + } + + for _, want := range []string{ + "ExecStart=/opt/app%%herder sync", + `PathChanged="/tmp/App Images/%%apps"`, + `ReadWritePaths="/tmp/App Images/%%apps" /tmp/data/applications "/tmp/bin dir"`, + } { + if !strings.Contains(got, want) { + t.Fatalf("rendered unit missing %q:\n%s", want, got) + } + } +} + +func TestRenderUnitTemplateRejectsNewlinePaths(t *testing.T) { + app := appherder.NewAppWithDirs( + "/tmp/AppImages\nbad", + "/tmp/data/applications", + "/tmp/AppImages/.icons", + "/tmp/bin", + ) + + if _, err := renderUnitTemplate("PathChanged={{APPIMAGES_DIR}}", "/bin/appherder", app); err == nil { + t.Fatal("expected newline path error") + } +} diff --git a/internal/appherder/app.go b/internal/appherder/app.go index 08b7c8a..8dfe94e 100644 --- a/internal/appherder/app.go +++ b/internal/appherder/app.go @@ -56,3 +56,14 @@ func (a App) WithProgress(p Progress) App { a.progress = p return a } + +// AppImagesDir is the directory appherder manages as the source of truth. +func (a App) AppImagesDir() string { + return a.appimagesDir +} + +// ServiceWritePaths returns the directories systemd services need writable for +// sync and upgrade. Icons and saved versions live under AppImagesDir. +func (a App) ServiceWritePaths() []string { + return []string{a.appimagesDir, a.applicationsDir, a.binDir} +} -- 2.51.2