Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Page MenuHomePhabricator
Create Task
Maniphest T346235

Add/Remove mentor bypass restrictions applied by abusefilter
Closed, ResolvedPublicBUG REPORT
Actions

Description

Steps to replicate the issue (include links if applicable):

  • Imagine there's an abusefilter looks like the following, with action disallow
page_title = ' MediaWiki:GrowthMentors.json'
& user_name = 'FooBar'
  • User FooBar wants to add them as a mentor

What happens?:
User FooBar could do so, which means they successfully edited the config file MediaWiki:GrowthMentors.json

What should have happened instead?:
User FooBar should not be allowed to do so, as they will trap abusefilter

Software version (skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):

Details

SubjectRepoBranchLines +/-
WikiPageConfigWriter: Do not save edit when AbuseFilter disallows itmediawiki/extensions/GrowthExperimentsmaster+46 -21
WikiPageConfigWriter: Run EditFilterMergedContentHookmediawiki/extensions/GrowthExperimentsmaster+94 -39
Customize query in gerrit

Related Objects

Event Timeline

Stang created this task.Sep 13 2023, 12:52 PM
Restricted Application added a project: Growth-Team. · View Herald TranscriptSep 13 2023, 12:52 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
SCP-2000 subscribed.Sep 13 2023, 12:56 PM
Urbanecm_WMF subscribed.Sep 16 2023, 11:37 PM
Urbanecm_WMF claimed this task.Sep 18 2023, 11:04 AM
Urbanecm_WMF triaged this task as High priority.
Urbanecm_WMF edited projects, added Growth-Team (Sprint 0 (Growth Team)); removed Growth-Team.

Thanks for reporting! We intentionally bypass a substantial part of the permissions system (to allow non-admins to enroll as a mentor; normally, they wouldn't be allowed to do so, as the list of mentors is in the MediaWiki namespace). That being said, I don't think we should be bypassing the EditFilterMergedContent hook, which enables AbuseFilters to disallow an edit. I'll upload a patch to change that.

@Stang Please note the filter from the task description would not work even if GrowthExperiments invoked the EditFilterMergedContent hook, because page_title is an unprefixed title (for MediaWiki:GrowthMentors.json, its value would be GrowthMentors.json). For the filter to work (once my patch gets deployed), you'd need to either use page_prefixedtitle = 'MediaWiki:GrowthMentors.json' or page_namespace = 8 & page_title = 'GrowthMentors.json' (one can also condition based on page_id). For details, see AbuseFilter's documentation on MediaWiki.org.

I'd also like to note that setting an AbuseFilter to prevent one user from making /any/ edit to GrowthMentors.json is probably not necessary. It should be possible to (partially) block any user from the JSON page, and that should be already fixed (see T322047).

Hope this helps!

gerritbot added a comment.Sep 18 2023, 11:14 AM

Change 958444 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] WikiPageConfigWriter: Run EditFilterMergedContentHook

https://gerrit.wikimedia.org/r/958444

gerritbot added a project: Patch-For-Review.Sep 18 2023, 11:14 AM
Urbanecm_WMF changed the task status from Open to In Progress.Sep 18 2023, 11:15 AM
Urbanecm_WMF moved this task from Incoming to Code Review on the Growth-Team (Sprint 0 (Growth Team)) board.
Urbanecm_WMF edited projects, added GrowthExperiments-EditGrowthConfig, GrowthExperiments-Mentorship; removed GrowthExperiments.Oct 4 2023, 5:31 PM
Maintenance_bot moved this task from New Tasks to In Progress on the GrowthExperiments-Mentorship board.Oct 4 2023, 6:15 PM
Urbanecm_WMF moved this task from Code Review to QA on the Growth-Team (Sprint 0 (Growth Team)) board.Oct 24 2023, 12:55 PM
Urbanecm_WMF moved this task from In Progress to Ready for QA on the GrowthExperiments-Mentorship board.
gerritbot added a comment.Oct 24 2023, 1:16 PM

Change 958444 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] WikiPageConfigWriter: Run EditFilterMergedContentHook

https://gerrit.wikimedia.org/r/958444

Maintenance_bot removed a project: Patch-For-Review.Oct 24 2023, 1:29 PM
Urbanecm_WMF changed the task status from In Progress to Open.Oct 24 2023, 1:59 PM
ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.3; 2023-10-31).Oct 24 2023, 2:00 PM
Urbanecm_WMF edited projects, added Growth-Team (Sprint 1 (Growth Team)); removed Growth-Team (Sprint 0 (Growth Team)).Oct 24 2023, 2:02 PM
Urbanecm_WMF moved this task from Incoming to QA on the Growth-Team (Sprint 1 (Growth Team)) board.
SunAfterRain added a comment.EditedOct 25 2023, 2:54 AM

According to testing, although AbuseFilter was indeed recorded after the patch, edits were not properly blocked. Moreover, the differences left by AbuseFilter are not correct, and a JSON escape sequence appears.
log: https://zh.wikipedia.beta.wmflabs.org/wiki/Special:AbuseLog/1210
diff: https://zh.wikipedia.beta.wmflabs.org/wiki/Special:Diff/23457

Urbanecm_WMF moved this task from QA to Doing on the Growth-Team (Sprint 1 (Growth Team)) board.Oct 25 2023, 8:33 AM

Okay...I can reproduce this in beta. The enrollment form shows the error message, but the edit goes through nevertheless:

image.png (748×1 px, 128 KB)

Thanks for testing and reporting! Moving back to Doing for me to investigate why this happens.

gerritbot added a comment.Oct 25 2023, 2:41 PM

Change 968701 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] WikiPageConfigWriter: Do not save edit when AbuseFilter disallows it

https://gerrit.wikimedia.org/r/968701

gerritbot added a project: Patch-For-Review.Oct 25 2023, 2:41 PM
Urbanecm_WMF moved this task from Doing to Code Review on the Growth-Team (Sprint 1 (Growth Team)) board.Oct 25 2023, 2:41 PM
gerritbot added a comment.Oct 25 2023, 9:23 PM

Change 968757 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] i18n: Add GENDER support to a message

https://gerrit.wikimedia.org/r/968757

Urbanecm_WMF added a comment.Oct 25 2023, 9:24 PM
In T346235#9281890, @gerritbot wrote:

Change 968757 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] i18n: Add GENDER support to a message

https://gerrit.wikimedia.org/r/968757

(linked accidentally)

gerritbot added a comment.Oct 26 2023, 9:37 AM

Change 968701 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] WikiPageConfigWriter: Do not save edit when AbuseFilter disallows it

https://gerrit.wikimedia.org/r/968701

Urbanecm_WMF moved this task from Code Review to QA on the Growth-Team (Sprint 1 (Growth Team)) board.Oct 26 2023, 12:33 PM
Urbanecm_WMF removed a project: Patch-For-Review.Oct 26 2023, 1:17 PM
Etonkovidova moved this task from QA to Test in Production on the Growth-Team (Sprint 1 (Growth Team)) board.Oct 31 2023, 9:32 PM
Etonkovidova subscribed.

Checked in beta - seems to work as expected; a disallowed user cannot sign up as a mentor - the following message is displayed:

Screen Shot 2023-10-31 at 2.21.34 PM.png (838×1 px, 132 KB)

Etonkovidova closed this task as Resolved.Nov 21 2023, 10:43 PM
Stang unsubscribed.Nov 25 2023, 10:33 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits