Commit f2e20aa
fix(execution): stop treating the workflow owner as a live execution identity (#7330)
* fix(execution): stop treating the workflow owner as a live execution 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>
* fix(execution): suspend personal secrets for banned owners, and gate 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>
* fix(execution): enforce suspension on every personal-secret path, and 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>
* fix(webhooks): route every background env resolution through the identity 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>
* fix(execution): make the suspension gate fail closed on an undeclared 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>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>1 parent 1c197c1 commit f2e20aa
36 files changed
Lines changed: 929 additions & 123 deletions
File tree
- apps
- docs
- content/docs/platform
- sim
- app/api
- chat/[identifier]
- v1/logs/[id]
- v2
- logs/[runId]
- workflows/[workflowId]/execute
- workflows/[id]/execute
- background
- executor/handlers/workflow
- lib
- api/contracts/v2
- openapi
- billing/core
- environment
- execution
- logs
- application
- webhooks
- providers
- workflows/custom-blocks
- packages
- sim-cli/src/generated
- testing/src/mocks
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
177 | 177 | | |
178 | 178 | | |
179 | 179 | | |
180 | | - | |
| 180 | + | |
181 | 181 | | |
182 | 182 | | |
183 | 183 | | |
184 | 184 | | |
| 185 | + | |
| 186 | + | |
185 | 187 | | |
186 | 188 | | |
187 | 189 | | |
| |||
193 | 195 | | |
194 | 196 | | |
195 | 197 | | |
196 | | - | |
| 198 | + | |
| 199 | + | |
197 | 200 | | |
198 | 201 | | |
199 | 202 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1344 | 1344 | | |
1345 | 1345 | | |
1346 | 1346 | | |
| 1347 | + | |
| 1348 | + | |
| 1349 | + | |
| 1350 | + | |
| 1351 | + | |
| 1352 | + | |
| 1353 | + | |
| 1354 | + | |
| 1355 | + | |
| 1356 | + | |
| 1357 | + | |
| 1358 | + | |
| 1359 | + | |
1347 | 1360 | | |
1348 | 1361 | | |
1349 | 1362 | | |
| |||
1398 | 1411 | | |
1399 | 1412 | | |
1400 | 1413 | | |
1401 | | - | |
| 1414 | + | |
| 1415 | + | |
1402 | 1416 | | |
1403 | 1417 | | |
1404 | 1418 | | |
| |||
1577 | 1591 | | |
1578 | 1592 | | |
1579 | 1593 | | |
| 1594 | + | |
1580 | 1595 | | |
1581 | 1596 | | |
1582 | 1597 | | |
| |||
1614 | 1629 | | |
1615 | 1630 | | |
1616 | 1631 | | |
| 1632 | + | |
1617 | 1633 | | |
1618 | 1634 | | |
1619 | 1635 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
180 | 180 | | |
181 | 181 | | |
182 | 182 | | |
| 183 | + | |
| 184 | + | |
183 | 185 | | |
184 | 186 | | |
185 | 187 | | |
| |||
273 | 275 | | |
274 | 276 | | |
275 | 277 | | |
276 | | - | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
277 | 288 | | |
278 | 289 | | |
279 | 290 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
54 | | - | |
55 | 54 | | |
56 | 55 | | |
57 | 56 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
49 | 49 | | |
50 | 50 | | |
51 | 51 | | |
| 52 | + | |
52 | 53 | | |
53 | 54 | | |
54 | 55 | | |
| |||
86 | 87 | | |
87 | 88 | | |
88 | 89 | | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
89 | 97 | | |
90 | 98 | | |
| 99 | + | |
91 | 100 | | |
92 | 101 | | |
93 | 102 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
49 | 49 | | |
50 | 50 | | |
51 | 51 | | |
| 52 | + | |
52 | 53 | | |
53 | 54 | | |
54 | 55 | | |
55 | 56 | | |
56 | 57 | | |
| 58 | + | |
57 | 59 | | |
58 | 60 | | |
59 | 61 | | |
| |||
Lines changed: 23 additions & 19 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
218 | 218 | | |
219 | 219 | | |
220 | 220 | | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
221 | 237 | | |
222 | 238 | | |
223 | 239 | | |
| |||
759 | 775 | | |
760 | 776 | | |
761 | 777 | | |
762 | | - | |
763 | | - | |
764 | | - | |
| 778 | + | |
765 | 779 | | |
766 | 780 | | |
767 | 781 | | |
| |||
774 | 788 | | |
775 | 789 | | |
776 | 790 | | |
777 | | - | |
778 | | - | |
779 | | - | |
| 791 | + | |
780 | 792 | | |
781 | 793 | | |
782 | 794 | | |
783 | 795 | | |
784 | 796 | | |
785 | 797 | | |
786 | | - | |
787 | | - | |
788 | | - | |
| 798 | + | |
789 | 799 | | |
790 | 800 | | |
791 | 801 | | |
| |||
798 | 808 | | |
799 | 809 | | |
800 | 810 | | |
801 | | - | |
802 | | - | |
803 | | - | |
| 811 | + | |
804 | 812 | | |
805 | 813 | | |
806 | 814 | | |
| |||
837 | 845 | | |
838 | 846 | | |
839 | 847 | | |
840 | | - | |
| 848 | + | |
841 | 849 | | |
842 | 850 | | |
843 | 851 | | |
| |||
870 | 878 | | |
871 | 879 | | |
872 | 880 | | |
873 | | - | |
874 | | - | |
875 | | - | |
| 881 | + | |
876 | 882 | | |
877 | 883 | | |
878 | 884 | | |
| |||
891 | 897 | | |
892 | 898 | | |
893 | 899 | | |
894 | | - | |
895 | | - | |
896 | | - | |
| 900 | + | |
897 | 901 | | |
898 | 902 | | |
899 | 903 | | |
| |||
Lines changed: 16 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| 24 | + | |
24 | 25 | | |
25 | 26 | | |
26 | 27 | | |
| |||
173 | 174 | | |
174 | 175 | | |
175 | 176 | | |
176 | | - | |
177 | 177 | | |
178 | 178 | | |
179 | 179 | | |
| |||
183 | 183 | | |
184 | 184 | | |
185 | 185 | | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
186 | 198 | | |
187 | | - | |
| 199 | + | |
188 | 200 | | |
189 | 201 | | |
190 | 202 | | |
191 | 203 | | |
192 | 204 | | |
193 | 205 | | |
194 | | - | |
| 206 | + | |
195 | 207 | | |
196 | 208 | | |
197 | 209 | | |
| |||
343 | 355 | | |
344 | 356 | | |
345 | 357 | | |
346 | | - | |
| 358 | + | |
347 | 359 | | |
348 | 360 | | |
349 | 361 | | |
| |||
Lines changed: 3 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
| 41 | + | |
41 | 42 | | |
42 | 43 | | |
43 | 44 | | |
| |||
89 | 90 | | |
90 | 91 | | |
91 | 92 | | |
| 93 | + | |
92 | 94 | | |
93 | 95 | | |
94 | 96 | | |
95 | 97 | | |
96 | 98 | | |
97 | 99 | | |
| 100 | + | |
98 | 101 | | |
99 | 102 | | |
100 | 103 | | |
| |||
345 | 348 | | |
346 | 349 | | |
347 | 350 | | |
348 | | - | |
349 | 351 | | |
350 | 352 | | |
351 | 353 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| 23 | + | |
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
| |||
596 | 597 | | |
597 | 598 | | |
598 | 599 | | |
599 | | - | |
600 | 600 | | |
601 | 601 | | |
602 | 602 | | |
| |||
607 | 607 | | |
608 | 608 | | |
609 | 609 | | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
610 | 623 | | |
611 | | - | |
| 624 | + | |
612 | 625 | | |
613 | 626 | | |
614 | 627 | | |
615 | 628 | | |
616 | 629 | | |
617 | 630 | | |
618 | 631 | | |
619 | | - | |
| 632 | + | |
620 | 633 | | |
621 | 634 | | |
622 | 635 | | |
| |||
0 commit comments