Keep the scheduled run's served slot in a folder of root's alone, written never through a link

Third review of #147. The run writes as root into /var/lib/zfs-scrub (0700, root's), apart from the
operator account's error counts in /var/lib/zfs-tools, and refuses a folder that is a link, open to
others or someone else's, and a file that is a link: a new file opened O_NOFOLLOW|O_EXCL, renamed over.

modules/zfs/README.md

@@ -105,7 +105,10 @@ the last scheduled time not after now. If the slot is not yet served, the run ac
 
 It never pauses or stops a scrub. The unit has no ExecStop, so stopping the unit ends only the wait. A
 resilver that is running defers the scrub to the next run after it. The served slot is kept in
-`/var/lib/zfs-tools/scrub-served.json`. Without that file, a scrub that status shows started in the slot
+`/var/lib/zfs-scrub/scrub-served.json`, in a folder of root's alone (0700), apart from the account's state.
+The run writes it only into a real folder of its own and never through a link: a new file opened with
+`O_NOFOLLOW` and `O_EXCL`, then renamed over the old one. The tools run as the operator account and cannot
+read it, so they judge a missed scrub from status. Without that file, a scrub that status shows started in the slot
 serves it. So a scrub starts within 15 minutes of its time, and after the machine was off, within 15
 minutes of its coming back.
 

modules/zfs/cmd/zfs-tools/manifest_test.go

@@ -94,8 +94,12 @@ func TestTheHealthChecksAndWhoRunsWhat(t *testing.T) {
 		t.Errorf("the follower runs as root, judged by its unit: %v", e)
 	}
 	if d := byID["state-dir"]; d["path"] != "/var/lib/zfs-tools" || d["owner"] != "${machine:account}" ||
-		filepath.Dir(ErrorsFile) != d["path"] || filepath.Dir(ServedFile) != d["path"] {
-		t.Errorf("the state the watch and the run keep: %v", d)
+		filepath.Dir(ErrorsFile) != d["path"] {
+		t.Errorf("the state the account keeps: %v", d)
+	}
+	// The scheduled run's state is root's alone, apart from the account's (third review).
+	if d := byID["scrub-state-dir"]; d["path"] != filepath.Dir(ServedFile) || d["mode"] != "0700" || d["owner"] != nil {
+		t.Errorf("the state the run keeps: %v", d)
 	}
 	if strings.Contains(raw, `"package"`) {
 		t.Errorf("the module installs no package: ZFS is the machine's, and its kernel module is built against its kernel")

modules/zfs/cmd/zfs-tools/schedule.go

@@ -9,6 +9,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"syscall"
 	"time"
 )
 
@@ -31,8 +32,11 @@ const (
 	ScrubService = "zfs-mesh-scrub.service"
 )
 
-// ServedFile is where the scheduled run keeps the last slot it served, per pool.
-const ServedFile = "/var/lib/zfs-tools/scrub-served.json"
+// ServedFile is where the scheduled run keeps the last slot it served, per pool: in a folder of root's
+// alone (0700), apart from the operator account's state, so nothing the account runs can steer the run's
+// writes (third review of mesh-catalog #147). The tools, which run as the account, cannot read it, and
+// judge a missed scrub from status instead.
+const ServedFile = "/var/lib/zfs-scrub/scrub-served.json"
 
 // MissedAfter is how long after its slot a scrub that has not started is said to be missed: the run
 // fires every quarter of an hour.
@@ -164,6 +168,49 @@ func (m *Machine) readServed(path string) map[string]int64 {
 	return out
 }
 
+// writeServed keeps the served slots, never through a link: the folder must be a real folder of this
+// process's account alone (0700, made so when missing), and the file is written new beside it with
+// O_NOFOLLOW and O_EXCL, then renamed over the old one, which replaces a link rather than following it.
+func writeServed(path string, served map[string]int64) error {
+	dir := filepath.Dir(path)
+	if _, err := os.Lstat(dir); os.IsNotExist(err) {
+		if err := os.Mkdir(dir, 0o700); err != nil {
+			return err
+		}
+	}
+	fi, err := os.Lstat(dir)
+	if err != nil {
+		return err
+	}
+	st, _ := fi.Sys().(*syscall.Stat_t)
+	switch {
+	case fi.Mode()&os.ModeSymlink != 0 || !fi.IsDir():
+		return fmt.Errorf("%s is not a folder of its own (a link or a file): the served slot is not written", dir)
+	case fi.Mode().Perm() != 0o700:
+		return fmt.Errorf("%s is %v, not 0700: the served slot is not written", dir, fi.Mode().Perm())
+	case st != nil && int(st.Uid) != os.Geteuid():
+		return fmt.Errorf("%s belongs to uid %d, not this process's %d: the served slot is not written", dir, st.Uid, os.Geteuid())
+	}
+	if fi, err := os.Lstat(path); err == nil && !fi.Mode().IsRegular() {
+		return fmt.Errorf("%s is not a plain file (a link or something else): the served slot is not written", path)
+	}
+	b, _ := json.Marshal(served)
+	tmp := path + ".new"
+	_ = os.Remove(tmp)
+	f, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_EXCL|syscall.O_NOFOLLOW, 0o600)
+	if err != nil {
+		return err
+	}
+	if _, err := f.Write(b); err != nil {
+		f.Close()
+		return err
+	}
+	if err := f.Close(); err != nil {
+		return err
+	}
+	return os.Rename(tmp, path)
+}
+
 // startedSince says whether status shows the pool's scrub started at or after a time.
 func startedSince(p *PoolStatus, slot time.Time) bool {
 	if p == nil {
@@ -200,15 +247,7 @@ func (m *Machine) ScrubRun(ctx context.Context, servedPath string) (string, erro
 	}
 	mark := func() error {
 		served[s.Pool] = slot.Unix()
-		b, _ := json.Marshal(served)
-		if err := os.MkdirAll(filepath.Dir(servedPath), 0o755); err != nil {
-			return err
-		}
-		tmp := servedPath + ".new"
-		if err := os.WriteFile(tmp, b, 0o644); err != nil {
-			return err
-		}
-		return os.Rename(tmp, servedPath)
+		return writeServed(servedPath, served)
 	}
 	if startedSince(p, slot) && p.Scrub().State != "running" {
 		return fmt.Sprintf("the scrub of %s scheduled %s ran already", s.Pool, slot.Format("2006-01-02 15:04")), mark()

modules/zfs/cmd/zfs-tools/scrubrun_test.go

@@ -52,7 +52,7 @@ func rootFake(t *testing.T, conf string) (*fake, string) {
 	f := withZFS(t)
 	f.uid = 0
 	f.files[ScrubConfig] = conf
-	dir := t.TempDir()
+	dir := filepath.Join(t.TempDir(), "zfs-scrub")
 	return f, filepath.Join(dir, "scrub-served.json")
 }
 
@@ -166,3 +166,41 @@ func TestAScheduledScrubThatDidNotStartIsAWarning(t *testing.T) {
 		t.Errorf("warnings: %v", h.Warnings)
 	}
 }
+
+// Third review: the scheduled run's state is root's alone — a folder of its own, 0700, never through a
+// link — so the operator account, or anything it runs, cannot make the run write elsewhere.
+func TestTheServedSlotIsWrittenOnlyIntoAPrivateFolderAndNeverThroughALink(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), "zfs-scrub")
+	path := filepath.Join(dir, "scrub-served.json")
+	if err := writeServed(path, map[string]int64{"tank": 1}); err != nil {
+		t.Fatal(err)
+	}
+	if fi, err := os.Lstat(dir); err != nil || fi.Mode().Perm() != 0o700 || !fi.IsDir() {
+		t.Errorf("the folder: %v %v", fi.Mode(), err)
+	}
+	// The file replaced by a link to somewhere else: refused, and the other place untouched.
+	victim := filepath.Join(t.TempDir(), "victim")
+	os.WriteFile(victim, []byte("keep"), 0o644)
+	os.Remove(path)
+	os.Symlink(victim, path)
+	if err := writeServed(path, map[string]int64{"tank": 2}); err == nil {
+		t.Errorf("written through a link")
+	}
+	if b, _ := os.ReadFile(victim); string(b) != "keep" {
+		t.Errorf("the link's target was changed: %q", b)
+	}
+	// The folder itself a link, or open to others: refused.
+	linked := filepath.Join(t.TempDir(), "linked")
+	os.Symlink(t.TempDir(), linked)
+	if err := writeServed(filepath.Join(linked, "scrub-served.json"), map[string]int64{"tank": 3}); err == nil {
+		t.Errorf("written into a linked folder")
+	}
+	open := filepath.Join(t.TempDir(), "open")
+	os.Mkdir(open, 0o755)
+	if err := writeServed(filepath.Join(open, "scrub-served.json"), map[string]int64{"tank": 3}); err == nil {
+		t.Errorf("written into a folder others can read")
+	}
+	if filepath.Dir(ServedFile) == filepath.Dir(ErrorsFile) {
+		t.Errorf("the run's state shares the account's folder")
+	}
+}

modules/zfs/module.json

@@ -153,6 +153,12 @@
       "mode": "0644",
       "content": "# Written by the mesh (module zfs) from this machine's assignment. Replaced on every push; change the\n# settings scrub-pool, scrub-cadence, scrub-day and scrub-time, never this file. The module's tools read it.\npool=${setting:scrub-pool}\ncadence=${setting:scrub-cadence}\nday=${setting:scrub-day}\ntime=${setting:scrub-time}\n"
     },
+    {
+      "id": "scrub-state-dir",
+      "type": "directory",
+      "path": "/var/lib/zfs-scrub",
+      "mode": "0700"
+    },
     {
       "id": "scrub-run",
       "type": "process",