# Аудит репозитория krona-staff (третий проход) - Дата: 2026-09-18 - Ревизия: `7c8e7fc` — ветка `codex/bootstrap-platform-foundation`, 9 коммитов - Предыдущие проходы: `14ed610` (первый отчёт), `b88ff91` (второй отчёт) - Репозиторий: https://git.ochenstarik.ru/ochenstarik/krona-staff ## 1. Что изменилось Два новых коммита, 53 файла, +1725/−1069 строк: - `64ac5c0 fix: enforce tenant isolation and runtime contracts` — Node-сервис, БД, Docker, CI; - `7c8e7fc fix: close terminal audit findings` — терминал, документация, лицензия ZKTeco. Добавлен `docs/AUDIT-REMEDIATION-2026-09-18.md` — ответ на **первый** отчёт (ревизия `14ed610`). Замечания второго отчёта (Н1–Н10, ревизия `b88ff91`) в нём не учтены: коммиты готовились до него. Главное из сделанного: - разделены роли PostgreSQL: `krona_migrator` (миграции) и `krona_staff_app` (`NOSUPERUSER NOBYPASSRLS`); - мигратор ведёт `schema_migrations`, берёт advisory lock, откатывает только последнюю миграцию и только при `ALLOW_MIGRATION_DOWN=true`; - миграция `0002` добавляет `subject`/`actor` в outbox, `CHECK` на ULID и делает аудит-лог доступным приложению только на чтение и добавление; - маршруты перенесены под `/api/v1/staff`, `/ready` отвечает `503` без деталей, `x-correlation-id` проверяется как ULID и возвращается в ответе; - появились интеграционные тесты: реальная проверка изоляции тенантов на PostgreSQL и сверка OpenAPI с фактическими маршрутами; таймаут тестов поднят до 15 с; - Redis и BullMQ удалены из зависимостей и compose; образы Docker закреплены по digest; `format:check` и миграции добавлены в CI; - в терминале: экспорт журнала в CSV, маскирование имён на публичном экране, настраиваемая максимальная длительность смены, ограничение журнала в 100 000 записей, глобальные обработчики необработанных исключений, ротация `crash.log`, автозагрузка выключена по умолчанию, очищен `Package.appxmanifest`, добавлена EULA ZKTeco. ## 2. Проверки | Проверка | Результат | | -------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | | `npm ci` | OK | | `npm run check` (lint, typecheck, тесты, миграции, контракты, build) | **Пройдено полностью** | | `npm run format:check` | Пройдено по коду репозитория (замечание только к этому файлу отчёта, он не в репозитории) | | `npm audit --audit-level=moderate` | 0 уязвимостей | | `dotnet build` терминала (Release, x64) | OK, 0 ошибок, 0 предупреждений | | Запрос `GET /live` (корень) | **404** — цель Docker HEALTHCHECK, см. Н11 | | Запрос `GET /api/v1/staff/live` | 200 | | Запрос `GET /api/v1/staff/ready` при сбое БД | 503, тело `{"status":"unavailable"}`, текст ошибки БД не раскрывается | | Некорректный `x-correlation-id` | Заменяется на сгенерированный ULID | | `NODE_ENV=production` + `standalone` | Запуск запрещён с понятной ошибкой | **Не проверялось:** Docker, PostgreSQL и сканер SLK20R на машине аудита отсутствуют. Тест изоляции тенантов локально пропускается (`DATABASE_TEST_URL` не задан) — в CI он выполняется, но результата прогона CI я не видел. ## 3. Сводка | Категория | Итог | | ----------------------------------- | --------------------------------------------------------- | | Закрыто из первого отчёта | 24 находки из 33 + сознательно оставлена №24 | | Осталось из второго отчёта (Н1–Н10) | 9 открыто, 1 частично | | Новые находки | 11 (2 высоких, 5 средних, 4 низких) | | Архитектура и право (A1, A2) | Вынесены владельцу, зафиксированы в документе исправлений | ## 4. Новые находки ### Н11. 🟠 Docker HEALTHCHECK проверяет несуществующий маршрут - **Где:** `Dockerfile:21` - **Суть:** после переноса маршрутов под `/api/v1/staff` проверка осталась на `http://127.0.0.1:8080/live`. Проверено запросом: корневой `/live` теперь отдаёт 404, `wget` вернёт ошибку. - **Последствия:** контейнер всегда в состоянии `unhealthy`. В Docker Compose это ломает `depends_on: condition: service_healthy`, в оркестраторах (Swarm, Kubernetes при аналогичной пробе) — приводит к бесконечному перезапуску или к тому, что контейнер не попадает в балансировку. Автоматическая проверка развёртывания при этом молчит. - **Исправление:** `CMD wget -q -O - http://127.0.0.1:8080/api/v1/staff/live || exit 1`; добавить healthcheck для сервиса `app` в `compose.yaml` и тест, который сверяет путь из `Dockerfile` с маршрутами. ### Н12. 🟠 Контейнер приложения получает учётные данные мигратора - **Где:** `compose.yaml:4`, `.env.example:5-8` - **Суть:** у сервиса `app` указан `env_file: .env`, а в `.env` лежат `DATABASE_MIGRATION_URL`, `POSTGRES_MIGRATION_PASSWORD` и `POSTGRES_APP_PASSWORD`. Переопределение `DATABASE_URL` не убирает остальные переменные из окружения контейнера. - **Последствия:** пароль роли-владельца схемы доступен процессу приложения и любому, кто прочитает `/proc/1/environ` или `docker inspect`. Разделение ролей, ради которого сделана находка №6, частично обесценивается. `docs/DEPLOYMENT.md` прямо утверждает обратное: «миграционный URL хранится отдельно и не передаётся runtime-контейнеру». - **Исправление:** отдельные файлы `.env.app` и `.env.migrate`, либо перечислить нужные переменные в `environment:` без `env_file`. ### Н13. 🟡 Обновление существующей установки ломается - **Где:** `compose.yaml:23-32`, `docker/postgres/init-app-role.sh` - **Суть:** `POSTGRES_USER` изменён с `krona_staff` на `krona_migrator`, а роль приложения создаётся скриптом из `docker-entrypoint-initdb.d`, который выполняется только при инициализации пустого каталога данных. На уже существующем томе: - роль `krona_staff_app` не появится; - блок `GRANT` в миграциях обёрнут в `IF EXISTS (SELECT 1 FROM pg_roles ...)` и молча ничего не сделает; - приложение не сможет подключиться, в кластере останется прежний суперпользователь `krona_staff`. - **Исправление:** выполнять `scripts/create-app-role.mjs` в сервисе `migrate` перед миграциями и описать порядок обновления в `docs/DEPLOYMENT.md`. ### Н14. 🟡 Права выдаются «если роль есть» и не распространяются на будущие таблицы - **Где:** `migrations/0001_infrastructure.up.sql:80-89`, `migrations/0002_security_hardening.up.sql:59-68` - **Суть:** при отсутствии роли миграция завершается успешно, но без выдачи прав — ошибка проявится позже как `permission denied` в рантайме. `ALTER DEFAULT PRIVILEGES` не настроен, поэтому каждая новая миграция обязана повторять блок `GRANT`; забытый блок — новая таблица, недоступная приложению. - **Исправление:** падать с понятной ошибкой, если роли нет; настроить `ALTER DEFAULT PRIVILEGES FOR ROLE krona_migrator IN SCHEMA public`. ### Н15. 🟡 Новое правило «Приход/Уход» теряет длинные смены и не имеет ручной корректировки - **Где:** `MainPage.xaml.cs:586-588` - **Суть:** «Уход» проставляется, только если предыдущее событие — «Приход» и с него прошло не больше `MaximumShiftHours` (по умолчанию 20 ч). Переход через полночь теперь обрабатывается верно (№14 закрыта), но появились новые случаи: - смена длиннее настроенного значения (суточное дежурство при значении 20 ч) даёт второй «Приход» вместо «Ухода»; - если сотрудник не отметил уход, «Приход» остаётся незакрытым навсегда, и следующая отметка тоже станет «Приходом»; - исправить ошибочную запись в интерфейсе нельзя — доступна только полная очистка журнала. - **Исправление:** редактирование или удаление отдельной записи под администратором с пометкой о ручной правке; предупреждение при незакрытом «Приходе». ### Н16. 🟡 Ограничение журнала в 100 000 записей делает сохранение очень дорогим - **Где:** `MainPage.xaml.cs:599-601`, `Services/TerminalStore.cs:127` - **Суть:** предел в 100 000 записей — это десятки мегабайт JSON с отступами. Каждая отметка выполняет два сохранения, каждое сохранение полностью сериализует состояние, шифрует его, пишет с принудительным сбросом на диск и копирует три резервные копии. То есть на одну отметку приходится до восьми полных записей файла. - Записи сверх лимита удаляются молча, без архива и без предложения выгрузить CSV. - **Исправление:** перейти на журнал с добавлением записей (SQLite или файл-журнал), либо хранить в состоянии только последние отметки, а историю писать отдельно; перед усечением требовать экспорт. ### Н17. 🟡 Приватный режим скрывает имена только в списке - **Где:** `MainPage.xaml.cs:581`, `:605` - **Суть:** при включённом маскировании (по умолчанию) список последних отметок показывает «И\*\*\* П\*\*\*», но крупный текст на экране терминала по-прежнему выводит полное имя: «Иван Петров / Приход отмечен в 09:14», то же при отклонении повторного скана. - **Последствия:** цель находки №26 не достигнута — имя всё равно видно постороннему у терминала, причём более заметно, чем в списке. - **Исправление:** применять то же маскирование к `TerminalMessage` и к сообщениям строки состояния в публичном режиме. ### Н18. ⚪ Экспорт CSV не фиксируется и не защищён - **Где:** `MainPage.xaml.cs:641-667` - **Суть:** экспорт выгружает полные имена, табельные номера, должности и все отметки в произвольный незашифрованный файл. Факт экспорта нигде не сохраняется. - **Исправление:** записывать в журнал событие экспорта (кто, когда, сколько записей); предупреждать, что файл не зашифрован. ### Н19. ⚪ Изменена уже применённая миграция, контрольные суммы не проверяются - **Где:** `migrations/0001_infrastructure.up.sql`, `scripts/migrate.mjs` - **Суть:** файл `0001` изменён задним числом (в `CREATE TABLE` добавлены `subject_*`/`actor_*` и `CHECK`), при этом на уже мигрированных базах таблица создана по старой версии, а колонки добавляет `0002`. Сейчас расхождение компенсируется, но мигратор не хранит контрольные суммы, поэтому следующая правка применённого файла останется незамеченной. - **Исправление:** хранить хеш файла в `schema_migrations` и падать при расхождении. ### Н20. ⚪ Подстановка Release вместо Debug в пути автозагрузки - **Где:** `Services/StartupService.cs:25-38` - **Суть:** если путь процесса содержит `\Debug\`, он заменяется на `\Release\`, а при отсутствии файла выбрасывается исключение. Установка в каталог, в названии которого есть «Debug», сломает автозагрузку; подставленный Release-файл может быть старой сборкой. - **Исправление:** определять отладочную сборку по `#if DEBUG` или по атрибуту сборки, а не по пути. ### Н21. ⚪ ADR-001 не синхронизирован с кодом - **Где:** `docs/adr/ADR-001-technology-and-runtime-modes.md:8`, `:12` - **Суть:** ADR по-прежнему фиксирует Redis/BullMQ как принятое решение, хотя зависимости и сервис удалены; `AGENTS.md` требует синхронизировать ADR при изменении контракта. `docs/ARCHITECTURE.md` обновлён, ADR — нет. - **Исправление:** дополнить ADR-001 или выпустить ADR-002 «Redis откладывается до появления сценария очередей». ## 5. Находки второго отчёта: почти все открыты `Services/TerminalStore.cs` в новых коммитах **не менялся**, поэтому связанные находки в силе. | № | Находка | Статус | | --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------- | | Н1 | 🔴 После переименования повреждённого файла следующий запуск стартует с пустым состоянием: терминал открыт для создания чужого PIN, резервные копии затираются через 3–4 сохранения. То же при простом удалении файла состояния | ❌ Открыто | | Н2 | 🟠 Любая ошибка чтения, включая временную блокировку файла антивирусом, считается повреждением; откат к резервной копии происходит без предупреждения | ❌ Открыто | | Н3 | 🟡 Резервные копии обновляются на каждом сохранении и покрывают 1–2 скана; ошибочная очистка журнала попадает во все копии | ❌ Открыто, усугублено Н16 | | Н4 | 🟡 `GetDeviceCount() - 1` не проверен на оборудовании | ❌ Открыто | | Н5 | 🟡 Счётчик неверных PIN живёт в памяти, задержка блокирует и вход по отпечатку, старые хеши не пересчитываются | ❌ Открыто | | Н6 | ⚪ `Disconnect` освобождает дескрипторы после 2 с ожидания независимо от результата | ❌ Открыто | | Н7 | ⚪ Самопроизвольная отмена регистрации | 🟨 Частично: добавлено событие `RegistrationCancelled`, но `RefreshLists()` по-прежнему сбрасывает выбор и отменяет регистрацию | | Н8 | ⚪ Редактирование сотрудника переписывает имена в прошлых отметках | ❌ Открыто | | Н9 | ⚪ При недоступном хранилище приложение нельзя закрыть штатно | ❌ Открыто | | Н10 | ⚪ Блокирующие циклы в пуле потоков, гонка при освобождении события активации, мьютекс в пространстве `Local\` | ❌ Открыто | ## 6. Находки первого отчёта: статус ### Закрыто | № | Находка | Как закрыто | | ---------- | ------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | | 1, 4, 5 | Регистрация отпечатка, гонка дескрипторов, потеря связи со сканером | Отмена регистрации, передача дескрипторов, контроль связи (остаточные Н4, Н6) | | 3 | Гонка записи состояния | `SemaphoreSlim`, `WriteThrough` | | 6 | RLS обходился суперпользователем | Роль `krona_staff_app` с `NOSUPERUSER NOBYPASSRLS`, тест проверяет `rolsuper`/`rolbypassrls` | | 7 | Несовпадение паролей в быстром старте | Пароли и URL согласованы | | 8 | Мигратор | `schema_migrations`, advisory lock, откат только последней, `ALLOW_MIGRATION_DOWN` | | 9 | Потеря отметок при открытом диалоге | Отметка записывается | | 11, 12 | Второй экземпляр, дубликат пальца | Мьютекс с активацией, `DBIdentify` перед регистрацией | | 13 | Журнал без экспорта и без границ | CSV-экспорт и лимит (новые замечания Н16, Н18) | | 14 | Смена через полночь | Настраиваемая максимальная длительность смены (новое замечание Н15) | | 15 | `/ready` отдавал 500 с текстом ошибки | 503 без деталей, проверено | | 16 | Расхождение OpenAPI и маршрутов | Префикс `/api/v1/staff` и тест сверки | | 17 | Пул PostgreSQL | `pool.on('error')`, `release(error)`, защищённый `ROLLBACK` | | 18 | Тесты не проверяли инварианты | Реальный тест изоляции двух тенантов, сверка OpenAPI, таймаут 15 с | | 19 | Аудит-лог и outbox | Политики `SELECT`/`INSERT`, `subject`/`actor`, `CHECK` на ULID | | 20 | Неиспользуемый Redis | Удалён из зависимостей, compose и конфигурации (ADR не обновлён — Н21) | | 21, 22 | Автозагрузка | `--autostart` открывает в трее, по умолчанию выключена (остаток — Н20) | | 23 | Защита последнего администратора | Учитываются все администраторы, отдельная проверка при удалении отпечатков | | 25 | `Trim` у PIN | Проверяются оба варианта | | 26 | Имена на публичном экране | Маскирование в списке (неполно — Н17) | | 27 | Ошибки фоновых задач и `crash.log` | Глобальные обработчики, ротация по 1 МБ на 3 файла | | 28, 29 | Платформы проекта, шаблонный манифест | Только x64, манифест очищен, BOM убран | | 31, 32, 33 | `x-correlation-id`, версия, shutdown, CI, digest | Проверка ULID, `src/version.ts` и сверка версий, таймаут остановки, `format:check` в CI, образы по digest, запрет standalone в production | ### Оставлено сознательно и вынесено владельцу - **№24** — повторный запрос PIN перед разрушительными действиями сохранён по требованию владельца. - **№30** — EULA ZKTeco добавлена, но сборки терминала и подписи в CI нет: нужны Windows-раннер и сертификат. Стоит отметить, что EULA запрещает включение компонента в продукты, извлекающие из этого коммерческую выгоду, — перед коммерческим распространением нужна юридическая проверка. - **A1** (роль терминала: локальное хранилище против edge-клиента Staff) и **A2** (письменные согласия на биометрию, сроки хранения, трансграничная передача) — решения владельца и юриста, зафиксированы в `docs/AUDIT-REMEDIATION-2026-09-18.md`. ## 7. Оценка качества исправлений Сделано аккуратно: исправления не косметические, к ним добавлены тесты (изоляция тенантов, сверка OpenAPI), документация обновлена вместе с кодом, отдельный документ фиксирует, что оставлено сознательно. Обход RLS, мигратор и контракт API — самые опасные места первого отчёта — закрыты по существу, а не «подпоркой». Основные замечания к процессу: 1. Изменения не проверялись в реальной среде: Docker и PostgreSQL не запускались, поэтому неработающий HEALTHCHECK (Н11) и сломанный путь обновления (Н13) не были замечены. `AGENTS.md` прямо требует не считать Docker и CI проверенными, если среда не запускалась, — документ исправлений это честно фиксирует. 2. Замечания второго отчёта не попали в работу, а хранилище состояния — источник самой опасной из открытых проблем (Н1). 3. Коммиты по-прежнему крупные и смешивают исправления с новыми функциями (экспорт CSV, приватный режим, редактирование). ## 8. План | Приоритет | Задача | Находки | | --------- | -------------------------------------------------------------------------------------------------------------------------------- | ------------- | | 1 | Не стартовать с пустым состоянием при наличии `*.corrupt-*`/`.bak*`; не затирать копии; отличать повреждение от временных ошибок | Н1, Н2 | | 2 | Исправить HEALTHCHECK и добавить проверку сервиса `app` в compose | Н11 | | 3 | Убрать миграционные учётные данные из контейнера приложения | Н12 | | 4 | Создание роли приложения в сервисе `migrate`, порядок обновления существующих установок, явная ошибка при отсутствии роли | Н13, Н14 | | 5 | Журнал с добавлением записей вместо полной перезаписи, резервные копии по времени, ручная правка отметок | Н3, Н15, Н16 | | 6 | Маскирование имени на главном экране терминала | Н17 | | 7 | Проверить `GetDeviceCount() - 1` на оборудовании, довести устойчивость сканера | Н4, Н6 | | 8 | Счётчик попыток PIN в хранилище, не блокировать вход по отпечатку, пересчёт старых хешей | Н5 | | 9 | Контрольные суммы миграций, обновление ADR-001, аудит экспорта CSV | Н19, Н21, Н18 | | 10 | Решения владельца: контракт терминала и правовая база обработки биометрии | A1, A2 |