diff --git a/.changeset/20590-flow-credential-positions.md b/.changeset/20590-flow-credential-positions.md new file mode 100644 index 00000000000..53076603308 --- /dev/null +++ b/.changeset/20590-flow-credential-positions.md @@ -0,0 +1,36 @@ +--- +'@objectstack/service-automation': patch +'@objectstack/metadata-protocol': patch +'@objectstack/runtime': patch +--- + +fix(security): every credential a flow definition holds is withheld from what is served, at every depth, and an edit round trip keeps each one where it belongs (#20590) + +Clause-②: no + +**What is now withheld.** Beside an `api` flow's inbound-hook secret (the start node's +`config.secret`), every served flow definition now also withholds an `http` node's +outbound signing secret (`config.signingSecret`), and both are withheld wherever the +node sits: at the top level, or inside a `loop` body, a `parallel` branch, or a +`try_catch` region. The engine still executes the stored values. + +**Removing a signing secret.** A definition saved back without the key keeps the +stored secret, because an absent key is what every read serves. To remove it, save +the key as the empty string (`signingSecret: ''`): the durable callout is then +delivered unsigned, and the empty value is served as written, so the next round trip +keeps it cleared. + +**Changing a node's kind.** An edit that keeps a node's `id` and changes its kind no +longer carries that node's stored credential onto it. The credential belonged to the +old kind; a start node that needs a secret asks for one again at registration. + +**Moving a node.** A node moved into or out of a `loop` body, a `parallel` branch or a +`try_catch` region keeps its stored credential across the round trip, as long as its +`id` and kind are unchanged and it is the only node, at the top level or in any region, +that carries that `id`. An edge or a config value with the same `id` does not count. + +**The `/meta` list read on a dispatcher host.** When the metadata protocol's list read +fails, the list answers that failure (`503 SERVICE_UNAVAILABLE` for a store outage, or +the protocol's own refusal) instead of serving the metadata service's stored list, +which applies no credential redaction. A host whose protocol has no list verb keeps +its metadata-service fallback. diff --git a/packages/metadata-protocol/src/metadata-redaction.ts b/packages/metadata-protocol/src/metadata-redaction.ts index 0bff2b3f871..b5269351a3d 100644 --- a/packages/metadata-protocol/src/metadata-redaction.ts +++ b/packages/metadata-protocol/src/metadata-redaction.ts @@ -171,90 +171,222 @@ function elementWithIdentity( * reorders a flow's `nodes` sends the same start node back at another index, * and grafting by position would put its credential on whichever node now sits * where it used to. So an array hop is resolved ONCE, against the stored body, - * into the element's `id`, and every body — served and incoming — is then - * walked by that identity, never by the index. + * into an identity, and every body — served and incoming — is then walked by + * that identity, never by the index. Two kinds of identity: * - * An array hop whose stored element carries no string `id`, or shares it with - * a sibling, resolves to nothing: the path is skipped, exactly as every array - * hop was skipped before identity resolution existed. + * - **the element's own `id`** (`{ elementId }`) — a flow node; + * - **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 + * `nodes..config.branches..nodes..config.signingSecret`. The branch + * is the one element of `branches` whose own walk down the rest of the + * path reaches node ``'s `id` — and a flow's node ids are one space + * across every region (`FlowSchema`), so at most one branch does. `anchor` + * 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 — + * resolves to nothing, and the path is skipped. */ -type PathHop = { readonly key: string } | { readonly elementId: string }; +type PathHop = + | { readonly key: string } + | { readonly elementId: string } + | { readonly anchor: readonly PathHop[] }; /** * Resolve every CONTAINER hop of `segments` (all but the last, which names the * redacted key itself) against `stored`. `undefined` when the stored body does - * not reach that far — which the caller reads as "nothing at rest to carry". + * not reach that far, or an array hop has no identity — which the caller reads + * as "nothing at rest to carry". */ function resolveHops(stored: unknown, segments: readonly string[]): PathHop[] | undefined { - const hops: PathHop[] = []; + // First pass: key hops and `id` hops; `null` marks an element with no `id`. + const found: Array = []; let node: unknown = stored; for (let i = 0; i < segments.length - 1; i += 1) { const segment = segments[i] as string; if (Array.isArray(node)) { if (!/^(0|[1-9][0-9]*)$/.test(segment)) return undefined; const element = node[Number(segment)]; + if (!isPlainRecord(element)) return undefined; const elementId = identityOf(element); - if (elementId === undefined || !elementWithIdentity(node, elementId)) return undefined; - hops.push({ elementId }); + if (elementId !== undefined && !elementWithIdentity(node, elementId)) return undefined; + found.push(elementId === undefined ? null : { elementId }); node = element; continue; } if (!isPlainRecord(node)) return undefined; - hops.push({ key: segment }); + found.push({ key: segment }); node = node[segment]; } + // Second pass, from the end: anchor each id-less element on the first + // identified element below it. The anchor may itself cross an id-less + // element (a branch inside a branch), whose own anchor is already built. + const hops: PathHop[] = new Array(found.length); + let nextIdentified = -1; + for (let i = found.length - 1; i >= 0; i -= 1) { + const hop = found[i]; + if (hop === null) { + if (nextIdentified < 0) return undefined; + hops[i] = { anchor: hops.slice(i + 1, nextIdentified + 1) }; + continue; + } + hops[i] = hop as PathHop; + if ('elementId' in (hop as PathHop)) nextIdentified = i; + } return hops; } /** - * Walk to the plain object that OWNS the redacted key, hop by hop. + * Resolve ONE hop against `node`: the segment it stands for in THIS body (a + * key, or the element's index here) and the value it leads to. `undefined` + * when this body does not speak to the hop — the wrong kind of container, or + * an array hop no single element answers (none does, or two do and cannot be + * told apart: guessing would graft a credential onto whichever came first). + */ +function stepInto(node: unknown, hop: PathHop): { segment: string; next: unknown } | undefined { + if ('key' in hop) { + return isPlainRecord(node) ? { segment: hop.key, next: node[hop.key] } : undefined; + } + if (!Array.isArray(node)) return undefined; + let index: number | undefined; + for (let i = 0; i < node.length; i += 1) { + const hit = 'elementId' in hop + ? identityOf(node[i]) === hop.elementId + : isPlainRecord(node[i]) && containerAt(node[i], hop.anchor) !== undefined; + if (!hit) continue; + if (index !== undefined) return undefined; + index = i; + } + return index === undefined ? undefined : { segment: String(index), next: node[index] }; +} + +/** + * Walk `hops` in `root`: the plain object that OWNS the redacted key, and the + * concrete segments (a key, or an array index in THIS body) that reach it. * * `undefined` when any hop along the way is absent, is the wrong kind of - * container, or (for an array hop) holds no single element with that identity - * — which the caller must read as "this body does not speak to that path at - * all", never as "the value is absent". The distinction is the whole guard: a - * PUT body carrying no `config` key is an author removing the container, and + * container, or (for an array hop) is answered by no single element — which + * the caller must read as "this body does not speak to that path at all", + * never as "the value is absent". The distinction is the whole guard: a PUT + * body carrying no `config` key is an author removing the container, and * grafting `config.password` back onto it would MINT a config that holds * nothing but a credential. */ -function containerAt(root: unknown, hops: readonly PathHop[]): Record | undefined { +function locate(root: unknown, hops: readonly PathHop[]): { at: string[]; container: Record } | undefined { + const at: string[] = []; let node: unknown = root; for (const hop of hops) { - if ('key' in hop) { - if (!isPlainRecord(node)) return undefined; - node = node[hop.key]; - continue; + const step = stepInto(node, hop); + if (!step) return undefined; + at.push(step.segment); + node = step.next; + } + return isPlainRecord(node) ? { at, container: node } : undefined; +} + +/** {@link locate}, container only. */ +function containerAt(root: unknown, hops: readonly PathHop[]): Record | undefined { + return locate(root, hops)?.container; +} + +/** The value `at` names in `root`; every segment resolves (it came from a walk of `root`). */ +function valueAt(root: unknown, at: readonly string[]): unknown { + let node: unknown = root; + for (const segment of at) node = Array.isArray(node) ? node[Number(segment)] : (node as Record)[segment]; + return node; +} + +/** + * [#20590 round 1] Where the redacted key's container sits in `root` when the + * STORED path to it no longer resolves there — because the element that owns + * it MOVED, keeping its identity: a flow node taken out of a `loop` body, or + * put into a `parallel` branch, keeps its `id`, its kind and the withheld form, + * and only the regions around it change. Skipping the graft there would drop + * the stored credential at save, silently. + * + * The owner is the path's deepest identified element (its last `elementId` + * hop). It is looked for by that `id` across the WHOLE body — but only among + * the elements that stand where the owner stood: the elements of an array + * held under the SAME key as the owner's own array in the stored path (for a + * flow node that key is `nodes`, so a node at the top level or in any region + * counts, and an edge or a config value that happens to carry the same `id` + * does not — a flow keeps node ids and edge ids in separate spaces, #20590 + * round 2). The key is read from the stored hops, never named here. It is used + * only when exactly ONE such element carries that `id` — none, and the author + * removed it; two or more, and they cannot be told apart, so nothing is + * chosen. Uniqueness across the whole body is what keeps this safe for a type + * whose ids are not one space the way a flow's node ids are. From the owner, + * the rest of the path is walked as usual (the key hops down to the container, + * so an owner whose `config` the author removed still grafts nothing). + * + * An owner whose array is not held under a key — an array directly inside + * another array — has no key to scope by, and is not relocated. + * + * A path with no identified element — every datasource path, whose redactor + * never crosses an array — has no owner to find, and is unaffected. + */ +function relocateById(root: unknown, hops: readonly PathHop[]): { at: string[]; container: Record } | undefined { + let owner = -1; + for (let i = hops.length - 1; i >= 0; i -= 1) { + if ('elementId' in (hops[i] as PathHop)) { + owner = i; + break; } - if (!Array.isArray(node)) return undefined; - const found = elementWithIdentity(node, hop.elementId); - if (!found) return undefined; - node = found.element; } - return isPlainRecord(node) ? node : undefined; + if (owner < 1) return undefined; + const id = (hops[owner] as { elementId: string }).elementId; + // The key the owner's array is held under in the stored path. + const parent = hops[owner - 1] as PathHop; + if (!('key' in parent)) return undefined; + const arrayKey = parent.key; + + const matches: string[][] = []; + const visit = (node: unknown, at: string[]): void => { + if (Array.isArray(node)) { + node.forEach((value, index) => visit(value, [...at, String(index)])); + return; + } + if (!isPlainRecord(node)) return; + for (const [key, value] of Object.entries(node)) { + if (!value || typeof value !== 'object') continue; + if (key === arrayKey && Array.isArray(value)) { + value.forEach((element, index) => { + if (isPlainRecord(element) && element.id === id) matches.push([...at, key, String(index)]); + }); + } + visit(value, [...at, key]); + } + }; + visit(root, []); + if (matches.length !== 1) return undefined; + + const ownerAt = matches[0] as string[]; + const rest = locate(valueAt(root, ownerAt), hops.slice(owner + 1)); + return rest ? { at: [...ownerAt, ...rest.at], container: rest.container } : undefined; } /** - * Copy-on-write set of `value` under `key` at the end of `hops`, returning a - * new root and copying only the containers along the path — an array hop - * copies the array and replaces the one element it names. + * Copy-on-write set of `value` under `key` in the container `at` names, + * returning a new root and copying only the containers along the way — an + * array segment copies the array and replaces the one element it names. * - * Called only after {@link containerAt} found the container in `root`, so - * every hop resolves. The incoming request body belongs to the caller - * (`saveMetaItem` hands the same object to the audit trail and the registry - * write-through), so the carry-forward must not mutate it in place. + * `at` came from a walk of `root`, so every segment resolves. The incoming + * request body belongs to the caller (`saveMetaItem` hands the same object to + * the audit trail and the registry write-through), so the carry-forward must + * not mutate it in place. */ -function withValueAt(root: unknown, hops: readonly PathHop[], key: string, value: unknown): unknown { - if (hops.length === 0) return { ...(root as Record), [key]: value }; - const [hop, ...rest] = hops as [PathHop, ...PathHop[]]; - if ('key' in hop) { - const record = root as Record; - return { ...record, [hop.key]: withValueAt(record[hop.key], rest, key, value) }; +function withValueAt(root: unknown, at: readonly string[], key: string, value: unknown): unknown { + if (at.length === 0) return { ...(root as Record), [key]: value }; + const [head, ...rest] = at as [string, ...string[]]; + if (Array.isArray(root)) { + const next = root.slice(); + next[Number(head)] = withValueAt(root[Number(head)], rest, key, value); + return next; } - const array = root as unknown[]; - const found = elementWithIdentity(array, hop.elementId) as { index: number; element: Record }; - const next = array.slice(); - next[found.index] = withValueAt(found.element, rest, key, value); - return next; + const record = root as Record; + return { ...record, [head]: withValueAt(record[head], rest, key, value) }; } /** Structural equality for the values a redactor hides (scalars in practice; general by construction). */ @@ -295,8 +427,29 @@ function sameValue(a: unknown, b: unknown): boolean { * A path through an ARRAY (a flow's start node, `nodes..config.secret`, * #20552) is walked by the stored element's `id`, not by its index, so a body * that reorders the array still carries the value onto the element it came - * from — see {@link resolveHops}. An element with no `id`, or an `id` shared - * with a sibling, is never carried into. + * from — see {@link resolveHops}. An element with no `id` is walked by the + * identified element below it on the same path (a `parallel` branch, by the + * node inside it that holds the credential, #20590); one with neither, or an + * `id` shared with a sibling, is never carried into. + * + * ⛔ A carried value never lands where the read would SERVE it (#20590). The + * array hop follows an identity, while a redactor chooses what to withhold by + * whatever rule it has — a flow's by the node's KIND — and the two can + * disagree: an edit that keeps a node's `id` and changes its kind would have + * the stored credential grafted onto a node the next read serves whole. So + * the type's redactor is run over the grafted body, and a carried value whose + * path it no longer withholds is dropped rather than persisted. Changing the + * kind of the node that held a credential is the author's word about that + * credential: it is gone, and a kind that needs one asks for it again (an + * `api` flow's start node without a secret is refused at registration). + * + * A node MOVED across regions — out of a loop body, into a parallel branch — + * keeps its identity while the stored path around it stops resolving. Its + * container is then found by the owning element's `id` across the whole + * incoming body, exactly one match or none ({@link relocateById}), and the + * same position check decides whether the value lands: a moved node whose + * kind still holds the credential keeps it; one moved AND changed in kind + * does not. * * ⚠️ The first case is genuinely INDISTINGUISHABLE, not merely treated as * equal: an author who hand-deletes `:password` from a URL sends exactly the @@ -305,7 +458,10 @@ function sameValue(a: unknown, b: unknown): boolean { * deliberate choice of the safe side — preserving a credential an operator may * still depend on, over silently destroying one. The same ambiguity exists in * `restoreRedactedConfig`, and clearing a credential on purpose has an - * unambiguous door: change it, or delete the row. + * unambiguous door: change it, or delete the row — or, where the type's + * redactor serves one value as written because it holds no credential, send + * that value (a flow's empty string, #20590): it differs from the absent key + * that was served, so it is the author's word and it wins. * * @param type request-shaped metadata type (plural or singular). * @param incoming the body about to be persisted. @@ -324,7 +480,7 @@ export function carryForwardRedactedValues(type: string, incoming: T, stored: const served = redactor(stored as Record); if (served.redactedKeys.length === 0) return incoming; - let out = incoming as unknown as Record; + const grafts: Array<{ at: string[]; key: string; value: unknown }> = []; for (const path of served.redactedKeys) { // Dotted, item-relative — the registry's documented contract for // `redactedKeys` (`config.password`; an array hop is an index into the @@ -339,13 +495,36 @@ export function carryForwardRedactedValues(type: string, incoming: T, stored: const storedValue = storedParent?.[key]; if (storedValue === undefined) continue; - const incomingParent = containerAt(out, hops); - if (!incomingParent) continue; + // Where the container sits in the incoming body: along the stored + // path, or — the node that owns it having MOVED — at the one element + // anywhere in the body that carries the owner's id (#20590 round 1). + const target = locate(incoming, hops) ?? relocateById(incoming, hops); + if (!target) continue; const servedParent = containerAt(served.item, hops); - if (!sameValue(incomingParent[key], servedParent?.[key])) continue; + if (!sameValue(target.container[key], servedParent?.[key])) continue; + + grafts.push({ at: target.at, key, value: storedValue }); + } + + // Two stored paths landing on ONE incoming position cannot be told apart; + // neither is carried rather than one silently overwriting the other. + const landing = (graft: { at: readonly string[]; key: string }) => [...graft.at, graft.key].join('.'); + const counts = new Map(); + for (const graft of grafts) counts.set(landing(graft), (counts.get(landing(graft)) ?? 0) + 1); - out = withValueAt(out, hops, key, storedValue) as Record; + // [#20590] Keep only what the read would withhold where it now lands. A + // graft sets a leaf and moves no container, so each pass re-grafts the + // survivors onto the untouched incoming body; the set only shrinks, so + // this settles in at most one pass per graft. + let kept = grafts.filter((graft) => counts.get(landing(graft)) === 1); + for (;;) { + let out: unknown = incoming; + for (const graft of kept) out = withValueAt(out, graft.at, graft.key, graft.value); + if (kept.length === 0) return out as T; + const withheld = new Set(redactor(out as Record).redactedKeys); + const next = kept.filter((graft) => withheld.has(landing(graft))); + if (next.length === kept.length) return out as T; + kept = next; } - return out as unknown as T; } diff --git a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts index 41f55eee5ea..8a40f599541 100644 --- a/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts +++ b/packages/metadata-protocol/src/protocol.metadata-redaction.test.ts @@ -726,3 +726,452 @@ describe('#20552 — first save of a registry-only (code-authored) flow keeps it expect(startNodeOf(JSON.parse(overlayRow(rows, 'active')!.metadata)).config.secret).toBe(ROTATED); }); }); + +// --------------------------------------------------------------------------- +// #20590 position 3 — the carry-forward never lands a credential where the +// projection does not withhold it +// --------------------------------------------------------------------------- +// +// The two halves pick positions by different rules. The read side withholds by +// the element's KIND (a flow's start node); the write side follows an array hop +// by the stored element's IDENTITY (`id`). An ordinary round trip that keeps a +// node's id and changes its kind therefore used to graft the stored credential +// onto a node the next read serves whole. The save door runs the carry-forward +// after every authoring gate, so no node-config check stood in the way there. + +/** The served body of the seeded flow, relocated: the old start node's kind changed, a new start node added. */ +function relocate(served: any) { + const [finish, begin] = served.nodes; + return { + ...served, + nodes: [ + finish, + { ...begin, type: 'assignment', label: 'Was the start node', config: {} }, + { id: 'begin_v2', type: 'start', label: 'On Webhook', config: { triggerType: 'api', hookId: 'intake' } }, + ], + edges: [{ id: 'e1', source: 'begin_v2', target: 'finish' }], + }; +} + +describe('#20590 — a relocating round trip never carries a credential into a served position', () => { + beforeEach(() => registerMetadataTypeRedactor('flow', flowStandInRedactor)); + + it('carryForwardRedactedValues drops a carried value the projection would not withhold at its new position', () => { + const stored = storedInboundFlow(); + const incoming = relocate(redactMetadataItem('flow', stored)); + const out: any = carryForwardRedactedValues('flow', incoming, stored); + // What the next read serves from the persisted body. + expect(allStrings(redactMetadataItem('flow', out))).not.toContain(FLOW_SECRET); + expect(allStrings(out)).not.toContain(FLOW_SECRET); + }); + + it('the save door: the next served read of a relocated flow carries no credential', async () => { + const { engine, rows } = makeStubEngine(); + seedFlowRow(rows); + const protocol = new ObjectStackProtocolImplementation(engine); + + const served: any = (await protocol.getMetaItem({ type: 'flow', name: 'inbound_hook' })).item; + const { _diagnostics: _d, ...editable } = served; + void _d; + await protocol.saveMetaItem({ type: 'flow', name: 'inbound_hook', item: relocate(editable) }); + + const next: any = await protocol.getMetaItem({ type: 'flow', name: 'inbound_hook' }); + expect(next.item.nodes.map((n: any) => n.id)).toEqual(['finish', 'begin', 'begin_v2']); + expect(allStrings(next)).not.toContain(FLOW_SECRET); + const list: any = await protocol.getMetaItems({ type: 'flow' }); + expect(allStrings(list)).not.toContain(FLOW_SECRET); + // Nothing was grafted at rest either: the relocated node holds no credential. + expect(allStrings(storedFlowBody(rows))).not.toContain(FLOW_SECRET); + }); +}); + +// --------------------------------------------------------------------------- +// #20590 position 1 — a credential inside a REGION, and the removal door +// --------------------------------------------------------------------------- +// +// The real `flow` projection (service-automation `flow-credential-projection.ts`) +// withholds an `http` node's `config.signingSecret` and a start node's +// `config.secret` at every depth, and serves the cleared form (`''`) as +// written. This stand-in emits the SAME path shapes that projection is pinned to +// emit in `flow-credential-positions.test.ts` +// (`nodes..config.body.nodes..config.signingSecret`, +// `nodes..config.branches..nodes..config.signingSecret`), so what is +// exercised here is this package's half: the inverse walks those paths. + +const SIGNING = 'stored-signing-secret-20590'; +const ROTATED_SIGNING = 'rotated-signing-secret-20590'; +const STAND_IN_KEYS: Record = { start: 'secret', http: 'signingSecret' }; +const STAND_IN_REGIONS: Record> = { + loop: [['body', 'one']], + parallel: [['branches', 'many']], + try_catch: [['try', 'one'], ['catch', 'one']], +}; + +const regionFlowStandIn = (item: Record) => { + const redactedKeys: string[] = []; + const walkNodes = (nodes: any[], path: string): any[] => nodes.map((node, i) => { + if (!node || typeof node !== 'object' || !node.config || typeof node.config !== 'object') return node; + const at = `${path}.${i}`; + const config = { ...node.config }; + const key = STAND_IN_KEYS[node.type]; + if (key && key in config && config[key] !== '') { + delete config[key]; + redactedKeys.push(`${at}.config.${key}`); + } + for (const [slot, arity] of STAND_IN_REGIONS[node.type] ?? []) { + const value = config[slot]; + if (arity === 'one' && value?.nodes) config[slot] = { ...value, nodes: walkNodes(value.nodes, `${at}.config.${slot}.nodes`) }; + if (arity === 'many' && Array.isArray(value)) { + config[slot] = value.map((r: any, b: number) => (r?.nodes ? { ...r, nodes: walkNodes(r.nodes, `${at}.config.${slot}.${b}.nodes`) } : r)); + } + } + return { ...node, config }; + }); + if (!Array.isArray(item.nodes)) return { item, redactedKeys }; + const nodes = walkNodes(item.nodes as any[], 'nodes'); + return redactedKeys.length === 0 ? { item, redactedKeys } : { item: { ...item, nodes }, redactedKeys: redactedKeys.sort() }; +}; + +/** A flow whose outbound callouts sit inside a loop body and a parallel branch — the branch deliberately not first. */ +function storedCalloutFlow() { + return { + name: 'callouts', + label: 'Callouts', + type: 'autolaunched', + nodes: [ + { id: 'begin', type: 'start', label: 'Start', config: {} }, + { + id: 'each', type: 'loop', label: 'Each', + config: { + collection: '{rows}', + body: { nodes: [{ id: 'per_row', type: 'http', label: 'Per row', config: { url: 'https://a', durable: true, signingSecret: SIGNING } }], edges: [] }, + }, + }, + { + id: 'fan', type: 'parallel', label: 'Fan', + config: { + branches: [ + { name: 'quiet', nodes: [{ id: 'note', type: 'assignment', label: 'Note', config: {} }] }, + { name: 'loud', nodes: [{ id: 'push', type: 'http', label: 'Push', config: { url: 'https://b', durable: true, signingSecret: SIGNING } }] }, + ], + }, + }, + ], + edges: [], + }; +} + +const calloutOf = (flow: any, id: string): any => { + let hit: any; + const visit = (nodes: any[]) => nodes.forEach((n) => { + if (n?.id === id) hit = n; + const c = n?.config ?? {}; + if (c.body?.nodes) visit(c.body.nodes); + if (Array.isArray(c.branches)) c.branches.forEach((b: any) => visit(b?.nodes ?? [])); + for (const k of ['try', 'catch']) if (c[k]?.nodes) visit(c[k].nodes); + }); + visit(flow.nodes); + return hit; +}; + +describe('#20590 — the inverse carries a credential back into a region', () => { + beforeEach(() => registerMetadataTypeRedactor('flow', regionFlowStandIn)); + afterEach(() => registerMetadataTypeRedactor('flow', flowStandInRedactor)); + + it('keeps both nested signing secrets on a round trip that also REORDERS the parallel branches', () => { + const stored = storedCalloutFlow(); + const served: any = redactMetadataItem('flow', stored); + expect(allStrings(served)).not.toContain(SIGNING); + + const fan = served.nodes[2]; + const incoming = { + ...served, + label: 'Edited', + nodes: [served.nodes[0], served.nodes[1], { ...fan, config: { ...fan.config, branches: [fan.config.branches[1], fan.config.branches[0]] } }], + }; + const out: any = carryForwardRedactedValues('flow', incoming, stored); + expect(calloutOf(out, 'per_row').config.signingSecret).toBe(SIGNING); + expect(calloutOf(out, 'push').config.signingSecret).toBe(SIGNING); + // The branch the credential came from, not the one now at its old index. + expect(out.nodes[2].config.branches[0].name).toBe('loud'); + expect(calloutOf(out, 'note').config).toEqual({}); + expect(out.label).toBe('Edited'); + // Copy-on-write: the caller's body is untouched. + expect(allStrings(incoming)).not.toContain(SIGNING); + }); + + it('carries nothing into a branch that cannot be told apart from another', () => { + const stored = storedCalloutFlow(); + const served: any = redactMetadataItem('flow', stored); + const fan = served.nodes[2]; + const twinBranches = { ...fan, config: { ...fan.config, branches: [fan.config.branches[1], structuredClone(fan.config.branches[1])] } }; + const out: any = carryForwardRedactedValues('flow', { ...served, nodes: [served.nodes[0], served.nodes[1], twinBranches] }, stored); + expect(out.nodes[2].config.branches.every((b: any) => b.nodes[0].config.signingSecret === undefined)).toBe(true); + expect(calloutOf(out, 'per_row').config.signingSecret).toBe(SIGNING); + }); + + it('the removal door: the empty string clears a signing secret, an absent key keeps it, a value replaces it', () => { + const stored = storedCalloutFlow(); + const served: any = redactMetadataItem('flow', stored); + + const cleared = structuredClone(served); + calloutOf(cleared, 'push').config.signingSecret = ''; + const clearedOut: any = carryForwardRedactedValues('flow', cleared, stored); + expect(calloutOf(clearedOut, 'push').config.signingSecret).toBe(''); + expect(calloutOf(clearedOut, 'per_row').config.signingSecret).toBe(SIGNING); + // …and the cleared form is served as written, so the next round trip keeps it cleared. + const reread: any = redactMetadataItem('flow', clearedOut); + expect(calloutOf(reread, 'push').config.signingSecret).toBe(''); + expect(calloutOf(carryForwardRedactedValues('flow', reread, clearedOut), 'push').config.signingSecret).toBe(''); + + const rotated = structuredClone(served); + calloutOf(rotated, 'push').config.signingSecret = ROTATED_SIGNING; + expect(calloutOf(carryForwardRedactedValues('flow', rotated, stored), 'push').config.signingSecret).toBe(ROTATED_SIGNING); + + // Absent is never "remove": the served form saved straight back keeps both. + const kept: any = carryForwardRedactedValues('flow', served, stored); + expect(calloutOf(kept, 'push').config.signingSecret).toBe(SIGNING); + expect(calloutOf(kept, 'per_row').config.signingSecret).toBe(SIGNING); + }); + + it('a callout left in place inside a rebuilt region keeps its secret; one whose kind changed in place does not carry it', () => { + const stored = storedCalloutFlow(); + const served: any = redactMetadataItem('flow', stored); + const perRow = served.nodes[1].config.body.nodes[0]; + // `push` becomes an assignment in place; `per_row` stays an http node. + const fan = served.nodes[2]; + const incoming = { + ...served, + nodes: [ + served.nodes[0], + { ...served.nodes[1], config: { ...served.nodes[1].config, body: { nodes: [perRow], edges: [] } } }, + { ...fan, config: { ...fan.config, branches: [fan.config.branches[0], { ...fan.config.branches[1], nodes: [{ ...fan.config.branches[1].nodes[0], type: 'assignment', config: {} }] }] } }, + ], + }; + const out: any = carryForwardRedactedValues('flow', incoming, stored); + expect(calloutOf(out, 'per_row').config.signingSecret).toBe(SIGNING); + expect(calloutOf(out, 'push').type).toBe('assignment'); + expect(calloutOf(out, 'push').config.signingSecret).toBeUndefined(); + expect(allStrings(redactMetadataItem('flow', out))).not.toContain(SIGNING); + }); + + it('the save door round trip: the stored row keeps both nested secrets, and no served read carries them', async () => { + const { engine, rows } = makeStubEngine(); + const where = { type: 'flow', name: 'callouts', organization_id: null, package_id: null, state: 'active' }; + const body = storedCalloutFlow(); + rows.set(keyOf(where), { id: 'r_callouts', ...where, metadata: JSON.stringify(body), checksum: hashSpec(body), version: 1 } as Row); + const protocol = new ObjectStackProtocolImplementation(engine); + + const served: any = (await protocol.getMetaItem({ type: 'flow', name: 'callouts' })).item; + expect(allStrings(served)).not.toContain(SIGNING); + const { _diagnostics: _d, ...editable } = served; + void _d; + await protocol.saveMetaItem({ type: 'flow', name: 'callouts', item: { ...editable, label: 'Edited' } }); + + const at = JSON.parse(Array.from(rows.values()).find((r) => r.name === 'callouts' && r.state === 'active')!.metadata); + expect(at.label).toBe('Edited'); + expect(calloutOf(at, 'per_row').config.signingSecret).toBe(SIGNING); + expect(calloutOf(at, 'push').config.signingSecret).toBe(SIGNING); + expect(allStrings(await protocol.getMetaItems({ type: 'flow' }))).not.toContain(SIGNING); + }); +}); + +// --------------------------------------------------------------------------- +// #20590 patch round 1 — a node MOVED across regions keeps its credential +// --------------------------------------------------------------------------- +// +// The stored path names the credential's container through the regions the +// node sat in at rest. An author who moves that node — out of a loop body, into +// a parallel branch — keeping its id, its kind and the withheld form, sends a +// body in which that path no longer resolves. Skipping the graft there dropped +// the stored secret at save, silently, and the next durable callout went out +// unsigned. The node is found by its id across the whole incoming body instead +// (exactly one match), and the position check still decides whether it lands. + +/** {@link storedCalloutFlow} plus a top-level callout, so a node can move INTO a region too. */ +function storedMovesFlow() { + const flow: any = storedCalloutFlow(); + flow.name = 'moves'; + flow.nodes.push({ id: 'call', type: 'http', label: 'Call', config: { url: 'https://c', durable: true, signingSecret: SIGNING } }); + return flow; +} + +const NOOP = (id: string) => ({ id, type: 'assignment', label: id, config: {} }); + +/** The served body with `per_row` moved from the loop body to the top level (the body keeps a stand-in node). */ +function movedOutOfLoop(served: any) { + const [begin, each, fan, call] = served.nodes; + const perRow = each.config.body.nodes[0]; + return { + ...served, + nodes: [begin, { ...each, config: { ...each.config, body: { nodes: [NOOP('body_noop')], edges: [] } } }, fan, call, perRow], + }; +} + +describe('#20590 round 1 — a node moved across regions keeps its credential', () => { + beforeEach(() => registerMetadataTypeRedactor('flow', regionFlowStandIn)); + afterEach(() => registerMetadataTypeRedactor('flow', flowStandInRedactor)); + + it('(a) an http node moved out of a loop body to the top level: the secret is kept at rest and not served', () => { + const stored = storedMovesFlow(); + const served: any = redactMetadataItem('flow', stored); + const incoming = movedOutOfLoop(served); + const out: any = carryForwardRedactedValues('flow', incoming, stored); + expect(out.nodes[4].id).toBe('per_row'); + expect(out.nodes[4].config.signingSecret).toBe(SIGNING); + expect(calloutOf(out, 'push').config.signingSecret).toBe(SIGNING); + expect(calloutOf(out, 'call').config.signingSecret).toBe(SIGNING); + expect(allStrings(redactMetadataItem('flow', out))).not.toContain(SIGNING); + expect(allStrings(incoming)).not.toContain(SIGNING); + }); + + it('(b) an http node moved from the top level into a parallel branch: the secret is kept at rest and not served', () => { + const stored = storedMovesFlow(); + const served: any = redactMetadataItem('flow', stored); + const [begin, each, fan, call] = served.nodes; + const quiet = fan.config.branches[0]; + const incoming = { + ...served, + nodes: [begin, each, { ...fan, config: { ...fan.config, branches: [{ ...quiet, nodes: [...quiet.nodes, call] }, fan.config.branches[1]] } }], + }; + const out: any = carryForwardRedactedValues('flow', incoming, stored); + expect(out.nodes[2].config.branches[0].nodes[1].id).toBe('call'); + expect(out.nodes[2].config.branches[0].nodes[1].config.signingSecret).toBe(SIGNING); + expect(calloutOf(out, 'per_row').config.signingSecret).toBe(SIGNING); + expect(allStrings(redactMetadataItem('flow', out))).not.toContain(SIGNING); + }); + + it('(c) a node moved into a region AND changed in kind: the value is dropped — the position check still wins', () => { + const stored = storedMovesFlow(); + const served: any = redactMetadataItem('flow', stored); + const [begin, each, fan, call] = served.nodes; + const incoming = { + ...served, + nodes: [ + begin, + { ...each, config: { ...each.config, body: { nodes: [...each.config.body.nodes, { ...call, type: 'assignment', config: {} }], edges: [] } } }, + fan, + ], + }; + const out: any = carryForwardRedactedValues('flow', incoming, stored); + expect(calloutOf(out, 'call').type).toBe('assignment'); + expect(calloutOf(out, 'call').config.signingSecret).toBeUndefined(); + expect(calloutOf(out, 'per_row').config.signingSecret).toBe(SIGNING); + expect(allStrings(redactMetadataItem('flow', out))).not.toContain(SIGNING); + }); + + it('(d) an id duplicated across two regions: nothing is grafted onto either copy', () => { + const stored = storedMovesFlow(); + const served: any = redactMetadataItem('flow', stored); + const [begin, each, fan, call] = served.nodes; + const perRow = each.config.body.nodes[0]; + const [quiet, loud] = fan.config.branches; + const incoming = { + ...served, + nodes: [ + begin, + { ...each, config: { ...each.config, body: { nodes: [NOOP('body_noop')], edges: [] } } }, + { ...fan, config: { ...fan.config, branches: [{ ...quiet, nodes: [...quiet.nodes, perRow] }, { ...loud, nodes: [...loud.nodes, structuredClone(perRow)] }] } }, + call, + ], + }; + const out: any = carryForwardRedactedValues('flow', incoming, stored); + const copies = out.nodes[2].config.branches.flatMap((b: any) => b.nodes.filter((n: any) => n.id === 'per_row')); + expect(copies).toHaveLength(2); + expect(copies.every((n: any) => n.config.signingSecret === undefined)).toBe(true); + // The unambiguous ones are unaffected. + expect(calloutOf(out, 'call').config.signingSecret).toBe(SIGNING); + }); + + it('(a) through the save door: the row at rest keeps the moved node\'s secret, and no served read carries it', async () => { + const { engine, rows } = makeStubEngine(); + const where = { type: 'flow', name: 'moves', organization_id: null, package_id: null, state: 'active' }; + const body = storedMovesFlow(); + rows.set(keyOf(where), { id: 'r_moves', ...where, metadata: JSON.stringify(body), checksum: hashSpec(body), version: 1 } as Row); + const protocol = new ObjectStackProtocolImplementation(engine); + + const served: any = (await protocol.getMetaItem({ type: 'flow', name: 'moves' })).item; + expect(allStrings(served)).not.toContain(SIGNING); + const { _diagnostics: _d, ...editable } = served; + void _d; + await protocol.saveMetaItem({ type: 'flow', name: 'moves', item: movedOutOfLoop(editable) }); + + const at = JSON.parse(Array.from(rows.values()).find((r) => r.name === 'moves' && r.state === 'active')!.metadata); + expect(at.nodes.map((n: any) => n.id)).toEqual(['begin', 'each', 'fan', 'call', 'per_row']); + expect(at.nodes[4].config.signingSecret).toBe(SIGNING); + expect(calloutOf(at, 'body_noop').config).toEqual({}); + expect(allStrings(await protocol.getMetaItem({ type: 'flow', name: 'moves' }))).not.toContain(SIGNING); + expect(allStrings(await protocol.getMetaItems({ type: 'flow' }))).not.toContain(SIGNING); + }); +}); + +// --------------------------------------------------------------------------- +// #20590 patch round 2 — only a NODE can stand where the moved node stood +// --------------------------------------------------------------------------- +// +// The by-id lookup must count the elements that stand where the owner stood — +// the elements of an array under the same key as the owner's container — and +// nothing else. A flow keeps node ids and edge ids in separate spaces, so an +// edge may share a moved node's id and still be a valid flow; counted as a +// second match, it blocked the relocation and dropped the credential at save. + +describe('#20590 round 2 — the relocation counts nodes, not every object carrying the id', () => { + beforeEach(() => registerMetadataTypeRedactor('flow', regionFlowStandIn)); + afterEach(() => registerMetadataTypeRedactor('flow', flowStandInRedactor)); + + /** {@link movedOutOfLoop} plus a top-level EDGE whose id is the moved node's. */ + function movedWithEdgeTwin(served: any) { + const moved: any = movedOutOfLoop(served); + return { ...moved, edges: [...moved.edges, { id: 'per_row', source: 'begin', target: 'per_row' }] }; + } + + it('(e) a moved node plus an edge sharing its id: the credential is kept', () => { + const stored = storedMovesFlow(); + const incoming = movedWithEdgeTwin(redactMetadataItem('flow', stored)); + const out: any = carryForwardRedactedValues('flow', incoming, stored); + expect(out.edges.map((e: any) => e.id)).toEqual(['per_row']); + expect(out.nodes[4].id).toBe('per_row'); + expect(out.nodes[4].config.signingSecret).toBe(SIGNING); + expect(allStrings(redactMetadataItem('flow', out))).not.toContain(SIGNING); + }); + + it('(e) through the save door: the edge twin is a valid flow, and the row at rest keeps the moved node\'s secret', async () => { + const { engine, rows } = makeStubEngine(); + const where = { type: 'flow', name: 'moves', organization_id: null, package_id: null, state: 'active' }; + const body = storedMovesFlow(); + rows.set(keyOf(where), { id: 'r_moves', ...where, metadata: JSON.stringify(body), checksum: hashSpec(body), version: 1 } as Row); + const protocol = new ObjectStackProtocolImplementation(engine); + + const served: any = (await protocol.getMetaItem({ type: 'flow', name: 'moves' })).item; + const { _diagnostics: _d, ...editable } = served; + void _d; + await protocol.saveMetaItem({ type: 'flow', name: 'moves', item: movedWithEdgeTwin(editable) }); + + const at = JSON.parse(Array.from(rows.values()).find((r) => r.name === 'moves' && r.state === 'active')!.metadata); + expect(at.edges.map((e: any) => e.id)).toEqual(['per_row']); + expect(at.nodes[4].config.signingSecret).toBe(SIGNING); + expect(allStrings(await protocol.getMetaItem({ type: 'flow', name: 'moves' }))).not.toContain(SIGNING); + }); + + it('(f) two NODES sharing the moved node\'s id in different regions: nothing is grafted', () => { + const stored = storedMovesFlow(); + const served: any = redactMetadataItem('flow', stored); + const [begin, each, fan, call] = served.nodes; + const [quiet, loud] = fan.config.branches; + const incoming = { + ...served, + nodes: [ + begin, + { ...each, config: { ...each.config, body: { nodes: [...each.config.body.nodes, call], edges: [] } } }, + { ...fan, config: { ...fan.config, branches: [{ ...quiet, nodes: [...quiet.nodes, { ...NOOP('call') }] }, loud] } }, + ], + }; + const out: any = carryForwardRedactedValues('flow', incoming, stored); + const moved = out.nodes[1].config.body.nodes.find((n: any) => n.id === 'call'); + expect(moved.type).toBe('http'); + expect(moved.config.signingSecret).toBeUndefined(); + expect(out.nodes[2].config.branches[0].nodes.find((n: any) => n.id === 'call').config).toEqual({}); + // The unambiguous ones are unaffected. + expect(calloutOf(out, 'per_row').config.signingSecret).toBe(SIGNING); + }); +}); diff --git a/packages/runtime/src/domains/automation-flow-credential-projection.test.ts b/packages/runtime/src/domains/automation-flow-credential-projection.test.ts index 18ef6668a3f..ae3086b2c88 100644 --- a/packages/runtime/src/domains/automation-flow-credential-projection.test.ts +++ b/packages/runtime/src/domains/automation-flow-credential-projection.test.ts @@ -164,3 +164,29 @@ describe('#20552 — anti-vacuity: the door serves exactly what the registry ent expect(JSON.stringify(result.response?.body)).toContain(SECRET); }); }); + +describe('#20590 — a relocating PUT never lands the secret where the next read serves it', () => { + it('the start node’s kind changed and a new start node added: the member’s next read carries no credential', async () => { + const { dispatcher, spies } = makeDispatcher(); + const served = dataOf(await dispatcher.handleAutomation('/inbound_hook', 'GET', undefined, MEMBER)); + const [finish, begin] = served.nodes; + const relocated = { + ...served, + nodes: [ + finish, + { ...begin, type: 'assignment', label: 'Was the start node', config: {} }, + { id: 'begin_v2', type: 'start', label: 'On Webhook', config: { triggerType: 'api', hookId: 'intake' } }, + ], + edges: [{ id: 'e1', source: 'begin_v2', target: 'finish' }], + }; + + const put = await dispatcher.handleAutomation('/inbound_hook', 'PUT', relocated, AUTHOR); + expect(put.response?.status).toBe(200); + const next = await dispatcher.handleAutomation('/inbound_hook', 'GET', undefined, MEMBER); + expect(next.response?.status).toBe(200); + expect(dataOf(next).nodes.map((n: any) => n.id)).toEqual(['finish', 'begin', 'begin_v2']); + expect(JSON.stringify(next.response?.body)).not.toContain(SECRET); + // …and nothing was grafted into what the engine was handed. + expect(JSON.stringify(spies.registerFlow.mock.calls.at(-1)![1])).not.toContain(SECRET); + }); +}); diff --git a/packages/runtime/src/domains/meta-list-protocol-fault.test.ts b/packages/runtime/src/domains/meta-list-protocol-fault.test.ts new file mode 100644 index 00000000000..4c64485d587 --- /dev/null +++ b/packages/runtime/src/domains/meta-list-protocol-fault.test.ts @@ -0,0 +1,143 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #20590 position 2 — the dispatcher's `/meta/:type` list answers a protocol + * FAULT as that fault. It never falls through to the metadata service's raw + * list, which applies no per-type read-path redaction. + * + * ## What the protocol's throw means, measured + * + * `ObjectStackProtocolImplementation.getMetaItems` (the one implementation in + * this repository) answers a type it holds nothing for with `{ items: [] }`: + * it merges the metadata service's runtime-registered items (agents, tools) + * into its own answer, so no type is "unknown" to it in a way it signals by + * throwing. What it throws is a fault or a refusal: the store read failed + * (`503 SERVICE_UNAVAILABLE`, or a metadata app's marked refusal), a + * registered redactor threw (fail-closed by design, `redactMetadataItem`), or + * the segment is an unrecognised spelling of a declared type + * (`400 INVALID_REQUEST`). The branch used to swallow all of them and serve + * `metadataService.list(type)` instead — the stored bodies, so a flow's hook + * secret and a datasource's password went out to a member-level caller on + * any protocol fault. `RestServer`'s list route has no such fallback and + * answers the same throw as itself. + * + * The fallback itself stays, for the host shape it exists for: a protocol + * slot with no list verb at all. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { HttpDispatcher } from '../http-dispatcher.js'; + +const HOOK_SECRET = 'stored-hook-secret-20590'; +const DB_PASSWORD = 'stored-db-password-20590'; + +const STORED: Record = { + flow: [{ + name: 'inbound_hook', + label: 'Inbound hook', + type: 'api', + nodes: [{ id: 'begin', type: 'start', label: 'Start', config: { triggerType: 'api', secret: HOOK_SECRET } }], + edges: [], + }], + datasource: [{ name: 'warehouse', label: 'Warehouse', driver: 'postgres', config: { host: 'db', password: DB_PASSWORD } }], + agent: [{ name: 'triage_agent', label: 'Triage agent' }], +}; + +const clone = (v: T): T => JSON.parse(JSON.stringify(v)); +const singular = (type: unknown): string => String(type ?? '').replace(/s$/, ''); + +/** A member-level caller: authenticated, and nothing else. */ +const MEMBER = { userId: 'u_member', isSystem: false, systemPermissions: [] as string[] }; + +/** The store fault the protocol raises when its `sys_metadata` read fails (`metadataStoreUnavailableError`). */ +function storeFault(): Error { + return Object.assign(new Error('The metadata store could not be read, so whether this item exists is unknown.'), { + code: 'SERVICE_UNAVAILABLE', + status: 503, + }); +} + +function boot(protocol: Record) { + const metadata = { list: vi.fn(async (type: string) => clone(STORED[singular(type)] ?? [])) }; + const services: Record = { + protocol, + metadata, + security: { resolvePermissionSetNames: async () => [], getMetadataReadableFields: async () => [] }, + }; + const get = (n: string) => services[n] ?? null; + const kernel: any = { context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) }; + const dispatcher = new HttpDispatcher(kernel); + (dispatcher as any).timedResolveExecutionContext = async () => clone(MEMBER); + const list = async (type: string) => { + const res = await dispatcher.dispatch('GET', `/meta/${type}`, undefined, {}, { request: { headers: {} } } as any); + return { status: res.response?.status ?? 0, body: res.response?.body }; + }; + return { list, metadata }; +} + +const text = (v: unknown) => JSON.stringify(v ?? null); + +describe('#20590 — a protocol fault on the list read is answered as itself', () => { + for (const [type, credential] of [['flow', HOOK_SECRET], ['datasource', DB_PASSWORD]] as const) { + it(`${type}: a store fault answers 503 SERVICE_UNAVAILABLE and never the stored bodies`, async () => { + const protocol = { + getMetaTypes: vi.fn(async () => ({ types: Object.keys(STORED) })), + getMetaItems: vi.fn(async () => { throw storeFault(); }), + }; + const { list, metadata } = boot(protocol); + const res = await list(type); + expect(text(res.body)).not.toContain(credential); + expect({ status: res.status, code: res.body?.error?.code }).toEqual({ status: 503, code: 'SERVICE_UNAVAILABLE' }); + expect(metadata.list).not.toHaveBeenCalled(); + }); + } + + it('a protocol refusal keeps its own status and code (400 INVALID_REQUEST)', async () => { + const protocol = { + getMetaTypes: vi.fn(async () => ({ types: Object.keys(STORED) })), + getMetaItems: vi.fn(async () => { + throw Object.assign(new Error('not a recognised spelling of a declared metadata type'), { code: 'INVALID_REQUEST', status: 400 }); + }), + }; + const { list, metadata } = boot(protocol); + const res = await list('flow'); + expect({ status: res.status, code: res.body?.error?.code }).toEqual({ status: 400, code: 'INVALID_REQUEST' }); + expect(text(res.body)).not.toContain(HOOK_SECRET); + expect(metadata.list).not.toHaveBeenCalled(); + }); + + it('an undeclared throw (a redactor failing closed) is a 500, never the unredacted list', async () => { + const protocol = { + getMetaTypes: vi.fn(async () => ({ types: Object.keys(STORED) })), + getMetaItems: vi.fn(async () => { throw new Error('redactor for flow threw'); }), + }; + const { list, metadata } = boot(protocol); + const res = await list('flow'); + expect(res.status).toBe(500); + expect(text(res.body)).not.toContain(HOOK_SECRET); + expect(metadata.list).not.toHaveBeenCalled(); + }); +}); + +describe('#20590 — what still reaches the metadata service fallback', () => { + it('a type the protocol holds nothing for is answered with its empty list — the protocol’s own "unknown" answer', async () => { + const protocol = { + getMetaTypes: vi.fn(async () => ({ types: Object.keys(STORED) })), + getMetaItems: vi.fn(async () => ({ items: [] })), + }; + const { list, metadata } = boot(protocol); + const res = await list('agent'); + expect(res.status).toBe(200); + expect(res.body?.data?.items ?? res.body?.data).toEqual([]); + expect(metadata.list).not.toHaveBeenCalled(); + }); + + it('a protocol slot with no list verb still lists the metadata service’s runtime-registered items', async () => { + const protocol = { getMetaTypes: vi.fn(async () => ({ types: Object.keys(STORED) })) }; + const { list, metadata } = boot(protocol); + const res = await list('agent'); + expect(res.status).toBe(200); + expect(metadata.list).toHaveBeenCalledWith('agent'); + expect(text(res.body)).toContain('triage_agent'); + }); +}); diff --git a/packages/runtime/src/domains/meta-list-read-gate-parity.test.ts b/packages/runtime/src/domains/meta-list-read-gate-parity.test.ts index 97923bcbf44..01c0346d040 100644 --- a/packages/runtime/src/domains/meta-list-read-gate-parity.test.ts +++ b/packages/runtime/src/domains/meta-list-read-gate-parity.test.ts @@ -139,16 +139,21 @@ const CALLERS: Record<'holder' | 'non-holder' | 'anonymous', Caller> = { type CallerName = keyof typeof CALLERS; /** - * One protocol double, the same shape both transports read. `unknownTypes` - * makes `getMetaItems` throw for those types — the "protocol doesn't know this - * type" answer that sends the dispatcher on to its fallback stores — while it - * still answers every other type, the books the doc audience reads included. + * One protocol double, the same shape both transports read. `unansweredTypes` + * makes `getMetaItems` answer NO list for those types — the one protocol + * answer that sends the dispatcher on to its fallback stores — while it still + * answers every other type, the books the doc audience reads included. + * + * [#20590] Not a throw. A protocol throw is a fault and is answered as itself + * (`meta-list-protocol-fault.test.ts`); it used to be read as "the protocol + * does not know this type", which the one real protocol never signals that + * way — it answers such a type with an empty list. */ -function protocolDouble(unknownTypes: string[] = []) { +function protocolDouble(unansweredTypes: string[] = []) { return { getMetaTypes: vi.fn(async () => ({ types: Object.keys(STORE) })), getMetaItems: vi.fn(async ({ type }: any) => { - if (unknownTypes.includes(singular(type))) throw new Error(`unknown metadata type '${type}'`); + if (unansweredTypes.includes(singular(type))) return undefined; return clone(STORE[singular(type)] ?? []); }), }; diff --git a/packages/runtime/src/domains/meta.ts b/packages/runtime/src/domains/meta.ts index b4ddb362cc2..0857fe07d18 100644 --- a/packages/runtime/src/domains/meta.ts +++ b/packages/runtime/src/domains/meta.ts @@ -1874,8 +1874,20 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin const data = await protocol.getMetaItems({ type: typeOrName, packageId, organizationId, previewDrafts }); // Return any valid response from protocol (including empty items arrays) if (data && (data.items !== undefined || Array.isArray(data))) listed = data; - } catch { - // Protocol doesn't know this type, fall through + } catch (e: any) { + // [#20590] A throw here is a FAULT, answered as itself — never a + // cue to serve the metadata service's list below, which holds + // the stored bodies and applies no per-type read-path redaction + // (a flow's hook secret, a datasource's password). The protocol + // answers a type it holds nothing for with an empty list — it + // merges the metadata service's runtime-registered items (agents, + // tools) into its own answer — so it never signals "unknown + // type" by throwing. What it throws is a failed store read + // (503), a metadata app's marked refusal, a redactor failing + // closed, or a refused spelling (400). `RestServer`'s list route + // answers the same throw the same way ("prefer failing to + // falling back", AGENTS.md). + return { handled: true, response: deps.errorFromThrown(e, 500) }; } } // [ADR-0106 D5(2)] The dispatcher's list read is the same outlet as @@ -1883,6 +1895,8 @@ export async function handleMetadataRequest(deps: DomainHandlerDeps, path: strin if (listed !== undefined) return answerList(listed); // Try MetadataService directly for runtime-registered metadata (agents, tools, etc.) + // — reached only by a host whose protocol slot has no list verb, or + // whose protocol answered no list at all; never on a protocol fault. const metadataService = await deps.getService(_context, CoreServiceName.enum.metadata); if (metadataService && typeof (metadataService as any).list === 'function') { let items: any; diff --git a/packages/services/service-automation/src/flow-credential-positions.test.ts b/packages/services/service-automation/src/flow-credential-positions.test.ts new file mode 100644 index 00000000000..03733905e2f --- /dev/null +++ b/packages/services/service-automation/src/flow-credential-positions.test.ts @@ -0,0 +1,283 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #20590 — every credential position a flow definition can hold is withheld by + * the registered `flow` redactor, at every depth a node can sit. + * + * #20552 withheld one position: the start node's inbound-hook secret. This + * file is the family's enumeration pin. It does not list the positions by + * hand. It READS them from the node config contracts the platform declares — + * every builtin executor's descriptor `configSchema` (the vocabulary + * `registerFlow` checks a node's `config` against), the schemaless builtins' + * spec Zod contracts, and the approval node's — and asks the one projection to + * withhold each credential-named key it finds. So a node kind that declares a + * new credential key turns this file red until the projection covers it, or + * until the key is reviewed into {@link NOT_A_CREDENTIAL} with a reason. + * + * "Every depth": an ADR-0031 container (`loop`, `parallel`, `try_catch`) holds + * whole sub-graphs inside its own `config` (`FLOW_REGION_SLOTS`), so an `http` + * callout inside a loop body is a node whose definition is served with the + * flow like any other. A projection that walked only the top-level `nodes` + * would serve every nested one. + */ + +import { describe, it, expect } from 'vitest'; +import { + FLOW_REGION_SLOTS_BY_TYPE, + getApprovalNodeConfigJsonSchema, + getSchemalessNodeConfigJsonSchemas, +} from '@objectstack/spec/automation'; +import { AutomationEngine } from './engine.js'; +import { installBuiltinNodes } from './builtin/index.js'; +import { + FLOW_CREDENTIAL_CLEARED, + FLOW_NODE_CREDENTIAL_KEYS, + redactFlowCredentials, +} from './flow-credential-projection.js'; + +function silentLogger(): any { + return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } }; +} + +/** The node config contracts the platform declares, keyed by `node.type`. */ +function declaredNodeConfigSchemas(): Map { + const engine = new AutomationEngine(silentLogger()); + installBuiltinNodes(engine, { logger: silentLogger(), getService() { throw new Error('none'); } } as any); + const out = new Map(); + for (const descriptor of engine.getActionDescriptors()) { + if (descriptor.configSchema) out.set(descriptor.type, descriptor.configSchema); + } + for (const [type, schema] of Object.entries(getSchemalessNodeConfigJsonSchemas())) { + if (!out.has(type)) out.set(type, schema); + } + out.set('approval', getApprovalNodeConfigJsonSchema()); + return out; +} + +function isRecord(value: unknown): value is Record { + return !!value && typeof value === 'object' && !Array.isArray(value); +} + +/** + * Every declared property path under one node type's `config`, as dotted + * segments (`*` for an array item or an open map's value). A container's + * region slots are not descended into: what they hold is nodes, each with its + * own type and its own contract, which this enumeration already reads. + */ +function declaredConfigPaths(nodeType: string, root: unknown): string[][] { + const regionKeys = new Set((FLOW_REGION_SLOTS_BY_TYPE.get(nodeType) ?? []).map((slot) => slot.key)); + const defs = isRecord(root) && isRecord(root.$defs) ? root.$defs : {}; + const out: string[][] = []; + const seen = new Set(); + const walk = (schema: unknown, prefix: string[]): void => { + if (!isRecord(schema)) return; + if (typeof schema.$ref === 'string' && schema.$ref.startsWith('#/$defs/')) { + const target = defs[schema.$ref.slice('#/$defs/'.length)]; + if (seen.has(target)) return; + seen.add(target); + walk(target, prefix); + return; + } + for (const branch of ['anyOf', 'oneOf', 'allOf'] as const) { + const list = schema[branch]; + if (Array.isArray(list)) for (const sub of list) walk(sub, prefix); + } + if (isRecord(schema.properties)) { + for (const [key, sub] of Object.entries(schema.properties)) { + const path = [...prefix, key]; + out.push(path); + if (prefix.length === 0 && regionKeys.has(key)) continue; + walk(sub, path); + } + } + if (isRecord(schema.items)) walk(schema.items, [...prefix, '*']); + if (isRecord(schema.additionalProperties)) walk(schema.additionalProperties, [...prefix, '*']); + }; + walk(root, []); + return out; +} + +/** + * The credential-name vocabulary. Deliberately broad: under-redacting is the + * dangerous direction, so a key this matches must be covered by the + * projection or reviewed out below — a false positive costs one line with a + * reason, a false negative serves a secret. + */ +const CREDENTIAL_NAME = /secret|passw(?:or)?d|pwd|token|api[-_]?key|access[-_]?key|private[-_]?key|credential|bearer|signing/i; + +/** + * Declared keys the vocabulary matches that are NOT credentials, each with the + * reason it was reviewed out. Empty today; an entry needs its reason. + */ +const NOT_A_CREDENTIAL: ReadonlyMap = new Map([]); + +/** + * The positions no schema declares, with their authority. The start node's + * `config` is an open record (FlowNodeSchema), so its inbound-hook secret is + * declared by ADR-0041 and enforced by the engine's `validateApiTriggerSecret` + * rather than read off a contract. + */ +const SCHEMALESS_POSITIONS: ReadonlyArray = [['start', 'secret']]; + +function credentialPositions(): Array { + const found: Array = [...SCHEMALESS_POSITIONS]; + for (const [nodeType, schema] of declaredNodeConfigSchemas()) { + for (const path of declaredConfigPaths(nodeType, schema)) { + const key = path[path.length - 1]!; + if (key === '*' || !CREDENTIAL_NAME.test(key)) continue; + const id = `${nodeType}.${path.join('.')}`; + if (NOT_A_CREDENTIAL.has(id)) continue; + found.push([nodeType, path.join('.')] as const); + } + } + return found; +} + +const SENTINEL = 'credential-position-sentinel-20590'; + +/** A node of `nodeType` carrying the sentinel at `configKey`, with a unique id. */ +function nodeWith(id: string, nodeType: string, configKey: string): Record { + return { id, type: nodeType, label: id, config: { [configKey]: SENTINEL } }; +} + +/** The same credential-bearing node, placed at each depth a node can sit. */ +function placements(nodeType: string, configKey: string): Array<{ where: string; flow: Record }> { + const inner = () => nodeWith('inner', nodeType, configKey); + const tail = { id: 'tail', type: 'assignment', label: 'Tail', config: {} }; + const shell = (node: Record) => ({ + name: 'positions_20590', + label: 'Positions', + type: 'autolaunched', + nodes: [{ id: 'begin', type: 'start', label: 'Start', config: {} }, node], + edges: [], + }); + return [ + { where: 'top level', flow: shell(inner()) }, + { + where: 'a loop body', + flow: shell({ id: 'box', type: 'loop', label: 'Loop', config: { collection: '{items}', body: { nodes: [inner()], edges: [] } } }), + }, + { + where: 'a parallel branch', + flow: shell({ + id: 'box', type: 'parallel', label: 'Parallel', + config: { branches: [{ name: 'a', nodes: [tail] }, { name: 'b', nodes: [inner()] }] }, + }), + }, + { + where: 'a try region', + flow: shell({ id: 'box', type: 'try_catch', label: 'Try', config: { try: { nodes: [inner()], edges: [] } } }), + }, + { + where: 'a catch region', + flow: shell({ + id: 'box', type: 'try_catch', label: 'Try', + config: { try: { nodes: [tail], edges: [] }, catch: { nodes: [inner()], edges: [] } }, + }), + }, + ]; +} + +describe('#20590 — the enumeration: every declared credential position is withheld', () => { + const positions = credentialPositions(); + + it('reads a real population — the walk is not vacuous', () => { + // Anti-vacuity: the declared universe is large, and the `http` + // node's outbound HMAC secret is in it (HttpConfigSchema, + // `signingSecret`). A walk that stopped reading schemas would find + // only the hand-listed start position and pass everything below. + const universe = [...declaredNodeConfigSchemas()].flatMap(([t, s]) => declaredConfigPaths(t, s).map((p) => `${t}.${p.join('.')}`)); + expect(universe.length).toBeGreaterThan(40); + expect(positions.map(([t, k]) => `${t}.${k}`)).toContain('http.signingSecret'); + }); + + it('the projection covers exactly the declared positions — no fewer, and none that is no longer declared', () => { + const declared = positions.map(([t, k]) => `${t}.${k}`).sort(); + const covered = [...FLOW_NODE_CREDENTIAL_KEYS].flatMap(([t, keys]) => keys.map((k) => `${t}.${k}`)).sort(); + expect(covered).toEqual(declared); + }); + + for (const [nodeType, configKey] of credentialPositions()) { + for (const { where, flow } of placements(nodeType, configKey)) { + it(`withholds ${nodeType}.config.${configKey} at ${where}`, () => { + const before = structuredClone(flow); + const { item, redactedKeys } = redactFlowCredentials(flow); + expect(JSON.stringify(item)).not.toContain(SENTINEL); + expect(redactedKeys.length).toBeGreaterThan(0); + // Pure: the stored body the engine executes keeps it. + expect(flow).toEqual(before); + }); + } + } +}); + +describe('#20590 — what the projection answers, position by position', () => { + it('names each withheld path, through every region kind, by the index in the body it was handed', () => { + const flow = { + name: 'nested', + label: 'Nested', + nodes: [ + { id: 'begin', type: 'start', label: 'Start', config: { secret: 'hook' } }, + { + id: 'each', type: 'loop', label: 'Each', + config: { + collection: '{rows}', + body: { nodes: [{ id: 'call', type: 'http', label: 'Call', config: { url: 'https://x', signingSecret: 'sign-1' } }], edges: [] }, + }, + }, + { + id: 'fan', type: 'parallel', label: 'Fan', + config: { + branches: [ + { name: 'a', nodes: [{ id: 'noop', type: 'assignment', label: 'Noop', config: {} }] }, + { name: 'b', nodes: [{ id: 'push', type: 'http', label: 'Push', config: { url: 'https://y', signingSecret: 'sign-2' } }] }, + ], + }, + }, + ], + edges: [], + }; + const { item, redactedKeys } = redactFlowCredentials(flow); + expect(redactedKeys).toEqual([ + 'nodes.0.config.secret', + 'nodes.1.config.body.nodes.0.config.signingSecret', + 'nodes.2.config.branches.1.nodes.0.config.signingSecret', + ]); + const text = JSON.stringify(item); + for (const s of ['hook', 'sign-1', 'sign-2']) expect(text).not.toContain(`"${s}"`); + // Everything that is not a credential survives, and untouched subtrees are shared. + const nodes = item.nodes as any[]; + expect(nodes[1].config.body.nodes[0].config).toEqual({ url: 'https://x' }); + expect(nodes[2].config.branches[0]).toBe((flow.nodes[2] as any).config.branches[0]); + }); + + it('serves the cleared form as written — the empty string holds no credential', () => { + const flow = { + name: 'cleared', + label: 'Cleared', + nodes: [ + { id: 'begin', type: 'start', label: 'Start', config: { secret: FLOW_CREDENTIAL_CLEARED } }, + { id: 'call', type: 'http', label: 'Call', config: { url: 'https://x', signingSecret: FLOW_CREDENTIAL_CLEARED } }, + ], + edges: [], + }; + const result = redactFlowCredentials(flow); + expect(result.redactedKeys).toEqual([]); + expect(result.item).toBe(flow); + // A blank that is not empty is still a value somebody typed: withheld. + const spaced = { ...flow, nodes: [flow.nodes[0], { ...flow.nodes[1], config: { url: 'https://x', signingSecret: ' ' } }] }; + expect(redactFlowCredentials(spaced).redactedKeys).toEqual(['nodes.1.config.signingSecret']); + }); + + it('withholds nothing from a key of the same name on a kind that does not hold that credential', () => { + const flow = { + name: 'lookalike', + label: 'Lookalike', + nodes: [{ id: 'n', type: 'assignment', label: 'N', config: { signingSecret: 'not-a-position', secret: 'nor-this' } }], + edges: [], + }; + const result = redactFlowCredentials(flow); + expect(result.redactedKeys).toEqual([]); + expect(result.item).toBe(flow); + }); +}); diff --git a/packages/services/service-automation/src/flow-credential-projection.ts b/packages/services/service-automation/src/flow-credential-projection.ts index 339c0e8b4da..b616a937d89 100644 --- a/packages/services/service-automation/src/flow-credential-projection.ts +++ b/packages/services/service-automation/src/flow-credential-projection.ts @@ -2,22 +2,44 @@ /** * # The flow credential projection — the ONE definition of what a served flow - * withholds (#20552) - * - * ADR-0041's `trigger-api` acceptance criteria give an inbound hook exactly one - * credential: "a per-flow secret; HMAC signature verification". That secret is - * authored as a literal on the flow's START node, `config.secret` — the object - * `AutomationEngine.deriveTriggerBinding` hands `trigger-api`'s - * `start()` as `binding.config`, and the key the engine's - * `validateApiTriggerSecret` refuses a flow without. Whoever holds it can sign - * posts to the hook, and the documented inbound pattern runs `runAs: 'system'`. - * - * Until this module every definition read served it verbatim: the automation - * domain's `GET /:name`, and on the metadata plane the item read, the list - * read, the layered read, the draft preview, the package export. A + * withholds (#20552, #20590) + * + * A flow definition holds two kinds of credential, each a literal in one node + * kind's `config`: + * + * - **the inbound hook's secret** — the start node's `config.secret`. ADR-0041's + * `trigger-api` acceptance criteria give an inbound hook exactly one + * credential: "a per-flow secret; HMAC signature verification". It is the + * object `AutomationEngine.deriveTriggerBinding` hands `trigger-api`'s + * `start()` as `binding.config`, and the key the engine's + * `validateApiTriggerSecret` refuses a flow without. Whoever holds it can + * sign posts to the hook, and the documented inbound pattern runs + * `runAs: 'system'`. + * - **an outbound callout's signing secret** — the `http` node's + * `config.signingSecret` (`HttpConfigSchema`). The durable arm hands it to + * the messaging outbox, which signs every delivery with it; whoever holds it + * can forge a delivery the receiving endpoint accepts as this platform's. + * + * Until this module every definition read served both verbatim: the + * automation domain's `GET /:name`, and on the metadata plane the item read, + * the list read, the layered read, the draft preview, the package export. A * credential is never readable back, so the projection below removes it from * what is SERVED — and nothing else. * + * {@link FLOW_NODE_CREDENTIAL_KEYS} is the table of those positions, by node + * kind. `flow-credential-positions.test.ts` holds it to the node config + * contracts the platform declares: it reads every declared key, and a + * credential-named one this table does not cover turns it red. + * + * ## At every depth a node can sit + * + * An ADR-0031 container (`loop`, `parallel`, `try_catch`) holds whole + * sub-graphs inside its own `config` — the slots `FLOW_REGION_SLOTS` declares + * — and an `http` callout inside a loop body is served with the flow like any + * top-level node. So the walk follows those slots, through every region, at + * every depth. A start node inside a region is not a valid flow, and its + * secret is withheld there too: under-redacting is the dangerous direction. + * * ## Where it applies, and where it deliberately does not * * It is registered as the `flow` metadata-type redactor (the @@ -29,7 +51,7 @@ * One helper, applied where each surface's definition leaves the process. * * It is NOT applied to anything the engine EXECUTES. The flow map the engine - * arms triggers from keeps the stored secret, and so does the in-process + * arms triggers from keeps the stored secrets, and so does the in-process * `getFlow` (the clone door copies a whole definition through it, ADR-0126 * §7.1): redaction is a serving act, and a raw-record consumer keeps reading * the stored body (`spec/kernel/metadata-type-redaction.ts`). That is also why @@ -39,36 +61,51 @@ * * ## Dropped, not masked * - * The key is REMOVED, the datasource redactor's posture: a mask would be a - * non-blank string, `validateApiTriggerSecret` would accept it, and any write - * path that missed the carry-forward would store the mask as the literal HMAC - * secret — silently. An absent key is what every registration door already - * refuses loudly, so a round trip that misses the carry-forward fails at the - * door instead of arming a hook nobody can sign for. + * A withheld key is REMOVED, the datasource redactor's posture: a mask would be + * a non-blank string, `validateApiTriggerSecret` would accept it, the outbox + * would sign with it, and any write path that missed the carry-forward would + * store the mask as the literal secret — silently. An absent start-node secret + * is what every registration door already refuses loudly, so a round trip that + * misses the carry-forward fails at the door instead of arming a hook nobody + * can sign for. * - * ## The write-path inverse + * ## The write-path inverse, and the one door that removes a credential * * `@objectstack/metadata-protocol`'s `carryForwardRedactedValues` is the one - * inverse, on both planes: a body that carries the projected form — no - * `secret` on the start node, exactly what was served — keeps the stored - * secret; an explicit value replaces it. The `redactedKeys` below address the - * node by its index in the stored body, and the inverse resolves that index to - * the node's `id`, so an edit that reorders `nodes` still carries the secret - * back onto the start node it came from. + * inverse, on both planes: a body that carries the projected form — the key + * absent, exactly what was served — keeps the stored value; an explicit value + * replaces it. The `redactedKeys` below address each node by its index in the + * stored body, and the inverse resolves that index to the node's `id` (a + * flow's node ids are one space across every region), so an edit that + * reorders `nodes`, or a region's `nodes`, or a `parallel` block's branches, + * still carries each value back onto the node it came from. And it re-runs + * this projection over what it carried, so a value never lands on a node whose + * kind no longer holds that credential (#20590 position 3). + * + * Absent therefore means "unchanged", never "removed" — so `signingSecret`, + * which is optional, needs a door of its own: the empty string + * ({@link FLOW_CREDENTIAL_CLEARED}). It is what the contract already accepts + * (`z.string().optional()`), it holds no secret, and the messaging outbox + * signs only with a non-empty one, so a callout whose secret is cleared is + * delivered unsigned. It is not withheld: the cleared form is served as written, so a + * reader can tell "cleared" from "withheld", and a round trip of it keeps it + * cleared. The start node's secret takes the same value and meets the engine's + * non-blank refusal, which is the answer an `api` flow with no secret is owed. * * ## Why registered by this plugin, not built into `@objectstack/spec` * * The registry's own note prefers a built-in for a type whose rows exist * without the owning plugin, because the credential stays live there. This - * one does not: the secret is a credential only to an armed hook, and a hook - * is armed only through this engine — `trigger-api` registers against the - * `automation` service and has no other source of flows. The knowledge of - * which key is the credential lives here too (`validateApiTriggerSecret`), and - * `packages/spec` declares no start-node config shape to derive it from, so a - * spec-side copy would be this package's business rule restated in the - * contract package (Prime Directive #2). + * one does not: the hook secret is a credential only to an armed hook, a hook + * is armed only through this engine, and an `http` node signs only when this + * engine runs it. The knowledge of which key is the credential lives here too + * (`validateApiTriggerSecret`, the `http` executor), and `packages/spec` + * declares no start-node config shape to derive it from, so a spec-side copy + * would be this package's business rule restated in the contract package + * (Prime Directive #2). */ +import { FLOW_REGION_SLOTS_BY_TYPE } from '@objectstack/spec/automation'; import { registerMetadataTypeRedactor } from '@objectstack/spec/kernel'; import type { MetadataRedactionResult, MetadataTypeRedactor } from '@objectstack/spec/kernel'; @@ -78,17 +115,99 @@ export const FLOW_METADATA_TYPE = 'flow'; /** The start-node `config` key that holds the inbound hook's HMAC secret (ADR-0041). */ export const FLOW_HOOK_SECRET_KEY = 'secret'; +/** The `http`-node `config` key that holds the outbound delivery's HMAC signing secret (`HttpConfigSchema`). */ +export const HTTP_SIGNING_SECRET_KEY = 'signingSecret'; + +/** + * Every credential position a flow node holds, by `node.type` → `config` keys. + * + * A `Map`, not an object literal: `node.type` is author-controlled and an open + * namespace (ADR-0018), and an object lookup would resolve `'constructor'` + * through `Object`'s prototype — the reason `FLOW_REGION_SLOTS_BY_TYPE` is one. + */ +export const FLOW_NODE_CREDENTIAL_KEYS: ReadonlyMap = new Map([ + ['start', [FLOW_HOOK_SECRET_KEY]], + ['http', [HTTP_SIGNING_SECRET_KEY]], +]); + +/** + * The explicit clearing value: a credential key set to it holds no credential, + * so it is served as written rather than withheld — the one unambiguous way to + * remove an optional credential across a round trip whose absent key means + * "unchanged". + */ +export const FLOW_CREDENTIAL_CLEARED = ''; + function isPlainRecord(value: unknown): value is Record { return !!value && typeof value === 'object' && !Array.isArray(value); } +/** Project a list of nodes; `undefined` when nothing in it was withheld. */ +function projectNodes(nodes: readonly unknown[], path: string, redactedKeys: string[]): unknown[] | undefined { + let out: unknown[] | undefined; + nodes.forEach((node, index) => { + const projected = projectNode(node, `${path}.${index}`, redactedKeys); + if (projected === node) return; + out ??= nodes.slice(); + out[index] = projected; + }); + return out; +} + +/** One region (`{ nodes, edges }`), projected; the input by reference when nothing was withheld. */ +function projectRegion(region: unknown, path: string, redactedKeys: string[]): unknown { + if (!isPlainRecord(region) || !Array.isArray(region.nodes)) return region; + const nodes = projectNodes(region.nodes, `${path}.nodes`, redactedKeys); + return nodes ? { ...region, nodes } : region; +} + +/** One node: its own credential keys, then every region its kind holds. */ +function projectNode(node: unknown, path: string, redactedKeys: string[]): unknown { + if (!isPlainRecord(node) || typeof node.type !== 'string') return node; + const config = node.config; + if (!isPlainRecord(config)) return node; + + let next: Record | undefined; + for (const key of FLOW_NODE_CREDENTIAL_KEYS.get(node.type) ?? []) { + if (!Object.prototype.hasOwnProperty.call(config, key)) continue; + if (config[key] === FLOW_CREDENTIAL_CLEARED) continue; + next ??= { ...config }; + delete next[key]; + redactedKeys.push(`${path}.config.${key}`); + } + + for (const slot of FLOW_REGION_SLOTS_BY_TYPE.get(node.type) ?? []) { + const value = config[slot.key]; + const slotPath = `${path}.config.${slot.key}`; + let projected: unknown = value; + if (slot.arity === 'one') { + projected = projectRegion(value, slotPath, redactedKeys); + } else if (Array.isArray(value)) { + let regions: unknown[] | undefined; + value.forEach((region, index) => { + const p = projectRegion(region, `${slotPath}.${index}`, redactedKeys); + if (p === region) return; + regions ??= value.slice(); + regions[index] = p; + }); + projected = regions ?? value; + } + if (projected === value) continue; + next ??= { ...config }; + next[slot.key] = projected; + } + + return next ? { ...node, config: next } : node; +} + /** - * Project the inbound-hook secret out of a flow definition. + * Project every credential in {@link FLOW_NODE_CREDENTIAL_KEYS} out of a flow + * definition, at every depth. * - * Every `start` node's `config.secret` is removed — not only the first start - * node's, and not only on a flow whose trigger resolves to `api`. The engine - * reads the first start node of an `api` flow, but a secret authored anywhere - * else is still a secret, and under-redacting is the dangerous direction. + * Every node of a listed kind is projected — not only the first start node, + * and not only on a flow whose trigger resolves to `api`. The engine reads the + * first start node of an `api` flow, but a secret authored anywhere else is + * still a secret, and under-redacting is the dangerous direction. * * Pure: the input is never mutated, and when there is nothing to withhold the * input is returned by reference with `redactedKeys: []`. @@ -98,19 +217,8 @@ export const redactFlowCredentials: MetadataTypeRedactor = (item): MetadataRedac if (!Array.isArray(nodes)) return { item, redactedKeys: [] }; const redactedKeys: string[] = []; - const projected = nodes.map((node, index) => { - if (!isPlainRecord(node) || node.type !== 'start') return node; - const config = node.config; - if (!isPlainRecord(config) || !Object.prototype.hasOwnProperty.call(config, FLOW_HOOK_SECRET_KEY)) { - return node; - } - const { [FLOW_HOOK_SECRET_KEY]: _withheld, ...rest } = config; - void _withheld; - redactedKeys.push(`nodes.${index}.config.${FLOW_HOOK_SECRET_KEY}`); - return { ...node, config: rest }; - }); - - if (redactedKeys.length === 0) return { item, redactedKeys: [] }; + const projected = projectNodes(nodes, 'nodes', redactedKeys); + if (!projected) return { item, redactedKeys: [] }; return { item: { ...item, nodes: projected }, redactedKeys: redactedKeys.sort() }; };