Skip to content

Fix: importer validates meta fields - #232

Open
DocArmoryTech wants to merge 4 commits into
cerebrate-project:mainfrom
DocArmoryTech:fix/importer-validates-meta-fields
Open

Fix: importer validates meta fields#232
DocArmoryTech wants to merge 4 commits into
cerebrate-project:mainfrom
DocArmoryTech:fix/importer-validates-meta-fields

Conversation

@DocArmoryTech

Copy link
Copy Markdown
Contributor

Fixes #231

What

cake importer wrote meta-field values without ever validating them. Regexes,
values_list and type handlers were all bypassed, so an import could store
values that the same template rejects through every other route - and that
migrateMetafieldsToNewestTemplate() will later refuse to carry to a newer
template version.

Cause

marshalData() builds each meta-field by assigning properties to an empty
entity rather than marshalling input through it:

$metaEntity = $this->MetaFields->newEmptyEntity();
$metaEntity->field = $fieldName;
...
$metaEntity->value = $fieldValue;

Direct assignment skips marshalling, and save() applies rules, not
validators, so the validMetaField rule declared in
MetaFieldsTable::validationDefault() never ran at any point.

The intent to validate was already present but could not fire:
saveMetaFields() checks $metaEntity->hasErrors(), and hasErrors() is
populated by marshalling - on an entity that was never marshalled it is always
empty.

Approach

Each value is validated against its meta-template field as it is assigned, via
MetaFieldsTable::isValidMetaFieldForMetaTemplateField(). Failures are
reported the way parent-entity errors already are - every failure listed, then
abort before anything is written.

patchEntity() was deliberately not used - uuid is required on create but
is only populated by UUIDBehavior::beforeSave(), so marshalling at this point
fails on a field the importer is not responsible for supplying.

MetaTemplateFields are now fetched as entities rather than an id list, since
validation needs the type, regex and values_list rather than the primary
key. The existing $metaTemplateFieldsMapping is derived from them, so the
surrounding code is unchanged.

Behaviour change, and workaround

Imports that previously "succeeded" while writing invalid data will now fail.
That is the intent, but it is a behaviour change and existing pipelines may
depend on the current leniency - particularly where an upstream source is
untidier than the template anticipates.

--skip-validation downgrades failures to warnings and imports anyway. The
abort message points at it, and both mention that values failing validation
cannot be migrated to a later template version.

Verifying the behaviour

  1. Load a template with a regex-constrained field - ENISA CSIRT Network constrains website to https?:\/\/.+.
  2. Map that field to a failing value, e.g. www.example.org.
  3. ./bin/cake importer <config> <source> - before this change it reports success and stores the value; after, it reports the failing value and aborts.
  4. --skip-validation restores the previous behaviour, with warnings.

iglocska and others added 4 commits June 11, 2026 09:45
The importer wrote meta-field values without ever validating them. Regexes,
values_list and type handlers were all bypassed, so an import could store
values the same template rejects through every other route.

`marshalData()` builds each meta-field by assigning properties to an
empty entity rather than marshalling input through it. Direct assignment
skips marshalling, and `save()` applies rules, not validators, so the
`validMetaField` rule declared in `MetaFieldsTable::validationDefault()` never
ran at any point.

The intent to validate was already there but could not fire.
`saveMetaFields()` checks `$metaEntity->hasErrors()`, and `hasErrors()` is
populated by marshalling - on an entity that was never marshalled it is
always empty.

As a result, values that fail their template's validation cannot be carried to a newer template version.
`migrateMetafieldsToNewestTemplate()` correctly refuses them, so the records
are stranded on a superseded version with no non-destructive way forward.

This validates each value against its meta-template field as it is
assigned, and reports failures the way parent-entity errors are already
reported -- listing each one, then aborting before anything is written.

`patchEntity()` is deliberately not used: `uuid` is required on create but is
only populated by `UUIDBehavior::beforeSave()`, so marshalling would fail on a field the importer is not responsible for supplying.
Validating the value directly seemed like the the narrower change.

`MetaTemplateFields` are now fetched as entities rather than an `id` list,
since validation needs the `type`, `regex` and `values_list`.

Adds `--skip-validation`, which downgrades failures to warnings and imports
anyway, for operators whose upstream sources are untidier than the template
anticipates. Imports that previously succeeded while writing invalid data
will otherwise now fail, which is the point, but it is a behaviour change
and existing pipelines may depend on the current leniency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cake importer writes meta-field values without validating them

2 participants