fix(inbox): stop an unattributed sender inheriting owner write authority - #7006
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Also closes the matching hole in headless Copilot: client-routed tools like Reviewed by Cursor Bugbot for commit 6a0ce48. Configure here. |
Greptile SummaryThe PR separates the identity used for inbox execution and billing from the authority granted to inbox tools.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/mothership/inbox/executor.ts | Separates owner execution identity from tool authority by capping unattributed inbox runs at read permission. |
| apps/sim/lib/copilot/tool-executor/executor.ts | Applies a write requirement when client-routed tools execute through registered server-side handlers in headless mode. |
| apps/sim/lib/mothership/inbox/executor.test.ts | Covers member permissions, the unattributed-sender read ceiling, and preservation of absent permission. |
| apps/sim/lib/copilot/tool-executor/executor.test.ts | Verifies that headless workflow fallback execution accepts write authority and rejects read or missing authority. |
Sequence Diagram
sequenceDiagram
participant Sender as Inbox sender
participant Inbox as Inbox executor
participant Lifecycle as Headless Copilot
participant Tool as Tool executor
participant Workflow as Workflow handler
Inbox->>Inbox: Resolve execution and secret actors
alt Attributed workspace member
Inbox->>Lifecycle: Member identity and own permission
else Unattributed sender
Inbox->>Lifecycle: Owner execution identity and read ceiling
end
Lifecycle->>Tool: Execute requested tool with userPermission
Tool->>Tool: Check catalog or fallback write requirement
alt Permission satisfies requirement
Tool->>Workflow: Invoke registered handler
else Read or missing permission
Tool-->>Lifecycle: Permission denied
end
Reviews (2): Last reviewed commit: "fix(copilot): bar the headless client-to..." | Re-trigger Greptile
resolveInboxExecutionActor refuses to name a raw-secret actor when the sender matches no workspace member, then hands the run ws.ownerId for everything else. That identity also supplies userPermission, which is what executeTool gates on, so the owner's admin satisfied every requiredPermission check. In headless mode the client-routed workflow tools fall back to their registered server handlers (see the comment in tool-executor/executor.ts), so create_workflow, edit_workflow and run_workflow — all requiredPermission 'write' — were reachable. runWorkflowFromCopilot then executes with enforceCredentialAccess and the owner as actor, which resolves the owner's workspace and personal secrets. An allowlisted external correspondent could therefore reach, through a workflow it had the agent build and run, exactly what the null secret actor refuses for a direct mount. Cap the run's tool permission at read when no member owns the message. An attributed message is unchanged and still uses the sender's own permission, so a read-only member emailing the inbox still cannot run or edit anything. Read rather than none because answering an external correspondent from workspace context is the point of the inbox; only mutation and execution are withheld. The owner identity itself stays: billing attribution and workspace reads need a real user. This separates that need from the authority that came with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client-routed tools carry no catalog requiredPermission because the browser runs them through the workflow APIs, which authorize the caller's own session. The headless fallback in executeTool has no session and runs under the request's principal instead, with nothing standing in for that check. So the read cap from the previous commit did not reach run_workflow, run_workflow_until_block, run_block or run_from_block: all four are route 'client' with no requiredPermission, unlike create_workflow and edit_workflow. An unattributed inbox sender could therefore still run an existing workflow, which executes with enforceCredentialAccess under the workspace owner and resolves the owner's workspace and personal secrets. Derive the requirement at the gate instead: a client-routed tool taking the headless fallback requires write. Interactive callers never reach this branch, so the browser path is unaffected. The catalog itself is generated from the copilot contracts repo and cannot carry this rule, which only applies to the fallback. Also corrects the inboxToolPermission doc, which claimed run_workflow gates on requiredPermission 'write'. It does not; it is gated here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cf7a29d to
6a0ce48
Compare
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6a0ce48. Configure here.
What
An inbox message from a sender that matches no workspace member runs as
ws.ownerId. That identity also suppliesuserPermission, which is what tool authorization gates on — so the owner'sadminsatisfied every check. This caps such a run atread.Follow-up to #7004, which established that Copilot's
function_executegate was routable around via the workflow tools. Same shape, larger blast radius.The path
resolveInboxExecutionActordeliberately refuses to name a raw-secret actor for an unmatched sender:The
nullguardsfunction_execute's direct mount. Nothing guarded the rest:executor.ts:218→userPermission = workspaceAccess.permission, resolved forws.ownerId→admin.tool-executor/executor.ts:57-59says exactly this, andregister-handlers.ts:142,148wirescreate_workflowandrun_workflow.create_workflow,edit_workflow,run_workflowcarryrequiredPermission: 'write', checked atexecutor.ts:41-53against thatuserPermission.runWorkflowFromCopilot(lib/workflows/application/run-workflow-from-copilot.ts:223) callsexecuteWorkflow(..., actorUserId, { enforceCredentialAccess: true }).enforceCredentialAccessmakesidentifiedCallerUserId = metadata.userId(execution-core.ts:452), so bothpersonalEnvUserIdandworkspaceEnvUserIdbecome the owner.Net: an allowlisted external correspondent could reach the owner's workspace and personal secrets by having the agent build and run a workflow — precisely what
secretActorUserId: nullrefuses for a direct mount.Bounded by
mothership_inbox_allowed_sender(webhooks/agentmail/route.ts:304), so an admin deliberately added the address. Not the open internet, but a far larger implicit grant than adding a vendor's address suggests.Change
One predicate,
inboxToolPermission: an attributed message keeps the sender's own permission; an unattributed one is capped atread.Attributed runs are unchanged —
executionUserIdandsecretActorUserIdare already the same member there, so an emailed request is equivalent to that person acting in the app, and a read-only member still cannot run or edit anything.Read rather than none because answering an external correspondent from workspace context is the point of the inbox; only mutation and execution are withheld.
nullstaysnull— no promotion.The owner identity itself stays. Billing attribution and workspace reads need a real user; that need is why the fallback exists, and it's now separated from the authority that rode along with it.
Test plan
lib/mothership/inbox/executor.test.ts— 4 passed.caps an external sender at read even when the owner is an admin— verified this fails without the fix (reverted the one line, watched it go red, restored).does not lend a read-only member write authority— attributed path unchanged.leaves an external sender with no permission at none rather than promoting to read.userPermission.bun run type-checkclean, biome clean.Not addressed
This caps authority; it does not change the fact that an unattributed run still reads workspace context as the owner, which is the inbox's intended behaviour. If that should narrow too, it's a product decision about what an external correspondent may be told, not an authorization bug.
🤖 Generated with Claude Code