diff --git a/docs/SPEC.md b/docs/SPEC.md index 1e24c81..48b2b4a 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -214,14 +214,24 @@ auth logic of its own. | Path | Config | Lifecycle | Use | | --- | --- | --- | --- | -| OAuth | `auth: "oauth"` | `lazy` | interactive default | +| OAuth | `auth: "oauth"`, `oauth.clientId: "pi"` | `lazy` | interactive default | | API key | `auth: "bearer"`, `bearerTokenEnv: "RENDER_API_KEY"` | `lazy` | CI / non-interactive | -**OAuth** is the interactive default and needs no API key, matching the Claude Code plugin. The -adapter performs dynamic client registration when `oauth.clientId` is omitted; the user drives -it with `/mcp-auth render` (or `settings.autoAuth: true`), and credentials land in the OS +**OAuth** is the interactive default and needs no API key, matching the Claude Code plugin. The user +drives it with `/mcp-auth render` (or `settings.autoAuth: true`), and credentials land in the OS credential store. Headless Linux requires an unlocked libsecret keyring. +`oauth.clientId` is **required, not optional**. Render's authorization server +(`https://api.render.com`, discovered through the MCP server's protected-resource metadata) +publishes `authorization_code` and `refresh_token` grants with S256 PKCE and no +`registration_endpoint`. Dynamic client registration is therefore impossible. The adapter attempts +registration precisely when `oauth.clientId` is omitted, and that attempt fails with a 401 and a +misleading "does not appear to speak MCP" error, breaking the default auth path for every user. + +Render pre-registers one public PKCE client per integration — `claude`, `cursor`, `codex`, and `pi` +for this package. These IDs are public and carry no client secret. This is why the Claude Code +plugin ships `oauth.clientId: "claude"` rather than relying on registration. + **Lifecycle is not a free choice.** Both paths are `lazy`, so the adapter does not launch OAuth or keep a Render connection open at startup. The adapter has one documented exception: when its metadata cache does not exist, the first session makes a best-effort connection to populate it. diff --git a/src/mcp.ts b/src/mcp.ts index 4114b21..0767d74 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -1,10 +1,22 @@ const RENDER_MCP_URL = "https://mcp.render.com/mcp"; const RENDER_API_KEY_ENV = "RENDER_API_KEY"; +/** + * Render's authorization server publishes no `registration_endpoint`, so dynamic client + * registration cannot work. The adapter only falls back to registration when `oauth.clientId` + * is omitted, which is why this MUST stay set: without it every OAuth attempt fails at the + * probe with a 401 and a misleading "does not appear to speak MCP" error. + * + * The value is a public, pre-registered client ID, matching the sibling plugins' `claude`, + * `cursor`, and `codex`. It is not a secret and there is no client secret: Render registers + * these as public PKCE clients. + */ +const RENDER_OAUTH_CLIENT_ID = "pi"; + export type RenderMcpEnvironment = Readonly>; type RenderMcpAuth = - | { auth: "oauth" } + | { auth: "oauth"; oauth: { clientId: typeof RENDER_OAUTH_CLIENT_ID } } | { auth: "bearer"; bearerTokenEnv: typeof RENDER_API_KEY_ENV }; export interface RenderMcpConfig { @@ -29,7 +41,7 @@ export interface RenderMcpConfig { export function buildRenderMcpConfig(env: RenderMcpEnvironment): RenderMcpConfig { const auth: RenderMcpAuth = env[RENDER_API_KEY_ENV] ? { auth: "bearer" as const, bearerTokenEnv: RENDER_API_KEY_ENV } - : { auth: "oauth" as const }; + : { auth: "oauth" as const, oauth: { clientId: RENDER_OAUTH_CLIENT_ID } }; return { mcpServers: { diff --git a/tests/extension/loads.test.ts b/tests/extension/loads.test.ts index 94cce44..3de94c3 100644 --- a/tests/extension/loads.test.ts +++ b/tests/extension/loads.test.ts @@ -39,6 +39,7 @@ describe("pi-render extension", () => { render: { url: "https://mcp.render.com/mcp", auth: "oauth", + oauth: { clientId: "pi" }, lifecycle: "lazy", directTools: false, }, diff --git a/tests/unit/mcp.test.ts b/tests/unit/mcp.test.ts index 35e1920..7011c8e 100644 --- a/tests/unit/mcp.test.ts +++ b/tests/unit/mcp.test.ts @@ -17,8 +17,26 @@ describe("buildRenderMcpConfig", () => { expect(config.mcpServers.render).not.toHaveProperty("bearerTokenEnv"); }); + it("sends the pre-registered OAuth client ID so the adapter never attempts registration", () => { + // Render's authorization server publishes no registration_endpoint. The adapter only tries + // dynamic client registration when oauth.clientId is missing, and that attempt fails the + // probe with a 401 and a misleading "does not appear to speak MCP" error. Dropping this + // field silently breaks the default auth path for every user, so assert it explicitly. + const config = buildRenderMcpConfig({}); + + expect(config.mcpServers.render.auth).toBe("oauth"); + expect(config.mcpServers.render).toHaveProperty("oauth.clientId", "pi"); + }); + + it("registers a public client, so it carries no client secret", () => { + expect(JSON.stringify(buildRenderMcpConfig({}))).not.toMatch(/clientSecret|client_secret/i); + }); + it("treats an empty API key as absent", () => { - expect(buildRenderMcpConfig({ RENDER_API_KEY: "" }).mcpServers.render.auth).toBe("oauth"); + const config = buildRenderMcpConfig({ RENDER_API_KEY: "" }); + + expect(config.mcpServers.render.auth).toBe("oauth"); + expect(config.mcpServers.render).toHaveProperty("oauth.clientId", "pi"); }); it("references RENDER_API_KEY without copying its value into config", () => { @@ -35,6 +53,12 @@ describe("buildRenderMcpConfig", () => { expect(JSON.stringify(config)).not.toContain(secret); }); + it("omits the OAuth client ID on the bearer path, which never runs an OAuth flow", () => { + const config = buildRenderMcpConfig({ RENDER_API_KEY: "present" }); + + expect(config.mcpServers.render).not.toHaveProperty("oauth"); + }); + it.each([ ["OAuth", {}], ["bearer", { RENDER_API_KEY: "present" }],