Merge pull request #545 from ndycode/claude/audit-27-isrecord-sweep

refactor(lib): dedupe drifted isRecord guards into the canonical utils helper

Author: ndycode

lib/codex-manager/commands/rotation.ts

@@ -49,6 +49,7 @@ import {
 import { isRateLimitedMarker } from "../rate-limit-markers.js";
 import type { PluginConfig } from "../../types.js";
 import type { AccountMetadataV3, AccountStorageV3 } from "../../storage.js";
+import { isRecord } from "../../utils.js";
 
 type LoadedStorage = AccountStorageV3 | null;
 
@@ -500,10 +501,6 @@ function formatEnvOverride(): string {
 	return parsed ? "enabled" : "disabled";
 }
 
-function isRecord(value: unknown): value is Record<string, unknown> {
-	return typeof value === "object" && value !== null;
-}
-
 function readOptionalNumber(record: Record<string, unknown>, key: string): number | null {
 	const value = record[key];
 	return typeof value === "number" && Number.isFinite(value) ? value : null;

lib/refresh-lease.ts

@@ -6,6 +6,7 @@ import { createLogger } from "./logger.js";
 import { getCodexMultiAuthDir } from "./runtime-paths.js";
 import { safeParseTokenResult } from "./schemas.js";
 import type { TokenResult } from "./types.js";
+import { isRecord } from "./utils.js";
 
 const log = createLogger("refresh-lease");
 
@@ -66,10 +67,6 @@ function hashRefreshToken(refreshToken: string): string {
 	return createHash("sha256").update(refreshToken).digest("hex");
 }
 
-function isRecord(value: unknown): value is Record<string, unknown> {
-	return value !== null && typeof value === "object";
-}
-
 function parseLeasePayload(raw: unknown): LeaseFilePayload | null {
 	if (!isRecord(raw)) return null;
 	const tokenHash = typeof raw.tokenHash === "string" ? raw.tokenHash : "";

test/codex-manager-rotation-command.test.ts

@@ -446,6 +446,25 @@ describe("codex-multi-auth rotation command", () => {
 		expect(infos.join("\n")).toContain("Codex app helper: not running");
 	});
 
+	it("treats an array helper status file as not running", async () => {
+		// Pins the canonical isRecord contract (lib/utils.ts): a status file
+		// whose top-level JSON value is an array must read as "no status", not
+		// as a record with all-null fields.
+		const root = await createTempRoot("codex-rotation-helper-array-");
+		process.env.CODEX_MULTI_AUTH_DIR = root;
+		await mkdir(root, { recursive: true });
+		await writeFile(
+			join(root, "runtime-rotation-app-helper.json"),
+			"[]\n",
+			"utf8",
+		);
+		const { deps, infos } = createDeps({ storage: null });
+
+		await expect(runRotationCommand(["status"], deps)).resolves.toBe(0);
+
+		expect(infos.join("\n")).toContain("Codex app helper: not running");
+	});
+
 	it("handles overlapping enable commands without dropping saves or app binds", async () => {
 		const {
 			deps,

test/refresh-lease.test.ts

@@ -521,6 +521,39 @@ describe("RefreshLeaseCoordinator", () => {
     expect(handle.role).toBe("bypass");
   });
 
+  it("rejects array JSON payloads in result and lock files", async () => {
+    // Pins the canonical isRecord contract (lib/utils.ts): a top-level JSON
+    // array must never be treated as a lease or result record, even though
+    // arrays satisfy `typeof value === "object"`.
+    const refreshToken = "token-array-payload";
+    const tokenHash = hashToken(refreshToken);
+    const lockPath = join(leaseDir, `${tokenHash}.lock`);
+    const resultPath = join(leaseDir, `${tokenHash}.result.json`);
+    await mkdir(leaseDir, { recursive: true });
+
+    // An array result file must not turn the caller into a follower.
+    await writeFile(resultPath, "[]\n", "utf8");
+    const coordinator = new RefreshLeaseCoordinator({
+      enabled: true,
+      leaseDir,
+      leaseTtlMs: 2_000,
+      waitTimeoutMs: 120,
+      pollIntervalMs: 20,
+      resultTtlMs: 2_000,
+    });
+    const owner = await coordinator.acquire(refreshToken);
+    expect(owner.role).toBe("owner");
+    await owner.release();
+
+    // An array lock file is an invalid payload: never owned, never stale, so
+    // the acquire times out and bypasses instead of stealing the lock.
+    await writeFile(lockPath, "[]\n", "utf8");
+    const blocked = await coordinator.acquire(refreshToken);
+    expect(blocked.role).toBe("bypass");
+    await blocked.release();
+    await expect(readFile(lockPath, "utf8")).resolves.toBe("[]\n");
+  });
+
   it("prunes stale artifacts while keeping non-file entries", async () => {
     await mkdir(leaseDir, { recursive: true });
     const staleLock = join(leaseDir, "stale.lock");