From 801d1eeb7261e755c8a7d34b568ef27a12c2ae09 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Thu, 25 Jun 2026 00:53:10 +0800 Subject: [PATCH 1/2] fix(security): surface schedule/user-less flow runAs fail-open (#1888 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .changeset/scheduled-flow-runas-unscoped.md | 54 +++++++ .../src/flows/stale-opportunity.flow.ts | 4 + examples/app-todo/src/flows/task.flow.ts | 6 + .../cli/src/utils/lint-flow-patterns.test.ts | 86 +++++++++- packages/cli/src/utils/lint-flow-patterns.ts | 44 +++++ .../test/flow-runas-schedule.dogfood.test.ts | 152 ++++++++++++++++++ .../src/builtin/crud-runas.test.ts | 101 +++++++++++- .../services/service-automation/src/engine.ts | 44 ++++- .../src/runtime-identity.ts | 38 +++++ packages/spec/src/automation/flow.zod.ts | 3 +- 10 files changed, 522 insertions(+), 10 deletions(-) create mode 100644 .changeset/scheduled-flow-runas-unscoped.md create mode 100644 packages/dogfood/test/flow-runas-schedule.dogfood.test.ts diff --git a/.changeset/scheduled-flow-runas-unscoped.md b/.changeset/scheduled-flow-runas-unscoped.md new file mode 100644 index 0000000000..b43303252f --- /dev/null +++ b/.changeset/scheduled-flow-runas-unscoped.md @@ -0,0 +1,54 @@ +--- +'@objectstack/service-automation': minor +'@objectstack/cli': minor +'@objectstack/spec': 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) 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. diff --git a/examples/app-crm/src/flows/stale-opportunity.flow.ts b/examples/app-crm/src/flows/stale-opportunity.flow.ts index a424128486..037c70d48a 100644 --- a/examples/app-crm/src/flows/stale-opportunity.flow.ts +++ b/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: [ { diff --git a/examples/app-todo/src/flows/task.flow.ts b/examples/app-todo/src/flows/task.flow.ts index fe5a44ba3e..be0abc10e4 100644 --- a/examples/app-todo/src/flows/task.flow.ts +++ b/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 }, diff --git a/packages/cli/src/utils/lint-flow-patterns.test.ts b/packages/cli/src/utils/lint-flow-patterns.test.ts index 2f0cfbd668..b04b55f6ac 100644 --- a/packages/cli/src/utils/lint-flow-patterns.test.ts +++ b/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; + 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); + }); + }); +}); diff --git a/packages/cli/src/utils/lint-flow-patterns.ts b/packages/cli/src/utils/lint-flow-patterns.ts index 3c535b59e5..9bda03fa2a 100644 --- a/packages/cli/src/utils/lint-flow-patterns.ts +++ b/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. diff --git a/packages/dogfood/test/flow-runas-schedule.dogfood.test.ts b/packages/dogfood/test/flow-runas-schedule.dogfood.test.ts new file mode 100644 index 0000000000..40fd73ee84 --- /dev/null +++ b/packages/dogfood/test/flow-runas-schedule.dogfood.test.ts @@ -0,0 +1,152 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// FLOW runAs — the SCHEDULE (user-less) fail-open, exercised end-to-end through +// the real automation + security + data stack (#1888 follow-up, ADR-0049). +// +// @proof: flow-runas-schedule +// Sibling of flow-runas.dogfood.test.ts. That gate proves runAs switches identity +// for a USER-triggered run; this one pins the boundary case #1888 deliberately +// left open: a SCHEDULE-triggered run has NO trigger user, so an effective +// `runAs:'user'` (the default) resolves no identity → CRUD nodes pass no ObjectQL +// context → the security middleware SKIPS (it delegates auth to the auth layer) +// → the run executes UNSCOPED (effectively elevated), not restricted. +// +// We reuse the owner-isolated runas_note fixture and drive the flows directly +// through the automation service with a USER-LESS context — exactly the shape the +// schedule trigger builds ({ event:'schedule', params }, no userId) — proving: +// • runAs:'user' (user-less) → UNSCOPED: reads + writes the admin's note a +// member cannot touch, and the engine emits the [runAs] warning (the +// fail-open is now AUDIBLE, not silent); +// • runAs:'system'(user-less) → the explicit, attributable elevation (same +// access) with NO warning — the fix authors should declare. +// +// This is the live, revert-provable form of "passes static checks / silently +// elevated at runtime": if a future change makes the user-less case fail-closed +// (deny) or auto-elevate, the behavioral assertions here go RED and force the +// product decision to be revisited deliberately. + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { runasFixtureStack, runasFixtureSecurity } from './fixtures/flow-runas-fixture.js'; + +/** An AutomationContext shaped exactly like the schedule trigger's: an event + + * params carrying the flow input, but NO userId (a cron fire has no user). */ +function scheduleContext(noteId: string): Record { + return { event: 'schedule', params: { jobId: 'flow-schedule:runas', noteId } }; +} + +/** + * Capture the platform logger's output. In Node the core logger writes via + * `process.stdout/stderr.write` (NOT console.*), so we intercept the streams and + * count lines carrying the engine's `[runAs]` tag. Restore with `.restore()`. + */ +function captureRunAsWarnings() { + const lines: string[] = []; + const sink = (chunk: unknown): boolean => { + lines.push(String(chunk)); + return true; + }; + const outSpy = vi.spyOn(process.stdout, 'write').mockImplementation(sink as never); + const errSpy = vi.spyOn(process.stderr, 'write').mockImplementation(sink as never); + return { + count: () => lines.filter((l) => l.includes('[runAs]')).length, + restore: () => { + outSpy.mockRestore(); + errSpy.mockRestore(); + }, + }; +} + +describe('objectstack verify FLOW: schedule/user-less runAs fail-open (#flow-runas-schedule)', () => { + let stack: VerifyStack; + let adminToken: string; + let memberToken: string; + // The automation engine, registered under the 'automation' service. + let automation: { execute(flow: string, ctx?: unknown): Promise<{ success?: boolean; output?: any }> }; + + beforeAll(async () => { + stack = await bootStack(runasFixtureStack, { automation: true, security: runasFixtureSecurity() }); + adminToken = await stack.signIn(); + // First user is the seeded dev admin → this fresh sign-up is a plain member + // who falls back to the owner-scoped fixture permission set. + memberToken = await stack.signUp('sched-member@runas.test'); + automation = stack.kernel.getService('automation') as typeof automation; + expect(automation?.execute, 'automation service (engine.execute) must be wired').toBeTruthy(); + }, 60_000); + + afterAll(async () => { + await stack?.stop(); + }); + + async function adminCreateNote(name: string): Promise { + const res = await stack.apiAs(adminToken, 'POST', '/data/runas_note', { name, status: 'new' }); + expect(res.status, `admin create ${name} failed: ${res.status}`).toBeLessThan(300); + const j = (await res.json()) as { id?: string; record?: { id?: string } }; + const id = j.id ?? j.record?.id; + expect(id, 'no id returned from create').toBeTruthy(); + return id as string; + } + + async function adminStatusOf(id: string): Promise { + const res = await stack.apiAs(adminToken, 'GET', `/data/runas_note/${id}`); + expect(res.status).toBe(200); + const j = (await res.json()) as { record?: Record } & Record; + return (j.record ?? j).status; + } + + it('precondition: the member is genuinely RLS-denied on the admin note (isolation is real)', async () => { + const id = await adminCreateNote('sched-iso'); + expect(await adminStatusOf(id)).toBe('new'); + const res = await stack.apiAs(memberToken, 'GET', `/data/runas_note/${id}`); + if (res.status === 200) { + const j = (await res.json()) as { record?: Record } & Record; + const rec = (j.record ?? j) as Record; + expect(rec.id ?? rec.name, 'member could READ the admin note — RLS isolation is not in effect').toBeFalsy(); + } else { + expect(res.status).toBe(404); + } + }); + + it("FAIL-OPEN (pinned): a user-less runAs:'user' run executes UNSCOPED — reads + writes the admin note, audibly", async () => { + const id = await adminCreateNote('sched-user'); + const warns = captureRunAsWarnings(); + try { + // READ: user mode + no user → unscoped → finds the admin's note (a member can't). + const read = await automation.execute('runas_user_read', scheduleContext(id)); + expect(read.success, `read run not successful: ${JSON.stringify(read)}`).toBe(true); + const found = read.output?.found; + expect( + found && typeof found === 'object' ? (found as any).id : found, + 'a user-less user-mode run did NOT read unscoped — the fail-open behavior changed; revisit the product decision (ADR-0049/#1888)', + ).toBeTruthy(); + + // WRITE: user mode + no user → unscoped → stamps the admin's note. + const write = await automation.execute('runas_user_touch', scheduleContext(id)); + expect(write.success, `write run not successful: ${JSON.stringify(write)}`).toBe(true); + expect(await adminStatusOf(id)).toBe('touched-user'); + + // ...and the fail-open is AUDIBLE — the engine warned (≥1 across the two runs). + expect( + warns.count(), + 'expected the engine to warn that a user-less user-mode run executes unscoped', + ).toBeGreaterThanOrEqual(1); + } finally { + warns.restore(); + } + }); + + it("EXPLICIT FIX: a user-less runAs:'system' run elevates explicitly — same access, NO warning", async () => { + const id = await adminCreateNote('sched-system'); + const warns = captureRunAsWarnings(); + try { + const read = await automation.execute('runas_system_read', scheduleContext(id)); + expect(read.success, `system read not successful: ${JSON.stringify(read)}`).toBe(true); + expect(read.output?.found, 'system run could not read the record it should see').toBeTruthy(); + + // system names an explicit principal → the run is intentional, not a fail-open. + expect(warns.count(), 'runAs:system must NOT emit the unscoped warning').toBe(0); + } finally { + warns.restore(); + } + }); +}); diff --git a/packages/services/service-automation/src/builtin/crud-runas.test.ts b/packages/services/service-automation/src/builtin/crud-runas.test.ts index 1245659723..1dd5ea98e5 100644 --- a/packages/services/service-automation/src/builtin/crud-runas.test.ts +++ b/packages/services/service-automation/src/builtin/crud-runas.test.ts @@ -14,7 +14,7 @@ import { describe, it, expect } from 'vitest'; import { AutomationEngine } from '../engine.js'; import { registerCrudNodes } from './crud-nodes.js'; -import { resolveRunDataContext } from '../runtime-identity.js'; +import { resolveRunDataContext, runIsUnscopedUserMode, flowTouchesData } from '../runtime-identity.js'; import type { AutomationContext } from '@objectstack/spec/contracts'; function makeLogger(): any { @@ -193,3 +193,102 @@ describe('resolveRunDataContext (#1888 unit)', () => { expect(resolveRunDataContext(undefined)).toBeUndefined(); }); }); + +/** + * #1888 FOLLOW-UP — the user-less fail-open. A schedule-triggered run carries no + * trigger user, so an effective `runAs:'user'` (the default) resolves no identity + * → CRUD nodes omit `options.context` → the data security middleware skips → the + * run executes UNSCOPED (effectively elevated). Denying would break legitimate + * scheduled CRUD and silently elevating would hide the author's intent, so the + * engine keeps the run working but makes the fail-open AUDIBLE: one clear warning + * per run, recommending `runAs:'system'` (ADR-0049). These tests pin both the + * (unchanged, non-breaking) data behavior AND the new warning. + */ +function recordingLogger(): { logger: any; warns: string[] } { + const warns: string[] = []; + const l: any = { info() {}, warn: (m: string) => warns.push(m), error() {}, debug() {} }; + l.child = () => l; + return { logger: l, warns }; +} +const runAsWarns = (warns: string[]) => warns.filter((w) => w.includes('[runAs]')); + +describe('schedule/user-less runs surface the unscoped fail-open (#1888 follow-up)', () => { + it('warns ONCE when a user-mode run has no trigger user and the flow touches data', async () => { + const { logger, warns } = recordingLogger(); + const engine = new AutomationEngine(logger); + const { data, calls } = fakeData(); + registerCrudNodes(engine, ctxWith(data)); + engine.registerFlow('sched', allOpsFlow('sched')); // no runAs → default 'user' + + // Simulate a schedule trigger's context: an event, but NO userId. + const res = await engine.execute('sched', { event: 'schedule', params: { jobId: 'j1' } }); + expect(res.success).toBe(true); + + // Non-breaking: the run still executes, but every data op is UNSCOPED (no ctx). + expect(calls.length).toBeGreaterThan(0); + for (const c of calls) expect(c.ctx, `${c.op} should be unscoped (no identity)`).toBeUndefined(); + + // ...and the fail-open is AUDIBLE: exactly one runAs warning, naming the flow + the fix. + const w = runAsWarns(warns); + expect(w).toHaveLength(1); + expect(w[0]).toContain("flow 'sched'"); + expect(w[0]).toMatch(/UNSCOPED/); + expect(w[0]).toMatch(/runAs:'system'/); + }); + + it("does NOT warn when a user-less run declares runAs:'system' (explicit elevation)", async () => { + const { logger, warns } = recordingLogger(); + const engine = new AutomationEngine(logger); + const { data, calls } = fakeData(); + registerCrudNodes(engine, ctxWith(data)); + engine.registerFlow('sys', allOpsFlow('sys', 'system')); + + await engine.execute('sys', { event: 'schedule', params: {} }); + expect(runAsWarns(warns)).toHaveLength(0); + // Elevation is REAL + explicit (isSystem), not the accidental no-context skip. + for (const c of calls) expect(c.ctx?.isSystem).toBe(true); + }); + + it('does NOT warn when a user IS present (a normal REST/record trigger), even in user mode', async () => { + const { logger, warns } = recordingLogger(); + const engine = new AutomationEngine(logger); + const { data } = fakeData(); + registerCrudNodes(engine, ctxWith(data)); + engine.registerFlow('usr', allOpsFlow('usr', 'user')); + await engine.execute('usr', { userId: 'u1' }); + expect(runAsWarns(warns)).toHaveLength(0); + }); + + it('does NOT warn for a user-less run when the flow touches NO data (runAs is moot)', async () => { + const { logger, warns } = recordingLogger(); + const engine = new AutomationEngine(logger); + const { data } = fakeData(); + registerCrudNodes(engine, ctxWith(data)); + engine.registerFlow('noop', { + name: 'noop', label: 'noop', type: 'schedule', + nodes: [{ id: 'start', type: 'start', label: 'Start' }, { id: 'end', type: 'end', label: 'End' }], + edges: [{ id: 'e1', source: 'start', target: 'end' }], + } as any); + await engine.execute('noop', { event: 'schedule' }); + expect(runAsWarns(warns)).toHaveLength(0); + }); +}); + +describe('runtime-identity unscoped-run predicates (#1888 follow-up unit)', () => { + it('runIsUnscopedUserMode: true ONLY for a non-system run with no user', () => { + expect(runIsUnscopedUserMode({ runAs: 'user' })).toBe(true); // explicit user, no userId + expect(runIsUnscopedUserMode({})).toBe(true); // unset runAs, no userId + expect(runIsUnscopedUserMode(undefined)).toBe(true); + expect(runIsUnscopedUserMode({ runAs: 'user', userId: 'u1' })).toBe(false); // has a user + expect(runIsUnscopedUserMode({ runAs: 'system' })).toBe(false); // elevated + expect(runIsUnscopedUserMode({ runAs: 'system', userId: 'u1' })).toBe(false); + }); + + it('flowTouchesData: true iff the flow contains a data-op node', () => { + expect(flowTouchesData({ nodes: [{ type: 'start' }, { type: 'create_record' }] })).toBe(true); + expect(flowTouchesData({ nodes: [{ type: 'get_record' }] })).toBe(true); + expect(flowTouchesData({ nodes: [{ type: 'start' }, { type: 'notify' }, { type: 'script' }] })).toBe(false); + expect(flowTouchesData({ nodes: [] })).toBe(false); + expect(flowTouchesData(undefined)).toBe(false); + }); +}); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 75d351845b..d7fb293db2 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -15,6 +15,7 @@ import { ConnectorSchema } from '@objectstack/spec/integration'; // `previous.*`, `budget > 100000`, …) skipped its flow. A static import binds the // engine at module load in both ESM and CJS builds. import { ExpressionEngine, validateExpression } from '@objectstack/formula'; +import { runIsUnscopedUserMode, flowTouchesData } from './runtime-identity.js'; // ─── Node Executor Interface (Plugin Extension Point) ─────────────── @@ -915,6 +916,36 @@ export class AutomationEngine implements IAutomationService { return this.executionLogs.find(l => l.id === runId) ?? null; } + /** + * Build the run's effective {@link AutomationContext} from `flow.runAs` — a + * COPY, never mutating the caller's context, so the elevation is scoped to + * this run and the caller's identity is restored when the run returns + * (ADR-0049 / #1888). The single construction point shared by `execute()` and + * `executeWithoutRetry()`. + * + * Also surfaces the user-less **fail-open** footgun (#1888 follow-up): a flow + * whose effective `runAs` is `'user'` but whose trigger carries no user — e.g. + * a schedule-triggered run — has no user to scope to, so its data nodes run + * UNSCOPED (the data security middleware skips when there is no identity). + * Denying would break legitimate scheduled CRUD and silently elevating would + * hide the author's intent, so the run proceeds — but we log a clear warning so + * the elevation is *audible* rather than silent. Authors should declare + * `runAs:'system'` to make scheduled elevation explicit (the build-time lint + * `flow-schedule-runas-unscoped` flags the same shape earlier). + */ + private resolveRunContext(flow: FlowParsed, context?: AutomationContext): AutomationContext { + const runContext: AutomationContext = { ...(context ?? {}), runAs: flow.runAs ?? 'user' }; + if (runIsUnscopedUserMode(runContext) && flowTouchesData(flow)) { + this.logger.warn( + `[runAs] flow '${flow.name}' executes with runAs:'user' but its trigger carries no user ` + + `(e.g. a schedule) — its data operations run UNSCOPED (elevated, RLS-bypassing), not ` + + `restricted. Declare runAs:'system' to make the elevation explicit and intended ` + + `(ADR-0049, #1888).`, + ); + } + return runContext; + } + async execute(flowName: string, context?: AutomationContext): Promise { const startTime = Date.now(); const flow = this.flows.get(flowName); @@ -967,11 +998,10 @@ export class AutomationEngine implements IAutomationService { const steps: StepLogEntry[] = []; // ADR-0049 / #1888 — establish the run's effective execution identity - // from flow.runAs. Thread a COPY (never mutating the caller's context) - // so the elevation is scoped to this run; the caller's identity is - // restored (unchanged) when execute() returns. Data nodes translate - // context.runAs into the ObjectQL context they pass (resolveRunDataContext). - const runContext: AutomationContext = { ...(context ?? {}), runAs: flow.runAs ?? 'user' }; + // from flow.runAs (a COPY, never mutating the caller's context, so the + // elevation is scoped to this run and the caller's identity is restored + // when execute() returns). Surfaces the user-less fail-open (see helper). + const runContext = this.resolveRunContext(flow, context); try { // Find the start node @@ -2226,8 +2256,8 @@ export class AutomationEngine implements IAutomationService { const steps: StepLogEntry[] = []; // ADR-0049 / #1888 — establish the run's effective execution identity - // from flow.runAs (see execute()); threaded to node executors below. - const runContext: AutomationContext = { ...(context ?? {}), runAs: flow.runAs ?? 'user' }; + // from flow.runAs (see execute() / resolveRunContext); threaded below. + const runContext = this.resolveRunContext(flow, context); try { const startNode = flow.nodes.find(n => n.type === 'start'); diff --git a/packages/services/service-automation/src/runtime-identity.ts b/packages/services/service-automation/src/runtime-identity.ts index b844ef71cc..3c75a733fb 100644 --- a/packages/services/service-automation/src/runtime-identity.ts +++ b/packages/services/service-automation/src/runtime-identity.ts @@ -58,3 +58,41 @@ export function resolveRunDataContext(context: AutomationContext | undefined): R if (context.tenantId) out.tenantId = context.tenantId; return out; } + +/** + * Node types that perform an ObjectQL data operation — the ones that thread + * {@link resolveRunDataContext} into the data engine as `options.context`. A + * run's `runAs` only has teeth for a flow that contains at least one of these: + * a flow that merely sends email / waits / branches touches no data, so its + * execution identity is moot. + */ +export const DATA_NODE_TYPES: ReadonlySet = new Set([ + 'get_record', + 'create_record', + 'update_record', + 'delete_record', +]); + +/** True when `flow` contains at least one data-operation node ({@link DATA_NODE_TYPES}). */ +export function flowTouchesData(flow: { nodes?: ReadonlyArray<{ type?: string }> } | undefined): boolean { + return !!flow?.nodes?.some((n) => typeof n?.type === 'string' && DATA_NODE_TYPES.has(n.type)); +} + +/** + * True when a run's effective identity is the fail-open *unscoped* case: an + * effective `runAs:'user'` (explicit or defaulted) with NO resolvable trigger + * user — e.g. a schedule-triggered run, which has no user to scope to (#1888). + * + * {@link resolveRunDataContext} returns `undefined` for this case, so the CRUD + * node omits `options.context` and the data security middleware — which *skips* + * when there is no identity (delegating auth to the auth layer) — runs the + * operation UNSCOPED (effectively elevated). An author who left `runAs` at the + * `'user'` default expecting a restricted run instead gets an unscoped one. The + * engine uses this predicate to surface the footgun at run time (a loud warning, + * not a silent elevation); the build-time lint `flow-schedule-runas-unscoped` + * catches it earlier, and declaring `runAs:'system'` makes the elevation + * explicit and intended (ADR-0049). + */ +export function runIsUnscopedUserMode(context: AutomationContext | undefined): boolean { + return context?.runAs !== 'system' && !context?.userId; +} diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 908247f706..20fe402cab 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -273,7 +273,8 @@ export const FlowSchema = lazySchema(() => z.object({ .enum(['system', 'user']) .default('user') .describe( - 'Execution identity for the run: system = elevated (bypasses RLS), user = the triggering user (RLS-respecting).', + 'Execution identity for the run: system = elevated (bypasses RLS), user = the triggering user (RLS-respecting). ' + + 'A schedule-triggered run has no trigger user, so under user it runs UNSCOPED (elevated) — declare system to make that explicit.', ), /** Error Handling Strategy */ From 7c5cd37c0492a5f60f821cd5d1ab25133ded464b Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Thu, 25 Jun 2026 01:12:39 +0800 Subject: [PATCH 2/2] test(trigger-schedule): end-to-end runAs fail-open via the real cron path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .changeset/scheduled-flow-runas-unscoped.md | 8 +- .../triggers/trigger-schedule/package.json | 1 + .../src/schedule-runas-e2e.test.ts | 131 ++++++++++++++++++ pnpm-lock.yaml | 3 + 4 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 packages/triggers/trigger-schedule/src/schedule-runas-e2e.test.ts diff --git a/.changeset/scheduled-flow-runas-unscoped.md b/.changeset/scheduled-flow-runas-unscoped.md index b43303252f..621d5879a5 100644 --- a/.changeset/scheduled-flow-runas-unscoped.md +++ b/.changeset/scheduled-flow-runas-unscoped.md @@ -2,6 +2,7 @@ '@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) @@ -46,8 +47,11 @@ 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) and a dogfood gate (`flow-runas-schedule.dogfood.test.ts`) that -drives user-less runs through the real automation + security + data stack: 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. diff --git a/packages/triggers/trigger-schedule/package.json b/packages/triggers/trigger-schedule/package.json index d25fb32866..b88b4c8df6 100644 --- a/packages/triggers/trigger-schedule/package.json +++ b/packages/triggers/trigger-schedule/package.json @@ -21,6 +21,7 @@ "@objectstack/spec": "workspace:*" }, "devDependencies": { + "@objectstack/service-automation": "workspace:*", "@types/node": "^26.0.0", "typescript": "^6.0.3", "vitest": "^4.1.9" diff --git a/packages/triggers/trigger-schedule/src/schedule-runas-e2e.test.ts b/packages/triggers/trigger-schedule/src/schedule-runas-e2e.test.ts new file mode 100644 index 0000000000..dfc8805744 --- /dev/null +++ b/packages/triggers/trigger-schedule/src/schedule-runas-e2e.test.ts @@ -0,0 +1,131 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// END-TO-END: the REAL ScheduleTrigger wired to the REAL AutomationEngine, proving +// that a scheduled flow's JOB FIRE produces a USER-LESS context that reaches the +// engine and trips the unscoped-run fail-open warning (#1888 follow-up, ADR-0049). +// +// The unit + dogfood tests for this fix call `engine.execute()` with a hand-made +// `{ event:'schedule' }` context. This test closes that gap by exercising the +// ACTUAL cron path the platform runs: +// +// job fires -> ScheduleTrigger builds { event:'schedule', params } (NO userId) +// -> engine's activateFlowTrigger callback -> engine.execute -> resolveRunContext +// -> warns AND threads the unscoped (user-mode, user-less) identity to the data node. +// +// ScheduleTrigger declares the FlowTrigger contract structurally (no build dep on +// the automation package), so this is the first place the two real halves meet. + +import { describe, it, expect } from 'vitest'; +import { AutomationEngine } from '@objectstack/service-automation'; +import type { AutomationContext, JobSchedule, JobHandler } from '@objectstack/spec/contracts'; +import { ScheduleTrigger } from './schedule-trigger.js'; + +function recordingLogger(): { logger: any; warns: string[] } { + const warns: string[] = []; + const l: any = { info() {}, warn: (m: string) => warns.push(m), error() {}, debug() {} }; + l.child = () => l; + return { logger: l, warns }; +} +const runAsWarns = (warns: string[]) => warns.filter((w) => w.includes('[runAs]')); + +const silent = { info() {}, warn() {}, debug() {} }; +const flush = () => new Promise((r) => setTimeout(r, 0)); + +/** Fake IJobService slice with a deterministic `fire()` standing in for the cron tick. */ +function fakeJobService() { + const jobs = new Map(); + return { + service: { + async schedule(name: string, schedule: JobSchedule, handler: JobHandler) { + jobs.set(name, { schedule, handler }); + }, + async cancel(name: string) { + jobs.delete(name); + }, + }, + has: (name: string) => jobs.has(name), + fire: async (name: string, jobId = 'tick1') => { + await jobs.get(name)?.handler({ jobId } as never); + }, + }; +} + +/** A schedule-triggered flow that touches data, parameterized by runAs. */ +function scheduledDataFlow(name: string, runAs?: 'system' | 'user') { + return { + name, + label: name, + type: 'schedule', + ...(runAs ? { runAs } : {}), + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: { schedule: { type: 'interval', intervalMs: 1000 } } }, + { id: 'mk', type: 'create_record', label: 'Create', config: { objectName: 'thing', fields: { a: 1 } } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'mk' }, + { id: 'e2', source: 'mk', target: 'end' }, + ], + } as never; +} + +/** Register a stub data executor that captures the context it is handed. */ +function captureDataContext(engine: AutomationEngine): AutomationContext[] { + const seen: AutomationContext[] = []; + engine.registerNodeExecutor({ + type: 'create_record', + async execute(_node: unknown, _vars: unknown, context: AutomationContext) { + seen.push(context); + return { success: true, output: {} }; + }, + } as never); + return seen; +} + +describe('schedule trigger -> engine: user-less runAs fail-open via the REAL cron path (#1888)', () => { + it('a fired scheduled job runs the flow UNSCOPED (user-less) and the engine warns', async () => { + const { logger, warns } = recordingLogger(); + const engine = new AutomationEngine(logger); + const seen = captureDataContext(engine); + + engine.registerFlow('nightly_sweep', scheduledDataFlow('nightly_sweep')); // no runAs -> default 'user' + const job = fakeJobService(); + engine.registerTrigger(new ScheduleTrigger(() => job.service, silent)); + await flush(); // let ScheduleTrigger.start() schedule the job + + expect(job.has('flow-schedule:nightly_sweep'), 'flow was not bound to the schedule trigger').toBe(true); + + // Simulate the cron tick - this is what the job service does on schedule. + await job.fire('flow-schedule:nightly_sweep', 'tick-42'); + + // The data node ran with the schedule's USER-LESS, user-mode context. + expect(seen).toHaveLength(1); + expect(seen[0].event).toBe('schedule'); + expect(seen[0].params).toMatchObject({ jobId: 'tick-42', flowName: 'nightly_sweep' }); + expect(seen[0].userId, 'a scheduled run must carry no trigger user').toBeUndefined(); + expect(seen[0].runAs, 'effective identity is user (the default)').toBe('user'); + + // ...and the engine made the fail-open AUDIBLE. + const w = runAsWarns(warns); + expect(w).toHaveLength(1); + expect(w[0]).toContain("flow 'nightly_sweep'"); + expect(w[0]).toMatch(/UNSCOPED/); + expect(w[0]).toMatch(/runAs:'system'/); + }); + + it("a scheduled runAs:'system' flow reaches the engine elevated, with NO warning", async () => { + const { logger, warns } = recordingLogger(); + const engine = new AutomationEngine(logger); + const seen = captureDataContext(engine); + + engine.registerFlow('sys_sweep', scheduledDataFlow('sys_sweep', 'system')); + const job = fakeJobService(); + engine.registerTrigger(new ScheduleTrigger(() => job.service, silent)); + await flush(); + await job.fire('flow-schedule:sys_sweep'); + + expect(seen).toHaveLength(1); + expect(seen[0].runAs, 'explicit elevation propagated through the cron path').toBe('system'); + expect(runAsWarns(warns), 'an explicitly-elevated scheduled run must not warn').toHaveLength(0); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f59df3c491..b50b897638 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2216,6 +2216,9 @@ importers: specifier: workspace:* version: link:../../spec devDependencies: + '@objectstack/service-automation': + specifier: workspace:* + version: link:../../services/service-automation '@types/node': specifier: ^26.0.0 version: 26.0.0