fix(rest): 零声明字段的公开表单拒收提交,而不是接受调用方发的任意键 (#6920)

.changeset/public-form-empty-declaration-refusal.md

@@ -0,0 +1,55 @@
+---
+"@objectstack/rest": minor
+---
+
+fix(rest): a public form that declares no fields now REFUSES the submit instead of accepting every key the caller sent (#6920)
+
+`POST /api/v1/forms/:slug/submit` narrows a visitor's suppliable keys to the
+fields the matched FormView's `sections` declare — and the filter read
+`allowedFields.size === 0 || allowedFields.has(k)`. For a form with no declared
+fields that limb degenerated, and not into "every field of the object": it
+accepted **every key the caller sent**, minus the `#3022` server-managed anchors
+and the three prototype keys. Measured on the real registered handler,
+anonymously, against a `sections: []` form:
+
+    submit accepted = ["email","internal_margin","internal_tier",
+                       "not_even_a_field","status","subject"]
+
+`not_even_a_field` is not declared on the target object at all. So an anonymous
+visitor could set `status`, a workflow stage, an internal tier — anything, on
+the one object the form targets. `publicFormGrant` (ADR-0056) keeps the insert
+scoped to that object, so this was never a cross-object hole; it was an
+unbounded **column** surface on one. The way in is an ordinary authoring
+mid-state: the author creates the public form and wires its sections later.
+
+**What changes.** A form whose sections declare no fields now answers
+`400 VALIDATION_ERROR` and inserts nothing. The message names the empty
+declaration and gives the author's fix ("wire the fields it collects into the
+form's sections"); it names no object, field or slug, because this reply is
+readable by anyone on the internet. The three authoring shapes that reach it —
+`sections: []`, sections present but declaring no fields, and `sections` omitted
+— are treated identically, and the refusal keys off the **declaration**, not the
+body, so an empty POST is refused too rather than inserting a blank row.
+
+`VALIDATION_ERROR` is the standard ADR-0112 catalog's generic validation
+failure, and what `HttpStatusErrorCodeMap[400]` already names a bare 400. It is
+deliberately not a newly minted `FORM_*` synonym of a condition the catalog
+already covers.
+
+**Why a refusal and not a silent drop.** Dropping the keys would have kept the
+`201` and changed no wire status, but it would swallow data the caller believes
+it wrote — a visitor is told their support ticket was filed and an empty row is
+stored. Loud is also the only answer that reaches the author, who is the one who
+can fix it.
+
+**This is a behaviour change on a shipped success path.** A deployment that
+today collects submissions through a section-less public form starts getting
+`400`s. That form's read side already publishes nothing (`fields: {}`) since
+`#6601`, so it cannot render either — the two planes now enforce the same rule,
+"the form declares what it collects", on both. **Fix: declare the fields in the
+form's `sections`.** Forms that already declare sections are entirely
+unaffected — that path never consulted the removed limb.
+
+`#3022`'s anchor guarantee is preserved unchanged: `owner_id`,
+`organization_id`, `id` and the audit columns remain unsuppliable on this
+surface, including when a FormView mis-declares one in a section.

content/docs/ui/forms.mdx

@@ -96,6 +96,7 @@ export default defineView({
 > Rules:
 > - The slug in `publicLink` (`contact-us`) becomes the `:slug` segment in the REST URL.
 > - Anything not in the `sections[].fields[]` whitelist is silently stripped at submit time. Treat the whitelist as the form's authoritative "what the public is allowed to set" list.
+> - A form whose sections declare **no** fields collects nothing, so the submit is **refused** (`400 VALIDATION_ERROR`) rather than accepting whatever the caller sent (#6920). Its `GET /forms/:slug` publishes no schema either (#6601) — declare the fields and both planes come alive together.
 > - Multiple form views per object are fine — only the one(s) with `sharing.allowAnonymous === true` are exposed.
 
 ## 2. (Optional) Create the `guest_portal` permission set
@@ -223,6 +224,7 @@ Errors:
 | Status | Code | When |
 |---|---|---|
 | `400 INVALID_REQUEST` | missing / blank slug (an empty body is coerced to `{}` and surfaces as `VALIDATION_FAILED` below, not here) |
+| `400 VALIDATION_ERROR` | the form's sections declare **no** fields, so it collects nothing — wire the fields and resubmit (#6920) |
 | `400 VALIDATION_FAILED` | object schema validators fail (`required`, `format`, `length`, …) |
 | `403 PERMISSION_DENIED` | the resolved profile does not allow create on the target object |
 | `404 FORM_NOT_FOUND` | slug not registered on any `sharing.allowAnonymous: true` view |

packages/rest/src/public-form-routes.test.ts

@@ -2,11 +2,16 @@
 //
 // [#3022] Anonymous public-form routes (ADR-0056 Option A) — server-managed
 // anchor enforcement. The submit route must never accept `owner_id` /
-// `organization_id` / audit columns from a visitor: not via an explicit
-// section declaration, and not via the zero-declared-sections fallback that
-// previously merged the raw body wholesale (the insert-forge of #3004, but
-// with no credentials at all). The resolve/lookup routes must agree with the
-// submit boundary so a form never collects what the submit refuses.
+// `organization_id` / audit columns from a visitor, not even via an explicit
+// section declaration (the insert-forge of #3004, but with no credentials at
+// all). The resolve/lookup routes must agree with the submit boundary so a
+// form never collects what the submit refuses.
+//
+// [#6601 / #6920] And a form that declares NO fields publishes nothing and
+// accepts nothing — one rule on both planes. #3022 originally pinned the
+// zero-declared-sections FALL-THROUGH as intentional alongside its anchor
+// property; maintainer ruling 5229989845 re-judged that half. See the #6920
+// block for what replaced it and what was kept.
 
 import { describe, it, expect, vi } from 'vitest';
 import { RestServer } from './rest-server';
@@ -121,22 +126,44 @@ describe('POST /forms/:slug/submit — server-managed anchors (#3022)', () => {
     expect(createData.mock.calls[0][0].data).toEqual({ subject: 'Help', email: 'a@b.c' });
   });
 
-  it('zero declared sections: business fields fall through, anchors do NOT', async () => {
-    // The all-fields fallback previously merged the raw body wholesale —
-    // the unauthenticated insert-forge of the issue. Business fields keep
-    // the documented fall-through; every server-managed anchor is excluded.
-    const { submit, createData } = buildServer([]);
+  // [#6920] The case that used to live here — 'zero declared sections:
+  // business fields fall through, anchors do NOT' — asserted the
+  // `allowedFields.size === 0 ||` fall-through as INTENDED, and showed an
+  // undeclared `status` being accepted to prove it. Maintainer ruling
+  // 5229989845 (2026-08-09) re-judged that pin: the fall-through half was a
+  // wrong invariant (an anonymous visitor could mass-assign the whole object),
+  // and the section-less form now REFUSES — pinned in the #6920 block below.
+  //
+  // #3022's ANCHOR half is untouched and stays pinned, in the stronger of the
+  // two spellings it ever had: 'declared-field whitelist' above supplies the
+  // full `FORGED` anchor set to a form that even MIS-DECLARES `owner_id`, and
+  // the anchors are still dropped. That is the property #3022 bought. Its
+  // zero-sections spelling is now vacuous — a refused submit inserts nothing,
+  // so "anchors are not among what was inserted" is true of a set that does
+  // not exist — which is why it is replaced rather than re-spelled, and why
+  // the #6920 block pins `createData` was never called at all. The anchor
+  // property is additionally re-asserted on the accepting path below, where it
+  // still does work.
+
+  it('anchors alone on a DECLARED form: nothing forgeable survives (#3022, on the accepting path)', async () => {
+    // The anchor half of #3022, standing on its own: every server-managed key
+    // and nothing else. The form accepts (it declares `subject`), so this
+    // exercises the real filter rather than a refusal that trivially inserts
+    // nothing.
+    const { submit, createData } = buildServer([{ fields: ['subject'] }]);
     const res = mockRes();
-    await submit.handler(
-      { params: { slug: 'test' }, body: { subject: 'Help', status: 'open', ...FORGED } } as any,
-      res,
-    );
+    await submit.handler({ params: { slug: 'test' }, body: { ...FORGED } } as any, res);
     expect(res.statusCode).toBe(201);
-    expect(createData.mock.calls[0][0].data).toEqual({ subject: 'Help', status: 'open' });
+    expect(createData.mock.calls[0][0].data).toEqual({});
   });
 
   it('a __proto__ body key cannot smuggle inherited anchors past the filter', async () => {
-    const { submit, createData } = buildServer([]);
+    // [#6920] Re-spelled from `buildServer([])` to a form that DECLARES
+    // `subject`. The prototype-pollution property was never about zero
+    // sections — it rode the fall-through only because that was the shortest
+    // way to get a body through the filter. On the declared path it is a
+    // stronger pin, because the assignment loop it guards actually runs.
+    const { submit, createData } = buildServer([{ fields: ['subject'] }]);
     const res = mockRes();
     // JSON.parse produces `__proto__` as an OWN key; a naive `obj[k] = v`
     // assignment would replace the payload's prototype with this object and
@@ -159,6 +186,130 @@ describe('POST /forms/:slug/submit — server-managed anchors (#3022)', () => {
   });
 });
 
+describe('POST /forms/:slug/submit — a form that declares nothing accepts nothing (#6920)', () => {
+  // The write-side twin of #6601, ruled by maintainer comment 5229989845.
+  //
+  // Before this pin the filter read `allowedFields.size === 0 || allowedFields.has(k)`.
+  // A form with no declared fields therefore did NOT accept "every field of the
+  // object" — it accepted every KEY THE CALLER SENT, minus the #3022 anchors and
+  // the three prototype keys. Measured on the real registered handler at
+  // `origin/main`, anonymously, with a `sections: []` form:
+  //
+  //     submit accepted = ["email","internal_margin","internal_tier",
+  //                        "not_even_a_field","status","subject"]
+  //
+  // `not_even_a_field` is not declared on `ticket` at all, which is what makes
+  // "unbounded" literal rather than rhetorical: an anonymous visitor could set
+  // `status`, an internal tier, a formula column — anything, on the one object
+  // the form targets. A form created before its sections are wired is an
+  // ordinary authoring mid-state, so nothing exotic was needed to reach it.
+  //
+  // Why a REFUSAL and not a silent drop: dropping the keys keeps the `201`, so a
+  // caller that believes it submitted a support ticket is told it succeeded and
+  // gets an empty row — the silence AGENTS.md's warn-vs-error rule names. Loud
+  // is also the only answer that reaches the AUTHOR, who is the one who can fix
+  // it. And a refusal that still INSERTED (a blank row per visitor) would be a
+  // worse bug than the one being fixed, so every case here pins `createData`.
+
+  const BUSINESS_KEYS = {
+    subject: 'x',
+    email: 'a@b.c',
+    status: 'closed',
+    internal_tier: 'strategic',
+    internal_margin: '999',
+    not_even_a_field: 'accepted',
+  };
+
+  async function submitTo(sections: any[] | undefined, body: any) {
+    const { submit, createData } = buildServer(sections);
+    const res = mockRes();
+    await submit.handler({ params: { slug: 'test' }, body } as any, res);
+    return { res, createData };
+  }
+
+  it('zero declared sections: the measured accepted set is REFUSED, envelope and all', async () => {
+    const { res } = await submitTo([], { ...BUSINESS_KEYS, ...FORGED });
+    // Both halves of the ADR-0112 envelope. A `toThrow`-shaped assertion would
+    // not do here: this route never throws on the fall-through — it ANSWERED
+    // 201 — so only the code+status pair separates "refused" from "accepted",
+    // and only the code separates it from an unrelated 400 (a blank slug is
+    // `INVALID_REQUEST`, a failed object validator is `VALIDATION_FAILED`).
+    expect(res.statusCode).toBe(400);
+    expect(res.body.code).toBe('VALIDATION_ERROR');
+  });
+
+  it('zero declared sections: NOTHING is written — the refusal does not insert a blank row', async () => {
+    const { createData } = await submitTo([], { ...BUSINESS_KEYS, ...FORGED });
+    expect(createData, 'a refusal must not reach the data plane at all').toHaveBeenCalledTimes(0);
+  });
+
+  it('the refusal names the empty declaration and tells the author to wire sections', async () => {
+    // The wording is contract here (#5240): this is the only signal the AUTHOR
+    // gets, and "wire your sections" is the entire fix. Asserted on top of
+    // code+status, never instead of them.
+    const { res } = await submitTo([], BUSINESS_KEYS);
+    expect(res.body.error).toContain('declares no fields');
+    expect(res.body.error).toContain('sections');
+  });
+
+  it('the refusal leaks no object internals — it is readable by anyone on the internet', async () => {
+    const { res } = await submitTo([], BUSINESS_KEYS);
+    const wire = JSON.stringify(res.body);
+    for (const leak of ['ticket', 'internal_tier', 'internal_margin', 'not_even_a_field', 'owner_id']) {
+      expect(wire, `${leak} must not appear in an anonymous refusal`).not.toContain(leak);
+    }
+  });
+
+  it('sections that exist but declare no fields are refused too', async () => {
+    // Same degenerate `allowedFields`, reached by a different authoring shape —
+    // and the same case the read side already pins ('sections that exist but
+    // declare no fields publish nothing either').
+    const { res, createData } = await submitTo([{ label: 'Details', fields: [] }, { label: 'More' }], BUSINESS_KEYS);
+    expect(res.statusCode).toBe(400);
+    expect(res.body.code).toBe('VALIDATION_ERROR');
+    expect(createData).toHaveBeenCalledTimes(0);
+  });
+
+  it('`sections` omitted entirely is refused too', async () => {
+    const { res, createData } = await submitTo(undefined, BUSINESS_KEYS);
+    expect(res.statusCode).toBe(400);
+    expect(res.body.code).toBe('VALIDATION_ERROR');
+    expect(createData).toHaveBeenCalledTimes(0);
+  });
+
+  it('the refusal is on the DECLARATION, not on the body: an empty body is refused identically', async () => {
+    // Gating on "the caller sent undeclared keys" instead would let an empty
+    // POST through and insert the blank row this whole card is about. The form
+    // collects nothing, so there is no body it can accept.
+    const { res, createData } = await submitTo([], {});
+    expect(res.statusCode).toBe(400);
+    expect(res.body.code).toBe('VALIDATION_ERROR');
+    expect(createData).toHaveBeenCalledTimes(0);
+  });
+
+  it('NO-REGRESSION: a form that declares sections accepts exactly those fields, as it always did', async () => {
+    // GUARD, not evidence. This assertion held BEFORE the change too — the
+    // declared path always took `allowedFields.has(k)` and never consulted the
+    // removed limb. It is here to prove working forms did not break.
+    const { res, createData } = await submitTo(
+      [{ fields: ['subject', { field: 'email' }] }],
+      { ...BUSINESS_KEYS, ...FORGED },
+    );
+    expect(res.statusCode).toBe(201);
+    expect(createData).toHaveBeenCalledTimes(1);
+    expect(createData.mock.calls[0][0].data).toEqual({ subject: 'x', email: 'a@b.c' });
+  });
+
+  it('NO-REGRESSION: a declaring form still accepts an empty body (201, empty row)', async () => {
+    // GUARD. The refusal must key off the DECLARATION only — a declaring form
+    // with an empty body behaves exactly as before, letting the object's own
+    // validators and hooks have the last word rather than this filter.
+    const { res, createData } = await submitTo([{ fields: ['subject'] }], {});
+    expect(res.statusCode).toBe(201);
+    expect(createData.mock.calls[0][0].data).toEqual({});
+  });
+});
+
 describe('GET /forms/:slug — schema/sections agree with the submit boundary (#3022)', () => {
   // The zero-sections case used to live here as "the all-fields schema
   // expansion excludes managed anchors", asserting
@@ -193,13 +344,13 @@ describe('GET /forms/:slug — the published schema IS the declared field set (#
   // form" was simply false there.
   //
   // Why the fix is "declare it or it is not published" rather than "publish
-  // what the submit route accepts": the submit route's accepted set degenerates
-  // the SAME way for a section-less form (`allowedFields.size === 0 ||`, pinned
-  // by 'zero declared sections: business fields fall through' above, which
-  // shows an undeclared `status` being accepted). Aligning the read surface to
-  // that write surface would have republished exactly the set we are removing.
-  // The submit-side whitelist is a WRITE control and never was the backstop for
-  // a READ disclosure.
+  // what the submit route accepts": when this landed, the submit route's
+  // accepted set degenerated the SAME way for a section-less form
+  // (`allowedFields.size === 0 ||`), so aligning the read surface to that write
+  // surface would have republished exactly the set being removed. The
+  // submit-side whitelist is a WRITE control and never was the backstop for a
+  // READ disclosure. #6920 has since closed the write-side twin, so both planes
+  // enforce the declaration — each on its own, not by deferring to the other.
 
   const SENSITIVE = ['internal_margin', 'internal_tier'];