fix(scale): create a missing config as owner-only and stop notifications on shutdown
A config created by a scale save is 0600, as it may hold commands. notify-send processes now end with the session. Store tests cover CRLF and comments containing = and #.
- commit
- 35f6f59ed98feb5c4c6764c7c7b97586eb0bca7b
- author
- brice · · 2026-09-27 13:57 UTC
- parent
- b931c88
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +17 -11
M internal/adapters/config/scalestore.go +2 -1
| @@ -27,7 +27,8 @@ func (s ScaleStore) SaveOutputScale(output string, scale float64) error { | |||
| 27 | 27 | } | |
| 28 | 28 | } | |
| 29 | 29 | data, err := os.ReadFile(path) | |
| 30 | - | mode := os.FileMode(0o644) | |
| 30 | + | // A new config may hold commands: only its owner reads it. | |
| 31 | + | mode := os.FileMode(0o600) | |
| 31 | 32 | switch { | |
| 32 | 33 | case errors.Is(err, os.ErrNotExist): | |
| 33 | 34 | if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { | |
| @@ -27,7 +27,8 @@ func (s ScaleStore) SaveOutputScale(output string, scale float64) error { | |||||
| 27 | } | 27 | } | ||
| 28 | } | 28 | } | ||
| 29 | data, err := os.ReadFile(path) | 29 | data, err := os.ReadFile(path) | ||
| 30 | - | mode := os.FileMode(0o644) | 30 | + | // A new config may hold commands: only its owner reads it. |
| 31 | + | mode := os.FileMode(0o600) | |||
| 31 | switch { | 32 | switch { | ||
| 32 | case errors.Is(err, os.ErrNotExist): | 33 | case errors.Is(err, os.ErrNotExist): | ||
| 33 | if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { | 34 | if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { | ||
M internal/adapters/config/scalestore_test.go +4 -1
| @@ -15,6 +15,8 @@ func TestSaveOutputScale(t *testing.T) { | |||
| 15 | 15 | {"missing file", "", "output.DP-2.scale = 1.5\n", 1.5}, | |
| 16 | 16 | {"append", "# my config\nborder.width = 3", "# my config\nborder.width = 3\noutput.DP-2.scale = 1.5\n", 1.5}, | |
| 17 | 17 | {"replace in place", "output.DP-2.scale = 2 # big\n# end\n", "output.DP-2.scale = 1.25 # big\n# end\n", 1.25}, | |
| 18 | + | {"keeps CRLF", "output.DP-2.scale = 2\r\nborder.width = 3\r\n", "output.DP-2.scale = 1.5\r\nborder.width = 3\r\n", 1.5}, | |
| 19 | + | {"comment with = and #", "output.DP-2.scale = 2 # was a=b #1\n", "output.DP-2.scale = 1.5 # was a=b #1\n", 1.5}, | |
| 18 | 20 | {"keeps spacing", "output.DP-2.scale=2\n", "output.DP-2.scale=4/3\n", 4.0 / 3}, | |
| 19 | 21 | {"last line wins", "output.DP-2.scale = 2\noutput.DP-2.scale = 3\n", "output.DP-2.scale = 2\noutput.DP-2.scale = 5/3\n", 5.0 / 3}, | |
| 20 | 22 | {"ignores comments and other outputs", "# output.DP-2.scale = 2\noutput.DP-1.scale = 2\n", "# output.DP-2.scale = 2\noutput.DP-1.scale = 2\noutput.DP-2.scale = 1\n", 1}, | |
| @@ -49,7 +51,8 @@ func TestSaveOutputScale(t *testing.T) { | |||
| 49 | 51 | t.Fatalf("parsed %v, want %v", o.Scale, tc.scale) | |
| 50 | 52 | } | |
| 51 | 53 | } | |
| 52 | - | if tc.in != "" { | |
| 54 | + | // Existing files keep their mode; new ones are private. | |
| 55 | + | { | |
| 53 | 56 | if info, _ := os.Stat(path); info.Mode().Perm() != 0o600 { | |
| 54 | 57 | t.Fatalf("mode %v", info.Mode().Perm()) | |
| 55 | 58 | } | |
| @@ -15,6 +15,8 @@ func TestSaveOutputScale(t *testing.T) { | |||||
| 15 | {"missing file", "", "output.DP-2.scale = 1.5\n", 1.5}, | 15 | {"missing file", "", "output.DP-2.scale = 1.5\n", 1.5}, | ||
| 16 | {"append", "# my config\nborder.width = 3", "# my config\nborder.width = 3\noutput.DP-2.scale = 1.5\n", 1.5}, | 16 | {"append", "# my config\nborder.width = 3", "# my config\nborder.width = 3\noutput.DP-2.scale = 1.5\n", 1.5}, | ||
| 17 | {"replace in place", "output.DP-2.scale = 2 # big\n# end\n", "output.DP-2.scale = 1.25 # big\n# end\n", 1.25}, | 17 | {"replace in place", "output.DP-2.scale = 2 # big\n# end\n", "output.DP-2.scale = 1.25 # big\n# end\n", 1.25}, | ||
| 18 | + | {"keeps CRLF", "output.DP-2.scale = 2\r\nborder.width = 3\r\n", "output.DP-2.scale = 1.5\r\nborder.width = 3\r\n", 1.5}, | |||
| 19 | + | {"comment with = and #", "output.DP-2.scale = 2 # was a=b #1\n", "output.DP-2.scale = 1.5 # was a=b #1\n", 1.5}, | |||
| 18 | {"keeps spacing", "output.DP-2.scale=2\n", "output.DP-2.scale=4/3\n", 4.0 / 3}, | 20 | {"keeps spacing", "output.DP-2.scale=2\n", "output.DP-2.scale=4/3\n", 4.0 / 3}, | ||
| 19 | {"last line wins", "output.DP-2.scale = 2\noutput.DP-2.scale = 3\n", "output.DP-2.scale = 2\noutput.DP-2.scale = 5/3\n", 5.0 / 3}, | 21 | {"last line wins", "output.DP-2.scale = 2\noutput.DP-2.scale = 3\n", "output.DP-2.scale = 2\noutput.DP-2.scale = 5/3\n", 5.0 / 3}, | ||
| 20 | {"ignores comments and other outputs", "# output.DP-2.scale = 2\noutput.DP-1.scale = 2\n", "# output.DP-2.scale = 2\noutput.DP-1.scale = 2\noutput.DP-2.scale = 1\n", 1}, | 22 | {"ignores comments and other outputs", "# output.DP-2.scale = 2\noutput.DP-1.scale = 2\n", "# output.DP-2.scale = 2\noutput.DP-1.scale = 2\noutput.DP-2.scale = 1\n", 1}, | ||
| @@ -49,7 +51,8 @@ func TestSaveOutputScale(t *testing.T) { | |||||
| 49 | t.Fatalf("parsed %v, want %v", o.Scale, tc.scale) | 51 | t.Fatalf("parsed %v, want %v", o.Scale, tc.scale) | ||
| 50 | } | 52 | } | ||
| 51 | } | 53 | } | ||
| 52 | - | if tc.in != "" { | 54 | + | // Existing files keep their mode; new ones are private. |
| 55 | + | { | |||
| 53 | if info, _ := os.Stat(path); info.Mode().Perm() != 0o600 { | 56 | if info, _ := os.Stat(path); info.Mode().Perm() != 0o600 { | ||
| 54 | t.Fatalf("mode %v", info.Mode().Perm()) | 57 | t.Fatalf("mode %v", info.Mode().Perm()) | ||
| 55 | } | 58 | } | ||
M internal/adapters/launcher/notify.go +6 -5
| @@ -11,14 +11,15 @@ import ( | |||
| 11 | 11 | // Notifier shows desktop notifications with notify-send, so any | |
| 12 | 12 | // notification daemon of the session (mako, dunst…) displays them. | |
| 13 | 13 | type Notifier struct { | |
| 14 | + | ctx context.Context | |
| 14 | 15 | env []string | |
| 15 | 16 | log zerowrap.Logger | |
| 16 | 17 | } | |
| 17 | 18 | ||
| 18 | 19 | // NewNotifier runs notify-send with the child environment, which carries | |
| 19 | - | // the session bus address. | |
| 20 | - | func NewNotifier(env []string, log zerowrap.Logger) *Notifier { | |
| 21 | - | return &Notifier{env: append([]string(nil), env...), log: log} | |
| 20 | + | // the session bus address. Ending ctx kills running notify-send processes. | |
| 21 | + | func NewNotifier(ctx context.Context, env []string, log zerowrap.Logger) *Notifier { | |
| 22 | + | return &Notifier{ctx: ctx, env: append([]string(nil), env...), log: log} | |
| 22 | 23 | } | |
| 23 | 24 | ||
| 24 | 25 | // Notify starts notify-send and returns; it is killed after five seconds. | |
| @@ -30,11 +31,11 @@ func (n *Notifier) Notify(summary, body string) { | |||
| 30 | 31 | return | |
| 31 | 32 | } | |
| 32 | 33 | go func() { | |
| 33 | - | ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | |
| 34 | + | ctx, cancel := context.WithTimeout(n.ctx, 5*time.Second) | |
| 34 | 35 | defer cancel() | |
| 35 | 36 | cmd := exec.CommandContext(ctx, path, "--app-name=neferwl", "--", summary, body) | |
| 36 | 37 | cmd.Env = n.env | |
| 37 | - | if out, err := cmd.CombinedOutput(); err != nil { | |
| 38 | + | if out, err := cmd.CombinedOutput(); err != nil && n.ctx.Err() == nil { | |
| 38 | 39 | n.log.Warn().Err(err).Str("output", string(out)).Str("summary", summary).Msg("notification failed") | |
| 39 | 40 | } | |
| 40 | 41 | }() | |
| @@ -11,14 +11,15 @@ import ( | |||||
| 11 | // Notifier shows desktop notifications with notify-send, so any | 11 | // Notifier shows desktop notifications with notify-send, so any | ||
| 12 | // notification daemon of the session (mako, dunst…) displays them. | 12 | // notification daemon of the session (mako, dunst…) displays them. | ||
| 13 | type Notifier struct { | 13 | type Notifier struct { | ||
| 14 | + | ctx context.Context | |||
| 14 | env []string | 15 | env []string | ||
| 15 | log zerowrap.Logger | 16 | log zerowrap.Logger | ||
| 16 | } | 17 | } | ||
| 17 | 18 | ||||
| 18 | // NewNotifier runs notify-send with the child environment, which carries | 19 | // NewNotifier runs notify-send with the child environment, which carries | ||
| 19 | - | // the session bus address. | 20 | + | // the session bus address. Ending ctx kills running notify-send processes. |
| 20 | - | func NewNotifier(env []string, log zerowrap.Logger) *Notifier { | 21 | + | func NewNotifier(ctx context.Context, env []string, log zerowrap.Logger) *Notifier { |
| 21 | - | return &Notifier{env: append([]string(nil), env...), log: log} | 22 | + | return &Notifier{ctx: ctx, env: append([]string(nil), env...), log: log} |
| 22 | } | 23 | } | ||
| 23 | 24 | ||||
| 24 | // Notify starts notify-send and returns; it is killed after five seconds. | 25 | // Notify starts notify-send and returns; it is killed after five seconds. | ||
| @@ -30,11 +31,11 @@ func (n *Notifier) Notify(summary, body string) { | |||||
| 30 | return | 31 | return | ||
| 31 | } | 32 | } | ||
| 32 | go func() { | 33 | go func() { | ||
| 33 | - | ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | 34 | + | ctx, cancel := context.WithTimeout(n.ctx, 5*time.Second) |
| 34 | defer cancel() | 35 | defer cancel() | ||
| 35 | cmd := exec.CommandContext(ctx, path, "--app-name=neferwl", "--", summary, body) | 36 | cmd := exec.CommandContext(ctx, path, "--app-name=neferwl", "--", summary, body) | ||
| 36 | cmd.Env = n.env | 37 | cmd.Env = n.env | ||
| 37 | - | if out, err := cmd.CombinedOutput(); err != nil { | 38 | + | if out, err := cmd.CombinedOutput(); err != nil && n.ctx.Err() == nil { |
| 38 | n.log.Warn().Err(err).Str("output", string(out)).Str("summary", summary).Msg("notification failed") | 39 | n.log.Warn().Err(err).Str("output", string(out)).Str("summary", summary).Msg("notification failed") | ||
| 39 | } | 40 | } | ||
| 40 | }() | 41 | }() | ||
M internal/adapters/launcher/notify_test.go +4 -3
| @@ -19,8 +19,9 @@ func TestNotifier(t *testing.T) { | |||
| 19 | 19 | if err := os.WriteFile(filepath.Join(dir, "notify-send"), []byte(script), 0o755); err != nil { | |
| 20 | 20 | t.Fatal(err) | |
| 21 | 21 | } | |
| 22 | - | log := logging.For(context.Background(), "launcher") | |
| 23 | - | NewNotifier([]string{"PATH=" + dir + ":/usr/bin:/bin", "DBUS_SESSION_BUS_ADDRESS=unix:path=/tmp/bus"}, log).Notify("DP-2 scale saved", "-Scale 1.5") | |
| 22 | + | ctx := context.Background() | |
| 23 | + | log := logging.For(ctx, "launcher") | |
| 24 | + | NewNotifier(ctx, []string{"PATH=" + dir + ":/usr/bin:/bin", "DBUS_SESSION_BUS_ADDRESS=unix:path=/tmp/bus"}, log).Notify("DP-2 scale saved", "-Scale 1.5") | |
| 24 | 25 | // Notify returns at once; wait for the script to record the call. | |
| 25 | 26 | data, err := os.ReadFile(out) | |
| 26 | 27 | for deadline := time.Now().Add(5 * time.Second); err != nil; data, err = os.ReadFile(out) { | |
| @@ -33,5 +34,5 @@ func TestNotifier(t *testing.T) { | |||
| 33 | 34 | t.Fatalf("got %q, want %q", data, want) | |
| 34 | 35 | } | |
| 35 | 36 | // A missing notify-send is skipped, not fatal. | |
| 36 | - | NewNotifier([]string{"PATH=" + t.TempDir()}, log).Notify("x", "y") | |
| 37 | + | NewNotifier(ctx, []string{"PATH=" + t.TempDir()}, log).Notify("x", "y") | |
| 37 | 38 | } | |
| @@ -19,8 +19,9 @@ func TestNotifier(t *testing.T) { | |||||
| 19 | if err := os.WriteFile(filepath.Join(dir, "notify-send"), []byte(script), 0o755); err != nil { | 19 | if err := os.WriteFile(filepath.Join(dir, "notify-send"), []byte(script), 0o755); err != nil { | ||
| 20 | t.Fatal(err) | 20 | t.Fatal(err) | ||
| 21 | } | 21 | } | ||
| 22 | - | log := logging.For(context.Background(), "launcher") | 22 | + | ctx := context.Background() |
| 23 | - | NewNotifier([]string{"PATH=" + dir + ":/usr/bin:/bin", "DBUS_SESSION_BUS_ADDRESS=unix:path=/tmp/bus"}, log).Notify("DP-2 scale saved", "-Scale 1.5") | 23 | + | log := logging.For(ctx, "launcher") |
| 24 | + | NewNotifier(ctx, []string{"PATH=" + dir + ":/usr/bin:/bin", "DBUS_SESSION_BUS_ADDRESS=unix:path=/tmp/bus"}, log).Notify("DP-2 scale saved", "-Scale 1.5") | |||
| 24 | // Notify returns at once; wait for the script to record the call. | 25 | // Notify returns at once; wait for the script to record the call. | ||
| 25 | data, err := os.ReadFile(out) | 26 | data, err := os.ReadFile(out) | ||
| 26 | for deadline := time.Now().Add(5 * time.Second); err != nil; data, err = os.ReadFile(out) { | 27 | for deadline := time.Now().Add(5 * time.Second); err != nil; data, err = os.ReadFile(out) { | ||
| @@ -33,5 +34,5 @@ func TestNotifier(t *testing.T) { | |||||
| 33 | t.Fatalf("got %q, want %q", data, want) | 34 | t.Fatalf("got %q, want %q", data, want) | ||
| 34 | } | 35 | } | ||
| 35 | // A missing notify-send is skipped, not fatal. | 36 | // A missing notify-send is skipped, not fatal. | ||
| 36 | - | NewNotifier([]string{"PATH=" + t.TempDir()}, log).Notify("x", "y") | 37 | + | NewNotifier(ctx, []string{"PATH=" + t.TempDir()}, log).Notify("x", "y") |
| 37 | } | 38 | } | ||
M internal/app/app.go +1 -1
| @@ -161,7 +161,7 @@ func run(ctx context.Context, opts Options, inject func(chan<- ports.InputEvent) | |||
| 161 | 161 | workers.Add(1) | |
| 162 | 162 | go func() { | |
| 163 | 163 | defer workers.Done() | |
| 164 | - | core.PersistScales(ctx, scales, config.ScaleStore{Path: path}, launcher.NewNotifier(childEnv, logging.For(ctx, "launcher")), time.Second) | |
| 164 | + | core.PersistScales(ctx, scales, config.ScaleStore{Path: path}, launcher.NewNotifier(ctx, childEnv, logging.For(ctx, "launcher")), time.Second) | |
| 165 | 165 | }() | |
| 166 | 166 | } | |
| 167 | 167 | filtered := make(chan ports.ConfigChanged, 8) | |
| @@ -161,7 +161,7 @@ func run(ctx context.Context, opts Options, inject func(chan<- ports.InputEvent) | |||||
| 161 | workers.Add(1) | 161 | workers.Add(1) | ||
| 162 | go func() { | 162 | go func() { | ||
| 163 | defer workers.Done() | 163 | defer workers.Done() | ||
| 164 | - | core.PersistScales(ctx, scales, config.ScaleStore{Path: path}, launcher.NewNotifier(childEnv, logging.For(ctx, "launcher")), time.Second) | 164 | + | core.PersistScales(ctx, scales, config.ScaleStore{Path: path}, launcher.NewNotifier(ctx, childEnv, logging.For(ctx, "launcher")), time.Second) |
| 165 | }() | 165 | }() | ||
| 166 | } | 166 | } | ||
| 167 | filtered := make(chan ports.ConfigChanged, 8) | 167 | filtered := make(chan ports.ConfigChanged, 8) | ||