diff --git a/.changeset/cross-session-resume-org-write.md b/.changeset/cross-session-resume-org-write.md new file mode 100644 index 0000000000..299d43bb24 --- /dev/null +++ b/.changeset/cross-session-resume-org-write.md @@ -0,0 +1,5 @@ +--- +"@executor-js/cloud": patch +--- + +An admin who resumes a paused execution from a different MCP session (for example after the client reconnects) keeps workspace-write access. The forwarded resume now carries the requester's access to the session that owns the execution, so a pending `addServer`, `addSpec`, or similar write no longer fails with `org_write_denied`. diff --git a/apps/cloud/src/mcp/session-durable-object.ts b/apps/cloud/src/mcp/session-durable-object.ts index 701166ece7..81d9b50d4e 100644 --- a/apps/cloud/src/mcp/session-durable-object.ts +++ b/apps/cloud/src/mcp/session-durable-object.ts @@ -33,6 +33,7 @@ import { type BuiltMcpServer, type IncomingTraceHeaders, type McpApprovalOwner, + type McpModelResumeCaller, type McpSessionModelResumeResult, type McpSessionInit, type SessionMeta, @@ -235,7 +236,7 @@ export class McpSessionDOSqlite extends McpAgentSessionDOBase { diff --git a/apps/host-cloudflare/src/mcp/session-durable-object.ts b/apps/host-cloudflare/src/mcp/session-durable-object.ts index 62d8685747..1ddd680093 100644 --- a/apps/host-cloudflare/src/mcp/session-durable-object.ts +++ b/apps/host-cloudflare/src/mcp/session-durable-object.ts @@ -12,7 +12,7 @@ import type { ExecutorDbHandle } from "@executor-js/api/server"; import { McpAgentSessionDOBase, type BuiltMcpServer, - type McpApprovalOwner, + type McpModelResumeCaller, type McpSessionModelResumeResult, type McpSessionInit, type SessionMeta, @@ -95,7 +95,7 @@ export class McpSessionDO extends McpAgentSessionDOBase { diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts index 5a90dc5978..7613459221 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts @@ -21,7 +21,13 @@ import { } from "@executor-js/host-mcp/tool-server"; import { defaultMcpResource, mcpResourceKey, type McpResource } from "@executor-js/host-mcp"; import { decodeResumeResponse, type McpToolMode } from "@executor-js/host-mcp/browser-approval"; -import { ElicitationResponse } from "@executor-js/sdk"; +import { + CurrentOrgWriteAccess, + ElicitationResponse, + currentOrgWriteAccess, + makeOrgWriteAccessState, + type OrgWriteAccess, +} from "@executor-js/sdk"; import type { IncomingPropagationHeaders, McpElicitationMode } from "./do-headers"; import { classifyDurableObjectError, type DurableObjectFailure } from "./durable-object-errors"; @@ -97,6 +103,12 @@ export type McpApprovalOwner = { readonly organizationId: string; }; +/** A model `resume` forwarded from another session of the same owner, carrying + * the workspace-write access its own request was authenticated with. */ +export type McpModelResumeCaller = McpApprovalOwner & { + readonly orgWriteAccess: OrgWriteAccess; +}; + /** Authenticated browser approver with a freshly resolved organization role. */ export type McpApprovalPrincipal = McpApprovalOwner & { readonly orgRole: "admin" | "member"; @@ -546,7 +558,7 @@ export abstract class McpAgentSessionDOBase< protected forwardModelResumeToOwner( _owner: McpExecutionOwnerRoute, - _identity: McpApprovalOwner, + _identity: McpModelResumeCaller, _executionId: string, _response: ResumeResponse, ): Effect.Effect { @@ -1678,7 +1690,7 @@ export abstract class McpAgentSessionDOBase< async resumeExecutionForModel( executionId: string, - identity: McpApprovalOwner, + identity: McpModelResumeCaller, response: ResumeResponse, incoming?: IncomingTraceHeaders, ): Promise { @@ -1698,7 +1710,17 @@ export abstract class McpAgentSessionDOBase< return { status: "execution_expired" as const, ttlMs: PAUSED_APPROVAL_TIMEOUT_MS }; } - const outcome = yield* self.resumeEngineWithLifecycle(executionId, response); + // This RPC runs outside any MCP request, so nothing else binds the + // caller's workspace-write access; without it the resume would rebind + // the paused execution to the fail-closed default and deny an admin's + // pending write. A caller that predates the field is treated as denied. + const orgWriteAccess: OrgWriteAccess = + identity.orgWriteAccess === "allowed" ? "allowed" : "denied"; + const outcome = yield* self + .resumeEngineWithLifecycle(executionId, response) + .pipe( + Effect.provideService(CurrentOrgWriteAccess, makeOrgWriteAccessState(orgWriteAccess)), + ); if (!outcome) { const alreadySettled = self.engine.isExecutionSettled ? yield* self.engine.isExecutionSettled(executionId) @@ -1959,9 +1981,10 @@ export abstract class McpAgentSessionDOBase< const sessionMeta = yield* self.loadSessionMeta(); if (!sessionMeta) return { status: "execution_forbidden" } as const; - const identity: McpApprovalOwner = { + const identity: McpModelResumeCaller = { accountId: sessionMeta.userId, organizationId: sessionMeta.organizationId, + orgWriteAccess: yield* currentOrgWriteAccess, }; if ( identity.accountId !== record.accountId || diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-model-resume.test.ts b/packages/hosts/cloudflare/src/mcp/agent-session-model-resume.test.ts index 8a95011ed4..7c8128c8b0 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-model-resume.test.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-model-resume.test.ts @@ -2,6 +2,12 @@ import { afterEach, beforeEach, describe, expect, it } from "@effect/vitest"; // oxlint-disable-next-line executor/no-vitest-import -- boundary: vi.mock must come from vitest itself for mock hoisting to resolve import { vi } from "vitest"; import { Cause, Effect } from "effect"; +import { + CurrentOrgWriteAccess, + currentOrgWriteAccess, + makeOrgWriteAccessState, + type OrgWriteAccess, +} from "@executor-js/sdk"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { defaultMcpResource } from "@executor-js/host-mcp"; import { @@ -18,7 +24,7 @@ import type { import { McpAgentSessionDOBase, type BuiltMcpServer, - type McpApprovalOwner, + type McpModelResumeCaller, type McpSessionInit, type McpSessionModelResumeResult, type SessionMeta, @@ -249,8 +255,10 @@ const makeEngine = ( resultForResume: (executionId: string, response: ResumeResponse) => ExecutionResult | null, ) => { const calls: ResumeCall[] = []; + const orgWriteAccesses: OrgWriteAccess[] = []; const resume = vi.fn((executionId: string, response: ResumeResponse) => - Effect.sync(() => { + Effect.gen(function* () { + orgWriteAccesses.push(yield* currentOrgWriteAccess); calls.push({ executionId, response }); return resultForResume(executionId, response); }), @@ -266,7 +274,7 @@ const makeEngine = ( // The fake forks nothing, so there is no sandbox fiber to end. shutdown: Effect.void, }; - return { calls, engine, resume }; + return { calls, engine, orgWriteAccesses, resume }; }; const sessionMeta = (input?: Partial): SessionMeta => ({ @@ -299,7 +307,7 @@ class HarnessSession extends McpAgentSessionDOBase private readonly directory: McpExecutionOwnerDirectory | null; private readonly modelResumeForward: ( owner: McpExecutionOwnerRoute, - identity: McpApprovalOwner, + identity: McpModelResumeCaller, executionId: string, response: ResumeResponse, ) => Effect.Effect; @@ -337,7 +345,7 @@ class HarnessSession extends McpAgentSessionDOBase protected override forwardModelResumeToOwner( owner: McpExecutionOwnerRoute, - identity: McpApprovalOwner, + identity: McpModelResumeCaller, executionId: string, response: ResumeResponse, ): Effect.Effect { @@ -379,13 +387,21 @@ class HarnessSession extends McpAgentSessionDOBase await this.fakeState.flushWaitUntil(); } + /** Resume as the MCP `resume` tool does, under the request's write access. */ async resumeViaModelTool( executionId: string, response: ResumeResponse, + orgWriteAccess: OrgWriteAccess = "denied", ): Promise { - const local = await Effect.runPromise(this["engine"]!.resume(executionId, response)); + const bound = Effect.provideService( + CurrentOrgWriteAccess, + makeOrgWriteAccessState(orgWriteAccess), + ); + const local = await Effect.runPromise( + this["engine"]!.resume(executionId, response).pipe(bound), + ); if (local) return { status: "result", result: formatMcpExecutionOutcome(local) }; - return Effect.runPromise(this.modelResumeFallback(executionId, response)); + return Effect.runPromise(this.modelResumeFallback(executionId, response).pipe(bound)); } pendingLease(executionId: string): PendingApprovalLeaseSnapshot | undefined { @@ -466,7 +482,7 @@ describe("McpAgentSessionDOBase cross-session model resume", () => { const forward = vi.fn( ( owner: McpExecutionOwnerRoute, - identity: McpApprovalOwner, + identity: McpModelResumeCaller, executionId: string, response: ResumeResponse, ) => @@ -534,6 +550,67 @@ describe("McpAgentSessionDOBase cross-session model resume", () => { expect(ownerEngine.calls).toEqual([{ executionId: "exec_owner", response: approval }]); }); + for (const orgWriteAccess of ["allowed", "denied"] as const) { + it(`resumes the owning session under the requester's ${orgWriteAccess} workspace-write access`, async () => { + const { namespace } = makeDirectory(); + const ownerEngine = makeEngine(() => completed("owner-result")); + const requesterEngine = makeEngine(() => null); + const sessions = new Map(); + const sessionNamespace = { + idFromName: (name: string) => name, + get: (id: string) => sessions.get(id), + }; + const sessionA = new HarnessSession({ + sessionId: "session-a", + engine: ownerEngine.engine, + directoryNamespace: namespace, + }); + const sessionB = new HarnessSession({ + sessionId: "session-b", + engine: requesterEngine.engine, + directoryNamespace: namespace, + forwardModelResumeToOwner: (owner, identity, executionId, response) => + Effect.promise(() => + mcpSessionStub(sessionNamespace, owner.sessionId).resumeExecutionForModel( + executionId, + identity, + response, + ), + ), + }); + sessions.set(mcpSessionDurableObjectName("session-a"), sessionA); + await sessionA.storeSessionMeta(); + await sessionB.storeSessionMeta(); + await sessionA.startPause("exec_owner"); + + await sessionB.resumeViaModelTool("exec_owner", approval, orgWriteAccess); + + expect(ownerEngine.orgWriteAccesses).toEqual([orgWriteAccess]); + }); + } + + it("treats a forwarded resume without workspace-write access as denied", async () => { + const { namespace } = makeDirectory(); + const ownerEngine = makeEngine(() => completed("owner-result")); + const sessionA = new HarnessSession({ + sessionId: "session-a", + engine: ownerEngine.engine, + directoryNamespace: namespace, + }); + await sessionA.storeSessionMeta(); + await sessionA.startPause("exec_owner"); + + // A requester still running the previous deploy sends only the owner pair. + const legacyIdentity = { accountId: "acct_1", organizationId: "org_1" }; + await sessionA.resumeExecutionForModel( + "exec_owner", + legacyIdentity as McpModelResumeCaller, + approval, + ); + + expect(ownerEngine.orgWriteAccesses).toEqual(["denied"]); + }); + it("rejects identity mismatch without invoking the owning session engine", async () => { const { directory, namespace } = makeDirectory(); const ownerEngine = makeEngine(() => completed("should-not-run")); diff --git a/packages/hosts/cloudflare/src/mcp/session-stub.ts b/packages/hosts/cloudflare/src/mcp/session-stub.ts index 696e16b304..184161e908 100644 --- a/packages/hosts/cloudflare/src/mcp/session-stub.ts +++ b/packages/hosts/cloudflare/src/mcp/session-stub.ts @@ -5,6 +5,7 @@ import type { IncomingTraceHeaders, McpApprovalOwner, McpApprovalPrincipal, + McpModelResumeCaller, McpSessionApprovalResult, McpSessionModelResumeResult, McpSessionResumeApprovalResult, @@ -35,7 +36,7 @@ export interface McpSessionStub { ) => Promise; readonly resumeExecutionForModel: ( executionId: string, - identity: McpApprovalOwner, + identity: McpModelResumeCaller, response: ResumeResponse, incoming?: IncomingTraceHeaders, ) => Promise;