Skip to content
Open
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
5 changes: 5 additions & 0 deletions .changeset/bounded-auth-network.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"openwiki": patch
---

fix: bound OAuth and ngrok discovery requests with timeouts
15 changes: 10 additions & 5 deletions src/agent/openai-chatgpt-oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
OPENAI_CHATGPT_PLAN_ENV_KEY,
OPENAI_CHATGPT_REFRESH_TOKEN_ENV_KEY,
} from "../constants.js";
import { fetchWithAuthTimeout } from "../auth/http.js";

/**
* ChatGPT/Codex OAuth client.
Expand Down Expand Up @@ -315,11 +316,15 @@ function isCodexResponsesRequest(input: Parameters<typeof fetch>[0]): boolean {
}

async function exchangeToken(body: URLSearchParams): Promise<CodexTokens> {
const res = await fetch(TOKEN_URL, {
method: "POST",
headers: { "Content-Type": "application/x-www-form-urlencoded" },
body,
});
const res = await fetchWithAuthTimeout(
TOKEN_URL,
{
method: "POST",
headers: { "Content-Type": "application/x-www-form-urlencoded" },
body,
},
{ operation: "ChatGPT token exchange" },
);

if (!res.ok) {
throw new Error(
Expand Down
71 changes: 71 additions & 0 deletions src/auth/http.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
const DEFAULT_AUTH_FETCH_TIMEOUT_MS = 15_000;

export type AuthFetchOptions = {
timeoutMs?: number;
operation?: string;
};

/** Run one authentication request with a hard deadline; exchanges are not retried. */
export async function fetchWithAuthTimeout(
input: Parameters<typeof fetch>[0],
init: Parameters<typeof fetch>[1] = {},
options: AuthFetchOptions = {},
): Promise<Response> {
const timeoutMs = options.timeoutMs ?? DEFAULT_AUTH_FETCH_TIMEOUT_MS;
const operation = options.operation ?? "Authentication request";
const timeoutController = new AbortController();
const callerSignal = init.signal;
const signal = callerSignal
? AbortSignal.any([callerSignal, timeoutController.signal])
: timeoutController.signal;

let rejectTimeout!: (error: Error) => void;
const timeoutError = new Error(
`${operation} timed out after ${timeoutMs}ms. Check your network connection and try again.`,
);
timeoutError.name = "AuthFetchTimeoutError";
const timeoutPromise = new Promise<never>((_, reject) => {
rejectTimeout = reject;
});
let removeCallerAbortListener: (() => void) | undefined;
const callerAbortPromise = callerSignal
? new Promise<never>((_, reject) => {
const rejectCaller = () => {
const reason: unknown = callerSignal.reason as unknown;
reject(
reason instanceof Error
? reason
: new Error("Request cancelled by caller"),
);
};

if (callerSignal.aborted) {
rejectCaller();
return;
}

callerSignal.addEventListener("abort", rejectCaller, { once: true });
removeCallerAbortListener = () =>
callerSignal.removeEventListener("abort", rejectCaller);
})
: null;

const timeoutId = setTimeout(() => {
timeoutController.abort(timeoutError);
rejectTimeout(timeoutError);
}, timeoutMs);

try {
const request = fetch(input, { ...init, signal });
return await Promise.race(
callerAbortPromise
? [request, timeoutPromise, callerAbortPromise]
: [request, timeoutPromise],
);
} finally {
clearTimeout(timeoutId);
removeCallerAbortListener?.();
}
}

export const AUTH_FETCH_TIMEOUT_MS = DEFAULT_AUTH_FETCH_TIMEOUT_MS;
30 changes: 25 additions & 5 deletions src/auth/ngrok.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import { spawn } from "node:child_process";
import type { ChildProcess } from "node:child_process";
import { saveOpenWikiEnv } from "../env.js";
import { fetchWithAuthTimeout } from "./http.js";

const DEFAULT_CALLBACK_PORT = 53682;
const OAUTH_CALLBACK_PORT_ENV_KEY = "OPENWIKI_OAUTH_CALLBACK_PORT";
const HTTPS_OAUTH_REDIRECT_URI_ENV_KEY = "OPENWIKI_HTTPS_OAUTH_REDIRECT_URI";
const NGROK_API_URL = "http://127.0.0.1:4040/api/tunnels";
const NGROK_DISCOVERY_TIMEOUT_MS = 15_000;
const NGROK_DISCOVERY_POLL_MS = 500;
const NGROK_FETCH_TIMEOUT_MS = 2_000;

export type NgrokStartOptions = {
port?: number;
Expand Down Expand Up @@ -255,22 +257,40 @@ async function waitForRandomNgrokRedirectUri(
port: number,
): Promise<string | null> {
const startedAt = Date.now();
const deadline = startedAt + NGROK_DISCOVERY_TIMEOUT_MS;

while (Date.now() - startedAt < NGROK_DISCOVERY_TIMEOUT_MS) {
const redirectUri = await fetchNgrokRedirectUri(port);
while (Date.now() < deadline) {
const remainingMs = deadline - Date.now();
const redirectUri = await fetchNgrokRedirectUri(
port,
Math.min(NGROK_FETCH_TIMEOUT_MS, remainingMs),
);
if (redirectUri) {
return redirectUri;
}

await sleep(NGROK_DISCOVERY_POLL_MS);
const delayMs = Math.min(NGROK_DISCOVERY_POLL_MS, deadline - Date.now());
if (delayMs > 0) {
await sleep(delayMs);
}
}

return null;
}

async function fetchNgrokRedirectUri(port: number): Promise<string | null> {
async function fetchNgrokRedirectUri(
port: number,
timeoutMs: number,
): Promise<string | null> {
try {
const response = await fetch(NGROK_API_URL);
const response = await fetchWithAuthTimeout(
NGROK_API_URL,
{},
{
operation: "ngrok tunnel discovery",
timeoutMs,
},
);
if (!response.ok) {
return null;
}
Expand Down
7 changes: 5 additions & 2 deletions src/auth/oauth-discovery.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { isIP } from "node:net";
import { fetchWithAuthTimeout } from "./http.js";

export type OAuthMetadata = {
authorization_endpoint?: string;
Expand Down Expand Up @@ -30,13 +31,14 @@ export async function discoverProtectedResourceMetadata(
];

for (const candidate of candidates) {
const response = await fetch(
const response = await fetchWithAuthTimeout(
validateOAuthEndpointUrl(
candidate,
"MCP protected resource metadata",
options,
),
{ redirect: "manual" },
{ operation: "OAuth protected-resource discovery" },
);
if (response.ok) {
return (await response.json()) as ProtectedResourceMetadata;
Expand All @@ -63,13 +65,14 @@ export async function discoverAuthorizationServerMetadata(
];

for (const candidate of candidates) {
const response = await fetch(
const response = await fetchWithAuthTimeout(
validateOAuthEndpointUrl(
candidate,
"OAuth authorization server metadata",
options,
),
{ redirect: "manual" },
{ operation: "OAuth authorization-server discovery" },
);
if (response.ok) {
return (await response.json()) as OAuthMetadata;
Expand Down
34 changes: 20 additions & 14 deletions src/auth/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
validateOAuthEndpointUrl,
} from "./oauth-discovery.js";
import { getAuthProvider } from "./providers.js";
import { fetchWithAuthTimeout } from "./http.js";
import type {
AuthProviderId,
OAuthClientRegistration,
Expand Down Expand Up @@ -203,20 +204,24 @@ async function registerMcpOAuthClient(
validationOptions,
).toString();

const registrationResponse = await fetch(registrationEndpoint, {
body: JSON.stringify({
client_name: "OpenWiki",
grant_types: ["authorization_code", "refresh_token"],
redirect_uris: [redirectUri],
response_types: ["code"],
token_endpoint_auth_method: "none",
}),
headers: {
"Content-Type": "application/json",
const registrationResponse = await fetchWithAuthTimeout(
registrationEndpoint,
{
body: JSON.stringify({
client_name: "OpenWiki",
grant_types: ["authorization_code", "refresh_token"],
redirect_uris: [redirectUri],
response_types: ["code"],
token_endpoint_auth_method: "none",
}),
headers: {
"Content-Type": "application/json",
},
method: "POST",
redirect: "manual",
},
method: "POST",
redirect: "manual",
});
{ operation: `${provider.displayName} client registration` },
);

if (!registrationResponse.ok) {
throw new Error(
Expand Down Expand Up @@ -306,7 +311,7 @@ async function exchangeAuthorizationCode({
body.set("resource", provider.mcpResourceUrl);
}

const response = await fetch(
const response = await fetchWithAuthTimeout(
validateOAuthEndpointUrl(
registration.tokenUrl,
`${provider.displayName} token endpoint`,
Expand All @@ -320,6 +325,7 @@ async function exchangeAuthorizationCode({
method: "POST",
redirect: "manual",
},
{ operation: `${provider.displayName} token exchange` },
);

if (!response.ok) {
Expand Down
4 changes: 3 additions & 1 deletion src/auth/tokens.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { loadOpenWikiEnv, saveOpenWikiEnv } from "../env.js";
import { fetchWithAuthTimeout } from "./http.js";
import {
discoverAuthorizationServerMetadata,
discoverProtectedResourceMetadata,
Expand Down Expand Up @@ -92,7 +93,7 @@ export async function refreshOAuthAccessToken(
body.set("resource", provider.mcpResourceUrl);
}

const response = await fetch(
const response = await fetchWithAuthTimeout(
validateOAuthEndpointUrl(
tokenUrl,
`${provider.displayName} token endpoint`,
Expand All @@ -107,6 +108,7 @@ export async function refreshOAuthAccessToken(
method: "POST",
redirect: "manual",
},
{ operation: `${provider.displayName} token refresh` },
);

if (!response.ok) {
Expand Down
63 changes: 63 additions & 0 deletions test/auth-http.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { afterEach, describe, expect, test, vi } from "vitest";
import { fetchWithAuthTimeout } from "../src/auth/http.ts";

describe("fetchWithAuthTimeout", () => {
afterEach(() => {
vi.useRealTimers();
vi.unstubAllGlobals();
});

test("rejects a fetch that never settles and aborts its signal", async () => {
vi.useFakeTimers();
let requestSignal: AbortSignal | undefined;
vi.stubGlobal(
"fetch",
vi.fn((_input: unknown, init?: { signal?: AbortSignal }) => {
requestSignal = init?.signal;
return new Promise<Response>(() => {});
}),
);

const pending = fetchWithAuthTimeout(
"https://auth.example.test/token",
{},
{ operation: "Token exchange", timeoutMs: 1000 },
);
const rejection = expect(pending).rejects.toThrow(
"Token exchange timed out after 1000ms",
);
await vi.advanceTimersByTimeAsync(1000);

await rejection;
expect(requestSignal?.aborted).toBe(true);
});

test("propagates caller cancellation instead of converting it to a timeout", async () => {
const controller = new AbortController();
vi.stubGlobal(
"fetch",
vi.fn(
(_input: unknown, init?: { signal?: AbortSignal }) =>
new Promise<Response>((_, reject) => {
init?.signal?.addEventListener("abort", () =>
reject(
init.signal?.reason instanceof Error
? init.signal.reason
: new Error("cancelled"),
),
);
}),
),
);

const pending = fetchWithAuthTimeout(
"https://auth.example.test/token",
{ signal: controller.signal },
{ timeoutMs: 60_000 },
);
const reason = new Error("cancelled by caller");
controller.abort(reason);

await expect(pending).rejects.toBe(reason);
});
});
20 changes: 20 additions & 0 deletions test/oauth-url-validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ describe("validateOAuthEndpointUrl", () => {

describe("OAuth discovery fetches", () => {
afterEach(() => {
vi.useRealTimers();
vi.unstubAllGlobals();
});

Expand All @@ -63,4 +64,23 @@ describe("OAuth discovery fetches", () => {
expect(call[1]).toMatchObject({ redirect: "manual" });
}
});

test("times out a stalled metadata response", async () => {
vi.useFakeTimers();
vi.stubGlobal(
"fetch",
vi.fn(() => new Promise<Response>(() => {})),
);

const pending = discoverAuthorizationServerMetadata(
"https://auth.notion.com/oauth",
{ allowedHosts: ["notion.com"] },
);
const rejection = expect(pending).rejects.toThrow(
"OAuth authorization-server discovery timed out",
);
await vi.advanceTimersByTimeAsync(15_000);

await rejection;
});
});