fix(security): surface schedule/user-less flow runAs fail-open (#1888 follow-up)

.changeset/scheduled-flow-runas-unscoped.md

@@ -0,0 +1,58 @@
+---
+'@objectstack/service-automation': minor
+'@objectstack/cli': minor
+'@objectstack/spec': patch
+'@objectstack/trigger-schedule': patch
+---
+
+fix(security): surface the schedule/user-less `runAs:'user'` fail-open (#1888 follow-up)
+
+With `flow.runAs` now enforced (#1888), a **schedule-triggered** flow with the
+default `runAs:'user'` has no trigger user. `resolveRunDataContext` returns
+`undefined` for that case, so the CRUD nodes pass no ObjectQL `options.context`
+and the security middleware — which *skips* when there is no identity (it
+delegates auth to the auth layer) — runs the operation **UNSCOPED** (effectively
+elevated). An author who left `runAs` at the `'user'` default expecting a
+restricted run silently gets an unscoped one — a fail-open footgun (ADR-0049: a
+security property must not silently do the opposite of what it implies).
+
+This is the **product decision** to make that explicit, chosen to keep legitimate
+scheduled CRUD working (denying outright would break it, and silently elevating
+would hide the author's intent). Prevention happens where the platform can tell
+intent apart (author/build time); the runtime stays non-breaking but is no longer
+silent:
+
+- **Author-time lint** (`@objectstack/cli`, `lintFlowPatterns`): a new advisory
+  rule `flow-schedule-runas-unscoped` flags a schedule-triggered flow whose
+  effective `runAs` is `user` (explicit or unset) and which performs a data
+  operation — pointing the author at `runAs:'system'`. Catches the footgun at
+  compile time, before deploy (most flows are AI-authored).
+- **Runtime warning** (`@objectstack/service-automation`): the engine now emits a
+  clear one-per-run warning when a user-mode run resolves no trigger identity and
+  the flow touches data — the fail-open is *audible* rather than silent. Behavior
+  is otherwise unchanged (the run still executes), so scheduled CRUD that relied
+  on this is not broken. New helpers `runIsUnscopedUserMode`, `flowTouchesData`,
+  and `DATA_NODE_TYPES` are exported alongside `resolveRunDataContext`.
+- **Spec describe** (`@objectstack/spec`): `FlowSchema.runAs` now states that a
+  scheduled run has no user, so under `user` it runs unscoped — declare `system`.
+
+The first-party example apps that tripped the new lint are fixed to declare
+`runAs:'system'` explicitly (`stale_opportunity_sweep`, the app-todo
+`task_reminder` / `overdue_escalation` sweeps) — they read/write across owners and
+were running unscoped by default.
+
+Longer term, attributing scheduled runs to a dedicated service principal (so they
+are scopable + audit-attributable rather than unscoped) is the right enforcement;
+tracked as M2 follow-up.
+
+Proven by a service-automation unit test (the engine warns once for a user-less
+user-mode data run; stays silent for `system`, for an identified user, and for a
+data-less flow), an end-to-end test wiring the **real `ScheduleTrigger` to the
+real engine** (`@objectstack/trigger-schedule`) that fires a job and asserts the
+user-less identity reaches the engine + trips the warning through the actual cron
+path, and a dogfood gate (`flow-runas-schedule.dogfood.test.ts`) that drives
+user-less runs through the real automation + security + data stack: a
+`runAs:'user'` run reads + writes an owner-scoped note a member cannot — audibly —
+while `runAs:'system'` is the explicit, warning-free equivalent.
+
+Refs #1888, ADR-0049.

examples/app-crm/src/flows/stale-opportunity.flow.ts

@@ -17,6 +17,10 @@ export const StaleOpportunityFlow: Flow = {
   label: 'Stale Opportunity Sweep',
   description: 'Daily sweep that notifies owners of open opportunities untouched for 30+ days and opens a follow-up task.',
   type: 'schedule',
+  // A scheduled run has no trigger user, so it must declare its elevation: this
+  // sweep spans every owner's opportunities and opens follow-up tasks across
+  // them. Without this it would run UNSCOPED by default. (ADR-0049 / #1888)
+  runAs: 'system',
 
   nodes: [
     {

examples/app-todo/src/flows/task.flow.ts

@@ -8,6 +8,9 @@ export const TaskReminderFlow: Flow = {
   label: 'Task Reminder Notification',
   description: 'Automated flow to send reminders for tasks approaching their due date',
   type: 'schedule',
+  // A scheduled run has no trigger user, so it must declare its elevation: this
+  // daily sweep reads every user's tasks to remind them. (ADR-0049 / #1888)
+  runAs: 'system',
 
   variables: [
     { name: 'tasksToRemind', type: 'record_collection', isInput: false, isOutput: false },
@@ -52,6 +55,9 @@ export const OverdueEscalationFlow: Flow = {
   label: 'Overdue Task Escalation',
   description: 'Escalates tasks that have been overdue for more than 3 days',
   type: 'schedule',
+  // A scheduled run has no trigger user; this daily sweep reads + escalates every
+  // user's overdue tasks, so it must run elevated. (ADR-0049 / #1888)
+  runAs: 'system',
 
   variables: [
     { name: 'overdueTasks', type: 'record_collection', isInput: false, isOutput: false },

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

@@ -11,13 +11,20 @@ import {
   FLOW_APPROVAL_REVISE_DEAD_END,
   FLOW_APPROVAL_REVISE_UNMARKED_BACKEDGE,
   FLOW_APPROVAL_REVISE_DISABLED,
+  FLOW_SCHEDULE_RUNAS_UNSCOPED,
 } from './lint-flow-patterns.js';
 
 const CEL = (source: string) => ({ dialect: 'cel', source });
-/** A scheduled flow with a get_record node carrying `filter`. */
+/**
+ * A scheduled flow with a get_record node carrying `filter`. Declares
+ * `runAs: 'system'` so it is the correct shape for a scheduled data flow and
+ * does not also trip the schedule-runAs lint (FLOW_SCHEDULE_RUNAS_UNSCOPED) —
+ * keeping these date-equality cases focused on the filter rule.
+ */
 const filterFlow = (filter: unknown) => ({
   flows: [{
     name: 'expiry_alert',
+    runAs: 'system',
     nodes: [
       { id: 'start', type: 'start', config: { triggerType: 'schedule', schedule: 'cron:0 9 * * *' } },
       { id: 'query', type: 'get_record', config: { objectName: 'contract', filter } },
@@ -252,3 +259,80 @@ describe('lintFlowPatterns — approval revise loop (ADR-0044)', () => {
     expect(lintFlowPatterns(approvalFlow(edges))).toEqual([]);
   });
 });
+
+describe('lintFlowPatterns — schedule runAs unscoped (#1888 / ADR-0049)', () => {
+  /** A schedule-triggered flow that performs a data op, parameterized by runAs / detection signal. */
+  const scheduledDataFlow = (opts: {
+    runAs?: 'system' | 'user';
+    flowType?: string;
+    startConfig?: Record<string, unknown>;
+    nodeType?: string;
+  } = {}) => ({
+    flows: [{
+      name: 'nightly_sweep',
+      ...(opts.flowType !== undefined ? { type: opts.flowType } : { type: 'schedule' }),
+      ...(opts.runAs ? { runAs: opts.runAs } : {}),
+      nodes: [
+        { id: 'start', type: 'start', config: opts.startConfig ?? { triggerType: 'schedule', cron: '0 8 * * *' } },
+        { id: 'op', type: opts.nodeType ?? 'create_record', config: { objectName: 'thing', fields: { a: 1 } } },
+      ],
+      edges: [],
+    }],
+  });
+
+  it('flags a schedule flow whose runAs is unset (defaults to user → unscoped)', () => {
+    const fnds = lintFlowPatterns(scheduledDataFlow());
+    expect(fnds).toHaveLength(1);
+    expect(fnds[0].rule).toBe(FLOW_SCHEDULE_RUNAS_UNSCOPED);
+    expect(fnds[0].where).toContain('nightly_sweep');
+    expect(fnds[0].message).toMatch(/default .*runAs:'user'/);
+    expect(fnds[0].message).toMatch(/UNSCOPED/);
+    expect(fnds[0].hint).toMatch(/runAs:'system'/);
+  });
+
+  it("flags an EXPLICIT runAs:'user' on a schedule (incoherent — no user to scope to)", () => {
+    const fnds = lintFlowPatterns(scheduledDataFlow({ runAs: 'user' }));
+    expect(fnds).toHaveLength(1);
+    expect(fnds[0].rule).toBe(FLOW_SCHEDULE_RUNAS_UNSCOPED);
+    expect(fnds[0].message).toMatch(/runAs:'user'/);
+  });
+
+  it('detects the schedule trigger via any of the three signals', () => {
+    // (a) flow.type === 'schedule' (default in the helper)
+    expect(lintFlowPatterns(scheduledDataFlow({ startConfig: {} }))).toHaveLength(1);
+    // (b) start config.triggerType === 'schedule'
+    expect(lintFlowPatterns(scheduledDataFlow({ flowType: 'autolaunched', startConfig: { triggerType: 'schedule' } }))).toHaveLength(1);
+    // (c) start config carries a schedule descriptor
+    expect(lintFlowPatterns(scheduledDataFlow({ flowType: 'autolaunched', startConfig: { schedule: { type: 'interval', intervalMs: 60000 } } }))).toHaveLength(1);
+  });
+
+  it('flags each data-op node type (get/create/update/delete)', () => {
+    for (const t of ['get_record', 'create_record', 'update_record', 'delete_record']) {
+      const fnds = lintFlowPatterns(scheduledDataFlow({ nodeType: t }));
+      expect(fnds.map((f) => f.rule), `node ${t}`).toContain(FLOW_SCHEDULE_RUNAS_UNSCOPED);
+    }
+  });
+
+  describe('does NOT flag (false-positive guards)', () => {
+    it("a schedule flow that declares runAs:'system' (the correct shape)", () => {
+      expect(lintFlowPatterns(scheduledDataFlow({ runAs: 'system' }))).toHaveLength(0);
+    });
+    it('a schedule flow with NO data node (runAs is moot — e.g. a notify-only digest)', () => {
+      expect(lintFlowPatterns({
+        flows: [{
+          name: 'digest',
+          type: 'schedule',
+          nodes: [
+            { id: 'start', type: 'start', config: { schedule: { type: 'interval', intervalMs: 60000 } } },
+            { id: 'notify', type: 'notify', config: { topic: 'x' } },
+          ],
+          edges: [],
+        }],
+      })).toHaveLength(0);
+    });
+    it('a NON-schedule flow with a data op (record-change / screen runs carry a user)', () => {
+      expect(lintFlowPatterns(scheduledDataFlow({ flowType: 'record_change', startConfig: { triggerType: 'record-after-update', objectName: 'thing' } }))).toHaveLength(0);
+      expect(lintFlowPatterns(scheduledDataFlow({ flowType: 'screen', startConfig: {} }))).toHaveLength(0);
+    });
+  });
+});

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

@@ -53,6 +53,21 @@ export const FLOW_BARE_DOLLAR_REF = 'flow-bare-dollar-reference';
 export const FLOW_APPROVAL_REVISE_DEAD_END = 'flow-approval-revise-dead-end';
 export const FLOW_APPROVAL_REVISE_UNMARKED_BACKEDGE = 'flow-approval-revise-unmarked-backedge';
 export const FLOW_APPROVAL_REVISE_DISABLED = 'flow-approval-revise-disabled';
+export const FLOW_SCHEDULE_RUNAS_UNSCOPED = 'flow-schedule-runas-unscoped';
+
+/** Node types that perform a data operation — the ones `flow.runAs` governs (#1888). */
+const DATA_NODE_TYPES = new Set(['get_record', 'create_record', 'update_record', 'delete_record']);
+
+/**
+ * Does this flow auto-launch on a SCHEDULE (so a run carries no trigger user)?
+ * Accepts the three author-time signals: `flow.type === 'schedule'`, a start-node
+ * `config.triggerType === 'schedule'`, or a start-node `config.schedule` descriptor.
+ */
+function isScheduleTriggered(flow: AnyRec, startCfg: AnyRec): boolean {
+  if (flow.type === 'schedule') return true;
+  if (typeof startCfg.triggerType === 'string' && startCfg.triggerType === 'schedule') return true;
+  return startCfg.schedule != null;
+}
 
 /**
  * Node-config keys that name a capability the automation engine does NOT have.
@@ -282,6 +297,35 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] {
       }
     }
 
+    // (a4) #1888 / ADR-0049 — a SCHEDULE-triggered flow has no trigger user at
+    //      runtime, so an effective `runAs:'user'` (explicit, or unset → the spec
+    //      default 'user') run executes its data nodes UNSCOPED (elevated,
+    //      RLS-bypassing) rather than restricted — the data security middleware
+    //      skips when there is no identity. An author who left `runAs` at the
+    //      default expecting a restricted run gets a fail-open one. Only flagged
+    //      when the flow actually performs a data operation (otherwise runAs is
+    //      moot). The robust shape is an explicit `runAs:'system'`, which makes
+    //      the elevation intentional + audit-attributable; a schedule cannot scope
+    //      to a user because there is none.
+    const runAs = typeof flow.runAs === 'string' ? flow.runAs : 'user';
+    if (isScheduleTriggered(flow, startCfg) && runAs !== 'system') {
+      const dataNode = nodes.find((n) => DATA_NODE_TYPES.has(typeof n.type === 'string' ? (n.type as string) : ''));
+      if (dataNode) {
+        const declared = typeof flow.runAs === 'string' ? `\`runAs:'${runAs}'\`` : `the default \`runAs:'user'\``;
+        findings.push({
+          where: `flow '${flowName}' · runAs`,
+          message:
+            `schedule-triggered flow runs as ${declared}, but a scheduled run has no trigger user — so its ` +
+            `data node '${dataNode.id}' (${dataNode.type}) executes UNSCOPED (elevated, RLS-bypassing), not ` +
+            `restricted to a user.`,
+          hint:
+            `Declare \`runAs:'system'\` to make the elevation explicit and intended (the run reads/writes ` +
+            `every record). A scheduled flow cannot scope to a user — there is none. (ADR-0049, #1888)`,
+          rule: FLOW_SCHEDULE_RUNAS_UNSCOPED,
+        });
+      }
+    }
+
     // (b) #1315 — wrong interpolation syntax in any node's template values. Flow
     //     node values use SINGLE braces; double-brace `{{ }}` and bare `$ref.x`
     //     are carried over from the formula template dialect / other platforms.