fix(lint): the flow-template rules read an http node's request payload

.changeset/lint-flow-template-rules-reach-http-payload.md

@@ -0,0 +1,15 @@
+---
+"@objectstack/lint": patch
+---
+
+`flow-double-brace-interpolation` and `flow-bare-dollar-reference` now read an `http` node's request payload. Both rules were blind to the whole of `config.body` on every node type — the one key where an uninterpolated token has an outbound consequence.
+
+The recursive template scan in `lint-flow-patterns.ts` read a region-stripped view of each node's config, and it built that view from the FLAT UNION of every config key that holds a region on *any* node type (`body`, `try`, `catch`, `branches`) rather than from the slots the node in hand actually owns. `body` is `loop`'s region slot **and** the canonical request-payload key on an `http` node, so `config.body` was deleted from every node's view before the scan ever read it.
+
+That made the two rules silent exactly where they matter most: `http-nodes.ts` interpolates the raw config wholesale, so a double-brace `{{ record.title }}` or a bare `$source.id` written in a payload is never interpolated and ships to the endpoint as literal text. Measured before this change, an `http` node whose `body` carried either token shape — at the top level or nested inside a `try_catch` region — produced zero findings from either rule.
+
+- **The call site passes its own slots.** `stripRegions(node.config, ownRegionKeys(node.type))`. The remedy was already written in `stripRegions`' own docblock ("Pass the OWNING node's slots, not the flat union") and the sibling call site in `flow-walk.ts` already followed it; this one did not.
+- **The trapping default is gone.** `stripRegions`' `regionKeys` parameter is now REQUIRED. The flat union survived as a default only to bound an earlier change, and the cost of leaving it was this defect: the shorter call compiled and quietly asked a different question. A caller that has not decided which set it means now fails to compile instead.
+- **The double-count direction is unchanged and pinned.** A token inside a `loop` body is still reported exactly ONCE, against the node that carries it and not also against the container — the reason the strip exists, and the direction that breaks if a repair over-corrects to stripping nothing.
+
+Both rules keep their existing severity. New findings appear only where a `{{ }}` or bare `$ref.field` sits in a previously-hidden key; measured across `examples/app-showcase`, `app-crm` and `app-todo` (34 flows, `http` payloads inside a `parallel` branch and a `try_catch` try among them), the count is unchanged at zero — those payloads use correct single-brace tokens.

packages/lint/src/flow-walk.test.ts

@@ -194,11 +194,38 @@ describe('walkFlowNodes', () => {
       expect(ownRegionKeys('constructor')).toEqual([]);
     });
 
-    it('treats an empty key list as a real answer, distinct from omitting it', () => {
+    it('treats an empty key list as a real answer: strip nothing, same reference', () => {
       const config = { body: 'payload', try: 'kept' };
       expect(stripRegions(config, [])).toBe(config);
-      // Omitted: the flat-union view its remaining caller was written against.
-      expect(Object.keys(stripRegions(config) ?? {})).toEqual([]);
+    });
+
+    /**
+     * #16405 — `regionKeys` is REQUIRED, and this is the pin that keeps it so.
+     *
+     * It carried the flat union as a DEFAULT until #16405, which meant the
+     * shorter call compiled and answered a different question than the caller
+     * was asking: "every key that holds a region on SOME node type" rather than
+     * "this node's own slots". `lint-flow-patterns.ts` wrote that shorter call
+     * and was silently blind to an `http` node's `body` — its request payload —
+     * for as long as it existed. With no default, the omission does not compile.
+     *
+     * A `@ts-expect-error` rather than a runtime assertion because the trap was
+     * only ever visible to the type checker; `tsconfig.test.json` compiles this
+     * file, so the directive is evaluated (`pnpm --filter @objectstack/lint
+     * check:test-typecheck`) and re-adding a default makes it unused — TS2578.
+     */
+    it('does not compile when the key list is omitted', () => {
+      const config = { body: 'payload', try: 'kept' };
+      // @ts-expect-error — `regionKeys` is required: the flat-union default is gone.
+      expect(stripRegions(config)).toBeDefined();
+    });
+
+    it('keeps a non-region key a node type owns as ordinary config', () => {
+      // `body` on an `http` node is its request payload, not a region.
+      const config = { url: 'https://x.test', body: { text: 'hi' } };
+      expect(stripRegions(config, ownRegionKeys('http'))).toBe(config);
+      // …and is still stripped on the type that owns it as a region.
+      expect(Object.keys(stripRegions(config, ownRegionKeys('loop')) ?? {})).toEqual(['url']);
     });
 
     it('returns undefined for a non-record config, whatever the key list', () => {

packages/lint/src/flow-walk.ts

@@ -151,17 +151,18 @@ export function ownRegionKeys(nodeType: unknown): readonly string[] {
  * is the reverse of the double-count this view exists to prevent and strictly
  * worse: a double-count is visible in the output.
  *
- * The union stays the DEFAULT to bound this change to the two rules #16111
- * names — ⛔ NOT because it is the right argument for the caller still taking
- * it. `lint-flow-patterns.ts` reads the union view for its own recursive
- * template scan, so it is blind to an `http` node's `body` for exactly the
- * reason above: the same defect, one call site over, tracked on #16405. Once
- * that caller passes its own slots this default has no callers left and
- * `regionKeys` must become REQUIRED, so no later caller inherits the trap by
- * writing the shorter call.
+ * `regionKeys` is REQUIRED — there is deliberately no default (#16405). The
+ * union was the default until #16111's remaining caller was fixed, to bound
+ * that change to the two rules it named, and the cost of leaving it was exactly
+ * what this parameter documents: `lint-flow-patterns.ts` inherited the trap by
+ * writing the shorter call and was blind to an `http` node's `body` for the
+ * reason above, silently, for as long as the default existed. With the
+ * parameter required, the next caller that has not decided which question it is
+ * asking does not compile instead of quietly asking the wrong one.
  *
  * `regionKeys: []` is a real answer (strip nothing) and is distinct from
- * omitting the parameter.
+ * {@link ownRegionKeys}' answer for a non-container type, which happens to be
+ * the same empty list arrived at by asking.
  *
  * Exported since #5383 because {@link WalkedFlowNode.localConfig} is not the only
  * consumer that needs this view. `lint-flow-patterns.ts` walks graphs rather than
@@ -171,7 +172,7 @@ export function ownRegionKeys(nodeType: unknown): readonly string[] {
  */
 export function stripRegions(
   config: unknown,
-  regionKeys: Iterable<string> = REGION_CONFIG_KEYS,
+  regionKeys: Iterable<string>,
 ): AnyRec | undefined {
   if (!isRec(config)) return undefined;
   const strip = regionKeys instanceof Set ? regionKeys : new Set(regionKeys);

packages/lint/src/lint-flow-patterns.test.ts

@@ -1,7 +1,7 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
 import { describe, it, expect } from 'vitest';
-import { TimeRelativeTriggerSchema, LoopConfigSchema, ParallelConfigSchema, TryCatchConfigSchema, FlowSchema } from '@objectstack/spec/automation';
+import { TimeRelativeTriggerSchema, LoopConfigSchema, ParallelConfigSchema, TryCatchConfigSchema, HttpConfigSchema, FlowSchema } from '@objectstack/spec/automation';
 // [#5659] The shared identity reduction, asserted beside the rule that consumes
 // it — the rule's verdict and the drivers' verdict are one object now.
 import { reduceFilterVerdict } from '@objectstack/spec/data';
@@ -2273,3 +2273,184 @@ describe('per-iteration containment (#13681 / #14394)', () => {
     });
   });
 });
+
+/**
+ * #16405 — the two #1315 template rules reach an `http` node's REQUEST PAYLOAD.
+ *
+ * The recursive template scan read a region-stripped view of each node's config
+ * built from the FLAT UNION of every region key on ANY node type (`body`,
+ * `try`, `catch`, `branches`), rather than from the slots the node in hand
+ * actually owns. `body` is `loop`'s region slot AND the canonical request-payload
+ * key on an `http` node (`HttpConfigSchema.body`), so `config.body` was deleted
+ * from EVERY node's view before the scan read it — and the payload is the one
+ * place an uninterpolated token reaches a real outbound request, because
+ * `http-nodes.ts` interpolates the raw config wholesale.
+ *
+ * Measured on the parent commit, both directions: every case in this block
+ * returned ZERO findings for its rule before the call site passed
+ * `ownRegionKeys(node.type)`.
+ *
+ * The over-correction direction is pinned too, and it is the one that breaks if
+ * a repair strips NOTHING: the last case here keeps a payload token inside a
+ * `loop` body reported exactly ONCE, on the node carrying it.
+ */
+
+/** An `http` push node's config: a real URL, a real method, and the payload under test. */
+const httpPushConfig = (body: unknown) => ({
+  url: 'https://api.example.com/v1/incidents',
+  method: 'POST',
+  body,
+});
+
+/** A scheduled flow whose one `http` node carries the payload under test. */
+function httpFlow(body: unknown) {
+  return {
+    flows: [{
+      name: 'incident_push',
+      runAs: 'system',
+      nodes: [
+        { id: 'start', type: 'start', config: { triggerType: 'schedule', schedule: 'cron:0 3 * * *' } },
+        { id: 'push', type: 'http', label: 'POST incident', config: httpPushConfig(body) },
+      ],
+      edges: [{ id: 'e1', source: 'start', target: 'push' }],
+    }],
+  };
+}
+
+/** The same `http` node, moved inside a `try_catch`'s `try` region. */
+function guardedHttpFlow(body: unknown) {
+  return {
+    flows: [{
+      name: 'incident_push',
+      runAs: 'system',
+      nodes: [
+        { id: 'start', type: 'start', config: { triggerType: 'schedule', schedule: 'cron:0 3 * * *' } },
+        {
+          id: 'guard', type: 'try_catch', label: 'Guard',
+          config: {
+            try: {
+              nodes: [{ id: 'push', type: 'http', label: 'POST incident', config: httpPushConfig(body) }],
+              edges: [],
+            },
+            catch: {
+              nodes: [{ id: 'log_failure', type: 'create_record', label: 'Log failure', config: { objectName: 'sync_error' } }],
+              edges: [],
+            },
+          },
+        },
+      ],
+      edges: [{ id: 'e1', source: 'start', target: 'guard' }],
+    }],
+  };
+}
+
+describe('#16405 — an `http` node payload is not a region, and both #1315 rules read it', () => {
+  /**
+   * #5700's bar, applied here: a payload these rules judge has to be one an
+   * author can really write, or the pins prove a rule against metadata the
+   * schema refuses. `HttpConfigSchema` is a `strictObject`, so a misspelled key
+   * would surface as an `unrecognized_key` rather than being dropped.
+   */
+  it('pins the fixture payload as an authorable `http` config', () => {
+    const parsed = HttpConfigSchema.safeParse(httpPushConfig({ text: 'Incident {{record.title}}' }));
+    expect(parsed.success ? [] : parsed.error.issues.map((i) => `${i.path.join('.')}: ${i.message}`)).toEqual([]);
+    expect(parsed.success).toBe(true);
+  });
+
+  it('pins the guarded fixture as an authorable `try_catch` config', () => {
+    const cfg = (guardedHttpFlow({ text: '{{record.title}}' }).flows[0].nodes[1] as { config: unknown }).config;
+    const parsed = TryCatchConfigSchema.safeParse(cfg);
+    expect(parsed.success ? [] : parsed.error.issues.map((i) => `${i.path.join('.')}: ${i.message}`)).toEqual([]);
+    expect(parsed.success).toBe(true);
+  });
+
+  describe('flow-double-brace-interpolation', () => {
+    it('flags a `{{ }}` token in a top-level `http` node payload', () => {
+      const fnds = lintFlowPatterns(httpFlow({ text: 'Incident {{record.title}}' }))
+        .filter((f) => f.rule === FLOW_DOUBLE_BRACE_INTERP);
+      expect(fnds).toHaveLength(1);
+      expect(fnds[0].where).toBe("flow 'incident_push' · node 'push' (http)");
+      expect(fnds[0].message).toContain('{{record.title}}');
+    });
+
+    it('flags the same token when the `http` node sits inside a region', () => {
+      const fnds = lintFlowPatterns(guardedHttpFlow({ text: 'Incident {{record.title}}' }))
+        .filter((f) => f.rule === FLOW_DOUBLE_BRACE_INTERP);
+      expect(fnds).toHaveLength(1);
+      expect(fnds[0].where).toBe("flow 'incident_push' · try_catch 'guard' try · node 'push' (http)");
+    });
+
+    it('reaches a token nested deep inside the payload, not only its top level', () => {
+      const fnds = lintFlowPatterns(httpFlow({ fields: [{ value: '{{record.amount}}' }] }))
+        .filter((f) => f.rule === FLOW_DOUBLE_BRACE_INTERP);
+      expect(fnds).toHaveLength(1);
+    });
+  });
+
+  describe('flow-bare-dollar-reference', () => {
+    it('flags a bare `$ref.field` in a top-level `http` node payload', () => {
+      const fnds = lintFlowPatterns(httpFlow({ ticket: '$source.id' }))
+        .filter((f) => f.rule === FLOW_BARE_DOLLAR_REF);
+      expect(fnds).toHaveLength(1);
+      expect(fnds[0].where).toBe("flow 'incident_push' · node 'push' (http)");
+      expect(fnds[0].message).toContain('$source.id');
+    });
+
+    it('flags the same reference when the `http` node sits inside a region', () => {
+      const fnds = lintFlowPatterns(guardedHttpFlow({ ticket: '$source.id' }))
+        .filter((f) => f.rule === FLOW_BARE_DOLLAR_REF);
+      expect(fnds).toHaveLength(1);
+      expect(fnds[0].where).toBe("flow 'incident_push' · try_catch 'guard' try · node 'push' (http)");
+    });
+  });
+
+  it('still raises nothing for a correct single-brace payload', () => {
+    const fnds = lintFlowPatterns(httpFlow({ id: '{record.id}', owner: '{$User.Id}', note: 'Total $5' }))
+      .filter((f) => f.rule === FLOW_DOUBLE_BRACE_INTERP || f.rule === FLOW_BARE_DOLLAR_REF);
+    expect(fnds).toEqual([]);
+  });
+
+  /**
+   * The over-correction guard, and the reason the second argument must be the
+   * node's OWN slots rather than nothing at all: a `loop`'s config physically
+   * CONTAINS its body, so a repair that stopped stripping would report this
+   * payload token twice — once on the `http` node that carries it, once on the
+   * `loop` that merely wraps it.
+   */
+  it('reports a payload token inside a `loop` body ONCE, on the node carrying it', () => {
+    const fnds = lintFlowPatterns(loopBodyFlow({
+      nodes: [{
+        id: 'push', type: 'http', label: 'POST incident',
+        config: httpPushConfig({ text: 'Lead {{lead.name}}' }),
+      }],
+      edges: [],
+    // Scoped to this rule: an `http` node in a bare loop body also trips the
+    // #14394 containment warning, a different finding about a different defect.
+    })).filter((f) => f.rule === FLOW_DOUBLE_BRACE_INTERP);
+    expect(fnds).toHaveLength(1);
+    expect(fnds[0].where).toBe(
+      "flow 'campaign_enrollment' · loop 'loop_leads' body · node 'push' (http)",
+    );
+    expect(fnds[0].where).not.toContain("node 'loop_leads'");
+  });
+
+  /**
+   * `try` / `catch` / `branches` — the rest of the flat union — for the same
+   * reason, on a node type that does not own them. No node type in the protocol
+   * declares these as ordinary config today (only `try_catch` and `parallel`
+   * own them, as regions), but `FlowNodeSchema.config` is an open `z.record`, so
+   * an authored key by any of those names on any other node type is metadata a
+   * rule must still read rather than silently delete.
+   */
+  it('reads a `try` / `catch` / `branches` key authored on a node that owns no region', () => {
+    const fnds = lintFlowPatterns(nodeFlow({
+      objectName: 'm',
+      fields: { try: '{{a}}', catch: '{{b}}', branches: '{{c}}' },
+    })).filter((f) => f.rule === FLOW_DOUBLE_BRACE_INTERP);
+    expect(fnds).toHaveLength(3);
+    // Top level too, not only nested under a declared key.
+    const top = lintFlowPatterns(nodeFlow({ objectName: 'm', try: '{{a}}', catch: '{{b}}', branches: '{{c}}' }))
+      .filter((f) => f.rule === FLOW_DOUBLE_BRACE_INTERP);
+    expect(top).toHaveLength(3);
+  });
+});

packages/lint/src/lint-flow-patterns.ts

@@ -163,7 +163,7 @@ import type { FlowNodeParsed, FlowEdgeParsed } from '@objectstack/spec/automatio
 // driver-sql, driver-mongodb and driver-memory execute. This linter asks it
 // rather than hand-writing a fourth copy; see {@link filterCarriesNoCondition}.
 import { reduceFilterVerdict } from '@objectstack/spec/data';
-import { stripRegions, REGION_SLOTS, MAX_REGION_DEPTH } from './flow-walk.js';
+import { stripRegions, ownRegionKeys, REGION_SLOTS, MAX_REGION_DEPTH } from './flow-walk.js';
 import { recordsOf } from './object-graph.js';
 
 export interface FlowLintFinding {
@@ -1552,14 +1552,27 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] {
           }
         }
 
-        // Region-STRIPPED: this scan is recursive and a container's config
-        // physically contains every descendant's, which the walk above already
-        // visits in its own right. Without the strip a `{{ }}` in a loop body
-        // would be reported twice — once here against the `loop`, once against the
-        // node that carries it. With it, the count stays 1 and the finding lands
-        // on the right node (before #5383 it landed only on the container).
+        // Region-STRIPPED, by THIS node type's own slots (#16405). The scan is
+        // recursive and a container's config physically contains every
+        // descendant's, which the walk above already visits in its own right:
+        // without the strip a `{{ }}` in a loop body would be reported twice —
+        // once here against the `loop`, once against the node that carries it.
+        // With it, the count stays 1 and the finding lands on the right node
+        // (before #5383 it landed only on the container).
+        //
+        // `ownRegionKeys(node.type)` rather than the flat union of every region
+        // key on ANY node type, which is what this call site passed until #16405
+        // by taking `stripRegions`' default. That union deleted `body` from every
+        // node's view — and `body` is `loop`'s region slot AND the canonical
+        // request payload on an `http` node, so the whole of an `http` node's
+        // payload was invisible to both rules below. That is the one key where an
+        // uninterpolated token has an outbound consequence: `http-nodes.ts`
+        // interpolates the raw config wholesale, so a `{{ }}` or a bare `$ref.x`
+        // there ships to the endpoint as literal text. Remove fewer than the
+        // node's own slots and the double-count returns; remove more and a key
+        // that was never a region is deleted unread.
         const strings: string[] = [];
-        collectTemplateStrings(stripRegions(node.config), undefined, strings);
+        collectTemplateStrings(stripRegions(node.config, ownRegionKeys(node.type)), undefined, strings);
         for (const str of strings) {
           if (DOUBLE_BRACE.test(str)) {
             findings.push({