diff --git a/packages/coding-agent/src/core/mcp-servers.ts b/packages/coding-agent/src/core/mcp-servers.ts index 858d8f2fa..a30a7a727 100644 --- a/packages/coding-agent/src/core/mcp-servers.ts +++ b/packages/coding-agent/src/core/mcp-servers.ts @@ -93,6 +93,11 @@ export interface McpHttpServerConfig extends McpServerConfigBase { /** Values may reference environment variables (`${NAME}`) or commands (`!cmd`). */ headers?: Record; oauth?: McpOAuthConfig; + /** + * Send the token of a pi provider (`/login `) instead of using OAuth. Not allowed in project + * `mcp.json` files, and requires https except on loopback hosts, since it sends the credential to `url`. + */ + auth?: { provider: string }; } export type McpServerConfig = McpStdioServerConfig | McpHttpServerConfig; @@ -217,6 +222,15 @@ export function validateMcpServerConfig(name: string, raw: unknown): McpServerCo } const oauthError = validateOAuth(value.oauth); if (oauthError) return `server "${name}": ${oauthError}`; + if (value.auth !== undefined) { + if (!isRecord(value.auth) || typeof value.auth.provider !== "string" || !value.auth.provider) { + return `server "${name}": auth.provider must be a provider name`; + } + const url = new URL(value.url); + if (url.protocol !== "https:" && !LOOPBACK_HOSTS.includes(url.hostname)) { + return `server "${name}": auth requires an https URL, or http on localhost, 127.0.0.1, or [::1]`; + } + } return value as unknown as McpHttpServerConfig; } if (typeof value.command === "string" && (type === undefined || type === "stdio")) { diff --git a/packages/coding-agent/src/extensions/mcp/config.ts b/packages/coding-agent/src/extensions/mcp/config.ts index b1b685779..c2b7efb8e 100644 --- a/packages/coding-agent/src/extensions/mcp/config.ts +++ b/packages/coding-agent/src/extensions/mcp/config.ts @@ -17,6 +17,8 @@ * ``` * * HTTP servers without an `Authorization` header use OAuth when they answer 401 (sign in with `/mcp`). + * `"auth": { "provider": "" }` sends the token of a `/login` provider instead. Project files + * cannot use it, so a repository cannot pick where the credential goes. * * The top-level `autoEnableCodemode` (default true) activates the codemode tool when a server * with `codemode` exposure connects. A project value overrides the global one. @@ -98,6 +100,10 @@ function readConfigFile(path: string, scope: "global" | "project", state: McpCon errors.push(`${path}: server "${name}" conflicts with "${clash}"`); continue; } + if (scope === "project" && "url" in config && config.auth) { + errors.push(`${path}: server "${name}": auth is only allowed in the global mcp.json`); + continue; + } servers.set(name, { name, config, source: path, scope }); } } diff --git a/packages/coding-agent/src/extensions/mcp/index.ts b/packages/coding-agent/src/extensions/mcp/index.ts index 14b81a82c..0248112b9 100644 --- a/packages/coding-agent/src/extensions/mcp/index.ts +++ b/packages/coding-agent/src/extensions/mcp/index.ts @@ -38,6 +38,7 @@ import type { ToolDefinition, } from "../../core/extensions/types.ts"; import { mcpNamespace } from "../../core/mcp-servers.ts"; +import type { ModelRegistry } from "../../core/model-registry.ts"; import { openBrowser } from "../../utils/open-browser.ts"; import { CODEMODE_TOOL_NAME, isCodemodeTool } from "../codemode/tool.ts"; import { isToolSearchTool, TOOL_SEARCH_TOOL_NAME } from "../tool-search/tool.ts"; @@ -287,6 +288,8 @@ export function createMcpExtension(options: McpExtensionOptions = {}): Extension /** Working directory of the session, for stdio servers. */ let sessionCwd = process.cwd(); let credentials = options.credentials; + /** The session's model registry, which resolves `auth.provider` tokens. */ + let modelRegistry: ModelRegistry | undefined; let serverLog: McpServerLog | undefined; const openUrl = options.openUrl ?? openBrowser; const updateConfig = @@ -507,6 +510,7 @@ export function createMcpExtension(options: McpExtensionOptions = {}): Extension cwd: sessionCwd, createTransport: options.createTransport ?? runtime.createDefaultTransport, credentials: getCredentials(runtime), + providerToken: async (provider) => modelRegistry?.getApiKeyForProvider(provider), log: getServerLog(runtime), onTools: registerTools, onChange: onConnectionChange, @@ -945,6 +949,7 @@ export function createMcpExtension(options: McpExtensionOptions = {}): Extension warnedUnreachable = false; waitedForStartup = false; sessionCwd = ctx.cwd; + modelRegistry = ctx.modelRegistry; const current = ++generation; sessionActive = true; configuredEntries = loaded.servers; diff --git a/packages/coding-agent/src/extensions/mcp/runtime.ts b/packages/coding-agent/src/extensions/mcp/runtime.ts index f848854f1..01e8b5b1d 100644 --- a/packages/coding-agent/src/extensions/mcp/runtime.ts +++ b/packages/coding-agent/src/extensions/mcp/runtime.ts @@ -73,14 +73,15 @@ function isTransientError(error: unknown): boolean { return error instanceof TypeError; } -function signInRequiredMessage(name: string): string { - return `MCP server "${name}" requires sign-in. Run /mcp to sign in.`; +function signInRequiredMessage(entry: McpServerEntry): string { + const provider = "url" in entry.config ? entry.config.auth?.provider : undefined; + return `MCP server "${entry.name}" requires sign-in. Run ${provider ? `/login ${provider}` : "/mcp"} to sign in.`; } -/** HTTP servers authenticate with OAuth unless the config supplies an `Authorization` header. */ +/** HTTP servers authenticate with OAuth unless the config supplies an `Authorization` header or `auth`. */ function usesOAuth(entry: McpServerEntry): boolean { const { config } = entry; - if (!("url" in config)) return false; + if (!("url" in config) || config.auth) return false; return !Object.keys(config.headers ?? {}).some((header) => header.toLowerCase() === "authorization"); } @@ -184,6 +185,8 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer { cwd: string; createTransport: McpTransportFactory; credentials: McpOAuthCredentialStore; + /** The current token of a pi provider, for servers with `auth.provider`. */ + providerToken?: (provider: string) => Promise; onTools: (connection: McpServerConnection) => void; /** Called when `state`, `error`, or `tools` change. */ onChange?: (connection: McpServerConnection) => void; @@ -197,6 +200,7 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer { this.onChange = options.onChange; this.log = options.log; const url = this.oauthUrl; + const provider = "url" in this.entry.config ? this.entry.config.auth?.provider : undefined; this.authProvider = url ? createMcpAuthProvider({ serverUrl: url, @@ -206,7 +210,10 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer { this.challenge = challenge; }, }) - : undefined; + : provider + ? // Read on every request, so the provider's refreshes apply; MCP stores no copy. + { token: async () => options.providerToken?.(provider), settled: async () => {} } + : undefined; } get name(): string { @@ -299,7 +306,7 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer { if (!this.needsSignIn(error)) throw error; await this.dropClient(client); this.markNeedsAuth(); - throw new Error(signInRequiredMessage(this.entry.name)); + throw new Error(signInRequiredMessage(this.entry)); } } } @@ -322,7 +329,7 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer { private needsSignIn(error: unknown): boolean { return ( error instanceof McpOAuthAuthorizationRequiredError || - (this.oauthUrl !== undefined && error instanceof McpAuthRequiredError) + (this.authProvider !== undefined && error instanceof McpAuthRequiredError) ); } @@ -412,7 +419,7 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer { private connectFailed(error: unknown): Error { if (this.needsSignIn(error) && !this.closed) { this.markNeedsAuth(); - return new Error(signInRequiredMessage(this.entry.name)); + return new Error(signInRequiredMessage(this.entry)); } this.state = this.closed ? "closed" : "failed"; this.error = this.stderrTail ? `${errorMessage(error)}\n${this.stderrTail}` : errorMessage(error); diff --git a/packages/coding-agent/test/mcp-extension.test.ts b/packages/coding-agent/test/mcp-extension.test.ts index 4afc5a909..b94fc8174 100644 --- a/packages/coding-agent/test/mcp-extension.test.ts +++ b/packages/coding-agent/test/mcp-extension.test.ts @@ -1,4 +1,6 @@ import { mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -179,6 +181,33 @@ describe("MCP config", () => { "codemode", ); }); + + it("validates provider auth and accepts it only in the global mcp.json", () => { + const paths = setup( + { + mcpServers: { + radius: { url: "https://radius.example/mcp", auth: { provider: "radius" } }, + local: { url: "http://localhost:8788/mcp", auth: { provider: "radius-dev" } }, + plain: { url: "http://radius.example/mcp", auth: { provider: "radius" } }, + empty: { url: "https://radius.example/mcp", auth: { provider: "" } }, + }, + }, + { mcpServers: { radius: { url: "https://evil.example/mcp", auth: { provider: "radius" } } } }, + ); + const { servers, errors } = loadMcpConfig({ ...paths, projectTrusted: true }); + // The project entry cannot replace the global one: it would send the credential to its own URL. + expect(servers.map((server) => [server.name, server.scope, "url" in server.config && server.config.url])).toEqual( + [ + ["radius", "global", "https://radius.example/mcp"], + ["local", "global", "http://localhost:8788/mcp"], + ], + ); + expect(errors).toEqual([ + expect.stringContaining('server "plain": auth requires an https URL'), + expect.stringContaining('server "empty": auth.provider must be a provider name'), + expect.stringContaining('server "radius": auth is only allowed in the global mcp.json'), + ]); + }); }); describe("MCP tools", () => { @@ -535,6 +564,40 @@ for await (const line of createInterface({ input: process.stdin })) { await connection.close(); }); + it("sends the provider token and asks for the provider login when the server rejects it", async () => { + const authorizations: (string | undefined)[] = []; + const server = createServer((request, response) => { + authorizations.push(request.headers.authorization); + request.resume(); + response.writeHead(401).end(); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const { port } = server.address() as AddressInfo; + const connection = new McpServerConnection({ + entry: { + name: "radius", + config: { url: `http://127.0.0.1:${port}/mcp`, auth: { provider: "radius" } }, + source: "test", + }, + cwd: process.cwd(), + createTransport: createDefaultTransport, + credentials: new McpOAuthCredentialStore(new InMemoryAuthStorageBackend()), + providerToken: async (provider) => (provider === "radius" ? "tok" : undefined), + onTools: () => {}, + }); + try { + expect(connection.oauthUrl).toBeUndefined(); + await expect(connection.getClient()).rejects.toThrow( + 'MCP server "radius" requires sign-in. Run /login radius to sign in.', + ); + expect(connection.state).toBe("needs-auth"); + expect(authorizations).toEqual(["Bearer tok"]); + } finally { + await connection.close(); + await new Promise((resolve) => server.close(resolve)); + } + }); + it("appends server log messages to the log file", async () => { const dir = mkdtempSync(join(tmpdir(), "pi-mcp-log-")); try {