Skip to content

Commit 564eac4

Browse files
LiuwqGitcursoragent
LiuwqGit
and
cursoragent
committed
fix(mcp): preserve OAuth redirect and error body text
Persist localhost redirect after DCR fallback so --code exchange matches, and read body-less foreign response text before SDK error parsing. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 71ef1e3 commit 564eac4

3 files changed

Lines changed: 104 additions & 22 deletions

File tree

‎src/agents/mcp-http-fetch.ts‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,22 +56,30 @@ function resolveFetchRequest(input: RequestInfo | URL, init?: RequestInit) {
5656
};
5757
}
5858

59-
function ensureGlobalFetchResponse(response: Response): Response {
60-
return new Response(response.body, {
59+
async function ensureGlobalFetchResponse(response: Response): Promise<Response> {
60+
const init = {
6161
status: response.status,
6262
statusText: response.statusText,
6363
headers: response.headers,
64-
});
64+
};
65+
if (response.body != null) {
66+
return new Response(response.body, init);
67+
}
68+
if (typeof response.text === "function") {
69+
const text = await response.text();
70+
return new Response(text, init);
71+
}
72+
return new Response(null, init);
6573
}
6674

67-
function buildManagedMcpResponse(
75+
async function buildManagedMcpResponse(
6876
response: Response,
6977
release: () => Promise<void>,
7078
refreshTimeout?: () => void,
71-
): Response {
79+
): Promise<Response> {
7280
if (!response.body) {
7381
void release();
74-
return ensureGlobalFetchResponse(response);
82+
return await ensureGlobalFetchResponse(response);
7583
}
7684

7785
const source = response.body;
@@ -115,7 +123,7 @@ function buildManagedMcpResponse(
115123
},
116124
});
117125
managedMcpResponseCleanupRegistry.register(wrappedBody, { finalize }, cleanupRegistrationToken);
118-
return ensureGlobalFetchResponse(
126+
return await ensureGlobalFetchResponse(
119127
new Response(wrappedBody, {
120128
status: response.status,
121129
statusText: response.statusText,
@@ -165,7 +173,7 @@ export function buildMcpHttpFetch(params: {
165173
...(needsCustomDispatcher ? { resolveDispatcherPolicy: resolveCustomDispatcherPolicy } : {}),
166174
};
167175
const guarded = await fetchWithSsrFGuard(guardedFetchOptions);
168-
return buildManagedMcpResponse(guarded.response, guarded.release, guarded.refreshTimeout);
176+
return await buildManagedMcpResponse(guarded.response, guarded.release, guarded.refreshTimeout);
169177
};
170178
}
171179

‎src/agents/mcp-oauth.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,51 @@ describe("MCP OAuth provider", () => {
116116
]);
117117
});
118118

119+
it("persists localhost redirect for a later code exchange login", async () => {
120+
await withTempHome(
121+
async (home) => {
122+
authMock.mockReset();
123+
authMock
124+
.mockRejectedValueOnce(new Error("invalid_client_metadata: redirect_uri rejected"))
125+
.mockResolvedValueOnce("REDIRECT");
126+
127+
await expect(
128+
runMcpOAuthLogin({
129+
serverName: "Calendly",
130+
serverUrl: "https://mcp.calendly.com/",
131+
onAuthorizationUrl: () => {},
132+
}),
133+
).resolves.toBe("redirect");
134+
135+
const tokenDir = `${home}/.openclaw/mcp-oauth`;
136+
const entries = await fs.readdir(tokenDir);
137+
const store = JSON.parse(await fs.readFile(`${tokenDir}/${entries[0]}`, "utf-8")) as {
138+
redirectUrl?: string;
139+
};
140+
expect(store.redirectUrl).toBe("http://localhost:8989/oauth/callback");
141+
142+
authMock.mockReset();
143+
authMock.mockResolvedValueOnce("AUTHORIZED");
144+
await runMcpOAuthLogin({
145+
serverName: "Calendly",
146+
serverUrl: "https://mcp.calendly.com/",
147+
authorizationCode: "code-123",
148+
});
149+
expect(authMock.mock.calls[0]?.[0]?.clientMetadata.redirect_uris).toEqual([
150+
"http://localhost:8989/oauth/callback",
151+
]);
152+
},
153+
{
154+
prefix: "openclaw-mcp-oauth-localhost-persist-",
155+
skipSessionCleanup: true,
156+
env: {
157+
OPENCLAW_CONFIG_PATH: undefined,
158+
OPENCLAW_STATE_DIR: undefined,
159+
},
160+
},
161+
);
162+
});
163+
119164
it("does not start hidden authorization flows without an authorization callback", async () => {
120165
// Normal agent/tool execution must not open browser auth flows implicitly;
121166
// operators use the explicit mcp login command instead.

‎src/agents/mcp-oauth.ts‎

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
* private OpenClaw state directory with one hashed file per MCP server URL.
44
*/
55
import { createHash, randomUUID } from "node:crypto";
6-
import fs from "node:fs/promises";
6+
import fs from "node:fs";
7+
import fsPromises from "node:fs/promises";
78
import path from "node:path";
89
import {
910
auth,
@@ -26,6 +27,7 @@ type McpOAuthStore = {
2627
codeVerifier?: string;
2728
discoveryState?: OAuthDiscoveryState;
2829
lastAuthorizationUrl?: string;
30+
redirectUrl?: string;
2931
state?: string;
3032
};
3133

@@ -59,24 +61,42 @@ function oauthStorePath(serverName: string, serverUrl: string): string {
5961

6062
async function readStore(filePath: string): Promise<McpOAuthStore> {
6163
try {
62-
return JSON.parse(await fs.readFile(filePath, "utf-8")) as McpOAuthStore;
64+
return JSON.parse(await fsPromises.readFile(filePath, "utf-8")) as McpOAuthStore;
65+
} catch {
66+
return {};
67+
}
68+
}
69+
70+
function readStoreSync(filePath: string): McpOAuthStore {
71+
try {
72+
return JSON.parse(fs.readFileSync(filePath, "utf-8")) as McpOAuthStore;
6373
} catch {
6474
return {};
6575
}
6676
}
6777

6878
async function writeStore(filePath: string, store: McpOAuthStore): Promise<void> {
69-
await fs.mkdir(path.dirname(filePath), { recursive: true, mode: 0o700 });
70-
await fs.writeFile(filePath, JSON.stringify(store, null, 2), { encoding: "utf-8", mode: 0o600 });
71-
await fs.chmod(filePath, 0o600).catch(() => {});
79+
await fsPromises.mkdir(path.dirname(filePath), { recursive: true, mode: 0o700 });
80+
await fsPromises.writeFile(filePath, JSON.stringify(store, null, 2), {
81+
encoding: "utf-8",
82+
mode: 0o600,
83+
});
84+
await fsPromises.chmod(filePath, 0o600).catch(() => {});
7285
}
7386

74-
function resolveOAuthRedirectUrl(config: McpOAuthConfig): string {
75-
return normalizeOptionalString(config.redirectUrl) ?? LEGACY_DEFAULT_REDIRECT_URL;
87+
function resolveOAuthRedirectUrl(config: McpOAuthConfig, store: McpOAuthStore = {}): string {
88+
return (
89+
normalizeOptionalString(config.redirectUrl) ??
90+
normalizeOptionalString(store.redirectUrl) ??
91+
LEGACY_DEFAULT_REDIRECT_URL
92+
);
7693
}
7794

78-
function buildOAuthClientMetadata(config: McpOAuthConfig): OAuthClientMetadata {
79-
const redirectUrl = resolveOAuthRedirectUrl(config);
95+
function buildOAuthClientMetadata(
96+
config: McpOAuthConfig,
97+
store: McpOAuthStore = {},
98+
): OAuthClientMetadata {
99+
const redirectUrl = resolveOAuthRedirectUrl(config, store);
80100
return {
81101
client_name: "OpenClaw MCP",
82102
redirect_uris: [redirectUrl],
@@ -99,7 +119,6 @@ export function createMcpOAuthClientProvider(params: {
99119
}): OAuthClientProvider {
100120
const config = params.config ?? {};
101121
const filePath = oauthStorePath(params.serverName, params.serverUrl);
102-
const redirectUrl = resolveOAuthRedirectUrl(config);
103122
const allowAuthorizationRedirect =
104123
params.allowAuthorizationRedirect ?? Boolean(params.onAuthorizationUrl);
105124
const assertAuthorizationRedirectAllowed = () => {
@@ -111,11 +130,11 @@ export function createMcpOAuthClientProvider(params: {
111130
};
112131
return {
113132
get redirectUrl() {
114-
return redirectUrl;
133+
return resolveOAuthRedirectUrl(config, readStoreSync(filePath));
115134
},
116135
clientMetadataUrl: normalizeOptionalString(config.clientMetadataUrl),
117136
get clientMetadata() {
118-
return buildOAuthClientMetadata(config);
137+
return buildOAuthClientMetadata(config, readStoreSync(filePath));
119138
},
120139
async state() {
121140
assertAuthorizationRedirectAllowed();
@@ -188,7 +207,7 @@ export async function clearMcpOAuthCredentials(params: {
188207
serverName: string;
189208
serverUrl: string;
190209
}): Promise<void> {
191-
await fs.rm(oauthStorePath(params.serverName, params.serverUrl), { force: true });
210+
await fsPromises.rm(oauthStorePath(params.serverName, params.serverUrl), { force: true });
192211
}
193212

194213
/** Reads stored OAuth credential presence without exposing credential values. */
@@ -238,13 +257,23 @@ export async function runMcpOAuthLogin(params: {
238257
fetchFn?: FetchLike;
239258
onAuthorizationUrl?: (url: URL) => void | Promise<void>;
240259
}): Promise<"authorized" | "redirect"> {
260+
const filePath = oauthStorePath(params.serverName, params.serverUrl);
261+
const store = await readStore(filePath);
262+
const loginParams = {
263+
...params,
264+
config: {
265+
...params.config,
266+
redirectUrl: normalizeOptionalString(params.config?.redirectUrl) ?? store.redirectUrl,
267+
},
268+
};
241269
try {
242-
return await runMcpOAuthLoginAttempt(params);
270+
return await runMcpOAuthLoginAttempt(loginParams);
243271
} catch (error) {
244272
if (
245273
!normalizeOptionalString(params.config?.redirectUrl) &&
246274
isMcpOAuthRedirectRegistrationError(error)
247275
) {
276+
await writeStore(filePath, { ...store, redirectUrl: LOCALHOST_REDIRECT_URL });
248277
return await runMcpOAuthLoginAttempt({
249278
...params,
250279
config: {

0 commit comments

Comments
 (0)