Prevent creating saved objects with empty IDs #120693
Conversation
`create` now coalesces an empty ID into a randomly generated one. `update` and `incrementCounter` now throw a Bad Request error when an empty ID is used.
jportner
added
v8.0.0
release_note:skip
💚 Build Succeeded
Metrics [docs]
To update your PR or re-run it, just comment with: |
| if (!id) { | ||
| throw SavedObjectsErrorHelpers.createBadRequestError('id cannot be empty'); // prevent potentially upserting a saved object with an empty ID | ||
| } |
There was a problem hiding this comment.
If the intent of the PR is to avoid creating objects with empty ids, should we only throw if options.upsert is set, both for update and incrementCounter? OTOH it doesn't make sense to try to update an object with empty id, but in that case, maybe we should ensure this for all methods?
But the PR does what it's supposed to do, protect against introducing corrupted docs, so it's fine with me.
There was a problem hiding this comment.
Yeah, I wanted to minimize the changes but at least keep the responses consistent within the update method. I think we should probably ensure this for all methods (along with other ID validation) but I'd prefer to leave it for the follow-on issue 👍
jportner
added
auto-backport
|
This was originally supposed to make it into the 7.16.1 release but I think the backport merged a bit too late on Dec 9 and didn't make the cut: https://github.com/elastic/kibana/commits/v7.16.1 I'm changing the label to reflect that this actually shipped with 7.16.2: https://github.com/elastic/kibana/commits/v7.16.2?after=9b678a13a6a3f45286f1d21856a7536a9297f42f+34&branch=v7.16.2 |
Resolves #118957.
The SavedObjectsRepository no longer allows consumers to create saved objects with empty IDs.
Scenarios:
bulkCreatew/id: undefined-> generate a random object IDbulkCreatew/id: ''-> generate a random object IDcreatew/id: undefined-> generate a random object IDcreatew/id: ''-> generate a random object IDupdatew/id: undefined-> throw 400 Bad Requestupdatew/id: ''-> throw 400 Bad RequestincrementCounterw/id: undefined-> throw 400 Bad RequestincrementCounterw/id: ''-> throw 400 Bad RequestNotes:
bulkCreate, it always behaved this waycreateto match the behavior ofbulkCreateupdateandincrementCounterbecause these can use theupsertoption to create new saved objectsbulkUpdatebecause it doesn't support theupsertoptionupdate, I'm on the fence about itFurther validation of saved object IDs should be added in a follow-on issue (#105039). This issue is minimal in scope and is intended to prevent users from accidentally creating saved objects that can't be deserialized.