From af6afda3d98ca42963add38a796b5fe80086c00c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 15:16:32 +0000 Subject: [PATCH 1/2] feat(spec)!: hooks and datasources reject unknown keys (#4001 data step) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the last two ledger entries that still carried a provisional (p) classification. The ledger's own rule for these was "verify before tightening", and verification changed the answer for one of them. CLASSIFICATION. Both types are authorable on the same evidence: they sit in BUILTIN_METADATA_TYPE_SCHEMAS, so one shape backs defineStack() parsing, /api/v1/meta/types/:type, and the Studio form. But the blanket `authorable (p)` on hook.zod.ts was too wide — HookContextSchema in the same file is the RUNTIME shape the engine hands a handler. It stays tolerant, and must: strictness there would turn an engine-internal enrichment (as `provenance` was in #3712) into a breaking change for anyone parsing a context they were given. Strict now: HookSchema + retryPolicy; both hook-body branches; DatasourceSchema + pool/healthCheck/ssl/retryPolicy; ExternalDatasourceSettingsSchema + its validation block; DatasourceCapabilities; DriverDefinitionSchema. Still open, deliberately: HookContextSchema and its session/provenance/user blocks; datasource `config` and `readReplicas` (per-driver by construction — the driver's own configSchema validates them). That openness is exactly why the top level had to close: a connection key written one level too high was stripped and the datasource then connected on driver defaults rather than failing. Those keys are prescribed into `config`; a top-level `password` is pointed at `external.credentialsRef`, because relocating an inlined secret is not the fix. CORRECTS AN ASSUMPTION THE EARLIER STEPS WERE WRITTEN UNDER. Strictness does not change the published JSON Schema. build-schemas.ts converts with the default io:'output', and in output mode zod emits additionalProperties:false for a .strip() object too — the post-parse shape genuinely has no extra keys. Verified by regenerating with and without these flips: Datasource.json is byte-identical. So the JSON Schema had been advertising additionalProperties:false while the zod parse quietly accepted and discarded unknown keys. These flips align the parse with the contract already published rather than widening it. The approval note in the ledger reads as though its flip carried strictness INTO the JSON schema; it did not, and the ledger now says so. SCOPE LIMIT, since it bounds who this reaches. There is no defineHook() factory — it is referenced twice in object.zod.ts describe strings but does not exist, unlike definePosition/defineTool/defineAgent/defineApp/defineView/ defineDatasource. Authors write `const H: Hook = {…}`, a bare type annotation that never parses, so hook strictness bites at artifact/registry LOAD time, not as they type. Datasource has its factory and so rejects at authoring time. Adding defineHook() is a new API surface and belongs in its own change. The metadata-authoring-lint coverage test failed on cue: lintables fell 16 → 14 because both types graduated out of lint coverage into parse coverage. That test exists to force a human to confirm a shrink is a graduation and not a bug — so `hook` moved out of the pinned-coverage list and both moved into the strict list. Verified: full spec suite (7136) green; gen:schema clean (covers the #3746 toJSONSchema-on-strict-lazySchema hazard); authorable-surface, liveness, empty-state, variant-docs, doc-authoring and changeset gates green; both first-party example datasources parse under strict (no finding this time — unlike the app step, these were already clean). objectql has 26 pre-existing failures in protocol-data / protocol-unknown-query- param (#4164/#4134 territory, untouched here). Confirmed pre-existing by stashing these changes and re-running: identical 26 failed | 59 passed. Refs #4001 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0147tNF4Snk7Ry1KGt4a5PY4 --- .../unknown-key-strictness-data-step.md | 44 +++ content/docs/releases/v17.mdx | 32 ++ .../2026-07-unknown-key-strictness-ledger.md | 33 ++- packages/spec/src/data/datasource.zod.ts | 275 +++++++++++++++++- packages/spec/src/data/hook-body.zod.ts | 67 ++++- packages/spec/src/data/hook.zod.ts | 85 +++++- .../kernel/metadata-authoring-lint.test.ts | 10 +- 7 files changed, 524 insertions(+), 22 deletions(-) create mode 100644 .changeset/unknown-key-strictness-data-step.md diff --git a/.changeset/unknown-key-strictness-data-step.md b/.changeset/unknown-key-strictness-data-step.md new file mode 100644 index 0000000000..7cec7ffe89 --- /dev/null +++ b/.changeset/unknown-key-strictness-data-step.md @@ -0,0 +1,44 @@ +--- +"@objectstack/spec": minor +--- + +Hooks and datasources reject unknown keys (#4001 data step). + +Closes the last two entries in the strictness ledger that still carried a +provisional classification. Both were confirmed authorable the same way: they sit +in `BUILTIN_METADATA_TYPE_SCHEMAS`, so one shape backs `defineStack()` parsing, +`/api/v1/meta/types/:type`, and the Studio form. + +Now strict: + +- `HookSchema` + its `retryPolicy`, and both hook-body branches + (`ExpressionBodySchema`, `ScriptBodySchema`). A misspelt `capabilities` + stripped to the empty default and the sandbox threw at invocation time instead + of at parse; a misspelt `timeoutMs`/`memoryMb` silently downgraded the body to + the enclosing hook's limits. +- `DatasourceSchema` + `pool` / `healthCheck` / `ssl` / `retryPolicy`, + `ExternalDatasourceSettingsSchema` + its `validation` block, + `DatasourceCapabilities`, and `DriverDefinitionSchema`. + +Deliberately still tolerant: + +- `HookContextSchema` and its `session` / `provenance` / `user` blocks — the + runtime shape the engine hands a handler. Strictness there would turn an + engine-internal enrichment (as `provenance` was in #3712) into a breaking + change for anyone parsing a context they were given. +- `datasource.config` and `readReplicas` — per-driver by construction; the + driver's own `configSchema` validates them. + +Errors are self-fixing: connection keys written one level too high (`host`, +`port`, `filename`, `url`, …) are prescribed into `config`; a top-level +`password` is pointed at `external.credentialsRef` rather than merely relocated; +and the two near-miss spellings that cross between shapes carry aliases +(hook-level `timeout` vs body-level `timeoutMs`; hook `retryPolicy.backoffMs` vs +datasource `retryPolicy.baseDelayMs`). + +Note for anyone reading the earlier steps: strictness does not change the +published JSON Schema. `build-schemas.ts` converts with `io: 'output'`, where zod +emits `additionalProperties: false` for `.strip()` objects too — verified by +regenerating both ways (`Datasource.json` is byte-identical). The JSON Schema was +already advertising `additionalProperties: false` while the parse silently +dropped keys; this aligns the parse with the published contract. diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index 0ab0823350..1073be5022 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -358,6 +358,38 @@ schema to the two highest-risk authorable surfaces, per the triage in schema carries `additionalProperties: false` into the Studio form and `registerFlow()` config validation, so a mis-keyed approval `config` is rejected at registration too. +- **Hooks** — `HookSchema`, its `retryPolicy`, and both hook-body branches + (`expression` and `js`). A body was the worst place to lose a key quietly: a + misspelt `capabilities` stripped to the empty default and the sandbox then + threw at *invocation* time, on whichever code path first touched `ctx.api` — + far from the typo. A misspelt `timeoutMs`/`memoryMb` silently downgraded the + body to the enclosing hook's looser limits. Two near-miss spellings now carry + aliases because they are genuinely easy to cross: hook-level `timeout` vs + body-level `timeoutMs`, and hook `retryPolicy.backoffMs` vs datasource + `retryPolicy.baseDelayMs`. `HookContextSchema` — the runtime shape the engine + hands your handler — stays tolerant and always will. +- **Datasources** — `DatasourceSchema` with its `pool` / `healthCheck` / `ssl` / + `retryPolicy` blocks, the ADR-0015 `external` federation settings and their + `validation` policy, `DatasourceCapabilities`, and `DriverDefinitionSchema`. + `config` and `readReplicas` stay **open** records: their shape is per-driver + and the driver's own `configSchema` validates them. That openness is why the + top level had to close — a connection key written one level too high (`host` + next to `driver` instead of inside `config`) was stripped, and the datasource + then connected on driver defaults rather than failing. Those keys now + prescribe the move into `config`; a top-level `password` is instead pointed at + `external.credentialsRef`, because relocating an inlined secret is not the fix. + A dropped key in `capabilities` was quieter still: an unregistered capability + reads as `false`, so the engine stopped pushing that work down to the driver + and recomputed it in memory. + +One clarification, since these flips are easy to over-read: making a schema +strict does **not** change its published JSON Schema. `build-schemas.ts` +converts with `io: 'output'`, and in output mode zod emits +`additionalProperties: false` for a `.strip()` object too — the post-parse shape +genuinely has no extra keys. So the JSON Schema was already advertising +`additionalProperties: false` while the zod parse quietly accepted and discarded +unknown keys. These flips align the parse with the contract that was already +published; they do not widen it. Every rejection is written to be self-fixing: it names the offending key and, where recognisable, the canonical spelling (`steps` → `nodes`, edge diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 1bd1542226..635b4362a1 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -157,10 +157,10 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). | `field.zod.ts` | 11 | authorable | partially strict | | `filter.zod.ts` / `query.zod.ts` | 11+10 | open | query dialect — user data flows through; validated semantically elsewhere | | `driver-nosql.zod.ts` / `driver.zod.ts` / `driver-sql.zod.ts` | 10+9+2 | wire | driver capability contracts | -| `datasource.zod.ts` | 9 | authorable (p) | stack-authored config — **candidate** | +| `datasource.zod.ts` | 9 | authorable | **strict as of #4001 data step** — all 9: `DatasourceSchema` (+ `pool` / `healthCheck` / `ssl` / `retryPolicy`), `ExternalDatasourceSettingsSchema` (+ `validation`), `DatasourceCapabilities`, `DriverDefinitionSchema`. `config` + `readReplicas` stay `z.record` by construction (per-driver shapes; the driver's own `configSchema` validates them) — which is precisely why the top level had to close: a connection key written one level too high was stripped, and the datasource then connected on driver defaults instead of failing | | `analytics.zod.ts` | 8 | mixed (p) | | | `document.zod.ts` | 8 | wire (p) | | -| `hook.zod.ts` / `hook-body.zod.ts` | 6+2 | authorable (p) | `defineHook` — **candidate** | +| `hook.zod.ts` / `hook-body.zod.ts` | 6+2 | mixed | **strict as of #4001 data step** for the AUTHORING shapes: `HookSchema` (+ `retryPolicy`) and both body branches (`ExpressionBodySchema` / `ScriptBodySchema`). `HookContextSchema` and its `session` / `provenance` / `user` blocks are the RUNTIME shape the engine hands a handler — they stay tolerant, and must: strictness there would make an engine-internal enrichment (as `provenance` was in #3712) a breaking change for anyone parsing a context they were given. The file's old blanket `authorable (p)` was too wide — verification split it | | `mapping.zod.ts` | 3 | authorable (p) | | | `external-catalog.zod.ts` | 4 | wire (p) | | | `field-value.zod.ts` / `seed.zod.ts` / `validation.zod.ts` | 1 ea | mixed (p) | | @@ -213,13 +213,11 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). ## Next steps (verify-then-enforce, one shape at a time) -1. `data/hook.zod.ts`, `data/datasource.zod.ts` — `defineHook` / stack config - (both still provisional (p) classifications — verify before tightening). -2. The `@objectstack/lint` unknown-key WARNING layer: non-breaking, shippable +1. The `@objectstack/lint` unknown-key WARNING layer: non-breaking, shippable in a minor, and it extends AI-detectable coverage to every remaining authorable site at once while accumulating evidence (which keys real tenant projects actually carry) for a v18 strict close-out. -3. Promote this ledger to a machine-checked gate (pattern of +2. Promote this ledger to a machine-checked gate (pattern of `packages/spec/liveness/` + `check:liveness`) once enough of the surface is classified that the table above is enforceable rather than descriptive. @@ -241,5 +239,28 @@ Done in the app step, PR A: the seven audit-dead AppSchema keys tombstoned migration entry), clearing the enforce-or-remove precondition for the app strict step (PR B). +Done in the data step: `data/hook.zod.ts` + `data/hook-body.zod.ts` + +`data/datasource.zod.ts` — the last two entries that carried a provisional +`(p)` classification. Verification changed the answer for one of them: the +blanket `authorable (p)` on `hook.zod.ts` was too wide, because +`HookContextSchema` in the same file is a runtime shape and stays tolerant. +Both types were confirmed authorable the same way — they sit in +`BUILTIN_METADATA_TYPE_SCHEMAS`, so one shape backs `defineStack()` parsing, +`/api/v1/meta/types/:type`, and the Studio form. + +Measured while doing it, and worth recording because it contradicts the +assumption the earlier steps were written under: **strictness does not change +the published JSON Schema.** `build-schemas.ts` converts with the default +`io: 'output'`, and in output mode zod emits `additionalProperties: false` for +a `.strip()` object too — the post-parse shape genuinely has no extra keys. +Verified by regenerating both ways: `Datasource.json` is byte-identical before +and after. So the JSON Schema had been advertising `additionalProperties: false` +while the zod parse quietly accepted and dropped unknown keys. These flips align +the parse with the contract that was already published, rather than widening it. +(Note this cuts against the `approval.zod.ts` note above, which reads as though +the flip carried strictness INTO the JSON schema. It did not; approval's schema +would have said `false` regardless. The registration-time rejection it describes +is real, but it came from the published schema, not from the flip.) + Long tail stays gated on a verification pass per shape — never a one-shot "make all ~453 sites strict" (ADR-0054 ratchet; #4001's own recommendation). diff --git a/packages/spec/src/data/datasource.zod.ts b/packages/spec/src/data/datasource.zod.ts index f77ceee130..a5015edbc1 100644 --- a/packages/spec/src/data/datasource.zod.ts +++ b/packages/spec/src/data/datasource.zod.ts @@ -8,6 +8,261 @@ import { z } from 'zod'; * Can be a built-in driver or a plugin-contributed driver (e.g., "com.vendor.snowflake"). */ import { lazySchema } from '../shared/lazy-schema'; +import { strictUnknownKeyError } from '../shared/suggestions.zod'; + +/* + * ── Unknown-key strictness (#4001 data step) ──────────────────────────────── + * + * Every AUTHORING shape in this module is `.strict()`. `datasource` is a + * registered metadata type (BUILTIN_METADATA_TYPE_SCHEMAS), so one shape backs + * `defineDatasource()`, `defineStack({ datasources })`, the + * `/api/v1/meta/datasource` endpoint, and the Setup → Datasources form. + * + * TWO ESCAPE HATCHES STAY OPEN, and must: + * - `config` is per-driver by construction (a sqlite `filename` and a + * postgres `host`/`port` share no shape), so it stays `z.record`. The + * driver's own `configSchema` is what validates it. + * - `readReplicas` carries the same per-driver config objects. + * + * That openness is exactly why the TOP level had to close. Before this, a + * connection key written one level too high — `host` next to `driver` instead + * of inside `config` — was stripped in silence, and the datasource then + * connected on driver defaults (localhost, default port) rather than failing. + * A misplaced `password` is the same bug wearing a worse hat, which is why it + * is prescribed toward `external.credentialsRef` rather than merely relocated. + */ + +/** Keys {@link DriverDefinitionSchema} declares (drift-guarded by datasource.test.ts). */ +const DRIVER_DEFINITION_KEYS = ['id', 'label', 'description', 'icon', 'configSchema', 'capabilities'] as const; + +/** Keys {@link ExternalDatasourceSettingsSchema} declares (drift-guarded by datasource.test.ts). */ +const EXTERNAL_SETTINGS_KEYS = [ + 'label', 'allowedSchemas', 'allowWrites', 'validation', + 'credentialsRef', 'queryTimeoutMs', 'requirePermission', +] as const; + +/** Keys the external `validation` block declares (drift-guarded by datasource.test.ts). */ +const EXTERNAL_VALIDATION_KEYS = ['onMismatch', 'checkOnBoot', 'checkIntervalMs'] as const; + +/** Keys {@link DatasourceSchema} declares (drift-guarded by datasource.test.ts). */ +const DATASOURCE_KEYS = [ + 'name', 'label', 'driver', 'config', 'pool', 'readReplicas', 'capabilities', + 'healthCheck', 'ssl', 'retryPolicy', 'description', 'active', 'autoConnect', + 'schemaMode', 'external', 'origin', +] as const; + +/** Keys the datasource `pool` block declares (drift-guarded by datasource.test.ts). */ +const POOL_KEYS = ['min', 'max', 'idleTimeoutMillis', 'connectionTimeoutMillis'] as const; + +/** Keys the datasource `healthCheck` block declares (drift-guarded by datasource.test.ts). */ +const HEALTH_CHECK_KEYS = ['enabled', 'intervalMs', 'timeoutMs'] as const; + +/** Keys the datasource `ssl` block declares (drift-guarded by datasource.test.ts). */ +const SSL_KEYS = ['enabled', 'rejectUnauthorized', 'ca', 'cert', 'key'] as const; + +/** Keys the datasource `retryPolicy` block declares (drift-guarded by datasource.test.ts). */ +const DATASOURCE_RETRY_POLICY_KEYS = ['maxRetries', 'baseDelayMs', 'maxDelayMs', 'backoffMultiplier'] as const; + +/** A connection detail written one level too high — it belongs inside `config`. */ +const belongsInConfig = (key: string) => + `\`${key}\` is a driver connection detail — it belongs inside \`config\`, not at the top ` + + `level. Move it to \`config: { ${key}: … }\`; the driver's own configSchema validates it there.`; + +const driverDefinitionUnknownKeyError = strictUnknownKeyError({ + surface: 'this driver definition', + knownKeys: DRIVER_DEFINITION_KEYS, + aliases: { + name: 'id', + driver: 'id', + title: 'label', + config: 'configSchema', + schema: 'configSchema', + capability: 'capabilities', + }, + history: 'Until #4001 these were dropped silently — the driver still registered.', +}); + +const externalSettingsUnknownKeyError = strictUnknownKeyError({ + surface: "this datasource's external settings", + knownKeys: EXTERNAL_SETTINGS_KEYS, + aliases: { + schemas: 'allowedSchemas', + allowedschema: 'allowedSchemas', + writable: 'allowWrites', + allowwrite: 'allowWrites', + credentials: 'credentialsRef', + secretref: 'credentialsRef', + timeoutms: 'queryTimeoutMs', + querytimeout: 'queryTimeoutMs', + permission: 'requirePermission', + }, + guidance: { + password: + '`password` must never be inlined. Put the secret in the secrets store and reference ' + + 'it with `credentialsRef` (e.g. `credentialsRef: "secret:warehouse/password"`).', + readOnly: + '`readOnly` is not an external-settings key. Use `allowWrites: false` here for the ' + + 'datasource-wide gate, or `capabilities.readOnly` to describe the driver.', + }, + history: 'Until #4001 these were dropped silently — federation ran on the defaults instead.', +}); + +const externalValidationUnknownKeyError = strictUnknownKeyError({ + surface: "this datasource's external validation policy", + knownKeys: EXTERNAL_VALIDATION_KEYS, + aliases: { + onmismatch: 'onMismatch', + mismatch: 'onMismatch', + checkonboot: 'checkOnBoot', + validateonboot: 'checkOnBoot', + interval: 'checkIntervalMs', + checkinterval: 'checkIntervalMs', + }, + history: + 'Until #4001 these were dropped silently — drift checking ran on the defaults ' + + '(fail on mismatch, check at boot) regardless of what was written.', +}); + +const datasourceUnknownKeyError = strictUnknownKeyError({ + surface: 'this datasource', + knownKeys: DATASOURCE_KEYS, + aliases: { + type: 'driver', + connection: 'config', + connectionconfig: 'config', + options: 'config', + enabled: 'active', + pooling: 'pool', + replicas: 'readReplicas', + mode: 'schemaMode', + schema_mode: 'schemaMode', + federation: 'external', + retry: 'retryPolicy', + tls: 'ssl', + }, + guidance: { + host: belongsInConfig('host'), + port: belongsInConfig('port'), + database: belongsInConfig('database'), + user: belongsInConfig('user'), + username: belongsInConfig('username'), + filename: belongsInConfig('filename'), + url: belongsInConfig('url'), + connectionString: belongsInConfig('connectionString'), + password: + '`password` must never be inlined on a datasource. Interpolate it from the environment ' + + 'inside `config`, or for an external datasource reference the secrets store via ' + + '`external.credentialsRef`.', + }, + history: + 'Until #4001 these were dropped silently — a connection key written one level too high ' + + 'left the datasource connecting on driver defaults rather than failing.', +}); + +const poolUnknownKeyError = strictUnknownKeyError({ + surface: "this datasource's pool config", + knownKeys: POOL_KEYS, + aliases: { + minimum: 'min', + maximum: 'max', + minconnections: 'min', + maxconnections: 'max', + idletimeout: 'idleTimeoutMillis', + idletimeoutms: 'idleTimeoutMillis', + connectiontimeout: 'connectionTimeoutMillis', + connectiontimeoutms: 'connectionTimeoutMillis', + acquiretimeoutmillis: 'connectionTimeoutMillis', + }, + history: + 'Until #4001 these were dropped silently — the pool ran on its defaults (min 0, max 10) ' + + 'no matter what was written. Note both timeouts end in `Millis`, not `Ms`.', +}); + +const healthCheckUnknownKeyError = strictUnknownKeyError({ + surface: "this datasource's healthCheck config", + knownKeys: HEALTH_CHECK_KEYS, + aliases: { + active: 'enabled', + interval: 'intervalMs', + intervalmillis: 'intervalMs', + timeout: 'timeoutMs', + timeoutmillis: 'timeoutMs', + }, + history: 'Until #4001 these were dropped silently — health checks ran on the defaults.', +}); + +const sslUnknownKeyError = strictUnknownKeyError({ + surface: "this datasource's ssl config", + knownKeys: SSL_KEYS, + aliases: { + active: 'enabled', + ssl: 'enabled', + tls: 'enabled', + rejectunauthorised: 'rejectUnauthorized', + cacert: 'ca', + certificate: 'cert', + clientcert: 'cert', + privatekey: 'key', + clientkey: 'key', + }, + guidance: { + insecure: + '`insecure` is not an ssl key. To accept a self-signed certificate set ' + + '`rejectUnauthorized: false` — deliberately, and never against a production database.', + }, + history: + 'Until #4001 these were dropped silently — which meant a TLS setting that never took ' + + 'effect looked identical to one that did.', +}); + +const datasourceRetryPolicyUnknownKeyError = strictUnknownKeyError({ + surface: "this datasource's retryPolicy", + knownKeys: DATASOURCE_RETRY_POLICY_KEYS, + aliases: { + retries: 'maxRetries', + attempts: 'maxRetries', + backoffms: 'baseDelayMs', + basedelay: 'baseDelayMs', + delayms: 'baseDelayMs', + maxdelay: 'maxDelayMs', + multiplier: 'backoffMultiplier', + backoff: 'backoffMultiplier', + }, + history: + 'Until #4001 these were dropped silently — reconnects ran on the defaults. Note a hook ' + + 'retryPolicy spells its delay `backoffMs`; a datasource spells it `baseDelayMs`.', +}); + +const capabilitiesUnknownKeyError = strictUnknownKeyError({ + surface: 'these datasource capabilities', + knownKeys: [ + 'transactions', 'queryFilters', 'queryAggregations', 'querySorting', + 'queryPagination', 'queryWindowFunctions', 'querySubqueries', 'joins', + 'fullTextSearch', 'readOnly', 'dynamicSchema', + ], + aliases: { + transaction: 'transactions', + filters: 'queryFilters', + filtering: 'queryFilters', + aggregations: 'queryAggregations', + aggregation: 'queryAggregations', + sorting: 'querySorting', + sort: 'querySorting', + pagination: 'queryPagination', + windowfunctions: 'queryWindowFunctions', + subqueries: 'querySubqueries', + join: 'joins', + fulltext: 'fullTextSearch', + search: 'fullTextSearch', + readonly: 'readOnly', + schemaless: 'dynamicSchema', + }, + history: + 'Until #4001 these were dropped silently — and a capability that fails to register ' + + 'reads as FALSE, so the engine quietly stopped pushing that work down to the driver ' + + 'and recomputed it in memory instead.', +}); + export const DriverType = z.string().describe('Underlying driver identifier'); /** @@ -33,7 +288,7 @@ export const DriverDefinitionSchema = lazySchema(() => z.object({ * What this driver supports out-of-the-box. */ capabilities: z.lazy(() => DatasourceCapabilities).optional(), -})); +}, { error: driverDefinitionUnknownKeyError }).strict()); /** * Datasource Capabilities Schema @@ -86,7 +341,7 @@ export const DatasourceCapabilities = z.object({ /** Is scheme-less (needs schema inference)? */ dynamicSchema: z.boolean().default(false), -}); +}, { error: capabilitiesUnknownKeyError }).strict(); /** * Schema Ownership Mode (ADR-0015) @@ -126,14 +381,16 @@ export const ExternalDatasourceSettingsSchema = z.object({ .describe('Validate federated objects against the remote schema at boot.'), checkIntervalMs: z.number().optional() .describe('Optional background drift-check interval in milliseconds.'), - }).default({ onMismatch: 'fail', checkOnBoot: true }).describe('Boot/drift validation policy'), + }, { error: externalValidationUnknownKeyError }).strict() + .default({ onMismatch: 'fail', checkOnBoot: true }).describe('Boot/drift validation policy'), credentialsRef: z.string().optional() .describe('Reference into the secrets store; never inline credentials.'), queryTimeoutMs: z.number().default(30_000) .describe('Hard cap on per-query execution time.'), requirePermission: z.string().optional() .describe('Optional convenience: gate the entire datasource behind a single role.'), -}).describe('External datasource federation settings (schemaMode != "managed")'); +}, { error: externalSettingsUnknownKeyError }).strict() + .describe('External datasource federation settings (schemaMode != "managed")'); export type ExternalDatasourceSettings = z.infer; @@ -167,7 +424,7 @@ export const DatasourceSchema = lazySchema(() => z.object({ max: z.number().default(10).describe('Maximum connections'), idleTimeoutMillis: z.number().default(30000).describe('Idle timeout'), connectionTimeoutMillis: z.number().default(3000).describe('Connection establishment timeout'), - }).optional().describe('Connection pool settings'), + }, { error: poolUnknownKeyError }).strict().optional().describe('Connection pool settings'), /** * Read Replicas @@ -187,7 +444,7 @@ export const DatasourceSchema = lazySchema(() => z.object({ enabled: z.boolean().default(true).describe('Enable health check endpoint'), intervalMs: z.number().default(30000).describe('Health check interval in milliseconds'), timeoutMs: z.number().default(5000).describe('Health check timeout in milliseconds'), - }).optional().describe('Datasource health check configuration'), + }, { error: healthCheckUnknownKeyError }).strict().optional().describe('Datasource health check configuration'), /** SSL/TLS Configuration */ ssl: z.object({ @@ -196,7 +453,7 @@ export const DatasourceSchema = lazySchema(() => z.object({ ca: z.string().optional().describe('CA certificate (PEM format or path to file)'), cert: z.string().optional().describe('Client certificate (PEM format or path to file)'), key: z.string().optional().describe('Client private key (PEM format or path to file)'), - }).optional().describe('SSL/TLS configuration for secure database connections'), + }, { error: sslUnknownKeyError }).strict().optional().describe('SSL/TLS configuration for secure database connections'), /** Retry Policy */ retryPolicy: z.object({ @@ -204,7 +461,7 @@ export const DatasourceSchema = lazySchema(() => z.object({ baseDelayMs: z.number().default(1000).describe('Base delay between retries in milliseconds'), maxDelayMs: z.number().default(30000).describe('Maximum delay between retries in milliseconds'), backoffMultiplier: z.number().default(2).describe('Exponential backoff multiplier'), - }).optional().describe('Connection retry policy for transient failures'), + }, { error: datasourceRetryPolicyUnknownKeyError }).strict().optional().describe('Connection retry policy for transient failures'), /** Description */ description: z.string().optional().describe('Internal description'), @@ -251,7 +508,7 @@ export const DatasourceSchema = lazySchema(() => z.object({ */ origin: z.enum(['code', 'runtime']).default('code') .describe('Datasource provenance (server-managed, read-only)'), -}).superRefine((ds, ctx) => { +}, { error: datasourceUnknownKeyError }).strict().superRefine((ds, ctx) => { if (ds.schemaMode !== 'managed' && !ds.external) { ctx.addIssue({ code: 'custom', diff --git a/packages/spec/src/data/hook-body.zod.ts b/packages/spec/src/data/hook-body.zod.ts index 9374851319..80c350bb64 100644 --- a/packages/spec/src/data/hook-body.zod.ts +++ b/packages/spec/src/data/hook-body.zod.ts @@ -1,6 +1,69 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { z } from 'zod'; +import { strictUnknownKeyError } from '../shared/suggestions.zod'; + +/* + * ── Unknown-key strictness (#4001 data step) ──────────────────────────────── + * + * Both body shapes are `.strict()`. A hook body is the one place where a + * silently-stripped key is worst-understood by the author: the body still + * parses, still runs, and the thing they configured simply is not in effect. + * + * The two concrete traps this closes: + * - `capabilities` misspelt on an L2 body strips to the `[]` default, and the + * sandbox then throws at INVOCATION time on the first `ctx.api` call — far + * from the typo, and only on the code path that happens to use it. + * - `timeoutMs` / `memoryMb` misspelt strip to undefined, so the body runs + * under the enclosing hook's limits instead of the tighter ones the author + * wrote. Nothing reports the downgrade. + * + * The L1 schema additionally PRESCRIBES the L2-only keys rather than guessing a + * rename: `capabilities` on an `expression` body is not a typo, it is a + * misunderstanding of which level owns the key. + */ + +/** Keys {@link ExpressionBodySchema} declares (drift-guarded by hook-body.test.ts). */ +const EXPRESSION_BODY_KEYS = ['language', 'source'] as const; + +/** Keys {@link ScriptBodySchema} declares (drift-guarded by hook-body.test.ts). */ +const SCRIPT_BODY_KEYS = ['language', 'source', 'capabilities', 'timeoutMs', 'memoryMb'] as const; + +const L2_ONLY_ON_L1 = + 'is an L2 key — it only applies to `language: "js"`. An expression body is a pure ' + + 'formula: it performs no IO, so it has nothing to grant and no sandbox to bound.'; + +const expressionBodyUnknownKeyError = strictUnknownKeyError({ + surface: 'this expression (L1) hook body', + knownKeys: EXPRESSION_BODY_KEYS, + aliases: { expression: 'source', formula: 'source', code: 'source', script: 'source' }, + guidance: { + capabilities: `\`capabilities\` ${L2_ONLY_ON_L1}`, + timeoutMs: `\`timeoutMs\` ${L2_ONLY_ON_L1}`, + memoryMb: `\`memoryMb\` ${L2_ONLY_ON_L1}`, + }, + history: 'Until #4001 these were dropped silently.', +}); + +const scriptBodyUnknownKeyError = strictUnknownKeyError({ + surface: 'this sandboxed JS (L2) hook body', + knownKeys: SCRIPT_BODY_KEYS, + aliases: { + capability: 'capabilities', + caps: 'capabilities', + permissions: 'capabilities', + timeout: 'timeoutMs', + timeoutms: 'timeoutMs', + memory: 'memoryMb', + memorymb: 'memoryMb', + code: 'source', + script: 'source', + body: 'source', + }, + history: + 'Until #4001 these were dropped silently — the body still ran, just not under the ' + + 'limits or grants that were written.', +}); /** * Capability tokens a script body may request. @@ -46,7 +109,7 @@ export const ExpressionBodySchema = z.object({ language: z.literal('expression'), /** Formula-engine expression. Pure, side-effect-free. */ source: z.string().min(1).describe('Formula expression source'), -}).describe('L1 expression body — pure formula, no IO'); +}, { error: expressionBodyUnknownKeyError }).strict().describe('L1 expression body — pure formula, no IO'); export type ExpressionBody = z.infer; /** @@ -100,7 +163,7 @@ export const ScriptBodySchema = z.object({ * Subject to engine support (isolated-vm enforces, quickjs approximates). */ memoryMb: z.number().int().positive().max(256).optional().describe('Per-invocation memory cap (MB)'), -}).describe('L2 sandboxed JS body — runs inside an isolated VM with declared capabilities'); +}, { error: scriptBodyUnknownKeyError }).strict().describe('L2 sandboxed JS body — runs inside an isolated VM with declared capabilities'); export type ScriptBody = z.infer; /** diff --git a/packages/spec/src/data/hook.zod.ts b/packages/spec/src/data/hook.zod.ts index ca29b6bef6..efd35b9317 100644 --- a/packages/spec/src/data/hook.zod.ts +++ b/packages/spec/src/data/hook.zod.ts @@ -8,7 +8,88 @@ import { ExpressionInputSchema } from '../shared/expression.zod'; * Defines the interception points in the ObjectQL execution pipeline. */ import { lazySchema } from '../shared/lazy-schema'; +import { strictUnknownKeyError } from '../shared/suggestions.zod'; import { HookBodySchema } from './hook-body.zod'; + +/* + * ── Unknown-key strictness (#4001 data step) ──────────────────────────────── + * + * {@link HookSchema} (the AUTHORING shape) and its `retryPolicy` block are + * `.strict()`. `hook` is a registered metadata type — it sits in + * BUILTIN_METADATA_TYPE_SCHEMAS, so the same shape backs `defineStack({ hooks })` + * parsing, the `/api/v1/meta/types/hook` endpoint, and the Studio form. A key it + * silently dropped was invisible on every one of those surfaces. + * + * {@link HookContextSchema} is deliberately NOT strict, and must not become so. + * It is the RUNTIME shape the engine hands to a handler — nobody authors it. + * Strictness there would invert the contract: adding a field to the context + * (as `provenance` was in #3712) would turn an engine-internal enrichment into + * a breaking change for anyone parsing a context they were handed. Same + * reasoning as `RLSUserContextSchema` / `FlowVersionHistorySchema`. + * + * Two near-miss key names, both now aliased because they are genuinely easy to + * cross: + * - hook-level `timeout` vs body-level `timeoutMs` (see hook-body.zod.ts) + * - hook `retryPolicy.backoffMs` vs datasource `retryPolicy.baseDelayMs` + */ + +/** Keys {@link HookSchema} declares (drift-guarded by hook.test.ts). */ +const HOOK_KEYS = [ + 'name', 'label', 'object', 'events', 'handler', 'body', 'priority', + 'async', 'condition', 'description', 'retryPolicy', 'timeout', 'onError', +] as const; + +/** Keys the hook `retryPolicy` block declares (drift-guarded by hook.test.ts). */ +const HOOK_RETRY_POLICY_KEYS = ['maxRetries', 'backoffMs'] as const; + +const hookUnknownKeyError = strictUnknownKeyError({ + surface: 'this hook', + knownKeys: HOOK_KEYS, + aliases: { + hookname: 'name', + objectname: 'object', + objects: 'object', + event: 'events', + fn: 'handler', + callback: 'handler', + order: 'priority', + sequence: 'priority', + background: 'async', + isasync: 'async', + when: 'condition', + predicate: 'condition', + retry: 'retryPolicy', + timeoutms: 'timeout', + errorpolicy: 'onError', + onfailure: 'onError', + }, + guidance: { + enabled: + '`enabled` is not a hook key — a hook has no on/off switch. Gate it with `condition` ' + + '(the hook is skipped when the predicate is false), or remove the hook.', + active: + '`active` is not a hook key — a hook has no on/off switch. Gate it with `condition`, ' + + 'or remove the hook.', + }, + history: 'Until #4001 these were dropped silently — the hook still registered and ran.', +}); + +const hookRetryPolicyUnknownKeyError = strictUnknownKeyError({ + surface: "this hook's retryPolicy", + knownKeys: HOOK_RETRY_POLICY_KEYS, + aliases: { + retries: 'maxRetries', + attempts: 'maxRetries', + basedelayms: 'backoffMs', + backoff: 'backoffMs', + delayms: 'backoffMs', + }, + history: + 'Until #4001 these were dropped silently — the hook retried on the defaults rather ' + + 'than the policy that was written. Note a datasource retryPolicy spells its delay ' + + '`baseDelayMs`; a hook spells it `backoffMs`.', +}); + export const HookEvent = z.enum([ // Read — one event per read, regardless of shape. `beforeFind`/`afterFind` // fire for BOTH `find` and `findOne` (the event attaches to record @@ -141,7 +222,7 @@ export const HookSchema = lazySchema(() => z.object({ retryPolicy: z.object({ maxRetries: z.number().default(3).describe('Maximum retry attempts on failure'), backoffMs: z.number().default(1000).describe('Backoff delay between retries in milliseconds'), - }).optional().describe('Retry policy for failed hook executions'), + }, { error: hookRetryPolicyUnknownKeyError }).strict().optional().describe('Retry policy for failed hook executions'), /** * Execution Timeout @@ -155,7 +236,7 @@ export const HookSchema = lazySchema(() => z.object({ * - log: Log error and continue */ onError: z.enum(['abort', 'log']).default('abort').describe('Error handling strategy'), -})); +}, { error: hookUnknownKeyError }).strict()); /** * Hook Runtime Context diff --git a/packages/spec/src/kernel/metadata-authoring-lint.test.ts b/packages/spec/src/kernel/metadata-authoring-lint.test.ts index ecadf24256..4b7033f490 100644 --- a/packages/spec/src/kernel/metadata-authoring-lint.test.ts +++ b/packages/spec/src/kernel/metadata-authoring-lint.test.ts @@ -27,11 +27,11 @@ describe('coverage derivation (#3786 — no third hand-written list)', () => { // #4148 covered object+field: 2 surfaces. The point of this walker is the // rest. If the derivation regresses to a handful, the "evidence base" for // the #4001 strict tiers quietly becomes a sample again. - expect(lintables.length).toBeGreaterThanOrEqual(15); + expect(lintables.length).toBeGreaterThanOrEqual(14); // `view` matters doubly: it is a UNION (container | ViewItem | overlay), so // its presence pins the union half of the posture logic — a regression that // silently dropped unions would shrink coverage without failing the count. - for (const expected of ['object', 'page', 'agent', 'dashboard', 'action', 'report', 'hook', 'view']) { + for (const expected of ['object', 'page', 'agent', 'dashboard', 'action', 'report', 'view']) { expect(lintableTypes, `expected '${expected}' to be lint-covered`).toContain(expected); } }); @@ -42,7 +42,11 @@ describe('coverage derivation (#3786 — no third hand-written list)', () => { // `app` graduated mid-flight (#4165) while this very test was in review: // the derivation adapted on its own, and the pinned expectation above is // what forced a human to confirm the shrink was a graduation, not a bug. - for (const strict of ['flow', 'permission', 'position', 'tool', 'app']) { + // It did that job again for `hook` + `datasource` (#4001 data step): the + // count fell 16 → 14 and the pin above failed until both were confirmed + // graduations and moved into this list. `hook` also had to leave the + // pinned-coverage list above, where it had been an expected lint target. + for (const strict of ['flow', 'permission', 'position', 'tool', 'app', 'hook', 'datasource']) { expect(lintableTypes, `'${strict}' is .strict(); the lint must not double-report`).not.toContain(strict); } }); From 28eb299263c4900cf9675522b8bda6e2b58380d9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 15:23:34 +0000 Subject: [PATCH 2/2] fix(spec): keep the hook-body capability table out of the strictness block's way MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check:docs failed on the previous commit (inside the "TypeScript Type Check" job — tsc itself was clean). The regenerated reference page had REPLACED the whole capability-token table in content/docs/references/data/hook-body.mdx with one line: "Keys `ExpressionBodySchema` declares (drift-guarded by ...)". build-docs.ts takes the FIRST JSDoc in a module as that page's blurb. Putting the strictness constants — which carry `/** … */` per the approval.zod.ts house style — above `HookBodyCapability` handed its documentation slot to a private constant, silently deleting the `api.read` / `api.write` / `api.transaction` / `crypto.*` / `log` reference table authors actually read. Moved the block below `HookBodyCapability` and left a placement note so the next person adding a documented constant to this file does not rediscover it. Regenerating now leaves content/docs/references untouched; check:docs passes. The other two files were never at risk — their first JSDoc sits above the imports, so the inserted block could not take the slot. That is also why only this one page moved. Verified: check:docs green, full spec suite 7136 green. Refs #4001 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0147tNF4Snk7Ry1KGt4a5PY4 --- packages/spec/src/data/hook-body.zod.ts | 64 ++++++++++++++----------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/packages/spec/src/data/hook-body.zod.ts b/packages/spec/src/data/hook-body.zod.ts index 80c350bb64..37a1d3bab4 100644 --- a/packages/spec/src/data/hook-body.zod.ts +++ b/packages/spec/src/data/hook-body.zod.ts @@ -3,6 +3,35 @@ import { z } from 'zod'; import { strictUnknownKeyError } from '../shared/suggestions.zod'; +/** + * Capability tokens a script body may request. + * + * The runtime sandbox enforces these — if a body uses a `ctx` API that requires + * a capability it did not declare, the call throws at invocation time. + * + * - `api.read` — `ctx.api.object(...).find / findOne / count / aggregate` + * - `api.write` — `ctx.api.object(...).insert / update / delete` + * - `api.transaction` — `ctx.api.transaction(async () => { … })` — runs the + * callback's `ctx.api` writes/reads inside one driver transaction, committed + * on return and rolled back if the callback throws. Requires `api.write` + * alongside it to be useful (the transaction body still needs write access). + * - `crypto.uuid` — `ctx.crypto.randomUUID()` + * - `crypto.hash` — `ctx.crypto.hash(algo, data)` + * - `log` — `ctx.log.info / warn / error` + * + * `http.fetch` is intentionally absent — outbound calls go through Connector + * recipes (separate spec) so they remain auditable and replayable. + */ +export const HookBodyCapability = z.enum([ + 'api.read', + 'api.write', + 'api.transaction', + 'crypto.uuid', + 'crypto.hash', + 'log', +]); +export type HookBodyCapability = z.infer; + /* * ── Unknown-key strictness (#4001 data step) ──────────────────────────────── * @@ -21,6 +50,12 @@ import { strictUnknownKeyError } from '../shared/suggestions.zod'; * The L1 schema additionally PRESCRIBES the L2-only keys rather than guessing a * rename: `capabilities` on an `expression` body is not a typo, it is a * misunderstanding of which level owns the key. + * + * Placement note: this block sits AFTER `HookBodyCapability` on purpose. + * build-docs.ts takes the file's FIRST JSDoc as the reference page's module + * blurb, so a `/** … *\/` comment above the capability enum silently replaced + * the whole capability-token table in content/docs/references/data/hook-body.mdx. + * Keep declarations that carry JSDoc below the first exported symbol here. */ /** Keys {@link ExpressionBodySchema} declares (drift-guarded by hook-body.test.ts). */ @@ -65,35 +100,6 @@ const scriptBodyUnknownKeyError = strictUnknownKeyError({ + 'limits or grants that were written.', }); -/** - * Capability tokens a script body may request. - * - * The runtime sandbox enforces these — if a body uses a `ctx` API that requires - * a capability it did not declare, the call throws at invocation time. - * - * - `api.read` — `ctx.api.object(...).find / findOne / count / aggregate` - * - `api.write` — `ctx.api.object(...).insert / update / delete` - * - `api.transaction` — `ctx.api.transaction(async () => { … })` — runs the - * callback's `ctx.api` writes/reads inside one driver transaction, committed - * on return and rolled back if the callback throws. Requires `api.write` - * alongside it to be useful (the transaction body still needs write access). - * - `crypto.uuid` — `ctx.crypto.randomUUID()` - * - `crypto.hash` — `ctx.crypto.hash(algo, data)` - * - `log` — `ctx.log.info / warn / error` - * - * `http.fetch` is intentionally absent — outbound calls go through Connector - * recipes (separate spec) so they remain auditable and replayable. - */ -export const HookBodyCapability = z.enum([ - 'api.read', - 'api.write', - 'api.transaction', - 'crypto.uuid', - 'crypto.hash', - 'log', -]); -export type HookBodyCapability = z.infer; - /** * L1 — Pure expression body. *