- TRUSTED_HOSTS env: configurable trusted hostnames for proxy-domain access (default stays strict: localhost/loopback/private IP); k8s manifest sets tg.workstation.internal (L-8 follow-up) - Media allowlist +10: mkv/mk3d/heic/tgs/flv/3gp/ogv/asf/wmv/djvu (live disk has .tgs x44, .mkv x2) - Cache buster: app.js?v=4 -> ?v=5 so browsers pick up the new bundle - +6 tests (64 passing); REVIEW.md updated with live-cluster rollout notes
27 KiB
REVIEW.md — telegram-scraper
Дата: 2026-09-07 Скоуп: полный аудит кода — безопасности и корректности (webui_server.py, app_state.py, scraper_jobs.py, telegram_scraper_with_forwarding.py, main.py, health.py, webui/*.js, деплой, тесты).
Статус: что уже исправлено
10 пунктов (критичные/высокие) исправлены 07.09.2026. Подробности — в истории коммита.
| ID | Проблема | Файл | Статус |
|---|---|---|---|
| C-2 | SSE-поток не завершался при успешном job (done vs completed) — утечка потоков до 30 мин |
webui_server.py | ✅ fixed |
| C-3 | Пути Path("data")/Path("session") относительно CWD расходились с BASE_DIR webui — тихая рассинхронизация данных |
telegram_scraper_with_forwarding.py, main.py, scraper_jobs.py | ✅ fixed |
| H-1 | TelegramAuthManager.lock объявлен, но не использовался — гонки на auth_data/clients между event-loop и HTTP-потоками |
webui_server.py | ✅ fixed (RLock) |
| H-2 | Коллизия job_id из миллисекундного timestamp |
webui_server.py | ✅ fixed (uuid4) |
| H-3 | Экспорт секретов через include_secrets=1 (api_hash/api_id) |
webui_server.py | ✅ fixed (всегда redact) |
| H-4 | CSRF: любой Content-Type, нет Origin/Sec-Fetch-Site проверки | webui_server.py | ✅ fixed (415 + same-origin 403) |
| H-5 | Нет rate limiting на брутфорс phone-code/2FA | webui_server.py | ✅ fixed (5 попыток → 60s lockout, 429) |
| H-6 | StateStore.load() возвращал мелкую копию — расшаренная мутация вложенных dict между потоками |
app_state.py | ✅ fixed (deepcopy) |
| H-7 | Бесконечная рекурсия forward_message при FloodWaitError |
telegram_scraper_with_forwarding.py | ✅ fixed (cap 3 retry) |
| H-8 | Повторная регистрация forward-хендлера → сообщения форвардились N раз | telegram_scraper_with_forwarding.py | ✅ fixed (remove_event_handler) |
Бонус при фиксах: migrate_database больше не глотает исключения (логирует), миграция покрывает все колонки MessageData; фронтенд app.js корректно распознаёт 'done' как терминальный статус.
Follow-up (второй проход по итогам ревью фиксов)
| ID | Проблема | Статус |
|---|---|---|
| H-6 | deepcopy теперь на всех путях load() (cache-hit + cache-miss + fallback-ветки) |
✅ fixed |
| H-5 | cooldown 30s на успешный запрос кода (анти-SMS-флуд) + _auth_attempts ограничен (sweep при >10k записей) |
✅ fixed |
| — | Origin-проверка на GET /api/jobs/{id}/events (403 до открытия SSE) |
✅ fixed |
| — | SSE-потоки на аккаунт ограничены (MAX_EVENT_STREAMS=10, revoke самого старого) — закрыт thread-exhaustion |
✅ fixed |
| — | read_json_body: кап тела 1 MB → 413 (sentinel), malformed Content-Length → не 500, не-UTF-8 → 400 |
✅ fixed (M-2 закрыт) |
| — | +6 регрессионных тестов (H-4/H-5/H-6/C-2, sweep, oversize body) | ✅ fixed |
Тесты: 29 passed (все зелёные после фиксов).
Follow-up (третий проход)
| ID | Что исправлено | Статус |
|---|---|---|
| M-17 | clean_continuous_channels() — валидация каналов на приёме в обоих POST-эндпоинтах (per-account + legacy); невалидные отбрасываются и возвращаются как dropped_invalid (drop, не 400 — фронтенд api() бросает на non-OK) |
✅ fixed |
| M-5 | PerAccountContinuousScrapeManager.join(timeout=20) + remove_account делает stop()+join перед rmtree; stop() идемпотентен; loop проверяет stop event на границах итераций |
✅ fixed |
| M-6 | self.config присваивается под локом; refresh_config больше не врёт о running (только stop_event); running=False ставится в finally потока при реальном выходе |
✅ fixed |
| — | +4 теста (33 passed) | ✅ fixed |
Follow-up (четвёртый проход)
| ID | Что исправлено | Статус |
|---|---|---|
| M-1 | /media/ lockdown: запрещены state.json/*.db/*.session + allowlist расширений (архивы, документы) |
✅ fixed |
| M-3 | Общий parse_bool() — строка "false"/"0" больше не даёт True |
✅ fixed |
| M-4 | HEAD /api/jobs/{id}/events для несуществующего job → 404 |
✅ fixed |
| M-7 | JobRunner.shutdown — дренаж очереди, оставшиеся jobs → failed/cancelled |
✅ fixed |
| M-8 | _parse_range: single-range edge cases (пустой файл, мульти-диапазоны) |
✅ fixed |
| M-19 | int(query...) → try/except → 400 JSON; пути из ответов ред.актированы |
✅ fixed |
| L-1 | Access log — только path, без query-параметров | ✅ fixed |
| L-4 | Security-заголовки: CSP, X-Content-Type-Options, X-Frame-Options, Referrer-Policy | ✅ fixed |
| L-5 | QR-токен: one-time + TTL 60s | ✅ fixed |
| L-6 | Legacy channels add/remove переведены на StateStore.update() — lost-update закрыт |
✅ fixed |
| L-8 | Trusted-host allowlist для same-origin проверки — DNS-rebinding закрыт | ✅ fixed |
| F-1 | Импорт и legacy-миграция прогоняют каналы через clean_continuous_channels |
✅ fixed |
| F-3 | start() при drain — проверка thread.is_alive()/join перед стартом |
✅ fixed |
| F-4 | remove_account — tombstone при таймауте join (дубли воркеров исключены) |
✅ fixed |
| M-10 | save(): flush()+fsync, уникальные tmp (mkstemp), sweep старых tmp |
✅ fixed |
| M-11 | Выделенный поток с new_event_loop(); set_scrape_media пишет per-account |
✅ fixed |
| M-12 | Media пакетами с ограничением размера чанка | ✅ fixed |
| M-13 | save_state — throttle (не перезапись каждые 50 сообщений) |
✅ fixed |
| M-14 | Точное переиспользование файлов (без произвольного {id}-* совпадения) |
✅ fixed |
| M-15 | /health агрегирует per-account проверки |
✅ fixed |
| M-16 | k8s: securityContext (runAsNonRoot, readOnlyRootFilesystem) + resources.limits |
✅ fixed |
| M-18 | chmod 700 на data/session, StateStore пишет 0600 | ✅ fixed |
| M-9 | Legacy GET /api/channels//api/dashboard делегируют в legacy_account_id |
✅ fixed |
| — | Фронтенд: F-2 (toast dropped_invalid), L-9 (предупреждение о redacted кредах при импорте), L-2 (swagger.js через textContent) |
✅ fixed |
| — | CI: job pip-audit (non-blocking) |
✅ fixed |
| — | +17 тестов → 50 passed | ✅ fixed |
Тесты: 50 passed (все зелёные после четвёртого прохода).
ОСТАВШИЕСЯ НАХОДКИ
🔴 C-1. Нет аутентификации на веб-панели, bind 0.0.0.0 + публичный ingress — won't fix (by design)
Файл/строки: webui_server.py:47-48 (DEFAULT_HOST=0.0.0.0), весь роутинг без auth-check, compose.yaml:11-12 (порт 7887 на всех интерфейсах), k8s/telegram-scraper.yaml:76-89 (IngressRoute tg.workstation.internal без middleware/basicAuth).
Любой, кто достаёт порт/домен, может:
- прочитать
api_hash/api_id(через export — теперь redact, но есть и другие пути, см. M-1:/media/accounts/<id>/state.json), - прочитать QR-токен авторизации и угнать Telegram-сессию владельца,
- подменить креды, удалить аккаунт (
DELETE /api/accounts/{id}→shutil.rmtree), - читать все чаты, медиа, логи, continuous-scrape состояние.
Won't fix — by design (решение пользователя: «аутх не надо, он онли локал» — только локальный деплой; риск сознательно принят). Блок рекомендаций ниже остаётся как справочник на случай, если панель когда-нибудь станет публичной. Рекомендуемый порядок:
BasicAuth/ForwardAuth/OIDC на Traefik IngressRoute (быстро, закрывает сетевой доступ).- App-level сессионная авторизация (cookie + random token), проверка в
do_GET/do_POST/do_DELETEдо диспатча. - Дефолт bind
127.0.0.1+ не публиковать 7887 на всех интерфейсах. - После ввода auth — пересмотреть M-1 (см. ниже), который сейчас маскируется отсутствием auth.
🟠 СРЕДНИЕ
| # | Файл:строка (актуально) | Проблема | Предложение |
|---|---|---|---|
serve_media) |
/media/ рутится в DATA_DIR целиком: GET /media/accounts/<id>/state.json отдаёт api_hash (plaintext), /media/accounts/<id>/<ch>/*.db — базы. Conтент-проверки нет, только containment |
✅ fixed | |
read_json_body |
✅ закрыт follow-up: кап 1 MB → 413, try/except, не-UTF-8 → 400 | ||
bool(body.get("value"/"enabled"/"run_all_tracked")) — строка "false"/"0" приходит как True. Фиксы H-4 не тронули эти места |
✅ fixed | ||
do_HEAD (2120) + stream_job_events (2064) |
/api/jobs/{id}/events для несуществующего job → 200 вместо 404 |
✅ fixed | |
ContinuousScrapeOrchestrator.remove_account) |
stop() только ставит event → shutil.rmtree может удалить DB/media, которые поток ещё пишет |
✅ fixed | |
refresh_config) |
status["running"]=False, пока поток ещё крутится (status врёт); update() присваивает self.config вне лока |
✅ fixed | |
JobRunner.shutdown) |
"queued" навсегда (worker выходит, не дрена́я очередь) |
✅ fixed | |
_parse_range) |
bytes=0-1,5-6 → 416; bytes=0-0 на пустом файле → 416 |
✅ fixed | |
webui_server.py:116-120 (load_state/save_state через STATE_STORE) vs app_state.py:123-131 (_GLOBAL_STORE) |
data/state.json — расхождение TTL-кэшей до 1s, конфликтные .tmp. Legacy GET /api/channels//api/dashboard после миграции читают пустой глобальный state (не делегируют в migrated account) |
✅ fixed | |
save) |
fsync перед rename (потеря питания → пустой/битый файл); фиксированное имя .tmp (два писателя в файл клообьют друг друга) |
✅ fixed | |
asyncio.run() на каждый job — RuntimeError при вызове из потока с существующим loop (e.g. auth loop thread); set_scrape_media пишет в глобальный STATE_STORE вместо per-account |
✅ fixed | ||
| ✅ fixed | |||
save_state) |
✅ fixed | ||
{id}-* может быть stale/частичным файлом |
✅ fixed | ||
/health читает глобальный state: в multi-account режиме всегда has_api_credentials: false, tracked_channels: 0 — вводит в заблуждение |
✅ fixed | ||
securityContext (root, r/w FS, нет limits); в Dockerfile нет USER (compose задаёт 1000:1000, k8s — нет) |
✅ fixed | ||
/api/continuous и /api/accounts/{id}/continuous) |
str().strip() без normalize_channel_id — безопасно только пока фильтрует _resolve_channels по tracked |
✅ fixed | |
root:root 755, state.json пишется 644 — session-файлы Telethon (полные auth-ключи) и api_hash читаемы локальными юзерами |
✅ fixed | ||
int(query...) без try/except → ValueError убивает поток + traceback в stderr; многие хендлеры эхат str(exc) (абс-пути в ответах) |
✅ fixed | ||
| M-20 | webui_server.py (все POST) | CSRF-фикс (H-4) закрыл Origin/Content-Type, но CSRF-токенов per-session нет; при вводе реальной auth (C-1) нужны | won't fix (by design: no auth, local-only deployment) |
⚪ НИЗКИЕ
| # | Файл | Проблема |
|---|---|---|
self.path с query-параметрами (поисковые запросы и т.п.) |
||
innerHTML с ошибкой из /openapi.json (низкий риск — серверный контент) |
||
~uv audit/pip-audit в CI |
||
| ~ |
||
StateStore.update() — lost-update race между потоками |
||
| L-7 | telegram_scraper_with_forwarding.py:838-841 | Прогресс-бар врут на инкрементальных прогонах (total vs only-new) — не исправлено (косметика) |
_check_same_origin |
Host == Origin.netloc проходит, если оба — домен атакующего (при rebinding Sec-Fetch-Site = same-origin). Закрыть allowlist'ом (localhost/127.0.0.1) или дефолт-bind 127.0.0.1 |
|
api_id/api_hash (H-3 redact): UI не предупреждает, что креды нужно ввести заново после импорта |
Follow-up findings (round 3)
🟠 СРЕДНИЕ (from round-3 review)
| # | Файл:строка | Проблема | Предложение |
|---|---|---|---|
continuous_scraping.channels как есть, минуя валидацию M-17 (латентно, т.к. _resolve_channels фильтрует по normalized tracked) |
✅ fixed | ||
dropped_invalid возвращается, но ни один JS его не читает — юзер не видит, что каналы отброшены |
✅ fixed | ||
start() early-return по status["running"], потом finally ставит False — аккаунт enabled=True, но мёртв до ручного переключения |
✅ fixed | ||
remove_account удаляет менеджера даже при таймауте join — recreate того же id создаёт второй воркер поверх живого (дубли) |
✅ fixed |
⚪ НИЗКИЕ (from round-3 review)
| # | Файл | Проблема |
|---|---|---|
channels: null/не-список молча стирает весь список каналов (([], [])) — рассмотреть 400 на malformed payload |
||
refresh_config/_save_config стрипают, но не нормализуют — @-значения с диска (import/migration) никогда не матчатся с normalized tracked, молча не скрейпятся |
||
except в _run_loop: исключение в refresh/auth-check убивает поток с последним_error нетронутым |
||
join()→False (таймаут) и start-during-drain; assert running is True после stop завязан на GIL-timing |
Round-4 residual notes — остаток после четвёртого прохода
Открыто после round 4:
- F-5 (
channels: nullмолча стирает список), F-6 (@-значения с диска никогда не нормализуются), F-7 (нет верхнегоexceptв_run_loop), F-8 (join-timeout / start-during-drain не покрыты тестами) — закрыты в round 5 (см. ниже). - M-20 CSRF-токены — won't fix (auth нет by design).
- L-7 прогресс-бар — открыт (косметика).
- Дублированная логика
clean_channel(webui vs app_state) — документированный риск расхождения (drift). - Экспорт
.json/.csvраздаётся через/media/(креды redact — риск низкий). - Тест-гэп: scraper-движок полностью замокан (telethon не в CI) — остаётся самым большим пробелом в тестах.
Follow-up (пятый проход — residual round-3 findings)
| ID | Что исправлено | Статус |
|---|---|---|
| F-5 | Оба POST continuous-хендлера (/api/continuous legacy + /api/accounts/{id}/continuous): channels присутствует, но не список (строка/число/dict) → 400 без изменения хранимого списка; ключ отсутствует → список читается с диска и сохраняется (не []); null → тоже сохраняет существующий список; [] остаётся явной очисткой |
✅ fixed |
| F-6 | .lstrip("@") в _load_config()/_save_config()/refresh_config() менеджера + app_state.StateStore.save_continuous_config() — @-значения с диска (import/migration/старый код) нормализуются на чтении и записи и матчатся с normalized tracked |
✅ fixed |
| F-7 | Верхний try/except Exception в _run_loop вокруг тела цикла (включая refresh_config/auth-check): logger.exception(...), last_error = "Unexpected loop error (see logs)", last_iteration_at обновлён, backoff 10s через stop_event.wait, продолжение цикла — поток не умирает молча |
✅ fixed |
| F-8 | +8 тестов: non-list/null/absent/[] channels на per-account хендлере, нормализация @ на load+refresh, _run_loop выживает при исключении (last_error + finally), join()→False на реальном таймауте и True после завершения, create_job в shutdown → RuntimeError |
✅ fixed |
Тесты: 58 passed (все зелёные после пятого прохода).
🧪 Пробелы в тестах
Покрыто новыми тестами (round 2, +6): deepcopy-изоляция load() (все пути), Content-Type/oversize в read_json_body, same-origin проверка, rate limiter (lockout + cooldown кода), sweep _auth_attempts, терминальные статусы SSE.
Осталось:
- tests/test_integration.py:29-38 — весь scraper-движок замокан (
sys.modules["telegram_scraper_with_forwarding"] = MagicMock()): реальный код (media naming, flood, forwarding, DB миграция, session) не покрыт вообще. Рекомендация: ставить telethon в CI и импортировать реальный модуль. - tests/test_integration.py:176-201 — тест удаления аккаунта дублирует логику хендлера инлайн, не вызывает продакшн-путь → регрессии в
_handle_delete_accountне ловятся. - Нет тестов на:
StateStoreTTL/atomic-write/конкурентныйupdate();_parse_range/ Range-ответы;normalize_media_url/guess_media_kind; 404 SSE; auth-флоу (QR/phone/2FA state machine); H-1 (lock-дисциплина); H-7/H-8 (scraper engine — упирается в полный мок движка). - tests/test_integration.py:48-50 — мутация
os.environна уровне импорта (leak между модулями). Лучшеmonkeypatch. - tests/test_integration.py:337-397 —
START_CONTINUOUS = Falseвыставляется в setup и не восстанавливается.
✅ Проверено — уязвимостей НЕТ
- Path traversal:
serve_media/serve_static—resolve()+relative_to()(корректно, включая symlink);normalize_channel_idотвергает/,\, control chars,./... - SQL injection: все запросы параметризованы,
search— черезLIKE ?. - XSS: viewer.js рендерит контент через
textContent/createTextNode; media-URL всегда префиксуется/media/(нетjavascript:схемы). - SSRF: юзер-контролируемого фетча URL нет (только MTProto).
- Десериализация: только JSON, без pickle/yaml.
- Command injection: нет subprocess/os.system в продакшн-путях.
- Secrets в image:
.dockerignoreисключаетdata/иsession/.
Follow-up (шестой проход — k8s-rollout audit живого кластера)
- TRUSTED_HOSTS (
webui_server.py, L-8):_is_trusted_hostотвергал ЛЮБОЙ hostname → через Traefik-доменtg.workstation.internal(k8s/telegram-scraper.yaml:97) браузерные POST/DELETE + SSE/api/jobs/*/eventsвозвращали 403. Добавлен_TRUSTED_HOSTS_ENV(parseTRUSTED_HOSTSна импорте, нормализация.strip().lower().rstrip(".")); проверка после localhost-set. Дефолт строгий: без env результаты идентичны прежним для ВСЕХ входов (regression guard); с env настроенный hostname (любой case, опциональный trailing dot) проходит, остальные — нет. IPv6-with-port ([::1]:8080) — out of scope, не тронут. - Media allowlist (
_MEDIA_FILE_EXTENSIONS): +10 суффиксов с комментарием# extended coverage (animated stickers, matroska, legacy containers)—.mkv .mk3d .heic .tgs .flv .3gp .ogv .asf .wmv .djvu. Живой диск:.tgsx44 /.mkvx2 (возвращали 403)..exe/.tsоставлены 403.guess_media_kindдля новых суффиксов возвращает"file"(fall-through как у.zip) — viewer рендерит "Open file" link.serve_mediaуже lowercases suffix,.MP4/.MOVок. - Cache-buster:
webui/index.htmlapp.js?v=4→app.js?v=5(иначе stale JS после deploy). - k8s-манифест:
containers[0].env: TRUSTED_HOSTS="tg.workstation.internal"(послеtty: true); image/probes/securityContext/resources/PVCs/IngressRoute не тронуты; YAML проверенyaml.safe_load. - Live-cluster факты: local-path PVCs, state v2
accounts=[default, forust], смешанное владение 1000/root → обязательныйchown -R 1000:1000 /app/data /app/sessionДО первого старта нового пода.
Тесты: 58 → 64 passed (+3 TRUSTED_HOSTS env, +3 extended media).