From ab89c873216f940b16d97c8eda53c00a56cafddf Mon Sep 17 00:00:00 2001 From: ayurishchev Date: Mon, 21 Sep 2026 10:30:03 +0300 Subject: [PATCH] Do not trust empty backups when sources are configured Restoring a backup with no data while sources are configured now sets the db_recreated.json marker (503 until data is collected), and the backup job skips copying an empty database in that state. Adds finding 11 to the review. Co-Authored-By: Claude Sonnet 5 --- README.md | 4 +-- collector_daemon.py | 3 ++ db.py | 50 +++++++++++++++++++++++++++--- docs/plan-empty-backup-guard.md | 28 +++++++++++++++++ docs/review-2026-09-21.md | 3 +- docs/summary-empty-backup-guard.md | 18 +++++++++++ tests/test_db.py | 35 +++++++++++++++++++++ 7 files changed, 134 insertions(+), 7 deletions(-) create mode 100644 docs/plan-empty-backup-guard.md create mode 100644 docs/summary-empty-backup-guard.md diff --git a/README.md b/README.md index 9bd4b45..d8fb438 100644 --- a/README.md +++ b/README.md @@ -522,11 +522,11 @@ Automatic: on the first start of any process (API, collector or CLI) `data.json` Stop both services, rename the `*.migrated-*` files back to `data.json` / `fqdn_data.json`, remove `ripe.db*`, start the previous version of the code. Addresses collected after the migration are lost in that case. ### Backup and automatic restore -**Backup job.** The collector daemon runs the job `backup` (default `30 4 * * *`, change it with `POST /schedule` and `"type": "backup"` or in `schedule.backup`). It makes an online copy of `ripe.db` (safe while the services run), checks the live database and the copy with `PRAGMA quick_check`, and keeps the last `backup_keep` copies (default 7; older ones are deleted only after a new copy succeeded). If the live database fails the check or the copy is bad, no copy is written, the older copies stay, and `GET /health` shows `degraded` (`jobs.backup.last_error`). +**Backup job.** The collector daemon runs the job `backup` (default `30 4 * * *`, change it with `POST /schedule` and `"type": "backup"` or in `schedule.backup`). It makes an online copy of `ripe.db` (safe while the services run), checks the live database and the copy with `PRAGMA quick_check`, and keeps the last `backup_keep` copies (default 7; older ones are deleted only after a new copy succeeded). An empty database is not copied while sources are configured (a warning is logged, the job stays successful, older copies are kept): an empty copy would push good ones out of the rotation and restoring it would give an empty list. If the live database fails the check or the copy is bad, no copy is written, the older copies stay, and `GET /health` shows `degraded` (`jobs.backup.last_error`). Copies are named `ripe-.db` and stored in `RIPE_BACKUP_DIR` (default `/backups`, i.e. `/data/backups` in Docker). **By default they are on the same volume as the database**: this protects against a corrupted file, not against losing the volume. For that, mount a separate volume/host directory and set `RIPE_BACKUP_DIR` to it, or copy the directory elsewhere regularly (e.g. `docker compose cp collector:/data/backups ./backups`). -**Automatic restore.** If `ripe.db` cannot be opened as a database (any process: API, daemon, CLI), the file is moved to `ripe.db.corrupt-` and the newest copy that passes the integrity check is put in its place; concurrent processes are serialized with a lock. The event is logged (ERROR), written to `last_restore.json` and shown in `GET /health` as `last_restore`. Data collected after that copy is lost; the collector gathers it again on the next runs. If there is no valid copy, a new empty database is created and the marker file `db_recreated.json` is written. **While the data is not gathered again, `GET /addresses` and `GET /addresses/diff` answer `503` with `Retry-After: 300`** instead of an empty list (a consumer could take it for the truth and wipe its rules); a type without configured sources (e.g. no FQDNs) is not blocked. The collector removes the marker after a run when every configured type has data again; to accept an empty result earlier, delete `db_recreated.json` by hand. `GET /health` shows `degraded` and `db_recreated` (`at`, `pending`) meanwhile. A fresh installation (no database yet) and a manually deleted database file are not treated as a loss and return an empty list until the first collection. +**Automatic restore.** If `ripe.db` cannot be opened as a database (any process: API, daemon, CLI), the file is moved to `ripe.db.corrupt-` and the newest copy that passes the integrity check is put in its place; concurrent processes are serialized with a lock. The event is logged (ERROR), written to `last_restore.json` and shown in `GET /health` as `last_restore`. Data collected after that copy is lost; the collector gathers it again on the next runs. If the newest valid copy has no data although sources are configured, it is restored and the same marker is written. If there is no valid copy, a new empty database is created and the marker file `db_recreated.json` is written. **While the data is not gathered again, `GET /addresses` and `GET /addresses/diff` answer `503` with `Retry-After: 300`** instead of an empty list (a consumer could take it for the truth and wipe its rules); a type without configured sources (e.g. no FQDNs) is not blocked. The collector removes the marker after a run when every configured type has data again; to accept an empty result earlier, delete `db_recreated.json` by hand. `GET /health` shows `degraded` and `db_recreated` (`at`, `pending`) meanwhile. A fresh installation (no database yet) and a manually deleted database file are not treated as a loss and return an empty list until the first collection. - The change journal goes back with the copy, so after a restore all cursors and times issued earlier answer `410` on `/addresses/diff` (the client makes a full download). The journal counter is shifted by 1,000,000 for that (a heuristic: it assumes fewer changes than that between two copies). - Only corruption detected **when the database is opened** is restored automatically. Damage inside the file shows up as `503` on reads and is caught by the `backup` job (`quick_check`); restore it by hand: stop both services, keep the damaged `ripe.db*`, copy the chosen `backups/ripe-*.db` to `ripe.db`, start the services. diff --git a/collector_daemon.py b/collector_daemon.py index 9d69415..6d0d826 100644 --- a/collector_daemon.py +++ b/collector_daemon.py @@ -28,6 +28,9 @@ def run_backup(): keep = max(1, int(load_full_config().get("backup_keep", cc.DEFAULT_BACKUP_KEEP))) with db.session() as conn: path = db.backup_database(conn, datetime.datetime.now(), keep) + if path is None: + logger.warning("Database backup skipped: no data yet although sources are configured") + return logger.info("Database backup written to %s (keeping the last %d)", path, keep) diff --git a/db.py b/db.py index d7ba9c7..5e4541a 100644 --- a/db.py +++ b/db.py @@ -102,6 +102,11 @@ def _recover(path, error, cc): if not _quick_check_file(candidate): logger.warning("Backup %s failed the integrity check, skipping", candidate) continue + if _backup_is_empty(candidate) and _sources_configured(cc): + # Пустая копия при настроенных источниках: метка ставится до подмены файла, как при пересоздании + logger.error("Backup %s has no data although sources are configured; the API withholds " + "addresses until the collector gathers data again", candidate) + _mark_recreated(cc, quarantine, error, "restored backup has no data for the configured sources") try: conn = _restore_backup(candidate, path, cc) except (sqlite3.Error, OSError) as e: @@ -115,8 +120,7 @@ def _recover(path, error, cc): logger.error("No valid backup for %s: a new empty database is created; the API withholds " "addresses until the collector gathers data again (remove %s to override)", path, cc.RECREATED_FILE) # Метка ставится до создания базы: сбой между шагами не оставит пустую базу без метки - save_json_atomic(cc.RECREATED_FILE, {"at": datetime.datetime.now().isoformat(timespec="seconds"), - "quarantine": quarantine, "error": str(error)}) + _mark_recreated(cc, quarantine, error, "no valid backup") conn = _open(path, cc) try: _reset_journal(conn) @@ -126,6 +130,40 @@ def _recover(path, error, cc): return conn +def _mark_recreated(cc, quarantine, error, reason): + """Метка «данные потеряны»: пока сборщик не соберёт их заново, API не отдаёт пустой список (recreated_pending).""" + save_json_atomic(cc.RECREATED_FILE, {"at": datetime.datetime.now().isoformat(timespec="seconds"), + "quarantine": quarantine, "error": str(error), "reason": reason}) + + +def _backup_is_empty(path): + """Копия не содержит ни одного адреса.""" + try: + conn = sqlite3.connect(f"file:{path}?mode=ro", uri=True, timeout=5.0) + try: + return conn.execute("SELECT COUNT(*) FROM addresses").fetchone()[0] == 0 + finally: + conn.close() + except sqlite3.Error: + return False + + +def _sources_configured(cc): + """В config.json есть хотя бы один ASN или FQDN. Нечитаемый конфиг - True (блокировка по умолчанию).""" + try: + config = cc.load_full_config() + except StorageError: + return True + return bool(config.get("asns") or config.get("fqdns")) + + +def empty_despite_sources(conn): + """В базе нет ни одного значения, хотя источники настроены (пустая база не должна выдаваться и копироваться).""" + import cidr_collector as cc + + return count_values(conn) == 0 and _sources_configured(cc) + + def _reset_journal(conn): """Очищает журнал и сдвигает счётчик: курсоры, выданные до потери или отката базы, станут недействительными.""" conn.execute("BEGIN IMMEDIATE") @@ -188,16 +226,20 @@ def settle_recreated(conn): def backup_database(conn, now, keep, backup_dir=None): - """Онлайн-копия базы с проверкой и ротацией. Возвращает путь копии. + """Онлайн-копия базы с проверкой и ротацией. Возвращает путь копии или None, если копировать нечего. Живая база и копия проверяются PRAGMA quick_check; при неудаче копия не появляется, старые не трогаются - (исключение StorageError). Лишние старые копии (сверх keep) удаляются только после успешной новой. + (исключение StorageError). Пустая база при настроенных источниках не копируется (None): пустая копия + вытеснила бы хорошие при ротации, а её восстановление дало бы пустую выдачу. Лишние старые копии + (сверх keep) удаляются только после успешной новой. """ import cidr_collector as cc backup_dir = backup_dir or cc.BACKUP_DIR if conn.execute("PRAGMA quick_check").fetchone()[0] != "ok": raise StorageError("Live database failed the integrity check, backup skipped") + if empty_despite_sources(conn): + return None os.makedirs(backup_dir, exist_ok=True) for stale in glob.glob(os.path.join(backup_dir, "ripe-*.db.tmp")): # хвосты прерванной копии os.unlink(stale) diff --git a/docs/plan-empty-backup-guard.md b/docs/plan-empty-backup-guard.md new file mode 100644 index 0000000..72629d4 --- /dev/null +++ b/docs/plan-empty-backup-guard.md @@ -0,0 +1,28 @@ +# План: защита от восстановления из пустой копии (находка 11) + +Источник: трассировка `_recover()` по графу знаний и проверка запуском (см. `docs/review-2026-09-21.md`, находка 11). + +## Проблема +`_recover` ставит на место повреждённой базы самую свежую исправную копию, но не смотрит на её содержимое. Если копия пуста (ночное задание `backup` сработало до первого сбора или во время его сбоев), а источники в `config.json` настроены, то после порчи базы API отвечает `200 []` без метки: та же угроза, что в находке 1 (потребитель может принять пустой список за истину), только через восстановление. Подтверждено запуском: копия пустой базы -> порча -> `/addresses` даёт `200 []`, метки нет. + +## Дизайн +- **Признак «пусто несмотря на источники»:** `db.empty_despite_sources(conn)`: в базе нет ни одного значения, а в `config.json` настроен хотя бы один ASN или FQDN. Сначала проверяется число значений (конфиг не читается, если данные есть); нечитаемый конфиг при пустой базе считается «пусто» (блокировка по умолчанию). + Критерий намеренно общий, а не по типам: копия, где есть ASN, но нет адресов FQDN (например, все адреса отфильтрованы как неглобальные), не должна вечно держать выдачу в ожидании. +- **Восстановление:** после успешного `_restore_backup` при `empty_despite_sources` пишется та же метка `db_recreated.json` (с полем `reason`). Дальше работает уже существующая защита: `/addresses` и `/addresses/diff` дают `503` с `Retry-After`, `/health` показывает `degraded` и `db_recreated`, сборщик снимает метку, когда данные собраны. Восстановление копии с данными метку не ставит. +- **Задание `backup`:** пустую базу при настроенных источниках не копирует: `backup_database` возвращает `None` (не ошибка: статус задания остаётся успешным, в лог пишется сообщение), ротация не выполняется, существующие копии не затрагиваются. Так пустые копии не вытесняют хорошие за `backup_keep` дней и не появляются вовсе. Пустая база без источников копируется как раньше. + +## Изменения +1. `db.py`: `empty_despite_sources`; `recreated_pending` использует общий разбор конфигурации; метка ставится в `_recover` (общая функция записи метки для обеих веток); `backup_database` возвращает `None` для пустой базы при источниках. +2. `collector_daemon.py`: `run_backup` обрабатывает `None` (сообщение в лог). +3. `README.md`: разделы о резервной копии и автовосстановлении. +4. `docs/review-2026-09-21.md`: находка 11; итоги в `docs/summary-empty-backup-guard.md`. +5. Тесты (1 новый, всего 28): пустая база при настроенных источниках копию не создаёт, копия с данными восстанавливается без метки, пустая копия (снятая, когда источников не было) при появлении источников даёт метку и `503`-состояние. + +## Не входит +Тип-специфичное ожидание при восстановлении (см. выше); копии вне сервера; удаление уже существующих пустых копий (при необходимости - вручную из `backups/`). + +## Проверка +Тесты в контейнере; повтор запуска, воспроизводившего проблему: восстановление из пустой копии при настроенном источнике теперь даёт `503`, метка есть, после появления данных выдача возобновляется; копия с данными - без метки. + +## Откат +Убрать проверку в `_recover` и `backup_database` (возврат к предыдущему коммиту); схема данных не затрагивается, метку можно удалить вручную. diff --git a/docs/review-2026-09-21.md b/docs/review-2026-09-21.md index 32ec80f..2ac6b8d 100644 --- a/docs/review-2026-09-21.md +++ b/docs/review-2026-09-21.md @@ -22,6 +22,7 @@ | 8 | Низкая | `Dockerfile` копирует модули явным списком: новый модуль не попадёт в образ, ошибка проявится при запуске (healthcheck поймает, но поздно). | `Dockerfile:12` | Исправлено (`summary-review-fixes-5-10.md`) | | 9 | Низкая | Запросы к RIPEstat без повторов и без параметра `sourceapp`. | `cidr_collector.py:119` | Исправлено (`summary-review-fixes-5-10.md`) | | 10 | Низкая | `get_changes`: по запросу на каждое значение при проверке наличия; на десятках тысяч изменений станет заметно. | `db.py` | Исправлено (`summary-review-fixes-5-10.md`) | +| 11 | Низкая-средняя | **Восстановление из пустой копии давало `200 []` без метки (найдено трассировкой `_recover()` по графу, подтверждено запуском).** Копия, снятая до первого сбора или во время его сбоев, при настроенных источниках восстанавливалась без защиты из находки 1. | `db.py` (`_recover`, `backup_database`) | Исправлено (`summary-empty-backup-guard.md`) | ## Архитектура и сопровождение - Цикл `db` ↔ `cidr_collector` обходится отложенным импортом; чище вынести пути и значения по умолчанию в `settings.py`. @@ -45,4 +46,4 @@ ## Рекомендуемый порядок 1. Доработка «Безопасность ввода и данных»: п. 2, 3, 4 и тесты (`plan-input-hardening.md`). 2. Находка 1: после порчи без копий отвечать 503 до первого успешного сбора (`plan-loss-guard.md`, выполнено). -3. Пункты 5-10 исправлены отдельной доработкой (`plan-review-fixes-5-10.md`, выполнено); из ревью остаются только архитектурные замечания (вынос путей в `settings.py`, разделение `api_server.py` и `cidr_collector.py`). +3. Находка 11 добавлена после трассировки графа и исправлена (`plan-empty-backup-guard.md`). Пункты 5-10 исправлены отдельной доработкой (`plan-review-fixes-5-10.md`, выполнено); из ревью остаются только архитектурные замечания (вынос путей в `settings.py`, разделение `api_server.py` и `cidr_collector.py`). diff --git a/docs/summary-empty-backup-guard.md b/docs/summary-empty-backup-guard.md new file mode 100644 index 0000000..a917e7a --- /dev/null +++ b/docs/summary-empty-backup-guard.md @@ -0,0 +1,18 @@ +# Итоги: защита от восстановления из пустой копии (находка 11) + +План: `docs/plan-empty-backup-guard.md`. Находка добавлена в `docs/review-2026-09-21.md` после трассировки `_recover()` по графу знаний. + +## Сделано +- **Восстановление (`db._recover`):** копия, в которой нет ни одного адреса, при настроенных источниках (`config.json` с ASN или FQDN, нечитаемый конфиг тоже считается «настроены») восстанавливается вместе с меткой `db_recreated.json` (поле `reason`). Метка ставится до подмены файла базы, чтобы другой процесс не успел открыть пустую базу без метки. Дальше действует прежняя защита: `503` с `Retry-After` на `/addresses` и `/addresses/diff`, `degraded` и `db_recreated` в `/health`, сборщик снимает метку после появления данных. Копия с данными метку не ставит. +- **Задание `backup` (`db.backup_database`, `run_backup`):** пустая база при настроенных источниках не копируется (`None`, предупреждение в логе, статус задания успешный, ротация не выполняется). Пустые копии не вытесняют хорошие при ротации и не появляются вовсе; без источников копия делается как раньше. +- **Общие функции:** `_mark_recreated` (метка для обеих веток восстановления), `_sources_configured`, `empty_despite_sources`, `_backup_is_empty`. +- **README, отчёт ревью** (находка 11 исправлена). +- **Тесты:** 1 новый (пустая база не копируется при источниках; пустая копия при появлении источников даёт метку; копия с данными восстанавливается без метки). Всего 28, в контейнере 28 passed. + +## Проверка +Повтор запуска, воспроизводившего проблему (контейнерное окружение): пустая копия -> порча -> `/addresses` теперь `503` (`Retry-After: 300`), `/addresses/diff` `503`, `/health` `db_recreated: {pending: true}`; после появления данных `200` с адресами и метка снята. Задание `backup` при настроенном источнике и пустой базе пропущено (копий было 1, стало 1). + +## Замечания +- Критерий «пусто» общий, а не по типам: копия с ASN, но без адресов FQDN (например, все отфильтрованы как неглобальные), метку не ставит и не блокирует выдачу. +- Уже существующие пустые копии не удаляются: при необходимости - вручную из `backups/`. +- Docker Compose не запускался: изменение затрагивает только логику восстановления и копирования, проверенную тестом и запуском в контейнере тестового образа. diff --git a/tests/test_db.py b/tests/test_db.py index f025353..49c5633 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -158,3 +158,38 @@ def test_recreated_database_guard(files): db.merge_source(conn, "asn", "1", {"a"}, datetime.datetime.now(), 90) db.settle_recreated(conn) assert not os.path.exists(cc.RECREATED_FILE) + + +def test_empty_backup_is_not_trusted(files): + now = datetime.datetime.now() + later = now + datetime.timedelta(minutes=1) + (files / "config.json").write_text('{"asns": [], "fqdns": []}') + with db.session() as conn: + # Источников нет: пустая база копируется как раньше + empty_copy = db.backup_database(conn, now, keep=5) + assert empty_copy and db.list_backups(cc.BACKUP_DIR) == [empty_copy] + + # Источник настроен, данных ещё нет: копия не создаётся (None), существующие остаются + (files / "config.json").write_text('{"asns": [1], "fqdns": []}') + assert db.empty_despite_sources(conn) and db.backup_database(conn, later, keep=5) is None + assert db.list_backups(cc.BACKUP_DIR) == [empty_copy] + + # Порча базы: восстановление из пустой копии не должно дать законный пустой список - ставится метка + (files / "ripe.db").write_bytes(b"garbage" * 1000) + with db.session() as conn: + assert db.get_values(conn) == [] and os.path.exists(cc.RECREATED_FILE) + assert db.recreated_pending(conn, ("asn",)) + assert json.loads((files / "db_recreated.json").read_text())["reason"].startswith("restored backup") + + # Данные собраны: метка снимается, копия с данными есть, её восстановление метку не ставит + with db.transaction(conn): + db.merge_source(conn, "asn", "1", {"a"}, later, 90) + db.settle_recreated(conn) + assert not os.path.exists(cc.RECREATED_FILE) and not db.empty_despite_sources(conn) + good_copy = db.backup_database(conn, later, keep=5) + assert good_copy + for stale in glob.glob(str(files / "ripe.db*")): + os.unlink(stale) + (files / "ripe.db").write_bytes(b"garbage" * 1000) + with db.session() as conn: + assert db.get_values(conn) == ["a"] and not os.path.exists(cc.RECREATED_FILE)