Безопасность: проверка секретов, блокировка перебора входа, безопасные редиректы

Безопасность, пункты 1–4 ревью (docs/changes/021):
- приложение не стартует со слабыми секретами: API_TOKEN и SESSION_SECRET
  пустые, change-me или короче 32 символов, ADMIN_PASSWORD короче 12,
  невалидный SECRET_KEY; значения секретов в ошибку не попадают;
- вход в UI: 5 неверных попыток за 10 минут блокируют IP на 10 минут
  (429, пароль не проверяется); неверный пароль — 401; событие
  auth.locked пишется один раз, журнал не засыпается перебором;
- SESSION_COOKIE_SECURE — флаг Secure у cookie сессии (по умолчанию выкл.);
- _safe_next: редирект только на локальный путь (/ui/move, удаление бэкапов);
- .env.bak-* в .gitignore.

Тесты: 28 из 28. Стенд: change-me → 401, новый токен → 200, отказ старта
со слабым токеном, блокировка входа и редиректы проверены, боевые данные
не изменены. Ручная проверка UI пользователем на момент коммита не
подтверждена.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
ayurishchevandClaude Opus 5.5 committed 2026-09-28 12:41:58 +03:00
1 parent 6b5c521461
commit c4079724c5
11 files changed
+332 -58

No files matched your search

+12 -6
View File
@@ -1,16 +1,22 @@
# База метаданных
DATABASE_URL=sqlite:///./data/ros_control.db
# Ключ Fernet для шифрования паролей устройств:
# Ключ Fernet для шифрования паролей устройств (обязателен, валидный ключ Fernet):
# python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())"
SECRET_KEY=
# Секрет подписи cookie-сессии UI
SESSION_SECRET=change-me
# Секрет подписи cookie-сессии UI (обязателен, ≥ 32 символов):
# python -c "import secrets; print(secrets.token_urlsafe(32))"
SESSION_SECRET=
# Secure-флаг cookie сессии (требует HTTPS); включить, когда приложение работает за TLS
SESSION_COOKIE_SECURE=false
# Администратор UI и токен API
# Администратор UI (пароль обязателен, ≥ 12 символов) и токен API (обязателен, ≥ 32 символов):
# python -c "import secrets; print(secrets.token_urlsafe(32))"
ADMIN_USER=admin
ADMIN_PASSWORD=change-me
API_TOKEN=change-me
ADMIN_PASSWORD=
API_TOKEN=
# Без заданных выше значений (или со значением change-me/короче требуемой длины) приложение не запускается —
# см. README «Безопасность».
# Yandex Object Storage (S3)
S3_ENDPOINT=https://storage.yandexcloud.net
+1
View File
@@ -1,6 +1,7 @@
venv/
data/
.env
.env.bak-*
__pycache__/
*.pyc
.pytest_cache/
+11 -7
View File
@@ -51,19 +51,21 @@ Admin Dashboard ⇄ Control Server ⇄ RouterOS REST (на каждом устр
## Безопасность
- Замените значения по умолчанию `API_TOKEN` и `SESSION_SECRET` на случайные (`openssl rand -hex 24`); порт 8000 не стоит открывать в недоверенную сеть.
- **Секреты обязательны:** `API_TOKEN`, `SESSION_SECRET` (≥ 32 символов) и `ADMIN_PASSWORD` (≥ 12 символов) не могут быть пустыми или равными `change-me`; `SECRET_KEY` должен быть валидным ключом Fernet. С небезопасными значениями приложение не стартует (`RuntimeError` при старте, без секретов в тексте ошибки и в логе) — сгенерировать: `python -c "import secrets; print(secrets.token_urlsafe(32))"`. Порт 8000 не стоит открывать в недоверенную сеть.
- **Вход в UI ограничен по IP клиента**: 5 неверных попыток за 10 минут блокируют IP на 10 минут (в блокировке пароль не проверяется); единственного администратора нельзя заблокировать чужими попытками, т.к. блокировка не привязана к имени пользователя. Ограничение достоверно только пока перед приложением нет reverse-proxy — иначе все запросы приходят с одного IP прокси, и понадобится доверенный `X-Forwarded-For`.
- **Cookie сессии UI**: `SESSION_COOKIE_SECURE=false` по умолчанию (UI по HTTP продолжает работать); включите `true`, когда приложение работает за TLS.
- Пароли устройств шифруются ключом `SECRET_KEY` (Fernet); потеря ключа = потеря доступа к сохранённым паролям.
- **Секреты в бэкапах:** `.rsc` создаётся с `show-sensitive`, а `.backup` — без шифрования, поэтому файлы содержат пароли и ключи открытым текстом. Ограничьте доступ к бакету и ссылкам скачивания.
## Ограничения
- **Один процесс на БД**: сервер держит файловую блокировку `<файл БД>.lock` рядом с БД (снимается при остановке); второй процесс на той же БД (`--workers 2+`, вторая копия контейнера на том же томе) не стартует — понятная ошибка вместо молчаливой порчи данных. В памяти процесса (не переживает перезапуск и не разделяется между процессами) — семафор фоновых задач, блокировка синхронизации бэкапов, счётчики неудачных попыток очистки журнала и кэш списка бакета.
- **Один процесс на БД**: сервер держит файловую блокировку `<файл БД>.lock` рядом с БД (снимается при остановке); второй процесс на той же БД (`--workers 2+`, вторая копия контейнера на том же томе) не стартует — понятная ошибка вместо молчаливой порчи данных. В памяти процесса (не переживает перезапуск и не разделяется между процессами) — семафор фоновых задач, блокировка синхронизации бэкапов, счётчики неудачных попыток входа в UI и очистки журнала, кэш списка бакета.
- **Кэш списка бакета**: страница «Бэкапы» и `GET /api/v1/backups` не перечитывают бакет на каждый просмотр — список живёт `BACKUPS_CACHE_TTL` секунд (по умолчанию 60; `0` — кэш выключен). Изменения бакета, сделанные не через это приложение, видны не позже TTL или сразу — кнопкой «Обновить список» (`refresh=1`). Собственные изменения (бэкап, удаление) сбрасывают кэш сами.
## Запуск
```bash
cp .env.example .env # заполнить SECRET_KEY, ADMIN_PASSWORD, API_TOKEN, S3_*
cp .env.example .env # заполнить SECRET_KEY, SESSION_SECRET, ADMIN_PASSWORD, API_TOKEN, S3_*
docker compose up -d --build # UI: http://localhost:8000, OpenAPI: /docs
```
@@ -76,9 +78,10 @@ docker compose up -d --build # UI: http://localhost:8000, OpenAPI: /docs
| Переменная | По умолчанию | Назначение |
|---|---|---|
| `SECRET_KEY` | — | ключ Fernet для паролей устройств (обязателен) |
| `SESSION_SECRET` | `change-me` | подпись cookie-сессии UI |
| `ADMIN_USER` / `ADMIN_PASSWORD` | `admin` / `change-me` | вход в UI |
| `API_TOKEN` | `change-me` | Bearer-токен API |
| `SESSION_SECRET` | — | подпись cookie-сессии UI (обязателен, ≥ 32 символов) |
| `SESSION_COOKIE_SECURE` | `false` | Secure-флаг cookie сессии; включить за TLS |
| `ADMIN_USER` / `ADMIN_PASSWORD` | `admin` / — | вход в UI (`ADMIN_PASSWORD` обязателен, ≥ 12 символов) |
| `API_TOKEN` | — | Bearer-токен API (обязателен, ≥ 32 символов) |
| `DATABASE_URL` | `sqlite:///./data/ros_control.db` | база метаданных (в compose — том) |
| `S3_ENDPOINT`, `S3_REGION`, `S3_BUCKET`, `S3_ACCESS_KEY`, `S3_SECRET_KEY`, `S3_PREFIX` | Yandex Object Storage, `backups` | бакет для резервных копий |
| `BACKUPS_CACHE_TTL` | `60` | кэш списка бакета, с; `0` — выключить (см. «Ограничения») |
@@ -95,7 +98,7 @@ docker compose up -d --build # UI: http://localhost:8000, OpenAPI: /docs
python3 -m venv venv && ./venv/bin/pip install -r requirements.txt
set -a; . ./.env; set +a
./venv/bin/uvicorn app.main:app --reload
./venv/bin/python -m pytest # 24 теста, фоновый опрос в тестах выключен
./venv/bin/python -m pytest # 28 тестов, фоновый опрос в тестах выключен
```
## API v1
@@ -162,3 +165,4 @@ curl -s -H "Authorization: Bearer $API_TOKEN" http://localhost:8000/api/v1/devic
- `018-correctness-consistency` — одиночное удаление бэкапа в UI через общий сервис `delete_many`, единая система миграций (колонки старой схемы — в `migrations.run` до миграции ID), групповая смена канала фоновыми задачами (`set_channel`).
- `019-performance-scaling` — кэш списка бакета (`BACKUPS_CACHE_TTL`, «Обновить список»); обработчики без обращений к event loop — обычные функции (пул потоков FastAPI), запись статуса и тяжёлые операции с БД в фоне — через `asyncio.to_thread`; SQLite — WAL и `busy_timeout`; файловая блокировка БД — один процесс на БД.
- `020-custom-select-menus` — выпадающие списки (фильтры, «Группа») в стиле меню действий «⋯»: прогрессивное улучшение в JS, нативный `select` остаётся в разметке и работает без JavaScript.
- `021-security-hardening` — отказ старта при небезопасных секретах (`API_TOKEN`/`SESSION_SECRET`/`ADMIN_PASSWORD`/`SECRET_KEY`), блокировка входа в UI по IP клиента, `SESSION_COOKIE_SECURE`, безопасный `next` в редиректах (`/ui/move`, `/backups/delete-many`).
+32
View File
@@ -1,7 +1,13 @@
from functools import lru_cache
from cryptography.fernet import Fernet
from pydantic_settings import BaseSettings, SettingsConfigDict
# минимальная длина секретов, принимаемая при старте (см. insecure_settings)
API_TOKEN_MIN_LEN = 32
SESSION_SECRET_MIN_LEN = 32
ADMIN_PASSWORD_MIN_LEN = 12
class Settings(BaseSettings):
model_config = SettingsConfigDict(env_file=".env", extra="ignore")
@@ -9,6 +15,7 @@ class Settings(BaseSettings):
database_url: str = "sqlite:///./data/ros_control.db"
secret_key: str = "" # ключ Fernet для паролей устройств
session_secret: str = "change-me"
session_cookie_secure: bool = False # Secure-флаг cookie сессии; включить за TLS
admin_user: str = "admin"
admin_password: str = "change-me"
@@ -37,3 +44,28 @@ class Settings(BaseSettings):
@lru_cache
def get_settings() -> Settings:
return Settings()
def insecure_settings(s: Settings) -> list[str]:
"""Проблемы конфигурации, с которыми приложению нельзя стартовать. Без значений секретов — только названия проблем."""
problems = []
if not s.api_token or s.api_token == "change-me":
problems.append("API_TOKEN: пустой или change-me")
elif len(s.api_token) < API_TOKEN_MIN_LEN:
problems.append(f"API_TOKEN: короче {API_TOKEN_MIN_LEN} символов")
if not s.session_secret or s.session_secret == "change-me":
problems.append("SESSION_SECRET: пустой или change-me")
elif len(s.session_secret) < SESSION_SECRET_MIN_LEN:
problems.append(f"SESSION_SECRET: короче {SESSION_SECRET_MIN_LEN} символов")
if not s.admin_password or s.admin_password == "change-me":
problems.append("ADMIN_PASSWORD: пустой или change-me")
elif len(s.admin_password) < ADMIN_PASSWORD_MIN_LEN:
problems.append(f"ADMIN_PASSWORD: короче {ADMIN_PASSWORD_MIN_LEN} символов")
if not s.secret_key:
problems.append("SECRET_KEY: пустой")
else:
try:
Fernet(s.secret_key.encode())
except Exception: # noqa: BLE001
problems.append("SECRET_KEY: не ключ Fernet")
return problems
+5 -2
View File
@@ -10,7 +10,7 @@ from starlette.middleware.sessions import SessionMiddleware
from app import process_lock
from app.api import v1
from app.config import get_settings
from app.config import get_settings, insecure_settings
from app.db import init_db
from app.services import backups, jobs, poller, rotation
from app.ui import routes as ui
@@ -18,6 +18,9 @@ from app.ui import routes as ui
@asynccontextmanager
async def lifespan(app: FastAPI):
problems = insecure_settings(get_settings())
if problems:
raise RuntimeError("Небезопасная конфигурация: " + "; ".join(problems) + "; см. README «Настройки»")
process_lock.acquire(get_settings().database_url) # один процесс на файловую БД (--workers 1)
try:
init_db()
@@ -39,7 +42,7 @@ def create_app() -> FastAPI:
app = FastAPI(title="ros_control", lifespan=lifespan)
app.add_middleware(
SessionMiddleware, secret_key=get_settings().session_secret,
same_site="strict", https_only=False,
same_site="strict", https_only=get_settings().session_cookie_secure,
)
app.mount("/static", StaticFiles(directory=Path(__file__).parent / "ui" / "static"), name="static")
app.include_router(v1.router)
+16 -13
View File
@@ -33,7 +33,7 @@ def check_admin(user: str, password: str) -> bool:
return ok_user and ok_pass
# --- подтверждение действий паролем (очистка журнала) и защита от перебора ---
# --- защита от перебора: вход в UI (по IP) и подтверждение действий паролем (очистка журнала, по пользователю) ---
FAIL_LIMIT, WINDOW_S, LOCK_S = 5, 600, 600 # 5 неверных за 10 минут → блокировка на 10 минут
_guard = threading.Lock()
_fails: dict[str, list[float]] = {}
@@ -47,29 +47,32 @@ def verify_password(user: str, password: str) -> bool:
and hmac.compare_digest(password.encode(), s.admin_password.encode()))
def lockout_remaining(user: str) -> int:
"""Сколько секунд осталось до конца блокировки (0 — не заблокирован)."""
def lockout_remaining(key: str) -> int:
"""Сколько секунд осталось до конца блокировки по ключу (0 — не заблокирован).
Ключ — своё пространство на каждый вид перебора (например, `f"login:{ip}"` для входа в UI
и имя пользователя для очистки журнала), чтобы счётчики не пересекались."""
with _guard:
return max(0, int(_locked_until.get(user, 0) - time.monotonic() + 0.999))
return max(0, int(_locked_until.get(key, 0) - time.monotonic() + 0.999))
def register_failure(user: str) -> int:
"""Учитывает неверный пароль; возвращает число оставшихся попыток (0 — пользователь заблокирован)."""
def register_failure(key: str) -> int:
"""Учитывает неверную попытку по ключу; возвращает число оставшихся попыток (0 — ключ заблокирован)."""
now = time.monotonic()
with _guard:
recent = [t for t in _fails.get(user, []) if now - t < WINDOW_S] + [now]
_fails[user] = recent
recent = [t for t in _fails.get(key, []) if now - t < WINDOW_S] + [now]
_fails[key] = recent
if len(recent) >= FAIL_LIMIT:
_locked_until[user] = now + LOCK_S
_fails[user] = []
_locked_until[key] = now + LOCK_S
_fails[key] = []
return 0
return FAIL_LIMIT - len(recent)
def reset_failures(user: str) -> None:
def reset_failures(key: str) -> None:
with _guard:
_fails.pop(user, None)
_locked_until.pop(user, None)
_fails.pop(key, None)
_locked_until.pop(key, None)
async def require_api_token(cred: HTTPAuthorizationCredentials | None = Depends(_bearer)) -> None:
+40 -10
View File
@@ -2,7 +2,7 @@
import json
from datetime import date
from pathlib import Path
from urllib.parse import urlencode
from urllib.parse import urlencode, urlsplit
from fastapi import APIRouter, Depends, Form, Request, Response
from fastapi.responses import HTMLResponse, RedirectResponse
@@ -34,7 +34,8 @@ templates.env.filters["num"] = lambda n: f"{int(n):,}".replace(",", "\u00a0")
templates.env.filters["dts"] = lambda v: v.strftime("%Y-%m-%d %H:%M:%S") if v else "—"
# смысловая окраска типов событий (как в утверждённом макете)
_EV_KIND = {"bad": ("device.offline", "job.failed", "auth.failed", "backup.failed", "journal.clear_denied", "journal.clear_locked"),
_EV_KIND = {"bad": ("device.offline", "job.failed", "auth.failed", "auth.locked", "backup.failed", "journal.clear_denied",
"journal.clear_locked"),
"ok": ("device.online", "job.done", "backup.done", "auth.login"),
"run": ("job.started",),
"warn": ("backup.deleted", "device.deleted", "group.deleted", "system.migrated", "journal.cleared", "journal.rotated")}
@@ -61,8 +62,8 @@ async def require_login(request: Request) -> None:
router = APIRouter(include_in_schema=False)
def _render(request: Request, name: str, **ctx):
return templates.TemplateResponse(request, name, ctx)
def _render(request: Request, name: str, status_code: int = 200, **ctx):
return templates.TemplateResponse(request, name, ctx, status_code=status_code)
def _is_htmx(request: Request) -> bool:
@@ -137,13 +138,31 @@ def login_form(request: Request):
@router.post("/login")
def login(request: Request, username: str = Form(), password: str = Form()):
"""Вход в UI: попытки ограничены по IP клиента (не по имени — иначе одного администратора
можно заблокировать чужими неверными попытками). При блокировке пароль не проверяется."""
ip = request.client.host if request.client else "unknown"
key = f"login:{ip}"
events.set_actor("anonymous")
lock = security.lockout_remaining(key)
if lock:
# событие auth.locked уже записано в момент начала блокировки (ветка left == 0 ниже);
# повторные попытки во время блокировки в журнал не пишем — иначе перебор засыпает его и вытесняет историю ротацией
minutes = max(1, -(-lock // 60))
return _render(request, "login.html", status_code=429,
error=f"Слишком много попыток входа. Повторите через {minutes} мин.")
if not security.check_admin(username, password):
events.set_actor("anonymous")
events.record("auth.failed", "session", None, "Неудачная попытка входа в UI", data={"user": username[:64]})
return _render(request, "login.html", error="Неверный логин или пароль")
left = security.register_failure(key)
if left == 0:
events.record("auth.locked", "session", None, "Вход заблокирован: превышено число попыток входа",
data={"ip": ip, "user": username[:64]})
else:
events.record("auth.failed", "session", None, "Неудачная попытка входа в UI",
data={"ip": ip, "user": username[:64], "attempts_left": left})
return _render(request, "login.html", status_code=401, error="Неверный логин или пароль")
security.reset_failures(key)
request.session["user"] = username
events.set_actor(f"ui:{username}")
events.record("auth.login", "session", None, f"Вход в UI: {username}", data={"user": username})
events.record("auth.login", "session", None, f"Вход в UI: {username}", data={"user": username, "ip": ip})
return RedirectResponse("/", status_code=303)
@@ -213,12 +232,23 @@ async def device_action(request: Request, device_id: str, action: str, channel:
return _render(request, "_devices.html", oob=True, **_devices_ctx(_flt(await request.form())))
def _safe_next(value: str, default: str = "/", prefix: str = "/") -> str:
"""Адрес для редиректа после формы, присланный пользователем (`next`/`back`): только локальный путь,
иначе открытый редирект (в т.ч. `\\evil.com`, который браузеры трактуют как `//evil.com`)."""
value = value or ""
if (not value.startswith(prefix) or value.startswith("//")
or any(ord(ch) < 0x20 or ch in ("\\", "\x7f") for ch in value)):
return default
parts = urlsplit(value)
return value if not parts.scheme and not parts.netloc else default
@router.post("/ui/move", dependencies=[Depends(require_login)])
def move(device_ids: list[str] = Form(default=[]), group_id: str = Form(""), next: str = Form("/")):
"""Перенос отмеченных устройств в группу (обычный POST + редирект: счётчики вкладок обновляются)."""
if device_ids:
groups.move_devices(device_ids, _opt_id(group_id, "grp"))
return RedirectResponse(next if next.startswith("/") and not next.startswith("//") else "/", status_code=303)
return RedirectResponse(_safe_next(next), status_code=303)
# --- окна (dialog) и формы: одна разметка для окна и запасной страницы ---
@@ -402,7 +432,7 @@ async def backups_page(request: Request, device: str = "", group: str = "", kind
async def backups_delete_many(key: list[str] = Form(default=[]), next: str = Form("/backups")):
"""Групповое удаление выбранных файлов; возврат на страницу с теми же фильтрами и итогом."""
deleted, failed = await backups.delete_many(key)
back = next if next.startswith("/backups") else "/backups"
back = _safe_next(next, default="/backups", prefix="/backups")
return RedirectResponse(back + ("&" if "?" in back else "?") + urlencode({"deleted": deleted, "failed": failed}), status_code=303)
@@ -0,0 +1,87 @@
# План: 021 — безопасность (по ревью 2026-09-27)
## Context
Ревью `docs/reviews/2026-09-27-codebase-review.md`, раздел «Безопасность», пункты 1–4:
- **п. 1** — `API_TOKEN`, `ADMIN_PASSWORD`, `SESSION_SECRET` по умолчанию `change-me` (`app/config.py`) и при старте не проверяются.
На стенде так и было: `API_TOKEN=change-me` — API был открыт с общеизвестным токеном.
- **п. 2** — вход в UI (`POST /login`, `app/ui/routes.py::login`) без ограничения попыток: пароль администратора можно перебирать.
- **п. 3** — сессионная cookie без флага Secure (`SessionMiddleware(..., https_only=False)` в `app/main.py`).
- **п. 4** — `POST /ui/move`: проверка `next` (`startswith("/") and not startswith("//")`) пропускает `/\evil.com`,
которое браузеры трактуют как `//evil.com` — открытый редирект.
Решения пользователя:
- п. 1 — при старте приложение **не запускается**, если `API_TOKEN`, `ADMIN_PASSWORD`, `SESSION_SECRET` пустые или `change-me`,
а также при недостаточной длине: `API_TOKEN` и `SESSION_SECRET` ≥ 32 символов, `ADMIN_PASSWORD` ≥ 12.
- `.env` стенда — оркестратор уже сгенерировал новые `API_TOKEN` (43), `SESSION_SECRET` (64), `ADMIN_PASSWORD` (20);
прежний файл — `.env.bak-20260928` (добавлен в `.gitignore` шаблоном `.env.bak-*`). Сессии UI и внешние скрипты со старым токеном перестанут работать.
- п. 2 — блокировка **по IP клиента**: 5 неверных попыток за 10 минут → IP заблокирован на 10 минут. Администратора (единственного) нельзя
заблокировать чужими попытками. Reverse-proxy сейчас нет — `request.client.host` достоверен (ограничение описать в README).
- п. 3 — настройка `SESSION_COOKIE_SECURE`, **по умолчанию `false`** (UI по http продолжает работать); README — включить за TLS.
## Изменения
### п. 1 — проверка секретов при старте (`app/config.py`, `app/main.py`, `.env.example`, `tests/conftest.py`)
- `config.py`: `def insecure_settings(s: Settings) -> list[str]` — список проблем **без значений секретов** (например,
«API_TOKEN: пустой или change-me», «SESSION_SECRET: короче 32 символов», «SECRET_KEY: не ключ Fernet»).
Правила: `API_TOKEN`, `SESSION_SECRET` — не пустые, не `change-me`, длина ≥ 32; `ADMIN_PASSWORD` — не пустой, не `change-me`, длина ≥ 12;
`SECRET_KEY` — не пустой и валидный ключ Fernet (`Fernet(key)` без исключения) — сейчас пустой ключ всплывает только при первом шифровании.
Константы минимальной длины — рядом с правилами.
- `main.py::lifespan`: **первым шагом** (до `process_lock.acquire`) — если список не пуст, `RuntimeError("Небезопасная конфигурация: …; см. README «Настройки»")`,
приложение не стартует. `create_app()` проверку не делает (модуль импортируется в тестах).
- `.env.example`: у секретов — пустые значения вместо `change-me` и комментарии с требованиями и командой генерации
(`python -c "import secrets; print(secrets.token_urlsafe(32))"`).
- `tests/conftest.py`: `API_TOKEN`, `ADMIN_PASSWORD` — значения, проходящие проверку (≥ 32 / ≥ 12); обновить тесты,
где эти значения захардкожены (`"pw"`, `"test-token"`) — через общие константы в conftest, а не копипастой.
### п. 2 — ограничение попыток входа по IP (`app/security.py`, `app/ui/routes.py`, `app/ui/templates/login.html`)
- **Переиспользовать** счётчики `security.lockout_remaining / register_failure / reset_failures` (5 за 10 минут → блок 10 минут).
Они сейчас ключуются именем пользователя (очистка журнала) — параметр переименовать в `key`, для входа ключ `f"login:{ip}"`,
для очистки журнала — прежний (имя пользователя), чтобы пространства ключей не пересекались. Поведение очистки журнала не меняется.
- `login`: `ip = request.client.host if request.client else "unknown"`.
1) если `lockout_remaining(key) > 0` — пароль **не проверяется**, форма с ошибкой «Слишком много попыток входа. Повторите через N мин.»,
код 429, событие `auth.locked` (IP, введённый логин ≤ 64 символов);
2) неверные данные — `register_failure(key)`; при 0 оставшихся — событие `auth.locked`, иначе `auth.failed` (в `data` — IP и число оставшихся попыток);
ответ, как сейчас (ошибка «Неверный логин или пароль», код 200 → **401**);
3) успех — `reset_failures(key)`, событие `auth.login` (в `data` добавить IP).
- `_EV_KIND` в `routes.py`: `auth.locked` — в группу `bad`.
- Пароли и секреты в журнал не пишутся (как сейчас).
### п. 3 — Secure-cookie (`app/config.py`, `app/main.py`, `.env.example`, README)
- `Settings.session_cookie_secure: bool = False`; `SessionMiddleware(..., https_only=get_settings().session_cookie_secure)`.
### п. 4 — безопасный `next` (`app/ui/routes.py`)
- Хелпер `_safe_next(value: str, default: str = "/", prefix: str = "/") -> str`: допускается только локальный путь —
начинается с `prefix`, не начинается с `//`, не содержит `\` и управляющих символов (`ord < 32`, `\x7f`),
`urllib.parse.urlsplit(value)` без `scheme` и `netloc`; иначе `default`.
- Применить в `move` (`prefix="/"`) и в `backups_delete_many` (`prefix="/backups"`, вместо текущего `startswith("/backups")`).
- Найти (grep `RedirectResponse(` и параметры `next`/`back`) другие редиректы с пользовательским вводом и применить тот же хелпер.
## Тесты (минимально, `tests/test_app.py`)
- п. 1: `insecure_settings` для `change-me`, коротких значений и невалидного `SECRET_KEY` возвращает проблемы и **не содержит значений секретов**;
запуск `TestClient(create_app())` с `API_TOKEN=change-me` падает с `RuntimeError`.
- п. 2: 5 неверных входов → 6-й (даже с верным паролем) → 429 и событие `auth.locked`; с другого IP (TestClient `client=("10.0.0.2", 123)`)
вход работает; после успешного входа счётчик сброшен.
- п. 4: `POST /ui/move` с `next` = `/\evil.com`, `//evil.com`, `https://evil.com`, `/%0d%0aX` → `Location: /`; `next=/?f_group=none` сохраняется.
- п. 3 — без отдельного теста (одна строка конфигурации).
## Документация
README: «Безопасность» (требования к секретам и отказ старта, блокировка входа по IP и ограничение за reverse-proxy, `SESSION_COOKIE_SECURE`),
«Настройки» (новые требования, `SESSION_COOKIE_SECURE`), число тестов, строка 021 в истории изменений. `summary.md` — оркестратор.
## Исполнение
Исполнитель (Sonnet): код, тесты, README, `.env.example`, пересборка стенда. `.env` стенда **не трогает** (уже обновлён оркестратором) и не выводит.
Тесты не запускает, не коммитит.
## Проверка
- `pytest` — все зелёные.
- Стенд (override 8001, `--force-recreate`): стартует с новым `.env`; `/login` → 200; новый код в контейнере.
- Отказ старта: временный контейнер из того же образа **без тома** с `API_TOKEN=change-me` → `RuntimeError`, в логе нет значений секретов.
- API: старый токен `change-me` → 401, новый → 200.
- Вход: 5 неверных паролей с одного адреса → 6-я попытка 429 даже с верным паролем; события `auth.failed` ×4, `auth.locked`; верный пароль с
другого адреса — вход (стенд без прокси: второй адрес — запрос изнутри контейнера или с другого интерфейса хоста).
После проверки — блокировка в памяти снимается перезапуском контейнера (данные не затрагиваются).
- Редирект: `POST /ui/move` с `next=/\evil.com` → `Location: /`.
- Боевые данные: сверка по ID до/после (согласованная копия через `sqlite3 backup`).
- Ручная проверка UI — пользователь: вход с новым паролем из `.env`, сообщение о блокировке после 5 ошибок.
@@ -0,0 +1,35 @@
# Итоги: 021 — безопасность (по ревью 2026-09-27)
Источник — ревью `docs/reviews/2026-09-27-codebase-review.md`, раздел «Безопасность» (пункты 1–4).
## Сделано
- **п. 1 — проверка секретов при старте** (`app/config.py::insecure_settings`, `app/main.py::lifespan` — первым шагом):
`API_TOKEN`, `SESSION_SECRET` — не пустые, не `change-me`, ≥ 32 символов; `ADMIN_PASSWORD` — ≥ 12; `SECRET_KEY` — валидный ключ Fernet.
Иначе `RuntimeError` со списком проблем без значений секретов, приложение не стартует. `.env.example` — пустые секреты с требованиями и командой генерации.
- **п. 2 — ограничение попыток входа по IP** (`app/ui/routes.py::login`): 5 неверных за 10 минут → IP заблокирован на 10 минут, пароль не проверяется, 429.
Неверный пароль — 401 (было 200). Счётчики `security.lockout_remaining/register_failure/reset_failures` (параметр `user` → `key`), ключ `login:<ip>`
не пересекается с ключом очистки журнала. События: `auth.failed` (IP, оставшиеся попытки), `auth.locked` — **один раз** при начале блокировки, `auth.login` с IP.
- **п. 3 — Secure-cookie**: `SESSION_COOKIE_SECURE` (по умолчанию `false`), README — включить за TLS.
- **п. 4 — безопасный `next`** (`_safe_next`): только локальный путь с нужным префиксом; `//`, `\`, управляющие символы, схема или хост → адрес по умолчанию.
Применён в `/ui/move` и групповом удалении бэкапов.
- `.env` стенда: сгенерированы новые `API_TOKEN` (43), `SESSION_SECRET` (64), `ADMIN_PASSWORD` (20); прежний — `.env.bak-20260928`;
`.gitignore` — шаблон `.env.bak-*`. README: «Безопасность», «Настройки», «Ограничения», история изменений.
## Найдено на ревью и исправлено
- Каждая попытка входа с заблокированного IP писала `auth.locked` — перебором можно было засыпать журнал и вытеснить историю ротацией. Теперь событие пишется один раз.
- Тест блокировки проверял 303 без `follow_redirects=False` и падал.
- Найдено до реализации: на стенде `API_TOKEN` был `change-me` (API открыт с общеизвестным токеном), `SESSION_SECRET` — 10 символов, `ADMIN_PASSWORD` — 8.
## Проверено
- `pytest`: 28 из 28 (новые: слабые секреты без утечки значений, отказ старта, блокировка входа по IP, открытый редирект).
- Отказ старта: временный контейнер без тома с `API_TOKEN=change-me` → `RuntimeError`; значений секретов в логе нет.
- API: `change-me` → 401, новый токен → 200.
- Вход: 5 неверных → 401 ×5; 6-я с верным паролем и ещё 3 → 429; с 127.0.0.1 (изнутри контейнера) → 303; события: `auth.failed` ×4, `auth.locked` ×1.
- Редирект: `/\evil.com`, `//evil.com`, `https://evil.com` → `/`; `/?f_group=none` сохраняется.
- Боевые данные: группы 4/4, устройства 14/14, бэкапы 47/47, задачи 73/73 — совпадают по ID; добавлено 7 событий входа проверки. Блокировка снята перезапуском контейнера.
## Оговорки
- **Ломающее для эксплуатации**: новые пароль администратора и токен API (в `.env`); старые сессии UI и скрипты со старым токеном не работают.
- Блокировка входа — в памяти процесса (сбрасывается перезапуском). За reverse-proxy все клиенты видны с IP прокси — нужна настройка доверенных прокси.
- API-токен не ограничен по попыткам (≥ 32 случайных символа — перебор непрактичен).
- Ручная проверка UI пользователем на момент коммита не подтверждена.
+9 -2
View File
@@ -5,19 +5,26 @@ from app import db, security
from app.config import get_settings
from app.services import backups
# секреты для тестов: значения, проходящие проверку insecure_settings (см. app/config.py), но не боевые
API_TOKEN = "test-api-token-0123456789abcdef0" # 32 символа
ADMIN_PASSWORD = "test-admin-pass123" # 12+ символов
SESSION_SECRET = "test-session-secret-0123456789ab" # 32 символа
@pytest.fixture(autouse=True)
def env(tmp_path, monkeypatch):
monkeypatch.setenv("DATABASE_URL", f"sqlite:///{tmp_path}/test.db")
monkeypatch.setenv("SECRET_KEY", Fernet.generate_key().decode())
monkeypatch.setenv("API_TOKEN", "test-token")
monkeypatch.setenv("SESSION_SECRET", SESSION_SECRET)
monkeypatch.setenv("API_TOKEN", API_TOKEN)
monkeypatch.setenv("S3_BUCKET", "test-bucket")
monkeypatch.setenv("S3_ACCESS_KEY", "") # без ключей S3: сверка бакета при старте отключена
monkeypatch.setenv("POLL_INTERVAL", "0") # фоновый опрос в тестах выключен
monkeypatch.setenv("ADMIN_USER", "admin") # не зависеть от реального .env
monkeypatch.setenv("ADMIN_PASSWORD", "pw")
monkeypatch.setenv("ADMIN_PASSWORD", ADMIN_PASSWORD)
get_settings.cache_clear()
security.reset_failures("admin") # блокировки очистки журнала — в памяти процесса
security.reset_failures("login:testclient") # блокировки входа — в памяти процесса (IP TestClient по умолчанию)
backups.invalidate() # кэш списка бакета — модульное состояние, не должен переживать тест
db.init_db()
yield
+84 -18
View File
@@ -7,15 +7,18 @@ from datetime import date, datetime, timedelta, timezone
import httpx
import pytest
from cryptography.fernet import Fernet
from fastapi.testclient import TestClient
from app import db, ids, process_lock, s3, security
from app.config import Settings, get_settings, insecure_settings
from app.main import create_app
from app.models import Backup, Device, Event, now
from app.db import session_scope
from app.ros import operations as ros
from app.ros.client import RosClient
from app.services import backups, devices, events, groups, jobs, ops, settings
from tests.conftest import ADMIN_PASSWORD, API_TOKEN
def ros_client(handler) -> RosClient:
@@ -88,7 +91,7 @@ async def test_backup_flow(monkeypatch):
def test_api_auth_and_no_password_leak():
with TestClient(create_app()) as client:
assert client.get("/api/v1/devices").status_code == 401
h = {"Authorization": "Bearer test-token"}
h = {"Authorization": f"Bearer {API_TOKEN}"}
r = client.post("/api/v1/devices", headers=h, json={
"name": "r1", "host": "10.0.0.1", "username": "admin", "password": "pw"})
assert r.status_code == 201 and "password" not in r.text
@@ -185,7 +188,7 @@ def test_create_device_via_ui_binds_group(monkeypatch):
form = dict(host="10.0.0.1", port="80", username="u", password="p")
htmx = {"HX-Request": "true"}
with TestClient(create_app()) as c:
c.post("/login", data={"username": "admin", "password": "pw"})
c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD})
# 1) существующая группа
r = c.post("/devices/new", data={**form, "name": "a", "group_id": str(office.id), "note": " Серверная, 2 этаж "},
follow_redirects=False)
@@ -256,7 +259,7 @@ def test_bulk_delete_backups(monkeypatch):
monkeypatch.setattr(s3, "list_backups", empty_list)
ok = ["backups/a/1.backup", "backups/a/1.rsc"]
with TestClient(create_app()) as c:
c.post("/login", data={"username": "admin", "password": "pw"})
c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD})
# UI: ключ вне префикса бэкапов -> ошибка, ничего не удалено
r = c.post("/backups/delete-many", data={"key": ok + ["other/secret.txt"]}, follow_redirects=False)
assert r.status_code == 400 and removed == []
@@ -266,7 +269,7 @@ def test_bulk_delete_backups(monkeypatch):
assert sorted(removed) == ok
# API
removed.clear()
h = {"Authorization": "Bearer test-token"}
h = {"Authorization": f"Bearer {API_TOKEN}"}
assert c.post("/api/v1/backups/delete", headers=h, json={"keys": ok}).json() == {"deleted": 2, "failed": 0}
assert c.post("/api/v1/backups/delete", headers=h, json={"keys": ["x/../y"]}).status_code == 400
# UI: одиночное удаление — тот же сервис delete_many (метаданные и событие backup.deleted не теряются)
@@ -299,8 +302,8 @@ async def test_chr_status_and_version_compare():
def test_device_name_is_immutable():
"""Имя задаётся только при создании: API отклоняет смену, форма изменения имя игнорирует."""
with TestClient(create_app()) as c:
c.post("/login", data={"username": "admin", "password": "pw"})
h = {"Authorization": "Bearer test-token"}
c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD})
h = {"Authorization": f"Bearer {API_TOKEN}"}
d = c.post("/api/v1/devices", headers=h, json={"name": "r1", "host": "10.0.0.1", "username": "u", "password": "p"}).json()
r = c.patch(f"/api/v1/devices/{d['id']}", headers=h, json={"name": "r2"})
assert r.status_code == 400 and "нельзя изменить" in r.text
@@ -434,7 +437,7 @@ async def test_batch_channel_runs_as_jobs(monkeypatch):
d2 = devices.create_device("r2", "10.0.0.2", 443, "admin", "pw")
with TestClient(create_app()) as c:
h = {"Authorization": "Bearer test-token"}
h = {"Authorization": f"Bearer {API_TOKEN}"}
r = c.put("/api/v1/batch/channel", headers=h, json={"device_ids": [d1.id, d2.id], "channel": "testing"})
assert r.status_code == 202
job_ids = r.json()["job_ids"]
@@ -490,9 +493,9 @@ async def test_events_link_entities(monkeypatch):
def test_events_api_and_id_validation():
"""API отдаёт журнал с ID; ID чужого типа и числовые ID дают 404; вход в UI попадает в журнал."""
with TestClient(create_app()) as c:
h = {"Authorization": "Bearer test-token"}
h = {"Authorization": f"Bearer {API_TOKEN}"}
c.post("/login", data={"username": "admin", "password": "wrong"})
c.post("/login", data={"username": "admin", "password": "pw"})
c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD})
d = c.post("/api/v1/devices", headers=h, json={"name": "r1", "host": "10.0.0.1", "username": "u", "password": "p"}).json()
assert ids.is_id(d["id"], "dev")
evs = c.get("/api/v1/events", headers=h, params={"entity_id": d["id"]}).json()
@@ -500,7 +503,7 @@ def test_events_api_and_id_validation():
assert c.get(f"/api/v1/events/{evs[0]['id']}", headers=h).json()["entity_id"] == d["id"]
auth = c.get("/api/v1/events", headers=h, params={"type": "auth"}).json()
assert sorted(e["type"] for e in auth) == ["auth.failed", "auth.login"]
assert "pw" not in json.dumps(auth) # пароли в журнал не попадают
assert ADMIN_PASSWORD not in json.dumps(auth) # пароли в журнал не попадают
assert c.get("/api/v1/devices/1", headers=h).status_code == 404
assert c.get(f"/api/v1/devices/{ids.new_id('job')}", headers=h).status_code == 404
assert c.get(f"/api/v1/jobs/{d['id']}", headers=h).status_code == 404
@@ -567,14 +570,14 @@ def test_journal_clear_requires_password_through_modal():
успех оставляет одну запись об очистке; в API очистки нет."""
htmx = {"HX-Request": "true"}
with TestClient(create_app()) as c:
h = {"Authorization": "Bearer test-token"}
c.post("/login", data={"username": "admin", "password": "pw"})
h = {"Authorization": f"Bearer {API_TOKEN}"}
c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD})
_fill_events(30)
before = events.count()
page = c.get("/ui/dialog/events-clear", headers=htmx).text
assert "Очистить журнал" in page and 'id="clear-confirm" disabled' in page # кнопка неактивна без пароля
assert c.post("/events/clear", data={"password": "pw"}).status_code == 400 # не через окно — отклонено
assert c.post("/events/clear", data={"password": ADMIN_PASSWORD}).status_code == 400 # не через окно — отклонено
assert events.count() == before
r = c.post("/events/clear", data={"password": ""}, headers=htmx)
assert "Введите пароль" in r.text and events.count() == before
@@ -583,29 +586,29 @@ def test_journal_clear_requires_password_through_modal():
assert f"Осталось попыток: {left}" in r.text and "HX-Refresh" not in r.headers
r = c.post("/events/clear", data={"password": "wrong"}, headers=htmx) # 5-я неверная — блокировка
assert "Слишком много неверных попыток" in r.text
r = c.post("/events/clear", data={"password": "pw"}, headers=htmx) # верный пароль при блокировке не принимается
r = c.post("/events/clear", data={"password": ADMIN_PASSWORD}, headers=htmx) # верный пароль при блокировке не принимается
assert "Повторите через" in r.text and "HX-Refresh" not in r.headers
denied = events.list_events(type_="journal.clear_denied")
assert len(denied) == 5 and "wrong" not in json.dumps([e.data for e in denied]) # пароль в журнал не пишется
assert events.list_events(type_="journal.clear_locked")
security.reset_failures("admin") # окончание блокировки
r = c.post("/events/clear", data={"password": "pw"}, headers=htmx)
r = c.post("/events/clear", data={"password": ADMIN_PASSWORD}, headers=htmx)
assert r.headers["HX-Refresh"] == "true"
left = events.list_events()
assert [e.type for e in left] == ["journal.cleared"] and left[0].actor == "ui:admin"
assert json.loads(left[0].data)["deleted"] > 30
assert c.delete("/api/v1/events", headers=h).status_code in (404, 405) # очистки через API нет
assert c.post("/api/v1/events/clear", headers=h, json={"password": "pw"}).status_code in (404, 405, 422)
assert c.post("/api/v1/events/clear", headers=h, json={"password": ADMIN_PASSWORD}).status_code in (404, 405, 422)
def test_journal_page_filters_cursor_dialogs_and_settings():
"""Страница журнала: фильтры, «Показать ещё» по курсору, окно записи, сохранение настроек."""
htmx = {"HX-Request": "true"}
with TestClient(create_app()) as c:
h = {"Authorization": "Bearer test-token"}
c.post("/login", data={"username": "admin", "password": "pw"})
h = {"Authorization": f"Bearer {API_TOKEN}"}
c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD})
d = c.post("/api/v1/devices", headers=h, json={"name": "r1", "host": "10.0.0.1", "username": "u", "password": "p"}).json()
_fill_events(105)
page = c.get("/events").text
@@ -685,3 +688,66 @@ def test_process_lock_blocks_second_process(tmp_path):
process_lock.release()
process_lock.acquire(db_url) # после освобождения — успешно
process_lock.release()
def test_insecure_settings_rejects_weak_secrets_without_leaking_them():
"""Небезопасные секреты выявляются по каждому правилу; значения секретов в описание проблем не попадают."""
secret = "s3cr3t-value-must-not-leak-anywhere"
bad = Settings(api_token="change-me", session_secret=secret[:10], admin_password=secret[:8], secret_key=secret)
problems = insecure_settings(bad)
assert len(problems) == 4 # все четыре секрета нарушают правила
text = " ".join(problems)
assert secret not in text and secret[:10] not in text and secret[:8] not in text
ok = Settings(api_token="x" * 32, session_secret="y" * 32, admin_password="z" * 12,
secret_key=Fernet.generate_key().decode())
assert insecure_settings(ok) == []
def test_app_refuses_to_start_with_insecure_config(monkeypatch):
"""Приложение не стартует с небезопасной конфигурацией (например, API_TOKEN=change-me)."""
monkeypatch.setenv("API_TOKEN", "change-me")
get_settings.cache_clear()
with pytest.raises(RuntimeError, match="Небезопасная конфигурация"):
with TestClient(create_app()):
pass
get_settings.cache_clear()
def test_login_lockout_by_ip():
"""5 неверных попыток входа с одного IP блокируют его на 6-ю (пароль уже не проверяется, код 429);
повторные попытки во время блокировки не засоряют журнал новыми auth.locked; блокировка не распространяется
на другой IP; успешный вход сбрасывает счётчик неудач."""
with TestClient(create_app()) as c:
for _ in range(5):
r = c.post("/login", data={"username": "admin", "password": "wrong"})
assert r.status_code == 401
r = c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}) # верный пароль уже не спасает
assert r.status_code == 429 and "Слишком много попыток" in r.text
assert len(events.list_events(type_="auth.locked")) == 1
for _ in range(3): # попытки во время блокировки — 429, но новых auth.locked не пишут (не засоряют журнал)
assert c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}).status_code == 429
assert len(events.list_events(type_="auth.locked")) == 1
with TestClient(create_app(), client=("10.0.0.2", 1)) as c2:
r = c2.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}, follow_redirects=False)
assert r.status_code == 303 # другой IP свободен
with TestClient(create_app(), client=("10.0.0.3", 1)) as c3:
for _ in range(3):
assert c3.post("/login", data={"username": "admin", "password": "wrong"}).status_code == 401
r = c3.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD}, follow_redirects=False)
assert r.status_code == 303
for _ in range(4): # если бы счётчик не сбросился успехом, пятая по счёту неудача (2-я в этом цикле) заблокировала бы
assert c3.post("/login", data={"username": "admin", "password": "wrong"}).status_code == 401
def test_move_redirect_rejects_open_redirect_next():
"""`next` в /ui/move принимает только локальный путь — иначе редирект на «/» (открытый редирект)."""
with TestClient(create_app()) as c:
c.post("/login", data={"username": "admin", "password": ADMIN_PASSWORD})
for bad in ("/\\evil.com", "//evil.com", "https://evil.com", "/\r\nX"): # последний — декодированный /%0d%0aX
r = c.post("/ui/move", data={"next": bad}, follow_redirects=False)
assert r.headers["location"] == "/"
r = c.post("/ui/move", data={"next": "/?f_group=none"}, follow_redirects=False)
assert r.headers["location"] == "/?f_group=none" # обычный путь с фильтром сохраняется