Skip to content

Commit 04141a6

Browse files
fix(rest)!: the public form submit door refuses a value outside a form row's declared options subset (#27766)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DGPE5j6DCgVYUDTiGjVBjhW)_ Fixes #27606 domain:server#4 · os-elon-musk · session_016WV4czUVLWPn8CY8QLDrjQ Scope: the submit door only, per triage 6500544109. The renderer half (`ui/apps/console/src/components/FormPage.tsx` listing every option) is the `domain:ui` card and is not touched here. Clause-②: no (narrowing) Metadata-protocol: none Skills-docs: none (no published skill describes how the public form submit door judges a select value; `objectstack-api` names the door as a whitelist-filtered insert, which stays true) ## 维护者速读 - 改了什么:公开表单提交口 `POST /forms/SLUG/submit` 现在拒收不在表单行 `options` 子集里的值,返回 400 `VALIDATION_FAILED`,`fields[]` 点名字段(代码 `invalid_option`,只列出子集内的可选值);子集内的值照常保存。 - 为什么改:表单作者给字段只开放部分选项(如 HotCRM 工单表单只给访客 low/medium/high),读取口已只发子集,但提交口不判,访客或脚本照样能存下 critical。 - 风险与回滚:只收窄一个匿名入口;没列 `options` 的行、同一字段另有不带子集的行、空值都不受影响。控制台公开表单页目前仍列出全部选项(另一张 ui 卡修),访客若选了子集外的值会看到点名字段的 400。回滚即还原 `form-routes.ts` 两处改动。 - 先看:`packages/rest/src/form-routes.ts` 的 `optionSubsets` 收集段与 `refused` 信封里新增的子集判断循环;`packages/rest/src/public-form-option-subset.test.ts`。 - 两分钟验证:`pnpm --filter @objectstack/rest exec vitest run src/public-form-option-subset.test.ts`,预期 11/11 通过。 - 需要你做的:无,席位落地。 ## What changes - While it reads the form's entries for the allow-list and `required`, the submit handler now also collects each field's offered `options` subset: the union of every entry that lists `options`. An entry that lists none, or a bare-string entry, offers the object's full set and marks the field not narrowed, whatever another entry lists. - Inside the existing refusal envelope (one `validationFailure`, the flat `400 VALIDATION_FAILED` with `fields[]`, already shared by the `required` and file judgements), a value outside a field's subset is refused before the write with the closed ADR-0114 catalog's `invalid_option`. The message is the catalog's (`invalid_option`, or `invalid_option_value` naming the element for a multi-valued field), the field is named by the object's label, and `constraint.allowed` lists the subset's values only. No second envelope and no new code. - Judged on what the door writes, with the same emptiness rule as `required` (`isEmptyForRequired`): 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. - Changeset: `@objectstack/rest` patch, with the BREAKING note and the ADR-0087 `not-required (no-migration-prescription)` marker. ### Deviation from the dispatch's mechanism assumption The order guessed the refusal would join the envelope as `invalid_value`. Measured: the catalog has `invalid_option` ("not a member of the field's declared options"), and it is what the engine answers for a value outside the object's options on this same route. The row subset is a declared option set, so the door uses `invalid_option` and the catalog's own sentences, keeping one vocabulary for "not an offered option" on both levels. ## Tests `packages/rest/src/public-form-option-subset.test.ts`, through the real route (`RestServer` → `ObjectStackProtocolImplementation` → `ObjectQL` → `SqlDriver` sqlite in memory; only the metadata read is stubbed): - refused: a value outside the subset (`critical` on a `[low, medium, high]` row) answers 400 naming `priority` with `invalid_option`, `constraint.allowed` is `low, medium, high`, the excluded member appears nowhere in the body, the write is never attempted and no row is stored; a value that is no member of the object is refused the same way before the write; a multi-valued element outside the subset is refused naming it; with a missing required field, both are told in one envelope; - control: a value inside the subset saves; absent, `null`, `''` and `[]` submit; - boundary: a row with no `options` keeps the object's full set; a field another row offers without a subset is not narrowed. ## Acceptance notes - The console's public form page still offers every option of the field until the `domain:ui` half reads the row's subset; until then a visitor who picks an outside value gets the 400 naming the field and the allowed values. - An entry's `options: []` is read as listing no subset (the object's full set), not as offering nothing. - An entry that is `hidden: true` or conditionally visible is still judged: the door does not evaluate `visibleWhen`, and a value outside every subset the form lists is never one the form offers. ## Verification Read at head `09cd48e0`. <details><summary>Premise on main, ablation, package and gate readings</summary> - Premise on `main` (`81d00a4e`), the new test before the fix: 4 failed, 7 passed. `critical` on the `[low, medium, high]` row answered `201` and stored the row; `["billing", "security"]` on a `[billing, outage]` row answered `201`. All 7 controls were already green. - After the fix: the new file 11/11, and the six public-form test files (option subset, required, file field, routes, stored row, submit answer) 65/65. - Ablation (`scripts/ablation-replace.mjs`, the anchor `if (!subset) continue;` replaced by a bare `continue`, 1 hit to 0, blob `acba7d5b` to `95a39697`): 4 failed, 7 passed, the four refusal cases. Restored: blob equals HEAD (`acba7d5b`), `git diff HEAD` empty. The test imports the route from `src`, so no build sat between the mutation and the run. - `pnpm --filter @objectstack/rest build`: exit 0. `pnpm --filter @objectstack/rest typecheck`: exit 0. `pnpm --filter @objectstack/rest test`: exit 0, 343/343 files, 6706 passed, 326 skipped. - `eslint` on the two changed source files, `--no-inline-config`: exit 0. The repo-wide `pnpm lint` is left to CI's `Lint & Repo Gates`. - `dispatch-gates --commands` derived 59 families; 58 exited 0. `check:dual-build-cjs-loads` exited 3, PREREQUISITE NOT MET (it reads every package's dist and needs a whole-tree build), so it is NOT MEASURED locally and CI carries it. `--ran` with exit codes: 59 derived, 58 run, 1 NOT-MEASURED (derived from the recorded exit 3), 0 UNRUN. - `check:commit-card-trailers --range origin/main..HEAD`: exit 0. `check:nul-bytes`: exit 0. - Dogfood battery (`(narrowing)` arm): running on this head under the verification lock; its reading lands in the `os-dev-report` comment on #27606. - No open PR touches `form-routes.ts` (the claim's read of the open PRs). </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016WV4czUVLWPn8CY8QLDrjQ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 308f826 commit 04141a6

4 files changed

Lines changed: 396 additions & 9 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
fix(rest)!: the public form submit door refuses a value outside a form row's declared `options` subset (#27606)
6+
7+
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.
8+
Metadata-protocol: none
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: one REST door now enforces the `options` subset a form entry already declares and `GET /forms/:slug` already serves. There is nothing for a migration to rewrite and no tombstone. -->
11+
12+
**BREAKING** accept-set narrowing at the anonymous public form submit door (`@objectstack/rest`).
13+
14+
- **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`.
15+
- **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.
16+
- **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.
17+
- **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).
18+
19+
**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.

‎content/docs/ui/public-data-collection.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ POST /api/v1/forms/contact-us/submit → INSERT one showcase_inquiry
4444

4545
### 2. The field whitelist *is* the security boundary
4646

47-
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.
47+
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.
4848

4949
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.
5050

‎packages/rest/src/form-routes.ts‎

Lines changed: 89 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -460,12 +460,34 @@ export function registerFormEndpoints(this: FormRouteHost, basePath: string): vo
460460
// the refusal below for why a hidden or conditionally visible
461461
// entry is not judged here.
462462
const requiredFields: string[] = [];
463+
// [#27606] …and the `options` subset each field's entries offer:
464+
// the union of every entry that lists `options`. An entry that
465+
// lists none (or a bare-string entry) offers the object's full
466+
// set, so it marks the field `null`, not narrowed, whatever
467+
// another entry lists. See the refusal below.
468+
const optionSubsets = new Map<string, Set<string> | null>();
463469
for (const section of match.form?.sections ?? []) {
464470
const sectionConditional = section?.visibleWhen != null;
465471
for (const f of section?.fields ?? []) {
466-
if (typeof f === 'string') allowedFields.add(f);
467-
else if (f?.field) {
472+
if (typeof f === 'string') {
473+
allowedFields.add(f);
474+
optionSubsets.set(f, null);
475+
} else if (f?.field) {
468476
allowedFields.add(f.field);
477+
const offered = Array.isArray(f.options) && f.options.length > 0 ? f.options : null;
478+
const sofar = optionSubsets.get(f.field);
479+
if (offered === null || sofar === null) {
480+
optionSubsets.set(f.field, null);
481+
} else {
482+
const subset = sofar ?? new Set<string>();
483+
for (const o of offered) {
484+
// A bare-string option is its own value, as the engine's
485+
// and the console's option readers take it.
486+
const v = typeof o === 'string' ? o : o?.value;
487+
if (v !== undefined && v !== null) subset.add(String(v));
488+
}
489+
optionSubsets.set(f.field, subset);
490+
}
469491
if (
470492
f.required === true
471493
&& f.hidden !== true
@@ -616,20 +638,45 @@ export function registerFormEndpoints(this: FormRouteHost, basePath: string): vo
616638
// `400 VALIDATION_FAILED` with `fields[]`, so a submission with
617639
// both problems is told both at once. The field code is the
618640
// closed ADR-0114 catalog's `invalid_value`.
641+
// [#27606] A value outside the `options` subset a form entry
642+
// offers is refused, naming the field, before anything is
643+
// written. `GET /forms/:slug` serves that subset; the engine
644+
// judges a value against the OBJECT's options only, so this door
645+
// stored a member the form never offered (a guest filing a
646+
// `critical` case through a form that offers up to `high`).
647+
//
648+
// - Judged on what this door WRITES, with the same emptiness
649+
// rule as `required`: an empty or absent value is not judged.
650+
// - A multi-valued value is judged element by element, the first
651+
// outside element named, as the engine judges the object's set.
652+
// - The same envelope again, with the closed catalog's
653+
// `invalid_option`, worded by the catalog (`invalid_option`,
654+
// or `invalid_option_value` for an element). The allowed values
655+
// it names are the subset's only, which the form publishes.
656+
// - A field no entry narrows keeps the object's full set, judged
657+
// by the engine as before.
619658
// The declared fields this submission carries a value in: the
620-
// only ones the file judgement can refuse, so a submission that
621-
// carries none reads no object declarations for it.
622-
const fileValued = [...allowedFields].filter((name) =>
659+
// only ones the file and subset judgements can refuse, so a
660+
// submission that carries none reads no object declarations for
661+
// them.
662+
const valued = [...allowedFields].filter((name) =>
623663
Object.prototype.hasOwnProperty.call(filteredData, name)
624664
&& !isMissing(filteredData[name]));
625-
if (judged.length > 0 || fileValued.length > 0) {
665+
if (judged.length > 0 || valued.length > 0) {
626666
const objectDef = (await this.readFormObjectDefinitions(p, environmentId))
627667
.find((o: any) => o?.name === match.object);
628668
const labelOf = (def: any, name: string): string => {
629669
const declared = typeof def?.label === 'string' ? def.label.trim() : '';
630670
return declared.length > 0 ? declared : name;
631671
};
632-
const refused: Array<{ field: string; code: 'required' | 'invalid_value'; message: string; label: string }> = [];
672+
const refused: Array<{
673+
field: string;
674+
code: 'required' | 'invalid_value' | 'invalid_option';
675+
message: string;
676+
label: string;
677+
constraint?: Record<string, unknown>;
678+
value?: string;
679+
}> = [];
633680
for (const name of judged) {
634681
const def = objectDef?.fields?.[name];
635682
const value = Object.prototype.hasOwnProperty.call(filteredData, name)
@@ -647,7 +694,7 @@ export function registerFormEndpoints(this: FormRouteHost, basePath: string): vo
647694
label,
648695
});
649696
}
650-
for (const name of fileValued) {
697+
for (const name of valued) {
651698
const def = objectDef?.fields?.[name];
652699
if (!def || typeof def.type !== 'string' || !FILE_REFERENCE_TYPES.has(def.type)) continue;
653700
if (isEmptyForRequired(def, filteredData[name])) continue;
@@ -660,6 +707,40 @@ export function registerFormEndpoints(this: FormRouteHost, basePath: string): vo
660707
label,
661708
});
662709
}
710+
for (const name of valued) {
711+
const subset = optionSubsets.get(name);
712+
// A subset that collected no values narrows nothing (fail
713+
// open to the engine's judgement, never refuse every value).
714+
if (!subset || subset.size === 0) continue;
715+
const def = objectDef?.fields?.[name];
716+
const value = filteredData[name];
717+
const empty = def && typeof def.type === 'string'
718+
? isEmptyForRequired(def, value)
719+
: isMissing(value);
720+
if (empty) continue;
721+
const elements = Array.isArray(value) ? value : [value];
722+
const outside = elements.findIndex((v) => !subset.has(String(v)));
723+
if (outside < 0) continue;
724+
const allowed = [...subset].join(', ');
725+
const label = labelOf(def, name);
726+
refused.push({
727+
field: name,
728+
code: 'invalid_option',
729+
message: Array.isArray(value)
730+
? renderValidationMessage({
731+
messageKey: 'invalid_option_value',
732+
label,
733+
field: name,
734+
params: { allowed, value: String(elements[outside]) },
735+
})
736+
: renderValidationMessage({ messageKey: 'invalid_option', label, field: name, params: { allowed } }),
737+
label,
738+
constraint: { allowed },
739+
// The offending element, as the engine's multi-valued
740+
// `invalid_option` ships it; its single-value arm ships none.
741+
...(Array.isArray(value) ? { value: String(elements[outside]) } : {}),
742+
});
743+
}
663744
if (refused.length > 0) {
664745
throw validationFailure(refused.map((f) => f.message).join('; '), refused);
665746
}

0 commit comments

Comments
 (0)