This is an automated email from the ASF dual-hosted git repository. cgivre pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/drill-mcp.git
commit df7c9d0d9d89546f5aac5e984360bd44d14ed3b8 Author: cgivre <[email protected]> AuthorDate: Wed Aug 12 16:25:59 2026 -0400 fix: strip URL userinfo from REST client error messages config.url is free-form and unvalidated; a password embedded there as userinfo (e.g. http://alice:s3cret@drill:8047) previously reached the model verbatim through DrillError messages raised by RestClient (connection failures, auth failures, JSON-decode errors). Add a _safe_url helper, mirroring the defense client_jdbc.py's _jdbc_url already applies, and use it at every site that echoes config.url. --- drill_mcp/client_rest.py | 39 ++++++++++++++++++++++++++++----------- tests/test_client_rest.py | 21 +++++++++++++++++++++ 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/drill_mcp/client_rest.py b/drill_mcp/client_rest.py index 2165ba0..1ca1461 100644 --- a/drill_mcp/client_rest.py +++ b/drill_mcp/client_rest.py @@ -31,6 +31,7 @@ import re from collections.abc import Callable from dataclasses import dataclass, field from typing import Any +from urllib.parse import urlparse, urlunparse import httpx @@ -180,6 +181,22 @@ def _check_query_id(query_id: str) -> str: return query_id +def _safe_url(url: str) -> str: + """Return `url` with any embedded userinfo (e.g. a password) stripped. + + `config.url` is free-form and unvalidated, so nothing stops a value like + `http://alice:s3cret@drill:8047`. Every message that echoes the URL back + to the model must use this instead of the raw config value -- the + `client_jdbc.py` backend already applies this exact defense to its + connection string; the REST backend must not diverge. + """ + parsed = urlparse(url) + if not parsed.hostname: + return url + netloc = parsed.hostname if parsed.port is None else f"{parsed.hostname}:{parsed.port}" + return urlunparse(parsed._replace(netloc=netloc)) + + def _json(response: httpx.Response, url: str) -> Any: """Decode a response body as JSON, converting a decode failure to `DrillError`. @@ -512,23 +529,23 @@ class RestClient: # "/j_security_check", so that check false-positives on success.) if response.status_code >= 400: raise DrillError( - f"authentication endpoint at {self._config.url} returned " + f"authentication endpoint at {_safe_url(self._config.url)} returned " f"HTTP {response.status_code}" ) if _contains_invalid_credentials_marker(response.text): raise DrillError( - f"authentication failed for user {self._config.user!r} at {self._config.url}" + f"authentication failed for user {self._config.user!r} at {_safe_url(self._config.url)}" ) self._authenticated = True def _transport_error(self, exc: httpx.HTTPError) -> DrillError: if isinstance(exc, httpx.TimeoutException): return DrillError( - f"request to {self._config.url} timed out after " + f"request to {_safe_url(self._config.url)} timed out after " f"{self._config.timeout_seconds}s" ) return DrillError( - f"could not reach Drill at {self._config.url} " + f"could not reach Drill at {_safe_url(self._config.url)} " f"(auth mode: {self._config.auth}): {type(exc).__name__}" ) @@ -546,7 +563,7 @@ class RestClient: if response.status_code == 401: raise DrillError( - f"authentication rejected by Drill at {self._config.url} " + f"authentication rejected by Drill at {_safe_url(self._config.url)} " f"for user {self._config.user!r}" ) if response.status_code >= 400: @@ -561,7 +578,7 @@ class RestClient: "/query.json", json={"queryType": "SQL", "query": sql, "autoLimit": max_rows}, ) - payload = _json(response, self._config.url) + payload = _json(response, _safe_url(self._config.url)) rows = payload.get("rows") or [] return QueryResult( columns=payload.get("columns") or [], @@ -594,11 +611,11 @@ class RestClient: def storage_plugins(self) -> list[dict[str, Any]]: response = self._request("GET", "/storage.json") - return redact(_json(response, self._config.url)) + return redact(_json(response, _safe_url(self._config.url))) def cluster_status(self) -> dict[str, Any]: - cluster = _json(self._request("GET", "/cluster.json"), self._config.url) - status = _json(self._request("GET", "/status.json"), self._config.url) + cluster = _json(self._request("GET", "/cluster.json"), _safe_url(self._config.url)) + status = _json(self._request("GET", "/status.json"), _safe_url(self._config.url)) merged = dict(cluster) if isinstance(cluster, dict) else {"cluster": cluster} if isinstance(status, dict): merged.update(status) @@ -609,7 +626,7 @@ class RestClient: def profiles(self, limit: int) -> list[dict[str, Any]]: limit = max(limit, 0) response = self._request("GET", "/profiles.json") - payload = _json(response, self._config.url) + payload = _json(response, _safe_url(self._config.url)) if not isinstance(payload, dict): payload = {} running = payload.get("runningQueries") or [] @@ -619,7 +636,7 @@ class RestClient: def profile(self, query_id: str) -> dict[str, Any]: _check_query_id(query_id) response = self._request("GET", f"/profiles/{query_id}.json") - return _json(response, self._config.url) + return _json(response, _safe_url(self._config.url)) def cancel_query(self, query_id: str) -> str: _check_query_id(query_id) diff --git a/tests/test_client_rest.py b/tests/test_client_rest.py index 983be61..72c4299 100644 --- a/tests/test_client_rest.py +++ b/tests/test_client_rest.py @@ -172,6 +172,27 @@ class TestQuery: assert BASE in str(exc.value) assert "s3cret" not in str(exc.value) + @respx.mock + def test_connection_failure_message_drops_a_password_embedded_in_the_url(self): + # Mirrors client_jdbc.py's + # test_jdbc_url_drops_userinfo_from_a_url_that_embeds_credentials -- + # the REST backend must apply the same defense. config.url is + # free-form and unvalidated, so nothing stops + # DRILL_URL=http://alice:s3cret@drill:8047; every message that + # echoes config.url back to the model must not leak the password + # embedded there. + url = "http://alice:s3cret@drill:8047"; + respx.post(f"{url}/j_security_check").mock(side_effect=httpx.ConnectError("refused")) + respx.post(f"{url}/query.json").mock(side_effect=httpx.ConnectError("refused")) + with pytest.raises(DrillError) as exc: + make_client(url=url, auth="basic", user="alice", password="s3cret").query( + "SELECT 1", max_rows=1 + ) + message = str(exc.value) + assert "s3cret" not in message + assert "alice" not in message + assert "drill:8047" in message + @respx.mock def test_timeout_is_reported_clearly(self): respx.post(f"{BASE}/query.json").mock(side_effect=httpx.ReadTimeout("slow"))
