Repository navigation
Commit 91e7286
feat(service-settings): warn when a settings value misses the format its type declares (18.x half, 「甲」) (#25001)
Part of #24882. This PR is the 18.x warning half only; refusing these
values is the next major's half, which the `domain:services` seat files
when this lands.
Clause-②: no (warning only: every save and every `OS_*` override
accepted before is still accepted, with the same value stored or served,
the same status and the same errors)
## What changes
ADR-0007's specifier table declares a value format for four types:
`email` / `url` / `phone` take "format validation", and `color` takes a
"hex string". Nothing judged it. `PUT /api/settings/branding` stored a
Logo URL of `not a url` and answered 200, and an `OS_*` override was
served unchecked.
`packages/services/service-settings/src/settings-service.ts` now judges
that format at both doors through one shared helper,
`firstFormatMiss(type, value)`. The helper returns a verdict that
carries the ADR-0114 code the refusal will use (`invalid_email` /
`invalid_url` / `invalid_phone` / `invalid_format`). Both call sites
report a miss and keep the value:
- **Save door (`validatePatch`).** The format branch runs after every
refusal family, so the errors a refused patch carries are exactly what
they were. A miss is collected, never pushed to `errors`. When the patch
as a whole is accepted, one `warn` line is logged per key, naming the
namespace, key, type, the expected format and the value's length.
- **Env door (`effectiveEnvOverride`).** The format is judged last, only
on an override that every refusal family has already let through. A miss
draws one `warn` line, deduplicated per (variable, value) like the
existing rejection report. The override stays in force: it is served
with `source: 'env'` and it still locks the key. `auditEnvOverrides`
also walks format-typed keys, so the line arrives at boot rather than on
a first read.
- **The value is never printed** in either line, for any key. That
covers the secret-bearing case, and it keeps an environment variable's
value out of the log.
The grammars are the platform's existing ones for the same words, not
new dialects:
| type | grammar | why this one |
|---|---|---|
| `email` | local@domain with a dotted domain, no spaces | the address
check of `plugin-email`, the live reader of `mail.from_email`. A value
without a warning is one the transport can send from.
`no-reply@localhost` warns. |
| `url` | `scheme://…`, a root-relative `/path`, `data:`, or `blob:` |
the record validator's `url` field grammar. Not http(s)-only: the reader
of `branding.logo_url` (`RuntimeConfigPlugin.logoUrl`) declares
"absolute or relative URL", and storage hands back
`/api/v1/storage/files/…` for an upload. |
| `phone` | digits, spaces and `+ ( ) - .`, at least 5 characters | the
record validator's `phone` field grammar. No shipped manifest authors a
`phone` specifier (`company.phone` is `text`). |
| `color` | `#rgb`, `#rgba`, `#rrggbb`, `#rrggbbaa` | CSS hex notation.
ADR-0007 says "hex string", the picker emits `#rrggbb`, and the reader
serves it as a browser theme colour. |
Empty values, blank values and non-string values are not judged,
matching how `firstPatternMiss` treats them. Surrounding whitespace is
ignored because both live readers trim before use.
## 「甲」 reading
The standing rule (#6015, comment 6292005854), verbatim: "No conversion
can carry it: 18.x ships only the loud half (a warning, an explain line,
a reported event). The refusal is filed for 19.0."
The seat's derivation for this card is comment 6302777692:
- Refusing a malformed `email` / `url` / `phone` / `color` save narrows
what the settings door accepts today.
- Stored values live in `sys_setting` (runtime state). An `OS_*`
override is a deployment environment value. No ADR-0087 conversion can
rewrite either.
- So 18.x warns.
This PR implements exactly that:
- No `FieldError`, no `400` and no new `409` anywhere.
- The response shape is unchanged.
- Every stored and served value is byte-identical to before.
The pins below assert all of this through the real route. The refusal
half reads `miss.code` from the verdict, so turning the warning into a
`FieldError` changes the two call sites and nothing else.
**Where the warning reaches.** Measured at `7adfaec2a`:
- The `PUT` answer is `{ values }` only.
- `setMany` returns a record of `ResolvedSettingValue` keyed by setting,
which has no advisory member.
- The `settings:changed` event (`SettingsChangeEventSchema`) and the two
audit sinks carry no advisory member either.
So the server log, through the service's existing `logger.warn` (wired
from `ctx.logger`; `console.warn` without one), is the only surface
reachable without a new response member. Whether a log-only warning is
loud enough for an admin saving in Setup is put to the seat as a
decision in the report. The console half (constraint validation on Save)
is objectui's, beside objectstack-ai/objectui#13239.
## Tests
New file
`packages/services/service-settings/src/settings-format-warning.test.ts`,
with 31 cases. The save door is driven through the real route
(`registerSettingsRoutes` over a real `SettingsService`, as
`settings-routes.test.ts` does). Status starts at 0, so `200` is
asserted rather than defaulted. The env door is driven through `get`,
`getNamespace` and the `PUT` lock.
- Per type, on every shipped key with the type: `branding.logo_url`,
`company.website`, `branding.accent_color`,
`company.primary_contact_email` and `mail.from_email`, plus a synthetic
`phone` key.
- A malformed save answers 200, is stored as written, and draws exactly
one warning naming namespace, key and type, without the value.
- A well-formed save draws none.
- Per type at the env door:
- A malformed override is in force (`source: 'env'`, `locked: true`, a
save still answers 409). It draws exactly one warning, at boot, which
re-reads do not repeat.
- A well-formed override draws none.
- A secret-bearing `email` key at both doors: the value and its
fragments never appear in the warning.
- Precedence: a value refused for `maxLength` carries only the
`max_length` FieldError. A batch refused for one key stores nothing and
warns about nothing.
- The other shapes each grammar admits (relative, `data:` and `http:`
logos, 3/4/6/8-digit hex, a padded email) draw no warning. Empty and
blank values draw no warning.
- The synthetic fixture parses under `SettingsManifestSchema`.
Readings on the final head `b3b236b77`, 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, 960 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. These import `service-settings` from source.
## Ablations
Each leg ran on `b3b236b77` with the implementation committed. The
mutation went through `node scripts/ablation-replace.mjs` in WRAP mode:
the anchor hit 1 time and fell to 0, and the blob changed on disk. A
trap-armed wrapper then ran the format pins and restored the file. The
restore is proven with blob == HEAD (`f3caa344f9a9`) and an empty `git
diff HEAD`. The tests import the service from source, so no `dist/` is
involved.
| leg | mutation | result |
|---|---|---|
| A1 | the judge accepts everything (`return null`) | 14 red: every
malformed-value pin at both doors, the secret-bearing ones included
(`expected [] to have a length of 1 but got +0`) |
| A2s | the save door refuses a miss (`errors.push` + `continue`) | 8
red: the 7 save-door malformed pins (`expected 400 to be 200`), and the
precedence pin (`[ 'support_phone', 'docs_url' ]` vs `[ 'docs_url' ]`) |
| A2e | the env door rejects a miss (`return null`) | 7 red: every
env-door malformed pin (`expected null to be …`, `expected [] to have a
length of 1`) |
| A3 | the judge flags every value | 13 red: every well-formed pin at
both doors, and the other-shapes pin |
| A4 | the warning echoes the value | 14 red: every malformed pin at
both doors, the two secret-bearing pins included (`expected … not to
contain …`) |
| A5 | the value is judged untrimmed | 1 red: the other-shapes pin
(padded email) |
## Gates
Gate commands were derived from this worktree with `node
scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` (no paths) on `b3b236b77`: 67 commands. Each
ran locally, with its exit code captured before any pipe. The readings
are in the os-dev report on the card. CI owns the families outside that
list.
## Acceptance notes
- Grammar mirrors. The email grammar is now spelled in four places: the
record validator, the rule validator, `plugin-email` and this file. The
url and phone grammars are spelled in two places each, and the record
validator's and rule validator's phone grammars already differ. No
shared predicate exists in `packages/spec`, and nothing pins the copies
equal. Owner: none.
- Non-string values on string-persisting specifiers. ADR-0007 says
"Persists: string". A number written to `logo_url` is stored as given
and dropped by the branding reader. That is a type constraint, not a
format, so it is left alone here exactly as `pattern` leaves it. It was
not measured at a public door. Owner: none.
- `company.phone` is authored as `type: 'text'` with an "E.164
recommended" description, so no phone format applies to the one
phone-like shipped key. The manifests are outside this card. Owner:
none.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 0a3ce08 commit 91e7286
3 files changed
Lines changed: 571 additions & 3 deletions
File tree
- .changeset
- packages/services/service-settings/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
0 commit comments