|
| 1 | +"""Dependency spans must not carry credentials out of the process. |
| 2 | +
|
| 3 | +This is the reason ``requests`` and ``urllib3`` were pulled from the export allowlist once |
| 4 | +before: their spans write the full request URL, and providers put API keys in the query |
| 5 | +string (Gemini's ``?key=``, among others). httpx is the transport the OpenAI and Anthropic |
| 6 | +SDKs ride on, so admitting these scopes is only safe while the export boundary redacts URLs. |
| 7 | +
|
| 8 | +Each case runs in a subprocess because the tracer provider is process-global. |
| 9 | +""" |
| 10 | + |
| 11 | +import gzip |
| 12 | +import http.server |
| 13 | +import json |
| 14 | +import os |
| 15 | +import subprocess |
| 16 | +import sys |
| 17 | +import tempfile |
| 18 | +import threading |
| 19 | +from pathlib import Path |
| 20 | + |
| 21 | +import pytest |
| 22 | +from lfx.observability import APPLICATION_INSTRUMENTATION_SCOPES |
| 23 | + |
| 24 | + |
| 25 | +class _Collector(http.server.BaseHTTPRequestHandler): |
| 26 | + def do_POST(self) -> None: |
| 27 | + body = self.rfile.read(int(self.headers.get("Content-Length", 0))) |
| 28 | + if self.headers.get("Content-Encoding") == "gzip": |
| 29 | + body = gzip.decompress(body) |
| 30 | + self.server.requests.append((self.path, body)) |
| 31 | + self.send_response(200) |
| 32 | + self.send_header("Content-Length", "0") |
| 33 | + self.end_headers() |
| 34 | + |
| 35 | + def log_message(self, *args) -> None: |
| 36 | + pass |
| 37 | + |
| 38 | + |
| 39 | +@pytest.fixture |
| 40 | +def collector(): |
| 41 | + server = http.server.HTTPServer(("127.0.0.1", 0), _Collector) |
| 42 | + server.requests = [] |
| 43 | + thread = threading.Thread(target=server.serve_forever, daemon=True) |
| 44 | + thread.start() |
| 45 | + yield server |
| 46 | + server.shutdown() |
| 47 | + server.server_close() |
| 48 | + thread.join(timeout=5) |
| 49 | + |
| 50 | + |
| 51 | +SECRET = "lfx-serve-api-key-NOT-REAL" # noqa: S105 # pragma: allowlist secret |
| 52 | + |
| 53 | +# The server span is already allowlisted, and ``lfx serve`` accepts its API key as a query |
| 54 | +# parameter, so this is a live path today rather than a hypothetical one. |
| 55 | +SERVER_SPAN_PROBE = f""" |
| 56 | +import asyncio, json |
| 57 | +
|
| 58 | +from opentelemetry import trace |
| 59 | +from opentelemetry.sdk.trace import TracerProvider |
| 60 | +from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter |
| 61 | +
|
| 62 | +from lfx.observability import ApplicationOnlySpanProcessor, instrument_fastapi_app |
| 63 | +
|
| 64 | +exporter = InMemorySpanExporter() |
| 65 | +provider = TracerProvider() |
| 66 | +provider.add_span_processor(ApplicationOnlySpanProcessor(exporter)) |
| 67 | +trace.set_tracer_provider(provider) |
| 68 | +
|
| 69 | +from fastapi import FastAPI |
| 70 | +from httpx import ASGITransport, AsyncClient |
| 71 | +
|
| 72 | +app = FastAPI() |
| 73 | +
|
| 74 | +
|
| 75 | +@app.get("/flows/run") |
| 76 | +async def run(): |
| 77 | + return {{"ok": True}} |
| 78 | +
|
| 79 | +
|
| 80 | +instrument_fastapi_app(app) |
| 81 | +SECRET = {SECRET!r} |
| 82 | +
|
| 83 | +
|
| 84 | +async def main(): |
| 85 | + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://probe") as client: |
| 86 | + await client.get(f"/flows/run?x-api-key={{SECRET}}&flow=abc") |
| 87 | + provider.force_flush() |
| 88 | + spans = [ |
| 89 | + {{"name": s.name, "attrs": dict(s.attributes)}} |
| 90 | + for s in exporter.get_finished_spans() |
| 91 | + if s.attributes and "url.path" in s.attributes |
| 92 | + ] |
| 93 | + print("PROBE_RESULT " + json.dumps({{"spans": spans}})) |
| 94 | +
|
| 95 | +
|
| 96 | +asyncio.run(main()) |
| 97 | +""" |
| 98 | + |
| 99 | + |
| 100 | +def run_probe(source: str) -> dict: |
| 101 | + env = {k: v for k, v in os.environ.items() if not k.startswith("OTEL_")} |
| 102 | + with tempfile.TemporaryDirectory() as tmp: |
| 103 | + probe_path = Path(tmp) / "probe.py" |
| 104 | + probe_path.write_text(source, encoding="utf-8") |
| 105 | + completed = subprocess.run( # noqa: S603 |
| 106 | + [sys.executable, str(probe_path)], |
| 107 | + env=env, |
| 108 | + capture_output=True, |
| 109 | + text=True, |
| 110 | + timeout=300, |
| 111 | + check=False, |
| 112 | + ) |
| 113 | + assert completed.returncode == 0, completed.stderr |
| 114 | + line = next(ln for ln in completed.stdout.splitlines() if ln.startswith("PROBE_RESULT ")) |
| 115 | + return json.loads(line.removeprefix("PROBE_RESULT ")) |
| 116 | + |
| 117 | + |
| 118 | +def test_the_serve_api_key_never_reaches_the_apm(): |
| 119 | + """Regression for a live leak: the key is accepted as a query param and the span kept it.""" |
| 120 | + result = run_probe(SERVER_SPAN_PROBE) |
| 121 | + |
| 122 | + assert result["spans"], "expected a server span carrying url.path" |
| 123 | + blob = json.dumps(result["spans"]) |
| 124 | + assert SECRET not in blob, f"serve API key reached the APM: {blob}" |
| 125 | + |
| 126 | + |
| 127 | +def test_redaction_keeps_the_route_an_operator_needs(): |
| 128 | + """Blanking everything would be safe and useless; path and method must survive.""" |
| 129 | + result = run_probe(SERVER_SPAN_PROBE) |
| 130 | + attrs = result["spans"][0]["attrs"] |
| 131 | + |
| 132 | + assert attrs["url.path"] == "/flows/run" |
| 133 | + assert attrs["url.query"] == "" |
| 134 | + assert attrs["http.request.method"] == "GET" |
| 135 | + |
| 136 | + |
| 137 | +def test_outbound_http_scopes_stay_out_of_the_allowlist(): |
| 138 | + """Redacting URLs does not make the LLM transports safe to export; that boundary stands.""" |
| 139 | + for scope in ( |
| 140 | + "opentelemetry.instrumentation.httpx", |
| 141 | + "opentelemetry.instrumentation.requests", |
| 142 | + "opentelemetry.instrumentation.urllib3", |
| 143 | + ): |
| 144 | + assert scope not in APPLICATION_INSTRUMENTATION_SCOPES |
| 145 | + |
| 146 | + |
| 147 | +def test_database_spans_are_allowlisted(): |
| 148 | + """Verified separately to carry bound-parameter placeholders, never row values.""" |
| 149 | + assert "opentelemetry.instrumentation.sqlalchemy" in APPLICATION_INSTRUMENTATION_SCOPES |
| 150 | + |
| 151 | + |
| 152 | +# The ticket's sampler criterion, run against the real bootstrap rather than a hand-built |
| 153 | +# provider: the same request at ratio 0.0 must export nothing and at 1.0 must export the lot. |
| 154 | +SAMPLER_PROBE = """ |
| 155 | +import json |
| 156 | +from opentelemetry import trace |
| 157 | +from lfx.observability import APPLICATION_TRACER_NAME, bootstrap_application_telemetry |
| 158 | +
|
| 159 | +telemetry = bootstrap_application_telemetry() |
| 160 | +tracer = trace.get_tracer(APPLICATION_TRACER_NAME) |
| 161 | +for _ in range(20): |
| 162 | + with tracer.start_as_current_span("flow.execute"): |
| 163 | + pass |
| 164 | +telemetry.shutdown() |
| 165 | +print("PROBE_RESULT done") |
| 166 | +""" |
| 167 | + |
| 168 | + |
| 169 | +def _run_sampler_probe(endpoint: str, ratio: str) -> None: |
| 170 | + env = {k: v for k, v in os.environ.items() if not k.startswith("OTEL_")} |
| 171 | + env["OTEL_EXPORTER_OTLP_ENDPOINT"] = endpoint |
| 172 | + env["OTEL_EXPORTER_OTLP_PROTOCOL"] = "http/protobuf" |
| 173 | + env["OTEL_TRACES_SAMPLER"] = "traceidratio" |
| 174 | + env["OTEL_TRACES_SAMPLER_ARG"] = ratio |
| 175 | + completed = subprocess.run( # noqa: S603 |
| 176 | + [sys.executable, "-c", SAMPLER_PROBE], |
| 177 | + env=env, |
| 178 | + capture_output=True, |
| 179 | + text=True, |
| 180 | + timeout=300, |
| 181 | + check=False, |
| 182 | + ) |
| 183 | + assert completed.returncode == 0, completed.stderr |
| 184 | + |
| 185 | + |
| 186 | +def _exported_span_count(requests_seen) -> int: |
| 187 | + from opentelemetry.proto.collector.trace.v1.trace_service_pb2 import ExportTraceServiceRequest |
| 188 | + |
| 189 | + total = 0 |
| 190 | + for path, body in requests_seen: |
| 191 | + if path != "/v1/traces": |
| 192 | + continue |
| 193 | + request = ExportTraceServiceRequest() |
| 194 | + request.ParseFromString(body) |
| 195 | + for rs in request.resource_spans: |
| 196 | + for ss in rs.scope_spans: |
| 197 | + total += len(ss.spans) |
| 198 | + return total |
| 199 | + |
| 200 | + |
| 201 | +def test_sampling_off_exports_nothing(collector): |
| 202 | + port = collector.server_address[1] |
| 203 | + _run_sampler_probe(f"http://127.0.0.1:{port}", "0.0") |
| 204 | + |
| 205 | + assert _exported_span_count(collector.requests) == 0 |
| 206 | + |
| 207 | + |
| 208 | +def test_sampling_on_exports_every_span(collector): |
| 209 | + port = collector.server_address[1] |
| 210 | + _run_sampler_probe(f"http://127.0.0.1:{port}", "1.0") |
| 211 | + |
| 212 | + assert _exported_span_count(collector.requests) == 20 |
0 commit comments