I'm guessing it was https://github.com/wikimedia/mediawiki/commit/97a8fb78faa4ca2d9604140e5547f5deae5a2524 / possibly https://github.com/wikimedia/mediawiki/commit/51be1c92de17b7906a6610fb0a69a9a000d5a4ef

https://gerrit.wikimedia.org/r/#/c/352863

Pulling this in to the security zone because the attack vector it exposes.

Is this a regression?

@Jalexander: the workaround is to suppress the log event manually, no? Doesn't seem worth keeping private accordingly unless I am misunderstanding...

In T171405#3464081, @lfaraone wrote:

@Jalexander: the workaround is to suppress the log event manually, no? Doesn't seem worth keeping private accordingly unless I am misunderstanding..

Well. The workaround is complicated and fraught with opportunity for error, because it requires (a) suppressing all of the revisions of the page after deleting it and (b) hunting down and suppressing the log event. Both of these increase the complexity significantly. It is also difficult to ascertain that all of the instances of the title are suppressed that way - the previous iteration was supposed to do that, and I am not 100% certain that happens with suppressing the deletion log entry.

Should we be treating this as unbreak now then?

@jrbs wrt. regression. Seems so.

@lfaraone I'd say to keep it private until fixed given the potential abuse vector this could be. I'm quoting myself from the dupe task to signal the vulnerabilities I've identified:

After the latest MediaWiki update (or so it seems as the deletion form design has changed) the option to directly suppress pages have dissapeared. This doubles oversighters' work since then they'll have to suppress individual deletion log entries (leaving traces in logs and leaks of private data for those parsing IRC feeds, EventStreams, etc. where you cannot remove them afterwards) and revisions of the suppressed title so they ain't viewable to administrators either. Please restore the functionality to directly suppress (oversight) a page when deleting it. Thanks.

Although yes, the page title once it was created appeared on IRC/EventStreams.

@Legoktm UBN seems appropriate to me.

In T171405#3464081, @lfaraone wrote:

@Jalexander: the workaround is to suppress the log event manually, no? Doesn't seem worth keeping private accordingly unless I am misunderstanding...

Yeah, there are workarounds but they're not guaranteed and requires multiple steps. While I'll have no problem pulling it public again once fixed but until then I think it's better to be safe since it is easy to miss something or not realize you need to do the workaround especially on smaller wikis where it's likely someone will point out the error.

In T171405#3464134, @Legoktm wrote:

Should we be treating this as unbreak now then?

I know already answered/done but for the record, yes, I think so. Thanks for working on the patch

I can't test right now but code looks good. Issues/thoughts:

• The change of getVal() to getCheck() at the beginning looks unrelated to the rest of the changes?
• PHPCS complains about the double newline at the end of the patch.

In T171405#3464228, @matmarex wrote:

I can't test right now but code looks good. Issues/thoughts:

• The change of getVal() to getCheck() at the beginning looks unrelated to the rest of the changes?

The checkbox is being posted as &wpSuppress= so it has no value, meaning getVal() is falsey. So I switched it to getCheck().

• PHPCS complains about the double newline at the end of the patch.

Fixed.

[16:04:56] <MatmaRex> legoktm: but ok, either way works. fine by me. ship it

Deploying it now...

Thanks! I confirm it works as well.

Change 367339 merged by jenkins-bot:
[mediawiki/core@master] [SECURITY] Restore ability to suppress pages while deleting

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