diff --git a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py index ba99adc..112f8cc 100644 --- a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py @@ -56,6 +56,8 @@ class FastAPIConfig( prometheus_expose_params: dict[str, typing.Any] = dataclasses.field(default_factory=dict) def __post_init__(self) -> None: + # @dataclass(slots=True) replaces the class object, breaking bare super(). + super(FastAPIConfig, self).__post_init__() if not import_checker.is_fastapi_installed: msg = "fastapi is not installed" raise ConfigurationError(msg) @@ -65,14 +67,11 @@ def __post_init__(self) -> None: # FastAPIConfig stays frozen for user-facing immutability; __post_init__ needs # to set application after construction, so we bypass the freeze here. object.__setattr__(self, "application", application) - else: - application = self.application - if self.application_kwargs: - warnings.warn("application_kwargs must be used without application", stacklevel=2) - - application.title = self.service_name - application.debug = self.service_debug - application.version = self.service_version + application.title = self.service_name + application.debug = self.service_debug + application.version = self.service_version + elif self.application_kwargs: + warnings.warn("application_kwargs must be used without application", stacklevel=2) def _narrow_app(config: "FastAPIConfig") -> "fastapi.FastAPI": diff --git a/lite_bootstrap/bootstrappers/faststream_bootstrapper.py b/lite_bootstrap/bootstrappers/faststream_bootstrapper.py index 6d78517..6ac2b89 100644 --- a/lite_bootstrap/bootstrappers/faststream_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/faststream_bootstrapper.py @@ -66,7 +66,9 @@ class FastStreamConfig( ): application: "AsgiFastStream" = dataclasses.field(default_factory=_make_asgi_faststream) opentelemetry_middleware_cls: type[FastStreamTelemetryMiddlewareProtocol] | None = None + opentelemetry_excluded_urls: list[str] = dataclasses.field(default_factory=list) prometheus_middleware_cls: type[FastStreamPrometheusMiddlewareProtocol] | None = None + prometheus_collector_registry: "prometheus_client.CollectorRegistry | None" = None faststream_log_level: int = logging.WARNING faststream_health_check_broker_timeout: float = 5.0 @@ -156,12 +158,14 @@ def _make_collector_registry() -> "prometheus_client.CollectorRegistry": @dataclasses.dataclass(kw_only=True) class FastStreamPrometheusInstrument(PrometheusInstrument): bootstrap_config: FastStreamConfig - collector_registry: "prometheus_client.CollectorRegistry" = dataclasses.field( - default_factory=_make_collector_registry, init=False - ) + collector_registry: "prometheus_client.CollectorRegistry" = dataclasses.field(init=False) not_ready_message = PrometheusInstrument.not_ready_message + " or prometheus_middleware_cls is missing" missing_dependency_message = "prometheus_client is not installed" + def __post_init__(self) -> None: + injected = self.bootstrap_config.prometheus_collector_registry + self.collector_registry = injected if injected is not None else _make_collector_registry() + @classmethod def is_configured(cls, bootstrap_config: "FastStreamConfig") -> bool: # ty: ignore[invalid-method-override] return super().is_configured(bootstrap_config) and bool(bootstrap_config.prometheus_middleware_cls) diff --git a/lite_bootstrap/helpers/fastapi_helpers.py b/lite_bootstrap/helpers/fastapi_helpers.py index 2972819..76518de 100644 --- a/lite_bootstrap/helpers/fastapi_helpers.py +++ b/lite_bootstrap/helpers/fastapi_helpers.py @@ -1,8 +1,10 @@ import pathlib import typing +import warnings from lite_bootstrap import import_checker from lite_bootstrap.exceptions import ConfigurationError +from lite_bootstrap.helpers.path import is_valid_path if import_checker.is_fastapi_installed: @@ -13,6 +15,26 @@ from starlette.routing import Route +def _safe_root_path(scope_root_path: str) -> str: + """Strip trailing slash and validate against the project's path allowlist. + + An empty ``root_path`` is the normal case (no proxy prefix) and is allowed without warning. + Any other path that fails the ``is_valid_path`` regex is rejected (falls back to empty) so + that proxy-header-derived root paths can't inject HTML into the offline-docs response. + """ + candidate = scope_root_path.rstrip("/") + if not candidate: + return "" + if not is_valid_path(candidate): + warnings.warn( + f"root_path {candidate!r} contains characters outside the valid-path allowlist; " + "falling back to empty root_path to prevent HTML injection in offline docs.", + stacklevel=3, + ) + return "" + return candidate + + def enable_offline_docs( app: "FastAPI", static_path: str, @@ -36,7 +58,7 @@ def enable_offline_docs( @app.get(docs_url, include_in_schema=False) async def custom_swagger_ui_html(request: Request) -> HTMLResponse: - root_path = request.scope.get("root_path", "").rstrip("/") + root_path = _safe_root_path(request.scope.get("root_path", "")) return get_swagger_ui_html( openapi_url=f"{root_path}{app_openapi_url}", title=f"{app.title} - Swagger UI", @@ -51,7 +73,7 @@ async def swagger_ui_redirect() -> HTMLResponse: @app.get(redoc_url, include_in_schema=False) async def redoc_html(request: Request) -> HTMLResponse: - root_path = request.scope.get("root_path", "").rstrip("/") + root_path = _safe_root_path(request.scope.get("root_path", "")) return get_redoc_html( openapi_url=f"{root_path}{app_openapi_url}", title=f"{app.title} - ReDoc", diff --git a/lite_bootstrap/instruments/base.py b/lite_bootstrap/instruments/base.py index 7d7f159..f511250 100644 --- a/lite_bootstrap/instruments/base.py +++ b/lite_bootstrap/instruments/base.py @@ -12,6 +12,14 @@ class BaseConfig: service_environment: str | None = None service_debug: bool = True + def __post_init__(self) -> None: + """Terminate the MRO __post_init__ cascade safely. + + Subclasses call super().__post_init__() to propagate through multiple-inheritance + chains (e.g. FastAPIConfig → CorsConfig → OpenTelemetryConfig → BaseConfig). + Without this no-op, the chain would raise AttributeError on object. + """ + @classmethod def from_dict(cls, data: dict[str, typing.Any]) -> typing_extensions.Self: """Build a config from a dict; unknown keys are silently dropped, explicit None overrides defaults.""" diff --git a/lite_bootstrap/instruments/cors_instrument.py b/lite_bootstrap/instruments/cors_instrument.py index e00ccc0..3188d56 100644 --- a/lite_bootstrap/instruments/cors_instrument.py +++ b/lite_bootstrap/instruments/cors_instrument.py @@ -1,8 +1,13 @@ import dataclasses +import typing +from lite_bootstrap.exceptions import ConfigurationError from lite_bootstrap.instruments.base import BaseConfig, BaseInstrument +_PERMISSIVE_ORIGIN_REGEX: typing.Final[frozenset[str]] = frozenset({".*", r".+"}) + + @dataclasses.dataclass(kw_only=True, frozen=True) class CorsConfig(BaseConfig): cors_allowed_origins: list[str] = dataclasses.field(default_factory=list) @@ -13,6 +18,19 @@ class CorsConfig(BaseConfig): cors_allowed_origin_regex: str | None = None cors_max_age: int = 600 + def __post_init__(self) -> None: + if self.cors_allowed_credentials: + wildcard_in_origins = "*" in self.cors_allowed_origins + permissive_regex = self.cors_allowed_origin_regex in _PERMISSIVE_ORIGIN_REGEX + if wildcard_in_origins or permissive_regex: + msg = ( + "Unsafe CORS configuration: cors_allowed_credentials=True combined with a " + "wildcard origin is rejected by browsers and is a security misconfiguration. " + "Use an explicit list of allowed origins (or a narrow regex)." + ) + raise ConfigurationError(msg) + super().__post_init__() + @dataclasses.dataclass(kw_only=True, slots=True) class CorsInstrument(BaseInstrument[CorsConfig]): diff --git a/lite_bootstrap/instruments/opentelemetry_instrument.py b/lite_bootstrap/instruments/opentelemetry_instrument.py index 65ca701..e2a6781 100644 --- a/lite_bootstrap/instruments/opentelemetry_instrument.py +++ b/lite_bootstrap/instruments/opentelemetry_instrument.py @@ -2,6 +2,8 @@ import logging import os import typing +import urllib.parse +import warnings from lite_bootstrap import import_checker from lite_bootstrap.instruments.base import BaseConfig, BaseInstrument @@ -38,6 +40,9 @@ class OpenTelemetryServiceFieldsConfig(BaseConfig): opentelemetry_namespace: str | None = None +_LOCAL_HOSTS: typing.Final[frozenset[str]] = frozenset({"localhost", "127.0.0.1", "::1", ""}) + + @dataclasses.dataclass(kw_only=True, frozen=True) class OpenTelemetryConfig(OpenTelemetryServiceFieldsConfig): opentelemetry_container_name: str | None = dataclasses.field( @@ -51,6 +56,33 @@ class OpenTelemetryConfig(OpenTelemetryServiceFieldsConfig): opentelemetry_log_traces: bool = False opentelemetry_generate_health_check_spans: bool = True + def __post_init__(self) -> None: + host = self._parse_remote_insecure_host() + if host is not None: + warnings.warn( + f"OTLP exporter sending traces unencrypted to non-local host {host!r}; " + "set opentelemetry_insecure=False or use a localhost/unix endpoint.", + stacklevel=2, + ) + super().__post_init__() + + def _parse_remote_insecure_host(self) -> str | None: + """Return the host name if the endpoint is insecure AND non-local; else None.""" + if not self.opentelemetry_endpoint or not self.opentelemetry_insecure: + return None + if self.opentelemetry_endpoint.startswith("unix://"): + return None + # urlparse treats schemeless input as `path`, misparsing `host:port` forms. + # Prepend `//` so urlparse always sees a network-location-style input. + raw = self.opentelemetry_endpoint + if "://" not in raw: + raw = f"//{raw}" + parsed = urllib.parse.urlparse(raw) + host = (parsed.hostname or "").lower() + if host in _LOCAL_HOSTS: + return None + return host + if import_checker.is_opentelemetry_installed and import_checker.is_pyroscope_installed: _OTEL_PROFILE_ID_KEY: typing.Final = "pyroscope.profile.id" diff --git a/planning/plans/2026-06-05-pr2-config-security.md b/planning/plans/2026-06-05-pr2-config-security.md new file mode 100644 index 0000000..f319532 --- /dev/null +++ b/planning/plans/2026-06-05-pr2-config-security.md @@ -0,0 +1,1003 @@ +# PR2 — Config UX & Security Validation Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Land six config-layer fixes from the 2026-06-05 audit (UX-1, UX-2, UX-3, SEC-1, SEC-2, SEC-3 + folded TEST-NEW-1 and TEST-NEW-6) in one PR. + +**Architecture:** Each fix is a small TDD cycle. The work all happens at config-construction time or at the user-facing helper layer — no instrument lifecycle changes. Tasks are ordered to land additive UX improvements first (UX-1, UX-2, UX-3), then the security validators (SEC-1, SEC-2, SEC-3) which are slightly more cross-cutting (SEC-2 introduces an `OpenTelemetryConfig.__post_init__` that interacts with the FastAPIConfig override touched in Task 1). + +**Tech Stack:** Python 3.10+, `uv` workspace, `pytest` (with `pytest-asyncio`), `ty` type checker, `ruff` formatter, `structlog`, `prometheus_client`, `opentelemetry-sdk`, `fastapi`, `litestar`, `faststream`, `fastmcp`. + +**Branch:** `fix/bug-audit-v2-pr2-config-security` (branch off `main` after PR1 merges, or off PR1's branch if parallel work is desired). + +**Sequencing prerequisite:** PR1 should merge first. PR2 doesn't structurally depend on PR1, but Task 5 introduces `OpenTelemetryConfig.__post_init__` which interacts with the `_prior_logger_disabled` field added in PR1 Task 4 (both touch `OpenTelemetryConfig`/`OpenTelemetryInstrument`). Rebasing PR2 onto a merged PR1 keeps the conflict resolution trivial. + +--- + +## File Structure + +Modifications to existing files only — no new files. + +| File | What changes | +|------|-------------| +| `lite_bootstrap/bootstrappers/fastapi_bootstrapper.py` | Move user-app field overrides into UnsetType branch (Task 1); add `super().__post_init__()` for Task 5 cascade | +| `lite_bootstrap/bootstrappers/faststream_bootstrapper.py` | Add `prometheus_collector_registry` field + thread to instrument (Task 2); add `opentelemetry_excluded_urls` field (Task 3) | +| `lite_bootstrap/helpers/fastapi_helpers.py` | Validate `root_path` via `is_valid_path` + fall back on warning (Task 4) | +| `lite_bootstrap/instruments/cors_instrument.py` | Add `__post_init__` to `CorsConfig` rejecting wildcard + credentials combo (Task 6) | +| `lite_bootstrap/instruments/opentelemetry_instrument.py` | Add `__post_init__` to `OpenTelemetryConfig` warning on insecure non-local endpoint (Task 5) | +| `tests/test_fastapi_bootstrap.py` | Test for Task 1 | +| `tests/test_fastapi_offline_docs.py` | Test for Task 4 | +| `tests/test_faststream_bootstrap.py` | Tests for Tasks 2, 3 | +| `tests/instruments/test_cors_instrument.py` | Test for Task 6 | +| `tests/instruments/test_opentelemetry_instrument.py` | Test for Task 5 | + +--- + +## Task 1: UX-1 — `FastAPIConfig` respects user app's title/debug/version + +**Files:** +- Modify: `lite_bootstrap/bootstrappers/fastapi_bootstrapper.py:58-75` +- Test: `tests/test_fastapi_bootstrap.py` + +- [ ] **Step 1: Write the failing test** + +Add to `tests/test_fastapi_bootstrap.py` (at the bottom, after existing tests): + +```python +def test_user_supplied_app_keeps_title_version_debug() -> None: + user_app = fastapi.FastAPI(title="user-title", version="9.9.9", debug=False) + config = FastAPIConfig( + application=user_app, + service_name="lite-name", + service_version="1.0.0", + service_debug=True, + ) + assert config.application is user_app + assert user_app.title == "user-title" + assert user_app.version == "9.9.9" + assert user_app.debug is False +``` + +`fastapi`, `FastAPIConfig` are already imported in this file. + +- [ ] **Step 2: Run test to verify it fails** + +```bash +just test -- tests/test_fastapi_bootstrap.py::test_user_supplied_app_keeps_title_version_debug +``` + +Expected: FAIL — current `__post_init__` overwrites `application.title/.debug/.version` with the config defaults even when the user supplies a pre-configured app. + +- [ ] **Step 3: Move the field overrides into the UnsetType branch** + +In `lite_bootstrap/bootstrappers/fastapi_bootstrapper.py`, the current `__post_init__` (lines 58-75) reads: + +```python +def __post_init__(self) -> None: + if not import_checker.is_fastapi_installed: + msg = "fastapi is not installed" + raise ConfigurationError(msg) + + if isinstance(self.application, UnsetType): + application = fastapi.FastAPI(docs_url=self.swagger_path, **self.application_kwargs) + # FastAPIConfig stays frozen for user-facing immutability; __post_init__ needs + # to set application after construction, so we bypass the freeze here. + object.__setattr__(self, "application", application) + else: + application = self.application + if self.application_kwargs: + warnings.warn("application_kwargs must be used without application", stacklevel=2) + + application.title = self.service_name + application.debug = self.service_debug + application.version = self.service_version +``` + +Replace with: + +```python +def __post_init__(self) -> None: + if not import_checker.is_fastapi_installed: + msg = "fastapi is not installed" + raise ConfigurationError(msg) + + if isinstance(self.application, UnsetType): + application = fastapi.FastAPI(docs_url=self.swagger_path, **self.application_kwargs) + # FastAPIConfig stays frozen for user-facing immutability; __post_init__ needs + # to set application after construction, so we bypass the freeze here. + object.__setattr__(self, "application", application) + application.title = self.service_name + application.debug = self.service_debug + application.version = self.service_version + elif self.application_kwargs: + warnings.warn("application_kwargs must be used without application", stacklevel=2) +``` + +Key changes: +- Three `application.X = ...` lines moved inside the `if isinstance(...)` branch +- The `else: application = self.application` line is gone (no longer needed since we don't reference `application` after the branch) +- `else: if self.application_kwargs:` collapsed to `elif self.application_kwargs:` + +- [ ] **Step 4: Run the new test to verify it passes** + +```bash +just test -- tests/test_fastapi_bootstrap.py::test_user_supplied_app_keeps_title_version_debug +``` + +Expected: PASS. + +- [ ] **Step 5: Verify nothing else broke** + +```bash +just test -- tests/test_fastapi_bootstrap.py tests/test_fastapi_offline_docs.py +``` + +Expected: all green. In particular `test_fastapi_bootstrapper_apps_and_kwargs_warning` still triggers when both `application` and `application_kwargs` are passed. + +- [ ] **Step 6: Run full suite + lint** + +```bash +just test +just lint-ci +``` + +Expected: 165 passed (164 + 1 new), 100% coverage, lint clean. + +- [ ] **Step 7: Commit** + +```bash +git add lite_bootstrap/bootstrappers/fastapi_bootstrapper.py tests/test_fastapi_bootstrap.py +git commit -m "$(cat <<'EOF' +fix: FastAPIConfig respects user-supplied app title/version/debug (UX-1) + +Move the application.title/.debug/.version assignments inside the UnsetType +branch so they only apply when lite-bootstrap built the FastAPI() instance. +Pre-configured user apps now keep their construction-time values. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 2: UX-2 — Injectable `prometheus_collector_registry` on FastStream + +**Files:** +- Modify: `lite_bootstrap/bootstrappers/faststream_bootstrapper.py:63-72` (add config field), `:143-167` (thread to instrument) +- Test: `tests/test_faststream_bootstrap.py` + +- [ ] **Step 1: Write the failing test** + +Add to `tests/test_faststream_bootstrap.py`: + +```python +import uuid + + +async def test_faststream_prometheus_uses_injected_registry(broker: RedisBroker) -> None: + custom_registry = prometheus_client.CollectorRegistry() + counter_name = f"injected_counter_{uuid.uuid4().hex}_total" + counter = prometheus_client.Counter(counter_name, "Injected registry counter", registry=custom_registry) + counter.inc() + + bootstrap_config = dataclasses.replace( + build_faststream_config(broker=broker), + prometheus_collector_registry=custom_registry, + ) + bootstrapper = FastStreamBootstrapper(bootstrap_config=bootstrap_config) + application = bootstrapper.bootstrap() + try: + with TestClient(app=application) as test_client, TestRedisBroker(broker): + response = test_client.get(bootstrap_config.prometheus_metrics_path) + assert response.status_code == status.HTTP_200_OK + assert counter_name.encode() in response.content + finally: + bootstrapper.teardown() +``` + +Imports — verify `prometheus_client` is imported at module level (it isn't yet in this test file). Add: + +```python +import prometheus_client +``` + +Near the existing `import` block at the top of `tests/test_faststream_bootstrap.py`. `dataclasses`, `uuid`, `status`, `TestClient`, `TestRedisBroker`, `RedisBroker` are already imported or will be from new imports. + +- [ ] **Step 2: Run test to verify it fails** + +```bash +just test -- tests/test_faststream_bootstrap.py::test_faststream_prometheus_uses_injected_registry +``` + +Expected: FAIL — `FastStreamConfig` doesn't have `prometheus_collector_registry` field; `dataclasses.replace(...)` raises `TypeError: __init__() got an unexpected keyword argument 'prometheus_collector_registry'`. + +- [ ] **Step 3: Add the config field** + +In `lite_bootstrap/bootstrappers/faststream_bootstrapper.py`, the current `FastStreamConfig` (lines 63-72) reads: + +```python +@dataclasses.dataclass(kw_only=True, slots=True, frozen=True) +class FastStreamConfig( + HealthChecksConfig, LoggingConfig, OpenTelemetryConfig, PrometheusConfig, PyroscopeConfig, SentryConfig +): + application: "AsgiFastStream" = dataclasses.field(default_factory=_make_asgi_faststream) + opentelemetry_middleware_cls: type[FastStreamTelemetryMiddlewareProtocol] | None = None + prometheus_middleware_cls: type[FastStreamPrometheusMiddlewareProtocol] | None = None + faststream_log_level: int = logging.WARNING + faststream_health_check_broker_timeout: float = 5.0 +``` + +Add a `prometheus_collector_registry` field after `prometheus_middleware_cls`: + +```python +@dataclasses.dataclass(kw_only=True, slots=True, frozen=True) +class FastStreamConfig( + HealthChecksConfig, LoggingConfig, OpenTelemetryConfig, PrometheusConfig, PyroscopeConfig, SentryConfig +): + application: "AsgiFastStream" = dataclasses.field(default_factory=_make_asgi_faststream) + opentelemetry_middleware_cls: type[FastStreamTelemetryMiddlewareProtocol] | None = None + prometheus_middleware_cls: type[FastStreamPrometheusMiddlewareProtocol] | None = None + prometheus_collector_registry: "prometheus_client.CollectorRegistry | None" = None + faststream_log_level: int = logging.WARNING + faststream_health_check_broker_timeout: float = 5.0 +``` + +The string-annotation form `"prometheus_client.CollectorRegistry | None"` works because `prometheus_client` is imported conditionally at module scope (`if import_checker.is_prometheus_client_installed: import prometheus_client`). Dataclass field type annotations are not resolved at runtime by default, so the conditional import is fine. + +- [ ] **Step 4: Thread the field into `FastStreamPrometheusInstrument`** + +Currently (lines 143-167): + +```python +@dataclasses.dataclass(kw_only=True) +class FastStreamPrometheusInstrument(PrometheusInstrument): + bootstrap_config: FastStreamConfig + collector_registry: "prometheus_client.CollectorRegistry" = dataclasses.field( + default_factory=_make_collector_registry, init=False + ) + not_ready_message = PrometheusInstrument.not_ready_message + " or prometheus_middleware_cls is missing" + missing_dependency_message = "prometheus_client is not installed" + ... +``` + +Replace the `collector_registry` field declaration with an `init=False` field set in `__post_init__`: + +```python +@dataclasses.dataclass(kw_only=True) +class FastStreamPrometheusInstrument(PrometheusInstrument): + bootstrap_config: FastStreamConfig + collector_registry: "prometheus_client.CollectorRegistry" = dataclasses.field(init=False) + not_ready_message = PrometheusInstrument.not_ready_message + " or prometheus_middleware_cls is missing" + missing_dependency_message = "prometheus_client is not installed" + + def __post_init__(self) -> None: + injected = self.bootstrap_config.prometheus_collector_registry + self.collector_registry = injected if injected is not None else _make_collector_registry() + + @classmethod + def is_configured(cls, bootstrap_config: "FastStreamConfig") -> bool: # ty: ignore[invalid-method-override] + return super().is_configured(bootstrap_config) and bool(bootstrap_config.prometheus_middleware_cls) + ... +``` + +`__post_init__` runs after dataclass `__init__`; assignment via `self.collector_registry = ...` works because the instrument is non-frozen. + +Keep the rest of the class unchanged (the `is_configured`, `check_dependencies`, `bootstrap` methods). + +- [ ] **Step 5: Run the new test to verify it passes** + +```bash +just test -- tests/test_faststream_bootstrap.py::test_faststream_prometheus_uses_injected_registry +``` + +Expected: PASS. + +- [ ] **Step 6: Verify default-path still works** + +```bash +just test -- tests/test_faststream_bootstrap.py +``` + +Expected: all green. Existing `test_faststream_bootstrap` (which doesn't supply `prometheus_collector_registry`) continues to use the per-instance default. + +- [ ] **Step 7: Full suite + lint** + +```bash +just test +just lint-ci +``` + +Expected: 166 passed (165 + 1 new), 100% coverage, lint clean. + +- [ ] **Step 8: Commit** + +```bash +git add lite_bootstrap/bootstrappers/faststream_bootstrapper.py tests/test_faststream_bootstrap.py +git commit -m "$(cat <<'EOF' +feat: support injectable Prometheus CollectorRegistry on FastStream (UX-2) + +Add prometheus_collector_registry: CollectorRegistry | None config field on +FastStreamConfig. When non-None, FastStreamPrometheusInstrument uses the +injected registry; otherwise the existing per-instance default is preserved. +Lets users expose metrics that were registered on a shared registry. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 3: UX-3 — Add `opentelemetry_excluded_urls` to FastStreamConfig + +**Files:** +- Modify: `lite_bootstrap/bootstrappers/faststream_bootstrapper.py:63-72` +- Test: `tests/test_faststream_bootstrap.py` + +- [ ] **Step 1: Write the failing test** + +Add to `tests/test_faststream_bootstrap.py`: + +```python +def test_faststream_opentelemetry_excluded_urls_in_built_set(broker: RedisBroker) -> None: + from lite_bootstrap.bootstrappers.faststream_bootstrapper import ( + FastStreamOpenTelemetryInstrument, + ) + + bootstrap_config = dataclasses.replace( + build_faststream_config(broker=broker), + opentelemetry_excluded_urls=["/foo", "/bar"], + ) + instrument = FastStreamOpenTelemetryInstrument(bootstrap_config=bootstrap_config) + excluded = instrument._build_excluded_urls() # noqa: SLF001 + assert "/foo" in excluded + assert "/bar" in excluded +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +just test -- tests/test_faststream_bootstrap.py::test_faststream_opentelemetry_excluded_urls_in_built_set +``` + +Expected: FAIL — `dataclasses.replace(...)` raises because `FastStreamConfig` has no `opentelemetry_excluded_urls` field. + +- [ ] **Step 3: Add the field** + +In `lite_bootstrap/bootstrappers/faststream_bootstrapper.py`, the current `FastStreamConfig` (after Task 2's edit) reads approximately: + +```python +@dataclasses.dataclass(kw_only=True, slots=True, frozen=True) +class FastStreamConfig( + HealthChecksConfig, LoggingConfig, OpenTelemetryConfig, PrometheusConfig, PyroscopeConfig, SentryConfig +): + application: "AsgiFastStream" = dataclasses.field(default_factory=_make_asgi_faststream) + opentelemetry_middleware_cls: type[FastStreamTelemetryMiddlewareProtocol] | None = None + prometheus_middleware_cls: type[FastStreamPrometheusMiddlewareProtocol] | None = None + prometheus_collector_registry: "prometheus_client.CollectorRegistry | None" = None + faststream_log_level: int = logging.WARNING + faststream_health_check_broker_timeout: float = 5.0 +``` + +Add `opentelemetry_excluded_urls` after `opentelemetry_middleware_cls`: + +```python +@dataclasses.dataclass(kw_only=True, slots=True, frozen=True) +class FastStreamConfig( + HealthChecksConfig, LoggingConfig, OpenTelemetryConfig, PrometheusConfig, PyroscopeConfig, SentryConfig +): + application: "AsgiFastStream" = dataclasses.field(default_factory=_make_asgi_faststream) + opentelemetry_middleware_cls: type[FastStreamTelemetryMiddlewareProtocol] | None = None + opentelemetry_excluded_urls: list[str] = dataclasses.field(default_factory=list) + prometheus_middleware_cls: type[FastStreamPrometheusMiddlewareProtocol] | None = None + prometheus_collector_registry: "prometheus_client.CollectorRegistry | None" = None + faststream_log_level: int = logging.WARNING + faststream_health_check_broker_timeout: float = 5.0 +``` + +The behavior of `_build_excluded_urls` (in `opentelemetry_instrument.py`) is unchanged — it already reads via `getattr(self.bootstrap_config, "opentelemetry_excluded_urls", [])`. The new field is just for discoverability (IDE help, tab completion). + +- [ ] **Step 4: Run the new test to verify it passes** + +```bash +just test -- tests/test_faststream_bootstrap.py::test_faststream_opentelemetry_excluded_urls_in_built_set +``` + +Expected: PASS. + +- [ ] **Step 5: Full suite + lint** + +```bash +just test +just lint-ci +``` + +Expected: 167 passed (166 + 1 new), 100% coverage, lint clean. + +- [ ] **Step 6: Commit** + +```bash +git add lite_bootstrap/bootstrappers/faststream_bootstrapper.py tests/test_faststream_bootstrap.py +git commit -m "$(cat <<'EOF' +feat: add opentelemetry_excluded_urls to FastStreamConfig (UX-3) + +FastAPIConfig and LitestarConfig already expose opentelemetry_excluded_urls; +FastStream relied on getattr fallback with no discoverable field. Add the field +to FastStreamConfig matching the FastAPI/Litestar pattern. _build_excluded_urls +behavior is unchanged — the getattr access still works the same way. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 4: SEC-1 — Validate `root_path` in offline-docs HTML + +**Files:** +- Modify: `lite_bootstrap/helpers/fastapi_helpers.py` +- Test: `tests/test_fastapi_offline_docs.py` + +- [ ] **Step 1: Write the failing test** + +Add to `tests/test_fastapi_offline_docs.py`: + +```python +def test_offline_docs_rejects_unsafe_root_path() -> None: + malicious_root = "/foo" + app = FastAPI(title="Tests", root_path=malicious_root, docs_url="/custom_docs") + enable_offline_docs(app, static_path="/static") + + with TestClient(app, root_path=malicious_root) as client, pytest.warns(UserWarning, match="root_path"): + response = client.get("/custom_docs") + assert response.status_code == HTTPStatus.OK + assert "" not in response.text + assert "/static/swagger-ui.css" in response.text # falls back to empty root_path +``` + +`FastAPI`, `TestClient`, `HTTPStatus`, `pytest`, `enable_offline_docs` are already imported in this file. + +- [ ] **Step 2: Run test to verify it fails** + +```bash +just test -- tests/test_fastapi_offline_docs.py::test_offline_docs_rejects_unsafe_root_path +``` + +Expected: FAIL — current handler reflects `root_path` verbatim into the swagger HTML, so the `" + app = FastAPI(title="Tests", root_path=malicious_root, docs_url="/custom_docs", redoc_url="/custom_redoc") + enable_offline_docs(app, static_path="/static") + + with TestClient(app, root_path=malicious_root) as client: + with pytest.warns(UserWarning, match="root_path"): + swagger_response = client.get("/custom_docs") + with pytest.warns(UserWarning, match="root_path"): + redoc_response = client.get("/custom_redoc") + + assert swagger_response.status_code == HTTPStatus.OK + assert "" not in swagger_response.text + assert "/static/swagger-ui.css" in swagger_response.text # falls back to empty root_path + + assert redoc_response.status_code == HTTPStatus.OK + assert "" not in redoc_response.text + assert "/static/redoc.standalone.js" in redoc_response.text # falls back to empty root_path diff --git a/tests/test_faststream_bootstrap.py b/tests/test_faststream_bootstrap.py index 9126c38..7af2eba 100644 --- a/tests/test_faststream_bootstrap.py +++ b/tests/test_faststream_bootstrap.py @@ -1,9 +1,11 @@ import dataclasses import logging import typing +import uuid from unittest.mock import AsyncMock, patch import faststream.asgi +import prometheus_client import pytest import structlog from faststream._internal.broker import BrokerUsecase @@ -15,6 +17,7 @@ from starlette.testclient import TestClient from lite_bootstrap import FastStreamBootstrapper, FastStreamConfig +from lite_bootstrap.bootstrappers.faststream_bootstrapper import FastStreamOpenTelemetryInstrument from tests.conftest import ( CustomInstrumentor, SentryTestTransport, @@ -200,3 +203,36 @@ async def test_faststream_health_check_uses_configured_broker_timeout(broker: Re mock_ping.assert_called_once_with(timeout=expected_timeout) finally: bootstrapper.teardown() + + +def test_faststream_opentelemetry_excluded_urls_in_built_set(broker: RedisBroker) -> None: + bootstrap_config = dataclasses.replace( + build_faststream_config(broker=broker), + opentelemetry_excluded_urls=["/foo", "/bar"], + ) + instrument = FastStreamOpenTelemetryInstrument(bootstrap_config=bootstrap_config) + excluded = instrument._build_excluded_urls() # noqa: SLF001 + assert "/foo" in excluded + assert "/bar" in excluded + + +async def test_faststream_prometheus_uses_injected_registry(broker: RedisBroker) -> None: + custom_registry = prometheus_client.CollectorRegistry() + counter_name = f"injected_counter_{uuid.uuid4().hex}_total" + counter = prometheus_client.Counter(counter_name, "Injected registry counter", registry=custom_registry) + counter.inc() + + bootstrap_config = dataclasses.replace( + build_faststream_config(broker=broker), + prometheus_collector_registry=custom_registry, + ) + bootstrapper = FastStreamBootstrapper(bootstrap_config=bootstrap_config) + application = bootstrapper.bootstrap() + try: + with TestClient(app=application) as test_client: + async with TestRedisBroker(broker): + response = test_client.get(bootstrap_config.prometheus_metrics_path) + assert response.status_code == status.HTTP_200_OK + assert counter_name.encode() in response.content + finally: + bootstrapper.teardown()