krona-staff/docs/audit/2026-09-18-audit.md
Victor Trushenko f5ac179623
Some checks are pending
ci / verify (push) Waiting to run
docs(audit): add 2026-09-26 audit, acceptance and executor tasks
Executors could not see the audit: Grok reported that the owner's
AUDIT.md and evidence never reached it, so its scenarios could not be
mapped to the audit IDs. Keeping the audit, the acceptance report and
the task for each executor in the repository lets Codex, Antigravity
and Grok take work and report through git.

The live-check scripts are stored as .mjs.txt so the project linter,
which type-checks every .mjs file, does not break for everyone; the
maintained version of these checks is Grok's harness in tests/e2e.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-26 15:13:56 +07:00

35 KiB
Raw Permalink Blame History

Аудит репозитория 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