diff --git a/.changeset/24131-core-required-emptiness.md b/.changeset/24131-core-required-emptiness.md new file mode 100644 index 00000000000..c71578d32b2 --- /dev/null +++ b/.changeset/24131-core-required-emptiness.md @@ -0,0 +1,11 @@ +--- +"@objectstack/core": minor +--- + +feat(core): export the emptiness rule a `required` declaration is judged by — `isEmptyForRequired(def, value)` and `isMissing(value)` + +Clause-②: yes (widening) + +- **New exports from `@objectstack/core`'s `.` entry.** `isMissing(value)` is `true` for `undefined`, `null`, and a string that is empty or only whitespace. `isEmptyForRequired(def, value)` is `isMissing`, plus an empty list `[]` on a field whose DECLARED value is a set (`@objectstack/spec/data`'s `isMultiValueField`: `tags`, `multiselect`, `checkboxes`, or a `select` / `radio` / `lookup` / `user` / `file` / `image` field flagged `multiple: true`). `def` is a `ValueShapeFieldDef` (`{ type, multiple? }`). A structured-JSON field's `[]` is a document, not an empty value. +- **Why it moved.** It was private to `@objectstack/objectql`'s record validator, where the object-level `required` check runs. `@objectstack/rest`'s anonymous public-form submit route now judges a FORM's `required` with it, and rest does not depend on the engine at runtime. So the one rule lives in the package both depend on, and the two doors call the same field missing. +- **Unchanged:** the rule itself, byte for byte — the engine's object-level `required` refusal answers exactly what it did. diff --git a/.changeset/24131-objectql-required-emptiness-import.md b/.changeset/24131-objectql-required-emptiness-import.md new file mode 100644 index 00000000000..9153b9ec995 --- /dev/null +++ b/.changeset/24131-objectql-required-emptiness-import.md @@ -0,0 +1,10 @@ +--- +"@objectstack/objectql": patch +--- + +refactor(objectql): the record validator reads `isMissing` / `isEmptyForRequired` from `@objectstack/core` instead of private copies + +Clause-②: no + +- **No behaviour change.** The object-level `required` check, the `required_cleared` PATCH check, the blank-to-`null` normalisation of typed columns and the value-shape scan judge "missing" exactly as before; the private copies of the two predicates are deleted and `@objectstack/core`'s exports, moved there unchanged, are imported at every read site. +- **Why:** `@objectstack/rest`'s public-form submit route now judges a form's `required` by the same rule, and it cannot import the engine. diff --git a/.changeset/24131-rest-public-form-required.md b/.changeset/24131-rest-public-form-required.md new file mode 100644 index 00000000000..23be29afc7e --- /dev/null +++ b/.changeset/24131-rest-public-form-required.md @@ -0,0 +1,14 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest)!: an anonymous public-form submit that leaves a form-required field empty answers `400 VALIDATION_FAILED` naming the field, and stores nothing + +Clause-②: no (narrowing) + + + +- **What changed.** `POST /forms/:slug/submit` now enforces the form's own `required`: a section entry `{ field: 'email', required: true }` is judged before the write, with the rule the engine's object-level check uses (`isEmptyForRequired` from `@objectstack/core`). A submission whose value for such a field is absent, `null`, `''`, only whitespace, or `[]` on a multi-value field is refused, and no row is created. Before, the engine judged only the fields the OBJECT requires, so a field only the form required answered `201` and was stored empty — while the rendered form refused the same submission. +- **The refusal** is the object-level one's envelope on this route, key for key: `400` with top-level `code: 'VALIDATION_FAILED'`, `error` (the messages), and `fields[]` of `{ field, code: 'required', message, label }`, worded by the same validation catalog (for example `Email is required`). Every empty form-required field is named in one refusal, in the form's order. +- **BREAKING — a narrowing, and who it affects.** A script or integration that posts to a public form and omits a field the form marks `required: true` now gets `400` instead of `201`. The fix is to send that field. Nothing that was refused before is accepted now. +- **Judged only where the form asks.** An entry the form hides (`hidden: true`), or shows only conditionally (`visibleWhen` on the entry or on its section), is not judged — nothing server-side evaluates a form-view `visibleWhen`. A server-managed anchor (`owner_id`, `organization_id`, …) is never written by this route, so it is never judged. An object `defaultValue` or a `beforeInsert` hook does not satisfy a field the visitor omitted: the route judges what the visitor sent. diff --git a/content/docs/ui/public-data-collection.mdx b/content/docs/ui/public-data-collection.mdx index f992e62b066..ecea0119dce 100644 --- a/content/docs/ui/public-data-collection.mdx +++ b/content/docs/ui/public-data-collection.mdx @@ -44,7 +44,7 @@ POST /api/v1/forms/contact-us/submit → INSERT one showcase_inquiry ### 2. The field whitelist *is* the security boundary -Only the fields listed in `sections[].fields` are accepted on submit; everything else is stripped server-side. Server-controlled fields (`status`, `source`, owner stamps) are set by a defaults hook, never by the submitter. The submit route derives a narrow `publicFormGrant: { object: 'showcase_inquiry' }` — create + read-back of the row it just made, and nothing else. +Only the fields listed in `sections[].fields` are accepted on submit; everything else is stripped server-side. The submit route also enforces each entry's `required: true` before anything is written — a submission that leaves such a field absent, `null`, blank, or an empty list on a multi-value field is refused with `400 VALIDATION_FAILED` naming the field — except on an entry the form hides (`hidden: true`) or shows only conditionally (`visibleWhen` on the entry or its section), which nothing server-side evaluates. Server-controlled fields (`status`, `source`, owner stamps) are set by a defaults hook, never by the submitter. The submit route derives a narrow `publicFormGrant: { object: 'showcase_inquiry' }` — create + read-back of the row it just made, and nothing else. System-managed anchors (`owner_id`, `organization_id`, audit columns, `id`) are **never** accepted from a public submission — not even when a section declares them, and not through the zero-sections all-fields fallback. The route filter and the data layer enforce the same set (`PUBLIC_FORM_SERVER_MANAGED_FIELDS` from `@objectstack/spec/security`), so an anonymous visitor cannot forge record ownership or tenant placement regardless of how the form is declared (#3022). diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 131fb58cdf4..c139b1055a0 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -157,6 +157,13 @@ export * from './utils/temporal-storage-form.js'; // because those two do not depend on each other at runtime. export * from './utils/multi-value-storage-form.js'; +// [#24131] …and the emptiness rule a `required` declaration is judged by. +// `@objectstack/objectql`'s record validator judges an object field's +// `required` with it, and `@objectstack/rest`'s public-form submit door the +// form's own `required` — one rule, here because rest does not depend on the +// engine at runtime. +export * from './utils/required-emptiness.js'; + // [#21007] …and the refusal a scalar comparison gets on a field stored as a // JSON column: the operator set and the words. `driver-sql` refuses it on // `where`, and `@objectstack/objectql` on the per-aggregation `filter` it diff --git a/packages/core/src/utils/required-emptiness.test.ts b/packages/core/src/utils/required-emptiness.test.ts new file mode 100644 index 00000000000..7ac3f244d01 --- /dev/null +++ b/packages/core/src/utils/required-emptiness.test.ts @@ -0,0 +1,79 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24131] The emptiness rule `required` is judged by, pinned per arm, and + * read through core's `.` entry — the specifier both of its doors import + * (`@objectstack/objectql`'s record validator, `@objectstack/rest`'s public-form + * submit route). + * + * The arms are the ones the rule moved out of `record-validator.ts` with: + * absent, `null`, `''`, a whitespace-only string, `[]` on a field whose + * declared value is a set (empty), and `[]` on a structured-JSON field (a + * document, NOT empty). Each "empty" row sits beside a present twin, so a + * predicate that answered `true` for everything fails as loudly as one that + * answered `false`. + */ + +import { describe, it, expect } from 'vitest'; +import { isEmptyForRequired, isMissing } from '../index.js'; + +const TEXT = { type: 'text' }; +const TAGS = { type: 'tags' }; +const MULTISELECT = { type: 'multiselect' }; +const MULTI_LOOKUP = { type: 'lookup', multiple: true }; +const SINGLE_LOOKUP = { type: 'lookup' }; +const JSON_FIELD = { type: 'json' }; + +describe('[#24131] isMissing — the def-free half', () => { + it.each([ + ['absent (undefined)', undefined], + ['null', null], + ["the empty string ''", ''], + ['a whitespace-only string', ' '], + ['a tab-and-newline string', '\t\n'], + ])('%s is missing', (_label, value) => { + expect(isMissing(value)).toBe(true); + }); + + it.each([ + ['a non-blank string', 'Ada'], + ['a padded non-blank string', ' Ada '], + ['zero', 0], + ['false', false], + ['an empty list (def-free: never missing here)', []], + ['an empty object', {}], + ])('%s is present', (_label, value) => { + expect(isMissing(value)).toBe(false); + }); +}); + +describe('[#24131] isEmptyForRequired — isMissing, plus [] where the declared value is a set', () => { + it.each([ + ['absent', TEXT, undefined], + ['null', TEXT, null], + ["''", TEXT, ''], + ['whitespace-only', TEXT, ' '], + ['[] on tags (an inherently-multi type)', TAGS, []], + ['[] on multiselect', MULTISELECT, []], + ['[] on a lookup flagged multiple: true', MULTI_LOOKUP, []], + ['null on tags', TAGS, null], + ])('%s is empty', (_label, def, value) => { + expect(isEmptyForRequired(def, value)).toBe(true); + }); + + it.each([ + ['a non-blank string', TEXT, 'ada@example.test'], + ['zero', { type: 'number' }, 0], + ['false', { type: 'boolean' }, false], + ['a one-member list on tags', TAGS, ['x']], + ['a one-member list on a multi lookup', MULTI_LOOKUP, ['rec_1']], + // A structured-JSON field's `[]` is a legitimate document, not an emptied + // set — the half of the rule a value-shape guess would get wrong. + ['[] on a json field (a document, not an emptied set)', JSON_FIELD, []], + // A single-valued lookup's declared value is NOT a set, so its `[]` is + // not judged empty here; the value-shape arm downstream refuses it. + ['[] on a single-valued lookup', SINGLE_LOOKUP, []], + ])('%s is not empty', (_label, def, value) => { + expect(isEmptyForRequired(def, value)).toBe(false); + }); +}); diff --git a/packages/core/src/utils/required-emptiness.ts b/packages/core/src/utils/required-emptiness.ts new file mode 100644 index 00000000000..8886336ab5d --- /dev/null +++ b/packages/core/src/utils/required-emptiness.ts @@ -0,0 +1,63 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24131] THE emptiness rule a `required` declaration is judged by — one rule, + * read by every door that refuses a missing required value. + * + * ## Two faces, one rule + * + * - `@objectstack/objectql`'s record validator judges an object field's + * `required` with it on every insert, and a PATCH that clears a required + * field (`required_cleared`). + * - `@objectstack/rest`'s anonymous public-form submit door judges the FORM's + * own `required` (a form-field entry's `required: true`) with it before the + * write, so a field the form requires and the object does not is refused in + * the same words the object-level refusal uses. + * + * `@objectstack/rest` does not depend on the engine at runtime (the + * `check:undeclared-dep-imports` ledger records why), and a copy each is how + * one value comes to be "missing" at one door and "present" at the other — the + * whitespace-only string and the empty list are the two cells a hand-written + * check drops. So the rule lives here, where both already depend, beside the + * other value-level rules those two packages share + * (`multi-value-storage-form.ts`). It moved out of `record-validator.ts` + * unchanged. + * + * ## The rule + * + * - {@link isMissing}: `undefined`, `null`, and a string that is empty or only + * whitespace. Def-free. + * - {@link isEmptyForRequired}: everything {@link isMissing} calls missing, and + * — only where the DECLARED value is a set (`@objectstack/spec/data`'s + * `isMultiValueField`, ADR-0104 D1) — an empty list. The #9447 maintainer + * ruling sets that half: `required` on a multi-value field means a NON-EMPTY + * list, because the empty set is representable (it reads back as `[]`, never + * `null`), so an explicit `[]` is an empty value exactly as `null` / `''` + * are. A structured-JSON field's `[]` is a legitimate document, not an + * emptied set, and is NOT empty here. + * + * ⛔ What a door does with "missing" is the door's: the engine skips a + * runtime-owned `autonumber`, the form door judges only the entries the visitor + * is asked to fill. Neither belongs in this rule, and neither is a reason for a + * second copy of it. + */ + +import { isMultiValueField, type ValueShapeFieldDef } from '@objectstack/spec/data'; + +/** + * Whether a value is missing: `undefined`, `null`, or a string that is empty + * or only whitespace. The def-free half of {@link isEmptyForRequired}. + */ +export function isMissing(v: unknown): boolean { + return v === undefined || v === null || (typeof v === 'string' && v.trim() === ''); +} + +/** + * Whether a value leaves a `required` field empty — {@link isMissing}, plus an + * empty list on a field whose declared value is a set. Judged against the + * field's DECLARATION (`type`, `multiple`), never against the value's shape. + */ +export function isEmptyForRequired(def: ValueShapeFieldDef, value: unknown): boolean { + if (isMissing(value)) return true; + return isMultiValueField(def) && Array.isArray(value) && value.length === 0; +} diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index 671387d31cb..1bf983da76e 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -105,7 +105,10 @@ import { classifyFilterToken, } from '@objectstack/spec/data'; import type { FieldErrorCode } from '@objectstack/spec/api'; -import { SUPPORTED_TEMPORAL_YEARS, isOutsideTemporalYearRange, isUninterpretableTemporalComparand, multiValueStorageForm } from '@objectstack/core'; +// [#24131] `isMissing` / `isEmptyForRequired` — the emptiness rule `required` +// is judged by — live in core, so the public-form submit door in +// `@objectstack/rest` judges a form's `required` by the same rule. +import { SUPPORTED_TEMPORAL_YEARS, isOutsideTemporalYearRange, isUninterpretableTemporalComparand, multiValueStorageForm, isMissing, isEmptyForRequired } from '@objectstack/core'; import { isValueDomainMember, type ValueDomain } from '@objectstack/spec/shared'; import { renderValidationMessage, @@ -301,27 +304,13 @@ interface FieldDef { picklist?: string; } -function isMissing(v: unknown): boolean { - return v === undefined || v === null || (typeof v === 'string' && v.trim() === ''); -} - -/** - * #9476 — the emptiness judgment the `required` check runs, def-aware where - * `isMissing` is not. The #9447 maintainer ruling (2026-08-18) sets the - * contract: `required` on a multi-value field means NON-EMPTY array — the - * empty set is representable (it reads back as `[]`, never `null`), so an - * explicit `[]` is an empty value exactly as `null` / `''` are. - * - * Judged only where the DECLARED value is a set (`isMultiValueField`, - * ADR-0104 D1): a structured-JSON field's `[]` is a legitimate document, not - * an emptied set, and every other type keeps the def-free `isMissing` - * semantics. Only the two `required` read sites call this — a bare `[]` on a - * non-required multi-value field still flows to the array-shape branch. - */ -function isEmptyForRequired(def: FieldDef, value: unknown): boolean { - if (isMissing(value)) return true; - return isMultiValueField(def) && Array.isArray(value) && value.length === 0; -} +// [#24131] `isMissing` (the def-free "missing": `undefined`, `null`, a blank +// string) and `isEmptyForRequired` (#9476 — that, plus `[]` where the DECLARED +// value is a set, per the #9447 ruling) moved to `@objectstack/core` +// (`utils/required-emptiness.ts`, which carries their contract) unchanged, and +// are imported above. Only the two `required` read sites call +// `isEmptyForRequired` — a bare `[]` on a non-required multi-value field still +// flows to the array-shape branch. /** * How many decimal places a finite number carries (#7501). diff --git a/packages/rest/src/form-routes.ts b/packages/rest/src/form-routes.ts index 464bbaa4d12..e5f50db0a75 100644 --- a/packages/rest/src/form-routes.ts +++ b/packages/rest/src/form-routes.ts @@ -71,8 +71,11 @@ import { anonymousFormIntakeWithdrawnIn, anonymousFormObjectName, } from '@objectstack/metadata-core'; +import { isEmptyForRequired, isMissing } from '@objectstack/core'; import type { CreateDataRequest, GetMetaItemsRequest } from '@objectstack/spec/api'; import { PUBLIC_FORM_SERVER_MANAGED_FIELDS } from '@objectstack/spec/security'; +import { renderValidationMessage } from '@objectstack/spec/system'; +import { validationFailure } from '@objectstack/types'; import type { ServerScopedDataRequest } from './data-action-routes.js'; import { isExpectedRouteError, mapDataError } from './error-response.js'; import { logError } from './log.js'; @@ -432,10 +435,26 @@ export function registerFormEndpoints(this: FormRouteHost, basePath: string): vo // for stamping server-side defaults — see the CRM // `lead.hook.ts` / `case.hook.ts` for the canonical pattern. const allowedFields = new Set(); + // [#24131] …and, off the SAME entries, the fields the form + // requires the visitor to fill: an entry declaring + // `required: true` that the form shows unconditionally. See + // the refusal below for why a hidden or conditionally visible + // entry is not judged here. + const requiredFields: string[] = []; for (const section of match.form?.sections ?? []) { + const sectionConditional = section?.visibleWhen != null; for (const f of section?.fields ?? []) { if (typeof f === 'string') allowedFields.add(f); - else if (f?.field) allowedFields.add(f.field); + else if (f?.field) { + allowedFields.add(f.field); + if ( + f.required === true + && f.hidden !== true + && f.visibleWhen == null + && !sectionConditional + && !requiredFields.includes(f.field) + ) requiredFields.push(f.field); + } } } // [#6920] A form that declares NO fields collects nothing, so @@ -513,6 +532,62 @@ export function registerFormEndpoints(this: FormRouteHost, basePath: string): vo }; const p = await this.resolveProtocol(environmentId, req); + + // [#24131] The form's own `required`, enforced BEFORE the write. + // The engine judges only the fields the OBJECT requires, so a + // field the form requires and the object does not used to be + // stored empty with a `201` — while the rendered form refused + // the same submission. This door is exactly the one a script or + // bot uses without the rendered form. + // + // - ONE emptiness rule: `isEmptyForRequired` from + // `@objectstack/core`, the rule the engine's object-level check + // runs (absent, `null`, a blank string, and `[]` where the + // declared value is a set). Judged on what this door WRITES + // (`filteredData`), so an object default or a hook stamp does + // not satisfy a field the visitor omitted. + // - ONE envelope: thrown as `validationFailure`, so the catch's + // `mapDataError` arm answers the flat `400 VALIDATION_FAILED` + // with `fields[]` the object-level refusal answers on this + // route, worded by the same catalog (`renderValidationMessage`, + // `required`) and naming the field by the object's label. + // - Judged only where the form ASKS: an entry that is + // `hidden: true`, or carries a `visibleWhen` on itself or its + // section, is not judged — nothing server-side evaluates a + // form-view `visibleWhen`, and judging it unconditionally would + // refuse a submission whose visitor was never shown the field. + // A server-managed anchor is never written by this door, so it + // is never judged either. + const judged = requiredFields.filter((name) => + !PUBLIC_FORM_SERVER_MANAGED_FIELDS.has(name) + && name !== '__proto__' && name !== 'constructor' && name !== 'prototype'); + if (judged.length > 0) { + const objectDef = (await this.readFormObjectDefinitions(p, environmentId)) + .find((o: any) => o?.name === match.object); + const missing: Array<{ field: string; code: 'required'; message: string; label: string }> = []; + for (const name of judged) { + const def = objectDef?.fields?.[name]; + const value = Object.prototype.hasOwnProperty.call(filteredData, name) + ? filteredData[name] + : undefined; + const empty = def && typeof def.type === 'string' + ? isEmptyForRequired(def, value) + : isMissing(value); + if (!empty) continue; + const declared = typeof def?.label === 'string' ? def.label.trim() : ''; + const label = declared.length > 0 ? declared : name; + missing.push({ + field: name, + code: 'required', + message: renderValidationMessage({ messageKey: 'required', label, field: name, params: {} }), + label, + }); + } + if (missing.length > 0) { + throw validationFailure(missing.map((f) => f.message).join('; '), missing); + } + } + const formCreateRequest: ServerScopedDataRequest = { object: match.object, data: filteredData, diff --git a/packages/rest/src/public-form-required.test.ts b/packages/rest/src/public-form-required.test.ts new file mode 100644 index 00000000000..28c59f1e98e --- /dev/null +++ b/packages/rest/src/public-form-required.test.ts @@ -0,0 +1,237 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24131] The anonymous public-form submit enforces the FORM's own `required`. + * + * A form entry may declare `{ field: 'email', required: true }` on a field the + * OBJECT does not require — `examples/app-crm`'s `web_to_lead` form does + * exactly that. The engine judges only the object's `required`, so the submit + * door answered `201` and stored the row with the field empty, while the + * rendered form refused the same submission. Measured on the real route before + * the fix: absent, `null`, `''` and a whitespace-only string on a form-required + * text field, and `[]` on a form-required multi-value field, all answered `201` + * and were stored. + * + * Through the real route — `RestServer` → `ObjectStackProtocolImplementation` + * → `ObjectQL` → `SqlDriver` (sqlite in memory). Only the metadata read is + * stubbed, to serve the form and the object the route resolves. + * + * What is pinned, by class: + * - each empty cell of the shared rule (`isEmptyForRequired`) is refused with + * `400`, the flat `VALIDATION_FAILED` envelope naming the field, and NO row; + * - the envelope is the object-level refusal's, key for key, and a field both + * levels require is refused in the exact words the engine used before; + * - control: every field present answers `201` and the row lands as sent; + * - the object-level refusal of a field only the object requires is the + * engine's, unchanged; + * - the boundary: an entry the form hides, or shows only conditionally, is not + * judged by the door (nothing server-side evaluates a form-view + * `visibleWhen`). + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; + +const OBJECT = 'pf_required_lead_24131'; +const SLUG = 'pf-required-24131'; +const SYS = { context: { isSystem: true } }; + +/** `name` and `company` are object-required; `email` / `tags` / the conditional ones are not. */ +const LEAD = { + name: OBJECT, + label: 'Lead', + fields: { + name: { name: 'name', type: 'text' as const, label: 'Lead Name', required: true }, + company: { name: 'company', type: 'text' as const, label: 'Company', required: true }, + email: { name: 'email', type: 'email' as const, label: 'Email' }, + tags: { name: 'tags', type: 'tags' as const, label: 'Tags' }, + stage: { name: 'stage', type: 'text' as const, label: 'Stage' }, + reason: { name: 'reason', type: 'text' as const, label: 'Reason' }, + referrer: { name: 'referrer', type: 'text' as const, label: 'Referrer' }, + region: { name: 'region', type: 'text' as const, label: 'Region' }, + }, +}; + +const FORM = { + name: 'pf_required_lead_form', + object: OBJECT, + viewKind: 'form', + config: { + type: 'simple', + data: { provider: 'object', object: OBJECT }, + sections: [ + { + name: 'contact', + fields: [ + { field: 'name', required: true }, + { field: 'company' }, + { field: 'email', required: true }, + { field: 'tags', required: true }, + { field: 'stage' }, + // Shown only when the predicate holds — nothing server-side evaluates it. + { field: 'reason', required: true, visibleWhen: { dialect: 'cel', source: "record.stage == 'lost'" } }, + // Never shown to the visitor. + { field: 'referrer', required: true, hidden: true }, + ], + }, + { + name: 'conditional_section', + visibleWhen: { dialect: 'cel', source: "record.stage == 'won'" }, + fields: [{ field: 'region', required: true }], + }, + ], + sharing: { enabled: true, allowAnonymous: true, publicLink: `/forms/${SLUG}` }, + }, +}; + +/** Every field the visitor is asked to fill, filled. */ +const COMPLETE = { name: 'Ada', company: 'Analytical Engines', email: 'ada@example.test', tags: ['lead'] }; + +/** {@link COMPLETE} with one key left out — absent, not `undefined`. */ +function without(key: keyof typeof COMPLETE): Record { + const body: Record = { ...COMPLETE }; + delete body[key]; + return body; +} + +function mockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + header: () => res, + end: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: unknown) => { res._json = body; return res; }, + }; + return res; +} + +interface Answer { status: number; body: any } + +describe('[#24131] POST /forms/:slug/submit enforces the form\'s own `required`', () => { + let engine: ObjectQL; + let post: (body: Record) => Promise; + const rowCount = async (): Promise => (await engine.find(OBJECT, {}, SYS)).length; + + beforeAll(async () => { + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true } as any); + engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(LEAD as any); + await engine.syncSchemas(); + + const protocol: any = new ObjectStackProtocolImplementation(engine as any); + const realGetMetaItems = protocol.getMetaItems.bind(protocol); + protocol.getMetaItems = async (request: any) => { + if (request?.type === 'view') return [FORM]; + if (request?.type === 'object') return [LEAD]; + return realGetMetaItems(request); + }; + const rest = new RestServer(mockServer() as any, protocol, { api: { requireAuth: false } } as any); + rest.registerRoutes(); + const submit = rest.getRoutes().find((r: any) => r.method === 'POST' && r.path === '/api/v1/forms/:slug/submit'); + expect(submit, 'the submit route is registered').toBeDefined(); + post = async (body) => { + const res = makeRes(); + await submit!.handler({ params: { slug: SLUG }, body: JSON.parse(JSON.stringify(body)) } as any, res); + return { status: res._status ?? 200, body: res._json }; + }; + }, 60_000); + + afterAll(async () => { + await engine?.destroy().catch(() => {}); + }); + + /** A refusal that names exactly `field`, and stores nothing. */ + const expectRefused = async (body: Record, field: string, label: string) => { + const before = await rowCount(); + const answer = await post(body); + expect(answer.status, JSON.stringify(answer.body)).toBe(400); + expect(answer.body.code).toBe('VALIDATION_FAILED'); + expect(answer.body.fields).toEqual([ + { field, code: 'required', message: `${label} is required`, label }, + ]); + expect(answer.body.error).toBe(`${label} is required`); + expect(await rowCount(), 'a refused submit stores no row').toBe(before); + return answer; + }; + + it.each([ + ['absent', without('email')], + ['null', { ...COMPLETE, email: null }], + ["''", { ...COMPLETE, email: '' }], + ['whitespace-only', { ...COMPLETE, email: ' ' }], + ])('a form-required text field the object does not require, %s → 400 naming it, no row', async (_cell, body) => { + await expectRefused(body, 'email', 'Email'); + }, 60_000); + + it.each([ + ['[]', { ...COMPLETE, tags: [] }], + ['absent', without('tags')], + ])('a form-required multi-value field, %s → 400 naming it, no row', async (_cell, body) => { + await expectRefused(body, 'tags', 'Tags'); + }, 60_000); + + it('two form-required fields missing → one refusal naming both, in the form\'s order', async () => { + const before = await rowCount(); + const answer = await post({ name: 'Ada', company: 'Analytical Engines' }); + expect(answer.status).toBe(400); + expect(answer.body.code).toBe('VALIDATION_FAILED'); + expect(answer.body.fields.map((f: any) => [f.field, f.code])).toEqual([['email', 'required'], ['tags', 'required']]); + expect(answer.body.error).toBe('Email is required; Tags is required'); + expect(await rowCount()).toBe(before); + }, 60_000); + + it('CONTROL — every field the visitor is asked to fill, filled → 201, and the row lands as sent', async () => { + const before = await rowCount(); + const answer = await post(COMPLETE); + expect(answer.status, JSON.stringify(answer.body)).toBe(201); + expect(typeof answer.body.id).toBe('string'); + const stored = await engine.findOne(OBJECT, { where: { id: answer.body.id } }, SYS); + expect(stored).toMatchObject({ name: 'Ada', company: 'Analytical Engines', email: 'ada@example.test', tags: ['lead'] }); + expect(await rowCount()).toBe(before + 1); + }, 60_000); + + it('a field only the OBJECT requires is still refused by the engine, unchanged — and the form-level refusal is the same envelope', async () => { + const before = await rowCount(); + const engineAnswer = await post(without('company')); + expect(engineAnswer.status).toBe(400); + expect(engineAnswer.body).toEqual({ + error: 'Company is required', + code: 'VALIDATION_FAILED', + fields: [{ field: 'company', code: 'required', message: 'Company is required', label: 'Company' }], + }); + expect(await rowCount()).toBe(before); + + const formAnswer = await expectRefused(without('email'), 'email', 'Email'); + expect(Object.keys(formAnswer.body)).toEqual(Object.keys(engineAnswer.body)); + expect(Object.keys(formAnswer.body.fields[0])).toEqual(Object.keys(engineAnswer.body.fields[0])); + }, 60_000); + + it('a field BOTH levels require is refused in the exact words the engine answered before the door judged it', async () => { + const answer = await expectRefused(without('name'), 'name', 'Lead Name'); + // The body the engine's object-level check answered on this route before the + // fix, measured on origin/main 747c8e9c4. + expect(answer.body).toEqual({ + error: 'Lead Name is required', + code: 'VALIDATION_FAILED', + fields: [{ field: 'name', code: 'required', message: 'Lead Name is required', label: 'Lead Name' }], + }); + }, 60_000); + + it('BOUNDARY — a hidden entry, a conditionally visible entry and an entry in a conditional section are not judged by the door', async () => { + const answer = await post(COMPLETE); + expect(answer.status, JSON.stringify(answer.body)).toBe(201); + const stored = await engine.findOne(OBJECT, { where: { id: answer.body.id } }, SYS); + expect(stored?.reason ?? null).toBeNull(); + expect(stored?.referrer ?? null).toBeNull(); + expect(stored?.region ?? null).toBeNull(); + }, 60_000); +});