From c12b596f56ec0cac07b219bf094017f04e6eabd2 Mon Sep 17 00:00:00 2001
From: Dreamy Jazz <wpgbrown@wikimedia.org>
Date: Wed, 7 Jan 2026 20:26:24 +0000
Subject: [PATCH] SECURITY: Check suppressrevision for edit filter check

Why:
* The AbuseFilterPermissionManager::canEditFilter method returns
  whether a user can edit a given filter
** This is a generic check just for the abusefilter-modify right,
   as it is assumed that filters with additional visibility
   permissions have the same right for viewing and editing
*** However, this assumption fails for suppressed filters where the
    right to view (viewsuppressed) is different to editing filters
    (suppressrevision)
* Therefore AbuseFilterPermissionManager::canEditFilter
  needs to check the suppressrevision right if the filter is
  suppressed

What:
* Update AbuseFilterPermissionManager::canEditFilter to return
  false early if the filter is suppressed and ::canSuppress
  returns false
* Add PHPUnit tests to check this

Bug: T414011
Change-Id: Idafaedbf82d5f5e9dde5f07b94a66fe264762339
---
 includes/AbuseFilterPermissionManager.php     |  6 +++++
 .../unit/AbuseFilterPermissionManagerTest.php | 26 +++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/includes/AbuseFilterPermissionManager.php b/includes/AbuseFilterPermissionManager.php
index 9c17314b..1f5da675 100644
--- a/includes/AbuseFilterPermissionManager.php
+++ b/includes/AbuseFilterPermissionManager.php
@@ -59,6 +59,12 @@ class AbuseFilterPermissionManager {
 	 * @return bool
 	 */
 	public function canEditFilter( Authority $performer, AbstractFilter $filter ): bool {
+		// A user with viewsuppressed can view suppressed filters but if they lack
+		// the suppressrevision right then they shouldn't be able to edit it (T414011)
+		if ( $filter->isSuppressed() && !$this->canSuppress( $performer ) ) {
+			return false;
+		}
+
 		return (
 			$this->canEdit( $performer ) &&
 			!( $filter->isGlobal() && !$this->canEditGlobal( $performer ) )
diff --git a/tests/phpunit/unit/AbuseFilterPermissionManagerTest.php b/tests/phpunit/unit/AbuseFilterPermissionManagerTest.php
index 0356aed9..eeadce90 100644
--- a/tests/phpunit/unit/AbuseFilterPermissionManagerTest.php
+++ b/tests/phpunit/unit/AbuseFilterPermissionManagerTest.php
@@ -147,6 +147,32 @@ class AbuseFilterPermissionManagerTest extends MediaWikiUnitTestCase {
 		yield 'both' => [ [ 'abusefilter-modify', 'abusefilter-view-private' ], true ];
 	}
 
+	/** @dataProvider provideCanEditFilterWhenFilterIsSuppressed */
+	public function testCanEditFilterWhenFilterIsSuppressed( array $rights, bool $expected ) {
+		$filter = MutableFilter::newDefault();
+		$filter->setSuppressed( true );
+		$performer = $this->mockRegisteredAuthorityWithPermissions( $rights );
+
+		$this->assertSame(
+			$expected,
+			$this->getPermMan()->canEditFilter( $performer, $filter )
+		);
+	}
+
+	public static function provideCanEditFilterWhenFilterIsSuppressed(): array {
+		return [
+			'User has both viewsuppressed and suppressrevision' => [
+				[ 'abusefilter-modify', 'viewsuppressed', 'suppressrevision' ], true,
+			],
+			'User has suppressrevision' => [
+				[ 'abusefilter-modify', 'suppressrevision' ], true,
+			],
+			'User has viewsuppressed' => [
+				[ 'abusefilter-modify', 'viewsuppressed' ], false,
+			],
+		];
+	}
+
 	/**
 	 * @dataProvider provideCanViewPrivateFilters
 	 */
-- 
2.34.1