# Audit — Implicit `$Record` Default from PR #1106 (issue #1120)

Date: 2026-04-17
Auditor: amber
Scope: Active FlowDefinitions in `new-prod-essential` (local test branch of prod cluster) and `staging` TiDB.

## TL;DR

- **New-prod-essential:** 18 flows flagged, **0 executed**, **0 needs-fix**, 17 dormant + 1 scheduled-but-unscheduled.
- **Staging:** 34 flows flagged, **0 executed**, **0 needs-fix**. Includes one extra REST-invocable AutoLaunched flow (`BN_Account_Lead_Relate_Leads_to_Accounts`) — dormant, marked for deactivation.
- No caller passes `$Record` to any flagged flow. The new default at `executor_record_ops.go:100-104` cannot fire for any currently-callable route on these flows.
- **Recommendation:** Close #1120 as no action required for current metadata. Keep a narrow defensive follow-up on the executor to bound the blast radius for the next wave of imported legacy Workflow metadata — see §5.

---

## 1. The change under review

`backend/internal/features/automation/executor_record_ops.go` (introduced by PR #1106 / commit `3a2adb12`) added:

```go
inputRef := update.InputReference
if inputRef == "" {
    if _, hasRecord := interview.ContextData["$Record"]; hasRecord {
        inputRef = "$Record"
    }
}
```

Before PR #1106, a `recordUpdate` with empty `InputReference` fell through the single-record branch without setting `recordId` and was therefore a silent no-op. After PR #1106, if the interview's `ContextData` contains `$Record`, the default pushes `inputRef = "$Record"` and the single-record path at lines 106–145 resolves a record ID and calls `persistence.Update(...)` (or the in-place branch for BeforeSave listeners).

The change was correct for legacy Workflow flows migrated into the executor. The risk called out in #1120 is that **REST-invoked** (`POST /services/data/v63.0/actions/custom/flow/:name`) or **Scheduled** flows may receive `$Record` through a code path that does not wrap the context with `WithBeforeSaveFlow` / `WithTransientFlowInterviews`, silently writing where the prior behavior was a no-op.

### How `$Record` can land in `ContextData`

Reviewed all code paths that populate flow inputs:

| Caller | How `$Record` could appear | Seen in code |
|---|---|---|
| `FlowTriggerListener.executeRecordTriggerFlows` | Sets `inputs["$Record"] = payload.Record` explicitly (`flow_trigger_listener.go:154-156`). | Yes — wraps ctx with `WithBeforeSaveFlow` / `WithTransientFlowInterviews`, so executor takes the correct branch. |
| `FlowExecutor.buildScheduledRunInputs` | Sets `runInputs["$Record"] = record` for each record returned by `QueryWithFilter` (`flow_executor.go:290-291`). | Yes — but the ctx is NOT wrapped. This IS the risk surface. |
| `FlowHandler.ExecuteFlow` (`POST /actions/custom/flow/:name`) | Passes caller-supplied JSON map verbatim (`flow_invocable.go:110-111`). | Only if external caller includes `"$Record"` key. |
| `FlowExecutor.executeFlowAction` / `executeSubflow` | Passes parent flow's explicitly-declared `InputParameters`. Only if the parent metadata names the param `"$Record"`. | Possible but unusual. |
| `initializeFlowContext` | Copies all input keys into ContextData verbatim (`executor_helpers.go:68-84`). | This is the funnel. |

Bottom line: the risk is real whenever a flow (a) has a `recordUpdate` with empty `inputReference` AND (b) is reachable via the Scheduled path or a REST/subflow caller that happens to pass `$Record`.

---

## 2. SQL + analysis used

Both DBs were queried with the same template. Replace user/password for staging.

```sql
SELECT fd.DeveloperName,
       fd.ProcessType,
       IFNULL(fd.TriggerType, '') AS TriggerType,
       IFNULL(fd.TriggerObjectOrEvent, '') AS TriggerObjectOrEvent,
       f.Metadata
FROM FlowDefinition fd
JOIN flow f ON f.Id = fd.ActiveVersionId
WHERE fd.IsDeleted = 0
  AND fd.ActiveVersionId IS NOT NULL AND fd.ActiveVersionId != ''
  AND (
    fd.ProcessType IN ('Flow','Workflow','InvocableProcess')
    OR (fd.ProcessType = 'AutoLaunchedFlow' AND (fd.TriggerType IS NULL OR fd.TriggerType = '' OR fd.TriggerType = 'Scheduled'))
  );
```

Rationale for the `ProcessType` filter:

- Omits `AutoLaunchedFlow` with `TriggerType IN ('RecordBeforeSave','RecordAfterSave','RecordBeforeDelete','RecordAfterDelete')` — those fire exclusively through `FlowTriggerListener`, which always wraps the ctx correctly; PR #1106's behavior on that path is the intended semantics.
- Includes `Workflow` (migrated legacy), `Flow` (screen), `AutoLaunchedFlow`-no-trigger (REST / subflow), `AutoLaunchedFlow`-Scheduled, `InvocableProcess` — every path whose invocation route is not the trigger listener.

For each row, the JSON `Metadata` was parsed and every `recordUpdates[]` entry with a missing / empty `inputReference` was recorded. Plus the flow's input variables and its `start` element (for scheduling / trigger facts).

Analysis script: recorded alongside this doc at `/tmp/audit1120/analyze.py` during the investigation; the algorithm is equivalent to:

```python
for flow in rows:
    md = json.loads(flow["Metadata"])
    for ru in md.get("recordUpdates", []):
        if not (ru.get("inputReference") or "").strip():
            flag(flow, ru)
```

Cross-reference: `FlowInterview` counts per flagged flow were fetched from both DBs to see whether any had ever executed under the current build.

```sql
SELECT fd.DeveloperName, COUNT(fi.Id) AS interviews
FROM FlowDefinition fd
LEFT JOIN FlowInterview fi ON fi.FlowVersionId = fd.ActiveVersionId
WHERE fd.DeveloperName IN (<flagged list>)
GROUP BY fd.DeveloperName;
```

Additional grep: every flagged flow's API name was searched across `backend/`, `frontend/`, `scripts/` for explicit callers.

---

## 3. Candidate flows

### 3.1 new-prod-essential (152 active; 18 flagged)

| DeveloperName | ProcessType | TriggerType | start.object / start.triggerType | Input variables | Risky recordUpdates | Verdict |
|---|---|---|---|---|---|---|
| Campaign_Member_Key_Message | Workflow | — | none | myVariable_current, myVariable_old | 1 → CampaignMember | safe |
| Campaign_Member_Key_Message_from_Contact | Workflow | — | none | same | 1 → CampaignMember | safe |
| Campaign_Member_Key_Message_from_Lead_when_create | Workflow | — | none | same | 1 → CampaignMember | safe |
| Exchange_Rate_in_RevRec | Workflow | — | none | myVariable_current, myVariable_old, RecursiveCountVariable | 8 → Revenue__c / Order | safe |
| Lead_Internal_User | Workflow | — | none | myVariable_current, myVariable_old | 2 → Lead | safe |
| Opp_New_OR_Lost_Date | Workflow | — | none | same | 2 → Opportunity | safe |
| PoC_Status_Change | Workflow | — | none | same | 1 → PoC__c | safe |
| Quote_Contact_Update | Workflow | — | none | same | 1 → Quote | safe |
| Quote_Partner_Name | Workflow | — | none | same | 1 → Quote | safe |
| Quote_Product_Name | Workflow | — | none | myVariable_current, myVariable_old, RecursiveCountVariable | 2 → Quote | safe |
| Quote_Start_Date_End_Date | Workflow | — | none | same | 1 → Quote | safe |
| RevRec_Partner_Name | Workflow | — | none | myVariable_current, myVariable_old | 1 → Revenue__c | safe |
| Sales_Contact_Email_Opt_Out | Workflow | — | none | same | 1 → Contact | safe |
| Servicemandayownerdm | Workflow | — | none | same | 1 → Service_Man_day__c | safe |
| TiDB_Cloud_User | Workflow | — | none | same | 1 → Lead | safe |
| Lead_Partner_Change_Owner_To_Sales | Flow | — | none | recordId | 1 → Lead | safe |
| Partner_Lead_Reject | Flow | — | none | recordId | 1 → Lead | safe |
| Order_Expiration_Notice_90 | AutoLaunchedFlow | Scheduled | Invoice_Collection__c / Scheduled | $Record, $Record__Old, RecursiveCountVariable | 3 → Order | safe-by-accident (see §4.3) |

### 3.2 staging (75 active; 34 flagged)

Same 18 from above (minus a few renames) plus additional Workflow-type migrations. One extra category of interest:

| DeveloperName | ProcessType | TriggerType | Input variables | Notes |
|---|---|---|---|---|
| BN_Account_Lead_Relate_Leads_to_Accounts | AutoLaunchedFlow | — (REST-invocable) | var_Account_Id, var_Domain, var_Lead_Id, var_Object_Field_Changed | 1 recordUpdate with filter `Id = var_Lead_Id`, empty `inputReference`. Does NOT declare `$Record` as an input. Listed for deactivation by `scripts/deactivate-obsolete-flows`. 0 interviews ever. |

The other 33 staging flags fall into the same Workflow (migrated) or Flow (screen, `recordId`-driven) categories as the prod branch. Complete raw data in `/tmp/audit1120/staging_findings.json`; list was truncated here for readability.

---

## 4. Per-flow risk classification

### 4.1 Workflow (15 in prod branch / 31 in staging)

**Verdict: SAFE — dormant legacy metadata.**

- Their input variable schema is `myVariable_current`, `myVariable_old`, (± `RecursiveCountVariable`). None declare `$Record`.
- `ProcessType='Workflow'` rows have `TriggerType=NULL` and `start=null` in metadata. The `FlowTriggerListener` matches on `def.TriggerType`, so nothing dispatches these.
- No backend source file executes a flow by `ProcessType='Workflow'` — verified with `grep ProcessType.*Workflow`. (Only `_System_WorkflowAlert` email-alert lookups exist, unrelated.)
- No frontend / REST caller references any of these DeveloperNames.
- `FlowInterview` count across all of them: **0** in both DBs. They have never run under the current executor.

Even if somebody later plugs one of these into a migrated trigger listener, the listener always sets `inputs["$Record"]` AND wraps `WithBeforeSaveFlow` + `WithTransientFlowInterviews`, landing on the intended in-place branch — exactly the path PR #1106 is designed for.

### 4.2 Screen flows (2, both DBs: `Lead_Partner_Change_Owner_To_Sales`, `Partner_Lead_Reject`)

**Verdict: SAFE — callers pass `recordId` only.**

- `ProcessType='Flow'`, `input = recordId` (screen-flow UI convention).
- The flagged `recordUpdate` has `inputReference=null` and filter `Id = recordId`. The executor's single-record update path ignores filters, so this has always been a silent no-op.
- Neither flow name appears in `frontend/` or `backend/`. 0 interviews.
- Screen-flow UI invocation passes only the `recordId` key through `ExecuteFlow`. No mechanism inserts `$Record` into `ContextData` on this path.

### 4.3 Scheduled with missing schedule (1 in prod: `Order_Expiration_Notice_90`)

**Verdict: SAFE-BY-ACCIDENT — currently unreachable, but the only flow in the corpus where the risk mechanism is real if the metadata were completed.**

- `start.triggerType = "Scheduled"`, `start.object = "Invoice_Collection__c"`, `$Record` declared as an input of SObject<Invoice_Collection__c>.
- **`start.schedule` is null** → `SchedulerService.shouldRunFlow` returns `false` at `scheduler_service.go:247`. Flow is scheduled but has no schedule spec. 0 interviews observed.
- If that metadata gap were fixed and a schedule added:
  1. `buildScheduledRunInputs` (`flow_executor.go:290`) would populate `inputs["$Record"] = <Invoice_Collection__c row>`.
  2. On each of the 3 `recordUpdate` elements — each targets `Order` (not Invoice_Collection__c) and uses filter `Id = $Record.Order__c` — the new default at `executor_record_ops.go:101-103` would set `inputRef = "$Record"`.
  3. Line 108-109: resolve `ContextData["$Record"]` as an SObject, take `record["Id"]` — the **Invoice_Collection__c Id**, not the `Order__c` lookup the filter actually intends.
  4. `persistence.Update(ctx, "Order", invoiceCollectionId, updates, user)` → either (a) "no rows affected" (no Order row exists with that PK) or (b) a hard error. Not silent cross-object corruption.
- This would be a bug, but it is a bug of *visibility* (error or no-op on mismatched Id) not a bug of silent wrong-record write, because the filter-based single-record-update semantics are not implemented in the executor at all. Pre-existing gap, unchanged by #1106.
- Staging has the same flow under `ProcessType='Workflow'` (metadata drift between clusters), which cannot be scheduled anyway.

### 4.4 AutoLaunched-no-trigger (1 in staging: `BN_Account_Lead_Relate_Leads_to_Accounts`)

**Verdict: SAFE — REST-invocable but dormant and slated for deactivation.**

- Input variables are string IDs (`var_Account_Id`, `var_Lead_Id`, etc.); none is named `$Record`.
- The flagged `recordUpdate` filter is `Id = var_Lead_Id` — again, the executor ignores filters, so this has always been a silent no-op (pre-existing behavior, not a regression from #1106).
- Listed in `scripts/deactivate-obsolete-flows/main.go`. 0 interviews.
- For the new default to fire, a REST caller would have to manually POST `{"inputs": [{"$Record": {...}}]}`. No caller in this repo does that.

---

## 5. Recommendation

1. **Close #1120 as no action required for the current metadata corpus.** The change at `executor_record_ops.go:100-104` has no active victim in either cluster today. The cataloged scheduled flow that *could* be one (`Order_Expiration_Notice_90`) is unreachable by a second independent defect.

2. **Optional, low-cost hardening** (can be done as a minor follow-up, does not block #1120):

   Defense-in-depth — tighten the implicit-`$Record` default so it only fires when the flow metadata explicitly declares `$Record` as an input variable, OR when the ctx has been marked by a record-triggered path. Pseudocode:

   ```go
   if inputRef == "" {
       // Only apply the legacy-Workflow default when the executor knows
       // $Record is the *triggering* record — not just an input variable
       // that happens to be named $Record via a REST caller.
       if _, hasRecord := interview.ContextData["$Record"]; hasRecord {
           if isBeforeSaveFlow(ctx) || isTransientFlowInterview(ctx) || flowMeta.Start != nil && flowMeta.Start.RecordTriggerType != "" {
               inputRef = "$Record"
           }
       }
   }
   ```

   That keeps correctness for the listener path (which is how every currently-active migrated Workflow reaches the executor), while shutting off the default for Scheduled and REST paths. Does not alter any currently-observed behavior because no currently-executing scheduled / REST flow has an empty-`inputReference` `recordUpdate`.

3. **Track the orphans.** The 17 dormant Workflow / screen flows in prod (and 33 in staging) accumulate risk because (a) the filter-based single-record update path is still unimplemented — they were silent no-ops before #1106 and remain silent no-ops, which means any *next* PR that implements filter-based updates will suddenly make all of them live. Recommend adding them to `scripts/deactivate-obsolete-flows` (staging already deactivates a superset) so they cannot regress into executable state.

---

## 6. Appendix — raw evidence

- Queries + exports: `/tmp/audit1120/flows.tsv` (prod branch), `/tmp/audit1120/staging_flows.tsv` (staging).
- Parser: `/tmp/audit1120/analyze.py`, `/tmp/audit1120/analyze_staging.py`.
- Parsed findings: `/tmp/audit1120/findings.json`, `/tmp/audit1120/staging_findings.json`.
- Code locations inspected:
  - `backend/internal/features/automation/executor_record_ops.go:94-150`
  - `backend/internal/features/automation/flow_trigger_listener.go:132-189`
  - `backend/internal/features/automation/scheduler_service.go:195-298`
  - `backend/internal/features/automation/flow_executor.go:158-298`
  - `backend/internal/features/automation/executor_helpers.go:68-84`
  - `backend/internal/interfaces/rest/flow_invocable.go`