docs/chore: Phase 1 doc/code hygiene (code-review.md § Phase 1)

Removes ~30 stale "Phase N" / historical-staging comment references from
code and shell scripts now that v1.0 is done; fixes a stale dashboard
comment claiming applications/send-log were unimplemented; adds a CSRF ADR
to security.md documenting the Origin-check-over-tokens decision; resolves
docs/logo in roadmap.md (directory doesn't exist, criterion already met);
adds a gofmt -l check to CI so unformatted Go fails the build.

The known-limitations write-up for the log-tailer offset gap (the other
Phase 1 item) was already present in architecture.md § Log tailer, so no
change was needed there.

gofmt/go vet/go test clean on both Go modules (main + test/e2e), verified
on the dev server.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-06 16:13:36 +03:00
parent ecc4184324
commit 670982fb3e
27 changed files with 113 additions and 70 deletions
+9
View File
@@ -16,6 +16,15 @@ jobs:
go-version: "1.26" go-version: "1.26"
cache: true cache: true
- name: Format
run: |
fmt="$(gofmt -l .)"
if [ -n "$fmt" ]; then
echo "$fmt"
echo "gofmt needed on the files above; run 'gofmt -w .'"
exit 1
fi
- name: Vet - name: Vet
run: go vet ./... run: go vet ./...
+11
View File
@@ -5,6 +5,17 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
## [Unreleased] ## [Unreleased]
### Changed
- Phase 1 doc/code hygiene (`docs/code-review.md`): removed ~30 stale
"Phase N" / historical-staging references from code and shell-script
comments (`cmd/panel`, `internal/*`, `build/*`) now that v1.0 is done;
fixed a stale dashboard comment (`internal/web/handlers_domains.go`)
claiming applications/send-log were unimplemented; added a CSRF ADR to
`docs/security.md` (why Origin-check, not tokens); resolved `docs/logo` in
`docs/roadmap.md` (directory doesn't exist, criterion already met); added a
`gofmt -l` check to CI (`.github/workflows/test.yml`).
### Security ### Security
- Pre-release security review (plan § D, model Fable, 2026-08-06): full pass - Pre-release security review (plan § D, model Fable, 2026-08-06): full pass
+2 -3
View File
@@ -13,8 +13,7 @@ WORKDIR /src
# Version stamped into both binaries; MUST match the image tag (spec 7.5.A). # Version stamped into both binaries; MUST match the image tag (spec 7.5.A).
ARG VERSION=dev ARG VERSION=dev
# Module metadata first for layer caching. go.sum arrived in Phase 2 with the # Module metadata first for layer caching.
# SQLite driver and bcrypt.
COPY go.mod go.sum ./ COPY go.mod go.sum ./
RUN go mod download RUN go mod download
@@ -36,7 +35,7 @@ FROM debian:bookworm-slim AS runtime
ENV DEBIAN_FRONTEND=noninteractive ENV DEBIAN_FRONTEND=noninteractive
# Preseed Postfix so its install is non-interactive and yields a working # Preseed Postfix so its install is non-interactive and yields a working
# main.cf. The real relay configuration is generated by the panel in Phase 5. # main.cf. The real relay configuration is generated by the panel.
RUN echo "postfix postfix/mailname string localhost" | debconf-set-selections \ RUN echo "postfix postfix/mailname string localhost" | debconf-set-selections \
&& echo "postfix postfix/main_mailer_type string Internet Site" | debconf-set-selections \ && echo "postfix postfix/main_mailer_type string Internet Site" | debconf-set-selections \
&& apt-get update \ && apt-get update \
+4 -4
View File
@@ -36,7 +36,7 @@ find /data/opendkim/keys -type f -name '*.private' -exec chmod 0640 {} +
# sasldb2 via saslpasswd2; Postfix (user `postfix`) reads it to authenticate SMTP # sasldb2 via saslpasswd2; Postfix (user `postfix`) reads it to authenticate SMTP
# clients. Share it through the `selfpost` group the same way as the DKIM tree: # clients. Share it through the `selfpost` group the same way as the DKIM tree:
# setgid directory so new files inherit the group, and the database itself # setgid directory so new files inherit the group, and the database itself
# group-readable (0640). Postfix wiring to actually consult it lands in Phase 5. # group-readable (0640).
mkdir -p /data/sasl mkdir -p /data/sasl
chown -R panel:selfpost /data/sasl chown -R panel:selfpost /data/sasl
chmod 2750 /data/sasl chmod 2750 /data/sasl
@@ -51,9 +51,9 @@ chown -R panel:selfpost /data/postfix
chmod 2750 /data/postfix chmod 2750 /data/postfix
chmod 0640 /data/postfix/sender_login_maps chmod 0640 /data/postfix/sender_login_maps
# Milter socket directories (spec 5 p.3, 7.3). From Phase 5 Postfix (user # Milter socket directories (spec 5 p.3, 7.3). Postfix (user `postfix`) must
# `postfix`) must actually CONNECT to both milter sockets — OpenDKIM's and the # actually CONNECT to both milter sockets — OpenDKIM's and the panel's
# panel's journal-milter — not just probe them at start-up. The sockets are # journal-milter — not just probe them at start-up. The sockets are
# created by the opendkim and panel users respectively, so bridge them to # created by the opendkim and panel users respectively, so bridge them to
# `postfix` through the shared `selfpost` group: group-owned + setgid dirs mean # `postfix` through the shared `selfpost` group: group-owned + setgid dirs mean
# each socket created inside inherits group `selfpost`, and group-traversable # each socket created inside inherits group `selfpost`, and group-traversable
+1 -1
View File
@@ -106,7 +106,7 @@ postconf -e \
"smtpd_sender_restrictions=reject_sender_login_mismatch, permit" "smtpd_sender_restrictions=reject_sender_login_mismatch, permit"
# Level-1 rate limit by client IP (spec 5 p.5). Backstop that keeps working even # Level-1 rate limit by client IP (spec 5 p.5). Backstop that keeps working even
# if the journal-milter (level 2, Phase 8) is down. # if the journal-milter (level 2) is down.
postconf -e \ postconf -e \
"smtpd_client_message_rate_limit=${RATE_MSGS}" \ "smtpd_client_message_rate_limit=${RATE_MSGS}" \
"anvil_rate_time_unit=${RATE_WINDOW}s" "anvil_rate_time_unit=${RATE_WINDOW}s"
+1 -1
View File
@@ -8,7 +8,7 @@
# an explicit startup failure instead of a relay running blind. # an explicit startup failure instead of a relay running blind.
# #
# This handles cold start only. Runtime milter failures after a successful start # This handles cold start only. Runtime milter failures after a successful start
# are governed by fail-open (milter_default_action), configured in Phase 5. # are governed by fail-open (milter_default_action).
set -eu set -eu
OPENDKIM_SOCK="${OPENDKIM_SOCKET:-/run/opendkim/opendkim.sock}" OPENDKIM_SOCK="${OPENDKIM_SOCKET:-/run/opendkim/opendkim.sock}"
+2 -2
View File
@@ -16,8 +16,8 @@ import (
) )
// serveHTTP runs the control-panel HTTP server until ctx is cancelled, using // serveHTTP runs the control-panel HTTP server until ctx is cancelled, using
// the database handle shared by all roles. From Phase 2 this serves the real // the database handle shared by all roles: setup, login and the authenticated
// setup, login and authenticated panel surface (spec 7.6). // panel surface (spec 7.6).
func serveHTTP(ctx context.Context, cfg config, st *store.Store) error { func serveHTTP(ctx context.Context, cfg config, st *store.Store) error {
// Applications own the SASL accounts and the Postfix sender map; the domain // Applications own the SASL accounts and the Postfix sender map; the domain
// service delegates to them when a domain (and its applications) is deleted. // service delegates to them when a domain (and its applications) is deleted.
+3 -7
View File
@@ -1,10 +1,6 @@
// Command panel is the SelfPost control panel. In the finished product this // Command panel is the SelfPost control panel. This single binary combines
// single binary combines several roles (spec 7.1): the HTTP panel server, // several roles (spec 7.1) as a supervised process: the HTTP panel server,
// the journal-milter, the mail.log tailer and the rate-limit checks. // the journal-milter, the mail.log tailer and the rate-limit checks.
//
// Phase 1 wires those roles up as a supervised process with a minimal HTTP
// stub, a journal-milter socket stub (so the Postfix start wrapper's readiness
// probe passes) and a log-tailer stub. Real behaviour lands in later phases.
package main package main
import ( import (
@@ -133,7 +129,7 @@ func loadConfig() config {
// saslRealm chooses the realm new SASL accounts live under. It mirrors the // saslRealm chooses the realm new SASL accounts live under. It mirrors the
// hostname Postfix's SASL layer uses so a client authenticating with a bare // hostname Postfix's SASL layer uses so a client authenticating with a bare
// login resolves to the right account (finalised in Phase 5). // login resolves to the right account.
func saslRealm() string { func saslRealm() string {
if r := os.Getenv("SASL_REALM"); r != "" { if r := os.Getenv("SASL_REALM"); r != "" {
return r return r
+2 -1
View File
@@ -50,7 +50,8 @@
- **Документация:** план D1–D9 закрыт ([documentation-plan.md](documentation-plan.md) — только метод и правила поддержки). Хвост v1.x (Codeberg в Quick start, тег образа, `docs/logo`) — [roadmap.md](roadmap.md) § «v1.x — хвост документации и деплоя». - **Документация:** план D1–D9 закрыт ([documentation-plan.md](documentation-plan.md) — только метод и правила поддержки). Хвост v1.x (Codeberg в Quick start, тег образа, `docs/logo`) — [roadmap.md](roadmap.md) § «v1.x — хвост документации и деплоя».
- **Рецензирование кодовой базы** (2026-08-05): [code-review.md](code-review.md) — 10 разделов (архитектура, качество, docs, GUI, legacy, риски), приоритизированный план реализации и маршрутизация моделей. Критичных багов не найдено; блокер релиза — § D ниже. - **Рецензирование кодовой базы** (2026-08-05): [code-review.md](code-review.md) — 10 разделов (архитектура, качество, docs, GUI, legacy, риски), приоритизированный план реализации и маршрутизация моделей. Критичных багов не найдено; блокер релиза — § D ниже.
- **§ D выполнен (2026-08-06):** предрелизная ревизия безопасности моделью Fable — диф от аудита v1.0 (Фаза 11, `bd64e80`) до HEAD + полный проход по чек-листу [security.md](security.md) (бывшее ТЗ 7.6). Эксплуатируемых находок нет; одна правка defence-in-depth (`--` перед логином в argv `saslpasswd2`, `internal/app/sasl.go` + тест). Принятые риски не пополнились. Детали — [implementation-plan.md](implementation-plan.md) § D и CHANGELOG `[Unreleased]/Security`. Локально `go vet`/`go test ./internal/app/...` чистые; падения `internal/domain` (`TestWriteLoadPrivateKeyRoundtrip`, `TestRenderTables`) и `internal/logtail` (`TestFollowTailsAndRotates`) — Windows-специфика (права файлов/`\` в путях/rename открытого файла), на Linux CI зелено. - **§ D выполнен (2026-08-06):** предрелизная ревизия безопасности моделью Fable — диф от аудита v1.0 (Фаза 11, `bd64e80`) до HEAD + полный проход по чек-листу [security.md](security.md) (бывшее ТЗ 7.6). Эксплуатируемых находок нет; одна правка defence-in-depth (`--` перед логином в argv `saslpasswd2`, `internal/app/sasl.go` + тест). Принятые риски не пополнились. Детали — [implementation-plan.md](implementation-plan.md) § D и CHANGELOG `[Unreleased]/Security`. Локально `go vet`/`go test ./internal/app/...` чистые; падения `internal/domain` (`TestWriteLoadPrivateKeyRoundtrip`, `TestRenderTables`) и `internal/logtail` (`TestFollowTailsAndRotates`) — Windows-специфика (права файлов/`\` в путях/rename открытого файла), на Linux CI зелено.
- **Дальше:** релизный гейт открыт (e2e C.4 + ревизия § D) — по явной команде пользователя: резка версии в CHANGELOG, тег, пуш образа (workflow release.yml). Остальные пункты из [code-review.md](code-review.md) — polish (фазы 13). - **Фаза 1 выполнена (2026-08-06)** ([code-review.md](code-review.md) § Фаза 1 — doc/code hygiene, P1): cleanup ~30 stale «Phase N» комментариев в коде и shell-скриптах; исправлен stale-комментарий в `handlers_domains.go`; ADR CSRF (Origin vs токены) добавлен в [security.md](security.md); known-limitations по log-tailer уже был в [architecture.md](architecture.md) § Log tailer — отдельного действия не потребовалось; `docs/logo` в [roadmap.md](roadmap.md) закрыт (каталога нет, критерию соответствует); `gofmt -l` добавлен в CI (`.github/workflows/test.yml`). `gofmt`/`go vet`/`go test ./...` чистые в обоих модулях (dev-server).
- **Дальше:** релизный гейт (Фаза 0) закрыт по существу — e2e C.4 и ревизия § D пройдены; остаются только шаги, которые делаются в момент резки версии (бамп тега образа + Codeberg URL, git tag) по явной команде пользователя. Остальные пункты из [code-review.md](code-review.md) — polish (фазы 23, 1.5).
- **Принятые риски** — [security.md](security.md). **Опционально v1.x / 2.x** — [roadmap.md](roadmap.md) (хвост документации, send-log gaps, Фаза O1+, роль администратора домена). - **Принятые риски** — [security.md](security.md). **Опционально v1.x / 2.x** — [roadmap.md](roadmap.md) (хвост документации, send-log gaps, Фаза O1+, роль администратора домена).
- **Прод:** `selfpost.example.com`, реальный Let's Encrypt сертификат, живой e2e (DKIM/SPF pass). Контейнер там всё ещё на образе v1.0 — Фаза 14 в него не выкатывалась. При апгрейде: админа один раз разлогинит (сменилось имя cookie), а от reverse-proxy требуется передача исходного `Host` (Apache-фрагмент из `deploy/` это делает). - **Прод:** `selfpost.example.com`, реальный Let's Encrypt сертификат, живой e2e (DKIM/SPF pass). Контейнер там всё ещё на образе v1.0 — Фаза 14 в него не выкатывалась. При апгрейде: админа один раз разлогинит (сменилось имя cookie), а от reverse-proxy требуется передача исходного `Host` (Apache-фрагмент из `deploy/` это делает).
+5 -6
View File
@@ -31,9 +31,6 @@ Codeberg (`codeberg.org/mix/selfpost/raw/branch/main/deploy/...`).
`vX.Y.Z` — не раньше. Сейчас может отставать от целевой версии релиза; `vX.Y.Z` — не раньше. Сейчас может отставать от целевой версии релиза;
несовпадение мешает только до первого выката по тегу. несовпадение мешает только до первого выката по тегу.
**Каталог `docs/logo`.** Пустой; либо наполнить (если нужен отдельный asset для
внешних ссылок), либо удалить каталог, чтобы не создавать ложное ожидание.
**Убрать `implementation-plan.md` — в релизном коммите.** Документ закрыт: **Убрать `implementation-plan.md` — в релизном коммите.** Документ закрыт:
уникального содержания в нём нет, § D (предрелизная ревизия безопасности) уникального содержания в нём нет, § D (предрелизная ревизия безопасности)
продублирован в [progress.md](progress.md), [security.md](security.md) и продублирован в [progress.md](progress.md), [security.md](security.md) и
@@ -57,9 +54,11 @@ CHANGELOG `[Unreleased]/Security`, а разделы B.1–B.3 и C.4 вырез
он выполнен. он выполнен.
**Готово, когда:** Quick start указывает на Codeberg; тег образа в compose **Готово, когда:** Quick start указывает на Codeberg; тег образа в compose
совпадает с релизом; `docs/logo` либо содержит файлы, либо отсутствует; совпадает с релизом; `implementation-plan.md` в `docs/archive/`, ссылок на него
`implementation-plan.md` в `docs/archive/`, ссылок на него в активных документах в активных документах и в коде/CI не осталось.
и в коде/CI не осталось.
(`docs/logo` уже отсутствует как каталог — критерию «либо содержит файлы, либо
отсутствует» удовлетворяет; отдельного действия не требуется.)
**Send-log vs `mail.log` (опционально).** После рестарта панели или пересоздания **Send-log vs `mail.log` (опционально).** После рестарта панели или пересоздания
контейнера строки send-log могут навсегда остаться `queued` — log-tailer не контейнера строки send-log могут навсегда остаться `queued` — log-tailer не
+30
View File
@@ -98,6 +98,36 @@ Hardening сверх обязательного (security-заголовки, п
`/data` и теряется при recreate. Rename-ротация это не лечит. См. [architecture.md](architecture.md) `/data` и теряется при recreate. Rename-ротация это не лечит. См. [architecture.md](architecture.md)
§ Log tailer — known gaps. § Log tailer — known gaps.
## ADR: CSRF через проверку Origin, без токенов
**Контекст.** Панель — формы (`POST`) с cookie-сессией; классическая CSRF-
поверхность. Нужен способ отличить запрос со страницы панели от запроса,
инициированного сторонним сайтом в браузере залогиненного админа.
**Решение.** `originAllowed` в
[internal/web/security.go](../internal/web/security.go) сверяет `Sec-Fetch-Site`
(если браузер его шлёт) либо `Origin` (fallback) с хостом панели; запрос без
обоих заголовков **пропускается**, а не отклоняется. Токенов, привязанных к
сессии и встроенных в формы, нет.
**Почему не токены.** Панель однопользовательская (один администратор на
инстанс) — модель угроз не включает межпользовательский CSRF внутри самой
панели, только внешний сайт, заставляющий браузер админа отправить запрос.
Origin-проверка закрывает это без изменения ни одного шаблона: токен потребовал
бы скрытого поля примерно в двух десятках форм и синхронизации при каждой
новой форме, а от XSS внутри панели токен всё равно не защищает — код,
исполняющийся в origin панели, читает токен и отправляет запрос сам. От XSS
защищают автоэкранирование `html/template` и CSP, поэтому это отдельная линия
обороны, не CSRF-токен.
**Компромисс.** Клиент, не посылающий ни `Sec-Fetch-Site`, ни `Origin`
(по-настоящему старый браузер или webview с замороженным движком), остаётся
уязвим — см. «Принятые риски» выше. Это осознанный выбор в пользу не ломать
панель в таком клиенте ценой узкой остаточной поверхности.
**Пересмотр, если:** появится требование защиты, не зависящей от поведения
браузера, или панель станет многопользовательской.
## Как этот список пополняется ## Как этот список пополняется
Предрелизная проверка на уязвимости ([implementation-plan.md](implementation-plan.md) Предрелизная проверка на уязвимости ([implementation-plan.md](implementation-plan.md)
+1 -1
View File
@@ -28,7 +28,7 @@ type SASLDB struct {
// NewSASLDB builds a manager for the sasldb2 at path with the given realm. The // NewSASLDB builds a manager for the sasldb2 at path with the given realm. The
// realm should match SELFPOST_HOSTNAME so the account identity lines up with // realm should match SELFPOST_HOSTNAME so the account identity lines up with
// Postfix's SASL configuration in Phase 5. // Postfix's SASL configuration.
func NewSASLDB(path, realm string) *SASLDB { func NewSASLDB(path, realm string) *SASLDB {
return &SASLDB{path: path, realm: realm, run: runSaslpasswd2, dump: dumpSASLDB} return &SASLDB{path: path, realm: realm, run: runSaslpasswd2, dump: dumpSASLDB}
} }
+5 -6
View File
@@ -37,7 +37,7 @@ type Store interface {
// session accumulates the fields of one message as the milter callbacks fire. // session accumulates the fields of one message as the milter callbacks fire.
// Milter macros arrive per-stage and do not accumulate, so each value is // Milter macros arrive per-stage and do not accumulate, so each value is
// captured at the stage that carries it (spec 7.3 / Phase 0 spike): SASL login // captured at the stage that carries it (spec 7.3): SASL login
// and From at MAIL, each recipient at RCPT, Subject in the headers, and the // and From at MAIL, each recipient at RCPT, Subject in the headers, and the
// queue-id at end-of-message. go-milter creates one session per connection; a // queue-id at end-of-message. go-milter creates one session per connection; a
// connection may carry several messages, so per-message fields are reset at // connection may carry several messages, so per-message fields are reset at
@@ -55,8 +55,8 @@ type session struct {
} }
// Connect captures the client IP, which comes from the addr parameter rather // Connect captures the client IP, which comes from the addr parameter rather
// than a macro (the {client_addr} macro was empty in the spike). It is the // than a macro (the {client_addr} macro was empty in testing). It is the
// rate-limit key for Phase 8; here it is recorded for completeness. // rate-limit key; here it is recorded for completeness.
func (s *session) Connect(host, family string, port uint16, addr net.IP, m *milter.Modifier) (milter.Response, error) { func (s *session) Connect(host, family string, port uint16, addr net.IP, m *milter.Modifier) (milter.Response, error) {
if addr != nil { if addr != nil {
s.clientIP = addr.String() s.clientIP = addr.String()
@@ -133,8 +133,7 @@ func (s *session) Body(m *milter.Modifier) (milter.Response, error) {
// macro reads a milter macro, tolerating Postfix's convention of wrapping // macro reads a milter macro, tolerating Postfix's convention of wrapping
// multi-character macro names in curly braces (e.g. {auth_authen}) while // multi-character macro names in curly braces (e.g. {auth_authen}) while
// single-character names (e.g. i) arrive bare. go-milter stores whatever name // single-character names (e.g. i) arrive bare. go-milter stores whatever name
// Postfix sends verbatim, so a lookup must try both forms — this is exactly the // Postfix sends verbatim, so a lookup must try both forms.
// distinction the SASL-less Phase 0 spike could not observe.
func macro(m *milter.Modifier, name string) string { func macro(m *milter.Modifier, name string) string {
if v, ok := m.Macros[name]; ok { if v, ok := m.Macros[name]; ok {
return v return v
@@ -180,7 +179,7 @@ func cleanAddress(a string) string {
} }
// domainOf returns the lower-cased domain of an email address, or "" if there // domainOf returns the lower-cased domain of an email address, or "" if there
// is no domain part. Sender binding (Phase 4) guarantees the From domain equals // is no domain part. Sender binding guarantees the From domain equals
// the application's domain, so this is the sending domain (spec 7.3). // the application's domain, so this is the sending domain (spec 7.3).
func domainOf(addr string) string { func domainOf(addr string) string {
if i := strings.LastIndexByte(addr, '@'); i >= 0 { if i := strings.LastIndexByte(addr, '@'); i >= 0 {
+5 -6
View File
@@ -1,8 +1,7 @@
// Package postfix owns the Postfix configuration files the panel edits at // Package postfix owns the Postfix configuration files the panel edits at
// runtime and the privileged reload that applies them (spec 5.1, 7.6.3-4). In // runtime and the privileged reload that applies them (spec 5.1, 7.6.3-4): the
// Phase 4 that is the smtpd_sender_login_maps table binding each application's // smtpd_sender_login_maps table binding each application's SASL login to the
// SASL login to the sender addresses it may use; the full relay configuration // sender addresses it may use, plus the relay configuration in main.cf.
// lands in Phase 5.
package postfix package postfix
import ( import (
@@ -33,8 +32,8 @@ func New(dir string) *Postfix {
} }
} }
// SenderLoginMapsPath is the absolute path of the generated map, so the Postfix // SenderLoginMapsPath is the absolute path of the generated map, so main.cf
// main.cf written in Phase 5 can point smtpd_sender_login_maps at it. // can point smtpd_sender_login_maps at it.
func (p *Postfix) SenderLoginMapsPath() string { func (p *Postfix) SenderLoginMapsPath() string {
return p.senderLoginMapsPath return p.senderLoginMapsPath
} }
+2 -2
View File
@@ -62,8 +62,8 @@ func TestDeleteDomainCascadesApplications(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
// Insert an application + address directly (the AddApplication API lands in // Insert an application + address directly (bypassing the AddApplication
// Phase 4); this verifies the ON DELETE CASCADE wiring now. // API) to verify the ON DELETE CASCADE wiring.
now := time.Now().UTC().Format(time.RFC3339) now := time.Now().UTC().Format(time.RFC3339)
res, err := st.db.Exec( res, err := st.db.Exec(
"INSERT INTO applications (domain_id, login, address_mode, created_at) VALUES (?, ?, 'wildcard', ?)", "INSERT INTO applications (domain_id, login, address_mode, created_at) VALUES (?, ?, 'wildcard', ?)",
+2 -2
View File
@@ -5,8 +5,8 @@ import (
"time" "time"
) )
// readSendLog returns every send_log row ordered by id. Phase 6 has no read // readSendLog returns every send_log row ordered by id. The monitoring UI has
// query yet (the monitoring UI is Phase 7), so tests read the table directly. // no equivalent read query, so tests read the table directly.
type sendLogRow struct { type sendLogRow struct {
QueueID string QueueID string
Domain string Domain string
+3 -3
View File
@@ -97,8 +97,8 @@ func (s *Server) renderDomainDetail(w http.ResponseWriter, r *http.Request, stat
} }
// What DNS actually publishes for the domain today, checked against the key // What DNS actually publishes for the domain today, checked against the key
// this server signs with (phase 13.B). Cached by the checker, so // this server signs with. Cached by the checker, so re-rendering the page
// re-rendering the page after a form post costs nothing. // after a form post costs nothing.
dns, srv := s.domainDNS(d, record, false) dns, srv := s.domainDNS(d, record, false)
s.render(w, status, "domain_detail", map[string]any{ s.render(w, status, "domain_detail", map[string]any{
@@ -139,7 +139,7 @@ func (s *Server) renderDomainDetail(w http.ResponseWriter, r *http.Request, stat
} }
// domainDNS resolves what the world sees for a domain: its DKIM, SPF and DMARC // domainDNS resolves what the world sees for a domain: its DKIM, SPF and DMARC
// records (phase 13.B). The server's own address comes from the (separately // records. The server's own address comes from the (separately
// cached) hostname check, so the SPF heuristic knows which IP it is looking for // cached) hostname check, so the SPF heuristic knows which IP it is looking for
// and no extra environment variable is needed. That server result is returned // and no extra environment variable is needed. That server result is returned
// alongside, because the page's suggested SPF record is built from the same // alongside, because the page's suggested SPF record is built from the same
+2 -2
View File
@@ -13,7 +13,7 @@ import (
// TLS in front, so CookieSecure — it carries the __Host- prefix, which turns // TLS in front, so CookieSecure — it carries the __Host- prefix, which turns
// what the cookie's attributes merely promise into something the browser // what the cookie's attributes merely promise into something the browser
// enforces: Secure, Path=/ and, the point of the exercise, no Domain // enforces: Secure, Path=/ and, the point of the exercise, no Domain
// attribute, so no other host may set a cookie by this name (phase 14.B). // attribute, so no other host may set a cookie by this name.
// The prefix is only valid on a Secure cookie, so a development instance on // The prefix is only valid on a Secure cookie, so a development instance on
// plain HTTP has to keep the bare name: with the prefix the browser would // plain HTTP has to keep the bare name: with the prefix the browser would
// discard the Set-Cookie outright and logging in would silently never stick. // discard the Set-Cookie outright and logging in would silently never stick.
@@ -42,7 +42,7 @@ func (s *Server) sessionCookie() string {
// is denial of service, not compromise; refusing the request and saying so in // is denial of service, not compromise; refusing the request and saying so in
// the log is what makes it diagnosable instead of an endless login loop. The // the log is what makes it diagnosable instead of an endless login loop. The
// __Host- prefix prevents this outright, but only where it applies — this // __Host- prefix prevents this outright, but only where it applies — this
// check also covers the plain-HTTP development shape (phase 14.B). // check also covers the plain-HTTP development shape.
func (s *Server) sessionToken(r *http.Request) (string, bool) { func (s *Server) sessionToken(r *http.Request) (string, bool) {
name := s.sessionCookie() name := s.sessionCookie()
var token string var token string
+2 -2
View File
@@ -11,7 +11,7 @@ import (
// handleDashboard is the authenticated landing page: the list of sending // handleDashboard is the authenticated landing page: the list of sending
// domains with their DKIM/selector and application counts, plus the add-domain // domains with their DKIM/selector and application counts, plus the add-domain
// form (spec 7.2.2). Applications and the send log arrive in later phases. // form (spec 7.2.2).
func (s *Server) handleDashboard(w http.ResponseWriter, r *http.Request) { func (s *Server) handleDashboard(w http.ResponseWriter, r *http.Request) {
s.renderDashboard(w, r, http.StatusOK, "", "") s.renderDashboard(w, r, http.StatusOK, "", "")
} }
@@ -118,7 +118,7 @@ func (s *Server) handleDeleteDomain(w http.ResponseWriter, r *http.Request) {
// handleReload re-applies both the OpenDKIM configuration and the Postfix // handleReload re-applies both the OpenDKIM configuration and the Postfix
// sender map on demand (spec 7.2.12). Each Resync regenerates its files from the // sender map on demand (spec 7.2.12). Each Resync regenerates its files from the
// database and reloads its daemon, so the button doubles as a drift-recovery. // database and reloads its daemon, so the button doubles as a drift-recovery.
// The button lives on the status page (phase 13.D): it is a "put the daemons // The button lives on the status page: it is a "put the daemons
// back in the state the database describes" action, which belongs with the rest // back in the state the database describes" action, which belongs with the rest
// of the server-health screen rather than in the domain list's top bar. // of the server-health screen rather than in the domain list's top bar.
func (s *Server) handleReload(w http.ResponseWriter, r *http.Request) { func (s *Server) handleReload(w http.ResponseWriter, r *http.Request) {
+2 -2
View File
@@ -8,8 +8,8 @@ import (
) )
// handleStatus renders the server status page: the panel's landing page and the // handleStatus renders the server status page: the panel's landing page and the
// one screen that answers "is the service healthy and will mail be accepted" // one screen that answers "is the service healthy and will mail be accepted".
// (phase 13.A). The cheap local checks live in the polled "status_body" // The cheap local checks live in the polled "status_body"
// fragment; the hostname/PTR lookup and the configuration reload sit outside it, // fragment; the hostname/PTR lookup and the configuration reload sit outside it,
// because neither belongs on a five-second timer. // because neither belongs on a five-second timer.
func (s *Server) handleStatus(w http.ResponseWriter, r *http.Request) { func (s *Server) handleStatus(w http.ResponseWriter, r *http.Request) {
+7 -7
View File
@@ -8,7 +8,7 @@ import (
// contentSecurityPolicy is the panel's CSP. Everything the pages load — // contentSecurityPolicy is the panel's CSP. Everything the pages load —
// stylesheet, HTMX, the panel's own script, the favicon — is served from // stylesheet, HTMX, the panel's own script, the favicon — is served from
// /static by this same origin, and no template carries an inline <script>, // /static by this same origin, and no template carries an inline <script>,
// an inline event handler or a style="..." attribute (phase 14.A; the // an inline event handler or a style="..." attribute (the
// template guard test enforces that), so 'self' needs no exemptions: // template guard test enforces that), so 'self' needs no exemptions:
// //
// - default-src 'self' covers scripts, styles, images and the XHR that // - default-src 'self' covers scripts, styles, images and the XHR that
@@ -38,8 +38,8 @@ const contentSecurityPolicy = "default-src 'self'; " +
// subdomain of that domain for a year, which is not SelfPost's call to make. // subdomain of that domain for a year, which is not SelfPost's call to make.
const strictTransportSecurity = "max-age=31536000" const strictTransportSecurity = "max-age=31536000"
// secure wraps the whole router with the panel's two transport-level defences // secure wraps the whole router with the panel's two transport-level
// (phase 14.A): the security response headers, and an origin check on every // defences: the security response headers, and an origin check on every
// state-changing request. // state-changing request.
// //
// It sits outside the authentication middleware on purpose, so that POST // It sits outside the authentication middleware on purpose, so that POST
@@ -81,7 +81,7 @@ func (s *Server) secure(next http.Handler) http.Handler {
} }
// originAllowed reports whether a state-changing request came from the panel's // originAllowed reports whether a state-changing request came from the panel's
// own origin (phase 14.A). This is what the session cookie's // own origin. This is what the session cookie's
// SameSite=Lax attribute cannot do on its own: SameSite is judged per *site* // SameSite=Lax attribute cannot do on its own: SameSite is judged per *site*
// (registrable domain), so a neighbouring subdomain — a CMS on the same // (registrable domain), so a neighbouring subdomain — a CMS on the same
// domain, a forgotten staging host — counts as same-site and its forged POST // domain, a forgotten staging host — counts as same-site and its forged POST
@@ -111,9 +111,9 @@ func originAllowed(r *http.Request) bool {
origin := r.Header.Get("Origin") origin := r.Header.Get("Origin")
if origin == "" { if origin == "" {
// Neither header. A client this old cannot be checked at all; it is // Neither header. A client this old cannot be checked at all; it is
// let through as the risk consciously accepted for phase 14.A // let through as the risk consciously accepted (single-admin panel,
// (single-admin panel, the administrator picks the browser). Turning // the administrator picks the browser). Turning this return into
// this return into false is the whole of the stricter policy. // false is the whole of the stricter policy.
return true return true
} }
u, err := url.Parse(origin) u, err := url.Parse(origin)
+1 -1
View File
@@ -32,7 +32,7 @@ func post(secFetchSite, origin string) *http.Request {
return r return r
} }
// The full matrix the origin check has to get right (phase 14.A). The row that // The full matrix the origin check has to get right. The row that
// matters most is "same-site": a neighbouring host on example.com is same-site // matters most is "same-site": a neighbouring host on example.com is same-site
// as far as the session cookie's SameSite=Lax is concerned, so this check is // as far as the session cookie's SameSite=Lax is concerned, so this check is
// the only thing standing between it and a forged POST. // the only thing standing between it and a forged POST.
+1 -1
View File
@@ -1,6 +1,6 @@
/* Panel stylesheet. It lives in a file rather than in a <style> block in the /* Panel stylesheet. It lives in a file rather than in a <style> block in the
layout so the panel's Content-Security-Policy can be a plain layout so the panel's Content-Security-Policy can be a plain
"default-src 'self'" with no inline-style exemption (phase 14.A). Any rule "default-src 'self'" with no inline-style exemption. Any rule
added here must therefore stay here: an inline style="..." attribute in a added here must therefore stay here: an inline style="..." attribute in a
template is blocked by that policy and silently does nothing. */ template is blocked by that policy and silently does nothing. */
+1 -1
View File
@@ -36,7 +36,7 @@
// Forms that delete something or invalidate a working credential carry a // Forms that delete something or invalidate a working credential carry a
// data-confirm message. The prompt lives here rather than in an inline // data-confirm message. The prompt lives here rather than in an inline
// onsubmit attribute because the panel's Content-Security-Policy allows no // onsubmit attribute because the panel's Content-Security-Policy allows no
// inline script (phase 14.A). The listener is delegated from the document, // inline script. The listener is delegated from the document,
// so it also covers markup swapped in by HTMX. With JavaScript disabled the // so it also covers markup swapped in by HTMX. With JavaScript disabled the
// form submits without asking — exactly as the inline handler behaved. // form submits without asking — exactly as the inline handler behaved.
document.addEventListener("submit", function (ev) { document.addEventListener("submit", function (ev) {
+1 -1
View File
@@ -15,7 +15,7 @@
{{/* Unless told otherwise, htmx injects a stylesheet element of its own into {{/* Unless told otherwise, htmx injects a stylesheet element of its own into
the head for the request-indicator classes. The panel uses no the head for the request-indicator classes. The panel uses no
hx-indicator, and that injected element would be the one thing the hx-indicator, and that injected element would be the one thing the
Content-Security-Policy (phase 14.A) has to make an exception for, so it Content-Security-Policy has to make an exception for, so it
is switched off here. */}} is switched off here. */}}
<meta name="htmx-config" content='{"includeIndicatorStyles":false}'> <meta name="htmx-config" content='{"includeIndicatorStyles":false}'>
<script src="/static/htmx.min.js" defer></script> <script src="/static/htmx.min.js" defer></script>
+4 -4
View File
@@ -142,9 +142,9 @@ func TestNavLeadsWithStatusAndPointsDomainsAtItsOwnPath(t *testing.T) {
} }
} }
// Since the panel root now redirects to the status page, a link left pointing at // Since the panel root redirects to the status page, a link left pointing at
// "/" silently lands on the wrong screen instead of failing — so no template may // "/" silently lands on the wrong screen instead of failing — so no template may
// contain one (phase 13.C). // contain one.
func TestNoTemplateLinksToTheBareRoot(t *testing.T) { func TestNoTemplateLinksToTheBareRoot(t *testing.T) {
forEachTemplate(t, func(name, body string) { forEachTemplate(t, func(name, body string) {
if strings.Contains(body, `href="/"`) { if strings.Contains(body, `href="/"`) {
@@ -154,7 +154,7 @@ func TestNoTemplateLinksToTheBareRoot(t *testing.T) {
} }
// The reload action is a server-health control and lives only on the status // The reload action is a server-health control and lives only on the status
// page (phase 13.D). // page.
func TestReloadFormLivesOnlyOnTheStatusPage(t *testing.T) { func TestReloadFormLivesOnlyOnTheStatusPage(t *testing.T) {
forEachTemplate(t, func(name, body string) { forEachTemplate(t, func(name, body string) {
if strings.Contains(body, `action="/reload"`) && name != "status.html" { if strings.Contains(body, `action="/reload"`) && name != "status.html" {
@@ -164,7 +164,7 @@ func TestReloadFormLivesOnlyOnTheStatusPage(t *testing.T) {
} }
// The panel's Content-Security-Policy is a plain default-src 'self' with no // The panel's Content-Security-Policy is a plain default-src 'self' with no
// inline exemption (phase 14.A), which makes inline script and inline style a // inline exemption, which makes inline script and inline style a
// failure mode rather than a style question: an onclick= handler or a // failure mode rather than a style question: an onclick= handler or a
// style="..." attribute added to a template does not error, it silently stops // style="..." attribute added to a template does not error, it silently stops
// working in the browser. Behaviour belongs in static/panel.js (triggered from // working in the browser. Behaviour belongs in static/panel.js (triggered from
+4 -4
View File
@@ -110,7 +110,7 @@ func New(st *store.Store, domains *domain.Service, apps *app.Service, cfg Config
sessions: newSessionStore(st, time.Duration(idleDays)*24*time.Hour), sessions: newSessionStore(st, time.Duration(idleDays)*24*time.Hour),
// Published-DNS checks for the status page and the domain pages. The // Published-DNS checks for the status page and the domain pages. The
// checker caches its own results, so page views do not each pay for a // checker caches its own results, so page views do not each pay for a
// round of lookups (phase 13). // round of lookups.
dns: dnscheck.New(cfg.DNSResolvers), dns: dnscheck.New(cfg.DNSResolvers),
// Setup: a handful of attempts per minute per IP is plenty for a // Setup: a handful of attempts per minute per IP is plenty for a
// legitimate admin and blunts automated probing (spec 7.6.1). // legitimate admin and blunts automated probing (spec 7.6.1).
@@ -152,7 +152,7 @@ func (s *Server) Handler() http.Handler {
// above falls through to this sub-mux, wrapped once in the auth middleware. // above falls through to this sub-mux, wrapped once in the auth middleware.
authed := http.NewServeMux() authed := http.NewServeMux()
// The landing page is the server status (phase 13.C): the first thing an // The landing page is the server status: the first thing an
// administrator should see after logging in is whether the service is // administrator should see after logging in is whether the service is
// healthy, not the domain list. handleLogin still redirects to "/". // healthy, not the domain list. handleLogin still redirects to "/".
authed.HandleFunc("GET /{$}", redirectToStatus) authed.HandleFunc("GET /{$}", redirectToStatus)
@@ -196,12 +196,12 @@ func (s *Server) Handler() http.Handler {
mux.Handle("/", s.requireAuth(authed)) mux.Handle("/", s.requireAuth(authed))
// Security headers and the origin check wrap everything, including the // Security headers and the origin check wrap everything, including the
// unauthenticated login and setup routes (phase 14.A). // unauthenticated login and setup routes.
return s.secure(mux) return s.secure(mux)
} }
// redirectToStatus points the panel root at the status page, so there is one // redirectToStatus points the panel root at the status page, so there is one
// canonical URL for that content instead of two (phase 13.C). // canonical URL for that content instead of two.
func redirectToStatus(w http.ResponseWriter, r *http.Request) { func redirectToStatus(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "/status", http.StatusSeeOther) http.Redirect(w, r, "/status", http.StatusSeeOther)
} }