From 62717f0263ba57308656ab989710eec625bf36e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 07:59:18 +0000 Subject: [PATCH 1/2] feat(spec)!: reject unknown keys on flow and permission authoring schemas (#4001 Tier-A) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zod's default .strip silently discarded undeclared keys on authorable schemas — the instance kept parsing, so a mis-spelled or wrong-layer key shipped as metadata that quietly ignored the author's config (#3405, #1535). This lands the #4001 Tier-A slice inside the v17 breaking window: - security/permission.zod.ts: PermissionSetSchema, ObjectPermissionSchema, FieldPermissionSchema, AdminScopeSchema are .strict() with fixable errors (semantic aliases, wrong-layer pointers, tombstones for the retired contextVariables / isProfile). EffectiveObjectPermissionSchema explicitly .strip()s back — response shapes stay wire-tolerant. - automation/flow.zod.ts: FlowSchema, FlowNodeSchema, FlowEdgeSchema, FlowVariableSchema are .strict(); a node's config record stays open (the executor's configSchema, #4027/#4040, and the ADR-0087 conversion layer own it). - shared/suggestions.zod.ts: new strictUnknownKeyError factory generalizing the #3746 hand-rolled map; ui/action.zod.ts re-homes onto it with byte-identical messages. - PermissionSetSchema gains the declared `description` key — the strict gate's first catch: the built-in default sets author it and the Setup projection reads it, but the schema could not represent it, so it was silently stripped at every parse (ADR-0078 §3 inverse drift). - docs/audits/2026-07-unknown-key-strictness-ledger.md: the authorable / wire / open triage (#4001 step 1) with the ratchet's next targets. Verified zero breakage: spec 6960 tests + tsc clean; lint / metadata / platform-objects / metadata-core / metadata-protocol / sdui-parser / service-automation / plugin-security / core / rest / objectql / runtime suites green; showcase / crm / todo `objectstack validate` clean; all 12 spec check gates pass; no doc teaches a now-rejected key. Refs #4001 (Tier-A slice: ledger + first ratchet step; the long tail stays verify-then-enforce per the ledger). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0147tNF4Snk7Ry1KGt4a5PY4 --- .changeset/unknown-key-strictness-tier-a.md | 51 +++++ .../docs/references/security/permission.mdx | 1 + content/docs/releases/v17.mdx | 36 ++++ .../2026-07-unknown-key-strictness-ledger.md | 180 +++++++++++++++++ packages/spec/api-surface.json | 2 + packages/spec/authorable-surface.json | 1 + packages/spec/liveness/permission.json | 6 + packages/spec/src/automation/flow.test.ts | 127 ++++++++++++ packages/spec/src/automation/flow.zod.ts | 125 +++++++++++- packages/spec/src/security/permission.test.ts | 161 ++++++++++++++- packages/spec/src/security/permission.zod.ts | 183 +++++++++++++++++- packages/spec/src/shared/suggestions.zod.ts | 84 ++++++++ packages/spec/src/ui/action.zod.ts | 45 ++--- .../references/_index.md | 3 + 14 files changed, 964 insertions(+), 41 deletions(-) create mode 100644 .changeset/unknown-key-strictness-tier-a.md create mode 100644 docs/audits/2026-07-unknown-key-strictness-ledger.md diff --git a/.changeset/unknown-key-strictness-tier-a.md b/.changeset/unknown-key-strictness-tier-a.md new file mode 100644 index 0000000000..404728a018 --- /dev/null +++ b/.changeset/unknown-key-strictness-tier-a.md @@ -0,0 +1,51 @@ +--- +"@objectstack/spec": major +--- + +feat(spec)!: reject unknown keys on the flow and permission authoring schemas (#4001 Tier-A) + +Zod's default `.strip` silently discarded any key these schemas did not +declare — the instance kept parsing, so a mis-spelled or wrong-layer key +shipped as metadata that quietly ignored the author's config (#3405's +action-param `reference`, #1535's object-level `workflows`). #3746 tightened +one schema; this extends the same treatment to the two highest-risk +authorable surfaces, per the ADR-0054 ratchet and the +`docs/audits/2026-07-unknown-key-strictness-ledger.md` triage: + +- **`security/permission.zod.ts`** — `PermissionSetSchema`, + `ObjectPermissionSchema`, `FieldPermissionSchema`, `AdminScopeSchema` are + now `.strict()`. A silently dropped key on the capability container meant + the author believed a grant or restriction was in place that the runtime + never saw. `EffectiveObjectPermissionSchema` (response-side) explicitly + `.strip()`s back and stays wire-tolerant. +- **`automation/flow.zod.ts`** — `FlowSchema`, `FlowNodeSchema`, + `FlowEdgeSchema`, `FlowVariableSchema` are now `.strict()`. A node's + `config` record stays **open**: it is per-node-type, owned by the + registered executor's `configSchema` (#4027/#4040) and the ADR-0087 + conversion layer. +- **`shared/suggestions.zod.ts`** — new `strictUnknownKeyError` factory (the + #3746 hand-rolled map, generalized): every rejection names the offending + key(s) and, where recognisable, the canonical spelling or a retired-key + tombstone. `ui/action.zod.ts` re-homes onto it with byte-identical messages. +- **`PermissionSetSchema.description` is now a declared key.** The strict + gate's first catch: the built-in default sets always authored it and the + Setup projection reads it, but the schema could not represent it, so it was + silently stripped at every parse (ADR-0078 §3 inverse drift). + +**Migration.** Any key these schemas now reject was previously stripped and +therefore had **no runtime effect** — removing or renaming it never changes +the behavior of a working app; validation simply stops lying about it. The +error message carries the fix; the FROM → TO mappings baked into it include: + +- Permission set: `objectPermissions`→`objects`, `fieldPermissions`/`fls`→`fields`, + `tabs`→`tabPermissions`, `rls`/`policies`→`rowLevelSecurity`; + `read`/`edit`/`export`/…→`allowRead`/`allowEdit`/`allowExport`/…; + `readable`/`editable` vocabulary for FLS (`hidden` → declare `readable: false`). + Retired keys carry tombstones: `contextVariables` (ADR-0105 D11 — use a + registered `rlsMembership` resolver or an inline literal), `isProfile` + (ADR-0090 D2 — use `isDefault`). +- Flow: `steps`→`nodes`, `connections`/`transitions`/`links`→`edges`, + `trigger`/`triggerType`→`type`, `title`→`label`; edge `from`/`to`→`source`/`target`, + `guard`/`when`/`expression`→`condition`; a top-level `object`/`objectName`/`schedule` + belongs on the START node's `config` (`{ objectName, triggerType, condition, + schedule }`), not on the flow. diff --git a/content/docs/references/security/permission.mdx b/content/docs/references/security/permission.mdx index 3b63556a60..d8a879d32f 100644 --- a/content/docs/references/security/permission.mdx +++ b/content/docs/references/security/permission.mdx @@ -127,6 +127,7 @@ const result = AdminScope.parse(data); | :--- | :--- | :--- | :--- | | **name** | `string` | ✅ | Permission set unique name (lowercase snake_case) | | **label** | `string` | optional | Display label | +| **description** | `string` | optional | Human-readable description shown in Setup (persisted as sys_permission_set.description) | | **packageId** | `string` | optional | [ADR-0086 D3] Owning package id for a package-shipped set (absent = env-authored) | | **managedBy** | `Enum<'package' \| 'platform' \| 'user'>` | optional | [ADR-0086 D3] Record provenance: package (upgrade-owned metadata) vs platform/user (env config) | | **isDefault** | `boolean` | ✅ | [ADR-0090 D5] App baseline for the everyone position: app-level sets are auto-bound at boot (guarded, idempotent); package-level sets become install-time suggestions an admin confirms | diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index 1b6355f563..8bc18b29e3 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -304,6 +304,42 @@ model), while `url` platform-wide means an HTTP endpoint to call (`http` node, webhooks). The singular `input` on `map` / `subflow` / `connector_action` is those nodes' own canonical key and is untouched. +### Flow and permission schemas reject unknown keys (#4001) + +Zod's default is `.strip`: a key a schema does not declare is silently +discarded and the instance keeps parsing. On an authorable surface that is the +worst failure mode — the author (human or AI) gets a success envelope and +ships metadata that quietly ignores their config; that is exactly how a +correctly intended action-param `reference: 'sys_user'` degraded into a +paste-a-UUID text box (#3405). 17.0 extends #3746's strictness from that one +schema to the two highest-risk authorable surfaces, per the triage in +`docs/audits/2026-07-unknown-key-strictness-ledger.md`: + +- **Permission sets** — `PermissionSetSchema`, `ObjectPermissionSchema`, + `FieldPermissionSchema`, `AdminScopeSchema`. A silently dropped key on the + capability container meant the author believed a grant or restriction was in + place that the runtime never saw. The response-side + `EffectiveObjectPermissionSchema` stays wire-tolerant. +- **Flows** — `FlowSchema`, `FlowNodeSchema`, `FlowEdgeSchema`, + `FlowVariableSchema`. A node's `config` stays an **open** record: it is + per-node-type, owned by the executor's `configSchema` and the conversion + layer (see the alias table above). + +Every rejection is written to be self-fixing: it names the offending key and, +where recognisable, the canonical spelling (`steps` → `nodes`, edge +`from`/`to` → `source`/`target`, `read` → `allowRead`, `tabs` → +`tabPermissions`), a wrong-layer pointer (a flow-level `objectName` belongs on +the start node's `config`; `apiOperations` is response-side only), or a +retired-key tombstone (`contextVariables` → ADR-0105 D11's `rlsMembership` +resolvers; `isProfile` → `isDefault`, ADR-0090 D2). + +Migration is by construction behavior-preserving: any key now rejected was +previously stripped, so it never had a runtime effect — remove it or rename it +to the canonical key the error suggests; nothing about a working app changes. +One gap the gate itself caught: `PermissionSetSchema` gains the previously +unrepresentable-but-consumed **`description`** key (persisted to +`sys_permission_set.description`, shown in Setup). + ### `agent.tools[]` is removed — capability comes from skills (ADR-0109, #3820) ADR-0064's invariant is "an agent's tool set is the union of its diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md new file mode 100644 index 0000000000..1060d6e40b --- /dev/null +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -0,0 +1,180 @@ +# Unknown-key strictness ledger — the authorable / wire / open triage (#4001) + +**Date**: 2026-07-30 · **Issue**: [#4001](https://github.com/objectstack-ai/objectstack/issues/4001) · **Builds on**: #3405 / #3746 (the `ActionParamSchema` template), #1535 (`ObjectSchema.create` unknown-key error), ADR-0089 D3a (view/page strict), ADR-0078 (no-silently-inert-metadata), ADR-0049 (enforce-or-remove), ADR-0054 (ratchet idiom) + +Zod's default is `.strip`: a key the schema does not declare is **silently +discarded** and the instance goes on parsing. On an authorable surface that is +the worst failure mode — the author (human or AI) gets a success envelope and +ships metadata that quietly ignores their config. This ledger is #4001 step 1: +the persisted classification of *which* `z.object` sites that default is wrong +for, so the `.strict()` ratchet advances on evidence instead of a one-shot +sweep. **1885 sites was never the target number** — a large share of the spec +is wire/response shape where strictness would be a forward-compat bug. + +## Classification rule + +One question decides the class: **who writes this schema's input?** + +| Class | Input written by | Unknown-key policy | +|---|---|---| +| **authorable** | A human or AI author, into `*.object.ts` / `defineStack` config / Studio / MCP | `.strict()` + fixable error (the ratchet target) | +| **wire** | Another machine: server responses, connector payloads, runtime envelopes, persisted runtime state | stay tolerant (`.strip` / `.passthrough`); strictness here turns an upstream *addition* into our parse crash | +| **open** | Deliberately schemaless user data (record bodies, per-node-type `config`, React props) | stay open; a *sibling* contract validates it (e.g. a node executor's `configSchema`, #4027/#4040) | + +Mixed files carry both — classify per schema, not per file. A **response-side +extension of an authoring schema** (e.g. `EffectiveObjectPermissionSchema`) +must explicitly `.strip()` back, because `.extend()` inherits `.strict()`. + +## Standard wiring + +`strictUnknownKeyError` in `shared/suggestions.zod.ts` (generalized from the +#3746 hand-rolled map) is the one factory every strict authoring schema wires: + +```ts +z.object({ ... }, { error: strictUnknownKeyError({ surface, knownKeys, aliases, guidance, history }) }).strict() +``` + +- `aliases` — semantic near-misses edit distance cannot reach (`visibleWhen` → + `visible`, `from` → `source`, `read` → `allowRead`). +- `guidance` — exact-key prescriptions: **tombstones for retired keys** (the + rejection carries the upgrade — AGENTS.md Post-Task Checklist #3) and + wrong-layer pointers (`apiOperations` is response-side; `objectName` belongs + on the start node). +- Key lists live beside the schema and are **drift-guarded by tests** (an + "accepts every declared key" probe), because the schema body is lazy. + +Every ratchet step ships only with the empirical zero-breakage pass: full +`@objectstack/spec` suite + `tsc`, downstream consumer suites, and +`objectstack validate` on all three example apps. Inference is not evidence — +the first application of this gate (below) caught a real spec gap. + +## Ratchet state + +### Already strict before #4001 (for reference) + +| Surface | Since | +|---|---| +| `data/object.zod.ts` — `ObjectSchema.create()` unknown-key error + `UNKNOWN_KEY_GUIDANCE` tombstones; strict capabilities / tenancy / CRUD-override blocks | #1535, #2377, #2763 | +| `ui/view.zod.ts` / `ui/page.zod.ts` — form/page component schemas | ADR-0089 D3a | +| `ui/dashboard.zod.ts`, `data/field.zod.ts` (blocks), `ai/agent.zod.ts`, `ai/tool.zod.ts` (blocks) | various | +| `ui/action.zod.ts` — `ActionParamSchema` (the template) | #3405 / #3746 | + +### This step (#4001 Tier-A slice) + +| Schema | File | Evidence class | +|---|---|---| +| `PermissionSetSchema`, `ObjectPermissionSchema`, `FieldPermissionSchema`, `AdminScopeSchema` | `security/permission.zod.ts` | A silently dropped key on the capability container is the ADR-0049 asymmetry itself; retired `contextVariables` (ADR-0105 D11) / `isProfile` (ADR-0090 D2) get tombstones | +| `FlowSchema`, `FlowNodeSchema`, `FlowEdgeSchema`, `FlowVariableSchema` | `automation/flow.zod.ts` | Most AI-authored surface; cloud#688 / #2419 class. Node `config` stays **open** (executor `configSchema` owns it, #4027/#4040) | +| `ActionParamSchema` re-homed onto the shared factory | `ui/action.zod.ts` | #3746 template, byte-identical messages | + +**Findings log** (what the gate caught the day it was turned on): +`PermissionSetSchema` could not represent `description` — yet the built-in +default sets author it and `plugin-security`'s Setup projection reads it +(`permission-set-projection.ts`). It had been silently stripped at every parse +(the ADR-0078 §3 inverse-drift class). Fixed contract-first: `description` is +now a declared key. This is the empirical argument for the ratchet: the +inference "no metadata in the repo carries unknown keys" was **false**, and +only the strict gate could prove it. + +## File-level triage — the five authorable directories + +Site counts are `z.object(` occurrences per file (2026-07-30, this branch). +Classification is per the rule above; **(p)** marks a provisional call made +from the file's exports/JSDoc rather than a full read — verify before +tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). + +### `ui/` — 192 sites + +| File | Sites | Class | Note / next action | +|---|---|---|---| +| `action.zod.ts` | 8 | authorable | param schema strict (#3746); remaining blocks ride later steps | +| `view.zod.ts` | 51 | authorable | partially strict (ADR-0089); long tail of sub-blocks | +| `component.zod.ts` | 29 | authorable | **next candidate** — SDUI component defs; check React-prop open slots first (p) | +| `theme.zod.ts` | 14 | authorable (p) | authored themes | +| `app.zod.ts` | 11 | authorable | **next verified step** — `AppSchema` + nav-item union; recursive `NavigationItemSchema` needs union-error care | +| `dashboard.zod.ts` | 11 | authorable | partially strict | +| `widget.zod.ts` | 9 | authorable (p) | | +| `page.zod.ts` | 7 | authorable | partially strict (ADR-0089) | +| `chart.zod.ts` / `i18n.zod.ts` / `responsive.zod.ts` | 6+6+6 | authorable (p) | i18n label shapes are wide-open records by design — verify | +| `dataset.zod.ts` / `animation.zod.ts` / `dnd.zod.ts` / `keyboard.zod.ts` / `touch.zod.ts` | 4 ea | authorable (p) | interaction configs | +| `notification.zod.ts` / `offline.zod.ts` / `report.zod.ts` | 3 ea | authorable (p) | | +| `sharing.zod.ts` | 2 | authorable (p) | public-sharing config | + +### `data/` — 163 sites + +| File | Sites | Class | Note | +|---|---|---|---| +| `object.zod.ts` | 20 | authorable | top-level already guarded (#1535); inner blocks partially strict | +| `data-engine.zod.ts` | 14 | wire (p) | engine contract shapes | +| `external-lookup.zod.ts` | 12 | mixed (p) | authored config + wire results | +| `seed-loader.zod.ts` | 12 | mixed (p) | seed file shapes are authored; loader state is runtime | +| `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** | +| `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** | +| `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) | | + +### `automation/` — 80 sites + +| File | Sites | Class | Note | +|---|---|---|---| +| `flow.zod.ts` | 12 | authorable | **strict as of #4001** (4 schemas; `FlowVersionHistorySchema` is runtime — stays tolerant) | +| `sync.zod.ts` / `etl.zod.ts` | 12+10 | authorable (p) | authored pipelines — **candidates** | +| `trigger-registry.zod.ts` | 11 | mixed | descriptors are code-registered (wire-ish); bindings authored | +| `execution.zod.ts` | 8 | wire | run-state envelopes — never strict | +| `state-machine.zod.ts` | 7 | authorable (p) | | +| `control-flow.zod.ts` | 6 | authorable (p) | validated structurally by `validateControlFlow` | +| `bpmn-interop.zod.ts` | 5 | wire (p) | interop import shapes | +| `approval.zod.ts` | 4 | authorable | **next candidate** — v17 approval nodes are new authoring surface | +| `node-executor.zod.ts` | 4 | wire | executor contract | +| `webhook.zod.ts` | 1 | authorable (p) | spec-only (#3461) | + +### `security/` — 20 sites + +| File | Sites | Class | Note | +|---|---|---|---| +| `explain.zod.ts` | 11 | wire | permission-explain responses — never strict | +| `permission.zod.ts` | 4 | authorable | **strict as of #4001**; `EffectiveObjectPermissionSchema` explicitly `.strip()`s (wire) | +| `rls.zod.ts` | 3 | authorable | **next candidate** — a stripped RLS key is a silent policy hole | +| `sharing.zod.ts` | 2 | authorable | **next candidate** — same class | + +### `studio/` — 27 sites + +| File | Sites | Class | Note | +|---|---|---|---| +| `object-designer.zod.ts` | 12 | authorable (p) | Studio-written JSON — machine-authored but *our* machine; strict protects the builder itself | +| `plugin.zod.ts` | 8 | mixed (p) | | +| `flow-builder.zod.ts` | 7 | authorable (p) | independent of `FlowSchema` shapes | + +## Other directories (coarse; classify per schema before touching) + +| Dir | Sites | Dominant class | Rationale | +|---|---|---|---| +| `api/` | 426 | wire | REST/GraphQL request/response contracts — tolerant by design | +| `system/` | 383 | mixed | manifest/datasource blocks are authored; runtime envelopes are wire | +| `kernel/` | 351 | wire | plugin/kernel contracts, code-to-code | +| `cloud/` | 83 | wire | multi-tenant runtime | +| `ai/` | 75 | mixed | agent/tool/skill definitions authored (partially strict already); model/provider payloads wire | +| `integration/` | 64 | wire | connector payloads — upstream adds fields freely | +| `identity/` | 34 | mixed | position/user shapes authored; auth payloads wire | +| `shared/` | 25 | n/a | utilities and building blocks; strictness decided at the consuming schema | +| `qa/` | 6 | n/a | test fixtures | + +## Next steps (verify-then-enforce, one shape at a time) + +1. `ui/app.zod.ts` — `AppSchema` + navigation union (highest-traffic remaining + authorable type; needs union-error design so the strict error is readable). +2. `security/rls.zod.ts` + `security/sharing.zod.ts` — small, security-class. +3. `automation/approval.zod.ts` — new v17 authoring surface, tighten while young. +4. `data/hook.zod.ts`, `data/datasource.zod.ts` — `defineHook` / stack config. +5. 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. + +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/api-surface.json b/packages/spec/api-surface.json index c1841144d3..3a4321459d 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -4469,6 +4469,7 @@ "SortItemSchema (const)", "StaticMount (type)", "StaticMountSchema (const)", + "StrictUnknownKeyErrorOptions (interface)", "SystemIdentifier (type)", "SystemIdentifierSchema (const)", "TemplateExpressionInput (type)", @@ -4498,6 +4499,7 @@ "resilientFetch (function)", "safeParsePretty (function)", "singularToPlural (function)", + "strictUnknownKeyError (function)", "strictVisibilityError (const)", "suggestFieldType (function)", "tmpl (function)" diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 0caa8cc0d5..724f32d2b6 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -5721,6 +5721,7 @@ "security/ObjectPermission:viewAllRecords", "security/ObjectPermission:writeScope", "security/PermissionSet:adminScope", + "security/PermissionSet:description", "security/PermissionSet:fields", "security/PermissionSet:isDefault", "security/PermissionSet:label", diff --git a/packages/spec/liveness/permission.json b/packages/spec/liveness/permission.json index 5d380d0ca7..edc9232227 100644 --- a/packages/spec/liveness/permission.json +++ b/packages/spec/liveness/permission.json @@ -14,6 +14,12 @@ "evidence": "rendered as the record title of sys_permission_set in Setup's generic object views (objectui app-shell list/detail rendering via displayNameField)", "note": "display metadata." }, + "description": { + "status": "live", + "verifiedAt": "2026-07-30", + "evidence": "persisted to sys_permission_set.description (plugin-security sys-permission-set.object.ts: textarea field, list columns + form); read from the authored set by permission-set-projection.ts (description: ps.description ?? null) and diffed on re-seed", + "note": "Declared in #4001: the built-in default sets always authored it, but the schema could not represent it, so it was silently stripped at parse (ADR-0078 §3 inverse drift) — the .strict() gate surfaced the gap." + }, "packageId": { "status": "live", "verifiedAt": "2026-07-30", diff --git a/packages/spec/src/automation/flow.test.ts b/packages/spec/src/automation/flow.test.ts index 80817c9403..54be2b56ba 100644 --- a/packages/spec/src/automation/flow.test.ts +++ b/packages/spec/src/automation/flow.test.ts @@ -1103,3 +1103,130 @@ describe('BPMN — Fault Edge Enhancement', () => { }); }); }); + +// #4001 — the authorable flow surface is `.strict()`: an undeclared key used to +// be dropped by zod's default `.strip`, so a trigger binding or config the +// author wrote was quietly ignored — worst on this, the most AI-authored +// surface (cloud#688 / #2419). A node's `config` record deliberately stays +// OPEN: it is per-node-type, owned by the executor's `configSchema` +// (#4027/#4040) and the ADR-0087 conversion layer. +describe('unknown keys are rejected, not stripped (#4001)', () => { + const unknownKeyIssue = (schema: { safeParse: (v: unknown) => any }, value: unknown) => { + const result = schema.safeParse(value); + expect(result.success).toBe(false); + return result.error!.issues.find((i: { code: string }) => i.code === 'unrecognized_keys'); + }; + + const minimalFlow = { + name: 'f', label: 'F', type: 'autolaunched' as const, + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end' }], + }; + + describe('FlowSchema', () => { + it('rejects an undeclared key instead of silently dropping it', () => { + const issue = unknownKeyIssue(FlowSchema, { ...minimalFlow, notAKey: 1 }); + expect(issue!.message).toContain('`notAKey`'); + }); + + it('points builder vocabulary (steps/connections/trigger) at the canonical keys', () => { + expect(unknownKeyIssue(FlowSchema, { ...minimalFlow, steps: [] })!.message) + .toContain('`steps` → `nodes`'); + expect(unknownKeyIssue(FlowSchema, { ...minimalFlow, connections: [] })!.message) + .toContain('`connections` → `edges`'); + expect(unknownKeyIssue(FlowSchema, { ...minimalFlow, trigger: 'record_change' })!.message) + .toContain('`trigger` → `type`'); + }); + + it('points a top-level object binding at the START node config', () => { + for (const key of ['object', 'objectName']) { + const message = unknownKeyIssue(FlowSchema, { ...minimalFlow, [key]: 'task' })!.message; + expect(message, `\`${key}\` should point at the start node`).toContain('START node'); + } + }); + + it('accepts every key the schema declares (guards FLOW_KEYS drift)', () => { + const probes: Record = { + description: 'd', successMessage: 's', errorMessage: 'e', version: 2, + status: 'active', template: false, variables: [{ name: 'v', type: 'text' }], + active: true, runAs: 'system', + errorHandling: { strategy: 'retry', maxRetries: 2 }, + protection: { lock: 'none' }, + }; + for (const [key, value] of Object.entries(probes)) { + const result = FlowSchema.safeParse({ ...minimalFlow, [key]: value }); + const unknown = result.success + ? undefined + : result.error.issues.find((i) => i.code === 'unrecognized_keys'); + expect(unknown, `\`${key}\` should be a declared Flow key`).toBeUndefined(); + } + }); + }); + + describe('FlowNodeSchema', () => { + it('keeps `config` open — per-node-type keys are the executor contract', () => { + const parsed = FlowNodeSchema.parse({ + id: 'n1', type: 'create_record', label: 'Create', + config: { objectName: 'task', fields: { subject: 'hi' }, anyExecutorKey: 1 }, + }); + expect((parsed.config as Record).anyExecutorKey).toBe(1); + }); + + it('points config synonyms at `config`', () => { + const node = { id: 'n1', type: 'script', label: 'S' }; + expect(unknownKeyIssue(FlowNodeSchema, { ...node, settings: {} })!.message) + .toContain('`settings` → `config`'); + expect(unknownKeyIssue(FlowNodeSchema, { ...node, parameters: {} })!.message) + .toContain('`parameters` → `config`'); + }); + + it('explains where node inputs live for an `inputs` key', () => { + expect(unknownKeyIssue(FlowNodeSchema, { id: 'n1', type: 'script', label: 'S', inputs: {} })!.message) + .toContain('`config`'); + }); + + it('accepts every key the schema declares (guards FLOW_NODE_KEYS drift)', () => { + const probes: Record = { + config: { a: 1 }, + connectorConfig: { connectorId: 'c', actionId: 'a', input: {} }, + position: { x: 0, y: 0 }, timeoutMs: 100, + inputSchema: { p: { type: 'string' } }, outputSchema: { o: { type: 'number' } }, + waitEventConfig: { eventType: 'timer', timerDuration: 'PT1H' }, + boundaryConfig: { attachedToNodeId: 'n0', eventType: 'error' }, + }; + for (const [key, value] of Object.entries(probes)) { + const result = FlowNodeSchema.safeParse({ id: 'n1', type: 'script', label: 'S', [key]: value }); + const unknown = result.success + ? undefined + : result.error.issues.find((i) => i.code === 'unrecognized_keys'); + expect(unknown, `\`${key}\` should be a declared FlowNode key`).toBeUndefined(); + } + }); + }); + + describe('FlowEdgeSchema', () => { + it('points from/to at source/target (the n8n/mermaid slip)', () => { + expect(unknownKeyIssue(FlowEdgeSchema, { id: 'e1', source: 'a', target: 'b', from: 'a' })!.message) + .toContain('`from` → `source`'); + expect(unknownKeyIssue(FlowEdgeSchema, { id: 'e1', source: 'a', target: 'b', to: 'b' })!.message) + .toContain('`to` → `target`'); + }); + + it('points a guard/when/expression key at `condition`', () => { + for (const key of ['guard', 'when', 'expression']) { + expect(unknownKeyIssue(FlowEdgeSchema, { id: 'e1', source: 'a', target: 'b', [key]: 'x > 1' })!.message) + .toContain(`\`${key}\` → \`condition\``); + } + }); + }); + + describe('FlowVariableSchema', () => { + it('rejects an undeclared key with a suggestion', () => { + expect(unknownKeyIssue(FlowVariableSchema, { name: 'v', type: 'text', is_input: true })!.message) + .toContain('`is_input` → `isInput`'); + }); + }); +}); diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 56f3425c1b..8b1f3a0d68 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -4,6 +4,7 @@ import { z } from 'zod'; import { ProtectionSchema } from '../shared/protection.zod'; import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { ExpressionInputSchema } from '../shared/expression.zod'; +import { strictUnknownKeyError } from '../shared/suggestions.zod'; /** * Flow Node Types — **built-in seed set** (ADR-0018). @@ -61,6 +62,35 @@ export const FLOW_BUILTIN_NODE_TYPES: readonly string[] = FlowNodeAction.options */ export const FLOW_STRUCTURAL_NODE_TYPES: readonly string[] = ['start', 'end']; +/* + * ── Unknown-key strictness (#4001, ADR-0078) ──────────────────────────────── + * + * The four AUTHORING schemas in this module (flow / node / edge / variable) + * are `.strict()`: a key they do not declare is a loud, fixable parse error, + * not a silent strip. Flows are the most AI-authored surface in the platform, + * and an AI author + a silently dropped key is the worst combination — the + * agent gets a success envelope and reports "done" over dead metadata + * (cloud#688 / #2419 is exactly this class). A node's `config` stays an OPEN + * record here: it is per-node-type, owned by the registered executor's + * `configSchema` (#4027/#4040) and the ADR-0087 conversion layer — outer-shell + * strictness must not close the plugin node-type namespace. + * + * Key lists are kept beside the schemas rather than derived from `.shape` + * (bodies are allocated lazily; `flow.test.ts` drift-guards every entry). + */ + +/** Keys {@link FlowVariableSchema} declares (drift-guarded by flow.test.ts). */ +const FLOW_VARIABLE_KEYS = ['name', 'type', 'isInput', 'isOutput'] as const; + +const flowVariableUnknownKeyError = strictUnknownKeyError({ + surface: 'this flow variable', + knownKeys: FLOW_VARIABLE_KEYS, + aliases: { input: 'isInput', output: 'isOutput' }, + history: + 'Until #4001 these were dropped silently — the variable still parsed, so a ' + + 'mis-declared input/output contract shipped without a diagnostic.', +}); + /** * Flow Variable Schema * Variables available within the flow execution context. @@ -70,7 +100,7 @@ export const FlowVariableSchema = lazySchema(() => z.object({ type: z.string().describe('Data type (text, number, boolean, object, list)'), isInput: z.boolean().default(false).describe('Is input parameter'), isOutput: z.boolean().default(false).describe('Is output parameter'), -})); +}, { error: flowVariableUnknownKeyError }).strict()); /** * Flow Node Schema @@ -90,6 +120,33 @@ export const FlowVariableSchema = lazySchema(() => z.object({ * position: { x: 300, y: 200 } * } */ +/** Keys {@link FlowNodeSchema} declares (drift-guarded by flow.test.ts). */ +const FLOW_NODE_KEYS = [ + 'id', 'type', 'label', 'config', 'connectorConfig', 'position', 'timeoutMs', + 'inputSchema', 'outputSchema', 'waitEventConfig', 'boundaryConfig', +] as const; + +const flowNodeUnknownKeyError = strictUnknownKeyError({ + surface: 'this flow node', + knownKeys: FLOW_NODE_KEYS, + aliases: { + configuration: 'config', + settings: 'config', + properties: 'config', + options: 'config', + params: 'config', + parameters: 'config', + }, + guidance: { + inputs: + '`inputs` is not a FlowNode key — a node\'s runtime inputs live under `config` ' + + '(e.g. `config.inputs` for script/function nodes); `inputSchema` declares their types.', + }, + history: + 'Until #4001 these were dropped silently — the node still parsed, so a mis-placed ' + + 'config shipped as a step that quietly ignored it.', +}); + export const FlowNodeSchema = lazySchema(() => z.object({ id: z.string().describe('Node unique ID'), type: z.string().min(1).describe( @@ -170,7 +227,28 @@ export const FlowNodeSchema = lazySchema(() => z.object({ /** Signal name — only for signal boundary events */ signalName: z.string().optional().describe('Named signal to catch'), }).optional().describe('Configuration for boundary events attached to host nodes'), -})); +}, { error: flowNodeUnknownKeyError }).strict()); + +/** Keys {@link FlowEdgeSchema} declares (drift-guarded by flow.test.ts). */ +const FLOW_EDGE_KEYS = ['id', 'source', 'target', 'condition', 'type', 'label', 'isDefault'] as const; + +const flowEdgeUnknownKeyError = strictUnknownKeyError({ + surface: 'this flow edge', + knownKeys: FLOW_EDGE_KEYS, + aliases: { + // n8n / mermaid / BPMN-tool vocabulary an author (or AI) imports wholesale. + from: 'source', + to: 'target', + sourceid: 'source', + targetid: 'target', + expression: 'condition', + when: 'condition', + guard: 'condition', + }, + history: + 'Until #4001 these were dropped silently — the edge still parsed, so a branch ' + + 'predicate or endpoint the author wrote was quietly ignored.', +}); /** * Flow Edge Schema @@ -200,7 +278,7 @@ export const FlowEdgeSchema = lazySchema(() => z.object({ */ isDefault: z.boolean().default(false) .describe('Marks this edge as the default path when no other conditions match'), -})); +}, { error: flowEdgeUnknownKeyError }).strict()); /** * Flow Schema @@ -228,6 +306,45 @@ export const FlowEdgeSchema = lazySchema(() => z.object({ * ] * } */ +/** Keys {@link FlowSchema} declares (drift-guarded by flow.test.ts). */ +const FLOW_KEYS = [ + 'name', 'label', 'description', 'successMessage', 'errorMessage', 'version', + 'status', 'template', 'type', 'variables', 'nodes', 'edges', 'active', 'runAs', + 'errorHandling', 'protection', + // ADR-0010 runtime protection envelope (MetadataProtectionFields spread). + '_lock', '_lockReason', '_lockSource', '_provenance', '_packageId', + '_packageVersion', '_lockDocsUrl', +] as const; + +const flowUnknownKeyError = strictUnknownKeyError({ + surface: 'this flow', + knownKeys: FLOW_KEYS, + aliases: { + steps: 'nodes', + connections: 'edges', + transitions: 'edges', + links: 'edges', + trigger: 'type', + triggertype: 'type', + title: 'label', + }, + guidance: { + object: + '`object` is not a Flow field — a record-change flow binds its object on the ' + + 'START node\'s `config` (`{ objectName, triggerType, condition }`), not at the ' + + 'flow top level.', + objectName: + '`objectName` is not a Flow field — it belongs on the START node\'s `config` ' + + '(`{ objectName, triggerType, condition }`), not at the flow top level.', + schedule: + '`schedule` is not a Flow field — a schedule flow declares its cron/interval as ' + + '`config.schedule` on the START node, not at the flow top level.', + }, + history: + 'Until #4001 these were dropped silently — the flow still parsed, so a trigger ' + + 'binding or config the author wrote was quietly ignored.', +}); + export const FlowSchema = lazySchema(() => z.object({ /** Identity */ name: z.string().regex(/^[a-z_][a-z0-9_]*$/).describe('Machine name'), @@ -307,7 +424,7 @@ export const FlowSchema = lazySchema(() => z.object({ // ADR-0010 — runtime protection envelope (internal — set by loader). ...MetadataProtectionFields, -})); +}, { error: flowUnknownKeyError }).strict()); /** * Type-safe factory for creating flow definitions. diff --git a/packages/spec/src/security/permission.test.ts b/packages/spec/src/security/permission.test.ts index c31c017034..53e2fc30ad 100644 --- a/packages/spec/src/security/permission.test.ts +++ b/packages/spec/src/security/permission.test.ts @@ -141,10 +141,27 @@ describe('EffectiveObjectPermissionSchema (#3391 response-side)', () => { }); it('does not leak apiOperations onto the authoring ObjectPermissionSchema', () => { - // The authoring schema stays unextended — a stray apiOperations key is - // stripped (or rejected) there, never a valid authoring field. - const parsed = ObjectPermissionSchema.parse({ allowRead: true, apiOperations: ['get'] } as any); - expect((parsed as any).apiOperations).toBeUndefined(); + // The authoring schema stays unextended — since #4001 a stray apiOperations + // key is REJECTED there (loud, with the response-side pointer), never a + // valid authoring field and no longer a silent strip. + const result = ObjectPermissionSchema.safeParse({ allowRead: true, apiOperations: ['get'] } as any); + expect(result.success).toBe(false); + const issue = result.error!.issues.find((i) => i.code === 'unrecognized_keys'); + expect(issue!.message).toContain('`apiOperations`'); + expect(issue!.message).toContain('RESPONSE surface'); + }); + + it('EffectiveObjectPermissionSchema stays wire-tolerant (strips, never rejects)', () => { + // `.extend()` inherits `.strict()`, so the response shape explicitly + // `.strip()`s back — a newer server adding a response key must not crash + // an older parser (#4001 authorable/wire split). + const parsed = EffectiveObjectPermissionSchema.parse({ + allowRead: true, + apiOperations: ['get'], + someFutureServerKey: true, + } as any); + expect((parsed as any).someFutureServerKey).toBeUndefined(); + expect((parsed as any).allowRead).toBe(true); }); }); @@ -533,3 +550,139 @@ describe('PermissionSetSchema - tabPermissions', () => { expect(result.tabPermissions).toBeUndefined(); }); }); + +// #4001 — the authorable permission surface is `.strict()`: an undeclared key +// used to be dropped by zod's default `.strip`, so the author believed a grant +// or restriction was in place that the runtime never saw (the ADR-0049 +// asymmetry at the capability container itself). Strictness plus the shared +// `strictUnknownKeyError` factory turns that into a loud, fixable error. +describe('unknown keys are rejected, not stripped (#4001)', () => { + const unknownKeyIssue = (schema: { safeParse: (v: unknown) => any }, value: unknown) => { + const result = schema.safeParse(value); + expect(result.success).toBe(false); + return result.error!.issues.find((i: { code: string }) => i.code === 'unrecognized_keys'); + }; + + describe('PermissionSetSchema', () => { + it('rejects an undeclared key instead of silently dropping it', () => { + const issue = unknownKeyIssue(PermissionSetSchema, { + name: 'p', objects: {}, notAKey: true, + }); + expect(issue!.message).toContain('`notAKey`'); + }); + + it('points neighbouring-vocabulary spellings at the canonical key', () => { + expect(unknownKeyIssue(PermissionSetSchema, { name: 'p', objects: {}, objectPermissions: {} })!.message) + .toContain('`objectPermissions` → `objects`'); + expect(unknownKeyIssue(PermissionSetSchema, { name: 'p', objects: {}, rls: [] })!.message) + .toContain('`rls` → `rowLevelSecurity`'); + expect(unknownKeyIssue(PermissionSetSchema, { name: 'p', objects: {}, tabs: {} })!.message) + .toContain('`tabs` → `tabPermissions`'); + }); + + it('carries the ADR-0105 D11 tombstone for the retired contextVariables', () => { + const message = unknownKeyIssue(PermissionSetSchema, { + name: 'p', objects: {}, contextVariables: { teams: ['a'] }, + })!.message; + expect(message).toContain('ADR-0105 D11'); + expect(message).toContain('rlsMembership'); + }); + + it('carries the ADR-0090 D2 tombstone for the retired isProfile', () => { + const message = unknownKeyIssue(PermissionSetSchema, { + name: 'p', objects: {}, isProfile: true, + })!.message; + expect(message).toContain('ADR-0090 D2'); + expect(message).toContain('`isDefault`'); + }); + + it('points wrong-layer keys (profiles/roles/users) at the binding mechanism', () => { + for (const key of ['profiles', 'roles', 'users']) { + const message = unknownKeyIssue(PermissionSetSchema, { + name: 'p', objects: {}, [key]: [], + })!.message; + expect(message, `\`${key}\` should carry guidance`).toContain(`\`${key}\` is not a PermissionSet field`); + } + }); + + it('accepts every key the schema declares (guards PERMISSION_SET_KEYS drift)', () => { + const probes: Record = { + name: 'p', label: 'P', description: 'd', packageId: 'pkg', managedBy: 'user', + isDefault: true, + objects: { task: { allowRead: true } }, fields: { 'task.secret': { readable: false } }, + systemPermissions: ['manage_users'], tabPermissions: { app_crm: 'visible' }, + rowLevelSecurity: [], adminScope: { businessUnit: 'east' }, + }; + for (const [key, value] of Object.entries(probes)) { + const result = PermissionSetSchema.safeParse({ name: 'p', objects: {}, [key]: value }); + const unknown = result.success + ? undefined + : result.error.issues.find((i) => i.code === 'unrecognized_keys'); + expect(unknown, `\`${key}\` should be a declared PermissionSet key`).toBeUndefined(); + } + }); + }); + + describe('ObjectPermissionSchema', () => { + it('points the bare CRUD verbs at the allow* bits', () => { + expect(unknownKeyIssue(ObjectPermissionSchema, { read: true })!.message) + .toContain('`read` → `allowRead`'); + expect(unknownKeyIssue(ObjectPermissionSchema, { edit: true })!.message) + .toContain('`edit` → `allowEdit`'); + expect(unknownKeyIssue(ObjectPermissionSchema, { export: true })!.message) + .toContain('`export` → `allowExport`'); + expect(unknownKeyIssue(ObjectPermissionSchema, { viewAll: true })!.message) + .toContain('`viewAll` → `viewAllRecords`'); + }); + + it('accepts every key the schema declares (guards OBJECT_PERMISSION_KEYS drift)', () => { + const probes: Record = { + allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true, + allowExport: true, allowTransfer: true, allowRestore: true, allowPurge: true, + viewAllRecords: true, modifyAllRecords: true, readScope: 'org', writeScope: 'own', + }; + for (const [key, value] of Object.entries(probes)) { + const result = ObjectPermissionSchema.safeParse({ [key]: value }); + const unknown = result.success + ? undefined + : result.error.issues.find((i) => i.code === 'unrecognized_keys'); + expect(unknown, `\`${key}\` should be a declared ObjectPermission key`).toBeUndefined(); + } + }); + }); + + describe('FieldPermissionSchema', () => { + it('points read/write vocabulary at readable/editable', () => { + expect(unknownKeyIssue(FieldPermissionSchema, { read: true })!.message) + .toContain('`read` → `readable`'); + expect(unknownKeyIssue(FieldPermissionSchema, { write: true })!.message) + .toContain('`write` → `editable`'); + }); + + it('explains that FLS is declared positively for a `hidden` key', () => { + expect(unknownKeyIssue(FieldPermissionSchema, { hidden: true })!.message) + .toContain('`readable: false`'); + }); + }); + + describe('AdminScopeSchema', () => { + it('rejects an undeclared key with a typo suggestion', () => { + expect(unknownKeyIssue(AdminScopeSchema, { businessUnit: 'east', business_unit: 'x' })!.message) + .toContain('`business_unit` → `businessUnit`'); + }); + + it('accepts every key the schema declares (guards ADMIN_SCOPE_KEYS drift)', () => { + const probes: Record = { + businessUnit: 'east', includeSubtree: false, manageAssignments: true, + manageBindings: true, authorEnvironmentSets: true, assignablePermissionSets: ['a'], + }; + for (const [key, value] of Object.entries(probes)) { + const result = AdminScopeSchema.safeParse({ businessUnit: 'east', [key]: value }); + const unknown = result.success + ? undefined + : result.error.issues.find((i) => i.code === 'unrecognized_keys'); + expect(unknown, `\`${key}\` should be a declared AdminScope key`).toBeUndefined(); + } + }); + }); +}); diff --git a/packages/spec/src/security/permission.zod.ts b/packages/spec/src/security/permission.zod.ts index a9dd4d9402..e2371df35f 100644 --- a/packages/spec/src/security/permission.zod.ts +++ b/packages/spec/src/security/permission.zod.ts @@ -15,6 +15,7 @@ import { ApiOperationSchema } from '../data/object.zod'; * - Purge (Hard delete / Compliance) */ import { lazySchema } from '../shared/lazy-schema'; +import { strictUnknownKeyError } from '../shared/suggestions.zod'; /** * [ADR-0057 D1] Object access DEPTH — the Dataverse "access level" axis, * layered on top of OWD. Widens the owner-match for owner-scoped objects. @@ -22,6 +23,74 @@ import { lazySchema } from '../shared/lazy-schema'; export const ObjectAccessScopeSchema = z.enum(['own', 'own_and_reports', 'unit', 'unit_and_below', 'org']); export type ObjectAccessScope = z.infer; +/* + * ── Unknown-key strictness (#4001, ADR-0078) ──────────────────────────────── + * + * Every AUTHORING schema in this module is `.strict()`: a key it does not + * declare is a loud, fixable parse error, not a silent strip. A permission + * set is the worst possible place for zod's default `.strip` — a mis-spelled + * or wrong-layer key here means the author believes a grant or restriction is + * in place that the runtime never saw (the ADR-0049 asymmetry, at the + * capability container itself). The RESPONSE-side extension + * (`EffectiveObjectPermissionSchema`) deliberately stays tolerant — see below. + * + * Key lists are kept beside the schemas rather than derived from `.shape` + * (the bodies are allocated lazily; `permission.test.ts` drift-guards every + * entry), and each error map names the canonical key when the authored one is + * a recognisable spelling of it. + */ + +/** Keys {@link ObjectPermissionSchema} declares (drift-guarded by permission.test.ts). */ +const OBJECT_PERMISSION_KEYS = [ + 'allowCreate', 'allowRead', 'allowEdit', 'allowDelete', 'allowExport', + 'allowTransfer', 'allowRestore', 'allowPurge', + 'viewAllRecords', 'modifyAllRecords', 'readScope', 'writeScope', +] as const; + +/** + * Semantic near-misses for object-permission bits — mostly the bare CRUD verbs + * (Salesforce object-permission vocabulary) an author reaches for before + * learning the `allow*` prefix. Edit distance cannot bridge `read` → + * `allowRead`, so they are named explicitly. + */ +const OBJECT_PERMISSION_KEY_ALIASES: Readonly> = { + read: 'allowRead', + create: 'allowCreate', + edit: 'allowEdit', + update: 'allowEdit', + write: 'allowEdit', + delete: 'allowDelete', + remove: 'allowDelete', + export: 'allowExport', + transfer: 'allowTransfer', + restore: 'allowRestore', + purge: 'allowPurge', + canread: 'allowRead', + cancreate: 'allowCreate', + canedit: 'allowEdit', + candelete: 'allowDelete', + viewall: 'viewAllRecords', + viewalldata: 'viewAllRecords', + modifyall: 'modifyAllRecords', + modifyalldata: 'modifyAllRecords', +}; + +const objectPermissionUnknownKeyError = strictUnknownKeyError({ + surface: 'this object permission', + knownKeys: OBJECT_PERMISSION_KEYS, + aliases: OBJECT_PERMISSION_KEY_ALIASES, + guidance: { + apiOperations: + '`apiOperations` is the server-resolved effective operation set (#3391) — it exists ' + + 'only on the RESPONSE surface (`/me/permissions`) and is never authored. Grant ' + + 'capability with the `allow*` bits here; tighten an object\'s exposure with ' + + '`apiMethods` on the object schema.', + }, + history: + 'Until #4001 these were dropped silently — the permission set still parsed, so the ' + + 'author believed a grant or restriction was in place that the runtime never saw.', +}); + export const ObjectPermissionSchema = lazySchema(() => z.object({ /** C: Create */ allowCreate: z.boolean().default(false).describe('Create permission'), @@ -119,7 +188,7 @@ export const ObjectPermissionSchema = lazySchema(() => z.object({ readScope: ObjectAccessScopeSchema.optional().describe('[ADR-0057 D1] Read depth: own|unit|unit_and_below|org'), /** [ADR-0057 D1] Write (edit/delete) access DEPTH — same enum as readScope. */ writeScope: ObjectAccessScopeSchema.optional().describe('[ADR-0057 D1] Write depth: own|unit|unit_and_below|org'), -})); +}, { error: objectPermissionUnknownKeyError }).strict()); /** * RESPONSE-side extension of {@link ObjectPermissionSchema} carrying the @@ -142,7 +211,12 @@ export const EffectiveObjectPermissionSchema = lazySchema(() => apiOperations: z.array(ApiOperationSchema).optional().describe( 'Server-resolved effective API operations for this object (#3391). Present only when the object tightens exposure via apiMethods; absent = default-allow. The frontend renders this effective set, never the raw whitelist. Vocabulary is the EFFECTIVE ApiOperation set (six primitives + eight derived verbs, #3543), not the authored six-value ApiMethod enum.', ), - }), + // WIRE shape: `.extend()` inherits the authoring schema's `.strict()`, and a + // strict response parser is forward-incompatible — a newer server adding a + // response key would crash an older client. `.strip()` restores zod-default + // tolerance here; strictness is an AUTHORING-side contract only (#4001's + // authorable/wire split). + }).strip(), ); export type EffectiveObjectPermission = z.infer; @@ -163,6 +237,20 @@ export type EffectiveObjectPermission = z.infer z.object({ /** Root of the delegated subtree — `sys_business_unit.name` (machine name, portable across environments). */ businessUnit: z.string().describe('[ADR-0090 D12] Delegation boundary: sys_business_unit.name of the subtree root'), @@ -181,12 +269,32 @@ export const AdminScopeSchema = lazySchema(() => z.object({ * grantor's scope to STRICTLY contain the granted one. */ assignablePermissionSets: z.array(z.string()).default([]).describe('Allowlist of permission-set names the delegate may hand out'), -})); +}, { error: adminScopeUnknownKeyError }).strict()); export type AdminScope = z.infer; /** Authoring input for {@link AdminScope} — defaulted fields are optional. */ export type AdminScopeInput = z.input; +const fieldPermissionUnknownKeyError = strictUnknownKeyError({ + surface: 'this field permission', + knownKeys: ['readable', 'editable'], + aliases: { + read: 'readable', + visible: 'readable', + write: 'editable', + edit: 'editable', + update: 'editable', + }, + guidance: { + hidden: + '`hidden` is not a FieldPermission key — FLS is declared positively: set ' + + '`readable: false` to hide the field.', + }, + history: + 'Until #4001 these were dropped silently — the entry still parsed, so field-level ' + + 'security the author intended was never applied.', +}); + /** * Field Level Security (FLS) */ @@ -195,7 +303,7 @@ export const FieldPermissionSchema = lazySchema(() => z.object({ readable: z.boolean().default(true).describe('Field read access'), /** Can edit this field */ editable: z.boolean().default(false).describe('Field edit access'), -})); +}, { error: fieldPermissionUnknownKeyError }).strict()); /** * Permission Set Schema @@ -221,6 +329,61 @@ export const FieldPermissionSchema = lazySchema(() => z.object({ * - 'SystemAdmin' (mixed case) * - 'Read Only' (spaces) */ +/** Keys {@link PermissionSetSchema} declares (drift-guarded by permission.test.ts). */ +const PERMISSION_SET_KEYS = [ + 'name', 'label', 'description', 'packageId', 'managedBy', 'isDefault', 'objects', + 'fields', 'systemPermissions', 'tabPermissions', 'rowLevelSecurity', 'adminScope', +] as const; + +/** Semantic near-misses borrowed from neighbouring schemas / products. */ +const PERMISSION_SET_KEY_ALIASES: Readonly> = { + objectpermissions: 'objects', + entities: 'objects', + entitypermissions: 'objects', + fieldpermissions: 'fields', + fls: 'fields', + fieldlevelsecurity: 'fields', + tabs: 'tabPermissions', + tabvisibility: 'tabPermissions', + rls: 'rowLevelSecurity', + rowlevelsecurityrules: 'rowLevelSecurity', + policies: 'rowLevelSecurity', +}; + +const permissionSetUnknownKeyError = strictUnknownKeyError({ + surface: 'this permission set', + knownKeys: PERMISSION_SET_KEYS, + aliases: PERMISSION_SET_KEY_ALIASES, + guidance: { + // ── Tombstones for RETIRED keys (upgrade prescriptions; they age out ~two + // majors after removal — pattern of data/object.zod.ts UNKNOWN_KEY_GUIDANCE). + contextVariables: + '`contextVariables` was removed by ADR-0105 D11 (enforce-or-remove, ADR-0049): it ' + + 'was authorable but had zero runtime consumers. A custom membership set a policy ' + + 'needs as `field IN (current_user.)` is staged by a registered rlsMembership ' + + 'resolver; a constant belongs inline in the policy\'s `using` expression.', + isProfile: + '`isProfile` was removed by ADR-0090 D2 — there is no Profile concept. Permission ' + + 'sets are the only capability container; use `isDefault` to mark the app baseline ' + + 'bound to the built-in `everyone` position.', + // ── Wrong-layer pointers for keys that are never authored on a set. + profiles: + '`profiles` is not a PermissionSet field (ADR-0090 D2: no Profile concept). ' + + 'Distribution is a runtime binding — positions bind sets to people ' + + '(`sys_position_permission_set`), never the set itself.', + roles: + '`roles` is not a PermissionSet field — ObjectStack has no role hierarchy: ' + + 'capability = permission sets (union-merged), distribution = positions, ' + + 'visibility depth = business units (ADR-0057 / ADR-0090).', + users: + '`users` is not a PermissionSet field — assignment is a runtime binding ' + + '(`sys_user_permission_set` / positions), never authored on the set (ADR-0090).', + }, + history: + 'Until #4001 these were dropped silently — the set still parsed, so the author ' + + 'believed a capability boundary was declared that the runtime never saw.', +}); + export const PermissionSetSchema = lazySchema(() => z.object({ /** Unique permission set name */ name: SnakeCaseIdentifierSchema.describe('Permission set unique name (lowercase snake_case)'), @@ -228,6 +391,16 @@ export const PermissionSetSchema = lazySchema(() => z.object({ /** Display label */ label: z.string().optional().describe('Display label'), + /** + * Human-readable description, surfaced on `sys_permission_set` (list columns + * + form) and the Setup projection (`permission-set-projection.ts` reads it + * from the authored set). The built-in default sets have always authored it — + * but until #4001 the schema could not represent it, so it was silently + * stripped at parse (the ADR-0078 §3 inverse-drift class); the `.strict()` + * gate surfaced the gap and it is now declared. + */ + description: z.string().optional().describe('Human-readable description shown in Setup (persisted as sys_permission_set.description)'), + /** * [ADR-0086 D3] Owning package for a package-shipped permission set * (absent ⇒ environment-authored). Persisted on `sys_permission_set` @@ -340,7 +513,7 @@ export const PermissionSetSchema = lazySchema(() => z.object({ */ adminScope: AdminScopeSchema.optional() .describe('[ADR-0090 D12] Scoped delegated-administration grant (BU subtree + assignable-set allowlist)'), -})); +}, { error: permissionSetUnknownKeyError }).strict()); export type PermissionSet = z.infer; /** Authoring input for {@link PermissionSet} — defaulted fields are optional. */ diff --git a/packages/spec/src/shared/suggestions.zod.ts b/packages/spec/src/shared/suggestions.zod.ts index bc2c617a6c..ab875575fc 100644 --- a/packages/spec/src/shared/suggestions.zod.ts +++ b/packages/spec/src/shared/suggestions.zod.ts @@ -1,5 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +import type { z } from 'zod'; import { FieldType } from '../data/field.zod'; /** @@ -182,3 +183,86 @@ export function formatSuggestion(suggestions: string[]): string { if (suggestions.length === 1) return `Did you mean '${suggestions[0]}'?`; return `Did you mean one of: ${suggestions.map((s) => `'${s}'`).join(', ')}?`; } + +/** `reference_to` / `referenceTo` / `Reference-To` all collapse onto one probe. */ +const aliasProbe = (key: string): string => key.toLowerCase().replace(/[_\-\s]/g, ''); + +/** Options for {@link strictUnknownKeyError}. */ +export interface StrictUnknownKeyErrorOptions { + /** Prose name of the authoring surface the key was written on (e.g. `'this permission set'`). */ + surface: string; + /** The schema's declared keys — candidates for the edit-distance fallback. */ + knownKeys: readonly string[]; + /** + * Semantic near-misses: a different *word* for the same intent, usually + * borrowed from a neighbouring schema or product where that word is correct. + * Edit distance cannot reach these, so they are named explicitly; plain + * case/underscore slips are left to {@link findClosestMatches}. Map keys are + * matched case-insensitively with `_` / `-` / space separators removed. + */ + aliases?: Readonly>; + /** + * Exact-key prescriptions, appended verbatim as bullet lines: tombstones for + * retired keys (the rejection must carry the upgrade — see the Post-Task + * Checklist in AGENTS.md), or wrong-layer pointers for keys that belong to a + * different surface. An entry here suppresses the rename suggestion for that + * key. Matched case-sensitively (exact authored spelling). + */ + guidance?: Readonly>; + /** One sentence of history: why this key would previously have failed silently. */ + history: string; +} + +/** + * Build the custom zod `error` map for a `.strict()` authorable schema — the + * shared factory behind the unknown-key strictness ratchet (#4001, ADR-0078). + * + * Zod's default is `.strip`: a key the schema does not declare is **silently + * discarded** and the instance goes on parsing. On an authorable surface that + * is the worst failure mode — the author (human or AI) gets a success and + * ships metadata that quietly ignores their config (#3405 action-param + * `reference`, #1535 object-level `workflows`). `.strict()` makes the drop + * loud; this factory makes it *fixable*: the error names the offending key(s) + * and, when one is a recognisable spelling of a declared key, points at the + * canonical one — alias table first (semantic near-misses), then a + * length-relative edit-distance fallback (matching `suggestKey` in + * `data/object.zod.ts`: a flat distance of 3 is noise on a short key). + * + * Wire it as the object's `error` alongside `.strict()`: + * + * ```ts + * z.object({ ... }, { error: strictUnknownKeyError({ ... }) }).strict() + * ``` + * + * First consumers: `ui/action.zod.ts` (#3746, the template this generalizes), + * `security/permission.zod.ts`, `automation/flow.zod.ts`. + */ +export function strictUnknownKeyError(options: StrictUnknownKeyErrorOptions): z.core.$ZodErrorMap { + const { surface, knownKeys, guidance = {}, history } = options; + const aliases: Record = {}; + for (const [key, canonical] of Object.entries(options.aliases ?? {})) { + aliases[aliasProbe(key)] = canonical; + } + return (issue) => { + if (issue.code !== 'unrecognized_keys') return undefined; + const keys = (issue as { keys?: readonly string[] }).keys ?? []; + const renames: string[] = []; + const prescriptions: string[] = []; + for (const key of keys) { + const prescription = guidance[key]; + if (prescription) { + prescriptions.push(prescription); + continue; + } + const maxDistance = Math.max(2, Math.floor(key.length / 3)); + const canonical = + aliases[aliasProbe(key)] ?? findClosestMatches(key, knownKeys, maxDistance, 1)[0]; + if (canonical && canonical !== key) renames.push(`\`${key}\` → \`${canonical}\``); + } + let message = + `Unrecognized key(s) on ${surface}: ${keys.map((k) => `\`${k}\``).join(', ')}. ${history}`; + if (renames.length) message += ` Did you mean ${renames.join(', ')}?`; + if (prescriptions.length) message += `\n${prescriptions.map((p) => ` • ${p}`).join('\n')}`; + return message; + }; +} diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index 1457b0f003..f6a92f80b8 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -10,7 +10,7 @@ import { HookBodySchema } from '../data/hook-body.zod'; // Imported file-directly (not via the kernel barrel): the module is // deliberately import-free, so this cannot introduce a cycle. import { PUBLIC_AUTH_FEATURE_NAMES, lowerRequiresFeature } from '../kernel/public-auth-features'; -import { findClosestMatches } from '../shared/suggestions.zod'; +import { strictUnknownKeyError } from '../shared/suggestions.zod'; /** * Action Parameter Schema @@ -67,10 +67,11 @@ const ACTION_PARAM_KEYS = [ * borrowed from a neighbouring schema where that word is correct. Edit distance * cannot reach these (`visibleWhen` → `visible` is 4 apart), so they are named * explicitly; plain case/underscore slips (`help_text` → `helpText`) are left to - * {@link findClosestMatches}. Mirrors the `FIELD_TYPE_ALIASES` pattern in - * `shared/suggestions.zod.ts`. + * the factory's edit-distance fallback. Mirrors the `FIELD_TYPE_ALIASES` + * pattern in `shared/suggestions.zod.ts`. * - * Keys are normalised by {@link aliasProbe} — lowercase, separators removed. + * Keys are matched case-insensitively with separators removed (see + * {@link strictUnknownKeyError}). */ const ACTION_PARAM_KEY_ALIASES: Readonly> = { // The objectql/runtime field shape spells a lookup target `reference_to`, and @@ -91,9 +92,6 @@ const ACTION_PARAM_KEY_ALIASES: Readonly> = { default: 'defaultValue', }; -/** `reference_to` / `referenceTo` / `Reference-To` all collapse onto one probe. */ -const aliasProbe = (key: string): string => key.toLowerCase().replace(/[_\-\s]/g, ''); - /** * Custom zod `error` for the `.strict()` {@link ActionParamSchema} (#3405 part 3). * @@ -103,28 +101,19 @@ const aliasProbe = (key: string): string => key.toLowerCase().replace(/[_\-\s]/g * UUID, with no error anywhere — the config was eaten and the UI lied about why * (ADR-0078 no-silently-inert-metadata, ADR-0049 enforce-or-remove). * - * Strict alone would only say "unrecognized key". This map makes the rejection - * *fixable*: it names the offending key(s) and, when one is a recognisable - * spelling of a declared key, points at the canonical one. + * Built by {@link strictUnknownKeyError} — the shared factory this schema's + * hand-rolled #3746 map was generalized into (#4001): it names the offending + * key(s) and, when one is a recognisable spelling of a declared key, points at + * the canonical one. */ -const actionParamUnknownKeyError: z.core.$ZodErrorMap = (issue) => { - if (issue.code !== 'unrecognized_keys') return undefined; - const keys = (issue as { keys?: readonly string[] }).keys ?? []; - const suggestions = keys.flatMap((key) => { - // Length-relative bound, matching `suggestKey` in `data/object.zod.ts`: a - // flat distance of 3 is noise on a short key (`wibble` → `visible`). - const maxDistance = Math.max(2, Math.floor(key.length / 3)); - const canonical = - ACTION_PARAM_KEY_ALIASES[aliasProbe(key)] ?? - findClosestMatches(key, ACTION_PARAM_KEYS, maxDistance, 1)[0]; - return canonical && canonical !== key ? [`\`${key}\` → \`${canonical}\``] : []; - }); - const base = - `Unrecognized key(s) on this action param: ${keys.map((k) => `\`${k}\``).join(', ')}. ` + - `Until #3405 these were dropped silently — the param still parsed, so a mis-spelled ` + - `config shipped as a control that quietly ignored it.`; - return suggestions.length ? `${base} Did you mean ${suggestions.join(', ')}?` : base; -}; +const actionParamUnknownKeyError = strictUnknownKeyError({ + surface: 'this action param', + knownKeys: ACTION_PARAM_KEYS, + aliases: ACTION_PARAM_KEY_ALIASES, + history: + 'Until #3405 these were dropped silently — the param still parsed, so a mis-spelled ' + + 'config shipped as a control that quietly ignored it.', +}); export const ActionParamSchema = lazySchema(() => z.object({ diff --git a/skills/objectstack-automation/references/_index.md b/skills/objectstack-automation/references/_index.md index 9e781455f1..5e85df2ae6 100644 --- a/skills/objectstack-automation/references/_index.md +++ b/skills/objectstack-automation/references/_index.md @@ -21,10 +21,13 @@ from `node_modules` — there is no local copy in the skill bundle. ## Transitive dependencies +- `node_modules/@objectstack/spec/src/data/field.zod.ts` — Field Type Enum +- `node_modules/@objectstack/spec/src/data/filter.zod.ts` — Unified Query DSL Specification - `node_modules/@objectstack/spec/src/kernel/metadata-protection.zod.ts` — Metadata Protection Model — Phase 1 (ADR-0010) - `node_modules/@objectstack/spec/src/shared/expression.zod.ts` — Expression Protocol - `node_modules/@objectstack/spec/src/shared/identifiers.zod.ts` — System Identifier Schema - `node_modules/@objectstack/spec/src/shared/protection.zod.ts` — Package-level metadata protection (ADR-0010 §3.7 — Phase 4.3) +- `node_modules/@objectstack/spec/src/shared/suggestions.zod.ts` — "Did you mean?" Suggestion Utilities ## How to read these From 5501533a2f09dfbd666ed095106395a73ec077c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 08:29:07 +0000 Subject: [PATCH 2/2] fix(spec,dogfood): declare the protection envelope on permission sets; drop a dead flow key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dogfood gate caught two more instances of the exact class #4001 targets — both invisible until the .strict() gate made them loud: - PermissionSetSchema could not represent the ADR-0010 runtime protection envelope. MetadataPlugin's artifact loader calls applyProtection on EVERY metadata type, and getMetaItemLayered -> saveMetaItem round-trips a body carrying the stamped _packageId/_provenance; every sibling registered type (object/view/app/dashboard/report/dataset/flow/agent/tool/skill/ email_template) spreads MetadataProtectionFields and permission was the outlier, so the envelope was stripped at every parse. Now declared, with the author-facing `protection` block alongside it. Without this a package-owned set cannot round-trip through an ADR-0094 environment overlay. - flow-touch-fixture.ts declared `sharingModel` on a FLOW — an object-level OWD key copy-pasted with an ADR-0090 grandfather-stamp comment describing a gate flows do not have. Authored in good faith, silently discarded, believed to be in effect. Removed; the identical stamp on the fixture's object is real. Verified: dogfood 71 files / 410 tests pass locally; spec 6961 tests; all 12 spec check gates green. Refs #4001 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0147tNF4Snk7Ry1KGt4a5PY4 --- .changeset/unknown-key-strictness-tier-a.md | 13 ++++-- .../docs/references/security/permission.mdx | 8 ++++ content/docs/releases/v17.mdx | 12 +++-- .../2026-07-unknown-key-strictness-ledger.md | 44 +++++++++++++++---- .../test/fixtures/flow-touch-fixture.ts | 3 -- packages/spec/authorable-surface.json | 8 ++++ packages/spec/src/security/permission.test.ts | 19 ++++++++ packages/spec/src/security/permission.zod.ts | 29 ++++++++++++ 8 files changed, 118 insertions(+), 18 deletions(-) diff --git a/.changeset/unknown-key-strictness-tier-a.md b/.changeset/unknown-key-strictness-tier-a.md index 404728a018..44e92752b4 100644 --- a/.changeset/unknown-key-strictness-tier-a.md +++ b/.changeset/unknown-key-strictness-tier-a.md @@ -27,10 +27,15 @@ authorable surfaces, per the ADR-0054 ratchet and the #3746 hand-rolled map, generalized): every rejection names the offending key(s) and, where recognisable, the canonical spelling or a retired-key tombstone. `ui/action.zod.ts` re-homes onto it with byte-identical messages. -- **`PermissionSetSchema.description` is now a declared key.** The strict - gate's first catch: the built-in default sets always authored it and the - Setup projection reads it, but the schema could not represent it, so it was - silently stripped at every parse (ADR-0078 §3 inverse drift). +- **`PermissionSetSchema` gains `description`, `protection` and the ADR-0010 + runtime protection envelope (`_lock`, `_packageId`, `_provenance`, …).** The + strict gate's own catches: all of these are written by real code — the + built-in default sets author `description` and the Setup projection reads + it; `applyProtection` stamps the envelope on every metadata type and + `getMetaItemLayered` → `saveMetaItem` round-trips it — but the schema could + not represent them, so they were silently stripped at every parse (ADR-0078 + §3 inverse drift). Every sibling registered metadata type already spread + `MetadataProtectionFields`; permission was the outlier. **Migration.** Any key these schemas now reject was previously stripped and therefore had **no runtime effect** — removing or renaming it never changes diff --git a/content/docs/references/security/permission.mdx b/content/docs/references/security/permission.mdx index d8a879d32f..f3dfd541c3 100644 --- a/content/docs/references/security/permission.mdx +++ b/content/docs/references/security/permission.mdx @@ -137,6 +137,14 @@ const result = AdminScope.parse(data); | **tabPermissions** | `Record>` | optional | App/tab visibility: visible, hidden, default_on (shown by default), default_off (available but hidden initially) | | **rowLevelSecurity** | `{ name: string; label?: string; description?: string; object: string; … }[]` | optional | Row-level security policies (see rls.zod.ts for full spec) | | **adminScope** | `{ businessUnit: string; includeSubtree: boolean; manageAssignments: boolean; manageBindings: boolean; … }` | optional | [ADR-0090 D12] Scoped delegated-administration grant (BU subtree + assignable-set allowlist) | +| **protection** | `{ lock: Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>; reason: string; docsUrl?: string }` | optional | Package author protection block — lock policy for this permission set. | +| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | +| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | +| **_lockSource** | `Enum<'artifact' \| 'package' \| 'env-forced'>` | optional | Layer that set _lock (artifact \| package \| env-forced). | +| **_provenance** | `Enum<'package' \| 'org' \| 'env-forced'>` | optional | Origin of the item (package \| org \| env-forced). | +| **_packageId** | `string` | optional | Owning package machine id. | +| **_packageVersion** | `string` | optional | Owning package version. | +| **_lockDocsUrl** | `string` | optional | Optional documentation link surfaced next to _lockReason. | --- diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index 8bc18b29e3..e799257947 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -336,9 +336,15 @@ resolvers; `isProfile` → `isDefault`, ADR-0090 D2). Migration is by construction behavior-preserving: any key now rejected was previously stripped, so it never had a runtime effect — remove it or rename it to the canonical key the error suggests; nothing about a working app changes. -One gap the gate itself caught: `PermissionSetSchema` gains the previously -unrepresentable-but-consumed **`description`** key (persisted to -`sys_permission_set.description`, shown in Setup). + +The gate paid for itself immediately by exposing keys the platform *writes* +but the spec could not express, so `PermissionSetSchema` gains three: the +**`description`** shown in Setup (persisted to +`sys_permission_set.description`), the author-facing **`protection`** block, +and the ADR-0010 **runtime protection envelope** (`_lock`, `_packageId`, +`_provenance`, …) that every sibling metadata type already carried — without +it, a package-owned permission set could not round-trip through an +environment overlay. ### `agent.tools[]` is removed — capability comes from skills (ADR-0109, #3820) diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 1060d6e40b..48fbe4705f 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -67,14 +67,42 @@ the first application of this gate (below) caught a real spec gap. | `FlowSchema`, `FlowNodeSchema`, `FlowEdgeSchema`, `FlowVariableSchema` | `automation/flow.zod.ts` | Most AI-authored surface; cloud#688 / #2419 class. Node `config` stays **open** (executor `configSchema` owns it, #4027/#4040) | | `ActionParamSchema` re-homed onto the shared factory | `ui/action.zod.ts` | #3746 template, byte-identical messages | -**Findings log** (what the gate caught the day it was turned on): -`PermissionSetSchema` could not represent `description` — yet the built-in -default sets author it and `plugin-security`'s Setup projection reads it -(`permission-set-projection.ts`). It had been silently stripped at every parse -(the ADR-0078 §3 inverse-drift class). Fixed contract-first: `description` is -now a declared key. This is the empirical argument for the ratchet: the -inference "no metadata in the repo carries unknown keys" was **false**, and -only the strict gate could prove it. +**Findings log** — three real defects the gate caught in its first two runs. +All three had been invisible: each key was written by real code, silently +dropped at parse, and nothing failed. + +1. **`PermissionSetSchema` could not represent `description`** — yet the + built-in default sets author it and `plugin-security`'s Setup projection + reads it (`permission-set-projection.ts`). Fixed contract-first: it is now + a declared key. +2. **`PermissionSetSchema` could not represent the ADR-0010 protection + envelope** — `MetadataPlugin`'s artifact loader calls `applyProtection` on + **every** metadata type, and `getMetaItemLayered` → `saveMetaItem` + round-trips a body carrying the stamped `_packageId` / `_provenance`. Every + sibling registered type (object / view / app / dashboard / report / + dataset / flow / agent / tool / skill / email_template) spreads + `MetadataProtectionFields`; permission was the outlier, so the envelope was + stripped on every parse. Now declared, with the author-facing `protection` + block alongside it (translated generically by `applyProtection`). + Caught by the dogfood gate as a hard 422 on the ADR-0094 overlay path. +3. **A dogfood fixture flow carried `sharingModel`** — an object-level OWD key + copy-pasted onto `flowTouch` in `flow-touch-fixture.ts`, complete with an + ADR-0090 "grandfather stamp" comment describing a gate that does not exist + for flows. Exactly the #4001 failure mode in the wild: authored in good + faith, silently discarded, believed to be in effect. Removed (the identical + stamp on the fixture's *object* is real and stays). + +This is the empirical argument for the ratchet: the inference "no metadata in +the repo carries unknown keys" was **false three times over**, and only the +strict gate could prove it. Note the asymmetry in the two schema gaps — both +were *inverse* drift (runtime writes a key the spec cannot express), which the +liveness ledger's per-property direction cannot see. + +**Known sibling gap (follow-up, not this step):** `identity/position.zod.ts` +— the other registered security type — also omits `MetadataProtectionFields` +while `applyProtection` stamps it. Harmless today because the schema is not +strict (the keys are silently stripped, as permission's were), but it must be +declared before `position` joins the ratchet. ## File-level triage — the five authorable directories diff --git a/packages/qa/dogfood/test/fixtures/flow-touch-fixture.ts b/packages/qa/dogfood/test/fixtures/flow-touch-fixture.ts index eb237a138b..a01cabbff4 100644 --- a/packages/qa/dogfood/test/fixtures/flow-touch-fixture.ts +++ b/packages/qa/dogfood/test/fixtures/flow-touch-fixture.ts @@ -44,9 +44,6 @@ export const FlowNote = ObjectSchema.create({ */ export const flowTouch = { name: 'flow_touch', - // [ADR-0090 D1] grandfather stamp: this fixture's gate under test is - // permission-set RLS / flow scoping, not owner-sharing. - sharingModel: 'public_read_write', label: 'Flow Touch', type: 'autolaunched', variables: [{ name: 'noteId', type: 'text', isInput: true }], diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 724f32d2b6..c8412a04b0 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -5720,6 +5720,13 @@ "security/ObjectPermission:readScope", "security/ObjectPermission:viewAllRecords", "security/ObjectPermission:writeScope", + "security/PermissionSet:_lock", + "security/PermissionSet:_lockDocsUrl", + "security/PermissionSet:_lockReason", + "security/PermissionSet:_lockSource", + "security/PermissionSet:_packageId", + "security/PermissionSet:_packageVersion", + "security/PermissionSet:_provenance", "security/PermissionSet:adminScope", "security/PermissionSet:description", "security/PermissionSet:fields", @@ -5729,6 +5736,7 @@ "security/PermissionSet:name", "security/PermissionSet:objects", "security/PermissionSet:packageId", + "security/PermissionSet:protection", "security/PermissionSet:rowLevelSecurity", "security/PermissionSet:systemPermissions", "security/PermissionSet:tabPermissions", diff --git a/packages/spec/src/security/permission.test.ts b/packages/spec/src/security/permission.test.ts index 53e2fc30ad..70cb8d2d51 100644 --- a/packages/spec/src/security/permission.test.ts +++ b/packages/spec/src/security/permission.test.ts @@ -596,6 +596,24 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { expect(message).toContain('`isDefault`'); }); + it('round-trips the ADR-0010 runtime protection envelope', () => { + // `MetadataPlugin`'s artifact loader calls `applyProtection` on EVERY + // metadata type, and `getMetaItemLayered` → `saveMetaItem` round-trips a + // body carrying the stamped envelope. Until #4001 the schema could not + // represent these keys, so they were stripped at every parse. + const parsed = PermissionSetSchema.parse({ + name: 'showcase_contributor', + objects: {}, + _packageId: 'com.showcase', + _packageVersion: '1.0.0', + _provenance: 'package', + _lock: 'full', + }); + expect(parsed._packageId).toBe('com.showcase'); + expect(parsed._provenance).toBe('package'); + expect(parsed._lock).toBe('full'); + }); + it('points wrong-layer keys (profiles/roles/users) at the binding mechanism', () => { for (const key of ['profiles', 'roles', 'users']) { const message = unknownKeyIssue(PermissionSetSchema, { @@ -612,6 +630,7 @@ describe('unknown keys are rejected, not stripped (#4001)', () => { objects: { task: { allowRead: true } }, fields: { 'task.secret': { readable: false } }, systemPermissions: ['manage_users'], tabPermissions: { app_crm: 'visible' }, rowLevelSecurity: [], adminScope: { businessUnit: 'east' }, + protection: { lock: 'none' }, }; for (const [key, value] of Object.entries(probes)) { const result = PermissionSetSchema.safeParse({ name: 'p', objects: {}, [key]: value }); diff --git a/packages/spec/src/security/permission.zod.ts b/packages/spec/src/security/permission.zod.ts index e2371df35f..724028118e 100644 --- a/packages/spec/src/security/permission.zod.ts +++ b/packages/spec/src/security/permission.zod.ts @@ -4,6 +4,8 @@ import { z } from 'zod'; import { SnakeCaseIdentifierSchema } from '../shared/identifiers.zod'; import { RowLevelSecurityPolicySchema } from './rls.zod'; import { ApiOperationSchema } from '../data/object.zod'; +import { ProtectionSchema } from '../shared/protection.zod'; +import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; /** * Entity (Object) Level Permissions @@ -333,6 +335,10 @@ export const FieldPermissionSchema = lazySchema(() => z.object({ const PERMISSION_SET_KEYS = [ 'name', 'label', 'description', 'packageId', 'managedBy', 'isDefault', 'objects', 'fields', 'systemPermissions', 'tabPermissions', 'rowLevelSecurity', 'adminScope', + 'protection', + // ADR-0010 runtime protection envelope (MetadataProtectionFields spread). + '_lock', '_lockReason', '_lockSource', '_provenance', '_packageId', + '_packageVersion', '_lockDocsUrl', ] as const; /** Semantic near-misses borrowed from neighbouring schemas / products. */ @@ -513,6 +519,29 @@ export const PermissionSetSchema = lazySchema(() => z.object({ */ adminScope: AdminScopeSchema.optional() .describe('[ADR-0090 D12] Scoped delegated-administration grant (BU subtree + assignable-set allowlist)'), + + /** + * ADR-0010 §3.7 — Package-level protection envelope. Package authors declare + * lock policy here; the loader translates it into the private `_lock` + * envelope at registration time and strips this block before persistence. + * See `shared/protection.zod.ts`. + */ + protection: ProtectionSchema.optional().describe( + 'Package author protection block — lock policy for this permission set.', + ), + + // ADR-0010 — runtime protection envelope (internal — set by loader). + // + // Declared in #4001 alongside every sibling registered metadata type + // (object / view / app / dashboard / report / dataset / flow / agent / tool / + // skill / email_template). `MetadataPlugin`'s artifact loader calls + // `applyProtection` on EVERY type, so a permission set has always carried + // these keys at runtime — and `getMetaItemLayered` → `saveMetaItem` + // round-trips a body that includes them. The schema simply could not + // represent them, so they were silently stripped at every parse (the same + // ADR-0078 §3 inverse-drift class as `description`); the `.strict()` gate + // turned that into a visible 422 and surfaced the gap. + ...MetadataProtectionFields, }, { error: permissionSetUnknownKeyError }).strict()); export type PermissionSet = z.infer;