diff --git a/CHANGELOG.md b/CHANGELOG.md index 939fd7a..02eaf87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,14 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version the form's own message, naming the field and the application (audit 2026-10-12, M10). +- **A domain added again starts with an empty log.** Send-log rows belonged + to a domain by its name, so a domain deleted and added again showed its + earlier mail to whoever it was assigned to next, and its statistics and + level-2 count went on counting it. A row now belongs to the domain of the + application that sent it, by id (migration `0008`, filled once from the + registry); the rows of a deleted domain are the global role's alone. The + inbound log follows the same rule (audit 2026-10-12, L12). + ### Added - **The delivery log measures its own guesses** (migration `0007`). A table diff --git a/docs/audit-2026-10-12.md b/docs/audit-2026-10-12.md index c0bd8ac..1aa17c6 100644 --- a/docs/audit-2026-10-12.md +++ b/docs/audit-2026-10-12.md @@ -672,7 +672,7 @@ has already run is never edited — the next file is added; the published - [x] **Import validates like the form**: route `DMARCRua`, IPs, limit mode, window and count through the same parsers; refuse the file with the validator's text. Tests per field. — **Opus** -- [ ] **Send-log rows belong to a domain by id** (L12; owner, 2026-10-12). +- [x] **Send-log rows belong to a domain by id** (L12; owner, 2026-10-12). `send_log.domain_id INTEGER REFERENCES domains(id) ON DELETE SET NULL` in migration `0008`, backfilled once by name against the registry as it is at migration time (rows of a domain already gone stay NULL); written by the @@ -686,6 +686,8 @@ has already run is never edited — the next file is added; the published migration — my proposal, so that the two logs do not follow two rules. Tests: delete-and-re-add shows nothing to the new assignee; the old rows are still in the global log. — **Opus** + *Done 2026-10-14.* DMARC reports stay by name on purpose: a report is + about the name in DNS, and whoever holds the name now is its reader. - [ ] Docs: security.md § Roles (the guard) and the struck accepted risk, guide § Domain export, schema-migrations.md. — **Sonnet** - [ ] **Which key OpenDKIM signs with, seen and not reasoned** (owner, diff --git a/docs/roadmap.md b/docs/roadmap.md index 351d8a0..d82f54f 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`; next stages 1+2 → `2.5.0`) | 18/62 | [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`; next stages 1+2 → `2.5.0`) | 19/62 | [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/docs/schema-migrations.md b/docs/schema-migrations.md index c32052d..07b8fe7 100644 --- a/docs/schema-migrations.md +++ b/docs/schema-migrations.md @@ -82,7 +82,7 @@ audit (`2.5.0`). | 5 | `0005_send_log_rejected_index.sql` | 2.3.0 | DDL | `idx_send_log_rejected`, a partial index of the refused rows of `send_log` by time; a file of its own because the test installation had run 0004 before the review asked for it | | 6 | `0006_sender_check.sql` | 2.4.0 | DDL | `send_log.reason` and `send_log.header_from` (both default `''`: a row refused earlier keeps no reason), and `sender_check_days` | | 7 | `0007_delivery_log_days.sql` | 2.5.0 | DDL | `delivery_log_days`: what the reconcile sweep and the log tailer did, a row per UTC day, kept a year, shown nowhere - the measurement that decides the sweep's fate (audit 2026-10-12, stage 1). `send_log.reason` gains the values `reconcile` and `reconcile-contradicted` without a schema change | -| 8 | `0008_tenant_boundary.sql` | 2.5.0 | DDL | `idx_applications_login_nocase`: an application login is unique without regard to case (audit 2026-10-12, H4) | +| 8 | `0008_tenant_boundary.sql` | 2.5.0 | DDL + data | `idx_applications_login_nocase`: an application login is unique without regard to case (audit 2026-10-12, H4). `send_log.domain_id` and `inbound_spam_log.inbound_domain_id` (`ON DELETE SET NULL`), each with an index, filled once by name from the registry as it was then: a row belongs to a domain by id, and a domain deleted and added again starts with an empty log (L12) | **Kind:** *DDL* — schema only; *data* — `INSERT`/`UPDATE` that must stay correct for operators upgrading from older 2.x images. diff --git a/internal/store/inboundspam.go b/internal/store/inboundspam.go index 62e0bc8..23388f6 100644 --- a/internal/store/inboundspam.go +++ b/internal/store/inboundspam.go @@ -42,6 +42,10 @@ type InboundSpamEntry struct { At time.Time } +// inboundDomainID is the inbound_domain_id a journal row is written with: the +// inbound domain registered under the row's name now (audit 2026-10-12, L12). +const inboundDomainID = `(SELECT d.id FROM inbound_domains d WHERE d.name = ?)` + // InsertInboundSpam records one journal row. It is called from the inbound // journal milter's hot path and from the log tailer, so it returns any error // for the caller to log; neither of them may hold up mail over it. @@ -52,10 +56,10 @@ func (s *Store) InsertInboundSpam(e InboundSpamEntry) error { } _, err := s.db.Exec( `INSERT INTO inbound_spam_log - (inbound_domain, client_ip, from_addr, to_addr, subject, decision, engine, detail, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, + (inbound_domain, client_ip, from_addr, to_addr, subject, decision, engine, detail, created_at, inbound_domain_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, `+inboundDomainID+`)`, e.InboundDomain, e.ClientIP, e.From, e.To, e.Subject, e.Decision, e.Engine, e.Detail, - at.UTC().Format(time.RFC3339), + at.UTC().Format(time.RFC3339), e.InboundDomain, ) if err != nil { return fmt.Errorf("insert inbound_spam_log: %w", err) @@ -73,13 +77,13 @@ func (s *Store) InsertInboundSpamOnce(e InboundSpamEntry) (bool, error) { at := e.At.UTC().Format(time.RFC3339) res, err := s.db.Exec( `INSERT INTO inbound_spam_log - (inbound_domain, client_ip, from_addr, to_addr, subject, decision, engine, detail, created_at) - SELECT ?, ?, ?, ?, ?, ?, ?, ?, ? + (inbound_domain, client_ip, from_addr, to_addr, subject, decision, engine, detail, created_at, inbound_domain_id) + SELECT ?, ?, ?, ?, ?, ?, ?, ?, ?, `+inboundDomainID+` WHERE NOT EXISTS ( SELECT 1 FROM inbound_spam_log WHERE created_at = ? AND client_ip = ? AND from_addr = ? AND to_addr = ? AND decision = ? AND engine = ? AND detail = ?)`, - e.InboundDomain, e.ClientIP, e.From, e.To, e.Subject, e.Decision, e.Engine, e.Detail, at, + e.InboundDomain, e.ClientIP, e.From, e.To, e.Subject, e.Decision, e.Engine, e.Detail, at, e.InboundDomain, at, e.ClientIP, e.From, e.To, e.Decision, e.Engine, e.Detail, ) if err != nil { @@ -133,9 +137,10 @@ type InboundSpamRow struct { // the UI: an empty field matches everything. Since and Until are days, both // inclusive and read as UTC days whatever the time of day they carry. // -// Domains is the authorization scope, which no query parameter may widen: the -// exhaustive set of inbound domain names the caller may read, applied as an IN -// constraint. nil lifts it (the global role reads the whole journal, including +// DomainIDs is the authorization scope, which no query parameter may widen: +// the exhaustive set of inbound domains the caller may read, by id - a row of +// a domain deleted since is nobody's (audit 2026-10-12, L12) - applied as an +// IN constraint. nil lifts it (the global role reads the whole journal, including // rows of a domain deleted since); an empty set is a caller who reaches // nothing and gets nothing. Unlike SendLogFilter, forgetting the scope is // therefore the permissive case - the caller states "all" by leaving it out - @@ -145,7 +150,9 @@ type InboundSpamFilter struct { Decision string Since time.Time Until time.Time - Domains []string + // DomainIDs: the inbound domains the caller may read, by id; nil reads + // everything (the global role). + DomainIDs []int64 } // QueryInboundSpam returns journal rows matching filter, newest first, for @@ -197,16 +204,16 @@ func (s *Store) CountInboundSpam(filter InboundSpamFilter) (int64, error) { func inboundSpamWhere(f InboundSpamFilter) (string, []any) { var clauses []string var args []any - if f.Domains != nil { - if len(f.Domains) == 0 { + if f.DomainIDs != nil { + if len(f.DomainIDs) == 0 { return " WHERE 1 = 0", nil } - marks := make([]string, len(f.Domains)) - for i, name := range f.Domains { + marks := make([]string, len(f.DomainIDs)) + for i, id := range f.DomainIDs { marks[i] = "?" - args = append(args, name) + args = append(args, id) } - clauses = append(clauses, "inbound_domain IN ("+strings.Join(marks, ", ")+")") + clauses = append(clauses, "inbound_domain_id IN ("+strings.Join(marks, ", ")+")") } if f.Domain != "" { clauses = append(clauses, "inbound_domain = ?") diff --git a/internal/store/inboundspam_test.go b/internal/store/inboundspam_test.go index 91e5c01..16a19ba 100644 --- a/internal/store/inboundspam_test.go +++ b/internal/store/inboundspam_test.go @@ -172,6 +172,14 @@ func TestQueryInboundSpamFilters(t *testing.T) { t.Fatalf("InsertInboundSpam: %v", err) } } + ids := map[string]int64{} + for _, name := range []string{"a.example", "b.example"} { + d, err := st.AddInboundDomain(name) + if err != nil { + t.Fatal(err) + } + ids[name] = d.ID + } add("a.example", "1@a.example", SpamDecisionAccept, day(20, 23)) add("a.example", "2@a.example", SpamDecisionReject, day(21, 0)) add("b.example", "3@b.example", SpamDecisionAccept, day(21, 12)) @@ -227,13 +235,13 @@ func TestQueryInboundSpamFilters(t *testing.T) { // The scope: nil is everything, a set is its domains, an empty set nothing, // and a chosen domain outside the set is nothing too. - if got := to(InboundSpamFilter{Domains: []string{"b.example"}}); got != "43" { + if got := to(InboundSpamFilter{DomainIDs: []int64{ids["b.example"]}}); got != "43" { t.Errorf("scope = %q, want 43", got) } - if got := to(InboundSpamFilter{Domains: []string{}}); got != "" { + if got := to(InboundSpamFilter{DomainIDs: []int64{}}); got != "" { t.Errorf("empty scope = %q, want nothing", got) } - if got := to(InboundSpamFilter{Domains: []string{"b.example"}, Domain: "a.example"}); got != "" { + if got := to(InboundSpamFilter{DomainIDs: []int64{ids["b.example"]}, Domain: "a.example"}); got != "" { t.Errorf("a chosen domain outside the scope = %q, want nothing", got) } diff --git a/internal/store/migrations/0008_tenant_boundary.sql b/internal/store/migrations/0008_tenant_boundary.sql index 336988e..239aeae 100644 --- a/internal/store/migrations/0008_tenant_boundary.sql +++ b/internal/store/migrations/0008_tenant_boundary.sql @@ -9,3 +9,20 @@ -- (case-sensitive) stays: SQLite cannot change a column's collation without -- rebuilding the table, and this index is the stricter rule. CREATE UNIQUE INDEX idx_applications_login_nocase ON applications (login COLLATE NOCASE); + +-- A send-log row belongs to a domain by its id, not its name (audit L12; the +-- owner, 2026-10-12). A domain deleted and added again under its old name +-- used to show the earlier rows to whoever it was assigned to next. The id is +-- the domain of the login the row was written for; a row of a domain deleted +-- since - and every row older than this migration whose domain is gone - is +-- NULL and nobody's but the global role's. The name stays for display and for +-- the global view. Filled once from the registry as it is now. +ALTER TABLE send_log ADD COLUMN domain_id INTEGER REFERENCES domains(id) ON DELETE SET NULL; +UPDATE send_log SET domain_id = (SELECT d.id FROM domains d WHERE d.name = send_log.domain); +CREATE INDEX idx_send_log_domain_id_created_at ON send_log (domain_id, created_at); + +-- The inbound journal follows the same rule, so the two logs do not follow two +-- (security.md's accepted risk of the inbound log, struck). +ALTER TABLE inbound_spam_log ADD COLUMN inbound_domain_id INTEGER REFERENCES inbound_domains(id) ON DELETE SET NULL; +UPDATE inbound_spam_log SET inbound_domain_id = (SELECT d.id FROM inbound_domains d WHERE d.name = inbound_spam_log.inbound_domain); +CREATE INDEX idx_inbound_spam_log_domain_id_created_at ON inbound_spam_log (inbound_domain_id, created_at); diff --git a/internal/store/notify.go b/internal/store/notify.go index 7a85f1e..43af752 100644 --- a/internal/store/notify.go +++ b/internal/store/notify.go @@ -185,8 +185,8 @@ func (s *Store) LimitRefusalsSince(since time.Time) ([]LimitRefusal, error) { rows, err := s.db.Query( `SELECT d.name, a.login, COUNT(*), MIN(l.created_at) FROM send_log l - JOIN applications a ON a.login = l.app_login - JOIN domains d ON d.id = a.domain_id + JOIN domains d ON d.id = l.domain_id + JOIN applications a ON a.login = l.app_login AND a.domain_id = d.id WHERE l.status = 'rejected' AND l.created_at >= ? AND l.reason IN ('', 'limit') AND (EXISTS (SELECT 1 FROM rate_limits r WHERE r.scope = 'application' AND r.ref_id = a.id) OR EXISTS (SELECT 1 FROM rate_limits r WHERE r.scope = 'domain' AND r.ref_id = d.id)) diff --git a/internal/store/ratelimits.go b/internal/store/ratelimits.go index db2a23d..962e497 100644 --- a/internal/store/ratelimits.go +++ b/internal/store/ratelimits.go @@ -170,13 +170,17 @@ func (s *Store) RateLimit(scope, ref string) (RateLimit, bool, error) { // semantics — and excludes rows that were themselves rejected by a limit (they // were never sent). It reuses the send log the journal already writes (README // § Rate limiting — the limiter reuses the send log). +// +// A domain's messages are the rows of the domain registered under ref now, by +// id: rows a deleted domain of the same name left behind are not its (audit +// 2026-10-12, L12). func (s *Store) CountMessages(scope, ref string, since time.Time) (int64, error) { - var column string + var match string switch scope { case RateLimitScopeDomain: - column = "domain" + match = domainByName case RateLimitScopeApp: - column = "app_login" + match = "app_login = ?" default: return 0, fmt.Errorf("unknown rate-limit scope %q", scope) } @@ -185,7 +189,7 @@ func (s *Store) CountMessages(scope, ref string, since time.Time) (int64, error) // same format is chronologically correct (as in DeleteSendLogBefore). err := s.db.QueryRow( `SELECT COUNT(DISTINCT queue_id) FROM send_log - WHERE `+column+` = ? AND status != ? AND created_at >= ?`, + WHERE `+match+` AND status != ? AND created_at >= ?`, ref, StatusRejected, since.UTC().Format(time.RFC3339), ).Scan(&n) if err != nil { diff --git a/internal/store/ratelimits_test.go b/internal/store/ratelimits_test.go index 561de10..df64d8c 100644 --- a/internal/store/ratelimits_test.go +++ b/internal/store/ratelimits_test.go @@ -84,6 +84,11 @@ func TestRateLimitByNameAndLogin(t *testing.T) { func TestCountMessagesDistinctAndWindowed(t *testing.T) { st := openTestStore(t) + // A domain's messages are those of its applications (send_log.domain_id). + d, _ := st.AddDomain("example.com", "selfpost") + if _, err := st.AddApplication(d.ID, "app1", AddressModeWildcard, nil); err != nil { + t.Fatal(err) + } // Two recipients share a queue-id → one message. A second message → two. for _, to := range []string{"a@x.net", "b@x.net"} { diff --git a/internal/store/schema_test.go b/internal/store/schema_test.go index 2e634be..0387dbb 100644 --- a/internal/store/schema_test.go +++ b/internal/store/schema_test.go @@ -47,7 +47,7 @@ func TestBaselineSchema(t *testing.T) { "applications": "id domain_id login address_mode auth_ip_restrict auth_allowed_ips created_at", "application_addresses": "id application_id address", "rate_limits": "id scope ref_id max_messages window_seconds mode auto_multiplier auto_updated_at", - "send_log": "id queue_id domain app_login from_addr to_addr subject status created_at updated_at reason header_from", + "send_log": "id queue_id domain app_login from_addr to_addr subject status created_at updated_at reason header_from domain_id", "logtail_state": "path fingerprint read_offset updated_at", "inbound_domains": "id name recipient_mode created_at", "inbound_transports": "inbound_domain_id host port tls_mode", @@ -55,7 +55,7 @@ func TestBaselineSchema(t *testing.T) { "user_inbound_domains": "user_id inbound_domain_id", "dmarc_reports": "id domain reporter report_id period_begin period_end received_at contact_email policy_p policy_sp policy_pct policy_adkim policy_aspf pass_count fail_count recipient", "dmarc_report_records": "id report_row_id source_ip count disposition spf_result dkim_result header_from", - "inbound_spam_log": "id inbound_domain client_ip from_addr to_addr subject decision engine detail created_at", + "inbound_spam_log": "id inbound_domain client_ip from_addr to_addr subject decision engine detail created_at inbound_domain_id", "inbound_spam_list": "id list_type entry_type value note created_at", "send_log_lines": "id queue_id logged_at line", "notify_state": "key phase count mark since sent_at", @@ -105,8 +105,8 @@ func TestBaselineSchema(t *testing.T) { rows.Close() wantIdx := []string{ "idx_applications_login_nocase", "idx_dmarc_report_records_report", "idx_dmarc_reports_domain", "idx_dmarc_reports_received", - "idx_inbound_spam_log_created_at", "idx_inbound_spam_log_domain_created_at", - "idx_send_log_app_login_created_at", "idx_send_log_created_at", "idx_send_log_domain", "idx_send_log_lines_queue_id", "idx_send_log_queue_id", "idx_send_log_rejected", + "idx_inbound_spam_log_created_at", "idx_inbound_spam_log_domain_created_at", "idx_inbound_spam_log_domain_id_created_at", + "idx_send_log_app_login_created_at", "idx_send_log_created_at", "idx_send_log_domain", "idx_send_log_domain_id_created_at", "idx_send_log_lines_queue_id", "idx_send_log_queue_id", "idx_send_log_rejected", "idx_sessions_expires_at", } sort.Strings(wantIdx) diff --git a/internal/store/sendlog.go b/internal/store/sendlog.go index 8b16727..bd90379 100644 --- a/internal/store/sendlog.go +++ b/internal/store/sendlog.go @@ -40,6 +40,12 @@ type SendLogEntry struct { HeaderFrom string } +// loginDomainID is the domain_id a row is written with: the domain of the +// application whose login it is, NULL for a login that is no application's +// (audit 2026-10-12, L12). The domain name of the row stays what the +// envelope said. +const loginDomainID = `(SELECT a.domain_id FROM applications a WHERE a.login = ?)` + // InsertQueued records an accepted message in the send log with status // "queued". It is called from the journal-milter hot path, so it returns any // error for the caller to log rather than deciding policy here; the milter @@ -48,9 +54,9 @@ func (s *Store) InsertQueued(e SendLogEntry) error { now := time.Now().UTC().Format(time.RFC3339) _, err := s.db.Exec( `INSERT INTO send_log - (queue_id, domain, app_login, from_addr, to_addr, subject, status, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, - e.QueueID, e.Domain, e.AppLogin, e.From, e.To, e.Subject, StatusQueued, now, now, + (queue_id, domain, app_login, from_addr, to_addr, subject, status, created_at, updated_at, domain_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, `+loginDomainID+`)`, + e.QueueID, e.Domain, e.AppLogin, e.From, e.To, e.Subject, StatusQueued, now, now, e.AppLogin, ) if err != nil { return fmt.Errorf("insert send_log: %w", err) @@ -67,9 +73,9 @@ func (s *Store) InsertRejected(e SendLogEntry) error { now := time.Now().UTC().Format(time.RFC3339) _, err := s.db.Exec( `INSERT INTO send_log - (queue_id, domain, app_login, from_addr, to_addr, subject, status, reason, header_from, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, - e.QueueID, e.Domain, e.AppLogin, e.From, e.To, e.Subject, StatusRejected, e.Reason, e.HeaderFrom, now, now, + (queue_id, domain, app_login, from_addr, to_addr, subject, status, reason, header_from, created_at, updated_at, domain_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, `+loginDomainID+`)`, + e.QueueID, e.Domain, e.AppLogin, e.From, e.To, e.Subject, StatusRejected, e.Reason, e.HeaderFrom, now, now, e.AppLogin, ) if err != nil { return fmt.Errorf("insert rejected send_log: %w", err) @@ -201,9 +207,12 @@ func (s *Store) ListQueuedOlderThan(cutoff time.Time) ([]QueuedDelivery, error) // Persistence): a SendLogEntry plus the fields that only exist once a row has // been written (id, current status, timestamps). type SendLogRow struct { - ID int64 - QueueID string - Domain string + ID int64 + QueueID string + Domain string + // DomainID is the domain the row belongs to; 0 when it is nobody's but + // the global role's (its domain was deleted, or its login was none). + DomainID int64 AppLogin string From string To string @@ -231,9 +240,9 @@ func (s *Store) GetSendLog(id int64) (SendLogRow, error) { createdAt, updatedAt string ) err := s.db.QueryRow( - `SELECT id, queue_id, domain, app_login, from_addr, to_addr, subject, status, reason, header_from, created_at, updated_at + `SELECT id, queue_id, domain, COALESCE(domain_id, 0), app_login, from_addr, to_addr, subject, status, reason, header_from, created_at, updated_at FROM send_log WHERE id = ?`, id, - ).Scan(&row.ID, &row.QueueID, &row.Domain, &row.AppLogin, &row.From, &row.To, + ).Scan(&row.ID, &row.QueueID, &row.Domain, &row.DomainID, &row.AppLogin, &row.From, &row.To, &row.Subject, &row.Status, &row.Reason, &row.HeaderFrom, &createdAt, &updatedAt) if errors.Is(err, sql.ErrNoRows) { return SendLogRow{}, ErrSendLogNotFound @@ -252,9 +261,10 @@ func (s *Store) GetSendLog(id int64) (SendLogRow, error) { // Domain and AppLogin are the operator's own filters, chosen in the UI: an // empty field matches everything. // -// Domains and AllDomains are the authorization scope, which no query parameter -// may widen. Domains is the exhaustive set of domain names the caller is -// entitled to read, applied as an IN constraint; AllDomains lifts that +// DomainIDs and AllDomains are the authorization scope, which no query +// parameter may widen. DomainIDs is the exhaustive set of domains the caller +// is entitled to read, by id - a row whose domain was deleted is nobody's +// (audit 2026-10-12, L12) - applied as an IN constraint; AllDomains lifts that // restriction and is the only way to read the whole journal. A zero-valued // filter therefore matches *no* rows: a caller that forgets to state a scope // gets an empty log rather than every tenant's mail, which is the failure mode @@ -262,7 +272,7 @@ func (s *Store) GetSendLog(id int64) (SendLogRow, error) { type SendLogFilter struct { Domain string AppLogin string - Domains []string + DomainIDs []int64 AllDomains bool } @@ -273,7 +283,7 @@ func (s *Store) QuerySendLog(filter SendLogFilter, limit, offset int) ([]SendLog where, args := sendLogWhere(filter) args = append(args, limit, offset) rows, err := s.db.Query( - `SELECT id, queue_id, domain, app_login, from_addr, to_addr, subject, status, reason, header_from, created_at, updated_at + `SELECT id, queue_id, domain, COALESCE(domain_id, 0), app_login, from_addr, to_addr, subject, status, reason, header_from, created_at, updated_at FROM send_log`+where+` ORDER BY id DESC LIMIT ? OFFSET ?`, @@ -290,7 +300,7 @@ func (s *Store) QuerySendLog(filter SendLogFilter, limit, offset int) ([]SendLog row SendLogRow createdAt, updatedAt string ) - if err := rows.Scan(&row.ID, &row.QueueID, &row.Domain, &row.AppLogin, + if err := rows.Scan(&row.ID, &row.QueueID, &row.Domain, &row.DomainID, &row.AppLogin, &row.From, &row.To, &row.Subject, &row.Status, &row.Reason, &row.HeaderFrom, &createdAt, &updatedAt); err != nil { return nil, fmt.Errorf("scan send_log row: %w", err) } @@ -319,15 +329,15 @@ func sendLogWhere(f SendLogFilter) (string, []any) { // No scope is not "no restriction": a domain administrator whose last // assignment was deleted owns nothing in the journal and must see // nothing, and the same clause catches a caller that never set a scope. - if len(f.Domains) == 0 { + if len(f.DomainIDs) == 0 { return " WHERE 1 = 0", nil } - marks := make([]string, len(f.Domains)) - for i, name := range f.Domains { + marks := make([]string, len(f.DomainIDs)) + for i, id := range f.DomainIDs { marks[i] = "?" - args = append(args, name) + args = append(args, id) } - clauses = append(clauses, "domain IN ("+strings.Join(marks, ", ")+")") + clauses = append(clauses, "domain_id IN ("+strings.Join(marks, ", ")+")") } if f.Domain != "" { clauses = append(clauses, "domain = ?") diff --git a/internal/store/sendlog_test.go b/internal/store/sendlog_test.go index e09c850..3f4832d 100644 --- a/internal/store/sendlog_test.go +++ b/internal/store/sendlog_test.go @@ -274,7 +274,13 @@ func TestUpdateStatusNoMatch(t *testing.T) { // decided who is asking, and the safe answer to that is nothing. func TestSendLogScopeIsMandatory(t *testing.T) { st := openTestStore(t) + ids := map[string]int64{} for _, domain := range []string{"first.example.ru", "second.example.ru"} { + d := addTestDomain(t, st, domain) + ids[domain] = d.ID + if _, err := st.AddApplication(d.ID, "app-"+domain, AddressModeWildcard, nil); err != nil { + t.Fatal(err) + } if err := st.InsertQueued(SendLogEntry{ QueueID: "Q-" + domain, Domain: domain, AppLogin: "app-" + domain, From: "noreply@" + domain, To: "public@example.net", Subject: domain, @@ -288,14 +294,14 @@ func TestSendLogScopeIsMandatory(t *testing.T) { want []string }{ "no scope": {SendLogFilter{}, nil}, - "empty scope": {SendLogFilter{Domains: []string{}}, nil}, + "empty scope": {SendLogFilter{DomainIDs: []int64{}}, nil}, "all domains": {SendLogFilter{AllDomains: true}, []string{"first.example.ru", "second.example.ru"}}, - "one domain": {SendLogFilter{Domains: []string{"first.example.ru"}}, []string{"first.example.ru"}}, - "two domains": {SendLogFilter{Domains: []string{"first.example.ru", "second.example.ru"}}, []string{"first.example.ru", "second.example.ru"}}, - "unknown domain": {SendLogFilter{Domains: []string{"third.example.ru"}}, nil}, - "filter within": {SendLogFilter{Domain: "first.example.ru", Domains: []string{"first.example.ru", "second.example.ru"}}, []string{"first.example.ru"}}, - "filter outside": {SendLogFilter{Domain: "second.example.ru", Domains: []string{"first.example.ru"}}, nil}, - "app filter outside": {SendLogFilter{AppLogin: "app-second.example.ru", Domains: []string{"first.example.ru"}}, nil}, + "one domain": {SendLogFilter{DomainIDs: []int64{ids["first.example.ru"]}}, []string{"first.example.ru"}}, + "two domains": {SendLogFilter{DomainIDs: []int64{ids["first.example.ru"], ids["second.example.ru"]}}, []string{"first.example.ru", "second.example.ru"}}, + "unknown domain": {SendLogFilter{DomainIDs: []int64{9999}}, nil}, + "filter within": {SendLogFilter{Domain: "first.example.ru", DomainIDs: []int64{ids["first.example.ru"], ids["second.example.ru"]}}, []string{"first.example.ru"}}, + "filter outside": {SendLogFilter{Domain: "second.example.ru", DomainIDs: []int64{ids["first.example.ru"]}}, nil}, + "app filter outside": {SendLogFilter{AppLogin: "app-second.example.ru", DomainIDs: []int64{ids["first.example.ru"]}}, nil}, } { rows, err := st.QuerySendLog(tc.filter, 50, 0) if err != nil { @@ -352,3 +358,46 @@ func TestDeleteSendLogBefore(t *testing.T) { t.Fatalf("retention kept wrong rows: %+v", rows) } } + +// A domain deleted and added again under its old name starts with an empty +// log for whoever it is assigned to: its earlier rows belong to no domain and +// only the global role reads them (audit 2026-10-12, L12). Statistics and the +// level-2 count follow the same rule. +func TestADomainAddedAgainStartsWithAnEmptyLog(t *testing.T) { + st := openTestStore(t) + old := addTestDomain(t, st, "shop.example") + if _, err := st.AddApplication(old.ID, "shop", AddressModeWildcard, nil); err != nil { + t.Fatal(err) + } + if err := st.InsertQueued(SendLogEntry{QueueID: "OLD", Domain: "shop.example", AppLogin: "shop", To: "a@example.net"}); err != nil { + t.Fatal(err) + } + if err := st.DeleteDomain(old.ID); err != nil { + t.Fatal(err) + } + again := addTestDomain(t, st, "shop.example") + if _, err := st.AddApplication(again.ID, "shop", AddressModeWildcard, nil); err != nil { + t.Fatal(err) + } + if err := st.InsertQueued(SendLogEntry{QueueID: "NEW", Domain: "shop.example", AppLogin: "shop", To: "a@example.net"}); err != nil { + t.Fatal(err) + } + + rows, err := st.QuerySendLog(SendLogFilter{DomainIDs: []int64{again.ID}}, 10, 0) + if err != nil { + t.Fatal(err) + } + if len(rows) != 1 || rows[0].QueueID != "NEW" || rows[0].DomainID != again.ID { + t.Fatalf("the new domain's log = %+v, want its own row alone", rows) + } + all, _ := st.QuerySendLog(SendLogFilter{AllDomains: true}, 10, 0) + if len(all) != 2 { + t.Fatalf("the global log has %d rows, want both", len(all)) + } + if n, err := st.CountMessages(RateLimitScopeDomain, "shop.example", time.Now().Add(-time.Hour)); err != nil || n != 1 { + t.Fatalf("level-2 count = %d, %v; want the new domain's one message", n, err) + } + if stats, err := st.DomainSendStats("shop.example", 90, time.Now().Add(-time.Hour)); err != nil || stats.Total != 1 { + t.Fatalf("statistics = %+v, %v; want one message", stats, err) + } +} diff --git a/internal/store/stats.go b/internal/store/stats.go index 2fecdd3..41c9045 100644 --- a/internal/store/stats.go +++ b/internal/store/stats.go @@ -46,10 +46,11 @@ func StatsWindow(retentionDays int, entityCreated time.Time) (since time.Time, h } // DomainSendStats returns 30-day (or shorter when retention is lower) send -// statistics for a domain name. +// statistics for a domain, named by its name: the rows that belong to the +// domain registered under it now (audit 2026-10-12, L12). func (s *Store) DomainSendStats(name string, retentionDays int, createdAt time.Time) (SendStats, error) { since, hours, windowDays := StatsWindow(retentionDays, createdAt) - stats, err := s.sendStats("domain", name, since, hours) + stats, err := s.sendStats(domainByName, name, since, hours) if err != nil { return SendStats{}, err } @@ -61,7 +62,7 @@ func (s *Store) DomainSendStats(name string, retentionDays int, createdAt time.T // AppSendStats returns send statistics for an application login. func (s *Store) AppSendStats(login string, retentionDays int, createdAt time.Time) (SendStats, error) { since, hours, windowDays := StatsWindow(retentionDays, createdAt) - stats, err := s.sendStats("app_login", login, since, hours) + stats, err := s.sendStats("app_login = ?", login, since, hours) if err != nil { return SendStats{}, err } @@ -70,30 +71,36 @@ func (s *Store) AppSendStats(login string, retentionDays int, createdAt time.Tim return stats, nil } -func (s *Store) sendStats(column, ref string, since time.Time, hours float64) (SendStats, error) { +// domainByName selects a domain's send-log rows by the id of the domain +// registered under the name it is given. +const domainByName = "domain_id = (SELECT d.id FROM domains d WHERE d.name = ?)" + +// sendStats counts the rows that match, a condition with one placeholder +// taking ref. +func (s *Store) sendStats(match, ref string, since time.Time, hours float64) (SendStats, error) { sinceStr := since.UTC().Format(time.RFC3339) var total int64 err := s.db.QueryRow( `SELECT COUNT(DISTINCT queue_id) FROM send_log - WHERE `+column+` = ? AND status != ? AND created_at >= ?`, + WHERE `+match+` AND status != ? AND created_at >= ?`, ref, StatusRejected, sinceStr, ).Scan(&total) if err != nil { - return SendStats{}, fmt.Errorf("send stats total %s %q: %w", column, ref, err) + return SendStats{}, fmt.Errorf("send stats total %q: %w", ref, err) } var peak int64 err = s.db.QueryRow( `SELECT COALESCE(MAX(bucket_count), 0) FROM ( SELECT COUNT(DISTINCT queue_id) AS bucket_count FROM send_log - WHERE `+column+` = ? AND status != ? AND created_at >= ? + WHERE `+match+` AND status != ? AND created_at >= ? GROUP BY substr(created_at, 1, 13) )`, ref, StatusRejected, sinceStr, ).Scan(&peak) if err != nil { - return SendStats{}, fmt.Errorf("send stats peak %s %q: %w", column, ref, err) + return SendStats{}, fmt.Errorf("send stats peak %q: %w", ref, err) } avg := float64(total) / hours diff --git a/internal/web/handlers/authz.go b/internal/web/handlers/authz.go index 5b2fd20..12b3cf7 100644 --- a/internal/web/handlers/authz.go +++ b/internal/web/handlers/authz.go @@ -57,3 +57,13 @@ func domainNameSet(domains []store.Domain) map[string]bool { } return m } + +// domainIDSet is the set of the ids of domains; a send-log row's DomainID is +// in it when the row belongs to one of them (0, a row of no domain, never is). +func domainIDSet(domains []store.Domain) map[int64]bool { + m := make(map[int64]bool, len(domains)) + for _, d := range domains { + m[d.ID] = true + } + return m +} diff --git a/internal/web/handlers/handlers_health_page_test.go b/internal/web/handlers/handlers_health_page_test.go index b45463a..d9aa616 100644 --- a/internal/web/handlers/handlers_health_page_test.go +++ b/internal/web/handlers/handlers_health_page_test.go @@ -189,6 +189,10 @@ func TestOverviewPageShowsVerdictsAndDomains(t *testing.T) { if err != nil { t.Fatalf("AddDomain: %v", err) } + // A row is the domain's through its application (send_log.domain_id). + if _, err := h.store.AddApplication(d.ID, "app", store.AddressModeWildcard, nil); err != nil { + t.Fatal(err) + } if err := h.store.InsertQueued(store.SendLogEntry{QueueID: "Q1", Domain: d.Name, AppLogin: "app", From: "noreply@" + d.Name, To: "public@example.net", Subject: "Hello"}); err != nil { t.Fatalf("InsertQueued: %v", err) } diff --git a/internal/web/handlers/handlers_inbound_log.go b/internal/web/handlers/handlers_inbound_log.go index 57ba196..79f4aa2 100644 --- a/internal/web/handlers/handlers_inbound_log.go +++ b/internal/web/handlers/handlers_inbound_log.go @@ -39,9 +39,11 @@ func (h *Handlers) HandleInboundLog(w http.ResponseWriter, r *http.Request) { return } names := make([]string, 0, len(assigned)) + ids := make([]int64, 0, len(assigned)) offered := make(map[string]bool, len(assigned)) for _, d := range assigned { names = append(names, d.Name) + ids = append(ids, d.ID) offered[d.Name] = true } @@ -65,7 +67,7 @@ func (h *Handlers) HandleInboundLog(w http.ResponseWriter, r *http.Request) { // behind by a domain that has since been deleted; anyone else only what is // assigned to them, however empty that is. if !p.IsGlobal() { - query.Domains = names + query.DomainIDs = ids } total, err := h.store.CountInboundSpam(query) diff --git a/internal/web/handlers/handlers_monitor.go b/internal/web/handlers/handlers_monitor.go index f6be110..acde537 100644 --- a/internal/web/handlers/handlers_monitor.go +++ b/internal/web/handlers/handlers_monitor.go @@ -94,7 +94,7 @@ func (h *Handlers) HandleDelivery(w http.ResponseWriter, r *http.Request) { http.Error(w, "internal error", http.StatusInternalServerError) return } - if !p.IsGlobal() && !domainNameSet(allowed)[row.Domain] { + if !p.IsGlobal() && !domainIDSet(allowed)[row.DomainID] { http.NotFound(w, r) return } @@ -113,7 +113,7 @@ func (h *Handlers) HandleDelivery(w http.ResponseWriter, r *http.Request) { // The names link to their pages when there is a page to go to: a domain // that was deleted since, or an application of another domain, is text. for _, d := range allowed { - if d.Name != row.Domain { + if d.ID != row.DomainID { continue } in.DomainHref = view.DomainHref(d.ID) @@ -317,8 +317,10 @@ func (h *Handlers) outLogPage(r *http.Request) (*view.OutLog, error) { allowedNames := domainNameSet(assigned) domainNames := make([]string, 0, len(assigned)) + domainIDs := make([]int64, 0, len(assigned)) for _, d := range assigned { domainNames = append(domainNames, d.Name) + domainIDs = append(domainIDs, d.ID) } loginSet := make(map[string]bool) @@ -341,8 +343,10 @@ func (h *Handlers) outLogPage(r *http.Request) (*view.OutLog, error) { Domain: q.Get("domain"), AppLogin: q.Get("app"), // A global administrator reads the whole journal, including rows left - // behind by a domain that has since been deleted. - Domains: domainNames, + // behind by a domain that has since been deleted; anyone else the rows + // of the domains assigned to them, by id - not those an earlier domain + // of the same name left (audit 2026-10-12, L12). + DomainIDs: domainIDs, AllDomains: p.IsGlobal(), } if !p.IsGlobal() { diff --git a/internal/web/handlers/handlers_users_test.go b/internal/web/handlers/handlers_users_test.go index dc5f652..8b65b74 100644 --- a/internal/web/handlers/handlers_users_test.go +++ b/internal/web/handlers/handlers_users_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/mixeme/selfpost/internal/store" + "github.com/mixeme/selfpost/internal/web/auth" ) // The users form's writes, driven through the handlers (audit 2026-10-12, M7 @@ -97,3 +98,55 @@ func TestUserFormCreatesWithEmailAndReach(t *testing.T) { t.Fatalf("created %+v", u) } } + +// A domain deleted and added again under its old name starts with an empty +// log for whoever it is assigned to; its earlier rows are the global role's +// alone (audit 2026-10-12, L12). +func TestADomainAddedAgainShowsItsAssigneeNothingOld(t *testing.T) { + h, st := inboundHandlers(t) + root := userWith(t, st, "root", store.RoleGlobal, store.Reach{}) + old, _ := st.AddDomain("shop.example", "mail") + st.AddApplication(old.ID, "shop", store.AddressModeWildcard, nil) + if err := st.InsertQueued(store.SendLogEntry{QueueID: "OLDQ", Domain: "shop.example", AppLogin: "shop", To: "a@example.net", Subject: "old mail"}); err != nil { + t.Fatal(err) + } + if err := st.DeleteDomain(old.ID); err != nil { + t.Fatal(err) + } + again, _ := st.AddDomain("shop.example", "mail") + st.AddApplication(again.ID, "shop", store.AddressModeWildcard, nil) + if err := st.InsertQueued(store.SendLogEntry{QueueID: "NEWQ", Domain: "shop.example", AppLogin: "shop", To: "a@example.net", Subject: "new mail"}); err != nil { + t.Fatal(err) + } + owner := userWith(t, st, "owner", store.RoleDomain, store.Reach{DomainIDs: []int64{again.ID}}) + rows, _ := st.QuerySendLog(store.SendLogFilter{AllDomains: true}, 10, 0) + var oldID, newID int64 + for _, r := range rows { + if r.QueueID == "OLDQ" { + oldID = r.ID + } else { + newID = r.ID + } + } + + page := func(p *auth.Principal, id int64) int { + return send(h.HandleDelivery, p, "GET", "/outbound/log/"+idStr(id), map[string]string{"id": idStr(id)}, nil).Code + } + if code := page(&owner, oldID); code != http.StatusNotFound { + t.Errorf("the old row's page for the new assignee = %d, want 404", code) + } + if code := page(&owner, newID); code != http.StatusOK { + t.Errorf("the new row's page for its assignee = %d, want 200", code) + } + if code := page(&root, oldID); code != http.StatusOK { + t.Errorf("the old row's page for the global role = %d, want 200", code) + } + body := send(h.HandleDeliveries, &owner, "GET", "/outbound/log", nil, nil).Body.String() + if strings.Contains(body, "old mail") || !strings.Contains(body, "new mail") { + t.Errorf("the new assignee's log shows the old row or misses the new one") + } + body = send(h.HandleDeliveries, &root, "GET", "/outbound/log", nil, nil).Body.String() + if !strings.Contains(body, "old mail") || !strings.Contains(body, "new mail") { + t.Errorf("the global log is missing a row") + } +}