From 16b70e332e3fbcfe82bb7b712ec34efd2629af44 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 02:32:52 +0000 Subject: [PATCH 1/6] wip(rest): parse crud/metadata/batch/routes at construction (#11984 work in progress) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- packages/rest/src/rest-server.ts | 207 ++++++++--- .../rest-sub-config-parse-not-cast.test.ts | 347 ++++++++++++++++++ 2 files changed, 494 insertions(+), 60 deletions(-) create mode 100644 packages/rest/src/rest-sub-config-parse-not-cast.test.ts diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 81e8ced492..db8b978dba 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -71,7 +71,7 @@ import { refuseRepeatedQueryParams, assertFilterParamSuppliedOnce } from './quer // ignored filter is the one wrong answer a caller cannot detect. import { refuseUnknownQueryParams } from './query-allowlist.js'; import type { DirectMountedRoute, MountedRouteSource } from './direct-mount.js'; -import { RestServerConfig, RestApiConfig, CrudEndpointsConfig, MetadataEndpointsConfig, BatchEndpointsConfig, RouteGenerationConfig } from '@objectstack/spec/api'; +import { RestServerConfig, RestApiConfig, CrudEndpointsConfigParsed, RouteGenerationConfigParsed } from '@objectstack/spec/api'; // [#11683] The catalog's own floor for "a required `code` and no more specific // one" — see its use in `registerSharingEndpoints`, where the nested ADR-0112 // envelope declares `code` REQUIRED while the flat classification it re-dresses @@ -80,7 +80,14 @@ import { standardErrorCodeForHttpStatus } from '@objectstack/spec/api'; // [#11637] The DECLARED contract for `config.api`, imported as a VALUE rather // than a type. Both hops into this package were casts, so this schema had // never run on any deployment path — see `assertDeclaredApiConfig` below. -import { RestApiConfigSchema } from '@objectstack/spec/api'; +import { + RestApiConfigSchema, + CrudEndpointsConfigSchema, + MetadataEndpointsConfigSchema, + BatchEndpointsConfigSchema, + RouteGenerationConfigSchema, +} from '@objectstack/spec/api'; +import type { z } from 'zod'; import { DataProtocol, MetadataProtocol } from '@objectstack/spec/api'; // [#9741] Declared request shapes for the meta-read doors below — imported so // each door's request literal is compiled against the spec contract instead of @@ -738,7 +745,7 @@ type NormalizedRestServerConfig = { delete: boolean; list: boolean; }; - patterns: CrudEndpointsConfig['patterns']; + patterns: CrudEndpointsConfigParsed['patterns']; dataPrefix: string; objectParamStyle: 'path' | 'query'; }; @@ -774,24 +781,94 @@ type NormalizedRestServerConfig = { includeObjects: string[] | undefined; excludeObjects: string[] | undefined; nameTransform: 'none' | 'plural' | 'kebab-case' | 'camelCase'; - overrides: RouteGenerationConfig['overrides']; + overrides: RouteGenerationConfigParsed['overrides']; + }; +}; + +/** + * The DECLARED contract of each `RestServerConfig` sub-object this seam parses, + * keyed by the sub-object's name — the table `normalizeConfig` runs before it + * builds anything (#11637 for `api`, #11984 for the four siblings). + * + * `api` is the one entry with a subtraction: its retired `requireAuth` + * tombstone is `.omit()`ed because this seam does not own that key's posture + * (see {@link RestServer.assertDeclaredApiConfig}). The four siblings carry no + * tombstone of their own and are taken whole. ⛔ `RestServerConfigSchema` — the + * whole-config schema — is deliberately NOT in this table: its `openApi31` + * tombstone (#4579) is a `retiredKey()` whose parse REFUSES the key, while + * #3963 chose warn-and-ignore for a retired REST config key. Parsing per + * sub-object leaves that tombstone unexecuted, which keeps the posture a + * maintainer would have to flip on purpose. + * + * Built on first use, not at module load: every schema here is a `lazySchema` + * Proxy whose whole point is deferring allocation until someone parses, and + * calling `.omit()` at module top level would resolve `RestApiConfigSchema` on + * every import of this file. Cached because `.omit()` allocates a fresh schema + * and a `RestServer` is constructed per boot (and per test). + */ +function buildDeclaredSubConfigSchemas() { + return { + api: RestApiConfigSchema.omit({ requireAuth: true }), + crud: CrudEndpointsConfigSchema, + metadata: MetadataEndpointsConfigSchema, + batch: BatchEndpointsConfigSchema, + routes: RouteGenerationConfigSchema, }; +} +type DeclaredSubConfigSchemas = ReturnType; +type DeclaredSubConfigName = keyof DeclaredSubConfigSchemas; +let declaredSubConfigSchemasCache: DeclaredSubConfigSchemas | undefined; +function declaredSubConfigSchemas(): DeclaredSubConfigSchemas { + return (declaredSubConfigSchemasCache ??= buildDeclaredSubConfigSchemas()); +} + +/** + * The exported name of each declared schema, for the refusal text: the + * prescription is the payload, and an operator reading a boot failure must be + * able to find the rule that refused them without reading our source. + */ +const DECLARED_SUB_CONFIG_SCHEMA_NAMES: Record = { + api: 'RestApiConfigSchema', + crud: 'CrudEndpointsConfigSchema', + metadata: 'MetadataEndpointsConfigSchema', + batch: 'BatchEndpointsConfigSchema', + routes: 'RouteGenerationConfigSchema', }; /** - * The declared `api` contract, minus the ONE retired key whose posture this seam - * does not own (see {@link RestServer.assertDeclaredApiConfig}). + * Run one sub-object's DECLARED contract and return the parsed output — + * defaults applied, unknown keys stripped (every schema in the table is a + * non-strict `z.object()`). Throws on a value the schema rejects, naming the + * sub-object and every failing key with zod's own issue text. This is a + * construction-time refusal, not an HTTP envelope: nothing has been mounted + * yet, and the operator reading the boot log is the audience. + * + * An absent sub-object parses as `{}` — exactly what the `?? {}` in front of + * the old casts read — so the schema's defaults fill it. * - * Built on first use, not at module load: `RestApiConfigSchema` is a - * `lazySchema` Proxy whose whole point is deferring allocation until someone - * parses, and calling `.omit()` at module top level would resolve it on every - * import of this file. Cached because `.omit()` allocates a fresh schema and a - * `RestServer` is constructed per boot (and per test). + * `rationale` lets a caller append a paragraph the issues justify (the + * `api.version` mount rationale) and ONLY then: appending it unconditionally + * was measured to send an operator to a line of their config they never wrote. */ -function buildDeclaredApiConfigSchema() { - return RestApiConfigSchema.omit({ requireAuth: true }); +function parseDeclaredSubConfig( + name: DeclaredSubConfigName, + schema: T, + value: unknown, + rationale?: (issues: ReadonlyArray) => string, +): z.output { + const result = schema.safeParse(value ?? {}); + if (result.success) return result.data; + + const details = result.error.issues + .map((issue) => ` - ${name}.${issue.path.join('.') || '(root)'}: ${issue.message}`) + .join('\n'); + throw new Error( + `REST API configuration is invalid: \`${name}\` does not satisfy ` + + `\`${DECLARED_SUB_CONFIG_SCHEMA_NAMES[name]}\` (@objectstack/spec/api), the schema that declares it.\n` + + details + + (rationale?.(result.error.issues) ?? ''), + ); } -let declaredApiConfigSchemaCache: ReturnType | undefined; /** * RestServer @@ -3407,55 +3484,59 @@ export class RestServer { * `.omit()` to make some config boot — a strategy outside the enum is * wrong where it is WRITTEN, not where it is read. * - * The sibling sub-objects (`crud`, `metadata`, `batch`, `routes`) are still - * cast, not parsed, and carry declared constraints of their own - * (`batch.maxBatchSize: z.number().int().min(1).max(1000)`, the - * `routes.nameTransform` enum, ...). Same defect class, filed separately: - * this change deliberately puts ONE narrowing in front of contract review - * rather than five. + * The sibling sub-objects (`crud`, `metadata`, `batch`, `routes`) went + * through the same door in [#11984], one narrowing later — parsed by + * `parseDeclaredSubConfig` from the same table, and their parsed output + * CONSUMED. The asymmetry with `api` is measured, not stylistic: for each + * of the four, every key `normalizeConfig` reads is one its schema + * declares (the key diff is empty), and none carries a tombstone, so a + * consumed parse cannot strip anything the runtime honours. `api` keeps + * #11637's validate-only shape here; its `??` chain below duplicates the + * schema's defaults key for key today, and folding it onto the parse is a + * separate, separately-measured change — not a rider on the siblings. */ private assertDeclaredApiConfig(api: unknown): void { - declaredApiConfigSchemaCache ??= buildDeclaredApiConfigSchema(); - const result = declaredApiConfigSchemaCache.safeParse(api ?? {}); - if (result.success) return; - - const details = result.error.issues - .map((issue) => ` - api.${issue.path.join('.') || '(root)'}: ${issue.message}`) - .join('\n'); - // The `version` rationale is appended only when `version` is what - // failed. Measured during this change's own ablation: a - // `projectResolution` refusal printed the whole "an empty version - // mounts the entire API at /api//" paragraph, which reads as a - // diagnosis of a key the operator did not write — worse than no - // rationale, because it sends them to the wrong line of their config. - const versionFailed = result.error.issues.some((issue) => issue.path[0] === 'version'); - throw new Error( - 'REST API configuration is invalid: `api` does not satisfy `RestApiConfigSchema` ' - + '(@objectstack/spec/api), the schema that declares it.\n' - + details - + (versionFailed + parseDeclaredSubConfig('api', declaredSubConfigSchemas().api, api, (issues) => ( + // The `version` rationale is appended only when `version` is what + // failed. Measured during #11637's own ablation: a + // `projectResolution` refusal printed the whole "an empty version + // mounts the entire API at /api//" paragraph, which reads as a + // diagnosis of a key the operator did not write — worse than no + // rationale, because it sends them to the wrong line of their config. + issues.some((issue) => issue.path[0] === 'version') ? '\nThis is refused at construction because `api.version` becomes a path segment in ' + 'EVERY route this server mounts (`getApiBasePath()` = `apiPath ?? ' + '`${basePath}/${version}``) — an empty version mounts the entire API at `/api//`, ' + 'and one carrying `/` splices an extra segment into every route.' - : ''), - ); + : '' + )); } /** * Normalize configuration with defaults */ private normalizeConfig(config: RestServerConfig): NormalizedRestServerConfig { - // [#11637] Parse before the cast, not instead of it: the cast below is - // what makes the rest of this method type-check, and it is only sound - // once the declared contract has actually been run. + // [#11637] `api`: parse BEFORE the cast, not instead of it — the cast + // is what makes the `api` block below type-check, and it is only sound + // once the declared contract has actually been run. Validate-only; see + // `assertDeclaredApiConfig` for why its parsed output is discarded. this.assertDeclaredApiConfig(config.api); const api = (config.api ?? {}) as Partial; - const crud = (config.crud ?? {}) as Partial; - const metadata = (config.metadata ?? {}) as Partial; - const batch = (config.batch ?? {}) as Partial; - const routes = (config.routes ?? {}) as Partial; - + // [#11984] The four siblings: parsed AND consumed. Each used to be + // `(config. ?? {}) as Partial<...>`, so `batch.maxBatchSize: 0` + // was the live batch cap (`0` is not nullish) and + // `routes.nameTransform: 'snake_case'` sat in this config as if it were + // declared. The parsed output is safe to build from because, per + // sub-object, every key read below is one its schema declares + // (measured key by key — the diff is empty for all four), so the + // non-strict parse cannot strip anything the runtime honours, and the + // schema's `.default()`s ARE the defaults: one source, not two. + const schemas = declaredSubConfigSchemas(); + const crud = parseDeclaredSubConfig('crud', schemas.crud, config.crud); + const metadata = parseDeclaredSubConfig('metadata', schemas.metadata, config.metadata); + const batch = parseDeclaredSubConfig('batch', schemas.batch, config.batch); + const routes = parseDeclaredSubConfig('routes', schemas.routes, config.routes); + return { api: { version: api.version ?? 'v1', @@ -3476,8 +3557,11 @@ export class RestServer { crud: { // Per key, not per object: since ADR-0122 `crud.operations` is the // AUTHOR state, so a caller may enable three of the five and leave the - // rest to the schema's own per-key `.default(true)`. `??` on the whole - // object would only have filled it when it was absent entirely. + // rest to the schema's own per-key `.default(true)` — which the parse + // above applies whenever the object is PRESENT. The `??` here covers + // the one case the schema leaves open: `operations` itself is + // `.optional()`, so an absent object arrives as `undefined`, not as + // five defaults. operations: { create: crud.operations?.create ?? true, read: crud.operations?.read ?? true, @@ -3486,13 +3570,13 @@ export class RestServer { list: crud.operations?.list ?? true, }, patterns: crud.patterns, - dataPrefix: crud.dataPrefix ?? '/data', - objectParamStyle: crud.objectParamStyle ?? 'path', + dataPrefix: crud.dataPrefix, + objectParamStyle: crud.objectParamStyle, }, metadata: { - prefix: metadata.prefix ?? '/meta', - enableCache: metadata.enableCache ?? true, - cacheTtl: metadata.cacheTtl ?? 3600, + prefix: metadata.prefix, + enableCache: metadata.enableCache, + cacheTtl: metadata.cacheTtl, // [ADR-0106 D8] Default ON — masking is the platform default and // ships with the current major. The key has a declared seat // (`MetadataEndpointsConfigSchema.maskObjectFields` in @@ -3502,6 +3586,8 @@ export class RestServer { // knob the runtime `/metadata` dispatcher shares (it has no REST // config to read). maskObjectFields: isObjectSchemaMaskingEnabled(metadata.maskObjectFields), + // `endpoints` is `.optional()` like `crud.operations` above: the + // `??` is for the absent object; the parse fills a present one. endpoints: { types: metadata.endpoints?.types ?? true, items: metadata.endpoints?.items ?? true, @@ -3510,20 +3596,21 @@ export class RestServer { }, }, batch: { - maxBatchSize: batch.maxBatchSize ?? 200, - enableBatchEndpoint: batch.enableBatchEndpoint ?? true, + maxBatchSize: batch.maxBatchSize, + enableBatchEndpoint: batch.enableBatchEndpoint, + // `operations` is `.optional()` — same shape as `crud.operations`. operations: { createMany: batch.operations?.createMany ?? true, updateMany: batch.operations?.updateMany ?? true, deleteMany: batch.operations?.deleteMany ?? true, upsertMany: batch.operations?.upsertMany ?? true, }, - defaultAtomic: batch.defaultAtomic ?? true, + defaultAtomic: batch.defaultAtomic, }, routes: { includeObjects: routes.includeObjects, excludeObjects: routes.excludeObjects, - nameTransform: routes.nameTransform ?? 'none', + nameTransform: routes.nameTransform, overrides: routes.overrides, }, }; diff --git a/packages/rest/src/rest-sub-config-parse-not-cast.test.ts b/packages/rest/src/rest-sub-config-parse-not-cast.test.ts new file mode 100644 index 0000000000..ab6c31dd49 --- /dev/null +++ b/packages/rest/src/rest-sub-config-parse-not-cast.test.ts @@ -0,0 +1,347 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#11984] The REST server runs the DECLARED contract of its four sibling + * sub-objects — `crud`, `metadata`, `batch`, `routes` — at construction. + * + * #11637 made `RestServer.normalizeConfig` parse `config.api` against + * `RestApiConfigSchema` before the cast the rest of the method type-checks + * against, and deliberately left the four siblings cast-only so that ONE + * narrowing went in front of contract review rather than five. Each sibling + * carries declared constraints that consequently never executed + * (`packages/spec/src/api/rest-server.zod.ts`): + * + * batch.maxBatchSize: z.number().int().min(1).max(1000).default(200) + * routes.nameTransform: z.enum(['none', 'plural', 'kebab-case', 'camelCase']) + * crud.objectParamStyle: z.enum(['path', 'query']) + * metadata.cacheTtl: z.number().int().default(3600) + * + * Measured on the pre-change tree (`origin/main` @ `08e49496f`): every value + * in §A constructed a server. `batch.maxBatchSize: 0` became the live batch + * cap (`maxBatch = batch.maxBatchSize ?? 200` — `0` is not nullish), and + * `routes.nameTransform: 'snake_case'` sat in the normalized config as if it + * were declared. + * + * ⛔ ANTI-VACUITY — the same rule as `rest-config-parse-not-cast.test.ts`: a + * pin that asks the SCHEMA whether it refuses `maxBatchSize: 0` is green on + * every tree (`packages/spec`'s own `rest-server.test.ts` already pins that). + * Every case below drives the REAL `RestServer` construction — or, in §B, the + * real plugin composition — so what it measures is whether the SERVER refuses. + * `refusal()` answers `''` when construction succeeds, and `''` contains no + * key name, so every `toContain` below is its own positive control. + * + * §C bounds the narrowing: the seam refuses exactly what the schema declares + * and nothing this seam invented. Two bounds are named because the card that + * filed this defect guessed them wrong: a NEGATIVE `cacheTtl` is declared + * `.int()` only, so it stays accepted; and an UNKNOWN key inside a sub-object + * is stripped, not refused — all four schemas are non-strict `z.object()`s. + * + * §D pins the consumption decision. For these four sub-objects every key + * `normalizeConfig` reads is declared by the sub-object's schema (measured + * key by key; the diff is empty for all four), so the PARSED output is what + * the normalized config is built from and the schema's own defaults are the + * defaults. `api` keeps #11637's validate-only posture — its `.omit()`ed + * tombstone is the reason — and is not this file's subject. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { IHttpServer } from '@objectstack/spec/contracts'; +import type { RestServerConfig } from '@objectstack/spec/api'; +import { RestServer, type RestProtocol } from './rest-server.js'; +import { createRestApiPlugin } from './rest-api-plugin.js'; + +function makeServer(): IHttpServer { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), + use: vi.fn(), listen: vi.fn(), close: vi.fn(), + } as unknown as IHttpServer; +} + +function makeProtocol(): RestProtocol { + return { + getMetaItems: vi.fn(async ({ type }: { type: string }) => ({ type, items: [] })), + } as unknown as RestProtocol; +} + +/** Construct the real server with the config as given — the seam under test. */ +function construct(config: RestServerConfig): RestServer { + return new RestServer(makeServer(), makeProtocol(), config); +} + +/** + * The construction refusal's message, or `''` when the server constructed. + * An empty answer fails every `toContain` below on its own, which is what + * makes each of them a positive control for the `not.toContain` beside it. + */ +function refusal(config: RestServerConfig): string { + try { + construct(config); + return ''; + } catch (err) { + return err instanceof Error ? err.message : String(err); + } +} + +/** The part of the normalized config these pins read back. */ +type NormalizedView = { + crud: { + operations: Record<'create' | 'read' | 'update' | 'delete' | 'list', boolean>; + patterns: Record | undefined; + dataPrefix: string; + objectParamStyle: string; + }; + metadata: { + prefix: string; + cacheTtl: number; + maskObjectFields: boolean; + endpoints: Record<'types' | 'items' | 'item' | 'schema', boolean>; + }; + batch: { + maxBatchSize: number; + defaultAtomic: boolean; + operations: Record<'createMany' | 'updateMany' | 'deleteMany' | 'upsertMany', boolean>; + }; + routes: { + includeObjects: string[] | undefined; + nameTransform: string; + }; +}; + +/** Read the normalized config back off a constructed server. */ +function normalized(config: RestServerConfig): NormalizedView { + return (construct(config) as unknown as { config: NormalizedView }).config; +} + +// --------------------------------------------------------------------------- +// §A — the server refuses what each sibling schema declares invalid +// --------------------------------------------------------------------------- + +describe('[#11984] §A RestServer construction runs the four sibling schemas', () => { + it('refuses `batch.maxBatchSize: 0` — the value that became the live batch cap', () => { + const message = refusal({ batch: { maxBatchSize: 0 } }); + expect(message, 'the refusal must name the sub-object AND the key').toContain('batch.maxBatchSize'); + expect(message, 'and the schema that declares the bound').toContain('BatchEndpointsConfigSchema'); + expect(message, "zod's own issue text carries the declared bound").toContain('>=1'); + }); + + it('refuses `batch.maxBatchSize: 2000` — above the declared maximum', () => { + const message = refusal({ batch: { maxBatchSize: 2000 } }); + expect(message).toContain('batch.maxBatchSize'); + expect(message).toContain('<=1000'); + }); + + it('refuses `batch.maxBatchSize: 2.5` — declared `.int()`', () => { + expect(refusal({ batch: { maxBatchSize: 2.5 } })).toContain('batch.maxBatchSize'); + }); + + it('refuses `routes.nameTransform: "snake_case"` — an option outside the declared enum', () => { + const message = refusal({ routes: { nameTransform: 'snake_case' as never } }); + expect(message).toContain('routes.nameTransform'); + expect(message).toContain('RouteGenerationConfigSchema'); + expect(message, 'the declared vocabulary is part of the prescription').toContain('kebab-case'); + }); + + it('refuses `crud.objectParamStyle: "header"` — an option outside the declared enum', () => { + const message = refusal({ crud: { objectParamStyle: 'header' as never } }); + expect(message).toContain('crud.objectParamStyle'); + expect(message).toContain('CrudEndpointsConfigSchema'); + }); + + it('refuses `metadata.cacheTtl: 2.5` — declared `.int()`', () => { + const message = refusal({ metadata: { cacheTtl: 2.5 } }); + expect(message).toContain('metadata.cacheTtl'); + expect(message).toContain('MetadataEndpointsConfigSchema'); + }); + + it('refuses a declared key written with the wrong type', () => { + expect(refusal({ crud: { dataPrefix: 42 as never } })).toContain('crud.dataPrefix'); + expect(refusal({ metadata: { enableCache: 'yes' as never } })).toContain('metadata.enableCache'); + expect(refusal({ routes: { includeObjects: 'account' as never } })).toContain('routes.includeObjects'); + }); + + it('refuses `crud.patterns` keyed by an operation the CRUD vocabulary does not contain', () => { + // `patterns` is `z.record(CrudOperation, ...)`: an enum-keyed record, + // which zod validates key by key — so a pattern for an operation that + // does not exist is refused, not carried along and never matched. + const message = refusal({ crud: { patterns: { bogus: { method: 'GET', path: '/x' } } as never } }); + expect(message).toContain('crud.patterns'); + expect(message).toContain('bogus'); + }); + + it('refuses a partial `routes.overrides..operations` — the declared record is exhaustive over the five operations', () => { + // Same enum-keyed record, with a NON-optional value: zod requires + // every declared operation, so the missing ones are named one by one. + // The input TYPE already demanded all five at typed authoring sites; + // this is the day the runtime agrees with `tsc`. + const message = refusal({ routes: { overrides: { account: { operations: { list: false } as never } } } }); + expect(message).toContain('routes.overrides.account.operations.create'); + expect(message).toContain('routes.overrides.account.operations.read'); + }); + + it('lists every failing key of the sub-object in one refusal', () => { + const message = refusal({ batch: { maxBatchSize: 0, defaultAtomic: 'yes' as never } }); + expect(message).toContain('batch.maxBatchSize'); + expect(message).toContain('batch.defaultAtomic'); + }); + + it('a sibling refusal never diagnoses `api.version` — a key this config did not write', () => { + // #11637 appends an "empty version mounts the API at /api//" rationale + // when `api.version` is what failed. A sibling refusal must not + // inherit it: that paragraph sends the operator to a line they never + // wrote. + const message = refusal({ batch: { maxBatchSize: 0 } }); + expect(message, 'positive control: the refusal is present').toContain('batch.maxBatchSize'); + expect(message).not.toContain('/api//'); + expect(message).not.toContain('api.version'); + }); +}); + +// --------------------------------------------------------------------------- +// §B — the real plugin composition, i.e. BOTH cast hops +// --------------------------------------------------------------------------- + +type StartContext = Parameters['start']>>[0]; + +function bootCtx(): StartContext { + const services: Record = { 'http.server': makeServer(), protocol: makeProtocol() }; + return { + registerService: vi.fn(), + getService: vi.fn((name: string) => { + if (name in services) return services[name]; + throw new Error(`Service '${name}' not found`); + }), + getServices: vi.fn(() => new Map(Object.entries(services))), + hook: vi.fn(), + trigger: vi.fn().mockResolvedValue(undefined), + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + getKernel: vi.fn(), + } as unknown as StartContext; +} + +describe('[#11984] §B the refusal survives the plugin path', () => { + it('CONTROL: this ctx really does boot a REST server', async () => { + // `createRestApiPlugin.start()` returns quietly when `http.server` or + // `protocol` is missing, so a rejection below could otherwise be + // attributed to a thin ctx rather than to the config. + const ctx = bootCtx(); + await expect(createRestApiPlugin({ api: { batch: { maxBatchSize: 200 } } }).start!(ctx)).resolves.toBeUndefined(); + }); + + it('rejects `createRestApiPlugin({ api: { batch: { maxBatchSize: 0 } } }).start()`', async () => { + await expect( + createRestApiPlugin({ api: { batch: { maxBatchSize: 0 } } }).start!(bootCtx()), + ).rejects.toThrow(/batch\.maxBatchSize/); + }); + + it('rejects `createRestApiPlugin({ api: { routes: { nameTransform: "snake_case" } } }).start()`', async () => { + await expect( + createRestApiPlugin({ api: { routes: { nameTransform: 'snake_case' as never } } }).start!(bootCtx()), + ).rejects.toThrow(/routes\.nameTransform/); + }); +}); + +// --------------------------------------------------------------------------- +// §C — REGRESSION GUARDS (green BEFORE this change and after) +// --------------------------------------------------------------------------- + +describe('[#11984] §C regression guards — the narrowing is exactly the declared one', () => { + it('an empty config still constructs, with the declared defaults', () => { + const cfg = normalized({}); + expect(cfg.batch.maxBatchSize).toBe(200); + expect(cfg.metadata.cacheTtl).toBe(3600); + expect(cfg.metadata.prefix).toBe('/meta'); + expect(cfg.crud.dataPrefix).toBe('/data'); + expect(cfg.crud.objectParamStyle).toBe('path'); + expect(cfg.routes.nameTransform).toBe('none'); + }); + + it('accepts the declared `maxBatchSize` bounds inclusively', () => { + expect(normalized({ batch: { maxBatchSize: 1 } }).batch.maxBatchSize).toBe(1); + expect(normalized({ batch: { maxBatchSize: 1000 } }).batch.maxBatchSize).toBe(1000); + }); + + it('accepts every option the declared enums contain, and reads each back', () => { + for (const nameTransform of ['none', 'plural', 'kebab-case', 'camelCase'] as const) { + expect(normalized({ routes: { nameTransform } }).routes.nameTransform, nameTransform).toBe(nameTransform); + } + for (const objectParamStyle of ['path', 'query'] as const) { + expect(normalized({ crud: { objectParamStyle } }).crud.objectParamStyle, objectParamStyle).toBe(objectParamStyle); + } + }); + + it('KEEPS a negative `metadata.cacheTtl` — declared `.int()` only, with no lower bound', () => { + // The bound on the narrowing: the card that filed this defect listed + // "a negative TTL" among the values the parse would refuse, and the + // schema declares no such rule. This seam enforces the contract as + // written; a lower bound is `packages/spec`'s to declare. + expect(normalized({ metadata: { cacheTtl: -1 } }).metadata.cacheTtl).toBe(-1); + expect(normalized({ metadata: { cacheTtl: 0 } }).metadata.cacheTtl).toBe(0); + }); + + it('STRIPS an unknown key inside a sub-object rather than refusing it — the schemas are non-strict', () => { + expect(() => construct({ batch: { bogus: 1 } as never })).not.toThrow(); + expect(() => construct({ routes: { overrides: { account: { enabled: false, bogus: 1 } as never } } })).not.toThrow(); + }); + + it('KEEPS the retired top-level `openApi31` key at its ignore posture — the whole-config tombstone is not run here', () => { + // `RestServerConfigSchema.openApi31` is a `retiredKey()` tombstone + // (#4579) whose parse REFUSES the key. This seam parses the five + // sub-objects, never the whole config, so the tombstone keeps the + // posture #3963 chose for `api.requireAuth`: flipping either into a + // boot failure is that decision's to make, not this seam's. + expect(() => construct({ openApi31: {} } as never)).not.toThrow(); + }); + + it('the `api` sub-object still runs its own declared contract (#11637)', () => { + expect(refusal({ api: { version: '' } })).toContain('api.version'); + }); +}); + +// --------------------------------------------------------------------------- +// §D — the parsed output is CONSUMED: defaults come from the schema, and what +// an author wrote survives it +// --------------------------------------------------------------------------- + +describe('[#11984] §D the four siblings consume the parsed output', () => { + it('a declared in-range value is preserved, not replaced by the default', () => { + expect(normalized({ batch: { maxBatchSize: 500 } }).batch.maxBatchSize).toBe(500); + expect(normalized({ metadata: { cacheTtl: 60 } }).metadata.cacheTtl).toBe(60); + expect(normalized({ crud: { dataPrefix: '/records' } }).crud.dataPrefix).toBe('/records'); + expect(normalized({ routes: { includeObjects: ['account'] } }).routes.includeObjects).toEqual(['account']); + }); + + it('KEEPS `metadata.maskObjectFields: false` — the ADR-0106 D8 opt-out survives the parse', () => { + // The reason this card waited on #11983: before the key had a + // declared seat, a consumed parse would have STRIPPED it and turned + // masking back on for a deployment that turned it off. + expect(normalized({ metadata: { maskObjectFields: false } }).metadata.maskObjectFields).toBe(false); + expect(normalized({}).metadata.maskObjectFields).toBe(true); + }); + + it('KEEPS a partial `crud.operations` — per-key defaults, the AUTHOR state (ADR-0122)', () => { + expect(normalized({ crud: { operations: { list: false } } }).crud.operations).toEqual({ + create: true, read: true, update: true, delete: true, list: false, + }); + expect(normalized({}).crud.operations).toEqual({ + create: true, read: true, update: true, delete: true, list: true, + }); + }); + + it('KEEPS a partial `batch.operations` and `metadata.endpoints` the same way', () => { + expect(normalized({ batch: { operations: { deleteMany: false } } }).batch.operations).toEqual({ + createMany: true, updateMany: true, deleteMany: false, upsertMany: true, + }); + expect(normalized({ metadata: { endpoints: { schema: false } } }).metadata.endpoints).toEqual({ + types: true, items: true, item: true, schema: false, + }); + }); + + it('KEEPS a partial `crud.patterns` without inventing entries for the operations it does not name', () => { + // `patterns` is an enum-keyed record with an OPTIONAL value: zod walks + // every declared operation, and this pin is the guarantee that the + // ones an author did not write do not come back as explicit + // `undefined` entries (a consumer iterating the record would see them). + const cfg = normalized({ crud: { patterns: { list: { method: 'GET', path: '/x' } } } }); + expect(Object.keys(cfg.crud.patterns ?? {})).toEqual(['list']); + }); +}); From abb61564c608eecf15d0027d914231cd7aaf1fe9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:06:08 +0000 Subject: [PATCH 2/6] feat(rest): parse crud/metadata/batch/routes at construction and consume the parse Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- .../rest-sub-configs-parsed-not-cast.md | 102 ++++++++++++++++++ content/docs/permissions/system-context.mdx | 8 +- .../rest-sub-config-parse-not-cast.test.ts | 22 ++-- 3 files changed, 121 insertions(+), 11 deletions(-) create mode 100644 .changeset/rest-sub-configs-parsed-not-cast.md diff --git a/.changeset/rest-sub-configs-parsed-not-cast.md b/.changeset/rest-sub-configs-parsed-not-cast.md new file mode 100644 index 0000000000..9c8da934dd --- /dev/null +++ b/.changeset/rest-sub-configs-parsed-not-cast.md @@ -0,0 +1,102 @@ +--- +'@objectstack/rest': minor +--- + +**BREAKING (accept-set tightening)**: `RestServer` now parses `config.crud`, +`config.metadata`, `config.batch` and `config.routes` against the schemas that +declare them (`CrudEndpointsConfigSchema`, `MetadataEndpointsConfigSchema`, +`BatchEndpointsConfigSchema`, `RouteGenerationConfigSchema` in +`@objectstack/spec/api`) at construction, instead of casting to them — and +builds the normalized config from the parsed output (#11984). + +The constraints were always declared. `packages/spec/src/api/rest-server.zod.ts` +carries `batch.maxBatchSize: z.number().int().min(1).max(1000)`, the +`routes.nameTransform` and `crud.objectParamStyle` enums and +`metadata.cacheTtl: z.number().int()`, and nothing ran them: both hops into +`@objectstack/rest` are casts, the plugin declares no `configSchema`, and #11637 +deliberately parsed `api` alone so that one narrowing went in front of contract +review rather than five. Measured on the pre-fix tree: `batch.maxBatchSize: 0` +constructed happily and became the live batch cap (`?? 200` does not fire — `0` +is not nullish), and `routes.nameTransform: 'snake_case'` sat in the normalized +config as if it were declared. + +**Newly refused, all at `new RestServer(...)` / `createRestApiPlugin().start()`, +with a message naming the sub-object, the failing key(s) and the declaring +schema** (a construction-time refusal, not an HTTP envelope): + +- `batch.maxBatchSize` outside `1..1000` or not an integer — `0`, `-5`, + `2000`, `2.5`. Refused with zod's own bound text (`expected number to be >=1`, + `<=1000`, `expected int`). +- `routes.nameTransform` outside `'none' | 'plural' | 'kebab-case' | 'camelCase'`. +- `crud.objectParamStyle` outside `'path' | 'query'`. +- `metadata.cacheTtl` that is not an integer (`2.5`, `'60'`). +- A declared key of any of the four written with the wrong type: + `crud.dataPrefix: 42`, `metadata.enableCache: 'yes'`, + `routes.includeObjects: 'account'`, `batch.defaultAtomic: 'yes'`, ... +- `crud.patterns` keyed by an operation outside the CRUD vocabulary + (`patterns: { bogus: {...} }`), or a pattern whose `method` is not an HTTP + method — `patterns` is an enum-keyed `z.record`, which zod validates key by key. +- A **partial** `routes.overrides..operations`. That record is + `z.record(CrudOperation, z.boolean())` with a non-optional value, which zod 4 + reads as exhaustive: all five operations must be present. The input TYPE + already demanded all five at typed authoring sites; this is the day the + runtime agrees with `tsc`. + +**Deliberately NOT refused** — the narrowing is exactly what the schemas +declare, and no more: + +- A **negative** `metadata.cacheTtl`. The card that filed this defect listed + "a negative TTL" among the values the parse would refuse; the schema declares + `.int()` only, with no lower bound, so `-1` and `0` stay accepted. A lower + bound is `packages/spec`'s to declare, and is filed separately. +- **Unknown keys inside a sub-object**: all four schemas are non-strict + `z.object()`s, so `batch: { bogus: 1 }` is stripped, not refused — as before, + where the cast simply never read it. +- The retired whole-config key `openApi31` (#4579). Its `retiredKey()` + tombstone lives on `RestServerConfigSchema`, and this seam parses the five + sub-objects rather than the whole config, so the tombstone stays unexecuted: + the key keeps the ignore posture #3963 chose for `api.requireAuth`, and + flipping it into a boot failure is a maintainer's decision, not this seam's. +- `api`: unchanged from #11637 / #12450 (validate-only, `requireAuth` still + `.omit()`ed). + +**The parsed output is now consumed** for the four sub-objects — defaults come +from the schema and unknown keys are stripped — because the decision was +measured per sub-object rather than inherited from `api`: for each of the four, +every key `normalizeConfig` reads is one its schema declares (the key diff is +empty), and none carries a tombstone, so nothing a consumed parse could strip +is anything the runtime honours. The one honoured-but-undeclared key this +family ever had, `metadata.maskObjectFields`, gained its declared seat in +#11983 and is pinned to survive the parse. Defaults are unchanged +(`maxBatchSize` 200, `cacheTtl` 3600, `dataPrefix` `/data`, `prefix` `/meta`, +`nameTransform` `'none'`, every operation/endpoint switch on, masking on per +ADR-0106 D8), and a partial `crud.operations` / `batch.operations` / +`metadata.endpoints` still takes per-key defaults (ADR-0122 author state). The +seam is one table of declared sub-object schemas and one `parseDeclaredSubConfig`; +`api` runs through the same table with its `.omit()`. + +**Migration.** Correct the offending key at its producer; the refusal names the +sub-object, the key, the declared rule and the schema that declares it. A +deployment that meant "no batch cap" wants `enableBatchEndpoint: false` or +`api.enableBatch: false` (the cap's range is the declared policy), and a +partial `routes.overrides..operations` wants all five operations +spelled out. + +**In-repo blast radius, measured per sub-object on `origin/main` @ `08e49496f`.** +140 files construct a REST server (`new RestServer(` or +`createRestApiPlugin(`, 277 sites); across all of them, **zero** pass a +`crud` / `metadata` / `batch` / `routes` block carrying any key the four +schemas declare (the `routes: { data: '', ... }` fixtures are `discovery.routes` +payloads, and every `metadata: { ... }` inside those files is endpoint or plugin +metadata — verified by scanning each block for the schema's own keys; positive +control: `rest-server.ts`'s own `@example` and `normalizeConfig` blocks hit). +Repo-wide value census of the constrained keys, every file type: `maxBatchSize` +24 lines, 3 out-of-range literals — two are `packages/spec`'s own schema tests +(`0`, `2000`, which never construct a server) and one is a different schema's +key (`tracing.test.ts`); `nameTransform` and `objectParamStyle` 6 lines each, 0 +unknown values; `cacheTtl` 69 lines, 1 non-integer literal (`30.7` in +`packages/runtime`'s endpoint-policy tests — the declarative endpoint's +`cacheTtl`, a different schema). No fixture changes; no in-repo boot path is +affected. + + diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 7fe8c239f8..2c6327f59d 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -64,7 +64,7 @@ not on any flag. ## How the flag is set `isSystem` is **server-constructed and never client-supplied**. Inbound HTTP -cannot set it (`packages/rest/src/rest-server.ts:1445`, `:1474`), and neither +cannot set it (`packages/rest/src/rest-server.ts:1522`, `:1551`), and neither can an action body (`packages/runtime/src/domains/actions.ts:404`). It is written by internal callers only, as an option on the engine call: @@ -103,7 +103,7 @@ that silently does not happen. | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:246` | | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` | | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` | -| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1477` | +| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1554` | ### 2. Write pipeline and data integrity @@ -158,7 +158,7 @@ The largest single consumer — **20 of the 109 sites**. |:--|:---|:---|:---|:---| | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:136` | | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:399` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4629`, `:5992`, `:6240`, `:6671`, `:6864` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4716`, `:6079`, `:6327`, `:6758`, `:6951` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | | 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:246`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | @@ -199,7 +199,7 @@ assuming `isSystem` covers it is a documented source of bugs. | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:286` | -| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1445`, `:1474`; `domains/actions.ts:404` | +| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1522`, `:1551`; `domains/actions.ts:404` | --- diff --git a/packages/rest/src/rest-sub-config-parse-not-cast.test.ts b/packages/rest/src/rest-sub-config-parse-not-cast.test.ts index ab6c31dd49..8480db2195 100644 --- a/packages/rest/src/rest-sub-config-parse-not-cast.test.ts +++ b/packages/rest/src/rest-sub-config-parse-not-cast.test.ts @@ -336,12 +336,20 @@ describe('[#11984] §D the four siblings consume the parsed output', () => { }); }); - it('KEEPS a partial `crud.patterns` without inventing entries for the operations it does not name', () => { - // `patterns` is an enum-keyed record with an OPTIONAL value: zod walks - // every declared operation, and this pin is the guarantee that the - // ones an author did not write do not come back as explicit - // `undefined` entries (a consumer iterating the record would see them). - const cfg = normalized({ crud: { patterns: { list: { method: 'GET', path: '/x' } } } }); - expect(Object.keys(cfg.crud.patterns ?? {})).toEqual(['list']); + it('KEEPS a partial `crud.patterns` — the written pattern survives, and no pattern is invented', () => { + // `patterns` is `z.record(CrudOperation, CrudEndpointPatternSchema.optional())`, + // which zod 4 reads as an EXHAUSTIVE record: the parse walks all five + // operations and writes each one's value into the output, so the four + // an author did not write come back as explicit `undefined` entries — + // exactly the declared shape (`Record`, + // which is also why this fixture needs `as never`: the input TYPE + // demands all five keys while the runtime accepts a partial). That + // key-enumeration quirk is the spec's to settle (`z.partialRecord`), + // filed separately, so this pin asserts only what the contract + // promises whichever way that lands: the one written pattern is + // preserved, and no operation gains a pattern it was not given. + const cfg = normalized({ crud: { patterns: { list: { method: 'GET', path: '/x' } } as never } }); + expect(cfg.crud.patterns?.list).toEqual({ method: 'GET', path: '/x' }); + expect(Object.values(cfg.crud.patterns ?? {}).filter((pattern) => pattern !== undefined)).toHaveLength(1); }); }); From e9fddb90bb98e5978d00b9a2e358b35d7fa763c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:20:43 +0000 Subject: [PATCH 3/6] chore(docs): regenerate the system-context census page after merging origin/main Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- content/docs/permissions/system-context.mdx | 24 ++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 2c6327f59d..fb64816d23 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -109,18 +109,18 @@ that silently does not happen. | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:10914` | -| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11076` | -| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9772` | +| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:10981` | +| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11149` | +| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9777` | | 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1746` | -| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9809`, `readonly-strict-errors.ts:66` | -| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5730` | -| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3599`, `:3609`, `:3636` | +| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9814`, `readonly-strict-errors.ts:66` | +| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5735` | +| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3604`, `:3614`, `:3641` | | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` | | 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:98` | -| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6428` | -| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11662` | -| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11591` | +| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6433` | +| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11742` | +| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11671` | ### 3. Sharing (`plugin-sharing`) @@ -179,8 +179,8 @@ a reader tracing where elevation travels needs them. | # | Site | Package | What it does | |:--|:---|:---|:---| -| 62 | `objectql/src/engine.ts:3406` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | -| 63 | `objectql/src/engine.ts:14011` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | +| 62 | `objectql/src/engine.ts:3411` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | +| 63 | `objectql/src/engine.ts:14091` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | | 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report | | 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across | @@ -195,7 +195,7 @@ assuming `isSystem` covers it is a documented source of bugs. |:---|:---|:---| | "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1909` (rationale at `:1819`–`1821`, #3760), `flow.zod.ts:685` | | "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) | -| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9755`–`9772` | +| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9760`–`9777` | | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:286` | From 4fe663ec902cd164f849d0989dc8f6061ff06063 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 03:59:31 +0000 Subject: [PATCH 4/6] docs(changeset): name the census positive control precisely Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- .changeset/rest-sub-configs-parsed-not-cast.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.changeset/rest-sub-configs-parsed-not-cast.md b/.changeset/rest-sub-configs-parsed-not-cast.md index 9c8da934dd..41a5813e8a 100644 --- a/.changeset/rest-sub-configs-parsed-not-cast.md +++ b/.changeset/rest-sub-configs-parsed-not-cast.md @@ -89,7 +89,8 @@ spelled out. schemas declare (the `routes: { data: '', ... }` fixtures are `discovery.routes` payloads, and every `metadata: { ... }` inside those files is endpoint or plugin metadata — verified by scanning each block for the schema's own keys; positive -control: `rest-server.ts`'s own `@example` and `normalizeConfig` blocks hit). +control: `rest-server.ts`'s own `NormalizedRestServerConfig` and `normalizeConfig` +blocks hit). Repo-wide value census of the constrained keys, every file type: `maxBatchSize` 24 lines, 3 out-of-range literals — two are `packages/spec`'s own schema tests (`0`, `2000`, which never construct a server) and one is a different schema's From 741d9c2d9dce2a31c5266ad74121da70b066fcb3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 09:25:06 +0000 Subject: [PATCH 5/6] chore(docs): regenerate the system-context census after merging origin/main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `content/docs/permissions/system-context.mdx` is a generated artifact anchored by source line numbers. `origin/main` added 24 lines to `packages/rest/src/rest-server.ts` and moved anchors in `objectql/src/engine.ts`, `plugin-auth`, `metadata-protocol` and `objectql/src/registry.ts`, so the plain merge left the page stale: `check:system-context-census` reported 16 problems over 145 anchors and 109 census sites (8 `site-without-a-row`, 6 `anchor-is-not-a-read-site`, 2 `ledger-row-unused` — all pure line rot). `pnpm gen:system-context-census` re-anchored 10 citations; the gate now reads `OK -- 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read`. This is the regeneration half of `scripts/pm/os-regen-merge.sh` (its step 4), discharging the deferral the pre-commit hook recorded for the merge commit. No source, test or changeset content is touched. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WXyGTWPbbreqXow7Z2pZCk --- content/docs/permissions/system-context.mdx | 32 ++++++++++----------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index fb64816d23..087009f37f 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -97,7 +97,7 @@ that silently does not happen. | 8 | `explain()` may target a principal other than the caller | plugin-security | Get: no `manage_users` / delegated-admin check | `security-plugin.ts:3857` | | 9 | Anonymous-deny treats the caller as authenticated | core | Get: passes the 401 seam with no `userId` | `anonymous-deny.ts:154` | | 10 | Permission-set projection middleware skipped | plugin-security | Lose: projection of permission-set-derived columns | `permission-set-projection.ts:1015` | -| 11 | Session-resolution middleware skipped | plugin-auth | Get: no session lookup attempted | `auth-plugin.ts:1296` | +| 11 | Session-resolution middleware skipped | plugin-auth | Get: no session lookup attempted | `auth-plugin.ts:1301` | | 12 | Per-request performance timings disclosed | observability | Get: timing headers a normal caller cannot pull | `perf-timing.ts:474` | | 13 | Permission-set **overlay discard** skips the tenant-admin assertion | plugin-security | Get: an overlay can be discarded with no authenticated tenant administrator | `permission-set-overlay-discard.ts:142` | | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:246` | @@ -109,18 +109,18 @@ that silently does not happen. | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:10981` | -| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11149` | -| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9777` | +| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11128` | +| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11296` | +| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9895` | | 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1746` | -| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9814`, `readonly-strict-errors.ts:66` | -| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5735` | -| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3604`, `:3614`, `:3641` | +| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9943`, `readonly-strict-errors.ts:66` | +| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5762` | +| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3606`, `:3616`, `:3643` | | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` | | 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:98` | -| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6433` | -| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11742` | -| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11671` | +| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6460` | +| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11889` | +| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11818` | ### 3. Sharing (`plugin-sharing`) @@ -160,7 +160,7 @@ The largest single consumer — **20 of the 109 sites**. | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:399` | | 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4716`, `:6079`, `:6327`, `:6758`, `:6951` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | -| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:246`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | +| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:276`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | | 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:102` | | 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:95`, `:128` | @@ -179,8 +179,8 @@ a reader tracing where elevation travels needs them. | # | Site | Package | What it does | |:--|:---|:---|:---| -| 62 | `objectql/src/engine.ts:3411` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | -| 63 | `objectql/src/engine.ts:14091` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | +| 62 | `objectql/src/engine.ts:3413` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | +| 63 | `objectql/src/engine.ts:14238` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | | 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report | | 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across | @@ -193,9 +193,9 @@ assuming `isSystem` covers it is a documented source of bugs. | Assumption | Reality | Anchor | |:---|:---|:---| -| "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1909` (rationale at `:1819`–`1821`, #3760), `flow.zod.ts:685` | +| "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1971` (rationale at `:1881`–`1883`, #3760), `flow.zod.ts:685` | | "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) | -| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9760`–`9777` | +| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9878`–`9895` | | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:286` | @@ -235,7 +235,7 @@ should recognise it instead of re-deriving it. rule-materialised grant that the next reconcile silently restores. 5. **`applySystemFields` does not read this flag.** It is named as if it did. - `packages/objectql/src/registry.ts:459` is **schema-side column + `packages/objectql/src/registry.ts:464` is **schema-side column provisioning** — which columns an object carries — and consumes `ExecutionContext.isSystem` zero times. The write-time ownership behaviour people attribute to it is row 2, in `plugin-security`. From 02fd466d5cc024c162723015f73fff695ccfdede Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 11:26:26 +0000 Subject: [PATCH 6/6] docs(changeset): enumerate the three newly-refused shapes the contract review named (#11984) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The isolated contract reviewer (comment 5507350669 §4) and the director's reconciliation adopt as required prose three refusals the changeset left under its generic "wrong type" bullet: an explicit `null` at a declared key (the cast-era `??` chain read it as absent and defaulted it; zod's `.default()` fills `undefined` only), a `crud.patterns` entry missing its required `path`, and a sub-object that is not an object at all. Every bullet is a measurement, not a transcription: a throwaway driver constructed the REAL `RestServer` with each shape and the refusal text quoted here is what it printed, with an empty config as the discriminating control (the driver is not committed). The partial-`operations` bullet and the Migration sentence now cite #14365, so the later `z.partialRecord` widening is traceable from the refusal it reverses. Changeset prose only — no source, test or other file changed. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WXyGTWPbbreqXow7Z2pZCk --- .../rest-sub-configs-parsed-not-cast.md | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/.changeset/rest-sub-configs-parsed-not-cast.md b/.changeset/rest-sub-configs-parsed-not-cast.md index 41a5813e8a..c0c336dcf8 100644 --- a/.changeset/rest-sub-configs-parsed-not-cast.md +++ b/.changeset/rest-sub-configs-parsed-not-cast.md @@ -33,14 +33,28 @@ schema** (a construction-time refusal, not an HTTP envelope): - A declared key of any of the four written with the wrong type: `crud.dataPrefix: 42`, `metadata.enableCache: 'yes'`, `routes.includeObjects: 'account'`, `batch.defaultAtomic: 'yes'`, ... +- An explicit **`null`** at any declared key of the four — + `batch: { maxBatchSize: null }`, `metadata: { cacheTtl: null }`, + `crud: { dataPrefix: null }`. The cast-era `??` chain read `null` as absent + and applied the default; the parse refuses it (`batch.maxBatchSize: Invalid + input: expected number, received null`), because zod's `.default()` fills + `undefined` only. +- A sub-object that is not an object at all — `batch: 'x'`, `routes: []` — + refused at the sub-object root (`batch.(root): Invalid input: expected + object, received string`), where the cast admitted it unchanged and every key + read came back `undefined`, so every key silently took its default. - `crud.patterns` keyed by an operation outside the CRUD vocabulary (`patterns: { bogus: {...} }`), or a pattern whose `method` is not an HTTP - method — `patterns` is an enum-keyed `z.record`, which zod validates key by key. + method, or a pattern missing its required `path` — `patterns` is an + enum-keyed `z.record`, which zod validates key by key, and + `CrudEndpointPatternSchema.path` is a plain `z.string()`. - A **partial** `routes.overrides..operations`. That record is `z.record(CrudOperation, z.boolean())` with a non-optional value, which zod 4 reads as exhaustive: all five operations must be present. The input TYPE already demanded all five at typed authoring sites; this is the day the - runtime agrees with `tsc`. + runtime agrees with `tsc`. #14365 proposes `z.partialRecord` for this record; + when that lands the refusal reverses, and the §A pin for it in + `rest-sub-config-parse-not-cast.test.ts` is deleted with it. **Deliberately NOT refused** — the narrowing is exactly what the schemas declare, and no more: @@ -80,7 +94,7 @@ sub-object, the key, the declared rule and the schema that declares it. A deployment that meant "no batch cap" wants `enableBatchEndpoint: false` or `api.enableBatch: false` (the cap's range is the declared policy), and a partial `routes.overrides..operations` wants all five operations -spelled out. +spelled out (until #14365 lands). **In-repo blast radius, measured per sub-object on `origin/main` @ `08e49496f`.** 140 files construct a REST server (`new RestServer(` or