fix(cache): канонизация ключа канала (Durov/durov/@name -> один ключ) + one-shot миграция #47

Merged
vvzvlad merged 2 commits from fix/24-channel-key into main 2026-07-10 04:14:38 +03:00
Collaborator

Summary

Один канал жил под ключами Durov/durov/@name в трёх слоях (tgcache-файлы, data/cache/ каталоги, SQLite-строки) → дубли кеша, двойные походы в Telegram, осиротевшие деревья. ID↔username НЕ унифицируем (сознательная не-цель).

  • channel_key.pycanonical_channel_key (strip @, -100… числовые verbatim, иначе lowercase; безопасно и для Telegram API-вызовов).
  • tg_cache._cache_file_path прогоняет ключ через canonical.
  • post_parser.get_channel_username → lowercase (обе ветки — канонизирует SQLite-строки, media-URL, t.me-ссылки). Диф — ровно эти 2 строки, числовая ветка не тронута.
  • api_server.get_media: fs_channel = canonical_channel_key(channel) ПОСЛЕ проверки дайджеста (дайджест — по ИСХОДНОМУ url, иначе ломаются все выданные ссылки); fs_channel во всех путях/ключах/download ниже.
  • migrate_channel_keys_sync в lifespan (после init_db_sync, до client.start, через asyncio.to_thread): канал целиком FS-потом-SQL. Обязательный os.path.samefile guard ДО merge/rmtree — на case-insensitive FS Durov//durov/ один inode, иначе снесли бы весь кеш канала. FS-сбой → SQL-шаг пропускается (строки старорегистровые, старое дерево видно свиперу — нет вечных orphan). SQL-merge PK-safe (twin → merge added=max/mime non-NULL + DELETE, иначе plain UPDATE; НЕ bare SET channel=lower()). Идемпотентно.

closes #24

Важно про миграцию

  • Остаточные старорегистровые строки/каталоги, не покрытые миграцией (канал, всплывший позже), доживают до 20-дневного вытеснения свипером.
  • Миграция односторонняя (reverse нет): откат PR после её выполнения зеркалит re-download-сценарий (ограниченный, самоизлечивается за 20 дней) — ожидаемый признак отката, не авария.

How verified

  • .venv/bin/python -m pytest tests/ -q571 passed (+16 новых в test_channel_key.py): unit-ключ (Durov/@durov/-1001… int&str); tgcache-путь идентичен для Durov/durov; get_channel_username('MixedCase')→'mixedcase' (обе ветки); миграция — SQL-merge → одна строка max(added)+non-NULL mime; samefile-guard no-op без потери данных; merge двух реально разных каталогов (target выигрывает); повторный запуск no-op.
  • Внутреннее ревreview подтвердило 8 критических инвариантов (дайджест vs исходный url; samefile-guard перед rmtree; FS→SQL порядок; PK-safe merge; идемпотентность; scope post_parser 2 строки; проброс fs_channel).

Внутренний цикл: 1 проход coder → внутреннее ревреview → APPROVE (без правок; 2 косметических нита).

Checklist

  • /rss/Durov и /rss/durov → один tgcache-файл; /media/Durov/... и /media/durov/... → один файл на диске
  • миграция на case-insensitive FS = no-op без потери данных; pytest зелёный; post_parser — только lowercase
## Summary Один канал жил под ключами `Durov`/`durov`/`@name` в трёх слоях (tgcache-файлы, `data/cache/` каталоги, SQLite-строки) → дубли кеша, двойные походы в Telegram, осиротевшие деревья. ID↔username НЕ унифицируем (сознательная не-цель). - **`channel_key.py`** — `canonical_channel_key` (strip `@`, `-100…` числовые verbatim, иначе lowercase; безопасно и для Telegram API-вызовов). - **`tg_cache._cache_file_path`** прогоняет ключ через canonical. - **`post_parser.get_channel_username`** → lowercase (обе ветки — канонизирует SQLite-строки, media-URL, t.me-ссылки). Диф — ровно эти 2 строки, числовая ветка не тронута. - **`api_server.get_media`**: `fs_channel = canonical_channel_key(channel)` **ПОСЛЕ** проверки дайджеста (дайджест — по ИСХОДНОМУ url, иначе ломаются все выданные ссылки); `fs_channel` во всех путях/ключах/download ниже. - **`migrate_channel_keys_sync`** в `lifespan` (после `init_db_sync`, до `client.start`, через `asyncio.to_thread`): канал целиком **FS-потом-SQL**. Обязательный `os.path.samefile` guard **ДО** merge/rmtree — на case-insensitive FS `Durov/`/`durov/` один inode, иначе снесли бы весь кеш канала. FS-сбой → SQL-шаг пропускается (строки старорегистровые, старое дерево видно свиперу — нет вечных orphan). SQL-merge PK-safe (twin → merge `added=max`/`mime` non-NULL + `DELETE`, иначе plain `UPDATE`; НЕ bare `SET channel=lower()`). Идемпотентно. closes #24 ## Важно про миграцию - Остаточные старорегистровые строки/каталоги, не покрытые миграцией (канал, всплывший позже), доживают до 20-дневного вытеснения свипером. - Миграция **односторонняя** (reverse нет): откат PR после её выполнения зеркалит re-download-сценарий (ограниченный, самоизлечивается за 20 дней) — ожидаемый признак отката, не авария. ## How verified - `.venv/bin/python -m pytest tests/ -q` → **571 passed** (+16 новых в `test_channel_key.py`): unit-ключ (`Durov`/`@durov`/`-1001…` int&str); tgcache-путь идентичен для Durov/durov; `get_channel_username('MixedCase')→'mixedcase'` (обе ветки); миграция — SQL-merge → одна строка `max(added)`+non-NULL mime; samefile-guard no-op без потери данных; merge двух реально разных каталогов (target выигрывает); повторный запуск no-op. - Внутреннее ревreview подтвердило 8 критических инвариантов (дайджест vs исходный url; samefile-guard перед rmtree; FS→SQL порядок; PK-safe merge; идемпотентность; scope post_parser 2 строки; проброс fs_channel). Внутренний цикл: 1 проход coder → внутреннее ревреview → APPROVE (без правок; 2 косметических нита). ## Checklist - [x] /rss/Durov и /rss/durov → один tgcache-файл; /media/Durov/... и /media/durov/... → один файл на диске - [x] миграция на case-insensitive FS = no-op без потери данных; pytest зелёный; post_parser — только lowercase
agent_coder added 1 commit 2026-07-10 03:41:24 +03:00
Один канал жил под ключами Durov/durov/@name в трёх слоях (tgcache-файлы,
data/cache/ каталоги, SQLite-строки) -> дубли кеша, двойные походы в Telegram,
осиротевшие деревья. ID<->username НЕ унифицируем (сознательно).

- channel_key.py: canonical_channel_key (strip @, -100... числовые verbatim,
  иначе lowercase; безопасно и для API-вызовов).
- tg_cache._cache_file_path прогоняет ключ через canonical.
- post_parser.get_channel_username -> lowercase (обе ветки; канонизирует
  SQLite/media-URL/t.me). Только эти 2 строки.
- api_server.get_media: fs_channel = canonical_channel_key(channel) ПОСЛЕ проверки
  дайджеста (дайджест — по ИСХОДНОМУ url, иначе ломаются выданные ссылки); fs_channel
  во всех путях/ключах/download ниже.
- migrate_channel_keys_sync в lifespan (после init_db_sync, до client.start, через
  asyncio.to_thread): канал целиком FS-потом-SQL. Обязательный os.path.samefile
  guard ДО merge/rmtree — на case-insensitive FS Durov/durov один inode, иначе
  снесли бы кеш. FS-сбой -> SQL пропуск (строки старорегистровые, дерево видно
  свиперу, нет вечных orphan). SQL-merge PK-safe (twin -> merge added=max/mime
  non-NULL + DELETE, иначе plain UPDATE). Идемпотентно.

closes #24

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_coder added the review/needs label 2026-07-10 03:41:25 +03:00
Collaborator

Ревью — #47 fix(cache): канонизация ключа канала (Durov/durov/@name -> один ключ) + one-shot миграция

Вердикт: ПРАВКИ (CHANGES) — 2 правки (тест data-loss-guard'а миграции + traversal-гард на деструктивные FS-операции). Ядро корректно.

Полный веер из 9 аспектов по всему диффу PR (merge-base d5a38c3f..11bfd2b3). Гейт зелёный: pytest tests/ -p no:cacheprovider571 passed.

Что проверено и корректно (клик)
  • Канонизация (canonical_channel_key): username → lower, -100…+isdigit → точная форма, @ стрипается. Применена КОНСИСТЕНТНО на всех сайтах ключа (api_server/tg_cache/post_parser, read+write) — сплит-кеша не остаётся (подтверждено stability).
  • Миграция (migrate_channel_keys_sync, авто на старте через to_thread): collision-мерж корректен (dst/twin выигрывает, added=max, non-NULL mime; DELETE старой строки без PK-конфликта). Forward FS-fail консистентность есть (FS упал → SQL пропущен, continue :122). Case-insensitive FS guard (samefile) не даёт снести кеш на macOS/APFS. Идемпотентно и дёшево: кандидаты = WHERE channel != lower(channel) → после миграции пусто, FS-обход не запускается (marker-флаг не нужен).
  • Безопасность прочего: ReDoS/секретов нет.

Правки (сделать, потом ре-ревью)

  • [test-coverage] Не покрыт data-loss-guard миграции (FS-fail → SQL пропуск) + однонаправленный maxmigrate_channel_keys.py:114-122, тесты tests/test_channel_key.py
    Гард «FS-шаг упал → пропустить SQL, оставить строки old-cased» (защита от орфанов/потери) НЕ тестируется: мутация — убрать continue на :122 → все тесты зелёные. Плюс max(added) тестируется только в одну сторону (Durov=200/durov=100→200); мутация max(added,t_added)→added проходит (данные маскируют баг). Fix: (a) test_migration_fs_failure_skips_sql — замокать os.rename на OSError, ассертить что миграция не падает, DB-строки остались old-cased, счётчик failures вырос; (b) реверс-тест max: Durov=100/durov=300 → 300.

  • [security] Деструктивная миграция делает FS-операции по имени канала из БД без traversal-гардаmigrate_channel_keys.py:97-113
    src/dst = os.path.join(cache_dir, name) где name из SELECT DISTINCT channel, затем os.rename / _merge_dir_tree / shutil.rmtree. Если в БД попадёт канал с / или .. (напр. через pre-existing route-traversal /media/{channel} — см. #46, вне scope), миграция переименует/удалит директории ВНЕ cache_dir. Это новая деструктивная поверхность (#47 добавляет rename/rmtree на потенциально грязных данных). Fix: перед FS-шагом отбрасывать (skip + log, НЕ падать на старте) каналы, чьё имя содержит / или .. (или os.path.basename). canonical_channel_key — естественная точка гарда, но так, чтобы миграция скипала, а не крашила стартап.

DROP — кодеру НЕ делать (лог калибровки)
  • [speculative] high→low [security] symlink-атака в _merge_dir_tree (src — symlink наружу → файлы уезжают, rmtree молча не чистит) — требует, чтобы атакующий уже имел FS-доступ на запись в cache_dir (тогда игра и так проиграна). Нереалистичная модель угрозы.
  • [below-threshold] medium→low [stability] обратный кейс (FS успел, SQL упал → DB old-cased, файлы lowercase) не обработан — само-заживает (промах → ре-даунлоуд, не потеря данных), триггер редкий (простой UPDATE, busy_timeout=5000); two-phase-commit несоразмерен.
  • [below-threshold] low [coherence] remove_old_cached_files_sync/download_new_files строят путь инлайн вместо media_cache_path() — работает (каналы в БД канонические), централизация — nice-to-have.
  • [below-threshold] low [arch] нет migration-version маркера — идемпотентно и дёшево (см. выше), маркер это микро-оптимизация.
  • [below-threshold] low [documentation/simplification] докстринг «SAFE for API» размыт; редундантные str() перед media_cache_path — косметика.

Канонизация корректна и полна, миграция аккуратная (collision-мерж, forward-консистентность, идемпотентность). Остались: тест на data-loss-guard (мутационно не покрыт) + traversal-гард на деструктивные FS-операции. После правок — PASS.

9 аспектов (security · stability · regressions · test-coverage · conventions · documentation · simplification · architecture · coherence). Консистентность всех сайтов ключа, идемпотентность-дёшево и FS-ops-по-БД-имени проверены мной по коду. Две «CRITICAL/HIGH ESCALATE» security-находки ревьюера пересмотрены: traversal → MEDIUM DO (реальная деструктивная поверхность, но эксплойт требует pre-existing route-traversal), symlink → DROP (модель угрозы). Escalate нет.

<!-- state:review reviewed_head=11bfd2b3 round=1 verdict=changes-requested --> ## Ревью — #47 `fix(cache): канонизация ключа канала (Durov/durov/@name -> один ключ) + one-shot миграция` **Вердикт: ПРАВКИ (CHANGES)** — 2 правки (тест data-loss-guard'а миграции + traversal-гард на деструктивные FS-операции). Ядро корректно. Полный веер из 9 аспектов по всему диффу PR (merge-base `d5a38c3f`..`11bfd2b3`). Гейт зелёный: `pytest tests/ -p no:cacheprovider` → **571 passed**. <details> <summary>Что проверено и корректно (клик)</summary> - **Канонизация** (`canonical_channel_key`): username → lower, `-100…`+isdigit → точная форма, `@` стрипается. Применена КОНСИСТЕНТНО на всех сайтах ключа (api_server/tg_cache/post_parser, read+write) — сплит-кеша не остаётся (подтверждено stability). - **Миграция** (`migrate_channel_keys_sync`, авто на старте через `to_thread`): collision-мерж корректен (dst/twin выигрывает, `added=max`, non-NULL mime; DELETE старой строки без PK-конфликта). Forward FS-fail консистентность есть (FS упал → SQL пропущен, `continue` :122). Case-insensitive FS guard (`samefile`) не даёт снести кеш на macOS/APFS. **Идемпотентно и дёшево**: кандидаты = `WHERE channel != lower(channel)` → после миграции пусто, FS-обход не запускается (marker-флаг не нужен). - Безопасность прочего: ReDoS/секретов нет. </details> ### Правки (сделать, потом ре-ревью) - **[test-coverage] Не покрыт data-loss-guard миграции (FS-fail → SQL пропуск) + однонаправленный max** — `migrate_channel_keys.py:114-122`, тесты `tests/test_channel_key.py` Гард «FS-шаг упал → пропустить SQL, оставить строки old-cased» (защита от орфанов/потери) НЕ тестируется: мутация — убрать `continue` на :122 → все тесты зелёные. Плюс `max(added)` тестируется только в одну сторону (Durov=200/durov=100→200); мутация `max(added,t_added)→added` проходит (данные маскируют баг). Fix: (a) `test_migration_fs_failure_skips_sql` — замокать `os.rename` на `OSError`, ассертить что миграция не падает, DB-строки остались old-cased, счётчик failures вырос; (b) реверс-тест max: Durov=100/durov=300 → 300. - **[security] Деструктивная миграция делает FS-операции по имени канала из БД без traversal-гарда** — `migrate_channel_keys.py:97-113` `src/dst = os.path.join(cache_dir, name)` где `name` из `SELECT DISTINCT channel`, затем `os.rename` / `_merge_dir_tree` / `shutil.rmtree`. Если в БД попадёт канал с `/` или `..` (напр. через pre-existing route-traversal `/media/{channel}` — см. #46, вне scope), миграция переименует/удалит директории ВНЕ `cache_dir`. Это новая деструктивная поверхность (#47 добавляет rename/rmtree на потенциально грязных данных). Fix: перед FS-шагом отбрасывать (skip + log, НЕ падать на старте) каналы, чьё имя содержит `/` или `..` (или `os.path.basename`). `canonical_channel_key` — естественная точка гарда, но так, чтобы миграция скипала, а не крашила стартап. <details> <summary>⛔ DROP — кодеру НЕ делать (лог калибровки)</summary> - `[speculative]` `high→low` **[security]** symlink-атака в `_merge_dir_tree` (src — symlink наружу → файлы уезжают, rmtree молча не чистит) — требует, чтобы атакующий уже имел FS-доступ на запись в `cache_dir` (тогда игра и так проиграна). Нереалистичная модель угрозы. - `[below-threshold]` `medium→low` **[stability]** обратный кейс (FS успел, SQL упал → DB old-cased, файлы lowercase) не обработан — само-заживает (промах → ре-даунлоуд, не потеря данных), триггер редкий (простой UPDATE, busy_timeout=5000); two-phase-commit несоразмерен. - `[below-threshold]` `low` **[coherence]** `remove_old_cached_files_sync`/`download_new_files` строят путь инлайн вместо `media_cache_path()` — работает (каналы в БД канонические), централизация — nice-to-have. - `[below-threshold]` `low` **[arch]** нет migration-version маркера — идемпотентно и дёшево (см. выше), маркер это микро-оптимизация. - `[below-threshold]` `low` **[documentation/simplification]** докстринг «SAFE for API» размыт; редундантные `str()` перед `media_cache_path` — косметика. </details> Канонизация корректна и полна, миграция аккуратная (collision-мерж, forward-консистентность, идемпотентность). Остались: тест на data-loss-guard (мутационно не покрыт) + traversal-гард на деструктивные FS-операции. После правок — PASS. <sub>9 аспектов (security · stability · regressions · test-coverage · conventions · documentation · simplification · architecture · coherence). Консистентность всех сайтов ключа, идемпотентность-дёшево и FS-ops-по-БД-имени проверены мной по коду. Две «CRITICAL/HIGH ESCALATE» security-находки ревьюера пересмотрены: traversal → MEDIUM DO (реальная деструктивная поверхность, но эксплойт требует pre-existing route-traversal), symlink → DROP (модель угрозы). Escalate нет.</sub>
agent_reviewer added review/changes-requested and removed review/needs labels 2026-07-10 03:51:58 +03:00
Author
Collaborator

Поправил обе правки.

[security] Traversal-гард на деструктивные FS-операции миграции — добавлен. _is_safe_channel_segment(name) отвергает имя канала с /, \, компонентом .. или где os.path.basename(name) != name. Вызывается в НАЧАЛЕ цикла миграции — до вычисления canonical и до любого os.rename/_merge_dir_tree/shutil.rmtree; т.к. candidates = union БД-строк и листинга каталогов, один гард покрывает оба источника. Грязный канал (например если пролез через pre-existing route-traversal /media/{channel}) → warning + failures++ + continue: старт не падает, деструктивная rename/rmtree не уходит за cache_dir, старорегистровые строки/каталоги доживают до 20-дневного свипа как любой немигрированный канал.

[test-coverage] добавлены 3 теста (мутационно проверены):

  • test_migration_fs_failure_skips_sql: os.renameOSError → миграция не падает, DB-строки остались old-cased (SQL пропущен), в сводке failures 1. Снять continue после FS-сбоя → тест краснеет.
  • test_migration_sql_merge_reverse_max: Durov added=100 / durov added=300 → выживший durov с added=300 (max в обратную сторону) + non-NULL mime.
  • test_migration_traversal_guard_skips_dirty_channel: каналы '../Evil'/'a/B' → миграция скипает, os.rename/shutil.rmtree НЕ вызваны, без краха. Отключить гард → тест краснеет.

Проверка: .venv/bin/python -m pytest tests/ -q574 passed; git diff --stat — только migrate_channel_keys.py + tests/test_channel_key.py. Готово к повторному ревью.

(Заодно этот гард защищает миграцию от того самого path-traversal в /media, который я вынес тебе отдельным вопросом — независимо от того, чинишь ли ты сам роут.)

Поправил обе правки. **[security] Traversal-гард на деструктивные FS-операции миграции** — добавлен. `_is_safe_channel_segment(name)` отвергает имя канала с `/`, `\`, компонентом `..` или где `os.path.basename(name) != name`. Вызывается в НАЧАЛЕ цикла миграции — до вычисления `canonical` и до любого `os.rename`/`_merge_dir_tree`/`shutil.rmtree`; т.к. `candidates` = union БД-строк и листинга каталогов, один гард покрывает оба источника. Грязный канал (например если пролез через pre-existing route-traversal `/media/{channel}`) → warning + `failures++` + `continue`: старт не падает, деструктивная rename/rmtree не уходит за `cache_dir`, старорегистровые строки/каталоги доживают до 20-дневного свипа как любой немигрированный канал. **[test-coverage]** добавлены 3 теста (мутационно проверены): - `test_migration_fs_failure_skips_sql`: `os.rename`→`OSError` → миграция не падает, DB-строки остались old-cased (SQL пропущен), в сводке `failures 1`. Снять `continue` после FS-сбоя → тест краснеет. - `test_migration_sql_merge_reverse_max`: `Durov` added=100 / `durov` added=300 → выживший `durov` с `added=300` (max в обратную сторону) + non-NULL mime. - `test_migration_traversal_guard_skips_dirty_channel`: каналы `'../Evil'`/`'a/B'` → миграция скипает, `os.rename`/`shutil.rmtree` НЕ вызваны, без краха. Отключить гард → тест краснеет. Проверка: `.venv/bin/python -m pytest tests/ -q` → **574 passed**; `git diff --stat` — только `migrate_channel_keys.py` + `tests/test_channel_key.py`. Готово к повторному ревью. (Заодно этот гард защищает миграцию от того самого path-traversal в `/media`, который я вынес тебе отдельным вопросом — независимо от того, чинишь ли ты сам роут.)
agent_coder added review/needs and removed review/changes-requested labels 2026-07-10 03:56:57 +03:00
agent_coder added 1 commit 2026-07-10 03:56:57 +03:00
- _is_safe_channel_segment: отвергает имя канала с '/'/'\\'/'..' или не-basename;
  вызывается в начале цикла миграции ДО любого os.rename/_merge_dir_tree/rmtree,
  покрывает оба источника кандидатов (SELECT DISTINCT channel + листинг каталогов).
  Грязный канал (напр. из pre-existing route-traversal) -> skip+warning+failures++,
  старт не падает, деструктивная rename/rmtree не уходит за cache_dir.
- Тесты (мутационно проверены): FS-fail -> SQL пропущен, строки old-cased,
  failures++ (снять continue -> тест краснеет); reverse max(added) (Durov=100/
  durov=300 -> 300); traversal-гард скипает '../Evil'/'a/B' без FS-операций.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator

Ревью (раунд 2) — #47 fix(cache): канонизация ключа канала (Durov/durov/@name -> один ключ) + one-shot миграция

Вердикт: PASS — обе правки раунда 1 закрыты.

Полный веер из 9 аспектов по всему диффу PR (merge-base d5a38c3f..76f79a1c). Гейт зелёный: pytest tests/ -p no:cacheprovider574 passed (+3 к раунду 1). Обе правки проверил адверсариально (мутации).

Закрытие правок раунда 1 (мутационно подтверждено)

  • [security] Traversal-гард на деструктивные FS-операции миграции_is_safe_channel_segment(name) (migrate_channel_keys.py): отвергает /, \, ..-компонент, non-basename, пустое/.; вызывается на :115 ПЕРЕД os.rename/_merge_dir_tree/shutil.rmtree, грязный канал скипается (log + continue, не крашит стартап), FS и SQL для него не трогаются. Легитимные каналы (username, -100…) все проходят — ложных скипов нет. Мутация: return True в гарде → test_migration_traversal_guard_skips_dirty_channel падает.
  • [test-coverage] Data-loss-guard + reverse-max покрыты:
    • test_migration_fs_failure_skips_sql — мокает os.rename→OSError, ассертит: миграция не падает, DB-строки остались old-cased, failures вырос. Мутация: убрать continue на FS-fail (:153) → тест падает (был не покрыт в раунде 1 — теперь держит).
    • test_migration_sql_merge_reverse_max — Durov=100/durov=300 → 300. Мутация: max(added,t_added)→added → тест падает.
Контекст раунда 1 (всё подтверждено) — клик

Канонизация корректна и применена на всех сайтах ключа (нет сплит-кеша); миграция: collision-мерж (dst/twin, max added, non-NULL mime), forward FS-fail консистентность, case-insensitive FS guard, идемпотентна и дёшева. DROP'ы раунда 1 (symlink — нереалистичная модель угрозы; reverse FS/SQL — само-заживает; path-DRY; no-marker) остаются в силе.

DO/ESCALATE нет. Обе правки раунда 1 закрыты корректно и мутационно-подтверждены; traversal-гард полон (проверено на 18 edge-кейсах: абсолютные пути, .., null-байты, сепараторы — всё блокируется, легит проходит); гейт зелёный. Approve.

9 аспектов. Обе правки проверены мной адверсариально (3 мутации: FS-fail continue, max-direction, guard) + независимо двумя субагентами; полнота гарда подтверждена. Escalate нет.

<!-- state:review reviewed_head=76f79a1c round=2 verdict=approved --> ## Ревью (раунд 2) — #47 `fix(cache): канонизация ключа канала (Durov/durov/@name -> один ключ) + one-shot миграция` **Вердикт: PASS** ✅ — обе правки раунда 1 закрыты. Полный веер из 9 аспектов по всему диффу PR (merge-base `d5a38c3f`..`76f79a1c`). Гейт зелёный: `pytest tests/ -p no:cacheprovider` → **574 passed** (+3 к раунду 1). Обе правки проверил адверсариально (мутации). ### Закрытие правок раунда 1 (мутационно подтверждено) - **[security] Traversal-гард на деструктивные FS-операции миграции** — `_is_safe_channel_segment(name)` (migrate_channel_keys.py): отвергает `/`, `\`, `..`-компонент, non-basename, пустое/`.`; вызывается на :115 ПЕРЕД `os.rename`/`_merge_dir_tree`/`shutil.rmtree`, грязный канал скипается (log + `continue`, не крашит стартап), FS и SQL для него не трогаются. Легитимные каналы (username, `-100…`) все проходят — ложных скипов нет. Мутация: `return True` в гарде → `test_migration_traversal_guard_skips_dirty_channel` падает. - **[test-coverage] Data-loss-guard + reverse-max покрыты**: - `test_migration_fs_failure_skips_sql` — мокает `os.rename→OSError`, ассертит: миграция не падает, DB-строки остались old-cased, `failures` вырос. Мутация: убрать `continue` на FS-fail (:153) → тест падает (был не покрыт в раунде 1 — теперь держит). - `test_migration_sql_merge_reverse_max` — Durov=100/durov=300 → 300. Мутация: `max(added,t_added)→added` → тест падает. <details> <summary>Контекст раунда 1 (всё подтверждено) — клик</summary> Канонизация корректна и применена на всех сайтах ключа (нет сплит-кеша); миграция: collision-мерж (dst/twin, max added, non-NULL mime), forward FS-fail консистентность, case-insensitive FS guard, идемпотентна и дёшева. DROP'ы раунда 1 (symlink — нереалистичная модель угрозы; reverse FS/SQL — само-заживает; path-DRY; no-marker) остаются в силе. </details> DO/ESCALATE нет. Обе правки раунда 1 закрыты корректно и мутационно-подтверждены; traversal-гард полон (проверено на 18 edge-кейсах: абсолютные пути, `..`, null-байты, сепараторы — всё блокируется, легит проходит); гейт зелёный. Approve. <sub>9 аспектов. Обе правки проверены мной адверсариально (3 мутации: FS-fail continue, max-direction, guard) + независимо двумя субагентами; полнота гарда подтверждена. Escalate нет.</sub>
agent_reviewer added review/approved and removed review/needs labels 2026-07-10 04:07:24 +03:00
vvzvlad merged commit 4cf1293adc into main 2026-07-10 04:14:38 +03:00
Sign in to join this conversation.