From e33e17f5e9e38399d721a7d66bffc8c884091a6a Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 11:38:14 -0600 Subject: [PATCH 01/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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 99dae9082e286e33678e6c2c980b96b84ebd8ee0 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 14:15:32 -0600 Subject: [PATCH 07/17] feat(semantic-layer): add provider readiness and backend fault contracts --- UPDATING.md | 14 + .../extensions/contribution-types.md | 6 +- .../extensions/semantic-provider-boundary.md | 91 +++++++ .../extensions/semantic-provider-transport.md | 6 +- .../superset_core/semantic_layers/__init__.py | 4 + .../superset_core/semantic_layers/config.py | 12 + .../superset_core/semantic_layers/errors.py | 16 ++ .../superset_core/semantic_layers/layer.py | 12 + .../semantic_layers/test_provider_boundary.py | 62 +++++ superset/semantic_layers/api.py | 14 +- superset/semantic_layers/exceptions.py | 15 +- superset/semantic_layers/masking.py | 19 +- superset/translations/messages.pot | 6 + .../superset_e2e_semantic_stub/__init__.py | 3 + .../semantic_layer/tool/test_get_table.py | 45 ++++ tests/unit_tests/semantic_layers/api_test.py | 40 ++- tests/unit_tests/semantic_layers/conftest.py | 8 +- .../semantic_layers/masking_test.py | 22 +- .../semantic_layers/provider_boundary_test.py | 243 ++++++++++++++++++ .../semantic_layers/provider_masking_test.py | 6 +- .../semantic_layers/query_errors_test.py | 92 ++++++- .../semantic_layers/transport_wiring_test.py | 12 +- 22 files changed, 711 insertions(+), 37 deletions(-) create mode 100644 docs/developer_docs/extensions/semantic-provider-boundary.md create mode 100644 superset-core/tests/semantic_layers/test_provider_boundary.py create mode 100644 tests/unit_tests/semantic_layers/provider_boundary_test.py diff --git a/UPDATING.md b/UPDATING.md index 9995402fb552..edb11437538f 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,20 @@ assists people when migrating to a new version. ## Next +- **Semantic-layer schema readiness and failures.** Both provider schema hooks + require keyword-only `readiness: SemanticCredentialReadiness` alongside + `transport`. This plain Enum is exported by `superset_core.semantic_layers`. + Discover metadata only when `readiness is SemanticCredentialReadiness.RESOLVED` + and required configuration fields are valid; otherwise return a static schema + without I/O. Do not interpret host masking placeholders. Upgrade host and + provider schema implementations together; omitted arguments fail explicitly. + `SemanticBackendError` signals execution or invalid-result faults with generic + host guidance (500); `ProviderRequestError` keeps safe transport detail (400), + and query refusal/completeness retain their separate meanings. Provider error + translations should suppress diagnostic chains. The semantic execution boundary + logs fixed categories without provider exception tracebacks. See the + [provider boundary guide](docs/developer_docs/extensions/semantic-provider-boundary.md). + - **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 diff --git a/docs/developer_docs/extensions/contribution-types.md b/docs/developer_docs/extensions/contribution-types.md index ecc61b86e6d6..d978fd8f65a1 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, and the [provider transport contract](semantic-provider-transport.md) for bounded HTTP requests. +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. See [readiness and failures](semantic-provider-boundary.md) for required schema arguments and safe error signals. 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.config import SemanticCredentialReadiness from superset_core.semantic_layers.transport import ProviderTransport from my_extension.config import MyConfig @@ -293,13 +294,14 @@ class MySemanticLayer(SemanticLayer[MyConfig, MySemanticView]): @classmethod def get_configuration_schema( cls, configuration: MyConfig | None = None, *, transport: ProviderTransport, + readiness: SemanticCredentialReadiness, ) -> dict[str, Any]: return MyConfig.model_json_schema() @classmethod def get_runtime_schema( cls, configuration: MyConfig, runtime_data: dict[str, Any] | None = None, - *, transport: ProviderTransport, + *, transport: ProviderTransport, readiness: SemanticCredentialReadiness, ) -> dict[str, Any]: return {"type": "object", "properties": {}} diff --git a/docs/developer_docs/extensions/semantic-provider-boundary.md b/docs/developer_docs/extensions/semantic-provider-boundary.md new file mode 100644 index 000000000000..dd6957bcb3c0 --- /dev/null +++ b/docs/developer_docs/extensions/semantic-provider-boundary.md @@ -0,0 +1,91 @@ +--- +title: Semantic provider readiness and failures +--- + + + +# Semantic provider readiness and failures + +## Schema discovery + +Both schema methods require keyword-only `transport` and `readiness`. Import +`SemanticCredentialReadiness` from `superset_core.semantic_layers.config` (also +exported from `superset_core.semantic_layers`). It is a plain Enum, not a string. + +```python +@classmethod +def get_configuration_schema( + cls, configuration: MyConfig | None = None, *, + transport: ProviderTransport, readiness: SemanticCredentialReadiness, +) -> dict[str, Any]: + schema: dict[str, Any] = MyConfig.model_json_schema() + if readiness is not SemanticCredentialReadiness.RESOLVED: + return schema + if configuration is None or not configuration.password: + return schema + # Discover only after provider-specific required-field checks. + # Use the supplied transport for any HTTP request. + return schema +``` + +Apply the same positive identity check in `get_runtime_schema(configuration, +runtime_data=None, *, transport, readiness)`. Strings such as `"resolved"`, booleans +and unrelated objects must not enable discovery. A static schema must perform no +network activity or credential lookup and must not echo secrets in defaults/options. +Readiness is neither complete configuration nor authenticated/authorized access. +Providers still validate required fields and allowlisted runtime overrides. + +The host checks submitted preview values before partial validation can discard an +unresolved field. A preview does not fetch or restore saved secrets. Runtime +schemas use the saved connection after the existing layer access check. The host +alone understands its masking convention; client payload fields never select the +trusted readiness keyword. Static masking and fallback calls explicitly pass +UNRESOLVED. Direct SDK hosts must supply an explicit state using their own secret +resolution rules, without adopting Superset's mask. + +This is a coordinated schema API upgrade. Update all overrides and direct callers +with the host/core pin containing transport and readiness; there is no fallback +that retries without safety arguments. The factory's transport/context/credentials +contract is unchanged. No SQL parser limit or memo policy is added to the SDK. + +## Error signals + +| Situation | Signal | Host behavior | +| --- | --- | --- | +| Invalid or unsupported query intent | `SemanticQueryRejectedError` with a `SemanticQueryErrorCode` | Fixed actionable guidance, HTTP 400 | +| Transport failure | Original SDK `ProviderRequestError` | Its sanitized message, query HTTP 400 | +| Terminal execution failure or invalid result | `SemanticBackendError()` | Fixed generic backend guidance, HTTP 500 | +| Incomplete or unverifiable result coverage | `SemanticResultCompletenessError` | Existing completeness guidance; never partial success | + +Use INVALID_QUERY for a rejected query payload, including query compilation errors. +Keep a result-page coverage failure separate from initial/poll query rejection. +If a retrieval budget prevents coverage proof, retain the provider's completeness +classification; other transport errors propagate unchanged. A failed remote status +is not Python cancellation. Preserve security, OAuth and cancellation control flow. +Never retry without a filter or return an empty/partial successful result on failure. +MCP preserves its QueryError envelope with host-safe guidance. + +Raise newly translated backend errors/refusals `from None`; never place backend +responses, credentials or SQL in their public text. The host semantic execution +boundary does not log exception causes, contexts or tracebacks, even when a plugin +chains a sensitive cause. It logs fixed event categories. Transport passthrough +is restricted to the SDK exception type; a message that merely looks safe does +not qualify. Independent final-tree security review remains required for changes +that alter these boundaries. diff --git a/docs/developer_docs/extensions/semantic-provider-transport.md b/docs/developer_docs/extensions/semantic-provider-transport.md index 22646e7c96a9..948d2de8b953 100644 --- a/docs/developer_docs/extensions/semantic-provider-transport.md +++ b/docs/developer_docs/extensions/semantic-provider-transport.md @@ -32,8 +32,8 @@ Providers import only `superset_core` for this capability, never host HTTP helpe 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)` +- `get_configuration_schema(configuration=None, *, transport, readiness)` +- `get_runtime_schema(configuration, runtime_data=None, *, transport, readiness)` 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 @@ -152,3 +152,5 @@ class FakeTransport(ProviderTransport): Add fixture-specific failures by raising `ProviderRequestError`, rather than returning a non-2xx `ProviderResponse`. Keep credentials out of recorded calls. + +Schema hooks also require the host-normalized [credential readiness signal](semantic-provider-boundary.md). Transport errors retain their safe text at the query boundary; providers must not replace them with a generic backend error. diff --git a/superset-core/src/superset_core/semantic_layers/__init__.py b/superset-core/src/superset_core/semantic_layers/__init__.py index 2ccd96b58d9d..bb96c593ca6d 100644 --- a/superset-core/src/superset_core/semantic_layers/__init__.py +++ b/superset-core/src/superset_core/semantic_layers/__init__.py @@ -17,6 +17,8 @@ """Semantic layer contracts for extension authors.""" +from superset_core.semantic_layers.config import SemanticCredentialReadiness +from superset_core.semantic_layers.errors import SemanticBackendError from superset_core.semantic_layers.transport import ( ProviderRequestError, ProviderRequestFailure, @@ -26,6 +28,8 @@ ) __all__: list[str] = [ + "SemanticBackendError", + "SemanticCredentialReadiness", "ProviderRequestError", "ProviderRequestFailure", "ProviderResponse", diff --git a/superset-core/src/superset_core/semantic_layers/config.py b/superset-core/src/superset_core/semantic_layers/config.py index 7c9aa520b118..6efc632d126f 100644 --- a/superset-core/src/superset_core/semantic_layers/config.py +++ b/superset-core/src/superset_core/semantic_layers/config.py @@ -17,11 +17,23 @@ from __future__ import annotations +from enum import Enum from typing import Any from pydantic import BaseModel +class SemanticCredentialReadiness(Enum): + """Host-normalized secret resolution, not authentication or authorization. + + Providers must check identity with RESOLVED before discovery, then validate + their required fields. Strings and truthy values never authorize discovery. + """ + + UNRESOLVED = "unresolved" + RESOLVED = "resolved" + + def build_configuration_schema( config_class: type[BaseModel], configuration: BaseModel | None = None, diff --git a/superset-core/src/superset_core/semantic_layers/errors.py b/superset-core/src/superset_core/semantic_layers/errors.py index 4fececc99d5a..9017ed323bec 100644 --- a/superset-core/src/superset_core/semantic_layers/errors.py +++ b/superset-core/src/superset_core/semantic_layers/errors.py @@ -74,3 +74,19 @@ def __init__( def __str__(self) -> str: """Return a stable explanation without arbitrary provider text.""" return "Semantic query rejected." + + +class SemanticBackendError(Exception): + """Signal execution or invalid-result failure without provider diagnostics. + + Use query rejection for invalid input, completeness for uncertain coverage, + and let ProviderRequestError retain its safe transport detail. Translate + backend failures with ``raise SemanticBackendError() from None``. + """ + + def __init__(self) -> None: + super().__init__() + + def __str__(self) -> str: + """Return fixed text independent of provider diagnostics.""" + return "Semantic provider execution failed." diff --git a/superset-core/src/superset_core/semantic_layers/layer.py b/superset-core/src/superset_core/semantic_layers/layer.py index 2ce548897919..d3151be11ae6 100644 --- a/superset-core/src/superset_core/semantic_layers/layer.py +++ b/superset-core/src/superset_core/semantic_layers/layer.py @@ -24,6 +24,7 @@ from pydantic import BaseModel +from superset_core.semantic_layers.config import SemanticCredentialReadiness from superset_core.semantic_layers.context import ( ExecutionMethod, SemanticCredentials, @@ -140,10 +141,16 @@ def get_configuration_schema( configuration: ConfigT | None = None, *, transport: ProviderTransport, + readiness: SemanticCredentialReadiness, ) -> dict[str, Any]: """ Get the JSON schema for the configuration needed to add the semantic layer. + Only ``readiness is SemanticCredentialReadiness.RESOLVED`` permits + discovery, and required configuration fields must still be valid. + Otherwise return the static schema without I/O. Hosts own secret + masking/resolution; providers must not interpret host placeholders. + A partial configuration `configuration` can be sent to improve the schema, allowing for progressive validation and better UX. For example, a semantic layer might require: @@ -171,10 +178,15 @@ def get_runtime_schema( runtime_data: dict[str, Any] | None = None, *, transport: ProviderTransport, + readiness: SemanticCredentialReadiness, ) -> dict[str, Any]: """ Get the JSON schema for the runtime parameters needed to load semantic views. + Apply the same identity check and no-I/O static behavior as + ``get_configuration_schema``. Readiness never grants access or permits + runtime overrides of credentials. + This returns the schema needed to connect to a semantic view given the configuration for the semantic layer. For example, a semantic layer might be configured by: diff --git a/superset-core/tests/semantic_layers/test_provider_boundary.py b/superset-core/tests/semantic_layers/test_provider_boundary.py new file mode 100644 index 000000000000..fdc0b4e7d104 --- /dev/null +++ b/superset-core/tests/semantic_layers/test_provider_boundary.py @@ -0,0 +1,62 @@ +# 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. + +"""Public provider readiness and fault contracts.""" + +from enum import Enum +from inspect import Parameter, signature + +import pytest +from superset_core.semantic_layers import ( + SemanticBackendError, + SemanticCredentialReadiness, +) +from superset_core.semantic_layers.layer import SemanticLayer + + +def test_readiness_is_not_a_string_or_boolean() -> None: + """Readiness cannot compare equal to an untrusted scalar.""" + assert issubclass(SemanticCredentialReadiness, Enum) + assert not issubclass(SemanticCredentialReadiness, str) + assert SemanticCredentialReadiness.RESOLVED != "resolved" + assert SemanticCredentialReadiness.RESOLVED != True # noqa: E712 + assert ( + SemanticCredentialReadiness.RESOLVED + is not SemanticCredentialReadiness.UNRESOLVED + ) + + +@pytest.mark.parametrize("method", ["get_configuration_schema", "get_runtime_schema"]) +def test_schema_readiness_and_transport_are_required_keywords(method: str) -> None: + """Both discovery capabilities must be supplied explicitly.""" + parameters: dict[str, Parameter] = dict( + signature(getattr(SemanticLayer, method)).parameters + ) + name: str + for name in ("readiness", "transport"): + assert parameters[name].kind is Parameter.KEYWORD_ONLY + assert parameters[name].default is Parameter.empty + with pytest.raises(TypeError, match="readiness"): + getattr(SemanticLayer, method)({}, transport=object()) + + +def test_backend_error_has_no_diagnostic_message_argument() -> None: + """Backend faults expose only fixed text and reject diagnostic input.""" + error: SemanticBackendError = SemanticBackendError() + assert str(error) == "Semantic provider execution failed." + with pytest.raises(TypeError): + SemanticBackendError("private diagnostics") # type: ignore[call-arg] diff --git a/superset/semantic_layers/api.py b/superset/semantic_layers/api.py index a266acdce23b..9a2cc78a7c66 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.config import SemanticCredentialReadiness from superset_core.semantic_layers.transport import ProviderRequestError from superset import db, event_logger, is_feature_enabled, security_manager @@ -67,7 +68,7 @@ from superset.exceptions import SupersetSecurityException from superset.models.core import Database from superset.semantic_layers.access import is_semantic_layers_enabled -from superset.semantic_layers.masking import mask_configuration +from superset.semantic_layers.masking import credential_readiness, mask_configuration from superset.semantic_layers.models import SemanticLayer, SemanticView from superset.semantic_layers.registry import registry from superset.semantic_layers.schemas import ( @@ -757,15 +758,19 @@ def configuration_schema(self) -> FlaskResponse: # configuring a layer, so it requires write access; read-only callers # get the static schema. parsed_config = None + readiness: SemanticCredentialReadiness = SemanticCredentialReadiness.UNRESOLVED if (config := body.get("configuration")) and security_manager.can_access( "can_write", "SemanticLayer" ): + readiness = credential_readiness(config) parsed_config = _parse_partial_config(cls, config) + if parsed_config is None: + readiness = SemanticCredentialReadiness.UNRESOLVED warning: str | None = None try: schema: dict[str, Any] = cls.get_configuration_schema( - parsed_config, transport=build_provider_transport() + parsed_config, transport=build_provider_transport(), readiness=readiness ) except Exception: # pylint: disable=broad-except # Show a stable, user-friendly message in the UI; the exception @@ -783,7 +788,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, transport=build_provider_transport() + None, + transport=build_provider_transport(), + readiness=SemanticCredentialReadiness.UNRESOLVED, ) payload: dict[str, Any] = {"result": schema} @@ -849,6 +856,7 @@ def runtime_schema(self, uuid: str) -> FlaskResponse: layer.implementation.configuration, # type: ignore[attr-defined] runtime_data, transport=build_provider_transport(), + readiness=credential_readiness(json.loads(layer.configuration)), ) except SupersetSecurityException as ex: return self.response(403, message=ex.message, errors=[ex.to_dict()]) diff --git a/superset/semantic_layers/exceptions.py b/superset/semantic_layers/exceptions.py index 0fa472315e92..41ec8ebbd61d 100644 --- a/superset/semantic_layers/exceptions.py +++ b/superset/semantic_layers/exceptions.py @@ -24,9 +24,11 @@ from billiard.exceptions import SoftTimeLimitExceeded from flask_babel import gettext as _ from superset_core.semantic_layers.errors import ( + SemanticBackendError, SemanticQueryErrorCode, SemanticQueryRejectedError, ) +from superset_core.semantic_layers.transport import ProviderRequestError from superset_core.semantic_layers.types import SemanticResult from superset.errors import ErrorLevel, SupersetError, SupersetErrorType @@ -110,7 +112,12 @@ def execute_semantic_query( with provider_completeness(): return dispatcher(*args, **kwargs) except SemanticQueryRejectedError as ex: - raise SemanticLayerQueryRejectedError(ex.code) from ex + raise SemanticLayerQueryRejectedError(ex.code) from None + except ProviderRequestError as ex: + raise QueryObjectValidationError(str(ex)) from None + except SemanticBackendError: + logger.error("Semantic provider reported an execution failure") + raise SemanticLayerExecutionError() from None except ( QueryObjectValidationError, SemanticLayerQueryRejectedError, @@ -122,6 +129,6 @@ def execute_semantic_query( SoftTimeLimitExceeded, ): raise - except Exception as ex: # pylint: disable=broad-except - logger.exception("Semantic provider query execution failed") - raise SemanticLayerExecutionError() from ex + except Exception: # pylint: disable=broad-except + logger.error("Semantic provider query execution failed") + raise SemanticLayerExecutionError() from None diff --git a/superset/semantic_layers/masking.py b/superset/semantic_layers/masking.py index 7280a59f5c45..3a89cde81005 100644 --- a/superset/semantic_layers/masking.py +++ b/superset/semantic_layers/masking.py @@ -63,6 +63,7 @@ import logging from typing import Any +from superset_core.semantic_layers.config import SemanticCredentialReadiness from superset_core.semantic_layers.layer import SemanticLayer as CoreSemanticLayer from superset.constants import PASSWORD_MASK @@ -363,7 +364,8 @@ def mask_configuration(layer_type: str, configuration: Any) -> dict[str, Any]: # 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( - transport=build_provider_transport() + transport=build_provider_transport(), + readiness=SemanticCredentialReadiness.UNRESOLVED, ) except Exception: # pylint: disable=broad-except logger.warning( @@ -391,6 +393,21 @@ def _contains_mask(value: Any) -> bool: return value == PASSWORD_MASK +def credential_readiness(configuration: Any) -> SemanticCredentialReadiness: + """Normalize raw host configuration before provider validation loses fields. + + This does not resolve saved secrets or check required provider fields. Only + the host interprets its sentinel; client assertions never set this signal. + """ + if ( + not isinstance(configuration, dict) + or not configuration + or _contains_mask(configuration) + ): + return SemanticCredentialReadiness.UNRESOLVED + return SemanticCredentialReadiness.RESOLVED + + def _matches_visible_values(reference: Any, submitted: Any) -> bool: """Compare clear values without treating a guessed field as a stable key.""" if reference == PASSWORD_MASK or submitted == PASSWORD_MASK: diff --git a/superset/translations/messages.pot b/superset/translations/messages.pot index 99f89528d4b3..81507c056e95 100644 --- a/superset/translations/messages.pot +++ b/superset/translations/messages.pot @@ -15702,6 +15702,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/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 b07bb2a556f5..83a06e16f1f7 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 @@ -36,6 +36,7 @@ import pyarrow as pa from pydantic import BaseModel, ConfigDict +from superset_core.semantic_layers.config import SemanticCredentialReadiness from superset_core.semantic_layers.context import ( SemanticCredentials, SemanticExecutionContext, @@ -267,6 +268,7 @@ def get_configuration_schema( configuration: StubConfiguration | None = None, *, transport: ProviderTransport, + readiness: SemanticCredentialReadiness, ) -> dict[str, Any]: return StubConfiguration.model_json_schema() @@ -277,6 +279,7 @@ def get_runtime_schema( runtime_data: dict[str, Any] | None = None, *, transport: ProviderTransport, + readiness: SemanticCredentialReadiness, ) -> dict[str, Any]: return {"type": "object", "properties": {}} diff --git a/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py b/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py index ff48602ba2ab..983fd5979260 100644 --- a/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py +++ b/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py @@ -36,7 +36,13 @@ import pytz from dateutil import tz as dateutil_tz from fastmcp import Client, FastMCP +from fastmcp.client.client import CallToolResult from flask import current_app +from superset_core.semantic_layers.errors import SemanticBackendError +from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure, +) from superset_core.semantic_layers.types import Dimension, Grains from superset.commands.exceptions import CommandException @@ -50,6 +56,7 @@ SemanticLayerError, ) from superset.security.guest_token import GuestToken, GuestTokenRlsRule, GuestUser +from superset.semantic_layers.exceptions import execute_semantic_query from superset.semantic_layers.models import SemanticView from superset.utils import json from superset.utils.core import GenericDataType @@ -2603,3 +2610,41 @@ async def test_get_table_normalizes_nested_builtin_cells( assert data["source"] == "builtin" assert data["dataset_id"] == 42 assert data["dataset_name"] == mock_ds.table_name + + +@pytest.mark.asyncio +@pytest.mark.parametrize("transport", [False, True]) +async def test_sdk_fault_retains_mcp_query_error_without_cause( + mcp_server: FastMCP, + monkeypatch: pytest.MonkeyPatch, + app_context: None, + transport: bool, +) -> None: + """The MCP entry point preserves safe host errors in its QueryError envelope.""" + error: Exception = ( + ProviderRequestError( + ProviderRequestFailure.HTTP_STATUS, status=403, provider="MetricFlow" + ) + if transport + else SemanticBackendError() + ) + error.__cause__ = ValueError("private-provider-cause") + dispatcher: Mock = Mock(side_effect=error) + + async def fail_query(*args: Any, **kwargs: Any) -> None: + execute_semantic_query(dispatcher) + + monkeypatch.setattr(get_table_module, "_run_get_table_query", fail_query) + client: Client + async with Client(mcp_server) as client: + result: CallToolResult = await client.call_tool( + "get_table", {"request": {"dataset_id": 42, "metrics": ["revenue"]}} + ) + returned: SemanticLayerError = SemanticLayerError.model_validate( + json.loads(result.content[0].text) + ) + assert returned.error_type == "QueryError" + assert "private-provider" not in returned.message + if transport: + assert str(error) in returned.message + dispatcher.assert_called_once() diff --git a/tests/unit_tests/semantic_layers/api_test.py b/tests/unit_tests/semantic_layers/api_test.py index dec9023e9204..30de69c206e8 100644 --- a/tests/unit_tests/semantic_layers/api_test.py +++ b/tests/unit_tests/semantic_layers/api_test.py @@ -366,7 +366,9 @@ 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, transport=ANY) + mock_cls.get_configuration_schema.assert_called_once_with( + None, transport=ANY, readiness=ANY + ) @SEMANTIC_LAYERS_APP @@ -406,7 +408,7 @@ def test_configuration_schema_with_partial_config( assert response.status_code == 200 mock_cls.get_configuration_schema.assert_called_once_with( - mock_config_obj, transport=ANY + mock_config_obj, transport=ANY, readiness=ANY ) @@ -439,7 +441,9 @@ 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, transport=ANY) + mock_cls.get_configuration_schema.assert_called_once_with( + None, transport=ANY, readiness=ANY + ) @SEMANTIC_LAYERS_APP @@ -477,7 +481,7 @@ def test_configuration_schema_writer_gets_enriched_schema( 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, transport=ANY + mock_config_obj, transport=ANY, readiness=ANY ) @@ -515,7 +519,9 @@ def test_configuration_schema_with_invalid_partial_config( ) assert response.status_code == 200 - mock_cls.get_configuration_schema.assert_called_once_with(None, transport=ANY) + mock_cls.get_configuration_schema.assert_called_once_with( + None, transport=ANY, readiness=ANY + ) @SEMANTIC_LAYERS_APP @@ -549,6 +555,7 @@ def test_runtime_schema( test_uuid = str(uuid_lib.uuid4()) mock_layer = MagicMock() mock_layer.type = "snowflake" + mock_layer.configuration = '{"account": "test"}' mock_layer.implementation.configuration = {"account": "test"} mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO") @@ -571,7 +578,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"}, transport=ANY + {"account": "test"}, {"database": "mydb"}, transport=ANY, readiness=ANY ) @@ -585,6 +592,7 @@ def test_runtime_schema_no_body( test_uuid = str(uuid_lib.uuid4()) mock_layer = MagicMock() mock_layer.type = "snowflake" + mock_layer.configuration = '{"account": "test"}' mock_layer.implementation.configuration = {"account": "test"} mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO") @@ -605,7 +613,7 @@ def test_runtime_schema_no_body( assert response.status_code == 200 mock_cls.get_runtime_schema.assert_called_once_with( - {"account": "test"}, None, transport=ANY + {"account": "test"}, None, transport=ANY, readiness=ANY ) @@ -693,6 +701,7 @@ def test_runtime_schema_exception( test_uuid: str = str(uuid_lib.uuid4()) mock_layer: MagicMock = MagicMock() mock_layer.type = "snowflake" + mock_layer.configuration = '{"account": "test"}' mock_layer.implementation.configuration = {"account": "test"} mock_dao: MagicMock = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO") @@ -1132,7 +1141,9 @@ def test_get_list_semantic_layers( "superset.semantic_layers.api.registry", { "snowflake": MagicMock( - get_configuration_schema=lambda *, transport: {"properties": {}} + get_configuration_schema=lambda *, transport, readiness: { + "properties": {} + } ) }, clear=True, @@ -1201,7 +1212,9 @@ def test_get_semantic_layer( "superset.semantic_layers.api.registry", { "snowflake": MagicMock( - get_configuration_schema=lambda *, transport: {"properties": {}} + get_configuration_schema=lambda *, transport, readiness: { + "properties": {} + } ) }, clear=True, @@ -1295,7 +1308,9 @@ def test_serialize_layer_string_config( "superset.semantic_layers.api.registry", { "snowflake": MagicMock( - get_configuration_schema=lambda *, transport: {"properties": {}} + get_configuration_schema=lambda *, transport, readiness: { + "properties": {} + } ) }, clear=True, @@ -1329,7 +1344,9 @@ def test_serialize_layer_dict_config( "superset.semantic_layers.api.registry", { "snowflake": MagicMock( - get_configuration_schema=lambda *, transport: {"properties": {}} + get_configuration_schema=lambda *, transport, readiness: { + "properties": {} + } ) }, clear=True, @@ -3453,6 +3470,7 @@ def test_metadata_endpoints_preserve_only_typed_transport_errors( layer_uuid: str = str(uuid_lib.uuid4()) layer: MagicMock = MagicMock() layer.type = "fixture" + layer.configuration = '{"account": "fixture"}' dao: MagicMock = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO") dao.find_by_uuid.return_value = layer provider: MagicMock = MagicMock() diff --git a/tests/unit_tests/semantic_layers/conftest.py b/tests/unit_tests/semantic_layers/conftest.py index c62d25b1179e..6ab015741209 100644 --- a/tests/unit_tests/semantic_layers/conftest.py +++ b/tests/unit_tests/semantic_layers/conftest.py @@ -24,6 +24,7 @@ import pyarrow as pa from pydantic import BaseModel +from superset_core.semantic_layers.config import SemanticCredentialReadiness from superset_core.semantic_layers.context import ( ExecutionOrigin, SemanticCredentials, @@ -165,7 +166,11 @@ def from_configuration( @classmethod def get_configuration_schema( - cls, configuration: BaseModel | None = None, *, transport: ProviderTransport + cls, + configuration: BaseModel | None = None, + *, + transport: ProviderTransport, + readiness: SemanticCredentialReadiness, ) -> dict[str, Any]: return {"type": "object"} @@ -176,6 +181,7 @@ def get_runtime_schema( runtime_data: dict[str, Any] | None = None, *, transport: ProviderTransport, + readiness: SemanticCredentialReadiness, ) -> dict[str, Any]: return {"type": "object"} diff --git a/tests/unit_tests/semantic_layers/masking_test.py b/tests/unit_tests/semantic_layers/masking_test.py index 2ed4977ebd41..7afb0256ad43 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.config import SemanticCredentialReadiness from superset_core.semantic_layers.transport import ProviderTransport from superset.constants import PASSWORD_MASK @@ -60,7 +61,10 @@ class DemoConfig(BaseModel): class DemoLayerType: @staticmethod def get_configuration_schema( - configuration: Any = None, *, transport: ProviderTransport + configuration: Any = None, + *, + transport: ProviderTransport, + readiness: SemanticCredentialReadiness, ) -> Any: return DemoConfig.model_json_schema() @@ -203,7 +207,10 @@ class DivergentConfig(BaseModel): class DivergentType: @staticmethod def get_configuration_schema( - configuration: Any = None, *, transport: ProviderTransport + configuration: Any = None, + *, + transport: ProviderTransport, + readiness: SemanticCredentialReadiness, ) -> Any: return DivergentConfig.model_json_schema() @@ -331,7 +338,11 @@ def __init__(self, schema: Any = None, raises: bool = False) -> None: self._raises: bool = raises def get_configuration_schema( - self, configuration: Any = None, *, transport: ProviderTransport + self, + configuration: Any = None, + *, + transport: ProviderTransport, + readiness: SemanticCredentialReadiness, ) -> Any: if self._raises: raise RuntimeError("schema generation failed") @@ -412,7 +423,10 @@ class DivergentListConfig(BaseModel): class DivergentListType: @staticmethod def get_configuration_schema( - configuration: Any = None, *, transport: ProviderTransport + configuration: Any = None, + *, + transport: ProviderTransport, + readiness: SemanticCredentialReadiness, ) -> Any: return DivergentListConfig.model_json_schema() diff --git a/tests/unit_tests/semantic_layers/provider_boundary_test.py b/tests/unit_tests/semantic_layers/provider_boundary_test.py new file mode 100644 index 000000000000..93c251941f1d --- /dev/null +++ b/tests/unit_tests/semantic_layers/provider_boundary_test.py @@ -0,0 +1,243 @@ +# 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. + +"""Trusted readiness and safe failure handling at real host entry points.""" + +import logging +from unittest.mock import MagicMock + +import pytest +from flask.testing import FlaskClient +from pydantic import BaseModel +from pytest_mock import MockerFixture +from superset_core.semantic_layers import ( + SemanticBackendError, + SemanticCredentialReadiness, +) +from superset_core.semantic_layers.errors import SemanticQueryRejectedError +from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure, +) +from werkzeug.test import TestResponse + +from superset.constants import PASSWORD_MASK +from superset.exceptions import QueryObjectValidationError +from superset.semantic_layers.exceptions import ( + execute_semantic_query, + SemanticLayerExecutionError, + SemanticLayerQueryRejectedError, +) +from superset.semantic_layers.masking import credential_readiness, mask_configuration +from superset.utils import json + + +class PreviewConfiguration(BaseModel): + """Deliberately discard unknown fields like a real partial provider model.""" + + account: str | None = None + password: str | None = None + + +@pytest.mark.parametrize( + "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True +) +@pytest.mark.parametrize("writer", [False, True]) +@pytest.mark.parametrize("masked", [False, True]) +def test_preview_readiness_uses_raw_input_not_client_assertion( + client: FlaskClient, + full_api_access: None, + mocker: MockerFixture, + writer: bool, + masked: bool, +) -> None: + """Normalize before dropped fields, without looking up saved credentials.""" + provider: MagicMock = MagicMock() + provider.configuration_class = PreviewConfiguration + provider.get_configuration_schema.return_value = {"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, + ) + lookup: MagicMock = mocker.patch( + "superset.semantic_layers.api.SemanticLayerDAO.find_by_uuid" + ) + response: TestResponse = client.post( + "/api/v1/semantic_layer/schema/configuration", + json={ + "type": "test", + "readiness": "resolved", + "configuration": { + "account": "fixture", + "password": "fixture-password", + # Pydantic drops this unknown subtree; readiness must be computed first. + "nested": [{"password": PASSWORD_MASK if masked else "fixture"}], + "readiness": "resolved", + }, + }, + ) + assert response.status_code == 200 + assert provider.get_configuration_schema.call_args.kwargs["readiness"] is ( + SemanticCredentialReadiness.RESOLVED + if writer and not masked + else SemanticCredentialReadiness.UNRESOLVED + ) + lookup.assert_not_called() + if not writer: + assert provider.get_configuration_schema.call_args.args == (None,) + + +@pytest.mark.parametrize( + "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True +) +def test_configuration_fallback_is_explicitly_unresolved( + client: FlaskClient, + full_api_access: None, + mocker: MockerFixture, +) -> None: + """A failed enrichment retries only a static unresolved schema.""" + provider: MagicMock = MagicMock() + provider.configuration_class = PreviewConfiguration + provider.get_configuration_schema.side_effect = [ + ValueError("fixture error"), + {"type": "object"}, + ] + mocker.patch.dict("superset.semantic_layers.api.registry", {"test": provider}) + 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_args.args == (None,) + assert ( + provider.get_configuration_schema.call_args.kwargs["readiness"] + is SemanticCredentialReadiness.UNRESOLVED + ) + + +@pytest.mark.parametrize( + "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True +) +@pytest.mark.parametrize("masked", [False, True]) +def test_runtime_readiness_uses_saved_raw_configuration( + client: FlaskClient, + full_api_access: None, + mocker: MockerFixture, + masked: bool, +) -> None: + """Client runtime data cannot override readiness of the saved connection.""" + layer: MagicMock = MagicMock(type="test") + layer.configuration = json.dumps( + {"password": PASSWORD_MASK if masked else "fixture"} + ) + layer.implementation.configuration = PreviewConfiguration(account="fixture") + 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={"readiness": "resolved", "runtime_data": {"readiness": "resolved"}}, + ) + assert response.status_code == 200 + layer.raise_for_access.assert_called_once() + assert provider.get_runtime_schema.call_args.kwargs["readiness"] is ( + SemanticCredentialReadiness.UNRESOLVED + if masked + else SemanticCredentialReadiness.RESOLVED + ) + + +def test_masking_schema_is_explicitly_unresolved(mocker: MockerFixture) -> None: + """Masking asks for a static schema with unresolved readiness.""" + provider: MagicMock = MagicMock() + provider.get_configuration_schema.return_value = { + "properties": {"password": {"writeOnly": True}} + } + mocker.patch.dict("superset.semantic_layers.masking.registry", {"test": provider}) + assert mask_configuration("test", {"password": "fixture"}) == { + "password": PASSWORD_MASK + } + assert ( + provider.get_configuration_schema.call_args.kwargs["readiness"] + is SemanticCredentialReadiness.UNRESOLVED + ) + + +@pytest.mark.parametrize("status", [401, 403, 502]) +def test_transport_keeps_safe_text_and_suppresses_chain( + app_context: None, status: int +) -> None: + """Typed transport diagnostics survive without exporting an exception chain.""" + error: ProviderRequestError = ProviderRequestError( + ProviderRequestFailure.HTTP_STATUS, status=status, provider="MetricFlow" + ) + error.__cause__ = ValueError("private-cause") + dispatcher: MagicMock = MagicMock(side_effect=error) + captured: pytest.ExceptionInfo[QueryObjectValidationError] + with pytest.raises(QueryObjectValidationError) as captured: + execute_semantic_query(dispatcher) + assert str(captured.value) == str(error) + assert captured.value.__cause__ is None + assert captured.value.__suppress_context__ + + +@pytest.mark.parametrize("kind", ["backend", "unexpected", "refusal"]) +def test_execution_does_not_log_sensitive_cause_or_context( + app_context: None, + caplog: pytest.LogCaptureFixture, + kind: str, +) -> None: + """Typed and unknown provider failures never export diagnostic chains.""" + error: Exception = { + "backend": SemanticBackendError(), + "unexpected": RuntimeError("private-error"), + "refusal": SemanticQueryRejectedError("INVALID_QUERY"), + }[kind] + error.__cause__ = ValueError("private-cause") + error.__context__ = ValueError("private-context") + dispatcher: MagicMock = MagicMock(side_effect=error) + captured: pytest.ExceptionInfo[ + SemanticLayerExecutionError | SemanticLayerQueryRejectedError + ] + with ( + caplog.at_level(logging.ERROR), + pytest.raises( + (SemanticLayerExecutionError, SemanticLayerQueryRejectedError) + ) as captured, + ): + execute_semantic_query(dispatcher) + assert captured.value.status == (400 if kind == "refusal" else 500) + assert captured.value.__cause__ is None + assert captured.value.__suppress_context__ + assert "private" not in caplog.text + assert "private" not in str(captured.value.to_dict()) + assert all(record.exc_info is None for record in caplog.records) + + +@pytest.mark.parametrize("configuration", [None, {}, [], "resolved", True]) +def test_absent_or_non_object_configuration_is_unresolved( + configuration: object, +) -> None: + """Invalid raw configuration never attests resolved secrets.""" + assert credential_readiness(configuration) is SemanticCredentialReadiness.UNRESOLVED diff --git a/tests/unit_tests/semantic_layers/provider_masking_test.py b/tests/unit_tests/semantic_layers/provider_masking_test.py index 319da21b68c8..4bfb5e667d5c 100644 --- a/tests/unit_tests/semantic_layers/provider_masking_test.py +++ b/tests/unit_tests/semantic_layers/provider_masking_test.py @@ -76,7 +76,11 @@ 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 *, transport: schema)}, + { + "snowflake": MagicMock( + get_configuration_schema=lambda *, transport, readiness: schema + ) + }, clear=True, ) mocker.patch( diff --git a/tests/unit_tests/semantic_layers/query_errors_test.py b/tests/unit_tests/semantic_layers/query_errors_test.py index 9e49721ada8c..05e0df2cefa6 100644 --- a/tests/unit_tests/semantic_layers/query_errors_test.py +++ b/tests/unit_tests/semantic_layers/query_errors_test.py @@ -24,7 +24,14 @@ import pytest from flask.testing import FlaskClient from pytest_mock import MockerFixture -from superset_core.semantic_layers.errors import SemanticQueryRejectedError +from superset_core.semantic_layers.errors import ( + SemanticBackendError, + SemanticQueryRejectedError, +) +from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure, +) from superset_core.semantic_layers.types import Dimension, Metric, SemanticResult from superset_core.semantic_layers.view import SemanticView as ProviderView from werkzeug.test import TestResponse @@ -305,8 +312,8 @@ def test_provider_execution_error_http( ] assert len(records) == 1 assert records[0].getMessage() == "Semantic provider query execution failed" - assert records[0].exc_info is not None - assert records[0].exc_info[1] is date_view.get_table.side_effect + assert records[0].exc_info is None + assert "private-provider" not in caplog.text date_view.get_table.assert_called_once() @@ -836,3 +843,82 @@ def test_async_provider_error_matches_sync_chart_error( assert "private" not in str(captured.value.to_dict()) task_context.update_task.assert_not_called() date_view.get_table.assert_called_once() + + +@pytest.mark.parametrize( + "kind,status", [("backend", 500), ("transport", 400), ("refusal", 400)] +) +def test_sdk_failure_chart_status_and_no_success_cache( + client: FlaskClient, + full_api_access: None, + date_view: MagicMock, + mocker: MockerFixture, + caplog: pytest.LogCaptureFixture, + kind: str, + status: int, +) -> None: + """Real chart execution preserves SDK failure categories without cache writes.""" + error: Exception = { + "backend": SemanticBackendError(), + "transport": ProviderRequestError( + ProviderRequestFailure.HTTP_STATUS, status=401, provider="MetricFlow" + ), + "refusal": SemanticQueryRejectedError("INVALID_QUERY"), + }[kind] + error.__cause__ = ValueError("private-provider-cause") + date_view.get_table.side_effect = error + cache_write: MagicMock = mocker.patch( + "superset.common.query_context_processor.QueryCacheManager.set_query_result" + ) + response: TestResponse = client.post( + "/api/v1/chart/data", + json={ + "datasource": {"id": 7, "type": "semantic_view"}, + "queries": [{"columns": ["ORDER_DATE"], "metrics": ["ORDER_COUNT"]}], + "result_format": "json", + "result_type": "full", + "force": True, + }, + ) + assert response.status_code == status + assert "private-provider" not in response.get_data(as_text=True) + assert "private-provider" not in caplog.text + if kind == "transport": + assert str(error) in response.get_data(as_text=True) + cache_write.assert_not_called() + date_view.get_table.assert_called_once() + + +@pytest.mark.parametrize("transport", [False, True]) +def test_sdk_failure_values_lookup_keeps_category_and_never_retries( + date_view: MagicMock, + caplog: pytest.LogCaptureFixture, + transport: bool, +) -> None: + """Filter suggestions preserve the same safe SDK failure boundary as charts.""" + from superset.exceptions import QueryObjectValidationError + from superset.semantic_layers.exceptions import SemanticLayerExecutionError + + error: Exception = ( + ProviderRequestError( + ProviderRequestFailure.HTTP_STATUS, status=403, provider="MetricFlow" + ) + if transport + else SemanticBackendError() + ) + error.__cause__ = ValueError("private-provider-cause") + date_view.get_values.side_effect = error + expected: type[Exception] = ( + QueryObjectValidationError if transport else SemanticLayerExecutionError + ) + view: SemanticView = SemanticView(id=7, name="Orders", configuration="{}") + caught: pytest.ExceptionInfo[Exception] + with pytest.raises(expected) as caught: + view.values_for_column("ORDER_DATE") + assert caught.value.__suppress_context__ + assert caught.value.__cause__ is None + assert "private-provider" not in str(caught.value) + assert "private-provider" not in caplog.text + if transport: + assert str(caught.value) == str(error) + date_view.get_values.assert_called_once() diff --git a/tests/unit_tests/semantic_layers/transport_wiring_test.py b/tests/unit_tests/semantic_layers/transport_wiring_test.py index 69540c906d38..8c467b95226c 100644 --- a/tests/unit_tests/semantic_layers/transport_wiring_test.py +++ b/tests/unit_tests/semantic_layers/transport_wiring_test.py @@ -18,7 +18,7 @@ """Every host construction/discovery boundary supplies the same SDK capability.""" from typing import Any -from unittest.mock import MagicMock, PropertyMock +from unittest.mock import ANY, MagicMock, PropertyMock import pytest from pytest_mock import MockerFixture @@ -69,7 +69,7 @@ def test_configuration_paths_inject_transport_preserving_static_gate( 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 call.kwargs == {"transport": capability, "readiness": ANY} assert factory.call_count == provider.get_configuration_schema.call_count if not writer: provider.configuration_class.model_validate.assert_not_called() @@ -98,7 +98,7 @@ def test_runtime_schema_injects_transport_after_access( create=True, return_value=capability, ) - layer: MagicMock = MagicMock(type="test") + layer: MagicMock = MagicMock(type="test", configuration='{"account": "fixture"}') provider: MagicMock = MagicMock() provider.get_runtime_schema.return_value = {"type": "object"} mocker.patch( @@ -112,7 +112,7 @@ def test_runtime_schema_injects_transport_after_access( 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 + layer.implementation.configuration, {}, transport=capability, readiness=ANY ) factory.assert_called_once_with() @@ -131,7 +131,9 @@ def test_masking_static_schema_receives_transport(mocker: MockerFixture) -> None } 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) + provider.get_configuration_schema.assert_called_once_with( + transport=capability, readiness=ANY + ) factory.assert_called_once_with() From 7dafa0530f0bb31d22392523044d3244094365fa Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 20 Oct 2026 17:01:41 -0600 Subject: [PATCH 08/17] fix(semantic-layer): validate transport messages and retain debug diagnostics --- UPDATING.md | 3 +- .../extensions/semantic-provider-boundary.md | 17 +- superset/semantic_layers/api.py | 30 +-- superset/semantic_layers/exceptions.py | 49 +++- tests/unit_tests/semantic_layers/api_test.py | 6 +- .../semantic_layers/provider_boundary_test.py | 211 ++++++++++++++++++ .../semantic_layers/query_errors_test.py | 4 +- 7 files changed, 291 insertions(+), 29 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index edb11437538f..dbdea5025237 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -35,7 +35,8 @@ assists people when migrating to a new version. host guidance (500); `ProviderRequestError` keeps safe transport detail (400), and query refusal/completeness retain their separate meanings. Provider error translations should suppress diagnostic chains. The semantic execution boundary - logs fixed categories without provider exception tracebacks. See the + logs typed failures without diagnostic chains. Unknown failures retain their + type at ERROR and their full traceback/chain at DEBUG in server logs only. See the [provider boundary guide](docs/developer_docs/extensions/semantic-provider-boundary.md). - **Semantic-layer SDK transport argument.** Provider implementations must accept diff --git a/docs/developer_docs/extensions/semantic-provider-boundary.md b/docs/developer_docs/extensions/semantic-provider-boundary.md index dd6957bcb3c0..6315f7d6876d 100644 --- a/docs/developer_docs/extensions/semantic-provider-boundary.md +++ b/docs/developer_docs/extensions/semantic-provider-boundary.md @@ -83,9 +83,14 @@ Never retry without a filter or return an empty/partial successful result on fai MCP preserves its QueryError envelope with host-safe guidance. Raise newly translated backend errors/refusals `from None`; never place backend -responses, credentials or SQL in their public text. The host semantic execution -boundary does not log exception causes, contexts or tracebacks, even when a plugin -chains a sensitive cause. It logs fixed event categories. Transport passthrough -is restricted to the SDK exception type; a message that merely looks safe does -not qualify. Independent final-tree security review remains required for changes -that alter these boundaries. +responses, credentials or SQL in their public text. The host validates transport +failure/status fields and reconstructs messages with a freshly sanitized provider +label. Mutated fields and subclass formatters cannot supply public text; malformed +transport fields become generic backend faults. + +Typed SDK failures log fixed event categories without causes or tracebacks. +Unexpected exceptions log their type at ERROR and retain the full traceback and +exception chain at DEBUG, server-side only. Keep these diagnostic logs restricted +to trusted operators: they may contain provider details. Public responses remain +fixed generic guidance and do not include the exception chain. The same logging +policy applies to schema enrichment, runtime schemas and structure discovery. diff --git a/superset/semantic_layers/api.py b/superset/semantic_layers/api.py index 9a2cc78a7c66..ade21eb60fce 100644 --- a/superset/semantic_layers/api.py +++ b/superset/semantic_layers/api.py @@ -68,6 +68,10 @@ from superset.exceptions import SupersetSecurityException from superset.models.core import Database from superset.semantic_layers.access import is_semantic_layers_enabled +from superset.semantic_layers.exceptions import ( + log_provider_failure, + provider_request_message, +) from superset.semantic_layers.masking import credential_readiness, mask_configuration from superset.semantic_layers.models import SemanticLayer, SemanticView from superset.semantic_layers.registry import registry @@ -310,9 +314,9 @@ 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.exception("Error fetching structure for semantic view %d", pk) + log_provider_failure("Error fetching semantic view structure", ex) return self.response_422( - message=str(ex) + message=provider_request_message(ex) if isinstance(ex, ProviderRequestError) else t("Unable to fetch semantic view structure.") ) @@ -759,8 +763,10 @@ def configuration_schema(self) -> FlaskResponse: # get the static schema. parsed_config = None readiness: SemanticCredentialReadiness = SemanticCredentialReadiness.UNRESOLVED - if (config := body.get("configuration")) and security_manager.can_access( - "can_write", "SemanticLayer" + if ( + isinstance(config := body.get("configuration"), dict) + and config + and security_manager.can_access("can_write", "SemanticLayer") ): readiness = credential_readiness(config) parsed_config = _parse_partial_config(cls, config) @@ -772,18 +778,16 @@ def configuration_schema(self) -> FlaskResponse: schema: dict[str, Any] = cls.get_configuration_schema( parsed_config, transport=build_provider_transport(), readiness=readiness ) - except Exception: # pylint: disable=broad-except - # Show a stable, user-friendly message in the UI; the exception - # detail goes to the server log below. + except Exception as ex: # pylint: disable=broad-except + # Keep public guidance stable; unknown diagnostics require DEBUG logs. warning = str( t( "Could not load metadata for this configuration; " "showing the default form. See the server logs for details." ) ) - logger.exception( - "Error enriching semantic layer configuration schema for type %s", - sl_type, + log_provider_failure( + "Error enriching semantic layer configuration schema", ex ) # Connection or query failures during schema enrichment should not # prevent the form from rendering — return the base schema instead. @@ -861,11 +865,9 @@ 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 - logger.exception( - "Error fetching runtime schema for semantic layer %s", uuid - ) + log_provider_failure("Error fetching semantic layer runtime schema", ex) return self.response_400( - message=str(ex) + message=provider_request_message(ex) if isinstance(ex, ProviderRequestError) else t("Unable to fetch semantic layer runtime schema.") ) diff --git a/superset/semantic_layers/exceptions.py b/superset/semantic_layers/exceptions.py index 41ec8ebbd61d..ae637b1439d2 100644 --- a/superset/semantic_layers/exceptions.py +++ b/superset/semantic_layers/exceptions.py @@ -19,7 +19,7 @@ import logging from collections.abc import Callable -from typing import ParamSpec +from typing import Any, ParamSpec from billiard.exceptions import SoftTimeLimitExceeded from flask_babel import gettext as _ @@ -28,7 +28,10 @@ SemanticQueryErrorCode, SemanticQueryRejectedError, ) -from superset_core.semantic_layers.transport import ProviderRequestError +from superset_core.semantic_layers.transport import ( + ProviderRequestError, + ProviderRequestFailure, +) from superset_core.semantic_layers.types import SemanticResult from superset.errors import ErrorLevel, SupersetError, SupersetErrorType @@ -102,6 +105,42 @@ def __init__(self) -> None: ) +def provider_request_message(error: ProviderRequestError) -> str: + """Rebuild transport text without trusting mutable fields or subclass formatters.""" + fields: dict[str, Any] = vars(error) + failure: Any = fields.get("failure") + status: Any = fields.get("status") + provider: Any = fields.get("provider") + valid_status: bool = ( + type(status) is int and 100 <= status <= 599 + if failure is ProviderRequestFailure.HTTP_STATUS + else status is None + ) + if type(failure) is not ProviderRequestFailure or not valid_status: + logger.error("Semantic provider supplied invalid transport failure fields") + raise SemanticLayerExecutionError() from None + return str( + ProviderRequestError( + failure, + status=status, + provider=provider if type(provider) is str else "Provider", + ) + ) + + +def log_provider_failure(message: str, error: Exception) -> None: + """Keep typed failures quiet; retain unknown diagnostics in server DEBUG logs.""" + if isinstance( + error, (ProviderRequestError, SemanticBackendError, SemanticQueryRejectedError) + ): + logger.error("%s", message) + return + logger.error( + "%s (%s.%s)", message, type(error).__module__, type(error).__qualname__ + ) + logger.debug(message, exc_info=(type(error), error, error.__traceback__)) + + def execute_semantic_query( dispatcher: Callable[ProviderArguments, SemanticResult], *args: ProviderArguments.args, @@ -114,7 +153,7 @@ def execute_semantic_query( except SemanticQueryRejectedError as ex: raise SemanticLayerQueryRejectedError(ex.code) from None except ProviderRequestError as ex: - raise QueryObjectValidationError(str(ex)) from None + raise QueryObjectValidationError(provider_request_message(ex)) from None except SemanticBackendError: logger.error("Semantic provider reported an execution failure") raise SemanticLayerExecutionError() from None @@ -129,6 +168,6 @@ def execute_semantic_query( SoftTimeLimitExceeded, ): raise - except Exception: # pylint: disable=broad-except - logger.error("Semantic provider query execution failed") + except Exception as ex: # pylint: disable=broad-except + log_provider_failure("Semantic provider query execution failed", ex) raise SemanticLayerExecutionError() from None diff --git a/tests/unit_tests/semantic_layers/api_test.py b/tests/unit_tests/semantic_layers/api_test.py index 30de69c206e8..500b0a80c1db 100644 --- a/tests/unit_tests/semantic_layers/api_test.py +++ b/tests/unit_tests/semantic_layers/api_test.py @@ -697,7 +697,8 @@ def test_runtime_schema_exception( mocker: MockerFixture, caplog: pytest.LogCaptureFixture, ) -> None: - """Keep provider details in server logs, not the runtime-schema response.""" + """Keep provider details in DEBUG server logs, not the runtime-schema response.""" + caplog.set_level("DEBUG", logger="superset.semantic_layers.exceptions") test_uuid: str = str(uuid_lib.uuid4()) mock_layer: MagicMock = MagicMock() mock_layer.type = "snowflake" @@ -2852,7 +2853,8 @@ def test_get_semantic_view_structure_implementation_error( caplog: pytest.LogCaptureFixture, method: str, ) -> None: - """Keep provider details in server logs, not the structure response.""" + """Keep provider details in DEBUG server logs, not the structure response.""" + caplog.set_level("DEBUG", logger="superset.semantic_layers.exceptions") mock_view: MagicMock = MagicMock() getattr(mock_view.implementation, method).side_effect = RuntimeError( "provider-private-detail" diff --git a/tests/unit_tests/semantic_layers/provider_boundary_test.py b/tests/unit_tests/semantic_layers/provider_boundary_test.py index 93c251941f1d..f9c334044bb5 100644 --- a/tests/unit_tests/semantic_layers/provider_boundary_test.py +++ b/tests/unit_tests/semantic_layers/provider_boundary_test.py @@ -18,6 +18,7 @@ """Trusted readiness and safe failure handling at real host entry points.""" import logging +from typing import cast from unittest.mock import MagicMock import pytest @@ -241,3 +242,213 @@ def test_absent_or_non_object_configuration_is_unresolved( ) -> None: """Invalid raw configuration never attests resolved secrets.""" assert credential_readiness(configuration) is SemanticCredentialReadiness.UNRESOLVED + + +class OverriddenTransportError(ProviderRequestError): + """Represent an extension overriding the exception's public formatter.""" + + def __str__(self) -> str: + return "private-subclass-text" + + +def _untrusted_transport_error(case: str) -> ProviderRequestError: + """Construct malformed or modified SDK errors without bypassing the host.""" + error: ProviderRequestError = ProviderRequestError( + ProviderRequestFailure.HTTP_STATUS, status=403, provider="MetricFlow" + ) + if case == "status_text": + error.status = cast(int, "private-response-body") + elif case == "status_bool": + error.status = True + elif case == "status_range": + error.status = 600 + elif case == "status_missing": + error.status = None + elif case == "failure": + error.failure = cast(ProviderRequestFailure, "private-failure") + elif case == "failure_string": + error.failure = cast(ProviderRequestFailure, "http_status") + elif case == "status_on_timeout": + error.failure = ProviderRequestFailure.TIMEOUT + elif case == "provider": + error.provider = "https://private-endpoint/token" + elif case == "subclass": + error = OverriddenTransportError( + ProviderRequestFailure.HTTP_STATUS, status=403, provider="MetricFlow" + ) + error.__cause__ = ValueError("private-cause") + return error + + +@pytest.mark.parametrize( + "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True +) +@pytest.mark.parametrize("endpoint", ["query", "structure", "runtime"]) +@pytest.mark.parametrize( + "case", + [ + "status_text", + "status_bool", + "status_range", + "status_missing", + "failure", + "failure_string", + "status_on_timeout", + "provider", + "subclass", + ], +) +def test_transport_errors_rebuilt_at_every_public_boundary( + client: FlaskClient, + full_api_access: None, + mocker: MockerFixture, + caplog: pytest.LogCaptureFixture, + endpoint: str, + case: str, +) -> None: + """Neither mutable SDK fields nor a subclass formatter confer safe text.""" + error: ProviderRequestError = _untrusted_transport_error(case) + valid: bool = case in {"provider", "subclass"} + message: str + with caplog.at_level(logging.DEBUG, logger="superset.semantic_layers"): + if endpoint == "query": + captured: pytest.ExceptionInfo[ + QueryObjectValidationError | SemanticLayerExecutionError + ] + with pytest.raises( + (QueryObjectValidationError, SemanticLayerExecutionError) + ) as captured: + execute_semantic_query(MagicMock(side_effect=error)) + assert isinstance(captured.value, QueryObjectValidationError) == valid + assert captured.value.__cause__ is None + assert captured.value.__suppress_context__ + message = str(captured.value) + else: + provider: MagicMock = MagicMock() + layer: MagicMock = MagicMock( + type="test", configuration='{"account":"fixture"}' + ) + if endpoint == "structure": + layer.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 = ( + layer + ) + response: TestResponse = client.get("/api/v1/semantic_view/1/structure") + else: + provider.get_runtime_schema.side_effect = error + mocker.patch.dict( + "superset.semantic_layers.api.registry", {"test": provider} + ) + mocker.patch( + "superset.semantic_layers.api.SemanticLayerDAO.find_by_uuid", + return_value=layer, + ) + response = client.post( + "/api/v1/semantic_layer/00000000-0000-0000-0000-000000000001/schema/runtime" + ) + assert response.status_code == ( + (422 if endpoint == "structure" else 400) if valid else 500 + ) + message = response.get_data(as_text=True) + assert "private" not in message + assert "private" not in caplog.text + if valid: + assert ( + "Provider" if case == "provider" else "MetricFlow" + ) + " request failed with HTTP 403." in message + + +def test_unknown_fault_keeps_debug_traceback_and_chain_server_side( + app_context: None, + caplog: pytest.LogCaptureFixture, +) -> None: + """Unknown faults remain diagnosable without changing their public response.""" + error: AttributeError = AttributeError("private-unknown-message") + error.__cause__ = ValueError("private-original-cause") + captured: pytest.ExceptionInfo[SemanticLayerExecutionError] + with caplog.at_level(logging.DEBUG, logger="superset.semantic_layers.exceptions"): + with pytest.raises(SemanticLayerExecutionError) as captured: + execute_semantic_query(MagicMock(side_effect=error)) + records: list[logging.LogRecord] = [ + r for r in caplog.records if r.name == "superset.semantic_layers.exceptions" + ] + assert any( + r.levelno == logging.ERROR + and "builtins.AttributeError" in r.getMessage() + and r.exc_info is None + for r in records + ) + assert any( + r.levelno == logging.DEBUG + and r.exc_info + and r.exc_info[1] is error + and r.exc_info[2] is not None + for r in records + ) + assert "private-original-cause" in caplog.text + assert "private-unknown-message" in caplog.text + assert "private" not in str(captured.value.to_dict()) + assert captured.value.status == 500 + + +@pytest.mark.parametrize( + "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True +) +@pytest.mark.parametrize("configuration", [{"account": 123}, ["fixture"], "fixture"]) +def test_failed_partial_preview_downgrades_readiness( + client: FlaskClient, + full_api_access: None, + mocker: MockerFixture, + configuration: object, +) -> None: + """Non-masked input that cannot form a partial config remains unresolved.""" + provider: MagicMock = MagicMock() + provider.configuration_class = PreviewConfiguration + provider.get_configuration_schema.return_value = {"type": "object"} + mocker.patch.dict("superset.semantic_layers.api.registry", {"test": provider}) + response: TestResponse = client.post( + "/api/v1/semantic_layer/schema/configuration", + json={"type": "test", "configuration": configuration}, + ) + assert response.status_code == 200 + assert provider.get_configuration_schema.call_args.args == (None,) + assert ( + provider.get_configuration_schema.call_args.kwargs["readiness"] + is SemanticCredentialReadiness.UNRESOLVED + ) + + +@pytest.mark.parametrize( + "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True +) +@pytest.mark.parametrize("transport", [False, True]) +def test_preview_typed_failure_does_not_log_chain( + client: FlaskClient, + full_api_access: None, + mocker: MockerFixture, + caplog: pytest.LogCaptureFixture, + transport: bool, +) -> None: + """Metadata fallback never logs typed diagnostics, even with DEBUG enabled.""" + error: Exception = ( + ProviderRequestError(ProviderRequestFailure.TIMEOUT) + if transport + else SemanticBackendError() + ) + error.__cause__ = ValueError("private-provider-cause") + provider: MagicMock = MagicMock() + provider.configuration_class = PreviewConfiguration + provider.get_configuration_schema.side_effect = [error, {"type": "object"}] + mocker.patch.dict("superset.semantic_layers.api.registry", {"test": provider}) + with caplog.at_level(logging.DEBUG, logger="superset.semantic_layers"): + response: TestResponse = client.post( + "/api/v1/semantic_layer/schema/configuration", + json={"type": "test", "configuration": {"account": "fixture"}}, + ) + assert response.status_code == 200 + assert "private" not in response.get_data(as_text=True) + assert "private" not in caplog.text + assert provider.get_configuration_schema.call_count == 2 diff --git a/tests/unit_tests/semantic_layers/query_errors_test.py b/tests/unit_tests/semantic_layers/query_errors_test.py index 05e0df2cefa6..c12584f35fcd 100644 --- a/tests/unit_tests/semantic_layers/query_errors_test.py +++ b/tests/unit_tests/semantic_layers/query_errors_test.py @@ -311,7 +311,9 @@ def test_provider_execution_error_http( and record.levelno == logging.ERROR ] assert len(records) == 1 - assert records[0].getMessage() == "Semantic provider query execution failed" + assert records[0].getMessage() == ( + "Semantic provider query execution failed (builtins.ValueError)" + ) assert records[0].exc_info is None assert "private-provider" not in caplog.text date_view.get_table.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 09/17] 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 10/17] 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 11/17] 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 12/17] 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 13/17] 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 7ad72749f9571e776c0510471d0c6b4abedb89ba Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Mon, 26 Oct 2026 16:39:42 -0600 Subject: [PATCH 14/17] fix(semantic): normalize transport failures during query preparation --- .../extensions/semantic-provider-boundary.md | 13 ++- .../semantic_layer/tool/get_table.py | 9 +- superset/semantic_layers/exceptions.py | 16 +++- superset/semantic_layers/mapper.py | 6 +- superset/semantic_layers/models.py | 4 + .../semantic_layer/tool/test_get_table.py | 82 +++++++++++++++++ .../semantic_layers/provider_boundary_test.py | 14 ++- .../semantic_layers/query_errors_test.py | 90 ++++++++++++++++++- 8 files changed, 217 insertions(+), 17 deletions(-) diff --git a/docs/developer_docs/extensions/semantic-provider-boundary.md b/docs/developer_docs/extensions/semantic-provider-boundary.md index 6315f7d6876d..edfd32ee51a9 100644 --- a/docs/developer_docs/extensions/semantic-provider-boundary.md +++ b/docs/developer_docs/extensions/semantic-provider-boundary.md @@ -62,8 +62,11 @@ resolution rules, without adopting Superset's mask. This is a coordinated schema API upgrade. Update all overrides and direct callers with the host/core pin containing transport and readiness; there is no fallback -that retries without safety arguments. The factory's transport/context/credentials -contract is unchanged. No SQL parser limit or memo policy is added to the SDK. +that retries without safety arguments. `SemanticLayer.from_configuration` also +gains the required keyword-only `transport: ProviderTransport` parameter; update +provider factories and direct callers alongside the host/core pin. The existing +`cache_token`, `context`, and `credentials` parameters retain their contracts. +No SQL parser limit or memo policy is added to the SDK. ## Error signals @@ -80,7 +83,11 @@ If a retrieval budget prevents coverage proof, retain the provider's completenes classification; other transport errors propagate unchanged. A failed remote status is not Python cancellation. Preserve security, OAuth and cancellation control flow. Never retry without a filter or return an empty/partial successful result on failure. -MCP preserves its QueryError envelope with host-safe guidance. +Transport failures during query preparation (including provider construction and +dimension/metric discovery), dispatch, and filter-value preparation use the same +safe normalization. MCP `get_table` preserves its QueryError envelope with +host-safe guidance. Semantic-layer structure and schema endpoints retain their +own response contracts. Raise newly translated backend errors/refusals `from None`; never place backend responses, credentials or SQL in their public text. The host validates transport diff --git a/superset/mcp_service/semantic_layer/tool/get_table.py b/superset/mcp_service/semantic_layer/tool/get_table.py index 9a3518b31d6a..86851d3db7ec 100644 --- a/superset/mcp_service/semantic_layer/tool/get_table.py +++ b/superset/mcp_service/semantic_layer/tool/get_table.py @@ -64,6 +64,7 @@ is_semantic_layers_enabled, SemanticLayersDisabledError, ) +from superset.semantic_layers.exceptions import normalize_provider_request_errors from superset.utils import json if TYPE_CHECKING: @@ -172,6 +173,7 @@ def _grains_by_column(dimensions: Iterable[Dimension]) -> dict[str, dict[str, st return grains +@normalize_provider_request_errors() def _resolve_external_view( request: GetTableRequest, ) -> _ResolvedDatasource | SemanticLayerError: @@ -489,9 +491,10 @@ async def _run_get_table_query( and request.metrics and required_dimensions ): - compatible: set[str] = set( - resolved.view.get_compatible_dimensions(request.metrics, []) - ) + with normalize_provider_request_errors(): + compatible: set[str] = set( + resolved.view.get_compatible_dimensions(request.metrics, []) + ) incompatible: list[str] = sorted(required_dimensions - compatible) if incompatible: return SemanticLayerError.create( diff --git a/superset/semantic_layers/exceptions.py b/superset/semantic_layers/exceptions.py index ae637b1439d2..b2831218c227 100644 --- a/superset/semantic_layers/exceptions.py +++ b/superset/semantic_layers/exceptions.py @@ -18,7 +18,8 @@ """Host-owned presentation and classification for semantic provider execution.""" import logging -from collections.abc import Callable +from collections.abc import Callable, Iterator +from contextlib import contextmanager from typing import Any, ParamSpec from billiard.exceptions import SoftTimeLimitExceeded @@ -141,6 +142,15 @@ def log_provider_failure(message: str, error: Exception) -> None: logger.debug(message, exc_info=(type(error), error, error.__traceback__)) +@contextmanager +def normalize_provider_request_errors() -> Iterator[None]: + """Give query preparation and dispatch the same safe transport failure.""" + try: + yield + except ProviderRequestError as ex: + raise QueryObjectValidationError(provider_request_message(ex)) from None + + def execute_semantic_query( dispatcher: Callable[ProviderArguments, SemanticResult], *args: ProviderArguments.args, @@ -148,12 +158,10 @@ def execute_semantic_query( ) -> SemanticResult: """Translate provider signals without changing host validation or retries.""" try: - with provider_completeness(): + with provider_completeness(), normalize_provider_request_errors(): return dispatcher(*args, **kwargs) except SemanticQueryRejectedError as ex: raise SemanticLayerQueryRejectedError(ex.code) from None - except ProviderRequestError as ex: - raise QueryObjectValidationError(provider_request_message(ex)) from None except SemanticBackendError: logger.error("Semantic provider reported an execution failure") raise SemanticLayerExecutionError() from None diff --git a/superset/semantic_layers/mapper.py b/superset/semantic_layers/mapper.py index 405b7d7e5f06..70e9753c42e8 100644 --- a/superset/semantic_layers/mapper.py +++ b/superset/semantic_layers/mapper.py @@ -82,7 +82,10 @@ DimensionUsage, resolve_dimensions, ) -from superset.semantic_layers.exceptions import execute_semantic_query +from superset.semantic_layers.exceptions import ( + execute_semantic_query, + normalize_provider_request_errors, +) from superset.semantic_layers.members import members_by_key from superset.semantic_layers.ordering import with_default_order from superset.superset_typing import ( @@ -216,6 +219,7 @@ def from_view(cls, view: SemanticView) -> "_ViewMetadata": return cls(tuple(view.get_metrics()), tuple(view.get_dimensions())) +@normalize_provider_request_errors() def get_results(query_object: QueryObject) -> QueryResult: """ Run 1+ queries based on `QueryObject` and return the results. diff --git a/superset/semantic_layers/models.py b/superset/semantic_layers/models.py index 63790838288a..cb580e7d66d2 100644 --- a/superset/semantic_layers/models.py +++ b/superset/semantic_layers/models.py @@ -69,6 +69,7 @@ ) from superset.semantic_layers.exceptions import ( execute_semantic_query, + normalize_provider_request_errors, SemanticLayerQueryRejectedError, ) from superset.semantic_layers.execution import ( @@ -480,6 +481,7 @@ def normalize_columns(self) -> bool: """ return False + @normalize_provider_request_errors() def values_for_column( self, column_name: str, @@ -611,6 +613,7 @@ def type(self) -> str: return "semantic_view" @property + @normalize_provider_request_errors() def metrics(self) -> list[MetricMetadata]: return [ MetricMetadata( @@ -624,6 +627,7 @@ def metrics(self) -> list[MetricMetadata]: ] @property + @normalize_provider_request_errors() def _unique_dimensions(self) -> list[Dimension]: # A semantic view may expose multiple ``Dimension`` objects sharing the # same ``name`` but different grains (one variant per supported time diff --git a/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py b/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py index 983fd5979260..58b329ec8885 100644 --- a/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py +++ b/tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py @@ -2648,3 +2648,85 @@ async def fail_query(*args: Any, **kwargs: Any) -> None: if transport: assert str(error) in returned.message dispatcher.assert_called_once() + + +@pytest.mark.usefixtures("semantic_layers_enabled") +@pytest.mark.asyncio +@pytest.mark.parametrize("stage", ["factory", "dimensions", "metrics"]) +async def test_transport_failure_during_mcp_preparation_is_query_error( + mcp_server: FastMCP, + caplog: pytest.LogCaptureFixture, + stage: str, +) -> None: + """Transport failures before query dispatch use the safe QueryError envelope.""" + caplog.set_level("DEBUG") + error: ProviderRequestError = ProviderRequestError( + ProviderRequestFailure.HTTP_STATUS, status=403, provider="MetricFlow" + ) + error.__cause__ = ValueError("private-preparation-cause") + provider: MagicMock = MagicMock() + provider.get_dimensions.return_value = [] + provider.get_metrics.return_value = [] + factory: MagicMock = MagicMock(return_value=provider) + if stage == "factory": + factory.side_effect = error + else: + getattr(provider, f"get_{stage}").side_effect = error + view: SemanticView = SemanticView(id=7, name="Orders", configuration="{}") + execute: MagicMock = MagicMock() + with ( + patch( + "superset.daos.semantic_layer.SemanticViewDAO.find_by_id", return_value=view + ), + patch.object(SemanticView, "raise_for_access"), + patch.object(SemanticView, "implementation", property(lambda self: factory())), + patch.object(get_table_module, "execute_tabular_query", execute), + ): + client: Client + async with Client(mcp_server) as client: + result: CallToolResult = await client.call_tool( + "get_table", {"request": {"view_id": 7, "metrics": ["revenue"]}} + ) + returned: SemanticLayerError = SemanticLayerError.model_validate( + json.loads(result.content[0].text) + ) + assert returned.error_type == "QueryError" + assert str(error) in returned.message + assert "private-preparation" not in returned.message + assert "private-preparation" not in caplog.text + execute.assert_not_called() + provider.get_table.assert_not_called() + + +@pytest.mark.usefixtures("semantic_layers_enabled") +@pytest.mark.asyncio +async def test_transport_failure_during_mcp_compatibility_is_query_error( + mcp_server: FastMCP, + temporal_view: MagicMock, + caplog: pytest.LogCaptureFixture, +) -> None: + """A later discovery request gets the same envelope after resolution succeeds.""" + caplog.set_level("DEBUG") + error: ProviderRequestError = ProviderRequestError(ProviderRequestFailure.TIMEOUT) + error.__cause__ = ValueError("private-compatibility-cause") + temporal_view.get_compatible_dimensions.side_effect = error + client: Client + async with Client(mcp_server) as client: + result: CallToolResult = await client.call_tool( + "get_table", + { + "request": { + "view_id": 5, + "metrics": ["bookings"], + "dimensions": ["country_name"], + } + }, + ) + returned: SemanticLayerError = SemanticLayerError.model_validate( + json.loads(result.content[0].text) + ) + assert returned.error_type == "QueryError" + assert str(error) in returned.message + assert "private-compatibility" not in returned.message + assert "private-compatibility" not in caplog.text + temporal_view.get_compatible_dimensions.assert_called_once() diff --git a/tests/unit_tests/semantic_layers/provider_boundary_test.py b/tests/unit_tests/semantic_layers/provider_boundary_test.py index f9c334044bb5..15252dd7e1da 100644 --- a/tests/unit_tests/semantic_layers/provider_boundary_test.py +++ b/tests/unit_tests/semantic_layers/provider_boundary_test.py @@ -18,6 +18,8 @@ """Trusted readiness and safe failure handling at real host entry points.""" import logging +from collections.abc import Callable +from functools import partial from typing import cast from unittest.mock import MagicMock @@ -40,6 +42,7 @@ from superset.exceptions import QueryObjectValidationError from superset.semantic_layers.exceptions import ( execute_semantic_query, + normalize_provider_request_errors, SemanticLayerExecutionError, SemanticLayerQueryRejectedError, ) @@ -283,7 +286,7 @@ def _untrusted_transport_error(case: str) -> ProviderRequestError: @pytest.mark.parametrize( "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True ) -@pytest.mark.parametrize("endpoint", ["query", "structure", "runtime"]) +@pytest.mark.parametrize("endpoint", ["query", "preparation", "structure", "runtime"]) @pytest.mark.parametrize( "case", [ @@ -311,14 +314,19 @@ def test_transport_errors_rebuilt_at_every_public_boundary( valid: bool = case in {"provider", "subclass"} message: str with caplog.at_level(logging.DEBUG, logger="superset.semantic_layers"): - if endpoint == "query": + if endpoint in {"query", "preparation"}: captured: pytest.ExceptionInfo[ QueryObjectValidationError | SemanticLayerExecutionError ] + invoke: Callable[[], object] = ( + normalize_provider_request_errors()(MagicMock(side_effect=error)) + if endpoint == "preparation" + else partial(execute_semantic_query, MagicMock(side_effect=error)) + ) with pytest.raises( (QueryObjectValidationError, SemanticLayerExecutionError) ) as captured: - execute_semantic_query(MagicMock(side_effect=error)) + invoke() assert isinstance(captured.value, QueryObjectValidationError) == valid assert captured.value.__cause__ is None assert captured.value.__suppress_context__ diff --git a/tests/unit_tests/semantic_layers/query_errors_test.py b/tests/unit_tests/semantic_layers/query_errors_test.py index a37147304974..98d33faa9e9c 100644 --- a/tests/unit_tests/semantic_layers/query_errors_test.py +++ b/tests/unit_tests/semantic_layers/query_errors_test.py @@ -16,7 +16,9 @@ # under the License. import logging +from collections.abc import Callable from datetime import date +from functools import partial from typing import Any from unittest.mock import MagicMock, PropertyMock @@ -417,7 +419,10 @@ def test_required_secondary_query_rejection( @pytest.mark.parametrize("kind", ["oauth", "cancel", "worker_timeout", "security"]) -def test_provider_control_signal_is_preserved(app_context: None, kind: str) -> None: +@pytest.mark.parametrize("preparation", [False, True]) +def test_provider_control_signal_is_preserved( + app_context: None, kind: str, preparation: bool +) -> None: """Do not turn authentication, cancellation or worker control into faults.""" from billiard.exceptions import SoftTimeLimitExceeded from superset_core.semantic_layers.types import SemanticQuery @@ -428,7 +433,10 @@ def test_provider_control_signal_is_preserved(app_context: None, kind: str) -> N SupersetCancelQueryException, SupersetSecurityException, ) - from superset.semantic_layers.exceptions import execute_semantic_query + from superset.semantic_layers.exceptions import ( + execute_semantic_query, + normalize_provider_request_errors, + ) control: Exception = { "security": SupersetSecurityException( @@ -443,9 +451,16 @@ def test_provider_control_signal_is_preserved(app_context: None, kind: str) -> N "worker_timeout": SoftTimeLimitExceeded(), }[kind] dispatcher: MagicMock = MagicMock(side_effect=control) + operation: Callable[[], object] = ( + normalize_provider_request_errors()(dispatcher) + if preparation + else partial( + execute_semantic_query, dispatcher, SemanticQuery(dimensions=[], metrics=[]) + ) + ) captured: pytest.ExceptionInfo[Exception] with pytest.raises(type(control)) as captured: - execute_semantic_query(dispatcher, SemanticQuery(dimensions=[], metrics=[])) + operation() assert captured.value is control dispatcher.assert_called_once() @@ -932,3 +947,72 @@ def test_sdk_failure_values_lookup_keeps_category_and_never_retries( if transport: assert str(caught.value) == str(error) date_view.get_values.assert_called_once() + + +@pytest.mark.parametrize("operation", ["query", "values"]) +@pytest.mark.parametrize("stage", ["factory", "dimensions"]) +def test_transport_failure_during_query_preparation_is_normalized( + date_view: MagicMock, + mocker: MockerFixture, + operation: str, + stage: str, +) -> None: + """Preparation gets the dispatch classification without executing or retrying.""" + error: ProviderRequestError = ProviderRequestError(ProviderRequestFailure.TIMEOUT) + error.__cause__ = ValueError("private-preparation-cause") + if stage == "factory": + mocker.patch.object( + SemanticView, "implementation", new_callable=PropertyMock, side_effect=error + ) + else: + date_view.get_dimensions.side_effect = error + view: SemanticView = SemanticView(id=7, name="Orders", configuration="{}") + prepare: Callable[[], object] + if operation == "query": + prepare = partial( + get_results, + QueryObject(datasource=view, columns=["ORDER_DATE"], metrics=[]), + ) + else: + prepare = partial(view.values_for_column, "ORDER_DATE") + caught: pytest.ExceptionInfo[QueryObjectValidationError] + with pytest.raises(QueryObjectValidationError) as caught: + prepare() + assert str(caught.value) == str(error) + assert caught.value.__cause__ is None + assert caught.value.__suppress_context__ + date_view.get_table.assert_not_called() + date_view.get_values.assert_not_called() + + +@pytest.mark.parametrize("stage", ["factory", "dimensions"]) +def test_transport_failure_during_chart_preparation_returns_400( + client: FlaskClient, + full_api_access: None, + date_view: MagicMock, + mocker: MockerFixture, + stage: str, +) -> None: + """Chart preparation exposes only the same safe transport guidance as dispatch.""" + error: ProviderRequestError = ProviderRequestError(ProviderRequestFailure.TIMEOUT) + error.__cause__ = ValueError("private-preparation-cause") + if stage == "factory": + mocker.patch.object( + SemanticView, "implementation", new_callable=PropertyMock, side_effect=error + ) + else: + date_view.get_dimensions.side_effect = error + response: TestResponse = client.post( + "/api/v1/chart/data", + json={ + "datasource": {"id": 7, "type": "semantic_view"}, + "queries": [{"columns": ["ORDER_DATE"], "metrics": ["ORDER_COUNT"]}], + "result_format": "json", + "result_type": "full", + "force": True, + }, + ) + assert response.status_code == 400 + assert str(error) in response.get_data(as_text=True) + assert "private-preparation" not in response.get_data(as_text=True) + date_view.get_table.assert_not_called() From e791fe98e818bc024e7afbef128a25f3cc87b6fc Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Mon, 26 Oct 2026 19:58:22 -0600 Subject: [PATCH 15/17] 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 b05772649bd9c699086d2159c7a07647c02541f8 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Wed, 28 Oct 2026 09:47:30 -0600 Subject: [PATCH 16/17] fix(semantic-layer): enrich empty preview configurations Treat an explicit empty configuration object like equivalent default-only settings while retaining dictionary validation and write permission checks. Cover writer/read-only, empty/nonempty objects and non-object payloads through the preview route, without saved-secret lookup. Addresses review thread r4397335672. --- superset/semantic_layers/api.py | 8 +-- .../semantic_layers/provider_boundary_test.py | 55 +++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/superset/semantic_layers/api.py b/superset/semantic_layers/api.py index f89fb00a488d..5577e495c3cd 100644 --- a/superset/semantic_layers/api.py +++ b/superset/semantic_layers/api.py @@ -843,11 +843,9 @@ def configuration_schema(self) -> FlaskResponse: parsed_config = None readiness: SemanticCredentialReadiness = SemanticCredentialReadiness.UNRESOLVED config: Any - if ( - isinstance(config := body.get("configuration"), dict) - and config - and security_manager.can_access("can_write", "SemanticLayer") - ): + if isinstance( + config := body.get("configuration"), dict + ) and security_manager.can_access("can_write", "SemanticLayer"): readiness = credential_readiness(config) parsed_config = _parse_partial_config(cls, config) if parsed_config is None: diff --git a/tests/unit_tests/semantic_layers/provider_boundary_test.py b/tests/unit_tests/semantic_layers/provider_boundary_test.py index 282eed56b052..2e401728a37f 100644 --- a/tests/unit_tests/semantic_layers/provider_boundary_test.py +++ b/tests/unit_tests/semantic_layers/provider_boundary_test.py @@ -107,6 +107,61 @@ def test_preview_readiness_uses_raw_input_not_client_assertion( assert provider.get_configuration_schema.call_args.args == (None,) +@pytest.mark.parametrize( + "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True +) +@pytest.mark.parametrize("writer", [False, True]) +@pytest.mark.parametrize("configuration", [{}, {"ignored": "fixture"}, None, [], False]) +def test_preview_defaults_preserve_readiness_and_write_permission( + client: FlaskClient, + full_api_access: None, + mocker: MockerFixture, + writer: bool, + configuration: object, +) -> None: + """Only writers with an explicit object may discover default-only choices.""" + provider: MagicMock = MagicMock() + provider.configuration_class = PreviewConfiguration + + def configuration_schema( + config: PreviewConfiguration | None, + *, + transport: ProviderTransport, + readiness: SemanticCredentialReadiness, + ) -> dict[str, Any]: + """Expose choices only when the host supplies resolved parsed settings.""" + if readiness is SemanticCredentialReadiness.RESOLVED: + assert config == PreviewConfiguration() + return {"enum": ["orders"]} + assert config is None + return {"enum": []} + + provider.get_configuration_schema.side_effect = configuration_schema + 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, + ) + lookup: MagicMock = mocker.patch( + "superset.semantic_layers.api.SemanticLayerDAO.find_by_uuid" + ) + response: TestResponse = client.post( + "/api/v1/semantic_layer/schema/configuration", + json={"type": "test", "configuration": configuration}, + ) + assert response.status_code == 200 + assert response.get_json()["result"]["enum"] == ( + ["orders"] if writer and isinstance(configuration, dict) else [] + ) + provider.get_configuration_schema.assert_called_once() + assert provider.get_configuration_schema.call_args.kwargs["readiness"] is ( + SemanticCredentialReadiness.RESOLVED + if writer and isinstance(configuration, dict) + else SemanticCredentialReadiness.UNRESOLVED + ) + lookup.assert_not_called() + + @pytest.mark.parametrize( "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True ) From 4cd9e5c053453b5369fa79aa1be3f96e44763e2c Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Wed, 28 Oct 2026 17:29:56 -0600 Subject: [PATCH 17/17] fix(semantic-layer): identify targets in provider failure logs Address review thread r4405395077: retain the view id and layer UUID in structure/runtime failure events so operators can locate the failing object without logging configuration. Preview has no persisted target and records absent identifiers explicitly. Preserve typed-failure secrecy and unknown-fault DEBUG diagnostics, with route-level regression coverage. --- superset/semantic_layers/api.py | 19 +++- superset/semantic_layers/exceptions.py | 9 +- .../semantic_layers/provider_boundary_test.py | 98 +++++++++++++++++++ .../semantic_layers/query_errors_test.py | 3 +- 4 files changed, 124 insertions(+), 5 deletions(-) diff --git a/superset/semantic_layers/api.py b/superset/semantic_layers/api.py index 5577e495c3cd..57becc43f2fa 100644 --- a/superset/semantic_layers/api.py +++ b/superset/semantic_layers/api.py @@ -346,7 +346,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 - log_provider_failure("Error fetching semantic view structure", ex) + log_provider_failure( + "Error fetching semantic view structure", + ex, + view_id=pk, + layer_uuid=view.semantic_layer_uuid, + ) return self.response_422( message=provider_request_message(ex) if isinstance(ex, ProviderRequestError) @@ -865,7 +870,11 @@ def configuration_schema(self) -> FlaskResponse: ) ) log_provider_failure( - "Error enriching semantic layer configuration schema", ex + "Error enriching semantic layer configuration schema", + ex, + # Preview has no persisted view or layer; never use submitted IDs. + view_id=None, + layer_uuid=None, ) # Connection or query failures during schema enrichment should not # prevent the form from rendering — return the base schema instead. @@ -943,7 +952,11 @@ 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 - log_provider_failure("Error fetching semantic layer runtime schema", ex) + log_provider_failure( + "Error fetching semantic layer runtime schema", + ex, + layer_uuid=layer.uuid, + ) return self.response_400( message=provider_request_message(ex) if isinstance(ex, ProviderRequestError) diff --git a/superset/semantic_layers/exceptions.py b/superset/semantic_layers/exceptions.py index aa7f5166e291..602053fe0854 100644 --- a/superset/semantic_layers/exceptions.py +++ b/superset/semantic_layers/exceptions.py @@ -133,8 +133,15 @@ def provider_request_message(error: ProviderRequestError) -> str: ) -def log_provider_failure(message: str, error: Exception) -> None: +def log_provider_failure( + message: str, + error: Exception, + *, + view_id: int | None = None, + layer_uuid: UUID | None = None, +) -> None: """Keep typed failures quiet; retain unknown diagnostics in server DEBUG logs.""" + message = f"{message} view_id={view_id} layer_uuid={layer_uuid}" if isinstance( error, (ProviderRequestError, SemanticBackendError, SemanticQueryRejectedError) ): diff --git a/tests/unit_tests/semantic_layers/provider_boundary_test.py b/tests/unit_tests/semantic_layers/provider_boundary_test.py index 2e401728a37f..a43d52510aff 100644 --- a/tests/unit_tests/semantic_layers/provider_boundary_test.py +++ b/tests/unit_tests/semantic_layers/provider_boundary_test.py @@ -22,6 +22,7 @@ from functools import partial from typing import Any, cast from unittest.mock import MagicMock +from uuid import UUID import pytest from flask.testing import FlaskClient @@ -58,6 +59,103 @@ class PreviewConfiguration(BaseModel): password: str | None = None +@pytest.mark.parametrize( + "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True +) +@pytest.mark.parametrize("endpoint", ["structure", "preview", "runtime"]) +@pytest.mark.parametrize( + "failure", ["transport", "backend", "unknown", "unknown_debug"] +) +def test_provider_failure_logs_identify_target_without_credentials( + client: FlaskClient, + full_api_access: None, + mocker: MockerFixture, + caplog: pytest.LogCaptureFixture, + endpoint: str, + failure: str, +) -> None: + """Use persisted identifiers, never submitted configuration or provider text.""" + layer_uuid: UUID = UUID("ed218dc4-8522-4e1c-a72e-d5b3693e9425") + sentinel: str = "password=private-credential-sentinel" + error: Exception = ( + ProviderRequestError(ProviderRequestFailure.TIMEOUT) + if failure == "transport" + else SemanticBackendError() + if failure == "backend" + else ConnectionError(sentinel) + ) + error.__cause__ = ValueError( + "https://private-provider.example/?token=private-token" + ) + provider: MagicMock = MagicMock() + provider.configuration_class = PreviewConfiguration + layer: MagicMock = MagicMock( + uuid=layer_uuid, type="test", configuration=json.dumps({"password": sentinel}) + ) + view: MagicMock = MagicMock(id=17, semantic_layer_uuid=layer_uuid) + view.implementation.get_dimensions.side_effect = error + provider.get_runtime_schema.side_effect = error + provider.get_configuration_schema.side_effect = [error, {"type": "object"}] + mocker.patch.dict("superset.semantic_layers.api.registry", {"test": provider}) + mocker.patch( + "superset.semantic_layers.api.SemanticLayerDAO.find_by_uuid", return_value=layer + ) + session: MagicMock = mocker.patch("superset.semantic_layers.api.db.session") + session.query.return_value.filter_by.return_value.first.return_value = view + response: TestResponse + # Unknown exceptions deliberately retain diagnostics at DEBUG, not ERROR. + with caplog.at_level( + logging.ERROR if failure == "unknown" else logging.DEBUG, + logger="superset.semantic_layers.exceptions", + ): + if endpoint == "structure": + response = client.get("/api/v1/semantic_view/17/structure") + elif endpoint == "runtime": + response = client.post( + f"/api/v1/semantic_layer/{layer_uuid}/schema/runtime" + ) + else: + response = client.post( + "/api/v1/semantic_layer/schema/configuration", + json={ + "type": "test", + "configuration": {"password": sentinel}, + "view_id": sentinel, + "layer_uuid": sentinel, + }, + ) + assert ( + response.status_code + == {"structure": 422, "runtime": 400, "preview": 200}[endpoint] + ) + records: list[logging.LogRecord] = [ + record + for record in caplog.records + if record.name == "superset.semantic_layers.exceptions" + ] + assert len(records) == (2 if failure == "unknown_debug" else 1) + record: logging.LogRecord + for record in records: + assert f"view_id={17 if endpoint == 'structure' else None}" in record.message + assert ( + f"layer_uuid={layer_uuid if endpoint != 'preview' else None}" + in record.message + ) + assert sentinel not in record.message + assert "private-provider" not in record.message + assert "private-token" not in record.message + assert records[0].exc_info is None + if failure == "unknown_debug": + assert records[1].levelno == logging.DEBUG + assert records[1].exc_info is not None + assert records[1].exc_info[1] is error + else: + assert sentinel not in caplog.text + assert "private-provider" not in caplog.text + assert "private-token" not in caplog.text + assert sentinel not in response.get_data(as_text=True) + + @pytest.mark.parametrize( "app", [{"FEATURE_FLAGS": {"SEMANTIC_LAYERS": True}}], indirect=True ) diff --git a/tests/unit_tests/semantic_layers/query_errors_test.py b/tests/unit_tests/semantic_layers/query_errors_test.py index f41fcabfc64b..c3926b145e62 100644 --- a/tests/unit_tests/semantic_layers/query_errors_test.py +++ b/tests/unit_tests/semantic_layers/query_errors_test.py @@ -778,7 +778,8 @@ def test_provider_execution_error_http( ] assert len(records) == 1 assert records[0].getMessage() == ( - "Semantic provider query execution failed (builtins.ValueError)" + "Semantic provider query execution failed " + "view_id=None layer_uuid=None (builtins.ValueError)" ) assert records[0].exc_info is None assert "private-provider" not in caplog.text