Keep the bans, the clients and GeoJS's answers in state files (closes #17)

smallwebwaf now copies its state to bans.json, clients.json and
lookups.json in SWWAF_STATE_DIR, as "Persistent state" in SPEC.md
describes, and reads them back at start, so a restart lifts no ban and
gives no client a fresh allowance. Each client gains a history, and a
ban's notes count the netblock's requests. bans.json is written
SWWAF_STATE_WRITE_DELAY after a ban, and every file every
SWWAF_STATE_COUNTER_INTERVAL and at the stop. A ban read back is masked
to its netblock and refuses every client in it. A file that does not
parse, an unknown version, an entry without a field it needs, or an
unwritable directory stops the start.

Deviation: no AS number or name, and no ban cause, reason or lifting yet.

Model: opus-5-5

README.md

@@ -290,10 +290,14 @@ read as the netblock it is in, `203.0.113.0/24`. Buckets and answers whose time
 has passed are dropped. A missing file is empty state, as on a first start. A
 file that does not parse, or has another `version`, stops the start with a
 message naming the file, and the line and column where Go's JSON decoder gives
-them; so does a state directory `smallwebwaf` cannot write. An edit made while
-`smallwebwaf` runs is overwritten by its next write: taking it in comes with
-https://git.eeqj.de/sneak/smallwebwaf/issues/68. The AS number and AS name come
-with their lookup.
+them; so does a state directory `smallwebwaf` cannot write. So does an entry
+without a field it needs, named with the entry's place in the file: a ban's
+`netblock`, `start` or `expires`, which is `null` for a permanent ban; a
+client's `client`, or the `start` of a window in which it has requests; an
+answer's `client`, `country`, which is `""` for a client GeoJS cannot place, or
+`answered`. An edit made while `smallwebwaf` runs is overwritten by its next
+write: taking it in comes with https://git.eeqj.de/sneak/smallwebwaf/issues/68.
+The AS number and AS name come with their lookup.
 
 ## Why
 

internal/state/state.go

@@ -38,7 +38,11 @@ const (
 	lookupsJSON = "lookups.json"
 )
 
-var errVersion = errors.New("unknown version")
+var (
+	errVersion = errors.New("unknown version")
+	// errMissing is for an entry without a field it needs.
+	errMissing = errors.New("has no")
+)
 
 // Params are what Load needs.
 type Params struct {
@@ -92,11 +96,21 @@ type lookupsFile struct {
 	Lookups []lookup.Answer `json:"lookups"`
 }
 
+// stateFile is the struct of a state file. Once the file is decoded, its
+// check refuses the first entry without a field it needs, which would
+// otherwise be read as something the entry does not say. data is the
+// file, for a field that may be null or "" but not left out, which the
+// struct cannot tell apart.
+type stateFile interface {
+	check(data []byte) error
+}
+
 // Load checks that files can be written in Dir, and reads the state files
 // in it into the ledger, the limiter and GeoJS. A missing file is empty
-// state, as on a first start. A file that does not parse, or has an
-// unknown version, is an error that names the file and, where the JSON
-// decoder tells it, the line and column.
+// state, as on a first start. A file that does not parse, has an unknown
+// version, or has an entry without a field it needs, is an error that
+// names the file and, where the JSON decoder tells it, the line and
+// column, or else the entry.
 func Load(params Params) (*Files, error) {
 	err := checkWritable(params.Dir)
 	if err != nil {
@@ -233,6 +247,100 @@ func (e banEntry) ban() bans.Ban {
 	return ban
 }
 
+// check refuses a ban without a netblock, which would refuse every IPv6
+// client, a start, from which the length of the netblock's next ban is
+// worked out, or an expires, which would make it permanent. A permanent
+// ban's expires is null, which Bans cannot tell from a missing one, so
+// each expires is read again as written.
+func (f *bansFile) check(data []byte) error {
+	var written struct {
+		Bans []struct {
+			Expires json.RawMessage `json:"expires"`
+		} `json:"bans"`
+	}
+
+	err := json.Unmarshal(data, &written)
+	if err != nil {
+		return err
+	}
+
+	for i, entry := range f.Bans {
+		switch {
+		case !entry.Netblock.IsValid():
+			return missing(i, "netblock")
+		case entry.Start.IsZero():
+			return missing(i, "start")
+		case written.Bans[i].Expires == nil:
+			return missing(i, "expires")
+		}
+	}
+
+	return nil
+}
+
+// check refuses a client without its address, which would count nobody's
+// requests, or with requests in a window but no start, which would drop
+// them and give the client a fresh allowance.
+func (f *clientsFile) check([]byte) error {
+	for i, client := range f.Clients {
+		switch {
+		case !client.Client.IsValid():
+			return missing(i, "client")
+		case countsWithoutStart(client.Minute):
+			return missing(i, "minute.start")
+		case countsWithoutStart(client.Hour):
+			return missing(i, "hour.start")
+		case countsWithoutStart(client.Day):
+			return missing(i, "day.start")
+		}
+	}
+
+	return nil
+}
+
+// check refuses an answer without a client, which would answer for
+// nobody, a country, which would place the client nowhere, or the time
+// GeoJS gave it, which would drop it. "" is the country of a client
+// GeoJS cannot place, which Lookups cannot tell from a missing one, so
+// each country is read again as written.
+func (f *lookupsFile) check(data []byte) error {
+	var written struct {
+		Lookups []struct {
+			Country *string `json:"country"`
+		} `json:"lookups"`
+	}
+
+	err := json.Unmarshal(data, &written)
+	if err != nil {
+		return err
+	}
+
+	for i, answer := range f.Lookups {
+		switch {
+		case !answer.Client.IsValid():
+			return missing(i, "client")
+		case written.Lookups[i].Country == nil:
+			return missing(i, "country")
+		case answer.Answered.IsZero():
+			return missing(i, "answered")
+		}
+	}
+
+	return nil
+}
+
+// countsWithoutStart reports whether b holds requests but no start, which
+// places them in time.
+func countsWithoutStart(b ratelimit.Buckets) bool {
+	return b.Start.IsZero() && (b.Current != 0 || b.Previous != 0)
+}
+
+// missing returns the error for entry i, counted from 0, of a state file,
+// which has no field.
+func missing(i int, field string) error {
+	return fmt.Errorf("entry %d %w %q", i+1, errMissing, field)
+}
+
 // encodeOnePerLine encodes a state file whose entries, under key, are one
 // to a line, so that grep shows everything about one client.
 func encodeOnePerLine[E any](key string, entries []E) ([]byte, error) {
@@ -270,8 +378,9 @@ func checkWritable(dir string) error {
 }
 
 // read reads the state file name in dir into file, a pointer to that
-// file's struct. A missing file leaves file as it is.
-func read(dir, name string, file any) error {
+// file's struct, and checks its entries. A missing file leaves file as it
+// is.
+func read(dir, name string, file stateFile) error {
 	path := filepath.Join(dir, name)
 
 	data, err := os.ReadFile(path) //nolint:gosec // a state file, in SWWAF_STATE_DIR
@@ -303,6 +412,10 @@ func read(dir, name string, file any) error {
 		err = decoder.Decode(file)
 	}
 
+	if err == nil {
+		err = file.check(data)
+	}
+
 	if err != nil {
 		return fmt.Errorf("%s%s: %w", path, position(data, err), err)
 	}
@@ -344,17 +457,16 @@ func write(dir, name string, data []byte) error {
 	temporary := path + ".tmp"
 
 	err := writeSynced(temporary, data)
+	if err == nil {
+		err = os.Rename(temporary, path)
+	}
+
 	if err != nil {
 		_ = os.Remove(temporary)
 
 		return err
 	}
 
-	err = os.Rename(temporary, path)
-	if err != nil {
-		return err
-	}
-
 	directory, err := os.Open(dir) //nolint:gosec // SWWAF_STATE_DIR itself
 	if err != nil {
 		return err

internal/state/state_test.go

@@ -164,18 +164,87 @@ func TestFileThatDoesNotParseStopsTheStart(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			dir := t.TempDir()
-			path := filepath.Join(dir, tc.file)
+			wantRefused(t, tc.file, tc.content, tc.want)
+		})
+	}
+}
 
-			err := os.WriteFile(path, []byte(tc.content), 0o600)
-			if err != nil {
-				t.Fatalf("write %s: %v", tc.file, err)
-			}
+func TestEntryWithoutAFieldItNeedsStopsTheStart(t *testing.T) {
+	t.Parallel()
 
-			_, err = state.Load(newParams(dir))
-			if err == nil || !strings.HasPrefix(err.Error(), path+tc.want) {
-				t.Errorf("error %v, want one starting %s%s", err, path, tc.want)
-			}
+	const (
+		// The other fields each entry needs.
+		ban    = `"start": "2026-10-06T00:00:00Z", "expires": null`
+		answer = `"answered": "2026-10-06T00:00:00Z"`
+
+		noNetblock = `: entry 1 has no "netblock"`
+	)
+
+	for _, tc := range []struct {
+		name, file, content string
+		// want is what the error says after the file's path.
+		want string
+	}{
+		{
+			"a ban without a netblock", bansJSON,
+			`{"version": 1, "bans": [{` + ban + `}]}`,
+			noNetblock,
+		},
+		{
+			"a ban whose netblock is null", bansJSON,
+			`{"version": 1, "bans": [{"netblock": null, ` + ban + `}]}`,
+			noNetblock,
+		},
+		{
+			"a ban whose netblock is empty", bansJSON,
+			`{"version": 1, "bans": [{"netblock": "", ` + ban + `}]}`,
+			noNetblock,
+		},
+		{
+			"a ban without a start", bansJSON,
+			`{"version": 1, "bans": [{"netblock": "203.0.113.9/32", "expires": null}]}`,
+			`: entry 1 has no "start"`,
+		},
+		{
+			// The first ban's expires is null, as a permanent ban's is.
+			"a ban without an expires", bansJSON,
+			`{"version": 1, "bans": [{"netblock": "203.0.113.9/32", ` + ban + `}, ` +
+				`{"netblock": "203.0.113.10/32", "start": "2026-10-06T00:00:00Z"}]}`,
+			`: entry 2 has no "expires"`,
+		},
+		{
+			"a client without its address", clientsJSON,
+			`{"version": 1, "clients": [{"history": {"requests": 3}}]}`,
+			`: entry 1 has no "client"`,
+		},
+		{
+			"a client with requests in a window without its start", clientsJSON,
+			`{"version": 1, "clients": [{"client": "203.0.113.9/32", ` +
+				`"hour": {"current": 3}}]}`,
+			`: entry 1 has no "hour.start"`,
+		},
+		{
+			"an answer without a client", lookupsJSON,
+			`{"version": 1, "lookups": [{"country": "DE", ` + answer + `}]}`,
+			`: entry 1 has no "client"`,
+		},
+		{
+			// A country of "" is a client GeoJS cannot place.
+			"an answer without a country", lookupsJSON,
+			`{"version": 1, "lookups": [{"client": "192.0.2.1/32", "country": "", ` +
+				answer + `}, {"client": "203.0.113.9/32", ` + answer + `}]}`,
+			`: entry 2 has no "country"`,
+		},
+		{
+			"an answer without the time GeoJS gave it", lookupsJSON,
+			`{"version": 1, "lookups": [{"client": "203.0.113.9/32", "country": "DE"}]}`,
+			`: entry 1 has no "answered"`,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			wantRefused(t, tc.file, tc.content, tc.want)
 		})
 	}
 }
@@ -188,18 +257,7 @@ func TestUnknownVersionStopsTheStart(t *testing.T) {
 			t.Run(file+" "+content, func(t *testing.T) {
 				t.Parallel()
 
-				dir := t.TempDir()
-				path := filepath.Join(dir, file)
-
-				err := os.WriteFile(path, []byte(content), 0o600)
-				if err != nil {
-					t.Fatalf("write %s: %v", file, err)
-				}
-
-				_, err = state.Load(newParams(dir))
-				if err == nil || !strings.HasPrefix(err.Error(), path+": unknown version ") {
-					t.Errorf("error %v, want one naming %s and its version", err, path)
-				}
+				wantRefused(t, file, content, ": unknown version ")
 			})
 		}
 	}
@@ -344,6 +402,26 @@ func TestFailedWriteLeavesTheFileAsItWas(t *testing.T) {
 	}
 }
 
+func TestFailedRenameLeavesNoTemporaryFile(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	files := load(t, newParams(dir))
+
+	// A directory named bans.json cannot be renamed over.
+	err := os.Mkdir(filepath.Join(dir, bansJSON), 0o700)
+	if err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+
+	err = files.WriteAll()
+	if err == nil {
+		t.Error("writing over a directory did not fail")
+	}
+
+	wantFiles(t, dir, bansJSON, clientsJSON, lookupsJSON)
+}
+
 // midnight is the time of the tests' clock.
 func midnight() time.Time {
 	return time.Date(2026, 10, 6, 0, 0, 0, 0, time.UTC)
@@ -474,6 +552,26 @@ func readFile(t *testing.T, path string) string {
 	return string(data)
 }
 
+// wantRefused writes content to the state file named file in a new
+// directory, and checks that Load refuses it with an error that is the
+// file's path and then starts with want.
+func wantRefused(t *testing.T, file, content, want string) {
+	t.Helper()
+
+	dir := t.TempDir()
+	path := filepath.Join(dir, file)
+
+	err := os.WriteFile(path, []byte(content), 0o600)
+	if err != nil {
+		t.Fatalf("write %s: %v", file, err)
+	}
+
+	_, err = state.Load(newParams(dir))
+	if err == nil || !strings.HasPrefix(err.Error(), path+want) {
+		t.Errorf("error %v, want one starting %s%s", err, path, want)
+	}
+}
+
 // removeFiles removes the named files from dir.
 func removeFiles(t *testing.T, dir string, names ...string) {
 	t.Helper()