Page MenuHomePhabricator

Rate limit or restrict access to comment removal
Closed, WontfixPublic

Description

Administrators and members of projects with specific permissions should be able to delete comments in Maniphest.

Use case: vandals, spammers, users accidentally leaking sensitive information.

Deleting would mean removing the information. A reference to the deleted post would be left in the UI, but there shouldn't be any way to access to the removed content by accessing to previous versions or diffs.

If deleting means making them private and accessible only to users with permissions to delete comments, that might work as well.

Event Timeline

qgil created this task.Apr 26 2014, 7:23 PM
qgil raised the priority of this task from to Needs Triage.
qgil updated the task description. (Show Details)
qgil added a project: Maniphest.
qgil added a subscriber: qgil.

We have a handful of random scripts for deleting other types of objects right now, but plan to consolidate them under a single script which would be accessed from the command line (see T4749). This would let administrators with server access permanently destroy comments with, e.g., some command along these lines:

phabricator/ $ ./bin/destroy completely-destroy-forver <some-comment-id>

I'm not sure if that would solve most of the problem for you (e.g., this is a rare issue and you just want some way to do it), or if you'd prefer to have a way to do it from the web UI.

From the web UI, we'd probably want to just hide the information rather than delete it (in general, administrators have very few real powers right now, which means they can't do very much damage if they go rogue or an attacker compromises an admin account). This probably isn't too difficult to implement but I'd want to think it through a bit since we may want to start developing a better set of approaches for dealing with spam.

How much of this stuff do you anticipate needing to deal with? We have a tiny little bit of it, but mostly security researchers creating things trying to find XSS holes -- probably like 3-5 things a week, which we mostly ignore and occasionally close/archive. We could possibly look into preventing this sort of thing technically (e.g., require new users to fill out a CAPTCHA, or have approval queues for comments, or something like that).

qgil added a subscriber: aklapper.Apr 26 2014, 8:40 PM

CCing Andre, Wikimedia bugmeister, who has to deal with this problem regularly.

Hiding via UI and completely destroying via command-line sounds like a good approach to me, similar to how we deal with these actions in MediaWiki.

This might work for the time being - no stats, but I guess it happens once or twice a month that I (or other WMF Bugzilla admins) need to hide a comment or hide/delete an attachment.

Still I'd prefer if hiding comments or attachments from others would not require shell access (WMF is pretty reluctant to hand out shell access) but had some checkbox in the web UI, like "Mark as private" or "Hide".
(Just in the unlikely case there are more generic plans to provide some kind of tagging for comments in Maniphest tasks, "hide from public" could also be a tag.)

qgil moved this task to Potential blockers on the Wikimedia board.Apr 30 2014, 2:29 PM
qgil moved this task from Potential blockers to Backlog on the Wikimedia board.Apr 30 2014, 2:42 PM
qgil moved this task from Backlog to Potential blockers on the Wikimedia board.
epriestley edited this Maniphest Task.May 1 2014, 11:49 PM
epriestley edited this Maniphest Task.May 2 2014, 12:53 AM
epriestley edited this Maniphest Task.May 2 2014, 1:23 AM
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.May 2 2014, 2:27 AM
epriestley edited this Maniphest Task.May 2 2014, 3:04 AM

I've marked D8945 as resolving this. After that lands, these actions will be available to you:

  • There will be a new "Remove" action on comments in the web UI. This is available to administrators and the user who made a comment.
    • Removing a comment permanently hides it and its history.
    • A removed comment can not be edited or restored.
    • The fact that the user made the comment is still shown, but the text is replaced with: This comment was removed by <username>.
    • The history is preserved in the database, but there's currently no mechanism (other than mucking around in the database) to retrieve it.
  • The new bin/remove destroy Txxx script will allow you to completely destroy a task or other object by name (Txxx, Dxxx, etc). Not all objects are supported yet but we can add more support as it arises. This is a complete, permanent deletion of the object and as many related objects as we can identify (including the task, its entire transaction history, all comments, all their transaction histories).
  • The existing policy controls allow you to hide a task; non-administrative users could use this in some cases and refer the issue to an administrator.
  • The existing bin/policy unlock Txxx script will let you unlock an object (set its policies liberally) if necessary.

I think these should cover the common cases (accidentally posting credentials, casual drive-by spam). If you run into more actively malicious users we can figure out how to deal with them (by, e.g., rate limiting actions, or giving you better tools to rewind all the actions a user has taken, or whatever else makes sense).

qgil added a comment.May 2 2014, 4:36 AM

Thank you so much! The features described satisfy our use cases. You just removed the blocker #1 we had in our potential migration plans.

Can the possibility to delete be restricted by a policy? For instance, only admins can delete. The fact that all users can delete their own posts kept me thinking. MediaWiki and Bugzilla users cannot do this (in the first case you can revert, in the second case comments are not editable). There is the theoretical case of the upset volunteer that one evening decides to start deleting their own contributions. I will leave that to @aklapper because he has seen enough to write a couple of books.

I think it'd be better to have policy controls on removing comments as well, especially since it removes the ability to view the history of the comment.

I think it'd be better to have policy controls on removing comments as well, especially since it removes the ability to view the history of the comment.

What's the scenario you're concerned about here? Is it the same "frustrated user deletes all their stuff" case?

For instance, only admins can delete.

How about if we put a rate limit on this action instead -- say 6 comment removals per hour for non-administrators? We have an easy technical way to do that, and it should give users wide latitude to use this feature legitimately without giving them much room to go on a rampage if they get frustrated or upset.

Nah it's not that, it's more that even administrators can't view the history of a removed comment, so if someone wants to find out what was written in a deleted comment, I've got to start doing SQL queries to find it out.

I'm fine with regular users not being able to view history, but it's a little inconvenient for administrators to have to SQL query to find it out.

Can the possibility to delete be restricted by a policy? For instance, only admins can delete. The fact that all users can delete their own posts kept me thinking. MediaWiki and Bugzilla users cannot do this (in the first case you can revert, in the second case comments are not editable). There is the theoretical case of the upset volunteer that one evening decides to start deleting their own contributions. I will leave that to @aklapper because he has seen enough to write a couple of books.

Yes, this could happen. In Mozilla, GNOME and Wikimedia Bugzillas I've seen 'frustrated' users asking to delete their bugtracker account and all comments they ever made.
We strive for openness but hiding/deleting comments always creates some intransparency (it makes understanding the flow of discussion and what led to certain decisions hard when comments quote or refer to content of now deleted comments). Hence I prefer to make hiding/deleting attachments/comments very restrictive (only few and good reasons like spam/personal attacks, and only few trusted admins being able to hide/delete stuff with a shared understanding of good reasons).

but it's a little inconvenient for administrators to have to SQL query to find it out.

I think letting users remove their own comments to fix mistakes (credentials, sensitive content, embarrassing things they accidentally copy/pasted) is an important part of this feature, and it's important that administrators not be able to see content hidden in this way from the web UI.

Particularly, if this feature is ever used to hide credentials and administrators can still view history from the web UI, a compromised administrator account can read all the hidden credentials (a clever attacker might review all of the removed comments first, as they're likely the most sensitive ones). While this would be somewhat involved mechanically today, they could probably do this efficiently via the API in the future.

The only cases I see for administrative review here are to satisfy curiosity (which I think is hugely outweighed by the value of limiting administrative power) and repair damage (which we should just prevent and/or provide tooling for -- this use case is theoretical, and not time-sensitive).

There a reasonable argument to be made that the "janitorial" balance of administrative power is wrong for some installs, and installs might prefer powerful administrators who can bypass policies, edit anyone's content, delete objects freely, review any edits, impersonate users, etc. I'm open to pursuing this, but I think we should commit to it fully, so it's clear that when you enable the security.if-someone-gets-an-admin-account-the-can-steal-and-delete-all-the-data flag you're trading away all of the protections that the janitorial model provides against rogue admins and admins with compromised accounts.

Until we do implement such a flag, administrative power should adhere to the "janitorial" model as strictly as possible. In general, we're moving to continue reducing administrative power.

In Mozilla, GNOME and Wikimedia Bugzillas I've seen 'frustrated' users asking to delete their bugtracker account and all comments they ever made.

Would you be comfortable with a rate limit instead of restricting this power to administrators? Or limiting user removal to the first hour after the comment is posted, or some similar rule?

In general:

  • I always want to avoid introducing configuration unless absolutely necessary. Configuration makes the software harder to develop, test and use. Rules like a rate limit or removal window seem like they'd satisfy these concerns without requiring configuration.
  • I want to retain the ability for users to remove their own comments. I think this is valuable for legitimate users who make mistakes and post credentials. We had this happen to someone recently, and they were frustrated they could not remove the comment.
qgil added a comment.May 4 2014, 4:41 AM

Another idea: users can delete an own posts as long as it is still the last one in a thread. Otherwise they need help from an admin. This would solve the urgent cases without introducing an arbitrary time frame, and would avoid ugly situations when a user has deleted their own post, but some data is still visible, quoted in a reply. This would would limit the real damage a user can make deleting own posts for whatever reason. Still, keeping a throttling of 5 messages per day may be a good idea.

epriestley edited this Maniphest Task.May 5 2014, 5:55 PM
epriestley edited this Maniphest Task.
epriestley closed this task as Resolved.May 5 2014, 5:55 PM

Closed by commit rP58f66fea804e.

epriestley reopened this task as Open.May 5 2014, 6:12 PM

I marked this as fixed before the followup discussion about limiting users from removing their own comments, but it should probably remain open until we address that to everyone's satisfaction.

qgil added a comment.May 5 2014, 6:24 PM
This comment was removed by qgil.
qgil added a comment.May 5 2014, 6:26 PM

This comment was removed by qgil.

qgil added a comment.EditedMay 5 2014, 6:28 PM

The two posts above look identical (((not anymore!))), and only one is a real removal. Which one? ;)

A different icon for posts removed would make them stand apart without any possibility to fake them.

(I don't know who would want to fake a deleted post or why. Humans are weird sometimes.)

PS: in fact they are not identical. Only one has the settings icon active. Still. Just details.

epriestley added a subscriber: chad.May 5 2014, 6:33 PM

Hmm, that's a good point. Maybe something like this? @chad, what do you think?

epriestley edited this Maniphest Task.May 5 2014, 6:37 PM
epriestley edited this Maniphest Task.May 5 2014, 7:00 PM
epriestley renamed this task from Possibility to delete comments in Maniphest to Rate limit or restrict access to comment removal.May 6 2014, 1:31 PM
epriestley triaged this task as Low priority.
qgil moved this task from Potential blockers to Details on the Wikimedia board.May 6 2014, 2:47 PM

A "time limit" (users can only delete within (n) minutes of initially saving the comment) or a "rate limit" (users can only make (n) deletions per day) would be a good minimal protection. I'd like to suggest/request raising the priority of this task. Ideally, this feature could be available before Nov 24.

(Context: I've created detailed notes at https://phabricator.wikimedia.org/T1185 explaining the concerns. It's particularly problematic in our traditionally very-transparent wiki culture, and where there are strictly vetted people who have the ability to "delete from visibility" (rather than just "editing it"). In the long run, I think we're going to need to be able to restrict this to a user-role/group, but in the short-term, a time/rate limit would be much appreciated.)

chad added a comment.Nov 11 2014, 12:37 AM

This is my anecdote:
I run a fairly large auto web forum with many millions of posts, and very very rarely have I seen someone "rage quit". It happens, but maybe it's happened like 5 times in 8 years. The usefulness of their content added to the site is debatable, and I've found there is always someone else willing to offer that same value at the end of the day. It makes me feel better to serve the person who wants their content just removed, as I've personally felt the same way, and the price that maybe yes, the site is 0.01% less useful temporarily is an easy one for me to pay. And more specifically, we've had zero issues or questions from the rage quits over missing information.

My concern here is there isn't any immediate reason to upstream such a feature like rate limiting, maybe @epriestley thinks otherwise though (he initially suggested it). In order for this to be something we'd usually consider, it'd need to be built in a way that serves many installs with similar issues. We just haven't seen that across our other open source installs, at least right now.

My suggestion would be to keep this task open and circle back to if/when a real issue arrises. There already is significant friction to 'rage quitting' Phabricator (you'd have to spend hours doing it), and it already scales (longer term users have more work to do). It is very hard for us to prioritize any task that only potentially solves an issue for one customer. (See T4778 for more info on prioritization).

Yeah, we're very unlikely to pursue this in the upstream based only on concerns about this potentially being an issue. If there are significant concerns on the WMF side, I can roughly walk you through how to support this locally with a small patch, but given that we haven't seen this actually be a problem and the data isn't actually deleted (just rendered totally inaccessible) this is way down the list of priorities and vanishingly unlikely to land in the next two weeks.

nemobis added a subscriber: nemobis.EditedNov 26 2014, 10:24 AM

Removal of comments is not the only issue: editing of comments

  • is not publicly logged AFAICS,
  • shows no diff.

An attacker who managed to get access to a very active account of an issue tracker would be able to delete substantial portions of the issue tracker without anyone noticing or being able to revert.

Is there a task for adding a line like trac's "last edited at X by Y (previous) (diff)" (cf. https://trac.torproject.org/projects/tor/ticket/13319#comment:12 )? I couldn't find one.

I just clicked the drop down on your comment and selected "View Edit History". The comment also shows "Edited" on it.

nemobis added a comment.EditedNov 26 2014, 10:56 AM

Thanks, good thing I added "AFAICS". ;-) Then the only missing parts are notifications and diffing (optionally, also a direct link to the diff e.g. from "Edited"). Diffing is partly covered by T4961.

qgil moved this task from Details to Important on the Wikimedia board.Dec 4 2014, 1:02 PM
eadler added a subscriber: eadler.Apr 30 2015, 4:17 AM
epriestley moved this task from Backlog to Monitor on the Abuse board.Jan 25 2016, 11:06 AM
epriestley closed this task as Wontfix.Aug 23 2016, 10:38 PM
epriestley claimed this task.

As far as I know, no users have actually gone berserk and deleted all their comments in nearly two years now, so I don't plan to specifically build comment removal rate limiting: this action does not seem particularly more dangerous or abuse-prone in practice than other actions like adding comments, merging tasks, etc. If a user did do this, recovery is likely not very difficult even without limiting.

T10215 has a broader discussion of general thinking about abuse. If comment removal did become more of a problem in the future, I suspect we'd try to build something more like a "rewind/undo" tool for all of a user's actions (see T11254), since that would also address other abuse scenarios.

Generally, we're keeping an eye on abuse but don't want to take an overly proactive stance in trying to combat it because we can imagine a million abuse scenarios but protecting against all of them would take a substantial effort and, in some cases, harm/annoy legitimate users with restrictions and limits, while preventing little real abuse today. It's also generally hard for abusive users to do any permanent damage in Phabricator (since they can't really destroy information in most cases, and almost always leave a trail). They can certainly be annoying and create headaches, and I expect that we will improve our abuse tooling over time, but I generally want to respond to real abuse rather than trying to anticipate possible abuse.