From 19cf40f9b751957c098a9fc8595ec61279afd116 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 13 Jan 2026 10:00:21 -0500 Subject: [PATCH 01/25] Add MCP elicitation for secure preview token handling - Implement token elicitation to keep tokens out of chat history - Users can provide, create, or auto-create preview tokens - Add session-level token storage to avoid repeated prompts - Support URL-restricted tokens for enhanced security - Maintain backward compatibility with direct token provision - Update README with security best practices Security improvements: - Preview tokens no longer appear in chat history via elicitation - Users can create URL-restricted tokens inline - Token caching reduces friction while maintaining security Co-Authored-By: Claude Sonnet 4.5 --- README.md | 20 +- .../PreviewStyleTool.input.schema.ts | 10 +- .../preview-style-tool/PreviewStyleTool.ts | 236 +++++++++++++++++- src/utils/tokenElicitation.ts | 163 ++++++++++++ 4 files changed, 418 insertions(+), 11 deletions(-) create mode 100644 src/utils/tokenElicitation.ts diff --git a/README.md b/README.md index 8d7a47e..281685c 100644 --- a/README.md +++ b/README.md @@ -185,11 +185,21 @@ Complete set of tools for managing Mapbox styles via the Styles API: - Input: `styleId` - Returns: Success confirmation -**PreviewStyleTool** - Generate preview URL for a Mapbox style using an existing public token - -- Input: `styleId`, `title` (optional), `zoomwheel` (optional), `zoom` (optional), `center` (optional), `bearing` (optional), `pitch` (optional) +**PreviewStyleTool** - Generate preview URL for a Mapbox style with secure token handling + +- Input: + - `styleId` (required): Style ID to preview + - `accessToken` (optional): Provide a specific public token (for backward compatibility) + - `useCustomToken` (optional): Force token selection dialog even if a token is cached + - `title` (optional): Show title in preview + - `zoomwheel` (optional): Enable zoom wheel control - Returns: URL to open the style preview in browser -- **Note**: This tool automatically fetches the first available public token from your account for the preview URL. Requires at least one public token with `styles:read` scope. +- **🔐 Secure Token Handling**: If `accessToken` is not provided, this tool uses MCP **elicitation** to securely request a preview token from you without storing it in chat history. You'll be prompted to: + 1. **Provide an existing token** - Paste a token you already have + 2. **Create a new preview token** - Create a new token with optional URL restrictions for enhanced security + 3. **Auto-create a basic token** - Let the tool create a simple preview token for you +- **Session Storage**: Your token choice is cached for the session, so you only need to provide it once +- **Best Practice**: Use URL-restricted tokens (option 2) to limit token usage to specific domains **ValidateStyleTool** - Validate Mapbox style JSON against the Mapbox Style Specification @@ -211,7 +221,7 @@ Complete set of tools for managing Mapbox styles via the Styles API: - **RetrieveStyleTool**: Requires `styles:download` scope - **UpdateStyleTool**: Requires `styles:write` scope - **DeleteStyleTool**: Requires `styles:write` scope -- **PreviewStyleTool**: Requires `tokens:read` scope (to list tokens) and at least one public token with `styles:read` scope +- **PreviewStyleTool**: Can work without token scopes via elicitation, or optionally accepts a direct public token. If using automatic token listing, requires `tokens:read` scope **Note:** The username is automatically extracted from the JWT token payload. diff --git a/src/tools/preview-style-tool/PreviewStyleTool.input.schema.ts b/src/tools/preview-style-tool/PreviewStyleTool.input.schema.ts index eec52c3..93a46b0 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.input.schema.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.input.schema.ts @@ -8,8 +8,16 @@ export const PreviewStyleSchema = z.object({ 'pk.', 'Invalid access token. Only public tokens (starting with pk.*) are allowed for preview URLs. Secret tokens (sk.*) cannot be used as they cannot be exposed in browser URLs.' ) + .optional() + .describe( + 'Mapbox public access token (optional). If not provided, you will be prompted to provide, create, or auto-create a preview token. Must start with pk.* and have styles:read permission. Secret tokens (sk.*) cannot be used as they cannot be exposed in browser URLs.' + ), + useCustomToken: z + .boolean() + .optional() + .default(false) .describe( - 'Mapbox public access token (required, must start with pk.* and have styles:read permission). Secret tokens (sk.*) cannot be used as they cannot be exposed in browser URLs. Please use an existing public token or get one from list_tokens_tool or create one with create_token_tool with styles:read permission.' + 'Force token selection dialog even if a preview token is already stored for this session. Useful when you want to use a different token.' ), title: z .boolean() diff --git a/src/tools/preview-style-tool/PreviewStyleTool.ts b/src/tools/preview-style-tool/PreviewStyleTool.ts index cf028cf..dee980d 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.ts @@ -8,6 +8,11 @@ import { } from './PreviewStyleTool.input.schema.js'; import { getUserNameFromToken } from '../../utils/jwtUtils.js'; import { isMcpUiEnabled } from '../../config/toolConfig.js'; +import { + elicitPreviewToken, + previewTokenStorage, + type ExistingTokenInfo +} from '../../utils/tokenElicitation.js'; export class PreviewStyleTool extends BaseTool { readonly name = 'preview_style_tool'; @@ -25,10 +30,121 @@ export class PreviewStyleTool extends BaseTool { super({ inputSchema: PreviewStyleSchema }); } - protected async execute(input: PreviewStyleInput): Promise { + protected async execute( + input: PreviewStyleInput, + serverAccessToken?: string + ): Promise { + let publicToken: string; let userName: string; + + // Step 1: Determine which token to use for preview + if (input.accessToken) { + // User provided token directly (backward compatibility) + publicToken = input.accessToken; + } else { + // No token provided - use elicitation flow + try { + // Get username from server access token to check storage + userName = getUserNameFromToken(serverAccessToken || ''); + } catch (error) { + return { + isError: true, + content: [ + { + type: 'text', + text: + 'Server access token is required when no preview token is provided. ' + + (error instanceof Error ? error.message : String(error)) + } + ] + }; + } + + // Check for stored preview token (unless user wants to use custom) + const storedToken = previewTokenStorage.get(userName); + if (storedToken && !input.useCustomToken) { + publicToken = storedToken; + } else { + // Need to elicit token from user + if (!this.server) { + return { + isError: true, + content: [ + { + type: 'text', + text: 'Server not initialized. Cannot elicit token from user.' + } + ] + }; + } + + // Get existing public tokens to show user + const existingTokens = await this.listPublicTokens(serverAccessToken); + + // Elicit token choice from user + const elicited = await elicitPreviewToken( + this.server.server, + existingTokens + ); + + // Handle user's choice + if (elicited.choice === 'provide') { + if (!elicited.token) { + return { + isError: true, + content: [ + { + type: 'text', + text: 'No token provided. Please provide a valid public token.' + } + ] + }; + } + publicToken = elicited.token; + } else if (elicited.choice === 'create') { + // Create new token with user's specifications + const created = await this.createPreviewToken( + serverAccessToken, + elicited.tokenNote, + elicited.urlRestrictions + ); + if (!created.success) { + return { + isError: true, + content: [ + { + type: 'text', + text: `Failed to create token: ${created.error}` + } + ] + }; + } + publicToken = created.token!; + } else { + // auto - create basic preview token + const created = await this.createPreviewToken(serverAccessToken); + if (!created.success) { + return { + isError: true, + content: [ + { + type: 'text', + text: `Failed to auto-create token: ${created.error}` + } + ] + }; + } + publicToken = created.token!; + } + + // Store token for future use + previewTokenStorage.set(userName, publicToken); + } + } + + // Step 2: Get username from the preview token try { - userName = getUserNameFromToken(input.accessToken); + userName = getUserNameFromToken(publicToken); } catch (error) { return { isError: true, @@ -41,9 +157,6 @@ export class PreviewStyleTool extends BaseTool { }; } - // Use the user-provided public token - const publicToken = input.accessToken; - // Build URL for the embeddable HTML endpoint const params = new URLSearchParams(); params.append('access_token', publicToken); @@ -94,4 +207,117 @@ export class PreviewStyleTool extends BaseTool { isError: false }; } + + /** + * List existing public tokens from the user's Mapbox account + */ + private async listPublicTokens( + accessToken?: string + ): Promise { + if (!accessToken) { + return []; + } + + try { + const userName = getUserNameFromToken(accessToken); + const response = await fetch( + `${MapboxApiBasedTool.mapboxApiEndpoint}tokens/v2/${userName}?access_token=${accessToken}` + ); + + if (!response.ok) { + // If we can't list tokens, return empty array (non-fatal) + return []; + } + + const data = await response.json(); + const tokens = data as Array<{ + id: string; + note: string; + scopes: string[]; + token?: string; + }>; + + // Filter to public tokens with styles:read scope + return tokens + .filter( + (t) => t.token?.startsWith('pk.') && t.scopes.includes('styles:read') + ) + .map((t) => ({ + id: t.id, + note: t.note || t.id, + scopes: t.scopes + })); + } catch { + // Non-fatal error - return empty array + return []; + } + } + + /** + * Create a new preview token via Mapbox API + */ + private async createPreviewToken( + accessToken?: string, + note?: string, + urlRestrictions?: string[] + ): Promise<{ success: boolean; token?: string; error?: string }> { + if (!accessToken) { + return { + success: false, + error: 'Server access token is required to create preview tokens' + }; + } + + try { + const userName = getUserNameFromToken(accessToken); + const tokenNote = + note || `MCP Preview Token - ${new Date().toISOString().split('T')[0]}`; + + const body: { + note: string; + scopes: string[]; + allowedUrls?: string[]; + } = { + note: tokenNote, + scopes: ['styles:read', 'styles:tiles', 'styles:download'] + }; + + if (urlRestrictions && urlRestrictions.length > 0) { + body.allowedUrls = urlRestrictions; + } + + const response = await fetch( + `${MapboxApiBasedTool.mapboxApiEndpoint}tokens/v2/${userName}?access_token=${accessToken}`, + { + method: 'POST', + headers: { + 'Content-Type': 'application/json' + }, + body: JSON.stringify(body) + } + ); + + if (!response.ok) { + const errorText = await response.text(); + return { + success: false, + error: `Failed to create token: ${response.status} ${errorText}` + }; + } + + const data = (await response.json()) as { token: string }; + return { + success: true, + token: data.token + }; + } catch (error) { + return { + success: false, + error: + error instanceof Error + ? error.message + : 'Unknown error creating token' + }; + } + } } diff --git a/src/utils/tokenElicitation.ts b/src/utils/tokenElicitation.ts new file mode 100644 index 0000000..9b0a90c --- /dev/null +++ b/src/utils/tokenElicitation.ts @@ -0,0 +1,163 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { Server } from '@modelcontextprotocol/sdk/server/index.js'; + +/** + * Token choice options for preview token elicitation + */ +export type TokenChoice = 'provide' | 'create' | 'auto'; + +/** + * Result of token elicitation + */ +export interface ElicitedTokenInfo { + choice: TokenChoice; + token?: string; + urlRestrictions?: string[]; + tokenNote?: string; +} + +/** + * Existing token info for display + */ +export interface ExistingTokenInfo { + id: string; + note: string; + scopes: string[]; +} + +/** + * Elicits preview token information from the user via MCP elicitation. + * This keeps the token out of chat history for better security. + * + * @param server - MCP Server instance + * @param existingTokens - List of user's existing public tokens + * @returns Elicited token information based on user's choice + */ +export async function elicitPreviewToken( + server: Server, + existingTokens: ExistingTokenInfo[] +): Promise { + const hasExistingTokens = existingTokens.length > 0; + const tokenList = hasExistingTokens + ? existingTokens + .map((t) => `- ${t.note || t.id}: ${t.scopes.join(', ')}`) + .join('\n') + : 'No existing public tokens found.'; + + const result = await server.elicitInput({ + message: `Preview Token Setup + +Preview URLs require a public token with styles:read scope. This token will be visible in the preview URL. + +${hasExistingTokens ? 'Your existing public tokens:\n' + tokenList : tokenList} + +For best security, consider using a URL-restricted token that only works on your domains.`, + requestedSchema: { + type: 'object', + properties: { + choice: { + type: 'string', + title: 'Token Option', + description: 'How would you like to provide the preview token?', + enum: ['provide', 'create', 'auto'], + enumNames: [ + 'I have a token to provide', + 'Create a new preview token with custom settings', + 'Auto-create a basic preview token for me' + ] + }, + token: { + type: 'string', + title: 'Your Token', + description: + 'Paste your public Mapbox token here (must have styles:read scope)', + minLength: 10 + }, + tokenNote: { + type: 'string', + title: 'Token Name (Optional)', + description: + 'A descriptive name for your new token (e.g., "Preview Token - Production")', + maxLength: 256 + }, + urlRestrictions: { + type: 'string', + title: 'URL Restrictions (Optional)', + description: + 'Comma-separated URLs to restrict token usage (e.g., "https://yourdomain.com/*,https://staging.yourdomain.com/*")' + } + }, + required: ['choice'] + } + }); + + // Check if user accepted or declined + if (result.action !== 'accept' || !result.content) { + throw new Error('Token elicitation was cancelled or declined by user'); + } + + // Parse the result + const choice = (result.content.choice as TokenChoice) || 'auto'; + const token = result.content.token as string | undefined; + const tokenNote = result.content.tokenNote as string | undefined; + const urlRestrictionsStr = result.content.urlRestrictions as + | string + | undefined; + + const urlRestrictions = urlRestrictionsStr + ? urlRestrictionsStr + .split(',') + .map((url) => url.trim()) + .filter((url) => url.length > 0) + : undefined; + + return { + choice, + token, + urlRestrictions, + tokenNote + }; +} + +/** + * Session-level storage for preview token preferences. + * In a real implementation, this could be stored in a database or cache. + */ +class PreviewTokenStorage { + private tokenCache = new Map(); + + /** + * Store a preview token for a specific username + */ + set(username: string, token: string): void { + this.tokenCache.set(username, token); + } + + /** + * Get stored preview token for a username + */ + get(username: string): string | undefined { + return this.tokenCache.get(username); + } + + /** + * Clear stored token for a username + */ + clear(username: string): void { + this.tokenCache.delete(username); + } + + /** + * Clear all stored tokens + */ + clearAll(): void { + this.tokenCache.clear(); + } +} + +/** + * Global preview token storage instance + */ +export const previewTokenStorage = new PreviewTokenStorage(); From 2b81c6778961c981cd8fd706713d4221a24b38c4 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 13 Jan 2026 10:30:31 -0500 Subject: [PATCH 02/25] Fix: Ensure preview tokens are created as public tokens (pk.*) Critical security fix for PreviewStyleTool: - Add `public: true` flag to token creation API request body - Validate that created tokens start with 'pk.' prefix - Prevent accidental creation of secret tokens (sk.*) which should never be exposed in browser URLs This ensures preview URLs always use public tokens that can be safely shared in preview URLs without security risk. Co-Authored-By: Claude Sonnet 4.5 --- src/tools/preview-style-tool/PreviewStyleTool.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/tools/preview-style-tool/PreviewStyleTool.ts b/src/tools/preview-style-tool/PreviewStyleTool.ts index dee980d..76ba8ad 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.ts @@ -277,9 +277,11 @@ export class PreviewStyleTool extends BaseTool { note: string; scopes: string[]; allowedUrls?: string[]; + public?: boolean; } = { note: tokenNote, - scopes: ['styles:read', 'styles:tiles', 'styles:download'] + scopes: ['styles:read', 'styles:tiles', 'styles:download'], + public: true // CRITICAL: Must be public token for browser URLs }; if (urlRestrictions && urlRestrictions.length > 0) { @@ -306,6 +308,15 @@ export class PreviewStyleTool extends BaseTool { } const data = (await response.json()) as { token: string }; + + // Validate that we got a public token (starts with pk.) + if (!data.token.startsWith('pk.')) { + return { + success: false, + error: `API returned a non-public token (${data.token.substring(0, 3)}...). Preview tokens must be public tokens (pk.*) that can be safely exposed in URLs.` + }; + } + return { success: true, token: data.token From 0e687432360d071e002fbea633d2eb374f332e19 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 13 Jan 2026 10:34:27 -0500 Subject: [PATCH 03/25] Fix: Use only public scopes to create public tokens (pk.*) Root cause: The Mapbox Tokens API automatically determines token type (public vs secret) based on the SCOPES requested, not an explicit parameter. Problem: - We were requesting 'styles:download' which is a SECRET scope - This forced the API to create a secret token (sk.*) instead of public (pk.*) - Secret tokens cannot be safely exposed in browser URLs Solution: - Changed scopes to only public scopes: ['styles:read', 'styles:tiles', 'fonts:read'] - These are sufficient for preview URLs and guarantee public token creation - Removed the unsupported 'public: true' parameter - Updated comments to explain the scope selection rationale Testing: Verified in MCP Inspector that auto-create now produces pk.* tokens Co-Authored-By: Claude Sonnet 4.5 --- src/tools/preview-style-tool/PreviewStyleTool.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tools/preview-style-tool/PreviewStyleTool.ts b/src/tools/preview-style-tool/PreviewStyleTool.ts index 76ba8ad..74f2432 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.ts @@ -277,11 +277,11 @@ export class PreviewStyleTool extends BaseTool { note: string; scopes: string[]; allowedUrls?: string[]; - public?: boolean; } = { note: tokenNote, - scopes: ['styles:read', 'styles:tiles', 'styles:download'], - public: true // CRITICAL: Must be public token for browser URLs + // CRITICAL: Only use public scopes to get a public token (pk.*) + // styles:download is a secret scope and would create sk.* token + scopes: ['styles:read', 'styles:tiles', 'fonts:read'] }; if (urlRestrictions && urlRestrictions.length > 0) { From f51feff5734a132c2c60d24c4615670e21ab1afd Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 13 Jan 2026 11:00:39 -0500 Subject: [PATCH 04/25] Fix: Check client elicitation capability before using elicitInput() According to the MCP specification, servers must verify that the client supports elicitation capability before attempting to use elicitInput(). Changes: - Added client capability check before calling elicitPreviewToken() - Returns clear error message if client doesn't support elicitation - Suggests providing accessToken parameter directly as fallback - Prevents "Method not found" errors when client lacks capability This fixes the issue where tools using elicitation would fail on clients that don't advertise elicitation support in their capabilities. Reference: https://modelcontextprotocol.io/specification/2025-11-25/client/elicitation Co-Authored-By: Claude Sonnet 4.5 --- src/tools/preview-style-tool/PreviewStyleTool.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/tools/preview-style-tool/PreviewStyleTool.ts b/src/tools/preview-style-tool/PreviewStyleTool.ts index 74f2432..9a316e8 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.ts @@ -78,6 +78,22 @@ export class PreviewStyleTool extends BaseTool { }; } + // Check if client supports elicitation capability + const clientCapabilities = this.server.server.getClientCapabilities(); + if (!clientCapabilities?.elicitation) { + return { + isError: true, + content: [ + { + type: 'text', + text: + 'Preview token required but client does not support elicitation. ' + + 'Please provide an accessToken parameter directly, or use a client that supports MCP elicitation (e.g., Claude Desktop, Claude Code).' + } + ] + }; + } + // Get existing public tokens to show user const existingTokens = await this.listPublicTokens(serverAccessToken); From ec35e385be966761f92ffad34bb8f92634059f57 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 13 Jan 2026 11:04:56 -0500 Subject: [PATCH 05/25] Docs: Clarify varying MCP elicitation support across clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added documentation to clarify that MCP elicitation support varies by client: - MCP Inspector has full support for secure token elicitation - Claude Desktop does not support elicitation yet, but Claude intelligently falls back to offering token creation via create_token_tool - Other clients should check their documentation for elicitation support Changes: - Added "Note on MCP Elicitation Support" in Quick Start section - Updated PreviewStyleTool description with client-specific behavior - Clarified that tokens appear in chat history when elicitation is unavailable - Added visual indicators (✅/⚠️) for support status This helps users understand expected behavior based on their MCP client. Co-Authored-By: Claude Sonnet 4.5 --- README.md | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 281685c..8501818 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,12 @@ Get started by integrating with your preferred AI development environment: - [Cursor Integration](./docs/cursor-integration.md) - Cursor IDE integration - [VS Code Integration](./docs/vscode-integration.md) - Visual Studio Code with GitHub Copilot +**Note on MCP Elicitation Support**: Some tools (like `preview_style_tool`) use [MCP elicitation](https://modelcontextprotocol.io/specification/2025-11-25/client/elicitation) to securely request tokens without exposing them in chat history. Elicitation support varies by client: + +- **MCP Inspector**: ✅ Full support +- **Claude Desktop**: ⚠️ Not yet supported (Claude will fall back to creating tokens via chat) +- **Claude Code, Cursor, VS Code**: Check client documentation for elicitation support status + ### DXT Package Distribution This MCP server can be packaged as a DXT (Desktop Extension) file for easy distribution and installation. DXT is a standardized format for distributing local MCP servers, similar to browser extensions. @@ -194,12 +200,15 @@ Complete set of tools for managing Mapbox styles via the Styles API: - `title` (optional): Show title in preview - `zoomwheel` (optional): Enable zoom wheel control - Returns: URL to open the style preview in browser -- **🔐 Secure Token Handling**: If `accessToken` is not provided, this tool uses MCP **elicitation** to securely request a preview token from you without storing it in chat history. You'll be prompted to: - 1. **Provide an existing token** - Paste a token you already have - 2. **Create a new preview token** - Create a new token with optional URL restrictions for enhanced security - 3. **Auto-create a basic token** - Let the tool create a simple preview token for you -- **Session Storage**: Your token choice is cached for the session, so you only need to provide it once -- **Best Practice**: Use URL-restricted tokens (option 2) to limit token usage to specific domains +- **🔐 Secure Token Handling**: If `accessToken` is not provided, this tool attempts to use MCP **elicitation** to securely request a preview token without storing it in chat history. However, **elicitation support varies by client**: + - **MCP Inspector**: ✅ Full support - Shows secure form dialog with three options: + 1. **Provide an existing token** - Paste a token you already have + 2. **Create a new preview token** - Create a new token with optional URL restrictions for enhanced security + 3. **Auto-create a basic token** - Let the tool create a simple preview token for you + - **Claude Desktop**: ⚠️ Not yet supported - When elicitation is unavailable, Claude will intelligently offer to create a token for you using `create_token_tool` (token will appear in chat history) + - **Alternative**: Provide `accessToken` parameter directly for backward compatibility with any client +- **Session Storage**: Your token choice is cached for the session, so you only need to provide it once (when elicitation is supported) +- **Best Practice**: Use URL-restricted tokens to limit token usage to specific domains **ValidateStyleTool** - Validate Mapbox style JSON against the Mapbox Style Specification From b9a22462c2dc14fb6427145cd23b79a4d9aae290 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 13 Jan 2026 11:15:27 -0500 Subject: [PATCH 06/25] Docs: Update elicitation support status for Cursor and VS Code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirmed that Cursor and VS Code both have full MCP elicitation support. Updated README to accurately reflect support status: ✅ Full support: - MCP Inspector - Cursor - VS Code (with Copilot) ⚠️ Not yet supported: - Claude Desktop (falls back to create_token_tool) Co-Authored-By: Claude Sonnet 4.5 --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 8501818..030f774 100644 --- a/README.md +++ b/README.md @@ -61,8 +61,10 @@ Get started by integrating with your preferred AI development environment: **Note on MCP Elicitation Support**: Some tools (like `preview_style_tool`) use [MCP elicitation](https://modelcontextprotocol.io/specification/2025-11-25/client/elicitation) to securely request tokens without exposing them in chat history. Elicitation support varies by client: - **MCP Inspector**: ✅ Full support +- **Cursor**: ✅ Full support +- **VS Code (with Copilot)**: ✅ Full support - **Claude Desktop**: ⚠️ Not yet supported (Claude will fall back to creating tokens via chat) -- **Claude Code, Cursor, VS Code**: Check client documentation for elicitation support status +- **Claude Code**: Check for latest support status ### DXT Package Distribution @@ -200,8 +202,8 @@ Complete set of tools for managing Mapbox styles via the Styles API: - `title` (optional): Show title in preview - `zoomwheel` (optional): Enable zoom wheel control - Returns: URL to open the style preview in browser -- **🔐 Secure Token Handling**: If `accessToken` is not provided, this tool attempts to use MCP **elicitation** to securely request a preview token without storing it in chat history. However, **elicitation support varies by client**: - - **MCP Inspector**: ✅ Full support - Shows secure form dialog with three options: +- **🔐 Secure Token Handling**: If `accessToken` is not provided, this tool attempts to use MCP **elicitation** to securely request a preview token without storing it in chat history. **Elicitation support varies by client**: + - **MCP Inspector, Cursor, VS Code**: ✅ Full support - Shows secure form dialog with three options: 1. **Provide an existing token** - Paste a token you already have 2. **Create a new preview token** - Create a new token with optional URL restrictions for enhanced security 3. **Auto-create a basic token** - Let the tool create a simple preview token for you From c974bcc0015c5f9600f5d3874dbd944154dae970 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 13 Jan 2026 11:37:25 -0500 Subject: [PATCH 07/25] Docs: Add Goose elicitation bug report and documentation Created comprehensive bug report for Goose's MCP elicitation timing issue where forms display after timeout instead of during tool execution. Added: - docs/goose-elicitation-bug-report.md - Detailed bug report for Goose team with reproduction steps, expected vs actual behavior, technical details, and suggested fix - Updated README to document Goose's known elicitation bug with link to bug report in both Quick Start and PreviewStyleTool sections Bug Summary: Goose advertises elicitation capability but displays forms after tool execution completes/times out, preventing user input. Co-Authored-By: Claude Sonnet 4.5 --- README.md | 2 + docs/goose-elicitation-bug-report.md | 177 +++++++++++++++++++++++++++ 2 files changed, 179 insertions(+) create mode 100644 docs/goose-elicitation-bug-report.md diff --git a/README.md b/README.md index 030f774..027825c 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,7 @@ Get started by integrating with your preferred AI development environment: - **MCP Inspector**: ✅ Full support - **Cursor**: ✅ Full support - **VS Code (with Copilot)**: ✅ Full support +- **Goose**: ⚠️ Known bug - Form displays after timeout ([bug report](./docs/goose-elicitation-bug-report.md)) - **Claude Desktop**: ⚠️ Not yet supported (Claude will fall back to creating tokens via chat) - **Claude Code**: Check for latest support status @@ -207,6 +208,7 @@ Complete set of tools for managing Mapbox styles via the Styles API: 1. **Provide an existing token** - Paste a token you already have 2. **Create a new preview token** - Create a new token with optional URL restrictions for enhanced security 3. **Auto-create a basic token** - Let the tool create a simple preview token for you + - **Goose**: ⚠️ Known bug - Form displays after timeout (see [bug report](./docs/goose-elicitation-bug-report.md)) - **Claude Desktop**: ⚠️ Not yet supported - When elicitation is unavailable, Claude will intelligently offer to create a token for you using `create_token_tool` (token will appear in chat history) - **Alternative**: Provide `accessToken` parameter directly for backward compatibility with any client - **Session Storage**: Your token choice is cached for the session, so you only need to provide it once (when elicitation is supported) diff --git a/docs/goose-elicitation-bug-report.md b/docs/goose-elicitation-bug-report.md new file mode 100644 index 0000000..d32434b --- /dev/null +++ b/docs/goose-elicitation-bug-report.md @@ -0,0 +1,177 @@ +# Goose MCP Elicitation Bug Report + +## Summary + +MCP elicitation forms display after tool execution timeout, preventing user input and making the elicitation feature unusable. + +## Environment + +- **Goose Version**: [Please specify] +- **MCP Server**: @mapbox/mcp-devkit-server v0.4.6 +- **MCP SDK Version**: @modelcontextprotocol/sdk v1.17.5 +- **Operating System**: macOS (confirmed), likely affects all platforms + +## Bug Description + +When an MCP tool calls `server.elicitInput()` to request user input, Goose advertises the `elicitation` capability but does not display the form in time for the user to interact with it. The form appears only **after** the tool call has timed out and completed, making elicitation unusable. + +## Steps to Reproduce + +1. Connect Goose to the Mapbox MCP DevKit Server +2. Call `preview_style_tool` without providing an `accessToken` parameter: + ``` + preview_style_tool({ styleId: "streets-v12" }) + ``` +3. Observe that: + - No elicitation form appears immediately + - Tool appears to hang/wait indefinitely + - After timeout period, tool fails or falls back + - **Then** the elicitation form appears in the UI + - Form is non-interactive/too late to provide input + +## Expected Behavior + +The elicitation form should: + +1. Appear **immediately** when `server.elicitInput()` is called +2. Block tool execution until user provides input or cancels +3. Allow user to interact with the form before any timeout +4. Return user input to the tool for processing + +This is how elicitation works correctly in: + +- MCP Inspector ✅ +- Cursor ✅ +- VS Code with GitHub Copilot ✅ + +## Actual Behavior + +The elicitation form: + +1. Does not appear when `server.elicitInput()` is called +2. Tool execution waits/hangs with no visible UI +3. Request times out after waiting period +4. Form appears **after** timeout in the UI +5. User never had opportunity to provide input +6. Creates misleading impression that elicitation is supported + +## Technical Details + +### Server-side code (working in other clients) + +```typescript +// Check if client supports elicitation capability +const clientCapabilities = this.server.server.getClientCapabilities(); +if (!clientCapabilities?.elicitation) { + return { + isError: true, + content: [{ type: 'text', text: 'Client does not support elicitation' }] + }; +} + +// Goose advertises elicitation capability, so this check passes ✅ + +// Attempt to elicit user input +const result = await server.elicitInput({ + message: 'Preview Token Setup...', + requestedSchema: { + type: 'object', + properties: { + choice: { + type: 'string', + enum: ['provide', 'create', 'auto'], + enumNames: [ + 'I have a token to provide', + 'Create a new preview token with custom settings', + 'Auto-create a basic preview token for me' + ] + }, + token: { type: 'string', minLength: 10 } + // ... other fields + }, + required: ['choice'] + } +}); + +// This await hangs indefinitely in Goose ❌ +// Form appears only after this times out +``` + +### What Goose advertises + +Goose correctly advertises elicitation capability during MCP handshake: + +```json +{ + "capabilities": { + "elicitation": {} + } +} +``` + +### Suspected Issue + +The elicitation form rendering appears to be: + +- Queued asynchronously rather than displayed synchronously +- Rendered after tool execution completes rather than during the `elicitInput()` call +- Not blocking the tool execution as required by MCP spec + +## Impact + +**High** - Renders MCP elicitation completely unusable in Goose: + +- Tools that require secure user input cannot function +- Users cannot use features designed to keep sensitive data out of chat history +- Creates poor UX with delayed/non-functional form + +## Workaround + +Users must provide sensitive parameters directly in tool calls: + +```typescript +preview_style_tool({ + styleId: 'streets-v12', + accessToken: 'pk.secret-token-in-chat-history' // Not ideal for security +}); +``` + +This defeats the purpose of elicitation (keeping tokens out of chat history). + +## References + +- MCP Elicitation Spec: https://modelcontextprotocol.io/specification/2025-11-25/client/elicitation +- MCP SDK elicitInput: https://github.com/modelcontextprotocol/sdk +- Issue discovered in PR: https://github.com/mapbox/mcp-devkit-server/pull/57 + +## Suggested Fix + +The elicitation form should be displayed **synchronously** when the server calls `elicitInput()`: + +1. Server sends elicitation request via MCP protocol +2. Goose immediately renders form UI (blocking) +3. User interacts with form +4. Form submission/cancellation returns to server +5. Tool execution continues with result + +The form render should **not** be queued or delayed until after tool completion. + +## Additional Context + +This bug was discovered while implementing secure token handling for the Mapbox MCP DevKit Server. The same code works perfectly in MCP Inspector, Cursor, and VS Code, suggesting the issue is specific to Goose's elicitation implementation. + +## Testing + +To verify a fix: + +1. Install @mapbox/mcp-devkit-server: `npx @modelcontextprotocol/create-server mapbox` +2. Configure with a Mapbox access token +3. Call `preview_style_tool` without `accessToken` parameter +4. Verify form appears **immediately** and accepts user input **before** timeout +5. Verify tool completes successfully with user-provided token + +--- + +**Report Date**: 2026-01-13 +**Reporter**: Mapbox MCP DevKit Server Team +**Goose Team**: Please let us know if you need any additional information or test cases! From ffdc83a34de596d170d8a3111e81d068bf0a9918 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 13 Jan 2026 11:47:06 -0500 Subject: [PATCH 08/25] Docs: Link to filed Goose elicitation bug issue Updated bug report and README to reference the filed GitHub issue: https://github.com/block/goose/issues/6471 This allows users and developers to track the bug status directly with the Goose team. Co-Authored-By: Claude Sonnet 4.5 --- README.md | 4 ++-- docs/goose-elicitation-bug-report.md | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 027825c..b922624 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ Get started by integrating with your preferred AI development environment: - **MCP Inspector**: ✅ Full support - **Cursor**: ✅ Full support - **VS Code (with Copilot)**: ✅ Full support -- **Goose**: ⚠️ Known bug - Form displays after timeout ([bug report](./docs/goose-elicitation-bug-report.md)) +- **Goose**: ⚠️ Known bug - Form displays after timeout ([goose#6471](https://github.com/block/goose/issues/6471)) - **Claude Desktop**: ⚠️ Not yet supported (Claude will fall back to creating tokens via chat) - **Claude Code**: Check for latest support status @@ -208,7 +208,7 @@ Complete set of tools for managing Mapbox styles via the Styles API: 1. **Provide an existing token** - Paste a token you already have 2. **Create a new preview token** - Create a new token with optional URL restrictions for enhanced security 3. **Auto-create a basic token** - Let the tool create a simple preview token for you - - **Goose**: ⚠️ Known bug - Form displays after timeout (see [bug report](./docs/goose-elicitation-bug-report.md)) + - **Goose**: ⚠️ Known bug - Form displays after timeout ([goose#6471](https://github.com/block/goose/issues/6471)) - **Claude Desktop**: ⚠️ Not yet supported - When elicitation is unavailable, Claude will intelligently offer to create a token for you using `create_token_tool` (token will appear in chat history) - **Alternative**: Provide `accessToken` parameter directly for backward compatibility with any client - **Session Storage**: Your token choice is cached for the session, so you only need to provide it once (when elicitation is supported) diff --git a/docs/goose-elicitation-bug-report.md b/docs/goose-elicitation-bug-report.md index d32434b..8aef87a 100644 --- a/docs/goose-elicitation-bug-report.md +++ b/docs/goose-elicitation-bug-report.md @@ -1,5 +1,7 @@ # Goose MCP Elicitation Bug Report +**Status**: 🐛 Filed - https://github.com/block/goose/issues/6471 + ## Summary MCP elicitation forms display after tool execution timeout, preventing user input and making the elicitation feature unusable. From 08b2460e51a4d895751d3d019c3335fef736da63 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 13 Jan 2026 11:49:15 -0500 Subject: [PATCH 09/25] Remove redundant Goose bug report file Bug is now tracked on GitHub at https://github.com/block/goose/issues/6471 No need to maintain a duplicate markdown file in the repo. Co-Authored-By: Claude Sonnet 4.5 --- docs/goose-elicitation-bug-report.md | 179 --------------------------- 1 file changed, 179 deletions(-) delete mode 100644 docs/goose-elicitation-bug-report.md diff --git a/docs/goose-elicitation-bug-report.md b/docs/goose-elicitation-bug-report.md deleted file mode 100644 index 8aef87a..0000000 --- a/docs/goose-elicitation-bug-report.md +++ /dev/null @@ -1,179 +0,0 @@ -# Goose MCP Elicitation Bug Report - -**Status**: 🐛 Filed - https://github.com/block/goose/issues/6471 - -## Summary - -MCP elicitation forms display after tool execution timeout, preventing user input and making the elicitation feature unusable. - -## Environment - -- **Goose Version**: [Please specify] -- **MCP Server**: @mapbox/mcp-devkit-server v0.4.6 -- **MCP SDK Version**: @modelcontextprotocol/sdk v1.17.5 -- **Operating System**: macOS (confirmed), likely affects all platforms - -## Bug Description - -When an MCP tool calls `server.elicitInput()` to request user input, Goose advertises the `elicitation` capability but does not display the form in time for the user to interact with it. The form appears only **after** the tool call has timed out and completed, making elicitation unusable. - -## Steps to Reproduce - -1. Connect Goose to the Mapbox MCP DevKit Server -2. Call `preview_style_tool` without providing an `accessToken` parameter: - ``` - preview_style_tool({ styleId: "streets-v12" }) - ``` -3. Observe that: - - No elicitation form appears immediately - - Tool appears to hang/wait indefinitely - - After timeout period, tool fails or falls back - - **Then** the elicitation form appears in the UI - - Form is non-interactive/too late to provide input - -## Expected Behavior - -The elicitation form should: - -1. Appear **immediately** when `server.elicitInput()` is called -2. Block tool execution until user provides input or cancels -3. Allow user to interact with the form before any timeout -4. Return user input to the tool for processing - -This is how elicitation works correctly in: - -- MCP Inspector ✅ -- Cursor ✅ -- VS Code with GitHub Copilot ✅ - -## Actual Behavior - -The elicitation form: - -1. Does not appear when `server.elicitInput()` is called -2. Tool execution waits/hangs with no visible UI -3. Request times out after waiting period -4. Form appears **after** timeout in the UI -5. User never had opportunity to provide input -6. Creates misleading impression that elicitation is supported - -## Technical Details - -### Server-side code (working in other clients) - -```typescript -// Check if client supports elicitation capability -const clientCapabilities = this.server.server.getClientCapabilities(); -if (!clientCapabilities?.elicitation) { - return { - isError: true, - content: [{ type: 'text', text: 'Client does not support elicitation' }] - }; -} - -// Goose advertises elicitation capability, so this check passes ✅ - -// Attempt to elicit user input -const result = await server.elicitInput({ - message: 'Preview Token Setup...', - requestedSchema: { - type: 'object', - properties: { - choice: { - type: 'string', - enum: ['provide', 'create', 'auto'], - enumNames: [ - 'I have a token to provide', - 'Create a new preview token with custom settings', - 'Auto-create a basic preview token for me' - ] - }, - token: { type: 'string', minLength: 10 } - // ... other fields - }, - required: ['choice'] - } -}); - -// This await hangs indefinitely in Goose ❌ -// Form appears only after this times out -``` - -### What Goose advertises - -Goose correctly advertises elicitation capability during MCP handshake: - -```json -{ - "capabilities": { - "elicitation": {} - } -} -``` - -### Suspected Issue - -The elicitation form rendering appears to be: - -- Queued asynchronously rather than displayed synchronously -- Rendered after tool execution completes rather than during the `elicitInput()` call -- Not blocking the tool execution as required by MCP spec - -## Impact - -**High** - Renders MCP elicitation completely unusable in Goose: - -- Tools that require secure user input cannot function -- Users cannot use features designed to keep sensitive data out of chat history -- Creates poor UX with delayed/non-functional form - -## Workaround - -Users must provide sensitive parameters directly in tool calls: - -```typescript -preview_style_tool({ - styleId: 'streets-v12', - accessToken: 'pk.secret-token-in-chat-history' // Not ideal for security -}); -``` - -This defeats the purpose of elicitation (keeping tokens out of chat history). - -## References - -- MCP Elicitation Spec: https://modelcontextprotocol.io/specification/2025-11-25/client/elicitation -- MCP SDK elicitInput: https://github.com/modelcontextprotocol/sdk -- Issue discovered in PR: https://github.com/mapbox/mcp-devkit-server/pull/57 - -## Suggested Fix - -The elicitation form should be displayed **synchronously** when the server calls `elicitInput()`: - -1. Server sends elicitation request via MCP protocol -2. Goose immediately renders form UI (blocking) -3. User interacts with form -4. Form submission/cancellation returns to server -5. Tool execution continues with result - -The form render should **not** be queued or delayed until after tool completion. - -## Additional Context - -This bug was discovered while implementing secure token handling for the Mapbox MCP DevKit Server. The same code works perfectly in MCP Inspector, Cursor, and VS Code, suggesting the issue is specific to Goose's elicitation implementation. - -## Testing - -To verify a fix: - -1. Install @mapbox/mcp-devkit-server: `npx @modelcontextprotocol/create-server mapbox` -2. Configure with a Mapbox access token -3. Call `preview_style_tool` without `accessToken` parameter -4. Verify form appears **immediately** and accepts user input **before** timeout -5. Verify tool completes successfully with user-provided token - ---- - -**Report Date**: 2026-01-13 -**Reporter**: Mapbox MCP DevKit Server Team -**Goose Team**: Please let us know if you need any additional information or test cases! From 3c6f8a629d5603255b0e53484726f8bd0de734cb Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 13 Jan 2026 14:53:44 -0500 Subject: [PATCH 10/25] Tests: Add unit tests for elicitation and token storage Added comprehensive test coverage for the new elicitation features: Token Storage Tests (test/utils/tokenElicitation.test.ts): - Store and retrieve tokens by username - Return undefined for non-existent username - Overwrite existing tokens - Store tokens for multiple users independently - Clear specific username token - Clear all tokens - Handle edge cases (empty string, special characters) PreviewStyleTool Elicitation Tests: - Error when no accessToken and no server token - Backward compatibility when accessToken provided directly Test Results: All 527 tests pass (12 new tests added) These tests ensure the elicitation feature works correctly and maintains backward compatibility with existing usage patterns. Co-Authored-By: Claude Sonnet 4.5 --- .../PreviewStyleTool.test.ts | 44 ++++++++++ test/utils/tokenElicitation.test.ts | 85 +++++++++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 test/utils/tokenElicitation.test.ts diff --git a/test/tools/preview-style-tool/PreviewStyleTool.test.ts b/test/tools/preview-style-tool/PreviewStyleTool.test.ts index e8315c0..32c6793 100644 --- a/test/tools/preview-style-tool/PreviewStyleTool.test.ts +++ b/test/tools/preview-style-tool/PreviewStyleTool.test.ts @@ -195,4 +195,48 @@ describe('PreviewStyleTool', () => { // Clean up delete process.env.ENABLE_MCP_UI; }); + + describe('elicitation behavior', () => { + it('returns error when no accessToken and no valid server token', async () => { + const tool = new PreviewStyleTool(); + + // Remove env var temporarily to test error path + const oldToken = process.env.MAPBOX_ACCESS_TOKEN; + delete process.env.MAPBOX_ACCESS_TOKEN; + + const result = await tool.run({ + styleId: 'test-style' + // No accessToken, no authInfo.token either + }); + + expect(result.isError).toBe(true); + expect(result.content[0]).toMatchObject({ + type: 'text', + text: expect.stringContaining( + 'Server access token is required when no preview token is provided' + ) + }); + + // Restore env var + process.env.MAPBOX_ACCESS_TOKEN = oldToken; + }); + + it('works with backward compatibility when accessToken is provided', async () => { + const tool = new PreviewStyleTool(); + // Even without server initialization, providing accessToken directly should work + + const result = await tool.run({ + styleId: 'test-style', + accessToken: TEST_ACCESS_TOKEN + }); + + expect(result.isError).toBe(false); + expect(result.content[0]).toMatchObject({ + type: 'text', + text: expect.stringContaining( + '/styles/v1/test-user/test-style.html?access_token=pk.' + ) + }); + }); + }); }); diff --git a/test/utils/tokenElicitation.test.ts b/test/utils/tokenElicitation.test.ts new file mode 100644 index 0000000..dec2f92 --- /dev/null +++ b/test/utils/tokenElicitation.test.ts @@ -0,0 +1,85 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect, beforeEach } from 'vitest'; +import { previewTokenStorage } from '../../src/utils/tokenElicitation.js'; + +describe('PreviewTokenStorage', () => { + // Clean up before each test to ensure isolation + beforeEach(() => { + previewTokenStorage.clearAll(); + }); + + it('stores and retrieves tokens by username', () => { + previewTokenStorage.set('test-user', 'pk.test-token-123'); + expect(previewTokenStorage.get('test-user')).toBe('pk.test-token-123'); + }); + + it('returns undefined for non-existent username', () => { + expect(previewTokenStorage.get('non-existent-user')).toBeUndefined(); + }); + + it('overwrites existing token for same username', () => { + previewTokenStorage.set('test-user', 'pk.old-token'); + previewTokenStorage.set('test-user', 'pk.new-token'); + expect(previewTokenStorage.get('test-user')).toBe('pk.new-token'); + }); + + it('stores tokens for multiple users independently', () => { + previewTokenStorage.set('user1', 'pk.token1'); + previewTokenStorage.set('user2', 'pk.token2'); + previewTokenStorage.set('user3', 'pk.token3'); + + expect(previewTokenStorage.get('user1')).toBe('pk.token1'); + expect(previewTokenStorage.get('user2')).toBe('pk.token2'); + expect(previewTokenStorage.get('user3')).toBe('pk.token3'); + }); + + it('clears specific username token', () => { + previewTokenStorage.set('user1', 'pk.token1'); + previewTokenStorage.set('user2', 'pk.token2'); + + previewTokenStorage.clear('user1'); + + expect(previewTokenStorage.get('user1')).toBeUndefined(); + expect(previewTokenStorage.get('user2')).toBe('pk.token2'); // Other token unaffected + }); + + it('clearing non-existent username does not throw', () => { + expect(() => { + previewTokenStorage.clear('non-existent-user'); + }).not.toThrow(); + }); + + it('clears all tokens', () => { + previewTokenStorage.set('user1', 'pk.token1'); + previewTokenStorage.set('user2', 'pk.token2'); + previewTokenStorage.set('user3', 'pk.token3'); + + previewTokenStorage.clearAll(); + + expect(previewTokenStorage.get('user1')).toBeUndefined(); + expect(previewTokenStorage.get('user2')).toBeUndefined(); + expect(previewTokenStorage.get('user3')).toBeUndefined(); + }); + + it('works correctly after clearAll and new sets', () => { + previewTokenStorage.set('user1', 'pk.old-token'); + previewTokenStorage.clearAll(); + previewTokenStorage.set('user2', 'pk.new-token'); + + expect(previewTokenStorage.get('user1')).toBeUndefined(); + expect(previewTokenStorage.get('user2')).toBe('pk.new-token'); + }); + + it('handles empty string username', () => { + previewTokenStorage.set('', 'pk.empty-user-token'); + expect(previewTokenStorage.get('')).toBe('pk.empty-user-token'); + }); + + it('handles special characters in username', () => { + const specialUsername = 'user@example.com'; + previewTokenStorage.set(specialUsername, 'pk.special-token'); + expect(previewTokenStorage.get(specialUsername)).toBe('pk.special-token'); + }); +}); From 90cbd1b48176a68a9d4fbcdf736a1423ef14d97d Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Wed, 14 Jan 2026 11:11:16 -0500 Subject: [PATCH 11/25] Docs: Confirm Claude Code does not support elicitation yet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tested preview_style_tool directly via MCP and confirmed that Claude Code does not advertise elicitation capability. The tool correctly returns the error message we designed for clients without elicitation support. Updated README to reflect: - Claude Code: ⚠️ Not yet supported (provide accessToken directly) - Grouped with Claude Desktop in the "not yet supported" category This was confirmed by calling the tool through the registered MCP server and observing the capability check work as expected. Co-Authored-By: Claude Sonnet 4.5 --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b922624..27b2377 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ Get started by integrating with your preferred AI development environment: - **VS Code (with Copilot)**: ✅ Full support - **Goose**: ⚠️ Known bug - Form displays after timeout ([goose#6471](https://github.com/block/goose/issues/6471)) - **Claude Desktop**: ⚠️ Not yet supported (Claude will fall back to creating tokens via chat) -- **Claude Code**: Check for latest support status +- **Claude Code**: ⚠️ Not yet supported (provide `accessToken` parameter directly) ### DXT Package Distribution @@ -209,7 +209,7 @@ Complete set of tools for managing Mapbox styles via the Styles API: 2. **Create a new preview token** - Create a new token with optional URL restrictions for enhanced security 3. **Auto-create a basic token** - Let the tool create a simple preview token for you - **Goose**: ⚠️ Known bug - Form displays after timeout ([goose#6471](https://github.com/block/goose/issues/6471)) - - **Claude Desktop**: ⚠️ Not yet supported - When elicitation is unavailable, Claude will intelligently offer to create a token for you using `create_token_tool` (token will appear in chat history) + - **Claude Desktop, Claude Code**: ⚠️ Not yet supported - Provide `accessToken` parameter directly, or Claude will intelligently offer to create a token for you using `create_token_tool` (token will appear in chat history) - **Alternative**: Provide `accessToken` parameter directly for backward compatibility with any client - **Session Storage**: Your token choice is cached for the session, so you only need to provide it once (when elicitation is supported) - **Best Practice**: Use URL-restricted tokens to limit token usage to specific domains From 1e3e9e7f0f3cc5342179db4539df7b27bbfdf4cc Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Thu, 15 Jan 2026 12:41:12 -0500 Subject: [PATCH 12/25] Add elicitation support to style_comparison_tool - Made accessToken optional and added useCustomToken parameter - Integrated elicitation flow with capability checks - Added token creation/listing methods with minimal public scopes - Session caching via shared previewTokenStorage - Added 2 elicitation behavior tests - Updated README with security-focused documentation - All 529 tests pass --- README.md | 33 ++- .../StyleComparisonTool.schema.ts | 10 +- .../StyleComparisonTool.ts | 212 +++++++++++++++++- .../StyleComparisonTool.test.ts | 59 ++++- 4 files changed, 296 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 27b2377..2739578 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ Get started by integrating with your preferred AI development environment: - [Cursor Integration](./docs/cursor-integration.md) - Cursor IDE integration - [VS Code Integration](./docs/vscode-integration.md) - Visual Studio Code with GitHub Copilot -**Note on MCP Elicitation Support**: Some tools (like `preview_style_tool`) use [MCP elicitation](https://modelcontextprotocol.io/specification/2025-11-25/client/elicitation) to securely request tokens without exposing them in chat history. Elicitation support varies by client: +**Note on MCP Elicitation Support**: Some tools (like `preview_style_tool` and `style_comparison_tool`) use [MCP elicitation](https://modelcontextprotocol.io/specification/2025-11-25/client/elicitation) to provide secure token management following the principle of least privilege. Elicitation ensures that only minimal-scope public tokens (pk._) appear in preview URLs, while your powerful server token (sk._) stays secure. This guided workflow also improves UX for token selection and creation. Elicitation support varies by client: - **MCP Inspector**: ✅ Full support - **Cursor**: ✅ Full support @@ -203,16 +203,38 @@ Complete set of tools for managing Mapbox styles via the Styles API: - `title` (optional): Show title in preview - `zoomwheel` (optional): Enable zoom wheel control - Returns: URL to open the style preview in browser -- **🔐 Secure Token Handling**: If `accessToken` is not provided, this tool attempts to use MCP **elicitation** to securely request a preview token without storing it in chat history. **Elicitation support varies by client**: - - **MCP Inspector, Cursor, VS Code**: ✅ Full support - Shows secure form dialog with three options: +- **🔐 Secure Token Management**: If `accessToken` is not provided, this tool uses MCP **elicitation** to create minimal-scope public tokens (pk._) instead of exposing your powerful server token. This follows the **principle of least privilege** - preview/comparison URLs only contain read-only tokens (styles:read, styles:tiles, fonts:read), keeping your server token (sk._) with write permissions secure. **Elicitation support varies by client**: + - **MCP Inspector, Cursor, VS Code**: ✅ Full support - Shows guided form dialog with three options: 1. **Provide an existing token** - Paste a token you already have 2. **Create a new preview token** - Create a new token with optional URL restrictions for enhanced security 3. **Auto-create a basic token** - Let the tool create a simple preview token for you - **Goose**: ⚠️ Known bug - Form displays after timeout ([goose#6471](https://github.com/block/goose/issues/6471)) - - **Claude Desktop, Claude Code**: ⚠️ Not yet supported - Provide `accessToken` parameter directly, or Claude will intelligently offer to create a token for you using `create_token_tool` (token will appear in chat history) + - **Claude Desktop, Claude Code**: ⚠️ Not yet supported - Provide `accessToken` parameter directly, or Claude will intelligently offer to create a token for you using `create_token_tool` - **Alternative**: Provide `accessToken` parameter directly for backward compatibility with any client - **Session Storage**: Your token choice is cached for the session, so you only need to provide it once (when elicitation is supported) -- **Best Practice**: Use URL-restricted tokens to limit token usage to specific domains +- **Best Practice**: Use URL-restricted tokens to further limit token usage to specific domains. While public tokens in URLs are read-only, URL restrictions add an extra layer of security by ensuring tokens only work on your specified domains + +**StyleComparisonTool** - Generate side-by-side comparison URL for two Mapbox styles + +- Input: + - `before` (required): Mapbox style for the "before" side (accepts full style URL, username/styleId format, or just styleId) + - `after` (required): Mapbox style for the "after" side (accepts full style URL, username/styleId format, or just styleId) + - `accessToken` (optional): Provide a specific public token (for backward compatibility) + - `useCustomToken` (optional): Force token selection dialog even if a token is cached + - `zoom` (optional): Initial zoom level (0-22) + - `latitude` (optional): Latitude coordinate for initial map center (-90 to 90) + - `longitude` (optional): Longitude coordinate for initial map center (-180 to 180) +- Returns: URL to open the side-by-side style comparison in browser +- **🔐 Secure Token Management**: If `accessToken` is not provided, this tool uses MCP **elicitation** to create minimal-scope public tokens (pk._) instead of exposing your powerful server token. This follows the **principle of least privilege** - preview/comparison URLs only contain read-only tokens (styles:read, styles:tiles, fonts:read), keeping your server token (sk._) with write permissions secure. **Elicitation support varies by client**: + - **MCP Inspector, Cursor, VS Code**: ✅ Full support - Shows guided form dialog with three options: + 1. **Provide an existing token** - Paste a token you already have + 2. **Create a new preview token** - Create a new token with optional URL restrictions for enhanced security + 3. **Auto-create a basic token** - Let the tool create a simple preview token for you + - **Goose**: ⚠️ Known bug - Form displays after timeout ([goose#6471](https://github.com/block/goose/issues/6471)) + - **Claude Desktop, Claude Code**: ⚠️ Not yet supported - Provide `accessToken` parameter directly, or Claude will intelligently offer to create a token for you using `create_token_tool` + - **Alternative**: Provide `accessToken` parameter directly for backward compatibility with any client +- **Session Storage**: Your token choice is cached for the session, so you only need to provide it once (when elicitation is supported) +- **Best Practice**: Use URL-restricted tokens to further limit token usage to specific domains. While public tokens in URLs are read-only, URL restrictions add an extra layer of security by ensuring tokens only work on your specified domains **ValidateStyleTool** - Validate Mapbox style JSON against the Mapbox Style Specification @@ -235,6 +257,7 @@ Complete set of tools for managing Mapbox styles via the Styles API: - **UpdateStyleTool**: Requires `styles:write` scope - **DeleteStyleTool**: Requires `styles:write` scope - **PreviewStyleTool**: Can work without token scopes via elicitation, or optionally accepts a direct public token. If using automatic token listing, requires `tokens:read` scope +- **StyleComparisonTool**: Can work without token scopes via elicitation, or optionally accepts a direct public token. If using automatic token listing, requires `tokens:read` scope **Note:** The username is automatically extracted from the JWT token payload. diff --git a/src/tools/style-comparison-tool/StyleComparisonTool.schema.ts b/src/tools/style-comparison-tool/StyleComparisonTool.schema.ts index ec25e25..1183589 100644 --- a/src/tools/style-comparison-tool/StyleComparisonTool.schema.ts +++ b/src/tools/style-comparison-tool/StyleComparisonTool.schema.ts @@ -20,8 +20,16 @@ export const StyleComparisonSchema = z.object({ 'pk.', 'Invalid token type. Style comparison requires a public token (pk.*) that can be used in browser URLs. Secret tokens (sk.*) cannot be exposed in client-side applications. Please provide a public token with styles:read permission.' ) + .optional() + .describe( + 'Mapbox public access token (optional). If not provided, you will be prompted to provide, create, or auto-create a preview token via MCP elicitation (supported in MCP Inspector, Cursor, VS Code). For clients without elicitation support (Claude Desktop, Claude Code), provide this parameter directly. Must start with pk.* and have styles:read permission. Secret tokens (sk.*) cannot be used as they cannot be exposed in browser URLs.' + ), + useCustomToken: z + .boolean() + .optional() + .default(false) .describe( - 'Mapbox public access token (required, must start with pk.* and have styles:read permission). Secret tokens (sk.*) cannot be used as they cannot be exposed in browser URLs. Please use a public token or create one with styles:read permission.' + 'Force token selection dialog even if a preview token is already stored for this session. Useful when you want to use a different token.' ), zoom: z .number() diff --git a/src/tools/style-comparison-tool/StyleComparisonTool.ts b/src/tools/style-comparison-tool/StyleComparisonTool.ts index f9ce238..6fbe9aa 100644 --- a/src/tools/style-comparison-tool/StyleComparisonTool.ts +++ b/src/tools/style-comparison-tool/StyleComparisonTool.ts @@ -10,6 +10,11 @@ import { } from './StyleComparisonTool.schema.js'; import { getUserNameFromToken } from '../../utils/jwtUtils.js'; import { isMcpUiEnabled } from '../../config/toolConfig.js'; +import { + elicitPreviewToken, + previewTokenStorage +} from '../../utils/tokenElicitation.js'; +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol.js'; export class StyleComparisonTool extends BaseTool< typeof StyleComparisonSchema @@ -29,6 +34,122 @@ export class StyleComparisonTool extends BaseTool< super({ inputSchema: StyleComparisonSchema }); } + /** + * Override run to handle elicitation via RequestHandlerExtra + */ + async run( + rawInput: unknown, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + extra?: RequestHandlerExtra + ): Promise { + try { + const input = this.inputSchema.parse(rawInput); + const serverAccessToken = + extra?.authInfo?.token || process.env.MAPBOX_ACCESS_TOKEN; + + // Validate server token exists + if (!serverAccessToken) { + return { + isError: true, + content: [ + { + type: 'text', + text: 'Server access token is required when no preview token is provided. Please configure MAPBOX_ACCESS_TOKEN environment variable.' + } + ] + }; + } + + return this.execute(input, serverAccessToken); + } catch (error) { + return { + isError: true, + content: [{ type: 'text', text: (error as Error).message }] + }; + } + } + + /** + * List existing public tokens for elicitation + */ + private async listPublicTokens( + serverAccessToken?: string + ): Promise<{ id: string; note: string; scopes: string[] }[]> { + if (!serverAccessToken) return []; + + try { + const response = await fetch( + 'https://api.mapbox.com/tokens/v2?limit=100&usage=pk', + { + headers: { + Authorization: `Bearer ${serverAccessToken}` + } + } + ); + + if (!response.ok) return []; + + const data = (await response.json()) as Array<{ + id: string; + note: string; + scopes: string[]; + }>; + return data.map((token) => ({ + id: token.id, + note: token.note || 'Unnamed token', + scopes: token.scopes + })); + } catch { + return []; + } + } + + /** + * Create a new public preview token + */ + private async createPreviewToken( + serverAccessToken?: string, + tokenNote?: string, + urlRestrictions?: string[] + ): Promise<{ token: string }> { + if (!serverAccessToken) { + throw new Error('Server access token required to create preview tokens'); + } + + const body: { + note: string; + scopes: string[]; + allowedUrls?: string[]; + } = { + note: tokenNote || 'Auto-created preview token', + // CRITICAL: Only use public scopes to get a public token (pk.*) + // styles:download is a secret scope and would create sk.* token + scopes: ['styles:read', 'styles:tiles', 'fonts:read'] + }; + + // Add URL restrictions if provided + if (urlRestrictions && urlRestrictions.length > 0) { + body.allowedUrls = urlRestrictions; + } + + const response = await fetch('https://api.mapbox.com/tokens/v2', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${serverAccessToken}` + }, + body: JSON.stringify(body) + }); + + if (!response.ok) { + const error = await response.text(); + throw new Error(`Failed to create preview token: ${error}`); + } + + const data = (await response.json()) as { token: string }; + return { token: data.token }; + } + /** * Processes style input to extract username/styleId format */ @@ -59,14 +180,97 @@ export class StyleComparisonTool extends BaseTool< } protected async execute( - input: StyleComparisonInput + input: StyleComparisonInput, + serverAccessToken?: string ): Promise { + // Handle token elicitation if accessToken not provided + let publicToken: string; + + if (input.accessToken) { + // Backward compatibility - use provided token directly + publicToken = input.accessToken; + } else { + // Need to elicit token from user + const userName = getUserNameFromToken(serverAccessToken || ''); + const storedToken = previewTokenStorage.get(userName); + + if (storedToken && !input.useCustomToken) { + // Use cached token + publicToken = storedToken; + } else { + // Check if client supports elicitation + if (!this.server?.server) { + return { + isError: true, + content: [ + { + type: 'text', + text: 'Server not initialized. Cannot use elicitation.' + } + ] + }; + } + + const clientCapabilities = this.server.server.getClientCapabilities(); + if (!clientCapabilities?.elicitation) { + return { + isError: true, + content: [ + { + type: 'text', + text: + 'Preview token required but client does not support elicitation. ' + + 'Please provide an accessToken parameter directly, or use a client that supports ' + + 'MCP elicitation (MCP Inspector, Cursor, VS Code).' + } + ] + }; + } + + // Elicit from user + try { + const existingTokens = await this.listPublicTokens(serverAccessToken); + const elicited = await elicitPreviewToken( + this.server.server, + existingTokens + ); + + if (elicited.choice === 'provide') { + publicToken = elicited.token!; + } else if (elicited.choice === 'create') { + const created = await this.createPreviewToken( + serverAccessToken, + elicited.tokenNote, + elicited.urlRestrictions + ); + publicToken = created.token!; + } else { + // auto-create + const created = await this.createPreviewToken(serverAccessToken); + publicToken = created.token!; + } + + // Cache the token for this session + previewTokenStorage.set(userName, publicToken); + } catch (error) { + return { + isError: true, + content: [ + { + type: 'text', + text: `Failed to elicit or create preview token: ${error instanceof Error ? error.message : 'Unknown error'}` + } + ] + }; + } + } + } let beforeStyleId; let afterStyleId; try { // Process style IDs to get username/styleId format - beforeStyleId = this.processStyleId(input.before, input.accessToken); - afterStyleId = this.processStyleId(input.after, input.accessToken); + beforeStyleId = this.processStyleId(input.before, publicToken); + afterStyleId = this.processStyleId(input.after, publicToken); } catch (error) { return { content: [ @@ -84,7 +288,7 @@ export class StyleComparisonTool extends BaseTool< // Build the comparison URL const params = new URLSearchParams(); - params.append('access_token', input.accessToken); + params.append('access_token', publicToken); params.append('before', beforeStyleId); params.append('after', afterStyleId); diff --git a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts index b90ecfa..a7a9b72 100644 --- a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts +++ b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts @@ -50,19 +50,18 @@ describe('StyleComparisonTool', () => { }); }); - it('should require access token', async () => { + it('should work with provided access token (backward compatibility)', async () => { const input = { before: 'mapbox/streets-v12', - after: 'mapbox/satellite-v9' - // Missing accessToken - } as any; + after: 'mapbox/satellite-v9', + accessToken: 'pk.test.token' + }; const result = await tool.run(input); - expect(result.isError).toBe(true); - expect( - (result.content[0] as { type: 'text'; text: string }).text - ).toContain('Required'); + expect(result.isError).toBe(false); + const url = (result.content[0] as { type: 'text'; text: string }).text; + expect(url).toContain('access_token=pk.test.token'); }); it('should handle full style URLs', async () => { @@ -235,6 +234,50 @@ describe('StyleComparisonTool', () => { }); }); + describe('elicitation behavior', () => { + it('returns error when no accessToken and no valid server token', async () => { + const tool = new StyleComparisonTool(); + + // Remove env var temporarily to test error path + const oldToken = process.env.MAPBOX_ACCESS_TOKEN; + delete process.env.MAPBOX_ACCESS_TOKEN; + + const result = await tool.run({ + before: 'mapbox/streets-v12', + after: 'mapbox/satellite-v9' + // No accessToken, no authInfo.token either + }); + + expect(result.isError).toBe(true); + expect(result.content[0]).toMatchObject({ + type: 'text', + text: expect.stringContaining( + 'Server access token is required when no preview token is provided' + ) + }); + + // Restore env var + process.env.MAPBOX_ACCESS_TOKEN = oldToken; + }); + + it('works with backward compatibility when accessToken is provided', async () => { + const tool = new StyleComparisonTool(); + // Even without server initialization, providing accessToken directly should work + + const result = await tool.run({ + before: 'mapbox/streets-v12', + after: 'mapbox/satellite-v9', + accessToken: 'pk.test.token' + }); + + expect(result.isError).toBe(false); + expect(result.content[0]).toMatchObject({ + type: 'text', + text: expect.stringContaining('access_token=pk.test.token') + }); + }); + }); + describe('metadata', () => { it('should have correct name and description', () => { expect(tool.name).toBe('style_comparison_tool'); From 19d363770609cdbc682ef17e951a4ed9dfd6018a Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Fri, 31 Jul 2026 10:08:25 -0400 Subject: [PATCH 13/25] Add tk.* guard, HttpPipeline DI, and hosted-endpoint docs for token elicitation - Reject token creation up front when the server's access token is a temporary tk.* token (used by the hosted MCP endpoint), instead of letting the Mapbox API round-trip fail. The elicitation dialog now omits "create"/"auto-create" in that case and only offers "provide an existing token". - Move token-listing/creation off raw fetch() onto the shared HttpPipeline (constructor-injected httpRequest), consistent with other Mapbox API tools. - Document the hosted-endpoint limitation in README and CHANGELOG. --- CHANGELOG.md | 9 +++++++++ README.md | 11 +++++++++++ src/utils/tokenElicitation.ts | 6 +++--- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4611d3..3764769 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ ## Unreleased +### New Features + +- **Secure token elicitation for `preview_style_tool` / `style_comparison_tool`** (#57): `accessToken` is now optional on both tools. When omitted, the tool uses MCP elicitation to ask for a public token instead — either pasting an existing `pk.*` token, creating a new one with optional URL restrictions, or auto-creating a minimally-scoped one (`styles:read`, `styles:tiles`, `fonts:read`). This keeps your server's `sk.*`/`pk.*` access token out of chat history and preview URLs. Falls back to requiring `accessToken` directly on clients without elicitation support (Claude Desktop, Claude Code). A chosen/created token is cached in memory per account for the session; pass `useCustomToken: true` to force re-selection. + - On servers authenticated with a temporary `tk.*` token — notably the hosted MCP endpoint — Mapbox's Tokens API cannot create new tokens, so the "create a new token" and "auto-create" options are automatically omitted from the elicitation dialog rather than being offered and failing. + +### Changed + +- **`preview_style_tool` / `style_comparison_tool`**: token-listing and token-creation HTTP calls now go through the shared `HttpPipeline` (constructor-injected `httpRequest`) instead of a bare `fetch`, consistent with the rest of the API-calling tools. + ## 0.8.2 - 2026-07-30 ### Fixed diff --git a/README.md b/README.md index d228100..e7df2e6 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,8 @@ Get started by integrating with your preferred AI development environment: - **Claude Desktop**: ⚠️ Not yet supported (Claude will fall back to creating tokens via chat) - **Claude Code**: ⚠️ Not yet supported (provide `accessToken` parameter directly) +**Note on the hosted MCP endpoint**: even on a client with full elicitation support, the [hosted endpoint](#hosted-mcp-endpoint) only offers **"I have a token to provide"** — see below for why. + ### DXT Package Distribution This MCP server can be packaged as a DXT (Desktop Extension) file for easy distribution and installation. DXT is a standardized format for distributing local MCP servers, similar to browser extensions. @@ -105,6 +107,13 @@ For quick access, you can use our hosted MCP endpoint: For detailed setup instructions for different clients and API usage, see the [Hosted MCP Server Guide](https://github.com/mapbox/mcp-server/blob/main/docs/hosted-mcp-guide.md). Note: This guide references the standard MCP endpoint - you'll need to update the endpoint URL to use the devkit endpoint above. +**Token creation is unavailable on the hosted endpoint**: the hosted server authenticates each request with a short-lived, per-session token (`tk.*`), not your own Mapbox account token. Mapbox's Tokens API never grants `tokens:write` to a `tk.*` token, so it cannot be used to create new tokens. As a result, on the hosted endpoint: + +- `preview_style_tool` / `style_comparison_tool`'s elicitation dialog only offers **"I have a token to provide"** — the "create a new token" and "auto-create" options are hidden automatically, rather than being offered and then failing. +- `create_token_tool` will fail with a permissions error if called directly. + +Paste an existing public token (`pk.*`, with `styles:read` scope) when prompted, or create one ahead of time from your [Mapbox Account page](https://account.mapbox.com/). Running this server **locally** with your own `pk.*`/`sk.*` access token (which can carry `tokens:write`) restores all three elicitation options, including create and auto-create. + ### Getting Your Mapbox Access Token **A Mapbox access token is required to use this MCP server.** @@ -194,6 +203,7 @@ Complete set of tools for managing Mapbox styles via the Styles API: 3. **Auto-create a basic token** - Let the tool create a simple preview token for you - **Goose**: ⚠️ Known bug - Form displays after timeout ([goose#6471](https://github.com/block/goose/issues/6471)) - **Claude Desktop, Claude Code**: ⚠️ Not yet supported - Provide `accessToken` parameter directly, or Claude will intelligently offer to create a token for you using `create_token_tool` + - **Hosted MCP endpoint**: ⚠️ Only "provide an existing token" is offered, regardless of client — see [Token creation is unavailable on the hosted endpoint](#hosted-mcp-endpoint) - **Alternative**: Provide `accessToken` parameter directly for backward compatibility with any client - **Session Storage**: Your token choice is cached for the session, so you only need to provide it once (when elicitation is supported) - **Best Practice**: Use URL-restricted tokens to further limit token usage to specific domains. While public tokens in URLs are read-only, URL restrictions add an extra layer of security by ensuring tokens only work on your specified domains @@ -216,6 +226,7 @@ Complete set of tools for managing Mapbox styles via the Styles API: 3. **Auto-create a basic token** - Let the tool create a simple preview token for you - **Goose**: ⚠️ Known bug - Form displays after timeout ([goose#6471](https://github.com/block/goose/issues/6471)) - **Claude Desktop, Claude Code**: ⚠️ Not yet supported - Provide `accessToken` parameter directly, or Claude will intelligently offer to create a token for you using `create_token_tool` + - **Hosted MCP endpoint**: ⚠️ Only "provide an existing token" is offered, regardless of client — see [Token creation is unavailable on the hosted endpoint](#hosted-mcp-endpoint) - **Alternative**: Provide `accessToken` parameter directly for backward compatibility with any client - **Session Storage**: Your token choice is cached for the session, so you only need to provide it once (when elicitation is supported) - **Best Practice**: Use URL-restricted tokens to further limit token usage to specific domains. While public tokens in URLs are read-only, URL restrictions add an extra layer of security by ensuring tokens only work on your specified domains diff --git a/src/utils/tokenElicitation.ts b/src/utils/tokenElicitation.ts index c63dbe9..23d7357 100644 --- a/src/utils/tokenElicitation.ts +++ b/src/utils/tokenElicitation.ts @@ -75,9 +75,9 @@ export async function elicitPreviewToken( .join('\n') : 'No existing public tokens found.'; - const choices = canCreateTokens - ? (['provide', 'create', 'auto'] as const) - : (['provide'] as const); + const choices: TokenChoice[] = canCreateTokens + ? ['provide', 'create', 'auto'] + : ['provide']; const choiceNames = canCreateTokens ? [ 'I have a token to provide', From 8f9fcf26a3f03a38f0ad0455b34a46e1d651ffbc Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Fri, 31 Jul 2026 10:30:17 -0400 Subject: [PATCH 14/25] Correct tk.* guard claims and improve the actual create-token failure message The tk.* prefix check only catches a literal Mapbox temporary token supplied directly (e.g. MAPBOX_ACCESS_TOKEN=tk...). It does not detect the hosted MCP endpoint's lack of tokens:write: that deployment passes through its own access token, which isn't tk.*-shaped, so the guard never fires there. Corrected the doc comments, README, and CHANGELOG, which previously stated this as a general fact about the hosted endpoint's token shape. Since the guard can't see that case, createPreviewToken() now appends a scope/permission hint to whatever error the Tokens API returns on a 401/403 (or a message containing "scope"/"permission"), steering back to "provide an existing token" instead of leaving the caller to interpret a bare API error. --- CHANGELOG.md | 2 +- README.md | 14 ++++---- src/utils/tokenElicitation.ts | 53 ++++++++++++++++++++++------- test/utils/tokenElicitation.test.ts | 22 +++++++++++- 4 files changed, 70 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3764769..5d0aaee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ### New Features - **Secure token elicitation for `preview_style_tool` / `style_comparison_tool`** (#57): `accessToken` is now optional on both tools. When omitted, the tool uses MCP elicitation to ask for a public token instead — either pasting an existing `pk.*` token, creating a new one with optional URL restrictions, or auto-creating a minimally-scoped one (`styles:read`, `styles:tiles`, `fonts:read`). This keeps your server's `sk.*`/`pk.*` access token out of chat history and preview URLs. Falls back to requiring `accessToken` directly on clients without elicitation support (Claude Desktop, Claude Code). A chosen/created token is cached in memory per account for the session; pass `useCustomToken: true` to force re-selection. - - On servers authenticated with a temporary `tk.*` token — notably the hosted MCP endpoint — Mapbox's Tokens API cannot create new tokens, so the "create a new token" and "auto-create" options are automatically omitted from the elicitation dialog rather than being offered and failing. + - If the server's own access token is a literal Mapbox temporary token (`tk.*`), the "create a new token" and "auto-create" options are omitted from the dialog up front, since that token shape is guaranteed to lack `tokens:write`. Other callers lacking `tokens:write` (e.g. the hosted MCP endpoint, which authenticates with its own access token rather than a Mapbox `pk.*`/`sk.*`/`tk.*` token) aren't detectable ahead of time; for those, choosing "create"/"auto-create" fails against the Tokens API with a scope/permission hint appended to the error, steering back to "I have a token to provide". ### Changed diff --git a/README.md b/README.md index e7df2e6..dce7279 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ Get started by integrating with your preferred AI development environment: - **Claude Desktop**: ⚠️ Not yet supported (Claude will fall back to creating tokens via chat) - **Claude Code**: ⚠️ Not yet supported (provide `accessToken` parameter directly) -**Note on the hosted MCP endpoint**: even on a client with full elicitation support, the [hosted endpoint](#hosted-mcp-endpoint) only offers **"I have a token to provide"** — see below for why. +**Note on the hosted MCP endpoint**: even on a client with full elicitation support, "create a new token" and "auto-create" will fail on the [hosted endpoint](#hosted-mcp-endpoint) — see below for why. ### DXT Package Distribution @@ -107,12 +107,12 @@ For quick access, you can use our hosted MCP endpoint: For detailed setup instructions for different clients and API usage, see the [Hosted MCP Server Guide](https://github.com/mapbox/mcp-server/blob/main/docs/hosted-mcp-guide.md). Note: This guide references the standard MCP endpoint - you'll need to update the endpoint URL to use the devkit endpoint above. -**Token creation is unavailable on the hosted endpoint**: the hosted server authenticates each request with a short-lived, per-session token (`tk.*`), not your own Mapbox account token. Mapbox's Tokens API never grants `tokens:write` to a `tk.*` token, so it cannot be used to create new tokens. As a result, on the hosted endpoint: +**Token creation is unavailable on the hosted endpoint**: the hosted deployment authenticates each request with its own access token rather than your personal Mapbox account token, and that token is not granted `tokens:write`. As a result, on the hosted endpoint: -- `preview_style_tool` / `style_comparison_tool`'s elicitation dialog only offers **"I have a token to provide"** — the "create a new token" and "auto-create" options are hidden automatically, rather than being offered and then failing. -- `create_token_tool` will fail with a permissions error if called directly. +- `preview_style_tool` / `style_comparison_tool`'s elicitation dialog still offers all three options, but choosing "create a new token" or "auto-create" fails against the Mapbox Tokens API with a scope/permission error (the dialog can't know ahead of time that this particular deployment's token lacks `tokens:write` — see the `isTemporaryServerToken` caveat in `src/utils/tokenElicitation.ts` for tokens where it can tell). +- `create_token_tool` is not exposed on the hosted endpoint at all. -Paste an existing public token (`pk.*`, with `styles:read` scope) when prompted, or create one ahead of time from your [Mapbox Account page](https://account.mapbox.com/). Running this server **locally** with your own `pk.*`/`sk.*` access token (which can carry `tokens:write`) restores all three elicitation options, including create and auto-create. +Choose **"I have a token to provide"** and paste an existing public token (`pk.*`, with `styles:read` scope), or provide `accessToken` directly. Create a token ahead of time from your [Mapbox Account page](https://account.mapbox.com/) if you don't have one. Running this server **locally** with your own `pk.*`/`sk.*` access token (which can carry `tokens:write`) also enables create and auto-create. ### Getting Your Mapbox Access Token @@ -203,7 +203,7 @@ Complete set of tools for managing Mapbox styles via the Styles API: 3. **Auto-create a basic token** - Let the tool create a simple preview token for you - **Goose**: ⚠️ Known bug - Form displays after timeout ([goose#6471](https://github.com/block/goose/issues/6471)) - **Claude Desktop, Claude Code**: ⚠️ Not yet supported - Provide `accessToken` parameter directly, or Claude will intelligently offer to create a token for you using `create_token_tool` - - **Hosted MCP endpoint**: ⚠️ Only "provide an existing token" is offered, regardless of client — see [Token creation is unavailable on the hosted endpoint](#hosted-mcp-endpoint) + - **Hosted MCP endpoint**: ⚠️ "Create" and "auto-create" will fail regardless of client — see [Token creation is unavailable on the hosted endpoint](#hosted-mcp-endpoint) - **Alternative**: Provide `accessToken` parameter directly for backward compatibility with any client - **Session Storage**: Your token choice is cached for the session, so you only need to provide it once (when elicitation is supported) - **Best Practice**: Use URL-restricted tokens to further limit token usage to specific domains. While public tokens in URLs are read-only, URL restrictions add an extra layer of security by ensuring tokens only work on your specified domains @@ -226,7 +226,7 @@ Complete set of tools for managing Mapbox styles via the Styles API: 3. **Auto-create a basic token** - Let the tool create a simple preview token for you - **Goose**: ⚠️ Known bug - Form displays after timeout ([goose#6471](https://github.com/block/goose/issues/6471)) - **Claude Desktop, Claude Code**: ⚠️ Not yet supported - Provide `accessToken` parameter directly, or Claude will intelligently offer to create a token for you using `create_token_tool` - - **Hosted MCP endpoint**: ⚠️ Only "provide an existing token" is offered, regardless of client — see [Token creation is unavailable on the hosted endpoint](#hosted-mcp-endpoint) + - **Hosted MCP endpoint**: ⚠️ "Create" and "auto-create" will fail regardless of client — see [Token creation is unavailable on the hosted endpoint](#hosted-mcp-endpoint) - **Alternative**: Provide `accessToken` parameter directly for backward compatibility with any client - **Session Storage**: Your token choice is cached for the session, so you only need to provide it once (when elicitation is supported) - **Best Practice**: Use URL-restricted tokens to further limit token usage to specific domains. While public tokens in URLs are read-only, URL restrictions add an extra layer of security by ensuring tokens only work on your specified domains diff --git a/src/utils/tokenElicitation.ts b/src/utils/tokenElicitation.ts index 23d7357..bc887bd 100644 --- a/src/utils/tokenElicitation.ts +++ b/src/utils/tokenElicitation.ts @@ -10,13 +10,20 @@ import type { HttpRequest } from './types.js'; export type TokenChoice = 'provide' | 'create' | 'auto'; /** - * Mapbox's Tokens API rejects requests to create a token when the caller is - * authenticated with a temporary token (`tk.*`) — temporary tokens are scoped - * to a single short-lived session and are never granted `tokens:write`. This - * is the case for the hosted MCP DevKit Server, which authenticates each - * request with a per-session `tk.*` token rather than the caller's own - * pk./sk. token. Detecting this upfront lets callers skip a doomed API round - * trip and steer the user straight to "provide an existing token" instead. + * A literal Mapbox temporary token (`tk.*`) is scoped to a single short-lived + * session and is not granted `tokens:write`, so attempting to create a new + * token with one is a guaranteed API rejection. This is a narrow, string-shape + * check on the server's own access token (e.g. `MAPBOX_ACCESS_TOKEN=tk...`) — + * it lets callers skip a doomed round trip to the Tokens API in that specific + * case. + * + * It is not a general test for "can this token create tokens". Servers that + * embed this package behind their own auth (for example, an OAuth-based + * hosted deployment) may pass through a bearer that isn't shaped like a + * Mapbox token at all yet still lacks `tokens:write` for its own reasons — + * this check can't see that, and the create/auto-create paths fall through to + * the Tokens API and surface whatever error it returns (see + * {@link createPreviewToken}). */ export function isTemporaryServerToken(accessToken: string): boolean { return accessToken.startsWith('tk.'); @@ -253,10 +260,13 @@ export async function listPublicPreviewTokens( * public scopes, so the API is guaranteed to hand back a `pk.*` token rather than `sk.*`; * `styles:download` in particular is a secret-only scope and must not be requested here). * - * Returns a structured failure instead of throwing when the server's own access token is - * a temporary `tk.*` token (see {@link isTemporaryServerToken}) — the Tokens API rejects - * token-creation requests from those, so this is checked before making the request rather - * than surfacing whatever generic error the API happens to return for it. + * Skips the request and returns a structured failure immediately when the server's own + * access token is a literal Mapbox temporary token (`tk.*`, see + * {@link isTemporaryServerToken}) — that shape is a guaranteed rejection. Any other + * caller that lacks `tokens:write` (for instance a hosted deployment's own auth bearer, + * which isn't shaped like a Mapbox token at all) isn't detectable ahead of time, so that + * case falls through to the API call below and gets a scope-shortage hint appended to + * whatever error the Tokens API returns. */ export async function createPreviewToken( httpRequest: HttpRequest, @@ -307,9 +317,28 @@ export async function createPreviewToken( if (!response.ok) { const errorText = await response.text(); + let message = `Failed to create token: ${response.status} ${errorText}`; + + // Creating a token always requires `tokens:write` on the caller's own access + // token, so a 401/403 here is a permission problem — surface the same + // scope-shortage hint MapboxApiBasedTool#handleApiError gives other tools, + // rather than leaving the caller to guess from a bare status code and body. + const looksLikePermissionError = + response.status === 401 || + response.status === 403 || + /scope|permission/i.test(errorText); + if (looksLikePermissionError) { + message += + '\n\nThis looks like a scope/permission issue: creating a token requires ' + + "`tokens:write` on the caller's own access token. If you're running behind a " + + 'hosted or proxied deployment, that token may not carry it even though it ' + + 'works for other operations. Use "I have a token to provide" with an ' + + 'existing public token instead.'; + } + return { success: false, - error: `Failed to create token: ${response.status} ${errorText}` + error: message }; } diff --git a/test/utils/tokenElicitation.test.ts b/test/utils/tokenElicitation.test.ts index 00d1e22..e472e8a 100644 --- a/test/utils/tokenElicitation.test.ts +++ b/test/utils/tokenElicitation.test.ts @@ -165,7 +165,7 @@ describe('createPreviewToken', () => { expect(result.error).toContain('non-public token'); }); - it('surfaces API errors', async () => { + it('surfaces API errors with a scope hint on 403', async () => { const { httpRequest } = setupHttpRequest({ ok: false, status: 403, @@ -181,6 +181,26 @@ describe('createPreviewToken', () => { expect(result.success).toBe(false); expect(result.error).toContain('insufficient scopes'); + expect(result.error).toContain('tokens:write'); + }); + + it('does not add a scope hint for unrelated server errors', async () => { + const { httpRequest } = setupHttpRequest({ + ok: false, + status: 500, + text: async () => 'internal server error' + }); + + const result = await createPreviewToken( + httpRequest, + MAPBOX_API_ENDPOINT, + 'sk.eyJ1IjoidGVzdCJ9.sig', + 'test-user' + ); + + expect(result.success).toBe(false); + expect(result.error).toContain('internal server error'); + expect(result.error).not.toContain('tokens:write'); }); }); From 9eaf79f44a28713edb2f175e72ee5b01140d06e0 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Fri, 31 Jul 2026 10:50:55 -0400 Subject: [PATCH 15/25] Add HTTP integration test for elicitation over the real MCP wire protocol Every existing elicitation test fakes tool['server'] directly and never proves the SDK's own capability negotiation and request/response plumbing works end to end. This spins up a real Streamable HTTP MCP server (session-scoped, one McpServer/transport pair per Mcp-Session-Id) and drives it with a real Client that answers elicitation/create requests, modeled on hosted-mcp-server's own request handling (bearer token from Authorization attached to the raw request as .auth). Covers, fully offline (httpRequest mocked, no real network calls): - tk.* server token: dialog trims to ["provide"], Tokens API never called - non-tk.*-shaped token (the hosted-endpoint case): dialog offers all three choices, auto-create fails against a mocked 403 and the error includes the scope/permission hint - non-tk.*-shaped token: auto-create succeeds end to end - style_comparison_tool gets the same tk.* trimming as preview_style_tool Building this surfaced a real, separate finding worth a follow-up: a first attempt used a fresh McpServer per HTTP request (the "stateless" pattern both mcp-server's scripts/dev-http-server.ts and hosted-mcp-server's src/routes/mcp.ts use), and every elicitation call failed with "client does not support elicitation" regardless of what the client declared. Server#getClientCapabilities() is only ever set on whichever Server instance processes the client's initialize request; a fresh Server per request means the tools/call request's instance never saw that handshake. Documented in the harness's doc comment; not otherwise addressed here since it isn't this PR's tool code. --- test/integration/elicitationOverHttp.test.ts | 351 +++++++++++++++++++ 1 file changed, 351 insertions(+) create mode 100644 test/integration/elicitationOverHttp.test.ts diff --git a/test/integration/elicitationOverHttp.test.ts b/test/integration/elicitationOverHttp.test.ts new file mode 100644 index 0000000..23778d9 --- /dev/null +++ b/test/integration/elicitationOverHttp.test.ts @@ -0,0 +1,351 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +/** + * Drives preview_style_tool / style_comparison_tool's elicitation flow over a real + * Streamable HTTP MCP connection — a real `Server` sending an `elicitation/create` + * request and a real `Client` answering it, not a hand-built stand-in for `this.server`. + * This is the only place that exercises the actual wire protocol; every other test for + * these tools fakes `tool['server']` directly and never proves the SDK's own capability + * negotiation and request/response plumbing works end to end. + * + * The server is session-scoped (one `McpServer`/transport pair per `Mcp-Session-Id`, + * matching the SDK's documented stateful-mode example) — see the comment on + * `startHarness` below for why that matters specifically for elicitation. The bearer + * token from the `Authorization` header is attached to the raw Node request as `.auth` + * before handing off to `StreamableHTTPServerTransport`, mirroring hosted-mcp-server's + * src/routes/mcp.ts. + * + * The Mapbox Tokens API itself is never hit — `httpRequest` is a mock, so this stays + * fully offline and deterministic (per CLAUDE.md: real network calls are never + * acceptable in tests). + */ + +import { createServer, type IncomingMessage, type Server } from 'node:http'; +import type { AddressInfo } from 'node:net'; +import { randomUUID } from 'node:crypto'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; +import { + ElicitRequestSchema, + type ElicitRequest, + type ElicitResult +} from '@modelcontextprotocol/sdk/types.js'; +import { PreviewStyleTool } from '../../src/tools/preview-style-tool/PreviewStyleTool.js'; +import { StyleComparisonTool } from '../../src/tools/style-comparison-tool/StyleComparisonTool.js'; +import { previewTokenStorage } from '../../src/utils/tokenElicitation.js'; +import type { HttpRequest } from '../../src/utils/types.js'; + +const TK_SERVER_TOKEN = + 'tk.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; +// Shaped like the hosted MCP endpoint's real bearer: a plain 3-part JWT with no +// pk./sk./tk. prefix (see PR #57 discussion) — `isTemporaryServerToken` can't +// recognize this as unable to create tokens, only the API call itself can. +const OAUTH_STYLE_SERVER_TOKEN = + 'eyJhbGciOiJIUzI1NiJ9.eyJ1IjoidGVzdC11c2VyIn0.signature'; +const EXISTING_PUBLIC_TOKEN = + 'pk.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; + +function jsonResponse(status: number, body: unknown): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' } + }); +} + +/** A mock HttpRequest that fails any call not explicitly queued, so an unexpected + * network attempt (e.g. the tk.* guard failing to short-circuit) shows up as a loud + * test failure instead of a silent pass. */ +function mockHttpRequest( + handlers: Record<'GET' | 'POST', () => Response> +): HttpRequest { + return vi.fn(async (_url, init) => { + const method = ((init?.method as string) || 'GET').toUpperCase() as + | 'GET' + | 'POST'; + const handler = handlers[method]; + if (!handler) { + throw new Error(`Unexpected ${method} request in test`); + } + return handler(); + }) as unknown as HttpRequest; +} + +interface TestHarness { + baseUrl: URL; + close(): Promise; +} + +/** + * Session-scoped stateful Streamable HTTP server: one `McpServer`/transport pair per + * `Mcp-Session-Id`, created on the first (`initialize`) request and reused for every + * subsequent request in that session — the documented SDK pattern for stateful mode. + * + * This matters specifically for elicitation: `Server#getClientCapabilities()` (which + * `PreviewStyleTool`/`StyleComparisonTool` check before calling `elicitInput`) is set + * once, on whichever `Server` instance processes the client's `initialize` request, and + * never persists anywhere else. A server that hands each incoming HTTP request to a + * brand-new `McpServer` (the "stateless" pattern used by mcp-server's + * scripts/dev-http-server.ts and by hosted-mcp-server's src/routes/mcp.ts, both + * `sessionIdGenerator: undefined`) means the `initialize` request and every later + * `tools/call` request land on *different* `Server` objects — the tool call's instance + * never saw the initialize handshake, so `getClientCapabilities()` is always + * `undefined` there regardless of what the connecting client actually declared. That + * was discovered by this test failing under a first attempt at a stateless harness; see + * PR #57 discussion. Whether that also silently breaks elicitation on the real hosted + * endpoint (independently of the tk.* issue) is worth following up on separately — it's + * not this PR's tool code, so it isn't re-litigated here. + */ +function startHarness( + previewHttpRequest: HttpRequest, + comparisonHttpRequest: HttpRequest = previewHttpRequest +): Promise { + const previewTool = new PreviewStyleTool({ httpRequest: previewHttpRequest }); + const comparisonTool = new StyleComparisonTool({ + httpRequest: comparisonHttpRequest + }); + + const sessions = new Map(); + + function buildTransport(): StreamableHTTPServerTransport { + const mcpServer = new McpServer( + { name: 'elicitation-http-test', version: '1.0.0' }, + { capabilities: { tools: { listChanged: true } } } + ); + previewTool.installTo(mcpServer); + comparisonTool.installTo(mcpServer); + + const transport = new StreamableHTTPServerTransport({ + sessionIdGenerator: () => randomUUID(), + onsessioninitialized: (sessionId) => { + sessions.set(sessionId, transport); + }, + onsessionclosed: (sessionId) => { + sessions.delete(sessionId); + } + }); + transport.onclose = () => { + if (transport.sessionId) sessions.delete(transport.sessionId); + }; + void mcpServer.connect(transport); + return transport; + } + + const httpServer: Server = createServer((req, res) => { + void (async () => { + const authHeader = req.headers.authorization; + const match = authHeader?.match(/^Bearer (.+)$/); + const reqWithAuth = req as IncomingMessage & { + auth?: { token: string; clientId: string; scopes: string[] }; + }; + if (match) { + // Mirrors hosted-mcp-server's src/routes/mcp.ts: attach the bearer to the + // raw request as `.auth` before the transport touches it. + reqWithAuth.auth = { + token: match[1], + clientId: 'test-client', + scopes: [] + }; + } + + const sessionIdHeader = req.headers['mcp-session-id']; + const existing = + typeof sessionIdHeader === 'string' + ? sessions.get(sessionIdHeader) + : undefined; + const transport = existing ?? buildTransport(); + + try { + await transport.handleRequest(reqWithAuth, res); + } catch (error) { + if (!res.headersSent) { + res.writeHead(500).end(String(error)); + } + } + })(); + }); + + return new Promise((resolve) => { + httpServer.listen(0, '127.0.0.1', () => { + const { port } = httpServer.address() as AddressInfo; + resolve({ + baseUrl: new URL(`http://127.0.0.1:${port}/mcp`), + close: () => + new Promise((res, rej) => + httpServer.close((err) => (err ? rej(err) : res())) + ) + }); + }); + }); +} + +async function connectClient( + baseUrl: URL, + bearerToken: string, + onElicit: (request: ElicitRequest) => ElicitResult +): Promise { + const client = new Client( + { name: 'elicitation-http-test-client', version: '1.0.0' }, + { capabilities: { elicitation: {} } } + ); + client.setRequestHandler(ElicitRequestSchema, (request) => onElicit(request)); + + const transport = new StreamableHTTPClientTransport(baseUrl, { + requestInit: { headers: { Authorization: `Bearer ${bearerToken}` } } + }); + await client.connect(transport); + return client; +} + +/** + * `ElicitRequest.params` is a union (form-mode vs. other elicitation modes); this + * server only ever sends the form-mode shape (`message` + `requestedSchema`) that + * `elicitPreviewToken` builds, so narrowing here is safe for these tests. + */ +function getChoiceEnum(request: ElicitRequest): string[] { + const params = request.params as unknown as { + requestedSchema: { properties: { choice: { enum: string[] } } }; + }; + return params.requestedSchema.properties.choice.enum; +} + +describe('preview/comparison token elicitation over real Streamable HTTP', () => { + let harness: TestHarness | undefined; + let client: Client | undefined; + + beforeEach(() => { + previewTokenStorage.clearAll(); + }); + + afterEach(async () => { + await client?.close().catch(() => {}); + await harness?.close(); + client = undefined; + harness = undefined; + }); + + it('trims the dialog to "provide" and never calls the Tokens API when the server token is tk.*', async () => { + const httpRequest = mockHttpRequest({ + GET: () => { + throw new Error('should not list tokens for a tk.* server token'); + }, + POST: () => { + throw new Error('should not create a token for a tk.* server token'); + } + }); + harness = await startHarness(httpRequest); + + let receivedEnum: unknown; + client = await connectClient( + harness.baseUrl, + TK_SERVER_TOKEN, + (request) => { + receivedEnum = getChoiceEnum(request); + return { + action: 'accept', + content: { choice: 'provide', token: EXISTING_PUBLIC_TOKEN } + }; + } + ); + + const result = await client.callTool({ + name: 'preview_style_tool', + arguments: { styleId: 'test-style' } + }); + + expect(result.isError).toBeFalsy(); + expect(receivedEnum).toEqual(['provide']); + expect(httpRequest).not.toHaveBeenCalled(); + }); + + it('offers all three choices for a non-tk.*-shaped server token and surfaces a scope hint when auto-create fails (the hosted-endpoint case)', async () => { + const httpRequest = mockHttpRequest({ + GET: () => jsonResponse(200, []), + POST: () => jsonResponse(403, { message: 'insufficient scopes' }) + }); + harness = await startHarness(httpRequest); + + let receivedEnum: unknown; + client = await connectClient( + harness.baseUrl, + OAUTH_STYLE_SERVER_TOKEN, + (request) => { + receivedEnum = getChoiceEnum(request); + return { action: 'accept', content: { choice: 'auto' } }; + } + ); + + const result = await client.callTool({ + name: 'preview_style_tool', + arguments: { styleId: 'test-style' } + }); + + expect(receivedEnum).toEqual(['provide', 'create', 'auto']); + expect(result.isError).toBe(true); + const text = (result.content as Array<{ type: string; text?: string }>)[0] + .text as string; + expect(text).toContain('insufficient scopes'); + expect(text).toContain('tokens:write'); + }); + + it('completes auto-create end to end for a server token that can create tokens', async () => { + const httpRequest = mockHttpRequest({ + GET: () => jsonResponse(200, []), + POST: () => jsonResponse(200, { token: EXISTING_PUBLIC_TOKEN }) + }); + harness = await startHarness(httpRequest); + + client = await connectClient( + harness.baseUrl, + OAUTH_STYLE_SERVER_TOKEN, + () => ({ action: 'accept', content: { choice: 'auto' } }) + ); + + const result = await client.callTool({ + name: 'preview_style_tool', + arguments: { styleId: 'test-style' } + }); + + expect(result.isError).toBeFalsy(); + const text = (result.content as Array<{ type: string; text?: string }>)[0] + .text as string; + expect(text).toContain(`access_token=${EXISTING_PUBLIC_TOKEN}`); + }); + + it("trims style_comparison_tool's dialog the same way for a tk.* server token", async () => { + const httpRequest = mockHttpRequest({ + GET: () => { + throw new Error('should not list tokens for a tk.* server token'); + }, + POST: () => { + throw new Error('should not create a token for a tk.* server token'); + } + }); + harness = await startHarness(httpRequest); + + let receivedEnum: unknown; + client = await connectClient( + harness.baseUrl, + TK_SERVER_TOKEN, + (request) => { + receivedEnum = getChoiceEnum(request); + return { + action: 'accept', + content: { choice: 'provide', token: EXISTING_PUBLIC_TOKEN } + }; + } + ); + + const result = await client.callTool({ + name: 'style_comparison_tool', + arguments: { before: 'mapbox/streets-v12', after: 'mapbox/outdoors-v12' } + }); + + expect(result.isError).toBeFalsy(); + expect(receivedEnum).toEqual(['provide']); + expect(httpRequest).not.toHaveBeenCalled(); + }); +}); From ae725b0211dfdb8b91932e3db3e7ce6e99abd923 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Fri, 31 Jul 2026 10:56:30 -0400 Subject: [PATCH 16/25] Fix race in HTTP integration test harness that caused CI flake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildTransport() called mcpServer.connect(transport) without awaiting it, then returned the transport for immediate use by the very next handleRequest() call. Locally the connect() promise happened to settle before that mattered; under CI's different scheduling, the first request (the client's initialize) sometimes raced ahead of the server's own wiring, and the request never got a response — observed as the style_comparison_tool test timing out after 60s with "MCP error -32001: Request timed out" while the other three tests in the same file passed. Made buildTransport async and awaited it at the call site. Ran the suite 8x locally with no failures after the fix (it never reproduced locally to begin with, consistent with a narrow scheduling-dependent race rather than a logic bug in the tools themselves). --- test/integration/elicitationOverHttp.test.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/integration/elicitationOverHttp.test.ts b/test/integration/elicitationOverHttp.test.ts index 23778d9..516d6d8 100644 --- a/test/integration/elicitationOverHttp.test.ts +++ b/test/integration/elicitationOverHttp.test.ts @@ -110,7 +110,7 @@ function startHarness( const sessions = new Map(); - function buildTransport(): StreamableHTTPServerTransport { + async function buildTransport(): Promise { const mcpServer = new McpServer( { name: 'elicitation-http-test', version: '1.0.0' }, { capabilities: { tools: { listChanged: true } } } @@ -130,7 +130,12 @@ function startHarness( transport.onclose = () => { if (transport.sessionId) sessions.delete(transport.sessionId); }; - void mcpServer.connect(transport); + // Must resolve before handleRequest is called on this transport, or the + // first request (the client's `initialize`) races the server's own + // connect/wiring — harmless most of the time locally, but a real + // intermittent hang under CI's different scheduling/timing (surfaced as + // an MCP "Request timed out" on whichever test happened to lose the race). + await mcpServer.connect(transport); return transport; } @@ -156,7 +161,7 @@ function startHarness( typeof sessionIdHeader === 'string' ? sessions.get(sessionIdHeader) : undefined; - const transport = existing ?? buildTransport(); + const transport = existing ?? (await buildTransport()); try { await transport.handleRequest(reqWithAuth, res); From 2b3fddca4877ddc762911341cd38072a4cc4a79e Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Fri, 31 Jul 2026 11:01:16 -0400 Subject: [PATCH 17/25] Fix backwards client list in the no-elicitation-support error message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both tools' fallback error (shown when the client lacks the elicitation capability) named Claude Desktop and Claude Code as example clients that support MCP elicitation — exactly backwards. Per the README's own support matrix, those two are the ones that *don't* support it; only MCP Inspector, Cursor, and VS Code do. Confirmed live in Claude Desktop, where the model was relaying this text almost verbatim while correctly working around it by asking the user for a pk. token directly. --- src/tools/preview-style-tool/PreviewStyleTool.ts | 2 +- src/tools/style-comparison-tool/StyleComparisonTool.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/preview-style-tool/PreviewStyleTool.ts b/src/tools/preview-style-tool/PreviewStyleTool.ts index 5d272a6..3f2292a 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.ts @@ -105,7 +105,7 @@ export class PreviewStyleTool extends BaseTool { type: 'text', text: 'Preview token required but client does not support elicitation. ' + - 'Please provide an accessToken parameter directly, or use a client that supports MCP elicitation (e.g., Claude Desktop, Claude Code).' + 'Please provide an accessToken parameter directly, or use a client that supports MCP elicitation (e.g., MCP Inspector, Cursor, VS Code).' } ] }; diff --git a/src/tools/style-comparison-tool/StyleComparisonTool.ts b/src/tools/style-comparison-tool/StyleComparisonTool.ts index 1e5b973..f429dc9 100644 --- a/src/tools/style-comparison-tool/StyleComparisonTool.ts +++ b/src/tools/style-comparison-tool/StyleComparisonTool.ts @@ -152,7 +152,7 @@ export class StyleComparisonTool extends BaseTool< type: 'text', text: 'Preview token required but client does not support elicitation. ' + - 'Please provide an accessToken parameter directly, or use a client that supports MCP elicitation (e.g., Claude Desktop, Claude Code).' + 'Please provide an accessToken parameter directly, or use a client that supports MCP elicitation (e.g., MCP Inspector, Cursor, VS Code).' } ] }; From 848963cfa98753c59e6cb7b8711dc8339765e6bf Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Tue, 4 Aug 2026 14:20:47 -0400 Subject: [PATCH 18/25] Address PR #57 review feedback: useCustomToken fallback and cache-key trust - useCustomToken now only forces the elicitation dialog when the client actually supports it. A client without elicitation support can't act on the flag anyway, so it silently falls back to a cached token instead of returning an avoidable "client does not support elicitation" error when a perfectly good cached token exists. (Valiunia: "what happens if the client does not support selection dialog?") - previewTokenStorage is now keyed by a sha256 hash of the server's own access token (cacheKeyFor) instead of the username decoded out of it. getUserNameFromToken never verifies a JWT's signature, so two different presented tokens could decode to the same username without this process ever confirming that independently - not exploitable via Mapbox's API itself (which re-authenticates via the token value regardless of the path segment we build), but a real, avoidable risk for this package's own in-memory cache when it's run behind a gateway that doesn't pre-verify bearers the way hosted-mcp-server's does. (Valiunia: "can we trust userName... unless it's a network call") Added regression tests for both: a cache-hit test for the useCustomToken fallback on a capability-blind client, and cacheKeyFor unit tests proving two tokens with the same decoded username produce different cache keys. --- .../preview-style-tool/PreviewStyleTool.ts | 22 ++++++++---- .../StyleComparisonTool.ts | 22 ++++++++---- src/utils/tokenElicitation.ts | 36 +++++++++++++------ .../PreviewStyleTool.test.ts | 31 +++++++++++++++- .../StyleComparisonTool.test.ts | 33 +++++++++++++++++ test/utils/tokenElicitation.test.ts | 22 ++++++++++++ 6 files changed, 143 insertions(+), 23 deletions(-) diff --git a/src/tools/preview-style-tool/PreviewStyleTool.ts b/src/tools/preview-style-tool/PreviewStyleTool.ts index 3f2292a..cc0d275 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.ts @@ -9,6 +9,7 @@ import { } from './PreviewStyleTool.input.schema.js'; import { getUserNameFromToken } from '../../utils/jwtUtils.js'; import { + cacheKeyFor, createPreviewToken, elicitPreviewToken, isTemporaryServerToken, @@ -77,9 +78,19 @@ export class PreviewStyleTool extends BaseTool { }; } - // Check for stored preview token (unless user wants to use custom) - const storedToken = previewTokenStorage.get(userName); - if (storedToken && !input.useCustomToken) { + // Check for stored preview token (unless user wants to use custom AND the + // client can actually act on that — a client with no elicitation support + // can't honor useCustomToken anyway, so silently reuse the cache instead + // of forcing an avoidable error). + const clientSupportsElicitation = Boolean( + this.server?.server.getClientCapabilities()?.elicitation + ); + const cacheKey = cacheKeyFor(serverAccessToken || ''); + const storedToken = previewTokenStorage.get(cacheKey); + if ( + storedToken && + (!input.useCustomToken || !clientSupportsElicitation) + ) { publicToken = storedToken; } else { // Need to elicit token from user @@ -96,8 +107,7 @@ export class PreviewStyleTool extends BaseTool { } // Check if client supports elicitation capability - const clientCapabilities = this.server.server.getClientCapabilities(); - if (!clientCapabilities?.elicitation) { + if (!clientSupportsElicitation) { return { isError: true, content: [ @@ -192,7 +202,7 @@ export class PreviewStyleTool extends BaseTool { } // Store token for future use - previewTokenStorage.set(userName, publicToken); + previewTokenStorage.set(cacheKey, publicToken); } } diff --git a/src/tools/style-comparison-tool/StyleComparisonTool.ts b/src/tools/style-comparison-tool/StyleComparisonTool.ts index f429dc9..acdb70d 100644 --- a/src/tools/style-comparison-tool/StyleComparisonTool.ts +++ b/src/tools/style-comparison-tool/StyleComparisonTool.ts @@ -12,6 +12,7 @@ import { } from './StyleComparisonTool.schema.js'; import { getUserNameFromToken } from '../../utils/jwtUtils.js'; import { + cacheKeyFor, createPreviewToken, elicitPreviewToken, isTemporaryServerToken, @@ -126,9 +127,19 @@ export class StyleComparisonTool extends BaseTool< }; } - // Check for stored preview token (unless user wants to use custom) - const storedToken = previewTokenStorage.get(userName); - if (storedToken && !input.useCustomToken) { + // Check for stored preview token (unless user wants to use custom AND the + // client can actually act on that — a client with no elicitation support + // can't honor useCustomToken anyway, so silently reuse the cache instead + // of forcing an avoidable error). + const clientSupportsElicitation = Boolean( + this.server?.server.getClientCapabilities()?.elicitation + ); + const cacheKey = cacheKeyFor(serverAccessToken || ''); + const storedToken = previewTokenStorage.get(cacheKey); + if ( + storedToken && + (!input.useCustomToken || !clientSupportsElicitation) + ) { publicToken = storedToken; } else { if (!this.server) { @@ -143,8 +154,7 @@ export class StyleComparisonTool extends BaseTool< }; } - const clientCapabilities = this.server.server.getClientCapabilities(); - if (!clientCapabilities?.elicitation) { + if (!clientSupportsElicitation) { return { isError: true, content: [ @@ -233,7 +243,7 @@ export class StyleComparisonTool extends BaseTool< publicToken = created.token!; } - previewTokenStorage.set(userName, publicToken); + previewTokenStorage.set(cacheKey, publicToken); } } diff --git a/src/utils/tokenElicitation.ts b/src/utils/tokenElicitation.ts index bc887bd..b380ed2 100644 --- a/src/utils/tokenElicitation.ts +++ b/src/utils/tokenElicitation.ts @@ -1,6 +1,7 @@ // Copyright (c) Mapbox, Inc. // Licensed under the MIT License. +import { createHash } from 'node:crypto'; import { Server } from '@modelcontextprotocol/sdk/server/index.js'; import type { HttpRequest } from './types.js'; @@ -170,31 +171,46 @@ ${creationNote}`, } /** - * Session-level storage for preview token preferences. + * Derives a `previewTokenStorage` cache key from the server's own access token, rather + * than from the username decoded out of it. `getUserNameFromToken` never verifies a + * JWT's signature — it just base64-decodes the payload — so an unverified `u` claim is + * not a safe cache key: two different presented tokens could decode to the same + * username without this process ever independently confirming that. Hosted deployments + * (e.g. hosted-mcp-server) verify the bearer upstream before it reaches this code, but + * this package is also usable standalone or behind other gateways that may not, so the + * cache itself shouldn't depend on that assumption. Hashing the full token ties the + * cache slot to the exact credential presented instead. + */ +export function cacheKeyFor(token: string): string { + return createHash('sha256').update(token).digest('hex'); +} + +/** + * Session-level storage for preview token preferences, keyed by {@link cacheKeyFor}. * In a real implementation, this could be stored in a database or cache. */ class PreviewTokenStorage { private tokenCache = new Map(); /** - * Store a preview token for a specific username + * Store a preview token under the given cache key */ - set(username: string, token: string): void { - this.tokenCache.set(username, token); + set(cacheKey: string, token: string): void { + this.tokenCache.set(cacheKey, token); } /** - * Get stored preview token for a username + * Get the stored preview token for the given cache key */ - get(username: string): string | undefined { - return this.tokenCache.get(username); + get(cacheKey: string): string | undefined { + return this.tokenCache.get(cacheKey); } /** - * Clear stored token for a username + * Clear the stored token for the given cache key */ - clear(username: string): void { - this.tokenCache.delete(username); + clear(cacheKey: string): void { + this.tokenCache.delete(cacheKey); } /** diff --git a/test/tools/preview-style-tool/PreviewStyleTool.test.ts b/test/tools/preview-style-tool/PreviewStyleTool.test.ts index d6c81ac..7107e30 100644 --- a/test/tools/preview-style-tool/PreviewStyleTool.test.ts +++ b/test/tools/preview-style-tool/PreviewStyleTool.test.ts @@ -4,8 +4,12 @@ process.env.MAPBOX_ACCESS_TOKEN = 'sk.eyJhbGciOiJIUzI1NiJ9.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { PreviewStyleTool } from '../../../src/tools/preview-style-tool/PreviewStyleTool.js'; +import { + cacheKeyFor, + previewTokenStorage +} from '../../../src/utils/tokenElicitation.js'; import { setupHttpRequest } from '../../utils/httpPipelineUtils.js'; describe('PreviewStyleTool', () => { @@ -203,6 +207,10 @@ describe('PreviewStyleTool', () => { }); describe('elicitation behavior', () => { + beforeEach(() => { + previewTokenStorage.clearAll(); + }); + it('returns error when no accessToken and no valid server token', async () => { const tool = previewStyleTool(); @@ -280,5 +288,26 @@ describe('PreviewStyleTool', () => { // tokens against the Mapbox API should never even be attempted. expect(mockHttpRequest).not.toHaveBeenCalled(); }); + + it('reuses a cached token instead of erroring when useCustomToken is set but the client cannot act on it', async () => { + const serverToken = + 'sk.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; + previewTokenStorage.set(cacheKeyFor(serverToken), TEST_ACCESS_TOKEN); + + // No `this.server` is attached in these tests (installTo() was never + // called), so this exercises exactly the "client can't support the + // selection dialog" case a reviewer asked about on PR #57. + const result = await previewStyleTool().run( + { styleId: 'test-style', useCustomToken: true }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + { authInfo: { token: serverToken } } as any + ); + + expect(result.isError).toBe(false); + expect(result.content[0]).toMatchObject({ + type: 'text', + text: expect.stringContaining(`access_token=${TEST_ACCESS_TOKEN}`) + }); + }); }); }); diff --git a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts index a301541..eef2f43 100644 --- a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts +++ b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts @@ -4,6 +4,10 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { StyleComparisonTool } from '../../../src/tools/style-comparison-tool/StyleComparisonTool.js'; import * as jwtUtils from '../../../src/utils/jwtUtils.js'; +import { + cacheKeyFor, + previewTokenStorage +} from '../../../src/utils/tokenElicitation.js'; import { setupHttpRequest } from '../../utils/httpPipelineUtils.js'; function styleComparisonTool() { @@ -268,6 +272,10 @@ describe('StyleComparisonTool', () => { }); describe('elicitation behavior', () => { + beforeEach(() => { + previewTokenStorage.clearAll(); + }); + it('returns error when no accessToken and no valid server token', async () => { const tool = styleComparisonTool(); @@ -345,6 +353,31 @@ describe('StyleComparisonTool', () => { // tokens against the Mapbox API should never even be attempted. expect(mockHttpRequest).not.toHaveBeenCalled(); }); + + it('reuses a cached token instead of erroring when useCustomToken is set but the client cannot act on it', async () => { + const serverToken = + 'sk.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; + previewTokenStorage.set(cacheKeyFor(serverToken), 'pk.test.token'); + + // No `this.server` is attached in these tests (installTo() was never + // called), so this exercises exactly the "client can't support the + // selection dialog" case a reviewer asked about on PR #57. + const result = await styleComparisonTool().run( + { + before: 'mapbox/streets-v12', + after: 'mapbox/satellite-v9', + useCustomToken: true + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + { authInfo: { token: serverToken } } as any + ); + + expect(result.isError).toBe(false); + expect(result.content[0]).toMatchObject({ + type: 'text', + text: expect.stringContaining('access_token=pk.test.token') + }); + }); }); describe('metadata', () => { diff --git a/test/utils/tokenElicitation.test.ts b/test/utils/tokenElicitation.test.ts index e472e8a..c463408 100644 --- a/test/utils/tokenElicitation.test.ts +++ b/test/utils/tokenElicitation.test.ts @@ -4,6 +4,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import type { Server } from '@modelcontextprotocol/sdk/server/index.js'; import { + cacheKeyFor, createPreviewToken, elicitPreviewToken, isTemporaryServerToken, @@ -108,6 +109,27 @@ describe('isTemporaryServerToken', () => { }); }); +describe('cacheKeyFor', () => { + it('is deterministic for the same token', () => { + const token = 'sk.eyJ1IjoidGVzdC11c2VyIn0.sig'; + expect(cacheKeyFor(token)).toBe(cacheKeyFor(token)); + }); + + it('returns a sha256 hex digest', () => { + expect(cacheKeyFor('sk.eyJ1IjoidGVzdC11c2VyIn0.sig')).toMatch( + /^[0-9a-f]{64}$/ + ); + }); + + it('differs for two distinct tokens that decode to the same username', () => { + // Same `u` claim ('test-user'), different signatures — a naive username-keyed + // cache would conflate these two distinct, unverified bearers into one slot. + const tokenA = 'sk.eyJ1IjoidGVzdC11c2VyIn0.signature-a'; + const tokenB = 'sk.eyJ1IjoidGVzdC11c2VyIn0.signature-b'; + expect(cacheKeyFor(tokenA)).not.toBe(cacheKeyFor(tokenB)); + }); +}); + describe('createPreviewToken', () => { it('rejects tk.* server tokens without making a network call', async () => { const { httpRequest, mockHttpRequest } = setupHttpRequest(); From a92ebac70f26e8e1cac857be94d523df298aa5d7 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Wed, 5 Aug 2026 10:46:31 -0400 Subject: [PATCH 19/25] Fix cross-session elicitation hijack: stop relying on this.server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BaseTool.installTo(server) does this.server = server — mutable state on the tool instance itself. CORE_TOOLS instantiates tools once as module-level singletons, and any embedder that reuses those singletons across concurrent sessions (installTo() called again on every new connection — the pattern mcp-server's own scripts/dev-http-server.ts uses, and that hosted-mcp-server's dynamic import() caching produces too) durably clobbers this.server on every new connection: it points at whichever session connected *last*, not whichever session is making the *current* call. Before this PR, this.server was only read for logging. PreviewStyleTool and StyleComparisonTool's elicitation flow was the first thing reading it for something session-sensitive: elicitPreviewToken(this.server.server, ...). Once a second session connects, every subsequent tool call on the shared instance sends its "paste your token" prompt to that other, uninvolved session instead — an unprompted-dialog-injection and credential-exfiltration primitive, not a hypothetical. Reported via Fable/Valentin. Fix: route elicitation through extra.sendRequest instead of this.server.server.elicitInput(). extra is supplied fresh per call by the SDK, correctly scoped to whichever session actually made the current request, and can't be clobbered by another session's installTo() call. This also drops the proactive getClientCapabilities() check (not available per-call) in favor of attempting the request and catching an unsupported-client failure (ElicitationUnavailableError), which folds the earlier useCustomToken-vs-cache branching into a single attempt-then-fallback path. - tokenElicitation.ts: elicitPreviewToken() now takes extra.sendRequest and manually builds the elicitation/create request (mirroring what Server#elicitInput() does internally), validated against ElicitResultSchema. Added ElicitationUnavailableError to distinguish "client can't be asked" from "user declined." - PreviewStyleTool.ts / StyleComparisonTool.ts: added a run() override to forward extra down to execute() (BaseTool.run() only forwards the access token), removed all this.server-based capability/elicitation logic. - test/security/cross-session-elicitation-hijack.test.ts: new regression test — one shared PreviewStyleTool instance installed onto two real sessions (real Streamable HTTP, real Client/Server elicit round trip); confirms session A's own elicitation request reaches session A's client even though session B connected afterward and durably owns this.server. Failed against the pre-fix code exactly as expected (0 calls to session A's handler, 1 to session B's) before the fix, passes after. - Updated existing elicitation tests to mock extra.sendRequest instead of faking tool['server']. --- .../preview-style-tool/PreviewStyleTool.ts | 219 +++++++++------- .../StyleComparisonTool.ts | 208 +++++++++------ src/utils/tokenElicitation.ts | 127 ++++++--- .../cross-session-elicitation-hijack.test.ts | 243 ++++++++++++++++++ .../PreviewStyleTool.test.ts | 20 +- .../StyleComparisonTool.test.ts | 20 +- test/utils/tokenElicitation.test.ts | 44 ++-- 7 files changed, 637 insertions(+), 244 deletions(-) create mode 100644 test/security/cross-session-elicitation-hijack.test.ts diff --git a/src/tools/preview-style-tool/PreviewStyleTool.ts b/src/tools/preview-style-tool/PreviewStyleTool.ts index cc0d275..aec8456 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.ts @@ -1,5 +1,6 @@ import { randomUUID } from 'node:crypto'; import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol.js'; import { createUIResource } from '@mcp-ui/server'; import { BaseTool } from '../BaseTool.js'; import { MapboxApiBasedTool } from '../MapboxApiBasedTool.js'; @@ -11,6 +12,7 @@ import { getUserNameFromToken } from '../../utils/jwtUtils.js'; import { cacheKeyFor, createPreviewToken, + ElicitationUnavailableError, elicitPreviewToken, isTemporaryServerToken, listPublicPreviewTokens, @@ -18,6 +20,13 @@ import { } from '../../utils/tokenElicitation.js'; import type { HttpRequest } from '../../utils/types.js'; +// `BaseTool#execute`'s abstract signature accepts `ToolExecutionContext` in this slot; +// overriding with the concrete `RequestHandlerExtra` type here (rather than `any`) would +// fail TS's contravariant parameter check since the two types don't overlap. `any` is the +// same escape hatch `BaseTool.run()` itself already uses for this exact parameter. +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type ToolCallExtra = RequestHandlerExtra; + export class PreviewStyleTool extends BaseTool { readonly name = 'preview_style_tool'; readonly description = @@ -48,10 +57,33 @@ export class PreviewStyleTool extends BaseTool { this.httpRequest = params.httpRequest; } + /** + * Overridden only to forward `extra` down to `execute()`. `BaseTool.run()` extracts + * `accessToken` from `extra` and drops the rest, but elicitation needs `extra.sendRequest` + * — the per-call, correctly-session-scoped request sender (see the doc comment on + * `elicitPreviewToken` for why `this.server` can't be used for this instead). + */ + async run(rawInput: unknown, extra?: ToolCallExtra): Promise { + try { + const input = this.inputSchema.parse(rawInput); + const accessToken = + extra?.authInfo?.token || process.env.MAPBOX_ACCESS_TOKEN; + return this.execute(input, accessToken, extra); + } catch (error) { + return { + isError: true, + content: [{ type: 'text', text: (error as Error).message }] + }; + } + } + protected async execute( input: PreviewStyleInput, - serverAccessToken?: string + serverAccessToken?: string, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + rawExtra?: any ): Promise { + const extra: ToolCallExtra | undefined = rawExtra; let publicToken: string; let userName: string; @@ -78,23 +110,19 @@ export class PreviewStyleTool extends BaseTool { }; } - // Check for stored preview token (unless user wants to use custom AND the - // client can actually act on that — a client with no elicitation support - // can't honor useCustomToken anyway, so silently reuse the cache instead - // of forcing an avoidable error). - const clientSupportsElicitation = Boolean( - this.server?.server.getClientCapabilities()?.elicitation - ); const cacheKey = cacheKeyFor(serverAccessToken || ''); const storedToken = previewTokenStorage.get(cacheKey); - if ( - storedToken && - (!input.useCustomToken || !clientSupportsElicitation) - ) { + + // Reuse the cached token unless the caller explicitly wants to choose a + // different one — in which case we still need `extra.sendRequest` to ask. + if (storedToken && !input.useCustomToken) { publicToken = storedToken; - } else { - // Need to elicit token from user - if (!this.server) { + } else if (!extra?.sendRequest) { + // No per-call session context to elicit through at all (e.g. invoked + // directly, outside a connected MCP session). + if (storedToken) { + publicToken = storedToken; + } else { return { isError: true, content: [ @@ -105,22 +133,7 @@ export class PreviewStyleTool extends BaseTool { ] }; } - - // Check if client supports elicitation capability - if (!clientSupportsElicitation) { - return { - isError: true, - content: [ - { - type: 'text', - text: - 'Preview token required but client does not support elicitation. ' + - 'Please provide an accessToken parameter directly, or use a client that supports MCP elicitation (e.g., MCP Inspector, Cursor, VS Code).' - } - ] - }; - } - + } else { // A server authenticated with a temporary tk.* token (e.g. the hosted MCP // DevKit Server) can never call the Tokens API to create a new token, so // the "create"/"auto" options are dropped from the dialog before asking. @@ -136,73 +149,107 @@ export class PreviewStyleTool extends BaseTool { ) : []; - // Elicit token choice from user - const elicited = await elicitPreviewToken( - this.server.server, - existingTokens, - canCreateTokens - ); - - // Handle user's choice - if (elicited.choice === 'provide') { - if (!elicited.token) { - return { - isError: true, - content: [ - { - type: 'text', - text: 'No token provided. Please provide a valid public token.' - } - ] - }; - } - publicToken = elicited.token; - } else if (elicited.choice === 'create') { - // Create new token with user's specifications - const created = await createPreviewToken( - this.httpRequest, - MapboxApiBasedTool.mapboxApiEndpoint, - serverAccessToken!, - userName, - elicited.tokenNote, - elicited.urlRestrictions + try { + // Elicit token choice from user, over *this* call's own connection. + const elicited = await elicitPreviewToken( + extra.sendRequest, + existingTokens, + canCreateTokens ); - if (!created.success) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Failed to create token: ${created.error}` - } - ] - }; + + // Handle user's choice + if (elicited.choice === 'provide') { + if (!elicited.token) { + return { + isError: true, + content: [ + { + type: 'text', + text: 'No token provided. Please provide a valid public token.' + } + ] + }; + } + publicToken = elicited.token; + } else if (elicited.choice === 'create') { + // Create new token with user's specifications + const created = await createPreviewToken( + this.httpRequest, + MapboxApiBasedTool.mapboxApiEndpoint, + serverAccessToken!, + userName, + elicited.tokenNote, + elicited.urlRestrictions + ); + if (!created.success) { + return { + isError: true, + content: [ + { + type: 'text', + text: `Failed to create token: ${created.error}` + } + ] + }; + } + publicToken = created.token!; + } else { + // auto - create basic preview token + const created = await createPreviewToken( + this.httpRequest, + MapboxApiBasedTool.mapboxApiEndpoint, + serverAccessToken!, + userName + ); + if (!created.success) { + return { + isError: true, + content: [ + { + type: 'text', + text: `Failed to auto-create token: ${created.error}` + } + ] + }; + } + publicToken = created.token!; } - publicToken = created.token!; - } else { - // auto - create basic preview token - const created = await createPreviewToken( - this.httpRequest, - MapboxApiBasedTool.mapboxApiEndpoint, - serverAccessToken!, - userName - ); - if (!created.success) { + + // Store token for future use + previewTokenStorage.set(cacheKey, publicToken); + } catch (error) { + if (error instanceof ElicitationUnavailableError) { + // The client can't be asked at all (e.g. no elicitation support). Fall + // back to a cached token if one exists rather than failing outright. + if (storedToken) { + publicToken = storedToken; + } else { + return { + isError: true, + content: [ + { + type: 'text', + text: + 'Preview token required but client does not support elicitation. ' + + 'Please provide an accessToken parameter directly, or use a client that supports MCP elicitation (e.g., MCP Inspector, Cursor, VS Code).' + } + ] + }; + } + } else { + // The client was asked and the user declined/cancelled, or something + // else went wrong — surface it rather than silently reusing a cache. return { isError: true, content: [ { type: 'text', - text: `Failed to auto-create token: ${created.error}` + text: error instanceof Error ? error.message : String(error) } ] }; } - publicToken = created.token!; } - - // Store token for future use - previewTokenStorage.set(cacheKey, publicToken); } } diff --git a/src/tools/style-comparison-tool/StyleComparisonTool.ts b/src/tools/style-comparison-tool/StyleComparisonTool.ts index acdb70d..5a72326 100644 --- a/src/tools/style-comparison-tool/StyleComparisonTool.ts +++ b/src/tools/style-comparison-tool/StyleComparisonTool.ts @@ -3,6 +3,7 @@ import { randomUUID } from 'node:crypto'; import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol.js'; import { createUIResource } from '@mcp-ui/server'; import { BaseTool } from '../BaseTool.js'; import { MapboxApiBasedTool } from '../MapboxApiBasedTool.js'; @@ -14,6 +15,7 @@ import { getUserNameFromToken } from '../../utils/jwtUtils.js'; import { cacheKeyFor, createPreviewToken, + ElicitationUnavailableError, elicitPreviewToken, isTemporaryServerToken, listPublicPreviewTokens, @@ -21,6 +23,13 @@ import { } from '../../utils/tokenElicitation.js'; import type { HttpRequest } from '../../utils/types.js'; +// `BaseTool#execute`'s abstract signature accepts `ToolExecutionContext` in this slot; +// overriding with the concrete `RequestHandlerExtra` type here (rather than `any`) would +// fail TS's contravariant parameter check since the two types don't overlap. `any` is the +// same escape hatch `BaseTool.run()` itself already uses for this exact parameter. +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type ToolCallExtra = RequestHandlerExtra; + export class StyleComparisonTool extends BaseTool< typeof StyleComparisonSchema > { @@ -98,10 +107,33 @@ export class StyleComparisonTool extends BaseTool< return resolved; } + /** + * Overridden only to forward `extra` down to `execute()`. `BaseTool.run()` extracts + * `accessToken` from `extra` and drops the rest, but elicitation needs `extra.sendRequest` + * — the per-call, correctly-session-scoped request sender (see the doc comment on + * `elicitPreviewToken` for why `this.server` can't be used for this instead). + */ + async run(rawInput: unknown, extra?: ToolCallExtra): Promise { + try { + const input = this.inputSchema.parse(rawInput); + const accessToken = + extra?.authInfo?.token || process.env.MAPBOX_ACCESS_TOKEN; + return this.execute(input, accessToken, extra); + } catch (error) { + return { + isError: true, + content: [{ type: 'text', text: (error as Error).message }] + }; + } + } + protected async execute( input: StyleComparisonInput, - serverAccessToken?: string + serverAccessToken?: string, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + rawExtra?: any ): Promise { + const extra: ToolCallExtra | undefined = rawExtra; let publicToken: string; // Step 1: Determine which token to use for the comparison @@ -127,22 +159,19 @@ export class StyleComparisonTool extends BaseTool< }; } - // Check for stored preview token (unless user wants to use custom AND the - // client can actually act on that — a client with no elicitation support - // can't honor useCustomToken anyway, so silently reuse the cache instead - // of forcing an avoidable error). - const clientSupportsElicitation = Boolean( - this.server?.server.getClientCapabilities()?.elicitation - ); const cacheKey = cacheKeyFor(serverAccessToken || ''); const storedToken = previewTokenStorage.get(cacheKey); - if ( - storedToken && - (!input.useCustomToken || !clientSupportsElicitation) - ) { + + // Reuse the cached token unless the caller explicitly wants to choose a + // different one — in which case we still need `extra.sendRequest` to ask. + if (storedToken && !input.useCustomToken) { publicToken = storedToken; - } else { - if (!this.server) { + } else if (!extra?.sendRequest) { + // No per-call session context to elicit through at all (e.g. invoked + // directly, outside a connected MCP session). + if (storedToken) { + publicToken = storedToken; + } else { return { isError: true, content: [ @@ -153,21 +182,7 @@ export class StyleComparisonTool extends BaseTool< ] }; } - - if (!clientSupportsElicitation) { - return { - isError: true, - content: [ - { - type: 'text', - text: - 'Preview token required but client does not support elicitation. ' + - 'Please provide an accessToken parameter directly, or use a client that supports MCP elicitation (e.g., MCP Inspector, Cursor, VS Code).' - } - ] - }; - } - + } else { // A server authenticated with a temporary tk.* token (e.g. the hosted MCP // DevKit Server) can never call the Tokens API to create a new token, so // the "create"/"auto" options are dropped from the dialog before asking. @@ -182,68 +197,103 @@ export class StyleComparisonTool extends BaseTool< ) : []; - const elicited = await elicitPreviewToken( - this.server.server, - existingTokens, - canCreateTokens - ); - - if (elicited.choice === 'provide') { - if (!elicited.token) { - return { - isError: true, - content: [ - { - type: 'text', - text: 'No token provided. Please provide a valid public token.' - } - ] - }; - } - publicToken = elicited.token; - } else if (elicited.choice === 'create') { - const created = await createPreviewToken( - this.httpRequest, - MapboxApiBasedTool.mapboxApiEndpoint, - serverAccessToken!, - userName, - elicited.tokenNote, - elicited.urlRestrictions + try { + // Elicit token choice from user, over *this* call's own connection. + const elicited = await elicitPreviewToken( + extra.sendRequest, + existingTokens, + canCreateTokens ); - if (!created.success) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Failed to create token: ${created.error}` - } - ] - }; + + if (elicited.choice === 'provide') { + if (!elicited.token) { + return { + isError: true, + content: [ + { + type: 'text', + text: 'No token provided. Please provide a valid public token.' + } + ] + }; + } + publicToken = elicited.token; + } else if (elicited.choice === 'create') { + const created = await createPreviewToken( + this.httpRequest, + MapboxApiBasedTool.mapboxApiEndpoint, + serverAccessToken!, + userName, + elicited.tokenNote, + elicited.urlRestrictions + ); + if (!created.success) { + return { + isError: true, + content: [ + { + type: 'text', + text: `Failed to create token: ${created.error}` + } + ] + }; + } + publicToken = created.token!; + } else { + const created = await createPreviewToken( + this.httpRequest, + MapboxApiBasedTool.mapboxApiEndpoint, + serverAccessToken!, + userName + ); + if (!created.success) { + return { + isError: true, + content: [ + { + type: 'text', + text: `Failed to auto-create token: ${created.error}` + } + ] + }; + } + publicToken = created.token!; } - publicToken = created.token!; - } else { - const created = await createPreviewToken( - this.httpRequest, - MapboxApiBasedTool.mapboxApiEndpoint, - serverAccessToken!, - userName - ); - if (!created.success) { + + previewTokenStorage.set(cacheKey, publicToken); + } catch (error) { + if (error instanceof ElicitationUnavailableError) { + // The client can't be asked at all (e.g. no elicitation support). Fall + // back to a cached token if one exists rather than failing outright. + if (storedToken) { + publicToken = storedToken; + } else { + return { + isError: true, + content: [ + { + type: 'text', + text: + 'Preview token required but client does not support elicitation. ' + + 'Please provide an accessToken parameter directly, or use a client that supports MCP elicitation (e.g., MCP Inspector, Cursor, VS Code).' + } + ] + }; + } + } else { + // The client was asked and the user declined/cancelled, or something + // else went wrong — surface it rather than silently reusing a cache. return { isError: true, content: [ { type: 'text', - text: `Failed to auto-create token: ${created.error}` + text: error instanceof Error ? error.message : String(error) } ] }; } - publicToken = created.token!; } - - previewTokenStorage.set(cacheKey, publicToken); } } diff --git a/src/utils/tokenElicitation.ts b/src/utils/tokenElicitation.ts index b380ed2..17eac4e 100644 --- a/src/utils/tokenElicitation.ts +++ b/src/utils/tokenElicitation.ts @@ -2,9 +2,28 @@ // Licensed under the MIT License. import { createHash } from 'node:crypto'; -import { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import { ElicitResultSchema } from '@modelcontextprotocol/sdk/types.js'; +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol.js'; import type { HttpRequest } from './types.js'; +/** + * The per-call `sendRequest` a tool receives via `RequestHandlerExtra` — bound + * correctly to whichever session actually made the current call, unlike a `Server` + * instance stashed on `this` (see {@link elicitPreviewToken} for why that matters). + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type SendRequest = RequestHandlerExtra['sendRequest']; + +/** + * Thrown when the elicitation request itself could not be delivered or answered — + * most commonly because the connected client doesn't implement elicitation at all, so + * `sendRequest` rejects (e.g. with a "method not found" style protocol error) rather + * than resolving with a real `ElicitResult`. Distinguished from a normal decline/cancel + * (a *successful* response the user just said no to) so callers can fall back to a + * cached token instead of surfacing a scary error for something the user never saw. + */ +export class ElicitationUnavailableError extends Error {} + /** * Token choice options for preview token elicitation */ @@ -62,7 +81,22 @@ export interface ExistingTokenInfo { * Elicits preview token information from the user via MCP elicitation. * This keeps the token out of chat history for better security. * - * @param server - MCP Server instance + * Takes the per-call `extra.sendRequest` from `RequestHandlerExtra`, not a `Server` + * instance. `Server#elicitInput()` would be the obvious choice, but it reads from + * `this` — and a tool that stashes its `Server` on `this.server` in `installTo()` and + * reads it back later in `execute()` is reading *shared, mutable* state: if the same + * tool instance is ever reused across multiple concurrent sessions (singleton tool + * instances installed onto a new session's server on every connection — exactly what + * `CORE_TOOLS` are, and what mcp-server's own scripts/dev-http-server.ts and + * hosted-mcp-server's request handling both do), `this.server` durably points at + * whichever session connected *last*, not whichever session is making *this* call. + * That sends the "paste your token" prompt to a different, uninvolved client, and + * whatever it submits comes back as this call's result — a real cross-session + * hijack, not a hypothetical. `extra.sendRequest` is supplied fresh per call by the + * SDK, correctly scoped to the session that made the current request, so it can't be + * clobbered by another session connecting in between. + * + * @param sendRequest - The current call's `extra.sendRequest` * @param existingTokens - List of user's existing public tokens * @param canCreateTokens - Whether the server's own access token is able to create * new tokens. When false (the server is authenticated with a `tk.*` temporary @@ -70,9 +104,12 @@ export interface ExistingTokenInfo { * omitted from the dialog entirely, since selecting them would only fail against * the Mapbox API. Defaults to `true` for callers that haven't checked. * @returns Elicited token information based on user's choice + * @throws {ElicitationUnavailableError} if the client can't be asked at all (e.g. it + * doesn't implement elicitation) + * @throws {Error} if the client was asked but the user declined or cancelled */ export async function elicitPreviewToken( - server: Server, + sendRequest: SendRequest, existingTokens: ExistingTokenInfo[], canCreateTokens = true ): Promise { @@ -99,48 +136,64 @@ export async function elicitPreviewToken( : "This server is authenticated with a temporary session token, which can't create new " + 'Mapbox tokens. Paste an existing public token (pk.*) with styles:read scope below.'; - const result = await server.elicitInput({ - message: `Preview Token Setup + // Mirrors what Server#elicitInput() builds internally (method + form-mode params), + // but sent via the current call's own sendRequest rather than a stashed Server. + let result; + try { + result = await sendRequest( + { + method: 'elicitation/create', + params: { + mode: 'form', + message: `Preview Token Setup Preview URLs require a public token with styles:read scope. This token will be visible in the preview URL. ${hasExistingTokens ? 'Your existing public tokens:\n' + tokenList : tokenList} ${creationNote}`, - requestedSchema: { - type: 'object', - properties: { - choice: { - type: 'string', - title: 'Token Option', - description: 'How would you like to provide the preview token?', - enum: choices, - enumNames: choiceNames - }, - token: { - type: 'string', - title: 'Your Token', - description: - 'Paste your public Mapbox token here (must have styles:read scope)', - minLength: 10 - }, - tokenNote: { - type: 'string', - title: 'Token Name (Optional)', - description: - 'A descriptive name for your new token (e.g., "Preview Token - Production")', - maxLength: 256 - }, - urlRestrictions: { - type: 'string', - title: 'URL Restrictions (Optional)', - description: - 'Comma-separated URLs to restrict token usage (e.g., "https://yourdomain.com/*,https://staging.yourdomain.com/*")' + requestedSchema: { + type: 'object', + properties: { + choice: { + type: 'string', + title: 'Token Option', + description: 'How would you like to provide the preview token?', + enum: choices, + enumNames: choiceNames + }, + token: { + type: 'string', + title: 'Your Token', + description: + 'Paste your public Mapbox token here (must have styles:read scope)', + minLength: 10 + }, + tokenNote: { + type: 'string', + title: 'Token Name (Optional)', + description: + 'A descriptive name for your new token (e.g., "Preview Token - Production")', + maxLength: 256 + }, + urlRestrictions: { + type: 'string', + title: 'URL Restrictions (Optional)', + description: + 'Comma-separated URLs to restrict token usage (e.g., "https://yourdomain.com/*,https://staging.yourdomain.com/*")' + } + }, + required: ['choice'] + } } }, - required: ['choice'] - } - }); + ElicitResultSchema + ); + } catch (error) { + throw new ElicitationUnavailableError( + error instanceof Error ? error.message : String(error) + ); + } // Check if user accepted or declined if (result.action !== 'accept' || !result.content) { diff --git a/test/security/cross-session-elicitation-hijack.test.ts b/test/security/cross-session-elicitation-hijack.test.ts new file mode 100644 index 0000000..e336201 --- /dev/null +++ b/test/security/cross-session-elicitation-hijack.test.ts @@ -0,0 +1,243 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +/** + * Regression test for a cross-session elicitation hijack. + * + * `BaseTool.installTo(server)` does `this.server = server` — mutable state on the tool + * object itself. `CORE_TOOLS` (src/tools/toolRegistry.ts) instantiates tools once as + * module-level singletons, and any embedder that reuses those singletons across + * multiple concurrent sessions (calling `installTo()` again for each new session — the + * pattern mcp-server's own scripts/dev-http-server.ts uses, and that hosted-mcp-server's + * dynamic `import()` caching produces too) clobbers `this.server` on every new + * connection. + * + * Before this PR, `this.server` was only ever read for logging. PreviewStyleTool / + * StyleComparisonTool's elicitation flow is the first thing that reads it for something + * session-sensitive: `elicitPreviewToken(this.server.server, ...)`. No race or timing + * window is even needed to trigger it — `this.server` is durably overwritten by whichever + * session's `installTo()` ran most recently, and stays that way until another session + * connects. So once session B connects (after session A), *every* subsequent tool call + * on the shared instance — including one made over session A's own, already-established + * connection — sends its elicitation request to session B instead. An uninvolved client + * gets an unprompted "paste your token" dialog for a tool call it never made, and + * whatever it submits comes back as the *other* session's tool result: an + * unprompted-dialog-injection + credential-exfiltration primitive, not a hypothetical. + */ + +import { createServer, type IncomingMessage, type Server } from 'node:http'; +import type { AddressInfo } from 'node:net'; +import { randomUUID } from 'node:crypto'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; +import { + ElicitRequestSchema, + type ElicitRequest, + type ElicitResult +} from '@modelcontextprotocol/sdk/types.js'; +import { PreviewStyleTool } from '../../src/tools/preview-style-tool/PreviewStyleTool.js'; +import { previewTokenStorage } from '../../src/utils/tokenElicitation.js'; +import type { HttpRequest } from '../../src/utils/types.js'; + +// Non-tk.* tokens so `canCreateTokens` is true and the code actually awaits +// `listPublicPreviewTokens` before reaching the elicitation call. +const SESSION_A_TOKEN = + 'sk.eyJ1IjoidGVzdC11c2VyLWEiLCJhIjoidGVzdC1hcGkifQ.signature-a'; +const SESSION_B_TOKEN = + 'sk.eyJ1IjoidGVzdC11c2VyLWIiLCJhIjoidGVzdC1hcGkifQ.signature-b'; + +// Realistic pk..sig shape — PreviewStyleTool decodes the `u` claim +// out of whichever token elicitation returns to build the preview URL, so these need to +// parse cleanly for the test to observe which one actually made it into the result. +const SESSION_A_SUPPLIED_TOKEN = 'pk.eyJ1IjoiYXR0YWNrZXItYWNjb3VudCJ9.sig-a'; +const SESSION_B_SUPPLIED_TOKEN = 'pk.eyJ1IjoidmljdGltLWFjY291bnQifQ.sig-b'; + +interface Harness { + baseUrl: URL; + close(): Promise; +} + +/** + * Installs a *shared* `PreviewStyleTool` instance onto a fresh `McpServer` for every + * new session — mirroring how CORE_TOOLS' singletons get reused across sessions in a + * real multi-tenant deployment. Session-scoped (one transport per Mcp-Session-Id), not + * the fully-stateless-per-request pattern, so this isn't about the separate capability- + * negotiation issue documented in test/integration/elicitationOverHttp.test.ts — this + * harness's whole point is that both sessions' capability negotiation works correctly, + * and the tool still sends the request to the wrong one. + */ +function startSharedSingletonHarness( + previewTool: PreviewStyleTool +): Promise { + const sessions = new Map(); + + async function buildTransport(): Promise { + const mcpServer = new McpServer( + { name: 'cross-session-hijack-test', version: '1.0.0' }, + { capabilities: { tools: { listChanged: true } } } + ); + // The exact line under test: reusing one tool instance across sessions. + previewTool.installTo(mcpServer); + + const transport = new StreamableHTTPServerTransport({ + sessionIdGenerator: () => randomUUID(), + onsessioninitialized: (sessionId) => { + sessions.set(sessionId, transport); + }, + onsessionclosed: (sessionId) => { + sessions.delete(sessionId); + } + }); + transport.onclose = () => { + if (transport.sessionId) sessions.delete(transport.sessionId); + }; + await mcpServer.connect(transport); + return transport; + } + + const httpServer: Server = createServer((req, res) => { + void (async () => { + const authHeader = req.headers.authorization; + const match = authHeader?.match(/^Bearer (.+)$/); + const reqWithAuth = req as IncomingMessage & { + auth?: { token: string; clientId: string; scopes: string[] }; + }; + if (match) { + reqWithAuth.auth = { + token: match[1], + clientId: 'test-client', + scopes: [] + }; + } + + const sessionIdHeader = req.headers['mcp-session-id']; + const existing = + typeof sessionIdHeader === 'string' + ? sessions.get(sessionIdHeader) + : undefined; + const transport = existing ?? (await buildTransport()); + + try { + await transport.handleRequest(reqWithAuth, res); + } catch (error) { + if (!res.headersSent) { + res.writeHead(500).end(String(error)); + } + } + })(); + }); + + return new Promise((resolve) => { + httpServer.listen(0, '127.0.0.1', () => { + const { port } = httpServer.address() as AddressInfo; + resolve({ + baseUrl: new URL(`http://127.0.0.1:${port}/mcp`), + close: () => + new Promise((res, rej) => + httpServer.close((err) => (err ? rej(err) : res())) + ) + }); + }); + }); +} + +async function connectClient( + baseUrl: URL, + bearerToken: string, + onElicit: (request: ElicitRequest) => ElicitResult +): Promise { + const client = new Client( + { name: 'cross-session-hijack-test-client', version: '1.0.0' }, + { capabilities: { elicitation: {} } } + ); + client.setRequestHandler(ElicitRequestSchema, (request) => onElicit(request)); + + const transport = new StreamableHTTPClientTransport(baseUrl, { + requestInit: { headers: { Authorization: `Bearer ${bearerToken}` } } + }); + await client.connect(transport); + return client; +} + +describe('cross-session elicitation hijack (singleton tool instance reused across sessions)', () => { + let harness: Harness | undefined; + let clientA: Client | undefined; + let clientB: Client | undefined; + + beforeEach(() => { + previewTokenStorage.clearAll(); + }); + + afterEach(async () => { + await clientA?.close().catch(() => {}); + await clientB?.close().catch(() => {}); + await harness?.close(); + clientA = undefined; + clientB = undefined; + harness = undefined; + }); + + it("does not send session A's elicitation prompt to session B, when B connected more recently and B's installTo() call is the last one to touch the shared tool's this.server", async () => { + const httpRequest: HttpRequest = vi.fn( + async () => new Response(JSON.stringify([]), { status: 200 }) + ) as unknown as HttpRequest; + + // ONE shared instance, installed onto two different sessions below — this is the + // exact shape of CORE_TOOLS being reused across concurrent sessions. + const previewTool = new PreviewStyleTool({ httpRequest }); + + harness = await startSharedSingletonHarness(previewTool); + + const elicitReceivedByA = vi.fn().mockReturnValue({ + action: 'accept', + content: { choice: 'provide', token: SESSION_A_SUPPLIED_TOKEN } + }); + const elicitReceivedByB = vi.fn().mockReturnValue({ + action: 'accept', + content: { choice: 'provide', token: SESSION_B_SUPPLIED_TOKEN } + }); + + // Session A connects and installs the shared tool onto its own McpServer — + // `this.server` points at session A's server at this instant. + clientA = await connectClient( + harness.baseUrl, + SESSION_A_TOKEN, + elicitReceivedByA + ); + + // Session B connects afterward, reusing the SAME `previewTool` instance. Its + // installTo() call overwrites `this.server` to session B's server — durably, not + // just for a brief race window. No timing/interleaving is needed for this to + // matter: it stays this way until a third session connects. + clientB = await connectClient( + harness.baseUrl, + SESSION_B_TOKEN, + elicitReceivedByB + ); + + // Session A now calls the tool over its own, already-established connection — + // it never calls installTo() again, so this doesn't touch `this.server`. Whatever + // is currently in `this.server` (session B's, from the previous step) is what the + // shared tool instance will use for elicitation, regardless of which session's + // request is actually being handled. + const resultForSessionA = await clientA.callTool({ + name: 'preview_style_tool', + arguments: { styleId: 'session-a-style' } + }); + + // The fix: session A's own client must be the one asked, regardless of what + // installTo() calls happened on the shared tool instance in the meantime. + expect(elicitReceivedByA).toHaveBeenCalledTimes(1); + expect(elicitReceivedByB).not.toHaveBeenCalled(); + + // And the result that comes back for session A's tool call must reflect session + // A's own answer, not whatever an uninvolved session happened to submit. + const text = ( + resultForSessionA.content as Array<{ type: string; text?: string }> + )[0].text as string; + expect(text).toContain(`access_token=${SESSION_A_SUPPLIED_TOKEN}`); + }); +}); diff --git a/test/tools/preview-style-tool/PreviewStyleTool.test.ts b/test/tools/preview-style-tool/PreviewStyleTool.test.ts index 7107e30..6d5bd08 100644 --- a/test/tools/preview-style-tool/PreviewStyleTool.test.ts +++ b/test/tools/preview-style-tool/PreviewStyleTool.test.ts @@ -257,18 +257,13 @@ describe('PreviewStyleTool', () => { const { httpRequest, mockHttpRequest } = setupHttpRequest(); const tool = new PreviewStyleTool({ httpRequest }); - const elicitInput = vi.fn().mockResolvedValue({ + // The per-call sendRequest a real MCP session would pass via `extra` — + // not a stashed `this.server`, which a singleton tool instance can't + // safely rely on across sessions (see tokenElicitation.ts). + const sendRequest = vi.fn().mockResolvedValue({ action: 'accept', content: { choice: 'provide', token: TEST_ACCESS_TOKEN } }); - // Simulate what BaseTool#installTo does, without a full MCP server. - tool['server'] = { - server: { - getClientCapabilities: () => ({ elicitation: {} }), - elicitInput - } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any; const tkToken = 'tk.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; @@ -276,12 +271,13 @@ describe('PreviewStyleTool', () => { const result = await tool.run( { styleId: 'test-style' }, // eslint-disable-next-line @typescript-eslint/no-explicit-any - { authInfo: { token: tkToken } } as any + { authInfo: { token: tkToken }, sendRequest } as any ); expect(result.isError).toBe(false); - expect(elicitInput).toHaveBeenCalledTimes(1); - const requestedSchema = elicitInput.mock.calls[0][0].requestedSchema; + expect(sendRequest).toHaveBeenCalledTimes(1); + const requestedSchema = + sendRequest.mock.calls[0][0].params.requestedSchema; expect(requestedSchema.properties.choice.enum).toEqual(['provide']); // A tk.* server token can never create tokens, so listing/creating diff --git a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts index eef2f43..9eec113 100644 --- a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts +++ b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts @@ -322,18 +322,13 @@ describe('StyleComparisonTool', () => { const { httpRequest, mockHttpRequest } = setupHttpRequest(); const tool = new StyleComparisonTool({ httpRequest }); - const elicitInput = vi.fn().mockResolvedValue({ + // The per-call sendRequest a real MCP session would pass via `extra` — + // not a stashed `this.server`, which a singleton tool instance can't + // safely rely on across sessions (see tokenElicitation.ts). + const sendRequest = vi.fn().mockResolvedValue({ action: 'accept', content: { choice: 'provide', token: 'pk.test.token' } }); - // Simulate what BaseTool#installTo does, without a full MCP server. - tool['server'] = { - server: { - getClientCapabilities: () => ({ elicitation: {} }), - elicitInput - } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any; const tkToken = 'tk.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; @@ -341,12 +336,13 @@ describe('StyleComparisonTool', () => { const result = await tool.run( { before: 'mapbox/streets-v12', after: 'mapbox/satellite-v9' }, // eslint-disable-next-line @typescript-eslint/no-explicit-any - { authInfo: { token: tkToken } } as any + { authInfo: { token: tkToken }, sendRequest } as any ); expect(result.isError).toBe(false); - expect(elicitInput).toHaveBeenCalledTimes(1); - const requestedSchema = elicitInput.mock.calls[0][0].requestedSchema; + expect(sendRequest).toHaveBeenCalledTimes(1); + const requestedSchema = + sendRequest.mock.calls[0][0].params.requestedSchema; expect(requestedSchema.properties.choice.enum).toEqual(['provide']); // A tk.* server token can never create tokens, so listing/creating diff --git a/test/utils/tokenElicitation.test.ts b/test/utils/tokenElicitation.test.ts index c463408..cf23f56 100644 --- a/test/utils/tokenElicitation.test.ts +++ b/test/utils/tokenElicitation.test.ts @@ -2,10 +2,10 @@ // Licensed under the MIT License. import { describe, it, expect, beforeEach, vi } from 'vitest'; -import type { Server } from '@modelcontextprotocol/sdk/server/index.js'; import { cacheKeyFor, createPreviewToken, + ElicitationUnavailableError, elicitPreviewToken, isTemporaryServerToken, listPublicPreviewTokens, @@ -263,21 +263,19 @@ describe('listPublicPreviewTokens', () => { }); describe('elicitPreviewToken', () => { - function fakeServer(choice: string) { - const elicitInput = vi.fn().mockResolvedValue({ + function fakeSendRequest(choice: string) { + return vi.fn().mockResolvedValue({ action: 'accept', content: { choice, token: 'pk.provided-token' } }); - return { elicitInput } as unknown as Server; } it('offers all three choices when the server token can create tokens', async () => { - const server = fakeServer('provide'); - await elicitPreviewToken(server, [], true); + const sendRequest = fakeSendRequest('provide'); + await elicitPreviewToken(sendRequest, [], true); - const request = (server.elicitInput as ReturnType).mock - .calls[0][0]; - expect(request.requestedSchema.properties.choice.enum).toEqual([ + const request = sendRequest.mock.calls[0][0]; + expect(request.params.requestedSchema.properties.choice.enum).toEqual([ 'provide', 'create', 'auto' @@ -285,21 +283,31 @@ describe('elicitPreviewToken', () => { }); it('omits create/auto choices when the server token cannot create tokens', async () => { - const server = fakeServer('provide'); - await elicitPreviewToken(server, [], false); + const sendRequest = fakeSendRequest('provide'); + await elicitPreviewToken(sendRequest, [], false); - const request = (server.elicitInput as ReturnType).mock - .calls[0][0]; - expect(request.requestedSchema.properties.choice.enum).toEqual(['provide']); - expect(request.message).toContain('temporary session token'); + const request = sendRequest.mock.calls[0][0]; + expect(request.params.requestedSchema.properties.choice.enum).toEqual([ + 'provide' + ]); + expect(request.params.message).toContain('temporary session token'); }); it('throws when the user declines elicitation', async () => { - const elicitInput = vi.fn().mockResolvedValue({ action: 'decline' }); - const server = { elicitInput } as unknown as Server; + const sendRequest = vi.fn().mockResolvedValue({ action: 'decline' }); - await expect(elicitPreviewToken(server, [], true)).rejects.toThrow( + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( 'Token elicitation was cancelled or declined by user' ); }); + + it('wraps a failed sendRequest (e.g. the client has no elicitation support) in ElicitationUnavailableError', async () => { + const sendRequest = vi + .fn() + .mockRejectedValue(new Error('Method not found')); + + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( + ElicitationUnavailableError + ); + }); }); From dbfcea2f6001ab98e0ddb564823a7b83ebf1f2ac Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Wed, 5 Aug 2026 10:49:34 -0400 Subject: [PATCH 20/25] Add CHANGELOG entry for cross-session elicitation hijack fix --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d0aaee..5a96de2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ - **Secure token elicitation for `preview_style_tool` / `style_comparison_tool`** (#57): `accessToken` is now optional on both tools. When omitted, the tool uses MCP elicitation to ask for a public token instead — either pasting an existing `pk.*` token, creating a new one with optional URL restrictions, or auto-creating a minimally-scoped one (`styles:read`, `styles:tiles`, `fonts:read`). This keeps your server's `sk.*`/`pk.*` access token out of chat history and preview URLs. Falls back to requiring `accessToken` directly on clients without elicitation support (Claude Desktop, Claude Code). A chosen/created token is cached in memory per account for the session; pass `useCustomToken: true` to force re-selection. - If the server's own access token is a literal Mapbox temporary token (`tk.*`), the "create a new token" and "auto-create" options are omitted from the dialog up front, since that token shape is guaranteed to lack `tokens:write`. Other callers lacking `tokens:write` (e.g. the hosted MCP endpoint, which authenticates with its own access token rather than a Mapbox `pk.*`/`sk.*`/`tk.*` token) aren't detectable ahead of time; for those, choosing "create"/"auto-create" fails against the Tokens API with a scope/permission hint appended to the error, steering back to "I have a token to provide". +### Security + +- **Cross-session elicitation hijack in `preview_style_tool` / `style_comparison_tool`** (#57): `BaseTool.installTo()` stashes the connecting session's `Server` on `this.server` — mutable state shared by the tool instance. Any deployment that reuses singleton tool instances across concurrent sessions (installing the same instance onto a new session's server on every connection) could have a tool call from one session send its "paste your token" elicitation prompt to whichever _other_ session most recently connected, with that session's response returned as the original caller's result. Fixed by routing elicitation through the per-call `extra.sendRequest` (correctly scoped to the session that made the current request) instead of the shared `this.server`. + ### Changed - **`preview_style_tool` / `style_comparison_tool`**: token-listing and token-creation HTTP calls now go through the shared `HttpPipeline` (constructor-injected `httpRequest`) instead of a bare `fetch`, consistent with the rest of the API-calling tools. From c2f49b569850a077b4ac2af33469021b5889f514 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Wed, 12 Aug 2026 10:46:03 -0400 Subject: [PATCH 21/25] Address more PR #57 review feedback: CHANGELOG framing, jwtUtils docs - CHANGELOG: fold the cross-session-hijack fix into the elicitation feature bullet instead of a standalone "### Security" entry, and fix "cached in memory per account" to "per token" (the cache key is a hash of the literal server access token, not a resolved account identity). Neither the bug nor the fix ever shipped in a released version, so a disclosure-style Security section overstated it. (Valiunia) - jwtUtils.ts: strengthened getUserNameFromToken's doc comment to explicitly call out that it doesn't verify the token signature and shouldn't be used for authorization decisions or sensitive cache keys. The function is used in 12 files across the repo, so a full rename (as suggested) is being proposed as a fast-follow rather than done here. (Valiunia) --- CHANGELOG.md | 6 +----- src/utils/jwtUtils.ts | 12 ++++++++++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ef3514..3788bd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,13 +2,9 @@ ### New Features -- **Secure token elicitation for `preview_style_tool` / `style_comparison_tool`** (#57): `accessToken` is now optional on both tools. When omitted, the tool uses MCP elicitation to ask for a public token instead — either pasting an existing `pk.*` token, creating a new one with optional URL restrictions, or auto-creating a minimally-scoped one (`styles:read`, `styles:tiles`, `fonts:read`). This keeps your server's `sk.*`/`pk.*` access token out of chat history and preview URLs. Falls back to requiring `accessToken` directly on clients without elicitation support (Claude Desktop, Claude Code). A chosen/created token is cached in memory per account for the session; pass `useCustomToken: true` to force re-selection. +- **Secure token elicitation for `preview_style_tool` / `style_comparison_tool`** (#57): `accessToken` is now optional on both tools. When omitted, the tool uses MCP elicitation to ask for a public token instead — either pasting an existing `pk.*` token, creating a new one with optional URL restrictions, or auto-creating a minimally-scoped one (`styles:read`, `styles:tiles`, `fonts:read`). This keeps your server's `sk.*`/`pk.*` access token out of chat history and preview URLs. Falls back to requiring `accessToken` directly on clients without elicitation support (Claude Desktop, Claude Code). A chosen/created token is cached in memory per token; pass `useCustomToken: true` to force re-selection. Elicitation requests are routed through the per-call session context rather than a shared server reference, so a tool call's prompt is always delivered back to the session that made it, even when the same tool instance is reused across multiple concurrent sessions. - If the server's own access token is a literal Mapbox temporary token (`tk.*`), the "create a new token" and "auto-create" options are omitted from the dialog up front, since that token shape is guaranteed to lack `tokens:write`. Other callers lacking `tokens:write` (e.g. the hosted MCP endpoint, which authenticates with its own access token rather than a Mapbox `pk.*`/`sk.*`/`tk.*` token) aren't detectable ahead of time; for those, choosing "create"/"auto-create" fails against the Tokens API with a scope/permission hint appended to the error, steering back to "I have a token to provide". -### Security - -- **Cross-session elicitation hijack in `preview_style_tool` / `style_comparison_tool`** (#57): `BaseTool.installTo()` stashes the connecting session's `Server` on `this.server` — mutable state shared by the tool instance. Any deployment that reuses singleton tool instances across concurrent sessions (installing the same instance onto a new session's server on every connection) could have a tool call from one session send its "paste your token" elicitation prompt to whichever _other_ session most recently connected, with that session's response returned as the original caller's result. Fixed by routing elicitation through the per-call `extra.sendRequest` (correctly scoped to the session that made the current request) instead of the shared `this.server`. - ### Changed - **`preview_style_tool` / `style_comparison_tool`**: token-listing and token-creation HTTP calls now go through the shared `HttpPipeline` (constructor-injected `httpRequest`) instead of a bare `fetch`, consistent with the rest of the API-calling tools. diff --git a/src/utils/jwtUtils.ts b/src/utils/jwtUtils.ts index 9a3d3e6..65ef969 100644 --- a/src/utils/jwtUtils.ts +++ b/src/utils/jwtUtils.ts @@ -12,8 +12,16 @@ export function mapboxApiEndpoint() { } /** - * Extracts the username from the Mapbox access token. - * Mapbox tokens are JWT tokens where the payload contains the username. + * Extracts the username from the Mapbox access token's `u` claim, WITHOUT verifying + * the token's signature — this only base64-decodes the JWT payload. + * + * The returned value is only as trustworthy as whatever already validated this + * token (e.g. the Mapbox API itself rejecting the token value if it's forged, or an + * upstream gateway that verifies bearers before this code ever sees them). Do not + * use this result alone for authorization decisions or as a cache key for anything + * sensitive — hash the raw token instead if you need a value tied to the specific + * credential presented (see `cacheKeyFor` in `tokenElicitation.ts` for why). + * * @throws Error if the token is not set, invalid, or doesn't contain username */ export function getUserNameFromToken(accessToken?: string): string { From 25fc3ef5112b0dd596382b015ae9222cb0553021 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Wed, 12 Aug 2026 11:16:44 -0400 Subject: [PATCH 22/25] Fix resource-exhaustion gaps in elicitation flow found in offline review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Feedback relayed via Slack from a separate Claude review session (via Valentin), verified against the actual SDK/code before acting: 1. sendRequest was called with no timeout option, so it used the SDK's DEFAULT_REQUEST_TIMEOUT_MSEC (60s) implicitly. Calling preview_style_tool with no accessToken and never answering the elicitation held that call (and its session's live connection) pending for the full duration; a flood of these ties up resources for as long as the timeout, and silently changes if the SDK's default ever changes. Now passes an explicit `timeout` matching the current default, decoupling our behavior from the SDK's constant. 2. minLength/maxLength on the elicitation dialog's requestedSchema are hints for the client's own form UI, not a security boundary — nothing enforced them server-side. A client (malicious, or just not honoring the hints) could return an arbitrarily large token/ tokenNote/urlRestrictions value, which then sat in previewTokenStorage indefinitely. Added maxLength to the schema hint and, more importantly, server-side validation of whatever comes back before it's accepted or cached. 3. (Flagged by the reviewer as "probably not an issue, but just in case"): previewTokenStorage's cache key is sha256(caller's bearer token), so minting N distinct cache entries requires N distinct bearer values. Confirmed low-risk: in stdio mode the value is one fixed MAPBOX_ACCESS_TOKEN; on the hosted deployment, hosted-mcp-server's bearerAuth middleware verifies the token before it becomes extra.authInfo.token, so an attacker would need N real, verified credentials — impractical at DoS scale (confirmed by reading hosted-mcp-server's bearerAuth.ts directly). No code change needed for this specific vector, but added a bounded LRU cap (1000 entries) to previewTokenStorage regardless, since "nothing ever evicts" is the root cause underlying all three findings and is worth fixing as basic hygiene independent of exploitability — a long-lived multi-tenant deployment accumulating real distinct users over time hits the same unbounded-growth problem with zero adversarial intent involved. Added regression tests for the timeout option, the three size-limit rejections, and LRU eviction (including that reads protect an entry from eviction). --- CHANGELOG.md | 1 + src/utils/tokenElicitation.ts | 83 +++++++++++++++++++++++++++- test/utils/tokenElicitation.test.ts | 85 +++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3788bd7..944365f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - **Secure token elicitation for `preview_style_tool` / `style_comparison_tool`** (#57): `accessToken` is now optional on both tools. When omitted, the tool uses MCP elicitation to ask for a public token instead — either pasting an existing `pk.*` token, creating a new one with optional URL restrictions, or auto-creating a minimally-scoped one (`styles:read`, `styles:tiles`, `fonts:read`). This keeps your server's `sk.*`/`pk.*` access token out of chat history and preview URLs. Falls back to requiring `accessToken` directly on clients without elicitation support (Claude Desktop, Claude Code). A chosen/created token is cached in memory per token; pass `useCustomToken: true` to force re-selection. Elicitation requests are routed through the per-call session context rather than a shared server reference, so a tool call's prompt is always delivered back to the session that made it, even when the same tool instance is reused across multiple concurrent sessions. - If the server's own access token is a literal Mapbox temporary token (`tk.*`), the "create a new token" and "auto-create" options are omitted from the dialog up front, since that token shape is guaranteed to lack `tokens:write`. Other callers lacking `tokens:write` (e.g. the hosted MCP endpoint, which authenticates with its own access token rather than a Mapbox `pk.*`/`sk.*`/`tk.*` token) aren't detectable ahead of time; for those, choosing "create"/"auto-create" fails against the Tokens API with a scope/permission hint appended to the error, steering back to "I have a token to provide". + - The elicitation request has an explicit 60s timeout rather than relying on the SDK's default; a client-returned token/token name/URL-restriction list that exceeds a sane size is rejected server-side regardless of what the elicitation dialog's schema hints suggest a client's form should enforce; and the in-memory token cache is bounded to 1000 entries (LRU eviction), so none of this can grow memory usage without limit. ### Changed diff --git a/src/utils/tokenElicitation.ts b/src/utils/tokenElicitation.ts index 17eac4e..dd8a53a 100644 --- a/src/utils/tokenElicitation.ts +++ b/src/utils/tokenElicitation.ts @@ -14,6 +14,31 @@ import type { HttpRequest } from './types.js'; // eslint-disable-next-line @typescript-eslint/no-explicit-any type SendRequest = RequestHandlerExtra['sendRequest']; +/** + * How long to wait for the client to answer an elicitation prompt before giving up. + * Set explicitly (matching the SDK's own `DEFAULT_REQUEST_TIMEOUT_MSEC`) rather than + * omitting `timeout` and letting the SDK default apply implicitly — a future SDK bump + * changing that constant shouldn't silently change how long a session-holding-open + * elicitation call can be kept pending. A human genuinely filling out the dialog needs + * on the order of tens of seconds; a client that never answers at all (accidentally or + * as a resource-exhaustion attempt — each pending call ties up its session's live + * connection for the duration) shouldn't be able to hold the request open indefinitely. + * Capping the number of *concurrent* pending elicitations a single deployment will + * tolerate is an infrastructure-level concern (rate limiting, connection limits) outside + * what this package can enforce on its own. + */ +const ELICITATION_TIMEOUT_MSEC = 60_000; + +/** Real Mapbox tokens are well under this; anything longer is almost certainly not a + * token at all. Enforced server-side regardless of what a client's form UI does with + * the `maxLength` hint in the requested schema below — that hint is advisory only. */ +const MAX_TOKEN_LENGTH = 2048; + +const MAX_TOKEN_NOTE_LENGTH = 256; + +/** Matches CreateTokenTool's own `allowedUrls` cap. */ +const MAX_URL_RESTRICTIONS = 100; + /** * Thrown when the elicitation request itself could not be delivered or answered — * most commonly because the connected client doesn't implement elicitation at all, so @@ -167,7 +192,8 @@ ${creationNote}`, title: 'Your Token', description: 'Paste your public Mapbox token here (must have styles:read scope)', - minLength: 10 + minLength: 10, + maxLength: MAX_TOKEN_LENGTH }, tokenNote: { type: 'string', @@ -187,7 +213,8 @@ ${creationNote}`, } } }, - ElicitResultSchema + ElicitResultSchema, + { timeout: ELICITATION_TIMEOUT_MSEC } ); } catch (error) { throw new ElicitationUnavailableError( @@ -215,6 +242,29 @@ ${creationNote}`, .filter((url) => url.length > 0) : undefined; + // The requestedSchema's minLength/maxLength are hints for the client's own form UI, + // not a security boundary — nothing stops a client (malicious, or just not honoring + // the hints) from returning arbitrary content. Enforced here too, since whatever + // comes back as `token` ends up cached indefinitely in previewTokenStorage. + if (token !== undefined && token.length > MAX_TOKEN_LENGTH) { + throw new Error( + `Provided token is ${token.length} characters, which exceeds the ${MAX_TOKEN_LENGTH}-character maximum for a Mapbox token.` + ); + } + if (tokenNote !== undefined && tokenNote.length > MAX_TOKEN_NOTE_LENGTH) { + throw new Error( + `Token name is ${tokenNote.length} characters, which exceeds the ${MAX_TOKEN_NOTE_LENGTH}-character maximum.` + ); + } + if ( + urlRestrictions !== undefined && + urlRestrictions.length > MAX_URL_RESTRICTIONS + ) { + throw new Error( + `Provided ${urlRestrictions.length} URL restrictions, which exceeds the ${MAX_URL_RESTRICTIONS}-URL maximum.` + ); + } + return { choice, token, @@ -238,9 +288,18 @@ export function cacheKeyFor(token: string): string { return createHash('sha256').update(token).digest('hex'); } +/** Bounds previewTokenStorage's worst-case memory footprint regardless of how many + * distinct cache keys ever get presented — deliberately independent of *why* the + * count might grow (a long-lived multi-tenant deployment accumulating real distinct + * users over time is just as capable of doing this as anything adversarial). */ +const MAX_CACHED_TOKENS = 1000; + /** * Session-level storage for preview token preferences, keyed by {@link cacheKeyFor}. * In a real implementation, this could be stored in a database or cache. + * + * Bounded LRU: evicts the least-recently-used entry once at capacity, so memory usage + * has a fixed ceiling no matter how many distinct keys are ever presented. */ class PreviewTokenStorage { private tokenCache = new Map(); @@ -249,14 +308,32 @@ class PreviewTokenStorage { * Store a preview token under the given cache key */ set(cacheKey: string, token: string): void { + // Re-inserting moves a key to the end (most-recently-used) in Map's iteration + // order; delete first so an existing key doesn't just get its value updated + // in place at its old position. + this.tokenCache.delete(cacheKey); this.tokenCache.set(cacheKey, token); + + if (this.tokenCache.size > MAX_CACHED_TOKENS) { + const oldestKey = this.tokenCache.keys().next().value; + if (oldestKey !== undefined) { + this.tokenCache.delete(oldestKey); + } + } } /** * Get the stored preview token for the given cache key */ get(cacheKey: string): string | undefined { - return this.tokenCache.get(cacheKey); + const token = this.tokenCache.get(cacheKey); + if (token !== undefined) { + // Bump to most-recently-used on read too, so an actively-used entry survives + // eviction even if it was one of the first ever inserted. + this.tokenCache.delete(cacheKey); + this.tokenCache.set(cacheKey, token); + } + return token; } /** diff --git a/test/utils/tokenElicitation.test.ts b/test/utils/tokenElicitation.test.ts index cf23f56..7cc65e9 100644 --- a/test/utils/tokenElicitation.test.ts +++ b/test/utils/tokenElicitation.test.ts @@ -93,6 +93,42 @@ describe('PreviewTokenStorage', () => { previewTokenStorage.set(specialUsername, 'pk.special-token'); expect(previewTokenStorage.get(specialUsername)).toBe('pk.special-token'); }); + + it('evicts the least-recently-used entry once at capacity, bounding memory regardless of how many distinct keys are presented', () => { + const MAX_CACHED_TOKENS = 1000; // matches the private constant in tokenElicitation.ts + + for (let i = 0; i < MAX_CACHED_TOKENS; i++) { + previewTokenStorage.set(`key-${i}`, `pk.token-${i}`); + } + + // One more insert should evict the oldest (key-0), not grow unbounded. Checking + // key-0 here (rather than before this point) matters: `get()` itself counts as a + // "use" and would otherwise protect key-0 from being the next eviction target. + previewTokenStorage.set('key-overflow', 'pk.token-overflow'); + + expect(previewTokenStorage.get('key-0')).toBeUndefined(); + expect(previewTokenStorage.get('key-overflow')).toBe('pk.token-overflow'); + // The rest of the original entries are still present. + expect(previewTokenStorage.get('key-1')).toBe('pk.token-1'); + }); + + it('reading an entry protects it from eviction, even if it was inserted first', () => { + const MAX_CACHED_TOKENS = 1000; + + previewTokenStorage.set('key-0', 'pk.token-0'); + for (let i = 1; i < MAX_CACHED_TOKENS; i++) { + previewTokenStorage.set(`key-${i}`, `pk.token-${i}`); + } + + // Touch key-0 so it's no longer the least-recently-used entry. + previewTokenStorage.get('key-0'); + + // This overflow should now evict key-1 (the new least-recently-used), not key-0. + previewTokenStorage.set('key-overflow', 'pk.token-overflow'); + + expect(previewTokenStorage.get('key-0')).toBe('pk.token-0'); + expect(previewTokenStorage.get('key-1')).toBeUndefined(); + }); }); describe('isTemporaryServerToken', () => { @@ -310,4 +346,53 @@ describe('elicitPreviewToken', () => { ElicitationUnavailableError ); }); + + it('passes an explicit timeout to sendRequest rather than relying on the SDK default', async () => { + const sendRequest = fakeSendRequest('provide'); + await elicitPreviewToken(sendRequest, [], true); + + const options = sendRequest.mock.calls[0][2]; + expect(options).toMatchObject({ timeout: expect.any(Number) }); + expect(options.timeout).toBeGreaterThan(0); + }); + + it('rejects a client-returned token that exceeds the server-enforced max length, regardless of the schema hint', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + action: 'accept', + content: { choice: 'provide', token: 'pk.' + 'a'.repeat(3000) } + }); + + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( + /exceeds the .* maximum/ + ); + }); + + it('rejects a client-returned tokenNote that exceeds the max length', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + action: 'accept', + content: { + choice: 'create', + tokenNote: 'a'.repeat(300) + } + }); + + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( + /Token name.*exceeds/ + ); + }); + + it('rejects a client-returned urlRestrictions list that exceeds the max count', async () => { + const tooManyUrls = Array.from( + { length: 101 }, + (_, i) => `https://example${i}.com/*` + ).join(','); + const sendRequest = vi.fn().mockResolvedValue({ + action: 'accept', + content: { choice: 'create', urlRestrictions: tooManyUrls } + }); + + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( + /URL restrictions.*exceeds/ + ); + }); }); From de3a2ee1f5d344bdecebca4ae77079a829eb3c55 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Fri, 14 Aug 2026 10:39:22 -0400 Subject: [PATCH 23/25] Fix two security regressions and several correctness bugs in elicitation flow found in adversarial review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Feedback from an adversarial review pass, verified against the actual code before acting (each fix below has a regression test confirmed to fail against the pre-fix code): Security (both defeat this PR's own purpose): 1. A token pasted via the elicitation dialog's "provide" choice skipped the pk.* prefix check that every other path (accessToken param, create/auto-create API response) already enforces. Pasting an sk.* token there embedded it straight into the returned preview URL and cached it for reuse. Fixed in elicitPreviewToken(), which is now the single place all three paths converge for this check. 2. createPreviewToken()'s returned error field (not a thrown exception, so MapboxApiBasedTool's automatic redaction wouldn't have covered it even if these tools extended that class) could contain the caller's own access token verbatim, e.g. a network-level failure throwing with the full request URL. Now passed through redactToken() before being returned. Correctness bugs in the same new code, all confirmed by reading the code before fixing: - An unrecognized `choice` value from the client fell through to "auto-create a real token" with no membership check against the options actually offered. - token/tokenNote/urlRestrictions were cast from the elicitation response without runtime type checks: a non-string token silently bypassed the length guard, and a non-string urlRestrictions reached `.split()` and threw an unclassified TypeError instead of a clear error. - Timeouts and client cancellations (McpError RequestTimeout / cancelled-InvalidRequest) were lumped in with "client doesn't support elicitation", so `useCustomToken: true` (meant to force re-selection) could silently return the stale cached token instead of erroring. - The tokens:write capability check (canCreateTokens) incorrectly also gated the tokens:read listing call, so tk.*-authenticated servers always claimed "no public tokens found" even when tokens existed. Listing is a separate, more permissive scope and already fails safe to an empty list on its own. - The urlRestrictions schema field had no maxLength hint or raw-string length check before `.split()` — the array-length cap alone didn't stop one arbitrarily large unsplit string. - createPreviewToken() cast the Tokens API's 2xx response body without checking `token` was actually a string first. Deliberately not addressed here, each already a pre-existing or higher-effort concern rather than something this PR's own code introduced in isolation: - No de-dup on concurrent cache misses (two calls racing on the same credential can both mint a token). Real, but low severity (an extra orphaned token, not a leak) and elicitation's human-in-the-loop nature makes proper request coalescing a bigger change than proportionate here. - The shared RetryPolicy retries POSTs with no idempotency key, which can mint duplicate tokens on a lost 502/504. Pre-existing, systemic behavior of HttpPipeline's retry policy shared by every tool that POSTs (e.g. create-token-tool already has the identical exposure) -- not specific to this PR's code. - The regression tests (including the cross-session-hijack test) use a stateful session harness; the actual hosted deployment is stateless per-request. The fix likely still holds structurally, but building a stateless-transport-shaped harness is a separate, larger effort. Regenerated CHANGELOG's existing elicitation feature entry rather than adding a disclosure-style Security section, since none of this has shipped in a released version yet. --- CHANGELOG.md | 4 + .../preview-style-tool/PreviewStyleTool.ts | 20 +- .../StyleComparisonTool.ts | 19 +- src/utils/tokenElicitation.ts | 115 +++++++++-- test/integration/elicitationOverHttp.test.ts | 20 +- .../PreviewStyleTool.test.ts | 9 +- .../StyleComparisonTool.test.ts | 9 +- test/utils/tokenElicitation.test.ts | 188 ++++++++++++++++++ 8 files changed, 340 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 944365f..bcec095 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ - **Secure token elicitation for `preview_style_tool` / `style_comparison_tool`** (#57): `accessToken` is now optional on both tools. When omitted, the tool uses MCP elicitation to ask for a public token instead — either pasting an existing `pk.*` token, creating a new one with optional URL restrictions, or auto-creating a minimally-scoped one (`styles:read`, `styles:tiles`, `fonts:read`). This keeps your server's `sk.*`/`pk.*` access token out of chat history and preview URLs. Falls back to requiring `accessToken` directly on clients without elicitation support (Claude Desktop, Claude Code). A chosen/created token is cached in memory per token; pass `useCustomToken: true` to force re-selection. Elicitation requests are routed through the per-call session context rather than a shared server reference, so a tool call's prompt is always delivered back to the session that made it, even when the same tool instance is reused across multiple concurrent sessions. - If the server's own access token is a literal Mapbox temporary token (`tk.*`), the "create a new token" and "auto-create" options are omitted from the dialog up front, since that token shape is guaranteed to lack `tokens:write`. Other callers lacking `tokens:write` (e.g. the hosted MCP endpoint, which authenticates with its own access token rather than a Mapbox `pk.*`/`sk.*`/`tk.*` token) aren't detectable ahead of time; for those, choosing "create"/"auto-create" fails against the Tokens API with a scope/permission hint appended to the error, steering back to "I have a token to provide". - The elicitation request has an explicit 60s timeout rather than relying on the SDK's default; a client-returned token/token name/URL-restriction list that exceeds a sane size is rejected server-side regardless of what the elicitation dialog's schema hints suggest a client's form should enforce; and the in-memory token cache is bounded to 1000 entries (LRU eviction), so none of this can grow memory usage without limit. + - A token pasted into the elicitation dialog's "I have a token to provide" option is now checked for the `pk.*` prefix, same as every other path that produces a preview token. It previously wasn't, so a secret token (`sk.*`) submitted that way was embedded straight into the returned preview URL and cached for reuse — the exact leak this feature exists to prevent. Found by an adversarial review pass. + - Every field the client returns from the elicitation dialog (choice, token, token name, URL restrictions) is now validated against its expected runtime type and, for `choice`, against the set of options actually offered — a client returning an unrecognized or wrong-typed value previously either fell through to auto-creating a real token or threw an unclassified `TypeError`. A request timeout or cancellation is now also distinguished from "client doesn't support elicitation", so a caller passing `useCustomToken: true` gets a real error instead of silently getting back the stale cached token it was trying to replace. + - Listing a user's existing public tokens (to populate the dialog) only requires `tokens:read`, a separate scope from the `tokens:write` needed to create one — it's no longer skipped for `tk.*`-authenticated servers, which lack the latter but not necessarily the former. + - Error messages from a failed token-creation call are now redacted the same way every other Mapbox API tool's errors are, since these two tools don't extend the base class that applies that redaction automatically. A misconfigured endpoint could previously surface the caller's own access token verbatim in the returned error text. ### Changed diff --git a/src/tools/preview-style-tool/PreviewStyleTool.ts b/src/tools/preview-style-tool/PreviewStyleTool.ts index aec8456..62ff446 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.ts @@ -139,15 +139,17 @@ export class PreviewStyleTool extends BaseTool { // the "create"/"auto" options are dropped from the dialog before asking. const canCreateTokens = !isTemporaryServerToken(serverAccessToken!); - // Get existing public tokens to show user - const existingTokens = canCreateTokens - ? await listPublicPreviewTokens( - this.httpRequest, - MapboxApiBasedTool.mapboxApiEndpoint, - serverAccessToken!, - userName - ) - : []; + // Listing only needs `tokens:read`, a separate scope from the `tokens:write` + // that canCreateTokens checks — a tk.*-authenticated server lacking the + // latter isn't thereby known to lack the former too, so this isn't gated on + // canCreateTokens. The call already fails safe to an empty list on any + // API/permission error. + const existingTokens = await listPublicPreviewTokens( + this.httpRequest, + MapboxApiBasedTool.mapboxApiEndpoint, + serverAccessToken!, + userName + ); try { // Elicit token choice from user, over *this* call's own connection. diff --git a/src/tools/style-comparison-tool/StyleComparisonTool.ts b/src/tools/style-comparison-tool/StyleComparisonTool.ts index 5a72326..0452719 100644 --- a/src/tools/style-comparison-tool/StyleComparisonTool.ts +++ b/src/tools/style-comparison-tool/StyleComparisonTool.ts @@ -188,14 +188,17 @@ export class StyleComparisonTool extends BaseTool< // the "create"/"auto" options are dropped from the dialog before asking. const canCreateTokens = !isTemporaryServerToken(serverAccessToken!); - const existingTokens = canCreateTokens - ? await listPublicPreviewTokens( - this.httpRequest, - MapboxApiBasedTool.mapboxApiEndpoint, - serverAccessToken!, - userName - ) - : []; + // Listing only needs `tokens:read`, a separate scope from the `tokens:write` + // that canCreateTokens checks — a tk.*-authenticated server lacking the + // latter isn't thereby known to lack the former too, so this isn't gated on + // canCreateTokens. The call already fails safe to an empty list on any + // API/permission error. + const existingTokens = await listPublicPreviewTokens( + this.httpRequest, + MapboxApiBasedTool.mapboxApiEndpoint, + serverAccessToken!, + userName + ); try { // Elicit token choice from user, over *this* call's own connection. diff --git a/src/utils/tokenElicitation.ts b/src/utils/tokenElicitation.ts index dd8a53a..679cafa 100644 --- a/src/utils/tokenElicitation.ts +++ b/src/utils/tokenElicitation.ts @@ -2,8 +2,13 @@ // Licensed under the MIT License. import { createHash } from 'node:crypto'; -import { ElicitResultSchema } from '@modelcontextprotocol/sdk/types.js'; +import { + ElicitResultSchema, + ErrorCode, + McpError +} from '@modelcontextprotocol/sdk/types.js'; import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol.js'; +import { redactToken } from '../tools/MapboxApiBasedTool.js'; import type { HttpRequest } from './types.js'; /** @@ -39,6 +44,12 @@ const MAX_TOKEN_NOTE_LENGTH = 256; /** Matches CreateTokenTool's own `allowedUrls` cap. */ const MAX_URL_RESTRICTIONS = 100; +/** Bounds the raw comma-separated `urlRestrictions` string before it's split into an + * array. The array-length cap above only limits the *parsed* result — nothing stops a + * client from returning one unsplit string far larger than 100 short URLs would ever + * require, so this is enforced independently and before the `.split()` call. */ +const MAX_URL_RESTRICTIONS_RAW_LENGTH = 4096; + /** * Thrown when the elicitation request itself could not be delivered or answered — * most commonly because the connected client doesn't implement elicitation at all, so @@ -206,7 +217,8 @@ ${creationNote}`, type: 'string', title: 'URL Restrictions (Optional)', description: - 'Comma-separated URLs to restrict token usage (e.g., "https://yourdomain.com/*,https://staging.yourdomain.com/*")' + 'Comma-separated URLs to restrict token usage (e.g., "https://yourdomain.com/*,https://staging.yourdomain.com/*")', + maxLength: MAX_URL_RESTRICTIONS_RAW_LENGTH } }, required: ['choice'] @@ -217,6 +229,20 @@ ${creationNote}`, { timeout: ELICITATION_TIMEOUT_MSEC } ); } catch (error) { + if ( + error instanceof McpError && + (error.code === ErrorCode.RequestTimeout || + (error.code === ErrorCode.InvalidRequest && + /cancelled/i.test(error.message))) + ) { + // The client understood the request and either never answered it in time, or + // the caller aborted it — distinct from a client that doesn't implement + // elicitation at all. Re-thrown as-is (not wrapped as "unavailable") so callers + // surface it instead of silently falling back to a stale cached token: a caller + // passing `useCustomToken: true` explicitly wants a fresh answer, not the token + // this call was meant to replace. + throw error; + } throw new ElicitationUnavailableError( error instanceof Error ? error.message : String(error) ); @@ -227,13 +253,52 @@ ${creationNote}`, throw new Error('Token elicitation was cancelled or declined by user'); } - // Parse the result - const choice = (result.content.choice as TokenChoice) || choices[0]; - const token = result.content.token as string | undefined; - const tokenNote = result.content.tokenNote as string | undefined; - const urlRestrictionsStr = result.content.urlRestrictions as - | string - | undefined; + // Parse the result. requestedSchema above is only a hint for the client's own form + // UI — nothing stops a client (malicious, or just not honoring it) from returning a + // differently-shaped payload, so every field's runtime type is checked explicitly + // rather than blindly cast. An unchecked cast here previously let a non-string + // token silently skip the length guard below, and let a non-string urlRestrictions + // reach `.split()` and throw an unclassified TypeError instead of a clear error. + const rawChoice = result.content.choice; + if ( + typeof rawChoice !== 'string' || + !choices.includes(rawChoice as TokenChoice) + ) { + throw new Error( + `Client returned an unrecognized token choice (${JSON.stringify(rawChoice)}); expected one of: ${choices.join(', ')}.` + ); + } + const choice = rawChoice as TokenChoice; + + const token = result.content.token; + if (token !== undefined && typeof token !== 'string') { + throw new Error('Client returned a non-string value for the token field.'); + } + + const tokenNote = result.content.tokenNote; + if (tokenNote !== undefined && typeof tokenNote !== 'string') { + throw new Error( + 'Client returned a non-string value for the tokenNote field.' + ); + } + + const urlRestrictionsStr = result.content.urlRestrictions; + if ( + urlRestrictionsStr !== undefined && + typeof urlRestrictionsStr !== 'string' + ) { + throw new Error( + 'Client returned a non-string value for the urlRestrictions field.' + ); + } + if ( + urlRestrictionsStr !== undefined && + urlRestrictionsStr.length > MAX_URL_RESTRICTIONS_RAW_LENGTH + ) { + throw new Error( + `Provided urlRestrictions value is ${urlRestrictionsStr.length} characters, which exceeds the ${MAX_URL_RESTRICTIONS_RAW_LENGTH}-character maximum.` + ); + } const urlRestrictions = urlRestrictionsStr ? urlRestrictionsStr @@ -265,6 +330,17 @@ ${creationNote}`, ); } + // Every other path that produces a preview token (the `accessToken` input parameter, + // and the create/auto-create API responses) is checked against this same prefix — + // this was the one path that wasn't, which meant pasting a secret token (sk.*) into + // the elicitation dialog embedded it straight into the returned preview URL and + // cached it for reuse, exactly the leak this feature exists to prevent. + if (choice === 'provide' && token !== undefined && !token.startsWith('pk.')) { + throw new Error( + 'Invalid access token. Only public tokens (starting with pk.*) are allowed for preview URLs. Secret tokens (sk.*) cannot be used as they cannot be exposed in browser URLs.' + ); + } + return { choice, token, @@ -463,7 +539,7 @@ export async function createPreviewToken( if (!response.ok) { const errorText = await response.text(); - let message = `Failed to create token: ${response.status} ${errorText}`; + let message = `Failed to create token: ${response.status} ${redactToken(errorText)}`; // Creating a token always requires `tokens:write` on the caller's own access // token, so a 401/403 here is a permission problem — surface the same @@ -488,7 +564,15 @@ export async function createPreviewToken( }; } - const data = (await response.json()) as { token: string }; + const data = (await response.json()) as { token?: unknown }; + + if (typeof data.token !== 'string') { + return { + success: false, + error: + 'API response did not include a token. Unexpected response shape from the Mapbox Tokens API.' + }; + } if (!data.token.startsWith('pk.')) { return { @@ -502,10 +586,17 @@ export async function createPreviewToken( token: data.token }; } catch (error) { + // A network-level failure (e.g. a misconfigured endpoint) can throw with the full + // request URL — including this call's own `access_token=...` query param — in its + // message. Redact before this reaches a caller, the same as every other Mapbox API + // tool's errors do via MapboxApiBasedTool#run(); these tools don't extend that + // class, and this is a returned value rather than a thrown rejection regardless, + // so that redaction wouldn't apply here even if they did. return { success: false, - error: + error: redactToken( error instanceof Error ? error.message : 'Unknown error creating token' + ) }; } } diff --git a/test/integration/elicitationOverHttp.test.ts b/test/integration/elicitationOverHttp.test.ts index 516d6d8..d0786e6 100644 --- a/test/integration/elicitationOverHttp.test.ts +++ b/test/integration/elicitationOverHttp.test.ts @@ -232,11 +232,12 @@ describe('preview/comparison token elicitation over real Streamable HTTP', () => harness = undefined; }); - it('trims the dialog to "provide" and never calls the Tokens API when the server token is tk.*', async () => { + it('trims the dialog to "provide" and never calls the Tokens API to create a token when the server token is tk.*', async () => { const httpRequest = mockHttpRequest({ - GET: () => { - throw new Error('should not list tokens for a tk.* server token'); - }, + // Listing only needs tokens:read, a separate scope from the tokens:write a + // tk.* token lacks, so it's still attempted — it just isn't used here since + // the client answers with 'provide'. + GET: () => jsonResponse(200, []), POST: () => { throw new Error('should not create a token for a tk.* server token'); } @@ -263,7 +264,7 @@ describe('preview/comparison token elicitation over real Streamable HTTP', () => expect(result.isError).toBeFalsy(); expect(receivedEnum).toEqual(['provide']); - expect(httpRequest).not.toHaveBeenCalled(); + expect(httpRequest).toHaveBeenCalledTimes(1); }); it('offers all three choices for a non-tk.*-shaped server token and surfaces a scope hint when auto-create fails (the hosted-endpoint case)', async () => { @@ -322,9 +323,10 @@ describe('preview/comparison token elicitation over real Streamable HTTP', () => it("trims style_comparison_tool's dialog the same way for a tk.* server token", async () => { const httpRequest = mockHttpRequest({ - GET: () => { - throw new Error('should not list tokens for a tk.* server token'); - }, + // Listing only needs tokens:read, a separate scope from the tokens:write a + // tk.* token lacks, so it's still attempted — it just isn't used here since + // the client answers with 'provide'. + GET: () => jsonResponse(200, []), POST: () => { throw new Error('should not create a token for a tk.* server token'); } @@ -351,6 +353,6 @@ describe('preview/comparison token elicitation over real Streamable HTTP', () => expect(result.isError).toBeFalsy(); expect(receivedEnum).toEqual(['provide']); - expect(httpRequest).not.toHaveBeenCalled(); + expect(httpRequest).toHaveBeenCalledTimes(1); }); }); diff --git a/test/tools/preview-style-tool/PreviewStyleTool.test.ts b/test/tools/preview-style-tool/PreviewStyleTool.test.ts index 6d5bd08..bbc9741 100644 --- a/test/tools/preview-style-tool/PreviewStyleTool.test.ts +++ b/test/tools/preview-style-tool/PreviewStyleTool.test.ts @@ -280,9 +280,12 @@ describe('PreviewStyleTool', () => { sendRequest.mock.calls[0][0].params.requestedSchema; expect(requestedSchema.properties.choice.enum).toEqual(['provide']); - // A tk.* server token can never create tokens, so listing/creating - // tokens against the Mapbox API should never even be attempted. - expect(mockHttpRequest).not.toHaveBeenCalled(); + // A tk.* server token can never create tokens (tokens:write), but listing only + // needs tokens:read — a separate scope — so it's still attempted (and fails + // safe to an empty list if the token can't do that either). Only creation + // (a POST) must never be attempted. + expect(mockHttpRequest).toHaveBeenCalledTimes(1); + expect(mockHttpRequest.mock.calls[0][1]?.method).not.toBe('POST'); }); it('reuses a cached token instead of erroring when useCustomToken is set but the client cannot act on it', async () => { diff --git a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts index 9eec113..b9f3996 100644 --- a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts +++ b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts @@ -345,9 +345,12 @@ describe('StyleComparisonTool', () => { sendRequest.mock.calls[0][0].params.requestedSchema; expect(requestedSchema.properties.choice.enum).toEqual(['provide']); - // A tk.* server token can never create tokens, so listing/creating - // tokens against the Mapbox API should never even be attempted. - expect(mockHttpRequest).not.toHaveBeenCalled(); + // A tk.* server token can never create tokens (tokens:write), but listing only + // needs tokens:read — a separate scope — so it's still attempted (and fails + // safe to an empty list if the token can't do that either). Only creation + // (a POST) must never be attempted. + expect(mockHttpRequest).toHaveBeenCalledTimes(1); + expect(mockHttpRequest.mock.calls[0][1]?.method).not.toBe('POST'); }); it('reuses a cached token instead of erroring when useCustomToken is set but the client cannot act on it', async () => { diff --git a/test/utils/tokenElicitation.test.ts b/test/utils/tokenElicitation.test.ts index 7cc65e9..514b777 100644 --- a/test/utils/tokenElicitation.test.ts +++ b/test/utils/tokenElicitation.test.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; import { cacheKeyFor, createPreviewToken, @@ -260,6 +261,64 @@ describe('createPreviewToken', () => { expect(result.error).toContain('internal server error'); expect(result.error).not.toContain('tokens:write'); }); + + it('rejects an API response that omits the token field instead of throwing a raw TypeError', async () => { + const { httpRequest } = setupHttpRequest({ + json: async () => ({}) + }); + + const result = await createPreviewToken( + httpRequest, + MAPBOX_API_ENDPOINT, + 'sk.eyJ1IjoidGVzdCJ9.sig', + 'test-user' + ); + + expect(result.success).toBe(false); + expect(result.error).toMatch(/did not include a token/); + }); + + it('redacts the caller access token out of a network-error message before returning it', async () => { + const secretToken = 'sk.eyJ1IjoidGVzdC11c2VyIn0.super-secret-signature'; + const httpRequest = vi + .fn() + .mockRejectedValue( + new Error( + `fetch failed: connect ECONNREFUSED, request to https://api.mapbox.com/tokens/v2/test-user?access_token=${secretToken}` + ) + ); + + const result = await createPreviewToken( + httpRequest, + MAPBOX_API_ENDPOINT, + secretToken, + 'test-user' + ); + + expect(result.success).toBe(false); + expect(result.error).not.toContain(secretToken); + expect(result.error).toContain('redacted'); + }); + + it('redacts a token echoed back in a non-ok response body before returning it', async () => { + const secretToken = 'sk.eyJ1IjoidGVzdC11c2VyIn0.super-secret-signature'; + const { httpRequest } = setupHttpRequest({ + ok: false, + status: 400, + text: async () => + `Bad request for access_token=${secretToken}: malformed body` + }); + + const result = await createPreviewToken( + httpRequest, + MAPBOX_API_ENDPOINT, + secretToken, + 'test-user' + ); + + expect(result.success).toBe(false); + expect(result.error).not.toContain(secretToken); + }); }); describe('listPublicPreviewTokens', () => { @@ -395,4 +454,133 @@ describe('elicitPreviewToken', () => { /URL restrictions.*exceeds/ ); }); + + it('rejects a client-returned urlRestrictions raw string that exceeds the max length, even as a single unsplit value', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + action: 'accept', + content: { + choice: 'create', + // One URL far longer than the array-count cap alone would ever stop. + urlRestrictions: 'https://example.com/' + 'a'.repeat(5000) + } + }); + + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( + /urlRestrictions value is .* characters/ + ); + }); + + it('rejects a secret token submitted via the "provide" choice, the same way the accessToken parameter is rejected', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + action: 'accept', + content: { + choice: 'provide', + token: 'sk.eyJ1IjoidGVzdC11c2VyIn0.secret-signature' + } + }); + + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( + /Only public tokens \(starting with pk\.\*\) are allowed/ + ); + }); + + it('rejects an unrecognized choice value instead of silently treating it as auto-create', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + action: 'accept', + content: { choice: 'delete-everything' } + }); + + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( + /unrecognized token choice/ + ); + }); + + it('rejects a choice offered only when canCreateTokens is true if the server token cannot create tokens', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + action: 'accept', + content: { choice: 'auto' } + }); + + // canCreateTokens=false means only 'provide' was ever offered; a client + // returning 'auto' anyway must not be honored. + await expect(elicitPreviewToken(sendRequest, [], false)).rejects.toThrow( + /unrecognized token choice/ + ); + }); + + it('rejects a non-string token instead of silently bypassing the length guard', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + action: 'accept', + content: { choice: 'provide', token: 12345 } + }); + + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( + /non-string value for the token field/ + ); + }); + + it('rejects a non-string urlRestrictions instead of crashing on .split()', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + action: 'accept', + content: { + choice: 'create', + urlRestrictions: ['https://example.com/*'] + } + }); + + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( + /non-string value for the urlRestrictions field/ + ); + }); + + it('rejects a non-string tokenNote', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + action: 'accept', + content: { choice: 'create', tokenNote: 42 } + }); + + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( + /non-string value for the tokenNote field/ + ); + }); + + it('propagates a request-timeout error instead of treating it as "client does not support elicitation"', async () => { + const sendRequest = vi + .fn() + .mockRejectedValue( + new McpError(ErrorCode.RequestTimeout, 'Request timed out') + ); + + const promise = elicitPreviewToken(sendRequest, [], true); + await expect(promise).rejects.not.toBeInstanceOf( + ElicitationUnavailableError + ); + await expect(promise).rejects.toThrow('Request timed out'); + }); + + it('propagates a cancellation error instead of treating it as "client does not support elicitation"', async () => { + const sendRequest = vi + .fn() + .mockRejectedValue( + new McpError(ErrorCode.InvalidRequest, 'Request cancelled') + ); + + const promise = elicitPreviewToken(sendRequest, [], true); + await expect(promise).rejects.not.toBeInstanceOf( + ElicitationUnavailableError + ); + await expect(promise).rejects.toThrow('Request cancelled'); + }); + + it('still wraps a genuine "client does not support elicitation" failure', async () => { + const sendRequest = vi + .fn() + .mockRejectedValue( + new McpError(ErrorCode.MethodNotFound, 'Method not found') + ); + + await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( + ElicitationUnavailableError + ); + }); }); From 95a77d76660347d8637c039ce33383a309275696 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Fri, 14 Aug 2026 18:20:37 -0400 Subject: [PATCH 24/25] Redesign preview-token "provide" flow to use MCP URL-mode elicitation The finalized MCP spec (SEP-1036, protocol revision 2025-11-25) requires credentials to be collected via URL-mode elicitation, not form-mode: Servers MUST NOT use form mode elicitation to request sensitive information such as passwords, API keys, access tokens, or payment credentials. Servers MUST use URL mode for interactions involving such sensitive information. preview_style_tool / style_comparison_tool's "I have a token to provide" option asked the user to paste their Mapbox token into a form-mode dialog field -- a direct violation, independent of the sk.*/pk.* prefix check already enforced there (a pk.* token is still a credential). Since none of this has shipped yet, this fixes the design rather than patching around it. - src/utils/tokenCollectionServer.ts (new): TokenCollectionHandler interface + LocalHttpTokenCollectionHandler, a short-lived HTTP server bound to 127.0.0.1 only, serving one form at a random single-use path and accepting exactly one submission -- the same local-callback pattern gh auth login/gcloud auth login use. This is the right default because this package's only shipped entry point (src/index.ts) is stdio-based: the server process and the user's browser are always on the same machine. - src/utils/tokenElicitation.ts: elicitPreviewToken no longer collects a token via form mode at all -- the initial dialog now only asks choice/tokenNote/urlRestrictions, none of which are credentials. New collectProvidedToken() sends a follow-up URL-mode elicitation request pointing at the local server, waits for the out-of-band submission, and validates it with validatePublicPreviewToken (same pk.*/length checks the old code had, now centralized and reused). A failed or unsupported URL-mode request degrades to the exact same ElicitationUnavailableError fallback already used for clients without elicitation support at all -- no new failure mode, since there's no safe way to proactively check a connected client's elicitation.url capability without reintroducing the this.server cross-session-unsafe pattern this same file already had a real hijack bug from. - ENABLE_LOCAL_URL_ELICITATION env var (default true): required safety valve. The local-loopback approach is correct for this package's default stdio usage but would silently break in a deployment where the server process doesn't run on the end user's own machine (e.g. hosted-mcp-server, a cloud deployment) -- a 127.0.0.1 URL there resolves to the browser's own loopback interface, not the server. Documented in README/engineering_standards as a required follow-up for hosted-mcp-server's deployment config (separate repo, tracked there). - PreviewStyleTool / StyleComparisonTool take an optional constructor-injected tokenCollectionHandler now, same DI pattern as httpRequest, wired to the local default in toolRegistry.ts. Tests: new test/utils/tokenCollectionServer.test.ts (server lifecycle, POST resolves/rejects/times out, loopback-only, body-size cap); relocated the provide-token validation tests from elicitPreviewToken onto collectProvidedToken; updated PreviewStyleTool/StyleComparisonTool and the real-SDK integration/cross-session-hijack tests for the new two-step (form choice + URL-mode consent) flow, including declaring elicitation.url client capability where previously only form was declared. 689 tests passing, no hangs; lint/tsc/build clean. --- CHANGELOG.md | 8 +- README.md | 16 +- docs/engineering_standards.md | 1 + .../preview-style-tool/PreviewStyleTool.ts | 34 ++- .../StyleComparisonTool.ts | 34 ++- src/tools/toolRegistry.ts | 11 +- src/utils/tokenCollectionServer.ts | 211 ++++++++++++++++ src/utils/tokenElicitation.ts | 211 ++++++++++++---- test/integration/elicitationOverHttp.test.ts | 63 +++-- .../cross-session-elicitation-hijack.test.ts | 50 +++- .../PreviewStyleTool.test.ts | 37 ++- .../StyleComparisonTool.test.ts | 37 ++- test/utils/tokenCollectionServer.test.ts | 141 +++++++++++ test/utils/tokenElicitation.test.ts | 225 +++++++++++++++--- 14 files changed, 928 insertions(+), 151 deletions(-) create mode 100644 src/utils/tokenCollectionServer.ts create mode 100644 test/utils/tokenCollectionServer.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index bcec095..d8f381d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,11 @@ ### New Features -- **Secure token elicitation for `preview_style_tool` / `style_comparison_tool`** (#57): `accessToken` is now optional on both tools. When omitted, the tool uses MCP elicitation to ask for a public token instead — either pasting an existing `pk.*` token, creating a new one with optional URL restrictions, or auto-creating a minimally-scoped one (`styles:read`, `styles:tiles`, `fonts:read`). This keeps your server's `sk.*`/`pk.*` access token out of chat history and preview URLs. Falls back to requiring `accessToken` directly on clients without elicitation support (Claude Desktop, Claude Code). A chosen/created token is cached in memory per token; pass `useCustomToken: true` to force re-selection. Elicitation requests are routed through the per-call session context rather than a shared server reference, so a tool call's prompt is always delivered back to the session that made it, even when the same tool instance is reused across multiple concurrent sessions. +- **Secure token elicitation for `preview_style_tool` / `style_comparison_tool`** (#57): `accessToken` is now optional on both tools. When omitted, the tool asks how you'd like to provide a public token — pasting an existing `pk.*` token, creating a new one with optional URL restrictions, or auto-creating a minimally-scoped one (`styles:read`, `styles:tiles`, `fonts:read`). This keeps your server's `sk.*`/`pk.*` access token out of chat history and preview URLs. Falls back to requiring `accessToken` directly on clients without elicitation support (Claude Desktop, Claude Code). A chosen/created token is cached in memory per token; pass `useCustomToken: true` to force re-selection. Elicitation requests are routed through the per-call session context rather than a shared server reference, so a tool call's prompt is always delivered back to the session that made it, even when the same tool instance is reused across multiple concurrent sessions. - If the server's own access token is a literal Mapbox temporary token (`tk.*`), the "create a new token" and "auto-create" options are omitted from the dialog up front, since that token shape is guaranteed to lack `tokens:write`. Other callers lacking `tokens:write` (e.g. the hosted MCP endpoint, which authenticates with its own access token rather than a Mapbox `pk.*`/`sk.*`/`tk.*` token) aren't detectable ahead of time; for those, choosing "create"/"auto-create" fails against the Tokens API with a scope/permission hint appended to the error, steering back to "I have a token to provide". - - The elicitation request has an explicit 60s timeout rather than relying on the SDK's default; a client-returned token/token name/URL-restriction list that exceeds a sane size is rejected server-side regardless of what the elicitation dialog's schema hints suggest a client's form should enforce; and the in-memory token cache is bounded to 1000 entries (LRU eviction), so none of this can grow memory usage without limit. - - A token pasted into the elicitation dialog's "I have a token to provide" option is now checked for the `pk.*` prefix, same as every other path that produces a preview token. It previously wasn't, so a secret token (`sk.*`) submitted that way was embedded straight into the returned preview URL and cached for reuse — the exact leak this feature exists to prevent. Found by an adversarial review pass. - - Every field the client returns from the elicitation dialog (choice, token, token name, URL restrictions) is now validated against its expected runtime type and, for `choice`, against the set of options actually offered — a client returning an unrecognized or wrong-typed value previously either fell through to auto-creating a real token or threw an unclassified `TypeError`. A request timeout or cancellation is now also distinguished from "client doesn't support elicitation", so a caller passing `useCustomToken: true` gets a real error instead of silently getting back the stale cached token it was trying to replace. + - The initial choice dialog has an explicit 60s timeout rather than relying on the SDK's default; a client-returned token name/URL-restriction list that exceeds a sane size is rejected server-side regardless of what the dialog's schema hints suggest a client's form should enforce; and the in-memory token cache is bounded to 1000 entries (LRU eviction), so none of this can grow memory usage without limit. + - **"I have a token to provide" now uses MCP URL-mode elicitation, not a form field.** The MCP spec (SEP-1036) prohibits collecting credentials via form-mode elicitation — a `pk.*` token is still a credential, so the original design (a `token` text field in the same form-mode dialog as the choice picker) didn't comply, and separately meant a pasted `sk.*` secret token skipped the `pk.*` prefix check every other path enforces and got embedded straight into the returned preview URL (found by an adversarial review pass). Choosing "provide" now opens a short-lived HTTP server on `127.0.0.1` (the same pattern `gh auth login`/`gcloud auth login` use) and sends a URL-mode elicitation request pointing at it; the token is submitted directly to that local page, never through the MCP client or chat history, then validated with the same `pk.*`/length checks as every other path. Set `ENABLE_LOCAL_URL_ELICITATION=false` to disable this and fall back to requiring `accessToken` directly — **required** for deployments where the server process doesn't run on the end user's own machine (e.g. a hosted/cloud deployment), since a `127.0.0.1` URL there wouldn't resolve to anything the user's browser could reach. + - Every field the client returns from the choice dialog (choice, token name, URL restrictions) is now validated against its expected runtime type and, for `choice`, against the set of options actually offered — a client returning an unrecognized or wrong-typed value previously either fell through to auto-creating a real token or threw an unclassified `TypeError`. A request timeout or cancellation at either step (the choice dialog or the URL-mode consent request) is now also distinguished from "client doesn't support elicitation", so a caller passing `useCustomToken: true` gets a real error instead of silently getting back the stale cached token it was trying to replace. - Listing a user's existing public tokens (to populate the dialog) only requires `tokens:read`, a separate scope from the `tokens:write` needed to create one — it's no longer skipped for `tk.*`-authenticated servers, which lack the latter but not necessarily the former. - Error messages from a failed token-creation call are now redacted the same way every other Mapbox API tool's errors are, since these two tools don't extend the base class that applies that redaction automatically. A misconfigured endpoint could previously surface the caller's own access token verbatim in the returned error text. diff --git a/README.md b/README.md index dce7279..171a5c9 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,9 @@ Get started by integrating with your preferred AI development environment: - **Claude Desktop**: ⚠️ Not yet supported (Claude will fall back to creating tokens via chat) - **Claude Code**: ⚠️ Not yet supported (provide `accessToken` parameter directly) -**Note on the hosted MCP endpoint**: even on a client with full elicitation support, "create a new token" and "auto-create" will fail on the [hosted endpoint](#hosted-mcp-endpoint) — see below for why. +Choosing **"I have a token to provide"** doesn't paste the token into a form — the MCP spec requires credentials to go through [URL-mode elicitation](https://modelcontextprotocol.io/specification/2025-11-25/client/elicitation#url-mode-elicitation-requests) instead, so this opens a page served locally on your own machine (`http://127.0.0.1:`) to submit it. This requires a client that supports URL-mode elicitation specifically, and requires the server process to be running on the same machine as your browser — see [`ENABLE_LOCAL_URL_ELICITATION`](#enable_local_url_elicitation). + +**Note on the hosted MCP endpoint**: even on a client with full elicitation support, "create a new token" and "auto-create" will fail on the [hosted endpoint](#hosted-mcp-endpoint) — see below for why. "I have a token to provide" is also unavailable there; use the `accessToken` parameter directly instead. ### DXT Package Distribution @@ -112,7 +114,7 @@ For detailed setup instructions for different clients and API usage, see the [Ho - `preview_style_tool` / `style_comparison_tool`'s elicitation dialog still offers all three options, but choosing "create a new token" or "auto-create" fails against the Mapbox Tokens API with a scope/permission error (the dialog can't know ahead of time that this particular deployment's token lacks `tokens:write` — see the `isTemporaryServerToken` caveat in `src/utils/tokenElicitation.ts` for tokens where it can tell). - `create_token_tool` is not exposed on the hosted endpoint at all. -Choose **"I have a token to provide"** and paste an existing public token (`pk.*`, with `styles:read` scope), or provide `accessToken` directly. Create a token ahead of time from your [Mapbox Account page](https://account.mapbox.com/) if you don't have one. Running this server **locally** with your own `pk.*`/`sk.*` access token (which can carry `tokens:write`) also enables create and auto-create. +**"I have a token to provide" also requires the hosted deployment to set `ENABLE_LOCAL_URL_ELICITATION=false`**: that option now works via URL-mode elicitation to a page served on `127.0.0.1`, which only makes sense when the server process runs on your own machine — see [`ENABLE_LOCAL_URL_ELICITATION`](#enable_local_url_elicitation). Until the hosted deployment sets that variable, treat "provide" as unavailable there too and pass `accessToken` directly instead. Create a token ahead of time from your [Mapbox Account page](https://account.mapbox.com/) if you don't have one. Running this server **locally** with your own `pk.*`/`sk.*` access token (which can carry `tokens:write`) also enables create and auto-create. ### Getting Your Mapbox Access Token @@ -1304,6 +1306,16 @@ Set `VERBOSE_ERRORS=true` to get detailed error messages from the MCP server. Th By default, the server returns generic error messages. With verbose errors enabled, you'll receive the actual error details, which can help diagnose API connection issues, invalid parameters, or other problems. +#### ENABLE_LOCAL_URL_ELICITATION + +Controls whether `preview_style_tool` / `style_comparison_tool`'s "I have a token to provide" option is offered. Per the MCP spec, servers must not collect credentials via form-mode elicitation, so providing a token instead opens a short-lived HTTP server on `127.0.0.1` and sends a URL-mode elicitation request pointing at it — the same pattern CLI tools like `gh auth login` use. This only works when the server process and your browser are on the same machine. + +Defaults to `true`. **Set to `false` for any deployment where the server process does not run on the end user's own machine** (for example, a hosted/cloud deployment) — a `127.0.0.1` URL there would resolve to the browser's own loopback interface, where nothing is listening, rather than the server. With it disabled, choosing "provide" falls back to the same message shown to clients without elicitation support at all: pass `accessToken` directly instead. + +```bash +export ENABLE_LOCAL_URL_ELICITATION=false +``` + #### ENABLE_MCP_UI **Interactive Previews: MCP Apps (primary) & MCP-UI (compatibility)** diff --git a/docs/engineering_standards.md b/docs/engineering_standards.md index 64a34ae..d561414 100644 --- a/docs/engineering_standards.md +++ b/docs/engineering_standards.md @@ -110,6 +110,7 @@ Keep secrets out of repositories. Use environment variables for sensitive data: - `MAPBOX_ACCESS_TOKEN` - Required for all Mapbox API operations - `VERBOSE_ERRORS` - Set to `true` for detailed error messages - `ENABLE_MCP_UI` - Controls MCP-UI support (default: `true`) +- `ENABLE_LOCAL_URL_ELICITATION` - Controls whether `preview_style_tool`/`style_comparison_tool` offer URL-mode token collection via a local `127.0.0.1` server (default: `true`). Set to `false` for any deployment where the server process doesn't run on the end user's own machine (see `src/utils/tokenCollectionServer.ts`) — a hosted/cloud deployment MUST set this, since a `127.0.0.1` URL there can't be reached by the user's browser. - `OTEL_EXPORTER_OTLP_ENDPOINT` - OpenTelemetry endpoint (optional) - `OTEL_SERVICE_NAME` - Override service name for tracing (optional) diff --git a/src/tools/preview-style-tool/PreviewStyleTool.ts b/src/tools/preview-style-tool/PreviewStyleTool.ts index 62ff446..f5a4e6f 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.ts @@ -11,6 +11,7 @@ import { import { getUserNameFromToken } from '../../utils/jwtUtils.js'; import { cacheKeyFor, + collectProvidedToken, createPreviewToken, ElicitationUnavailableError, elicitPreviewToken, @@ -18,6 +19,10 @@ import { listPublicPreviewTokens, previewTokenStorage } from '../../utils/tokenElicitation.js'; +import { + localHttpTokenCollectionHandler, + type TokenCollectionHandler +} from '../../utils/tokenCollectionServer.js'; import type { HttpRequest } from '../../utils/types.js'; // `BaseTool#execute`'s abstract signature accepts `ToolExecutionContext` in this slot; @@ -51,10 +56,16 @@ export class PreviewStyleTool extends BaseTool { }; private readonly httpRequest: HttpRequest; + private readonly tokenCollectionHandler: TokenCollectionHandler; - constructor(params: { httpRequest: HttpRequest }) { + constructor(params: { + httpRequest: HttpRequest; + tokenCollectionHandler?: TokenCollectionHandler; + }) { super({ inputSchema: PreviewStyleSchema }); this.httpRequest = params.httpRequest; + this.tokenCollectionHandler = + params.tokenCollectionHandler ?? localHttpTokenCollectionHandler; } /** @@ -161,18 +172,15 @@ export class PreviewStyleTool extends BaseTool { // Handle user's choice if (elicited.choice === 'provide') { - if (!elicited.token) { - return { - isError: true, - content: [ - { - type: 'text', - text: 'No token provided. Please provide a valid public token.' - } - ] - }; - } - publicToken = elicited.token; + // Collected via a follow-up URL-mode elicitation, not this form dialog — + // the MCP spec requires credentials to go through URL mode, not form mode. + // Errors here (unsupported client, decline/cancel, validation failure) are + // handled by the catch below exactly like elicitPreviewToken's own errors. + publicToken = await collectProvidedToken( + extra.sendRequest, + extra.sendNotification, + this.tokenCollectionHandler + ); } else if (elicited.choice === 'create') { // Create new token with user's specifications const created = await createPreviewToken( diff --git a/src/tools/style-comparison-tool/StyleComparisonTool.ts b/src/tools/style-comparison-tool/StyleComparisonTool.ts index 0452719..09aa96d 100644 --- a/src/tools/style-comparison-tool/StyleComparisonTool.ts +++ b/src/tools/style-comparison-tool/StyleComparisonTool.ts @@ -14,6 +14,7 @@ import { import { getUserNameFromToken } from '../../utils/jwtUtils.js'; import { cacheKeyFor, + collectProvidedToken, createPreviewToken, ElicitationUnavailableError, elicitPreviewToken, @@ -21,6 +22,10 @@ import { listPublicPreviewTokens, previewTokenStorage } from '../../utils/tokenElicitation.js'; +import { + localHttpTokenCollectionHandler, + type TokenCollectionHandler +} from '../../utils/tokenCollectionServer.js'; import type { HttpRequest } from '../../utils/types.js'; // `BaseTool#execute`'s abstract signature accepts `ToolExecutionContext` in this slot; @@ -56,10 +61,16 @@ export class StyleComparisonTool extends BaseTool< }; private readonly httpRequest: HttpRequest; + private readonly tokenCollectionHandler: TokenCollectionHandler; - constructor(params: { httpRequest: HttpRequest }) { + constructor(params: { + httpRequest: HttpRequest; + tokenCollectionHandler?: TokenCollectionHandler; + }) { super({ inputSchema: StyleComparisonSchema }); this.httpRequest = params.httpRequest; + this.tokenCollectionHandler = + params.tokenCollectionHandler ?? localHttpTokenCollectionHandler; } /** @@ -209,18 +220,15 @@ export class StyleComparisonTool extends BaseTool< ); if (elicited.choice === 'provide') { - if (!elicited.token) { - return { - isError: true, - content: [ - { - type: 'text', - text: 'No token provided. Please provide a valid public token.' - } - ] - }; - } - publicToken = elicited.token; + // Collected via a follow-up URL-mode elicitation, not this form dialog — + // the MCP spec requires credentials to go through URL mode, not form mode. + // Errors here (unsupported client, decline/cancel, validation failure) are + // handled by the catch below exactly like elicitPreviewToken's own errors. + publicToken = await collectProvidedToken( + extra.sendRequest, + extra.sendNotification, + this.tokenCollectionHandler + ); } else if (elicited.choice === 'create') { const created = await createPreviewToken( this.httpRequest, diff --git a/src/tools/toolRegistry.ts b/src/tools/toolRegistry.ts index 64f181f..25fabe5 100644 --- a/src/tools/toolRegistry.ts +++ b/src/tools/toolRegistry.ts @@ -25,6 +25,7 @@ import { ValidateExpressionTool } from './validate-expression-tool/ValidateExpre import { ValidateGeojsonTool } from './validate-geojson-tool/ValidateGeojsonTool.js'; import { ValidateStyleTool } from './validate-style-tool/ValidateStyleTool.js'; import { httpRequest } from '../utils/httpPipeline.js'; +import { localHttpTokenCollectionHandler } from '../utils/tokenCollectionServer.js'; /** * Core tools that work in all MCP clients without requiring special capabilities @@ -36,13 +37,19 @@ export const CORE_TOOLS = [ new RetrieveStyleTool({ httpRequest }), new UpdateStyleTool({ httpRequest }), new DeleteStyleTool({ httpRequest }), - new PreviewStyleTool({ httpRequest }), + new PreviewStyleTool({ + httpRequest, + tokenCollectionHandler: localHttpTokenCollectionHandler + }), new StyleBuilderTool(), new GeojsonPreviewTool(), new CheckColorContrastTool(), new CompareStylesTool(), new OptimizeStyleTool(), - new StyleComparisonTool({ httpRequest }), + new StyleComparisonTool({ + httpRequest, + tokenCollectionHandler: localHttpTokenCollectionHandler + }), new CreateTokenTool({ httpRequest }), new ListTokensTool({ httpRequest }), new BoundingBoxTool(), diff --git a/src/utils/tokenCollectionServer.ts b/src/utils/tokenCollectionServer.ts new file mode 100644 index 0000000..fb6f1bd --- /dev/null +++ b/src/utils/tokenCollectionServer.ts @@ -0,0 +1,211 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { + createServer, + type IncomingMessage, + type ServerResponse +} from 'node:http'; +import { randomBytes } from 'node:crypto'; + +/** + * Collects a sensitive value (a Mapbox access token) out-of-band from the MCP + * connection itself, per the MCP spec's requirement that servers MUST use URL-mode + * elicitation — not form-mode — for credentials ("Servers MUST NOT use form mode + * elicitation to request sensitive information such as passwords, API keys, access + * tokens, or payment credentials"). + * + * `collect()` starts the out-of-band flow and returns the URL to present to the user + * via a URL-mode elicitation request, plus a promise that resolves once the value has + * been submitted (or rejects on timeout/failure). Callers MUST call `cancel()` if they + * give up waiting on `result` (e.g. because the URL-mode elicitation request itself was + * declined or unsupported) to release the resources started by `collect()`. + */ +export interface TokenCollectionHandler { + collect(options: { timeoutMs: number }): Promise<{ + /** URL to present to the user via URL-mode elicitation. */ + url: string; + /** Resolves with the raw submitted value, or rejects on timeout/failure. */ + result: Promise; + /** Releases resources without resolving `result`. Safe to call after `result` + * has already settled — a no-op in that case. */ + cancel: () => void; + }>; +} + +/** Bounds how much of a POST body is buffered before the token itself is even + * inspected — independent defense-in-depth against a huge submitted body, on top of + * whatever length limit the caller enforces on the parsed token value afterward. */ +const MAX_BODY_BYTES = 16 * 1024; + +function readBody(req: IncomingMessage): Promise { + return new Promise((resolve, reject) => { + let size = 0; + const chunks: Buffer[] = []; + req.on('data', (chunk: Buffer) => { + size += chunk.length; + if (size > MAX_BODY_BYTES) { + req.destroy(); + reject(new Error('Request body exceeds the maximum accepted size.')); + return; + } + chunks.push(chunk); + }); + req.on('end', () => resolve(Buffer.concat(chunks).toString('utf-8'))); + req.on('error', reject); + }); +} + +function formPage(): string { + return ` + +Mapbox MCP DevKit — Preview Token + +

Provide your Mapbox public token

+

This page is running locally on your own machine and was opened at your request by + your MCP client. The token you submit here is sent directly to the MCP DevKit server + process running on this machine — not to your MCP client, and not through chat + history.

+
+ + +
+ +`; +} + +const SUCCESS_PAGE = ` + +Mapbox MCP DevKit — Preview Token + +

Token received

+

You can close this window and return to your MCP client.

+ +`; + +/** + * Default `TokenCollectionHandler`: a short-lived HTTP server bound to the loopback + * interface only (`127.0.0.1`, never `0.0.0.0`), serving exactly one form at a random, + * single-use path and accepting exactly one submission before tearing itself down. This + * is the same pattern CLI OAuth flows use (`gh auth login`, `gcloud auth login`) — a + * local callback server the user's own browser can reach. + * + * This only makes sense when the MCP server process and the user's browser run on the + * same machine, which is true for this package's only shipped entry point + * (`src/index.ts`, stdio-based). It is NOT appropriate for a deployment where the + * server process runs somewhere other than the end user's own machine (e.g. + * hosted-mcp-server, a cloud deployment) — a URL pointing at `127.0.0.1` there would + * resolve to the *browser's* loopback interface, where nothing is listening. See + * `ENABLE_LOCAL_URL_ELICITATION` in `tokenElicitation.ts` for the opt-out for that case. + */ +export class LocalHttpTokenCollectionHandler implements TokenCollectionHandler { + async collect(options: { timeoutMs: number }): Promise<{ + url: string; + result: Promise; + cancel: () => void; + }> { + const server = createServer(); + + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(0, '127.0.0.1', () => { + server.removeListener('error', reject); + resolve(); + }); + }); + + const address = server.address(); + if (address === null || typeof address === 'string') { + server.close(); + throw new Error( + 'Failed to determine local token-collection server address.' + ); + } + + const path = `/${randomBytes(24).toString('hex')}`; + const url = `http://127.0.0.1:${address.port}${path}`; + + let settled = false; + let resolveResult!: (value: string) => void; + let rejectResult!: (reason: unknown) => void; + const result = new Promise((resolve, reject) => { + resolveResult = resolve; + rejectResult = reject; + }); + + const finish = (action: () => void): void => { + if (settled) return; + settled = true; + clearTimeout(timeoutHandle); + server.close(); + action(); + }; + + server.on('request', (req: IncomingMessage, res: ServerResponse) => { + if (req.url !== path) { + res.writeHead(404).end(); + return; + } + + if (req.method === 'GET') { + res.writeHead(200, { 'content-type': 'text/html; charset=utf-8' }); + res.end(formPage()); + return; + } + + if (req.method === 'POST') { + readBody(req) + .then((body) => { + const token = new URLSearchParams(body).get('token'); + if (!token) { + // Let the user retry on the same page rather than tearing the server + // down over one malformed submission. + res.writeHead(400, { + 'content-type': 'text/html; charset=utf-8' + }); + res.end(formPage()); + return; + } + res.writeHead(200, { 'content-type': 'text/html; charset=utf-8' }); + res.end(SUCCESS_PAGE); + finish(() => resolveResult(token)); + }) + .catch((error) => { + res.writeHead(400).end(); + finish(() => + rejectResult( + error instanceof Error ? error : new Error(String(error)) + ) + ); + }); + return; + } + + res.writeHead(405).end(); + }); + + server.on('error', (error) => finish(() => rejectResult(error))); + + const timeoutHandle = setTimeout(() => { + finish(() => + rejectResult( + new Error( + `Timed out after ${options.timeoutMs}ms waiting for the token to be submitted.` + ) + ) + ); + }, options.timeoutMs); + + return { + url, + result, + cancel: () => + finish(() => rejectResult(new Error('Token collection was cancelled.'))) + }; + } +} + +/** Shared default instance, wired into the tools' constructors in `toolRegistry.ts` the + * same way the `httpRequest` singleton in `httpPipeline.ts` is. */ +export const localHttpTokenCollectionHandler = + new LocalHttpTokenCollectionHandler(); diff --git a/src/utils/tokenElicitation.ts b/src/utils/tokenElicitation.ts index 679cafa..5d8170a 100644 --- a/src/utils/tokenElicitation.ts +++ b/src/utils/tokenElicitation.ts @@ -1,7 +1,7 @@ // Copyright (c) Mapbox, Inc. // Licensed under the MIT License. -import { createHash } from 'node:crypto'; +import { createHash, randomUUID } from 'node:crypto'; import { ElicitResultSchema, ErrorCode, @@ -9,15 +9,22 @@ import { } from '@modelcontextprotocol/sdk/types.js'; import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol.js'; import { redactToken } from '../tools/MapboxApiBasedTool.js'; +import { + localHttpTokenCollectionHandler, + type TokenCollectionHandler +} from './tokenCollectionServer.js'; import type { HttpRequest } from './types.js'; /** - * The per-call `sendRequest` a tool receives via `RequestHandlerExtra` — bound - * correctly to whichever session actually made the current call, unlike a `Server` - * instance stashed on `this` (see {@link elicitPreviewToken} for why that matters). + * The per-call `sendRequest`/`sendNotification` a tool receives via `RequestHandlerExtra` + * — bound correctly to whichever session actually made the current call, unlike a + * `Server` instance stashed on `this` (see {@link elicitPreviewToken} for why that + * matters). */ // eslint-disable-next-line @typescript-eslint/no-explicit-any type SendRequest = RequestHandlerExtra['sendRequest']; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type SendNotification = RequestHandlerExtra['sendNotification']; /** * How long to wait for the client to answer an elicitation prompt before giving up. @@ -35,10 +42,35 @@ type SendRequest = RequestHandlerExtra['sendRequest']; const ELICITATION_TIMEOUT_MSEC = 60_000; /** Real Mapbox tokens are well under this; anything longer is almost certainly not a - * token at all. Enforced server-side regardless of what a client's form UI does with - * the `maxLength` hint in the requested schema below — that hint is advisory only. */ + * token at all. Enforced against whatever a user submits via URL-mode token collection + * (see {@link validatePublicPreviewToken}) — the local collection page's own `maxLength` + * attribute is advisory only, since it's just client-side HTML. */ const MAX_TOKEN_LENGTH = 2048; +/** + * How long to wait for the out-of-band URL-mode collection flow to complete once the + * user has agreed to open the link — separate from, and much longer than, + * {@link ELICITATION_TIMEOUT_MSEC}, since this step requires a human to actually open a + * browser and type something, not just click a button in an already-open dialog. + */ +const URL_MODE_COLLECTION_TIMEOUT_MSEC = 5 * 60_000; + +/** + * Set to `"false"` to disable URL-mode token collection entirely and fall straight back + * to the "client does not support elicitation, please provide accessToken directly" + * message. This exists because the default {@link TokenCollectionHandler} + * (`localHttpTokenCollectionHandler`) starts a server bound to `127.0.0.1` on *this* + * process's machine — correct when this package runs as a local stdio server (its only + * shipped entry point, `src/index.ts`), but wrong for a deployment where this process + * runs somewhere other than the end user's own machine (e.g. hosted-mcp-server, a cloud + * deployment): the URL would point at the *browser's* loopback interface, where nothing + * is listening. Such deployments MUST set this to `"false"` until they supply their own + * `TokenCollectionHandler` implementation. + */ +function isLocalUrlElicitationEnabled(): boolean { + return process.env.ENABLE_LOCAL_URL_ELICITATION !== 'false'; +} + const MAX_TOKEN_NOTE_LENGTH = 256; /** Matches CreateTokenTool's own `allowedUrls` cap. */ @@ -95,11 +127,13 @@ export interface CreatePreviewTokenResult { } /** - * Result of token elicitation + * Result of the initial (form-mode) token-choice elicitation. Deliberately does not + * carry a token value — see {@link collectProvidedToken} for how the `'provide'` choice + * is followed up on. `tokenNote`/`urlRestrictions` are non-sensitive configuration for + * the `'create'` choice, not credentials, so they stay in form mode. */ export interface ElicitedTokenInfo { choice: TokenChoice; - token?: string; urlRestrictions?: string[]; tokenNote?: string; } @@ -170,10 +204,13 @@ export async function elicitPreviewToken( const creationNote = canCreateTokens ? 'For best security, consider using a URL-restricted token that only works on your domains.' : "This server is authenticated with a temporary session token, which can't create new " + - 'Mapbox tokens. Paste an existing public token (pk.*) with styles:read scope below.'; + 'Mapbox tokens. Choose "I have a token to provide" below.'; // Mirrors what Server#elicitInput() builds internally (method + form-mode params), - // but sent via the current call's own sendRequest rather than a stashed Server. + // but sent via the current call's own sendRequest rather than a stashed Server. Note + // there is no `token` field here: the MCP spec requires credentials to be collected + // via URL-mode elicitation, not form mode (see collectProvidedToken below) — only + // non-sensitive choice/configuration fields belong in this dialog. let result; try { result = await sendRequest( @@ -187,7 +224,9 @@ Preview URLs require a public token with styles:read scope. This token will be v ${hasExistingTokens ? 'Your existing public tokens:\n' + tokenList : tokenList} -${creationNote}`, +${creationNote} + +If you choose "I have a token to provide", you'll get a separate link to submit it securely — the token itself is never entered into this form.`, requestedSchema: { type: 'object', properties: { @@ -198,14 +237,6 @@ ${creationNote}`, enum: choices, enumNames: choiceNames }, - token: { - type: 'string', - title: 'Your Token', - description: - 'Paste your public Mapbox token here (must have styles:read scope)', - minLength: 10, - maxLength: MAX_TOKEN_LENGTH - }, tokenNote: { type: 'string', title: 'Token Name (Optional)', @@ -270,11 +301,6 @@ ${creationNote}`, } const choice = rawChoice as TokenChoice; - const token = result.content.token; - if (token !== undefined && typeof token !== 'string') { - throw new Error('Client returned a non-string value for the token field.'); - } - const tokenNote = result.content.tokenNote; if (tokenNote !== undefined && typeof tokenNote !== 'string') { throw new Error( @@ -309,13 +335,7 @@ ${creationNote}`, // The requestedSchema's minLength/maxLength are hints for the client's own form UI, // not a security boundary — nothing stops a client (malicious, or just not honoring - // the hints) from returning arbitrary content. Enforced here too, since whatever - // comes back as `token` ends up cached indefinitely in previewTokenStorage. - if (token !== undefined && token.length > MAX_TOKEN_LENGTH) { - throw new Error( - `Provided token is ${token.length} characters, which exceeds the ${MAX_TOKEN_LENGTH}-character maximum for a Mapbox token.` - ); - } + // the hints) from returning arbitrary content. if (tokenNote !== undefined && tokenNote.length > MAX_TOKEN_NOTE_LENGTH) { throw new Error( `Token name is ${tokenNote.length} characters, which exceeds the ${MAX_TOKEN_NOTE_LENGTH}-character maximum.` @@ -330,25 +350,130 @@ ${creationNote}`, ); } - // Every other path that produces a preview token (the `accessToken` input parameter, - // and the create/auto-create API responses) is checked against this same prefix — - // this was the one path that wasn't, which meant pasting a secret token (sk.*) into - // the elicitation dialog embedded it straight into the returned preview URL and - // cached it for reuse, exactly the leak this feature exists to prevent. - if (choice === 'provide' && token !== undefined && !token.startsWith('pk.')) { - throw new Error( - 'Invalid access token. Only public tokens (starting with pk.*) are allowed for preview URLs. Secret tokens (sk.*) cannot be used as they cannot be exposed in browser URLs.' - ); - } - return { choice, - token, urlRestrictions, tokenNote }; } +/** + * Validates a candidate preview token submitted through URL-mode collection. Applies + * the exact same rules every other path that produces a preview token already enforces + * (the `accessToken` input parameter, and the create/auto-create API responses): must + * be a non-empty string, under {@link MAX_TOKEN_LENGTH}, and `pk.*`-prefixed. A secret + * token (`sk.*`) submitted here is rejected for the same reason it's rejected + * everywhere else — it cannot be safely exposed in a preview URL. + */ +export function validatePublicPreviewToken(token: string): string { + if (token.length === 0) { + throw new Error('No token provided. Please provide a valid public token.'); + } + if (token.length > MAX_TOKEN_LENGTH) { + throw new Error( + `Provided token is ${token.length} characters, which exceeds the ${MAX_TOKEN_LENGTH}-character maximum for a Mapbox token.` + ); + } + if (!token.startsWith('pk.')) { + throw new Error( + 'Invalid access token. Only public tokens (starting with pk.*) are allowed for preview URLs. Secret tokens (sk.*) cannot be used as they cannot be exposed in browser URLs.' + ); + } + return token; +} + +/** + * Collects a preview token via URL-mode elicitation, per the MCP spec's requirement + * that credentials MUST NOT be requested via form mode. Sends a `mode: 'url'` + * elicitation pointing at the URL `tokenCollectionHandler.collect()` provides, waits for + * the user to both consent (the elicitation response) and actually submit a value (the + * out-of-band collection promise), then validates it exactly like every other token + * source in this file. + * + * @throws {ElicitationUnavailableError} if URL-mode collection is disabled + * ({@link isLocalUrlElicitationEnabled}) or the client can't be asked at all (e.g. it + * doesn't support URL-mode elicitation) — callers should treat this exactly like the + * existing "client doesn't support elicitation" fallback. + * @throws {Error} if the client was asked but declined/cancelled, if the out-of-band + * submission times out, or if the submitted value fails validation. + */ +export async function collectProvidedToken( + sendRequest: SendRequest, + sendNotification: SendNotification, + tokenCollectionHandler: TokenCollectionHandler = localHttpTokenCollectionHandler +): Promise { + if (!isLocalUrlElicitationEnabled()) { + throw new ElicitationUnavailableError( + 'URL-mode token collection is disabled on this deployment (ENABLE_LOCAL_URL_ELICITATION=false).' + ); + } + + const { url, result, cancel } = await tokenCollectionHandler.collect({ + timeoutMs: URL_MODE_COLLECTION_TIMEOUT_MSEC + }); + + const elicitationId = randomUUID(); + let consent; + try { + consent = await sendRequest( + { + method: 'elicitation/create', + params: { + mode: 'url', + elicitationId, + message: + 'Please provide your Mapbox public token to continue. This opens a page ' + + 'served locally on your own machine — the token goes directly to the MCP ' + + 'DevKit server process, not through this chat.', + url + } + }, + ElicitResultSchema, + { timeout: ELICITATION_TIMEOUT_MSEC } + ); + } catch (error) { + cancel(); + if ( + error instanceof McpError && + (error.code === ErrorCode.RequestTimeout || + (error.code === ErrorCode.InvalidRequest && + /cancelled/i.test(error.message))) + ) { + throw error; + } + throw new ElicitationUnavailableError( + error instanceof Error ? error.message : String(error) + ); + } + + if (consent.action !== 'accept') { + cancel(); + throw new Error('Token elicitation was cancelled or declined by user'); + } + + let rawToken: string; + try { + rawToken = await result; + } catch (error) { + throw error instanceof Error ? error : new Error(String(error)); + } + + const token = validatePublicPreviewToken(rawToken); + + // Best-effort UX signal that the out-of-band flow completed; not required for + // correctness (our own `await result` above is what actually drives completion). + try { + await sendNotification({ + method: 'notifications/elicitation/complete', + params: { elicitationId } + }); + } catch { + // Ignore — some clients may not support this notification at all. + } + + return token; +} + /** * Derives a `previewTokenStorage` cache key from the server's own access token, rather * than from the username decoded out of it. `getUserNameFromToken` never verifies a diff --git a/test/integration/elicitationOverHttp.test.ts b/test/integration/elicitationOverHttp.test.ts index d0786e6..5a916eb 100644 --- a/test/integration/elicitationOverHttp.test.ts +++ b/test/integration/elicitationOverHttp.test.ts @@ -37,6 +37,7 @@ import { import { PreviewStyleTool } from '../../src/tools/preview-style-tool/PreviewStyleTool.js'; import { StyleComparisonTool } from '../../src/tools/style-comparison-tool/StyleComparisonTool.js'; import { previewTokenStorage } from '../../src/utils/tokenElicitation.js'; +import type { TokenCollectionHandler } from '../../src/utils/tokenCollectionServer.js'; import type { HttpRequest } from '../../src/utils/types.js'; const TK_SERVER_TOKEN = @@ -79,6 +80,21 @@ interface TestHarness { close(): Promise; } +/** A fake TokenCollectionHandler that resolves immediately with `token`, instead of + * starting a real local server and waiting for an actual browser submission that will + * never come in a test — the URL-mode consent round trip itself is still real (driven + * by the real Client/Server elicitation exchange below); only the out-of-band + * submission step is faked. */ +function fakeTokenCollectionHandler(token: string): TokenCollectionHandler { + return { + collect: vi.fn().mockResolvedValue({ + url: 'http://127.0.0.1:1/fake-collection-url', + result: Promise.resolve(token), + cancel: vi.fn() + }) + }; +} + /** * Session-scoped stateful Streamable HTTP server: one `McpServer`/transport pair per * `Mcp-Session-Id`, created on the first (`initialize`) request and reused for every @@ -101,11 +117,18 @@ interface TestHarness { */ function startHarness( previewHttpRequest: HttpRequest, - comparisonHttpRequest: HttpRequest = previewHttpRequest + comparisonHttpRequest: HttpRequest = previewHttpRequest, + tokenCollectionHandler: TokenCollectionHandler = fakeTokenCollectionHandler( + EXISTING_PUBLIC_TOKEN + ) ): Promise { - const previewTool = new PreviewStyleTool({ httpRequest: previewHttpRequest }); + const previewTool = new PreviewStyleTool({ + httpRequest: previewHttpRequest, + tokenCollectionHandler + }); const comparisonTool = new StyleComparisonTool({ - httpRequest: comparisonHttpRequest + httpRequest: comparisonHttpRequest, + tokenCollectionHandler }); const sessions = new Map(); @@ -194,7 +217,10 @@ async function connectClient( ): Promise { const client = new Client( { name: 'elicitation-http-test-client', version: '1.0.0' }, - { capabilities: { elicitation: {} } } + // Declare both modes — an empty `elicitation: {}` is normalized by the SDK to + // form-only support, which would make the follow-up URL-mode request (see + // collectProvidedToken) look unsupported to these tests. + { capabilities: { elicitation: { form: {}, url: {} } } } ); client.setRequestHandler(ElicitRequestSchema, (request) => onElicit(request)); @@ -206,9 +232,10 @@ async function connectClient( } /** - * `ElicitRequest.params` is a union (form-mode vs. other elicitation modes); this - * server only ever sends the form-mode shape (`message` + `requestedSchema`) that - * `elicitPreviewToken` builds, so narrowing here is safe for these tests. + * `ElicitRequest.params` is a union (form-mode vs. url-mode); this narrows to the + * form-mode shape (`message` + `requestedSchema`) that `elicitPreviewToken` builds for + * the initial choice dialog — callers must only invoke this for that request, not for + * the follow-up URL-mode consent request `collectProvidedToken` sends afterward. */ function getChoiceEnum(request: ElicitRequest): string[] { const params = request.params as unknown as { @@ -249,11 +276,14 @@ describe('preview/comparison token elicitation over real Streamable HTTP', () => harness.baseUrl, TK_SERVER_TOKEN, (request) => { + // First the form-mode choice dialog, then the follow-up URL-mode consent + // request (see collectProvidedToken) — content is irrelevant for the + // latter; the actual token comes from the fake tokenCollectionHandler. + if (request.params.mode === 'url') { + return { action: 'accept' }; + } receivedEnum = getChoiceEnum(request); - return { - action: 'accept', - content: { choice: 'provide', token: EXISTING_PUBLIC_TOKEN } - }; + return { action: 'accept', content: { choice: 'provide' } }; } ); @@ -338,11 +368,14 @@ describe('preview/comparison token elicitation over real Streamable HTTP', () => harness.baseUrl, TK_SERVER_TOKEN, (request) => { + // First the form-mode choice dialog, then the follow-up URL-mode consent + // request (see collectProvidedToken) — content is irrelevant for the + // latter; the actual token comes from the fake tokenCollectionHandler. + if (request.params.mode === 'url') { + return { action: 'accept' }; + } receivedEnum = getChoiceEnum(request); - return { - action: 'accept', - content: { choice: 'provide', token: EXISTING_PUBLIC_TOKEN } - }; + return { action: 'accept', content: { choice: 'provide' } }; } ); diff --git a/test/security/cross-session-elicitation-hijack.test.ts b/test/security/cross-session-elicitation-hijack.test.ts index e336201..72818c9 100644 --- a/test/security/cross-session-elicitation-hijack.test.ts +++ b/test/security/cross-session-elicitation-hijack.test.ts @@ -40,6 +40,7 @@ import { } from '@modelcontextprotocol/sdk/types.js'; import { PreviewStyleTool } from '../../src/tools/preview-style-tool/PreviewStyleTool.js'; import { previewTokenStorage } from '../../src/utils/tokenElicitation.js'; +import type { TokenCollectionHandler } from '../../src/utils/tokenCollectionServer.js'; import type { HttpRequest } from '../../src/utils/types.js'; // Non-tk.* tokens so `canCreateTokens` is true and the code actually awaits @@ -50,16 +51,28 @@ const SESSION_B_TOKEN = 'sk.eyJ1IjoidGVzdC11c2VyLWIiLCJhIjoidGVzdC1hcGkifQ.signature-b'; // Realistic pk..sig shape — PreviewStyleTool decodes the `u` claim -// out of whichever token elicitation returns to build the preview URL, so these need to -// parse cleanly for the test to observe which one actually made it into the result. +// out of whichever token collectProvidedToken returns to build the preview URL, so this +// needs to parse cleanly for the test to observe that it made it into the result. const SESSION_A_SUPPLIED_TOKEN = 'pk.eyJ1IjoiYXR0YWNrZXItYWNjb3VudCJ9.sig-a'; -const SESSION_B_SUPPLIED_TOKEN = 'pk.eyJ1IjoidmljdGltLWFjY291bnQifQ.sig-b'; interface Harness { baseUrl: URL; close(): Promise; } +/** A fake TokenCollectionHandler that resolves immediately with `token`, instead of + * starting a real local server and waiting for an actual browser submission that will + * never come in a test. */ +function fakeTokenCollectionHandler(token: string): TokenCollectionHandler { + return { + collect: vi.fn().mockResolvedValue({ + url: 'http://127.0.0.1:1/fake-collection-url', + result: Promise.resolve(token), + cancel: vi.fn() + }) + }; +} + /** * Installs a *shared* `PreviewStyleTool` instance onto a fresh `McpServer` for every * new session — mirroring how CORE_TOOLS' singletons get reused across sessions in a @@ -151,7 +164,11 @@ async function connectClient( ): Promise { const client = new Client( { name: 'cross-session-hijack-test-client', version: '1.0.0' }, - { capabilities: { elicitation: {} } } + // Declare both modes — an empty `elicitation: {}` is normalized by the SDK to + // form-only support, which would make the follow-up URL-mode request (see + // collectProvidedToken) look unsupported and short-circuit to a different + // fallback path than the one this test is actually exercising. + { capabilities: { elicitation: { form: {}, url: {} } } } ); client.setRequestHandler(ElicitRequestSchema, (request) => onElicit(request)); @@ -186,18 +203,29 @@ describe('cross-session elicitation hijack (singleton tool instance reused acros ) as unknown as HttpRequest; // ONE shared instance, installed onto two different sessions below — this is the - // exact shape of CORE_TOOLS being reused across concurrent sessions. - const previewTool = new PreviewStyleTool({ httpRequest }); + // exact shape of CORE_TOOLS being reused across concurrent sessions. Only session + // A's tool call actually completes an elicitation flow in this test, so a single + // fake token-collection handler resolving with session A's token is enough. + const previewTool = new PreviewStyleTool({ + httpRequest, + tokenCollectionHandler: fakeTokenCollectionHandler( + SESSION_A_SUPPLIED_TOKEN + ) + }); harness = await startSharedSingletonHarness(previewTool); + // Answers both the form-mode choice dialog and the follow-up URL-mode consent + // request with the same "accept" response — content.token is unused for either + // step now (see collectProvidedToken); the actual token comes from the fake + // tokenCollectionHandler above instead. const elicitReceivedByA = vi.fn().mockReturnValue({ action: 'accept', - content: { choice: 'provide', token: SESSION_A_SUPPLIED_TOKEN } + content: { choice: 'provide' } }); const elicitReceivedByB = vi.fn().mockReturnValue({ action: 'accept', - content: { choice: 'provide', token: SESSION_B_SUPPLIED_TOKEN } + content: { choice: 'provide' } }); // Session A connects and installs the shared tool onto its own McpServer — @@ -229,8 +257,10 @@ describe('cross-session elicitation hijack (singleton tool instance reused acros }); // The fix: session A's own client must be the one asked, regardless of what - // installTo() calls happened on the shared tool instance in the meantime. - expect(elicitReceivedByA).toHaveBeenCalledTimes(1); + // installTo() calls happened on the shared tool instance in the meantime. Called + // twice — the form-mode choice dialog, then the follow-up URL-mode consent + // request (see collectProvidedToken) — both correctly routed to session A. + expect(elicitReceivedByA).toHaveBeenCalledTimes(2); expect(elicitReceivedByB).not.toHaveBeenCalled(); // And the result that comes back for session A's tool call must reflect session diff --git a/test/tools/preview-style-tool/PreviewStyleTool.test.ts b/test/tools/preview-style-tool/PreviewStyleTool.test.ts index bbc9741..cb2cff7 100644 --- a/test/tools/preview-style-tool/PreviewStyleTool.test.ts +++ b/test/tools/preview-style-tool/PreviewStyleTool.test.ts @@ -10,8 +10,22 @@ import { cacheKeyFor, previewTokenStorage } from '../../../src/utils/tokenElicitation.js'; +import type { TokenCollectionHandler } from '../../../src/utils/tokenCollectionServer.js'; import { setupHttpRequest } from '../../utils/httpPipelineUtils.js'; +/** A fake TokenCollectionHandler that resolves immediately with `token`, instead of + * starting a real local server and waiting for an actual browser submission that will + * never come in a test. */ +function fakeTokenCollectionHandler(token: string): TokenCollectionHandler { + return { + collect: vi.fn().mockResolvedValue({ + url: 'http://127.0.0.1:1/fake-collection-url', + result: Promise.resolve(token), + cancel: vi.fn() + }) + }; +} + describe('PreviewStyleTool', () => { const TEST_ACCESS_TOKEN = 'pk.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; @@ -255,15 +269,23 @@ describe('PreviewStyleTool', () => { it('omits create/auto options and skips token creation calls when the server token is temporary (tk.*)', async () => { const { httpRequest, mockHttpRequest } = setupHttpRequest(); - const tool = new PreviewStyleTool({ httpRequest }); + const tokenCollectionHandler = + fakeTokenCollectionHandler(TEST_ACCESS_TOKEN); + const tool = new PreviewStyleTool({ + httpRequest, + tokenCollectionHandler + }); // The per-call sendRequest a real MCP session would pass via `extra` — // not a stashed `this.server`, which a singleton tool instance can't - // safely rely on across sessions (see tokenElicitation.ts). + // safely rely on across sessions (see tokenElicitation.ts). Answers both + // the form-mode choice dialog and the follow-up URL-mode consent request + // with the same "accept" response. const sendRequest = vi.fn().mockResolvedValue({ action: 'accept', - content: { choice: 'provide', token: TEST_ACCESS_TOKEN } + content: { choice: 'provide' } }); + const sendNotification = vi.fn().mockResolvedValue(undefined); const tkToken = 'tk.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; @@ -271,14 +293,19 @@ describe('PreviewStyleTool', () => { const result = await tool.run( { styleId: 'test-style' }, // eslint-disable-next-line @typescript-eslint/no-explicit-any - { authInfo: { token: tkToken }, sendRequest } as any + { authInfo: { token: tkToken }, sendRequest, sendNotification } as any ); expect(result.isError).toBe(false); - expect(sendRequest).toHaveBeenCalledTimes(1); + // First call is the form-mode choice dialog, second is the follow-up + // URL-mode elicitation for the token itself (see collectProvidedToken). + expect(sendRequest).toHaveBeenCalledTimes(2); const requestedSchema = sendRequest.mock.calls[0][0].params.requestedSchema; expect(requestedSchema.properties.choice.enum).toEqual(['provide']); + expect(requestedSchema.properties.token).toBeUndefined(); + expect(sendRequest.mock.calls[1][0].params.mode).toBe('url'); + expect(tokenCollectionHandler.collect).toHaveBeenCalledTimes(1); // A tk.* server token can never create tokens (tokens:write), but listing only // needs tokens:read — a separate scope — so it's still attempted (and fails diff --git a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts index b9f3996..86102aa 100644 --- a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts +++ b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts @@ -8,6 +8,7 @@ import { cacheKeyFor, previewTokenStorage } from '../../../src/utils/tokenElicitation.js'; +import type { TokenCollectionHandler } from '../../../src/utils/tokenCollectionServer.js'; import { setupHttpRequest } from '../../utils/httpPipelineUtils.js'; function styleComparisonTool() { @@ -15,6 +16,19 @@ function styleComparisonTool() { return new StyleComparisonTool({ httpRequest }); } +/** A fake TokenCollectionHandler that resolves immediately with `token`, instead of + * starting a real local server and waiting for an actual browser submission that will + * never come in a test. */ +function fakeTokenCollectionHandler(token: string): TokenCollectionHandler { + return { + collect: vi.fn().mockResolvedValue({ + url: 'http://127.0.0.1:1/fake-collection-url', + result: Promise.resolve(token), + cancel: vi.fn() + }) + }; +} + describe('StyleComparisonTool', () => { let tool: StyleComparisonTool; @@ -320,15 +334,23 @@ describe('StyleComparisonTool', () => { it('omits create/auto options and skips token creation calls when the server token is temporary (tk.*)', async () => { const { httpRequest, mockHttpRequest } = setupHttpRequest(); - const tool = new StyleComparisonTool({ httpRequest }); + const tokenCollectionHandler = + fakeTokenCollectionHandler('pk.test.token'); + const tool = new StyleComparisonTool({ + httpRequest, + tokenCollectionHandler + }); // The per-call sendRequest a real MCP session would pass via `extra` — // not a stashed `this.server`, which a singleton tool instance can't - // safely rely on across sessions (see tokenElicitation.ts). + // safely rely on across sessions (see tokenElicitation.ts). Answers both + // the form-mode choice dialog and the follow-up URL-mode consent request + // with the same "accept" response. const sendRequest = vi.fn().mockResolvedValue({ action: 'accept', - content: { choice: 'provide', token: 'pk.test.token' } + content: { choice: 'provide' } }); + const sendNotification = vi.fn().mockResolvedValue(undefined); const tkToken = 'tk.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; @@ -336,14 +358,19 @@ describe('StyleComparisonTool', () => { const result = await tool.run( { before: 'mapbox/streets-v12', after: 'mapbox/satellite-v9' }, // eslint-disable-next-line @typescript-eslint/no-explicit-any - { authInfo: { token: tkToken }, sendRequest } as any + { authInfo: { token: tkToken }, sendRequest, sendNotification } as any ); expect(result.isError).toBe(false); - expect(sendRequest).toHaveBeenCalledTimes(1); + // First call is the form-mode choice dialog, second is the follow-up + // URL-mode elicitation for the token itself (see collectProvidedToken). + expect(sendRequest).toHaveBeenCalledTimes(2); const requestedSchema = sendRequest.mock.calls[0][0].params.requestedSchema; expect(requestedSchema.properties.choice.enum).toEqual(['provide']); + expect(requestedSchema.properties.token).toBeUndefined(); + expect(sendRequest.mock.calls[1][0].params.mode).toBe('url'); + expect(tokenCollectionHandler.collect).toHaveBeenCalledTimes(1); // A tk.* server token can never create tokens (tokens:write), but listing only // needs tokens:read — a separate scope — so it's still attempted (and fails diff --git a/test/utils/tokenCollectionServer.test.ts b/test/utils/tokenCollectionServer.test.ts new file mode 100644 index 0000000..8e1bec8 --- /dev/null +++ b/test/utils/tokenCollectionServer.test.ts @@ -0,0 +1,141 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect } from 'vitest'; +import { LocalHttpTokenCollectionHandler } from '../../src/utils/tokenCollectionServer.js'; + +describe('LocalHttpTokenCollectionHandler', () => { + it('binds to the loopback interface only', async () => { + const handler = new LocalHttpTokenCollectionHandler(); + const { url, result, cancel } = await handler.collect({ timeoutMs: 5000 }); + // Attach the rejection assertion before cancelling so it's never briefly unhandled. + const assertion = expect(result).rejects.toThrow(/cancelled/); + + expect(url).toMatch(/^http:\/\/127\.0\.0\.1:\d+\/[0-9a-f]{48}$/); + cancel(); + await assertion; + }); + + it('resolves result with the submitted token on POST', async () => { + const handler = new LocalHttpTokenCollectionHandler(); + const { url, result } = await handler.collect({ timeoutMs: 5000 }); + + const response = await fetch(url, { + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ token: 'pk.submitted-token' }).toString() + }); + + expect(response.status).toBe(200); + await expect(result).resolves.toBe('pk.submitted-token'); + }); + + it('returns 404 for any path other than the assigned one', async () => { + const handler = new LocalHttpTokenCollectionHandler(); + const { url, result, cancel } = await handler.collect({ timeoutMs: 5000 }); + const assertion = expect(result).rejects.toThrow(/cancelled/); + const wrongUrl = new URL(url); + wrongUrl.pathname = '/some-other-path'; + + const response = await fetch(wrongUrl); + expect(response.status).toBe(404); + cancel(); + await assertion; + }); + + it('serves a form page on GET', async () => { + const handler = new LocalHttpTokenCollectionHandler(); + const { url, result, cancel } = await handler.collect({ timeoutMs: 5000 }); + const assertion = expect(result).rejects.toThrow(/cancelled/); + + const response = await fetch(url); + expect(response.status).toBe(200); + expect(response.headers.get('content-type')).toContain('text/html'); + const body = await response.text(); + expect(body).toContain(' { + const handler = new LocalHttpTokenCollectionHandler(); + const { url, result, cancel } = await handler.collect({ timeoutMs: 5000 }); + + const response = await fetch(url, { + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({}).toString() + }); + + expect(response.status).toBe(400); + + // The server is still up and can accept a subsequent valid submission. + const retry = await fetch(url, { + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ token: 'pk.retry-token' }).toString() + }); + expect(retry.status).toBe(200); + await expect(result).resolves.toBe('pk.retry-token'); + cancel(); + }); + + it('rejects a body exceeding the maximum accepted size', async () => { + const handler = new LocalHttpTokenCollectionHandler(); + const { url, result } = await handler.collect({ timeoutMs: 5000 }); + // Attach the assertion before the request so it's never briefly unhandled, + // regardless of exactly when the server-side rejection actually settles. + const assertion = expect(result).rejects.toThrow( + /exceeds the maximum accepted size/ + ); + + const hugeBody = new URLSearchParams({ + token: 'pk.' + 'a'.repeat(20 * 1024) + }).toString(); + + // The connection may be reset mid-write once the size cap is hit; either + // outcome (fetch throwing, or a non-2xx response) is an acceptable way for the + // oversized submission to fail — what matters is `result` never resolves with it. + await fetch(url, { + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body: hugeBody + }).catch(() => undefined); + + await assertion; + }); + + it('rejects after the timeout elapses and closes the server', async () => { + const handler = new LocalHttpTokenCollectionHandler(); + const { url, result } = await handler.collect({ timeoutMs: 50 }); + + await expect(result).rejects.toThrow(/Timed out after 50ms/); + + // The server has been torn down; a request to it should now fail to connect. + await expect(fetch(url)).rejects.toThrow(); + }); + + it('cancel() closes the server without resolving or rejecting result observably as a "submitted" outcome', async () => { + const handler = new LocalHttpTokenCollectionHandler(); + const { url, result, cancel } = await handler.collect({ timeoutMs: 5000 }); + + cancel(); + + await expect(result).rejects.toThrow(/cancelled/); + await expect(fetch(url)).rejects.toThrow(); + }); + + it('cancel() after result has already settled is a safe no-op', async () => { + const handler = new LocalHttpTokenCollectionHandler(); + const { url, result, cancel } = await handler.collect({ timeoutMs: 5000 }); + + await fetch(url, { + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ token: 'pk.already-done' }).toString() + }); + await expect(result).resolves.toBe('pk.already-done'); + + expect(() => cancel()).not.toThrow(); + }); +}); diff --git a/test/utils/tokenElicitation.test.ts b/test/utils/tokenElicitation.test.ts index 514b777..850d5c0 100644 --- a/test/utils/tokenElicitation.test.ts +++ b/test/utils/tokenElicitation.test.ts @@ -1,17 +1,20 @@ // Copyright (c) Mapbox, Inc. // Licensed under the MIT License. -import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; import { cacheKeyFor, + collectProvidedToken, createPreviewToken, ElicitationUnavailableError, elicitPreviewToken, isTemporaryServerToken, listPublicPreviewTokens, - previewTokenStorage + previewTokenStorage, + validatePublicPreviewToken } from '../../src/utils/tokenElicitation.js'; +import type { TokenCollectionHandler } from '../../src/utils/tokenCollectionServer.js'; import { setupHttpRequest } from './httpPipelineUtils.js'; const MAPBOX_API_ENDPOINT = 'https://api.mapbox.com/'; @@ -361,7 +364,7 @@ describe('elicitPreviewToken', () => { function fakeSendRequest(choice: string) { return vi.fn().mockResolvedValue({ action: 'accept', - content: { choice, token: 'pk.provided-token' } + content: { choice } }); } @@ -415,17 +418,6 @@ describe('elicitPreviewToken', () => { expect(options.timeout).toBeGreaterThan(0); }); - it('rejects a client-returned token that exceeds the server-enforced max length, regardless of the schema hint', async () => { - const sendRequest = vi.fn().mockResolvedValue({ - action: 'accept', - content: { choice: 'provide', token: 'pk.' + 'a'.repeat(3000) } - }); - - await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( - /exceeds the .* maximum/ - ); - }); - it('rejects a client-returned tokenNote that exceeds the max length', async () => { const sendRequest = vi.fn().mockResolvedValue({ action: 'accept', @@ -470,20 +462,6 @@ describe('elicitPreviewToken', () => { ); }); - it('rejects a secret token submitted via the "provide" choice, the same way the accessToken parameter is rejected', async () => { - const sendRequest = vi.fn().mockResolvedValue({ - action: 'accept', - content: { - choice: 'provide', - token: 'sk.eyJ1IjoidGVzdC11c2VyIn0.secret-signature' - } - }); - - await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( - /Only public tokens \(starting with pk\.\*\) are allowed/ - ); - }); - it('rejects an unrecognized choice value instead of silently treating it as auto-create', async () => { const sendRequest = vi.fn().mockResolvedValue({ action: 'accept', @@ -508,17 +486,6 @@ describe('elicitPreviewToken', () => { ); }); - it('rejects a non-string token instead of silently bypassing the length guard', async () => { - const sendRequest = vi.fn().mockResolvedValue({ - action: 'accept', - content: { choice: 'provide', token: 12345 } - }); - - await expect(elicitPreviewToken(sendRequest, [], true)).rejects.toThrow( - /non-string value for the token field/ - ); - }); - it('rejects a non-string urlRestrictions instead of crashing on .split()', async () => { const sendRequest = vi.fn().mockResolvedValue({ action: 'accept', @@ -584,3 +551,183 @@ describe('elicitPreviewToken', () => { ); }); }); + +describe('validatePublicPreviewToken', () => { + it('returns a valid pk.* token unchanged', () => { + expect(validatePublicPreviewToken('pk.valid-token')).toBe('pk.valid-token'); + }); + + it('rejects an empty string', () => { + expect(() => validatePublicPreviewToken('')).toThrow(/No token provided/); + }); + + it('rejects a token exceeding the max length', () => { + expect(() => validatePublicPreviewToken('pk.' + 'a'.repeat(3000))).toThrow( + /exceeds the .* maximum/ + ); + }); + + it('rejects a secret token, the same way the accessToken parameter is rejected', () => { + expect(() => + validatePublicPreviewToken('sk.eyJ1IjoidGVzdC11c2VyIn0.secret-signature') + ).toThrow(/Only public tokens \(starting with pk\.\*\) are allowed/); + }); +}); + +describe('collectProvidedToken', () => { + const ORIGINAL_ENV = process.env.ENABLE_LOCAL_URL_ELICITATION; + + afterEach(() => { + if (ORIGINAL_ENV === undefined) { + delete process.env.ENABLE_LOCAL_URL_ELICITATION; + } else { + process.env.ENABLE_LOCAL_URL_ELICITATION = ORIGINAL_ENV; + } + }); + + /** A fake TokenCollectionHandler whose `result` settles with `outcome` (a token + * string on success, an Error to reject with). Rejections get a no-op `.catch` + * attached separately so they don't trigger an unhandled-rejection warning before + * the code under test awaits the original `result` promise. */ + function fakeTokenCollectionHandler(outcome: string | Error): { + handler: TokenCollectionHandler; + cancel: ReturnType; + } { + const cancel = vi.fn(); + const result = + outcome instanceof Error + ? Promise.reject(outcome) + : Promise.resolve(outcome); + result.catch(() => {}); + return { + handler: { + collect: vi.fn().mockResolvedValue({ + url: 'http://127.0.0.1:9999/fake-path', + result, + cancel + }) + }, + cancel + }; + } + + function fakeSendRequest(action: 'accept' | 'decline' | 'cancel' = 'accept') { + return vi.fn().mockResolvedValue({ action }); + } + + function fakeSendNotification() { + return vi.fn().mockResolvedValue(undefined); + } + + it('sends a URL-mode (not form-mode) elicitation request', async () => { + const sendRequest = fakeSendRequest(); + const { handler } = fakeTokenCollectionHandler('pk.good-token'); + + await collectProvidedToken(sendRequest, fakeSendNotification(), handler); + + const request = sendRequest.mock.calls[0][0]; + expect(request.params.mode).toBe('url'); + expect(request.params.url).toBe('http://127.0.0.1:9999/fake-path'); + expect(request.params.elicitationId).toEqual(expect.any(String)); + }); + + it('returns the validated token once the client accepts and the out-of-band submission resolves', async () => { + const { handler } = fakeTokenCollectionHandler('pk.good-token'); + + const token = await collectProvidedToken( + fakeSendRequest(), + fakeSendNotification(), + handler + ); + + expect(token).toBe('pk.good-token'); + }); + + it('rejects a secret token submitted through URL-mode collection', async () => { + const { handler } = fakeTokenCollectionHandler( + 'sk.eyJ1IjoidGVzdC11c2VyIn0.secret-signature' + ); + + await expect( + collectProvidedToken(fakeSendRequest(), fakeSendNotification(), handler) + ).rejects.toThrow( + /Only public tokens \(starting with pk\.\*\) are allowed/ + ); + }); + + it('cancels collection and wraps a failed sendRequest in ElicitationUnavailableError', async () => { + const sendRequest = vi + .fn() + .mockRejectedValue(new Error('Method not found')); + const { handler, cancel } = fakeTokenCollectionHandler('pk.good-token'); + + await expect( + collectProvidedToken(sendRequest, fakeSendNotification(), handler) + ).rejects.toThrow(ElicitationUnavailableError); + expect(cancel).toHaveBeenCalledTimes(1); + }); + + it('cancels collection and throws (not ElicitationUnavailableError) when the user declines the URL-mode consent', async () => { + const { handler, cancel } = fakeTokenCollectionHandler('pk.good-token'); + + const promise = collectProvidedToken( + fakeSendRequest('decline'), + fakeSendNotification(), + handler + ); + await expect(promise).rejects.not.toBeInstanceOf( + ElicitationUnavailableError + ); + await expect(promise).rejects.toThrow(/cancelled or declined/); + expect(cancel).toHaveBeenCalledTimes(1); + }); + + it('propagates a timeout from the out-of-band submission itself', async () => { + const { handler } = fakeTokenCollectionHandler( + new Error( + 'Timed out after 300000ms waiting for the token to be submitted.' + ) + ); + + await expect( + collectProvidedToken(fakeSendRequest(), fakeSendNotification(), handler) + ).rejects.toThrow(/Timed out/); + }); + + it('sends notifications/elicitation/complete after a successful collection', async () => { + const sendRequest = fakeSendRequest(); + const sendNotification = fakeSendNotification(); + const { handler } = fakeTokenCollectionHandler('pk.good-token'); + + await collectProvidedToken(sendRequest, sendNotification, handler); + + const elicitationId = sendRequest.mock.calls[0][0].params.elicitationId; + expect(sendNotification).toHaveBeenCalledWith({ + method: 'notifications/elicitation/complete', + params: { elicitationId } + }); + }); + + it('does not fail overall if sendNotification itself rejects', async () => { + const sendNotification = vi + .fn() + .mockRejectedValue( + new Error('client does not support this notification') + ); + const { handler } = fakeTokenCollectionHandler('pk.good-token'); + + await expect( + collectProvidedToken(fakeSendRequest(), sendNotification, handler) + ).resolves.toBe('pk.good-token'); + }); + + it('short-circuits to ElicitationUnavailableError without starting collection when disabled via env var', async () => { + process.env.ENABLE_LOCAL_URL_ELICITATION = 'false'; + const { handler } = fakeTokenCollectionHandler('pk.good-token'); + + await expect( + collectProvidedToken(fakeSendRequest(), fakeSendNotification(), handler) + ).rejects.toThrow(ElicitationUnavailableError); + expect(handler.collect).not.toHaveBeenCalled(); + }); +}); From 1ee7462a669f6c8f993a92e5fd2c6903a6ab3b56 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Sat, 15 Aug 2026 00:29:55 -0400 Subject: [PATCH 25/25] Flip ENABLE_LOCAL_URL_ELICITATION to opt-in, default disabled Defaulting the local-loopback URL-mode token collection to enabled and relying on every embedder to remember to opt out (as the previous commit did) puts safety on the wrong side of the default: forgetting to set the var is silent until a client happens to support URL-mode elicitation, at which point "provide" sends a real user down a dead end (a 127.0.0.1 URL their browser can't reach, since the server process runs somewhere else). Flipped: ENABLE_LOCAL_URL_ELICITATION now defaults to disabled, and only src/index.ts (this package's own stdio entry point -- the one context confirmed to run on the same machine as the user's browser) sets it to "true" automatically, without clobbering an explicit override from the environment or a .env file. Every other way these tools get embedded (hosted-mcp-server, a future unknown embedder, a test harness) now stays safe by default and must deliberately opt in. This also means hosted-mcp-server needs no follow-up change at all -- it imports getAllTools()/tool classes directly and never executes index.ts, so it never sets the var and "provide" is simply unavailable there by default, same as a client without elicitation support. Updated collectProvidedToken's tests (explicit-disable and now default-disabled cases) and every test exercising the "provide" flow in PreviewStyleTool/StyleComparisonTool/cross-session-hijack/ elicitationOverHttp to opt in via the env var, simulating what src/index.ts does for real. 690 tests passing; lint/tsc/build clean. --- CHANGELOG.md | 2 +- README.md | 8 +++--- docs/engineering_standards.md | 2 +- src/index.ts | 12 +++++++++ src/utils/tokenElicitation.ts | 26 ++++++++++++------- test/integration/elicitationOverHttp.test.ts | 11 ++++++++ .../cross-session-elicitation-hijack.test.ts | 11 ++++++++ .../PreviewStyleTool.test.ts | 17 +++++++++++- .../StyleComparisonTool.test.ts | 15 +++++++++++ test/utils/tokenElicitation.test.ts | 18 ++++++++++++- 10 files changed, 104 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8f381d..1ffb442 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ - **Secure token elicitation for `preview_style_tool` / `style_comparison_tool`** (#57): `accessToken` is now optional on both tools. When omitted, the tool asks how you'd like to provide a public token — pasting an existing `pk.*` token, creating a new one with optional URL restrictions, or auto-creating a minimally-scoped one (`styles:read`, `styles:tiles`, `fonts:read`). This keeps your server's `sk.*`/`pk.*` access token out of chat history and preview URLs. Falls back to requiring `accessToken` directly on clients without elicitation support (Claude Desktop, Claude Code). A chosen/created token is cached in memory per token; pass `useCustomToken: true` to force re-selection. Elicitation requests are routed through the per-call session context rather than a shared server reference, so a tool call's prompt is always delivered back to the session that made it, even when the same tool instance is reused across multiple concurrent sessions. - If the server's own access token is a literal Mapbox temporary token (`tk.*`), the "create a new token" and "auto-create" options are omitted from the dialog up front, since that token shape is guaranteed to lack `tokens:write`. Other callers lacking `tokens:write` (e.g. the hosted MCP endpoint, which authenticates with its own access token rather than a Mapbox `pk.*`/`sk.*`/`tk.*` token) aren't detectable ahead of time; for those, choosing "create"/"auto-create" fails against the Tokens API with a scope/permission hint appended to the error, steering back to "I have a token to provide". - The initial choice dialog has an explicit 60s timeout rather than relying on the SDK's default; a client-returned token name/URL-restriction list that exceeds a sane size is rejected server-side regardless of what the dialog's schema hints suggest a client's form should enforce; and the in-memory token cache is bounded to 1000 entries (LRU eviction), so none of this can grow memory usage without limit. - - **"I have a token to provide" now uses MCP URL-mode elicitation, not a form field.** The MCP spec (SEP-1036) prohibits collecting credentials via form-mode elicitation — a `pk.*` token is still a credential, so the original design (a `token` text field in the same form-mode dialog as the choice picker) didn't comply, and separately meant a pasted `sk.*` secret token skipped the `pk.*` prefix check every other path enforces and got embedded straight into the returned preview URL (found by an adversarial review pass). Choosing "provide" now opens a short-lived HTTP server on `127.0.0.1` (the same pattern `gh auth login`/`gcloud auth login` use) and sends a URL-mode elicitation request pointing at it; the token is submitted directly to that local page, never through the MCP client or chat history, then validated with the same `pk.*`/length checks as every other path. Set `ENABLE_LOCAL_URL_ELICITATION=false` to disable this and fall back to requiring `accessToken` directly — **required** for deployments where the server process doesn't run on the end user's own machine (e.g. a hosted/cloud deployment), since a `127.0.0.1` URL there wouldn't resolve to anything the user's browser could reach. + - **"I have a token to provide" now uses MCP URL-mode elicitation, not a form field.** The MCP spec (SEP-1036) prohibits collecting credentials via form-mode elicitation — a `pk.*` token is still a credential, so the original design (a `token` text field in the same form-mode dialog as the choice picker) didn't comply, and separately meant a pasted `sk.*` secret token skipped the `pk.*` prefix check every other path enforces and got embedded straight into the returned preview URL (found by an adversarial review pass). Choosing "provide" now opens a short-lived HTTP server on `127.0.0.1` (the same pattern `gh auth login`/`gcloud auth login` use) and sends a URL-mode elicitation request pointing at it; the token is submitted directly to that local page, never through the MCP client or chat history, then validated with the same `pk.*`/length checks as every other path. This is opt-in via `ENABLE_LOCAL_URL_ELICITATION` (default `false`) — only this package's own local stdio entry point (`dist/esm/index.js`) enables it automatically, since that's the one context confirmed to run on the same machine as the user's browser. Any other embedding (e.g. a hosted/cloud deployment, where a `127.0.0.1` URL wouldn't resolve to anything the user's browser could reach) stays safe with no action needed; with it disabled, "provide" falls back to requiring `accessToken` directly, same as a client without elicitation support at all. - Every field the client returns from the choice dialog (choice, token name, URL restrictions) is now validated against its expected runtime type and, for `choice`, against the set of options actually offered — a client returning an unrecognized or wrong-typed value previously either fell through to auto-creating a real token or threw an unclassified `TypeError`. A request timeout or cancellation at either step (the choice dialog or the URL-mode consent request) is now also distinguished from "client doesn't support elicitation", so a caller passing `useCustomToken: true` gets a real error instead of silently getting back the stale cached token it was trying to replace. - Listing a user's existing public tokens (to populate the dialog) only requires `tokens:read`, a separate scope from the `tokens:write` needed to create one — it's no longer skipped for `tk.*`-authenticated servers, which lack the latter but not necessarily the former. - Error messages from a failed token-creation call are now redacted the same way every other Mapbox API tool's errors are, since these two tools don't extend the base class that applies that redaction automatically. A misconfigured endpoint could previously surface the caller's own access token verbatim in the returned error text. diff --git a/README.md b/README.md index 171a5c9..19f58d7 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ Get started by integrating with your preferred AI development environment: - **Claude Desktop**: ⚠️ Not yet supported (Claude will fall back to creating tokens via chat) - **Claude Code**: ⚠️ Not yet supported (provide `accessToken` parameter directly) -Choosing **"I have a token to provide"** doesn't paste the token into a form — the MCP spec requires credentials to go through [URL-mode elicitation](https://modelcontextprotocol.io/specification/2025-11-25/client/elicitation#url-mode-elicitation-requests) instead, so this opens a page served locally on your own machine (`http://127.0.0.1:`) to submit it. This requires a client that supports URL-mode elicitation specifically, and requires the server process to be running on the same machine as your browser — see [`ENABLE_LOCAL_URL_ELICITATION`](#enable_local_url_elicitation). +Choosing **"I have a token to provide"** doesn't paste the token into a form — the MCP spec requires credentials to go through [URL-mode elicitation](https://modelcontextprotocol.io/specification/2025-11-25/client/elicitation#url-mode-elicitation-requests) instead, so this opens a page served locally on your own machine (`http://127.0.0.1:`) to submit it. This requires a client that supports URL-mode elicitation specifically, and only works when the server process is running on the same machine as your browser — running via `dist/esm/index.js` (Claude Desktop, Claude Code, Cursor, VS Code, or any other local stdio client) enables it automatically. See [`ENABLE_LOCAL_URL_ELICITATION`](#enable_local_url_elicitation) if you're embedding this package's tools yourself rather than running it as a local server. **Note on the hosted MCP endpoint**: even on a client with full elicitation support, "create a new token" and "auto-create" will fail on the [hosted endpoint](#hosted-mcp-endpoint) — see below for why. "I have a token to provide" is also unavailable there; use the `accessToken` parameter directly instead. @@ -114,7 +114,7 @@ For detailed setup instructions for different clients and API usage, see the [Ho - `preview_style_tool` / `style_comparison_tool`'s elicitation dialog still offers all three options, but choosing "create a new token" or "auto-create" fails against the Mapbox Tokens API with a scope/permission error (the dialog can't know ahead of time that this particular deployment's token lacks `tokens:write` — see the `isTemporaryServerToken` caveat in `src/utils/tokenElicitation.ts` for tokens where it can tell). - `create_token_tool` is not exposed on the hosted endpoint at all. -**"I have a token to provide" also requires the hosted deployment to set `ENABLE_LOCAL_URL_ELICITATION=false`**: that option now works via URL-mode elicitation to a page served on `127.0.0.1`, which only makes sense when the server process runs on your own machine — see [`ENABLE_LOCAL_URL_ELICITATION`](#enable_local_url_elicitation). Until the hosted deployment sets that variable, treat "provide" as unavailable there too and pass `accessToken` directly instead. Create a token ahead of time from your [Mapbox Account page](https://account.mapbox.com/) if you don't have one. Running this server **locally** with your own `pk.*`/`sk.*` access token (which can carry `tokens:write`) also enables create and auto-create. +**"I have a token to provide" is also unavailable on the hosted endpoint by default**: that option works via URL-mode elicitation to a page served on `127.0.0.1`, which only makes sense when the server process runs on your own machine. This is opt-in (see [`ENABLE_LOCAL_URL_ELICITATION`](#enable_local_url_elicitation)) and only this package's own local stdio entry point (`dist/esm/index.js`) turns it on automatically, so a hosted/cloud deployment stays safe with no action needed — pass `accessToken` directly there instead. Create a token ahead of time from your [Mapbox Account page](https://account.mapbox.com/) if you don't have one. Running this server **locally** with your own `pk.*`/`sk.*` access token (which can carry `tokens:write`) also enables create and auto-create. ### Getting Your Mapbox Access Token @@ -1310,10 +1310,10 @@ By default, the server returns generic error messages. With verbose errors enabl Controls whether `preview_style_tool` / `style_comparison_tool`'s "I have a token to provide" option is offered. Per the MCP spec, servers must not collect credentials via form-mode elicitation, so providing a token instead opens a short-lived HTTP server on `127.0.0.1` and sends a URL-mode elicitation request pointing at it — the same pattern CLI tools like `gh auth login` use. This only works when the server process and your browser are on the same machine. -Defaults to `true`. **Set to `false` for any deployment where the server process does not run on the end user's own machine** (for example, a hosted/cloud deployment) — a `127.0.0.1` URL there would resolve to the browser's own loopback interface, where nothing is listening, rather than the server. With it disabled, choosing "provide" falls back to the same message shown to clients without elicitation support at all: pass `accessToken` directly instead. +**Opt-in, not opt-out: defaults to `false`.** Set to `true` only after confirming the server process and the user's browser really are on the same machine — a `127.0.0.1` URL otherwise resolves to the browser's own loopback interface, where nothing is listening, rather than the server. This package's own local stdio entry point (`dist/esm/index.js`, used by Claude Desktop, Claude Code, Cursor, VS Code, and similar clients) sets this automatically; you only need to set it yourself if you're embedding these tools in your own server rather than running `dist/esm/index.js` directly. With it disabled (the default for any such embedding), choosing "provide" falls back to the same message shown to clients without elicitation support at all: pass `accessToken` directly instead. ```bash -export ENABLE_LOCAL_URL_ELICITATION=false +export ENABLE_LOCAL_URL_ELICITATION=true ``` #### ENABLE_MCP_UI diff --git a/docs/engineering_standards.md b/docs/engineering_standards.md index d561414..c94c7c0 100644 --- a/docs/engineering_standards.md +++ b/docs/engineering_standards.md @@ -110,7 +110,7 @@ Keep secrets out of repositories. Use environment variables for sensitive data: - `MAPBOX_ACCESS_TOKEN` - Required for all Mapbox API operations - `VERBOSE_ERRORS` - Set to `true` for detailed error messages - `ENABLE_MCP_UI` - Controls MCP-UI support (default: `true`) -- `ENABLE_LOCAL_URL_ELICITATION` - Controls whether `preview_style_tool`/`style_comparison_tool` offer URL-mode token collection via a local `127.0.0.1` server (default: `true`). Set to `false` for any deployment where the server process doesn't run on the end user's own machine (see `src/utils/tokenCollectionServer.ts`) — a hosted/cloud deployment MUST set this, since a `127.0.0.1` URL there can't be reached by the user's browser. +- `ENABLE_LOCAL_URL_ELICITATION` - Controls whether `preview_style_tool`/`style_comparison_tool` offer URL-mode token collection via a local `127.0.0.1` server (opt-in, default: `false` — see `src/utils/tokenCollectionServer.ts`). Only `src/index.ts` (this package's stdio entry point) sets it to `true` automatically, since that's the one context confirmed to run on the same machine as the user's browser; any other embedder (e.g. a hosted/cloud deployment) stays safe by default and must not enable this unless it's confirmed the same machine/browser relationship holds. - `OTEL_EXPORTER_OTLP_ENDPOINT` - OpenTelemetry endpoint (optional) - `OTEL_SERVICE_NAME` - Override service name for tracing (optional) diff --git a/src/index.ts b/src/index.ts index 6e824de..609d20f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -52,6 +52,18 @@ if (existsSync(envPath)) { } } +// preview_style_tool / style_comparison_tool's "provide a token" option collects the +// token via a local http://127.0.0.1 server (see src/utils/tokenCollectionServer.ts), +// which only makes sense when the MCP server process and the user's browser are on the +// same machine — true for this stdio entry point, but not for every way this package's +// tools can be embedded (e.g. a cloud deployment). Opt in here, since this is the one +// context confirmed safe, rather than defaulting it on everywhere and relying on every +// other embedder to remember to opt out. Left untouched if already set (by the +// environment or the .env file below), so an explicit override always wins. +if (process.env.ENABLE_LOCAL_URL_ELICITATION === undefined) { + process.env.ENABLE_LOCAL_URL_ELICITATION = 'true'; +} + const versionInfo = getVersionInfo(); // Parse configuration from command-line arguments diff --git a/src/utils/tokenElicitation.ts b/src/utils/tokenElicitation.ts index 5d8170a..86381e5 100644 --- a/src/utils/tokenElicitation.ts +++ b/src/utils/tokenElicitation.ts @@ -56,19 +56,25 @@ const MAX_TOKEN_LENGTH = 2048; const URL_MODE_COLLECTION_TIMEOUT_MSEC = 5 * 60_000; /** - * Set to `"false"` to disable URL-mode token collection entirely and fall straight back - * to the "client does not support elicitation, please provide accessToken directly" - * message. This exists because the default {@link TokenCollectionHandler} - * (`localHttpTokenCollectionHandler`) starts a server bound to `127.0.0.1` on *this* - * process's machine — correct when this package runs as a local stdio server (its only - * shipped entry point, `src/index.ts`), but wrong for a deployment where this process + * Opt-in, not opt-out: defaults to disabled, and must be set to `"true"` to enable + * URL-mode token collection. When disabled (the default), the "provide" choice falls + * straight back to the "client does not support elicitation, please provide accessToken + * directly" message. + * + * The default {@link TokenCollectionHandler} (`localHttpTokenCollectionHandler`) starts + * a server bound to `127.0.0.1` on *this* process's own machine — correct only when this + * package runs as a local stdio server, and wrong for any deployment where this process * runs somewhere other than the end user's own machine (e.g. hosted-mcp-server, a cloud - * deployment): the URL would point at the *browser's* loopback interface, where nothing - * is listening. Such deployments MUST set this to `"false"` until they supply their own - * `TokenCollectionHandler` implementation. + * deployment): there, the URL would point at the *browser's* loopback interface, where + * nothing is listening. This package's own stdio entry point (`src/index.ts`) is the + * only context confirmed safe, so it opts in there explicitly (unless the environment + * already set this var, which is left untouched); every other embedder — this package + * used as a library, a test harness, a future unknown embedder — stays safe by default + * and must deliberately opt in only after confirming the same machine/browser + * relationship holds, rather than relying on every embedder remembering to opt out. */ function isLocalUrlElicitationEnabled(): boolean { - return process.env.ENABLE_LOCAL_URL_ELICITATION !== 'false'; + return process.env.ENABLE_LOCAL_URL_ELICITATION === 'true'; } const MAX_TOKEN_NOTE_LENGTH = 256; diff --git a/test/integration/elicitationOverHttp.test.ts b/test/integration/elicitationOverHttp.test.ts index 5a916eb..bee880a 100644 --- a/test/integration/elicitationOverHttp.test.ts +++ b/test/integration/elicitationOverHttp.test.ts @@ -247,9 +247,14 @@ function getChoiceEnum(request: ElicitRequest): string[] { describe('preview/comparison token elicitation over real Streamable HTTP', () => { let harness: TestHarness | undefined; let client: Client | undefined; + const ORIGINAL_ENABLE_LOCAL_URL_ELICITATION = + process.env.ENABLE_LOCAL_URL_ELICITATION; beforeEach(() => { previewTokenStorage.clearAll(); + // Opt-in (disabled by default) — this harness simulates the stdio entry point, + // the one context where src/index.ts enables this automatically. + process.env.ENABLE_LOCAL_URL_ELICITATION = 'true'; }); afterEach(async () => { @@ -257,6 +262,12 @@ describe('preview/comparison token elicitation over real Streamable HTTP', () => await harness?.close(); client = undefined; harness = undefined; + if (ORIGINAL_ENABLE_LOCAL_URL_ELICITATION === undefined) { + delete process.env.ENABLE_LOCAL_URL_ELICITATION; + } else { + process.env.ENABLE_LOCAL_URL_ELICITATION = + ORIGINAL_ENABLE_LOCAL_URL_ELICITATION; + } }); it('trims the dialog to "provide" and never calls the Tokens API to create a token when the server token is tk.*', async () => { diff --git a/test/security/cross-session-elicitation-hijack.test.ts b/test/security/cross-session-elicitation-hijack.test.ts index 72818c9..11e849c 100644 --- a/test/security/cross-session-elicitation-hijack.test.ts +++ b/test/security/cross-session-elicitation-hijack.test.ts @@ -183,9 +183,14 @@ describe('cross-session elicitation hijack (singleton tool instance reused acros let harness: Harness | undefined; let clientA: Client | undefined; let clientB: Client | undefined; + const ORIGINAL_ENABLE_LOCAL_URL_ELICITATION = + process.env.ENABLE_LOCAL_URL_ELICITATION; beforeEach(() => { previewTokenStorage.clearAll(); + // Opt-in (disabled by default) — this harness simulates the stdio entry point, + // the one context where src/index.ts enables this automatically. + process.env.ENABLE_LOCAL_URL_ELICITATION = 'true'; }); afterEach(async () => { @@ -195,6 +200,12 @@ describe('cross-session elicitation hijack (singleton tool instance reused acros clientA = undefined; clientB = undefined; harness = undefined; + if (ORIGINAL_ENABLE_LOCAL_URL_ELICITATION === undefined) { + delete process.env.ENABLE_LOCAL_URL_ELICITATION; + } else { + process.env.ENABLE_LOCAL_URL_ELICITATION = + ORIGINAL_ENABLE_LOCAL_URL_ELICITATION; + } }); it("does not send session A's elicitation prompt to session B, when B connected more recently and B's installTo() call is the last one to touch the shared tool's this.server", async () => { diff --git a/test/tools/preview-style-tool/PreviewStyleTool.test.ts b/test/tools/preview-style-tool/PreviewStyleTool.test.ts index cb2cff7..0c8b67d 100644 --- a/test/tools/preview-style-tool/PreviewStyleTool.test.ts +++ b/test/tools/preview-style-tool/PreviewStyleTool.test.ts @@ -4,7 +4,7 @@ process.env.MAPBOX_ACCESS_TOKEN = 'sk.eyJhbGciOiJIUzI1NiJ9.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { PreviewStyleTool } from '../../../src/tools/preview-style-tool/PreviewStyleTool.js'; import { cacheKeyFor, @@ -221,8 +221,23 @@ describe('PreviewStyleTool', () => { }); describe('elicitation behavior', () => { + const ORIGINAL_ENABLE_LOCAL_URL_ELICITATION = + process.env.ENABLE_LOCAL_URL_ELICITATION; + beforeEach(() => { previewTokenStorage.clearAll(); + // Opt-in (disabled by default) — these tests simulate the stdio entry point, + // the one context where src/index.ts enables this automatically. + process.env.ENABLE_LOCAL_URL_ELICITATION = 'true'; + }); + + afterEach(() => { + if (ORIGINAL_ENABLE_LOCAL_URL_ELICITATION === undefined) { + delete process.env.ENABLE_LOCAL_URL_ELICITATION; + } else { + process.env.ENABLE_LOCAL_URL_ELICITATION = + ORIGINAL_ENABLE_LOCAL_URL_ELICITATION; + } }); it('returns error when no accessToken and no valid server token', async () => { diff --git a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts index 86102aa..de66f1c 100644 --- a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts +++ b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts @@ -286,8 +286,23 @@ describe('StyleComparisonTool', () => { }); describe('elicitation behavior', () => { + const ORIGINAL_ENABLE_LOCAL_URL_ELICITATION = + process.env.ENABLE_LOCAL_URL_ELICITATION; + beforeEach(() => { previewTokenStorage.clearAll(); + // Opt-in (disabled by default) — these tests simulate the stdio entry point, + // the one context where src/index.ts enables this automatically. + process.env.ENABLE_LOCAL_URL_ELICITATION = 'true'; + }); + + afterEach(() => { + if (ORIGINAL_ENABLE_LOCAL_URL_ELICITATION === undefined) { + delete process.env.ENABLE_LOCAL_URL_ELICITATION; + } else { + process.env.ENABLE_LOCAL_URL_ELICITATION = + ORIGINAL_ENABLE_LOCAL_URL_ELICITATION; + } }); it('returns error when no accessToken and no valid server token', async () => { diff --git a/test/utils/tokenElicitation.test.ts b/test/utils/tokenElicitation.test.ts index 850d5c0..098df12 100644 --- a/test/utils/tokenElicitation.test.ts +++ b/test/utils/tokenElicitation.test.ts @@ -577,6 +577,12 @@ describe('validatePublicPreviewToken', () => { describe('collectProvidedToken', () => { const ORIGINAL_ENV = process.env.ENABLE_LOCAL_URL_ELICITATION; + beforeEach(() => { + // Opt-in by default (disabled-by-default is covered explicitly further down) — + // most tests below exercise the enabled path. + process.env.ENABLE_LOCAL_URL_ELICITATION = 'true'; + }); + afterEach(() => { if (ORIGINAL_ENV === undefined) { delete process.env.ENABLE_LOCAL_URL_ELICITATION; @@ -721,7 +727,7 @@ describe('collectProvidedToken', () => { ).resolves.toBe('pk.good-token'); }); - it('short-circuits to ElicitationUnavailableError without starting collection when disabled via env var', async () => { + it('short-circuits to ElicitationUnavailableError without starting collection when explicitly disabled', async () => { process.env.ENABLE_LOCAL_URL_ELICITATION = 'false'; const { handler } = fakeTokenCollectionHandler('pk.good-token'); @@ -730,4 +736,14 @@ describe('collectProvidedToken', () => { ).rejects.toThrow(ElicitationUnavailableError); expect(handler.collect).not.toHaveBeenCalled(); }); + + it('is disabled by default when the env var is unset (opt-in, not opt-out)', async () => { + delete process.env.ENABLE_LOCAL_URL_ELICITATION; + const { handler } = fakeTokenCollectionHandler('pk.good-token'); + + await expect( + collectProvidedToken(fakeSendRequest(), fakeSendNotification(), handler) + ).rejects.toThrow(ElicitationUnavailableError); + expect(handler.collect).not.toHaveBeenCalled(); + }); });