recipe: harden auth/metrics, add rate limiting and tests

2026-08-31 17:58:21 +03:00
parent 3d846d0be5
commit caeaa8c6fa
9 changed files with 452 additions and 71 deletions
@@ -641,3 +641,111 @@ nginx-маршруты без redirect и health `/receipt/health`. Старые
 IP `127.0.0.1`, MIME `image/png`, размер 531101 байт, длину prompt 88 и
 непустой usage JSON. Тестовый image-файл удалён после запроса; приложение
 читает изображение в память и не пишет его на диск.
+
+## 2026-08-31: Реализация backend-фиксов по плану v2
+
+По команде пользователя выполнены изменения backend-компонентов и тестов.
+
+### Изменения в recipe_service
+
+- `recipe_service/metrics.py`:
+	- из `record()` удалён вызов `initialize()`;
+	- добавлена `count_since(client_ip, started_at_from)` для rate limiting.
+- `recipe_service/app.py`:
+	- добавлен `ProxyFix(..., x_for=1, x_proto=1, x_host=1)`;
+	- сравнение токена переведено на `hmac.compare_digest`;
+	- введён единый финализатор `finalize(...)` вместо дублирования `record(...)`;
+	- `duration_ms` считается во всех ветках через `time.monotonic()`;
+	- добавлен rate limit `20` запросов/минута на IP (`429 too many requests`);
+	- зафиксирован контракт `502`:
+		`{"error":"upstream recognition failed","code":"upstream_error"}`;
+	- детали апстрима пишутся только в лог сервера с `request_id`.
+- добавлен `recipe_service/test_app.py` (покрытие: `health`, `401`, `400`,
+	`415`, `413`, `200`, `502`-контракт, `429`).
+
+### Изменения в gemini_proxy
+
+- `gemini_proxy/app.py`: удалён `api_key_override`; ключ только из
+	`GEMINI_API_KEY`.
+- `gemini_proxy/test_app.py`: добавлен тест, что `api_key_override` в форме
+	не даёт доступ без `GEMINI_API_KEY`.
+
+### Изменения зависимостей
+
+- выровнен root `requirements.txt` по version bounds с
+	`recipe_service/requirements.txt`:
+	- `Flask>=3.0,<4`
+	- `gunicorn>=21.2,<24`
+	- `requests>=2.31,<3`
+
+### Проверки
+
+- `py_compile` изменённых Python-файлов: успешно.
+- `recipe_service`: `pytest -q` -> `8 passed`.
+- `gemini_proxy`: `pytest -q` -> `4 passed`.
+
+### Отдельно зафиксировано
+
+Первый запуск тестов `recipe_service` дал `PermissionError` на `/var/lib/recipe`
+при import-time `initialize()`. Исправлено в тесте ранней установкой
+`RECIPE_METRICS_DB` в временный путь до импорта `app`.
+
+## 2026-08-31: Безопасная оптимизация без смены поведения
+
+По дополнительной команде пользователя выполнен пакет low-risk улучшений,
+направленный на производительность и устойчивость, без изменения основного
+контракта API.
+
+### Изменения
+
+- `recipe_service/metrics.py`:
+	- добавлен индекс
+		`idx_requests_client_ip_started_at ON requests(client_ip, started_at)`
+		для ускорения выборки rate limiting.
+- `recipe_service/app.py`:
+	- ответ `429` унифицирован и дополнен стабильным полем
+		`code="rate_limited"` при сохранении `error="too many requests"`.
+- `recipe_service/test_app.py`:
+	- обновлена проверка `429` с новым полем `code`;
+	- добавлен тест граничного случая лимитера (`19` запросов -> `200`);
+	- добавлен тест чтения последней записи в SQLite-метриках с проверкой
+		`status_code`, `duration_ms` и `error` после ветки `502`.
+
+### Проверки
+
+- `py_compile` изменённых Python-файлов: успешно.
+- `recipe_service`: `pytest -q` -> `10 passed`.
+- `gemini_proxy`: `pytest -q` -> `4 passed`.
+
+### Вывод
+
+Оптимизации применены без регрессий. Поведение успешного запроса, а также
+статусы `400/401/413/415/502` сохранены; `429` дополнен машинным кодом
+ошибки для стабильной клиентской обработки.
+
+## 2026-08-31: Nginx-level rate limiting (основной лимитер)
+
+По команде пользователя добавлен основной лимит запросов на уровне nginx,
+при сохранении app-level fallback в `recipe_service/app.py`.
+
+### Изменения конфигурации
+
+- Добавлен новый файл `recipe_service/nginx-rate-limit-http.conf`:
+	- `limit_req_zone $binary_remote_addr zone=recipe_api_per_ip:10m rate=20r/m;`
+	- `limit_req_status 429;`
+	- файл предназначен для single-include внутри `http { ... }`.
+- Обновлён `recipe_service/nginx-recipe.conf`:
+	- для `location = /recipe`, `location /recipe/`, `location = /receipt`,
+		`location /receipt/` добавлен
+		`limit_req zone=recipe_api_per_ip burst=5 nodelay;`.
+
+### Результат
+
+- Лимит теперь применяется единообразно для всех воркеров gunicorn на входе
+	nginx, а не только внутри отдельного процесса приложения.
+- Python fallback-лимитер сохранён как защитный второй контур.
+
+### Проверки
+
+- `py_compile` изменённых Python-файлов: успешно.
+- `recipe_service`: `pytest -q` -> `10 passed`.
@@ -25,7 +25,6 @@ async def recognize(
    image: Annotated[UploadFile, File(...)],
    prompt: Annotated[str, Form(...)],
    generation_config: Annotated[str, Form()] = "{}",
-    api_key_override: Annotated[str | None, Form()] = None,
 ) -> dict:
    if image.content_type not in ALLOWED_TYPES:
        raise HTTPException(status_code=415, detail="Unsupported image type")
@@ -34,7 +33,7 @@ async def recognize(
    if len(image_data) > MAX_IMAGE_BYTES:
        raise HTTPException(status_code=413, detail="Image is too large")

-    api_key = api_key_override or os.getenv("GEMINI_API_KEY")
+    api_key = os.getenv("GEMINI_API_KEY")
    if not api_key:
        raise HTTPException(status_code=503, detail="Gemini is not configured")

@@ -28,4 +28,19 @@ def test_missing_key_returns_service_unavailable(monkeypatch) -> None:
        files={"image": ("input.png", b"not-an-image", "image/png")},
        data={"prompt": "test", "generation_config": "{}"},
    )
+    assert response.status_code == 503
+
+
+def test_rejects_when_only_override_provided(monkeypatch) -> None:
+    monkeypatch.delenv("GEMINI_API_KEY", raising=False)
+    client = TestClient(app)
+    response = client.post(
+        "/gemini",
+        files={"image": ("input.png", b"not-an-image", "image/png")},
+        data={
+            "prompt": "test",
+            "generation_config": "{}",
+            "api_key_override": "manual-key",
+        },
+    )
    assert response.status_code == 503
@@ -1,19 +1,24 @@
 import json
 import os
+import hmac
+import time

 import requests
 from flask import Flask, jsonify, request
+from werkzeug.middleware.proxy_fix import ProxyFix

 try:
-    from recipe_service.metrics import initialize, record, request_context, usage_json
+    from recipe_service.metrics import count_since, initialize, record, request_context, usage_json
 except ModuleNotFoundError:
-    from metrics import initialize, record, request_context, usage_json
+    from metrics import count_since, initialize, record, request_context, usage_json


 app = Flask(__name__)
+app.wsgi_app = ProxyFix(app.wsgi_app, x_for=1, x_proto=1, x_host=1)
 MAX_IMAGE_BYTES = 10 * 1024 * 1024
 ALLOWED_TYPES = {"image/jpeg", "image/png", "image/webp"}
 PROXY_URL = "http://127.0.0.1:8768/gemini"
+RATE_LIMIT_REQUESTS_PER_MINUTE = 20

 initialize()

@@ -31,7 +36,18 @@ def settings() -> dict:
 def authorized() -> bool:
    expected = os.environ.get("RECIPE_API_TOKEN")
    authorization = request.headers.get("Authorization", "")
-    return bool(expected and authorization == f"Bearer {expected}")
+    return bool(expected and hmac.compare_digest(authorization, f"Bearer {expected}"))
+
+
+def minute_start_utc(epoch_seconds: float) -> str:
+    return time.strftime("%Y-%m-%dT%H:%M:00Z", time.gmtime(epoch_seconds))
+
+
+def is_rate_limited(client_ip: str, now_epoch: float) -> bool:
+    if not client_ip:
+        return False
+    window_start = minute_start_utc(now_epoch)
+    return count_since(client_ip=client_ip, started_at_from=window_start) >= RATE_LIMIT_REQUESTS_PER_MINUTE


@app.get("/health")
@@ -49,54 +65,58 @@ def recipe():
    image_mime = None
    image_bytes = None
    prompt_chars = None
-    status_code = 500
-    response_bytes = None
    usage = {}
-    error = None
-    if not authorized():
-        status_code = 401
-        error = "unauthorized"
-        response = jsonify(error=error)
-        record(request_id=request_id, started_at=started_at, client_ip=request.remote_addr,
-               user_agent=request.user_agent.string, method=request.method, path=request.path,
-               image_mime=image_mime, image_bytes=image_bytes, prompt_chars=prompt_chars,
-               status_code=status_code, duration_ms=int((__import__('time').monotonic() - started_monotonic) * 1000),
-               response_bytes=len(response.get_data()), usage_json=usage_json(usage), error=error)
+    client_ip = request.remote_addr
+
+    def finalize(response, status_code: int, error: str | None):
+        duration_ms = int((time.monotonic() - started_monotonic) * 1000)
+        response_bytes = len(response.get_data())
+        record(
+            request_id=request_id,
+            started_at=started_at,
+            client_ip=client_ip,
+            user_agent=request.user_agent.string,
+            method=request.method,
+            path=request.path,
+            image_mime=image_mime,
+            image_bytes=image_bytes,
+            prompt_chars=prompt_chars,
+            status_code=status_code,
+            duration_ms=duration_ms,
+            response_bytes=response_bytes,
+            usage_json=usage_json(usage),
+            error=error,
+        )
        return response, status_code
+
+    if not authorized():
+        return finalize(jsonify(error="unauthorized"), 401, "unauthorized")
+
+    if is_rate_limited(client_ip=client_ip or "", now_epoch=time.time()):
+        return finalize(
+            jsonify(error="too many requests", code="rate_limited"),
+            429,
+            "rate_limited",
+        )
+
    image = request.files.get("image")
    prompt = request.form.get("prompt")
    if image is None or not prompt:
-        status_code = 400
-        error = "image and prompt are required"
-        response = jsonify(error=error)
-        record(request_id=request_id, started_at=started_at, client_ip=request.remote_addr,
-               user_agent=request.user_agent.string, method=request.method, path=request.path,
-               status_code=status_code, duration_ms=0, response_bytes=len(response.get_data()),
-               usage_json=usage_json(usage), error=error)
-        return response, status_code
+        return finalize(jsonify(error="image and prompt are required"), 400, "image and prompt are required")
+
+    prompt_chars = len(prompt)
+
    if image.mimetype not in ALLOWED_TYPES:
-        status_code = 415
-        error = "unsupported image type"
-        response = jsonify(error=error)
-        record(request_id=request_id, started_at=started_at, client_ip=request.remote_addr,
-               user_agent=request.user_agent.string, method=request.method, path=request.path,
-               image_mime=image.mimetype, prompt_chars=len(prompt), status_code=status_code,
-               duration_ms=0, response_bytes=len(response.get_data()), usage_json=usage_json(usage), error=error)
-        return response, status_code
+        image_mime = image.mimetype
+        return finalize(jsonify(error="unsupported image type"), 415, "unsupported image type")
+
    image_data = image.read(MAX_IMAGE_BYTES + 1)
-    if len(image_data) > MAX_IMAGE_BYTES:
-        status_code = 413
-        error = "image is too large"
-        response = jsonify(error=error)
-        record(request_id=request_id, started_at=started_at, client_ip=request.remote_addr,
-               user_agent=request.user_agent.string, method=request.method, path=request.path,
-               image_mime=image.mimetype, image_bytes=len(image_data), prompt_chars=len(prompt),
-               status_code=status_code, duration_ms=0, response_bytes=len(response.get_data()),
-               usage_json=usage_json(usage), error=error)
-        return response, status_code
    image_mime = image.mimetype
    image_bytes = len(image_data)
-    prompt_chars = len(prompt)
+
+    if len(image_data) > MAX_IMAGE_BYTES:
+        return finalize(jsonify(error="image is too large"), 413, "image is too large")
+
    try:
        config = settings()
        response = requests.post(
@@ -107,34 +127,21 @@ def recipe():
        )
    except (requests.RequestException, RuntimeError) as exc:
        error = str(exc) if isinstance(exc, RuntimeError) else "Gemini unavailable"
-        status_code = 503
-        response = jsonify(error=error)
-        record(request_id=request_id, started_at=started_at, client_ip=request.remote_addr,
-               user_agent=request.user_agent.string, method=request.method, path=request.path,
-               image_mime=image_mime, image_bytes=image_bytes, prompt_chars=prompt_chars,
-               status_code=status_code, duration_ms=0, response_bytes=len(response.get_data()),
-               usage_json=usage_json(usage), error=error)
-        return response, status_code
+        return finalize(jsonify(error=error), 503, error)
+
    if response.status_code != 200:
        try:
            detail = response.json().get("error", {}).get("message", "Gemini request failed")
        except ValueError:
            detail = "Gemini request failed"
-        error = detail
-        status_code = 502
-        response = jsonify(error=error)
-        record(request_id=request_id, started_at=started_at, client_ip=request.remote_addr,
-               user_agent=request.user_agent.string, method=request.method, path=request.path,
-               image_mime=image_mime, image_bytes=image_bytes, prompt_chars=prompt_chars,
-               status_code=status_code, duration_ms=0, response_bytes=len(response.get_data()),
-               usage_json=usage_json(usage), error=error)
-        return response, status_code
+        app.logger.warning("upstream_failure request_id=%s detail=%s", request_id, detail)
+        return finalize(
+            jsonify(error="upstream recognition failed", code="upstream_error"),
+            502,
+            "upstream_error",
+        )
+
    data = response.json()
    usage = data.get("usage", {})
    result = jsonify(text=data.get("text"), usage=usage)
-    record(request_id=request_id, started_at=started_at, client_ip=request.remote_addr,
-           user_agent=request.user_agent.string, method=request.method, path=request.path,
-           image_mime=image_mime, image_bytes=image_bytes, prompt_chars=prompt_chars,
-           status_code=200, duration_ms=0, response_bytes=len(result.get_data()),
-           usage_json=usage_json(usage), error=None)
-    return result
+    return finalize(result, 200, None)
@@ -37,10 +37,12 @@ def initialize() -> None:
        """)
        connection.execute("CREATE INDEX IF NOT EXISTS idx_requests_started_at ON requests(started_at)")
        connection.execute("CREATE INDEX IF NOT EXISTS idx_requests_status_code ON requests(status_code)")
+        connection.execute(
+            "CREATE INDEX IF NOT EXISTS idx_requests_client_ip_started_at ON requests(client_ip, started_at)"
+        )


 def record(**values) -> None:
-    initialize()
    columns = [
        "request_id", "started_at", "client_ip", "user_agent", "method",
        "path", "image_mime", "image_bytes", "prompt_chars", "status_code",
@@ -54,6 +56,21 @@ def record(**values) -> None:
        )


+def count_since(client_ip: str, started_at_from: str) -> int:
+    with sqlite3.connect(db_path()) as connection:
+        row = connection.execute(
+            """
+            SELECT COUNT(*)
+            FROM requests
+            WHERE client_ip = ?
+              AND started_at >= ?
+              AND path IN ('/receipt', '/receipt/', '/recipe', '/recipe/')
+            """,
+            (client_ip, started_at_from),
+        ).fetchone()
+    return int(row[0] if row else 0)
+
+
 def request_context() -> tuple[str, str, float]:
    return str(uuid.uuid4()), time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()), time.monotonic()

@@ -0,0 +1,8 @@
+# Include this file once inside nginx `http { ... }` block.
+# Example: include /etc/nginx/conf.d/recipe-rate-limit-http.conf;
+
+# Per-client limit for recipe/receipt API requests.
+limit_req_zone $binary_remote_addr zone=recipe_api_per_ip:10m rate=20r/m;
+
+# Return 429 for throttled requests.
+limit_req_status 429;
@@ -1,4 +1,5 @@
 location = /recipe {
+    limit_req zone=recipe_api_per_ip burst=5 nodelay;
    proxy_pass http://127.0.0.1:8770/recipe;
    proxy_http_version 1.1;
    proxy_set_header Host $host;
@@ -12,6 +13,7 @@ location = /recipe {
 }

 location /recipe/ {
+    limit_req zone=recipe_api_per_ip burst=5 nodelay;
    proxy_pass http://127.0.0.1:8770/recipe/;
    proxy_http_version 1.1;
    proxy_set_header Host $host;
@@ -25,6 +27,7 @@ location /recipe/ {
 }

 location = /receipt {
+    limit_req zone=recipe_api_per_ip burst=5 nodelay;
    proxy_pass http://127.0.0.1:8770/receipt;
    proxy_http_version 1.1;
    proxy_set_header Host $host;
@@ -38,6 +41,7 @@ location = /receipt {
 }

 location /receipt/ {
+    limit_req zone=recipe_api_per_ip burst=5 nodelay;
    proxy_pass http://127.0.0.1:8770/receipt/;
    proxy_http_version 1.1;
    proxy_set_header Host $host;
@@ -0,0 +1,223 @@
+import io
+import json
+import os
+import sqlite3
+import tempfile
+
+import requests
+
+os.environ.setdefault(
+    "RECIPE_METRICS_DB",
+    os.path.join(tempfile.gettempdir(), "recipe-service-tests-metrics.sqlite3"),
+)
+
+from app import app
+
+
+class MockResponse:
+    def __init__(self, status_code: int, payload: dict | None = None):
+        self.status_code = status_code
+        self._payload = payload or {}
+
+    def json(self) -> dict:
+        return self._payload
+
+
+def auth_header() -> dict[str, str]:
+    return {"Authorization": "Bearer test-token"}
+
+
+def make_image(content: bytes = b"img") -> tuple[io.BytesIO, str, str]:
+    return io.BytesIO(content), "sample.png", "image/png"
+
+
+def test_health() -> None:
+    client = app.test_client()
+    response = client.get("/health")
+    assert response.status_code == 200
+    assert response.get_json() == {"status": "ok"}
+
+
+def test_requires_authorization(monkeypatch) -> None:
+    monkeypatch.setenv("RECIPE_API_TOKEN", "test-token")
+    client = app.test_client()
+    response = client.post("/receipt")
+    assert response.status_code == 401
+    assert response.get_json() == {"error": "unauthorized"}
+
+
+def test_missing_image_or_prompt(monkeypatch) -> None:
+    monkeypatch.setenv("RECIPE_API_TOKEN", "test-token")
+    client = app.test_client()
+    response = client.post("/receipt", headers=auth_header())
+    assert response.status_code == 400
+    assert response.get_json() == {"error": "image and prompt are required"}
+
+
+def test_unsupported_type(monkeypatch) -> None:
+    monkeypatch.setenv("RECIPE_API_TOKEN", "test-token")
+    client = app.test_client()
+    response = client.post(
+        "/receipt",
+        headers=auth_header(),
+        data={
+            "prompt": "p",
+            "image": (io.BytesIO(b"x"), "bad.txt", "text/plain"),
+        },
+        content_type="multipart/form-data",
+    )
+    assert response.status_code == 415
+    assert response.get_json() == {"error": "unsupported image type"}
+
+
+def test_image_too_large(monkeypatch) -> None:
+    monkeypatch.setenv("RECIPE_API_TOKEN", "test-token")
+    client = app.test_client()
+    payload = b"a" * (10 * 1024 * 1024 + 1)
+    response = client.post(
+        "/receipt",
+        headers=auth_header(),
+        data={
+            "prompt": "p",
+            "image": (io.BytesIO(payload), "big.png", "image/png"),
+        },
+        content_type="multipart/form-data",
+    )
+    assert response.status_code == 413
+    assert response.get_json() == {"error": "image is too large"}
+
+
+def test_success(monkeypatch) -> None:
+    monkeypatch.setenv("RECIPE_API_TOKEN", "test-token")
+
+    def fake_post(*args, **kwargs):
+        return MockResponse(200, {"text": "ok", "usage": {"totalTokens": 10}})
+
+    monkeypatch.setattr(requests, "post", fake_post)
+    client = app.test_client()
+    response = client.post(
+        "/receipt",
+        headers=auth_header(),
+        data={
+            "prompt": "p",
+            "image": make_image(),
+        },
+        content_type="multipart/form-data",
+    )
+    assert response.status_code == 200
+    assert response.get_json() == {"text": "ok", "usage": {"totalTokens": 10}}
+
+
+def test_upstream_502_contract(monkeypatch) -> None:
+    monkeypatch.setenv("RECIPE_API_TOKEN", "test-token")
+
+    def fake_post(*args, **kwargs):
+        return MockResponse(500, {"error": {"message": "provider detail"}})
+
+    monkeypatch.setattr(requests, "post", fake_post)
+    client = app.test_client()
+    response = client.post(
+        "/receipt",
+        headers=auth_header(),
+        data={
+            "prompt": "p",
+            "image": make_image(),
+        },
+        content_type="multipart/form-data",
+    )
+    assert response.status_code == 502
+    assert response.get_json() == {
+        "error": "upstream recognition failed",
+        "code": "upstream_error",
+    }
+
+
+def test_rate_limit_returns_429(monkeypatch) -> None:
+    monkeypatch.setenv("RECIPE_API_TOKEN", "test-token")
+
+    def fake_count_since(client_ip: str, started_at_from: str) -> int:
+        return 20
+
+    monkeypatch.setattr("app.count_since", fake_count_since)
+    client = app.test_client()
+    response = client.post(
+        "/receipt",
+        headers=auth_header(),
+        data={
+            "prompt": "p",
+            "image": make_image(),
+        },
+        content_type="multipart/form-data",
+        environ_base={"REMOTE_ADDR": "198.51.100.10"},
+    )
+    assert response.status_code == 429
+    assert response.get_json() == {"error": "too many requests", "code": "rate_limited"}
+
+
+def test_rate_limit_allows_below_threshold(monkeypatch) -> None:
+    monkeypatch.setenv("RECIPE_API_TOKEN", "test-token")
+
+    def fake_count_since(client_ip: str, started_at_from: str) -> int:
+        return 19
+
+    def fake_post(*args, **kwargs):
+        return MockResponse(200, {"text": "ok", "usage": {}})
+
+    monkeypatch.setattr("app.count_since", fake_count_since)
+    monkeypatch.setattr(requests, "post", fake_post)
+
+    client = app.test_client()
+    response = client.post(
+        "/receipt",
+        headers=auth_header(),
+        data={
+            "prompt": "p",
+            "image": make_image(),
+        },
+        content_type="multipart/form-data",
+        environ_base={"REMOTE_ADDR": "198.51.100.11"},
+    )
+    assert response.status_code == 200
+
+
+def test_metrics_record_duration_and_status(monkeypatch) -> None:
+    db_file = os.path.join(tempfile.gettempdir(), "recipe-service-tests-metrics-duration.sqlite3")
+    if os.path.exists(db_file):
+        os.remove(db_file)
+
+    monkeypatch.setenv("RECIPE_METRICS_DB", db_file)
+    monkeypatch.setenv("RECIPE_API_TOKEN", "test-token")
+
+    from metrics import initialize
+
+    initialize()
+
+    def fake_post(*args, **kwargs):
+        return MockResponse(500, {"error": {"message": "provider detail"}})
+
+    monkeypatch.setattr(requests, "post", fake_post)
+    client = app.test_client()
+    response = client.post(
+        "/receipt",
+        headers=auth_header(),
+        data={
+            "prompt": "p",
+            "image": make_image(),
+        },
+        content_type="multipart/form-data",
+    )
+
+    assert response.status_code == 502
+    connection = sqlite3.connect(db_file)
+    try:
+        row = connection.execute(
+            "SELECT status_code, duration_ms, error FROM requests ORDER BY rowid DESC LIMIT 1"
+        ).fetchone()
+    finally:
+        connection.close()
+
+    assert row is not None
+    status_code, duration_ms, error = row
+    assert status_code == 502
+    assert duration_ms >= 0
+    assert error == "upstream_error"
@@ -1,3 +1,3 @@
-Flask>=3.0
-gunicorn>=21.2
-requests>=2.31
+Flask>=3.0,<4
+gunicorn>=21.2,<24
+requests>=2.31,<3