diff --git a/CHANGELOG.md b/CHANGELOG.md index d63166c..8cdfb92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ ## Unreleased +### New Features + +- **`preview_style_tool` and `style_comparison_tool` no longer require an existing `accessToken`.** Both previously made `accessToken` a required `pk.*` input — meaning a caller had to already have (or separately go create) a public token before either tool would do anything, for even a quick one-off preview. `accessToken` is now optional on both: when omitted, the tool auto-generates a preview token from the server's own access token (the same pattern `geojson_preview_tool`'s resource already used) via a new shared `mintScopedPreviewToken` utility, so a first call needs no setup at all. + - Pass `share: true` alongside your own `accessToken` on either tool to get a link built from a token you manage yourself, for when you actually want something durable to bookmark or hand to someone else — `share: true` without an `accessToken` is a clear validation error, not a silent fallback. + - The two tools' auto-mint tokens differ in shape, confirmed live against the real API: `preview_style_tool` mints a short-lived (~1h) `tk.*` token, since the Styles API's embeddable HTML preview page and GL JS both accept it fine. `style_comparison_tool` mints a non-expiring `pk.*` token instead — `agent.mapbox.com/tools/style-compare`, which its comparison link embeds, validates the token prefix itself and hard-rejects anything but `pk.*` (a `tk.*` token 400s there with "Configuration Error: Invalid token type"). The `pk.*` token is still narrowly scoped (`styles:tiles`/`styles:read`/`fonts:read`) and auto-generated, just not self-expiring; it persists on the account until manually revoked. + - Both tools now take `httpRequest` as a constructor dependency (matching every other network-calling tool in this repo) to make the mint call. + - **Hosted-endpoint-aware error messages.** Per `README.md`, the hosted MCP endpoint authenticates with its own access token rather than a personal Mapbox account token, and that token isn't granted `tokens:write` — so auto-mint can't work there. Without this, a hosted caller who omits `accessToken` would have hit a raw, misleading `jwtUtils` error (`"MAPBOX_ACCESS_TOKEN is not in valid JWT format"`, referencing an env var the hosted deployment doesn't even use) or a bare `Token API 403`. A new `describeAutoMintFailure` helper rewrites both into an actionable message pointing at `accessToken`/`list_tokens_tool`/`create_token_tool` instead. + ### Breaking Changes - **Consolidated `GeojsonPreviewUIResource` and `PreviewStyleUIResource` into a single `MapPreviewUIResource`.** Both were near-identical hand-written MCP Apps templates — the same postMessage handshake, fullscreen/open-link controls, and resize handling copy-pasted across ~650 lines, differing only in what they drew on the map once a tool result arrived (a GeoJSON overlay on the default Standard style vs. swapping to an arbitrary preview style). `geojson_preview_tool` and `preview_style_tool` now both declare `_meta.ui.resourceUri: 'ui://mapbox/map-preview/index.html'`, served by the merged resource, which dispatches on the shape of the tool-result URL it receives (a `geojson.io` URL vs. a Styles API `.html?access_token=...` preview URL) rather than assuming a fixed mode. Neither tool's own input/output contract changed. diff --git a/src/resources/ui-apps/MapPreviewUIResource.ts b/src/resources/ui-apps/MapPreviewUIResource.ts index 4eefe0d..220ce81 100644 --- a/src/resources/ui-apps/MapPreviewUIResource.ts +++ b/src/resources/ui-apps/MapPreviewUIResource.ts @@ -9,46 +9,10 @@ import type { } from '@modelcontextprotocol/sdk/types.js'; import { RESOURCE_MIME_TYPE } from '@modelcontextprotocol/ext-apps/server'; import { BaseResource } from '../BaseResource.js'; -import { - getUserNameFromToken, - mapboxApiEndpoint -} from '../../utils/jwtUtils.js'; +import { mintScopedPreviewToken } from '../../utils/mintScopedPreviewToken.js'; const MAPBOX_GL_VERSION = '3.12.0'; -// GL JS needs a public token; mint a short-lived one per request from the -// caller's sk.*. Do NOT cache it in module scope — on a multi-tenant server a -// process-global cache can return one caller's token to a different caller. -// -// Only used to bootstrap the map before any tool result has arrived (the -// GeoJSON-preview path draws on top of the default Standard style using this -// token). The style-preview path never needs it: its tool result URL already -// carries a token scoped to the style being previewed, which takes over via -// mapboxgl.accessToken once that result arrives — see handleToolResult below. -async function createPreviewToken(skToken: string): Promise { - const username = getUserNameFromToken(skToken); - const expires = new Date(Date.now() + 60 * 60 * 1000).toISOString(); // 1 hour - const url = `${mapboxApiEndpoint()}tokens/v2/${username}?access_token=${skToken}`; - - const response = await fetch(url, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - note: 'Map Preview (auto-generated, expires in 1h)', - scopes: ['styles:tiles', 'styles:read', 'fonts:read'], - expires - }) - }); - - if (!response.ok) { - // Do not include the response body — it may echo the token back. - throw new Error(`Token API ${response.status}`); - } - - const data = (await response.json()) as { token: string }; - return data.token; -} - /** * Serves the UI App HTML shared by `geojson_preview_tool` and * `preview_style_tool` — one Mapbox GL JS map that either draws a GeoJSON @@ -87,12 +51,10 @@ export class MapPreviewUIResource extends BaseResource { let accessToken = ''; if (skToken.startsWith('sk.')) { try { - const minted = await createPreviewToken(skToken); - // Defense in depth: only embed a token minted for the caller's own - // account, so a token can never be served to a different caller. - if (getUserNameFromToken(minted) === getUserNameFromToken(skToken)) { - accessToken = minted; - } + accessToken = await mintScopedPreviewToken(fetch, skToken, { + note: 'Map Preview (auto-generated, expires in 1h)', + scopes: ['styles:tiles', 'styles:read', 'fonts:read'] + }); } catch { // Non-fatal — map won't render until a style-preview result // supplies its own token, but the link button still works. diff --git a/src/tools/index.ts b/src/tools/index.ts index ca134f3..91a537e 100644 --- a/src/tools/index.ts +++ b/src/tools/index.ts @@ -124,7 +124,7 @@ export const listTokens = new ListTokensTool({ httpRequest }); export const optimizeStyle = new OptimizeStyleTool(); /** Preview a Mapbox style */ -export const previewStyle = new PreviewStyleTool(); +export const previewStyle = new PreviewStyleTool({ httpRequest }); /** Retrieve a Mapbox style */ export const retrieveStyle = new RetrieveStyleTool({ httpRequest }); @@ -133,7 +133,7 @@ export const retrieveStyle = new RetrieveStyleTool({ httpRequest }); export const styleBuilder = new StyleBuilderTool(); /** Compare styles side-by-side */ -export const styleComparison = new StyleComparisonTool(); +export const styleComparison = new StyleComparisonTool({ httpRequest }); /** Query tiles at a location */ export const tilequery = new TilequeryTool({ httpRequest }); diff --git a/src/tools/preview-style-tool/PreviewStyleTool.input.schema.ts b/src/tools/preview-style-tool/PreviewStyleTool.input.schema.ts index fb9111b..78dbb6d 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.input.schema.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.input.schema.ts @@ -9,8 +9,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( + 'Existing Mapbox public token (must start with pk.* and have styles:read permission). Required when share is true, to build a durable link with a token you control. Optional otherwise — if omitted, a short-lived (~1 hour) preview token is generated automatically, so no existing token is needed for a quick inline look.' + ), + share: 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.' + 'Set to true to build a durable, shareable preview link using accessToken (required together with share). Defaults to false: generates a short-lived preview token automatically for viewing the style right now — the returned URL expires in about an hour and is not meant to be bookmarked or shared with others.' ), title: z .boolean() diff --git a/src/tools/preview-style-tool/PreviewStyleTool.ts b/src/tools/preview-style-tool/PreviewStyleTool.ts index 60d0b84..de7544c 100644 --- a/src/tools/preview-style-tool/PreviewStyleTool.ts +++ b/src/tools/preview-style-tool/PreviewStyleTool.ts @@ -8,11 +8,16 @@ import { PreviewStyleInput } from './PreviewStyleTool.input.schema.js'; import { getUserNameFromToken } from '../../utils/jwtUtils.js'; +import { + mintScopedPreviewToken, + describeAutoMintFailure +} from '../../utils/mintScopedPreviewToken.js'; +import type { HttpRequest } from '../../utils/types.js'; export class PreviewStyleTool extends BaseTool { readonly name = 'preview_style_tool'; readonly description = - 'Generate preview URL for a Mapbox style using an existing public token'; + 'Generate a live preview of a Mapbox style. By default, auto-generates a short-lived preview token so you can view it right away — no existing token needed. Pass `share: true` with an existing public `accessToken` instead to generate a durable, shareable preview link.'; readonly annotations = { readOnlyHint: true, destructiveHint: false, @@ -32,29 +37,79 @@ export class PreviewStyleTool extends BaseTool { } }; - constructor() { + private readonly httpRequest: HttpRequest; + + constructor(params: { httpRequest: HttpRequest }) { super({ inputSchema: PreviewStyleSchema }); + this.httpRequest = params.httpRequest; } - protected async execute(input: PreviewStyleInput): Promise { + protected async execute( + input: PreviewStyleInput, + accessToken?: string + ): Promise { let userName: string; - try { - userName = getUserNameFromToken(input.accessToken); - } catch (error) { + let publicToken: string; + + if (input.accessToken) { + // Caller-supplied token — used as-is, for either mode. + try { + userName = getUserNameFromToken(input.accessToken); + } catch (error) { + return { + isError: true, + content: [ + { + type: 'text', + text: error instanceof Error ? error.message : String(error) + } + ] + }; + } + publicToken = input.accessToken; + } else if (input.share) { return { isError: true, content: [ { type: 'text', - text: error instanceof Error ? error.message : String(error) + text: + '`share: true` requires an existing public token via `accessToken` — a persistent ' + + 'pk.* token is needed for a durable, shareable link. Get one via list_tokens_tool ' + + 'or create_token_tool, or omit `share` for a quick inline preview (no token needed).' } ] }; + } else { + if (!accessToken) { + return { + isError: true, + content: [ + { + type: 'text', + text: 'No Mapbox access token available to generate a preview.' + } + ] + }; + } + try { + userName = getUserNameFromToken(accessToken); + publicToken = await mintScopedPreviewToken( + this.httpRequest, + accessToken, + { + note: 'Style Preview (auto-generated, expires in 1h)', + scopes: ['styles:tiles', 'styles:read', 'fonts:read'] + } + ); + } catch (error) { + return { + isError: true, + content: [{ type: 'text', text: describeAutoMintFailure(error) }] + }; + } } - // 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); diff --git a/src/tools/style-comparison-tool/StyleComparisonTool.schema.ts b/src/tools/style-comparison-tool/StyleComparisonTool.schema.ts index 1265c11..f23d215 100644 --- a/src/tools/style-comparison-tool/StyleComparisonTool.schema.ts +++ b/src/tools/style-comparison-tool/StyleComparisonTool.schema.ts @@ -28,8 +28,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( + 'Existing Mapbox public token (must start with pk.* and have styles:read permission). Required when share is true, to build the comparison link with a token you control. Optional otherwise — if omitted, a scoped preview token is generated automatically, so no existing token is needed for a quick inline comparison.' + ), + share: 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.' + 'Set to true to build the comparison link with an existing public token you provide via accessToken (required together with share) — use this for a link you intend to keep or share with someone else. Defaults to false: generates a scoped preview token automatically, tied to this comparison only and not tracked or named by you, so no existing token is needed for a quick look right now.' ), zoom: z .number() diff --git a/src/tools/style-comparison-tool/StyleComparisonTool.ts b/src/tools/style-comparison-tool/StyleComparisonTool.ts index e916b5a..f925957 100644 --- a/src/tools/style-comparison-tool/StyleComparisonTool.ts +++ b/src/tools/style-comparison-tool/StyleComparisonTool.ts @@ -10,13 +10,18 @@ import { StyleComparisonInput } from './StyleComparisonTool.schema.js'; import { getUserNameFromToken } from '../../utils/jwtUtils.js'; +import { + mintScopedPreviewToken, + describeAutoMintFailure +} from '../../utils/mintScopedPreviewToken.js'; +import type { HttpRequest } from '../../utils/types.js'; export class StyleComparisonTool extends BaseTool< typeof StyleComparisonSchema > { readonly name = 'style_comparison_tool'; readonly description = - 'Generate a comparison URL for comparing two Mapbox styles side-by-side'; + 'Generate a live side-by-side comparison of two Mapbox styles. By default, auto-generates a scoped preview token so you can compare them right away — no existing token needed. Pass `share: true` with an existing public `accessToken` instead to generate a comparison link using a token you manage yourself.'; readonly annotations = { readOnlyHint: true, destructiveHint: false, @@ -36,8 +41,11 @@ export class StyleComparisonTool extends BaseTool< } }; - constructor() { + private readonly httpRequest: HttpRequest; + + constructor(params: { httpRequest: HttpRequest }) { super({ inputSchema: StyleComparisonSchema }); + this.httpRequest = params.httpRequest; } /** @@ -86,14 +94,70 @@ export class StyleComparisonTool extends BaseTool< } protected async execute( - input: StyleComparisonInput + input: StyleComparisonInput, + accessToken?: string ): Promise { + let publicToken: string; + + if (input.accessToken) { + // Caller-supplied token — used as-is, for either mode. + publicToken = input.accessToken; + } else if (input.share) { + return { + isError: true, + content: [ + { + type: 'text', + text: + '`share: true` requires an existing public token via `accessToken` — a persistent ' + + 'pk.* token is needed for a durable, shareable link. Get one via list_tokens_tool ' + + 'or create_token_tool, or omit `share` for a quick inline comparison (no token needed).' + } + ] + }; + } else { + if (!accessToken) { + return { + isError: true, + content: [ + { + type: 'text', + text: 'No Mapbox access token available to generate a comparison.' + } + ] + }; + } + try { + // Unlike the inline style preview, the comparison page this URL + // points to (agent.mapbox.com/tools/style-compare) validates the + // token prefix itself and hard-rejects anything but pk.* — a + // short-lived tk.* token (this repo's usual auto-mint default) + // fails there with a "Configuration Error", confirmed live. So + // this mints a real, non-expiring pk.* token instead: narrowly + // scoped, but it persists on the account until manually revoked. + publicToken = await mintScopedPreviewToken( + this.httpRequest, + accessToken, + { + note: 'Style Comparison (auto-generated)', + scopes: ['styles:tiles', 'styles:read', 'fonts:read'], + expiresInMs: null + } + ); + } catch (error) { + return { + isError: true, + content: [{ type: 'text', text: describeAutoMintFailure(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: [ @@ -111,7 +175,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/src/tools/toolRegistry.ts b/src/tools/toolRegistry.ts index 5e9237d..64f181f 100644 --- a/src/tools/toolRegistry.ts +++ b/src/tools/toolRegistry.ts @@ -36,13 +36,13 @@ export const CORE_TOOLS = [ new RetrieveStyleTool({ httpRequest }), new UpdateStyleTool({ httpRequest }), new DeleteStyleTool({ httpRequest }), - new PreviewStyleTool(), + new PreviewStyleTool({ httpRequest }), new StyleBuilderTool(), new GeojsonPreviewTool(), new CheckColorContrastTool(), new CompareStylesTool(), new OptimizeStyleTool(), - new StyleComparisonTool(), + new StyleComparisonTool({ httpRequest }), new CreateTokenTool({ httpRequest }), new ListTokensTool({ httpRequest }), new BoundingBoxTool(), diff --git a/src/utils/mintScopedPreviewToken.ts b/src/utils/mintScopedPreviewToken.ts new file mode 100644 index 0000000..fafeccd --- /dev/null +++ b/src/utils/mintScopedPreviewToken.ts @@ -0,0 +1,123 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import type { HttpRequest } from './types.js'; +import { getUserNameFromToken, mapboxApiEndpoint } from './jwtUtils.js'; + +/** + * Mints a public token scoped to `scopes`, on the same account as + * `serverAccessToken`. Used to auto-generate a preview token when a caller + * hasn't supplied — and doesn't need to manage — a token of their own, e.g. + * for a one-off inline style preview or comparison. + * + * By default (`expiresInMs` omitted or a number), the Tokens API mints a + * short-lived **`tk.*`** temporary token — confirmed live against the real + * API — which is what GL JS and the Styles API's embeddable HTML preview + * page both accept. Pass `expiresInMs: null` to mint a genuine, non-expiring + * **`pk.*`** public token instead — required for any consumer that validates + * the token prefix itself and rejects non-`pk.*` tokens outright (confirmed + * live: `agent.mapbox.com/tools/style-compare`, which `style_comparison_tool` + * embeds, does exactly this and 400s on a `tk.*` token). A `pk.*` token + * minted this way does not self-expire; it persists on the account (scoped + * narrowly to `scopes`) until manually revoked. + * + * Verifies the minted token belongs to the caller's own account before + * returning it — defense in depth against a misbehaving/misconfigured + * backend returning a different account's token (see the AGI-905 regression + * suite this same check protects against in `MapPreviewUIResource`). + * + * Do NOT cache the result across calls: on a multi-tenant server, caching + * by anything less than the caller's own identity risks handing one + * caller's token to another. + */ +export async function mintScopedPreviewToken( + httpRequest: HttpRequest, + serverAccessToken: string, + params: { note: string; scopes: string[]; expiresInMs?: number | null } +): Promise { + const expectedUsername = getUserNameFromToken(serverAccessToken); + const expires = + params.expiresInMs === null + ? undefined + : new Date( + Date.now() + (params.expiresInMs ?? 60 * 60 * 1000) + ).toISOString(); + + const url = `${mapboxApiEndpoint()}tokens/v2/${expectedUsername}?access_token=${serverAccessToken}`; + const response = await httpRequest(url, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + note: params.note, + scopes: params.scopes, + ...(expires ? { expires } : {}) + }) + }); + + if (!response.ok) { + // Do not include the response body — it may echo the token back. + throw new Error(`Token API ${response.status}`); + } + + const data = (await response.json()) as { token: string }; + if (getUserNameFromToken(data.token) !== expectedUsername) { + throw new Error('Minted token does not match caller account'); + } + return data.token; +} + +/** + * Turns a failure from the auto-mint path (either `getUserNameFromToken` + * on the server's own token, or `mintScopedPreviewToken` itself) into a + * message that actually tells the caller what to do next, instead of + * surfacing jwtUtils' raw, `MAPBOX_ACCESS_TOKEN`-flavored error text or a + * bare `Token API 403`. + * + * Two known failure shapes, both confirmed against this repo's own code + * and docs rather than assumed: + * + * 1. The server's own access token isn't a personal Mapbox account token + * at all, so a username can't even be resolved from it before a mint + * attempt — this is the hosted MCP endpoint's normal case. Per + * README.md: "the hosted deployment authenticates each request with + * its own access token rather than your personal Mapbox account + * token" and doesn't expose `create_token_tool` at all. There's no + * reliable way to detect this shape ahead of time (it isn't a Mapbox + * `pk./sk./tk.` token, so it never reaches the mint call), so this is + * keyed off `getUserNameFromToken`'s own error text. + * 2. The server's own token IS a Mapbox token but lacks `tokens:write` — + * the mint call itself 401s/403s. + * + * Anything else (e.g. a transient network/5xx failure) is passed through + * unchanged rather than guessed at. + */ +export function describeAutoMintFailure(error: unknown): string { + const message = error instanceof Error ? error.message : String(error); + const actionableSuffix = + 'Pass accessToken directly instead: get an existing public token via ' + + 'list_tokens_tool, or create one with create_token_tool if it is ' + + 'available in this deployment.'; + + const cannotResolveAccount = + message.includes('is not in valid JWT format') || + message.includes('does not contain username in payload') || + message.includes('No access token provided'); + if (cannotResolveAccount) { + return ( + "Could not auto-generate a preview token: this server's own access " + + "token isn't a personal Mapbox account token (expected on " + + 'deployments — like the hosted MCP endpoint — that authenticate a ' + + `different way). ${actionableSuffix}` + ); + } + + if (/^Token API 40[13]$/.test(message)) { + return ( + "Could not auto-generate a preview token: this server's access " + + 'token does not have permission to create new tokens (tokens:write). ' + + actionableSuffix + ); + } + + return message; +} diff --git a/test/security/path-traversal.test.ts b/test/security/path-traversal.test.ts index f7b27bd..e26bca8 100644 --- a/test/security/path-traversal.test.ts +++ b/test/security/path-traversal.test.ts @@ -200,7 +200,8 @@ describe('path traversal security', () => { it('PreviewStyleTool encodes username containing "/" in preview URL and resource URI', async () => { const maliciousPublicToken = makePublicToken('user/attacker'); - const result = await new PreviewStyleTool().run({ + const { httpRequest } = setupHttpRequest(); + const result = await new PreviewStyleTool({ httpRequest }).run({ styleId: VALID_STYLE_ID, accessToken: maliciousPublicToken }); diff --git a/test/tools/__snapshots__/tool-naming-convention.test.ts.snap b/test/tools/__snapshots__/tool-naming-convention.test.ts.snap index a7277a3..fe074f7 100644 --- a/test/tools/__snapshots__/tool-naming-convention.test.ts.snap +++ b/test/tools/__snapshots__/tool-naming-convention.test.ts.snap @@ -74,7 +74,7 @@ exports[`Tool Naming Convention > should maintain consistent tool list (snapshot }, { "className": "PreviewStyleTool", - "description": "Generate preview URL for a Mapbox style using an existing public token", + "description": "Generate a live preview of a Mapbox style. By default, auto-generates a short-lived preview token so you can view it right away — no existing token needed. Pass \`share: true\` with an existing public \`accessToken\` instead to generate a durable, shareable preview link.", "toolName": "preview_style_tool", }, { @@ -149,7 +149,7 @@ If a layer type is not recognized, the tool will provide helpful suggestions sho }, { "className": "StyleComparisonTool", - "description": "Generate a comparison URL for comparing two Mapbox styles side-by-side", + "description": "Generate a live side-by-side comparison of two Mapbox styles. By default, auto-generates a scoped preview token so you can compare them right away — no existing token needed. Pass \`share: true\` with an existing public \`accessToken\` instead to generate a comparison link using a token you manage yourself.", "toolName": "style_comparison_tool", }, { diff --git a/test/tools/preview-style-tool/PreviewStyleTool.test.ts b/test/tools/preview-style-tool/PreviewStyleTool.test.ts index 93ee55e..5bd2568 100644 --- a/test/tools/preview-style-tool/PreviewStyleTool.test.ts +++ b/test/tools/preview-style-tool/PreviewStyleTool.test.ts @@ -2,21 +2,29 @@ // Licensed under the MIT License. process.env.MAPBOX_ACCESS_TOKEN = - 'sk.eyJhbGciOiJIUzI1NiJ9.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; + 'sk.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { PreviewStyleTool } from '../../../src/tools/preview-style-tool/PreviewStyleTool.js'; +// None of the existing tests below hit the network (they all supply their +// own accessToken), but the tool now takes httpRequest as a constructor +// dependency regardless, matching every other network-calling tool in this +// repo — see toolRegistry.ts. +function newTool(httpRequest = vi.fn()) { + return new PreviewStyleTool({ httpRequest }); +} + describe('PreviewStyleTool', () => { const TEST_ACCESS_TOKEN = 'pk.eyJ1IjoidGVzdC11c2VyIiwiYSI6InRlc3QtYXBpIn0.signature'; describe('tool metadata', () => { it('should have correct name and description', () => { - const tool = new PreviewStyleTool(); + const tool = newTool(); expect(tool.name).toBe('preview_style_tool'); - expect(tool.description).toBe( - 'Generate preview URL for a Mapbox style using an existing public token' + expect(tool.description).toContain( + 'auto-generates a short-lived preview token' ); }); @@ -28,7 +36,7 @@ describe('PreviewStyleTool', () => { }); it('uses user-provided public token and returns preview URL', async () => { - const result = await new PreviewStyleTool().run({ + const result = await newTool().run({ styleId: 'cmojrmkc9002t01ry96yi6h48', accessToken: TEST_ACCESS_TOKEN, title: false, @@ -45,7 +53,7 @@ describe('PreviewStyleTool', () => { }); it('includes styleId in URL', async () => { - const result = await new PreviewStyleTool().run({ + const result = await newTool().run({ styleId: 'cmojrmkc9002t01ry96yi6h49', accessToken: TEST_ACCESS_TOKEN, title: false, @@ -61,7 +69,7 @@ describe('PreviewStyleTool', () => { }); it('includes title parameter when provided', async () => { - const result = await new PreviewStyleTool().run({ + const result = await newTool().run({ styleId: 'cmojrmkc9002t01ry96yi6h48', accessToken: TEST_ACCESS_TOKEN, title: true, @@ -75,7 +83,7 @@ describe('PreviewStyleTool', () => { }); it('includes zoomwheel parameter when provided', async () => { - const result = await new PreviewStyleTool().run({ + const result = await newTool().run({ styleId: 'cmojrmkc9002t01ry96yi6h48', accessToken: TEST_ACCESS_TOKEN, zoomwheel: false, @@ -89,7 +97,7 @@ describe('PreviewStyleTool', () => { }); it('includes fresh parameter for secure access', async () => { - const result = await new PreviewStyleTool().run({ + const result = await newTool().run({ styleId: 'cmojrmkc9002t01ry96yi6h48', accessToken: TEST_ACCESS_TOKEN, title: false, @@ -103,7 +111,7 @@ describe('PreviewStyleTool', () => { }); it('rejects secret tokens', async () => { - const result = await new PreviewStyleTool().run({ + const result = await newTool().run({ styleId: 'cmojrmkc9002t01ry96yi6h48', accessToken: 'sk.eyJhbGciOiJIUzI1NiJ9.eyJ1IjoidGVzdC11c2VyIn0.secret_token', @@ -121,7 +129,7 @@ describe('PreviewStyleTool', () => { }); it('rejects temporary tokens', async () => { - const result = await new PreviewStyleTool().run({ + const result = await newTool().run({ styleId: 'cmojrmkc9002t01ry96yi6h48', accessToken: 'tk.eyJhbGciOiJIUzI1NiJ9.eyJ1IjoidGVzdC11c2VyIn0.temp_token', title: false, @@ -138,7 +146,7 @@ describe('PreviewStyleTool', () => { }); it('returns URL and MCP-UI resource on success (default)', async () => { - const result = await new PreviewStyleTool().run({ + const result = await newTool().run({ styleId: 'cmojrmkc9002t01ry96yi6h48', accessToken: TEST_ACCESS_TOKEN, title: false, @@ -174,7 +182,7 @@ describe('PreviewStyleTool', () => { }); it('returns URL and MCP-UI resource for backward compatibility', async () => { - const result = await new PreviewStyleTool().run({ + const result = await newTool().run({ styleId: 'cmojrmkc9002t01ry96yi6h48', accessToken: TEST_ACCESS_TOKEN, title: false, @@ -195,4 +203,136 @@ describe('PreviewStyleTool', () => { type: 'resource' }); }); + + describe('auto-minted inline preview (no accessToken, no share)', () => { + function stubMintingFetch() { + return vi.fn( + async (_input: string | URL | Request, _init?: RequestInit) => + new Response( + JSON.stringify({ + token: 'pk.eyJ1IjoidGVzdC11c2VyIn0.minted' + }), + { status: 200 } + ) + ); + } + + it('auto-generates a short-lived token from the server access token and needs no accessToken input', async () => { + const httpRequest = stubMintingFetch(); + const result = await new PreviewStyleTool({ httpRequest }).run({ + styleId: 'cmojrmkc9002t01ry96yi6h48', + title: false, + zoomwheel: false + }); + + expect(result.isError).toBe(false); + expect(httpRequest).toHaveBeenCalledTimes(1); + const [url, init] = httpRequest.mock.calls[0]; + expect(String(url)).toContain('tokens/v2/test-user'); + expect(JSON.parse((init as RequestInit).body as string)).toMatchObject({ + scopes: ['styles:tiles', 'styles:read', 'fonts:read'] + }); + expect(result.content[0]).toMatchObject({ + type: 'text', + text: expect.stringContaining( + '/styles/v1/test-user/cmojrmkc9002t01ry96yi6h48.html?access_token=pk.' + ) + }); + }); + + it('errors clearly when share is true but accessToken is missing', async () => { + const httpRequest = stubMintingFetch(); + const result = await new PreviewStyleTool({ httpRequest }).run({ + styleId: 'cmojrmkc9002t01ry96yi6h48', + share: true, + title: false, + zoomwheel: false + }); + + expect(result.isError).toBe(true); + expect(httpRequest).not.toHaveBeenCalled(); + expect(result.content[0]).toMatchObject({ + type: 'text', + text: expect.stringContaining('share: true` requires') + }); + }); + + it('an explicit accessToken is honored even when share is true', async () => { + const httpRequest = stubMintingFetch(); + const result = await new PreviewStyleTool({ httpRequest }).run({ + styleId: 'cmojrmkc9002t01ry96yi6h48', + share: true, + accessToken: TEST_ACCESS_TOKEN, + title: false, + zoomwheel: false + }); + + expect(result.isError).toBe(false); + expect(httpRequest).not.toHaveBeenCalled(); + expect(result.content[0]).toMatchObject({ + type: 'text', + text: expect.stringContaining(`access_token=${TEST_ACCESS_TOKEN}`) + }); + }); + + it('errors when no server access token is available to mint from', async () => { + const saved = process.env.MAPBOX_ACCESS_TOKEN; + delete process.env.MAPBOX_ACCESS_TOKEN; + try { + const httpRequest = stubMintingFetch(); + const result = await new PreviewStyleTool({ httpRequest }).run({ + styleId: 'cmojrmkc9002t01ry96yi6h48', + title: false, + zoomwheel: false + }); + + expect(result.isError).toBe(true); + expect(httpRequest).not.toHaveBeenCalled(); + } finally { + if (saved !== undefined) process.env.MAPBOX_ACCESS_TOKEN = saved; + } + }); + + it('surfaces an actionable error, not the raw Token API status, when minting fails (e.g. missing tokens:write)', async () => { + const httpRequest = vi.fn( + async () => new Response('forbidden', { status: 403 }) + ); + const result = await new PreviewStyleTool({ httpRequest }).run({ + styleId: 'cmojrmkc9002t01ry96yi6h48', + title: false, + zoomwheel: false + }); + + expect(result.isError).toBe(true); + expect(result.content[0]).toMatchObject({ + type: 'text', + text: expect.stringContaining('tokens:write') + }); + expect( + (result.content[0] as { type: 'text'; text: string }).text + ).toContain('list_tokens_tool'); + }); + + it('surfaces a hosted-endpoint-aware error, not a raw jwtUtils message, when the server token is not a Mapbox token at all', async () => { + const saved = process.env.MAPBOX_ACCESS_TOKEN; + process.env.MAPBOX_ACCESS_TOKEN = 'not-a-mapbox-token'; + try { + const httpRequest = stubMintingFetch(); + const result = await new PreviewStyleTool({ httpRequest }).run({ + styleId: 'cmojrmkc9002t01ry96yi6h48', + title: false, + zoomwheel: false + }); + + expect(result.isError).toBe(true); + expect(httpRequest).not.toHaveBeenCalled(); + const text = (result.content[0] as { type: 'text'; text: string }).text; + expect(text).toContain('list_tokens_tool'); + expect(text).not.toContain('MAPBOX_ACCESS_TOKEN'); + } finally { + if (saved !== undefined) process.env.MAPBOX_ACCESS_TOKEN = saved; + else delete process.env.MAPBOX_ACCESS_TOKEN; + } + }); + }); }); diff --git a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts index 5024af7..6bd01a8 100644 --- a/test/tools/style-comparison-tool/StyleComparisonTool.test.ts +++ b/test/tools/style-comparison-tool/StyleComparisonTool.test.ts @@ -9,7 +9,10 @@ describe('StyleComparisonTool', () => { let tool: StyleComparisonTool; beforeEach(() => { - tool = new StyleComparisonTool(); + // Unused by tests below that supply their own accessToken; the tool + // takes httpRequest as a constructor dependency regardless, matching + // every other network-calling tool in this repo — see toolRegistry.ts. + tool = new StyleComparisonTool({ httpRequest: vi.fn() }); }); afterEach(() => { @@ -50,19 +53,20 @@ describe('StyleComparisonTool', () => { }); }); - it('should require access token', async () => { + it('requires an access token when share is true', async () => { const input = { before: 'mapbox/streets-v12', - after: 'mapbox/satellite-v9' + after: 'mapbox/satellite-v9', + share: true // Missing accessToken - } as any; + }; const result = await tool.run(input); expect(result.isError).toBe(true); expect( (result.content[0] as { type: 'text'; text: string }).text - ).toContain('invalid_type'); + ).toContain('share: true` requires'); }); it('should handle full style URLs', async () => { @@ -262,11 +266,145 @@ describe('StyleComparisonTool', () => { }); }); + describe('auto-minted inline comparison (no accessToken, no share)', () => { + const SERVER_TOKEN = 'sk.eyJ1IjoidGVzdC11c2VyIn0.signature'; + + function stubMintingFetch() { + return vi.fn( + async (_input: string | URL | Request, _init?: RequestInit) => + new Response( + JSON.stringify({ token: 'pk.eyJ1IjoidGVzdC11c2VyIn0.minted' }), + { status: 200 } + ) + ); + } + + it('auto-generates a token from the server access token and needs no accessToken input', async () => { + const saved = process.env.MAPBOX_ACCESS_TOKEN; + process.env.MAPBOX_ACCESS_TOKEN = SERVER_TOKEN; + try { + const httpRequest = stubMintingFetch(); + const result = await new StyleComparisonTool({ httpRequest }).run({ + before: 'mapbox/streets-v12', + after: 'mapbox/outdoors-v12' + }); + + expect(result.isError).toBe(false); + expect(httpRequest).toHaveBeenCalledTimes(1); + const [url] = httpRequest.mock.calls[0]; + expect(String(url)).toContain('tokens/v2/test-user'); + const resultUrl = (result.content[0] as { type: 'text'; text: string }) + .text; + expect(resultUrl).toContain( + 'access_token=pk.eyJ1IjoidGVzdC11c2VyIn0.minted' + ); + } finally { + if (saved !== undefined) process.env.MAPBOX_ACCESS_TOKEN = saved; + else delete process.env.MAPBOX_ACCESS_TOKEN; + } + }); + + it('mints a non-expiring token, not the short-lived tk.* the other preview tools use — agent.mapbox.com/tools/style-compare rejects tk.* outright (confirmed live)', async () => { + const saved = process.env.MAPBOX_ACCESS_TOKEN; + process.env.MAPBOX_ACCESS_TOKEN = SERVER_TOKEN; + try { + const httpRequest = stubMintingFetch(); + await new StyleComparisonTool({ httpRequest }).run({ + before: 'mapbox/streets-v12', + after: 'mapbox/outdoors-v12' + }); + + const [, init] = httpRequest.mock.calls[0]; + const body = JSON.parse((init as RequestInit).body as string); + expect(body).not.toHaveProperty('expires'); + } finally { + if (saved !== undefined) process.env.MAPBOX_ACCESS_TOKEN = saved; + else delete process.env.MAPBOX_ACCESS_TOKEN; + } + }); + + it('an explicit accessToken is honored even when share is true', async () => { + const httpRequest = stubMintingFetch(); + const result = await new StyleComparisonTool({ httpRequest }).run({ + before: 'mapbox/streets-v12', + after: 'mapbox/outdoors-v12', + share: true, + accessToken: 'pk.test.token' + }); + + expect(result.isError).toBe(false); + expect(httpRequest).not.toHaveBeenCalled(); + const resultUrl = (result.content[0] as { type: 'text'; text: string }) + .text; + expect(resultUrl).toContain('access_token=pk.test.token'); + }); + + it('errors when no server access token is available to mint from', async () => { + const saved = process.env.MAPBOX_ACCESS_TOKEN; + delete process.env.MAPBOX_ACCESS_TOKEN; + try { + const httpRequest = stubMintingFetch(); + const result = await new StyleComparisonTool({ httpRequest }).run({ + before: 'mapbox/streets-v12', + after: 'mapbox/outdoors-v12' + }); + + expect(result.isError).toBe(true); + expect(httpRequest).not.toHaveBeenCalled(); + } finally { + if (saved !== undefined) process.env.MAPBOX_ACCESS_TOKEN = saved; + } + }); + + it('surfaces a hosted-endpoint-aware error, not a raw jwtUtils message, when the server token is not a Mapbox token at all', async () => { + const saved = process.env.MAPBOX_ACCESS_TOKEN; + process.env.MAPBOX_ACCESS_TOKEN = 'not-a-mapbox-token'; + try { + const httpRequest = stubMintingFetch(); + const result = await new StyleComparisonTool({ httpRequest }).run({ + before: 'mapbox/streets-v12', + after: 'mapbox/outdoors-v12' + }); + + expect(result.isError).toBe(true); + expect(httpRequest).not.toHaveBeenCalled(); + const text = (result.content[0] as { type: 'text'; text: string }).text; + expect(text).toContain('list_tokens_tool'); + expect(text).not.toContain('MAPBOX_ACCESS_TOKEN'); + } finally { + if (saved !== undefined) process.env.MAPBOX_ACCESS_TOKEN = saved; + else delete process.env.MAPBOX_ACCESS_TOKEN; + } + }); + + it('surfaces an actionable error, not the raw Token API status, when minting fails (e.g. missing tokens:write)', async () => { + const saved = process.env.MAPBOX_ACCESS_TOKEN; + process.env.MAPBOX_ACCESS_TOKEN = SERVER_TOKEN; + try { + const httpRequest = vi.fn( + async () => new Response('forbidden', { status: 403 }) + ); + const result = await new StyleComparisonTool({ httpRequest }).run({ + before: 'mapbox/streets-v12', + after: 'mapbox/outdoors-v12' + }); + + expect(result.isError).toBe(true); + const text = (result.content[0] as { type: 'text'; text: string }).text; + expect(text).toContain('tokens:write'); + expect(text).toContain('list_tokens_tool'); + } finally { + if (saved !== undefined) process.env.MAPBOX_ACCESS_TOKEN = saved; + else delete process.env.MAPBOX_ACCESS_TOKEN; + } + }); + }); + describe('metadata', () => { it('should have correct name and description', () => { expect(tool.name).toBe('style_comparison_tool'); - expect(tool.description).toBe( - 'Generate a comparison URL for comparing two Mapbox styles side-by-side' + expect(tool.description).toContain( + 'auto-generates a scoped preview token' ); }); }); diff --git a/test/utils/mintScopedPreviewToken.test.ts b/test/utils/mintScopedPreviewToken.test.ts new file mode 100644 index 0000000..698572a --- /dev/null +++ b/test/utils/mintScopedPreviewToken.test.ts @@ -0,0 +1,184 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect, vi } from 'vitest'; +import { + mintScopedPreviewToken, + describeAutoMintFailure +} from '../../src/utils/mintScopedPreviewToken.js'; +import { getUserNameFromToken } from '../../src/utils/jwtUtils.js'; + +function makeToken(username: string, signature = 'sig'): string { + const payload = Buffer.from(JSON.stringify({ u: username })).toString( + 'base64' + ); + return `pk.${payload}.${signature}`; +} + +function stubHttpRequest(token: string, status = 200) { + return vi.fn(async (input: string | URL | Request, init?: RequestInit) => { + void input; + void init; + return new Response(JSON.stringify({ token }), { status }); + }); +} + +describe('mintScopedPreviewToken', () => { + it('POSTs to tokens/v2/{username} for the server token’s own account with the requested scopes and note', async () => { + const serverToken = makeToken('acct', 'server-sig'); + const httpRequest = stubHttpRequest(makeToken('acct', 'minted-sig')); + + await mintScopedPreviewToken(httpRequest, serverToken, { + note: 'Test preview', + scopes: ['styles:tiles', 'styles:read'] + }); + + expect(httpRequest).toHaveBeenCalledTimes(1); + const [url, init] = httpRequest.mock.calls[0]; + expect(String(url)).toContain('tokens/v2/acct'); + expect(String(url)).toContain(`access_token=${serverToken}`); + const body = JSON.parse((init as RequestInit).body as string); + expect(body).toMatchObject({ + note: 'Test preview', + scopes: ['styles:tiles', 'styles:read'] + }); + expect(new Date(body.expires).getTime()).toBeGreaterThan(Date.now()); + }); + + it('defaults to a ~1 hour expiry, overridable via expiresInMs', async () => { + const serverToken = makeToken('acct'); + const httpRequest = stubHttpRequest(makeToken('acct')); + + const before = Date.now(); + await mintScopedPreviewToken(httpRequest, serverToken, { + note: 'n', + scopes: [] + }); + const [, defaultInit] = httpRequest.mock.calls[0]; + const defaultExpires = new Date( + JSON.parse((defaultInit as RequestInit).body as string).expires + ).getTime(); + expect(defaultExpires - before).toBeGreaterThan(59 * 60 * 1000); + expect(defaultExpires - before).toBeLessThan(61 * 60 * 1000); + + await mintScopedPreviewToken(httpRequest, serverToken, { + note: 'n', + scopes: [], + expiresInMs: 5 * 60 * 1000 + }); + const [, customInit] = httpRequest.mock.calls[1]; + const customExpires = new Date( + JSON.parse((customInit as RequestInit).body as string).expires + ).getTime(); + expect(customExpires - before).toBeLessThan(6 * 60 * 1000); + }); + + it('returns the minted token when it belongs to the same account as the server token', async () => { + const serverToken = makeToken('acct'); + const minted = makeToken('acct', 'minted-sig'); + const httpRequest = stubHttpRequest(minted); + + const result = await mintScopedPreviewToken(httpRequest, serverToken, { + note: 'n', + scopes: [] + }); + + expect(result).toBe(minted); + }); + + it('throws if the minted token belongs to a different account (AGI-905-style cross-account check)', async () => { + const serverToken = makeToken('victim'); + const httpRequest = stubHttpRequest(makeToken('attacker')); + + await expect( + mintScopedPreviewToken(httpRequest, serverToken, { + note: 'n', + scopes: [] + }) + ).rejects.toThrow('Minted token does not match caller account'); + }); + + it('omits `expires` entirely (mints a genuine, non-expiring pk.* token) when expiresInMs is null', async () => { + const serverToken = makeToken('acct'); + const httpRequest = stubHttpRequest(makeToken('acct')); + + await mintScopedPreviewToken(httpRequest, serverToken, { + note: 'n', + scopes: [], + expiresInMs: null + }); + + const [, init] = httpRequest.mock.calls[0]; + const body = JSON.parse((init as RequestInit).body as string); + expect(body).not.toHaveProperty('expires'); + }); + + it('throws with the status code, not the response body, when the Token API call fails', async () => { + const serverToken = makeToken('acct'); + const httpRequest = vi.fn( + async () => new Response('super secret leak', { status: 403 }) + ); + + await expect( + mintScopedPreviewToken(httpRequest, serverToken, { + note: 'n', + scopes: [] + }) + ).rejects.toThrow('Token API 403'); + }); +}); + +describe('describeAutoMintFailure', () => { + it('rewrites getUserNameFromToken’s "not in valid JWT format" error (the hosted-endpoint case) into hosted-aware, actionable wording', () => { + let caught: unknown; + try { + // A hosted-deployment-style opaque bearer: not a Mapbox pk./sk./tk. + // token at all, so it fails jwtUtils' part-count check. + getUserNameFromToken('not-a-mapbox-token'); + } catch (error) { + caught = error; + } + + const message = describeAutoMintFailure(caught); + expect(message).toContain('list_tokens_tool'); + expect(message).toContain('create_token_tool'); + expect(message).not.toContain('MAPBOX_ACCESS_TOKEN'); + }); + + it('rewrites getUserNameFromToken’s "does not contain username" error the same way', () => { + const noUsernameToken = `pk.${Buffer.from(JSON.stringify({ notU: 'x' })).toString('base64')}.sig`; + let caught: unknown; + try { + getUserNameFromToken(noUsernameToken); + } catch (error) { + caught = error; + } + + const message = describeAutoMintFailure(caught); + expect(message).toContain('list_tokens_tool'); + }); + + it('rewrites a Token API 401/403 (missing tokens:write) into actionable wording', () => { + expect(describeAutoMintFailure(new Error('Token API 403'))).toContain( + 'tokens:write' + ); + expect(describeAutoMintFailure(new Error('Token API 401'))).toContain( + 'tokens:write' + ); + }); + + it('passes through any other error unchanged (e.g. a transient failure)', () => { + expect(describeAutoMintFailure(new Error('Token API 500'))).toBe( + 'Token API 500' + ); + expect( + describeAutoMintFailure( + new Error('Minted token does not match caller account') + ) + ).toBe('Minted token does not match caller account'); + }); + + it('handles a non-Error thrown value', () => { + expect(describeAutoMintFailure('a plain string')).toBe('a plain string'); + }); +});