diff --git a/.changeset/connector-read-undeclared-root-keys.md b/.changeset/connector-read-undeclared-root-keys.md new file mode 100644 index 00000000000..8c2857d9549 --- /dev/null +++ b/.changeset/connector-read-undeclared-root-keys.md @@ -0,0 +1,11 @@ +--- +'@objectstack/spec': patch +'@objectstack/metadata-protocol': patch +--- + +fix(spec): the connector read withholds root keys the entry shape does not declare + +Clause-②: no +Metadata-protocol: none + +Read-side hardening of the built-in `connector` redactor. A stored connector body is served without any top-level key the authored connector entry (`DeclarativeConnectorEntrySchema`) does not declare — including a cleared one (`null`, `''`, `{}`) and one whose own name carries a dot; the declared set is read from that schema's own shape, and the read decorations (`_diagnostics`, `_draft`) are left alone. Under `providerConfig`, an array, or a key whose own name carries a dot, that holds a URL with credential parts is withheld whole rather than served. Each withheld position is named in `redactedKeys`, and the metadata protocol's carry-forward keeps the stored value at rest when the served form is saved — resolving a dotted key by the whole-string fallback when the dotted split resolves nowhere. The runtime's execution read is unchanged, and nothing an author can write changes. diff --git a/packages/metadata-protocol/src/metadata-redaction.ts b/packages/metadata-protocol/src/metadata-redaction.ts index 9a22cc8a0ca..a480a8d3809 100644 --- a/packages/metadata-protocol/src/metadata-redaction.ts +++ b/packages/metadata-protocol/src/metadata-redaction.ts @@ -516,6 +516,41 @@ export function redactedPathsCarriedForward(type: string, incoming: unknown, sto * {@link redactedPathsCarriedForward}: the grafted body, and where each graft * landed. `landed` is empty exactly when `out` is `incoming` by reference. */ +/** + * A `redactedKeys` entry resolved against the STORED body into the container + * hops that reach the key and the key itself — which may carry dots. + * + * `redactedKeys` joins each path on dots, so a key whose own name carries a dot + * (a connector root key spelled `v1.url`, or one held under `providerConfig`) + * is indistinguishable from a nested path by the string alone. The two are told + * apart by what the stored body actually holds, trying the partitions in a + * fixed order: the standard split FIRST — every dot a separator, the only + * reading before dotted keys were withheld — then progressively longer trailing + * keys, ending with the whole string as a single key. The first partition whose + * stored container holds the key wins, so a nested position always shadows a + * dotted-key alias of the same spelling (the alias is then left to the author's + * word, never carried onto the wrong position). `undefined` when no partition + * reaches a present value — the caller reads that as "nothing at rest to carry". + */ +function resolveRedactedPath( + stored: unknown, + path: string, +): { hops: PathHop[]; key: string } | undefined { + const segments = path.split('.'); + for (let cut = segments.length - 1; cut >= 0; cut -= 1) { + const key = segments.slice(cut).join('.'); + // resolveHops reads the last entry as the key and the rest as container + // hops, so a placeholder last entry leaves exactly `cut` container hops. + const hops = resolveHops(stored, [...segments.slice(0, cut), key]); + if (!hops) continue; + const container = containerAt(stored, hops); + if (container && Object.prototype.hasOwnProperty.call(container, key) && container[key] !== undefined) { + return { hops, key }; + } + } + return undefined; +} + function planCarryForward(type: string, incoming: T, stored: unknown): { out: T; landed: string[] } { const unchanged = { out: incoming, landed: [] as string[] }; if (!incoming || typeof incoming !== 'object' || Array.isArray(incoming)) return unchanged; @@ -535,11 +570,15 @@ function planCarryForward(type: string, incoming: T, stored: unknown): { out: // Dotted, item-relative — the registry's documented contract for // `redactedKeys` (`config.password`; an array hop is an index into the // stored body, `nodes.0.config.secret`, resolved to an identity by - // {@link resolveHops}). - const segments = path.split('.'); - const key = segments[segments.length - 1] as string; - const hops = resolveHops(stored, segments); - if (!hops) continue; + // {@link resolveHops}). A key whose OWN name carries a dot (a connector + // root key spelled `v1.url`, or one under `providerConfig`) cannot be + // told from a nested path by the join alone, so the split is tried + // first — every dot a separator, the existing reading — and only when + // that resolves to nothing at rest is a longer trailing key tried, + // ending with the whole string as a single key. {@link resolveRedactedPath}. + const resolved = resolveRedactedPath(stored, path); + if (!resolved) continue; + const { hops, key } = resolved; const storedParent = containerAt(stored, hops); const storedValue = storedParent?.[key]; diff --git a/packages/metadata-protocol/src/protocol.diff-credential-redaction.test.ts b/packages/metadata-protocol/src/protocol.diff-credential-redaction.test.ts index 38e7bf99ea9..66688435dbb 100644 --- a/packages/metadata-protocol/src/protocol.diff-credential-redaction.test.ts +++ b/packages/metadata-protocol/src/protocol.diff-credential-redaction.test.ts @@ -331,3 +331,75 @@ describe('#8671 — anti-vacuity: ablate the redactor', () => { expect(label.to).toBe('B'); }); }); + +describe('the connector history diff withholds root keys the entry shape does not declare', () => { + const ROOT_V1 = 'TEST_PLACEHOLDER_ROOT_KEY_V1'; + const ROOT_V2 = 'TEST_PLACEHOLDER_ROOT_KEY_V2'; + const connector = (label: string, extra: Record) => ({ + name: 'vendor_api', + label, + type: 'api', + provider: 'rest', + providerConfig: { baseUrl: 'https://api.vendor.example/v1' }, + auth: { type: 'bearer', credentialRef: 'VENDOR_API_TOKEN' }, + ...extra, + }); + const BUILTIN_CONNECTOR_REDACTOR = getMetadataTypeRedactor('connector')!; + afterEach(() => registerMetadataTypeRedactor('connector', BUILTIN_CONNECTOR_REDACTOR)); + + async function connectorDiff(from: number, to: number) { + const { engine, tables } = makeStubEngine(); + const base = { organization_id: null, type: 'connector', name: 'vendor_api' }; + const bodies = [ + connector('Vendor', {}), + connector('Vendor', { apiKey: ROOT_V1 }), + connector('Vendor API', { apiKey: ROOT_V2 }), + ]; + bodies.forEach((body, i) => { + tables.sys_metadata_history!.push({ + ...base, + id: `h_${i + 1}`, + version: i + 1, + event_seq: i + 1, + operation_type: i === 0 ? 'create' : 'update', + metadata: JSON.stringify(body), + checksum: hashSpec(body), + recorded_at: new Date(i + 1).toISOString(), + }); + }); + const protocol = new ObjectStackProtocolImplementation(engine); + return await protocol.diffMetaItem({ type: 'connector', name: 'vendor_api', fromVersion: from, toVersion: to }) as any; + } + + const holdsRootValue = (payload: unknown) => + allStrings(payload).some((s) => s.includes(ROOT_V1) || s.includes(ROOT_V2)); + + it('withholds undeclared root keys on read: the history diff, all three buckets', async () => { + const changed = await connectorDiff(2, 3); + const rotated = entry(changed.changed, 'apiKey') as any; + // The change is still reported; neither value is served. + expect(rotated).toBeDefined(); + expect(rotated.from).toBeUndefined(); + expect(rotated.to).toBeUndefined(); + expect((entry(changed.changed, 'label') as any).to).toBe('Vendor API'); + expect(holdsRootValue(changed)).toBe(false); + + const added = await connectorDiff(1, 2); + expect(entry(added.added, 'apiKey')).toBeDefined(); + expect((entry(added.added, 'apiKey') as any).value).toBeUndefined(); + expect(holdsRootValue(added)).toBe(false); + + const removed = await connectorDiff(2, 1); + expect(entry(removed.removed, 'apiKey')).toBeDefined(); + expect((entry(removed.removed, 'apiKey') as any).value).toBeUndefined(); + expect(holdsRootValue(removed)).toBe(false); + }); + + it('ABLATION — with an identity `connector` redactor the diff serves the values again', async () => { + registerMetadataTypeRedactor('connector', (item) => ({ item, redactedKeys: [] })); + const res = await connectorDiff(2, 3); + const rotated = entry(res.changed, 'apiKey') as any; + expect(rotated.from).toBe(ROOT_V1); + expect(rotated.to).toBe(ROOT_V2); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts index bcb35c62dd5..d9702da29b5 100644 --- a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts +++ b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts @@ -1371,6 +1371,259 @@ describe('#20611 — the runtime authoring gate reads a withheld-and-stored secr }); }); +// --------------------------------------------------------------------------- +// The `connector` read: a top-level key the authored entry does not declare +// --------------------------------------------------------------------------- +// +// A row stored while the connector entry was open can hold a top-level key the +// entry does not declare. Seeded directly, never through `saveMetaItem`: the +// save door now refuses such a key, so only a direct seed reproduces the row. + +const ROOT_KEY_VALUE = 'TEST_PLACEHOLDER_ROOT_KEY_VALUE'; +const ROOT_PLAIN_VALUE = 'TEST_PLACEHOLDER_ROOT_PLAIN_VALUE'; + +function storedConnectorBody() { + return { + name: 'vendor_api', + label: 'Vendor API', + type: 'api', + provider: 'rest', + providerConfig: { baseUrl: 'https://api.vendor.example/v1' }, + auth: { type: 'bearer', credentialRef: 'VENDOR_API_TOKEN' }, + apiKey: ROOT_KEY_VALUE, + region: ROOT_PLAIN_VALUE, + }; +} + +function seedConnectorRow(rows: Map) { + const where = { type: 'connector', name: 'vendor_api', organization_id: null, package_id: null, state: 'active' }; + const body = storedConnectorBody(); + rows.set(keyOf(where), { + id: 'r_connector', + ...where, + metadata: JSON.stringify(body), + checksum: hashSpec(body), + version: 1, + } as Row); +} + +const storedConnector = (rows: Map) => + JSON.parse(Array.from(rows.values()).find((r) => r.type === 'connector' && r.state === 'active')!.metadata); + +function expectRootKeysWithheld(served: any) { + expect(served).not.toHaveProperty('apiKey'); + expect(served).not.toHaveProperty('region'); + expect(served.name).toBe('vendor_api'); + expect(served.providerConfig).toEqual({ baseUrl: 'https://api.vendor.example/v1' }); +} + +function expectNoRootKeyValue(payload: unknown) { + const strings = allStrings(payload); + expect(strings.some((s) => s.includes(ROOT_KEY_VALUE) || s.includes(ROOT_PLAIN_VALUE))).toBe(false); +} + +const BUILTIN_CONNECTOR_REDACTOR = getMetadataTypeRedactor('connector')!; + +describe('the connector read withholds root keys the entry shape does not declare', () => { + afterEach(() => registerMetadataTypeRedactor('connector', BUILTIN_CONNECTOR_REDACTOR)); + + it('withholds undeclared root keys on read: the item read', async () => { + const { engine, rows } = makeStubEngine(); + seedConnectorRow(rows); + const protocol = new ObjectStackProtocolImplementation(engine); + + const res: any = await protocol.getMetaItem({ type: 'connector', name: 'vendor_api' }); + expectRootKeysWithheld(res.item); + expectNoRootKeyValue(res); + // The badge is computed on the raw body: the row is still reported invalid. + expect(res.item._diagnostics.valid).toBe(false); + expect(storedConnector(rows).apiKey).toBe(ROOT_KEY_VALUE); + }); + + it('withholds undeclared root keys on read: the list read, both spellings', async () => { + const { engine, rows } = makeStubEngine(); + seedConnectorRow(rows); + const protocol = new ObjectStackProtocolImplementation(engine); + + for (const type of ['connector', 'connectors']) { + const res: any = await protocol.getMetaItems({ type }); + const item = res.items.find((i: any) => i.name === 'vendor_api'); + expect(item, type).toBeDefined(); + expectRootKeysWithheld(item); + expectNoRootKeyValue(res); + } + }); + + it('withholds undeclared root keys on read: the layered read', async () => { + const { engine, rows } = makeStubEngine(); + seedConnectorRow(rows); + const protocol = new ObjectStackProtocolImplementation(engine); + + const res: any = await protocol.getMetaItemLayered({ type: 'connector', name: 'vendor_api' }); + expect(res.overlay).not.toBeNull(); + expectRootKeysWithheld(res.overlay); + expectRootKeysWithheld(res.effective); + expectNoRootKeyValue(res); + }); + + it('a save of the served form keeps the withheld keys at rest', async () => { + const { engine, rows } = makeStubEngine(); + seedConnectorRow(rows); + const protocol = new ObjectStackProtocolImplementation(engine); + + const served: any = (await protocol.getMetaItem({ type: 'connector', name: 'vendor_api' })).item; + await protocol.saveMetaItem({ + type: 'connector', + name: 'vendor_api', + item: { ...served, label: 'Vendor API (edited)' }, + }); + + const atRest = storedConnector(rows); + expect(atRest.label).toBe('Vendor API (edited)'); + expect(atRest.apiKey).toBe(ROOT_KEY_VALUE); + expect(atRest.region).toBe(ROOT_PLAIN_VALUE); + const reread: any = await protocol.getMetaItem({ type: 'connector', name: 'vendor_api' }); + expectRootKeysWithheld(reread.item); + }); + + it('an undeclared root key the author types is refused, and the row is unchanged', async () => { + const { engine, rows } = makeStubEngine(); + seedConnectorRow(rows); + const protocol = new ObjectStackProtocolImplementation(engine); + + const served: any = (await protocol.getMetaItem({ type: 'connector', name: 'vendor_api' })).item; + await expect(protocol.saveMetaItem({ + type: 'connector', + name: 'vendor_api', + item: { ...served, apiKey: 'typed-by-hand' }, + })).rejects.toMatchObject({ code: 'INVALID_METADATA', status: 422 }); + expect(storedConnector(rows)).toEqual(storedConnectorBody()); + }); + + it('ABLATION — with an identity `connector` redactor the item, list and layered reads carry the keys again', async () => { + registerMetadataTypeRedactor('connector', (item) => ({ item, redactedKeys: [] })); + const { engine, rows } = makeStubEngine(); + seedConnectorRow(rows); + const protocol = new ObjectStackProtocolImplementation(engine); + + expect(((await protocol.getMetaItem({ type: 'connector', name: 'vendor_api' })) as any).item.apiKey).toBe(ROOT_KEY_VALUE); + expect(((await protocol.getMetaItems({ type: 'connector' })) as any).items[0].apiKey).toBe(ROOT_KEY_VALUE); + expect(((await protocol.getMetaItemLayered({ type: 'connector', name: 'vendor_api' })) as any).effective.apiKey).toBe(ROOT_KEY_VALUE); + }); +}); + +// --------------------------------------------------------------------------- +// The carry-forward keeps a withheld key whose own name carries a dot +// --------------------------------------------------------------------------- +// +// `redactedKeys` joins each path on dots, so a key spelled `v1.url` reads like +// a nested path `v1` → `url`. A save of the served form must still keep it at +// rest: the carry-forward tries the standard split first, then the whole +// string as a single key. + +const DOTTED_ROOT_VALUE = 'TEST_PLACEHOLDER_DOTTED_ROOT_VALUE'; +const DOTTED_PROVIDER_URL = 'https://svc:TEST_PLACEHOLDER_DOTTED_PROVIDER@api.vendor.example/v1'; + +function storedDottedConnectorBody() { + return { + name: 'vendor_api', + label: 'Vendor API', + type: 'api', + provider: 'rest', + providerConfig: { baseUrl: 'https://api.vendor.example/v1', 'v1.url': DOTTED_PROVIDER_URL }, + auth: { type: 'bearer', credentialRef: 'VENDOR_API_TOKEN' }, + 'x.y': DOTTED_ROOT_VALUE, + }; +} + +function seedDottedConnectorRow(rows: Map) { + const where = { type: 'connector', name: 'vendor_api', organization_id: null, package_id: null, state: 'active' }; + const body = storedDottedConnectorBody(); + rows.set(keyOf(where), { + id: 'r_connector_dotted', ...where, metadata: JSON.stringify(body), checksum: hashSpec(body), version: 1, + } as Row); +} + +describe('the connector carry-forward keeps a withheld dot-spelled key at rest', () => { + const BUILTIN_CONNECTOR_REDACTOR = getMetadataTypeRedactor('connector')!; + afterEach(() => registerMetadataTypeRedactor('connector', BUILTIN_CONNECTOR_REDACTOR)); + + it('the served read withholds the dot-spelled root key and the dot-spelled providerConfig URL', async () => { + const { engine, rows } = makeStubEngine(); + seedDottedConnectorRow(rows); + const protocol = new ObjectStackProtocolImplementation(engine); + const served: any = (await protocol.getMetaItem({ type: 'connector', name: 'vendor_api' })).item; + expect(served['x.y']).toBeUndefined(); + expect(served.providerConfig['v1.url']).toBeUndefined(); + expect(served.providerConfig.baseUrl).toBe('https://api.vendor.example/v1'); + expect(allStrings(served)).not.toContain(DOTTED_ROOT_VALUE); + }); + + it('carryForwardRedactedValues restores both dot-spelled positions into a served body', () => { + const stored = storedDottedConnectorBody(); + const served: any = redactMetadataItem('connector', stored); + const out: any = carryForwardRedactedValues('connector', served, stored); + expect(out['x.y']).toBe(DOTTED_ROOT_VALUE); + expect(out.providerConfig['v1.url']).toBe(DOTTED_PROVIDER_URL); + // redactedPathsCarriedForward names the same positions it fills. + expect(redactedPathsCarriedForward('connector', served, stored).sort()) + .toEqual(['providerConfig.v1.url', 'x.y']); + }); + + it('a save of the served form keeps both dot-spelled values at rest', async () => { + const { engine, rows } = makeStubEngine(); + seedDottedConnectorRow(rows); + const protocol = new ObjectStackProtocolImplementation(engine); + const served: any = (await protocol.getMetaItem({ type: 'connector', name: 'vendor_api' })).item; + await protocol.saveMetaItem({ type: 'connector', name: 'vendor_api', item: { ...served, label: 'Vendor API (edited)' } }); + const atRest = storedConnector(rows); + expect(atRest.label).toBe('Vendor API (edited)'); + expect(atRest['x.y']).toBe(DOTTED_ROOT_VALUE); + expect(atRest.providerConfig['v1.url']).toBe(DOTTED_PROVIDER_URL); + }); + + it('the author`s own value wins — a new dot-spelled providerConfig URL is not refused by the entry (it is not a top-level key) and replaces the stored one', async () => { + // A dot-spelled key UNDER providerConfig is a provider setting, not a + // top-level entry key, so the closed entry does not refuse it; the + // author editing it is the author's word. + const { engine, rows } = makeStubEngine(); + seedDottedConnectorRow(rows); + const protocol = new ObjectStackProtocolImplementation(engine); + const served: any = (await protocol.getMetaItem({ type: 'connector', name: 'vendor_api' })).item; + const edited = { ...served, providerConfig: { ...served.providerConfig, 'v1.url': 'https://api.vendor.example/v2' } }; + await protocol.saveMetaItem({ type: 'connector', name: 'vendor_api', item: edited }); + expect(storedConnector(rows).providerConfig['v1.url']).toBe('https://api.vendor.example/v2'); + }); + + it('the dotted-alias ambiguity is explicit: a nested position and a same-spelled dotted key collide, and neither is carried', () => { + // Two stored positions whose `redactedKeys` entry is the IDENTICAL + // string `providerConfig.v1.url`: the nested `{ v1: { url } }` (its URL + // rewritten in place) and the literal `'v1.url'` (withheld whole). The + // two cannot be told apart from one dotted string, so the carry-forward + // refuses to guess — its two-onto-one guard carries NEITHER rather than + // graft a stored credential onto the wrong position. The read stays + // fail-closed (both secrets absent from what is served); a save of the + // served form does not restore either, which is the honest outcome of a + // genuinely ambiguous spelling. + const nestedUrl = 'https://svc:TEST_PLACEHOLDER_NESTED@api.vendor.example/nested'; + const literalUrl = 'https://svc:TEST_PLACEHOLDER_LITERAL@api.vendor.example/literal'; + const stored = { + name: 'vendor_api', label: 'Vendor API', type: 'api', provider: 'rest', + auth: { type: 'bearer', credentialRef: 'VENDOR_API_TOKEN' }, + providerConfig: { v1: { url: nestedUrl }, 'v1.url': literalUrl }, + }; + const served: any = redactMetadataItem('connector', stored); + expect(allStrings(served)).not.toContain('TEST_PLACEHOLDER_NESTED'); + expect(allStrings(served)).not.toContain('TEST_PLACEHOLDER_LITERAL'); + // The same string names both withheld positions. + const redactedForStored = (getMetadataTypeRedactor('connector')!)(stored as any).redactedKeys; + expect(redactedForStored.filter((p) => p === 'providerConfig.v1.url')).toHaveLength(2); + // The collision trips the two-onto-one guard: nothing is carried. + expect(redactedPathsCarriedForward('connector', served, stored)).toEqual([]); + expect(carryForwardRedactedValues('connector', served, stored)).toBe(served); + }); +}); + describe('the read exits withhold the declared ssl secret member of a datasource', () => { /** The stored client key, which no read exit may serve; the public members it sits beside. */ const SSL_KEY = 'TEST_PLACEHOLDER_SSL_KEY'; diff --git a/packages/qa/dogfood/test/connector-meta-door-root-key-read.dogfood.test.ts b/packages/qa/dogfood/test/connector-meta-door-root-key-read.dogfood.test.ts new file mode 100644 index 00000000000..87072910eda --- /dev/null +++ b/packages/qa/dogfood/test/connector-meta-door-root-key-read.dogfood.test.ts @@ -0,0 +1,216 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The connector metadata read doors, booted on the showcase: a stored row +// holding a top-level key the connector entry does not declare is served +// without that key, and a save of the served form keeps it at rest. +// +// - READ: the row is seeded through the system context (the save door refuses +// such a key, so that is the only way the row is written). The item read, +// the layered view, the list (both spellings) and the history diff serve +// neither the key's value nor the key, to a member and to an administrator. +// - CARRY-FORWARD: a save of the served form succeeds, keeps both keys at rest, +// and the next read still withholds them. +// +// Every value 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 { type VerifyStack } from '@objectstack/verify'; +import { bootShowcase } from './showcase-boot.js'; + +const SYSTEM = { isSystem: true, positions: [], permissions: [] }; + +const PLACEHOLDERS = { + credentialShaped: 'TEST_PLACEHOLDER_ROOT_CREDENTIAL_SHAPED', + plain: 'TEST_PLACEHOLDER_ROOT_PLAIN', + dottedRoot: 'TEST_PLACEHOLDER_DOTTED_ROOT', + dottedUrl: 'TEST_PLACEHOLDER_DOTTED_PROVIDER_URL', +} as const; +const ALL_PLACEHOLDERS = Object.values(PLACEHOLDERS); + +const ROW = 'crk_root_keys_stored'; +const DOTTED = 'crk_dotted_keys_stored'; + +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 ALL_PLACEHOLDERS.some((p) => text.includes(p)); +}; + +describe('connector metadata read doors: root keys the entry does not declare are withheld', () => { + let stack: VerifyStack; + let admin: string; + let member: string; + let ql: any; + + 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: `CRK ${name}`, + type: 'api', + description: 'A provider-bound instance for the root-key read pins.', + provider: 'rest', + providerConfig: { baseUrl: 'http://127.0.0.1:9/v1' }, + auth: { type: 'bearer', credentialRef: 'CRK_CREDENTIAL_REF' }, + ...over, + }); + + /** One door read: 200, names the 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 root key`).toBe(false); + return text; + }; + + const expectRootKeysAbsent = (served: Record, at: string) => { + expect(served, at).not.toHaveProperty('apiKey'); + expect(served, at).not.toHaveProperty('region'); + expect(served.provider, at).toBe('rest'); + }; + + beforeAll(async () => { + stack = await bootShowcase({}); + ql = await stack.kernel.getServiceAsync('objectql'); + admin = await stack.signIn(); + member = await stack.signUp('crk-member@connector-root-keys.test'); + + // A clean save makes the row; the system context then writes the legacy body over it. + const put = await stack.apiAs(admin, 'PUT', `/meta/connector/${ROW}`, instance(ROW)); + expect(put.status, await put.clone().text()).toBe(200); + const [row] = await stored(ROW); + await ql.update('sys_metadata', { + id: row.id, + metadata: JSON.stringify({ + ...instance(ROW, { label: 'CRK root keys (seeded)' }), + apiKey: PLACEHOLDERS.credentialShaped, + region: PLACEHOLDERS.plain, + }), + }, { context: SYSTEM }); + }, 180_000); + + afterAll(async () => { + await stack?.stop?.(); + }); + + it('guard the guard: the stored row holds both undeclared root keys', async () => { + const body = bodyOf((await stored(ROW))[0]); + expect(body.label).toBe('CRK root keys (seeded)'); + expect(body.apiKey).toBe(PLACEHOLDERS.credentialShaped); + expect(body.region).toBe(PLACEHOLDERS.plain); + }); + + it('withholds undeclared root keys on read: the item read and the layered view, for a member and an administrator', async () => { + for (const token of [member, admin]) { + const text = await expectWithheld(token, `/meta/connector/${ROW}`, [ROW, 'CRK root keys (seeded)']); + const served = itemOf(JSON.parse(text)); + expectRootKeysAbsent(served, 'item'); + expect(served.providerConfig).toEqual({ baseUrl: 'http://127.0.0.1:9/v1' }); + + const layered = JSON.parse(await expectWithheld(token, `/meta/connector/${ROW}/layers`, [ROW])); + const layers = layered?.data ?? layered; + expectRootKeysAbsent(layers.effective, 'layers.effective'); + if (layers.overlay) expectRootKeysAbsent(layers.overlay, 'layers.overlay'); + } + }); + + it('withholds undeclared root keys on read: the list, both spellings, for a member and an administrator', async () => { + for (const token of [member, admin]) { + await expectWithheld(token, '/meta/connector', [ROW]); + await expectWithheld(token, '/meta/connectors', [ROW]); + } + }); + + it('a save of the served form keeps the keys at rest; the next read and the history diff still withhold them', async () => { + const read = await stack.apiAs(admin, 'GET', `/meta/connector/${ROW}`); + const served = itemOf(await read.json()); + expectRootKeysAbsent(served, 'served'); + + const body = Object.fromEntries(Object.entries(served).filter(([k]) => !k.startsWith('_'))); + const resave = await stack.apiAs(admin, 'PUT', `/meta/connector/${ROW}`, { ...body, label: 'CRK root keys (edited)' }); + expect(resave.status, await resave.clone().text()).toBe(200); + + const atRest = bodyOf((await stored(ROW))[0]); + expect(atRest.label).toBe('CRK root keys (edited)'); + expect(atRest.apiKey).toBe(PLACEHOLDERS.credentialShaped); + expect(atRest.region).toBe(PLACEHOLDERS.plain); + + const reread = itemOf(JSON.parse(await expectWithheld(admin, `/meta/connector/${ROW}`, ['CRK root keys (edited)']))); + expectRootKeysAbsent(reread, 'reread'); + + // The history now holds the clean first save and the re-save that kept the keys. + const diffText = await expectWithheld(admin, `/meta/connector/${ROW}/diff`, ['CRK root keys (edited)']); + const diff = JSON.parse(diffText); + const answer = diff?.data ?? diff; + const added = (answer.added ?? []) as Array<{ path: string; value?: unknown }>; + const apiKey = added.find((e) => e.path === 'apiKey'); + expect(apiKey, diffText.slice(0, 400)).toBeDefined(); + expect(apiKey!.value).toBeUndefined(); + }); + + it('the stored row is never redacted: redaction is a serving act', async () => { + const body = bodyOf((await stored(ROW))[0]); + expect(body.apiKey).toBe(PLACEHOLDERS.credentialShaped); + expect(body.region).toBe(PLACEHOLDERS.plain); + }); + + it('a cleared undeclared root key is withheld so a plain read then save answers 200, not 422', async () => { + const put = await stack.apiAs(admin, 'PUT', `/meta/connector/crk_cleared`, instance('crk_cleared')); + expect(put.status, await put.clone().text()).toBe(200); + const [row] = await stored('crk_cleared'); + await ql.update('sys_metadata', { + id: row.id, + metadata: JSON.stringify({ ...instance('crk_cleared', { label: 'CRK cleared (seeded)' }), apiKey: '', extra: {} }), + }, { context: SYSTEM }); + + const text = await expectWithheld(admin, `/meta/connector/crk_cleared`, ['crk_cleared']); + const served = itemOf(JSON.parse(text)); + expect(served).not.toHaveProperty('apiKey'); + expect(served).not.toHaveProperty('extra'); + + const body = Object.fromEntries(Object.entries(served).filter(([k]) => !k.startsWith('_'))); + const resave = await stack.apiAs(admin, 'PUT', `/meta/connector/crk_cleared`, { ...body, label: 'CRK cleared (edited)' }); + expect(resave.status, await resave.clone().text()).toBe(200); + }); + + it('a withheld dot-spelled root key and a dot-spelled providerConfig URL survive a save of the served form', async () => { + const put = await stack.apiAs(admin, 'PUT', `/meta/connector/${DOTTED}`, instance(DOTTED)); + expect(put.status, await put.clone().text()).toBe(200); + const [row] = await stored(DOTTED); + const dottedUrl = `https://svc:${PLACEHOLDERS.dottedUrl}@api.vendor.example/v1`; + await ql.update('sys_metadata', { + id: row.id, + metadata: JSON.stringify({ + ...instance(DOTTED, { label: 'CRK dotted (seeded)' }), + providerConfig: { baseUrl: 'https://api.vendor.example/v1', 'v1.url': dottedUrl }, + 'x.y': PLACEHOLDERS.dottedRoot, + }), + }, { context: SYSTEM }); + + const text = await expectWithheld(admin, `/meta/connector/${DOTTED}`, ['CRK dotted (seeded)']); + const served = itemOf(JSON.parse(text)); + expect(served).not.toHaveProperty('x.y'); + expect(served.providerConfig).not.toHaveProperty('v1.url'); + + const body = Object.fromEntries(Object.entries(served).filter(([k]) => !k.startsWith('_'))); + const resave = await stack.apiAs(admin, 'PUT', `/meta/connector/${DOTTED}`, { ...body, label: 'CRK dotted (edited)' }); + expect(resave.status, await resave.clone().text()).toBe(200); + + const atRest = bodyOf((await stored(DOTTED))[0]); + expect(atRest.label).toBe('CRK dotted (edited)'); + expect(atRest['x.y']).toBe(PLACEHOLDERS.dottedRoot); + expect(atRest.providerConfig['v1.url']).toBe(dottedUrl); + + // And the next read still withholds both. + const reread = itemOf(JSON.parse(await expectWithheld(admin, `/meta/connector/${DOTTED}`, ['CRK dotted (edited)']))); + expect(reread).not.toHaveProperty('x.y'); + expect(reread.providerConfig).not.toHaveProperty('v1.url'); + }); +}); diff --git a/packages/spec/src/integration/connector-credential-redaction.test.ts b/packages/spec/src/integration/connector-credential-redaction.test.ts index 1c62f51377c..e25e4bbe5b2 100644 --- a/packages/spec/src/integration/connector-credential-redaction.test.ts +++ b/packages/spec/src/integration/connector-credential-redaction.test.ts @@ -13,8 +13,11 @@ import { CONNECTOR_CREDENTIAL_MAP_PATHS, CONNECTOR_CREDENTIAL_URL_QUERY_PARAM_NAMES, declaredConnectorAuthKeys, + declaredConnectorEntryKeys, redactConnectorItem, } from './connector-credential-redaction'; +import { DeclarativeConnectorEntrySchema } from './connector.zod'; +import { METADATA_READ_DECORATIONS } from '../kernel/metadata-read-decorations'; const P = (n: number) => `TEST_PLACEHOLDER_${n}`; @@ -224,15 +227,30 @@ describe('the connector redactor — credentials inside a URL under `providerCon expect(redactConnectorItem(item)).toEqual({ item, redactedKeys: [] }); }); - it('arrays and dot-spelled keys are off the walk (the carry-forward cannot address them)', () => { - const item = { - ...base, - providerConfig: { - servers: [`https://u:${P(39)}@a.example`], - 'v1.url': `https://u:${P(40)}@b.example`, - }, - }; - expect(redactConnectorItem(item)).toEqual({ item, redactedKeys: [] }); + it('withholds whole an array holding a URL with credential parts, at any depth', () => { + const { item: served, redactedKeys } = at({ + servers: [`https://u:${P(39)}@a.example`, 'https://b.example'], + mirrors: ['https://c.example', `https://d.example/v1?token=${P(42)}`], + transport: { kind: 'http', fallbacks: [{ url: `https://u:${P(43)}@e.example` }] }, + include: ['search', 'https://f.example/v1?region=eu'], + }); + expect(redactedKeys).toEqual(['providerConfig.mirrors', 'providerConfig.servers', 'providerConfig.transport.fallbacks']); + expect(served.providerConfig).toEqual({ + transport: { kind: 'http' }, + include: ['search', 'https://f.example/v1?region=eu'], + }); + expect(holds(served, P(39), P(42), P(43))).toBe(false); + }); + + it('withholds whole a dot-spelled key under providerConfig that holds a credential URL (fail closed)', () => { + const { item: served, redactedKeys } = at({ + 'v1.url': `https://u:${P(40)}@b.example`, + 'legacy.list': ['https://c.example', `https://d.example/v1?token=${P(44)}`], + 'plain.note': 'https://e.example/v1?region=eu', + }); + expect(redactedKeys).toEqual(['providerConfig.legacy.list', 'providerConfig.v1.url']); + expect(served.providerConfig).toEqual({ 'plain.note': 'https://e.example/v1?region=eu' }); + expect(holds(served, P(40), P(44))).toBe(false); }); it('a map withheld whole is not walked for URLs: it is reported once, as the map', () => { @@ -247,6 +265,65 @@ describe('the connector redactor — credentials inside a URL under `providerCon }); }); +describe('the connector redactor — top-level keys the entry does not declare', () => { + it.each([ + ['a credential-shaped scalar', { apiKey: P(50) }, ['apiKey']], + ['several credential-shaped keys', { token: P(51), clientSecret: P(52), password: P(53) }, ['clientSecret', 'password', 'token']], + ['a nested container', { credentials: { user: 'svc', secret: P(54) } }, ['credentials']], + ['a plain undeclared key', { region: 'eu-west-1' }, ['region']], + ] as const)('withholds undeclared root keys on read: %s', (_label, extra, keys) => { + const item = { ...base, providerConfig: { baseUrl: 'https://v.example' }, ...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(50), P(51), P(52), P(53), P(54), 'eu-west-1')).toBe(false); + expect(served.providerConfig).toEqual({ baseUrl: 'https://v.example' }); + expect(served._diagnostics).toEqual(base._diagnostics); + }); + + it('the declared set is the closed entry\'s own shape, and every declared key is served as stored', () => { + const shape = (DeclarativeConnectorEntrySchema as unknown as { shape: Record }).shape; + expect([...declaredConnectorEntryKeys()].sort()).toEqual(Object.keys(shape).sort()); + for (const key of ['name', 'label', 'type', 'provider', 'providerConfig', 'auth', 'enabled', '_packageId', '_provenance']) { + expect(declaredConnectorEntryKeys().has(key), key).toBe(true); + } + const item = { ...base, providerConfig: { baseUrl: 'https://v.example' }, retryConfig: { maxAttempts: 3 }, _packageId: 'com.vendor' }; + expect(redactConnectorItem(item)).toEqual({ item, redactedKeys: [] }); + }); + + it('the read decorations are the response\'s and are left alone', () => { + const item = { ...base, _draft: true }; + for (const key of METADATA_READ_DECORATIONS) expect(item).toHaveProperty(key); + expect(redactConnectorItem(item)).toEqual({ item, redactedKeys: [] }); + }); + + it('withholds a cleared undeclared key too, and names it (the save door refuses it whatever its value)', () => { + const item = { ...base, apiKey: '', token: null, extra: {} }; + const { item: served, redactedKeys } = redactConnectorItem(item); + expect(redactedKeys).toEqual(['apiKey', 'extra', 'token']); + for (const key of ['apiKey', 'token', 'extra']) expect(served).not.toHaveProperty(key); + }); + + it('withholds an undeclared root key whose own name carries a dot, and names it whole', () => { + const item = { ...base, 'x.y': P(59) }; + const { item: served, redactedKeys } = redactConnectorItem(item); + expect(redactedKeys).toEqual(['x.y']); + expect(served).not.toHaveProperty('x.y'); + expect(holds(served, P(59))).toBe(false); + }); + + it('withholds an undeclared root key beside the other positions, each named once', () => { + const { item: served, redactedKeys } = redactConnectorItem({ + ...base, + apiKey: P(55), + auth: { type: 'bearer', credentialRef: 'VENDOR_TOKEN', token: P(56) }, + providerConfig: { baseUrl: `https://u:${P(57)}@v.example`, headers: { Authorization: P(58) } }, + }); + expect(redactedKeys).toEqual(['apiKey', 'auth.token', 'providerConfig.baseUrl', 'providerConfig.headers']); + expect(holds(served, P(55), P(56), P(57), P(58))).toBe(false); + }); +}); + 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 6c94d4d1979..0e5f3f194c6 100644 --- a/packages/spec/src/integration/connector-credential-redaction.ts +++ b/packages/spec/src/integration/connector-credential-redaction.ts @@ -59,10 +59,34 @@ * `providerConfig.transport.url` (`mcp` over http) and a URL * `providerConfig.spec` (`openapi`), and any other URL a provider keeps at * an OBJECT depth of `providerConfig`: a string is judged when it starts - * with `scheme://`. Arrays are off the walk, as they are for the datasource - * scrub, and so is a key spelled with a dot: the carry-forward addresses a - * withheld position by its dotted path, so a position it cannot address - * would turn a save of the served form into a silent credential deletion. + * with `scheme://`. + * 6. **An array under `providerConfig` that holds such a URL, WHOLE**, and + * **a key under `providerConfig` whose own name carries a dot that holds + * such a URL at any depth, WHOLE.** The carry-forward cannot address an + * array element by position, nor a dotted key segment by segment (its + * dotted name is indistinguishable from a nested path once `redactedKeys` + * joins on dots), so neither is rewritten in place; each is withheld whole + * instead, as a non-object `auth` is (item 4), and the carry-forward keeps + * the stored value at rest by the whole-string fallback the protocol's + * carry-forward runs when the dotted split resolves to nothing. This is + * item 5's "avoid silent deletion" yielding to item 7's fail-closed: a + * dotted position that cannot be rewritten part-for-part is withheld + * rather than served as stored. An array or dotted key holding no such URL + * is served as stored. + * 7. **Every top-level key the authored entry does not declare, WHOLE — + * whatever its value.** The save door stores the body it accepts (ADR-0005 + * appendix (c), ruling B), so a row stored while the entry was open can + * hold a key no declared position names — and a key no position names is + * one this redactor cannot judge. The declared keys are read from + * {@link DeclarativeConnectorEntrySchema} itself (its read-through + * `shape`), so a key the entry declares later is served with nothing to + * edit here. The read decorations ({@link METADATA_READ_DECORATIONS}) are + * the response's, not the entry's, and are left alone. Unlike the + * credential positions above, a CLEARED undeclared key (`null`, `''`, an + * empty map) is withheld too, not served: the closed entry refuses the key + * 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). * * ## Cleared forms, and what is never touched * @@ -82,6 +106,8 @@ */ import { ConnectorInstanceAuthSchema } from '../shared/connector-auth.zod'; +import { DeclarativeConnectorEntrySchema } 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'; import { objectShapeOf, wrapperChainOf, zodDefOf } from '../kernel/metadata-credential-fields'; @@ -236,6 +262,41 @@ function readAt(item: Record, path: readonly string[]): { prese return { present: true, value: parent[leaf] }; } +/** + * Read once, on first use: the top-level keys the authored entry declares + * (item 7). The entry is a lazy schema and is not evaluated at module load; its + * read-through `shape` is the closed entry's own declaration. + */ +let declaredEntryKeys: ReadonlySet | undefined; +export function declaredConnectorEntryKeys(): ReadonlySet { + if (!declaredEntryKeys) { + const shape = (DeclarativeConnectorEntrySchema as unknown as { shape?: unknown }).shape; + // No readable shape ⇒ nothing is declared, and every key is withheld: fail closed. + declaredEntryKeys = new Set(isPlainRecord(shape) ? Object.keys(shape) : []); + } + return declaredEntryKeys; +} + +const READ_DECORATION_KEYS: ReadonlySet = new Set(METADATA_READ_DECORATIONS); + +/** + * Item 7: every top-level key the entry does not declare, as item-relative + * single-segment paths. The read decorations ({@link METADATA_READ_DECORATIONS}) + * are the response's, not the entry's, and are never counted. Withheld whole + * regardless of value — a cleared one included — unlike the credential + * positions below: an undeclared key the entry refuses can never be served, so + * serving its cleared form back makes a plain GET then PUT meet the closed save + * door. The carry-forward keeps the stored value at rest. + */ +function undeclaredRootKeysOf(item: Record): string[][] { + const declared = declaredConnectorEntryKeys(); + const paths: string[][] = []; + for (const key of Object.keys(item)) { + if (!declared.has(key) && !READ_DECORATION_KEYS.has(key)) paths.push([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[][] = []; @@ -264,25 +325,50 @@ function withValueAt(item: Record, path: readonly string[], val return { ...item, [head]: withValueAt(item[head] as Record, rest, value) }; } +/** Whether `value` is a URL whose credential parts {@link redactConnectorUrl} removes. */ +function isCredentialUrl(value: string, params: readonly string[]): boolean { + return URL_SCHEME_RE.test(value) && redactConnectorUrl(value, params) !== value; +} + +/** Whether `value` holds, at any depth, a URL {@link isCredentialUrl} judges (item 6). */ +function holdsCredentialUrl(value: unknown, params: readonly string[]): boolean { + if (typeof value === 'string') return isCredentialUrl(value, params); + if (Array.isArray(value)) return value.some((element) => holdsCredentialUrl(element, params)); + if (isPlainRecord(value)) return Object.values(value).some((child) => holdsCredentialUrl(child, params)); + return false; +} + /** - * Item 5: every URL under `providerConfig`, at object depth, whose credential - * parts {@link redactConnectorUrl} removes — the path and the rewritten value. + * Items 5 and 6: every URL under `providerConfig`, at object depth, whose + * credential parts {@link redactConnectorUrl} removes — the path and the + * rewritten value — and every array there holding such a URL, withheld whole + * (`value` absent). */ function credentialUrlRewritesOf( item: Record, -): Array<{ path: string[]; value: string }> { - const rewrites: Array<{ path: string[]; value: string }> = []; +): Array<{ path: string[]; value?: string }> { + const rewrites: Array<{ path: string[]; value?: string }> = []; const providerConfig = item.providerConfig; if (!isPlainRecord(providerConfig)) return rewrites; const params = credentialQueryParamsOf(item); const walk = (node: Record, prefix: readonly string[]): void => { for (const [key, value] of Object.entries(node)) { - if (key.includes('.')) continue; const path = [...prefix, key]; + // A key whose own name carries a dot cannot be addressed segment by + // segment (its dotted name is indistinguishable from a nested path once + // `redactedKeys` joins on dots), so it is never rewritten in place or + // descended into. Fail closed instead: if it holds a credential URL at + // any depth, withhold the key WHOLE — the carry-forward keeps the stored + // value at rest by the whole-string fallback. Item 5's "avoid silent + // deletion" yields to item 7's fail-closed here. + if (key.includes('.')) { + if (holdsCredentialUrl(value, params)) rewrites.push({ path }); + continue; + } if (typeof value === 'string') { - if (!URL_SCHEME_RE.test(value)) continue; - const redacted = redactConnectorUrl(value, params); - if (redacted !== value) rewrites.push({ path, value: redacted }); + if (isCredentialUrl(value, params)) rewrites.push({ path, value: redactConnectorUrl(value, params) }); + } else if (Array.isArray(value)) { + if (holdsCredentialUrl(value, params)) rewrites.push({ path }); } else if (isPlainRecord(value)) { walk(value, path); } @@ -300,6 +386,14 @@ 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)) { + const { present } = readAt(served, path); + if (!present) continue; + served = withoutKeyAt(served, path); + redactedKeys.push(path.join('.')); + } for (const path of credentialPathsOf(item)) { const { present, value } = readAt(served, path); if (!present || isClearedForm(value)) continue; @@ -308,7 +402,7 @@ export function redactConnectorItem(item: Record): MetadataReda } // After the removals, so a map withheld whole is never walked for URLs. for (const { path, value } of credentialUrlRewritesOf(served)) { - served = withValueAt(served, path, value); + served = value === undefined ? withoutKeyAt(served, path) : withValueAt(served, path, value); redactedKeys.push(path.join('.')); } if (redactedKeys.length === 0) return { item, redactedKeys: [] };