feat(sentry-apps): Validate org membership before sending webhook disable emails by Christinarlong · Pull Request #114799 · getsentry/sentry

Christinarlong · 2026-05-04T22:56:21Z

Summary

Adds an RPC to app_service to get the first valid email (is org member and verified, else an owner)

Context

In #114115 , @GabeVillalobos raised a good point where it's possible the creator of an app may not be apart of an organization (removed, deleted etc.) so we should validate org membership. Since webhooks are primarily sent from cell/region side, we need RPCs to do the validation which is all on the User/UserEmail/OrganizationMemberMapping models which are all in control. So to reduce the RPC calls we're consolidating to 1 specific RPC.

Changes

Adds get_notification_emails_for_sentry_app which either
- returns the given creator_label (the creator of the app's email) if they are active, verified and an org member (_validate_creator_email)
- If the creator email is not valid then we get the first email from an org owner (_get_org_owner_email)

…able emails Add RPC method get_notification_emails_for_sentry_app to app_service that validates the creator email against UserEmail (verified, active, org member) before sending circuit breaker notifications. Falls back to org owner email when the creator is no longer a valid recipient. Previously, webhook disable emails were sent to sentry_app.creator_label without verifying the creator was still an org member — a security risk if the creator had since left the organization. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

Stop mocking the entire NotificationService class. Instead, patch only notify_async and enable the notification-platform feature flag with rollout options, matching the pattern in notification platform tests. Also removes _get_notification_recipients mocks so integration tests exercise the real RPC recipient resolution path. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

The caller already has the sentry_app with owner_id, so pass organization_id directly instead of sentry_app_id + re-querying. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

Rename to _is_valid_creator_email and return bool instead of list[str]. The caller now wraps the email in a list only when validation passes. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

_get_org_owner_emails now returns the full list so the RPC layer doesn't impose a limit. The webhook caller truncates to 1 recipient to avoid spamming when there are many owners. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

The limit-to-1 decision belongs in the notification sender, not the recipient resolver. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

GabeVillalobos · 2026-05-07T18:06:36Z

+    if not recipient_emails:
+        logger.info(
+            "sentry_app.webhook.circuit_breaker.no_recipients",
+            extra={"slug": sentry_app.slug, "owner_id": sentry_app.owner_id},
+        )


I missed this in my earlier review, but we need to split this into its own PR. RPC service methods need to be deployed before usage is rolled out unless this is behind a FF

Revert webhooks.py and test changes — RPC methods must be deployed before callers are rolled out. This PR now only contains the RPC service definition; webhook usage will follow in a separate PR. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

_get_org_owner_emails was reading User.email directly, which is not guaranteed to be verified. Now joins through UserEmail with is_verified=True to match the same verification standard used by _is_valid_creator_email. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 5a4e45e. Configure here.}

cursor · 2026-05-07T20:26:21Z

+            user__is_active=True,
+            user__emails__is_verified=True,
+        ).values_list("user__emails__email", flat=True)
+    )


Query returns all verified emails per owner, causing duplicates

Medium Severity

_get_org_owner_emails joins through user__emails and uses values_list("user__emails__email", flat=True), which produces one row per verified UserEmail per org owner. If an owner has multiple verified email addresses, all of them are returned. This contradicts the PR description which says the function returns "the first email from an org owner," and could result in duplicate notifications being sent to the same person at different addresses.

^{Reviewed by Cursor Bugbot for commit 5a4e45e. Configure here.}

ill change the pr description 💀

…pp RPC Test the RPC implementation directly covering: valid creator, non-member fallback, unverified email fallback, username without @, no valid recipients, inactive creator, and unverified owner exclusion. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

…able emails (#114799)

github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 4, 2026

Christinarlong and others added 2 commits May 4, 2026 16:18

ref(sentry-apps): Pass organization_id directly to avoid extra DB lookup

8ad8c03

The caller already has the sentry_app with owner_id, so pass organization_id directly instead of sentry_app_id + re-querying. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

Christinarlong marked this pull request as ready for review May 4, 2026 23:45

Christinarlong requested review from a team as code owners May 4, 2026 23:45

Christinarlong requested review from a team and GabeVillalobos May 4, 2026 23:56

GabeVillalobos reviewed May 6, 2026

View reviewed changes

Comment thread src/sentry/sentry_apps/services/app/impl.py Outdated

Comment thread src/sentry/sentry_apps/services/app/impl.py Outdated

Comment thread src/sentry/sentry_apps/services/app/impl.py Outdated

Christinarlong and others added 3 commits May 6, 2026 12:41

ref(sentry-apps): Change _validate_creator_email to return bool

3c745d5

Rename to _is_valid_creator_email and return bool instead of list[str]. The caller now wraps the email in a list only when validation passes. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

ref(sentry-apps): Move recipient truncation to _notify_webhook_disabled

1d996ac

The limit-to-1 decision belongs in the notification sender, not the recipient resolver. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>

Christinarlong requested a review from GabeVillalobos May 6, 2026 21:52

GabeVillalobos reviewed May 7, 2026

View reviewed changes

sentry Bot reviewed May 7, 2026

View reviewed changes

Comment thread src/sentry/sentry_apps/services/app/impl.py

cursor Bot reviewed May 7, 2026

View reviewed changes

Christinarlong requested a review from GabeVillalobos May 7, 2026 21:01

GabeVillalobos approved these changes May 7, 2026

View reviewed changes

Christinarlong merged commit 543fb56 into master May 7, 2026
73 checks passed

Christinarlong deleted the Christinarlong/org-membership-rpc branch May 7, 2026 23:01

Christinarlong mentioned this pull request May 7, 2026

feat(sentry-apps): Use RPC recipient resolution in webhook disable notifications #115157

Merged

constantinius pushed a commit that referenced this pull request May 8, 2026

feat(sentry-apps): Validate org membership before sending webhook dis…

f684120

…able emails (#114799)

nikkikapadia pushed a commit that referenced this pull request May 12, 2026

feat(sentry-apps): Validate org membership before sending webhook dis…

c6e7393

…able emails (#114799)

sentry-release-bot Bot mentioned this pull request May 15, 2026

publish: getsentry/sentry@26.5.0 getsentry/publish#8191

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(sentry-apps): Validate org membership before sending webhook disable emails#114799

feat(sentry-apps): Validate org membership before sending webhook disable emails#114799
Christinarlong merged 9 commits into
masterfrom
Christinarlong/org-membership-rpc

Christinarlong commented May 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GabeVillalobos May 7, 2026

Uh oh!

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot May 7, 2026

Uh oh!

Christinarlong May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Christinarlong commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Context

Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GabeVillalobos May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot May 7, 2026

Choose a reason for hiding this comment

Query returns all verified emails per owner, causing duplicates

Uh oh!

Christinarlong May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Christinarlong commented May 4, 2026 •

edited

Loading