fix(clients): prevent symlink races in LazyCodex role mirror reads

src/clients/omo-role-models.ts

@@ -6,7 +6,7 @@
  * dashboard or CLI pick, never creates the file, and refuses a file with comments: the write
  * re-serializes JSON, and a comment the user wrote would be lost without a word.
  */
-import { lstatSync, readFileSync } from "node:fs";
+import { closeSync, constants, fstatSync, lstatSync, openSync, readFileSync } from "node:fs";
 import { homedir } from "node:os";
 import { join } from "node:path";
 import { atomicWriteFileNoFollowUnclaimed } from "../config/atomic-write";
@@ -40,12 +40,26 @@ type Loaded =
 
 function load(path: string): Loaded {
   let text: string;
+  let fd: number | undefined;
   try {
-    if (!lstatSync(path).isFile()) return { kind: "invalid" };
-    text = readFileSync(path, "utf8");
+    // POSIX FIFOs must not block before fstat can reject them. Windows omits
+    // these flags and retains the descriptor/path identity checks below.
+    const flags = process.platform === "win32"
+      ? constants.O_RDONLY
+      : constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK;
+    fd = openSync(path, flags);
+    const opened = fstatSync(fd);
+    const current = lstatSync(path);
+    if (!opened.isFile() || !current.isFile() || opened.dev !== current.dev || opened.ino !== current.ino) {
+      return { kind: "invalid" };
+    }
+    text = readFileSync(fd, "utf8");
   } catch (error) {
     if ((error as NodeJS.ErrnoException).code === "ENOENT") return { kind: "absent" };
+    if ((error as NodeJS.ErrnoException).code === "ELOOP") return { kind: "invalid" };
     throw error;
+  } finally {
+    if (fd !== undefined) closeSync(fd);
   }
   if (hasJsoncComments(text)) return { kind: "comments" };
   try {

structure/clients/integrations.md

@@ -453,6 +453,11 @@ not an object reports `invalid`, and a file containing any `//` or block comment
 `skipped_comments`, because re-serializing JSONC would drop those comments. The management
 response carries that status and the dashboard shows it; the role TOML write described in
 [subagents](../subagents.md#per-role-model-pins) is not rolled back by a skipped mirror.
+The loader binds validation and reading to one file descriptor and rejects a path whose
+directory entry no longer identifies that opened regular file. On POSIX, `O_NOFOLLOW` rejects
+symlinks at open and `O_NONBLOCK` lets the regular-file check reject a FIFO without waiting
+for a writer. Windows omits those POSIX flags and retains the descriptor/path identity checks;
+those checks do not claim POSIX no-follow open semantics.
 An explicit `null` in any of those three places counts as not an object. A file that exists but
 cannot be read lists as `unreadable`, so the role table still loads, and a save reports
 `write_failed` for the mirror.

tests/clients/omo-role-models.test.ts

@@ -1,6 +1,7 @@
 import { afterEach, describe, expect, spyOn, test } from "bun:test";
+import { spawnSync } from "node:child_process";
 import * as fs from "node:fs";
-import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { omoJsoncPath, readOmoRoleModels, writeOmoRoleModel } from "../../src/clients/omo-role-models";
@@ -55,6 +56,50 @@ describe("omo role models", () => {
     expect(() => readFileSync(path)).toThrow();
   });
 
+  test("refuses a symlink substituted before the integration file is opened", () => {
+    const path = file("{}");
+    const secret = join(dir!, "secret.json");
+    writeFileSync(secret, '{ "tokens": { "access_token": "secret" } }');
+    const realOpen = fs.openSync;
+    const spy = spyOn(fs, "openSync").mockImplementation(((target, flags, mode) => {
+      if (target === path) {
+        rmSync(path);
+        symlinkSync(secret, path);
+      }
+      return realOpen(target, flags, mode);
+    }) as typeof fs.openSync);
+    try {
+      expect(writeOmoRoleModel("explorer", "m", path)).toBe("invalid");
+      expect(readFileSync(secret, "utf8")).toBe('{ "tokens": { "access_token": "secret" } }');
+    } finally {
+      spy.mockRestore();
+    }
+  });
+
+  test.skipIf(process.platform === "win32").each(["read", "write"])(
+    "%s refuses a FIFO without waiting for a writer",
+    operation => {
+      const path = file();
+      expect(spawnSync("mkfifo", [path], { timeout: 3000 }).status).toBe(0);
+      const moduleUrl = new URL("../../src/clients/omo-role-models.ts", import.meta.url).href;
+      const script = `
+        const { readOmoRoleModels, writeOmoRoleModel } = await import(${JSON.stringify(moduleUrl)});
+        const result = process.argv[2] === "read"
+          ? readOmoRoleModels(process.argv[1])
+          : writeOmoRoleModel("explorer", "m", process.argv[1]);
+        console.log(JSON.stringify(result));
+      `;
+      // A subprocess deadline bounds the test even if synchronous open regresses.
+      const child = spawnSync(process.execPath, ["--eval", script, path, operation], {
+        timeout: 3000, killSignal: "SIGKILL", encoding: "utf8",
+      });
+      expect(child.error).toBeUndefined();
+      expect(child.status).toBe(0);
+      expect(JSON.parse(child.stdout)).toEqual(operation === "read" ? { state: "invalid" } : "invalid");
+      expect(fs.lstatSync(path).isFIFO()).toBe(true);
+    },
+  );
+
   test("a codex value of the wrong shape is invalid rather than overwritten", () => {
     const text = '{ "codex": { "agents": ["explorer"] } }';
     const path = file(text);

tests/server/codex-agent-role-routes.test.ts

@@ -120,11 +120,11 @@ describe("/api/codex-agent-roles", () => {
   test("an unreadable omo.jsonc still lists the roles", async () => {
     const omoPath = join(root, "home", ".omo", "omo.jsonc");
     writeFileSync(omoPath, '{ "codex": {} }\n');
-    const nativeRead = fs.readFileSync;
-    const spy = spyOn(fs, "readFileSync").mockImplementation(((path: fs.PathOrFileDescriptor, options?: unknown) => {
+    const nativeOpen = fs.openSync;
+    const spy = spyOn(fs, "openSync").mockImplementation(((path, flags, mode) => {
       if (path === omoPath) throw Object.assign(new Error("EACCES: permission denied"), { code: "EACCES" });
-      return nativeRead(path, options as BufferEncoding);
-    }) as never);
+      return nativeOpen(path, flags, mode);
+    }) as typeof fs.openSync);
     try {
       const listed = await call("/api/codex-agent-roles");
       expect(listed.status).toBe(200);