Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
## Unreleased

### Security

- **static_map_image_tool**: Validate `style` parameter against `username/style-id` format to prevent path traversal attacks where a crafted style value (e.g., `../../tokens/v2`) could escape the `/styles/v1/` URL path and access arbitrary Mapbox API endpoints using the server operator's token
- **static_map_image_tool**: Remove access token from URL returned in text content — the token is only used internally for the HTTP fetch and the MCP Apps iframe URL, not exposed to the model context

### Exports

- Added `getAllTools` to `@mapbox/mcp-server/tools` subpath export for batch access to all registered tools
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,10 +339,14 @@ export const StaticMapImageInputSchema = z.object({
),
style: z
.string()
.regex(
/^[a-zA-Z0-9_-]+\/[a-zA-Z0-9_-]+$/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would something like path.resolve(), path.normalize() be better than regex?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe not "better" but in addition to regex

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question — but path.resolve()/path.normalize() are the wrong fit here. They operate on filesystem paths and would canonicalize a traversal payload (e.g. ../../tokens/v2/tokens/v2) rather than reject it. The goal is to validate that the input matches a known-good format, not to sanitize an arbitrary path. A whitelist regex that only accepts username/style-id (alphanumeric, hyphens, underscores, exactly one slash) is the right approach — anything outside that shape is rejected outright.

'Style must be in the format username/style-id using only alphanumeric characters, hyphens, and underscores (e.g., mapbox/streets-v12)'
)
.optional()
.default('mapbox/streets-v12')
.describe(
'Mapbox style ID (e.g., mapbox/streets-v12, mapbox/satellite-v9, mapbox/dark-v11)'
'Mapbox style ID in the format username/style-id (e.g., mapbox/streets-v12, mapbox/satellite-v9, mapbox/dark-v11)'
),
highDensity: z
.boolean()
Expand Down
10 changes: 8 additions & 2 deletions src/tools/static-map-image-tool/StaticMapImageTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ export class StaticMapImageTool extends MapboxApiBasedTool<
}

const density = input.highDensity ? '@2x' : '';
const url = `${MapboxApiBasedTool.mapboxApiEndpoint}styles/v1/${input.style}/static/${overlayString}${lng},${lat},${input.zoom}/${width}x${height}${density}?access_token=${accessToken}`;
const encodedStyle = input.style
.split('/')
.map(encodeURIComponent)
.join('/');
const publicUrl = `${MapboxApiBasedTool.mapboxApiEndpoint}styles/v1/${encodedStyle}/static/${overlayString}${lng},${lat},${input.zoom}/${width}x${height}${density}`;
const url = `${publicUrl}?access_token=${accessToken}`;

// Fetch and encode image as base64 for clients without MCP Apps support
const response = await this.httpRequest(url);
Expand All @@ -125,10 +130,11 @@ export class StaticMapImageTool extends MapboxApiBasedTool<
const mimeType = isRasterStyle ? 'image/jpeg' : 'image/png';

// content[0] MUST be the URL text — MCP Apps UI finds it via content.find(c => c.type === 'text')
// Use public URL (without credentials) to avoid leaking the access token
const content: CallToolResult['content'] = [
{
type: 'text',
text: url
text: publicUrl
},
{
type: 'image',
Expand Down
28 changes: 26 additions & 2 deletions test/tools/static-map-image-tool/StaticMapImageTool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('StaticMapImageTool', () => {
);
expect(textContent.text).toContain('-74.006,40.7128,10');
expect(textContent.text).toContain('800x600');
expect(textContent.text).toContain('access_token=');
expect(textContent.text).not.toContain('access_token=');
} finally {
// Restore environment variable
if (originalEnv !== undefined) {
Expand Down Expand Up @@ -152,7 +152,7 @@ describe('StaticMapImageTool', () => {
expect(url).toContain('styles/v1/mapbox/dark-v10/static/');
expect(url).toContain('-122.4194,37.7749,15');
expect(url).toContain('1024x768');
expect(url).toContain('access_token=');
expect(url).not.toContain('access_token=');
});

it('uses default style when not specified', async () => {
Expand All @@ -168,6 +168,30 @@ describe('StaticMapImageTool', () => {
expect(url).toContain('styles/v1/mapbox/streets-v12/static/');
});

it('rejects style values with path traversal patterns', async () => {
const { httpRequest } = setupHttpRequest();
const tool = new StaticMapImageTool({ httpRequest });

const traversalPayloads = [
'../../tokens/v2',
'../styles',
'mapbox/../../../tokens',
'/etc/passwd',
'mapbox/streets-v12/../../tokens'
];

for (const style of traversalPayloads) {
await expect(
tool.run({
center: { longitude: -74, latitude: 40 },
zoom: 10,
size: { width: 600, height: 400 },
style
})
).resolves.toMatchObject({ isError: true });
}
});

it('validates coordinate constraints', async () => {
const { httpRequest } = setupHttpRequest();
const tool = new StaticMapImageTool({ httpRequest });
Expand Down
Loading