From fe3386d1a5c44ab410849601d967dd63b2da4965 Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Mon, 27 Jul 2026 11:23:17 +0400 Subject: [PATCH 1/3] fix(oauth): avoid unrelated macOS lock collisions --- src/oauth/local-lock.ts | 51 +++++++++- tests/oauth-local-lock.test.ts | 168 ++++++++++++++++++++++++++++++++- 2 files changed, 215 insertions(+), 4 deletions(-) diff --git a/src/oauth/local-lock.ts b/src/oauth/local-lock.ts index d1dcbf75..a217b1e3 100644 --- a/src/oauth/local-lock.ts +++ b/src/oauth/local-lock.ts @@ -4,7 +4,9 @@ import type { ListenOptions, Server, Socket } from "node:net"; const localLockPortStart = 49_152; const localLockPortCount = 16_384; -// macOS retains one canonical candidate to preserve coordination with older Miftah versions. +const macOSFallbackCandidateCount = 7; +// macOS retains one canonical candidate to preserve coordination with older Miftah versions, then +// uses a bounded deterministic fallback set only when that candidate is proven unrelated. // Windows and Linux recognize an exact holder on that legacy candidate and hold a best-effort // companion listener for rolling upgrades, but acquire collision-free kernel endpoints. Windows // uses a named pipe because this entire TCP range is also its default ephemeral range; Linux uses @@ -68,6 +70,19 @@ export function createOAuthLocalLockStrategy( return { key, probeEndpoints: [legacyEndpoint], acquisitionEndpoint: legacyEndpoint }; } +function macOSFallbackEndpoints(key: string, legacyEndpoint: OAuthLocalLockProbeEndpoint): readonly OAuthLocalLockProbeEndpoint[] { + const ports = new Set([legacyEndpoint.port]); + const endpoints: OAuthLocalLockProbeEndpoint[] = []; + for (let index = 1; index <= macOSFallbackCandidateCount; index += 1) { + const chunk = key.slice(index * 8, (index + 1) * 8); + const port = localLockPortStart + (Number.parseInt(chunk, 16) % localLockPortCount); + if (ports.has(port)) continue; + ports.add(port); + endpoints.push({ kind: "tcp", port }); + } + return endpoints; +} + function sameLocalLockEndpoint(left: OAuthLocalLockEndpoint, right: OAuthLocalLockEndpoint): boolean { if (left.kind === "tcp") return right.kind === "tcp" && left.port === right.port; return right.kind === "pipe" && left.path === right.path; @@ -150,6 +165,25 @@ async function tryAcquireLocalLock(endpoint: OAuthLocalLockEndpoint, key: string }); } +async function tryAcquireMacOSFallback( + endpoints: readonly OAuthLocalLockProbeEndpoint[], + key: string +): Promise { + let candidate: OAuthLocalLockProbeEndpoint | undefined; + for (const endpoint of endpoints) { + const state = await inspectLocalLockEndpoint(endpoint, key); + // A matching holder or incomplete probe may be a same-key process acquiring a fallback. Do not + // choose another endpoint until that state has resolved, or one key could split across locks. + if (state === "held" || state === "unknown") return undefined; + if (state === "occupied") continue; + candidate ??= endpoint; + } + if (candidate === undefined) return undefined; + // All contenders choose the first available endpoint. If it is won concurrently, retry from the + // complete stable set instead of moving to another candidate and splitting a same-key lock. + return tryAcquireLocalLock(candidate, key); +} + async function releaseLocalLock(lock: LocalLock): Promise { for (const client of lock.clients) client.destroy(); await new Promise((resolve) => { @@ -171,10 +205,14 @@ async function acquireLocalLock( const strategy = createOAuthLocalLockStrategy(scope, value, platform); const legacyEndpoint = strategy.probeEndpoints[0]; const acquiresLegacyEndpoint = sameLocalLockEndpoint(legacyEndpoint, strategy.acquisitionEndpoint); + const fallbackEndpoints = platform === "darwin" ? macOSFallbackEndpoints(strategy.key, legacyEndpoint) : undefined; while (true) { if (Date.now() - startedAt >= waitMilliseconds) throw new OAuthLocalLockUnavailableError(); const legacyState = await inspectLocalLockEndpoint(legacyEndpoint, strategy.key); - const mustWait = acquiresLegacyEndpoint ? legacyState !== "available" : legacyState === "held"; + const canUseMacOSFallback = legacyState === "occupied" && fallbackEndpoints !== undefined; + const mustWait = acquiresLegacyEndpoint + ? legacyState !== "available" && !canUseMacOSFallback + : legacyState === "held"; if (!mustWait) { let primaryLock: LocalLock | undefined; try { @@ -212,6 +250,15 @@ async function acquireLocalLock( }; } } + if (canUseMacOSFallback) { + let fallbackLock: LocalLock | undefined; + try { + fallbackLock = await tryAcquireMacOSFallback(fallbackEndpoints, strategy.key); + } catch { + throw new OAuthLocalLockUnavailableError(); + } + if (fallbackLock !== undefined) return async () => releaseLocalLock(fallbackLock); + } await new Promise((resolve) => setTimeout(resolve, 10)); } } diff --git a/tests/oauth-local-lock.test.ts b/tests/oauth-local-lock.test.ts index aacd63c1..a9bab2bf 100644 --- a/tests/oauth-local-lock.test.ts +++ b/tests/oauth-local-lock.test.ts @@ -35,6 +35,19 @@ function firstCandidatePort(scope: string, value: string): number { return portStart + (Number.parseInt(key.slice(0, 8), 16) % portCount); } +function fallbackCandidatePorts(scope: string, value: string): readonly number[] { + const key = createHash("sha256").update(`${protocol}\u0000${scope}\u0000${value}`, "utf8").digest("hex"); + const ports = new Set([firstCandidatePort(scope, value)]); + const candidates: number[] = []; + for (let index = 1; index <= 7; index += 1) { + const port = portStart + (Number.parseInt(key.slice(index * 8, (index + 1) * 8), 16) % portCount); + if (ports.has(port)) continue; + ports.add(port); + candidates.push(port); + } + return candidates; +} + function lockGreeting(scope: string, value: string): string { const key = createHash("sha256").update(`${protocol}\u0000${scope}\u0000${value}`, "utf8").digest("hex"); return `${protocol} ${key}\n`; @@ -64,6 +77,18 @@ async function tryHoldLegacyLock(port: number, greeting: string): Promise { + const server = createServer((socket) => socket.resume()); + return new Promise((resolve) => { + const onError = (): void => resolve(undefined); + server.once("error", onError); + server.listen({ host: "127.0.0.1", port, exclusive: true }, () => { + server.off("error", onError); + resolve(server); + }); + }); +} + async function close(server: Server): Promise { await new Promise((resolve, reject) => { server.close((error) => (error === undefined ? resolve() : reject(error))); @@ -220,7 +245,34 @@ describe("OAuth local lock", () => { } }); - it("fails closed while the canonical macOS TCP candidate is occupied", async () => { + it("keeps distinct macOS locks independent when their legacy TCP candidates collide", async () => { + const scope = "macos-collision-regression"; + const [firstValue, secondValue] = collidingValues(scope); + let releaseFirst!: () => void; + const holdFirst = new Promise((resolve) => { + releaseFirst = resolve; + }); + let markFirstEntered!: () => void; + const firstEntered = new Promise((resolve) => { + markFirstEntered = resolve; + }); + const first = withOAuthLocalLock(scope, firstValue, 2_000, async () => { + markFirstEntered(); + await holdFirst; + }, "darwin"); + await firstEntered; + + const secondOperation = vi.fn(async () => undefined); + try { + await withOAuthLocalLock(scope, secondValue, 200, secondOperation, "darwin"); + expect(secondOperation).toHaveBeenCalledOnce(); + } finally { + releaseFirst(); + await first; + } + }); + + it("uses a macOS fallback while an unrelated listener occupies the canonical TCP candidate", async () => { const scope = "occupied-candidate-regression"; let value = ""; let blocker: Server | undefined; @@ -230,15 +282,127 @@ describe("OAuth local lock", () => { } if (blocker === undefined) throw new Error("Could not reserve a deterministic OAuth lock candidate for the regression test"); + const operation = vi.fn(async () => undefined); + try { + await withOAuthLocalLock(scope, value, 100, operation, "darwin"); + expect(operation).toHaveBeenCalledOnce(); + } finally { + await close(blocker); + } + }); + + it("does not split one macOS key across fallback candidates", async () => { + const scope = "macos-fallback-serialization-regression"; + const [primaryValue, value] = collidingValues(scope); + const [firstFallback] = fallbackCandidatePorts(scope, value); + if (firstFallback === undefined) throw new Error("Expected a macOS fallback candidate"); + const blocker = await tryOccupy(firstFallback); + if (blocker === undefined) throw new Error("Could not reserve a macOS fallback candidate"); + let blockerClosed = false; + + let releasePrimary!: () => void; + const holdPrimary = new Promise((resolve) => { + releasePrimary = resolve; + }); + let markPrimaryEntered!: () => void; + const primaryEntered = new Promise((resolve) => { + markPrimaryEntered = resolve; + }); + const primary = withOAuthLocalLock(scope, primaryValue, 2_000, async () => { + markPrimaryEntered(); + await holdPrimary; + }, "darwin"); + + let releaseFirst!: () => void; + const holdFirst = new Promise((resolve) => { + releaseFirst = resolve; + }); + let markFirstEntered!: () => void; + const firstEntered = new Promise((resolve) => { + markFirstEntered = resolve; + }); + let first: Promise | undefined; + let duplicate: Promise | undefined; + try { + await primaryEntered; + first = withOAuthLocalLock(scope, value, 2_000, async () => { + markFirstEntered(); + await holdFirst; + }, "darwin"); + await firstEntered; + await close(blocker); + blockerClosed = true; + + let markDuplicateEntered!: () => void; + const duplicateEntered = new Promise((resolve) => { + markDuplicateEntered = resolve; + }); + duplicate = withOAuthLocalLock(scope, value, 2_000, async () => { + markDuplicateEntered(); + }, "darwin"); + const state = await Promise.race([ + duplicateEntered.then(() => "entered" as const), + new Promise<"blocked">((resolve) => setTimeout(() => resolve("blocked"), 200)) + ]); + expect(state).toBe("blocked"); + + releaseFirst(); + await first; + await duplicate; + } finally { + releaseFirst(); + releasePrimary(); + if (!blockerClosed) await close(blocker); + await Promise.allSettled([ + primary, + ...(first === undefined ? [] : [first]), + ...(duplicate === undefined ? [] : [duplicate]) + ]); + } + }); + + it("fails closed when an ambiguous macOS legacy listener does not identify its holder", async () => { + const scope = "macos-ambiguous-legacy-listener-regression"; + let value = ""; + let blocker: Server | undefined; + for (let index = 0; index < 256 && blocker === undefined; index += 1) { + value = `connection-${index}`; + blocker = await tryHoldAmbiguousListener(firstCandidatePort(scope, value)); + } + if (blocker === undefined) throw new Error("Could not reserve an ambiguous macOS OAuth lock candidate"); + + const operation = vi.fn(async () => undefined); try { - await expect(withOAuthLocalLock(scope, value, 100, async () => undefined, "darwin")).rejects.toBeInstanceOf( + await expect(withOAuthLocalLock(scope, value, 150, operation, "darwin")).rejects.toBeInstanceOf( OAuthLocalLockUnavailableError ); + expect(operation).not.toHaveBeenCalled(); } finally { await close(blocker); } }); + it("does not bypass an exact older macOS holder through a fallback candidate", async () => { + const scope = "macos-legacy-holder-regression"; + let value = ""; + let holder: Server | undefined; + for (let index = 0; index < 256 && holder === undefined; index += 1) { + value = `connection-${index}`; + holder = await tryHoldLegacyLock(firstCandidatePort(scope, value), lockGreeting(scope, value)); + } + if (holder === undefined) throw new Error("Could not reserve a legacy macOS OAuth lock candidate"); + + const operation = vi.fn(async () => undefined); + try { + await expect(withOAuthLocalLock(scope, value, 100, operation, "darwin")).rejects.toBeInstanceOf( + OAuthLocalLockUnavailableError + ); + expect(operation).not.toHaveBeenCalled(); + } finally { + await close(holder); + } + }); + it("keeps one key serialized", async () => { const scope = "same-key-regression"; const value = "connection"; From 599c85509736b8f8acab14848fe883c0fac5fe8f Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Mon, 27 Jul 2026 12:34:40 +0400 Subject: [PATCH 2/3] fix(oauth): probe macos fallback endpoints concurrently --- src/oauth/local-lock.ts | 11 +++-- tests/oauth-local-lock.test.ts | 73 +++++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/src/oauth/local-lock.ts b/src/oauth/local-lock.ts index a217b1e3..7f555612 100644 --- a/src/oauth/local-lock.ts +++ b/src/oauth/local-lock.ts @@ -4,7 +4,7 @@ import type { ListenOptions, Server, Socket } from "node:net"; const localLockPortStart = 49_152; const localLockPortCount = 16_384; -const macOSFallbackCandidateCount = 7; +export const macOSFallbackCandidateCount = 7; // macOS retains one canonical candidate to preserve coordination with older Miftah versions, then // uses a bounded deterministic fallback set only when that candidate is proven unrelated. // Windows and Linux recognize an exact holder on that legacy candidate and hold a best-effort @@ -169,14 +169,17 @@ async function tryAcquireMacOSFallback( endpoints: readonly OAuthLocalLockProbeEndpoint[], key: string ): Promise { + // Every endpoint must be inspected before choosing a fallback because a later matching or + // incomplete probe overrides an earlier available one. Run that complete inspection together + // so the bounded fallback scan takes one probe round trip rather than one per endpoint. + const states = await Promise.all(endpoints.map((endpoint) => inspectLocalLockEndpoint(endpoint, key))); let candidate: OAuthLocalLockProbeEndpoint | undefined; - for (const endpoint of endpoints) { - const state = await inspectLocalLockEndpoint(endpoint, key); + for (const [index, state] of states.entries()) { // A matching holder or incomplete probe may be a same-key process acquiring a fallback. Do not // choose another endpoint until that state has resolved, or one key could split across locks. if (state === "held" || state === "unknown") return undefined; if (state === "occupied") continue; - candidate ??= endpoint; + candidate ??= endpoints[index]; } if (candidate === undefined) return undefined; // All contenders choose the first available endpoint. If it is won concurrently, retry from the diff --git a/tests/oauth-local-lock.test.ts b/tests/oauth-local-lock.test.ts index a9bab2bf..1a284a3b 100644 --- a/tests/oauth-local-lock.test.ts +++ b/tests/oauth-local-lock.test.ts @@ -1,10 +1,11 @@ import { createHash } from "node:crypto"; import { createServer } from "node:net"; -import type { Server } from "node:net"; +import type { Server, Socket } from "node:net"; import { describe, expect, it, vi } from "vitest"; import { createOAuthLocalLockListenOptions, createOAuthLocalLockStrategy, + macOSFallbackCandidateCount, OAuthLocalLockUnavailableError, withOAuthLocalLock } from "../src/oauth/local-lock.js"; @@ -39,7 +40,7 @@ function fallbackCandidatePorts(scope: string, value: string): readonly number[] const key = createHash("sha256").update(`${protocol}\u0000${scope}\u0000${value}`, "utf8").digest("hex"); const ports = new Set([firstCandidatePort(scope, value)]); const candidates: number[] = []; - for (let index = 1; index <= 7; index += 1) { + for (let index = 1; index <= macOSFallbackCandidateCount; index += 1) { const port = portStart + (Number.parseInt(key.slice(index * 8, (index + 1) * 8), 16) % portCount); if (ports.has(port)) continue; ports.add(port); @@ -89,6 +90,42 @@ async function tryHoldAmbiguousListener(port: number): Promise { + const sockets = new Set(); + let released = false; + const release = (): void => { + if (released) return; + released = true; + for (const socket of sockets) socket.end("unrelated-listener\n"); + }; + const servers: Server[] = []; + for (const port of ports) { + const server = createServer((socket) => { + if (released) { + socket.end("unrelated-listener\n"); + return; + } + sockets.add(socket); + socket.once("close", () => sockets.delete(socket)); + if (sockets.size === ports.length) release(); + }); + const listening = await new Promise((resolve) => { + const onError = (): void => resolve(false); + server.once("error", onError); + server.listen({ host: "127.0.0.1", port, exclusive: true }, () => { + server.off("error", onError); + resolve(true); + }); + }); + if (!listening) { + await Promise.all(servers.map((existing) => close(existing))); + return undefined; + } + servers.push(server); + } + return servers; +} + async function close(server: Server): Promise { await new Promise((resolve, reject) => { server.close((error) => (error === undefined ? resolve() : reject(error))); @@ -291,6 +328,38 @@ describe("OAuth local lock", () => { } }); + it("probes macOS fallback candidates concurrently before selecting the first available endpoint", async () => { + const scope = "macos-fallback-concurrent-probe-regression"; + let value = ""; + let legacyBlocker: Server | undefined; + let fallbackBlockers: readonly Server[] = []; + for (let index = 0; index < 256 && legacyBlocker === undefined; index += 1) { + const candidateValue = `connection-${index}`; + const fallbackPorts = fallbackCandidatePorts(scope, candidateValue); + if (fallbackPorts.length < 2) continue; + const candidateFallbackBlockers = await tryHoldFallbackProbeBarriers(fallbackPorts.slice(0, -1)); + if (candidateFallbackBlockers === undefined) continue; + const candidateLegacyBlocker = await tryOccupy(firstCandidatePort(scope, candidateValue)); + if (candidateLegacyBlocker === undefined) { + await Promise.all(candidateFallbackBlockers.map((server) => close(server))); + continue; + } + value = candidateValue; + fallbackBlockers = candidateFallbackBlockers; + legacyBlocker = candidateLegacyBlocker; + } + if (legacyBlocker === undefined) throw new Error("Could not reserve deterministic macOS fallback probe barriers"); + + const operation = vi.fn(async () => undefined); + try { + await withOAuthLocalLock(scope, value, 400, operation, "darwin"); + expect(operation).toHaveBeenCalledOnce(); + } finally { + await close(legacyBlocker); + await Promise.all(fallbackBlockers.map((server) => close(server))); + } + }); + it("does not split one macOS key across fallback candidates", async () => { const scope = "macos-fallback-serialization-regression"; const [primaryValue, value] = collidingValues(scope); From e7341766274125d4c1d3a0afc1ba34d5c8e423a6 Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Mon, 27 Jul 2026 13:33:29 +0400 Subject: [PATCH 3/3] fix(oauth): skip occupied macos legacy bind --- src/oauth/local-lock.ts | 4 +++- tests/oauth-local-lock.test.ts | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/oauth/local-lock.ts b/src/oauth/local-lock.ts index 7f555612..c3ac5fc2 100644 --- a/src/oauth/local-lock.ts +++ b/src/oauth/local-lock.ts @@ -216,7 +216,9 @@ async function acquireLocalLock( const mustWait = acquiresLegacyEndpoint ? legacyState !== "available" && !canUseMacOSFallback : legacyState === "held"; - if (!mustWait) { + // A positively unrelated legacy listener cannot become this lock through a bind retry. On + // macOS, go directly to the deterministic fallback scan instead of making that doomed bind. + if (!mustWait && !canUseMacOSFallback) { let primaryLock: LocalLock | undefined; try { primaryLock = await tryAcquireLocalLock(strategy.acquisitionEndpoint, strategy.key); diff --git a/tests/oauth-local-lock.test.ts b/tests/oauth-local-lock.test.ts index 1a284a3b..2849eccc 100644 --- a/tests/oauth-local-lock.test.ts +++ b/tests/oauth-local-lock.test.ts @@ -11,6 +11,7 @@ import { } from "../src/oauth/local-lock.js"; const connectTargets = vi.hoisted(() => ({ ports: [] as number[], paths: [] as string[] })); +const listenTargets = vi.hoisted(() => ({ ports: [] as number[], paths: [] as string[] })); vi.mock("node:net", async (importOriginal) => { const actual = await importOriginal(); @@ -23,6 +24,19 @@ vi.mock("node:net", async (importOriginal) => { connectTargets.ports.push(Number((options as { port: unknown }).port)); } return Reflect.apply(actual.connect, undefined, args); + }, + createServer: (...args: Parameters) => { + const server = Reflect.apply(actual.createServer, undefined, args); + const listen = server.listen.bind(server); + server.listen = ((...listenArgs: unknown[]) => { + const options = listenArgs[0]; + if (typeof options === "string") listenTargets.paths.push(options); + if (typeof options === "object" && options !== null && "port" in options) { + listenTargets.ports.push(Number((options as { port: unknown }).port)); + } + return Reflect.apply(listen, undefined, listenArgs); + }) as typeof server.listen; + return server; } }; }); @@ -321,8 +335,11 @@ describe("OAuth local lock", () => { const operation = vi.fn(async () => undefined); try { + listenTargets.ports.length = 0; + listenTargets.paths.length = 0; await withOAuthLocalLock(scope, value, 100, operation, "darwin"); expect(operation).toHaveBeenCalledOnce(); + expect(listenTargets.ports).not.toContain(firstCandidatePort(scope, value)); } finally { await close(blocker); }