feat(spec): bind webhook/connector/sharing_rule schemas at the /meta write door (#6245)

.changeset/meta-bind-unregistered-kinds.md

@@ -0,0 +1,59 @@
+---
+"@objectstack/spec": minor
+---
+
+fix(spec): `webhook` / `connector` / `sharing_rule` are validated at the `/meta` write door (#6245)
+
+Three stack collections could be written through `PUT /api/v1/meta/<type>/:name`
+with **zero validation**. A spec-invalid webhook body was stored verbatim and
+answered `success: true` — the repo pinned that behaviour itself
+(`protocol-meta.test.ts`, which saved `{ name, url, events: ['x.created'] }` and
+asserted success, where `events` is an alias of `triggers` and `'x.created'` is
+not a `WebhookTriggerType`; that webhook subscribed to nothing).
+
+This is the hole #5271 closed for `api`, arriving through three more doors:
+the kinds are produced and consumed today — artifact ingest maps
+`defineStack({ webhooks, connectors, sharingRules })` onto items of exactly
+these type names — but none is a member of `MetadataTypeSchema`, so
+`getMetadataTypeSchema()` returned `undefined`, `resolveOverlaySchema()`
+returned `null`, and `saveMetaItem` took its documented "unregistered type →
+store without validation" branch. Enforced but undeclared.
+
+**FROM** `PUT /meta/webhook/my_hook` with any JSON → `200 { success: true }`,
+stored unvalidated.
+**TO** a malformed body → `422 INVALID_METADATA` with structured `issues[]`,
+the same envelope every other kind already returned. A well-formed body is
+accepted exactly as before.
+
+Each type binds the **same schema its stack collection is validated against**,
+so no body can be legal in a stack and illegal through `/meta` or the reverse:
+`WebhookSchema`, `DeclarativeConnectorEntrySchema`, `SharingRuleSchema`.
+`connector` binds the *declarative entry* schema rather than the bare
+`ConnectorSchema` deliberately — the entry schema carries the ADR-0097 §3/§5
+rules (a provider-bound instance may not inline credentials via
+`authentication`, nor author `actions`/`triggers`), and binding the base would
+have left the inline-secret shape a stack refuses reachable through `/meta`,
+which is this very bug class wearing a different key.
+
+**No new capability surface.** These are bound for shape validation only: no
+`MetadataTypeSchema` member, no `DEFAULT_METADATA_TYPE_REGISTRY` entry, so
+every authorization verdict keeps taking the identical "no static entry ⇒
+synthesised `allowRuntimeCreate: true`" branch. The write *door* is unchanged;
+only the 422 is new. #2657's B/C decision on whether these should become kinds
+is untouched and unprejudged.
+
+Graded **minor**, following #5271: a write that previously returned 200 can now
+return 422. Nothing well-formed changes behaviour, but a caller relying on the
+API accepting malformed bodies will see the difference.
+
+**One schema change rides along, and it is load-bearing.**
+`CriteriaSharingRule` / `SharingRule` now declare the ADR-0010 protection
+envelope (`_lock`, `_lockReason`, `_lockSource`, `_lockDocsUrl`, `_packageId`,
+`_packageVersion`, `_provenance`). Both metadata load paths call
+`applyProtection` on **every** type, so a package-loaded sharing rule already
+carries those keys — and this shape is `.strict()`, so it did not drop them, it
+*rejected* them. That was invisible only while the type resolved no schema at
+the overlay door. Binding the door without this spread would have aimed the new
+422 at the runtime's own stamp instead of at malformed author input. The
+existing guard in `metadata-type-schemas.test.ts` names this failure exactly and
+prescribes this fix. Additive and internal-only — no authored field changes.

content/docs/references/security/sharing.mdx

@@ -37,6 +37,13 @@ const result = CriteriaSharingRuleSchema.parse(data);
 | **active** | `boolean` | optional |  |
 | **accessLevel** | `Enum<'read' \| 'edit'>` | optional |  |
 | **sharedWith** | `{ type: Enum<'user' \| 'team' \| 'position' \| 'unit_and_subordinates' \| 'business_unit'>; value: string }` | ✅ | The recipient of the shared access |
+| **_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. |
 | **type** | `'criteria'` | ✅ |  |
 | **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | ✅ | Predicate (CEL). e.g. P`record.department == "Sales"` |
 
@@ -91,6 +98,13 @@ const result = CriteriaSharingRuleSchema.parse(data);
 | **active** | `boolean` | optional |  |
 | **accessLevel** | `Enum<'read' \| 'edit'>` | optional |  |
 | **sharedWith** | `{ type: Enum<'user' \| 'team' \| 'position' \| 'unit_and_subordinates' \| 'business_unit'>; value: string }` | ✅ | The recipient of the shared access |
+| **_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. |
 | **type** | `'criteria'` | ✅ |  |
 | **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | ✅ | Predicate (CEL). e.g. P`record.department == "Sales"` |
 

packages/objectql/src/protocol-meta.test.ts

@@ -1445,8 +1445,19 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => {
         // would have kept the assertion green through the *other* branch of
         // `isRuntimeCreateAllowed` (statically registered with
         // `allowRuntimeCreate: true`) while the test claimed to prove the
-        // "no static entry" fall-through. `theme` and `webhook` still exercise
-        // that branch; `api`'s own behaviour is pinned in the test below.
+        // "no static entry" fall-through.
+        //
+        // [#6245] `webhook` left it too, for the SAME reason and by the same
+        // rule — but note the difference from `api`: webhook/connector/
+        // sharing_rule gained a SCHEMA, not a registry entry, so they still
+        // take this test's "no static entry" branch for AUTHORIZATION. What
+        // they no longer do is accept any JSON, which is what disqualifies the
+        // old specimen: its body (`events`, an alias of `triggers`, plus the
+        // non-enum trigger `'x.created'`) is spec-INVALID and now 422s. Kept
+        // here it would have flipped this test red for a reason that has
+        // nothing to do with what it proves. `theme` and `policy` still carry
+        // the branch — neither resolves a schema. Each newly-bound type's own
+        // behaviour, door and 422 both, is pinned in the tests below.
         // ───────────────────────────────────────────────────────────────
 
         it('accepts brand-new plugin-registered type (no static registry entry)', async () => {
@@ -1458,15 +1469,149 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => {
                 item: { name: 'my_theme', label: 'Test', tokens: {} },
                 organizationId: 'org_alpha',
             });
-            const webhookResult = await scoped.saveMetaItem({
+            const policyResult = await scoped.saveMetaItem({
+                type: 'policy',
+                name: 'my_policy',
+                item: { name: 'my_policy', label: 'Test' },
+                organizationId: 'org_alpha',
+            });
+
+            expect(themeResult.success).toBe(true);
+            expect(policyResult.success).toBe(true);
+        });
+
+        // ───────────────────────────────────────────────────────────────
+        // [#6245] `webhook` / `connector` / `sharing_rule` — the write door is
+        // UNCHANGED, the shape check is new. Same shape as the `api` pair
+        // below, and asserted the same way: a change that quietly CLOSED the
+        // door would fail the "accepts" half, which is why both halves are
+        // pinned per type rather than just the 422.
+        //
+        // These three resolve a schema WITHOUT a `DEFAULT_METADATA_TYPE_REGISTRY`
+        // entry, so `isRuntimeCreateAllowed` still takes its synthesised
+        // `allowRuntimeCreate: true` fall-through — the authorization verdict is
+        // byte-identical to before.
+        // ───────────────────────────────────────────────────────────────
+
+        it('accepts a spec-valid `webhook` item (write door unchanged by the schema binding)', async () => {
+            mockEngine.findOne.mockResolvedValue(null);
+
+            const result = await scoped.saveMetaItem({
                 type: 'webhook',
                 name: 'my_webhook',
-                item: { name: 'my_webhook', url: 'https://e.example/x', events: ['x.created'] },
+                item: {
+                    name: 'my_webhook',
+                    object: 'case',
+                    triggers: ['create'],
+                    url: 'https://e.example/x',
+                },
                 organizationId: 'org_alpha',
             });
 
-            expect(themeResult.success).toBe(true);
-            expect(webhookResult.success).toBe(true);
+            expect(result.success).toBe(true);
+        });
+
+        it('refuses a spec-INVALID `webhook` item with 422 instead of storing it unvalidated', async () => {
+            mockEngine.findOne.mockResolvedValue(null);
+
+            // The exact body the "plugin-registered types" case above used to
+            // save with `success: true`: `events` is an ALIAS of `triggers`
+            // (so the strict surface names the real key), and `'x.created'` is
+            // not a `WebhookTriggerType`. Stored verbatim, this webhook
+            // subscribed to nothing.
+            await expect(
+                scoped.saveMetaItem({
+                    type: 'webhook',
+                    name: 'my_webhook',
+                    item: { name: 'my_webhook', url: 'https://e.example/x', events: ['x.created'] },
+                    organizationId: 'org_alpha',
+                }),
+            ).rejects.toMatchObject({
+                code: 'INVALID_METADATA',
+                status: 422,
+            });
+        });
+
+        it('accepts a spec-valid `connector` item (write door unchanged by the schema binding)', async () => {
+            mockEngine.findOne.mockResolvedValue(null);
+
+            const result = await scoped.saveMetaItem({
+                type: 'connector',
+                name: 'my_connector',
+                item: { name: 'my_connector', label: 'My Connector', type: 'api' },
+                organizationId: 'org_alpha',
+            });
+
+            expect(result.success).toBe(true);
+        });
+
+        it('refuses a `connector` that breaks an ADR-0097 authoring rule with 422', async () => {
+            mockEngine.findOne.mockResolvedValue(null);
+
+            // Bound to `DeclarativeConnectorEntrySchema` — the same schema
+            // `stack.zod.ts` validates `connectors:` with — so the ADR-0097 §3
+            // rule that a provider-bound instance may not inline credentials
+            // holds at BOTH authoring doors, not just the stack one.
+            await expect(
+                scoped.saveMetaItem({
+                    type: 'connector',
+                    name: 'my_connector',
+                    item: {
+                        name: 'my_connector',
+                        label: 'My Connector',
+                        type: 'api',
+                        provider: 'github',
+                        authentication: { type: 'basic', username: 'u', password: 'p' },
+                    },
+                    organizationId: 'org_alpha',
+                }),
+            ).rejects.toMatchObject({
+                code: 'INVALID_METADATA',
+                status: 422,
+            });
+        });
+
+        it('accepts a spec-valid `sharing_rule` item (write door unchanged by the schema binding)', async () => {
+            mockEngine.findOne.mockResolvedValue(null);
+
+            const result = await scoped.saveMetaItem({
+                type: 'sharing_rule',
+                name: 'sales_records',
+                item: {
+                    name: 'sales_records',
+                    type: 'criteria',
+                    object: 'case',
+                    condition: "record.department == 'Sales'",
+                    sharedWith: { type: 'team', value: 'sales' },
+                },
+                organizationId: 'org_alpha',
+            });
+
+            expect(result.success).toBe(true);
+        });
+
+        it('refuses a spec-INVALID `sharing_rule` item with 422 instead of storing it unvalidated', async () => {
+            mockEngine.findOne.mockResolvedValue(null);
+
+            // No `condition` and no `object`: a criteria rule that names
+            // neither what it shares nor which records. Stored unvalidated this
+            // is the #3896 shape — a rule that binds to nothing, or worse
+            // compiles to the empty filter.
+            await expect(
+                scoped.saveMetaItem({
+                    type: 'sharing_rule',
+                    name: 'sales_records',
+                    item: {
+                        name: 'sales_records',
+                        type: 'criteria',
+                        sharedWith: { type: 'team', value: 'sales' },
+                    },
+                    organizationId: 'org_alpha',
+                }),
+            ).rejects.toMatchObject({
+                code: 'INVALID_METADATA',
+                status: 422,
+            });
         });
 
         // ───────────────────────────────────────────────────────────────

packages/spec/authorable-surface/security.json

@@ -26,6 +26,13 @@
     "security/CapabilityDeclaration:name",
     "security/CapabilityDeclaration:packageId",
     "security/CapabilityDeclaration:scope",
+    "security/CriteriaSharingRule:_lock",
+    "security/CriteriaSharingRule:_lockDocsUrl",
+    "security/CriteriaSharingRule:_lockReason",
+    "security/CriteriaSharingRule:_lockSource",
+    "security/CriteriaSharingRule:_packageId",
+    "security/CriteriaSharingRule:_packageVersion",
+    "security/CriteriaSharingRule:_provenance",
     "security/CriteriaSharingRule:accessLevel",
     "security/CriteriaSharingRule:active",
     "security/CriteriaSharingRule:condition",
@@ -133,6 +140,13 @@
     "security/RowLevelSecurityPolicy:priority [RETIRED]",
     "security/RowLevelSecurityPolicy:tags",
     "security/RowLevelSecurityPolicy:using",
+    "security/SharingRule:_lock",
+    "security/SharingRule:_lockDocsUrl",
+    "security/SharingRule:_lockReason",
+    "security/SharingRule:_lockSource",
+    "security/SharingRule:_packageId",
+    "security/SharingRule:_packageVersion",
+    "security/SharingRule:_provenance",
     "security/SharingRule:accessLevel",
     "security/SharingRule:active",
     "security/SharingRule:condition",