diff --git a/cmd/e2a/main.go b/cmd/e2a/main.go index e762b3cf4..2b77d43ed 100644 --- a/cmd/e2a/main.go +++ b/cmd/e2a/main.go @@ -352,6 +352,9 @@ func main() { // External-sending-access decisions (shadow impact / enforce refusals) // as bounded counters; a no-op while the control is disabled. sendingpolicy.SetExternalAccessObserver(metrics.ExternalAccessDecision) + // Deletion-resistant feedback ingestion outcomes (B8): bounded + // outcome × bucket counter, no address/account/id labels. + sendingpolicy.SetFeedbackObserver(metrics.SendingFeedbackIngested) outboxWorker := webhookpub.NewOutboxWorker(pool, store).WithMetrics(metrics) smtpRelay := outbound.NewSMTPRelay(&cfg.OutboundSMTP) sender := outbound.NewSenderWithDKIM(smtpRelay, cfg.OutboundSMTP.FromDomain, store) @@ -421,7 +424,8 @@ func main() { // window, durable in Postgres): the cross-replica counterpart of the // acceptance-time in-memory limiter, enforced immediately before // provider submission so scheduled-send bursts can't exceed it. - rate: sendrate.NewStore(pool, time.Minute, 60), + rate: sendrate.NewStore(pool, time.Minute, 60), + sharedDomains: nonEmpty(cfg.SharedDomain), }) outboundJobs := outboundSending.jobs registrars = append(registrars, outboundJobs) @@ -429,6 +433,22 @@ func main() { // seam with tokens from the same gate. sendingGate, providerSubmitter := outboundSending.gate, outboundSending.submitter registrars = append(registrars, sendramp.NewMaintenanceJobs(rampStore)) + // Deletion-resistant feedback provenance (B8): refuse to start if any + // retained recipient row was signed under a key version this keyring + // does not hold — feedback for it could never be matched and the + // detector would be silently blind. The retention janitor and the + // consumer's accounting seam hang off the same module. + if outboundSending.module == nil { + log.Fatalf("sending policy gate is not the concrete module; feedback accounting cannot be wired") + } + if err := outboundSending.module.VerifyKeyringCoverage(ctx); err != nil { + log.Fatalf("Sending feedback keyring coverage: %v", err) + } + // The purge seal resolves the post-deletion horizon at purge time from + // the EFFECTIVE policy — the same accessor the retention janitor reads — + // not from the config-file policy captured here at boot. + store.SetFeedbackRetentionResolver(outboundSending.module.EffectiveFeedbackRetention) + registrars = append(registrars, outboundSending.feedbackMaintenance()) // Queue depth/age gauges: a 30s maintenance periodic sampling river_job // per queue+state (docs/observability.md). registrars = append(registrars, jobs.NewQueueStatsJobs(pool, metrics)) @@ -987,7 +1007,8 @@ func main() { // 4b). Fail-closed: the SNS signature is verified and the TopicArn must be // in the configured allow-list (empty allow-list → every message is // rejected, so this is inert until ops wires the topic). - deliveryConsumer := delivery.NewConsumer(store, deliveryEventFirer(webhookOutbox), outboundSendStore.FinalizeProviderAcceptedTx) + deliveryConsumer := outboundSending.armDeliveryConsumer( + delivery.NewConsumer(store, deliveryEventFirer(webhookOutbox), outboundSendStore.FinalizeProviderAcceptedTx)) deliveryVerifier := delivery.NewVerifier(cfg.DeliveryFeedback.SNSTopicARNs, delivery.HTTPCertFetcher) // Public webhook receiver for AWS SNS (SES delivery/bounce/complaint). Named // /webhooks/ — it's an inbound third-party callback, not an internal diff --git a/cmd/e2a/outbound_wiring.go b/cmd/e2a/outbound_wiring.go index aa16a4a77..110be93ab 100644 --- a/cmd/e2a/outbound_wiring.go +++ b/cmd/e2a/outbound_wiring.go @@ -1,9 +1,12 @@ package main import ( + "strings" + "github.com/jackc/pgx/v5/pgxpool" "github.com/tokencanopy/e2a/internal/agent" + "github.com/tokencanopy/e2a/internal/delivery" "github.com/tokencanopy/e2a/internal/hitlnotify" "github.com/tokencanopy/e2a/internal/identity" "github.com/tokencanopy/e2a/internal/outbound" @@ -25,13 +28,19 @@ type outboundSendingDeps struct { sesConfigSet string metrics outboundsend.Metrics rate outboundsend.RateGate + // sharedDomains are the deployment's shared agent domains (config + // shared_domain). Deliveries to agents hosted on them never count toward + // the outcome detector's denominator. + sharedDomains []string } // outboundSending is the composed outbound send path. type outboundSending struct { gate sendingpolicy.Gate // module is the same policy owner behind gate, exposed through its other - // narrow roles (external-sending-access preflight/status/requests). + // narrow roles: external-sending-access preflight/status/requests, the + // deletion-resistant feedback processor, the keyring coverage check, and + // the retention janitor all hang off it. module *sendingpolicy.Module submitter *outbound.ProviderSubmitter jobs *outboundsend.Jobs @@ -44,7 +53,8 @@ type outboundSending struct { // enqueue and authorizes every worker execution through the same gate. No // raw sender and no direct ramp store reach the worker from here. func newOutboundSending(d outboundSendingDeps) outboundSending { - module := sendingpolicy.NewPolicyModule(d.pool, d.secrets, d.source, d.policy) + module := sendingpolicy.NewPolicyModule(d.pool, d.secrets, d.source, d.policy). + WithFeedbackExcludedDomains(d.sharedDomains...) var gate sendingpolicy.Gate = module submitter := outbound.NewProviderSubmitter(d.relay, gate) // Delivery feedback: tag outbound with the SES configuration set so SES @@ -99,3 +109,30 @@ func (s outboundSending) armAPI(api *agent.API) { api.SetProviderSubmitter(s.submitter, s.gate) api.SetExternalAccess(s.module) } + +// armDeliveryConsumer installs the deletion-resistant accounting seam on the +// SES feedback consumer. Without it the consumer still acks and still runs +// the message lifecycle, so the omission is silent: provider evidence for a +// purged message is simply dropped and the detector reads as healthy. +func (s outboundSending) armDeliveryConsumer(c *delivery.Consumer) *delivery.Consumer { + return c.WithFeedbackProcessor(s.module) +} + +// feedbackMaintenance is the retention janitor for feedback provenance and +// daily outcome aggregates. Unregistered, nothing enforces the +// post-deletion horizon. +func (s outboundSending) feedbackMaintenance() *sendingpolicy.MaintenanceJobs { + return sendingpolicy.NewMaintenanceJobs(s.module) +} + +// nonEmpty returns the non-blank values, so an unset config string does not +// become an empty-domain entry. +func nonEmpty(values ...string) []string { + var out []string + for _, v := range values { + if strings.TrimSpace(v) != "" { + out = append(out, v) + } + } + return out +} diff --git a/cmd/e2a/sending_policy_wiring_test.go b/cmd/e2a/sending_policy_wiring_test.go index 206113818..7035e9b64 100644 --- a/cmd/e2a/sending_policy_wiring_test.go +++ b/cmd/e2a/sending_policy_wiring_test.go @@ -10,6 +10,7 @@ import ( "github.com/tokencanopy/e2a/internal/agent" "github.com/tokencanopy/e2a/internal/config" + "github.com/tokencanopy/e2a/internal/delivery" "github.com/tokencanopy/e2a/internal/outbound" "github.com/tokencanopy/e2a/internal/sendingpolicy" "github.com/tokencanopy/e2a/internal/testutil/testdb" @@ -46,6 +47,9 @@ func TestSendingPolicyWiring(t *testing.T) { if got := composed.submitter.SESConfigurationSet(); got != "e2a-delivery-test" { t.Fatalf("submitter configuration set = %q, want the deployment's — delivery feedback must stay on", got) } + if composed.module == nil || sendingpolicy.Gate(composed.module) != composed.gate { + t.Fatal("the concrete module (feedback processor, keyring coverage, retention janitor) is not the composed gate") + } if composed.jobs.Gate() != composed.gate { t.Fatal("the jobs bundle does not hold the composed gate") } @@ -125,3 +129,44 @@ func TestNotificationAndPlatformMailWiring(t *testing.T) { t.Fatal("armAPI did not hand the API the submitter and gate") } } + +// TestFeedbackAccountingWiring pins the two composition-root edges that are +// invisible at runtime when they are missing. A delivery consumer without +// the accounting seam still acks every SES notification and still runs the +// message lifecycle, so nothing fails — the detector simply never receives +// evidence. An unregistered retention janitor never enforces the +// post-deletion horizon, so provenance accumulates forever. Both are one +// line in main.go, and neither has any other test. +func TestFeedbackAccountingWiring(t *testing.T) { + pool := testdb.TestDB(t) + relay := outbound.NewSMTPRelay(&config.OutboundSMTPConfig{Host: "relay.invalid", Port: 587, FromDomain: "test.e2a.dev"}) + composed := newOutboundSending(outboundSendingDeps{ + pool: pool, + relay: relay, + secrets: sendingpolicy.Secrets{}, + source: sendingpolicy.PolicySourceConfig, + policy: sendingpolicy.DisabledPolicy(), + // main passes nonEmpty(cfg.SharedDomain). + sharedDomains: nonEmpty("agents.localhost", " "), + }) + if !composed.module.ExcludesFeedbackDomain("agents.localhost") { + t.Fatal("the shared agent domain did not reach the feedback denominator exclusion") + } + + bare := delivery.NewConsumer(nil, nil) + if bare.FeedbackProcessorWired() { + t.Fatal("a bare consumer must not claim an accounting seam") + } + if armed := composed.armDeliveryConsumer(bare); !armed.FeedbackProcessorWired() { + t.Fatal("the composition root did not install the feedback processor on the delivery consumer") + } + + janitor := composed.feedbackMaintenance() + if janitor == nil { + t.Fatal("no feedback retention janitor composed") + } + periodics := janitor.RegisterJobs(river.NewWorkers()) + if len(periodics) != 2 { + t.Fatalf("retention periodics = %d, want 2 (retention pass + reconcile)", len(periodics)) + } +} diff --git a/docs/design/async-message-pipeline.md b/docs/design/async-message-pipeline.md index 09b111ffd..848dfd229 100644 --- a/docs/design/async-message-pipeline.md +++ b/docs/design/async-message-pipeline.md @@ -363,12 +363,117 @@ of Prepare itself, not of every caller. Two consequences worth knowing. Notification and feedback mail now cross the same submitter as customer mail, so it carries `X-SES-CONFIGURATION-SET` -and SES publishes delivery feedback for it; none of it correlates to a -message row, and the SNS consumer acks it as unknown (a log line, no -suppression). And the closure guard fences `net/smtp` and the SES v2 SDK +and SES publishes delivery feedback for it. None of it correlates to a +message row, so the SNS consumer's message-lifecycle half still acks it as +unknown; since B8 its retained sending correlation does match, which feeds +the detector but never repairs a suppression (see that addendum). And the closure guard fences `net/smtp` and the SES v2 SDK import; a send through some other HTTP provider API would be a new dependency, which is where review catches it. +## Addendum (2026-09-07): deletion-resistant feedback provenance (B8) + +Slice B8 makes provider feedback count even when the message, the agent, or +the whole account it belongs to is gone. The SNS consumer now runs a +deletion-resistant accounting seam (`delivery.FeedbackProcessor`, implemented +by the sending-policy module) BEFORE it looks for a live message, in its own +transaction, and fails the notification if that accounting fails so the +provider retries. The seam never reads `messages`, `agent_identities`, or +`users`: + +- **Correlation** is by the SES message id bound at settlement (normalized + to SES's bare form), then by the random `X-E2A-Provider-Attempt` marker SES + echoes from the submitted headers. Each recipient the event names is + matched against the keyed HMACs recorded at authorization; an address + outside the authorized envelope proves nothing and is ignored. +- **One row per provider event id** (`sending_feedback_events`), so a + redelivered notification is a zero delta. +- **Buckets** are derived from the full kind and retained subtypes: + `delivered`; `terminal_other` for a transient or undetermined bounce; + `hard_bounce` for a permanent bounce, including the global-list subtype + `Suppressed`; `complaint` for a genuine complaint. The account- and + tenant-suppression-list subtypes are `none` (SES never attempted delivery) + but still repair the local suppression list, and so does a complaint with + a suppression-list `complaintSubType`. +- **Evidence is monotonic per authorized recipient**: `none < delivered < + terminal_other < hard_bounce < complaint`. Higher evidence replaces lower + (the prior bucket is subtracted from the epoch/day it was counted in and + the new one added to the account's current `outcome_epoch` and the + ingestion UTC day, on the correlation's immutable shared/dedicated path); + a delayed lower-ranked callback never erases a hard bounce or complaint; + equal rank is a zero delta with provider time then event id deciding the + stored provenance. +- **Suppression ownership stays with the live message.** The seam only + *reports* the repairs an event proves; when a message row survives, the + consumer writes the suppression inside its own transaction exactly as + before, keeping the `source_message_id` and diagnostic reason the + suppression API returns and keeping the row's insert atomic with the + `suppression_added` event that announces it. Only when no message can own + the row — the purged-message case this slice exists for, including a + message purged between correlation and the live transaction's lock — does + the consumer apply the repair through the seam, in one transaction with a + `suppression.added` (no message id) for each row it actually inserted; an + address already suppressed is refreshed but not re-announced. Repair is + limited to customer messages: a bounce on an approval notice must not + suppress the account owner's own address. Upserts go through + `internal/suppressionsync`, which advances the row's `sync_generation` and + clears `removal_pending`; that guard has no production remover yet and is + the contract Task 11's provider reconciliation will build on. +- **After account deletion** feedback still advances the retained bucket + provenance but recreates no customer state. The 30-day retention horizon is + stamped on the account's correlations and events at **purge** (the seal + transaction that makes the account irrecoverable), not at the user's delete + click: a trashed account stays restorable for the trash window and is not + stamped, so provenance for a self-deleted account can live for the trash + window plus 30 days after purge. The horizon is read at purge time from the + effective (database-source when configured) policy. Correlation lookups take + `FOR SHARE` and skip rows past their horizon, so feedback racing the seal + inherits its expiry and a concurrent janitor delete cannot produce a + spurious "uncorrelated" result. The hourly `sending_feedback_maintenance` + job removes expired correlations together with their recipients and events, + and daily outcome rows older than the detector window plus one day. The + daily `sending_feedback_reconcile` job stamps the horizon on customer + provenance whose account no longer exists (a purge before B8, or a + correlation authorized in a race with a purge) and sweeps events whose + correlation is gone. Migration 124 ran that stamp once for the backlog with + a **fixed 30 days** (the policy default), not the effective policy value. +- **Keyring coverage is a startup gate**: a server that HAS a keyring + refuses to start if any unexpired recipient row was signed under a version + that keyring does not hold. Rotation is superset-first: add the new key + while the old stays active, then move the active version, and drop the old + key only once no retained row references it. Because a live account's rows + never expire, a version that has signed for a live account is effectively + permanent — treat the keyring as append-only. A deployment with no keyring + at all is not checked: it signs and matches nothing by design, and + bricking it over rows an earlier configuration wrote would turn a disabled + feature into an outage. + +- **Denominator exclusion.** A delivery or terminal non-hard bounce to a + recipient a sender can generate at will is bucket `none`: the SES mailbox + simulator, the configured `shared_domain`, platform-owned verified + `domains` rows, and the sending account's own verified domains. Hard + bounces and complaints from those recipients still count. Known limits: + the match is on the exact recipient domain (a subdomain of an excluded + domain still counts), domains verified by *other* accounts still count + (a two-account dilution is not closed), and there is no DNS/MX lookup at + ingestion, so a customer domain whose MX points at this deployment is not + recognized as hosted. +- **IDN recipients.** Authorization accepts internationalized domains as + typed; both the recipient HMAC and feedback matching use the IDNA + lookup-profile ASCII form of the domain (raw form as a fallback for rows + signed earlier), and the send-time suppression lookup checks the A-label + and Unicode spellings, so a suppression repaired from provider feedback in + A-label form still blocks a Unicode-typed recipient. +- **Metric.** `e2a_sending_feedback_ingested_total{outcome,bucket}` (see + `docs/observability.md`); `uncorrelated_with_marker` is the alert signal, + and `dead_account` with bucket `complaint` is the operator's cue to consider + a manual `-escalate-deleted-account-to-abuse`. + +The detector itself (thresholds, pauses, notices) is B9. B8 captures +evidence; the one new customer-visible behavior is that a hard bounce or +complaint for a message that has since been purged now repairs the account +suppression, where before it was acked and dropped. Suppressions for live +messages keep their existing shape and events exactly. + ## Addendum (2026-09-26): external sending access `internal/sendingpolicy/external_access.go` adds one permission decision that diff --git a/docs/observability.md b/docs/observability.md index 6c1e1058f..7152b9f31 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -107,6 +107,7 @@ acceptance SLI below deliberately excludes them. | `e2a_outbound_attempt_duration_seconds` | histogram | — | Upstream (SES/SMTP relay) submission duration. | | `e2a_outbound_rate_deferred_total` | counter | — | Submissions deferred by the per-agent fire-time rate limiter (`internal/sendrate`, 60/min/agent sliding window, enforced in the send worker immediately before provider submission). A deferral snoozes the job without burning an attempt, metering, or emitting a terminal event — the message fires when the window frees capacity (re-fire spread by a deterministic per-message jitter). A sustained high rate means agents are queueing behind their own budget. Deferral loops are bounded by the 72h retry horizon, so a backlog beyond ~259,200 messages/agent/burst (60 × 60 × 72) fails its tail terminally with `send_rate_timeout` (`failed_local_retries`). | | `e2a_external_access_decisions_total` | counter | `stage`, `route`, `mode` | External-sending-access evaluations (`internal/sendingpolicy`), only when the control is in shadow or enforce mode. `stage` ∈ preflight/acceptance/authorization/redemption; `route` ∈ not_applicable/custom_identity/operator_approval/paid_entitlement/restricted_recipients/denied; `mode` ∈ shadow/enforce. In shadow, `route="denied"` counts the sends enforcement would refuse — the evidence for moving to enforce. No account, address or domain labels. | +| `e2a_sending_feedback_ingested_total` | counter | `outcome`, `bucket` | Deletion-resistant SES feedback ingestion (`internal/sendingpolicy`). `outcome` ∈ correlated/dead_account/unmatched_recipient/uncorrelated_with_marker/uncorrelated/duplicate; `bucket` ∈ delivered/hard_bounce/complaint/terminal_other/none (the detector bucket actually applied; deliveries to the SES simulator, the shared agent domain, or the sender's own verified domains are `none`). One sample per recipient of a first-seen correlated event, one per uncorrelated or duplicate event. `uncorrelated_with_marker` is mail e2a stamped whose retained correlation is missing — alert on it; `dead_account` is evidence for a purged account (provenance only). No account, address, domain or id labels. | | `e2a_outbound_terminal_total` | counter | `outcome` | Messages reaching a terminal submission outcome, **exactly once per message**: `sent`, `failed_suppressed`, `failed_provider`, `failed_local_retries`, `failed_cancelled` (policy cancel settled by the reconciler — kept out of `failed_local_retries` so cancellations can't mask a retries-exhausted regression). A deferred final attempt is counted when the terminal reconciler settles it — as `sent` when provider-accept evidence arrived (never a false failure), else as a failure. | | `e2a_outbound_terminal_latency_seconds` | histogram | — | Eligibility→terminal latency per message (the terminal write's occurred_at − the **submission anchor**), observed **at most once per message**, co-located with the `e2a_outbound_terminal_total` emission so the two share their exactly-once contract (the SNS-feedback settle path is deliberately uninstrumented for both; a terminal landing before its own anchor records the count with no sample — see the SLO section). The anchor is `messages.created_at` for an ordinary send, and the latest of `created_at`, `scheduled_at`, and `reviewed_at` otherwise: a HITL hold anchors at the moment it was approved into the send pipeline, a scheduled send at its fire time. Neither wait is e2a latency, and charging them here made every hold approved after >5 min — and every send scheduled further out than the window — an automatic SLO miss. Provider-evidence settles use the evidence's accept time as occurred_at, so they measure anchor→provider-accept, not anchor→sweep. Buckets span seconds→days (the 72h retry horizon is the tail). | diff --git a/internal/delivery/consumer.go b/internal/delivery/consumer.go index c75f2f9d8..377b69e9c 100644 --- a/internal/delivery/consumer.go +++ b/internal/delivery/consumer.go @@ -156,6 +156,70 @@ type Consumer struct { store Store fire Firer finalizeAcceptance ProviderAcceptanceFinalizer + // feedback is the deletion-resistant accounting seam (B8). It runs + // before any live-message lookup, in its own transaction, so provider + // evidence for a purged message still lands; nil means no accounting + // (self-host with the policy module absent). + feedback FeedbackProcessor +} + +// WithFeedbackProcessor installs the deletion-resistant accounting seam. +func (c *Consumer) WithFeedbackProcessor(p FeedbackProcessor) *Consumer { + c.feedback = p + return c +} + +// FeedbackProcessorWired reports whether the accounting seam is installed. +// Without it every notification is still acked and the lifecycle half still +// runs, so a missing processor is invisible at runtime — the detector just +// never sees anything. The composition root's test asserts this. +func (c *Consumer) FeedbackProcessorWired() bool { return c.feedback != nil } + +// repairPending reports whether the accounting seam proved suppressions +// that only the provenance path can write. +func (c *Consumer) repairPending(accounted FeedbackResult) bool { + return c.feedback != nil && accounted.AccountRef != "" && len(accounted.RepairNeeded) > 0 +} + +// repairWithoutMessageTx applies the account-wide suppressions the retained +// provenance proved when no live message can own them, and fires one +// suppression.added per row it actually INSERTED. An address that was +// already suppressed (a manual entry, an earlier bounce) is refreshed by the +// upsert but never re-announced. Rows and events share tx, so a failure +// anywhere rolls both back and the SNS retry redoes both: exactly one row and +// one event either way. The event is keyed on the provider event, so an +// outbox-level redelivery dedupes too. +func (c *Consumer) repairWithoutMessageTx(ctx context.Context, tx pgx.Tx, ev *Event, accounted FeedbackResult) error { + if !c.repairPending(accounted) { + return nil + } + inserted, err := c.feedback.RepairSuppressionsTx(ctx, tx, accounted.AccountRef, accounted.RepairNeeded) + if err != nil { + return fmt.Errorf("suppression repair: %w", err) + } + if c.fire != nil { + // A row appearing in the customer's suppression list with no event + // is the state/notification desync the message-backed path avoids; + // the payload's message id is documented as present only when still + // known, which is exactly this case. + for _, rep := range inserted { + if err := c.fire(ctx, tx, FiredEvent{ + UserID: accounted.AccountRef, + Type: EventSuppressionAdded, + Data: eventpayload.DomainSuppressionAddedData{ + Address: rep.Address, Source: rep.Source, Reason: rep.Reason, + }, + DedupKey: "provider-feedback:" + ev.ProviderEventID + ":" + rep.Address + ":" + EventSuppressionAdded, + OccurredAt: ev.OccurredAt, + }); err != nil { + return fmt.Errorf("announce repaired suppression: %w", err) + } + } + } + if len(inserted) > 0 { + log.Printf("[delivery] SES %s repaired %d suppression(s) for a message that no longer exists", ev.Kind, len(inserted)) + } + return nil } // NewConsumer builds the consumer. fire may be nil (no events). @@ -181,6 +245,21 @@ func (c *Consumer) Process(ctx context.Context, ev *Event) error { if ev.OccurredAt.IsZero() { return fmt.Errorf("provider event timestamp is required") } + // Deletion-resistant accounting first, and on its own: it must not + // depend on a surviving message, agent, or user row, and a failure here + // must make the provider retry rather than let the lifecycle path below + // consume the notification. Its event-id dedupe makes that retry a zero + // delta, and the lifecycle path has its own dedupe keys, so the two + // halves committing independently is safe in both orders. + var accounted FeedbackResult + if c.feedback != nil { + var err error + accounted, err = c.feedback.ProcessProviderFeedback(ctx, ev.FeedbackFor()) + if err != nil { + return fmt.Errorf("provider feedback accounting: %w", err) + } + } + m, found, err := c.store.CorrelateBySESMessageID(ctx, ev.SESMessageID) if err != nil { return err @@ -198,6 +277,18 @@ func (c *Consumer) Process(ctx context.Context, ev *Event) error { } } if !found { + // No live message can own a suppression for this event, but the + // retained provenance may still prove one — this is the purged / + // deleted-message case B8 exists for. The account-wide repair and + // the suppression.added events for the rows it actually inserted + // commit in one transaction of their own (see repairWithoutMessageTx). + if c.repairPending(accounted) { + if err := c.store.WithTx(ctx, func(tx pgx.Tx) error { + return c.repairWithoutMessageTx(ctx, tx, ev, accounted) + }); err != nil { + return err + } + } if len(ev.Recipients) == 0 { return nil } @@ -219,7 +310,10 @@ func (c *Consumer) Process(ctx context.Context, ev *Event) error { return err } if !found { - return nil + // Purged between correlation and this lock: no live message can + // own the suppression any more, so the retained provenance does, + // exactly as in the !found branch above. + return c.repairWithoutMessageTx(ctx, tx, ev, accounted) } if ev.Kind.requiresApplicableRecipient() { addresses := make([]string, 0, len(ev.Recipients)) @@ -233,7 +327,12 @@ func (c *Consumer) Process(ctx context.Context, ev *Event) error { return err } if !applicable { - return nil + // The message (or its recipient rows) went away under us. + // RepairNeeded holds only recipients whose HMAC matched the + // authorized envelope, so falling through to the provenance + // repair cannot suppress an address this account never + // sent to. + return c.repairWithoutMessageTx(ctx, tx, ev, accounted) } } if ev.Kind.impliesProviderAcceptance() { @@ -286,6 +385,13 @@ func (c *Consumer) Process(ctx context.Context, ev *Event) error { source = suppressionSourceCompl suppressionReason = messagelifecycle.ReasonSuppressionComplaintApplied } + // The live message owns the customer-visible row: it is the + // only writer that knows the source message id and the + // diagnostic the suppression API returns, and its insert + // must share this transaction with the event that + // announces it. The accounting seam deliberately does not + // write it here (see repairWithoutMessageTx, used only when + // no message can own the repair). suppressionID, added, err := c.store.AddSuppressionTx(ctx, tx, m.UserID, r.Address, r.Detail, source, m.MessageID) if err != nil { return err diff --git a/internal/delivery/consumer_test.go b/internal/delivery/consumer_test.go index fbb29688e..a4d012cc8 100644 --- a/internal/delivery/consumer_test.go +++ b/internal/delivery/consumer_test.go @@ -33,6 +33,8 @@ type fakeConsumerStore struct { applicable bool preflights int lockOrder []string + // agentGone makes LockAgentTx report the agent purged after correlation. + agentGone bool } func newFakeConsumerStore() *fakeConsumerStore { @@ -59,7 +61,7 @@ func (f *fakeConsumerStore) WithTx(ctx context.Context, fn func(tx pgx.Tx) error } func (f *fakeConsumerStore) LockAgentTx(context.Context, pgx.Tx, string) (bool, error) { f.lockOrder = append(f.lockOrder, "agent") - return true, nil + return !f.agentGone, nil } func (f *fakeConsumerStore) HasApplicableRecipientTx(context.Context, pgx.Tx, string, []string) (bool, error) { f.lockOrder = append(f.lockOrder, "preflight") diff --git a/internal/delivery/feedback_seam.go b/internal/delivery/feedback_seam.go new file mode 100644 index 000000000..117893e00 --- /dev/null +++ b/internal/delivery/feedback_seam.go @@ -0,0 +1,85 @@ +package delivery + +import ( + "context" + "time" + + "github.com/jackc/pgx/v5" +) + +// ProviderAttemptHeader is the random per-attempt correlation marker the +// provider adapter stamps on every submission (outbound.ProviderAttemptHeader +// — that package imports this one, so the name is mirrored here and pinned +// equal by a test there). SES echoes it back in mail.headers; it is the +// correlation fallback when the worker died between SES accepting a message +// and the provider id being stored. +const ProviderAttemptHeader = "X-E2A-Provider-Attempt" + +// ProviderFeedback is one signed provider notification reduced to what the +// deletion-resistant accounting needs: identity (event id, time), kind and +// subtypes, the two correlation keys, and the plaintext recipients the +// provider named. It carries no message, agent, or account reference — the +// processor derives those from its own retained rows. +type ProviderFeedback struct { + ProviderEventID string + OccurredAt time.Time + Kind EventKind + // BounceType is the normalized permanent | transient | undetermined; + // BounceSubType and ComplaintSubType are the raw SES subtypes, retained + // because the detector bucket is derived from them. + BounceType string + BounceSubType string + ComplaintSubType string + // ProviderMessageID is the SES mail.messageId; AttemptCorrelationID is + // the echoed ProviderAttemptHeader (empty when headers were absent). + ProviderMessageID string + AttemptCorrelationID string + // Recipients are the normalized addresses this notification is about. + Recipients []string +} + +// FeedbackRepair names an account-wide suppression the signed event proves +// should exist. The processor reports these rather than writing them +// whenever a live message still owns the customer-visible row: that row +// carries the source message id and diagnostic the suppression API returns, +// and its insert must share a transaction with the event that announces it. +type FeedbackRepair struct { + Address string + Source string + Reason string +} + +// FeedbackResult is what the processor did with one notification. +type FeedbackResult struct { + // Correlated is false when no retained correlation matched either key; + // the notification is then foreign or past its retention. + Correlated bool + // Duplicate is true when this provider event id was already accounted; + // evidence was not moved again. RepairNeeded is still reported, so a + // retry that failed after the accounting commit can still finish. + Duplicate bool + // AccountRef is the opaque source account when it still exists; empty + // after account deletion and for non-customer purposes. + AccountRef string + // RepairNeeded lists the suppressions this event proves, for recipients + // that matched the authorized envelope, while the account exists. + RepairNeeded []FeedbackRepair +} + +// FeedbackProcessor is the deletion-resistant accounting seam the consumer +// calls BEFORE it looks for a live message: it must succeed (or fail, so the +// provider retries) without any message, agent, or user row surviving. +// Implemented by the sending-policy module. +type FeedbackProcessor interface { + ProcessProviderFeedback(context.Context, ProviderFeedback) (FeedbackResult, error) + // RepairSuppressionsTx writes the account-wide suppressions a signed + // event proved, for the case where no live message row can own them + // (the message was purged, or belongs to another deployment), inside the + // caller's transaction so the rows and the events announcing them commit + // together. It returns only the repairs that INSERTED a row: an address + // already suppressed (manually, or by an earlier bounce) is refreshed but + // not reported, so nothing re-announces an existing suppression. The live + // path keeps ownership whenever a message survives, so this never + // competes with it. + RepairSuppressionsTx(ctx context.Context, tx pgx.Tx, accountRef string, repairs []FeedbackRepair) (inserted []FeedbackRepair, err error) +} diff --git a/internal/delivery/feedback_seam_test.go b/internal/delivery/feedback_seam_test.go new file mode 100644 index 000000000..cfd1b3cb9 --- /dev/null +++ b/internal/delivery/feedback_seam_test.go @@ -0,0 +1,346 @@ +package delivery + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/jackc/pgx/v5" + + "github.com/tokencanopy/e2a/internal/eventpayload" +) + +type fakeProcessor struct { + calls []ProviderFeedback + repairs [][]FeedbackRepair + // existing is the set of addresses already suppressed for the account: + // a repair for one of them refreshes the row but is not an insert. + existing map[string]bool + result FeedbackResult + err error + // order records "processor" / "correlate" so the test can pin that + // accounting runs before any live-message lookup. + order *[]string +} + +func (p *fakeProcessor) ProcessProviderFeedback(_ context.Context, fb ProviderFeedback) (FeedbackResult, error) { + p.calls = append(p.calls, fb) + if p.order != nil { + *p.order = append(*p.order, "processor") + } + return p.result, p.err +} + +func (p *fakeProcessor) RepairSuppressionsTx(_ context.Context, _ pgx.Tx, _ string, repairs []FeedbackRepair) ([]FeedbackRepair, error) { + p.repairs = append(p.repairs, repairs) + if p.existing == nil { + p.existing = map[string]bool{} + } + var inserted []FeedbackRepair + for _, r := range repairs { + if !p.existing[r.Address] { + p.existing[r.Address] = true + inserted = append(inserted, r) + } + } + return inserted, nil +} + +type orderingStore struct { + *fakeConsumerStore + order *[]string + // failTxOnce makes the first live transaction fail, the two-phase + // window an SNS retry has to recover from. + failTxOnce bool + txCalls int + // rollback discards whatever the failed transaction's firer recorded. + rollback func() +} + +func (s *orderingStore) CorrelateBySESMessageID(ctx context.Context, id string) (*CorrelatedMessage, bool, error) { + if s.order != nil { + *s.order = append(*s.order, "correlate") + } + return s.fakeConsumerStore.CorrelateBySESMessageID(ctx, id) +} + +func (s *orderingStore) WithTx(ctx context.Context, fn func(tx pgx.Tx) error) error { + s.txCalls++ + // Mirror a real rollback: the body runs, then every write it made — + // suppression rows AND the outbox events it fired — is discarded. That + // fidelity is the whole point: the retry must be able to redo them. + before := make(map[string]bool, len(s.suppressed)) + for k, v := range s.suppressed { + before[k] = v + } + err := fn(nil) + if s.failTxOnce && s.txCalls == 1 { + s.fakeConsumerStore.suppressed = before + if s.rollback != nil { + s.rollback() + } + return errors.New("live transaction failed") + } + return err +} + +func bounceEvent(id, ses string) *Event { + return &Event{ + Kind: KindBounce, SESMessageID: ses, ProviderEventID: id, OccurredAt: time.Date(2026, 9, 7, 12, 0, 0, 0, time.UTC), + BounceType: "permanent", BounceSubType: "General", AttemptCorrelationID: "cor_0123abcd", + Recipients: []RecipientOutcome{{Address: "Bob@Example.test", Status: StatusBounced, Detail: "550", Suppress: true}}, + } +} + +// TestConsumerRunsAccountingBeforeLiveLookupAndForUncorrelated: the +// deletion-resistant seam sees every notification first — including one +// whose message is gone — and receives the retained subtypes, the attempt +// marker, and the normalized recipients. +func TestConsumerRunsAccountingBeforeLiveLookupAndForUncorrelated(t *testing.T) { + var order []string + st := &orderingStore{fakeConsumerStore: newFakeConsumerStore(), order: &order} + p := &fakeProcessor{order: &order, result: FeedbackResult{Correlated: true}} + c := NewConsumer(st, nil).WithFeedbackProcessor(p) + + if err := c.Process(context.Background(), bounceEvent("evt-1", "ses-unknown")); err != nil { + t.Fatalf("Process: %v", err) + } + if len(order) < 2 || order[0] != "processor" || order[1] != "correlate" { + t.Fatalf("order = %v, want accounting before the live lookup", order) + } + if len(p.calls) != 1 { + t.Fatalf("processor calls = %d, want 1 even though the message is unknown", len(p.calls)) + } + fb := p.calls[0] + if fb.ProviderEventID != "evt-1" || fb.Kind != KindBounce || fb.BounceType != "permanent" || fb.BounceSubType != "General" || + fb.ProviderMessageID != "ses-unknown" || fb.AttemptCorrelationID != "cor_0123abcd" || + len(fb.Recipients) != 1 || fb.Recipients[0] != "bob@example.test" { + t.Fatalf("processor input = %+v", fb) + } +} + +// TestConsumerProcessorErrorMakesProviderRetry: accounting failure is the +// consumer's failure; the notification must not be acked. +func TestConsumerProcessorErrorMakesProviderRetry(t *testing.T) { + st := newFakeConsumerStore() + st.corr["ses-1"] = &CorrelatedMessage{MessageID: "msg_1", UserID: "usr_1", AgentID: "agt_1"} + p := &fakeProcessor{err: errors.New("ledger unavailable")} + c := NewConsumer(st, nil).WithFeedbackProcessor(p) + if err := c.Process(context.Background(), bounceEvent("evt-2", "ses-1")); err == nil { + t.Fatal("a failing processor must fail Process so the provider retries") + } + if len(st.outcomes) != 0 { + t.Fatalf("lifecycle path ran despite accounting failure: %v", st.outcomes) + } +} + +// TestLiveMessageOwnsTheSuppression: when a message survives, the store +// writes the suppression inside the live transaction with the source +// message id and the provider diagnostic, and the seam's reported repair is +// NOT applied separately — one writer, one announcement. +func TestLiveMessageOwnsTheSuppression(t *testing.T) { + st := newFakeConsumerStore() + st.corr["ses-1"] = &CorrelatedMessage{MessageID: "msg_1", UserID: "usr_1", AgentID: "agt_1"} + fire, events := recordingFirer() + p := &fakeProcessor{result: FeedbackResult{Correlated: true, AccountRef: "usr_1", + RepairNeeded: []FeedbackRepair{{Address: "bob@example.test", Source: "bounce", Reason: "bounce:General"}}}} + c := NewConsumer(st, fire).WithFeedbackProcessor(p) + if err := c.Process(context.Background(), bounceEvent("evt-3", "ses-1")); err != nil { + t.Fatalf("Process: %v", err) + } + if !st.suppressed["usr_1|bob@example.test"] { + t.Fatal("the live path must write the suppression") + } + if len(p.repairs) != 0 { + t.Fatalf("the seam must not repair while a message owns the row: %v", p.repairs) + } + got := 0 + for _, e := range *events { + if e.eventType == EventSuppressionAdded { + got++ + } + } + if got != 1 { + t.Fatalf("suppression events = %d, want 1", got) + } +} + +// TestSuppressionSurvivesLiveTransactionFailureAndRetry is the regression +// this design exists for: the accounting seam commits on its own, the live +// transaction then fails, and the SNS retry must still end with exactly one +// suppression and exactly one suppression_added event. An earlier shape, +// where the seam wrote the row and the consumer trusted its insert verdict, +// lost the event forever on that retry. +func TestSuppressionSurvivesLiveTransactionFailureAndRetry(t *testing.T) { + base := newFakeConsumerStore() + base.corr["ses-1"] = &CorrelatedMessage{MessageID: "msg_1", UserID: "usr_1", AgentID: "agt_1"} + st := &orderingStore{fakeConsumerStore: base, failTxOnce: true} + fire, events := recordingFirer() + st.rollback = func() { *events = (*events)[:0] } + // The seam is a duplicate on the retry, exactly as the real one is. + p := &fakeProcessor{result: FeedbackResult{Correlated: true, AccountRef: "usr_1", + RepairNeeded: []FeedbackRepair{{Address: "bob@example.test", Source: "bounce", Reason: "bounce:General"}}}} + c := NewConsumer(st, fire).WithFeedbackProcessor(p) + + if err := c.Process(context.Background(), bounceEvent("evt-4", "ses-1")); err == nil { + t.Fatal("a failing live transaction must fail Process so SNS retries") + } + p.result.Duplicate = true + if err := c.Process(context.Background(), bounceEvent("evt-4", "ses-1")); err != nil { + t.Fatalf("retry: %v", err) + } + got := 0 + for _, e := range *events { + if e.eventType == EventSuppressionAdded { + got++ + } + } + if got != 1 { + t.Fatalf("suppression events across the retry = %d, want exactly 1", got) + } + if !st.suppressed["usr_1|bob@example.test"] { + t.Fatal("the retry must leave the suppression in place") + } +} + +// TestPurgedMessageRepairsThroughTheSeam: with no live message to own the +// row, the account-wide repair is applied through the seam instead and +// announced without a message id. +func TestPurgedMessageRepairsThroughTheSeam(t *testing.T) { + st := newFakeConsumerStore() // no correlation → message is gone + fire, events := recordingFirer() + p := &fakeProcessor{result: FeedbackResult{Correlated: true, AccountRef: "usr_1", + RepairNeeded: []FeedbackRepair{{Address: "bob@example.test", Source: "bounce", Reason: "bounce:General"}}}} + c := NewConsumer(st, fire).WithFeedbackProcessor(p) + if err := c.Process(context.Background(), bounceEvent("evt-5", "ses-gone")); err != nil { + t.Fatalf("Process: %v", err) + } + if len(p.repairs) != 1 || len(p.repairs[0]) != 1 || p.repairs[0][0].Address != "bob@example.test" { + t.Fatalf("repairs = %v, want the one address", p.repairs) + } + // It announces: a row appearing in the customer's suppression list with + // no event is the state/notification desync the message-backed path + // exists to avoid. The payload carries no message id, which the event + // schema documents as present only when still known. + if len(*events) != 1 || (*events)[0].eventType != EventSuppressionAdded { + t.Fatalf("a repaired suppression must be announced, got %v", *events) + } + if (*events)[0].messageID != "" { + t.Fatalf("a repair has no message to reference, got %q", (*events)[0].messageID) + } + if (*events)[0].userID != "usr_1" { + t.Fatalf("event user = %q, want the account the seam resolved", (*events)[0].userID) + } +} + +// TestParseRetainsSubtypesAndAttemptMarker: complaintSubType and the +// attempt header survive parsing; a malformed marker is dropped, not used. +func TestParseRetainsSubtypesAndAttemptMarker(t *testing.T) { + body := `{"eventType":"Complaint","mail":{"messageId":"ses-9","headers":[ + {"name":"x-e2a-provider-attempt","value":" cor_00ff11aa "}, + {"name":"X-E2A-Message-ID","value":"msg_abc"}]}, + "complaint":{"complainedRecipients":[{"emailAddress":"D@x.com"}],"complaintFeedbackType":"abuse","complaintSubType":"OnAccountSuppressionList"}}` + ev, err := ParseSESNotification([]byte(body)) + if err != nil { + t.Fatal(err) + } + if ev.ComplaintSubType != "OnAccountSuppressionList" || ev.AttemptCorrelationID != "cor_00ff11aa" || ev.E2AMessageID != "msg_abc" { + t.Fatalf("parsed = %+v", ev) + } + fb := ev.FeedbackFor() + if fb.ComplaintSubType != "OnAccountSuppressionList" || len(fb.Recipients) != 1 || fb.Recipients[0] != "d@x.com" { + t.Fatalf("FeedbackFor = %+v", fb) + } + + bad := `{"eventType":"Delivery","mail":{"messageId":"ses-10","headers":[{"name":"X-E2A-Provider-Attempt","value":"cor_NOTHEX; rm"}]},"delivery":{"recipients":["a@x.com","A@x.com"]}}` + ev, err = ParseSESNotification([]byte(bad)) + if err != nil { + t.Fatal(err) + } + if ev.AttemptCorrelationID != "" { + t.Fatalf("malformed marker must be dropped, got %q", ev.AttemptCorrelationID) + } + if fb := ev.FeedbackFor(); len(fb.Recipients) != 1 { + t.Fatalf("recipients must be deduplicated after normalization: %v", fb.Recipients) + } +} + +func countSuppressionEvents(events []firedEvent) int { + n := 0 + for _, e := range events { + if e.eventType == EventSuppressionAdded { + n++ + } + } + return n +} + +// TestRepairAnnouncesOnlyInsertedRows: a repair whose address is already +// suppressed (a manual entry, an earlier bounce) refreshes the row but must +// not fire suppression.added — only a genuinely new row is news. +func TestRepairAnnouncesOnlyInsertedRows(t *testing.T) { + st := newFakeConsumerStore() // message gone + fire, events := recordingFirer() + p := &fakeProcessor{ + existing: map[string]bool{"manual@example.test": true}, + result: FeedbackResult{Correlated: true, AccountRef: "usr_1", RepairNeeded: []FeedbackRepair{ + {Address: "manual@example.test", Source: "bounce", Reason: "bounce:General"}, + {Address: "new@example.test", Source: "bounce", Reason: "bounce:General"}, + }}, + } + c := NewConsumer(st, fire).WithFeedbackProcessor(p) + if err := c.Process(context.Background(), bounceEvent("evt-ins", "ses-gone")); err != nil { + t.Fatalf("Process: %v", err) + } + if got := countSuppressionEvents(*events); got != 1 { + t.Fatalf("suppression events = %d, want 1 (the inserted row only)", got) + } + if (*events)[0].data.(eventpayload.DomainSuppressionAddedData).Address != "new@example.test" { + t.Fatalf("announced %+v, want the new address", (*events)[0]) + } + // A redelivery repairs nothing new and announces nothing. + p.result.Duplicate = true + if err := c.Process(context.Background(), bounceEvent("evt-ins", "ses-gone")); err != nil { + t.Fatalf("redelivery: %v", err) + } + if got := countSuppressionEvents(*events); got != 1 { + t.Fatalf("suppression events after redelivery = %d, want still 1", got) + } +} + +// TestFoundMessagePurgedBeforeLockFallsBackToProvenanceRepair: the message +// correlated before the transaction but its agent was purged before the +// lock. The live path can no longer own the row, so the provenance repair +// must write it instead of silently dropping the suppression. +func TestFoundMessagePurgedBeforeLockFallsBackToProvenanceRepair(t *testing.T) { + for _, tc := range []struct { + name string + setup func(*fakeConsumerStore) + }{ + {"agent purged", func(s *fakeConsumerStore) { s.agentGone = true }}, + {"recipients purged", func(s *fakeConsumerStore) { s.applicable = false }}, + } { + t.Run(tc.name, func(t *testing.T) { + st := newFakeConsumerStore() + st.corr["ses-1"] = &CorrelatedMessage{MessageID: "msg_1", UserID: "usr_1", AgentID: "agt_1"} + tc.setup(st) + fire, events := recordingFirer() + p := &fakeProcessor{result: FeedbackResult{Correlated: true, AccountRef: "usr_1", + RepairNeeded: []FeedbackRepair{{Address: "bob@example.test", Source: "bounce", Reason: "bounce:General"}}}} + c := NewConsumer(st, fire).WithFeedbackProcessor(p) + if err := c.Process(context.Background(), bounceEvent("evt-race", "ses-1")); err != nil { + t.Fatalf("Process: %v", err) + } + if len(p.repairs) != 1 || p.repairs[0][0].Address != "bob@example.test" { + t.Fatalf("repairs = %v, want the provenance repair", p.repairs) + } + if st.suppressed["usr_1|bob@example.test"] { + t.Fatal("the live path must not have written the row") + } + if got := countSuppressionEvents(*events); got != 1 { + t.Fatalf("suppression events = %d, want 1", got) + } + }) + } +} diff --git a/internal/delivery/integration_test.go b/internal/delivery/integration_test.go index d87c797a1..6194bd81b 100644 --- a/internal/delivery/integration_test.go +++ b/internal/delivery/integration_test.go @@ -107,10 +107,11 @@ func TestConsumerCausalSuppressionLifecycleAndEventParity(t *testing.T) { detail string feedbackReason messagelifecycle.ReasonCode suppressionReason messagelifecycle.ReasonCode + suppressionSource string eventType string }{ - {"hard bounce", delivery.KindBounce, delivery.StatusBounced, "permanent", "550 5.1.1 no such user", messagelifecycle.ReasonDeliveryPermanentBounce, messagelifecycle.ReasonSuppressionHardBounceApplied, delivery.EventEmailBounced}, - {"complaint", delivery.KindComplaint, delivery.StatusComplained, "", "abuse", messagelifecycle.ReasonComplaintRecipientReported, messagelifecycle.ReasonSuppressionComplaintApplied, delivery.EventEmailComplained}, + {"hard bounce", delivery.KindBounce, delivery.StatusBounced, "permanent", "550 5.1.1 no such user", messagelifecycle.ReasonDeliveryPermanentBounce, messagelifecycle.ReasonSuppressionHardBounceApplied, "bounce", delivery.EventEmailBounced}, + {"complaint", delivery.KindComplaint, delivery.StatusComplained, "", "abuse", messagelifecycle.ReasonComplaintRecipientReported, messagelifecycle.ReasonSuppressionComplaintApplied, "complaint", delivery.EventEmailComplained}, } for i, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -147,6 +148,20 @@ func TestConsumerCausalSuppressionLifecycleAndEventParity(t *testing.T) { if suppressions != 1 { t.Fatalf("suppressions=%d want 1", suppressions) } + // The stored reason is the PROVIDER DIAGNOSTIC, not a derived + // label: it is a public field of GET /v1/account/suppressions, + // and an accounting layer that quietly rewrote it would change + // what every existing integration reads. + var storedReason, storedSource string + if err := pool.QueryRow(ctx, `SELECT reason, source FROM suppressions WHERE user_id=$1 AND address=$2`, userID, recipient).Scan(&storedReason, &storedSource); err != nil { + t.Fatal(err) + } + if storedReason != tc.detail { + t.Fatalf("suppression reason = %q, want the provider diagnostic %q", storedReason, tc.detail) + } + if storedSource != tc.suppressionSource { + t.Fatalf("suppression source = %q, want %q", storedSource, tc.suppressionSource) + } var lifecycleCount int if err := pool.QueryRow(ctx, `SELECT count(*) FROM message_lifecycle_transitions WHERE message_id=$1 AND reason_code=ANY($2)`, messageID, []string{string(tc.feedbackReason), string(tc.suppressionReason)}).Scan(&lifecycleCount); err != nil { t.Fatal(err) diff --git a/internal/delivery/ses.go b/internal/delivery/ses.go index ef9d9a183..ba70050f3 100644 --- a/internal/delivery/ses.go +++ b/internal/delivery/ses.go @@ -3,6 +3,8 @@ package delivery import ( "encoding/json" "fmt" + "net/mail" + "sort" "strings" "time" ) @@ -74,6 +76,13 @@ type Event struct { // bounceSubType (e.g. General, NoEmail, MailboxFull). BounceType string BounceSubType string + // ComplaintSubType is the raw SES complaintSubType (Complaint events + // only; empty otherwise). A suppression-list value means SES did not + // deliver, which the detector must not count as a genuine complaint. + ComplaintSubType string + // AttemptCorrelationID is the ProviderAttemptHeader SES echoed back from + // the submitted headers: the deletion-resistant correlation fallback. + AttemptCorrelationID string } // sesNotification is the SES event JSON carried in the SNS Message field. @@ -108,6 +117,7 @@ type sesNotification struct { EmailAddress string `json:"emailAddress"` } `json:"complainedRecipients"` ComplaintFeedbackType string `json:"complaintFeedbackType"` + ComplaintSubType string `json:"complaintSubType"` } `json:"complaint"` Delivery *struct { Recipients []string `json:"recipients"` @@ -141,11 +151,15 @@ func ParseSESNotification(messageBody []byte) (*Event, error) { } ev := &Event{SESMessageID: n.Mail.MessageID} for _, h := range n.Mail.Headers { - if strings.EqualFold(h.Name, MessageIDHeader) { + switch { + case strings.EqualFold(h.Name, MessageIDHeader) && ev.E2AMessageID == "": // Defensive trim: the marker is stamped bare, but tolerate an // angle-bracketed echo. ev.E2AMessageID = strings.Trim(strings.TrimSpace(h.Value), "<>") - break + case strings.EqualFold(h.Name, ProviderAttemptHeader) && ev.AttemptCorrelationID == "": + if v := strings.TrimSpace(h.Value); validAttemptCorrelationID(v) { + ev.AttemptCorrelationID = v + } } } @@ -178,6 +192,7 @@ func ParseSESNotification(messageBody []byte) (*Event, error) { case "Complaint": ev.Kind = KindComplaint if n.Complaint != nil { + ev.ComplaintSubType = n.Complaint.ComplaintSubType for _, r := range n.Complaint.ComplainedRecipients { ev.Recipients = append(ev.Recipients, RecipientOutcome{ Address: norm(r.EmailAddress), Status: StatusComplained, @@ -217,7 +232,48 @@ func ParseSESNotification(messageBody []byte) (*Event, error) { return ev, nil } -func norm(addr string) string { return strings.ToLower(strings.TrimSpace(addr)) } +// norm reduces a provider-reported address to the bare, lower-cased +// addr-spec the rest of the system stores and signs. +// +// It is deliberately the ONE normalizer for provider feedback: the detector +// matches a recipient against the HMAC of the address authorization signed, +// while the live path writes the same address into the customer's +// suppression list. If the two disagreed, a decorated value would credit +// the detector under the real address while suppressing a literal +// "Bob " the customer can never send to and never clear. +// +// SES reports a bounced recipient from the DSN's Final-Recipient field, +// which per RFC 3464 may carry an address-type prefix ("rfc822; bob@x.test") +// and, in the wild, angle brackets or a display name. +func norm(addr string) string { + addr = strings.TrimSpace(addr) + if semi := strings.IndexByte(addr, ';'); semi >= 0 { + if prefix := strings.TrimSpace(addr[:semi]); isAddressType(prefix) { + addr = strings.TrimSpace(addr[semi+1:]) + } + } + if parsed, err := mail.ParseAddress(addr); err == nil { + return strings.ToLower(strings.TrimSpace(parsed.Address)) + } + return strings.ToLower(strings.Trim(addr, "<>")) +} + +// isAddressType reports whether s is a DSN address-type token (letters, +// digits and dashes) rather than part of an address. Only such a prefix is +// stripped, so a local part containing a semicolon is left alone. +func isAddressType(s string) bool { + if s == "" { + return false + } + for _, r := range s { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '-': + default: + return false + } + } + return true +} // maxE2AMessageIDLen bounds a plausible e2a message id ("msg_" + 32 hex chars // today; headroom for future id shapes without accepting arbitrary strings). @@ -243,6 +299,55 @@ func validE2AMessageID(s string) bool { return true } +// validAttemptCorrelationID reports whether s is shaped like the attempt +// marker the adapter stamps (`cor_` + hex). Same rationale as +// validE2AMessageID: the value came off a signed notification but originated +// in a header block, so it is shape-checked before it becomes a lookup key. +func validAttemptCorrelationID(s string) bool { + const prefix = "cor_" + if !strings.HasPrefix(s, prefix) || len(s) <= len(prefix) || len(s) > maxE2AMessageIDLen { + return false + } + for _, r := range s[len(prefix):] { + switch { + case r >= 'a' && r <= 'f', r >= '0' && r <= '9': + default: + return false + } + } + return true +} + +// FeedbackFor reduces a parsed event to the processor's input: every +// recipient the notification named, normalized and deduplicated, plus the +// two correlation keys and the retained subtypes. +func (ev *Event) FeedbackFor() ProviderFeedback { + seen := map[string]bool{} + var recipients []string + for _, r := range ev.Recipients { + a := norm(r.Address) + if a == "" || seen[a] { + continue + } + seen[a] = true + recipients = append(recipients, a) + } + // Deterministic order, so a failure is reproducible and two runs over + // the same event report repairs in the same sequence. + sort.Strings(recipients) + return ProviderFeedback{ + ProviderEventID: ev.ProviderEventID, + OccurredAt: ev.OccurredAt, + Kind: ev.Kind, + BounceType: ev.BounceType, + BounceSubType: ev.BounceSubType, + ComplaintSubType: ev.ComplaintSubType, + ProviderMessageID: ev.SESMessageID, + AttemptCorrelationID: ev.AttemptCorrelationID, + Recipients: recipients, + } +} + // normalizeBounceType maps SES's bounceType (Permanent | Transient | // Undetermined, case per SES docs) to the stable event vocabulary. Anything // unrecognized — including a missing value — is "undetermined", so diff --git a/internal/httpapi/contacts_import.go b/internal/httpapi/contacts_import.go index b03c0d75b..0d2223d50 100644 --- a/internal/httpapi/contacts_import.go +++ b/internal/httpapi/contacts_import.go @@ -363,7 +363,10 @@ func (s *Server) markSuppressedImportRows(ctx context.Context, userID, agentID s } blockedSet := make(map[string]struct{}, len(blocked)) for _, a := range blocked { - blockedSet[identity.NormalizeMailboxAddress(a)] = struct{}{} + // The stored spelling may be the other IDNA form of the imported one. + for _, f := range identity.SuppressionLookupForms(identity.NormalizeMailboxAddress(a)) { + blockedSet[f] = struct{}{} + } } for i := range items { if _, ok := blockedSet[items[i].Address]; ok { diff --git a/internal/identity/account_trash.go b/internal/identity/account_trash.go index d30997994..3c6cf2678 100644 --- a/internal/identity/account_trash.go +++ b/internal/identity/account_trash.go @@ -707,6 +707,13 @@ func (s *Store) purgeAccount(ctx context.Context, userID string, force bool, per } } + // The post-deletion feedback horizon, resolved now from the effective + // sending policy (not cached at boot), before the seal takes its locks. + feedbackRetention, err := s.resolveFeedbackRetention(ctx) + if err != nil { + return false, fmt.Errorf("purge: %w", err) + } + // Seal: the remaining account rows and the user itself. err = s.WithTx(ctx, func(tx pgx.Tx) error { var locked string @@ -732,6 +739,32 @@ func (s *Store) purgeAccount(ctx context.Context, userID string, force bool, per if _, err := tx.Exec(ctx, `DELETE FROM usage_events WHERE user_id = $1`, userID); err != nil { return fmt.Errorf("purge: usage_events: %w", err) } + + // Deletion-resistant feedback provenance (B8): the account is + // genuinely gone at this seal step — either an immediate permanent + // erase, or a trashed account's retention window expiring — so this + // is where the post-deletion horizon is stamped on its retained + // correlations/events (which carry no FK and no plaintext recipient, + // so a controlled recipient cannot erase a complaint by deleting the + // account first). TrashAccount deliberately does NOT stamp this: a + // restore must not resurrect rows whose retention countdown already + // started, and the trash window itself is not the deletion event. + if _, err := tx.Exec(ctx, ` + UPDATE sending_feedback_correlations + SET expires_at = $2 + WHERE source_account_ref = $1 AND expires_at IS NULL`, + userID, time.Now().UTC().Add(feedbackRetention)); err != nil { + return fmt.Errorf("purge: stamp feedback retention: %w", err) + } + if _, err := tx.Exec(ctx, ` + UPDATE sending_feedback_events e + SET expires_at = c.expires_at + FROM sending_feedback_correlations c + WHERE c.correlation_id = e.correlation_id AND c.source_account_ref = $1 AND e.expires_at IS NULL`, + userID); err != nil { + return fmt.Errorf("purge: stamp feedback event retention: %w", err) + } + // A domain other accounts' agents live on (the shared domain a probe // account adopted) cannot cascade away with this user — the agents' // FK is ON DELETE NO ACTION and the purge would wedge forever. Hand it diff --git a/internal/identity/account_trash_test.go b/internal/identity/account_trash_test.go index a92e0c89c..964ba7eb1 100644 --- a/internal/identity/account_trash_test.go +++ b/internal/identity/account_trash_test.go @@ -839,6 +839,102 @@ func TestSummaryCountsSurviveMessageErasureThroughUsageEvents(t *testing.T) { } } +// TestEraseAccountStampsFeedbackRetention: the real account-deletion path +// (identity.EraseAccount, DELETE /v1/account?permanent=true) trashes then +// immediately purges the account in one call — the purge's seal transaction +// is where a retained correlation and its events pick up the post-deletion +// horizon instead of being removed, since they carry no FK and no plaintext +// recipient (B8). TrashAccount alone (no purge) must NOT stamp it: a restore +// must not resurrect rows whose retention countdown already started. +func TestEraseAccountStampsFeedbackRetention(t *testing.T) { + pool := testutil.TestDB(t) + store := identity.NewStore(pool) + ctx := context.Background() + store.SetFeedbackRetention(10 * 24 * time.Hour) + + user, err := store.CreateOrGetUser(ctx, "feedbackerase@example.test", "F", "sub-feedbackerase") + if err != nil { + t.Fatal(err) + } + other, err := store.CreateOrGetUser(ctx, "feedbackerase-other@example.test", "O", "sub-feedbackerase-other") + if err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, ` + INSERT INTO sending_feedback_correlations (correlation_id, operation_id, submission_attempt, source_account_ref, policy_subject_ref, purpose, shared_reputation, tenant_mode) + VALUES ('cor_erase_1', 'msg_erase_1', 1, $1, $1, 'customer_message', true, 'none'), + ('cor_erase_other', 'msg_erase_2', 1, $2, $2, 'customer_message', true, 'none')`, + user.ID, other.ID); err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, ` + INSERT INTO sending_feedback_events (provider_event_id, correlation_id, provider_occurred_at) + VALUES ('evt_erase_1', 'cor_erase_1', now()), ('evt_erase_other', 'cor_erase_other', now())`); err != nil { + t.Fatal(err) + } + + if _, err := store.EraseAccount(ctx, user.ID, nil); err != nil { + t.Fatalf("EraseAccount: %v", err) + } + + var corrExpiry, evtExpiry *time.Time + if err := pool.QueryRow(ctx, `SELECT expires_at FROM sending_feedback_correlations WHERE correlation_id = 'cor_erase_1'`).Scan(&corrExpiry); err != nil { + t.Fatalf("correlation must survive account erasure: %v", err) + } + if err := pool.QueryRow(ctx, `SELECT expires_at FROM sending_feedback_events WHERE provider_event_id = 'evt_erase_1'`).Scan(&evtExpiry); err != nil { + t.Fatalf("event must survive account erasure: %v", err) + } + lo, hi := time.Now().Add(9*24*time.Hour), time.Now().Add(11*24*time.Hour) + if corrExpiry == nil || corrExpiry.Before(lo) || corrExpiry.After(hi) { + t.Fatalf("correlation expiry = %v, want ~10 days out", corrExpiry) + } + if evtExpiry == nil || !evtExpiry.Equal(*corrExpiry) { + t.Fatalf("event expiry = %v, want the correlation's %v", evtExpiry, corrExpiry) + } + + var otherExpiry *time.Time + if err := pool.QueryRow(ctx, `SELECT expires_at FROM sending_feedback_correlations WHERE correlation_id = 'cor_erase_other'`).Scan(&otherExpiry); err != nil { + t.Fatal(err) + } + if otherExpiry != nil { + t.Fatalf("another account's provenance was stamped: %v", otherExpiry) + } +} + +// TestTrashAccountDoesNotStampFeedbackRetention: trashing alone (no purge) +// must leave retained provenance's expires_at NULL — the account is still +// restorable, so starting the deletion clock here would let a restore bring +// back an account whose feedback evidence is already counting down to purge. +func TestTrashAccountDoesNotStampFeedbackRetention(t *testing.T) { + pool := testutil.TestDB(t) + store := identity.NewStore(pool) + ctx := context.Background() + store.SetFeedbackRetention(10 * 24 * time.Hour) + + user, err := store.CreateOrGetUser(ctx, "feedbacktrash@example.test", "T", "sub-feedbacktrash") + if err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, ` + INSERT INTO sending_feedback_correlations (correlation_id, operation_id, submission_attempt, source_account_ref, policy_subject_ref, purpose, shared_reputation, tenant_mode) + VALUES ('cor_trash_1', 'msg_trash_1', 1, $1, $1, 'customer_message', true, 'none')`, + user.ID); err != nil { + t.Fatal(err) + } + + if _, err := store.TrashAccount(ctx, user.ID, nil); err != nil { + t.Fatalf("TrashAccount: %v", err) + } + + var expiry *time.Time + if err := pool.QueryRow(ctx, `SELECT expires_at FROM sending_feedback_correlations WHERE correlation_id = 'cor_trash_1'`).Scan(&expiry); err != nil { + t.Fatal(err) + } + if expiry != nil { + t.Fatalf("trashing alone must not start the feedback retention clock, got expires_at = %v", expiry) + } +} + func TestRecentDeletionTombstoneForAPlainErase(t *testing.T) { store, pool, _ := tombstoneStore(t) ctx := context.Background() diff --git a/internal/identity/agent_suppressions.go b/internal/identity/agent_suppressions.go index 223a02f97..a1eb3b2d2 100644 --- a/internal/identity/agent_suppressions.go +++ b/internal/identity/agent_suppressions.go @@ -175,7 +175,7 @@ func (s *Store) EffectiveSuppressions(ctx context.Context, userID, agentID strin UNION SELECT address FROM agent_suppressions WHERE user_id = $1 AND agent_id = $3 AND address = ANY($2)`, - userID, normalized, NormalizeEmail(agentID)) + userID, suppressionLookupSet(normalized), NormalizeEmail(agentID)) if err != nil { return nil, err } diff --git a/internal/identity/delivery_store.go b/internal/identity/delivery_store.go index 5f37f013c..9f943e08e 100644 --- a/internal/identity/delivery_store.go +++ b/internal/identity/delivery_store.go @@ -11,6 +11,7 @@ import ( "github.com/jackc/pgx/v5" "github.com/tokencanopy/e2a/internal/delivery" "github.com/tokencanopy/e2a/internal/messagelifecycle" + "github.com/tokencanopy/e2a/internal/suppressionsync" ) // OutboundSendClaimStaleWindow exceeds River's one-minute worker timeout and @@ -1111,24 +1112,16 @@ func (s *Store) AddSuppression(ctx context.Context, userID, address, reason, sou return added, err } +// AddSuppressionTx upserts through suppressionsync: an existing row keeps its +// reason and source but advances sync_generation and clears removal_pending, +// so a suppression re-proven by feedback defeats a stale pending removal. +// added reports a genuine insert. func (s *Store) AddSuppressionTx(ctx context.Context, tx pgx.Tx, userID, address, reason, source, sourceMessageID string) (string, bool, error) { - id := "supp_" + generateID() - tag, err := tx.Exec(ctx, - `INSERT INTO suppressions (id, user_id, address, reason, source, source_message_id) - VALUES ($1, $2, $3, $4, $5, $6) - ON CONFLICT (user_id, address) DO NOTHING`, - id, userID, NormalizeEmail(address), reason, source, nullIfEmpty(sourceMessageID), - ) + up, err := suppressionsync.UpsertTx(ctx, tx, "supp_"+generateID(), userID, NormalizeEmail(address), reason, source, sourceMessageID) if err != nil { return "", false, err } - if tag.RowsAffected() == 0 { - if err := tx.QueryRow(ctx, `SELECT id FROM suppressions WHERE user_id=$1 AND address=$2`, userID, NormalizeEmail(address)).Scan(&id); err != nil { - return "", false, err - } - return id, false, nil - } - return id, true, nil + return up.ID, up.Inserted, nil } // SuppressedAddresses returns the subset of addrs that are suppressed for the @@ -1143,7 +1136,7 @@ func (s *Store) SuppressedAddresses(ctx context.Context, userID string, addrs [] } rows, err := s.pool.Query(ctx, `SELECT address FROM suppressions WHERE user_id = $1 AND address = ANY($2)`, - userID, norm, + userID, suppressionLookupSet(norm), ) if err != nil { return nil, err diff --git a/internal/identity/email.go b/internal/identity/email.go index 55b2999b5..eab0a09e8 100644 --- a/internal/identity/email.go +++ b/internal/identity/email.go @@ -3,6 +3,8 @@ package identity import ( "net/mail" "strings" + + "golang.org/x/net/idna" ) // NormalizeEmail returns the canonical lookup form of an email address: @@ -57,3 +59,44 @@ func NormalizeMailboxAddress(value string) string { } return NormalizeEmail(value) } + +// SuppressionLookupForms returns every spelling under which a suppression of +// the normalized address may be stored: the address itself plus, for an +// internationalized domain, its A-label (punycode) and Unicode spellings. +// Provider feedback reports an IDN recipient in A-label form while a +// customer may type it in Unicode (or the reverse), and both kinds of row +// land in the same table — so a send-time lookup that compared only the +// typed spelling would let a bounced or complained Unicode address through. +// A domain the IDNA lookup profile refuses contributes only its raw form. +func SuppressionLookupForms(normalized string) []string { + forms := []string{normalized} + at := strings.LastIndexByte(normalized, '@') + if at <= 0 || at == len(normalized)-1 { + return forms + } + local, domain := normalized[:at+1], normalized[at+1:] + for _, conv := range []func(string) (string, error){idna.Lookup.ToASCII, idna.Lookup.ToUnicode} { + if d, err := conv(domain); err == nil && d != "" { + if f := local + strings.ToLower(d); f != forms[0] && (len(forms) < 2 || f != forms[1]) { + forms = append(forms, f) + } + } + } + return forms +} + +// suppressionLookupSet expands normalized addresses into every stored +// spelling SuppressionLookupForms names, deduplicated. +func suppressionLookupSet(normalized []string) []string { + seen := make(map[string]struct{}, len(normalized)) + out := make([]string, 0, len(normalized)) + for _, n := range normalized { + for _, f := range SuppressionLookupForms(n) { + if _, ok := seen[f]; !ok { + seen[f] = struct{}{} + out = append(out, f) + } + } + } + return out +} diff --git a/internal/identity/store.go b/internal/identity/store.go index 0f4629009..9bdfef4d5 100644 --- a/internal/identity/store.go +++ b/internal/identity/store.go @@ -593,6 +593,9 @@ type Store struct { // transactions (mode "trash" / "restore") — how the durable billing // notice is enqueued atomically with the transition. accountStateHook func(ctx context.Context, tx pgx.Tx, userID, mode string) error + // feedbackRetention resolves the post-deletion horizon for retained + // feedback provenance at purge time; nil means DefaultFeedbackRetention. + feedbackRetention func(context.Context) (time.Duration, error) } // SetAccountStateHook installs the in-transaction account-state hook (see diff --git a/internal/identity/suppression_forms_test.go b/internal/identity/suppression_forms_test.go new file mode 100644 index 000000000..71bf4dd58 --- /dev/null +++ b/internal/identity/suppression_forms_test.go @@ -0,0 +1,22 @@ +package identity_test + +import ( + "reflect" + "testing" + + "github.com/tokencanopy/e2a/internal/identity" +) + +func TestSuppressionLookupForms(t *testing.T) { + for in, want := range map[string][]string{ + "plain@example.test": {"plain@example.test"}, + "leser@bücher.example": {"leser@bücher.example", "leser@xn--bcher-kva.example"}, + "leser@xn--bcher-kva.example": {"leser@xn--bcher-kva.example", "leser@bücher.example"}, + "odd@my_host.example": {"odd@my_host.example"}, + "no-at-sign": {"no-at-sign"}, + } { + if got := identity.SuppressionLookupForms(in); !reflect.DeepEqual(got, want) { + t.Errorf("SuppressionLookupForms(%q) = %v, want %v", in, got, want) + } + } +} diff --git a/internal/identity/user_data_rights.go b/internal/identity/user_data_rights.go index 2620428d1..4aa2a6b6e 100644 --- a/internal/identity/user_data_rights.go +++ b/internal/identity/user_data_rights.go @@ -298,6 +298,13 @@ func (s *Store) DeleteUserData(ctx context.Context, userID string) (*DeleteUserD // delete. A nil hook is a plain account delete (dev / no SES). The DB FK // cascade still removes the domain rows; the hook only schedules the remote // SES cleanup that the cascade cannot do. +// +// TEST-ONLY: production account deletion goes DeleteUserDataCore → +// EraseAccount / TrashAccount, whose purge seal also stamps the retained +// feedback-provenance horizon; this path does not. Nothing outside tests +// calls DeleteUserData or DeleteUserDataTx. +// TODO(tokencanopy/e2a#1013): remove both, porting their tests to +// EraseAccount. func (s *Store) DeleteUserDataTx(ctx context.Context, userID string, perDomainInTx func(ctx context.Context, tx pgx.Tx, domain string) error) (*DeleteUserDataResult, error) { tx, err := s.pool.BeginTx(ctx, pgx.TxOptions{IsoLevel: pgx.Serializable}) if err != nil { @@ -657,3 +664,42 @@ func scanUsageEventsForUser(ctx context.Context, tx pgx.Tx, userID string) ([]Us } return out, rows.Err() } + +// DefaultFeedbackRetention is the post-account-deletion horizon for retained +// feedback provenance, matching the sending-protection policy default +// (sending_feedback_post_account_retention_days = 30). +const DefaultFeedbackRetention = 30 * 24 * time.Hour + +// SetFeedbackRetention pins a fixed post-deletion feedback horizon. Tests +// use it; production installs SetFeedbackRetentionResolver instead. +func (s *Store) SetFeedbackRetention(d time.Duration) { + if d > 0 { + s.feedbackRetention = func(context.Context) (time.Duration, error) { return d, nil } + } +} + +// SetFeedbackRetentionResolver installs the accessor the purge seal reads +// the post-deletion horizon from, AT PURGE TIME: the sending-policy +// module's effective policy, so a database-source deployment whose +// activated policy differs from the config file stamps the horizon the +// janitor and non-customer correlations use. +func (s *Store) SetFeedbackRetentionResolver(fn func(context.Context) (time.Duration, error)) { + s.feedbackRetention = fn +} + +// resolveFeedbackRetention returns the horizon the seal stamps. A resolver +// error fails the seal; the purge resumes on its next pass rather than +// stamping a horizon nobody configured. +func (s *Store) resolveFeedbackRetention(ctx context.Context) (time.Duration, error) { + if s.feedbackRetention == nil { + return DefaultFeedbackRetention, nil + } + d, err := s.feedbackRetention(ctx) + if err != nil { + return 0, fmt.Errorf("resolve feedback retention: %w", err) + } + if d <= 0 { + return 0, fmt.Errorf("resolve feedback retention: non-positive horizon %s", d) + } + return d, nil +} diff --git a/internal/outbound/provider_attempt_header_test.go b/internal/outbound/provider_attempt_header_test.go new file mode 100644 index 000000000..3666136a1 --- /dev/null +++ b/internal/outbound/provider_attempt_header_test.go @@ -0,0 +1,17 @@ +package outbound + +import ( + "testing" + + "github.com/tokencanopy/e2a/internal/delivery" +) + +// TestProviderAttemptHeaderNameMatchesDelivery: the adapter stamps the +// attempt marker under one name and the feedback parser looks for it under +// another constant (delivery cannot import this package). They must agree, +// or the deletion-resistant correlation fallback silently never matches. +func TestProviderAttemptHeaderNameMatchesDelivery(t *testing.T) { + if ProviderAttemptHeader != delivery.ProviderAttemptHeader { + t.Fatalf("outbound.ProviderAttemptHeader=%q delivery.ProviderAttemptHeader=%q", ProviderAttemptHeader, delivery.ProviderAttemptHeader) + } +} diff --git a/internal/sendingpolicy/feedback.go b/internal/sendingpolicy/feedback.go new file mode 100644 index 000000000..f634b11de --- /dev/null +++ b/internal/sendingpolicy/feedback.go @@ -0,0 +1,963 @@ +package sendingpolicy + +import ( + "context" + "errors" + "fmt" + "log" + "strings" + "sync/atomic" + "time" + + "github.com/jackc/pgx/v5" + "golang.org/x/net/idna" + + "github.com/tokencanopy/e2a/internal/delivery" + "github.com/tokencanopy/e2a/internal/suppressionsync" +) + +// Deletion-resistant feedback provenance (slice B8). +// +// Provider feedback arrives long after the message that caused it, and the +// message, its agent, or the whole account may be gone by then. The detector +// must still see it: an abuser cannot be allowed to erase a complaint by +// deleting the message it belongs to. So this path never reads messages, +// agent_identities, or users. It correlates against the retained +// sending_feedback_correlations row (by provider message id, then by the +// random attempt marker SES echoes back), matches each recipient against the +// keyed HMACs recorded at authorization, and moves per-recipient detector +// buckets with monotonic evidence into the account's daily aggregates. +// +// Suppression repair is the one customer-visible effect: while the account +// still exists, a hard bounce, a genuine complaint, or a suppression-list +// subtype recreates the account-wide suppression from the signed event's +// plaintext recipient. The retained rows themselves never store an address. + +// Bucket is the detector classification of one recipient's best evidence. +type Bucket string + +const ( + BucketNone Bucket = "none" + BucketDelivered Bucket = "delivered" + BucketTerminalOther Bucket = "terminal_other" + BucketHardBounce Bucket = "hard_bounce" + BucketComplaint Bucket = "complaint" +) + +// Rank is the deterministic evidence order none < delivered < terminal_other +// < hard_bounce < complaint. Higher evidence replaces lower; a delayed +// lower-ranked callback never erases an observed hard bounce or complaint. +func (b Bucket) Rank() int { + switch b { + case BucketDelivered: + return 1 + case BucketTerminalOther: + return 2 + case BucketHardBounce: + return 3 + case BucketComplaint: + return 4 + } + return 0 +} + +// column is the aggregate column a bucket adds to; empty for none. +func (b Bucket) column() string { + switch b { + case BucketDelivered: + return "delivered_count" + case BucketTerminalOther: + return "terminal_other_count" + case BucketHardBounce: + return "hard_bounce_count" + case BucketComplaint: + return "complaint_count" + } + return "" +} + +// Suppression-list subtypes SES emits when it did not attempt delivery. +// AWS documents these as not affecting sender reputation, so they are +// excluded from both numerator and denominator; the global-list subtype +// "Suppressed" is documented as affecting reputation and stays included. +const ( + subtypeOnAccountSuppressionList = "OnAccountSuppressionList" + subtypeOnTenantSuppressionList = "OnTenantSuppressionList" +) + +// isSuppressionListSubtype matches the two subtypes case-insensitively, the +// same way the bounce type is compared: a case variant from the provider +// must not turn an excluded suppression-list bounce into a hard bounce and +// inflate the numerator that pauses accounts. +func isSuppressionListSubtype(sub string) bool { + sub = strings.TrimSpace(sub) + return strings.EqualFold(sub, subtypeOnAccountSuppressionList) || + strings.EqualFold(sub, subtypeOnTenantSuppressionList) +} + +// Derivation is a bucket plus whether the event should repair the local +// suppression list and, if so, under which source. +type Derivation struct { + Bucket Bucket + // Repair is true when the event proves the address should be suppressed: + // an eligible hard bounce, a genuine complaint, or a suppression-list + // subtype (SES already refuses it; the local list must agree). + Repair bool + // Source is the suppressions.source the repair records. + Source string +} + +// DeriveBucket maps a full event kind and its retained subtypes to the +// detector bucket. Every terminal bucket contributes one denominator unit; +// only hard bounce and complaint contribute numerators. +func DeriveBucket(kind delivery.EventKind, bounceType, bounceSubType, complaintSubType string) Derivation { + switch kind { + case delivery.KindDelivery: + return Derivation{Bucket: BucketDelivered} + case delivery.KindBounce: + if isSuppressionListSubtype(bounceSubType) { + return Derivation{Bucket: BucketNone, Repair: true, Source: suppressionsync.SourceBounce} + } + if strings.EqualFold(strings.TrimSpace(bounceType), "permanent") { + return Derivation{Bucket: BucketHardBounce, Repair: true, Source: suppressionsync.SourceBounce} + } + // SES emits a bounce only once it has given up: a transient or + // undetermined bounce is terminal for this recipient, just not an + // eligible hard bounce. + return Derivation{Bucket: BucketTerminalOther} + case delivery.KindComplaint: + if isSuppressionListSubtype(complaintSubType) { + return Derivation{Bucket: BucketNone, Repair: true, Source: suppressionsync.SourceComplaint} + } + return Derivation{Bucket: BucketComplaint, Repair: true, Source: suppressionsync.SourceComplaint} + } + // Send, DeliveryDelay, Reject, Other: acceptance, non-terminal, or a + // submission verdict — none are delivery outcomes. + return Derivation{Bucket: BucketNone} +} + +// feedbackCorrelation is the retained row a notification is matched to. +type feedbackCorrelation struct { + id string + sourceAccount *string + purpose Purpose + shared bool + expiresAt *time.Time +} + +// feedbackRecipient is one retained recipient row, keyed by HMAC. +type feedbackRecipient struct { + mac []byte + keyVersion int + bucket Bucket + rank int + occurredAt *time.Time + eventID *string + epoch *int64 + day *time.Time +} + +// ProcessProviderFeedback implements delivery.FeedbackProcessor: it moves +// detector evidence for one signed notification and reports the account +// suppressions that notification proves. It deliberately writes NO +// suppression itself — the live message path owns that row when a message +// survives, and the consumer applies the repair through RepairSuppressionsTx +// only when none does. +func (m *Module) ProcessProviderFeedback(ctx context.Context, fb delivery.ProviderFeedback) (delivery.FeedbackResult, error) { + if strings.TrimSpace(fb.ProviderEventID) == "" { + return delivery.FeedbackResult{}, errors.New("sendingpolicy: provider event id is required") + } + if fb.OccurredAt.IsZero() { + return delivery.FeedbackResult{}, errors.New("sendingpolicy: provider event time is required") + } + + tx, err := m.pool.Begin(ctx) + if err != nil { + return delivery.FeedbackResult{}, fmt.Errorf("sendingpolicy: begin feedback: %w", err) + } + defer func() { _ = tx.Rollback(ctx) }() + + derived := DeriveBucket(fb.Kind, fb.BounceType, fb.BounceSubType, fb.ComplaintSubType) + + corr, found, err := lookupCorrelation(ctx, tx, fb.ProviderMessageID, fb.AttemptCorrelationID) + if err != nil { + return delivery.FeedbackResult{}, err + } + if !found { + expired := false + if validAttemptMarker(fb.AttemptCorrelationID) { + if expired, err = pastRetention(ctx, tx, fb.AttemptCorrelationID); err != nil { + return delivery.FeedbackResult{}, err + } + } + if validAttemptMarker(fb.AttemptCorrelationID) && !expired { + // e2a stamped this mail, yet no retained correlation answers + // it: a correlation GC'd early, a write that never landed, or a + // forged marker. Spec: count and alert, never treat as healthy. + // The marker value is deliberately not logged or labelled. + observeFeedback(FeedbackOutcomeUncorrelatedWithMarker, derived.Bucket) + log.Printf("[sendingpolicy:feedback] %s event %s carries the provider-attempt marker but matches no retained correlation", + fb.Kind, fb.ProviderEventID) + } else { + observeFeedback(FeedbackOutcomeUncorrelated, derived.Bucket) + } + return delivery.FeedbackResult{}, nil + } + result := delivery.FeedbackResult{Correlated: true} + // Metric samples are emitted only after commit: a rolled-back pass is + // retried by SNS and must not be counted twice. + var samples []feedbackSample + + // One row per provider event id. A redelivered notification does not + // move evidence again — though the evidence rules are themselves + // idempotent for a replay, since a replayed event can only ever tie its + // own rank. Repairs are still reported below, so a retry whose live + // half failed after this commit can still finish its work. + tag, err := tx.Exec(ctx, ` + INSERT INTO sending_feedback_events (provider_event_id, correlation_id, provider_occurred_at, expires_at) + VALUES ($1, $2, $3, $4) + ON CONFLICT (provider_event_id) DO NOTHING`, + fb.ProviderEventID, corr.id, fb.OccurredAt.UTC(), corr.expiresAt) + if err != nil { + return delivery.FeedbackResult{}, fmt.Errorf("sendingpolicy: record feedback event: %w", err) + } + firstTime := tag.RowsAffected() == 1 + result.Duplicate = !firstTime + if !firstTime { + samples = append(samples, feedbackSample{FeedbackOutcomeDuplicate, derived.Bucket}) + } + + // Lock the account control row when the account still exists: it holds + // the epoch the new evidence is assigned to, and the pause transition + // (B9) takes the same lock, so an epoch cannot move under this write. + // + // Lock order note: this transaction takes the control row and then the + // account's aggregate row (a users foreign key). Account deletion takes + // the users row first and cascades into the control row, so the two can + // deadlock. Postgres aborts one; this side is an SNS retry, and a + // delete that wins leaves the lookup below returning no rows, which is + // the provenance-only path. Deletion is rare and operator-initiated, so + // the exposure is one retried notification. + accountExists := false + var accountID string + var epoch int64 + if corr.purpose.isCustomer() && corr.sourceAccount != nil { + err := tx.QueryRow(ctx, + `SELECT user_id, outcome_epoch FROM account_sending_controls WHERE user_id = $1 FOR UPDATE`, + *corr.sourceAccount, + ).Scan(&accountID, &epoch) + switch { + case errors.Is(err, pgx.ErrNoRows): + // Account deleted: provenance only, never recreate customer state. + case err != nil: + return delivery.FeedbackResult{}, fmt.Errorf("sendingpolicy: lock account control: %w", err) + default: + accountExists = true + result.AccountRef = accountID + } + } + + rows, err := loadFeedbackRecipients(ctx, tx, corr.id) + if err != nil { + return delivery.FeedbackResult{}, err + } + // The ingestion day is "now", not the provider time: the spec assigns + // new evidence to the current epoch and UTC day so a delayed callback + // counts where the detector is looking. + now := m.now().UTC() + today := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, time.UTC) + + unmatched := 0 + for _, addr := range fb.Recipients { + addr = strings.ToLower(strings.TrimSpace(addr)) + if addr == "" { + continue + } + row, matched := m.matchRecipient(rows, addr) + if !matched { + // Not in the authorized envelope: no accounting, no repair. A + // mismatched recipient on a signed event is either a provider + // quirk or forged input; either way it proves nothing about an + // address this account sent to. Counted, never logged with the + // address itself. + unmatched++ + if firstTime { + samples = append(samples, feedbackSample{FeedbackOutcomeUnmatchedRecipient, derived.Bucket}) + } + continue + } + if firstTime { + bucket := derived.Bucket + if bucket.denominatorOnly() { + excluded, err := m.excludedFromDenominator(ctx, tx, corr, accountExists, addr) + if err != nil { + return delivery.FeedbackResult{}, err + } + if excluded { + bucket = BucketNone + } + } + if err := m.applyEvidence(ctx, tx, corr, row, bucket, fb, accountExists, accountID, epoch, today); err != nil { + return delivery.FeedbackResult{}, err + } + outcome := FeedbackOutcomeCorrelated + if corr.purpose.isCustomer() && corr.sourceAccount != nil && !accountExists { + outcome = FeedbackOutcomeDeadAccount + } + samples = append(samples, feedbackSample{outcome, bucket}) + } + // Repair is reported for CUSTOMER MESSAGES only. Platform mail this + // account merely triggered — an approval notice, a webhook health + // warning — is addressed to the account owner, and a bounce on it + // suppressing that address account-wide would block the customer's + // own sends to it: a new customer-visible effect the pre-B8 path + // never had. Those purposes still feed the detector above, which is + // what the spec requires of them. + if derived.Repair && accountExists && corr.purpose == PurposeCustomerMessage { + result.RepairNeeded = append(result.RepairNeeded, delivery.FeedbackRepair{ + Address: addr, Source: derived.Source, Reason: repairReason(fb, derived), + }) + } + } + if unmatched > 0 && firstTime { + // A systematic mismatch (a normalization divergence, a rotated key) + // would otherwise be invisible: the detector would simply see + // nothing and read as healthy. + log.Printf("[sendingpolicy:feedback] %d recipient(s) on event %s did not match correlation %s's authorized envelope", + unmatched, fb.ProviderEventID, corr.id) + } + + if err := tx.Commit(ctx); err != nil { + return delivery.FeedbackResult{}, fmt.Errorf("sendingpolicy: commit feedback: %w", err) + } + for _, sm := range samples { + observeFeedback(sm.outcome, sm.bucket) + } + return result, nil +} + +// Feedback ingestion outcomes, the bounded `outcome` label of +// e2a_sending_feedback_ingested_total. One sample per matched or unmatched +// recipient of a first-seen event, one per uncorrelated or duplicate event. +const ( + // FeedbackOutcomeCorrelated: a recipient matched its retained HMAC and + // its evidence was applied (to a live account, or to a non-customer + // purpose that has no account to aggregate into). + FeedbackOutcomeCorrelated = "correlated" + // FeedbackOutcomeDeadAccount: correlated, but the customer account is + // gone — provenance-only, no aggregate, no customer state. + FeedbackOutcomeDeadAccount = "dead_account" + // FeedbackOutcomeUnmatchedRecipient: the event correlated but this + // recipient is not in the authorized envelope (or its key is not held). + FeedbackOutcomeUnmatchedRecipient = "unmatched_recipient" + // FeedbackOutcomeUncorrelatedWithMarker: e2a's attempt marker is present + // but no retained correlation answers it. The alerting outcome. + FeedbackOutcomeUncorrelatedWithMarker = "uncorrelated_with_marker" + // FeedbackOutcomeUncorrelated: no marker and no provider-id match — + // mail from before B8, from another deployment on the topic, or past + // its retention. + FeedbackOutcomeUncorrelated = "uncorrelated" + // FeedbackOutcomeDuplicate: the provider event id was already recorded. + FeedbackOutcomeDuplicate = "duplicate" +) + +// FeedbackObserver receives one bounded (outcome, bucket) sample per +// ingestion result. Set once at startup by the composition root +// (telemetry.Metrics.SendingFeedbackIngested); nil = no-op. Never carries an +// address, account, correlation, or provider id. +type FeedbackObserver func(outcome, bucket string) + +var feedbackObserver atomic.Value // FeedbackObserver + +// SetFeedbackObserver installs the process-wide ingestion observer. +func SetFeedbackObserver(o FeedbackObserver) { feedbackObserver.Store(o) } + +func observeFeedback(outcome string, bucket Bucket) { + if o, ok := feedbackObserver.Load().(FeedbackObserver); ok && o != nil { + o(outcome, string(bucket)) + } +} + +type feedbackSample struct { + outcome string + bucket Bucket +} + +// validAttemptMarker mirrors the consumer's shape check: the parser already +// drops a malformed header, so any non-empty value here is e2a-shaped. +func validAttemptMarker(v string) bool { return strings.HasPrefix(strings.TrimSpace(v), "cor_") } + +// sesMailboxSimulatorDomain is SES's mailbox simulator. Mail to it never +// reaches a real inbox, so its deliveries prove nothing about a sender. +const sesMailboxSimulatorDomain = "simulator.amazonses.com" + +// WithFeedbackExcludedDomains configures the deployment's shared agent +// domains (config shared_domain): a delivery to an agent this deployment +// hosts is free for any sender to manufacture and must not dilute the +// detector's denominator. The SES mailbox simulator is always excluded. +// Domains are canonicalized (IDNA ASCII, lower case). +func (m *Module) WithFeedbackExcludedDomains(domains ...string) *Module { + set := map[string]struct{}{} + for _, d := range domains { + if d = canonicalDomain(d); d != "" { + set[d] = struct{}{} + } + } + m.feedbackExcludedDomains = set + return m +} + +// ExcludesFeedbackDomain reports whether deliveries to domain are excluded +// from the detector denominator by configuration (the simulator or a +// configured shared agent domain). The composition-root test reads it. +func (m *Module) ExcludesFeedbackDomain(domain string) bool { + d := canonicalDomain(domain) + if d == sesMailboxSimulatorDomain { + return true + } + _, ok := m.feedbackExcludedDomains[d] + return ok +} + +// denominatorOnly reports whether a bucket adds to the detector's +// denominator without adding to any numerator. +func (b Bucket) denominatorOnly() bool { + return b == BucketDelivered || b == BucketTerminalOther +} + +// excludedFromDenominator reports whether a denominator-only outcome for +// addr must not count: the SES mailbox simulator, the deployment's shared +// agent domains (configured, plus the platform-owned verified domains rows +// seeded for them), and the sending account's own verified domains. Each is +// mail a sender can generate at will to itself, so counting it would let an +// abuser dilute a bounce or complaint rate below the pause threshold with +// self-addressed traffic. Only denominator-only outcomes are excluded: a +// hard bounce or complaint from any of these recipients is still evidence +// against the sender and is never discarded. Decided from configuration and +// the domains table, never DNS, because it runs on the ingestion path. +func (m *Module) excludedFromDenominator(ctx context.Context, tx pgx.Tx, corr feedbackCorrelation, accountExists bool, addr string) (bool, error) { + at := strings.LastIndexByte(addr, '@') + if at < 0 || at == len(addr)-1 { + return false, nil + } + raw := strings.TrimSuffix(strings.ToLower(strings.TrimSpace(addr[at+1:])), ".") + domain := canonicalDomain(raw) + if m.ExcludesFeedbackDomain(domain) { + return true, nil + } + var account *string + if accountExists && corr.sourceAccount != nil { + account = corr.sourceAccount + } + var excluded bool + if err := tx.QueryRow(ctx, ` + SELECT EXISTS ( + SELECT 1 FROM domains + WHERE domain = ANY($1::text[]) AND verified + AND (user_id IS NULL OR user_id = $2))`, + []string{domain, raw}, account, + ).Scan(&excluded); err != nil { + return false, fmt.Errorf("sendingpolicy: classify feedback recipient domain: %w", err) + } + return excluded, nil +} + +// RepairSuppressionsTx writes the account-wide suppressions a signed event +// proved, for the case where no live message row can own them, inside the +// caller's transaction. Upserts go through suppressionsync, so a row +// re-proven here advances its generation and clears a pending removal. It +// returns only the repairs that inserted a new row: an address already +// suppressed — manually, by an earlier bounce — is refreshed, never +// reported, so the consumer announces nothing for it. +func (m *Module) RepairSuppressionsTx(ctx context.Context, tx pgx.Tx, accountRef string, repairs []delivery.FeedbackRepair) ([]delivery.FeedbackRepair, error) { + if strings.TrimSpace(accountRef) == "" || len(repairs) == 0 { + return nil, nil + } + // The account must still exist: a suppression is customer state, and + // the row carries a foreign key to the user. + // FOR KEY SHARE, not a bare EXISTS: the suppression rows carry a foreign + // key to this user, so a deletion committing between the check and the + // upsert would turn a no-op into a constraint violation. The key share + // blocks that delete for the length of this transaction and is exactly + // what the insert would take anyway. + var live string + err := tx.QueryRow(ctx, `SELECT id FROM users WHERE id = $1 FOR KEY SHARE`, accountRef).Scan(&live) + if errors.Is(err, pgx.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("sendingpolicy: check account for repair: %w", err) + } + var inserted []delivery.FeedbackRepair + for _, r := range repairs { + up, err := suppressionsync.UpsertTx(ctx, tx, "supp_"+randomSuffix(), accountRef, + strings.ToLower(strings.TrimSpace(r.Address)), r.Reason, r.Source, "") + if err != nil { + return nil, err + } + if up.Inserted { + inserted = append(inserted, r) + } + } + return inserted, nil +} + +// RepairSuppressions is RepairSuppressionsTx in a transaction of its own, +// for operator tooling and tests that have no enclosing transaction. +func (m *Module) RepairSuppressions(ctx context.Context, accountRef string, repairs []delivery.FeedbackRepair) ([]delivery.FeedbackRepair, error) { + tx, err := m.pool.Begin(ctx) + if err != nil { + return nil, fmt.Errorf("sendingpolicy: begin suppression repair: %w", err) + } + defer func() { _ = tx.Rollback(ctx) }() + inserted, err := m.RepairSuppressionsTx(ctx, tx, accountRef, repairs) + if err != nil { + return nil, err + } + if err := tx.Commit(ctx); err != nil { + return nil, fmt.Errorf("sendingpolicy: commit suppression repair: %w", err) + } + return inserted, nil +} + +// lookupCorrelation resolves the retained row by provider message id first +// (normalized to SES's bare form, the same function that bound it), then by +// the echoed attempt marker. +// +// Both reads take FOR SHARE. The account purge's seal stamps expires_at on +// this row and then on its events; without the share lock, a notification +// could read the pre-seal NULL expiry, the seal could stamp and commit, and +// this transaction would then insert an event row with a NULL expiry that +// nothing ever stamps again. With it, the seal's UPDATE waits for this +// transaction (whose event row its follow-up events UPDATE then sees), or +// this read waits for the seal and re-reads the stamped expiry. Lock order +// is correlation (share) → control row (update) → users (key share, via the +// aggregate FK); the seal holds users FOR NO KEY UPDATE, which does not +// conflict with key share, so the two cannot deadlock. The retention +// janitor's DELETE of an expired correlation likewise waits for, or is seen +// by, this lock, so no event can be inserted for a correlation the janitor +// just removed. +func lookupCorrelation(ctx context.Context, tx pgx.Tx, providerMessageID, attemptID string) (feedbackCorrelation, bool, error) { + const cols = `SELECT correlation_id, source_account_ref, purpose, shared_reputation, expires_at + FROM sending_feedback_correlations ` + scan := func(row pgx.Row) (feedbackCorrelation, bool, error) { + var c feedbackCorrelation + var purpose string + err := row.Scan(&c.id, &c.sourceAccount, &purpose, &c.shared, &c.expiresAt) + if errors.Is(err, pgx.ErrNoRows) { + return feedbackCorrelation{}, false, nil + } + if err != nil { + return feedbackCorrelation{}, false, fmt.Errorf("sendingpolicy: lookup correlation: %w", err) + } + c.purpose = Purpose(purpose) + return c, true, nil + } + if id := NormalizeProviderMessageID(providerMessageID); id != "" { + // provider_message_id is not unique (a re-driven send can bind a new + // id to a new attempt), so order deterministically. Rows past their + // horizon are excluded outright rather than merely ordered last: the + // janitor deletes exactly those rows, and a LIMIT 1 FOR SHARE whose + // chosen row is deleted concurrently returns NO row instead of the + // next one — a spurious uncorrelated result. An unexpired row is + // never a GC target, so the lock cannot lose it. + c, ok, err := scan(tx.QueryRow(ctx, cols+`WHERE provider_message_id = $1 + AND (expires_at IS NULL OR expires_at > now()) + ORDER BY created_at DESC, correlation_id + LIMIT 1 + FOR SHARE`, id)) + if err != nil || ok { + return c, ok, err + } + } + if attemptID = strings.TrimSpace(attemptID); attemptID != "" { + return scan(tx.QueryRow(ctx, cols+`WHERE correlation_id = $1 + AND (expires_at IS NULL OR expires_at > now()) + FOR SHARE`, attemptID)) + } + return feedbackCorrelation{}, false, nil +} + +// pastRetention reports whether the marker names a correlation that still +// exists but is past its horizon (or is being removed by the janitor right +// now — this unlocked read sees the pre-delete snapshot). Feedback for it is +// retention working as designed, not the lost-correlation alert. +func pastRetention(ctx context.Context, tx pgx.Tx, attemptID string) (bool, error) { + var exists bool + if err := tx.QueryRow(ctx, + `SELECT EXISTS (SELECT 1 FROM sending_feedback_correlations WHERE correlation_id = $1)`, + strings.TrimSpace(attemptID)).Scan(&exists); err != nil { + return false, fmt.Errorf("sendingpolicy: check expired correlation: %w", err) + } + return exists, nil +} + +func loadFeedbackRecipients(ctx context.Context, tx pgx.Tx, correlationID string) ([]*feedbackRecipient, error) { + rows, err := tx.Query(ctx, ` + SELECT recipient_hmac, hmac_key_version, detector_bucket, evidence_rank, + provider_occurred_at, evidence_event_id, bucket_epoch, bucket_day + FROM sending_feedback_recipients + WHERE correlation_id = $1 + ORDER BY recipient_hmac + FOR UPDATE`, correlationID) + if err != nil { + return nil, fmt.Errorf("sendingpolicy: load feedback recipients: %w", err) + } + defer rows.Close() + var out []*feedbackRecipient + for rows.Next() { + r := &feedbackRecipient{} + var bucket string + if err := rows.Scan(&r.mac, &r.keyVersion, &bucket, &r.rank, &r.occurredAt, &r.eventID, &r.epoch, &r.day); err != nil { + return nil, fmt.Errorf("sendingpolicy: scan feedback recipient: %w", err) + } + r.bucket = Bucket(bucket) + out = append(out, r) + } + return out, rows.Err() +} + +// matchRecipient finds the retained row whose keyed HMAC verifies for addr. +// Without a keyring nothing can match, which is the fail-closed answer: a +// process that does not hold the key cannot vouch for a recipient. +// +// The HMAC subject is canonicalRecipient(addr), the same function the gate +// signed with. A row signed before canonicalization existed (a Unicode +// domain signed as typed) is still matched through the raw form. +func (m *Module) matchRecipient(rows []*feedbackRecipient, addr string) (*feedbackRecipient, bool) { + if m.secrets.Keyring == nil { + return nil, false + } + subjects := [][]byte{[]byte(canonicalRecipient(addr))} + if raw := strings.ToLower(strings.TrimSpace(addr)); raw != string(subjects[0]) { + subjects = append(subjects, []byte(raw)) + } + for _, r := range rows { + for _, subject := range subjects { + if m.secrets.Keyring.Verify(r.keyVersion, subject, r.mac) { + return r, true + } + } + } + return nil, false +} + +// canonicalRecipient is the ONE form a recipient's keyed HMAC is computed +// over, on both sides of the provider: the gate signs it at authorization, +// and feedback verifies against it. The local part is lower-cased as typed; +// the domain goes through IDNA ToASCII (lookup profile), so an +// internationalized domain authorized as Unicode ("bücher.test") and +// reported back by the provider in its A-label form ("xn--bcher-kva.test") +// produce the same HMAC. A domain the lookup profile refuses keeps its +// lower-cased raw form — deterministic, so both sides still agree. +func canonicalRecipient(addr string) string { + addr = strings.ToLower(strings.TrimSpace(addr)) + at := strings.LastIndexByte(addr, '@') + if at <= 0 || at == len(addr)-1 { + return addr + } + return addr[:at+1] + canonicalDomain(addr[at+1:]) +} + +// canonicalDomain is canonicalRecipient's domain half. +func canonicalDomain(domain string) string { + domain = strings.TrimSuffix(strings.ToLower(strings.TrimSpace(domain)), ".") + if ascii, err := idna.Lookup.ToASCII(domain); err == nil && ascii != "" { + return strings.ToLower(ascii) + } + return domain +} + +// applyEvidence moves one recipient's bucket under the monotonic evidence +// order and keeps the daily aggregates in step: subtract the prior bucket +// from the epoch/day it was counted in, add the new one to the current +// epoch and today, all in the caller's transaction. +func (m *Module) applyEvidence(ctx context.Context, tx pgx.Tx, corr feedbackCorrelation, row *feedbackRecipient, bucket Bucket, fb delivery.ProviderFeedback, accountExists bool, accountID string, epoch int64, today time.Time) error { + newRank := bucket.Rank() + occurred := fb.OccurredAt.UTC() + switch { + case newRank > row.rank: + // replace below + case newRank == row.rank: + // Equal evidence: zero delta. Provider time, then event id, decide + // which event the row remembers as its provenance. + if row.occurredAt != nil && (occurred.Before(*row.occurredAt) || + (occurred.Equal(*row.occurredAt) && row.eventID != nil && fb.ProviderEventID <= *row.eventID)) { + return nil + } + if _, err := tx.Exec(ctx, ` + UPDATE sending_feedback_recipients + SET provider_occurred_at = $3, evidence_event_id = $4, updated_at = now() + WHERE correlation_id = $1 AND recipient_hmac = $2`, + corr.id, row.mac, occurred, fb.ProviderEventID); err != nil { + return fmt.Errorf("sendingpolicy: update feedback provenance: %w", err) + } + return nil + default: + // Lower evidence than already observed: never regress. + return nil + } + + // Subtract the prior bucket where it was counted. A missing aggregate + // row (expired, or the account is gone and the FK cascaded) is a no-op. + if col := row.bucket.column(); col != "" && row.epoch != nil && row.day != nil && corr.sourceAccount != nil { + if _, err := tx.Exec(ctx, fmt.Sprintf(` + UPDATE account_sending_outcomes_daily + SET %[1]s = GREATEST(%[1]s - 1, 0) + WHERE user_id = $1 AND outcome_epoch = $2 AND day = $3 AND shared_reputation = $4`, col), + *corr.sourceAccount, *row.epoch, *row.day, corr.shared); err != nil { + return fmt.Errorf("sendingpolicy: subtract prior outcome: %w", err) + } + } + + var newEpoch *int64 + var newDay *time.Time + if col := bucket.column(); col != "" && accountExists { + if _, err := tx.Exec(ctx, fmt.Sprintf(` + INSERT INTO account_sending_outcomes_daily (user_id, outcome_epoch, day, shared_reputation, %[1]s) + VALUES ($1, $2, $3, $4, 1) + ON CONFLICT (user_id, outcome_epoch, day, shared_reputation) DO UPDATE + SET %[1]s = account_sending_outcomes_daily.%[1]s + 1`, col), + accountID, epoch, today, corr.shared); err != nil { + return fmt.Errorf("sendingpolicy: add outcome: %w", err) + } + newEpoch, newDay = &epoch, &today + } + + if _, err := tx.Exec(ctx, ` + UPDATE sending_feedback_recipients + SET detector_bucket = $3, evidence_rank = $4, provider_occurred_at = $5, + evidence_event_id = $6, bucket_epoch = $7, bucket_day = $8, updated_at = now() + WHERE correlation_id = $1 AND recipient_hmac = $2`, + corr.id, row.mac, string(bucket), newRank, occurred, fb.ProviderEventID, newEpoch, newDay); err != nil { + return fmt.Errorf("sendingpolicy: update feedback recipient: %w", err) + } + row.bucket, row.rank, row.epoch, row.day = bucket, newRank, newEpoch, newDay + row.occurredAt, row.eventID = &occurred, &fb.ProviderEventID + return nil +} + +// repairReason is the suppression reason recorded: the retained subtype, so +// the customer sees why (never the diagnostic text, which can quote the +// recipient's server). +func repairReason(fb delivery.ProviderFeedback, d Derivation) string { + switch fb.Kind { + case delivery.KindBounce: + if s := strings.TrimSpace(fb.BounceSubType); s != "" { + return "bounce:" + s + } + return "bounce:" + fb.BounceType + case delivery.KindComplaint: + if s := strings.TrimSpace(fb.ComplaintSubType); s != "" { + return "complaint:" + s + } + return "complaint" + } + return string(d.Bucket) +} + +func randomSuffix() string { return strings.TrimPrefix(randomID("x_"), "x_") } + +// VerifyKeyringCoverage fails closed when a retained, unexpired recipient +// row was signed under a key version this process does not hold. Feedback +// for those rows could never be matched, which would leave the detector +// silently blind; refusing to start is the alert. +// +// A deployment with NO keyring configured is not checked: it signs nothing +// and matches nothing by design (self-host with every control disabled), and +// bricking such a server over rows an earlier configuration wrote would turn +// a disabled feature into an outage. Removing a key version while it is +// still in use is the case this guards. +func (m *Module) VerifyKeyringCoverage(ctx context.Context) error { + if m.secrets.Keyring == nil { + return nil + } + held := m.secrets.Keyring.Versions() + // One question, first answer wins. A violation stops at the first row; + // proving the healthy case is inherently a full pass over the retained + // rows, which no index can serve for a "not in this set" predicate. + // That cost is boot-only, and the alternative — starting blind — is + // the failure this gate exists to prevent. + var missing *int + err := m.pool.QueryRow(ctx, ` + SELECT r.hmac_key_version + FROM sending_feedback_recipients r + JOIN sending_feedback_correlations c ON c.correlation_id = r.correlation_id + WHERE NOT (r.hmac_key_version = ANY($1::int[])) + AND (c.expires_at IS NULL OR c.expires_at > now()) + LIMIT 1`, held).Scan(&missing) + if errors.Is(err, pgx.ErrNoRows) { + return nil + } + if err != nil { + return fmt.Errorf("sendingpolicy: read retained key versions: %w", err) + } + return fmt.Errorf("sendingpolicy: retained feedback rows are signed under HMAC key version %d, which this keyring (versions %v) does not hold; keep the old key until those rows expire, then remove it", *missing, held) +} + +// FeedbackGCStats reports what one retention pass removed. +type FeedbackGCStats struct { + Events int64 + Recipients int64 + Correlations int64 + Outcomes int64 +} + +// GCFeedback removes feedback provenance past its horizon and daily outcome +// rows outside the detector window plus one UTC day of safety. Customer +// correlations carry no expiry while the account exists; the account purge's +// seal transaction (identity.Store.purgeAccount) stamps one when the account +// becomes irrecoverable, so this pass is what makes the post-purge retention +// real. A trashed-but-restorable account is not stamped. +// +// Events and recipients are removed BY THEIR CORRELATION, in the statement +// that removes it, rather than by their own expiry column: an event whose +// expiry was never stamped (a pre-fix race with the purge seal left some +// with NULL) must still go when its correlation does. Events are also +// removed by their own stamped expiry, which is always the correlation's. +func (m *Module) GCFeedback(ctx context.Context, now time.Time, windowDays int) (FeedbackGCStats, error) { + var st FeedbackGCStats + now = now.UTC() + // The outer SELECT reads only the CTEs' RETURNING output, never a table + // a CTE modified, so the statement-snapshot hazard does not apply. + if err := m.pool.QueryRow(ctx, ` + WITH gone AS ( + DELETE FROM sending_feedback_correlations + WHERE expires_at IS NOT NULL AND expires_at <= $1 + RETURNING correlation_id + ), recipients AS ( + DELETE FROM sending_feedback_recipients r + USING gone WHERE r.correlation_id = gone.correlation_id + RETURNING 1 + ), events AS ( + DELETE FROM sending_feedback_events e + USING gone WHERE e.correlation_id = gone.correlation_id + RETURNING 1 + ) + SELECT (SELECT count(*) FROM gone), (SELECT count(*) FROM recipients), (SELECT count(*) FROM events)`, + now).Scan(&st.Correlations, &st.Recipients, &st.Events); err != nil { + return st, fmt.Errorf("sendingpolicy: gc feedback provenance: %w", err) + } + tag, err := m.pool.Exec(ctx, `DELETE FROM sending_feedback_events WHERE expires_at IS NOT NULL AND expires_at <= $1`, now) + if err != nil { + return st, fmt.Errorf("sendingpolicy: gc feedback events: %w", err) + } + st.Events += tag.RowsAffected() + if windowDays < 1 { + windowDays = 7 + } + cutoff := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, time.UTC).AddDate(0, 0, -(windowDays + 1)) + tag, err = m.pool.Exec(ctx, `DELETE FROM account_sending_outcomes_daily WHERE day < $1`, cutoff) + if err != nil { + return st, fmt.Errorf("sendingpolicy: gc daily outcomes: %w", err) + } + st.Outcomes = tag.RowsAffected() + return st, nil +} + +// FeedbackReconcileStats reports what one retention reconciliation changed. +type FeedbackReconcileStats struct { + // StampedCorrelations are customer correlations whose account no longer + // exists but which had no expiry yet. + StampedCorrelations int64 + // StampedEvents are events given their correlation's expiry. + StampedEvents int64 + // OrphanEvents are events whose correlation was already gone. + OrphanEvents int64 +} + +// ReconcileFeedbackRetention closes the gaps the purge seal cannot: it +// stamps expires_at = now + retention on every customer correlation whose +// source account no longer has a users row (erased before the seal stamped +// anything, or a correlation authorized in a race with the purge), gives +// every unstamped event its correlation's expiry, and deletes events whose +// correlation is already gone. Idempotent: only NULL expiries are written. +// Migration 124 runs the same predicate once for the backlog; this pass is +// the standing backstop. +// +// A trashed account still has its users row, so it is never stamped here — +// the same rule the seal follows. +func (m *Module) ReconcileFeedbackRetention(ctx context.Context, now time.Time, retention time.Duration) (FeedbackReconcileStats, error) { + var st FeedbackReconcileStats + if retention <= 0 { + return st, errors.New("sendingpolicy: feedback retention must be positive") + } + expires := now.UTC().Add(retention) + tag, err := m.pool.Exec(ctx, ` + UPDATE sending_feedback_correlations c + SET expires_at = $1 + WHERE c.expires_at IS NULL + AND c.purpose IN ('customer_message', 'customer_notification') + AND c.source_account_ref IS NOT NULL + AND NOT EXISTS (SELECT 1 FROM users u WHERE u.id = c.source_account_ref)`, expires) + if err != nil { + return st, fmt.Errorf("sendingpolicy: stamp orphaned feedback correlations: %w", err) + } + st.StampedCorrelations = tag.RowsAffected() + tag, err = m.pool.Exec(ctx, ` + UPDATE sending_feedback_events e + SET expires_at = c.expires_at + FROM sending_feedback_correlations c + WHERE c.correlation_id = e.correlation_id + AND e.expires_at IS NULL AND c.expires_at IS NOT NULL`) + if err != nil { + return st, fmt.Errorf("sendingpolicy: stamp feedback event retention: %w", err) + } + st.StampedEvents = tag.RowsAffected() + tag, err = m.pool.Exec(ctx, ` + DELETE FROM sending_feedback_events e + WHERE NOT EXISTS (SELECT 1 FROM sending_feedback_correlations c WHERE c.correlation_id = e.correlation_id)`) + if err != nil { + return st, fmt.Errorf("sendingpolicy: sweep orphaned feedback events: %w", err) + } + st.OrphanEvents = tag.RowsAffected() + return st, nil +} + +// effectivePolicyNow reads the policy this deployment actually runs: the +// database singleton when the source is the database, the validated config +// otherwise. +func (m *Module) effectivePolicyNow(ctx context.Context) (RuntimePolicy, error) { + if m.source != PolicySourceDatabase { + return m.configPolicy, nil + } + tx, err := m.pool.Begin(ctx) + if err != nil { + return RuntimePolicy{}, fmt.Errorf("sendingpolicy: begin policy read: %w", err) + } + defer func() { _ = tx.Rollback(ctx) }() + policy, err := m.effectivePolicy(ctx, tx) + if err != nil { + return RuntimePolicy{}, err + } + if err := tx.Commit(ctx); err != nil { + return RuntimePolicy{}, fmt.Errorf("sendingpolicy: commit policy read: %w", err) + } + return policy, nil +} + +// EffectiveDetectorWindowDays reads the detector window from the effective +// policy. +func (m *Module) EffectiveDetectorWindowDays(ctx context.Context) (int, error) { + policy, err := m.effectivePolicyNow(ctx) + if err != nil { + return 0, err + } + return policy.DetectorWindowDays, nil +} + +// EffectiveFeedbackRetention is the post-account-deletion horizon for +// retained feedback provenance, from the effective policy — the same +// accessor the retention janitor reads, so the purge seal, the janitor's +// backstop stamp, and non-customer correlations all agree on a database- +// source deployment whose activated policy differs from the config file. +func (m *Module) EffectiveFeedbackRetention(ctx context.Context) (time.Duration, error) { + policy, err := m.effectivePolicyNow(ctx) + if err != nil { + return 0, err + } + if policy.SendingFeedbackPostAcctRetention < 1 { + return 0, fmt.Errorf("sendingpolicy: effective feedback retention is %d days", policy.SendingFeedbackPostAcctRetention) + } + return time.Duration(policy.SendingFeedbackPostAcctRetention) * 24 * time.Hour, nil +} diff --git a/internal/sendingpolicy/feedback_canonical_internal_test.go b/internal/sendingpolicy/feedback_canonical_internal_test.go new file mode 100644 index 000000000..94091fe1c --- /dev/null +++ b/internal/sendingpolicy/feedback_canonical_internal_test.go @@ -0,0 +1,24 @@ +package sendingpolicy + +import "testing" + +// TestCanonicalRecipient pins the one HMAC subject both sides of the +// provider compute: IDNA-ASCII domain, lower-cased, so the Unicode spelling +// authorization accepts and the A-label spelling a provider reports agree. +func TestCanonicalRecipient(t *testing.T) { + for in, want := range map[string]string{ + "Leser@Bücher.Example": "leser@xn--bcher-kva.example", + "leser@xn--bcher-kva.example": "leser@xn--bcher-kva.example", + " Plain@Example.TEST ": "plain@example.test", + "under_score@my_host.example": "under_score@my_host.example", // lookup refuses: raw, deterministic + "trailing@example.test.": "trailing@example.test", + "no-at-sign": "no-at-sign", + } { + if got := canonicalRecipient(in); got != want { + t.Errorf("canonicalRecipient(%q) = %q, want %q", in, got, want) + } + } + if canonicalRecipient("a@bücher.example") != canonicalRecipient("A@XN--BCHER-KVA.EXAMPLE") { + t.Fatal("Unicode and A-label spellings must canonicalize identically") + } +} diff --git a/internal/sendingpolicy/feedback_correlation_integration_test.go b/internal/sendingpolicy/feedback_correlation_integration_test.go new file mode 100644 index 000000000..cc76d5950 --- /dev/null +++ b/internal/sendingpolicy/feedback_correlation_integration_test.go @@ -0,0 +1,628 @@ +package sendingpolicy_test + +import ( + "fmt" + "testing" + "time" + + "github.com/jackc/pgx/v5" + + "github.com/tokencanopy/e2a/internal/delivery" + "github.com/tokencanopy/e2a/internal/sendingpolicy" +) + +// authorizedSend drives one message through accept + authorization and +// returns the token, its attempt correlation id, and the SES id it settles +// under, so feedback can be fed back by either key. +func (f *fixture) authorizedSend(g sendingpolicy.Gate, messageID string, recipients []string) (auth sendingpolicy.ProviderAuthorization, correlationID, sesID string) { + f.t.Helper() + accept, ref := f.prepareMessage(g, messageID) + if accept != sendingpolicy.AcceptanceAccept { + f.t.Fatalf("accept = %s", accept) + } + _, attempt, err := g.Reserve(f.ctx, ref) + if err != nil { + f.t.Fatal(err) + } + decision, token, err := g.ConsumeAttempt(f.ctx, attempt) + if err != nil || !decision.Allow || token == nil { + f.t.Fatalf("consume: allow=%v err=%v", decision.Allow, err) + } + headers, err := token.ValidateEnvelope(recipients) + if err != nil { + f.t.Fatal(err) + } + if err := g.RedeemProviderCall(f.ctx, *token); err != nil { + f.t.Fatal(err) + } + sesID = fmt.Sprintf("<%s-ses@us-east-2.amazonses.com>", messageID) + if err := g.SettleProvider(f.ctx, sendingpolicy.ProviderSettlement{Attempt: token.Attempt(), Outcome: sendingpolicy.SettlementProviderAccepted, ProviderMessageID: sesID}); err != nil { + f.t.Fatal(err) + } + return *token, headers.AttemptCorrelationID, sesID +} + +// messageTo inserts an outbound message with explicit recipients. +func (f *fixture) messageTo(agentID, sentAs string, recipients []string) string { + f.t.Helper() + messageSeq++ + id := fmt.Sprintf("msg_fb_%d", messageSeq) + if _, err := f.pool.Exec(f.ctx, + `INSERT INTO messages (id, agent_id, direction, to_recipients, sent_as, status) + VALUES ($1, $2, 'outbound', $3, $4, 'sent')`, id, agentID, recipients, sentAs); err != nil { + f.t.Fatalf("insert message: %v", err) + } + return id +} + +func (f *fixture) outcomes(userID string) map[string][4]int { + f.t.Helper() + rows, err := f.pool.Query(f.ctx, ` + SELECT outcome_epoch, day, shared_reputation, delivered_count, terminal_other_count, hard_bounce_count, complaint_count + FROM account_sending_outcomes_daily WHERE user_id = $1`, userID) + if err != nil { + f.t.Fatal(err) + } + defer rows.Close() + out := map[string][4]int{} + for rows.Next() { + var epoch int64 + var day time.Time + var shared bool + var c [4]int + if err := rows.Scan(&epoch, &day, &shared, &c[0], &c[1], &c[2], &c[3]); err != nil { + f.t.Fatal(err) + } + out[fmt.Sprintf("e%d/%s/shared=%v", epoch, day.Format("2006-01-02"), shared)] = c + } + return out +} + +func (f *fixture) recipientRow(correlationID string) (bucket string, rank int, epoch *int64) { + f.t.Helper() + if err := f.pool.QueryRow(f.ctx, `SELECT detector_bucket, evidence_rank, bucket_epoch FROM sending_feedback_recipients WHERE correlation_id = $1`, correlationID).Scan(&bucket, &rank, &epoch); err != nil { + f.t.Fatal(err) + } + return +} + +// provenance returns the event id and provider time the recipient row +// remembers — what the equal-rank tie-break decides. +func (f *fixture) provenance(correlationID string) (eventID string, at time.Time) { + f.t.Helper() + var id *string + var ts *time.Time + if err := f.pool.QueryRow(f.ctx, `SELECT evidence_event_id, provider_occurred_at FROM sending_feedback_recipients WHERE correlation_id = $1`, correlationID).Scan(&id, &ts); err != nil { + f.t.Fatal(err) + } + if id != nil { + eventID = *id + } + if ts != nil { + at = ts.UTC() + } + return +} + +// repair applies what the seam reported, the way the consumer does when no +// live message owns the row. +func (f *fixture) repair(m *sendingpolicy.Module, res delivery.FeedbackResult) { + f.t.Helper() + if _, err := m.RepairSuppressions(f.ctx, res.AccountRef, res.RepairNeeded); err != nil { + f.t.Fatalf("repair: %v", err) + } +} + +func (f *fixture) suppressed(userID, address string) bool { + f.t.Helper() + var n int + if err := f.pool.QueryRow(f.ctx, `SELECT count(*) FROM suppressions WHERE user_id = $1 AND address = $2`, userID, address).Scan(&n); err != nil { + f.t.Fatal(err) + } + return n > 0 +} + +func feedback(id string, at time.Time, kind delivery.EventKind, sesID, attemptID string, recipients ...string) delivery.ProviderFeedback { + return delivery.ProviderFeedback{ProviderEventID: id, OccurredAt: at, Kind: kind, ProviderMessageID: sesID, AttemptCorrelationID: attemptID, Recipients: recipients} +} + +func todayKey(epoch int64, shared bool) string { + d := time.Now().UTC() + return fmt.Sprintf("e%d/%s/shared=%v", epoch, d.Format("2006-01-02"), shared) +} + +// TestFeedbackSurvivesMessageAndAgentPurge: authorize a send, purge the +// message and then the agent, and prove feedback by SES id and by the +// attempt header still lands on the right account with a valid HMAC match, +// while a recipient outside the authorized envelope is rejected. +func TestFeedbackSurvivesMessageAndAgentPurge(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + rcpts := []string{"alice@example.test", "bob@example.test"} + msg := f.messageTo(agent, "relay", rcpts) + _, corrID, sesID := f.authorizedSend(g, msg, rcpts) + + // Purge message then agent — the deletion order the store uses. + if _, err := f.pool.Exec(f.ctx, `DELETE FROM messages WHERE id = $1`, msg); err != nil { + t.Fatal(err) + } + if _, err := f.pool.Exec(f.ctx, `DELETE FROM agent_identities WHERE id = $1`, agent); err != nil { + t.Fatal(err) + } + + at := time.Now().UTC() + // Hard bounce for alice by SES id. + res, err := module.ProcessProviderFeedback(f.ctx, delivery.ProviderFeedback{ + ProviderEventID: "evt-hb", OccurredAt: at, Kind: delivery.KindBounce, BounceType: "permanent", BounceSubType: "General", + ProviderMessageID: sesID, Recipients: []string{"Alice@Example.test"}, + }) + if err != nil { + t.Fatal(err) + } + if !res.Correlated || res.Duplicate || res.AccountRef != user { + t.Fatalf("result = %+v", res) + } + if len(res.RepairNeeded) != 1 || res.RepairNeeded[0].Address != "alice@example.test" || res.RepairNeeded[0].Source != "bounce" { + t.Fatalf("repairs = %+v", res.RepairNeeded) + } + // The seam reports; it never writes while a message could own the row. + if f.suppressed(user, "alice@example.test") { + t.Fatal("the seam must not write the suppression itself") + } + f.repair(module, res) + if !f.suppressed(user, "alice@example.test") { + t.Fatal("hard bounce must recreate the account suppression while the account exists") + } + // Delivered for bob by the attempt header only (no SES id). + if _, err := module.ProcessProviderFeedback(f.ctx, feedback("evt-del", at, delivery.KindDelivery, "", corrID, "bob@example.test")); err != nil { + t.Fatal(err) + } + // A recipient never in the envelope: rejected, no accounting, no suppression. + res, err = module.ProcessProviderFeedback(f.ctx, delivery.ProviderFeedback{ + ProviderEventID: "evt-forged", OccurredAt: at, Kind: delivery.KindBounce, BounceType: "permanent", BounceSubType: "General", + ProviderMessageID: sesID, Recipients: []string{"mallory@example.test"}, + }) + if err != nil { + t.Fatal(err) + } + if !res.Correlated || len(res.RepairNeeded) != 0 { + t.Fatalf("forged recipient must be rejected: %+v", res) + } + f.repair(module, res) + if f.suppressed(user, "mallory@example.test") { + t.Fatal("a recipient outside the authorized envelope must never be suppressed") + } + + got := f.outcomes(user) + if c := got[todayKey(1, true)]; c != [4]int{1, 0, 1, 0} { + t.Fatalf("shared aggregate = %v, want delivered=1 hard_bounce=1: %v", c, got) + } + if len(got) != 1 { + t.Fatalf("unexpected aggregate rows: %v", got) + } + + // Duplicate provider event: zero delta. + res, err = module.ProcessProviderFeedback(f.ctx, delivery.ProviderFeedback{ + ProviderEventID: "evt-hb", OccurredAt: at, Kind: delivery.KindBounce, BounceType: "permanent", BounceSubType: "General", + ProviderMessageID: sesID, Recipients: []string{"alice@example.test"}, + }) + if err != nil || !res.Duplicate { + t.Fatalf("duplicate: res=%+v err=%v", res, err) + } + if c := f.outcomes(user)[todayKey(1, true)]; c != [4]int{1, 0, 1, 0} { + t.Fatalf("duplicate changed the aggregate: %v", c) + } +} + +// TestFeedbackEvidenceIsMonotonic: delivered → complaint replaces (delivered +// subtracted, complaint added); a delayed delivery after a complaint is +// ignored; a suppression-list subtype after a hard bounce keeps the hard +// bounce but still repairs the list; equal-rank duplicates are zero delta. +func TestFeedbackEvidenceIsMonotonic(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + rcpt := []string{"carol@example.test"} + msg := f.messageTo(agent, "own_address", rcpt) // dedicated path + _, corrID, sesID := f.authorizedSend(g, msg, rcpt) + t0 := time.Now().UTC().Add(-time.Hour) + + step := func(id string, at time.Time, kind delivery.EventKind, btype, bsub, csub string) { + t.Helper() + res, err := module.ProcessProviderFeedback(f.ctx, delivery.ProviderFeedback{ + ProviderEventID: id, OccurredAt: at, Kind: kind, BounceType: btype, BounceSubType: bsub, ComplaintSubType: csub, + ProviderMessageID: sesID, AttemptCorrelationID: corrID, Recipients: rcpt, + }) + if err != nil { + t.Fatal(err) + } + f.repair(module, res) + } + key := todayKey(1, false) + + step("e1", t0, delivery.KindDelivery, "", "", "") + if c := f.outcomes(user)[key]; c != [4]int{1, 0, 0, 0} { + t.Fatalf("after delivered: %v", c) + } + step("e2", t0.Add(time.Minute), delivery.KindComplaint, "", "", "") + if c := f.outcomes(user)[key]; c != [4]int{0, 0, 0, 1} { + t.Fatalf("delivered→complaint must move the unit: %v", c) + } + if b, r, _ := f.recipientRow(corrID); b != "complaint" || r != 4 { + t.Fatalf("row = %s/%d", b, r) + } + // Delayed lower evidence never regresses. + step("e3", t0.Add(2*time.Minute), delivery.KindDelivery, "", "", "") + step("e4", t0.Add(3*time.Minute), delivery.KindBounce, "permanent", "OnAccountSuppressionList", "") + if c := f.outcomes(user)[key]; c != [4]int{0, 0, 0, 1} { + t.Fatalf("lower evidence regressed the complaint: %v", c) + } + // Equal rank duplicate (a second complaint event): zero delta. + step("e5", t0.Add(4*time.Minute), delivery.KindComplaint, "", "", "") + if c := f.outcomes(user)[key]; c != [4]int{0, 0, 0, 1} { + t.Fatalf("equal-rank duplicate changed counts: %v", c) + } + if !f.suppressed(user, "carol@example.test") { + t.Fatal("complaint must have repaired the suppression") + } + + // A second message: hard bounce first, then a suppression-list bounce + // (excluded, repairs) and a delayed transient bounce (lower rank). + msg2 := f.messageTo(agent, "own_address", []string{"dave@example.test"}) + _, corr2, ses2 := f.authorizedSend(g, msg2, []string{"dave@example.test"}) + fb := func(id string, kind delivery.EventKind, btype, bsub string) delivery.ProviderFeedback { + return delivery.ProviderFeedback{ProviderEventID: id, OccurredAt: t0, Kind: kind, BounceType: btype, BounceSubType: bsub, ProviderMessageID: ses2, AttemptCorrelationID: corr2, Recipients: []string{"dave@example.test"}} + } + for _, x := range []delivery.ProviderFeedback{fb("d1", delivery.KindBounce, "permanent", "General"), fb("d2", delivery.KindBounce, "permanent", "OnTenantSuppressionList"), fb("d3", delivery.KindBounce, "transient", "MailboxFull")} { + res, err := module.ProcessProviderFeedback(f.ctx, x) + if err != nil { + t.Fatal(err) + } + f.repair(module, res) + } + if c := f.outcomes(user)[key]; c != [4]int{0, 0, 1, 1} { + t.Fatalf("second message: %v", c) + } + if b, _, _ := f.recipientRow(corr2); b != "hard_bounce" { + t.Fatalf("row bucket = %s, want hard_bounce kept", b) + } + // terminal_other is a denominator unit on its own. + msg3 := f.messageTo(agent, "own_address", []string{"erin@example.test"}) + _, _, ses3 := f.authorizedSend(g, msg3, []string{"erin@example.test"}) + if _, err := module.ProcessProviderFeedback(f.ctx, delivery.ProviderFeedback{ProviderEventID: "t1", OccurredAt: t0, Kind: delivery.KindBounce, BounceType: "undetermined", ProviderMessageID: ses3, Recipients: []string{"erin@example.test"}}); err != nil { + t.Fatal(err) + } + if c := f.outcomes(user)[key]; c != [4]int{0, 1, 1, 1} { + t.Fatalf("terminal_other: %v", c) + } + if f.suppressed(user, "erin@example.test") { + t.Fatal("a soft bounce must not suppress") + } +} + +// TestFeedbackSharedAndDedicatedAggregatesStaySeparate: a hosted shared- +// mailbox message and a custom-domain message from one account land in +// different aggregate rows; a HITL notification is shared like the +// hosted message. +func TestFeedbackSharedAndDedicatedAggregatesStaySeparate(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + at := time.Now().UTC() + + shared := f.messageTo(agent, "relay", []string{"s@example.test"}) + _, _, sesShared := f.authorizedSend(g, shared, []string{"s@example.test"}) + dedicated := f.messageTo(agent, "own_address", []string{"d@example.test"}) + _, _, sesDed := f.authorizedSend(g, dedicated, []string{"d@example.test"}) + for _, x := range []delivery.ProviderFeedback{ + feedback("s1", at, delivery.KindDelivery, sesShared, "", "s@example.test"), + feedback("d1", at, delivery.KindDelivery, sesDed, "", "d@example.test"), + } { + if _, err := module.ProcessProviderFeedback(f.ctx, x); err != nil { + t.Fatal(err) + } + } + // HITL notification for a held message: authorized under the owner's + // address, counted on the shared path. + held := f.pendingMessage(agent, "own_address") + var ref sendingpolicy.OperationRef + f.inTx(func(tx pgx.Tx) error { + var err error + ref, err = g.PrepareNotificationTx(f.ctx, tx, sendingpolicy.NewHITLNotificationRef(held)) + return err + }) + _, attempt, err := g.Reserve(f.ctx, ref) + if err != nil { + t.Fatal(err) + } + _, token, err := g.ConsumeAttempt(f.ctx, attempt) + if err != nil || token == nil { + t.Fatalf("consume notification: %v", err) + } + owner := token.AuthorizedRecipients() + headers, err := token.ValidateEnvelope(owner) + if err != nil { + t.Fatal(err) + } + if _, err := module.ProcessProviderFeedback(f.ctx, feedback("n1", at, delivery.KindDelivery, "", headers.AttemptCorrelationID, owner...)); err != nil { + t.Fatal(err) + } + + got := f.outcomes(user) + if got[todayKey(1, true)] != [4]int{2, 0, 0, 0} || got[todayKey(1, false)] != [4]int{1, 0, 0, 0} { + t.Fatalf("aggregates = %v", got) + } +} + +// TestFeedbackAfterAccountDeletionUpdatesProvenanceOnly: once the account is +// gone, feedback still advances the retained bucket but recreates no +// suppression and no aggregate. +func TestFeedbackAfterAccountDeletionUpdatesProvenanceOnly(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + msg := f.messageTo(agent, "relay", []string{"gone@example.test"}) + _, corrID, sesID := f.authorizedSend(g, msg, []string{"gone@example.test"}) + + // Delete the whole account (cascades agents, messages, controls, aggregates). + if _, err := f.pool.Exec(f.ctx, `DELETE FROM users WHERE id = $1`, user); err != nil { + t.Fatal(err) + } + res, err := module.ProcessProviderFeedback(f.ctx, delivery.ProviderFeedback{ + ProviderEventID: "post-del", OccurredAt: time.Now().UTC(), Kind: delivery.KindComplaint, + ProviderMessageID: sesID, Recipients: []string{"gone@example.test"}, + }) + if err != nil { + t.Fatal(err) + } + if !res.Correlated || res.AccountRef != "" || len(res.RepairNeeded) != 0 { + t.Fatalf("result after deletion = %+v", res) + } + b, r, epoch := f.recipientRow(corrID) + if b != "complaint" || r != 4 || epoch != nil { + t.Fatalf("provenance = %s/%d epoch=%v, want complaint with no epoch", b, r, epoch) + } + var n int + if err := f.pool.QueryRow(f.ctx, `SELECT count(*) FROM suppressions WHERE address = 'gone@example.test'`).Scan(&n); err != nil { + t.Fatal(err) + } + if n != 0 { + t.Fatal("a deleted account must not get customer state recreated") + } +} + +// TestKeyringRotationAndCoverage: rows signed under version 1 still match +// after the active key moves to 2 while 1 is retained; a keyring without 1 +// fails coverage until those rows expire; a process holding only 2 cannot +// match a version-1 row. +func TestKeyringRotationAndCoverage(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) // signs under version 1 (fxHMAC active) + user := f.user("standard") + agent := f.agent(user) + msg := f.messageTo(agent, "relay", []string{"rot@example.test"}) + _, _, sesID := f.authorizedSend(g, msg, []string{"rot@example.test"}) + + superset, err := sendingpolicy.LoadKeyring(`{"active":2,"keys":{"1":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","2":"AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQE"}}`) + if err != nil { + t.Fatal(err) + } + onlyNew, err := sendingpolicy.LoadKeyring(`{"active":2,"keys":{"2":"AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQE"}}`) + if err != nil { + t.Fatal(err) + } + rotated := sendingpolicy.NewModule(f.pool, sendingpolicy.Secrets{Keyring: superset}) + if err := rotated.VerifyKeyringCoverage(f.ctx); err != nil { + t.Fatalf("superset keyring must cover retained rows: %v", err) + } + res, err := rotated.ProcessProviderFeedback(f.ctx, feedback("r1", time.Now().UTC(), delivery.KindDelivery, sesID, "", "rot@example.test")) + if err != nil || !res.Correlated { + t.Fatalf("rotated: %+v %v", res, err) + } + if c := f.outcomes(user)[todayKey(1, true)]; c != [4]int{1, 0, 0, 0} { + t.Fatalf("version-1 row must still match under the superset keyring: %v", c) + } + + narrow := sendingpolicy.NewModule(f.pool, sendingpolicy.Secrets{Keyring: onlyNew}) + if err := narrow.VerifyKeyringCoverage(f.ctx); err == nil { + t.Fatal("removing version 1 while its rows are retained must fail coverage") + } + res, err = narrow.ProcessProviderFeedback(f.ctx, feedback("r2", time.Now().UTC(), delivery.KindComplaint, sesID, "", "rot@example.test")) + if err != nil { + t.Fatal(err) + } + if c := f.outcomes(user)[todayKey(1, true)]; c != [4]int{1, 0, 0, 0} { + t.Fatalf("a keyring without version 1 must not match its rows: %v", c) + } + + // Expire the row's correlation: coverage no longer needs version 1. + if _, err := f.pool.Exec(f.ctx, `UPDATE sending_feedback_correlations SET expires_at = now() - interval '1 minute' WHERE provider_message_id = $1`, sendingpolicy.NormalizeProviderMessageID(sesID)); err != nil { + t.Fatal(err) + } + if err := narrow.VerifyKeyringCoverage(f.ctx); err != nil { + t.Fatalf("expired rows must not pin a key version: %v", err) + } +} + +// TestFeedbackGC: expired provenance is removed with its recipients and +// events; unexpired rows and recent aggregates stay; old aggregates go. +func TestFeedbackGC(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + live := f.messageTo(agent, "relay", []string{"live@example.test"}) + _, liveCorr, liveSES := f.authorizedSend(g, live, []string{"live@example.test"}) + old := f.messageTo(agent, "relay", []string{"old@example.test"}) + _, oldCorr, oldSES := f.authorizedSend(g, old, []string{"old@example.test"}) + at := time.Now().UTC() + for _, x := range []delivery.ProviderFeedback{ + feedback("g-live", at, delivery.KindDelivery, liveSES, "", "live@example.test"), + feedback("g-old", at, delivery.KindDelivery, oldSES, "", "old@example.test"), + } { + if _, err := module.ProcessProviderFeedback(f.ctx, x); err != nil { + t.Fatal(err) + } + } + if _, err := f.pool.Exec(f.ctx, `UPDATE sending_feedback_correlations SET expires_at = now() - interval '1 hour' WHERE correlation_id = $1`, oldCorr); err != nil { + t.Fatal(err) + } + if _, err := f.pool.Exec(f.ctx, `UPDATE sending_feedback_events SET expires_at = now() - interval '1 hour' WHERE correlation_id = $1`, oldCorr); err != nil { + t.Fatal(err) + } + if _, err := f.pool.Exec(f.ctx, `INSERT INTO account_sending_outcomes_daily (user_id, outcome_epoch, day, shared_reputation, delivered_count) VALUES ($1, 1, current_date - 30, true, 5)`, user); err != nil { + t.Fatal(err) + } + + st, err := module.GCFeedback(f.ctx, time.Now(), 7) + if err != nil { + t.Fatal(err) + } + if st.Correlations != 1 || st.Recipients != 1 || st.Events != 1 || st.Outcomes != 1 { + t.Fatalf("gc stats = %+v", st) + } + var n int + if err := f.pool.QueryRow(f.ctx, `SELECT count(*) FROM sending_feedback_correlations WHERE correlation_id IN ($1, $2)`, liveCorr, oldCorr).Scan(&n); err != nil { + t.Fatal(err) + } + if n != 1 { + t.Fatalf("correlations left = %d, want the live one only", n) + } + if got := f.outcomes(user); len(got) != 1 || got[todayKey(1, true)] != [4]int{2, 0, 0, 0} { + t.Fatalf("aggregates after gc = %v", got) + } +} + +// TestEqualRankProvenanceTieBreak: an equal-rank duplicate never changes +// counts, and the row remembers the LATER provider event — an earlier +// straggler must not overwrite the provenance of the evidence already +// recorded. +func TestEqualRankProvenanceTieBreak(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + rcpt := []string{"tie@example.test"} + msg := f.messageTo(agent, "relay", rcpt) + _, corrID, sesID := f.authorizedSend(g, msg, rcpt) + // Microsecond precision, because that is what timestamptz stores: a + // nanosecond-granular Go clock (Linux) would round-trip to a different + // value than the one asserted, while a microsecond-granular one (macOS) + // would not — the test must not depend on which host runs it. + base := time.Now().UTC().Truncate(time.Microsecond).Add(-time.Hour) + + send := func(id string, at time.Time) { + t.Helper() + if _, err := module.ProcessProviderFeedback(f.ctx, feedback(id, at, delivery.KindDelivery, sesID, corrID, rcpt[0])); err != nil { + t.Fatal(err) + } + } + send("mid", base) + if id, at := f.provenance(corrID); id != "mid" || !at.Equal(base) { + t.Fatalf("provenance = %s/%v", id, at) + } + // A later equal-rank event advances provenance, counts unchanged. + send("late", base.Add(time.Minute)) + if id, at := f.provenance(corrID); id != "late" || !at.Equal(base.Add(time.Minute)) { + t.Fatalf("later equal-rank event must own provenance, got %s/%v", id, at) + } + // An earlier straggler does not. + send("early", base.Add(-time.Minute)) + if id, _ := f.provenance(corrID); id != "late" { + t.Fatalf("an earlier equal-rank event overwrote provenance: %s", id) + } + if c := f.outcomes(user)[todayKey(1, true)]; c != [4]int{1, 0, 0, 0} { + t.Fatalf("equal-rank events changed counts: %v", c) + } +} + +// TestSuppressionRepairRequiresALiveAccount: the fallback writer refuses to +// recreate customer state for an account that no longer exists. +func TestSuppressionRepairRequiresALiveAccount(t *testing.T) { + f := newFixture(t) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + if _, err := module.RepairSuppressions(f.ctx, "usr_does_not_exist", + []delivery.FeedbackRepair{{Address: "x@example.test", Source: "bounce", Reason: "bounce:General"}}); err != nil { + t.Fatalf("a deleted account must be a no-op, not an error: %v", err) + } + var n int + if err := f.pool.QueryRow(f.ctx, `SELECT count(*) FROM suppressions WHERE address = 'x@example.test'`).Scan(&n); err != nil { + t.Fatal(err) + } + if n != 0 { + t.Fatal("repair wrote a suppression for a nonexistent account") + } +} + +// TestNotificationFeedbackAccountsButDoesNotRepair: a bounce on platform +// mail the account merely triggered — an approval notice — still feeds the +// detector on the shared path, because the spec counts customer-triggered +// notifications. It must NOT suppress the address, which is the account +// owner's own: doing so would block the customer's sends to it, an effect +// the pre-B8 path never had. +func TestNotificationFeedbackAccountsButDoesNotRepair(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + held := f.pendingMessage(agent, "own_address") + + var ref sendingpolicy.OperationRef + f.inTx(func(tx pgx.Tx) error { + var err error + ref, err = g.PrepareNotificationTx(f.ctx, tx, sendingpolicy.NewHITLNotificationRef(held)) + return err + }) + _, attempt, err := g.Reserve(f.ctx, ref) + if err != nil { + t.Fatal(err) + } + _, token, err := g.ConsumeAttempt(f.ctx, attempt) + if err != nil || token == nil { + t.Fatalf("consume notification: %v", err) + } + owner := token.AuthorizedRecipients() + headers, err := token.ValidateEnvelope(owner) + if err != nil { + t.Fatal(err) + } + + res, err := module.ProcessProviderFeedback(f.ctx, delivery.ProviderFeedback{ + ProviderEventID: "notify-hb", OccurredAt: time.Now().UTC(), Kind: delivery.KindBounce, + BounceType: "permanent", BounceSubType: "General", + AttemptCorrelationID: headers.AttemptCorrelationID, Recipients: owner, + }) + if err != nil { + t.Fatal(err) + } + if !res.Correlated || res.AccountRef != user { + t.Fatalf("notification feedback must attribute to the triggering account: %+v", res) + } + if len(res.RepairNeeded) != 0 { + t.Fatalf("platform mail must not suppress the owner's address: %+v", res.RepairNeeded) + } + // It still counts, on the shared path. + if c := f.outcomes(user)[todayKey(1, true)]; c != [4]int{0, 0, 1, 0} { + t.Fatalf("notification bounce must feed the detector: %v", c) + } + f.repair(module, res) + var n int + if err := f.pool.QueryRow(f.ctx, `SELECT count(*) FROM suppressions WHERE user_id = $1`, user).Scan(&n); err != nil { + t.Fatal(err) + } + if n != 0 { + t.Fatalf("suppressions created for platform mail: %d", n) + } +} diff --git a/internal/sendingpolicy/feedback_review_integration_test.go b/internal/sendingpolicy/feedback_review_integration_test.go new file mode 100644 index 000000000..ba829019b --- /dev/null +++ b/internal/sendingpolicy/feedback_review_integration_test.go @@ -0,0 +1,719 @@ +package sendingpolicy_test + +import ( + "context" + "errors" + "sync" + "testing" + "time" + + "github.com/tokencanopy/e2a/internal/delivery" + "github.com/tokencanopy/e2a/internal/identity" + "github.com/tokencanopy/e2a/internal/sendingpolicy" + "github.com/tokencanopy/e2a/migrations" +) + +// recordFeedbackMetrics installs a process-wide observer for the test and +// returns the samples it saw. Tests using it must not run in parallel. +func recordFeedbackMetrics(t *testing.T) func() map[string]int { + t.Helper() + var mu sync.Mutex + seen := map[string]int{} + sendingpolicy.SetFeedbackObserver(func(outcome, bucket string) { + mu.Lock() + defer mu.Unlock() + seen[outcome+"/"+bucket]++ + }) + t.Cleanup(func() { sendingpolicy.SetFeedbackObserver(nil) }) + return func() map[string]int { + mu.Lock() + defer mu.Unlock() + out := make(map[string]int, len(seen)) + for k, v := range seen { + out[k] = v + } + return out + } +} + +func (f *fixture) correlationExpiry(correlationID string) *time.Time { + f.t.Helper() + var at *time.Time + if err := f.pool.QueryRow(f.ctx, `SELECT expires_at FROM sending_feedback_correlations WHERE correlation_id = $1`, correlationID).Scan(&at); err != nil { + f.t.Fatalf("correlation expiry: %v", err) + } + return at +} + +func (f *fixture) eventExpiry(eventID string) *time.Time { + f.t.Helper() + var at *time.Time + if err := f.pool.QueryRow(f.ctx, `SELECT expires_at FROM sending_feedback_events WHERE provider_event_id = $1`, eventID).Scan(&at); err != nil { + f.t.Fatalf("event expiry: %v", err) + } + return at +} + +// provenanceRows counts the three B8 tables' rows for one correlation. +func (f *fixture) provenanceRows(correlationID string) (correlations, recipients, events int) { + f.t.Helper() + if err := f.pool.QueryRow(f.ctx, ` + SELECT (SELECT count(*) FROM sending_feedback_correlations WHERE correlation_id = $1), + (SELECT count(*) FROM sending_feedback_recipients WHERE correlation_id = $1), + (SELECT count(*) FROM sending_feedback_events WHERE correlation_id = $1)`, correlationID, + ).Scan(&correlations, &recipients, &events); err != nil { + f.t.Fatal(err) + } + return +} + +func (f *fixture) waitForLockWaiter() { + f.t.Helper() + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + var n int + if err := f.pool.QueryRow(f.ctx, ` + SELECT count(*) FROM pg_stat_activity + WHERE datname = current_database() AND wait_event_type = 'Lock'`).Scan(&n); err != nil { + f.t.Fatal(err) + } + if n > 0 { + return + } + time.Sleep(20 * time.Millisecond) + } + f.t.Fatal("the concurrent feedback transaction never blocked on the seal") +} + +// TestFeedbackRacingThePurgeSealInheritsTheSealExpiry is the NULL-expiry +// race: the purge seal stamps the account's correlations and events, deletes +// the user, and is still open when a notification for that account arrives. +// The notification must end up with the seal's expiry on its event row (not +// NULL, which no janitor would ever remove), and the GC past the horizon +// must empty all three tables. +func TestFeedbackRacingThePurgeSealInheritsTheSealExpiry(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + msg := f.messageTo(agent, "relay", []string{"race@example.test"}) + _, corrID, sesID := f.authorizedSend(g, msg, []string{"race@example.test"}) + + // The seal's shape: stamp correlations, stamp events, delete the user — + // and hold the transaction open. + seal, err := f.pool.Begin(f.ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = seal.Rollback(f.ctx) }() + horizon := time.Now().UTC().Add(30 * 24 * time.Hour).Truncate(time.Microsecond) + if _, err := seal.Exec(f.ctx, `UPDATE sending_feedback_correlations SET expires_at = $2 WHERE source_account_ref = $1 AND expires_at IS NULL`, user, horizon); err != nil { + t.Fatal(err) + } + if _, err := seal.Exec(f.ctx, ` + UPDATE sending_feedback_events e SET expires_at = c.expires_at + FROM sending_feedback_correlations c + WHERE c.correlation_id = e.correlation_id AND c.source_account_ref = $1 AND e.expires_at IS NULL`, user); err != nil { + t.Fatal(err) + } + if _, err := seal.Exec(f.ctx, `DELETE FROM users WHERE id = $1`, user); err != nil { + t.Fatal(err) + } + + type outcome struct { + res delivery.FeedbackResult + err error + } + done := make(chan outcome, 1) + go func() { + res, err := module.ProcessProviderFeedback(context.Background(), + feedback("race-evt", time.Now().UTC(), delivery.KindComplaint, sesID, "", "race@example.test")) + done <- outcome{res, err} + }() + f.waitForLockWaiter() + if err := seal.Commit(f.ctx); err != nil { + t.Fatal(err) + } + var got outcome + select { + case got = <-done: + case <-time.After(15 * time.Second): + t.Fatal("feedback never finished after the seal committed") + } + if got.err != nil { + t.Fatalf("feedback: %v", got.err) + } + if !got.res.Correlated || got.res.AccountRef != "" { + t.Fatalf("result = %+v, want correlated provenance-only", got.res) + } + + corrExp, evtExp := f.correlationExpiry(corrID), f.eventExpiry("race-evt") + if corrExp == nil || evtExp == nil || !evtExp.Equal(*corrExp) { + t.Fatalf("event expiry = %v, want the correlation's %v", evtExp, corrExp) + } + if _, err := module.GCFeedback(f.ctx, corrExp.Add(time.Minute), 7); err != nil { + t.Fatal(err) + } + if c, r, e := f.provenanceRows(corrID); c+r+e != 0 { + t.Fatalf("after the horizon: correlations=%d recipients=%d events=%d, want none", c, r, e) + } +} + +// TestGCRemovesEventsByCorrelationEvenWithoutTheirOwnExpiry: an event row +// that a pre-fix race left with a NULL expiry still goes when its +// correlation does, and the reconcile sweep removes an event whose +// correlation is already gone. +func TestGCRemovesEventsByCorrelationEvenWithoutTheirOwnExpiry(t *testing.T) { + f := newFixture(t) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + f.exec(` + INSERT INTO sending_feedback_correlations + (correlation_id, operation_id, submission_attempt, source_account_ref, policy_subject_ref, purpose, shared_reputation, tenant_mode, expires_at) + VALUES ('cor_nullevt', 'op_nullevt', 1, 'usr_gone_1', 'usr_gone_1', 'customer_message', true, 'none', now() - interval '1 minute')`) + f.exec(`INSERT INTO sending_feedback_recipients (correlation_id, recipient_hmac, hmac_key_version) VALUES ('cor_nullevt', '\xaa', 1)`) + f.exec(`INSERT INTO sending_feedback_events (provider_event_id, correlation_id, provider_occurred_at, expires_at) + VALUES ('evt_null_1', 'cor_nullevt', now(), NULL), + ('evt_orphan_1', 'cor_never_existed', now(), NULL)`) + + st, err := module.GCFeedback(f.ctx, time.Now(), 7) + if err != nil { + t.Fatal(err) + } + if st.Correlations != 1 || st.Recipients != 1 || st.Events != 1 { + t.Fatalf("gc stats = %+v, want the correlation with its recipient and its NULL-expiry event", st) + } + if c, r, e := f.provenanceRows("cor_nullevt"); c+r+e != 0 { + t.Fatalf("left behind: correlations=%d recipients=%d events=%d", c, r, e) + } + + rs, err := module.ReconcileFeedbackRetention(f.ctx, time.Now(), 30*24*time.Hour) + if err != nil { + t.Fatal(err) + } + if rs.OrphanEvents != 1 { + t.Fatalf("reconcile = %+v, want the one orphaned event swept", rs) + } + var n int + if err := f.pool.QueryRow(f.ctx, `SELECT count(*) FROM sending_feedback_events WHERE provider_event_id = 'evt_orphan_1'`).Scan(&n); err != nil { + t.Fatal(err) + } + if n != 0 { + t.Fatal("orphaned event survived the sweep") + } +} + +// seedOrphanProvenance lays down the backfill cases: a customer correlation +// of an account erased before B8 (and its NULL-expiry event), a live +// account's, a trashed account's, and a non-customer correlation that +// already carries its own horizon. +func (f *fixture) seedOrphanProvenance() (live, trashed string) { + f.t.Helper() + live, trashed = f.user("standard"), f.user("standard") + f.trash(trashed) + f.exec(` + INSERT INTO sending_feedback_correlations + (correlation_id, operation_id, submission_attempt, source_account_ref, policy_subject_ref, purpose, shared_reputation, tenant_mode, expires_at) + VALUES ('cor_bf_erased', 'op_bf_1', 1, 'usr_erased_before_b8', 'usr_erased_before_b8', 'customer_message', true, 'none', NULL), + ('cor_bf_notice', 'op_bf_2', 1, 'usr_erased_before_b8', 'usr_erased_before_b8', 'customer_notification', true, 'none', NULL), + ('cor_bf_live', 'op_bf_3', 1, $1, $1, 'customer_message', true, 'none', NULL), + ('cor_bf_trash', 'op_bf_4', 1, $2, $2, 'customer_message', true, 'none', NULL), + ('cor_bf_system', 'op_bf_5', 1, NULL, 'system', 'critical_operational', true, 'none', now() + interval '3 days')`, + live, trashed) + f.exec(`INSERT INTO sending_feedback_events (provider_event_id, correlation_id, provider_occurred_at, expires_at) + VALUES ('evt_bf_erased', 'cor_bf_erased', now(), NULL), + ('evt_bf_live', 'cor_bf_live', now(), NULL)`) + return live, trashed +} + +func (f *fixture) assertBackfilled(label string) { + f.t.Helper() + lo, hi := time.Now().Add(29*24*time.Hour), time.Now().Add(31*24*time.Hour) + for _, id := range []string{"cor_bf_erased", "cor_bf_notice"} { + at := f.correlationExpiry(id) + if at == nil || at.Before(lo) || at.After(hi) { + f.t.Fatalf("%s: %s expiry = %v, want ~30 days out", label, id, at) + } + } + if at, want := f.eventExpiry("evt_bf_erased"), f.correlationExpiry("cor_bf_erased"); at == nil || !at.Equal(*want) { + f.t.Fatalf("%s: erased account's event expiry = %v, want the correlation's %v", label, at, want) + } + for _, id := range []string{"cor_bf_live", "cor_bf_trash"} { + if at := f.correlationExpiry(id); at != nil { + f.t.Fatalf("%s: %s (account still has a users row) was stamped: %v", label, id, at) + } + } + if at := f.eventExpiry("evt_bf_live"); at != nil { + f.t.Fatalf("%s: a live account's event was stamped: %v", label, at) + } + if at := f.correlationExpiry("cor_bf_system"); at == nil || at.After(time.Now().Add(4*24*time.Hour)) { + f.t.Fatalf("%s: a non-customer correlation's own horizon was rewritten: %v", label, at) + } +} + +// TestMigration124BackfillsOrphanedCustomerProvenance: the one-time backlog +// pass stamps exactly the rows of accounts that no longer exist, and is +// idempotent. +func TestMigration124BackfillsOrphanedCustomerProvenance(t *testing.T) { + f := newFixture(t) + f.seedOrphanProvenance() + sql, err := migrations.FS.ReadFile("124_sending_feedback_orphan_retention.sql") + if err != nil { + t.Fatal(err) + } + if _, err := f.pool.Exec(f.ctx, string(sql)); err != nil { + t.Fatalf("apply 124: %v", err) + } + f.assertBackfilled("first application") + first := f.correlationExpiry("cor_bf_erased") + + if _, err := f.pool.Exec(f.ctx, string(sql)); err != nil { + t.Fatalf("re-apply 124: %v", err) + } + f.assertBackfilled("second application") + if again := f.correlationExpiry("cor_bf_erased"); !again.Equal(*first) { + t.Fatalf("re-applying moved an already-stamped expiry: %v -> %v", first, again) + } +} + +// TestReconcileJobStampsOrphanedProvenanceFromTheEffectivePolicy: the daily +// backstop runs the same predicate as migration 124, with the horizon read +// from the effective policy. +func TestReconcileJobStampsOrphanedProvenanceFromTheEffectivePolicy(t *testing.T) { + f := newFixture(t) + f.seedOrphanProvenance() + short := sendingpolicy.DisabledPolicy() + short.SendingFeedbackPostAcctRetention = 5 + // Database source: the singleton's retention (the 30-day default), not + // the config's 5, is what the job must stamp. + module := sendingpolicy.NewPolicyModule(f.pool, f.secrets(), sendingpolicy.PolicySourceDatabase, short) + if err := sendingpolicy.NewFeedbackReconcileWorker(module).Work(f.ctx, nil); err != nil { + t.Fatalf("reconcile: %v", err) + } + f.assertBackfilled("reconcile") +} + +// TestLateComplaintAfterRealAccountEraseIsProvenanceOnly drives the REAL +// deletion path (identity.EraseAccount → purge seal) instead of a raw +// DELETE FROM users: the seal stamps the horizon from the effective policy, +// a late complaint then lands as provenance only — its event carrying the +// correlation's expiry, no aggregate row, no customer state — and the GC +// past the horizon removes all of it. +func TestLateComplaintAfterRealAccountEraseIsProvenanceOnly(t *testing.T) { + f := newFixture(t) + metrics := recordFeedbackMetrics(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + msg := f.messageTo(agent, "relay", []string{"late@example.test"}) + _, corrID, sesID := f.authorizedSend(g, msg, []string{"late@example.test"}) + + // The purge reads its horizon from a DATABASE-source module whose + // config-file policy says 5 days, while the activated (effective) policy + // says 45 — neither is the 30-day default, so the stamp proves which + // source the seal read. + short := sendingpolicy.DisabledPolicy() + short.SendingFeedbackPostAcctRetention = 5 + dbSourced := sendingpolicy.NewPolicyModule(f.pool, f.secrets(), sendingpolicy.PolicySourceDatabase, short) + f.activateRetention(dbSourced, 45) + store := identity.NewStore(f.pool) + store.SetFeedbackRetentionResolver(dbSourced.EffectiveFeedbackRetention) + if _, err := store.EraseAccount(f.ctx, user, nil); err != nil { + t.Fatalf("EraseAccount: %v", err) + } + var users int + if err := f.pool.QueryRow(f.ctx, `SELECT count(*) FROM users WHERE id = $1`, user).Scan(&users); err != nil { + t.Fatal(err) + } + if users != 0 { + t.Fatal("EraseAccount must purge the user row") + } + corrExp := f.correlationExpiry(corrID) + if corrExp == nil || corrExp.Before(time.Now().Add(44*24*time.Hour)) || corrExp.After(time.Now().Add(46*24*time.Hour)) { + t.Fatalf("seal stamped %v, want ~45 days (the effective policy), not the config's 5 or the default 30", corrExp) + } + + res, err := module.ProcessProviderFeedback(f.ctx, feedback("late-evt", time.Now().UTC(), delivery.KindComplaint, sesID, "", "late@example.test")) + if err != nil { + t.Fatal(err) + } + if !res.Correlated || res.AccountRef != "" || len(res.RepairNeeded) != 0 { + t.Fatalf("result = %+v, want provenance only", res) + } + if evtExp := f.eventExpiry("late-evt"); evtExp == nil || !evtExp.Equal(*corrExp) { + t.Fatalf("event expiry = %v, want the correlation's %v", evtExp, corrExp) + } + if got := f.outcomes(user); len(got) != 0 { + t.Fatalf("an erased account must have no aggregate rows: %v", got) + } + if b, _, epoch := f.recipientRow(corrID); b != "complaint" || epoch != nil { + t.Fatalf("provenance = %s epoch=%v, want complaint with no epoch", b, epoch) + } + if got := metrics()["dead_account/complaint"]; got != 1 { + t.Fatalf("metrics = %v, want one dead_account/complaint sample", metrics()) + } + + if _, err := module.GCFeedback(f.ctx, corrExp.Add(time.Minute), 7); err != nil { + t.Fatal(err) + } + if c, r, e := f.provenanceRows(corrID); c+r+e != 0 { + t.Fatalf("after the horizon: correlations=%d recipients=%d events=%d, want none", c, r, e) + } +} + +// activateRetention activates a database policy whose post-account +// feedback retention is `days`. +func (f *fixture) activateRetention(m *sendingpolicy.Module, days int) { + f.t.Helper() + before, err := m.InspectPolicy(f.ctx) + if err != nil { + f.t.Fatalf("inspect policy: %v", err) + } + next := before.Policy + next.SendingFeedbackPostAcctRetention = days + if _, err := m.ActivatePolicy(f.ctx, sendingpolicy.ActivationRequest{ + ExpectedGeneration: before.Generation, Policy: next, + Actor: "integration-test", Reason: "non-default feedback retention", + }); err != nil { + f.t.Fatalf("activate policy: %v", err) + } +} + +// TestPurgeRetentionResolverErrorLeavesTheAccountUnpurged: a seal that +// cannot resolve its horizon must not stamp a guess — it fails, the user row +// survives for the purge's next pass, and nothing is stamped. +func TestPurgeRetentionResolverErrorLeavesTheAccountUnpurged(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + user := f.user("standard") + agent := f.agent(user) + msg := f.messageTo(agent, "relay", []string{"unresolved@example.test"}) + _, corrID, _ := f.authorizedSend(g, msg, []string{"unresolved@example.test"}) + + store := identity.NewStore(f.pool) + store.SetFeedbackRetentionResolver(func(context.Context) (time.Duration, error) { + return 0, errors.New("policy store unavailable") + }) + if _, err := store.EraseAccount(f.ctx, user, nil); err == nil { + t.Fatal("EraseAccount must fail when the retention horizon cannot be resolved") + } + var users int + if err := f.pool.QueryRow(f.ctx, `SELECT count(*) FROM users WHERE id = $1`, user).Scan(&users); err != nil { + t.Fatal(err) + } + if users != 1 { + t.Fatal("the user row must survive a seal that could not resolve its horizon") + } + if at := f.correlationExpiry(corrID); at != nil { + t.Fatalf("nothing may be stamped without a resolved horizon, got %v", at) + } +} + +// TestLegacyRawFormHMACStillMatches: a recipient row signed before +// canonicalization (over the raw Unicode spelling, which differs from the +// canonical A-label form) is still matched through the raw-form fallback. +func TestLegacyRawFormHMACStillMatches(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + raw := "alt@bücher.example" + msg := f.messageTo(agent, "relay", []string{raw}) + _, corrID, sesID := f.authorizedSend(g, msg, []string{raw}) + + keyring, err := sendingpolicy.LoadKeyring(fxHMAC) + if err != nil { + t.Fatal(err) + } + version, legacy := keyring.Sign([]byte(raw)) + _, canonical := keyring.Sign([]byte("alt@xn--bcher-kva.example")) + if string(legacy) == string(canonical) { + t.Fatal("fixture must sign two distinct subjects") + } + f.exec(`UPDATE sending_feedback_recipients SET recipient_hmac = $2, hmac_key_version = $3 WHERE correlation_id = $1`, corrID, legacy, version) + + if _, err := module.ProcessProviderFeedback(f.ctx, feedback("legacy-1", time.Now().UTC(), delivery.KindDelivery, sesID, "", raw)); err != nil { + t.Fatal(err) + } + if b, _, _ := f.recipientRow(corrID); b != "delivered" { + t.Fatalf("a legacy raw-form row must still match: bucket %s", b) + } +} + +// TestFeedbackMetricCarriesAppliedBucketAndOnlyAfterCommit: the bucket label +// is the one actually APPLIED (a simulator delivery is correlated/none, not +// delivered), and a pass whose transaction rolls back emits nothing. +func TestFeedbackMetricCarriesAppliedBucketAndOnlyAfterCommit(t *testing.T) { + f := newFixture(t) + metrics := recordFeedbackMetrics(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + sim := "bounce-check@simulator.amazonses.com" + msg := f.messageTo(agent, "relay", []string{sim}) + _, _, sesID := f.authorizedSend(g, msg, []string{sim}) + if _, err := module.ProcessProviderFeedback(f.ctx, feedback("applied-1", time.Now().UTC(), delivery.KindDelivery, sesID, "", sim)); err != nil { + t.Fatal(err) + } + if got := metrics(); len(got) != 1 || got["correlated/none"] != 1 { + t.Fatalf("metrics = %v, want exactly one correlated/none sample", got) + } + + // Rollback: the simulator recipient is processed first (a sample is + // collected, no aggregate write); the ordinary one's aggregate insert + // then fails, rolling the whole pass back. + f.exec(` + CREATE OR REPLACE FUNCTION fx_fail_outcome_insert() RETURNS trigger LANGUAGE plpgsql AS $$ + BEGIN RAISE EXCEPTION 'fixture: aggregate write refused'; END $$`) + f.exec(`CREATE TRIGGER fx_fail_outcome_insert BEFORE INSERT ON account_sending_outcomes_daily + FOR EACH ROW EXECUTE FUNCTION fx_fail_outcome_insert()`) + t.Cleanup(func() { + _, _ = f.pool.Exec(context.Background(), `DROP TRIGGER IF EXISTS fx_fail_outcome_insert ON account_sending_outcomes_daily`) + _, _ = f.pool.Exec(context.Background(), `DROP FUNCTION IF EXISTS fx_fail_outcome_insert()`) + }) + both := []string{sim, "ordinary@example.test"} + msg2 := f.messageTo(agent, "relay", both) + _, _, ses2 := f.authorizedSend(g, msg2, both) + before := metrics() + if _, err := module.ProcessProviderFeedback(f.ctx, feedback("rollback-1", time.Now().UTC(), delivery.KindDelivery, ses2, "", both...)); err == nil { + t.Fatal("the fixture trigger must fail the pass") + } + if after := metrics(); len(after) != len(before) || after["correlated/none"] != before["correlated/none"] { + t.Fatalf("a rolled-back pass emitted samples: before=%v after=%v", before, after) + } +} + +// TestProvenanceRepairInALabelBlocksAUnicodeTypedSend: SES reports an IDN +// recipient in A-label form, so that is the spelling the repair stores; the +// send-time suppression lookup must still block the customer's next send to +// the Unicode spelling they typed. +func TestProvenanceRepairInALabelBlocksAUnicodeTypedSend(t *testing.T) { + f := newFixture(t) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + if _, err := module.RepairSuppressions(f.ctx, user, []delivery.FeedbackRepair{ + {Address: "leser@xn--bcher-kva.example", Source: "bounce", Reason: "bounce:General"}, + }); err != nil { + t.Fatal(err) + } + store := identity.NewStore(f.pool) + typed := []string{"Leser@Bücher.example"} + eff, err := store.EffectiveSuppressions(f.ctx, user, agent, typed) + if err != nil || len(eff) != 1 { + t.Fatalf("EffectiveSuppressions(%v) = %v (err %v), want the A-label row", typed, eff, err) + } + acct, err := store.SuppressedAddresses(f.ctx, user, typed) + if err != nil || len(acct) != 1 { + t.Fatalf("SuppressedAddresses(%v) = %v (err %v), want the A-label row", typed, acct, err) + } + if other, err := store.EffectiveSuppressions(f.ctx, user, agent, []string{"leser@bucher.example"}); err != nil || len(other) != 0 { + t.Fatalf("a different domain matched: %v (err %v)", other, err) + } +} + +// TestFeedbackIngestionMetrics pins the bounded outcome × bucket samples: +// correlated per matched recipient, unmatched_recipient per stranger, +// duplicate per replay, and the two uncorrelated flavours — the one +// carrying e2a's attempt marker being the alerting signal. +func TestFeedbackIngestionMetrics(t *testing.T) { + f := newFixture(t) + metrics := recordFeedbackMetrics(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + msg := f.messageTo(agent, "relay", []string{"m1@example.test"}) + _, _, sesID := f.authorizedSend(g, msg, []string{"m1@example.test"}) + at := time.Now().UTC() + + for _, fb := range []delivery.ProviderFeedback{ + feedback("mx-1", at, delivery.KindBounce, sesID, "", "m1@example.test", "stranger@example.test"), + feedback("mx-1", at, delivery.KindBounce, sesID, "", "m1@example.test"), + feedback("mx-2", at, delivery.KindDelivery, "", "cor_00aa11bb", "m1@example.test"), + feedback("mx-3", at, delivery.KindDelivery, "", "", "m1@example.test"), + } { + if fb.Kind == delivery.KindBounce { + fb.BounceType = "permanent" + } + if _, err := module.ProcessProviderFeedback(f.ctx, fb); err != nil { + t.Fatal(err) + } + } + want := map[string]int{ + "correlated/hard_bounce": 1, + "unmatched_recipient/hard_bounce": 1, + "duplicate/hard_bounce": 1, + "uncorrelated_with_marker/delivered": 1, + "uncorrelated/delivered": 1, + } + got := metrics() + for k, v := range want { + if got[k] != v { + t.Errorf("%s = %d, want %d (all: %v)", k, got[k], v, got) + } + } + if len(got) != len(want) { + t.Errorf("unexpected samples: %v", got) + } +} + +// TestSelfAddressedDeliveriesDoNotDiluteTheDenominator: deliveries to the +// SES simulator, the configured shared agent domain, a platform-owned shared +// domain row, and the sender's own verified domain are all mail a sender can +// manufacture; none may count toward the denominator. A delivery to an +// ordinary recipient counts, and a complaint from an excluded recipient is +// still evidence against the sender. +func TestSelfAddressedDeliveriesDoNotDiluteTheDenominator(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()).WithFeedbackExcludedDomains("Agents.Localhost") + user := f.user("standard") + agent := f.agent(user) + f.customDomain(user, "own.example.test", "verified") + f.exec(`INSERT INTO domains (domain, user_id, verified, verified_at) VALUES ('platform-shared.example.test', NULL, true, now())`) + other := f.user("standard") + f.customDomain(other, "someone-else.example.test", "verified") + + excluded := []string{ + "success@simulator.amazonses.com", + "peer@agents.localhost", + "self@own.example.test", + "peer@platform-shared.example.test", + } + counted := []string{"customer@example.test", "partner@someone-else.example.test"} + all := append(append([]string{}, excluded...), counted...) + msg := f.messageTo(agent, "relay", all) + _, corrID, sesID := f.authorizedSend(g, msg, all) + + res, err := module.ProcessProviderFeedback(f.ctx, feedback("dil-1", time.Now().UTC(), delivery.KindDelivery, sesID, "", all...)) + if err != nil || !res.Correlated { + t.Fatalf("delivery: %+v %v", res, err) + } + if c := f.outcomes(user)[todayKey(1, true)]; c != [4]int{2, 0, 0, 0} { + t.Fatalf("aggregate = %v, want only the two ordinary recipients delivered", c) + } + var none int + if err := f.pool.QueryRow(f.ctx, `SELECT count(*) FROM sending_feedback_recipients WHERE correlation_id = $1 AND detector_bucket = 'none'`, corrID).Scan(&none); err != nil { + t.Fatal(err) + } + if none != len(excluded) { + t.Fatalf("recipients left in bucket none = %d, want %d", none, len(excluded)) + } + + // A soft (terminal_other) bounce from an excluded recipient is equally + // denominator-only and equally excluded. + soft := feedback("dil-2", time.Now().UTC(), delivery.KindBounce, sesID, "", "success@simulator.amazonses.com") + soft.BounceType = "transient" + if _, err := module.ProcessProviderFeedback(f.ctx, soft); err != nil { + t.Fatal(err) + } + if c := f.outcomes(user)[todayKey(1, true)]; c != [4]int{2, 0, 0, 0} { + t.Fatalf("aggregate after an excluded soft bounce = %v, want terminal_other still 0", c) + } + // Complaints and hard bounces from every excluded class still count. + for i, addr := range excluded { + kind := delivery.KindComplaint + fb := feedback("dil-c-"+addr, time.Now().UTC(), kind, sesID, "", addr) + if i%2 == 1 { + fb.Kind, fb.BounceType = delivery.KindBounce, "permanent" + } + if _, err := module.ProcessProviderFeedback(f.ctx, fb); err != nil { + t.Fatal(err) + } + } + if c := f.outcomes(user)[todayKey(1, true)]; c != [4]int{2, 0, 2, 2} { + t.Fatalf("aggregate = %v, want delivered 2, hard bounces 2, complaints 2", c) + } +} + +// TestInternationalizedRecipientDomainMatchesAcrossSpellings: authorization +// accepts an IDN recipient as typed (Unicode) while the provider may report +// it in A-label (punycode) form. Both sides HMAC the IDNA-ASCII canonical +// form, so feedback in either spelling matches the authorized envelope. +func TestInternationalizedRecipientDomainMatchesAcrossSpellings(t *testing.T) { + f := newFixture(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + unicode := "leser@bücher.example" + msg := f.messageTo(agent, "relay", []string{unicode}) + _, corrID, sesID := f.authorizedSend(g, msg, []string{unicode}) + + res, err := module.ProcessProviderFeedback(f.ctx, feedback("idn-1", time.Now().UTC(), delivery.KindDelivery, sesID, "", "leser@xn--bcher-kva.example")) + if err != nil || !res.Correlated { + t.Fatalf("punycode feedback: %+v %v", res, err) + } + if b, _, _ := f.recipientRow(corrID); b != "delivered" { + t.Fatalf("punycode spelling did not match the Unicode authorization: bucket %s", b) + } + c := feedback("idn-2", time.Now().UTC(), delivery.KindComplaint, sesID, "", "LESER@BÜCHER.example") + if _, err := module.ProcessProviderFeedback(f.ctx, c); err != nil { + t.Fatal(err) + } + if b, _, _ := f.recipientRow(corrID); b != "complaint" { + t.Fatalf("Unicode spelling did not match: bucket %s", b) + } +} + +// TestRepairReportsOnlyInsertedRows: the provenance repair refreshes an +// existing suppression (manual or bounce) but reports only rows it actually +// inserted, which is what the consumer announces. +func TestRepairReportsOnlyInsertedRows(t *testing.T) { + f := newFixture(t) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + f.exec(`INSERT INTO suppressions (id, user_id, address, reason, source) VALUES ('supp_manual_fx', $1, 'manual@example.test', 'customer request', 'manual')`, user) + inserted, err := module.RepairSuppressions(f.ctx, user, []delivery.FeedbackRepair{ + {Address: "manual@example.test", Source: "bounce", Reason: "bounce:General"}, + {Address: "fresh@example.test", Source: "bounce", Reason: "bounce:General"}, + }) + if err != nil { + t.Fatal(err) + } + if len(inserted) != 1 || inserted[0].Address != "fresh@example.test" { + t.Fatalf("inserted = %+v, want only the fresh address", inserted) + } + var source string + if err := f.pool.QueryRow(f.ctx, `SELECT source FROM suppressions WHERE user_id = $1 AND address = 'manual@example.test'`, user).Scan(&source); err != nil { + t.Fatal(err) + } + if source != "manual" { + t.Fatalf("the manual row was rewritten to %q", source) + } + again, err := module.RepairSuppressions(f.ctx, user, []delivery.FeedbackRepair{{Address: "fresh@example.test", Source: "bounce", Reason: "bounce:General"}}) + if err != nil || len(again) != 0 { + t.Fatalf("a repeated repair reported %+v (err %v), want nothing", again, err) + } +} + +// TestExpiredCorrelationIsPastRetentionNotLost: a correlation past its +// horizon is never matched (the janitor may be deleting it), and feedback +// carrying its marker counts as plain uncorrelated — retention working — +// not as the lost-correlation alert. +func TestExpiredCorrelationIsPastRetentionNotLost(t *testing.T) { + f := newFixture(t) + metrics := recordFeedbackMetrics(t) + g := f.gate(sendingpolicy.DisabledPolicy()) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + user := f.user("standard") + agent := f.agent(user) + msg := f.messageTo(agent, "relay", []string{"aged@example.test"}) + _, corrID, sesID := f.authorizedSend(g, msg, []string{"aged@example.test"}) + f.exec(`UPDATE sending_feedback_correlations SET expires_at = now() - interval '1 minute' WHERE correlation_id = $1`, corrID) + + res, err := module.ProcessProviderFeedback(f.ctx, feedback("aged-1", time.Now().UTC(), delivery.KindComplaint, sesID, corrID, "aged@example.test")) + if err != nil { + t.Fatal(err) + } + if res.Correlated { + t.Fatal("an expired correlation must not be matched") + } + if got := metrics(); got["uncorrelated/complaint"] != 1 || got["uncorrelated_with_marker/complaint"] != 0 { + t.Fatalf("metrics = %v, want uncorrelated (past retention), not the alert", got) + } +} diff --git a/internal/sendingpolicy/feedback_test.go b/internal/sendingpolicy/feedback_test.go new file mode 100644 index 000000000..8d93264d6 --- /dev/null +++ b/internal/sendingpolicy/feedback_test.go @@ -0,0 +1,61 @@ +package sendingpolicy_test + +import ( + "testing" + + "github.com/tokencanopy/e2a/internal/delivery" + "github.com/tokencanopy/e2a/internal/sendingpolicy" +) + +// TestDeriveBucket pins the bucket a full event kind + subtype maps to, and +// which of them repair the suppression list. Suppression-list subtypes are +// excluded from accounting (SES never attempted delivery) but still repair; +// the global-list subtype Suppressed stays a hard bounce. +func TestDeriveBucket(t *testing.T) { + cases := []struct { + name string + kind delivery.EventKind + btype, bsub string + csub string + bucket sendingpolicy.Bucket + repair bool + source string + wantRankAbove sendingpolicy.Bucket // must rank strictly above this bucket + }{ + {"delivered", delivery.KindDelivery, "", "", "", sendingpolicy.BucketDelivered, false, "", sendingpolicy.BucketNone}, + {"hard bounce general", delivery.KindBounce, "permanent", "General", "", sendingpolicy.BucketHardBounce, true, "bounce", sendingpolicy.BucketTerminalOther}, + {"hard bounce no email", delivery.KindBounce, "permanent", "NoEmail", "", sendingpolicy.BucketHardBounce, true, "bounce", sendingpolicy.BucketTerminalOther}, + {"global suppressed counts", delivery.KindBounce, "permanent", "Suppressed", "", sendingpolicy.BucketHardBounce, true, "bounce", sendingpolicy.BucketTerminalOther}, + {"account list excluded but repairs", delivery.KindBounce, "permanent", "OnAccountSuppressionList", "", sendingpolicy.BucketNone, true, "bounce", ""}, + {"tenant list excluded but repairs", delivery.KindBounce, "permanent", "OnTenantSuppressionList", "", sendingpolicy.BucketNone, true, "bounce", ""}, + {"transient bounce is terminal other", delivery.KindBounce, "transient", "MailboxFull", "", sendingpolicy.BucketTerminalOther, false, "", sendingpolicy.BucketDelivered}, + {"undetermined bounce is terminal other", delivery.KindBounce, "undetermined", "", "", sendingpolicy.BucketTerminalOther, false, "", sendingpolicy.BucketDelivered}, + {"genuine complaint", delivery.KindComplaint, "", "", "", sendingpolicy.BucketComplaint, true, "complaint", sendingpolicy.BucketHardBounce}, + {"complaint on account list excluded but repairs", delivery.KindComplaint, "", "", "OnAccountSuppressionList", sendingpolicy.BucketNone, true, "complaint", ""}, + {"delay is nothing", delivery.KindDeliveryDelay, "", "", "", sendingpolicy.BucketNone, false, "", ""}, + {"send is nothing", delivery.KindSend, "", "", "", sendingpolicy.BucketNone, false, "", ""}, + {"reject is nothing", delivery.KindReject, "", "", "", sendingpolicy.BucketNone, false, "", ""}, + {"other is nothing", delivery.KindOther, "", "", "", sendingpolicy.BucketNone, false, "", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + d := sendingpolicy.DeriveBucket(tc.kind, tc.btype, tc.bsub, tc.csub) + if d.Bucket != tc.bucket || d.Repair != tc.repair || d.Source != tc.source { + t.Fatalf("got bucket=%s repair=%v source=%q, want %s/%v/%q", d.Bucket, d.Repair, d.Source, tc.bucket, tc.repair, tc.source) + } + if tc.wantRankAbove != "" && d.Bucket.Rank() <= tc.wantRankAbove.Rank() { + t.Fatalf("%s rank %d must exceed %s rank %d", d.Bucket, d.Bucket.Rank(), tc.wantRankAbove, tc.wantRankAbove.Rank()) + } + }) + } +} + +// TestBucketRankOrder pins the exact evidence order the monotonic rule uses. +func TestBucketRankOrder(t *testing.T) { + order := []sendingpolicy.Bucket{sendingpolicy.BucketNone, sendingpolicy.BucketDelivered, sendingpolicy.BucketTerminalOther, sendingpolicy.BucketHardBounce, sendingpolicy.BucketComplaint} + for i, b := range order { + if b.Rank() != i { + t.Fatalf("%s rank = %d, want %d", b, b.Rank(), i) + } + } +} diff --git a/internal/sendingpolicy/gate.go b/internal/sendingpolicy/gate.go index 9c8774c71..ac73cc3c7 100644 --- a/internal/sendingpolicy/gate.go +++ b/internal/sendingpolicy/gate.go @@ -1384,7 +1384,11 @@ func (m *Module) recordCorrelation(ctx context.Context, tx pgx.Tx, st authState, return correlationID, nil } for _, addr := range st.envelope { - version, mac := m.secrets.Keyring.Sign([]byte(addr)) + // canonicalRecipient, not the envelope string: authorization accepts + // an internationalized domain as typed (Unicode), while the provider + // may report it back in A-label form. Feedback verifies against the + // same canonical form, so both spellings produce one HMAC. + version, mac := m.secrets.Keyring.Sign([]byte(canonicalRecipient(addr))) if _, err := tx.Exec(ctx, ` INSERT INTO sending_feedback_recipients (correlation_id, recipient_hmac, hmac_key_version) VALUES ($1, $2, $3) diff --git a/internal/sendingpolicy/maintenance.go b/internal/sendingpolicy/maintenance.go new file mode 100644 index 000000000..8f1880469 --- /dev/null +++ b/internal/sendingpolicy/maintenance.go @@ -0,0 +1,124 @@ +package sendingpolicy + +import ( + "context" + "log" + "time" + + "github.com/riverqueue/river" + + "github.com/tokencanopy/e2a/internal/jobs" +) + +// feedbackMaintenanceInterval paces the retention pass. Nothing here is +// urgent: expiries are stamped days ahead, and the detector window is +// measured in days. +const feedbackMaintenanceInterval = time.Hour + +// FeedbackMaintenanceArgs is the periodic retention job. +type FeedbackMaintenanceArgs struct{} + +func (FeedbackMaintenanceArgs) Kind() string { return "sending_feedback_maintenance" } + +// FeedbackMaintenanceWorker runs one retention pass over feedback provenance +// and daily outcome aggregates. +type FeedbackMaintenanceWorker struct { + river.WorkerDefaults[FeedbackMaintenanceArgs] + module *Module +} + +// NewFeedbackMaintenanceWorker builds the retention worker over a module. +// Exported so the janitor — the one component here that DELETES evidence — +// can be driven directly by a test rather than only through River. +func NewFeedbackMaintenanceWorker(module *Module) *FeedbackMaintenanceWorker { + return &FeedbackMaintenanceWorker{module: module} +} + +func (w *FeedbackMaintenanceWorker) Work(ctx context.Context, _ *river.Job[FeedbackMaintenanceArgs]) error { + // The EFFECTIVE policy, not the config file: on a database-source + // deployment an operator who widened the detector window would + // otherwise get a janitor that keeps deleting daily outcomes at the old + // width, silently shrinking the evidence the detector sums. + window, err := w.module.EffectiveDetectorWindowDays(ctx) + if err != nil { + return err + } + st, err := w.module.GCFeedback(ctx, w.module.now(), window) + if err != nil { + return err + } + if st.Events+st.Recipients+st.Correlations+st.Outcomes > 0 { + log.Printf("[sendingpolicy:feedback-gc] removed events=%d recipients=%d correlations=%d daily_outcomes=%d", + st.Events, st.Recipients, st.Correlations, st.Outcomes) + } + return nil +} + +// feedbackReconcileInterval paces the retention reconciliation: daily is +// ample for a 30-day horizon. It also runs on start, because an interval +// periodic's first tick is one interval after process start and a +// deployment that rolls more often than daily would otherwise never run it. +const feedbackReconcileInterval = 24 * time.Hour + +// FeedbackReconcileArgs is the periodic retention backstop. +type FeedbackReconcileArgs struct{} + +func (FeedbackReconcileArgs) Kind() string { return "sending_feedback_reconcile" } + +// FeedbackReconcileWorker stamps the post-deletion horizon on provenance of +// accounts that are gone but were never stamped, and sweeps orphaned +// events. See Module.ReconcileFeedbackRetention. +type FeedbackReconcileWorker struct { + river.WorkerDefaults[FeedbackReconcileArgs] + module *Module +} + +// NewFeedbackReconcileWorker builds the reconcile worker over a module. +func NewFeedbackReconcileWorker(module *Module) *FeedbackReconcileWorker { + return &FeedbackReconcileWorker{module: module} +} + +func (w *FeedbackReconcileWorker) Work(ctx context.Context, _ *river.Job[FeedbackReconcileArgs]) error { + // The same effective-policy horizon the purge seal stamps with. + retention, err := w.module.EffectiveFeedbackRetention(ctx) + if err != nil { + return err + } + st, err := w.module.ReconcileFeedbackRetention(ctx, w.module.now(), retention) + if err != nil { + return err + } + if st.StampedCorrelations+st.StampedEvents+st.OrphanEvents > 0 { + log.Printf("[sendingpolicy:feedback-reconcile] stamped correlations=%d events=%d; removed orphan events=%d", + st.StampedCorrelations, st.StampedEvents, st.OrphanEvents) + } + return nil +} + +// MaintenanceJobs registers the feedback retention periodics. Implements +// jobs.Registrar. +type MaintenanceJobs struct{ module *Module } + +// NewMaintenanceJobs builds the registrar over the gate's module. +func NewMaintenanceJobs(module *Module) *MaintenanceJobs { return &MaintenanceJobs{module: module} } + +func (m *MaintenanceJobs) RegisterJobs(w *river.Workers) []*river.PeriodicJob { + river.AddWorker(w, &FeedbackMaintenanceWorker{module: m.module}) + river.AddWorker(w, &FeedbackReconcileWorker{module: m.module}) + return []*river.PeriodicJob{ + river.NewPeriodicJob( + river.PeriodicInterval(feedbackMaintenanceInterval), + func() (river.JobArgs, *river.InsertOpts) { + return FeedbackMaintenanceArgs{}, &river.InsertOpts{Queue: jobs.QueueMaintenance} + }, + &river.PeriodicJobOpts{RunOnStart: false}, + ), + river.NewPeriodicJob( + river.PeriodicInterval(feedbackReconcileInterval), + func() (river.JobArgs, *river.InsertOpts) { + return FeedbackReconcileArgs{}, &river.InsertOpts{Queue: jobs.QueueMaintenance} + }, + &river.PeriodicJobOpts{RunOnStart: true}, + ), + } +} diff --git a/internal/sendingpolicy/maintenance_test.go b/internal/sendingpolicy/maintenance_test.go new file mode 100644 index 000000000..4754d8697 --- /dev/null +++ b/internal/sendingpolicy/maintenance_test.go @@ -0,0 +1,147 @@ +package sendingpolicy_test + +import ( + "context" + "testing" + + "github.com/riverqueue/river" + + "github.com/tokencanopy/e2a/internal/jobs" + "github.com/tokencanopy/e2a/internal/sendingpolicy" +) + +// TestFeedbackMaintenanceWorkerHonorsTheEffectiveWindow is the guard on the +// one component in this slice that DELETES evidence. +// +// A database-source deployment reads its detector window from the audited +// policy row, not from the config file. An operator who widens the window +// there and gets a janitor still cutting at the config width would lose the +// daily outcomes the detector is summing — an abuser's bad days would age +// out early, and nothing would say so. +func TestFeedbackMaintenanceWorkerHonorsTheEffectiveWindow(t *testing.T) { + f := newFixture(t) + ctx := context.Background() + + // Config says 30 days; the database singleton (generation zero) says 7. + wide := sendingpolicy.DisabledPolicy() + wide.DetectorWindowDays = 30 + dbSourced := sendingpolicy.NewGate(f.pool, f.secrets(), sendingpolicy.PolicySourceDatabase, wide).(*sendingpolicy.Module) + configSourced := sendingpolicy.NewGate(f.pool, f.secrets(), sendingpolicy.PolicySourceConfig, wide).(*sendingpolicy.Module) + + got, err := dbSourced.EffectiveDetectorWindowDays(ctx) + if err != nil { + t.Fatalf("effective window: %v", err) + } + if got != 7 { + t.Fatalf("database-source window = %d, want the policy row's 7 (not the config's 30)", got) + } + if got, err := configSourced.EffectiveDetectorWindowDays(ctx); err != nil || got != 30 { + t.Fatalf("config-source window = %d (err %v), want 30", got, err) + } + + // The worker must cut at the window it just read. Seed one aggregate + // just inside the database window and one outside it. + user := f.user("standard") + for _, age := range []int{6, 9} { + if _, err := f.pool.Exec(ctx, + `INSERT INTO account_sending_outcomes_daily (user_id, outcome_epoch, day, shared_reputation, delivered_count) + VALUES ($1, 1, current_date - $2::int, true, 1)`, user, age); err != nil { + t.Fatal(err) + } + } + if err := sendingpolicy.NewFeedbackMaintenanceWorker(dbSourced).Work(ctx, &river.Job[sendingpolicy.FeedbackMaintenanceArgs]{}); err != nil { + t.Fatalf("Work: %v", err) + } + var days []int + rows, err := f.pool.Query(ctx, `SELECT current_date - day FROM account_sending_outcomes_daily WHERE user_id = $1 ORDER BY 1`, user) + if err != nil { + t.Fatal(err) + } + defer rows.Close() + for rows.Next() { + var d int + if err := rows.Scan(&d); err != nil { + t.Fatal(err) + } + days = append(days, d) + } + if len(days) != 1 || days[0] != 6 { + t.Fatalf("remaining aggregate ages = %v, want only the 6-day-old row (window 7 + 1 day of safety)", days) + } +} + +// TestFeedbackMaintenanceWorkerRemovesExpiredProvenance: the worker is the +// only thing that honors the post-deletion horizon, so it has to actually +// remove what account deletion stamped. +func TestFeedbackMaintenanceWorkerRemovesExpiredProvenance(t *testing.T) { + f := newFixture(t) + ctx := context.Background() + module := sendingpolicy.NewModule(f.pool, f.secrets()) + + if _, err := f.pool.Exec(ctx, ` + INSERT INTO sending_feedback_correlations + (correlation_id, operation_id, submission_attempt, policy_subject_ref, purpose, shared_reputation, tenant_mode, expires_at) + VALUES ('cor_gc_expired', 'op_gc_1', 1, 'usr_gone', 'customer_message', true, 'none', now() - interval '1 hour'), + ('cor_gc_live', 'op_gc_2', 1, 'usr_here', 'customer_message', true, 'none', NULL)`); err != nil { + t.Fatal(err) + } + if _, err := f.pool.Exec(ctx, ` + INSERT INTO sending_feedback_recipients (correlation_id, recipient_hmac, hmac_key_version) + VALUES ('cor_gc_expired', '\xaa', 1), ('cor_gc_live', '\xbb', 1)`); err != nil { + t.Fatal(err) + } + if _, err := f.pool.Exec(ctx, ` + INSERT INTO sending_feedback_events (provider_event_id, correlation_id, provider_occurred_at, expires_at) + VALUES ('evt_gc_expired', 'cor_gc_expired', now(), now() - interval '1 hour'), + ('evt_gc_live', 'cor_gc_live', now(), NULL)`); err != nil { + t.Fatal(err) + } + + if err := sendingpolicy.NewFeedbackMaintenanceWorker(module).Work(ctx, &river.Job[sendingpolicy.FeedbackMaintenanceArgs]{}); err != nil { + t.Fatalf("Work: %v", err) + } + for table, column := range map[string]string{ + "sending_feedback_correlations": "correlation_id", + "sending_feedback_recipients": "correlation_id", + "sending_feedback_events": "provider_event_id", + } { + var expired, live int + if err := f.pool.QueryRow(ctx, + `SELECT count(*) FILTER (WHERE `+column+` LIKE '%expired'), count(*) FILTER (WHERE `+column+` LIKE '%live') + FROM `+table+` WHERE `+column+` LIKE '%gc%'`).Scan(&expired, &live); err != nil { + t.Fatal(err) + } + if expired != 0 { + t.Errorf("%s: %d expired row(s) survived the janitor", table, expired) + } + if live != 1 { + t.Errorf("%s: retained row was deleted (live=%d)", table, live) + } + } +} + +// TestFeedbackMaintenanceRegistersOnTheMaintenanceQueue: the periodic has +// to reach River on the low-urgency queue, or the retention horizon is +// never enforced at all. +func TestFeedbackMaintenanceRegistersOnTheMaintenanceQueue(t *testing.T) { + f := newFixture(t) + module := sendingpolicy.NewModule(f.pool, f.secrets()) + periodics := sendingpolicy.NewMaintenanceJobs(module).RegisterJobs(river.NewWorkers()) + if len(periodics) != 2 { + t.Fatalf("periodic jobs = %d, want 2 (retention pass + reconcile)", len(periodics)) + } + // River keeps the periodic's constructor unexported, so assert the two + // facts that are observable and load-bearing: the job kind the worker + // is registered under, and that the registrar hands River a real + // client-buildable set. A registrar that returned no periodic, or a + // kind rename that left the worker unreachable, both fail here. + if kind := (sendingpolicy.FeedbackMaintenanceArgs{}).Kind(); kind != "sending_feedback_maintenance" { + t.Fatalf("periodic kind = %q", kind) + } + if kind := (sendingpolicy.FeedbackReconcileArgs{}).Kind(); kind != "sending_feedback_reconcile" { + t.Fatalf("reconcile kind = %q", kind) + } + if _, err := jobs.New(f.pool, jobs.Config{}, sendingpolicy.NewMaintenanceJobs(module)); err != nil { + t.Fatalf("the registrar must produce a buildable River client: %v", err) + } +} diff --git a/internal/sendingpolicy/store.go b/internal/sendingpolicy/store.go index 3d386ffbe..bfa6b12d9 100644 --- a/internal/sendingpolicy/store.go +++ b/internal/sendingpolicy/store.go @@ -74,6 +74,29 @@ type Module struct { configPolicy RuntimePolicy commitAttestation func(context.Context, pgx.Tx) error + + // feedbackExcludedDomains are the canonical shared agent domains whose + // deliveries never count toward the detector denominator (see + // excludedFromDenominator); nil means only the SES simulator. + feedbackExcludedDomains map[string]struct{} + + // clock is the ingestion clock the feedback path stamps aggregates with; + // nil means time.Now. Tests and staging drills pin it to cross UTC days. + clock func() time.Time +} + +// WithClock pins the module's ingestion clock (feedback day assignment and +// retention); nil restores time.Now. +func (m *Module) WithClock(fn func() time.Time) *Module { + m.clock = fn + return m +} + +func (m *Module) now() time.Time { + if m.clock != nil { + return m.clock() + } + return time.Now() } // NewModule binds the module to a pool and the immutable trust roots parsed at diff --git a/internal/suppressionsync/store.go b/internal/suppressionsync/store.go new file mode 100644 index 000000000..12c8824bc --- /dev/null +++ b/internal/suppressionsync/store.go @@ -0,0 +1,103 @@ +// Package suppressionsync owns the write side of the account suppression list +// that provider feedback drives. +// +// Slice B8 of the sending abuse prevention plan moves suppression upsert +// ownership here ahead of the provider-facing reconciliation (Task 11): every +// feedback-driven upsert advances the row's sync_generation and clears any +// pending removal, so a stale delete that raced the upsert loses. The race +// contract is fixed now, before a remote list exists, so Task 11 can add the +// SES-facing half without changing it. +// +// The package is a leaf over pgx: it never reads a message, an agent, or a +// user row, which is what lets the deletion-resistant feedback path call it +// after the message that caused the bounce is long gone. +package suppressionsync + +import ( + "context" + "errors" + "fmt" + "strings" + + "github.com/jackc/pgx/v5" +) + +// Source values match the suppressions.source CHECK constraint. +const ( + SourceBounce = "bounce" + SourceComplaint = "complaint" + SourceManual = "manual" +) + +// Upsert is the outcome of one feedback-driven upsert. +type Upsert struct { + ID string + Inserted bool // a new row; false when an existing row was refreshed + Generation int64 // sync_generation after this write +} + +// UpsertTx inserts the (user, address) suppression or, when it already +// exists, advances its sync_generation and clears removal_pending. The +// existing row's reason and source are kept: the first evidence wins, and a +// refresh must not rewrite a manual entry into a bounce. +// +// The address must already be normalized (lower-cased, trimmed); this +// package does not own address canonicalization. +func UpsertTx(ctx context.Context, tx pgx.Tx, id, userID, address, reason, source, sourceMessageID string) (Upsert, error) { + if strings.TrimSpace(id) == "" || strings.TrimSpace(userID) == "" || strings.TrimSpace(address) == "" { + return Upsert{}, errors.New("suppressionsync: id, user and address are required") + } + var out Upsert + var srcMsg *string + if sourceMessageID != "" { + srcMsg = &sourceMessageID + } + if err := tx.QueryRow(ctx, ` + INSERT INTO suppressions (id, user_id, address, reason, source, source_message_id) + VALUES ($1, $2, $3, $4, $5, $6) + ON CONFLICT (user_id, address) DO UPDATE + SET sync_generation = suppressions.sync_generation + 1, + removal_pending = false + RETURNING id, (xmax = 0) AS inserted, sync_generation`, + id, userID, address, reason, source, srcMsg, + ).Scan(&out.ID, &out.Inserted, &out.Generation); err != nil { + return Upsert{}, fmt.Errorf("suppressionsync: upsert: %w", err) + } + return out, nil +} + +// MarkRemovalPendingTx flags the row for removal and returns the generation +// the remover must present to CompleteRemovalTx. found=false when there is +// no such suppression. +func MarkRemovalPendingTx(ctx context.Context, tx pgx.Tx, userID, address string) (generation int64, found bool, err error) { + err = tx.QueryRow(ctx, ` + UPDATE suppressions + SET removal_pending = true + WHERE user_id = $1 AND address = $2 + RETURNING sync_generation`, userID, address, + ).Scan(&generation) + if errors.Is(err, pgx.ErrNoRows) { + return 0, false, nil + } + if err != nil { + return 0, false, fmt.Errorf("suppressionsync: mark removal pending: %w", err) + } + return generation, true, nil +} + +// CompleteRemovalTx deletes the row only if it is still pending removal at +// the generation the remover observed. A feedback upsert in between advanced +// the generation and cleared the flag, so the stale delete removes nothing: +// the address stays suppressed, which is the only safe answer when the +// provider has just said it bounces. +func CompleteRemovalTx(ctx context.Context, tx pgx.Tx, userID, address string, generation int64) (removed bool, err error) { + tag, err := tx.Exec(ctx, ` + DELETE FROM suppressions + WHERE user_id = $1 AND address = $2 + AND removal_pending = true + AND sync_generation = $3`, userID, address, generation) + if err != nil { + return false, fmt.Errorf("suppressionsync: complete removal: %w", err) + } + return tag.RowsAffected() > 0, nil +} diff --git a/internal/suppressionsync/store_test.go b/internal/suppressionsync/store_test.go new file mode 100644 index 000000000..6eeb240eb --- /dev/null +++ b/internal/suppressionsync/store_test.go @@ -0,0 +1,126 @@ +package suppressionsync_test + +import ( + "context" + "testing" + + "github.com/jackc/pgx/v5" + + "github.com/tokencanopy/e2a/internal/suppressionsync" + "github.com/tokencanopy/e2a/internal/testutil/testdb" +) + +// TestUpsertAdvancesGenerationAndDefeatsStaleRemoval pins the race contract +// the provider-facing reconciliation (Task 11) will build on: a feedback +// upsert on an existing row bumps sync_generation and clears +// removal_pending, so a removal that observed the earlier generation +// deletes nothing. +func TestUpsertAdvancesGenerationAndDefeatsStaleRemoval(t *testing.T) { + ctx := context.Background() + pool := testdb.TestDB(t) + const user = "usr_suppsync_1" + if _, err := pool.Exec(ctx, `INSERT INTO users (id, email, google_subject, account_class) VALUES ($1, 'suppsync@reviewer.test', 'google-suppsync', 'standard') ON CONFLICT (id) DO NOTHING`, user); err != nil { + t.Fatal(err) + } + inTx := func(fn func(tx pgx.Tx) error) { + t.Helper() + tx, err := pool.Begin(ctx) + if err != nil { + t.Fatal(err) + } + if err := fn(tx); err != nil { + _ = tx.Rollback(ctx) + t.Fatal(err) + } + if err := tx.Commit(ctx); err != nil { + t.Fatal(err) + } + } + + var first, second suppressionsync.Upsert + inTx(func(tx pgx.Tx) error { + var err error + first, err = suppressionsync.UpsertTx(ctx, tx, "supp_a", user, "bounce@example.test", "bounce:General", suppressionsync.SourceBounce, "msg_1") + return err + }) + if !first.Inserted || first.Generation != 1 { + t.Fatalf("first upsert = %+v, want inserted at generation 1", first) + } + inTx(func(tx pgx.Tx) error { + var err error + second, err = suppressionsync.UpsertTx(ctx, tx, "supp_b", user, "bounce@example.test", "complaint", suppressionsync.SourceComplaint, "") + return err + }) + if second.Inserted || second.ID != first.ID || second.Generation != 2 { + t.Fatalf("second upsert = %+v, want refresh of %s at generation 2", second, first.ID) + } + var reason, source string + if err := pool.QueryRow(ctx, `SELECT reason, source FROM suppressions WHERE id = $1`, first.ID).Scan(&reason, &source); err != nil { + t.Fatal(err) + } + if reason != "bounce:General" || source != "bounce" { + t.Fatalf("refresh must keep the first evidence, got reason=%q source=%q", reason, source) + } + + // A remover marks the row pending at generation 2; feedback re-proves + // the address (generation 3, pending cleared); the stale removal at 2 + // deletes nothing and the address stays suppressed. + var gen int64 + inTx(func(tx pgx.Tx) error { + var found bool + var err error + gen, found, err = suppressionsync.MarkRemovalPendingTx(ctx, tx, user, "bounce@example.test") + if err == nil && !found { + t.Fatal("row must be found") + } + return err + }) + if gen != 2 { + t.Fatalf("pending generation = %d, want 2", gen) + } + inTx(func(tx pgx.Tx) error { + up, err := suppressionsync.UpsertTx(ctx, tx, "supp_c", user, "bounce@example.test", "bounce:General", suppressionsync.SourceBounce, "") + if err == nil && (up.Generation != 3 || up.Inserted) { + t.Fatalf("racing upsert = %+v, want generation 3 refresh", up) + } + return err + }) + inTx(func(tx pgx.Tx) error { + removed, err := suppressionsync.CompleteRemovalTx(ctx, tx, user, "bounce@example.test", gen) + if err == nil && removed { + t.Fatal("a stale removal must delete nothing after a racing upsert") + } + return err + }) + var pending bool + var count int + if err := pool.QueryRow(ctx, `SELECT count(*), bool_or(removal_pending) FROM suppressions WHERE user_id = $1 AND address = 'bounce@example.test'`, user).Scan(&count, &pending); err != nil { + t.Fatal(err) + } + if count != 1 || pending { + t.Fatalf("row count=%d pending=%v, want the row kept and not pending", count, pending) + } + + // An uncontested removal at the current generation succeeds. + inTx(func(tx pgx.Tx) error { + g, _, err := suppressionsync.MarkRemovalPendingTx(ctx, tx, user, "bounce@example.test") + if err != nil { + return err + } + removed, err := suppressionsync.CompleteRemovalTx(ctx, tx, user, "bounce@example.test", g) + if err == nil && !removed { + t.Fatal("an uncontested removal must delete the row") + } + return err + }) + if _, found, err := func() (int64, bool, error) { + tx, err := pool.Begin(ctx) + if err != nil { + return 0, false, err + } + defer func() { _ = tx.Rollback(ctx) }() + return suppressionsync.MarkRemovalPendingTx(ctx, tx, user, "bounce@example.test") + }(); err != nil || found { + t.Fatalf("row should be gone: found=%v err=%v", found, err) + } +} diff --git a/internal/telemetry/metrics.go b/internal/telemetry/metrics.go index 9308c483d..6f2df0e60 100644 --- a/internal/telemetry/metrics.go +++ b/internal/telemetry/metrics.go @@ -148,6 +148,17 @@ type Metrics interface { // Bounded labels only: never an account id, address or domain. ExternalAccessDecision(stage, route, mode string) + // SendingFeedbackIngested records one deletion-resistant SES feedback + // ingestion result (internal/sendingpolicy). outcome ∈ {correlated, + // dead_account, unmatched_recipient, uncorrelated_with_marker, + // uncorrelated, duplicate}; bucket ∈ {delivered, hard_bounce, complaint, + // terminal_other, none}. One sample per recipient of a first-seen + // correlated event, one per uncorrelated or duplicate event. + // uncorrelated_with_marker is e2a-stamped mail whose retained + // correlation is missing — the spec's alerting signal. Bounded labels + // only: never an address, account, correlation or provider id. + SendingFeedbackIngested(outcome, bucket string) + // WebhookAttempt records one webhook delivery attempt. outcome ∈ // {delivered, retryable_failure, exhausted, webhook_deleted, // skipped_disabled}. statusClass is the HTTP status class of the @@ -318,6 +329,7 @@ func (NoOp) OutboundTerminalLatency(float64) {} func (NoOp) OutboundAttempt(string, float64) {} func (NoOp) OutboundRateDeferred() {} func (NoOp) ExternalAccessDecision(string, string, string) {} +func (NoOp) SendingFeedbackIngested(string, string) {} func (NoOp) WebhookAttempt(string, string, float64) {} func (NoOp) WebhookTerminal(string, string, int) {} func (NoOp) WebhookNotify(string, string) {} @@ -452,6 +464,10 @@ func (l *Log) ExternalAccessDecision(stage, route, mode string) { log.Printf("[metrics] event=external_access.decision stage=%s route=%s mode=%s", stage, route, mode) } +func (l *Log) SendingFeedbackIngested(outcome, bucket string) { + log.Printf("[metrics] event=sending_feedback.ingested outcome=%s bucket=%s", outcome, bucket) +} + func (l *Log) WebhookAttempt(outcome, statusClass string, seconds float64) { log.Printf("[metrics] event=webhook.attempt outcome=%s status_class=%s duration=%.3f", outcome, statusClass, seconds) } diff --git a/internal/telemetry/prom.go b/internal/telemetry/prom.go index 4824fb827..f074cc05c 100644 --- a/internal/telemetry/prom.go +++ b/internal/telemetry/prom.go @@ -31,6 +31,7 @@ type Prom struct { outAttemptDur prometheus.Histogram outRateDeferred prometheus.Counter externalAccess *prometheus.CounterVec + sendingFeedback *prometheus.CounterVec whAttempts *prometheus.CounterVec whAttemptDur prometheus.Histogram whTerminal *prometheus.CounterVec @@ -102,8 +103,11 @@ var ( externalAccessStageSet = set("preflight", "acceptance", "authorization", "redemption") externalAccessRouteSet = set("not_applicable", "custom_identity", "operator_approval", "paid_entitlement", "restricted_recipients", "denied") - externalAccessModeSet = set("shadow", "enforce") - whSet = set("delivered", "retryable_failure", "exhausted", + externalAccessModeSet = set("shadow", "enforce") + sendingFeedbackOutcomeSet = set("correlated", "dead_account", "unmatched_recipient", + "uncorrelated_with_marker", "uncorrelated", "duplicate") + sendingFeedbackBucketSet = set("delivered", "hard_bounce", "complaint", "terminal_other", "none") + whSet = set("delivered", "retryable_failure", "exhausted", "webhook_deleted", "skipped_disabled") whTerminalSet = set("delivered", "e2a_failure", "endpoint_failure", "excluded") whScopeSet = set("initial", "replay", "test", "unknown") @@ -271,6 +275,10 @@ func NewProm(build string) *Prom { Name: "e2a_external_access_decisions_total", Help: "External-sending-access evaluations by stage, deciding route and policy mode (shadow route=denied = sends enforcement would refuse).", }, []string{"stage", "route", "mode"}), + sendingFeedback: prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "e2a_sending_feedback_ingested_total", + Help: "Deletion-resistant SES feedback ingestion results by outcome and detector bucket (uncorrelated_with_marker = e2a-stamped mail with no retained correlation).", + }, []string{"outcome", "bucket"}), outRateDeferred: prometheus.NewCounter(prometheus.CounterOpts{ Name: "e2a_outbound_rate_deferred_total", Help: "Outbound submissions deferred by the per-agent fire-time rate limiter (snoozed, re-fired when the window frees capacity).", @@ -436,7 +444,7 @@ func NewProm(build string) *Prom { registerer.MustRegister( p.httpRequests, p.httpDuration, p.smtpInbound, p.smtpDuration, - p.outQueueWait, p.outTerminal, p.outTerminalLat, p.outAttempts, p.outAttemptDur, p.outRateDeferred, p.externalAccess, + p.outQueueWait, p.outTerminal, p.outTerminalLat, p.outAttempts, p.outAttemptDur, p.outRateDeferred, p.externalAccess, p.sendingFeedback, p.whAttempts, p.whAttemptDur, p.whTerminal, p.whNotify, p.whExpiredPending, p.whFanOutRescued, p.whDeliveryRescued, p.whFirstTryLat, p.wsConnects, p.wsDisconnects, p.wsRejected, p.wsDrained, p.wsSendFailures, p.wsActive, p.delegatedFailures, p.delegatedRefresh, p.oidcDiscovery, p.oidcCallback, p.provisioning, @@ -513,6 +521,10 @@ func (p *Prom) ExternalAccessDecision(stage, route, mode string) { p.externalAccess.WithLabelValues(enum(externalAccessStageSet, stage), enum(externalAccessRouteSet, route), enum(externalAccessModeSet, mode)).Inc() } +func (p *Prom) SendingFeedbackIngested(outcome, bucket string) { + p.sendingFeedback.WithLabelValues(enum(sendingFeedbackOutcomeSet, outcome), enum(sendingFeedbackBucketSet, bucket)).Inc() +} + func (p *Prom) WebhookAttempt(outcome, statusClass string, seconds float64) { p.whAttempts.WithLabelValues(enum(whSet, outcome), enum(classSet, statusClass)).Inc() // seconds < 0 = "no duration sample" (outcomes with no HTTP POST — diff --git a/internal/telemetry/prom_test.go b/internal/telemetry/prom_test.go index 0ff51174a..306f1bb14 100644 --- a/internal/telemetry/prom_test.go +++ b/internal/telemetry/prom_test.go @@ -206,6 +206,9 @@ func TestPromEmitsSMTPOutboundWebhookWSSeries(t *testing.T) { p.SetQueueOldestAge("outbound", 45.5) p.ExternalAccessDecision("authorization", "denied", "shadow") p.ExternalAccessDecision("preflight", "attacker-controlled", "enforce") + p.SendingFeedbackIngested("uncorrelated_with_marker", "complaint") + p.SendingFeedbackIngested("correlated", "delivered") + p.SendingFeedbackIngested("bob@example.test", "cor_0123") out := scrape(t, p) for _, want := range []string{ @@ -223,6 +226,9 @@ func TestPromEmitsSMTPOutboundWebhookWSSeries(t *testing.T) { `e2a_outbound_rate_deferred_total 1`, `e2a_external_access_decisions_total{mode="shadow",route="denied",stage="authorization"} 1`, `e2a_external_access_decisions_total{mode="enforce",route="other",stage="preflight"} 1`, + `e2a_sending_feedback_ingested_total{bucket="complaint",build="unknown",outcome="uncorrelated_with_marker"} 1`, + `e2a_sending_feedback_ingested_total{bucket="delivered",build="unknown",outcome="correlated"} 1`, + `e2a_sending_feedback_ingested_total{bucket="other",build="unknown",outcome="other"} 1`, `e2a_webhook_attempts_total{outcome="delivered",status_class="2xx"} 1`, `e2a_webhook_attempts_total{outcome="retryable_failure",status_class="5xx"} 1`, `e2a_webhook_delivery_terminal_total{outcome="delivered",scope="initial"} 1`, diff --git a/migrations/123_sending_feedback_indexes.sql b/migrations/123_sending_feedback_indexes.sql new file mode 100644 index 000000000..0c4faaa8f --- /dev/null +++ b/migrations/123_sending_feedback_indexes.sql @@ -0,0 +1,44 @@ +-- 123_sending_feedback_indexes.sql +-- +-- Access paths for the B8 feedback provenance tables. Migration 114 created +-- sending_feedback_events with only its primary key and no expiry index, and +-- gave sending_feedback_correlations an expiry index led by +-- source_account_ref, which the retention janitor's `WHERE expires_at <= $1` +-- cannot use. Both of the janitor's deletes and the account-deletion +-- retention stamp were therefore sequential scans over tables that grow one +-- row per send. +-- +-- Deliberately NOT indexed: sending_feedback_recipients.hmac_key_version. +-- Its only reader is the startup keyring gate, whose predicate is +-- "version NOT IN (held versions)" — a btree cannot serve that, so the index +-- would be write amplification on the hottest of these tables with no +-- reader. That scan is boot-only and bounded in practice by the gate's +-- LIMIT 1 in the failing case. +-- +-- Index-only additions. Note these are NON-concurrent CREATE INDEX, which +-- takes SHARE and blocks writes to the table for its duration. These tables +-- are NOT empty at the release that carries this migration: the gate has +-- written a correlation (plus one recipient row per envelope address) for +-- every authorized provider attempt since the sending-policy gate shipped, +-- so a production deployment holds weeks of rows. That is still a small +-- table by index-build standards — one row per send, a few narrow columns — +-- so the build completes in well under a second to a few seconds, and the +-- only writers it blocks are feedback/authorization inserts, which wait +-- rather than fail. lock_timeout bounds the wait to ACQUIRE the lock, so a +-- long-running transaction on these tables fails the migration (and the +-- boot, which retries) instead of queueing every writer behind it. A +-- deployment with a far larger backlog should pre-create these indexes +-- CONCURRENTLY by hand; IF NOT EXISTS then makes this file a no-op. + +SET LOCAL lock_timeout = '2s'; + +CREATE INDEX IF NOT EXISTS sending_feedback_correlations_expiry_idx + ON sending_feedback_correlations (expires_at) + WHERE expires_at IS NOT NULL; + +CREATE INDEX IF NOT EXISTS sending_feedback_events_expiry_idx + ON sending_feedback_events (expires_at) + WHERE expires_at IS NOT NULL; + +CREATE INDEX IF NOT EXISTS sending_feedback_events_correlation_idx + ON sending_feedback_events (correlation_id); diff --git a/migrations/124_sending_feedback_orphan_retention.sql b/migrations/124_sending_feedback_orphan_retention.sql new file mode 100644 index 000000000..3a445da6d --- /dev/null +++ b/migrations/124_sending_feedback_orphan_retention.sql @@ -0,0 +1,48 @@ +-- 124_sending_feedback_orphan_retention.sql +-- +-- Backfill the post-deletion retention horizon on feedback provenance whose +-- account is already gone. +-- +-- The sending-policy gate writes customer-purpose correlations with +-- expires_at NULL: they must outlive the message for as long as the account +-- exists. B8 stamps the horizon in the account purge's seal transaction — +-- but only for accounts purged by a release that carries B8. Every account +-- erased or purged earlier left its correlations (and their events) with a +-- NULL expiry that nothing would ever stamp, i.e. retained forever, beyond +-- the disclosed 30-day post-deletion window. +-- +-- This stamps them now, by the same rule the seal uses: +-- * customer-purpose correlations (customer_message, customer_notification) +-- whose source_account_ref has no users row get expires_at = now() + 30 +-- days — the policy default sending_feedback_post_account_retention_days +-- (identity.DefaultFeedbackRetention). A trashed account still has its +-- users row and is deliberately NOT stamped, exactly as the seal defers +-- until purge. +-- * every event with a NULL expiry whose correlation now has one inherits +-- the correlation's expiry. Recipient rows carry no expiry column; the +-- retention janitor removes them with their correlation. +-- +-- The sending-policy reconcile job runs the same predicate daily, which also +-- catches a correlation authorized in a race with a purge. This file is the +-- one-time backlog pass. +-- +-- Idempotent: only NULL expiries are written, so re-applying changes +-- nothing already stamped. Row-level UPDATEs only (no table lock beyond +-- ROW EXCLUSIVE); lock_timeout bounds waiting on a row a concurrent feedback +-- transaction holds, failing the boot (which retries) rather than hanging it. + +SET LOCAL lock_timeout = '5s'; + +UPDATE sending_feedback_correlations c + SET expires_at = now() + interval '30 days' + WHERE c.expires_at IS NULL + AND c.purpose IN ('customer_message', 'customer_notification') + AND c.source_account_ref IS NOT NULL + AND NOT EXISTS (SELECT 1 FROM users u WHERE u.id = c.source_account_ref); + +UPDATE sending_feedback_events e + SET expires_at = c.expires_at + FROM sending_feedback_correlations c + WHERE c.correlation_id = e.correlation_id + AND e.expires_at IS NULL + AND c.expires_at IS NOT NULL;