feat(api): strict api key validation with issue paths

.changeset/api-v1-strict-validation.md

@@ -0,0 +1,11 @@
+---
+'@quill/db': minor
+---
+
+The admin API explains its validation errors and holds API keys to what the editor can show.
+
+- Every validation 400 of the form, branding and destinations routes now carries `issues: [{ path, message, code }]` with every problem at once. `path` is the dot path inside the request body (`config.steps.2.question`); `message` is still the first issue's, so nothing that reads only `message` changes.
+- `POST /v1/forms` drops `formGuid` and `formSignature` from HubSpot destinations, for every caller, as duplicating already did. A config copied from another form no longer points the new form at the original's HubSpot mirror form. `@quill/db` exports the helper both paths use, `withoutOwnedMirrorState`.
+- With an API key only (a browser session saves exactly as before, legacy fields included), a form config is refused with `UNSUPPORTED_FIELD` and the field's exact path when it carries a field the editor has no control for: `outcomes[].booking`, `steps[].buttonText`, `steps[].sliderUnitLabel`, `steps[].reveal.subtitleTemplate`, `steps[].reveal.enabled`, `steps[].scheduler.prefill`, `steps[].flowGroup`, `branding.clientLogos`, the webhook `signatureHeader` and `timeoutMs` (checked where destinations are written: `POST /v1/forms` and `PUT /v1/forms/:id/destinations`; `PUT /v1/forms/:id` drops destinations from the draft, so it does not refuse them), and the legacy `config.reveal`, `config.revealAfterStep` and `steps[].triggersReveal`. The value the editor itself writes passes, so a form read and sent straight back saves: the derived `flowGroup`, the step defaults `buttonText` "Continue", `reveal.enabled` true and `scheduler.prefill` true (on any step, since changing a step's type in the builder keeps them), and the brand kit's own client logos.
+- With an API key, a step without a question is refused with `EMPTY_QUESTION` (a reveal step asks nothing and is exempt), and `PUT /v1/forms/:id` with a config that has no `steps` answers 400 `STEPS_REQUIRED` and writes nothing.
+- With an API key, the saved config is the engine's normalized one (`normalizeConfig`: unique step keys, deduplicated option values, derived flow groups and scoring), and the response returns it.

apps/api/src/admin-crud.controller.ts

@@ -45,6 +45,7 @@ import {
   setMemberStatus,
   updateForm,
   upsertNotificationSetting,
+  withoutOwnedMirrorState,
   type CrudResult,
   getMemberProfileRaw,
   setMemberLocale,
@@ -88,7 +89,6 @@ import {
   workspaceTimezoneSchema,
   type FormConfig,
 } from '@quill/types';
-import { ZodError } from 'zod';
 import { AdminService } from './admin.service';
 import { SubmissionService } from './submission.service';
 import { AnalyticsService } from './analytics.service';
@@ -98,23 +98,15 @@ import { EmailEffects } from './email-effects';
 import { AnalyticsEffects } from './analytics-effects';
 import { UploadService } from './upload.service';
 import { unwrap } from './http';
+import { parse } from './validation';
+import { keyFormBodyIssues, keyNormalizedConfig } from './strict-config';
 import type { ServerEnv } from '@quill/config/env';
 import { assertAdmin, assertCanManageTarget, assertNotSelf, assertOwner } from './permissions';
 import { parseIntParam, parseKinds, parseOutboxStatuses } from './query-params';
 import { parseSort, parseSubmissionFilter, workspaceZone, type FilterQuery } from './submission-filter';
 import type { CaptchaVerifier } from './captcha';
 import { CAPTCHA, DB, ENV } from './tokens';
 
-function parse<T>(schema: { parse: (v: unknown) => T }, body: unknown): T {
-  try {
-    return schema.parse(body);
-  } catch (err) {
-    if (err instanceof ZodError)
-      throw new BadRequestException({ error: 'BAD_REQUEST', message: err.issues[0]?.message });
-    throw err;
-  }
-}
-
 function unwrapCrud<T>(r: CrudResult<T>): T {
   if (r.ok) return r.value;
   if (r.reason === 'NOT_FOUND') throw new NotFoundException({ error: 'NOT_FOUND', message: 'Not found.' });
@@ -585,7 +577,11 @@ export class AdminCrudController {
     const p = await this.auth.resolveHost(req);
     // The create schema (not the shared input one): only creation may name a
     // folder; PUT /v1/forms/:id never moves a form, PATCH /forms/:id/folder does.
-    const input = parse(formCreateInputSchema, body);
+    const input = parse(
+      formCreateInputSchema,
+      body,
+      await keyFormBodyIssues(this.db, p, body, { requireSteps: false, storesDestinations: true }),
+    );
     // The other path that AUTHORS a `destinations` array (PUT /forms/:id stages
     // a draft, and drafts strip the key; duplicate copies stored state and is
     // exempt — see `hasExtraHubspotDestination`). Same one-HubSpot rule as
@@ -602,7 +598,9 @@ export class AdminCrudController {
     // kit exactly like dashboard-created ones. Caller-supplied branding wins
     // over the kit (explicit input beats a default).
     const kit = await getAccountBranding(this.db, p.accountId);
-    let config: unknown = input.config;
+    // The HubSpot mirror's `formGuid`/`formSignature` belong to the form that
+    // made them (a copied config would aim this one at it), as on duplicate.
+    let config: unknown = withoutOwnedMirrorState(keyNormalizedConfig(p, input.config));
     if (kit) {
       const base =
         config && typeof config === 'object' && !Array.isArray(config)
@@ -657,7 +655,12 @@ export class AdminCrudController {
   @Put('forms/:id')
   async updateForm(@Req() req: ReqLike, @Param('id') id: string, @Body() body: unknown) {
     const p = await this.auth.resolveHost(req);
-    const { config, slug, expectedUpdatedAt, ...meta } = parse(formInputSchema.partial(), body);
+    const { config: sent, slug, expectedUpdatedAt, ...meta } = parse(
+      formInputSchema.partial(),
+      body,
+      await keyFormBodyIssues(this.db, p, body, { requireSteps: true, storesDestinations: false }),
+    );
+    const config = keyNormalizedConfig(p, sent);
     if (slug !== undefined) {
       unwrapCrud(await setFormSlug(this.db, p.accountId, id, slugify(slug)));
     }

apps/api/src/branding.controller.ts

@@ -1,5 +1,4 @@
 import {
-  BadRequestException,
   Body,
   Controller,
   Get,
@@ -16,21 +15,12 @@ import {
   upsertAccountBranding,
 } from '@quill/db';
 import { brandKitSchema } from '@quill/types';
-import { z, ZodError } from 'zod';
+import { z } from 'zod';
 import { AuthService, type ReqLike } from './auth.service';
 import { assertAdmin } from './permissions';
+import { parse } from './validation';
 import { DB } from './tokens';
 
-function parse<T>(schema: { parse: (v: unknown) => T }, body: unknown): T {
-  try {
-    return schema.parse(body);
-  } catch (err) {
-    if (err instanceof ZodError)
-      throw new BadRequestException({ error: 'BAD_REQUEST', message: err.issues[0]?.message });
-    throw err;
-  }
-}
-
 const formIdsSchema = z.object({ formIds: z.array(z.string().min(1)).min(1).max(500) });
 
 /**

apps/api/src/integrations.controller.ts

@@ -46,6 +46,8 @@ import { syncMirrorForm, type MirrorSyncResult } from './hubspot-mirror';
 import type { ServerEnv } from '@quill/config/env';
 import { AuthService, type ReqLike } from './auth.service';
 import { assertAdmin } from './permissions';
+import { parse } from './validation';
+import { keyDestinationsBodyIssues } from './strict-config';
 import { RateLimitGuard } from './rate-limit';
 import { captchaActive, type CaptchaVerifier } from './captcha';
 import { fileLinksFor } from './file-link';
@@ -752,14 +754,7 @@ export class FormDestinationsController {
   @Put('forms/:id/destinations')
   async putDestinations(@Req() req: ReqLike, @Param('id') id: string, @Body() body: unknown) {
     const p = await this.auth.resolveHost(req);
-    let destinations: unknown[];
-    try {
-      destinations = destinationsBodySchema.parse(body).destinations;
-    } catch (err) {
-      if (err instanceof ZodError)
-        throw new BadRequestException({ error: 'BAD_REQUEST', message: err.issues[0]?.message });
-      throw err;
-    }
+    const { destinations } = parse(destinationsBodySchema, body, keyDestinationsBodyIssues(p, body));
     // At most one HubSpot destination — see `hasExtraHubspotDestination`. Not in
     // the schema above, so a form that already stores two still READS fine.
     // Compared against what is STORED, not against 1: the builder's per-question

apps/api/src/strict-config.ts

@@ -0,0 +1,227 @@
+/**
+ * The stricter rules a form config is held to when an API KEY writes it.
+ *
+ * An agent will write any field the schema accepts, so with a key the API
+ * refuses what a person could not see or edit afterwards in the builder: the
+ * fields that have no control in the editor (UNSUPPORTED_FIELD), steps with no
+ * question (EMPTY_QUESTION), and a PUT config without `steps`, which would
+ * replace every step with none (STEPS_REQUIRED). What a key saves is the
+ * engine's normalized config, the same canonical shape the editor saves.
+ *
+ * None of this applies to a browser session. The editor saves through the very
+ * same routes, and forms built before these fields lost their control still
+ * carry them; refusing them there would stop people saving their own forms.
+ *
+ * The rules read the raw body (duck-typed), not the parsed one, so their
+ * issues join the schema's in a single 400 even when the schema refuses too.
+ */
+import { isDeepStrictEqual } from 'node:util';
+import type { Db } from '@quill/db';
+import { getAccountBranding } from '@quill/db';
+import {
+  defaultFlowGroup,
+  normalizeConfig,
+  type FormConfig as EngineConfig,
+  type FormFieldType,
+} from '@quill/engine';
+import { isApiKeyPrincipal, type HostPrincipal } from './auth.provider';
+import { dotPath, type ValidationIssue } from './validation';
+
+export const UNSUPPORTED_FIELD = 'UNSUPPORTED_FIELD';
+export const EMPTY_QUESTION = 'EMPTY_QUESTION';
+export const STEPS_REQUIRED = 'STEPS_REQUIRED';
+
+const UNSUPPORTED_MESSAGE = 'This field has no control in the editor.';
+const EMPTY_QUESTION_MESSAGE = 'Every step needs a question.';
+const STEPS_REQUIRED_MESSAGE =
+  'Send the full steps array: a config without steps would replace every step with none.';
+
+type Path = Array<string | number>;
+type Obj = Record<string, unknown>;
+
+const isObj = (v: unknown): v is Obj => !!v && typeof v === 'object' && !Array.isArray(v);
+/** `null` stores nothing a person would need a control for, so it reads as absent. */
+const isSet = (v: unknown): boolean => v !== undefined && v !== null;
+
+/**
+ * A step field with no control in the editor, and the one value the editor
+ * itself writes there (`createEmptyStep` and `normalizeConfig`). That value is
+ * accepted: every step the builder makes carries it, and so does every config
+ * this API answers with, so refusing it would refuse a form read and sent
+ * straight back. Any other value is one the builder cannot show.
+ *
+ * The seeded values pass on ANY step type, not only the type that seeds them:
+ * changing a step's type in the builder keeps them (a message turned into a
+ * text question still carries `buttonText` "Continue"). `flowGroup` is the
+ * exception, because the type change rewrites it to the new type's default.
+ */
+interface StepField {
+  path: Path;
+  editorWrites?: (value: unknown, type: unknown) => boolean;
+}
+
+/** The research list of config fields without an editor control (see the API spec). */
+const STEP_FIELDS: StepField[] = [
+  { path: ['buttonText'], editorWrites: (v) => v === 'Continue' },
+  { path: ['sliderUnitLabel'] },
+  { path: ['reveal', 'subtitleTemplate'] },
+  { path: ['reveal', 'enabled'], editorWrites: (v) => v === true },
+  { path: ['scheduler', 'prefill'], editorWrites: (v) => v === true },
+  {
+    path: ['flowGroup'],
+    editorWrites: (v, type) => typeof type === 'string' && v === defaultFlowGroup(type as FormFieldType),
+  },
+  // Legacy: the single form-level reveal's position, before reveal steps.
+  { path: ['triggersReveal'] },
+];
+const OUTCOME_FIELDS: Path[] = [['booking']];
+/** Legacy: the single form-level reveal, before reveal steps. */
+const TOP_LEVEL_FIELDS: Path[] = [['reveal'], ['revealAfterStep']];
+const WEBHOOK_FIELDS: Path[] = [
+  ['settings', 'signatureHeader'],
+  ['settings', 'timeoutMs'],
+];
+
+function at(value: unknown, path: Path): unknown {
+  let node = value;
+  for (const segment of path) {
+    if (!isObj(node)) return undefined;
+    node = node[segment as string];
+  }
+  return node;
+}
+
+const unsupported = (path: Path): ValidationIssue => ({
+  path: dotPath(path),
+  message: UNSUPPORTED_MESSAGE,
+  code: UNSUPPORTED_FIELD,
+});
+
+/** Webhook entries of a `destinations` array carrying fields Connect cannot show. */
+export function strictDestinationIssues(destinations: unknown, prefix: Path): ValidationIssue[] {
+  if (!Array.isArray(destinations)) return [];
+  const issues: ValidationIssue[] = [];
+  destinations.forEach((entry, i) => {
+    if (!isObj(entry) || entry.type !== 'webhook') return;
+    for (const field of WEBHOOK_FIELDS) {
+      if (isSet(at(entry, field))) issues.push(unsupported([...prefix, i, ...field]));
+    }
+  });
+  return issues;
+}
+
+interface StrictConfigOptions {
+  /** Where the config sits in the request body (`['config']`). */
+  prefix: Path;
+  /**
+   * The workspace brand kit's client logos. Creating a form (and applying the
+   * kit) copies them into `branding.clientLogos`, so that exact value is the
+   * server's own and passes; any other is refused like the rest.
+   */
+  kitClientLogos?: unknown;
+  /**
+   * Whether the route stores `config.destinations`. `POST /v1/forms` does;
+   * `PUT /v1/forms/:id` drops them from the draft (the integrations screen owns
+   * them live), so a webhook field there is never saved and refusing it would
+   * refuse every edit of a form whose live webhook already carries one.
+   */
+  destinations: boolean;
+}
+
+/** Every strict-rule issue of one config, steps in order. */
+export function strictConfigIssues(config: unknown, opts: StrictConfigOptions): ValidationIssue[] {
+  if (!isObj(config)) return [];
+  const { prefix } = opts;
+  const issues: ValidationIssue[] = [];
+
+  if (Array.isArray(config.steps)) {
+    config.steps.forEach((step, i) => {
+      if (!isObj(step)) return;
+      // A reveal asks nothing: it has no title and plays its own copy.
+      // A question of the wrong type is the schema's to report, not ours.
+      const question = step.question;
+      const blank = !isSet(question) || (typeof question === 'string' && question.trim() === '');
+      if (step.type !== 'reveal' && blank) {
+        issues.push({
+          path: dotPath([...prefix, 'steps', i, 'question']),
+          message: EMPTY_QUESTION_MESSAGE,
+          code: EMPTY_QUESTION,
+        });
+      }
+      for (const field of STEP_FIELDS) {
+        const value = at(step, field.path);
+        if (!isSet(value) || field.editorWrites?.(value, step.type)) continue;
+        issues.push(unsupported([...prefix, 'steps', i, ...field.path]));
+      }
+    });
+  }
+
+  if (Array.isArray(config.outcomes)) {
+    config.outcomes.forEach((outcome, i) => {
+      for (const field of OUTCOME_FIELDS) {
+        if (isSet(at(outcome, field))) issues.push(unsupported([...prefix, 'outcomes', i, ...field]));
+      }
+    });
+  }
+
+  const logos = at(config, ['branding', 'clientLogos']);
+  // Key order is not part of the value (Postgres jsonb reorders keys).
+  if (isSet(logos) && !isDeepStrictEqual(logos, opts.kitClientLogos)) {
+    issues.push(unsupported([...prefix, 'branding', 'clientLogos']));
+  }
+
+  for (const field of TOP_LEVEL_FIELDS) {
+    if (isSet(at(config, field))) issues.push(unsupported([...prefix, ...field]));
+  }
+
+  if (opts.destinations) {
+    issues.push(...strictDestinationIssues(config.destinations, [...prefix, 'destinations']));
+  }
+  return issues;
+}
+
+/**
+ * The strict-rule issues of a form write's body (`POST /v1/forms`,
+ * `PUT /v1/forms/:id`) for this principal: none for a session. `requireSteps`
+ * is the PUT's: a `config` without `steps` there replaces every step with none.
+ * The PUT's draft also drops `destinations`, so their webhook fields are only
+ * checked on create (and on `PUT .../destinations`).
+ */
+export async function keyFormBodyIssues(
+  db: Db,
+  p: HostPrincipal,
+  body: unknown,
+  opts: { requireSteps: boolean; storesDestinations: boolean },
+): Promise<ValidationIssue[]> {
+  if (!isApiKeyPrincipal(p) || !isObj(body) || !isObj(body.config)) return [];
+  const config = body.config;
+  const issues: ValidationIssue[] = [];
+  if (opts.requireSteps && config.steps === undefined) {
+    issues.push({ path: 'config.steps', message: STEPS_REQUIRED_MESSAGE, code: STEPS_REQUIRED });
+  }
+  // Only read the kit when there is something to compare it with.
+  const kitClientLogos = isSet(at(config, ['branding', 'clientLogos']))
+    ? (await getAccountBranding(db, p.accountId))?.config.clientLogos
+    : undefined;
+  issues.push(
+    ...strictConfigIssues(config, { prefix: ['config'], kitClientLogos, destinations: opts.storesDestinations }),
+  );
+  return issues;
+}
+
+/** The strict-rule issues of a `PUT /v1/forms/:id/destinations` body: none for a session. */
+export function keyDestinationsBodyIssues(p: HostPrincipal, body: unknown): ValidationIssue[] {
+  if (!isApiKeyPrincipal(p) || !isObj(body)) return [];
+  return strictDestinationIssues(body.destinations, ['destinations']);
+}
+
+/**
+ * The config an API key saves: the engine's normalized one (unique step keys
+ * with references following them, deduped option values, derived flow groups
+ * and scoring, sorted outcomes), the same canonical shape the editor saves. A
+ * session's config is stored as sent, exactly as before.
+ */
+export function keyNormalizedConfig<T>(p: HostPrincipal, config: T): T {
+  if (!isApiKeyPrincipal(p) || config == null) return config;
+  return normalizeConfig(config as unknown as EngineConfig) as unknown as T;
+}