2 Commits
Author SHA1 Message Date
sneak d0fd1371da prepare-data-dir leaves files with a second name to their owner (closes #143)
check / check (push) Canceled after 0s
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 regular file or symbolic link 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. A symbolic link is opened without following it
and given to the netwatch user itself, as the server must own a link
it follows in a sticky directory anyone can write to. Other files,
such as devices, keep their owner.

Model: opus-5-5
2026-10-18 21:19:37 +00:00
clawbot e43a710114 Summary reads --/-- after an interval change, not 0/26 in red (closes #140)
check / check (push) Canceled after 0s
The interval menu resets every host to pending and updates the summary,
which counted the WAN hosts with a latency recorded, none, and showed
0/26 in red until the second round after the change ended. While every
WAN host still waits for its first check, the Reachable slot now reads
--/-- in white, as at page load. updateSummary is exported for a unit
test that resets the hosts as the interval menu does; a second test
checks that the slot still reads 0/26 in red once every check failed.

Model: opus-5-5
2026-10-18 23:00:35 +02:00
8 changed files with 147 additions and 52 deletions

No files matched your search

+17 -8
View File
@@ -23,14 +23,23 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
- 2026-10-18: `netwatch-server prepare-data-dir` no longer changes the owner of - 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 a file outside `/data` through a hard link in the volume
([#143](https://git.eeqj.de/sneak/netwatch/issues/143)). A regular file with a ([#143](https://git.eeqj.de/sneak/netwatch/issues/143)). A regular file or
second name (hard link) keeps its owner, and the start logs its path. Owners symbolic link with a second name (hard link) keeps its owner, and the start
are now set on the opened directory or file, never by name, so a hard link logs its path. Owners are now set on the opened directory, file or link, never
swapped in after the check is caught too; symbolic links and other files that by name, so a hard link swapped in after the check is caught too; a symbolic
are neither directories nor regular files keep their owner, as setting it by link is opened without following it and given to the netwatch user itself, so
name could reach a hard link swapped in at that name. The test that runs as the server can still follow one in a sticky directory that anyone can write
root hard-links a file from outside the volume into it and finds its owner to. Other files that are neither directories, regular files nor symbolic links
unchanged keep their owner. The test that runs as root hard-links a file from outside
the volume into it and finds its owner 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
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 - 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, way, the periodic flush's or the one `Append` starts when 10 MiB are waiting,
before the final flush before the final flush
+6 -5
View File
@@ -66,11 +66,12 @@ done > /etc/nginx/trusted-proxies.conf
# netwatch-server keeps its report files in DATA_DIR, on the /data # 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 # 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 # uid. Here, as root, netwatch-server prepare-data-dir creates DATA_DIR
# and gives /data and the directories and regular files in it to the # and gives /data and the directories, regular files and symbolic links
# netwatch user, so the host directory needs no preparing. It stops the # in it to the netwatch user, so the host directory needs no preparing,
# start, naming DATA_DIR, unless DATA_DIR is /data or a path below it, # except a file with a second name (hard link), which keeps its owner,
# and it acts on nothing outside /data, whatever symbolic or hard links # as that name could be outside /data. It stops the start, naming
# it meets there. # 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}" export DATA_DIR="${DATA_DIR:-/data/reports}"
netwatch-server prepare-data-dir "$DATA_DIR" || exit 1 netwatch-server prepare-data-dir "$DATA_DIR" || exit 1
+1 -1
View File
@@ -17,6 +17,7 @@ require (
github.com/slok/go-http-metrics v0.13.0 github.com/slok/go-http-metrics v0.13.0
github.com/spf13/viper v1.21.0 github.com/spf13/viper v1.21.0
go.uber.org/fx v1.24.0 go.uber.org/fx v1.24.0
golang.org/x/sys v0.47.0
) )
require ( require (
@@ -41,7 +42,6 @@ require (
go.uber.org/multierr v1.10.0 // indirect go.uber.org/multierr v1.10.0 // indirect
go.uber.org/zap v1.26.0 // indirect go.uber.org/zap v1.26.0 // indirect
go.yaml.in/yaml/v3 v3.0.4 // 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 golang.org/x/text v0.40.0 // indirect
google.golang.org/protobuf v1.36.11 // indirect google.golang.org/protobuf v1.36.11 // indirect
) )
+39 -28
View File
@@ -9,6 +9,8 @@ import (
"path/filepath" "path/filepath"
"strconv" "strconv"
"syscall" "syscall"
"golang.org/x/sys/unix"
) )
// ErrDataDirOutsideVolume is returned by PrepareDataDir for a DATA_DIR // ErrDataDirOutsideVolume is returned by PrepareDataDir for a DATA_DIR
@@ -19,13 +21,13 @@ var ErrDataDirOutsideVolume = errors.New(
// PrepareDataDir gets dir, the server's DATA_DIR, ready for owner, the // 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, // 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 // /data in the image, needs no preparing: it creates dir, gives volume
// and the directories and regular files in it to owner, and gives // and the directories, regular files and symbolic links in it to
// volume and dir the mode the server gives a directory it creates. It // owner, and gives volume and dir the mode the server gives a directory
// leaves as they are a regular file with a second name (hard link), as // it creates. It leaves as they are a regular file or symbolic link
// that name could be outside volume, and any file that is neither a // with a second name (hard link), as that name could be outside volume,
// directory nor a regular file, such as a symbolic link (see // and any other file, such as a device (see chownAll). dir must be
// chownAll). dir must be volume or a path below it, with no '.', '..', // volume or a path below it, with no '.', '..', empty part or '/' at
// empty part or '/' at the end. // the end.
// //
// bin/entrypoint.sh runs this as root, which would follow a symbolic // bin/entrypoint.sh runs this as root, which would follow a symbolic
// link anywhere, so every step goes through an os.Root opened on // link anywhere, so every step goes through an os.Root opened on
@@ -83,18 +85,22 @@ func PrepareDataDir(volume, dir string, owner *user.User) error {
return chmodDir(root, rel) return chmodDir(root, rel)
} }
// chownAll gives name, a directory inside root, and the directories // chownAll gives name, a directory inside root, and the directories,
// and regular files in it to uid and gid. It opens each through root // regular files and symbolic links in it to uid and gid. It opens each
// and gives the open file to them, as chmodDir sets a mode on the open // through root and gives the open file to them, as chmodDir sets a
// directory. Root.Lchown would not hold: it acts on whatever is at the // mode on the open directory. Root.Lchown would not hold: it acts on
// name by then, and a hard link swapped in there shares its owner with // whatever is at the name by then, and a hard link swapped in there
// a file that could be outside root. chownAll goes into an entry only // shares its owner with a file that could be outside root. A symbolic
// when the read found a directory there, so it follows no link it // link is given to them itself, never what it points to, because in a
// finds; one swapped in afterwards for a directory or a regular file // directory that anyone can write to and that has the sticky bit set,
// is followed only as the os.Root allows. It leaves any file that is // as /tmp has, the kernel follows a link only for the link's owner or
// neither a directory nor a regular file, such as a symbolic link: // when the link and the directory have the same owner. chownAll goes
// opening one would follow the link or open the device, Root.Lchown // into an entry only when the read found a directory there, so it
// would not hold, and the server needs to own none of them. // 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 a
// regular file or symbolic link is not followed at all (see
// chownFile). It leaves any other file, such as a device: the server
// needs to own none of them.
func chownAll(root *os.Root, name string, uid, gid int) error { func chownAll(root *os.Root, name string, uid, gid int) error {
entries, err := chownDir(root, name, uid, gid) entries, err := chownDir(root, name, uid, gid)
if err != nil { if err != nil {
@@ -107,7 +113,7 @@ func chownAll(root *os.Root, name string, uid, gid int) error {
switch { switch {
case entry.IsDir(): case entry.IsDir():
err = chownAll(root, entryName, uid, gid) err = chownAll(root, entryName, uid, gid)
case entry.Type().IsRegular(): case entry.Type().IsRegular(), entry.Type() == fs.ModeSymlink:
err = chownFile(root, entryName, uid, gid) err = chownFile(root, entryName, uid, gid)
} }
@@ -149,14 +155,16 @@ func chownDir(root *os.Root, name string, uid, gid int) ([]fs.DirEntry, error) {
return dir.ReadDir(-1) return dir.ReadDir(-1)
} }
// chownFile gives name, a regular file inside root, to uid and gid, // chownFile gives name, a regular file or symbolic link inside root, to
// unless it has a second name (hard link): all the names of a file // uid and gid, unless it has a second name (hard link): all the names
// share its owner, and the other could be outside root. It opens name // of a file share its owner, and the other could be outside root. It
// through root, reads the number of names from the open file and gives // opens name through root with O_PATH, which reads nothing from the
// that file to them, so a hard link swapped in at name is caught too. // file; os.Root adds O_NOFOLLOW, and the two together open a symbolic
// It logs a file it leaves. // link itself, never what it points to. It reads the number of names
// from the open file and gives that file to them, so a hard link
// swapped in at name is caught too. It logs a file it leaves.
func chownFile(root *os.Root, name string, uid, gid int) error { func chownFile(root *os.Root, name string, uid, gid int) error {
file, err := root.Open(name) file, err := root.OpenFile(name, unix.O_PATH, 0)
if err != nil { if err != nil {
return err return err
} }
@@ -168,9 +176,12 @@ func chownFile(root *os.Root, name string, uid, gid int) error {
return err return err
} }
// 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.
stat, ok := info.Sys().(*syscall.Stat_t) stat, ok := info.Sys().(*syscall.Stat_t)
if ok && stat.Nlink == 1 { if ok && stat.Nlink == 1 {
return file.Chown(uid, gid) return unix.Fchownat(int(file.Fd()), "", uid, gid, unix.AT_EMPTY_PATH)
} }
slog.Warn("owner not changed: file has a second name (hard link)", slog.Warn("owner not changed: file has a second name (hard link)",
+25 -8
View File
@@ -254,11 +254,12 @@ func TestPrepareDataDirTakesDirectoryNamedInLatin1(t *testing.T) {
requireOwner(t, filepath.Join(latin1, "f"), uint32(uid), uint32(gid)) requireOwner(t, filepath.Join(latin1, "f"), uint32(uid), uint32(gid))
} }
// TestPrepareDataDirGivesVolumeToOwner gives the directories and // TestPrepareDataDirGivesVolumeToOwner gives the directories, regular
// regular files in the volume to a uid and gid that own nothing, which // files and symbolic links in the volume to a uid and gid that own
// only root can do. A symbolic link in the volume to a directory // nothing, which only root can do. A symbolic link in the volume to a
// outside it, and a hard link to a file outside it, are left as they // directory outside it is given to them itself; what it points to is
// were, and so is what they lead to. // left as it was. A hard link to a file outside the volume, and one to
// a symbolic link outside it, are left as they were.
func TestPrepareDataDirGivesVolumeToOwner(t *testing.T) { func TestPrepareDataDirGivesVolumeToOwner(t *testing.T) {
t.Parallel() t.Parallel()
@@ -295,6 +296,19 @@ func TestPrepareDataDirGivesVolumeToOwner(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
// os.Link does not follow a symbolic link: it gives the link itself
// a second name.
err = os.Symlink("f", filepath.Join(outside, "link"))
if err != nil {
t.Fatal(err)
}
err = os.Link(filepath.Join(outside, "link"),
filepath.Join(old, "hardlink-to-link"))
if err != nil {
t.Fatal(err)
}
err = reportbuf.PrepareDataDir(volume, filepath.Join(volume, reports), err = reportbuf.PrepareDataDir(volume, filepath.Join(volume, reports),
&user.User{Uid: "4242", Gid: "4343"}) &user.User{Uid: "4242", Gid: "4343"})
if err != nil { if err != nil {
@@ -303,11 +317,14 @@ func TestPrepareDataDirGivesVolumeToOwner(t *testing.T) {
for _, path := range []string{ for _, path := range []string{
volume, filepath.Join(volume, reports), old, filepath.Join(old, "f"), volume, filepath.Join(volume, reports), old, filepath.Join(old, "f"),
filepath.Join(old, "link"),
} { } {
requireOwner(t, path, 4242, 4343) requireOwner(t, path, 4242, 4343)
} }
requireOwner(t, filepath.Join(old, "link"), 0, 0) for _, path := range []string{
requireOwner(t, outside, 0, 0) outside, filepath.Join(outside, "f"), filepath.Join(outside, "link"),
requireOwner(t, filepath.Join(outside, "f"), 0, 0) } {
requireOwner(t, path, 0, 0)
}
} }
+4 -1
View File
@@ -1,6 +1,9 @@
// Package reportbuf accumulates telemetry reports in memory // Package reportbuf accumulates telemetry reports in memory
// and periodically flushes them to zstd-compressed JSONL files, // 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 package reportbuf
import ( import (
+47
View File
@@ -17,6 +17,7 @@ import {
measureLatency, measureLatency,
Reporter, Reporter,
tick, tick,
updateSummary,
} from "../../web/src/main.js"; } from "../../web/src/main.js";
// There is no page here, so the tests stand in for it. The debug log looks // There is no page here, so the tests stand in for it. The debug log looks
@@ -654,6 +655,52 @@ 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");
});
// An app state in which, of the WAN targets, the first timedOut timed out, // An app state in which, of the WAN targets, the first timedOut timed out,
// the next unreachable were found unreachable, the next answered answered // the next unreachable were found unreachable, the next answered answered
// after latency ms, and the rest have not been checked yet. // after latency ms, and the rest have not been checked yet.
+8 -1
View File
@@ -995,7 +995,7 @@ function updateHostRow(host, index) {
SparklineRenderer.draw(canvas, host.history); SparklineRenderer.draw(canvas, host.history);
} }
function updateSummary(state) { export function updateSummary(state) {
const stats = state.wanStats(); const stats = state.wanStats();
const hstats = state.wanHistoryStats(); const hstats = state.wanHistoryStats();
@@ -1014,6 +1014,12 @@ function updateSummary(state) {
const hmaxUnitEl = document.getElementById("summary-hmax-unit"); const hmaxUnitEl = document.getElementById("summary-hmax-unit");
if (!reachableEl) return; if (!reachableEl) return;
// 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.textContent = `${stats.reachable}/${stats.total}`;
reachableEl.className = reachableEl.className =
stats.reachable === stats.total stats.reachable === stats.total
@@ -1021,6 +1027,7 @@ function updateSummary(state) {
: stats.reachable === 0 : stats.reachable === 0
? "text-red-400" ? "text-red-400"
: "text-yellow-400"; : "text-yellow-400";
}
if (stats.min !== null) { if (stats.min !== null) {
minEl.textContent = `${stats.min}`; minEl.textContent = `${stats.min}`;