feat(api): support URL-backed phone gateways - #989
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add Phone.NotificationTransport and Phone.NotificationURL helpers with entity-level validation for FCM tokens and public HTTPS endpoints. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Clear TLS dial hooks and ServerName so policy validation and hostname verification cannot be bypassed. Restrict retries to standard 5xx statuses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Validate adapter URLs with the same cached endpoint policy used by the secure notification dialer. Ignore private-host allowlists outside local environments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Keep trusted HTTP middleware around the endpoint-policy transport so notification delivery retains telemetry and connection hardening. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Only service-created marked transports may preserve middleware. Opaque caller transports are replaced with policy-hardened transports so they cannot bypass SSRF dialing and TLS controls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Enforce transport provenance and per-attempt endpoint checks to close policy bypass and DNS timeout gaps. Redact callback tokens from logs and telemetry. Encode TTLs with protobuf JSON duration syntax. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Greptile SummaryThe PR extends the existing phone notification channel so an HTTPS callback URL can replace an FCM token while preserving the established wake-up lifecycle.
Confidence Score: 5/5The PR appears safe to merge, with no concrete correctness or security defect identified in the changed notification paths. The HTTPS adapter path preserves the existing wake-up lifecycle and consistently enforces endpoint validation, dial-time SSRF protection, TLS hostname verification, bounded retry behavior, and sensitive-data redaction. Important Files Changed
Sequence DiagramsequenceDiagram
participant Scheduler as Notification scheduler
participant Service as PhoneNotificationService
participant Dispatcher as NotificationDispatcher
participant Policy as Endpoint policy
participant FCM as Firebase
participant Adapter as HTTPS adapter
participant Gateway as Phone gateway
Scheduler->>Service: Send message or heartbeat wake-up
Service->>Dispatcher: GatewayNotification
alt Firebase token
Dispatcher->>FCM: FCM data message
FCM-->>Gateway: Wake-up
else HTTPS callback URL
Dispatcher->>Policy: Validate HTTPS and DNS addresses
Policy-->>Dispatcher: Approved public addresses
Dispatcher->>Policy: Revalidate at dial time
Dispatcher->>Adapter: POST FCM-compatible payload
Adapter-->>Gateway: Start gateway work
end
Dispatcher-->>Service: Accepted or failed
Service-->>Scheduler: Update notification lifecycle
Reviews (1): Last reviewed commit: "fix(api): harden adapter notifications" | Re-trigger Greptile |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 2 minor 21 high 1 critical 22 medium |
| CodeStyle | 11 minor |
🟢 Metrics 317 complexity · 35 duplication
Metric Results Complexity 317 Duplication 35
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Allow standard URL user information while retaining HTTPS and SSRF checks. Construct endpoint policies on demand and share one only within each HTTP sender graph so secured transport identity remains intact. Keep FCM-token examples opaque to preserve the existing API guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Use the existing OpenTelemetry HTTP transport and retry-go delivery pattern. Remove endpoint network policy and custom dialing while preserving callback URL redaction in telemetry. Reuse EventDispatcher directly and clarify the phone transport dispatcher name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Use the same OpenTelemetry HTTP client as webhooks and preserve default request, database, and notification logging without feature-specific redaction. Keep GORM query variables in traces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Reuse Firebase messages across transports and initialize one reusable retry policy per HTTP sender. Split phone transport dispatch into its own component and rely on the existing HTTP instrumentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Reuse the existing FCMClient contract across Firebase and HTTP so new phone transports only require DI map configuration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
There was a problem hiding this comment.
🟡 Changes recommended
Current validation and logging around URL-backed fcm_token values allow internal/private callback targets and can leak URL credentials in logs, which conflicts with the PR’s stated security/redaction expectations.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends the Go API’s phone notification delivery to support URL-backed phone gateways by treating fcm_token values that are valid https:// URLs as adapter callback endpoints, while preserving the existing Firebase (FCM) flow. It also adds an HTTPS adapter emulator and integration scenarios to exercise outgoing, incoming, and heartbeat wake-ups end-to-end.
Changes:
- Add
Phonehelpers to classify notification transport (fcmvshttp) based onfcm_token, and route sends via a transport-keyed client map. - Implement an FCM-compatible HTTP notification sender with bounded retries/timeouts and response-body discard limits.
- Expand integration infrastructure: adapter emulator service (HTTPS callback + HTTP control), Compose wiring, certificate generation, and new adapter E2E tests/docs.
File summaries
| File | Description |
|---|---|
| tests/README.md | Updates integration test architecture/docs to include the adapter emulator flow. |
| tests/helpers_test.go | Adds helper utilities for adapter gateway setup, callbacks, and internal event dispatch. |
| tests/generate-adapter-certificates.sh | Generates a short-lived CA + server certs for adapter emulator HTTPS. |
| tests/docker-compose.yml | Adds adapter-emulator service and mounts CA into the API container for TLS trust. |
| tests/adapter-emulator/notification_handler.go | Implements HTTPS callback endpoint that processes FCM-like envelopes. |
| tests/adapter-emulator/main.go | Boots HTTPS callback server + HTTP control server and handles shutdown. |
| tests/adapter-emulator/go.mod | Declares isolated emulator module (std-lib only). |
| tests/adapter-emulator/emulator.go | In-memory gateway registry and notification record tracking. |
| tests/adapter-emulator/emulator_test.go | Unit tests for emulator behavior (records, callbacks, control endpoints). |
| tests/adapter-emulator/Dockerfile | Builds and packages the adapter emulator as an unprivileged container. |
| tests/adapter-emulator/control_handler.go | HTTP control API for tests (register gateway, trigger incoming, list records, health). |
| tests/adapter-emulator/api_client.go | Emulator client for calling existing API endpoints using phone API keys. |
| tests/adapter_integration_test.go | New host-side integration tests for adapter outgoing/incoming/heartbeat scenarios. |
| docs/superpowers/specs/2026-09-02-url-backed-phone-notification-adapter-design.md | Design spec describing the URL-backed transport approach and constraints. |
| docs/superpowers/plans/2026-09-02-url-backed-phone-notification-adapter.md | Implementation plan and expected touched files/tests. |
| api/pkg/validators/phone_handler_validator.go | Adds notification-token validation hook (currently syntax/transport only). |
| api/pkg/validators/phone_handler_validator_test.go | Adds validator tests for URL-backed tokens (including private/loopback acceptance). |
| api/pkg/services/phone_notification_service.go | Routes notifications via transport-keyed clients and adds HTTP-vs-FCM guidance. |
| api/pkg/services/phone_notification_service_test.go | Adds service tests for transport selection and HTTP/FCM failure guidance. |
| api/pkg/services/http_notification_sender.go | Adds HTTP sender implementing FCMClient with retries, timeouts, and bounded discards. |
| api/pkg/services/http_notification_sender_test.go | Adds payload/retry/timeout/body-boundary tests for HTTP sender. |
| api/pkg/services/fcm_client.go | Updates interface comment to reflect multi-transport usage. |
| api/pkg/requests/phone_update_request.go | Documents that fcm_token may be a Firebase token or HTTPS adapter URL. |
| api/pkg/requests/phone_fcm_token_request.go | Documents that fcm_token may be a Firebase token or HTTPS adapter URL. |
| api/pkg/middlewares/http_request_logger_middleware.go | Minor request-body logging adjustment (avoids string conversion). |
| api/pkg/handlers/phone_handler.go | Updates Swagger descriptions and alters validation/error logging for phone endpoints. |
| api/pkg/entities/phone.go | Adds notification transport classification + URL parsing helpers. |
| api/pkg/entities/phone_test.go | Adds unit tests for transport classification and URL parsing. |
| api/pkg/entities/phone_notification.go | Updates comment to be transport-neutral (gateway wake-up notification). |
| api/pkg/di/container.go | Wires transport-keyed notification clients + adds OTel HTTP client for adapters. |
| api/pkg/di/container_test.go | Adds DI tests for OTel round-tripper wiring and client-map construction. |
| api/docs/swagger.yaml | Regenerated Swagger docs reflecting dual-purpose fcm_token + handler descriptions. |
| api/docs/swagger.json | Regenerated Swagger docs reflecting dual-purpose fcm_token + handler descriptions. |
| api/docs/docs.go | Regenerated embedded Swagger template reflecting dual-purpose fcm_token. |
| .gitignore | Ignores generated adapter certs and adds .worktrees/. |
| .github/workflows/api.yml | Generates adapter certificates in CI before starting the Compose stack. |
Review details
Files not reviewed (1)
- api/docs/docs.go: Generated file
Suppressed comments (3)
api/pkg/handlers/phone_handler.go:136
- This error log includes the raw request body, which may now contain adapter callback URLs with embedded credentials in
fcm_token. Prefer logging non-sensitive identifiers (e.g., phone number) instead of the entire JSON body.
ctxLogger.Error(stacktrace.Propagatef(
err,
"cannot update phone with request [%s]",
c.Body(),
))
api/pkg/handlers/phone_handler.go:212
- This log includes the raw request body. Since
fcm_tokencan now be an HTTPS adapter URL with userinfo, loggingc.Body()can leak credentials. Log only the validation errors (or redactfcm_token) for this endpoint.
ctxLogger.Warn(stacktrace.NewErrorf(
"validation errors [%s], while updating phone token request [%s]",
spew.Sdump(errors),
c.Body(),
))
api/pkg/handlers/phone_handler.go:222
- This error log includes the raw request body, which may contain adapter callback URLs with embedded credentials in
fcm_token. Prefer logging non-sensitive identifiers (e.g., phone number) instead of the entire JSON body.
ctxLogger.Error(stacktrace.Propagatef(
err,
"cannot update phone token with request [%s]",
c.Body(),
))
- Files reviewed: 34/36 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if errors := h.validator.ValidateUpsert(ctx, h.userIDFomContext(c), request.Sanitize()); len(errors) != 0 { | ||
| ctxLogger.Warn(stacktrace.NewErrorf("validation errors [%s], while updating phones [%+#v]", spew.Sdump(errors), request)) | ||
| ctxLogger.Warn(stacktrace.NewErrorf( | ||
| "validation errors [%s], while updating phone request [%s]", | ||
| spew.Sdump(errors), | ||
| c.Body(), | ||
| )) |
| func (validator *PhoneHandlerValidator) validateNotificationToken( | ||
| token string, | ||
| result url.Values, | ||
| ) { | ||
| token = strings.TrimSpace(token) | ||
| if token == "" { | ||
| return | ||
| } | ||
|
|
||
| phone := &entities.Phone{FcmToken: &token} | ||
| _, err := phone.NotificationTransport() | ||
| if err != nil { | ||
| result.Add("fcm_token", err.Error()) | ||
| } |
| if phone.FcmToken == nil { | ||
| msg := fmt.Sprintf("phone with id [%s] has no FCM token", phone.ID) | ||
| return service.handleNotificationFailed(ctx, errors.New(msg), params) | ||
| } |
Undo request-log scrubbing so phone handler failures retain the original URL and complete request parameters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2884b08e-2828-4b50-a9e6-702dce51ec0d
Summary
fcm_tokenfield as phone gateway callback endpointsValidation
cd api && go test ./... -count=1cd tests/adapter-emulator && go test ./... -count=1Note
Docker Compose runtime E2E could not be executed locally because Docker is unavailable; CI is configured to run the complete stack.