diff --git a/src/clients/omo-role-models.ts b/src/clients/omo-role-models.ts index 1a94853e14d..a02d315dd34 100644 --- a/src/clients/omo-role-models.ts +++ b/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 { diff --git a/structure/clients/integrations.md b/structure/clients/integrations.md index 541e566762a..869de7c9122 100644 --- a/structure/clients/integrations.md +++ b/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. diff --git a/tests/clients/omo-role-models.test.ts b/tests/clients/omo-role-models.test.ts index 2115ddaedc8..f3f5316f3bb 100644 --- a/tests/clients/omo-role-models.test.ts +++ b/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); diff --git a/tests/server/codex-agent-role-routes.test.ts b/tests/server/codex-agent-role-routes.test.ts index f0420bcb9e5..f82408e5fbf 100644 --- a/tests/server/codex-agent-role-routes.test.ts +++ b/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);