Skip to content

Commit 3b3830c

Browse files
fix(service-settings)!: a settings value that misses its type's declared format is refused at the save door and the env door (#25022)
Fixes #24882 Clause-②: no (narrowing) This PR lands the card's last half: both settings doors now refuse a value that misses the format its type declares. PR #25001 (`91e728669`) landed the warning half. The console half is objectstack-ai/objectui#13287. ## What changes ADR-0007 declares a format for four specifier types: `email`, `url` and `phone` take "format validation", and `color` takes a hex string. PR #25001 added one shared verdict, `firstFormatMiss(type, value)`, which already carries the ADR-0114 code for each type. This PR changes its two call sites in `packages/services/service-settings/src/settings-service.ts` from warning to refusal. The grammars are unchanged. - **The save door (`validatePatch`).** A miss pushes one `FieldError` for the key, with the verdict's code (`invalid_email`, `invalid_url`, `invalid_phone`, or `invalid_format` for `color`), and `continue`s like every other family. The route answers through the existing `SETTINGS_VALIDATION` envelope: `400`, with the entry in `error.details.fields`. Nothing in the save is stored. The format is still judged last, so a value that also breaks another family (a length bound, a pattern) is refused for that family. - The message names the expected format, for example `Logo URL must be a well-formed URL (scheme://…, a root-relative /path, a data: URI or a blob: URL).` It never echoes the value, so the entry carries no `value` member. - `constraint` is `{ type: 'url' }` and so on. This is the existing `FieldError.constraint` member, keyed by the declaring property as the pattern branch's `{ pattern }` is. It lets a client tell a colour's `invalid_format` from a declared pattern's. - **The env door (`effectiveEnvOverride`).** A miss is rejected through `reportRejectedEnvOverride`, the same reporter every other family uses. The override is not in force, the line is logged once per (variable, value) at `error`, and the key is no longer locked. The reporter gains one optional fragment, `shape`. When it is set, the line carries the value's length and never the value, for any key, not only an encrypted one. `auditEnvOverrides` still walks format-typed keys, so the line arrives at boot. - **Removed:** the warning reporter `reportEnvFormatMiss`, its dedupe set, the save door's `formatMisses` list and its post-accept `warn` loop. No response member is added, and nothing outside `service-settings` changes. No organization predicate or rung resolution is touched. ## 「甲」 reading (amended) The standing rule 「甲」 (#6015, comment 6292005854), as the maintainer amended it (comment 6303865025, quoting 「暂时不考虑 v19,协议的调整原则上也是尽快,不考虑退役版本。」): > Rule 2 changes: a narrowing that no conversion carries ALSO ships now, in 18.x, as a BREAKING minor (the #24813 changeset shape); no refusal is filed for 19.0, and no `pm:on-hold` card waits on a 19.0 line. The warning half alone is no longer the 18.x deliverable. The seat's derivation for this card (comment 6302777692) still holds on its facts: - Refusing these values narrows what the settings door accepts. - No ADR-0087 conversion can carry it. Stored values are runtime state in `sys_setting`, and an `OS_*` override is a deployment's environment. Under the amended rule 2, that narrowing ships now. The seat re-timed the card on that reading (comment 6304784014). So this PR is a BREAKING `minor`: - the changeset declares `Clause-②: no (narrowing)`; - its ADR-0087 disposition is `not-required (no-migration-prescription)`, accepted by `check-adr-0087-registration --base origin/main`; - the narrowing owes a contract review at `CONTRACT_REVIEW_TIER` before the queue (claim comment 6304887064). ## Measured first **Stored rows.** A value already stored that misses its format was measured on every shipped format-typed key and on the synthetic `phone` key. Each was stored as `text`, then the real manifest was registered over it, the method the option-table and value-domain TOUCH pins use. - **Reads serve it.** `get()` and `GET /api/settings/:namespace` return it unchanged. Reads judge nothing. This was expected. - **A save of another key in the same namespace is not blocked.** For example, `branding.workspace_name` saves over a stored malformed `logo_url`, and `mail.from_name` saves over a stored `no-reply@localhost`. This was expected: the save door judges only the keys a save touches. - Only a save that writes the key judges it. The malformed value is refused, a well-formed one replaces it, and an all-null reset clears it. - **One precision on "touches".** `validatePatch`'s TOUCH gate also counts a key whose `visible` predicate reads a patched key. A synthetic `url` key gated on `mode` is judged when `mode` is saved and it stays visible. That rule is the same for every family. No shipped format-typed key declares `visible`, so no shipped namespace is affected. It is pinned so the reading is exact. **The env door's fallback.** A rejected override contributes nothing, so the key resolves from the next layer: the stored value when one exists, else the manifest default. None of the five shipped format-typed keys declares a default, so the default is `null`. Reads report that layer as the source, and the key is no longer locked. Both legs are pinned. The changeset states this for operators. ## Pending release note corrected This PR deletes `.changeset/24882-settings-format-warning.md`, PR #25001's changeset, and replaces it with `.changeset/24882-settings-format-refusal.md`. - **What it said.** The pending note says a malformed save "is still accepted", that an override "stays in force", that a `warn` line is logged, and that refusal "waits for the next major version". - **Why it is now false.** This PR changes every one of those statements. No release has consumed the note: `@objectstack/service-settings` is at `18.0.0`, and its `CHANGELOG.md` has no entry for it. - **Why it is replaced, not left beside the new one.** Keeping it would publish two contradictory entries in one release. This is the DELIBERATE CORRECTION class that `check-empty-changeset.mjs` names. Its foreign-changeset refusal is red by design here, and `Check Changeset` will show red. The remedy that gate prescribes is this paragraph: the note and what changed under it, put on the PR for confirmation. The note is not restored, and no `skip-changeset` is applied. ## Tests The pin file is renamed: `settings-format-warning.test.ts` becomes `settings-format-refusal.test.ts`, with 41 cases. - The save door is driven through the real route (`registerSettingsRoutes` over a real `SettingsService`). Status starts at 0, so every status is asserted, not defaulted. - The env door is driven through `get`, `getNamespace`, the `GET` route and the `PUT` lock. What the pins hold: - **Save door, per type.** Every shipped key with a format type (`branding.logo_url`, `company.website`, `branding.accent_color`, `company.primary_contact_email`, `mail.from_email`) and a synthetic `phone` key are covered. - A malformed value answers `400 SETTINGS_VALIDATION` with exactly one field error: `field`, ADR-0114 `code`, `constraint.type`, no `value`, and it parses under `FieldErrorSchema`. Nothing is stored, and no log line is written. - A well-formed value saves. - **Save door, other cases.** - The other shapes each grammar admits still save, and empty and blank values are not judged. - A secret-bearing key's answer carries neither the value nor any fragment of it. - Precedence: a value that breaks a length bound is refused for the bound. A batch with one malformed key is refused whole. - **Env door, per type.** - A malformed override is not in force: source `default`, `locked: false`, no `env` entry in the cascade chain. - It is reported exactly once, at boot, and re-reads do not repeat it. The line never carries the value. - A save of the key then succeeds and is served from the stored value. - A well-formed override is in force and locks the key with `409`. - **Env door, other cases.** A malformed override set over a stored value leaves the stored value answering. A secret-bearing key's line carries no fragment of the value. An override that breaks a length bound is rejected for the bound. - **Stored rows.** The measurements above, per type, plus the reset and the `visible`-TOUCH pin. Readings on the final head `58d0be84f`, each through `scripts/pm/os-verify-lock.sh` (`VERDICT command-exit 0`): - `pnpm --filter @objectstack/service-settings run typecheck`: exit 0. The program includes the new test (`tsc --listFiles`, 57 test files). - `pnpm --filter @objectstack/service-settings exec vitest run --maxWorkers=2`: 57 files, 970 tests passed. - `pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2` over `auth-settings-ordering.pin`, `auth-settings-seeded-boot.pin` and `auth-settings-refusal-isolation`: 3 files, 13 tests passed. Their config aliases `service-settings` to source. - `pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=2` over `test/settings-audit-global-rung-sibling`, `test/settings-config-change-audit` and `test/settings-organization-isolation`: 3 files, 14 tests passed. The rebuilt `service-settings` dist carries the refusal. No dogfood fixture saves a format-typed key. ## Ablations Each leg ran with the implementation committed, on the file blob `ae561682b08a`. The mutation went through `node scripts/ablation-replace.mjs` in WRAP mode, which proves the anchor fell 1 to 0 and the blob changed on disk. The restore after every leg is proven: blob == HEAD (`ae561682b08a`) and an empty `git diff HEAD`. The tests import the service from source, so no `dist/` is involved. | leg | mutation | result | |---|---|---| | A1 | the verdict accepts everything | 24 of 41 red: every refusal pin at both doors, both secret pins, the batch and precedence pins, the re-save step of every stored-row pin and the `visible`-TOUCH pin (`expected 200 to be 400`, `expected 'env' to be 'default'`). The 17 green are the well-formed, other-shape, empty, reset, fixture and env-precedence pins. | | A2 | the env line prints the value (the `shape` branch disabled) | 7 red: every non-secret env malformed pin and the stored-value fallback pin (`expected '… env override …' not to contain 'not a url'`). The secret pin stays green because the encrypted-key redaction still applies, as expected. | | A3 | the save door's field error carries `value` | 7 red: the six save malformed pins (`to not have property "value"`) and the secret save pin (`not to contain 'billing-desk-at-corp-internal'`). | | A4 | the TOUCH gate removed | 9 red: the six stored-row "not blocked" pins (`expected 400 to be 200`) and three `mail` pins that then trip unrelated required keys. This shows the stored-row pins can fail. | A3's first attempt was a no-op that the tool refused: the replacement contained the anchor, so the anchor count did not fall. It was re-run with a non-overlapping replacement. The table records only the leg that landed. ## Gates Gate commands were derived from this worktree with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (no paths) on `58d0be84f`: 67 commands, the same set as the PM's reading at `93e6c1a5f`. Each ran locally with its exit code captured before any pipe. - `--ran` reconciliation: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN. - 65 exited 0. - `node scripts/check-empty-changeset.mjs --base origin/main` exited 1. This is the designed red for the deliberate correction above. - `pnpm check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET, no dist) on the battery. It was re-run with exit 0 after a full turbo build (112 entries across 69 packages load). - `check:dts-closure` and `check:sourcemap-no-sources-content` re-ran with exit 0 over the full build (76 and 72 packages). - The roster gates whose roster sits under a path of this diff ran with exit 0: `check-changeset-fixed.mjs`, `check:authz-resolver`, `check:error-code-casing` and `check:filter-alias-parity`. CI owns the families outside that list. ## Acceptance notes - `content/docs/deployment/environment-variables.mdx` says the settings layer's `OS_BRANDING_LOGO_URL` / `OS_BRANDING_ACCENT_COLOR` outrank a saved value and lock the field. That stays true for a well-formed value. The page does not say a malformed one is now ignored. Docs are outside this card's file surface. Owner: none. - The host-level `OS_LOGO_URL` / `OS_BRAND_COLOR` that `RuntimeConfigPlugin` reads are a different reader and are not judged by the settings door. This is unchanged here. Owner: none. - `origin/main` moved one commit after the merge (`aaee0840b`, `plugin-security` only). It was not re-merged, so the measured head stays `58d0be84f`. --- _Generated by [Claude Code](https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 601b258 commit 3b3830c

5 files changed

Lines changed: 583 additions & 447 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
'@objectstack/service-settings': minor
3+
---
4+
5+
fix(service-settings)!: a settings value that misses the format its `email`, `url`, `phone` or `color` type declares is refused, both when it is saved and when it arrives as an `OS_*` override (#24882)
6+
7+
ADR-0007 declares a format for four specifier types: `email`, `url` and `phone` take "format validation", and `color` takes a hex string. Until now nothing enforced it. `PUT /api/settings/branding` stored a Logo URL of `not a url` and answered `200`, and an `OS_*` override was served whatever it held. Both doors now judge the format through one shared check, after every other declared constraint, and refuse a value that misses it.
8+
9+
Clause-②: no (narrowing)
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, object definition or stored shape is added, removed, renamed or re-shaped, and no exported declaration is removed or narrowed, so there is nothing for `objectstack migrate meta` to rewrite. What changes is which runtime values two settings doors accept: a value saved through `PUT /api/settings/:namespace` or `setMany`, and a deployment's `OS_*` environment override. Settings values are runtime state in `sys_setting` (ADR-0007) and environment values belong to the deployment, and no metadata conversion reaches either; a row already stored is not rewritten and is still served. The remedy is to supply a well-formed value, never a rewrite of a metadata source. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
12+
13+
**BREAKING** (the settings doors' accept set narrows), shipped as `minor` under the launch-window convention for breaking changes.
14+
15+
The formats are the platform's existing grammars for the same words:
16+
17+
- `email`: an address with a dotted domain, the grammar the mail transport sends with. `no-reply@localhost` misses it.
18+
- `url`: `scheme://…`, a root-relative `/path`, a `data:` URI or a `blob:` URL. A relative logo path that the branding reader serves is accepted.
19+
- `phone`: digits, spaces and `+ ( ) - .`, at least five characters.
20+
- `color`: `#rgb`, `#rgba`, `#rrggbb` or `#rrggbbaa`.
21+
22+
Empty values and non-string values are not judged. Surrounding whitespace is ignored, because the readers trim it before use.
23+
24+
What changes:
25+
26+
- **Saving, FROM:** a save naming a value that misses its key's format answered `200` and stored the value as written.
27+
- **Saving, TO:** the save answers `400` with `error.code: 'SETTINGS_VALIDATION'` and one entry per such key in `error.details.fields`, carrying `code` `invalid_email`, `invalid_url`, `invalid_phone` or `invalid_format` (for `color`), and `constraint.type`. Nothing in the save is stored. The answer names the expected format and never echoes the value.
28+
- **Environment override, FROM:** a malformed `OS_*` override was in force: reads served it with source `env`, and it locked the key against saves.
29+
- **Environment override, TO:** a malformed `OS_*` override is no longer in force. The server logs one `error` line per variable and value, naming the variable, the setting, the type and the expected format, never the value. Reads serve the next layer instead: the stored value when one exists, otherwise the manifest default, with that layer as the source. The key is no longer locked, so it can be saved through Setup.
30+
- **Unchanged:** a well-formed value saves, and a well-formed override is in force and locks the key. A value that breaks another declared constraint (a length bound, a pattern) is still refused for that constraint. A value stored before this release is still served by every read, and it does not block a save of the namespace's other keys; only a save that writes that key, or a key its `visible` condition reads, judges it. An all-null save (reset) still clears it.
31+
- **What to do:** fix the value. For an `OS_*` override, set the variable to a well-formed value or unset it and configure the setting in Setup. For a stored value, save a well-formed value for the key, or clear it.

‎.changeset/24882-settings-format-warning.md‎

Lines changed: 0 additions & 26 deletions
This file was deleted.

0 commit comments

Comments
 (0)