Merge "SECURITY: Run edit filters on submit (undo/restore)"

repo/includes/Actions/SubmitEntityAction.php

@@ -3,7 +3,6 @@
 namespace Wikibase\Repo\Actions;
 
 use Article;
-use Content;
 use IContextSource;
 use LogicException;
 use MediaWiki\MediaWikiServices;
@@ -14,6 +13,7 @@
 use Title;
 use Wikibase\Lib\Summary;
 use Wikibase\Repo\Content\EntityContent;
+use Wikibase\Repo\EditEntity\EditFilterHookRunner;
 use Wikibase\Repo\SummaryFormatter;
 use Wikibase\Repo\WikibaseRepo;
 
@@ -29,10 +29,8 @@
  */
 class SubmitEntityAction extends EditEntityAction {
 
-	/**
-	 * @var SummaryFormatter
-	 */
-	private $summaryFormatter;
+	private EditFilterHookRunner $editFilterHookRunner;
+	private SummaryFormatter $summaryFormatter;
 
 	/**
 	 * @see EditEntityAction::__construct
@@ -43,6 +41,7 @@ class SubmitEntityAction extends EditEntityAction {
 	public function __construct( Article $article, IContextSource $context ) {
 		parent::__construct( $article, $context );
 
+		$this->editFilterHookRunner = WikibaseRepo::getEditFilterHookRunner();
 		$this->summaryFormatter = WikibaseRepo::getSummaryFormatter();
 	}
 
@@ -213,7 +212,7 @@ public function execute() {
 
 	/**
 	 * @param Title $title
-	 * @param Content $content
+	 * @param EntityContent $content
 	 * @param string $summary
 	 * @param int $undidRevId
 	 * @param int $originalRevId
@@ -222,7 +221,7 @@ public function execute() {
 	 * @return Status
 	 */
 	private function attemptSave(
-		Title $title, Content $content, $summary, $undidRevId, $originalRevId, $editToken
+		Title $title, EntityContent $content, $summary, $undidRevId, $originalRevId, $editToken
 	) {
 		$status = $this->getEditTokenStatus( $editToken );
 
@@ -236,6 +235,12 @@ private function attemptSave(
 			return $status;
 		}
 
+		$status = $this->editFilterHookRunner->run( $content, $this->getContext(), $summary );
+
+		if ( !$status->isOK() ) {
+			return $status;
+		}
+
 		// save edit
 		$page = MediaWikiServices::getInstance()->getWikiPageFactory()
 			->newFromTitle( $title );

repo/includes/EditEntity/EditFilterHookRunner.php

@@ -8,6 +8,7 @@
 use Status;
 use Wikibase\DataModel\Entity\EntityDocument;
 use Wikibase\DataModel\Entity\EntityRedirect;
+use Wikibase\Repo\Content\EntityContent;
 
 /**
  * Interface to run a hook before and edit is saved.
@@ -20,7 +21,7 @@ interface EditFilterHookRunner {
 	/**
 	 * Call EditFilterMergedContent hook, if registered.
 	 *
-	 * @param EntityDocument|EntityRedirect|null $new The entity or redirect we are trying to save
+	 * @param EntityDocument|EntityRedirect|EntityContent|null $new The entity or redirect (content) we are trying to save
 	 * @param IContextSource $context The request context for the edit
 	 * @param string $summary The edit summary
 	 *

repo/includes/EditEntity/MediaWikiEditFilterHookRunner.php

@@ -13,6 +13,7 @@
 use Wikibase\DataModel\Entity\EntityId;
 use Wikibase\DataModel\Entity\EntityRedirect;
 use Wikibase\Lib\Store\EntityNamespaceLookup;
+use Wikibase\Repo\Content\EntityContent;
 use Wikibase\Repo\Content\EntityContentFactory;
 use Wikibase\Repo\Store\EntityTitleStoreLookup;
 
@@ -59,7 +60,7 @@ public function __construct(
 	/**
 	 * Call EditFilterMergedContent hook, if registered.
 	 *
-	 * @param EntityDocument|EntityRedirect|null $new The entity or redirect we are trying to save
+	 * @param EntityDocument|EntityRedirect|EntityContent|null $new The entity or redirect (content) we are trying to save
 	 * @param IContextSource $context The request context for the edit
 	 * @param string $summary The edit summary
 	 *
@@ -95,13 +96,23 @@ public function run( $new, IContextSource $context, string $summary ) {
 			$entityId = $new->getEntityId();
 			$entityType = $entityId->getEntityType();
 
+			$context = $this->getContextForEditFilter(
+				$context,
+				$entityId,
+				$entityType
+			);
+		} elseif ( $new instanceof EntityContent ) {
+			$entityContent = $new;
+			$entityId = $entityContent->getEntityId();
+			$entityType = $entityId->getEntityType();
+
 			$context = $this->getContextForEditFilter(
 				$context,
 				$entityId,
 				$entityType
 			);
 		} else {
-			throw new InvalidArgumentException( '$new must be instance of EntityDocument or EntityRedirect' );
+			throw new InvalidArgumentException( '$new must be instance of EntityDocument, EntityRedirect or EntityContent' );
 		}
 
 		$slotRole = $this->namespaceLookup->getEntitySlotRole( $entityType );

repo/includes/EditEntity/StatsdTimeRecordingEditFilterHookRunner.php

@@ -7,6 +7,7 @@
 use Status;
 use Wikibase\DataModel\Entity\EntityDocument;
 use Wikibase\DataModel\Entity\EntityRedirect;
+use Wikibase\Repo\Content\EntityContent;
 
 /**
  * EditFilterHookRunning that collects stats for edits.
@@ -37,7 +38,7 @@ public function __construct(
 	}
 
 	/**
-	 * @param null|EntityDocument|EntityRedirect $new
+	 * @param null|EntityDocument|EntityRedirect|EntityContent $new
 	 * @param IContextSource $context
 	 * @param string $summary
 	 * @return Status
@@ -47,10 +48,21 @@ public function run( $new, IContextSource $context, $summary ) {
 		$hookStatus = $this->hookRunner->run( $new, $context, $summary );
 		$attemptSaveFilterEnd = microtime( true );
 
-		$this->stats->timing(
-			"{$this->timingPrefix}.run.{$new->getType()}",
-			( $attemptSaveFilterEnd - $attemptSaveFilterStart ) * 1000
-		);
+		if ( $new !== null ) {
+			if ( $new instanceof EntityDocument ) {
+				$entityType = $new->getType();
+			} elseif ( $new instanceof EntityRedirect ) {
+				$entityType = $new->getEntityId()->getEntityType();
+			} elseif ( $new instanceof EntityContent ) {
+				$entityType = $new->getEntityId()->getEntityType();
+			} else {
+				$entityType = 'UNKNOWN';
+			}
+			$this->stats->timing(
+				"{$this->timingPrefix}.run.{$entityType}",
+				( $attemptSaveFilterEnd - $attemptSaveFilterStart ) * 1000
+			);
+		}
 
 		return $hookStatus;
 	}

repo/tests/phpunit/includes/Actions/ActionTestCase.php

@@ -91,6 +91,10 @@ private function makeTestItemData() {
 		$item->setLabel( 'de', 'London' );
 		$items['London'][] = $item;
 
+		$item = new Item();
+		$item->setLabel( 'en', 'Oslo' );
+		$items['Oslo'][] = $item;
+
 		$item = new Item();
 		$item->setLabel( 'de', 'Oslo' );
 		$item->setLabel( 'en', 'Oslo' );

repo/tests/phpunit/includes/Actions/EditEntityActionTest.php

@@ -2,12 +2,16 @@
 
 namespace Wikibase\Repo\Tests\Actions;
 
+use IContextSource;
 use MediaWiki\MediaWikiServices;
 use RuntimeException;
+use Status;
 use Title;
 use User;
+use Wikibase\DataModel\Term\LabelsProvider;
 use Wikibase\Repo\Actions\EditEntityAction;
 use Wikibase\Repo\Actions\SubmitEntityAction;
+use Wikibase\Repo\Content\EntityContent;
 use Wikibase\Repo\WikibaseRepo;
 use WikiPage;
 
@@ -32,6 +36,27 @@ protected function setUp(): void {
 
 		// Remove handlers for the "OutputPageParserOutput" hook
 		$this->clearHook( 'OutputPageParserOutput' );
+
+		// Install EditFilterMergedContent hook blocking any edits to "Oslo"@en
+		$method = __METHOD__;
+		$this->setTemporaryHook(
+			'EditFilterMergedContent',
+			function ( IContextSource $context, EntityContent $entityContent, Status $status ) use ( $method ) {
+				if ( !$entityContent->isRedirect() ) {
+					$entity = $entityContent->getEntity();
+					if ( $entity instanceof LabelsProvider ) {
+						$labels = $entity->getLabels();
+						if ( $labels->hasTermForLanguage( 'en' )
+							&& $labels->getByLanguage( 'en' )->getText() === 'Oslo'
+						) {
+							$status->fatal( $method . ': block "Oslo"@en edit' );
+							return false;
+						}
+					}
+				}
+				return true;
+			}
+		);
 	}
 
 	public function testActionForPage() {
@@ -845,6 +870,20 @@ public function provideUndoSubmit() {
 			null, // user
 			'/wikibase-undo-redirect-latestnoredirect/', // htmlPattern: should contain error
 		];
+
+		// blocked by edit filter (e.g. AbuseFilter, T250720), see setUp()
+		yield 'edit blocked by edit filter' => [
+			'submit', // action
+			'Oslo', // handle
+			[ // params
+				'wpSave' => 1,
+				'wpEditToken' => true, // automatic token
+				'undo' => 0, // current revision
+			],
+			true, // post
+			null, // user
+			'/block "Oslo"@en edit/', // htmlPattern: should contain filter error message
+		];
 	}
 
 	/**