Page MenuHomePhabricator

Fix rows in ipblocks that point to a non-existing user in ipb_by_actor field, due to T260485
Closed, DeclinedPublicSecurity

Description

The root cause of T260485 has been around for about a year, leading to several hundred blocks across wikis that are invisible and/or cannot be undone.

There are two cases to consider:

  • A row exists in the local actor table with an id that matches the ipb_by_actor field, but it's referring to the wrong user. In this case, the block is mis-attributed, but functional. It's unclear how to detect this case, see T261143: Fix misattribution of block due to bad values in the ipblocks ipb_by_actor field.
  • No row exists in the local actor table with an id that matches the ipb_by_actor field, rendering the block entry defunct. This is easy to detect.

A quick spot check find 20 such entries on dewiki and 96 on enwiki, around 2019-09-19.

We could try to find an entry in the ipblocks table on meta with the same ipb_address value, and a close-by value of ipb_timestamp. But given the small number of cases and the limited relevance of the information, it may be easiest to just attribute such blocks to a reserved user, probably "Unknown user". We can leave fixing the block's attribution for later, see T261143.

So for now, we need to write a script that finds rows with bad values in ipb_by_actor, and replaces that value with the actor id of "Unknown user". This script needs to be run against all wikis. Given that the relevant query runs in under 2 seconds for enwiki and yields fewer than 100 rows to update, this should be quick and easy.

Event Timeline

daniel created this task.Aug 26 2020, 4:17 PM
daniel triaged this task as High priority.EditedAug 26 2020, 4:19 PM
daniel moved this task from Inbox to Later on the Platform Team Workboards (Clinic Duty Team) board.

Oh darn, it is... hm... but the discussion there is about fixing attribution. Here I want to focus on restoring functionality. I'll keep them separate, and clarify the distinction.

daniel renamed this task from Clean up rows in ipblocks that have bad values in the ipb_by_actor field, due to T260485 to Fix rows in ipblocks that point to a non-existing user in ipb_by_actor field, due to T260485.Aug 26 2020, 4:24 PM
daniel updated the task description. (Show Details)
daniel claimed this task.Sep 8 2020, 6:09 PM
Urbanecm moved this task from Backlog to Reviewing on the User-Urbanecm board.
daniel added a subscriber: kevinbazira.

The patch with the maintenance script is merged, now we need to run it. Moving back to "ready".

The root cause of T260485 has been around for about a year, leading to several hundred blocks across wikis that are invisible and/or cannot be undone.

Obviously needs backporting to REL1_35 (https://gerrit.wikimedia.org/r/c/mediawiki/core/+/629199)

What about REL1_34? Is REL1_31 irrelevant?

Reedy added a comment.EditedSep 23 2020, 11:00 AM

Backport to REL1_34 was relatively trivial - https://gerrit.wikimedia.org/r/c/mediawiki/core/+/629200

But REL1_34 is due to be EOL in November 2020...

EDIT: Well, except for needing to adjust for various new services...

Reedy added a comment.Sep 23 2020, 7:23 PM

Poke. Planning on doing releases tomorrow, so suggestion whether it should land in REL1_34 (and REL1_31?) is appreciated :)

@Reedy I believe it's needed in REL1_34 but not REL1_31, since the problem was introduced by this patch: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/525605

More details about timings of the block bug in T260485#6389423

Reedy added a comment.Sep 23 2020, 7:46 PM

@Reedy I believe it's needed in REL1_34 but not REL1_31, since the problem was introduced by this patch: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/525605

More details about timings of the block bug in T260485#6389423

Thanks! That works for me as reason enough for merging the REL1_34 patch and not even attempting a REL1_31 backport :)

The patch with the maintenance script is merged, now we need to run it. Moving back to "ready".

I'll run it once we get wmf.11 somewhere.

Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:01 PM

I don't understand why T261143 is difficult and why you would write "(unknown)" to these rows rather than setting them to the actually correct username. I'll write more on T261143, I just wanted to suggest not running the maintenance script right now, while I have a look at this.

dbarratt removed a subscriber: dbarratt.Nov 6 2020, 1:18 PM
tstarling closed this task as Declined.Nov 11 2020, 1:46 AM

My script is ready to run now.

tstarling changed the visibility from "Custom Policy" to "Public (No Login Required)".Nov 13 2020, 2:54 AM
tstarling changed the edit policy from "Custom Policy" to "All Users".