domains: a domain's DMARC reports are deleted with it (audit A5/W2)
Security audit 2026-10-17, finding A5/W2 (low), stage 1; the mockup is
7effc98. dmarc_reports belong to a domain by name, outlive the domain (no
FK), and a domain deleted and added again handed its old reports to
whoever it was assigned to then - the logs were moved to ids in 2.5.0 for
exactly this case, reports were not.
The cheap form of the plan: store.DeleteDomain deletes the domain's
reports (their records by the table's cascade) in the same transaction as
the domain. The confirmation page says so when the domain has reports
("delete the 12 DMARC reports received for it;"), counted by the new
store.CountDMARCReports; NewOutDomainDelete takes the count.
Tests: TestDeletingADomainDeletesItsReports (the domain added again has
none, another domain keeps its report and record, a second delete is
ErrDomainNotFound); TestOutDomainDeleteNamesWhatGoesWithIt with 12, 1 and
0 reports.
Docs: security.md § Roles; CHANGELOG; plan.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -28,6 +28,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
|
||||
reports into, under any domain, or another domain's hosted one. Only the
|
||||
hosted address of a domain that chose it is a mailbox now, and a report
|
||||
must arrive at the address tagged with its domain.
|
||||
- **A domain's DMARC reports are deleted with it** (audit A5/W2): reports
|
||||
belong to a domain by name, and a domain deleted and added again showed
|
||||
its old reports to whoever it was assigned to then.
|
||||
- **A stranger's recipient no longer writes an Inbound › Log row** (audit
|
||||
M2): Postfix quotes a refused recipient in its reply with the quotes
|
||||
removed, so `RCPT TO:<"; from=<x> to=<y> …"@domain>` spelled a second
|
||||
|
||||
@@ -745,9 +745,10 @@ the argument.
|
||||
2026-10-17:* the live registry holds two hosted addresses and no custom
|
||||
one on the hostname; TestCustomReportAddressOnTheHostnameIsRefused, the
|
||||
untagged cases in TestIngestMessageRejectsTagMismatch.
|
||||
- [ ] **A5/W2.** Delete a domain's reports in `domain.Service.Delete` (the
|
||||
- [x] **A5/W2.** Delete a domain's reports in `domain.Service.Delete` (the
|
||||
cheap form); security.md § Roles says reports go with the domain. —
|
||||
**Sonnet**, 0.5 h. (The `domain_id` form is better and 2 h; take it if
|
||||
**Sonnet**, 0.5 h. *Done 2026-10-17:* in `store.DeleteDomain`, one
|
||||
transaction with the domain; TestDeletingADomainDeletesItsReports. (The `domain_id` form is better and 2 h; take it if
|
||||
stage 1 is split across two sittings.)
|
||||
- [ ] **W3.** A mutex around `Resync`'s read + write + reload. — **Sonnet**,
|
||||
0.5 h.
|
||||
|
||||
+1
-1
@@ -193,7 +193,7 @@ assigned outbound domains and inbound domains **separately**, each list with
|
||||
| Overview, Server (Health, System log, Backup, Users, Settings, kit page) | yes | no — 404 |
|
||||
| Outbound queue | yes | no — 404 (a queue is not filtered by domain) |
|
||||
| Outbound: domain page, applications, Domain settings (report address, rate limit, export), log | every domain | assigned outbound domains |
|
||||
| Outbound › DMARC reports | every report, with the server's ingest statistics | reports of assigned domains, without the ingest box |
|
||||
| Outbound › DMARC reports | every report, with the server's ingest statistics | reports of assigned domains, without the ingest box (a domain's reports are deleted with it, so a domain added again under the same name starts with none) |
|
||||
| Inbound: domain page, upstream, recipients | every inbound domain | assigned inbound domains |
|
||||
| Inbound › Log | every row, including those of a domain deleted since | rows of assigned inbound domains (by id: not those of an earlier domain of the same name), without the links into the filter lists |
|
||||
| Inbound › Filter lists; the inbound log's retention | yes | no — 404 |
|
||||
|
||||
@@ -161,6 +161,16 @@ func (s *Store) PruneDMARCReports(domain string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// CountDMARCReports is how many reports a domain has: what deleting it
|
||||
// deletes with it (DeleteDomain).
|
||||
func (s *Store) CountDMARCReports(domain string) (int, error) {
|
||||
var n int
|
||||
if err := s.db.QueryRow(`SELECT COUNT(*) FROM dmarc_reports WHERE domain = ?`, domain).Scan(&n); err != nil {
|
||||
return 0, fmt.Errorf("count dmarc reports of %s: %w", domain, err)
|
||||
}
|
||||
return n, nil
|
||||
}
|
||||
|
||||
// IncrDMARCParseFailures bumps the failure counter in settings.
|
||||
func (s *Store) IncrDMARCParseFailures() error {
|
||||
raw, err := s.GetSetting(DMARCParseFailuresKey)
|
||||
|
||||
@@ -106,3 +106,42 @@ func TestListDMARCReportsEmptyListIsNoRows(t *testing.T) {
|
||||
t.Errorf("all domains with a list = %d report(s), %v; want both (AllDomains wins)", len(list), err)
|
||||
}
|
||||
}
|
||||
|
||||
// A domain's DMARC reports go when the domain does, records included, and
|
||||
// another domain's stay: a domain added again under the old name starts with
|
||||
// none (security audit 2026-10-17, A5/W2).
|
||||
func TestDeletingADomainDeletesItsReports(t *testing.T) {
|
||||
st := openTestStore(t)
|
||||
gone, _ := st.AddDomain("gone.example", "sel")
|
||||
if _, err := st.AddDomain("kept.example", "sel"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
now := time.Now().UTC().Truncate(time.Second)
|
||||
for _, d := range []string{"gone.example", "kept.example"} {
|
||||
if _, err := st.InsertDMARCReport(DMARCReport{
|
||||
Domain: d, Reporter: "google.com", ReportID: "r-" + d, PeriodBegin: now.Add(-24 * time.Hour), PeriodEnd: now, ReceivedAt: now,
|
||||
Records: []DMARCReportRecord{{SourceIP: "203.0.113.1", Count: 1, SPFResult: "pass", DKIMResult: "pass"}},
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if err := st.DeleteDomain(gone.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := st.AddDomain("gone.example", "sel"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if list, err := st.ListDMARCReports(DMARCReportScope{Domains: []string{"gone.example"}}, 10); err != nil || len(list) != 0 {
|
||||
t.Errorf("the domain added again has %d old reports (%v)", len(list), err)
|
||||
}
|
||||
if list, err := st.ListDMARCReports(DMARCReportScope{Domains: []string{"kept.example"}}, 10); err != nil || len(list) != 1 {
|
||||
t.Errorf("the other domain has %d reports (%v), want 1", len(list), err)
|
||||
}
|
||||
var records int
|
||||
if err := st.db.QueryRow("SELECT COUNT(*) FROM dmarc_report_records").Scan(&records); err != nil || records != 1 {
|
||||
t.Errorf("%d report records left (%v), want the kept domain's one", records, err)
|
||||
}
|
||||
if err := st.DeleteDomain(gone.ID); err != ErrDomainNotFound {
|
||||
t.Errorf("deleting it again = %v, want ErrDomainNotFound", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -132,8 +132,22 @@ func (s *Store) GetDomain(id int64) (Domain, error) {
|
||||
// foreign_keys ON, which Open's DSN sets on every one
|
||||
// (https://sqlite.org/foreignkeys.html § 2, § 4.3); the rows the cascade
|
||||
// removes are not in RowsAffected (https://sqlite.org/c3ref/changes.html).
|
||||
//
|
||||
// The domain's DMARC reports go with it, in the same transaction (their
|
||||
// records by the table's cascade): reports belong to a domain by name, and a
|
||||
// domain deleted and added again - by anyone, for anyone - must not hand its
|
||||
// old reports to whoever it is assigned to then (security audit 2026-10-17,
|
||||
// A5/W2). The logs keep their rows, by id, for the global role.
|
||||
func (s *Store) DeleteDomain(id int64) error {
|
||||
res, err := s.db.Exec("DELETE FROM domains WHERE id = ?", id)
|
||||
tx, err := s.db.Begin()
|
||||
if err != nil {
|
||||
return fmt.Errorf("delete domain: begin: %w", err)
|
||||
}
|
||||
defer tx.Rollback() //nolint:errcheck // a no-op after Commit
|
||||
if _, err := tx.Exec(`DELETE FROM dmarc_reports WHERE domain = (SELECT name FROM domains WHERE id = ?)`, id); err != nil {
|
||||
return fmt.Errorf("delete domain: its DMARC reports: %w", err)
|
||||
}
|
||||
res, err := tx.Exec("DELETE FROM domains WHERE id = ?", id)
|
||||
if err != nil {
|
||||
return fmt.Errorf("delete domain: %w", err)
|
||||
}
|
||||
@@ -144,6 +158,9 @@ func (s *Store) DeleteDomain(id int64) error {
|
||||
if n == 0 {
|
||||
return ErrDomainNotFound
|
||||
}
|
||||
if err := tx.Commit(); err != nil {
|
||||
return fmt.Errorf("delete domain: commit: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -136,7 +136,13 @@ func (h *Handlers) HandleDeleteConfirm(w http.ResponseWriter, r *http.Request) {
|
||||
for i, a := range apps {
|
||||
logins[i] = a.Login
|
||||
}
|
||||
h.view.Render(w, http.StatusOK, "out-domain-delete", view.NewOutDomainDelete(h.shellMeta(r), d.ID, d.Name, logins))
|
||||
reports, err := h.store.CountDMARCReports(d.Name)
|
||||
if err != nil {
|
||||
logf("panel: delete domain %d: %v", d.ID, err)
|
||||
http.Error(w, "internal error", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
h.view.Render(w, http.StatusOK, "out-domain-delete", view.NewOutDomainDelete(h.shellMeta(r), d.ID, d.Name, logins, reports))
|
||||
}
|
||||
|
||||
// HandleDeleteDomain performs the deletion and returns to the domain list.
|
||||
|
||||
@@ -260,7 +260,7 @@ func outAppCreatedFixture() *OutAppCreated {
|
||||
// outDomainDeleteFixture is the mockup's confirmation: the domain has the two
|
||||
// applications that go with it.
|
||||
func outDomainDeleteFixture() *OutDomainDelete {
|
||||
return NewOutDomainDelete(admin(), 1, "example.org", []string{"prod-server", "alerts"})
|
||||
return NewOutDomainDelete(admin(), 1, "example.org", []string{"prod-server", "alerts"}, 12)
|
||||
}
|
||||
|
||||
// outLogFixture is the mockup's Outbound log: six messages of three domains on
|
||||
|
||||
@@ -768,8 +768,9 @@ type OutDomainDelete struct {
|
||||
}
|
||||
|
||||
// NewOutDomainDelete builds the page. logins are the applications the domain
|
||||
// has: they are named, because they go with it.
|
||||
func NewOutDomainDelete(m Meta, id int64, name string, logins []string) *OutDomainDelete {
|
||||
// has: they are named, because they go with it; reports is how many DMARC
|
||||
// reports go with it (security audit 2026-10-17, A5/W2), not said when none.
|
||||
func NewOutDomainDelete(m Meta, id int64, name string, logins []string, reports int) *OutDomainDelete {
|
||||
m.Title, m.Section, m.Page = "Delete "+name, "outbound", "domains"
|
||||
href := DomainHref(id)
|
||||
p := &OutDomainDelete{
|
||||
@@ -797,6 +798,12 @@ func NewOutDomainDelete(m Meta, id int64, name string, logins []string) *OutDoma
|
||||
item = append(item, Inline{Text: ") with their credentials and sender bindings;"})
|
||||
p.Consequences = append(p.Consequences, item)
|
||||
}
|
||||
switch {
|
||||
case reports == 1:
|
||||
p.Consequences = append(p.Consequences, Rich("delete the ", Strong("DMARC report"), " received for it;"))
|
||||
case reports > 1:
|
||||
p.Consequences = append(p.Consequences, Rich("delete the ", Strong(strconv.Itoa(reports)+" DMARC reports"), " received for it;"))
|
||||
}
|
||||
p.Consequences = append(p.Consequences, Rich("stop signing mail as the domain."))
|
||||
return p
|
||||
}
|
||||
|
||||
@@ -435,18 +435,20 @@ func TestOutDomainDeleteNamesWhatGoesWithIt(t *testing.T) {
|
||||
`<h1 class="title is-3">Delete example.org</h1>`, `<div class="box sp-danger-zone">`, `<h2>This cannot be undone</h2>`,
|
||||
`Deleting <strong>example.org</strong> will:`, `permanently delete its DKIM signing key;`,
|
||||
`delete <strong>both applications</strong> (<span class="sp-mono">prod-server</span>, <span class="sp-mono">alerts</span>) with their credentials and sender bindings;`,
|
||||
`delete the <strong>12 DMARC reports</strong> received for it;`,
|
||||
`stop signing mail as the domain.`, `Inbound domains are not touched.`,
|
||||
`<form method="post" action="/outbound/domains/1/delete">`, `class="button is-danger"`, `Delete example.org</button>`,
|
||||
`href="/outbound/domains/1">Keep it</a>`,
|
||||
`<h2>Moving it instead?</h2>`, `<a href="/outbound/domains/1/settings">Export the domain</a>`,
|
||||
)
|
||||
|
||||
// No applications: nothing to say about them.
|
||||
out = renderSignedIn(t, "out-domain-delete", NewOutDomainDelete(admin(), 1, "example.org", nil))
|
||||
pageLacks(t, "Delete", out, `applications`, `credentials`)
|
||||
// No applications and no reports: nothing to say about them.
|
||||
out = renderSignedIn(t, "out-domain-delete", NewOutDomainDelete(admin(), 1, "example.org", nil, 0))
|
||||
pageLacks(t, "Delete", out, `applications`, `credentials`, `received for it`)
|
||||
pageHas(t, "Delete", out, `permanently delete its DKIM signing key;`, `stop signing mail as the domain.`)
|
||||
out = renderSignedIn(t, "out-domain-delete", NewOutDomainDelete(admin(), 1, "example.org", []string{"only"}))
|
||||
pageHas(t, "Delete", out, `<strong>its application</strong> (<span class="sp-mono">only</span>)`)
|
||||
out = renderSignedIn(t, "out-domain-delete", NewOutDomainDelete(admin(), 1, "example.org", []string{"only"}, 1))
|
||||
pageHas(t, "Delete", out, `<strong>its application</strong> (<span class="sp-mono">only</span>)`,
|
||||
`delete the <strong>DMARC report</strong> received for it;`)
|
||||
}
|
||||
|
||||
// ---- Helpers
|
||||
|
||||
Reference in New Issue
Block a user