fix(execution): stop treating the workflow owner as a live execution identity by icecrasher321 · Pull Request #7330 · simstudioai/sim

icecrasher321 · 2026-09-01T01:03:16Z

What broke

A customer's deployed chats all started failing with Access denied to workspace <id> and zero blocks executed. Traced from the prod replica plus the S3 trace blob:

15:06:01 — an org owner removed a member
The removal transaction reassigned that member's 6 workflows to the workspace billing account (workflow.updated_at = 15:06:01.571), then deleted their permissions rows
19:26–20:03 — 5 chat runs failed, "completionFailure": "Access denied to workspace …", executedBlocks: []

The irony: reassignWorkflowOwnershipForWorkspaceMemberRemovalTx exists precisely to keep the execution identity an active workspace identity, and its TSDoc names workflow.userId. Deployed chat was the one surface reading chat.userId instead — so the same transaction repaired the pointer every other trigger reads and broke the only one chat read.

Chat's principal (kind: 'system') and billing attribution (resolveSystemBillingAttribution) were always correct — the persisted attribution on the failed run shows actorUserId = the workspace billing account. The bug was a third identity.

The rule

A run acts as its caller when one is identifiable, otherwise as the workspace billing account. workflow.userId is the personal-secret fallback only — metadata.workflowUserId has exactly one behavioral consumer, execution-core.ts → personalEnvUserId. This matches platform/credentials.mdx as shipped.

Four surfaces treated that stored pointer as a live permission.

Changes

Chat (app/api/chat/[identifier]/route.ts) — passes workflowRecord.userId, not deployment.userId. It already had the record in scope and used it for two other fields.

getExecutionEnvironment — already tolerated a stale actor (with a paragraph of TSDoc explaining why). Now treats both identities as stored pointers:

actor	personal	before	after
✅	✅	split resolution	unchanged
❌	✅	falls back to owner	unchanged
✅	❌	throws	workspace slice only, no personal vars
❌	❌	throws	throws (nobody to authorize against)

Dropping the personal slice is the more correct outcome, not just the safer one: it stops lending a removed member's personal secrets to their former org indefinitely. An unresolvable {{VAR}} survives as its literal and fails at the block that needs it, naming the variable.

Public API (v1 + v2) — validatePublicApiAllowed and authorizeWorkflowByWorkspacePermission gated on the owner, though an anonymous call acts as the billing account and resolves no personal variables. Both now use a new getWorkspaceBilledAccountUserId. The enable-time gate is unchanged — update-workflow-deployment-settings.ts still checks the acting user, so a restricted member still cannot turn public API on.

Custom blocks — the child ran wholly as the source owner. Billing actor and delegated-tool subject deliberately stay the publisher (changing those would move which OAuth credentials its integrations use), but env now splits like any deployed run. This also closes a silent inconsistency: the child previously saw a narrower workspace-secret selection than a schedule on the very same workflow.

Webhook provider-config — resolveWebhookExecutionProviderConfig takes an optional actorUserId; the call site already had it from preprocessing. New shared resolveBackgroundWebhookEnv for callers with no actor threaded, used by Zoom's URL-validation challenge — a failed challenge makes Zoom deactivate the endpoint rather than drop one delivery — and by provider-subscriptions.ts cleanup, where reading both slices as the owner let a non-admin owner without a credential grant hand the provider a literal {{VAR}} as its credential and silently orphan the subscription. Every path inside that helper routes through getExecutionEnvironment, including the two with no second identity (workspaceless webhook, workspace with no billing account), so none of them can skip the suspension check.

Ban gate — userId is a candidate unless the caller declares it a stored reference. Callers overload that parameter: the workflow owner from the webhook processor, the chat's creator from the deployed-chat route, the live authenticated resumer from the resume route. userIdIsStoredReference defaults to false, so an undeclared call site keeps blocking; only the three that genuinely pass a stored reference opt out. Suspending one member should not take down the schedules, webhooks, and chats their teammates depend on — but it must not admit that person's own actions either, which is why the default is the blocking one.

Suspension — a suspended identity no longer lends its personal namespace. getExecutionEnvironment withholds it and resolves workspace variables only, the same answer it gives a departed identity and an anonymous public-API run. The check runs before the single-identity shortcut, so it is path-independent: a custom-block child (admitted by admitCustomBlockChildExecution, which checks usage limits and nothing else) and a provider URL-validation challenge (no admission at all) are covered, including when publisher and billing account are the same person. Only the personal slice is withheld — workspace variables are workspace-owned, so teammates' runs keep working.

Logs — new run-level executedByEmail, joined from the immutable per-run execution_data->'billingAttribution'->>'actorUserId'. The owner email described whoever owns the workflow now, so a reassignment silently rewrote the answer for runs from months earlier. Internal log projections drop workflow.userId (zero consumers).

API compatibility

workflow.ownerEmail is required in the published openapi-v2-logs.json, so it is kept as a deprecated: true nullable field fed by its own alias(user, 'workflow_owner') join. The two joins must stay separate or the deprecated field silently starts answering the new question — a test pins two different addresses.

Prod impact

5 active deployed chats across 5 workspaces are broken today. 3 need the chat change, 2 need the resolver change (their workflow.userId is itself stale) — both halves are load-bearing.

Not broken

Credential members/admins — filtering logic byte-for-byte unchanged; the degraded path only ever removes secrets, pinned by a test asserting a non-admin actor still gets filtered.
Permission groups — the enable-time gate is untouched. Only an explicit-member group can diverge at invoke time; prod has exactly one such group and its workspaces have zero public-API workflows.

Deliberately out of scope

webhooks/processor.ts keeps single-identity resolution (product call). It already catches and degrades to an empty env, so the symptom is a rejected delivery rather than an error — provider auth verification simply fails.
The bun audit CI step reports 67 pre-existing dependency advisories. Unrelated to this branch and tolerated on staging today.

provider-subscriptions.ts was on this list and is now fixed — see the webhook bullet above. Hardening the public-API invoke-time gate with the org default-group policy was also listed and turned out to be redundant: resolveWorkspaceGroup already falls through to resolveDefaultGroup when the billing account is not in a more specific group, and when they are, most-specific-wins is the intended semantics rather than a bypass.

Verification

bunx turbo run type-check 26/26 · biome · the full 40-audit suite (check:audits) · ~10k app tests · all 18 non-app workspace packages.

Cleared review after 5 rounds: 6 findings, all 6 real. Three were regressions introduced while fixing an earlier one, so each round is its own commit rather than a squash — the history shows what each finding actually changed.

lib/workflows/diff/diff-engine.test.ts is load-flaky on this machine (it.concurrent, 10s timeout) — confirmed pre-existing: clean staging fails the identical test twice under the same load.

Also fixed a lying mock: packages/testing's environment mirror omitted personalOwners/workspaceUnredactedKeys and dropped the latter on the split path, letting mocked snapshots reach production code as undefined.

🤖 Generated with Claude Code

vercel · 2026-09-01T01:03:21Z

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

Project	Deployment	Actions	Updated
docs	Ready	Preview	Sep 1, 2026 2:55am UTC

greptile-apps · 2026-09-01T01:06:25Z

Greptile Summary

This PR separates live execution identity from stored workflow ownership so background runs authorize workspace access using their actual caller or billing account while retaining the owner only as a personal-secret fallback.

Updates chat, public API, webhook, custom-block, and preprocessing paths to use the appropriate execution identities.
Withholds personal variables when their stored owner is departed or suspended while preserving authorized workspace variables.
Adds immutable run-level executor attribution to log APIs while retaining the deprecated workflow-owner field for compatibility.
Updates tests, generated API types, mocks, and credential documentation to reflect the split-identity contract.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
apps/sim/lib/environment/utils.ts	Splits personal and workspace environment resolution and withholds the personal namespace for departed or suspended stored identities.
apps/sim/lib/execution/preprocessing.ts	Distinguishes live callers from stored identity references when constructing suspension-admission candidates.
apps/sim/app/api/chat/[identifier]/route.ts	Uses the maintained workflow owner as the deployed chat’s personal-environment fallback while marking the deployer as stored state.
apps/sim/executor/handlers/workflow/workflow-handler.ts	Resolves custom-block child environments using the publisher for personal variables and the source billing account for workspace variables.
apps/sim/background/webhook-execution.ts	Threads the elected execution actor into webhook provider-config environment resolution.
apps/sim/lib/api/contracts/v2/logs.ts	Adds immutable run-level executor email attribution and deprecates the mutable workflow-owner email field without removing it.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Run[Execution request] --> Caller{Identifiable live caller?}
  Caller -->|Yes| Actor[Actor = caller]
  Caller -->|No| Billing[Actor = workspace billing account]
  Actor --> Workspace[Authorize workspace environment]
  Billing --> Workspace
  Run --> Owner[Stored workflow owner]
  Owner --> Eligible{Owner active and workspace-accessible?}
  Eligible -->|Yes| Personal[Resolve personal environment]
  Eligible -->|No| NoPersonal[Omit personal environment]
  Workspace --> Execute[Execute workflow]
  Personal --> Execute
  NoPersonal --> Execute

_{Reviews (5): Last reviewed commit: "fix(execution): make the suspension gate..." | Re-trigger Greptile}

cubic-dev-ai

All reported issues were addressed across 30 files

_{Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

Fix all with cubic | Re-trigger cubic}

icecrasher321 · 2026-09-01T01:24:06Z

@greptile

icecrasher321 · 2026-09-01T01:24:07Z

@cubic review

cubic-dev-ai · 2026-09-01T01:24:12Z

@cubic review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 31 files

_{Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

Fix all with cubic | Re-trigger cubic}

icecrasher321 · 2026-09-01T02:16:03Z

@greptile

icecrasher321 · 2026-09-01T02:16:04Z

@cubic review

cubic-dev-ai · 2026-09-01T02:16:14Z

@cubic review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 33 files

_{Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

Fix all with cubic | Re-trigger cubic}

icecrasher321 · 2026-09-01T02:43:12Z

@greptile

icecrasher321 · 2026-09-01T02:43:14Z

@cubic review

cubic-dev-ai · 2026-09-01T02:43:18Z

@cubic review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

No issues found across 34 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

Re-trigger cubic}

…identity A background run acts as the workspace billing account; `workflow.userId` is only the personal-variable fallback. Several surfaces treated that stored pointer as a live permission, so each broke when its owner left the workspace. Deployed chat read `chat.userId` — the person who clicked "Deploy as chat" — where every other trigger reads `workflow.userId`. Org member removal reassigns `workflow.userId` to keep it an active workspace identity and has no equivalent for the chat row, so the same transaction repaired the pointer every other trigger reads and broke the only one chat read. Chat now passes the owner. `getExecutionEnvironment` already tolerated a stale actor but not a stale personal identity. Both are stored pointers, so a personal identity that cannot reach the workspace now contributes no personal namespace — the judgment already applied to an anonymous public-API run, and it stops lending a removed member's secrets to their former organization. Only "neither identity reachable" raises. The public API gated `validatePublicApiAllowed` and the workflow read on the owner, though an anonymous call acts as the billing account and resolves no personal variables at all. Both now use `getWorkspaceBilledAccountUserId`. The enable-time gate, which checks the acting user, is unchanged. Custom-block children and webhook provider-config resolved both environment slices as the owner. They now split the two identities like any deployed run, which also closes a silent inconsistency: a custom block saw a narrower workspace-secret selection than a schedule on the very same workflow. The ban gate no longer blocks on the workflow owner — banning one member should not take down the schedules, webhooks, and chats their teammates depend on. Logs gain a run-level `executedByEmail`, joined from the immutable per-run attribution rather than from a workflow row that ownership transfer rewrites. `workflow.ownerEmail` stays as a deprecated field, fed by its own aliased join, because it is required in the published v2 schema. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…the ban candidate correctly Review round 1 (Greptile P1 security, cubic P1 + P2). Removing the workflow owner from the ban gate let a suspended account's personal secrets keep flowing into background runs: a ban revokes neither workspace membership nor the pointer naming that person, so the run continued on their own keys. `getExecutionEnvironment` now drops the personal namespace when that identity is suspended, the same answer it already gives a departed one — the run survives, their credentials do not. Placing it in the shared resolver rather than in `execution-core` covers the webhook and custom-block paths too, and keeps the ban module out of the executor's import graph. The ban candidate itself was also inconsistent: callers overload `userId`, so it is an authenticated caller on a manual run but a stored pointer everywhere else — the workflow owner from `checkWebhookPreprocessing`, the chat's creator from the deployed-chat route, `'unknown'` from a schedule. Reading it unconditionally meant the same ban suspended a webhook while the schedule beside it kept running. It is now gated on `useAuthenticatedUserAsActor`, which is exactly the flag that distinguishes the two — `workflow-column-execution` toggles them together. Also corrects the custom-block authority TSDoc, which still claimed the owner supplies both environment slices after this branch split them. Regenerates the CLI API client for the v2 log contract change, which CI's `check:cli-api` audit caught. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… align webhook cleanup Review round 2 (Greptile P1 security, cubic P0 — both found the same gap). The suspension check sat next to the split-identity access lookups, so it was skipped by the single-identity shortcut above it. That shortcut is taken whenever the two identities coincide — which is exactly what happens when a custom-block publisher is also their workspace's billing account. The check now runs before the shortcut, unconditionally. Round 1's placement rested on "admission already cleared this identity", and that is not true everywhere: a custom-block child is admitted by `admitCustomBlockChildExecution`, which checks usage limits and nothing else, and a provider URL-validation challenge resolves its secret with no admission at all. Neither path has ever had a ban gate. Only the personal namespace is withheld. Workspace variables belong to the workspace rather than to a person, so they keep resolving and a suspended member's teammates keep working — the reason admission stopped blocking on this identity to begin with. Webhook cleanup now resolves through the same two-identity reader as delivery. Reading both slices as the owner let cleanup see a narrower selection than the delivery that created the subscription: a non-admin owner without a credential grant left `{{VAR}}` unresolved, the provider was handed the literal reference as its credential, and the non-fatal catch silently orphaned the subscription. `resolveBackgroundWebhookEnv` imports the billing reader statically. The dynamic import bought nothing — every boundary audit passes without it — and made each worker pay a cold module load on the first webhook resolution. Restores the `@sim/testing` mock-shape test, which pins that the default snapshot carries every field of the real one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…tity resolver Review round 3 (cubic P1). `resolveBackgroundWebhookEnv` short-circuited to `getEffectiveDecryptedEnv` for the two cases with no second identity — a legacy workspaceless webhook, and a workspace with no billing account — which read the owner's variables without passing the resolver's suspension check. cubic flagged the first; the second was the same bypass one line down. Both now name the owner as both identities and go through the resolver, which produces the identical resolution while putting them behind the same gate. Also corrects `provider-subscriptions.test.ts`, which still asserted cleanup resolves via `getEffectiveDecryptedEnv`. That assertion was passing intermittently rather than failing outright: `mockGetEffectiveDecryptedEnv` is a shared singleton on `environmentUtilsMockFns`, so whether it had been called depended on which other files shared the worker. It now asserts the two-identity call, with the billing reader mocked so the split is actually exercised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… userId Review round 4 (Greptile P1 security). Round 2 keyed the ban candidate on `useAuthenticatedUserAsActor`, assuming that flag separates a live caller from a stored reference. It does not. The interactive resume route reads `access.auth?.userId` and passes that live resumer as `userId` while leaving the flag false on purpose — attribution is captured before the pause and must not move — so a suspended user could resume a paused run whose persisted attribution named a different, unsuspended actor. The distinction is per-caller and cannot be inferred, so it is now declared. `userIdIsStoredReference` defaults to false, which means an undeclared call site keeps blocking; only the three that genuinely pass a stored reference opt out: the webhook processor (the workflow owner), the deployed-chat route (the chat's creator), and table-cell dispatch (the owner, but only when nothing triggered it). Resume, manual, API, and async paths are candidates again. Withholding a suspended account's personal variables stays where it was, in `getExecutionEnvironment`, so the two concerns remain separable: a suspended stored reference does not block the run, and does not lend its credentials either. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

icecrasher321 · 2026-09-01T02:53:57Z

@greptile

icecrasher321 · 2026-09-01T02:53:58Z

@cubic review

cubic-dev-ai · 2026-09-01T02:54:03Z

@cubic review

@icecrasher321 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

No issues found across 36 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Re-trigger cubic}

vercel Bot deployed to Preview September 1, 2026 01:05 View deployment

greptile-apps Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread apps/sim/lib/execution/preprocessing.ts