docs+test(ingestor): document writeStatsAtomic symlink-replace semantics + regression test (#1170) (#1588)

Fixes #1170.

## What

1. **Doc comment** on `writeStatsAtomic` (`cmd/ingestor/stats_file.go`)
spelling out the two-sided symlink story:
- tmp side (`path+".tmp"`): protected by `O_NOFOLLOW` (existing
behavior, already noted).
- rename side (`path` itself): NOT protected by `O_NOFOLLOW`; instead
`os.Rename` semantics are relied upon — rename atomically replaces any
existing entry at `path` (including a symlink) with the new regular
file. The symlink target is never written through because all writes
happened to the unrelated tmp file before rename.
2. **Regression guardrail test**
`TestWriteStatsAtomic_SymlinkAtDestIsReplaced` in
`cmd/ingestor/stats_file_test.go` that pre-plants a symlink at the
destination path pointing to an unrelated target file, calls
`writeStatsAtomic`, and asserts:
- (a) `os.Lstat(path).Mode()&os.ModeSymlink == 0` (post-write path is a
regular file, not a symlink)
   - (b) the original symlink target's sentinel bytes are unchanged.

If a future refactor swaps `os.Rename` for a
destination-symlink-following primitive (e.g. `open(path, O_WRONLY)`
without `O_NOFOLLOW`, or a copy-then-truncate), the test fails loudly.

## TDD note (red-commit exemption)

The current `writeStatsAtomic` ALREADY satisfies the new test's
assertions — `os.Rename` does the right thing today. Per the fix-issue
skill's exemption for pure-documentation / guardrail tests on
already-correct behavior, no fabricated red commit was constructed; the
test stands as a pinning regression guard. The two commits are
therefore: (1) test addition, (2) doc comment.

## Scope

- `cmd/ingestor/stats_file.go` — doc comment only
- `cmd/ingestor/stats_file_test.go` — one new test function

No production behavior change. No public API change. No new
dependencies. No CI workflow changes. `O_NOFOLLOW` and the existing
tmp-side behavior are untouched.

## Preflight

All hard gates pass (PII, branch scope, red commit, CSS vars,
LIKE-on-JSON, sync/async migration, XSS sinks). No warnings.

---------

Co-authored-by: meshcore-bot <bot@meshcore.local>

cmd/ingestor/stats_file.go

@@ -61,6 +61,25 @@ func statsFilePath() string {
 
 // writeStatsAtomic writes b to path via a tmp-then-rename, refusing to follow
 // symlinks on the tmp file. Returns nil on success, an error otherwise.
+//
+// Symlink semantics (refs #1170):
+//
+//   - tmp side (path+".tmp"): protected by O_NOFOLLOW below. If tmp is a
+//     pre-planted symlink, openat fails with ELOOP instead of writing
+//     through it. This is the defensive-coding path that matters when the
+//     default stats path lives under world-writable /tmp.
+//
+//   - rename side (path): NOT protected by O_NOFOLLOW. Instead, os.Rename's
+//     semantics are relied upon — rename atomically replaces any existing
+//     entry at path (including a symlink) with the new regular file. The
+//     symlink's target is NEVER written through, because all writes happened
+//     to the unrelated tmp file before rename. Post-rename, path is a
+//     regular file (not a symlink) and any prior symlink target's contents
+//     are unchanged. The regression guardrail
+//     TestWriteStatsAtomic_SymlinkAtDestIsReplaced pins this behavior so a
+//     future refactor that swaps os.Rename for a destination-symlink-
+//     following primitive (e.g. an open(path, O_WRONLY) without O_NOFOLLOW)
+//     fails loudly.
 func writeStatsAtomic(path string, b []byte) error {
 	tmp := path + ".tmp"
 	// O_NOFOLLOW: if tmp is a pre-existing symlink, openat fails with ELOOP

cmd/ingestor/stats_file_test.go

@@ -96,3 +96,73 @@ func TestStatsFileWriter_PublishesProcIO(t *testing.T) {
 		}
 	}
 }
+
+// TestWriteStatsAtomic_SymlinkAtDestIsReplaced is a regression guardrail for
+// #1170. The tmp side of writeStatsAtomic uses O_NOFOLLOW so a pre-planted
+// symlink at path+".tmp" cannot redirect the write — but the rename target
+// (`path` itself) is not protected by O_NOFOLLOW. Instead, os.Rename's
+// semantics are relied upon: rename atomically replaces any existing entry
+// at the destination, including a symlink, with the new regular file. The
+// original symlink's target is never written through (because the write
+// happened to the unrelated tmp file).
+//
+// This test pre-plants a symlink at `path` pointing to an unrelated target
+// file and asserts:
+//   (a) post-write, path is a regular file (not a symlink), and
+//   (b) the original target's contents are unchanged.
+//
+// If a future refactor swaps os.Rename for something that follows the
+// destination symlink (e.g. ioutil.WriteFile, or an open(path, O_WRONLY)
+// without O_NOFOLLOW), this test will fail loudly.
+func TestWriteStatsAtomic_SymlinkAtDestIsReplaced(t *testing.T) {
+	dir := t.TempDir()
+
+	// Unrelated target file with sentinel bytes. If writeStatsAtomic ever
+	// followed the symlink at `path`, it would overwrite this file.
+	target := filepath.Join(dir, "unrelated-target.bin")
+	sentinel := []byte("DO-NOT-OVERWRITE-ME-#1170")
+	if err := os.WriteFile(target, sentinel, 0o600); err != nil {
+		t.Fatalf("seed target: %v", err)
+	}
+
+	// Pre-plant a symlink at the destination path.
+	path := filepath.Join(dir, "stats.json")
+	if err := os.Symlink(target, path); err != nil {
+		t.Fatalf("symlink: %v", err)
+	}
+
+	payload := []byte(`{"sampledAt":"2026-01-01T00:00:00Z"}`)
+	if err := writeStatsAtomic(path, payload); err != nil {
+		t.Fatalf("writeStatsAtomic: %v", err)
+	}
+
+	// (a) post-write, path must NOT be a symlink.
+	info, err := os.Lstat(path)
+	if err != nil {
+		t.Fatalf("lstat path: %v", err)
+	}
+	if info.Mode()&os.ModeSymlink != 0 {
+		t.Errorf("post-write path is still a symlink (mode=%v); os.Rename should have atomically replaced it with a regular file", info.Mode())
+	}
+	if !info.Mode().IsRegular() {
+		t.Errorf("post-write path is not a regular file (mode=%v)", info.Mode())
+	}
+
+	// Path now contains the new payload.
+	got, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("read path: %v", err)
+	}
+	if string(got) != string(payload) {
+		t.Errorf("path contents: want %q, got %q", payload, got)
+	}
+
+	// (b) the original symlink target must be unchanged.
+	gotTarget, err := os.ReadFile(target)
+	if err != nil {
+		t.Fatalf("read target: %v", err)
+	}
+	if string(gotTarget) != string(sentinel) {
+		t.Errorf("symlink target was clobbered: want %q, got %q", sentinel, gotTarget)
+	}
+}