diff --git a/.changeset/24882-settings-format-warning.md b/.changeset/24882-settings-format-warning.md new file mode 100644 index 00000000000..038d99db690 --- /dev/null +++ b/.changeset/24882-settings-format-warning.md @@ -0,0 +1,26 @@ +--- +'@objectstack/service-settings': patch +--- + +feat(service-settings): a settings value that misses the format its `email`, `url`, `phone` or `color` type declares is now reported in the server log; the save is still accepted (#24882) + +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) + +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 judged that format. `PUT /api/settings/branding` stored a Logo URL of `not a url` and answered 200, and an `OS_*` override was served unchecked. + +Both doors now judge the format through one shared check, and report a miss instead of refusing it: + +- **Save.** When a save is accepted and a key it touches holds a value that misses its type's format, the service logs one `warn` line for that key. The line names the setting, the type, the expected format and the value's length. +- **Environment override.** An in-force `OS_*` override whose value misses the format draws one `warn` line, at boot or on the first read. The line names the variable, the setting, the type and the expected format. The override stays in force, is served with source `env`, and still locks the key. +- **The value is never printed** in either line, for any key. + +The formats are the platform's existing grammars for the same words: + +- `email`: an address with a dotted domain, the grammar the mail transport sends with. +- `url`: `scheme://…`, a root-relative `/path`, a `data:` URI or a `blob:` URL. An http(s)-only grammar would flag a relative logo path that the branding reader serves. +- `phone`: digits, spaces and `+ ( ) - .`, at least five characters. +- `color`: `#rgb`, `#rgba`, `#rrggbb` or `#rrggbbaa`. + +Empty values and non-string values are not judged. Surrounding whitespace is ignored, because the readers trim it before use. + +**Unchanged.** Status codes, response bodies, stored values, the `FieldError`s a refused save carries, and which overrides are in force. Under the standing rule 「甲」, refusing these values waits for the next major version. diff --git a/packages/services/service-settings/src/settings-format-warning.test.ts b/packages/services/service-settings/src/settings-format-warning.test.ts new file mode 100644 index 00000000000..2a112a02328 --- /dev/null +++ b/packages/services/service-settings/src/settings-format-warning.test.ts @@ -0,0 +1,312 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #24882 — the format each specifier TYPE declares (ADR-0007: `email` / `url` / + * `phone` "format validation", `color` "hex string") is judged at both doors + * and, under the standing rule 「甲」, REPORTED rather than refused. + * + * Refusing these values narrows what the settings door has always accepted, + * and no ADR-0087 conversion can carry that: stored values are runtime state + * and an `OS_*` override is a deployment's environment. So this major version + * ships only the loud half, and these pins hold both of its halves: + * + * - ACCEPTED, unchanged: a malformed save still answers `200` and the value is + * stored and served as written; a malformed override is still in force, + * still served with `source: 'env'` and still locks the key. + * - REPORTED: exactly one `warn` line, naming namespace, key and type — and + * never the value, for any key, so a secret-bearing key's value cannot reach + * a log line through it. + * + * The save door is driven through the real route (`registerSettingsRoutes` + * over a real `SettingsService`, as `settings-routes.test.ts` does); the env + * door through the service's own read and write paths, which is where an + * override takes effect. Shipped manifests are used wherever a shipped key has + * the type; `phone` and a secret-bearing key are synthetic because no shipped + * manifest authors either, and both fixtures are parsed against + * `SettingsManifestSchema` below so they pin a shape an author can write. + */ + +import { describe, expect, it, vi } from 'vitest'; +import type { IHttpServer, IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts'; +import { SettingsManifestSchema, type SettingsManifest } from '@objectstack/spec/system'; +import { SettingsService } from './settings-service.js'; +import { registerSettingsRoutes } from './settings-routes.js'; +import { brandingSettingsManifest } from './manifests/branding.manifest.js'; +import { companySettingsManifest } from './manifests/company.manifest.js'; +import { mailSettingsManifest } from './manifests/mail.manifest.js'; +import { LocalCryptoProvider } from './local-crypto-provider.js'; + +class MockHttp implements IHttpServer { + routes = new Map(); + private add(method: string, path: string, handler: RouteHandler) { + this.routes.set(`${method} ${path}`, handler); + } + get(path: string, h: RouteHandler) { this.add('GET', path, h); return this as any; } + post(path: string, h: RouteHandler) { this.add('POST', path, h); return this as any; } + put(path: string, h: RouteHandler) { this.add('PUT', path, h); return this as any; } + delete(path: string, h: RouteHandler) { this.add('DELETE', path, h); return this as any; } + patch(path: string, h: RouteHandler) { this.add('PATCH', path, h); return this as any; } + use() { return this as any; } + listen() { return Promise.resolve(); } + close() { return Promise.resolve(); } + getInstance() { return null; } +} + +/** A diagnostics sink that records both levels, so an `error` line would be seen too. */ +function spyLogger() { + const warns: string[] = []; + const errors: string[] = []; + return { + warns, + errors, + logger: { + warn: (m: string) => void warns.push(m), + error: (m: string) => void errors.push(m), + }, + }; +} + +/** Synthetic: a `phone` specifier (no shipped manifest authors one) and a secret-bearing `email`. */ +const formatLabManifest = { + namespace: 'format_lab', + version: 1, + label: 'Format Lab', + scope: 'global', + readPermission: 'setup.access', + writePermission: 'setup.write', + specifiers: [ + { type: 'phone', key: 'support_phone', label: 'Support phone', required: false }, + { type: 'email', key: 'billing_email', label: 'Billing email', required: false, encrypted: true }, + { type: 'url', key: 'docs_url', label: 'Docs URL', required: false, maxLength: 12 }, + ], +} as unknown as SettingsManifest; + +/** The value a secret-bearing pin must never see in a log line. */ +const SECRET_MALFORMED = 'billing-desk-at-corp-internal'; + +const ADMIN = () => ({ + enforced: true, + permissions: ['setup.access', 'setup.write', 'manage_platform_settings'], + userId: 'usr_admin', + organizationId: 'org_a', +}); + +function makeStack(env: Record = {}) { + const spy = spyLogger(); + const secretRows = new Map(); + const svc = new SettingsService({ + env, + logger: spy.logger, + // A confidentiality-providing crypto pair, so a write to the secret-bearing + // key is accepted by the fail-closed encryption gate and actually stored. + cryptoProvider: new LocalCryptoProvider(), + secretStore: { + async insert(row) { secretRows.set(row.id, row); return { id: row.id }; }, + async get(id) { return secretRows.get(id) ?? null; }, + async update(id, patch) { secretRows.set(id, { ...secretRows.get(id), ...patch }); }, + }, + }); + svc.registerManifest(brandingSettingsManifest); + svc.registerManifest(companySettingsManifest); + svc.registerManifest(mailSettingsManifest); + svc.registerManifest(formatLabManifest); + const http = new MockHttp(); + registerSettingsRoutes(http, svc, { contextFromRequest: ADMIN }); + return { svc, http, ...spy }; +} + +/** `PUT /api/settings/:namespace` through the real route; status starts at 0 so `200` is asserted, not defaulted. */ +async function put(http: MockHttp, namespace: string, body: Record) { + const req: IHttpRequest = { params: { namespace }, query: {}, body, headers: {}, method: 'PUT', path: '/' }; + const state: { status: number; body?: any } = { status: 0 }; + const res: IHttpResponse = { + json: vi.fn((data) => { state.body = data; }) as any, + send: vi.fn() as any, + status: vi.fn((code: number) => { state.status = code; return res; }) as any, + header: vi.fn(() => res) as any, + }; + await http.routes.get('PUT /api/settings/:namespace')!(req, res); + return state; +} + +/** One malformed and one well-formed value per type, each on a key whose type declares it. */ +const PER_TYPE = [ + { type: 'url', ns: 'branding', key: 'logo_url', bad: 'not a url', good: 'https://cdn.example.com/logo.svg' }, + { type: 'url', ns: 'company', key: 'website', bad: 'example dot com', good: 'https://example.com' }, + { type: 'color', ns: 'branding', key: 'accent_color', bad: 'e11d48', good: '#e11d48' }, + { type: 'email', ns: 'company', key: 'primary_contact_email', bad: 'ops at example.com', good: 'ops@example.com' }, + { type: 'email', ns: 'mail', key: 'from_email', bad: 'no-reply@localhost', good: 'no-reply@example.com' }, + { type: 'phone', ns: 'format_lab', key: 'support_phone', bad: 'call the desk', good: '+1 415 555 0100' }, +] as const; + +describe('synthetic fixture is a spec-valid authoring surface', () => { + it('format_lab parses under SettingsManifestSchema', () => { + // A fixture spelling a key the schema rejects would pin behaviour no author + // can reach. + expect(() => SettingsManifestSchema.parse(formatLabManifest)).not.toThrow(); + }); +}); + +describe('save door — a format miss is accepted and stored, and warned about once (#24882, 「甲」)', () => { + for (const c of PER_TYPE) { + it(`${c.ns}.${c.key} (${c.type}): a malformed value answers 200, is stored as written, and draws exactly one warning`, async () => { + const { svc, http, warns, errors } = makeStack(); + const r = await put(http, c.ns, { [c.key]: c.bad }); + + // ACCEPTED: no refusal anywhere — the status, the envelope and the store. + expect(r.status).toBe(200); + expect(r.body.success).toBe(true); + expect(r.body.error).toBeUndefined(); + expect(r.body.data.values[c.key].value).toBe(c.bad); + const stored = await svc.get(c.ns, c.key, ADMIN()); + expect(stored.value).toBe(c.bad); + expect(stored.source).not.toBe('default'); + + // REPORTED: exactly one line, at warn, naming namespace, key and type. + expect(warns).toHaveLength(1); + expect(warns[0]).toContain(`'${c.ns}.${c.key}'`); + expect(warns[0]).toContain(`(type '${c.type}')`); + expect(warns[0]).toContain('The save is accepted'); + // Never the value itself. + expect(warns[0]).not.toContain(c.bad); + expect(errors).toHaveLength(0); + }); + + it(`${c.ns}.${c.key} (${c.type}): a well-formed value draws no warning`, async () => { + const { svc, http, warns, errors } = makeStack(); + const r = await put(http, c.ns, { [c.key]: c.good }); + expect(r.status).toBe(200); + expect((await svc.get(c.ns, c.key, ADMIN())).value).toBe(c.good); + expect(warns).toHaveLength(0); + expect(errors).toHaveLength(0); + }); + } + + it('the other shapes each grammar admits draw no warning either', async () => { + // The readers' own contracts: `logoUrl` is "absolute or relative", storage + // hands back a root-relative path for an upload, and CSS hex colours come + // in four lengths. + const { http, warns } = makeStack(); + for (const logo of ['/api/v1/storage/files/abc123', 'data:image/svg+xml;base64,PHN2Zy8+', 'http://intranet.local/logo.png']) { + expect((await put(http, 'branding', { logo_url: logo })).status).toBe(200); + } + for (const colour of ['#abc', '#abcd', '#AABBCC', '#aabbccdd']) { + expect((await put(http, 'branding', { accent_color: colour })).status).toBe(200); + } + // Surrounding whitespace is trimmed by both live readers before use, so it + // is not a format miss; inner whitespace still is (the per-type cases above). + expect((await put(http, 'company', { primary_contact_email: ' ops@example.com ' })).status).toBe(200); + expect(warns).toEqual([]); + }); + + it('a secret-bearing key: the malformed save is accepted, and its warning never carries the value', async () => { + const { svc, http, warns } = makeStack(); + const r = await put(http, 'format_lab', { billing_email: SECRET_MALFORMED }); + expect(r.status).toBe(200); + // Stored: the in-process read decrypts the real plaintext back. + expect((await svc.get('format_lab', 'billing_email', ADMIN())).value).toBe(SECRET_MALFORMED); + + expect(warns).toHaveLength(1); + expect(warns[0]).toContain("'format_lab.billing_email'"); + expect(warns[0]).toContain("(type 'email')"); + expect(warns[0]).not.toContain(SECRET_MALFORMED); + // Not a fragment of it either. + expect(warns[0]).not.toContain('billing-desk'); + expect(warns[0]).not.toContain('corp-internal'); + }); + + it('a refused patch carries exactly the errors it carried before, and draws no format warning', async () => { + const { svc, http, warns } = makeStack(); + // `docs_url` declares `maxLength: 12`: the length refusal keeps precedence + // over the format, and the one FieldError for the key is the length one. + const r = await put(http, 'format_lab', { docs_url: 'not a url at all' }); + expect(r.status).toBe(400); + expect(r.body.error.code).toBe('SETTINGS_VALIDATION'); + expect(r.body.error.details.fields).toHaveLength(1); + expect(r.body.error.details.fields[0].field).toBe('docs_url'); + expect(r.body.error.details.fields[0].code).toBe('max_length'); + // A batch refused for one key is refused whole: the malformed neighbour is + // not stored, so it is not reported either. + const r2 = await put(http, 'format_lab', { docs_url: 'not a url at all', support_phone: 'call the desk' }); + expect(r2.status).toBe(400); + expect(r2.body.error.details.fields.map((f: { field: string }) => f.field)).toEqual(['docs_url']); + expect((await svc.get('format_lab', 'support_phone', ADMIN())).source).toBe('default'); + expect(warns).toEqual([]); + }); + + it('an empty or blank value is not a format miss', async () => { + const { http, warns } = makeStack(); + expect((await put(http, 'branding', { logo_url: '' })).status).toBe(200); + expect((await put(http, 'branding', { logo_url: ' ' })).status).toBe(200); + expect(warns).toEqual([]); + }); +}); + +describe('env door — a format-missing override stays in force, and is warned about once (#24882, 「甲」)', () => { + for (const c of PER_TYPE) { + const envName = `OS_${c.ns}_${c.key}`.toUpperCase(); + + it(`${envName} (${c.type}): a malformed override is in force, served and locking, with exactly one warning`, async () => { + const { svc, http, warns, errors } = makeStack({ [envName]: c.bad }); + // Reported at boot — `registerManifest` audits the overrides — not on a + // first read that may never come. + expect(warns).toHaveLength(1); + + const r = await svc.get(c.ns, c.key, ADMIN()); + expect(r.value).toBe(c.bad); + expect(r.source).toBe('env'); + expect(r.locked).toBe(true); + // Still locking: a save of the key is refused 409 exactly as before. + const w = await put(http, c.ns, { [c.key]: c.good }); + expect(w.status).toBe(409); + expect(w.body.error.code).toBe('SETTINGS_LOCKED'); + // Re-reads and the namespace payload do not repeat the line. + await svc.get(c.ns, c.key, ADMIN()); + await svc.getNamespace(c.ns, ADMIN()); + + expect(warns).toHaveLength(1); + expect(warns[0]).toContain(envName); + expect(warns[0]).toContain(`'${c.ns}.${c.key}'`); + expect(warns[0]).toContain(`(type '${c.type}')`); + expect(warns[0]).toContain('It IS in force'); + // An environment variable's value never reaches the log line. + expect(warns[0]).not.toContain(c.bad); + expect(errors).toHaveLength(0); + }); + + it(`${envName} (${c.type}): a well-formed override draws no warning`, async () => { + const { svc, warns, errors } = makeStack({ [envName]: c.good }); + const r = await svc.get(c.ns, c.key, ADMIN()); + expect(r.value).toBe(c.good); + expect(r.source).toBe('env'); + expect(warns).toHaveLength(0); + expect(errors).toHaveLength(0); + }); + } + + it('a secret-bearing key: the override is in force, and its warning never carries the value', async () => { + const { svc, warns } = makeStack({ OS_FORMAT_LAB_BILLING_EMAIL: SECRET_MALFORMED }); + const r = await svc.get('format_lab', 'billing_email', ADMIN()); + expect(r.value).toBe(SECRET_MALFORMED); + expect(r.source).toBe('env'); + expect(warns).toHaveLength(1); + expect(warns[0]).toContain('OS_FORMAT_LAB_BILLING_EMAIL'); + expect(warns[0]).toContain("'format_lab.billing_email'"); + expect(warns[0]).toContain("(type 'email')"); + expect(warns[0]).not.toContain(SECRET_MALFORMED); + expect(warns[0]).not.toContain('billing-desk'); + expect(warns[0]).not.toContain('corp-internal'); + }); + + it('an override REJECTED by a refusal family is reported as rejected, not as a format miss', async () => { + // `docs_url` declares `maxLength: 12`; a 16-character override is ignored + // with the existing `error` line, and the format judgment never runs on a + // value that is not in force. + const { svc, warns, errors } = makeStack({ OS_FORMAT_LAB_DOCS_URL: 'not a url at all' }); + const r = await svc.get('format_lab', 'docs_url', ADMIN()); + expect(r.source).not.toBe('env'); + expect(errors).toHaveLength(1); + expect(errors[0]).toContain('IGNORED'); + expect(warns).toEqual([]); + }); +}); diff --git a/packages/services/service-settings/src/settings-service.ts b/packages/services/service-settings/src/settings-service.ts index c17c8841450..f3caa344f9a 100644 --- a/packages/services/service-settings/src/settings-service.ts +++ b/packages/services/service-settings/src/settings-service.ts @@ -594,6 +594,136 @@ function firstPatternMiss(declared: DeclaredPattern, value: unknown): { value: s return declared.re.test(value) ? null : { value }; } +/** + * The value format each format-bearing specifier TYPE declares — ADR-0007's + * specifier table: "`email` / `url` / `phone` | typed input | string | format + * validation" and "`color` | colour picker | hex string" (#24882). + * + * Keyed on the TYPE, unlike the bounds and the pattern next door, because here + * the type IS the declaration: a specifier authors no separate format property, + * so choosing `type: 'url'` is what declares the format. A `Map`, not an object + * literal, so a type string spelled like an `Object.prototype` member can never + * look one up. + * + * Each grammar is the platform's existing one for the same word, not a new + * dialect invented here: + * + * - `email` — the address grammar of the record validator's `email` field type + * and of `plugin-email`'s `isSendableAddress`, which is the live reader of + * `mail.from_email`. So a value this judge passes is one the mail transport + * will send from. It requires a dotted domain (`no-reply@localhost` is not + * sendable there) and is linear-time (no ReDoS). + * - `url` — the record validator's `url` field grammar: `scheme://…`, a + * root-relative `/path`, a `data:` URI or a `blob:` URL. NOT http(s)-only: + * the reader of `branding.logo_url` (`RuntimeConfigPlugin.logoUrl`) declares + * "absolute or relative URL", and the platform's own storage hands back a + * root-relative `/api/v1/storage/files/…` for an uploaded file, so a + * narrower grammar would flag values the reader serves correctly. + * - `phone` — the record validator's `phone` field grammar. No shipped settings + * manifest authors a `phone` specifier today (`company.phone` is `text`); + * the type is in ADR-0007's row, so it is judged rather than skipped. + * - `color` — CSS hex colour notation (`#rgb`, `#rgba`, `#rrggbb`, + * `#rrggbbaa`): ADR-0007 declares "hex string", the colour picker emits the + * six-digit form, and the reader serves the value as a browser theme colour. + * + * `code` is the ADR-0114 `FieldErrorCode` the refusal of this format carries + * (the catalog's closed set has no colour member, so `color` takes + * `invalid_format`, the member for "a declared pattern/format the value does + * not match"). It travels in the verdict so the refusal half reads it from + * here rather than choosing a second time. + */ +const DECLARED_VALUE_FORMATS: ReadonlyMap< + string, + { + re: RegExp; + code: 'invalid_email' | 'invalid_url' | 'invalid_phone' | 'invalid_format'; + expected: string; + } +> = new Map([ + ['email', { + re: /^[^\s@]+@[^\s@.]+(?:\.[^\s@.]+)+$/, + code: 'invalid_email' as const, + expected: 'a well-formed email address (local@domain.tld, no spaces)', + }], + ['url', { + re: /^(?:[a-z][a-z0-9+.-]*:\/\/\S+|\/\S*|data:\S+|blob:\S+)$/i, + code: 'invalid_url' as const, + expected: 'a well-formed URL (scheme://…, a root-relative /path, a data: URI or a blob: URL)', + }], + ['phone', { + re: /^[+()\-\s\d.]{5,}$/, + code: 'invalid_phone' as const, + expected: 'a well-formed phone number (digits, spaces and + ( ) - . only, at least 5 characters)', + }], + ['color', { + re: /^#(?:[0-9a-f]{3,4}|[0-9a-f]{6}|[0-9a-f]{8})$/i, + code: 'invalid_format' as const, + expected: 'a hex colour (#rgb, #rgba, #rrggbb or #rrggbbaa)', + }], +]); + +/** A value that fails its type's declared format — see {@link firstFormatMiss}. */ +interface FormatMiss { + /** The specifier type whose format was missed (`email` / `url` / `phone` / `color`). */ + type: string; + /** The `FieldErrorCode` (ADR-0114) the refusal of this format carries. */ + code: 'invalid_email' | 'invalid_url' | 'invalid_phone' | 'invalid_format'; + /** What the format expects, in prose that names no part of the value. */ + expected: string; + /** The value's shape — its length — never its content. */ + shape: string; +} + +/** + * The format verdict for a value of this specifier type: a {@link FormatMiss} + * when the type declares a format (ADR-0007) and the value does not satisfy + * it, else `null`. + * + * ONE judgment, shared by both paths that produce an effective value — the + * save path ({@link SettingsService.validatePatch}) and the env path + * ({@link SettingsService.effectiveEnvOverride}) — for the reason the option, + * bounds and pattern judges next door are shared: one comparison living on one + * door only is how #5204 and #6580 happened. + * + * ## The verdict WARNS in this major version; it does not refuse + * + * Under the standing rule 「甲」 a narrowing that no ADR-0087 conversion can + * carry ships its loud half first, and refusing these values narrows what the + * settings door has always accepted. Stored values are runtime state in + * `sys_setting`, and an `OS_*` override is a deployment's environment, so no + * conversion can rewrite either. Both call sites therefore REPORT a miss and + * keep the value: the save is accepted and stored, the override stays in + * force. The refusal is the next major version's, and it reads `code` from + * this verdict, so turning the warning into a `FieldError` is a change at each + * call site and nowhere else. + * + * A non-string value is left alone, as `firstPatternMiss` leaves it: a format + * constrains character shape, and policing the value's type is a different + * constraint (`invalid_type`) with a different owner. An empty or blank string + * is left alone too, because "no value" is the `required` check's business. + */ +function firstFormatMiss(type: string, value: unknown): FormatMiss | null { + const declared = DECLARED_VALUE_FORMATS.get(type); + if (!declared) return null; + if (typeof value !== 'string' || value.trim() === '') return null; + // Judged trimmed: both live readers trim before use (`plugin-email`'s + // address check, `RuntimeConfigPlugin`'s branding read), so surrounding + // whitespace is not a format any reader trips over. Inner whitespace is. + if (declared.re.test(value.trim())) return null; + return { + type, + code: declared.code, + expected: declared.expected, + shape: `a string of ${value.length} character${value.length === 1 ? '' : 's'}`, + }; +} + +/** The declared `type` of one specifier key, or `''` when the manifest declares no such key. */ +function declaredTypeOf(manifest: SettingsManifest, key: string): string { + const spec = (manifest.specifiers ?? []).find((s) => s.key === key); + return spec ? String(spec.type) : ''; +} + interface RegisteredManifest { manifest: SettingsManifest; /** Resolved specifier scopes for fast lookup. */ @@ -674,6 +804,14 @@ export class SettingsService { * the value is part of the key and not just the var name. */ private readonly reportedEnvOverrides = new Set(); + /** + * `OS_*` overrides already reported for missing their type's declared format + * (#24882), keyed `=` for the reason + * {@link reportedEnvOverrides} is: `get()` runs the env judgment on every + * read, and a genuinely new value must still be heard. The key lives only in + * memory; the warning itself never carries the value. + */ + private readonly reportedEnvFormatMisses = new Set(); /** * `.` pairs whose fail-closed encryption refusal (#8026) has * already been reported to the logger. Deduped for the same reason @@ -1060,14 +1198,18 @@ export class SettingsService { if (!reg) return; // Only the keys that declare something enforceable can be rejected, so only // they are worth walking: an option table (#5131/#5204), a value window - // (#5932), a standard value domain (#5712) or a pattern (#6580). - // `effectiveEnvOverride` does the judging (and the reporting); the value it - // returns is of no interest here. + // (#5932), a standard value domain (#5712) or a pattern (#6580). A key whose + // TYPE declares a format (#24882) is walked too: its override is never + // rejected for the format, but a miss is reported here, at boot, beside the + // rest of the startup output. `effectiveEnvOverride` does the judging (and + // the reporting); the value it returns is of no interest here. const enforceable = new Set([ ...reg.optionTables.keys(), ...reg.bounds.keys(), ...reg.valueDomains.keys(), ...reg.patterns.keys(), + ...[...reg.scopes.keys()].filter((key) => + DECLARED_VALUE_FORMATS.has(declaredTypeOf(reg.manifest, key))), ]); for (const key of enforceable) { this.effectiveEnvOverride(reg, namespace, key); @@ -1211,9 +1353,56 @@ export class SettingsService { } } + // The type's declared format (#24882) — judged LAST and never a rejection. + // Every refusal family above has passed, so this override IS in force; + // under 「甲」 a format miss is reported (`reportEnvFormatMiss`) and the + // override keeps winning the cascade and pinning the key, exactly as it did + // before this judgment existed. Same helper as the save path + // ({@link firstFormatMiss}). + const formatMiss = firstFormatMiss(declaredTypeOf(reg.manifest, key), value); + if (formatMiss) this.reportEnvFormatMiss(namespace, key, envName, value, formatMiss); + return { envName, value }; } + /** + * Emit the one warning an in-force `OS_*` override owes its operator when its + * value misses the type's declared format (#24882), at most once per + * (env var, value) for the life of the service — the dedupe + * {@link reportRejectedEnvOverride} applies, for the same reason. + * + * **Level is `warn`, not `error`.** AGENTS.md → "Degradation log levels": the + * override is in force and is served as configured, so nothing the system + * claims has failed to land. That is the line between this report and the + * `error` a REJECTED override gets. + * + * **The value is never printed**, for any key — secret-bearing or not: an + * environment variable's value does not go into a log line. The line carries + * the variable's name, the setting, the type, what the format expects and the + * value's length, which is enough to find and fix it. No structured `meta`, + * for the reason {@link reportRejectedEnvOverride} gives. + */ + private reportEnvFormatMiss( + namespace: string, + key: string, + envName: string, + value: unknown, + miss: FormatMiss, + ): void { + const dedupeAt = `${envName}=${String(value)}`; + if (this.reportedEnvFormatMisses.has(dedupeAt)) return; + this.reportedEnvFormatMisses.add(dedupeAt); + const message = + `[SettingsService] env override ${envName} for setting '${namespace}.${key}' ` + + `(type '${miss.type}') is not ${miss.expected} — the value is ${miss.shape}. ` + + `It IS in force: reads serve it with source 'env' and the key stays locked against saves. ` + + `The declared format is reported, not enforced, in this major version; a later major ` + + `version refuses it. Fix: set ${envName} to ${miss.expected}, or unset it and ` + + `configure '${namespace}.${key}' through the settings UI.`; + if (this.logger?.warn) this.logger.warn(message); + else console.warn(message); + } + /** * Emit the one loud line a rejected `OS_*` override owes its operator, at * most once per (env var, value) for the life of the service. @@ -2266,6 +2455,12 @@ export class SettingsService { * - `step` + non-empty numeric value that misses the declared grid * (`min + k * step`, or `k * step` where no `min` is declared) → rejected * (`invalid_value`, #6199). + * - The format the specifier TYPE declares (`email` / `url` / `phone` / + * `color`, ADR-0007; #24882) + non-empty string value that misses it → + * ACCEPTED, and reported: one `warn` line per key, emitted only when the + * patch as a whole is accepted. Never a `FieldError` in this major version + * (「甲」 — see {@link firstFormatMiss}); judged after every refusal family, + * so it never changes which errors a refused patch carries. * - All-null patches (namespace reset) skip validation rather than block the * write. * - A `visible` predicate `evaluateVisibility` cannot parse → the write is @@ -2352,6 +2547,9 @@ export class SettingsService { // a `key → message` map and could only re-emit the prose — so the code is // stamped at the point the check fails rather than inferred from the text. const errors: FieldError[] = []; + // Format misses (#24882), reported — never refused — once the patch is + // known to be accepted. See the format branch at the bottom of the loop. + const formatMisses: Array<{ key: string; miss: FormatMiss }> = []; for (const spec of (reg.manifest.specifiers ?? []) as Array>) { const key = spec.key as string | undefined; @@ -2596,13 +2794,45 @@ export class SettingsService { constraint: breach.constraint, ...(secret ? {} : { value }), }); + continue; } } + + // The format the specifier TYPE declares (ADR-0007; #24882), judged last + // so every refusal family above keeps precedence and the errors a refused + // patch carries are exactly what they were. Under 「甲」 a miss is + // RECORDED, not refused: the save is accepted and the value is stored as + // written. The refusal half turns this push into an `errors.push` of a + // `FieldError` built from `miss.code` — this branch and the env door's are + // the only two sites (`firstFormatMiss`). + if (!empty) { + const miss = firstFormatMiss(type, value); + if (miss) formatMisses.push({ key, miss }); + } } if (errors.length > 0) { throw new SettingsValidationError(namespace, errors); } + + // The patch is accepted. A key whose value misses its declared format is + // reported once per save, at `warn`: the value lands and is served, so + // nothing the system claims has failed (AGENTS.md → "Degradation log + // levels"). The value itself is never printed — not for a secret-bearing + // key, and not for any other: the line carries the setting, the type, what + // the format expects and the value's length, which is what an operator + // needs to find it. No structured `meta`, for the reason + // `reportRejectedEnvOverride` gives. + for (const { key, miss } of formatMisses) { + const message = + `[SettingsService] Setting '${namespace}.${key}' (type '${miss.type}') holds a value ` + + `that is not ${miss.expected} after this save — the value is ${miss.shape}. ` + + `The save is accepted: the value is stored and served as written. The declared format ` + + `is reported, not enforced, in this major version; a later major version refuses it. ` + + `Fix: save ${miss.expected} for '${namespace}.${key}', or clear it.`; + if (this.logger?.warn) this.logger.warn(message); + else console.warn(message); + } } /**