Make WordPress Core

Opened 6 weeks ago

Last modified 6 weeks ago

#65760 new defect (bug)

wp_delete_site() does not enforce the main-site protection that wpmu_delete_blog() does

Reported by: courane01 Owned by:
Priority: low Milestone: Future Release
Component: Networks and Sites Version: 5.1
Severity: normal Keywords: has-patch
Cc: Focuses: docs, multisite

Description

wp_delete_site() validates only that the site ID is non-empty and that the site exists. It then fires the wp_validate_site_deletion filter and proceeds. Core registers no callback on that filter, so a direct call to wp_delete_site( 1 ) uninitializes and removes a network's main site.

The main-site protection lives only in wpmu_delete_blog() in wp-admin/includes/ms.php, which blocks deletion for site ID 1, the main site, and the network domain/path root. Network Admin routes through that wrapper, so the admin UI is safe, and no default core caller bypasses it.

Steps to reproduce

On a disposable multisite network:

site1_exists = yes   main = yes   total_sites = 10
has_filter( 'wp_validate_site_deletion' ) = false

wp_delete_site( 1 )  ->  returns WP_Site, not a WP_Error

network root  ->  HTTP 500

The main site's tables are dropped and its row is removed from wp_blogs, so the network no longer resolves.

Why this is worth considering

This is not remotely reachable and is not a security issue. PHP has to already be executing inside WordPress, and on multisite that generally means Super Admin territory. It is an API footgun rather than a vulnerability.

The reason it seems worth raising is the direction of travel: wp_delete_site() is the modern API introduced in #41333, and it is the function plugin authors, migration scripts, and custom CLI tooling are pointed toward, while the safety check sits in the older wrapper. Someone reasonably calling the newer function directly gets no protection against destroying the primary site, and the failure is unrecoverable without a backup.

Possible directions

Any of these would address it:

  • Enforce the invariant inside wp_delete_site() itself, returning a WP_Error for the main site.
  • Register a default callback on wp_validate_site_deletion that adds the error.
  • Document the constraint prominently in the wp_delete_site() docblock, making clear that the guard is the caller's responsibility.

The first two would be behavior changes for anyone currently relying on the unguarded call, so documentation alone may be the safer option.

Related: #41333 introduced wp_initialize_site() and wp_uninitialize_site(). #48442 covers orphaned user options on site deletion.

Tested on 7.1-beta4, PHP 7.4, subdirectory multisite. The test network was rebuilt afterwards.

Change History (3)

#1 @jeremyfelt
6 weeks ago

  • Focuses docs added
  • Keywords needs-patch added
  • Milestone Awaiting Review5.1

Thanks for the ticket @courane01!

Document the constraint prominently in the wp_delete_site() docblock, making clear that the guard is the caller's responsibility.

I think this is the option that makes the most sense here.

wp_delete_site() was specifically introduced as part of a set of CRUD functions to avoid some of the extra cruft in things like wpmu_delete_blog().

From what I can recall, the thinking was that: an admin screen should call something like wpmu_delete_blog() first so that appropriate checks are involved before data is removed. But a lower-level function should be available to actually do the deleting.

From that initial commit, the one thing wp_delete_site() replaced was a direct $wpdb->delete() call. It then fires wp_validate_site_deletion to give other extenders the ability to prevent deletion.

If anyone wants more context, this may be the most productive Slack conversation at the time, though I think there are a couple back there. :)

All that said - yes to more documentation! I think we could probably clarify something like "this should be used specifically to delete sites from the database and does not protect against ..... something".

#2 @jeremyfelt
6 weeks ago

  • Milestone 5.1Future Release
  • Version trunk5.1

This ticket was mentioned in PR #12762 on WordPress/wordpress-develop by @irozum.


6 weeks ago
#3

  • Keywords has-patch added; needs-patch removed

wp_delete_site() validates only that the $site_id is non-empty and that the site exists, then fires the wp_validate_site_deletion action and proceeds — core registers no callback on that action, so a direct call to wp_delete_site( 1 ) uninitializes and deletes a network's main site with no error returned. The equivalent protection (blocking deletion of site ID 1, the main site, or the network's own domain/path root) lives only in wpmu_delete_blog() in wp-admin/includes/ms.php, which the Network Admin UI routes through, so the admin screens are safe. wp_delete_site() is the newer CRUD-style API (introduced in #41333) that plugin authors, migration scripts, and custom WP-CLI tooling are pointed toward, so someone calling it directly gets no protection against destroying the primary site, and the result is unrecoverable without a backup.

As discussed on the ticket, a behavior change to wp_delete_site() itself would affect any code currently relying on the unguarded call, so this PR takes the documentation-only route the ticket comments converged on: it adds a paragraph to the function's docblock in src/wp-includes/ms-site.php stating plainly that wp_delete_site() does not guard against deleting the main site, and pointing to wpmu_delete_blog() as where that guard lives for callers who need it. No behavior changes.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Sonnet 5
Used for: Implementation and PR description. Reviewed by irozum.

Note: See TracTickets for help on using tickets.