From c122168e9cd165ef9684e4b781dd3ec37a67def6 Mon Sep 17 00:00:00 2001 From: ghosteau Date: Wed, 2 Sep 2026 23:24:31 -0400 Subject: [PATCH] fix: parse list settings from env instead of crashing at startup The container never started. pydantic-settings treats list-typed fields as complex and JSON-decodes the environment value inside the settings source, which runs before field validators -- so CORS_ORIGINS=https://embeddings. mannymcgrail.com failed json.loads and raised SettingsError at import, before _split_csv ever saw it. ALLOWED_MODELS would have failed identically on the next line. Development never hit this because both fields fall back to default_factory when unset, and they are only set in deployment. Marking both fields NoDecode hands the raw string to the validator. That on its own would have silently dropped support for JSON array values, which is what pydantic-settings accepted before, so the validator now takes either form. Adds test_config.py covering both, plus the exact environment docker-compose.yml sets. Raises the pydantic-settings floor to 2.3, which introduced NoDecode. Co-Authored-By: Claude Opus 5 --- backend/app/config.py | 38 ++++++++++---- backend/requirements.txt | 2 +- backend/tests/test_config.py | 97 ++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 10 deletions(-) create mode 100644 backend/tests/test_config.py diff --git a/backend/app/config.py b/backend/app/config.py index 7be9025..cf7d69c 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -7,11 +7,13 @@ from __future__ import annotations +import json from functools import lru_cache from pathlib import Path +from typing import Annotated from pydantic import Field, field_validator -from pydantic_settings import BaseSettings, SettingsConfigDict +from pydantic_settings import BaseSettings, NoDecode, SettingsConfigDict class Settings(BaseSettings): @@ -50,7 +52,13 @@ class Settings(BaseSettings): # CORS: comma-separated list of allowed origins. Defaults to the common # Vite dev-server origins. In production set this to your real frontend URL. - cors_origins: list[str] = Field( + # NoDecode is load-bearing: without it pydantic-settings treats a list + # field as "complex" and JSON-decodes the environment value inside the + # settings source, which runs *before* field validators. A comma-separated + # value is not JSON, so startup died with an opaque SettingsError and the + # _split_csv validator below never ran. NoDecode hands the raw string to + # the validator instead. + cors_origins: Annotated[list[str], NoDecode] = Field( default_factory=lambda: [ "http://localhost:5173", "http://127.0.0.1:5173", @@ -102,19 +110,31 @@ class Settings(BaseSettings): # Optional allow-list. When non-empty, only these Hugging Face model ids may # be loaded — important for a public deployment so visitors cannot trigger # arbitrary multi-gigabyte downloads. Empty list = allow any model. - allowed_models: list[str] = Field(default_factory=list) + allowed_models: Annotated[list[str], NoDecode] = Field(default_factory=list) @field_validator("cors_origins", "allowed_models", mode="before") @classmethod def _split_csv(cls, value: object) -> object: - """Allow these list fields to be supplied as a comma-separated string. + """Parse these list fields from a single environment string. - ``pydantic-settings`` reads env vars as strings; this lets - ``CORS_ORIGINS="https://a.com,https://b.com"`` work as expected. + Comma-separated is the documented form, so ``CORS_ORIGINS="https://a.com, + https://b.com"`` works as expected. A JSON array is also accepted: + that is what pydantic-settings itself parsed before these fields were + marked ``NoDecode``, and silently rejecting it would break any + deployment already written that way. """ - if isinstance(value, str): - return [item.strip() for item in value.split(",") if item.strip()] - return value + if not isinstance(value, str): + return value + + text = value.strip() + if text.startswith("["): + try: + return json.loads(text) + except json.JSONDecodeError: + # Fall through: a malformed array is reported far more clearly + # by the list-of-str validation than by a JSON error here. + pass + return [item.strip() for item in text.split(",") if item.strip()] @field_validator("static_dir", mode="before") @classmethod diff --git a/backend/requirements.txt b/backend/requirements.txt index 6f78269..321d76c 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -2,7 +2,7 @@ fastapi>=0.116,<1.0 uvicorn[standard]>=0.35,<1.0 pydantic>=2.7,<3.0 -pydantic-settings>=2.2,<3.0 +pydantic-settings>=2.3,<3.0 # 2.3 introduced NoDecode, required by config.py # --- Numerical / ML --- numpy>=1.26,<3.0 diff --git a/backend/tests/test_config.py b/backend/tests/test_config.py new file mode 100644 index 0000000..4e35ded --- /dev/null +++ b/backend/tests/test_config.py @@ -0,0 +1,97 @@ +"""Settings parsing, exercised through the environment. + +These tests read the environment the way a deployed container does. That +matters: the list fields are the ones most likely to be set in production and +least likely to be set in development, so a parsing bug in them survives every +local run and only appears on the server. +""" + +from __future__ import annotations + +import pytest + +from app.config import Settings + + +@pytest.fixture(autouse=True) +def _clear_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Isolate each test from the developer's real environment and .env file.""" + for name in ( + "CORS_ORIGINS", + "ALLOWED_MODELS", + "MAX_EMBEDDING_PARAMS", + "MAX_DOWNLOAD_BYTES", + "ENVIRONMENT", + ): + monkeypatch.delenv(name, raising=False) + + +def test_comma_separated_cors_origins(monkeypatch: pytest.MonkeyPatch) -> None: + """A CSV env value must parse, not be treated as JSON. + + pydantic-settings decodes "complex" fields (list, dict) inside the settings + source, which runs before field validators. Without NoDecode on the field, + a comma-separated value raised SettingsError at import time and the + container never started. + """ + monkeypatch.setenv("CORS_ORIGINS", "https://a.example.com,https://b.example.com") + assert Settings(_env_file=None).cors_origins == [ + "https://a.example.com", + "https://b.example.com", + ] + + +def test_single_cors_origin(monkeypatch: pytest.MonkeyPatch) -> None: + """The one-value case is the production case, and is not valid JSON.""" + monkeypatch.setenv("CORS_ORIGINS", "https://embeddings.example.com") + assert Settings(_env_file=None).cors_origins == ["https://embeddings.example.com"] + + +def test_comma_separated_allowed_models(monkeypatch: pytest.MonkeyPatch) -> None: + """ALLOWED_MODELS is the other list field and shares the same validator.""" + monkeypatch.setenv("ALLOWED_MODELS", "distilgpt2,gpt2,bert-base-uncased") + assert Settings(_env_file=None).allowed_models == [ + "distilgpt2", + "gpt2", + "bert-base-uncased", + ] + + +def test_list_fields_tolerate_whitespace_and_blanks( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Values wrap across lines in compose files; padding must not survive.""" + monkeypatch.setenv("ALLOWED_MODELS", " distilgpt2 , gpt2 ,, ") + assert Settings(_env_file=None).allowed_models == ["distilgpt2", "gpt2"] + + +def test_list_fields_default_when_unset() -> None: + """Unset values keep the development defaults.""" + settings = Settings(_env_file=None) + assert settings.allowed_models == [] + assert "http://localhost:5173" in settings.cors_origins + + +def test_json_list_still_accepted(monkeypatch: pytest.MonkeyPatch) -> None: + """A JSON array keeps working, so existing deployments do not break.""" + monkeypatch.setenv("ALLOWED_MODELS", '["distilgpt2", "gpt2"]') + assert Settings(_env_file=None).allowed_models == ["distilgpt2", "gpt2"] + + +def test_production_compose_values_parse(monkeypatch: pytest.MonkeyPatch) -> None: + """The exact environment docker-compose.yml sets must construct cleanly.""" + monkeypatch.setenv("ENVIRONMENT", "production") + monkeypatch.setenv("CORS_ORIGINS", "https://embeddings.mannymcgrail.com") + monkeypatch.setenv( + "ALLOWED_MODELS", + "distilgpt2,gpt2,distilbert-base-uncased,bert-base-uncased,roberta-base", + ) + monkeypatch.setenv("MAX_EMBEDDING_PARAMS", "60000000") + monkeypatch.setenv("MAX_DOWNLOAD_BYTES", "2000000000") + + settings = Settings(_env_file=None) + + assert settings.is_production + assert settings.cors_origins == ["https://embeddings.mannymcgrail.com"] + assert len(settings.allowed_models) == 5 + assert settings.max_embedding_params == 60_000_000