From c3d99b1f1558eb549aa7963ac4b84cdf0e7dff31 Mon Sep 17 00:00:00 2001 From: Mix Date: Tue, 13 Oct 2026 19:13:04 +0300 Subject: [PATCH] roadmap: decisions of 2026-10-13 on the candidates Agreed with the owner, one question at a time; documents only, no code. - csrf-tokens -> origin-strict (agreed): no tokens, the origin check refuses a request with neither header. security.md's ADR records the decision and when tokens are built after all. - structured-logging, contributing: dropped, each with what brings it back (new roadmap section "Dropped, with the condition for return"). - template-data-typing: done by 2.0.0; its last map (healthChecks) is a step of the audit's stage 4. - preflight -> health-setup-checks (certificate name, Postfix's greeting, trusted proxy as lines on Health) and test-email, both agreed. The port check is dropped with its reason; plans/preflight.md removed. - password-reset: agreed; a reset link goes only to a confirmed e-mail, so the item builds confirmation first. - phone-desktop-mode: agreed, after stages 1 and 2 of the audit. - backup-reauth (agreed): the account password before a backup or a domain export, out of review-2026-08-followups. - from-header-binding, Not done: the Delivery log box is required on a refused message's page, with the line Postfix wrote, for all three kinds of refusal (audit stage 1); the Outbound log gets a filter by status and by day (audit stage 4). The audit's checklist is 48 lines (was 45). Co-Authored-By: Claude Opus 5.5 --- docs/audit-2026-10-12.md | 42 ++++- docs/plans/from-header-binding.md | 13 +- docs/plans/preflight.md | 77 --------- docs/roadmap.md | 270 ++++++++++++++++++------------ docs/security.md | 26 ++- 5 files changed, 233 insertions(+), 195 deletions(-) delete mode 100644 docs/plans/preflight.md diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index db02815..685473a 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -597,6 +597,25 @@ has already run is never edited — the next file is added; the published grown for a tick and the new file has a line). Both are fixes to the tailer's blindness and stay on the list whatever the sweep's fate; their priority follows `tailer_blind_seconds`. +- [ ] **A refused message's page shows what Postfix wrote** (owner, + 2026-10-13: "the log box is required" - the box is the reason + delivery-log-storage was built, and its absence from the picture he + approved for 2.4.0 was an oversight, not a decision). Today every + `rejected` row - `wrong From`, `wrong client`, `rate-limited` - has no + queue id, and its page shows the box with one sentence, + "This message never reached the queue, so Postfix wrote no delivery lines + for it" ([handlers_monitor.go](../internal/web/handlers/handlers_monitor.go)). + The sentence is untrue: Postfix wrote a line for each (`milter-reject: + END-OF-MESSAGE ... 5.7.1 From: ...` from cleanup, `NOQUEUE: reject: ...` + from smtpd). The tailer attaches that line to the row it belongs to, by + client address, time and the addresses in the line, and stores it in + `send_log_lines` like any other; the page shows it. First on the stand: + read `mail.log` for the three kinds of refusal and write down what + identifies each line. Then the mockup: the refused variant of `out-message` + gets the box with the line in it (`design:` commit), then the code. The + sentence stays only for a row whose line was truly never written or is + older than this change, and says that. Done when the e2e shows the stored + line on the page of each of the three refusals. - **Opus** - [ ] **Anchor `deliveryRe`** at the head of the line like its two siblings; test with the probed HELO and quoted-sender lines. — **Opus** - [ ] **TLS re-copy** (H2): `postfix-cert-reload.sh` compares the mounted files @@ -689,6 +708,24 @@ has already run is never edited — the next file is added; the published the flash and the two comments say what it covers. — **Sonnet** - [ ] `domain.Service.Delete`: say the domain is deleted when a later step fails, and run `apps.Resync()` anyway. — **Opus** +- [ ] **A status filter and a day filter in the Outbound log** (owner, 2026-10-13, from the + Not done list of from-header-binding). The mockup `out-log` has drawn a + third list since the redesign and the built log never had it; the Inbound + log has its counterpart built (domain, decision, day). One list: *Any + status*, `queued`, `sent`, `deferred`, `bounced`, `rejected` - the statuses + of a row, and `rejected` shows all three kinds of refusal together. He was + shown the list with the refusals split by reason and chose it without the + split. The mockup's own list is wrong (it offers `rate-limited`, which is + a reason, and lacks `queued` and `rejected`): correct it first (`design:` + commit). And a filter by day, as the Inbound log has it (owner, the same + day): the same control, drawn in the mockup first. Both filters survive + paging and the rows' polling. - **Sonnet** +- [ ] **The last untyped page data** (owner, 2026-10-13): `healthChecks` + ([handlers_status.go](../internal/web/handlers/handlers_status.go)) returns + a struct instead of `map[string]any`, so a mistyped key in `healthPage` / + `healthCards` is a compile error and not an empty card on Server › Health. + No template change. This closes what is left of the roadmap's former + `template-data-typing`. — **Sonnet** - [ ] **Upstream TLS mode validated at save** (L5): saving an inbound domain whose `[host]:port` is already used by another domain with a different TLS mode is refused with the validator's text naming that domain and its @@ -727,8 +764,9 @@ has already run is never edited — the next file is added; the published ### Stage 6 — documentation and roadmap sweep (no product code, no cut) - [ ] Every item under *Doc drift* above, file by file. — **Sonnet** -- [ ] roadmap.md: strike `template-data-typing` (done by 2.0; one sampler map - left, typed before render); reword `structured-logging`; in +- [ ] roadmap.md: `template-data-typing` is struck already (owner, 2026-10-13; + its last map is a step of stage 4); `structured-logging` is dropped (owner, + 2026-10-13, roadmap § Dropped) - nothing to reword; in `review-2026-08-followups` mark `parsePage` and `secretfile` rows as addressed by stages 3-4, keep the `offset.go` inode row (still open: fingerprint of the first 512 bytes only), and the `dnscheck` fan-out row diff --git a/docs/plans/from-header-binding.md b/docs/plans/from-header-binding.md index 354347a..ec132a2 100644 --- a/docs/plans/from-header-binding.md +++ b/docs/plans/from-header-binding.md @@ -168,13 +168,18 @@ silence; each line says what is missing and what it would take. - **The Delivery log box is on a refused message's page.** The picture he approved had left it out; the built page keeps it, with one sentence. And the sentence is less than the truth for a `From:` refusal: cleanup did - write a line about it under a queue id (`milter-reject: END-OF-MESSAGE … - 5.7.1 From: …`), but the check answers before the queue id reaches it, so - the row keeps none and the page cannot show that line. To do: either drop - the box for a refused row, or have the tailer attach the line to the row. + write a line about it under a queue id (`milter-reject: END-OF-MESSAGE ... + 5.7.1 From: ...`), but the check answers before the queue id reaches it, so + the row keeps none and the page cannot show that line. **Decided + 2026-10-13 (owner):** the box is required - he had overlooked its absence + from the picture, it was never a decision to drop it. The tailer attaches + the line to the row, for all three kinds of refusal; a step of + [audit-2026-10-12](../audit-2026-10-12.md) stage 1. - **The Outbound log has no status filter.** The mockup draws one; the built log filters by domain and application. With three kinds of refusal now told apart by tag, a filter by status and reason would be worth having. + **Decided 2026-10-13 (owner):** a filter by status, without the split by + reason, and a filter by day as the Inbound log has; a step of [audit-2026-10-12](../audit-2026-10-12.md) stage 4. - **"Messages deferred" counts connections Postfix turned away.** Postfix refuses from the connection on, before a message is offered, so a monitor that only connects during an outage is counted. The guide says so; the diff --git a/docs/plans/preflight.md b/docs/plans/preflight.md deleted file mode 100644 index 5d1167d..0000000 --- a/docs/plans/preflight.md +++ /dev/null @@ -1,77 +0,0 @@ -# Plan: preflight (installation check page) - -**Status:** candidate -**Date:** 2026-08-19 -**Version:** `1.x` MINOR; `candidate` until explicitly agreed. - ---- - -## Goal - -A dedicated `/server/preflight` page (global-admin only) that runs infrastructure-level -checks and helps the administrator confirm the instance is fully configured and -ready to deliver mail. Each failed check shows an actionable recommendation -(what to do or where to look). - -## Mockup - -[preflight](../assets/panel-redesign/panel/preflight.html), under the design -contract of [panel-redesign.md](panel-redesign.md): **Server › Preflight**, -path `/server/preflight`. - -## Scope - -**In:** - -- rDNS / PTR verification (reuse `dnscheck.Checker.checkServer`) -- TLS certificate validity + hostname match (CN/SAN contains `SELFPOST_HOSTNAME`) -- Port connectivity: 25, 465, and 587 (if `SUBMISSION_ENABLE`) — dial own - public IP from inside the container, expect a 220 banner -- HELO/EHLO banner — connect to localhost:25, verify Postfix announces - `SELFPOST_HOSTNAME` (not `localhost` or a container ID) -- Trusted proxy headers: - - If `TRUSTED_PROXY_CIDR` set: verify peer IP is in range and - `X-Forwarded-For` is present - - If `PANEL_COOKIE_SECURE=true`: verify `X-Forwarded-Proto: https` - - If `TRUSTED_PROXY_CIDR` empty: informational warning (panel exposed - directly) -- DKIM signing — OpenDKIM milter socket reachable (reuse `health.CheckSocket`) -- Send test email — admin enters recipient address, system sends via - localhost:25, shows queued/error result - -**Out:** - -- Per-domain DNS checks (SPF, DKIM TXT, DMARC) — those live on domain detail - pages. -- Delivery tracking of test email (only confirms local MTA accepted). - -## Done when - -An administrator can open `/server/preflight`, see a traffic-light summary of all -infrastructure prerequisites, read specific fix instructions for any failures, -and send a test email to verify end-to-end flow. - -## Risks - -- Port-connectivity check from inside the container may not reflect external - reachability if NAT hairpin is unsupported (document limitation). -- Test email requires at least one configured domain (or falls back to - `postmaster@SELFPOST_HOSTNAME`). - -## Implementation checklist - -Target version cut: TBD (MINOR). One commit per step; code only after roadmap -status is **agreed**. See [development.md](../development.md) § Plan checklists. - -- [ ] Port-connectivity helper (`internal/health/port.go`) — dial own IP, read - SMTP banner — **Sonnet** -- [ ] HELO banner check (connect localhost:25, parse 220 line) — **Sonnet** -- [ ] Hostname-in-certificate check (extend or wrap `health.CheckCertificate`) - — **Sonnet** -- [ ] Handler `handlers_preflight.go` (assemble all checks, actionable - recommendations) — **Sonnet** -- [ ] Test-email handler (`POST /server/preflight/test-email`, `net/smtp` to - localhost:25) — **Sonnet** -- [ ] Template `preflight.html` (traffic-light cards + test-email form) — - **Sonnet** -- [ ] Routes in `web.go`, nav link — **Sonnet** diff --git a/docs/roadmap.md b/docs/roadmap.md index 017c24e..7cfeac8 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -28,21 +28,20 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | ID | Topic | Status | Progress | Plan | |---|---|---|---|---| -| preflight | Installation check page (`/preflight`) | candidate | — | [plans/preflight.md](plans/preflight.md) | -| contributing | `CONTRIBUTING.md` | candidate | — | — | +| health-setup-checks | Server › Health also checks the certificate's name, Postfix's greeting and the trusted proxy (was preflight) | agreed (2026-10-13) | — | — | +| test-email | Send a test message from the panel (was preflight) | agreed (2026-10-13) | — | — | | inbound-antispam-panel | Inbound antispam journal + allow/deny lists | shipped `2.1.0` | 12/12 | [plans/inbound-antispam-panel.md](plans/inbound-antispam-panel.md) | -| csrf-tokens | Per-session CSRF tokens on state-changing forms | candidate | — | — | -| template-data-typing | Typed structs for template data instead of `map[string]any` | candidate | — | — | -| structured-logging | `log/slog` with levels and fields | candidate | — | — | +| origin-strict | Refuse a state-changing request that carries neither `Sec-Fetch-Site` nor `Origin` (was csrf-tokens) | agreed (2026-10-13) | — | — | +| backup-reauth | Ask for the account password before a backup or a domain export is downloaded | agreed (2026-10-13) | — | — | | review-2026-08-followups | Remaining findings of the 2026-08-19 code review | candidate | — | — | | panel-notifications | E-mail notifications about important events | shipped `2.3.0` | 10/10 | [plans/panel-notifications.md](plans/panel-notifications.md) | | delivery-log-storage | Keep a message's `mail.log` lines with its send-log row | shipped `2.2.0` | 8/8 | [plans/delivery-log-storage.md](plans/delivery-log-storage.md) | -| password-reset | Password reset by e-mail | candidate | — | — | -| phone-desktop-mode | A phone browser in "desktop site" mode gets the phone navigation | candidate | — | — | +| password-reset | Password reset by e-mail | agreed (2026-10-13) | — | — | +| phone-desktop-mode | A phone browser in "desktop site" mode gets the phone navigation | agreed (2026-10-13: after stages 1 and 2 of audit-2026-10-12) | — | — | | from-header-binding | Hold a message's `From:` header against the application that sends it | shipped `2.4.0` | 9/9 | [plans/from-header-binding.md](plans/from-header-binding.md) | | panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | | schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) | -| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, six stages | agreed (2026-10-12: stage 0 first, then 1+2; the rest after) | 0/45 | [audit-2026-10-12.md](audit-2026-10-12.md) | +| audit-2026-10-12 | Follow-ups of the 2026-10-12 full-tree code audit, six stages | agreed (2026-10-12: stage 0 first, then 1+2; the rest after) | 0/48 | [audit-2026-10-12.md](audit-2026-10-12.md) | **Recommended order** (not binding; owner, 2026-10-09: feature order follows development convenience): **panel-redesign** shipped as `2.0.0` (2026-10-11) and @@ -60,9 +59,12 @@ queue-retries in `[1.3.1]`; the 2026-08-13 full-tree review follow-ups are in `[1.3.0]`. Candidates need explicit agreement before they join the queue. The 2026-08-19 full-tree code review is closed except for four items, all -listed above and none of them urgent: [csrf-tokens](#csrf-tokens), -[template-data-typing](#template-data-typing), -[structured-logging](#structured-logging) and +listed above and none of them urgent: [origin-strict](#origin-strict) (its +"add CSRF tokens", decided otherwise on 2026-10-13), +template-data-typing (done by `2.0.0`; its last map is a step of +[audit-2026-10-12](audit-2026-10-12.md) stage 4), +structured-logging ([dropped](#dropped-with-the-condition-for-return) on +2026-10-13) and [review-2026-08-followups](#review-2026-08-followups). What that review produced is in [CHANGELOG.md](../CHANGELOG.md) `[1.9.2]`; its own document was removed once these four items carried everything still open, and stays in @@ -75,24 +77,6 @@ is `done/total` checklist steps in that plan ([development.md](development.md) --- -## contributing - -**Goal:** `CONTRIBUTING.md` in the root — the dev loop, the checks to run -before a PR, the commit protocol; [development.md](development.md) links to it -rather than repeating it. - -**Boundary:** process documentation; worth writing once there is an external -flow of PRs. - -**Done when:** the file is in the root and development.md does not duplicate -it. - -**Dependencies / risks:** with a single developer and no PRs, this is low -priority. -**Version:** no bearing on semver. - ---- - ## panel-docs **Status:** done — shipped in `[1.8.0]` (2026-08-18). The implementation plan @@ -144,105 +128,122 @@ journal PII; global-admin-only RBAC. --- -## preflight +## health-setup-checks -**Goal:** a `/preflight` page (global-admin only) that verifies instance-level -infrastructure: rDNS/PTR, TLS certificate + hostname match, port 25/465/587 -reachability, HELO banner correctness, reverse-proxy headers, DKIM milter -socket, and a test-email sender. Each check shows an actionable fix -recommendation on failure. +**Decided by the owner on 2026-10-13**, in place of the former candidate +*preflight* (a page of its own, Server › Preflight, planned in August before +Server › Health existed). Of that plan's eight checks Health already makes +three (reverse DNS, certificate validity, the DKIM socket); five were missing. +Three of the five go into Health, one becomes [test-email](#test-email), one +is not built ([Dropped](#dropped-with-the-condition-for-return)). No second +page of checks. -**Boundary:** instance infrastructure only; per-domain DNS checks remain on -domain detail pages. Does not track test-email delivery beyond local MTA -acceptance. +**Goal:** three more lines on Server › Health, in the cards that are there: -**Done when:** the page loads, all checks run with traffic-light results and -recommendations, and the test-email form successfully submits a message through -Postfix. +- **The certificate is for this server**: its names include + `SELFPOST_HOSTNAME` (TLS certificate card). Today a certificate for another + name is "valid". +- **Postfix greets with the hostname**: the 220 line on port 25 names + `SELFPOST_HOSTNAME`, not `localhost` or a container id (Hostname card). +- **The trusted proxy**: the address the panel sees this request from, and + whether it took it from `X-Forwarded-For`. With `TRUSTED_PROXY_CIDR` wrong + or empty behind a proxy the header is ignored + ([internal/web/auth/auth.go](../internal/web/auth/auth.go)), every client + has the proxy's address and the sign-in limiter is shared by all of them - + silently. -**Dependencies / risks:** port-connectivity check may not reflect external -reachability behind NAT without hairpin; test email needs at least one domain or -falls back to `postmaster@SELFPOST_HOSTNAME`. +**Boundary:** lines in existing cards, each with what to do when it fails; +no new card unless the mockup shows the lines do not fit. Mockup-first +([plans/panel-redesign.md](plans/panel-redesign.md) § Enforcement). The +mockup `preflight` stays drawn and unbuilt. -**Version:** `1.x` MINOR; `candidate`. +**Done when:** each line is shown wrong on the e2e stand or in a test with a +wrong certificate name, a wrong greeting and an untrusted proxy, and right on +the test server. + +**Version:** `2.x` MINOR; `agreed`. --- -## csrf-tokens +## test-email -**Goal:** a per-session CSRF token on every state-changing form, checked -server-side in addition to the `Sec-Fetch-Site` / `Origin` check the panel -relies on today (`originAllowed` in -[internal/web/security.go](../internal/web/security.go)). +**Decided by the owner on 2026-10-13**, taken out of the former *preflight*. -**Boundary:** defence in depth, not a replacement — the header check stays and -keeps covering requests that carry no form. The token is bound to the existing -session record; no new session model, no cookie changes. +**Goal:** a global administrator sends a test message from the panel to an +address they type, and sees whether Postfix took it; the message then appears +in Outbound › Log like any other. Today nobody can send through the server +without setting up an application first. -**Done when:** every POST form and htmx fragment that changes state carries a -token, a missing or mismatched token is refused with a test that proves it, and -[security.md](security.md) describes the layered model in one place instead of -two. +**Boundary:** global role only; from `postmaster@SELFPOST_HOSTNAME`, the +sender notifications use, or from a sending domain the administrator picks - +settled at the mockup. It does not follow the message further than the log +does. Where the form lives (Health, or a sending domain's page) is settled at +the mockup; mockup-first. -**Dependencies / risks:** [security.md](security.md) currently records the -opposite decision — origin checking without tokens, with the accepted risk and -its revisit triggers written out in the ADR § *CSRF via origin checking*. The -2026-08-19 review recorded "add tokens" without turning it into a task, which is -why this item exists. Settle which decision stands before writing code; both -outcomes are honest, and "keep headers" means deleting this item rather than -leaving the contradiction standing. The work itself is a hidden field in roughly -two dozen forms plus the htmx swap paths. +**Done when:** the form sends, the row appears in the log, the form is +rate-limited and refuses a recipient list, and it passes a security review - +a form that sends mail is a new surface. -**Version:** `1.x` MINOR; `candidate`. +**Version:** `2.x` MINOR; `agreed`. --- -## template-data-typing +## origin-strict -**Goal:** replace the `map[string]any` template payloads in -`internal/web/handlers` with typed structs (`DomainDetailData`, `SettingsData`, -...), so a renamed or mistyped key is a compile error instead of a silently -empty page section. +**Decided by the owner on 2026-10-13**, in place of the former candidate +*csrf-tokens* (per-session tokens on every form, recorded by the 2026-08-19 +review): no tokens; the origin check is made strict instead. -**Boundary:** handler-to-template plumbing only; no template redesign, no route -or behaviour change. `renderDomainDetail` -([handlers_apps.go](../internal/web/handlers/handlers_apps.go)) sets around -forty keys on one map and is the case that motivates the item; roughly ten -render sites still pass maps. +**Goal:** `originAllowed` +([internal/web/security.go](../internal/web/security.go)) refuses a +state-changing request that carries neither `Sec-Fetch-Site` nor `Origin`. +Today such a request is let through, which leaves a client that sends neither +header open to a request forged by any site +([security.md](security.md) § Accepted risks). -**Done when:** the page handlers pass structs, the shared footer/base fields -(`Version`, `Copyright`, `SourceURL`) still arrive on every page, and the -template guard tests pass unchanged. +**Boundary:** one branch of one function, its test, and the documents. No +template changes, no hidden fields. Before the change, find what posts to the +panel without a browser (the e2e stand, scripts) and make it send `Origin`. -**Dependencies / risks:** mechanical but wide — it touches every -`handlers_*.go` and every template that reads those keys, and a missed key is -invisible unless a test renders that page. Worth pairing with the handler tests -in [review-2026-08-followups](#review-2026-08-followups) so the pages are -covered before they are rewired. Reviewer's estimate: 2–3 hours. +**Done when:** a `POST` with neither header is refused with a test that proves +it, the e2e stand passes, the accepted risk about header-less requests is +struck from [security.md](security.md) and its ADR states the strict rule. -**Version:** no bearing on semver; `candidate`. +**Dependencies / risks:** a browser that sends neither header can no longer +save anything in the panel - accepted, the panel's users pick their own +browsers. Tokens stay a recorded alternative, not a task: the conditions under +which they are built are in [security.md](security.md), ADR § *CSRF via origin +checking* (**Revisit if**). + +**Version:** `2.x` PATCH; `agreed`. --- -## structured-logging +## backup-reauth -**Goal:** move from `log.Printf` to `log/slog` — levels, structured fields, and -one logger injected through constructors, instead of the package-level -`logf = log.Printf` alias repeated in five packages. +**Agreed with the owner on 2026-10-13**, out of +[review-2026-08-followups](#review-2026-08-followups). -**Boundary:** internal logging only. The destination stays stdout/stderr under -supervisord; no log shipping, no new dependency. Lines the panel's own Status -page and log tailer depend on must keep parsing. +**Problem:** Server › Backup hands the full backup and a domain export to any +live session of a global administrator +([handlers_backup.go](../internal/web/handlers/handlers_backup.go)). The +archive holds the whole database - panel password hashes, application (SASL) +passwords, DKIM keys - so a stolen session cookie, or a panel left open, is a +quiet copy of the server. *Encrypt with a password* does not help: the thief +chooses that password. -**Done when:** packages take a `*slog.Logger`, the per-package `logf` aliases -are gone, and levels are used deliberately (an error is not an info). +**Goal:** both downloads ask for the current account password, checked the +way a password change checks it and under the same limiter. -**Dependencies / risks:** the log tailer reads Postfix's log rather than the -panel's, so the blast radius is small — but anything that greps the panel's -output (e2e assertions, operator habits) sees a new format. Reviewer's -estimate: 3–4 hours. +**Boundary:** one field in each of the two forms (mockup-first); import is +not touched; the `selfpost-backup` CLI is not touched - it runs inside the +container. -**Version:** no bearing on semver; `candidate`. +**Done when:** a download without the password or with a wrong one is refused +with a test that proves it, attempts are limited, and +[security.md](security.md) § Backup says so. + +**Version:** `2.x` MINOR; `agreed`. --- @@ -255,7 +256,7 @@ is in the table below; the rest of it shipped in | Area | Item | |---|---| -| Security | Re-authenticate (ask for the current password) before a backup download — [handlers_backup.go](../internal/web/handlers/handlers_backup.go) hands the archive to any signed-in session | +| Security | ~~Re-authenticate (ask for the current password) before a backup download~~ — struck: agreed on 2026-10-13 as an item of its own, [backup-reauth](#backup-reauth) | | Tests | The `internal/dmarc` service layer (`service.go`, `addresses.go`) has no tests | | Tests | No direct tests for the apps / domains / users / dmarc handlers; `authz_test` covers them only indirectly | | Tests | Nothing covers error page templates, a corrupted backup archive, malformed or oversized DMARC XML, delivery-log pagination edges, concurrent store operations, or zero-traffic `store` stats | @@ -273,9 +274,7 @@ becomes its own roadmap item instead. **Done when:** each row is either implemented or struck with a written reason. -**Dependencies / risks:** the test rows overlap -[template-data-typing](#template-data-typing) — write the handler tests first if -both are taken up. One finding of that review was **declined** and is +**Dependencies / risks:** One finding of that review was **declined** and is deliberately absent: `//go:build linux` for the `/proc` reader in [internal/health/machine.go](../internal/health/machine.go). That code is parameterised by root directory, calls nothing platform-specific, already @@ -383,8 +382,22 @@ account e-mail instead of asking the global administrator to set a new one. same answer whether or not the username exists. The only global administrator without a working e-mail still recovers from the container (today's path). +**The e-mail must be confirmed (owner, 2026-10-13).** A reset link goes only +to an address its owner has confirmed; an account whose e-mail is not +confirmed has no reset, and recovers as today (the global administrator sets +a password, or the container). The panel does not confirm addresses today - +`users.email` is whatever was typed - so the item builds that first: saving +an e-mail under Account mails a one-time link to it, opening the link marks +the address confirmed (a migration: the mark on `users`), changing the +address clears the mark, and Account shows which it is. An address typed for +a user by the global administrator is unconfirmed until that user opens the +link. To settle at the mockup: whether notifications (2.3.0) keep going to an +unconfirmed address. + **Done when:** the flow works end to end, tokens expire and cannot be reused, -requests are rate-limited, and it passes a security review. +requests are rate-limited, an account with an unconfirmed e-mail gets no +link and the sign-in page answers the same for it, and it passes a security +review. **Dependencies / risks:** the account e-mail; an account-takeover surface where there was none — security review is part of the item, not an afterthought. @@ -392,7 +405,10 @@ Not drawn yet. Changing the account e-mail must then require the current password (2.0 security review), because the e-mail becomes where a reset link goes and today Account saves it without the password. -**Version:** `2.x` MINOR; `candidate`. +**Version:** `2.x` MINOR; `agreed` (owner, 2026-10-13: domain administrators +to come need it - they are not the owner and should not have to ask him for +a new password). Mockup-first; changing the account e-mail asks for the +current password in the same cut. --- @@ -427,7 +443,9 @@ expected there. A design change is mockup-first has one coherent layout, checked on a phone in "desktop site" mode and in a 980 px window. -**Version:** `2.x` PATCH or MINOR; `candidate`. +**Version:** `2.x` PATCH or MINOR; `agreed` (owner, 2026-10-13), taken up +after stages 1 and 2 of [audit-2026-10-12](audit-2026-10-12.md) - mail shown +wrong and the tenant boundary come before layout. --- @@ -542,6 +560,46 @@ its own. A missed gate leaves a mid-chain 1.x database silently stuck. --- +## Dropped, with the condition for return + +Items the owner took off the roadmap. Each names what would bring it back; +without that it is not offered again. + +- **structured-logging** (dropped 2026-10-13) - move the panel from + `log.Printf` to `log/slog` with levels and fields, one logger passed through + constructors. Dropped because the panel's log has one reader, the owner, by + eye in `docker logs`; the change is wide (some 225 call sites), changes no + behaviour, and gives a new format to everything that reads the output. + **Returns if:** the log goes to an outside collector, or it has to be + filtered by level. +- **contributing** (dropped 2026-10-13) - a `CONTRIBUTING.md` in the root: + the dev loop, the checks before a PR, the commit protocol. Dropped because + there is one developer and no PRs, and everything the file would say is in + [development.md](development.md); a second file is a second copy to keep + in step. **Returns if:** the first outside PR arrives, or the owner decides + to invite contributors. +- **A check that ports 25, 465 and 587 are reachable** (dropped 2026-10-13, + from the former *preflight*). The panel can only dial its own public + address from inside the container: behind NAT without hairpin that shows an + open port as closed, and with hairpin it shows as open a port the provider + blocks from outside. A card that is wrong both ways is worse than none, and + an honest check needs a probe outside the server, which is outside the + product ([product.md](product.md)). The administrator's browser cannot do + it either: 25, 465 and 587 are on the Fetch standard's list of blocked + ports. Two substitutes were offered and declined the same day: a Health + line "last connection from outside on each port", read from `mail.log`, and + a command in the help to run from one's own machine. **Returns if:** the + owner accepts an outside probe, or asks for one of the two substitutes. +- **preflight as a page of its own** (dropped 2026-10-13): see + [health-setup-checks](#health-setup-checks). Its plan, + `plans/preflight.md`, was removed and stays in git history. +- **csrf-tokens** (decided 2026-10-13) - session-bound tokens on every form. + Replaced by [origin-strict](#origin-strict); the conditions for building + tokens after all are in [security.md](security.md), ADR § *CSRF via origin + checking*. + +--- + ## Ideas, not on the roadmap Thoughts kept so they are not lost. They are not candidates: nothing here is diff --git a/docs/security.md b/docs/security.md index aca88c9..ad09533 100644 --- a/docs/security.md +++ b/docs/security.md @@ -165,7 +165,7 @@ assigned outbound domains and inbound domains **separately**, each list with (`originAllowed`, [internal/web/security.go](../internal/web/security.go)): `Sec-Fetch-Site` must be `same-origin`, or `Origin` must name the panel's own host; the session cookie is `SameSite` as well. -- There are **no CSRF tokens yet**. The decision, its price and the conditions +- There are **no CSRF tokens**. The decision, its price and the conditions for revisiting it are in the accepted risks and the ADR below. ### Errors shown to users @@ -306,7 +306,9 @@ deferred item from the roadmap. against, and a strict mode would not "protect" such a client, it would simply break the panel in it. Tightening is one line in `originAllowed` ([internal/web/security.go](../internal/web/security.go)): return `false` - instead of `true` in the "neither header present" branch. + instead of `true` in the "neither header present" branch. That tightening + is agreed (owner, 2026-10-13) as + [origin-strict](roadmap.md#origin-strict); this bullet goes when it ships. - **Session-bound CSRF tokens are not implemented.** The origin check closes the neighbouring-subdomain case but depends on browser behaviour; a token does not. The price is a hidden field in roughly two dozen forms. The trigger to @@ -315,10 +317,12 @@ deferred item from the roadmap. (see the ADR below). A token would not save the panel from XSS inside it either: code executing in the panel's origin sends the request itself — against that, `html/template` auto-escaping and CSP do the work, which is why - templates must contain no inline scripts and no inline styles. Tracked as - [csrf-tokens](roadmap.md#csrf-tokens): implementing that item replaces this - bullet and the ADR below, and dropping it leaves them as the single record of - the decision. The two must not disagree. + templates must contain no inline scripts and no inline styles. Decided by + the owner on 2026-10-13: no tokens; the roadmap item that asked for them is + dropped and the origin check is made strict instead + ([origin-strict](roadmap.md#origin-strict)). This bullet and the ADR below + are the single record of the decision, and the ADR's **Revisit if** says + when tokens are built after all. - **Destructive-action confirmation (`data-confirm`) is JavaScript-only.** Delete, regenerate-password, and clear-rate-limit forms carry a `data-confirm` prompt handled entirely in @@ -573,6 +577,16 @@ whose browsers/devices are not vetted) — at that point cross-role request forgery inside the panel would need its own analysis, separate from the external-site case this ADR covers. +**Decision of 2026-10-13 (owner).** Asked to choose between this ADR, tokens, +and a strict origin check, the owner chose the ADR with the strict check +([origin-strict](roadmap.md#origin-strict)) and no tokens. Session-bound tokens +are the answer to return to - a hidden field in every state-changing form and +htmx path, checked against the session, on top of the origin check - when +either condition above comes true, and also when: a browser the panel must +support is found to send neither header or to send them wrongly; the panel +gains a state-changing request that a browser sends without these headers; or +an outside review requires a defence that the server alone holds. + ## How this list grows The pre-release vulnerability review (history — CHANGELOG `[0.5.0]` Security)