diff --git a/HISTORY/2026-08-28-obdai-receipt.md b/HISTORY/2026-08-28-obdai-receipt.md index 6be4fbf..726ef17 100644 --- a/HISTORY/2026-08-28-obdai-receipt.md +++ b/HISTORY/2026-08-28-obdai-receipt.md @@ -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`. diff --git a/gemini_proxy/app.py b/gemini_proxy/app.py index bed5ff5..47fd74b 100644 --- a/gemini_proxy/app.py +++ b/gemini_proxy/app.py @@ -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") diff --git a/gemini_proxy/test_app.py b/gemini_proxy/test_app.py index 76bd003..65c1504 100644 --- a/gemini_proxy/test_app.py +++ b/gemini_proxy/test_app.py @@ -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 \ No newline at end of file diff --git a/recipe_service/app.py b/recipe_service/app.py index d8f9bb3..6071019 100644 --- a/recipe_service/app.py +++ b/recipe_service/app.py @@ -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 \ No newline at end of file + return finalize(result, 200, None) \ No newline at end of file diff --git a/recipe_service/metrics.py b/recipe_service/metrics.py index 23eef41..a72a20d 100644 --- a/recipe_service/metrics.py +++ b/recipe_service/metrics.py @@ -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() diff --git a/recipe_service/nginx-rate-limit-http.conf b/recipe_service/nginx-rate-limit-http.conf new file mode 100644 index 0000000..b315084 --- /dev/null +++ b/recipe_service/nginx-rate-limit-http.conf @@ -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; diff --git a/recipe_service/nginx-recipe.conf b/recipe_service/nginx-recipe.conf index 02fa70b..d29fad6 100644 --- a/recipe_service/nginx-recipe.conf +++ b/recipe_service/nginx-recipe.conf @@ -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; diff --git a/recipe_service/test_app.py b/recipe_service/test_app.py new file mode 100644 index 0000000..90a0196 --- /dev/null +++ b/recipe_service/test_app.py @@ -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" diff --git a/requirements.txt b/requirements.txt index 0a5d8dd..ee10536 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ -Flask>=3.0 -gunicorn>=21.2 -requests>=2.31 \ No newline at end of file +Flask>=3.0,<4 +gunicorn>=21.2,<24 +requests>=2.31,<3 \ No newline at end of file