From 6023feb40504cb02ae6901037cd189943d19430b Mon Sep 17 00:00:00 2001 From: Mix Date: Thu, 15 Oct 2026 14:41:24 +0300 Subject: [PATCH] audit 6: documents, comments, dead code and staticcheck; the test gaps closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last stage of the 2026-10-12 audit; no cut. Doc drift: architecture.md (the sender check in § OpenDKIM, the limit: row's 48 hours, the inbound log and filter-list routes and who may open them, migrations to 0011 and every table, KitPage as production data, Reload configuration as a restore writes, the panel's roles counted), schema-migrations.md (tables and columns since v2), guide.md (a delivery's lines go at the next prune; INBOUND_JOURNAL_MILTER_SOCKET moved to architecture.md's internal list), development.md (the env guard, the deferral the e2e proves, its fixed sleeps), roadmap.md. Comments say what the code does now; TestBaselineIsTheOnlyMigration is TestChainStartsAtTheBaseline; the milter package doc names its three milters. Dead code gone: view.Facts, dmarc HostedSuggestion and ValidateHostedAddress, app UpdateMode (each with the test that alone kept it), the method checks the method-bound routes made dead; Settings.WithResult calls resultFlash. staticcheck 2026.1 in test.yml on both modules, clean: the e2e package builds without its tests now (a test and two constants moved out of non-test files), three findings fixed. The test moved, TestImageBuildPreservesLogrotateMode, had sat in a non-test file and never run; run, it failed on the entrypoint (docker run without a hostname) and now asks with --entrypoint stat. The test gaps no step named, each closed: follow() with mail.log gone past blindMin and made again; CheckRestore with a manifest that is not JSON; the origin check with a port; every /server/ pattern in web.go through server() and in the routes table (it found GET /server/health/fragment missing from the table - guarded, not tested); L9 as the owner decided it on 2026-10-12, the comment on senderSession.Headers and the test passing an h that holds the earlier messages' From:. Plan stage 6 ticked; roadmap audit row done, 81/81. Tested on the test server: go vet + go test of 27 packages, make e2e (all pass, 513 s) on this tree; staticcheck locally on both modules. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test.yml | 9 +++ deploy/docker-compose.yml | 6 +- docs/architecture.md | 56 +++++++++----- docs/audit-2026-10-12.md | 86 +++++++++++++++++++++- docs/development.md | 24 ++++-- docs/guide.md | 5 +- docs/roadmap.md | 29 ++++---- docs/schema-migrations.md | 11 ++- internal/app/service.go | 18 ----- internal/app/service_test.go | 20 ----- internal/backup/backup_test.go | 22 ++++++ internal/dmarc/refusals_test.go | 12 +-- internal/dmarc/service.go | 14 ---- internal/health/machine.go | 2 +- internal/logtail/logtail.go | 9 ++- internal/logtail/logtail_test.go | 54 +++++++++++++- internal/milter/milter.go | 17 +++-- internal/milter/sender.go | 5 ++ internal/milter/sender_test.go | 9 ++- internal/store/inboundspam.go | 5 +- internal/store/schema_test.go | 5 +- internal/store/sendlog.go | 21 +++--- internal/store/store.go | 2 +- internal/web/guard_routes_test.go | 3 +- internal/web/handlers/handlers_dmarc.go | 5 -- internal/web/handlers/handlers_settings.go | 11 +-- internal/web/handlers/handlers_users.go | 32 ++++---- internal/web/home_test.go | 26 +++++++ internal/web/security_test.go | 24 ++++++ internal/web/view/account.go | 7 +- internal/web/view/components.go | 10 --- internal/web/view/components_test.go | 3 - test/e2e/cert_reload_check.go | 4 +- test/e2e/logrotate_check.go | 47 ------------ test/e2e/logrotate_mode_test.go | 55 ++++++++++++++ test/e2e/mail_helpers.go | 9 +++ test/e2e/main_test.go | 8 -- test/e2e/sasldb_test.go | 3 +- test/e2e/smtp_client.go | 2 +- 39 files changed, 434 insertions(+), 256 deletions(-) create mode 100644 test/e2e/logrotate_mode_test.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c60f8f0..804214a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -40,6 +40,14 @@ jobs: - name: Vet run: go vet ./... + # Unused code, dead branches and the usual slips go vet does not look + # for. The version is pinned: a new release adds checks, and a check + # that appears under a push is a red build nobody asked for. 2026.1 is + # the release for Go 1.26. test/e2e is a module of its own, so ./... + # skips it; it is checked in its own step below. + - name: Staticcheck + run: go run honnef.co/go/tools/cmd/staticcheck@2026.1 ./... + - name: Test run: go test ./... @@ -52,3 +60,4 @@ jobs: run: | go vet ./... go test -c -o /dev/null . + go run honnef.co/go/tools/cmd/staticcheck@2026.1 ./... diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 739a02a..919108b 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -5,7 +5,7 @@ # on the HOST (the target audience for this project typically runs Apache there # already) and reverse-proxies HTTPS for the panel; it is not containerised # here. See ../apache/selfpost-vhost.conf for a ready vhost fragment, and -# docs/guide.md "Reverse proxy" for the certbot steps that produce the PEM files +# docs/guide.md "Reverse proxy (mandatory)" for the certbot steps that produce the PEM files # this compose file mounts. # # Usage: @@ -75,7 +75,7 @@ services: # ./certs holds fullchain.pem and privkey.pem: the files themselves, or # links to where your reverse proxy's certbot keeps them. - ./certs:/etc/postfix/tls:ro - # With certbot on the host (docs/guide.md, "Reverse proxy", Apache): + # With certbot on the host (docs/guide.md, "Reverse proxy (mandatory)", Apache): # certs/ holds links into /etc/letsencrypt/live/, and certbot's live/ # files are themselves relative links into archive/ - so the whole # directory is mounted, at the same path, for the links to lead @@ -90,7 +90,7 @@ services: # reachable from the internet without TLS. - "465:465" # 587 is mapped even when SUBMISSION_ENABLE=false; Postfix listens only - # when the variable is true — see docs/guide.md "Published ports". + # when the variable is true — see docs/guide.md "Ports". - "587:587" # 25 is mapped even when INBOUND_RELAY_ENABLE=false; Postfix listens only # when the variable is true. diff --git a/docs/architecture.md b/docs/architecture.md index 4423ce3..d11e137 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -207,10 +207,10 @@ The server's own name (`SELFPOST_HOSTNAME`) has a key and a line in both tables too, without being a row of the registry (`domain.Service.SetServerName`): the panel's notifications are sent as `postmaster@` that name and signed with it. No application can use it as an -envelope sender - the sender map is built from the registry - but the -`From:` header is not held against the login, so an application can still -have a message signed as the server ([security.md](security.md) § Accepted -risks). A sending domain of the same name shares its key and selector with +envelope sender - the sender map is built from the registry - and since +`2.4.0` none can have a message signed as the server by writing it into +`From:` either: the sender check holds the header against the login (§ Sender +check). A sending domain of the same name shares its key and selector with the server; deleting that domain leaves the server signed under the default selector, which is the same key only if that was the domain's. At its start the notifier makes sure the tables have the line and name the selector the @@ -219,7 +219,8 @@ domain is added or deleted. ### Panel binary ([cmd/panel](../cmd/panel)) -One process, five roles (six with the inbound relay on): +One process, six roles (seven with the inbound relay on: its inbound journal +milter, § Postfix above, port 25): 1. **HTTP server** — `:8080` (`PANEL_HTTP_ADDR`); HTTPS terminated by reverse proxy only. On start it runs `postconf -h -x` once for the deferred-mail retry @@ -398,7 +399,8 @@ is sent; the operator's view is guide § Notifications. minutes after it - a count alone would let the last check before a restart and the first after it stand for "an hour apart"; back the same way. The certificate's row remembers which threshold was said for which expiry. A - deleted domain's row goes with it. + deleted domain's `dns:` row goes at the next check; its `limit:` row stays + until it was last said more than 48 hours ago (`CheckLimits`). - **Said means sent.** A problem that arises while no user can be told, or while Postfix does not take the message, stays unsaid and is told at a later check. Its ending, with nobody left to tell, is simply over. @@ -675,6 +677,7 @@ only reads (the delete confirmation at `GET …/delete` renders a form). | Outbound › Log, Queue | `GET /outbound/log` · `/outbound/log/fragment` · `/outbound/log/{id}` · `GET /outbound/queue` · `/outbound/queue/fragment` | | Outbound › DMARC reports | `GET /outbound/dmarc` · `…/dmarc/domains/{id}` · `…/dmarc/reports/{id}` — registered only with `DMARC_REPORTS_ENABLE=true` | | Inbound › Domains | `GET, POST /inbound/domains` · `/inbound/domains/{id}` · `POST …/dns-recheck`, `…/upstream`, `…/recipients` · `GET, POST …/delete` — registered only with `INBOUND_RELAY_ENABLE=true` | +| Inbound › Log, Filter lists | `GET /inbound/log` · `GET, POST /inbound/filter-lists` · `POST /inbound/filter-lists/{id}/delete` — registered only with `INBOUND_RELAY_ENABLE=true` | | Server | `GET /server/health` (+ `/fragment`) · `POST /server/health/recheck`, `/server/health/reload` · `GET /server/log` (+ `/fragment`) · `GET, POST /server/backup` · `POST /server/backup/import` · `/server/users`, `…/new`, `…/{uid}`, `…/{uid}/delete` · `GET, POST /server/settings` · `GET /server/components` | | Account | `GET /account` · `POST /account/profile`, `/account/password` | | User menu | `GET /help` · `POST /logout` (public mux) | @@ -718,7 +721,11 @@ is not. ([handlers/handlers_inbound.go](../internal/web/handlers/handlers_inbound.go)): `requireInbound` (the list), `requireInboundDomain` (one domain — page, upstream, recipients, DNS re-check) and `requireInboundGlobal` (add, delete). - All three answer 404 alike. + All three answer 404 alike. The **Inbound log** (`/inbound/log`) is open to + anyone who reaches an inbound domain (`requireInbound`) and shows the rows + of their domains only; **Filter lists** (`/inbound/filter-lists`) are the + instance's and the global role's (`requireInboundGlobal`) + ([handlers/handlers_inbound_log.go](../internal/web/handlers/handlers_inbound_log.go)). - **The DMARC hub** is open to anyone with outbound reach for the reports of their own domains; the *Ingest* box (server-wide statistics) is rendered for the global role only. @@ -796,8 +803,9 @@ is one layout and one way to render. `panel.css` (the mockups' stylesheet, rule for rule), `htmx.min.js`, `panel.js`, the IBM Plex subset and the logo files. Served with a content-derived `ETag`, and `/static/` does not list its files. The kit page - `GET /server/components` renders every partial in every state from fixtures - (global role only). + `GET /server/components` renders every partial in every state from the data + of `KitPage` ([kit.go](../internal/web/view/kit.go), production code that the + guard tests render too; global role only). **Guard tests** hold the design in place; they run in `go test ./...` and CI. They read their rules from the accepted mockups @@ -958,9 +966,9 @@ flowchart TB dmarcSvc --> postfix ``` -The four roles inside the `panel` process (HTTP server, journal-milter, -log-tailer, rate-limit recalc) share one binary and one SQLite handle on -purpose — see +The roles inside the `panel` process (HTTP server, sender check, +journal-milter, log-tailer, rate-limit recalc, notifier) share one binary and +one SQLite handle on purpose — see [Panel binary](#panel-binary-cmdpanel) for why, and *Persistence* below for the single-connection trade-off that follows from it. @@ -993,8 +1001,9 @@ full backups also archive the operator's `./certs` PEM files when present. ### Database The schema is an embedded baseline, `internal/store/migrations/0001_init.sql`, -and the migrations after it (`0002_inbound_spam_log.sql`), applied by -`store.Open` in file order; `PRAGMA user_version` counts the files applied and +and the migrations after it (`0002` to `0011` today; the next is `0012`), +applied by `store.Open` in file order; `PRAGMA user_version` counts the files +applied and `PRAGMA application_id` is `0x53503230` ("SP20"). There is no 1.x migration chain. **2.0 starts from an empty data directory:** a database with a schema version but without the 2.x `application_id` — what every 1.x file looks like — @@ -1029,7 +1038,12 @@ The tables that carry the panel's model: `application_addresses`, `rate_limits` (no `allowed_ips` column), `send_log`, `send_log_lines` (what Postfix logged under a queue id — § Log tailer), `logtail_state`, `inbound_domains`, `inbound_transports`, - `inbound_recipients`, `dmarc_reports`, `dmarc_report_records`. + `inbound_recipients`, `inbound_spam_log` (port 25's decisions, § Postfix), + `inbound_spam_list` (the filter lists), `dmarc_reports`, + `dmarc_report_records`, `notify_state` and `notify_addressees` (what the + notifier has said and to whom, § Notifications), `sender_check_days` (the + sender check's downtime and the mail it deferred, per UTC day) and + `delivery_log_days` (the queue sweep's measurements, § Log tailer). Two processes write: the panel, on one connection, and `dmarc-ingest`, which Postfix starts per report. SQLite runs in WAL mode (see @@ -1051,13 +1065,14 @@ under `/data`), `docker-compose.yml`, `.env`, and `certs/` when present; version check on restore. Requires the project directory mounted read-only at `SELFPOST_DEPLOY_ROOT` (`/selfpost-deploy` in the default compose file). On the first successful boot after restore, the panel runs one **Resync** — OpenDKIM's -tables, Postfix's sender map, inbound relay maps (when +tables, Postfix's sender map, inbound relay maps and the filter lists (when `INBOUND_RELAY_ENABLE=true`), and DMARC maps (when `DMARC_REPORTS_ENABLE=true`) are re-derived from SQLite and both daemons are reloaded, so drift between the extracted archive and the database is healed before mail flows. Manual `POST /server/health/reload` (the *Reload configuration* button on Server › -Health) runs the same Resync, the filter lists with it, since `2.5.4`. -Stopped-container +Health) writes the same files in the same order (`HandleReload`; before +`2.5.4` it left out the filter lists and the DMARC maps), and on a failure +stops at that step and names it on the page. Stopped-container `tar` of `./data` alone remains possible for state-only copies (see guide). **Optional encryption** of the two secret-bearing downloads @@ -1095,7 +1110,10 @@ unsupported rather than as a missing doc: (`/data/selfpost.db`), `SELFPOST_SETUP_TOKEN_FILE` (`/data/setup-token`), `PANEL_HTTP_ADDR` (`:8080`), `JOURNAL_MILTER_SOCKET` (`/run/selfpost/journal.sock`), - `SENDER_CHECK_SOCKET` (`/run/selfpost/sender.sock`), `MAIL_LOG` + `SENDER_CHECK_SOCKET` (`/run/selfpost/sender.sock`), + `INBOUND_JOURNAL_MILTER_SOCKET` (`/run/selfpost/inbound-journal.sock` — the + inbound journal milter's socket, read by the panel, `build/postfix-config.sh` + and `build/postfix-wrapper.sh`, which must agree), `MAIL_LOG` (`/data/log/mail.log` — read by the panel and written by Postfix, so a change here has to be matched in `build/postfix-config.sh`), `PANEL_COOKIE_SECURE` (`true`), `OPENDKIM_SOCKET` diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index 46224d0..e2a5c09 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -496,6 +496,49 @@ tests; `TestRenderInboundMaps` has no two-routes-one-next-hop case (L5); rows of `review-2026-08-followups` (apps/domains/users/dmarc handlers directly, `internal/dmarc` service) are still open. +*Checked in stage 6 (2026-10-15):* **closed** since, each by the test that +now holds it: the H1 stamp (e2e `image_stamped_with_version`); cert re-copy +(e2e `renewed_certificate_picked_up`); a recreated log and lines written to +the old inode after the switch (`TestFollowTailsAndRotates`, +`TestFollowReadsTheRotatedFileAfterTheSwitch`, +`TestALogRotatedWhileDownIsReadToItsEnd`); a truncated `postqueue -p` +(`TestQueueIDsRefusesATruncatedListing`); a `sent` row left alone by the +sweep and by a reused id (`TestCloseUnreportedClosesOnlyQueuedRows`, +`TestUpdateStatusRewritesOnlyTheNewestRow`); H4 +(`TestAddApplicationRefusesALoginDifferingOnlyInCase`, +`TestServiceCreateFoldsTheLogin`); M1's ordering +(`TestARestoreStartThatFailsIsARestoreAgain`; the real reload by e2e +`full_backup_restore`); M3 (`TestSnapshotRefusesAMissingDatabase`, +`TestBackupOfAMissingDatabaseIsRefused`); M5 (`TestEncryptionPasswordFloor`); +L1 (`TestDecryptLeavesTheOutputAloneOnFailure`); L2 +(`TestCertsLinksAreFollowed`, `TestCheckRestoreReadsOnlyRegularFiles`); L3 +(`TestBytesAfterTheLastChunkRejected`); M13 (e2e +`panel_hung_mid_message_is_counted`, and the unit fixture now holds the +`cleanup` lines as the stand wrote them); M15 +(`TestServerPTRTimeoutIsUnknown`, +`TestServerAddressTimeoutIsUnknownAndKeepsTheAddresses`); +`TestLimitRefusalsSince` (the real `limitRefusalsQuery` is planned in +`TestSendLogQueriesUseTheirIndexes`); `TestOpenMigratesA20Database` (migrates +to the head, 11); M10 (`TestDomainImportIsCheckedLikeTheForms`); M11 +(`TestPanelJS`: `poll_test.js`, `unsaved_test.js`); M8 +(`TestLoginTakesAsLongForAnUnknownName`); M7 +(`TestTheLastGlobalSurvivesTwoAtOnce`); `internal/atomicfile` and +`internal/supervisor` (tests of their own); L5 +(`TestRenderInboundMapsOneTLSLinePerNextHop`); the handler rows of +`review-2026-08-followups` (struck in the roadmap with their reasons; users, +DMARC and apps have handler tests). The five then **still open**, named by +no step, were closed in stage 6 the same day: `follow()` with a log gone +past `blindMin` and made again +(`TestFollowCountsALogItCannotReadAndReadsItAgain`); `CheckRestore` with a +manifest that is not JSON (`TestCheckRestoreRefusesAManifestThatIsNotJSON`); +a Host with a port against an Origin (`TestOriginCheckComparesThePort`); +every `/server/` pattern in web.go registered through `server()` and listed +in the routes table (`TestEveryServerRouteIsGuardedAndListed` - it found +`GET /server/health/fragment` missing from the table; the route was guarded, +not tested); L9 as the owner decided it, the comment on +`senderSession.Headers` and `TestSenderCheckResetsBetweenMessages` passing +an `h` that holds the earlier messages' `From:`. + ### Checked and sound Worth writing down so the next review does not re-read it: the sender check's @@ -1512,20 +1555,55 @@ log it owns anyway; envdoc_test reads `${VAR:-}` and the Go helpers, not bare `$VAR` or python (none today); the 600 s pruning in crashexit is not tested (the count and the spared are). -### Stage 6 — documentation and roadmap sweep (no product code, no cut) +### Stage 6 — documentation and roadmap sweep (no product code, no cut) (done 2026-10-15) -- [ ] Every item under *Doc drift* above, file by file. — **Sonnet** +- [x] Every item under *Doc drift* above, file by file. — **Sonnet** + *Done 2026-10-15:* architecture.md (the sender check in § OpenDKIM, the + `limit:` row's 48 hours, the inbound log and filter-list routes and who + may open them, migrations to `0011` and every table, `KitPage` as + production data, Reload configuration as a restore writes, the panel's + roles counted again); schema-migrations.md (the tables and columns since + v2); guide.md (a delivery's lines go at the next prune; + `INBOUND_JOURNAL_MILTER_SOCKET` out of the operator's table, into + architecture.md's internal list); development.md (the env guard as + envdoc_test reads now, the deferral the e2e proves, the e2e's fixed + sleeps said where they are); roadmap.md's closing paragraph. Already right + by then: the `.alive` mark, `dmarc-ingest` as a second writer, the chain + head, M1's wording, the version stamp. - [ ] roadmap.md: done ahead on 2026-10-13/14 with the owner - `template-data-typing` struck, `structured-logging` dropped, `review-2026-08-followups` closed with each row placed in a stage or dropped with its reason. What is left: check that every *Test gaps* entry above is named by the step that touches its code, and add it to that step where it is not. — **Sonnet** -- [ ] Comments named above (`milter.go`, `sendlog.go`, `store.go`, `schema_test.go`, + *Done 2026-10-15:* see *Checked in stage 6* under *Test gaps*: five were + open and named by no step; each got its test in this stage (L9 as the + owner decided it). +- [x] Comments named above (`milter.go`, `sendlog.go`, `store.go`, `schema_test.go`, `machine.go`, `inboundspam.go`, `logtail.go`, supervisord.conf, entrypoint.sh, docker-compose.yml). — **Haiku** -- [ ] Dead code removal from § Nits, with `staticcheck` added to `test.yml` so it + *Done 2026-10-15:* each said what its code does now (the journal-milter + tempfails at MAIL FROM; the rejected status covers the sender check; the + two roles; the log is read whole for a filter list's hits; logrotate + uses `create`; the guide's headings as they are). `TestBaselineIsTheOnlyMigration` + renamed `TestChainStartsAtTheBaseline`. supervisord.conf and + entrypoint.sh were already right. +- [x] Dead code removal from § Nits, with `staticcheck` added to `test.yml` so it stays gone. — **Haiku** + *Done 2026-10-15:* gone: `view.Facts`, `dmarc.Service.HostedSuggestion` + and `ValidateHostedAddress`, `app.Service.UpdateMode` (each with the + test that alone kept it), the method checks of `HandleDomainDMARC` and the + `default:` branches of the user and settings handlers; + `Settings.WithResult` calls `resultFlash`. Kept: `domainIDSet` (used by + the send log's authorization), `KitPage()`'s `User` (the fixtures render + `KitPage()` without the handler); the `set-permissions` branch was gone + already. staticcheck 2026.1 (the release for Go 1.26) runs in test.yml + on both modules, clean; for the e2e module the package now builds without + its tests (a test and two constants moved out of non-test files), and + three findings there were fixed (ST1005, SA4006). The test moved, + `TestImageBuildPreservesLogrotateMode`, had sat in a non-test file since + it was written, and go test had never run it; run, it failed on the + entrypoint (`docker run` without a hostname) and now asks past it. ### Decided with the owner on 2026-10-12 diff --git a/docs/development.md b/docs/development.md index e4dca58..8096537 100644 --- a/docs/development.md +++ b/docs/development.md @@ -420,9 +420,14 @@ go test ./... ### Env documentation regression -`go test ./cmd/panel -run 'TestLoadConfigKeysDocumented|TestBuildScriptKeysDocumented|TestDocumentedKeysAreRead'` -— every new `loadConfig` key must appear in the env lists in [guide.md](guide.md) -([cmd/panel/envdoc_test.go](../cmd/panel/envdoc_test.go)). +`go test ./cmd/panel -run 'TestEnvironment'` compares what the code reads with +what the documents name ([cmd/panel/envdoc_test.go](../cmd/panel/envdoc_test.go)). +Read: the `envDefault` / `envInt` / `envBool` / `envSeconds` / `envDuration` / +`os.Getenv` / `os.LookupEnv` calls in `cmd/panel` and `internal` (tests aside), +and `${VAR:-…}` in `build/*.sh`. Named: [guide.md](guide.md) § Environment +variables (the table and the paragraph under it) and the internal list in +[architecture.md](architecture.md) § Configuration. A variable read and named +nowhere fails the first test; one named and read nowhere fails the second. ### Panel pages: the component kit and its guards @@ -470,12 +475,16 @@ operator would actually use. **Coverage (summary):** bootstrap → SMTP AUTH → delivery → DKIM verify → send-log `queued → sent`; negatives (no AUTH, relay, sender/login mismatch, -L1/L2 limits, milter fail-open, bad `SELFPOST_HOSTNAME`, session survives -`docker restart`); startup checks that supervisord actually brought up +L1/L2 limits, the panel down or hung mid-message (Postfix defers the mail +rather than letting it through: `panel_down_defers_mail`, +`panel_hung_mid_message_is_counted`), bad `SELFPOST_HOSTNAME`, session +survives `docker restart`); startup checks that supervisord actually brought up OpenDKIM, the panel, and Postfix (`checkSupervisorProcesses`), plus logrotate config-mode and forced-rotation checks (`checkLogrotateConfigMode`, `checkLogrotateRotation` — [test/e2e/logrotate_check.go](../test/e2e/logrotate_check.go)). -Polling with timeouts only — no fixed `sleep`. +Waits poll with a timeout (`waitFor`); a fixed sleep is the exception and says +why where it stands (`sender_check_test.go`: a downtime worth a second or two, +and 35 s to show a row is not written again). Requires **Docker + Compose v2** on the machine running the suite. @@ -573,7 +582,8 @@ Out of scope for v1.x: `CONTRIBUTING.md`, man pages, a separate docs site behaviour ships together with [guide.md](guide.md) / `.env.example` and a CHANGELOG entry (see [§ Commits and release build](#commits-and-release-build)). 2. **Env regression:** [cmd/panel/envdoc_test.go](../cmd/panel/envdoc_test.go) - fails on an undocumented `loadConfig` or build-script key. + fails on a variable the code or a build script reads that no document names, +and on one a document names that nothing reads. 3. **New gaps** go into [roadmap.md](roadmap.md) (or the active plan file), not silent drive-by edits. 4. **Schema changes:** update [schema-migrations.md](schema-migrations.md) in the diff --git a/docs/guide.md b/docs/guide.md index be6e488..5ab0ec4 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -228,7 +228,6 @@ expected to set; defaults match the code exactly. | `INBOUND_ANTISPAM_MILTER` | Optional milter on the inbound relay listener only (requires `INBOUND_RELAY_ENABLE=true`; not 465/587 or DMARC-only port 25). Empty = off. Format `inet:host:port` or `unix:/path`. Example with [deploy/antispam/docker-compose.antispam.yml](../deploy/antispam/docker-compose.antispam.yml): `inet:antispam:11332`. The panel reads it to report on the *Spam filter* box of the inbound pages and to check that the filter answers (Server › Health). | *(empty)* | `.env` | | `INBOUND_ANTISPAM_MILTER_ACTION` | What Postfix does if that milter is down: `accept` (fail-open) or `tempfail` (refuse with a temporary error). The *Spam filter* box states which. | `accept` | `.env` | | `INBOUND_SPAM_LOG_RETENTION_DAYS` | How long the inbound journal keeps a row, 7-365 days; it also never holds more than 10 000 rows. Seeds the setting on the first start with the inbound relay on. | `90` | `.env` | -| `INBOUND_JOURNAL_MILTER_SOCKET` | Unix socket of the panel's inbound journal milter, the last milter on port 25 while the inbound relay is on. It records accepted inbound mail and never refuses any; there is no reason to change it. | `/run/selfpost/inbound-journal.sock` | compose | | `INBOUND_RATE_LIMIT_MESSAGES_PER_IP` | Coarse per-client-IP cap on port 25: how many times one client IP may start a message (`MAIL FROM`) per window, accepted or not (`smtpd_client_message_rate_limit`). Uses the same window as `RATE_LIMIT_WINDOW_SECONDS`. | `20` | `.env` | | `INBOUND_MESSAGE_SIZE_LIMIT` | Maximum message size in bytes on inbound smtpd (`message_size_limit`). When both inbound relay and DMARC ingest are on, the shared listener uses the **greater** of this and `DMARC_MESSAGE_SIZE_LIMIT`. This is port 25's whole limit; 465 and 587 keep Postfix's own, 10 240 000 bytes. | `26214400` (25 MiB) | `.env` | | `RATE_LIMIT_MESSAGES_PER_IP` | Level-1 backstop: how many times one client IP may start a message (`MAIL FROM`) on one listener per window, accepted or not; 465 and 587 count separately (Postfix `smtpd_client_message_rate_limit`). See [Rate limiting — level 1](#rate-limiting--level-1-ip-backstop). | `100` | `.env` | @@ -1428,7 +1427,9 @@ with the *domain* role see them here because they cannot open the queue. Under both sit the `mail.log` lines for its queue id: the connection to the receiving server, the server's reply, and the status that reply was filed as. The lines are kept in the database with the row, so they stay on the -page after `mail.log` has rotated and are deleted together with the row; a +page after `mail.log` has rotated. They go at the next six-hourly prune after +the row is deleted by retention, and not while another row carries the same +queue id (Postfix reuses ids); a message accepted before `2.2.0` shows them only while they are still in the current file, and the page says so when there are none. Retention is set under **Server › Settings** (global role); `SEND_LOG_RETENTION_DAYS` in `.env` is only the initial default until it diff --git a/docs/roadmap.md b/docs/roadmap.md index e73798a..0153bc8 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -46,7 +46,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | 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, seven stages | in progress (stage 0 shipped `2.4.2`, stages 1+2 `2.5.0`, 2b `2.5.1`, 2c `2.5.2`, 3 `2.5.3`, 4 `2.5.4`, 5 `2.6.0`; next stage 6, docs only) | 77/81 | [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, seven stages | done 2026-10-15 (stage 0 shipped `2.4.2`, stages 1+2 `2.5.0`, 2b `2.5.1`, 2c `2.5.2`, 3 `2.5.3`, 4 `2.5.4`, 5 `2.6.0`; stage 6 docs and tests, no cut) | 81/81 | [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 @@ -57,9 +57,9 @@ shipped as `2.2.0` the same day, **panel-notifications** as `2.3.0` and [audit-2026-10-12](audit-2026-10-12.md) shipped as `2.4.2` (2026-10-14), its stages 1 and 2 as `2.5.0` the same day, stage 2b as `2.5.1`, stage 2c as `2.5.2` (2026-10-15), and stages 3, 4 and 5 as `2.5.3`, `2.5.4` and -`2.6.0` the same day. -Nothing is agreed after it: every remaining -item is a candidate. panel-docs shipped in +`2.6.0` the same day; stage 6 (documents and tests, no cut) closed the +audit. What is agreed after it is marked so in the index; every other +remaining item is a candidate. panel-docs shipped in [CHANGELOG.md](../CHANGELOG.md) `[1.8.0]`; dmarc-reports in `[1.7.0]` (its ingest path first ran in `2.0.0` and was reviewed on 2026-10-11, see [security.md](security.md)); domain-stats-auto-ratelimit in @@ -67,17 +67,16 @@ item is a candidate. panel-docs shipped in 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: [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 -git history. +The 2026-08-19 full-tree code review is closed except for one item, listed +above and not urgent: [origin-strict](#origin-strict) (its "add CSRF tokens", +decided otherwise on 2026-10-13). Of its other three, +template-data-typing is done (the last untyped map, the health sampler's, went +in [audit-2026-10-12](audit-2026-10-12.md) stage 4, `2.5.4`), +structured-logging is [dropped](#dropped-with-the-condition-for-return) +(2026-10-13) and [review-2026-08-followups](#review-2026-08-followups) is +closed (2026-10-14). 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 git history. After a context reset, pick an item marked `agreed` or `in progress`, then work the **Implementation checklist** in its linked plan. The `Progress` column above diff --git a/docs/schema-migrations.md b/docs/schema-migrations.md index b6d3d2e..9b35c1d 100644 --- a/docs/schema-migrations.md +++ b/docs/schema-migrations.md @@ -94,7 +94,7 @@ for operators upgrading from older 2.x images. --- -## Schema at head (v2) +## Schema at head | Table | Role | |-------|------| @@ -106,7 +106,7 @@ for operators upgrading from older 2.x images. | `applications` | SASL applications per domain, with the client-IP restriction (`auth_ip_restrict`, `auth_allowed_ips`) | | `application_addresses` | Explicit From addresses (`list` mode) | | `rate_limits` | Level-2 limits per domain / application, manual or auto | -| `send_log` | Delivery journal | +| `send_log` | Delivery journal. `reason` says why a row was refused (`limit`, `client`, `from`) or that the sweep closed it (`reconcile`, `reconcile-contradicted`), `header_from` what the From: line of a message refused for it said (v6), `refusal_line`, `client_ip` and `recovered` (v9), and `domain_id` (v8: the row belongs to a domain by id, `ON DELETE SET NULL`) | | `logtail_state` | Log-tailer read position | | `inbound_domains` | Inbound relay domains | | `inbound_transports` | Upstream host / port / TLS per inbound domain | @@ -114,9 +114,12 @@ for operators upgrading from older 2.x images. | `user_inbound_domains` | Inbound domains assigned to a `domain` user — granted separately from the outbound ones | | `dmarc_reports` | Parsed aggregate report summaries | | `dmarc_report_records` | Per-source rows inside a report | -| `inbound_spam_log` | Inbound antispam journal: one row per recipient of a message offered on port 25, with the decision (`accept`, `reject`, `tempfail`, `quarantine`) and who made it (`rspamd`, `postfix`, `selfpost`). Metadata only (v2) | +| `inbound_spam_log` | Inbound antispam journal: one row per recipient of a message offered on port 25, with the decision (`accept`, `reject`, `tempfail`, `quarantine`) and who made it (`rspamd`, `postfix`, `selfpost`). Metadata only. `inbound_domain_id` (v8) ties a row to its domain by id (v2) | | `inbound_spam_list` | Allow / deny entries for the external filter (`address`, `domain`, `ip`), instance-wide; unique per list (v2) | | `notify_state` | What the panel has already written to its users about, one row per subject (`dns:`, `cert`, `queue`, `limit:`), so that each thing is said once and a restart says nothing again (v4) | +| `sender_check_days` | What the sender check costs, a row per UTC day that has something to say: seconds the panel did not answer while Postfix ran, connections turned away (`deferred`), messages deferred mid-message (`messages`), and whether the day's downtime is an upper bound (`estimated`). Kept a year (v6, v9) | +| `delivery_log_days` | What the reconcile sweep and the log tailer did, a row per UTC day, kept a year. Read off a copy of the database; no page shows it (v7) | +| `notify_addressees` | Who a subject's notification is owed to and whether each was handed it; goes with the subject's `notify_state` row (v11) | | `send_log_lines` | The `mail.log` lines Postfix wrote under a queue id, with the line's own time; what a message's page shows once the log has rotated. No reference to `send_log`: a line belongs to a row by queue id and time, and goes when no row carries its id (v3) | `TestBaselineSchema` (`internal/store/schema_test.go`) pins every table, column @@ -150,7 +153,7 @@ for the domain. Nothing refers to a user: deleting one changes no domain. 1. Add `internal/store/migrations/00NN_short_name.sql` (next number). 2. Update the table in [Migration chain](#migration-chain), the snapshot at the - top, and the affected rows of [Schema at head](#schema-at-head-v2). + top, and the affected rows of [Schema at head](#schema-at-head). 3. Update `TestBaselineSchema` if tables, columns or indexes change, and add a test that opens a database at the previous version and migrates it. 4. A *data* migration states in a comment which 2.x versions it is written for. diff --git a/internal/app/service.go b/internal/app/service.go index f3b8bbe..1a18000 100644 --- a/internal/app/service.go +++ b/internal/app/service.go @@ -127,24 +127,6 @@ func (s *Service) Secret(login string) (string, error) { return s.sasl.Secret(login) } -// UpdateMode switches an application's address mode / list and rebuilds the -// sender map (product.md). The login and password are untouched. Addresses are -// re-validated against the application's domain. -func (s *Service) UpdateMode(id int64, mode string, rawAddresses []string) error { - a, err := s.store.GetApplication(id) - if err != nil { - return err - } - addresses, err := s.validateForDomain(a.DomainID, a.Login, mode, rawAddresses) - if err != nil { - return err - } - if err := s.store.UpdateApplicationMode(id, mode, addresses); err != nil { - return err - } - return s.Resync() -} - // RegeneratePassword issues a fresh password for an existing application (spec // 7.2.9). The old password is invalidated by overwriting the SASL account; the // address mode and bindings are unchanged, so no map rebuild is needed. The new diff --git a/internal/app/service_test.go b/internal/app/service_test.go index 2b5b544..fb69806 100644 --- a/internal/app/service_test.go +++ b/internal/app/service_test.go @@ -247,26 +247,6 @@ func TestServiceDeleteKeepsRowWhenSASLFails(t *testing.T) { } } -func TestServiceUpdateMode(t *testing.T) { - svc, st, _, maps := newServiceHarness(t) - d := addDomain(t, st, "example.com") - a, _, err := svc.Create(d.ID, "app1", store.AddressModeWildcard, nil) - if err != nil { - t.Fatal(err) - } - - if err := svc.UpdateMode(a.ID, store.AddressModeList, []string{"alerts@example.com"}); err != nil { - t.Fatalf("UpdateMode: %v", err) - } - if len(maps.last) != 1 || maps.last[0].Address != "alerts@example.com" { - t.Errorf("map after mode switch = %+v", maps.last) - } - got, _ := st.GetApplication(a.ID) - if got.AddressMode != store.AddressModeList || len(got.Addresses) != 1 { - t.Errorf("stored app after switch = %+v", got) - } -} - func TestServiceRegeneratePassword(t *testing.T) { svc, st, rec, _ := newServiceHarness(t) d := addDomain(t, st, "example.com") diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index 0431c04..2dd7da6 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -409,6 +409,28 @@ func TestCheckRestoreWrongFormatRejected(t *testing.T) { } } +// A manifest that is not JSON - cut short, or not one at all - stops the +// start and stays for the operator to look at. +func TestCheckRestoreRefusesAManifestThatIsNotJSON(t *testing.T) { + for _, body := range []string{`{"format": "selfpost-full", "version":`, "not json", ""} { + dir := t.TempDir() + path := filepath.Join(dir, ManifestName) + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + restored, err := CheckRestore(path, "1.0.0") + if err == nil || !strings.Contains(err.Error(), "not valid JSON") { + t.Errorf("manifest %q: err = %v, want not valid JSON", body, err) + } + if restored { + t.Errorf("manifest %q reported a restore", body) + } + if _, statErr := os.Stat(path); statErr != nil { + t.Errorf("manifest %q must be kept: %v", body, statErr) + } + } +} + // No archive holds the Postfix queue; the rest is there, the private key // included, and Check agrees that it can be made. func TestCreateLeavesTheQueueOut(t *testing.T) { diff --git a/internal/dmarc/refusals_test.go b/internal/dmarc/refusals_test.go index ee8b768..143db40 100644 --- a/internal/dmarc/refusals_test.go +++ b/internal/dmarc/refusals_test.go @@ -91,10 +91,7 @@ func TestServiceResyncAndHostedAddress(t *testing.T) { d, _ := st.AddDomain("example.org", "sel") maps := &recordingMaps{} s := NewService(st, maps, " Mail.Example.COM ", true) - if got := s.HostedSuggestion("example.org"); got != "dmarc-reports+example.org@mail.example.com" { - t.Errorf("suggestion = %q", got) - } - if err := st.SetDomainDMARCAddress(d.ID, s.HostedSuggestion("example.org")); err != nil { + if err := st.SetDomainDMARCAddress(d.ID, HostedReportAddress("mail.example.com", "example.org")); err != nil { t.Fatal(err) } if err := s.Resync(); err != nil { @@ -112,11 +109,4 @@ func TestServiceResyncAndHostedAddress(t *testing.T) { if last := maps.got[len(maps.got)-1]; len(last.Recipients) != 0 || len(last.Domains) != 0 { t.Errorf("switched off, the maps hold %+v", last) } - - if err := s.ValidateHostedAddress("dmarc-reports+example.org@mail.example.com"); err != nil { - t.Errorf("the hosted address refused: %v", err) - } - if err := s.ValidateHostedAddress("reports@elsewhere.example"); err == nil { - t.Error("an address on another host passed as hosted") - } } diff --git a/internal/dmarc/service.go b/internal/dmarc/service.go index 350de3a..97e38b9 100644 --- a/internal/dmarc/service.go +++ b/internal/dmarc/service.go @@ -3,7 +3,6 @@ package dmarc import ( - "fmt" "sort" "strings" @@ -94,16 +93,3 @@ func recipientDomains(addrs []string) []string { sort.Strings(out) return out } - -// HostedSuggestion returns the address the panel should suggest for a domain. -func (s *Service) HostedSuggestion(domain string) string { - return HostedReportAddress(s.hostname, domain) -} - -// ValidateHostedAddress ensures addr is on this hostname before saving. -func (s *Service) ValidateHostedAddress(addr string) error { - if !IsHostedOnHostname(addr, s.hostname) { - return fmt.Errorf("hosted report address must be on %s", s.hostname) - } - return nil -} diff --git a/internal/health/machine.go b/internal/health/machine.go index b7fdb47..3438bad 100644 --- a/internal/health/machine.go +++ b/internal/health/machine.go @@ -225,7 +225,7 @@ func (m *MachineSampler) Sample() Machine { // cpuUsage grades the processor over the window. Detail carries only the // core and thread counts — load average and busy prose stay out of the Status -// card (see roadmap panel-docs for operator-facing explanation later). +// card (the readings are described in docs/guide.md, "Overview and Health"). func cpuUsage(prev, cur cpuTimes, cores, threads int, load [3]float64, fresh bool, err error) CPU { c := CPU{Cores: cores, Threads: threads} if !isZeroLoad(load) { diff --git a/internal/logtail/logtail.go b/internal/logtail/logtail.go index 45ce17e..8e59d6d 100644 --- a/internal/logtail/logtail.go +++ b/internal/logtail/logtail.go @@ -612,11 +612,12 @@ func SplitTimestamp(line string) (stamp, rest string) { // follow tails path line by line, calling handle for each complete line, until // ctx is cancelled. Where it starts is tr's decision (a persisted offset, the // start of a file that changed while the panel was down, or end-of-file on a -// first ever run); it reopens the file when it is rotated (inode change from -// logrotate's create, or truncation from copytruncate) so nothing is missed. +// first ever run); it reopens the file when it is rotated, so nothing is missed. +// The inode changes under logrotate's create, which build/logrotate-mail.conf +// uses; a file shorter than the read position is taken as truncation. // logrotate 3.21.0, logrotate.c, rotateSingleLog(): without copytruncate the -// file is rename()d and, with create (build/logrotate-mail.conf), a new one -// made by createOutputFile(); with copytruncate it is copied and truncated. +// file is rename()d and, with create, a new one made by createOutputFile(); +// with copytruncate it is copied and truncated, which that config does not use. // // caughtUp is closed after the first read that reaches end-of-file, which is // the point where every line the panel missed while it was down has been diff --git a/internal/logtail/logtail_test.go b/internal/logtail/logtail_test.go index dc6b1d7..8db4228 100644 --- a/internal/logtail/logtail_test.go +++ b/internal/logtail/logtail_test.go @@ -258,6 +258,8 @@ type captureStore struct { missed int64 stuckCount int64 stuck []int64 + // blind holds the stretches AddTailerBlind was given. + blind []time.Duration // lines are the log lines kept for messages' pages; pruned is the cutoff of // each PruneSendLogLines call. @@ -357,7 +359,12 @@ func (c *captureStore) AddReconcileMissed(_ time.Time, n int64) error { c.missed += n return nil } -func (c *captureStore) AddTailerBlind(time.Time, time.Duration) error { return nil } +func (c *captureStore) AddTailerBlind(_ time.Time, d time.Duration) error { + c.mu.Lock() + defer c.mu.Unlock() + c.blind = append(c.blind, d) + return nil +} func (c *captureStore) AttachRefusalLine(string, string, string, time.Time, string) (bool, error) { return true, nil } @@ -456,6 +463,51 @@ func TestFollowTailsAndRotates(t *testing.T) { } } +// mail.log gone for longer than blindMin is a stretch the tailer could not +// read, counted for Health; the file made again is read from its start. Like +// TestFollowTailsAndRotates this runs on the server. +func TestFollowCountsALogItCannotReadAndReadsItAgain(t *testing.T) { + old := pollInterval + pollInterval = 10 * time.Millisecond + t.Cleanup(func() { pollInterval = old }) + + dir := t.TempDir() + path := filepath.Join(dir, "mail.log") + if err := os.WriteFile(path, nil, 0o644); err != nil { + t.Fatalf("seed log: %v", err) + } + cs := &captureStore{} + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- Run(ctx, path, cs, func() int { return 90 }, nil, nil) }() + time.Sleep(50 * time.Millisecond) + appendLine(t, path, "host postfix/smtp[1]: Q1: to=, relay=x, dsn=2.0.0, status=sent (ok)") + waitFor(t, func() bool { return contains(cs.snapshot(), "Q1|a@example.net|sent") }) + + if err := os.Remove(path); err != nil { + t.Fatalf("remove: %v", err) + } + // blindness counts by the clock: the name must stay missing past blindMin. + time.Sleep(blindMin + 500*time.Millisecond) + if err := os.WriteFile(path, nil, 0o644); err != nil { + t.Fatalf("recreate: %v", err) + } + appendLine(t, path, "host postfix/smtp[1]: Q2: to=, relay=x, dsn=5.1.1, status=bounced (nope)") + waitFor(t, func() bool { return contains(cs.snapshot(), "Q2|b@example.net|bounced") }) + waitFor(t, func() bool { + cs.mu.Lock() + defer cs.mu.Unlock() + return len(cs.blind) == 1 && cs.blind[0] >= blindMin + }) + + cancel() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("Run did not return after cancel") + } +} + // TestFollowResumesAfterRestart covers the persisted read offset: a restart // must parse the delivery lines written while the tailer was down (rows that // would otherwise stay "queued" forever), without re-parsing what it already diff --git a/internal/milter/milter.go b/internal/milter/milter.go index f5ffbd4..b317b72 100644 --- a/internal/milter/milter.go +++ b/internal/milter/milter.go @@ -1,14 +1,17 @@ -// Package milter implements the SelfPost journal-milter: a lightweight milter -// (architecture.md § Mail path) attached to Postfix's smtpd_milters alongside -// OpenDKIM. On the receive path it reads the SASL login, From, recipients and +// Package milter implements the SelfPost milters: the journal-milter here, the +// sender check (sender.go) and the inbound journal (inbound.go). +// +// The journal-milter is a lightweight milter (architecture.md § Mail path) +// attached to Postfix's smtpd_milters alongside OpenDKIM. On the receive path it reads the SASL login, From, recipients and // Subject of each accepted message and records one send-log row per (queue-id, // recipient), giving the panel a structured, filterable history that raw // mail.log cannot. // -// It is monitoring only: it never rejects, and every callback returns Continue -// or Accept so a failure of this milter can never block the relay. Postfix is -// configured with default_action=accept for this milter's socket, so even a -// crash or hang fails open (architecture.md § Mail path). +// It refuses only at MAIL FROM, with a 4xx tempfail, when a level-2 rate limit +// or the application's client-address list says no (see MailFrom); every other +// callback returns Continue. A failure of this milter never blocks the relay: +// Postfix is configured with default_action=accept for this milter's socket, +// so even a crash or hang fails open (architecture.md § Mail path). // // Postfix 3.7.11, src/milter/milter8.c, milter8_comm_error(): a milter that // cannot be reached, or does not answer within milter_*_timeout, gives no diff --git a/internal/milter/sender.go b/internal/milter/sender.go index 3ec93f2..b1dcdce 100644 --- a/internal/milter/sender.go +++ b/internal/milter/sender.go @@ -328,6 +328,11 @@ func (s *senderSession) Header(name, value string, m *milter.Modifier) (milter.R // src/cleanup/cleanup_milter.c, cleanup_milter_apply(): a "5" rejects the // message, and src/smtpd/smtpd.c, common_post_message_handling(), says that // reply to the client. +// +// h is not read: go-milter v0.4.1 adds every header of the connection to it +// and empties it only on ABORT (session.go, CodeHeader, CodeAbort), so on a +// second message it can still hold the first one's From:. The session keeps +// its own, emptied at each MAIL FROM (audit 2026-10-12, L9). func (s *senderSession) Headers(h textproto.MIMEHeader, m *milter.Modifier) (milter.Response, error) { shown, why, err := s.judge() if err != nil { diff --git a/internal/milter/sender_test.go b/internal/milter/sender_test.go index 96d1744..f268d3c 100644 --- a/internal/milter/sender_test.go +++ b/internal/milter/sender_test.go @@ -1,6 +1,7 @@ package milter import ( + "net/textproto" "os" "path/filepath" "strconv" @@ -301,15 +302,19 @@ func TestBindingsDuplicateKeyBreaksTheTable(t *testing.T) { } } -// A second message on the connection is judged by its own headers. +// A second message on the connection is judged by its own headers - not by +// the h go-milter passes, which still holds the earlier messages' From: (it +// is emptied only on ABORT; audit 2026-10-12, L9). func TestSenderCheckResetsBetweenMessages(t *testing.T) { bind := NewBindings(writeMaps(t, senderMaps)) s := &senderSession{bind: bind} auth := mods(map[string]string{"{auth_authen}": "shop"}) + h := textproto.MIMEHeader{} for i, v := range []string{"director@example.org", "a@shop.example.org", "director@example.org"} { _, _ = s.MailFrom("shop@shop.example.org", auth) + h.Add("From", v) _, _ = s.Header("From", v, mods(nil)) - resp, _ := s.Headers(nil, mods(nil)) + resp, _ := s.Headers(h, mods(nil)) if passed := resp == milter.RespContinue; passed != (i == 1) { t.Fatalf("message %d (From: %s): passed=%v", i, v, passed) } diff --git a/internal/store/inboundspam.go b/internal/store/inboundspam.go index 45722c2..fb8868e 100644 --- a/internal/store/inboundspam.go +++ b/internal/store/inboundspam.go @@ -356,8 +356,9 @@ type SpamListHit struct { // ListSpamListHits returns the journal rows of the filter in whose detail one // of the marks occurs - the marks the filter lists leave (internal/inbound). -// Which entry a row belongs to is for the caller to work out; here the rows -// are only narrowed, so that a page view does not read the whole journal. +// Which entry a row belongs to is for the caller to work out. No index covers +// detail or engine, so the query goes through the whole journal row by row and +// returns the rspamd rows whose detail holds a mark. func (s *Store) ListSpamListHits(marks ...string) ([]SpamListHit, error) { if len(marks) == 0 { return nil, nil diff --git a/internal/store/schema_test.go b/internal/store/schema_test.go index 08c8d2a..74e5500 100644 --- a/internal/store/schema_test.go +++ b/internal/store/schema_test.go @@ -12,8 +12,9 @@ import ( ) // 2.0 starts from one baseline: the 1.x chain is gone from the binary, not -// carried along (docs/schema-migrations.md). -func TestBaselineIsTheOnlyMigration(t *testing.T) { +// carried along (docs/schema-migrations.md): the chain starts at the +// baseline, and the baseline holds none of the 1.x schema. +func TestChainStartsAtTheBaseline(t *testing.T) { entries, err := fs.ReadDir(migrationsFS, "migrations") if err != nil { t.Fatalf("read migrations: %v", err) diff --git a/internal/store/sendlog.go b/internal/store/sendlog.go index 2fdb4d8..d71252e 100644 --- a/internal/store/sendlog.go +++ b/internal/store/sendlog.go @@ -16,10 +16,13 @@ const ( StatusSent = "sent" StatusDeferred = "deferred" StatusBounced = "bounced" - // StatusRejected marks a message the journal-milter refused with a 4xx under - // a level-2 rate limit (guide § Rate limiting). Such a row never gets a - // queue-id and is excluded from the level-2 message count (it was never - // sent). + // StatusRejected marks a message the relay refused, and the row of that + // refusal. The journal-milter refuses at MAIL FROM with a 4xx when a level-2 + // rate limit or the application's client-address list says no (Reason is + // ReasonLimit or ReasonClient; such a row has no queue-id). The sender check + // refuses a From: line that is not the application's with a 550 (ReasonFrom; + // its row has the queue-id). Every refused row is left out of the level-2 + // message count (it was never sent). StatusRejected = "rejected" ) @@ -72,11 +75,11 @@ func (s *Store) InsertQueued(e SendLogEntry) error { return nil } -// InsertRejected records a message the journal-milter refused under a level-2 -// rate limit (guide § Rate limiting), so the rejection is visible in the -// send-log UI. Only the fields known at MAIL FROM are set (domain, sender, app -// login); there is no queue-id or recipient because the message was rejected -// before it was queued. +// InsertRejected records a refused message (StatusRejected), so the refusal is +// visible in the send-log UI with its reason. The journal-milter writes its +// MAIL FROM refusals with only the fields known there (domain, sender, app +// login, client address): no queue-id, no recipient. The sender check writes +// its From: refusals with the queue-id and the first recipient. func (s *Store) InsertRejected(e SendLogEntry) error { now := time.Now().UTC().Format(time.RFC3339) _, err := s.db.Exec( diff --git a/internal/store/store.go b/internal/store/store.go index 3b195c7..75bb218 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -1,5 +1,5 @@ // Package store owns the SelfPost SQLite database: the single file under /data -// that persists panel users (global administrators and domain-admins), sending +// that persists panel users (the global and domain roles), sending // domains and applications, optional inbound-relay domains, the send log and // rate-limit settings (architecture.md § Persistence). It exposes typed queries // so the rest of the panel never builds SQL by hand. diff --git a/internal/web/guard_routes_test.go b/internal/web/guard_routes_test.go index b6fc46c..025305d 100644 --- a/internal/web/guard_routes_test.go +++ b/internal/web/guard_routes_test.go @@ -22,7 +22,8 @@ var routes = struct{ always, inbound, dmarc []string }{ always: []string{ "GET /{$}", "GET /overview", "GET /overview/fragment", - "GET /server/health", "POST /server/health/recheck", "POST /server/health/reload", + "GET /server/health", "GET /server/health/fragment", + "POST /server/health/recheck", "POST /server/health/reload", "GET /outbound/domains", "POST /outbound/domains", "GET /outbound/domains/{id}", "POST /outbound/domains/{id}/dns-recheck", "GET /outbound/domains/{id}/settings", diff --git a/internal/web/handlers/handlers_dmarc.go b/internal/web/handlers/handlers_dmarc.go index 60794bd..6b9ce64 100644 --- a/internal/web/handlers/handlers_dmarc.go +++ b/internal/web/handlers/handlers_dmarc.go @@ -17,11 +17,6 @@ import ( // "custom" (an address typed for the domain). What is stored is the address // alone; "" is no reports. func (h *Handlers) HandleDomainDMARC(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost { - w.Header().Set("Allow", "POST") - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) - return - } d, ok := h.lookupDomain(w, r) if !ok { return diff --git a/internal/web/handlers/handlers_settings.go b/internal/web/handlers/handlers_settings.go index 0f19eaa..8a39abf 100644 --- a/internal/web/handlers/handlers_settings.go +++ b/internal/web/handlers/handlers_settings.go @@ -25,15 +25,12 @@ func (h *Handlers) HandleServerSettings(w http.ResponseWriter, r *http.Request) if _, ok := h.requireGlobal(w, r); !ok { return } - switch r.Method { - case http.MethodGet: - h.renderServerSettings(w, r, http.StatusOK, "", h.currentRetentions()) - case http.MethodPost: + // The routes are method-bound (GET and POST), so it is one or the other. + if r.Method == http.MethodPost { h.submitServerSettings(w, r) - default: - w.Header().Set("Allow", "GET, POST") - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return } + h.renderServerSettings(w, r, http.StatusOK, "", h.currentRetentions()) } // retentions are the two journal retentions the Settings form shows: the diff --git a/internal/web/handlers/handlers_users.go b/internal/web/handlers/handlers_users.go index cac3ed6..8989ef9 100644 --- a/internal/web/handlers/handlers_users.go +++ b/internal/web/handlers/handlers_users.go @@ -67,15 +67,12 @@ func (h *Handlers) HandleUserNew(w http.ResponseWriter, r *http.Request) { if _, ok := h.requireGlobal(w, r); !ok { return } - switch r.Method { - case http.MethodGet: - h.renderUserForm(w, r, http.StatusOK, 0, userFormView{FormRole: string(store.RoleDomain)}) - case http.MethodPost: + // The routes are method-bound (GET and POST), so it is one or the other. + if r.Method == http.MethodPost { h.submitUserCreate(w, r) - default: - w.Header().Set("Allow", "GET, POST") - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return } + h.renderUserForm(w, r, http.StatusOK, 0, userFormView{FormRole: string(store.RoleDomain)}) } // HandleUserEdit edits or deletes a panel user (global only). @@ -97,24 +94,21 @@ func (h *Handlers) HandleUserEdit(w http.ResponseWriter, r *http.Request) { http.Error(w, "internal error", http.StatusInternalServerError) return } - switch r.Method { - case http.MethodGet: - h.renderUserForm(w, r, http.StatusOK, u.ID, userFormView{ - FormUsername: u.Username, - FormEmail: u.Email, - FormRole: string(u.Role), - Reach: u.Reach(), - }) - case http.MethodPost: + // The routes are method-bound (GET and POST), so it is one or the other. + if r.Method == http.MethodPost { if err := r.ParseForm(); err != nil { h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: "Invalid form submission.", FormUsername: u.Username, FormEmail: u.Email, FormRole: string(u.Role)}) return } h.submitUserUpdate(w, r, u) - default: - w.Header().Set("Allow", "GET, POST") - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return } + h.renderUserForm(w, r, http.StatusOK, u.ID, userFormView{ + FormUsername: u.Username, + FormEmail: u.Email, + FormRole: string(u.Role), + Reach: u.Reach(), + }) } func (h *Handlers) renderUserForm(w http.ResponseWriter, r *http.Request, status int, userID int64, form userFormView) { diff --git a/internal/web/home_test.go b/internal/web/home_test.go index 2427152..83c7525 100644 --- a/internal/web/home_test.go +++ b/internal/web/home_test.go @@ -3,6 +3,9 @@ package web import ( "net/http" "net/http/httptest" + "os" + "regexp" + "slices" "strings" "testing" @@ -61,3 +64,26 @@ func TestServerSubtreeIsGlobalOnly(t *testing.T) { t.Fatalf("only %d /server/ routes were checked", checked) } } + +// The test above walks the routes table; a /server/ route registered beside +// it, or past the server() guard, would not be looked at. Every pattern under +// /server/ that web.go registers goes through server() and is in the table. +func TestEveryServerRouteIsGuardedAndListed(t *testing.T) { + src, err := os.ReadFile("web.go") + if err != nil { + t.Fatal(err) + } + found := regexp.MustCompile(`([\w.]+)\("((?:GET|POST|PUT|DELETE|PATCH|HEAD) )?(/server/[^"]*)"`).FindAllStringSubmatch(string(src), -1) + if len(found) < 15 { + t.Fatalf("only %d /server/ patterns found in web.go", len(found)) + } + for _, m := range found { + pattern := m[2] + m[3] + if m[1] != "server" { + t.Errorf("%q is registered by %s, not server(): the subtree's guard is skipped", pattern, m[1]) + } + if !slices.Contains(routes.always, pattern) { + t.Errorf("%q is not in routes.always, so no test opens it as a domain administrator", pattern) + } + } +} diff --git a/internal/web/security_test.go b/internal/web/security_test.go index 43e90c6..53d9c68 100644 --- a/internal/web/security_test.go +++ b/internal/web/security_test.go @@ -64,6 +64,30 @@ func TestOriginCheck(t *testing.T) { } } +// A panel on a port of its own: the browser's Origin carries the port, as the +// Host the proxy passes on does, and the two compare whole (Fetch Standard +// § 3.2: the serialised origin has the port when it is not the scheme's +// default). The same host on another port is another origin. +func TestOriginCheckComparesThePort(t *testing.T) { + tests := []struct { + host, origin string + wantRejected bool + }{ + {"panel.example.com:8443", "https://panel.example.com:8443", false}, + {"panel.example.com:8443", "https://panel.example.com", true}, + {"panel.example.com", "https://panel.example.com:8443", true}, + {"panel.example.com:8443", "https://panel.example.com:9443", true}, + } + for _, tc := range tests { + r := post("", tc.origin) + r.Host = tc.host + rejected := serveSecured(true, r).Code == http.StatusForbidden + if rejected != tc.wantRejected { + t.Errorf("Host %s, Origin %s: rejected = %t, want %t", tc.host, tc.origin, rejected, tc.wantRejected) + } + } +} + // The proxy terminates TLS and forwards plain HTTP, so the panel never learns // its own external scheme: comparing it against the https in Origin would // reject every legitimate form submission in the documented deployment. diff --git a/internal/web/view/account.go b/internal/web/view/account.go index 9dfcb88..c5a867d 100644 --- a/internal/web/view/account.go +++ b/internal/web/view/account.go @@ -135,12 +135,7 @@ func (s *Settings) WithInbound(days string) *Settings { // WithResult shows the result of the last action, or its refusal, between the // head and the form. func (s *Settings) WithResult(flash, formErr string) *Settings { - switch { - case formErr != "": - s.Flash = &Flash{Error: true, Text: Plain(formErr)} - case flash != "": - s.Flash = &Flash{Text: Plain(flash)} - } + s.Flash = resultFlash(flash, formErr) return s } diff --git a/internal/web/view/components.go b/internal/web/view/components.go index ffc1757..69803ba 100644 --- a/internal/web/view/components.go +++ b/internal/web/view/components.go @@ -449,16 +449,6 @@ type Fact struct { Note Text } -// Facts is a shorthand for plain label / value pairs: Facts("Domain", -// "example.org", "Port", "587"). -func Facts(pairs ...string) []Fact { - var out []Fact - for i := 0; i+1 < len(pairs); i += 2 { - out = append(out, Fact{Label: pairs[i], Value: Plain(pairs[i+1])}) - } - return out -} - // SideMenu is the input of side_menu, the left column of a detail page: groups // of entries with icons and count tags. type SideMenu struct { diff --git a/internal/web/view/components_test.go b/internal/web/view/components_test.go index 3e6cf06..4156206 100644 --- a/internal/web/view/components_test.go +++ b/internal/web/view/components_test.go @@ -311,9 +311,6 @@ func TestFactsBigMonoAndNote(t *testing.T) { `
`, `
4XcB7k2Jm9z1
`, `
5 days
a supporting line
`) - if got := Facts("A", "1", "B", "2", "odd"); len(got) != 2 || got[1].Label != "B" { - t.Errorf("Facts(pairs) = %#v", got) - } } func TestSideMenu(t *testing.T) { diff --git a/test/e2e/cert_reload_check.go b/test/e2e/cert_reload_check.go index 069061f..b5807ee 100644 --- a/test/e2e/cert_reload_check.go +++ b/test/e2e/cert_reload_check.go @@ -87,10 +87,10 @@ func checkCertRenewalPickedUp(s *stack) error { "postconf -h smtpd_tls_cert_file && cmp /etc/postfix/tls/fullchain.pem /etc/postfix/tls-internal/fullchain.pem && "+ "cmp /etc/postfix/tls/privkey.pem /etc/postfix/tls-internal/privkey.pem && stat -c '%a %U' /etc/postfix/tls-internal/privkey.pem") if err != nil { - return fmt.Errorf("Postfix's copy of the certificate: %v\n%s", err, out) + return fmt.Errorf("the certificate Postfix reads: %v\n%s", err, out) } if !strings.Contains(out, "/etc/postfix/tls-internal/fullchain.pem\n") || !strings.Contains(out, "600 root") { - return fmt.Errorf("Postfix's copy of the certificate: %q, want Postfix reading it and the key 600 root", out) + return fmt.Errorf("the certificate Postfix reads: %q, want Postfix reading it and the key 600 root", out) } return nil } diff --git a/test/e2e/logrotate_check.go b/test/e2e/logrotate_check.go index 115354f..80371f4 100644 --- a/test/e2e/logrotate_check.go +++ b/test/e2e/logrotate_check.go @@ -2,11 +2,7 @@ package e2e import ( "fmt" - "os" - "os/exec" - "path/filepath" "strings" - "testing" "time" ) @@ -61,46 +57,3 @@ func checkLogrotateRotation(s *stack) error { } return nil } - -// TestImageBuildPreservesLogrotateMode builds from a context where -// logrotate-mail.conf is group-writable and checks the image still ships 0644. -func TestImageBuildPreservesLogrotateMode(t *testing.T) { - conf := filepath.Join(h.repoRoot, "build", "logrotate-mail.conf") - info, err := os.Stat(conf) - if err != nil { - t.Fatalf("stat source config: %v", err) - } - origMode := info.Mode().Perm() - - if err := os.Chmod(conf, origMode|0o020); err != nil { - t.Fatalf("chmod g+w source config: %v", err) - } - t.Cleanup(func() { - _ = os.Chmod(conf, origMode) - }) - - tag := "selfpost:e2e-logrotate-mode" - build := exec.Command("docker", "build", - "-f", filepath.Join(h.repoRoot, "build", "Dockerfile"), - "-t", tag, - "--build-arg", "VERSION=e2e", - h.repoRoot, - ) - build.Env = os.Environ() - if out, err := build.CombinedOutput(); err != nil { - t.Fatalf("docker build with group-writable context config: %v\n%s", err, out) - } - t.Cleanup(func() { - _, _ = exec.Command("docker", "rmi", "-f", tag).CombinedOutput() - }) - - run := exec.Command("docker", "run", "--rm", tag, "stat", "-c", "%a", "/etc/logrotate.d/mail") - run.Env = os.Environ() - out, err := run.CombinedOutput() - if err != nil { - t.Fatalf("stat in image: %v\n%s", err, out) - } - if strings.TrimSpace(string(out)) != "644" { - t.Fatalf("image logrotate config mode is %q, want 644", strings.TrimSpace(string(out))) - } -} diff --git a/test/e2e/logrotate_mode_test.go b/test/e2e/logrotate_mode_test.go new file mode 100644 index 0000000..fe6e35c --- /dev/null +++ b/test/e2e/logrotate_mode_test.go @@ -0,0 +1,55 @@ +package e2e + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// TestImageBuildPreservesLogrotateMode builds from a context where +// logrotate-mail.conf is group-writable and checks the image still ships 0644. +func TestImageBuildPreservesLogrotateMode(t *testing.T) { + conf := filepath.Join(h.repoRoot, "build", "logrotate-mail.conf") + info, err := os.Stat(conf) + if err != nil { + t.Fatalf("stat source config: %v", err) + } + origMode := info.Mode().Perm() + + if err := os.Chmod(conf, origMode|0o020); err != nil { + t.Fatalf("chmod g+w source config: %v", err) + } + t.Cleanup(func() { + _ = os.Chmod(conf, origMode) + }) + + tag := "selfpost:e2e-logrotate-mode" + build := exec.Command("docker", "build", + "-f", filepath.Join(h.repoRoot, "build", "Dockerfile"), + "-t", tag, + "--build-arg", "VERSION=e2e", + h.repoRoot, + ) + build.Env = os.Environ() + if out, err := build.CombinedOutput(); err != nil { + t.Fatalf("docker build with group-writable context config: %v\n%s", err, out) + } + t.Cleanup(func() { + _, _ = exec.Command("docker", "rmi", "-f", tag).CombinedOutput() + }) + + // Past the entrypoint, which would want a hostname and the rest: only the + // file's mode is asked. (Until stage 6 of the 2026-10-12 audit this test + // sat in a non-test file and go test never ran it.) + run := exec.Command("docker", "run", "--rm", "--entrypoint", "stat", tag, "-c", "%a", "/etc/logrotate.d/mail") + run.Env = os.Environ() + out, err := run.CombinedOutput() + if err != nil { + t.Fatalf("stat in image: %v\n%s", err, out) + } + if strings.TrimSpace(string(out)) != "644" { + t.Fatalf("image logrotate config mode is %q, want 644", strings.TrimSpace(string(out))) + } +} diff --git a/test/e2e/mail_helpers.go b/test/e2e/mail_helpers.go index 2898c6c..870b9e3 100644 --- a/test/e2e/mail_helpers.go +++ b/test/e2e/mail_helpers.go @@ -11,6 +11,15 @@ import ( "github.com/emersion/go-msgauth/dkim" ) +// senderDomain and recipient are fixed for the whole run: one sending domain +// registered in the panel, and the sink-MX as the single recipient every +// positive send targets. Here, not in main_test.go: smtp_client.go uses them, +// and the package builds without its tests (staticcheck reads it so). +const ( + senderDomain = "sender.e2e.test" + recipient = "rcpt@sink.e2e.test" +) + // findSinkMessage polls the sink-MX's dump directory (see test/e2e/sink) for // a file whose contents contain token — the unique Subject-line marker every // test send carries — and returns it. Polling a directory listing rather than diff --git a/test/e2e/main_test.go b/test/e2e/main_test.go index 19b5979..11de5d1 100644 --- a/test/e2e/main_test.go +++ b/test/e2e/main_test.go @@ -62,14 +62,6 @@ func TestMain(m *testing.M) { os.Exit(code) } -// senderDomain and recipient are fixed for the whole run: one sending domain -// registered in the panel, and the sink-MX as the single recipient every -// positive send targets. -const ( - senderDomain = "sender.e2e.test" - recipient = "rcpt@sink.e2e.test" -) - // The one administrator of the run. const ( adminUser = "e2eadmin" diff --git a/test/e2e/sasldb_test.go b/test/e2e/sasldb_test.go index 85966f0..5cef798 100644 --- a/test/e2e/sasldb_test.go +++ b/test/e2e/sasldb_test.go @@ -23,7 +23,7 @@ import ( // mode and group stay) and that nothing fails under the load; it cannot prove // a race absent. func testSASLDBChangedByRename(t *testing.T, sc *scenario) { - churnLogin, churnPassword, err := sc.panel.addApplication(sc.domainID, "churn", "wildcard", "") + churnLogin, _, err := sc.panel.addApplication(sc.domainID, "churn", "wildcard", "") if err != nil { t.Fatalf("add application: %v", err) } @@ -55,6 +55,7 @@ func testSASLDBChangedByRename(t *testing.T, sc *scenario) { // while the old file still exists, so it cannot have the old file's // number. (After many changes it may: a freed inode number is reused.) before := strings.Fields(stat()) + var churnPassword string if churnPassword, err = newPassword(0); err != nil { t.Fatal(err) } diff --git a/test/e2e/smtp_client.go b/test/e2e/smtp_client.go index b5ca4c6..daed05f 100644 --- a/test/e2e/smtp_client.go +++ b/test/e2e/smtp_client.go @@ -252,7 +252,7 @@ func waitForSenderMap(login, password, from string) error { return nil } if replyIs(err, 553, "not owned by user "+login) != nil || time.Now().After(deadline) { - return fmt.Errorf("Postfix does not yet let %s send as %s: %w", login, from, err) + return fmt.Errorf("not yet: Postfix does not let %s send as %s: %w", login, from, err) } time.Sleep(300 * time.Millisecond) }