T414011.patch
Dreamy_Jazz (WBrown (WMF))
Actions

Authored By

	Dreamy_Jazz
	Jan 7 2026, 8:30 PM

Size

3 KB

Referenced Files

None

Subscribers

None

T414011.patch
View Options

	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

File Metadata

Mime Type: text/x-diff
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 22884984
Default Alt Text: T414011.patch (3 KB)

T414011.patchDreamy_Jazz (WBrown (WMF))Actions

T414011.patchView Options

File Metadata

Event Timeline

T414011.patch
Dreamy_Jazz (WBrown (WMF))
Actions

T414011.patch
View Options