From ca876c16311fe438bc73fcfa0327819d3362dcf3 Mon Sep 17 00:00:00 2001 From: Mix Date: Thu, 15 Oct 2026 11:20:18 +0300 Subject: [PATCH] audit 4: log paging at its edges, NaN refused, a red rate limit, recalculation asks, reload writes all Step 4 of stage 4 (L10, L11, L13, L18, L19 and a row of review-2026-08-followups): - The Outbound log counts first and clamps the page to the last; a number too large for an int (strconv's ErrRange with the largest int) is the last page on both logs. Tested at the edges: first, last, past the last however far, nonsense, an empty log asked for page 5. - checkAutoMultiplier is "not within", so NaN and the infinities are out. - rate-limited is red (components.go; the mockup first, 643892a). - data-unsaved on both recalculate buttons: panel.js asks on submit (ev.submitter) when a field of the page differs from what it was drawn with; testdata/unsaved_test.js under TestPanelJS. - HandleReload writes what a restore's first start writes - the filter lists and the DMARC maps too - and a failed step is an error flash on Health that names it, not plain text; the success flash says what was written. Tested on the test server: go vet + go test of 25 packages, make e2e (all pass, 695 s) on this tree. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 20 ++++ docs/architecture.md | 4 +- docs/audit-2026-10-12.md | 16 ++- docs/guide.md | 18 +-- docs/roadmap.md | 2 +- internal/web/handlers/handlers_dmarc_test.go | 36 ++++++ internal/web/handlers/handlers_domains.go | 41 ++++--- .../web/handlers/handlers_inbound_log_test.go | 5 + internal/web/handlers/handlers_monitor.go | 23 ++-- .../handlers/handlers_outbound_log_test.go | 17 +++ internal/web/handlers/handlers_ratelimit.go | 5 +- internal/web/handlers/handlers_status.go | 2 +- internal/web/handlers/ratelimit_parse_test.go | 23 ++++ internal/web/view/components.go | 3 + internal/web/view/components_test.go | 2 +- internal/web/view/kit.go | 2 +- internal/web/view/outbound_log_test.go | 2 +- internal/web/view/paneljs_test.go | 22 ++-- internal/web/view/static/panel.js | 49 ++++++++ internal/web/view/templates/out_app.html | 2 +- .../view/templates/out_domain_settings.html | 2 +- internal/web/view/testdata/poll_test.js | 2 +- internal/web/view/testdata/unsaved_test.js | 105 ++++++++++++++++++ 23 files changed, 355 insertions(+), 48 deletions(-) create mode 100644 internal/web/handlers/ratelimit_parse_test.go create mode 100644 internal/web/view/testdata/unsaved_test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 53e5fd9..bfde786 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Changed + +- **Reload configuration writes everything a restore's first start writes.** + The button on Server › Health left the DMARC report maps and the filter + lists out while it said the configuration was regenerated; it now writes + them too, and a step that fails is a message on the page that names it, + not a plain-text error (audit 2026-10-12, L13). + ### Security - **The sign-in told which usernames exist by how long it took.** A wrong @@ -31,6 +39,18 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version failure in a row, up to a minute. A box whose session had expired was filled with the sign-in page; the whole page goes to sign-in now (audit 2026-10-12, M11). +- **The Outbound log showed an empty table under "Page 9 of 2"** for a page + past its end - a link kept while old rows were pruned, or a number typed. + It shows the last page, as the Inbound log does (audit 2026-10-12, L10). +- **An auto limit's multiplier took "NaN"**, stored nothing, and was + recalculated with the default without a word. It is refused like any + number out of range (audit 2026-10-12, L11). +- **A message refused under a rate limit was a grey tag** beside its two + red siblings, *wrong From* and *wrong client*. It is red (audit + 2026-10-12, L18). +- **Recalculate rate limit dropped what was typed on the page.** The button + leaves the page; when a field holds an edit not saved yet, it now asks + first (audit 2026-10-12, L19). ## [2.5.3] - 2026-10-15 diff --git a/docs/architecture.md b/docs/architecture.md index cc9c7b2..dc04014 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1038,8 +1038,8 @@ tables, Postfix's sender map, inbound relay maps (when 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) resyncs OpenDKIM, the sender map, and -inbound maps only — not DMARC maps. Stopped-container +Health) runs the same Resync, the filter lists with it, since `2.5.4`. +Stopped-container `tar` of `./data` alone remains possible for state-only copies (see guide). **Optional encryption** of the two secret-bearing downloads diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index e0bc59c..7e5c6e9 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -1217,14 +1217,26 @@ command whose leftover is the owner's alone. `handleAjaxResponse()` follows before it looks at the status. No jsdom (no node tooling in the repo): `testdata/poll_test.js` runs panel.js in node against a stand-in for the DOM and htmx with a moved clock, from - `TestPanelJSPolling` (skipped where node is absent - the test server's + `TestPanelJS` (skipped where node is absent - the test server's Go has none; it fails on the old panel.js with ten findings). -- [ ] `parsePage` clamp; `NaN` refused; `rate-limited` tag red (add the status to +- [x] `parsePage` clamp; `NaN` refused; `rate-limited` tag red (add the status to `components.go`, flip the test); the recalc button posts only what it needs or warns; `HandleReload` either covers DMARC maps and filter lists or the flash and the two comments say what it covers. The log's paging is tested at its edges: first page, last, past the last, an empty log (a row of review-2026-08-followups, placed here 2026-10-14). — **Sonnet** + *Done 2026-10-15:* the Outbound log counts first and clamps to the last + page; a number too large for an int is the last page too (strconv's + ErrRange with the largest int), on both logs. `checkAutoMultiplier` is + written "not within", so NaN and the infinities are out. The mockup drew + rate-limited grey: `design:` commit first (out-log, and the kit's status + row, which had also lost `reject`), then `components.go`. The recalc + button warns: `data-unsaved` on both recalc buttons, and panel.js asks + on submit (`ev.submitter`) when a field of the page differs from what it + was drawn with (`testdata/unsaved_test.js` under `TestPanelJS`). + `HandleReload` covers what the restore's Resync covers, the filter lists + and the DMARC maps included, and a failed step is an error flash on + Health naming it. - [ ] `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 diff --git a/docs/guide.md b/docs/guide.md index 9430edb..1693602 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -473,11 +473,15 @@ readings, in the order of what stops mail first: record the internet publishes for this server's IP (forward-confirmed reverse DNS), see [Server-level DNS](#server-level-dns-ptrrdns); use *Re-check DNS* after changing DNS. -- **Configuration** — the **Reload configuration** button re-applies OpenDKIM - tables and the Postfix sender map from the database (and inbound relay maps - when `INBOUND_RELAY_ENABLE=true`) — use it if daemons drifted from what the - panel shows after manual edits under `/data`. It does **not** rebuild DMARC - Postfix maps; a full restore Resync does (see [Restore](#restore)). +- **Configuration** — the **Reload configuration** button writes again, from + the database, every file the panel keeps for the mail path - OpenDKIM's + tables, the Postfix sender map, the inbound relay maps and the filter lists + (with `INBOUND_RELAY_ENABLE=true`), the DMARC report maps (with + `DMARC_REPORTS_ENABLE=true`) - and has OpenDKIM and Postfix re-read them: + what the first start after a restore does (see [Restore](#restore)). Use it + if the daemons drifted from what the panel shows after manual edits under + `/data`. A step that fails is named on the page, and the panel's log says + why. ### Queue and System log @@ -817,8 +821,8 @@ when `INBOUND_RELAY_ENABLE=true`; DMARC maps when `DMARC_REPORTS_ENABLE=true`) and both daemons are reloaded, healing any drift between the extracted files and the database. The manifest is deleted only after that Resync is done: a first start that fails on the way leaves it, and the next start is a restore again -and runs the Resync. The *Reload configuration* button on Server › Health resyncs -OpenDKIM, the sender map, and inbound maps only — not DMARC maps. This is why the +and runs the Resync. The *Reload configuration* button on Server › Health runs the same Resync. +This is why the compose file pins a fixed tag rather than `:latest`: without a known version, there'd be no way to tell which image restoring a given backup actually requires (see [Fixed image tag](#fixed-image-tag)). diff --git a/docs/roadmap.md b/docs/roadmap.md index 19986be..033c124 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -42,7 +42,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`; next stage 4 → `2.5.4`) | 58/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 | 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`; next stage 4 → `2.5.4`) | 59/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 diff --git a/internal/web/handlers/handlers_dmarc_test.go b/internal/web/handlers/handlers_dmarc_test.go index 094a987..6ee0df7 100644 --- a/internal/web/handlers/handlers_dmarc_test.go +++ b/internal/web/handlers/handlers_dmarc_test.go @@ -1,6 +1,7 @@ package handlers import ( + "errors" "net/http" "net/http/httptest" "net/url" @@ -10,6 +11,7 @@ import ( "github.com/mixeme/selfpost/internal/dmarc" "github.com/mixeme/selfpost/internal/domain" + "github.com/mixeme/selfpost/internal/health" "github.com/mixeme/selfpost/internal/postfix" "github.com/mixeme/selfpost/internal/store" "github.com/mixeme/selfpost/internal/web/auth" @@ -293,3 +295,37 @@ func TestDeletingAUserLeavesDomainAddressesAndNamesNobody(t *testing.T) { confirm := send(s.h.HandleUserDeleteConfirm, &globalPrincipal, "GET", "/server/users/"+idStr(admin.ID)+"/delete", map[string]string{"uid": idStr(admin.ID)}, nil).Body.String() lacks(t, "delete confirmation", confirm, `report address`, `will have no`) } + +// failingDMARCMaps stands in for Postfix refusing the DMARC maps. +type failingDMARCMaps struct{} + +func (failingDMARCMaps) RebuildDMARCMaps(postfix.DMARCMapsConfig) error { + return errors.New("disk full") +} + +// Reload configuration writes what a restore's first start writes, the DMARC +// maps among it, and a failure is a message on Health that names the step, +// not a plain-text page (audit 2026-10-12, L13). +func TestReloadWritesTheDMARCMapsAndSaysWhereItFailed(t *testing.T) { + s, maps := hostedStand(t) + // An OpenDKIM that reloads nothing - the stand has no supervisor - and + // a sampler for the Health page a failure is shown on. + odk := domain.NewOpenDKIM(t.TempDir()) + odk.SetReloadHook(func() error { return nil }) + s.h.domains = domain.NewService(s.h.store, odk, s.h.apps, "mail") + s.h.machine = &health.MachineSampler{} + rec := postFormAs(s.h.HandleReload, globalPrincipal, "/server/health/reload", nil, url.Values{}) + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/server/health?reloaded=1" { + t.Fatalf("reload = %d, Location %q", rec.Code, rec.Header().Get("Location")) + } + if len(maps.got) != 1 { + t.Errorf("the reload rebuilt the DMARC maps %d times, want once", len(maps.got)) + } + + s.h.dmarc = dmarc.NewService(s.h.store, failingDMARCMaps{}, "mail.example.org", true) + rec = postFormAs(s.h.HandleReload, globalPrincipal, "/server/health/reload", nil, url.Values{}) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("a failed reload = %d, want 500", rec.Code) + } + has(t, "failed reload", rec.Body.String(), "Reload failed at the DMARC report maps", "Reload configuration") +} diff --git a/internal/web/handlers/handlers_domains.go b/internal/web/handlers/handlers_domains.go index c422754..f88b14a 100644 --- a/internal/web/handlers/handlers_domains.go +++ b/internal/web/handlers/handlers_domains.go @@ -162,26 +162,41 @@ func (h *Handlers) HandleDeleteDomain(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/outbound/domains?deleted=1", http.StatusSeeOther) } -// HandleReload re-applies both the OpenDKIM configuration and the Postfix -// sender map on demand (architecture.md § Panel HTTP surface). +// HandleReload writes again, from the database, every file the panel keeps +// for the mail path and has OpenDKIM and Postfix re-read them - what the +// first start after a restore does (cmd/panel mailStack.Resync): OpenDKIM's +// tables, the sender map, with the inbound relay on its maps and the filter +// lists, with DMARC reports on their maps (architecture.md § Panel HTTP +// surface). Before 2.5.4 it left the last two out, and said "regenerated" +// over them (audit 2026-10-12, L13). A failure is a message on Health that +// names the step; the log has the error. func (h *Handlers) HandleReload(w http.ResponseWriter, r *http.Request) { if _, ok := h.requireGlobal(w, r); !ok { return } - if err := h.domains.Resync(); err != nil { - logf("panel: manual reload (opendkim): %v", err) - http.Error(w, "reload failed", http.StatusInternalServerError) - return + type step struct { + name string + run func() error } - if err := h.apps.Resync(); err != nil { - logf("panel: manual reload (postfix): %v", err) - http.Error(w, "reload failed", http.StatusInternalServerError) - return + steps := []step{ + {"OpenDKIM's tables", h.domains.Resync}, + {"the sender map", h.apps.Resync}, } if h.cfg.InboundEnabled && h.inbound != nil { - if err := h.inbound.Resync(); err != nil { - logf("panel: manual reload (inbound): %v", err) - http.Error(w, "reload failed", http.StatusInternalServerError) + steps = append(steps, step{"the inbound relay maps", h.inbound.Resync}) + } + if h.cfg.InboundEnabled && h.lists != nil { + steps = append(steps, step{"the filter lists", h.lists.Sync}) + } + if h.dmarc != nil && h.dmarc.Enabled() { + steps = append(steps, step{"the DMARC report maps", h.dmarc.Resync}) + } + for _, s := range steps { + if err := s.run(); err != nil { + logf("panel: manual reload (%s): %v", s.name, err) + page := h.healthView(r, "") + page.Flash = &view.Flash{Error: true, Text: view.Plain("Reload failed at " + s.name + "; what came before it was written and re-read. The panel's log says why.")} + h.view.Render(w, http.StatusInternalServerError, "health", page) return } } diff --git a/internal/web/handlers/handlers_inbound_log_test.go b/internal/web/handlers/handlers_inbound_log_test.go index 7336a23..b75c5d1 100644 --- a/internal/web/handlers/handlers_inbound_log_test.go +++ b/internal/web/handlers/handlers_inbound_log_test.go @@ -229,6 +229,8 @@ func TestInboundLogPages(t *testing.T) { // A page past the end, or nonsense, is the last page, or the first. has(t, "page 9", s.get(s.h.HandleInboundLog, "/inbound/log?domain=a.example.com&p=9", globalPrincipal).Body.String(), `Page 2 of 2`, `Hello A`) has(t, "page x", s.get(s.h.HandleInboundLog, "/inbound/log?domain=a.example.com&p=x", globalPrincipal).Body.String(), `Page 1 of 2`) + has(t, "page huge", s.get(s.h.HandleInboundLog, "/inbound/log?domain=a.example.com&p=99999999999999999999", globalPrincipal).Body.String(), `Page 2 of 2`, `Hello A`) + has(t, "page 0", s.get(s.h.HandleInboundLog, "/inbound/log?domain=a.example.com&p=0", globalPrincipal).Body.String(), `Page 1 of 2`) } func TestInboundLogEmptyAndOff(t *testing.T) { @@ -237,6 +239,9 @@ func TestInboundLogEmptyAndOff(t *testing.T) { body := s.get(h.HandleInboundLog, "/inbound/log", globalPrincipal).Body.String() has(t, "empty log", body, `No messages logged yet.`) lacks(t, "empty log", body, ` 0 { lastPage = int((total + sendLogPageSize - 1) / sendLogPageSize) } + // A page past the end is the last one, as on the Inbound log: a link + // kept while the log was pruned, or a number typed, shows rows, not an + // empty table under "Page 9 of 2" (audit 2026-10-12, L10). + page := min(parsePage(q.Get("p")), lastPage) + rows, err := h.store.QuerySendLog(filter, sendLogPageSize, (page-1)*sendLogPageSize) + if err != nil { + return nil, err + } out := view.NewOutLog(h.shellMeta(r), h.sendLogRetentionDays(), p.IsGlobal(), domainNames, logins, filter.Domain, filter.AppLogin) table := make([]view.OutLogRow, len(rows)) for i, row := range rows { @@ -409,8 +410,16 @@ func (h *Handlers) outLogPage(r *http.Request) (*view.OutLog, error) { // parsePage clamps the "p" query parameter to a valid page number, defaulting // to 1 for anything missing or malformed rather than rejecting the request. +// +// A number too large for an int is past every last page, and is taken as +// that: strconv returns it as the largest int with ErrRange (Go 1.26, +// strconv.ParseInt: "the maximum magnitude integer of the appropriate +// bitSize and sign"), and the caller clamps it to the last page. func parsePage(v string) int { n, err := strconv.Atoi(v) + if errors.Is(err, strconv.ErrRange) && n > 0 { + return n + } if err != nil || n < 1 { return 1 } diff --git a/internal/web/handlers/handlers_outbound_log_test.go b/internal/web/handlers/handlers_outbound_log_test.go index 05c81d8..7055b09 100644 --- a/internal/web/handlers/handlers_outbound_log_test.go +++ b/internal/web/handlers/handlers_outbound_log_test.go @@ -74,6 +74,19 @@ func TestOutLogPagesKeepTheFiltersAndThePage(t *testing.T) { lacks(t, "page 2", second, `Older →`) // A row's page is opened from the page it is on and leads back to it. has(t, "page 2", second, `?domain=bs.example.ru&p=2">Details`) + + // The edges (audit 2026-10-12, L10): a page past the end is the last, + // however far past; nonsense, zero or a negative number is the first. + for _, p := range []string{"3", "9", "99999999999999999999", "9223372036854775807"} { + body := getBody(t, h.HandleDeliveries, "/outbound/log?domain=bs.example.ru&p="+p) + has(t, "page "+p, body, `Page 2 of 2`, `← Newer`) + if got := strings.Count(body, `">Details`); got == 0 { + t.Errorf("page %s shows no rows", p) + } + } + for _, p := range []string{"0", "-1", "x", ""} { + has(t, "page "+p, getBody(t, h.HandleDeliveries, "/outbound/log?domain=bs.example.ru&p="+p), `Page 1 of 2`) + } } // Nothing logged yet is a sentence in the polled region, which goes on polling, @@ -84,6 +97,10 @@ func TestOutLogEmpty(t *testing.T) { body := getBodyAs(t, h.HandleDeliveries, "/outbound/log", p) has(t, "empty log", body, `No messages logged yet.`, `id="out-log-rows"`, `data-poll`, `New rows appear on their own`) lacks(t, "empty log", body, `Settings`) diff --git a/internal/web/handlers/handlers_ratelimit.go b/internal/web/handlers/handlers_ratelimit.go index 7b3e897..d504956 100644 --- a/internal/web/handlers/handlers_ratelimit.go +++ b/internal/web/handlers/handlers_ratelimit.go @@ -50,8 +50,11 @@ func parseAutoMultiplier(raw string) (float64, error) { // checkAutoMultiplier is the rule for an auto limit's multiplier, shared by // the form and a domain import. +// Written as "not within" so NaN, which compares false with everything, is +// outside too: ParseFloat reads "NaN" (audit 2026-10-12, L11 - stored as NULL, +// read back as 0, recalculated as the default without a word). func checkAutoMultiplier(v float64) error { - if v < store.MinAutoMultiplier || v > store.MaxAutoMultiplier { + if !(v >= store.MinAutoMultiplier && v <= store.MaxAutoMultiplier) { return fmt.Errorf("multiplier must be between %.1f and %.1f", store.MinAutoMultiplier, store.MaxAutoMultiplier) } return nil diff --git a/internal/web/handlers/handlers_status.go b/internal/web/handlers/handlers_status.go index d2f2733..2d2b3a3 100644 --- a/internal/web/handlers/handlers_status.go +++ b/internal/web/handlers/handlers_status.go @@ -501,7 +501,7 @@ func queueSummary(out string) string { func statusFlash(r *http.Request) string { switch { case r.URL.Query().Get("reloaded") != "": - return "Configuration regenerated from the database; OpenDKIM and Postfix have re-read it." + return "Configuration regenerated from the database - OpenDKIM's tables, every map the panel writes for Postfix, the filter lists - and OpenDKIM and Postfix have re-read it." case r.URL.Query().Get("rechecked") != "": return "DNS re-checked." default: diff --git a/internal/web/handlers/ratelimit_parse_test.go b/internal/web/handlers/ratelimit_parse_test.go new file mode 100644 index 0000000..57c2e99 --- /dev/null +++ b/internal/web/handlers/ratelimit_parse_test.go @@ -0,0 +1,23 @@ +package handlers + +import ( + "testing" + + "github.com/mixeme/selfpost/internal/store" +) + +// An auto limit's multiplier is a number within its bounds: NaN and the +// infinities, which strconv reads, are refused like any other number out of +// range (audit 2026-10-12, L11). +func TestAutoMultiplierIsANumberInRange(t *testing.T) { + for _, raw := range []string{"NaN", "nan", "+Inf", "-Inf", "inf", "1.4", "5.1", "x"} { + if v, err := parseAutoMultiplier(raw); err == nil { + t.Errorf("%q accepted as %v", raw, v) + } + } + for raw, want := range map[string]float64{"": store.DefaultAutoMultiplier, "1.5": 1.5, " 2.5 ": 2.5, "5": 5} { + if v, err := parseAutoMultiplier(raw); err != nil || v != want { + t.Errorf("%q = %v, %v; want %v", raw, v, err, want) + } + } +} diff --git a/internal/web/view/components.go b/internal/web/view/components.go index d0e66c8..8db7de1 100644 --- a/internal/web/view/components.go +++ b/internal/web/view/components.go @@ -146,6 +146,9 @@ var statusClass = map[string]string{ "error": "tag is-danger is-light", "fail": "tag is-danger is-light", "mismatch": "tag is-danger is-light", "bounced": "tag is-danger is-light", "reject": "tag is-danger is-light", + // A message refused under a level-2 limit, beside its two siblings that + // are red (audit 2026-10-12, L18). + "rate-limited": "tag is-danger is-light", "quarantine": "tag is-primary is-light", "held": "tag is-primary is-light", "global": "tag is-primary is-light", diff --git a/internal/web/view/components_test.go b/internal/web/view/components_test.go index 9914844..3e6cf06 100644 --- a/internal/web/view/components_test.go +++ b/internal/web/view/components_test.go @@ -85,7 +85,7 @@ func TestStatusTagMapsEveryStatusOfTheKitTable(t *testing.T) { } func TestStatusTagTreatsAnUnknownStatusAsNeutral(t *testing.T) { - for _, status := range []string{"", "rate-limited", "3", "DKIM", "something new"} { + for _, status := range []string{"", "limited", "3", "DKIM", "something new"} { if got := StatusTag(status); got != "tag is-light" { t.Errorf("StatusTag(%q) = %q, want the neutral tag", status, got) } diff --git a/internal/web/view/kit.go b/internal/web/view/kit.go index 79115be..5641b23 100644 --- a/internal/web/view/kit.go +++ b/internal/web/view/kit.go @@ -101,7 +101,7 @@ func KitPage() *Kit { StatusRows: []KitStatusRow{ {"ok, published, running, sent, accept", Tag{Status: "ok"}}, {"warn, deferred, quiet, tempfail", Tag{Status: "warn"}}, - {"error, fail, mismatch, bounced, reject", Tag{Status: "fail"}}, + {"error, fail, mismatch, bounced, reject, rate-limited", Tag{Status: "fail"}}, {"unknown, off, neutral counts", Tag{Status: "off"}}, {"held: quarantine · role: global", Tag{Status: "quarantine"}}, }, diff --git a/internal/web/view/outbound_log_test.go b/internal/web/view/outbound_log_test.go index 742985c..edaef6a 100644 --- a/internal/web/view/outbound_log_test.go +++ b/internal/web/view/outbound_log_test.go @@ -22,7 +22,7 @@ func TestOutLogShowsTheRowsAndTheirFilters(t *testing.T) { `Time, UTC`, `09-21 14:01:52`, `shop@shop.example.org`, `m.keller@gmx.de`, `title="[FIRING] disk usage above 90 % on db-2"`, `sent`, `deferred`, - `bounced`, `rate-limited`, `wrong From`, + `bounced`, `rate-limited`, `wrong From`, `href="/outbound/log/184220">Details`, `Page 1 of 412`, `Older →`, `New rows appear on their own`, ) diff --git a/internal/web/view/paneljs_test.go b/internal/web/view/paneljs_test.go index 58eccb7..382ae4d 100644 --- a/internal/web/view/paneljs_test.go +++ b/internal/web/view/paneljs_test.go @@ -6,17 +6,23 @@ import ( "testing" ) -// The refreshes of the monitoring boxes survive a failure and back off, -// checked by running panel.js in node against a stand-in for the DOM and -// htmx (testdata/poll_test.js; audit 2026-10-12, M11). Skipped where node is -// not installed - the test server's Go has none; the workstation runs it. -func TestPanelJSPolling(t *testing.T) { +// panel.js run in node against a stand-in for the parts of the DOM and htmx +// it uses (testdata/*_test.js): the refreshes of the monitoring boxes +// survive a failure and back off (audit 2026-10-12, M11), and a button that +// leaves the page asks first when a field holds an unsaved edit (L19). +// Skipped where node is not installed - the test server's Go has none; the +// workstation runs it. +func TestPanelJS(t *testing.T) { node, err := exec.LookPath("node") if err != nil { t.Skip("node is not installed") } - out, err := exec.Command(node, filepath.Join("testdata", "poll_test.js"), filepath.Join("static", "panel.js")).CombinedOutput() - if err != nil { - t.Fatalf("panel.js polling: %v\n%s", err, out) + for _, script := range []string{"poll_test.js", "unsaved_test.js"} { + t.Run(script, func(t *testing.T) { + out, err := exec.Command(node, filepath.Join("testdata", script), filepath.Join("static", "panel.js")).CombinedOutput() + if err != nil { + t.Fatalf("%v\n%s", err, out) + } + }) } } diff --git a/internal/web/view/static/panel.js b/internal/web/view/static/panel.js index 689ea55..18fa075 100644 --- a/internal/web/view/static/panel.js +++ b/internal/web/view/static/panel.js @@ -99,9 +99,58 @@ var form = ev.target.closest("form[data-confirm]"); if (form && !window.confirm(form.dataset.confirm)) { ev.preventDefault(); + return; + } + // A button that does something else than save - recalculate a rate + // limit - leaves the page, and what was typed on it and not saved goes + // with it. Such a button carries data-unsaved, the question to ask when + // a field of the page differs from what the page was drawn with (audit + // 2026-10-12, L19). HTML Living Standard, SubmitEvent: submitter is the + // button that submitted the form. + var button = ev.submitter; + if (button && button.dataset && button.dataset.unsaved && hasUnsavedEdit() && + !window.confirm(button.dataset.unsaved)) { + ev.preventDefault(); } }); + // hasUnsavedEdit tells whether a field of the page holds a value the page + // was not drawn with: defaultValue, defaultChecked and defaultSelected are + // what the markup said (HTML Living Standard, the input, select and + // textarea elements). A select none of whose options says selected was + // drawn with its first option, or none when it is a list (size or multiple). + function hasUnsavedEdit() { + var fields = document.querySelectorAll("input, select, textarea"); + for (var i = 0; i < fields.length; i++) { + var f = fields[i]; + if (f.disabled || f.type === "hidden" || f.type === "submit" || f.type === "button" || f.type === "reset") { + continue; + } + if (f.type === "checkbox" || f.type === "radio") { + if (f.checked !== f.defaultChecked) { + return true; + } + } else if (f.tagName === "SELECT") { + var drawn = -1; + for (var j = 0; j < f.options.length; j++) { + if (f.options[j].defaultSelected) { + drawn = j; + break; + } + } + if (drawn < 0 && !f.multiple && f.size <= 1 && f.options.length > 0) { + drawn = 0; + } + if (f.selectedIndex !== drawn) { + return true; + } + } else if (f.value !== f.defaultValue) { + return true; + } + } + return false; + } + // --- Conditional field visibility ------------------------------------ // Several forms hide a block until a select, checkbox or file input says it // applies. One rule table drives them all so the five near-identical helpers diff --git a/internal/web/view/templates/out_app.html b/internal/web/view/templates/out_app.html index 21e27e7..88e75f7 100644 --- a/internal/web/view/templates/out_app.html +++ b/internal/web/view/templates/out_app.html @@ -64,7 +64,7 @@
- {{if .Recalc}}{{end}} + {{if .Recalc}}{{end}} Cancel
diff --git a/internal/web/view/templates/out_domain_settings.html b/internal/web/view/templates/out_domain_settings.html index f55ab6e..bb802e0 100644 --- a/internal/web/view/templates/out_domain_settings.html +++ b/internal/web/view/templates/out_domain_settings.html @@ -66,7 +66,7 @@
- {{if .Rate.Auto}}
{{end}} + {{if .Rate.Auto}}
{{end}} {{if .Rate.Active}}
{{end}}

{{template "inline" .LimitNote}}

diff --git a/internal/web/view/testdata/poll_test.js b/internal/web/view/testdata/poll_test.js index 4213215..358dee4 100644 --- a/internal/web/view/testdata/poll_test.js +++ b/internal/web/view/testdata/poll_test.js @@ -1,6 +1,6 @@ // The polling of panel.js, run in node against a stand-in for the few parts // of the DOM and htmx it uses, with a clock the test moves. Run by -// TestPanelJSPolling (paneljs_test.go): node poll_test.js . +// TestPanelJS (paneljs_test.go): node poll_test.js . // Exits non-zero with what went wrong. "use strict"; const fs = require("fs"); diff --git a/internal/web/view/testdata/unsaved_test.js b/internal/web/view/testdata/unsaved_test.js new file mode 100644 index 0000000..454d92e --- /dev/null +++ b/internal/web/view/testdata/unsaved_test.js @@ -0,0 +1,105 @@ +// The question panel.js asks before a button that leaves the page - a rate +// limit's recalculation - when a field holds an edit not saved yet (audit +// 2026-10-12, L19). Run by TestPanelJS (paneljs_test.go): node +// unsaved_test.js . Exits non-zero with what went wrong. +"use strict"; +const fs = require("fs"); +const vm = require("vm"); + +const failures = []; +function check(cond, what) { + if (!cond) failures.push(what); +} + +const listeners = {}; +let fields = []; +const document = { + hidden: false, + addEventListener: (type, fn) => (listeners[type] = listeners[type] || []).push(fn), + body: { addEventListener() {} }, + getElementById: () => null, + querySelectorAll: (sel) => (sel === "input, select, textarea" ? fields : []), +}; +let asked = []; +let answer = false; +const window = { + addEventListener() {}, + confirm(q) { + asked.push(q); + return answer; + }, +}; +vm.runInNewContext(fs.readFileSync(process.argv[2], "utf8"), { + document, window, htmx: { ajax() {} }, setTimeout, clearTimeout, Date, Object, Math, console, +}); + +function input(type, value, defaultValue) { + return { tagName: "INPUT", type, value, defaultValue, disabled: false }; +} +function box(checked, defaultChecked) { + return { tagName: "INPUT", type: "checkbox", checked, defaultChecked, disabled: false }; +} +function select(selectedIndex, defaults, opts) { + return Object.assign({ + tagName: "SELECT", type: "select-one", selectedIndex, disabled: false, multiple: false, size: 0, + options: defaults.map((d) => ({ defaultSelected: d })), + }, opts || {}); +} + +// submit fires the document's submit listeners for a form with no +// data-confirm, sent by a button with the given data-unsaved, and says +// whether the submission went on. +function submit(unsaved) { + let prevented = false; + const ev = { + target: { closest: () => null }, + submitter: { dataset: unsaved === undefined ? {} : { unsaved } }, + preventDefault() { prevented = true; }, + }; + (listeners.submit || []).forEach((fn) => fn(ev)); + return !prevented; +} + +const Q = "Recalculating leaves this page. Recalculate anyway?"; +const drawn = () => [ + input("text", "100", "100"), + input("hidden", "x", "y"), // never an edit + box(true, true), + select(0, [false, false, false]), // no option says selected: the first + select(2, [false, false, true]), +]; + +function scenario(name, edit, wantAsked) { + fields = drawn(); + edit(fields); + asked = []; + answer = false; + const went = submit(Q); + check(asked.length === (wantAsked ? 1 : 0), name + ": asked " + asked.length + " times, want " + (wantAsked ? 1 : 0)); + check(went === !wantAsked, name + ": the submission " + (went ? "went on" : "was stopped")); +} + +scenario("nothing changed", () => {}, false); +scenario("a number typed", (f) => (f[0].value = "150"), true); +scenario("a box unticked", (f) => (f[2].checked = false), true); +scenario("a select moved off its first option", (f) => (f[3].selectedIndex = 1), true); +scenario("a select moved off its selected option", (f) => (f[4].selectedIndex = 0), true); +scenario("a disabled field differs", (f) => { f[0].value = "150"; f[0].disabled = true; }, false); + +// Answered yes: the submission goes on. +fields = drawn(); +fields[0].value = "150"; +asked = []; +answer = true; +check(submit(Q) === true && asked.length === 1, "answered yes, the recalculation did not go on"); + +// A button without the question - Save - never asks. +asked = []; +answer = false; +check(submit(undefined) === true && asked.length === 0, "a plain submit asked or was stopped"); + +if (failures.length) { + console.log(failures.join("\n")); + process.exit(1); +} +console.log("ok");