Spitballing some tags here.

This is being thrown in https://gerrit.wikimedia.org/g/mediawiki/extensions/Wikibase/+/9d617f33075a35e2de369f1ecd82f28ec1686520/lib/packages/wikibase/data-model/src/Entity/EntityRedirect.php#41

		if ( $entityId->getSerialization() === $targetId->getSerialization() ) {
			throw new InvalidArgumentException( '$entityId (' . $entityId . ') and $targetId can not be the same.' );
		}

(It would probably be nice to parametrize that message, but not sure how easy it is to inject a logger there.)

Looking at logstash, it does not seem to be common, 6 events in the last hour. However, widening the timeframe to one year, shows entries that seem to have started in August 2023 with 1.41.0-wmf.20.

Suspiciously, all of them are about L123. Do we somehow have a corrupted value/redirect in our database? Or is there somehow a way to create redirects from Lexemes (other Entites too?) to themselves but so far it was only used on L123?

I did not find any tasks that look related to this, so if there is indeed a possibility to create redirects of some Entities to themselves, it seems to be so far unknown.
I'm changing this to a security issue until we can rule that out. The intention is to limit the access to this to prevent it from being abused, if such a problem exists.

This might hint at a possibility to create redirects of (some?) Wikidata Entities to themselves, and thus inject redirects into the DB that should not exist there. Limiting the scope of this task until we can rule that out.

As far as I can tell, there is no ongoing abuse of this potential vulnerability, if it exists.

I hope, I'm not overreacting here.

Ok, maybe I did overreact a bit here. I now found a previous mention of this in T217329: bug in 1.33.0-wmf.18+ breaks abstract dumps on testwikidatawiki, among other things | MWContentSerializationException $entityId and $targetId can not be the same and it also already mentions L123. How do I turn this back into a normal, public production error task?

@brennen Thank you for fixing the visibility of this task 🙏

Adding Wikibase Reuse Team since the Data Model Lib is in their scope

The bad revision is 843735157, if you try to view it you can also get the error via index.php instead of api.php.

What if we just revdelete that revision? It doesn’t sound like there are any others like it, and it’s no longer possible to make a lexeme redirect to itself, so I think we only need to decide what to do with this one weird legacy revision.

In T349230#9267669, @Lucas_Werkmeister_WMDE wrote:

What if we just revdelete that revision? It doesn’t sound like there are any others like it, and it’s no longer possible to make a lexeme redirect to itself, so I think we only need to decide what to do with this one weird legacy revision.

Yes, I think this would also be my preferred solution. This should never have happened in the first place, and if it ever happens again, then a hard error is probably what we want, because something is very wrong.
If we can "fix" this particular instance in our database safely by just doing a revdelete, then that seems like a good way forward to me to resolve this particular error.

Making that error "nicer" in some form can still be a follow-up, if we consider that worthwhile.

Task Review:

• This is probably a fluke revision, created before the check mentioned in the code example was introduced.
• Therefore, revdeleting is probably the best course of action

Task Prio Notes:

• Affects end users /production
• Does not affect monitoring
• Does not affect development efforts
• Does not affect onboarding efforts
• Does not affect additional stakeholders

Revdeleted. Getting all the L123 revisions via the API succeeds now, so I think this is resolved. (It’s probably possible for a sysop to trigger the error by looking at the deleted revision, but let’s not worry about that IMHO.)

Let’s look at logstash again in a few days.

Let’s look at logstash again in a few days.

Note: there will be a bunch of exceptions from today (just now), because I was testing with the URL from T217329#7181299 and trying to figure out why I was still seeing the error – I didn’t realize that, unlike the normal user interface, the API will directly (try to) show you deleted content as long as you have the permission to see it. So if you access that URL while logged into an account with sysop rights, you get the error, but otherwise you get a response indicating hidden content instead.

The revision is indeed deleted. However, searching the error message on Logstash shows there were two more occurrences on the 25th of October and another on 26th.

Both of those exceptions have an api.php URL with action=query, prop=revisions and rvprop=content (and they’re also both for L123 btw). They also have the referrer set to https://www.wikidata.org/wiki/Lexeme:L123, which I find unexpected (I was first thinking of a bot, but that wouldn’t have a referrer)… is there some user script that loads the history (hundreds of revisions back), and therefore makes privileged API requests if a sysop using that script visits the page?

And I think I realized the answer to my own question while writing it: I bet it’s WikidataTrust.js by @Ricordisamoa, which annotates statements with the users who added them (a task that requires going through the whole history). :/

So, quick summary as I see it:

• L123 has one (1) bad revision which causes server errors when viewed
  • it is now impossible to create more revisions like it, so the only question is what to do about the existing revision
• we deleted the revision, in the expectation that no sysop would have a reason to view it again, which should effectively eliminate the error
• however, it turns out the API will directly try to return the content of deleted revisions if the user has the right to see it
  • unlike the normal interface where you have to click through a confirmation dialog to go to Special:Undelete first
• and, there is a user script used by some users with the right to see deleted revisions which will make such API requests

I’m not sure where to go from here, to be honest. I don’t think the user script could easily skip the bad revision range. Would it be reasonable to ask for the user script to be disabled on L123 entirely? Or should we try to fix the error server-side after all?

I think it's reasonable to ask that the script should have an exclusion list for high edit volume items / lexemes / properties that are used frequently for testing purposes, such as L123. As we already realized that this is just a case of a particular revision misbehaving since it had the misfortune to be persisted while the code was at a volatile state. Or, am I missing something here? What I'm trying to say is that I don't think we should make backend changes just to support one known edge case on one persisted revision.

Alright. @Ricordisamoa: how would you feel about disabling WikidataTrust on L123? (I’m thinking of something like: if ( mw.config.get( 'wbEntityId' ) === 'L123' ) { return; } )

(In theory, it would only need to be disabled for sysops, but it’s probably less confusing to just disable it for everyone. Perhaps a console.log() would also be nice, so people at least have some way to find out why the annotations aren’t showing up.)

I put an edit request for interface administrators on the talk page: https://www.wikidata.org/wiki/User_talk:Ricordisamoa/WikidataTrust.js#Disable_on_L123

This is still reporting errors: https://www.wikidata.org/w/index.php?title=Q4672180&oldid=410362640

@Lucas_Werkmeister_WMDE Do you think we should still close this as declined?

According to logstash the error hasn’t happened in the past 30 days, so yeah, let’s close it IMHO. If someone complains about excessive logspam in future, we can reconsider it.