diff --git a/.changeset/settings-write-before-engine-bind.md b/.changeset/settings-write-before-engine-bind.md new file mode 100644 index 0000000000..aca2a1c876 --- /dev/null +++ b/.changeset/settings-write-before-engine-bind.md @@ -0,0 +1,20 @@ +--- +"@objectstack/service-settings": patch +"@objectstack/spec": patch +--- + +**Behaviour change (tightening, boot-time only):** a settings write issued before `SettingsService`'s data engine is bound is now **refused loudly** instead of resolving successfully while nothing reaches `sys_setting` (#10159). + +`upsertRow` picks its store on `if (this.engine)`, and the engine is bound in exactly one place — `SettingsServicePlugin` registers a `kernel:ready` hook from its `start()` and calls `bindEngine` inside it. `kernel:ready` handlers run in registration order and every plugin's `init()` runs before any plugin's `start()`, so **every `kernel:ready` hook registered from an `init()` fires inside that window**. A `set()` from there landed in the in-process memory fallback, re-resolved off that same array, and handed the caller a fully resolved value; `sys_setting` received nothing, and neither audit ledger recorded anything (both sinks bind on the same `bindEngine` call). Nothing was logged at any level, because the write did not fail — it succeeded against the wrong store. + +**What an operator will now observe.** A write in that window throws `SettingsEngineNotBoundError` — code `SETTINGS_ENGINE_NOT_BOUND`, status **503** — whose message names the window, the reason, and the fix: move the write to `kernel:bootstrapped` (or later), which fires strictly after every `kernel:ready` handler has settled. Previously that same call returned a resolved value and the setting was silently absent after restart. + +**Nothing outside the window changes.** The refusal is armed only by the new opt-in `SettingsServiceOptions.engineBindPending`, which `SettingsServicePlugin` sets in `init()` and clears on both branches of its `kernel:ready` hook — by `bindEngine` when `objectql` is present, or by the new `SettingsService.settleWithoutEngine()` when it is not. So: + +- a `SettingsService` constructed directly (unit tests, bootstrap, control-plane mock) keeps the in-memory fallback exactly as before — it declares no pending bind, and the guard never arms; +- a lean kernel with no `objectql` keeps the plugin's deliberate degradation: once its `kernel:ready` hook has established that no engine is coming, writes resolve into the memory fallback again (now with a `warn` saying those values are lost on restart); +- reads are untouched in every state, so an ordinary boot-time read of a setting still resolves. + +No shipped caller wrote settings inside the window, so no existing startup sequence becomes an error. + +`SETTINGS_ENGINE_NOT_BOUND` is registered in `ERROR_CODE_LEDGER` per ADR-0112. The status is declared on the error class rather than at an HTTP door because no door can reach it: the window closes at `kernel:ready`, and HTTP servers open their socket at `kernel:listening`, strictly after. diff --git a/content/docs/references/api/contract.mdx b/content/docs/references/api/contract.mdx index 8a04e65c8d..4f4f0a3680 100644 --- a/content/docs/references/api/contract.mdx +++ b/content/docs/references/api/contract.mdx @@ -27,7 +27,7 @@ const result = ApiErrorSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +283 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) | +| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +284 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) | | **declaredCode** | `string` | optional | The producer-declared code, verbatim, when it is not a member of the closed `code` vocabulary — the open, author-authored channel (app-specific spellings; ADR-0112, #9106) | | **message** | `string` | ✅ | Readable error message | | **userMessage** | `string` | optional | Producer-marked user-facing refusal text, verbatim (#9934). Present exactly when the producer opted in at throw time; consumers render it to end users and keep their generic substitution (#3821) for anything unmarked. Status-agnostic; never replaces `message`. | @@ -290,6 +290,7 @@ const result = ApiErrorSchema.parse(data); * `SCHEDULE_DELETE_FAILED` * `SETTINGS_ACTION_FAILED` * `SETTINGS_CRYPTO_UNAVAILABLE` +* `SETTINGS_ENGINE_NOT_BOUND` * `SETTINGS_FORBIDDEN` * `SETTINGS_LOCKED` * `SETTINGS_UNKNOWN_KEY` diff --git a/content/docs/references/api/error-code-ledger.mdx b/content/docs/references/api/error-code-ledger.mdx index bad1569af2..eb774d8487 100644 --- a/content/docs/references/api/error-code-ledger.mdx +++ b/content/docs/references/api/error-code-ledger.mdx @@ -394,6 +394,7 @@ const result = ErrorCode.parse(data); * `SCHEDULE_DELETE_FAILED` * `SETTINGS_ACTION_FAILED` * `SETTINGS_CRYPTO_UNAVAILABLE` +* `SETTINGS_ENGINE_NOT_BOUND` * `SETTINGS_FORBIDDEN` * `SETTINGS_LOCKED` * `SETTINGS_UNKNOWN_KEY` diff --git a/packages/services/service-settings/src/index.ts b/packages/services/service-settings/src/index.ts index 395a4150ed..f82f1738f6 100644 --- a/packages/services/service-settings/src/index.ts +++ b/packages/services/service-settings/src/index.ts @@ -41,6 +41,11 @@ export { // it. Exported so an in-process caller can branch on the refusal (there is no // dedicated wire code for it yet; see the class doc). SettingsCryptoUnavailableError, + // The pre-bind write refusal. Exported for the same reason: an in-process + // caller that runs during boot branches on `code` to tell "too early" + // apart from "locked" / "invalid", and the class carries the 503 itself + // because no HTTP door can reach it (see the class doc). + SettingsEngineNotBoundError, SettingsLockedError, SettingsValidationError, UnknownKeyError, diff --git a/packages/services/service-settings/src/settings-engine-bind-window.test.ts b/packages/services/service-settings/src/settings-engine-bind-window.test.ts new file mode 100644 index 0000000000..6717b9f7ff --- /dev/null +++ b/packages/services/service-settings/src/settings-engine-bind-window.test.ts @@ -0,0 +1,382 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The pre-bind window — **a write that answers "resolved" while nothing + * reaches `sys_setting`.** + * + * ## The defect + * + * `SettingsService.upsertRow` picks its store on `if (this.engine)`, and the + * engine is bound in exactly one place: `SettingsServicePlugin` registers a + * `kernel:ready` hook from its `start()` and calls `bindEngine` inside it. + * `kernel:ready` handlers run in REGISTRATION order (`hooks.get(name).push(…)` + * in `packages/core/src/kernel-base.ts`, dispatched in array order), and every + * plugin's `init()` runs before any plugin's `start()`. So every `kernel:ready` + * hook registered from an `init()` fires BEFORE the engine is bound — and a + * `set()` from there landed in the in-process `memory` array, re-resolved off + * that same array, and handed the caller a fully resolved value while + * `sys_setting` received nothing. + * + * Nothing said so at any level. The write did not fail; it succeeded against + * the wrong store. Both audit ledgers were silent for the same reason (`audit` + * and `auditWriter` bind on the same `bindEngine` call), so the usual evidence + * that a settings write happened was absent too. + * + * The population is ordinarily occupied, not hypothetical: + * `assembleMetadataProtocol` registers the three platform migrations' + * `kernel:ready` hook from `ObjectQLPlugin.init()` + * (`packages/objectql/src/plugin.ts` `init = async` → + * `packages/metadata-protocol/src/plugin.ts`). + * + * ## What the fix is, and what it deliberately is NOT + * + * A write in the window now raises `SettingsEngineNotBoundError` + * (`SETTINGS_ENGINE_NOT_BOUND`, 503) naming `kernel:bootstrapped` as the + * earliest safe phase. The refusal is scoped to a DECLARED, pending bind — + * `SettingsServiceOptions.engineBindPending`, which only + * `SettingsServicePlugin` sets and which both branches of its `kernel:ready` + * hook clear. Every other engine-less reading of the in-memory fallback ("unit + * tests, bootstrap, control-plane mock", and the lean kernel the plugin's + * OPTIONAL `objectql` dependency exists for) is untouched — cases 4 and 5 are + * that assertion, and they are the reason this change alters nothing a + * non-window caller observes. + * + * ## The direction each case pins + * + * The load-bearing assertion is the REFUSAL, not the empty table: on the + * silent-accept behaviour this replaces, `sys_setting` was empty after the + * in-window write too. A case asserting only "no row landed" would have passed + * against the defect. Case 1 therefore records the write's OUTCOME as a + * string — `resolved:…` on the old behaviour, `threw:SETTINGS_ENGINE_NOT_BOUND:503` + * on the new one — and asserts the second. + */ + +import { describe, expect, it } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import type { Plugin, PluginContext } from '@objectstack/core'; +import { ObjectQL } from '@objectstack/objectql'; +import { SysSecret, SysSetting } from '@objectstack/platform-objects/system'; +import type { SettingsManifest } from '@objectstack/spec/system'; +import { SettingsService } from './settings-service.js'; +import { SettingsServicePlugin, wrapEngineAsSettingsEngine } from './settings-service-plugin.js'; +import { SettingsEngineNotBoundError } from './settings-service.types.js'; + +const OWNER_PACKAGE = 'com.objectstack.test.settings-engine-bind-window'; + +/** One plain global key. The window is about WHEN a write lands, not what. */ +const probeManifest: SettingsManifest = { + namespace: 'receipt_probe', + version: 1, + label: 'Receipt probe', + scope: 'global', + specifiers: [ + { type: 'text', key: 'last_run', label: 'Last run', required: false, default: 'never' }, + ], +}; + +// --------------------------------------------------------------------------- +// A driver over plain Maps — enough of `IDataDriver` for the settings write +// path. Same shape as `settings-secret-rotation.test.ts`'s, and for the same +// reason: the real `ObjectQL` sits on top of it, so what these cases measure is +// the real engine's row state rather than a fake's bookkeeping. +// --------------------------------------------------------------------------- + +function makeMemoryDriver() { + const store = new Map>>(); + let nextId = 0; + const copy = (r: Record) => ({ ...r }); + const rowsOf = (object: string) => { + let s = store.get(object); + if (!s) { s = new Map(); store.set(object, s); } + return s; + }; + const matches = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + return (row[k] ?? null) === (v ?? null); + }); + }; + const driver: any = { + name: 'memory', version: '0.0.0', supports: {} as any, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, + async find(o: string, ast: any) { + return [...rowsOf(o).values()].filter((r) => matches(r, ast?.where)).map(copy); + }, + async findOne(o: string, ast: any) { + for (const r of rowsOf(o).values()) if (matches(r, ast?.where)) return copy(r); + return null; + }, + async create(o: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `row_${nextId}`; + const row = { ...data, id }; + rowsOf(o).set(id, row); + return copy(row); + }, + async update(o: string, id: string, data: Record) { + const s = rowsOf(o); + const cur = s.get(id); + if (!cur) return null; + const next = { ...cur, ...data, id }; + s.set(id, next); + return copy(next); + }, + async upsert(o: string, data: Record) { + const id = data.id as string | undefined; + return id && rowsOf(o).has(id) ? this.update(o, id, data) : this.create(o, data); + }, + async delete(o: string, id: string) { return rowsOf(o).delete(id); }, + async count(o: string, ast: any) { return (await this.find(o, ast)).length; }, + async bulkCreate(o: string, rows: Record[]) { + return Promise.all(rows.map((r) => this.create(o, r))); + }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async updateMany(o: string, ast: any, data: Record) { + const rows = await this.find(o, ast); + const s = rowsOf(o); + for (const r of rows) s.set(r.id as string, { ...s.get(r.id as string), ...data, id: r.id }); + return rows.length; + }, + async deleteMany(o: string, ast: any) { + const rows = await this.find(o, ast); + for (const r of rows) rowsOf(o).delete(r.id as string); + return rows.length; + }, + async syncSchema() {}, async dropTable() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, rowsOf }; +} + +/** + * Stands in for `ObjectQLPlugin` in the one respect this file is about: it + * publishes the `objectql` service from `init()`, which is where the real one + * publishes it too (`providesServices` + `init = async` in + * `packages/objectql/src/plugin.ts`). + */ +class EnginePlugin implements Plugin { + name = 'com.objectstack.engine.objectql'; + version = '0.0.0'; + type = 'standard' as const; + providesServices = ['objectql']; + constructor(private readonly engine: ObjectQL) {} + init = async (ctx: PluginContext) => { + ctx.registerService('objectql', this.engine); + }; +} + +/** What one in-window attempt observed. Strings, so the two behaviours are + * distinguishable in a single assertion rather than by absence. */ +interface WindowObservation { + serviceResolvableAtReady?: boolean; + engineBoundAtReady?: boolean; + /** `resolved:` (the defect) or `threw::` (the fix). */ + writeAtReady?: string; + /** `resolved:` — reads are deliberately NOT gated. */ + readAtReady?: string; +} + +/** + * The named population: a plugin registering its `kernel:ready` hook from + * `init()`. It writes AND reads from inside that hook. + */ +class ReadyHookFromInitPlugin implements Plugin { + name = 'com.objectstack.test.ready-hook-from-init'; + version = '0.0.0'; + type = 'standard' as const; + readonly observed: WindowObservation = {}; + init = async (ctx: PluginContext) => { + ctx.hook('kernel:ready', async () => { + let svc: SettingsService | undefined; + try { svc = ctx.getService('settings'); } catch { /* not registered */ } + this.observed.serviceResolvableAtReady = Boolean(svc); + // Reaching into the private field on purpose: "was the engine bound at + // this instant" is the fact the whole window is defined by, and there is + // no public spelling of it. + this.observed.engineBoundAtReady = Boolean((svc as unknown as { engine?: unknown })?.engine); + if (!svc) return; + svc.registerManifest(probeManifest); + try { + const r = await svc.set('receipt_probe', 'last_run', 'written-at-kernel-ready'); + this.observed.writeAtReady = `resolved:${JSON.stringify(r.value)}`; + } catch (e: unknown) { + const err = e as { code?: string; status?: number }; + this.observed.writeAtReady = `threw:${err?.code}:${err?.status}`; + } + try { + const r = await svc.get('receipt_probe', 'last_run'); + this.observed.readAtReady = `resolved:${JSON.stringify(r.value)}`; + } catch (e: unknown) { + const err = e as { code?: string; status?: number }; + this.observed.readAtReady = `threw:${err?.code}:${err?.status}`; + } + }); + }; +} + +/** + * A real kernel booting the real plugin, with the probe above registered + * alongside. `withEngine: false` boots the lean kernel the plugin's OPTIONAL + * `objectql` dependency exists for. + */ +async function bootKernel(opts: { withEngine?: boolean } = {}) { + const withEngine = opts.withEngine !== false; + const { driver, rowsOf } = makeMemoryDriver(); + const engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + for (const o of [SysSetting, SysSecret]) engine.registry.registerObject(o as any, OWNER_PACKAGE); + + const probe = new ReadyHookFromInitPlugin(); + const kernel = new LiteKernel({ logger: { level: 'error' } as never }); + if (withEngine) kernel.use(new EnginePlugin(engine)); + // `manifests: []` + `actionHandlers: {}` — the shipped bundles are irrelevant + // here and registering an action against an unregistered namespace throws in + // `init()`. + kernel.use(new SettingsServicePlugin({ registerRoutes: false, manifests: [], actionHandlers: {} })); + kernel.use(probe); + await kernel.bootstrap(); + + const svc = kernel.getService('settings'); + return { + kernel, engine, svc, probe, rowsOf, + settingRows: () => [...rowsOf('sys_setting').values()], + auditRows: () => [...rowsOf('sys_setting_audit').values()], + }; +} + +// --------------------------------------------------------------------------- +// 1. The window refuses — loudly +// --------------------------------------------------------------------------- + +describe('the pre-bind window refuses a write instead of resolving it', () => { + it('a `kernel:ready` hook registered from `init()` is inside the window, and its write is refused', async () => { + const { probe, settingRows, auditRows } = await bootKernel(); + + // The window, measured: the service is already reachable, the engine is not + // yet bound. Both halves matter — a service that were NOT resolvable would + // not be a silent-loss window, just a missing dependency. + expect(probe.observed.serviceResolvableAtReady).toBe(true); + expect(probe.observed.engineBoundAtReady).toBe(false); + + // THE DIRECTION PIN. On the silent-accept behaviour this replaces the same + // probe recorded `resolved:"written-at-kernel-ready"` — measured on + // `origin/main` before the fix, alongside the identical empty table below. + // So the table being empty is NOT what this case tests; the refusal is. + expect(probe.observed.writeAtReady).toBe('threw:SETTINGS_ENGINE_NOT_BOUND:503'); + + // ADR-0112: `code` AND `status`, never one alone — asserted here on the + // class rather than off a wire envelope, because no HTTP door can reach + // this error (the window closes at `kernel:ready`; sockets open at + // `kernel:listening`). + const err = new SettingsEngineNotBoundError('receipt_probe', ['last_run']); + expect(err.code).toBe('SETTINGS_ENGINE_NOT_BOUND'); + expect(err.status).toBe(503); + // The refusal has to tell the caller what to do instead, or it is just a + // different way to lose the write. + expect(err.message).toContain('kernel:bootstrapped'); + + // And nothing was written anywhere — no row, and no audit row either. + expect(settingRows()).toEqual([]); + expect(auditRows()).toEqual([]); + }); + + it('reads in the window are NOT gated — the ordinary startup sequence still works', async () => { + const { probe } = await bootKernel(); + // The property under test: the read RESOLVED rather than raising. Refusing + // reads too would turn a correct, ordinary boot-time read into an error, + // which is a regression dressed as a fix. + expect(probe.observed.readAtReady).toMatch(/^resolved:/); + // And the value is the manifest DEFAULT. This half is deliberately coupled + // to the refusal in the same hook, and it is a second silent-loss witness: + // on the behaviour this replaces the write above landed in the memory + // fallback, so this read answered `resolved:"written-at-kernel-ready"` — + // the phantom value, read straight back out of the store that no longer + // exists after restart. Measured: neutering the guard turns this assertion + // red with exactly that string. + expect(probe.observed.readAtReady).toBe('resolved:"never"'); + }); +}); + +// --------------------------------------------------------------------------- +// 2. The window ENDS at bind — non-window callers are unchanged +// --------------------------------------------------------------------------- + +describe('after bind, writes behave exactly as before', () => { + it('the identical write lands a real `sys_setting` row and a real audit row', async () => { + const { svc, settingRows, auditRows } = await bootKernel(); + + svc.registerManifest(probeManifest); + const resolved = await svc.set('receipt_probe', 'last_run', 'written-after-boot'); + expect(resolved.value).toBe('written-after-boot'); + + const rows = settingRows(); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ + namespace: 'receipt_probe', + key: 'last_run', + scope: 'global', + value: 'written-after-boot', + }); + expect(auditRows()).toHaveLength(1); + }); +}); + +// --------------------------------------------------------------------------- +// 3. Clause-② pins — the two engine-less populations that are NOT the window +// --------------------------------------------------------------------------- + +describe('engine-less callers outside the window observe nothing new', () => { + it('a directly constructed SettingsService still resolves writes into the memory fallback', async () => { + // "unit tests, bootstrap, control-plane mock" — the documented second + // reading of the in-memory fallback. It declares no pending bind, so the + // guard never arms and this is byte-for-byte the pre-fix behaviour. + const svc = new SettingsService({ env: {} }); + svc.registerManifest(probeManifest); + + const resolved = await svc.set('receipt_probe', 'last_run', 'memory-is-the-store'); + expect(resolved.value).toBe('memory-is-the-store'); + expect((await svc.get('receipt_probe', 'last_run')).value).toBe('memory-is-the-store'); + }); + + it('a kernel with NO objectql settles the window and keeps its deliberate degradation', async () => { + // The plugin declares `objectql` an OPTIONAL dependency and degrades on + // purpose when it is absent. On such a kernel the answer to "is an engine + // coming?" is legitimately NO, and it becomes knowable exactly once — in + // the plugin's own `kernel:ready` hook. From there on writes resolve again. + const { svc, probe } = await bootKernel({ withEngine: false }); + + // Inside the window the answer is not yet known, so the write is still + // refused — the guard is temporal, not a judgement about this kernel. + expect(probe.observed.writeAtReady).toBe('threw:SETTINGS_ENGINE_NOT_BOUND:503'); + + svc.registerManifest(probeManifest); + const resolved = await svc.set('receipt_probe', 'last_run', 'lean-kernel'); + expect(resolved.value).toBe('lean-kernel'); + expect((await svc.get('receipt_probe', 'last_run')).value).toBe('lean-kernel'); + }); + + it('`bindEngine` closes a declared window even without the plugin', async () => { + const { driver } = makeMemoryDriver(); + const engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + for (const o of [SysSetting, SysSecret]) engine.registry.registerObject(o as any, OWNER_PACKAGE); + + const svc = new SettingsService({ env: {}, engineBindPending: true }); + svc.registerManifest(probeManifest); + + await expect(svc.set('receipt_probe', 'last_run', 'too-early')).rejects.toMatchObject({ + code: 'SETTINGS_ENGINE_NOT_BOUND', + status: 503, + }); + + svc.bindEngine(wrapEngineAsSettingsEngine(engine as never)); + const resolved = await svc.set('receipt_probe', 'last_run', 'now-fine'); + expect(resolved.value).toBe('now-fine'); + }); +}); diff --git a/packages/services/service-settings/src/settings-service-plugin.ts b/packages/services/service-settings/src/settings-service-plugin.ts index 5a0b715a31..5be6a27110 100644 --- a/packages/services/service-settings/src/settings-service-plugin.ts +++ b/packages/services/service-settings/src/settings-service-plugin.ts @@ -112,6 +112,12 @@ export class SettingsServicePlugin implements Plugin { this.service = new SettingsService({ crypto: this.opts.crypto, env: this.opts.env, + // The engine arrives later, from this plugin's own `kernel:ready` hook + // below. Declaring the pending bind here is what lets the service REFUSE + // a write in that window instead of resolving it into the in-memory + // fallback while nothing reaches `sys_setting` — see + // `SettingsEngineNotBoundError`. Both branches of that hook clear it. + engineBindPending: true, // #5204 — the service reports a rejected `OS_*` override at `error`, and // it must land in the deployment's real log pipeline rather than raw // stdout. Passed before `registerManifest` below, because that call is @@ -217,6 +223,17 @@ export class SettingsServicePlugin implements Plugin { cryptoProvider: this.opts.cryptoProvider ?? new LocalCryptoProvider(), }, ); + } else { + // No `objectql` on this kernel — the OPTIONAL dependency this plugin + // declares is genuinely absent, so no engine is ever coming and the + // in-memory fallback IS this deployment's store. Settle the window the + // other way rather than leaving writes refused forever: from here on the + // service behaves exactly as it did before the guard existed. + this.service!.settleWithoutEngine(); + ctx.logger?.warn?.( + 'SettingsServicePlugin: no objectql engine — settings persist to the in-process ' + + 'memory fallback only and are lost on restart.', + ); } if (this.opts.registerRoutes === false) return; diff --git a/packages/services/service-settings/src/settings-service.ts b/packages/services/service-settings/src/settings-service.ts index 5998e8a1be..ff35b55c7d 100644 --- a/packages/services/service-settings/src/settings-service.ts +++ b/packages/services/service-settings/src/settings-service.ts @@ -26,6 +26,7 @@ import { type SettingsServiceOptions, envKeyOf, SettingsCryptoUnavailableError, + SettingsEngineNotBoundError, SettingsForbiddenError, SettingsLockedError, SettingsValidationError, @@ -549,6 +550,13 @@ export class SettingsService { private readonly reportedCryptoRefusals = new Set(); /** In-memory fallback when no engine is wired. */ private readonly memory: SettingsRow[] = []; + /** + * True while a `bindEngine` call is DECLARED-but-not-yet-arrived — the + * pre-bind window. See `SettingsServiceOptions.engineBindPending` for why it + * is opt-in, and {@link SettingsEngineNotBoundError} for what the window did + * before this flag existed. + */ + private engineBindPending: boolean; /** Change subscribers, optionally scoped to a namespace. */ private readonly subscribers = new Set<{ ns?: string; @@ -565,6 +573,9 @@ export class SettingsService { this.env = opts.env ?? (typeof process !== 'undefined' ? process.env : {}); this.objectName = opts.objectName ?? DEFAULT_OBJECT; this.logger = opts.logger; + // An engine handed in at construction is already bound — there is no window + // to guard, whatever the caller declared. + this.engineBindPending = Boolean(opts.engineBindPending) && !this.engine; } /** @@ -583,6 +594,9 @@ export class SettingsService { }, ): void { this.engine = engine; + // The window is over: `upsertRow` now takes its engine branch, so the + // pre-bind write guard has nothing left to protect. + this.engineBindPending = false; if (audit) this.audit = audit; if (extras?.secretStore) this.secretStore = extras.secretStore; if (extras?.auditWriter) this.auditWriter = extras.auditWriter; @@ -604,6 +618,43 @@ export class SettingsService { } } + /** + * Settle the pre-bind window the OTHER way: no engine is coming, and the + * in-memory fallback is this deployment's intended store. + * + * The counterpart to {@link bindEngine}, and the half that keeps the refusal + * scoped to a genuine window rather than to "engine-less" in general. + * `SettingsServicePlugin` declares `objectql` an OPTIONAL dependency and + * degrades on purpose when it is absent ("lean test kernels without an + * engine … no sys table, service still up"), so on those kernels the answer + * to "is an engine coming?" is legitimately *no* — and it is knowable exactly + * once, at the moment the plugin's `kernel:ready` hook fails to resolve + * `objectql`. From here on such a service behaves exactly as it did before + * the window guard existed. + * + * Idempotent, and a no-op once an engine is bound. + */ + settleWithoutEngine(): void { + this.engineBindPending = false; + } + + /** + * Refuse a write issued inside the pre-bind window. + * + * Placed at the very top of the write door, BEFORE the manifest lookup and + * the capability gate, because it is a precondition of the SERVICE rather + * than a verdict on the request: in the window there is no durable store for + * any namespace, any key, or any caller, and answering with a + * request-specific error first would describe the wrong problem. There is no + * information-disclosure cost to that ordering here — the window closes at + * `kernel:ready` and no HTTP socket is open until `kernel:listening`, so no + * untrusted caller can reach this branch. + */ + private assertEngineBound(namespace: string, keys: string[]): void { + if (!this.engineBindPending || this.engine) return; + throw new SettingsEngineNotBoundError(namespace, keys); + } + /** * Cascade priority ranks for lock comparisons (lower = higher * precedence). env, ctx: SettingsContext = {}, ): Promise> { + // The pre-bind window: refuse rather than resolve against a store nothing + // will ever read. See `assertEngineBound`. + this.assertEngineBound(namespace, Object.keys(patch)); const reg = this.registry.get(namespace); if (!reg) throw new UnknownNamespaceError(namespace); // [Finding-1] Writing requires the manifest's write capability for an diff --git a/packages/services/service-settings/src/settings-service.types.ts b/packages/services/service-settings/src/settings-service.types.ts index b5ec27b54e..be4a618aed 100644 --- a/packages/services/service-settings/src/settings-service.types.ts +++ b/packages/services/service-settings/src/settings-service.types.ts @@ -318,6 +318,26 @@ export interface SettingsServiceOptions { * environment. */ env?: Record; + /** + * Declares that a `bindEngine` call is COMING — the caller owns a lifecycle + * in which the engine arrives later, and until it does this service has no + * durable store. + * + * Set, a write raises {@link SettingsEngineNotBoundError} instead of landing + * in the in-memory fallback and answering "resolved" (see that class for the + * measured silent-loss window it closes). Clear it — by binding + * (`bindEngine`) or by settling the question the other way + * (`settleWithoutEngine`) — as soon as the lifecycle knows the answer. + * + * ⚠️ Deliberately OPT-IN and defaulted OFF. The in-memory fallback has a + * second, legitimate reading — "unit tests, bootstrap, control-plane mock", + * where memory IS the intended store and no engine is ever coming — and a + * service constructed without this flag keeps that reading byte for byte. + * Only a caller that knows a bind is pending can distinguish the two, so only + * a caller that knows says so. `SettingsServicePlugin` sets it in `init()` + * and clears it on BOTH branches of its `kernel:ready` hook. + */ + engineBindPending?: boolean; /** Object name backing the K/V store. Defaults to 'sys_setting'. */ objectName?: string; /** @@ -409,6 +429,87 @@ export class SettingsCryptoUnavailableError extends Error { } } +/** + * Thrown when a write reaches `SettingsService` while its data engine binding + * is still PENDING — the pre-`bindEngine` window. + * + * ## What the refusal replaces + * + * `upsertRow` picks its store on `if (this.engine)`, so before the engine is + * bound a write landed in the in-process `memory` array and `setMany` + * re-resolved off that same array — the caller got a fully resolved value back + * and NOTHING reached `sys_setting`. No log line at any level, because the + * write did not fail: it succeeded against the wrong store. Both audit ledgers + * were silent for the same reason (`audit` and `auditWriter` are bound by the + * same `bindEngine` call), so the usual evidence that a settings write happened + * was absent too. + * + * ## Who was in that window + * + * `SettingsServicePlugin` binds the engine from a `kernel:ready` hook it + * registers in `start()`. Every plugin's `init()` runs before any plugin's + * `start()`, and hooks fire in registration order (`hooks.get(name).push(...)` + * in `packages/core/src/kernel-base.ts`, dispatched in array order) — so every + * `kernel:ready` hook registered from an `init()` runs INSIDE the window. That + * is an ordinarily-occupied position, not a hypothetical one: + * `assembleMetadataProtocol` registers the three platform migrations' hook from + * `ObjectQLPlugin.init()` (`packages/objectql/src/plugin.ts` `init = async` → + * `packages/metadata-protocol/src/plugin.ts`). + * + * ## Why a refusal rather than a buffered replay + * + * A buffer would have to keep the promise the resolved value makes, and it + * cannot: + * + * - **Pre-flight is evaluated against the wrong store.** `setMany`'s env-lock + * and upper-scope-lock checks read through `loadRows`, which in the window + * reads `memory` — so an in-window write is validated against a store that + * does not contain the persisted locks. Replaying it would commit a write a + * real pre-flight would have refused with `SETTINGS_LOCKED`. + * - **Encrypted specifiers cannot be buffered safely.** `cryptoProvider` and + * `secretStore` arrive on the SAME `bindEngine` call, so a buffer would have + * to hold plaintext in process memory until bind — the opposite direction + * from {@link SettingsCryptoUnavailableError}, which already refuses this + * exact combination. + * - **A replay has nobody left to report to.** The caller was told "resolved" + * during boot; a replay that then fails at bind time re-creates the silent + * loss this error exists to close, one phase later. + * + * ## Wire spelling — and why the status lives on the class + * + * Unreachable from an HTTP door by construction: the window closes at + * `kernel:ready`, and HTTP servers open their socket at `kernel:listening`, + * strictly after (`packages/spec/src/contracts/plugin-lifecycle-events.ts`). + * So there is no `sendError` site to carry the status the way + * `settings-routes.ts` carries the others, and it is declared here instead. + * + * **503, deliberately not 500** — and deliberately not + * `SETTINGS_CRYPTO_UNAVAILABLE`'s 500 either. That one is a configuration + * fault where no retry ever succeeds until an operator wires a provider; this + * one is purely temporal: the identical write succeeds, unchanged, one + * lifecycle phase later. 503 is "not yet", which is exactly the caller's + * remedy. + */ +export class SettingsEngineNotBoundError extends Error { + readonly code = 'SETTINGS_ENGINE_NOT_BOUND' as const; + readonly status = 503 as const; + constructor( + readonly namespace: string, + readonly keys: string[], + ) { + super( + `Refusing to write settings ${keys.length === 1 ? 'key' : 'keys'} ` + + `${keys.map((k) => `'${namespace}.${k}'`).join(', ')}: the SettingsService data ` + + 'engine is not bound yet, so the write would resolve successfully while nothing ' + + 'reached `sys_setting`. SettingsServicePlugin binds the engine from a `kernel:ready` ' + + 'hook registered in its `start()`, and hooks fire in registration order — so every ' + + '`kernel:ready` hook registered from a plugin `init()` runs inside this window. ' + + 'Move the write to `kernel:bootstrapped` (or later), which fires strictly after every ' + + '`kernel:ready` handler has settled. Reads are unaffected.', + ); + } +} + /** * [Finding-1] Thrown when an ENFORCED (HTTP-boundary) caller lacks the * capability a manifest declares for the operation — `readPermission` for diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index 2672d60664..b940646963 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -533,6 +533,7 @@ export const ERROR_CODE_LEDGER = { 'INTERNAL', 'SETTINGS_ACTION_FAILED', // a declared action ran and reported ok:false 'SETTINGS_CRYPTO_UNAVAILABLE', // [#8273] fail-closed write refusal: declared-encrypted value, nothing confidential wired to encrypt it — a SERVER fault (500, deliberately not 503: no retry succeeds until an operator wires a cryptoProvider; the message carries that fix) + 'SETTINGS_ENGINE_NOT_BOUND', // pre-bind write refusal: a write reached SettingsService before `bindEngine`, where it would have resolved successfully while nothing reached `sys_setting` (503, temporal — the identical write succeeds one lifecycle phase later; the class carries the status itself because the window closes at `kernel:ready` and no HTTP socket exists until `kernel:listening`, so no door can reach it) 'SETTINGS_FORBIDDEN', 'SETTINGS_LOCKED', 'SETTINGS_UNKNOWN_KEY',