From c8aab19958f0978bc2a63cb34a1041a130c692b3 Mon Sep 17 00:00:00 2001 From: Slava Date: Tue, 9 Jun 2026 23:45:51 +0300 Subject: [PATCH 1/7] docs: init ORCH-095 business request --- docs/work-items/ORCH-095/00-business-request.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 docs/work-items/ORCH-095/00-business-request.md diff --git a/docs/work-items/ORCH-095/00-business-request.md b/docs/work-items/ORCH-095/00-business-request.md new file mode 100644 index 0000000..9677cff --- /dev/null +++ b/docs/work-items/ORCH-095/00-business-request.md @@ -0,0 +1,7 @@ +# Business Request: BUG: карточка трекера застывает — HTML-инъекция «<1м» в render_task_tracker (parse_mode=HTML) + +Work Item ID: ORCH-095 + +## Description + +TBD -- 2.49.1 From d528f77b03e6877c9252981f026610bf0ab12fcf Mon Sep 17 00:00:00 2001 From: claude-bot Date: Tue, 9 Jun 2026 23:50:15 +0300 Subject: [PATCH 2/7] analyst(ET): auto-commit from analyst run_id=525 --- docs/work-items/ORCH-095/01-brd.md | 154 ++++++++++++++++++ docs/work-items/ORCH-095/02-trz.md | 132 +++++++++++++++ .../ORCH-095/03-acceptance-criteria.md | 97 +++++++++++ docs/work-items/ORCH-095/04-test-plan.yaml | 95 +++++++++++ 4 files changed, 478 insertions(+) create mode 100644 docs/work-items/ORCH-095/01-brd.md create mode 100644 docs/work-items/ORCH-095/02-trz.md create mode 100644 docs/work-items/ORCH-095/03-acceptance-criteria.md create mode 100644 docs/work-items/ORCH-095/04-test-plan.yaml diff --git a/docs/work-items/ORCH-095/01-brd.md b/docs/work-items/ORCH-095/01-brd.md new file mode 100644 index 0000000..352bccd --- /dev/null +++ b/docs/work-items/ORCH-095/01-brd.md @@ -0,0 +1,154 @@ +--- +work_item: ORCH-095 +stage: analysis +author_agent: analyst +status: ready-for-review +created_at: 2026-06-09 +model_used: claude-opus-4-8 +--- + +# 01 — BRD (бизнес-требования): ORCH-095 — HTML-инъекция «<1м» в render_task_tracker застывает live-карточку + +Work Item: **ORCH-095** · Repo: **orchestrator** · Стадия: analysis + +## 1. Бизнес-контекст и проблема + +Live-трекер задачи (`src/notifications.py::render_task_tracker`) — **основной канал +видимости конвейера для оператора**. Слава узнаёт состояние каждой задачи по её единственной +карточке в Telegram (инвариант «одна карточка на задачу», ORCH-042/067/087). Если карточка +перестаёт обновляться — оператор слепнет: задача реально идёт/завершилась, а карточка врёт. + +**Установленный факт (воспроизведён детерминированно 09.06, сырой ответ Telegram).** +Прямой вызов `editMessageText` для застрявшей карточки ORCH-093 (`message_id 18854`) вернул: + +``` +400 Bad Request: can't parse entities: Unsupported start tag "1м" at byte offset 500 +``` + +В тексте карточки на позиции ~379 присутствует подстрока `<1м · …` — длительность стадии +«меньше одной минуты», которую `_fmt_minutes` (`src/notifications.py:288-289`) рендерит как +литерал **`<1м`**. Карточка отправляется с `parse_mode=HTML` (`editMessageText`, +`notifications.py:175`). Telegram трактует `<1м` как **открывающий HTML-тег** → парсинг падает +с `400` → `edit_telegram` возвращает `EDIT_FAILED` → `update_task_tracker` по ветке +`EDIT_FAILED` (`notifications.py:733-739`) делает `return`, **не** отправляя новую карточку +(защита от дублей, ORCH-087) → карточка **застывает** на стейте, где `<1м` впервые попал в текст. + +**Цепочка отказа** (по коду): +`_fmt_minutes(<60s) → "<1м"` → интерполируется в HTML без экранирования → `editMessageText` +`400 can't parse entities` → `edit_telegram → EDIT_FAILED` → `update_task_tracker` ранний +`return` → карточка не обновляется до конца жизни задачи. + +**Почему проявляется не на каждой задаче.** Баг ловится **только** когда хотя бы одна +длительность стадии < 1 мин (`seconds < 60`) и эта строка попадает в текст, который затем +редактируется. Карточки ORCH-090/091 редактировались успешно (на момент `edit` в их тексте +`<1м` не было); ORCH-093 — упала. Это объясняет «плавающую» природу симптома. + +**Корневой класс дефекта — шире одного `<1м`.** Текст карточки собирается с `parse_mode=HTML` +из смеси (а) намеренной разметки-обёртки (`` номер задачи, ``) и (б) подставляемых +**данных**. Намеренная разметка экранироваться **не должна**; данные — должны. Сейчас +экранирован только заголовок (`esc_title`, `notifications.py:428`) и href/label внутри +`plane_issue_link`. Прочие данные — длительности (`_fmt_minutes`), метрики токенов/стоимости +(`fmt_tokens`/`fmt_cost`), имя модели (`short_model_name`), статус-лейбл +(`_card_status_label`) — вставляются **без** `html.escape`. `<1м` — первый сработавший +экземпляр этого класса; задача закрывает класс, а не единичный символ. + +## 2. Объём (scope) + +### В объёме +- Устранить HTML-инъекцию в `render_task_tracker`: любые **данные**, попадающие в текст + карточки с `parse_mode=HTML`, не должны ломать парсер Telegram (`< > &` в данных + безопасны). +- Привести формат «длительность < 1 мин» к HTML-безопасному виду (экранированный `<1м` + ИЛИ переформулировка `<1м` → `~0м` / `< 1 мин` с экранированием). +- Сохранить работоспособность **намеренной** разметки карточки (`` номер задачи, + жирный/прочее форматирование) — экранируются только данные, не обёртка. +- Восстановить обновления уже застрявших карточек (после фикса карточка возобновляет + обновления или переотправляется свежей). +- Юнит-покрытие HTML-безопасности всех динамических полей; зелёный регресс `pytest tests/ -q`; + запись в `CHANGELOG.md`. + +### Вне объёма +- Изменение `STAGE_TRANSITIONS`, `QG_CHECKS`, `check_*`, схемы БД — **не трогаются** (баг + чисто в слое рендера уведомлений). +- Изменение режима трекера (`bump`/`edit`), логики леджера сирот (ORCH-087), статусной модели + ORCH-066, транспортных примитивов (`send_telegram`/`edit_telegram`/`delete_telegram`) — + кроме точечной HTML-безопасности самого текста. +- Редизайн раскладки/состава карточки, новые метрики, перевод строк. +- Изменение машинных вердиктов / frontmatter-контракта. + +## 3. Заинтересованные стороны + +- **Заказчик / репортёр:** Слава (оператор) — обнаружил баг 09.06 (карточка ORCH-093 застряла, + «по 91 уже нету»). +- **Затронуты:** все наблюдатели Telegram-трекера по **всем** проектам (self-hosting: общий + прод-инстанс обслуживает и enduro-trails — карточки их задач так же уязвимы при стадии < 1 мин). +- **Принимает результат:** reviewer/tester конвейера ORCH; финальная приёмка — оператор + (карточки снова обновляются в реальном времени). + +## 4. Бизнес-требования (BR) + +- **BR-1** — Карточка трекера, в тексте которой есть стадия длительностью < 1 мин, должна + успешно редактироваться (`editMessageText` → `200`, не `400 can't parse entities`). Источник + отказа — литерал `<1м` от `_fmt_minutes` — устранён. (⇒ G1, G2) +- **BR-2** — **Все** динамические значения, вставляемые в текст карточки с `parse_mode=HTML` + (длительности, метрики токенов/стоимости, имя модели/эффорта, имена/лейблы стадий, + статус-лейбл, заголовок задачи), HTML-безопасны: символы `< > &` в **данных** не + интерпретируются Telegram как разметка. (⇒ G1) +- **BR-3** — Длительность «меньше минуты» рендерится так, чтобы не выглядеть открывающим + HTML-тегом: экранированный `<1м` **ИЛИ** переформулировка (`~0м` / `< 1 мин`) с + экранированием. Видимое оператору значение остаётся осмысленным («меньше минуты»). (⇒ G2) +- **BR-4** — **Регресс намеренной разметки:** кликабельный номер задачи (``, + `plane_issue_link`) и любое форматирование-обёртка (`` и т.п.) продолжают рендериться и + оставаться кликабельными/валидными — экранируются только подставляемые данные, не разметка. (⇒ G3) +- **BR-5** — Уже застрявшая карточка (класс ORCH-093) после деплоя фикса **возобновляет + обновления**: либо успешный `editMessageText` на следующем переходе стадии, либо + переотправка свежей карточки. Конкретный механизм восстановления (текст снова валиден → + edit проходит, ИЛИ классификация `can't parse entities` как пересоздаваемой) — решение + архитектора; бизнес-требование — карточка перестаёт быть «замёрзшей сиротой». (⇒ G... / AC-4) + +## 5. Нефункциональные требования (NFR) + +- **NFR-1 (never-raise):** `render_task_tracker` и весь путь уведомлений сохраняют контракт + «никогда не роняют конвейер» — любая ошибка рендера/экранирования деградирует к + fallback-строке, не исключение. +- **NFR-2 (нулевая регрессия разметки):** существующие зелёные тесты трекера + (`test_telegram_tracker.py`, `test_tracker_*`, `test_notifications_orphans.py`, + `test_notify_issue_links.py`) остаются зелёными; кликабельность номера и формат строк не + деградируют визуально (кроме намеренной смены вида «<1м»). +- **NFR-3 (self-hosting):** фикс — изменение **только** слоя рендера уведомлений; прод-контейнер + `orchestrator` не перезапускается в рамках стадий разработки; обязательна страховка + `deploy-staging` перед прод-деплоем. Машина стадий/гейты/схема БД не затрагиваются. +- **NFR-4 (совместимость):** изменение обратносовместимо по данным/схеме; не требует миграций; + применяется к новым рендерам сразу после деплоя. + +## 6. Допущения и ограничения + +- Карточка всегда отправляется с `parse_mode=HTML` (`send_telegram:58`, `edit_telegram:175`) — + это инвариант (ссылки/жирный требуют HTML); переход на `parse_mode=None`/MarkdownV2 **не** + рассматривается (сломает намеренную разметку, шире объёма). +- `fmt_tokens`/`fmt_cost` сейчас выдают только цифры/`.`/`k`/`M`/`$` (HTML-безопасно), но + требование BR-2 покрывает их **defence-in-depth** на случай будущих изменений формата. +- Telegram-лимит 48ч: карточки старше 48ч физически неудаляемы/неперезаписываемы — для них + восстановление недостижимо (known-limitation, унаследовано от ORCH-087); BR-5 относится к + карточкам в пределах окна. +- Источник `<1м` — `_fmt_minutes` (единственная функция, эмитящая литерал `<`); прочие данные + лишь потенциально опасны. Точка(и) внесения экранирования — решение архитектора (централизовать + в `_fmt_minutes`/на точке рендера/обёрткой-хелпером). + +## 7. Критерии успеха + +Карточка задачи со стадией < 1 мин успешно редактируется (нет `400 can't parse entities`); +все динамические поля HTML-безопасны; намеренная разметка (ссылка-номер, форматирование) +рендерится и кликабельна; застрявшие карточки возобновляют обновления; `never-raise` сохранён; +`pytest tests/ -q` зелёный; `CHANGELOG.md` обновлён. Детальные PASS/FAIL — `03-acceptance-criteria.md`. + +## 8. Риски + +- **Двойное экранирование** уже экранированных полей (`esc_title`, href/label в + `plane_issue_link`) → `&lt;` в выводе. Митигировать на стадии архитектуры (экранировать + ровно один раз на источник данных). +- **Случайное экранирование разметки-обёртки** (``, ``) → ссылки/жирный перестают + работать (регресс BR-4). Чёткая граница «данные vs обёртка». +- Изменение вида «<1м» меняет визуал карточки — согласовать формулировку с оператором (BR-3 + допускает оба варианта). +- Детали/перечень — `10-tech-risks.md` (заполняет архитектор). diff --git a/docs/work-items/ORCH-095/02-trz.md b/docs/work-items/ORCH-095/02-trz.md new file mode 100644 index 0000000..eb0a5e7 --- /dev/null +++ b/docs/work-items/ORCH-095/02-trz.md @@ -0,0 +1,132 @@ +--- +work_item: ORCH-095 +stage: analysis +author_agent: analyst +status: ready-for-review +created_at: 2026-06-09 +model_used: claude-opus-4-8 +--- + +# 02 — ТЗ (TRZ): ORCH-095 — HTML-безопасность динамических полей render_task_tracker + +Work Item: **ORCH-095** · Repo: **orchestrator** · Стадия: analysis + +> ТЗ описывает **конкретные изменения к реализации**, выведенные из BRD и фактического кода. +> Архитектурное обоснование/выбор точки внесения экранирования — задача архитектора (06-adr). + +## 1. Сводка изменения + +Текст live-карточки (`render_task_tracker`) собирается с `parse_mode=HTML` из намеренной +разметки-обёртки (`` номер задачи, форматирование) и подставляемых **данных**. Сейчас +экранирован только заголовок (`esc_title`) и href/label внутри `plane_issue_link`; остальные +данные вставляются сырыми. Литерал `<1м` (длительность < 1 мин), возвращаемый `_fmt_minutes`, +Telegram парсит как открывающий тег → `editMessageText` падает `400 can't parse entities` → +`edit_telegram → EDIT_FAILED` → `update_task_tracker` делает ранний `return` → карточка +застывает. + +Требуется: (а) сделать формат «< 1 мин» HTML-безопасным; (б) гарантировать HTML-безопасность +**всех** данных, попадающих в текст карточки, **не** экранируя намеренную разметку-обёртку; +(в) обеспечить возобновление обновлений ранее застрявших карточек. Изменение локализовано в +слое уведомлений; машина стадий/гейты/схема БД не затрагиваются. + +## 2. Задействованные модули / пути + +| Путь | Действие | +|------|----------| +| `src/notifications.py` | **изменить** — `_fmt_minutes` (~280) и/или точки рендера в `render_task_tracker` (~355): HTML-безопасность данных | +| `src/notifications.py::render_task_tracker` | **изменить** — экранировать данные: длительности (`dur`), `status_label`, `model`/`effort`, метрики (defence-in-depth); НЕ трогать `num_html`, `_done_link`-разметку | +| `src/notifications.py::_card_status_label` (~1173) | **проверить/экранировать на потребителе** — статус-лейбл вставляется в `status_line` сырым | +| `src/notifications.py::edit_telegram` (~157) | **возможно изменить** (на усмотрение архитектора) — классификация `can't parse entities` для восстановления застрявших карточек (BR-5/AC-4) | +| `src/notifications.py::update_task_tracker` (~650) | **возможно затронуть** — ветка `EDIT_FAILED` vs пересоздание при перманентном parse-фейле (BR-5/AC-4) | +| `tests/test_telegram_tracker.py` (или новый `tests/test_tracker_html_escape.py`) | **создать/дополнить** — юнит HTML-безопасности всех динамических полей | +| `CHANGELOG.md` | **изменить** — запись о фиксе | + +> Примечание: `fmt_tokens`/`fmt_cost`/`short_model_name` живут в `src/usage.py`; их выход +> сейчас HTML-безопасен (цифры/`.`/`k`/`M`/`$`/имя модели). Менять `src/usage.py` **не +> требуется** — defence-in-depth экранирование делается на потребителе в `notifications.py`. + +## 3. Функциональные требования + +### FR-1 — HTML-безопасный формат «меньше минуты» (⇒ BR-1, BR-3) +Длительность стадии < 60 с не должна порождать подстроку, которую Telegram трактует как +открывающий тег. Текущий `_fmt_minutes(seconds)` при `0 < seconds < 60` возвращает литерал +`"<1м"` (`notifications.py:288-289`). Поведение должно стать одним из (выбор — архитектор): +- экранированный вывод `<1м` (видится оператору как `<1м`), **либо** +- переформулировка `~0м` / `< 1 мин` с последующим экранированием. +Инвариант: для **любого** входа `_fmt_minutes` (включая `0м`, `Nм`, `~Nм` от +`_capped_review_str`) результат, попав в `parse_mode=HTML`, не ломает парсер. `_fmt_minutes` +сохраняет never-raise (нечисловой/None вход → `0м`). + +### FR-2 — HTML-безопасность всех данных карточки (⇒ BR-2) +Каждое **подставляемое значение-данные**, попадающее в текст `render_task_tracker`, +экранируется `html.escape(...)` ровно один раз перед вставкой в HTML-текст. Перечень полей-данных: + +| Поле | Источник | Текущий статус | +|------|----------|----------------| +| Заголовок задачи | `title` → `esc_title` | уже экранирован ✓ (не дублировать) | +| Длительности стадий / BRD / done | `_fmt_minutes`, `_capped_review_str` | **дыра** (FR-1) | +| Статус-лейбл карточки | `_card_status_label` → `status_label` | **дыра** — экранировать | +| Имя модели | `short_model_name(last["model"])` | экранировать (defence-in-depth) | +| Эффорт | `_run_effort(last)` | экранировать (defence-in-depth) | +| Токены / стоимость | `fmt_tokens`/`fmt_cost` | HTML-безопасны; экранировать defence-in-depth | +| Метка «попытка N» / лейблы стадий | статические константы `_TRACKER_STAGES`/`_BRD_LABEL` | статичны; не требуют, но безопасно | + +Инвариант FR-2: после рендера **ни один** символ `< > &`, пришедший из данных, не остаётся +неэкранированным в выходном тексте. + +### FR-3 — Сохранность намеренной разметки-обёртки (⇒ BR-4) +Намеренные HTML-фрагменты **не** экранируются: +- `num_html` = `plane_issue_link(...)` — кликабельный `` номер задачи (внутри уже + экранированы href через `html.escape(url, quote=True)` и label); +- `link_for(...)` в строке «⏳ ждёт …» — намеренные ссылки; +- `_done_link(...)` — строка `🔗 PR #n · 📦 Внедрено`. +После фикса эти фрагменты рендерятся как валидный HTML и остаются кликабельными. Запрещено +двойное экранирование уже экранированных полей (`esc_title`, внутренности `plane_issue_link`). + +### FR-4 — Возобновление обновлений застрявших карточек (⇒ BR-5) +После деплоя фикса карточка, ранее застрявшая на `400 can't parse entities`, должна +возобновить обновления. Достаточное условие по умолчанию: текст следующего рендера больше не +содержит небезопасной подстроки → `editMessageText` проходит (`200`) на ближайшем переходе +стадии. Опционально (решение архитектора): классифицировать перманентный parse-фейл в +`edit_telegram`/`update_task_tracker` как повод **переотправить** свежую карточку вместо +тихого `return` по `EDIT_FAILED` — но **без** регресса защиты от дублей (ORCH-087: транзиентные +фейлы по-прежнему НЕ плодят карточки). Если выбирается переклассификация — она должна отличать +перманентный `can't parse entities` от транзиентного (network/timeout/5xx). + +### FR-5 — never-raise (⇒ NFR-1) +Все изменённые функции сохраняют контракт «никогда не роняют конвейер»: ошибка +экранирования/рендера → деградация к существующему fallback (`f"task-{task_id}"` / +пропуск строки), не исключение наружу. + +## 4. Изменения API + +Нет. HTTP-эндпоинты не добавляются/не меняются. (Внешний вызов — только исходящий +`editMessageText`/`sendMessage` к Telegram Bot API; контракт вызова не меняется, меняется +лишь безопасность `text`.) + +## 5. Изменения схемы БД + +Нет. Таблицы `tasks`/`agent_runs`/`tracker_messages` не затрагиваются; миграций нет. + +## 6. Требования к новым/изменённым QG checks + +Нет. `QG_CHECKS` / `check_*` / `STAGE_TRANSITIONS` / машинные вердикты не затрагиваются. Баг — +в слое рендера уведомлений, вне Quality Gate. + +## 7. Совместимость / регресс + +- **Обратная совместимость:** изменение чисто в формировании строки текста карточки; данные + БД, схема, режимы трекера (`bump`/`edit`), леджер сирот (ORCH-087), статусная модель + (ORCH-066) — без изменений. +- **Область раската:** все проекты на общем прод-инстансе (self-hosting) — фикс применяется к + каждому новому рендеру сразу после деплоя; не требует миграции/бэкфилла. +- **Kill-switch:** не требуется (исправление дефекта корректности, а не новая фича-ветка). Если + архитектор выбирает переклассификацию parse-фейла в `update_task_tracker` (FR-4 опц.) — + оценить целесообразность флага; по умолчанию изменение поведения минимально и безопасно. +- **Обратимость:** изменение откатывается обычным revert PR (только `notifications.py` + + тесты + CHANGELOG); прод-контейнер не требует ручных операций над данными. +- **Артефакты pipeline:** обновляются `12-review.md` (reviewer), `13-test-report.md` (tester), + `06-adr/ADR-001-*.md` (архитектор — выбор точки экранирования и стратегии FR-4), + `CHANGELOG.md`. Машинные вердикты гейтов — без изменений. +- **Self-hosting:** обязательна стадия `deploy-staging` (8501) перед прод-деплоем; прод + `orchestrator` не рестартуется в рамках разработки. diff --git a/docs/work-items/ORCH-095/03-acceptance-criteria.md b/docs/work-items/ORCH-095/03-acceptance-criteria.md new file mode 100644 index 0000000..a4f0049 --- /dev/null +++ b/docs/work-items/ORCH-095/03-acceptance-criteria.md @@ -0,0 +1,97 @@ +--- +work_item: ORCH-095 +stage: analysis +author_agent: analyst +status: ready-for-review +created_at: 2026-06-09 +model_used: claude-opus-4-8 +--- + +# 03 — Критерии приёмки (Acceptance Criteria): ORCH-095 — HTML-инъекция «<1м» в render_task_tracker + +Work Item: **ORCH-095** · Repo: **orchestrator** · Стадия: analysis + +Формат: каждый критерий имеет **PASS** (что должно быть истинно для приёмки) и **FAIL** +(что считается провалом). Любой машинный/ручной reviewer проверяет их буквально по файлам +репозитория. + +--- + +## AC-1 — Стадия < 1 мин не ломает парсер Telegram + +**Условие:** `render_task_tracker` для задачи, у которой хотя бы одна стадия длилась < 60 с, +выдаёт текст, безопасный для `parse_mode=HTML` (нет неэкранированного `<` в данных длительности). +- **PASS:** В выходном тексте подстрока длительности «меньше минуты» представлена как `<1м` + (или переформулированный безопасный вид `~0м` / `< 1 мин` без сырого `<`); `editMessageText` + с этим текстом не вернул бы `400 can't parse entities: Unsupported start tag "1м"`. Юнит-тест + на `_fmt_minutes(30)` / `render_task_tracker(...)` подтверждает отсутствие сырого `<` от + длительности. +- **FAIL:** Текст содержит сырой `<1м` (или иной литерал `<`+нецифра) из данных длительности; + тест на парсинг/наличие сырого `<` падает. + +--- + +## AC-2 — Все динамические поля карточки HTML-безопасны (юнит) + +**Условие:** Существует юнит-тест, проверяющий, что каждое подставляемое **данные-поле** +`render_task_tracker` экранировано: длительность, токены, стоимость (`$`), заголовок с +спецсимволами `< > &`, статус-лейбл, имя модели/эффорт. +- **PASS:** Тест рендерит карточку с заголовком, содержащим `<`, `>`, `&` (напр. + `"A x & <1"`), и стадией < 1 мин; ассертит, что эти спецсимволы из ДАННЫХ + присутствуют в выводе только в экранированном виде (`<`/`>`/`&`) и НЕ как + сырые теги; одновременно нет двойного экранирования (`&lt;`). +- **FAIL:** Тест отсутствует, либо любое из перечисленных данных-полей попадает в текст без + экранирования, либо обнаруживается двойное экранирование. + +--- + +## AC-3 — Регресс намеренной разметки (ссылка-номер, форматирование) + +**Условие:** После фикса намеренная HTML-разметка карточки продолжает рендериться валидной и +кликабельной. +- **PASS:** Кликабельный номер задачи (`ORCH-095` от `plane_issue_link`) + присутствует в выводе как валидный незаэкранированный ``-тег; строки `🔗 PR #n`/`📦` + (`_done_link`) и любое форматирование-обёртка рендерятся; существующие тесты + `test_tracker_issue_link.py`/`test_notify_issue_links.py`/`test_telegram_tracker.py` + зелёные. Двойного экранирования href/label нет. +- **FAIL:** Номер задачи перестал быть кликабельным (`` заэкранирован в `<a>`), либо + любой регресс-тест разметки красный. + +--- + +## AC-4 — Застрявшая карточка возобновляет обновления + +**Условие:** Карточка, ранее застрявшая на `400 can't parse entities` (класс ORCH-093), после +фикса снова обновляется. +- **PASS:** На следующем переходе стадии текст рендера больше не содержит небезопасной + подстроки → `editMessageText` проходит (`200`); ИЛИ (если выбрана стратегия FR-4-опц.) + перманентный parse-фейл классифицируется как повод переотправить свежую карточку, и + `update_task_tracker` отправляет новую. Поведение покрыто тестом (рендер валиден → edit-путь + не возвращает `EDIT_FAILED` из-за parse-ошибки). +- **FAIL:** После фикса карточка с прежним содержимым по-прежнему даёт `EDIT_FAILED` и не + обновляется/не переотправляется; либо защита от дублей (ORCH-087) сломана — транзиентный + фейл теперь плодит дубликаты карточек. + +--- + +## AC-5 — never-raise, зелёный регресс, CHANGELOG + +**Условие:** Контракт надёжности и гигиена изменения сохранены. +- **PASS:** `render_task_tracker`/`update_task_tracker`/`edit_telegram` не выбрасывают + исключение наружу при любом входе (включая «битый» заголовок/None); `pytest tests/ -q` + полностью зелёный; в `CHANGELOG.md` есть запись о фиксе ORCH-095; `STAGE_TRANSITIONS`/ + `QG_CHECKS`/`check_*`/схема БД не изменены (diff их не трогает). +- **FAIL:** Любой тест в `tests/` красный; обнаружено непойманное исключение в пути рендера; + тронуты машина стадий/гейты/схема БД; нет записи в `CHANGELOG.md`. + +--- + +## Сводная матрица AC ↔ FR/BR + +| AC | Покрывает | +|----|-----------| +| AC-1 | BR-1, BR-3 / FR-1 | +| AC-2 | BR-2 / FR-2 | +| AC-3 | BR-4 / FR-3 | +| AC-4 | BR-5 / FR-4 | +| AC-5 | NFR-1, NFR-2 / FR-5 | diff --git a/docs/work-items/ORCH-095/04-test-plan.yaml b/docs/work-items/ORCH-095/04-test-plan.yaml new file mode 100644 index 0000000..ffb9a6e --- /dev/null +++ b/docs/work-items/ORCH-095/04-test-plan.yaml @@ -0,0 +1,95 @@ +work_item: ORCH-095 +stage: analysis +author_agent: analyst +status: ready-for-review +created_at: 2026-06-09 +model_used: claude-opus-4-8 +title: "HTML-безопасность динамических полей render_task_tracker (фикс инъекции «<1м»)" +framework: pytest +scope: > + Покрывается: HTML-безопасность всех подставляемых данных в render_task_tracker + (длительности < 1 мин, токены/стоимость, имя модели/эффорт, статус-лейбл, заголовок со + спецсимволами), сохранность намеренной разметки ( номер задачи, _done_link), + возобновление обновлений застрявшей карточки, never-raise. Вне покрытия: реальная сеть к + Telegram Bot API (мокируется httpx), изменения STAGE_TRANSITIONS/QG_CHECKS/схемы БД (не + трогаются). +notes: > + Тесты — изоляция от сети: httpx.post/get мокируются; БД — временная SQLite-фикстура с + задачей и agent_runs (стадия < 60 с). Полный регресс pytest tests/ -q должен оставаться + зелёным, включая существующие test_telegram_tracker.py / test_tracker_*.py / + test_notifications_orphans.py / test_notify_issue_links.py. Регрессом считается: красный + любой существующий тест трекера, заэкранированная намеренная разметка, двойное + экранирование, непойманное исключение в пути рендера. + +tests: + - id: TC-01 + type: unit + description: "_fmt_minutes для длительности < 60 с (напр. 30) не возвращает сырой '<1м': результат HTML-безопасен (<1м либо переформулированный '~0м'/'< 1 мин' без сырого '<')." + module: tests/test_tracker_html_escape.py + expected: PASS + + - id: TC-02 + type: unit + description: "_fmt_minutes для граничных входов (0, None, нечисловое, ровно 60, большое значение) — never-raise и HTML-безопасный вывод во всех ветках." + module: tests/test_tracker_html_escape.py + expected: PASS + + - id: TC-03 + type: unit + description: "render_task_tracker для задачи со стадией < 1 мин: в выходном тексте нет неэкранированного '<' из данных длительности; подстрока длительности безопасна для parse_mode=HTML." + module: tests/test_tracker_html_escape.py + expected: PASS + + - id: TC-04 + type: unit + description: "render_task_tracker с заголовком, содержащим спецсимволы '<', '>', '&' (напр. 'A x & <1'): спецсимволы данных присутствуют только экранированными (</>/&), не как сырые теги; двойного экранирования (&lt;) нет." + module: tests/test_tracker_html_escape.py + expected: PASS + + - id: TC-05 + type: unit + description: "Статус-лейбл (_card_status_label) и имя модели/эффорт, попадающие в текст карточки, экранированы (defence-in-depth): спецсимволы в них не ломают HTML." + module: tests/test_tracker_html_escape.py + expected: PASS + + - id: TC-06 + type: unit + description: "Метрики токенов/стоимости (fmt_tokens/fmt_cost) в карточке HTML-безопасны: '$' и числовой формат не порождают сырых тегов." + module: tests/test_tracker_html_escape.py + expected: PASS + + - id: TC-07 + type: unit + description: "Регресс намеренной разметки: кликабельный номер задачи (plane_issue_link -> ) присутствует в выводе как валидный незаэкранированный -тег; href/label не задвоены экранированием." + module: tests/test_tracker_html_escape.py + expected: PASS + + - id: TC-08 + type: unit + description: "Регресс _done_link: для завершённой задачи строка '🔗 PR #n · 📦 Внедрено' рендерится валидной (ссылочная разметка не экранирована)." + module: tests/test_tracker_html_escape.py + expected: PASS + + - id: TC-09 + type: integration + description: "update_task_tracker (edit-режим) с замоканным editMessageText: текст карточки со стадией < 1 мин принимается (мок ассертит отсутствие 'can't parse entities'-триггера, т.е. нет сырого '<1м' в payload text)." + module: tests/test_tracker_html_escape.py + expected: PASS + + - id: TC-10 + type: integration + description: "Возобновление застрявшей карточки (AC-4): после фикса валидный рендер проходит edit-путь без EDIT_FAILED из-за parse-ошибки; защита от дублей сохранена — транзиентный (network) фейл по-прежнему НЕ плодит новую карточку." + module: tests/test_tracker_html_escape.py + expected: PASS + + - id: TC-11 + type: unit + description: "never-raise: render_task_tracker на 'битых' входах (отсутствует задача, None-заголовок, нечисловые длительности) возвращает fallback-строку, не выбрасывает исключение." + module: tests/test_tracker_html_escape.py + expected: PASS + + - id: TC-12 + type: integration + description: "Полный регресс существующих тестов трекера (test_telegram_tracker.py, test_tracker_issue_link.py, test_tracker_status_line.py, test_notifications_orphans.py, test_notify_issue_links.py) остаётся зелёным после фикса." + module: tests/test_telegram_tracker.py + expected: PASS -- 2.49.1 From 6b14b07f40fd0543cc17f71b08baf15ea5e25ce8 Mon Sep 17 00:00:00 2001 From: claude-bot Date: Tue, 9 Jun 2026 23:56:33 +0300 Subject: [PATCH 3/7] architect(ET): auto-commit from architect run_id=526 --- docs/architecture/README.md | 2 +- docs/architecture/internals.md | 2 + .../ADR-001-html-safe-card-data-render.md | 209 ++++++++++++++++++ docs/work-items/ORCH-095/10-tech-risks.md | 37 ++++ 4 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 docs/work-items/ORCH-095/06-adr/ADR-001-html-safe-card-data-render.md create mode 100644 docs/work-items/ORCH-095/10-tech-risks.md diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 08744a5..6a16160 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -15,7 +15,7 @@ - **Reconciler** (`src/reconciler.py`, ORCH-053 — реализовано, [adr-0007](adr/adr-0007-reconciler.md)) — фоновый daemon-поток (паттерн `queue_worker`), стартует/останавливается в `main.lifespan` (после `worker.start()` / перед `worker.stop()`). Реконсилирует рассинхрон «источник истины ≠ стадия задачи» при потерянном webhook. F-1 gate-side (продвигает застрявшую стадию по локальной БД через штатный `advance_stage(..., finished_agent=None)`), F-2 plane-side (опрос Plane API → `handle_*` из `plane.py`), F-3 (БД-fallback `sha→branch` в `handle_ci_status`). Источник истины — гейт/Plane, не событие; идемпотентность (active-job guard + atomic-claim + grace); kill-switch `ORCH_RECONCILE_ENABLED`. `analysis` F-1 не трогает (человеческий гейт). F-1 также пропускает escalated (retry≥лимита) и Blocked/Needs-Input задачи (ORCH-060). Наблюдаемость — блок `reconcile` в `GET /queue`. - **Disk-watchdog** (`src/disk_watchdog.py`, ORCH-063 — [adr-0024](adr/adr-0024-disk-watchdog.md)) — фоновый daemon-поток (каркас `reconciler`/`job_reaper`), стартует/останавливается в `main.lifespan` (старт последним — после `reaper.start()`; стоп первым в reverse-порядке; гард `disk_monitor_enabled`). Каждые `disk_monitor_interval_s` (дефолт 300с) меряет заполнение **хост-ФС** по смонтированным bind-путям (`/repos`, `/app/data`) через stdlib `shutil.disk_usage` (не overlay `/` контейнера, не субпроцесс `df`; дедуп путей по `st_dev`). Решение об алерте — pure-функция `decide_action(used_pct, threshold, prev_state, now, realert_s)`: алерт на пересечении порога (дефолт **85%**), cooldown-повтор `disk_monitor_realert_s` (анти-спам, не на каждом тике), однократный recovery при возврате ниже порога. Алерт — `send_telegram` (notifying, best-effort). Состояние анти-спама — in-memory (без миграции БД). never-raise (per-path/per-tick/per-send); только читает и уведомляет — не трогает диск/контейнер, не рестартит прод (self-hosting безопасность). Kill-switch `ORCH_DISK_MONITOR_ENABLED`; снимок — блок `disk_monitor` в `GET /queue` (`enabled`/`threshold_pct`/`interval_s`/`realert_s`/`paths`[`used_pct`/`free_gb`/`alerting`/`last_alert_at`]). `STAGE_TRANSITIONS`/`QG_CHECKS`/схема БД — не тронуты. Детали — `docs/work-items/ORCH-063/06-adr/ADR-001-disk-watchdog.md`. - **Build-cache-pruner** (`src/build_cache_pruner.py`, ORCH-062 — [adr-0025](adr/adr-0025-build-cache-pruner.md)) — фоновый daemon-поток (каркас `disk_watchdog`), стартует/останавливается в `main.lifespan` (старт последним — после `disk_watchdog.start()`; стоп первым в reverse; гард `build_cache_prune_enabled`). «Вторая половина» disk-watchdog: **watchdog сигналит — pruner убирает**. Каждые `build_cache_prune_interval_s` (дефолт 21600с = 6ч) выполняет **строго `docker builder prune -f --filter until=`** (BuildKit GC; дефолт `until=24h` — удаляет build cache старше суток, тёплый кэш сохраняет; `-a` опционально, только в паре с фильтром). Затрагивает **только** build cache — НЕ образы/контейнеры; рестарт docker daemon/прода не выполняется (self-hosting безопасность). В контейнере нет `docker` CLI (`Dockerfile:11`), поэтому уборка идёт **на хосте через ssh** каналом `deploy_ssh_user@deploy_ssh_host` (как `image_freshness`/`self_deploy`); пустой `deploy_ssh_host` → тик no-op (скоуп на self-host). never-raise (per-команда/per-tick); учёт результата in-memory (без миграции БД). Kill-switch `ORCH_BUILD_CACHE_PRUNE_ENABLED`; снимок — блок `build_cache_prune` в `GET /queue` (`enabled`/`interval_s`/`until`/`last_run_ts`/`last_reclaimed`/`last_error`). `STAGE_TRANSITIONS`/`QG_CHECKS`/схема БД — не тронуты. Детали — `docs/work-items/ORCH-062/06-adr/ADR-001-build-cache-pruner.md`. -- **Notifications / Live-tracker** (`src/notifications.py`, ORCH-042/ORCH-067) — ОДНА live-карточка на задачу (`update_task_tracker`), обновляется на каждом переходе. Режим `ORCH_TRACKER_MODE` (дефолт `bump` с ORCH-067: delete+silent send+repoint внизу чата; `edit` — правка на месте). Карточка несёт строку Plane-статуса `📍 …` (оффлайн-ядро `plane_status_label` + best-effort live-overlay `_live_plane_branch_override`, kill-switch `ORCH_TRACKER_LIVE_STATUS`) и кликабельный номер задачи (`plane_issue_link`/`link_for` → ссылка в Plane, fail-safe на сырой номер). **ORCH-080:** оба низкоуровневых примитива (`send_telegram`/`edit_telegram`) шлют payload с `disable_web_page_preview: True` — Telegram больше не разворачивает баннер link-preview Plane под карточкой/уведомлениями; `parse_mode: HTML` сохранён (ссылка остаётся кликабельной), безусловно без kill-switch. Все алерты, упоминающие `work_item_id`, делают номер кликабельным. **ORCH-087:** bump ведёт авторитетный леджер всех созданных карточек (`tracker_messages`, `deleted_at IS NULL` = жива) и на каждом обновлении зачищает ВСЕ незакрытые mid (а не только скаляр `tracker_message_id`) → класс «замёрзшая сирота» устранён; строка стадии несёт фактический эффорт рядом с моделью (`· {model} · {effort}`, колонка `agent_runs.effort`, стамп в `launcher._spawn`); done-строка времени переписана на три подписанных метрики `⏱️ Агенты · твоё{~cap} · общее с ожиданием` (кап `ORCH_TRACKER_BRD_REVIEW_CAP_S`); deploy-цикл дополнен overlay-ключом `confirm_deploy`. **ORCH-091 (индикация-only):** три корректности рендера — (1) `_STAGE_STATUS_LABEL` покрывает ВСЕ ключи `STAGE_TRANSITIONS` (добавлены `deploy-staging`→«Deploying (staging)», `cancelled`→«Cancelled»; полнота гарантируется тестом по `stages.STAGE_TRANSITIONS`, не статичным списком — NFR-3), runtime-фолбэк для неизвестной стадии стал нейтральным (капитализированное имя) вместо «To Analyse»; (2) при откате конвейера `✅`-строки стадий ПОЗЖЕ текущей позиции (позиция — из порядка `STAGE_TRANSITIONS`, с нормализацией `deploy-staging→deploy` только в гейте подавления; `is_active_stage` не тронут) больше не рисуются; (3) строка стадии суммирует ВСЕ `agent_runs` агента (Σ cost/токены/время теми же формулами, что блок тоталов) → строгая сходимость с `SUM(agent_runs)`. Только `src/notifications.py` + тесты; `STAGE_TRANSITIONS`/`QG_CHECKS`/схема БД/транспорт — не тронуты. Контракт всего компонента — never raises; карточка всегда silent. Детали — [internals.md](internals.md) §7, [ADR-087](../work-items/ORCH-087/06-adr/ADR-001-tracker-orphan-cleanup.md) и [ORCH-091 ADR-001](../work-items/ORCH-091/06-adr/ADR-001-tracker-status-rollback-metrics.md). +- **Notifications / Live-tracker** (`src/notifications.py`, ORCH-042/ORCH-067) — ОДНА live-карточка на задачу (`update_task_tracker`), обновляется на каждом переходе. Режим `ORCH_TRACKER_MODE` (дефолт `bump` с ORCH-067: delete+silent send+repoint внизу чата; `edit` — правка на месте). Карточка несёт строку Plane-статуса `📍 …` (оффлайн-ядро `plane_status_label` + best-effort live-overlay `_live_plane_branch_override`, kill-switch `ORCH_TRACKER_LIVE_STATUS`) и кликабельный номер задачи (`plane_issue_link`/`link_for` → ссылка в Plane, fail-safe на сырой номер). **ORCH-080:** оба низкоуровневых примитива (`send_telegram`/`edit_telegram`) шлют payload с `disable_web_page_preview: True` — Telegram больше не разворачивает баннер link-preview Plane под карточкой/уведомлениями; `parse_mode: HTML` сохранён (ссылка остаётся кликабельной), безусловно без kill-switch. Все алерты, упоминающие `work_item_id`, делают номер кликабельным. **ORCH-087:** bump ведёт авторитетный леджер всех созданных карточек (`tracker_messages`, `deleted_at IS NULL` = жива) и на каждом обновлении зачищает ВСЕ незакрытые mid (а не только скаляр `tracker_message_id`) → класс «замёрзшая сирота» устранён; строка стадии несёт фактический эффорт рядом с моделью (`· {model} · {effort}`, колонка `agent_runs.effort`, стамп в `launcher._spawn`); done-строка времени переписана на три подписанных метрики `⏱️ Агенты · твоё{~cap} · общее с ожиданием` (кап `ORCH_TRACKER_BRD_REVIEW_CAP_S`); deploy-цикл дополнен overlay-ключом `confirm_deploy`. **ORCH-091 (индикация-only):** три корректности рендера — (1) `_STAGE_STATUS_LABEL` покрывает ВСЕ ключи `STAGE_TRANSITIONS` (добавлены `deploy-staging`→«Deploying (staging)», `cancelled`→«Cancelled»; полнота гарантируется тестом по `stages.STAGE_TRANSITIONS`, не статичным списком — NFR-3), runtime-фолбэк для неизвестной стадии стал нейтральным (капитализированное имя) вместо «To Analyse»; (2) при откате конвейера `✅`-строки стадий ПОЗЖЕ текущей позиции (позиция — из порядка `STAGE_TRANSITIONS`, с нормализацией `deploy-staging→deploy` только в гейте подавления; `is_active_stage` не тронут) больше не рисуются; (3) строка стадии суммирует ВСЕ `agent_runs` агента (Σ cost/токены/время теми же формулами, что блок тоталов) → строгая сходимость с `SUM(agent_runs)`. Только `src/notifications.py` + тесты; `STAGE_TRANSITIONS`/`QG_CHECKS`/схема БД/транспорт — не тронуты. Контракт всего компонента — never raises; карточка всегда silent. **ORCH-095 (HTML-безопасность данных):** текст карточки шлётся с `parse_mode=HTML`; каждый **data**-слот (длительности `_fmt_minutes`/`_capped_review_str`, статус-лейбл, модель/эффорт, токены/стоимость) экранируется `html.escape` ровно один раз на границе рендера, **markup**-слоты (`num_html`/`link_for`/`_done_link`/`esc_title`) — нет (двойное экранирование запрещено). Устранён класс «неэкранированные данные в HTML» (литерал `<1м` от `_fmt_minutes` → Telegram `400 can't parse entities` → застывшая карточка, инцидент ORCH-093); `_fmt_minutes` по-прежнему даёт `<1м` (escape рендерит визуально идентично). Застрявшая карточка в окне авто-восстанавливается следующим рендером; `edit_telegram`/`update_task_tracker`/леджер сирот не тронуты. Детали — [internals.md](internals.md) §7, [ADR-087](../work-items/ORCH-087/06-adr/ADR-001-tracker-orphan-cleanup.md), [ORCH-091 ADR-001](../work-items/ORCH-091/06-adr/ADR-001-tracker-status-rollback-metrics.md) и [ORCH-095 ADR-001](../work-items/ORCH-095/06-adr/ADR-001-html-safe-card-data-render.md). - **Project Registry** (`src/projects.py`, ORCH-6) — Plane project id → repo + prefix; фильтрация вебхуков по проекту. - **Plane Sync** (`src/plane_sync.py`) — синхронизация статусов/комментариев в Plane. Резолв статусов проекта `get_project_states` (ORCH-10) кэширует `{logical_key→uuid}` per-project; **ORCH-068** добавляет в кэш-запись `{uuid→group}` (для терминал-исключения F-2) и **TTL** `ORCH_PLANE_STATES_TTL_S` (дефолт 300с; `0` → прежний lifetime-кэш) — устаревший набор статусов самозалечивается без рестарта процесса через существующий `reload_project_states()` (баг кэша после появления нового Plane-статуса). Форма возврата `get_project_states` неизменна (обратная совместимость). diff --git a/docs/architecture/internals.md b/docs/architecture/internals.md index ae850c5..d68e62e 100644 --- a/docs/architecture/internals.md +++ b/docs/architecture/internals.md @@ -142,6 +142,8 @@ claude.exe --print --system-prompt --allowedTools Read,Write,Edit,Bash **Кликабельный номер задачи (ORCH-067).** Номер в заголовке карточки И во всех уведомлениях орка, где упоминается `work_item_id`, — HTML-ссылка на issue в Plane через общий `plane_issue_link` / `link_for` (URL строит `_plane_issue_url` с loopback/workspace/project-гардами, переиспользуя резолв ORCH-017). Fail-safe: при нехватке любого из (web-base/не-loopback, workspace, project_id, plane_issue_id) → `html.escape(work_item_id)` без ``; динамические части экранируются, ``-разметка валидна под `parse_mode=HTML`. Алерты `stage_engine`/`launcher`/`security_gate`/`reconciler` переведены на `link_for` (резолвит `repo`+`plane_issue_id` из БД по `task_id` или `work_item_id`). +**HTML-безопасность данных карточки (ORCH-095).** Текст карточки шлётся с `parse_mode=HTML` и собирается из слотов двух категорий: **markup** (намеренная разметка — `num_html`/`plane_issue_link`, `link_for(...)`, `_done_link(...)`, уже-экранированный `esc_title`) и **data** (подставляемые значения — длительности `_fmt_minutes`/`_capped_review_str`, статус-лейбл `_card_status_label`, имя модели `short_model_name`, эффорт `_run_effort`, токены/стоимость `fmt_tokens`/`fmt_cost`). Инвариант: **каждый data-слот экранируется `html.escape` ровно один раз на границе рендера** (`render_task_tracker`/`_stage_line`); функции-источники остаются HTML-агностичными, markup-слоты не экранируются (двойное экранирование запрещено). Это устранило класс «неэкранированные данные в HTML-тексте»: до фикса `_fmt_minutes(<60s)` возвращал литерал `<1м`, который Telegram парсил как открывающий тег → `editMessageText` `400 can't parse entities` → `EDIT_FAILED` → ранний `return` (анти-дубль ORCH-087) → карточка застывала (инцидент ORCH-093). `_fmt_minutes` по-прежнему возвращает `<1м` — escape на границе (`<1м`) рендерит его визуально идентично; формат не меняется. Застрявшая (в окне) карточка авто-восстанавливается следующим безопасным рендером; `edit_telegram`/`update_task_tracker`/леджер сирот/режимы `bump`/`edit` не тронуты. Детали — [ORCH-095 ADR-001](../work-items/ORCH-095/06-adr/ADR-001-html-safe-card-data-render.md). + ## Database Schema ```sql diff --git a/docs/work-items/ORCH-095/06-adr/ADR-001-html-safe-card-data-render.md b/docs/work-items/ORCH-095/06-adr/ADR-001-html-safe-card-data-render.md new file mode 100644 index 0000000..96e727b --- /dev/null +++ b/docs/work-items/ORCH-095/06-adr/ADR-001-html-safe-card-data-render.md @@ -0,0 +1,209 @@ +--- +work_item: ORCH-095 +stage: architecture +author_agent: architect +status: accepted +created_at: 2026-06-09 +model_used: claude-opus-4-8 +--- + +# ADR-001: HTML-безопасный рендер данных live-карточки трекера (устранение инъекции «<1м») + +Work Item: **ORCH-095** — HTML-инъекция `<1м` в `render_task_tracker` застывает live-карточку +Стадия: **architecture** +Сквозная регистрация: **N/A — локальное решение задачи.** Изменение целиком в слое рендера +уведомлений (`src/notifications.py`); новой стадии/QG/компонента/смены БД нет, инварианты +`STAGE_TRANSITIONS`/`QG_CHECKS`/схемы не затрагиваются → глобальный `adr-NNNN` не заводится +(прецедент — ORCH-091, такой же indication-only фикс рендера, тоже без сквозного ADR). + +## Статус +Accepted + +## Контекст + +Live-карточка задачи (`src/notifications.py::render_task_tracker`) — основной канал видимости +конвейера для оператора, инвариант «одна карточка на задачу» (ORCH-042/067/087). Карточка +отправляется и редактируется с `parse_mode=HTML` (`send_telegram:58`, `edit_telegram:175`). + +**Сверено по коду.** `_fmt_minutes(seconds)` (`notifications.py:280-290`) при `0 < seconds < 60` +возвращает литерал `"<1м"`: + +```python +if seconds < 60: + return "<1м" +``` + +Эта подстрока интерполируется в HTML-текст карточки **без экранирования** (`_stage_line`: +`dur = _fmt_minutes(dur_sum)` → строка `f"✅ {label:<13} {dur} · …"`; те же `_fmt_minutes` / +`_capped_review_str` в строке BRD и в итоговой строке времени). Telegram трактует `<1м` как +открывающий HTML-тег → `editMessageText` отвечает `400 Bad Request: can't parse entities: +Unsupported start tag "1м"`. В `edit_telegram` неизвестный `400` классифицируется как +`EDIT_FAILED` (`notifications.py:203`), а `update_task_tracker` по ветке `EDIT_FAILED` делает +ранний `return` (анти-дубль ORCH-087) → **карточка застывает** (воспроизведено детерминированно +09.06 на ORCH-093, `message_id 18854`). + +**Корневой класс шире одного `<1м`.** Текст карточки — смесь (а) намеренной разметки-обёртки +(`` номер задачи `num_html`, `link_for`, `_done_link`; заголовок уже экранирован как +`esc_title`, `notifications.py:428`) и (б) подставляемых **данных**. Экранирована только +категория-обёртка (href/label в `plane_issue_link` через `html.escape(..., quote=True)`) и +заголовок. Прочие данные — длительности (`_fmt_minutes`/`_capped_review_str`), статус-лейбл +(`_card_status_label` → `status_label`), имя модели (`short_model_name`), эффорт (`_run_effort`), +токены/стоимость (`fmt_tokens`/`fmt_cost`) — вставляются сырыми. `<1м` — первый сработавший +экземпляр класса «неэкранированные данные в HTML-тексте»; ТЗ требует закрыть класс, а не символ +(BR-2/FR-2). + +«Как есть» не годится: симптом плавающий (ловится только когда хотя бы одна стадия длилась +< 60 с и её строка попадает в редактируемый текст), а отказ перманентный для конкретной карточки +до конца жизни задачи — оператор слепнет. + +## Решение + +### Сводка + +Локализуем HTML-безопасность в **границе рендера**: каждое подставляемое **данные-значение** +экранируется `html.escape(...)` ровно один раз в точке интерполяции в `render_task_tracker`; +функции-источники данных (`_fmt_minutes`, `short_model_name`, `_run_effort`, `fmt_tokens`, +`fmt_cost`, `_card_status_label`) остаются **HTML-агностичными** (производят данные, не разметку). +Намеренная разметка-обёртка (`num_html`, `link_for(...)`, `_done_link`, уже-экранированный +`esc_title`) через экранирование **не** проходит. Литерал `<1м` в `_fmt_minutes` **сохраняется +как есть**: будучи экранированным на границе (`<1м`), он рендерится оператору визуально +идентично (`<1м`) → видимый формат не меняется, согласование формулировки не требуется. + +### D1 — Точка внесения экранирования: граница рендера, не источник данных (⇒ FR-1, FR-2) + +Экранирование делается на **потребителе** (внутри `render_task_tracker`/`_stage_line`), а не +внутри функций-источников. Модель «слотов»: текст карточки собирается из слотов двух категорий — + +- **Категория M (markup, НЕ экранировать):** `num_html` (`plane_issue_link`, внутри уже + экранированы href+label), `link_for(...)` в строке «⏳ ждёт …», `_done_link(...)` + («🔗 PR #n · 📦 Внедрено»), `esc_title` (уже экранирован в строке 428). +- **Категория D (data, экранировать ровно один раз):** `dur` (`_fmt_minutes`/`_capped_review_str`), + `status_label` (`_card_status_label`), `model` (`short_model_name`), `effort` (`_run_effort`), + `in_tok`/`out_tok` (`fmt_tokens`), `cost` (`fmt_cost`), а также числовые `attempt` и static-лейблы + стадий (`_TRACKER_STAGES`/`_BRD_LABEL` — статичны и безопасны, но проходят через D ради + единообразного инварианта). + +Рекомендуемая реализация (необязательна к буквальному следованию — выбор формы за developer): +завести тонкий модуль-локальный хелпер `def _esc(x): return html.escape(str(x))` (never-raise: +на исключении `str()` → пустая строка/исходный fallback) и обернуть им каждый D-слот в момент +присваивания, например `dur = _esc(_fmt_minutes(dur_sum))`, `model = _esc(short_model_name(...))`, +`status_label = _esc(status_label)`. Источники данных НЕ трогаются (в т.ч. `src/usage.py` — +`fmt_tokens`/`fmt_cost`/`short_model_name` остаются как есть; defence-in-depth делается на +потребителе, как зафиксировано в ТЗ §2). + +**Почему граница рендера, а не источник.** (1) Single-responsibility: `_fmt_minutes` и +`short_model_name` используются и вне HTML-контекста (логи, потенциально иные потребители) — +вшивать `<` в их вывод сделало бы данные «грязными» в не-HTML-контексте. (2) Инвариант FR-2 +формулируется и тестируется как свойство ОДНОЙ функции (`render_task_tracker`): «ни один символ +`< > &` из данных не остаётся неэкранированным в выходе» — а не как разрозненные контракты пяти +источников. (3) Экранирование на границе по построению исключает двойное экранирование: каждый +D-слот экранируется в ровно одной точке; M-слоты не экранируются вовсе. + +**Инвариант D1:** видимый оператору формат всех D-полей не меняется (escape `<1м`→`<1м` +рендерится как `<1м`; `~Nм`, `Nм`, токены/стоимость/модель символов `< > &` не содержат → +escape для них no-op). + +### D2 — Сохранение `<1м` в источнике; формат-источник `_fmt_minutes` не меняется (⇒ FR-1, BR-3) + +BR-3/FR-1 допускают два пути: (а) экранировать `<1м`, либо (б) переформулировать (`~0м` / +`< 1 мин`). Выбираем **(а)**: `_fmt_minutes` продолжает возвращать `"<1м"`, безопасность даёт +escape на границе (D1). Это минимизирует поверхность изменения (никаких правок числовой/строковой +логики `_fmt_minutes`, `_capped_review_str`, тестов формата длительности) и сохраняет видимый +оператору вид `<1м` без согласования новой формулировки. `_fmt_minutes` сохраняет never-raise +(нечисловой/None → `0м`) без изменений. + +### D3 — Defence-in-depth: экранируются ВСЕ D-поля, включая сейчас-безопасные (⇒ FR-2, BR-2) + +Экранируются все поля категории D, в т.ч. сейчас гарантированно безопасные (`fmt_tokens`/ +`fmt_cost` дают только цифры/`.`/`k`/`M`/`$`; `short_model_name` — `^claude-…$`). Стоимость +нулевая (escape безопасной строки — no-op), выгода — **структурный инвариант**: «каждый D-слот +карточки экранирован», который защищает от регрессии при будущей смене формата любого источника +(напр. если в имя модели/эффорта когда-нибудь попадёт пользовательский ввод). Тест AC-2 ассертит +инвариант, а не отдельные поля. + +### D4 — FR-4 (восстановление застрявших карточек): авто-recovery следующим рендером; парс-фейл НЕ переклассифицируется (⇒ BR-5, FR-4) + +Механизм восстановления — **достаточное условие по умолчанию** из FR-4: после деплоя фикса на +ближайшем переходе стадии `update_task_tracker` рендерит НОВЫЙ безопасный текст и вызывает +`edit_telegram(mid, new_text)` → Telegram отвечает `200` → застрявшая карточка (класс ORCH-093) +обновляется на месте. **Нового кода не требуется.** + +Опциональную переклассификацию `can't parse entities` в `edit_telegram`/`update_task_tracker` +(переотправка свежей карточки вместо `EDIT_FAILED`) **отвергаем**: + +- **Не помогает.** Если текст всё ещё небезопасен, `send_telegram` упадёт на том же `400` + идентично `editMessageText` (тот же `parse_mode=HTML`) и вернёт `None` → новой карточки нет. + После фикса D1–D3 источник `can't parse entities` из НАШИХ данных структурно устранён, поэтому + отдельная ветка восстановления лечит несуществующий после фикса случай. +- **Риск.** Любое касание ветки `EDIT_FAILED`/леджера сирот рискует инвариантом ORCH-087 + (транзиентный фейл НЕ должен плодить карточки). Минимальная поверхность безопаснее. + +`edit_telegram`, `update_task_tracker`, `send_telegram`, леджер `tracker_messages`, режимы +`bump`/`edit` — **не трогаются**. Known-limitation (унаследовано ORCH-087): для карточки, у +которой после фикса больше НЕ будет переходов стадии (задача завершилась до деплоя), повторного +рендера не возникнет → карточка остаётся замёрзшей; Telegram-лимит 48ч делает её неперезаписываемой +вне окна. BR-5 относится к карточкам в пределах окна с предстоящими переходами. + +### D5 — Граница «данные vs обёртка»: M-слоты неприкосновенны, двойное экранирование запрещено (⇒ FR-3, BR-4) + +`num_html` (`plane_issue_link`), `link_for(...)`, `_done_link(...)` и `esc_title` через `_esc` +НЕ проходят — остаются валидным HTML, номер задачи кликабелен. Внутренности `plane_issue_link` +(href `html.escape(url, quote=True)`, label `html.escape(work_item_id)`) уже экранированы — повторно +их не экранируем (иначе `&lt;`, регресс AC-2/AC-3). Граница явная и тестируемая: D-слот → `_esc`; +M-слот → as-is. + +### D6 — Трассировка и инварианты соседних маркеров (⇒ NFR-2, NFR-3) + +`render_task_tracker`/`_stage_line` несут маркеры ORCH-042/067/087/091. Изменение ORCH-095 +**аддитивно** к ним и обязано сохранить их инварианты: «одна карточка на задачу», леджер сирот и +анти-дубль (ORCH-087), отражение откатов + суммирование метрик `_stage_line` (ORCH-091), строка +Plane-статуса/кликабельный номер (ORCH-067). Поскольку ORCH-095 лишь оборачивает уже вычисленные +D-значения в `_esc`, не меняя ни состава строк, ни порядка, ни логики подавления/суммирования — +инварианты сохраняются по построению. Новые/изменённые строки помечаются маркером `ORCH-095`; +блок остаётся читаемым (не вводим 3+ новых маркера в один блок → сводный сквозной ADR не требуется, +TRACEABILITY анти-археология соблюдена). + +## Альтернативы + +- **Экранировать в источнике (`_fmt_minutes` возвращает `<1м`)** — отвергнуто: пачкает данные + в не-HTML-контексте (логи), размазывает инвариант FR-2 по пяти функциям, усложняет защиту от + двойного экранирования (D1). +- **Переформулировать `<1м` → `~0м`/`< 1 мин`** — отвергнуто: меняет видимый оператору формат + (требует согласования), трогает логику/тесты `_fmt_minutes`; escape на границе достигает того же + при меньшей поверхности и нулевом визуальном изменении (D2). +- **Переключить карточку на `parse_mode=None`/MarkdownV2** — отвергнуто (вне объёма BRD §6): + сломает намеренную разметку (`` номер, ``), MarkdownV2 требует экранирования ещё + большего набора символов. +- **Переклассификация `can't parse entities` → переотправка** — отвергнуто (D4): не помогает + (send падает идентично), риск инварианту анти-дубля ORCH-087. + +## Последствия + +- **+** Класс «неэкранированные данные в HTML-тексте карточки» закрыт целиком (BR-2); `<1м` и + любые будущие `< > &` из данных безопасны; карточка со стадией < 1 мин редактируется (`200`). +- **+** Структурный defence-in-depth инвариант («каждый D-слот экранирован»), тестируемый одним + свойством `render_task_tracker` (AC-2), устойчив к будущим сменам формата источников. +- **+** Видимый формат карточки и намеренная разметка (кликабельный номер, `_done_link`) без + изменений (BR-3/BR-4); никаких миграций/правок схемы/гейтов (NFR-3/NFR-4). +- **+** Застрявшие (в окне) карточки авто-восстанавливаются следующим рендером без нового кода + (BR-5). +- **−** Точечная дисциплина «D-слот → `_esc`, M-слот → as-is» вносит точку для будущих ошибок + (можно забыть обернуть новый D-слот или по ошибке обернуть M-слот → двойное экранирование). + Митигейшн: тест-инвариант AC-2 (нет сырого `< > &` из данных И нет `&lt;`) ловит обе + ошибки; явный реестр M-слотов в D5. +- **−** Карточки задач, завершившихся до деплоя фикса, не восстанавливаются (нет будущего + рендера) — known-limitation, унаследовано ORCH-087/Telegram-48ч; вне управляемого. +- **Откат:** обычный revert PR (только `src/notifications.py` + тесты + `CHANGELOG.md` + + doc-правки); прод-контейнер `orchestrator` не требует ручных операций над данными/БД. + +## Ссылки +- BRD: `docs/work-items/ORCH-095/01-brd.md` +- TRZ: `docs/work-items/ORCH-095/02-trz.md` +- Acceptance: `docs/work-items/ORCH-095/03-acceptance-criteria.md` +- Tech-risks: `docs/work-items/ORCH-095/10-tech-risks.md` +- Сверено по коду: `src/notifications.py` (`_fmt_minutes:280-290`, `_capped_review_str:315-336`, + `render_task_tracker:355-610`, `_stage_line:467-507`, `_card_status_label:1173-1186`, + `plane_issue_link:932-949`, `_done_link:613-647`, `link_for:952-984`, `edit_telegram:157-207`, + `update_task_tracker:650-746`, `send_telegram:42-71`, `esc_title:428`) +- Инварианты соседей: ORCH-042/067 (карточка/номер), ORCH-087 (леджер сирот/анти-дубль), + ORCH-091 (откаты/суммирование `_stage_line`) — `docs/architecture/internals.md` §7 diff --git a/docs/work-items/ORCH-095/10-tech-risks.md b/docs/work-items/ORCH-095/10-tech-risks.md new file mode 100644 index 0000000..b254da9 --- /dev/null +++ b/docs/work-items/ORCH-095/10-tech-risks.md @@ -0,0 +1,37 @@ +--- +work_item: ORCH-095 +stage: architecture +author_agent: architect +status: accepted +created_at: 2026-06-09 +model_used: claude-opus-4-8 +--- + +# 10 — Технические риски: ORCH-095 — HTML-безопасность данных live-карточки + +Work Item: **ORCH-095** · Repo: **orchestrator** · Стадия: architecture + +> Информационный (гейтом не парсится). Перечисляет риски реализации и их митигейшн. + +## Реестр рисков + +| ID | Риск | Вер. | Влия. | Митигейшн | +|----|------|------|-------|-----------| +| TR-1 | **Двойное экранирование** уже-экранированных полей (`esc_title`, href/label внутри `plane_issue_link`) → `&lt;` в выводе, визуальный мусор / регресс AC-2 | Сред. | Сред. | D1/D5: явный реестр M-слотов (markup) — через `_esc` НЕ проходят; `esc_title` остаётся единственной точкой escape заголовка; тест AC-2 ассертит отсутствие `&lt;` | +| TR-2 | **Случайное экранирование разметки-обёртки** (`num_html`/`link_for`/`_done_link`) → `` превращается в `<a>`, номер задачи перестаёт быть кликабельным (регресс BR-4/AC-3) | Низ. | Выс. | D5: M-слоты неприкосновенны; регресс-тесты `test_tracker_issue_link.py`/`test_notify_issue_links.py`/`test_telegram_tracker.py` зелёные; AC-3 проверяет наличие валидного `` в выводе | +| TR-3 | **Пропущен новый/существующий D-слот** (забыли обернуть `_esc`) → инъекция возвращается на другом поле | Низ. | Сред. | D3 defence-in-depth (обернуть ВСЕ D-поля разом); тест-инвариант AC-2 рендерит карточку с `< > &` в данных и ассертит отсутствие сырых спецсимволов из данных в выводе (свойство `render_task_tracker`, не пер-поле) | +| TR-4 | **Регресс never-raise**: `_esc(str(x))` на «битом» входе (объект с падающим `__str__`) бросает исключение в пути рендера (нарушение NFR-1) | Низ. | Сред. | FR-5: `_esc` сам never-raise (try/except → fallback-строка); путь `render_task_tracker`/`update_task_tracker` уже обёрнут `try/except` (строки 654/745); тест AC-5 с «битым» входом | +| TR-5 | **Застрявшая карточка не восстановилась** (задача завершилась до деплоя → нет будущего рендера) | Сред. | Низ. | Принятая known-limitation (D4): авто-recovery работает только при предстоящем переходе стадии; вне окна — Telegram-48ч (унаследовано ORCH-087); BR-5 ограничен карточками в окне | +| TR-6 | **Скрытая регрессия инвариантов соседних маркеров** (ORCH-087 анти-дубль, ORCH-091 суммирование `_stage_line`) при правке тела `_stage_line`/`render_task_tracker` | Низ. | Выс. | D6: изменение аддитивно (лишь оборачивает уже вычисленные значения в `_esc`), не меняет состав/порядок строк, логику подавления откатов и суммирования; полный регресс `pytest tests/ -q` зелёный (NFR-2) | +| TR-7 | **Self-hosting**: фикс деплоится на общий прод-инстанс (затронуты и enduro-trails) | Низ. | Сред. | NFR-3: изменение только слоя рендера; `STAGE_TRANSITIONS`/`QG_CHECKS`/схема БД не тронуты; обязательная страховка `deploy-staging` (8501) перед прод-деплоем; прод `orchestrator` не рестартится в рамках разработки | + +## Сводный вывод + +Доминирующий класс рисков — **регресс рендера** (двойное экранирование / случайное экранирование +разметки / пропущенный D-слот), полностью покрываемый тест-инвариантом AC-2 + существующими +регресс-тестами трекера (AC-3/AC-5). Изменение **локализовано** в `src/notifications.py` (слой +рендера уведомлений), аддитивно к маркерам ORCH-042/067/087/091, не затрагивает машину стадий, +Quality Gates, схему БД, транспортные примитивы и режимы трекера. Остаточный риск для +прод-конвейера (self-hosting) — **низкий**: контракт never-raise сохранён, откат — обычный revert +PR без операций над данными. Эскалация `arch:major-change` **не требуется**; возврат в анализ +**не требуется** (ТЗ реализуемо без нарушения архитектурных принципов). -- 2.49.1 From b38cc160410a25424f995f67e6b0c9c8f2ed5fa7 Mon Sep 17 00:00:00 2001 From: claude-bot Date: Wed, 10 Jun 2026 00:07:17 +0300 Subject: [PATCH 4/7] fix(notifications): escape all card data fields at the render boundary (ORCH-095) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit render_task_tracker sends/edits the live card with parse_mode=HTML. _fmt_minutes returns the literal "<1м" for a sub-minute stage; interpolated raw into HTML text Telegram parsed "<1м" as an opening tag -> editMessageText 400 can't parse entities -> edit_telegram EDIT_FAILED -> update_task_tracker early return (anti-duplicate ORCH-087) -> the card froze (incident ORCH-093, message_id 18854). Close the whole "unescaped data in HTML text" class per ADR-001: a module-local _esc(x)=html.escape(str(x)) (never-raise) wraps every DATA slot (durations, status label, model, effort, token/cost metrics) exactly once at the render boundary in render_task_tracker/_stage_line. Source functions stay HTML-agnostic (_fmt_minutes still returns "<1м"; escape on the boundary renders it visually identical as <1м, so the visible format is unchanged). Intentional MARKUP slots (num_html / link_for / _done_link / already-escaped esc_title) are NOT escaped, so the issue number stays a clickable tag and nothing is double-escaped. A previously-frozen card auto-recovers on the next stage transition (a new safe render edits in place, 200) — no new code, no touch to edit_telegram / update_task_tracker / the orphan ledger, so the ORCH-087 anti-duplicate invariant is preserved (a transient edit failure still does not spawn a new card). STAGE_TRANSITIONS / QG_CHECKS / check_* / notification transport / DB schema are untouched. New tests/test_tracker_html_escape.py (TC-01..TC-11); full suite green. Refs: ORCH-095 Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 7 + src/notifications.py | 55 +++-- tests/test_tracker_html_escape.py | 358 ++++++++++++++++++++++++++++++ 3 files changed, 406 insertions(+), 14 deletions(-) create mode 100644 tests/test_tracker_html_escape.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 624f669..4ae9791 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,13 @@ Формат: [Keep a Changelog](https://keepachangelog.com/). Записи — на смысловой PR/задачу. ## [Unreleased] +- **Live-карточка трекера: HTML-инъекция «<1м» больше не застывает карточку — экранирование всех данных-полей на границе рендера** (ORCH-095, `fix`): карточка задачи (`src/notifications.py::render_task_tracker`) шлётся/редактируется с `parse_mode=HTML`. `_fmt_minutes` для стадии < 60 с возвращает литерал `"<1м"`, который интерполировался в HTML-текст **сырым** → Telegram парсит `<1м` как открывающий тег → `editMessageText` отвечает `400 can't parse entities: Unsupported start tag "1м"` → `edit_telegram` классифицирует как `EDIT_FAILED` → `update_task_tracker` делает ранний `return` (анти-дубль ORCH-087) → **карточка застывает** (детерминированно воспроизведено 09.06 на ORCH-093, `message_id 18854`). Корневой класс шире одного `<1м`: все подставляемые **данные** (длительности, статус-лейбл, модель, эффорт, токены/стоимость) вставлялись сырыми; экранирован был только заголовок (`esc_title`) и href/label внутри `plane_issue_link`. **Аддитивно, never-raise, без нового поведения конвейера:** `STAGE_TRANSITIONS` / `QG_CHECKS` / `check_*` / транспорт нотификаций / схема БД — **не тронуты** (затронут ровно один модуль индикативного слоя); kill-switch не требуется (исправление дефекта корректности, откат = `git revert`). + - **Экранирование на границе рендера, не в источнике (ADR-001 D1/D2, AC-1/AC-2):** новый модуль-локальный хелпер `_esc(x) = html.escape(str(x))` (never-raise → `""` на исключении) оборачивает каждое подставляемое **данные-значение** (категория D) ровно один раз в точке интерполяции в `render_task_tracker`/`_stage_line`: длительности (`_fmt_minutes`/`_capped_review_str`), статус-лейбл (`_card_status_label`), модель (`short_model_name`), эффорт (`_run_effort`), токены/стоимость (`fmt_tokens`/`fmt_cost`). Функции-источники остаются **HTML-агностичными** (данные, не разметка): `src/usage.py` и `_fmt_minutes` не тронуты — `_fmt_minutes` продолжает возвращать `"<1м"`, безопасность даёт escape на границе (`<1м` рендерится оператору визуально идентично `<1м` → видимый формат не меняется). + - **Категория M (намеренная разметка) неприкосновенна (D5, AC-3):** кликабельный номер задачи `num_html` (`plane_issue_link`, внутри уже экранированы href+label), `link_for(...)` в строке «⏳ ждёт …», `_done_link(...)` («🔗 PR #n · 📦 Внедрено») и уже-экранированный `esc_title` через `_esc` **не** проходят → остаются валидным HTML, номер остаётся кликабельным. Двойное экранирование (`&lt;`) структурно исключено: D-слот → `_esc` ровно один раз, M-слот → as-is. + - **Defence-in-depth (D3):** экранируются и сейчас-безопасные D-поля (токены/стоимость/модель дают только цифры/`.`/`k`/`M`/`$`/`^claude-…$`) — escape для них no-op, выгода — структурный инвариант «каждый D-слот экранирован», устойчивый к будущей смене формата источника. + - **Восстановление застрявших карточек (D4, AC-4):** механизм — достаточное условие FR-4 без нового кода: на ближайшем переходе стадии `update_task_tracker` рендерит новый безопасный текст → `edit_telegram` отвечает `200` → застрявшая карточка обновляется на месте. Переклассификация `can't parse entities` → переотправка **отвергнута** (после фикса источник из наших данных устранён структурно; касание ветки `EDIT_FAILED`/леджера рискует анти-дублем ORCH-087). Known-limitation (унаследовано ORCH-087/Telegram-48ч): карточка задачи, завершившейся до деплоя фикса, не восстанавливается (нет будущего рендера). + - **Трассировка:** перед правкой блоков, помеченных ORCH-042/067/087/091, прочитаны их ADR — инварианты (одна карточка на задачу, леджер сирот + анти-дубль, отражение откатов + суммирование `_stage_line`, строка Plane-статуса/кликабельный номер) сохранены по построению (ORCH-095 лишь оборачивает уже вычисленные D-значения в `_esc`, не меняя состав строк/порядок/логику подавления). + - Тесты: новый `tests/test_tracker_html_escape.py` (TC-01..TC-11: sub-minute escape на границе, never-raise `_fmt_minutes`/`_esc` на граничных входах, рендер sub-minute без сырого `<1м`, заголовок со спецсимволами без двойного экранирования, escape статус-лейбла/модели/эффорта, HTML-безопасность токенов/стоимости, регресс кликабельного `` номера и `_done_link`, parse-safe edit-payload, edit-in-place без новой карточки + анти-дубль на транзиентном фейле, never-raise на битых входах). Полный регресс `tests/ -q` зелёный (1437). ADR: `docs/work-items/ORCH-095/06-adr/ADR-001-html-safe-card-data-render.md`. Откат: `git revert` (один модуль + тесты + CHANGELOG, без миграций/kill-switch). - **Терминальная (done) задача держит `Done` в Plane: terminal-window-aware гард deploy-статусов** (ORCH-094, `fix`): задача с БД `stage=done` и 0 активных job'ов (верифицировано на ORCH-061, task 47) стабильно флаппила в Plane `Awaiting Deploy ⟷ Monitoring after Deploy` (273 активности парами, само не затихает) вместо `Done`. Корень: три deploy-фазовых сеттера (`set_issue_awaiting_deploy`/`set_issue_deploying`/`set_issue_monitoring`) **терминал-слепы** — любой стейл/двойной/неизвестный вызов под бот-токеном перезаписывает `Done` промежуточным deploy-статусом, и обратно, бесконечно. **Аддитивно, never-raise, под kill-switch, в зоне self-hosting:** `STAGE_TRANSITIONS` / `QG_CHECKS` / `check_*` / machine-verdict ключи (`deploy_status:`/`staging_status:`/…) / схема БД — **не тронуты** (читается существующая `tasks.stage`, без миграции). - **Единый гард на низком чокпоинте (FR-2, D1/D2):** новый leaf `src/deploy_status_guard.py` (чистая, never-raise, config-gated логика; по образцу `serial_gate.py`/`labels.py`/`cancel.py`) — `decide(work_item_id, target, reason) -> ALLOW | CONVERGE_DONE | SUPPRESS`. Гард ставится на **входе** трёх сеттеров `plane_sync` (а не в caller'ах `stage_engine`) → перехватывает **любой** путь, включая неизвестный актор под бот-токеном. Предикат легитимности: deploy-статус легитимен ⇔ задача **нетерминальна** ИЛИ (`done` **И** активно пост-деплой-окно `post_deploy.window_active` = ARMED & не DONE). Для `done`: `monitoring`+окно-активно → `ALLOW`; иначе → `CONVERGE_DONE` (сеттер вместо PATCH'а зовёт `set_issue_done`, идемпотентно). `cancelled` → `SUPPRESS` (не штампуем поверх терминала ORCH-090). Нетерминальная задача → `ALLOW` (рабочий deploy-цикл 1:1, AC-4). Task не найден / не-self репо / kill-switch off / любое исключение → `ALLOW` (fail-safe к прежнему поведению 1:1, NFR-1). - **Перенос арм-блока перед terminal-sync (D3, AC-4):** в `advance_stage` (ветка `next_stage=="done"`) блок `post_deploy.arm_monitor` перемещён **выше** блока `set_issue_monitoring` (стр. 404). Критично: `update_task_stage(task_id,"done")` пишет `stage='done'` **раньше** легитимного первого `Monitoring` — без переноса гард ошибочно свёл бы его к Done. Арм-первым пишет `ARMED` → `window_active==True` → `ALLOW` пропускает легитимный `Monitoring`; re-drive `deploy→done` **после** закрытия окна (`DONE` present) → `window_active==False` → `CONVERGE_DONE` (не воскрешает `Monitoring`). Перенос безопасен: `arm_monitor` лишь пишет sentinel + ставит отложенный job, не зависит от Plane-статуса/merge-lease (release остаётся после terminal-sync). Инварианты ORCH-021 (идемпотентный арм по `ARMED`) и ORCH-066 (`deploy→done` self ⇒ `Monitoring`) сохранены. diff --git a/src/notifications.py b/src/notifications.py index dbc6b31..bc82174 100644 --- a/src/notifications.py +++ b/src/notifications.py @@ -290,6 +290,27 @@ def _fmt_minutes(seconds) -> str: return f"{seconds // 60}\u043c" +def _esc(x) -> str: + """ORCH-095: escape a DATA value for the parse_mode=HTML card text (never-raise). + + Every dynamic *data* value interpolated into ``render_task_tracker``'s HTML text + (durations, status label, model, effort, token/cost metrics) is wrapped here + exactly once at the render boundary (ADR-001, category D). This closes the class + "unescaped data in HTML text": a literal like ``<1м`` from ``_fmt_minutes`` (or any + future ``< > &`` from a data source) can no longer be parsed by Telegram as an + opening tag (``400 can't parse entities`` -> EDIT_FAILED -> frozen card, ORCH-093). + + Intentional markup slots (``num_html``/``link_for``/``_done_link``/already-escaped + ``esc_title`` — category M) are NOT passed through ``_esc`` so they stay valid, + clickable HTML and are never double-escaped. On any error ``str()``/escape degrades + to '' rather than raising (FR-5 never-raise). + """ + try: + return html.escape(str(x)) + except Exception: + return "" + + def _parse_sql_ts(ts): """Parse a SQLite 'YYYY-MM-DD HH:MM:SS' UTC timestamp -> aware datetime/None.""" if not ts: @@ -445,7 +466,9 @@ def render_task_tracker(task_id: int) -> str: ) except Exception: status_label = _DEFAULT_STATUS_LABEL - status_line = f"\U0001f4cd {status_label}" + # ORCH-095 (ADR-001 D3): status label is a DATA slot (offline core + live + # overlay) -> escaped at interpolation; intentional markup is never built here. + status_line = f"\U0001f4cd {_esc(status_label)}" lines = [header, status_line, bar] # ORCH-026 (B-4): waiting-line for a task blocked by an unfinished declared @@ -487,19 +510,23 @@ def render_task_tracker(task_id: int) -> str: d = _duration_seconds(run["started_at"], run["finished_at"]) if d is not None: dur_sum += d - in_tok = fmt_tokens(in_sum) - out_tok = fmt_tokens(out_sum) - cost = fmt_cost(cost_sum) - dur = _fmt_minutes(dur_sum) + # ORCH-095 (ADR-001 D1/D3): every interpolated DATA value (category D) is + # escaped here at the render boundary so a literal like '<1м' from + # _fmt_minutes can no longer break parse_mode=HTML; defence-in-depth for the + # token/cost/model/effort fields too (currently safe, structurally guarded). + in_tok = _esc(fmt_tokens(in_sum)) + out_tok = _esc(fmt_tokens(out_sum)) + cost = _esc(fmt_cost(cost_sum)) + dur = _esc(_fmt_minutes(dur_sum)) # Model/effort/"\u043f\u043e\u043f\u044b\u0442\u043a\u0430 N" come from the LAST run (agent_runs are id ASC). last = stage_runs[-1] if stage_runs else None - model = short_model_name(last["model"]) if last is not None else "" + model = _esc(short_model_name(last["model"])) if last is not None else "" model_suffix = f" \u00b7 {model}" if model else "" # ORCH-087 (BR-EFF): render the resolved --effort next to the model # ("\u00b7 opus-4-8 \u00b7 xhigh"). Stamped at launch in agent_runs.effort; empty / # missing -> suffix omitted (like the model suffix). Historical rows with # NULL effort fall back to the config-resolved effort for the agent. - effort = _run_effort(last) if last is not None else "" + effort = _esc(_run_effort(last)) if last is not None else "" effort_suffix = f" \u00b7 {effort}" if effort else "" return ( f"\u2705 {label:<13} {dur} \u00b7 " @@ -564,7 +591,7 @@ def render_task_tracker(task_id: int) -> str: if review_seconds is not None: # ORCH-042 (BR-10): approve-gate passed -> \u2705 (was \u23f8\ufe0f). The # still-waiting branch below keeps \u23f8\ufe0f + \u23f3 unchanged. - dur = _fmt_minutes(review_seconds) + dur = _esc(_fmt_minutes(review_seconds)) # ORCH-095: D-slot lines.append( f"\u2705 {brd_label} {dur} \u00b7 \u0442\u0432\u043e\u0451 \u0432\u0440\u0435\u043c\u044f" ) @@ -577,21 +604,21 @@ def render_task_tracker(task_id: int) -> str: waited = int( (datetime.now(timezone.utc) - start_dt).total_seconds() ) - dur = _fmt_minutes(waited) if waited is not None else "\u2026" + dur = _esc(_fmt_minutes(waited)) if waited is not None else "\u2026" # ORCH-095: D-slot lines.append( f"\u23f8\ufe0f {brd_label} {dur} \u00b7 \u0442\u0432\u043e\u0451 \u0432\u0440\u0435\u043c\u044f \u23f3" ) lines.append(bar) lines.append( - f"\U0001f4b0 {fmt_tokens(total_in)}\u2193 / {fmt_tokens(total_out)}\u2191 \u00b7 " - f"{fmt_cost(total_cost)}" + f"\U0001f4b0 {_esc(fmt_tokens(total_in))}\u2193 / {_esc(fmt_tokens(total_out))}\u2191 \u00b7 " + f"{_esc(fmt_cost(total_cost))}" ) if done: wall = _duration_seconds(task["created_at"], task["updated_at"]) - wall_str = _fmt_minutes(wall) if wall is not None else "?" - review_str = _capped_review_str(review_seconds) + wall_str = _esc(_fmt_minutes(wall)) if wall is not None else "?" # ORCH-095: D-slot + review_str = _esc(_capped_review_str(review_seconds)) # ORCH-095: D-slot # ORCH-087 (BR-G5): three INDEPENDENT, explicitly-labelled metrics. None is # presented as the sum of the others \u2014 queue/wait pauses are not logged, so # wall != agents + review; the old "\u0412\u0441\u0435\u0433\u043e {wall}" read like a (wrong) sum. @@ -599,7 +626,7 @@ def render_task_tracker(task_id: int) -> str: # \u0442\u0432\u043e\u0451 = human BRD-review, capped to drop anomalous stalls (T-2) # \u043e\u0431\u0449\u0435\u0435 \u0441 \u043e\u0436\u0438\u0434\u0430\u043d\u0438\u0435\u043c = wall-clock incl. queue/wait, NOT work time (T-3) lines.append( - f"\u23f1\ufe0f \u0410\u0433\u0435\u043d\u0442\u044b {_fmt_minutes(agent_seconds)} \u00b7 " + f"\u23f1\ufe0f \u0410\u0433\u0435\u043d\u0442\u044b {_esc(_fmt_minutes(agent_seconds))} \u00b7 " f"\u0442\u0432\u043e\u0451 {review_str} \u00b7 " f"\u043e\u0431\u0449\u0435\u0435 \u0441 \u043e\u0436\u0438\u0434\u0430\u043d\u0438\u0435\u043c {wall_str}" ) diff --git a/tests/test_tracker_html_escape.py b/tests/test_tracker_html_escape.py new file mode 100644 index 0000000..1c9ebdb --- /dev/null +++ b/tests/test_tracker_html_escape.py @@ -0,0 +1,358 @@ +"""ORCH-095 — HTML-safety of dynamic data fields in render_task_tracker. + +The live card text is sent/edited with parse_mode=HTML. It is assembled from +two kinds of slots: + + * category M (intentional markup, NEVER escaped): the clickable issue number + (plane_issue_link -> ), the ⏳-waiting links (link_for), the done + line (_done_link), and the already-escaped title (esc_title); + * category D (data, escaped EXACTLY once at the render boundary): durations + (_fmt_minutes / _capped_review_str), the status label, model, effort, and + the token/cost metrics. + +The bug (ORCH-093 incident): _fmt_minutes returns the literal "<1м" for a +sub-minute stage; interpolated raw into HTML text Telegram parsed "<1м" as an +opening tag -> 400 can't parse entities -> EDIT_FAILED -> the card froze. ADR-001 +closes the whole class by escaping every D-slot at the boundary (helper N._esc) +while keeping the M-slots intact (so the number stays clickable, no double-escape). + +These tests assert: sub-minute durations are safe (TC-01/02/03), all data fields +escape special chars without double-escaping (TC-04/05/06), markup survives +(TC-07/08), the edit payload is parse-safe and the anti-duplicate invariant +(ORCH-087) holds (TC-09/10), and the render path never raises (TC-11). + +Network is isolated (no live overlay HTTP); the DB is a temp SQLite. +""" + +import os +import tempfile + +os.environ.setdefault("ORCH_PLANE_API_TOKEN", "test-token") +os.environ.setdefault("ORCH_GITEA_TOKEN", "test-token") + +_test_db = os.path.join(tempfile.gettempdir(), "test_orchestrator_html_escape.db") +os.environ["ORCH_DB_PATH"] = _test_db + +from types import SimpleNamespace # noqa: E402 +from unittest.mock import MagicMock # noqa: E402 + +import pytest # noqa: E402 + +import src.db as db_module # noqa: E402 +import src.projects as projects_mod # noqa: E402 +from src.db import init_db, get_db # noqa: E402 +from src import notifications as N # noqa: E402 + +# orchestrator repo -> default project registry uuid (src/projects.py). +_ORCH_PROJECT_ID = "8da6aa25-a60e-44d6-a1e2-d8ae59aa7d6a" + + +@pytest.fixture(autouse=True) +def setup_db(monkeypatch): + monkeypatch.setattr(db_module.settings, "db_path", _test_db, raising=False) + if os.path.exists(_test_db): + os.unlink(_test_db) + init_db() + # Keep the render path fully offline (no live overlay HTTP). + monkeypatch.setattr(N._get_settings(), "tracker_live_status", False, + raising=False) + monkeypatch.setattr( + projects_mod, "get_project_by_repo", + lambda repo: (SimpleNamespace(plane_project_id=_ORCH_PROJECT_ID) + if repo == "orchestrator" else None), + ) + yield + if os.path.exists(_test_db): + os.unlink(_test_db) + + +def _set(monkeypatch, **kw): + s = N._get_settings() + for k, v in kw.items(): + monkeypatch.setattr(s, k, v, raising=False) + + +def _mk_task(wid="ORCH-095", repo="orchestrator", title="card", + plane_issue_id="issue-uuid-1", stage="development", + brd_start=None, brd_end=None): + conn = get_db() + cur = conn.execute( + "INSERT INTO tasks (plane_id, work_item_id, repo, branch, stage, title, " + "plane_issue_id, brd_review_started_at, brd_review_ended_at) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)", + ("p1", wid, repo, "feature/ORCH-095-x", stage, title, plane_issue_id, + brd_start, brd_end), + ) + tid = cur.lastrowid + conn.commit() + conn.close() + return tid + + +def _mk_run(task_id, agent, started, finished, in_tok=100, out_tok=50, + cache_read=0, cache_creation=0, cost=0.0, model=None, + effort=None, exit_code=0): + conn = get_db() + cur = conn.execute( + "INSERT INTO agent_runs (task_id, agent, started_at, finished_at, " + "exit_code, input_tokens, output_tokens, cache_read_tokens, " + "cache_creation_tokens, cost_usd, model, effort) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + (task_id, agent, started, finished, exit_code, in_tok, out_tok, + cache_read, cache_creation, cost, model, effort), + ) + rid = cur.lastrowid + conn.commit() + conn.close() + return rid + + +# A stage that lasted 30s (< 60s) -> _fmt_minutes -> "<1м". +_SUB_MIN_START = "2026-06-04 09:00:00" +_SUB_MIN_END = "2026-06-04 09:00:30" + + +# --------------------------------------------------------------------------- # +# TC-01 — sub-minute duration is HTML-safe at the render boundary +# --------------------------------------------------------------------------- # +def test_tc01_sub_minute_duration_escaped_at_boundary(): + # ADR-001 D2: _fmt_minutes keeps returning the literal "<1м" (source + # unchanged); safety comes from _esc at the boundary -> "<1м". + assert N._fmt_minutes(30) == "<1м" + escaped = N._esc(N._fmt_minutes(30)) + assert escaped == "<1м" + assert "<1" not in escaped # no raw opening-tag-looking substring + + +# --------------------------------------------------------------------------- # +# TC-02 — _fmt_minutes boundary inputs: never-raise + boundary-safe +# --------------------------------------------------------------------------- # +@pytest.mark.parametrize("value", [0, None, "abc", 59, 60, 61, 100000, -5, 30]) +def test_tc02_fmt_minutes_never_raise_and_safe(value): + out = N._fmt_minutes(value) # must not raise + assert isinstance(out, str) + safe = N._esc(out) + # After boundary escaping no raw '<' (or '>'/'&') survives in any branch. + assert "<" not in safe + assert ">" not in safe + + +# --------------------------------------------------------------------------- # +# TC-03 — render_task_tracker for a sub-minute stage: no raw '<1м' +# --------------------------------------------------------------------------- # +def test_tc03_render_sub_minute_stage_is_safe(): + tid = _mk_task(stage="development") + # Analysis stage lasted 30s; analysis sits before development -> ✅ line shown. + _mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, model="claude-opus-4-8") + + text = N.render_task_tracker(tid) + assert "<1м" not in text # the bug: raw literal must be gone + assert "<1м" in text # rendered safely instead + # And no double escaping leaked in. + assert "&lt;" not in text + + +# --------------------------------------------------------------------------- # +# TC-04 — title with '<', '>', '&' escaped, no raw tags, no double-escape +# --------------------------------------------------------------------------- # +def test_tc04_title_special_chars_escaped_no_double(): + tid = _mk_task(title="A x & <1", stage="development") + _mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END) + + text = N.render_task_tracker(tid) + # Data special chars present only escaped... + assert "<b>" in text + assert "&" in text + # ...never as raw markup from the title. + assert "" not in text + assert "" not in text + # No double escaping anywhere. + assert "&lt;" not in text + assert "&amp;" not in text + + +# --------------------------------------------------------------------------- # +# TC-05 — status label + model + effort are escaped (defence-in-depth) +# --------------------------------------------------------------------------- # +def test_tc05_status_label_escaped(monkeypatch): + monkeypatch.setattr(N, "_card_status_label", lambda *a, **k: "") + tid = _mk_task(stage="development") + text = N.render_task_tracker(tid) + assert "<danger>" in text + assert "" not in text + + +def test_tc05_model_escaped(monkeypatch): + # The model name is a D-slot: even if a '<' ever reached it, it is escaped. + import src.usage as U + monkeypatch.setattr(U, "short_model_name", lambda m: "" if m else "") + tid = _mk_task(stage="development") + _mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, model="whatever") + text = N.render_task_tracker(tid) + assert "<m>" in text + assert "" not in text + + +def test_tc05_effort_escaped(): + tid = _mk_task(stage="development") + _mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, + model="claude-opus-4-8", effort="") + text = N.render_task_tracker(tid) + assert "<e>" in text + assert "" not in text + + +# --------------------------------------------------------------------------- # +# TC-06 — token / cost metrics are HTML-safe ('$' + digits) +# --------------------------------------------------------------------------- # +def test_tc06_token_cost_metrics_safe(): + tid = _mk_task(stage="development") + _mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, + in_tok=1_100_000, out_tok=39_600, cost=2.38, + model="claude-opus-4-8") + text = N.render_task_tracker(tid) + # The 💰 totals line renders with a '$' cost and no stray angle brackets + # coming from the metric data. + assert "$" in text + assert "&" not in text or "&lt;" not in text # no double-escape + # No raw opening-tag substring produced by the metrics. + assert "<$" not in text + + +# --------------------------------------------------------------------------- # +# TC-07 — markup regression: clickable issue number stays a valid tag +# --------------------------------------------------------------------------- # +def test_tc07_issue_number_stays_clickable(monkeypatch): + _set(monkeypatch, plane_web_url="https://plane.example.org", + plane_api_url="http://localhost:8091", plane_workspace_slug="acme") + tid = _mk_task(plane_issue_id="abcd-issue-uuid", stage="development") + _mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END) + + text = N.render_task_tracker(tid) + expected_url = ( + f"https://plane.example.org/acme/projects/{_ORCH_PROJECT_ID}" + f"/issues/abcd-issue-uuid/" + ) + # The anchor markup is NOT escaped (M-slot) -> still clickable & valid. + assert f'ORCH-095' in text + assert text.count("") + # href/label are not double-escaped. + assert "<a href=" not in text + assert "&lt;" not in text + + +# --------------------------------------------------------------------------- # +# TC-08 — markup regression: _done_link renders valid '🔗 PR #n · 📦 Внедрено' +# --------------------------------------------------------------------------- # +def test_tc08_done_link_markup_preserved(monkeypatch): + tid = _mk_task(stage="done") + _mk_run(tid, "deployer", _SUB_MIN_START, _SUB_MIN_END) + + # Mock the Gitea PR lookup inside _done_link. + resp = MagicMock() + resp.status_code = 200 + resp.json = lambda: [{"number": 105}] + monkeypatch.setattr(N.httpx, "get", lambda *a, **k: resp) + + text = N.render_task_tracker(tid) + assert "\U0001f517 PR #105" in text # 🔗 PR #105 + assert "\U0001f4e6" in text # 📦 + # The done line is an M-slot -> not escaped. + assert "<" not in text.split("\n")[-1] + + +# --------------------------------------------------------------------------- # +# TC-09 — integration: edit payload for a sub-minute card is parse-safe +# --------------------------------------------------------------------------- # +def test_tc09_edit_payload_is_parse_safe(monkeypatch): + from src.db import set_tracker_message_id + _set(monkeypatch, tracker_mode="edit", + telegram_bot_token="bot-token", telegram_chat_id="chat-1") + tid = _mk_task(stage="development") + _mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, model="claude-opus-4-8") + set_tracker_message_id(tid, 18854) + + captured = {} + + def _fake_post(url, json=None, timeout=None, **kw): + captured["url"] = url + captured["json"] = json + return SimpleNamespace(status_code=200, json=lambda: {"ok": True}) + + monkeypatch.setattr(N.httpx, "post", _fake_post) + + N.update_task_tracker(tid) + + assert "editMessageText" in captured["url"] + payload_text = captured["json"]["text"] + assert captured["json"]["parse_mode"] == "HTML" + # The crux of ORCH-095: no raw '<1м' reaches Telegram -> no 'can't parse + # entities' -> the card does not freeze. + assert "<1м" not in payload_text + assert "<1м" in payload_text + + +# --------------------------------------------------------------------------- # +# TC-10 — stuck card resumes; anti-duplicate (ORCH-087) preserved +# --------------------------------------------------------------------------- # +def test_tc10_valid_render_edits_in_place_no_new_card(monkeypatch): + from src.db import set_tracker_message_id, get_tracker_message_id + _set(monkeypatch, tracker_mode="edit") + tid = _mk_task(stage="development") + _mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, model="claude-opus-4-8") + set_tracker_message_id(tid, 18854) + + # After the fix the render is valid -> edit succeeds in place (EDIT_OK). + monkeypatch.setattr(N, "edit_telegram", lambda mid, text: N.EDIT_OK) + send_mock = MagicMock(return_value=999) + monkeypatch.setattr(N, "send_telegram", send_mock) + + N.update_task_tracker(tid) + + send_mock.assert_not_called() # edited in place, no new card + assert get_tracker_message_id(tid) == 18854 # pointer unchanged + + +def test_tc10_transient_fail_does_not_duplicate(monkeypatch): + # ORCH-087 invariant: a transient edit failure must NOT spawn a new card. + from src.db import set_tracker_message_id, get_tracker_message_id + _set(monkeypatch, tracker_mode="edit") + tid = _mk_task(stage="development") + _mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, model="claude-opus-4-8") + set_tracker_message_id(tid, 18854) + + monkeypatch.setattr(N, "edit_telegram", lambda mid, text: N.EDIT_FAILED) + send_mock = MagicMock(return_value=999) + monkeypatch.setattr(N, "send_telegram", send_mock) + + N.update_task_tracker(tid) + + send_mock.assert_not_called() # no duplicate on transient fail + assert get_tracker_message_id(tid) == 18854 + + +# --------------------------------------------------------------------------- # +# TC-11 — never-raise on broken inputs +# --------------------------------------------------------------------------- # +def test_tc11_never_raise_missing_task(): + # No such task -> minimal fallback string, no exception. + assert N.render_task_tracker(999999) == "task-999999" + + +def test_tc11_never_raise_none_title_and_bad_timestamps(): + tid = _mk_task(title=None, stage="development") + # Unparseable timestamps -> _duration_seconds degrades to None, no raise. + _mk_run(tid, "analyst", "not-a-ts", "also-bad", model="claude-opus-4-8") + text = N.render_task_tracker(tid) # must not raise + assert isinstance(text, str) + assert "ORCH-095" in text # falls back to work_item_id + + +def test_tc11_esc_never_raises(): + class _Boom: + def __str__(self): + raise RuntimeError("boom") + + # _esc degrades to '' rather than propagating an exception (FR-5). + assert N._esc(_Boom()) == "" + assert N._esc(None) == "None" -- 2.49.1 From b77d412c361090fd45f2caba11ccbf7cd0daca9c Mon Sep 17 00:00:00 2001 From: claude-bot Date: Wed, 10 Jun 2026 00:11:01 +0300 Subject: [PATCH 5/7] reviewer(ET): auto-commit from reviewer run_id=529 --- docs/work-items/ORCH-095/12-review.md | 81 +++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 docs/work-items/ORCH-095/12-review.md diff --git a/docs/work-items/ORCH-095/12-review.md b/docs/work-items/ORCH-095/12-review.md new file mode 100644 index 0000000..e20bb24 --- /dev/null +++ b/docs/work-items/ORCH-095/12-review.md @@ -0,0 +1,81 @@ +--- +verdict: APPROVED +work_item: ORCH-095 +stage: review +author_agent: reviewer +status: approved +created_at: 2026-06-10 +model_used: claude-opus-4-8 +type: review +work_item_id: ORCH-095 +version: 1 +--- + +# Review ORCH-095 + +## Summary + +Фикс HTML-инъекции `<1м` в live-карточке трекера. Точечное, аддитивное, never-raise изменение +в индикативном слое (`src/notifications.py`): новый модуль-локальный хелпер `_esc(x) = +html.escape(str(x))` оборачивает каждый **data**-слот (`dur`/`_fmt_minutes`/`_capped_review_str`, +`status_label`, `model`, `effort`, токены/стоимость) ровно один раз на границе рендера +(`render_task_tracker`/`_stage_line`); **markup**-слоты (`num_html`/`link_for`/`_done_link`/ +уже-экранированный `esc_title`) не трогаются. Источники (`_fmt_minutes`, `src/usage.py`) остаются +HTML-агностичными. + +Проверены все четыре оси. Реализация соответствует ТЗ (FR-1…FR-5) и ADR-001 (D1…D6) буквально; +все 5 AC выполнены. `STAGE_TRANSITIONS` / `QG_CHECKS` / `check_*` / схема БД / транспорт нотификаций +— не тронуты (`git diff` пуст по `src/stages.py`, `src/qg/`, `src/stage_engine.py`, `src/db.py`). +Полный регресс `pytest tests/ -q` зелёный (**1437 passed**), новый `tests/test_tracker_html_escape.py` +(TC-01…TC-11) — зелёный. + +**Соответствие осям:** +1. **ТЗ / AC** — FR-1/AC-1 (`<1м`→`<1м` на границе, источник не меняется), FR-2/AC-2 (все + D-слоты экранированы — сверено по коду стр. 471/517-523/529/594/607/614-615/620-621/629), + FR-3/AC-3 (M-слоты не экранированы, двойного экранирования нет), FR-4/AC-4 (авто-восстановление + следующим рендером, без рискованной переклассификации `EDIT_FAILED` — корректно, защищает + инвариант ORCH-087), FR-5/AC-5 (never-raise + зелёный регресс + CHANGELOG). ✓ +2. **ADR + трассировка** — реализация 1:1 с ADR-001 (escape на границе рендера, не в источнике; + M-слоты неприкосновенны). Блоки с маркерами ORCH-042/067/087/091 правлены аддитивно: код лишь + оборачивает уже вычисленные D-значения в `_esc`, не меняя состав строк/порядок/логику подавления + и суммирования — инварианты сохранены по построению. Сквозной `adr-NNNN` обоснованно не заведён + (локальный indication-only фикс). ✓ +3. **Качество кода** — `_esc` с docstring и never-raise; тесты содержательные (11 TC покрывают + каждый AC, включая регресс кликабельного ``-номера, `_done_link` и анти-дубль ORCH-087 + на транзиентном фейле). ✓ +4. **Документация** — обновлены в том же PR: `CHANGELOG.md`, `docs/architecture/README.md` + (блок Notifications/Live-tracker), `docs/architecture/internals.md` §7, ADR-001. ✓ + +## Findings + +### P0 — Blocker +- Нет. + +### P1 — Must fix +- Нет. + +### P2 — Should fix +- Нет. + +### P3 — Nice-to-have +- [ ] `attempt` (`f"… попытка {attempt} …"`, ~стр. 572) и статичные лейблы стадий + (`_TRACKER_STAGES`/`_BRD_LABEL`) не проходят через `_esc`. ADR-001 D1 упоминает их в категории D + «ради единообразного инварианта», но `attempt` — всегда `int` (`len(agent_runs)`), а лейблы — + статичные константы → фактической поверхности инъекции нет, расхождение безвредно. Не блокирует; + можно унифицировать при будущем касании блока (оставляю на усмотрение, не требую правки). + +## Документация + +**Обновлена полностью в том же PR — требование правила 6 (CLAUDE.md) выполнено:** +- `CHANGELOG.md` — детальная запись ORCH-095 (механизм бага, D1–D5, восстановление, трассировка, тесты). +- `docs/architecture/README.md` — компонент «Notifications / Live-tracker» дополнен абзацем ORCH-095 + (data/markup-слоты, инвариант экранирования на границе, ссылка на ADR). +- `docs/architecture/internals.md` §7 — новая подсекция «HTML-безопасность данных карточки (ORCH-095)». +- `docs/work-items/ORCH-095/06-adr/ADR-001-html-safe-card-data-render.md` — архитектурное обоснование + (выбор точки экранирования, альтернативы, последствия). + +Пункт `README.md` «Известные ограничения» данным фиксом не закрывается (баг корректности, не числился +в витрине ограничений) → обновление обзорной витрины (ORCH-079) не требуется. + +**Вывод:** `src/` изменён — документация обновлена синхронно. P0 «документация не обновлена» не +применяется. -- 2.49.1 From cdc5e5c5486ec2d727525d996ee84a755f0a3099 Mon Sep 17 00:00:00 2001 From: claude-bot Date: Wed, 10 Jun 2026 00:13:15 +0300 Subject: [PATCH 6/7] tester(ET): auto-commit from tester run_id=530 --- docs/work-items/ORCH-095/13-test-report.md | 92 ++++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 docs/work-items/ORCH-095/13-test-report.md diff --git a/docs/work-items/ORCH-095/13-test-report.md b/docs/work-items/ORCH-095/13-test-report.md new file mode 100644 index 0000000..49156b5 --- /dev/null +++ b/docs/work-items/ORCH-095/13-test-report.md @@ -0,0 +1,92 @@ +--- +result: PASS # PASS | FAIL — машинный вердикт, UPPERCASE +work_item: ORCH-095 +stage: testing +author_agent: tester +status: pass +created_at: 2026-06-10 +model_used: claude-opus-4-8 +type: test-report +work_item_id: ORCH-095 +--- + +# Test Report — ORCH-095 + +Фикс HTML-инъекции «<1м» в live-карточке трекера (`render_task_tracker`). Прогон полного +регресса + профильной сюиты, smoke read-only API. Review-вердикт — `APPROVED` (12-review.md). + +## Окружение +- Python: 3.12.13 +- pytest: 8.3.3 +- Дата: 2026-06-10 +- Worktree: `/repos/_wt/orchestrator/feature_ORCH-095-bug-html-1-render-task-tracker` + (ветка `feature/ORCH-095-bug-html-1-render-task-tracker` — код именно этой задачи, не общий чекаут) + +## Smoke API (read-only, прод-контейнер не трогается) +- `GET /health` → `{"status":"ok","service":"orchestrator"}` ✓ +- `GET /status` → активная задача ORCH-095 (id=80) на стадии `testing`, agent_running=null ✓ +- `GET /queue` → блок `serial_gate` **присутствует** (ORCH-088): `enabled=true`, репо + `orchestrator` — active_task ORCH-095 `testing`, `frozen=false`, waiting пуст; блок + `auto_labels` **присутствует** (ORCH-089). Регресса смока нет. ✓ + +## Результаты + +### Полный регресс +`cd && pytest tests/ -v --tb=short` → **1437 passed, 1 warning in 46.89s**. +Единственное предупреждение — PydanticDeprecatedSince20 (унаследованное, не относится к задаче). + +### Профильная сюита (ORCH-095) +`pytest tests/test_tracker_html_escape.py -v` → **24 passed** (новый файл, TC-01…TC-11). + +### Регресс существующих тестов трекера (TC-12) +`pytest tests/test_telegram_tracker.py tests/test_tracker_issue_link.py +tests/test_tracker_status_line.py tests/test_notifications_orphans.py +tests/test_notify_issue_links.py -q` → **91 passed**. + +### Сопоставление с тест-планом (04-test-plan.yaml) + +| TC ID | Описание | Тест-функция | Результат | +|-------|----------|--------------|-----------| +| TC-01 | `_fmt_minutes(<60с)` → HTML-безопасно, без сырого `<1м` | `test_tc01_sub_minute_duration_escaped_at_boundary` | PASS | +| TC-02 | `_fmt_minutes` граничные входы (0/None/нечисло/60/большое/-5/59/61) — never-raise + безопасно | `test_tc02_fmt_minutes_never_raise_and_safe[*]` (9 кейсов) | PASS | +| TC-03 | `render_task_tracker` со стадией < 1 мин — нет неэкранированного `<` из длительности | `test_tc03_render_sub_minute_stage_is_safe` | PASS | +| TC-04 | Заголовок со спецсимволами `< > &` — только экранированно, без двойного экранирования | `test_tc04_title_special_chars_escaped_no_double` | PASS | +| TC-05 | Статус-лейбл / имя модели / эффорт экранированы (defence-in-depth) | `test_tc05_status_label_escaped`, `test_tc05_model_escaped`, `test_tc05_effort_escaped` | PASS | +| TC-06 | Токены/стоимость (`$`, числа) HTML-безопасны | `test_tc06_token_cost_metrics_safe` | PASS | +| TC-07 | Регресс намеренной разметки: `` номер задачи остаётся кликабельным, не задвоен | `test_tc07_issue_number_stays_clickable` | PASS | +| TC-08 | Регресс `_done_link`: строка `🔗 PR #n · 📦 Внедрено` валидна, не экранирована | `test_tc08_done_link_markup_preserved` | PASS | +| TC-09 | `update_task_tracker` (edit) — payload text не содержит сырого `<1м`-триггера | `test_tc09_edit_payload_is_parse_safe` | PASS | +| TC-10 | Возобновление застрявшей карточки + анти-дубль ORCH-087 на транзиентном фейле | `test_tc10_valid_render_edits_in_place_no_new_card`, `test_tc10_transient_fail_does_not_duplicate` | PASS | +| TC-11 | never-raise на битых входах (нет задачи / None-заголовок / битые timestamps / `_esc`) | `test_tc11_never_raise_missing_task`, `test_tc11_never_raise_none_title_and_bad_timestamps`, `test_tc11_esc_never_raises` | PASS | +| TC-12 | Полный регресс существующих тестов трекера остаётся зелёным | suite (91 passed) + полный регресс (1437 passed) | PASS | + +**Все 12 TC выполнены и сопоставлены.** + +### Сопоставление с критериями приёмки (03-acceptance-criteria.md) + +| AC | Содержание | Покрытие | Результат | +|----|------------|----------|-----------| +| AC-1 | Стадия < 1 мин не ломает парсер Telegram (`<1м`) | TC-01, TC-03, TC-09 | PASS | +| AC-2 | Все динамические поля HTML-безопасны, без двойного экранирования | TC-02, TC-04, TC-05, TC-06 | PASS | +| AC-3 | Регресс намеренной разметки (`` номер, `_done_link`, форматирование) | TC-07, TC-08, TC-12 | PASS | +| AC-4 | Застрявшая карточка возобновляет обновления; анти-дубль ORCH-087 цел | TC-10 | PASS | +| AC-5 | never-raise, зелёный регресс, CHANGELOG, машина стадий/гейты/схема БД не тронуты | TC-11, TC-12, полный регресс 1437 passed | PASS | + +## Вывод pytest +``` +======================= 1437 passed, 1 warning in 46.89s ======================= +``` +Профильная сюита: +``` +======================== 24 passed, 1 warning in 1.31s ========================= +``` +Регресс трекера (TC-12): +``` +91 passed, 1 warning in 4.32s +``` + +## Итог +PASS — полный регресс зелёный (1437 passed), профильная сюита ORCH-095 зелёная (24 passed), +каждый TC из тест-плана выполнен и сопоставлен с критериями приёмки, smoke API read-only +(`/health`, `/status`, `/queue` с блоками `serial_gate` + `auto_labels`) без регресса. +Обоснованных FAIL/смок-сбоев нет → `result: PASS` → задача переходит на `deploy-staging`. -- 2.49.1 From 2686e3e99f2627f5786a3b11d78c4d7817629e81 Mon Sep 17 00:00:00 2001 From: deploy-finalizer Date: Wed, 10 Jun 2026 00:21:48 +0300 Subject: [PATCH 7/7] deploy(ORCH-036): finalize SUCCESS for ORCH-095 --- docs/work-items/ORCH-095/14-deploy-log.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 docs/work-items/ORCH-095/14-deploy-log.md diff --git a/docs/work-items/ORCH-095/14-deploy-log.md b/docs/work-items/ORCH-095/14-deploy-log.md new file mode 100644 index 0000000..2714ec3 --- /dev/null +++ b/docs/work-items/ORCH-095/14-deploy-log.md @@ -0,0 +1,12 @@ +--- +deploy_status: SUCCESS +work_item: ORCH-095 +hook_exit_code: 0 +deployed_by: deploy-finalizer +--- + +# Deploy log — ORCH-036 executable self-deploy + +Прод-деплой завершён хост-хуком с exit-code `0` -> `deploy_status: SUCCESS`. + +Вердикт зафиксирован детерминированным finalizer'ом (Фаза C), не LLM. -- 2.49.1