Page MenuHomePhabricator

Applying deletelogentry restrictions to flow deletion log entries does not work
Closed, ResolvedPublic

Description

I went to handle https://meta.wikimedia.org/w/index.php?oldid=15684303#gom.wikipedia. While I could delete the flow-topic content w/o the need to suppress (a process I must say a bit messy, since you have to delete both the topic and the contents of such topic), I went to apply deletelogentry restrictions to the deletion logs created by deleting the flow topic. The result is what you can see here and here at large: whilst the deletion logs says that I've successfully applied visibility restrictions to such log entries, that ain't true. This poses potential risk, as it might not be possible to redact log entries containing topics from flow-titles which reveals any sort of innapropriate data. To be sure, I've tried to check those logs unlogged-in, and while the contents gets hidden in the RecentChanges page, if one accesses the deletion log such contents are still visible there. Thank you.

Event Timeline

Restricted Application added subscribers: Zppix, JEumerus, Aklapper. · View Herald TranscriptJun 8 2016, 11:29 AM

I'm not sure why deletelogentry is applying differently, but if you suppress the topic, the topic title will be hidden, and the suppression action will only show in Special:Log/suppress (only suppressors can view this log).

Protecting at least until the topic is dealt with.

Actually, never mind, since the topic has already been linked to from MediaWiki.org.

I'm not sure why deletelogentry is applying differently, but if you suppress the topic, the topic title will be hidden, and the suppression action will only show in Special:Log/suppress (only suppressors can view this log).

Yes, but if the title of the topic is abusive, it seems there's no way to hide that from the deletion log, which is what's happening here.

Quiddity triaged this task as High priority.
Quiddity added subscribers: Catrope, Quiddity, Trizek-WMF.
Mattflaschen-WMF claimed this task.EditedJun 9 2016, 10:09 PM

Yes, but if the title of the topic is abusive, it seems there's no way to hide that from the deletion log, which is what's happening here.

Yes. If you suppress it originally it works fine (it only goes into the suppress log, which is restricted), but there is a problem if it first deleted, then later suppressed.

Looking into that now.

We can solve this for suppression by checking the permission of the current topic. However, for LqtImportFormatter it may be better to make sure deletelogentry works (since that doesn't currently load the Flow collections, it just uses lqt_subject).

Mattflaschen-WMF set Security to Software security bug.Jun 10 2016, 1:29 AM
Mattflaschen-WMF added a project: Security.
Mattflaschen-WMF changed the visibility from "Public (No Login Required)" to "Custom Policy".
This comment was removed by MarcoAurelio.

This is ready for review, and can be deployed while I'm finishing T137593: Topics that are deleted then suppressed expose topic title in public deletion log.

@matthiasmullie said he will review today or tomorrow, but that it's fine if someone else wants to review it.

Patch looks fine.

I'll deploy this early Monday unless someone wants to make a case that it's bad enough to make an exception to the Friday policy.

I didn't get that out today. I'll do it at the hackathon.

dpatrick added a subscriber: dpatrick.EditedJun 24 2016, 9:05 PM

@Catrope deployed this on 2016-06-23:

19:24 greg-g: 19:21 < RoanKatto>  !log Synced patches for T137288 and T137593
dpatrick closed this task as Resolved.Jul 18 2016, 10:49 PM
Restricted Application removed a subscriber: Zppix. · View Herald TranscriptJul 18 2016, 10:50 PM

This has been merged to master as https://gerrit.wikimedia.org/r/#/c/299863/ .

Can this task be made public?

Yes, this can be made public.

demon changed the visibility from "Custom Policy" to "Public (No Login Required)".Aug 10 2016, 9:13 PM
demon changed Security from Software security bug to None.
Restricted Application added a subscriber: Malyacko. · View Herald TranscriptAug 10 2016, 9:13 PM