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, whatever
SWWAF_BAN_SCOPE_V4_PREFIX is now. A file that does not parse, an unknown
version 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,14 +290,10 @@ 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. So does an entry
+them; so does a state directory `smallwebwaf` cannot write. An edit made while
-without a field it needs, named with the entry's place in the file: a ban's
+`smallwebwaf` runs is overwritten by its next write: taking it in comes with
-`netblock`, `start` or `expires`, which is `null` for a permanent ban; a
+https://git.eeqj.de/sneak/smallwebwaf/issues/68. The AS number and AS name come
-client's `client`, or the `start` of a window in which it has requests; an
+with their lookup.
-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,11 +38,7 @@ const (
 	lookupsJSON = "lookups.json"
 )
 
-var (
+var errVersion = errors.New("unknown version")
-	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 {
@@ -96,21 +92,11 @@ 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, has an unknown
+// state, as on a first start. A file that does not parse, or has an
-// version, or has an entry without a field it needs, is an error that
+// unknown version, is an error that names the file and, where the JSON
-// names the file and, where the JSON decoder tells it, the line and
+// decoder tells it, the line and column.
-// column, or else the entry.
 func Load(params Params) (*Files, error) {
 	err := checkWritable(params.Dir)
 	if err != nil {
@@ -247,100 +233,6 @@ 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) {
@@ -378,9 +270,8 @@ func checkWritable(dir string) error {
 }
 
 // read reads the state file name in dir into file, a pointer to that
-// file's struct, and checks its entries. A missing file leaves file as it
+// file's struct. A missing file leaves file as it is.
-// is.
+func read(dir, name string, file any) error {
-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
@@ -412,10 +303,6 @@ func read(dir, name string, file stateFile) 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)
 	}
@@ -457,16 +344,17 @@ 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,87 +164,18 @@ func TestFileThatDoesNotParseStopsTheStart(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			wantRefused(t, tc.file, tc.content, tc.want)
+			dir := t.TempDir()
-		})
+			path := filepath.Join(dir, tc.file)
-	}
-}
 
-func TestEntryWithoutAFieldItNeedsStopsTheStart(t *testing.T) {
+			err := os.WriteFile(path, []byte(tc.content), 0o600)
-	t.Parallel()
+			if err != nil {
+				t.Fatalf("write %s: %v", tc.file, err)
+			}
 
-	const (
+			_, err = state.Load(newParams(dir))
-		// The other fields each entry needs.
+			if err == nil || !strings.HasPrefix(err.Error(), path+tc.want) {
-		ban    = `"start": "2026-10-06T00:00:00Z", "expires": null`
+				t.Errorf("error %v, want one starting %s%s", err, path, tc.want)
-		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)
 		})
 	}
 }
@@ -257,7 +188,18 @@ func TestUnknownVersionStopsTheStart(t *testing.T) {
 			t.Run(file+" "+content, func(t *testing.T) {
 				t.Parallel()
 
-				wantRefused(t, file, content, ": unknown version ")
+				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)
+				}
 			})
 		}
 	}
@@ -402,26 +344,6 @@ 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)
@@ -552,26 +474,6 @@ 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()