Page MenuHomePhabricator

Administrators can no longer view deleted history of js/css pages
Closed, ResolvedPublic

Assigned To
Authored By
Xaosflux
Aug 28 2018, 1:23 PM
Referenced Files
None
Tokens
"Like" token, awarded by Guycn2."The World Burns" token, awarded by Riley_Huntley."Evil Spooky Haunted Tree" token, awarded by matej_suchanek."Cookie" token, awarded by Xaosflux."Heartbreak" token, awarded by BethNaught.

Description

Following T190015, sysops can no longer use their deletedhistory/deletedtext access on css/js pages.

While it is expected that non-interface administrators should not be able to undelete jss/css pages, we do expect that they may still view the deleted history, and even advertise it in the page notice.

You do not have permission to view a page's deleted history, for the following reason:
The action you have requested is limited to users in one of the groups: Administrators, Researchers.

Example can be seen here: https://test.wikipedia.org/wiki/Special:Undelete/MediaWiki:TestJs.js (assuming you are a testwiki admin, and not something extra like an interface admin)

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Tgr moved this task from Backlog to Next on the User-Tgr board.

Out of curiosity, has there been any movement on this? It is a somewhat significant bug both from a transparency standpoint and from an anti-abuse standpoint.

I'm aware that phab tasks can sit around for a long time and that this might be more complex than I understand, but this seems like something where we would want some movement before two years are up. Not trying to be snarky, but I do think it's worth raising the issue again.

@Tgr courtesy ping, is your "next" list timeline somewhat short?

With a patch authored over a year ago, is there anyone else that can get this moving again?

With a patch authored over a year ago, is there anyone else that can get this moving again?

The reviewers are asking to include PHPUnit tests for this functionality in the proposed patch authored by @Tgr; which seems to be the only objection at this moment.

I've got a revised patch ready which doesn't have a merge conflict and does include tests, but I'm having trouble getting it submitted to gerrit; my patches keep getting rejected by remote. I'm going to keep troubleshooting, but for those who want to try their hand, Gergő Tisza's changes need to be moved to includes/Permissions/PermissionManager.php because the modified functions were moved out of Title.php in gerrit patch 495832

I've got a revised patch ready which doesn't have a merge conflict and does include tests, but I'm having trouble getting it submitted to gerrit; my patches keep getting rejected by remote. I'm going to keep troubleshooting, but for those who want to try their hand, Gergő Tisza's changes need to be moved to includes/Permissions/PermissionManager.php because the modified functions were moved out of Title.php in gerrit patch 495832

What’s the error?

I’m guessing either you’re pushing to gerrit incorrectly or you don’t have rights to amend other people’s patches...

I’m guessing either you’re pushing to gerrit incorrectly or you don’t have rights to amend other people’s patches...

I think the latter; the error is cannot add patch set to 456140 so I think the problem is that I just can't add to others' patches. The troubleshooting section on the error suggests this is the case as well.

Should I push to a new patch set?

I’m guessing either you’re pushing to gerrit incorrectly or you don’t have rights to amend other people’s patches...

I think the latter; the error is cannot add patch set to 456140 so I think the problem is that I just can't add to others' patches. The troubleshooting section on the error suggests this is the case as well.

Should I push to a new patch set?

I've added you to the trusted contributors group on gerrit - you should be able to now

Change 456140 had a related patch set uploaded (by Wugapodes; owner: Wugapodes):
[mediawiki/core@master] Fix CSS/JSON/JS deleted-view permissions

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

@Wugapodes have you been able to revisit this?

@Xaosflux Yep, I've just uploaded a revised patch and the builds are passing. Presuming the CR goes well, it should be ready.

From the merged task: this is particularly an issue since T200176, since admins can now delete user CSS/JS pages, but still can’t view what they just deleted.

It is the intention of this procedure:

  • Every regular sysop may delete malicious software immediately.
  • A sysop account may be hijacked, somebody may be persuaded to undelete or to forward the bad code. All misusing pages could be exploited then.
  • Once the code page was deleted experienced staff has to be asked for undeleting or even copying presumable malware, investigating thoroughly first.
  • Recovering malware is doing harm. A suspicious resource which is unavailable for a couple of days until checked and released again does not cause any damage, it is just a tool being temporarily unavailable.
  • If there is no good reason, a sysop should not delete pages.

@PerfektesChaos those pages can be deleted for any type of reason (a common reason is "owner" request) - occasionally the owners then ask what the content was. Also keep in mind, that while those content models are designed for code, users can put anything in them - and letting admins investigate what happened by users is normally a good thing. If there is actual "dangerous" code on those pages, revision/page suppression can still be used.

Given the explanation by @PerfektesChaos , I can understand why admins are not permitted to create/re-create a js/css page. However, viewing a deleted version seems reasonably harmless. I imagine that people who have the ability and intention to hijack an admin account, have better sources for malicious code than deleted js/css pages on Wikipedia.

I am going to pile on here. Admins should be able to see the deleted
revisions. If there is something malicious in there, it should be
oversighted/suppressed, not just deleted.

I expect ArbComs and WMF to be firm on cases of abuse by admins that make
malicious, deleted content available to the public. Whether that is
attack/libel/etc. material (material of legal concern), or whether that is
malicious code.

Dirk

Dealing with skin resources is no longer sysop business.

  • It is up to interface administrators to handle JS/CSS pages.
  • The ability to delete JS/CSS pages was left to react on threats as quick as possible, e.g. privacy violation via beacon just discovered.
  • People should think twice before asking for page deletion. Once something was deleted it always needs further explanation and requests to get back content which might be copyvio or legally questionable or harrassment or TOU conflict or whatever. It is up to interface administrators to decide upon JS/CSS page content.

[…] content which might be copyvio or legally questionable or harrassment or TOU conflict or whatever.

These possibilities are not unique to CSS/JS pages; in particular, copyvio is much more likely to happen on non-CSS/JS user subpages. (The rest is probably not more likely to happen on non-CSS/JS pages, but neither less likely.) I still don’t see the point in making it absolutely impossible for admins to correct their mistakes. We’re all humans and do mistakes every now and then. Sure, you can just ask the interface admins, but what if

  • there are no interface admins in your wiki, and
  • you don’t speak English well enough to write your request on Meta, and no global admin speaks your native language?

This is a completely useful thing to have. I don't want to go WP:BEANS here, but having access to those can really help non-IAs curb abuse. @TonyBallioni agrees with me here.

I personally had to ask a steward once for the content of a .js as the local IAs weren't available, and having to do that is an unnecessary bureaucracy.

I understand why a sysop doesn't have this permission now, but I think we should whitelist read-only actions, and trust sysops for viewing deleted content. The case of a sysop innocently revealing a bad script to someone else is an edge case, and probably won't happen at all.

Change 634262 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] PermissionManager: Allow sysops to view deleted config pages

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

Change 456140 abandoned by Urbanecm:
[mediawiki/core@master] Fix CSS/JSON/JS deleted-view permissions

Reason:
https://gerrit.wikimedia.org/r/c/mediawiki/core/ /634262/ is a newer version of this one

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

Dealing with JS/CSS resources is no longer sysop business.

  • It is up to IA to decide what shall happen with sanitized and deleted code.
  • If you do not want a page to be deleted do not ask for deletion.

You can repeat that over and over. It is tasks of administrators to
administrate Wikis. That means that they must be able to see evidence that
was deleted. Unless, which is extremely unlikely, the deleted page is
malicious code (which should have been suppressed, not deleted), what is on
the page is part of the accountability, it is part of patterns, etc.
Administrators need to see that.

I just want to pile on here and say that admins should be able to view deleted js/css. Currently admins can delete the content, but cannot even view what they've deleted. Not only is this logically inconsistent, it is also improper from a peer accountability perspective. The risk of not making this change is that we'll end up granting IAdmin to increasing numbers of people simply for the purpose of view access, which would defeat the entire purpose of making this change in the first place.

Agreed. I also want to add the perspective that it's not just English Wikipedia with plenty of interface admins that we're dealing with here. We're also looking at a vast majority of wiki's where the group of interface admins will be tiny, if even existent.

Adding to the pile on. The purpose of interface admin permission is so that non-technical administrators do not accidentally make breaking changes to technical pages. Administrators can still view these pages.

We trust administrators not to repost deleted copyvios, personal attacks, libel, etc, on all other pages (where these are almost infinitely more common) so there is no conceivable reason not to trust them to do the same on js/css pages.

Almost all content on deleted css/js pages is not malicious. There is no justification for preventing administrators viewing or restoring this. Where content does need to be hidden from administrators then we should suppress it, because that is the purpose of the suppression tool and those people with access to it are carefully vetted. Suppression is a tool of first resort so non-technical oversighters can and will suppress such pages (and pages which are potentially harmful) when alerted to them, and this can be reverted by/after discussion with their more technical colleagues if determined innocent.
If you have an oversighter who is willing to repost or share the content of a suppressed .js/.css page then you have an oversighter who is willing to do the same for other types of suppressed pages and you have a problem that is completely unrelated to the interface admin right.

For WMF wikis, we should probably expand the suppression policy to specifically allow for this type of removal. Feedback is welcome at https://meta.wikimedia.org/wiki/Talk:Oversight_policy#Expand_for_malicious_code_removal

There is no justification for preventing administrators viewing or restoring this.

Well, for restoring there is. Undeletion (in particular selective restoration after deletion) is tantamount to editing, which would otherwise be prohibited; depending on the page, that could also mean execution of code by users.

There is no justification for preventing administrators viewing or restoring this.

Well, for restoring there is. Undeletion (in particular selective restoration after deletion) is tantamount to editing, which would otherwise be prohibited; depending on the page, that could also mean execution of code by users.

Indeed - this patch will not allow administrators to undelete revisions, just to view deleted ones, for exactly this reason

Please give users back the ability to view the deleted histories with the (deletedhistory) permission and the deleted text with the (deletedtext) permission. Because the way things are now, users can only view the deleted histories and deleted text of pages where they have the permission to undelete the pages. Usually with the (undelete) right, though in some cases the (editinterface) permission may be needed if it is a system message. Not being able to restore the pages you're viewing is one thing. But making it so they can't view the histories of pages they don't have permission to undelete is exceedingly awkward.

I personally think that there should at least be a new permission that allows users to access the deleted histories and deleted text of pages, regardless whether or not they are able to undelete the said pages. The obvious exception would be if the pages were suppressed using the (suppressrevision) permission. But in all other cases, if a user has the (deletedhistory) and (deletedtext) permissions, then they should be able to view the contents of the deleted pages, regardless if they can undelete them or not.

@C.Syde65 what you are describing is exactly what this task is about - it is not actually restoring the view access on these pages to "administrators" - but to users with the permissions you mentioned.

Agreed. I also want to add the perspective that it's not just English Wikipedia with plenty of interface admins that we're dealing with here. We're also looking at a vast majority of wiki's where the group of interface admins will be tiny, if even existent.

The English Wikipedia also has a very tiny group. It’s the smallest user rights group on the project. This is a problem on all wikis, big and small.

Yeah well I'd like to have those things back. Because I prefer not to have my bot be flagged as an Administrator, but be flagged as pretty much everything else including Bureaucrat.

Patch merging now, should be ready to announce

Change 634262 merged by jenkins-bot:
[mediawiki/core@master] PermissionManager: Allow sysops to view deleted config pages

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

DannyS712 claimed this task.
DannyS712 removed a project: Patch-For-Review.