Skip to content

Commit ad143ce

Browse files
os-zhuangclaude
andauthored
fix(security): surface schedule/user-less flow runAs fail-open (#1888 follow-up) (#2308)
* fix(security): surface schedule/user-less flow runAs fail-open (#1888 follow-up) With flow `runAs` now enforced (#1888, PR #2302), a SCHEDULE-triggered flow with the default `runAs:'user'` has no trigger user: `resolveRunDataContext` returns undefined, 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. That is fail-open, and exactly the kind of "security property that silently does the opposite of what it implies" ADR-0049 prohibits. This makes the boundary case #1888 deliberately left open an explicit product decision. Denying outright (fail-closed) would break legitimate scheduled CRUD — empirically 2 of 3 example scheduled CRUD flows relied on the default — and silently elevating would hide the author's intent. So prevention happens where the platform can tell intent apart (author/build time), and 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 op, pointing the author at `runAs:'system'`. Catches it at compile time before deploy (most flows are AI-authored). - Runtime warning (@objectstack/service-automation): the engine emits one clear warning per run when a user-mode run resolves no trigger identity and the flow touches data — the fail-open is audible, not silent. Behavior is otherwise unchanged (scheduled CRUD is not broken). New helpers `runIsUnscopedUserMode`, `flowTouchesData`, `DATA_NODE_TYPES` exported beside `resolveRunDataContext`; the two runContext construction sites collapse into one `resolveRunContext`. - Spec describe (@objectstack/spec): FlowSchema.runAs now states a scheduled run has no user, so under `user` it runs unscoped — declare `system`. First-party example flows that tripped the new lint are fixed to declare `runAs:'system'` (stale_opportunity_sweep, app-todo task_reminder / overdue_escalation) — they read/write across owners and were running unscoped. Longer term, attributing scheduled runs to a dedicated service principal (scopable + audit-attributable) is the right enforcement — M2 follow-up. Proven by a service-automation unit test (warns once for a user-less user-mode data run; silent for system / an identified user / a data-less flow) 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. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(trigger-schedule): end-to-end runAs fail-open via the real cron path Add an e2e test wiring the REAL ScheduleTrigger to the REAL AutomationEngine: a fired scheduled job builds a user-less { event:'schedule' } context, reaches the engine, threads the unscoped (user-mode, user-less) identity to the data node, and trips the [runAs] warning — proving the fail-open via the actual cron path (the unit + dogfood tests call engine.execute() with a hand-made context). Also asserts runAs:'system' propagates elevated with no warning. Adds @objectstack/service-automation as a dev-dependency of trigger-schedule (the first place the two real halves meet; the FlowTrigger contract is declared structurally so there is no runtime/build dependency). Refs #1888, ADR-0049. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7d7fee7 commit ad143ce

13 files changed

Lines changed: 661 additions & 10 deletions

File tree

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

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ export const StaleOpportunityFlow: Flow = {
1717
label: 'Stale Opportunity Sweep',
1818
description: 'Daily sweep that notifies owners of open opportunities untouched for 30+ days and opens a follow-up task.',
1919
type: 'schedule',
20+
// A scheduled run has no trigger user, so it must declare its elevation: this
21+
// sweep spans every owner's opportunities and opens follow-up tasks across
22+
// them. Without this it would run UNSCOPED by default. (ADR-0049 / #1888)
23+
runAs: 'system',
2024

2125
nodes: [
2226
{

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ export const TaskReminderFlow: Flow = {
88
label: 'Task Reminder Notification',
99
description: 'Automated flow to send reminders for tasks approaching their due date',
1010
type: 'schedule',
11+
// A scheduled run has no trigger user, so it must declare its elevation: this
12+
// daily sweep reads every user's tasks to remind them. (ADR-0049 / #1888)
13+
runAs: 'system',
1114

1215
variables: [
1316
{ name: 'tasksToRemind', type: 'record_collection', isInput: false, isOutput: false },
@@ -52,6 +55,9 @@ export const OverdueEscalationFlow: Flow = {
5255
label: 'Overdue Task Escalation',
5356
description: 'Escalates tasks that have been overdue for more than 3 days',
5457
type: 'schedule',
58+
// A scheduled run has no trigger user; this daily sweep reads + escalates every
59+
// user's overdue tasks, so it must run elevated. (ADR-0049 / #1888)
60+
runAs: 'system',
5561

5662
variables: [
5763
{ name: 'overdueTasks', type: 'record_collection', isInput: false, isOutput: false },

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

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,20 @@ import {
1111
FLOW_APPROVAL_REVISE_DEAD_END,
1212
FLOW_APPROVAL_REVISE_UNMARKED_BACKEDGE,
1313
FLOW_APPROVAL_REVISE_DISABLED,
14+
FLOW_SCHEDULE_RUNAS_UNSCOPED,
1415
} from './lint-flow-patterns.js';
1516

1617
const CEL = (source: string) => ({ dialect: 'cel', source });
17-
/** A scheduled flow with a get_record node carrying `filter`. */
18+
/**
19+
* A scheduled flow with a get_record node carrying `filter`. Declares
20+
* `runAs: 'system'` so it is the correct shape for a scheduled data flow and
21+
* does not also trip the schedule-runAs lint (FLOW_SCHEDULE_RUNAS_UNSCOPED) —
22+
* keeping these date-equality cases focused on the filter rule.
23+
*/
1824
const filterFlow = (filter: unknown) => ({
1925
flows: [{
2026
name: 'expiry_alert',
27+
runAs: 'system',
2128
nodes: [
2229
{ id: 'start', type: 'start', config: { triggerType: 'schedule', schedule: 'cron:0 9 * * *' } },
2330
{ id: 'query', type: 'get_record', config: { objectName: 'contract', filter } },
@@ -252,3 +259,80 @@ describe('lintFlowPatterns — approval revise loop (ADR-0044)', () => {
252259
expect(lintFlowPatterns(approvalFlow(edges))).toEqual([]);
253260
});
254261
});
262+
263+
describe('lintFlowPatterns — schedule runAs unscoped (#1888 / ADR-0049)', () => {
264+
/** A schedule-triggered flow that performs a data op, parameterized by runAs / detection signal. */
265+
const scheduledDataFlow = (opts: {
266+
runAs?: 'system' | 'user';
267+
flowType?: string;
268+
startConfig?: Record<string, unknown>;
269+
nodeType?: string;
270+
} = {}) => ({
271+
flows: [{
272+
name: 'nightly_sweep',
273+
...(opts.flowType !== undefined ? { type: opts.flowType } : { type: 'schedule' }),
274+
...(opts.runAs ? { runAs: opts.runAs } : {}),
275+
nodes: [
276+
{ id: 'start', type: 'start', config: opts.startConfig ?? { triggerType: 'schedule', cron: '0 8 * * *' } },
277+
{ id: 'op', type: opts.nodeType ?? 'create_record', config: { objectName: 'thing', fields: { a: 1 } } },
278+
],
279+
edges: [],
280+
}],
281+
});
282+
283+
it('flags a schedule flow whose runAs is unset (defaults to user → unscoped)', () => {
284+
const fnds = lintFlowPatterns(scheduledDataFlow());
285+
expect(fnds).toHaveLength(1);
286+
expect(fnds[0].rule).toBe(FLOW_SCHEDULE_RUNAS_UNSCOPED);
287+
expect(fnds[0].where).toContain('nightly_sweep');
288+
expect(fnds[0].message).toMatch(/default .*runAs:'user'/);
289+
expect(fnds[0].message).toMatch(/UNSCOPED/);
290+
expect(fnds[0].hint).toMatch(/runAs:'system'/);
291+
});
292+
293+
it("flags an EXPLICIT runAs:'user' on a schedule (incoherent — no user to scope to)", () => {
294+
const fnds = lintFlowPatterns(scheduledDataFlow({ runAs: 'user' }));
295+
expect(fnds).toHaveLength(1);
296+
expect(fnds[0].rule).toBe(FLOW_SCHEDULE_RUNAS_UNSCOPED);
297+
expect(fnds[0].message).toMatch(/runAs:'user'/);
298+
});
299+
300+
it('detects the schedule trigger via any of the three signals', () => {
301+
// (a) flow.type === 'schedule' (default in the helper)
302+
expect(lintFlowPatterns(scheduledDataFlow({ startConfig: {} }))).toHaveLength(1);
303+
// (b) start config.triggerType === 'schedule'
304+
expect(lintFlowPatterns(scheduledDataFlow({ flowType: 'autolaunched', startConfig: { triggerType: 'schedule' } }))).toHaveLength(1);
305+
// (c) start config carries a schedule descriptor
306+
expect(lintFlowPatterns(scheduledDataFlow({ flowType: 'autolaunched', startConfig: { schedule: { type: 'interval', intervalMs: 60000 } } }))).toHaveLength(1);
307+
});
308+
309+
it('flags each data-op node type (get/create/update/delete)', () => {
310+
for (const t of ['get_record', 'create_record', 'update_record', 'delete_record']) {
311+
const fnds = lintFlowPatterns(scheduledDataFlow({ nodeType: t }));
312+
expect(fnds.map((f) => f.rule), `node ${t}`).toContain(FLOW_SCHEDULE_RUNAS_UNSCOPED);
313+
}
314+
});
315+
316+
describe('does NOT flag (false-positive guards)', () => {
317+
it("a schedule flow that declares runAs:'system' (the correct shape)", () => {
318+
expect(lintFlowPatterns(scheduledDataFlow({ runAs: 'system' }))).toHaveLength(0);
319+
});
320+
it('a schedule flow with NO data node (runAs is moot — e.g. a notify-only digest)', () => {
321+
expect(lintFlowPatterns({
322+
flows: [{
323+
name: 'digest',
324+
type: 'schedule',
325+
nodes: [
326+
{ id: 'start', type: 'start', config: { schedule: { type: 'interval', intervalMs: 60000 } } },
327+
{ id: 'notify', type: 'notify', config: { topic: 'x' } },
328+
],
329+
edges: [],
330+
}],
331+
})).toHaveLength(0);
332+
});
333+
it('a NON-schedule flow with a data op (record-change / screen runs carry a user)', () => {
334+
expect(lintFlowPatterns(scheduledDataFlow({ flowType: 'record_change', startConfig: { triggerType: 'record-after-update', objectName: 'thing' } }))).toHaveLength(0);
335+
expect(lintFlowPatterns(scheduledDataFlow({ flowType: 'screen', startConfig: {} }))).toHaveLength(0);
336+
});
337+
});
338+
});

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,21 @@ export const FLOW_BARE_DOLLAR_REF = 'flow-bare-dollar-reference';
5353
export const FLOW_APPROVAL_REVISE_DEAD_END = 'flow-approval-revise-dead-end';
5454
export const FLOW_APPROVAL_REVISE_UNMARKED_BACKEDGE = 'flow-approval-revise-unmarked-backedge';
5555
export const FLOW_APPROVAL_REVISE_DISABLED = 'flow-approval-revise-disabled';
56+
export const FLOW_SCHEDULE_RUNAS_UNSCOPED = 'flow-schedule-runas-unscoped';
57+
58+
/** Node types that perform a data operation — the ones `flow.runAs` governs (#1888). */
59+
const DATA_NODE_TYPES = new Set(['get_record', 'create_record', 'update_record', 'delete_record']);
60+
61+
/**
62+
* Does this flow auto-launch on a SCHEDULE (so a run carries no trigger user)?
63+
* Accepts the three author-time signals: `flow.type === 'schedule'`, a start-node
64+
* `config.triggerType === 'schedule'`, or a start-node `config.schedule` descriptor.
65+
*/
66+
function isScheduleTriggered(flow: AnyRec, startCfg: AnyRec): boolean {
67+
if (flow.type === 'schedule') return true;
68+
if (typeof startCfg.triggerType === 'string' && startCfg.triggerType === 'schedule') return true;
69+
return startCfg.schedule != null;
70+
}
5671

5772
/**
5873
* Node-config keys that name a capability the automation engine does NOT have.
@@ -282,6 +297,35 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] {
282297
}
283298
}
284299

300+
// (a4) #1888 / ADR-0049 — a SCHEDULE-triggered flow has no trigger user at
301+
// runtime, so an effective `runAs:'user'` (explicit, or unset → the spec
302+
// default 'user') run executes its data nodes UNSCOPED (elevated,
303+
// RLS-bypassing) rather than restricted — the data security middleware
304+
// skips when there is no identity. An author who left `runAs` at the
305+
// default expecting a restricted run gets a fail-open one. Only flagged
306+
// when the flow actually performs a data operation (otherwise runAs is
307+
// moot). The robust shape is an explicit `runAs:'system'`, which makes
308+
// the elevation intentional + audit-attributable; a schedule cannot scope
309+
// to a user because there is none.
310+
const runAs = typeof flow.runAs === 'string' ? flow.runAs : 'user';
311+
if (isScheduleTriggered(flow, startCfg) && runAs !== 'system') {
312+
const dataNode = nodes.find((n) => DATA_NODE_TYPES.has(typeof n.type === 'string' ? (n.type as string) : ''));
313+
if (dataNode) {
314+
const declared = typeof flow.runAs === 'string' ? `\`runAs:'${runAs}'\`` : `the default \`runAs:'user'\``;
315+
findings.push({
316+
where: `flow '${flowName}' · runAs`,
317+
message:
318+
`schedule-triggered flow runs as ${declared}, but a scheduled run has no trigger user — so its ` +
319+
`data node '${dataNode.id}' (${dataNode.type}) executes UNSCOPED (elevated, RLS-bypassing), not ` +
320+
`restricted to a user.`,
321+
hint:
322+
`Declare \`runAs:'system'\` to make the elevation explicit and intended (the run reads/writes ` +
323+
`every record). A scheduled flow cannot scope to a user — there is none. (ADR-0049, #1888)`,
324+
rule: FLOW_SCHEDULE_RUNAS_UNSCOPED,
325+
});
326+
}
327+
}
328+
285329
// (b) #1315 — wrong interpolation syntax in any node's template values. Flow
286330
// node values use SINGLE braces; double-brace `{{ }}` and bare `$ref.x`
287331
// are carried over from the formula template dialect / other platforms.

0 commit comments

Comments
 (0)