Skip to content

Restrict notification subscriptions and harden the webhook sender - #987

Merged
GaryJones merged 1 commit into
developfrom
vipplug-20-notifications-hardening
Jun 8, 2026
Merged

Restrict notification subscriptions and harden the webhook sender#987
GaryJones merged 1 commit into
developfrom
vipplug-20-notifications-hardening

Conversation

@GaryJones

Copy link
Copy Markdown
Contributor

Summary

Two pieces of defence-in-depth in the notifications module.

The subscription picker only ever offers users who can publish_posts, but both save paths — the AJAX handler and the post-save form handler — stored whatever user IDs arrived in the request. An authorised editor could therefore add arbitrary users (a plain subscriber, say) as followers of a post. Both paths now intersect the submitted IDs with that same publish_posts candidate set before saving, so only users the picker would offer can be subscribed. The users auto-subscribed by the plugin (the current user and the post author) go through a separate path and are unaffected.

Separately, send_to_webhook() sent the payload with wp_remote_post() and, on failure, called print_ajax_response() which ends in wp_die(). That notification fires on post transitions and editorial-comment insertion, which are not always AJAX requests, so a failing webhook could abort an otherwise valid save. It now uses wp_safe_remote_post() — which validates the URL and rejects private, loopback and link-local hosts (SSRF) — and on failure fires an ef_notification_webhook_failed action for logging and returns rather than dying.

Fixes VIPPLUG-20.

Test plan

  • composer test:integration -- --filter NotificationsTest — a user the picker would not offer is not subscribed; a failed webhook returns without aborting.
  • All notifications tests still pass (61 tests across the Notifications* classes).
  • PHPCS and php -l clean.
  • Manual: as an editor, subscribe a teammate (works) and confirm the saved followers never include non-editorial users; point the webhook at an unreachable URL and confirm post saves still succeed.

@GaryJones
GaryJones requested a review from a team as a code owner June 8, 2026 15:04
@GaryJones GaryJones added this to the Next milestone Jun 8, 2026
@GaryJones GaryJones self-assigned this Jun 8, 2026
@GaryJones
GaryJones force-pushed the vipplug-20-notifications-hardening branch from 04924c3 to 626c367 Compare June 8, 2026 15:15
@GaryJones
GaryJones force-pushed the vipplug-20-notifications-hardening branch from 626c367 to 16daf4f Compare June 8, 2026 15:21
@GaryJones
GaryJones merged commit e4e80eb into develop Jun 8, 2026
10 checks passed
@GaryJones
GaryJones deleted the vipplug-20-notifications-hardening branch June 8, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant