diff --git a/.changeset/27606-public-form-option-subset.md b/.changeset/27606-public-form-option-subset.md new file mode 100644 index 00000000000..9618f5227c8 --- /dev/null +++ b/.changeset/27606-public-form-option-subset.md @@ -0,0 +1,19 @@ +--- +'@objectstack/rest': patch +--- + +fix(rest)!: the public form submit door refuses a value outside a form row's declared `options` subset (#27606) + +Clause-②: no (narrowing) — `POST /forms/:slug/submit` now refuses a value outside the `options` subset a form entry declares, which it used to store with `201`. Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the anonymous public form submit door (`@objectstack/rest`). + +- **Before.** A form entry such as `{ field: 'priority', options: [low, medium, high] }` narrowed what the form offered, and `GET /forms/:slug` served that subset, but `POST /forms/:slug/submit` judged keys and the form's `required` only. The engine judges a value against the object's options, so a guest could store `critical` through that form with `201`. +- **Now.** The door answers `400 VALIDATION_FAILED` before anything is written, naming the field in `fields[]` with the ADR-0114 code `invalid_option`, the catalog's message, and `constraint.allowed` listing the subset's values only. It is the same envelope the form's `required` and file refusals use, so a submission with several problems is told all of them at once. A multi-valued field is judged element by element, the first outside element named. +- **Unchanged.** A value inside the subset saves as before. An empty or absent optional value is not judged. An entry that lists no `options` keeps the object's full set, judged by the engine as before, and so does a field that another entry of the same form offers without a subset. +- **Who sees it.** A client posting to a published form a value the form does not offer: a script or bot, or the console's public form page, which still lists every option of the field until its renderer reads the row's subset (a separate `domain:ui` change). + +**The remedy.** Send one of the values the form serves for that field in `GET /forms/:slug`, or remove the entry's `options` to offer the object's full set. diff --git a/content/docs/ui/public-data-collection.mdx b/content/docs/ui/public-data-collection.mdx index 12aafdf9fd1..e942f393b31 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. 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. +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. An entry that lists `options` offers that subset of the field's members, and the submit route refuses a value outside it with `400 VALIDATION_FAILED` naming the field (`invalid_option`, listing the subset's values only); an entry that lists none offers the object's full set. 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. The whitelist cannot hold a file field (`file`, `image`, `avatar`, `video`, `audio`): a visitor has no upload route, so a public form declaring one is refused at `os validate` / `os build` and at publish (`public-form-file-field`). Collect files from registered, signed-in users on an internal form instead. diff --git a/packages/rest/src/form-routes.ts b/packages/rest/src/form-routes.ts index e6855e87be1..ea1da3d785c 100644 --- a/packages/rest/src/form-routes.ts +++ b/packages/rest/src/form-routes.ts @@ -460,12 +460,34 @@ export function registerFormEndpoints(this: FormRouteHost, basePath: string): vo // the refusal below for why a hidden or conditionally visible // entry is not judged here. const requiredFields: string[] = []; + // [#27606] …and the `options` subset each field's entries offer: + // the union of every entry that lists `options`. An entry that + // lists none (or a bare-string entry) offers the object's full + // set, so it marks the field `null`, not narrowed, whatever + // another entry lists. See the refusal below. + const optionSubsets = new Map | null>(); 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) { + if (typeof f === 'string') { + allowedFields.add(f); + optionSubsets.set(f, null); + } else if (f?.field) { allowedFields.add(f.field); + const offered = Array.isArray(f.options) && f.options.length > 0 ? f.options : null; + const sofar = optionSubsets.get(f.field); + if (offered === null || sofar === null) { + optionSubsets.set(f.field, null); + } else { + const subset = sofar ?? new Set(); + for (const o of offered) { + // A bare-string option is its own value, as the engine's + // and the console's option readers take it. + const v = typeof o === 'string' ? o : o?.value; + if (v !== undefined && v !== null) subset.add(String(v)); + } + optionSubsets.set(f.field, subset); + } if ( f.required === true && f.hidden !== true @@ -616,20 +638,45 @@ export function registerFormEndpoints(this: FormRouteHost, basePath: string): vo // `400 VALIDATION_FAILED` with `fields[]`, so a submission with // both problems is told both at once. The field code is the // closed ADR-0114 catalog's `invalid_value`. + // [#27606] A value outside the `options` subset a form entry + // offers is refused, naming the field, before anything is + // written. `GET /forms/:slug` serves that subset; the engine + // judges a value against the OBJECT's options only, so this door + // stored a member the form never offered (a guest filing a + // `critical` case through a form that offers up to `high`). + // + // - Judged on what this door WRITES, with the same emptiness + // rule as `required`: an empty or absent value is not judged. + // - A multi-valued value is judged element by element, the first + // outside element named, as the engine judges the object's set. + // - The same envelope again, with the closed catalog's + // `invalid_option`, worded by the catalog (`invalid_option`, + // or `invalid_option_value` for an element). The allowed values + // it names are the subset's only, which the form publishes. + // - A field no entry narrows keeps the object's full set, judged + // by the engine as before. // The declared fields this submission carries a value in: the - // only ones the file judgement can refuse, so a submission that - // carries none reads no object declarations for it. - const fileValued = [...allowedFields].filter((name) => + // only ones the file and subset judgements can refuse, so a + // submission that carries none reads no object declarations for + // them. + const valued = [...allowedFields].filter((name) => Object.prototype.hasOwnProperty.call(filteredData, name) && !isMissing(filteredData[name])); - if (judged.length > 0 || fileValued.length > 0) { + if (judged.length > 0 || valued.length > 0) { const objectDef = (await this.readFormObjectDefinitions(p, environmentId)) .find((o: any) => o?.name === match.object); const labelOf = (def: any, name: string): string => { const declared = typeof def?.label === 'string' ? def.label.trim() : ''; return declared.length > 0 ? declared : name; }; - const refused: Array<{ field: string; code: 'required' | 'invalid_value'; message: string; label: string }> = []; + const refused: Array<{ + field: string; + code: 'required' | 'invalid_value' | 'invalid_option'; + message: string; + label: string; + constraint?: Record; + value?: string; + }> = []; for (const name of judged) { const def = objectDef?.fields?.[name]; const value = Object.prototype.hasOwnProperty.call(filteredData, name) @@ -647,7 +694,7 @@ export function registerFormEndpoints(this: FormRouteHost, basePath: string): vo label, }); } - for (const name of fileValued) { + for (const name of valued) { const def = objectDef?.fields?.[name]; if (!def || typeof def.type !== 'string' || !FILE_REFERENCE_TYPES.has(def.type)) continue; if (isEmptyForRequired(def, filteredData[name])) continue; @@ -660,6 +707,40 @@ export function registerFormEndpoints(this: FormRouteHost, basePath: string): vo label, }); } + for (const name of valued) { + const subset = optionSubsets.get(name); + // A subset that collected no values narrows nothing (fail + // open to the engine's judgement, never refuse every value). + if (!subset || subset.size === 0) continue; + const def = objectDef?.fields?.[name]; + const value = filteredData[name]; + const empty = def && typeof def.type === 'string' + ? isEmptyForRequired(def, value) + : isMissing(value); + if (empty) continue; + const elements = Array.isArray(value) ? value : [value]; + const outside = elements.findIndex((v) => !subset.has(String(v))); + if (outside < 0) continue; + const allowed = [...subset].join(', '); + const label = labelOf(def, name); + refused.push({ + field: name, + code: 'invalid_option', + message: Array.isArray(value) + ? renderValidationMessage({ + messageKey: 'invalid_option_value', + label, + field: name, + params: { allowed, value: String(elements[outside]) }, + }) + : renderValidationMessage({ messageKey: 'invalid_option', label, field: name, params: { allowed } }), + label, + constraint: { allowed }, + // The offending element, as the engine's multi-valued + // `invalid_option` ships it; its single-value arm ships none. + ...(Array.isArray(value) ? { value: String(elements[outside]) } : {}), + }); + } if (refused.length > 0) { throw validationFailure(refused.map((f) => f.message).join('; '), refused); } diff --git a/packages/rest/src/public-form-option-subset.test.ts b/packages/rest/src/public-form-option-subset.test.ts new file mode 100644 index 00000000000..8c100ac7b29 --- /dev/null +++ b/packages/rest/src/public-form-option-subset.test.ts @@ -0,0 +1,287 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27606] The anonymous public-form submit enforces a form row's declared + * `options` subset. + * + * A form row may list `options` to offer a deliberate subset of a select + * field's members — HotCRM's web-to-case form offers a guest + * `[low, medium, high]` of a priority that also has `critical`. `GET + * /forms/:slug` serves that subset, but the submit door judged keys and the + * form's `required` only, so `POST /forms/:slug/submit` stored `critical` with + * a `201`: the engine judges a value against the OBJECT's options, never a + * form's. Measured on the real route before the fix (single and multi-valued + * select alike). + * + * Through the real route — `RestServer` → `ObjectStackProtocolImplementation` + * → `ObjectQL` → `SqlDriver` (sqlite in memory). Only the metadata read is + * stubbed, to serve the forms and the object the route resolves. + * + * Pinned, by class: + * - a value outside the row's subset is refused: `400`, the flat + * `VALIDATION_FAILED` envelope naming the field with the ADR-0114 field + * code `invalid_option`, the allowed values the subset's only, the write + * never attempted and no row stored; + * - a multi-valued field: every element is judged, the first outside one named; + * - one envelope with the form's `required` judgement: both are told at once; + * - control: a value inside the subset saves, `201`, the row stored as sent; + * - control: an empty or absent optional value submits; + * - the boundary: a row that lists no `options` keeps the object's full set, + * and a field offered by another row without a subset is not narrowed. + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +const OBJECT = 'pf_subset_case_27606'; +const SLUG = 'pf-subset-27606'; +const WIDE_SLUG = 'pf-subset-wide-27606'; +const LOOSE_SLUG = 'pf-subset-loose-27606'; +const SYS = { context: { isSystem: true } }; + +const CASE = { + name: OBJECT, + label: 'Case', + fields: { + subject: { name: 'subject', type: 'text' as const, label: 'Subject', required: true }, + priority: { + name: 'priority', + type: 'select' as const, + label: 'Priority', + options: [ + { label: 'Low', value: 'low' }, + { label: 'Medium', value: 'medium' }, + { label: 'High', value: 'high' }, + { label: 'Critical', value: 'critical' }, + ], + }, + topics: { + name: 'topics', + type: 'select' as const, + label: 'Topics', + multiple: true, + options: [ + { label: 'Billing', value: 'billing' }, + { label: 'Outage', value: 'outage' }, + { label: 'Security', value: 'security' }, + ], + }, + channel: { + name: 'channel', + type: 'select' as const, + label: 'Channel', + options: [ + { label: 'Web', value: 'web' }, + { label: 'Email', value: 'email' }, + { label: 'Phone', value: 'phone' }, + ], + }, + }, +}; + +const form = (name: string, slug: string, sections: unknown[]) => ({ + name, + object: OBJECT, + viewKind: 'form', + config: { + type: 'simple', + data: { provider: 'object', object: OBJECT }, + sections, + sharing: { enabled: true, allowAnonymous: true, publicLink: `/forms/${slug}` }, + }, +}); + +const FORM = form('pf_subset_case_form', SLUG, [ + { + name: 'case', + fields: [ + { field: 'subject', required: true }, + { + field: 'priority', + options: [ + { label: 'Low', value: 'low' }, + { label: 'Medium', value: 'medium' }, + { label: 'High', value: 'high' }, + ], + }, + { field: 'topics', options: [{ label: 'Billing', value: 'billing' }, { label: 'Outage', value: 'outage' }] }, + // No `options`: the object's full set. + { field: 'channel' }, + ], + }, +]); + +// The same field on two rows, one narrowing it and one not: the form offers +// the full set, so the door does not narrow it. +const WIDE_FORM = form('pf_subset_case_wide_form', WIDE_SLUG, [ + { name: 'first', fields: [{ field: 'subject' }, { field: 'priority', options: [{ label: 'Low', value: 'low' }] }] }, + { name: 'second', fields: ['priority'] }, +]); + +// Option shapes the strict row schema refuses, reaching the door only from an +// unparsed source: bare-string options read as their own values, and options +// that carry no value at all narrow nothing. +const LOOSE_FORM = form('pf_subset_case_loose_form', LOOSE_SLUG, [ + { + name: 'loose', + fields: [ + { field: 'subject' }, + { field: 'priority', options: ['low', 'medium'] }, + { field: 'channel', options: [{ label: 'Web' }] }, + ], + }, +]); + +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('[#27606] POST /forms/:slug/submit: a form row\'s `options` subset is enforced', () => { + let engine: ObjectQL; + let createData: ReturnType; + let post: (body: Record, slug?: string) => Promise; + const rows = async (): Promise>> => (await engine.find(OBJECT, {}, SYS)) as any; + + 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(CASE as any); + await engine.syncSchemas(); + await installDefaultOrganization(engine); + + 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, WIDE_FORM, LOOSE_FORM]; + if (request?.type === 'object') return [CASE]; + return realGetMetaItems(request); + }; + const realCreateData = protocol.createData.bind(protocol); + createData = vi.fn(realCreateData); + protocol.createData = createData; + 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, slug = SLUG) => { + const res = makeRes(); + await submit!.handler({ params: { 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 naming exactly `expected` (field, code, label) in order, the write never attempted, no row stored. */ + const expectRefused = async (body: Record, expected: Array<[string, string, string]>, slug?: string) => { + const before = (await rows()).length; + const calls = createData.mock.calls.length; + const answer = await post(body, slug); + expect(answer.status, JSON.stringify(answer.body)).toBe(400); + expect(answer.body.code).toBe('VALIDATION_FAILED'); + expect(answer.body.fields.map((f: any) => [f.field, f.code, f.label])).toEqual(expected); + for (const f of answer.body.fields) expect(f.message).toContain(f.label); + expect(createData.mock.calls.length, 'the write is never attempted').toBe(calls); + expect((await rows()).length, 'a refused submit stores no row').toBe(before); + return answer; + }; + + const expectStored = async (body: Record, slug?: string) => { + const before = (await rows()).length; + const answer = await post(body, slug); + expect(answer.status, JSON.stringify(answer.body)).toBe(201); + expect(typeof answer.body.id).toBe('string'); + const all = await rows(); + expect(all.length).toBe(before + 1); + return all.find((r) => r.id === answer.body.id); + }; + + it('a value outside the row\'s subset is refused, naming the field and only the subset\'s values', async () => { + const answer = await expectRefused({ subject: 'Site down', priority: 'critical' }, [['priority', 'invalid_option', 'Priority']]); + const [refusal] = answer.body.fields; + expect(refusal.constraint).toEqual({ allowed: 'low, medium, high' }); + // The refusal discloses nothing the form does not publish. + expect(JSON.stringify(answer.body)).not.toContain('Critical'); + expect(refusal.message).not.toContain('critical'); + // The engine's single-value `invalid_option` ships no `value`; neither does the door's. + expect(refusal).not.toHaveProperty('value'); + }, 60_000); + + it('a value that is no member of the object at all is refused by the same judgement', async () => { + await expectRefused({ subject: 'Site down', priority: 'urgent' }, [['priority', 'invalid_option', 'Priority']]); + }, 60_000); + + it('a multi-valued field: an element outside the subset is refused, naming it', async () => { + const answer = await expectRefused( + { subject: 'Site down', topics: ['billing', 'security'] }, + [['topics', 'invalid_option', 'Topics']], + ); + expect(answer.body.fields[0].message).toContain('security'); + // The offending element, as the engine's multi-valued `invalid_option` ships it. + expect(answer.body.fields[0].value).toBe('security'); + expect(answer.body.fields[0].constraint).toEqual({ allowed: 'billing, outage' }); + }, 60_000); + + it('one envelope with the form\'s `required`: a missing required field and an outside value are told at once', async () => { + await expectRefused({ priority: 'critical' }, [['subject', 'required', 'Subject'], ['priority', 'invalid_option', 'Priority']]); + }, 60_000); + + it('CONTROL — a value inside the subset saves', async () => { + const stored = await expectStored({ subject: 'Slow page', priority: 'high', topics: ['billing', 'outage'] }); + expect(stored?.priority).toBe('high'); + expect(stored?.topics).toEqual(['billing', 'outage']); + }, 60_000); + + it.each([ + ['absent', {}], + ['null', { priority: null, topics: null }], + ["''", { priority: '' }], + ['[] on the multi-valued field', { topics: [] }], + ])('CONTROL — an empty or absent optional value submits (%s)', async (_cell, extra) => { + const stored = await expectStored({ subject: 'Question', ...extra }); + expect([null, undefined, '']).toContain(stored?.priority); + }, 60_000); + + it('BOUNDARY — a row that lists no `options` keeps the object\'s full set', async () => { + const stored = await expectStored({ subject: 'Called in', channel: 'phone' }); + expect(stored?.channel).toBe('phone'); + }, 60_000); + + it('a bare-string option is read as its own value', async () => { + const answer = await expectRefused({ subject: 'Site down', priority: 'critical' }, [['priority', 'invalid_option', 'Priority']], LOOSE_SLUG); + expect(answer.body.fields[0].constraint).toEqual({ allowed: 'low, medium' }); + const stored = await expectStored({ subject: 'Slow page', priority: 'medium' }, LOOSE_SLUG); + expect(stored?.priority).toBe('medium'); + }, 60_000); + + it('BOUNDARY — options that carry no value narrow nothing (never refuse every value)', async () => { + const stored = await expectStored({ subject: 'Called in', channel: 'phone' }, LOOSE_SLUG); + expect(stored?.channel).toBe('phone'); + }, 60_000); + + it('BOUNDARY — a field another row offers without a subset is not narrowed', async () => { + const stored = await expectStored({ subject: 'Site down', priority: 'critical' }, WIDE_SLUG); + expect(stored?.priority).toBe('critical'); + }, 60_000); +});