Skip to content

fix(runtime): gate the paused-run screen read to the run's trigger identity (#7968) - #8226

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-7968-screen-route-gating
Aug 12, 2026
Merged

fix(runtime): gate the paused-run screen read to the run's trigger identity (#7968)#8226
hotlong merged 2 commits into
mainfrom
claude/issue-7968-screen-route-gating

Conversation

@hotlong

Copy link
Copy Markdown
Contributor

Fixes#7968

Implements the binding maintainer ruling of 2026-08-12 (comment 5266069853, Option B): GET /automation/:name/runs/:runId/screen is gated to the run's own trigger identity (ExecutionLogEntry.trigger.userId) OR a holder of the sys_automation_run read grant (operator override).

The disclosure, reproduced before it was gated

A real screen flow was run through the real engine (registerScreenNodes, a screen node whose field defaults interpolate {record.email} / {record.phone} / {record.salary_band}, triggered on a crm_lead record). The persisted ScreenSpec that getSuspendedScreen returns:

{ "nodeId": "collect",
"title": "Confirm Acme Health",
"description": "Contact for ceo@acme-health.example",
"fields": [ { "name": "email", "defaultValue": "ceo@acme-health.example" },
{ "name": "phone", "defaultValue": "+1-555-0100" },
{ "name": "salary", "defaultValue": "L7 / 285000 USD" } ] }

Fed through the real dispatcher, a caller with valid auth, no relationship to the run, and a security service explicitly refusing them the sys_automation_run read grant received it under 200 { success: true } — the whole spec, every value. After the gate, the identical request answers:

{ "success": false,
"error": { "code": "PERMISSION_DENIED",
"message": "Reading a paused run's screen requires being the identity that triggered the run, or read access to 'sys_automation_run'.",
"httpStatus": 403 } }

That observed payload is the fixture in automation-screen-read-gate.test.ts, so the pins run against the measured disclosure rather than a plausible stand-in.

Why the obvious gate is the wrong one

Requiring the sys_automation_run grant alone — the #7900 mechanism the sibling run-state reads converged on — would refuse the screen to the very person the flow paused for. That is why #7900's audit left this route out of its convergence, and it is why the ruling needed a second mechanism rather than an application of the first. The grant is therefore the override half, never the whole question, and the over-block direction is pinned as hard as the under-block one.

mayReadRunState is now one shared predicate asking the grant question for both gates, so the two doors cannot drift into two policies that merely agree today.

Acceptance, and how each case is asserted

Ruling's acceptanceAssertion
stranger with valid auth + run id ⇒ deniedcode === 'PERMISSION_DENIED'andstatus === 403 (full ADR-0112 envelope, never status >= 400 — this route's refusal and its not-found answer are one status apart), plus each of the four record-derived strings asserted absent from the body, plus data.screen undefined
triggering user ⇒ screenserved under a security service refusing them the grant, deep-equal to the whole captured spec, fields length 3, every record-derived value present — not merely a 200
holder of sys_automation_run read ⇒ screena caller who did not trigger the run, served whole under the same assertion, with explain verified to ask read on sys_automation_run with the caller's own context and no target userId

Also pinned: a run whose trigger carries nouserId refuses rather than matching on absence (two undefineds); a getRun throw is unresolved identity, not a match; the end user is still served while the permission subsystem is down (only the override half fails closed) while the same outage still refuses the stranger.

Not-found behaviour is untouched, deliberately

The gate runs aftergetSuspendedScreen, so a run with no pending screen still answers 404 No pending screen for run for every caller, and the permission question is not even asked. Gating first would mean failing closed on an unresolvable run and converting today's 404 into a 403 for everyone, honest typos included. The consequence is stated on the gate rather than hidden: a stranger can still tell a paused run id (403) from an unknown one (404) — an existence oracle over run ids, strictly narrower than the record values it replaces, and closing it means answering 404 to the refused caller, which is a different design and not what was ruled. Both rows are pinned so a future change to that distinction is deliberate.

Reverse verification

Gate reverted (automation.ts back to origin/main, tests unchanged) — predicted before running, matched exactly: 9 red, 12 green. Reds are the denial pins and the "gate was consulted" pins, e.g.

AssertionError: expected undefined to be 'PERMISSION_DENIED' // Object.is equality
AssertionError: expected '{"success":true,"data":{"runId":"run_…' not to contain 'Acme Health'
AssertionError: expected [] to have a length of 1 but got +0

The 12 that stayed green are the ones that pin behaviour the gate deliberately leaves unchanged, and they should: the three admit cases (an ungated route serves everyone, so an over-block pin cannot see a missing gate — only a wrong one), every 404/501 row, the four non-denials, the 401 anonymous floor, and the resume-scope case.

Probe — the rejected mechanism, twice. Gating on the sys_automation_run grant alone: every denial pin stays green while the over-block guards red with expected 403 to be 200 on "serves it whole to the identity the flow paused for, with NO grant at all". Gating on an object-read grant over the flow's own record object (trigger.object) reds the same guards plus expected 'crm_lead' to be 'sys_automation_run'. Both wrong gates are caught only by the over-block direction — which is exactly what makes this ruling non-obvious. Restored after each probe; git diff HEAD empty.

Scope

Option A — a per-run authority read gate derived from the suspension's own resumeAuthority / assignee state (#3801 / #5561) — stays the recorded coherent end state and ADR-0019-class design work. Not built here. B does not preclude it: both refuse the same stranger and admit the same end user.

Unchanged: which runs exist; resume's own authority checks; the /runs list route (#8054); the POST/PUT registration paths (#8055 / #8123).

One pre-existing assertion moved rather than broke: http-dispatcher.test.ts's routing claim ("screen must not be swallowed by the getRun route below it") asserted itself as "getRun was never called". The gate reads the run to resolve its trigger identity, so that proxy stopped tracking the claim; it is now asserted on the answer — the caller gets the { runId, screen } envelope and not the ExecutionLogEntry shape — which pins the routing claim more directly than before. The #7900 audit table's screen row is retired with a note pointing at its new file, as that table's own rule requires.

Verification

  • pnpm --filter @objectstack/runtime test148 files, 2293 tests, all pass (21 new)
  • pnpm --filter @objectstack/runtime typecheck — clean
  • pnpm check:type-check-debt — OK, "none above its recorded number"; @objectstack/runtime measured exactly at its recorded 227, not raised
  • pnpm check:route-envelope — green including --self-test, no ratchet change
  • node scripts/check-nul-bytes.mjs — OK; targeted control-byte self-scan clean

Generated by Claude Code

…entity (#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
… 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
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 9:23pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/runtime.

20 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx(via packages/runtime)
  • content/docs/api/index.mdx(via @objectstack/runtime)
  • content/docs/api/wire-format.mdx(via @objectstack/runtime)
  • content/docs/automation/hook-bodies.mdx(via @objectstack/runtime)
  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/runtime)
  • content/docs/concepts/north-star.mdx(via packages/runtime)
  • content/docs/data-modeling/drivers.mdx(via @objectstack/runtime)
  • content/docs/deployment/index.mdx(via @objectstack/runtime)
  • content/docs/deployment/production-readiness.mdx(via @objectstack/runtime)
  • content/docs/deployment/single-project-mode.mdx(via @objectstack/runtime)
  • content/docs/deployment/vercel.mdx(via @objectstack/runtime)
  • content/docs/getting-started/your-first-project.mdx(via @objectstack/runtime)
  • content/docs/kernel/cluster.mdx(via @objectstack/runtime)
  • content/docs/permissions/authentication.mdx(via @objectstack/runtime)
  • content/docs/permissions/authorization.mdx(via packages/runtime)
  • content/docs/permissions/system-context.mdx(via packages/runtime)
  • content/docs/plugins/packages.mdx(via @objectstack/runtime)
  • content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/runtime)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/runtime)
  • content/docs/protocol/kernel/lifecycle.mdx(via @objectstack/runtime)

2 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/runtime)
  • content/docs/releases/v17.mdx(via @objectstack/runtime)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 12, 2026
@hotlongClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — domain:cli seat (#6024): accepted. Enqueueing once CI is green.

The probe returned the result that justifies the ruling

I made this mandatory because I could not otherwise tell whether the pins tested the thing that makes Option B non-obvious. You ran it twice, and the answer is the one that matters:

Gating on the sys_automation_run grant alone: every denial pin stays green while the over-block guards red with expected 403 to be 200 on "serves it whole to the identity the flow paused for, with NO grant at all".

So the wrong gate — the obvious one, the #7900 mechanism — passes every under-block assertion. A suite that only tested "the stranger is denied" would have certified an implementation that locks out the very person the flow paused for. The second probe (object-read on trigger.object) reds the same guards plus expected 'crm_lead' to be 'sys_automation_run'.

Both wrong gates are caught only by the over-block direction.

That sentence is the review. It is also why the reverse verification alone was insufficient, and you named that too: "an ungated route serves everyone, so an over-block pin cannot see a missing gate — only a wrong one." The revert and the probe test different failure modes, and neither substitutes for the other.

The disclosure is the fixture

A real screen flow through the real engine, {record.email} / {record.phone} / {record.salary_band} interpolated into defaults, and the verbatim payload — ceo@acme-health.example, +1-555-0100, L7 / 285000 USD — reaching a stranger under 200 { success: true }, with the security service explicitly refusing them the grant. Then: "that observed payload is the fixture."

Reproducing before gating was step 1 of the brief; making the measured payload the fixture rather than a plausible stand-in was not, and it is the difference between pinning the disclosure and pinning an idea of it.

The residual exposure is named instead of glossed

a stranger can still tell a paused run id (403) from an unknown one (404) — an existence oracle over run ids, strictly narrower than the record values it replaces, and closing it means answering 404 to the refused caller, which is a different design and not what was ruled. Both rows are pinned so a future change to that distinction is deliberate.

That is the right handling: the gate runs aftergetSuspendedScreen so an honest typo still gets its 404 and the permission question is never asked; the narrowing that buys is stated with its cost; and both sides are pinned so nobody later "tidies" the distinction without meaning to. Same discipline #8013 needed for its nonexistent-name row, arrived at here unprompted.

Three edges I did not ask for, all real

  • A run whose trigger carries no userId refuses rather than matching on absence.undefined === undefined is a live authorization bug class and it would have passed every acceptance case in the ruling.
  • A getRun throw is unresolved identity, not a match.
  • The end user is still served while the permission subsystem is down; only the override half fails closed — and the same outage still refuses the stranger. That is a considered availability/security split, not an accident, and it is pinned in both directions.

mayReadRunState as one shared predicate across both gates is the same lesson #8123 landed: two doors that merely agree today are one edit from disagreeing.

The moved assertion is the subtle part

http-dispatcher.test.ts asserted its routing claim ("screen must not be swallowed by the getRun route below it") via a proxy — "getRun was never called". The gate legitimately reads the run, so the proxy stopped tracking the claim while the claim itself stayed true. Re-asserting it on the answer (the caller receives the { runId, screen } envelope, not the ExecutionLogEntry shape) pins it more directly than before.

That is the correct resolution of a case that looks like "the fix broke a test": the test's subject was fine, its instrument was coupled to an implementation detail. Deleting it would have lost the routing claim; leaving it would have blocked a correct fix.

Scope held: Option A explicitly not built; /runs (#8054), resume's own authority, and the registration paths (#8055 / #8123) untouched; the #7900 audit table's screen row retired with a pointer, per that table's own rule.

Not raised: @objectstack/runtime measured exactly at 227. 148 files / 2293 tests, 21 new.

Flipping ready and enabling auto-merge once CI converges — both steps. This is the last dispatchable card in the domain:cli queue.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review August 12, 2026 21:41
@hotlong
hotlong added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit b16dcb4Aug 12, 2026
26 checks passed
@hotlong
hotlong deleted the claude/issue-7968-screen-route-gating branch August 12, 2026 21:57
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: GET /automation/:name/runs/:runId/screen still discloses record-derived values to any authenticated caller who knows a run id

2 participants

@hotlong@claude