prepare-data-dir leaves files with a second name to their owner (closes #143)

A hard link in /data shares its owner with a file that can be anywhere
on the same filesystem, so chowning it gave that file to the netwatch
user. A file with more than one name now keeps its owner and its path
is logged.

Owners are now set on the opened directory, file or link after
checking it, as chmodDir sets modes, so a hard link swapped in
afterwards is caught. Every other file is opened without following it;
a symbolic link is given to the netwatch user itself, and devices,
named pipes and sockets keep their owner. This needs Linux, the
image's system; elsewhere it returns an error. A failed owner change
names the file.

Model: opus-5-5

README.md

@@ -212,15 +212,8 @@ 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. 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).
+rate-limited instead. 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

@@ -29,29 +29,12 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
   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
+  pipes and sockets keep their owner. Opening a file without following it and
+  setting the owner on the open file needs Linux, the system of the image; 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: 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

configs/nginx.conf

@@ -46,9 +46,7 @@ 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. 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.
+    # with: the backend takes the first entry, which a client can write.
     proxy_set_header Host $host;
     proxy_set_header X-Real-IP $remote_addr;
     proxy_set_header X-Forwarded-For $remote_addr;

internal/middleware/middleware.go

@@ -11,7 +11,6 @@ import (
 	"net/http"
 	"net/netip"
 	"runtime/debug"
-	"slices"
 	"strings"
 	"time"
 
@@ -134,17 +133,6 @@ 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,
@@ -156,49 +144,28 @@ func clientIP(
 		return peer
 	}
 
-	forwarded := forwardedFor(header)
-
-	if len(forwarded) == 0 {
-		if xr := strings.TrimSpace(header.Get("X-Real-IP")); validIP(xr) {
-			return xr
-		}
-
-		return peer
+	if xff := firstForwardedFor(header.Get("X-Forwarded-For")); xff != "" {
+		return xff
 	}
 
-	for _, entry := range slices.Backward(forwarded) {
-		if !validIP(entry) {
-			return peer
-		}
-
-		if !addrInAny(entry, trusted) {
-			return entry
-		}
+	if xr := strings.TrimSpace(header.Get("X-Real-IP")); validIP(xr) {
+		return xr
 	}
 
 	return peer
 }
 
-// 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
-
+// firstForwardedFor returns the left-most valid address in an
+// X-Forwarded-For list (the original client), or "" if none.
+func firstForwardedFor(value string) string {
 	for part := range strings.SplitSeq(value, ",") {
-		entry := strings.TrimSpace(part)
-		if entry != "" {
-			entries = append(entries, ipFromHostPort(entry))
+		candidate := strings.TrimSpace(part)
+		if validIP(candidate) {
+			return candidate
 		}
 	}
 
-	return entries
+	return ""
 }
 
 func validIP(s string) bool {

internal/middleware/middleware_test.go

@@ -25,16 +25,10 @@ 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 {
@@ -91,35 +85,10 @@ func clientIPCases() []clientIPCase {
 			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 uses left-most of chain",
+			remoteAddr: "10.1.2.3:5000",
+			xff:        forwardedIP + ", 10.1.2.3",
+			want:       forwardedIP,
 		},
 		{
 			name:       "trusted proxy falls back to x-real-ip",
@@ -148,7 +117,7 @@ func clientIPCases() []clientIPCase {
 			name:       "trusted proxy with garbage header uses peer",
 			remoteAddr: loopbackPeer,
 			xff:        "not-an-ip",
-			want:       loopbackIP,
+			want:       "127.0.0.1",
 		},
 	}
 }
@@ -179,47 +148,6 @@ 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_linux.go

@@ -24,10 +24,9 @@ import (
 // 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).
+// This file is built only on Linux, because O_PATH and AT_EMPTY_PATH
+// exist only there; prepare-data-dir runs only in the Linux image. 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 {

internal/reportbuf/datadir_other.go

@@ -10,12 +10,11 @@ import (
 
 // 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.
+// giving it to a user on the open file, never by name, needs O_PATH
+// and AT_EMPTY_PATH, which only Linux has (see datadir_linux.go), and
+// prepare-data-dir runs only in the Linux image. 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}
 }