amadan

carlos / platform Public

console: name the sender in a pause/resume audit line only when it isn't the app sender (final review, item 3)

The pause/resume audit detail changed from "harbourmail: ..." to
"harbourmail/app: ...". These lines render on the app overview page,
so they are member-facing copy that never went through this repo's
copy-review gate. Every app has exactly one sender this release, so
"/app" carried no information a member could use.

mailAuditSubject(app, sender) returns the bare app name when sender is
the app sender, and "app/sender" otherwise, so every live app's audit
line is byte-identical to what it printed before senders existed.
Pinned with exact-match tests for both the pause and resume lines, plus
a test that a non-app sender still gets named.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell pushed by claude 1ab138a3f810c54f75ad70a9eb9518ab767879bf parent 1a642a6
2 files changed, +126 −2
  • internal/console/mailcounts.go +18 −2
  • internal/console/mailcounts_test.go +108 −0
diff --git a/internal/console/mailcounts.go b/internal/console/mailcounts.go
index eabf11d0b..2a6866af2 100644
--- a/internal/console/mailcounts.go
+++ b/internal/console/mailcounts.go
@@ -193,7 +193,7 @@ func (s *Server) resumeStaleCapPause(ctx context.Context, acct, app, sender stri
if a, err := s.store.AccountBySqid(acct); err != nil {
log.Printf("console: mail resume audit: resolving account %s: %v", acct, err)
} else if a != nil {
- s.audit(a, app, "ses", "mail.resumed", fmt.Sprintf("%s/%s: daily cap pause cleared on the UTC day roll", app, sender))
+ s.audit(a, app, "ses", "mail.resumed", fmt.Sprintf("%s: daily cap pause cleared on the UTC day roll", mailAuditSubject(app, sender)))
}
fresh, _, err := mail.GetSender(ctx, s.cfg.Releases, acct, app, sender)
if err != nil {
@@ -283,11 +283,27 @@ func (s *Server) enforceMailLimits(ctx context.Context, acct, app, sender string
if a, err := s.store.AccountBySqid(acct); err != nil {
log.Printf("console: mail pause audit: resolving account %s: %v", acct, err)
} else if a != nil {
- s.audit(a, app, "ses", "mail.paused", fmt.Sprintf("%s/%s: %s", app, sender, reason))
+ s.audit(a, app, "ses", "mail.paused", fmt.Sprintf("%s: %s", mailAuditSubject(app, sender), reason))
}
return true, reason, nil
}
+// mailAuditSubject names what a pause or resume audit line is about. These
+// lines render on the app overview page, so they are member-facing copy
+// that never went through this repo's copy-review gate for the "/sender"
+// form. Every app has exactly one sender today (the app sender, mail.
+// SenderAppID), so naming it on that line would add "/app" for zero
+// information a member could use, and would be an unreviewed copy change.
+// Only a sender OTHER than the app sender earns the "/sender" suffix, which
+// keeps every live app's audit line byte-identical to what it already
+// prints.
+func mailAuditSubject(app, sender string) string {
+ if sender == mail.SenderAppID {
+ return app
+ }
+ return app + "/" + sender
+}
+
// RATE AND MANUAL PAUSES NEVER AUTO-RESUME. A RATE pause that lifted itself
// at midnight would let an abusive tenant breach SES's account-review
// thresholds again every day forever, which is not a limit — it is a
diff --git a/internal/console/mailcounts_test.go b/internal/console/mailcounts_test.go
index 61a2a0ab0..d36654fc0 100644
--- a/internal/console/mailcounts_test.go
+++ b/internal/console/mailcounts_test.go
@@ -719,3 +719,111 @@ func TestResumeStaleCapPauseNeverTouchesAPerInstanceSender(t *testing.T) {
t.Fatal("a PerInstance sender's cap pause from yesterday was lifted")
}
}
+
+// mailAuditSubject is the naming rule directly: every app has exactly one
+// sender in this release (the app sender), so naming it would add "/app" to
+// a member-facing audit line for no information a member could use — and
+// that "/app" form never went through this repo's copy-review gate. Only a
+// sender that is NOT the app sender earns the "/sender" suffix.
+func TestMailAuditSubjectNamesOnlyANonAppSender(t *testing.T) {
+ if got := mailAuditSubject("harbourmail", mail.SenderAppID); got != "harbourmail" {
+ t.Fatalf("mailAuditSubject(app sender) = %q, want the bare app name", got)
+ }
+ if got := mailAuditSubject("harbourmail", "book.example.com"); got != "harbourmail/book.example.com" {
+ t.Fatalf("mailAuditSubject(non-app sender) = %q, want app/sender", got)
+ }
+}
+
+// The app sender's pause line must be byte-identical to what every live app
+// already prints today (pre-dating per-sender senders): "harbourmail: daily
+// cap 50 reached (50 sent)". Naming the sender here would be a member-facing
+// copy change this repo's copy-review gate never saw.
+func TestEnforceAuditDetailIsByteIdenticalForTheAppSender(t *testing.T) {
+ fx, now := mailFixture(t)
+ seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 50})
+ if _, _, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, now); err != nil {
+ t.Fatal(err)
+ }
+
+ entries, err := fx.st.Events(OpsAccountID, "harbourmail", 10)
+ if err != nil {
+ t.Fatal(err)
+ }
+ want := "harbourmail: daily cap 50 reached (50 sent)"
+ for _, e := range entries {
+ if e.Action == "mail.paused" {
+ if e.Detail != want {
+ t.Fatalf("pause detail = %q, want byte-identical %q", e.Detail, want)
+ }
+ return
+ }
+ }
+ t.Fatalf("no mail.paused audit entry found: %+v", entries)
+}
+
+// A non-app sender's pause line DOES carry the sender, since "harbourmail"
+// alone would not tell a member which of two senders paused.
+func TestEnforceAuditDetailNamesANonAppSender(t *testing.T) {
+ fx, now := mailFixture(t)
+ ctx := context.Background()
+ day := now.UTC().Format("2006-01-02")
+
+ sibling := &mail.Sender{
+ ID: "book.example.com", Scope: mail.ScopeInstance, Source: mail.SourceDomain,
+ Provisioned: &mail.Provisioned{IAMUser: "book-u", KeyID: "AKIA2", Active: true, At: now},
+ }
+ if _, err := mail.PutSender(ctx, fx.rel, release.OpsAccount, "harbourmail", sibling, ""); err != nil {
+ t.Fatal(err)
+ }
+ for i := 0; i < 50; i++ {
+ if err := fx.st.RecordMailEvent(ctx, release.OpsAccount, "harbourmail", "book.example.com", day, mail.EventSend); err != nil {
+ t.Fatal(err)
+ }
+ }
+ if _, _, err := fx.srv.enforceMailLimits(ctx, release.OpsAccount, "harbourmail", "book.example.com", now); err != nil {
+ t.Fatal(err)
+ }
+
+ entries, err := fx.st.Events(OpsAccountID, "harbourmail", 10)
+ if err != nil {
+ t.Fatal(err)
+ }
+ want := "harbourmail/book.example.com: daily cap 50 reached (50 sent)"
+ for _, e := range entries {
+ if e.Action == "mail.paused" {
+ if e.Detail != want {
+ t.Fatalf("pause detail = %q, want %q", e.Detail, want)
+ }
+ return
+ }
+ }
+ t.Fatalf("no mail.paused audit entry found: %+v", entries)
+}
+
+// The resume line follows the same rule: byte-identical for the app sender.
+func TestResumeAuditDetailIsByteIdenticalForTheAppSender(t *testing.T) {
+ fx, now := mailFixture(t)
+ pauseDirectly(t, fx, "harbourmail", mail.PauseKindCap, now.AddDate(0, 0, -1))
+ sn, _, err := mail.GetSender(context.Background(), fx.rel, release.OpsAccount, "harbourmail", mail.SenderAppID)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, resumed, err := fx.srv.resumeStaleCapPause(context.Background(), release.OpsAccount, "harbourmail", mail.SenderAppID, sn, now); err != nil || !resumed {
+ t.Fatalf("resume = %v %v, want it to resume", resumed, err)
+ }
+
+ entries, err := fx.st.Events(OpsAccountID, "harbourmail", 10)
+ if err != nil {
+ t.Fatal(err)
+ }
+ want := "harbourmail: daily cap pause cleared on the UTC day roll"
+ for _, e := range entries {
+ if e.Action == "mail.resumed" {
+ if e.Detail != want {
+ t.Fatalf("resume detail = %q, want byte-identical %q", e.Detail, want)
+ }
+ return
+ }
+ }
+ t.Fatalf("no mail.resumed audit entry found: %+v", entries)
+}