From e33e17f5e9e38399d721a7d66bffc8c884091a6a Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 11:38:14 -0600 Subject: [PATCH 01/23] feat(semantic-layer): define the provider HTTP transport contract --- .../superset_core/semantic_layers/__init__.py | 16 ++ .../superset_core/semantic_layers/layer.py | 9 ++ .../semantic_layers/transport.py | 153 ++++++++++++++++++ .../tests/semantic_layers/test_transport.py | 138 ++++++++++++++++ 4 files changed, 316 insertions(+) create mode 100644 superset-core/src/superset_core/semantic_layers/transport.py create mode 100644 superset-core/tests/semantic_layers/test_transport.py diff --git a/superset-core/src/superset_core/semantic_layers/__init__.py b/superset-core/src/superset_core/semantic_layers/__init__.py index a84ef804f854..2ccd96b58d9d 100644 --- a/superset-core/src/superset_core/semantic_layers/__init__.py +++ b/superset-core/src/superset_core/semantic_layers/__init__.py @@ -16,3 +16,19 @@ # under the License. """Semantic layer contracts for extension authors.""" + +from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure, + ProviderResponse, + ProviderTransport, + RequestBudget, +) + +__all__: list[str] = [ + "ProviderRequestError", + "ProviderRequestFailure", + "ProviderResponse", + "ProviderTransport", + "RequestBudget", +] diff --git a/superset-core/src/superset_core/semantic_layers/layer.py b/superset-core/src/superset_core/semantic_layers/layer.py index 6403db9ead9d..2ce548897919 100644 --- a/superset-core/src/superset_core/semantic_layers/layer.py +++ b/superset-core/src/superset_core/semantic_layers/layer.py @@ -29,6 +29,7 @@ SemanticCredentials, SemanticExecutionContext, ) +from superset_core.semantic_layers.transport import ProviderTransport from superset_core.semantic_layers.view import SemanticView ConfigT = TypeVar("ConfigT", bound=BaseModel) @@ -115,10 +116,14 @@ def from_configuration( *, cache_token: str, context: SemanticExecutionContext, + transport: ProviderTransport, credentials: SemanticCredentials | None = None, ) -> SemanticLayer[ConfigT, SemanticViewT]: """Bind this layer and its views to one host execution identity. + The host supplies transport separately from execution identity. + Use it for provider HTTP; no implicit or unbounded client is available. + Under CONNECTION, results and catalogs must not depend on the context. Store the supplied context and cache_token; partition provider caches on cache_token, never secrets. Non-connection credentials arrive exclusively @@ -133,6 +138,8 @@ def from_configuration( def get_configuration_schema( cls, configuration: ConfigT | None = None, + *, + transport: ProviderTransport, ) -> dict[str, Any]: """ Get the JSON schema for the configuration needed to add the semantic layer. @@ -162,6 +169,8 @@ def get_runtime_schema( cls, configuration: ConfigT, runtime_data: dict[str, Any] | None = None, + *, + transport: ProviderTransport, ) -> dict[str, Any]: """ Get the JSON schema for the runtime parameters needed to load semantic views. diff --git a/superset-core/src/superset_core/semantic_layers/transport.py b/superset-core/src/superset_core/semantic_layers/transport.py new file mode 100644 index 000000000000..7d413dfcee37 --- /dev/null +++ b/superset-core/src/superset_core/semantic_layers/transport.py @@ -0,0 +1,153 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Library-neutral, always-bounded HTTP capability supplied by the host.""" + +from __future__ import annotations + +import json # noqa: TID251 -- SDK cannot depend on the host JSON utility. +import math +from abc import ABC, abstractmethod +from collections.abc import Callable, Iterator, Mapping +from dataclasses import dataclass +from enum import Enum +from types import MappingProxyType +from typing import Any + + +@dataclass(frozen=True) +class RequestBudget: + """Absolute monotonic deadline and positive response-body byte cap.""" + + deadline: float + max_bytes: int + + def __post_init__(self) -> None: + if ( + isinstance(self.deadline, bool) + or not isinstance(self.deadline, (int, float)) + or not math.isfinite(self.deadline) + ): + raise ValueError("deadline must be finite monotonic seconds") + if type(self.max_bytes) is not int or self.max_bytes <= 0: + raise ValueError("max_bytes must be a positive integer") + + +class _Headers(Mapping[str, str]): + """Own an immutable, case-insensitive copy without HTTP-library types.""" + + def __init__(self, values: Mapping[str, str]) -> None: + self._values: Mapping[str, str] = MappingProxyType( + {key.lower(): value for key, value in values.items()} + ) + + def __getitem__(self, key: str) -> str: + return self._values[key.lower()] + + def __iter__(self) -> Iterator[str]: + return iter(self._values) + + def __len__(self) -> int: + return len(self._values) + + +@dataclass(frozen=True) +class ProviderResponse: + """Complete, bounded 2xx response; JSON decoding remains provider-owned.""" + + status: int + headers: Mapping[str, str] + body: bytes + url: str + + def __post_init__(self) -> None: + object.__setattr__(self, "headers", _Headers(self.headers)) + + def json(self) -> Any: + """Decode body bytes with the standard library; preserve decoding errors.""" + return json.loads(self.body) + + +class ProviderRequestFailure(str, Enum): + """Closed transport-failure categories, independent of provider payloads.""" + + INVALID_URL = "invalid_url" + REFUSED = "refused" + UNREACHABLE = "unreachable" + TIMEOUT = "timeout" + TOO_LARGE = "too_large" + UNSUPPORTED_ENCODING = "unsupported_encoding" + HTTP_STATUS = "http_status" + + +class ProviderRequestError(Exception): + """A safe failure containing no endpoint, credentials or response text.""" + + def __init__( + self, failure: ProviderRequestFailure, *, status: int | None = None + ) -> None: + self.failure: ProviderRequestFailure = failure + self.status: int | None = status + super().__init__(failure) + + def __str__(self) -> str: + messages: dict[ProviderRequestFailure, str] = { + ProviderRequestFailure.INVALID_URL: "Provider endpoint is not a valid URL.", + ProviderRequestFailure.REFUSED: "Provider endpoint is not allowed.", + ProviderRequestFailure.UNREACHABLE: ( + "Provider endpoint could not be reached." + ), + ProviderRequestFailure.TIMEOUT: ( + "Provider request exceeded its time budget." + ), + ProviderRequestFailure.TOO_LARGE: ( + "Provider response exceeded its byte budget." + ), + ProviderRequestFailure.UNSUPPORTED_ENCODING: ( + "Provider response encoding is not supported." + ), + ProviderRequestFailure.HTTP_STATUS: ( + f"Provider request failed with HTTP {self.status}." + ), + } + return messages[self.failure] + + +class ProviderTransport(ABC): + """Host destination policy and bounded HTTP, injectable in provider tests.""" + + @abstractmethod + def request( + self, + provider: str, + method: str, + url: str, + *, + budget: RequestBudget, + headers: Mapping[str, str] | None = None, + params: Mapping[str, Any] | None = None, + json: Any = None, + consume: Callable[[int], None] | None = None, + ) -> ProviderResponse: + """Return complete 2xx bytes or raise ProviderRequestError. + + The deadline covers resolution, connection, TLS, headers, redirects and + reads. Consume receives each chunk size before the byte-cap check. + Non-2xx bodies are unread; encoded bodies are refused. No proxy settings + from the environment apply. Only same-origin GET redirects are followed, + at most three. Providers share a deadline across pagination or polling. + """ diff --git a/superset-core/tests/semantic_layers/test_transport.py b/superset-core/tests/semantic_layers/test_transport.py new file mode 100644 index 000000000000..26369c8fab97 --- /dev/null +++ b/superset-core/tests/semantic_layers/test_transport.py @@ -0,0 +1,138 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Provider transport value contracts stay independent of an HTTP library.""" + +import math +from dataclasses import FrozenInstanceError + +import pytest + + +def test_response_headers_are_case_insensitive_and_owned() -> None: + from superset_core.semantic_layers.transport import ProviderResponse + + headers: dict[str, str] = {"Content-Type": "application/json"} + response: ProviderResponse = ProviderResponse( + 200, headers, b'{"ok":true}', "https://example.test/" + ) + headers["Content-Type"] = "changed" + assert response.headers["content-type"] == "application/json" + assert response.headers["CONTENT-TYPE"] == "application/json" + assert response.json() == {"ok": True} + with pytest.raises(FrozenInstanceError): + response.body = b"changed" # type: ignore[misc] + with pytest.raises(TypeError): + response.headers["x"] = "y" # type: ignore[index] + + +@pytest.mark.parametrize( + "deadline,cap", + [ + (math.inf, 1), + (math.nan, 1), + (True, 1), + (1.0, 0), + (1.0, -1), + (1.0, True), + (1.0, 1.5), + ], +) +def test_budget_rejects_unbounded_or_invalid_values(deadline: float, cap: int) -> None: + from superset_core.semantic_layers.transport import RequestBudget + + with pytest.raises(ValueError, match="deadline|max_bytes"): + RequestBudget(deadline, cap) + + +def test_budget_can_represent_an_expired_deadline() -> None: + from superset_core.semantic_layers.transport import RequestBudget + + assert RequestBudget(-1.0, 1).deadline == -1.0 + + +def test_errors_expose_only_categories_and_status() -> None: + from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure, + ) + + failure: ProviderRequestFailure + for failure in ProviderRequestFailure: + error: ProviderRequestError = ProviderRequestError( + failure, + status=503 if failure == ProviderRequestFailure.HTTP_STATUS else None, + ) + assert error.failure == failure + assert "https://" not in str(error) + assert "503" in str( + ProviderRequestError(ProviderRequestFailure.HTTP_STATUS, status=503) + ) + + +@pytest.mark.parametrize( + "method,args", + [ + ("from_configuration", ({},)), + ("get_configuration_schema", ()), + ("get_runtime_schema", ({},)), + ], +) +def test_sdk_entrypoints_require_explicit_transport( + method: str, args: tuple[object, ...] +) -> None: + from typing import Any + + from superset_core.semantic_layers.layer import SemanticLayer + + kwargs: dict[str, Any] = {} + if method == "from_configuration": + kwargs = {"cache_token": "generation", "context": None} + with pytest.raises(TypeError, match="transport"): + getattr(SemanticLayer, method)(*args, **kwargs) + + +@pytest.mark.parametrize( + "failure_name", + [ + "INVALID_URL", + "REFUSED", + "UNREACHABLE", + "TIMEOUT", + "TOO_LARGE", + "UNSUPPORTED_ENCODING", + "HTTP_STATUS", + ], +) +def test_transport_failure_round_trip_preserves_category_and_status( + failure_name: str, +) -> None: + import pickle + + from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure, + ) + + failure: ProviderRequestFailure = ProviderRequestFailure[failure_name] + error: ProviderRequestError = ProviderRequestError( + failure, status=503 if failure == ProviderRequestFailure.HTTP_STATUS else None + ) + restored: ProviderRequestError = pickle.loads(pickle.dumps(error)) # noqa: S301 -- locally constructed exception only + assert restored.failure is failure + assert restored.status == error.status + assert str(restored) == str(error) From 2ae9eeeb1a498432cb4390ae392e366e9723cbb1 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 11:38:24 -0600 Subject: [PATCH 02/23] feat(semantic-layer): pin provider connections and enforce request budgets --- superset/config.py | 3 + superset/semantic_layers/transport.py | 347 +++++ superset/utils/network.py | 146 +- .../semantic_layers/transport_test.py | 1245 +++++++++++++++++ tests/unit_tests/utils/test_network.py | 93 ++ 5 files changed, 1833 insertions(+), 1 deletion(-) create mode 100644 superset/semantic_layers/transport.py create mode 100644 tests/unit_tests/semantic_layers/transport_test.py diff --git a/superset/config.py b/superset/config.py index bf15334e8b72..11ed0c5cfb8c 100644 --- a/superset/config.py +++ b/superset/config.py @@ -3023,6 +3023,9 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument # noq # create or edit database connections. DATABASE_OAUTH2_ALLOW_INTERNAL_HOSTS: bool = False +# Provider-only opt-out: budgets, direct connections and redirect checks still apply. +SEMANTIC_LAYER_ALLOW_INTERNAL_HOSTS: bool = False + # Enable/disable CSP warning CONTENT_SECURITY_POLICY_WARNING = True diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py new file mode 100644 index 000000000000..f8b4210205f4 --- /dev/null +++ b/superset/semantic_layers/transport.py @@ -0,0 +1,347 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Host-owned provider HTTP policy with shared deadline enforcement.""" + +from __future__ import annotations + +import heapq +import logging +import socket +import time +from collections.abc import Callable, Mapping +from contextvars import Token +from dataclasses import dataclass, field +from io import BytesIO +from threading import Condition, Lock, Thread +from typing import Any +from urllib.parse import SplitResult, urljoin, urlsplit + +import requests +from flask import current_app, has_app_context +from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure as Failure, + ProviderResponse, + ProviderTransport, + RequestBudget, +) +from urllib3.exceptions import HTTPError + +from superset.utils.network import ( + ConnectionDeadline, + NonPublicAddressError, + PinnedHTTPAdapter, + PROVIDER_DEADLINE, +) + +logger: logging.Logger = logging.getLogger(__name__) +MAX_REDIRECTS: int = 3 +_CHUNK_BYTES: int = 65536 + + +class HostProviderTransport(ProviderTransport): + """Fix operator policy at construction; keep budgets and sockets call-local.""" + + def __init__(self, *, allow_internal_hosts: bool = False) -> None: + self._allow_internal: bool = allow_internal_hosts + + def request( + self, + provider: str, + method: str, + url: str, + *, + budget: RequestBudget, + headers: Mapping[str, str] | None = None, + params: Mapping[str, Any] | None = None, + json: Any = None, + consume: Callable[[int], None] | None = None, + ) -> ProviderResponse: + bound: _Deadline = _Deadline(budget.deadline) + token: Token[ConnectionDeadline | None] = PROVIDER_DEADLINE.set(bound) + try: + bound.remaining() # No parsing, resolution or socket work after expiry. + self._validated_url(url) + with _DirectSession() as session: + session.trust_env = False + adapter: PinnedHTTPAdapter = PinnedHTTPAdapter( + public_only=not self._allow_internal + ) + session.mount("https://", adapter) + session.mount("http://", adapter) + hop: int + for hop in range(MAX_REDIRECTS + 1): + with session.request( + method, + url, + headers={**(headers or {}), "Accept-Encoding": "identity"}, + params=params, + json=json, + timeout=bound.remaining(), + stream=True, + allow_redirects=False, + ) as response: + target: str | None = ( + self._next_redirect(method, response) + if hop < MAX_REDIRECTS + else None + ) + if target is not None: + url, params = target, None + continue # Context closes the unread redirect body. + if not 200 <= response.status_code < 300: + raise ProviderRequestError( + Failure.HTTP_STATUS, status=response.status_code + ) + if ( + response.headers.get("Content-Encoding", "identity").lower() + != "identity" + ): + raise ProviderRequestError(Failure.UNSUPPORTED_ENCODING) + body: bytes = self._read_body( + response, bound, budget.max_bytes, consume + ) + result: ProviderResponse = ProviderResponse( + response.status_code, response.headers, body, response.url + ) + bound.remaining() + return result + raise AssertionError("Redirect loop must return or fail") + except ( + requests.RequestException, + HTTPError, + OSError, + ProviderRequestError, + ) as exc: + error: ProviderRequestError = self._failure_for(exc, budget.deadline) + logger.warning( + "Provider request failed: category=%s status=%s", + error.failure.value, + error.status, + ) + raise error from None + finally: + PROVIDER_DEADLINE.reset(token) + bound.close() + + def _validated_url(self, url: str) -> SplitResult: + try: + parts: SplitResult = urlsplit(url) + parts.port # noqa: B018 Validate malformed ports before any I/O. + except ValueError: + raise ProviderRequestError(Failure.INVALID_URL) from None + if not parts.hostname: + raise ProviderRequestError(Failure.INVALID_URL) + if parts.scheme not in ( + {"https", "http"} if self._allow_internal else {"https"} + ): + raise ProviderRequestError(Failure.REFUSED) + return parts + + def _next_redirect(self, method: str, response: requests.Response) -> str | None: + if ( + method.upper() != "GET" + or response.status_code not in {301, 302, 303, 307, 308} + or not response.headers.get("Location") + ): + return None + before: SplitResult = self._validated_url(response.url) + try: + target: str = urljoin(response.url, response.headers["Location"]) + after: SplitResult = urlsplit(target) + after.port # noqa: B018 + except ValueError: + raise ProviderRequestError(Failure.INVALID_URL) from None + + def origin(parts: SplitResult) -> tuple[str, str | None, int]: + return ( + parts.scheme, + parts.hostname, + parts.port + if parts.port is not None + else (443 if parts.scheme == "https" else 80), + ) + + return target if origin(before) == origin(after) else None + + @staticmethod + def _read_body( + response: requests.Response, + bound: _Deadline, + cap: int, + consume: Callable[[int], None] | None, + ) -> bytes: + body: BytesIO = BytesIO() + while True: + bound.remaining() + chunk: bytes = response.raw.read1(_CHUNK_BYTES, decode_content=False) + bound.remaining() # Shutdown can look like EOF; refuse partial bytes. + if not chunk: + return body.getvalue() + if consume is not None: + consume(len(chunk)) + if body.tell() + len(chunk) > cap: + raise ProviderRequestError(Failure.TOO_LARGE) + body.write(chunk) + + @staticmethod + def _failure_for(exc: Exception, deadline: float) -> ProviderRequestError: + if time.monotonic() >= deadline or isinstance( + exc, (TimeoutError, requests.Timeout) + ): + return ProviderRequestError(Failure.TIMEOUT) + if isinstance(exc, ProviderRequestError): + return exc + pending: list[BaseException] = [exc] + seen: set[int] = set() + while pending: + cause: BaseException = pending.pop() + if id(cause) in seen: + continue + seen.add(id(cause)) + if isinstance(cause, NonPublicAddressError): + return ProviderRequestError(Failure.REFUSED) + pending.extend( + item + for item in ( + *cause.args, + cause.__cause__, + getattr(cause, "reason", None), + ) + if isinstance(item, BaseException) + ) + return ProviderRequestError( + Failure.INVALID_URL if isinstance(exc, ValueError) else Failure.UNREACHABLE + ) + + +def build_provider_transport() -> ProviderTransport: + """Fail closed outside app context; read only the provider-specific setting.""" + return HostProviderTransport( + allow_internal_hosts=has_app_context() + and current_app.config.get("SEMANTIC_LAYER_ALLOW_INTERNAL_HOSTS", False) is True + ) + + +class _DirectSession(requests.Session): + def get_redirect_target(self, resp: requests.Response) -> None: + # Requests otherwise reads redirect bodies despite allow_redirects=False. + return None + + +@dataclass(eq=False) +class _Deadline: + at: float + sockets: list[socket.socket] = field(default_factory=list) + closed: bool = False + expired: bool = False + + def remaining(self) -> float: + seconds: float = self.at - time.monotonic() + if seconds <= 0: + raise TimeoutError + return seconds + + def watch(self, sock: socket.socket) -> None: + _scheduler().register(self, sock) + + def close(self) -> None: + # A request rejected before connecting need not start a scheduler. + if _SCHEDULER is not None: + _SCHEDULER.close(self) + + +class DeadlineScheduler: + """One lazy worker interrupts I/O through owned duplicate descriptors.""" + + def __init__(self) -> None: + self._condition: Condition = Condition() + self._heap: list[tuple[float, int, _Deadline]] = [] + self._tokens: set[_Deadline] = set() + self._worker: Thread = Thread( + target=self._run, name="provider-deadlines", daemon=True + ) + self._worker.start() + + def register(self, bound: _Deadline, sock: socket.socket) -> None: + with self._condition: + if bound.closed: + return + bound.sockets.append(sock.dup()) + if bound.expired or bound.at <= time.monotonic(): + self._expire(bound) + elif bound not in self._tokens: + self._tokens.add(bound) + heapq.heappush(self._heap, (bound.at, id(bound), bound)) + self._condition.notify() + + def close(self, bound: _Deadline) -> None: + with self._condition: + bound.closed = True + self._tokens.discard(bound) + for sock in bound.sockets: + sock.close() + bound.sockets.clear() + # Completed long-deadline requests must not accumulate tombstones. + self._heap = [item for item in self._heap if item[2] in self._tokens] + heapq.heapify(self._heap) + self._condition.notify() + + def drain(self) -> None: + """Release all owned descriptors after isolated tests finish.""" + with self._condition: + for bound in tuple(self._tokens): + self._expire(bound) + self.close(bound) + + @staticmethod + def _expire(bound: _Deadline) -> None: + bound.expired = True + for sock in bound.sockets: + try: + sock.shutdown(socket.SHUT_RDWR) + except OSError: + pass + + def _run(self) -> None: + with self._condition: + while True: + if not self._heap: + self._condition.wait() + continue + deadline: float + bound: _Deadline + deadline, _, bound = self._heap[0] + remaining: float = deadline - time.monotonic() + if remaining > 0: + self._condition.wait(remaining) + else: + heapq.heappop(self._heap) + self._expire(bound) + + +_SCHEDULER: DeadlineScheduler | None = None +_SCHEDULER_LOCK: Lock = Lock() + + +def _scheduler() -> DeadlineScheduler: + global _SCHEDULER # pylint: disable=global-statement + with _SCHEDULER_LOCK: + if _SCHEDULER is None: + _SCHEDULER = DeadlineScheduler() + return _SCHEDULER diff --git a/superset/utils/network.py b/superset/utils/network.py index 03d25cb0dbd8..ab57dd878e0e 100644 --- a/superset/utils/network.py +++ b/superset/utils/network.py @@ -18,12 +18,16 @@ import platform import socket import subprocess -from typing import Any +from concurrent.futures import Future, ThreadPoolExecutor +from contextvars import ContextVar +from threading import Lock +from typing import Any, Protocol import requests from requests.adapters import HTTPAdapter from urllib3.connection import HTTPConnection, HTTPSConnection from urllib3.connectionpool import HTTPConnectionPool, HTTPSConnectionPool +from urllib3.exceptions import NewConnectionError # Networks that must never be reached via user-supplied hostnames. # Includes loopback, RFC-1918 private ranges, link-local (covers cloud @@ -260,3 +264,143 @@ def is_host_up(host: str) -> bool: return False return output == 0 + + +class ConnectionDeadline(Protocol): + """A caller-owned deadline; socket code does not choose request policy.""" + + def remaining(self) -> float: ... + def watch(self, sock: socket.socket) -> None: ... + + +PROVIDER_DEADLINE: ContextVar[ConnectionDeadline | None] = ContextVar( + "provider_connection_deadline", default=None +) +_RESOLVER: ThreadPoolExecutor | None = None +_RESOLVER_LOCK: Lock = Lock() + + +class NonPublicAddressError(OSError): + """Resolution included an address refused by the shared destination rules.""" + + +def _resolve_addresses( + host: str, port: int, timeout: float, *, public_only: bool +) -> list[str]: + """Bound waiting and pin the complete answer set before any connection.""" + global _RESOLVER # pylint: disable=global-statement + if timeout <= 0: + raise TimeoutError + with _RESOLVER_LOCK: + if _RESOLVER is None: + _RESOLVER = ThreadPoolExecutor(8, "provider-dns") + lookup: Future[Any] = _RESOLVER.submit( + socket.getaddrinfo, host, port, socket.AF_UNSPEC, socket.SOCK_STREAM + ) + try: + answers: Any = lookup.result(timeout=timeout) + finally: + lookup.cancel() # Queued work is disposable; running system DNS cannot stop. + addresses: list[str] = list(dict.fromkeys(str(info[4][0]) for info in answers)) + if not addresses: + raise OSError("No destination address") + if public_only and not all( + is_safe_ip(ipaddress.ip_address(address)) for address in addresses + ): + raise NonPublicAddressError + return addresses + + +def resolve_public_addresses(host: str, port: int, *, timeout: float) -> list[str]: + """Resolve once within timeout; reject the entire host if any answer is unsafe.""" + return _resolve_addresses(host, port, timeout, public_only=True) + + +def _connect_address(address: tuple[str, int], timeout: float) -> socket.socket: + """Connect to a numeric address without invoking a second resolver.""" + family: socket.AddressFamily = ( + socket.AF_INET6 + if ipaddress.ip_address(address[0]).version == 6 + else socket.AF_INET + ) + sock: socket.socket = socket.socket(family, socket.SOCK_STREAM) + try: + sock.settimeout(timeout) + sock.connect(address) + return sock + except BaseException: + sock.close() + raise + + +class _PinnedHTTPConnection(HTTPConnection): + _dns_host: str # urllib3 keeps the configured hostname here for resolution. + public_only: bool = True + + def _new_conn(self) -> socket.socket: + bound: ConnectionDeadline | None = PROVIDER_DEADLINE.get() + if bound is None: + raise RuntimeError("Pinned connections require a deadline") + addresses: list[str] = _resolve_addresses( + self._dns_host, self.port, bound.remaining(), public_only=self.public_only + ) + error: OSError = OSError("No reachable address") + address: str + for address in addresses: + try: + sock: socket.socket = _connect_address( + (address, self.port), bound.remaining() + ) + try: + bound.watch(sock) # Duplicate before TLS wrapping. + except BaseException: + sock.close() + raise + return sock + except OSError as exc: + error = exc + raise NewConnectionError(self, "Destination could not be reached") from error + + +class _PinnedHTTPSConnection(_PinnedHTTPConnection, HTTPSConnection): + """HTTPS retains the original host for SNI and certificate verification.""" + + +class _InternalHTTPConnection(_PinnedHTTPConnection): + public_only: bool = False + + +class _InternalHTTPSConnection(_InternalHTTPConnection, HTTPSConnection): + pass + + +class _PinnedHTTPPool(HTTPConnectionPool): + ConnectionCls: type[HTTPConnection] = _PinnedHTTPConnection + + +class _PinnedHTTPSPool(HTTPSConnectionPool): + ConnectionCls: type[HTTPSConnection] = _PinnedHTTPSConnection + + +class _InternalHTTPPool(HTTPConnectionPool): + ConnectionCls: type[HTTPConnection] = _InternalHTTPConnection + + +class _InternalHTTPSPool(HTTPSConnectionPool): + ConnectionCls: type[HTTPSConnection] = _InternalHTTPSConnection + + +class PinnedHTTPAdapter(HTTPAdapter): + """Direct connections with checked addresses and a caller-owned deadline.""" + + def __init__(self, *, public_only: bool) -> None: + self.public_only: bool = public_only + super().__init__(max_retries=0) + + def init_poolmanager(self, *args: Any, **kwargs: Any) -> None: + super().init_poolmanager(*args, **kwargs) + # Assign a new mapping; urllib3's original dictionary is shared globally. + self.poolmanager.pool_classes_by_scheme = { + "http": _PinnedHTTPPool if self.public_only else _InternalHTTPPool, + "https": _PinnedHTTPSPool if self.public_only else _InternalHTTPSPool, + } diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py new file mode 100644 index 000000000000..e2e77584808d --- /dev/null +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -0,0 +1,1245 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Semantic-layer provider HTTP requests reach only allowed destinations.""" + +from __future__ import annotations + +import ipaddress +import socket +import ssl +import time +from collections.abc import Iterator +from concurrent.futures import ThreadPoolExecutor +from contextlib import contextmanager +from datetime import datetime, timedelta, timezone +from http.server import BaseHTTPRequestHandler, HTTPServer +from pathlib import Path +from threading import get_ident, Thread +from typing import Any, TypeAlias +from unittest.mock import patch + +import pytest +import requests +import urllib3.util.connection +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import ec +from cryptography.x509.oid import NameOID +from flask import Flask +from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure, + ProviderResponse, + ProviderTransport, + RequestBudget, +) + +from superset.semantic_layers import transport as semantic_layer_http +from superset.utils import network + +# All tests run in their own pytest process; unrelated background traffic is +# explicitly exercised by the reference cases below. + + +def provider_request( + provider: str, method: str, url: str, *, timeout: float, **kwargs: Any +) -> ProviderResponse: + return semantic_layer_http.build_provider_transport().request( + provider, + method, + url, + budget=RequestBudget(time.monotonic() + timeout, 4 * 1024 * 1024), + **kwargs, + ) + + +def provider_body( + provider: str, + method: str, + url: str, + *, + deadline: float, + max_bytes: int, + timeout: tuple[float, float], + **kwargs: Any, +) -> bytes: + return ( + semantic_layer_http.build_provider_transport() + .request( + provider, method, url, budget=RequestBudget(deadline, max_bytes), **kwargs + ) + .body + ) + + +@pytest.fixture(autouse=True) +def loopback_connections_only(monkeypatch: pytest.MonkeyPatch) -> None: + """Make accidental outbound traffic fail the test before a socket connects.""" + original: Any = socket.socket.connect + + def connect(sock: socket.socket, address: Any) -> None: + assert isinstance(address, tuple) + assert ipaddress.ip_address(address[0]).is_loopback + original(sock, address) + + monkeypatch.setattr(socket.socket, "connect", connect) + + +Route: TypeAlias = tuple[int, dict[str, str], bytes] +OK: Route = (200, {}, b'{"ok": true}') + + +class _Handler(BaseHTTPRequestHandler): + routes: dict[str, Route] = {} + seen: list[str] = [] + + def _respond(self) -> None: + self.seen.append(f"{self.command} {self.path} {self.headers['Host']}") + status: int + headers: dict[str, str] + body: bytes + status, headers, body = self.routes.get(self.path, OK) + self.send_response(status) + for key, value in headers.items(): + self.send_header(key, value) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def do_GET(self) -> None: # noqa: N802 + self._respond() + + def do_POST(self) -> None: # noqa: N802 + self.rfile.read(int(self.headers.get("Content-Length", 0))) + self._respond() + + def log_message(self, *args: Any) -> None: + pass + + +@contextmanager +def _server( + routes: dict[str, Route] | None = None, context: ssl.SSLContext | None = None +) -> Iterator[tuple[int, list[str]]]: + seen: list[str] = [] + handler: type[_Handler] = type( + "Handler", (_Handler,), {"routes": routes or {}, "seen": seen} + ) + server: HTTPServer = HTTPServer(("127.0.0.1", 0), handler) + if context is not None: + server.socket = context.wrap_socket(server.socket, server_side=True) + thread: Thread = Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield server.server_address[1], seen + finally: + server.shutdown() + server.server_close() + + +@contextmanager +def _dns_answers(hostname: str, answers: list[Any]) -> Iterator[None]: + # Bounded requests resolve on provider-dns workers, so scope DNS by hostname. + original: Any = socket.getaddrinfo + + def resolve(host: str, *args: Any, **kwargs: Any) -> Any: + if host == hostname: + return answers + return original(host, *args, **kwargs) + + with patch.object(socket, "getaddrinfo", side_effect=resolve): + yield + + +def _background_connection() -> None: + """Complete real loopback traffic while a test's network patches are live.""" + with _server() as (port, _), ThreadPoolExecutor(1) as worker: + connection: socket.socket = worker.submit( + urllib3.util.connection.create_connection, ("127.0.0.1", port), timeout=1 + ).result(timeout=2) + connection.close() + + +def _background_request() -> None: + with _server() as (port, seen), ThreadPoolExecutor(1) as worker: + with requests.Session() as session: + session.trust_env = False + response: requests.Response = worker.submit( + session.get, f"http://127.0.0.1:{port}/background", timeout=1 + ).result(timeout=2) + assert response.status_code == 200 + assert seen == [f"GET /background 127.0.0.1:{port}"] + + +def _certificate(directory: Path, name: str) -> tuple[Path, Path]: + """Write a self-signed certificate for ``name``; return (cert, key) paths.""" + key: ec.EllipticCurvePrivateKey = ec.generate_private_key(ec.SECP256R1()) + subject: x509.Name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, name)]) + now: datetime = datetime.now(timezone.utc) + certificate: x509.Certificate = ( + x509.CertificateBuilder() + .subject_name(subject) + .issuer_name(subject) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now - timedelta(minutes=1)) + .not_valid_after(now + timedelta(hours=1)) + .add_extension(x509.SubjectAlternativeName([x509.DNSName(name)]), False) + .add_extension(x509.BasicConstraints(ca=True, path_length=None), True) + .sign(key, hashes.SHA256()) + ) + cert_path: Path = directory / f"{name}.crt" + key_path: Path = directory / f"{name}.key" + cert_path.write_bytes(certificate.public_bytes(serialization.Encoding.PEM)) + key_path.write_bytes( + key.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + ) + ) + return cert_path, key_path + + +@contextmanager +def _internal_hosts(allowed: bool) -> Iterator[None]: + app: Flask = Flask(__name__) + app.config.update( + WORKSPACE_NAME="semantic-layer-http-test", + SEMANTIC_LAYER_ALLOW_INTERNAL_HOSTS=allowed, + ) + with app.app_context(): + yield + + +@pytest.mark.parametrize("url", ["http://example.com/api", "ftp://example.com/x"]) +def test_https_guard_ignores_background_requests(url: str) -> None: + with patch("superset.semantic_layers.transport._DirectSession.request") as send: + _background_request() + with pytest.raises(ProviderRequestError, match="not allowed"): + provider_request("Cube", "GET", url, timeout=1) + send.assert_not_called() + + +@pytest.mark.parametrize("host", ["127.0.0.1", "localhost"]) +def test_internal_address_is_refused_before_connecting(host: str) -> None: + listener: socket.socket = socket.socket() + listener.bind(("127.0.0.1", 0)) + listener.listen() + listener.setblocking(False) + port: int = listener.getsockname()[1] + try: + with pytest.raises(ProviderRequestError) as raised: + provider_request("Cube", "GET", f"https://{host}:{port}/x", timeout=2) + with pytest.raises(BlockingIOError): + listener.accept() # Nothing ever connected. + finally: + listener.close() + assert host not in str(raised.value) + assert str(port) not in str(raised.value) + + +def test_any_internal_answer_refuses_the_host() -> None: + answers: list[tuple[Any, ...]] = [ + (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("93.184.216.34", 443)), + (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("10.0.0.5", 443)), + ] + with ( + _dns_answers("mixed.example.com", answers), + patch("superset.utils.network._connect_address") as connect, + ): + _background_connection() + with pytest.raises(ProviderRequestError): + provider_request("Cube", "GET", "https://mixed.example.com/x", timeout=1) + connect.assert_not_called() + + +@pytest.mark.parametrize("address", ["64:ff9b::a9fe:a9fe", "64:ff9b::a00:5"]) +def test_translated_ipv6_answers_are_checked_as_ipv4(address: str) -> None: + answers: list[tuple[Any, ...]] = [ + (socket.AF_INET6, socket.SOCK_STREAM, 6, "", (address, 443, 0, 0)), + ] + with ( + _dns_answers("nat64.example.com", answers), + patch("superset.utils.network._connect_address") as connect, + ): + _background_connection() + with pytest.raises(ProviderRequestError): + provider_request("Cube", "GET", "https://nat64.example.com/x", timeout=1) + connect.assert_not_called() + + +def test_checked_connection_ignores_background_connections() -> None: + answers: list[tuple[Any, ...]] = [ + (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("93.184.216.34", 443)), + ] + real_resolve: Any = socket.getaddrinfo + resolved: list[str] = [] + + def resolve(host: str, *args: Any, **kwargs: Any) -> Any: + # socket.getaddrinfo is process-global: answer and record only this + # test's host, and leave lookups from unrelated threads untouched. + if host != "public.example.com": + return real_resolve(host, *args, **kwargs) + resolved.append(host) + return answers + + with ( + patch.object(network.socket, "getaddrinfo", side_effect=resolve), + patch( + "superset.utils.network._connect_address", + side_effect=OSError("stop"), + ) as connect, + ): + _background_connection() + # Deterministically model an unrelated lookup while the global hook is live. + assert socket.getaddrinfo("localhost", 443) == real_resolve("localhost", 443) + with pytest.raises(ProviderRequestError): + provider_request("Cube", "GET", "https://public.example.com/x", timeout=1) + _background_connection() + assert resolved == ["public.example.com"] + assert connect.call_args.args[0] == ("93.184.216.34", 443) + + +@pytest.fixture +def public_tls( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> tuple[ssl.SSLContext, list[str]]: + """Treat ``*.example.test`` as public, served on 127.0.0.1 with a trusted cert.""" + cert: Path + key: Path + cert, key = _certificate(tmp_path, "public.example.test") + context: ssl.SSLContext = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + context.load_cert_chain(cert, key) + resolved: list[str] = [] + real_resolve: Any = socket.getaddrinfo + + owner: int = get_ident() + + def resolve(host: str, *args: Any, **kwargs: Any) -> Any: + # Names also resolve on provider-dns workers; IP-literal connects belong + # to the caller only. Do not record another thread's loopback traffic. + if host.endswith(".example.test") or ( + host == "127.0.0.1" and get_ident() == owner + ): + resolved.append(host) + local: str = "127.0.0.1" if host.endswith(".example.test") else host + return real_resolve(local, *args, **kwargs) + + original: Any = requests.Session.merge_environment_settings + + def trust_test_ca(self: requests.Session, *args: Any) -> dict[str, Any]: + settings: dict[str, Any] = original(self, *args) + if get_ident() != owner: + return settings + settings["verify"] = str(cert) + return settings + + monkeypatch.setattr(network.socket, "getaddrinfo", resolve) + monkeypatch.setattr(network, "is_safe_ip", lambda address: True) + monkeypatch.setattr(requests.Session, "merge_environment_settings", trust_test_ca) + return context, resolved + + +def test_public_tls_ignores_background_loopback_lookups( + public_tls: tuple[ssl.SSLContext, list[str]], +) -> None: + resolved: list[str] = public_tls[1] + assert socket.getaddrinfo("localhost", 443) + with ThreadPoolExecutor(1) as worker: + assert worker.submit(socket.getaddrinfo, "127.0.0.1", 443).result(timeout=2) + assert resolved == [] + assert socket.getaddrinfo("public.example.test", 443) + assert socket.getaddrinfo("public.example.test", 443) + assert resolved == ["public.example.test", "public.example.test"] + + +def test_default_path_verifies_the_configured_hostname( + public_tls: tuple[ssl.SSLContext, list[str]], +) -> None: + context: ssl.SSLContext + resolved: list[str] + context, resolved = public_tls + with _server(context=context) as (port, seen): + response: ProviderResponse = provider_request( + "Cube", "GET", f"https://public.example.test:{port}/meta", timeout=2 + ) + with pytest.raises(ProviderRequestError, match="could not be reached"): + # The certificate names public.example.test, not this host. + provider_request( + "Cube", "GET", f"https://other.example.test:{port}/meta", timeout=2 + ) + assert response.json() == {"ok": True} + assert seen == [f"GET /meta public.example.test:{port}"] + # Each hostname is resolved once; urllib3 then connects to the IP literal. + names: list[str] = [host for host in resolved if not host[0].isdigit()] + assert names == ["public.example.test", "other.example.test"] + assert set(resolved) - set(names) == set() # Numeric connects do not resolve again. + + +def test_default_path_follows_same_origin_get_redirects_only( + public_tls: tuple[ssl.SSLContext, list[str]], +) -> None: + context: ssl.SSLContext = public_tls[0] + routes: dict[str, Route] = { + "/accounts": (308, {"Location": "/accounts/"}, b""), + "/away": (302, {"Location": "https://other.example.test/x"}, b""), + } + with _server(routes, context) as (port, seen): + base: str = f"https://public.example.test:{port}" + response: ProviderResponse = provider_request( + "MetricFlow", "GET", f"{base}/accounts", timeout=2 + ) + with pytest.raises(ProviderRequestError, match="HTTP 302"): + provider_request("MetricFlow", "GET", f"{base}/away", timeout=2) + assert response.json() == {"ok": True} + assert [entry.split()[1] for entry in seen] == ["/accounts", "/accounts/", "/away"] + + +def test_flag_allows_local_http_development_endpoints() -> None: + with _server() as (port, seen), _internal_hosts(True): + response: ProviderResponse = provider_request( + "Cube", "GET", f"http://localhost:{port}/meta", timeout=2 + ) + assert response.json() == {"ok": True} + assert seen == [f"GET /meta localhost:{port}"] + + +def test_flag_off_keeps_local_endpoints_refused() -> None: + with _server() as (port, seen), _internal_hosts(False): + with pytest.raises(ProviderRequestError) as raised: + provider_request("Cube", "GET", f"https://localhost:{port}/m", timeout=2) + assert seen == [] + assert str(raised.value) == ("Provider endpoint is not allowed.") + + +def test_post_redirects_are_not_followed() -> None: + routes: dict[str, Route] = {"/graphql": (307, {"Location": "/other"}, b"")} + with _server(routes) as (port, seen), _internal_hosts(True): + with pytest.raises(ProviderRequestError, match=r"^Provider request failed"): + provider_request( + "DJ", "POST", f"http://127.0.0.1:{port}/graphql", json={}, timeout=2 + ) + assert [entry.split()[:2] for entry in seen] == [["POST", "/graphql"]] + + +def test_redirect_chains_are_bounded() -> None: + routes: dict[str, Route] = {"/loop": (302, {"Location": "/loop"}, b"")} + with _server(routes) as (port, seen), _internal_hosts(True): + with pytest.raises(ProviderRequestError, match="HTTP 302"): + provider_request("DJ", "GET", f"http://127.0.0.1:{port}/loop", timeout=2) + assert len(seen) == semantic_layer_http.MAX_REDIRECTS + 1 + + +def test_error_message_carries_status_only() -> None: + routes: dict[str, Route] = {"/a": (500, {}, b"internal detail")} + with _server(routes) as (port, _), _internal_hosts(True): + with pytest.raises(ProviderRequestError) as raised: + provider_request( + "MetricFlow", "GET", f"http://127.0.0.1:{port}/a", timeout=2 + ) + assert str(raised.value) == "Provider request failed with HTTP 500." + + +def test_transport_failure_hides_the_destination( + caplog: pytest.LogCaptureFixture, +) -> None: + with patch( + "superset.semantic_layers.transport._DirectSession.request", + side_effect=OSError("refused"), + ): + with pytest.raises(ProviderRequestError) as raised: + provider_request( + "DJ", + "GET", + "https://user:pass@dj.internal.example.com:8443/x?token=t", + timeout=1, + ) + assert "dj.internal" not in str(raised.value) + assert "8443" not in str(raised.value) + assert "dj.internal" not in caplog.text + assert "unreachable" in caplog.text + assert "pass" not in caplog.text + assert "token" not in caplog.text + + +@pytest.mark.parametrize("allowed", [False, True]) +def test_failure_logs_never_carry_url_secrets( + caplog: pytest.LogCaptureFixture, allowed: bool +) -> None: + listener: socket.socket = socket.socket() + listener.bind(("127.0.0.1", 0)) + port: int = listener.getsockname()[1] + listener.close() # Nothing listens: the connection is refused or blocked. + url: str = f"https://user:SECRET_PW@127.0.0.1:{port}/x?token=SECRET_TOKEN" + with _internal_hosts(allowed), caplog.at_level("WARNING"): + with pytest.raises(ProviderRequestError): + provider_request("Cube", "GET", url, timeout=2) + assert caplog.records + assert "SECRET" not in caplog.text + assert "unreachable" in caplog.text or "refused" in caplog.text + + +@pytest.mark.parametrize( + "location", + [ + "https://public.example.test:NOT_A_PORT/", + "https://[public.example.test/", + ], +) +def test_malformed_redirects_raise_the_safe_error( + public_tls: tuple[ssl.SSLContext, list[str]], + caplog: pytest.LogCaptureFixture, + location: str, +) -> None: + routes: dict[str, Route] = {"/a": (302, {"Location": location}, b"")} + with _server(routes, public_tls[0]) as (port, seen), caplog.at_level("WARNING"): + with pytest.raises(ProviderRequestError) as raised: + provider_request( + "Cube", "GET", f"https://public.example.test:{port}/a", timeout=2 + ) + assert "NOT_A_PORT" not in str(raised.value) + assert "NOT_A_PORT" not in caplog.text + assert len(seen) == 1 + + +@pytest.mark.parametrize( + "url", ["https://[dj.example.com/x", "https://dj.example.com:NOT_A_PORT/x"] +) +def test_malformed_endpoint_urls_raise_the_safe_error(url: str) -> None: + with patch("superset.semantic_layers.transport._DirectSession.request") as send: + _background_request() + with pytest.raises(ProviderRequestError) as raised: + provider_request("DJ", "GET", url, timeout=1) + send.assert_not_called() + assert "NOT_A_PORT" not in str(raised.value) + + +# --------------------------------------------------------------------------- # +# Bounded bodies: the same checks, plus a cumulative deadline and byte cap +# --------------------------------------------------------------------------- # +@contextmanager +def _dripping_server( + context: ssl.SSLContext, size: int, interval: float, stall_after: int = -1 +) -> Iterator[int]: + """Serve ``size`` bytes one at a time, ``interval`` seconds apart. + + After ``stall_after`` bytes, stop sending for longer than any test waits. + """ + + class Drip(BaseHTTPRequestHandler): + def do_POST(self) -> None: # noqa: N802 + self.rfile.read(int(self.headers.get("Content-Length", 0))) + self.send_response(200) + self.send_header("Content-Length", str(size)) + self.end_headers() + try: + for sent in range(size): + if sent == stall_after: + time.sleep(5) + self.wfile.write(b"x") + self.wfile.flush() + time.sleep(interval) + except OSError: + pass # The client gave up, as intended. + + def log_message(self, *args: Any) -> None: + pass + + server: HTTPServer = HTTPServer(("127.0.0.1", 0), Drip) + server.socket = context.wrap_socket(server.socket, server_side=True) + thread: Thread = Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield server.server_address[1] + finally: + server.shutdown() + server.server_close() + + +def _body( + url: str, *, seconds: float = 2.0, max_bytes: int = 4096, **kwargs: Any +) -> bytes: + return provider_body( + "MetricFlow", + "POST", + url, + deadline=time.monotonic() + seconds, + max_bytes=max_bytes, + timeout=(5.0, 30.0), + json={"query": "{ ok }"}, + **kwargs, + ) + + +def test_bounded_body_is_read_through_the_checked_connection( + public_tls: tuple[ssl.SSLContext, list[str]], +) -> None: + payload: bytes = b'{"data": {"ok": true}}' + consumed: list[int] = [] + with _server({"/q": (200, {}, payload)}, public_tls[0]) as (port, seen): + body: bytes = _body( + f"https://public.example.test:{port}/q", consume=consumed.append + ) + assert body == payload + assert sum(consumed) == len(payload) + # One lookup of the hostname; the connection then uses the checked address. + assert public_tls[1].count("public.example.test") == 1 + assert seen == [f"POST /q public.example.test:{port}"] + + +def test_bounded_body_is_refused_past_its_byte_cap( + public_tls: tuple[ssl.SSLContext, list[str]], +) -> None: + consumed: list[int] = [] + with _server({"/q": (200, {}, b"x" * 3000)}, public_tls[0]) as (port, _): + with pytest.raises(ProviderRequestError, match="byte budget"): + _body( + f"https://public.example.test:{port}/q", + max_bytes=1024, + consume=consumed.append, + ) + assert 1024 < sum(consumed) <= 3000 + + +def test_bounded_body_stops_at_the_deadline_while_dripping( + public_tls: tuple[ssl.SSLContext, list[str]], +) -> None: + started: float = time.monotonic() + with _dripping_server(public_tls[0], size=200, interval=0.05) as port: + with pytest.raises(ProviderRequestError) as raised: + _body(f"https://public.example.test:{port}/q", seconds=0.5) + assert raised.value.failure == ProviderRequestFailure.TIMEOUT + elapsed: float = time.monotonic() - started # Before server shutdown. + # Per-read timeouts alone (30 s) would let the drip run for ten seconds. + assert elapsed < 1.5 + + +def test_bounded_read_after_progress_waits_only_for_the_time_left( + public_tls: tuple[ssl.SSLContext, list[str]], +) -> None: + # Bytes arrive for ~0.8 s, then stop. The first read's timeout was the whole + # 1.2 s budget, so only re-arming it per read ends this near the deadline. + started: float = time.monotonic() + with _dripping_server(public_tls[0], 100, 0.05, stall_after=16) as port: + with pytest.raises(ProviderRequestError, match="time budget") as raised: + _body(f"https://public.example.test:{port}/q", seconds=1.2) + assert raised.value.failure == ProviderRequestFailure.TIMEOUT + elapsed: float = time.monotonic() - started # Before server shutdown. + assert elapsed < 1.7 + + +@pytest.mark.parametrize("allow_internal", [False, True]) +def test_bounded_lookup_waits_no_longer_than_the_deadline( + monkeypatch: pytest.MonkeyPatch, allow_internal: bool +) -> None: + def stuck(*args: Any, **kwargs: Any) -> Any: + time.sleep(1.5) + raise OSError("simulated stuck system resolver") + + real_resolve: Any = socket.getaddrinfo + + def scoped_resolve(host: str, *args: Any, **kwargs: Any) -> Any: + if host == "slow-dns.example.test": + return stuck(host, *args, **kwargs) + return real_resolve(host, *args, **kwargs) + + monkeypatch.setattr(network.socket, "getaddrinfo", scoped_resolve) + started: float = time.monotonic() + with ( + _internal_hosts(allow_internal), + pytest.raises(ProviderRequestError, match="time budget") as raised, + ): + _body("https://slow-dns.example.test/q", seconds=0.3) + assert raised.value.failure == ProviderRequestFailure.TIMEOUT + assert time.monotonic() - started < 1.0 + + +def test_bounded_body_refuses_internal_destinations() -> None: + answers: list[Any] = [(2, 1, 6, "", ("169.254.169.254", 443))] + with ( + _dns_answers("metadata.example.test", answers), + patch("superset.utils.network._connect_address") as connect, + ): + _background_connection() + with pytest.raises(ProviderRequestError, match="not allowed"): + _body("https://metadata.example.test/q") + connect.assert_not_called() + + +def test_bounded_error_bodies_are_not_read_or_logged( + public_tls: tuple[ssl.SSLContext, list[str]], caplog: pytest.LogCaptureFixture +) -> None: + routes: dict[str, Route] = {"/q": (500, {}, b"internal detail")} + with _server(routes, public_tls[0]) as (port, _), caplog.at_level("WARNING"): + with pytest.raises(ProviderRequestError) as raised: + _body(f"https://public.example.test:{port}/q") + assert str(raised.value) == "Provider request failed with HTTP 500." + assert "internal detail" not in caplog.text + + +def test_bounded_body_refuses_encoded_responses( + public_tls: tuple[ssl.SSLContext, list[str]], +) -> None: + routes: dict[str, Route] = {"/q": (200, {"Content-Encoding": "gzip"}, b"\x1f\x8b")} + with _server(routes, public_tls[0]) as (port, _): + with pytest.raises(ProviderRequestError, match="encoding is not supported"): + _body(f"https://public.example.test:{port}/q") + + +def test_bounded_post_redirects_are_not_followed( + public_tls: tuple[ssl.SSLContext, list[str]], +) -> None: + routes: dict[str, Route] = {"/q": (307, {"Location": "/elsewhere"}, b"")} + with _server(routes, public_tls[0]) as (port, seen): + with pytest.raises(ProviderRequestError, match="HTTP 307"): + _body(f"https://public.example.test:{port}/q") + assert [entry.split()[1] for entry in seen] == ["/q"] + + +def test_bounded_body_refuses_an_expired_deadline() -> None: + with patch("superset.semantic_layers.transport._DirectSession.request") as send: + _background_request() + with pytest.raises(ProviderRequestError, match="time budget") as raised: + _body("https://public.example.test/q", seconds=-1) + assert raised.value.failure == ProviderRequestFailure.TIMEOUT + send.assert_not_called() + + +@pytest.mark.parametrize("size", [0, 2 * 1024 * 1024]) +def test_bounded_redirect_bodies_are_never_read( + public_tls: tuple[ssl.SSLContext, list[str]], size: int +) -> None: + # Requests pre-reads a redirect's body even with allow_redirects=False. + observed: list[int] = [] + original: Any = vars(requests.Response)["content"].fget + + def content(response: requests.Response) -> bytes: + body: bytes = original(response) + # Response.content is process-global too; ignore other requests. + if response.url == url: + observed.append(len(body)) + return body + + routes: dict[str, Route] = {"/q": (307, {"Location": "/other"}, b"x" * size)} + with _server(routes, public_tls[0]) as (port, seen): + url: str = f"https://public.example.test:{port}/q" + with patch.object(requests.Response, "content", property(content)): + unrelated: requests.Response = requests.Response() + unrelated.url = "https://unrelated.example.test/" + unrelated._content = b"unrelated" + assert unrelated.content == b"unrelated" + assert observed == [] + owned: requests.Response = requests.Response() + owned.url = url + owned._content = b"owned" + assert owned.content == b"owned" + assert observed == [len(b"owned")] + observed.clear() + with pytest.raises(ProviderRequestError, match="HTTP 307"): + _body(url, max_bytes=1024) + assert len(seen) == 1 + assert observed == [] + + +@contextmanager +def _framing_server(context: ssl.SSLContext, mode: str) -> Iterator[int]: + """Drip HTTP framing bytes (a header or chunk trailer), not body bytes.""" + + class Handler(BaseHTTPRequestHandler): + def do_POST(self) -> None: # noqa: N802 + self.rfile.read(int(self.headers.get("Content-Length", 0))) + prefix: bytes = { + "headers": b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\nX-Test: ", + "chunk-trailers": b"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked" + b"\r\n\r\n0\r\nX-Trailer: ", + # No length or chunking: the body ends when the connection does. + "close-delimited": b"HTTP/1.0 200 OK\r\n\r\n", + }[mode] + try: + self.wfile.write(prefix) + self.wfile.flush() + for _ in range(14): + self.wfile.write(b"x") + self.wfile.flush() + time.sleep(0.04) + self.wfile.write(b"\r\n\r\n") + self.wfile.flush() + except OSError: + pass + + def log_message(self, *args: Any) -> None: + pass + + server: HTTPServer = HTTPServer(("127.0.0.1", 0), Handler) + server.socket = context.wrap_socket(server.socket, server_side=True) + thread: Thread = Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield server.server_address[1] + finally: + server.shutdown() + server.server_close() + thread.join() + + +@pytest.mark.parametrize("mode", ["headers", "chunk-trailers", "close-delimited"]) +@pytest.mark.parametrize("allow_internal", [False, True]) +def test_bounded_framing_obeys_the_absolute_deadline( + public_tls: tuple[ssl.SSLContext, list[str]], mode: str, allow_internal: bool +) -> None: + # Allowing internal hosts relaxes destinations only, never the deadline. + with _framing_server(public_tls[0], mode) as port, _internal_hosts(allow_internal): + started: float = time.monotonic() + with pytest.raises(ProviderRequestError) as raised: + _body(f"https://public.example.test:{port}/q", seconds=0.15) + assert raised.value.failure == ProviderRequestFailure.TIMEOUT + elapsed: float = time.monotonic() - started + assert elapsed < 0.40, (mode, allow_internal, elapsed) + + +def test_bounded_lookup_and_connect_attempts_share_the_deadline( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Two public answers that both stall on connect, after a slow lookup. + answers: list[Any] = [ + (2, 1, 6, "", ("192.0.2.10", 443)), + (2, 1, 6, "", ("192.0.2.11", 443)), + ] + + def slow_lookup(*args: Any, **kwargs: Any) -> list[Any]: + time.sleep(0.2) + return answers + + waits: list[float] = [] + + def stalled_connect(address: Any, timeout: Any = None, **kwargs: Any) -> Any: + waits.append(float(timeout)) + time.sleep(float(timeout)) + raise socket.timeout("timed out") + + real_resolve: Any = socket.getaddrinfo + + def scoped_resolve(host: str, *args: Any, **kwargs: Any) -> Any: + if host == "stalled.example.test": + return slow_lookup(host, *args, **kwargs) + return real_resolve(host, *args, **kwargs) + + monkeypatch.setattr(network.socket, "getaddrinfo", scoped_resolve) + monkeypatch.setattr(network, "is_safe_ip", lambda address: True) + started: float = time.monotonic() + with patch("superset.utils.network._connect_address", side_effect=stalled_connect): + with pytest.raises(ProviderRequestError) as raised: + _body("https://stalled.example.test/q", seconds=0.5) + assert raised.value.failure == ProviderRequestFailure.TIMEOUT + elapsed: float = time.monotonic() - started + # Each attempt waits only for what is left; separate full waits take 1.2 s. + assert elapsed < 0.75, (elapsed, waits) + + +def test_bounded_internal_requests_ignore_environment_proxies( + public_tls: tuple[ssl.SSLContext, list[str]], monkeypatch: pytest.MonkeyPatch +) -> None: + """A proxy would own the socket, so a bounded request connects directly.""" + monkeypatch.setenv("HTTPS_PROXY", "http://127.0.0.1:9") + monkeypatch.delenv("NO_PROXY", raising=False) + monkeypatch.delenv("no_proxy", raising=False) + payload: bytes = b'{"data": {"ok": true}}' + with ( + _server({"/q": (200, {}, payload)}, public_tls[0]) as ( + port, + seen, + ), + _internal_hosts(True), + ): + assert _body(f"https://public.example.test:{port}/q") == payload + assert seen == [f"POST /q public.example.test:{port}"] + + +def test_former_unbounded_internal_requests_ignore_environment_proxies( + public_tls: tuple[ssl.SSLContext, list[str]], monkeypatch: pytest.MonkeyPatch +) -> None: + """The SDK has no unbounded mode; Q7 requires direct connections for all calls.""" + monkeypatch.setenv("HTTPS_PROXY", "http://127.0.0.1:9") + monkeypatch.delenv("NO_PROXY", raising=False) + monkeypatch.delenv("no_proxy", raising=False) + with ( + _server({"/q": (200, {}, b"{}")}, public_tls[0]) as (port, seen), + _internal_hosts(True), + ): + assert ( + provider_request( + "MetricFlow", + "GET", + f"https://public.example.test:{port}/q", + timeout=2.0, + ).json() + == {} + ) + assert seen == [f"GET /q public.example.test:{port}"] + + +@pytest.mark.parametrize("allow_internal", [False, True]) +@pytest.mark.parametrize("bounded", [False, True]) +@pytest.mark.parametrize("failure", ["lookup", "empty"]) +def test_dns_failure_is_not_an_address_policy_refusal( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, + allow_internal: bool, + failure: str, + bounded: bool, +) -> None: + def resolve(*args: Any, **kwargs: Any) -> list[Any]: + if failure == "lookup": + raise socket.gaierror(socket.EAI_NONAME, "private DNS detail") + return [] + + real_resolve: Any = socket.getaddrinfo + + def scoped_resolve(host: str, *args: Any, **kwargs: Any) -> Any: + if host == "missing.example.test": + return resolve(host, *args, **kwargs) + return real_resolve(host, *args, **kwargs) + + monkeypatch.setattr(network.socket, "getaddrinfo", scoped_resolve) + monkeypatch.setenv("NO_PROXY", "*") + monkeypatch.setenv("no_proxy", "*") + + def call() -> None: + if bounded: + _body("https://missing.example.test/q") + else: + provider_request( + "MetricFlow", "POST", "https://missing.example.test/q", timeout=1 + ) + + with _internal_hosts(allow_internal), pytest.raises(ProviderRequestError) as raised: + call() + assert str(raised.value) == "Provider endpoint could not be reached." + assert "unreachable" in caplog.text + assert "refused" not in caplog.text + assert "private DNS detail" not in caplog.text + + +@pytest.mark.parametrize( + "size,cap,success", + [(16, 16, True), (17, 16, False), (65537, 65537, True), (65537, 65536, False)], +) +def test_byte_cap_boundaries_and_chunk_accounting( + public_tls: tuple[ssl.SSLContext, list[str]], size: int, cap: int, success: bool +) -> None: + consumed: list[int] = [] + with _server({"/q": (200, {}, b"x" * size)}, public_tls[0]) as (port, _): + if success: + assert ( + len( + _body( + f"https://public.example.test:{port}/q", + max_bytes=cap, + consume=consumed.append, + ) + ) + == size + ) + else: + with pytest.raises(ProviderRequestError) as raised: + _body( + f"https://public.example.test:{port}/q", + max_bytes=cap, + consume=consumed.append, + ) + assert raised.value.failure == ProviderRequestFailure.TOO_LARGE + assert sum(consumed) == size + assert max(consumed) <= 65536 + + +def test_error_body_is_unread( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + from unittest.mock import MagicMock + + response: MagicMock = MagicMock( + status_code=503, url="https://fixture.test/secret", headers={} + ) + response.__enter__.return_value = response + monkeypatch.setattr( + semantic_layer_http._DirectSession, "request", lambda *args, **kwargs: response + ) + with pytest.raises(ProviderRequestError) as raised: + provider_request("UNTRUSTED_LABEL", "GET", response.url, timeout=1) + assert raised.value.failure == ProviderRequestFailure.HTTP_STATUS + response.raw.read1.assert_not_called() + assert "UNTRUSTED_LABEL" not in caplog.text + assert "secret" not in caplog.text + assert "category=http_status status=503" in caplog.text + + +def test_tls_handshake_stall_is_bounded() -> None: + from threading import Event + + done: Event = Event() + listener: socket.socket = socket.socket() + listener.bind(("127.0.0.1", 0)) + listener.listen() + + def serve() -> None: + connection: socket.socket + connection, _ = listener.accept() + with connection: + done.wait(2) + + worker: Thread = Thread(target=serve, daemon=True) + worker.start() + try: + with _internal_hosts(True): + start: float = time.monotonic() + with pytest.raises(ProviderRequestError) as raised: + provider_request( + "fixture", + "GET", + f"https://127.0.0.1:{listener.getsockname()[1]}/", + timeout=0.15, + ) + assert raised.value.failure == ProviderRequestFailure.TIMEOUT + assert time.monotonic() - start < 1.15 + finally: + done.set() + listener.close() + worker.join(2) + + +def test_connect_boundary_stall_uses_real_socket_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Block real socket I/O at connect without a platform-specific SYN blackhole.""" + left: socket.socket + right: socket.socket + left, right = socket.socketpair() + + class ConnectingSocket: + def settimeout(self, seconds: float) -> None: + left.settimeout(seconds) + + def connect(self, address: tuple[str, int]) -> None: + left.recv(1) # Real blocking I/O, governed by production's connect timeout. + + def close(self) -> None: + left.close() + + monkeypatch.setattr( + network.socket, "socket", lambda *args, **kwargs: ConnectingSocket() + ) + try: + with _internal_hosts(True): + start: float = time.monotonic() + with pytest.raises(ProviderRequestError) as raised: + provider_request( + "fixture", "GET", "http://127.0.0.1:12345/", timeout=0.15 + ) + assert raised.value.failure == ProviderRequestFailure.TIMEOUT + assert time.monotonic() - start < 1.15 + finally: + left.close() + right.close() + + +def test_ten_concurrent_deadlines_release_descriptors() -> None: + from http.server import ThreadingHTTPServer + from threading import Event + + import psutil + + done: Event = Event() + + class Stall(BaseHTTPRequestHandler): + def do_GET(self) -> None: # noqa: N802 + self.send_response(200) + self.send_header("Content-Length", "10") + self.end_headers() + self.wfile.write(b"x") + self.wfile.flush() + done.wait(2) + + def log_message(self, *args: Any) -> None: + pass + + before: int = psutil.Process().num_fds() + + class ConcurrentServer(ThreadingHTTPServer): + request_queue_size: int = 32 + + server: ThreadingHTTPServer = ConcurrentServer(("127.0.0.1", 0), Stall) + worker: Thread = Thread( + target=lambda: server.serve_forever(poll_interval=0.02), daemon=True + ) + worker.start() + + def call(index: int) -> tuple[float, float]: + budget: float = 0.15 + index * 0.02 + start: float = time.monotonic() + transport: semantic_layer_http.HostProviderTransport = ( + semantic_layer_http.HostProviderTransport(allow_internal_hosts=True) + ) + with pytest.raises(ProviderRequestError) as raised: + transport.request( + "fixture", + "GET", + f"http://127.0.0.1:{server.server_port}/", + budget=RequestBudget(start + budget, 100), + ) + assert raised.value.failure == ProviderRequestFailure.TIMEOUT + return time.monotonic() - start, budget + + try: + with ThreadPoolExecutor(10) as pool: + results: list[tuple[float, float]] = list(pool.map(call, range(10))) + assert all(elapsed < budget + 1 for elapsed, budget in results) + finally: + done.set() + server.shutdown() + server.server_close() + worker.join(2) + deadline: float = time.monotonic() + 1 + while psutil.Process().num_fds() > before and time.monotonic() < deadline: + time.sleep(0.01) + assert psutil.Process().num_fds() <= before + assert not semantic_layer_http._scheduler()._tokens + + +def test_scheduler_registration_after_expiry_and_close_are_safe() -> None: + scheduler: semantic_layer_http.DeadlineScheduler = semantic_layer_http._scheduler() + bound: semantic_layer_http._Deadline = semantic_layer_http._Deadline( + time.monotonic() - 1 + ) + left: socket.socket + right: socket.socket + left, right = socket.socketpair() + try: + scheduler.register(bound, left) + right.settimeout(0.2) + assert right.recv(1) == b"" + scheduler.close(bound) + scheduler.close(bound) + assert not bound.sockets + finally: + left.close() + right.close() + + +def test_policy_is_default_off_and_isolated() -> None: + from superset import config + + assert config.SEMANTIC_LAYER_ALLOW_INTERNAL_HOSTS is False + app: Flask = Flask(__name__) + app.config.update( + SEMANTIC_LAYER_ALLOW_INTERNAL_HOSTS=True, + DATABASE_OAUTH2_ALLOW_INTERNAL_HOSTS=False, + ALERT_REPORTS_WEBHOOK_ALLOW_INTERNAL_HOSTS=False, + ) + with app.app_context(): + transport: ProviderTransport = semantic_layer_http.build_provider_transport() + assert app.config["DATABASE_OAUTH2_ALLOW_INTERNAL_HOSTS"] is False + assert app.config["ALERT_REPORTS_WEBHOOK_ALLOW_INTERNAL_HOSTS"] is False + with _server() as (port, _): + url: str = f"http://127.0.0.1:{port}/" + assert ( + transport.request( + "fixture", "GET", url, budget=RequestBudget(time.monotonic() + 1, 100) + ).status + == 200 + ) + with pytest.raises(ProviderRequestError): + semantic_layer_http.build_provider_transport().request( + "fixture", "GET", url, budget=RequestBudget(time.monotonic() + 1, 100) + ) + + +def test_consumer_failure_is_not_mislabeled_as_invalid_url( + public_tls: tuple[ssl.SSLContext, list[str]], +) -> None: + def consume(size: int) -> None: + raise ValueError("caller budget exhausted") + + with _server(context=public_tls[0]) as (port, _): + with pytest.raises(ValueError, match="caller budget exhausted"): + _body(f"https://public.example.test:{port}/", consume=consume) + + +def test_cross_scheme_redirect_returns_status_without_reading_body( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from unittest.mock import MagicMock + + response: MagicMock = MagicMock( + status_code=302, + url="https://fixture.test/", + headers={"Location": "http://fixture.test/"}, + ) + response.__enter__.return_value = response + monkeypatch.setattr( + semantic_layer_http._DirectSession, "request", lambda *args, **kwargs: response + ) + with pytest.raises(ProviderRequestError) as raised: + provider_request("fixture", "GET", response.url, timeout=1) + assert raised.value.failure == ProviderRequestFailure.HTTP_STATUS + assert raised.value.status == 302 + response.raw.read1.assert_not_called() + + +def test_explicit_zero_port_is_not_the_default_origin( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from unittest.mock import MagicMock + + response: MagicMock = MagicMock( + status_code=302, + url="https://fixture.test:0/", + headers={"Location": "https://fixture.test:443/"}, + ) + response.__enter__.return_value = response + send: MagicMock = MagicMock(return_value=response) + monkeypatch.setattr(semantic_layer_http._DirectSession, "request", send) + with pytest.raises(ProviderRequestError) as raised: + provider_request("fixture", "GET", response.url, timeout=1) + assert raised.value.failure == ProviderRequestFailure.HTTP_STATUS + send.assert_called_once() + response.raw.read1.assert_not_called() + + +def test_completed_body_does_not_require_a_second_full_copy() -> None: + """Keep an 8 MiB body to one buffer, allowing allocator slack and one chunk.""" + import tracemalloc + from unittest.mock import MagicMock + + size: int = 8 * 1024 * 1024 + chunk: bytes = b"x" * 65536 + response: MagicMock = MagicMock() + response.raw.read1.side_effect = [chunk] * (size // len(chunk)) + [b""] + bound: semantic_layer_http._Deadline = semantic_layer_http._Deadline( + time.monotonic() + 2 + ) + tracemalloc.start() + try: + body: bytes = semantic_layer_http.HostProviderTransport._read_body( + response, bound, size, None + ) + peak: int = tracemalloc.get_traced_memory()[1] + finally: + tracemalloc.stop() + assert len(body) == size + assert peak < size * 1.25 + len(chunk) diff --git a/tests/unit_tests/utils/test_network.py b/tests/unit_tests/utils/test_network.py index b20fa2f015d0..aa49fa0e9faa 100644 --- a/tests/unit_tests/utils/test_network.py +++ b/tests/unit_tests/utils/test_network.py @@ -270,3 +270,96 @@ def test_public_embedded_address_does_not_allow_6to4() -> None: assert address.sixtofour == ipaddress.IPv4Address("8.8.8.8") assert is_safe_ip(ipaddress.IPv4Address("8.8.8.8")) assert not is_safe_ip(address) + + +def test_public_resolution_deduplicates_and_rejects_any_unsafe_answer() -> None: + from superset.utils.network import NonPublicAddressError, resolve_public_addresses + + answers: list[tuple[int, int, int, str, tuple[str, int]]] = [ + (2, 1, 6, "", ("8.8.8.8", 443)) + ] * 2 + with patch("superset.utils.network.socket.getaddrinfo", return_value=answers): + assert resolve_public_addresses("fixture.test", 443, timeout=1) == ["8.8.8.8"] + answers.append((2, 1, 6, "", ("127.0.0.1", 443))) + with ( + patch("superset.utils.network.socket.getaddrinfo", return_value=answers), + pytest.raises(NonPublicAddressError), + ): + resolve_public_addresses("fixture.test", 443, timeout=1) + + +def test_pinned_connection_does_not_repeat_dns_lookup() -> None: + from contextvars import Token + + from superset.utils.network import ( + _PinnedHTTPSConnection, + ConnectionDeadline, + PROVIDER_DEADLINE, + ) + + bound: MagicMock = MagicMock() + bound.remaining.return_value = 1 + connection: _PinnedHTTPSConnection = _PinnedHTTPSConnection("fixture.test", 443) + assert connection._dns_host == "fixture.test" + assert callable(connection._new_conn) + with ( + patch( + "superset.utils.network.socket.getaddrinfo", + side_effect=[ + [(2, 1, 6, "", ("8.8.8.8", 443))], + [(2, 1, 6, "", ("127.0.0.1", 443))], + ], + ) as resolve, + patch("superset.utils.network._connect_address") as connect, + ): + token: Token[ConnectionDeadline | None] = PROVIDER_DEADLINE.set(bound) + try: + assert connection._new_conn() is connect.return_value + finally: + PROVIDER_DEADLINE.reset(token) + resolve.assert_called_once() + connect.assert_called_once_with(("8.8.8.8", 443), 1) + bound.watch.assert_called_once_with(connect.return_value) + assert ( + connection.host == "fixture.test" + ) # Original hostname remains available to TLS. + + +def test_failed_socket_registration_closes_connected_socket() -> None: + from contextvars import Token + + from urllib3.exceptions import NewConnectionError + + from superset.utils.network import ( + _PinnedHTTPSConnection, + ConnectionDeadline, + PROVIDER_DEADLINE, + ) + + bound: MagicMock = MagicMock() + bound.remaining.return_value = 1 + bound.watch.side_effect = OSError("descriptor limit") + connection: _PinnedHTTPSConnection = _PinnedHTTPSConnection("fixture.test", 443) + with ( + patch("superset.utils.network._resolve_addresses", return_value=["8.8.8.8"]), + patch("superset.utils.network._connect_address") as connect, + ): + token: Token[ConnectionDeadline | None] = PROVIDER_DEADLINE.set(bound) + try: + with pytest.raises(NewConnectionError): + connection._new_conn() + finally: + PROVIDER_DEADLINE.reset(token) + connect.return_value.close.assert_called_once() + + +def test_empty_resolution_fails_without_connecting() -> None: + from superset.utils.network import resolve_public_addresses + + with ( + patch("superset.utils.network.socket.getaddrinfo", return_value=[]), + patch("superset.utils.network._connect_address") as connect, + ): + with pytest.raises(OSError, match="No destination address"): + resolve_public_addresses("fixture.test", 443, timeout=1) + connect.assert_not_called() From e1b31671d5641037d79c3c244a1700c019f741e6 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 11:38:31 -0600 Subject: [PATCH 03/23] feat(semantic-layer): inject provider transport into host entry points --- UPDATING.md | 11 ++ .../extensions/contribution-types.md | 20 ++- .../extensions/semantic-execution-context.md | 8 + .../extensions/semantic-provider-transport.md | 145 +++++++++++++++ superset/semantic_layers/api.py | 12 +- superset/semantic_layers/execution.py | 2 + superset/semantic_layers/masking.py | 5 +- .../superset_e2e_semantic_stub/__init__.py | 6 + tests/unit_tests/semantic_layers/api_test.py | 46 +++-- tests/unit_tests/semantic_layers/conftest.py | 10 +- .../e2e_stub_extension_test.py | 7 +- .../semantic_layers/masking_test.py | 17 +- .../semantic_layers/metadata_contract_test.py | 3 + .../unit_tests/semantic_layers/models_test.py | 3 +- .../semantic_layers/provider_masking_test.py | 2 +- .../simple_cache_clear_test.py | 1 + .../semantic_layers/transport_wiring_test.py | 166 ++++++++++++++++++ 17 files changed, 433 insertions(+), 31 deletions(-) create mode 100644 docs/developer_docs/extensions/semantic-provider-transport.md create mode 100644 tests/unit_tests/semantic_layers/transport_wiring_test.py diff --git a/UPDATING.md b/UPDATING.md index 591f79f35033..9995402fb552 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,17 @@ assists people when migrating to a new version. ## Next +- **Semantic-layer SDK transport argument.** Provider implementations must accept + the required keyword-only `transport: ProviderTransport` in `from_configuration`, + `get_configuration_schema`, and `get_runtime_schema`. Superset supplies it even + for static schema discovery. Store it on the layer and pass it to views and HTTP + helpers; do not construct HTTP clients for provider endpoints. Every request now + requires an absolute monotonic deadline and a response byte cap. The default + host policy allows public HTTPS destinations only; the provider-specific + `SEMANTIC_LAYER_ALLOW_INTERNAL_HOSTS = True` setting also allows HTTP and internal + destinations. Both policies ignore environment proxies and preserve budgets and + TLS hostname verification. See the [provider transport guide](docs/developer_docs/extensions/semantic-provider-transport.md). + - **Pivot Table mixed-metric export totals.** CSV/Excel exports and scheduled CSV/text reports leave Total and Subtotal cells blank when they combine different metrics, including two SUM metrics. Metric-specific values and totals remain diff --git a/docs/developer_docs/extensions/contribution-types.md b/docs/developer_docs/extensions/contribution-types.md index a7cfa97a4d8f..ecc61b86e6d6 100644 --- a/docs/developer_docs/extensions/contribution-types.md +++ b/docs/developer_docs/extensions/contribution-types.md @@ -248,7 +248,7 @@ See [MCP Integration](./mcp.md) for implementation details. See [cache invalidation](../semantic-layer-cache-clear.md) for the required cache-token factory argument and provider cache requirements. -See the [execution context contract](semantic-execution-context.md) for provider construction and connection-mode guarantees. +See the [execution context contract](semantic-execution-context.md) for provider construction and connection-mode guarantees, and the [provider transport contract](semantic-provider-transport.md) for bounded HTTP requests. Extensions can register custom semantic layer implementations that allow Superset to connect to external data modeling frameworks. Each semantic layer defines how to authenticate, discover semantic views (tables/metrics/dimensions), and execute queries against the external system. @@ -258,6 +258,7 @@ from typing import Any from superset_core.semantic_layers.context import SemanticCredentials, SemanticExecutionContext from superset_core.semantic_layers.decorators import semantic_layer from superset_core.semantic_layers.layer import SemanticLayer +from superset_core.semantic_layers.transport import ProviderTransport from my_extension.config import MyConfig from my_extension.view import MySemanticView @@ -273,26 +274,33 @@ class MySemanticLayer(SemanticLayer[MyConfig, MySemanticView]): def __init__( self, configuration: MyConfig, *, cache_token: str, - context: SemanticExecutionContext, + context: SemanticExecutionContext, transport: ProviderTransport, ) -> None: self.configuration: MyConfig = configuration self.cache_token: str = cache_token self.context: SemanticExecutionContext = context + self.transport: ProviderTransport = transport @classmethod def from_configuration( cls, configuration: dict[str, Any], *, cache_token: str, - context: SemanticExecutionContext, credentials: SemanticCredentials | None = None, + context: SemanticExecutionContext, transport: ProviderTransport, + credentials: SemanticCredentials | None = None, ) -> "MySemanticLayer": config: MyConfig = MyConfig.model_validate(configuration) - return cls(config, cache_token=cache_token, context=context) + return cls(config, cache_token=cache_token, context=context, transport=transport) @classmethod - def get_configuration_schema(cls, configuration=None) -> dict: + def get_configuration_schema( + cls, configuration: MyConfig | None = None, *, transport: ProviderTransport, + ) -> dict[str, Any]: return MyConfig.model_json_schema() @classmethod - def get_runtime_schema(cls, configuration=None, runtime_data=None) -> dict: + def get_runtime_schema( + cls, configuration: MyConfig, runtime_data: dict[str, Any] | None = None, + *, transport: ProviderTransport, + ) -> dict[str, Any]: return {"type": "object", "properties": {}} def get_semantic_views(self, runtime_configuration: dict) -> set[MySemanticView]: diff --git a/docs/developer_docs/extensions/semantic-execution-context.md b/docs/developer_docs/extensions/semantic-execution-context.md index 7318419988f3..e87f1f6fc9e5 100644 --- a/docs/developer_docs/extensions/semantic-execution-context.md +++ b/docs/developer_docs/extensions/semantic-execution-context.md @@ -100,3 +100,11 @@ the host must add async resolution pinning, effective-identity keys and refusal checks at every cache boundary, containment identity migration, namespace startup validation, and TTL enforcement. The reserved credentials channel and origin enum do not enable those methods by themselves. + +## HTTP transport + +Provider construction and both schema-discovery entry points also receive a required +keyword-only `transport: ProviderTransport`. It is a separate host capability, not +a field of the execution context or a component of binding identity. See the +[provider transport guide](semantic-provider-transport.md) for budgets, destination +policy, errors, and a test fake. diff --git a/docs/developer_docs/extensions/semantic-provider-transport.md b/docs/developer_docs/extensions/semantic-provider-transport.md new file mode 100644 index 000000000000..3213bb7ffd8f --- /dev/null +++ b/docs/developer_docs/extensions/semantic-provider-transport.md @@ -0,0 +1,145 @@ +--- +title: Semantic provider HTTP transport +--- + + + +# Provider HTTP transport + +Superset supplies a `ProviderTransport` to semantic-layer providers. This keeps +HTTP policy in the host and lets an extension use the same SDK on different hosts. +Providers import only `superset_core` for this capability, never host HTTP helpers. + +## Accept the capability + +The following `SemanticLayer` entry points require keyword-only `transport=`: + +- `from_configuration(configuration, *, cache_token, context, transport, credentials=None)` +- `get_configuration_schema(configuration=None, *, transport)` +- `get_runtime_schema(configuration, runtime_data=None, *, transport)` + +Keep it on the constructed layer and pass it to views and discovery helpers. +It is separate from `SemanticExecutionContext` and does not change memo or cache +identity. Static schema discovery still receives the capability; providers must +not perform remote discovery when no validated configuration was supplied. + +## Set budgets for a logical operation + +```python +import time + +from superset_core.semantic_layers.transport import ( + ProviderResponse, ProviderTransport, RequestBudget, +) + + +def fetch_catalog(transport: ProviderTransport, endpoint: str) -> ProviderResponse: + budget: RequestBudget = RequestBudget( + deadline=time.monotonic() + 10.0, max_bytes=4 * 1024 * 1024, + ) + return transport.request("Example", "GET", endpoint, budget=budget) +``` + +`deadline` is an absolute monotonic time on this process, not a wall-clock timestamp. +Reuse that deadline across pages, redirects and polling attempts. Each call's +`max_bytes` limits its response body. An optional `consume(size)` callback receives +chunk sizes before the per-call cap check, so a provider can account for a shared +operation budget. Use a provider-specific exception when that shared budget is +exhausted. + +DNS, connect, TLS, headers and body reads all share the deadline. A timed-out or +truncated response is never returned as success. Resolution uses at most eight +shared workers; an OS resolver already running cannot be cancelled, but the caller +stops waiting at its deadline. Connected sockets are interrupted by one shared, +lazily started scheduler. Neither component starts at module import. + +## Responses and errors + +`ProviderResponse` owns complete bounded body bytes, case-insensitive immutable +headers, the status, and the final URL. Its `json()` uses the standard library; +JSON decoding errors remain provider errors, not transport errors. + +The host returns only 2xx responses. `ProviderRequestError.failure` identifies +`INVALID_URL`, `REFUSED`, `UNREACHABLE`, `TIMEOUT`, `TOO_LARGE`, +`UNSUPPORTED_ENCODING`, or `HTTP_STATUS`. For `HTTP_STATUS`, `status` contains the +status code. These are transport faults, not deliberate query refusals. Never +log URLs, credentials, query values or response payloads when handling them. +User-facing errors are generic; the provider label is not echoed into errors or logs. + +The transport sends `Accept-Encoding: identity` and refuses other response +encodings. Non-2xx bodies are not read. Logs contain only status and failure +category. GET may follow at most three redirects within the same scheme, hostname +and effective port. Other redirects fail with `HTTP_STATUS` without reading their +bodies. Redirects do not reset the deadline. No automatic retries are made. + +## Destination policy + +By default, only HTTPS endpoints whose DNS answers are all public are allowed. +The host applies its shared `is_safe_ip` policy, including special IPv6 ranges. +It connects directly to a checked numeric address without resolving again, while +preserving the original hostname for TLS SNI and certificate verification. + +Operators can set `SEMANTIC_LAYER_ALLOW_INTERNAL_HOSTS = True` in +`superset_config.py` to allow private addresses and HTTP for these providers. +This setting does not change OAuth or webhook policy. It never disables budgets, +TLS verification or redirect restrictions. Both settings ignore environment proxy +variables. Configure a reachable provider endpoint rather than relying on a proxy. +There is no per-user opt-out; trusted operators own the setting. + +## Use a fake in provider tests + +A fake must honor the same 2xx-or-error contract. A minimal fixed-response fake: + +```python +import time +from collections.abc import Callable, Mapping +from typing import Any + +from superset_core.semantic_layers.transport import ( + ProviderRequestError, ProviderRequestFailure, ProviderResponse, + ProviderTransport, RequestBudget, +) + + +class FakeTransport(ProviderTransport): + def __init__(self) -> None: + self.calls: list[tuple[str, str]] = [] + + def request( + self, provider: str, method: str, url: str, *, budget: RequestBudget, + headers: Mapping[str, str] | None = None, + params: Mapping[str, Any] | None = None, json: Any = None, + consume: Callable[[int], None] | None = None, + ) -> ProviderResponse: + if time.monotonic() >= budget.deadline: + raise ProviderRequestError(ProviderRequestFailure.TIMEOUT) + self.calls.append((method, url)) + body: bytes = b'{"metrics": []}' + if consume is not None: + consume(len(body)) + if len(body) > budget.max_bytes: + raise ProviderRequestError(ProviderRequestFailure.TOO_LARGE) + if time.monotonic() >= budget.deadline: + raise ProviderRequestError(ProviderRequestFailure.TIMEOUT) + return ProviderResponse(200, {"Content-Type": "application/json"}, body, url) +``` + +Add fixture-specific failures by raising `ProviderRequestError`, rather than +returning a non-2xx `ProviderResponse`. Keep credentials out of recorded calls. diff --git a/superset/semantic_layers/api.py b/superset/semantic_layers/api.py index 791cf3289636..041d4d54aeec 100644 --- a/superset/semantic_layers/api.py +++ b/superset/semantic_layers/api.py @@ -75,6 +75,7 @@ SemanticViewPostSchema, SemanticViewPutSchema, ) +from superset.semantic_layers.transport import build_provider_transport from superset.superset_typing import FlaskResponse from superset.utils import json from superset.views.base_api import ( @@ -763,7 +764,9 @@ def configuration_schema(self) -> FlaskResponse: warning: str | None = None try: - schema = cls.get_configuration_schema(parsed_config) + schema: dict[str, Any] = cls.get_configuration_schema( + parsed_config, transport=build_provider_transport() + ) except Exception: # pylint: disable=broad-except # Show a stable, user-friendly message in the UI; the exception # detail goes to the server log below. @@ -779,7 +782,9 @@ def configuration_schema(self) -> FlaskResponse: ) # Connection or query failures during schema enrichment should not # prevent the form from rendering — return the base schema instead. - schema = cls.get_configuration_schema(None) + schema = cls.get_configuration_schema( + None, transport=build_provider_transport() + ) payload: dict[str, Any] = {"result": schema} if warning: @@ -840,9 +845,10 @@ def runtime_schema(self, uuid: str) -> FlaskResponse: return self.response_400(message=f"Unknown type: {layer.type}") try: - schema = cls.get_runtime_schema( + schema: dict[str, Any] = cls.get_runtime_schema( layer.implementation.configuration, # type: ignore[attr-defined] runtime_data, + transport=build_provider_transport(), ) except SupersetSecurityException as ex: return self.response(403, message=ex.message, errors=[ex.to_dict()]) diff --git a/superset/semantic_layers/execution.py b/superset/semantic_layers/execution.py index 3d30610bfb25..7b6462ddcbc7 100644 --- a/superset/semantic_layers/execution.py +++ b/superset/semantic_layers/execution.py @@ -50,6 +50,7 @@ SEMANTIC_INSTALL_ID_KEY, ) from superset.semantic_layers.registry import registry +from superset.semantic_layers.transport import build_provider_transport from superset.utils import json if TYPE_CHECKING: @@ -280,6 +281,7 @@ def bind( cache_token=layer.metadata_generation, context=context, credentials=None, + transport=build_provider_transport(), ), {}, ) diff --git a/superset/semantic_layers/masking.py b/superset/semantic_layers/masking.py index b0c07f30ea6c..7280a59f5c45 100644 --- a/superset/semantic_layers/masking.py +++ b/superset/semantic_layers/masking.py @@ -67,6 +67,7 @@ from superset.constants import PASSWORD_MASK from superset.semantic_layers.registry import registry +from superset.semantic_layers.transport import build_provider_transport logger: logging.Logger = logging.getLogger(__name__) @@ -361,7 +362,9 @@ def mask_configuration(layer_type: str, configuration: Any) -> dict[str, Any]: # The connector's own published shape (the same source #43474's # top-level masker used); this walker extends it to nested/union # secrets rather than only top-level ``writeOnly`` properties. - schema: JsonSchema = cls.get_configuration_schema() + schema: JsonSchema = cls.get_configuration_schema( + transport=build_provider_transport() + ) except Exception: # pylint: disable=broad-except logger.warning( "Masking semantic layer type %s: schema generation failed", layer_type diff --git a/tests/e2e_extensions/semantic_stub/dist/backend/src/superset_e2e_semantic_stub/__init__.py b/tests/e2e_extensions/semantic_stub/dist/backend/src/superset_e2e_semantic_stub/__init__.py index 96effc5a8a15..b07bb2a556f5 100644 --- a/tests/e2e_extensions/semantic_stub/dist/backend/src/superset_e2e_semantic_stub/__init__.py +++ b/tests/e2e_extensions/semantic_stub/dist/backend/src/superset_e2e_semantic_stub/__init__.py @@ -46,6 +46,7 @@ SemanticQueryRejectedError, ) from superset_core.semantic_layers.layer import SemanticLayer +from superset_core.semantic_layers.transport import ProviderTransport from superset_core.semantic_layers.types import ( AggregationType, Dimension, @@ -249,6 +250,7 @@ def from_configuration( *, cache_token: str, context: SemanticExecutionContext, + transport: ProviderTransport, credentials: SemanticCredentials | None = None, ) -> StubSemanticLayer: instance: StubSemanticLayer = cls( @@ -263,6 +265,8 @@ def from_configuration( def get_configuration_schema( cls, configuration: StubConfiguration | None = None, + *, + transport: ProviderTransport, ) -> dict[str, Any]: return StubConfiguration.model_json_schema() @@ -271,6 +275,8 @@ def get_runtime_schema( cls, configuration: StubConfiguration, runtime_data: dict[str, Any] | None = None, + *, + transport: ProviderTransport, ) -> dict[str, Any]: return {"type": "object", "properties": {}} diff --git a/tests/unit_tests/semantic_layers/api_test.py b/tests/unit_tests/semantic_layers/api_test.py index f86058071d76..2bc9c32d9312 100644 --- a/tests/unit_tests/semantic_layers/api_test.py +++ b/tests/unit_tests/semantic_layers/api_test.py @@ -19,7 +19,7 @@ import logging import uuid as uuid_lib from typing import Annotated, Any, Literal -from unittest.mock import MagicMock, patch +from unittest.mock import ANY, MagicMock, patch import pytest from flask.testing import FlaskClient @@ -362,7 +362,7 @@ def test_configuration_schema( assert response.status_code == 200 assert response.json["result"] == {"type": "object"} - mock_cls.get_configuration_schema.assert_called_once_with(None) + mock_cls.get_configuration_schema.assert_called_once_with(None, transport=ANY) @SEMANTIC_LAYERS_APP @@ -401,7 +401,9 @@ def test_configuration_schema_with_partial_config( ) assert response.status_code == 200 - mock_cls.get_configuration_schema.assert_called_once_with(mock_config_obj) + mock_cls.get_configuration_schema.assert_called_once_with( + mock_config_obj, transport=ANY + ) @SEMANTIC_LAYERS_APP @@ -433,7 +435,7 @@ def test_configuration_schema_read_only_user_gets_static_schema( assert response.json["result"] == {"type": "object"} can_access.assert_any_call("can_write", "SemanticLayer") mock_cls.configuration_class.model_validate.assert_not_called() - mock_cls.get_configuration_schema.assert_called_once_with(None) + mock_cls.get_configuration_schema.assert_called_once_with(None, transport=ANY) @SEMANTIC_LAYERS_APP @@ -470,7 +472,9 @@ def test_configuration_schema_writer_gets_enriched_schema( assert response.status_code == 200 assert response.json["result"]["properties"] == {"account": {"enum": ["a1"]}} can_access.assert_any_call("can_write", "SemanticLayer") - mock_cls.get_configuration_schema.assert_called_once_with(mock_config_obj) + mock_cls.get_configuration_schema.assert_called_once_with( + mock_config_obj, transport=ANY + ) @SEMANTIC_LAYERS_APP @@ -507,7 +511,7 @@ def test_configuration_schema_with_invalid_partial_config( ) assert response.status_code == 200 - mock_cls.get_configuration_schema.assert_called_once_with(None) + mock_cls.get_configuration_schema.assert_called_once_with(None, transport=ANY) @SEMANTIC_LAYERS_APP @@ -563,7 +567,7 @@ def test_runtime_schema( assert response.status_code == 200 assert response.json["result"] == {"type": "object"} mock_cls.get_runtime_schema.assert_called_once_with( - {"account": "test"}, {"database": "mydb"} + {"account": "test"}, {"database": "mydb"}, transport=ANY ) @@ -596,7 +600,9 @@ def test_runtime_schema_no_body( ) assert response.status_code == 200 - mock_cls.get_runtime_schema.assert_called_once_with({"account": "test"}, None) + mock_cls.get_runtime_schema.assert_called_once_with( + {"account": "test"}, None, transport=ANY + ) @SEMANTIC_LAYERS_APP @@ -1117,7 +1123,11 @@ def test_get_list_semantic_layers( mocker.patch.dict( "superset.semantic_layers.api.registry", - {"snowflake": MagicMock(get_configuration_schema=lambda: {"properties": {}})}, + { + "snowflake": MagicMock( + get_configuration_schema=lambda *, transport: {"properties": {}} + ) + }, clear=True, ) mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO") @@ -1182,7 +1192,11 @@ def test_get_semantic_layer( mocker.patch.dict( "superset.semantic_layers.api.registry", - {"snowflake": MagicMock(get_configuration_schema=lambda: {"properties": {}})}, + { + "snowflake": MagicMock( + get_configuration_schema=lambda *, transport: {"properties": {}} + ) + }, clear=True, ) mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO") @@ -1272,7 +1286,11 @@ def test_serialize_layer_string_config( mocker.patch.dict( "superset.semantic_layers.api.registry", - {"snowflake": MagicMock(get_configuration_schema=lambda: {"properties": {}})}, + { + "snowflake": MagicMock( + get_configuration_schema=lambda *, transport: {"properties": {}} + ) + }, clear=True, ) mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO") @@ -1302,7 +1320,11 @@ def test_serialize_layer_dict_config( mocker.patch.dict( "superset.semantic_layers.api.registry", - {"snowflake": MagicMock(get_configuration_schema=lambda: {"properties": {}})}, + { + "snowflake": MagicMock( + get_configuration_schema=lambda *, transport: {"properties": {}} + ) + }, clear=True, ) mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO") diff --git a/tests/unit_tests/semantic_layers/conftest.py b/tests/unit_tests/semantic_layers/conftest.py index 1a55caea3c38..c62d25b1179e 100644 --- a/tests/unit_tests/semantic_layers/conftest.py +++ b/tests/unit_tests/semantic_layers/conftest.py @@ -30,6 +30,7 @@ SemanticExecutionContext, ) from superset_core.semantic_layers.layer import SemanticLayer +from superset_core.semantic_layers.transport import ProviderTransport from superset_core.semantic_layers.types import ( Dimension, Filter, @@ -153,6 +154,7 @@ def from_configuration( *, cache_token: str, context: SemanticExecutionContext, + transport: ProviderTransport, credentials: SemanticCredentials | None = None, ) -> LegacyLayer: layer: LegacyLayer = cls() @@ -163,13 +165,17 @@ def from_configuration( @classmethod def get_configuration_schema( - cls, configuration: BaseModel | None = None + cls, configuration: BaseModel | None = None, *, transport: ProviderTransport ) -> dict[str, Any]: return {"type": "object"} @classmethod def get_runtime_schema( - cls, configuration: BaseModel, runtime_data: dict[str, Any] | None = None + cls, + configuration: BaseModel, + runtime_data: dict[str, Any] | None = None, + *, + transport: ProviderTransport, ) -> dict[str, Any]: return {"type": "object"} diff --git a/tests/unit_tests/semantic_layers/e2e_stub_extension_test.py b/tests/unit_tests/semantic_layers/e2e_stub_extension_test.py index 2b50b8e470d0..5d4f3c8a50c6 100644 --- a/tests/unit_tests/semantic_layers/e2e_stub_extension_test.py +++ b/tests/unit_tests/semantic_layers/e2e_stub_extension_test.py @@ -51,6 +51,7 @@ SemanticView, ) from superset.semantic_layers.registry import registry +from superset.semantic_layers.transport import build_provider_transport from tests.unit_tests.semantic_layers.conftest import make_execution_context STUB_PATH: Path = ( @@ -84,6 +85,7 @@ def stub_view( {}, cache_token="test", # noqa: S106 context=make_execution_context(), + transport=build_provider_transport(), ).get_semantic_view("orders", {}) model: SemanticView = SemanticView( id=91, @@ -124,13 +126,14 @@ def test_stub_registers_through_the_extension_loader( stub_layer: type[SemanticLayer[Any, Any]], ) -> None: """The stub carries the extension-prefixed type that seeding code uses.""" - assert stub_layer.name == "E2E Semantic Stub" + assert vars(stub_layer)["name"] == "E2E Semantic Stub" assert { view.name for view in stub_layer.from_configuration( {}, cache_token="test", # noqa: S106 context=make_execution_context(), + transport=build_provider_transport(), ).get_semantic_views({}) } == {"orders"} @@ -145,12 +148,14 @@ def test_stub_rejects_configuration_and_unknown_views( {"account": "unexpected"}, cache_token="test", # noqa: S106 context=context, + transport=build_provider_transport(), ) with pytest.raises(ValueError, match="no semantic view"): stub_layer.from_configuration( {}, cache_token="test", # noqa: S106 context=context, + transport=build_provider_transport(), ).get_semantic_view("missing", {}) diff --git a/tests/unit_tests/semantic_layers/masking_test.py b/tests/unit_tests/semantic_layers/masking_test.py index 1cdf82e0191d..2ed4977ebd41 100644 --- a/tests/unit_tests/semantic_layers/masking_test.py +++ b/tests/unit_tests/semantic_layers/masking_test.py @@ -25,6 +25,7 @@ import pytest from pydantic import BaseModel, Field, SecretStr, StringConstraints from pytest_mock import MockerFixture +from superset_core.semantic_layers.transport import ProviderTransport from superset.constants import PASSWORD_MASK from superset.semantic_layers import masking @@ -58,7 +59,9 @@ class DemoConfig(BaseModel): class DemoLayerType: @staticmethod - def get_configuration_schema(configuration: Any = None) -> Any: + def get_configuration_schema( + configuration: Any = None, *, transport: ProviderTransport + ) -> Any: return DemoConfig.model_json_schema() @@ -199,7 +202,9 @@ class DivergentConfig(BaseModel): class DivergentType: @staticmethod - def get_configuration_schema(configuration: Any = None) -> Any: + def get_configuration_schema( + configuration: Any = None, *, transport: ProviderTransport + ) -> Any: return DivergentConfig.model_json_schema() value: dict[str, Any] = { @@ -325,7 +330,9 @@ def __init__(self, schema: Any = None, raises: bool = False) -> None: self._schema: Any = schema self._raises: bool = raises - def get_configuration_schema(self, configuration: Any = None) -> Any: + def get_configuration_schema( + self, configuration: Any = None, *, transport: ProviderTransport + ) -> Any: if self._raises: raise RuntimeError("schema generation failed") return self._schema @@ -404,7 +411,9 @@ class DivergentListConfig(BaseModel): class DivergentListType: @staticmethod - def get_configuration_schema(configuration: Any = None) -> Any: + def get_configuration_schema( + configuration: Any = None, *, transport: ProviderTransport + ) -> Any: return DivergentListConfig.model_json_schema() value: dict[str, Any] = {"conn": {"kind": "plain", "items": ["x", "y"]}} diff --git a/tests/unit_tests/semantic_layers/metadata_contract_test.py b/tests/unit_tests/semantic_layers/metadata_contract_test.py index 45a51dfbb29b..cfe2996911a1 100644 --- a/tests/unit_tests/semantic_layers/metadata_contract_test.py +++ b/tests/unit_tests/semantic_layers/metadata_contract_test.py @@ -26,6 +26,7 @@ SemanticQuery, ) +from superset.semantic_layers.transport import build_provider_transport from tests.unit_tests.semantic_layers.conftest import ( LegacyConfiguration, LegacyLayer, @@ -39,6 +40,7 @@ def test_legacy_provider_needs_no_metadata_overrides() -> None: {}, cache_token="test", # noqa: S106 context=make_execution_context(), + transport=build_provider_transport(), ) view: LegacyView = layer.get_semantic_view("legacy", {}) assert view.metadata_cache_token is None @@ -108,6 +110,7 @@ def test_factory_accepts_required_host_identity() -> None: {}, cache_token="host-generation", # noqa: S106 context=make_execution_context(), + transport=build_provider_transport(), ) ) assert isinstance(layer, LegacyLayer) diff --git a/tests/unit_tests/semantic_layers/models_test.py b/tests/unit_tests/semantic_layers/models_test.py index d2d5240b0858..bb705fc66ad3 100644 --- a/tests/unit_tests/semantic_layers/models_test.py +++ b/tests/unit_tests/semantic_layers/models_test.py @@ -26,7 +26,7 @@ from itertools import permutations from pathlib import Path from typing import Any, cast -from unittest.mock import MagicMock, patch, PropertyMock +from unittest.mock import ANY, MagicMock, patch, PropertyMock import pyarrow as pa import pytest @@ -382,6 +382,7 @@ def test_semantic_layer_implementation(mocker: MockerFixture) -> None: cache_token=layer.metadata_generation, context=current_host_execution_context(), credentials=None, + transport=ANY, ) diff --git a/tests/unit_tests/semantic_layers/provider_masking_test.py b/tests/unit_tests/semantic_layers/provider_masking_test.py index bad78b4c692b..319da21b68c8 100644 --- a/tests/unit_tests/semantic_layers/provider_masking_test.py +++ b/tests/unit_tests/semantic_layers/provider_masking_test.py @@ -76,7 +76,7 @@ def test_provider_nested_secrets_masked_on_read( layer.changed_on_delta_humanized.return_value = "" mocker.patch.dict( "superset.semantic_layers.registry.registry", - {"snowflake": MagicMock(get_configuration_schema=lambda: schema)}, + {"snowflake": MagicMock(get_configuration_schema=lambda *, transport: schema)}, clear=True, ) mocker.patch( diff --git a/tests/unit_tests/semantic_layers/simple_cache_clear_test.py b/tests/unit_tests/semantic_layers/simple_cache_clear_test.py index 59a4002ef4c1..58eebf041e27 100644 --- a/tests/unit_tests/semantic_layers/simple_cache_clear_test.py +++ b/tests/unit_tests/semantic_layers/simple_cache_clear_test.py @@ -156,6 +156,7 @@ def test_generation_binding_precedes_provider_construction( cache_token=layer.metadata_generation, context=ANY, credentials=None, + transport=ANY, ) view: SemanticView = SemanticView(name="view", semantic_layer=layer) assert view.metadata_generation == layer.metadata_generation diff --git a/tests/unit_tests/semantic_layers/transport_wiring_test.py b/tests/unit_tests/semantic_layers/transport_wiring_test.py new file mode 100644 index 000000000000..69540c906d38 --- /dev/null +++ b/tests/unit_tests/semantic_layers/transport_wiring_test.py @@ -0,0 +1,166 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Every host construction/discovery boundary supplies the same SDK capability.""" + +from typing import Any +from unittest.mock import MagicMock, PropertyMock + +import pytest +from pytest_mock import MockerFixture +from superset_core.semantic_layers.context import ExecutionMethod +from superset_core.semantic_layers.transport import ProviderTransport +from werkzeug.test import TestResponse + +from superset.semantic_layers.execution import SemanticBindingMemo +from superset.semantic_layers.masking import mask_configuration +from superset.semantic_layers.models import SemanticLayer +from tests.unit_tests.semantic_layers.conftest import make_execution_context + + +@pytest.mark.parametrize( + "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True +) +@pytest.mark.parametrize("fallback", [False, True]) +@pytest.mark.parametrize("writer", [False, True]) +def test_configuration_paths_inject_transport_preserving_static_gate( + client: Any, + full_api_access: None, + mocker: MockerFixture, + fallback: bool, + writer: bool, +) -> None: + capability: MagicMock = MagicMock(spec=ProviderTransport) + factory: MagicMock = mocker.patch( + "superset.semantic_layers.api.build_provider_transport", + create=True, + return_value=capability, + ) + provider: MagicMock = MagicMock() + provider.configuration_class.model_json_schema.return_value = {"properties": {}} + provider.get_configuration_schema.side_effect = ( + [OSError("safe fake failure"), {"type": "object"}] + if fallback + else [{"type": "object"}] + ) + mocker.patch.dict("superset.semantic_layers.api.registry", {"test": provider}) + mocker.patch( + "superset.semantic_layers.api.security_manager.can_access", + side_effect=lambda permission, view: permission != "can_write" or writer, + ) + response: TestResponse = client.post( + "/api/v1/semantic_layer/schema/configuration", + json={"type": "test", "configuration": {"account": "fixture"}}, + ) + assert response.status_code == 200 + assert provider.get_configuration_schema.call_count == (2 if fallback else 1) + for call in provider.get_configuration_schema.call_args_list: + assert call.kwargs == {"transport": capability} + assert factory.call_count == provider.get_configuration_schema.call_count + if not writer: + provider.configuration_class.model_validate.assert_not_called() + assert all( + call.args == (None,) + for call in provider.get_configuration_schema.call_args_list + ) + else: + assert ( + provider.get_configuration_schema.call_args_list[0].args[0] + is provider.configuration_class.model_validate.return_value + ) + if fallback: + assert provider.get_configuration_schema.call_args.args == (None,) + + +@pytest.mark.parametrize( + "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True +) +def test_runtime_schema_injects_transport_after_access( + client: Any, full_api_access: None, mocker: MockerFixture +) -> None: + capability: MagicMock = MagicMock(spec=ProviderTransport) + factory: MagicMock = mocker.patch( + "superset.semantic_layers.api.build_provider_transport", + create=True, + return_value=capability, + ) + layer: MagicMock = MagicMock(type="test") + provider: MagicMock = MagicMock() + provider.get_runtime_schema.return_value = {"type": "object"} + mocker.patch( + "superset.semantic_layers.api.SemanticLayerDAO.find_by_uuid", return_value=layer + ) + mocker.patch.dict("superset.semantic_layers.api.registry", {"test": provider}) + response: TestResponse = client.post( + "/api/v1/semantic_layer/00000000-0000-0000-0000-000000000001/schema/runtime", + json={"runtime_data": {}}, + ) + assert response.status_code == 200 + layer.raise_for_access.assert_called_once() + provider.get_runtime_schema.assert_called_once_with( + layer.implementation.configuration, {}, transport=capability + ) + factory.assert_called_once_with() + + +def test_masking_static_schema_receives_transport(mocker: MockerFixture) -> None: + capability: MagicMock = MagicMock(spec=ProviderTransport) + factory: MagicMock = mocker.patch( + "superset.semantic_layers.masking.build_provider_transport", + create=True, + return_value=capability, + ) + provider: MagicMock = MagicMock() + provider.get_configuration_schema.return_value = { + "type": "object", + "properties": {"plain": {"type": "string"}}, + } + mocker.patch.dict("superset.semantic_layers.masking.registry", {"test": provider}) + assert mask_configuration("test", {"plain": "value"}) == {"plain": "value"} + provider.get_configuration_schema.assert_called_once_with(transport=capability) + factory.assert_called_once_with() + + +def test_binding_injects_transport_only_on_memo_miss(mocker: MockerFixture) -> None: + capability: MagicMock = MagicMock(spec=ProviderTransport) + factory: MagicMock = mocker.patch( + "superset.semantic_layers.execution.build_provider_transport", + create=True, + return_value=capability, + ) + mocker.patch( + "superset.semantic_layers.execution.current_host_execution_context", + return_value=make_execution_context(), + ) + mocker.patch( + "superset.semantic_layers.execution.check_execution_allowed", + return_value=ExecutionMethod.CONNECTION, + ) + mocker.patch.object( + SemanticLayer, + "metadata_generation", + new_callable=PropertyMock, + return_value="generation", + ) + provider: MagicMock = MagicMock() + mocker.patch.dict("superset.semantic_layers.execution.registry", {"test": provider}) + layer: SemanticLayer = SemanticLayer(type="test", configuration="{}") + memo: SemanticBindingMemo = SemanticBindingMemo() + assert memo.bind(layer) is memo.bind(layer) + factory.assert_called_once_with() + provider.from_configuration.assert_called_once() + assert provider.from_configuration.call_args.kwargs["transport"] is capability From 5f2d2785ec146e77f3960e9749cacd9d1b23164b Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 12:14:56 -0600 Subject: [PATCH 04/23] fix(semantic-layer): address provider transport review findings --- .../extensions/semantic-provider-transport.md | 11 +- .../semantic_layers/transport.py | 35 ++-- .../tests/semantic_layers/test_transport.py | 33 +++- superset/semantic_layers/transport.py | 39 +++- superset/utils/network.py | 23 ++- .../semantic_layers/transport_test.py | 170 ++++++++++++++++-- tests/unit_tests/utils/test_network.py | 37 +++- 7 files changed, 307 insertions(+), 41 deletions(-) diff --git a/docs/developer_docs/extensions/semantic-provider-transport.md b/docs/developer_docs/extensions/semantic-provider-transport.md index 3213bb7ffd8f..6aa15c93ae91 100644 --- a/docs/developer_docs/extensions/semantic-provider-transport.md +++ b/docs/developer_docs/extensions/semantic-provider-transport.md @@ -81,10 +81,14 @@ The host returns only 2xx responses. `ProviderRequestError.failure` identifies `UNSUPPORTED_ENCODING`, or `HTTP_STATUS`. For `HTTP_STATUS`, `status` contains the status code. These are transport faults, not deliberate query refusals. Never log URLs, credentials, query values or response payloads when handling them. -User-facing errors are generic; the provider label is not echoed into errors or logs. +Errors and logs include the provider display name. Pass a constant label such as +`MetricFlow` or `dbt Semantic Layer`, never endpoint or configuration values. +Labels must start with an ASCII letter or digit and contain only ASCII letters, +digits, spaces, underscores or hyphens, up to 64 characters. Other labels fall +back to `Provider`; valid labels have trailing spaces removed. The transport sends `Accept-Encoding: identity` and refuses other response -encodings. Non-2xx bodies are not read. Logs contain only status and failure +encodings. Non-2xx bodies are not read. Logs contain only the sanitized provider name, status and failure category. GET may follow at most three redirects within the same scheme, hostname and effective port. Other redirects fail with `HTTP_STATUS` without reading their bodies. Redirects do not reset the deadline. No automatic retries are made. @@ -101,6 +105,9 @@ Operators can set `SEMANTIC_LAYER_ALLOW_INTERNAL_HOSTS = True` in This setting does not change OAuth or webhook policy. It never disables budgets, TLS verification or redirect restrictions. Both settings ignore environment proxy variables. Configure a reachable provider endpoint rather than relying on a proxy. +In allow-internal mode, `REQUESTS_CA_BUNDLE` (or `CURL_CA_BUNDLE` as fallback) +can supply the trusted CA bundle for private TLS endpoints. Certificate and +hostname verification remain enabled. The default policy ignores both variables. There is no per-user opt-out; trusted operators own the setting. ## Use a fake in provider tests diff --git a/superset-core/src/superset_core/semantic_layers/transport.py b/superset-core/src/superset_core/semantic_layers/transport.py index 7d413dfcee37..c15110e0c71c 100644 --- a/superset-core/src/superset_core/semantic_layers/transport.py +++ b/superset-core/src/superset_core/semantic_layers/transport.py @@ -21,6 +21,7 @@ import json # noqa: TID251 -- SDK cannot depend on the host JSON utility. import math +import re from abc import ABC, abstractmethod from collections.abc import Callable, Iterator, Mapping from dataclasses import dataclass @@ -98,33 +99,37 @@ class ProviderRequestError(Exception): """A safe failure containing no endpoint, credentials or response text.""" def __init__( - self, failure: ProviderRequestFailure, *, status: int | None = None + self, + failure: ProviderRequestFailure, + *, + status: int | None = None, + provider: str = "Provider", ) -> None: self.failure: ProviderRequestFailure = failure self.status: int | None = status + # Reject URL-like, control-bearing or excessive labels rather than echoing them. + self.provider: str = ( + provider.strip() + if re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9 _-]{0,63}", provider) + else "Provider" + ) super().__init__(failure) def __str__(self) -> str: messages: dict[ProviderRequestFailure, str] = { - ProviderRequestFailure.INVALID_URL: "Provider endpoint is not a valid URL.", - ProviderRequestFailure.REFUSED: "Provider endpoint is not allowed.", - ProviderRequestFailure.UNREACHABLE: ( - "Provider endpoint could not be reached." - ), - ProviderRequestFailure.TIMEOUT: ( - "Provider request exceeded its time budget." - ), - ProviderRequestFailure.TOO_LARGE: ( - "Provider response exceeded its byte budget." - ), + ProviderRequestFailure.INVALID_URL: "endpoint is not a valid URL.", + ProviderRequestFailure.REFUSED: "endpoint is not allowed.", + ProviderRequestFailure.UNREACHABLE: ("endpoint could not be reached."), + ProviderRequestFailure.TIMEOUT: ("request exceeded its time budget."), + ProviderRequestFailure.TOO_LARGE: ("response exceeded its byte budget."), ProviderRequestFailure.UNSUPPORTED_ENCODING: ( - "Provider response encoding is not supported." + "response encoding is not supported." ), ProviderRequestFailure.HTTP_STATUS: ( - f"Provider request failed with HTTP {self.status}." + f"request failed with HTTP {self.status}." ), } - return messages[self.failure] + return f"{self.provider} {messages[self.failure]}" class ProviderTransport(ABC): diff --git a/superset-core/tests/semantic_layers/test_transport.py b/superset-core/tests/semantic_layers/test_transport.py index 26369c8fab97..1f6b84127d4f 100644 --- a/superset-core/tests/semantic_layers/test_transport.py +++ b/superset-core/tests/semantic_layers/test_transport.py @@ -130,9 +130,40 @@ def test_transport_failure_round_trip_preserves_category_and_status( failure: ProviderRequestFailure = ProviderRequestFailure[failure_name] error: ProviderRequestError = ProviderRequestError( - failure, status=503 if failure == ProviderRequestFailure.HTTP_STATUS else None + failure, + status=503 if failure == ProviderRequestFailure.HTTP_STATUS else None, + provider="MetricFlow", ) restored: ProviderRequestError = pickle.loads(pickle.dumps(error)) # noqa: S301 -- locally constructed exception only assert restored.failure is failure assert restored.status == error.status + assert restored.provider == "MetricFlow" assert str(restored) == str(error) + + +@pytest.mark.parametrize( + "provider,expected", + [ + ("dbt Semantic Layer", "dbt Semantic Layer"), + ("Cube_2-beta", "Cube_2-beta"), + ("Cube\r\nforged", "Provider"), + ("https://user:secret@example.test/path?token=value", "Provider"), + ("x" * 65, "Provider"), + ("", "Provider"), + (" ", "Provider"), + ("Cube\x1b[31m", "Provider"), + ], +) +def test_provider_error_uses_only_a_bounded_display_name( + provider: str, expected: str +) -> None: + from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure, + ) + + error: ProviderRequestError = ProviderRequestError( + ProviderRequestFailure.HTTP_STATUS, status=503, provider=provider + ) + assert error.provider == expected + assert str(error) == f"{expected} request failed with HTTP 503." diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index f8b4210205f4..7c811b92fe38 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -21,6 +21,7 @@ import heapq import logging +import os import socket import time from collections.abc import Callable, Mapping @@ -28,7 +29,7 @@ from dataclasses import dataclass, field from io import BytesIO from threading import Condition, Lock, Thread -from typing import Any +from typing import Any, TYPE_CHECKING from urllib.parse import SplitResult, urljoin, urlsplit import requests @@ -49,6 +50,9 @@ PROVIDER_DEADLINE, ) +if TYPE_CHECKING: + from requests.sessions import _Settings + logger: logging.Logger = logging.getLogger(__name__) MAX_REDIRECTS: int = 3 _CHUNK_BYTES: int = 65536 @@ -78,7 +82,7 @@ def request( bound.remaining() # No parsing, resolution or socket work after expiry. self._validated_url(url) with _DirectSession() as session: - session.trust_env = False + session.trust_env = self._allow_internal adapter: PinnedHTTPAdapter = PinnedHTTPAdapter( public_only=not self._allow_internal ) @@ -128,9 +132,13 @@ def request( OSError, ProviderRequestError, ) as exc: - error: ProviderRequestError = self._failure_for(exc, budget.deadline) + failure: ProviderRequestError = self._failure_for(exc, budget.deadline) + error: ProviderRequestError = ProviderRequestError( + failure.failure, status=failure.status, provider=provider + ) logger.warning( - "Provider request failed: category=%s status=%s", + "%s request failed: category=%s status=%s", + error.provider, error.failure.value, error.status, ) @@ -239,6 +247,12 @@ def build_provider_transport() -> ProviderTransport: class _DirectSession(requests.Session): + def merge_environment_settings(self, *args: Any, **kwargs: Any) -> _Settings: + # Retain internal-mode CA bundles, but the deadline must own the direct socket. + settings: _Settings = super().merge_environment_settings(*args, **kwargs) + settings["proxies"] = {} + return settings + def get_redirect_target(self, resp: requests.Response) -> None: # Requests otherwise reads redirect bodies despite allow_redirects=False. return None @@ -297,9 +311,6 @@ def close(self, bound: _Deadline) -> None: for sock in bound.sockets: sock.close() bound.sockets.clear() - # Completed long-deadline requests must not accumulate tombstones. - self._heap = [item for item in self._heap if item[2] in self._tokens] - heapq.heapify(self._heap) self._condition.notify() def drain(self) -> None: @@ -327,6 +338,9 @@ def _run(self) -> None: deadline: float bound: _Deadline deadline, _, bound = self._heap[0] + if bound.closed: + heapq.heappop(self._heap) + continue remaining: float = deadline - time.monotonic() if remaining > 0: self._condition.wait(remaining) @@ -345,3 +359,14 @@ def _scheduler() -> DeadlineScheduler: if _SCHEDULER is None: _SCHEDULER = DeadlineScheduler() return _SCHEDULER + + +def _reset_scheduler_after_fork() -> None: + """A child cannot reuse the parent's worker or potentially held lock.""" + global _SCHEDULER, _SCHEDULER_LOCK # pylint: disable=global-statement + _SCHEDULER = None + _SCHEDULER_LOCK = Lock() + + +if hasattr(os, "register_at_fork"): + os.register_at_fork(after_in_child=_reset_scheduler_after_fork) diff --git a/superset/utils/network.py b/superset/utils/network.py index ab57dd878e0e..0760193c87cf 100644 --- a/superset/utils/network.py +++ b/superset/utils/network.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. import ipaddress +import os import platform import socket import subprocess @@ -316,7 +317,11 @@ def resolve_public_addresses(host: str, port: int, *, timeout: float) -> list[st return _resolve_addresses(host, port, timeout, public_only=True) -def _connect_address(address: tuple[str, int], timeout: float) -> socket.socket: +def _connect_address( + address: tuple[str, int], + timeout: float, + socket_options: list[tuple[int, int, int | bytes]] | None, +) -> socket.socket: """Connect to a numeric address without invoking a second resolver.""" family: socket.AddressFamily = ( socket.AF_INET6 @@ -326,6 +331,9 @@ def _connect_address(address: tuple[str, int], timeout: float) -> socket.socket: sock: socket.socket = socket.socket(family, socket.SOCK_STREAM) try: sock.settimeout(timeout) + option: tuple[int, int, int | bytes] + for option in socket_options or []: + sock.setsockopt(*option) sock.connect(address) return sock except BaseException: @@ -349,7 +357,7 @@ def _new_conn(self) -> socket.socket: for address in addresses: try: sock: socket.socket = _connect_address( - (address, self.port), bound.remaining() + (address, self.port), bound.remaining(), self.socket_options ) try: bound.watch(sock) # Duplicate before TLS wrapping. @@ -404,3 +412,14 @@ def init_poolmanager(self, *args: Any, **kwargs: Any) -> None: "http": _PinnedHTTPPool if self.public_only else _InternalHTTPPool, "https": _PinnedHTTPSPool if self.public_only else _InternalHTTPSPool, } + + +def _reset_resolver_after_fork() -> None: + """Discard executor state and locks whose owning threads do not survive fork.""" + global _RESOLVER, _RESOLVER_LOCK # pylint: disable=global-statement + _RESOLVER = None + _RESOLVER_LOCK = Lock() + + +if hasattr(os, "register_at_fork"): + os.register_at_fork(after_in_child=_reset_resolver_after_fork) diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index e2e77584808d..cbd7b071201d 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -20,8 +20,12 @@ from __future__ import annotations import ipaddress +import os import socket import ssl +import subprocess +import sys +import textwrap import time from collections.abc import Iterator from concurrent.futures import ThreadPoolExecutor @@ -425,13 +429,13 @@ def test_flag_off_keeps_local_endpoints_refused() -> None: with pytest.raises(ProviderRequestError) as raised: provider_request("Cube", "GET", f"https://localhost:{port}/m", timeout=2) assert seen == [] - assert str(raised.value) == ("Provider endpoint is not allowed.") + assert str(raised.value) == ("Cube endpoint is not allowed.") def test_post_redirects_are_not_followed() -> None: routes: dict[str, Route] = {"/graphql": (307, {"Location": "/other"}, b"")} with _server(routes) as (port, seen), _internal_hosts(True): - with pytest.raises(ProviderRequestError, match=r"^Provider request failed"): + with pytest.raises(ProviderRequestError, match=r"^DJ request failed"): provider_request( "DJ", "POST", f"http://127.0.0.1:{port}/graphql", json={}, timeout=2 ) @@ -446,14 +450,14 @@ def test_redirect_chains_are_bounded() -> None: assert len(seen) == semantic_layer_http.MAX_REDIRECTS + 1 -def test_error_message_carries_status_only() -> None: +def test_error_message_carries_provider_and_status_only() -> None: routes: dict[str, Route] = {"/a": (500, {}, b"internal detail")} with _server(routes) as (port, _), _internal_hosts(True): with pytest.raises(ProviderRequestError) as raised: provider_request( "MetricFlow", "GET", f"http://127.0.0.1:{port}/a", timeout=2 ) - assert str(raised.value) == "Provider request failed with HTTP 500." + assert str(raised.value) == "MetricFlow request failed with HTTP 500." def test_transport_failure_hides_the_destination( @@ -689,7 +693,7 @@ def test_bounded_error_bodies_are_not_read_or_logged( with _server(routes, public_tls[0]) as (port, _), caplog.at_level("WARNING"): with pytest.raises(ProviderRequestError) as raised: _body(f"https://public.example.test:{port}/q") - assert str(raised.value) == "Provider request failed with HTTP 500." + assert str(raised.value) == "MetricFlow request failed with HTTP 500." assert "internal detail" not in caplog.text @@ -828,7 +832,7 @@ def slow_lookup(*args: Any, **kwargs: Any) -> list[Any]: waits: list[float] = [] - def stalled_connect(address: Any, timeout: Any = None, **kwargs: Any) -> Any: + def stalled_connect(address: Any, timeout: Any, options: Any) -> Any: waits.append(float(timeout)) time.sleep(float(timeout)) raise socket.timeout("timed out") @@ -930,7 +934,7 @@ def call() -> None: with _internal_hosts(allow_internal), pytest.raises(ProviderRequestError) as raised: call() - assert str(raised.value) == "Provider endpoint could not be reached." + assert str(raised.value) == "MetricFlow endpoint could not be reached." assert "unreachable" in caplog.text assert "refused" not in caplog.text assert "private DNS detail" not in caplog.text @@ -968,8 +972,18 @@ def test_byte_cap_boundaries_and_chunk_accounting( assert max(consumed) <= 65536 +@pytest.mark.parametrize( + "provider,expected", + [ + ("UNTRUSTED_LABEL", "UNTRUSTED_LABEL"), + ("Cube\nforged=https://secret.test/", "Provider"), + ], +) def test_error_body_is_unread( - monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + provider: str, + expected: str, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, ) -> None: from unittest.mock import MagicMock @@ -981,10 +995,11 @@ def test_error_body_is_unread( semantic_layer_http._DirectSession, "request", lambda *args, **kwargs: response ) with pytest.raises(ProviderRequestError) as raised: - provider_request("UNTRUSTED_LABEL", "GET", response.url, timeout=1) + provider_request(provider, "GET", response.url, timeout=1) assert raised.value.failure == ProviderRequestFailure.HTTP_STATUS response.raw.read1.assert_not_called() - assert "UNTRUSTED_LABEL" not in caplog.text + assert f"{expected} request failed: category=http_status status=503" in caplog.text + assert str(raised.value) == f"{expected} request failed with HTTP 503." assert "secret" not in caplog.text assert "category=http_status status=503" in caplog.text @@ -1032,6 +1047,9 @@ def test_connect_boundary_stall_uses_real_socket_timeout( left, right = socket.socketpair() class ConnectingSocket: + def setsockopt(self, level: int, option: int, value: int | bytes) -> None: + assert (level, option, value) == (socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) + def settimeout(self, seconds: float) -> None: left.settimeout(seconds) @@ -1191,14 +1209,14 @@ def test_cross_scheme_redirect_returns_status_without_reading_body( headers={"Location": "http://fixture.test/"}, ) response.__enter__.return_value = response - monkeypatch.setattr( - semantic_layer_http._DirectSession, "request", lambda *args, **kwargs: response - ) + send: MagicMock = MagicMock(return_value=response) + monkeypatch.setattr(semantic_layer_http._DirectSession, "request", send) with pytest.raises(ProviderRequestError) as raised: provider_request("fixture", "GET", response.url, timeout=1) assert raised.value.failure == ProviderRequestFailure.HTTP_STATUS assert raised.value.status == 302 response.raw.read1.assert_not_called() + send.assert_called_once() def test_explicit_zero_port_is_not_the_default_origin( @@ -1243,3 +1261,129 @@ def test_completed_body_does_not_require_a_second_full_copy() -> None: tracemalloc.stop() assert len(body) == size assert peak < size * 1.25 + len(chunk) + + +@pytest.mark.parametrize("bundle_variable", ["REQUESTS_CA_BUNDLE", "CURL_CA_BUNDLE"]) +def test_internal_tls_honors_environment_ca_without_proxy( + bundle_variable: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + cert: Path + key: Path + cert, key = _certificate(tmp_path, "localhost") + context: ssl.SSLContext = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + context.load_cert_chain(cert, key) + monkeypatch.delenv("REQUESTS_CA_BUNDLE", raising=False) + monkeypatch.delenv("CURL_CA_BUNDLE", raising=False) + monkeypatch.setenv(bundle_variable, str(cert)) + variable: str + for variable in ("HTTPS_PROXY", "https_proxy", "ALL_PROXY", "all_proxy"): + monkeypatch.setenv(variable, "http://127.0.0.1:1") + monkeypatch.setenv("NO_PROXY", "") + monkeypatch.setenv("no_proxy", "") + # No Session/verify patch: real Requests must load the environment CA. + with _server(context=context) as (port, seen), _internal_hosts(True): + response: ProviderResponse = provider_request( + "Cube", "GET", f"https://localhost:{port}/private-ca", timeout=2 + ) + assert response.status == 200 + assert len(seen) == 1 + + +def test_closed_deadline_is_removed_by_worker_without_heap_rebuild() -> None: + scheduler: semantic_layer_http.DeadlineScheduler = semantic_layer_http._scheduler() + bound: semantic_layer_http._Deadline = semantic_layer_http._Deadline( + time.monotonic() + 3600 + ) + left: socket.socket + right: socket.socket + left, right = socket.socketpair() + try: + with ( + scheduler._condition, + patch.object(semantic_layer_http.heapq, "heapify") as rebuild, + ): + scheduler.register(bound, left) + scheduler.close(bound) + rebuild.assert_not_called() + assert not bound.sockets + until: float = time.monotonic() + 1 + while time.monotonic() < until: + with scheduler._condition: + if not any(item[2] is bound for item in scheduler._heap): + break + time.sleep(0.001) + with scheduler._condition: + assert not any(item[2] is bound for item in scheduler._heap) + assert not bound.expired + left.sendall(b"still open") + assert right.recv(10) == b"still open" + finally: + scheduler.close(bound) + left.close() + right.close() + + +@pytest.mark.skipif(not hasattr(os, "fork"), reason="POSIX worker lifecycle") +def test_fork_recreates_provider_workers_and_inherited_locks() -> None: + # Isolate fork from pytest/Flask background threads; kill a stuck child too. + script: str = textwrap.dedent(""" + import os + import signal + import socket + import time + from concurrent.futures import ThreadPoolExecutor + from superset.semantic_layers import transport + from superset.utils import network + + network._resolve_addresses("127.0.0.1", 443, 1, public_only=False) + transport._scheduler() + resolver: ThreadPoolExecutor | None = network._RESOLVER + scheduler: transport.DeadlineScheduler | None = transport._SCHEDULER + network._RESOLVER_LOCK.acquire() + transport._SCHEDULER_LOCK.acquire() + pid: int = os.fork() + if pid == 0: + signal.alarm(4) + try: + assert network._RESOLVER is None + assert transport._SCHEDULER is None + assert network._resolve_addresses( + "127.0.0.1", 443, 1, public_only=False + ) == ["127.0.0.1"] + left: socket.socket + right: socket.socket + left, right = socket.socketpair() + right.settimeout(1) + bound: transport._Deadline = transport._Deadline( + time.monotonic() + 0.05 + ) + bound.watch(left) + assert right.recv(1) == b"" + assert bound.expired + bound.close() + left.close() + right.close() + except BaseException: + import traceback + traceback.print_exc() + os._exit(1) + os._exit(0) + network._RESOLVER_LOCK.release() + transport._SCHEDULER_LOCK.release() + status: int + _, status = os.waitpid(pid, 0) + assert os.waitstatus_to_exitcode(status) == 0 + assert network._RESOLVER is resolver + assert transport._SCHEDULER is scheduler + assert network._resolve_addresses( + "127.0.0.1", 443, 1, public_only=False + ) == ["127.0.0.1"] + """) + result: subprocess.CompletedProcess[str] = subprocess.run( # noqa: S603 -- fixed local test script, no external input + [sys.executable, "-c", script], + capture_output=True, + text=True, + timeout=15, + check=False, + ) + assert result.returncode == 0, result.stdout + result.stderr diff --git a/tests/unit_tests/utils/test_network.py b/tests/unit_tests/utils/test_network.py index aa49fa0e9faa..f26a96f9e691 100644 --- a/tests/unit_tests/utils/test_network.py +++ b/tests/unit_tests/utils/test_network.py @@ -318,7 +318,7 @@ def test_pinned_connection_does_not_repeat_dns_lookup() -> None: finally: PROVIDER_DEADLINE.reset(token) resolve.assert_called_once() - connect.assert_called_once_with(("8.8.8.8", 443), 1) + connect.assert_called_once_with(("8.8.8.8", 443), 1, connection.socket_options) bound.watch.assert_called_once_with(connect.return_value) assert ( connection.host == "fixture.test" @@ -363,3 +363,38 @@ def test_empty_resolution_fails_without_connecting() -> None: with pytest.raises(OSError, match="No destination address"): resolve_public_addresses("fixture.test", 443, timeout=1) connect.assert_not_called() + + +@pytest.mark.parametrize("option_fails", [False, True]) +def test_pinned_connect_applies_socket_options_before_connect( + option_fails: bool, +) -> None: + import socket + + from superset.utils.network import _connect_address + + with patch("superset.utils.network.socket.socket") as factory: + sock: MagicMock = factory.return_value + if option_fails: + sock.setsockopt.side_effect = OSError("option rejected") + with pytest.raises(OSError, match="option rejected"): + _connect_address( + ("127.0.0.1", 443), 1, [(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)] + ) + sock.connect.assert_not_called() + sock.close.assert_called_once() + else: + assert ( + _connect_address( + ("127.0.0.1", 443), 1, [(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)] + ) + is sock + ) + assert [call[0] for call in sock.mock_calls] == [ + "settimeout", + "setsockopt", + "connect", + ] + sock.setsockopt.assert_called_once_with( + socket.IPPROTO_TCP, socket.TCP_NODELAY, 1 + ) From c2d612b39710409263d3ab4f8c92b180d3731461 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 12:22:03 -0600 Subject: [PATCH 05/23] fix(semantic-layer): preserve safe transport errors in metadata APIs --- superset/semantic_layers/api.py | 21 ++-- tests/unit_tests/semantic_layers/api_test.py | 105 ++++++++++++++++--- 2 files changed, 102 insertions(+), 24 deletions(-) diff --git a/superset/semantic_layers/api.py b/superset/semantic_layers/api.py index 041d4d54aeec..a266acdce23b 100644 --- a/superset/semantic_layers/api.py +++ b/superset/semantic_layers/api.py @@ -28,6 +28,7 @@ from marshmallow import ValidationError from pydantic import ValidationError as PydanticValidationError from sqlalchemy.orm import load_only, Query +from superset_core.semantic_layers.transport import ProviderRequestError from superset import db, event_logger, is_feature_enabled, security_manager from superset.commands.semantic_layer.create import ( @@ -308,13 +309,12 @@ def structure(self, pk: int) -> Response: except SupersetSecurityException as ex: return self.response(403, message=ex.message, errors=[ex.to_dict()]) except Exception as ex: # pylint: disable=broad-except - logger.error( - "Error fetching structure for semantic view %d: %s", - pk, - str(ex), - exc_info=True, + logger.exception("Error fetching structure for semantic view %d", pk) + return self.response_422( + message=str(ex) + if isinstance(ex, ProviderRequestError) + else t("Unable to fetch semantic view structure.") ) - return self.response_422(message=str(ex)) return self.response( 200, @@ -853,7 +853,14 @@ def runtime_schema(self, uuid: str) -> FlaskResponse: except SupersetSecurityException as ex: return self.response(403, message=ex.message, errors=[ex.to_dict()]) except Exception as ex: # pylint: disable=broad-except - return self.response_400(message=str(ex)) + logger.exception( + "Error fetching runtime schema for semantic layer %s", uuid + ) + return self.response_400( + message=str(ex) + if isinstance(ex, ProviderRequestError) + else t("Unable to fetch semantic layer runtime schema.") + ) return self.response(200, result=schema) diff --git a/tests/unit_tests/semantic_layers/api_test.py b/tests/unit_tests/semantic_layers/api_test.py index 2bc9c32d9312..dec9023e9204 100644 --- a/tests/unit_tests/semantic_layers/api_test.py +++ b/tests/unit_tests/semantic_layers/api_test.py @@ -25,6 +25,10 @@ from flask.testing import FlaskClient from pydantic import BaseModel, Field, model_validator, SecretStr from pytest_mock import MockerFixture +from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure, +) from werkzeug.test import TestResponse from superset.commands.semantic_layer.exceptions import ( @@ -680,21 +684,22 @@ def test_runtime_schema_unknown_type( @SEMANTIC_LAYERS_APP def test_runtime_schema_exception( - client: Any, + client: FlaskClient, full_api_access: None, mocker: MockerFixture, + caplog: pytest.LogCaptureFixture, ) -> None: - """Test POST //schema/runtime returns 400 when schema raises.""" - test_uuid = str(uuid_lib.uuid4()) - mock_layer = MagicMock() + """Keep provider details in server logs, not the runtime-schema response.""" + test_uuid: str = str(uuid_lib.uuid4()) + mock_layer: MagicMock = MagicMock() mock_layer.type = "snowflake" mock_layer.implementation.configuration = {"account": "test"} - mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO") + mock_dao: MagicMock = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO") mock_dao.find_by_uuid.return_value = mock_layer - mock_cls = MagicMock() - mock_cls.get_runtime_schema.side_effect = ValueError("Bad config") + mock_cls: MagicMock = MagicMock() + mock_cls.get_runtime_schema.side_effect = ValueError("provider-private-detail") mocker.patch.dict( "superset.semantic_layers.api.registry", @@ -702,12 +707,14 @@ def test_runtime_schema_exception( clear=True, ) - response = client.post( + response: TestResponse = client.post( f"/api/v1/semantic_layer/{test_uuid}/schema/runtime", ) assert response.status_code == 400 - assert "Bad config" in response.json["message"] + assert response.json["message"] == "Unable to fetch semantic layer runtime schema." + assert "provider-private-detail" not in response.get_data(as_text=True) + assert "ValueError: provider-private-detail" in caplog.text @pytest.mark.parametrize( @@ -2820,26 +2827,31 @@ def test_get_semantic_view_structure_feature_flag_off( @SEMANTIC_LAYERS_APP +@pytest.mark.parametrize("method", ["get_dimensions", "get_metrics"]) def test_get_semantic_view_structure_implementation_error( - client: Any, + client: FlaskClient, full_api_access: None, mocker: MockerFixture, + caplog: pytest.LogCaptureFixture, + method: str, ) -> None: - """Test GET //structure returns 422 when implementation raises.""" - mock_view = MagicMock() - mock_view.implementation.get_dimensions.side_effect = RuntimeError( - "Connection failed" + """Keep provider details in server logs, not the structure response.""" + mock_view: MagicMock = MagicMock() + getattr(mock_view.implementation, method).side_effect = RuntimeError( + "provider-private-detail" ) - mock_db_session = mocker.patch("superset.semantic_layers.api.db.session") + mock_db_session: MagicMock = mocker.patch("superset.semantic_layers.api.db.session") mock_db_session.query.return_value.filter_by.return_value.first.return_value = ( mock_view ) - response = client.get("/api/v1/semantic_view/1/structure") + response: TestResponse = client.get("/api/v1/semantic_view/1/structure") assert response.status_code == 422 - assert "Connection failed" in response.json["message"] + assert response.json["message"] == "Unable to fetch semantic view structure." + assert "provider-private-detail" not in response.get_data(as_text=True) + assert "RuntimeError: provider-private-detail" in caplog.text @SEMANTIC_LAYERS_APP @@ -3395,3 +3407,62 @@ def test_post_semantic_view_provider_validation( layer.implementation.get_semantic_view.assert_called_once_with( "orders", {"db": "prod"} ) + + +@SEMANTIC_LAYERS_APP +@pytest.mark.parametrize("endpoint", ["structure", "runtime_schema"]) +@pytest.mark.parametrize( + "failure,status,expected", + [ + ("HTTP_STATUS", 503, "MetricFlow request failed with HTTP 503."), + ("TIMEOUT", None, "MetricFlow request exceeded its time budget."), + ("UNREACHABLE", None, "MetricFlow endpoint could not be reached."), + (None, None, "fixed"), + ], +) +def test_metadata_endpoints_preserve_only_typed_transport_errors( + client: FlaskClient, + full_api_access: None, + mocker: MockerFixture, + endpoint: str, + failure: str | None, + status: int | None, + expected: str, +) -> None: + error: Exception = ( + ProviderRequestError( + ProviderRequestFailure[failure], status=status, provider="MetricFlow" + ) + if failure is not None + # Looking like a transport message must not grant an untyped error passthrough. + else ValueError( + "MetricFlow request failed with HTTP 503. provider-private-detail" + ) + ) + response: TestResponse + if endpoint == "structure": + view: MagicMock = MagicMock() + view.implementation.get_dimensions.side_effect = error + session: MagicMock = mocker.patch("superset.semantic_layers.api.db.session") + session.query.return_value.filter_by.return_value.first.return_value = view + response = client.get("/api/v1/semantic_view/1/structure") + assert response.status_code == 422 + if failure is None: + expected = "Unable to fetch semantic view structure." + else: + layer_uuid: str = str(uuid_lib.uuid4()) + layer: MagicMock = MagicMock() + layer.type = "fixture" + dao: MagicMock = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO") + dao.find_by_uuid.return_value = layer + provider: MagicMock = MagicMock() + provider.get_runtime_schema.side_effect = error + mocker.patch.dict( + "superset.semantic_layers.api.registry", {"fixture": provider}, clear=True + ) + response = client.post(f"/api/v1/semantic_layer/{layer_uuid}/schema/runtime") + assert response.status_code == 400 + if failure is None: + expected = "Unable to fetch semantic layer runtime schema." + assert response.json["message"] == expected + assert "provider-private-detail" not in response.get_data(as_text=True) From 99016b8ba3c471e7c6f02a2e2f7e12faa3cb9bb0 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 12:34:35 -0600 Subject: [PATCH 06/23] fix(semantic): isolate environment trust and guard provider labels --- .../extensions/semantic-provider-transport.md | 6 +- .../semantic_layers/transport.py | 3 +- .../tests/semantic_layers/test_transport.py | 17 ++++++ superset/semantic_layers/transport.py | 17 +++++- .../semantic_layers/transport_test.py | 60 ++++++++++++++++++- 5 files changed, 95 insertions(+), 8 deletions(-) diff --git a/docs/developer_docs/extensions/semantic-provider-transport.md b/docs/developer_docs/extensions/semantic-provider-transport.md index 6aa15c93ae91..22646e7c96a9 100644 --- a/docs/developer_docs/extensions/semantic-provider-transport.md +++ b/docs/developer_docs/extensions/semantic-provider-transport.md @@ -85,7 +85,8 @@ Errors and logs include the provider display name. Pass a constant label such as `MetricFlow` or `dbt Semantic Layer`, never endpoint or configuration values. Labels must start with an ASCII letter or digit and contain only ASCII letters, digits, spaces, underscores or hyphens, up to 64 characters. Other labels fall -back to `Provider`; valid labels have trailing spaces removed. +back to `Provider`; valid labels have trailing spaces removed. Non-string labels +also use that fallback, without converting the supplied object to a string. The transport sends `Accept-Encoding: identity` and refuses other response encodings. Non-2xx bodies are not read. Logs contain only the sanitized provider name, status and failure @@ -104,7 +105,8 @@ Operators can set `SEMANTIC_LAYER_ALLOW_INTERNAL_HOSTS = True` in `superset_config.py` to allow private addresses and HTTP for these providers. This setting does not change OAuth or webhook policy. It never disables budgets, TLS verification or redirect restrictions. Both settings ignore environment proxy -variables. Configure a reachable provider endpoint rather than relying on a proxy. +variables and never discover credentials from `.netrc`. Configure a reachable +provider endpoint rather than relying on a proxy. In allow-internal mode, `REQUESTS_CA_BUNDLE` (or `CURL_CA_BUNDLE` as fallback) can supply the trusted CA bundle for private TLS endpoints. Certificate and hostname verification remain enabled. The default policy ignores both variables. diff --git a/superset-core/src/superset_core/semantic_layers/transport.py b/superset-core/src/superset_core/semantic_layers/transport.py index c15110e0c71c..bd3f4ceaf0f7 100644 --- a/superset-core/src/superset_core/semantic_layers/transport.py +++ b/superset-core/src/superset_core/semantic_layers/transport.py @@ -110,7 +110,8 @@ def __init__( # Reject URL-like, control-bearing or excessive labels rather than echoing them. self.provider: str = ( provider.strip() - if re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9 _-]{0,63}", provider) + if isinstance(provider, str) + and re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9 _-]{0,63}", provider) else "Provider" ) super().__init__(failure) diff --git a/superset-core/tests/semantic_layers/test_transport.py b/superset-core/tests/semantic_layers/test_transport.py index 1f6b84127d4f..d2e4c7316e5f 100644 --- a/superset-core/tests/semantic_layers/test_transport.py +++ b/superset-core/tests/semantic_layers/test_transport.py @@ -19,6 +19,7 @@ import math from dataclasses import FrozenInstanceError +from typing import cast import pytest @@ -167,3 +168,19 @@ def test_provider_error_uses_only_a_bounded_display_name( ) assert error.provider == expected assert str(error) == f"{expected} request failed with HTTP 503." + + +@pytest.mark.parametrize( + "provider", [None, 42, b"https://secret.test/", {"secret": True}] +) +def test_non_string_error_labels_fall_back_without_coercion(provider: object) -> None: + from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure, + ) + + error: ProviderRequestError = ProviderRequestError( + ProviderRequestFailure.UNREACHABLE, provider=cast(str, provider) + ) + assert error.provider == "Provider" + assert str(error) == "Provider endpoint could not be reached." diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index 7c811b92fe38..9bb9bd9c0a42 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -76,13 +76,14 @@ def request( json: Any = None, consume: Callable[[int], None] | None = None, ) -> ProviderResponse: + # Do not coerce arbitrary objects while handling a URL-bearing failure. + provider = provider if isinstance(provider, str) else "Provider" bound: _Deadline = _Deadline(budget.deadline) token: Token[ConnectionDeadline | None] = PROVIDER_DEADLINE.set(bound) try: bound.remaining() # No parsing, resolution or socket work after expiry. self._validated_url(url) - with _DirectSession() as session: - session.trust_env = self._allow_internal + with _DirectSession(allow_environment_ca=self._allow_internal) as session: adapter: PinnedHTTPAdapter = PinnedHTTPAdapter( public_only=not self._allow_internal ) @@ -247,9 +248,19 @@ def build_provider_transport() -> ProviderTransport: class _DirectSession(requests.Session): + def __init__(self, *, allow_environment_ca: bool = False) -> None: + super().__init__() + self.trust_env = False # Never discover credentials from netrc. + self._allow_environment_ca: bool = allow_environment_ca + def merge_environment_settings(self, *args: Any, **kwargs: Any) -> _Settings: - # Retain internal-mode CA bundles, but the deadline must own the direct socket. settings: _Settings = super().merge_environment_settings(*args, **kwargs) + if self._allow_environment_ca and settings["verify"] is True: + settings["verify"] = ( + os.environ.get("REQUESTS_CA_BUNDLE") + or os.environ.get("CURL_CA_BUNDLE") + or settings["verify"] + ) settings["proxies"] = {} return settings diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index cbd7b071201d..d40ec72b219d 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -27,14 +27,15 @@ import sys import textwrap import time -from collections.abc import Iterator +import traceback +from collections.abc import Callable, Iterator from concurrent.futures import ThreadPoolExecutor from contextlib import contextmanager from datetime import datetime, timedelta, timezone from http.server import BaseHTTPRequestHandler, HTTPServer from pathlib import Path from threading import get_ident, Thread -from typing import Any, TypeAlias +from typing import Any, cast, TypeAlias from unittest.mock import patch import pytest @@ -1387,3 +1388,58 @@ def test_fork_recreates_provider_workers_and_inherited_locks() -> None: check=False, ) assert result.returncode == 0, result.stdout + result.stderr + + +@pytest.mark.parametrize("authorization", [None, "Bearer provider-owned-token"]) +def test_internal_requests_never_read_netrc( + authorization: str | None, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + from unittest.mock import Mock + + netrc_path: Path = tmp_path / "netrc" + netrc_path.write_text("machine localhost login fixture password netrc-secret\n") + netrc_path.chmod(0o600) + monkeypatch.setenv("NETRC", str(netrc_path)) + lookup: Mock = Mock(wraps=requests.sessions.get_netrc_auth) + monkeypatch.setattr(requests.sessions, "get_netrc_auth", lookup) + observed: list[str | None] = [] + original: Callable[[_Handler], None] = _Handler._respond + + def respond(handler: _Handler) -> None: + observed.append(handler.headers.get("Authorization")) + original(handler) + + monkeypatch.setattr(_Handler, "_respond", respond) + headers: dict[str, str] = ( + {"Authorization": authorization} if authorization is not None else {} + ) + with _server() as (port, _), _internal_hosts(True): + response: ProviderResponse = provider_request( + "Cube", "GET", f"http://localhost:{port}/", timeout=2, headers=headers + ) + assert response.status == 200 + assert observed == [authorization] + lookup.assert_not_called() + + +@pytest.mark.parametrize("provider", [None, 42, {"url": "https://secret.test/"}]) +def test_non_string_label_cannot_chain_library_secrets( + provider: object, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + from unittest.mock import Mock + + raw: str = "https://user:raw-secret@private.test/path?token=raw-secret" + send: Mock = Mock(side_effect=requests.ConnectionError(raw)) + monkeypatch.setattr(semantic_layer_http._DirectSession, "request", send) + with pytest.raises(ProviderRequestError) as raised: + provider_request(cast(str, provider), "GET", raw, timeout=2) + assert raised.value.failure is ProviderRequestFailure.UNREACHABLE + assert str(raised.value) == "Provider endpoint could not be reached." + assert raised.value.__cause__ is None + assert raised.value.__suppress_context__ + rendered: str = "".join(traceback.format_exception(raised.value)) + assert "raw-secret" not in rendered + assert "private.test" not in rendered + assert "raw-secret" not in caplog.text + assert "private.test" not in caplog.text + send.assert_called_once() From 6ce9dc284dd09e5892f7cae32d547eca2e6a50ec Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 18:46:45 -0600 Subject: [PATCH 07/23] test(semantic-layer): cover transport lifecycle guards --- superset/semantic_layers/transport.py | 7 -- .../semantic_layers/transport_test.py | 112 +++++++++++++++++- 2 files changed, 110 insertions(+), 9 deletions(-) diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index 9bb9bd9c0a42..764271066bfa 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -324,13 +324,6 @@ def close(self, bound: _Deadline) -> None: bound.sockets.clear() self._condition.notify() - def drain(self) -> None: - """Release all owned descriptors after isolated tests finish.""" - with self._condition: - for bound in tuple(self._tokens): - self._expire(bound) - self.close(bound) - @staticmethod def _expire(bound: _Deadline) -> None: bound.expired = True diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index d40ec72b219d..1425e2e3d20b 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -19,6 +19,7 @@ from __future__ import annotations +import importlib.util import ipaddress import os import socket @@ -33,10 +34,12 @@ from contextlib import contextmanager from datetime import datetime, timedelta, timezone from http.server import BaseHTTPRequestHandler, HTTPServer +from importlib.machinery import ModuleSpec from pathlib import Path -from threading import get_ident, Thread +from threading import get_ident, Lock, Thread +from types import ModuleType from typing import Any, cast, TypeAlias -from unittest.mock import patch +from unittest.mock import Mock, patch import pytest import requests @@ -1443,3 +1446,108 @@ def test_non_string_label_cannot_chain_library_secrets( assert "raw-secret" not in caplog.text assert "private.test" not in caplog.text send.assert_called_once() + + +def test_missing_hostname_is_rejected_before_io() -> None: + send: Mock + raised: pytest.ExceptionInfo[ProviderRequestError] + with patch.object(semantic_layer_http._DirectSession, "request") as send: + with pytest.raises(ProviderRequestError) as raised: + provider_request("fixture", "GET", "https:///missing-host", timeout=1) + assert raised.value.failure is ProviderRequestFailure.INVALID_URL + send.assert_not_called() + + +def test_closed_deadline_does_not_duplicate_or_shutdown_socket() -> None: + scheduler: semantic_layer_http.DeadlineScheduler = semantic_layer_http._scheduler() + bound: semantic_layer_http._Deadline = semantic_layer_http._Deadline( + time.monotonic() - 1 + ) + left: socket.socket + right: socket.socket + left, right = socket.socketpair() + try: + scheduler.close(bound) + scheduler.register(bound, left) + assert not bound.sockets + assert bound not in scheduler._tokens + assert not bound.expired + right.settimeout(0.2) + left.sendall(b"open") + assert right.recv(4) == b"open" + finally: + scheduler.close(bound) + left.close() + right.close() + + +def test_shutdown_error_does_not_skip_other_owned_sockets() -> None: + failed: Mock = Mock(spec=socket.socket) + failed.shutdown.side_effect = OSError("already disconnected") + left: socket.socket + right: socket.socket + left, right = socket.socketpair() + bound: semantic_layer_http._Deadline = semantic_layer_http._Deadline( + time.monotonic() - 1, sockets=[failed, left.dup()] + ) + try: + semantic_layer_http.DeadlineScheduler._expire(bound) + assert bound.expired + failed.shutdown.assert_called_once_with(socket.SHUT_RDWR) + right.settimeout(0.2) + assert right.recv(1) == b"" + finally: + semantic_layer_http._scheduler().close(bound) + left.close() + right.close() + failed.close.assert_called_once() + assert not bound.sockets + + +@pytest.mark.parametrize("fork_available", [True, False]) +def test_registered_fork_hook_resets_only_child_scheduler_state( + fork_available: bool, monkeypatch: pytest.MonkeyPatch +) -> None: + # Load a separate module instance: never replace the live worker or its lock. + spec: ModuleSpec | None = importlib.util.spec_from_file_location( + "_isolated_provider_transport", semantic_layer_http.__file__ + ) + assert spec is not None + assert spec.loader is not None + isolated: ModuleType = importlib.util.module_from_spec(spec) + monkeypatch.setitem(sys.modules, spec.name, isolated) + register: Mock = Mock() + platform: pytest.MonkeyPatch + with monkeypatch.context() as platform: + if fork_available: + platform.setattr(os, "register_at_fork", register, raising=False) + else: + platform.delattr(os, "register_at_fork", raising=False) + spec.loader.exec_module(isolated) + assert isolated._SCHEDULER is None + if not fork_available: + register.assert_not_called() + return + register.assert_called_once_with( + after_in_child=isolated._reset_scheduler_after_fork + ) + inherited_scheduler: Mock = Mock(spec=isolated.DeadlineScheduler) + monkeypatch.setattr(isolated, "_SCHEDULER", inherited_scheduler) + inherited_lock: Lock = isolated._SCHEDULER_LOCK + inherited_lock.acquire() + parent_scheduler: semantic_layer_http.DeadlineScheduler | None = ( + semantic_layer_http._SCHEDULER + ) + parent_lock: Lock = semantic_layer_http._SCHEDULER_LOCK + callback: Callable[[], None] = register.call_args.kwargs["after_in_child"] + try: + callback() + assert isolated._SCHEDULER is None + assert isolated._SCHEDULER_LOCK is not inherited_lock + assert isolated._SCHEDULER_LOCK.acquire(blocking=False) + isolated._SCHEDULER_LOCK.release() + assert semantic_layer_http._SCHEDULER is parent_scheduler + assert semantic_layer_http._SCHEDULER_LOCK is parent_lock + assert not inherited_scheduler.mock_calls + finally: + inherited_lock.release() From 205945c815569c22432324ed77d609c98b6204bd Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 19:13:10 -0600 Subject: [PATCH 08/23] test(semantic-layer): tighten TLS fixtures and sync error translations --- superset/translations/messages.pot | 6 ++++++ tests/unit_tests/semantic_layers/transport_test.py | 2 ++ 2 files changed, 8 insertions(+) diff --git a/superset/translations/messages.pot b/superset/translations/messages.pot index 4d0ae907a545..a52a088692c0 100644 --- a/superset/translations/messages.pot +++ b/superset/translations/messages.pot @@ -15724,6 +15724,12 @@ msgstr "" msgid "Unable to encode value" msgstr "" +msgid "Unable to fetch semantic layer runtime schema." +msgstr "" + +msgid "Unable to fetch semantic view structure." +msgstr "" + msgid "Unable to fetch semantic views. Check the layer configuration." msgstr "" diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 1425e2e3d20b..473ca024ca8b 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -333,6 +333,7 @@ def public_tls( key: Path cert, key = _certificate(tmp_path, "public.example.test") context: ssl.SSLContext = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + context.minimum_version = ssl.TLSVersion.TLSv1_2 context.load_cert_chain(cert, key) resolved: list[str] = [] real_resolve: Any = socket.getaddrinfo @@ -1275,6 +1276,7 @@ def test_internal_tls_honors_environment_ca_without_proxy( key: Path cert, key = _certificate(tmp_path, "localhost") context: ssl.SSLContext = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + context.minimum_version = ssl.TLSVersion.TLSv1_2 context.load_cert_chain(cert, key) monkeypatch.delenv("REQUESTS_CA_BUNDLE", raising=False) monkeypatch.delenv("CURL_CA_BUNDLE", raising=False) From 95ab635da3198373688d7e32e621acdf01da3c82 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Mon, 26 Oct 2026 09:47:12 -0600 Subject: [PATCH 09/23] fix(semantic-layer): compare prepared redirect destinations --- superset/semantic_layers/transport.py | 32 +++++- .../semantic_layers/transport_test.py | 103 ++++++++++++++---- 2 files changed, 109 insertions(+), 26 deletions(-) diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index 764271066bfa..befadbaf4a60 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -148,14 +148,34 @@ def request( PROVIDER_DEADLINE.reset(token) bound.close() - def _validated_url(self, url: str) -> SplitResult: + @staticmethod + def _check_url_characters(url: str) -> None: + """Refuse characters URL parsers can discard or interpret differently.""" + if any( + char == "\\" or char.isspace() or ord(char) < 32 or ord(char) == 127 + for char in url + ): + raise ProviderRequestError(Failure.INVALID_URL) + + @classmethod + def _prepared_url(cls, url: str) -> SplitResult: + """Compare destinations using the same URL preparation as Requests.""" + cls._check_url_characters(url) try: - parts: SplitResult = urlsplit(url) + prepared: requests.PreparedRequest = requests.PreparedRequest() + prepared.prepare_url(url, None) + if prepared.url is None: + raise ProviderRequestError(Failure.INVALID_URL) + parts: SplitResult = urlsplit(prepared.url) parts.port # noqa: B018 Validate malformed ports before any I/O. - except ValueError: + except (ValueError, requests.RequestException): raise ProviderRequestError(Failure.INVALID_URL) from None if not parts.hostname: raise ProviderRequestError(Failure.INVALID_URL) + return parts + + def _validated_url(self, url: str) -> SplitResult: + parts: SplitResult = self._prepared_url(url) if parts.scheme not in ( {"https", "http"} if self._allow_internal else {"https"} ): @@ -170,10 +190,10 @@ def _next_redirect(self, method: str, response: requests.Response) -> str | None ): return None before: SplitResult = self._validated_url(response.url) + self._check_url_characters(response.headers["Location"]) try: target: str = urljoin(response.url, response.headers["Location"]) - after: SplitResult = urlsplit(target) - after.port # noqa: B018 + after: SplitResult = self._prepared_url(target) except ValueError: raise ProviderRequestError(Failure.INVALID_URL) from None @@ -186,7 +206,7 @@ def origin(parts: SplitResult) -> tuple[str, str | None, int]: else (443 if parts.scheme == "https" else 80), ) - return target if origin(before) == origin(after) else None + return after.geturl() if origin(before) == origin(after) else None @staticmethod def _read_body( diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 473ca024ca8b..5a423bd37401 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -35,11 +35,13 @@ from datetime import datetime, timedelta, timezone from http.server import BaseHTTPRequestHandler, HTTPServer from importlib.machinery import ModuleSpec +from io import BytesIO from pathlib import Path from threading import get_ident, Lock, Thread from types import ModuleType from typing import Any, cast, TypeAlias from unittest.mock import Mock, patch +from urllib.parse import urlsplit import pytest import requests @@ -56,6 +58,7 @@ ProviderTransport, RequestBudget, ) +from urllib3.response import HTTPResponse from superset.semantic_layers import transport as semantic_layer_http from superset.utils import network @@ -420,6 +423,86 @@ def test_default_path_follows_same_origin_get_redirects_only( assert [entry.split()[1] for entry in seen] == ["/accounts", "/accounts/", "/away"] +@pytest.mark.parametrize( + "location,followed", + [ + ("https://evil.example\\@provider.example/", False), + ("https://evil.example%5C@provider.example/", True), + ("https://provider.example@evil.example/", False), + ("https://PROVIDER.EXAMPLE/next", True), + ("https://provider.example./next", False), + ("https://provider.example:443/next", True), + ("https://provider.example:0/next", True), + ("http://provider.example/next", False), + ("/next", True), + ("/next\t", False), + ("/next\n", False), + ("/next\x00", False), + ("/next\x7f", False), + ("/next path", False), + ], +) +def test_redirect_checks_prepared_destination( + monkeypatch: pytest.MonkeyPatch, location: str, followed: bool +) -> None: + """Exercise Requests preparation before capturing adapter-bound credentials.""" + sent: list[requests.PreparedRequest] = [] + + def send( + adapter: network.PinnedHTTPAdapter, + request: requests.PreparedRequest, + **kwargs: Any, + ) -> requests.Response: + sent.append(request) + response: requests.Response = requests.Response() + response.status_code = 302 if len(sent) == 1 else 200 + assert request.url is not None + response.url = request.url + response.request = request + response.raw = HTTPResponse(body=BytesIO(b"ok"), preload_content=False) + if len(sent) == 1: + response.headers["Location"] = location + return response + + monkeypatch.setattr(network.PinnedHTTPAdapter, "send", send) + if followed: + assert ( + provider_request( + "fixture", + "GET", + "https://provider.example/start", + timeout=2, + headers={"Authorization": "Bearer private", "X-API-Key": "private"}, + ).status + == 200 + ) + else: + with pytest.raises(ProviderRequestError): + provider_request( + "fixture", + "GET", + "https://provider.example/start", + timeout=2, + headers={"Authorization": "Bearer private", "X-API-Key": "private"}, + ) + assert len(sent) == (2 if followed else 1) + assert all(urlsplit(request.url).hostname == "provider.example" for request in sent) + assert all(request.headers["X-API-Key"] == "private" for request in sent) + + +@pytest.mark.parametrize("suffix", ["\\@provider.example/", "/path\n", "/a b"]) +def test_ambiguous_first_hop_is_rejected_before_adapter( + monkeypatch: pytest.MonkeyPatch, suffix: str +) -> None: + """First-hop validation must not accept syntax Requests interprets differently.""" + send: Mock = Mock(side_effect=AssertionError("Invalid URL reached adapter")) + monkeypatch.setattr(network.PinnedHTTPAdapter, "send", send) + with pytest.raises(ProviderRequestError) as raised: + provider_request("fixture", "GET", f"https://evil.example{suffix}", timeout=2) + assert raised.value.failure == ProviderRequestFailure.INVALID_URL + send.assert_not_called() + + def test_flag_allows_local_http_development_endpoints() -> None: with _server() as (port, seen), _internal_hosts(True): response: ProviderResponse = provider_request( @@ -1224,26 +1307,6 @@ def test_cross_scheme_redirect_returns_status_without_reading_body( send.assert_called_once() -def test_explicit_zero_port_is_not_the_default_origin( - monkeypatch: pytest.MonkeyPatch, -) -> None: - from unittest.mock import MagicMock - - response: MagicMock = MagicMock( - status_code=302, - url="https://fixture.test:0/", - headers={"Location": "https://fixture.test:443/"}, - ) - response.__enter__.return_value = response - send: MagicMock = MagicMock(return_value=response) - monkeypatch.setattr(semantic_layer_http._DirectSession, "request", send) - with pytest.raises(ProviderRequestError) as raised: - provider_request("fixture", "GET", response.url, timeout=1) - assert raised.value.failure == ProviderRequestFailure.HTTP_STATUS - send.assert_called_once() - response.raw.read1.assert_not_called() - - def test_completed_body_does_not_require_a_second_full_copy() -> None: """Keep an 8 MiB body to one buffer, allowing allocator slack and one chunk.""" import tracemalloc From 2be7b327f22770a0db1a61ada37c77d6eb911def Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Mon, 26 Oct 2026 12:58:43 -0600 Subject: [PATCH 10/23] fix(semantic-layer): refuse redirect credentials Reject userinfo in prepared redirect destinations before Requests can replace the caller Authorization header. Keep configured first-hop handling unchanged and verify the original headers at the HTTP adapter. --- superset/semantic_layers/transport.py | 2 ++ tests/unit_tests/semantic_layers/transport_test.py | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index befadbaf4a60..97dbc60dac8d 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -194,6 +194,8 @@ def _next_redirect(self, method: str, response: requests.Response) -> str | None try: target: str = urljoin(response.url, response.headers["Location"]) after: SplitResult = self._prepared_url(target) + if after.username is not None or after.password is not None: + raise ProviderRequestError(Failure.INVALID_URL) except ValueError: raise ProviderRequestError(Failure.INVALID_URL) from None diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 5a423bd37401..93b553c0c4b5 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -427,7 +427,10 @@ def test_default_path_follows_same_origin_get_redirects_only( "location,followed", [ ("https://evil.example\\@provider.example/", False), - ("https://evil.example%5C@provider.example/", True), + ("https://evil.example%5C@provider.example/", False), + ("https://attacker:pw@provider.example/x", False), + ("https://@provider.example/x", True), + ("https://:pw@provider.example/x", False), ("https://provider.example@evil.example/", False), ("https://PROVIDER.EXAMPLE/next", True), ("https://provider.example./next", False), @@ -488,6 +491,7 @@ def send( assert len(sent) == (2 if followed else 1) assert all(urlsplit(request.url).hostname == "provider.example" for request in sent) assert all(request.headers["X-API-Key"] == "private" for request in sent) + assert all(request.headers["Authorization"] == "Bearer private" for request in sent) @pytest.mark.parametrize("suffix", ["\\@provider.example/", "/path\n", "/a b"]) From 3074e8c25bbf37c79bbee2ad4f7c273a7d411bd0 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Mon, 26 Oct 2026 16:36:47 -0600 Subject: [PATCH 11/23] test(semantic-layer): cover invalid prepared URL rejection Exercise hostless URL preparation and missing preparation output before adapter I/O. Pin the invalid-url category and early rejection so the provider transport coverage gate covers both defensive branches. --- .../semantic_layers/transport_test.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 93b553c0c4b5..2209674a597d 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -507,6 +507,38 @@ def test_ambiguous_first_hop_is_rejected_before_adapter( send.assert_not_called() +def test_hostless_prepared_url_is_invalid_before_adapter( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Non-HTTP preparation can succeed without yielding a network hostname.""" + send: Mock = Mock(side_effect=AssertionError("Invalid URL reached adapter")) + monkeypatch.setattr(network.PinnedHTTPAdapter, "send", send) + raised: pytest.ExceptionInfo[ProviderRequestError] + with pytest.raises(ProviderRequestError) as raised: + provider_request("fixture", "GET", "mailto:provider@example.test", timeout=2) + assert raised.value.failure == ProviderRequestFailure.INVALID_URL + send.assert_not_called() + + +def test_missing_prepared_url_is_invalid_before_parsing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A missing result from preparation must not be passed to the URL parser.""" + prepare: Mock = Mock(return_value=None) + parse: Mock = Mock(wraps=urlsplit) + send: Mock = Mock(side_effect=AssertionError("Invalid URL reached adapter")) + monkeypatch.setattr(requests.PreparedRequest, "prepare_url", prepare) + monkeypatch.setattr(semantic_layer_http, "urlsplit", parse) + monkeypatch.setattr(network.PinnedHTTPAdapter, "send", send) + raised: pytest.ExceptionInfo[ProviderRequestError] + with pytest.raises(ProviderRequestError) as raised: + provider_request("fixture", "GET", "https://provider.example/", timeout=2) + assert raised.value.failure == ProviderRequestFailure.INVALID_URL + prepare.assert_called_once_with("https://provider.example/", None) + parse.assert_not_called() + send.assert_not_called() + + def test_flag_allows_local_http_development_endpoints() -> None: with _server() as (port, seen), _internal_hosts(True): response: ProviderResponse = provider_request( From e791fe98e818bc024e7afbef128a25f3cc87b6fc Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Mon, 26 Oct 2026 19:58:22 -0600 Subject: [PATCH 12/23] fix(semantic-layer): bound DNS admission and align transport validation --- .../extensions/semantic-provider-transport.md | 10 ++- superset/semantic_layers/transport.py | 2 + superset/utils/network.py | 32 ++++++--- .../semantic_layers/transport_test.py | 70 +++++++++++++++---- tests/unit_tests/utils/test_network.py | 63 +++++++++++++++++ 5 files changed, 151 insertions(+), 26 deletions(-) diff --git a/docs/developer_docs/extensions/semantic-provider-transport.md b/docs/developer_docs/extensions/semantic-provider-transport.md index 22646e7c96a9..817763e5547b 100644 --- a/docs/developer_docs/extensions/semantic-provider-transport.md +++ b/docs/developer_docs/extensions/semantic-provider-transport.md @@ -66,8 +66,10 @@ exhausted. DNS, connect, TLS, headers and body reads all share the deadline. A timed-out or truncated response is never returned as success. Resolution uses at most eight -shared workers; an OS resolver already running cannot be cancelled, but the caller -stops waiting at its deadline. Connected sockets are interrupted by one shared, +shared workers and at most eight outstanding submissions. At capacity, new +lookups fail immediately with `TIMEOUT`. A caller stops waiting at its deadline, +but its submission retains capacity until the worker completes it. An OS resolver +already running cannot be cancelled; this bound does not recover stalled workers. Connected sockets are interrupted by one shared, lazily started scheduler. Neither component starts at module import. ## Responses and errors @@ -96,6 +98,10 @@ bodies. Redirects do not reset the deadline. No automatic retries are made. ## Destination policy +URLs containing userinfo (`https://user:password@host/`) are refused as +`INVALID_URL`, both on the initial request and on redirects. Supply credentials +through the request headers instead. + By default, only HTTPS endpoints whose DNS answers are all public are allowed. The host applies its shared `is_safe_ip` policy, including special IPv6 ranges. It connects directly to a checked numeric address without resolving again, while diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index 97dbc60dac8d..700ea0366603 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -176,6 +176,8 @@ def _prepared_url(cls, url: str) -> SplitResult: def _validated_url(self, url: str) -> SplitResult: parts: SplitResult = self._prepared_url(url) + if parts.username is not None or parts.password is not None: + raise ProviderRequestError(Failure.INVALID_URL) if parts.scheme not in ( {"https", "http"} if self._allow_internal else {"https"} ): diff --git a/superset/utils/network.py b/superset/utils/network.py index 0760193c87cf..71f333cd291a 100644 --- a/superset/utils/network.py +++ b/superset/utils/network.py @@ -21,7 +21,7 @@ import subprocess from concurrent.futures import Future, ThreadPoolExecutor from contextvars import ContextVar -from threading import Lock +from threading import BoundedSemaphore, Lock from typing import Any, Protocol import requests @@ -279,6 +279,8 @@ def watch(self, sock: socket.socket) -> None: ... ) _RESOLVER: ThreadPoolExecutor | None = None _RESOLVER_LOCK: Lock = Lock() +_RESOLVER_LIMIT: int = 8 +_RESOLVER_SLOTS: BoundedSemaphore = BoundedSemaphore(_RESOLVER_LIMIT) class NonPublicAddressError(OSError): @@ -293,15 +295,22 @@ def _resolve_addresses( if timeout <= 0: raise TimeoutError with _RESOLVER_LOCK: - if _RESOLVER is None: - _RESOLVER = ThreadPoolExecutor(8, "provider-dns") - lookup: Future[Any] = _RESOLVER.submit( - socket.getaddrinfo, host, port, socket.AF_UNSPEC, socket.SOCK_STREAM - ) - try: - answers: Any = lookup.result(timeout=timeout) - finally: - lookup.cancel() # Queued work is disposable; running system DNS cannot stop. + slots: BoundedSemaphore = _RESOLVER_SLOTS + if not slots.acquire(blocking=False): + raise TimeoutError("Provider DNS capacity exhausted") + try: + if _RESOLVER is None: + _RESOLVER = ThreadPoolExecutor(_RESOLVER_LIMIT, "provider-dns") + lookup: Future[Any] = _RESOLVER.submit( + socket.getaddrinfo, host, port, socket.AF_UNSPEC, socket.SOCK_STREAM + ) + except BaseException: + slots.release() + raise + # Never cancel timed-out submissions: cancellation leaves the work item queued. + # Only worker completion returns capacity, including queued and running work. + lookup.add_done_callback(lambda _: slots.release()) + answers: Any = lookup.result(timeout=timeout) addresses: list[str] = list(dict.fromkeys(str(info[4][0]) for info in answers)) if not addresses: raise OSError("No destination address") @@ -416,9 +425,10 @@ def init_poolmanager(self, *args: Any, **kwargs: Any) -> None: def _reset_resolver_after_fork() -> None: """Discard executor state and locks whose owning threads do not survive fork.""" - global _RESOLVER, _RESOLVER_LOCK # pylint: disable=global-statement + global _RESOLVER, _RESOLVER_LOCK, _RESOLVER_SLOTS # pylint: disable=global-statement _RESOLVER = None _RESOLVER_LOCK = Lock() + _RESOLVER_SLOTS = BoundedSemaphore(_RESOLVER_LIMIT) if hasattr(os, "register_at_fork"): diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 2209674a597d..915ff0a9ccb3 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -353,21 +353,24 @@ def resolve(host: str, *args: Any, **kwargs: Any) -> Any: local: str = "127.0.0.1" if host.endswith(".example.test") else host return real_resolve(local, *args, **kwargs) - original: Any = requests.Session.merge_environment_settings - - def trust_test_ca(self: requests.Session, *args: Any) -> dict[str, Any]: - settings: dict[str, Any] = original(self, *args) - if get_ident() != owner: - return settings - settings["verify"] = str(cert) - return settings - monkeypatch.setattr(network.socket, "getaddrinfo", resolve) monkeypatch.setattr(network, "is_safe_ip", lambda address: True) - monkeypatch.setattr(requests.Session, "merge_environment_settings", trust_test_ca) + monkeypatch.setattr(requests.adapters, "DEFAULT_CA_BUNDLE_PATH", str(cert)) return context, resolved +def test_public_tls_fixture_preserves_verification_choice( + public_tls: tuple[ssl.SSLContext, list[str]], +) -> None: + """Trust installation must not override a caller disabling verification.""" + session: semantic_layer_http._DirectSession + with semantic_layer_http._DirectSession() as session: + settings: semantic_layer_http._Settings = session.merge_environment_settings( + "https://public.example.test/", {}, False, False, None + ) + assert settings["verify"] is False + + def test_public_tls_ignores_background_loopback_lookups( public_tls: tuple[ssl.SSLContext, list[str]], ) -> None: @@ -404,6 +407,27 @@ def test_default_path_verifies_the_configured_hostname( assert set(resolved) - set(names) == set() # Numeric connects do not resolve again. +def test_default_path_rejects_an_untrusted_certificate( + public_tls: tuple[ssl.SSLContext, list[str]], tmp_path: Path +) -> None: + """A matching hostname alone does not establish certificate trust.""" + untrusted: Path = tmp_path / "untrusted" + untrusted.mkdir() + cert: Path + key: Path + cert, key = _certificate(untrusted, "public.example.test") + context: ssl.SSLContext = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + context.load_cert_chain(cert, key) + port: int + seen: list[str] + with _server(context=context) as (port, seen): + with pytest.raises(ProviderRequestError, match="could not be reached"): + provider_request( + "fixture", "GET", f"https://public.example.test:{port}/", timeout=2 + ) + assert seen == [] + + def test_default_path_follows_same_origin_get_redirects_only( public_tls: tuple[ssl.SSLContext, list[str]], ) -> None: @@ -539,6 +563,22 @@ def test_missing_prepared_url_is_invalid_before_parsing( send.assert_not_called() +@pytest.mark.parametrize("userinfo", ["user:pass", "user", ":pass"]) +def test_first_hop_userinfo_is_refused_before_sending( + monkeypatch: pytest.MonkeyPatch, userinfo: str +) -> None: + """Provider credentials belong in headers, not URL authority.""" + send: Mock = Mock(side_effect=AssertionError("Userinfo reached adapter")) + monkeypatch.setattr(network.PinnedHTTPAdapter, "send", send) + raised: pytest.ExceptionInfo[ProviderRequestError] + with pytest.raises(ProviderRequestError) as raised: + provider_request( + "fixture", "GET", f"https://{userinfo}@provider.example/", timeout=2 + ) + assert raised.value.failure == ProviderRequestFailure.INVALID_URL + send.assert_not_called() + + def test_flag_allows_local_http_development_endpoints() -> None: with _server() as (port, seen), _internal_hosts(True): response: ProviderResponse = provider_request( @@ -595,7 +635,7 @@ def test_transport_failure_hides_the_destination( provider_request( "DJ", "GET", - "https://user:pass@dj.internal.example.com:8443/x?token=t", + "https://dj.internal.example.com:8443/x?token=t&password=pass", timeout=1, ) assert "dj.internal" not in str(raised.value) @@ -614,7 +654,7 @@ def test_failure_logs_never_carry_url_secrets( listener.bind(("127.0.0.1", 0)) port: int = listener.getsockname()[1] listener.close() # Nothing listens: the connection is refused or blocked. - url: str = f"https://user:SECRET_PW@127.0.0.1:{port}/x?token=SECRET_TOKEN" + url: str = f"https://127.0.0.1:{port}/x?token=SECRET_TOKEN&password=SECRET_PW" with _internal_hosts(allowed), caplog.at_level("WARNING"): with pytest.raises(ProviderRequestError): provider_request("Cube", "GET", url, timeout=2) @@ -1444,6 +1484,8 @@ def test_fork_recreates_provider_workers_and_inherited_locks() -> None: transport._scheduler() resolver: ThreadPoolExecutor | None = network._RESOLVER scheduler: transport.DeadlineScheduler | None = transport._SCHEDULER + for _ in range(network._RESOLVER_LIMIT): + assert network._RESOLVER_SLOTS.acquire(blocking=False) network._RESOLVER_LOCK.acquire() transport._SCHEDULER_LOCK.acquire() pid: int = os.fork() @@ -1473,6 +1515,8 @@ def test_fork_recreates_provider_workers_and_inherited_locks() -> None: traceback.print_exc() os._exit(1) os._exit(0) + for _ in range(network._RESOLVER_LIMIT): + network._RESOLVER_SLOTS.release() network._RESOLVER_LOCK.release() transport._SCHEDULER_LOCK.release() status: int @@ -1532,7 +1576,7 @@ def test_non_string_label_cannot_chain_library_secrets( ) -> None: from unittest.mock import Mock - raw: str = "https://user:raw-secret@private.test/path?token=raw-secret" + raw: str = "https://private.test/path?token=raw-secret" send: Mock = Mock(side_effect=requests.ConnectionError(raw)) monkeypatch.setattr(semantic_layer_http._DirectSession, "request", send) with pytest.raises(ProviderRequestError) as raised: diff --git a/tests/unit_tests/utils/test_network.py b/tests/unit_tests/utils/test_network.py index f26a96f9e691..2287aa18d9a3 100644 --- a/tests/unit_tests/utils/test_network.py +++ b/tests/unit_tests/utils/test_network.py @@ -398,3 +398,66 @@ def test_pinned_connect_applies_socket_options_before_connect( sock.setsockopt.assert_called_once_with( socket.IPPROTO_TCP, socket.TCP_NODELAY, 1 ) + + +def test_resolver_bounds_timed_out_queued_submissions( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Timeouts retain their slots until workers drain the submitted lookups.""" + from concurrent.futures import ThreadPoolExecutor + from threading import BoundedSemaphore, Event + + from superset.utils import network + + release: Event = Event() + slots: BoundedSemaphore = BoundedSemaphore(2) + monkeypatch.setattr(network, "_RESOLVER_SLOTS", slots, raising=False) + + def resolve(*args: object) -> list[tuple[None, None, None, None, tuple[str, int]]]: + assert release.wait(5) + return [(None, None, None, None, ("8.8.8.8", 443))] + + monkeypatch.setattr(network.socket, "getaddrinfo", resolve) + executor: ThreadPoolExecutor + with ThreadPoolExecutor(1) as executor: + monkeypatch.setattr(network, "_RESOLVER", executor) + submit: MagicMock + with patch.object(executor, "submit", wraps=executor.submit) as submit: + try: + _attempt: int + for _attempt in range(10): + with pytest.raises(TimeoutError): + network.resolve_public_addresses( + "fixture.test", 443, timeout=0.01 + ) + assert submit.call_count == 2 + finally: + release.set() + executor.shutdown(wait=True) + # Completion, not the caller timeout, returned both slots. + assert slots.acquire(blocking=False) + assert slots.acquire(blocking=False) + assert not slots.acquire(blocking=False) + slots.release() + slots.release() + + +def test_resolver_submission_failure_returns_capacity( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An executor refusing submission must not strand an admission slot.""" + from concurrent.futures import ThreadPoolExecutor + from threading import BoundedSemaphore + + from superset.utils import network + + slots: BoundedSemaphore = BoundedSemaphore(1) + executor: ThreadPoolExecutor + with ThreadPoolExecutor(1) as executor: + executor.shutdown(wait=True) + monkeypatch.setattr(network, "_RESOLVER", executor) + monkeypatch.setattr(network, "_RESOLVER_SLOTS", slots) + with pytest.raises(RuntimeError, match="cannot schedule"): + network.resolve_public_addresses("fixture.test", 443, timeout=1) + assert slots.acquire(blocking=False) + slots.release() From ca40800ad3dcc48ade89006920b91205245b9697 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 27 Oct 2026 17:34:11 -0600 Subject: [PATCH 13/23] fix(semantic-layer): serialize scheduler lock initialization Guard only lazy lock publication so native threads share one scheduler while preload gevent startup remains outside the native guard. Reset the guard after fork. Keep host transport construction outside provider fault attribution. Add widened-window race and host-fault regression coverage for the two review LOWs. --- superset/semantic_layers/execution.py | 4 +- superset/semantic_layers/transport.py | 9 +++-- .../semantic_layers/transport_test.py | 38 +++++++++++++++++++ .../semantic_layers/transport_wiring_test.py | 16 +++++++- 4 files changed, 62 insertions(+), 5 deletions(-) diff --git a/superset/semantic_layers/execution.py b/superset/semantic_layers/execution.py index 69bab2adbb6d..8bfbe7671892 100644 --- a/superset/semantic_layers/execution.py +++ b/superset/semantic_layers/execution.py @@ -39,6 +39,7 @@ SemanticExecutionContext, ) from superset_core.semantic_layers.layer import SemanticLayer as ProviderLayer +from superset_core.semantic_layers.transport import ProviderTransport from superset_core.semantic_layers.view import SemanticView as ProviderView from superset import db, security_manager @@ -332,13 +333,14 @@ def bind( provider: type[ProviderLayer[Any, Any]] = registry[layer.type] configuration: dict[str, Any] = json.loads(layer.configuration) cache_token: str = layer.metadata_generation + transport: ProviderTransport = build_provider_transport() with provider_code(): bound: ProviderLayer[Any, Any] = provider.from_configuration( configuration, cache_token=cache_token, context=context, credentials=None, - transport=build_provider_transport(), + transport=transport, ) self.bindings[key] = (bound, {}) return self.bindings[key] diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index a3814223e7f4..fd6844c48c36 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -382,13 +382,15 @@ def _run(self) -> None: _SCHEDULER: DeadlineScheduler | None = None _SCHEDULER_LOCK: Lock | None = None +_SCHEDULER_LOCK_INIT: Lock = Lock() def _scheduler() -> DeadlineScheduler: global _SCHEDULER, _SCHEDULER_LOCK # pylint: disable=global-statement # Resolve the factory after a preloaded gevent worker patches threading. - if _SCHEDULER_LOCK is None: - _SCHEDULER_LOCK = threading.Lock() + with _SCHEDULER_LOCK_INIT: + if _SCHEDULER_LOCK is None: + _SCHEDULER_LOCK = threading.Lock() with _SCHEDULER_LOCK: if _SCHEDULER is None: _SCHEDULER = DeadlineScheduler() @@ -397,9 +399,10 @@ def _scheduler() -> DeadlineScheduler: def _reset_scheduler_after_fork() -> None: """A child cannot reuse the parent's worker or potentially held lock.""" - global _SCHEDULER, _SCHEDULER_LOCK # pylint: disable=global-statement + global _SCHEDULER, _SCHEDULER_LOCK, _SCHEDULER_LOCK_INIT # pylint: disable=global-statement _SCHEDULER = None _SCHEDULER_LOCK = None + _SCHEDULER_LOCK_INIT = Lock() if hasattr(os, "register_at_fork"): diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index b617d82736cd..b62e1f5460c6 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -1499,6 +1499,7 @@ def test_fork_recreates_provider_workers_and_inherited_locks() -> None: network._RESOLVER_LOCK.acquire() assert transport._SCHEDULER_LOCK is not None transport._SCHEDULER_LOCK.acquire() + transport._SCHEDULER_LOCK_INIT.acquire() pid: int = os.fork() if pid == 0: signal.alarm(4) @@ -1530,6 +1531,7 @@ def test_fork_recreates_provider_workers_and_inherited_locks() -> None: network._RESOLVER_SLOTS.release() network._RESOLVER_LOCK.release() transport._SCHEDULER_LOCK.release() + transport._SCHEDULER_LOCK_INIT.release() status: int _, status = os.waitpid(pid, 0) assert os.waitstatus_to_exitcode(status) == 0 @@ -1778,3 +1780,39 @@ def test_scheduler_start_failure_does_not_publish_an_unstarted_worker( with pytest.raises(RuntimeError, match="thread startup failed"): semantic_layer_http._scheduler() assert semantic_layer_http._SCHEDULER is None + + +def test_scheduler_lock_initialization_is_serialized_across_os_threads() -> None: + """Widen first-use interleavings without leaving orphan workers in pytest.""" + script: str = textwrap.dedent(""" + import time + from concurrent.futures import ThreadPoolExecutor + from threading import Barrier, Lock + from types import SimpleNamespace + from superset.semantic_layers import transport + + original: type[transport.DeadlineScheduler] = transport.DeadlineScheduler + barrier: Barrier = Barrier(4) + def make_lock() -> Lock: + time.sleep(0.05) + return Lock() + def make_scheduler() -> transport.DeadlineScheduler: + time.sleep(0.05) + return original() + transport.threading = SimpleNamespace(Lock=make_lock) + transport.DeadlineScheduler = make_scheduler + def get_scheduler(index: int) -> object: + barrier.wait(timeout=5) + return transport._scheduler() + with ThreadPoolExecutor(4) as pool: + schedulers: list[object] = list(pool.map(get_scheduler, range(4))) + assert len({id(scheduler) for scheduler in schedulers}) == 1 + """) + result: subprocess.CompletedProcess[str] = subprocess.run( # noqa: S603 + [sys.executable, "-c", script], + capture_output=True, + text=True, + timeout=30, + check=False, + ) + assert result.returncode == 0, result.stdout + result.stderr diff --git a/tests/unit_tests/semantic_layers/transport_wiring_test.py b/tests/unit_tests/semantic_layers/transport_wiring_test.py index 69540c906d38..4f9716be2683 100644 --- a/tests/unit_tests/semantic_layers/transport_wiring_test.py +++ b/tests/unit_tests/semantic_layers/transport_wiring_test.py @@ -26,6 +26,7 @@ from superset_core.semantic_layers.transport import ProviderTransport from werkzeug.test import TestResponse +from superset.semantic_layers.exceptions import _is_provider_fault from superset.semantic_layers.execution import SemanticBindingMemo from superset.semantic_layers.masking import mask_configuration from superset.semantic_layers.models import SemanticLayer @@ -135,7 +136,10 @@ def test_masking_static_schema_receives_transport(mocker: MockerFixture) -> None factory.assert_called_once_with() -def test_binding_injects_transport_only_on_memo_miss(mocker: MockerFixture) -> None: +@pytest.mark.parametrize("host_failure", [False, True]) +def test_binding_injects_transport_only_on_memo_miss( + mocker: MockerFixture, host_failure: bool +) -> None: capability: MagicMock = MagicMock(spec=ProviderTransport) factory: MagicMock = mocker.patch( "superset.semantic_layers.execution.build_provider_transport", @@ -160,6 +164,16 @@ def test_binding_injects_transport_only_on_memo_miss(mocker: MockerFixture) -> N mocker.patch.dict("superset.semantic_layers.execution.registry", {"test": provider}) layer: SemanticLayer = SemanticLayer(type="test", configuration="{}") memo: SemanticBindingMemo = SemanticBindingMemo() + if host_failure: + error: RuntimeError = RuntimeError("host transport construction failed") + factory.side_effect = error + raised: pytest.ExceptionInfo[RuntimeError] + with pytest.raises(RuntimeError) as raised: + memo.bind(layer) + assert raised.value is error + assert not _is_provider_fault(error) + provider.from_configuration.assert_not_called() + return assert memo.bind(layer) is memo.bind(layer) factory.assert_called_once_with() provider.from_configuration.assert_called_once() From fc939cab302d6ece96d02b31463570e52888c574 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Wed, 28 Oct 2026 09:49:49 -0600 Subject: [PATCH 14/23] fix(semantic-layer): release abandoned redirect connections Close each abandoned response and its deadline-owned duplicate descriptors before following the next hop, so single-threaded keep-alive providers can accept it. Preserve the original deadline across hops. Add the real-server regression and strengthen refused-body and non-default-port tests for Joe's three redirect review threads. --- superset/semantic_layers/transport.py | 16 ++++- .../semantic_layers/transport_test.py | 67 +++++++++++++++++-- 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index fd6844c48c36..75393c26b5a6 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -108,8 +108,10 @@ def request( else None ) if target is not None: + response.close() + bound.release_sockets() url, params = target, None - continue # Context closes the unread redirect body. + continue if not 200 <= response.status_code < 300: raise ProviderRequestError( Failure.HTTP_STATUS, status=response.status_code @@ -312,6 +314,11 @@ def remaining(self) -> float: def watch(self, sock: socket.socket) -> None: _scheduler().register(self, sock) + def release_sockets(self) -> None: + """Release the abandoned hop without resetting the request deadline.""" + if _SCHEDULER is not None: + _SCHEDULER.release_sockets(self) + def close(self) -> None: # A request rejected before connecting need not start a scheduler. if _SCHEDULER is not None: @@ -346,10 +353,15 @@ def close(self, bound: _Deadline) -> None: with self._condition: bound.closed = True self._tokens.discard(bound) + self.release_sockets(bound) + self._condition.notify() + + def release_sockets(self, bound: _Deadline) -> None: + """Drop duplicate descriptors while preserving scheduled expiry.""" + with self._condition: for sock in bound.sockets: sock.close() bound.sockets.clear() - self._condition.notify() @staticmethod def _expire(bound: _Deadline) -> None: diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index b62e1f5460c6..8df9b650a5bc 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -460,6 +460,7 @@ def test_default_path_follows_same_origin_get_redirects_only( ("https://PROVIDER.EXAMPLE/next", True), ("https://provider.example./next", False), ("https://provider.example:443/next", True), + ("https://provider.example:8443/next", False), ("https://provider.example:0/next", True), ("http://provider.example/next", False), ("/next", True), @@ -1377,12 +1378,20 @@ def test_cross_scheme_redirect_returns_status_without_reading_body( ) -> None: from unittest.mock import MagicMock - response: MagicMock = MagicMock( - status_code=302, - url="https://fixture.test/", - headers={"Location": "http://fixture.test/"}, - ) - response.__enter__.return_value = response + response: requests.Response = requests.Response() + response.status_code = 302 + response.url = "https://fixture.test/" + response.headers["Location"] = "http://fixture.test/" + response.raw = MagicMock(spec=HTTPResponse) + response.raw.read.side_effect = AssertionError("Refused redirect body was read") + response.raw.read1.side_effect = AssertionError("Refused redirect body was read") + + def body_access(*args: Any, **kwargs: Any) -> bytes: + raise AssertionError("Refused redirect body accessor was used") + + monkeypatch.setattr(requests.Response, "content", property(body_access)) + monkeypatch.setattr(requests.Response, "text", property(body_access)) + monkeypatch.setattr(response, "iter_content", body_access) send: MagicMock = MagicMock(return_value=response) monkeypatch.setattr(semantic_layer_http._DirectSession, "request", send) with pytest.raises(ProviderRequestError) as raised: @@ -1816,3 +1825,49 @@ def get_scheduler(index: int) -> object: check=False, ) assert result.returncode == 0, result.stdout + result.stderr + + +def test_keep_alive_redirect_releases_abandoned_connection() -> None: + """A single-threaded provider must accept the next hop before the deadline.""" + seen: list[str] = [] + + class KeepAlive(BaseHTTPRequestHandler): + protocol_version: str = "HTTP/1.1" + + def do_GET(self) -> None: # noqa: N802 + seen.append(self.path) + if self.path == "/start": + self.send_response(302) + self.send_header("Location", "/final") + self.send_header("Content-Length", "4") + self.end_headers() + self.wfile.write(b"skip") + self.wfile.flush() + else: + self.send_response(200) + self.send_header("Content-Length", "2") + self.send_header("Connection", "close") + self.end_headers() + self.wfile.write(b"ok") + self.close_connection = True + + def log_message(self, *args: Any) -> None: + pass + + server: HTTPServer = HTTPServer(("127.0.0.1", 0), KeepAlive) + worker: Thread = Thread(target=server.serve_forever, daemon=True) + worker.start() + try: + with _internal_hosts(True): + response: ProviderResponse = provider_request( + "fixture", + "GET", + f"http://127.0.0.1:{server.server_port}/start", + timeout=1, + ) + assert response.body == b"ok" + assert seen == ["/start", "/final"] + finally: + server.shutdown() + server.server_close() + worker.join(2) From 080f84dc4c9f54c7a1d50dcbb614f4b580b9d1b5 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Wed, 28 Oct 2026 10:02:11 -0600 Subject: [PATCH 15/23] test(semantic-layer): cover cleanup before scheduler startup Exercise the no-scheduler release path without initializing deadline infrastructure, closing the final branch-coverage gap while keeping production behavior unchanged. --- .../semantic_layers/transport_test.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 8df9b650a5bc..85a41251abc4 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -1871,3 +1871,21 @@ def log_message(self, *args: Any) -> None: server.shutdown() server.server_close() worker.join(2) + + +def test_releasing_sockets_without_a_scheduler_is_a_noop( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Cleanup before a connection must not initialize deadline infrastructure.""" + monkeypatch.setattr(semantic_layer_http, "_SCHEDULER", None) + start: Mock = Mock(side_effect=AssertionError("Cleanup started a scheduler")) + monkeypatch.setattr(semantic_layer_http, "_scheduler", start) + bound: semantic_layer_http._Deadline = semantic_layer_http._Deadline( + time.monotonic() + 1 + ) + bound.release_sockets() + start.assert_not_called() + assert semantic_layer_http._SCHEDULER is None + assert not bound.sockets + assert not bound.closed + assert not bound.expired From 6f48535cfcf5e8eeac0971256ad56b27091a86fa Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Wed, 28 Oct 2026 17:27:45 -0600 Subject: [PATCH 16/23] test(semantic-layer): verify POST payload and query parameters on the wire --- .../semantic_layers/transport_test.py | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 85a41251abc4..7142bdb9a0e9 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -41,7 +41,7 @@ from types import ModuleType from typing import Any, cast, TypeAlias from unittest.mock import Mock, patch -from urllib.parse import urlsplit +from urllib.parse import parse_qs, urlsplit import pytest import requests @@ -61,7 +61,7 @@ from urllib3.response import HTTPResponse from superset.semantic_layers import transport as semantic_layer_http -from superset.utils import network +from superset.utils import json, network # All tests run in their own pytest process; unrelated background traffic is # explicitly exercised by the reference cases below. @@ -1889,3 +1889,45 @@ def test_releasing_sockets_without_a_scheduler_is_a_noop( assert not bound.sockets assert not bound.closed assert not bound.expired + + +def test_post_transmits_query_params_and_json_body() -> None: + """Verify provider payloads at the server, beyond request-call mocks.""" + received: list[tuple[str, bytes]] = [] + + class CapturePostHandler(_Handler): + """Capture wire bytes before returning the ordinary loopback response.""" + + seen: list[str] = [] + + def do_POST(self) -> None: # noqa: N802 + """Retain the request target and body for independent assertions.""" + body: bytes = self.rfile.read(int(self.headers.get("Content-Length", 0))) + received.append((self.path, body)) + self._respond() + + server: HTTPServer + with HTTPServer(("127.0.0.1", 0), CapturePostHandler) as server: + thread: Thread = Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + with _internal_hosts(True): + response: ProviderResponse = provider_request( + "Cube", + "POST", + f"http://127.0.0.1:{server.server_port}/graphql", + params={"page": "2"}, + json={"query": "{ ok }"}, + timeout=2, + ) + assert response.status == 200 + assert len(received) == 1 + path: str + body: bytes + path, body = received[0] + assert urlsplit(path).path == "/graphql" + assert parse_qs(urlsplit(path).query) == {"page": ["2"]} + assert json.loads(body) == {"query": "{ ok }"} + finally: + server.shutdown() + thread.join(timeout=2) From b54ff69fb26e05158afcbb8a57a942f3b7e000c2 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Thu, 29 Oct 2026 08:49:33 -0600 Subject: [PATCH 17/23] fix(semantic-layer): keep provider URLs out of connection-pool logs --- .../semantic-layer-cache-clear.md | 3 +- superset/semantic_layers/transport.py | 11 ++ .../semantic_layers/transport_test.py | 107 +++++++++++++++--- tests/unit_tests/utils/test_network.py | 15 +++ 4 files changed, 120 insertions(+), 16 deletions(-) diff --git a/docs/developer_docs/semantic-layer-cache-clear.md b/docs/developer_docs/semantic-layer-cache-clear.md index b2ceb04aa7c9..50d630c8dbdd 100644 --- a/docs/developer_docs/semantic-layer-cache-clear.md +++ b/docs/developer_docs/semantic-layer-cache-clear.md @@ -52,7 +52,8 @@ Providers implement one factory, invoked before eager discovery: @classmethod def from_configuration( cls, configuration: dict[str, Any], *, cache_token: str, - context: SemanticExecutionContext, credentials: SemanticCredentials | None = None, + context: SemanticExecutionContext, transport: ProviderTransport, + credentials: SemanticCredentials | None = None, ) -> SemanticLayer: ... ``` diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index 75393c26b5a6..240ae3340cad 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -59,6 +59,17 @@ _CHUNK_BYTES: int = 65536 +class _ProviderPoolLogFilter(logging.Filter): + """Keep provider URLs out of pool records without hiding unrelated traffic.""" + + def filter(self, record: logging.LogRecord) -> bool: + """Preserve records outside the active provider request context.""" + return PROVIDER_DEADLINE.get() is None + + +logging.getLogger("urllib3.connectionpool").addFilter(_ProviderPoolLogFilter()) + + class HostProviderTransport(ProviderTransport): """Fix operator policy at construction; keep budgets and sockets call-local.""" diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 7142bdb9a0e9..e03540163b34 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -21,6 +21,7 @@ import importlib.util import ipaddress +import logging import os import socket import ssl @@ -30,14 +31,14 @@ import time import traceback from collections.abc import Callable, Iterator -from concurrent.futures import ThreadPoolExecutor +from concurrent.futures import Future, ThreadPoolExecutor from contextlib import contextmanager from datetime import datetime, timedelta, timezone from http.server import BaseHTTPRequestHandler, HTTPServer from importlib.machinery import ModuleSpec from io import BytesIO from pathlib import Path -from threading import get_ident, Lock, Thread +from threading import Event, get_ident, Lock, Thread from types import ModuleType from typing import Any, cast, TypeAlias from unittest.mock import Mock, patch @@ -928,9 +929,15 @@ def content(response: requests.Response) -> bytes: @contextmanager -def _framing_server(context: ssl.SSLContext, mode: str) -> Iterator[int]: +def _framing_server( + context: ssl.SSLContext, mode: str +) -> Iterator[tuple[int, Event, Event]]: """Drip HTTP framing bytes (a header or chunk trailer), not body bytes.""" + ready: Event = Event() + release: Event = Event() + stop: Event = Event() + class Handler(BaseHTTPRequestHandler): def do_POST(self) -> None: # noqa: N802 self.rfile.read(int(self.headers.get("Content-Length", 0))) @@ -944,10 +951,12 @@ def do_POST(self) -> None: # noqa: N802 try: self.wfile.write(prefix) self.wfile.flush() - for _ in range(14): + ready.set() + if not release.wait(timeout=10): + return + while not stop.wait(0.04): self.wfile.write(b"x") self.wfile.flush() - time.sleep(0.04) self.wfile.write(b"\r\n\r\n") self.wfile.flush() except OSError: @@ -961,8 +970,10 @@ def log_message(self, *args: Any) -> None: thread: Thread = Thread(target=server.serve_forever, daemon=True) thread.start() try: - yield server.server_address[1] + yield server.server_address[1], ready, release finally: + stop.set() + release.set() server.shutdown() server.server_close() thread.join() @@ -971,16 +982,48 @@ def log_message(self, *args: Any) -> None: @pytest.mark.parametrize("mode", ["headers", "chunk-trailers", "close-delimited"]) @pytest.mark.parametrize("allow_internal", [False, True]) def test_bounded_framing_obeys_the_absolute_deadline( - public_tls: tuple[ssl.SSLContext, list[str]], mode: str, allow_internal: bool + public_tls: tuple[ssl.SSLContext, list[str]], + mode: str, + allow_internal: bool, + monkeypatch: pytest.MonkeyPatch, ) -> None: - # Allowing internal hosts relaxes destinations only, never the deadline. - with _framing_server(public_tls[0], mode) as port, _internal_hosts(allow_internal): - started: float = time.monotonic() + # Leave setup a generous budget. Only the transport's clock advances to + # the final 200 ms once the server has sent the named framing prefix. + offset: float = 0.0 + clock: Mock = Mock(wraps=time) + clock.monotonic.side_effect = lambda: time.monotonic() + offset + monkeypatch.setattr(semantic_layer_http, "time", clock) + worker: ThreadPoolExecutor + port: int + ready: Event + release: Event + with ( + ThreadPoolExecutor(1) as worker, + _framing_server(public_tls[0], mode) as (port, ready, release), + _internal_hosts(allow_internal), + ): + deadline: float = time.monotonic() + 30 + future: Future[bytes] = worker.submit( + provider_body, + "MetricFlow", + "POST", + f"https://public.example.test:{port}/q", + deadline=deadline, + max_bytes=4096, + timeout=(5.0, 30.0), + json={"query": "{ ok }"}, + ) + assert ready.wait(timeout=10), "server did not reach framing mode" + scheduler: semantic_layer_http.DeadlineScheduler = ( + semantic_layer_http._scheduler() + ) + with scheduler._condition: + offset = deadline - time.monotonic() - 0.2 + scheduler._condition.notify() + release.set() with pytest.raises(ProviderRequestError) as raised: - _body(f"https://public.example.test:{port}/q", seconds=0.15) + future.result(timeout=10) assert raised.value.failure == ProviderRequestFailure.TIMEOUT - elapsed: float = time.monotonic() - started - assert elapsed < 0.40, (mode, allow_internal, elapsed) def test_bounded_lookup_and_connect_attempts_share_the_deadline( @@ -1495,7 +1538,7 @@ def test_fork_recreates_provider_workers_and_inherited_locks() -> None: import signal import socket import time - from concurrent.futures import ThreadPoolExecutor + from concurrent.futures import Future, ThreadPoolExecutor from superset.semantic_layers import transport from superset.utils import network @@ -1795,7 +1838,7 @@ def test_scheduler_lock_initialization_is_serialized_across_os_threads() -> None """Widen first-use interleavings without leaving orphan workers in pytest.""" script: str = textwrap.dedent(""" import time - from concurrent.futures import ThreadPoolExecutor + from concurrent.futures import Future, ThreadPoolExecutor from threading import Barrier, Lock from types import SimpleNamespace from superset.semantic_layers import transport @@ -1931,3 +1974,37 @@ def do_POST(self) -> None: # noqa: N802 finally: server.shutdown() thread.join(timeout=2) + + +def test_provider_debug_logs_omit_query_tokens( + caplog: pytest.LogCaptureFixture, +) -> None: + """Provider requests stay private while unrelated urllib3 logs remain visible.""" + + def consume(count: int) -> None: + """Run unrelated HTTP while the provider request context is active.""" + worker: ThreadPoolExecutor + # Provider context is active here, but not in this unrelated thread. + with ThreadPoolExecutor(1) as worker: + worker.submit(_background_request).result(timeout=5) + + port: int + with ( + caplog.at_level(logging.DEBUG, logger="urllib3.connectionpool"), + _server() as (port, _), + _internal_hosts(True), + ): + response: ProviderResponse = provider_request( + "Example", + "GET", + f"http://127.0.0.1:{port}/catalog?token=SECRET", + timeout=2, + consume=consume, + ) + assert response.status == 200 + assert "SECRET" not in caplog.text + assert "/catalog" not in caplog.text + assert any( + record.name == "urllib3.connectionpool" and "/background" in record.getMessage() + for record in caplog.records + ) diff --git a/tests/unit_tests/utils/test_network.py b/tests/unit_tests/utils/test_network.py index 2287aa18d9a3..c5d91bd5dbf9 100644 --- a/tests/unit_tests/utils/test_network.py +++ b/tests/unit_tests/utils/test_network.py @@ -461,3 +461,18 @@ def test_resolver_submission_failure_returns_capacity( network.resolve_public_addresses("fixture.test", 443, timeout=1) assert slots.acquire(blocking=False) slots.release() + + +def test_pinned_connect_uses_ipv6_socket_for_public_numeric_target() -> None: + """A checked IPv6 address retains its family without a second DNS lookup.""" + import socket + + from superset.utils.network import _connect_address + + address: tuple[str, int] = ("2606:4700:4700::1111", 443) + factory: MagicMock + with patch("superset.utils.network.socket.socket") as factory: + sock: MagicMock = factory.return_value + assert _connect_address(address, 2, None) is sock + factory.assert_called_once_with(socket.AF_INET6, socket.SOCK_STREAM) + sock.connect.assert_called_once_with(address) From 0e3ff25dfb34dc86baf83434ee1157d35e723751 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Thu, 29 Oct 2026 16:52:26 -0600 Subject: [PATCH 18/23] fix(semantic-layer): cover provider URL warnings and internal framing Joe review threads r4415570656 and r4415570062 identified a urllib3.connection header warning that could log provider query tokens and a framing test whose worker lost the Flask internal-host context. Filter the warning during provider requests and build the test transport inside that context so the internal path is exercised. --- superset/semantic_layers/transport.py | 7 +- .../semantic_layers/transport_test.py | 76 +++++++++++++++---- 2 files changed, 67 insertions(+), 16 deletions(-) diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index 240ae3340cad..b4e77856ba76 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -59,15 +59,16 @@ _CHUNK_BYTES: int = 65536 -class _ProviderPoolLogFilter(logging.Filter): - """Keep provider URLs out of pool records without hiding unrelated traffic.""" +class _ProviderURLLogFilter(logging.Filter): + """Keep provider URLs out of urllib3 records without hiding unrelated traffic.""" def filter(self, record: logging.LogRecord) -> bool: """Preserve records outside the active provider request context.""" return PROVIDER_DEADLINE.get() is None -logging.getLogger("urllib3.connectionpool").addFilter(_ProviderPoolLogFilter()) +logging.getLogger("urllib3.connectionpool").addFilter(_ProviderURLLogFilter()) +logging.getLogger("urllib3.connection").addFilter(_ProviderURLLogFilter()) class HostProviderTransport(ProviderTransport): diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index e03540163b34..62fb73252041 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -930,7 +930,7 @@ def content(response: requests.Response) -> bytes: @contextmanager def _framing_server( - context: ssl.SSLContext, mode: str + context: ssl.SSLContext | None, mode: str ) -> Iterator[tuple[int, Event, Event]]: """Drip HTTP framing bytes (a header or chunk trailer), not body bytes.""" @@ -966,7 +966,8 @@ def log_message(self, *args: Any) -> None: pass server: HTTPServer = HTTPServer(("127.0.0.1", 0), Handler) - server.socket = context.wrap_socket(server.socket, server_side=True) + if context is not None: + server.socket = context.wrap_socket(server.socket, server_side=True) thread: Thread = Thread(target=server.serve_forever, daemon=True) thread.start() try: @@ -999,21 +1000,32 @@ def test_bounded_framing_obeys_the_absolute_deadline( release: Event with ( ThreadPoolExecutor(1) as worker, - _framing_server(public_tls[0], mode) as (port, ready, release), + _framing_server(None if allow_internal else public_tls[0], mode) as ( + port, + ready, + release, + ), _internal_hosts(allow_internal), ): deadline: float = time.monotonic() + 30 - future: Future[bytes] = worker.submit( - provider_body, - "MetricFlow", - "POST", - f"https://public.example.test:{port}/q", - deadline=deadline, - max_bytes=4096, - timeout=(5.0, 30.0), - json={"query": "{ ok }"}, + transport: ProviderTransport = semantic_layer_http.build_provider_transport() + url: str = ( + f"http://127.0.0.1:{port}/q" + if allow_internal + else f"https://public.example.test:{port}/q" ) - assert ready.wait(timeout=10), "server did not reach framing mode" + + def request_body() -> bytes: + return transport.request( + "MetricFlow", + "POST", + url, + budget=RequestBudget(deadline, 4096), + json={"query": "{ ok }"}, + ).body + + future: Future[bytes] = worker.submit(request_body) + assert ready.wait(timeout=5), "server did not reach framing mode" scheduler: semantic_layer_http.DeadlineScheduler = ( semantic_layer_http._scheduler() ) @@ -2008,3 +2020,41 @@ def consume(count: int) -> None: record.name == "urllib3.connectionpool" and "/background" in record.getMessage() for record in caplog.records ) + + +def test_malformed_provider_headers_do_not_log_query_tokens( + public_tls: tuple[ssl.SSLContext, list[str]], + caplog: pytest.LogCaptureFixture, +) -> None: + """A urllib3 header-parse warning must not reveal a provider URL.""" + + class MalformedHeaders(BaseHTTPRequestHandler): + def do_GET(self) -> None: # noqa: N802 + self.wfile.write( + b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nNot-A-Header\r\n\r\nok" + ) + self.wfile.flush() + + def log_message(self, *args: Any) -> None: + pass + + server: HTTPServer = HTTPServer(("127.0.0.1", 0), MalformedHeaders) + server.socket = public_tls[0].wrap_socket(server.socket, server_side=True) + thread: Thread = Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + with caplog.at_level(logging.WARNING, logger="urllib3.connection"): + response: ProviderResponse = provider_request( + "MetricFlow", + "GET", + f"https://public.example.test:{server.server_port}/q?token=SECRET", + timeout=2, + ) + assert response.body == b"ok" + finally: + server.shutdown() + server.server_close() + thread.join() + + assert "SECRET" not in caplog.text + assert all("SECRET" not in record.getMessage() for record in caplog.records) From 612c21a094a92247a84da8b352c530e481f6d754 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Sat, 31 Oct 2026 19:50:40 -0600 Subject: [PATCH 19/23] test(semantic-layer): verify concurrent deadline cleanup directly The concurrency test used process-wide descriptor counts and a one-second wall-clock margin. Under CI load and on first use in each worker, those assertions fail without a transport leak. Keep the server stalled until cleanup, wait for the concurrent requests to time out, and assert that the scheduler-owned duplicate sockets close. --- .../semantic_layers/transport_test.py | 54 +++++++++++++------ 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 62fb73252041..9e5d5dbdf204 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -31,7 +31,7 @@ import time import traceback from collections.abc import Callable, Iterator -from concurrent.futures import Future, ThreadPoolExecutor +from concurrent.futures import Future, ThreadPoolExecutor, wait from contextlib import contextmanager from datetime import datetime, timedelta, timezone from http.server import BaseHTTPRequestHandler, HTTPServer @@ -1306,13 +1306,34 @@ def close(self) -> None: right.close() -def test_ten_concurrent_deadlines_release_descriptors() -> None: +def test_ten_concurrent_deadlines_release_descriptors( + monkeypatch: pytest.MonkeyPatch, +) -> None: from http.server import ThreadingHTTPServer from threading import Event - import psutil - done: Event = Event() + watched: list[socket.socket] = [] + register: Callable[ + [ + semantic_layer_http.DeadlineScheduler, + semantic_layer_http._Deadline, + socket.socket, + ], + None, + ] = semantic_layer_http.DeadlineScheduler.register + + def record_registration( + scheduler: semantic_layer_http.DeadlineScheduler, + bound: semantic_layer_http._Deadline, + sock: socket.socket, + ) -> None: + register(scheduler, bound, sock) + watched.extend(bound.sockets) + + monkeypatch.setattr( + semantic_layer_http.DeadlineScheduler, "register", record_registration + ) class Stall(BaseHTTPRequestHandler): def do_GET(self) -> None: # noqa: N802 @@ -1321,13 +1342,11 @@ def do_GET(self) -> None: # noqa: N802 self.end_headers() self.wfile.write(b"x") self.wfile.flush() - done.wait(2) + done.wait() def log_message(self, *args: Any) -> None: pass - before: int = psutil.Process().num_fds() - class ConcurrentServer(ThreadingHTTPServer): request_queue_size: int = 32 @@ -1337,7 +1356,7 @@ class ConcurrentServer(ThreadingHTTPServer): ) worker.start() - def call(index: int) -> tuple[float, float]: + def call(index: int) -> None: budget: float = 0.15 + index * 0.02 start: float = time.monotonic() transport: semantic_layer_http.HostProviderTransport = ( @@ -1351,21 +1370,24 @@ def call(index: int) -> tuple[float, float]: budget=RequestBudget(start + budget, 100), ) assert raised.value.failure == ProviderRequestFailure.TIMEOUT - return time.monotonic() - start, budget + pool: ThreadPoolExecutor = ThreadPoolExecutor(10) try: - with ThreadPoolExecutor(10) as pool: - results: list[tuple[float, float]] = list(pool.map(call, range(10))) - assert all(elapsed < budget + 1 for elapsed, budget in results) + futures: list[Future[None]] = [pool.submit(call, index) for index in range(10)] + completed: set[Future[None]] + pending: set[Future[None]] + completed, pending = wait(futures, timeout=10) + assert not pending, "deadline requests did not complete while server stalled" + for future in completed: + future.result() finally: done.set() + pool.shutdown(wait=True) server.shutdown() server.server_close() worker.join(2) - deadline: float = time.monotonic() + 1 - while psutil.Process().num_fds() > before and time.monotonic() < deadline: - time.sleep(0.01) - assert psutil.Process().num_fds() <= before + assert watched, "no request registered a socket with the deadline scheduler" + assert all(sock.fileno() == -1 for sock in watched) assert not semantic_layer_http._scheduler()._tokens From 18dfebdf16c3c2378b744f094ba1e22a2b1de159 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 3 Nov 2026 11:04:29 -0600 Subject: [PATCH 20/23] fix(semantic-layer): read bounded responses on urllib3 2.1 Joe observed that read1 is unavailable before urllib3 2.2. Fall back to the older bounded read API while retaining the chunk limit and absolute deadline, and cover the missing-method path. --- superset/semantic_layers/transport.py | 5 ++- .../semantic_layers/transport_test.py | 31 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index b4e77856ba76..a66453e1b4ba 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -235,9 +235,12 @@ def _read_body( consume: Callable[[int], None] | None, ) -> bytes: body: BytesIO = BytesIO() + read_chunk: Callable[..., bytes] = getattr( + response.raw, "read1", response.raw.read + ) while True: bound.remaining() - chunk: bytes = response.raw.read1(_CHUNK_BYTES, decode_content=False) + chunk: bytes = read_chunk(_CHUNK_BYTES, decode_content=False) bound.remaining() # Shutdown can look like EOF; refuse partial bytes. if not chunk: return body.getvalue() diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 9e5d5dbdf204..352a414a7fd5 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -774,6 +774,37 @@ def test_bounded_body_is_read_through_the_checked_connection( assert seen == [f"POST /q public.example.test:{port}"] +def test_bounded_body_supports_urllib3_without_read1() -> None: + class RawWithoutRead1: + def __init__(self, body: bytes) -> None: + self.stream: BytesIO = BytesIO(body) + self.read_sizes: list[int] = [] + + def read(self, amt: int, decode_content: bool = False) -> bytes: + assert decode_content is False + self.read_sizes.append(amt) + return self.stream.read(amt) + + raw: RawWithoutRead1 = RawWithoutRead1(b"response body") + response: Mock = Mock(raw=raw) + bound: semantic_layer_http._Deadline = semantic_layer_http._Deadline( + time.monotonic() + 2 + ) + + assert ( + semantic_layer_http.HostProviderTransport._read_body(response, bound, 32, None) + == b"response body" + ) + assert raw.read_sizes == [semantic_layer_http._CHUNK_BYTES] * 2 + + oversized: RawWithoutRead1 = RawWithoutRead1(b"too many bytes") + with pytest.raises(ProviderRequestError, match="byte budget"): + semantic_layer_http.HostProviderTransport._read_body( + Mock(raw=oversized), bound, 4, None + ) + assert oversized.read_sizes == [semantic_layer_http._CHUNK_BYTES] + + def test_bounded_body_is_refused_past_its_byte_cap( public_tls: tuple[ssl.SSLContext, list[str]], ) -> None: From b0389b13005f942ce889a79b996bc8bc8ef9d7a0 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 3 Nov 2026 17:08:08 -0600 Subject: [PATCH 21/23] fix(semantic-layer): refuse truncated provider responses Joe identified that urllib3 1.x can end a declared-length response without raising. Check the declared byte count at EOF so a short provider reply fails closed, and link the transport guide from the Extensions sidebar as requested. --- docs/sidebarTutorials.js | 1 + superset/semantic_layers/transport.py | 12 ++++++++++++ .../unit_tests/semantic_layers/transport_test.py | 16 ++++++++++++++++ 3 files changed, 29 insertions(+) diff --git a/docs/sidebarTutorials.js b/docs/sidebarTutorials.js index b1063bd00635..ce3062259b44 100644 --- a/docs/sidebarTutorials.js +++ b/docs/sidebarTutorials.js @@ -87,6 +87,7 @@ const sidebars = { 'extensions/extension-points/semantic-layers', ], }, + 'extensions/semantic-provider-transport', 'extensions/development', 'extensions/deployment', 'extensions/mcp', diff --git a/superset/semantic_layers/transport.py b/superset/semantic_layers/transport.py index a66453e1b4ba..be6167cac966 100644 --- a/superset/semantic_layers/transport.py +++ b/superset/semantic_layers/transport.py @@ -235,6 +235,7 @@ def _read_body( consume: Callable[[int], None] | None, ) -> bytes: body: BytesIO = BytesIO() + content_length: str | None = response.headers.get("Content-Length") read_chunk: Callable[..., bytes] = getattr( response.raw, "read1", response.raw.read ) @@ -243,6 +244,17 @@ def _read_body( chunk: bytes = read_chunk(_CHUNK_BYTES, decode_content=False) bound.remaining() # Shutdown can look like EOF; refuse partial bytes. if not chunk: + if isinstance(content_length, str) and ( + response.request is None or response.request.method != "HEAD" + ): + if not content_length.isascii() or not content_length.isdecimal(): + raise ProviderRequestError(Failure.UNREACHABLE) + try: + expected_length: int = int(content_length) + except ValueError: + raise ProviderRequestError(Failure.UNREACHABLE) from None + if body.tell() != expected_length: + raise ProviderRequestError(Failure.UNREACHABLE) return body.getvalue() if consume is not None: consume(len(chunk)) diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 352a414a7fd5..c274bc725ea6 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -805,6 +805,22 @@ def read(self, amt: int, decode_content: bool = False) -> bytes: assert oversized.read_sizes == [semantic_layer_http._CHUNK_BYTES] +def test_bounded_body_rejects_short_urllib3_1_response() -> None: + raw: Mock = Mock(spec=["read"]) + raw.read.side_effect = [b"eleven bytes", b""] + response: Mock = Mock( + raw=raw, headers={"Content-Length": "100"}, request=Mock(method="GET") + ) + bound: semantic_layer_http._Deadline = semantic_layer_http._Deadline( + time.monotonic() + 2 + ) + + with pytest.raises(ProviderRequestError) as raised: + semantic_layer_http.HostProviderTransport._read_body(response, bound, 100, None) + + assert raised.value.failure == ProviderRequestFailure.UNREACHABLE + + def test_bounded_body_is_refused_past_its_byte_cap( public_tls: tuple[ssl.SSLContext, list[str]], ) -> None: From 2a88ef6aa0ae3d66074711b5bbfeb31ba8e11890 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 3 Nov 2026 17:21:27 -0600 Subject: [PATCH 22/23] test(semantic-layer): cover malformed response lengths The semantic-layer coverage gate requires every rejection branch. Exercise a nonnumeric length and an oversized numeric value so both fail with the existing transport error. --- .../semantic_layers/transport_test.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index c274bc725ea6..83ca22937698 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -821,6 +821,25 @@ def test_bounded_body_rejects_short_urllib3_1_response() -> None: assert raised.value.failure == ProviderRequestFailure.UNREACHABLE +@pytest.mark.parametrize("content_length", ["invalid", "9" * 5000]) +def test_bounded_body_rejects_malformed_content_length(content_length: str) -> None: + raw: Mock = Mock(spec=["read"]) + raw.read.return_value = b"" + response: Mock = Mock( + raw=raw, + headers={"Content-Length": content_length}, + request=Mock(method="GET"), + ) + bound: semantic_layer_http._Deadline = semantic_layer_http._Deadline( + time.monotonic() + 2 + ) + + with pytest.raises(ProviderRequestError) as raised: + semantic_layer_http.HostProviderTransport._read_body(response, bound, 100, None) + + assert raised.value.failure == ProviderRequestFailure.UNREACHABLE + + def test_bounded_body_is_refused_past_its_byte_cap( public_tls: tuple[ssl.SSLContext, list[str]], ) -> None: From 8ac1573fd38567283e427c5817dc8208ec65d9d4 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 3 Nov 2026 17:25:38 -0600 Subject: [PATCH 23/23] test(semantic-layer): type transport error witnesses Keep the new short-body and malformed-length regression bindings explicit under the Python annotation rules. --- tests/unit_tests/semantic_layers/transport_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit_tests/semantic_layers/transport_test.py b/tests/unit_tests/semantic_layers/transport_test.py index 83ca22937698..12d2fc5ca18a 100644 --- a/tests/unit_tests/semantic_layers/transport_test.py +++ b/tests/unit_tests/semantic_layers/transport_test.py @@ -815,6 +815,7 @@ def test_bounded_body_rejects_short_urllib3_1_response() -> None: time.monotonic() + 2 ) + raised: pytest.ExceptionInfo[ProviderRequestError] with pytest.raises(ProviderRequestError) as raised: semantic_layer_http.HostProviderTransport._read_body(response, bound, 100, None) @@ -834,6 +835,7 @@ def test_bounded_body_rejects_malformed_content_length(content_length: str) -> N time.monotonic() + 2 ) + raised: pytest.ExceptionInfo[ProviderRequestError] with pytest.raises(ProviderRequestError) as raised: semantic_layer_http.HostProviderTransport._read_body(response, bound, 100, None)