chore: harden document fetching by Valiunia · Pull Request #45 · mapbox/mcp-docs-server

Valiunia · 2026-10-20T18:19:57Z

What changed

The shared HTTP pipeline no longer follows redirects automatically (NoRedirectPolicy).
get_document_tool / batch_get_documents_tool follow up to 3 redirects, and only when each target is an allowed documentation URL (https://docs.mapbox.com, plus MAPBOX_DOCS_HOST_OVERRIDE when set). After a redirect, the .md variant of the new location is tried first.
The URL allowlist moves into one shared module, src/utils/docUrl.ts. It requires https, no explicit port and no credentials.
access_token detection is case-insensitive.

Why

The same URL rules now apply to every request the doc tools make, including redirect targets, and live in one place instead of two copies.

How to verify

npm test, npm run lint, npm run format
get_document_tool with https://docs.mapbox.com/help/getting-started/attribution/ (a moved page) returns the page content.

- Add NoRedirectPolicy to the shared HTTP pipeline. Requests no longer follow redirects automatically. - fetchDocContent follows up to 3 redirects (301, 302, 303, 307, 308), and only when each target is an allowed documentation URL. Targets are resolved against the current URL; a missing, empty or self-referencing Location is an error. After a redirect, the .md variant of the new location is tried before the HTML page. fetchCachedText treats any 3xx as an error. - Add a trailing slash to extension-less docs.mapbox.com paths on the fallback fetch. - Move the URL allowlist into src/utils/docUrl.ts, shared by both tools. It accepts https://docs.mapbox.com (plus MAPBOX_DOCS_HOST_OVERRIDE when set) with no explicit port or credentials. Bare mapbox.com and docs.tilestream.net (unless it is the override host) are no longer accepted. - hasAccessToken matches the parameter name case-insensitively. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

mattpodwysocki

Looks good overall. The redirect design is right: the pipeline refuses all redirects, and the doc tools follow one only after re-validating each hop against the same allowlist used for user input, without echoing the untrusted Location header.

Verified locally: all 146 tests pass and typecheck is clean on cc29dfe. Against live docs.mapbox.com through the real pipeline, /help/getting-started/attribution/ follows its 301 and returns the moved page's .md, and /api/search gets its 302 to /api/search/ handled. All 12 llms.txt index sources plus the root llms.txt return 200 with no redirect, and all 1,243 links in them are on https://docs.mapbox.com, so dropping bare mapbox.com / docs.tilestream.net shouldn't break any URL the search tool returns.

A few follow-ups inline. One more that isn't on a changed line: the HTTP Pipeline section of CLAUDE.md should say the shared pipeline no longer follows redirects, so future tools that call other hosts know they'll get raw 3xx responses.

mattpodwysocki · 2026-10-20T20:09:56Z

  if (cached !== null) return cached;

  const response = await httpRequest(url, {});
+  assertNotRedirect(response);


fetchCachedText refuses every redirect, while fetchDocContent follows allowed ones. If docs ever adds a 301 to one of the llms.txt index URLs, searchDocs catches the error and returns [], so that product quietly drops out of search results. These are hard-coded docs URLs, so could this use fetchFollowingDocsRedirects too?

Done in 04440a7. fetchCachedText now uses fetchFollowingDocsRedirects, so a validated redirect on an llms.txt URL no longer drops that product from search. One guard added: if a redirected response is text/html it is rejected and not cached, so an index that moves to an HTML page cannot be cached as index text. Tests cover both cases.

mattpodwysocki · 2026-10-20T20:09:56Z

  }

+  if (finalUrl !== fetchUrl) {
+    const movedMdUrl = toMarkdownUrl(finalUrl);


When MAPBOX_DOCS_HOST_OVERRIDE is set, finalUrl is on the staging host, and toMarkdownUrl only matches docs.mapbox.com, so this returns null and staging gets HTML instead of the moved page's .md. Staging only, but it means staging behaves differently from production here.

Done in 04440a7. The final URL is mapped from the override host back to docs.mapbox.com before toMarkdownUrl, which re-applies the override. Staging now returns the moved page's .md, same as production. Added a test with MAPBOX_DOCS_HOST_OVERRIDE set.

mattpodwysocki · 2026-10-20T20:09:56Z

+    if (next === null) {
+      throw new RedirectError(`${response.status} Redirect not followed`);
+    }
+    current = next;


Nit: for a moved page, this fully requests the new location's HTML before fetchDocContent tries its .md. That's 4 requests where 3 would do. Trying toMarkdownUrl(next) before requesting next would skip one. The body is cancelled, so the cost is small.

Done in 04440a7, slightly differently from the suggestion. fetchFollowingDocsRedirects takes a beforeFollow hook that is called with each validated hop before it is requested. fetchDocContent uses it to probe the .md of the new location, so a moved page takes 3 requests. The probe URL is re-checked with isDocsUrl and hasAccessToken, is a single request with no redirects (so the total stays bounded at 11 requests in the worst case), and the same .md URL is never probed twice. Tests cover the request order, a target with access_token, and the duplicate case.

mattpodwysocki · 2026-10-20T20:09:56Z

+  systemHttpPipeline
+} from '../../src/utils/httpPipeline.js';
+
+function listen(


This starts real node:http servers. It's loopback-only and it's the only way to prove redirect: 'manual' works with real fetch, but CLAUDE.md says "No real network calls in tests." Could you add a comment marking it as a deliberate exception, or update CLAUDE.md?

Done in 04440a7. Added a comment in httpPipeline.test.ts marking the loopback node:http servers as a deliberate exception, and CLAUDE.md now notes the exception under "Important Constraints".

mattpodwysocki · 2026-10-20T20:09:56Z

@@ -1,5 +1,6 @@
 ## Unreleased

+- chore: harden document fetching. The shared HTTP pipeline no longer follows redirects automatically (`NoRedirectPolicy`); document fetches follow up to 3 redirects only when each target is an allowed docs URL (e.g. a page that moved). `access_token` detection is now case-insensitive. Also, `get_document_tool` / `batch_get_documents_tool` now accept only `https://docs.mapbox.com` URLs (plus `MAPBOX_DOCS_HOST_OVERRIDE` when set) via one shared check in `src/utils/docUrl.ts`. Bare `mapbox.com` and `docs.tilestream.net` (unless it is the override host) are no longer accepted (#45)


Nit: the PR is labeled chore, but it rejects inputs that used to work (bare mapbox.com, and docs.tilestream.net without the override). Could the entry state plainly that this is a breaking change for those URLs? It could also be shorter.

Done in 04440a7. The entry now starts with "Breaking for some URLs:", names bare mapbox.com and docs.tilestream.net (without the override), and is shorter.

Follow validated redirects for llms.txt index fetches, probe the .md of a moved page before requesting its HTML, and map the staging host back to docs.mapbox.com before deriving the .md URL. Document the loopback test exception and the no-redirect pipeline in CLAUDE.md, and state the URL restriction as a breaking change in the changelog. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Valiunia · 2026-10-21T08:20:40Z

Also done in 04440a7: the HTTP Pipeline section of CLAUDE.md now says the shared pipeline does not follow redirects (NoRedirectPolicy), so tools calling other hosts get raw 3xx responses, and that docs fetches follow redirects themselves in docFetcher.ts. Follow-ups since: spellcheck and build fixes, and a case-insensitive text/html match (90abb00). CI was green on the previous commit; re-checking on the latest.

mattpodwysocki

Thanks for the quick turnaround, all five comments are addressed. Re-verified on 90abb00: 151 tests pass, and typecheck, lint and the tshy build are clean. Against live docs.mapbox.com, the moved attribution page now resolves in 3 requests (.md 404 → HTML 301 → new .md 200), /api/search returns from its .md on the first request, and api/llms.txt still loads.

One non-blocking nit: the overloads on fetchFollowingDocsRedirects are a bit heavy. Not worth holding the PR for.

Valiunia and others added 2 commits October 20, 2026 19:19

docs: fill in PR number in CHANGELOG entry

cc29dfe

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Valiunia marked this pull request as ready for review October 20, 2026 18:24

Valiunia requested a review from a team as a code owner October 20, 2026 18:24

mattpodwysocki reviewed Oct 20, 2026

View reviewed changes

mattpodwysocki previously approved these changes Oct 20, 2026

View reviewed changes

Valiunia dismissed mattpodwysocki’s stale review via 04440a7 October 21, 2026 08:03

Valiunia and others added 3 commits October 21, 2026 09:07

fix: use US spelling in CLAUDE.md for spellcheck

6ba7d53

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

fix: remove unreachable branch that broke the tshy build

644e5dd

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

fix: match text/html content type case-insensitively

90abb00

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Valiunia requested a review from mattpodwysocki October 21, 2026 08:20

mattpodwysocki approved these changes Oct 21, 2026

View reviewed changes

Valiunia merged commit 0b8fb99 into main Oct 23, 2026
3 checks passed

		@@ -1,5 +1,6 @@
		## Unreleased

		- chore: harden document fetching. The shared HTTP pipeline no longer follows redirects automatically (`NoRedirectPolicy`); document fetches follow up to 3 redirects only when each target is an allowed docs URL (e.g. a page that moved). `access_token` detection is now case-insensitive. Also, `get_document_tool` / `batch_get_documents_tool` now accept only `https://docs.mapbox.com` URLs (plus `MAPBOX_DOCS_HOST_OVERRIDE` when set) via one shared check in `src/utils/docUrl.ts`. Bare `mapbox.com` and `docs.tilestream.net` (unless it is the override host) are no longer accepted (#45)

Conversation

Valiunia commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Why

How to verify

Uh oh!

mattpodwysocki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Valiunia commented Oct 21, 2026

Uh oh!

mattpodwysocki left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Valiunia commented Oct 20, 2026 •

edited

Loading