chore: harden document fetching

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## Unreleased
 
+- chore: harden document fetching. **Breaking for some URLs:** `get_document_tool` and `batch_get_documents_tool` now accept only `https://docs.mapbox.com` URLs (plus `MAPBOX_DOCS_HOST_OVERRIDE` when set); bare `mapbox.com` and `docs.tilestream.net` (without the override) are rejected. The shared HTTP pipeline no longer follows redirects; document fetches follow up to 3 redirects, each only to an allowed docs URL. `access_token` detection is now case-insensitive (#45)
 - docs: fix two stale tool descriptions left over from the Algolia-to-`llms.txt` search migration. `search_mapbox_docs_tool`'s README entry still said it used "Algolia full-text search" — that was removed a while back (see the "Replace Algolia search with self-contained llms.txt search" entry below) and it now searches the self-hosted `llms.txt` index. Separately, `get_document_tool`'s own description (the text shown to the LLM, not just docs) referenced a tool named `get_latest_mapbox_docs_tool`, which doesn't exist — it should have said `search_mapbox_docs_tool`, and was actively misdirecting tool-chaining behavior.
 - fix: startup/shutdown/error logging messages now actually reach clients. The `McpServer` never declared the `logging` capability, so every `sendLoggingMessage` call (including the existing "server started"/"shutting down"/"Fatal error" messages) was a silent no-op regardless of when it was sent. Declared `logging: {}` in the server's capabilities.
 - feat: the server now identifies which MCP client connected to it. `server.server.getClientVersion()` (populated from the `clientInfo` sent in the client's `initialize` request) is now logged on connect, e.g. `Client identified as: claude-ai v1.0.0`. Reading it required moving the read to a `server.server.oninitialized` callback rather than right after `server.connect()`, since `getClientVersion()` is only populated once the client's `initialize` request has actually been processed, which is not guaranteed by the time `connect()`'s promise resolves. Also recorded as `mcp.client.name`/`mcp.client.version` on every subsequent tool-execution trace span (`withToolSpan`), so OTel-backed traces can be filtered or grouped by client. Matches the equivalent changes landing in `@mapbox/mcp-server` and `@mapbox/mcp-devkit-server`.

CLAUDE.md

@@ -71,6 +71,7 @@ export class MyTool extends BaseTool<
 - **Never patch `global.fetch`** — always use the injected `httpRequest`
 - The shared `httpRequest` from `httpPipeline.ts` adds User-Agent headers, 1-hour caching, and retry logic
 - Pass `httpRequest` to tool/resource constructors for testability
+- The shared pipeline does **not** follow redirects (`NoRedirectPolicy` sets `redirect: 'manual'`), so a tool calling any host gets the raw 3xx response. Docs fetches follow redirects themselves in `docFetcher.ts`, only to URLs that pass `isDocsUrl()` (`docUrl.ts`)
 
 ### Resource Architecture
 
@@ -123,6 +124,6 @@ npm run inspect:dev      # MCP Inspector against source (tsx, no build needed)
 
 - **No global patching**: use `httpRequest` from the pipeline
 - **Dependency injection**: tools and resources receive `httpRequest` in constructor
-- **No real network calls in tests**: mock `httpRequest` with `vi.fn()`
+- **No real network calls in tests**: mock `httpRequest` with `vi.fn()`. The one exception is `test/utils/httpPipeline.test.ts`, which uses loopback-only `node:http` servers to verify real `fetch` redirect behavior
 - **Strict types**: avoid `any`
 - **No access token env var**: docs tools are token-free; if a token is needed, it's an input field

src/tools/batch-get-documents-tool/BatchGetDocumentsTool.input.schema.ts

@@ -9,7 +9,7 @@ export const BatchGetDocumentsSchema = z.object({
     .min(1)
     .max(20)
     .describe(
-      'Array of Mapbox documentation page URLs to fetch (max 20). All must be mapbox.com URLs.'
+      'Array of Mapbox documentation page URLs to fetch (max 20). All must be https://docs.mapbox.com URLs.'
     )
 });
 

src/tools/batch-get-documents-tool/BatchGetDocumentsTool.ts

@@ -4,40 +4,14 @@
 import { CallToolResult } from '@modelcontextprotocol/sdk/types.js';
 import { docCache, normalizeCacheKey } from '../../utils/docCache.js';
 import { fetchDocContent } from '../../utils/docFetcher.js';
+import { hasAccessToken, isDocsUrl } from '../../utils/docUrl.js';
 import type { HttpRequest } from '../../utils/types.js';
 import { BaseTool } from '../BaseTool.js';
 import {
   BatchGetDocumentsSchema,
   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 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;
-  }
-}
-
 type DocumentResult =
   | { url: string; content: string }
   | { url: string; error: string };
@@ -66,13 +40,13 @@ export class BatchGetDocumentsTool extends BaseTool<
   protected async execute(
     input: BatchGetDocumentsInput
   ): Promise<CallToolResult> {
-    const invalidUrls = input.urls.filter((url) => !isMapboxUrl(url));
+    const invalidUrls = input.urls.filter((url) => !isDocsUrl(url));
     if (invalidUrls.length > 0) {
       return {
         content: [
           {
             type: 'text',
-            text: `Invalid URLs: only mapbox.com documentation URLs are supported. Invalid: ${invalidUrls.join(', ')}`
+            text: `Invalid URLs: only https://docs.mapbox.com documentation URLs are supported. Invalid: ${invalidUrls.join(', ')}`
           }
         ],
         isError: true

src/tools/get-document-tool/GetDocumentTool.input.schema.ts

@@ -8,7 +8,7 @@ export const GetDocumentSchema = z.object({
     .string()
     .url()
     .describe(
-      'URL of a Mapbox documentation page to fetch. Must be a mapbox.com URL (e.g. https://docs.mapbox.com/api/search/geocoding/).'
+      'URL of a Mapbox documentation page to fetch. Must be an https://docs.mapbox.com URL (e.g. https://docs.mapbox.com/api/search/geocoding/).'
     )
 });
 

src/tools/get-document-tool/GetDocumentTool.ts

@@ -4,40 +4,14 @@
 import { CallToolResult } from '@modelcontextprotocol/sdk/types.js';
 import { docCache } from '../../utils/docCache.js';
 import { fetchDocContent } from '../../utils/docFetcher.js';
+import { hasAccessToken, isDocsUrl } from '../../utils/docUrl.js';
 import type { HttpRequest } from '../../utils/types.js';
 import { BaseTool } from '../BaseTool.js';
 import {
   GetDocumentSchema,
   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 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;
-  }
-}
-
 export class GetDocumentTool extends BaseTool<typeof GetDocumentSchema> {
   name = 'get_document_tool';
   description =
@@ -58,12 +32,12 @@ export class GetDocumentTool extends BaseTool<typeof GetDocumentSchema> {
   }
 
   protected async execute(input: GetDocumentInput): Promise<CallToolResult> {
-    if (!isMapboxUrl(input.url)) {
+    if (!isDocsUrl(input.url)) {
       return {
         content: [
           {
             type: 'text',
-            text: `Invalid URL: only mapbox.com documentation URLs are supported. Received: ${input.url}`
+            text: `Invalid URL: only https://docs.mapbox.com documentation URLs are supported. Received: ${input.url}`
           }
         ],
         isError: true

src/utils/docFetcher.ts

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 import { docCache, MAX_ENTRY_BYTES, readBodyWithLimit } from './docCache.js';
+import { hasAccessToken, isDocsUrl } from './docUrl.js';
 import type { HttpRequest } from './types.js';
 
 const DOCS_HOSTNAME = 'docs.mapbox.com';
@@ -61,6 +62,106 @@ function applyHostOverride(url: string): string {
   }
 }
 
+/**
+ * docs.mapbox.com serves HTML pages at a trailing-slash path and answers the
+ * slash-less form with a redirect. Add the slash to extension-less paths so
+ * the fallback fetch gets the page directly.
+ */
+function withTrailingSlash(url: string): string {
+  try {
+    const parsed = new URL(url);
+    if (parsed.hostname !== getDocsHostname()) return url;
+    const lastSegment = parsed.pathname.split('/').pop() ?? '';
+    if (lastSegment !== '' && !lastSegment.includes('.')) {
+      parsed.pathname += '/';
+    }
+    return parsed.toString();
+  } catch {
+    return url;
+  }
+}
+
+const MAX_REDIRECTS = 3;
+const FOLLOWED_REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]);
+
+class RedirectError extends Error {}
+
+/**
+ * Resolve the Location of a redirect response against the URL that was
+ * requested. Returns null unless the status is one we follow and the target is
+ * a different, allowed documentation URL without an access_token.
+ */
+function redirectTarget(response: Response, current: string): string | null {
+  if (!FOLLOWED_REDIRECT_STATUSES.has(response.status)) return null;
+  const location = response.headers.get('location');
+  if (!location?.trim()) return null;
+  try {
+    const next = new URL(location, current).href;
+    if (next === new URL(current).href) return null;
+    if (!isDocsUrl(next) || hasAccessToken(next)) return null;
+    return next;
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * Map a URL on the effective docs host back to docs.mapbox.com. Needed because
+ * toMarkdownUrl only matches docs.mapbox.com and applies the host override
+ * itself.
+ */
+function toCanonicalDocsUrl(url: string): string {
+  try {
+    const parsed = new URL(url);
+    if (parsed.hostname === getDocsHostname()) {
+      parsed.hostname = DOCS_HOSTNAME;
+    }
+    return parsed.toString();
+  } catch {
+    return url;
+  }
+}
+
+/**
+ * GET a URL, following up to MAX_REDIRECTS redirects. Every hop is validated
+ * with isDocsUrl() before it is requested, so a redirect can only lead to
+ * another documentation URL. Throws RedirectError otherwise.
+ *
+ * `beforeFollow` is called with each validated hop before it is requested. If
+ * it returns content, that content is returned and the hop is not requested.
+ */
+async function fetchFollowingDocsRedirects(
+  url: string,
+  httpRequest: HttpRequest
+): Promise<{ response: Response; finalUrl: string }>;
+async function fetchFollowingDocsRedirects(
+  url: string,
+  httpRequest: HttpRequest,
+  beforeFollow: (next: string) => Promise<string | null>
+): Promise<{ response: Response; finalUrl: string } | { content: string }>;
+async function fetchFollowingDocsRedirects(
+  url: string,
+  httpRequest: HttpRequest,
+  beforeFollow?: (next: string) => Promise<string | null>
+): Promise<{ response: Response; finalUrl: string } | { content: string }> {
+  let current = url;
+  for (let hops = 0; ; hops++) {
+    const response = await httpRequest(current, {});
+    if (response.status < 300 || response.status >= 400) {
+      return { response, finalUrl: current };
+    }
+    void response.body?.cancel();
+    if (hops >= MAX_REDIRECTS) throw new RedirectError('Too many redirects');
+    const next = redirectTarget(response, current);
+    if (next === null) {
+      throw new RedirectError(`${response.status} Redirect not followed`);
+    }
+    const early = await beforeFollow?.(next);
+    if (early != null) return { content: early };
+    current = next;
+  }
+}
+
 /**
  * Fetch a URL and cache the result. Intended for fetching llms.txt index
  * files which are small, change infrequently, and are shared across multiple
@@ -73,11 +174,22 @@ export async function fetchCachedText(
   const cached = docCache.get(url);
   if (cached !== null) return cached;
 
-  const response = await httpRequest(url, {});
+  const { response, finalUrl } = await fetchFollowingDocsRedirects(
+    url,
+    httpRequest
+  );
 
   if (!response.ok) {
     throw new Error(`Failed to fetch ${url}: ${response.statusText}`);
   }
+  // A redirected index must still be text, not the HTML page it moved to.
+  if (
+    finalUrl !== url &&
+    response.headers.get('content-type')?.toLowerCase().includes('text/html')
+  ) {
+    void response.body?.cancel();
+    throw new Error(`Unexpected content type for ${url}`);
+  }
 
   const content = await response.text();
   if (Buffer.byteLength(content, 'utf8') > MAX_ENTRY_BYTES) {
@@ -90,7 +202,9 @@ export async function fetchCachedText(
 /**
  * Fetch a Mapbox documentation page, preferring the .md variant for
  * docs.mapbox.com URLs when available. Falls back to the original URL
- * if the .md endpoint returns a non-2xx response.
+ * if the .md endpoint returns a non-2xx response. Redirects are followed only
+ * to other allowed documentation URLs (e.g. a page that has moved); if the
+ * fallback ends up on a different URL, its .md variant is tried first.
  *
  * Set MAPBOX_DOCS_HOST_OVERRIDE=docs.tilestream.net to redirect all
  * docs.mapbox.com fetches to the staging environment.
@@ -99,18 +213,52 @@ export async function fetchDocContent(
   url: string,
   httpRequest: HttpRequest
 ): Promise<string> {
-  const mdUrl = toMarkdownUrl(url);
+  const tryMarkdown = async (mdUrl: string): Promise<string | null> => {
+    try {
+      const result = await fetchFollowingDocsRedirects(mdUrl, httpRequest);
+      if (result.response.ok) {
+        return await readBodyWithLimit(result.response, MAX_ENTRY_BYTES);
+      }
+      void result.response.body?.cancel();
+    } catch (error) {
+      if (!(error instanceof RedirectError)) throw error;
+    }
+    return null;
+  };
 
+  const mdUrl = toMarkdownUrl(url);
   if (mdUrl) {
-    const mdResponse = await httpRequest(mdUrl, {});
-    if (mdResponse.ok) {
-      return readBodyWithLimit(mdResponse, MAX_ENTRY_BYTES);
-    }
+    const markdown = await tryMarkdown(mdUrl);
+    if (markdown !== null) return markdown;
   }
 
-  const fetchUrl = applyHostOverride(url);
-  const response = await httpRequest(fetchUrl, {});
+  // For a moved page, try the .md variant of the new location before
+  // requesting its HTML. `next` has already passed isDocsUrl(); the derived
+  // .md URL is checked again before it is requested. A single request, no
+  // redirects: keeps the total request count bounded by MAX_REDIRECTS.
+  const probed = new Set<string>(mdUrl ? [mdUrl] : []);
+  const tryMovedMarkdown = async (next: string): Promise<string | null> => {
+    const movedMdUrl = toMarkdownUrl(toCanonicalDocsUrl(next));
+    if (!movedMdUrl || !isDocsUrl(movedMdUrl) || hasAccessToken(movedMdUrl)) {
+      return null;
+    }
+    if (probed.has(movedMdUrl)) return null;
+    probed.add(movedMdUrl);
+    const response = await httpRequest(movedMdUrl, {});
+    if (response.ok) return readBodyWithLimit(response, MAX_ENTRY_BYTES);
+    void response.body?.cancel();
+    return null;
+  };
+
+  const fetchUrl = withTrailingSlash(applyHostOverride(url));
+  const result = await fetchFollowingDocsRedirects(
+    fetchUrl,
+    httpRequest,
+    tryMovedMarkdown
+  );
+  if ('content' in result) return result.content;
 
+  const { response } = result;
   if (!response.ok) {
     throw new Error(`${response.status} ${response.statusText}`);
   }