fix(notify): harden init against symlink/TOCTOU writes (#616)

stations/notify/cmd/agent-notify/main.go

@@ -6,6 +6,7 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -22,6 +23,7 @@ import (
 	"github.com/escoffier-labs/agent-notify/internal/channels"
 	"github.com/escoffier-labs/agent-notify/internal/config"
 	"github.com/escoffier-labs/agent-notify/internal/router"
+	"github.com/escoffier-labs/agent-notify/internal/safeio"
 )
 
 const (
@@ -170,15 +172,15 @@ func runInit(args []string, stdout, stderr io.Writer) int {
 		fmt.Fprintln(stderr, "[agent-notify] config path is empty")
 		return exitConfig
 	}
-	if _, err := os.Stat(*configPath); err == nil && !*force {
-		fmt.Fprintf(stderr, "[agent-notify] config already exists: %s (use --force)\n", *configPath)
-		return exitConfig
-	}
 	if err := os.MkdirAll(filepath.Dir(*configPath), 0o700); err != nil {
 		fmt.Fprintf(stderr, "[agent-notify] create config dir: %v\n", err)
 		return exitConfig
 	}
-	if err := os.WriteFile(*configPath, []byte(sampleConfig()), 0o600); err != nil {
+	if err := safeio.WriteFile(*configPath, []byte(sampleConfig()), 0o600, *force); err != nil {
+		if errors.Is(err, safeio.ErrExists) {
+			fmt.Fprintf(stderr, "[agent-notify] config already exists: %s (use --force)\n", *configPath)
+			return exitConfig
+		}
 		fmt.Fprintf(stderr, "[agent-notify] write config: %v\n", err)
 		return exitConfig
 	}

stations/notify/cmd/agent-notify/main_test.go

@@ -422,6 +422,87 @@ func TestRun_InitWritesSampleConfig(t *testing.T) {
 	}
 }
 
+func TestRun_InitRefusesExistingWithoutForce(t *testing.T) {
+	cfgPath := filepath.Join(t.TempDir(), "agent-notify", "config.toml")
+	code, _, stderr := runMain(t, []string{"agent-notify", "init", "--config", cfgPath}, "", nil)
+	if code != 0 {
+		t.Fatalf("first init exit = %d, stderr = %s", code, stderr)
+	}
+	code, _, stderr = runMain(t, []string{"agent-notify", "init", "--config", cfgPath}, "", nil)
+	if code != 2 {
+		t.Fatalf("second init exit = %d, want 2 (stderr=%s)", code, stderr)
+	}
+	if !strings.Contains(stderr, cfgPath) || !strings.Contains(stderr, "use --force") {
+		t.Fatalf("stderr should name refused target and mention --force: %s", stderr)
+	}
+	if strings.Contains(stderr, "DISCORD") || strings.Contains(stderr, "TOKEN") {
+		t.Fatalf("stderr leaked secret-looking material: %s", stderr)
+	}
+}
+
+func TestRun_InitSymlinkCannotRedirectWrite(t *testing.T) {
+	dir := t.TempDir()
+	victim := filepath.Join(dir, "victim.txt")
+	if err := os.WriteFile(victim, []byte("KEEP\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	cfgPath := filepath.Join(dir, "config.toml")
+	if err := os.Symlink(victim, cfgPath); err != nil {
+		t.Skipf("symlink not supported: %v", err)
+	}
+	code, _, stderr := runMain(t, []string{"agent-notify", "init", "--config", cfgPath}, "", nil)
+	if code != 2 {
+		t.Fatalf("exit = %d, want 2 (stderr=%s)", code, stderr)
+	}
+	if !strings.Contains(stderr, cfgPath) {
+		t.Fatalf("stderr should identify refused target: %s", stderr)
+	}
+	got, err := os.ReadFile(victim)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(got) != "KEEP\n" {
+		t.Fatalf("victim overwritten via symlink: %q", got)
+	}
+}
+
+func TestRun_InitForceReplacesSymlinkWithoutTouchingTarget(t *testing.T) {
+	dir := t.TempDir()
+	victim := filepath.Join(dir, "victim.txt")
+	if err := os.WriteFile(victim, []byte("KEEP\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	cfgPath := filepath.Join(dir, "config.toml")
+	if err := os.Symlink(victim, cfgPath); err != nil {
+		t.Skipf("symlink not supported: %v", err)
+	}
+	code, _, stderr := runMain(t, []string{"agent-notify", "init", "--force", "--config", cfgPath}, "", nil)
+	if code != 0 {
+		t.Fatalf("exit = %d, stderr = %s", code, stderr)
+	}
+	fi, err := os.Lstat(cfgPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if fi.Mode()&os.ModeSymlink != 0 {
+		t.Fatal("config path still a symlink after --force")
+	}
+	body, err := os.ReadFile(cfgPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(body), "[profiles.agent-stop]") {
+		t.Fatalf("sample config missing after force: %s", body)
+	}
+	got, err := os.ReadFile(victim)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(got) != "KEEP\n" {
+		t.Fatalf("victim overwritten via --force symlink replace: %q", got)
+	}
+}
+
 func TestRun_DoctorJSONReportsMissingConfigAsUnconfigured(t *testing.T) {
 	missing := filepath.Join(t.TempDir(), "missing.toml")
 	for _, k := range []string{"DISCORD_WEBHOOK_URL", "TELEGRAM_BOT_TOKEN", "TELEGRAM_CHAT_ID", "SIGNAL_CLI_URL", "SIGNAL_FROM", "SIGNAL_TO"} {

stations/notify/internal/safeio/fsync_other.go

@@ -0,0 +1,11 @@
+//go:build !unix
+
+package safeio
+
+// fsyncParent is a no-op where directory fsync / O_NOFOLLOW are unavailable.
+// Those platforms therefore cannot refuse a symlinked parent here; Rename/Link
+// still provides the TOCTOU resistance for the final path.
+func fsyncParent(dir string) error {
+	_ = dir
+	return nil
+}

stations/notify/internal/safeio/fsync_unix.go

@@ -0,0 +1,37 @@
+//go:build unix
+
+package safeio
+
+import (
+	"fmt"
+	"path/filepath"
+	"syscall"
+)
+
+// fsyncParent opens dir without following a final-component symlink, verifies
+// it is a directory, and fsyncs it. Mirrors run_journal._fsync_directory.
+func fsyncParent(dir string) error {
+	if !supportsDirectoryFsync() {
+		return nil
+	}
+	flags := syscall.O_RDONLY | syscall.O_DIRECTORY | syscall.O_NOFOLLOW
+	fd, err := syscall.Open(dir, flags, 0)
+	if err != nil {
+		if err == syscall.ELOOP || err == syscall.ENOTDIR {
+			// Linux reports ENOTDIR (not ELOOP) for O_DIRECTORY|O_NOFOLLOW on a
+			// symlinked directory; treat both as symlink refusal.
+			return fmt.Errorf("refusing symlinked path: %s", filepath.Base(dir))
+		}
+		return err
+	}
+	defer func() { _ = syscall.Close(fd) }()
+
+	var st syscall.Stat_t
+	if err := syscall.Fstat(fd, &st); err != nil {
+		return err
+	}
+	if st.Mode&syscall.S_IFMT != syscall.S_IFDIR {
+		return fmt.Errorf("parent is not a directory: %s", filepath.Base(dir))
+	}
+	return syscall.Fsync(fd)
+}

stations/notify/internal/safeio/write.go

@@ -0,0 +1,119 @@
+// Package safeio provides race-resistant, no-follow file writes for agent-notify.
+//
+// The patterns mirror Brigade's Python helpers in localio.write_text_atomic /
+// write_text_exclusive and run_journal._open_nofollow / _fsync_directory:
+// same-directory temp, fsync the file, exclusive link or replace publish, then
+// fsync the parent directory without following a symlinked final component.
+package safeio
+
+import (
+	"errors"
+	"fmt"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"runtime"
+)
+
+// ErrExists is returned when an exclusive (non-force) write finds the
+// destination already occupied by a regular file, symlink, or other inode.
+var ErrExists = errors.New("destination already exists")
+
+var chmodTemp = func(tmp *os.File, mode os.FileMode) error {
+	return tmp.Chmod(mode)
+}
+
+// WriteFile publishes data at path with mode.
+//
+// Without force, publication is exclusive: a same-directory temp is fsynced
+// and hard-linked into place so a raced-in symlink or file at path cannot
+// redirect the write (link fails with ErrExists). With force, the temp is
+// published via Rename, which replaces a symlink at the destination rather
+// than following it. The parent directory is fsynced on POSIX after publish.
+func WriteFile(path string, data []byte, mode os.FileMode, force bool) error {
+	if path == "" {
+		return errors.New("path is empty")
+	}
+	dir := filepath.Dir(path)
+	base := filepath.Base(path)
+
+	// Refuse a symlinked parent before creating or publishing a temp file.
+	// The post-publish fsync below still makes the name durable.
+	if err := fsyncParent(dir); err != nil {
+		return fmt.Errorf("refuse parent of %s: %w", base, err)
+	}
+
+	if !force {
+		if err := refuseExisting(path); err != nil {
+			return err
+		}
+	}
+
+	tmp, err := os.CreateTemp(dir, "."+base+".*.tmp")
+	if err != nil {
+		return fmt.Errorf("create temp for %s: %w", base, err)
+	}
+	tmpName := tmp.Name()
+	cleanup := true
+	defer func() {
+		if cleanup {
+			_ = os.Remove(tmpName)
+		}
+	}()
+
+	if _, err := tmp.Write(data); err != nil {
+		_ = tmp.Close()
+		return fmt.Errorf("write temp for %s: %w", base, err)
+	}
+	if err := chmodTemp(tmp, mode); err != nil {
+		_ = tmp.Close()
+		return fmt.Errorf("chmod temp for %s: %w", base, err)
+	}
+	if err := tmp.Sync(); err != nil {
+		_ = tmp.Close()
+		return fmt.Errorf("sync temp for %s: %w", base, err)
+	}
+	if err := tmp.Close(); err != nil {
+		return fmt.Errorf("close temp for %s: %w", base, err)
+	}
+
+	if force {
+		if err := os.Rename(tmpName, path); err != nil {
+			return fmt.Errorf("replace %s: %w", base, err)
+		}
+		cleanup = false
+	} else {
+		if err := os.Link(tmpName, path); err != nil {
+			if isExist(err) {
+				return fmt.Errorf("%w: %s", ErrExists, path)
+			}
+			return fmt.Errorf("publish %s: %w", base, err)
+		}
+		// Temp remains until defer removes it; the hard link is the durable name.
+	}
+
+	if err := fsyncParent(dir); err != nil {
+		return fmt.Errorf("fsync parent of %s: %w", base, err)
+	}
+	return nil
+}
+
+func refuseExisting(path string) error {
+	fi, err := os.Lstat(path)
+	if err != nil {
+		if errors.Is(err, fs.ErrNotExist) {
+			return nil
+		}
+		return fmt.Errorf("stat %s: %w", filepath.Base(path), err)
+	}
+	_ = fi
+	return fmt.Errorf("%w: %s", ErrExists, path)
+}
+
+func isExist(err error) bool {
+	return errors.Is(err, fs.ErrExist)
+}
+
+func supportsDirectoryFsync() bool {
+	return runtime.GOOS != "windows" && runtime.GOOS != "js" && runtime.GOOS != "plan9"
+}

stations/notify/internal/safeio/write_internal_test.go

@@ -0,0 +1,48 @@
+package safeio
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+)
+
+func TestWriteFile_ChmodTargetSwappedMidWrite(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("symlink redirect tests require unix-style link semantics")
+	}
+	dir := t.TempDir()
+	victim := filepath.Join(dir, "victim.txt")
+	if err := os.WriteFile(victim, []byte("KEEP\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	stop := errors.New("stop after chmod")
+	original := chmodTemp
+	t.Cleanup(func() { chmodTemp = original })
+	chmodTemp = func(tmp *os.File, mode os.FileMode) error {
+		if err := os.Remove(tmp.Name()); err != nil {
+			return err
+		}
+		if err := os.Symlink(victim, tmp.Name()); err != nil {
+			return err
+		}
+		if err := tmp.Chmod(mode); err != nil {
+			return err
+		}
+		return stop
+	}
+
+	err := WriteFile(filepath.Join(dir, "config.toml"), []byte("SAFE\n"), 0o600, true)
+	if !errors.Is(err, stop) {
+		t.Fatalf("WriteFile error = %v, want %v", err, stop)
+	}
+	fi, err := os.Stat(victim)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := fi.Mode().Perm(); got != 0o644 {
+		t.Fatalf("victim permissions = %#o, want 0644", got)
+	}
+}