fix(service-automation,metadata-protocol,runtime): withhold the remaining stored flow-credential positions at every depth, and answer a /meta list fault as itself (#20590)

.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.

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);
+    });
+});

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<string, any[]> = {
+    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 = <T>(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<string, unknown>) {
+    const metadata = { list: vi.fn(async (type: string) => clone(STORED[singular(type)] ?? [])) };
+    const services: Record<string, unknown> = {
+        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');
+    });
+});

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)] ?? []);
         }),
     };

packages/runtime/src/domains/meta.ts

@@ -1874,15 +1874,29 @@ 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
         // REST's `GET /meta/object`, reached by a different door.
         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;