Skip to content

feat(api): support URL-backed phone gateways - #989

Merged
AchoArnold merged 22 commits into
mainfrom
feature/url-backed-phone-notifications
Sep 4, 2026
Merged

feat(api): support URL-backed phone gateways#989
AchoArnold merged 22 commits into
mainfrom
feature/url-backed-phone-notifications

Conversation

@AchoArnold

Copy link
Copy Markdown
Member

Summary

  • treat HTTPS values in the existing fcm_token field as phone gateway callback endpoints
  • dispatch outgoing message and heartbeat wake-ups through Firebase or the HTTP adapter transport
  • enforce HTTPS, SSRF-safe DNS validation, retry limits, TLS verification, and sensitive-data redaction
  • add an HTTPS adapter emulator with outgoing, incoming, and heartbeat integration scenarios

Validation

  • cd api && go test ./... -count=1
  • cd tests/adapter-emulator && go test ./... -count=1
  • API and emulator builds, vet, TLS certificate, Compose, and workflow static checks

Note

Docker Compose runtime E2E could not be executed locally because Docker is unavailable; CI is configured to run the complete stack.

AchoArnold and others added 16 commits September 2, 2026 22:35
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-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR extends the existing phone notification channel so an HTTPS callback URL can replace an FCM token while preserving the established wake-up lifecycle.

  • Adds transport classification and dispatch for Firebase and HTTPS adapters.
  • Adds HTTPS endpoint validation, DNS-rebinding-resistant dialing, TLS verification, bounded retries, and telemetry redaction.
  • Routes outbound-message and heartbeat wake-ups through the selected transport.
  • Adds an adapter emulator, integration coverage, generated API documentation, and CI certificate setup.

Confidence Score: 5/5

The 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

Filename Overview
api/pkg/services/http_notification_sender.go Implements bounded HTTPS delivery and retries using a secured transport, stable notification IDs, TLS verification, redacted errors, and transport-level telemetry.
api/pkg/services/notification_endpoint_policy.go Validates callback schemes and resolved addresses and repeats validation immediately before directly dialing approved IPs.
api/pkg/services/phone_notification_service.go Replaces direct Firebase calls with transport-neutral dispatch while preserving message notification and heartbeat behavior.
api/pkg/entities/phone.go Classifies stored notification destinations as opaque Firebase tokens or validated HTTPS callback URLs.
api/pkg/validators/phone_handler_validator.go Applies callback endpoint policy checks consistently to both phone upsert and token-refresh requests.
api/pkg/di/container.go Wires the shared endpoint policy, hardened HTTP client, notification dispatcher, and query-value redaction configuration.
tests/adapter_integration_test.go Adds end-to-end scenarios covering adapter registration, outgoing and incoming messaging, heartbeat wake-ups, and failure handling.

Sequence Diagram

sequenceDiagram
    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
Loading

Reviews (1): Last reviewed commit: "fix(api): harden adapter notifications" | Re-trigger Greptile

@codacy-production

codacy-production Bot commented Sep 3, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 1 critical · 21 high · 22 medium · 13 minor

Alerts:
⚠ 57 issues (≤ 0 issues of at least minor severity)

Results:
57 new issues

Category Results
Security 2 minor
21 high
1 critical
22 medium
CodeStyle 11 minor

View in Codacy

🟢 Metrics 317 complexity · 35 duplication

Metric Results
Complexity 317
Duplication 35

View in Codacy

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.

AchoArnold and others added 5 commits September 3, 2026 08:33
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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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 Phone helpers to classify notification transport (fcm vs http) based on fcm_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_token can now be an HTTPS adapter URL with userinfo, logging c.Body() can leak credentials. Log only the validation errors (or redact fcm_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.

Comment thread api/pkg/handlers/phone_handler.go Outdated
Comment on lines +121 to +126
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(),
))
Comment on lines +150 to +163
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())
}
Comment on lines 142 to 145
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
@AchoArnold
AchoArnold merged commit d5774ca into main Sep 4, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants