Skip to content

Commit b16dcb4

Browse files
hotlongclaude
andauthored
fix(runtime): gate the paused-run screen read to the run's trigger identity (#7968) (#8226)
* fix(runtime): gate the paused-run screen read to the run's trigger identity (#7968) `GET /automation/:name/runs/:runId/screen` served the paused run's ScreenSpec to any authenticated caller who knew a run id. A screen node's `defaults` and per-field `defaultValue` are interpolated against the live flow variables at suspend time, so a flow prefilling from its triggering record persists those values into the spec this route hands back — measured on a real screen flow over a `crm_lead` record: company in the title, email in the description, and email, phone and salary band as field defaults, answered 200 to a stranger explicitly refused the `sys_automation_run` read grant. Maintainer ruling 2026-08-12 (Option B): the route now requires the run's own trigger identity (`ExecutionLogEntry.trigger.userId`) OR read access to `sys_automation_run` as an operator override. The object grant ALONE is deliberately not the gate — it would refuse the screen to the very person the flow paused for, which is why #7900 audited this route out of its convergence. So the grant is the override half, and the over-block direction is pinned as hard as the under-block one. The `sys_automation_run` question is now one predicate (`mayReadRunState`) shared by both gates rather than a second copy of the resolution / feature-detection / fail-closed logic. Unchanged: the 404 for a run with no pending screen (the gate runs after the lookup, so every not-found path is byte-identical for every caller), the 501, the 401 anonymous floor, `resume`'s own authority checks, and which runs exist. Option A — the per-run `resumeAuthority` read gate — stays the recorded direction and is out of scope here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V * test(runtime): assert the screen route's routing claim on its answer, not on getRun being unused (#7968) `should get the pending screen via GET /:name/runs/:runId/screen` claimed that the screen path is not swallowed by the `/:name/runs/:runId` branch below it, and asserted it as "getRun was never called". The #7968 gate reads the run to resolve its trigger identity, so that proxy no longer tracks the claim. Asserted on the answer instead: the caller gets the screen envelope (`{ runId, screen }`) and not the `ExecutionLogEntry` the run-detail branch serves verbatim — the mock's entry is `{ id, status }`, so the two are distinguishable by shape. The routing claim is now pinned more directly than before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c4624f0 commit b16dcb4

5 files changed

Lines changed: 742 additions & 40 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): the paused-run screen read is gated to the run's trigger identity, or the `sys_automation_run` grant (#7968)
6+
7+
`GET /api/v1/automation/:name/runs/:runId/screen` answered **any authenticated
8+
caller who knew a run id** with the paused run's `ScreenSpec` — and that spec is
9+
not inert with respect to record data. A screen node's `defaults` and per-field
10+
`defaultValue` are interpolated against the live flow variables at suspend time,
11+
so a flow that prefills from its triggering record persists those values into the
12+
spec this route serves.
13+
14+
Measured on a real screen flow over a `crm_lead` record: a caller with valid
15+
auth, no relationship to the run, and explicitly refused the `sys_automation_run`
16+
read grant received `200` with the lead's company in the screen title, its email
17+
address in the description, and its email, phone and salary band as three field
18+
defaults. Reaching it needed only a session plus a leaked or guessed run id.
19+
20+
The route now requires **the identity that triggered the run**
21+
(`ExecutionLogEntry.trigger.userId`) **OR** read access to `sys_automation_run`
22+
as an operator override. A refused caller gets `403 PERMISSION_DENIED`.
23+
24+
**Why not the object grant on its own** — the mechanism the sibling run-state
25+
reads converged on in #7900: it would refuse the screen to the very person the
26+
flow paused for. The pause exists because the flow is asking *that* caller to
27+
fill a form in, so the grant is the override half here, never the whole question.
28+
Operator tooling that already holds the `sys_automation_run` read grant is
29+
unaffected, and so is the end user — including while the permission subsystem is
30+
unreachable, since only the override half fails closed.
31+
32+
**Unchanged**: which runs exist; `resume`'s own per-run `resumeAuthority` checks
33+
(#3801 / #5561); the `404 No pending screen for run` answer, which still comes
34+
back for an unknown or non-paused run id, for every caller, ahead of the gate; the
35+
`501` a deployment without screen lookup returns; and the `401` anonymous floor.
36+
37+
A deployment with no `plugin-security` (no object-permission system at all, so
38+
`/data/sys_automation_run` is itself ungated) keeps answering as before.

packages/runtime/src/domains/automation-run-read-permission-gate.test.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@
3131
* the ruling names and not some second permission invented here.
3232
* 4. **THE AUDIT** — the routes that stay authenticated-only stay
3333
* authenticated-only, and ask the security service nothing. A future change
34-
* to any of those verdicts has to come through this file.
34+
* to any of those verdicts has to come through this file. (#7968 is that
35+
* change, for one row: the paused-run `screen` read left this table when
36+
* the maintainer gated it on the run's trigger identity instead — see the
37+
* note in the table.)
3538
*
3639
* The three non-denials (system context, no security service, partial service)
3740
* are pinned too: each is a decision recorded on `refuseUngrantedRunRead`, and
@@ -323,7 +326,16 @@ describe('#7900 — /automation run-state reads require the sys_automation_run r
323326
{ path: 'approval_flow', why: 'getFlow — a flow definition, metadata-plane data' },
324327
{ path: 'actions', why: 'getActionDescriptors — the deployment action catalog' },
325328
{ path: '_status', why: 'getFlowRuntimeStates — per-flow enabled/bound state' },
326-
{ path: 'approval_flow/runs/run_7/screen', why: 'the interactive runner\'s re-fetch' },
329+
// [#7968] `approval_flow/runs/run_7/screen` USED to be this table's
330+
// fifth row. The audit's reason for leaving it here was right — the
331+
// grant alone would refuse the end user the flow paused for — but
332+
// "no grant" was not the same as "no gate": the route disclosed
333+
// record-derived screen defaults to any authenticated caller with a
334+
// run id. The 2026-08-12 ruling gates it on the run's own trigger
335+
// identity, with this grant as an operator override, so it is no
336+
// longer authenticated-only and no longer answers `explainCalls ===
337+
// 0` for a caller who is not the trigger identity. Its own file
338+
// owns it now: `automation-screen-read-gate.test.ts`.
327339
];
328340

329341
it.each(AUTHENTICATED_ONLY)('$path stays authenticated-only ($why)', async ({ path }) => {

0 commit comments

Comments
 (0)