Files
telegram-scraper/REVIEW.md
T
forust 2a75537fb9
ci / lint-prettier (push) Failing after 10s
ci / lint-ruff (push) Failing after 4s
ci / lint-yaml (push) Successful in 5s
ci / lint-dockerfiles (push) Successful in 5s
ci / validate (push) Successful in 5s
ci / lint-audit (push) Failing after 49s
ci / publish (push) Has been skipped
fix(server): k8s rollout readiness
- 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
2026-09-07 13:36:16 +02:00

27 KiB
Raw Blame History

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 (решение пользователя: «аутх не надо, он онли локал» — только локальный деплой; риск сознательно принят). Блок рекомендаций ниже остаётся как справочник на случай, если панель когда-нибудь станет публичной. Рекомендуемый порядок:

  1. BasicAuth/ForwardAuth/OIDC на Traefik IngressRoute (быстро, закрывает сетевой доступ).
  2. App-level сессионная авторизация (cookie + random token), проверка в do_GET/do_POST/do_DELETE до диспатча.
  3. Дефолт bind 127.0.0.1 + не публиковать 7887 на всех интерфейсах.
  4. После ввода auth — пересмотреть M-1 (см. ниже), который сейчас маскируется отсутствием auth.

🟠 СРЕДНИЕ

# Файл:строка (актуально) Проблема Предложение
M-1 webui_server.py:2829+ (serve_media) /media/ рутится в DATA_DIR целиком: GET /media/accounts/<id>/state.json отдаёт api_hash (plaintext), /media/accounts/<id>/<ch>/*.db — базы. Conтент-проверки нет, только containment fixed
M-2 read_json_body Нет капа тела, malformed Content-Length → 500 закрыт follow-up: кап 1 MB → 413, try/except, не-UTF-8 → 400
M-3 webui_server.py (много: 2168-2176, 2188, 2195, 2484, 2709, 2716) bool(body.get("value"/"enabled"/"run_all_tracked")) — строка "false"/"0" приходит как True. Фиксы H-4 не тронули эти места fixed
M-4 webui_server.py do_HEAD (2120) + stream_job_events (2064) HEAD на /api/jobs/{id}/events для несуществующего job → 200 вместо 404 fixed
M-5 webui_server.py:1102+ (ContinuousScrapeOrchestrator.remove_account) Удаление аккаунта не джойнит поток continuous scrape: stop() только ставит event → shutil.rmtree может удалить DB/media, которые поток ещё пишет fixed
M-6 webui_server.py:872+ (refresh_config) Ставит status["running"]=False, пока поток ещё крутится (status врёт); update() присваивает self.config вне лока fixed
M-7 webui_server.py:404-426 (JobRunner.shutdown) Очередные jobs остаются "queued" навсегда (worker выходит, не дрена́я очередь) fixed
M-8 webui_server.py:2879+ (_parse_range) Мульти-диапазоны bytes=0-1,5-6 → 416; bytes=0-0 на пустом файле → 416 fixed
M-9 webui_server.py legacy endpoints + webui_server.py:116-120 (load_state/save_state через STATE_STORE) vs app_state.py:123-131 (_GLOBAL_STORE) Два независимых StateStore на один файл data/state.json — расхождение TTL-кэшей до 1s, конфликтные .tmp. Legacy GET /api/channels//api/dashboard после миграции читают пустой глобальный state (не делегируют в migrated account) fixed
M-10 app_state.py:72-81 (save) Нет fsync перед rename (потеря питания → пустой/битый файл); фиксированное имя .tmp (два писателя в файл клообьют друг друга) fixed
M-11 scraper_jobs.py:14-25 asyncio.run() на каждый job — RuntimeError при вызове из потока с существующим loop (e.g. auth loop thread); set_scrape_media пишет в глобальный STATE_STORE вместо per-account fixed
M-12 telegram_scraper_with_forwarding.py (scrape_channel) Держит все media-объекты в памяти за весь проход (100k+ сообщений в большом канале) fixed
M-13 telegram_scraper_with_forwarding.py:127-131 (save_state) Перезапись всего per-account JSON каждые 50 сообщений — сотни сериализаций на длинный канал fixed
M-14 telegram_scraper_with_forwarding.py (existing_files glob) Первое произвольное совпадение {id}-* может быть stale/частичным файлом fixed
M-15 health.py:73-83 /health читает глобальный state: в multi-account режиме всегда has_api_credentials: false, tracked_channels: 0 — вводит в заблуждение fixed
M-16 webui_server.py(s) + k8s Контейнер в k8s без securityContext (root, r/w FS, нет limits); в Dockerfile нет USER (compose задаёт 1000:1000, k8s — нет) fixed
M-17 webui_server.py continuous endpoints (обе версии /api/continuous и /api/accounts/{id}/continuous) Список каналов сохраняется сырым str().strip() без normalize_channel_id — безопасно только пока фильтрует _resolve_channels по tracked fixed
M-18 data/ и session/ (хост) root:root 755, state.json пишется 644 — session-файлы Telethon (полные auth-ключи) и api_hash читаемы локальными юзерами fixed
M-19 webui_server.py:1860-1862, 2011-2013 и др. 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)

НИЗКИЕ

# Файл Проблема
L-1 webui_server.py:2776-2782 (access log) Логируется весь self.path с query-параметрами (поисковые запросы и т.п.) fixed
L-2 webui/swagger.js:52 innerHTML с ошибкой из /openapi.json (низкий риск — серверный контент) fixed
L-3 requirements.txt ~Зависимости корректны (aiohttp 3.12.14 — патч CVE-2025-53643), но Telethon 1.40.0 (есть 1.44.x); добавить uv audit/pip-audit в CI fixed
L-4 webui_server.py send_json/serve_file Нет security-заголовков: CSP, X-Content-Type-Options, X-Frame-Options/frame-ancestors, Referrer-Policy (clickjacking актуален после ввода auth) fixed
L-5 webui_server.py auth snapshots (582-589) ~QR-токен и его изображение висят в snapshot до сканирования — one-time + expiry 60s fixed
L-6 webui_server.py:2147-2173 (legacy channels add/remove) Паттерн load→save вместо StateStore.update() — lost-update race между потоками fixed
L-7 telegram_scraper_with_forwarding.py:838-841 Прогресс-бар врут на инкрементальных прогонах (total vs only-new) — не исправлено (косметика)
L-8 webui_server.py _check_same_origin DNS-rebinding: Host == Origin.netloc проходит, если оба — домен атакующего (при rebinding Sec-Fetch-Site = same-origin). Закрыть allowlist'ом (localhost/127.0.0.1) или дефолт-bind 127.0.0.1 fixed
L-9 webui/app.js:944-962, webui/settings.js:248-266 Import/export round-trip молча теряет api_id/api_hash (H-3 redact): UI не предупреждает, что креды нужно ввести заново после импорта

Follow-up findings (round 3)

🟠 СРЕДНИЕ (from round-3 review)

# Файл:строка Проблема Предложение
F-1 webui_server.py:2592, app_state.py:263 Импорт аккаунта и legacy-миграция пишут continuous_scraping.channels как есть, минуя валидацию M-17 (латентно, т.к. _resolve_channels фильтрует по normalized tracked) fixed
F-2 webui_server.py:2880 + webui/app.js:207-211 dropped_invalid возвращается, но ни один JS его не читает — юзер не видит, что каналы отброшены fixed
F-3 webui_server.py:1048-1058 Enable во время drain: start() early-return по status["running"], потом finally ставит False — аккаунт enabled=True, но мёртв до ручного переключения fixed
F-4 webui_server.py:1246-1254 remove_account удаляет менеджера даже при таймауте join — recreate того же id создаёт второй воркер поверх живого (дубли) fixed

НИЗКИЕ (from round-3 review)

# Файл Проблема
F-5 webui_server.py:237-238 channels: null/не-список молча стирает весь список каналов (([], [])) — рассмотреть 400 на malformed payload
F-6 webui_server.py:987-991 refresh_config/_save_config стрипают, но не нормализуют — @-значения с диска (import/migration) никогда не матчатся с normalized tracked, молча не скрейпятся
F-7 webui_server.py:1104-1171 Нет верхнего except в _run_loop: исключение в refresh/auth-check убивает поток с последним_error нетронутым
F-8 tests/test_integration.py:441-477 Новые тесты не покрывают 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 не ловятся.
  • Нет тестов на: StateStore TTL/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_staticresolve() + 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 (parse TRUSTED_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. Живой диск: .tgs x44 / .mkv x2 (возвращали 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.html app.js?v=4app.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).