Bulk writer lets a _-named schema property overwrite metadata columns #4668

Found during a Strict review of #4580 (release development → beta). Present on development at 8e7bbd87; introduced in #4661.

Impact

A schema property whose name is a metadata column (_owner, _organisation, _status, …) passes skippedOnWrite(), and line 233 writes the request's value into that column after line 178 stamped it.

The chain

Before #4661 the bulk writer skipped every _ key. Now MagicMetadataColumn::skippedOnWrite() lets a key through whenever the schema declares it, and lines 221-233 write it to sanitizeColumnName($key). For a property whose sanitised name is a metadata column (_owner, _organisation, _authorization, _status, _deleted, _uuid, _register, _schema, _locked, _frozen, _archived, _expires …; also _Owner, __owner, _owner_, which sanitise to _owner), the client value is written into the metadata column AFTER the authoritative value was set earlier in the same row ($preparedObject['_owner'] = $selfData['owner'], line 178), so it wins.

The read side already knows this collision: MagicMetadataColumn::field() keeps such a column as metadata ("the metadata column then wins"), and buildTableColumnsFromSchema() moves the property to a disambiguated column (_owner_1). The write side ignores both, so the value lands in the metadata column and the property itself reads back null.

SaveObjects::stripSelfInjectionFields() only removes top-level _owner/_organisation/_authorization for non-admins; keys inside object, and every other metadata name, pass. _authorization (per-object RBAC), _organisation (multitenancy) and _status (draft visibility, DraftStatusPolicy) are the security-relevant ones.

Precondition: a schema declares such a property. Not verified on a running instance (read-only review). The single-object path (MagicMapper::prepareObjectDataForTable, line 4798) has the same pre-existing write; #4661 extended it to bulk.

Suggested fix: in skippedOnWrite() (and the single-object loop) skip a key whose sanitised column is a metadata column, i.e. apply the same rule as field(): isset($schemaProperties[$key]) && $this->attribute(field: substr($column, 1)) === null; better, write to the column from buildTableColumnsFromSchema()'s map (the disambiguated name) instead of sanitizeColumnName(). Separately, refuse at schema save a property whose sanitised name collides with a metadata column.

Done when:

• MagicMetadataColumnTest: skippedOnWrite('_owner', ['_owner' => [...]]) is true (or the column resolves to _owner_1); red today
• a MagicBulkHandler test: a schema declaring _authorization and an object carrying object._authorization produces a prepared row whose _authorization is not the client value

Also found independently by the cross-PR pass: #4566 stamps _owner at line 178, then this loop writes the declared key into the same column.

Verified against 8e7bbd87eede51a19cd16b1eda5f1f9cbe21dfc0.

Reproduction

Verified by reading the code at 8e7bbd87; not executed.

Done when

• MagicMetadataColumnTest: a declared _owner is not written to the metadata column (red today)
• MagicBulkHandler test: client _authorization does not reach the prepared row's _authorization

Review thread

#4580 (comment)