Skip to content

feat(core): cross-flow analysis — subflow resolution, cross-subflow taint tracing, all-distribution wiring - #308

Open
RubenHalman wants to merge 11 commits into
mainfrom
claude/salesforce-flow-engine-gaps-cxv9pj
Open

feat(core): cross-flow analysis — subflow resolution, cross-subflow taint tracing, all-distribution wiring#308
RubenHalman wants to merge 11 commits into
mainfrom
claude/salesforce-flow-engine-gaps-cxv9pj

Conversation

@RubenHalman

@RubenHalman RubenHalman commented Aug 8, 2026

Copy link
Copy Markdown
Member

Summary

Gives LFS its first analysis tier that crosses flow boundaries, wired into every distribution. Every rule today is single-flow; this adds subflow resolution, a data-flow (def-use) layer, interprocedural taint tracing on top of it, and the plumbing so it works in the CLI, VS Code extension, GitHub Action, and browser/UMD consumers.

Four self-contained commits, each building and tested independently.

What's included

1. Subflow resolution foundation

Restores the previously-closed subflow-resolution work (PR #292) on top of current main and finishes the CLI wiring it never had.

  • SubflowResolver interface with NoOpResolver / PreloadedResolver, plumbed through IRulesConfig.subflowResolverScanFlows → each rule's check().
  • LoopRuleCommon gains interprocedural traversal: dml-in-loop / soql-in-loop / action-call-in-loop now follow subflow call chains (cycle-safe) and report on the subflow call node, including nested Parent → Middle → Child chains.
  • New system rule UnresolvedSubflow (issue #272). Flow.getSubflowNodes/getSubflowNames/hasSubflows helpers.
  • RuleRegistry reconciled with current main (options-object register(), isSystem filtering) preserving all existing rules.
  • Original review fixes applied (file-handle finally, async glob, CommonJS-safe imports).

2. Def-use / data-flow layer (FlowDataFlow)

Per-element read/write variable sets resolved from raw element data — the foundation taint tracing needs. ReferenceExtractor handles base-variable reduction, {!merge} fields, structured <value> references. Covers assignments (incl. compound operators), decisions, record ops (all output modes), loops (self-named current item), action calls, screens (nested fields), custom errors; formula resolution; subflow call boundaries; input/screen variables as sources.

3. Cross-subflow taint tracing + rule

  • TaintAnalyzer: propagates untrusted markings from sources (screen inputs, flow input variables, configurable globals) through the def-use graph to sinks via a flow-insensitive fixpoint (sound "may be tainted"); formula taint is transitive; cross-flow analysis follows subflow calls through the resolver with cycle detection.
  • New beta rule PreventPassingUserDataIntoElementWithSharing: flags user data reaching a DB op in a flow running System Mode Without Sharing, or passed into a subflow that runs without sharing.

4. All-distribution wiring + browser support

  • Core (browser-safe, pure): buildResolver(roots, source) resolves the transitive subflow closure breadth-first (deduped, cycle-safe, managed-skip, maxDepth) and returns an eager PreloadedResolver; parseFlowXml / createFlowParser parse Flow XML from a string with no filesystem. The environment supplies how to load one flow; core owns which flows are needed.
  • VS Code extension: both scan paths build a FileSystemResolver over the workspace and pass it in.
  • GitHub Action: builds a PreloadedResolver over all parsed flows once, passed into each per-flow scan.
  • Browser / UMD (e.g. Inspector Reloaded / Chrome): documented buildResolver + org-API source pattern in SUBFLOW_RESOLUTION.md.

Design note: why buildResolver

The rule engine resolves subflows synchronously (getSync) so rules stay sync. Node distributions read from disk eagerly; async sources (browser/org API) can't fetch mid-traversal, so the subflow dependency closure must be pre-resolved up front — that's what buildResolver does. The analysis core stays pure (no fs/glob); discovery lives behind the resolver interface per environment. UMD bundle grows ~0.3 KB gzipped.

Testing

  • 209 core unit tests pass (44 suites); all four packages build; UMD build verified.
  • New fixtures + suites: cross-subflow DML-in-loop (one and two hops); def-use extraction; taint (positive case, intra-flow sink, no-resolver case, and — guarding the false-positive class — a parent handing the same data to a with-sharing subflow, correctly not flagged); buildResolver closure/cycle/managed/maxDepth behavior.
  • Verified end-to-end through the CLI.

Notes for reviewers

  • Taint propagation is flow-insensitive — a deliberate first cut (sound over-approximation); flow-sensitivity is a follow-up precision upgrade.
  • New taint rule and UnresolvedSubflow are opt-in (beta / system), so default scans are unchanged.
  • buildResolver lives in core for now; it can be extracted to a dedicated package if/when it generalizes past flows (cross-object / OmniScript).
  • Minor deferred item: validateFlowContent is duplicated across the CLI and VSX resolvers and could be centralized.

Closes #272.

claude added 4 commits August 8, 2026 21:51
Restores the subflow-resolution work (previously closed PR #292) on top of
current main and finishes the CLI/VSX wiring it never had, giving LFS its
first analysis that crosses flow boundaries.

Core:
- SubflowResolver interface with NoOpResolver / PreloadedResolver, plumbed
  through IRulesConfig.subflowResolver and ScanFlows into each rule's check().
- LoopRuleCommon gains interprocedural traversal: DML/SOQL/Action-in-loop rules
  now follow subflow call chains (with cycle detection) and report on the
  subflow call node, including nested Parent -> Middle -> Child chains.
- New system rule UnresolvedSubflow flags broken subflow references (issue #272).
- Flow.getSubflowNodes/getSubflowNames/hasSubflows helpers.
- RuleRegistry reconciled with current main: options-object register(),
  isSystem support and system-rule filtering, preserving CognitiveComplexity;
  UnresolvedSubflow registered as a system rule (off by default).

CLI/VSX:
- FileSystemResolver (Node) resolves + parses referenced flows by name; the
  scan command builds an eager-loaded resolver over the scanned directories and
  passes it into scan() so rules resolve subflows synchronously.
- Fixes from the original review: close the file handle in a finally block,
  async glob instead of glob.sync, and CommonJS-safe core imports.

Verified end to end: scanning the subflow-resolution example flows reports
dml-in-loop on the parent's subflow call node across one and two subflow hops.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnauS2LtyUdsfqCS1Nm8sK
Adds the data-flow foundation taint tracing needs: for each flow element,
which variables it reads and writes, resolved from the raw element data.

- ReferenceExtractor: base-variable reduction, `{!merge}` field parsing,
  structured `<value><elementReference>` extraction, single/array normalization.
- FlowDataFlow: per-node read/write sets across assignments (incl. compound
  operators), decisions, record lookups/creates/updates/deletes (all output
  modes), loops (self-named current item), action calls, screens (nested
  fields), and custom errors; formula-variable resolution; subflow call
  boundaries (caller<->callee input/output variable mapping); and input/screen
  variables as candidate taint sources.

Reads/writes are stored as base variable names (Account.Name -> Account), a
deliberate over-approximation. Covered by unit tests over example flows.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnauS2LtyUdsfqCS1Nm8sK
Adds interprocedural taint analysis and a rule that uses it — the security
differentiator the def-use layer was built for.

- TaintAnalyzer: propagates "untrusted" markings from sources (screen inputs,
  flow input variables, configurable globals) through the def-use graph to
  sinks, via a flow-insensitive fixpoint (sound "may be tainted"). Formula
  variables taint transitively. Cross-flow analysis follows subflow calls
  through the resolver with cycle detection, mapping caller references onto
  callee input variables at each hop.
- PreventPassingUserDataIntoElementWithSharing (beta rule): flags user data
  reaching a database op in a flow running System Mode Without Sharing, or being
  passed into a subflow that runs without sharing. Intra-flow check needs no
  resolver; cross-subflow needs one.

Fixtures + tests cover the positive case (user input -> without-sharing
subflow), the intra-flow sink, the no-resolver case, and — guarding the
false-positive class — a parent handing the same data to a with-sharing
subflow, which must NOT be flagged. Verified end to end through the CLI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnauS2LtyUdsfqCS1Nm8sK
…upport

Makes subflow resolution / cross-subflow taint work in every distribution, not
just the CLI, and adds the browser (UMD) path.

Core (browser-safe, pure — no new Node deps):
- buildResolver(roots, source): resolves the transitive subflow dependency
  closure breadth-first (deduped, cycle-safe, managed-skip, maxDepth) and
  returns an eager PreloadedResolver. Environment supplies *how* to load one
  flow; core owns *which* flows are needed.
- parseFlowXml(name, xml) / createFlowParser(): parse Flow XML from a string
  without a filesystem, for org-API / in-memory sources. ParseFlows reuses the
  shared parser so behavior stays identical.

Distribution wiring:
- VS Code extension: both scan paths now build a FileSystemResolver over the
  workspace and pass it in (extension host is Node).
- GitHub Action: builds a PreloadedResolver over all parsed flows once and
  passes it into each per-flow scan.

Docs: SUBFLOW_RESOLUTION.md gains a browser/UMD section showing buildResolver
with an org-API source (the Inspector Reloaded / Chrome extension pattern).

Rationale: the rule engine resolves subflows synchronously (getSync), so async
sources (browser/org API) must pre-resolve the closure up front — buildResolver
is that step. UMD bundle grows ~0.3 KB gzipped. 209 core tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnauS2LtyUdsfqCS1Nm8sK
@RubenHalman RubenHalman changed the title feat(core): cross-flow analysis — subflow resolution + cross-subflow taint tracing feat(core): cross-flow analysis — subflow resolution, cross-subflow taint tracing, all-distribution wiring Aug 8, 2026
RubenHalman and others added 6 commits August 9, 2026 02:28
…ction paths

GetRuleDefinitions defaulted systemRules ON (!== false) while
RuleRegistry.getRules() defaulted it OFF (=== true), so scan() with
betaMode: true silently activated the error-severity unresolved-subflow
rule while core.getRules() — the call backing the vsx Configure Rules
UI — never listed it. Rules users can't see or disable must not run
unrequested.

- Single default everywhere: system rules are opt-in (systemRules: true),
  matching the registration comment and SUBFLOW_RESOLUTION.md.
- Explicitly configured rules (isolated mode / getRulesByNames) run
  regardless of the flag — explicit selection overrides the default
  gate, same as beta rules; getRules(['unresolved-subflow']) no longer
  returns [].
- missing-start-reference (category 'system') now registered with
  isSystem: true so registry-level gating matches the category-based
  filter in GetRuleDefinitions.
- Docs/JSDoc updated (IRulesConfig, system-rules.md + its generator).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
resolver.has() returns false for ns__FlowName references — either via
skipManaged or because installed-package flows never exist as local
source — so UnresolvedSubflow reported error-severity false positives
for every legitimate managed-package subflow call. Skip names matching
the managed-package convention (double underscore), consistent with
FileSystemResolver's own detection.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…is can't silently no-op

Rules resolve subflows synchronously via getSync(), which only reads the
loaded-flows cache — a resolver created without eager: true (the
previously documented usage) made TaintAnalyzer and LoopRuleCommon skip
all cross-flow analysis with zero findings and zero warnings. Eager is
now the default in both the cli and vsx resolvers; lazy mode remains
available for consumers that call loadAll() themselves.

Also fixes the MCP docs example that reached into PreloadedResolver's
private 'flows' field instead of using resolve() results.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
traceSubflowChain rebuilt a callee's FlowDataFlow at every call site, and
the analyzer itself was recreated per scanned flow, so a shared utility
subflow referenced by N flows was structurally re-analyzed N+ times per
scan. The analyzer now caches FlowDataFlow by flow name and lives for the
rule instance's lifetime (one scan). Taint fixpoints still run per call
site since tainted inputs differ per edge.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ver API surface

TaintAnalyzer.traceSubflowChain and LoopRuleCommon.findViolationsInSubflowRecursive
each implemented the same recursive subflow walk (visited-set cycle
detection, getSync guard, call-chain accumulation). Both now use one
walkSubflowChainSync helper, so cycle handling and chain building live in
one place.

Also removes never-used plumbing added by this PR:
- SubflowResolutionContext: declared on resolve()/resolveMany() across the
  interface and every implementation, but no call site ever constructed one.
- SubflowBoundary.outputs: computed per subflow call, read by nothing —
  callee outputs are already tracked conservatively as writes of the call
  node.
- LoopRuleCommon's per-check resolvedSubflows map: getSync() is already a
  cache lookup.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…very

The cli and vsx packages carried ~300-line near-identical copies of the
filesystem resolver, already diverging in their discovery strategy. The
shared implementation (index building, validation, load/cache,
resolve/getSync) now lives in core — which already uses fs for parse() —
with file discovery injected via a findFlowFiles option, so core gains no
glob dependency. cli and vsx are thin wrappers supplying their own glob
strategy. Drops the unused fromDirectory/fromFlow convenience statics.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@RubenHalman

Copy link
Copy Markdown
Member Author

Follow-up to the review comment above: the cli/vsx FileSystemResolver duplication is now resolved in this PR (b3fa617) rather than deferred. The shared implementation lives in core (which already uses fs for parse()), with file discovery injected via a findFlowFiles option so core gains no glob dependency — cli and vsx are now ~25-line wrappers supplying their own glob strategy. Net −257 lines; all tests green.

🤖 Generated with Claude Code

BREAKING CHANGE: Violation no longer has a polymorphic 'details' bag whose
shape depended on the element kind and rule. Every field now lives at the
top level with a fixed name and type:

- Element facts: dataType (variables), connectsTo/locationX/locationY
  (nodes), expression (attributes) — formerly nested under details.
- Rule context, unified across all rules: description (was details.error),
  referencedFlow (was subflowName/referencedFlow/targetFlow),
  referencedElement/referencedType (was subflowViolating*), callChain (was
  subflowCallChain/callChain), taintedVariables, sinkType.

On top of that:
- flatten(results): FlatViolation[] replaces exportDetails() and is
  lossless — every Violation field carries over plus flow/rule context;
  the old whitelist silently dropped all cross-flow and taint fields.
- scanFlat(flows, config) is the new one-call flat API for downstream
  consumers (CSV, CI annotations, dashboards); scan() keeps the per-flow
  tree for consumers that need grouping (SARIF).
- detailLevel: 'simple' now strips the optional fields instead of
  deleting the details object.
- CLI/vsx CSV exports and SARIF properties carry the full field set; the
  GitHub Action's dataType/expression columns — which read top-level
  fields that never existed — now actually populate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@RubenHalman

Copy link
Copy Markdown
Member Author

v7 breaking change: flat, uniformly-typed violations (c653283)

Violation.details was a polymorphic bag — {dataType} for variables, {connectsTo, locationX, locationY} for nodes, {expression} for attributes, plus ad-hoc per-rule keys — which made core's output hard to consume flat and led to real bugs (the GitHub Action read detail.dataType/detail.expression at the top level where they never existed, so those columns were always empty).

v7 shape: every field lives at the top level of Violation with a fixed name and type. Rule-specific context uses one shared vocabulary across all rules: description, referencedFlow, referencedElement, referencedType, callChain, taintedVariables, sinkType.

New API: scanFlat(flows, config): FlatViolation[] — one lossless, self-contained record per violation (violation + flow + rule context) — and flatten(results) for flattening an existing tree. exportDetails() is removed (it whitelisted 5 keys and silently dropped all cross-flow/taint fields). scan() keeps returning the per-flow tree for consumers that need grouping (SARIF).

All in-repo consumers migrated (CLI CSV/JSON, vsx CSV export, SARIF properties, Action). 216 tests green, CSV smoke test confirms cross-flow call chains and tainted variables now survive into flat output.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants