Federate comments only when the parent post is federated (#3374)

Author: pfefferle

.github/changelog/fix-comment-federation-parent-state

@@ -0,0 +1,4 @@
+Significance: minor
+Type: security
+
+Only share comment replies in the Fediverse when the post they belong to is itself federated, so replies on private or non-federated posts stay private.

includes/class-comment.php

@@ -299,6 +299,17 @@ public static function should_be_federated( $comment ) {
 			return false;
 		}
 
+		/*
+		 * Do not federate brand-new comments on a post that is not federated itself
+		 * (e.g. a private post, a post switched to local visibility, or a non-ActivityPub
+		 * post type). This prevents leaking replies on content the post type's read rules
+		 * would otherwise protect. Comments that were already sent are allowed through so
+		 * their Update and Delete activities can still federate (and tear down remote copies).
+		 */
+		if ( ! self::was_sent( $comment ) && ! is_post_federated( $comment->comment_post_ID ) ) {
+			return false;
+		}
+
 		// It is a comment to the post and can be federated.
 		if ( empty( $comment->comment_parent ) ) {
 			return true;

includes/functions-post.php

@@ -165,6 +165,42 @@ function is_post_publicly_queryable( $post ) {
 	return \apply_filters( 'activitypub_is_post_publicly_queryable', $queryable, $post );
 }
 
+/**
+ * Check whether a post is federated.
+ *
+ * A post is federated when it has been sent to the Fediverse (its federation
+ * state is "federated") AND it is still publicly queryable, i.e. its post type
+ * is enabled for ActivityPub, it has a public status, it is not password-
+ * protected, and its content visibility allows it (see `is_post_publicly_queryable()`).
+ *
+ * Re-checking the live queryability alongside the stored state guards against a
+ * stale "federated" status left behind when a post is moved to a private status,
+ * switched to local visibility, or its post type loses ActivityPub support.
+ *
+ * The federation-state check also keeps `is_post_publicly_queryable()`'s
+ * preview allowance inert here: a draft/pending post is never in the federated
+ * state, so the preview branch can never make this return true.
+ *
+ * @since unreleased
+ *
+ * @param mixed $post The post ID or object.
+ *
+ * @return boolean True if the post is federated, false otherwise.
+ */
+function is_post_federated( $post ) {
+	if ( empty( $post ) ) {
+		return false;
+	}
+
+	$post = \get_post( $post );
+
+	if ( ! $post ) {
+		return false;
+	}
+
+	return ACTIVITYPUB_OBJECT_STATE_FEDERATED === get_wp_object_state( $post ) && is_post_publicly_queryable( $post );
+}
+
 /**
  * Check if a post is an ActivityPub post.
  *

tests/phpunit/tests/includes/class-test-comment.php

@@ -117,6 +117,90 @@ public function test_check_ability_to_federate_threaded_comment( $parent_comment
 		$this->assertEquals( $expected['should_be_federated'], Comment::should_be_federated( $comment ) );
 	}
 
+	/**
+	 * Comments on a post that was never federated must not be federated.
+	 *
+	 * Regression: an approved reply by an ActivityPub-enabled user on a private or
+	 * non-ActivityPub post (one that was never federated itself) was sent to the public
+	 * outbox, leaking private content (e.g. a private Sensei LMS message reply).
+	 *
+	 * @covers ::should_be_federated
+	 */
+	public function test_does_not_federate_comment_on_non_federated_post() {
+		// A private post that was never federated (no activitypub_status meta).
+		$post_id = self::factory()->post->create(
+			array(
+				'post_status' => 'private',
+				'post_author' => 1,
+			)
+		);
+
+		$comment_id = self::factory()->comment->create(
+			array(
+				'comment_post_ID'  => $post_id,
+				'user_id'          => 1,
+				'comment_approved' => '1',
+				'comment_content'  => 'SECRET private reply',
+			)
+		);
+
+		$this->assertFalse(
+			Comment::should_be_federated( \get_comment( $comment_id ) ),
+			'A comment on a post that was never federated must not be federated.'
+		);
+	}
+
+	/**
+	 * Comments on a federated post are federated.
+	 *
+	 * @covers ::should_be_federated
+	 */
+	public function test_federates_comment_on_federated_post() {
+		$post_id = self::factory()->post->create( array( 'post_author' => 1 ) );
+		\update_post_meta( $post_id, 'activitypub_status', ACTIVITYPUB_OBJECT_STATE_FEDERATED );
+
+		$comment_id = self::factory()->comment->create(
+			array(
+				'comment_post_ID'  => $post_id,
+				'user_id'          => 1,
+				'comment_approved' => '1',
+				'comment_content'  => 'A public reply',
+			)
+		);
+
+		$this->assertTrue(
+			Comment::should_be_federated( \get_comment( $comment_id ) ),
+			'A comment on a federated post should be federated.'
+		);
+	}
+
+	/**
+	 * A comment that was already sent still federates even when its parent post
+	 * is no longer federated, so its Update and Delete activities can reach
+	 * remote copies.
+	 *
+	 * @covers ::should_be_federated
+	 */
+	public function test_federates_already_sent_comment_on_non_federated_post() {
+		// A post that is not federated.
+		$post_id = self::factory()->post->create( array( 'post_status' => 'private' ) );
+
+		// A comment that was already sent to the Fediverse.
+		$comment_id = self::factory()->comment->create(
+			array(
+				'comment_post_ID'  => $post_id,
+				'user_id'          => 1,
+				'comment_approved' => '1',
+				'comment_meta'     => array( 'activitypub_status' => ACTIVITYPUB_OBJECT_STATE_FEDERATED ),
+			)
+		);
+
+		$this->assertTrue(
+			Comment::should_be_federated( \get_comment( $comment_id ) ),
+			'An already-sent comment must still federate even when its parent post is not federated.'
+		);
+	}
+
 	/**
 	 * Test pre_comment_approved.
 	 *

tests/phpunit/tests/includes/class-test-functions-post.php

@@ -12,6 +12,68 @@
  */
 class Test_Functions_Post extends \WP_UnitTestCase {
 
+	/**
+	 * Test is_post_federated function.
+	 *
+	 * A post counts as federated only when its federation state is "federated"
+	 * AND it currently still meets the public-queryability criteria (post type
+	 * enabled, public status, allowed visibility, no password).
+	 *
+	 * @covers \Activitypub\is_post_federated
+	 */
+	public function test_is_post_federated() {
+		// Federated and still publicly queryable.
+		$federated_post_id = self::factory()->post->create();
+		\update_post_meta( $federated_post_id, 'activitypub_status', ACTIVITYPUB_OBJECT_STATE_FEDERATED );
+		$this->assertTrue( \Activitypub\is_post_federated( $federated_post_id ), 'A federated, publicly queryable post is federated.' );
+
+		// Never federated (no status), even though it is publicly queryable.
+		$never_post_id = self::factory()->post->create();
+		$this->assertFalse( \Activitypub\is_post_federated( $never_post_id ), 'A post that was never federated is not federated.' );
+
+		// Federated but soft-deleted (status no longer "federated").
+		$deleted_post_id = self::factory()->post->create();
+		\update_post_meta( $deleted_post_id, 'activitypub_status', ACTIVITYPUB_OBJECT_STATE_DELETED );
+		$this->assertFalse( \Activitypub\is_post_federated( $deleted_post_id ), 'A soft-deleted post is not federated.' );
+
+		// Federated status, but visibility later switched to local — must not count as federated.
+		$local_post_id = self::factory()->post->create();
+		\update_post_meta( $local_post_id, 'activitypub_status', ACTIVITYPUB_OBJECT_STATE_FEDERATED );
+		\update_post_meta( $local_post_id, 'activitypub_content_visibility', ACTIVITYPUB_CONTENT_VISIBILITY_LOCAL );
+		$this->assertFalse( \Activitypub\is_post_federated( $local_post_id ), 'A federated post switched to local visibility is no longer federated.' );
+
+		// Federated status, but post moved to a private status — must not count as federated.
+		$private_post_id = self::factory()->post->create( array( 'post_status' => 'private' ) );
+		\update_post_meta( $private_post_id, 'activitypub_status', ACTIVITYPUB_OBJECT_STATE_FEDERATED );
+		$this->assertFalse( \Activitypub\is_post_federated( $private_post_id ), 'A federated post moved to private status is no longer federated.' );
+
+		// Federated status, but password-protected — must not count as federated.
+		$password_post_id = self::factory()->post->create( array( 'post_password' => 'secret' ) );
+		\update_post_meta( $password_post_id, 'activitypub_status', ACTIVITYPUB_OBJECT_STATE_FEDERATED );
+		$this->assertFalse( \Activitypub\is_post_federated( $password_post_id ), 'A federated but password-protected post is not federated.' );
+
+		// Federated status, but post type no longer supports ActivityPub.
+		\register_post_type( 'unsupported', array() );
+		$unsupported_post_id = self::factory()->post->create( array( 'post_type' => 'unsupported' ) );
+		\update_post_meta( $unsupported_post_id, 'activitypub_status', ACTIVITYPUB_OBJECT_STATE_FEDERATED );
+		$this->assertFalse( \Activitypub\is_post_federated( $unsupported_post_id ), 'A federated post on an unsupported post type is not federated.' );
+		\unregister_post_type( 'unsupported' );
+
+		// An integration can veto via the public-queryability filter.
+		$filtered_post_id = self::factory()->post->create();
+		\update_post_meta( $filtered_post_id, 'activitypub_status', ACTIVITYPUB_OBJECT_STATE_FEDERATED );
+		$this->assertTrue( \Activitypub\is_post_federated( $filtered_post_id ), 'Federated post is federated before filtering.' );
+		\add_filter( 'activitypub_is_post_publicly_queryable', '__return_false' );
+		try {
+			$this->assertFalse( \Activitypub\is_post_federated( $filtered_post_id ), 'An integration filtering the post non-queryable makes it not federated.' );
+		} finally {
+			\remove_filter( 'activitypub_is_post_publicly_queryable', '__return_false' );
+		}
+
+		// Invalid input.
+		$this->assertFalse( \Activitypub\is_post_federated( 0 ), 'An empty post is not federated.' );
+	}
+
 	/**
 	 * Test is_post_disabled function.
 	 *

tests/phpunit/tests/includes/scheduler/class-test-comment.php

@@ -20,17 +20,23 @@ class Test_Comment extends \Activitypub\Tests\ActivityPub_Outbox_TestCase {
 	/**
 	 * Post ID for testing.
 	 *
+	 * Initialized to 0 so data providers (which run before set_up()) emit a
+	 * non-null value and the isset() substitution in test_no_activity_scheduled()
+	 * picks up the real post ID assigned in set_up().
+	 *
 	 * @var int
 	 */
-	protected static $comment_post_ID;
+	protected static $comment_post_ID = 0;
 
 	/**
-	 * Set up test resources.
+	 * Set up each test.
 	 */
-	public static function set_up_before_class() {
-		parent::set_up_before_class();
+	public function set_up() {
+		parent::set_up();
 
+		// Comments only federate on a post that was itself federated.
 		self::$comment_post_ID = self::factory()->post->create( array( 'post_author' => self::$user_id ) );
+		\update_post_meta( self::$comment_post_ID, 'activitypub_status', \ACTIVITYPUB_OBJECT_STATE_FEDERATED );
 	}
 
 	/**