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

TODO.md

@@ -21,6 +21,20 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
 
 # Completed Steps
 
+- 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. 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

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
 

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/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,64 @@
+//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, 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 {
+		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,20 @@
+//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, 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}
+}

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 (