Take in an admin's edits of the state files while running (closes #68)

smallwebwaf watches SWWAF_STATE_DIR with fsnotify and takes in a saved
edit of a state file in place of what it held. It tells its own writes
from an admin's by the SHA-256 of what it last read or wrote; each write
first takes in an edit made since. An edit that does not parse is
renamed to <name>.bad at the file's next write. Every ban on a netblock
is checked, and the next ban is worked out from the one that ended
last. Two metrics count the edits taken in and set aside. README.md
says how to add and lift a ban.

Judgement call: a broken edit is set aside at the next write, since an
editor's file can be read half written.

Model: opus-5-5

go.sum

@@ -32,7 +32,6 @@ go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
 go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
 go.yaml.in/yaml/v2 v2.4.4 h1:tuyd0P+2Ont/d6e2rl3be67goVK4R6deVxCUX5vyPaQ=
 go.yaml.in/yaml/v2 v2.4.4/go.mod h1:gMZqIpDtDqOfM0uNfy0SkpRhvUryYH0Z6wdMYcacYXQ=
-golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs=
 golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
 google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE=

internal/state/state.go

@@ -347,44 +347,35 @@ func (f *Files) takeIn(name string, data []byte) (int, error) {
 
 // writeFile writes the state file name from what smallwebwaf holds. An
 // edit made since smallwebwaf last read or wrote the file is taken in
-// first, so that it is not overwritten. An edit that does not parse is
-// renamed to name.bad, for the admin to mend, and logged with where in
-// the file the error is.
+// first, so that it is not overwritten, or set aside if it does not
+// parse. A file that cannot be read, or an edit that cannot be set
+// aside, is left as it is, and the write given up. Every write is counted
+// in the metrics, and one that fails or is given up as a failure.
 func (f *Files) writeFile(name string) error {
 	f.mu.Lock()
 	defer f.mu.Unlock()
 
-	path := filepath.Join(f.params.Dir, name)
-
 	data, changed, err := f.readChanged(name)
-	if err != nil {
-		return err
-	}
-
-	if changed {
+	if err == nil && changed {
 		_, err = f.takeIn(name, data)
 		if err == nil {
 			f.params.Metrics.StateFileEditTakenIn(name)
+		} else {
+			err = f.setAside(name, err)
 		}
 	}
 
-	if err != nil {
-		renameErr := os.Rename(path, path+".bad")
-		if renameErr != nil {
-			return errors.Join(err, renameErr)
-		}
-
-		f.params.ProcessLog.Error("set aside an edit of a state file that does not parse",
-			"file", path+".bad", "error", err.Error())
-		f.params.Metrics.StateFileEditSetAside(name)
-	}
-
+	if err == nil {
 		data, err = f.encode(name)
 		if err != nil {
-		return fmt.Errorf("encode %s: %w", name, err)
+			err = fmt.Errorf("encode %s: %w", name, err)
+		}
 	}
 
+	if err == nil {
 		err = write(f.params.Dir, name, data)
+	}
+
 	if err == nil {
 		// The file holds data from here on, even if the directory sync
 		// fails, so that its next read does not take it for an admin's
@@ -398,6 +389,25 @@ func (f *Files) writeFile(name string) error {
 	return err
 }
 
+// setAside renames the state file name, an edit that does not parse with
+// parseErr, to name.bad, for the admin to mend, and logs it with where in
+// the file the error is. If the rename fails, the edit is left as it is,
+// and the error returned is parseErr joined with the rename's.
+func (f *Files) setAside(name string, parseErr error) error {
+	path := filepath.Join(f.params.Dir, name)
+
+	err := os.Rename(path, path+".bad")
+	if err != nil {
+		return errors.Join(parseErr, err)
+	}
+
+	f.params.ProcessLog.Error("set aside an edit of a state file that does not parse",
+		"file", path+".bad", "error", parseErr.Error())
+	f.params.Metrics.StateFileEditSetAside(name)
+
+	return nil
+}
+
 // encode returns the state file name as smallwebwaf writes it, from a
 // snapshot of the part that keeps that state.
 func (f *Files) encode(name string) ([]byte, error) {

internal/state/state_test.go

@@ -511,7 +511,8 @@ func TestFileThatCannotBeReadIsNotWrittenOver(t *testing.T) {
 
 	dir := t.TempDir()
 	path := filepath.Join(dir, bansJSON)
-	files := load(t, newParams(dir))
+	params := newParams(dir)
+	files := load(t, params)
 
 	// bans.json is a socket, which cannot be opened as a file, even by
 	// root, as the tests run in Docker, but which a rename could replace.
@@ -536,6 +537,38 @@ func TestFileThatCannotBeReadIsNotWrittenOver(t *testing.T) {
 	}
 
 	wantFiles(t, dir, bansJSON, clientsJSON, lookupsJSON)
+	wantWriteFailed(t, params, bansJSON)
+}
+
+func TestBrokenEditThatCannotBeSetAsideIsNotWrittenOver(t *testing.T) {
+	t.Parallel()
+
+	const broken = `{"version": 1, "bans": [`
+
+	dir := t.TempDir()
+	path := filepath.Join(dir, bansJSON)
+	params := newParams(dir)
+	files := load(t, params)
+
+	// A directory named bans.json.bad cannot be renamed over, so the
+	// broken edit cannot be set aside, and is left as it is.
+	edit(t, dir, bansJSON, broken)
+
+	err := os.Mkdir(path+".bad", 0o700)
+	if err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+
+	err = files.WriteAll()
+	if err == nil {
+		t.Error("writing with bans.json.bad in the way did not fail")
+	}
+
+	if got := readFile(t, path); got != broken {
+		t.Errorf("bans.json holds\n%s\nwant the edit", got)
+	}
+
+	wantWriteFailed(t, params, bansJSON)
 }
 
 func TestEditOfEachFileTakenIn(t *testing.T) {
@@ -598,6 +631,39 @@ func TestOwnWritesAreNotTakenIn(t *testing.T) {
 	wantTakenIn(t, lines, dir, lookupsJSON)
 }
 
+func TestFileRenamedOverAStateFileTakenIn(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	path := filepath.Join(dir, bansJSON)
+	params := newParams(dir)
+	lines := logInto(&params)
+	files := load(t, params)
+
+	err := files.WriteAll()
+	if err != nil {
+		t.Fatalf("write: %v", err)
+	}
+
+	watch(t, files, lines)
+
+	// The admin mends bans.json.bad and moves it back, as editors that
+	// save by renaming do with a file of their own: nothing is written
+	// into bans.json itself. An edit of clients.json after it must be
+	// taken in second.
+	edit(t, dir, bansJSON+".bad", permanentBansJSON)
+
+	err = os.Rename(path+".bad", path)
+	if err != nil {
+		t.Fatalf("rename: %v", err)
+	}
+
+	edit(t, dir, clientsJSON, `{"version": 1, "clients": []}`)
+
+	wantTakenIn(t, lines, dir, bansJSON)
+	wantEqual(t, bansJSON, params.Ledger.Snapshot(), []bans.Ban{permanentBan()})
+}
+
 func TestBanAddedAndLiftedThroughBansJSON(t *testing.T) {
 	t.Parallel()
 
@@ -1078,6 +1144,18 @@ func metric(t *testing.T, text, series string) float64 {
 	return 0
 }
 
+// wantWriteFailed checks that the metrics of params count one write of the
+// state file name, and that it failed.
+func wantWriteFailed(t *testing.T, params state.Params, name string) {
+	t.Helper()
+
+	got := scrape(t, params)
+	file := `{file="` + name + `"}`
+
+	wantMetric(t, got, "smallwebwaf_state_file_writes_total"+file, 1)
+	wantMetric(t, got, "smallwebwaf_state_file_write_failures_total"+file, 1)
+}
+
 // wantMetric checks the value of series in text, the metrics, as metric
 // reads it.
 func wantMetric(t *testing.T, text, series string, want float64) {