From 6957ab1e0f7e82596d4fcf09df270ee4309dc572 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 1 Sep 2026 12:55:18 -0500 Subject: [PATCH 1/4] Theme Directory: map the moderation capabilities against the theme `suspend_theme` and `reinstate_theme` are called with the theme's post ID, but the mapping resolved them straight to a primitive without ever reading that context, so the answer did not depend on which theme was passed. Resolve the post, require it to be a repopackage, and require the post type's `edit_others_posts` alongside the primitive, since moderating a theme acts on a record somebody else owns. Also drops `author` from the two role lists, matching the comment above them, and sets an explicit `post_status` on the commercial shops query so an unpublished entry cannot reach the shared cache the public API reads. Co-Authored-By: Claude Opus 5 (1M context) --- .../plugins/theme-directory/admin-edit.php | 22 ++- .../theme-directory/class-themes-api.php | 1 + .../Theme_Moderation_Capabilities_Test.php | 184 ++++++++++++++++++ 3 files changed, 198 insertions(+), 9 deletions(-) create mode 100644 wordpress.org/public_html/wp-content/plugins/theme-directory/tests/Theme_Moderation_Capabilities_Test.php diff --git a/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php b/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php index 2f4b1a2076..04130033f3 100644 --- a/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php +++ b/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php @@ -9,7 +9,7 @@ function wporg_themes_add_caps() { // Give Editors and higher the ability to suspend and reinstate a theme. - foreach ( array( 'administrator', 'author', 'editor' ) as $role ) { + foreach ( array( 'administrator', 'editor' ) as $role ) { $wp_roles = get_role( $role ); $wp_roles->add_cap( 'suspend_themes' ); @@ -22,7 +22,7 @@ function wporg_themes_add_caps() { * Removes custom capabilities on plugin deactivation. */ function wporg_themes_remove_caps() { - foreach ( array( 'administrator', 'author', 'editor' ) as $role ) { + foreach ( array( 'administrator', 'editor' ) as $role ) { $wp_roles = get_role( $role ); $wp_roles->remove_cap( 'suspend_themes' ); @@ -76,14 +76,18 @@ function wporg_themes_map_meta_cap( $caps, $cap, $user_id, $context ) { break; case 'suspend_theme': - $caps[] = 'suspend_themes'; - unset( $caps[ array_search( $cap, $caps ) ] ); - break; - case 'reinstate_theme': - $caps[] = 'reinstate_themes'; - unset( $caps[ array_search( $cap, $caps ) ] ); - break; + // Refuse unless the call carries an actual theme. + $post = $context ? get_post( $context[0] ) : false; + if ( ! $post || 'repopackage' !== $post->post_type ) { + return [ 'do_not_allow' ]; + } + + // Moderating a theme acts on a record somebody else owns. + return [ + 'suspend_theme' === $cap ? 'suspend_themes' : 'reinstate_themes', + get_post_type_object( $post->post_type )->cap->edit_others_posts, + ]; case 'theme_configure_categorization_options': // Protect against a cap call without a theme context. diff --git a/wordpress.org/public_html/wp-content/plugins/theme-directory/class-themes-api.php b/wordpress.org/public_html/wp-content/plugins/theme-directory/class-themes-api.php index a9f7b80d2a..15482b98cd 100644 --- a/wordpress.org/public_html/wp-content/plugins/theme-directory/class-themes-api.php +++ b/wordpress.org/public_html/wp-content/plugins/theme-directory/class-themes-api.php @@ -674,6 +674,7 @@ function get_commercial_shops() { $theme_shops = new WP_Query( array( 'post_type' => 'theme_shop', + 'post_status' => 'publish', 'posts_per_page' => -1, // NOTE: This rand() disables WP_Query caching. 'orderby' => 'rand(' . gmdate('YmdH') . ')', diff --git a/wordpress.org/public_html/wp-content/plugins/theme-directory/tests/Theme_Moderation_Capabilities_Test.php b/wordpress.org/public_html/wp-content/plugins/theme-directory/tests/Theme_Moderation_Capabilities_Test.php new file mode 100644 index 0000000000..f65225a241 --- /dev/null +++ b/wordpress.org/public_html/wp-content/plugins/theme-directory/tests/Theme_Moderation_Capabilities_Test.php @@ -0,0 +1,184 @@ +post_ids as $post_id ) { + wp_delete_post( $post_id, true ); + } + add_filter( 'before_delete_post', 'wporg_theme_no_delete_repopackage' ); + + if ( ! function_exists( 'wp_delete_user' ) ) { + require_once ABSPATH . 'wp-admin/includes/user.php'; + } + foreach ( $this->user_ids as $user_id ) { + wp_delete_user( $user_id ); + } + + $this->post_ids = array(); + $this->user_ids = array(); + + parent::tearDown(); + } + + /** + * Creates a user holding a single role. + * + * @param string $role The role to grant. + * @return int The new user's ID. + */ + protected function create_user( string $role ): int { + $user_id = wp_insert_user( + array( + 'user_login' => $role . '-' . uniqid(), + 'user_pass' => 'password', + 'user_email' => uniqid() . '@example.org', + 'role' => $role, + ) + ); + + $this->user_ids[] = $user_id; + + return $user_id; + } + + /** + * Creates a repopackage. + * + * @param int $author_id The owning user's ID. + * @param string $post_status Optional. The package's status. Default 'publish'. + * @return int The new package's post ID. + */ + protected function create_repopackage( int $author_id, string $post_status = 'publish' ): int { + $post_id = wp_insert_post( + array( + 'post_type' => 'repopackage', + 'post_status' => $post_status, + 'post_title' => 'Test Theme', + 'post_name' => 'test-theme-' . uniqid(), + 'post_author' => $author_id, + ) + ); + + $this->post_ids[] = $post_id; + + return $post_id; + } + + /** + * An Author cannot moderate a theme somebody else owns. + */ + public function test_author_cannot_moderate_someone_elses_theme(): void { + $author_id = $this->create_user( 'author' ); + $theme_id = $this->create_repopackage( $this->create_user( 'author' ) ); + + $this->assertFalse( user_can( $author_id, 'suspend_theme', $theme_id ) ); + $this->assertFalse( user_can( $author_id, 'reinstate_theme', $theme_id ) ); + } + + /** + * Nor a theme the Author owns: moderation state is not self-service. + */ + public function test_author_cannot_moderate_their_own_theme(): void { + $author_id = $this->create_user( 'author' ); + $theme_id = $this->create_repopackage( $author_id ); + + $this->assertFalse( user_can( $author_id, 'suspend_theme', $theme_id ) ); + $this->assertFalse( user_can( $author_id, 'reinstate_theme', $theme_id ) ); + } + + /** + * Reviewers keep the access the moderation handlers are written for. + */ + public function test_editor_can_moderate_a_theme(): void { + $editor_id = $this->create_user( 'editor' ); + $theme_id = $this->create_repopackage( $this->create_user( 'author' ) ); + + $this->assertTrue( user_can( $editor_id, 'suspend_theme', $theme_id ) ); + $this->assertTrue( user_can( $editor_id, 'reinstate_theme', $theme_id ) ); + } + + /** + * A capability call carrying no theme is refused rather than answered. + */ + public function test_moderation_without_a_theme_context_is_denied(): void { + $editor_id = $this->create_user( 'editor' ); + + $this->assertFalse( user_can( $editor_id, 'suspend_theme' ) ); + $this->assertFalse( user_can( $editor_id, 'reinstate_theme' ) ); + } + + /** + * The context has to be a theme, not any post that happens to exist. + */ + public function test_moderation_of_a_non_theme_is_denied(): void { + $editor_id = $this->create_user( 'editor' ); + + $post_id = wp_insert_post( array( 'post_title' => 'Not a theme' ) ); + $this->post_ids[] = $post_id; + + $this->assertFalse( user_can( $editor_id, 'suspend_theme', $post_id ) ); + $this->assertFalse( user_can( $editor_id, 'reinstate_theme', $post_id ) ); + } + + /** + * A theme owner keeps editing their own package while it is unpublished. + */ + public function test_owner_still_edits_their_unpublished_package(): void { + $author_id = $this->create_user( 'author' ); + $theme_id = $this->create_repopackage( $author_id, 'draft' ); + + $this->assertTrue( user_can( $author_id, 'edit_post', $theme_id ) ); + } +} From 54fef1374f655811cf3c7ed204a4528daa6a2253 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 1 Sep 2026 12:56:05 -0500 Subject: [PATCH 2/4] AGENTS.md: document the role trust model on WordPress.org sites Roles on the multisite blogs are assigned individually rather than granted on sign-up, and a lot of the estate leans on that. Worth writing down, since it is not derivable from the code and it is the premise behind a recurring class of misread privilege-escalation findings. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 03bb19652c..ac90645700 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,6 +36,14 @@ The repository implements a shared SSO mechanism under [common/includes/wporg-ss --- +## Security Trust Model + +Roles on WordPress.org multisite blogs are assigned individually rather than granted on sign-up. Much of the estate leans on this: capability checks are often coarse, because on most sites there is no untrusted principal below the people who run them. + +This is not uniform — some sites register their own roles and appoint community members into them at scale — so confirm the trust model of the site you are working on before relying on it. The [HackerOne program policy](https://hackerone.com/wordpress) is the authority on which gaps count as in-scope vulnerabilities. + +--- + ## Coding Standards & Linting We enforce the **WordPress Coding Standards (WPCS)** with local adjustments configured in [phpcs.xml.dist](phpcs.xml.dist). From 70dc9814f7918c63b0aa41461f0c447711725934 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 1 Sep 2026 13:02:17 -0500 Subject: [PATCH 3/4] Theme Directory: tighten the moderation capability guard `get_post( 0 )` falls back to the global `$post`, so a context of `array( 0 )` was answered against whatever post happened to be current rather than refused. Key the guard on an empty context instead, and resolve the post type object before dereferencing it, since `get_post()` does not require the type to be registered. Stops the test teardown revoking the moderation primitives too: granting them to Editors and Administrators is the site's normal state, and these tests run without a transaction, so revoking left the state behind in the database. Co-Authored-By: Claude Opus 5 (1M context) --- .../wp-content/plugins/theme-directory/admin-edit.php | 9 +++++---- .../tests/Theme_Moderation_Capabilities_Test.php | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php b/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php index 04130033f3..6d27b6a286 100644 --- a/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php +++ b/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php @@ -77,16 +77,17 @@ function wporg_themes_map_meta_cap( $caps, $cap, $user_id, $context ) { case 'suspend_theme': case 'reinstate_theme': - // Refuse unless the call carries an actual theme. - $post = $context ? get_post( $context[0] ) : false; - if ( ! $post || 'repopackage' !== $post->post_type ) { + // Refuse unless the call carries an actual, registered theme. + $post = empty( $context[0] ) ? false : get_post( $context[0] ); + $post_type = $post ? get_post_type_object( $post->post_type ) : null; + if ( ! $post_type || 'repopackage' !== $post->post_type ) { return [ 'do_not_allow' ]; } // Moderating a theme acts on a record somebody else owns. return [ 'suspend_theme' === $cap ? 'suspend_themes' : 'reinstate_themes', - get_post_type_object( $post->post_type )->cap->edit_others_posts, + $post_type->cap->edit_others_posts, ]; case 'theme_configure_categorization_options': diff --git a/wordpress.org/public_html/wp-content/plugins/theme-directory/tests/Theme_Moderation_Capabilities_Test.php b/wordpress.org/public_html/wp-content/plugins/theme-directory/tests/Theme_Moderation_Capabilities_Test.php index f65225a241..cbc636d1b3 100644 --- a/wordpress.org/public_html/wp-content/plugins/theme-directory/tests/Theme_Moderation_Capabilities_Test.php +++ b/wordpress.org/public_html/wp-content/plugins/theme-directory/tests/Theme_Moderation_Capabilities_Test.php @@ -44,11 +44,12 @@ protected function setUp(): void { } /** - * Removes the fixtures and the capabilities the test granted. + * Removes the fixture posts and users. + * + * The moderation primitives are left in place: granting them to Editors and + * Administrators is this site's normal state, not something the test dirtied. */ protected function tearDown(): void { - wporg_themes_remove_caps(); - /* * The plugin prevents repopackages from being deleted; detach that * specific guard while cleaning up the fixture posts. From 7c1f9f57ebe7cf8acf1bb1e4346462485b400c18 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 1 Sep 2026 13:03:26 -0500 Subject: [PATCH 4/4] Theme Directory: drop the redundant post type object guard Every caller of these caps runs at admin render time or on `admin_action_*`, both well after `init`, so if the plugin is active the post type is registered by the time the mapping is asked anything. Co-Authored-By: Claude Opus 5 (1M context) --- .../wp-content/plugins/theme-directory/admin-edit.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php b/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php index 6d27b6a286..ae03fc7018 100644 --- a/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php +++ b/wordpress.org/public_html/wp-content/plugins/theme-directory/admin-edit.php @@ -77,17 +77,16 @@ function wporg_themes_map_meta_cap( $caps, $cap, $user_id, $context ) { case 'suspend_theme': case 'reinstate_theme': - // Refuse unless the call carries an actual, registered theme. - $post = empty( $context[0] ) ? false : get_post( $context[0] ); - $post_type = $post ? get_post_type_object( $post->post_type ) : null; - if ( ! $post_type || 'repopackage' !== $post->post_type ) { + // Refuse unless the call carries an actual theme. + $post = empty( $context[0] ) ? false : get_post( $context[0] ); + if ( ! $post || 'repopackage' !== $post->post_type ) { return [ 'do_not_allow' ]; } // Moderating a theme acts on a record somebody else owns. return [ 'suspend_theme' === $cap ? 'suspend_themes' : 'reinstate_themes', - $post_type->cap->edit_others_posts, + get_post_type_object( $post->post_type )->cap->edit_others_posts, ]; case 'theme_configure_categorization_options':