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 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5.5
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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**
|
||||
+164
-106
@@ -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
|
||||
|
||||
+20
-6
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user