diff --git a/packages/traceloop-sdk/tests/test_fortifyroot_auth_warnings.py b/packages/traceloop-sdk/tests/test_fortifyroot_auth_warnings.py index 10aee47554..0ffd33613f 100644 --- a/packages/traceloop-sdk/tests/test_fortifyroot_auth_warnings.py +++ b/packages/traceloop-sdk/tests/test_fortifyroot_auth_warnings.py @@ -12,6 +12,7 @@ from traceloop.sdk.exporters.auth_warnings import ( _AuthWarningClientProxy, + _endpoint_label, reset_auth_warning_state_for_tests, ) @@ -133,8 +134,8 @@ def test_http_exporters_warn_on_auth_failure_once(factory, signal, status_code, def test_http_exporter_dedupes_by_endpoint(caplog): reset_auth_warning_state_for_tests() - exporter_one = _make_traces_http_exporter("http://collector-one:4318") - exporter_two = _make_traces_http_exporter("http://collector-two:4318") + exporter_one = _make_traces_http_exporter("http://localhost:4318") + exporter_two = _make_traces_http_exporter("http://localhost:4319") assert hasattr(exporter_one, "_session") assert hasattr(exporter_two, "_session") exporter_one._session.post = _rejecting_post(401) @@ -147,8 +148,23 @@ def test_http_exporter_dedupes_by_endpoint(caplog): auth_warnings = _auth_warnings(caplog) assert len(auth_warnings) == 2 messages = [record.getMessage() for record in auth_warnings] - assert any("collector-one:4318" in message for message in messages) - assert any("collector-two:4318" in message for message in messages) + assert any("localhost:4318" in message for message in messages) + assert any("localhost:4319" in message for message in messages) + + +def test_endpoint_label_strips_userinfo(): + assert _endpoint_label("https://user:secret@collector.example.com:4318/v1/traces") == ( + "collector.example.com:4318" + ) + + +def test_endpoint_label_formats_ipv6_host(): + assert _endpoint_label("grpcs://[::1]:4317") == "[::1]:4317" + + +def test_endpoint_label_handles_invalid_port(): + endpoint = "https://collector.example.com:99999/v1/traces" + assert _endpoint_label(endpoint) == endpoint @pytest.mark.parametrize( diff --git a/packages/traceloop-sdk/tests/test_sdk_initialization.py b/packages/traceloop-sdk/tests/test_sdk_initialization.py index 8f31be1c3e..124e8d25e2 100644 --- a/packages/traceloop-sdk/tests/test_sdk_initialization.py +++ b/packages/traceloop-sdk/tests/test_sdk_initialization.py @@ -58,13 +58,25 @@ def test_http_endpoint_construction(self, endpoint, expected_endpoint): init_spans_exporter(endpoint, {}) mock.assert_called_once_with(endpoint=expected_endpoint, headers={}) + @pytest.mark.parametrize("endpoint,match", [ + ("http://collector.example.com:4318", "http:// OTLP export is insecure"), + ("http://host.docker.internal:4318", "http:// OTLP export is insecure"), + ("https://user:secret@collector.example.com:4318", "must not include username or password"), + ]) + def test_http_endpoint_security_rejections(self, endpoint, match): + from traceloop.sdk.tracing.tracing import init_spans_exporter + + with pytest.raises(ValueError, match=match): + init_spans_exporter(endpoint, {}) + @pytest.mark.parametrize("endpoint,expected_endpoint,insecure", [ ("grpc://localhost:4317", "localhost:4317", True), - ("GRPC://host:4317", "host:4317", True), + ("GRPC://127.0.0.1:4317", "127.0.0.1:4317", True), + ("grpc://[::1]:4317", "[::1]:4317", True), ("grpcs://localhost:4317", "localhost:4317", False), ("GRPCS://host:4317", "host:4317", False), (" grpc://localhost:4317 ", "localhost:4317", True), # whitespace stripped - ("localhost:4317", "localhost:4317", True), # no scheme = insecure gRPC + ("localhost:4317", "localhost:4317", False), # no scheme = secure gRPC ]) def test_grpc_schemes(self, endpoint, expected_endpoint, insecure): from traceloop.sdk.tracing.tracing import init_spans_exporter @@ -74,6 +86,61 @@ def test_grpc_schemes(self, endpoint, expected_endpoint, insecure): init_spans_exporter(endpoint, {}) mock.assert_called_once_with(endpoint=expected_endpoint, headers={}, insecure=insecure) + @pytest.mark.parametrize("endpoint,match", [ + ("grpc://collector.example.com:4317", "grpc:// OTLP export is insecure"), + ("grpc://host.docker.internal:4317", "grpc:// OTLP export is insecure"), + ("otlp://collector.example.com:4317", "Unsupported OTLP exporter endpoint scheme"), + ]) + def test_insecure_remote_or_unknown_grpc_schemes_are_rejected(self, endpoint, match): + from traceloop.sdk.tracing.tracing import init_spans_exporter + + with pytest.raises(ValueError, match=match): + init_spans_exporter(endpoint, {}) + + +class TestInitMetricsAndLoggingExporters: + """Security-sensitive exporter endpoint parsing shared by metrics and logs.""" + + def test_metrics_bare_grpc_endpoint_is_secure_by_default(self): + from traceloop.sdk.metrics.metrics import init_metrics_exporter + from opentelemetry.exporter.otlp.proto.grpc.metric_exporter import OTLPMetricExporter + + with patch.object(OTLPMetricExporter, "__init__", return_value=None) as mock: + init_metrics_exporter("collector.example.com:4317", {}) + + mock.assert_called_once_with( + endpoint="collector.example.com:4317", + headers={}, + insecure=False, + ) + + def test_logging_local_grpc_endpoint_can_be_insecure(self): + from traceloop.sdk.logging.logging import init_logging_exporter + from opentelemetry.exporter.otlp.proto.grpc._log_exporter import OTLPLogExporter + + with patch.object(OTLPLogExporter, "__init__", return_value=None) as mock: + init_logging_exporter("grpc://localhost:4317", {}) + + mock.assert_called_once_with( + endpoint="localhost:4317", + headers={}, + insecure=True, + ) + + @pytest.mark.parametrize("factory_path,endpoint", [ + ("traceloop.sdk.metrics.metrics.init_metrics_exporter", "grpc://collector.example.com:4317"), + ("traceloop.sdk.logging.logging.init_logging_exporter", "otlp://collector.example.com:4317"), + ("traceloop.sdk.metrics.metrics.init_metrics_exporter", "http://collector.example.com:4318"), + ("traceloop.sdk.logging.logging.init_logging_exporter", "https://user:secret@collector.example.com:4318"), + ]) + def test_metrics_and_logging_reject_unsafe_or_unknown_schemes(self, factory_path, endpoint): + module_path, _, function_name = factory_path.rpartition(".") + module = __import__(module_path, fromlist=[function_name]) + factory = getattr(module, function_name) + + with pytest.raises(ValueError): + factory(endpoint, {}) + @pytest.mark.vcr def test_resource_attributes(exporter, openai_client): diff --git a/packages/traceloop-sdk/traceloop/sdk/exporters/auth_warnings.py b/packages/traceloop-sdk/traceloop/sdk/exporters/auth_warnings.py index 2103cbec42..896de2ab32 100644 --- a/packages/traceloop-sdk/traceloop/sdk/exporters/auth_warnings.py +++ b/packages/traceloop-sdk/traceloop/sdk/exporters/auth_warnings.py @@ -51,8 +51,17 @@ def _endpoint_label(endpoint: Any) -> str: if not raw_endpoint: return "unknown endpoint" parsed = urlparse(raw_endpoint) - if parsed.netloc: - return parsed.netloc + if parsed.hostname: + host = parsed.hostname + if ":" in host and not host.startswith("["): + host = f"[{host}]" + try: + port = parsed.port + except ValueError: + return raw_endpoint + if port is not None: + return f"{host}:{port}" + return host return raw_endpoint diff --git a/packages/traceloop-sdk/traceloop/sdk/logging/logging.py b/packages/traceloop-sdk/traceloop/sdk/logging/logging.py index e43d6ea3ca..375f5f9dd6 100644 --- a/packages/traceloop-sdk/traceloop/sdk/logging/logging.py +++ b/packages/traceloop-sdk/traceloop/sdk/logging/logging.py @@ -2,6 +2,7 @@ # This file has been modified by FortifyRoot. # Original source: https://github.com/traceloop/openllmetry +import ipaddress import logging from typing import Dict, Optional, Any, cast from urllib.parse import urlparse @@ -17,6 +18,59 @@ FortifyRootHTTPLogExporter as HTTPExporter, ) +LOCAL_EXPORT_HOSTS = {"localhost"} + + +def _is_local_export_host(host: Optional[str]) -> bool: + if not host: + return False + normalized = host.strip().lower() + if normalized in LOCAL_EXPORT_HOSTS: + return True + try: + return ipaddress.ip_address(normalized).is_loopback + except ValueError: + return False + + +def _resolve_grpc_exporter_endpoint(endpoint: str) -> tuple[str, bool]: + trimmed_endpoint = endpoint.strip() + if "://" not in trimmed_endpoint: + return trimmed_endpoint, False + + parsed = urlparse(trimmed_endpoint) + if parsed.username or parsed.password: + raise ValueError("OTLP exporter endpoints must not include username or password") + scheme = parsed.scheme.lower() + if scheme == "grpcs": + return parsed.netloc, False + if scheme == "grpc": + if not _is_local_export_host(parsed.hostname): + raise ValueError( + "grpc:// OTLP export is insecure and is allowed only for local " + "development endpoints; use grpcs:// or https:// for remote collectors" + ) + return parsed.netloc, True + + raise ValueError( + f"Unsupported OTLP exporter endpoint scheme {parsed.scheme!r}; " + "use https://, http://, grpcs://, grpc://localhost, or a bare secure gRPC host:port" + ) + + +def _validate_http_exporter_endpoint(endpoint: str) -> None: + parsed = urlparse(endpoint.strip()) + if parsed.username or parsed.password: + raise ValueError("OTLP exporter endpoints must not include username or password") + if parsed.scheme.lower() != "http": + return + if _is_local_export_host(parsed.hostname): + return + raise ValueError( + "http:// OTLP export is insecure and is allowed only for local " + "development endpoints; use https:// for remote collectors" + ) + _FORTIFYROOT_LOGGING_HANDLER_MARKER = "_fortifyroot_logging_handler" _FORTIFYROOT_INTERNAL_EXPORTER_LOGGER_PREFIX = "fortifyroot.sdk.exporters." @@ -75,13 +129,18 @@ def get_logging_provider(cls) -> Optional[LoggerProvider]: def init_logging_exporter(endpoint: str, headers: Dict[str, str]) -> LogExporter: trimmed_endpoint = endpoint.strip() - if urlparse(trimmed_endpoint).scheme.lower() in {"http", "https"}: + scheme = urlparse(trimmed_endpoint).scheme.lower() + if scheme in {"http", "https"}: + _validate_http_exporter_endpoint(trimmed_endpoint) base_url = trimmed_endpoint.rstrip("/") if not base_url.endswith("/v1/logs"): base_url = f"{base_url}/v1/logs" return cast(LogExporter, HTTPExporter(endpoint=base_url, headers=headers)) - else: - return cast(LogExporter, GRPCExporter(endpoint=trimmed_endpoint, headers=headers)) + grpc_endpoint, insecure = _resolve_grpc_exporter_endpoint(trimmed_endpoint) + return cast( + LogExporter, + GRPCExporter(endpoint=grpc_endpoint, headers=headers, insecure=insecure), + ) def init_logging_provider( diff --git a/packages/traceloop-sdk/traceloop/sdk/metrics/metrics.py b/packages/traceloop-sdk/traceloop/sdk/metrics/metrics.py index 313a1657f6..b85d66f0d2 100644 --- a/packages/traceloop-sdk/traceloop/sdk/metrics/metrics.py +++ b/packages/traceloop-sdk/traceloop/sdk/metrics/metrics.py @@ -2,6 +2,7 @@ # This file has been modified by FortifyRoot. # Original source: https://github.com/traceloop/openllmetry +import ipaddress from collections.abc import Sequence from typing import Dict, Optional, Any from urllib.parse import urlparse @@ -21,6 +22,59 @@ FortifyRootHTTPMetricExporter as HTTPExporter, ) +LOCAL_EXPORT_HOSTS = {"localhost"} + + +def _is_local_export_host(host: Optional[str]) -> bool: + if not host: + return False + normalized = host.strip().lower() + if normalized in LOCAL_EXPORT_HOSTS: + return True + try: + return ipaddress.ip_address(normalized).is_loopback + except ValueError: + return False + + +def _resolve_grpc_exporter_endpoint(endpoint: str) -> tuple[str, bool]: + trimmed_endpoint = endpoint.strip() + if "://" not in trimmed_endpoint: + return trimmed_endpoint, False + + parsed = urlparse(trimmed_endpoint) + if parsed.username or parsed.password: + raise ValueError("OTLP exporter endpoints must not include username or password") + scheme = parsed.scheme.lower() + if scheme == "grpcs": + return parsed.netloc, False + if scheme == "grpc": + if not _is_local_export_host(parsed.hostname): + raise ValueError( + "grpc:// OTLP export is insecure and is allowed only for local " + "development endpoints; use grpcs:// or https:// for remote collectors" + ) + return parsed.netloc, True + + raise ValueError( + f"Unsupported OTLP exporter endpoint scheme {parsed.scheme!r}; " + "use https://, http://, grpcs://, grpc://localhost, or a bare secure gRPC host:port" + ) + + +def _validate_http_exporter_endpoint(endpoint: str) -> None: + parsed = urlparse(endpoint.strip()) + if parsed.username or parsed.password: + raise ValueError("OTLP exporter endpoints must not include username or password") + if parsed.scheme.lower() != "http": + return + if _is_local_export_host(parsed.hostname): + return + raise ValueError( + "http:// OTLP export is insecure and is allowed only for local " + "development endpoints; use https:// for remote collectors" + ) + class MetricsWrapper(object): resource_attributes: Dict[Any, Any] = {} @@ -63,13 +117,15 @@ def set_static_params( def init_metrics_exporter(endpoint: str, headers: Dict[str, str]) -> MetricExporter: trimmed_endpoint = endpoint.strip() - if urlparse(trimmed_endpoint).scheme.lower() in {"http", "https"}: + scheme = urlparse(trimmed_endpoint).scheme.lower() + if scheme in {"http", "https"}: + _validate_http_exporter_endpoint(trimmed_endpoint) base_url = trimmed_endpoint.rstrip("/") if not base_url.endswith("/v1/metrics"): base_url = f"{base_url}/v1/metrics" return HTTPExporter(endpoint=base_url, headers=headers) - else: - return GRPCExporter(endpoint=trimmed_endpoint, headers=headers) + grpc_endpoint, insecure = _resolve_grpc_exporter_endpoint(trimmed_endpoint) + return GRPCExporter(endpoint=grpc_endpoint, headers=headers, insecure=insecure) def init_metrics_provider( diff --git a/packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py b/packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py index c133980d42..81d0d3bbb2 100644 --- a/packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py +++ b/packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py @@ -3,6 +3,7 @@ # Original source: https://github.com/traceloop/openllmetry import atexit +import ipaddress import logging import os from urllib.parse import urlparse @@ -44,6 +45,7 @@ TRACER_NAME = "fortifyroot.tracer" +LOCAL_EXPORT_HOSTS = {"localhost"} EXCLUDED_URLS = """ iam.cloud.ibm.com, dataplatform.cloud.ibm.com, @@ -65,6 +67,57 @@ openaipublic.blob.core.windows.net""" +def _is_local_export_host(host: Optional[str]) -> bool: + if not host: + return False + normalized = host.strip().lower() + if normalized in LOCAL_EXPORT_HOSTS: + return True + try: + return ipaddress.ip_address(normalized).is_loopback + except ValueError: + return False + + +def _resolve_grpc_exporter_endpoint(api_endpoint: str) -> tuple[str, bool]: + trimmed_endpoint = api_endpoint.strip() + if "://" not in trimmed_endpoint: + return trimmed_endpoint, False + + parsed = urlparse(trimmed_endpoint) + if parsed.username or parsed.password: + raise ValueError("OTLP exporter endpoints must not include username or password") + scheme = parsed.scheme.lower() + if scheme == "grpcs": + return parsed.netloc, False + if scheme == "grpc": + if not _is_local_export_host(parsed.hostname): + raise ValueError( + "grpc:// OTLP export is insecure and is allowed only for local " + "development endpoints; use grpcs:// or https:// for remote collectors" + ) + return parsed.netloc, True + + raise ValueError( + f"Unsupported OTLP exporter endpoint scheme {parsed.scheme!r}; " + "use https://, http://, grpcs://, grpc://localhost, or a bare secure gRPC host:port" + ) + + +def _validate_http_exporter_endpoint(api_endpoint: str) -> None: + parsed = urlparse(api_endpoint.strip()) + if parsed.username or parsed.password: + raise ValueError("OTLP exporter endpoints must not include username or password") + if parsed.scheme.lower() != "http": + return + if _is_local_export_host(parsed.hostname): + return + raise ValueError( + "http:// OTLP export is insecure and is allowed only for local " + "development endpoints; use https:// for remote collectors" + ) + + class TracerWrapper(object): resource_attributes: dict = {} enable_content_tracing: bool = True @@ -334,9 +387,9 @@ def init_spans_exporter(api_endpoint: str, headers: Dict[str, str]) -> SpanExpor Supported schemes: - http:// or https:// → HTTP exporter - - grpc:// → gRPC exporter (insecure) + - grpc://localhost → gRPC exporter (insecure, local development only) - grpcs:// → gRPC exporter (secure/TLS) - - No scheme → gRPC exporter (insecure, for backward compatibility) + - No scheme → gRPC exporter (secure/TLS) Args: api_endpoint: The endpoint URL (with or without scheme) @@ -349,6 +402,7 @@ def init_spans_exporter(api_endpoint: str, headers: Dict[str, str]) -> SpanExpor match parsed.scheme.lower(): case "http" | "https": + _validate_http_exporter_endpoint(api_endpoint) base_url = api_endpoint.strip().rstrip('/') if not base_url.endswith('/v1/traces'): endpoint = f"{base_url}/v1/traces" @@ -356,17 +410,19 @@ def init_spans_exporter(api_endpoint: str, headers: Dict[str, str]) -> SpanExpor endpoint = base_url return HTTPExporter(endpoint=endpoint, headers=headers) case "grpc": + grpc_endpoint, insecure = _resolve_grpc_exporter_endpoint(api_endpoint) return GRPCExporter( - endpoint=parsed.netloc, headers=headers, insecure=True + endpoint=grpc_endpoint, headers=headers, insecure=insecure ) case "grpcs": + grpc_endpoint, insecure = _resolve_grpc_exporter_endpoint(api_endpoint) return GRPCExporter( - endpoint=parsed.netloc, headers=headers, insecure=False + endpoint=grpc_endpoint, headers=headers, insecure=insecure ) case _: - # No scheme → default to insecure gRPC for backward compatibility + grpc_endpoint, insecure = _resolve_grpc_exporter_endpoint(api_endpoint) return GRPCExporter( - endpoint=api_endpoint.strip(), headers=headers, insecure=True + endpoint=grpc_endpoint, headers=headers, insecure=insecure )