Skip to content

The #1888 user-less fail-open is wider than the lint that guards it — record-change flows fired by a system write run UNSCOPED, unlinted #3760

Description

@os-zhuang

Filed per Prime Directive #10 to capture the decision #3712 deliberately left open. #3749 (which closed #3712) decoupled write provenance from identity, so the fail-open can now be decided on its own merits instead of being forced by an unrelated fix — but it did not decide it.

The claim as it stands today

ADR-0049 / #1888 document the user-less fail-open as a schedule problem, guarded by two nets: a loud runtime [runAs] warning, and the build-time lint flow-schedule-runas-unscoped. Investigating both nets, neither is as strong as the narrative implies.

Net 1 — the lint never fails the build

packages/cli/src/commands/compile.ts:400-405, verbatim:

Flow authoring anti-pattern lint (#1874) — advisory warnings … NEVER fails the build.

It prints. Nothing stops a publish. For the audience this rule exists to protect — very often an AI generating flow metadata in bulk — an advisory line is close to no net at all.

Net 2 — the lint covers one shape; the runtime hole has more

The lint's predicate (packages/cli/src/utils/lint-flow-patterns.ts:311):

const runAs = typeof flow.runAs === 'string' ? flow.runAs : 'user';
if (isScheduleTriggered(flow, startCfg) && runAs !== 'system') {  }

The runtime's predicate (packages/services/service-automation/src/runtime-identity.ts):

export function runIsUnscopedUserMode(context) {
  return context?.runAs !== 'system' && !context?.userId;
}

isScheduleTriggered!userId. Anything that resolves no trigger user is unscoped at runtime; only the schedule shape is flagged at authoring time.

The unlinted path

packages/triggers/trigger-record-change/src/record-change-trigger.ts:325 forwards the writer's identity with no fallback:

userId: session.userId,

and dispatch is suppressed only for skipTriggers (line 233) — isSystem does not suppress it. So:

  1. a write carrying isSystem and no userId — a plugin/service write, the approvals status mirror, or a runAs:'system' flow's own data node — lands;
  2. it dispatches the record-change flows bound to that object with userId: undefined;
  3. if such a flow left runAs at the spec default 'user', its data nodes run UNSCOPED.

Nothing flags this at authoring time, and nothing can: whether the triggering write carries a user is only knowable at run time.

Given that record-change is the most common flow type and runAs defaults to 'user', this is plausibly the dominant real-world shape of the fail-open — not the schedule case the docs and the lint both centre on.

Exposure

First-party content is clean: every schedule-triggered flow in the repo (examples/app-todo ×2, examples/app-showcase) already declares runAs: 'system', with comments citing ADR-0049. The exposure is third-party and AI-authored metadata.

Options

A pure authoring-side fix cannot close this — the record-change path is not statically decidable. The convergence point has to be the runtime.

  1. Inherit the posture of the triggering write. A cascade whose triggering write was isSystem runs explicitly isSystem; only a genuinely origin-less run (a schedule) is fail-closed or must declare. Effective permissions are unchanged (today's behaviour is unscoped), so breakage is close to zero — but the elevation stops being accidental and becomes intentional and audit-attributable, which is what ADR-0049 actually asks for.
  2. Runtime fail-closed. User-less + effective runAs:'user' → refuse the data op, with an error naming runAs:'system' as the fix. Retires the footgun outright, but converts today's working record-change flows into run-time failures that no build surfaces.
  3. Baseline-member RLS. Run restricted instead of unscoped. No elevation, but flows that see every row today would silently see fewer or none — a quieter failure mode than an error, and harder to diagnose. This is the outcome Approval: a schedule-triggered run still can't write its own locked record — it carries no ObjectQL context to hold flowRunId (#3456 residual) #3712 predicted for "manufacturing a context".
  4. Author-side hardening only. Make the lint blocking and extend it to every statically-decidable shape. Zero breakage, but explicitly leaves the record-change path open.

Option 1 looks like the best breakage-to-benefit ratio; option 2 is the most thorough and the most disruptive.

Whichever is chosen, two fixes stand on their own:

Refs #1888, #3712, #3749, #3456, ADR-0049.

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