diff --git a/core/pkg/gateway/dependencies.go b/core/pkg/gateway/dependencies.go index 6820aa2..0812622 100644 --- a/core/pkg/gateway/dependencies.go +++ b/core/pkg/gateway/dependencies.go @@ -969,7 +969,14 @@ func buildPushDispatcher( }, logger.Logger)) } // APNs is fully credentialed — no YAML fallback. The presence of - // per-namespace credentials is the trigger. + // per-namespace credentials is the trigger. Bugboard #408: a + // single set of APNs credentials spawns BOTH an alert-kind + // provider (registered as "apns") AND a VoIP/PushKit provider + // (registered as "apns_voip"). Both share the same JWT signer + + // HTTP/2 client pool — VoIP only differs in the per-Send wire + // format (topic suffix, apns-push-type header, empty-payload + // acceptance). Tenants register PushKit voipPushTokens against + // provider="apns_voip" and the dispatcher routes accordingly. if c.Namespace != "" && credManager != nil { if cred, err := credManager.Get(ctx, c.Namespace, "apns"); err == nil && cred != nil { if apnsCfg, perr := pushapns.ParseCredentials(cred.JSON); perr == nil { @@ -981,6 +988,14 @@ func buildPushDispatcher( zap.String("namespace", c.Namespace), zap.Error(nerr)) } + if voipProvider, nerr := pushapns.NewVoIP(apnsCfg, logger.Logger); nerr == nil { + ps = append(ps, voipProvider) + } else { + logger.ComponentWarn(logging.ComponentGeneral, + "apns_voip provider construction failed", + zap.String("namespace", c.Namespace), + zap.Error(nerr)) + } } else { logger.ComponentWarn(logging.ComponentGeneral, "apns credentials parse failed", diff --git a/core/pkg/gateway/handlers/push/handlers.go b/core/pkg/gateway/handlers/push/handlers.go index 5b1e35f..828bc27 100644 --- a/core/pkg/gateway/handlers/push/handlers.go +++ b/core/pkg/gateway/handlers/push/handlers.go @@ -13,10 +13,18 @@ import ( // validProviders is the allowlist for the `provider` field on RegisterDevice. // Keep in sync with what the dispatcher actually has registered at startup. +// +// "apns_voip" (bugboard #408) is the PushKit/CallKit variant of "apns" — +// same underlying credentials, distinct dispatcher entry. Tenants +// register a second PushDevice row per iPhone with the PushKit +// voipPushToken to enable CallKit-triggering incoming-call pushes, +// keyed by a distinct device_id (typically `:voip`) so the +// `device_id` PK doesn't collide with the alert-path row. var validProviders = map[string]struct{}{ - "ntfy": {}, - "expo": {}, - "apns": {}, // future — accepted at registration so apps can pre-flight + "ntfy": {}, + "expo": {}, + "apns": {}, + "apns_voip": {}, } // MaxTokenBytes caps the device-token length to prevent abuse. diff --git a/core/pkg/gateway/handlers/push/handlers_test.go b/core/pkg/gateway/handlers/push/handlers_test.go index 19509d4..9b4d450 100644 --- a/core/pkg/gateway/handlers/push/handlers_test.go +++ b/core/pkg/gateway/handlers/push/handlers_test.go @@ -131,6 +131,45 @@ func TestRegister_unknown_provider_rejected(t *testing.T) { } } +// TestRegister_validProviders_allowlist locks in the supported provider +// names so a future allowlist regression breaks immediately at test +// time instead of at AnChat's deploy time. Bugboard #408 added +// "apns_voip" to enable the PushKit/CallKit registration path — +// without this entry, every voipPushToken registration would fail +// with "unknown provider" at /v1/push/devices and no incoming-call +// signal could ever be delivered to an iPhone. +func TestRegister_validProviders_allowlist(t *testing.T) { + cases := []struct { + provider string + want int + }{ + {"ntfy", http.StatusOK}, + {"expo", http.StatusOK}, + {"apns", http.StatusOK}, + {"apns_voip", http.StatusOK}, // bugboard #408 + {"fcm", http.StatusBadRequest}, + {"", http.StatusBadRequest}, + } + for _, tc := range cases { + t.Run(tc.provider, func(t *testing.T) { + h := newHandlers(&fakeStore{}, nil) + body, _ := json.Marshal(RegisterDeviceRequest{ + DeviceID: "iphone-x", + Provider: tc.provider, + Token: "device-token", + Platform: "ios", + }) + req := withAuth(httptest.NewRequest(http.MethodPost, "/v1/push/devices", bytes.NewReader(body)), "ns", "u") + rr := httptest.NewRecorder() + h.RegisterDeviceHandler(rr, req) + if rr.Code != tc.want { + t.Errorf("provider=%q: status=%d; want %d (body: %s)", + tc.provider, rr.Code, tc.want, rr.Body.String()) + } + }) + } +} + func TestRegister_oversize_token_rejected(t *testing.T) { h := newHandlers(&fakeStore{}, nil) huge := make([]byte, MaxTokenBytes+1) diff --git a/core/pkg/push/dispatcher.go b/core/pkg/push/dispatcher.go index 030378e..5876907 100644 --- a/core/pkg/push/dispatcher.go +++ b/core/pkg/push/dispatcher.go @@ -97,6 +97,22 @@ func (d *PushDispatcher) SendToUserDetailed( if err != nil { return nil, fmt.Errorf("list devices: %w", err) } + // Bugboard #408 — target_provider filter. When the caller sets + // msg.TargetProvider, drop every device whose Provider doesn't match + // BEFORE we attempt sends or count anything. This lets a chat-alert + // path send only to "apns" devices while a call-push path sends only + // to "apns_voip" devices, even though both are registered on the + // same iPhone. Unset = fanout (back-compat for every existing + // caller, including unmigrated functions in other namespaces). + if msg.TargetProvider != "" { + filtered := devs[:0] + for _, dev := range devs { + if dev.Provider == msg.TargetProvider { + filtered = append(filtered, dev) + } + } + devs = filtered + } out := &SendDetailedResult{ Ok: true, // flipped to false on the first failure DevicesAttempted: len(devs), diff --git a/core/pkg/push/dispatcher_target_provider_test.go b/core/pkg/push/dispatcher_target_provider_test.go new file mode 100644 index 0000000..18225d2 --- /dev/null +++ b/core/pkg/push/dispatcher_target_provider_test.go @@ -0,0 +1,236 @@ +package push + +import ( + "context" + "sync" + "testing" + + "go.uber.org/zap" +) + +// Bugboard #408 — target_provider dispatcher filter. +// +// Pin the four behaviors that matter for the AnChat CallKit-on-text +// bug class: +// +// 1. With TargetProvider="apns" set, ONLY apns devices are attempted. +// VoIP-registered devices on the same iPhone are silently skipped +// so a chat message doesn't trigger CallKit. +// +// 2. With TargetProvider="apns_voip", ONLY VoIP devices are attempted — +// the alert device is skipped so an incoming-call signal doesn't +// produce a silent alert. +// +// 3. With TargetProvider unset (legacy callers, unmigrated functions), +// fan-out behavior is UNCHANGED — all devices attempted. This is +// the back-compat guarantee that lets us ship the filter without +// breaking every existing call site in every namespace. +// +// 4. DevicesAttempted in the SendDetailedResult reflects the +// POST-FILTER count, not the raw device-store count. WASM callers +// interpreting `attempted=0` as "no devices" need this to be the +// real attempted count, not "user has zero devices anywhere". + +// targetFilterDeviceStore returns a fixed device list and records what was +// asked for. PushDeviceStore-conformant for use as Dispatcher dep. +type targetFilterDeviceStore struct { + devices []PushDevice +} + +func (f *targetFilterDeviceStore) Upsert(ctx context.Context, dev PushDevice) error { return nil } +func (f *targetFilterDeviceStore) Delete(ctx context.Context, ns, id string) error { return nil } +func (f *targetFilterDeviceStore) ListForUser(ctx context.Context, ns, userID string) ([]PushDevice, error) { + return f.devices, nil +} + +// recordingProvider implements PushProvider and just records which +// device tokens it was asked to send to. Lets the test assert exactly +// which devices reached which provider. +type recordingProvider struct { + name string + mu sync.Mutex + sent []string // device tokens received +} + +func (r *recordingProvider) Name() string { return r.name } +func (r *recordingProvider) Send(ctx context.Context, msg PushMessage) error { + r.mu.Lock() + defer r.mu.Unlock() + r.sent = append(r.sent, msg.DeviceToken) + return nil +} +func (r *recordingProvider) tokens() []string { + r.mu.Lock() + defer r.mu.Unlock() + out := make([]string, len(r.sent)) + copy(out, r.sent) + return out +} + +// twoIPhoneDevicesUser returns the canonical AnChat scenario: one user +// with one iPhone registered TWICE — alert + voip — per the documented +// registration model. +func twoIPhoneDevicesUser() []PushDevice { + return []PushDevice{ + { + DeviceID: "ios-base", + Provider: "apns", + Token: "ALERT-TOKEN", + }, + { + DeviceID: "ios-base:voip", + Provider: "apns_voip", + Token: "VOIP-TOKEN", + }, + } +} + +func newTestDispatcher(t *testing.T, devs []PushDevice, providers ...PushProvider) *PushDispatcher { + t.Helper() + d := New(&targetFilterDeviceStore{devices: devs}, zap.NewNop()) + for _, p := range providers { + d.Register(p) + } + return d +} + +func TestDispatcher_TargetProvider_FiltersToApns(t *testing.T) { + alert := &recordingProvider{name: "apns"} + voip := &recordingProvider{name: "apns_voip"} + d := newTestDispatcher(t, twoIPhoneDevicesUser(), alert, voip) + + res, err := d.SendToUserDetailed(context.Background(), "ns", "u1", PushMessage{ + Title: "new message", + Body: "hi", + TargetProvider: "apns", + }) + if err != nil { + t.Fatalf("SendToUserDetailed: %v", err) + } + + // Alert got the message; VoIP did NOT — this is the CallKit-on-text + // bug guard. If voip.tokens() is non-empty here, message-push-handler + // would ring CallKit on every chat message AnChat users receive. + if got := alert.tokens(); len(got) != 1 || got[0] != "ALERT-TOKEN" { + t.Errorf("alert provider tokens = %v; want [ALERT-TOKEN]", got) + } + if got := voip.tokens(); len(got) != 0 { + t.Errorf("voip provider should NOT have been called (CallKit-on-text bug); got tokens=%v", got) + } + + // DevicesAttempted reflects POST-filter count, not raw device count. + // WASM callers parse this to decide whether to retry / log "no + // devices" — must be the real attempt count. + if res.DevicesAttempted != 1 { + t.Errorf("DevicesAttempted = %d; want 1 (post-filter)", res.DevicesAttempted) + } + if res.DevicesSucceeded != 1 { + t.Errorf("DevicesSucceeded = %d; want 1", res.DevicesSucceeded) + } + if len(res.Results) != 1 { + t.Errorf("Results len = %d; want 1", len(res.Results)) + } +} + +func TestDispatcher_TargetProvider_FiltersToApnsVoip(t *testing.T) { + alert := &recordingProvider{name: "apns"} + voip := &recordingProvider{name: "apns_voip"} + d := newTestDispatcher(t, twoIPhoneDevicesUser(), alert, voip) + + res, err := d.SendToUserDetailed(context.Background(), "ns", "u1", PushMessage{ + Data: map[string]interface{}{"call_id": "c-1"}, + TargetProvider: "apns_voip", + }) + if err != nil { + t.Fatalf("SendToUserDetailed: %v", err) + } + + if got := voip.tokens(); len(got) != 1 || got[0] != "VOIP-TOKEN" { + t.Errorf("voip provider tokens = %v; want [VOIP-TOKEN]", got) + } + if got := alert.tokens(); len(got) != 0 { + t.Errorf("alert provider should NOT have been called (call-push targets voip only); got tokens=%v", got) + } + if res.DevicesAttempted != 1 { + t.Errorf("DevicesAttempted = %d; want 1", res.DevicesAttempted) + } +} + +func TestDispatcher_TargetProvider_UnsetFansOut(t *testing.T) { + // Back-compat guarantee. Every existing function in every namespace + // that doesn't set target_provider must continue to see fan-out. + // If this regresses, every unmigrated push call site breaks. + alert := &recordingProvider{name: "apns"} + voip := &recordingProvider{name: "apns_voip"} + d := newTestDispatcher(t, twoIPhoneDevicesUser(), alert, voip) + + res, err := d.SendToUserDetailed(context.Background(), "ns", "u1", PushMessage{ + Title: "x", + // TargetProvider intentionally unset. + }) + if err != nil { + t.Fatalf("SendToUserDetailed: %v", err) + } + + if got := alert.tokens(); len(got) != 1 { + t.Errorf("fan-out: alert tokens = %v; want 1", got) + } + if got := voip.tokens(); len(got) != 1 { + t.Errorf("fan-out: voip tokens = %v; want 1", got) + } + if res.DevicesAttempted != 2 { + t.Errorf("DevicesAttempted = %d; want 2 (fan-out)", res.DevicesAttempted) + } +} + +func TestDispatcher_TargetProvider_NoMatchingDevices_NoOp(t *testing.T) { + // User has only an alert device; call-push-handler asks for + // target_provider="apns_voip". Expected: no error, zero attempts, + // Ok=true (a user with no matching device is not an error — same + // semantics as "user has zero devices anywhere"). + alert := &recordingProvider{name: "apns"} + voip := &recordingProvider{name: "apns_voip"} + d := newTestDispatcher(t, []PushDevice{ + {DeviceID: "ios-only", Provider: "apns", Token: "T"}, + }, alert, voip) + + res, err := d.SendToUserDetailed(context.Background(), "ns", "u1", PushMessage{ + TargetProvider: "apns_voip", + }) + if err != nil { + t.Fatalf("expected no error for no-matching-devices; got %v", err) + } + if !res.Ok { + t.Errorf("Ok = false; want true (no matching devices is not a failure)") + } + if res.DevicesAttempted != 0 { + t.Errorf("DevicesAttempted = %d; want 0", res.DevicesAttempted) + } + if len(alert.tokens()) != 0 || len(voip.tokens()) != 0 { + t.Error("no provider should have been called") + } +} + +func TestDispatcher_TargetProvider_LegacySendToUser_AlsoFilters(t *testing.T) { + // SendToUser delegates to SendToUserDetailed under the hood, so the + // filter should apply identically. Pin this so a future refactor + // can't split the two paths. + alert := &recordingProvider{name: "apns"} + voip := &recordingProvider{name: "apns_voip"} + d := newTestDispatcher(t, twoIPhoneDevicesUser(), alert, voip) + + err := d.SendToUser(context.Background(), "ns", "u1", PushMessage{ + Title: "x", + Body: "y", + TargetProvider: "apns", + }) + if err != nil { + t.Fatalf("SendToUser: %v", err) + } + if len(alert.tokens()) != 1 { + t.Errorf("alert should have been called; got %v", alert.tokens()) + } + if len(voip.tokens()) != 0 { + t.Errorf("voip should NOT have been called via SendToUser+target_provider; got %v", voip.tokens()) + } +} diff --git a/core/pkg/push/providers/apns/apns.go b/core/pkg/push/providers/apns/apns.go index f700e10..a6c5e93 100644 --- a/core/pkg/push/providers/apns/apns.go +++ b/core/pkg/push/providers/apns/apns.go @@ -21,10 +21,13 @@ import ( const defaultSendTimeout = 10 * time.Second // Provider is the APNs push.PushProvider implementation, scoped to one -// (Team ID, Key ID, p8 key, Bundle ID, Environment) tuple. Construct -// one per namespace via the gateway dependency factory. +// (Team ID, Key ID, p8 key, Bundle ID, Environment, Kind) tuple. +// Construct one per (namespace, kind) via the gateway dependency +// factory — typically one KindAlert + one KindVoIP instance per +// namespace, both sharing the same JWT signer. type Provider struct { bundleID string + kind Kind client pushClient logger *zap.Logger } @@ -45,10 +48,28 @@ type pushClient interface { PushWithContext(ctx apns2.Context, notification *apns2.Notification) (*apns2.Response, error) } -// New constructs a Provider from a parsed Config. Returns an error if -// the p8 key fails to parse — this surfaces config errors at gateway -// startup / first-send rather than at every Push call. +// New constructs a KindAlert Provider — the standard user-visible-alert +// APNs path. Back-compat constructor: callers that want VoIP/PushKit +// behavior should use NewVoIP. Returns an error if the p8 key fails to +// parse so config errors surface at gateway startup rather than at +// every Push call. func New(c Config, logger *zap.Logger) (*Provider, error) { + return buildProvider(c, KindAlert, logger) +} + +// NewVoIP constructs a KindVoIP Provider — the PushKit/CallKit path for +// incoming-call signals. Same credentials (Team ID, Key ID, p8 key, +// Bundle ID, Environment) as the alert Provider; the wire-format +// differences (topic = bundle_id+".voip", apns-push-type = "voip", +// empty-content payloads allowed) are handled in Send. Bugboard #408. +func NewVoIP(c Config, logger *zap.Logger) (*Provider, error) { + return buildProvider(c, KindVoIP, logger) +} + +// buildProvider is the shared constructor for both kinds. The kind +// field gates Send's per-kind branching; everything else (JWT signer, +// HTTP/2 client, timeout) is identical. +func buildProvider(c Config, kind Kind, logger *zap.Logger) (*Provider, error) { if logger == nil { logger = zap.NewNop() } @@ -79,13 +100,17 @@ func New(c Config, logger *zap.Logger) (*Provider, error) { client.HTTPClient.Timeout = defaultSendTimeout return &Provider{ bundleID: c.BundleID, + kind: kind, client: client, - logger: logger.Named("apns"), + logger: logger.Named(providerNameForKind(kind)), }, nil } -// Name implements push.PushProvider. -func (p *Provider) Name() string { return "apns" } +// Name implements push.PushProvider. Returns "apns" for KindAlert and +// "apns_voip" for KindVoIP — these are the names the dispatcher routes +// devices against (device.Provider field) and the validProviders +// allowlist at the registration handler accepts. +func (p *Provider) Name() string { return providerNameForKind(p.kind) } // ErrDeviceUnregistered is returned by Send when APNs responds with // "Unregistered" (HTTP 410) — the token is no longer valid because the @@ -116,7 +141,12 @@ func (p *Provider) Send(ctx context.Context, msg push.PushMessage) error { if msg.DeviceToken == "" { return push.ErrEmptyToken } - if !hasVisibleContent(msg) { + // VoIP/PushKit pushes legally have no visible alert content — iOS + // renders the CallKit UI from the `data` dict alone. Skipping the + // hasVisibleContent guard ONLY on the VoIP kind keeps the bugboard + // #348 silent-drop protection in place for the alert path while + // unblocking incoming-call signals on the VoIP path (#408). + if p.kind != KindVoIP && !hasVisibleContent(msg) { return push.ErrEmptyContent } payload, err := buildAPSPayload(msg) @@ -125,11 +155,15 @@ func (p *Provider) Send(ctx context.Context, msg push.PushMessage) error { } n := &apns2.Notification{ DeviceToken: msg.DeviceToken, - Topic: p.bundleID, + Topic: p.topicForKind(), Payload: payload, + PushType: p.pushTypeForKind(), } // Priority mapping: APNs uses 10 (immediate) / 5 (power-saving). - if msg.Priority == push.PriorityHigh { + // VoIP MUST use immediate (10) — Apple rejects "5" for voip pushes + // with `BadPriority`. We honor msg.Priority for alert; force high + // for voip regardless of what the caller passed. + if p.kind == KindVoIP || msg.Priority == push.PriorityHigh { n.Priority = apns2.PriorityHigh } else { n.Priority = apns2.PriorityLow @@ -182,6 +216,27 @@ func (p *Provider) Send(ctx context.Context, msg push.PushMessage) error { } } +// topicForKind returns the APNs `apns-topic` header value for this +// Provider's kind. PushKit / VoIP pushes MUST target the bundle ID +// suffixed with `.voip` — Apple routes those to the PushKit delivery +// path that wakes the app via CallKit. Alert pushes use the bare bundle. +func (p *Provider) topicForKind() string { + if p.kind == KindVoIP { + return p.bundleID + ".voip" + } + return p.bundleID +} + +// pushTypeForKind returns the APNs `apns-push-type` header value. +// Required since iOS 13 — Apple rejects pushes lacking this header at +// the edge with `MissingTopic`/`InvalidPushType` errors. +func (p *Provider) pushTypeForKind() apns2.EPushType { + if p.kind == KindVoIP { + return apns2.PushTypeVOIP + } + return apns2.PushTypeAlert +} + // hasVisibleContent reports whether the message has any payload field // that Apple will display or process. An APNs push with none of these // is silently 200'd by Apple AND dropped — that's the bugboard #348 diff --git a/core/pkg/push/providers/apns/apns_test.go b/core/pkg/push/providers/apns/apns_test.go index 69a2479..05764b3 100644 --- a/core/pkg/push/providers/apns/apns_test.go +++ b/core/pkg/push/providers/apns/apns_test.go @@ -38,12 +38,21 @@ func (f *fakePushClient) PushWithContext(ctx apns2.Context, n *apns2.Notificatio return f.resp, f.err } -// newTestProvider constructs a Provider with a stub pushClient, -// bypassing real APNs. +// newTestProvider constructs an alert-kind Provider with a stub +// pushClient, bypassing real APNs. Existing call sites get the same +// behavior as pre-#408 — no need to thread a Kind through every test. func newTestProvider(t *testing.T, bundle string, fake *fakePushClient) *Provider { + t.Helper() + return newTestProviderKind(t, bundle, KindAlert, fake) +} + +// newTestProviderKind constructs a Provider of the given kind for +// VoIP-path coverage. Bugboard #408. +func newTestProviderKind(t *testing.T, bundle string, kind Kind, fake *fakePushClient) *Provider { t.Helper() return &Provider{ bundleID: bundle, + kind: kind, client: fake, logger: zap.NewNop(), } diff --git a/core/pkg/push/providers/apns/credentials.go b/core/pkg/push/providers/apns/credentials.go index e351511..0981828 100644 --- a/core/pkg/push/providers/apns/credentials.go +++ b/core/pkg/push/providers/apns/credentials.go @@ -33,6 +33,38 @@ const ( EnvProduction Environment = "production" ) +// Kind selects the APNs delivery mode for a Provider instance. The same +// (Team ID, Key ID, p8 key, Bundle ID, Environment) tuple supports BOTH +// kinds — they differ only in the per-Send wire format (topic suffix, +// apns-push-type header, empty-payload acceptance). +// +// - KindAlert: standard user-visible alerts. Topic = bundle_id, +// apns-push-type = "alert", REQUIRES visible content. Provider +// name "apns". +// - KindVoIP: PushKit / CallKit incoming-call signals. Topic = +// bundle_id + ".voip", apns-push-type = "voip", ALLOWS empty +// content (iOS renders CallKit UI from data dict alone). Provider +// name "apns_voip". +// +// Bugboard #408. A single PUT of APNs credentials enables both kinds +// when the gateway factory spawns both Provider instances. +type Kind string + +const ( + KindAlert Kind = "alert" + KindVoIP Kind = "voip" +) + +// providerNameForKind returns the dispatcher-registered name for a +// given Kind. Keep in sync with the validProviders allowlist in +// pkg/gateway/handlers/push/handlers.go. +func providerNameForKind(k Kind) string { + if k == KindVoIP { + return "apns_voip" + } + return "apns" +} + // Config is the per-namespace APNs credential record. JSON tags mirror // the public schema tenants PUT to /v1/namespace/push-credentials/apns. // diff --git a/core/pkg/push/providers/apns/voip_test.go b/core/pkg/push/providers/apns/voip_test.go new file mode 100644 index 0000000..aa6ed24 --- /dev/null +++ b/core/pkg/push/providers/apns/voip_test.go @@ -0,0 +1,187 @@ +package apns + +import ( + "context" + "net/http" + "testing" + + "github.com/DeBrosOfficial/network/pkg/push" + "github.com/sideshow/apns2" +) + +// Bugboard #408 — KindVoIP / PushKit Provider variant. +// +// These tests pin the three places where the VoIP path MUST differ +// from the alert path: +// +// 1. apns-topic header gets the ".voip" suffix appended (Apple routes +// this to the PushKit delivery system that wakes the app via +// CallKit; without the suffix, Apple silently rejects the push or +// ignores PushKit semantics). +// +// 2. apns-push-type header is "voip" (required since iOS 13; without +// it Apple rejects at the edge with InvalidPushType). +// +// 3. hasVisibleContent guard is SKIPPED. VoIP pushes legally have no +// alert content — iOS renders the CallKit UI from the `data` dict +// alone (caller name, call ID, etc.). The bugboard #348 empty- +// content guard would reject these — we bypass it ONLY on the +// VoIP kind so the alert path keeps its silent-drop protection. +// +// 4. Priority is forced to HIGH regardless of msg.Priority — Apple +// rejects VoIP pushes with priority 5 (`BadPriority`). +// +// Without these, the dispatcher path for `apns_voip`-registered +// devices either silently drops or returns errors at send time and +// CallKit never fires on the receiver — which defeats the whole +// purpose of registering a separate VoIP device row. + +func TestVoIP_Name_ReturnsApnsVoipForRouting(t *testing.T) { + // Dispatcher routes by device.Provider == provider.Name(). If the + // VoIP Provider returns "apns" the dispatcher would conflate it + // with the alert provider (or the second Register call would + // overwrite the first in the providers map). MUST be "apns_voip". + p := newTestProviderKind(t, "com.example.app", KindVoIP, &fakePushClient{}) + if got := p.Name(); got != "apns_voip" { + t.Errorf("KindVoIP Name() = %q; want %q (dispatcher routes by this)", got, "apns_voip") + } + // Alert kind unchanged — back-compat. + alert := newTestProviderKind(t, "com.example.app", KindAlert, &fakePushClient{}) + if got := alert.Name(); got != "apns" { + t.Errorf("KindAlert Name() = %q; want %q (back-compat)", got, "apns") + } +} + +func TestVoIP_Send_TopicHasVoIPSuffix(t *testing.T) { + fake := &fakePushClient{ + resp: &apns2.Response{StatusCode: http.StatusOK, ApnsID: "voip-1"}, + } + p := newTestProviderKind(t, "com.example.app", KindVoIP, fake) + err := p.Send(context.Background(), push.PushMessage{ + DeviceToken: "DEADBEEFVOIPTOKEN", + Data: map[string]interface{}{ + "call_id": "abc-123", + "caller_id": "user-42", + }, + }) + if err != nil { + t.Fatalf("VoIP Send: %v", err) + } + if fake.lastSent == nil { + t.Fatal("Send didn't dispatch to client") + } + const wantTopic = "com.example.app.voip" + if fake.lastSent.Topic != wantTopic { + t.Errorf("topic = %q; want %q (Apple routes the .voip suffix to PushKit)", fake.lastSent.Topic, wantTopic) + } +} + +func TestVoIP_Send_PushTypeIsVOIP(t *testing.T) { + fake := &fakePushClient{ + resp: &apns2.Response{StatusCode: http.StatusOK, ApnsID: "voip-2"}, + } + p := newTestProviderKind(t, "com.example.app", KindVoIP, fake) + err := p.Send(context.Background(), push.PushMessage{ + DeviceToken: "VOIP-TOKEN", + Data: map[string]interface{}{"call_id": "x"}, + }) + if err != nil { + t.Fatalf("Send: %v", err) + } + if fake.lastSent.PushType != apns2.PushTypeVOIP { + t.Errorf("apns-push-type = %q; want %q (required since iOS 13)", + fake.lastSent.PushType, apns2.PushTypeVOIP) + } +} + +func TestVoIP_Send_EmptyContentAccepted(t *testing.T) { + // CallKit-only pushes carry no alert. The bugboard #348 visible- + // content guard MUST be bypassed on the VoIP path or every + // incoming-call signal would fail with ErrEmptyContent before + // reaching Apple. + fake := &fakePushClient{ + resp: &apns2.Response{StatusCode: http.StatusOK, ApnsID: "voip-3"}, + } + p := newTestProviderKind(t, "com.example.app", KindVoIP, fake) + err := p.Send(context.Background(), push.PushMessage{ + DeviceToken: "VOIP-TOKEN", + // No Title, Body, Badge, Sound, or content_available marker — + // this would be ErrEmptyContent on the alert path. + }) + if err != nil { + t.Fatalf("VoIP empty-content Send should succeed; got %v", err) + } + if fake.lastSent == nil { + t.Fatal("Send didn't dispatch to client") + } +} + +func TestVoIP_Send_ForcesHighPriority(t *testing.T) { + // Apple rejects VoIP pushes with `apns-priority: 5` (BadPriority). + // Even if the caller passes Priority="" or PriorityNormal, the + // VoIP path forces High so we never produce a request Apple will + // reject for that reason. + cases := []struct { + name string + callerPrio push.PushPriority + wantApnsPrio int + }{ + {"caller_unset", "", apns2.PriorityHigh}, + {"caller_normal", push.PriorityNormal, apns2.PriorityHigh}, + {"caller_high", push.PriorityHigh, apns2.PriorityHigh}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fake := &fakePushClient{ + resp: &apns2.Response{StatusCode: http.StatusOK}, + } + p := newTestProviderKind(t, "com.example.app", KindVoIP, fake) + _ = p.Send(context.Background(), push.PushMessage{ + DeviceToken: "T", + Priority: tc.callerPrio, + Data: map[string]interface{}{"call_id": "x"}, + }) + if fake.lastSent.Priority != tc.wantApnsPrio { + t.Errorf("apns-priority = %d; want %d (VoIP forces High)", + fake.lastSent.Priority, tc.wantApnsPrio) + } + }) + } +} + +func TestAlert_Send_TopicIsBundleIDWithoutSuffix(t *testing.T) { + // Regression guard: VoIP suffix logic must NOT bleed into the alert + // path. Pre-#408 the topic was always the bare bundle; this test + // pins that behavior so a future refactor can't break the alert + // route by accident. + fake := &fakePushClient{ + resp: &apns2.Response{StatusCode: http.StatusOK}, + } + p := newTestProviderKind(t, "com.example.app", KindAlert, fake) + _ = p.Send(context.Background(), push.PushMessage{ + DeviceToken: "T", + Title: "hello", + }) + if fake.lastSent.Topic != "com.example.app" { + t.Errorf("alert topic = %q; want %q (bare bundle)", + fake.lastSent.Topic, "com.example.app") + } + if fake.lastSent.PushType != apns2.PushTypeAlert { + t.Errorf("alert push-type = %q; want %q", fake.lastSent.PushType, apns2.PushTypeAlert) + } +} + +func TestAlert_Send_EmptyContentStillRejected(t *testing.T) { + // Bugboard #348 guard MUST remain intact on the alert path even + // after the VoIP bypass landed. If this regresses, alert-path + // silent-drop bugs come back. + p := newTestProviderKind(t, "com.example.app", KindAlert, &fakePushClient{}) + err := p.Send(context.Background(), push.PushMessage{ + DeviceToken: "T", + // No Title/Body/Badge/Sound/content_available — should reject + // on the alert path even though the VoIP path accepts it. + }) + if err == nil { + t.Fatal("alert path should still reject empty-content (bugboard #348); got nil") + } +} diff --git a/core/pkg/push/types.go b/core/pkg/push/types.go index 8491d9d..32f9311 100644 --- a/core/pkg/push/types.go +++ b/core/pkg/push/types.go @@ -28,15 +28,25 @@ const ( // DeviceToken is the provider-specific identifier (e.g. an ntfy topic, // an Expo push token, an APNs device token). The PushDispatcher fills // it in per-device before calling Send. +// +// TargetProvider is consumed by the DISPATCHER (not by providers) to +// filter the device list pre-send. Empty = fan out to all registered +// devices regardless of provider (back-compat default). Non-empty = +// dispatcher skips any device whose Provider field doesn't equal this +// value. Bugboard #408 — needed so a chat-alert message-push-handler +// can target "apns" only and avoid waking the user's "apns_voip" +// (PushKit/CallKit) device on every text. Providers themselves ignore +// this field. type PushMessage struct { - DeviceToken string - Title string - Body string - Data map[string]interface{} - Badge int - Sound string - Channel string // "messages", "calls", etc — provider may map to its own channel concept - Priority PushPriority + DeviceToken string + Title string + Body string + Data map[string]interface{} + Badge int + Sound string + Channel string // "messages", "calls", etc — provider may map to its own channel concept + Priority PushPriority + TargetProvider string // dispatcher-side filter; "" = fanout. See type doc. } // PushProvider is implemented by each backend (ntfy, expo, apns). diff --git a/core/pkg/serverless/hostfunctions/push.go b/core/pkg/serverless/hostfunctions/push.go index c73fe5a..24dd2e8 100644 --- a/core/pkg/serverless/hostfunctions/push.go +++ b/core/pkg/serverless/hostfunctions/push.go @@ -12,14 +12,23 @@ import ( // PushSendArgs is the JSON payload format the WASM caller marshals into // the `msgJSON` argument of PushSend. Mirrors push.PushMessage minus the // device-token (which is filled in per-device by the dispatcher). +// +// TargetProvider (bugboard #408) is the dispatcher-side device filter. +// Empty = fan out to every registered device for the user (back-compat +// default). Set to a provider name ("apns", "apns_voip", "ntfy", +// "expo") = dispatcher only attempts devices whose Provider field +// matches. Required by call-push-handler (set to "apns_voip") to avoid +// CallKit-ring on every chat message, and by message-push-handler (set +// to "apns") so VoIP-only pushes don't show as a silent alert. type PushSendArgs struct { - Title string `json:"title,omitempty"` - Body string `json:"body,omitempty"` - Channel string `json:"channel,omitempty"` - Priority string `json:"priority,omitempty"` // "high" | "normal" | "" - Badge int `json:"badge,omitempty"` - Sound string `json:"sound,omitempty"` - Data map[string]interface{} `json:"data,omitempty"` + Title string `json:"title,omitempty"` + Body string `json:"body,omitempty"` + Channel string `json:"channel,omitempty"` + Priority string `json:"priority,omitempty"` // "high" | "normal" | "" + Badge int `json:"badge,omitempty"` + Sound string `json:"sound,omitempty"` + Data map[string]interface{} `json:"data,omitempty"` + TargetProvider string `json:"target_provider,omitempty"` } // MaxPushSendArgsBytes caps the JSON arg size to a few KB. Push payloads @@ -92,13 +101,14 @@ func (h *HostFunctions) PushSend(ctx context.Context, userID string, msgJSON []b } msg := push.PushMessage{ - Title: args.Title, - Body: args.Body, - Channel: args.Channel, - Priority: priority, - Badge: args.Badge, - Sound: args.Sound, - Data: args.Data, + Title: args.Title, + Body: args.Body, + Channel: args.Channel, + Priority: priority, + Badge: args.Badge, + Sound: args.Sound, + Data: args.Data, + TargetProvider: args.TargetProvider, } // Route through Manager when present so per-namespace push config @@ -187,13 +197,14 @@ func (h *HostFunctions) PushSendV2(ctx context.Context, userID string, msgJSON [ } msg := push.PushMessage{ - Title: args.Title, - Body: args.Body, - Channel: args.Channel, - Priority: priority, - Badge: args.Badge, - Sound: args.Sound, - Data: args.Data, + Title: args.Title, + Body: args.Body, + Channel: args.Channel, + Priority: priority, + Badge: args.Badge, + Sound: args.Sound, + Data: args.Data, + TargetProvider: args.TargetProvider, } // Prefer the Manager (per-namespace config); fall back to the legacy