Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/22310-elevated-flow-trigger-door.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'@objectstack/runtime': patch
---

fix(runtime): the trigger door refuses a self-triggered flow declared to run as system to any caller but the system principal

Clause-②: no

A self-triggered flow declared to run as system could be started by any signed-in member through the trigger door. `POST /api/v1/automation/:name/trigger` (and the legacy `POST /api/v1/automation/trigger/:name`, and `@objectstack/verify`'s `flows.run`, which answer through the same function) asked only whether the caller was anonymous, so a flow meant to run on its own trigger, or as a sub-flow, also ran elevated for anyone who named it.

**What changes.** A caller that is not the system principal now gets `403 PERMISSION_DENIED` (ADR-0112 envelope) when it starts a flow declared `runAs: 'system'` whose `type` is `autolaunched`, `record_change` or `schedule`. Nothing runs: no run record, no node, no side effect. The refusal names no part of the flow. This includes a platform admin's session, which is a signed-in user, not the system principal.

**What stays as it was.**

- `screen` and `api` flows, elevated or not: doors the author designed (ADR-0073 D2).
- Every flow that does not declare `runAs: 'system'`.
- A parent flow's `subflow` node calling an elevated flow: the child starts through the engine, never through the door.
- The system principal (an in-process caller, such as a job), which starts every flow.

**If a call now answers 403.** That flow runs on its own trigger. To start its work on a user's request, call it from a parent flow's `subflow` node, or, if it is meant to be a door, declare it `type: 'screen'` (or `type: 'api'` for a signed inbound hook) so the elevation is a reviewable choice.
16 changes: 8 additions & 8 deletions content/docs/permissions/system-context.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ the seed loader replaying package fixtures, a plugin's boot reconciler, a
service self-write, a migration.

This page is **the authority** for what that flag actually does. It exists
because the flag is not one concept: it is a single boolean read at **118
because the flag is not one concept: it is a single boolean read at **119
distinct sites across 20 packages**, and knowing three of those behaviours gives
no hint that the other hundred-and-four exist. Every documented app-side bug
traced to `isSystem` had the same shape — the metadata was complete and correct,
Expand Down Expand Up @@ -141,7 +141,7 @@ that silently does not happen.

### 3. Sharing (`plugin-sharing`)

The largest single consumer — **17 of the 118 sites**.
The largest single consumer — **17 of the 119 sites**.

| # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
|:--|:---|:---|:---|
Expand Down Expand Up @@ -180,7 +180,7 @@ The largest single consumer — **17 of the 118 sites**.
| 53 | Package REST route capability gate bypassed | rest | Get: a marketplace publish over REST (`POST /packages/publish`, the one route the REST registrar mounts since #14503) without `manage_metadata`; the package read cohort (`studio.access` / `setup.access`) is enforced by the dispatcher `/packages` domain's own read gate, where the reads are served | `packages/rest/src/package-routes.ts#refusePackageRequest` |
| 54 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `packages/runtime/src/domains/packages.ts#requireManageMetadata`, `#requireReadCapability` |
| 55 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `packages/runtime/src/domains/activation-gate.ts#refuseUngrantedActivationWrite`, `#refuseUngrantedActivationAuthoring` |
| 56 | Automation run-state read, flow-authoring write, the paused-run caller gate (screen read and resume) and the two operator run-lifecycle writes all pass | runtime | Get: run state, flow writes, a paused run's screen and its resume with no grant and without being the run's starter — and cancelling or restoring a suspension without the platform-operator rung. The screen read and the resume ask one predicate, so this is one bypass for both. The lifecycle bypass is the in-process owner's door: plugin-approvals' revise-window recall (ADR-0044) cancels on behalf of a decision it already authorized and recorded | `packages/runtime/src/domains/automation.ts#mayReadRunState`, `#refuseUngrantedFlowWrite`, `#isRunStarterOrRunStateReader`, `#refuseUngrantedRunLifecycleWrite` |
| 56 | Automation run-state read, flow-authoring write, the paused-run caller gate (screen read and resume), the two operator run-lifecycle writes and the trigger door's elevated self-triggered start all pass | runtime | Get: run state, flow writes, a paused run's screen and its resume with no grant and without being the run's starter — and cancelling or restoring a suspension without the platform-operator rung. The screen read and the resume ask one predicate, so this is one bypass for both. The lifecycle bypass is the in-process owner's door: plugin-approvals' revise-window recall (ADR-0044) cancels on behalf of a decision it already authorized and recorded. And starting, through the trigger door, a flow declared `runAs: 'system'` whose type is self-triggered (`autolaunched`, `record_change`, `schedule`), which every other caller is refused `403 PERMISSION_DENIED` (the maintainer's ruling, letter B). That bypass is the in-process caller's, such as a job: inbound HTTP never carries the flag | `packages/runtime/src/domains/automation.ts#mayReadRunState`, `#refuseUngrantedFlowWrite`, `#isRunStarterOrRunStateReader`, `#refuseUngrantedRunLifecycleWrite`, `#refusesElevatedSelfTriggeredStart` |
| 57 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts#assertTenantAdmin` |
| 58 | Email-template organization door passes; webhook provenance stamp skipped | plugin-email, plugin-webhooks | Get (`sys_email_template`): a create or update of a template row. The door refuses every other write that names a caller, `403 PERMISSION_DENIED` (ADR-0131 D6), so the seeds, the boot sweep and the projection of a Studio save are the only writers. Lose (`sys_webhook`): the row is not marked as an admin customization | `packages/plugins/plugin-email/src/email-template-door.ts#isOrganizationWrite`, `packages/plugins/plugin-webhooks/src/webhook-provenance.ts#bindWebhookProvenanceStamp` |
| 59 | **Automation flow data nodes re-add the `owner_id` stamp** (the one place row 2's gap is compensated inline) | service-automation | Get: a flow-authored INSERT under system elevation still lands owned, when the run resolved a user. Fill-only — flow-authored values win | `packages/services/service-automation/src/runtime-identity.ts#stampSystemInsertOwner`, called from `packages/services/service-automation/src/builtin/crud-nodes.ts#registerCrudNodes` |
Expand Down Expand Up @@ -284,7 +284,7 @@ Ownership injection, `readonly` bypass and sharing materialisation are
independent decisions, and a seed loader plausibly wants the first two but not
the third. The concept is nevertheless **staying as one boolean**:

- **Shipped semantics.** `isSystem` is a published contract with 118 read sites
- **Shipped semantics.** `isSystem` is a published contract with 119 read sites
in 20 packages. Splitting it is a breaking contract change across all of them.
(The ruling was taken when the census read 80 sites in 18 packages; the count
has grown, which strengthens rather than weakens the argument.)
Expand Down Expand Up @@ -358,16 +358,16 @@ still holds equal to the census on every pull request:
| Appearances of the bare identifier `isSystem` in non-test sources | 813 | — |
| — parsed as a declaration | 27 | ✅ |
| — parsed as an object-literal / type key (producers and option objects) | 310 | — |
| — parsed as a property **read** | 124 | ✅ |
| — parsed as a property **read** | 125 | ✅ |
| — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ |
| — the remainder: text inside comments and string literals | 358 | — |
| Of those reads: reads of one of the unrelated metadata fields | 6 | ✅ |
| Of those reads: reads of `ExecutionContext.isSystem` | **118** | ✅ |
| — behaviour-bearing (rows 1–61 above) | 115 | ✅ |
| Of those reads: reads of `ExecutionContext.isSystem` | **119** | ✅ |
| — behaviour-bearing (rows 1–61 above) | 116 | ✅ |
| — carry the flag onward only (rows 62–64 above) | 3 | ✅ |
| Packages containing at least one elevation read | **20** | ✅ |
| Files containing at least one elevation read | 55 | ✅ |
| — the distinct symbols those reads live in — what this page anchors | 101 | ✅ |
| — the distinct symbols those reads live in — what this page anchors | 102 | ✅ |
| — of those files, the ones holding more than one read in one symbol | 8 | ✅ |

The six rows marked — are a **dated decomposition, not a live claim**: they were
Expand Down
58 changes: 57 additions & 1 deletion packages/qa/dogfood/test/fixtures/flow-runas-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@
//
// Before the fix the user-mode flows wrongly succeed (security skipped) → RED.
// After the fix they are correctly denied while system-mode still succeeds → GREEN.
//
// The two system flows are `autolaunched`, and since the maintainer's ruling on
// the trigger door (letter B) a caller that is not the system principal may not
// start a `runAs: 'system'` flow of a self-triggered type through that door. So
// the member reaches them the way the platform keeps open for an elevated write:
// through a `runAs: 'user'` PARENT whose `subflow` node calls the elevated child
// (`runas_system_touch_via_parent` / `runas_system_read_via_parent`). The
// elevation the proof is about still belongs to the child; the parent adds none.

import { defineStack } from '@objectstack/spec';
import { ObjectSchema, Field } from '@objectstack/spec/data';
Expand Down Expand Up @@ -114,6 +122,47 @@ export const runasUserTouch = touchFlow('runas_user_touch', 'user', 'touched-use
export const runasSystemRead = readFlow('runas_system_read', 'system');
export const runasUserRead = readFlow('runas_user_read', 'user');

/**
* `<child>_via_parent` — start → subflow(child) → end, under `runAs: 'user'`:
* the non-elevated parent a member may start at the trigger door, which hands
* `noteId` to the elevated child. With `outputVariable`, the child's outputs
* land on the parent's `found` output (so a read child's own `found` arrives as
* `output.found.found` on the trigger response).
*/
function viaParent(child: Flow, withOutput: boolean): Flow {
return {
name: `${child.name}_via_parent`,
label: `${child.label} (via a user-mode parent)`,
type: 'autolaunched',
runAs: 'user',
variables: [
{ name: 'noteId', type: 'text', isInput: true },
...(withOutput ? [{ name: 'found', type: 'object', isOutput: true }] : []),
],
nodes: [
{ id: 'start', type: 'start', label: 'Start' },
{
id: 'call',
type: 'subflow',
label: 'Call the elevated child',
config: {
flowName: child.name,
input: { noteId: '{noteId}' },
...(withOutput ? { outputVariable: 'found' } : {}),
},
},
{ id: 'end', type: 'end', label: 'End' },
],
edges: [
{ id: 'e1', source: 'start', target: 'call' },
{ id: 'e2', source: 'call', target: 'end' },
],
};
}

export const runasSystemTouchViaParent = viaParent(runasSystemTouch, false);
export const runasSystemReadViaParent = viaParent(runasSystemRead, true);

/** A minimal, self-contained app config the dogfood harness can boot. */
export const runasFixtureStack = defineStack({
manifest: {
Expand All @@ -125,7 +174,14 @@ export const runasFixtureStack = defineStack({
description: 'Owner-isolated single-object app exercising flow.runAs identity enforcement.',
},
objects: [RunAsNote],
flows: [runasSystemTouch, runasUserTouch, runasSystemRead, runasUserRead],
flows: [
runasSystemTouch,
runasUserTouch,
runasSystemRead,
runasUserRead,
runasSystemTouchViaParent,
runasSystemReadViaParent,
],
});

/**
Expand Down
59 changes: 56 additions & 3 deletions packages/qa/dogfood/test/flow-runas.dogfood.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@
// different bug and must not pass here.
// Before the #1888 fix the user flows wrongly succeed (CRUD nodes passed no
// identity → security skipped) → this file is RED; after the fix → GREEN.
//
// The trigger door's own rule (the maintainer's ruling, letter B): a caller that
// is not the system principal may not start a `runAs: 'system'` flow of a
// self-triggered type — and both system flows here are `autolaunched` — through
// `POST /automation/:name/trigger`. So the elevation legs reach each system flow
// the way the platform keeps open for an elevated write: the member starts a
// `runAs: 'user'` parent whose `subflow` node calls it. The proof's subject is
// unchanged — what the CHILD's data nodes run as — and a separate case pins the
// door's half: the member's DIRECT start of either system flow is refused
// `403 PERMISSION_DENIED`, the note is untouched and no run is recorded.

import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { bootStack, type VerifyStack } from '@objectstack/verify';
Expand Down Expand Up @@ -134,6 +144,35 @@ describe('objectstack verify FLOW: runAs identity enforcement (#flow-runas)', ()
expect(failed?.status, `no failure entry for node '${failingNodeId}' in ${text}`).toBe('failure');
}

/** Run-log entries the engine holds for `flow` — the "nothing ran" witness. */
async function runCount(flow: string): Promise<number> {
const automation = stack.kernel.getService('automation') as { listRuns(name: string): Promise<unknown[]> };
return (await automation.listRuns(flow)).length;
}

/**
* Start a flow DIRECTLY as the restricted member and require the trigger door
* to refuse it: `403 PERMISSION_DENIED` in the ADR-0112 envelope, no inner
* `data`, and no run recorded for the flow — the door refuses before
* dispatch, so the engine is never asked.
*/
async function memberDirectTriggerExpectingDoorRefusal(flow: string, noteId: string) {
const runsBefore = await runCount(flow);
const res = await stack.apiAs(memberToken, 'POST', `/automation/${flow}/trigger`, { params: { noteId } });
const text = await res.clone().text();
expect(res.status, `direct trigger of ${flow} should be refused 403: ${res.status} ${text}`).toBe(403);
const body = (await res.json()) as {
success?: boolean;
data?: unknown;
error?: { code?: string; httpStatus?: number };
};
expect(body.success).toBe(false);
expect(body.error?.code, `expected PERMISSION_DENIED: ${text}`).toBe('PERMISSION_DENIED');
expect(body.error?.httpStatus).toBe(403);
expect(body.data).toBeUndefined();
expect(await runCount(flow), `a refused start of ${flow} still recorded a run`).toBe(runsBefore);
}

it('precondition: the automation service is wired and a flow is registered', async () => {
const res = await stack.apiAs(memberToken, 'GET', '/automation/runas_system_touch');
expect(res.status, `automation service not wired: ${res.status}`).toBe(200);
Expand All @@ -158,12 +197,22 @@ describe('objectstack verify FLOW: runAs identity enforcement (#flow-runas)', ()

it("runAs:'system' ELEVATES — member-triggered system flow WRITES a record the member cannot", async () => {
const id = await adminCreateNote('sys-touch');
const result = await memberTrigger('runas_system_touch', id);
// Through the user-mode parent: the parent itself is RLS-bound as the
// member, so the write landing is the elevated CHILD's doing.
const result = await memberTrigger('runas_system_touch_via_parent', id);
expect(result.success, `system flow run not successful: ${JSON.stringify(result)}`).toBe(true);
// The elevated run bypassed RLS and stamped the admin's note.
expect(await adminStatusOf(id)).toBe('touched-system');
});

it("the trigger door refuses the member a DIRECT start of either runAs:'system' flow — 403, nothing written, no run", async () => {
const id = await adminCreateNote('sys-direct');
await memberDirectTriggerExpectingDoorRefusal('runas_system_touch', id);
// Refused before dispatch: the elevated write never happened.
expect(await adminStatusOf(id)).toBe('new');
await memberDirectTriggerExpectingDoorRefusal('runas_system_read', id);
});

it("runAs:'user' DE-ELEVATES — member-triggered user flow is RLS-DENIED on the same record", async () => {
const id = await adminCreateNote('user-touch');
// The de-elevated run reaches the record layer as the MEMBER and the write
Expand All @@ -188,8 +237,12 @@ describe('objectstack verify FLOW: runAs identity enforcement (#flow-runas)', ()
it("runAs:'system' READS a record the member cannot; runAs:'user' cannot", async () => {
const id = await adminCreateNote('read-check');

const sys = await memberTrigger('runas_system_read', id);
expect(sys.output?.found, 'system flow could not read the record it should see (elevation broken)').toBeTruthy();
// Through the user-mode parent, whose `found` output carries the elevated
// child's outputs — so the child's own `found` is `output.found.found`.
const sys = await memberTrigger('runas_system_read_via_parent', id);
const sysFound = (sys.output?.found as { found?: { id?: unknown } } | undefined)?.found;
expect(sysFound, 'system flow could not read the record it should see (elevation broken)').toBeTruthy();
expect(sysFound?.id, 'the elevated read returned some row, not the admin note it was asked for').toBe(id);

const usr = await memberTrigger('runas_user_read', id);
const found = usr.output?.found;
Expand Down
Loading
Loading