diff --git a/projects/plugins/jetpack/changelog/fix-nl-715-post-access-level-non-string-guard b/projects/plugins/jetpack/changelog/fix-nl-715-post-access-level-non-string-guard new file mode 100644 index 000000000000..9beb57004022 --- /dev/null +++ b/projects/plugins/jetpack/changelog/fix-nl-715-post-access-level-non-string-guard @@ -0,0 +1,4 @@ +Significance: patch +Type: bugfix + +Newsletter: default the post access level to "everybody" when the stored meta is not a string, and sanitize non-string writes, so corrupt values can no longer cause a fatal error when rendering a post. diff --git a/projects/plugins/jetpack/extensions/blocks/subscriptions/subscriptions.php b/projects/plugins/jetpack/extensions/blocks/subscriptions/subscriptions.php index 4e329cc5a1c3..9fa785be2b56 100644 --- a/projects/plugins/jetpack/extensions/blocks/subscriptions/subscriptions.php +++ b/projects/plugins/jetpack/extensions/blocks/subscriptions/subscriptions.php @@ -79,10 +79,18 @@ function register_block() { 'post', META_NAME_FOR_POST_LEVEL_ACCESS_SETTINGS, array( - 'show_in_rest' => true, - 'single' => true, - 'type' => 'string', - 'auth_callback' => function () { + 'show_in_rest' => true, + 'single' => true, + 'type' => 'string', + // The REST schema already rejects non-strings, but non-REST writers + // (importers, XML-RPC, WP-CLI, direct update_post_meta, sync) only pass + // through sanitize_meta(). Coerce anything that isn't a string to '' + // ("everybody") so a corrupt value can't be persisted and later fatal the + // strict string-typed access checks. + 'sanitize_callback' => function ( $value ) { + return is_string( $value ) ? $value : ''; + }, + 'auth_callback' => function () { return wp_get_current_user()->has_cap( 'edit_posts' ); }, ) diff --git a/projects/plugins/jetpack/modules/memberships/class-jetpack-memberships.php b/projects/plugins/jetpack/modules/memberships/class-jetpack-memberships.php index 17436d438f65..b25a209c3d4b 100644 --- a/projects/plugins/jetpack/modules/memberships/class-jetpack-memberships.php +++ b/projects/plugins/jetpack/modules/memberships/class-jetpack-memberships.php @@ -700,7 +700,13 @@ public static function get_post_access_level( $post_id = null ) { } $post_access_level = get_post_meta( $post_id, self::$post_access_level_meta_name, true ); - if ( empty( $post_access_level ) ) { + // Defaults to "everybody" when unset, and also when the stored value is not a + // string. Corrupt rows (e.g. a serialized array like a:1:{i:0;s:0:"";}) can be + // persisted by non-REST write paths, and an array flows unchanged into the + // strict string-typed `earn_user_has_access` callback on WPCOM, fataling the + // render. Coercing here keeps this canonical accessor's documented string + // contract regardless of how the meta was written. + if ( empty( $post_access_level ) || ! is_string( $post_access_level ) ) { $post_access_level = Abstract_Token_Subscription_Service::POST_ACCESS_LEVEL_EVERYBODY; } diff --git a/projects/plugins/jetpack/tests/php/modules/subscriptions/Jetpack_Subscriptions_Access_Level_Meta_Test.php b/projects/plugins/jetpack/tests/php/modules/subscriptions/Jetpack_Subscriptions_Access_Level_Meta_Test.php new file mode 100644 index 000000000000..37b0f63250b3 --- /dev/null +++ b/projects/plugins/jetpack/tests/php/modules/subscriptions/Jetpack_Subscriptions_Access_Level_Meta_Test.php @@ -0,0 +1,193 @@ +insert( + $wpdb->postmeta, + array( + 'post_id' => $post_id, + 'meta_key' => META_NAME_FOR_POST_LEVEL_ACCESS_SETTINGS, + 'meta_value' => maybe_serialize( $value ), + ) + ); + wp_cache_delete( $post_id, 'post_meta' ); + Jetpack_Memberships::clear_post_access_level_cache(); + } + + /** + * The exact corrupt shape seen in production — an array containing one empty + * string — must be treated as "everybody" rather than flowing through as an + * array and fataling the strict string-typed `earn_user_has_access` callback. + * + * This is the regression test for NL-715: it fails before the guard (returns + * the array) and passes after (returns the EVERYBODY default). + * + * @return void + */ + public function test_array_access_level_meta_defaults_to_everybody() { + $post_id = self::factory()->post->create(); + $this->force_corrupt_access_meta( $post_id, array( '' ) ); + + // Sanity check: the corrupt array really is persisted and is not empty(). + $raw = get_post_meta( $post_id, META_NAME_FOR_POST_LEVEL_ACCESS_SETTINGS, true ); + $this->assertIsArray( $raw ); + $this->assertNotEmpty( $raw ); + + $this->assertSame( + Abstract_Token_Subscription_Service::POST_ACCESS_LEVEL_EVERYBODY, + Jetpack_Memberships::get_post_access_level( $post_id ) + ); + } + + /** + * A multi-value array (another corrupt shape a non-REST writer could persist) + * is likewise coerced to the everybody default rather than returned as an array. + * + * @return void + */ + public function test_multi_value_array_access_level_meta_defaults_to_everybody() { + $post_id = self::factory()->post->create(); + $this->force_corrupt_access_meta( + $post_id, + array( + Abstract_Token_Subscription_Service::POST_ACCESS_LEVEL_PAID_SUBSCRIBERS, + Abstract_Token_Subscription_Service::POST_ACCESS_LEVEL_SUBSCRIBERS, + ) + ); + + $this->assertSame( + Abstract_Token_Subscription_Service::POST_ACCESS_LEVEL_EVERYBODY, + Jetpack_Memberships::get_post_access_level( $post_id ) + ); + } + + /** + * A valid string access level is returned unchanged — the guard must not + * regress the normal path. + * + * @return void + */ + public function test_valid_string_access_level_meta_is_returned_unchanged() { + $post_id = self::factory()->post->create(); + update_post_meta( + $post_id, + META_NAME_FOR_POST_LEVEL_ACCESS_SETTINGS, + Abstract_Token_Subscription_Service::POST_ACCESS_LEVEL_SUBSCRIBERS + ); + Jetpack_Memberships::clear_post_access_level_cache(); + + $this->assertSame( + Abstract_Token_Subscription_Service::POST_ACCESS_LEVEL_SUBSCRIBERS, + Jetpack_Memberships::get_post_access_level( $post_id ) + ); + } + + /** + * An unset access level falls back to everybody, as before. + * + * @return void + */ + public function test_missing_access_level_meta_defaults_to_everybody() { + $post_id = self::factory()->post->create(); + + $this->assertSame( + Abstract_Token_Subscription_Service::POST_ACCESS_LEVEL_EVERYBODY, + Jetpack_Memberships::get_post_access_level( $post_id ) + ); + } + + /** + * Write-side defense (NL-715): once the meta is registered, an array written + * through the normal update_post_meta() path is coerced to '' by the + * sanitize_callback, so the corrupt value can never be persisted in the first + * place via writers that route through sanitize_meta(). + * + * @return void + */ + public function test_sanitize_callback_coerces_array_write_to_empty_string() { + // Activate the subscriptions module so register_block() actually registers + // the meta (it returns early when the module is inactive). + Jetpack_Options::update_option( 'active_modules', array( 'subscriptions' ) ); + register_subscription_block(); + + $post_id = self::factory()->post->create(); + update_post_meta( $post_id, META_NAME_FOR_POST_LEVEL_ACCESS_SETTINGS, array( '' ) ); + + $this->assertSame( + '', + get_post_meta( $post_id, META_NAME_FOR_POST_LEVEL_ACCESS_SETTINGS, true ) + ); + } + + /** + * The same write-side defense must leave a legitimate string write untouched. + * + * @return void + */ + public function test_sanitize_callback_preserves_valid_string_write() { + Jetpack_Options::update_option( 'active_modules', array( 'subscriptions' ) ); + register_subscription_block(); + + $post_id = self::factory()->post->create(); + update_post_meta( + $post_id, + META_NAME_FOR_POST_LEVEL_ACCESS_SETTINGS, + Abstract_Token_Subscription_Service::POST_ACCESS_LEVEL_SUBSCRIBERS + ); + + $this->assertSame( + Abstract_Token_Subscription_Service::POST_ACCESS_LEVEL_SUBSCRIBERS, + get_post_meta( $post_id, META_NAME_FOR_POST_LEVEL_ACCESS_SETTINGS, true ) + ); + } +}