# Рецензирование кодовой базы SelfPost **Дата:** 2026-08-05 **Объём ревью:** ~172 файла, 64 Go-исходника (~10 770 строк в `internal/` + `cmd/`), 29 unit-тестов, e2e-модуль `test/e2e/`, 17 HTML-шаблонов, 15 doc-файлов. Связанные документы: [architecture.md](architecture.md), [security.md](security.md), [implementation-plan.md](implementation-plan.md), [progress.md](progress.md), [roadmap.md](roadmap.md). --- ## Общая оценка | Критерий | Оценка | Комментарий | |----------|--------|-------------| | Архитектура | **Отлично** | Чёткое слоение, минимум связности | | Сложность vs масштаб | **Отлично** | Без over-engineering | | Качество кода | **Отлично** | Идиоматичный Go, продуманные rollback-пути | | Документация | **Хорошо** | As-built docs точны; есть RU/EN split и stale comments | | Поддерживаемость | **Хорошо** | Высокий порог входа из-за phase-комментариев | | Логические ошибки | **Минимально** | Критичных багов не найдено; есть принятые операционные gap'ы | | Legacy | **Низкий** | Только архив spec + phase-комментарии | | GUI | **Хорошо** | Нет «костылей»; осознанные CSP/layout компромиссы | **Вывод:** проект готов к релизному тегу после закрытия § D ([implementation-plan.md](implementation-plan.md)) — предрелизного security review. Остальное — polish, не блокеры. --- ## 1. Архитектура и структура проекта ### Текущая структура ```mermaid flowchart TB subgraph cmd [cmd] panel["panel (HTTP + milter + logtail)"] backup["selfpost-backup CLI"] end subgraph web [internal/web — 3030 LOC] handlers["handlers_*.go"] templates["templates/*.html"] security["security.go, session.go"] end subgraph services [Services] domainSvc["internal/domain"] appSvc["internal/app"] end subgraph persistence [Persistence] store["internal/store (SQLite)"] end subgraph adapters [Adapters] postfix["internal/postfix"] milterPkg["internal/milter"] logtail["internal/logtail"] dnscheck["internal/dnscheck"] backupPkg["internal/backup"] health["internal/health"] end panel --> web panel --> milterPkg panel --> logtail web --> domainSvc web --> appSvc domainSvc --> store appSvc --> store milterPkg --> store logtail --> store domainSvc --> postfix appSvc --> postfix ``` ### Сильные стороны - **Layered / ports-and-adapters:** handlers → services (`domain`, `app`) → `store`; инфраструктура изолирована в адаптерах ([`internal/web/web.go`](../internal/web/web.go), [`internal/app/service.go`](../internal/app/service.go)). - **Composition root** в [`cmd/panel/main.go`](../cmd/panel/main.go): три роли (HTTP, journal-milter, log-tailer) в одном процессе — оправдано для single-container deployment. - **Interface seams** для тестов: `milter.Store`, `app.SenderMaps`, `logtail.StatusStore`. - **Embedded migrations** ([`internal/store/store.go`](../internal/store/store.go)) — простой, надёжный подход для 2 миграций. - **E2E как отдельный модуль** (`test/e2e/go.mod`) — не загрязняет основной модуль. ### Замечания (не блокеры) - **`internal/web/` — 47 файлов, 3030 строк** — самый крупный пакет. При росте v2.x (роли, inbound relay) стоит выделить подпакеты (`web/handlers`, `web/auth`) или split по доменам. Сейчас — приемлемо. - **Один SQLite connection** (`MaxOpenConns(1)`) — сознательный trade-off; при росте нагрузки milter + HTTP + logtail будут сериализованы. Документировано, для single-admin panel — норма. ### Рекомендации | # | Действие | Приоритет | |---|----------|-----------| | A1 | Оставить текущую структуру; рефакторинг пакетов — только при старте 2.x | Низкий | | A2 | Добавить в [architecture.md](architecture.md) диаграмму слоёв (как выше) | Низкий | **Модель:** Sonnet (документация) --- ## 2. Соответствие сложности масштабу проекта ### Факты - Outbound SMTP relay + admin panel для одного оператора. - ~10.7K строк Go, 3 зависимости (`go-milter`, `x/crypto`, `modernc.org/sqlite`). - Нет ORM, нет SPA, нет message queue — всё уместно. ### Сильные стороны - Нет лишних абстракций (нет generic repository, нет DI-фреймворка). - Сервисный слой тонкий, но достаточный: координация SQLite + sasldb2 + Postfix maps ([`internal/app/service.go`](../internal/app/service.go)). - Fail-open/fail-closed решения явно задокументированы (OpenDKIM tempfail vs journal fail-open). ### Замечания - **Rate limiting в двух местах** (Postfix anvil L1 + milter L2) — сложность оправдана спецификацией, но требует понимания оператором. - **Backup с manifest version gate** ([`internal/backup/backup.go`](../internal/backup/backup.go)) — чуть тяжелее минимума, но оправдано для migration safety. **Вердикт:** сложность **адекватна** масштабу. Over-engineering не обнаружен. --- ## 3. Качество написанного кода ### Сильные стороны - **Комментарии объясняют «почему»**, не «что» — образцовый уровень ([`internal/web/security.go`](../internal/web/security.go), [`internal/milter/milter.go`](../internal/milter/milter.go)). - **Rollback-паттерны** при partial failure ([`internal/app/service.go`](../internal/app/service.go) `rollbackCreate`). - **Ordering guarantees:** SQLite row before SASL write — защита от race и password clobber. - **Validation centralized:** [`internal/web/validate.go`](../internal/web/validate.go), [`internal/app/validate.go`](../internal/app/validate.go). - **Atomic file writes** для конфигов ([`internal/postfix/write.go`](../internal/postfix/write.go), [`internal/domain/dkim.go`](../internal/domain/dkim.go)). - **Test coverage ~45%** file ratio (29 test / 64 source files); ключевые пути покрыты (auth, sessions, milter, backup, dnscheck). ### Замечания | Файл | Замечание | Severity | |------|-----------|----------| | [`internal/web/token.go`](../internal/web/token.go) | `panic` при сбое `crypto/rand` — осознанно, документировано | Info | | [`internal/web/handlers_domains.go`](../internal/web/handlers_domains.go) | Stale comment: «Applications and send log arrive in later phases» — уже реализовано | Low | | Phase/spec references | ~50+ файлов с «Phase N», «spec 7.x» — шум для новых контрибьюторов | Low | | [`cmd/panel/main.go`](../cmd/panel/main.go) | Package comment всё ещё упоминает «Phase 1 stubs» | Low | ### Потенциальные улучшения качества - Единый проход **gofmt + удаление stale phase-комментариев** (механическая работа). - Добавить **table-driven test** для edge cases в `parseDelivery` (exotic Postfix status values) — опционально. **Модель:** Haiku (механическая чистка комментариев), Sonnet (точечные правки) --- ## 4. Полнота документации и соответствие коду ### Сильные стороны - **[architecture.md](architecture.md)** — as-built source of truth; маршруты, процессы, persistence совпадают с кодом. - **[security.md](security.md)** — чеклист + принятые риски; каждый риск привязан к коду. - **[product.md](product.md)** — границы v1.0/out-of-scope чёткие. - **Regression guard:** [`cmd/panel/envdoc_test.go`](../cmd/panel/envdoc_test.go) — env vars в README = `loadConfig`. - **CHANGELOG** в формате Keep a Changelog. ### Расхождения docs ↔ code | Проблема | Где | Реальность | |----------|-----|------------| | Image tag `0.1.0` vs «v1.0» | [`deploy/docker-compose.yml`](../deploy/docker-compose.yml) vs README | Roadmap § v1.x — bump при теге | | Quick start URLs | README | GitHub raw; основной repo — Codeberg | | `docs/logo` | roadmap | Каталог отсутствует (не «пустой») | | `docs/specification.md` | documentation-plan D9 | Архивирован в `docs/archive/`; ссылки в коде на «spec 7.x» устарели | | Phase language в коде | 50+ файлов | Docs говорят «v1.0 done», код — «Phase 14» | | RU/EN split | progress, roadmap, implementation-plan (RU) vs README/architecture (EN) | Намеренно, но барьер для EN-only contributors | ### Комментирование кода - **Высокое качество** в security-critical paths. - **Среднее** в CRUD handlers (делегируют в services — acceptable). - **Рекомендация:** заменить «spec 7.x» на ссылки на [product.md](product.md) / [security.md](security.md) § или удалить. **Модель:** Sonnet (docs sync) --- ## 5. Читаемость и поддерживаемость ### Для кого код читаем - **Go-разработчик со знанием SMTP/Postfix** — да, без проблем. - **Новичок без почтового бэкграунда** — потребуется [architecture.md](architecture.md) + README. ### Факторы, помогающие поддержке - Предсказуемая структура handler → service → store. - Embedded templates (`//go:embed`) — один binary, нет внешних assets. - Makefile targets: `vet`, `test`, `build`, `e2e`. - E2E suite покрывает happy path + negatives. ### Факторы, затрудняющие поддержку - Phase-номера в комментариях без контекста. - Dual cookie names (`__Host-` vs plain) — хорошо документировано, но неочевидно. - HTMX fragment polling — нужно понимать SSR + partial updates. - Dev loop: Windows local edit → SSH to Debian server ([progress.md](progress.md)) — не стандартный `go run`. ### Рекомендации | # | Действие | Модель | |---|----------|--------| | M1 | Cleanup phase-комментариев → «as-built» language | Haiku | | M2 | Добавить `CONTRIBUTING.md` (dev loop, model routing, commit protocol) — опционально v1.x | Sonnet | | M3 | Consolidated doc index в README (ссылки на все docs/) | Sonnet | --- ## 6. Логические ошибки и риски ### Критичных багов не обнаружено E2E покрывает: bootstrap, SMTP AUTH, DKIM, send-log lifecycle, negatives (relay, sender mismatch, L1/L2 limits, milter fail-open, hostname gate, session survive restart). ### Принятые операционные gap'ы (не баги, но важно знать) | Gap | Описание | Документировано | |-----|----------|-----------------| | Send-log `queued` forever | Log-tailer стартует с EOF; после restart пропущенный хвост не дочитывается | [security.md](security.md), [roadmap.md](roadmap.md) | | CSRF without tokens | POST без Origin/Sec-Fetch-Site пропускается | [security.md](security.md) | | Fail-open L2 rate limit | DB error → mail проходит | [`internal/milter/ratelimit.go`](../internal/milter/ratelimit.go) | | Shallow SPF check | Не следует `include:`/`redirect=` | README, `internal/dnscheck/spf.go` | | Plaintext backup/export at rest | DKIM-ключи, SASL, пароли приложений в cleartext `.tar.gz`/`.json` | **Mitigation:** R13 (optional encryption) | **Не риск (решение оператора):** «Session resurrection from backup» — снято из [security.md](security.md). ### Потенциальные логические нюансы (низкий приоритет) 1. **Rate limit race:** `CountMessages` + `InsertQueued` не в одной транзакции — при высокой нагрузке возможен overshoot на 1 сообщение. Для differentiated limits — acceptable. 2. **Domain export с plaintext passwords** ([`internal/domain/transfer.go`](../internal/domain/transfer.go)) — by design; **mitigation:** R13 (optional password encryption). 3. **`macro()` dual lookup** ([`internal/milter/milter.go`](../internal/milter/milter.go)) — workaround для Postfix/go-milter; permanent, not a bug. ### Рекомендации | # | Действие | Приоритет | Модель | |---|----------|-----------|--------| | L1 | **Предрелизный security review** (§ D) — обязательный гейт | **P0** | **Fable** | | L2 | **Шифрование бэкапа и экспорта домена** (R13) — optional, checkbox + password | P1 | **Opus** + Sonnet | | L3 | Send-log gap mitigation — опционально | P2 | Opus | | L4 | Transaction wrap для rate limit count+insert — опционально | P3 | Opus | --- ## 7. Legacy-код и миграции ### SQL-миграции - [`0001_init.sql`](../internal/store/migrations/0001_init.sql) — initial schema - [`0002_sessions.sql`](../internal/store/migrations/0002_sessions.sql) — sessions (plan B.1) - Механизм: `PRAGMA user_version`, embedded FS, transactional apply — **чистый**, без legacy branches в коде. ### Архивная документация - [`docs/archive/specification-v1.0.md`](archive/specification-v1.0.md) — historical; помечен «не источник истины». - **Рекомендация:** оставить в archive; в коде заменить «spec 7.x» на актуальные doc-ссылки. ### Legacy patterns в runtime | Элемент | Статус | Действие | |---------|--------|----------| | Phase 0–14 comments | Historical noise | Cleanup (Haiku) | | `macro()` brace workaround | Permanent Postfix compat | Оставить, уже документировано | | Legacy charset (windows-1251) in milter | Keep raw header on decode fail | Оставить | | In-memory sessions | **Удалено** (B.1 → SQLite) | Done | | `copytruncate` log rotation | **Заменено** (B.2 → rename+reload) | Done | **Перспектива удаления:** единственный кандидат на cleanup — **phase-комментарии** и **архив spec** (оставить файл, убрать ссылки из кода). --- ## 8. GUI: «костыли» и оптимизация компоновки ### Стек Go `html/template` + HTMX polling + [`panel.css`](../internal/web/static/panel.css) + [`panel.js`](../internal/web/static/panel.js). **Нет React/Vue** — минимальный footprint. ### Поиск маркеров долга **TODO / FIXME / HACK / kostyl — 0 вхождений** по всему репозиторию. ### Осознанные компромиссы (не костыли) | Компромисс | Файл | Обоснование | |------------|------|-------------| | External CSS/JS only (no inline) | `panel.css`, `panel.js` | CSP `default-src 'self'` | | HTMX `includeIndicatorStyles: false` | `layout.html` | Avoid CSP exception | | Block layout for applications (not table) | `panel.css` | 4 cols + 6 controls don't fit 48rem | | Two-row nav | `panel.css` | Session block vs page links width | | Page-specific max-width (48/64/24rem) | `panel.css` | Monitoring vs forms | | Subject ellipsis via inner `` | `panel.css` | `max-width` on `