Unusable DATA_DIR error names DATA_DIR; report file number is atomic.Int64 (closes #148, closes #161)

When the backend runs on its own, a DATA_DIR that cannot be created or
read stopped the start with an error that named the path but not
DATA_DIR. Both errors now read DATA_DIR "<value>": <reason>, the form
config.New uses for the other settings. A test starts the buffer with
DATA_DIR below a regular file and checks the error.

The report file sequence counter is atomic.Int64 instead of
atomic.Uint64, as the Go style guide asks for int types. File names are
unchanged.

Model: opus-5-5

README.md

@@ -212,8 +212,15 @@ replaces the default; set and empty, the server trusts no proxy. Loopback is not
 in the default, so a reverse proxy on the same host has to be named, as in
 `TRUSTED_PROXIES=127.0.0.1/32`. A request whose direct peer is outside this set
 has its forwarded headers ignored, and the direct peer is logged and
-rate-limited instead. The container image does not use this default; see
-[Deployment](#deployment).
+rate-limited instead. From a direct peer inside the set, the client address is
+the right-most `X-Forwarded-For` entry that is not in the set: each proxy
+appends the address it took the request from, so the entries further left were
+written by the client and are not used. An entry written as an address with a
+port, such as `198.51.100.7:5000` or `[2001:db8::1]:443`, is read as that
+address. If the search from the right finds only addresses in the set, or meets
+an entry that is not an address at all, such as `unknown`, the direct peer is
+used. `X-Real-IP` is read only when `X-Forwarded-For` is missing or empty. The
+container image does not use this default; see [Deployment](#deployment).
 
 A variable set to a value the server cannot use, such as `PORT=abc`,
 `DEBUG=maybe` or a `BIND_ADDRESS` that is not an IP address, stops it from

TODO.md

@@ -21,6 +21,66 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
 
 # Completed Steps
 
+- 2026-10-18: when the backend runs on its own, a `DATA_DIR` that cannot be
+  created or read stops the start with an error naming `DATA_DIR` and its value,
+  as the other settings' errors do
+  ([#148](https://git.eeqj.de/sneak/netwatch/issues/148)); it named only the
+  path. A unit test starts the buffer with `DATA_DIR` below a regular file and
+  checks the error. The report file number is an `atomic.Int64`, as the Go style
+  guide asks for int types
+  ([#161](https://git.eeqj.de/sneak/netwatch/issues/161)); file names are
+  unchanged
+- 2026-10-18: a browser that cannot format the header's clocks still probes
+  ([#149](https://git.eeqj.de/sneak/netwatch/issues/149)). The clocks name the
+  local time zone through `Intl.DateTimeFormat`'s `shortGeneric` option, which
+  Chrome before 95, Firefox before 91 and Safari before 15.4 reject with an
+  error; it came before the first round started, so such a browser never checked
+  a target. The clocks now start last, after the rounds, and an error formatting
+  them leaves both blank. The footer's color legend reads `>=500ms` for red,
+  where it read `>500ms`, so 500 ms, which is red, is in one of its colors
+  ([#152](https://git.eeqj.de/sneak/netwatch/issues/152)). Unit tests check the
+  clocks with a working `Intl.DateTimeFormat` and with one that rejects
+  `shortGeneric`, and draw the page and check the legend against every entry of
+  `CONFIG.latencyColors`
+- 2026-10-18: `netwatch-server prepare-data-dir` no longer changes the owner of
+  a file outside `/data` through a hard link in the volume
+  ([#143](https://git.eeqj.de/sneak/netwatch/issues/143)). A file with a second
+  name (hard link) keeps its owner, and the start logs its path. Owners are now
+  set on the opened directory, file or link, never by name, so a hard link
+  swapped in after the check is caught too; a symbolic link is opened without
+  following it and given to the netwatch user itself, so the server can still
+  follow one in a sticky directory that anyone can write to. Devices, named
+  pipes and sockets keep their owner. Setting the owner on an open file that is
+  not a directory is built only for Linux: `prepare-data-dir` runs only in the
+  Linux image, and macOS, where the backend is also built for development, has
+  neither `O_PATH` nor `AT_EMPTY_PATH`. On any other system the package still
+  builds, and `prepare-data-dir` fails at the first file that is not a
+  directory. The test that runs as root hard-links a file, a symbolic link and a
+  named pipe from outside the volume into it and finds their owners unchanged
+- 2026-10-18: from a trusted proxy, the backend takes the client address from
+  the right-most `X-Forwarded-For` entry that is not in `TRUSTED_PROXIES`
+  ([#144](https://git.eeqj.de/sneak/netwatch/issues/144)). It took the left-most
+  entry, which the client writes itself behind a proxy that appends to the
+  header, so a client could choose the address the rate limit counts and the
+  request log records. An entry written as an address with a port is read as
+  that address. With only trusted addresses in the header, or an entry that is
+  not an address at all (such as `unknown`) met on the way, the direct peer is
+  used. `X-Real-IP` is read only without `X-Forwarded-For`, and every
+  `X-Forwarded-For` header line is read, not only the first. The image's nginx,
+  and the `TRUSTED_PROXIES=127.0.0.1/32` it starts the backend with, are
+  unchanged; the comment in `configs/nginx.conf` that said the backend takes the
+  first entry now says it takes the one address nginx passes on. Unit tests
+  resolve the address from chains ending in an untrusted address, in trusted
+  ones, of only trusted ones, with ports, with an entry that is not an address,
+  and split over two header lines
+- 2026-10-18: after an interval change, the summary's Reachable slot reads
+  `--/--` in white, as at page load, until the second round since the change
+  ends ([#140](https://git.eeqj.de/sneak/netwatch/issues/140)). It used to read
+  0/26 in red from the change on, as no WAN host had a check recorded, for up to
+  two intervals with the network fine. The summary update now writes `--/--`
+  while every WAN host still waits for its first check. A unit test resets the
+  hosts as the interval menu does and checks the slot; another checks that it
+  reads 0/26 in red once every WAN host's check has failed
 - 2026-10-14: stopping the server waits for a report file write already under
   way, the periodic flush's or the one `Append` starts when 10 MiB are waiting,
   before the final flush

bin/entrypoint.sh

@@ -66,10 +66,13 @@ done > /etc/nginx/trusted-proxies.conf
 # netwatch-server keeps its report files in DATA_DIR, on the /data
 # volume, which may be a host directory owned by root or by another
 # uid. Here, as root, netwatch-server prepare-data-dir creates DATA_DIR
-# and gives /data and everything in it to the netwatch user, so the
-# host directory needs no preparing. It stops the start, naming
-# DATA_DIR, unless DATA_DIR is /data or a path below it, and it acts on
-# nothing outside /data, whatever symbolic links it meets there.
+# and gives /data and the directories, regular files and symbolic links
+# in it to the netwatch user, so the host directory needs no preparing.
+# A file with a second name (hard link) keeps its owner, as that name
+# could be outside /data, and its path is logged. It stops the start,
+# naming DATA_DIR, unless DATA_DIR is /data or a path below it, and it
+# acts on nothing outside /data, whatever symbolic or hard links it
+# meets there.
 export DATA_DIR="${DATA_DIR:-/data/reports}"
 netwatch-server prepare-data-dir "$DATA_DIR" || exit 1
 

configs/nginx.conf

@@ -46,7 +46,9 @@ server {
     # 127.0.0.1:8081. These headers go with every request passed to it.
     # X-Forwarded-For carries only the client address, as resolved by
     # the real IP settings above, and not the chain the request came
-    # with: the backend takes the first entry, which a client can write.
+    # with. bin/entrypoint.sh starts the backend with
+    # TRUSTED_PROXIES=127.0.0.1/32, the address nginx connects from, so
+    # the backend takes that one entry as the client address.
     proxy_set_header Host $host;
     proxy_set_header X-Real-IP $remote_addr;
     proxy_set_header X-Forwarded-For $remote_addr;

go.mod

@@ -17,6 +17,7 @@ require (
 	github.com/slok/go-http-metrics v0.13.0
 	github.com/spf13/viper v1.21.0
 	go.uber.org/fx v1.24.0
+	golang.org/x/sys v0.47.0
 )
 
 require (
@@ -41,7 +42,6 @@ require (
 	go.uber.org/multierr v1.10.0 // indirect
 	go.uber.org/zap v1.26.0 // indirect
 	go.yaml.in/yaml/v3 v3.0.4 // indirect
-	golang.org/x/sys v0.47.0 // indirect
 	golang.org/x/text v0.40.0 // indirect
 	google.golang.org/protobuf v1.36.11 // indirect
 )

internal/middleware/middleware.go

@@ -11,6 +11,7 @@ import (
 	"net/http"
 	"net/netip"
 	"runtime/debug"
+	"slices"
 	"strings"
 	"time"
 
@@ -133,6 +134,17 @@ func ipFromHostPort(hostPort string) string {
 // X-Real-IP are honoured only when the direct peer is a
 // trusted proxy; otherwise the direct peer is returned so a
 // spoofed header cannot forge the logged address.
+//
+// Each proxy appends to X-Forwarded-For the address it took the
+// request from, so the entries on the right are written by the
+// trusted proxies and those further left by the client. The
+// client's address is therefore the right-most entry that is not
+// a trusted proxy. An entry written as an address with a port is
+// read as that address, both for the trust check and as the
+// result. When every entry is a trusted proxy, or the walk from
+// the right meets an entry that is not an address at all (such as
+// "unknown"), the peer is used. X-Real-IP is read only when there
+// is no X-Forwarded-For entry at all.
 func clientIP(
 	remoteAddr string,
 	header http.Header,
@@ -144,28 +156,49 @@ func clientIP(
 		return peer
 	}
 
-	if xff := firstForwardedFor(header.Get("X-Forwarded-For")); xff != "" {
-		return xff
+	forwarded := forwardedFor(header)
+
+	if len(forwarded) == 0 {
+		if xr := strings.TrimSpace(header.Get("X-Real-IP")); validIP(xr) {
+			return xr
+		}
+
+		return peer
 	}
 
-	if xr := strings.TrimSpace(header.Get("X-Real-IP")); validIP(xr) {
-		return xr
+	for _, entry := range slices.Backward(forwarded) {
+		if !validIP(entry) {
+			return peer
+		}
+
+		if !addrInAny(entry, trusted) {
+			return entry
+		}
 	}
 
 	return peer
 }
 
-// firstForwardedFor returns the left-most valid address in an
-// X-Forwarded-For list (the original client), or "" if none.
-func firstForwardedFor(value string) string {
+// forwardedFor returns the entries of X-Forwarded-For in order,
+// leaving out blank ones. A proxy may add its entry as a header
+// line of its own rather than append it to the line the client
+// sent, so every line is read, not only the first. Some proxies
+// write an entry as an address with a port, such as
+// 198.51.100.7:5000 or [2001:db8::1]:443; the port is removed so
+// that the entry is read as its address.
+func forwardedFor(header http.Header) []string {
+	value := strings.Join(header.Values("X-Forwarded-For"), ",")
+
+	var entries []string
+
 	for part := range strings.SplitSeq(value, ",") {
-		candidate := strings.TrimSpace(part)
-		if validIP(candidate) {
-			return candidate
+		entry := strings.TrimSpace(part)
+		if entry != "" {
+			entries = append(entries, ipFromHostPort(entry))
 		}
 	}
 
-	return ""
+	return entries
 }
 
 func validIP(s string) bool {

internal/middleware/middleware_test.go

@@ -25,10 +25,16 @@ import (
 const (
 	// loopbackPeer is a remote address inside the trusted-proxy allowlist.
 	loopbackPeer = "127.0.0.1:5000"
+	// loopbackIP is the address of loopbackPeer, which ClientIP returns
+	// when it uses the peer.
+	loopbackIP = "127.0.0.1"
 	// forwardedIP is the client address presented via X-Forwarded-For.
 	forwardedIP = "203.0.113.7"
 	// realIP is the client address presented via X-Real-IP.
 	realIP = "203.0.113.9"
+	// appendedIP is the X-Forwarded-For entry a trusted proxy appended
+	// after the entries the client wrote.
+	appendedIP = "198.51.100.7"
 )
 
 func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix {
@@ -85,10 +91,35 @@ func clientIPCases() []clientIPCase {
 			want:       forwardedIP,
 		},
 		{
-			name:       "trusted proxy uses left-most of chain",
-			remoteAddr: "10.1.2.3:5000",
-			xff:        forwardedIP + ", 10.1.2.3",
-			want:       forwardedIP,
+			name:       "trusted proxy uses right-most of chain",
+			remoteAddr: loopbackPeer,
+			xff:        "203.0.113.9, " + appendedIP,
+			want:       appendedIP,
+		},
+		{
+			name:       "trusted proxy entries are skipped from the right",
+			remoteAddr: loopbackPeer,
+			xff:        "203.0.113.9, 10.0.0.5",
+			want:       "203.0.113.9",
+		},
+		{
+			name:       "chain of only trusted proxies uses peer",
+			remoteAddr: loopbackPeer,
+			xff:        "10.0.0.5, 10.1.2.3",
+			want:       loopbackIP,
+		},
+		{
+			name:       "entry that is not an address uses peer",
+			remoteAddr: loopbackPeer,
+			xff:        "203.0.113.9, unknown",
+			want:       loopbackIP,
+		},
+		{
+			name:       "x-real-ip ignored when forwarded-for has entries",
+			remoteAddr: loopbackPeer,
+			xff:        "10.0.0.5",
+			xRealIP:    realIP,
+			want:       loopbackIP,
 		},
 		{
 			name:       "trusted proxy falls back to x-real-ip",
@@ -117,7 +148,7 @@ func clientIPCases() []clientIPCase {
 			name:       "trusted proxy with garbage header uses peer",
 			remoteAddr: loopbackPeer,
 			xff:        "not-an-ip",
-			want:       "127.0.0.1",
+			want:       loopbackIP,
 		},
 	}
 }
@@ -148,6 +179,47 @@ func TestClientIP(t *testing.T) {
 	}
 }
 
+// TestClientIPReadsEveryForwardedForLine checks that an entry a proxy
+// adds as an X-Forwarded-For line of its own is read, and the line the
+// client sent before it is not taken.
+func TestClientIPReadsEveryForwardedForLine(t *testing.T) {
+	t.Parallel()
+
+	header := http.Header{}
+	header.Add("X-Forwarded-For", "203.0.113.9")
+	header.Add("X-Forwarded-For", appendedIP)
+
+	got := middleware.ClientIP(loopbackPeer, header,
+		mustPrefixes(t, "127.0.0.1/32"))
+	if got != appendedIP {
+		t.Errorf("ClientIP() = %q, want %q", got, appendedIP)
+	}
+}
+
+// TestClientIPReadsAddressWithPort checks that an X-Forwarded-For entry
+// written as an address with a port is read as that address, both for
+// the trust check and as the result.
+func TestClientIPReadsAddressWithPort(t *testing.T) {
+	t.Parallel()
+
+	trusted := mustPrefixes(t, "127.0.0.1/32", "10.0.0.0/8")
+
+	for xff, want := range map[string]string{
+		"203.0.113.9, " + appendedIP + ":5000": appendedIP,
+		"203.0.113.9, [2001:db8::1]:443":       "2001:db8::1",
+		"203.0.113.9, 10.0.0.5:5000":           "203.0.113.9",
+	} {
+		header := http.Header{}
+		header.Set("X-Forwarded-For", xff)
+
+		got := middleware.ClientIP(loopbackPeer, header, trusted)
+		if got != want {
+			t.Errorf("X-Forwarded-For %q: ClientIP() = %q, want %q",
+				xff, got, want)
+		}
+	}
+}
+
 func TestSecurityHeaders(t *testing.T) {
 	t.Parallel()
 

internal/reportbuf/datadir.go

@@ -18,9 +18,15 @@ var ErrDataDirOutsideVolume = errors.New(
 // PrepareDataDir gets dir, the server's DATA_DIR, ready for owner, the
 // user the server runs as, so that a host directory mounted at volume,
 // /data in the image, needs no preparing: it creates dir, gives volume
-// and everything in it to owner, and gives volume and dir the mode the
-// server gives a directory it creates. dir must be volume or a path
-// below it, with no '.', '..', empty part or '/' at the end.
+// and the directories, regular files and symbolic links in it to
+// owner, and gives volume and dir the mode the server gives a directory
+// it creates. It leaves a file with a second name (hard link) as it
+// is, as that name could be outside volume, and logs its path; it
+// leaves a device, named pipe or socket as it is too (see chownFile).
+// dir must be volume or a path below it, with no '.', '..', empty part
+// or '/' at the end. On any system but Linux, the system of the image, it
+// fails at the first file in volume that is not a directory (see
+// chownFile).
 //
 // bin/entrypoint.sh runs this as root, which would follow a symbolic
 // link anywhere, so every step goes through an os.Root opened on
@@ -32,10 +38,9 @@ var ErrDataDirOutsideVolume = errors.New(
 // open, never following a link at the name it creates, and follows a
 // link on the path only as the os.Root allows, so a relative link
 // inside volume can lead it to create directories elsewhere inside
-// volume. Lchown never changes what a link points to, and the modes
-// are set on directories already opened (see chmodDir), so the most
-// that process can do is make a step fail or wait, or act on
-// something else inside volume.
+// volume. The owners and modes are set on files already opened (see
+// chownAll and chmodDir), so the most that process can do is make a
+// step fail or wait, or act on something else inside volume.
 func PrepareDataDir(volume, dir string, owner *user.User) error {
 	// rel is dir as a path from volume; IsLocal is false for one that
 	// leads out of it.
@@ -66,7 +71,7 @@ func PrepareDataDir(volume, dir string, owner *user.User) error {
 		return err
 	}
 
-	err = lchownAll(root, ".", uid, gid)
+	err = chownAll(root, ".", uid, gid)
 	if err != nil {
 		return err
 	}
@@ -79,28 +84,23 @@ func PrepareDataDir(volume, dir string, owner *user.User) error {
 	return chmodDir(root, rel)
 }
 
-// lchownAll gives name, a directory inside root, and everything in it
-// to uid and gid. It reads each directory opened through root, not
-// through root.FS(), which refuses a name that is not valid UTF-8, and
-// calls Lchown on every entry, which gives a symbolic link itself to
-// them, not what it points to. It goes into an entry only when the
-// read found a directory there, so it follows no link it finds; one
-// swapped in for that directory afterwards is followed only as the
-// os.Root allows.
-func lchownAll(root *os.Root, name string, uid, gid int) error {
-	err := root.Lchown(name, uid, gid)
-	if err != nil {
-		return err
-	}
-
-	dir, err := root.Open(name)
-	if err != nil {
-		return err
-	}
-
-	entries, err := dir.ReadDir(-1)
-	_ = dir.Close()
-
+// chownAll gives name, a directory inside root, and the directories,
+// regular files and symbolic links in it to uid and gid, but not the
+// files chownFile leaves. It opens each through root and gives the
+// open file to them, as chmodDir sets a mode on the open directory.
+// Root.Lchown would not hold: it acts on whatever is at the name by
+// then, and a hard link swapped in there shares its owner with a file
+// that could be outside root. A symbolic link is given to them itself,
+// never what it points to, because in a directory that anyone can
+// write to and that has the sticky bit set, as /tmp has, the kernel
+// follows a link only for the link's owner or when the link and the
+// directory have the same owner. chownAll goes into an entry only when
+// the read found a directory there, so it follows no link it finds;
+// one swapped in afterwards for a directory is followed only as the
+// os.Root allows, and one swapped in for any other file is not
+// followed at all (see chownFile).
+func chownAll(root *os.Root, name string, uid, gid int) error {
+	entries, err := chownDir(root, name, uid, gid)
 	if err != nil {
 		return err
 	}
@@ -108,9 +108,9 @@ func lchownAll(root *os.Root, name string, uid, gid int) error {
 	for _, entry := range entries {
 		entryName := filepath.Join(name, entry.Name())
 		if entry.IsDir() {
-			err = lchownAll(root, entryName, uid, gid)
+			err = chownAll(root, entryName, uid, gid)
 		} else {
-			err = root.Lchown(entryName, uid, gid)
+			err = chownFile(root, entryName, uid, gid)
 		}
 
 		if err != nil {
@@ -121,6 +121,36 @@ func lchownAll(root *os.Root, name string, uid, gid int) error {
 	return nil
 }
 
+// chownDir gives name, a directory inside root, to uid and gid, and
+// returns what it holds. Like chmodDir, it opens name through root and
+// refuses anything but a directory, which has no second name (hard
+// link). It reads the open directory, not root.FS(), which refuses a
+// name that is not valid UTF-8.
+func chownDir(root *os.Root, name string, uid, gid int) ([]fs.DirEntry, error) {
+	dir, err := root.Open(name)
+	if err != nil {
+		return nil, err
+	}
+
+	defer func() { _ = dir.Close() }()
+
+	info, err := dir.Stat()
+	if err != nil {
+		return nil, err
+	}
+
+	if !info.IsDir() {
+		return nil, &fs.PathError{Op: "chown", Path: name, Err: syscall.ENOTDIR}
+	}
+
+	err = dir.Chown(uid, gid)
+	if err != nil {
+		return nil, err
+	}
+
+	return dir.ReadDir(-1)
+}
+
 // chmodDir gives name, a directory inside root, the mode the server
 // gives a directory it creates. Root.Chmod would not hold: on Linux it
 // checks that name is not a symbolic link, then sets the mode by name,

internal/reportbuf/datadir_linux.go

@@ -0,0 +1,65 @@
+//go:build linux
+
+package reportbuf
+
+import (
+	"io/fs"
+	"log/slog"
+	"os"
+	"syscall"
+
+	"golang.org/x/sys/unix"
+)
+
+// chownFile gives name, a file inside root that is not a directory, to
+// uid and gid if it is a regular file or symbolic link with one name.
+// It leaves, and logs, a file of any kind with a second name (hard
+// link): all the names of a file share its owner, and the other could
+// be outside root. It leaves any other file with one name, such as a
+// device, named pipe or socket: the server needs to own none of them.
+// It opens name through root with O_PATH, which reads nothing from the
+// file, so opening a device or named pipe neither waits nor acts on
+// it; os.Root adds O_NOFOLLOW, and the two together open a symbolic
+// link itself, never what it points to. It reads the kind of file and
+// its number of names from the open file and gives that file to them,
+// so a hard link swapped in at name is caught too.
+//
+// This file is built only on Linux: prepare-data-dir runs only in the
+// Linux image, and macOS, where the backend is also built for
+// development, has neither O_PATH nor AT_EMPTY_PATH. On any other
+// system chownFile returns an error (see datadir_other.go).
+func chownFile(root *os.Root, name string, uid, gid int) error {
+	file, err := root.OpenFile(name, unix.O_PATH, 0)
+	if err != nil {
+		return err
+	}
+
+	defer func() { _ = file.Close() }()
+
+	info, err := file.Stat()
+	if err != nil {
+		return err
+	}
+
+	stat, ok := info.Sys().(*syscall.Stat_t)
+	if !ok || stat.Nlink > 1 {
+		slog.Warn("owner not changed: file has a second name (hard link)",
+			"path", file.Name())
+
+		return nil
+	}
+
+	if !info.Mode().IsRegular() && info.Mode().Type() != fs.ModeSymlink {
+		return nil
+	}
+
+	// File.Chown would fail: fchown refuses a file opened with O_PATH.
+	// Fchownat with an empty name and AT_EMPTY_PATH acts on the open
+	// file instead.
+	err = unix.Fchownat(int(file.Fd()), "", uid, gid, unix.AT_EMPTY_PATH)
+	if err != nil {
+		return &fs.PathError{Op: "chown", Path: file.Name(), Err: err}
+	}
+
+	return nil
+}

internal/reportbuf/datadir_other.go

@@ -0,0 +1,21 @@
+//go:build !linux
+
+package reportbuf
+
+import (
+	"errors"
+	"io/fs"
+	"os"
+)
+
+// chownFile returns an error on every system but Linux. Opening a file
+// that is not a directory without reading it or following it, and
+// giving it to a user on the open file, never by name, is built only
+// for Linux (see datadir_linux.go): prepare-data-dir runs only in the
+// Linux image, and macOS, where the backend is also built for
+// development, has neither O_PATH nor AT_EMPTY_PATH. This file lets
+// the package build elsewhere, such as on macOS, where a developer
+// runs make build and make run.
+func chownFile(_ *os.Root, name string, _, _ int) error {
+	return &fs.PathError{Op: "chown", Path: name, Err: errors.ErrUnsupported}
+}

internal/reportbuf/datadir_test.go

@@ -254,10 +254,13 @@ func TestPrepareDataDirTakesDirectoryNamedInLatin1(t *testing.T) {
 	requireOwner(t, filepath.Join(latin1, "f"), uint32(uid), uint32(gid))
 }
 
-// TestPrepareDataDirGivesVolumeToOwner gives everything in the volume
-// to a uid and gid that own nothing, which only root can do. A
-// symbolic link in the volume to a directory outside it is given to
-// them itself; what it points to is left as it was.
+// TestPrepareDataDirGivesVolumeToOwner gives the directories, regular
+// files and symbolic links in the volume to a uid and gid that own
+// nothing, which only root can do. A symbolic link in the volume to a
+// directory outside it is given to them itself; what it points to is
+// left as it was. A hard link to a file outside the volume, one to a
+// symbolic link outside it and one to a named pipe outside it are left
+// as they were, and so is a named pipe in the volume.
 func TestPrepareDataDirGivesVolumeToOwner(t *testing.T) {
 	t.Parallel()
 
@@ -274,6 +277,16 @@ func TestPrepareDataDirGivesVolumeToOwner(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	err = os.Symlink("f", filepath.Join(outside, "link"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = syscall.Mkfifo(filepath.Join(outside, "fifo"), 0o600)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	err = os.Mkdir(old, 0o700)
 	if err != nil {
 		t.Fatal(err)
@@ -289,6 +302,21 @@ func TestPrepareDataDirGivesVolumeToOwner(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	err = syscall.Mkfifo(filepath.Join(old, "fifo"), 0o600)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// os.Link does not follow a symbolic link: it gives the link itself
+	// a second name.
+	for _, name := range []string{"f", "link", "fifo"} {
+		err = os.Link(filepath.Join(outside, name),
+			filepath.Join(old, "hardlink-to-"+name))
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
 	err = reportbuf.PrepareDataDir(volume, filepath.Join(volume, reports),
 		&user.User{Uid: "4242", Gid: "4343"})
 	if err != nil {
@@ -296,12 +324,16 @@ func TestPrepareDataDirGivesVolumeToOwner(t *testing.T) {
 	}
 
 	for _, path := range []string{
-		volume, filepath.Join(volume, reports), old,
-		filepath.Join(old, "f"), filepath.Join(old, "link"),
+		volume, filepath.Join(volume, reports), old, filepath.Join(old, "f"),
+		filepath.Join(old, "link"),
 	} {
 		requireOwner(t, path, 4242, 4343)
 	}
 
-	requireOwner(t, outside, 0, 0)
-	requireOwner(t, filepath.Join(outside, "f"), 0, 0)
+	for _, path := range []string{
+		outside, filepath.Join(outside, "f"), filepath.Join(outside, "link"),
+		filepath.Join(outside, "fifo"), filepath.Join(old, "fifo"),
+	} {
+		requireOwner(t, path, 0, 0)
+	}
 }

internal/reportbuf/reportbuf.go

@@ -1,6 +1,9 @@
 // Package reportbuf accumulates telemetry reports in memory
 // and periodically flushes them to zstd-compressed JSONL files,
-// deleting the oldest files to keep them under a size cap.
+// deleting the oldest files to keep them under a size cap. Before the
+// server starts, PrepareDataDir gets the directory those files are
+// kept in, DATA_DIR, ready for the user the server runs as: it creates
+// it and gives it and the volume it is on to that user.
 package reportbuf
 
 import (
@@ -85,7 +88,7 @@ type Buffer struct {
 	now func() time.Time
 	// seq numbers the report files, so that two named in the same
 	// millisecond still get different names.
-	seq      atomic.Uint64
+	seq      atomic.Int64
 	stopOnce sync.Once
 	// usedBytes is what Append checks against maxBytes: the size
 	// of the report files in dataDir, plus the reports waiting to
@@ -124,16 +127,18 @@ func New(
 
 	lc.Append(fx.Hook{
 		OnStart: func(_ context.Context) error {
+			// The errors name DATA_DIR, as config.New's do for the
+			// settings it checks.
 			err := os.MkdirAll(b.dataDir, dirPerms)
 			if err != nil {
-				return fmt.Errorf("create data dir: %w", err)
+				return fmt.Errorf("DATA_DIR %q: %w", b.dataDir, err)
 			}
 
 			// Report files left by earlier runs count too, and are
 			// the first to be deleted to make room.
 			files, err := reportFiles(b.dataDir)
 			if err != nil {
-				return err
+				return fmt.Errorf("DATA_DIR %q: %w", b.dataDir, err)
 			}
 
 			b.mu.Lock()

internal/reportbuf/reportbuf_test.go

@@ -108,6 +108,38 @@ func TestFailedFinalFlushFailsStop(t *testing.T) {
 	}
 }
 
+// TestUncreatableDataDirStopsStart sets DATA_DIR below a regular file,
+// where no directory can be created, even by root, as the tests run in
+// the backend image. The start must fail with an error naming DATA_DIR
+// and its value, as the errors for the other settings do.
+func TestUncreatableDataDirStopsStart(t *testing.T) {
+	file := filepath.Join(t.TempDir(), "file")
+	writeBytes(t, file, 1)
+
+	dir := filepath.Join(file, "reports")
+	t.Setenv("DATA_DIR", dir)
+
+	app := fxtest.New(t,
+		fx.Provide(
+			globals.New,
+			logger.New,
+			config.New,
+			reportbuf.New,
+		),
+		fx.Invoke(func(*reportbuf.Buffer) {}),
+	)
+
+	err := app.Start(t.Context())
+	if err == nil {
+		t.Fatal("start succeeded with DATA_DIR below a regular file")
+	}
+
+	want := fmt.Sprintf("DATA_DIR %q: ", dir)
+	if !strings.Contains(err.Error(), want) {
+		t.Fatalf("start error = %q, want it to contain %q", err, want)
+	}
+}
+
 // TestStopWaitsForPeriodicWrite holds open the write the periodic flush
 // starts, then stops the app. The stop must wait until the write is
 // released, and the report must then be in its file. The test runs in

test/unit/main.test.js

@@ -6,6 +6,7 @@ import { beforeEach, test } from "node:test";
 import assert from "node:assert/strict";
 import {
     AppState,
+    buildUI,
     CONFIG,
     greyOutUI,
     hostRowHTML,
@@ -17,6 +18,8 @@ import {
     measureLatency,
     Reporter,
     tick,
+    updateClocks,
+    updateSummary,
 } from "../../web/src/main.js";
 
 // There is no page here, so the tests stand in for it. The debug log looks
@@ -27,8 +30,10 @@ import {
 // selector until the next test starts. As on a page, writing its text
 // replaces its markup; the status dot greyOutUI looks for in it is not
 // there. Drawing a sparkline does nothing; it looks for the pixel ratio on
-// window and finds none. In each test, console.error and console.info
-// print nothing and keep what they are given.
+// window and finds none. The commit the page's footer names, which
+// vite.config.js fills in when it builds the page, is blank. In each test,
+// console.error and console.info print nothing and keep what they are
+// given.
 const doNothing = () => {};
 let elements;
 beforeEach((t) => {
@@ -50,6 +55,8 @@ const canvasContext = {
 globalThis.window = {};
 globalThis.requestAnimationFrame = doNothing;
 globalThis.cancelAnimationFrame = doNothing;
+globalThis.__COMMIT_HASH__ = "";
+globalThis.__COMMIT_FULL__ = "";
 globalThis.document = {
     getElementById: () => null,
     createElement: () => ({}),
@@ -578,6 +585,32 @@ test("a check that timed out or found its target unreachable has a grey figure a
     assert.equal(latencyHex(null), "#6b7280");
 });
 
+// buildUI draws the page into #app, here a plain object. Each entry in the
+// footer's color legend is a colored dot and the latencies it stands for,
+// with < and > written as &lt; and &gt;. A latency takes the color of the
+// first entry in CONFIG.latencyColors whose limit it is below, so the
+// legend has, for each of those entries in order, a dot of its color (a
+// bg- class where CONFIG.latencyColors has a text- one) and the latencies
+// below its limit, or, for the last, which has no limit, the latencies
+// from the limit before it up. A gray dot for offline comes after them.
+test("the footer's color legend has each entry in CONFIG.latencyColors, with its color and limit, then offline", (t) => {
+    const app = {};
+    t.mock.method(globalThis.document, "getElementById", () => app);
+    buildUI(new AppState([]));
+
+    const legend = [
+        ...app.innerHTML.matchAll(/(bg-\w+-500) [^"]*"><\/span>(\S+)/g),
+    ].map(([, dot, latencies]) => [dot, latencies]);
+    const colors = CONFIG.latencyColors;
+    const expected = colors.map((color, i) => [
+        color.className.replace("text-", "bg-"),
+        color.below === Infinity
+            ? `&gt;=${colors[i - 1].below}ms`
+            : `&lt;${color.below}ms`,
+    ]);
+    assert.deepEqual(legend, [...expected, ["bg-gray-500", "offline"]]);
+});
+
 // A target whose checks, in turn, answered after each of latencies ms, or,
 // for null, found it unreachable.
 function hostAfter(latencies) {
@@ -654,6 +687,102 @@ test("the summary's min, max, median and average latency over the WAN targets' l
     });
 });
 
+// In the next two tests each element of the summary that updateSummary
+// writes into is a plain object, made the first time it is looked up and
+// kept in elements under "#" and its id.
+
+// The interval menu resets each target and counts the rounds from 0 again,
+// then updates the summary, before any check since the change is recorded.
+test("after an interval change, the summary's Reachable slot reads --/-- in white, as at page load", (t) => {
+    t.mock.method(
+        globalThis.document,
+        "getElementById",
+        (id) => (elements[`#${id}`] ??= {}),
+    );
+    const state = new AppState([]);
+    for (const host of state.wan) {
+        host.pushSample(Date.now(), { latency: 30, error: null });
+    }
+    state.tickCount = 5;
+
+    for (const host of state.allHosts) host.reset();
+    state.tickCount = 0;
+    updateSummary(state);
+
+    assert.equal(elements["#summary-reachable"].textContent, "--/--");
+    assert.equal(elements["#summary-reachable"].className, "text-white");
+});
+
+test("once every WAN target's check has failed, the summary's Reachable slot reads 0 of them, in red", (t) => {
+    t.mock.method(
+        globalThis.document,
+        "getElementById",
+        (id) => (elements[`#${id}`] ??= {}),
+    );
+    const state = new AppState([]);
+    for (const host of state.wan) {
+        host.pushSample(Date.now(), { latency: null, error: "timeout" });
+    }
+
+    updateSummary(state);
+
+    assert.equal(
+        elements["#summary-reachable"].textContent,
+        `0/${state.wan.length}`,
+    );
+    assert.equal(elements["#summary-reachable"].className, "text-red-400");
+});
+
+// In the next two tests each of the header's clocks is a plain object, made
+// the first time updateClocks looks it up and kept in elements under "#" and
+// its id.
+
+// The mocked clock reads 1970-01-01T00:00:00.000Z.
+test("the header's clocks show the local time with its time zone's name, and the UTC time", (t) => {
+    t.mock.method(
+        globalThis.document,
+        "getElementById",
+        (id) => (elements[`#${id}`] ??= {}),
+    );
+    t.mock.timers.enable({ apis: ["Date"] });
+
+    updateClocks();
+
+    assert.match(
+        elements["#clock-local"].textContent,
+        /^Local: \d{4}-\d\d-\d\d \d\d:\d\d:\d\d \S/,
+    );
+    assert.equal(
+        elements["#clock-utc"].textContent,
+        "UTC: 1970-01-01 00:00:00 UTC",
+    );
+});
+
+// Here Intl.DateTimeFormat is as in Chrome before 95, Firefox before 91 or
+// Safari before 15.4: it throws a RangeError when asked for the time zone's
+// "shortGeneric" name.
+test("in a browser that cannot name the time zone, updating the clocks throws nothing and leaves them blank", (t) => {
+    t.mock.method(
+        globalThis.document,
+        "getElementById",
+        (id) => (elements[`#${id}`] ??= {}),
+    );
+    const DateTimeFormat = Intl.DateTimeFormat;
+    t.mock.method(Intl, "DateTimeFormat", function (locales, options) {
+        if (options?.timeZoneName === "shortGeneric") {
+            throw new RangeError(
+                "Value shortGeneric out of range for Intl.DateTimeFormat options property timeZoneName",
+            );
+        }
+        return new DateTimeFormat(locales, options);
+    });
+
+    updateClocks();
+
+    assert.equal(elements["#clock-local"].textContent, "");
+    assert.equal(elements["#clock-utc"].textContent, "");
+});
+
 // An app state in which, of the WAN targets, the first timedOut timed out,
 // the next unreachable were found unreachable, the next answered answered
 // after latency ms, and the rest have not been checked yet.

web/src/main.js

@@ -829,7 +829,7 @@ function wanRowsHTML(wanHosts) {
     return rows.join("");
 }
 
-function buildUI(state) {
+export function buildUI(state) {
     const app = document.getElementById("app");
     app.innerHTML = `
     <div class="mx-auto px-[5%] py-8">
@@ -911,7 +911,7 @@ function buildUI(state) {
           <span class="inline-block w-3 h-3 rounded-full bg-lime-500 mr-1 ml-3 align-middle"></span>&lt;100ms
           <span class="inline-block w-3 h-3 rounded-full bg-yellow-500 mr-1 ml-3 align-middle"></span>&lt;200ms
           <span class="inline-block w-3 h-3 rounded-full bg-orange-500 mr-1 ml-3 align-middle"></span>&lt;500ms
-          <span class="inline-block w-3 h-3 rounded-full bg-red-500 mr-1 ml-3 align-middle"></span>&gt;500ms
+          <span class="inline-block w-3 h-3 rounded-full bg-red-500 mr-1 ml-3 align-middle"></span>&gt;=500ms
           <span class="inline-block w-3 h-3 rounded-full bg-gray-500 mr-1 ml-3 align-middle"></span>offline
         </p>
         <p class="mt-2"><a href="https://git.eeqj.de/sneak/netwatch/commit/${__COMMIT_FULL__}" target="_blank" rel="noopener" class="text-gray-600 hover:text-gray-400">${__COMMIT_HASH__}</a></p>
@@ -995,7 +995,7 @@ function updateHostRow(host, index) {
     SparklineRenderer.draw(canvas, host.history);
 }
 
-function updateSummary(state) {
+export function updateSummary(state) {
     const stats = state.wanStats();
     const hstats = state.wanHistoryStats();
 
@@ -1014,13 +1014,20 @@ function updateSummary(state) {
     const hmaxUnitEl = document.getElementById("summary-hmax-unit");
     if (!reachableEl) return;
 
-    reachableEl.textContent = `${stats.reachable}/${stats.total}`;
-    reachableEl.className =
-        stats.reachable === stats.total
-            ? "text-green-400"
-            : stats.reachable === 0
-              ? "text-red-400"
-              : "text-yellow-400";
+    // Before any WAN host's check is recorded, as after an interval change,
+    // no host has answered or failed, so the slot reads as at page load.
+    if (state.wan.every((h) => h.status === "pending")) {
+        reachableEl.textContent = "--/--";
+        reachableEl.className = "text-white";
+    } else {
+        reachableEl.textContent = `${stats.reachable}/${stats.total}`;
+        reachableEl.className =
+            stats.reachable === stats.total
+                ? "text-green-400"
+                : stats.reachable === 0
+                  ? "text-red-400"
+                  : "text-yellow-400";
+    }
 
     if (stats.min !== null) {
         minEl.textContent = `${stats.min}`;
@@ -1107,6 +1114,33 @@ function updateHealthBox(state) {
     else if (health === "healthy") log.notice(`Health: ${labels[health]}`);
 }
 
+// Writes the local time, with its time zone's name, and the UTC time into
+// the header's clocks. The clocks only decorate the page: in a browser
+// whose Intl.DateTimeFormat rejects the "shortGeneric" time zone name
+// (Chrome before 95, Firefox before 91, Safari before 15.4) they are left
+// blank.
+export function updateClocks() {
+    let local = "";
+    let utc = "";
+    try {
+        const now = new Date();
+        const zone = new Intl.DateTimeFormat("en-US", {
+            timeZoneName: "shortGeneric",
+        })
+            .formatToParts(now)
+            .find((p) => p.type === "timeZoneName").value;
+        const pad = (n) => String(n).padStart(2, "0");
+        local = `Local: ${now.toLocaleString("sv-SE", { hour12: false })} ${zone}`;
+        utc =
+            `UTC: ${now.getUTCFullYear()}-${pad(now.getUTCMonth() + 1)}-${pad(now.getUTCDate())} ` +
+            `${pad(now.getUTCHours())}:${pad(now.getUTCMinutes())}:${pad(now.getUTCSeconds())} UTC`;
+    } catch {
+        // Both clocks are left blank.
+    }
+    document.getElementById("clock-local").textContent = local;
+    document.getElementById("clock-utc").textContent = utc;
+}
+
 // --- Debug Log Renderer ------------------------------------------------------
 
 // One line of the debug log panel, its message set as text.
@@ -1502,24 +1536,6 @@ async function init() {
         sortAndRebuildWAN(state);
     });
 
-    function updateClocks() {
-        const now = new Date();
-        const pad = (n) => String(n).padStart(2, "0");
-        const utc =
-            `${now.getUTCFullYear()}-${pad(now.getUTCMonth() + 1)}-${pad(now.getUTCDate())} ` +
-            `${pad(now.getUTCHours())}:${pad(now.getUTCMinutes())}:${pad(now.getUTCSeconds())} UTC`;
-        const local =
-            now.toLocaleString("sv-SE", { hour12: false }) +
-            " " +
-            new Intl.DateTimeFormat("en-US", { timeZoneName: "shortGeneric" })
-                .formatToParts(now)
-                .find((p) => p.type === "timeZoneName").value;
-        document.getElementById("clock-local").textContent = "Local: " + local;
-        document.getElementById("clock-utc").textContent = "UTC: " + utc;
-    }
-    updateClocks();
-    setInterval(updateClocks, 1000);
-
     // Rounds never overlap: a round first gives up the last round's checks
     // if they are still waiting, and the last round then records nothing
     // more. At a steady interval they never are, as they time out at 80% of
@@ -1599,6 +1615,11 @@ async function init() {
 
     window.addEventListener("resize", () => handleResize(state));
     setTimeout(() => handleResize(state), CONFIG.resizeDelay);
+
+    // The clocks start last, after the rounds: they only decorate the page,
+    // so nothing in them may stop the monitor from probing.
+    updateClocks();
+    setInterval(updateClocks, 1000);
 }
 
 // Bootstrap only when loaded as the page: a real DOM containing the #app