Dry run for normalizeThrottleParameters.php
Open, HighPublic

Description

In order to fix throttle parameters, we need to execute the normalizeThrottleParameters.php script [1]. I'm hereby asking to please run it with --dry-run on all Wikimedia wikis to see how it goes. We don't need anything more than that, as the script will then be added to update.php [2].

[1]: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/459368/
[2]: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/459245/

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 15 2018, 7:59 AM
Daimona triaged this task as High priority.Nov 15 2018, 9:16 AM

Setting to high to reflect the importance of the parent task and related ones (specifically T203336 which is 1.32 blocker).

Daimona added subscribers: Anomie, Reedy, tstarling.

Adding Platform team per maintainers, plus CC'ing people active in this project and Tim who reviewed the patch. Being the related task UBN and release blockers, the plan is to get this done before 1.32 release. Also, a note for whoever will run the script: it's not included in production (it'll be in wmf.6), but given the circumstances, could you please deploy it and do the dry run? Deploying the script alone (and adding it to autoload) would be enough, no other code from that patch is required.

Daimona raised the priority of this task from High to Unbreak Now!.EditedNov 28 2018, 11:28 AM

Testing this script for the last time showed that it still has some problems:

  1. A get_object_vars is needed at line 234
  2. We should probably just use the AbuseFilter user to perform the change, as that avoids introducing a new, unknown, unlocalized system user (and maybe some perplexity by filter maintainers)
  3. The change by AbuseFilter updater doesn't show up next to "Filter last modified" when on Special:AbuseFilter/xxx, although it does show in Special:AbuseFilter/history/xxx (unless the throttle action is completely removed: in this case, it correctly shows up). UPDATE: This happen because when simply updating parameters we don't update the abuse_filter table, while we do that when removing throttle
  4. If you go to the history of a filter that has been changed and click "Changes" next to the edit by AbuseFilter updater, an empty diff could show up. This only seem to happen when removing duplicated groups (e.g. "user-ip-user" -> "user-ip"). UPDATE: this is mostly a visual bug. When computing the diff between old and new actions, we stringify action parameters using AbuseFilter::formatAction. This function was changed in the main patch so that it only shows unique groups. However, this means empty diffs with old entries. I'll probably revert this part and avoid showing unique groups.

Raising priority to UBN because parent task is UBN, and this also requires some extra work (although 1. and 2. are easy). I'll try to send a new patch today/tomorrow; @tstarling any chance you'll review this again?

Restricted Application added subscribers: Liuxinyu970226, TerraCodes. · View Herald TranscriptNov 28 2018, 11:28 AM

Change 476244 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] [WIP] Fix big problems with normalizeThrottleParameters

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

Change 476251 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_32] Fix big problems with normalizeThrottleParameters

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

Change 476244 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Fix big problems with normalizeThrottleParameters

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

Change 479998 had a related patch set uploaded (by Tim Starling; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@wmf/1.33.0-wmf.8] [1.33.0-wmf.8] Fix big problems with normalizeThrottleParameters

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

Change 479998 merged by Tim Starling:
[mediawiki/extensions/AbuseFilter@wmf/1.33.0-wmf.8] [1.33.0-wmf.8] Fix big problems with normalizeThrottleParameters

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

It would be convenient if the script would report all malformed throttle rows, instead of stopping on the first one. Or maybe we could think of some way automatically deal with the bad rows, since there seems to be quite a lot. For example, disabling the throttle and generating a report for admins to check later.

I checked enwiki filter 347 as a case study. It is a test filter that is marked as disabled and deleted, so it would not be harmful to disable throttling in it. afa_parameters is

347
3,300
user, page

Here is the script output, omitting wikis which gave "No throttle parameters to normalize."

arwiki: Throttle count and period for filter 66 are malformed or empty. Please fix them by hand in the way they're meant to be, then launch the script again.

bgwiki: Throttle groups are empty for filter 20. Please add some groups or disable throttling, then launch the script again.

cawiki:  normalizeThrottleParameter has found 1 rows to change in abuse_filter_action for the following IDs: 9
cawiki:  normalizeThrottleParameter would insert 1 rows in abuse_filter_history for the following filters: 9
cawiki:  Throttle parameter normalization would change a total of 2 rows.

cawikinews:  normalizeThrottleParameter has found 1 rows to change in abuse_filter_action for the following IDs: 15
cawikinews:  normalizeThrottleParameter would insert 1 rows in abuse_filter_history for the following filters: 15
cawikinews:  Throttle parameter normalization would change a total of 2 rows.

cswiki: Throttle count and period for filter 64 are malformed or empty. Please fix them by hand in the way they're meant to be, then launch the script again.

dawiki: Throttle groups are empty for filter 5. Please add some groups or disable throttling, then launch the script again.

dewikiversity: Throttle count and period for filter 4 are malformed or empty. Please fix them by hand in the way they're meant to be, then launch the script again.

enwiki: Throttle groups are empty for filter 347. Please add some groups or disable throttling, then launch the script again.

fawiki: Throttle count and period for filter 141 are malformed or empty. Please fix them by hand in the way they're meant to be, then launch the script again.

fiwiki: Throttle groups are empty for filter 99. Please add some groups or disable throttling, then launch the script again.

glwiki: Throttle groups are empty for filter 15. Please add some groups or disable throttling, then launch the script again.

itwiki: Throttle groups are empty for filter 148. Please add some groups or disable throttling, then launch the script again.

kkwiki: Throttle groups are empty for filter 56. Please add some groups or disable throttling, then launch the script again.

kmwiki:
[34d557fbdd3aae256897c940] [no req]   BadMethodCallException from line 2111 of /srv/mediawiki/php-1.33.0-wmf.8/extensions/AbuseFilter/includes/AbuseFilter.php: Call to a member function getGroups() on a non-object (null)
Backtrace:
#0 /srv/mediawiki/php-1.33.0-wmf.8/extensions/AbuseFilter/maintenance/normalizeThrottleParameters.php(136): AbuseFilter::getFilterUser()
#1 /srv/mediawiki/php-1.33.0-wmf.8/maintenance/Maintenance.php(1698): NormalizeThrottleParameters->doDBUpdates()
#2 /srv/mediawiki/php-1.33.0-wmf.8/maintenance/doMaintenance.php(94): LoggedUpdateMaintenance->execute()
#3 /srv/mediawiki/php-1.33.0-wmf.8/extensions/AbuseFilter/maintenance/normalizeThrottleParameters.php(404): include(string)
#4 /srv/mediawiki/multiversion/MWScript.php(100): include(string)
#5 {main}

kmwikibooks:
[d4ae6bb0701a7a9ebfe7d4a0] [no req]   BadMethodCallException from line 2111 of /srv/mediawiki/php-1.33.0-wmf.8/extensions/AbuseFilter/includes/AbuseFilter.php: Call to a member function getGroups() on a non-object (null)
Backtrace:
#0 /srv/mediawiki/php-1.33.0-wmf.8/extensions/AbuseFilter/maintenance/normalizeThrottleParameters.php(136): AbuseFilter::getFilterUser()
#1 /srv/mediawiki/php-1.33.0-wmf.8/maintenance/Maintenance.php(1698): NormalizeThrottleParameters->doDBUpdates()
#2 /srv/mediawiki/php-1.33.0-wmf.8/maintenance/doMaintenance.php(94): LoggedUpdateMaintenance->execute()
#3 /srv/mediawiki/php-1.33.0-wmf.8/extensions/AbuseFilter/maintenance/normalizeThrottleParameters.php(404): include(string)
#4 /srv/mediawiki/multiversion/MWScript.php(100): include(string)
#5 {main}

kmwiktionary:
[68a279e79273e48b0020e7dd] [no req]   BadMethodCallException from line 2111 of /srv/mediawiki/php-1.33.0-wmf.8/extensions/AbuseFilter/includes/AbuseFilter.php: Call to a member function getGroups() on a non-object (null)
Backtrace:
#0 /srv/mediawiki/php-1.33.0-wmf.8/extensions/AbuseFilter/maintenance/normalizeThrottleParameters.php(136): AbuseFilter::getFilterUser()
#1 /srv/mediawiki/php-1.33.0-wmf.8/maintenance/Maintenance.php(1698): NormalizeThrottleParameters->doDBUpdates()
#2 /srv/mediawiki/php-1.33.0-wmf.8/maintenance/doMaintenance.php(94): LoggedUpdateMaintenance->execute()
#3 /srv/mediawiki/php-1.33.0-wmf.8/extensions/AbuseFilter/maintenance/normalizeThrottleParameters.php(404): include(string)
#4 /srv/mediawiki/multiversion/MWScript.php(100): include(string)
#5 {main}

kowiki: Throttle count and period for filter 31 are malformed or empty. Please fix them by hand in the way they're meant to be, then launch the script again.

mrwiki: Throttle count and period for filter 9 are malformed or empty. Please fix them by hand in the way they're meant to be, then launch the script again.

ptwiki: Throttle count and period for filter 11 are malformed or empty. Please fix them by hand in the way they're meant to be, then launch the script again.

rowiki: Throttle groups are empty for filter 67. Please add some groups or disable throttling, then launch the script again.

ruwiki: Throttle groups are empty for filter 35. Please add some groups or disable throttling, then launch the script again.

ruwiktionary: Throttle count and period for filter 8 are malformed or empty. Please fix them by hand in the way they're meant to be, then launch the script again.

trwiki:  normalizeThrottleParameter has found 5 rows to change in abuse_filter_action for the following IDs: 37, 46, 47, 48, 50
trwiki:  normalizeThrottleParameter would insert 5 rows in abuse_filter_history for the following filters: 47, 48, 50, 46, 37
trwiki:  Throttle parameter normalization would change a total of 10 rows.

ukwiki: Throttle groups are empty for filter 29. Please add some groups or disable throttling, then launch the script again.

zhwiki: Throttle groups are empty for filter 78. Please add some groups or disable throttling, then launch the script again.
Daimona added a comment.EditedMon, Dec 17, 9:54 AM

@tstarling Thanks for running the script. I thought these bad cases would have been way less common. But now yes, I agree that we should list every bad filter instead of only the first one. As for automatically dealing with bad rows, we could improve the script by:

  • Adding special cases. For instance, the enwiki filter only has an extra comma in the first group ("user,")[1]; for instance, we could see what happens in cases like this and either remove the comma, or remove the group.
  • Disabling throttle for disabled/deleted filters.
  • Somehow fix the filter and warn admins to check it (where and how?)

The main problem is that disabling throttle alone isn't that good, especially if other actions are enabled.
I'll try to examinate some of the listed filters to see if I can spot common mistakes, plus I'll have to investigate the exception from km* projects.

[1] (update): actually, it's the extra space between "user" and "page".

Change 476251 merged by Daimona Eaytoy:
[mediawiki/extensions/AbuseFilter@REL1_32] Fix big problems with normalizeThrottleParameters

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

In the meanwhile I understood that the exception happens because the system user name isn't validated, and the km message contains an invalid character (\u200b). I sent https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/480089/ which will help in the long-term, and removed the character on translatewiki for a quick solution.
I also examined every wrong filter one by one; mostly, errors happen due to empty throttle groups, or all parameters left blank. This second case is especially common for filters which used to have valid parameters, but have been modified between the OOUI switch (which didn't recognize such groups) and the patch for the parent task, without users noticing; this is why I set UBN for this. I think the main question here is: how much do we want to change automatically? Depending on the degree of freedom we want to take, the answer is different. For instance, we could only fix typos (like for enwiki), or check if the last filter version is valid and restore its parameters, or even always set a default. In any case, the second important question is: how do we warn users about the changes made, asking them to check the involved filters?

Change 480157 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Report all filters with wrong throttle parameters

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

Change 480157 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Report all filters with wrong throttle parameters

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

Change 480449 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_32] Report all filters with wrong throttle parameters

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

Change 480449 merged by Daimona Eaytoy:
[mediawiki/extensions/AbuseFilter@REL1_32] Report all filters with wrong throttle parameters

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

Stryn added a subscriber: Stryn.EditedTue, Dec 18, 4:47 PM

What happened here?
Now in every wikis there is a local AbuseFilter (with localized user names) user account that is a sysop.
In Finnish language projects: https://meta.wikimedia.org/wiki/Special:CentralAuth/V%C3%A4%C3%A4rink%C3%A4ytt%C3%B6suodatin
In Finnish Wikipedia a local bureaucrat just removed user rights from this user, because they were granted to user account without consensus (as required by the local policy).

At least give a global user name and make a global user page for that account. And announce in wikis that there is going to be a new sysop, and not just surprise everyone, "oh we have now a new sysop!"

@Stryn while this is indeed a side effect of the script, it' not something to be worried about, nor it could make any harm. The "abusefilter user" (whose name is localisable by changing the "abusefilter-blocker" message) has existed since 2008 (yep, 10 years, i.e. since the creation of the extension), and is the account used to block people, if the "block" option is enabled on the wiki. Apparently it's not enabled on fi projects, but it is in other projects, and people there are aware of what this user does.
For this task, we're going to use such user to perform maintenance on broken abuse filters, and we didn't want to introduce a new user for that.
As for the rights, it's perfectly useless to remove them, because it is a system user and rights are automatically granted by MediaWiki when needed. Needless to say, if the "block" action isn't enabled that account won't ever block anyone, so there's nothing to be worried about.
There's a task about making this user's name the same on every project, although I don't know if we want to do it (I cannot find the task number right now), while a global user page should be created by meta admins, and of course would only work for WMF wikis.
Last but not least, while it wasn't intended to create the user everywhere with this script, we can still write down a line in tech/news, but -again- keep in mind that this feature has existed for 10 years.
(I'm sorry for any error in the message above, I'm writing from mobile)

Pxos added a subscriber: Pxos.Tue, Dec 18, 5:58 PM

I'm afraid the Script Masters fail to see how far the Finnish Wikipedia is upset. There are many issues, but rather than getting into them, I have a simple solution to the problem: create a new user group called "abuse filter sysop" as a global group. There are already several one-man-bands such as Founder, Abuse_Filter_Helper that contain just one user.

If this new system user must be a universal sysop everywhere, please do not make the account a normal sysop and upset the communities, but make him instead an "Abuse Filter Sysop" so that he might disappear from the lists of normal human sysops. There core of the problem seems to be that the Community does not understand and appreciate what the Codemasters are trying to do, and vice versa the Codemasters dismiss our legitimate concerns as trivial and unimportant. --Pxos

@Pxos I'm not a "Codemaster", and -disclaimer- I do not work with WMF. I'm a volunteer, just like you are, so I think I can understand what you're talking about. However, I do recognize that (personally speaking) I cannot understand how people could be upset by this. Would you block the "massmessage delivery" user, or deflag it, just because it runs as a non-explicitly-authorized bot? There's no need to add a new user group, mainly because that account is not associated to any user. It's an automatic account, no-one controls it. It can only perform blocks if the community wants to enable blocks for AbuseFilter AND enable blocking for a filter. I cannot really see how this could upset anyone, if they know why such system user exists. I'll be glad to provide further explanation, but -again- I admit that I fail to see this situation as a problem. I can still help to find a solution (although not soon due to holidays), but for the moment, couldn't you just create a userpage with some explanation and live with it, as every wiki with AbuseFilter enabled does?

PS (this came to my mind after writing the msg above), the AbuseFilter account doesn't need the sysop group to block people, it's just added so that wiki editors won't waste hours trying to understand how an autoconfirmed user could block someone.

Huji added a subscriber: Huji.Tue, Dec 18, 6:33 PM
This comment was removed by Huji.

@Pxos: Where exactly were legitimate concerns dismissed as trivial and unimportant?

Huji added a comment.EditedTue, Dec 18, 6:51 PM

Here is the script output, omitting wikis which gave "No throttle parameters to normalize."

dewikiversity: Throttle count and period for filter 4 are malformed or empty. Please fix them by hand in the way they're meant to be, then launch the script again.

Fixed.

fawiki: Throttle count and period for filter 141 are malformed or empty. Please fix them by hand in the way they're meant to be, then launch the script again.

Fixed using local sysop rights.

kkwiki: Throttle groups are empty for filter 56. Please add some groups or disable throttling, then launch the script again.

Fixed.

ruwiktionary: Throttle count and period for filter 8 are malformed or empty. Please fix them by hand in the way they're meant to be, then launch the script again.

Fixed.

The rest of them are wikis that have opted out of Global Sysop and I cannot fix them.

@Huji Thanks for the work! Don't worry for the others; actually, it's not totally bad to leave them wrong for another couple of days: they provide us some sort of live tests for the script.
As for the fiwiki inconvenience: do whatever you want with sysop rights, but I have to warn you that the right will be added once again when the script will be re-launched (and actually, @tstarling, would you mind backporting the last patches to wmf.8 and re-run it?). Should you have other doubts or questions, I'll be happy to help. You can contact me on any wiki (in english), or open a new phab task: this conversation is a bit off-topic here. We can think of a better solution, although my suggestion is still to ignore the user - at least temporary. A compromise could be to resolve T160666, and maybe add some code to automatically create a localised user page.

Ejs-80 added a subscriber: Ejs-80.Tue, Dec 18, 11:15 PM

fiwiki: Throttle groups are empty for filter 99. Please add some groups or disable throttling, then launch the script again.

Fixed.

Thanks for your feedback, @Stryn and @Pxos. I filed T212268 for the problem of automatically created sysop users. Please put any further discussion on that task.

Change 480681 had a related patch set uploaded (by Tim Starling; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@wmf/1.33.0-wmf.8] [1.33.0-wmf.8] Report all filters with wrong throttle parameters

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

Change 480681 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@wmf/1.33.0-wmf.8] [1.33.0-wmf.8] Report all filters with wrong throttle parameters

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

Ran it again:

bgwiki:  Throttle groups are empty for the following filters: 20, 20. Please add some groups or disable throttling, then launch the script again.
cawiki:  normalizeThrottleParameter has found 1 rows to change in abuse_filter_action for the following IDs: 9
cawiki:  normalizeThrottleParameter would insert 1 rows in abuse_filter_history for the following filters: 9
cawiki:  Throttle parameter normalization would change a total of 2 rows.
cawikinews:  normalizeThrottleParameter has found 1 rows to change in abuse_filter_action for the following IDs: 15
cawikinews:  normalizeThrottleParameter would insert 1 rows in abuse_filter_history for the following filters: 15
cawikinews:  Throttle parameter normalization would change a total of 2 rows.
dawiki:  Throttle groups are empty for the following filters: 5, 5. Please add some groups or disable throttling, then launch the script again.
enwiki:  Throttle groups are empty for the following filters: 347, 742, 347, 742. Please add some groups or disable throttling, then launch the script again.
glwiki:  Throttle groups are empty for the following filters: 15, 15. Please add some groups or disable throttling, then launch the script again.
itwiki:  Throttle groups are empty for the following filters: 148, 367, 148, 367. Please add some groups or disable throttling, then launch the script again.
mrwiki:  Throttle count and period are malformed or empty for the following filters: 9. Please fix them by hand in the way they're meant to be, then launch the script again. Throttle groups are empty for the following filters: 125, 127, 129, 130, 131, 134, 129, 130, 131, 134, 127, 125. Please add some groups or disable throttling, then launch the script again.
rowiki:  Throttle groups are empty for the following filters: 67, 67. Please add some groups or disable throttling, then launch the script again.
ruwiki:  Throttle groups are empty for the following filters: 35, 35. Please add some groups or disable throttling, then launch the script again.
ruwiktionary:  Throttle groups are empty for the following filters: 33, 33. Please add some groups or disable throttling, then launch the script again.
trwiki:  normalizeThrottleParameter has found 5 rows to change in abuse_filter_action for the following IDs: 37, 46, 47, 48, 50
trwiki:  normalizeThrottleParameter would insert 5 rows in abuse_filter_history for the following filters: 47, 48, 50, 46, 37
trwiki:  Throttle parameter normalization would change a total of 10 rows.
ukwiki:  Throttle groups are empty for the following filters: 29, 30, 53, 30, 29, 53. Please add some groups or disable throttling, then launch the script again.
zhwiki:  Throttle groups are empty for the following filters: 131, 131. Please add some groups or disable throttling, then launch the script again.

Change 480704 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Another couple of minor fixes, hopefully the last change to this script.

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

Change 480705 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_32] Re-fix the throttle script

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

Change 480706 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@wmf/1.33.0-wmf.8] Re-fix the throttle script

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

@tstarling Thanks. The patch above should solve the duplicate IDs problem; I guess it's not necessary to dry-run again just because of that. And now we're back to the big question: how to reduce the amount of manual work. Personally, I think the only idea worth investigating is checking past revisions of the filter (possibly the first after the offending patch was merged) to see if we can restore its parameters. Then we should write a couple of lines in T/N with a link to the above paste so that people can fix filters on their wikis.

MarcoAurelio added a subscriber: MarcoAurelio.EditedWed, Dec 19, 11:32 AM

What happened here?
Now in every wikis there is a local AbuseFilter (with localized user names) user account that is a sysop.
In Finnish language projects: https://meta.wikimedia.org/wiki/Special:CentralAuth/V%C3%A4%C3%A4rink%C3%A4ytt%C3%B6suodatin
In Finnish Wikipedia a local bureaucrat just removed user rights from this user, because they were granted to user account without consensus (as required by the local policy).

At least give a global user name and make a global user page for that account. And announce in wikis that there is going to be a new sysop, and not just surprise everyone, "oh we have now a new sysop!"

While I am sympathetic to the users feeling surprised due to the appearance of a new sysop account, I feel we're making a storm out of a teacup looking at comments of apparent outrageousness here and there. If we look at Special:ListUsers/bot the same has happened for tons of other maintenance scripts or system accounts and nobody has ever complained. Perhaps when this maintenance of AbuseFilter is done we can run a script server-side to remove the sysop flags for the everywhere should the account not need them anymore, or at least on the wikis where the filter is not allowed to block (cfr. T212268). Meanwhile I suggest that we add a User-notice to solve the apparent confusion this has caused. But I must say that I disagree strongly with the comments I read somewhat implying this was done in bad faith, because it was not; and I thank very much @Daimona for devoting his volunteer time to fix and amend this extremely complex and useful extension for all of us. Thanks.

Thanks @MarcoAurelio for your comment and your appreciation. I totally agree that this looks like much ado about nothing. Currently, our priority (or, at least, mine) should be to get this maintenance done, then we should understand what to do with the system user. Removing the flag is discussed in the linked task, but I'm not convinced in removing it only where block isn't enabled. First, because the AbuseFilter user can also degroup and block autopromotion. Second, because we could use it for other maintenance tasks in the future (like it's being done here). Again, any discussion should be in the other task.
I'm also adding tech news to inform people about the change. The message could be something like

Last week, in order to perform some technical maintenance on AbuseFilter, a new account with sysop rights and the localized name "{{int:abusefilter-blocker}}" has been created on several wikis. On wikis where AbuseFilter can perform blocks, this user already existed and is automatically used to block people. For other wikis, the account will do nothing but a one-time maintenance, so there's no reason to be worried.

And we could also add the following:

Some filters cannot be automatically edited and must be manually checked. You may find a list of affected filters [[link|here]]; if you're familiar with AbuseFilter, please take a look and fix the faulty filters.

And we should also explain what the problem is:

The "{{int:abusefilter-action-throttle}}" action takes three parameters: count, period and groups. Previously, users could freely insert everything in those fields. Now we require these parameters to be valid, i.e. they should: *not be empty; *count and period must be positive integers; * groups must be a combination of valid groups (listed here), split by commas or newlines depending on the effect you want to achieve. [The link above explains this too]

Anomie removed a subscriber: Anomie.Wed, Dec 19, 3:20 PM
Daimona moved this task from To Triage to Announce in next Tech/News on the User-notice board.EditedFri, Dec 21, 4:27 PM

Ah, I just realized that the next T/N issue will be on Jan 7th. Not the best period for "bad" things to happen :/ On the other hand, hopefully we have a couple of days more to decide what to do here, and what to announce.

EDIT: Looking at reports on meta and on dewiki, I guess we should also specify that:

  • I didn't think the account would have been created by this script (actually, I thought it existed even on wikis without blocks); matter of fact, on some wikis it exists since 2008 and hasn't changed since then.
  • There's no evil plan to take over any wiki: I see several complot-leaning comments, especially directed to WMF (they don't listen, they impose their decisions, don't notify communities, don't do this or that etc.). Needless to say, I'll gladly stand up for this affair, and I'm not affiliated with WMF :-)
  • Link T212268 too; specify that the user can be blocked, desysopped and anything else, but it'll continue to do what it's instructed to do.
  • Even if the wiki has block disabled, it doesn't mean that they don't need this user. First, because it could be used for maintance purposes like here. Second, because it's also used for degrouping and blocking autopromotion. The latter isn't currently logged, but it will after T49412.
  • Maybe explain that system accounts cannot be used by normal people because their token, email etc. are automatically invalidated.

Ah, I just realized that the next T/N issue will be on Jan 7th. Not the best period for "bad" things to happen :/ On the other hand, hopefully we have a couple of days more to decide what to do here, and what to announce.

@Daimona, have you decided on what to do and announce? T/N on Jan 7th means it is locked and ready for translation on Jan 4th.

@Trizek-WMF I totally forgot that we didn't have a message ready, nor I realized that the next T/N is almost here. I think we should just include as much info as possible without being too verbose. My proposal is to use two separate messages:

In abuse filters, the "{{int:abusefilter-action-throttle}}" action takes three parameters: count, period and groups. Previously, people could freely insert everything in those fields. Now we require these parameters to be valid, i.e.: *not be empty; *count and period must be positive integers; * groups must be a combination of valid groups (listed [[:mw:Extension:AbuseFilter/Actions#Throttling|here]]), separated with commas or newlines depending on the effect you want to achieve, as explained in the link above. We now want to fix all filters in WMF to make them valid, but unfortunately many of them cannot be edited automatically and must be manually checked. You may find a list of such filters [[:phab:P7956|here]]; if you're familiar with AbuseFilter, please take a look and fix them. [[:phab:T209565|]]

On December 17h, in order to perform some technical maintenance on AbuseFilter per point above, a new account with sysop rights and the localized name "{{int:abusefilter-blocker}}" has been created on several wikis. On wikis where AbuseFilter can perform blocks, this user already existed and is automatically used to block people. For other wikis, the account will only do maintenance tasks if required, so there's no reason to be worried: no real person will use that account, ever. The account creation wasn't expected to happen, and due to holidays there have been no occasions to announce this in Tech/News. See [[:phab:T212268|]] for future plans.

Trizek-WMF added a comment.EditedFri, Jan 4, 1:40 PM

@Daimona, I'm sorry but I don't understood your texts. :/

You have a list (P7956) of filters that are not formatted correctly, right? Your goal is to have those filters fixed by someone? If so, that's targeting a few wikis you should contact directly for a better impact. Tech News is too global for that, except if you want everyone to be aware of the new formatting you are working on (again, IIUC).

And what about T212268? What is the future? What should be added to Tech News?

@Trizek-WMF For the moment, I added the above proposals to T/N, of course I didn't mark them for translation as they have to be improved. To answer your questions:

  • Yes, the listed filters contain invalid data and thus don't work (as expected). We cannot fix them automatically and so yes, we're asking if someone could fix them - unless we find a way to do it automatically. Looking again, the list is pretty short and I could go to each wiki and explain the problem. Still, I think we should make everyone aware of the new validation requirements (i.e., just remove the last sentence).
  • As for T212268, future plans are to make the AbuseFilter user not be a sysop (and in general create a new user group for system users, which is T212720). We should probably change that last line to read something like

Given that several communities have reported concerns about system users being sysop (mostly because we lack a reliable way to identify them), we're working on making the AbuseFilter user not be a sysop anymore, and to add a dedicated user group for other system users. [https://phabricator.wikimedia.org/T212268] [https://phabricator.wikimedia.org/T212720]

The texts you have inserted are too long, I had to cut them a bit. Please review them ASAP, since I'm wrapping up the current issue: I plan to freeze it in 2 hours starting now.

I have shorten the first item to rely on the documentation. But I don't see any of the 3 required elements in it. Can you fix it?

@Trizek-WMF Thanks, the text was meant to be amended. I updated the docs to reflect the added validation, and reviewed your changes. I think they're fine, aside from these minor things:

  • The AbuseFilter account already existed on several wikis. The first message is for wikis where it was created on Dec 17th, to let people know that there's no reason to be worried because it's not a normal account. So I'd change it to:

On several wikis, an account named "<tvar|accountname>{{int:abusefilter-blocker}} </>" has been created on December 17th to perform some technical maintenance on AbuseFilter. This account has sysop rights but it's a system user and no human can use it. The account already existed on wikis where AbuseFilter can perform blocks, which are issued using this account. See [[<tvar|T212268>:phabricator:T212268</>|T212268]] for more information and future plans.

which has roughly the same length but focuses on different aspects (feel free to further amend it, of course).

  • The other message is OK, aside from a minor rewording:

They now require parameters as listed [[:mw:Extension:AbuseFilter/Actions#Throttling|on mediawiki.org]]

should be

They must now fulfill the requirements listed [[:mw:Extension:AbuseFilter/Actions#Throttling|on mediawiki.org]]

Change done, Thank you!
I've just changed "fulfill" by "strictly respect", which is less difficult to understand for non-native speakers (and translators).

Back to the core of this task, I think that any attempt to cover more cases would only end up with imposing a default. Thus, I think we should:

  1. (Merge the missing patch)
  2. Run the script without --dry-run
  3. Change the script so that, if it finds unfixable filters, it'll still change the other ones but report the faulty ones (and don't update the updatelog table, so that further executions will still report the errors).

Thoughts?

Change 480706 abandoned by Daimona Eaytoy:
Re-fix the throttle script

Reason:
All wikis to wmf.9

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

Actually, there's another improvement we can do: change groups to lowercase. Throttle groups are case sensitive and must be lowercase, but some filters user them with uppercase letters (see mrwiki for instance). The patch above covers this case and maps old groups to lowercase.

greg lowered the priority of this task from Unbreak Now! to High.Fri, Jan 11, 11:30 PM
greg added a subscriber: greg.

Raising priority to UBN because parent task is UBN, and this also requires some extra work (although 1. and 2. are easy). I'll try to send a new patch today/tomorrow; @tstarling any chance you'll review this again?

Lowering to High, this is not currently an UBN! issue.

Lowering to High, this is not currently an UBN! issue.

Hm, probably true. Summing up:

  • Any new filter is fine
  • Editing for the first time a filter last edited before the original offending patch will cause no troubles (specifically, throttle params won't be removed as it used to happen)
  • Filters with invalid parameters are still affected; for those, the behaviour is unclear (depends on the single filter: it could vary between "never take any action" and "completely ignore throttle") and they're responsible for several errors seen in production. Mostly, T203535 and another undefined index on AbuseFilterViewEdit.

So probably not UBN!, although defo not below high.

He7d3r added a subscriber: He7d3r.Sat, Jan 12, 10:39 PM