Rotation race in LogTailer: readRange re-opens by path after stat and can emit garbage lines from the replacement file

[LarsLaskowski]

Severity: Low
Category: Correctness (server, edge case)
Affected file: src/server/logTailer.ts (sync lines 204–217, readRange lines 320–333). Line numbers refer to main @ 3ac64da.

Problem

sync() derives offset and delta from stat(filePath) (via safeStat), and then readRange() performs a fresh open(filePath). If the file is replaced between those two calls — a rotation where the new file is created immediately after the rename — the read operates on the new file at the old file's byte offset:

• If the new file is shorter than offset, the read returns nothing (harmless; the next poll detects truncation/rotation).
• If the new file is already larger than offset (busy logger, or a file replaced in place by a bigger one), the chunk starts mid-line in unrelated content. Those bytes are decoded, split into lines, parsed, buffered, and broadcast to every client as bogus log rows. The next poll detects the inode change via toFileKey and emits rotated, but the garbage rows are already in the server buffer and in every client's view.

This is the non-crashing sibling of #129 (ENOENT crash): both stem from the same stat→open race, and it is natural to fix them together — ideally this refactor first, then #129's catch on top.

Suggested fix

Restructure the read path of sync() to operate on a single FileHandle, eliminating the race entirely: open first, then fstat the handle — the stats are then guaranteed to describe the same file the subsequent read uses.

import { open, type FileHandle } from 'node:fs/promises'; // FileHandle is exported here

// inside sync(), replacing the current safeStat()-then-readRange() sequence:
let fileHandle: FileHandle | null = null;
try {
  fileHandle = await open(this.sourceConfig.filePath, 'r');
} catch (error) {
  this.handleSyncError(error); // same ENOENT/EACCES/other mapping as safeStat() today — see #129
  return;
}

try {
  const fileStats = await fileHandle.stat();   // fstat: same file as the handle, no race
  const nextFileKey = toFileKey(fileStats);

  // ...existing decision logic unchanged: initial-load / reattach / rotation-key-change /
  // truncation / no-op / skip-ahead, based on fileStats and nextFileKey...

  // incremental read through the SAME handle instead of readRange(path, ...):
  const buffer = Buffer.alloc(delta);
  const { bytesRead } = await fileHandle.read(buffer, 0, delta, this.offset);
  const nextChunk = buffer.subarray(0, bytesRead);
  // ...consumeChunk / parser.parse / onLines exactly as today...
} finally {
  await fileHandle.close();
}

Implementation notes:

• The tail-loading branches need no restructuring. handleInitialLoad(), handleReattach(), and skipAheadToTail() call readLastLines(), which already stats via its own handle (fileHandle.stat(), line 346) and is therefore internally race-free. It is fine that readLastLines briefly opens a second handle while the sync() handle is still open. The tiny remaining window (the tail helpers may end up tailing a newer file than the one sync() just stat-ed) only affects which tail gets loaded — never a mis-offset read — and self-corrects on the next poll via the file-key check.
• Preserve the ENOENT semantics of safeStat(). The open() catch must reproduce what safeStat() does today for ENOENT (reset offset/currentFileKey/partial line, emit missing), EACCES/EPERM (emit permission-denied), and anything else (emit error). If Unhandled promise rejection in LogTailer.sync() crashes the process when the log file disappears between stat() and open() #129 is implemented, its handleSyncError() is exactly this mapping — reuse it. safeStat() itself then remains only for loadInitialLines() at startup, or can be dissolved entirely if that path is also switched to open-then-fstat.
• Keep the ordering guarantees: this.offset = fileStats.size must still be assigned only after a successful read (as today, lines 210–212), so a failed read does not skip bytes.
• readRange() (lines 320–333) becomes unused — delete it.
• Side effect worth knowing: the no-change poll path now does open+fstat+close instead of a bare stat per cycle. That is one extra syscall per poll (1 s default) and irrelevant in practice, but do not "optimize" it back to a path-based pre-stat — that would reintroduce the race.

Acceptance criteria

• Existing rotation/truncation tests in src/server/logTailer.test.ts stay green.
• New test: between two sync cycles, replace the tailed file with a larger file (different inode, size > previous offset) and assert that no mid-line garbage row is emitted — the cycle either detects rotation (offset reset, rotated status, clean re-tail) or skips, but never parses new-file bytes at the stale offset.
• New test (or assertion in the above): a file deleted between cycles yields a missing status and no crash (overlaps with Unhandled promise rejection in LogTailer.sync() crashes the process when the log file disappears between stat() and open() #129's criteria when both are implemented).
• npm test and npm run typecheck pass.