fix(runtime): bind action-body ctx.api/ctx.engine to a real execution context (#3914) - #3922
Merged
Conversation
… context (#3914) An action body's `ctx.api` was never bound. `buildSandboxApi` fell through its 3-way fallback — no `actionCtx.api`, and the raw ObjectQL engine has no `.object()` (that lives on `ScopedContext`, reachable only via `engine.createContext()`, which the action path never called) — and landed on a repo facade that proxied every call to the engine with NO `context`. `ctx.engine` had the identical hole. Context-less is not "trusted", it is identity-less, and identity-less is strictly worse than either coherent posture: plugin-sharing's write gate short-circuits on `!context.userId` and its bypass needs `context.isSystem`, so every owner-scoped write from an action body died `FORBIDDEN` — as the built-in admin — while the `[action-audit]` line on the same request announced RLS-bypassing TRUSTED execution. Objects with a `public` sharing model, no owner field, or a bypass listing passed the gate early, which is why only some actions broke and it read as object-dependent flakiness. Both dispatch paths (REST `/actions/:object/:action` and MCP `run_action`) now bind `ctx.api` to `engine.createContext(...)` and thread the same envelope through `ctx.engine`, matching what hook bodies already get from `buildHookApi`. The envelope is the caller's ExecutionContext elevated with `isSystem: true`: elevation is the posture #2849 documents and gates for at invoke time, and spreading the caller's fields first keeps writes attributable (`created_by`/`updated_by`), org-scoped (`tenantId`), and joined to an open transaction — rather than the unattributable, org-less rows a bare `{ isSystem: true }` would write. The audit line's wording now describes what the body actually gets. The sandbox's last-resort repo facade is elevated from the same envelope via `ctx.executionContext`, so a host that predates `createContext` is not left proxying context-less. Regression tests use a sharing-shaped engine stub that denies a context-less write the way the real gate does; reverting the wiring reproduces the reported `FORBIDDEN: insufficient privileges to update crm_case`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TufbW1LBcSwpta4gfX7Uso
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 18 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…ntext fix (#3914) The Check Changeset gate requires every PR to add one; a runtime behavior fix this visible warrants a real release note rather than an empty declaration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TufbW1LBcSwpta4gfX7Uso
#3919 (REST action-type dispatch, #3915) landed on main and touched the same handler. Resolved `packages/runtime/src/domains/actions.ts` to keep both: - main's type dispatch (flow → automation service, url/modal/form/api → 400) and its relocation of the `[action-audit]` line into the try block, after the flow branch — a flow is not trusted-elevated, so it must not print it; - this branch's #3914 wiring — `ctx.api`, `ctx.executionContext`, and the shared context-carrying `buildActionEngineFacade` on the action context — plus the corrected audit wording, now applied to main's relocated line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TufbW1LBcSwpta4gfX7Uso
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3914.
The defect
An action body's
ctx.apiwas never bound.buildSandboxApiwalked its whole 3-way fallback — noactionCtx.api, and the rawObjectQLengine has no.object()(that lives onScopedContext, reachable only viaengine.createContext(), which the action path never called) — and landed on a repo facade that proxied every call to the engine with nocontext.ctx.engine(buildActionEngineFacade) had the identical hole.Context-less is not "trusted", it is identity-less, and identity-less is strictly worse than either coherent posture:
canEditshort-circuits on!context.userId— there is no user to own anything;context.isSystem— which was never set.So every owner-scoped write from an action body died
FORBIDDEN: insufficient privileges to update <object>— as the built-in admin — while the[action-audit]line on the same request announced RLS-bypassing TRUSTED execution. Objects with apublicsharing model, no owner field, or a bypass listing passed the gate early, which is why only some actions broke and the defect read as object-dependent flakiness.The fix
Both dispatch paths — REST
/actions/:object/:actionand MCPrun_action— now bindctx.apitoengine.createContext(...)and thread the same envelope throughctx.engine, matching what hook bodies already get from the engine'sbuildHookApi.The envelope (
buildActionExecutionContext) is the caller's ExecutionContext elevated withisSystem: true. Of the two coherent options the issue offered, this is the one that matches the documented model: elevation is the posture #2849 already documents and gates for at invoke time (capability gate +ai.exposed), so RLS-scoping instead would silently break bodies that legitimately act beyond the caller.One refinement on the bare
{ isSystem: true }that hooks fall back to — the caller's fields are spread first, so writes stay:userIdstampscreated_by/updated_by;tenantIdstamps the org column and drives driver-level tenant isolation;transactionhandle is joined,instead of the unattributable, org-less rows bare elevation would write.
The sandbox's last-resort repo facade is elevated from the same envelope via
ctx.executionContext, so a host predatingcreateContextis not left proxying context-less either. The[action-audit]wording now describes what the body actually gets ("system-elevated context") rather than the false "context-less engine".Changes
packages/runtime/src/action-execution.tsbuildActionExecutionContext+buildActionApi;buildActionEngineFacadetakes the caller envelope and threadscontextthrough every verb; MCPactionContextgainsapi+executionContext; audit line + stale security-model comments correctedpackages/runtime/src/domains/actions.tsapi+executionContext, audit line correctedpackages/runtime/src/sandbox/body-runner.tsbuildSandboxApiprefersql.createContext(ctx.executionContext); the last-resort repo facade threads that context through every callpackages/runtime/src/action-body-identity.test.tscontent/docs/ui/actions.mdxengine; now coversctx.apiand states the elevation postureVerification
The regression tests use an engine stub that refuses a write carrying neither
userIdnorisSystem, the way the real sharing gate does. Reverting just the domain wiring reproduces the reported failure verbatim:With the fix, full monorepo suite green — 132/132 turbo tasks, including runtime (735 tests), objectql (1163), and the dogfood suite (395 passed / 3 skipped). ESLint clean on the changed files.
Out of scope
Generated by Claude Code