From 1fed7aac4c1cca532ec5ead1f10b23349fcb4e9e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 04:28:55 +0000 Subject: [PATCH 1/2] fix(spec): a connector read serves only the declared nested keys, and a save of the served form keeps the rest Inside retryConfig and each actions element, a connector read now serves only the keys those shapes declare; each withheld position is named in redactedKeys and the stored row is unchanged. The save door's carry-forward follows an actions element by its key, so a reordered list keeps each element's value. getPublished redacts the authored document alone and serves the metadata layer's bookkeeping beside it unchanged. What a save accepts is unchanged. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5mgePB9NQ4PNhvYVxk --- ...onnector-read-withholds-undeclared-keys.md | 18 ++ ...edaction.connector-undeclared-keys.test.ts | 76 +++++ .../src/metadata-redaction.ts | 18 +- packages/metadata/src/metadata-manager.ts | 27 +- ...blished-body-bookkeeping-redaction.test.ts | 57 ++++ ...r-undeclared-keys-withheld.dogfood.test.ts | 271 ++++++++++++++++++ .../connector-credential-redaction.test.ts | 56 ++++ .../connector-credential-redaction.ts | 79 ++++- 8 files changed, 583 insertions(+), 19 deletions(-) create mode 100644 .changeset/connector-read-withholds-undeclared-keys.md create mode 100644 packages/metadata-protocol/src/metadata-redaction.connector-undeclared-keys.test.ts create mode 100644 packages/metadata/src/published-body-bookkeeping-redaction.test.ts create mode 100644 packages/qa/dogfood/test/connector-meta-door-undeclared-keys-withheld.dogfood.test.ts diff --git a/.changeset/connector-read-withholds-undeclared-keys.md b/.changeset/connector-read-withholds-undeclared-keys.md new file mode 100644 index 00000000000..ed7b4b04fad --- /dev/null +++ b/.changeset/connector-read-withholds-undeclared-keys.md @@ -0,0 +1,18 @@ +--- +'@objectstack/spec': patch +'@objectstack/metadata-protocol': patch +'@objectstack/metadata': patch +--- + +fix(spec): a connector read serves only the keys `retryConfig` and each `actions` element declare, and a save of the served form keeps the rest + +Clause-②: no +Metadata-protocol: none + +The built-in `connector` read-path redactor now serves, inside `retryConfig` and inside each element of `actions`, only the keys those shapes declare. Every metadata read exit that serves a stored connector applies it. Each withheld position is named in `redactedKeys` (an `actions` element by its index), and the stored row is never changed. + +A save of the served form keeps every withheld nested value: the save door carries each one forward from the stored row. An `actions` element is followed by its `key`, so a reordered list keeps each element's value on that element; for every carried-forward position, an array element with no `id` is now identified by its `key`. A `retryConfig` or an `actions` element the author removes is removed. + +`MetadataManager.getPublished` now hands the type's redactor the authored document alone and serves the metadata layer's own bookkeeping beside the answer unchanged (`packageId`, `state`, and the publish keys). + +What a save accepts is unchanged. diff --git a/packages/metadata-protocol/src/metadata-redaction.connector-undeclared-keys.test.ts b/packages/metadata-protocol/src/metadata-redaction.connector-undeclared-keys.test.ts new file mode 100644 index 00000000000..da09ef832eb --- /dev/null +++ b/packages/metadata-protocol/src/metadata-redaction.connector-undeclared-keys.test.ts @@ -0,0 +1,76 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The write-path inverse of the connector read projection, for the keys a +// connector does not declare: a save of the served form keeps what the read +// withheld (top level, `retryConfig`, and an `actions` element addressed by +// its `key`), and a container the author removed is never minted back. +// +// Every value below is an obviously-fake placeholder. + +import { describe, it, expect } from 'vitest'; +import { carryForwardRedactedValues, redactMetadataItem } from './metadata-redaction.js'; + +const P = (n: number) => `TEST_PLACEHOLDER_UNDECLARED_${n}`; + +const stored = { + name: 'vendor_catalog', + label: 'Vendor catalog', + type: 'api', + apiKey: P(1), + retryConfig: { strategy: 'exponential_backoff', secret: P(2) }, + actions: [ + { key: 'get_invoice', label: 'Get invoice', token: P(3) }, + { key: 'list_invoices', label: 'List invoices' }, + ], +}; + +/** What a read exit serves for `stored`: the projection, as a client would send it back. */ +const served = () => JSON.parse(JSON.stringify(redactMetadataItem('connector', stored))) as Record; + +describe('carry-forward of the keys a connector does not declare', () => { + it('the served form withholds them (positive control for every case below)', () => { + const body = served(); + expect(JSON.stringify(body)).not.toContain('TEST_PLACEHOLDER_UNDECLARED_'); + expect(body.retryConfig).toEqual({ strategy: 'exponential_backoff' }); + expect(body.actions[0]).toEqual({ key: 'get_invoice', label: 'Get invoice' }); + }); + + it('a save of the served form keeps every withheld value, the edited label included', () => { + const out = carryForwardRedactedValues('connector', { ...served(), label: 'Vendor catalog (edited)' }, stored); + expect(out).toEqual({ ...stored, label: 'Vendor catalog (edited)' }); + }); + + it('an `actions` element is followed by its `key` when the author reorders the list', () => { + const body = served(); + body.actions.reverse(); + const out = carryForwardRedactedValues('connector', body, stored) as Record; + expect(out.actions).toEqual([ + { key: 'list_invoices', label: 'List invoices' }, + { key: 'get_invoice', label: 'Get invoice', token: P(3) }, + ]); + }); + + it('the author\'s own value at a withheld nested position wins', () => { + const body = served(); + body.retryConfig.secret = 'author-value'; + const out = carryForwardRedactedValues('connector', body, stored) as Record; + expect(out.retryConfig.secret).toBe('author-value'); + }); + + it('a removed `retryConfig` or `actions` element is not minted back', () => { + const body = served(); + delete body.retryConfig; + body.actions = body.actions.filter((a: { key: string }) => a.key !== 'get_invoice'); + const out = carryForwardRedactedValues('connector', body, stored) as Record; + expect(out).not.toHaveProperty('retryConfig'); + expect(out.actions).toEqual([{ key: 'list_invoices', label: 'List invoices' }]); + expect(out.apiKey).toBe(P(1)); + }); + + it('two `actions` elements sharing a `key` cannot be told apart: neither is carried', () => { + const twin = { ...stored, actions: [stored.actions[0], { key: 'get_invoice', label: 'Twin' }] }; + const body = JSON.parse(JSON.stringify(redactMetadataItem('connector', twin))); + const out = carryForwardRedactedValues('connector', body, twin) as Record; + expect(JSON.stringify(out.actions)).not.toContain(P(3)); + }); +}); diff --git a/packages/metadata-protocol/src/metadata-redaction.ts b/packages/metadata-protocol/src/metadata-redaction.ts index a480a8d3809..ca41d90ecdc 100644 --- a/packages/metadata-protocol/src/metadata-redaction.ts +++ b/packages/metadata-protocol/src/metadata-redaction.ts @@ -151,11 +151,17 @@ function isPlainRecord(value: unknown): value is Record { return !!value && typeof value === 'object' && !Array.isArray(value); } -/** An element's identity for an array hop: a non-empty string `id`, or none. */ +/** + * An element's identity for an array hop: a non-empty string `id`; for an + * element with no `id`, a non-empty string `key` (a connector action's machine + * name); otherwise none. + */ function identityOf(element: unknown): string | undefined { if (!isPlainRecord(element)) return undefined; const id = element.id; - return typeof id === 'string' && id !== '' ? id : undefined; + if (typeof id === 'string' && id !== '') return id; + const key = element.key; + return typeof key === 'string' && key !== '' ? key : undefined; } /** @@ -190,7 +196,9 @@ function elementWithIdentity( * into an identity, and every body — served and incoming — is then walked by * that identity, never by the index. Two kinds of identity: * - * - **the element's own `id`** (`{ elementId }`) — a flow node; + * - **the element's own `id`** (`{ elementId }`) — a flow node — or, for an + * element with no `id`, its own `key` — a connector action, whose `key` is + * its machine name; * - **for an element with no `id`, the identified element below it on the * same path** (`{ anchor }`, #20590). A `parallel` block's branch carries no * `id`, and a credential inside one sits at @@ -201,8 +209,8 @@ function elementWithIdentity( * is that walk: the hops from inside the element down to, and including, * the first identified element beneath it. * - * An array hop that reaches neither — an element with no `id` and no - * identified element below it on the path, or an `id` shared with a sibling — + * An array hop that reaches neither — an element with no `id` or `key` and no + * identified element below it on the path, or an identity shared with a sibling — * resolves to nothing, and the path is skipped. */ type PathHop = diff --git a/packages/metadata/src/metadata-manager.ts b/packages/metadata/src/metadata-manager.ts index 95a1f8c3446..dfcdbcd8b44 100644 --- a/packages/metadata/src/metadata-manager.ts +++ b/packages/metadata/src/metadata-manager.ts @@ -87,7 +87,7 @@ import type { import { EndpointMatcher } from './endpoint-matcher.js'; // [#5309] The stored ENVELOPE / authored BODY split — peeled before any spec // schema parses a stored row. See `stored-envelope.ts`. -import { peelStoredEnvelope } from './stored-envelope.js'; +import { peelStoredEnvelope, STORED_ENVELOPE_KEYS } from './stored-envelope.js'; // [#13913] Registry-free expansion of an aggregated view container reached // through THIS manager's own store — see `getViewsByObject`. import { expandRuntimeViewContainer } from './view-container-expansion.js'; @@ -2285,14 +2285,33 @@ export class MetadataManager implements IMetadataService { return this.redactServedBody(type, body); } - /** [#20552] The per-type read-path redaction, applied to one served body. */ + /** + * [#20552] The per-type read-path redaction, applied to one served body. + * + * The body can carry this layer's own bookkeeping beside the authored + * document (`packageId`, `state`, and the publish keys: the fallback serves + * the stored row itself, and a publish snapshot copies it whole). Those keys + * are not the type's vocabulary ({@link STORED_ENVELOPE_KEYS}), and a + * redactor that withholds every key its type does not declare would withhold + * them too. So the redactor is handed the authored document alone, and the + * bookkeeping is served beside its answer unchanged. + */ private redactServedBody(type: string, body: unknown): unknown { if (!body || typeof body !== 'object' || Array.isArray(body)) return body; // Keyed on the singular type name (Prime Directive #3); a plural spelling folds first. const redactor = getMetadataTypeRedactor(canonicalMetadataServiceType(type)); if (!redactor) return body; - const result = redactor(body as Record); - return result.redactedKeys.length > 0 ? result.item : body; + const record = body as Record; + const bookkeeping: Record = {}; + const authored: Record = {}; + for (const [key, value] of Object.entries(record)) { + if ((STORED_ENVELOPE_KEYS as readonly string[]).includes(key)) bookkeeping[key] = value; + else authored[key] = value; + } + const peeled = Object.keys(bookkeeping).length > 0; + const result = redactor(peeled ? authored : record); + if (result.redactedKeys.length === 0) return body; + return peeled ? { ...result.item, ...bookkeeping } : result.item; } // ========================================== diff --git a/packages/metadata/src/published-body-bookkeeping-redaction.test.ts b/packages/metadata/src/published-body-bookkeeping-redaction.test.ts new file mode 100644 index 00000000000..7caa328a2a2 --- /dev/null +++ b/packages/metadata/src/published-body-bookkeeping-redaction.test.ts @@ -0,0 +1,57 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// `MetadataManager.getPublished` serves a body that can carry this layer's own +// bookkeeping (`packageId`, `state`, the publish keys) beside the authored +// document. The type's read-path redactor judges the authored document alone: +// a key the type does not declare is withheld, the bookkeeping is served +// unchanged. Measured with the built-in `connector` redactor, which withholds +// every key a connector does not declare. +// +// Every value below is an obviously-fake placeholder. + +import { describe, it, expect, vi } from 'vitest'; +import { MetadataManager } from './metadata-manager.js'; +import { MemoryLoader } from './loaders/memory-loader.js'; +import { DEFAULT_METADATA_TYPE_REGISTRY } from '@objectstack/spec/kernel'; + +vi.mock('@objectstack/core', async (orig) => ({ + ...((await orig()) as object), + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }), +})); + +const SECRET = 'TEST_PLACEHOLDER_PUBLISHED_UNDECLARED'; +const authored = { label: 'Vendor', type: 'api', retryConfig: { maxAttempts: 2, secret: SECRET } }; + +function manager(): MetadataManager { + const m = new MetadataManager({ formats: ['json'], loaders: [new MemoryLoader()] }); + m.setTypeRegistry(DEFAULT_METADATA_TYPE_REGISTRY); + return m; +} + +describe('getPublished: the redactor judges the authored body, the bookkeeping is served as stored', () => { + it('a never-published row (the row itself is served): undeclared key withheld, packageId and state kept', async () => { + const m = manager(); + await m.register('connector', 'vendor', { name: 'vendor', ...authored, apiKey: SECRET, packageId: 'com.acme.x', state: 'draft' }); + const served = (await m.getPublished('connector', 'vendor')) as Record; + expect(JSON.stringify(served)).not.toContain(SECRET); + expect(served).toEqual({ name: 'vendor', label: 'Vendor', type: 'api', retryConfig: { maxAttempts: 2 }, packageId: 'com.acme.x', state: 'draft' }); + }); + + it('a published snapshot that copied the row whole: the same answer', async () => { + const m = manager(); + await m.register('connector', 'vendor', { name: 'vendor', ...authored, packageId: 'com.acme.x', state: 'draft' }); + await m.publishPackage('com.acme.x', { publishedBy: 'admin' }); + const served = (await m.getPublished('connector', 'vendor')) as Record; + expect(JSON.stringify(served)).not.toContain(SECRET); + expect(served.packageId).toBe('com.acme.x'); + expect(served).toHaveProperty('state'); + expect(served.retryConfig).toEqual({ maxAttempts: 2 }); + }); + + it('control: a body with nothing to withhold is served by reference-equal content, bookkeeping included', async () => { + const m = manager(); + const row = { name: 'clean', label: 'Clean', type: 'api', packageId: 'com.acme.x', state: 'draft' }; + await m.register('connector', 'clean', row); + expect(await m.getPublished('connector', 'clean')).toEqual(row); + }); +}); diff --git a/packages/qa/dogfood/test/connector-meta-door-undeclared-keys-withheld.dogfood.test.ts b/packages/qa/dogfood/test/connector-meta-door-undeclared-keys-withheld.dogfood.test.ts new file mode 100644 index 00000000000..0448b5a170c --- /dev/null +++ b/packages/qa/dogfood/test/connector-meta-door-undeclared-keys-withheld.dogfood.test.ts @@ -0,0 +1,271 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The connector metadata doors, booted on the showcase with its own connector +// plugins — keys a connector does not declare are never served: +// +// - READ: a stored row carrying top-level keys the connector does not declare +// (seeded through the system context, the only way such a row is written +// now), an instance whose `retryConfig` and a descriptor whose `actions` +// element carry an undeclared key (both accepted by the save door), are +// served with those keys withheld and every declared key as stored — to a +// member and to an administrator, in the item, layered, published and list +// reads. The stored rows are never touched. +// - CARRY-FORWARD: a save of the served form succeeds and keeps every stored +// value; a `retryConfig` the author removes is removed. +// - CONTROL: an instance that names its secret with `credentialRef` saves, +// materializes and dispatches upstream with the reference resolved. +// +// Every credential below is an obviously-fake placeholder, and every assertion +// on one is its absence from what a door served, or its presence at rest. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { createServer, type Server } from 'node:http'; +import type { AddressInfo } from 'node:net'; +import { type VerifyStack } from '@objectstack/verify'; +import { bootShowcase } from './showcase-boot.js'; + +const SYSTEM = { isSystem: true, positions: [], permissions: [] }; + +const PLACEHOLDERS = { + topLevel: 'TEST_PLACEHOLDER_UNDECLARED_TOP_LEVEL', + topLevelNested: 'TEST_PLACEHOLDER_UNDECLARED_TOP_LEVEL_NESTED', + retry: 'TEST_PLACEHOLDER_UNDECLARED_RETRY', + action: 'TEST_PLACEHOLDER_UNDECLARED_ACTION', + resolvedRef: 'TEST_PLACEHOLDER_UNDECLARED_RESOLVED_REFERENCE', +} as const; +const WITHHELD = [PLACEHOLDERS.topLevel, PLACEHOLDERS.topLevelNested, PLACEHOLDERS.retry, PLACEHOLDERS.action]; + +/** The reference every instance here names; set in this process's environment before any save. */ +const REF = 'CUK_CONTROL_CREDENTIAL_REF'; +const LEGACY = 'cuk_legacy_row'; +const RETRY = 'cuk_retry_instance'; +const DESCRIPTOR = 'cuk_actions_descriptor'; +const CONTROL = 'cuk_control'; + +const rowsOf = (r: any): any[] => (Array.isArray(r) ? r : Array.isArray(r?.records) ? r.records : Array.isArray(r?.value) ? r.value : []); +const bodyOf = (row: any): Record => + typeof row?.metadata === 'string' ? JSON.parse(row.metadata) : (row?.metadata ?? {}); +const itemOf = (json: any): Record => (json?.item ?? json?.data?.item ?? json?.data ?? json) as Record; +const holdsAPlaceholder = (v: unknown): boolean => { + const text = typeof v === 'string' ? v : JSON.stringify(v ?? null); + return WITHHELD.some((p) => text.includes(p)); +}; +/** A served item as a client sends it back: the read decorations dropped. */ +const authorable = (served: Record): Record => + Object.fromEntries(Object.entries(served).filter(([k]) => !k.startsWith('_'))); + +describe('connector metadata doors: keys a connector does not declare are withheld on read and kept on save', () => { + let stack: VerifyStack; + let admin: string; + let member: string; + let ql: any; + let automation: any; + let upstream: Server; + let upstreamUrl: string; + const received: string[] = []; + let previousRef: string | undefined; + + const stored = async (name: string) => + rowsOf(await ql.find('sys_metadata', { where: { type: 'connector', name }, context: SYSTEM })); + + const instance = (name: string, over: Record = {}) => ({ + name, + label: `CUK ${name}`, + type: 'api', + description: 'A provider-bound instance for the undeclared-key pins.', + provider: 'rest', + providerConfig: { baseUrl: `${upstreamUrl}/v1` }, + auth: { type: 'bearer', credentialRef: REF }, + ...over, + }); + + const descriptor = { + name: DESCRIPTOR, + label: 'CUK descriptor', + type: 'saas', + description: 'A catalog-only descriptor for the undeclared-key pins.', + actions: [ + { key: 'get_invoice', label: 'Get invoice', effect: 'read', token: PLACEHOLDERS.action }, + { key: 'list_invoices', label: 'List invoices' }, + ], + }; + + const read = async (token: string, name: string) => { + const res = await stack.apiAs(token, 'GET', `/meta/connector/${name}`); + const text = await res.text(); + expect(res.status, text.slice(0, 200)).toBe(200); + return { text, item: itemOf(JSON.parse(text)) }; + }; + + /** One door read: 200, names each connector (positive control), and serves no placeholder. */ + const expectWithheld = async (token: string, path: string, names: readonly string[]) => { + const res = await stack.apiAs(token, 'GET', path); + const text = await res.text(); + expect(res.status, `${path}: ${text.slice(0, 200)}`).toBe(200); + for (const name of names) expect(text.includes(name), `${path} names ${name}`).toBe(true); + expect(holdsAPlaceholder(text), `${path} withholds every undeclared key`).toBe(false); + }; + + const waitReady = async (name: string): Promise => { + const deadline = Date.now() + 15_000; + let last = ''; + while (Date.now() < deadline) { + const res = await stack.apiAs(admin, 'GET', '/automation/connectors'); + const json = (await res.json()) as any; + const found = (json?.data?.connectors ?? []).find((c: any) => c?.name === name); + last = JSON.stringify(found ?? null); + if (found?.state === 'ready') return; + await new Promise((r) => setTimeout(r, 100)); + } + throw new Error(`connector ${name} never reached ready: ${last}`); + }; + + beforeAll(async () => { + upstream = createServer((req, res) => { + received.push(req.url ?? ''); + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end('{"ok":true}'); + }); + await new Promise((r) => upstream.listen(0, '127.0.0.1', () => r())); + upstreamUrl = `http://127.0.0.1:${(upstream.address() as AddressInfo).port}`; + previousRef = process.env[REF]; + process.env[REF] = PLACEHOLDERS.resolvedRef; + + stack = await bootShowcase({}); + ql = await stack.kernel.getServiceAsync('objectql'); + automation = await stack.kernel.getServiceAsync('automation'); + admin = await stack.signIn(); + member = await stack.signUp('cuk-member@connector-undeclared.test'); + + // A clean save makes the legacy row; the system context then writes the + // undeclared top-level keys over it. + const put = await stack.apiAs(admin, 'PUT', `/meta/connector/${LEGACY}`, instance(LEGACY)); + expect(put.status, await put.clone().text()).toBe(200); + const [row] = await stored(LEGACY); + await ql.update('sys_metadata', { + id: row.id, + metadata: JSON.stringify({ + ...instance(LEGACY, { label: 'CUK legacy (seeded)' }), + apiKey: PLACEHOLDERS.topLevel, + extraConfig: { secret: PLACEHOLDERS.topLevelNested }, + }), + }, { context: SYSTEM }); + + const retry = await stack.apiAs(admin, 'PUT', `/meta/connector/${RETRY}`, instance(RETRY, { + retryConfig: { strategy: 'exponential_backoff', maxAttempts: 2, secret: PLACEHOLDERS.retry }, + })); + expect(retry.status, await retry.clone().text()).toBe(200); + const desc = await stack.apiAs(admin, 'PUT', `/meta/connector/${DESCRIPTOR}`, descriptor); + expect(desc.status, await desc.clone().text()).toBe(200); + }, 180_000); + + afterAll(async () => { + await stack?.stop?.(); + await new Promise((r) => (upstream ? upstream.close(() => r()) : r())); + if (previousRef === undefined) delete process.env[REF]; + else process.env[REF] = previousRef; + }); + + describe('READ: undeclared keys are withheld from every reader; declared keys are served', () => { + it('guard the guard: the stored rows hold every undeclared key', async () => { + const legacy = bodyOf((await stored(LEGACY))[0]); + expect(legacy.apiKey).toBe(PLACEHOLDERS.topLevel); + expect(legacy.extraConfig).toEqual({ secret: PLACEHOLDERS.topLevelNested }); + expect(bodyOf((await stored(RETRY))[0]).retryConfig.secret).toBe(PLACEHOLDERS.retry); + expect(bodyOf((await stored(DESCRIPTOR))[0]).actions[0].token).toBe(PLACEHOLDERS.action); + }); + + it('the item read and the layered view, for a member and for an administrator', async () => { + for (const token of [member, admin]) { + const legacy = await read(token, LEGACY); + expect(holdsAPlaceholder(legacy.text)).toBe(false); + expect(legacy.item).not.toHaveProperty('apiKey'); + expect(legacy.item).not.toHaveProperty('extraConfig'); + expect(legacy.item.label).toBe('CUK legacy (seeded)'); + expect(legacy.item.auth).toEqual({ type: 'bearer', credentialRef: REF }); + + const retry = await read(token, RETRY); + expect(holdsAPlaceholder(retry.text)).toBe(false); + expect(retry.item.retryConfig).toEqual({ strategy: 'exponential_backoff', maxAttempts: 2 }); + + const desc = await read(token, DESCRIPTOR); + expect(holdsAPlaceholder(desc.text)).toBe(false); + expect(desc.item.actions).toEqual([ + { key: 'get_invoice', label: 'Get invoice', effect: 'read' }, + { key: 'list_invoices', label: 'List invoices' }, + ]); + + for (const name of [LEGACY, RETRY, DESCRIPTOR]) { + await expectWithheld(token, `/meta/connector/${name}/layers`, [name]); + await expectWithheld(token, `/meta/connector/${name}/published`, [name]); + } + } + }); + + it('the list, both spellings, for a member and for an administrator', async () => { + for (const token of [member, admin]) { + await expectWithheld(token, '/meta/connector', [LEGACY, RETRY, DESCRIPTOR]); + await expectWithheld(token, '/meta/connectors', [LEGACY, RETRY, DESCRIPTOR]); + } + }); + + it('the stored rows are never redacted: redaction is a serving act', async () => { + expect(bodyOf((await stored(LEGACY))[0]).apiKey).toBe(PLACEHOLDERS.topLevel); + expect(bodyOf((await stored(RETRY))[0]).retryConfig.secret).toBe(PLACEHOLDERS.retry); + expect(bodyOf((await stored(DESCRIPTOR))[0]).actions[0].token).toBe(PLACEHOLDERS.action); + }); + }); + + describe('CARRY-FORWARD: a save of the served form keeps the stored values', () => { + it('the legacy row: the served form saves, and the row keeps its undeclared keys', async () => { + const served = (await read(admin, LEGACY)).item; + const res = await stack.apiAs(admin, 'PUT', `/meta/connector/${LEGACY}`, { ...authorable(served), label: 'CUK legacy (edited)' }); + expect(res.status, await res.clone().text()).toBe(200); + const atRest = bodyOf((await stored(LEGACY))[0]); + expect(atRest.label).toBe('CUK legacy (edited)'); + expect(atRest.apiKey).toBe(PLACEHOLDERS.topLevel); + expect(atRest.extraConfig).toEqual({ secret: PLACEHOLDERS.topLevelNested }); + expect(holdsAPlaceholder((await read(member, LEGACY)).text)).toBe(false); + }); + + it('a reordered `actions` list keeps each element\'s withheld key on that element', async () => { + const served = (await read(admin, DESCRIPTOR)).item; + const body = authorable(served); + body.actions = [...body.actions].reverse(); + const res = await stack.apiAs(admin, 'PUT', `/meta/connector/${DESCRIPTOR}`, body); + expect(res.status, await res.clone().text()).toBe(200); + const atRest = bodyOf((await stored(DESCRIPTOR))[0]); + expect(atRest.actions).toEqual([ + { key: 'list_invoices', label: 'List invoices' }, + { key: 'get_invoice', label: 'Get invoice', effect: 'read', token: PLACEHOLDERS.action }, + ]); + }); + + it('a `retryConfig` the author removes is removed; one sent back as served keeps its stored key', async () => { + const served = authorable((await read(admin, RETRY)).item); + const kept = await stack.apiAs(admin, 'PUT', `/meta/connector/${RETRY}`, { ...served, label: 'CUK retry (edited)' }); + expect(kept.status, await kept.clone().text()).toBe(200); + expect(bodyOf((await stored(RETRY))[0]).retryConfig.secret).toBe(PLACEHOLDERS.retry); + + const { retryConfig: _removed, ...withoutRetry } = served; + const cleared = await stack.apiAs(admin, 'PUT', `/meta/connector/${RETRY}`, withoutRetry); + expect(cleared.status, await cleared.clone().text()).toBe(200); + expect(bodyOf((await stored(RETRY))[0])).not.toHaveProperty('retryConfig'); + }); + }); + + describe('CONTROL: a declared-keys-only instance saves and works', () => { + it('saves, materializes ready, and dispatches with the reference resolved', async () => { + const put = await stack.apiAs(admin, 'PUT', `/meta/connector/${CONTROL}`, instance(CONTROL)); + expect(put.status, await put.clone().text()).toBe(200); + await waitReady(CONTROL); + const handler = automation.resolveConnectorAction(CONTROL, 'request'); + expect(handler, `${CONTROL} is materialized with its request action`).toBeTypeOf('function'); + const before = received.length; + const out = await handler({ method: 'GET', path: '/probe' }, {}); + expect(out?.status, JSON.stringify(out)).toBe(200); + expect(received.length).toBe(before + 1); + expect(received[received.length - 1]).toBe('/v1/probe'); + }); + }); +}); diff --git a/packages/spec/src/integration/connector-credential-redaction.test.ts b/packages/spec/src/integration/connector-credential-redaction.test.ts index e25e4bbe5b2..1859a594a61 100644 --- a/packages/spec/src/integration/connector-credential-redaction.test.ts +++ b/packages/spec/src/integration/connector-credential-redaction.test.ts @@ -324,6 +324,62 @@ describe('the connector redactor — top-level keys the entry does not declare', }); }); +describe('the connector redactor — keys `retryConfig` and `actions` elements do not declare', () => { + it('withholds an undeclared key inside `retryConfig`, keeping its declared keys', () => { + const retryConfig = { strategy: 'exponential_backoff', maxAttempts: 4, secret: P(30) }; + const { item: served, redactedKeys } = redactConnectorItem({ ...base, retryConfig }); + expect(redactedKeys).toEqual(['retryConfig.secret']); + expect(served.retryConfig).toEqual({ strategy: 'exponential_backoff', maxAttempts: 4 }); + expect(holds(served, P(30))).toBe(false); + }); + + it('withholds an undeclared key inside each `actions` element, addressed by index, keeping the declared keys', () => { + const actions = [ + { key: 'get_invoice', label: 'Get invoice', effect: 'read', inputSchema: { type: 'object' } }, + { key: 'post_invoice', label: 'Post invoice', token: P(31), extra: { secret: P(32) } }, + ]; + const { item: served, redactedKeys } = redactConnectorItem({ ...base, actions }); + expect(redactedKeys).toEqual(['actions.1.extra', 'actions.1.token']); + expect(served.actions).toEqual([actions[0], { key: 'post_invoice', label: 'Post invoice' }]); + expect((served.actions as unknown[])[0]).toBe(actions[0]); + expect(holds(served, P(31), P(32))).toBe(false); + }); + + it('withholds a cleared undeclared nested key too, as at the top level', () => { + const item = { ...base, retryConfig: { strategy: 'no_retry', note: null }, actions: [{ key: 'a', label: 'A', extra: {} }] }; + const { item: served, redactedKeys } = redactConnectorItem(item); + expect(redactedKeys).toEqual(['actions.0.extra', 'retryConfig.note']); + expect(served.retryConfig).toEqual({ strategy: 'no_retry' }); + expect(served.actions).toEqual([{ key: 'a', label: 'A' }]); + }); + + it('the declared nested keys are the schemas\' own', () => { + const retryConfig = { + strategy: 'exponential_backoff', maxAttempts: 3, initialDelayMs: 1000, maxDelayMs: 60000, + backoffMultiplier: 2, retryableStatusCodes: [429], retryOnNetworkError: true, jitter: true, + }; + const action = { key: 'a', label: 'A', description: 'd', inputSchema: {}, outputSchema: {}, effect: 'write' }; + const item = { ...base, retryConfig, actions: [action] }; + const result = redactConnectorItem(item); + expect(result.redactedKeys).toEqual([]); + expect(result.item).toBe(item); + }); + + it('a `retryConfig` or an `actions` element that is not an object is not walked', () => { + const item = { ...base, retryConfig: 'fast', actions: ['get_invoice', null] }; + const result = redactConnectorItem(item); + expect(result.redactedKeys).toEqual([]); + expect(result.item).toBe(item); + }); + + it('never mutates its input', () => { + const item = { ...base, retryConfig: { maxAttempts: 3, secret: P(33) }, actions: [{ key: 'a', label: 'A', token: P(34) }] }; + const before = JSON.stringify(item); + redactConnectorItem(item); + expect(JSON.stringify(item)).toBe(before); + }); +}); + describe('the connector redactor — purity and the untouched remainder', () => { it('never mutates its input, and leaves every other key (`_diagnostics` included) byte for byte', () => { const item = { diff --git a/packages/spec/src/integration/connector-credential-redaction.ts b/packages/spec/src/integration/connector-credential-redaction.ts index 0e5f3f194c6..3d717fda3a7 100644 --- a/packages/spec/src/integration/connector-credential-redaction.ts +++ b/packages/spec/src/integration/connector-credential-redaction.ts @@ -87,6 +87,17 @@ * at the save door, so serving its cleared form back makes a plain read * then save meet that refusal. The carry-forward keeps the stored value * (its dotted name included, by the whole-string fallback). + * 8. **Every key the two nested shapes the entry declares do not declare, + * WHOLE — whatever its value: inside `retryConfig`, and inside each + * element of `actions`.** Neither nested shape refuses an undeclared key, + * and the save door stores the body it accepts, so such a key is stored + * and, but for this item, served as written; nothing reads it. The + * declared keys are read from {@link RetryConfigSchema} and + * {@link ConnectorActionSchema} themselves. An `actions` element is + * addressed by its index (`actions..`), which the carry-forward + * resolves to the element's own `key`, so a save of a reordered list keeps + * each element's stored value on that element. A `retryConfig` or an + * element that is not an object is not walked. * * ## Cleared forms, and what is never touched * @@ -106,7 +117,7 @@ */ import { ConnectorInstanceAuthSchema } from '../shared/connector-auth.zod'; -import { DeclarativeConnectorEntrySchema } from './connector.zod'; +import { ConnectorActionSchema, DeclarativeConnectorEntrySchema, RetryConfigSchema } from './connector.zod'; import { METADATA_READ_DECORATIONS } from '../kernel/metadata-read-decorations'; import { CREDENTIAL_URL_QUERY_PARAM_NAMES } from '../data/driver/common.zod'; import { redactUrlCredentialQueryParams } from '../data/datasource-credential-redaction'; @@ -243,20 +254,36 @@ function isClearedForm(value: unknown): boolean { return value === undefined || value === null || value === '' || (isPlainRecord(value) && Object.keys(value).length === 0); } -/** `item` with the key at `path` removed, copying each container on the way; the input is never touched. */ -function withoutKeyAt(item: Record, path: readonly string[]): Record { +/** The child at `segment`: a key of a plain object, or an index of an array (item 8's `actions` hop). */ +function childAt(node: unknown, segment: string): unknown { + if (isPlainRecord(node)) return node[segment]; + if (Array.isArray(node) && /^(0|[1-9][0-9]*)$/.test(segment)) return node[Number(segment)]; + return undefined; +} + +/** + * `node` with the key at `path` removed, copying each container on the way (an + * array hop copies the array); the input is never touched. + */ +function withoutKeyAt(node: T, path: readonly string[]): T { const [head, ...rest] = path as [string, ...string[]]; + if (Array.isArray(node)) { + const copy = node.slice(); + copy[Number(head)] = withoutKeyAt(node[Number(head)], rest); + return copy as T; + } + const record = node as Record; if (rest.length === 0) { - const { [head]: _withheld, ...kept } = item; - return kept; + const { [head]: _withheld, ...kept } = record; + return kept as T; } - return { ...item, [head]: withoutKeyAt(item[head] as Record, rest) }; + return { ...record, [head]: withoutKeyAt(record[head], rest) } as T; } /** The value at `path` in `item`, and whether its parent is a plain object holding the key. */ function readAt(item: Record, path: readonly string[]): { present: boolean; value: unknown } { let parent: unknown = item; - for (const key of path.slice(0, -1)) parent = isPlainRecord(parent) ? parent[key] : undefined; + for (const key of path.slice(0, -1)) parent = childAt(parent, key); const leaf = path[path.length - 1] as string; if (!isPlainRecord(parent) || !Object.prototype.hasOwnProperty.call(parent, leaf)) return { present: false, value: undefined }; return { present: true, value: parent[leaf] }; @@ -297,6 +324,38 @@ function undeclaredRootKeysOf(item: Record): string[][] { return paths; } +/** Read once, on first use: the keys item 8's two nested shapes declare (the schemas are lazy). */ +let declaredNestedKeys: { retryConfig: ReadonlySet; action: ReadonlySet } | undefined; +function nestedKeys(): { retryConfig: ReadonlySet; action: ReadonlySet } { + // No readable shape ⇒ nothing is declared, and every nested key is withheld: fail closed. + declaredNestedKeys ??= { + retryConfig: new Set(Object.keys(objectShapeOf(RetryConfigSchema) ?? {})), + action: new Set(Object.keys(objectShapeOf(ConnectorActionSchema) ?? {})), + }; + return declaredNestedKeys; +} + +/** + * Item 8: every key `retryConfig` and each `actions` element do not declare, + * as item-relative paths (an `actions` hop is the element's index). + */ +function undeclaredNestedKeysOf(item: Record): string[][] { + const declared = nestedKeys(); + const paths: string[][] = []; + const retryConfig = item.retryConfig; + if (isPlainRecord(retryConfig)) { + for (const key of Object.keys(retryConfig)) if (!declared.retryConfig.has(key)) paths.push(['retryConfig', key]); + } + const actions = item.actions; + if (Array.isArray(actions)) { + actions.forEach((action, index) => { + if (!isPlainRecord(action)) return; + for (const key of Object.keys(action)) if (!declared.action.has(key)) paths.push(['actions', String(index), key]); + }); + } + return paths; +} + /** Every credential position present in `item` that is withheld by removal, as item-relative paths. */ function credentialPathsOf(item: Record): string[][] { const paths: string[][] = []; @@ -386,9 +445,9 @@ function credentialUrlRewritesOf( export function redactConnectorItem(item: Record): MetadataRedactionResult { let served = item; const redactedKeys: string[] = []; - // Item 7, first: an undeclared root key is withheld whatever its value — a - // cleared one included — because the entry refuses it at the save door. - for (const path of undeclaredRootKeysOf(item)) { + // Items 7 and 8, first: an undeclared key is withheld whatever its value — + // a cleared one included — because nothing can judge it. + for (const path of [...undeclaredRootKeysOf(item), ...undeclaredNestedKeysOf(item)]) { const { present } = readAt(served, path); if (!present) continue; served = withoutKeyAt(served, path); From 8daa64f14fc48cfb00e99cc3a400b6c609172db3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 06:15:50 +0000 Subject: [PATCH 2/2] fix(metadata): the published read serves a nested publish snapshot redacted, and a connector's malformed nested container is withheld whole getPublished serves only scalar bookkeeping keys unchanged; a snapshot nested under publishedDefinition is served through the same per-type redaction at every depth, and any other non-scalar bookkeeping value is not served. A connector's retryConfig that is not an object, and an actions value that is not an array or holds a non-object element, are withheld whole on read and kept at rest by the save door. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5mgePB9NQ4PNhvYVxk --- ...onnector-read-withholds-undeclared-keys.md | 4 +- ...edaction.connector-undeclared-keys.test.ts | 10 ++++ packages/metadata/src/metadata-manager.ts | 53 ++++++++++++++----- ...blished-body-bookkeeping-redaction.test.ts | 26 +++++++++ ...r-undeclared-keys-withheld.dogfood.test.ts | 24 ++++++++- .../connector-credential-redaction.test.ts | 25 +++++++-- .../connector-credential-redaction.ts | 34 +++++++----- 7 files changed, 144 insertions(+), 32 deletions(-) diff --git a/.changeset/connector-read-withholds-undeclared-keys.md b/.changeset/connector-read-withholds-undeclared-keys.md index ed7b4b04fad..34c422dc632 100644 --- a/.changeset/connector-read-withholds-undeclared-keys.md +++ b/.changeset/connector-read-withholds-undeclared-keys.md @@ -9,10 +9,10 @@ fix(spec): a connector read serves only the keys `retryConfig` and each `actions Clause-②: no Metadata-protocol: none -The built-in `connector` read-path redactor now serves, inside `retryConfig` and inside each element of `actions`, only the keys those shapes declare. Every metadata read exit that serves a stored connector applies it. Each withheld position is named in `redactedKeys` (an `actions` element by its index), and the stored row is never changed. +The built-in `connector` read-path redactor now serves, inside `retryConfig` and inside each element of `actions`, only the keys those shapes declare. Every metadata read exit that serves a stored connector applies it. Each withheld position is named in `redactedKeys` (an `actions` element by its index), and the stored row is never changed. A `retryConfig` that is not an object, and an `actions` that is not an array or holds an element that is not an object, are withheld whole (a cleared form excepted). A save of the served form keeps every withheld nested value: the save door carries each one forward from the stored row. An `actions` element is followed by its `key`, so a reordered list keeps each element's value on that element; for every carried-forward position, an array element with no `id` is now identified by its `key`. A `retryConfig` or an `actions` element the author removes is removed. -`MetadataManager.getPublished` now hands the type's redactor the authored document alone and serves the metadata layer's own bookkeeping beside the answer unchanged (`packageId`, `state`, and the publish keys). +`MetadataManager.getPublished` now hands the type's redactor the authored document alone and serves the metadata layer's own scalar bookkeeping beside the answer unchanged (`packageId`, `state`, and the publish keys). A publish snapshot nested under `publishedDefinition` (a package published more than once) is served through the same redaction at every depth; any other bookkeeping value that is not a scalar is not served. What a save accepts is unchanged. diff --git a/packages/metadata-protocol/src/metadata-redaction.connector-undeclared-keys.test.ts b/packages/metadata-protocol/src/metadata-redaction.connector-undeclared-keys.test.ts index da09ef832eb..cb232c02509 100644 --- a/packages/metadata-protocol/src/metadata-redaction.connector-undeclared-keys.test.ts +++ b/packages/metadata-protocol/src/metadata-redaction.connector-undeclared-keys.test.ts @@ -73,4 +73,14 @@ describe('carry-forward of the keys a connector does not declare', () => { const out = carryForwardRedactedValues('connector', body, twin) as Record; expect(JSON.stringify(out.actions)).not.toContain(P(3)); }); + + it('a malformed `retryConfig` or `actions` withheld whole is kept at rest by a save of the served form', () => { + const malformed = { ...stored, retryConfig: P(4), actions: [stored.actions[0], P(5)] }; + const body = JSON.parse(JSON.stringify(redactMetadataItem('connector', malformed))) as Record; + expect(body).not.toHaveProperty('retryConfig'); + expect(body).not.toHaveProperty('actions'); + expect(JSON.stringify(body)).not.toContain('TEST_PLACEHOLDER_UNDECLARED_'); + const out = carryForwardRedactedValues('connector', { ...body, label: 'Vendor catalog (edited)' }, malformed); + expect(out).toEqual({ ...malformed, label: 'Vendor catalog (edited)' }); + }); }); diff --git a/packages/metadata/src/metadata-manager.ts b/packages/metadata/src/metadata-manager.ts index dfcdbcd8b44..11d0f3c5638 100644 --- a/packages/metadata/src/metadata-manager.ts +++ b/packages/metadata/src/metadata-manager.ts @@ -333,6 +333,38 @@ export interface MetadataManagerOptions extends MetadataManagerConfig { driver?: IDataDriver; } +/** + * {@link MetadataManager.redactServedBody}'s walk, for one document and the + * snapshots nested in its bookkeeping. Returns the input by reference when + * nothing at any depth is withheld. + */ +function redactServedDocument( + redactor: NonNullable>, + record: Record, +): Record { + const bookkeeping: Record = {}; + const authored: Record = {}; + let bookkeepingChanged = false; + for (const [key, value] of Object.entries(record)) { + if (!(STORED_ENVELOPE_KEYS as readonly string[]).includes(key)) { + authored[key] = value; + } else if (value === null || typeof value !== 'object') { + bookkeeping[key] = value; + } else if (key === 'publishedDefinition' && !Array.isArray(value)) { + const served = redactServedDocument(redactor, value as Record); + if (served !== value) bookkeepingChanged = true; + bookkeeping[key] = served; + } else { + // A non-scalar bookkeeping value that is not a snapshot: not served. + bookkeepingChanged = true; + } + } + const peeled = Object.keys(bookkeeping).length > 0 || bookkeepingChanged; + const result = redactor(peeled ? authored : record); + if (result.redactedKeys.length === 0 && !bookkeepingChanged) return record; + return peeled ? { ...result.item, ...bookkeeping } : result.item; +} + /** * Main metadata manager class. * Implements IMetadataService contract for unified metadata management. @@ -2294,24 +2326,21 @@ export class MetadataManager implements IMetadataService { * are not the type's vocabulary ({@link STORED_ENVELOPE_KEYS}), and a * redactor that withholds every key its type does not declare would withhold * them too. So the redactor is handed the authored document alone, and the - * bookkeeping is served beside its answer unchanged. + * bookkeeping is served beside its answer. + * + * Only a SCALAR bookkeeping value is served unchanged. A publish snapshot + * copies the whole row, so a row published again carries the previous + * snapshot under `publishedDefinition`: that value is a document of the same + * type, and it is served through this same redaction, at every depth. Any + * other bookkeeping value that is not a scalar is not served: no envelope key + * but `publishedDefinition` holds a document, and nothing here can judge one. */ private redactServedBody(type: string, body: unknown): unknown { if (!body || typeof body !== 'object' || Array.isArray(body)) return body; // Keyed on the singular type name (Prime Directive #3); a plural spelling folds first. const redactor = getMetadataTypeRedactor(canonicalMetadataServiceType(type)); if (!redactor) return body; - const record = body as Record; - const bookkeeping: Record = {}; - const authored: Record = {}; - for (const [key, value] of Object.entries(record)) { - if ((STORED_ENVELOPE_KEYS as readonly string[]).includes(key)) bookkeeping[key] = value; - else authored[key] = value; - } - const peeled = Object.keys(bookkeeping).length > 0; - const result = redactor(peeled ? authored : record); - if (result.redactedKeys.length === 0) return body; - return peeled ? { ...result.item, ...bookkeeping } : result.item; + return redactServedDocument(redactor, body as Record); } // ========================================== diff --git a/packages/metadata/src/published-body-bookkeeping-redaction.test.ts b/packages/metadata/src/published-body-bookkeeping-redaction.test.ts index 7caa328a2a2..d80a9c2dfa3 100644 --- a/packages/metadata/src/published-body-bookkeeping-redaction.test.ts +++ b/packages/metadata/src/published-body-bookkeeping-redaction.test.ts @@ -48,6 +48,32 @@ describe('getPublished: the redactor judges the authored body, the bookkeeping i expect(served.retryConfig).toEqual({ maxAttempts: 2 }); }); + it('a package published twice (the second snapshot nests the first): withheld at every depth, scalar bookkeeping kept', async () => { + const m = manager(); + await m.register('connector', 'vendor', { name: 'vendor', ...authored, apiKey: SECRET, packageId: 'com.acme.x', state: 'draft' }); + await m.publishPackage('com.acme.x', { publishedBy: 'admin' }); + await m.publishPackage('com.acme.x', { publishedBy: 'admin' }); + // The stored row does nest: the second snapshot holds the first under its own `publishedDefinition`. + const stored = (await m.get('connector', 'vendor')) as Record; + expect(stored.publishedDefinition).toHaveProperty('publishedDefinition'); + expect(JSON.stringify(stored)).toContain(SECRET); + const served = (await m.getPublished('connector', 'vendor')) as Record; + expect(JSON.stringify(served)).not.toContain(SECRET); + expect(served.packageId).toBe('com.acme.x'); + expect(served.state).toBe('active'); + expect(served.retryConfig).toEqual({ maxAttempts: 2 }); + expect(served.publishedDefinition).toMatchObject({ name: 'vendor', retryConfig: { maxAttempts: 2 }, packageId: 'com.acme.x' }); + expect(served.publishedDefinition).not.toHaveProperty('apiKey'); + }); + + it('a non-scalar bookkeeping value that is not a snapshot is not served', async () => { + const m = manager(); + await m.register('connector', 'vendor', { name: 'vendor', label: 'Vendor', type: 'api', packageId: 'com.acme.x', package: { secret: SECRET } }); + const served = (await m.getPublished('connector', 'vendor')) as Record; + expect(JSON.stringify(served)).not.toContain(SECRET); + expect(served).toEqual({ name: 'vendor', label: 'Vendor', type: 'api', packageId: 'com.acme.x' }); + }); + it('control: a body with nothing to withhold is served by reference-equal content, bookkeeping included', async () => { const m = manager(); const row = { name: 'clean', label: 'Clean', type: 'api', packageId: 'com.acme.x', state: 'draft' }; diff --git a/packages/qa/dogfood/test/connector-meta-door-undeclared-keys-withheld.dogfood.test.ts b/packages/qa/dogfood/test/connector-meta-door-undeclared-keys-withheld.dogfood.test.ts index 0448b5a170c..d32b5c784c4 100644 --- a/packages/qa/dogfood/test/connector-meta-door-undeclared-keys-withheld.dogfood.test.ts +++ b/packages/qa/dogfood/test/connector-meta-door-undeclared-keys-withheld.dogfood.test.ts @@ -9,7 +9,8 @@ // element carry an undeclared key (both accepted by the save door), are // served with those keys withheld and every declared key as stored — to a // member and to an administrator, in the item, layered, published and list -// reads. The stored rows are never touched. +// reads, and on the published read of a package published twice (whose +// second snapshot nests the first). The stored rows are never touched. // - CARRY-FORWARD: a save of the served form succeeds and keeps every stored // value; a `retryConfig` the author removes is removed. // - CONTROL: an instance that names its secret with `credentialRef` saves, @@ -41,6 +42,8 @@ const LEGACY = 'cuk_legacy_row'; const RETRY = 'cuk_retry_instance'; const DESCRIPTOR = 'cuk_actions_descriptor'; const CONTROL = 'cuk_control'; +const TWICE = 'cuk_published_twice'; +const TWICE_PACKAGE = 'com.example.cuk-published-twice'; const rowsOf = (r: any): any[] => (Array.isArray(r) ? r : Array.isArray(r?.records) ? r.records : Array.isArray(r?.value) ? r.value : []); const bodyOf = (row: any): Record => @@ -209,6 +212,25 @@ describe('connector metadata doors: keys a connector does not declare are withhe } }); + it('the published read of a package published twice (the second snapshot nests the first), for a member and for an administrator', async () => { + const metadata = await stack.kernel.getServiceAsync('metadata'); + await metadata.register('connector', TWICE, { + ...instance(TWICE), + apiKey: PLACEHOLDERS.topLevel, + retryConfig: { maxAttempts: 2, secret: PLACEHOLDERS.retry }, + packageId: TWICE_PACKAGE, + }); + await metadata.publishPackage(TWICE_PACKAGE, { publishedBy: 'cuk-admin' }); + await metadata.publishPackage(TWICE_PACKAGE, { publishedBy: 'cuk-admin' }); + // Guard the guard: the snapshot nests, and the nested one holds the keys. + const row = await metadata.get('connector', TWICE); + expect(row?.publishedDefinition?.publishedDefinition).toBeTruthy(); + expect(holdsAPlaceholder(row.publishedDefinition.publishedDefinition)).toBe(true); + for (const token of [member, admin]) { + await expectWithheld(token, `/meta/connector/${TWICE}/published`, [TWICE]); + } + }); + it('the stored rows are never redacted: redaction is a serving act', async () => { expect(bodyOf((await stored(LEGACY))[0]).apiKey).toBe(PLACEHOLDERS.topLevel); expect(bodyOf((await stored(RETRY))[0]).retryConfig.secret).toBe(PLACEHOLDERS.retry); diff --git a/packages/spec/src/integration/connector-credential-redaction.test.ts b/packages/spec/src/integration/connector-credential-redaction.test.ts index 1859a594a61..2d11e005d61 100644 --- a/packages/spec/src/integration/connector-credential-redaction.test.ts +++ b/packages/spec/src/integration/connector-credential-redaction.test.ts @@ -365,11 +365,26 @@ describe('the connector redactor — keys `retryConfig` and `actions` elements d expect(result.item).toBe(item); }); - it('a `retryConfig` or an `actions` element that is not an object is not walked', () => { - const item = { ...base, retryConfig: 'fast', actions: ['get_invoice', null] }; - const result = redactConnectorItem(item); - expect(result.redactedKeys).toEqual([]); - expect(result.item).toBe(item); + it.each([ + ['a non-object `retryConfig`', { retryConfig: P(60) }, ['retryConfig']], + ['an `actions` element that is not an object', { actions: [{ key: 'a', label: 'A' }, P(61)] }, ['actions']], + ['a `null` `actions` element', { actions: [{ key: 'a', label: 'A', token: P(62) }, null] }, ['actions']], + ['an `actions` that is not an array', { actions: { key: 'a', token: P(63) } }, ['actions']], + ] as const)('withholds the container WHOLE when it is not the declared shape: %s', (_label, extra, keys) => { + const item = { ...base, ...extra }; + const { item: served, redactedKeys } = redactConnectorItem(item); + expect(redactedKeys).toEqual(keys); + for (const key of keys) expect(served).not.toHaveProperty(key); + expect(holds(served, P(60), P(61), P(62), P(63))).toBe(false); + }); + + it('a cleared `retryConfig` or `actions`, and an empty `actions` list, are served as written', () => { + for (const extra of [{ retryConfig: null }, { retryConfig: '' }, { actions: null }, { actions: [] }]) { + const item = { ...base, ...extra }; + const result = redactConnectorItem(item); + expect(result.redactedKeys).toEqual([]); + expect(result.item).toBe(item); + } }); it('never mutates its input', () => { diff --git a/packages/spec/src/integration/connector-credential-redaction.ts b/packages/spec/src/integration/connector-credential-redaction.ts index 3d717fda3a7..beb4428739d 100644 --- a/packages/spec/src/integration/connector-credential-redaction.ts +++ b/packages/spec/src/integration/connector-credential-redaction.ts @@ -89,15 +89,19 @@ * (its dotted name included, by the whole-string fallback). * 8. **Every key the two nested shapes the entry declares do not declare, * WHOLE — whatever its value: inside `retryConfig`, and inside each - * element of `actions`.** Neither nested shape refuses an undeclared key, - * and the save door stores the body it accepts, so such a key is stored - * and, but for this item, served as written; nothing reads it. The - * declared keys are read from {@link RetryConfigSchema} and - * {@link ConnectorActionSchema} themselves. An `actions` element is - * addressed by its index (`actions..`), which the carry-forward - * resolves to the element's own `key`, so a save of a reordered list keeps - * each element's stored value on that element. A `retryConfig` or an - * element that is not an object is not walked. + * element of `actions`.** A key no declared position names is one this + * redactor cannot judge, and nothing reads it. The declared keys are read + * from {@link RetryConfigSchema} and {@link ConnectorActionSchema} + * themselves. An `actions` element is addressed by its index + * (`actions..`), which the carry-forward resolves to the element's + * own `key`, so a save of a reordered list keeps each element's stored + * value on that element. A `retryConfig` that is not an object, and an + * `actions` that is not an array or holds an element that is not an + * object, are withheld WHOLE (a cleared form excepted), as a non-object + * `auth` is (item 4): the entry refuses each at every authoring door, so + * only a row stored outside them carries one, and nothing in it can be + * judged. An element is never removed on its own, which would shift every + * later index; the carry-forward keeps the stored value at rest. * * ## Cleared forms, and what is never touched * @@ -337,7 +341,8 @@ function nestedKeys(): { retryConfig: ReadonlySet; action: ReadonlySet): string[][] { const declared = nestedKeys(); @@ -345,13 +350,18 @@ function undeclaredNestedKeysOf(item: Record): string[][] { const retryConfig = item.retryConfig; if (isPlainRecord(retryConfig)) { for (const key of Object.keys(retryConfig)) if (!declared.retryConfig.has(key)) paths.push(['retryConfig', key]); + } else if (!isClearedForm(retryConfig)) { + paths.push(['retryConfig']); } const actions = item.actions; - if (Array.isArray(actions)) { + if (Array.isArray(actions) && actions.every(isPlainRecord)) { actions.forEach((action, index) => { - if (!isPlainRecord(action)) return; for (const key of Object.keys(action)) if (!declared.action.has(key)) paths.push(['actions', String(index), key]); }); + } else if (!isClearedForm(actions)) { + // Not an array, or an element is not an object: withheld whole (an element + // removed on its own would shift every later index). + paths.push(['actions']); } return paths; }