From 47bc1df0fe167e4677a399f4295c5f3b6656671c Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 7 Jan 2025 13:45:58 -0600 Subject: [PATCH] Post meta: Update meta keys to be private (#1090) * Post meta: Update meta keys to be private WordPress convention states that meta keys prefixed with an underscore are considered private and hidden from the custom fields UI. Since our meta keys are for internal plugin use only, they should follow this convention. This change helps prevent accidental modification of ActivityPub meta data through WordPress's custom fields interface while maintaining the expected pattern for plugin developers. See: https://developer.wordpress.org/plugins/metadata/managing-post-metadata/#hidden-custom-fields * Add changelog * Fix phpcs * Don't make activitypub_content_visibility private * Add missing bits * Also remove content_warning Needs to be editable in editor * update phpcs --- CHANGELOG.md | 4 + includes/class-activitypub.php | 12 +-- includes/class-migration.php | 23 ++++++ includes/collection/class-actors.php | 2 +- includes/collection/class-followers.php | 40 +++++----- includes/model/class-follower.php | 12 +-- includes/transformer/class-post.php | 2 +- integration/class-jetpack.php | 6 +- readme.txt | 1 + tests/includes/class-test-migration.php | 73 +++++++++++++++++++ .../collection/class-test-followers.php | 10 +-- .../includes/transformer/class-test-post.php | 2 +- 12 files changed, 144 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f037d172..1c2188304 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Added a filter to make custom comment types manageable in WP.com Calypso +### Changed + +* Hide ActivityPub post meta keys from the custom Fields UI + ### Fixed * Undefined array key warnings in various places diff --git a/includes/class-activitypub.php b/includes/class-activitypub.php index bdd6641d0..bf94e303c 100644 --- a/includes/class-activitypub.php +++ b/includes/class-activitypub.php @@ -339,7 +339,7 @@ public static function get_avatar_url( $comment ) { public static function trash_post( $post_id ) { \add_post_meta( $post_id, - 'activitypub_canonical_url', + '_activitypub_canonical_url', \get_permalink( $post_id ), true ); @@ -351,7 +351,7 @@ public static function trash_post( $post_id ) { * @param string $post_id The Post ID. */ public static function untrash_post( $post_id ) { - \delete_post_meta( $post_id, 'activitypub_canonical_url' ); + \delete_post_meta( $post_id, '_activitypub_canonical_url' ); } /** @@ -482,7 +482,7 @@ private static function register_post_types() { \register_post_meta( Followers::POST_TYPE, - 'activitypub_inbox', + '_activitypub_inbox', array( 'type' => 'string', 'single' => true, @@ -492,7 +492,7 @@ private static function register_post_types() { \register_post_meta( Followers::POST_TYPE, - 'activitypub_errors', + '_activitypub_errors', array( 'type' => 'string', 'single' => false, @@ -508,7 +508,7 @@ private static function register_post_types() { \register_post_meta( Followers::POST_TYPE, - 'activitypub_user_id', + '_activitypub_user_id', array( 'type' => 'string', 'single' => false, @@ -520,7 +520,7 @@ private static function register_post_types() { \register_post_meta( Followers::POST_TYPE, - 'activitypub_actor_json', + '_activitypub_actor_json', array( 'type' => 'string', 'single' => true, diff --git a/includes/class-migration.php b/includes/class-migration.php index fe352b478..0ad7ccf83 100644 --- a/includes/class-migration.php +++ b/includes/class-migration.php @@ -161,6 +161,9 @@ public static function maybe_migrate() { if ( \version_compare( $version_from_db, '4.5.0', '<' ) ) { \wp_schedule_single_event( \time() + MINUTE_IN_SECONDS, 'activitypub_update_comment_counts' ); } + if ( \version_compare( $version_from_db, '4.6.0', '<' ) ) { + self::migrate_to_4_6_0(); + } /** * Fires when the system has to be migrated. @@ -387,6 +390,26 @@ public static function migrate_to_4_1_0() { ); } + /** + * Updates post meta keys to be prefixed with an underscore. + */ + public static function migrate_to_4_6_0() { + global $wpdb; + + $meta_keys = array( + 'activitypub_actor_json', + 'activitypub_canonical_url', + 'activitypub_errors', + 'activitypub_inbox', + 'activitypub_user_id', + ); + + foreach ( $meta_keys as $meta_key ) { + // phpcs:ignore WordPress.DB + $wpdb->update( $wpdb->postmeta, array( 'meta_key' => '_' . $meta_key ), array( 'meta_key' => $meta_key ) ); + } + } + /** * Update comment counts for posts in batches. * diff --git a/includes/collection/class-actors.php b/includes/collection/class-actors.php index 9e862e028..88ad59462 100644 --- a/includes/collection/class-actors.php +++ b/includes/collection/class-actors.php @@ -100,7 +100,7 @@ public static function get_by_username( $username ) { 'meta_query' => array( 'relation' => 'OR', array( - 'key' => 'activitypub_user_identifier', + 'key' => '_activitypub_user_identifier', 'value' => $username, 'compare' => 'LIKE', ), diff --git a/includes/collection/class-followers.php b/includes/collection/class-followers.php index dc4e31507..d6f8b0727 100644 --- a/includes/collection/class-followers.php +++ b/includes/collection/class-followers.php @@ -52,11 +52,11 @@ public static function add_follower( $user_id, $actor ) { return $id; } - $post_meta = get_post_meta( $id, 'activitypub_user_id', false ); + $post_meta = get_post_meta( $id, '_activitypub_user_id', false ); // phpcs:ignore WordPress.PHP.StrictInArray.MissingTrueStrict if ( is_array( $post_meta ) && ! in_array( $user_id, $post_meta ) ) { - add_post_meta( $id, 'activitypub_user_id', $user_id ); + add_post_meta( $id, '_activitypub_user_id', $user_id ); wp_cache_delete( sprintf( self::CACHE_KEY_INBOXES, $user_id ), 'activitypub' ); } @@ -89,7 +89,7 @@ public static function remove_follower( $user_id, $actor ) { */ do_action( 'activitypub_followers_pre_remove_follower', $follower, $user_id, $actor ); - return delete_post_meta( $follower->get__id(), 'activitypub_user_id', $user_id ); + return delete_post_meta( $follower->get__id(), '_activitypub_user_id', $user_id ); } /** @@ -106,7 +106,7 @@ public static function get_follower( $user_id, $actor ) { // phpcs:ignore WordPress.DB.DirectDatabaseQuery $post_id = $wpdb->get_var( $wpdb->prepare( - "SELECT DISTINCT p.ID FROM $wpdb->posts p INNER JOIN $wpdb->postmeta pm ON p.ID = pm.post_id WHERE p.post_type = %s AND pm.meta_key = 'activitypub_user_id' AND pm.meta_value = %d AND p.guid = %s", + "SELECT DISTINCT p.ID FROM $wpdb->posts p INNER JOIN $wpdb->postmeta pm ON p.ID = pm.post_id WHERE p.post_type = %s AND pm.meta_key = '_activitypub_user_id' AND pm.meta_value = %d AND p.guid = %s", array( esc_sql( self::POST_TYPE ), esc_sql( $user_id ), @@ -188,7 +188,7 @@ public static function get_followers_with_count( $user_id, $number = -1, $page = // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_query 'meta_query' => array( array( - 'key' => 'activitypub_user_id', + 'key' => '_activitypub_user_id', 'value' => $user_id, ), ), @@ -219,11 +219,11 @@ public static function get_all_followers() { 'meta_query' => array( 'relation' => 'AND', array( - 'key' => 'activitypub_inbox', + 'key' => '_activitypub_inbox', 'compare' => 'EXISTS', ), array( - 'key' => 'activitypub_actor_json', + 'key' => '_activitypub_actor_json', 'compare' => 'EXISTS', ), ), @@ -247,15 +247,15 @@ public static function count_followers( $user_id ) { 'meta_query' => array( 'relation' => 'AND', array( - 'key' => 'activitypub_user_id', + 'key' => '_activitypub_user_id', 'value' => $user_id, ), array( - 'key' => 'activitypub_inbox', + 'key' => '_activitypub_inbox', 'compare' => 'EXISTS', ), array( - 'key' => 'activitypub_actor_json', + 'key' => '_activitypub_actor_json', 'compare' => 'EXISTS', ), ), @@ -290,15 +290,15 @@ public static function get_inboxes( $user_id ) { 'meta_query' => array( 'relation' => 'AND', array( - 'key' => 'activitypub_inbox', + 'key' => '_activitypub_inbox', 'compare' => 'EXISTS', ), array( - 'key' => 'activitypub_user_id', + 'key' => '_activitypub_user_id', 'value' => $user_id, ), array( - 'key' => 'activitypub_inbox', + 'key' => '_activitypub_inbox', 'value' => '', 'compare' => '!=', ), @@ -318,7 +318,7 @@ public static function get_inboxes( $user_id ) { $wpdb->prepare( "SELECT DISTINCT meta_value FROM {$wpdb->postmeta} WHERE post_id IN (" . implode( ', ', array_fill( 0, count( $posts ), '%d' ) ) . ") - AND meta_key = 'activitypub_inbox' + AND meta_key = '_activitypub_inbox' AND meta_value IS NOT NULL", $posts ) @@ -378,24 +378,24 @@ public static function get_faulty_followers( $number = 20 ) { 'meta_query' => array( 'relation' => 'OR', array( - 'key' => 'activitypub_errors', + 'key' => '_activitypub_errors', 'compare' => 'EXISTS', ), array( - 'key' => 'activitypub_inbox', + 'key' => '_activitypub_inbox', 'compare' => 'NOT EXISTS', ), array( - 'key' => 'activitypub_actor_json', + 'key' => '_activitypub_actor_json', 'compare' => 'NOT EXISTS', ), array( - 'key' => 'activitypub_inbox', + 'key' => '_activitypub_inbox', 'value' => '', 'compare' => '=', ), array( - 'key' => 'activitypub_actor_json', + 'key' => '_activitypub_actor_json', 'value' => '', 'compare' => '=', ), @@ -437,7 +437,7 @@ public static function add_error( $post_id, $error ) { return add_post_meta( $post_id, - 'activitypub_errors', + '_activitypub_errors', $error_message ); } diff --git a/includes/model/class-follower.php b/includes/model/class-follower.php index 45ef6157b..bcf5d39dd 100644 --- a/includes/model/class-follower.php +++ b/includes/model/class-follower.php @@ -36,7 +36,7 @@ class Follower extends Actor { * @return mixed */ public function get_errors() { - return get_post_meta( $this->_id, 'activitypub_errors', false ); + return get_post_meta( $this->_id, '_activitypub_errors', false ); } /** @@ -72,7 +72,7 @@ public function get_url() { * Reset (delete) all errors. */ public function reset_errors() { - delete_post_meta( $this->_id, 'activitypub_errors' ); + delete_post_meta( $this->_id, '_activitypub_errors' ); } /** @@ -216,9 +216,9 @@ public function delete() { * Update the post meta. */ protected function get_post_meta_input() { - $meta_input = array(); - $meta_input['activitypub_inbox'] = $this->get_shared_inbox(); - $meta_input['activitypub_actor_json'] = $this->to_json(); + $meta_input = array(); + $meta_input['_activitypub_inbox'] = $this->get_shared_inbox(); + $meta_input['_activitypub_actor_json'] = $this->to_json(); return $meta_input; } @@ -334,7 +334,7 @@ public function get_shared_inbox() { * @return \Activitypub\Activity\Base_Object|WP_Error */ public static function init_from_cpt( $post ) { - $actor_json = get_post_meta( $post->ID, 'activitypub_actor_json', true ); + $actor_json = get_post_meta( $post->ID, '_activitypub_actor_json', true ); $object = self::init_from_json( $actor_json ); $object->set__id( $post->ID ); $object->set_id( $post->guid ); diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index d01c121c4..894012320 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -152,7 +152,7 @@ public function get_url() { switch ( \get_post_status( $post ) ) { case 'trash': - $permalink = \get_post_meta( $post->ID, 'activitypub_canonical_url', true ); + $permalink = \get_post_meta( $post->ID, '_activitypub_canonical_url', true ); break; case 'draft': // Get_sample_permalink is in wp-admin, not always loaded. diff --git a/integration/class-jetpack.php b/integration/class-jetpack.php index f4cf31e40..5586cf79e 100644 --- a/integration/class-jetpack.php +++ b/integration/class-jetpack.php @@ -35,9 +35,9 @@ public static function add_sync_meta( $allow_list ) { return $allow_list; } $activitypub_meta_keys = array( - 'activitypub_user_id', - 'activitypub_inbox', - 'activitypub_actor_json', + '_activitypub_user_id', + '_activitypub_inbox', + '_activitypub_actor_json', ); return \array_merge( $allow_list, $activitypub_meta_keys ); } diff --git a/readme.txt b/readme.txt index 4ffeb18be..f13510122 100644 --- a/readme.txt +++ b/readme.txt @@ -135,6 +135,7 @@ For reasons of data protection, it is not possible to see the followers of other = Unreleased = * Added: A filter to make custom comment types manageable in WP.com Calypso +* Changed: Hide ActivityPub post meta keys from the custom Fields UI * Fixed: Undefined array key warnings in various places = 4.6.0 = diff --git a/tests/includes/class-test-migration.php b/tests/includes/class-test-migration.php index 73384c240..00589fe2f 100644 --- a/tests/includes/class-test-migration.php +++ b/tests/includes/class-test-migration.php @@ -186,6 +186,79 @@ public function test_migrate_to_4_1_0() { $this->assertFalse( $content_type ); } + /** + * Test migrate to 4.6.0. + * + * @covers ::migrate_to_4_6_0 + */ + public function test_migrate_to_4_6_0() { + $post1 = \wp_insert_post( + array( + 'post_author' => 1, + 'post_content' => 'Test post 1', + ) + ); + + $post2 = \wp_insert_post( + array( + 'post_author' => 1, + 'post_content' => 'Test post 2', + ) + ); + + // Set up test meta data. + $meta_data = array( + 'activitypub_actor_json' => '{"type":"Person"}', + 'activitypub_canonical_url' => 'https://example.com/post-1', + 'activitypub_errors' => 'Test error', + 'activitypub_inbox' => 'https://example.com/inbox', + 'activitypub_user_id' => '123', + 'unrelated_meta' => 'should not change', + ); + + foreach ( $meta_data as $key => $value ) { + \update_post_meta( $post1, $key, $value ); + \update_post_meta( $post2, $key, $value . '-2' ); + } + + // Run migration. + Migration::migrate_to_4_6_0(); + + // Clean post cache to ensure fresh meta data. + \clean_post_cache( $post1 ); + \clean_post_cache( $post2 ); + + // Check post 1 meta. + $this->assertEmpty( \get_post_meta( $post1, 'activitypub_actor_json', true ), 'Old actor_json meta should be empty' ); + $this->assertEmpty( \get_post_meta( $post1, 'activitypub_canonical_url', true ), 'Old canonical_url meta should be empty' ); + $this->assertEmpty( \get_post_meta( $post1, 'activitypub_errors', true ), 'Old errors meta should be empty' ); + $this->assertEmpty( \get_post_meta( $post1, 'activitypub_inbox', true ), 'Old inbox meta should be empty' ); + $this->assertEmpty( \get_post_meta( $post1, 'activitypub_user_id', true ), 'Old user_id meta should be empty' ); + + $this->assertEquals( '{"type":"Person"}', \get_post_meta( $post1, '_activitypub_actor_json', true ), 'New actor_json meta should match' ); + $this->assertEquals( 'https://example.com/post-1', \get_post_meta( $post1, '_activitypub_canonical_url', true ), 'New canonical_url meta should match' ); + $this->assertEquals( 'Test error', \get_post_meta( $post1, '_activitypub_errors', true ), 'New errors meta should match' ); + $this->assertEquals( 'https://example.com/inbox', \get_post_meta( $post1, '_activitypub_inbox', true ), 'New inbox meta should match' ); + $this->assertEquals( '123', \get_post_meta( $post1, '_activitypub_user_id', true ), 'New user_id meta should match' ); + + // Check post 2 meta. + $this->assertEmpty( \get_post_meta( $post2, 'activitypub_actor_json', true ), 'Old actor_json meta should be empty' ); + $this->assertEmpty( \get_post_meta( $post2, 'activitypub_canonical_url', true ), 'Old canonical_url meta should be empty' ); + $this->assertEmpty( \get_post_meta( $post2, 'activitypub_errors', true ), 'Old errors meta should be empty' ); + $this->assertEmpty( \get_post_meta( $post2, 'activitypub_inbox', true ), 'Old inbox meta should be empty' ); + $this->assertEmpty( \get_post_meta( $post2, 'activitypub_user_id', true ), 'Old user_id meta should be empty' ); + + $this->assertEquals( '{"type":"Person"}-2', \get_post_meta( $post2, '_activitypub_actor_json', true ), 'New actor_json meta should match' ); + $this->assertEquals( 'https://example.com/post-1-2', \get_post_meta( $post2, '_activitypub_canonical_url', true ), 'New canonical_url meta should match' ); + $this->assertEquals( 'Test error-2', \get_post_meta( $post2, '_activitypub_errors', true ), 'New errors meta should match' ); + $this->assertEquals( 'https://example.com/inbox-2', \get_post_meta( $post2, '_activitypub_inbox', true ), 'New inbox meta should match' ); + $this->assertEquals( '123-2', \get_post_meta( $post2, '_activitypub_user_id', true ), 'New user_id meta should match' ); + + // Verify unrelated meta is unchanged. + $this->assertEquals( 'should not change', \get_post_meta( $post1, 'unrelated_meta', true ), 'Unrelated meta should not change' ); + $this->assertEquals( 'should not change-2', \get_post_meta( $post2, 'unrelated_meta', true ), 'Unrelated meta should not change' ); + } + /** * Tests that a new migration lock can be successfully acquired when no lock exists. * diff --git a/tests/includes/collection/class-test-followers.php b/tests/includes/collection/class-test-followers.php index e19dca592..cd54412ed 100644 --- a/tests/includes/collection/class-test-followers.php +++ b/tests/includes/collection/class-test-followers.php @@ -291,7 +291,7 @@ public function test_get_faulty_followers() { $follower = Followers::get_follower( 1, 'http://sally.example.org' ); for ( $i = 1; $i <= 15; $i++ ) { - add_post_meta( $follower->get__id(), 'activitypub_errors', 'error ' . $i ); + add_post_meta( $follower->get__id(), '_activitypub_errors', 'error ' . $i ); } $follower = Followers::get_follower( 1, 'http://sally.example.org' ); @@ -332,7 +332,7 @@ public function test_add_duplicate_follower() { $this->assertContains( $follower, $db_followers ); $follower = current( $db_followers ); - $meta = get_post_meta( $follower->get__id(), 'activitypub_user_id', false ); + $meta = get_post_meta( $follower->get__id(), '_activitypub_user_id', false ); $this->assertCount( 1, $meta ); } @@ -477,7 +477,7 @@ public function test_get_inboxes() { $id = $follower->upsert(); - add_post_meta( $id, 'activitypub_user_id', 1 ); + add_post_meta( $id, '_activitypub_user_id', 1 ); } $inboxes = Followers::get_inboxes( 1 ); @@ -503,7 +503,7 @@ public function test_get_inboxes() { $id = $follower->upsert(); - add_post_meta( $id, 'activitypub_user_id', 1 ); + add_post_meta( $id, '_activitypub_user_id', 1 ); } $inboxes2 = Followers::get_inboxes( 1 ); @@ -533,7 +533,7 @@ public function test_get_all_followers() { $id = $follower->upsert(); - add_post_meta( $id, 'activitypub_user_id', 1 ); + add_post_meta( $id, '_activitypub_user_id', 1 ); } $followers = Followers::get_all_followers(); diff --git a/tests/includes/transformer/class-test-post.php b/tests/includes/transformer/class-test-post.php index 63cb574cf..6feb0fc22 100644 --- a/tests/includes/transformer/class-test-post.php +++ b/tests/includes/transformer/class-test-post.php @@ -286,7 +286,7 @@ public function test_to_object() { $this->assertEquals( $permalink, $activitypub_post->get_id() ); - $cached = \get_post_meta( $post, 'activitypub_canonical_url', true ); + $cached = \get_post_meta( $post, '_activitypub_canonical_url', true ); $this->assertEquals( $cached, $activitypub_post->get_id() ); }