Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions src/mcp/auth-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,69 @@ describe("mcp auth-store", () => {
).toBe(true);
});

test("overlapping updateAuthState from two processes keeps tokens and PKCE", async () => {
const home = await tempHome();
await saveAuthState(
linear,
{
clientInformation: {
client_id: "c1",
redirect_uris: ["http://127.0.0.1:1/callback"],
client_id_issued_at: 1,
},
},
home,
);

const storePath = join(import.meta.dirname, "auth-store.ts");
const barrier = join(home, "start");
const script = `
import { updateAuthState } from ${JSON.stringify(storePath)};
const home = process.argv[1];
const field = process.argv[2];
const barrier = process.argv[3];
const identity = { serverName: "linear", serverURL: "https://mcp.linear.app/mcp" };
while (!(await Bun.file(barrier).exists())) await Bun.sleep(5);
await updateAuthState(
identity,
(state) => {
Bun.sleepSync(150);
if (field === "tokens") {
state.tokens = {
access_token: "tok",
token_type: "bearer",
expires_in: 3600,
refresh_token: "ref",
};
} else {
state.codeVerifier = "verifier-from-other-session";
}
},
home,
);
`;
const processes = [
Bun.spawn([process.execPath, "-e", script, "--", home, "tokens", barrier], {
stdout: "ignore",
stderr: "pipe",
}),
Bun.spawn([process.execPath, "-e", script, "--", home, "verifier", barrier], {
stdout: "ignore",
stderr: "pipe",
}),
];
await Bun.sleep(50);
await writeFile(barrier, "go");
const exitCodes = await Promise.all(processes.map((child) => child.exited));
const errors = await Promise.all(processes.map((child) => new Response(child.stderr).text()));
expect(exitCodes, errors.join("\n")).toEqual([0, 0]);

const final = await loadAuthState(linear, home);
expect(final.tokens?.access_token).toBe("tok");
expect(final.codeVerifier).toBe("verifier-from-other-session");
expect(final.clientInformation?.client_id).toBe("c1");
});

test("concurrent saveAuthState calls do not throw ENOENT on temp rename", async () => {
const home = await tempHome();
await Promise.all(
Expand Down
128 changes: 78 additions & 50 deletions src/mcp/auth-store.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { createHash } from "node:crypto";
import { mkdir, readFile, rename, unlink, writeFile } from "node:fs/promises";
import { mkdir, open, readFile, rename, stat, unlink, writeFile } from "node:fs/promises";
import { readFileSync } from "node:fs";
import { homedir } from "node:os";
import { dirname, join } from "node:path";
import { setTimeout as delay } from "node:timers/promises";
import type {
OAuthClientInformationFull,
OAuthTokens,
Expand Down Expand Up @@ -126,6 +127,10 @@ export function tryLoadAuthStateSync(
return parseAuthState(raw);
}

function isEexist(err: unknown): boolean {
return typeof err === "object" && err !== null && "code" in err && err.code === "EEXIST";
}

// pid alone is not unique per call — concurrent saves in one process must not
// share a temp path or the second rename hits ENOENT after the first moves it.
let tmpWriteCounter = 0;
Expand All @@ -135,6 +140,70 @@ let tmpWriteCounter = 0;
// session's saveCodeVerifier wiping another's just-written tokens).
const updateChains = new Map<string, Promise<unknown>>();

const LOCK_STALE_MS = 5_000;
const LOCK_RETRY_MS = 25;

async function acquireAuthFileLock(lockPath: string) {
while (true) {
try {
return await open(lockPath, "wx", 0o600);
} catch (err) {
if (!isEexist(err)) throw err;
try {
const info = await stat(lockPath);
if (Date.now() - info.mtimeMs > LOCK_STALE_MS) {
try {
await unlink(lockPath);
} catch (unlinkErr) {
if (!isEnoent(unlinkErr)) throw unlinkErr;
}
continue;
}
} catch (statErr) {
if (isEnoent(statErr)) continue;
throw statErr;
}
await delay(LOCK_RETRY_MS);
}
}
}

async function withAuthFileLock<T>(path: string, op: () => Promise<T>): Promise<T> {
const lockPath = `${path}.lock`;
await mkdir(dirname(path), { recursive: true, mode: 0o700 });
const lock = await acquireAuthFileLock(lockPath);
try {
return await op();
} finally {
try {
await lock.close();
} catch {
// Close can fail if the handle was already torn down.
}
try {
await unlink(lockPath);
} catch {
// Missing lock is fine; a leftover file is recovered as stale.
}
}
}

function enqueueAuthFileOp<T>(path: string, op: () => Promise<T>): Promise<T> {
const previous = updateChains.get(path) ?? Promise.resolve();
const run = previous.then(
() => withAuthFileLock(path, op),
() => withAuthFileLock(path, op),
);
updateChains.set(
path,
run.then(
() => undefined,
() => undefined,
),
);
return run;
}

async function writeAuthFile(path: string, state: MCPAuthState): Promise<void> {
await mkdir(dirname(path), { recursive: true, mode: 0o700 });
const tmp = `${path}.${process.pid}.${(tmpWriteCounter += 1)}.tmp`;
Expand All @@ -151,19 +220,7 @@ export async function saveAuthState(
home: string = homedir(),
): Promise<void> {
const path = authFilePath(identity, home);
const previous = updateChains.get(path) ?? Promise.resolve();
const write = previous.then(
() => writeAuthFile(path, state),
() => writeAuthFile(path, state),
);
updateChains.set(
path,
write.then(
() => undefined,
() => undefined,
),
);
await write;
await enqueueAuthFileOp(path, () => writeAuthFile(path, state));
}

// Load → mutate → save under the per-file chain. Mutator receives a mutable
Expand All @@ -174,49 +231,20 @@ export async function updateAuthState(
home: string = homedir(),
): Promise<MCPAuthState> {
const path = authFilePath(identity, home);
const previous = updateChains.get(path) ?? Promise.resolve();
const run = previous.then(
async () => {
const state = await loadAuthState(identity, home);
mutator(state);
await writeAuthFile(path, state);
return state;
},
async () => {
const state = await loadAuthState(identity, home);
mutator(state);
await writeAuthFile(path, state);
return state;
},
);
updateChains.set(
path,
run.then(
() => undefined,
() => undefined,
),
);
return run;
return enqueueAuthFileOp(path, async () => {
const state = await loadAuthState(identity, home);
mutator(state);
await writeAuthFile(path, state);
return state;
});
}

export async function deleteAuthState(
identity: MCPAuthIdentity,
home: string = homedir(),
): Promise<void> {
const path = authFilePath(identity, home);
const previous = updateChains.get(path) ?? Promise.resolve();
const run = previous.then(
() => unlinkAuthFile(path),
() => unlinkAuthFile(path),
);
updateChains.set(
path,
run.then(
() => undefined,
() => undefined,
),
);
await run;
await enqueueAuthFileOp(path, () => unlinkAuthFile(path));
}

async function unlinkAuthFile(path: string): Promise<void> {
Expand Down
31 changes: 31 additions & 0 deletions src/mcp/oauth-provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,37 @@ describe("createOAuthProvider", () => {
expect(disk.tokens).toBeUndefined();
});

test("resetAuthorization does not delete a sibling's just-saved tokens", async () => {
const home = await tempHome();
const a = await createOAuthProvider({
serverName: "linear",
serverURL: linear.serverURL,
redirectUrl: "http://127.0.0.1:62000/callback",
onAuthURL: () => undefined,
home,
});
const b = await createOAuthProvider({
serverName: "linear",
serverURL: linear.serverURL,
redirectUrl: "http://127.0.0.1:60435/callback",
onAuthURL: () => undefined,
home,
});

await a.saveTokens({
access_token: "fresh",
token_type: "bearer",
expires_in: 3600,
refresh_token: "fresh-refresh",
});
expect((await syncValue(a.tokens()))?.access_token).toBe("fresh");

await b.resetAuthorization();

expect((await syncValue(a.tokens()))?.access_token).toBe("fresh");
expect((await loadAuthState(linear, home)).tokens?.access_token).toBe("fresh");
});

test("isolates same-name providers by endpoint and persists the same identity", async () => {
const home = await tempHome();
const customURL = "https://custom.example/mcp";
Expand Down
9 changes: 8 additions & 1 deletion src/mcp/oauth-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ export async function createOAuthProvider(
return stored.tokens;
},
saveTokens(tokens: OAuthTokens): Promise<void> {
stored.tokens = tokens;
return apply((state) => {
state.tokens = tokens;
});
Expand All @@ -220,8 +221,14 @@ export async function createOAuthProvider(
},
async resetAuthorization(): Promise<void> {
oauthState = undefined;
// Snapshot before the disk refresh so a session that never held tokens
// cannot adopt a sibling's credentials and then delete them.
const previous = stored.tokens?.access_token;
refreshDurableFromDisk();
await apply((state) => {
delete state.tokens;
if (state.tokens?.access_token === previous) {
delete state.tokens;
}
delete state.codeVerifier;
// Next browser flow needs a client registered for *this* loopback port.
if (!redirectUrisInclude(state.clientInformation, opts.redirectUrl)) {
Expand Down
Loading