Skip to content

Approval: a schedule-triggered run still can't write its own locked record — it carries no ObjectQL context to hold flowRunId (#3456 residual) #3712

Description

@os-zhuang

Residual left deliberately by #3703 (which closed #3456). Filing per Prime Directive #10 so it is tracked rather than living only in a code comment.

What #3703 fixed, and where it stops

#3703 made the approvals record lock exempt the run that opened the pending request, so a flow can write its own target record mid-approval instead of dying on its own RECORD_LOCKED. The exemption is keyed on flowRunId, which the automation engine stamps onto the run context and which travels into ctx.session via each data node's ObjectQL context.

That last hop is the catch. resolveRunDataContext (packages/services/service-automation/src/runtime-identity.ts) returns undefined for one shape:

if (context?.runAs === 'system') { /* … carries flowRunId … */ }
if (!context?.userId) return undefined;   // ← here

An effective runAs:'user' run with no trigger user — a schedule-triggered flow being the canonical case — passes no context at all to the data engine. No context means no flowRunId, so the lock hook cannot recognise the write as the owning run's, and the run is refused with RECORD_LOCKED exactly as before.

Why it wasn't just fixed there

Manufacturing a context solely to carry the run id would change that run's authorization, not only its provenance. Today the missing context is what makes the data security middleware skip — the documented, deliberate fail-open from #1888 (the run executes UNSCOPED, with a loud warning, and the flow-schedule-runas-unscoped lint flags the shape at build time). Passing { isSystem: false, positions: [], permissions: [], flowRunId } would flip it to baseline-member RLS, which is a different and larger decision than #3456 was scoped to make.

Impact

Low. This is the intersection of three conditions — a scheduled (or otherwise user-less) trigger, an approval node with lockRecord, and the flow writing its own target record while its request is pending. And it is not a dead end: the dead-run sweep from #3703 releases the record once the run reaches a terminal state, and the #3424 admin override remains the manual escape hatch.

Options

  1. Give the identity-less case a provenance-only context. Cleanest for Approval: a dead approval run can leave the record RECORD_LOCKED (#3424 follow-up, expected-behavior 3) #3456, but requires deciding what happens to the [P0][security] Flow runAs never switches execution identity #1888 fail-open — either accept the flip to baseline-member RLS, or find a way to carry flowRunId without presenting an identity the middleware keys on.
  2. Resolve the [P0][security] Flow runAs never switches execution identity #1888 fail-open first (make user-less runAs:'user' runs an authoring error, or default them to an explicit posture) and let this fall out of it. Larger, but it retires a footgun rather than routing around it.
  3. Leave it. The sweep already recovers the record; document the shape and close.

Option 2 is the one that removes the underlying ambiguity; option 1 is the narrow fix if the fail-open is being kept.

Where it is recorded today

Refs #3456, #3424, #1888, #3703.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions