diff --git a/.changeset/24882-settings-format-refusal.md b/.changeset/24882-settings-format-refusal.md new file mode 100644 index 00000000000..3692eb97bf7 --- /dev/null +++ b/.changeset/24882-settings-format-refusal.md @@ -0,0 +1,31 @@ +--- +'@objectstack/service-settings': minor +--- + +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) + +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. + +Clause-②: no (narrowing) + + + +**BREAKING** (the settings doors' accept set narrows), shipped as `minor` under the launch-window convention for breaking changes. + +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. `no-reply@localhost` misses it. +- `url`: `scheme://…`, a root-relative `/path`, a `data:` URI or a `blob:` URL. A relative logo path that the branding reader serves is accepted. +- `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. + +What changes: + +- **Saving, FROM:** a save naming a value that misses its key's format answered `200` and stored the value as written. +- **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. +- **Environment override, FROM:** a malformed `OS_*` override was in force: reads served it with source `env`, and it locked the key against saves. +- **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. +- **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. +- **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. diff --git a/.changeset/24882-settings-format-warning.md b/.changeset/24882-settings-format-warning.md deleted file mode 100644 index 038d99db690..00000000000 --- a/.changeset/24882-settings-format-warning.md +++ /dev/null @@ -1,26 +0,0 @@ ---- -'@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-refusal.test.ts b/packages/services/service-settings/src/settings-format-refusal.test.ts new file mode 100644 index 00000000000..07d04abe1ed --- /dev/null +++ b/packages/services/service-settings/src/settings-format-refusal.test.ts @@ -0,0 +1,477 @@ +// 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 REFUSED. + * + * Refusing these values narrows what the settings door accepted, and no + * ADR-0087 conversion can carry that: stored values are runtime state and an + * `OS_*` override is a deployment's environment. Under the standing rule 「甲」 + * as the maintainer amended it, such a narrowing ships now, as a breaking + * minor. These pins hold each part of it: + * + * - SAVE DOOR: a malformed value answers `400 SETTINGS_VALIDATION` with one + * `FieldError` naming the key and the ADR-0114 code the verdict carries + * (`invalid_email` / `invalid_url` / `invalid_phone` / `invalid_format`), and + * nothing is stored. A well-formed value saves. + * - ENV DOOR: a malformed override is rejected the way every other family's + * miss is — not in force, reported once at `error`, never printing the + * value — and the key resolves from the stored value, else the default. A + * well-formed override is in force. + * - STORED ROWS: a value stored before the check existed is still served by + * every read, and it does not block a save that does not touch its key. + * + * 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`, a secret-bearing key and a `visible`-gated key are + * synthetic because no shipped manifest authors them, and each fixture is + * parsed against `SettingsManifestSchema` below so it pins a shape an author + * can write. + */ + +import { describe, expect, it, vi } from 'vitest'; +import type { IHttpServer, IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts'; +import { FieldErrorSchema } from '@objectstack/spec/api'; +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 a stray `warn` 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; + +/** Synthetic: a format-typed key whose `visible` reads another key, so a save of that key TOUCHES it. */ +const hookLabManifest = { + namespace: 'hook_lab', + version: 1, + label: 'Hook Lab', + scope: 'global', + readPermission: 'setup.access', + writePermission: 'setup.write', + specifiers: [ + { type: 'select', key: 'mode', label: 'Mode', required: false, default: 'off', + options: [{ value: 'off', label: 'Off' }, { value: 'hook', label: 'Hook' }] }, + { type: 'url', key: 'callback_url', label: 'Callback URL', required: false, + visible: "${data.mode === 'hook'}" }, + ], +} as unknown as SettingsManifest; + +/** The value a secret-bearing pin must never see in a log line or an answer. */ +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); + svc.registerManifest(hookLabManifest); + const http = new MockHttp(); + registerSettingsRoutes(http, svc, { contextFromRequest: ADMIN }); + return { svc, http, ...spy }; +} + +/** Drive one route; status starts at 0 so `200` is asserted, not defaulted. */ +async function call(http: MockHttp, route: string, namespace: string, body?: Record) { + const req: IHttpRequest = { params: { namespace }, query: {}, body, headers: {}, method: route.split(' ')[0], 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(route)!(req, res); + return state; +} + +/** `PUT /api/settings/:namespace` through the real route. */ +const put = (http: MockHttp, namespace: string, body: Record) => + call(http, 'PUT /api/settings/:namespace', namespace, body); + +/** One malformed and one well-formed value per type, each on a key whose type declares it. */ +const PER_TYPE = [ + { type: 'url', code: 'invalid_url', ns: 'branding', key: 'logo_url', bad: 'not a url', good: 'https://cdn.example.com/logo.svg' }, + { type: 'url', code: 'invalid_url', ns: 'company', key: 'website', bad: 'example dot com', good: 'https://example.com' }, + { type: 'color', code: 'invalid_format', ns: 'branding', key: 'accent_color', bad: 'e11d48', good: '#e11d48' }, + { type: 'email', code: 'invalid_email', ns: 'company', key: 'primary_contact_email', bad: 'ops at example.com', good: 'ops@example.com' }, + { type: 'email', code: 'invalid_email', ns: 'mail', key: 'from_email', bad: 'no-reply@localhost', good: 'no-reply@example.com' }, + { type: 'phone', code: 'invalid_phone', ns: 'format_lab', key: 'support_phone', bad: 'call the desk', good: '+1 415 555 0100' }, +] as const; + +/** A key in the same namespace, with no declared format, that a save can write instead. */ +const OTHER_KEY: Record = { + branding: { key: 'workspace_name', value: 'Acme' }, + company: { key: 'legal_name', value: 'Acme Ltd' }, + mail: { key: 'from_name', value: 'Acme Ops' }, + format_lab: { key: 'docs_url', value: '/docs' }, +}; + +/** The manifest a namespace is registered with in {@link makeStack}. */ +const MANIFEST: Record = { + branding: brandingSettingsManifest, + company: companySettingsManifest, + mail: mailSettingsManifest, + format_lab: formatLabManifest, + hook_lab: hookLabManifest, +}; + +/** + * Store `value` under `key` the way a row stored before this check existed got + * there: save it while the key is declared `text` (no format), then register + * the real manifest over the namespace again — the method the option-table and + * value-domain TOUCH pins in `settings-service.test.ts` use. + */ +async function storeUnjudged(stack: ReturnType, ns: string, patch: Record) { + const real = MANIFEST[ns]; + const FORMAT_TYPES = new Set(['email', 'url', 'phone', 'color']); + stack.svc.registerManifest({ + ...real, + specifiers: (real.specifiers as Array>).map((s) => + Object.keys(patch).includes(String(s.key)) && FORMAT_TYPES.has(String(s.type)) ? { ...s, type: 'text' } : s), + } as unknown as SettingsManifest); + const seeded = await put(stack.http, ns, patch); + expect(seeded.status, JSON.stringify(seeded.body)).toBe(200); + stack.svc.registerManifest(real); +} + +describe('synthetic fixtures are a spec-valid authoring surface', () => { + it('format_lab and hook_lab parse under SettingsManifestSchema', () => { + // A fixture spelling a key the schema rejects would pin behaviour no author + // can reach. + expect(() => SettingsManifestSchema.parse(formatLabManifest)).not.toThrow(); + expect(() => SettingsManifestSchema.parse(hookLabManifest)).not.toThrow(); + }); +}); + +describe('save door — a format miss is refused with one FieldError, and nothing is stored (#24882)', () => { + for (const c of PER_TYPE) { + it(`${c.ns}.${c.key} (${c.type}): a malformed value answers 400 SETTINGS_VALIDATION with one ${c.code}, and is not stored`, async () => { + const { svc, http, warns, errors } = makeStack(); + const r = await put(http, c.ns, { [c.key]: c.bad }); + + expect(r.status).toBe(400); + expect(r.body.success).toBe(false); + expect(r.body.error.code).toBe('SETTINGS_VALIDATION'); + expect(r.body.error.details.namespace).toBe(c.ns); + const fields = r.body.error.details.fields; + expect(fields).toHaveLength(1); + expect(FieldErrorSchema.safeParse(fields[0]).success).toBe(true); + expect(fields[0]).toMatchObject({ field: c.key, code: c.code, constraint: { type: c.type } }); + // The answer names what to send, never what was sent. + expect(fields[0]).not.toHaveProperty('value'); + expect(JSON.stringify(r.body)).not.toContain(c.bad); + + // Nothing stored: the key still resolves from its default. + const after = await svc.get(c.ns, c.key, ADMIN()); + expect(after.source).toBe('default'); + expect(after.value).not.toBe(c.bad); + // A refusal handed to the caller is not a degradation: no log line. + expect(warns).toEqual([]); + expect(errors).toEqual([]); + }); + + it(`${c.ns}.${c.key} (${c.type}): a well-formed value saves`, async () => { + const { svc, http, warns, errors } = makeStack(); + const r = await put(http, c.ns, { [c.key]: c.good }); + expect(r.status).toBe(200); + expect(r.body.data.values[c.key].value).toBe(c.good); + const stored = await svc.get(c.ns, c.key, ADMIN()); + expect(stored.value).toBe(c.good); + expect(stored.source).not.toBe('default'); + expect(warns).toEqual([]); + expect(errors).toEqual([]); + }); + } + + it('the other shapes each grammar admits save too', 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 } = 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); + }); + + it('an empty or blank value is not a format miss', async () => { + const { http } = makeStack(); + expect((await put(http, 'branding', { logo_url: '' })).status).toBe(200); + expect((await put(http, 'branding', { logo_url: ' ' })).status).toBe(200); + }); + + it('a secret-bearing key: the malformed save is refused, and the answer never carries the value', async () => { + const { svc, http, warns, errors } = makeStack(); + const r = await put(http, 'format_lab', { billing_email: SECRET_MALFORMED }); + 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]).toMatchObject({ field: 'billing_email', code: 'invalid_email' }); + const answer = JSON.stringify(r.body); + expect(answer).not.toContain(SECRET_MALFORMED); + // Not a fragment of it either. + expect(answer).not.toContain('billing-desk'); + expect(answer).not.toContain('corp-internal'); + expect((await svc.get('format_lab', 'billing_email', ADMIN())).source).toBe('default'); + expect([...warns, ...errors]).toEqual([]); + }); + + it('precedence: a value that breaks a declared bound is refused for the bound, once', async () => { + const { svc, http } = 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]).toMatchObject({ field: 'docs_url', code: 'max_length' }); + // Each key speaks once, for its own first breach; the batch stores nothing. + 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; code: string }) => [f.field, f.code])).toEqual([ + ['support_phone', 'invalid_phone'], + ['docs_url', 'max_length'], + ]); + expect((await svc.get('format_lab', 'support_phone', ADMIN())).source).toBe('default'); + expect((await svc.get('format_lab', 'docs_url', ADMIN())).source).toBe('default'); + }); + + it('a batch with one malformed key is refused whole: its well-formed neighbour is not stored', async () => { + const { svc, http } = makeStack(); + const r = await put(http, 'branding', { workspace_name: 'Acme', logo_url: 'not a url' }); + expect(r.status).toBe(400); + expect(r.body.error.details.fields.map((f: { field: string }) => f.field)).toEqual(['logo_url']); + expect((await svc.get('branding', 'workspace_name', ADMIN())).source).toBe('default'); + }); +}); + +describe('env door — a format-missing override is rejected: not in force, reported once, never the value (#24882)', () => { + for (const c of PER_TYPE) { + const envName = `OS_${c.ns}_${c.key}`.toUpperCase(); + + it(`${envName} (${c.type}): a malformed override is not in force, falls back to the default, and is reported once`, 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(errors).toHaveLength(1); + + // Not in force: no stored value yet, so the default answers. + const r = await svc.get(c.ns, c.key, ADMIN()); + expect(r.source).toBe('default'); + expect(r.value).not.toBe(c.bad); + expect(r.locked).toBe(false); + expect((r.cascadeChain ?? []).map((e) => e.scope)).not.toContain('env'); + // 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((await call(http, 'GET /api/settings/:namespace', c.ns)).body.data.values[c.key].source).toBe('default'); + + expect(errors).toHaveLength(1); + expect(errors[0]).toContain(envName); + expect(errors[0]).toContain(`'${c.ns}.${c.key}'`); + expect(errors[0]).toContain(`type '${c.type}'`); + expect(errors[0]).toContain('IGNORED'); + // An environment variable's value never reaches the log line. + expect(errors[0]).not.toContain(c.bad); + expect(warns).toEqual([]); + + // Not locking either: the key is configurable through the settings door, + // and a stored value now answers in the override's place. + const w = await put(http, c.ns, { [c.key]: c.good }); + expect(w.status).toBe(200); + const stored = await svc.get(c.ns, c.key, ADMIN()); + expect(stored.value).toBe(c.good); + expect(stored.source).not.toBe('env'); + expect(stored.source).not.toBe('default'); + expect(errors).toHaveLength(1); + }); + + it(`${envName} (${c.type}): a well-formed override is in force and locks the key`, async () => { + const { svc, http, 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(r.locked).toBe(true); + const w = await put(http, c.ns, { [c.key]: c.good }); + expect(w.status).toBe(409); + expect(w.body.error.code).toBe('SETTINGS_LOCKED'); + expect(warns).toEqual([]); + expect(errors).toEqual([]); + }); + } + + it('a malformed override set over a stored value: the stored value answers', async () => { + // `env` is read live (it may be `process.env`), so an override can arrive + // after a value was saved — the deployment that sets a bad variable on a + // workspace that already configured the key. + const env: Record = {}; + const { svc, http, errors } = makeStack(env); + expect((await put(http, 'branding', { logo_url: 'https://cdn.example.com/logo.svg' })).status).toBe(200); + env.OS_BRANDING_LOGO_URL = 'not a url'; + + const r = await svc.get('branding', 'logo_url', ADMIN()); + expect(r.value).toBe('https://cdn.example.com/logo.svg'); + expect(r.source).toBe('tenant'); + expect(r.locked).toBe(false); + expect(errors).toHaveLength(1); + expect(errors[0]).toContain('OS_BRANDING_LOGO_URL'); + expect(errors[0]).not.toContain('not a url'); + }); + + it('a secret-bearing key: the override is rejected, and its line never carries the value', async () => { + const { svc, warns, errors } = makeStack({ OS_FORMAT_LAB_BILLING_EMAIL: SECRET_MALFORMED }); + const r = await svc.get('format_lab', 'billing_email', ADMIN()); + expect(r.source).toBe('default'); + expect(r.locked).toBe(false); + expect(errors).toHaveLength(1); + expect(errors[0]).toContain('OS_FORMAT_LAB_BILLING_EMAIL'); + expect(errors[0]).toContain("'format_lab.billing_email'"); + expect(errors[0]).toContain("type 'email'"); + expect(errors[0]).not.toContain(SECRET_MALFORMED); + expect(errors[0]).not.toContain('billing-desk'); + expect(errors[0]).not.toContain('corp-internal'); + expect(warns).toEqual([]); + }); + + it('precedence: an override that breaks a declared bound is rejected for the bound, once', async () => { + // `docs_url` declares `maxLength: 12`; a 16-character override is ignored + // for its length, the family the save door names first too. + 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).toBe('default'); + expect(errors).toHaveLength(1); + expect(errors[0]).toContain('IGNORED'); + expect(errors[0]).toContain('outside the declared length'); + expect(errors[0]).not.toContain("type 'url'"); + expect(warns).toEqual([]); + }); +}); + +describe('stored rows — a value stored before the check existed (#24882)', () => { + for (const c of PER_TYPE) { + const other = OTHER_KEY[c.ns]; + + it(`${c.ns}.${c.key} (${c.type}): a read still serves it, and a save of ${other.key} is not blocked by it`, async () => { + const stack = makeStack(); + await storeUnjudged(stack, c.ns, { [c.key]: c.bad }); + const { svc, http } = stack; + + // Reads judge nothing: the service and the route both serve it. + const read = await svc.get(c.ns, c.key, ADMIN()); + expect(read.value).toBe(c.bad); + expect(read.source).not.toBe('default'); + const viaRoute = await call(http, 'GET /api/settings/:namespace', c.ns); + expect(viaRoute.status).toBe(200); + expect(viaRoute.body.data.values[c.key].value).toBe(c.bad); + + // A save that does not touch the key is not refused on its account. + const w = await put(http, c.ns, { [other.key]: other.value }); + expect(w.status, JSON.stringify(w.body)).toBe(200); + expect((await svc.get(c.ns, other.key, ADMIN())).value).toBe(other.value); + expect((await svc.get(c.ns, c.key, ADMIN())).value).toBe(c.bad); + + // Only a save of the key itself is judged: the malformed value is + // refused, a well-formed one replaces it. + const again = await put(http, c.ns, { [c.key]: c.bad }); + expect(again.status).toBe(400); + expect(again.body.error.details.fields).toMatchObject([{ field: c.key, code: c.code }]); + expect((await put(http, c.ns, { [c.key]: c.good })).status).toBe(200); + expect((await svc.get(c.ns, c.key, ADMIN())).value).toBe(c.good); + }); + } + + it('a reset clears a stored malformed value: an all-null patch is never blocked', async () => { + const stack = makeStack(); + await storeUnjudged(stack, 'branding', { logo_url: 'not a url' }); + expect((await put(stack.http, 'branding', { logo_url: null })).status).toBe(200); + expect((await stack.svc.get('branding', 'logo_url', ADMIN())).source).toBe('default'); + }); + + it('a save TOUCHES a key through its `visible` predicate, and that judges the stored value', async () => { + // `validatePatch`'s TOUCH gate counts a key whose `visible` reads a patched + // key (switching a mode must validate the fields it reveals). No shipped + // format-typed key declares `visible`; this synthetic one does. + const stack = makeStack(); + await storeUnjudged(stack, 'hook_lab', { mode: 'hook', callback_url: 'not a url' }); + const { http } = stack; + const touched = await put(http, 'hook_lab', { mode: 'hook' }); + expect(touched.status).toBe(400); + expect(touched.body.error.details.fields).toMatchObject([{ field: 'callback_url', code: 'invalid_url' }]); + // Hidden by the patch, the key is not judged (hidden specifiers never are). + expect((await put(http, 'hook_lab', { mode: 'off' })).status).toBe(200); + }); +}); diff --git a/packages/services/service-settings/src/settings-format-warning.test.ts b/packages/services/service-settings/src/settings-format-warning.test.ts deleted file mode 100644 index 2a112a02328..00000000000 --- a/packages/services/service-settings/src/settings-format-warning.test.ts +++ /dev/null @@ -1,312 +0,0 @@ -// 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 f3caa344f9a..ae561682b08 100644 --- a/packages/services/service-settings/src/settings-service.ts +++ b/packages/services/service-settings/src/settings-service.ts @@ -626,10 +626,10 @@ function firstPatternMiss(declared: DeclaredPattern, value: unknown): { value: s * `#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 + * `code` is the ADR-0114 `FieldErrorCode` the save door's 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 call site reads it from * here rather than choosing a second time. */ const DECLARED_VALUE_FORMATS: ReadonlyMap< @@ -685,17 +685,23 @@ interface FormatMiss { * 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 + * ## Both doors REFUSE a miss * - * 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. + * Refusing these values narrows what the settings door accepted, and no + * ADR-0087 conversion can carry that: stored values are runtime state in + * `sys_setting`, and an `OS_*` override is a deployment's environment. Under + * the standing rule 「甲」 as the maintainer amended it, such a narrowing ships + * now, as a breaking minor, rather than waiting on a later major. So a miss is + * refused at both doors: the save door answers one `FieldError` for the key, + * with `code` (`validatePatch`), and stores nothing; the env door rejects the + * override the way it rejects every other family's miss + * (`reportRejectedEnvOverride`), so it is not in force and the key resolves + * from the next layer of the cascade. + * + * Neither door judges a value it is not handed. A row already stored that + * misses its format is still served by every read, and it does not block a + * save that does not TOUCH its key — the gate `validatePatch` applies to every + * family, for the reason it gives there. * * 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 @@ -804,14 +810,6 @@ 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 @@ -1198,11 +1196,10 @@ 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). 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. + // (#5932), a standard value domain (#5712), a pattern (#6580), or a format + // its TYPE declares (#24882) — that last one has no per-key table, so its + // keys are found by type. `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(), @@ -1252,7 +1249,9 @@ export class SettingsService { * still judged on one door only, now arrives through the same shared helper * the save path calls ({@link firstPatternMiss}), in the same family order * the save path applies — options → pattern → valueDomain → bounds — so the - * two doors report the same family for the same value. + * two doors report the same family for the same value. #24882 appends the + * format a key's TYPE declares (`email` / `url` / `phone` / `color`) to the + * end of that order, on both doors, through {@link firstFormatMiss}. */ private effectiveEnvOverride( reg: RegisteredManifest, @@ -1353,56 +1352,26 @@ 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}). + // The format the key's TYPE declares (#24882), judged LAST — after every + // family above, in the position it holds in `validatePatch`, so a value + // that breaks a declared bound and its format is rejected for the bound at + // both doors. Same helper as the save path ({@link firstFormatMiss}). The + // line never prints the value, for any key: it carries the value's length + // instead (`shape`). const formatMiss = firstFormatMiss(declaredTypeOf(reg.manifest, key), value); - if (formatMiss) this.reportEnvFormatMiss(namespace, key, envName, value, formatMiss); + if (formatMiss) { + this.reportRejectedEnvOverride(reg, namespace, key, envName, value, { + what: `does not match the format its type '${formatMiss.type}' declares for`, + detail: `Allowed values: ${formatMiss.expected}.`, + fix: formatMiss.expected, + shape: formatMiss.shape, + }); + return null; + } 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. @@ -1445,8 +1414,12 @@ export class SettingsService { * sentence template needs. Passed in rather than branched on here so a * third constraint family reuses the dedupe, the redaction rule and the * consequence/fix prose instead of growing a second reporter beside them. + * + * `shape`, when given, replaces the value in the line for EVERY key, not + * only an encrypted one: the format family (#24882) reports what the value + * looks like (its length) and never the value itself. */ - rejection: { what: string; detail: string; fix: string }, + rejection: { what: string; detail: string; fix: string; shape?: string }, ): void { const dedupeAt = `${envName}=${String(offending)}`; if (this.reportedEnvOverrides.has(dedupeAt)) return; @@ -1457,7 +1430,9 @@ export class SettingsService { // held encrypted, in a message that lands in logs. Same rule, same reason, // as the save path's `invalid_option` error. const secret = reg.encryptedKeys.has(key); - const rejected = secret ? '' : ` Rejected value: '${String(offending)}'.`; + const rejected = typeof rejection.shape === 'string' + ? ` The rejected value is ${rejection.shape}; it is not printed.` + : secret ? '' : ` Rejected value: '${String(offending)}'.`; const message = `[SettingsService] env override ${envName} ${rejection.what} ` + `setting '${namespace}.${key}' — IGNORED.${rejected} ${rejection.detail} ` + @@ -2457,10 +2432,10 @@ export class SettingsService { * (`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. + * rejected (`invalid_email` / `invalid_url` / `invalid_phone` / + * `invalid_format`, the code the verdict carries — see + * {@link firstFormatMiss}). Judged after every other family, so a value + * that also breaks one of those is refused for that one. * - All-null patches (namespace reset) skip validation rather than block the * write. * - A `visible` predicate `evaluateVisibility` cannot parse → the write is @@ -2479,7 +2454,9 @@ export class SettingsService { * for the same reason and one more: bounds get TIGHTENED over a product's * life (a `password_min_length` floor raised from 6 to 8), and a workspace * sitting below the new floor must still be able to edit its unrelated - * settings — it is told about the key only when it writes that key. + * settings — it is told about the key only when it writes that key. The + * format check (#24882) inherits it for the first reason: a value stored + * before that check existed is refused only by a patch that touches its key. * * At most one `FieldError` per offending key: every branch above `continue`s * once it has spoken, so a client is handed the first constraint the value @@ -2547,9 +2524,6 @@ 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; @@ -2799,40 +2773,32 @@ export class SettingsService { } // 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`). + // so every family above keeps precedence. The code is the one the + // verdict carries (`firstFormatMiss`), the same helper the env door + // calls. The message names what the format expects and never the value, + // for any key, so neither `value` nor a `Received` echo is attached: the + // caller sent the value and needs only to be told what to send instead. + // `constraint.type` names the declaring property, so a client can tell a + // colour's `invalid_format` from a declared pattern's (`constraint. + // pattern`) without parsing the sentence (ADR-0114). if (!empty) { const miss = firstFormatMiss(type, value); - if (miss) formatMisses.push({ key, miss }); + if (miss) { + errors.push({ + field: key, + code: miss.code, + message: `${label} must be ${miss.expected}.`, + label, + constraint: { type: miss.type }, + }); + continue; + } } } 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); - } } /**