Recheck the log directories before reusing the file handle

The reused log handle was checked only by lstat of the file path, which
follows links in the parent path. A logs directory replaced by a link
to the folder the open file was moved to led back to the same file, so
entries kept going there; main refused that directory on every write.

The writer now records the app and logs directories when it opens the
file and reuses the handle only while both are still those directories:
real directories, not links, and on POSIX mode 0700. Otherwise it
reopens with the full checks, which refuse a linked directory and
tighten a loosened mode.

New tests: an entry is not appended through a logs folder replaced by
a link (fails before), and on POSIX a loosened logs folder mode is
restored before the next append. Internals describes the directory
check and that each flushLogs call waits for its own close.

Part of #141

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Author: D0n9X1n

src/lib/log.ts

@@ -194,7 +194,8 @@ const sameFile = (left: Stats, right: Stats): boolean =>
 
 // Do not chmod through an arbitrary symlink. Open the already checked directory
 // without following its final component, and operate on the handle on POSIX.
-const ensurePrivateDirectory = async (directory: string): Promise<void> => {
+// Returns the checked directory's lstat, so a reused log handle can tell it was not replaced.
+const ensurePrivateDirectory = async (directory: string): Promise<Stats> => {
   await fs.mkdir(directory, { mode: 0o700 }).catch((error: unknown) => {
     if ((error as NodeJS.ErrnoException).code !== "EEXIST") {
       throw error
@@ -208,7 +209,7 @@ const ensurePrivateDirectory = async (directory: string): Promise<void> => {
 
   // Windows has no O_NOFOLLOW and no POSIX mode bits, so the lstat check above is all it gets.
   if (process.platform === "win32") {
-    return
+    return observed
   }
 
   const handle = await fs.open(directory, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW)
@@ -222,11 +223,20 @@ const ensurePrivateDirectory = async (directory: string): Promise<void> => {
   } finally {
     await handle.close()
   }
+
+  return observed
 }
 
-const ensureLogDirectory = async (): Promise<void> => {
-  await ensurePrivateDirectory(paths.appDir)
-  await ensurePrivateDirectory(paths.logsDir)
+// The app directory holds the logs directory, so it is checked first.
+const logDirectories = [paths.appDir, paths.logsDir]
+
+const ensureLogDirectory = async (): Promise<Array<Stats>> => {
+  const checked: Array<Stats> = []
+  for (const directory of logDirectories) {
+    checked.push(await ensurePrivateDirectory(directory))
+  }
+
+  return checked
 }
 
 interface LogEntry {
@@ -240,6 +250,8 @@ interface ActiveLog {
   filePath: string
   handle: FileHandle
   identity: Stats
+  // The log directories as checked when the file was opened, in logDirectories order.
+  directories: Array<Stats>
 }
 
 // Entries wait here and are written in call order by one drain at a time, so a burst costs one
@@ -254,19 +266,34 @@ let activeLog: ActiveLog | undefined
 // entry is under 64 KiB, so an entry is never split.
 const maxLogWriteBytes = 256 * 1024
 
-// The open handle is reused only while its path still names the same private file with one link.
-// A rename, deletion, replacement, second hard link or loosened mode reopens with the full checks.
+// Windows has no POSIX mode bits to check.
+const hasMode = (stats: Stats, mode: number): boolean =>
+  process.platform === "win32" || (stats.mode & 0o777) === mode
+
+const isSameDirectory = (current: Stats | undefined, checked: Stats): boolean =>
+  current !== undefined
+    && current.isDirectory()
+    && sameFile(current, checked)
+    && hasMode(current, 0o700)
+
+// The open handle is reused only while its path still names the same private file with one link,
+// inside the same private directories. lstat of the file follows links in its parent path, so the
+// directories are checked too. A rename, deletion, replacement, second hard link, replaced
+// directory or loosened mode reopens with the full checks.
 const isStillActive = async (active: ActiveLog): Promise<boolean> => {
-  const current = await fs.lstat(active.filePath).catch(() => undefined)
+  const [current, ...directories] = await Promise.all(
+    [active.filePath, ...logDirectories].map((target) => fs.lstat(target).catch(() => undefined)),
+  )
 
   return current !== undefined
     && current.isFile()
     && current.nlink === 1
     && sameFile(current, active.identity)
-    && (process.platform === "win32" || (current.mode & 0o777) === 0o600)
+    && hasMode(current, 0o600)
+    && active.directories.every((checked, index) => isSameDirectory(directories[index], checked))
 }
 
-const openPrivateLog = async (filePath: string): Promise<ActiveLog> => {
+const openPrivateLog = async (filePath: string, directories: Array<Stats>): Promise<ActiveLog> => {
   const observed = await fs.lstat(filePath).catch((error: unknown) => {
     if (!isMissing(error)) {
       throw error
@@ -303,7 +330,7 @@ const openPrivateLog = async (filePath: string): Promise<ActiveLog> => {
       await handle.chmod(0o600)
     }
 
-    return { filePath, handle, identity: opened }
+    return { filePath, handle, identity: opened, directories }
   } catch (error) {
     await handle.close().catch(() => undefined)
     throw error
@@ -322,8 +349,8 @@ const openActiveLog = async (filePath: string): Promise<FileHandle> => {
   }
 
   await closeActiveLog()
-  await ensureLogDirectory()
-  activeLog = await openPrivateLog(filePath)
+  const directories = await ensureLogDirectory()
+  activeLog = await openPrivateLog(filePath, directories)
 
   return activeLog.handle
 }

tests/unit/log-format.test.ts

@@ -491,6 +491,42 @@ test("a loosened active log mode is restored before the next append", {
   assert.match(await fs.readFile(getLogPath(), "utf8"), /before chmod\n.*after chmod\n$/)
 })
 
+// Why: lstat of the log path follows links in its parent path. A logs folder replaced by a link to
+// the folder the open file was moved to leads back to the same file, so the directories are
+// checked before the handle is reused (#141 review).
+test("an entry is not appended through a logs folder replaced by a link", async () => {
+  log.info("before move")
+  await readActiveLog()
+  const moved = path.join(paths.appDir, "logs-moved")
+  await fs.mkdir(moved)
+  await fs.rename(getLogPath(), path.join(moved, path.basename(getLogPath())))
+  await fs.rmdir(paths.logsDir)
+  await fs.symlink(moved, paths.logsDir, "junction")
+  try {
+    log.info("after move")
+    await flushLogs()
+
+    assert.match(await fs.readFile(path.join(moved, path.basename(getLogPath())), "utf8"), /^\S+ info before move\n$/)
+  } finally {
+    await fs.unlink(paths.logsDir)
+    await fs.rm(moved, { recursive: true, force: true })
+  }
+})
+
+test("a loosened logs folder mode is restored before the next append", {
+  skip: process.platform === "win32",
+}, async () => {
+  log.info("before folder chmod")
+  await readActiveLog()
+  await fs.chmod(paths.logsDir, 0o755)
+
+  log.info("after folder chmod")
+  await flushLogs()
+
+  assert.equal((await fs.stat(paths.logsDir)).mode & 0o777, 0o700)
+  assert.match(await fs.readFile(getLogPath(), "utf8"), /before folder chmod\n.*after folder chmod\n$/)
+})
+
 test.after(async () => {
   await flushLogs()
   await fs.rm(tempHome, { force: true, recursive: true })

wiki/EN-Internals.md

@@ -847,12 +847,17 @@ that end on entry boundaries: `FileHandle.appendFile` writes a larger buffer in
 between two of them.
 
 The handle is reused only while its path still names the same private file with
-one link and, on POSIX, mode 0600. A rename, deletion, replacement, second hard
-link or loosened mode makes the next batch reopen the path with the full checks:
-no symlink, one link, the same file before and after the open, then chmod 0600.
-Directories are checked when the file is opened and on each retention pass. A
-failed write never fails a request; it drops that batch and closes the handle.
-`flushLogs` waits for queued writes, then closes the file.
+one link and, on POSIX, mode 0600, and the app and logs directories are still the
+directories checked when it was opened: real directories, not links, with mode
+0700 on POSIX. lstat of the file follows links in its parent path, so without the
+directory check a logs directory replaced by a link to where the open file was
+moved would pass. A rename, deletion, replacement, second hard link, replaced
+directory or loosened mode makes the next batch reopen the path with the full
+checks: real private directories, then no symlink, one link, the same file before
+and after the open, and chmod 0600. A failed write never fails a request; it drops
+that batch and closes the handle. `flushLogs` waits for queued writes, then closes
+the file; each call waits for the close it queued, so overlapping calls both
+finish.
 
 Before #141 each entry ran its own directory checks, open, stat, chmod, append and
 close, and a burst could reach the file out of order.

wiki/ZH-Internals.md

@@ -711,11 +711,14 @@ UTC 戳会让格林尼治以西的人在本地下午的正中间发生文件切
 `FileHandle.appendFile` 会把更大的缓冲区分成 512 KiB 的几段写入，另一个向同一文件追加的
 进程可能插在两段之间。
 
-只有当路径仍指向同一个只有一个链接的私有文件，且在 POSIX 上权限仍为 0600 时，句柄才会
-被复用。重命名、删除、替换、第二个硬链接或放宽的权限，都会让下一批重新打开该路径并执行
-完整检查：不是符号链接、只有一个链接、打开前后是同一个文件，然后 chmod 0600。目录在打开
-文件时以及每次保留清理时检查。写入失败永远不会让请求失败；它丢弃这一批并关闭句柄。
-`flushLogs` 等待队列中的写入完成，再关闭文件。
+只有当路径仍指向同一个只有一个链接的私有文件、在 POSIX 上权限仍为 0600，且应用目录和
+日志目录仍是打开时检查过的目录（真实目录而不是链接，在 POSIX 上权限为 0700）时，句柄
+才会被复用。对文件的 lstat 会跟随父路径中的链接，所以没有目录检查时，被替换成指向已打开
+文件新位置的链接的日志目录也能通过检查。重命名、删除、替换、第二个硬链接、被替换的目录或
+放宽的权限，都会让下一批重新打开该路径并执行完整检查：先确认目录是真实的私有目录，再确认
+不是符号链接、只有一个链接、打开前后是同一个文件，然后 chmod 0600。写入失败永远不会让请求
+失败；它丢弃这一批并关闭句柄。`flushLogs` 等待队列中的写入完成，再关闭文件；每次调用只
+等待自己排入的关闭，所以重叠的调用都会结束。
 
 #141 之前，每条日志各自执行目录检查、open、stat、chmod、追加和 close，一批突发日志可能
 乱序写入文件。`tests/unit/log-format.test.ts` 覆盖调用顺序、调用时的时间戳和复用检查。