amadan

carlos / platform Public

email events: the log belongs to the app, not its name (review fixes)

From an Opus review of this branch.

A freed name handed on its history. Nothing in trash, purge, rename or a
move to another account touched mail_events, so a purged app's thirty
days of recipients sat under its name for whatever claimed it next — and
GET .../email/events deliberately does not need the email record, so the
next app could read them. PurgeAppRow now forgets them in the same
transaction that frees the name. RenameAppIn and MoveApp carry them with
the app, keyed by the new name or the new account's sqid, dropping any
predecessor's rows at the destination first and carrying only rows
received since this app's own row was created — the rule RenameAppIn
already applies to audit rows. A trashed app is kept nothing new.

A named sender that is not here fell back to the domain. senderAmong
answered "" both when SES named nobody and when it named a CARLOS identity
that was not on the matching domain, and "" meant every app on the domain
got the row. Now an identity CARLOS minted is the sender, full stop: the
row goes to that app if it is live and to nobody otherwise. Only an
identity CARLOS did not mint falls back to the domain.

AppOfCaller trusted any ARN's last segment, including an assumed-role
session, which is named by whoever assumes the role. Only a bare name or
an IAM user ARN is read now, and a standalone credential must have a name.

cleanField stripped C0 and DEL but kept C1 (U+009B is a CSI on some
terminals) and the invisible formatting class, bidi overrides among it.
The diagnostic is written by whoever runs the recipient's mail server and
printed straight to a terminal; both classes go.

Events were dated by the send. A bounce a day later showed the send time
and was invisible to --since 24h. At is now the event's own timestamp
(bounce, complaint, delivery, delay), SentAt the send.

The fallback notice key — only reachable without an SNS MessageId —
collapsed two bounces of one message to different recipients into one.
It now carries the recipients.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bill Horsman pushed by bill@keymail.dev f497a4d105a93ea8ba0d7da780e26a634c39f932 parent 819e7cb
7 files changed, +346 −53
  • internal/console/mailevents.go +72 −6
  • internal/console/mailevents_test.go +88 −5
  • internal/console/sesevents.go +42 −28
  • internal/console/store.go +47 −2
  • internal/mail/events.go +27 −2
  • internal/mail/events_test.go +50 −3
  • internal/mail/policy.go +20 −7
diff --git a/internal/console/mailevents.go b/internal/console/mailevents.go
index fb487a424..d27861e4c 100644
--- a/internal/console/mailevents.go
+++ b/internal/console/mailevents.go
@@ -48,12 +48,7 @@ func (s *Store) RecordMailMessageEvent(ctx context.Context, acct, app, notice st
if at.IsZero() {
at = now
}
- if notice == "" {
- // A notification with no SNS id of its own — nothing SNS sends, but
- // the recording path is also reachable directly. Key it on what it
- // says instead, so a repeat still collapses.
- notice = ev.MessageID + "/" + ev.Type + "/" + at.UTC().Format(time.RFC3339Nano)
- }
+ notice = mailNoticeKey(notice, ev, now)
sender := 0
if bySender {
sender = 1
@@ -71,6 +66,77 @@ VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
return s.pruneMailEvents(ctx, now)
}
+// mailNoticeKey is the key a notification is kept under: SNS's own
+// MessageId, or — for a notification with no id of its own, which is nothing
+// SNS sends but the recording path is reachable directly — what it says, so
+// a repeat still collapses and two different notifications do not.
+func mailNoticeKey(notice string, ev mail.Event, now time.Time) string {
+ if notice != "" {
+ return notice
+ }
+ at := ev.At
+ if at.IsZero() {
+ at = now
+ }
+ var to []string
+ for _, o := range ev.Recipients() {
+ to = append(to, o.Recipient)
+ }
+ // Two bounces for one message — one per recipient, or a delay reported
+ // twice — are two notifications, so what they are about is in the key.
+ return ev.MessageID + "/" + ev.Type + "/" + at.UTC().Format(time.RFC3339Nano) + "/" + strings.Join(to, ",")
+}
+
+// mailAppTables are the tables keyed by an app's (account sqid, name) that
+// belong to that app alone: a name that is freed must not hand them on.
+var mailAppTables = []string{"mail_events"}
+
+// execer is what carryMailApp and forgetMailApp need from a transaction.
+type execer interface {
+ ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error)
+}
+
+// carryMailApp moves an app's email history with the app, when a rename or
+// a move to another account changes the (sqid, name) it is keyed by. Rows
+// already at the destination belong to a purged predecessor of that name
+// and are dropped first, and only rows received since this app's own row was
+// created travel — the same rule RenameAppIn applies to audit rows, for the
+// same reason. Whatever is left under the old key is dropped.
+func carryMailApp(ctx context.Context, tx execer, fromSqid, fromApp, toSqid, toApp string, createdAt int64) error {
+ for _, table := range mailAppTables {
+ if _, err := tx.ExecContext(ctx, `DELETE FROM `+table+` WHERE account = ? AND app = ?`, toSqid, toApp); err != nil {
+ return err
+ }
+ if _, err := tx.ExecContext(ctx, `UPDATE `+table+` SET account = ?, app = ? WHERE account = ? AND app = ? AND `+
+ mailAppTimeColumn(table)+` >= ?`, toSqid, toApp, fromSqid, fromApp, createdAt); err != nil {
+ return err
+ }
+ if _, err := tx.ExecContext(ctx, `DELETE FROM `+table+` WHERE account = ? AND app = ?`, fromSqid, fromApp); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+// forgetMailApp drops everything a purged app left in these tables, so a
+// later app of the same name starts with none of it.
+func forgetMailApp(ctx context.Context, tx execer, sqid, app string) error {
+ for _, table := range mailAppTables {
+ if _, err := tx.ExecContext(ctx, `DELETE FROM `+table+` WHERE account = ? AND app = ?`, sqid, app); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+// mailAppTimeColumn is the column that says when a row came to be.
+func mailAppTimeColumn(table string) string {
+ if table == "mail_events" {
+ return "received_at"
+ }
+ return "created_at"
+}
+
// pruneMailEvents trims the log to its retention window, at most once per
// mailEventsPruneEvery. The stamp is taken before the DELETE runs, so two
// notifications arriving together do not both prune.
diff --git a/internal/console/mailevents_test.go b/internal/console/mailevents_test.go
index 5bbd0eb4b..99c5fc25f 100644
--- a/internal/console/mailevents_test.go
+++ b/internal/console/mailevents_test.go
@@ -164,9 +164,23 @@ func TestTheMailLogGoesToTheAppSESSaysSentIt(t *testing.T) {
}
}
- // SES naming nobody — or somebody who is not one of these apps — falls
- // back to the domain, and says so on the row.
- for i, caller := range []string{"", "someone-else", mail.UserName(release.OpsAccount, "not-on-this-domain")} {
+ // SES naming one of CARLOS's own identities that is not a live app gives
+ // the row to nobody — the name is the sender, and it is not here to be
+ // told. It does NOT fall back to the domain's other apps.
+ stray := bounceTo("eve@example.com", "m-stray", now)
+ stray.Caller = mail.UserName(release.OpsAccount, "not-an-app")
+ fx.srv.recordAndEnforceMail(ctx, stray, "sns-stray")
+ for _, app := range []string{"jb-book", "jb-site"} {
+ for _, e := range logOf(app) {
+ if e.MessageID == "m-stray" {
+ t.Errorf("%s was given a row SES attributed to another CARLOS app: %+v", app, e)
+ }
+ }
+ }
+
+ // SES naming nobody CARLOS minted falls back to the domain, and says so
+ // on the row.
+ for i, caller := range []string{"", "someone-else"} {
ev := bounceTo("bob@example.com", "m-unnamed", now)
ev.Caller = caller
fx.srv.recordAndEnforceMail(ctx, ev, "sns-unnamed-"+string(rune('a'+i)))
@@ -178,8 +192,8 @@ func TestTheMailLogGoesToTheAppSESSaysSentIt(t *testing.T) {
unnamed = append(unnamed, e)
}
}
- if len(unnamed) != 3 {
- t.Errorf("%s: %d domain-attributed rows, want 3", app, len(unnamed))
+ if len(unnamed) != 2 {
+ t.Errorf("%s: %d domain-attributed rows, want 2", app, len(unnamed))
}
for _, e := range unnamed {
if e.BySender {
@@ -188,3 +202,72 @@ func TestTheMailLogGoesToTheAppSESSaysSentIt(t *testing.T) {
}
}
}
+
+// A trashed app is told nothing new, and a name that is freed — by a purge,
+// a rename or a move to another account — hands none of its email history
+// to whatever holds the name next. The history of an app that is renamed or
+// moved goes with it.
+func TestTheMailLogBelongsToTheAppNotItsName(t *testing.T) {
+ fx := newConsoleFixture(t)
+ ctx := context.Background()
+ now := time.Now()
+ logOf := func(acct, app string) int {
+ t.Helper()
+ got, err := fx.st.MailEvents(ctx, acct, app, MailEventQuery{}, time.Now())
+ if err != nil {
+ t.Fatal(err)
+ }
+ return len(got)
+ }
+ send := func(app, message string) {
+ t.Helper()
+ ev := bounceTo("ann@example.com", message, now)
+ ev.Caller = mail.UserName(release.OpsAccount, app)
+ fx.srv.recordAndEnforceMail(ctx, ev, "sns-"+message)
+ }
+ for _, app := range []string{"harbour", "renamed-from", "moving"} {
+ if _, err := fx.st.CreateAppIn(OpsAccountID, app, "bill@tito.io", now.Add(-time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+ }
+
+ // Trashed: nothing new is kept.
+ send("harbour", "m-live")
+ if err := fx.st.TrashAppRow(OpsAccountID, "harbour", now); err != nil {
+ t.Fatal(err)
+ }
+ send("harbour", "m-trashed")
+ if n := logOf(release.OpsAccount, "harbour"); n != 1 {
+ t.Errorf("harbour has %d rows, want 1 — nothing arriving after the trash is kept", n)
+ }
+
+ // Purged: the name starts clean for the next app to claim it.
+ if err := fx.st.PurgeAppRow(OpsAccountID, "harbour"); err != nil {
+ t.Fatal(err)
+ }
+ if n := logOf(release.OpsAccount, "harbour"); n != 0 {
+ t.Errorf("a purged app's history is still there for its name: %d rows", n)
+ }
+
+ // Renamed: the history goes with the app, and the old name holds none.
+ send("renamed-from", "m-before-rename")
+ if err := fx.st.RenameAppIn(OpsAccountID, "renamed-from", "renamed-to"); err != nil {
+ t.Fatal(err)
+ }
+ if logOf(release.OpsAccount, "renamed-to") != 1 || logOf(release.OpsAccount, "renamed-from") != 0 {
+ t.Errorf("after a rename: to=%d from=%d, want 1 and 0",
+ logOf(release.OpsAccount, "renamed-to"), logOf(release.OpsAccount, "renamed-from"))
+ }
+
+ // Moved to another account: the same, keyed by the new account's sqid.
+ send("moving", "m-before-move")
+ if err := fx.st.CreateAccount(99, "dest", "Destination", "bill@tito.io", now); err != nil {
+ t.Fatal(err)
+ }
+ if err := fx.st.MoveApp("moving", OpsAccountID, 99); err != nil {
+ t.Fatal(err)
+ }
+ if logOf("dest", "moving") != 1 || logOf(release.OpsAccount, "moving") != 0 {
+ t.Errorf("after a move: dest=%d ops=%d, want 1 and 0", logOf("dest", "moving"), logOf(release.OpsAccount, "moving"))
+ }
+}
diff --git a/internal/console/sesevents.go b/internal/console/sesevents.go
index 489960a44..5b8c4c386 100644
--- a/internal/console/sesevents.go
+++ b/internal/console/sesevents.go
@@ -454,11 +454,11 @@ func domainMatchesEvent(d *mail.DomainRecord, domain string) bool {
//
// Each notification is also kept in the per-message log (mailevents.go),
// under notice — SNS's own id for it. The log is attributed more narrowly
-// than the counts where SES allows: when the notification names the IAM
-// identity that sent it, and that identity is one of the apps on the
-// matching domain, the row goes to that app alone. The counts keep their
-// domain-wide attribution untouched — they enforce a cap, and changing what
-// they measure is not this change's business.
+// than the counts where SES allows: when the notification names a sending
+// identity CARLOS minted, the row goes to that app alone; only when it names
+// none does the row go to every app on the matching domain. The counts keep
+// their domain-wide attribution untouched — they enforce a cap, and changing
+// what they measure is not this change's business.
func (s *Server) recordAndEnforceMail(ctx context.Context, ev mail.Event, notice string) {
accts, err := s.store.Accounts()
if err != nil {
@@ -477,6 +477,19 @@ func (s *Server) recordAndEnforceMail(ctx context.Context, ev mail.Event, notice
at = time.Now()
}
day := at.UTC().Format("2006-01-02")
+ now := time.Now()
+ key := mailNoticeKey(notice, ev, now)
+
+ // The per-message log. When SES names one of CARLOS's own sending
+ // identities, that app — and nobody else — gets the row, whatever domain
+ // the message went out on: the credential is minted per app, so the name
+ // is the sender. Only when SES names nobody CARLOS minted does the row
+ // fall back to every app on the matching domain, below.
+ callerAcct, callerApp, named := mail.AppOfCaller(ev.Caller)
+ if named {
+ s.recordMailMessage(ctx, callerAcct, callerApp, key, ev, true, now)
+ }
+
for _, a := range accts {
domains, err := mail.ListDomains(ctx, s.cfg.Releases, a.Sqid)
if err != nil {
@@ -487,14 +500,9 @@ func (s *Server) recordAndEnforceMail(ctx context.Context, ev mail.Event, notice
if !domainMatchesEvent(d, ev.Domain) {
continue
}
- sender := senderAmong(ev, a.Sqid, d.Apps)
for _, app := range d.Apps {
- // The log first, and on its own terms: a failure here is
- // logged and must not cost the count the cap relies on.
- if sender == "" || sender == app {
- if err := s.store.RecordMailMessageEvent(ctx, a.Sqid, app, notice, ev, sender == app, time.Now()); err != nil {
- log.Printf("console: ses events: logging %s for %s/%s: %v", ev.Type, a.Sqid, app, err)
- }
+ if !named {
+ s.recordMailMessage(ctx, a.Sqid, app, key, ev, false, now)
}
if err := s.store.RecordMailEvent(ctx, a.Sqid, app, day, ev.Type); err != nil {
log.Printf("console: ses events: recording %s for %s/%s: %v", ev.Type, a.Sqid, app, err)
@@ -510,23 +518,29 @@ func (s *Server) recordAndEnforceMail(ctx context.Context, ev mail.Event, notice
}
}
-// senderAmong names the app SES says sent ev, if it is one of apps in
-// account acct — and "" when SES named no identity, named one that is not
-// CARLOS's, or named an app in another account or not on this domain. ""
-// is the fallback to attribution by domain, which is what every event had
-// before SES's sender was read at all: no narrower, and never pointed at an
-// app the domain does not list.
-func senderAmong(ev mail.Event, acct string, apps []string) string {
- callerAcct, callerApp, ok := mail.AppOfCaller(ev.Caller)
- if !ok || callerAcct != acct {
- return ""
- }
- for _, app := range apps {
- if app == callerApp {
- return app
- }
+// recordMailMessage keeps one notification in one app's log, if that app is
+// live. A trashed app keeps nothing new: its name is not a member-facing app
+// any more, and what it is sent is not somebody else's to read after a
+// restore or a claim. Best effort, like the rest of the ingest: a failure
+// is logged and costs neither the count nor the 200.
+func (s *Server) recordMailMessage(ctx context.Context, acct, app, key string, ev mail.Event, bySender bool, now time.Time) {
+ if !s.mailAppLive(acct, app) {
+ return
+ }
+ if err := s.store.RecordMailMessageEvent(ctx, acct, app, key, ev, bySender, now); err != nil {
+ log.Printf("console: ses events: logging %s for %s/%s: %v", ev.Type, acct, app, err)
+ }
+}
+
+// mailAppLive reports whether acct (a sqid) holds app as a live, untrashed
+// app right now.
+func (s *Server) mailAppLive(acct, app string) bool {
+ a, err := s.store.AccountBySqid(acct)
+ if err != nil || a == nil {
+ return false
}
- return ""
+ ok, err := s.store.HasAppIn(a.ID, app)
+ return err == nil && ok
}
// confirmSNSSubscription GETs the verified, host-checked SubscribeURL. It is
diff --git a/internal/console/store.go b/internal/console/store.go
index 575adebfb..5ffbc7e9e 100644
--- a/internal/console/store.go
+++ b/internal/console/store.go
@@ -6,6 +6,7 @@
package console
import (
+ "context"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
@@ -1354,8 +1355,29 @@ func (s *Store) RestoreAppRow(accountID int64, app string) error {
// auto-purge pass this feeds must never be the thing that frees a serving
// app's name out from under it.
func (s *Store) PurgeAppRow(accountID int64, app string) error {
- _, err := s.db.Exec(`DELETE FROM apps WHERE account_id = ? AND app = ? AND deleted_at != 0`, accountID, app)
- return err
+ ctx := context.Background()
+ tx, err := s.db.BeginTx(ctx, nil)
+ if err != nil {
+ return err
+ }
+ defer tx.Rollback()
+ res, err := tx.ExecContext(ctx, `DELETE FROM apps WHERE account_id = ? AND app = ? AND deleted_at != 0`, accountID, app)
+ if err != nil {
+ return err
+ }
+ if n, _ := res.RowsAffected(); n > 0 {
+ // The name is free from here, so nothing keyed by it may outlive the
+ // row (mailevents.go): a later app of the same name starts clean.
+ var sqid string
+ if err := tx.QueryRowContext(ctx, `SELECT sqid FROM accounts WHERE id = ?`, accountID).Scan(&sqid); err == nil {
+ if err := forgetMailApp(ctx, tx, sqid, app); err != nil {
+ return err
+ }
+ } else if err != sql.ErrNoRows {
+ return err
+ }
+ }
+ return tx.Commit()
}
// TrashedApp is one app in an account's Trash: past-tense on purpose, since a
@@ -1534,6 +1556,10 @@ LIMIT 1`, cliKindService, to, cliKindService, from, app).Scan(&collidingName)
if err == nil {
return fmt.Errorf("console: the destination account already holds a live service credential named %q — rename or revoke it first", collidingName)
}
+ var createdAt int64
+ if err := tx.QueryRow(`SELECT created_at FROM apps WHERE app = ? AND account_id = ?`, app, from).Scan(&createdAt); err != nil {
+ return err
+ }
if _, err := tx.Exec(`UPDATE apps SET account_id = ? WHERE app = ? AND account_id = ?`,
to, app, from); err != nil {
return err
@@ -1542,6 +1568,18 @@ LIMIT 1`, cliKindService, to, cliKindService, from, app).Scan(&collidingName)
to, cliKindApp, cliKindService, from, app); err != nil {
return err
}
+ // Email history is keyed by the account's sqid, not its id, so it is
+ // carried by sqid (mailevents.go).
+ var fromSqid, toSqid string
+ if err := tx.QueryRow(`SELECT sqid FROM accounts WHERE id = ?`, from).Scan(&fromSqid); err != nil {
+ return err
+ }
+ if err := tx.QueryRow(`SELECT sqid FROM accounts WHERE id = ?`, to).Scan(&toSqid); err != nil {
+ return err
+ }
+ if err := carryMailApp(context.Background(), tx, fromSqid, app, toSqid, app, createdAt); err != nil {
+ return err
+ }
return tx.Commit()
}
@@ -1616,6 +1654,13 @@ func (s *Store) RenameAppIn(accountID int64, from, to string) error {
to, cliKindApp, cliKindService, accountID, from); err != nil {
return err
}
+ var sqid string
+ if err := tx.QueryRow(`SELECT sqid FROM accounts WHERE id = ?`, accountID).Scan(&sqid); err != nil {
+ return err
+ }
+ if err := carryMailApp(context.Background(), tx, sqid, from, sqid, to, createdAt); err != nil {
+ return err
+ }
return tx.Commit()
}
diff --git a/internal/mail/events.go b/internal/mail/events.go
index e7a9694d3..e830e02eb 100644
--- a/internal/mail/events.go
+++ b/internal/mail/events.go
@@ -8,6 +8,7 @@ import (
"fmt"
"strings"
"time"
+ "unicode"
"unicode/utf8"
)
@@ -26,7 +27,11 @@ type Event struct {
Domain string
From string
MessageID string
- At time.Time
+ // At is when what the event reports happened: the bounce, the delivery,
+ // the complaint — falling back to the send, which is all a Send or a
+ // Reject has. SentAt is always the send.
+ At time.Time
+ SentAt time.Time
// Caller is the IAM identity SES says sent the message — the automatic
// "ses:caller-identity" tag. CARLOS mints one per app (UserName), so
@@ -81,7 +86,10 @@ const (
// answered, and it ends up in a table and on a terminal.
func cleanField(s string, max int) string {
s = strings.Map(func(r rune) rune {
- if r < 32 || r == 127 {
+ // C0, DEL and C1 (U+009B is a CSI on some terminals), and the
+ // invisible formatting class — bidi overrides among it — which can
+ // make a line on a terminal say something the bytes do not.
+ if unicode.IsControl(r) || unicode.Is(unicode.Cf, r) {
return ' '
}
return r
@@ -150,15 +158,18 @@ func ParseEvent(body []byte) (Event, error) {
Tags map[string][]string `json:"tags"`
} `json:"mail"`
Bounce struct {
+ Timestamp string `json:"timestamp"`
BounceType string `json:"bounceType"`
BounceSubType string `json:"bounceSubType"`
BouncedRecipients []recipient `json:"bouncedRecipients"`
} `json:"bounce"`
Complaint struct {
+ Timestamp string `json:"timestamp"`
ComplaintFeedbackType string `json:"complaintFeedbackType"`
ComplainedRecipients []recipient `json:"complainedRecipients"`
} `json:"complaint"`
Delivery struct {
+ Timestamp string `json:"timestamp"`
Recipients []string `json:"recipients"`
SMTPResponse string `json:"smtpResponse"`
} `json:"delivery"`
@@ -166,6 +177,7 @@ func ParseEvent(body []byte) (Event, error) {
Reason string `json:"reason"`
} `json:"reject"`
DeliveryDelay struct {
+ Timestamp string `json:"timestamp"`
DelayType string `json:"delayType"`
DelayedRecipients []recipient `json:"delayedRecipients"`
} `json:"deliveryDelay"`
@@ -216,6 +228,19 @@ func ParseEvent(body []byte) (Event, error) {
}
if raw.Mail.Timestamp != "" {
if at, err := time.Parse(time.RFC3339, raw.Mail.Timestamp); err == nil {
+ ev.SentAt = at
+ ev.At = at
+ }
+ }
+ // When the thing this event reports happened, where SES says: a bounce
+ // or a delay can arrive a day after the send, and an event dated by the
+ // send is both wrong on a screen and invisible to "since yesterday".
+ eventTime := map[string]string{
+ EventBounce: raw.Bounce.Timestamp, EventComplaint: raw.Complaint.Timestamp,
+ EventDelivery: raw.Delivery.Timestamp, EventDeliveryDelay: raw.DeliveryDelay.Timestamp,
+ }[raw.EventType]
+ if eventTime != "" {
+ if at, err := time.Parse(time.RFC3339, eventTime); err == nil {
ev.At = at
}
}
diff --git a/internal/mail/events_test.go b/internal/mail/events_test.go
index 89853372a..33b58e96e 100644
--- a/internal/mail/events_test.go
+++ b/internal/mail/events_test.go
@@ -1,8 +1,10 @@
package mail
import (
+ "encoding/json"
"strings"
"testing"
+ "time"
)
// SES's event JSON nests the From address under mail.source and the type
@@ -304,9 +306,13 @@ func TestAppOfCallerReadsBackTheDeliveredIdentity(t *testing.T) {
{UserName("beb", "jb-book"), "beb", "jb-book", true},
{StandaloneUserName("beb", "jb-book", "laptop"), "beb", "jb-book", true},
{"arn:aws:iam::123456789012:user/carlos/mail/carlos-mail-beb-jb-book", "beb", "jb-book", true},
- {"carlos-mail-beb", "", "", false}, // no app
- {"carlos-mail-BEB-jb-book", "", "", false}, // not an account sqid
- {"someone-else-entirely", "", "", false}, // not ours
+ {"arn:aws:sts::123456789012:assumed-role/x/carlos-mail-beb-jb-book", "", "", false}, // a role session names itself
+ {"arn:aws:iam::123456789012:role/carlos-mail-beb-jb-book", "", "", false}, // not a user
+ {"carlos/mail/carlos-mail-beb-jb-book", "", "", false}, // a path with no ARN
+ {"carlos-mail-beb-jb-book+s+", "", "", false}, // a standalone with no name
+ {"carlos-mail-beb", "", "", false}, // no app
+ {"carlos-mail-BEB-jb-book", "", "", false}, // not an account sqid
+ {"someone-else-entirely", "", "", false}, // not ours
{"", "", "", false},
} {
acct, app, ok := AppOfCaller(c.caller)
@@ -315,3 +321,44 @@ func TestAppOfCallerReadsBackTheDeliveredIdentity(t *testing.T) {
}
}
}
+
+// A terminal can be made to show something the bytes do not say: an 8-bit
+// CSI, or a right-to-left override that reverses the rest of the line. The
+// far end writes the diagnostic, so none of that is kept.
+func TestEventTextLosesInvisibleAndEightBitControls(t *testing.T) {
+ sneaky := []rune{0x9b, 0x202e, 0x202c, 0x200b}
+ diag := "550 " + string(sneaky[0]) + "31m red " + string(sneaky[1]) + "evil" + string(sneaky[2]) + string(sneaky[3]) + " ok"
+ quoted, _ := json.Marshal(diag)
+ ev, err := ParseEvent([]byte(`{"eventType":"Bounce","mail":{"source":"a@lgc.li"},"bounce":{"bouncedRecipients":[
+ {"emailAddress":"ann@example.com","diagnosticCode":` + string(quoted) + `}]}}`))
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, r := range ev.Outcomes[0].Diagnostic {
+ for _, bad := range sneaky {
+ if r == bad {
+ t.Errorf("%U survived in %q", r, ev.Outcomes[0].Diagnostic)
+ }
+ }
+ }
+}
+
+// A bounce can come a day after the send. The event is dated by the bounce,
+// and the send is kept beside it.
+func TestAnEventIsDatedByWhatItReports(t *testing.T) {
+ ev, err := ParseEvent([]byte(`{"eventType":"Bounce","mail":{"source":"a@lgc.li","timestamp":"2026-09-14T08:00:00Z"},
+ "bounce":{"timestamp":"2026-09-15T09:30:00Z","bouncedRecipients":[{"emailAddress":"ann@example.com"}]}}`))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got := ev.At.UTC().Format(time.RFC3339); got != "2026-09-15T09:30:00Z" {
+ t.Errorf("At = %s, want the bounce's own time", got)
+ }
+ if got := ev.SentAt.UTC().Format(time.RFC3339); got != "2026-09-14T08:00:00Z" {
+ t.Errorf("SentAt = %s, want the send", got)
+ }
+ send, _ := ParseEvent([]byte(`{"eventType":"Send","mail":{"source":"a@lgc.li","timestamp":"2026-09-14T08:00:00Z"}}`))
+ if !send.At.Equal(send.SentAt) || send.At.IsZero() {
+ t.Errorf("a Send is dated by the send: %v / %v", send.At, send.SentAt)
+ }
+}
diff --git a/internal/mail/policy.go b/internal/mail/policy.go
index 100db4d8a..7654d3923 100644
--- a/internal/mail/policy.go
+++ b/internal/mail/policy.go
@@ -33,21 +33,34 @@ func UserName(acct, app string) string { return "carlos-mail-" + acct + "-" + ap
// (release.ValidAccount), so the first hyphen after the prefix is where the
// account ends, and everything after it is the app, hyphens and all.
//
-// SES is documented to report the identity's name; an ARN is accepted too,
-// by its last path segment, so a change in which of the two SES sends does
-// not quietly stop naming the sender. Anything that is not one of CARLOS's
-// own identities answers false, and the event falls back to being
-// attributed by its sending domain alone.
+// SES is documented to report the identity's name. An IAM USER ARN is
+// accepted too, by its last path segment, so a change in which of the two
+// SES sends does not quietly stop naming the sender — but only a user ARN:
+// an assumed-role session is named by whoever assumes the role, and a
+// session named carlos-mail-<acct>-<app> is not that app. Anything that is
+// not one of CARLOS's own identities answers false.
func AppOfCaller(caller string) (acct, app string, ok bool) {
name := caller
- if i := strings.LastIndex(name, "/"); i >= 0 {
- name = name[i+1:]
+ if strings.HasPrefix(name, "arn:") {
+ _, resource, found := strings.Cut(name, ":user/")
+ if !found || !strings.HasPrefix(name, "arn:aws") || strings.Count(name[:strings.Index(name, ":user/")], ":") != 4 {
+ return "", "", false
+ }
+ name = resource
+ if i := strings.LastIndex(name, "/"); i >= 0 {
+ name = name[i+1:]
+ }
+ } else if strings.Contains(name, "/") {
+ return "", "", false
}
rest, found := strings.CutPrefix(name, "carlos-mail-")
if !found {
return "", "", false
}
if i := strings.Index(rest, "+s+"); i >= 0 {
+ if rest[i+3:] == "" {
+ return "", "", false // a standalone credential always has a name
+ }
rest = rest[:i]
}
acct, app, found = strings.Cut(rest, "-")