smallwebwaf starts with SWWAF_STATE_DIR given as a file on a new volume (closes #137)

The run script gave ${SWWAF_STATE_DIR:-/var/lib/smallwebwaf} to the
smallwebwaf user and never read SWWAF_STATE_DIR_FILE, so a new volume
at the directory that file names stayed root's and smallwebwaf could
not write it. The run script now reads the setting as smallwebwaf does:
the variable, or what is in the file. When both are set or the file
cannot be read, it leaves the owner alone, and smallwebwaf stops with
its message naming the setting. script/example-app checks that a
container with its state files on a new volume at /srv/state, named in
a file only the smallwebwaf user can read, becomes healthy.

Model: opus-5-5

Dockerfile

@@ -140,14 +140,10 @@ RUN mkdir /etc/sysusers.d \
 # apt also fetches the live archive's InRelease files, which change daily
 # and which the install does not use. The snapshot service is HTTPS only
 # and this image has no CA certificates yet, so this step uses the Go
-# image's. The snapshot service answers 503 at times; apt retries a
-# failed download, waiting twice as long before each retry, and this
-# step gives it five retries, about half a minute in all, where its
-# default of three gives up after seven seconds.
+# image's.
 RUN --mount=type=bind,from=builder,source=/etc/ssl/certs/ca-certificates.crt,target=/tmp/go-image-ca.crt \
     apt-get update --snapshot 20261001T000000Z \
         -o Acquire::https::CaInfo=/tmp/go-image-ca.crt \
-        -o Acquire::Retries=5 \
     && printf '%s\n' \
         '45f95ce276cdba3e41870516a130e03c58b8b7a79e9546b0efe9e526d255740c  snapshot.ubuntu.com_ubuntu_20261001T000000Z_dists_resolute_InRelease' \
         '802e675dd9de4c7f3916434a95e7c1d8eec0e82886622d7805ab19a2c6fe0365  snapshot.ubuntu.com_ubuntu_20261001T000000Z_dists_resolute-updates_InRelease' \
@@ -157,7 +153,6 @@ RUN --mount=type=bind,from=builder,source=/etc/ssl/certs/ca-certificates.crt,tar
     && DEBIAN_FRONTEND=noninteractive apt-get install --yes --no-install-recommends \
         --snapshot 20261001T000000Z \
         -o Acquire::https::CaInfo=/tmp/go-image-ca.crt \
-        -o Acquire::Retries=5 \
         ca-certificates nix-bin runit \
     && rm -rf /var/lib/apt/lists/*
 

internal/bans/admin_test.go

@@ -61,59 +61,6 @@ func TestAdminsBansAreNeverDroppedAndDoNotCountTowardMaxBans(t *testing.T) {
 	wantBans(t, ledger, map[netip.Prefix]int{adminsOnly: 2, both: 1, second: 0, third: 1})
 }
 
-func TestEarlierBansCountABanDroppedAfterAnAdminsBan(t *testing.T) {
-	t.Parallel()
-
-	rules := defaultRules()
-	rules.MaxBans = 2
-	ledger := bans.New(rules)
-	netblock := netip.MustParsePrefix("203.0.113.1/32")
-	other := netip.MustParsePrefix("203.0.113.2/32")
-
-	// An admin bans the netblock for a minute, and it then breaks a limit
-	// twice, each time after its last ban has ended.
-	ledger.BanForAdmin(netblock, midnight(), midnight().Add(time.Minute),
-		"probes for logins")
-	first, _ := ledger.BanForLimit(netblock, midnight().Add(time.Minute), bans.Notes{})
-	second, _ := ledger.BanForLimit(netblock, first.Expires, bans.Notes{})
-
-	// A ban on another netblock fills the ledger, and the netblock's first
-	// ban for a limit, its earliest that is not an admin's, goes.
-	ledger.BanForLimit(other, second.Start, bans.Notes{})
-	wantBans(t, ledger, map[netip.Prefix]int{netblock: 2, other: 1})
-
-	// The netblock's next ban still counts it.
-	third, _ := ledger.BanForLimit(netblock, second.Expires, bans.Notes{})
-	if third.Notes.EarlierBans != (bans.EarlierBans{Limit: 2, Admin: 1}) {
-		t.Errorf("the third ban for a limit has earlier bans %+v, "+
-			"want 2 for a limit and 1 an admin's", third.Notes.EarlierBans)
-	}
-}
-
-func TestEarlierBansCountABanAnAdminKeepsOnce(t *testing.T) {
-	t.Parallel()
-
-	ledger := bans.New(defaultRules())
-	netblock := netip.MustParsePrefix("203.0.113.1/32")
-
-	// The netblock breaks a limit twice, the second time after its first
-	// ban has ended, and the second ban's notes count the first. An admin
-	// then keeps the first ban.
-	first, _ := ledger.BanForLimit(netblock, midnight(), bans.Notes{})
-	second, _ := ledger.BanForLimit(netblock, first.Expires, bans.Notes{})
-
-	kept := first
-	kept.Cause = bans.CauseAdmin
-	ledger.LoadEdit([]bans.Ban{kept, second})
-
-	// The netblock's next ban counts the kept ban once, as an admin's.
-	third, _ := ledger.BanForLimit(netblock, second.Expires, bans.Notes{})
-	if third.Notes.EarlierBans != (bans.EarlierBans{Limit: 1, Admin: 1}) {
-		t.Errorf("the third ban for a limit has earlier bans %+v, "+
-			"want 1 for a limit and 1 an admin's", third.Notes.EarlierBans)
-	}
-}
-
 func TestReasonOfTheBansSmallwebwafMakes(t *testing.T) {
 	t.Parallel()
 

internal/bans/bans.go

@@ -687,51 +687,26 @@ func (l *Ledger) ban(
 }
 
 // earlierBans returns how many bans a netblock with the bans held, oldest
-// first, has had, by cause. Each ban held gives a count: the bans its
-// notes count before it, and one for it and each ban held after it. A
-// ban's count misses a ban dropped after it to make room, and that of an
-// admin's ban added to bans.json without notes misses the bans before it,
-// so the count that names the most bans is taken, whole: a ban an admin
-// keeps by giving it CauseAdmin is still counted under its old cause in
-// later bans' notes, and taking each cause from another count would count
-// it twice.
+// first, has had, by cause: the first ban held counts the bans the
+// netblock had before that one, since dropped to make room, and each ban
+// held adds one.
 func earlierBans(held []Ban) EarlierBans {
-	// fromHere counts ban and the bans held after it.
-	var most, fromHere EarlierBans
+	earlier := held[0].Notes.EarlierBans
 
-	for _, ban := range slices.Backward(held) {
+	for _, ban := range held {
 		switch ban.Cause {
 		case CauseLimit:
-			fromHere.Limit++
+			earlier.Limit++
 		case CauseAttack:
-			fromHere.Attack++
+			earlier.Attack++
 		case CauseAdmin:
-			fromHere.Admin++
+			earlier.Admin++
 		case CauseCrowdSec:
-			fromHere.CrowdSec++
-		}
-
-		notes := ban.Notes.EarlierBans
-		count := EarlierBans{
-			Limit:    notes.Limit + fromHere.Limit,
-			Attack:   notes.Attack + fromHere.Attack,
-			Admin:    notes.Admin + fromHere.Admin,
-			CrowdSec: notes.CrowdSec + fromHere.CrowdSec,
-		}
-
-		// Of counts that name as many bans, the earlier ban's is taken: it
-		// counts more of the bans held by the cause they have now.
-		if count.total() >= most.total() {
-			most = count
+			earlier.CrowdSec++
 		}
 	}
 
-	return most
-}
-
-// total returns how many bans e counts, of all causes.
-func (e EarlierBans) total() int {
-	return e.Limit + e.Attack + e.Admin + e.CrowdSec
+	return earlier
 }
 
 // markChanged has Changed receive a value, unless one is waiting already.

internal/proxy/request.go

@@ -795,19 +795,9 @@ func (rq *request) responseTimedOut() {
 	}
 }
 
-// responseReceived is called once the app has sent its whole answer. An
-// app may answer before the client has sent its whole body, as with a 404
-// it sends without reading it. The timeouts then run on: until the
-// transport's read of the body returns, Go's server cannot send the
-// answer, and SWWAF_CLIENT_REQUEST_TIMEOUT is what ends that read for a
-// client that stops sending.
+// responseReceived is called once the app has sent its whole answer.
 func (rq *request) responseReceived() {
 	rq.complete = true
-
-	if rq.body != nil && !rq.body.received.Load() {
-		return
-	}
-
 	rq.stopTimers()
 }
 

internal/proxy/timeouts_test.go

@@ -194,60 +194,6 @@ func sendLargeBody(t *testing.T, addr string) net.Conn {
 	return conn
 }
 
-func TestClientStopsSendingAfterTheAppHasAnswered(t *testing.T) {
-	t.Parallel()
-
-	// answeredAt is when the app answered, or nil if the bytes the client
-	// sent never reached it.
-	var answeredAt atomic.Pointer[time.Time]
-
-	// The app answers without reading the whole body, as an app does for
-	// a path it does not serve, and only once smallwebwaf waits for the
-	// rest of it: the app takes the bytes the client sent, and then waits
-	// half the timeout, far longer than smallwebwaf takes to go on to
-	// wait.
-	app := startApp(t, func(w http.ResponseWriter, r *http.Request) {
-		_, err := io.ReadFull(r.Body, make([]byte, len("the first bytes")))
-		if err != nil {
-			return
-		}
-
-		time.Sleep(shortTimeout / 2)
-		answeredAt.Store(new(time.Now()))
-		http.NotFound(w, r)
-	})
-	addr, out := startProxy(t, app.URL, map[string]string{
-		clientRequestTimeout: shortTimeoutSetting,
-		metricsToken:         token,
-	})
-
-	start := time.Now()
-	got := readResponse(t, sendPartOfBody(t, addr))
-	wantTimedOut(t, start)
-
-	// Close returns once the app has finished with the request.
-	app.Close()
-
-	// The app's answer came before the time ran out, so the client is
-	// sent that answer, not smallwebwaf's 408. Only a hold-up of the test
-	// process can make it come later, and SPEC.md then asks for 408, as
-	// smallwebwaf was waiting for the client, or for 504 if the bytes the
-	// client sent had not reached the app.
-	at := answeredAt.Load()
-	want := http.StatusNotFound
-
-	switch {
-	case at == nil:
-		want = http.StatusGatewayTimeout
-	case at.Sub(start) >= shortTimeout:
-		want = http.StatusRequestTimeout
-	}
-
-	wantStatus(t, got, want)
-	wantLine(t, out.requestLine(t), want, requestlog.ActionTimedOut)
-	wantLimitHits(t, addr, clientRequestTimeout, 1)
-}
-
 func TestAppTooSlowToAnswer(t *testing.T) {
 	t.Parallel()
 

script/example-app

@@ -6,14 +6,13 @@
 # client, that a probe for /.env bans another client, which its next
 # request bans for good, that `sv stop` stops smallwebwaf in order, that
 # `docker stop` stops the container without having to kill it, that a
-# new container on the same volume still refuses the banned client, that
-# a container with its state files on a new volume mounted at the
-# directory SWWAF_STATE_DIR_FILE names becomes healthy, and that one whose
-# SWWAF_STATE_DIR does not exist logs smallwebwaf's message about it. The
-# containers run with SWWAF_LOOKUP_SOURCE=off, so that no address is sent
-# to GeoJS. The containers, the volumes and both images are removed
-# however the script ends. Building the app needs network access, for
-# nixpkgs' binary cache. script/check does not run this.
+# new container on the same volume still refuses the banned client, and
+# that a container with its state files on a new volume mounted at the
+# directory SWWAF_STATE_DIR_FILE names becomes healthy. The containers
+# run with SWWAF_LOOKUP_SOURCE=off, so that no address is sent to GeoJS.
+# The containers, the volumes and both images are removed however the
+# script ends. Building the app needs network access, for nixpkgs' binary
+# cache. script/check does not run this.
 set -eu
 
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -174,15 +173,6 @@ main() {
     start_moved_container
     echo "example-app: the run script gives the state files' directory named" \
         "by SWWAF_STATE_DIR_FILE to the smallwebwaf user"
-
-    docker rm --force "$CONTAINER" >/dev/null
-    docker run --detach --name "$CONTAINER" \
-        --env SWWAF_STATE_DIR=/srv/missing \
-        --env SWWAF_LOOKUP_SOURCE=off \
-        "$APP_IMAGE" >/dev/null
-    wait_for "smallwebwaf did not report the missing state directory" \
-        logged "SWWAF_STATE_DIR cannot be written"
-    echo "example-app: smallwebwaf reports a state directory that does not exist"
 }
 
 main "$@"

share/smallwebwaf.run

@@ -5,25 +5,24 @@ set -euo pipefail
 # exits; the wait spaces out the restarts. The state directory and every
 # file in it are given to the smallwebwaf user, so that a volume mounted
 # there needs no change of owner; chown -R changes a symbolic link itself,
-# never what it points to. When SWWAF_STATE_DIR, in either form, cannot be
-# read or is not the absolute path of a directory, no owner is changed,
-# and smallwebwaf starts and stops with a message naming the setting.
-# exec, so that the signal `sv stop` sends reaches smallwebwaf itself.
+# never what it points to. exec, so that the signal `sv stop` sends
+# reaches smallwebwaf itself.
 main() {
     sleep 1
     local dir
-    if dir="$(state_dir)" && [[ "$dir" == /* && -d "$dir" ]]; then
-        chown -R smallwebwaf:smallwebwaf "$dir"
+    if dir="$(state_dir)"; then
+        chown -R smallwebwaf:smallwebwaf "${dir:-/var/lib/smallwebwaf}"
     fi
     exec chpst -u smallwebwaf:smallwebwaf /usr/local/bin/smallwebwaf
 }
 
-# state_dir prints SWWAF_STATE_DIR, /var/lib/smallwebwaf when it is unset,
-# or what is in the file SWWAF_STATE_DIR_FILE names, less every newline at
-# its end. It fails when both are set or the file cannot be read.
+# state_dir prints SWWAF_STATE_DIR as smallwebwaf reads it: the variable,
+# or what is in the file SWWAF_STATE_DIR_FILE names. It fails when both
+# are set or the file cannot be read; the state directory then keeps its
+# owner, and smallwebwaf stops with a message naming the setting.
 state_dir() {
     if [[ ! -v SWWAF_STATE_DIR_FILE ]]; then
-        printf '%s\n' "${SWWAF_STATE_DIR-/var/lib/smallwebwaf}"
+        printf '%s\n' "${SWWAF_STATE_DIR:-}"
     elif [[ ! -v SWWAF_STATE_DIR ]]; then
         cat -- "$SWWAF_STATE_DIR_FILE" 2>/dev/null
     else