Page MenuHomePhabricator

VisualEditor: Allow AbuseFilter rules to identify VE edits
Open, NormalPublic

Description

I would like to request that some flag or other indicator be added such that abuse filter rules can tell whether the edit is coming from VE or from the source editor.

Because the abuse filter tends to encourage different sorts of editor errors, it would be useful to know which interface is being used when editing. For example, a nowiki tag added by the source editor is almost certainly intentional, while a nowiki added by VE is probably an accident and should be tagged for inspection.


Version: unspecified
Severity: enhancement
See Also:
T54281: VisualEditor: VE should use the existing "action" URL parameter instead of the new "veaction"

Details

Reference
bz51421
Related Gerrit Patches:
mediawiki/extensions/VisualEditor : master[WIP] Add user_visualeditor variable for AbuseFilter

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 2:01 AM
bzimport added a project: AbuseFilter.
bzimport set Reference to bz51421.
bzimport added a subscriber: Unknown Object (MLST).

Both AbuseFilter and VisualEditor use change tags. It might be possible for AbuseFilter to use VisualEditor's tag if AbuseFilter runs after, though I don't see tags mentioned at https://www.mediawiki.org/wiki/Extension:AbuseFilter/Rules_format

This would need AbuseFilter to be extended to understand tags (or, to understand VisualEditor, which seems a less satisfying and extensible way of achieving this).

Can the abusefilter detect veaction=edit. Using that and we could block edits which insert nowikis from the VE when they are almost always in error and allow them from wikitext editor, when they are normally ment.

nykevin.norris wrote:

Wait, if the AbuseFilter is going to understand tags, and already applies tags, what order will filters run in? What if a later filter applies a tag that would have triggered an earlier filter? Should MW just keep re-running the filters until everything's been triggered?

I'm pretty sure it's impossible to do this since any kind of tags can be only added after an edit is saved (ChangeTags::addTags() function requires the edit to have a know rev_id which implies that it already exists in the database).

(In reply to comment #5)

I'm pretty sure it's impossible to do this since any kind of tags can be only
added after an edit is saved (ChangeTags::addTags() function requires the
edit to have a know rev_id which implies that it already exists in the
database).

Does AF's hook happen after VE's save (which applies tags), at least?

AF's hook attempts *during* save, in EditPage#runPostMergeFilters and ApiEditPage#execute (the second one is only used to present nicer information about saving failure in the API and is what VE is actually hitting, but that doesn't matter here).

VisualEditor's ApiVisualEditorEdit calls the edit API first (#saveWikitext) and only after it has succeeded calls ChangeTags::addTags to add the tags to the edit.

So to make this work, we'd have to adjust code in core, in AbuseFilter and in VE to allow passing change tags with the edit API call or in some magical way before it. It'd make the logic really awkward (since tags could now be added before/during and after making the edit, and the second kind would be invisible to VE) and likely introduce back-compat issues (for the same reason).

There are also requests to make the tags editable by users (bug 18670), which generally seems like a pretty good idea, but complicates the issue here even further.

So to summarize, I don't see this happening, but feel free to prove me wrong if you think you can tackle this :)

s/attempts/happens/, of course.

s/invisible to VE/invisible to AF/. I swear, I did proofread that. :)

The original request was merely for the AbuseFilter to somehow be made aware whether or not an edit had come from VE as opposed to source editing.

For example, the Abuse Filter is aware of "action=" but not "veaction=". At present "action=edit" and "veaction=edit" look identical to the AbuseFilter.

I think the suggestion that AbuseFilter should somehow be allowed process tags generated by other entities is actually an unnecessary (and more complicated) diversion.

(In reply to comment #10)

The original request was merely for the AbuseFilter to somehow be made aware
whether or not an edit had come from VE as opposed to source editing.
For example, the Abuse Filter is aware of "action=" but not "veaction=". At
present "action=edit" and "veaction=edit" look identical to the AbuseFilter.

Adjusting comment accordingly.

I think the suggestion that AbuseFilter should somehow be allowed process
tags generated by other entities is actually an unnecessary (and more
complicated) diversion.

Sure.

matej_suchanek added a project: VisualEditor.
matej_suchanek set Security to None.
jhsoby added a subscriber: jhsoby.Dec 28 2016, 10:33 PM

Now with the 2017 wikitext editor I think this should get a priority bump, seeing that it too uses ?veaction instead of ?action, which makes it impossible (AFAIK) to use edit filters to identify those edits.

Now with the 2017 wikitext editor I think this should get a priority bump, seeing that it too uses ?veaction instead of ?action, which makes it impossible (AFAIK) to use edit filters to identify those edits.

The action URL path is a poor guide to the source of edits (see the Special:Edit or whatever it's called tool, the mobile Web wikitext and visual editors, and other mechanisms). Anyone trying to capture edits by use of the URL path is going to fail pretty terribly.

I also don't agree that this is a particular priority. People have been editing using different tools for years without the software allowing discriminatory edit validation.

Restricted Application added a subscriber: jeblad. · View Herald TranscriptApr 25 2018, 9:02 AM
Krinkle removed a subscriber: Krinkle.Apr 28 2018, 12:37 AM
Deskana lowered the priority of this task from Low to Lowest.Aug 30 2018, 1:13 PM

I'm sorry, I may have missed something, but... wouldn't it be much easier to generate a dedicated AF variable inside VisualEditor using AF hooks? This would avoid messing up with change tags and request params, and it's also the way used to distinguish edits coming from e.g. mobile app or mobile version.

Change 456418 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/VisualEditor@master] [WIP] Add user_visualeditor variable for AbuseFilter

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

Daimona claimed this task.Aug 30 2018, 4:59 PM

To complete the patch, I need a way to determine whether the edit being saved comes from VE or not (yep, the main part). The main thing we have access to is the current WebRequest. Is there something in the request which can uniquely identify VE edits? Sorry for asking, but I cannot set up Parsoid on my wiki and thus can't test VE.

Is there something in the request which can uniquely identify VE edits? Sorry for asking, but I cannot set up Parsoid on my wiki and thus can't test VE.

Not really. VE just calls the action=edit API internally (from PHP code), see ApiVisualEditorEdit::saveWikitext(). However, maybe you can add some magic parameter there and handle it on the other end in the AbuseFilter hook handler. I am not sure how it all works on the inside, you might need to experiment a bit :/

@matmarex That hurts :/ Adding a parameter would be surely working, but also highly specifical code which shouldn’t be the first choice; unless other parts of the code throughout MediaWiki will need to know if the current edit comes from VE. Before giving up, is there something else in the global scope which could help us?

Daimona raised the priority of this task from Lowest to Normal.Aug 31 2018, 7:03 AM