architect(ET): auto-commit from architect run_id=717
All checks were successful
CI / test (push) Successful in 1m10s
All checks were successful
CI / test (push) Successful in 1m10s
This commit is contained in:
@@ -0,0 +1,251 @@
|
||||
---
|
||||
work_item: ORCH-117
|
||||
stage: architecture
|
||||
author_agent: architect
|
||||
status: proposed
|
||||
created_at: 2026-06-15
|
||||
model_used: claude-opus-4-8
|
||||
---
|
||||
|
||||
# ADR-001: Sandbox-only fail-closed гард записи в Plane из тест-процесса
|
||||
|
||||
Work Item: **ORCH-117** — test/staging Plane writes must be sandbox-only and never mutate prod
|
||||
Стадия: **architecture**
|
||||
Сквозная регистрация: **`docs/architecture/adr/adr-0046-sandbox-only-plane-write-guard.md`**
|
||||
(кросс-каттинговое: вводит новый рантайм-leaf поверх **общего** Plane-клиента `plane_sync`,
|
||||
используемого ВСЕМИ проектами общего инстанса, + новый тест-харнесс-инвариант в `conftest.py`).
|
||||
|
||||
## Статус
|
||||
Proposed
|
||||
|
||||
## Контекст
|
||||
|
||||
**Инцидент (установленный факт, ORCH-114).** Тестовый/worktree-процесс выполнил РЕАЛЬНУЮ запись в
|
||||
Plane против **боевого** проекта ORCH: `PATCH …/issues/dd57ad23… state=<Done>` + комментарий
|
||||
«Stage: deploy → done». То есть `notify_stage_change("ORCH-114","deploy","done")`, запущенный из
|
||||
pytest, смутировал боевую задачу — «ложный Done» на боевой доске (источник индикации, слой B
|
||||
ORCH-066).
|
||||
|
||||
**Корень (сверено по коду).** Тест/staging-процессы имеют доступ к живому боевому Plane-токену и
|
||||
**ничто не принуждает** их писать только в sandbox:
|
||||
|
||||
- `src/plane_sync.py:17` `PLANE_HEADERS = {"X-API-Key": settings.plane_api_token}` и `:57`
|
||||
`PROJECT_ID = settings.plane_project_id or "7a79f0a9-…"` (боевой ORCH) **захватываются на импорте
|
||||
модуля** → подмена env/токена постфактум бесполезна (NFR-4).
|
||||
- Тестовые `os.environ.setdefault("ORCH_PLANE_API_TOKEN","test-token")` — **no-op** в контейнере с
|
||||
уже установленной боевой переменной → тесты наследуют **реальный** токен.
|
||||
- Все мутирующие записи сходятся в **три** примитива: `update_issue_state` (`httpx.patch`, стр. 861),
|
||||
`add_comment` (`httpx.post`, стр. 885), `_set_issue_state_direct` (`httpx.patch`, стр. 1047) — и
|
||||
**ни один** не проверяет тест-контекст и легитимность целевого проекта.
|
||||
|
||||
**Прецедент в репозитории.** `tests/conftest.py::_no_telegram` — autouse-фикстура, глушащая
|
||||
`send_telegram` во ВСЕХ тестах, ровно потому что «pytest на проде слал РЕАЛЬНЫЕ Telegram-сообщения
|
||||
Славе». Симметричной защиты для **Plane-записи** не было — это пробел того же класса, реализованный
|
||||
ORCH-114. Идентификатор sandbox уже зафиксирован: `scripts/staging_check.py:283`
|
||||
`SANDBOX_PROJECT_ID = "8c5a3025-4f9d-4190-b79f-fa06276bb27e"`.
|
||||
|
||||
**Почему «как есть» не годится.** Устойчивость стоит на стороне тестов (надеяться, что каждый тест
|
||||
замокает Plane), а не на стороне системы. Любой новый/будущий путь записи, забывший мок, снова
|
||||
смутирует боевую доску. Требуется **fail-closed**-инвариант: запись в боевой проект из
|
||||
тест/worktree-процесса должна быть **физически невозможна**, независимо от токена в окружении.
|
||||
|
||||
## Решение
|
||||
|
||||
### Сводка
|
||||
|
||||
Вводим **fail-closed гард записи в Plane на низком чокпоинте** — на входе трёх примитивов записи
|
||||
`plane_sync`, **в момент вызова** (не на импорте). Чистую логику держит **новый leaf
|
||||
`src/plane_write_guard.py`** (never-raise, по образцу `serial_gate`/`cancel`/`deploy_status_guard`):
|
||||
`decide(project_id, op, work_item_id) -> (ALLOW | BLOCK, reason)`. Гард активен **только в
|
||||
тест-процессе** (детект `pytest`-в-процессе) — для боевого и staging рантайма он **no-op**
|
||||
(byte-for-byte, NFR-2/NFR-3). В тест-процессе запись разрешена **исключительно** при
|
||||
одновременном (а) включённом opt-in-флаге **и** (б) целевом проекте ∈ sandbox-allowlist; иначе —
|
||||
блок (default-deny). Дополнительно — **независимый conftest-floor** (autouse-фикстура), который
|
||||
форсит безопасные дефолты во ВСЕХ тестах (BR-4). Изменение — bugfix-изоляция: **не** Quality Gate и
|
||||
**не** стадия; `STAGE_TRANSITIONS`/`QG_CHECKS`/`check_*`/machine-verdict/схема БД — байт-в-байт не
|
||||
тронуты.
|
||||
|
||||
### D1 — Точка перехвата: низкий чокпоинт в 3 примитивах `plane_sync`, на момент вызова
|
||||
|
||||
Гард врезается в `update_issue_state` / `add_comment` / `_set_issue_state_direct` **сразу после**
|
||||
`_resolve_project_id(...)` (локальное чтение БД) и **до** любого сетевого шага (`stage_to_state`,
|
||||
`find_issue_id`, `httpx.patch/post`):
|
||||
|
||||
```python
|
||||
project_id = _resolve_project_id(work_item_id, project_id)
|
||||
ok, reason = plane_write_guard.decide(project_id, "state", work_item_id) # op ∈ {"state","comment"}
|
||||
if not ok:
|
||||
plane_write_guard.audit_block(project_id, "state", work_item_id, reason)
|
||||
return # никакой сети — ни GET, ни PATCH/POST
|
||||
# ... обычный путь (ALLOW)
|
||||
```
|
||||
|
||||
**Почему этот чокпоинт (а не обёртка `httpx` и не только autouse-фикстура):**
|
||||
|
||||
- **Низкий и полный.** Все `set_issue_*`/`notify_*` сводятся к этим трём примитивам (верифицировано
|
||||
BRD §6) → один гард ловит любой путь, включая будущие (тот же довод, что у `deploy_status_guard` на
|
||||
входе сеттеров).
|
||||
- **На момент вызова → иммунитет к захвату на импорте (NFR-4/AC-7).** `PLANE_HEADERS`/`PROJECT_ID`
|
||||
захвачены литералами на импорте; гард читает контекст (тест-процесс + резолвенный `project_id`) при
|
||||
каждом вызове, поэтому защита не зависит от того, какой токен в `PLANE_HEADERS`, и не опирается на
|
||||
неработающий `os.environ.setdefault`.
|
||||
- **До сети.** Размещение до `find_issue_id`/`stage_to_state` исключает даже «безобидный» GET в
|
||||
боевой workspace из тестов.
|
||||
- Обёртка над транспортом `httpx` отвергнута (см. «Альтернативы») — она ниже уровня, на котором
|
||||
известны `project_id`/`work_item`/`op` для аудита, и хрупка к смене HTTP-клиента.
|
||||
|
||||
### D2 — Детект тест-процесса: `pytest`-в-процессе (call-time)
|
||||
|
||||
```python
|
||||
def _in_test_process() -> bool:
|
||||
import sys, os
|
||||
return ("pytest" in sys.modules) or bool(os.environ.get("PYTEST_CURRENT_TEST"))
|
||||
```
|
||||
|
||||
- **`"pytest" in sys.modules`** истинно на всём прогоне pytest (collection + выполнение) в **этом**
|
||||
процессе. Боевой и staging рантаймы запускаются `uvicorn src.main:app` и **не импортируют** pytest
|
||||
в свой процесс → детект там **никогда** не срабатывает (NFR-2/NFR-3, AC-5/AC-6). pytest установлен в
|
||||
образе (для merge-gate/coverage re-test), но запускается **отдельным субпроцессом** `python -m
|
||||
pytest` — он-то и есть worktree-тест-процесс из инцидента, и его гард **должен** ловить (✓).
|
||||
- **`PYTEST_CURRENT_TEST`** — вторичный сигнал (выставляется pytest на время тела теста), добавлен как
|
||||
дешёвый подтверждающий признак; основной — `sys.modules`.
|
||||
- Оба читаются **в момент вызова** (NFR-4). Признак консервативный: ложноположительное срабатывание в
|
||||
боевом рантайме требует, чтобы кто-то импортировал `pytest` в процесс uvicorn — чего штатный
|
||||
entrypoint не делает (зафиксировано как допущение, см. `10-tech-risks.md` TR-1).
|
||||
|
||||
### D3 — Решение `decide`: default-deny, sandbox-allowlist, опт-ин
|
||||
|
||||
`decide(project_id, op, work_item_id) -> (bool ok, str reason)` — чистая, never-raise:
|
||||
|
||||
| Шаг | Условие | Исход | reason |
|
||||
|-----|---------|-------|--------|
|
||||
| 1 | `not _in_test_process()` | **ALLOW** | `live-runtime` (прод/staging — no-op, NFR-2/3) |
|
||||
| 2 | `project_id` пуст/None/нерезолвим | **BLOCK** | `ambiguous-target` (NFR-1: «не знаю → не пишу») |
|
||||
| 3 | `not settings.plane_test_write_enabled` | **BLOCK** | `opt-in-disabled` |
|
||||
| 4 | `project_id ∉ sandbox_allowlist` | **BLOCK** | `prod-project-in-test` |
|
||||
| 5 | иначе | **ALLOW** | `sandbox-opt-in` (audit INFO) |
|
||||
|
||||
- **Allowlist sandbox-only (BR-2, AC-3).** Шаг 4 запрещает боевой ORCH (`7a79f0a9-…`) и любой боевой
|
||||
enduro-проект **даже при включённом opt-in** — opt-in лишь снимает шаг 3, allowlist остаётся
|
||||
жёстким полом.
|
||||
- **Fail-closed по неопределённости (NFR-1, AC-4).** Нерезолвимый целевой проект → блок (шаг 2).
|
||||
- **never-raise (NFR-5).** Любое внутреннее исключение `decide` интерпретируется вызывающим примитивом
|
||||
по контексту: в боевом пути это уже ALLOW (шаг 1 не достигнут — `_in_test_process` False); в
|
||||
тест-пути исключение трактуется как **BLOCK** (громко, fail-closed) — дефект всплывает, а не
|
||||
маскируется. (Реализация: `decide` ловит свои ошибки и в тест-контексте возвращает
|
||||
`(False, "guard-error")`, в live-контексте — `(True, …)`.)
|
||||
|
||||
### D4 — Kill-switch: его нет (умышленно, NFR-6/FR-7) — реверс через opt-in
|
||||
|
||||
**Сознательное расхождение с конвенцией «у каждого leaf есть `*_enabled` kill-switch».** Гард,
|
||||
делающий прод-запись из pytest *физически невозможной*, **не должен** поставляться с конфигом,
|
||||
который её переоткрывает — это и есть «чёрный ход», запрещённый NFR-6. Прямой прецедент в репозитории:
|
||||
`_no_telegram` тоже **не имеет** флага «разрешить реальный Telegram в тестах» — это безусловный
|
||||
страховочный пол.
|
||||
|
||||
- **Единственный реверсивный регулятор — opt-in** `plane_test_write_enabled` (default `False` =
|
||||
безопасно) + allowlist `plane_test_sandbox_projects` (default = единственный SANDBOX id). Он
|
||||
управляет **только sandbox-записью**; его off-состояние — безопасный дефолт, on-состояние —
|
||||
sandbox-bound. «Выключить защиту» ≠ «разрешить прод из pytest»: такого перехода в дизайне **нет**.
|
||||
- Рантайм-leaf **инертен в боевом рантайме** по построению (`_in_test_process()` False) → отдельный
|
||||
«выключатель» для безопасности прода не нужен; leaf never-raises → не может уронить боевой путь.
|
||||
- **Норматив на будущее (анти-дрейф):** не добавлять «общий kill-switch гарда», обнуляющий
|
||||
prod-блок в тест-процессе — это реинтродуцирует дефект ORCH-114. Зафиксировано в `10-tech-risks.md`
|
||||
(TR-4) и в сквозном adr-0046.
|
||||
|
||||
### D5 — Conftest-floor: независимый default-deny во всех тестах (BR-4, FR-3)
|
||||
|
||||
Autouse-фикстура `tests/conftest.py::_plane_sandbox_only` (по образцу `_reset_webhook_secrets` /
|
||||
`_disable_merge_verify`) форсит безопасные дефолты для **каждого** теста через `monkeypatch`,
|
||||
**перекрывая** любую боевую переменную, унаследованную из окружения контейнера:
|
||||
|
||||
```python
|
||||
@pytest.fixture(autouse=True)
|
||||
def _plane_sandbox_only(monkeypatch):
|
||||
from src import config as _cfg
|
||||
monkeypatch.setattr(_cfg.settings, "plane_test_write_enabled", False, raising=False)
|
||||
monkeypatch.setattr(_cfg.settings, "plane_test_sandbox_projects",
|
||||
"8c5a3025-4f9d-4190-b79f-fa06276bb27e", raising=False)
|
||||
yield
|
||||
```
|
||||
|
||||
- С opt-in `False` гард блокирует **все** записи в тестах (и sandbox, и прод) → AC-4 default-deny.
|
||||
- **Sandbox-e2e переопределяет** в собственной фикстуре *после* autouse (точно как
|
||||
`test_merge_verify`/`test_orch114_*` ре-энейблят свои флаги): `plane_test_write_enabled=True`
|
||||
(+ allowlist уже содержит sandbox) → запись в SANDBOX проходит (AC-2), в прод — по-прежнему блок
|
||||
(allowlist, AC-3).
|
||||
- Floor **независим от рантайм-логики**: даже если рантайм-leaf по ошибке вернёт ALLOW, инвариант
|
||||
«opt-in off» делает прод-запись из обычного pytest невозможной. Два слоя, оба sandbox-bound →
|
||||
ни один не способен разрешить прод-запись из pytest (двойной NFR-6).
|
||||
|
||||
### D6 — Конфиг-ключи
|
||||
|
||||
В `src/config.py` (дефолты = безопасные):
|
||||
|
||||
| Ключ | Env | Дефолт | Назначение |
|
||||
|------|-----|--------|------------|
|
||||
| `plane_test_write_enabled` | `ORCH_PLANE_TEST_WRITE_ENABLED` | `False` | opt-in реальной записи из тест-процесса |
|
||||
| `plane_test_sandbox_projects` | `ORCH_PLANE_TEST_SANDBOX_PROJECTS` | `"8c5a3025-4f9d-4190-b79f-fa06276bb27e"` | CSV allowlist sandbox-проектов |
|
||||
|
||||
- **НЕ `*_repos`-scope.** В отличие от гейт-leaf'ов (`serial_gate`/`coverage_gate` *действуют* на
|
||||
репо), этот гард защищает запись в **любой** боевой проект общего workspace (включая боевой enduro,
|
||||
BR-2). Регуляторов scope по репо нет; единственные гейты — `_in_test_process()` (рантайм) + opt-in
|
||||
(как у observer-leaf `lessons`, который тоже не скоупится по репо). Зафиксировано в adr-0046.
|
||||
- `.env.example` дополняется обоими ключами с безопасными дефолтами (deliverable developer'а).
|
||||
|
||||
### D7 — Аудит/наблюдаемость (BR-6, FR-5, AC-8)
|
||||
|
||||
- **Блок** → структурный `logger.warning`/`error` с полями `project_id`, `work_item`, `op`
|
||||
(`state`/`comment`), `reason` (`prod-project-in-test`/`opt-in-disabled`/`ambiguous-target`/
|
||||
`guard-error`). Формулировка делает инцидент класса ORCH-114 **очевидным** (не молчаливым).
|
||||
- **Разрешённая sandbox-запись** → audit `logger.info` с `project_id` и `op`.
|
||||
- Новый эндпоинт **не вводится** (TRZ §4): состояние гарда (флаг/allowlist) при желании дорисовывается
|
||||
read-only в `GET /queue` — **необязательно**, оставлено на усмотрение developer'а.
|
||||
|
||||
## Альтернативы
|
||||
|
||||
- **Только autouse-фикстура в `conftest.py` (без рантайм-leaf)** — отвергнуто: не делает прод-запись
|
||||
*физически невозможной* (AC-7) — любой путь, обошедший мок/фикстуру (прямой импорт `plane_sync` в
|
||||
скрипте под pytest, sandbox-e2e с опечаткой проекта), снова смутирует прод. Нужен рантайм-floor на
|
||||
момент вызова. Фикстура остаётся как **дополнительный** независимый слой (D5), не единственный.
|
||||
- **Обёртка/монки над `httpx`-транспортом** — отвергнуто: уровень ниже, чем известны
|
||||
`project_id`/`work_item`/`op` (хуже аудит); хрупко к смене HTTP-клиента; ловит и легитимные GET.
|
||||
Низкий чокпоинт в примитивах точнее и стабильнее.
|
||||
- **Подмена `settings.plane_api_token`/env на тестовый токен** — отвергнуто прямо BRD/NFR-4:
|
||||
`PLANE_HEADERS`/`PROJECT_ID` захвачены на импорте → подмена постфактум бесполезна; `setdefault`
|
||||
no-op в проде. Не лечит корень.
|
||||
- **Гонять тесты в sandbox-проекте по дефолту (сменить дефолтный `PROJECT_ID`)** — отвергнуто: не
|
||||
fail-closed (живой токен + забытый мок всё равно может адресовать прод явным `project_id`); не
|
||||
отличает прод от sandbox по *намерению*.
|
||||
- **Конвенциональный `plane_write_guard_enabled` kill-switch** — отвергнуто (D4): его off-состояние
|
||||
было бы «чёрным ходом» к прод-записи из pytest (NFR-6). Реверс обеспечивает opt-in.
|
||||
|
||||
## Последствия
|
||||
|
||||
- **+** Прод-запись в Plane из любого pytest/worktree-процесса **физически невозможна** независимо от
|
||||
токена (AC-1/AC-7); инцидент класса ORCH-114 закрыт у источника и стал **видимым** (аудит).
|
||||
- **+** Боевой и staging рантаймы — **байт-в-байт** (no-op гарда, `_in_test_process()` False);
|
||||
`STAGE_TRANSITIONS`/`QG_CHECKS`/`check_*`/схема БД не тронуты (NFR-2/NFR-3, AC-5/AC-6).
|
||||
- **+** Два независимых sandbox-bound слоя (рантайм-leaf + conftest-floor) → нет одиночной точки, чьё
|
||||
выключение переоткрывает прод (NFR-6).
|
||||
- **+** Sandbox-e2e сохранён (opt-in + allowlist, AC-2); `scripts/staging_check.py` Block C
|
||||
работоспособен.
|
||||
- **−** Детект тест-процесса завязан на «pytest-в-процессе» → теоретический ложноположительный риск,
|
||||
если кто-то импортирует `pytest` в боевой uvicorn-процесс (не делает штатный entrypoint). Митигейшн:
|
||||
TR-1, консервативный признак, аудит-видимость; в худшем случае под opt-in остаётся sandbox-запись.
|
||||
- **−** Намеренный отказ от kill-switch расходится с привычной конвенцией → требует явной фиксации,
|
||||
чтобы будущий агент не «добавил выключатель» (D4-норматив, TR-4, adr-0046).
|
||||
- **Откат:** удалить врезку гарда из 3 примитивов + autouse-фикстуру + 2 конфиг-ключа → поведение
|
||||
байт-в-байт до ORCH-117 (дефект возвращается). Частичный «мягкий» откат (oct-in `True` глобально) —
|
||||
**запрещён** как небезопасный (вернёт прод-риск только при условии allowlist; всё равно
|
||||
sandbox-bound).
|
||||
|
||||
## Ссылки
|
||||
- BRD: `docs/work-items/ORCH-117/01-brd.md`
|
||||
- TRZ: `docs/work-items/ORCH-117/02-trz.md`
|
||||
- Acceptance: `docs/work-items/ORCH-117/03-acceptance-criteria.md`
|
||||
- Tech-risks: `docs/work-items/ORCH-117/10-tech-risks.md`
|
||||
- Сквозной ADR: `docs/architecture/adr/adr-0046-sandbox-only-plane-write-guard.md`
|
||||
- Сверено по коду: `src/plane_sync.py:17,57,846-889,1038-1051`, `tests/conftest.py` (`_no_telegram`),
|
||||
`scripts/staging_check.py:283`, `src/deploy_status_guard.py` (образец leaf), `src/config.py`
|
||||
- Прецедент-инвариант: `_no_telegram` (autouse safety-floor), `docs/_standards/TRACEABILITY.md`
|
||||
Reference in New Issue
Block a user