diff --git a/.changeset/connector-protection-envelope.md b/.changeset/connector-protection-envelope.md new file mode 100644 index 0000000000..09c4db6413 --- /dev/null +++ b/.changeset/connector-protection-envelope.md @@ -0,0 +1,47 @@ +--- +"@objectstack/spec": minor +--- + +fix(spec): `connector` preserves the ADR-0010 protection envelope instead of silently stripping it (#6362) + +`ConnectorSchema` tolerated the ADR-0010 protection envelope but never declared +it, so every package-load round-trip through the schema dropped +`_packageId` / `_packageVersion` / `_provenance` / `_lock` / `_lockReason` / +`_lockSource` / `_lockDocsUrl` — all seven keys, with no error anywhere. +Tolerate is not preserve. + +Both metadata load paths call `applyProtection` on **every** type, so a +package-loaded connector carries that envelope by the time anything re-parses +it — and since #6245 bound `DeclarativeConnectorEntrySchema` to +`PUT /api/v1/meta/connector/:name`, something re-parses it on every write. + +This is the quiet half of the pair #6245 fixed, and being quiet is why it +outlived its siblings. `sharing_rule` is `.strict()`, so its undeclared +envelope was **rejected** — a hard 422, loud, fixed in #6245 the moment the +door was bound. `ConnectorSchema` is a plain (non-strict) `z.object`, so it +**accepted** the same envelope, answered `success`, and stripped it from the +output. Every downstream reader of `extractProtection` / `resolveLockState` +therefore saw an unlocked, unattributed, `org`-provenance connector where the +loader had stamped a locked, package-owned one. + +FROM: a stamped connector round-tripping through the schema came back having +lost all seven envelope keys, silently. +TO : all seven survive, by value, on the base schema, on the `/meta` write +door, and through the metadata registry's own lookup. + +The fix is the one #6245's dev verified on `sharing_rule` — a single +`...MetadataProtectionFields` spread. Pure-additive and internal: every key is +`_`-prefixed and optional, no author-facing field changes, and nothing that +parsed before stops parsing. `ConnectorSchema` stays non-strict, so an +undeclared `_`-prefixed key is still stripped rather than accepted; the spread +adds seven named keys, not a passthrough. + +**`webhook` was measured in the same pass and needs no change.** The issue +asked whether it had the same drop; it does not. `WebhookSchema` has carried +this spread since #4001 batch 11 and all seven keys already survive its +round-trip. That reading is now pinned by a test rather than left as a note, +so it cannot regress unobserved. + +The generated authorable-surface baselines gain the seven `_`-prefixed keys +under `integration/Connector`, matching what #6245 recorded for +`security/SharingRule` and `security/CriteriaSharingRule`. diff --git a/content/docs/references/integration/connector.mdx b/content/docs/references/integration/connector.mdx index 1047a4ecbc..b7ee9760ca 100644 --- a/content/docs/references/integration/connector.mdx +++ b/content/docs/references/integration/connector.mdx @@ -183,6 +183,13 @@ Circuit breaker configuration | **errorMapping** | `{ rules: object[]; defaultCategory?: Enum<'validation' \| 'authorization' \| 'not_found' \| 'conflict' \| 'rate_limit' \| … +3 more>; unmappedBehavior: Enum<'passthrough' \| 'generic_error' \| 'throw'>; logUnmapped?: boolean }` | optional | Error mapping configuration | | **health** | `{ healthCheck?: object; circuitBreaker?: object }` | optional | Health and resilience configuration | | **metadata** | `Record` | optional | Custom connector metadata | +| **_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. | --- @@ -498,6 +505,13 @@ Connector type | **errorMapping** | `{ rules: object[]; defaultCategory?: Enum<'validation' \| 'authorization' \| 'not_found' \| 'conflict' \| 'rate_limit' \| … +3 more>; unmappedBehavior: Enum<'passthrough' \| 'generic_error' \| 'throw'>; logUnmapped?: boolean }` | optional | Error mapping configuration | | **health** | `{ healthCheck?: object; circuitBreaker?: object }` | optional | Health and resilience configuration | | **metadata** | `Record` | optional | Custom connector metadata | +| **_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/packages/spec/authorable-surface/integration.json b/packages/spec/authorable-surface/integration.json index fa287d4e93..5330541da1 100644 --- a/packages/spec/authorable-surface/integration.json +++ b/packages/spec/authorable-surface/integration.json @@ -8,6 +8,13 @@ "integration/CircuitBreakerConfig:halfOpenMaxRequests", "integration/CircuitBreakerConfig:monitoringWindow", "integration/CircuitBreakerConfig:resetTimeoutMs", + "integration/Connector:_lock", + "integration/Connector:_lockDocsUrl", + "integration/Connector:_lockReason", + "integration/Connector:_lockSource", + "integration/Connector:_packageId", + "integration/Connector:_packageVersion", + "integration/Connector:_provenance", "integration/Connector:actions", "integration/Connector:auth", "integration/Connector:authentication", @@ -70,6 +77,13 @@ "integration/DataSyncConfig:schedule", "integration/DataSyncConfig:strategy", "integration/DataSyncConfig:timestampField", + "integration/DeclarativeConnectorEntry:_lock", + "integration/DeclarativeConnectorEntry:_lockDocsUrl", + "integration/DeclarativeConnectorEntry:_lockReason", + "integration/DeclarativeConnectorEntry:_lockSource", + "integration/DeclarativeConnectorEntry:_packageId", + "integration/DeclarativeConnectorEntry:_packageVersion", + "integration/DeclarativeConnectorEntry:_provenance", "integration/DeclarativeConnectorEntry:actions", "integration/DeclarativeConnectorEntry:auth", "integration/DeclarativeConnectorEntry:authentication", diff --git a/packages/spec/src/automation/webhook.test.ts b/packages/spec/src/automation/webhook.test.ts index d0d7d1458c..307d7f602e 100644 --- a/packages/spec/src/automation/webhook.test.ts +++ b/packages/spec/src/automation/webhook.test.ts @@ -275,6 +275,45 @@ describe('unknown keys are rejected, not stripped (#4001 batch 11)', () => { expect(parsed._provenance).toBe('package'); }); + /** + * [#6362] The measurement that card asked for, kept as a pin. + * + * #6362 fixed `connector`, which TOLERATED the stamped envelope and then + * stripped it — success with silent data loss — and asked whether `webhook` + * had the same drop. It does not: `WebhookSchema` has carried + * `...MetadataProtectionFields` since #4001 batch 11, and all SEVEN keys + * survive the round-trip. No spread was added here; this pin is what makes + * that reading durable rather than a note in a closed report. + * + * The test above is the one #4001 left, and it checks two keys by value — + * enough for "accepts", not enough for "preserves". The whole `connector` + * defect was invisible to an accepts-shaped assertion, so the preservation + * question gets an assertion over the complete key set, by value. + */ + it('[#6362] PRESERVES all seven envelope keys — measured, not assumed', () => { + const envelope = { + _lock: 'full', + _lockReason: 'Ships with the package.', + _lockSource: 'artifact', + _lockDocsUrl: 'https://docs.example.com/locked-webhooks', + _provenance: 'package', + _packageId: 'com.example.app', + _packageVersion: '1.0.0', + } as const; + + const parsed = WebhookSchema.parse({ ...valid, ...envelope }); + + expect({ + _lock: parsed._lock, + _lockReason: parsed._lockReason, + _lockSource: parsed._lockSource, + _lockDocsUrl: parsed._lockDocsUrl, + _provenance: parsed._provenance, + _packageId: parsed._packageId, + _packageVersion: parsed._packageVersion, + }).toEqual(envelope); + }); + it('accepts an authored `protection` block (the pre-envelope half of ADR-0010)', () => { expect(WebhookSchema.safeParse({ ...valid, diff --git a/packages/spec/src/integration/connector.test.ts b/packages/spec/src/integration/connector.test.ts index 73deea43ad..004c5e8d8f 100644 --- a/packages/spec/src/integration/connector.test.ts +++ b/packages/spec/src/integration/connector.test.ts @@ -40,8 +40,15 @@ import { type ConnectorFieldMapping, type DataSyncConfig, type WebhookConfig, + + // The `/meta/connector/:name` door's schema (#6245) — `ConnectorSchema` plus + // the ADR-0097 cross-field rules. The envelope pins below drive BOTH, because + // this is the shape the round-trip actually goes through. + DeclarativeConnectorEntrySchema, } from './connector.zod'; +import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas'; + // Import shared auth schemas from canonical source import { ConnectorAPIKeySchema as APIKeySchema, @@ -1203,3 +1210,144 @@ describe('[#4703] FieldMapping no longer names three declarations', () => { expect(conflicts.sort()).toEqual([]); }); }); + +// ============================================================================ +// ADR-0010 protection envelope — PRESERVED on round-trip, not merely tolerated +// (#6362, split out of #6245) +// ============================================================================ + +/** + * The defect these pins hold shut is the QUIET half of the envelope-handling + * pair, and it is quiet in a way that reads as success. + * + * Both metadata load paths call `applyProtection` on EVERY type, so a + * package-loaded connector carries the seven `_`-prefixed envelope keys by the + * time anything re-parses it — and since #6245 bound + * `DeclarativeConnectorEntrySchema` to `PUT /api/v1/meta/connector/:name`, + * something does re-parse it on every write. + * + * `sharing_rule` is `.strict()`, so its undeclared envelope was REJECTED: a + * hard 422, loud, fixed in #6245. `ConnectorSchema` is a plain `z.object`, so + * it TOLERATED the same envelope and answered `success` — then stripped every + * key from the output. Tolerate is not preserve. Measured on `origin/main` + * before the `...MetadataProtectionFields` spread: a stamped catalog descriptor + * came back having lost all seven keys, with no error anywhere, so every + * downstream reader of `extractProtection` / `resolveLockState` saw an + * unlocked, unattributed, org-provenance item. + * + * ⚠️ Assert the VALUES, key by key — not `success`, and not "some `_` key + * survived". A presence-only or parses-only assertion is green on the very + * shape this issue is about: the unfixed schema parses that body perfectly + * well. The whole defect lives in the output, so the output is what gets + * asserted. + */ +const STAMPED_ENVELOPE = { + _lock: 'no-overlay', + _lockReason: 'Ships with the billing package.', + _lockSource: 'artifact', + _lockDocsUrl: 'https://docs.example.com/locked-connectors', + _provenance: 'package', + _packageId: 'com.acme.billing', + _packageVersion: '1.2.3', +} as const; + +/** + * A catalog descriptor: no `provider`, so the ADR-0097 §3/§5 instance rules do + * not fire and the entry schema judges exactly what the base schema does. That + * keeps these pins about the envelope and nothing else. + */ +const STAMPED_CONNECTOR = { + name: 'billing_api', + label: 'Billing API', + type: 'api', + ...STAMPED_ENVELOPE, +} as const; + +describe('ADR-0010 protection envelope (#6362)', () => { + it('ConnectorSchema PRESERVES every stamped envelope key through a parse', () => { + const parsed = ConnectorSchema.parse(STAMPED_CONNECTOR); + + // The reverse-verification target: remove `...MetadataProtectionFields` + // from `ConnectorSchema` and this object is `{}` — every key stripped — + // while the parse above still succeeds. + expect({ + _lock: parsed._lock, + _lockReason: parsed._lockReason, + _lockSource: parsed._lockSource, + _lockDocsUrl: parsed._lockDocsUrl, + _provenance: parsed._provenance, + _packageId: parsed._packageId, + _packageVersion: parsed._packageVersion, + }).toEqual(STAMPED_ENVELOPE); + }); + + it('the /meta write door (DeclarativeConnectorEntrySchema) preserves it too', () => { + // #6245 bound this shape, not the base, to `PUT /meta/connector/:name`. + // The base carrying the spread is only half an answer if the door's own + // schema drops it, so the door is pinned separately. + const parsed = DeclarativeConnectorEntrySchema.parse(STAMPED_CONNECTOR); + expect(parsed._packageId).toBe('com.acme.billing'); + expect(parsed._provenance).toBe('package'); + expect(parsed._lock).toBe('no-overlay'); + expect(parsed._lockDocsUrl).toBe('https://docs.example.com/locked-connectors'); + }); + + it('the schema the metadata registry resolves for `connector` preserves it', () => { + // The registry lookup is the real entry point — a future rebinding that + // pointed `connector` at some third shape would pass both pins above and + // still strip the envelope in production. + const schema = getMetadataTypeSchema('connector'); + expect(schema, 'no schema bound for `connector`').toBeDefined(); + + const result = schema!.safeParse(STAMPED_CONNECTOR); + expect(result.success).toBe(true); + + const out = result.success ? (result.data as Record) : {}; + const survived = Object.keys(STAMPED_ENVELOPE).filter((k) => k in out); + expect(survived.sort()).toEqual(Object.keys(STAMPED_ENVELOPE).sort()); + }); + + it('a provider-bound instance keeps the envelope alongside the §3/§5 rules', () => { + // The envelope must not become a casualty of the cross-field rules: an + // instance declaration is package-loaded too, and it is the shape most + // likely to be locked. + const parsed = DeclarativeConnectorEntrySchema.parse({ + name: 'billing_openapi', + label: 'Billing (OpenAPI)', + type: 'api', + provider: 'openapi', + providerConfig: { spec: './billing-openapi.json' }, + auth: { type: 'bearer', credentialRef: 'BILLING_TOKEN' }, + ...STAMPED_ENVELOPE, + }); + expect(parsed._packageId).toBe('com.acme.billing'); + expect(parsed._lockSource).toBe('artifact'); + }); + + it('declaring the envelope did not open the schema to arbitrary `_` keys', () => { + // `ConnectorSchema` is deliberately non-strict (subtypes `.extend()` it), + // so an unknown key is stripped rather than refused. The point of this pin + // is the converse of the ones above: the spread adds SEVEN named keys, not + // a passthrough — an underscore key nobody declared still does not survive. + const parsed = ConnectorSchema.parse({ + ...STAMPED_CONNECTOR, + _notAnEnvelopeKey: 'should not survive', + }); + expect('_notAnEnvelopeKey' in parsed).toBe(false); + expect(parsed._packageId).toBe('com.acme.billing'); + }); + + it('WebhookConfigSchema — the nested webhook — preserves it as well', () => { + // `WebhookConfigSchema` extends `WebhookSchema`, which has carried the + // spread since #4001 batch 11. Pinned here so the inherited behaviour + // cannot regress silently through a future `.extend()`/`.omit()` on the + // connector side. + const parsed = WebhookConfigSchema.parse({ + name: 'billing_events', + url: 'https://example.com/hooks/billing', + ...STAMPED_ENVELOPE, + }); + expect(parsed._packageId).toBe('com.acme.billing'); + expect(parsed._provenance).toBe('package'); + }); +}); diff --git a/packages/spec/src/integration/connector.zod.ts b/packages/spec/src/integration/connector.zod.ts index 8803e4b565..d9c4315032 100644 --- a/packages/spec/src/integration/connector.zod.ts +++ b/packages/spec/src/integration/connector.zod.ts @@ -5,6 +5,7 @@ import { CronExpressionInputSchema } from '../shared/expression.zod'; import { WebhookSchema } from '../automation/webhook.zod'; import { ConnectorAuthConfigSchema, ConnectorInstanceAuthSchema } from '../shared/connector-auth.zod'; import { FieldMappingSchema as BaseFieldMappingSchema } from '../shared/mapping.zod'; +import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { retiredKey } from '../shared/retired-key'; /** @@ -846,6 +847,36 @@ export const ConnectorSchema = lazySchema(() => z.object({ * Custom metadata */ metadata: z.record(z.string(), z.unknown()).optional().describe('Custom connector metadata'), + + // ADR-0010 — runtime protection envelope (internal — set by loader). + // + // [#6362, split out of #6245] Declared for the reason `webhook.zod.ts` and + // `sharing.zod.ts` state for their own spreads: BOTH metadata load paths call + // `applyProtection` on EVERY type, so a package-loaded connector already + // carries these keys by the time anything re-parses it — and `/meta/connector` + // has re-parsed them since #6245 bound `DeclarativeConnectorEntrySchema` to + // that door. + // + // The failure mode here is the QUIET half of the pair, which is exactly why + // it outlived its two siblings. `sharing_rule` is `.strict()`, so its + // undeclared envelope was REJECTED — a hard 422, fixed in #6245 the moment + // the door was bound. This shape is a plain (non-strict) `z.object`, so it + // TOLERATES the envelope and returns `success` — then strips it. Tolerate is + // not preserve: measured on `origin/main` before this spread, a stamped + // catalog descriptor round-tripping through the bound schema came back having + // lost all seven keys (`_lock`, `_lockReason`, `_lockSource`, `_provenance`, + // `_packageId`, `_packageVersion`, `_lockDocsUrl`), so every consumer of + // `extractProtection` / `resolveLockState` downstream of a parse saw an + // unlocked, unattributed, org-provenance item. No error anywhere. + // + // `webhook` was measured in the same pass and needs nothing: it has carried + // this spread since #4001 batch 11, and all seven keys survive its + // round-trip. See `connector.test.ts` → 'ADR-0010 protection envelope' for + // the pins that hold both readings. + // + // Pure-additive and internal: every key is `_`-prefixed and optional, so no + // author-facing field changes and nothing that parsed before stops parsing. + ...MetadataProtectionFields, })); export type Connector = z.input;