Ah, I see, the new config change... I thought core had the 'suppress' group, but indeed it's commented out. I don't get it, though. wmf-config's InitialiseSettings assigns suppression-related right to the oversight group, which is totally fine. But they're not assigned to anyone in core. I don't understand the reason for this choice. Maybe core should assign those rights to the 'suppress' group, and then it should be removed from $wgGroupPermissions in wmf settings. But right now, there's no way for extensions to add a right to the suppress/oversight group. [If assigned to 'suppress', it will create a new group; if assigned to 'oversight', that's not a group "officially" defined in core].

I should repeat, the 'abusefilter-hide-log' and 'abusefilter-hidden-log' permissions are included into the (new) group 'suppress' group on WMF wikis. See ListGroupRights to see an entry named 'Suppressors' (this is about the new group).

In T230601#5417721, @Vlad5250 wrote:

I should repeat, the 'abusefilter-hide-log' and 'abusefilter-hidden-log' permissions are included into the (new) group 'suppress' group on WMF wikis. See ListGroupRights to see an entry named 'Suppressors' (this is about the new group).

Yes, thanks, I got it. I said it's because of a recent config change, and because core does weird things with the 'suppress' group.

In T230601#5417732, @Daimona wrote:

In T230601#5417721, @Vlad5250 wrote:

I should repeat, the 'abusefilter-hide-log' and 'abusefilter-hidden-log' permissions are included into the (new) group 'suppress' group on WMF wikis. See ListGroupRights to see an entry named 'Suppressors' (this is about the new group).

Yes, thanks, I got it. I said it's because of a recent config change, and because core does weird things with the 'suppress' group.

If wgGroupPermissions adds a group which isn;t defined in core, the group will be created with permissions assigned to that group.

In T230601#5417753, @Vlad5250 wrote:

In T230601#5417732, @Daimona wrote:

In T230601#5417721, @Vlad5250 wrote:

I should repeat, the 'abusefilter-hide-log' and 'abusefilter-hidden-log' permissions are included into the (new) group 'suppress' group on WMF wikis. See ListGroupRights to see an entry named 'Suppressors' (this is about the new group).

Yes, thanks, I got it. I said it's because of a recent config change, and because core does weird things with the 'suppress' group.

If wgGroupPermissions adds a group which isn;t defined in core, the group will be created with permissions assigned to that group.

Yes, I know. Let me reword my first comment:

• Core has commented assignments for a "suppress" group (here), thus giving the impression that this is the "official" group name
• Core doesn't assign suppression-related permissions (e.g. hideuser or suppressrevision) to any group, AFAICS. They're only *granted* as part of "oversight"
• WMF-config's InitialiseSettings assign these rights to its own "oversight" group (here)
• No "oversight" group is defined in core

Hence, if an extension wants to give a right to people who can suppress stuff (from extension.json), they could either:

1. Give the right to "oversight". But "oversight" is not a group defined in core, and thus the naming would be too WMF-specific and could introduce a new group in non-WMF wikis;
2. Give it to the "suppress" group. This way we get the problem that you reported, i.e. a new group on WMF wikis.

And consequently, the possible solutions I see are, respectively:

1. Change core to have a builtin "oversight" group, so that we don't need to recreate it for WMF, and extensions can then add rights to it
2. Change WMF's InitializeSettings to read core's "suppress" group, copy it in an "oversight" group, and unset the "suppress" group. The core assignments can then be uncommented. This will also handle extensions adding rights to "suppress".

I hope it's clearer now.

I'd be for enabling the suppress group in core, but I don't know why it isn't. Then we could reassign all rights from that group to the oversight group WMF config until T112147: Rename the oversight group on WMF projects is done.

If we can't do that or don't want to delay resolving the issue, I think we should either

1. not assign the rights by default in AF and add them to oversight in WMF config
   • The second part is already done.
2. reassign any suppress group rights (from extensions) to the oversight group in WMF config until T112147 is done
   • This would be better for other extensions that want to assign rights to the group, e.g. Flow.

Change 530612 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[operations/mediawiki-config@master] Assign all rights assigned to suppress group to oversight group

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

In T230601#5418442, @gerritbot wrote:

Change 530612 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[operations/mediawiki-config@master] Assign all rights assigned to suppress group to oversight group

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

Let's re-assign all rights that somehow appears assigned to suppress user group to oversight user group, so we don't have things broken/separated on our wikis.

Yeah, looks good as a short-term solution. Which can also be made long-term, simply by uncommenting those three lines in core, and updating the comment added by this patch.

Of note, I checked the blame to see when those lines were commented. And surprise, they're commented since their introduction back in ~2008 with rMWd04da1d5c872a408d4f7a7272dff6781075c728f. I'm unsure who could have knowledge about why those lines are commented. CC' @aaron as author of the patch (although it was 11 years ago...), and Platform Engineering + @Tgr per mw:Developers/Maintainers.

The naming inconsistency is probably because "suppress" makes more sense as the default name of the group, but Wikimedia sites already had a bunch of users in a group named "oversight" for historical reasons.

I'd guess they may have been commented out for the reason given in a comment that was removed in that commit: "Experimental permissions, not ready for production use". That was 1.13, while it looks like RevisionDeletion wasn't actually "released" until 1.16.

In T230601#5418575, @Anomie wrote:

The naming inconsistency is probably because "suppress" makes more sense as the default name of the group, but Wikimedia sites already had a bunch of users in a group named "oversight" for historical reasons.

Makes sense.

I'd guess they may have been commented out for the reason given in a comment that was removed in that commit: "Experimental permissions, not ready for production use". That was 1.13, while it looks like RevisionDeletion wasn't actually "released" until 1.16.

I didn't even notice that comment.

So I guess there's no harm in enabling those settings in core? Our config won't be affected, as long as @Urbanecm's patch is merged.

Change 530702 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/core@master] Actually assign suppression-related rights to 'suppress' group

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

Change 530612 merged by jenkins-bot:
[operations/mediawiki-config@master] Assign all rights assigned to suppress group to oversight group

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

Mentioned in SAL (#wikimedia-operations) [2019-08-19T18:26:57Z] <urbanecm@deploy1001> Synchronized wmf-config/CommonSettings.php: SWAT: 0a87e3c: Assign all rights assigned to suppress group to oversight group (T230601) (duration: 00m 48s)

Ah, thanks @Daimona.

I still see the suppress group on the wikis.

Change 531147 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[operations/mediawiki-config@master] Assign all rights assigned to suppress group to oversight group

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

Okay... By live testing, it seems moving to abusefilter.php helps. Not sure why, through... @Daimona, any thoughts?

Well, so it's easy to understand the problem here. When reaching CommonSettings, AF is already loaded. But its extension.json still has to be read, and this only happens in Setup.php L135, while CommonSettings is loaded earlier, at line 62. Which means that $wgGroupPermissions['suppress'] is still empty in CommonSettings, and unsetting it doesn't work.

It would work for permissions defined in core (i.e. if my patch is merged), so that's unblocked at least. But I don't think we should be forced to repeat your patch for every extension that wants to assign rights to the suppress group.

Now I don't know what we'd better do. I guess there are valid reasons for the load order to be like that, so we cannot change it. An alternative would be to use the 'oversight' group in core, so that extensions will have to add rights to it instead of 'suppress', which, in turn, would disappear completely. Note that in doing so, we'd face the same problem again for other groups having a name in core, and another in WMF config.

Of note, I'm not getting why it's working from abusefilter.php. All of the wmf-config files are loaded together, aren't they?

I guess there are valid reasons for the load order to be like that, so we cannot change it.

I think the reason is mainly that it's a huge complex mess and poking at it tends to break things.
Also at some point we tried to introduce a completely static config loading mechanism via extension registry; IMO that failed thoroughly (in that we have lots of non-static parts and no plans to ever change that) but that has never been recognized in the code (e.g. by providing a hook for dynamically changing settings that's guaranteed to run before those settings would be used).

We use extension functions for other things (see line 3759), although not for fully unsetting groups, but it's worth a try.

Of note, I'm not getting why it's working from abusefilter.php. All of the wmf-config files are loaded together, aren't they?

It's loaded on line 2142 (so a bit sooner than the code that didn't work). So in theory it should be even less likely to work. Could it be an error in live testing?

In T230601#5427857, @Tgr wrote:

Of note, I'm not getting why it's working from abusefilter.php. All of the wmf-config files are loaded together, aren't they?

It's loaded on line 2142 (so a bit sooner than the code that didn't work). So in theory it should be even less likely to work. Could it be an error in live testing?

Well everything's possible. Let me try again.

Mentioned in SAL (#wikimedia-operations) [2019-08-21T10:49:35Z] <Urbanecm> Wrapped code added to CommonSettings.php in T230601 to wgExtensionFunctions

Mentioned in SAL (#wikimedia-operations) [2019-08-21T10:52:43Z] <Urbanecm> Move 0a87e3c's code to abusefilter.php on mwdebug1002 (T230601)

Mentioned in SAL (#wikimedia-operations) [2019-08-21T10:57:05Z] <Urbanecm> Run scap pull on mwdebug1002 (T230601)

Okay, so now it didn't seem to work at all. I'll have a look again soon.

Hey, before I look further, does this change have consensus on the wikis? E.g. from stewards or something like that?

In T230601#5428835, @Krinkle wrote:

Hey, before I look further, does this change have consensus on the wikis? E.g. from stewards or something like that?

There shouldn't be any difference for WMF config.

In T230601#5428835, @Krinkle wrote:

Hey, before I look further, does this change have consensus on the wikis? E.g. from stewards or something like that?

We don't need a consensus to fix a bug. Currently, group named "oversight" is used. Hence, group "suppressor" which appeared on WM wikis shouldn't exist at all. That's a bug, not a change.

Change 531147 abandoned by Urbanecm:
Assign all rights assigned to suppress group to oversight group

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

In T230601#5429272, @Urbanecm wrote:

In T230601#5428835, @Krinkle wrote:

Hey, before I look further, does this change have consensus on the wikis? E.g. from stewards or something like that?

We don't need a consensus to fix a bug. Currently, group named "oversight" is used. Hence, group "suppressor" which appeared on WM wikis shouldn't exist at all. That's a bug, not a change.

I disagree, the Wikimedia wikis' config is anomalous and ideally would be corrected, but there's no appetite to do that right now, apparently.

In T230601#5447177, @Jdforrester-WMF wrote:

I disagree, the Wikimedia wikis' config is anomalous and ideally would be corrected, but there's no appetite to do that right now, apparently.

It is, but before we decide to change the Wikimedia config, it is a bug, since its behaviour is different from expected behaviour :-).

Change 533211 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[operations/mediawiki-config@master] Fix "Assign all rights assigned to suppress group to oversight group"

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

Change 533211 merged by jenkins-bot:
[operations/mediawiki-config@master] Fix "Assign all rights assigned to suppress group to oversight group"

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

In T230601#5452284, @gerritbot wrote:

Change 533211 merged by jenkins-bot:
[operations/mediawiki-config@master] Fix "Assign all rights assigned to suppress group to oversight group"

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

And yet the problem is still live... https://www.mediawiki.org/wiki/Special:ListGroupRights, https://en.wikipedia.org/wiki/Special:ListGroupRights both still show a suppressor group

Change 533288 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[operations/mediawiki-config@master] Follow up for e70da21

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

Mentioned in SAL (#wikimedia-operations) [2019-08-29T18:22:48Z] <urbanecm@deploy1001> Synchronized wmf-config/CommonSettings.php: SWAT: Fix "Assign all rights assigned to suppress group to oversight group" (T230601) (duration: 00m 54s)

Change 533288 merged by jenkins-bot:
[operations/mediawiki-config@master] Follow up for e70da21

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

In T230601#5452295, @DannyS712 wrote:

In T230601#5452284, @gerritbot wrote:

Change 533211 merged by jenkins-bot:
[operations/mediawiki-config@master] Fix "Assign all rights assigned to suppress group to oversight group"

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

And yet the problem is still live... https://www.mediawiki.org/wiki/Special:ListGroupRights, https://en.wikipedia.org/wiki/Special:ListGroupRights both still show a suppressor group

There are two steps that happens when config is to be deployed. 1) patch is merged 2) a file with the change is synchronized across the cluster. Those steps usually happen closely to each other, and usually in the order I wrote, but doesn't need to. The merging step is there to have the change in master, at which point I can git pull at the deployment machine. But I can also change stuff directly at the deployment machine. I don't do that often, but when a patch doesn't work, I change stuff at deployment machine directly and test, to detect where the issue is, and upload&merge after that. To save some time, I often sync before the commit is merged, at which point the patch is merged after it is deployed.

TLDR: The important piece to watch for when deciding if something should be fixed is the SAL entry posted by @Stashbot, not "merged" comment posted by @gerritbot.

@Urbanecm thanks for explaining

In T230601#5452355, @Daimona wrote:

Fixed now.

Not opposing closing, but see below.

In T230601#5422806, @Daimona wrote:

There's still T230601#5419718 open.

Still true?

Change 530702 merged by jenkins-bot:
[mediawiki/core@master] Actually assign suppression-related rights to 'suppress' group

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