diff --git a/CHANGELOG.md b/CHANGELOG.md index 89fc893..5fbc8fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ ## Unreleased +### Changed + +- **GeoJSON Preview UI resource** now mints its short-lived Mapbox GL token per request instead of reusing a process-wide cached one, and verifies the minted token belongs to the requesting account before embedding it. + +### Fixed + +- **geojson_preview_tool**: Generate `https://geojson.io/?data=...` instead of `https://geojson.io/next/?data=...`. The `/next/` path now returns 404, so the previewed link no longer opened a broken page. The `?data=` query-param format is unchanged. + ## 0.8.0 - 2026-05-05 ## 0.7.5 - 2026-05-05 diff --git a/README.md b/README.md index bd3a70a..9759c7a 100644 --- a/README.md +++ b/README.md @@ -1330,7 +1330,7 @@ Follow these steps to publish a new release: 5. **Publish via the [mcp-server-publisher](https://github.com/mapbox/mcp-server-publisher) workflow:** - Go to the Actions tab in the `mcp-server-publisher` repo - - Select "Manual Release MCP Server to NPM and MCP Registry" + - Select "Release MCP Server" - Choose `mcp-devkit-server` from the repository dropdown - Enter the version — it **must exactly match** the `package.json` version - Leave the branch field empty for stable releases (or specify a branch for dev releases) @@ -1352,7 +1352,7 @@ The `sync-manifest-version.cjs` script handles syncing these automatically from To publish a pre-release from a feature branch: -1. Set the version in `package.json` with a pre-release suffix (e.g., `1.0.0-dev` or `1.0.0-beta`) +1. Set the version in `package.json` with a pre-release suffix `-dev` (e.g., `1.0.0-dev`) 2. Run `node scripts/sync-manifest-version.cjs` 3. In the publisher workflow, enter the version and specify the branch name 4. The package will be published to NPM under the `dev` tag (won't affect `latest`) diff --git a/src/resources/ui-apps/GeojsonPreviewUIResource.ts b/src/resources/ui-apps/GeojsonPreviewUIResource.ts index 0b442bf..48052fd 100644 --- a/src/resources/ui-apps/GeojsonPreviewUIResource.ts +++ b/src/resources/ui-apps/GeojsonPreviewUIResource.ts @@ -16,23 +16,12 @@ import { const MAPBOX_GL_VERSION = '3.12.0'; -// GL JS requires a public (pk.*) token. We create a short-lived one from the -// sk.* server token and cache it until it's close to expiry. -interface CachedToken { - token: string; - expiresAt: number; // ms since epoch -} -let cachedPublicToken: CachedToken | null = null; - -async function getPublicToken(skToken: string): Promise { - const now = Date.now(); - // Re-use cached token if it has more than 5 minutes left - if (cachedPublicToken && cachedPublicToken.expiresAt - now > 5 * 60 * 1000) { - return cachedPublicToken.token; - } - +// 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. +async function createPreviewToken(skToken: string): Promise { const username = getUserNameFromToken(skToken); - const expires = new Date(now + 60 * 60 * 1000).toISOString(); // 1 hour + 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, { @@ -46,11 +35,11 @@ async function getPublicToken(skToken: string): Promise { }); if (!response.ok) { - throw new Error(`Token API ${response.status}: ${await response.text()}`); + // 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 }; - cachedPublicToken = { token: data.token, expiresAt: now + 60 * 60 * 1000 }; return data.token; } @@ -80,7 +69,12 @@ export class GeojsonPreviewUIResource extends BaseResource { let accessToken = ''; if (skToken.startsWith('sk.')) { try { - accessToken = await getPublicToken(skToken); + 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; + } } catch { // Non-fatal — map won't render but the link button still works } diff --git a/src/tools/geojson-preview-tool/GeojsonPreviewTool.ts b/src/tools/geojson-preview-tool/GeojsonPreviewTool.ts index 0710d1c..ce60f0c 100644 --- a/src/tools/geojson-preview-tool/GeojsonPreviewTool.ts +++ b/src/tools/geojson-preview-tool/GeojsonPreviewTool.ts @@ -14,7 +14,7 @@ import { export class GeojsonPreviewTool extends BaseTool { name = 'geojson_preview_tool'; description = - 'Generate a geojson.io/next URL to visualize GeoJSON data. Returns only the URL link.'; + 'Generate a geojson.io URL to visualize GeoJSON data. Returns only the URL link.'; readonly annotations = { readOnlyHint: true, destructiveHint: false, @@ -78,13 +78,13 @@ export class GeojsonPreviewTool extends BaseTool { }; } - // Generate geojson.io/next URL - // Note: geojson.io/next uses query params (?data=) not hash params (#data=) + // Generate geojson.io URL + // Note: geojson.io uses query params (?data=) not hash params (#data=) const geojsonString = JSON.stringify(geojsonData); const encodedGeoJSON = encodeURIComponent(geojsonString); - const geojsonIOUrl = `https://geojson.io/next/?data=data:application/json,${encodedGeoJSON}`; + const geojsonIOUrl = `https://geojson.io/?data=data:application/json,${encodedGeoJSON}`; - // Use geojson.io/next as the display URL + // Use geojson.io as the display URL const displayUrl = geojsonIOUrl; // Build content array with URL diff --git a/test/resources/ui-apps/GeojsonPreviewUIResource.test.ts b/test/resources/ui-apps/GeojsonPreviewUIResource.test.ts new file mode 100644 index 0000000..95ed635 --- /dev/null +++ b/test/resources/ui-apps/GeojsonPreviewUIResource.test.ts @@ -0,0 +1,149 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { GeojsonPreviewUIResource } from '../../../src/resources/ui-apps/GeojsonPreviewUIResource.js'; + +const uri = new URL('ui://mapbox/geojson-preview/index.html'); + +// Build a Mapbox-style 3-part JWT whose payload carries the username (`u`). +function makeToken(prefix: 'sk' | 'pk' | 'tk', username: string): string { + const payload = Buffer.from(JSON.stringify({ u: username })).toString( + 'base64' + ); + return `${prefix}.${payload}.sig`; +} + +function embeddedToken(html: string): string | null { + const m = html.match(/var TOKEN = '([^']*)'/); + return m ? m[1] : null; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function extra(token?: string): any { + return token ? { authInfo: { token } } : {}; +} + +async function readHtml( + resource: GeojsonPreviewUIResource, + token?: string +): Promise { + const result = await resource['readCallback'](uri, extra(token)); + return result.contents[0].text as string; +} + +// Stub global fetch to mirror real Mapbox behaviour: POST tokens/v2/{username} +// mints a `tk` token for THAT account. Returns the mock for call assertions. +function stubMintingFetch() { + const fn = vi.fn(async (input: string | URL | Request) => { + const url = String(input); + const username = decodeURIComponent( + url.match(/tokens\/v2\/([^?]+)/)?.[1] ?? '' + ); + return new Response(JSON.stringify({ token: makeToken('tk', username) }), { + status: 200 + }); + }); + vi.stubGlobal('fetch', fn); + return fn; +} + +describe('GeojsonPreviewUIResource — AGI-905 cross-account token leak', () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('embeds only the caller’s own minted token, never another account’s (regression)', async () => { + const fetchMock = stubMintingFetch(); + const resource = new GeojsonPreviewUIResource(); + + const htmlA = await readHtml(resource, makeToken('sk', 'accountA')); + const htmlB = await readHtml(resource, makeToken('sk', 'accountB')); + + const tokA = embeddedToken(htmlA); + const tokB = embeddedToken(htmlB); + + // Each caller receives a token minted for their own account. + expect(tokA).toBe(makeToken('tk', 'accountA')); + expect(tokB).toBe(makeToken('tk', 'accountB')); + + // B must never receive A's token. + expect(tokB).not.toBe(tokA); + expect(htmlB).not.toContain(tokA as string); + + // No process-global cache: each read mints fresh. + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('mints a fresh token on every read (no shared cache, even for the same caller)', async () => { + const fetchMock = stubMintingFetch(); + const resource = new GeojsonPreviewUIResource(); + const sk = makeToken('sk', 'acct'); + + await readHtml(resource, sk); + await readHtml(resource, sk); + + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('does not embed a token minted for a different account (identity assertion)', async () => { + // Simulate a (hypothetical) backend returning a token for someone else. + vi.stubGlobal( + 'fetch', + vi.fn( + async () => + new Response(JSON.stringify({ token: makeToken('tk', 'attacker') }), { + status: 200 + }) + ) + ); + const resource = new GeojsonPreviewUIResource(); + + const html = await readHtml(resource, makeToken('sk', 'victim')); + + expect(embeddedToken(html)).toBe(''); + }); + + it('renders without a token when minting fails (graceful degradation)', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('forbidden', { status: 403 })) + ); + const resource = new GeojsonPreviewUIResource(); + + const result = await resource['readCallback']( + uri, + extra(makeToken('sk', 'acct')) + ); + + expect(result.contents).toHaveLength(1); + expect(embeddedToken(result.contents[0].text as string)).toBe(''); + }); + + it('passes a pk token through unchanged without minting', async () => { + const fetchMock = stubMintingFetch(); + const resource = new GeojsonPreviewUIResource(); + const pk = makeToken('pk', 'acct'); + + const html = await readHtml(resource, pk); + + expect(embeddedToken(html)).toBe(pk); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('renders without a token when no token is provided', async () => { + const saved = process.env.MAPBOX_ACCESS_TOKEN; + delete process.env.MAPBOX_ACCESS_TOKEN; + try { + const fetchMock = stubMintingFetch(); + const resource = new GeojsonPreviewUIResource(); + + const html = await readHtml(resource); + + expect(embeddedToken(html)).toBe(''); + expect(fetchMock).not.toHaveBeenCalled(); + } finally { + if (saved !== undefined) process.env.MAPBOX_ACCESS_TOKEN = saved; + } + }); +}); diff --git a/test/tools/__snapshots__/tool-naming-convention.test.ts.snap b/test/tools/__snapshots__/tool-naming-convention.test.ts.snap index e8e0c2e..a7277a3 100644 --- a/test/tools/__snapshots__/tool-naming-convention.test.ts.snap +++ b/test/tools/__snapshots__/tool-naming-convention.test.ts.snap @@ -44,7 +44,7 @@ exports[`Tool Naming Convention > should maintain consistent tool list (snapshot }, { "className": "GeojsonPreviewTool", - "description": "Generate a geojson.io/next URL to visualize GeoJSON data. Returns only the URL link.", + "description": "Generate a geojson.io URL to visualize GeoJSON data. Returns only the URL link.", "toolName": "geojson_preview_tool", }, { diff --git a/test/tools/geojson-preview-tool/GeojsonPreviewTool.test.ts b/test/tools/geojson-preview-tool/GeojsonPreviewTool.test.ts index a022d2b..60c161d 100644 --- a/test/tools/geojson-preview-tool/GeojsonPreviewTool.test.ts +++ b/test/tools/geojson-preview-tool/GeojsonPreviewTool.test.ts @@ -14,7 +14,7 @@ describe('GeojsonPreviewTool', () => { const tool = new GeojsonPreviewTool(); expect(tool.name).toBe('geojson_preview_tool'); expect(tool.description).toBe( - 'Generate a geojson.io/next URL to visualize GeoJSON data. Returns only the URL link.' + 'Generate a geojson.io URL to visualize GeoJSON data. Returns only the URL link.' ); }); @@ -39,8 +39,8 @@ describe('GeojsonPreviewTool', () => { expect(result.content[0].type).toBe('text'); const content = result.content[0]; if (content.type === 'text') { - // Should return geojson.io/next URL with query param format - expect(content.text).toMatch(/^https:\/\/geojson\.io\/next\/\?data=/); + // Should return geojson.io URL with query param format + expect(content.text).toMatch(/^https:\/\/geojson\.io\/\?data=/); } // Verify MCP-UI resource is included by default @@ -53,9 +53,9 @@ describe('GeojsonPreviewTool', () => { } }); - // Verify the iframe URL is geojson.io/next with query param + // Verify the iframe URL is geojson.io with query param const iframeUrl = (result.content[1] as any).resource.text; - expect(iframeUrl).toMatch(/^https:\/\/geojson\.io\/next\/\?data=/); + expect(iframeUrl).toMatch(/^https:\/\/geojson\.io\/\?data=/); }); it('returns URL and MCP-UI resource for backward compatibility', async () => { @@ -94,8 +94,8 @@ describe('GeojsonPreviewTool', () => { expect(result.content).toHaveLength(2); const content = result.content[0]; if (content.type === 'text') { - // Should return geojson.io/next URL with query param - expect(content.text).toMatch(/^https:\/\/geojson\.io\/next\/\?data=/); + // Should return geojson.io URL with query param + expect(content.text).toMatch(/^https:\/\/geojson\.io\/\?data=/); } }); @@ -132,8 +132,8 @@ describe('GeojsonPreviewTool', () => { expect(result.content).toHaveLength(2); const content = result.content[0]; if (content.type === 'text') { - // Should return geojson.io/next URL with query param - expect(content.text).toMatch(/^https:\/\/geojson\.io\/next\/\?data=/); + // Should return geojson.io URL with query param + expect(content.text).toMatch(/^https:\/\/geojson\.io\/\?data=/); } }); @@ -185,8 +185,8 @@ describe('GeojsonPreviewTool', () => { expect(result.content).toHaveLength(2); const content = result.content[0]; if (content.type === 'text') { - // Should return geojson.io/next URL with query param - expect(content.text).toMatch(/^https:\/\/geojson\.io\/next\/\?data=/); + // Should return geojson.io URL with query param + expect(content.text).toMatch(/^https:\/\/geojson\.io\/\?data=/); } }); });