diff --git a/.changeset/automation-run-read-permission-gate.md b/.changeset/automation-run-read-permission-gate.md new file mode 100644 index 0000000000..aae5b40fe5 --- /dev/null +++ b/.changeset/automation-run-read-permission-gate.md @@ -0,0 +1,82 @@ +--- +"@objectstack/runtime": minor +--- + +fix(runtime): `/automation` run-state reads require the `sys_automation_run` read grant (#7900) + +**⚠️ BEHAVIOUR NARROWING — pre-grant before you upgrade.** Operator tooling that +read automation run detail with bare authentication now needs read access to the +`sys_automation_run` object. See the migration note at the bottom. + +**What was open.** `GET /automation/:name/runs/:runId` answered with +`deps.success(run)` — the `ExecutionLogEntry` verbatim, no projection, no +redaction, no masking, on any field. The only gate on that path was the #5519 +anonymous baseline, applied to the whole `/automation` domain rather than per +route, so the sole question the surface asked was *"are you authenticated?"*. Any +authenticated caller who knew a run id read whatever that run's log entry held — +including, through the variables snapshot and through `output`, the triggering +record's fields, **with that record's own field-level security never applying**. +`GET /:name/runs` served the same entries a page at a time, gated the same way. + +The identical snapshot has always had a second, differently-gated door: +`sys_automation_run.variables_json` persists it for every paused run, and that +read goes through the system object's permissions. One platform, two answers, +depending on which door you knocked on. + +**What this does — converge the two doors** (maintainer ruling, 2026-08-12). Both +run-state reads now consult the same permission the `sys_automation_run` object +read answers with: `ISecurityService.explain({ object: 'sys_automation_run', +operation: 'read' })`, which runs the same permission-set resolution, the same +`PermissionEvaluator` and the same RLS compiler the enforcement middleware runs. +No new permission system, no new cross-package seam — the `security` slot was +already on `DomainHandlerDeps`. A caller without the grant gets **403 +`PERMISSION_DENIED`**, and the automation service is never consulted, so the +snapshot is not even loaded. A caller **with** the grant reads exactly what they +read before, byte for byte. + +**Not** per-field filtering of `variables` — explicitly rejected by the ruling, on +the measurement that the map's keys (`.`, `record`, `previous`, `$runId`, seeded +inputs) are not decidably record fields, so a per-field rule is one an +implementation can get quietly wrong. + +**The rest of the domain was audited against the same rule**, and the routes that +stay authenticated-only carry their reason in the source rather than in silence: +`GET /`, `GET /:name`, `GET /actions`, `GET /connectors` and `GET /_status` serve +flow-definition and registry data, not `sys_automation_run`-class data, so the +grant this ruling names says nothing about them and requiring it would invent a +second policy rather than converge one. `GET /:name/runs/:runId/screen` is the +interactive runner's refresh-safe re-fetch for the caller the flow paused *for*, +and its write sibling `resume` already answers on the engine's per-run +`resumeAuthority` axis; its residual disclosure (a screen's defaults are +interpolated against live flow variables) is filed separately rather than closed +by an operator grant that would refuse the end user. + +Three non-denials, each deliberate: a **system** context passes (the middleware's +own first bypass); a deployment with **no `plugin-security`** passes, because +there is no object-permission system for either door to consult and refusing +would put them in disagreement the other way; a **partial** security service that +omits `explain` degrades rather than throwing. An `explain` that throws is a +denial — an access-narrowing answer fails closed. + +--- + +### Migration + +Deployments upgrading to this release should **pre-grant before upgrading**. + +Any identity that reads automation run history or run detail over HTTP — +operator dashboards, monitoring pollers of `GET /automation/:name/runs?status=failed`, +support tooling that opens a run by id, scripted health checks — must now hold +**read on `sys_automation_run`** in one of its permission sets: + +```ts +permissions: [{ + name: 'automation_operator', + objects: { sys_automation_run: { allowRead: true } }, +}] +``` + +Nothing else changes for a caller that already holds it: the response body is +unchanged, including the full `variables` map. Service/system-context callers and +the engine's own internal paths are unaffected — neither goes through this seam. +Screen-flow end users are unaffected: the screen re-fetch is not gated. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 6f8f4835f5..94dd662756 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -684,6 +684,25 @@ GET /api/v1/automation/{flow}/runs/{runId} # one run GET /api/v1/automation/{flow}/runs/{runId}/screen # re-fetch a paused screen ``` + +A run carries the flow's variable snapshot and its `output` — which is to say +the triggering record's fields. So `GET /runs` and `GET /runs/{runId}` are gated +on the same grant the `sys_automation_run` object read answers with, not on +authentication alone; a caller without it gets `403 PERMISSION_DENIED`. Grant it +to the identities your operator tooling and monitoring run as: + +```ts +permissions: [{ + name: 'automation_operator', + objects: { sys_automation_run: { allowRead: true } }, +}] +``` + +The screen re-fetch is deliberately **not** gated this way — it serves the end +user the flow paused for, and its `resume` counterpart answers on the pause's +own `resumeAuthority` instead. + + Each run's `steps[]` records every executed node — including loop iterations, parallel branch bodies, and try/catch region steps — which the Studio flow designer surfaces, nested by iteration / branch / handler, in its **Runs** side @@ -1238,8 +1257,8 @@ curl -b cookies.txt -X POST \ | Endpoint | Purpose | |:---|:---| | `POST /api/v1/automation/:name/trigger` | Start a flow (canonical) | -| `GET /api/v1/automation/:name/runs` | List runs (`?limit`, `?cursor`, `?status` — narrow to one execution status; an undeclared value is refused `400 VALIDATION_FAILED`) | -| `GET /api/v1/automation/:name/runs/:runId` | One run's detail (404 `Execution not found`) | +| `GET /api/v1/automation/:name/runs` | List runs (`?limit`, `?cursor`, `?status` — narrow to one execution status; an undeclared value is refused `400 VALIDATION_FAILED`). Requires read on `sys_automation_run` — see [Observing runs](#observing-runs) | +| `GET /api/v1/automation/:name/runs/:runId` | One run's detail (404 `Execution not found`). Requires read on `sys_automation_run` — see [Observing runs](#observing-runs) | | `POST /api/v1/automation/:name/runs/:runId/resume` | Resume a paused run — body `{ inputs, output, branchLabel }` | | `GET /api/v1/automation/:name/runs/:runId/screen` | The pending screen of a screen-flow run | diff --git a/packages/runtime/src/domains/automation-run-read-permission-gate.test.ts b/packages/runtime/src/domains/automation-run-read-permission-gate.test.ts new file mode 100644 index 0000000000..618e29899b --- /dev/null +++ b/packages/runtime/src/domains/automation-run-read-permission-gate.test.ts @@ -0,0 +1,369 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7900 — the `/automation` run-state read surface converges on the + * `sys_automation_run` object-read grant. + * + * Maintainer ruling, 2026-08-12: *"the `/automation` read surface requires the + * same permission the `sys_automation_run` object read answers with; per-field + * filtering of the variables map is rejected as the mechanism."* + * + * What was measured before this gate: `GET /automation/:name/runs/:runId` + * answered `deps.success(run)` — the `ExecutionLogEntry` verbatim, no + * projection, no redaction, no masking — behind the #5519 anonymous baseline + * and nothing else. So the only question the surface asked was "are you + * authenticated?", and any authenticated caller who knew a run id read the + * triggering record's fields with that record's own FLS never applying. The + * SAME snapshot's second door (`sys_automation_run.variables_json`) has always + * gone through the system object's permissions — one platform, two answers. + * + * This file pins the four claims the convergence rests on: + * + * 1. **REFUSAL** — a caller the security service says may not read + * `sys_automation_run` is refused, with `code` AND `status` (ADR-0112), on + * BOTH run-state reads, and the automation service is never consulted. + * 2. **POSITIVE CONTROL** — a caller WITH the grant reads exactly what they + * read today, byte for byte. This is the pin that says the change narrowed + * the gate rather than broke the route; it deep-equals the same fixture + * `automation-run-detail-passthrough.test.ts` asserts on. + * 3. **ONE QUESTION** — the gate asks for `read` on `sys_automation_run` and + * forwards the caller's own execution context, i.e. it consults the grant + * the ruling names and not some second permission invented here. + * 4. **THE AUDIT** — the routes that stay authenticated-only stay + * authenticated-only, and ask the security service nothing. A future change + * to any of those verdicts has to come through this file. + * + * The three non-denials (system context, no security service, partial service) + * are pinned too: each is a decision recorded on `refuseUngrantedRunRead`, and + * an untested decision is a comment. + */ + +import { describe, it, expect, vi } from 'vitest'; + +import { HttpDispatcher } from '../http-dispatcher.js'; +import type { HttpProtocolContext } from '../http-dispatcher.js'; +import { AUTOMATION_RUN_OBJECT } from './automation.js'; + +/** + * A paused run as the engine records it since #7639 — the exposure in one + * object: `variables.record` is the triggering record's fields, and this + * surface handed the whole entry back verbatim. + */ +const PAUSED_RUN = { + id: 'run_7', + flowName: 'approval_flow', + status: 'paused', + steps: [{ nodeId: 'stage1', nodeType: 'approval', status: 'success' }], + variables: { + 'stage1.decision': { route: 'dual', note: null }, + record: { id: 'ord_1', amount: 90_000, margin_pct: 4.5 }, + $runId: 'run_7', + }, +} as const; + +/** One explain call, as the gate makes it. */ +interface ExplainCall { + request: { object: string; operation: string; userId?: string; recordId?: string }; + context: unknown; +} + +interface Harness { + dispatcher: HttpDispatcher; + getRun: ReturnType; + listRuns: ReturnType; + listFlows: ReturnType; + getFlowRuntimeStates: ReturnType; + getSuspendedScreen: ReturnType; + explainCalls: ExplainCall[]; +} + +/** + * Build a dispatcher over a stub automation service and a stub `security` slot. + * + * `security` is what the deployment's security posture is expressed as here: + * `'granting'` / `'refusing'` are a service that answers, `'throwing'` is one + * whose resolution fails, `'partial'` is an implementation that omits `explain` + * (the contract's feature-detection case), `'absent'` is a deployment with no + * `plugin-security` at all. + */ +function makeDispatcher( + security: 'granting' | 'refusing' | 'throwing' | 'partial' | 'absent', +): Harness { + const explainCalls: ExplainCall[] = []; + const getRun = vi.fn(async () => PAUSED_RUN as unknown); + const listRuns = vi.fn(async () => [PAUSED_RUN] as unknown[]); + const listFlows = vi.fn(async () => ['approval_flow']); + const getFlowRuntimeStates = vi.fn(() => [{ name: 'approval_flow', enabled: true, bound: true }]); + const getSuspendedScreen = vi.fn(async () => ({ nodeId: 'collect', fields: [] } as unknown)); + + const explain = async ( + request: ExplainCall['request'], + context: unknown, + ): Promise<{ allowed: boolean; object: string; operation: string }> => { + explainCalls.push({ request, context }); + if (security === 'throwing') throw new Error('permission subsystem unavailable'); + return { allowed: security === 'granting', object: request.object, operation: request.operation }; + }; + + const services: Record = { + automation: { + handlerReady: true, + getRun, + listRuns, + listFlows, + getFlowRuntimeStates, + getSuspendedScreen, + getFlow: async (name: string) => ({ name, nodes: [] }), + getActionDescriptors: () => [{ type: 'notify', source: 'builtin' }], + }, + }; + if (security === 'partial') { + // A security service that predates / omits `explain` — resolvable, but + // unable to answer. Feature detection must degrade, not throw. + services.security = { getReadableFields: async () => undefined }; + } else if (security !== 'absent') { + services.security = { explain }; + } + + const resolve = (name: string): unknown => services[name]; + const kernel = { + getService: resolve, + getServiceAsync: async (name: string) => resolve(name), + context: { getService: resolve }, + }; + return { + dispatcher: new HttpDispatcher(kernel as never), + getRun, + listRuns, + listFlows, + getFlowRuntimeStates, + getSuspendedScreen, + explainCalls, + }; +} + +/** An ordinary authenticated caller. */ +const USER_CTX = (): HttpProtocolContext => + ({ request: {}, executionContext: { userId: 'user_1', positions: ['sales_rep'] } } as HttpProtocolContext); + +/** A platform-internal caller — the middleware's own first bypass. */ +const SYSTEM_CTX = (): HttpProtocolContext => + ({ request: {}, executionContext: { userId: 'usr_system', isSystem: true } } as HttpProtocolContext); + +/** The run payload out of the success envelope, whatever the envelope's shape. */ +const payloadOf = (response: unknown): any => { + const r = response as any; + return r?.data ?? r?.body?.data ?? r; +}; + +/** The semantic error code, from wherever the envelope parks it. */ +const codeOf = (response: unknown): unknown => { + const r = response as any; + return r?.body?.error?.code ?? r?.body?.error?.details?.code; +}; + +describe('#7900 — /automation run-state reads require the sys_automation_run read grant', () => { + describe('refusal', () => { + it('refuses run-detail with PERMISSION_DENIED + 403, and never reads the run', async () => { + const h = makeDispatcher('refusing'); + const { response } = await h.dispatcher.handleAutomation( + 'approval_flow/runs/run_7', 'GET', undefined, USER_CTX(), undefined, + ); + + // ADR-0112: the refusal asserts BOTH halves. One is not enough — a + // 403 carrying a derived code, or a PERMISSION_DENIED on a 200, + // would each satisfy exactly half of the contract. + expect(codeOf(response)).toBe('PERMISSION_DENIED'); + expect((response as any).status).toBe(403); + + // The gate fires ahead of the service, so the snapshot is not even + // read — a refusal that fetched first would still have loaded the + // record's fields into the process serving the refused caller. + expect(h.getRun).not.toHaveBeenCalled(); + }); + + it('refuses the runs LIST on the same policy — it serves the same entries', async () => { + const h = makeDispatcher('refusing'); + const { response } = await h.dispatcher.handleAutomation( + 'approval_flow/runs', 'GET', undefined, USER_CTX(), undefined, + ); + + expect(codeOf(response)).toBe('PERMISSION_DENIED'); + expect((response as any).status).toBe(403); + expect(h.listRuns).not.toHaveBeenCalled(); + }); + + it('does not answer the caller\'s authorization topology in the refusal (#7450)', async () => { + const h = makeDispatcher('refusing'); + const { response } = await h.dispatcher.handleAutomation( + 'approval_flow/runs/run_7', 'GET', undefined, USER_CTX(), undefined, + ); + + const serialized = JSON.stringify((response as any).body); + expect(serialized).not.toContain('sales_rep'); + expect(serialized).not.toContain('user_1'); + }); + + it('fails CLOSED when the permission answer cannot be computed', async () => { + const h = makeDispatcher('throwing'); + const { response } = await h.dispatcher.handleAutomation( + 'approval_flow/runs/run_7', 'GET', undefined, USER_CTX(), undefined, + ); + + // An access-NARROWING answer that could not be resolved is a denial, + // the stance plugin-security itself takes on an unresolvable object + // posture (#3545). "Could not evaluate" must never read as "allowed". + expect(codeOf(response)).toBe('PERMISSION_DENIED'); + expect((response as any).status).toBe(403); + expect(h.getRun).not.toHaveBeenCalled(); + }); + }); + + describe('positive control — a caller WITH the grant reads exactly what they read today', () => { + it('serves run-detail byte-for-byte, snapshot included', async () => { + const h = makeDispatcher('granting'); + const { response } = await h.dispatcher.handleAutomation( + 'approval_flow/runs/run_7', 'GET', undefined, USER_CTX(), undefined, + ); + const run = payloadOf(response); + + // Deep-equal against the WHOLE fixture: this is the pin that proves + // a narrowing rather than a breakage. The ruling explicitly rejects + // per-field filtering of `variables`, so a granted caller must still + // receive the map untouched — nested objects, numbers and a null. + expect(run).toEqual(PAUSED_RUN); + expect(run.variables).toEqual(PAUSED_RUN.variables); + expect(run.variables.record).toEqual(PAUSED_RUN.variables.record); + expect(h.getRun).toHaveBeenCalledWith('run_7'); + }); + + it('serves the runs list unchanged', async () => { + const h = makeDispatcher('granting'); + const { response } = await h.dispatcher.handleAutomation( + 'approval_flow/runs', 'GET', undefined, USER_CTX(), undefined, + ); + + expect(payloadOf(response)).toEqual({ runs: [PAUSED_RUN], hasMore: false }); + expect(h.listRuns).toHaveBeenCalled(); + }); + }); + + describe('one question, asked of the grant the ruling names', () => { + it('asks for `read` on sys_automation_run, with the caller\'s own context', async () => { + const h = makeDispatcher('granting'); + await h.dispatcher.handleAutomation( + 'approval_flow/runs/run_7', 'GET', undefined, USER_CTX(), undefined, + ); + + expect(h.explainCalls).toHaveLength(1); + expect(h.explainCalls[0]!.request.object).toBe('sys_automation_run'); + expect(h.explainCalls[0]!.request.operation).toBe('read'); + // No `userId` on the request: explaining ANOTHER user is an + // administrative act plugin-security gates on `manage_users`. The + // gate asks about the CALLER, so it must not name a target. + expect(h.explainCalls[0]!.request.userId).toBeUndefined(); + expect(h.explainCalls[0]!.context).toMatchObject({ userId: 'user_1' }); + }); + + it('names the object the other door answers with', () => { + // The two doors are one policy only if they are pointed at the same + // object. `sys_automation_run.variables_json` is where the identical + // snapshot is persisted. + expect(AUTOMATION_RUN_OBJECT).toBe('sys_automation_run'); + }); + }); + + describe('the three non-denials, each a recorded decision', () => { + it('lets a SYSTEM context through without asking', async () => { + const h = makeDispatcher('refusing'); + const { response } = await h.dispatcher.handleAutomation( + 'approval_flow/runs/run_7', 'GET', undefined, SYSTEM_CTX(), undefined, + ); + + // The middleware's very first act is `if (isSystem) return next()`. + // A gate stricter than the object read is not convergence either. + expect(payloadOf(response)).toEqual(PAUSED_RUN); + expect(h.explainCalls).toHaveLength(0); + }); + + it('serves the read where no security service exists — both doors say "authenticated is enough"', async () => { + const h = makeDispatcher('absent'); + const { response } = await h.dispatcher.handleAutomation( + 'approval_flow/runs/run_7', 'GET', undefined, USER_CTX(), undefined, + ); + + // A deployment without plugin-security has no object-permission + // system at all, so `/data/sys_automation_run` is ungated too. + // Refusing here would put the doors in disagreement the OTHER way. + expect(payloadOf(response)).toEqual(PAUSED_RUN); + }); + + it('degrades on a security service that omits `explain` rather than throwing', async () => { + const h = makeDispatcher('partial'); + const { response } = await h.dispatcher.handleAutomation( + 'approval_flow/runs/run_7', 'GET', undefined, USER_CTX(), undefined, + ); + + // The contract mandates feature detection: a partial implementation + // degrades to the pre-gate behaviour, it does not 500. + expect((response as any).status).not.toBe(500); + expect(payloadOf(response)).toEqual(PAUSED_RUN); + }); + }); + + describe('the audit — routes that stay authenticated-only, and why', () => { + /** + * Each row is a route the audit examined and left on the #5519 + * anonymous baseline alone. The reason is on the route in + * `automation.ts`; what this table pins is that the verdict is a + * DECISION — changing any of these has to change this file too. + */ + const AUTHENTICATED_ONLY: Array<{ path: string; why: string }> = [ + { path: '', why: 'listFlows — flow names, not run state' }, + { path: 'approval_flow', why: 'getFlow — a flow definition, metadata-plane data' }, + { path: 'actions', why: 'getActionDescriptors — the deployment action catalog' }, + { path: '_status', why: 'getFlowRuntimeStates — per-flow enabled/bound state' }, + { path: 'approval_flow/runs/run_7/screen', why: 'the interactive runner\'s re-fetch' }, + ]; + + it.each(AUTHENTICATED_ONLY)('$path stays authenticated-only ($why)', async ({ path }) => { + const h = makeDispatcher('refusing'); + const { response } = await h.dispatcher.handleAutomation( + path, 'GET', undefined, USER_CTX(), undefined, + ); + + // Not refused… + expect((response as any).status).not.toBe(403); + // …and the run-state grant was never consulted for it, so no cost + // and no accidental coupling to a permission this route does not use. + expect(h.explainCalls).toHaveLength(0); + }); + + it('gates neither the trigger nor any other write — the ruling is about the READ surface', async () => { + const h = makeDispatcher('refusing'); + const { response } = await h.dispatcher.handleAutomation( + 'approval_flow/runs/run_7/resume', 'POST', { inputs: {} }, USER_CTX(), undefined, + ); + + // `resume` answers on the engine's per-run `resumeAuthority` axis + // (#3801 / #5561), which this card does not touch. + expect((response as any).status).not.toBe(403); + expect(h.explainCalls).toHaveLength(0); + }); + }); + + it('still refuses an ANONYMOUS caller at the #5519 floor, ahead of the new gate', async () => { + const h = makeDispatcher('granting'); + const { response } = await h.dispatcher.handleAutomation( + 'approval_flow/runs/run_7', 'GET', undefined, + { request: {}, executionContext: {} } as HttpProtocolContext, undefined, + ); + + // Authentication is still the first question, and it is still answered + // as 401/UNAUTHENTICATED — a narrowing must not accidentally re-label + // the anonymous floor as an authorization failure. + expect(codeOf(response)).toBe('UNAUTHENTICATED'); + expect((response as any).status).toBe(401); + expect(h.explainCalls).toHaveLength(0); + }); +}); diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index 84ec853a64..0fd3d5cee9 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -13,7 +13,7 @@ import { shouldDenyAnonymous, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE, } from '@objectstack/core'; import { CoreServiceName } from '@objectstack/spec/system'; -import type { IAutomationService } from '@objectstack/spec/contracts'; +import type { IAutomationService, ISecurityService } from '@objectstack/spec/contracts'; import { isServiceServeable } from '../service-serveable.js'; import { validationFailure } from '../validation-failure.js'; import { ExecutionStatus } from '@objectstack/spec/automation'; @@ -107,6 +107,129 @@ export function createAutomationDomain(deps: DomainHandlerDeps): DomainRoute { }; } +/** + * [#7900] The system object whose READ grant governs automation RUN STATE — + * the durable row `service-automation` writes for every suspended run, whose + * `variables_json` column holds the very snapshot `GET /:name/runs/:runId` + * hands back (`sys-automation-run.object.ts`). + * + * It is named here because the two are ONE policy with two doors, not two + * policies: reading the row through `/data/sys_automation_run` has always + * answered with this object's permissions, while the `/automation` door asked + * only "are you authenticated?". + */ +export const AUTOMATION_RUN_OBJECT = 'sys_automation_run'; + +/** [#7900] Refusal vocabulary for the run-state read gate (ADR-0112: code AND status). */ +const RUN_READ_DENY_STATUS = 403; +const RUN_READ_DENY_CODE = 'PERMISSION_DENIED'; +const RUN_READ_DENY_MESSAGE = + `Reading automation run state requires read access to '${AUTOMATION_RUN_OBJECT}'.`; + +/** + * [#7900] Which `/automation` GET routes serve `sys_automation_run`-class data. + * + * Declared as ONE predicate rather than a check per branch on purpose — the + * whole point of the ruling is that this domain gets one policy, and a policy + * spelled out at three call sites is three policies that happen to agree today. + * `parts` is the flow-scoped path split (`parts[0]` is the flow name). + * + * `/:name/runs` → listRuns, an `ExecutionLogEntry[]` + * `/:name/runs/:runId` → getRun, an `ExecutionLogEntry` served verbatim + * + * `/:name/runs/:runId/screen` is deliberately NOT here; the audit reason is on + * the route itself, below. + */ +function isRunStateRead(parts: string[], method: string): boolean { + if (method !== 'GET') return false; + if (parts.length < 2 || parts[1] !== 'runs') return false; + // `/:name/runs` (listRuns) and `/:name/runs/:runId` (getRun) — nothing deeper. + return parts.length === 2 || parts.length === 3; +} + +/** + * [#7900] Ask the SAME question the other door answers with: may this caller + * READ `sys_automation_run`? + * + * Returns a refusal result when the answer is no, `undefined` when the read may + * proceed — so the caller reads as a guard clause and no route can accidentally + * consume a "denied" as a value. + * + * ## Why `explain`, and why nothing new was built + * + * `ISecurityService` is the contract for exactly this: "the query surface that + * lets code OUTSIDE the ObjectQL engine middleware ask the same questions the + * middleware answers when it enforces access", with a standing instruction that + * a consumer re-deriving any of these answers locally will drift. `explain` runs + * the same permission-set resolution, the same `PermissionEvaluator` and the + * same RLS compiler the middleware runs — `allowed` is `!capsDeny && + * crudAllowed && !denyAll && !delegatorMissing` over that shared machinery — so + * this gate cannot answer differently from the `/data` door by drifting. The + * slot is already on `DomainHandlerDeps` (`domains/meta.ts` resolves it the same + * way for ADR-0106 masking), so no new cross-package seam exists to invent. + * + * ⛔ It is deliberately NOT a per-field filter of the run's `variables` map — + * rejected by the ruling, on the card's own measurement that the map's keys + * (`.`, `record`, `previous`, `$runId`, seeded inputs) are not decidably + * record fields. + * + * ## The three non-denials, each of which is a decision + * + * 1. **System context passes.** The middleware's very first act is + * `if (opCtx.context?.isSystem) return next()`. A gate that refused what the + * object read admits would not be convergence. + * 2. **No security service ⇒ no grant to require.** In a deployment without + * `plugin-security` there is no object-permission system at all, so + * `/data/sys_automation_run` is itself ungated: "authenticated is enough" is + * what BOTH doors answer, and refusing here would make them disagree in the + * other direction. The contract mandates this tolerance ("Consumers MUST + * tolerate absence"). Same for a partial implementation that omits `explain`. + * 3. **An `explain` THROW is a denial, not a pass.** This is an + * access-narrowing answer, so it fails CLOSED — the stance `plugin-security` + * itself takes when an object's posture cannot be resolved (#3545). + * + * ## The one place this is STRICTER than the door it converges on + * + * The middleware skips its CRUD gate entirely for an authenticated caller whose + * permission-set resolution comes back EMPTY (`if (permissionSets.length > 0)`), + * while `explain` runs `checkObjectPermission` over that empty list and gets + * `false`. ADR-0090 D5's additive baseline plus the post-resolution fallback + * make an empty resolution reachable only on a deployment that configures NO + * baseline permission set at all — and on that deployment this surface refuses + * where `/data` falls open. Left as-is deliberately: the divergence is in the + * closed direction on the door this card was filed about, and closing it the + * other way would mean re-deriving the middleware's own empty-set rule here, + * which is the drift `ISecurityService` exists to prevent. + */ +async function refuseUngrantedRunRead( + deps: DomainHandlerDeps, + context: HttpProtocolContext, +): Promise { + const ec = context?.executionContext; + if (ec?.isSystem === true) return undefined; + + const security = await deps.resolveService(context, 'security').catch(() => undefined) as + Partial | undefined; + if (!security || typeof security.explain !== 'function') return undefined; + + let allowed = false; + try { + const decision = await security.explain({ object: AUTOMATION_RUN_OBJECT, operation: 'read' }, ec); + allowed = decision?.allowed === true; + } catch { + allowed = false; + } + if (allowed) return undefined; + + // The refusal names the GRANT it wants and nothing about the caller — no + // positions, no permission-set names (#7450: a denial must not answer the + // caller's authorization topology). + return { + handled: true, + response: deps.error(RUN_READ_DENY_MESSAGE, RUN_READ_DENY_STATUS, { code: RUN_READ_DENY_CODE }), + }; +} + /** * Handles Automation requests * path: sub-path after /automation/ @@ -125,7 +248,9 @@ export function createAutomationDomain(deps: DomainHandlerDeps): DomainRoute { * POST /:name/toggle → toggleFlow (unknown name → 404, #7535) * GET /:name/runs → listRuns (query: limit, cursor — validated, #7300; * status — validated AND honoured, #7359) + * ⚑ run-state read — `sys_automation_run` grant (#7900) * GET /:name/runs/:runId → getRun + * ⚑ run-state read — `sys_automation_run` grant (#7900) * POST /:name/runs/:runId/resume → resume a paused run (screen input / ADR-0019) * GET /:name/runs/:runId/screen → the screen a paused run awaits */ @@ -160,6 +285,34 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str }; } } + const m = method.toUpperCase(); + const parts = path.replace(/^\/+/, '').split('/').filter(Boolean); + + // [#7900] RUN-STATE READ GATE — the maintainer ruling of 2026-08-12: the + // `/automation` read surface requires the same permission the + // `sys_automation_run` object read answers with. One policy, two doors, one + // answer. + // + // What it closes, measured: `GET /:name/runs/:runId` answered with + // `deps.success(run)` — the `ExecutionLogEntry` verbatim, no projection, no + // redaction, no masking — so any AUTHENTICATED caller who knew a run id read + // the triggering record's fields with that record's own FLS never applying. + // `listRuns` serves the same entries a page at a time and was gated the same + // (i.e. not at all). + // + // Placed with the #5519 anonymous floor and AHEAD of the service probe below + // for that gate's own reason, read one authorization tier up: which + // permission a route requires must not vary with which automation service a + // deployment happens to mount, and a 501-vs-403 should not be the thing that + // tells an ungranted caller whether automation is mounted here. + // + // Which routes: `isRunStateRead` above — deliberately one predicate, so the + // domain's policy cannot drift route by route the way the finding described. + if (isRunStateRead(parts, m)) { + const refusal = await refuseUngrantedRunRead(deps, context); + if (refusal) return refusal; + } + const automationService = await deps.getService(context, CoreServiceName.enum.automation); // [#4058] Empty slot — or a slot filled by a self-declared non-handler // (`handlerReady: false`, ADR-0076 D12), which is the same amount of @@ -173,9 +326,6 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // this request") described neither half truthfully. See ./unavailable.ts. if (!isServiceServeable(automationService)) return capabilityUnavailable(deps, 'automation'); - const m = method.toUpperCase(); - const parts = path.replace(/^\/+/, '').split('/').filter(Boolean); - // Legacy: POST /automation/trigger/:name — the shape // `client.automation.trigger()` calls. Same handling as // `POST /:name/trigger` below: one context builder, one service method. @@ -196,6 +346,18 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str } // GET / → listFlows + // + // [#7900 AUDIT — stays authenticated-only, with a reason] Together with + // `GET /:name`, `GET /actions`, `GET /connectors` and `GET /_status`, this + // serves FLOW-DEFINITION and REGISTRY data: names, definitions, the + // deployment's action/connector catalogs, per-flow enabled/bound state. None + // of it is `sys_automation_run`-class data — no run, no trigger record, no + // variable snapshot — so the grant the ruling names says nothing about it, + // and requiring it here would not be convergence but a SECOND policy + // invented for a different data class, which is precisely what the ruling + // forbids. Flow definitions are metadata and are governed on the metadata + // plane (`/meta`, ADR-0106); if their read posture should narrow, that is a + // metadata-plane decision and belongs to its own card. if (parts.length === 0 && m === 'GET') { if (typeof automationService.listFlows === 'function') { const names = await automationService.listFlows(); @@ -477,6 +639,32 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // GET /:name/runs/:runId/screen → the screen a paused run awaits // (refresh-safe re-fetch for the UI flow-runner). + // + // [#7900 AUDIT — stays authenticated-only, with a reason] This is the + // one run-scoped read the `sys_automation_run` grant is NOT applied to, + // and the omission is a decision rather than an oversight, so it is + // recorded here rather than only in the PR: + // + // - It is the END USER's surface, not the operator's. The pause exists + // because the flow is asking THIS caller to fill a form in; the + // trigger response already handed them the same `screen` inline, and + // this route exists so a browser refresh does not lose it. Requiring + // an operator grant here would refuse the screen to the very person + // the flow paused for — a breakage, not the narrowing the ruling + // prices in ("operator tooling … now needs the grant"). + // - Its WRITE sibling one route up already answers on a different + // axis: `resume` is gated in the engine by the suspension's declared + // `resumeAuthority` (#3801 / #5561) — a per-run authority model, not + // an object grant. Read and write on the same pause answering to two + // unrelated permissions would be the incoherence this card is about. + // + // The residual is real and is NOT claimed closed: a `ScreenSpec` carries + // `defaults` / `defaultValue` interpolated against the live flow + // variables (`builtin/screen-nodes.ts`), so an authenticated caller who + // knows a run id can still read record-derived values through this door. + // Closing it wants the per-run authority read gate the resume path + // already has, which is a different mechanism from this card's ruling — + // filed as its own issue and linked from the PR. if (parts[1] === 'runs' && parts[2] && parts[3] === 'screen' && m === 'GET') { if (typeof automationService.getSuspendedScreen === 'function') { const screen = await automationService.getSuspendedScreen(parts[2]);