Harden saved object deserialization

[jportner]

This is a follow-on to #94616.

Saved object deserialization is not currently consistent -- the SavedObjectsSerializer class's isRawSavedObject() method is stricter than the rawToSavedObject() method. In other words, a raw object that isRawSavedObject() would return false for, may still be successfully deserialized by rawToSavedObject().

The simplest solution is to add a check in rawToSavedObject() to first check the outcome of rawToSavedObject(), and throw an error if that returned false.

[jportner]

In addition to the stricter namespace prefix validation, the isRawSavedObject() method is also stricter about the presence of the [type] field (e.g., the object's attributes).

Seeing as the current check doesn't actually check if the [type] field's value is valid (it just checks to see if it is present), I thought it might make also make sense to loosen that condition for isRawSavedObject(). Something like this:

Click to see diff

diff --git a/src/core/server/saved_objects/serialization/serializer.ts b/src/core/server/saved_objects/serialization/serializer.ts
index 6e8e6344745..922a86968c5 100644
--- a/src/core/server/saved_objects/serialization/serializer.ts
+++ b/src/core/server/saved_objects/serialization/serializer.ts
@@ -28,85 +28,85 @@ export class SavedObjectsSerializer {
 
   /**
    * @internal
    */
   constructor(registry: ISavedObjectTypeRegistry) {
     this.registry = registry;
   }
   /**
    * Determines whether or not the raw document can be converted to a saved object.
    *
    * @param {SavedObjectsRawDoc} doc - The raw ES document to be tested
    * @param {SavedObjectsRawDocParseOptions} options - Options for parsing the raw document.
    */
   public isRawSavedObject(doc: SavedObjectsRawDoc, options: SavedObjectsRawDocParseOptions = {}) {
     const { namespaceTreatment = 'strict' } = options;
     const { _id, _source } = doc;
     const { type, namespace } = _source;
     if (!type) {
       return false;
     }
     const { idMatchesPrefix } = this.parseIdPrefix(namespace, type, _id, namespaceTreatment);
-    return idMatchesPrefix && _source.hasOwnProperty(type);
+    return idMatchesPrefix;
   }
 
   /**
    * Converts a document from the format that is stored in elasticsearch to the saved object client format.
    *
    * @param {SavedObjectsRawDoc} doc - The raw ES document to be converted to saved object format.
    * @param {SavedObjectsRawDocParseOptions} options - Options for parsing the raw document.
    */
   public rawToSavedObject(
     doc: SavedObjectsRawDoc,
     options: SavedObjectsRawDocParseOptions = {}
   ): SavedObjectSanitizedDoc {
     if (!this.isRawSavedObject(doc, options)) {
       throw new Error(`Raw document '${doc._id}' is not a valid saved object`);
     }
     const { namespaceTreatment = 'strict' } = options;
     const { _id, _source, _seq_no, _primary_term } = doc;
     const {
       type,
       namespaces,
       originId,
       migrationVersion,
       references,
       coreMigrationVersion,
     } = _source;
 
     const version =
       _seq_no != null || _primary_term != null
         ? encodeVersion(_seq_no!, _primary_term!)
         : undefined;
     const { id, namespace } = this.trimIdPrefix(_source.namespace, type, _id, namespaceTreatment);
     const includeNamespace =
       namespace && (namespaceTreatment === 'lax' || this.registry.isSingleNamespace(type));
     const includeNamespaces = this.registry.isMultiNamespace(type);
 
     return {
       type,
       id,
       ...(includeNamespace && { namespace }),
       ...(includeNamespaces && { namespaces }),
       ...(originId && { originId }),
-      attributes: _source[type],
+      attributes: _source[type] ?? {},
       references: references || [],
       ...(migrationVersion && { migrationVersion }),
       ...(coreMigrationVersion && { coreMigrationVersion }),
       ...(_source.updated_at && { updated_at: _source.updated_at }),
       ...(version && { version }),
     };
   }
 
   /**
    * Converts a document from the saved object client format to the format that is stored in elasticsearch.
    *
    * @param {SavedObjectSanitizedDoc} savedObj - The saved object to be converted to raw ES format.
    */
   public savedObjectToRaw(savedObj: SavedObjectSanitizedDoc): SavedObjectsRawDoc {
     const {
       id,
       type,
       namespace,
       namespaces,
       originId,
       attributes,

Thoughts @elastic/kibana-core ?

Edit: I had to add the first part of this diff in 4421a06 due to CI failures.

[jportner]

@elasticmachine merge upstream

[mshustov]

Maybe we should show a reason for the error? There are several possible reasons: SO _source.type is not defined, id prefix doesn't match, probably we will have more in the future.

[jportner]

Good idea, done in cfa3520.

[jportner]

@elasticmachine merge upstream

[jportner]

I think the CI failure is transient, I'm going to retry it.

[jportner]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 6b7ca5a
• Storybooks Preview

Metrics [docs]

✅ unchanged

History

• 💔 Build #119852 failed e35a760
• 💔 Build #119678 failed 40e8143
• 💔 Build #119662 failed cfa3520
• 💔 Build #119663 failed ccd684e
• 💚 Build #115827 succeeded cfa3520

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[kibanamachine]

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.