Skip to content

Commit ec09558

Browse files
committed
feat(service-settings): warn when a value misses its type's declared format
ADR-0007 declares a format for the email, url, phone and color specifier types. Both doors now judge it through one shared helper (firstFormatMiss) and, under the standing rule 甲, report a miss instead of refusing it: a save is still accepted and stored, an OS_* override is still in force, and one warn line names namespace, key and type without ever printing the value. Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7adfaec commit ec09558

2 files changed

Lines changed: 539 additions & 3 deletions

File tree

Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,309 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #24882 — the format each specifier TYPE declares (ADR-0007: `email` / `url` /
5+
* `phone` "format validation", `color` "hex string") is judged at both doors
6+
* and, under the standing rule 「甲」, REPORTED rather than refused.
7+
*
8+
* Refusing these values narrows what the settings door has always accepted,
9+
* and no ADR-0087 conversion can carry that: stored values are runtime state
10+
* and an `OS_*` override is a deployment's environment. So this major version
11+
* ships only the loud half, and these pins hold both of its halves:
12+
*
13+
* - ACCEPTED, unchanged: a malformed save still answers `200` and the value is
14+
* stored and served as written; a malformed override is still in force,
15+
* still served with `source: 'env'` and still locks the key.
16+
* - REPORTED: exactly one `warn` line, naming namespace, key and type — and
17+
* never the value, for any key, so a secret-bearing key's value cannot reach
18+
* a log line through it.
19+
*
20+
* The save door is driven through the real route (`registerSettingsRoutes`
21+
* over a real `SettingsService`, as `settings-routes.test.ts` does); the env
22+
* door through the service's own read and write paths, which is where an
23+
* override takes effect. Shipped manifests are used wherever a shipped key has
24+
* the type; `phone` and a secret-bearing key are synthetic because no shipped
25+
* manifest authors either, and both fixtures are parsed against
26+
* `SettingsManifestSchema` below so they pin a shape an author can write.
27+
*/
28+
29+
import { describe, expect, it, vi } from 'vitest';
30+
import type { IHttpServer, IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
31+
import { SettingsManifestSchema, type SettingsManifest } from '@objectstack/spec/system';
32+
import { SettingsService } from './settings-service.js';
33+
import { registerSettingsRoutes } from './settings-routes.js';
34+
import { brandingSettingsManifest } from './manifests/branding.manifest.js';
35+
import { companySettingsManifest } from './manifests/company.manifest.js';
36+
import { mailSettingsManifest } from './manifests/mail.manifest.js';
37+
import { LocalCryptoProvider } from './local-crypto-provider.js';
38+
39+
class MockHttp implements IHttpServer {
40+
routes = new Map<string, RouteHandler>();
41+
private add(method: string, path: string, handler: RouteHandler) {
42+
this.routes.set(`${method} ${path}`, handler);
43+
}
44+
get(path: string, h: RouteHandler) { this.add('GET', path, h); return this as any; }
45+
post(path: string, h: RouteHandler) { this.add('POST', path, h); return this as any; }
46+
put(path: string, h: RouteHandler) { this.add('PUT', path, h); return this as any; }
47+
delete(path: string, h: RouteHandler) { this.add('DELETE', path, h); return this as any; }
48+
patch(path: string, h: RouteHandler) { this.add('PATCH', path, h); return this as any; }
49+
use() { return this as any; }
50+
listen() { return Promise.resolve(); }
51+
close() { return Promise.resolve(); }
52+
getInstance() { return null; }
53+
}
54+
55+
/** A diagnostics sink that records both levels, so an `error` line would be seen too. */
56+
function spyLogger() {
57+
const warns: string[] = [];
58+
const errors: string[] = [];
59+
return {
60+
warns,
61+
errors,
62+
logger: {
63+
warn: (m: string) => void warns.push(m),
64+
error: (m: string) => void errors.push(m),
65+
},
66+
};
67+
}
68+
69+
/** Synthetic: a `phone` specifier (no shipped manifest authors one) and a secret-bearing `email`. */
70+
const formatLabManifest = {
71+
namespace: 'format_lab',
72+
version: 1,
73+
label: 'Format Lab',
74+
scope: 'global',
75+
readPermission: 'setup.access',
76+
writePermission: 'setup.write',
77+
specifiers: [
78+
{ type: 'phone', key: 'support_phone', label: 'Support phone', required: false },
79+
{ type: 'email', key: 'billing_email', label: 'Billing email', required: false, encrypted: true },
80+
{ type: 'url', key: 'docs_url', label: 'Docs URL', required: false, maxLength: 12 },
81+
],
82+
} as unknown as SettingsManifest;
83+
84+
/** The value a secret-bearing pin must never see in a log line. */
85+
const SECRET_MALFORMED = 'billing-desk-at-corp-internal';
86+
87+
const ADMIN = () => ({
88+
enforced: true,
89+
permissions: ['setup.access', 'setup.write', 'manage_platform_settings'],
90+
userId: 'usr_admin',
91+
organizationId: 'org_a',
92+
});
93+
94+
function makeStack(env: Record<string, string> = {}) {
95+
const spy = spyLogger();
96+
const secretRows = new Map<string, any>();
97+
const svc = new SettingsService({
98+
env,
99+
logger: spy.logger,
100+
// A confidentiality-providing crypto pair, so a write to the secret-bearing
101+
// key is accepted by the fail-closed encryption gate and actually stored.
102+
cryptoProvider: new LocalCryptoProvider(),
103+
secretStore: {
104+
async insert(row) { secretRows.set(row.id, row); return { id: row.id }; },
105+
async get(id) { return secretRows.get(id) ?? null; },
106+
async update(id, patch) { secretRows.set(id, { ...secretRows.get(id), ...patch }); },
107+
},
108+
});
109+
svc.registerManifest(brandingSettingsManifest);
110+
svc.registerManifest(companySettingsManifest);
111+
svc.registerManifest(mailSettingsManifest);
112+
svc.registerManifest(formatLabManifest);
113+
const http = new MockHttp();
114+
registerSettingsRoutes(http, svc, { contextFromRequest: ADMIN });
115+
return { svc, http, ...spy };
116+
}
117+
118+
/** `PUT /api/settings/:namespace` through the real route; status starts at 0 so `200` is asserted, not defaulted. */
119+
async function put(http: MockHttp, namespace: string, body: Record<string, unknown>) {
120+
const req: IHttpRequest = { params: { namespace }, query: {}, body, headers: {}, method: 'PUT', path: '/' };
121+
const state: { status: number; body?: any } = { status: 0 };
122+
const res: IHttpResponse = {
123+
json: vi.fn((data) => { state.body = data; }) as any,
124+
send: vi.fn() as any,
125+
status: vi.fn((code: number) => { state.status = code; return res; }) as any,
126+
header: vi.fn(() => res) as any,
127+
};
128+
await http.routes.get('PUT /api/settings/:namespace')!(req, res);
129+
return state;
130+
}
131+
132+
/** One malformed and one well-formed value per type, each on a key whose type declares it. */
133+
const PER_TYPE = [
134+
{ type: 'url', ns: 'branding', key: 'logo_url', bad: 'not a url', good: 'https://cdn.example.com/logo.svg' },
135+
{ type: 'url', ns: 'company', key: 'website', bad: 'example dot com', good: 'https://example.com' },
136+
{ type: 'color', ns: 'branding', key: 'accent_color', bad: 'e11d48', good: '#e11d48' },
137+
{ type: 'email', ns: 'company', key: 'primary_contact_email', bad: 'ops at example.com', good: 'ops@example.com' },
138+
{ type: 'email', ns: 'mail', key: 'from_email', bad: 'no-reply@localhost', good: 'no-reply@example.com' },
139+
{ type: 'phone', ns: 'format_lab', key: 'support_phone', bad: 'call the desk', good: '+1 415 555 0100' },
140+
] as const;
141+
142+
describe('synthetic fixture is a spec-valid authoring surface', () => {
143+
it('format_lab parses under SettingsManifestSchema', () => {
144+
// A fixture spelling a key the schema rejects would pin behaviour no author
145+
// can reach.
146+
expect(() => SettingsManifestSchema.parse(formatLabManifest)).not.toThrow();
147+
});
148+
});
149+
150+
describe('save door — a format miss is accepted and stored, and warned about once (#24882, 「甲」)', () => {
151+
for (const c of PER_TYPE) {
152+
it(`${c.ns}.${c.key} (${c.type}): a malformed value answers 200, is stored as written, and draws exactly one warning`, async () => {
153+
const { svc, http, warns, errors } = makeStack();
154+
const r = await put(http, c.ns, { [c.key]: c.bad });
155+
156+
// ACCEPTED: no refusal anywhere — the status, the envelope and the store.
157+
expect(r.status).toBe(200);
158+
expect(r.body.success).toBe(true);
159+
expect(r.body.error).toBeUndefined();
160+
expect(r.body.data.values[c.key].value).toBe(c.bad);
161+
const stored = await svc.get(c.ns, c.key, ADMIN());
162+
expect(stored.value).toBe(c.bad);
163+
expect(stored.source).not.toBe('default');
164+
165+
// REPORTED: exactly one line, at warn, naming namespace, key and type.
166+
expect(warns).toHaveLength(1);
167+
expect(warns[0]).toContain(`'${c.ns}.${c.key}'`);
168+
expect(warns[0]).toContain(`(type '${c.type}')`);
169+
expect(warns[0]).toContain('The save is accepted');
170+
// Never the value itself.
171+
expect(warns[0]).not.toContain(c.bad);
172+
expect(errors).toHaveLength(0);
173+
});
174+
175+
it(`${c.ns}.${c.key} (${c.type}): a well-formed value draws no warning`, async () => {
176+
const { svc, http, warns, errors } = makeStack();
177+
const r = await put(http, c.ns, { [c.key]: c.good });
178+
expect(r.status).toBe(200);
179+
expect((await svc.get(c.ns, c.key, ADMIN())).value).toBe(c.good);
180+
expect(warns).toHaveLength(0);
181+
expect(errors).toHaveLength(0);
182+
});
183+
}
184+
185+
it('the other shapes each grammar admits draw no warning either', async () => {
186+
// The readers' own contracts: `logoUrl` is "absolute or relative", storage
187+
// hands back a root-relative path for an upload, and CSS hex colours come
188+
// in four lengths.
189+
const { http, warns } = makeStack();
190+
for (const logo of ['/api/v1/storage/files/abc123', 'data:image/svg+xml;base64,PHN2Zy8+', 'http://intranet.local/logo.png']) {
191+
expect((await put(http, 'branding', { logo_url: logo })).status).toBe(200);
192+
}
193+
for (const colour of ['#abc', '#abcd', '#AABBCC', '#aabbccdd']) {
194+
expect((await put(http, 'branding', { accent_color: colour })).status).toBe(200);
195+
}
196+
expect(warns).toEqual([]);
197+
});
198+
199+
it('a secret-bearing key: the malformed save is accepted, and its warning never carries the value', async () => {
200+
const { svc, http, warns } = makeStack();
201+
const r = await put(http, 'format_lab', { billing_email: SECRET_MALFORMED });
202+
expect(r.status).toBe(200);
203+
// Stored: the in-process read decrypts the real plaintext back.
204+
expect((await svc.get('format_lab', 'billing_email', ADMIN())).value).toBe(SECRET_MALFORMED);
205+
206+
expect(warns).toHaveLength(1);
207+
expect(warns[0]).toContain("'format_lab.billing_email'");
208+
expect(warns[0]).toContain("(type 'email')");
209+
expect(warns[0]).not.toContain(SECRET_MALFORMED);
210+
// Not a fragment of it either.
211+
expect(warns[0]).not.toContain('billing-desk');
212+
expect(warns[0]).not.toContain('corp-internal');
213+
});
214+
215+
it('a refused patch carries exactly the errors it carried before, and draws no format warning', async () => {
216+
const { svc, http, warns } = makeStack();
217+
// `docs_url` declares `maxLength: 12`: the length refusal keeps precedence
218+
// over the format, and the one FieldError for the key is the length one.
219+
const r = await put(http, 'format_lab', { docs_url: 'not a url at all' });
220+
expect(r.status).toBe(400);
221+
expect(r.body.error.code).toBe('SETTINGS_VALIDATION');
222+
expect(r.body.error.details.fields).toHaveLength(1);
223+
expect(r.body.error.details.fields[0].field).toBe('docs_url');
224+
expect(r.body.error.details.fields[0].code).toBe('max_length');
225+
// A batch refused for one key is refused whole: the malformed neighbour is
226+
// not stored, so it is not reported either.
227+
const r2 = await put(http, 'format_lab', { docs_url: 'not a url at all', support_phone: 'call the desk' });
228+
expect(r2.status).toBe(400);
229+
expect(r2.body.error.details.fields.map((f: { field: string }) => f.field)).toEqual(['docs_url']);
230+
expect((await svc.get('format_lab', 'support_phone', ADMIN())).source).toBe('default');
231+
expect(warns).toEqual([]);
232+
});
233+
234+
it('an empty or blank value is not a format miss', async () => {
235+
const { http, warns } = makeStack();
236+
expect((await put(http, 'branding', { logo_url: '' })).status).toBe(200);
237+
expect((await put(http, 'branding', { logo_url: ' ' })).status).toBe(200);
238+
expect(warns).toEqual([]);
239+
});
240+
});
241+
242+
describe('env door — a format-missing override stays in force, and is warned about once (#24882, 「甲」)', () => {
243+
for (const c of PER_TYPE) {
244+
const envName = `OS_${c.ns}_${c.key}`.toUpperCase();
245+
246+
it(`${envName} (${c.type}): a malformed override is in force, served and locking, with exactly one warning`, async () => {
247+
const { svc, http, warns, errors } = makeStack({ [envName]: c.bad });
248+
// Reported at boot — `registerManifest` audits the overrides — not on a
249+
// first read that may never come.
250+
expect(warns).toHaveLength(1);
251+
252+
const r = await svc.get(c.ns, c.key, ADMIN());
253+
expect(r.value).toBe(c.bad);
254+
expect(r.source).toBe('env');
255+
expect(r.locked).toBe(true);
256+
// Still locking: a save of the key is refused 409 exactly as before.
257+
const w = await put(http, c.ns, { [c.key]: c.good });
258+
expect(w.status).toBe(409);
259+
expect(w.body.error.code).toBe('SETTINGS_LOCKED');
260+
// Re-reads and the namespace payload do not repeat the line.
261+
await svc.get(c.ns, c.key, ADMIN());
262+
await svc.getNamespace(c.ns, ADMIN());
263+
264+
expect(warns).toHaveLength(1);
265+
expect(warns[0]).toContain(envName);
266+
expect(warns[0]).toContain(`'${c.ns}.${c.key}'`);
267+
expect(warns[0]).toContain(`(type '${c.type}')`);
268+
expect(warns[0]).toContain('It IS in force');
269+
// An environment variable's value never reaches the log line.
270+
expect(warns[0]).not.toContain(c.bad);
271+
expect(errors).toHaveLength(0);
272+
});
273+
274+
it(`${envName} (${c.type}): a well-formed override draws no warning`, async () => {
275+
const { svc, warns, errors } = makeStack({ [envName]: c.good });
276+
const r = await svc.get(c.ns, c.key, ADMIN());
277+
expect(r.value).toBe(c.good);
278+
expect(r.source).toBe('env');
279+
expect(warns).toHaveLength(0);
280+
expect(errors).toHaveLength(0);
281+
});
282+
}
283+
284+
it('a secret-bearing key: the override is in force, and its warning never carries the value', async () => {
285+
const { svc, warns } = makeStack({ OS_FORMAT_LAB_BILLING_EMAIL: SECRET_MALFORMED });
286+
const r = await svc.get('format_lab', 'billing_email', ADMIN());
287+
expect(r.value).toBe(SECRET_MALFORMED);
288+
expect(r.source).toBe('env');
289+
expect(warns).toHaveLength(1);
290+
expect(warns[0]).toContain('OS_FORMAT_LAB_BILLING_EMAIL');
291+
expect(warns[0]).toContain("'format_lab.billing_email'");
292+
expect(warns[0]).toContain("(type 'email')");
293+
expect(warns[0]).not.toContain(SECRET_MALFORMED);
294+
expect(warns[0]).not.toContain('billing-desk');
295+
expect(warns[0]).not.toContain('corp-internal');
296+
});
297+
298+
it('an override REJECTED by a refusal family is reported as rejected, not as a format miss', async () => {
299+
// `docs_url` declares `maxLength: 12`; a 16-character override is ignored
300+
// with the existing `error` line, and the format judgment never runs on a
301+
// value that is not in force.
302+
const { svc, warns, errors } = makeStack({ OS_FORMAT_LAB_DOCS_URL: 'not a url at all' });
303+
const r = await svc.get('format_lab', 'docs_url', ADMIN());
304+
expect(r.source).not.toBe('env');
305+
expect(errors).toHaveLength(1);
306+
expect(errors[0]).toContain('IGNORED');
307+
expect(warns).toEqual([]);
308+
});
309+
});

0 commit comments

Comments
 (0)