Wikibase REST and action api allow adding statements with wikibase-item values that are not valid item ids
Closed, ResolvedPublicBUG REPORT
Actions

Assigned To

None

Authored By

	Tarrow
	Jun 6 2023, 11:07 AM

Description

Steps to replicate the issue (include links if applicable):
Using the rest API make are request like

curl -X 'POST' \
  'https://www.wikidata.org/w/rest.php/wikibase/v0/entities/items/Q4115189/statements' \
  -H 'accept: application/json' \
  -H 'Content-Type: application/json' \
  -d '{
        "statement": {
                "rank": "normal",
                "property": {
                        "id": "P369"
                },
                "value": { "type": "value", "content": "Itemasdfadsf:Q42"
                }
        },
        "bot": false,
        "comment": "API Editing investigating how item ids are validated"
}'
{"id":"Q4115189$8D8522B2-7E68-49A8-B310-B15F0964EDA6","rank":"normal","qualifiers":[],"references":[],"property":{"id":"P369","data-type":"wikibase-item"},"value":{"type":"value","content":"Itemasdfadsf:Q42"}}

Or using the action api like:

http://carrotwiki.wbaas.localhost/w/api.php?action=wbeditentity&format=json&id=Q5&token=5189b89a71be3b3919b44d978c9e06de647f03db%2B%5C&data=%7B%0A%09%22type%22%3A%20%22item%22%2C%0A%09%22claims%22%3A%20%7B%0A%09%09%22P1%22%3A%20%5B%7B%0A%09%09%09%22mainsnak%22%3A%20%7B%0A%09%09%09%09%22snaktype%22%3A%20%22value%22%2C%0A%09%09%09%09%22property%22%3A%20%22P1%22%2C%0A%09%09%09%09%22hash%22%3A%20%22d95bd3fe3e52cdad57730bfbb6c998da0408543c%22%2C%0A%09%09%09%09%22datavalue%22%3A%20%7B%0A%09%09%09%09%09%22value%22%3A%20%7B%0A%09%09%09%09%09%09%22entity-type%22%3A%20%22item%22%2C%0A%09%09%09%09%09%09%22numeric-id%22%3A%202%2C%0A%09%09%09%09%09%09%22id%22%3A%20%22Itefmasdf%3AQ2%22%0A%09%09%09%09%09%7D%2C%0A%09%09%09%09%09%22type%22%3A%20%22wikibase-entityid%22%0A%09%09%09%09%7D%2C%0A%09%09%09%09%22datatype%22%3A%20%22wikibase-item%22%0A%09%09%09%7D%2C%0A%09%09%09%22type%22%3A%20%22statement%22%2C%0A%09%09%09%22rank%22%3A%20%22normal%22%0A%09%09%7D%5D%0A%09%7D%2C%0A%09%22sitelinks%22%3A%20%7B%7D%0A%7D

What happens?:

a statement is added and then appears in the UI as shown below:

What should have happened instead?:
The API should have errored

Software version (skip for WMF-hosted wikis like Wikipedia):
Live on Wikidata, also on Wikibase Cloud

Details

Subject	Repo	Branch	Lines +/-
Drop support for repo names in entity IDs	mediawiki/extensions/Wikibase	master	+28 -114
Add entity ID validator to assert empty repo name	mediawiki/extensions/Wikibase	master	+46 -0
Log warning when repo name is nonempty	mediawiki/extensions/Wikibase	master	+11 -0
Remove tests relying on foreign entity IDs	mediawiki/extensions/WikibaseLexeme	master	+10 -27
Remove tests relying on foreign entity IDs	mediawiki/extensions/WikibaseMediaInfo	master	+0 -1
Remove tests relying on foreign entity IDs	mediawiki/extensions/WikibaseLexeme	master	+5 -13

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Open		None	T338301 Wikibase allows edits with nonempty “repository names” in entity IDs
		Resolved	BUG REPORT	None	T338223 Wikibase REST and action api allow adding statements with wikibase-item values that are not valid item ids

Event Timeline

Tarrow created this task.Jun 6 2023, 11:07 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 6 2023, 11:07 AM

WMDE-leszek subscribed.Jun 6 2023, 11:07 AM

Tarrow renamed this task from Wikibase REST and action api allow adding statements with wikibase-item values that are very malformed to Wikibase REST and action api allow adding statements with wikibase-item values that are not valid item ids.Jun 6 2023, 11:08 AM

Tarrow mentioned this in T317109: 👶On Wikibase.cloud Cradle instance some entity searches erroneously search wikidata.org.Jun 6 2023, 11:21 AM

Triaging this high because invalid data is pretty bad.

I can’t reproduce this using the action API – both wbeditentity (as mentioned in the task description) and wbsetclaimvalue give me an error like this:

Okay, it is reproducible with { "entity-type": "item", "id": "foobar:Q50" } as the value in the wbeditentity payload. (If you leave foobar empty, i.e. "id": ":Q50", you just get a normal, working statement.) I expect this is related to the remnants of “entity IDs with repository names” that we still have in the code (before we decided to do federation differently), ~~and I have a feeling it’s pretty much unrelated to the REST API issue.~~ (edit: I might have misunderstood the REST API issue; perhaps it’s the same)

Change 927654 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Wikibase@master] Drop support for repo names in entity IDs

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

gerritbot added a project: Patch-For-Review.Jun 6 2023, 12:48 PM

In T338223#8905912, @gerritbot wrote:

Change 927654 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Wikibase@master] Drop support for repo names in entity IDs

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

This would be a “brute force” solution; it seems to work well in local testing (the thrown exception gets caught and turned into “can’t parse this”, like other invalid entity IDs – i.e. it doesn’t become an internal server error), though it’s probably up for debate whether we want to go with this or not.

Apparently we had already planned to remove support for “old federation” methods: T291823: Remove "old federation" methods from EntityId

Change 927679 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Wikibase@master] Log warning when repo name is nonempty

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

Change 927706 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/WikibaseLexeme@master] Remove tests relying on foreign entity IDs

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

Change 927707 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/WikibaseLexeme@master] Assert that foreign entity IDs are no longer supported

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

Change 927724 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/WikibaseMediaInfo@master] Remove tests relying on foreign entity IDs

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

Change 927726 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/WikibaseMediaInfo@master] Assert that foreign entity IDs are no longer supported

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

(While I’ve now gone down a bit further into the “let’s disallow nonempty repo names entity IDs” rabbit hole, that doesn’t have to be the only solution. Maybe it would be simpler to instead implement another validator that asserts the repo name is empty in new edits, for instance?)

Lucas_Werkmeister_WMDE mentioned this in T338255: Lexemes (some?) are not properly indexed by CirrusSearch.Jun 7 2023, 9:13 AM

Change 927996 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Wikibase@master] Add entity ID validator to assert empty repo name

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

In T338223#8906424, @Lucas_Werkmeister_WMDE wrote:

Maybe it would be simpler to instead implement another validator that asserts the repo name is empty in new edits, for instance?

Tried that approach as well – seems to work locally (though this, unlike the other approach, probably doesn’t cover uses of entity IDs outside of data values, e.g. as the lexeme language as in T338255), and hopefully it’ll require fewer test changes to pass CI.

Lucas_Werkmeister_WMDE added a parent task: T338301: Wikibase allows edits with nonempty “repository names” in entity IDs.Jun 7 2023, 10:06 AM

Change 927706 merged by jenkins-bot:

[mediawiki/extensions/WikibaseLexeme@master] Remove tests relying on foreign entity IDs

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

Change 927724 merged by jenkins-bot:

[mediawiki/extensions/WikibaseMediaInfo@master] Remove tests relying on foreign entity IDs

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

ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.13; 2023-06-13).Jun 8 2023, 9:01 AM

Change 928550 had a related patch set uploaded (by Jakob; author: Jakob):

[mediawiki/extensions/WikibaseLexeme@master] Remove tests relying on foreign entity IDs

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

Jakob_WMDE subscribed.Jun 8 2023, 1:51 PM

Change 928550 merged by jenkins-bot:

[mediawiki/extensions/WikibaseLexeme@master] Remove tests relying on foreign entity IDs

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

Change 927679 abandoned by Lucas Werkmeister (WMDE):

[mediawiki/extensions/Wikibase@master] Log warning when repo name is nonempty

Reason:

don’t think we’ll be pursuing this

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

Change 927996 abandoned by Lucas Werkmeister (WMDE):

[mediawiki/extensions/Wikibase@master] Add entity ID validator to assert empty repo name

Reason:

I46b7eff7cf was merged so I doubt we’ll be pursuing this approach further

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

Change 927654 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Drop support for repo names in entity IDs

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

ReleaseTaggerBot edited projects, added MW-1.41-notes (1.41.0-wmf.16; 2023-07-04); removed MW-1.41-notes (1.41.0-wmf.13; 2023-06-13).Jun 28 2023, 12:00 PM

Hallo, we're clearing up REST API tickets and is there anything left open here from our end?

In T338223#9063073, @Ifrahkhanyaree wrote:

Hallo, we're clearing up REST API tickets and is there anything left open here from our end?

Nope, pretty sure we're done here. The curl command from the description now gets a 400 statement-data-invalid-field error, as it should. The remaining clean-up work around the code that caused this is tracked in T291823.

Jakob_WMDE removed Jakob_WMDE as the assignee of this task.Aug 2 2023, 2:40 PM

	F37095468: image.png
	Jun 6 2023, 12:29 PM

Wikibase REST and action api allow adding statements with wikibase-item values that are not valid item idsClosed, ResolvedPublicBUG REPORTActions

Description

Details

Related ObjectsSearch...

Event Timeline

Wikibase REST and action api allow adding statements with wikibase-item values that are not valid item ids
Closed, ResolvedPublicBUG REPORT
Actions

Related Objects
Search...