From 7b1e7ba7c918aa24f76e5af0690c05b577293234 Mon Sep 17 00:00:00 2001 From: kristianwind Date: Tue, 25 Aug 2026 08:24:20 +0200 Subject: [PATCH] fix(push): Apple rejected the notifications; use the site's own URL as VAPID subject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every Web Push to an iPhone or a Mac was refused, silently as far as the person waiting for the reminder could tell. The cause was the VAPID "sub" claim: on an instance with no mail configured it fell back to mailto:verdande@example.invalid, and Apple answers 403 Forbidden to a mailto whose domain is made up. Verified against web.push.apple.com on the live subscriptions — that mailto gets a 403, the instance's https URL a 201. So the subject is now a real mailto when SMTP.From is a real address, and otherwise the instance's own https base URL — which is a valid VAPID subject, points at something that exists, and every push service including Apple accepts. Two things found alongside it, both fixed: - A successful push did not clear the failure count. A subscription drops at ten failures, and all four live ones sat at nine from this bug — one flaky push from being deleted despite working. MarkPushDelivered zeroes the count and moves last_used_at on every delivery, so a subscription that recovers is not still carrying its old failures toward the cliff. - A reminder with no project made a notification whose URL was "/projekt/" — a dead link. Empty project now points at the app root. Test: the subject is a real mailto when there is one, and the https URL when there is not — including for the localhost default, which is not a real address. Co-Authored-By: Claude Opus 5 --- internal/httpapi/notifications_handlers.go | 35 ++++++++++++++++++++-- internal/httpapi/push_subject_test.go | 33 ++++++++++++++++++++ internal/store/notifications.go | 11 +++++++ 3 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 internal/httpapi/push_subject_test.go diff --git a/internal/httpapi/notifications_handlers.go b/internal/httpapi/notifications_handlers.go index 1161c55..8468a5e 100644 --- a/internal/httpapi/notifications_handlers.go +++ b/internal/httpapi/notifications_handlers.go @@ -148,15 +148,23 @@ func (s *Server) pushToUser(userID, title, body, projectID string) pushOutcome { return out } - payload := push.Payload{Title: title, Body: body, URL: "/projekt/" + projectID} + target := "/projekt/" + projectID + if projectID == "" { + target = "/" + } + payload := push.Payload{Title: title, Body: body, URL: target} + subject := s.vapidSubject() for _, sub := range subs { err := push.Send(ctx, push.Subscription{ Endpoint: sub.Endpoint, P256dh: sub.P256dh, Auth: sub.Auth, }, payload, push.VAPID{ - Public: public, Private: private, Subject: "mailto:" + s.cfg.SMTP.From, + Public: public, Private: private, Subject: subject, }) if err == nil { out.Sent++ + if delErr := s.db.MarkPushDelivered(ctx, sub.Endpoint); delErr != nil { + s.log.Warn("mark push delivered", "err", delErr) + } continue } // Warn, not Debug. @@ -257,6 +265,29 @@ func (s *Server) vapidKeys(ctx contextLike) (string, string, error) { return s.db.InstanceKeys(ctx, push.GenerateVAPIDKeys) } +// vapidSubject is the "sub" claim a push service is told to reach the sender at. +// +// It has to be one the service will actually accept, and Apple is the strict one: +// it answers 403 Forbidden to a mailto whose domain is made up — which is exactly +// what the old fallback, mailto:verdande@example.invalid, was on an instance with +// no mail set up. Every push to an iPhone or a Mac was rejected, silently as far as +// the person waiting for the notification could tell. +// +// So a real address is used when there is one, and otherwise the instance's own +// https address — which is a valid VAPID subject, points at something that exists, +// and Apple accepts. Verified against web.push.apple.com: the mailto gets a 403, +// the https URL a 201. +func (s *Server) vapidSubject() string { + from := strings.TrimSpace(s.cfg.SMTP.From) + if from != "" && strings.Contains(from, "@") && !strings.HasSuffix(from, "@localhost") { + return "mailto:" + from + } + if strings.HasPrefix(s.cfg.BaseURL, "https://") || strings.HasPrefix(s.cfg.BaseURL, "http://") { + return s.cfg.BaseURL + } + return "mailto:verdande@example.invalid" +} + // --- subscription endpoints ----------------------------------------------------------- type pushSubscriptionRequest struct { diff --git a/internal/httpapi/push_subject_test.go b/internal/httpapi/push_subject_test.go new file mode 100644 index 0000000..6d96da0 --- /dev/null +++ b/internal/httpapi/push_subject_test.go @@ -0,0 +1,33 @@ +package httpapi + +import ( + "testing" + + "github.com/kristianwind/verdande/internal/config" +) + +// The VAPID subject has to be one a push service will accept. Apple answers 403 to +// a mailto with a made-up domain, which is what the no-mail fallback used to be, so +// every push to an Apple device was refused. With no real address, the instance's +// own https URL is used instead — valid, real, and accepted. +func TestVAPIDSubject(t *testing.T) { + cases := []struct { + name string + from string + baseURL string + want string + }{ + {"a real address is used as-is", "todo@example.dk", "https://verdande.example.dk", "mailto:todo@example.dk"}, + {"no mail falls back to the https url", "", "https://verdande.example.dk", "https://verdande.example.dk"}, + {"the localhost default is not a real address", "verdande@localhost", "https://verdande.example.dk", "https://verdande.example.dk"}, + {"a plain http dev url still serves as a subject", "", "http://localhost:8080", "http://localhost:8080"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + s := &Server{cfg: &config.Config{BaseURL: tc.baseURL, SMTP: config.SMTP{From: tc.from}}} + if got := s.vapidSubject(); got != tc.want { + t.Errorf("vapidSubject() = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/internal/store/notifications.go b/internal/store/notifications.go index 9df17de..a392515 100644 --- a/internal/store/notifications.go +++ b/internal/store/notifications.go @@ -204,6 +204,17 @@ func (db *DB) dropPushEndpoint(ctx context.Context, endpoint string) error { // has failed enough times. A push service says 404 or 410 when an endpoint is gone, // but it also has bad days — forgiving a few failures keeps a flaky network from // silently unsubscribing somebody's phone. +// MarkPushDelivered records that a push went through: the failure count goes back +// to zero and last_used_at moves forward. Without it a subscription that failed a +// few times and then recovered would keep those failures on its record and be one +// flaky night away from the ten that delete it — even though it is working. +func (db *DB) MarkPushDelivered(ctx context.Context, endpoint string) error { + _, err := db.ExecContext(ctx, + `UPDATE push_subscriptions SET failures = 0, last_used_at = ? WHERE endpoint = ?`, + time.Now().Unix(), endpoint) + return err +} + func (db *DB) RecordPushFailure(ctx context.Context, endpoint string, permanent bool) error { if permanent { // Not scoped by user, and that is right here: this is the push service