fix(service-settings): 保存期强制 select 声明的 options (#5131) (#5151)

`SettingsService.validatePatch` 只校验 `required` 与 `pattern` 两项,
manifest 声明的 `options` 表从头到尾不参与校验。走控制台碰不到——下拉框
只会发出合法值;但 `PUT /api/settings/:ns` 是公开的可授权面,脚本、迁移
工具、AI 写的初始化代码可以直接写入枚举外的值,而且一路静默:存下来了,
读回来了,消费端各自随机应对。这不是 mail 专有的,所有 `type: 'select'`
的键都一样。

这正是 #5094 缺失的 API 侧闸门:那一单把 `sendgrid` / `ses` 从 mail 的
选项表里退役(本服务器无法通过它们投递),而没有写入期强制,刚退役的值
当天就能被重新写回去。

现在 `select` / `radio` / `multiselect` 的越界值以 `invalid_option` 拒绝,
`constraint` 带上允许值集合(ADR-0114:constraint kind 在失败点打戳,
不让路由层从文案反推)。强制的类型集合取自 spec 自身——`SpecifierSchema`
的 superRefine 恰好要求这三类声明非空 `options`,所以「声明」与「强制」
指的是同一份清单,不存在第三份会漂移的列表。

两条刻意的边界:

- **按 touch 语义校验**,与既有 required/pattern 一致。存量越界值只让
  写该键的那次 patch 失败,只改 `from_name` 不会因为库里躺着一个老的
  `provider` 而被拒。相反做法会把带历史脏值的工作区锁死在设置页里改不动
  任何东西,比现状更糟。全 null 的重置永不阻塞。
- **没有声明 options 的 specifier 放行**:它说不出什么是合法的,保持宽容
  而不是拒绝所有写入。

值按字符串形态比较,声明为 `value: 30` 的选项经 JSON 或表单往返后仍然匹配。
不设逃生舱:确需自定义值的 manifest 应在 spec 侧显式声明,而不是靠消费端宽容。


Claude-Session: https://claude.ai/code/session_017MCKJaEomEqg4tvz4SzdNd

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/settings-select-options-enforced.md

@@ -0,0 +1,53 @@
+---
+"@objectstack/service-settings": patch
+---
+
+fix(service-settings): a settings `select` now rejects values outside its declared `options` (#5131)
+
+`SettingsService.validatePatch` enforced two of the constraints a settings
+manifest declares — `required` and `pattern` — and skipped the third. A
+specifier's `options` table never took part in save-time validation, so any
+string at all could be written into a dropdown field:
+
+```ts
+await svc.setMany('mail', { provider: 'sendgrid', from_email: 'a@b.com' }); // stored
+```
+
+Going through the console this was unreachable: the dropdown only ever emits a
+value from the table. But `PUT /api/settings/:ns` is an authorizable public
+surface, and scripts, migration tools and AI-authored bootstrap code write it
+directly — where the bad value was accepted, persisted and read back **in
+silence**, leaving every consumer to improvise its own answer for an
+enumeration member that does not exist. It was not `mail`-specific:
+`storage.adapter`, `sms.provider`, `ai.provider`, `localization.date_format` and
+every other `select` behaved the same way.
+
+This is the API-side gate that #5094 was missing. That change retired
+`sendgrid` / `ses` from the `mail` provider table because this server cannot
+deliver through them — with no write-side enforcement, the values it had just
+retired could be written straight back in the same afternoon.
+
+**Now:** a `select` / `radio` / `multiselect` value that is not a member of the
+declared table is rejected with a `FieldError` whose `code` is `invalid_option`
+and whose `constraint` carries the allowed set (`{ allowed: 'smtp, resend,
+postmark, log' }`), so a client composes its own message instead of parsing
+ours. The enforced set is the spec's own: `SpecifierSchema` already *requires* a
+non-empty `options` on exactly those three types, so declared and enforced name
+one list rather than two that can drift.
+
+Two deliberate limits keep this from breaking workspaces that already carry
+drift:
+
+- **The check is gated on TOUCH**, like `required` and `pattern` before it. A
+  value that pre-dates the current option table only fails the patch that
+  writes that key — editing `from_name` is not rejected because a stale
+  `provider` sits in the store. The opposite rule would lock every workspace
+  with historical drift out of its own settings page entirely, which is worse
+  than the gap being closed. Resets (all-null patches) are never blocked.
+- **A specifier that declares no option table is left alone.** It cannot say
+  what is legal, so it stays lenient rather than rejecting every write.
+
+Values are compared in string form, so an option declared `value: 30` still
+matches after a round trip through JSON or a form post. There is no opt-out: a
+manifest that needs to accept custom values would declare that explicitly in
+the spec, not rely on a tolerant consumer.

packages/services/service-settings/src/envelope.conformance.test.ts

@@ -430,6 +430,33 @@ describe('settings envelope (#4224) — SETTINGS_VALIDATION speaks the field-lev
     expect(fields.map((f: any) => f.code).sort()).toEqual(['invalid_format', 'required']);
   });
 
+  it('an out-of-table select reaches the client as a parseable invalid_option (#5131)', async () => {
+    const { http, service } = mount();
+    service.registerManifest({
+      namespace: 'enumerated',
+      label: 'Enumerated',
+      writePermission: 'setup.write',
+      readPermission: 'setup.access',
+      specifiers: [
+        { key: 'provider', type: 'select', label: 'Provider',
+          options: [{ value: 'smtp', label: 'SMTP' }, { value: 'log', label: 'Log' }] },
+      ],
+    } as any);
+    const { status, body } = await drive(http, 'PUT /api/settings/:namespace', {
+      params: { namespace: 'enumerated' },
+      body: { provider: 'sendgrid' },
+    });
+    expect(status).toBe(400);
+    expect(body.error.code).toBe('SETTINGS_VALIDATION');
+
+    const [field] = body.error.details.fields;
+    expect(FieldErrorSchema.safeParse(field).success).toBe(true);
+    // The constraint kind is stamped where the check failed, so the route
+    // never has to infer it back out of the prose (ADR-0114).
+    expect(field.code).toBe('invalid_option');
+    expect(field.constraint).toEqual({ allowed: 'smtp, log' });
+  });
+
   it('the pre-#4224 map is gone from both of its old spellings', async () => {
     const http = lockedPattern();
     const { body } = await drive(http, 'PUT /api/settings/:namespace', {

packages/services/service-settings/src/settings-service.test.ts

@@ -413,6 +413,188 @@ describe('SettingsService — save-time validation (required/visible/pattern)',
   });
 });
 
+/**
+ * #5131 — a manifest's `options` table is enforced at SAVE time.
+ *
+ * Until this suite existed the enumeration was a front-end convention: the
+ * console dropdown only ever emitted legal values, so an admin going through
+ * the UI could not produce a bad one — but `PUT /api/settings/:ns` is an
+ * authorizable public surface, and a script, a migration or AI-authored
+ * bootstrap code could write any string at all into a `select` and have it
+ * stored, read back, and improvised over by each consumer in turn.
+ *
+ * The load-bearing case is `mail.provider`: #5094/#5133 retired `sendgrid`
+ * and `ses` from the option table because this server cannot deliver through
+ * them, and that manifest-side tightening had no matching gate on the API
+ * side — the very values just retired could be written straight back in.
+ */
+describe('SettingsService — save-time validation (declared options are enforced)', () => {
+  const mailService = () => {
+    const svc = new SettingsService({ env: {} });
+    svc.registerManifest(mailSettingsManifest);
+    return svc;
+  };
+
+  it('refuses a provider outside the declared table, naming the allowed set', async () => {
+    const svc = mailService();
+    // `sendgrid` left the table in #5094; before this gate it could be written
+    // back the same afternoon it was retired.
+    await expect(
+      svc.setMany('mail', { provider: 'sendgrid', from_email: 'a@b.com' }),
+    ).rejects.toMatchObject({
+      code: 'SETTINGS_VALIDATION',
+      fields: [
+        {
+          field: 'provider',
+          code: 'invalid_option',
+          label: 'Provider',
+          // The allowed set travels as a discrete constraint (ADR-0114), so a
+          // client branches on the machine value instead of parsing our prose.
+          constraint: { allowed: 'smtp, resend, postmark, log' },
+          value: 'sendgrid',
+        },
+      ],
+    });
+    // Atomic: the rejected batch persisted nothing, not even the legal key.
+    expect((await svc.get('mail', 'provider')).source).toBe('default');
+    expect((await svc.get('mail', 'from_email')).value).toBeNull();
+  });
+
+  it('accepts every value the table does declare', async () => {
+    for (const [provider, extra] of [
+      ['smtp', { smtp_host: 'smtp.example.com' }],
+      ['resend', { api_key: 're-key' }],
+      ['postmark', { api_key: 'pm-key' }],
+      ['log', {}],
+    ] as const) {
+      const svc = mailService();
+      await expect(
+        svc.setMany('mail', { provider, ...extra, from_email: 'a@b.com' }),
+      ).resolves.toBeDefined();
+      expect((await svc.get('mail', 'provider')).value).toBe(provider);
+    }
+  });
+
+  it('checks the option table only when the patch TOUCHES the key', async () => {
+    // A workspace that saved `sendgrid` while the option existed still carries
+    // it. Simulated exactly as it happened: write under the OLD table, then
+    // re-register the narrowed manifest (#5094) over the same namespace.
+    const svc = new SettingsService({ env: {} });
+    svc.registerManifest({
+      ...mailSettingsManifest,
+      specifiers: mailSettingsManifest.specifiers.map((s: any) =>
+        s.key === 'provider'
+          ? { ...s, options: [...s.options, { value: 'sendgrid', label: 'SendGrid' }] }
+          : s,
+      ),
+    } as any);
+    await svc.setMany('mail', { provider: 'sendgrid', api_key: 'sg-key', from_email: 'a@b.com' });
+    svc.registerManifest(mailSettingsManifest);
+
+    // The stale value is still there …
+    expect((await svc.get('mail', 'provider')).value).toBe('sendgrid');
+    // … and it does NOT lock the workspace out of editing anything else. A
+    // patch that never mentions `provider` is not rejected on its account —
+    // the opposite rule would make the settings page unusable for every
+    // workspace carrying historical drift, which is worse than the gap.
+    await expect(svc.setMany('mail', { from_name: 'Acme Ops' })).resolves.toBeDefined();
+    expect((await svc.get('mail', 'from_name')).value).toBe('Acme Ops');
+    // Only re-writing the key itself is refused.
+    await expect(svc.setMany('mail', { provider: 'sendgrid' })).rejects.toMatchObject({
+      code: 'SETTINGS_VALIDATION',
+      fields: [{ field: 'provider', code: 'invalid_option' }],
+    });
+    // And a reset still clears it — an all-null patch is never blocked.
+    await expect(svc.resetNamespace('mail')).resolves.toBeGreaterThan(0);
+  });
+
+  it('leaves the value alone when the specifier declares no option table', async () => {
+    // `registerManifest` takes manifests as given (no Zod pass), so a
+    // hand-built select without `options` reaches the validator. It cannot say
+    // what is legal, so it stays lenient rather than rejecting every write.
+    const svc = new SettingsService({ env: {} });
+    svc.registerManifest({
+      namespace: 'freeform',
+      label: 'Freeform',
+      specifiers: [{ type: 'select', key: 'mode', label: 'Mode' }],
+    } as any);
+    await expect(svc.setMany('freeform', { mode: 'whatever' })).resolves.toBeDefined();
+  });
+
+  it('enforces radio and multiselect from the same table, element-wise', async () => {
+    // All three types are covered because the SPEC requires an `options` table
+    // on all three; `radio`/`multiselect` have no producer manifest today and
+    // would otherwise be a hole the first one to author them falls into.
+    const svc = new SettingsService({ env: {} });
+    svc.registerManifest({
+      namespace: 'shapes',
+      label: 'Shapes',
+      specifiers: [
+        { type: 'radio', key: 'tier', label: 'Tier',
+          options: [{ value: 'free', label: 'Free' }, { value: 'pro', label: 'Pro' }] },
+        { type: 'multiselect', key: 'channels', label: 'Channels',
+          options: [{ value: 'email', label: 'Email' }, { value: 'sms', label: 'SMS' }] },
+      ],
+    } as any);
+
+    await expect(svc.setMany('shapes', { tier: 'enterprise' })).rejects.toMatchObject({
+      fields: [{ field: 'tier', code: 'invalid_option', constraint: { allowed: 'free, pro' } }],
+    });
+    await expect(svc.setMany('shapes', { tier: 'pro' })).resolves.toBeDefined();
+
+    // Every element is checked, and the rejected one is the one reported.
+    await expect(
+      svc.setMany('shapes', { channels: ['email', 'carrier-pigeon'] }),
+    ).rejects.toMatchObject({
+      fields: [{ field: 'channels', code: 'invalid_option', value: 'carrier-pigeon' }],
+    });
+    await expect(svc.setMany('shapes', { channels: ['email', 'sms'] })).resolves.toBeDefined();
+    await expect(svc.setMany('shapes', { channels: [] })).resolves.toBeDefined();
+  });
+
+  it('matches option values by string form, so a number option survives JSON', async () => {
+    // A stored value has been through JSON and, over REST, a form post: an
+    // option declared `value: 30` legitimately reads back as '30'. Rejecting
+    // that would enforce the transport rather than the enumeration.
+    const svc = new SettingsService({ env: {} });
+    svc.registerManifest({
+      namespace: 'retention',
+      label: 'Retention',
+      specifiers: [
+        { type: 'select', key: 'days', label: 'Days',
+          options: [{ value: 7, label: '7' }, { value: 30, label: '30' }] },
+        { type: 'select', key: 'archive', label: 'Archive',
+          options: [{ value: true, label: 'On' }, { value: false, label: 'Off' }] },
+      ],
+    } as any);
+    await expect(svc.setMany('retention', { days: 30 })).resolves.toBeDefined();
+    await expect(svc.setMany('retention', { days: '30' })).resolves.toBeDefined();
+    await expect(svc.setMany('retention', { archive: false })).resolves.toBeDefined();
+    await expect(svc.setMany('retention', { days: 45 })).rejects.toMatchObject({
+      fields: [{ field: 'days', code: 'invalid_option', constraint: { allowed: '7, 30' } }],
+    });
+  });
+
+  it('never echoes the rejected value for an encrypted specifier', async () => {
+    // `encrypted` is authorable on any specifier, and this message lands in
+    // logs — so the offending value is named only where it is safe to name.
+    const svc = new SettingsService({ env: {} });
+    svc.registerManifest({
+      namespace: 'vault',
+      label: 'Vault',
+      specifiers: [
+        { type: 'select', key: 'key_ref', label: 'Key', encrypted: true,
+          options: [{ value: 'primary', label: 'Primary' }] },
+      ],
+    } as any);
+    const err = await svc.setMany('vault', { key_ref: 's3cr3t-handle' }).catch((e) => e);
+    expect(err.code).toBe('SETTINGS_VALIDATION');
+    expect(err.fields[0]).toMatchObject({ field: 'key_ref', code: 'invalid_option' });
+    expect(err.fields[0].value).toBeUndefined();
+    expect(err.message).not.toContain('s3cr3t-handle');
+  });
+});
+
 describe('SettingsService — user-scoped values', () => {
   it('isolates writes by ctx.userId', async () => {
     const svc = new SettingsService({ env: {} });

packages/services/service-settings/src/settings-service.ts

@@ -46,6 +46,43 @@ const LAYOUT_ONLY_TYPES = new Set([
   'action_button',
 ]);
 
+/**
+ * Specifier types whose stored value must be a member of the declared
+ * `options` table.
+ *
+ * THE list is the spec's, not a judgement call made here: `SpecifierSchema`'s
+ * superRefine (`settings-manifest.zod.ts`) rejects a manifest that authors one
+ * of exactly these three types without a non-empty `options`. So "the types
+ * that must declare an option table" and "the types whose value is checked
+ * against it" name the same set — declared IS enforced, with no third list to
+ * drift. `radio` and `multiselect` have no producer manifest today; they are
+ * covered anyway because the alternative is that the first manifest to author
+ * one silently re-opens this exact hole.
+ */
+const OPTION_BEARING_TYPES = new Set(['select', 'radio', 'multiselect']);
+
+/**
+ * The declared option values, in string form.
+ *
+ * String form because a stored value has been through JSON (and, over the REST
+ * boundary, a form post): an option declared `value: 30` is legitimately read
+ * back as `'30'`, and rejecting that would be enforcing the transport rather
+ * than the enumeration. Same rule the record validator applies to
+ * `select`/`multiselect` field options (objectui#2729).
+ */
+function declaredOptionValues(options: unknown): string[] {
+  if (!Array.isArray(options)) return [];
+  const out: string[] = [];
+  for (const opt of options) {
+    if (!opt || typeof opt !== 'object') continue;
+    const v = (opt as Record<string, unknown>).value;
+    if (typeof v === 'string' || typeof v === 'number' || typeof v === 'boolean') {
+      out.push(String(v));
+    }
+  }
+  return out;
+}
+
 interface RegisteredManifest {
   manifest: SettingsManifest;
   /** Resolved specifier scopes for fast lookup. */
@@ -613,8 +650,19 @@ export class SettingsService {
    *   is (switching provider must validate that provider's fields).
    * - `required` + visible + empty → rejected.
    * - `pattern` (text fields) + non-empty value that mismatches → rejected.
+   * - `options` (`select`/`radio`/`multiselect`) + non-empty value outside
+   *   the declared table → rejected (`invalid_option`).
    * - All-null patches (namespace reset) and unparseable visibility
    *   expressions skip validation rather than block the write.
+   *
+   * The TOUCH gate is what keeps the options check from being a regression
+   * for existing workspaces: a value that pre-dates a manifest's current
+   * option table (a `mail.provider` of `sendgrid`, retired in #5094) only
+   * fails the patch that writes that key. A patch changing `from_name`
+   * alone is not rejected because a stale `provider` sits in the store —
+   * otherwise every workspace carrying historical drift would be locked out
+   * of its own settings page, unable to edit anything, which is worse than
+   * the gap this closes.
    */
   private async validatePatch(
     namespace: string,
@@ -679,6 +727,58 @@ export class SettingsService {
         });
         continue;
       }
+
+      // A `select`/`radio`/`multiselect` value must be a member of the option
+      // table the manifest declares. Until this check existed the `options`
+      // list was a front-end convention only — the console dropdown emitted
+      // legal values, but `PUT /api/settings/:ns` accepted any string at all,
+      // so a script, a migration or AI-authored bootstrap code could write
+      // `provider: 'sendgrid'` into a namespace that has no such provider and
+      // the write would succeed silently, leaving each consumer to improvise.
+      if (!empty && OPTION_BEARING_TYPES.has(type)) {
+        const allowed = declaredOptionValues(spec.options);
+        // A manifest with no option table cannot say what is legal. The spec
+        // refuses that shape at parse time, but `registerManifest` takes
+        // manifests as given (no Zod pass), so skip rather than reject every
+        // write to a hand-built manifest — same leniency the unparseable
+        // `visible` and invalid `pattern` branches already take.
+        if (allowed.length > 0) {
+          // `multiselect` stores an array, `select`/`radio` a scalar; both are
+          // checked element-wise against the one table. A scalar arriving at a
+          // multiselect is wrapped rather than rejected — policing the value's
+          // SHAPE is a different constraint (`invalid_type`) with a different
+          // owner, and inventing it here would reject writes this change was
+          // never asked to touch.
+          const picked = Array.isArray(value) ? value : [value];
+          // `findIndex`, not `find`: a `find` returning `undefined` cannot say
+          // whether nothing was rejected or whether the rejected element WAS
+          // `undefined` — and the latter would slip through the check.
+          const at = picked.findIndex((v) => !allowed.includes(String(v)));
+          if (at !== -1) {
+            const offending = picked[at];
+            // An option value is not a secret, but `encrypted` is authorable on
+            // any specifier — so never echo the rejected value for a key whose
+            // contents are held encrypted, in a message that lands in logs.
+            const secret = reg.encryptedKeys.has(key);
+            const got = secret ? '' : ` Received '${String(offending)}'.`;
+            errors.push({
+              field: key,
+              code: 'invalid_option',
+              message: `${label} must be one of: ${allowed.join(', ')}.${got}`,
+              label,
+              // The allowed set as a discrete constraint, so a client composes
+              // its own sentence instead of parsing ours (`FieldError.
+              // constraint`, ADR-0114). Key and comma-joined form are the ones
+              // the spec's own `{ allowed: 'draft, sent' }` example documents
+              // and the record validator already emits for this same code.
+              constraint: { allowed: allowed.join(', ') },
+              ...(secret ? {} : { value: String(offending) }),
+            });
+            continue;
+          }
+        }
+      }
+
       if (!empty && typeof spec.pattern === 'string' && typeof value === 'string') {
         let re: RegExp | undefined;
         try {