From 69dd09a7c992c3853a3f82fe0464982202d63d32 Mon Sep 17 00:00:00 2001 From: Codex Date: Mon, 31 Aug 2026 03:17:55 +0300 Subject: [PATCH] docs: specify the sys.modules guard scope fix (#398) User-Visible: no Issue: #398 --- docs/specs/398-sysmodules-guard-scope.md | 175 +++++++++++++++++++++++ 1 file changed, 175 insertions(+) create mode 100755 docs/specs/398-sysmodules-guard-scope.md diff --git a/docs/specs/398-sysmodules-guard-scope.md b/docs/specs/398-sysmodules-guard-scope.md new file mode 100755 index 00000000..601f33a7 --- /dev/null +++ b/docs/specs/398-sysmodules-guard-scope.md @@ -0,0 +1,175 @@ +# ТЗ #398 — Гвард против отравы `sys.modules` должен ловить свой класс целиком + +- Issue: https://github.com/Matysh/houseplan-card/issues/398 +- Приоритет: P2, infra/tests; полный трек — класс B (тесты и гейт), меняется + правило, а не продуктовый код +- Ревизия: 1 (2026-08-31) + +## Сценарий + +Разработчик добавляет backend-тест, которому нужен модуль интеграции без +поднятого Home Assistant. Пишет привычное «зарегистрирую модуль в +`sys.modules`, чтобы относительные импорты резолвились», гоняет тесты — зелено. +Через день на `dev` красный CI: 85 тестов харнесса падают с голым +`assert False`, а на причину не указывает ничего. Так было дважды: #389 +(`scripts/dump-config-schema.py`) и #42 (`tests_backend/test_backend_quality.py`); +оба раза диагностика стоила часов. + +Гвард против этого класса уже есть (#394), но он ловит только одну форму +записи — и потому сам по себе даёт ложное спокойствие. + +## Что человек увидит до и после + +Пользователь продукта — ничего. Разработчик: попытка зарегистрировать модуль +интеграции в `sys.modules` из тестового файла краснеет сразу и с объяснением, +а не через день в чужой job. + +## Проблема и контракт + +`test/backend-test-hygiene.test.mjs:39` ищет **строковый литерал**: + +```js +const assigns = /sys\.modules\[\s*(['"])custom_components/; +``` + +а `tests_backend/pure_imports.py:31` пишет через переменную: + +```python +sys.modules[name] = module +``` + +и в поле зрения гварда не попадает. Регистрация переживает свой тест — +проверено исполнением: после `import conftest; import test_junction_limits` +в `sys.modules` остаются + +``` +custom_components.houseplan.junction_limits +custom_components.houseplan.wall_segment_model +custom_components.houseplan.coordinate_canonicalization +``` + +(пустышки `custom_components` и `custom_components.houseplan` ставит conftest — +это легитимный условный путь, введённый #394 и сохраняемый). + +Сегодня отказа нет: в CI пакет поднимает Home Assistant, и модуль, загруженный +`load_pure`, совпадает по содержимому с тем, что видит загрузчик HA. Но это +удача порядка импортов — ровно тот же аргумент, который #394 уже признал +негодным основанием («корректность харнесса держалась на именах файлов»). + +**Контракт**: гвард ловит **любую** запись в `sys.modules` по ключу, который +начинается с `custom_components`, независимо от формы выражения — литерал, +переменная, f-строка, конкатенация. Единственное исключение — +`tests_backend/conftest.py`, где подмена условна («нет Home Assistant — нечего +ломать»), как решено в #394. + +Форма проверки меняется с регулярного выражения на разбор синтаксиса Python: +регулярка догоняет формы записи бесконечно, разбор видит сам факт присваивания +в подписку `sys.modules[...]`. Значение ключа при этом может быть неизвестно +статически — тогда гвард обязан отказывать (fail-closed): «не могу доказать, +что ключ не из `custom_components`» — это отказ, а не пропуск. + +**Что делать с `load_pure`.** Два допустимых исхода, выбор — за реализацией +после замера: + +1. **Снимать за собой.** `load_pure` восстанавливает `sys.modules` к состоянию + до вызова (образец — обратимая подмена в `scripts/dump-config-schema.py` + после #389). Плюс: класс закрыт полностью. Минус: относительные импорты + внутри загруженного модуля должны успеть отработать до восстановления — + проверить, что `exec_module` завершается раньше. +2. **Объявить единственным легальным исключением** рядом с conftest, с + комментарием, почему именно ему можно, и с тестом, фиксирующим, что список + исключений состоит ровно из двух файлов и не растёт. + +Предпочтителен (1): исключение, которое нельзя объяснить в одну строку, +завтра станет прецедентом. + +## Скоуп / не-скоуп + +**В скоупе**: `test/backend-test-hygiene.test.mjs`, `tests_backend/pure_imports.py`, +при необходимости — способ загрузки в `tests_backend/test_junction_limits.py` и +`tests_backend/test_validation.py`. + +**Не в скоупе**: условная подмена в `tests_backend/conftest.py` (контракт #394), +правило про `sys.path` (#393), поведение продуктового кода. + +## UX + +Не применимо. + +## Модель данных и миграция + +Не применимо. + +## i18n + +Новых строк нет. + +## Критерии приёмки + +- **AC1**. Файл, записывающий `sys.modules[<переменная>] = …`, где переменная + может содержать имя пакета интеграции, отклоняется гвардом. Доказательство: + тест-контракт на синтетическом файле-нарушителе. +- **AC2**. Формы `sys.modules[f"custom_components.{name}"]`, + `sys.modules["custom_" + "components"]` и присваивание через + `sys.modules.setdefault(...)`/`update(...)` тоже отклоняются либо приводят к + явному отказу «не могу доказать безопасность». Доказательство: по одному + контракту на форму. +- **AC3**. `tests_backend/conftest.py` остаётся разрешённым, и это единственное + исключение (либо два файла, если выбран путь 2 — тогда список + зафиксирован тестом и его рост краснеет). +- **AC4**. После прогона всего `tests_backend/` в `sys.modules` нет ключей + `custom_components*` сверх тех, что положил conftest. Доказательство: + исполняемая проверка, а не чтение исходников. +- **AC5**. Существующие backend-тесты продолжают проходить: относительные + импорты внутри загружаемых модулей (`from .wall_segment_model import …`) + резолвятся, `pytest tests_backend/` зелёный. +- **AC6**. Правило #393 (`sys.path`) и правило #394 (условность подмены в + conftest) не ослаблены: их контракты остаются на месте и продолжают + краснеть на своих нарушителях. + +## План автотестов + +**Unit** (`test/backend-test-hygiene.test.mjs`): + +1. Синтетический файл с записью через переменную → гвард краснеет (AC1). +2. По контракту на каждую форму из AC2. +3. `conftest.py` со своей условной подменой → зелено (AC3). +4. Попытка добавить третий файл в список исключений (если выбран путь 2) → + красный. + +**Backend** (`tests_backend/test_backend_quality.py` или новый файл): + +5. Исполняемая проверка AC4: собрать `sys.modules` до и после прогона набора, + разница пуста сверх conftest. + +**Мутант** (`scripts/mutation-gate.mjs`): + +- `sysmodules-guard-blind-to-variable`: вернуть запись через переменную в + `pure_imports.py` → гвард красный. + +## Риски + +- **Разбор Python из Node.** Полноценный парсер тянуть не нужно и нельзя: + достаточно узкого разбора строк вида «присваивание в подписку `sys.modules`» + с fail-closed на всё непонятое. Риск — ложные срабатывания на комментариях и + строковых литералах; смягчение: тест-контракты на «похожий, но безопасный» + код (например, `# sys.modules[...]` в комментарии) должны оставаться + зелёными. +- **Снятие регистрации ломает относительные импорты.** Если выбран путь (1) и + восстановление происходит слишком рано, `from .x import y` внутри модуля + упадёт. Смягчение: восстановление строго после `exec_module`, AC5 покрывает + оба существующих места использования. +- **Скрытая зависимость от повторного импорта.** Модуль, загруженный дважды, + даёт два объекта классов; тесты, сравнивающие типы, могли на этом молча + держаться. Смягчение: AC5 гоняет весь набор, а не отдельный файл. + +## Откат + +Возврат регулярки и прежнего `load_pure` — два коммита назад, продуктовый код +не затронут. + +## Release-артефакты + +Пользовательских изменений нет: `User-Visible: no`, changelog не трогается. +`docs/ARCHITECTURE.md` — при необходимости одна строка в разделе про гейты +бэкенда о том, что гвард разбирает синтаксис, а не текст.