Last modified: 2014-04-04 20:54:53 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T44734, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 42734 - Non-admins can see contents of deleted pages when viewing abusefilter details
Non-admins can see contents of deleted pages when viewing abusefilter details
Status: ASSIGNED
Product: MediaWiki extensions
Classification: Unclassified
AbuseFilter (Other open bugs)
unspecified
All All
: Normal enhancement (vote)
: ---
Assigned To: Marius Hoch
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-05 19:38 UTC by Kunal Mehta (Legoktm)
Modified: 2014-04-04 20:54 UTC (History)
8 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Kunal Mehta (Legoktm) 2012-12-05 19:38:58 UTC
[[Healthy Children Project's Center for Breastfeeding]] was deleted on enwiki, however I can still see parts of the edit history (and therefore, article content) by looking at specific abuse filter log entries like [[Special:AbuseLog/7900305]] even though I am a non-sysop, and shouldn't be able to see deleted pages.

I'm marking this as major since non-sysops shouldn't be able to see deleted pages.
Comment 1 Kunal Mehta (Legoktm) 2012-12-05 19:40:26 UTC
First link is broken, try https://en.wikipedia.org/wiki/Healthy_Children_Project%27s_Center_for_Breastfeeding
Comment 2 Andre Klapper 2012-12-07 11:34:36 UTC
Confirming.
Comment 3 Chris Steipp 2012-12-07 14:14:02 UTC
We can definitely hide the data when the article has been deleted, as was done in this case. We store the article title in the AbuseFilter log, so we can check if it's been deleted.

However, if a public rule is triggered for a particular revision that later is deleted, AbuseFilter doesn't keep around enough information to prevent showing this.

A workaround is to restrict the abusefilter-log-detail permission. By default, it's given to *.
Comment 4 Kunal Mehta (Legoktm) 2012-12-07 14:34:08 UTC
(In reply to comment #3)
> We can definitely hide the data when the article has been deleted, as was
> done in this case. 

What was hidden in this case? I'm pretty sure I can see everything.

> We store the article title in the AbuseFilter log, so we can
> check if it's been deleted.
> 
> However, if a public rule is triggered for a particular revision that later
> is
> deleted, AbuseFilter doesn't keep around enough information to prevent
> showing
> this.

I'm not following you. Can't it just check if the title exists and choose to show the detailed information based on that?

> A workaround is to restrict the abusefilter-log-detail permission. By
> default,
> it's given to *.

I'm not sure if restricting abusefilter-log-detail is the right solution. There are non-sysops who have the abusefilter-modify right through the EFM group who technically shouldn't be able to see deleted content, but would still be able to.

And if we're still tracking harmless things like WikiLove ([[Special:AbuseFilter/423]]) in the EF, it makes no sense to hide that from typical users.
Comment 5 Alex Monk 2012-12-07 16:24:21 UTC
(In reply to comment #4)
> > We store the article title in the AbuseFilter log, so we can
> > check if it's been deleted.
> > 
> > However, if a public rule is triggered for a particular revision that later
> > is
> > deleted, AbuseFilter doesn't keep around enough information to prevent
> > showing
> > this.
> 
> I'm not following you. Can't it just check if the title exists and choose to
> show the detailed information based on that?

A page would be considered existing if it has at least one revision which is not deleted. This means that if you:

Create a page
Edit it, edit hits a filter and gets logged
Delete the page - the details of your edits should now be hidden in AF
Someone else creates the page with the same title

Suddenly, the AF entry is visible again because the title exists.
Comment 6 MZMcBride 2012-12-08 03:36:22 UTC
(In reply to comment #5)
> A page would be considered existing if it has at least one revision which is
> not deleted. This means that if you:
> 
> Create a page
> Edit it, edit hits a filter and gets logged
> Delete the page - the details of your edits should now be hidden in AF
> Someone else creates the page with the same title
> 
> Suddenly, the AF entry is visible again because the title exists.

I'm not sure this edge case is very important.
Comment 7 Chris Steipp 2012-12-11 23:19:07 UTC
I'm adding this to our backlog of Admin Tools, but just to flesh out the requirements:

* The AbuseLog entry should be hidden in cases where we have a legal obligation to remove the content. So as Krenair pointed out, we can't just not show it if the page doesn't exist, otherwise anyone could create the page to read the content.

* Is it really desired by most of the admins to have these log entries hidden whenever a page is deleted, even when, for example, it's just advertising vs copyright violation? Or would it only be for pages that are deleted for specific reasons?

* On the overall list of priorities, is this something admins are desperate for? Or more of a nice to have right now?
Comment 8 Kunal Mehta (Legoktm) 2012-12-15 19:47:06 UTC
(In reply to comment #7)
> 
> * The AbuseLog entry should be hidden in cases where we have a legal
> obligation
> to remove the content. So as Krenair pointed out, we can't just not show it
> if
> the page doesn't exist, otherwise anyone could create the page to read the
> content.

True, ideally the log entry would be undeleted when the page is undeleted.

> * Is it really desired by most of the admins to have these log entries hidden
> whenever a page is deleted, even when, for example, it's just advertising vs
> copyright violation? Or would it only be for pages that are deleted for
> specific reasons?

I think its more important for say, copyvios and major BLP violations to be removed rather than things that were deleted in AFD for being [[WP:NOT]] (out of scope). Maybe just any variables which contain page text (like old_wikitext, the diff, etc) should be removed from the details & examine pages so the log entry remains, similar to how if a page is deleted, any entries in [[Special:Log/move]] aren't.

> * On the overall list of priorities, is this something admins are desperate
> for? Or more of a nice to have right now?

Given that this bug has existed ever since the AbuseFilter was introduced (2 years), I don't think its that urgent since no one ever noticed it until I did.

As in interim solution, it would be nice to be able to delete certain log entries (possibly a separate bug in itself).
Comment 9 Chris Steipp 2012-12-17 18:04:21 UTC
> As in interim solution, it would be nice to be able to delete certain log
> entries (possibly a separate bug in itself).

Users with the 'abusefilter-hide-log' privilege can do this. Although now that we automatically hide a revision's logs when the revision is deleted, I'm not sure if admins are in the habit of deleting the AF log when an entire page is deleted.

We may be able to hook into the UI to make hiding the AF logs easy when someone is deleting a page. I'm going to mark this as a lower priority enhancement, since it sounds like it would be useful, but the overall result is possible in the existing tools (unless I'm missing something).

Actually, I'm pretty sure this could be implemented as a pretty simple gadget, which might be a good way to try out the requirements.
Comment 10 Marius Hoch 2012-12-29 01:50:01 UTC
I just wrote a patch which auto deletes all log entries for a given page by the time it's deleted. I'm not sure anymore this is the best way to go (as only Oversighters can see them then)... any ideas?

Alternatives would be to implement a automated deletion with user confirmation or adding a page_id field to the abuse log table.
Comment 11 Kunal Mehta (Legoktm) 2013-01-21 02:35:48 UTC
(In reply to comment #10)
> I just wrote a patch which auto deletes all log entries for a given page by
> the
> time it's deleted. I'm not sure anymore this is the best way to go (as only
> Oversighters can see them then)... any ideas?

I don't think giving non-oversight'ers a way to oversight log entries is a good idea. AF should probably have a way to "revdel" log entries to the "viewdeleted" level, and then a further level of "suppressed", making it equivalent to normal log entries. Maybe a separate bug?

> Alternatives would be to implement a automated deletion with user
> confirmation
> or adding a page_id field to the abuse log table.

I'm not too familiar if/how page_id's change when deleting and undeleting, but as long as it provided an way to delete them and then undelete them it would work.
Comment 12 Chris Steipp 2013-02-22 17:59:32 UTC
Would it be common for a page that needs to be deleted and suppress to be moved around first, so that we can't match the page's title in the abusefilter logs?

Again, if we're talking about a single revision for a page (excluding the very first revision when the page is created), suppressing the revision will also suppress the AbuseFilter logs automatically. So this is only when the initial edit needs to be suppressed (since AF logs it without a revision_id, since no revision exists at the time that we're filtering), or the page title needs suppression.

How about inserting a link on the delete confirmation page (using the hook), that links to the abuse filter logs for that page, if any abusefilter logs exist for that title and the user has suppression rights? That would let someone who is suppressing data know that they need to also check the logs and see if any entries need suppression as well. But the interface would remain unchanged for most users.
Comment 13 Marius Hoch 2013-02-22 18:52:04 UTC
(In reply to comment #12)
> Would it be common for a page that needs to be deleted and suppress to be
> moved
> around first, so that we can't match the page's title in the abusefilter
> logs?
No

> Again, if we're talking about a single revision for a page (excluding the
> very
> first revision when the page is created), suppressing the revision will also
> suppress the AbuseFilter logs automatically. So this is only when the initial
> edit needs to be suppressed (since AF logs it without a revision_id, since no
> revision exists at the time that we're filtering), or the page title needs
> suppression.
> 
> How about inserting a link on the delete confirmation page (using the hook),
> that links to the abuse filter logs for that page, if any abusefilter logs
> exist for that title and the user has suppression rights? That would let
> someone who is suppressing data know that they need to also check the logs
> and
> see if any entries need suppression as well. But the interface would remain
> unchanged for most users.
That would come with a different flavor of the problem I mentioned above: Only oversighters can hide and unhide log entries (at least on WMF wikis) as we only have one hidden level. If I recall it right it's not trivial to introduce more than one... I thought about this bug (and even hacked up some stuff) in December and wasn't able to come up with a perfect solution (see above)
Comment 14 Andre Klapper 2014-02-28 15:04:04 UTC
Marius Hoch: You assigned this issue to yourself 14 months ago. 
Could you please provide a status update and inform us whether you are still working (or still plan to work) on this issue? 
Only in case you do not plan to work on this issue anymore, should the assignee be set back to default and the bug status changed from ASSIGNED to NEW/UNCONFIRMED? Thanks.
Comment 15 Marius Hoch 2014-02-28 15:07:24 UTC
(In reply to Andre Klapper from comment #14)
> Marius Hoch: You assigned this issue to yourself 14 months ago. 
> Could you please provide a status update and inform us whether you are still
> working (or still plan to work) on this issue? 
> Only in case you do not plan to work on this issue anymore, should the
> assignee be set back to default and the bug status changed from ASSIGNED to
> NEW/UNCONFIRMED? Thanks.

Well, I (hopefully) still have the patch mentioned in #c10 around (and can rebase it to work with AbuseFilter master). But I want some more feedback about this... if this feedback is given, I guess I can work on this (again).
Comment 16 Chris Steipp 2014-03-03 18:49:14 UTC
Did we pull out the automatic hiding of deleted revisions? I'm sure I saw that, but I'm not seeing it right now. That would be along the same lines, where a non oversighter was essentially causing a suppression event. But if we don't have that, then hoo's patch probably is doing that, and seems wrong.

I think we could introduce delete and suppress for the log-- that seems like it's going to be needed in the long run, and would make this work.
Comment 17 Marius Hoch 2014-03-03 19:19:09 UTC
(In reply to Chris Steipp from comment #16)
> I think we could introduce delete and suppress for the log-- that seems like
> it's going to be needed in the long run, and would make this work.

I initially thought this would require a schema change, but that's not true:
        afl_deleted tinyint(1) NOT NULL DEFAULT 0,

If we have that, we can probably use it together with a slightly modified version of my patch mentioned above.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links