Skip to content

Commit c3b9808

Browse files
authored
fix: prevent path traversal in style and tileset tool URL construction (#103)
* fix: prevent path traversal in style and tileset tool URL construction Five tools (RetrieveStyle, DeleteStyle, UpdateStyle, PreviewStyle, TilequeryTool) concatenated user-supplied path parameters directly into Mapbox API URLs without validation or encoding. Because Node.js fetch uses the WHATWG URL parser, `../` sequences were normalized before sending, allowing requests to reach unintended API endpoints. Changes: - Add shared `styleIdSchema` with allowlist regex that rejects path separators, dots, percent-encoded sequences, and null bytes - Apply `styleIdSchema` to all four style tools via a shared module (src/tools/shared/styleId.schema.ts) - Add format validation to TilequeryTool tilesetId (owner.name format) - Wrap both username and styleId/tilesetId in `encodeURIComponent` at every URL construction site (defense-in-depth) - Replace silent fallback in output schema validation with explicit `isError: true` responses across all API tools, preventing unintended API responses from being forwarded to callers - Remove now-unused BaseTool.validateOutput() method - Add test/security/path-traversal.test.ts with 52 tests covering schema rejection, valid ID acceptance, URL encoding, and response schema mismatch behavior * fix: update tests for Prettier, Zod v4, and @mcp-ui/server v6.1.0 upgrades - Reformat dynamic imports to match Prettier 3.8.x style - Update Zod v4 error code assertions (invalid_value, too_big) - Update mimeType assertions for @mcp-ui/server v6.1.0 (text/html;profile=mcp-app) - Fix PreviewStyleTool test mimeType expectation * fix: reject cross-origin Link headers and redact tokens from logs - Validate that pagination next-page URLs from Link response headers share the same origin as the configured API endpoint; cross-origin URLs are rejected to prevent access token exfiltration - Add redactToken() utility that strips access_token query parameter values from strings before they reach log output or MCP client error responses (network errors include the full request URL in their message which would otherwise expose the token) - Remove full URL from info/debug level pagination log messages * refactor: extract handleValidationError helper and include error details in validation failures * test: assert validation error details are included in error response * test: add validation error response tests for TilequeryTool and RetrieveStyleTool
1 parent 759dbfc commit c3b9808

25 files changed

Lines changed: 564 additions & 171 deletions

File tree

src/tools/BaseTool.ts

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -135,26 +135,4 @@ export abstract class BaseTool<
135135
this.server.server.sendLoggingMessage({ level, data });
136136
}
137137
}
138-
139-
/**
140-
* Validates output data against the output schema.
141-
* If validation fails, logs a warning and returns the raw data instead of throwing an error.
142-
* This allows tools to continue functioning when API responses deviate from expected schemas.
143-
*/
144-
protected validateOutput<T>(
145-
schema: ZodTypeAny,
146-
rawData: unknown,
147-
toolName: string
148-
): T {
149-
try {
150-
return schema.parse(rawData) as T;
151-
} catch (validationError) {
152-
this.log(
153-
'warning',
154-
`${toolName}: Output schema validation failed - ${validationError instanceof Error ? validationError.message : 'Unknown validation error'}`
155-
);
156-
// Graceful fallback to raw data
157-
return rawData as T;
158-
}
159-
}
160138
}

src/tools/MapboxApiBasedTool.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ import { context, trace, SpanStatusCode } from '@opentelemetry/api';
1313
import type { ToolExecutionContext } from '../utils/tracing.js';
1414
import { createToolExecutionContext } from '../utils/tracing.js';
1515

16+
/** Remove access_token query parameter values from strings before logging or returning to callers. */
17+
export function redactToken(s: string): string {
18+
return s.replace(/access_token=[^&\s#"']+/g, 'access_token=***');
19+
}
20+
1621
/**
1722
* Standard error response format from Mapbox API
1823
*/
@@ -126,8 +131,8 @@ export abstract class MapboxApiBasedTool<
126131
toolContext.span.end();
127132
return result;
128133
} catch (error) {
129-
const errorMessage =
130-
error instanceof Error ? error.message : String(error);
134+
const rawMessage = error instanceof Error ? error.message : String(error);
135+
const errorMessage = redactToken(rawMessage);
131136
this.log(
132137
'error',
133138
`${this.name}: Error during execution: ${errorMessage}`
@@ -212,6 +217,18 @@ export abstract class MapboxApiBasedTool<
212217
};
213218
}
214219

220+
protected handleValidationError(error: unknown): CallToolResult {
221+
return {
222+
isError: true,
223+
content: [
224+
{
225+
type: 'text',
226+
text: `Unexpected API response format from Mapbox API: ${error instanceof Error ? error.message : 'Unknown validation error'}`
227+
}
228+
]
229+
};
230+
}
231+
215232
/**
216233
* Tool logic to be implemented by subclasses.
217234
* Must return a complete OutputSchema with content and optional structured content.

src/tools/create-token-tool/CreateTokenTool.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,12 @@ export class CreateTokenTool extends MapboxApiBasedTool<
9292

9393
const rawData = await response.json();
9494

95-
// Validate response against schema with graceful fallback
96-
const data = this.validateOutput<Record<string, unknown>>(
97-
CreateTokenOutputSchema,
98-
rawData,
99-
'CreateTokenTool'
100-
);
95+
let data;
96+
try {
97+
data = CreateTokenOutputSchema.parse(rawData);
98+
} catch (validationError) {
99+
return this.handleValidationError(validationError);
100+
}
101101

102102
this.log('info', `CreateTokenTool: Successfully created token`);
103103

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { z } from 'zod';
2+
import { styleIdSchema } from '../shared/styleId.schema.js';
23

34
export const DeleteStyleSchema = z.object({
4-
styleId: z.string().describe('Style ID to delete')
5+
styleId: styleIdSchema.describe('Style ID to delete')
56
});
67

78
export type DeleteStyleInput = z.infer<typeof DeleteStyleSchema>;

src/tools/delete-style-tool/DeleteStyleTool.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export class DeleteStyleTool extends MapboxApiBasedTool<
3434
_context: ToolExecutionContext
3535
): Promise<CallToolResult> {
3636
const username = getUserNameFromToken(accessToken);
37-
const url = `${MapboxApiBasedTool.mapboxApiEndpoint}styles/v1/${username}/${input.styleId}?access_token=${accessToken}`;
37+
const url = `${MapboxApiBasedTool.mapboxApiEndpoint}styles/v1/${encodeURIComponent(username)}/${encodeURIComponent(input.styleId)}?access_token=${accessToken}`;
3838

3939
const response = await this.httpRequest(url, {
4040
method: 'DELETE'

src/tools/list-styles-tool/ListStylesTool.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,12 @@ export class ListStylesTool extends MapboxApiBasedTool<
7070

7171
const rawData = await response.json();
7272

73-
// Validate the API response (which is an array) with graceful fallback
74-
const validatedData = this.validateOutput(
75-
StylesArraySchema,
76-
rawData,
77-
'ListStylesTool'
78-
);
73+
let validatedData;
74+
try {
75+
validatedData = StylesArraySchema.parse(rawData);
76+
} catch (validationError) {
77+
return this.handleValidationError(validationError);
78+
}
7979

8080
this.log('info', `ListStylesTool: Successfully listed styles`);
8181

src/tools/list-tokens-tool/ListTokensTool.ts

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import { CallToolResult } from '@modelcontextprotocol/sdk/types.js';
55
import type { HttpRequest } from '../../utils/types.js';
66
import type { ToolExecutionContext } from '../../utils/tracing.js';
7-
import { MapboxApiBasedTool } from '../MapboxApiBasedTool.js';
7+
import { MapboxApiBasedTool, redactToken } from '../MapboxApiBasedTool.js';
88
import {
99
ListTokensSchema,
1010
ListTokensInput
@@ -110,7 +110,7 @@ export class ListTokensTool extends MapboxApiBasedTool<
110110
while (url) {
111111
pageCount++;
112112
this.log('info', `ListTokensTool: Fetching page ${pageCount}`);
113-
this.log('debug', `ListTokensTool: Fetching URL: ${url}`);
113+
this.log('debug', `ListTokensTool: Fetching URL: ${redactToken(url)}`);
114114

115115
const response = await this.httpRequest(url, {
116116
method: 'GET',
@@ -130,12 +130,12 @@ export class ListTokensTool extends MapboxApiBasedTool<
130130
? data
131131
: (data as { tokens?: unknown[] }).tokens || [];
132132

133-
// Validate tokens array against TokenObjectSchema with graceful fallback
134-
const validatedTokens = this.validateOutput<unknown[]>(
135-
TokenObjectSchema.array(),
136-
tokens,
137-
'ListTokensTool'
138-
);
133+
let validatedTokens;
134+
try {
135+
validatedTokens = TokenObjectSchema.array().parse(tokens);
136+
} catch (validationError) {
137+
return this.handleValidationError(validationError);
138+
}
139139

140140
allTokens.push(...validatedTokens);
141141
this.log(
@@ -149,26 +149,34 @@ export class ListTokensTool extends MapboxApiBasedTool<
149149
if (linkHeader) {
150150
const links = this.parseLinkHeader(linkHeader);
151151
if (links.next) {
152-
// Ensure the next URL includes the access token
153152
const nextUrl = new URL(links.next);
154-
if (!nextUrl.searchParams.has('access_token')) {
155-
nextUrl.searchParams.append('access_token', accessToken);
156-
}
157-
const nextUrlString = nextUrl.toString();
158-
159-
if (shouldAutoPaginate) {
160-
url = nextUrlString;
161-
this.log('info', `ListTokensTool: Next page available: ${url}`);
153+
// Reject cross-origin Link headers to prevent token exfiltration
154+
const apiOrigin = new URL(ListTokensTool.mapboxApiEndpoint).origin;
155+
if (nextUrl.origin !== apiOrigin) {
156+
this.log(
157+
'warning',
158+
`ListTokensTool: Refusing cross-origin Link header: ${nextUrl.origin}`
159+
);
162160
} else {
163-
// For manual pagination, extract the start parameter from the next URL
164-
const nextUrl = new URL(links.next);
165-
const startParam = nextUrl.searchParams.get('start');
166-
if (startParam) {
167-
nextPageUrl = startParam;
168-
this.log(
169-
'info',
170-
`ListTokensTool: Next page start token saved for manual pagination: ${nextPageUrl}`
171-
);
161+
// Ensure the next URL includes the access token
162+
if (!nextUrl.searchParams.has('access_token')) {
163+
nextUrl.searchParams.append('access_token', accessToken);
164+
}
165+
const nextUrlString = nextUrl.toString();
166+
167+
if (shouldAutoPaginate) {
168+
url = nextUrlString;
169+
this.log('info', 'ListTokensTool: Next page available');
170+
} else {
171+
// For manual pagination, extract the start parameter from the next URL
172+
const startParam = nextUrl.searchParams.get('start');
173+
if (startParam) {
174+
nextPageUrl = startParam;
175+
this.log(
176+
'info',
177+
'ListTokensTool: Next page start token saved for manual pagination'
178+
);
179+
}
172180
}
173181
}
174182
}

src/tools/preview-style-tool/PreviewStyleTool.input.schema.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { z } from 'zod';
2+
import { styleIdSchema } from '../shared/styleId.schema.js';
23

34
export const PreviewStyleSchema = z.object({
4-
styleId: z.string().describe('Style ID to preview'),
5+
styleId: styleIdSchema.describe('Style ID to preview'),
56
accessToken: z
67
.string()
78
.startsWith(

src/tools/preview-style-tool/PreviewStyleTool.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export class PreviewStyleTool extends BaseTool<typeof PreviewStyleSchema> {
7474
const hashFragment =
7575
hashParams.length > 0 ? `#${hashParams.join('/')}` : '';
7676

77-
const url = `${MapboxApiBasedTool.mapboxApiEndpoint}styles/v1/${userName}/${input.styleId}.html?${params.toString()}${hashFragment}`;
77+
const url = `${MapboxApiBasedTool.mapboxApiEndpoint}styles/v1/${encodeURIComponent(userName)}/${encodeURIComponent(input.styleId)}.html?${params.toString()}${hashFragment}`;
7878

7979
// Build content array with URL
8080
const content: CallToolResult['content'] = [
@@ -86,7 +86,7 @@ export class PreviewStyleTool extends BaseTool<typeof PreviewStyleSchema> {
8686

8787
// Add MCP-UI resource (for legacy MCP-UI clients)
8888
const uiResource = createUIResource({
89-
uri: `ui://mapbox/preview-style/${userName}/${input.styleId}`,
89+
uri: `ui://mapbox/preview-style/${encodeURIComponent(userName)}/${encodeURIComponent(input.styleId)}`,
9090
content: {
9191
type: 'externalUrl',
9292
iframeUrl: url
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { z } from 'zod';
2+
import { styleIdSchema } from '../shared/styleId.schema.js';
23

34
export const RetrieveStyleSchema = z.object({
4-
styleId: z.string().describe('Style ID to retrieve')
5+
styleId: styleIdSchema.describe('Style ID to retrieve')
56
});
67

78
export type RetrieveStyleInput = z.infer<typeof RetrieveStyleSchema>;

0 commit comments

Comments
 (0)