fix(core): validate multi-column fields before splitting into physical columns

.changeset/tidy-otters-validate.md

@@ -0,0 +1,5 @@
+---
+'@opensaas/stack-core': patch
+---
+
+Fix multi-column fields (e.g. storage `image()`/`file()` in Keystone-parity mode) writing an unrecognised value silently instead of failing validation. The column split now runs after `validateFieldRules`, not before, at the top-level and nested write paths.

CONTEXT.md

@@ -31,7 +31,7 @@ The single module that runs the canonical, secured write sequence (hooks → val
 _Avoid_: operation handler, mutation service
 
 **Hook Pipeline**:
-The module that runs the transform+validate span of a write — list `resolveInput` → field `resolveInput` → list `validate` → field `validate` → built-in field rules — owning that order and the `resolvedData` threading through it. It throws a validation error (never silent) when a validate hook reports via `addValidationError` or a built-in field rule fails, and returns the transformed `resolvedData` on success. The Write Pipeline delegates this span to it; side-effect hooks (`beforeOperation`/`afterOperation`), access, writable-field filtering, persistence and Field Visibility stay in the Write Pipeline.
+The module that runs the transform+validate span of a write — list `resolveInput` → field `resolveInput` → list `validate` → field `validate` → built-in field rules → split multi-column fields — owning that order and the `resolvedData` threading through it. It throws a validation error (never silent) when a validate hook reports via `addValidationError` or a built-in field rule fails, and returns the transformed `resolvedData` on success. A multi-column field (e.g. storage `image()`/`file()` in Keystone-parity mode) is validated under its logical field key BEFORE it is split into its per-part physical columns, so an unrecognised value throws instead of being silently split into null/undefined columns (#789). The Write Pipeline delegates this span to it; side-effect hooks (`beforeOperation`/`afterOperation`), access, writable-field filtering, persistence and Field Visibility stay in the Write Pipeline.
 _Avoid_: validation service, input resolver
 
 **In-transaction hooks** (`beforeOperation` / `afterOperation`):

packages/core/src/access/field-access.ts

@@ -75,7 +75,7 @@
  * Simple filter matching for field-level access
  * Checks if an item matches a Prisma-like filter object
  */
 function matchesFilter(item: Record<string, unknown>, filter: Record<string, unknown>): boolean {
   for (const [key, condition] of Object.entries(filter)) {
     if (typeof condition === 'object' && condition !== null) {
       // Handle nested conditions like { equals: value }
@@ -128,12 +128,12 @@
   // Map each raw per-part column name contributed by a multi-column field
   // (e.g. storage image()/file() in Keystone-parity mode) back to its OWNING
   // declared field. These columns are injected into the write payload by the
-  // field's `splitColumns` AFTER resolveInput and are intentionally NOT declared
-  // as their own entries in `fieldConfigs`, so without this map they would trip
-  // the #564 undeclared-key reject below.
+  // field's `splitColumns`, AFTER validation (#789), and are intentionally NOT
+  // declared as their own entries in `fieldConfigs`, so without this map they
+  // would trip the #564 undeclared-key reject below.
   //
   // SECURITY (#568): a raw column must NOT be blanket-passed through. The hooks
-  // layer (`executeFieldResolveInputHooks`) only gates the owning field when the
+  // layer (`splitMultiColumnFields`) only gates the owning field when the
   // LOGICAL key (e.g. `media`) is present, because it iterates declared fields,
   // not data keys. A non-sudo caller who supplies the raw columns DIRECTLY
   // (`data: { media_url, media_size }`) never produces that logical key, so that
@@ -184,8 +184,8 @@
     // they must NOT be blanket-passed through either: gate each one by its
     // OWNING field's write access (see the SECURITY note where the map is built).
     // This is the real gate for callers who supply the raw columns directly,
-    // because the logical-key gate in `executeFieldResolveInputHooks` never fires
-    // for them. Denied (non-sudo) throws — same fail-loud behaviour as a denied
+    // because the logical-key gate in `splitMultiColumnFields` never fires for
+    // them. Denied (non-sudo) throws — same fail-loud behaviour as a denied
     // declared field (#568); allowed (or sudo, via `checkFieldAccess`) passes
     // through, preserving the legitimate multi-column write path.
     const splitColumnOwner = splitColumnOwners.get(fieldName)

packages/core/src/access/multi-column-read-write.test.ts

@@ -1,6 +1,6 @@
 import { describe, it, expect } from 'vitest'
 import { filterReadableFields } from './field-visibility.js'
-import { executeFieldResolveInputHooks } from '../hooks/index.js'
+import { executeFieldResolveInputHooks, splitMultiColumnFields } from '../hooks/index.js'
 import type { FieldConfig } from '../config/types.js'
 import type { AccessContext, FieldAccess } from './types.js'
 
@@ -10,6 +10,13 @@ import type { AccessContext, FieldAccess } from './types.js'
  * field-agnostic: they assert that ANY field implementing
  * getColumnNames/assembleColumns/splitColumns is assembled on read (raw columns
  * stripped) and split on write.
+ *
+ * The write side is split across two phases (#789): `executeFieldResolveInputHooks`
+ * (Phase 1.5) resolves the field's value under its LOGICAL key only — no split —
+ * so that validation (Phase 2-3, not exercised by these field-agnostic unit
+ * tests; see the Hook Pipeline / Write Pipeline test suites) runs against the
+ * logical value first. `splitMultiColumnFields` (Phase 4) then replaces the
+ * logical key with its physical per-part columns, AFTER validation has passed.
  */
 
 // A minimal multi-column field: two physical columns `m_url` and `m_size`
@@ -113,10 +120,10 @@ describe('multi-column read assembly (filterReadableFields)', () => {
   })
 })
 
-describe('multi-column write split (executeFieldResolveInputHooks)', () => {
+describe('multi-column resolveInput (executeFieldResolveInputHooks) does NOT split (#789)', () => {
   const fields = { media: multiColumnField() }
 
-  it('splits the logical value into per-part columns and removes the logical key', async () => {
+  it('resolves the field under its LOGICAL key and leaves it unsplit', async () => {
     const inputData = { media: { url: 'https://x/y.jpg', size: 99 } }
     const result = await executeFieldResolveInputHooks(
       inputData,
@@ -126,33 +133,81 @@ describe('multi-column write split (executeFieldResolveInputHooks)', () => {
       makeContext(),
       'Post',
     )
+    // Still under the logical key — no split, no per-part columns yet.
+    expect(result).toEqual({ media: { url: 'https://x/y.jpg', size: 99 } })
+    expect('m_url' in result).toBe(false)
+    expect('m_size' in result).toBe(false)
+  })
+
+  it('passes an unrecognised value through unsplit, under the logical key', async () => {
+    // The field's own resolveInput fallback (see fixture: "unknown → return as
+    // is and let validation catch it") must be able to hand this value to
+    // validation BEFORE any split happens.
+    const inputData = { media: 'not-a-valid-shape' }
+    const result = await executeFieldResolveInputHooks(
+      inputData,
+      { ...inputData },
+      fields,
+      'update',
+      makeContext(),
+      'Post',
+    )
+    expect(result).toEqual({ media: 'not-a-valid-shape' })
+  })
+
+  it('does not touch the field when it is absent from the write', async () => {
+    const inputData = { title: 'no media in payload' }
+    const result = await executeFieldResolveInputHooks(
+      inputData,
+      { ...inputData },
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any -- inline field configs
+      { ...fields, title: { type: 'text' } as any },
+      'update',
+      makeContext(),
+      'Post',
+    )
+    expect(result).toEqual({ title: 'no media in payload' })
+    expect('media' in result).toBe(false)
+  })
+})
+
+describe('multi-column write split (splitMultiColumnFields, AFTER validation — #789)', () => {
+  const fields = { media: multiColumnField() }
+
+  it('splits the logical value into per-part columns and removes the logical key', async () => {
+    const inputData = { media: { url: 'https://x/y.jpg', size: 99 } }
+    const result = await splitMultiColumnFields(
+      inputData,
+      { ...inputData },
+      fields,
+      'create',
+      makeContext(),
+    )
     expect(result).toEqual({ m_url: 'https://x/y.jpg', m_size: 99 })
     expect('media' in result).toBe(false)
   })
 
   it('splitting null clears all per-part columns', async () => {
     const inputData = { media: null }
-    const result = await executeFieldResolveInputHooks(
+    const result = await splitMultiColumnFields(
       inputData,
       { ...inputData },
       fields,
       'update',
       makeContext(),
-      'Post',
     )
     expect(result).toEqual({ m_url: null, m_size: null })
   })
 
   it('does not touch the columns when the logical field is absent from the write', async () => {
     const inputData = { title: 'no media in payload' }
-    const result = await executeFieldResolveInputHooks(
+    const result = await splitMultiColumnFields(
       inputData,
       { ...inputData },
       // eslint-disable-next-line @typescript-eslint/no-explicit-any -- inline field configs
       { ...fields, title: { type: 'text' } as any },
       'update',
       makeContext(),
-      'Post',
     )
     expect(result).toEqual({ title: 'no media in payload' })
     expect('m_url' in result).toBe(false)
@@ -163,13 +218,12 @@ describe('multi-column write split respects field-level write access', () => {
   it('does NOT write any per-part columns when update access is denied', async () => {
     const fields = { media: multiColumnField({ update: () => false }) }
     const inputData = { media: { url: 'https://x/y.jpg', size: 99 } }
-    const result = await executeFieldResolveInputHooks(
+    const result = await splitMultiColumnFields(
       inputData,
       { ...inputData },
       fields,
       'update',
       makeContext(),
-      'Post',
     )
     // The logical key is dropped (it is not a real column) AND none of its
     // per-part columns are written — identical to how filterWritableFields
@@ -183,13 +237,12 @@ describe('multi-column write split respects field-level write access', () => {
   it('does NOT write any per-part columns when create access is denied', async () => {
     const fields = { media: multiColumnField({ create: () => false }) }
     const inputData = { media: { url: 'https://x/y.jpg', size: 99 } }
-    const result = await executeFieldResolveInputHooks(
+    const result = await splitMultiColumnFields(
       inputData,
       { ...inputData },
       fields,
       'create',
       makeContext(),
-      'Post',
     )
     expect(result).toEqual({})
     expect('m_url' in result).toBe(false)
@@ -198,13 +251,12 @@ describe('multi-column write split respects field-level write access', () => {
   it('still splits/writes the columns when write access is granted', async () => {
     const fields = { media: multiColumnField({ update: () => true, create: () => true }) }
     const inputData = { media: { url: 'https://x/y.jpg', size: 99 } }
-    const result = await executeFieldResolveInputHooks(
+    const result = await splitMultiColumnFields(
       inputData,
       { ...inputData },
       fields,
       'update',
       makeContext(),
-      'Post',
     )
     expect(result).toEqual({ m_url: 'https://x/y.jpg', m_size: 99 })
     expect('media' in result).toBe(false)
@@ -214,41 +266,38 @@ describe('multi-column write split respects field-level write access', () => {
     // A field that denies `update` must still be writable on `create`.
     const fields = { media: multiColumnField({ update: () => false }) }
     const inputData = { media: { url: 'https://x/y.jpg', size: 99 } }
-    const result = await executeFieldResolveInputHooks(
+    const result = await splitMultiColumnFields(
       inputData,
       { ...inputData },
       fields,
       'create',
       makeContext(),
-      'Post',
     )
     expect(result).toEqual({ m_url: 'https://x/y.jpg', m_size: 99 })
   })
 
   it('sudo bypasses the field-access gate and still splits', async () => {
     const fields = { media: multiColumnField({ update: () => false }) }
     const inputData = { media: { url: 'https://x/y.jpg', size: 99 } }
-    const result = await executeFieldResolveInputHooks(
+    const result = await splitMultiColumnFields(
       inputData,
       { ...inputData },
       fields,
       'update',
       makeContext({ isSudo: true }),
-      'Post',
     )
     expect(result).toEqual({ m_url: 'https://x/y.jpg', m_size: 99 })
   })
 
   it('a multi-column field WITHOUT field-level access splits exactly as before', async () => {
     const fields = { media: multiColumnField() }
     const inputData = { media: { url: 'https://x/y.jpg', size: 99 } }
-    const result = await executeFieldResolveInputHooks(
+    const result = await splitMultiColumnFields(
       inputData,
       { ...inputData },
       fields,
       'update',
       makeContext(),
-      'Post',
     )
     expect(result).toEqual({ m_url: 'https://x/y.jpg', m_size: 99 })
   })

packages/core/src/context/hook-pipeline.ts

@@ -6,6 +6,7 @@ import {
   executeFieldResolveInputHooks,
   executeFieldValidateHooks,
   validateFieldRules,
+  splitMultiColumnFields,
   ValidationError,
 } from '../hooks/index.js'
 import { applyCreateDefaults } from './apply-defaults.js'
@@ -66,12 +67,17 @@ export interface HookPipeline {
  *     → list `validate`
  *     → field `validate`
  *     → built-in field rules (`validateFieldRules`)
+ *     → split multi-column fields (`splitMultiColumnFields`)
  *
  * Contract preserved exactly:
  *   - `resolvedData` starts as `inputData` and is threaded through each phase;
  *   - validate hooks report failures via `addValidationError` → THROW
  *     `ValidationError` (never silent);
  *   - built-in field rule failures THROW `ValidationError`;
+ *   - a multi-column field (e.g. storage image()/file() in Keystone-parity
+ *     mode) is validated under its LOGICAL key BEFORE it is split into
+ *     physical columns (#789) — an unrecognised value throws instead of being
+ *     silently split into null/undefined columns;
  *   - on success returns the transformed `resolvedData`.
  */
 async function runHookPipeline(args: HookPipelineArgs): Promise<HookPipelineResult> {
@@ -160,6 +166,19 @@ async function runHookPipeline(args: HookPipelineArgs): Promise<HookPipelineResu
     throw new ValidationError(validation.errors, validation.fieldErrors)
   }
 
+  // ── Phase 4: split multi-column fields into physical columns ──────────────
+  // Only reached once the logical value has passed validation (#789). Replaces
+  // each multi-column field's logical key with its per-part columns, gated by
+  // the field's own write access (see `splitMultiColumnFields`).
+  resolvedData = await splitMultiColumnFields(
+    inputData,
+    resolvedData,
+    listConfig.fields,
+    operation,
+    context,
+    item,
+  )
+
   return { resolvedData }
 }
 

packages/core/src/context/nested-operations.ts

@@ -12,6 +12,7 @@ import {
   executeFieldAfterOperationHooks,
   executeFieldValidateHooks,
   validateFieldRules,
+  splitMultiColumnFields,
   ValidationError,
 } from '../hooks/index.js'
 import { getDbKey } from '../lib/case-utils.js'
@@ -306,6 +307,16 @@ async function processNestedCreate(
         throw new ValidationError(validation.errors, validation.fieldErrors)
       }
 
+      // 5.5 Split multi-column fields into physical columns (#789). Only
+      // reached once the logical value has passed validation above.
+      resolvedData = await splitMultiColumnFields(
+        item,
+        resolvedData,
+        relatedListConfig.fields,
+        'create',
+        context,
+      )
+
       // 6. Filter writable fields
       const filtered = await filterWritableFields(
         resolvedData,
@@ -645,6 +656,17 @@ async function processNestedUpdate(
         throw new ValidationError(validation.errors, validation.fieldErrors)
       }
 
+      // Split multi-column fields into physical columns (#789). Only reached
+      // once the logical value has passed validation above.
+      resolvedData = await splitMultiColumnFields(
+        updateData,
+        resolvedData,
+        relatedListConfig.fields,
+        'update',
+        context,
+        originalItem,
+      )
+
       // Filter writable fields
       const filtered = await filterWritableFields(
         resolvedData,