Entrypoint checks DATA_DIR in full before acting on it as root (closes #80)

`bin/entrypoint.sh` now checks `DATA_DIR` before it creates anything or
changes an owner or mode: it must be `/data` or a path below it with no
`.`, `..` or empty part, and no part of it that exists, `/data`
included, may be a symbolic link. Anything else stops the start with one
message naming `DATA_DIR`. Only then does it create `DATA_DIR`, give
`/data` and everything in it to `netwatch` (`chown -R -h`, so a link in
it is not followed) and set mode 750 on `/data` and `DATA_DIR`. The
README section "Running under upaas" says which values are accepted.

Model: opus-5-5

README.md

@@ -52,8 +52,8 @@ halves, so the root `make check` fails if either one is broken. We provide:
 - `script/fmt` — format all files (writes): prettier, then gofmt over `backend/`
 - `script/fmt-check` — check formatting (read-only): prettier, then gofmt
 - `script/check` — run test, lint, and fmt-check
-- `script/frontend-test` — run the unit tests in `test/unit/` with Node's
-  built-in test runner, then the production build
+- `script/frontend-test` — run the production build as the frontend's test (no
+  unit tests yet)
 - `script/frontend-lint` — run prettier in check mode
 - `script/frontend-fmt` — format everything prettier understands (writes)
 - `script/frontend-fmt-check` — check prettier formatting (read-only)
@@ -136,11 +136,8 @@ Local hosts are tracked separately from WAN stats.
 ### Latency measurement
 
 HEAD requests with `mode: 'no-cors'` and `cache: 'no-store'`, timed with
-`performance.now()`. Each check times out after 80% of the refresh interval (24
-seconds at 30 seconds) and is then recorded as a timeout, so a round's checks
-have all finished before the next round is due. A round due while the last one
-is still waiting, which happens only after an interval change or when the
-recovery probe starts one, is skipped. IPv4 only.
+`performance.now()`. 1-second timeout; anything over 1000ms is clamped to
+unreachable. IPv4 only.
 
 ### Color coding
 
@@ -219,8 +216,10 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs:
     - `CORS_ALLOWED_ORIGINS`, default empty: other origins whose pages may call
       the API
     - `DEBUG`, default `false`: debug logging
-    - `DATA_DIR`, default `/data/reports`: leave unset; reports kept outside
-      `/data` do not survive a redeploy
+    - `DATA_DIR`, default `/data/reports`: the directory the reports are kept
+      in: `/data` or a path below it, with no `.` or `..` part and no extra `/`.
+      The container also stops if a part of the path that exists, `/data`
+      included, is a symbolic link
     - `TRUSTED_PROXIES`, default empty: set it to the address the reverse proxy
       in front of the container connects from, as an IP address or CIDR; several
       are separated by commas. nginx takes the client address from

TODO.md

@@ -23,13 +23,14 @@ latest run passes.
 
 # Completed Steps
 
-- 2026-09-29: each target check times out after 80% of the refresh interval
-  (issue #78), 24 seconds at 30 seconds, where it was capped at 3 seconds. A
-  round due while the last one's checks are still waiting is skipped, so rounds
-  never overlap, and the recovery probe starts no new checks while its last ones
-  are waiting. The frontend has its first unit tests, run by
-  `script/frontend-test` with Node's built-in test runner; for them,
-  `index.html` now links `src/styles.css`, which `src/main.js` used to import
+- 2026-09-29: `bin/entrypoint.sh` checks `DATA_DIR` in full before it acts on it
+  as root (issue #80): `DATA_DIR` must be `/data` or a path below it with no
+  `.`, `..` or empty part, and no part of it that exists, `/data` included, may
+  be a symbolic link; anything else stops the start with a message naming
+  `DATA_DIR`. Only then is `DATA_DIR` created and `/data` given to `netwatch`,
+  so a refused start no longer creates directories outside `/data`, and
+  `DATA_DIR=/etc` no longer gives `/etc` to `netwatch`. The `README.md` section
+  "Running under upaas" says which values are accepted
 - 2026-09-29: the container sets up its own data directory (issue #75):
   `bin/entrypoint.sh`, still as root, creates `DATA_DIR` if missing and gives it
   and `/data` to the `netwatch` user with mode 750 before starting the backend

bin/entrypoint.sh

@@ -63,25 +63,43 @@ 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. Both are given to the netwatch user here, with the mode the
-# server gives a directory it creates, so the host directory needs no
-# preparing.
+# uid. /data and everything in it are given to the netwatch user here,
+# and /data and DATA_DIR get the mode the server gives a directory it
+# creates, so the host directory needs no preparing.
 #
-# chown and chmod, run as root, change whatever a symbolic link on the
-# path points to, anywhere in the container, and the netwatch user can
-# put one in /data. So the start stops unless readlink -f, which
-# follows every link on a path, gives /data and DATA_DIR back as they
-# are. It also writes a path in full, so a DATA_DIR with '.', '..' or
-# an extra '/' in it is refused too.
+# This runs as root, so nothing is created or changed until DATA_DIR is
+# known to be /data or a path below it, with no '.', '..' or empty
+# part, and no part of it that exists, /data included, is a symbolic
+# link: the netwatch user can put one in /data, and root would follow
+# it anywhere in the container. Nothing else runs in the container yet,
+# so no link can appear after the check.
 export DATA_DIR="${DATA_DIR:-/data/reports}"
-mkdir -p "$DATA_DIR" || exit 1
-if [ "$(readlink -f /data)" != /data ] ||
-    [ "$(readlink -f "$DATA_DIR")" != "$DATA_DIR" ]; then
-    echo "entrypoint: DATA_DIR must be a full path with no '.', '..'," \
-        "extra '/' or symbolic link on it or on /data, not '$DATA_DIR'" >&2
+data_dir_ok() {
+    # With a / added at the end, a last part of '.' or '..', and a / at
+    # the end, match these patterns too.
+    case "$DATA_DIR/" in
+        */./* | */../* | *//*) return 1 ;;
+        /data/*) ;;
+        *) return 1 ;;
+    esac
+    # Each part from DATA_DIR up to /data. [ -L ] is false for a part
+    # that does not exist.
+    dir="$DATA_DIR"
+    while [ "$dir" != /data ]; do
+        [ -L "$dir" ] && return 1
+        dir="${dir%/*}"
+    done
+    [ ! -L /data ]
+}
+if ! data_dir_ok; then
+    echo "entrypoint: DATA_DIR must be /data or a path below it, with no" \
+        "'.', '..', extra '/' or symbolic link on it, not '$DATA_DIR'" >&2
     exit 1
 fi
-chown -R netwatch:netwatch /data "$DATA_DIR" || exit 1
+mkdir -p "$DATA_DIR" || exit 1
+# -h: a symbolic link in /data is itself given to netwatch, not what it
+# points to.
+chown -R -h netwatch:netwatch /data || exit 1
 chmod 750 /data "$DATA_DIR" || exit 1
 
 # A stop signal is only noted here; the loop below acts on it.

index.html

@@ -9,9 +9,6 @@
             type="image/svg+xml"
             href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 100 100'><text y='.9em' font-size='90'>📡</text></svg>"
         />
-        <!-- Linked here, not imported by src/main.js, so the unit tests can
-             import that module in Node, which cannot import CSS. -->
-        <link rel="stylesheet" href="/src/styles.css" />
     </head>
     <body class="bg-gray-900 text-white min-h-screen">
         <div id="app"></div>

script/frontend-test

@@ -1,14 +1,13 @@
 #!/bin/sh
-# script/frontend-test: run the frontend test suite: the unit tests in
-# test/unit/ with Node's built-in test runner, then the production
-# build, which fails on broken code.
+# script/frontend-test: run the frontend test suite. The frontend has no
+# unit tests; the production build serves as the test (fails on broken
+# code).
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
-    timeout 30 node --test test/unit/*.test.js
     timeout 30 yarn build
 }
 

src/main.js

@@ -1,14 +1,14 @@
+import "./styles.css";
+
 // --- Configuration -----------------------------------------------------------
 
-// Timing, axis labels, and display constants. A target check times out
-// after requestTimeout, 80% of updateInterval, so a round's checks have
-// all finished before the next round is due; latency above maxLatency is
-// recorded as a timeout. The sparkline Y-axis is capped at
+// Timing, axis labels, and display constants. Latency above maxLatency is
+// clamped to "unreachable". The sparkline Y-axis is capped at
 // graphMaxLatency — values above it pin to the top of the chart but still
 // display their real value in the latency figure. The history buffer holds
 // maxHistoryPoints samples (historyDuration / updateInterval).
 // reportInterval is how often collected samples are POSTed to the backend.
-export const CONFIG = {
+const CONFIG = {
     updateInterval: 3000,
     maxHistoryPoints: 100,
     reportInterval: 60000,
@@ -16,7 +16,7 @@ export const CONFIG = {
         return (this.maxHistoryPoints * this.updateInterval) / 1000;
     },
     get requestTimeout() {
-        return this.updateInterval * 0.8;
+        return Math.min(this.updateInterval - 100, 3000);
     },
     get maxLatency() {
         return this.requestTimeout;
@@ -503,7 +503,7 @@ class Reporter {
 
 // --- Latency Measurement -----------------------------------------------------
 
-export async function measureLatency(url) {
+async function measureLatency(url) {
     const controller = new AbortController();
     const timeoutId = setTimeout(
         () => controller.abort(),
@@ -1181,16 +1181,11 @@ function startRecoveryProbe(state, triggerTick) {
     log.notice(
         `Recovery probe started (${canaries.map((h) => h.name).join(", ")})`,
     );
-    // A check can wait up to CONFIG.requestTimeout, far longer than
-    // 500ms, so no new checks start while the last ones are waiting.
-    let checking = false;
     state._recoveryProbeId = setInterval(async () => {
-        if (state.paused || checking) return;
-        checking = true;
+        if (state.paused) return;
         const results = await Promise.all(
             canaries.map((h) => measureLatency(h.url)),
         );
-        checking = false;
         if (results.some((r) => r.error === null)) {
             log.notice("Recovery probe: connectivity detected");
             stopRecoveryProbe(state);
@@ -1379,18 +1374,8 @@ async function init() {
     updateClocks();
     setInterval(updateClocks, 1000);
 
-    // A round waits up to CONFIG.requestTimeout for its checks. A round
-    // asked for while one is still waiting, after an interval change or
-    // by the recovery probe, is skipped, so rounds never overlap.
-    let roundRunning = false;
-    async function doTick() {
-        if (roundRunning) return;
-        roundRunning = true;
-        try {
-            await tick(state, () => startRecoveryProbe(state, doTick));
-        } finally {
-            roundRunning = false;
-        }
+    function doTick() {
+        tick(state, () => startRecoveryProbe(state, doTick));
     }
 
     doTick();
@@ -1459,7 +1444,7 @@ async function init() {
 
 // Bootstrap only when loaded as the page: a real DOM containing the #app
 // mount point this module renders into. Importing the module in a unit test
-// (which has no #app) runs nothing, so its exports can be tested in isolation.
+// (which has no #app) runs nothing, so buildReport can be tested in isolation.
 if (typeof document !== "undefined" && document.getElementById("app")) {
     if (document.readyState === "loading") {
         document.addEventListener("DOMContentLoaded", init);

test/unit/main.test.js

@@ -1,52 +0,0 @@
-// Unit tests for src/main.js, run by script/frontend-test with Node's
-// built-in test runner. Importing the module does not start the page.
-
-import { after, before, test } from "node:test";
-import assert from "node:assert/strict";
-import { createServer } from "node:http";
-import { CONFIG, measureLatency } from "../../src/main.js";
-
-// measureLatency writes timeouts to the debug log, which looks for its
-// panel in the page. There is no page here.
-globalThis.document = { getElementById: () => null };
-
-// A target that answers after the number of milliseconds in the path,
-// e.g. /600.
-let server;
-let target;
-
-before(async () => {
-    server = createServer((req, res) => {
-        const delay = Number(new URL(req.url, "http://x").pathname.slice(1));
-        setTimeout(() => res.end(), delay);
-    });
-    await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve));
-    target = `http://127.0.0.1:${server.address().port}`;
-});
-
-after(() => {
-    server.closeAllConnections();
-    server.close();
-});
-
-test("the timeout is 80% of the refresh interval", () => {
-    CONFIG.updateInterval = 30000;
-    assert.equal(CONFIG.requestTimeout, 24000);
-    CONFIG.updateInterval = 3000;
-    assert.equal(CONFIG.requestTimeout, 2400);
-});
-
-test("an answer within the timeout is recorded with its real time, a later one as a timeout", async () => {
-    // 600ms is past the 400ms timeout of a 500ms interval...
-    CONFIG.updateInterval = 500;
-    assert.deepEqual(await measureLatency(`${target}/600`), {
-        latency: null,
-        error: "timeout",
-    });
-
-    // ...and within the 1200ms timeout of a 1500ms interval.
-    CONFIG.updateInterval = 1500;
-    const { latency, error } = await measureLatency(`${target}/600`);
-    assert.equal(error, null);
-    assert.ok(latency >= 600 && latency < 1200, `latency ${latency}ms`);
-});