codeant-ai-for-open-source[bot] commented on code in PR #43395:
URL: https://github.com/apache/superset/pull/43395#discussion_r3832332420
##########
superset/reports/notifications/webhook.py:
##########
@@ -32,10 +36,104 @@
)
from superset.utils import json
from superset.utils.decorators import statsd_gauge
-from superset.utils.network import is_safe_host
+from superset.utils.network import is_safe_host, is_safe_ip
logger = logging.getLogger(__name__)
+# Number of characters of a failing response body kept in the server-side log
+# line. Response bodies are never folded into the exception message raised
+# back to the caller -- that message is persisted verbatim as
+# ``ReportExecutionLog.error_message`` and readable via the execution log
+# API, which would otherwise turn the webhook target into a readback oracle
+# for whatever it chooses to return (including an internal host reached via
+# DNS rebinding).
+_LOGGED_RESPONSE_BODY_LIMIT = 500
+
+
+def _raise_for_unsafe_peer(conn: HTTPConnection) -> None:
+ """
+ Validate that a connection's actual peer is publicly routable.
+
+ ``_validate_webhook_url`` resolves and checks the hostname once, ahead of
+ time; the connection opened here is resolved independently and may reach
+ a different address (DNS rebinding via a low-TTL record), so the check
+ has to be repeated against the address actually connected to.
+ """
+ sock = conn.sock
+ if sock is None:
+ return
+ peer = sock.getpeername()[0]
+ if not is_safe_ip(ipaddress.ip_address(peer)):
+ raise NotificationParamException("Webhook URL target host is not
allowed.")
+
+
+class _PeerValidatingHTTPConnection(HTTPConnection):
+ """HTTP connection that validates the peer address on connect."""
+
+ def connect(self) -> None:
+ super().connect()
+ _raise_for_unsafe_peer(self)
+
+
+class _PeerValidatingHTTPSConnection(HTTPSConnection):
+ """HTTPS connection that validates the peer address after the handshake."""
+
+ def connect(self) -> None:
+ super().connect()
+ _raise_for_unsafe_peer(self)
+
+
+class _PeerValidatingHTTPConnectionPool(HTTPConnectionPool):
+ ConnectionCls = _PeerValidatingHTTPConnection
+
+
+class _PeerValidatingHTTPSConnectionPool(HTTPSConnectionPool):
+ ConnectionCls = _PeerValidatingHTTPSConnection
+
+
+class _PeerValidatingHTTPAdapter(HTTPAdapter):
+ """
+ Transport adapter that routes requests through connection classes which
+ validate the connected peer address, closing the TOCTOU window between
+ the hostname check in ``_validate_webhook_url`` and the connection that
+ ``send()`` actually opens.
+
+ Mirrors the peer-validation approach used for dataset-import data URIs
+ (``superset.commands.dataset.importers.v1.utils``), adapted to
+ ``requests``/``urllib3`` connection pooling instead of ``urllib``.
+ """
+
+ def init_poolmanager(self, *args: Any, **kwargs: Any) -> None:
+ super().init_poolmanager(*args, **kwargs)
+ # Assign a new dict rather than mutating the manager's dict in
+ # place -- the attribute otherwise aliases urllib3's module-global
+ # default scheme-to-pool-class mapping.
+ self.poolmanager.pool_classes_by_scheme = {
+ "http": _PeerValidatingHTTPConnectionPool,
+ "https": _PeerValidatingHTTPSConnectionPool,
+ }
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Do not flag the absence of destination peer validation for proxied webhook
requests; proxy connections intentionally validate only the operator-configured
proxy, while destination DNS resolution and connection are delegated to that
proxy.
**Applied to:**
- `superset/reports/notifications/webhook.py`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
