Stop quoting the address a redirect sent the query to (#569)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: alistair3149

CHANGELOG.md

@@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
 - `delete-page` no longer sends an empty deletion reason when the wiki sets `attributeEdits: false` and the call gave no comment. MediaWiki recorded that empty reason verbatim, leaving a blank deletion log entry; with no reason sent at all it can autogenerate its own `content was: …` reason instead.
 - `update-page` no longer advertises itself as idempotent: in `mode='append'` and `mode='prepend'` it never was, so a client replaying a call whose result never arrived adds the content a second time. A replace resends the same content rather than adding to it.
 - `upload-file-from-url` and `update-file-from-url` no longer leak a connection when they refuse a source URL whose declared size is over `MCP_UPLOAD_MAX_BYTES`. Each refused call held one connection open for as long as the server ran.
+- A connection failure from `wikibase-query` no longer quotes the address a redirect sent the query to. A `307` or `308` moves the query to a second address that inherits the endpoint's path, and the failure printed that address in full; it now reads as the query service, or as its host alone when it lies elsewhere. Text the query service wrote itself still reaches the caller as the service wrote it.
 - A host correcting its clock no longer changes what the server does with elapsed time: rate-limit allowances, the shutdown grace window, the readiness and extension-detection caches, and the window a hosted sign-in has to finish. Measured against the wall clock, a backwards NTP step could refuse a caller that had barely touched its rate-limit allowance with a `Retry-After` of up to an hour, and a forwards step could end a graceful shutdown early, aborting the tool calls it was waiting for.
 
 ## [0.16.0] - 2026-07-30

src/tools/extensions/wikibase/sparql.ts

@@ -20,6 +20,15 @@ const MAX_SERVICE_MESSAGE_CHARS = 500;
 /** A line break inside a cell, which `GROUP_CONCAT` puts there routinely. */
 const LINE_BREAK = /\r\n|[\r\n]/g;
 
+/** An absolute URL sitting in a message, running to the first space. */
+const ABSOLUTE_URL = /https?:\/\/\S+/g;
+
+/**
+ * What a URL is called once the parts that can hold a token are gone. Reads as
+ * the subject of the sentence a transport error puts it in.
+ */
+const QUERY_SERVICE = "the wiki's query service";
+
 /** A query service failure already classified into an MCP error category. */
 export class SparqlError extends Error {
 	public constructor(
@@ -157,7 +166,7 @@ function classifyQueryFailure(err: unknown, endpoint: string): SparqlError {
 		);
 	}
 	const message = err instanceof Error ? err.message : String(err);
-	return new SparqlError('upstream_failure', withoutEndpoint(message, endpoint));
+	return new SparqlError('upstream_failure', transportMessage(message, endpoint));
 }
 
 /**
@@ -172,13 +181,58 @@ function originOf(target: string): string {
 }
 
 /**
- * The endpoint named rather than quoted. A transport error quotes the URL it
- * failed on, and a query service echoes the request URI into its own error page.
- * That URL is the operator's to know: it can carry a token in its path or query,
- * and it reaches the caller and the logs from here.
+ * A message this server wrote about a request it made itself, with every URL in
+ * it cut back. Substituting the endpoint would reach only the endpoint: a 307 or
+ * 308 is followed, and a relative Location resolves against the endpoint's path,
+ * so the hop after it fails on a URL that carries the endpoint's token under a
+ * spelling no substitution finds. Reading each URL for what it is catches the
+ * endpoint too, so the substitution has no work left to do here.
+ *
+ * Only this server's own messages come through here. A query service's
+ * diagnostics are the caller's IRIs quoted back, and cutting those back would
+ * answer a malformed query by deleting the term it complained about.
+ */
+function transportMessage(message: string, endpoint: string): string {
+	const service = endpointOrigin(endpoint);
+	return message.replace(ABSOLUTE_URL, (url) => urlAsRead(url, service));
+}
+
+/**
+ * The origin the endpoint's own URLs carry, or nothing when the wiki published an
+ * address that is not one. Read the way the transport reads it, since a wiki
+ * names a server it answers under either scheme without a scheme of its own.
+ */
+function endpointOrigin(endpoint: string): string | undefined {
+	try {
+		return originOf(endpoint.startsWith('//') ? `https:${endpoint}` : endpoint);
+	} catch {
+		return undefined;
+	}
+}
+
+/**
+ * A URL from the endpoint's own origin is the endpoint reached by another path,
+ * and is named as one. Any other keeps its origin, which is how an operator sees
+ * the host a redirect went to, or the host to add to `MCP_TRUSTED_HOSTS`. What
+ * does not parse cannot be cut back, so it goes entirely.
+ */
+function urlAsRead(url: string, service: string | undefined): string {
+	try {
+		const origin = originOf(url);
+		return origin === service ? QUERY_SERVICE : origin;
+	} catch {
+		return QUERY_SERVICE;
+	}
+}
+
+/**
+ * The endpoint named rather than quoted, for the query service's own error page,
+ * which echoes the request URI into it. That URL is the operator's to know: it
+ * can carry a token in its path or query, and it reaches the caller and the logs
+ * from here.
  */
 function withoutEndpoint(message: string, endpoint: string): string {
-	return endpoint === '' ? message : message.split(endpoint).join("the wiki's query service");
+	return endpoint === '' ? message : message.split(endpoint).join(QUERY_SERVICE);
 }
 
 function categoryForStatus(status: number): ErrorCategory {

tests/tools/extensions/wikibase/sparql.test.ts

@@ -309,6 +309,123 @@ describe('runSparqlQuery', () => {
 		expect(error.message).toBe("request to the wiki's query service failed");
 	});
 
+	// A 307 or 308 is followed, and a relative Location keeps the endpoint's path,
+	// so the hop after it fails on a URL that carries the same token and is not a
+	// substring the endpoint substitution can find.
+	it('names a redirect-derived URL as the endpoint in an unclassified failure', async () => {
+		const endpoint = 'https://query.example.org/hunter2/sparql';
+		vi.mocked(postForm).mockRejectedValue(
+			new FetchError(
+				'request to https://query.example.org/hunter2/results failed, reason: socket hang up',
+				'system',
+			),
+		);
+
+		const error = await failureOf(runSparqlQuery(endpoint, CATS, MANY_ROWS));
+
+		expect(error.message).toBe(
+			"request to the wiki's query service failed, reason: socket hang up",
+		);
+	});
+
+	// A Location that resolves to the endpoint plus something derives a URL the
+	// endpoint is a prefix of, which a substitution takes the front off rather than
+	// missing outright — leaving what the redirect appended standing.
+	it('names a redirect-derived URL that extends the endpoint', async () => {
+		const endpoint = 'https://query.example.org/hunter2/sparql';
+		vi.mocked(postForm).mockRejectedValue(
+			new FetchError(
+				'request to https://query.example.org/hunter2/sparql?sig=hunter3 failed, reason: socket hang up',
+				'system',
+			),
+		);
+
+		const error = await failureOf(runSparqlQuery(endpoint, CATS, MANY_ROWS));
+
+		expect(error.message).toBe(
+			"request to the wiki's query service failed, reason: socket hang up",
+		);
+	});
+
+	it('cuts back every URL in a message, not only the first one it meets', async () => {
+		const endpoint = 'https://query.example.org/hunter2/sparql';
+		vi.mocked(postForm).mockRejectedValue(
+			new Error(
+				'request to https://query.example.org/hunter2/a failed after https://query.example.org/hunter2/b',
+			),
+		);
+
+		const error = await failureOf(runSparqlQuery(endpoint, CATS, MANY_ROWS));
+
+		expect(error.message).toBe(
+			"request to the wiki's query service failed after the wiki's query service",
+		);
+	});
+
+	// A wiki names a server it answers under either scheme without a scheme of its
+	// own, and the transport reads that as `https`.
+	it('names a URL from a protocol-relative endpoint as the endpoint', async () => {
+		vi.mocked(postForm).mockRejectedValue(
+			new Error('request to https://query.example.org/hunter2/results failed'),
+		);
+
+		const error = await failureOf(
+			runSparqlQuery('//query.example.org/hunter2/sparql', CATS, MANY_ROWS),
+		);
+
+		expect(error.message).toBe("request to the wiki's query service failed");
+	});
+
+	// An endpoint that will not parse leaves nothing to compare a URL against, and
+	// that must cost the other URLs in the message their host, not spread the
+	// endpoint's name over hosts that are not it.
+	it('keeps a foreign host when the wiki publishes an endpoint that is not a URL', async () => {
+		vi.mocked(postForm).mockRejectedValue(
+			new Error('Refusing to fetch URL: https://backend.internal/hunter2/results'),
+		);
+
+		const error = await failureOf(runSparqlQuery('/query/sparql', CATS, MANY_ROWS));
+
+		expect(error.message).toBe('Refusing to fetch URL: https://backend.internal');
+	});
+
+	it('names what it cannot read as a URL rather than passing it through', async () => {
+		const endpoint = 'https://query.example.org/hunter2/sparql';
+		vi.mocked(postForm).mockRejectedValue(new Error('request to https://[hunter2/results failed'));
+
+		const error = await failureOf(runSparqlQuery(endpoint, CATS, MANY_ROWS));
+
+		expect(error.message).toBe("request to the wiki's query service failed");
+	});
+
+	// A hop to another host is refused by address before it is sent, and only the
+	// host names what the operator has to allow.
+	it('keeps the host of a URL from outside the endpoint, without its path', async () => {
+		const endpoint = 'https://query.example.org/hunter2/sparql';
+		vi.mocked(postForm).mockRejectedValue(
+			new Error(
+				'Refusing to fetch URL resolving to non-public address 10.0.0.1 (private); the server operator must add the host to MCP_TRUSTED_HOSTS to allow it: https://backend.internal/hunter2/results',
+			),
+		);
+
+		const error = await failureOf(runSparqlQuery(endpoint, CATS, MANY_ROWS));
+
+		expect(error.message).toContain('https://backend.internal');
+		expect(error.message).toContain('MCP_TRUSTED_HOSTS');
+		expect(error.message).not.toContain('hunter2');
+	});
+
+	// A parser message is the caller's own query quoted back, so the IRIs in it are
+	// the complaint rather than something to hide.
+	it('leaves the IRIs of the query alone in a service message', async () => {
+		const body = 'Encountered "<http://example.org/p>" at line 1, column 30.';
+		vi.mocked(postForm).mockRejectedValue(new HttpStatusError(400, ENDPOINT, body));
+
+		const error = await failureOf(runSparqlQuery(ENDPOINT, CATS, MANY_ROWS));
+
+		expect(error.message).toBe(body);
+	});
+
 	// A query service that echoes the request URI into its error page hands the
 	// token straight back, and the service message reaches the caller and the logs.
 	it('names the endpoint rather than quoting it back in a service message', async () => {