fix(settings): valida csv_delimiter/decimal_separator/auto_logout_minutes (#23)

PUT /api/settings/ non validava nulla: un csv_delimiter vuoto o di piu'
caratteri arrivava intatto nel DB e faceva fallire csv.writer con TypeError
dentro close_run (500, run mai chiuso). Aggiunta validazione per chiave nota
(csv_delimiter 1 carattere fra , ; TAB |, csv_decimal_separator ',' o '.',
auto_logout_minutes vuoto o intero 1-480, company_name/company_logo_path
non vuoti) -> 422 sui valori fuori dominio. Chiavi non note passano senza
validazione (store generico).

Difesa in profondita' in production_export_service._csv_format: un
csv_delimiter gia' sporco nel DB (scritto prima di questa validazione) non
fa piu' fallire close_run, usa ';' e logga un WARNING.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

src/backend/api/routers/settings.py

@@ -13,6 +13,42 @@ from src.backend.models.orm.user import User
 
 router = APIRouter(prefix="/api/settings", tags=["settings"])
 
+# Domains for the settings keys the backend actually reads. Values outside these
+# would only fail later (csv.writer TypeError on close_run, etc) — reject up front.
+_VALID_CSV_DELIMITERS = {",", ";", "\t", "|"}
+_VALID_CSV_DECIMAL_SEPARATORS = {",", "."}
+
+
+def _validate_setting_value(key: str, value: str) -> None:
+    """Raise 422 for a value that would break the code reading this key.
+
+    Unrecognised keys pass through unvalidated: the settings store is generic.
+    """
+    if key == "csv_delimiter":
+        if value not in _VALID_CSV_DELIMITERS:
+            raise HTTPException(
+                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+                detail="csv_delimiter deve essere un carattere singolo fra ',' ';' TAB '|'",
+            )
+    elif key == "csv_decimal_separator":
+        if value not in _VALID_CSV_DECIMAL_SEPARATORS:
+            raise HTTPException(
+                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+                detail="csv_decimal_separator deve essere ',' o '.'",
+            )
+    elif key == "auto_logout_minutes":
+        if value != "" and (not value.isdigit() or not (1 <= int(value) <= 480)):
+            raise HTTPException(
+                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+                detail="auto_logout_minutes deve essere vuoto (disabilitato) o un intero fra 1 e 480",
+            )
+    elif key in ("company_name", "company_logo_path"):
+        if not value.strip() or len(value) > 200:
+            raise HTTPException(
+                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+                detail=f"{key} deve essere una stringa non vuota di massimo 200 caratteri",
+            )
+
 
 @router.get("/")
 async def get_settings(
@@ -49,6 +85,8 @@ async def update_settings(
     updated_keys = []
 
     for key, value in settings_data.items():
+        _validate_setting_value(key, value)
+
         # Check if setting exists
         result = await db.execute(
             select(SystemSetting).where(SystemSetting.setting_key == key)

src/backend/services/production_export_service.py

@@ -11,6 +11,7 @@ files that open correctly in their Excel.
 """
 import csv
 import io
+import logging
 from datetime import datetime
 from pathlib import Path
 
@@ -25,10 +26,18 @@ from src.backend.models.orm.setting import SystemSetting
 from src.backend.models.orm.station import Station
 from src.backend.models.orm.task import RecipeSubtask
 
+logger = logging.getLogger(__name__)
+
 # Written under the upload directory: it is the volume that is already mounted and
 # backed up, so the files survive a container being replaced.
 EXPORT_SUBDIR = "statistics"
 
+# `csv.writer` requires a 1-character delimiter. PUT /api/settings/ validates this
+# going forward (gitea#23), but a value written before that guard existed must not
+# be able to make close_run fail: fall back and log instead of raising.
+_VALID_CSV_DELIMITERS = {",", ";", "\t", "|"}
+_FALLBACK_CSV_DELIMITER = ";"
+
 HEADER = [
     "production_run_id",
     "station_code",
@@ -75,8 +84,17 @@ async def _csv_format(db: AsyncSession) -> tuple[str, str]:
         )
     )
     decimal = decimal_row.scalar_one_or_none()
+
+    delimiter_value = delimiter.setting_value if delimiter else ","
+    if delimiter_value not in _VALID_CSV_DELIMITERS:
+        logger.warning(
+            "csv_delimiter non valido nel DB (%r): uso %r",
+            delimiter_value, _FALLBACK_CSV_DELIMITER,
+        )
+        delimiter_value = _FALLBACK_CSV_DELIMITER
+
     return (
-        delimiter.setting_value if delimiter else ",",
+        delimiter_value,
         decimal.setting_value if decimal else ".",
     )
 

src/backend/tests/test_production_export_service.py

@@ -0,0 +1,57 @@
+"""Unit tests for production_export_service.py.
+
+- CSV format defenses (gitea#23): a dirty csv_delimiter read from the DB must
+  never make close_run fail with a csv.writer TypeError.
+"""
+from sqlalchemy.ext.asyncio import AsyncSession
+
+from src.backend.models.orm.setting import SystemSetting
+from src.backend.services.production_export_service import _csv_format
+
+
+class TestCsvFormatDefense:
+    """`_csv_format` must never hand csv.writer a delimiter it will reject."""
+
+    async def test_invalid_delimiter_in_db_falls_back_to_semicolon(
+        self, db_session: AsyncSession
+    ):
+        db_session.add(
+            SystemSetting(
+                setting_key="csv_delimiter", setting_value=";;", setting_type="string"
+            )
+        )
+        await db_session.flush()
+
+        delimiter, _ = await _csv_format(db_session)
+        assert delimiter == ";"
+
+    async def test_empty_delimiter_in_db_falls_back(self, db_session: AsyncSession):
+        db_session.add(
+            SystemSetting(
+                setting_key="csv_delimiter", setting_value="", setting_type="string"
+            )
+        )
+        await db_session.flush()
+
+        delimiter, _ = await _csv_format(db_session)
+        assert delimiter == ";"
+
+    async def test_valid_delimiter_passes_through_unchanged(
+        self, db_session: AsyncSession
+    ):
+        db_session.add(
+            SystemSetting(
+                setting_key="csv_delimiter", setting_value="|", setting_type="string"
+            )
+        )
+        await db_session.flush()
+
+        delimiter, _ = await _csv_format(db_session)
+        assert delimiter == "|"
+
+    async def test_no_delimiter_setting_defaults_to_comma(
+        self, db_session: AsyncSession
+    ):
+        delimiter, decimal = await _csv_format(db_session)
+        assert delimiter == ","
+        assert decimal == "."

src/backend/tests/test_settings.py

@@ -105,3 +105,80 @@ class TestUpdateSettings:
             json={"some_key": "some_value"},
         )
         assert resp.status_code == 403
+
+
+class TestUpdateSettingsValidation:
+    """gitea#23: an invalid csv_delimiter must be rejected, not accepted and
+    left to blow up close_run later with a csv.writer TypeError."""
+
+    async def test_csv_delimiter_two_chars_rejected(
+        self, client: AsyncClient, admin_user: User
+    ):
+        resp = await client.put(
+            "/api/settings/",
+            headers=auth_headers(admin_user),
+            json={"csv_delimiter": ";;"},
+        )
+        assert resp.status_code == 422
+
+    async def test_csv_delimiter_empty_rejected(
+        self, client: AsyncClient, admin_user: User
+    ):
+        resp = await client.put(
+            "/api/settings/",
+            headers=auth_headers(admin_user),
+            json={"csv_delimiter": ""},
+        )
+        assert resp.status_code == 422
+
+    async def test_csv_delimiter_tab_accepted(
+        self, client: AsyncClient, admin_user: User
+    ):
+        resp = await client.put(
+            "/api/settings/",
+            headers=auth_headers(admin_user),
+            json={"csv_delimiter": "\t"},
+        )
+        assert resp.status_code == 200
+
+    async def test_csv_decimal_separator_invalid_rejected(
+        self, client: AsyncClient, admin_user: User
+    ):
+        resp = await client.put(
+            "/api/settings/",
+            headers=auth_headers(admin_user),
+            json={"csv_decimal_separator": "x"},
+        )
+        assert resp.status_code == 422
+
+    async def test_auto_logout_minutes_out_of_range_rejected(
+        self, client: AsyncClient, admin_user: User
+    ):
+        resp = await client.put(
+            "/api/settings/",
+            headers=auth_headers(admin_user),
+            json={"auto_logout_minutes": "0"},
+        )
+        assert resp.status_code == 422
+
+    async def test_auto_logout_minutes_empty_disables(
+        self, client: AsyncClient, admin_user: User
+    ):
+        """Empty string is what the settings page sends to disable auto-logout."""
+        resp = await client.put(
+            "/api/settings/",
+            headers=auth_headers(admin_user),
+            json={"auto_logout_minutes": ""},
+        )
+        assert resp.status_code == 200
+
+    async def test_unknown_key_not_validated(
+        self, client: AsyncClient, admin_user: User
+    ):
+        """Keys with no known validator pass through (generic settings store)."""
+        resp = await client.put(
+            "/api/settings/",
+            headers=auth_headers(admin_user),
+            json={"some_future_key": "anything"},
+        )
+        assert resp.status_code == 200