diff --git a/packages/core/sdk/src/oauth-register-dynamic.test.ts b/packages/core/sdk/src/oauth-register-dynamic.test.ts index 3b8dce114..266c7cc4d 100644 --- a/packages/core/sdk/src/oauth-register-dynamic.test.ts +++ b/packages/core/sdk/src/oauth-register-dynamic.test.ts @@ -622,6 +622,88 @@ describe("oauth.registerDynamicClient", () => { ), ); + // Regression: Mercury's authorization server vets `client_name` and rejects + // any value containing its own brand with `invalid_client_metadata`, which + // the old auto-generated "Executor for Mercury MCP" always tripped. The UI + // flow now always registers under the bare product name, which brand-vetting + // servers accept. + it.effect("registers under the bare product name against a brand-vetting server", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ + scopes: ["read"], + // Mirror Mercury: reject any client_name containing the brand. + approveClientName: (name) => !name.includes("Acme"), + }); + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + const probe = yield* executor.oauth.probe({ url: server.mcpResourceUrl }); + + const slug = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: CLIENT, + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: probe.resource, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Executor", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: INTEG, + }); + expect(String(slug).length).toBeGreaterThan(0); + + const requests = yield* server.requests; + const registers = requests.filter((r) => r.path === "/register" && r.method === "POST"); + expect(registers).toHaveLength(1); + expect(registers[0]!.body).toContain('"client_name":"Executor"'); + }), + ), + ); + + // A server that rejects even the bare product name surfaces the RFC error + // untouched: no retry loop, no masking of the server-authored description. + it.effect("surfaces invalid_client_metadata when the server rejects the client_name", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ + scopes: ["read"], + approveClientName: () => false, + }); + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + const probe = yield* executor.oauth.probe({ url: server.mcpResourceUrl }); + + const error = yield* Effect.flip( + executor.oauth.registerDynamicClient({ + owner: "org", + slug: CLIENT, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: probe.resource, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Executor", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: INTEG, + }), + ); + expect(Predicate.isTagged("OAuthRegisterDynamicError")(error)).toBe(true); + const registerError = error as OAuthRegisterDynamicError; + expect(registerError.message).toContain( + "Dynamic Client Registration failed: invalid_client_metadata", + ); + + const requests = yield* server.requests; + const registers = requests.filter((r) => r.path === "/register" && r.method === "POST"); + expect(registers).toHaveLength(1); + }), + ), + ); + // Regression: issue #770. Vercel (and other RFC 8252-strict servers) only // approve loopback redirect URIs for anonymous DCR, so a hosted/tailnet/LAN // origin trips `invalid_redirect_uri`. The failure must explain the loopback diff --git a/packages/core/sdk/src/testing/oauth-test-server.ts b/packages/core/sdk/src/testing/oauth-test-server.ts index 17413f5f4..95ef31ec0 100644 --- a/packages/core/sdk/src/testing/oauth-test-server.ts +++ b/packages/core/sdk/src/testing/oauth-test-server.ts @@ -66,6 +66,11 @@ export interface OAuthTestServerOptions { * `redirect_uris` entry is approved. Mirrors authorization servers (e.g. * Vercel) that only accept loopback redirect URIs for anonymous DCR. */ readonly approveRedirectUri?: (uri: string) => boolean; + /** Gate Dynamic Client Registration on the requested `client_name`. When set, + * `/register` returns `400 invalid_client_metadata` unless the requested + * name is approved. Mirrors authorization servers (e.g. Mercury) that + * reject third-party client names containing their own brand. */ + readonly approveClientName?: (name: string) => boolean; } export interface OAuthTestServerShape { @@ -517,6 +522,14 @@ export const serveOAuthTestServer = ( if (!json) { return oauthError(400, "invalid_client_metadata", "Expected JSON body"); } + const requestedClientName = typeof json.client_name === "string" ? json.client_name : ""; + if (options.approveClientName && !options.approveClientName(requestedClientName)) { + return oauthError( + 400, + "invalid_client_metadata", + "The requested client_name is not allowed by this authorization server.", + ); + } const requestedMethod = typeof json.token_endpoint_auth_method === "string" ? json.token_endpoint_auth_method diff --git a/packages/react/src/components/add-account-modal.test.ts b/packages/react/src/components/add-account-modal.test.ts index 2b55eecc1..1d997a603 100644 --- a/packages/react/src/components/add-account-modal.test.ts +++ b/packages/react/src/components/add-account-modal.test.ts @@ -14,7 +14,6 @@ import { connectionLabel, connectionLabelForHost, createCredentialPayloadOrigin, - dcrClientNameForIntegration, DEFAULT_CONNECTION_OWNER, mergeCustomMethods, oauthIdentityLabelFromHealth, @@ -356,11 +355,6 @@ describe("runCimdConnect", () => { }); describe("runDcrConnect", () => { - it("names dynamically registered OAuth apps as Executor clients", () => { - expect(dcrClientNameForIntegration("PostHog MCP")).toBe("Executor for PostHog MCP"); - expect(dcrClientNameForIntegration(" ")).toBe("Executor"); - }); - it("auto-registers (no picker) then starts: probe → register → start in order", async () => { const calls: string[] = []; let registerArgs: RegisterArgs | null = null; @@ -393,7 +387,6 @@ describe("runDcrConnect", () => { { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", - integrationName: "Linear MCP", redirectUri: "https://localhost:5394/api/oauth/callback", integration: TEST_INTEGRATION, }, @@ -412,7 +405,9 @@ describe("runDcrConnect", () => { expect(registerArgs!.tokenUrl).toBe("https://auth.example.com/token"); expect(registerArgs!.resource).toBe("https://mcp.example.com/mcp"); expect(registerArgs!.tokenEndpointAuthMethodsSupported).toEqual(["none"]); - expect(registerArgs!.clientName).toBe("Executor for Linear MCP"); + // Always the bare product name: brand-vetting servers (e.g. Mercury) + // reject client_names containing their own brand. + expect(registerArgs!.clientName).toBe("Executor"); expect(registerArgs!.scopes).toEqual(["mcp.read"]); expect(registerArgs!.redirectUri).toBe("https://localhost:5394/api/oauth/callback"); expect(registerArgs!.originIntegration).toBe(TEST_INTEGRATION); @@ -443,7 +438,6 @@ describe("runDcrConnect", () => { discoveryUrl: "https://mcp.example.com/mcp", resourceFallback: "https://mcp.example.com/mcp", owner: "user", - integrationName: "App", declaredScopes: ["declared.scope"], integration: TEST_INTEGRATION, }, @@ -479,7 +473,6 @@ describe("runDcrConnect", () => { // discoveryUrl collapsed to the token endpoint (no real discovery URL). discoveryUrl: "https://auth.example.com/token", owner: "user", - integrationName: "App", integration: TEST_INTEGRATION, }, ); @@ -510,7 +503,6 @@ describe("runDcrConnect", () => { { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", - integrationName: "App", integration: TEST_INTEGRATION, }, ); @@ -544,7 +536,6 @@ describe("runDcrConnect", () => { { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", - integrationName: "App", integration: TEST_INTEGRATION, }, ); @@ -573,7 +564,6 @@ describe("runDcrConnect", () => { { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", - integrationName: "App", integration: TEST_INTEGRATION, }, ); @@ -611,7 +601,6 @@ describe("runDcrConnect", () => { { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", - integrationName: "App", integration: TEST_INTEGRATION, }, ); @@ -653,7 +642,6 @@ describe("runDcrConnect", () => { { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", - integrationName: "App", integration: TEST_INTEGRATION, }, ); diff --git a/packages/react/src/components/add-account-modal.tsx b/packages/react/src/components/add-account-modal.tsx index 7259b199c..9ffe20354 100644 --- a/packages/react/src/components/add-account-modal.tsx +++ b/packages/react/src/components/add-account-modal.tsx @@ -761,7 +761,6 @@ type RunDcrConnectInput = { * discovery URL (non-MCP DCR), collapsing the resource to null as before. */ readonly resourceFallback?: string; readonly owner: Owner; - readonly integrationName: string; /** Scopes declared by the integration's method (override the probed ones). */ readonly declaredScopes?: readonly string[]; /** Browser-facing callback URL registered with DCR when available. */ @@ -770,10 +769,14 @@ type RunDcrConnectInput = { readonly integration: IntegrationSlug; }; -export const dcrClientNameForIntegration = (integrationName: string): string => { - const trimmed = integrationName.trim(); - return trimmed.length > 0 ? `Executor for ${trimmed}` : "Executor"; -}; +/** RFC 7591 `client_name` sent for every dynamic registration. Deliberately + * the bare product name, never "Executor for ": some servers + * (e.g. Mercury) vet the name and reject any value containing their own + * brand with `invalid_client_metadata`, killing the automatic connect. The + * name is cosmetic (it only labels the provider's consent screen and app + * list, where the provider is already evident), so the suffix bought + * nothing worth that failure class. */ +const DCR_CLIENT_NAME = "Executor"; /** * Run the transparent DCR connect sequence: probe → register → start. @@ -806,7 +809,7 @@ export async function runDcrConnect( resource: probe.resource ?? input.resourceFallback ?? null, scopes, tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, - clientName: dcrClientNameForIntegration(input.integrationName), + clientName: DCR_CLIENT_NAME, redirectUri: input.redirectUri, originIntegration: input.integration, }); @@ -2099,7 +2102,6 @@ function AddAccountModalView(props: AddAccountModalProps) { // not, so pass the un-collapsed method value here. resourceFallback: method.oauth?.discoveryUrl, owner: dcrOwner, - integrationName, // DCR slugs are server-keyed (Part A): the connect path no longer depends // on the picker's app list, so it need not be threaded here. declaredScopes: method.oauth?.scopes,