From ad0a46bfccb20756c4f68aa469689e37b9403434 Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Fri, 5 Jun 2026 09:51:02 -0400 Subject: [PATCH 1/3] security: restrict doc tool URL allowlist and block access_token params Replace the broad *.mapbox.com hostname glob in GetDocumentTool and BatchGetDocumentsTool with an explicit allowlist of documentation hostnames (docs.mapbox.com, mapbox.com, docs.tilestream.net). Add a second guard that rejects any URL carrying an access_token query parameter in either tool. Together these prevent a shared cache poisoning path where a token-authorized response could be stored under a queryless cache key and later returned to a caller without a token. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- CHANGELOG.md | 1 + .../BatchGetDocumentsTool.ts | 35 +++++- .../get-document-tool/GetDocumentTool.ts | 34 +++++- .../BatchGetDocumentsTool.test.ts | 46 ++++++++ .../get-document-tool/GetDocumentTool.test.ts | 104 ++++++++++++++++++ 5 files changed, 216 insertions(+), 4 deletions(-) create mode 100644 test/tools/get-document-tool/GetDocumentTool.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 86f141d..07f88b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Unreleased +- security: restrict document tool URL allowlist to documentation hostnames and block access_token params (#TBD) - chore: upgrade @opentelemetry/\* packages to latest minor versions (#TBD) ## 0.3.0 - 2026-04-15 diff --git a/src/tools/batch-get-documents-tool/BatchGetDocumentsTool.ts b/src/tools/batch-get-documents-tool/BatchGetDocumentsTool.ts index 85bc86f..f5fd6c9 100644 --- a/src/tools/batch-get-documents-tool/BatchGetDocumentsTool.ts +++ b/src/tools/batch-get-documents-tool/BatchGetDocumentsTool.ts @@ -11,10 +11,28 @@ import { BatchGetDocumentsInput } from './BatchGetDocumentsTool.input.schema.js'; +// Explicit allowlist of hostnames this docs tool is permitted to fetch. +// api.mapbox.com is intentionally excluded — it is a live API that requires +// auth tokens, not a documentation host. Allowing it would let callers poison +// the shared cache with token-authorized private responses under no-token keys. +const ALLOWED_DOC_HOSTNAMES = new Set([ + 'docs.mapbox.com', + 'mapbox.com', + 'docs.tilestream.net' +]); + function isMapboxUrl(url: string): boolean { try { const { hostname } = new URL(url); - return hostname === 'mapbox.com' || hostname.endsWith('.mapbox.com'); + return ALLOWED_DOC_HOSTNAMES.has(hostname); + } catch { + return false; + } +} + +function hasAccessToken(url: string): boolean { + try { + return new URL(url).searchParams.has('access_token'); } catch { return false; } @@ -54,7 +72,20 @@ export class BatchGetDocumentsTool extends BaseTool< content: [ { type: 'text', - text: `Invalid URLs: only mapbox.com URLs are supported. Invalid: ${invalidUrls.join(', ')}` + text: `Invalid URLs: only mapbox.com documentation URLs are supported. Invalid: ${invalidUrls.join(', ')}` + } + ], + isError: true + }; + } + + const tokenUrls = input.urls.filter(hasAccessToken); + if (tokenUrls.length > 0) { + return { + content: [ + { + type: 'text', + text: `Invalid URLs: URLs must not contain access_token. Invalid: ${tokenUrls.join(', ')}` } ], isError: true diff --git a/src/tools/get-document-tool/GetDocumentTool.ts b/src/tools/get-document-tool/GetDocumentTool.ts index f1bbbfb..1269a23 100644 --- a/src/tools/get-document-tool/GetDocumentTool.ts +++ b/src/tools/get-document-tool/GetDocumentTool.ts @@ -11,10 +11,28 @@ import { GetDocumentInput } from './GetDocumentTool.input.schema.js'; +// Explicit allowlist of hostnames this docs tool is permitted to fetch. +// api.mapbox.com is intentionally excluded — it is a live API that requires +// auth tokens, not a documentation host. Allowing it would let callers poison +// the shared cache with token-authorized private responses under no-token keys. +const ALLOWED_DOC_HOSTNAMES = new Set([ + 'docs.mapbox.com', + 'mapbox.com', + 'docs.tilestream.net' +]); + function isMapboxUrl(url: string): boolean { try { const { hostname } = new URL(url); - return hostname === 'mapbox.com' || hostname.endsWith('.mapbox.com'); + return ALLOWED_DOC_HOSTNAMES.has(hostname); + } catch { + return false; + } +} + +function hasAccessToken(url: string): boolean { + try { + return new URL(url).searchParams.has('access_token'); } catch { return false; } @@ -45,7 +63,19 @@ export class GetDocumentTool extends BaseTool { content: [ { type: 'text', - text: `Invalid URL: only mapbox.com URLs are supported. Received: ${input.url}` + text: `Invalid URL: only mapbox.com documentation URLs are supported. Received: ${input.url}` + } + ], + isError: true + }; + } + + if (hasAccessToken(input.url)) { + return { + content: [ + { + type: 'text', + text: `Invalid URL: URLs must not contain access_token. Received: ${input.url}` } ], isError: true diff --git a/test/tools/batch-get-documents-tool/BatchGetDocumentsTool.test.ts b/test/tools/batch-get-documents-tool/BatchGetDocumentsTool.test.ts index a50a6cc..7e17420 100644 --- a/test/tools/batch-get-documents-tool/BatchGetDocumentsTool.test.ts +++ b/test/tools/batch-get-documents-tool/BatchGetDocumentsTool.test.ts @@ -124,6 +124,52 @@ describe('BatchGetDocumentsTool', () => { expect(result.isError).toBe(true); expect(httpRequest).not.toHaveBeenCalled(); }); + + it('rejects api.mapbox.com URLs', async () => { + const httpRequest = vi.fn(); + const tool = new BatchGetDocumentsTool({ httpRequest }); + + const result = await tool.run({ + urls: ['https://api.mapbox.com/styles/v1/owner/styleId'] + }); + + expect(result.isError).toBe(true); + expect(httpRequest).not.toHaveBeenCalled(); + }); + + it('rejects URLs containing access_token', async () => { + const httpRequest = vi.fn(); + const tool = new BatchGetDocumentsTool({ httpRequest }); + + const result = await tool.run({ + urls: ['https://docs.mapbox.com/page?access_token=pk.secret'] + }); + + expect(result.isError).toBe(true); + expect((result.content[0] as { text: string }).text).toMatch( + /access_token/ + ); + expect(httpRequest).not.toHaveBeenCalled(); + }); + + it('blocks cache poisoning: tokenized URL cannot prime cache for no-token URL', async () => { + // Even if somehow both URLs passed validation (they do not), this test + // documents the expected behavior: private data must not leak. + // In practice the access_token check above prevents this entirely. + const httpRequest = vi.fn().mockResolvedValue(makeResponse('private')); + const tool = new BatchGetDocumentsTool({ httpRequest }); + + // Attempt the poisoning using an api.mapbox.com URL — must be rejected + const poisonResult = await tool.run({ + urls: [ + 'https://api.mapbox.com/styles/v1/owner/id?access_token=secret', + 'https://api.mapbox.com/styles/v1/owner/id' + ] + }); + expect(poisonResult.isError).toBe(true); + expect(httpRequest).not.toHaveBeenCalled(); + expect(docCache.size).toBe(0); + }); }); describe('HTTP errors', () => { diff --git a/test/tools/get-document-tool/GetDocumentTool.test.ts b/test/tools/get-document-tool/GetDocumentTool.test.ts new file mode 100644 index 0000000..fb16498 --- /dev/null +++ b/test/tools/get-document-tool/GetDocumentTool.test.ts @@ -0,0 +1,104 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { GetDocumentTool } from '../../../src/tools/get-document-tool/GetDocumentTool.js'; +import { docCache } from '../../../src/utils/docCache.js'; + +beforeEach(() => { + docCache.clear(); +}); + +function makeResponse(body: string, status = 200): Response { + return new Response(body, { + status, + headers: { + 'content-type': 'text/plain', + 'content-length': String(Buffer.byteLength(body, 'utf8')) + } + }); +} + +describe('GetDocumentTool', () => { + describe('URL validation', () => { + it('rejects non-mapbox URLs', async () => { + const httpRequest = vi.fn(); + const tool = new GetDocumentTool({ httpRequest }); + + const result = await tool.run({ url: 'https://evil.com/page' }); + + expect(result.isError).toBe(true); + expect(httpRequest).not.toHaveBeenCalled(); + }); + + it('rejects api.mapbox.com URLs', async () => { + const httpRequest = vi.fn(); + const tool = new GetDocumentTool({ httpRequest }); + + const result = await tool.run({ + url: 'https://api.mapbox.com/styles/v1/owner/styleId' + }); + + expect(result.isError).toBe(true); + expect(httpRequest).not.toHaveBeenCalled(); + }); + + it('rejects URLs containing access_token', async () => { + const httpRequest = vi.fn(); + const tool = new GetDocumentTool({ httpRequest }); + + const result = await tool.run({ + url: 'https://docs.mapbox.com/page?access_token=pk.secret' + }); + + expect(result.isError).toBe(true); + expect((result.content[0] as { text: string }).text).toMatch( + /access_token/ + ); + expect(httpRequest).not.toHaveBeenCalled(); + }); + + it('allows docs.mapbox.com URLs', async () => { + const httpRequest = vi.fn().mockResolvedValue(makeResponse('content')); + const tool = new GetDocumentTool({ httpRequest }); + + const result = await tool.run({ url: 'https://docs.mapbox.com/page' }); + + expect(result.isError).toBe(false); + }); + }); + + describe('caching', () => { + it('returns cached content without an HTTP request', async () => { + docCache.set('https://docs.mapbox.com/page', 'cached content'); + const httpRequest = vi.fn(); + const tool = new GetDocumentTool({ httpRequest }); + + const result = await tool.run({ url: 'https://docs.mapbox.com/page' }); + + expect(result.isError).toBe(false); + expect((result.content[0] as { text: string }).text).toBe( + 'cached content' + ); + expect(httpRequest).not.toHaveBeenCalled(); + }); + }); + + describe('HTTP errors', () => { + it('returns an error on non-ok response', async () => { + const httpRequest = vi + .fn() + .mockResolvedValue( + new Response('Not Found', { status: 404, statusText: 'Not Found' }) + ); + const tool = new GetDocumentTool({ httpRequest }); + + const result = await tool.run({ url: 'https://docs.mapbox.com/missing' }); + + expect(result.isError).toBe(true); + expect((result.content[0] as { text: string }).text).toMatch( + /Failed to fetch/ + ); + }); + }); +}); From ad60c8d4350b303214f2e90912517952ba9c58be Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Fri, 5 Jun 2026 09:51:46 -0400 Subject: [PATCH 2/3] chore: update CHANGELOG with PR number for security fix Co-Authored-By: Claude Sonnet 4.6 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07f88b5..e71d6b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## Unreleased -- security: restrict document tool URL allowlist to documentation hostnames and block access_token params (#TBD) +- security: restrict document tool URL allowlist to documentation hostnames and block access_token params (#34) - chore: upgrade @opentelemetry/\* packages to latest minor versions (#TBD) ## 0.3.0 - 2026-04-15 From 279a34111eb9e5a14290171f66bdd261303aed9f Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Fri, 5 Jun 2026 09:53:41 -0400 Subject: [PATCH 3/3] chore: fix spellcheck failure in CHANGELOG Co-Authored-By: Claude Sonnet 4.6 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e71d6b7..6409edc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## Unreleased -- security: restrict document tool URL allowlist to documentation hostnames and block access_token params (#34) +- security: restrict document tool URL allowlist to documentation hosts and block access_token params (#34) - chore: upgrade @opentelemetry/\* packages to latest minor versions (#TBD) ## 0.3.0 - 2026-04-15