Last modified: 2014-10-12 10:52:40 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 T30633, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 28633 - AbuseFilter should respect revdel flags
AbuseFilter should respect revdel flags
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
AbuseFilter (Other open bugs)
unspecified
All All
: High major with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
triage
: platformeng
Depends on:
Blocks: SWMT
  Show dependency treegraph
 
Reported: 2011-04-21 01:01 UTC by John Mark Vandenberg
Modified: 2014-10-12 10:52 UTC (History)
13 users (show)

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


Attachments

Description John Mark Vandenberg 2011-04-21 01:01:10 UTC
Once a revision has been hidden from public view, the associated abusefilter log entries should also be redacted so that the revision/log details can not be seen without the appropriate permissions.

The AbuseFilter has its own 'visibility' system which isn't very flexible and it may not be necessary once the extension respects the revdel flags associated with the revision and log entries.
Comment 1 Mark A. Hershberger 2011-04-21 20:31:33 UTC
Is this related to why I can't see the contents of the page you
mention in Bug #28632 even though you have undeleted it?
Comment 2 John Mark Vandenberg 2011-04-21 21:08:57 UTC
(In reply to comment #1)
> Is this related to why I can't see the contents of the page you
> mention in Bug #28632 even though you have undeleted it?

No.
Comment 3 Thehelpfulone 2011-05-19 20:57:03 UTC
I just noticed this bug, was going to post a new one - but:


Basically, I would like to be able to have a feature for admins to be able to effectively revdelete abuse filter logs, just as Oversights can suppress them thanks to the bug being fixed.

There's some filters that are more likely to have the need for suppression, and whilst they will eventually need supression by an oversighter, and admin could easily help out temporarily to hide it.

I don't know how easy/difficult this would be to implement.

Thanks,

Thehelpfulone
Comment 4 Andrew Garrett 2011-05-21 14:44:14 UTC
For now this is probably in the "patches welcome" category.

What would be necessary would be to hook the "revision deleted" event, and, when that occurs, find the filter log entries with the same title, user and timestamp, and set their afl_deleted flags to the appropriate value.

It would not be advisable to do this dynamically, as the original reporter suggests.
Comment 5 John Mark Vandenberg 2011-05-22 02:38:09 UTC
This could be more easily implemented once bug 18374 is solved.
Also, I was wrong about AbuseFilter 'visibility' being unnecessary once this bug is solved, as it would still be needed for AF log entries about edits which were never saved.
Comment 6 Andrew Garrett 2011-05-22 08:07:59 UTC
(In reply to comment #5)
> This could be more easily implemented once bug 18374 is solved.

Well, not quite. They would both be made easier by a particular change to the filter's functionality.
Comment 7 Mark A. Hershberger 2012-01-03 17:40:27 UTC
Bumping priority. Thehelpfulone tells me via email:

> this bug is, in my
> opinon, very crucial to fix as it means that if a page has been oversighted
> but it was flagged by an abuse filter, then the logs of this abuse filter
> will show the content of the oversighted page. Therefore, this currently
> means that oversighters need to oversight 2 things - the page itself *and* the
> abuse filter log. This can unfortunately create a lot of extra work for
> them, so if this bug could be fixed, I'm sure they would appreciate it,
> especially as most of these oversighted pages contain sensitive personal
> information.
Comment 8 Rob Lanphier 2012-01-03 22:41:48 UTC
Bumping back to normal priority.  Mark, let's talk about the correct priority for this?
Comment 9 Mark A. Hershberger 2012-01-04 00:42:43 UTC
(In reply to comment #8)
> Bumping back to normal priority.  Mark, let's talk about the correct priority
> for this?

Yeah, highest was probably over-the-top.  But normal is higher than "lowest", so I'm happy.
Comment 10 Thehelpfulone 2012-01-12 18:33:23 UTC
So I asked an oversighter who knows about this bug to justify why it should be a higher priority, speaking from their experience:

"As it stands, when an oversighter suppresses something, we have to go hunting to find out if a log exists, then find it, then suppress it separately. It doubles both the time required, and the likelihood that private information will be missed accidentally"

I believe this is a strong argument for fixing this bug more quickly. Hope you agree. - THO
Comment 11 Mark A. Hershberger 2012-01-12 18:38:26 UTC
Re-raising to HIGHEST after reading Thehelpfulone's explanation and the possibility of private information leaking out.
Comment 12 Mark A. Hershberger 2012-01-12 19:29:46 UTC
Not on fire, so "Higher"
Comment 13 Andrew Garrett 2012-02-10 23:41:29 UTC
Fix committed in r111217. Comments welcomed.
Comment 14 Thehelpfulone 2012-06-02 18:05:13 UTC
Reopening, this still has not been fixed yet, https://gerrit.wikimedia.org/r/#/c/3435/ is where it is in gerrit.
Comment 15 Trijnstel 2013-03-10 13:39:51 UTC
still not fixed as the log entries are still visible when someone revdel or suppress and edit -- it seems to be deployed in gerrit (see above)?
Comment 16 billinghurst 2013-03-10 13:59:02 UTC
Confirmed.  One edit was revdel'd and then later suppressed at nlwiki.  It had tripped  and that resulted in 8 (2x4) logged filter additions that needed to be suppressed. And they had to be suppressed individually, no checkbox to select to group apply.

https://nl.wikipedia.org/w/index.php?title=Speciaal%3ALogboeken&type=suppress&user=&page=&year=&month=-1&tagfilter=&hide_patrol_log=1
Comment 17 Tim Starling 2013-03-18 23:44:19 UTC
(In reply to comment #16)
> Confirmed.  One edit was revdel'd and then later suppressed at nlwiki.  It
> had
> tripped  and that resulted in 8 (2x4) logged filter additions that needed to
> be
> suppressed. And they had to be suppressed individually, no checkbox to select
> to group apply.
> 
> https://nl.wikipedia.org/w/index.
> php?title=Speciaal%3ALogboeken&type=suppress&user=&page=&year=&month=-
> 1&tagfilter=&hide_patrol_log=1

So there are two bugs here.

The user attempted to save the page at 13:02:56, and received a warning. Then s/he clicked "save" again, and saved successfully at 13:03:20. 

Because the first edit attempt was rejected with a warning, it was not associated with a revision, so deleting the revision which was subsequently created had no effect. I'm not sure what the best solution here is, maybe just a bulk suppression UI.

The second edit attempt succeeded, but afl_rev_id was still not set, because of interaction with the autosummary feature.

if ( ( $vars->getVar('article_prefixedtext')->toString() !==
	$article->getTitle()->getPrefixedText() ) ||
	( $vars->getVar('summary')->toString() !== $summary )
) {
	return true;
}

If the summary is different after the edit is saved compared to when the filter was run, afl_rev_id is not updated. The edit in question did have an autosummary, and I've confirmed locally that enabling autosummaries allows the bug to be reproduced.

Just removing the summary condition should fix that part of it.
Comment 18 Tim Starling 2013-03-19 02:22:32 UTC
(In reply to comment #17)
> Just removing the summary condition should fix that part of it.

I94321905f38eafde8add00eff73745af255c1f15
Comment 19 James Forrester 2013-03-21 01:32:09 UTC
(In reply to comment #17)
> The user attempted to save the page at 13:02:56, and received a warning. Then
> s/he clicked "save" again, and saved successfully at 13:03:20. 
> 
> Because the first edit attempt was rejected with a warning, it was not
> associated with a revision, so deleting the revision which was subsequently
> created had no effect. I'm not sure what the best solution here is, maybe
> just a bulk suppression UI.

This is a mess of an issue. :-(

Perhaps we could automatically add a note on action=revisiondelete saying that there are AbuseFilter entries for a page of this title not associated with any particular revision, and asking the user to check to make sure they don't need to suppress these too? Somewhat succinctly (albeit not hugely accurately), "there are possible matching AbuseFilter entries for this page title"?
Comment 20 billinghurst 2013-03-21 12:11:53 UTC
That would probably suffice as long as there was an easy means to collect all the hits in the abusefilter, so the OS can review and then select those that should be culled.  Doing one by one (for either) is a tad trying.
Comment 21 Trijnstel 2013-03-21 20:58:49 UTC
(In reply to comment #19)
> (In reply to comment #17)
> > The user attempted to save the page at 13:02:56, and received a warning. Then
> > s/he clicked "save" again, and saved successfully at 13:03:20. 
> > 
> > Because the first edit attempt was rejected with a warning, it was not
> > associated with a revision, so deleting the revision which was subsequently
> > created had no effect. I'm not sure what the best solution here is, maybe
> > just a bulk suppression UI.
> 
> This is a mess of an issue. :-(
> 
> Perhaps we could automatically add a note on action=revisiondelete saying
> that
> there are AbuseFilter entries for a page of this title not associated with
> any
> particular revision, and asking the user to check to make sure they don't
> need
> to suppress these too? Somewhat succinctly (albeit not hugely accurately),
> "there are possible matching AbuseFilter entries for this page title"?

I agree, that's a possibility. But, abusefilter logs can only be hidden by oversighters and as nlwiki doesn't have local OS no one can hide the entries apart from the stewards (and most of the local users do not even know you *can* hide those). That's why it would be nice if they would be automatically be hidden (even a revdel system for the logs perhaps?)
Comment 22 James Forrester 2013-03-21 23:10:21 UTC
(In reply to comment #21)
> (In reply to comment #19)
> > Perhaps we could automatically add a note on action=revisiondelete saying
> > that there are AbuseFilter entries for a page of this title not 
> > associated with any particular revision, and asking the user to check to
> > make sure they don't need to suppress these too? Somewhat succinctly (albeit 
> > not hugely accurately), "there are possible matching AbuseFilter entries
> > for this page title"?
> 
> I agree, that's a possibility. But, abusefilter logs can only be hidden by
> oversighters and as nlwiki doesn't have local OS no one can hide the entries
> apart from the stewards (and most of the local users do not even know you
> *can* hide those). That's why it would be nice if they would be automatically
> be hidden (even a revdel system for the logs perhaps?)

The problem is that the RevDel system is about deleting revisions and log entries; AF log entries that block a save don't get associated with a revision, so AF can't automatically hide these. The automatic-hiding-of-AF-hits can only work because the AF hit is recorded against a revision that is being suppressed. My suggestion would at least highlight that further work could be needed.

Presumably if NLwiki has no oversighters it also can't suppress the edits using RevDelete, so they are just leaving them available to any passing sysop? That's rather troubling. Theoretically NLwiki could extend the "hide this AF hit" to local sysops (assuming that's OK with legal and the community itself?), but if they are dealing with a lot of these issues they should probably consider getting some local OSers like other big wikis with this issue. If neither of these options appeal, at the least if the notice is present it could be locally adjust to advise asking for a steward to help.
Comment 23 Trijnstel 2013-03-21 23:13:34 UTC
(In reply to comment #22)
> (In reply to comment #21)
> > (In reply to comment #19)
> > > Perhaps we could automatically add a note on action=revisiondelete saying
> > > that there are AbuseFilter entries for a page of this title not 
> > > associated with any particular revision, and asking the user to check to
> > > make sure they don't need to suppress these too? Somewhat succinctly (albeit 
> > > not hugely accurately), "there are possible matching AbuseFilter entries
> > > for this page title"?
> > 
> > I agree, that's a possibility. But, abusefilter logs can only be hidden by
> > oversighters and as nlwiki doesn't have local OS no one can hide the entries
> > apart from the stewards (and most of the local users do not even know you
> > *can* hide those). That's why it would be nice if they would be automatically
> > be hidden (even a revdel system for the logs perhaps?)
> 
> The problem is that the RevDel system is about deleting revisions and log
> entries; AF log entries that block a save don't get associated with a
> revision,
> so AF can't automatically hide these. The automatic-hiding-of-AF-hits can
> only
> work because the AF hit is recorded against a revision that is being
> suppressed. My suggestion would at least highlight that further work could be
> needed.
> 
> Presumably if NLwiki has no oversighters it also can't suppress the edits
> using
> RevDelete, so they are just leaving them available to any passing sysop?
> That's
> rather troubling. Theoretically NLwiki could extend the "hide this AF hit" to
> local sysops (assuming that's OK with legal and the community itself?), but
> if
> they are dealing with a lot of these issues they should probably consider
> getting some local OSers like other big wikis with this issue. If neither of
> these options appeal, at the least if the notice is present it could be
> locally
> adjust to advise asking for a steward to help.

Sure, stewards help where they can. And personally I don't see the need for local oversighters, nor does the community. But isn't it possible then to give admins the rights to revdel abuse filter log entries? (instead of only be able to suppress these) In that case admins could revdel them and with real privacy issues stewards could suppress them as well.
Comment 24 Rob Lanphier 2013-04-03 00:59:44 UTC
Would it be ok for us to close this one as fixed, since Tim's fix handles the most immediate issue?  At a minimum, I'm unassigning this from Tim, but we understand there's potentially a feature request or two buried in here.  Might be best to file those as separate issues.
Comment 25 Andre Klapper 2013-04-15 11:56:53 UTC
billinghurst / John Mark / Trijnstel:
Could you answer comment 24, please?
Comment 26 billinghurst 2013-04-15 12:03:02 UTC
I thought that was rhetorical.  I am comfortable to see changes at work, and it is one that I have only tripped over once, so not one in which I am an expert.
Comment 27 Greg Grossmeier 2013-04-15 15:47:52 UTC
Thanks billinghurst, closing out this bug for now. Feel free to reopen or file a new bug as appropriate.

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


Navigation
Links