From e14dd7a45cc1bedf66b9f53be4630108ec5fcc3c Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Fri, 25 Sep 2026 10:28:09 +0000 Subject: [PATCH 1/3] fix(remote): cancel expired workspace mutations --- src/remote-control/workspace-rpc.ts | 63 +++++++++++-- ...ADR-0108-remote-workspace-rpc-deadlines.md | 12 +++ structure/remote-workspace.md | 7 ++ .../remote-workspace-session-binding.test.ts | 2 +- tests/clients/remote-workspace.test.ts | 94 +++++++++++++++++++ 5 files changed, 168 insertions(+), 10 deletions(-) create mode 100644 structure/decisions/ADR-0108-remote-workspace-rpc-deadlines.md diff --git a/src/remote-control/workspace-rpc.ts b/src/remote-control/workspace-rpc.ts index 15f8c7a3811..eeba1d5c3a4 100644 --- a/src/remote-control/workspace-rpc.ts +++ b/src/remote-control/workspace-rpc.ts @@ -17,14 +17,22 @@ import { } from "./workspace-rpc-framing"; const REMOTE_WORKSPACE_RPC_VERSION = 1 as const; -const REMOTE_WORKSPACE_RPC_DEFAULT_TIMEOUT_MS = 30_000; +const REMOTE_WORKSPACE_RPC_DEFAULT_TIMEOUT_MS = 65_000; +const REMOTE_WORKSPACE_RPC_MAX_TIMEOUT_MS = 120_000; const REMOTE_WORKSPACE_RPC_MAX_ACTIVE_REQUESTS = 8; interface RemoteWorkspaceRpcRequest { version: typeof REMOTE_WORKSPACE_RPC_VERSION; kind: "request"; + timeoutMs: number; request: RemoteWorkspaceExecutionRequest; } +interface RemoteWorkspaceRpcCancel { + version: typeof REMOTE_WORKSPACE_RPC_VERSION; + kind: "cancel"; + requestId: string; +} + interface RemoteWorkspaceRpcResponse { version: typeof REMOTE_WORKSPACE_RPC_VERSION; kind: "response"; @@ -32,7 +40,7 @@ interface RemoteWorkspaceRpcResponse { result: RemoteWorkspaceToolResult; } -type RemoteWorkspaceRpcMessage = RemoteWorkspaceRpcRequest | RemoteWorkspaceRpcResponse; +type RemoteWorkspaceRpcMessage = RemoteWorkspaceRpcRequest | RemoteWorkspaceRpcResponse | RemoteWorkspaceRpcCancel; interface PendingRequest { resolve(value: RemoteWorkspaceToolResult): void; @@ -101,8 +109,14 @@ function parseMessage(value: Uint8Array): RemoteWorkspaceRpcMessage { if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) throw new Error("invalid remote workspace RPC message"); const raw = parsed as Record; if (raw.version !== REMOTE_WORKSPACE_RPC_VERSION) throw new Error("unsupported remote workspace RPC version"); - if (raw.kind === "request") { - return { version: REMOTE_WORKSPACE_RPC_VERSION, kind: "request", request: parseRequest(raw.request) }; + if (raw.kind === "request" && Number.isSafeInteger(raw.timeoutMs) + && (raw.timeoutMs as number) >= 1 && (raw.timeoutMs as number) <= REMOTE_WORKSPACE_RPC_MAX_TIMEOUT_MS) { + return { + version: REMOTE_WORKSPACE_RPC_VERSION, + kind: "request", + timeoutMs: raw.timeoutMs as number, + request: parseRequest(raw.request), + }; } if (raw.kind === "response" && boundedIdentifier(raw.requestId)) { return { @@ -112,6 +126,9 @@ function parseMessage(value: Uint8Array): RemoteWorkspaceRpcMessage { result: parseResult(raw.result), }; } + if (raw.kind === "cancel" && boundedIdentifier(raw.requestId)) { + return { version: REMOTE_WORKSPACE_RPC_VERSION, kind: "cancel", requestId: raw.requestId }; + } throw new Error("invalid remote workspace RPC message kind"); } @@ -132,7 +149,8 @@ export class EncryptedRemoteWorkspaceTransport implements RemoteWorkspaceTranspo constructor(private readonly options: EncryptedRemoteWorkspaceTransportOptions) { this.timeoutMs = options.timeoutMs ?? REMOTE_WORKSPACE_RPC_DEFAULT_TIMEOUT_MS; - if (!boundedIdentifier(options.executorDeviceId) || !Number.isSafeInteger(this.timeoutMs) || this.timeoutMs < 1) { + if (!boundedIdentifier(options.executorDeviceId) || !Number.isSafeInteger(this.timeoutMs) + || this.timeoutMs < 1 || this.timeoutMs > REMOTE_WORKSPACE_RPC_MAX_TIMEOUT_MS) { throw new Error("invalid encrypted remote workspace transport options"); } } @@ -150,7 +168,8 @@ export class EncryptedRemoteWorkspaceTransport implements RemoteWorkspaceTranspo const response = new Promise((resolve, reject) => { const timer = setTimeout(() => { this.pending.delete(request.requestId); - reject(new Error("remote workspace request timed out")); + void this.sendCancellation(request.requestId); + reject(new Error("remote workspace request timed out; executor cancellation was requested")); }, this.timeoutMs); this.pending.set(request.requestId, { resolve, reject, timer }); }); @@ -158,6 +177,7 @@ export class EncryptedRemoteWorkspaceTransport implements RemoteWorkspaceTranspo await this.sendMessage(encodeMessage({ version: REMOTE_WORKSPACE_RPC_VERSION, kind: "request", + timeoutMs: this.timeoutMs, request, })); } catch { @@ -203,6 +223,19 @@ export class EncryptedRemoteWorkspaceTransport implements RemoteWorkspaceTranspo this.sendTail = operation.catch(() => {}); return operation; } + + private async sendCancellation(requestId: string): Promise { + try { + await this.sendMessage(encodeMessage({ + version: REMOTE_WORKSPACE_RPC_VERSION, + kind: "cancel", + requestId, + })); + } catch { + // A failed encrypted write consumes the send counter; the session cannot safely continue. + this.close("remote workspace cancellation send failed"); + } + } } export interface EncryptedRemoteWorkspaceExecutorEndpointOptions { @@ -218,7 +251,10 @@ export interface EncryptedRemoteWorkspaceExecutorEndpointOptions { /** Executor-side endpoint. It accepts only authenticated, ordered E2EE session frames. */ export class EncryptedRemoteWorkspaceExecutorEndpoint { private closed = false; - private readonly active = new Map(); + private readonly active = new Map; + }>(); private readonly reassembler = new RemoteWorkspaceRpcReassembler(); private sendTail: Promise = Promise.resolve(); @@ -240,6 +276,10 @@ export class EncryptedRemoteWorkspaceExecutorEndpoint { const requestPlaintext = this.reassembler.accept(this.options.cipher.decrypt(value)); if (!requestPlaintext) return; const message = parseMessage(requestPlaintext); + if (message.kind === "cancel") { + this.active.get(message.requestId)?.controller.abort(); + return; + } if (message.kind !== "request") throw new Error("executor received a remote workspace response"); if (message.request.executorDeviceId !== this.options.executorDeviceId) { throw new Error("remote workspace encrypted request targeted another executor"); @@ -255,11 +295,13 @@ export class EncryptedRemoteWorkspaceExecutorEndpoint { throw new Error("remote workspace executor request limit reached"); } const controller = new AbortController(); - this.active.set(message.request.requestId, controller); + const timer = setTimeout(() => controller.abort(), message.timeoutMs); + this.active.set(message.request.requestId, { controller, timer }); let result: RemoteWorkspaceToolResult; try { result = await this.options.executor.invoke(message.request, controller.signal); } finally { + clearTimeout(timer); this.active.delete(message.request.requestId); } if (this.closed) return; @@ -286,7 +328,10 @@ export class EncryptedRemoteWorkspaceExecutorEndpoint { if (this.closed) return; this.closed = true; this.reassembler.clear(); - for (const controller of this.active.values()) controller.abort(); + for (const active of this.active.values()) { + clearTimeout(active.timer); + active.controller.abort(); + } this.active.clear(); this.options.cipher.destroy(); } diff --git a/structure/decisions/ADR-0108-remote-workspace-rpc-deadlines.md b/structure/decisions/ADR-0108-remote-workspace-rpc-deadlines.md new file mode 100644 index 00000000000..7514b6d4a42 --- /dev/null +++ b/structure/decisions/ADR-0108-remote-workspace-rpc-deadlines.md @@ -0,0 +1,12 @@ +# ADR-0108 — decision recorded under "Remote Workspace" + +- Contract owner: [remote-workspace.md](../remote-workspace.md) + +## Decision record + +- 목적과 의도: Ensure a coordinator timeout cannot leave a queued workspace mutation authorized to run later, while allowing the documented 60-second exec ceiling to return normally. +- 기존 구현 및 제약 조건: The coordinator discarded only its pending result after 30 seconds. Executor operations serialize behind one queue, and their abort controllers previously lived only at the endpoint with no request deadline or timeout signal from the coordinator. +- 검토한 주요 대안: Delete late responses only; give each tool an independent queue; use an absolute wall-clock timestamp; send cancellation alone; or combine a bounded relative lifetime with an authenticated cancel frame. +- 선택한 방식: Carry the transport timeout on every encrypted request, start an endpoint abort timer on receipt, check the signal after dequeue through the existing executor boundary, and send a best-effort encrypted cancel frame when the coordinator timer fires. Set the default transport window to 65 seconds and cap negotiated values at 120 seconds. +- 다른 대안 대신 이 방식을 선택한 이유: Relative lifetimes avoid cross-device clock assumptions and cover cancellation frames that are delayed or lost. The cancel frame shortens active work when delivery succeeds, while the request deadline independently prevents queued post-timeout writes. +- 장점, 단점 및 영향: Timed-out queued mutations do not execute, supported commands can use their full 60-second limit, and timeout text no longer claims confirmed cancellation. A non-cooperative running command still depends on its runner honoring AbortSignal, and mixed implementations fail closed rather than silently accepting a request without a lifetime. diff --git a/structure/remote-workspace.md b/structure/remote-workspace.md index 40e92ff39e8..16d93557fdf 100644 --- a/structure/remote-workspace.md +++ b/structure/remote-workspace.md @@ -6,6 +6,13 @@ `src/remote-control/workspace-agent-connection.ts` intersects presence with enrollment authority and negotiates explicit session grants. `src/remote-control/workspace-rpc.ts` snapshots session/device/root/capabilities and rejects mismatches before invoking the executor. The paired Hub is trusted to select an approved root over authenticated WSS; workspace control traffic is not an untrusted opaque relay protocol. +Each encrypted RPC request carries its bounded executor lifetime. The executor starts that deadline +on receipt, aborts queued work before dequeue can mutate, and also accepts an authenticated cancel +frame when the coordinator stops waiting. The default 65-second RPC window exceeds the supported +60-second command ceiling; a timeout reports cancellation as requested rather than confirmed. + +> Decision record: [ADR-0108](decisions/ADR-0108-remote-workspace-rpc-deadlines.md) + `src/remote-control/workspace-executor.ts` checks approved root identity, relative paths, file size and write preconditions. File reads and write preconditions open descriptors nonblocking before verifying regular-file identity, so special files cannot wait for a peer during open. Its optional command runner lives in `src/remote-control/workspace-command-runner.ts`. Linux uses bubblewrap outside writable workspace roots and checks executable/parent permissions before invocation. The official Windows and macOS native helpers refuse commands; file tools remain independent of command availability. `src/remote-control/workspace-hub.ts`, `src/remote-control/workspace-device.ts` and `src/remote-control/workspace-sessions.ts` own separate persisted state. `src/remote-control/workspace-secret-store.ts` requires private permissions and rejects access failures rather than treating them as first-run absence. Publication reuses `src/config/atomic-write.ts`; workspace file publication uses the remote-workspace publisher in `src/lib/windows-atomic-replace.ts`. diff --git a/tests/clients/remote-workspace-session-binding.test.ts b/tests/clients/remote-workspace-session-binding.test.ts index ac1ab7c7c38..8f090a7192e 100644 --- a/tests/clients/remote-workspace-session-binding.test.ts +++ b/tests/clients/remote-workspace-session-binding.test.ts @@ -39,7 +39,7 @@ function fixture() { invocations, async send(overrides: Partial = {}) { const message = new TextEncoder().encode(JSON.stringify({ - version: 1, kind: "request", request: { ...request, ...overrides }, + version: 1, kind: "request", timeoutMs: 5_000, request: { ...request, ...overrides }, })); for (const frame of frameRemoteWorkspaceRpcMessage(message)) { await endpoint.receiveCiphertext(client.encrypt(frame)); diff --git a/tests/clients/remote-workspace.test.ts b/tests/clients/remote-workspace.test.ts index a639c848f83..d7386fd3b90 100644 --- a/tests/clients/remote-workspace.test.ts +++ b/tests/clients/remote-workspace.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { createHash, randomUUID } from "node:crypto"; import { execFileSync, spawnSync } from "node:child_process"; import { + existsSync, mkdirSync, linkSync, mkdtempSync, @@ -501,4 +502,97 @@ describe("remote workspace coordinator and executor", () => { client.close(); endpoint.close(); }); + + test("a timed-out mutation queued behind a command is cancelled before dequeue", async () => { + const state = fixture(); + const account = generateRemoteControlIdentityKeyPair(); + const device = generateRemoteControlIdentityKeyPair(); + const cryptoDeviceId = randomUUID(); + const cryptoSessionId = randomUUID(); + const clientHandshake = RemoteControlClientHandshake.create({ + sessionId: cryptoSessionId, + deviceId: cryptoDeviceId, + commandProfile: "codex", + capabilities: ["workspace.read", "workspace.write", "workspace.exec"], + accountPrivateKey: account.privateKey, + }); + const accepted = acceptRemoteControlClientHello(clientHandshake.hello, { + expectedSessionId: cryptoSessionId, + expectedDeviceId: cryptoDeviceId, + accountPublicKey: account.publicKey, + devicePrivateKey: device.privateKey, + allowedCapabilities: ["workspace.read", "workspace.write", "workspace.exec"], + }); + const clientCipher = clientHandshake.complete(accepted.hello, device.publicKey); + let releaseCommand!: () => void; + let commandStarted!: () => void; + const commandGate = new Promise(resolvePromise => { releaseCommand = resolvePromise; }); + const started = new Promise(resolvePromise => { commandStarted = resolvePromise; }); + const runner: RemoteWorkspaceCommandRunner = { + async run() { + commandStarted(); + // Deliberately ignore AbortSignal: queued operations must still observe their own abort + // after this non-cooperative predecessor finally releases the shared executor queue. + await commandGate; + return { exitCode: 0, stdout: "", stderr: "" }; + }, + }; + + let client: EncryptedRemoteWorkspaceTransport; + let endpoint: EncryptedRemoteWorkspaceExecutorEndpoint; + let responses = 0; + let allResponses!: () => void; + const responsesDone = new Promise(resolvePromise => { allResponses = resolvePromise; }); + client = new EncryptedRemoteWorkspaceTransport({ + executorDeviceId: `device-${cryptoDeviceId}`, + cipher: clientCipher, + // A real WebSocket send settles after queueing bytes, not after remote execution. + sendCiphertext: value => { void endpoint.receiveCiphertext(value); }, + timeoutMs: 100, + }); + endpoint = new EncryptedRemoteWorkspaceExecutorEndpoint({ + executorDeviceId: `device-${cryptoDeviceId}`, + sessionId: cryptoSessionId, + rootId: "project-root", + capabilities: ["workspace.read", "workspace.write", "workspace.exec"], + cipher: accepted.cipher, + executor: new RemoteWorkspaceExecutor({ + deviceId: `device-${cryptoDeviceId}`, + roots: [{ id: "project-root", path: state.executorRoot }], + commandRunner: runner, + }), + sendCiphertext: value => { + client.receiveCiphertext(value); + responses++; + if (responses === 2) allResponses(); + }, + }); + + const first = client.invoke({ + requestId: randomUUID(), sessionId: cryptoSessionId, + executorDeviceId: `device-${cryptoDeviceId}`, rootId: "project-root", + tool: "exec", arguments: { command: ["ignored"], timeoutMs: 60_000 }, + }).catch(error => error as Error); + await started; + const target = join(state.executorRoot, "project", "after-timeout.txt"); + const queued = client.invoke({ + requestId: randomUUID(), sessionId: cryptoSessionId, + executorDeviceId: `device-${cryptoDeviceId}`, rootId: "project-root", + tool: "write_file", + arguments: { path: "project/after-timeout.txt", content: "must-not-run", expectedSha256: null }, + }).catch(error => error as Error); + + const queuedFailure = await queued; + expect(queuedFailure).toBeInstanceOf(Error); + expect((queuedFailure as Error).message).toContain("cancellation was requested"); + expect(existsSync(target)).toBe(false); + releaseCommand(); + const firstFailure = await first; + expect(firstFailure).toBeInstanceOf(Error); + expect((firstFailure as Error).message).toContain("cancellation was requested"); + await responsesDone; + expect(existsSync(target)).toBe(false); + client.close(); + endpoint.close(); + }); }); From e091bc835aad7add50470f465b5a4879b11daa43 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Fri, 25 Sep 2026 11:07:56 +0000 Subject: [PATCH 2/3] fix(remote): gate RPC execution after delivery --- src/remote-control/workspace-rpc.ts | 86 +++++++++++++++---- ...-0121-remote-workspace-execution-grants.md | 12 +++ structure/remote-workspace.md | 16 +++- .../remote-workspace-session-binding.test.ts | 28 +++++- tests/clients/remote-workspace.test.ts | 63 ++++++++++++++ 5 files changed, 181 insertions(+), 24 deletions(-) create mode 100644 structure/decisions/ADR-0121-remote-workspace-execution-grants.md diff --git a/src/remote-control/workspace-rpc.ts b/src/remote-control/workspace-rpc.ts index eeba1d5c3a4..fff503d01d4 100644 --- a/src/remote-control/workspace-rpc.ts +++ b/src/remote-control/workspace-rpc.ts @@ -16,13 +16,14 @@ import { frameRemoteWorkspaceRpcMessage, } from "./workspace-rpc-framing"; -const REMOTE_WORKSPACE_RPC_VERSION = 1 as const; +// Old endpoints execute request frames immediately and cannot safely participate in prepare/grant. +const REMOTE_WORKSPACE_RPC_VERSION = 2 as const; const REMOTE_WORKSPACE_RPC_DEFAULT_TIMEOUT_MS = 65_000; const REMOTE_WORKSPACE_RPC_MAX_TIMEOUT_MS = 120_000; const REMOTE_WORKSPACE_RPC_MAX_ACTIVE_REQUESTS = 8; interface RemoteWorkspaceRpcRequest { version: typeof REMOTE_WORKSPACE_RPC_VERSION; - kind: "request"; + kind: "prepare"; timeoutMs: number; request: RemoteWorkspaceExecutionRequest; } @@ -33,6 +34,12 @@ interface RemoteWorkspaceRpcCancel { requestId: string; } +interface RemoteWorkspaceRpcGrant { + version: typeof REMOTE_WORKSPACE_RPC_VERSION; + kind: "grant"; + requestId: string; +} + interface RemoteWorkspaceRpcResponse { version: typeof REMOTE_WORKSPACE_RPC_VERSION; kind: "response"; @@ -40,7 +47,7 @@ interface RemoteWorkspaceRpcResponse { result: RemoteWorkspaceToolResult; } -type RemoteWorkspaceRpcMessage = RemoteWorkspaceRpcRequest | RemoteWorkspaceRpcResponse | RemoteWorkspaceRpcCancel; +type RemoteWorkspaceRpcMessage = RemoteWorkspaceRpcRequest | RemoteWorkspaceRpcResponse | RemoteWorkspaceRpcCancel | RemoteWorkspaceRpcGrant; interface PendingRequest { resolve(value: RemoteWorkspaceToolResult): void; @@ -109,11 +116,11 @@ function parseMessage(value: Uint8Array): RemoteWorkspaceRpcMessage { if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) throw new Error("invalid remote workspace RPC message"); const raw = parsed as Record; if (raw.version !== REMOTE_WORKSPACE_RPC_VERSION) throw new Error("unsupported remote workspace RPC version"); - if (raw.kind === "request" && Number.isSafeInteger(raw.timeoutMs) + if (raw.kind === "prepare" && Number.isSafeInteger(raw.timeoutMs) && (raw.timeoutMs as number) >= 1 && (raw.timeoutMs as number) <= REMOTE_WORKSPACE_RPC_MAX_TIMEOUT_MS) { return { version: REMOTE_WORKSPACE_RPC_VERSION, - kind: "request", + kind: "prepare", timeoutMs: raw.timeoutMs as number, request: parseRequest(raw.request), }; @@ -126,8 +133,8 @@ function parseMessage(value: Uint8Array): RemoteWorkspaceRpcMessage { result: parseResult(raw.result), }; } - if (raw.kind === "cancel" && boundedIdentifier(raw.requestId)) { - return { version: REMOTE_WORKSPACE_RPC_VERSION, kind: "cancel", requestId: raw.requestId }; + if ((raw.kind === "cancel" || raw.kind === "grant") && boundedIdentifier(raw.requestId)) { + return { version: REMOTE_WORKSPACE_RPC_VERSION, kind: raw.kind, requestId: raw.requestId }; } throw new Error("invalid remote workspace RPC message kind"); } @@ -173,19 +180,33 @@ export class EncryptedRemoteWorkspaceTransport implements RemoteWorkspaceTranspo }, this.timeoutMs); this.pending.set(request.requestId, { resolve, reject, timer }); }); + const pending = this.pending.get(request.requestId)!; + // Observe the response immediately: transport backpressure must not defer timeout delivery + // or leave a rejected response promise unobserved while a write is still waiting to settle. + void this.prepareAndGrant(request, pending); + return await response; + } + + private async prepareAndGrant(request: RemoteWorkspaceExecutionRequest, pending: PendingRequest): Promise { try { await this.sendMessage(encodeMessage({ version: REMOTE_WORKSPACE_RPC_VERSION, - kind: "request", + kind: "prepare", timeoutMs: this.timeoutMs, request, })); + // Check again inside the serialized send queue: another write can delay this grant after + // prepare has settled. A request that has already timed out must never receive a grant. + await this.sendMessage(encodeMessage({ + version: REMOTE_WORKSPACE_RPC_VERSION, + kind: "grant", + requestId: request.requestId, + }), () => this.pending.get(request.requestId) === pending); } catch { // A failed encrypted write consumes a directional counter. Continuing would make every // later frame undecryptable, so fail every pending operation instead of waiting for timeout. this.close("remote workspace send failed"); } - return await response; } receiveCiphertext(value: Uint8Array): void { @@ -213,8 +234,9 @@ export class EncryptedRemoteWorkspaceTransport implements RemoteWorkspaceTranspo this.pending.clear(); } - private sendMessage(message: Uint8Array): Promise { + private sendMessage(message: Uint8Array, shouldSend: () => boolean = () => true): Promise { const operation = this.sendTail.then(async () => { + if (!shouldSend()) return; if (!this.online) throw new Error("remote workspace transport is closed"); for (const frame of frameRemoteWorkspaceRpcMessage(message)) { await this.options.sendCiphertext(this.options.cipher.encrypt(frame)); @@ -254,6 +276,8 @@ export class EncryptedRemoteWorkspaceExecutorEndpoint { private readonly active = new Map; + request: RemoteWorkspaceExecutionRequest; + started: boolean; }>(); private readonly reassembler = new RemoteWorkspaceRpcReassembler(); private sendTail: Promise = Promise.resolve(); @@ -277,10 +301,25 @@ export class EncryptedRemoteWorkspaceExecutorEndpoint { if (!requestPlaintext) return; const message = parseMessage(requestPlaintext); if (message.kind === "cancel") { - this.active.get(message.requestId)?.controller.abort(); + const active = this.active.get(message.requestId); + if (active) { + active.controller.abort(); + if (!active.started) { + clearTimeout(active.timer); + this.active.delete(message.requestId); + } + } return; } - if (message.kind !== "request") throw new Error("executor received a remote workspace response"); + if (message.kind === "grant") { + const active = this.active.get(message.requestId); + if (!active || active.controller.signal.aborted) return; + if (active.started) throw new Error("duplicate remote workspace execution grant"); + active.started = true; + await this.executeGranted(active.request, active.controller, active.timer); + return; + } + if (message.kind !== "prepare") throw new Error("executor received a remote workspace response"); if (message.request.executorDeviceId !== this.options.executorDeviceId) { throw new Error("remote workspace encrypted request targeted another executor"); } @@ -295,14 +334,25 @@ export class EncryptedRemoteWorkspaceExecutorEndpoint { throw new Error("remote workspace executor request limit reached"); } const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), message.timeoutMs); - this.active.set(message.request.requestId, { controller, timer }); + const timer = setTimeout(() => { + controller.abort(); + const active = this.active.get(message.request.requestId); + if (active && !active.started) this.active.delete(message.request.requestId); + }, message.timeoutMs); + this.active.set(message.request.requestId, { controller, timer, request: message.request, started: false }); + } + + private async executeGranted( + request: RemoteWorkspaceExecutionRequest, + controller: AbortController, + timer: ReturnType, + ): Promise { let result: RemoteWorkspaceToolResult; try { - result = await this.options.executor.invoke(message.request, controller.signal); + result = await this.options.executor.invoke(request, controller.signal); } finally { clearTimeout(timer); - this.active.delete(message.request.requestId); + this.active.delete(request.requestId); } if (this.closed) return; let responsePlaintext: Uint8Array; @@ -310,14 +360,14 @@ export class EncryptedRemoteWorkspaceExecutorEndpoint { responsePlaintext = encodeMessage({ version: REMOTE_WORKSPACE_RPC_VERSION, kind: "response", - requestId: message.request.requestId, + requestId: request.requestId, result, }); } catch { responsePlaintext = encodeMessage({ version: REMOTE_WORKSPACE_RPC_VERSION, kind: "response", - requestId: message.request.requestId, + requestId: request.requestId, result: { ok: false, error: "remote workspace result exceeded the encrypted frame limit" }, }); } diff --git a/structure/decisions/ADR-0121-remote-workspace-execution-grants.md b/structure/decisions/ADR-0121-remote-workspace-execution-grants.md new file mode 100644 index 00000000000..3bf7043f733 --- /dev/null +++ b/structure/decisions/ADR-0121-remote-workspace-execution-grants.md @@ -0,0 +1,12 @@ +# ADR-0121 — decision recorded under "Remote Workspace" + +- Contract owner: [remote-workspace.md](../remote-workspace.md) + +## Decision record + +- 목적과 의도: Prevent a request whose prepare send remains backpressured through coordinator timeout from acquiring execution authority when those bytes arrive later. +- 기존 구현 및 제약 조건: ADR-0108 starts a relative executor lifetime upon request receipt. Delayed delivery can restart that lifetime after the coordinator stops waiting, before the ordered cancellation arrives. Device wall clocks are not assumed synchronized. +- 검토한 주요 대안: Absolute timestamps require clock assumptions; cancellation alone loses the delayed-receipt race; immediate execution cannot distinguish a still-pending coordinator from an expired one. +- 선택한 방식: RPC v2 separates authenticated prepare from grant. Prepare validates and retains bounded request state but cannot invoke. After prepare send completion, a grant is admitted only while the same pending request remains live, with the check inside the serialized send queue. Cancellation and expiry discard ungranted state; the existing abort signal owns granted work. RPC v1 is rejected without downgrade; encrypted framing is unchanged. +- 다른 대안 대신 이 방식을 선택한 이유: The grant check closes the prepare-backpressure race without a shared clock, preserves directional encryption order, and prevents old immediate-execution endpoints from silently bypassing the new contract. +- 장점, 단점 및 영향: Timeout delivery no longer waits for a blocked send. Prepare-only requests never enter the executor. This adds one authenticated message and requires both peers to upgrade. A grant already sent can itself be delayed, and a running operation may already have committed; timeout remains an unknown outcome with cancellation requested, not proof of rollback or universal post-timeout non-execution. diff --git a/structure/remote-workspace.md b/structure/remote-workspace.md index 16d93557fdf..8d427f0340f 100644 --- a/structure/remote-workspace.md +++ b/structure/remote-workspace.md @@ -6,13 +6,21 @@ `src/remote-control/workspace-agent-connection.ts` intersects presence with enrollment authority and negotiates explicit session grants. `src/remote-control/workspace-rpc.ts` snapshots session/device/root/capabilities and rejects mismatches before invoking the executor. The paired Hub is trusted to select an approved root over authenticated WSS; workspace control traffic is not an untrusted opaque relay protocol. -Each encrypted RPC request carries its bounded executor lifetime. The executor starts that deadline -on receipt, aborts queued work before dequeue can mutate, and also accepts an authenticated cancel -frame when the coordinator stops waiting. The default 65-second RPC window exceeds the supported -60-second command ceiling; a timeout reports cancellation as requested rather than confirmed. +Encrypted RPC v2 prepares a request with a bounded executor lifetime without invoking it. Only a +separate authenticated grant admits execution. The coordinator sends that grant after prepare +delivery settles, checking the original pending request again when the serialized grant send starts. +A prepare whose send is still backpressured at coordinator timeout therefore cannot execute later. +The endpoint starts its relative deadline on prepare receipt, never resets it on grant, and removes +ungranted requests on cancellation or expiry. Granted work receives the same abort signal through +the executor queue. The default 65-second RPC window exceeds the supported 60-second command ceiling. +Timeout requests cancellation but does not confirm it: an already-sent grant can still be delayed +in transit or its operation can already be running. The wire framing and encryption are unchanged; +RPC v1 peers fail closed and must upgrade together rather than fall back to immediate execution. > Decision record: [ADR-0108](decisions/ADR-0108-remote-workspace-rpc-deadlines.md) +> Decision record: [ADR-0121](decisions/ADR-0121-remote-workspace-execution-grants.md) + `src/remote-control/workspace-executor.ts` checks approved root identity, relative paths, file size and write preconditions. File reads and write preconditions open descriptors nonblocking before verifying regular-file identity, so special files cannot wait for a peer during open. Its optional command runner lives in `src/remote-control/workspace-command-runner.ts`. Linux uses bubblewrap outside writable workspace roots and checks executable/parent permissions before invocation. The official Windows and macOS native helpers refuse commands; file tools remain independent of command availability. `src/remote-control/workspace-hub.ts`, `src/remote-control/workspace-device.ts` and `src/remote-control/workspace-sessions.ts` own separate persisted state. `src/remote-control/workspace-secret-store.ts` requires private permissions and rejects access failures rather than treating them as first-run absence. Publication reuses `src/config/atomic-write.ts`; workspace file publication uses the remote-workspace publisher in `src/lib/windows-atomic-replace.ts`. diff --git a/tests/clients/remote-workspace-session-binding.test.ts b/tests/clients/remote-workspace-session-binding.test.ts index 8f090a7192e..04ba60e0d4e 100644 --- a/tests/clients/remote-workspace-session-binding.test.ts +++ b/tests/clients/remote-workspace-session-binding.test.ts @@ -37,13 +37,21 @@ function fixture() { }; return { invocations, - async send(overrides: Partial = {}) { + async send(overrides: Partial = {}, grant = true, version = 2) { const message = new TextEncoder().encode(JSON.stringify({ - version: 1, kind: "request", timeoutMs: 5_000, request: { ...request, ...overrides }, + version, kind: version === 1 ? "request" : "prepare", timeoutMs: 5_000, request: { ...request, ...overrides }, })); for (const frame of frameRemoteWorkspaceRpcMessage(message)) { await endpoint.receiveCiphertext(client.encrypt(frame)); } + if (grant) { + const grantMessage = new TextEncoder().encode(JSON.stringify({ + version, kind: "grant", requestId: overrides.requestId ?? request.requestId, + })); + for (const frame of frameRemoteWorkspaceRpcMessage(grantMessage)) { + await endpoint.receiveCiphertext(client.encrypt(frame)); + } + } }, close() { endpoint.close(); client.destroy(); }, }; @@ -65,6 +73,22 @@ test("encrypted requests cannot leave their session grant before executor invoca } }); +test("an encrypted prepare cannot invoke without an execution grant", async () => { + const state = fixture(); + try { + await state.send({}, false); + expect(state.invocations).toEqual([]); + } finally { state.close(); } +}); + +test("legacy immediate-execution RPC requests fail closed", async () => { + const state = fixture(); + try { + await expect(state.send({}, false, 1)).rejects.toThrow("unsupported remote workspace RPC version"); + expect(state.invocations).toEqual([]); + } finally { state.close(); } +}); + test("a matching encrypted read reaches the selected executor once", async () => { const state = fixture(); try { diff --git a/tests/clients/remote-workspace.test.ts b/tests/clients/remote-workspace.test.ts index d7386fd3b90..a03d536ec02 100644 --- a/tests/clients/remote-workspace.test.ts +++ b/tests/clients/remote-workspace.test.ts @@ -503,6 +503,69 @@ describe("remote workspace coordinator and executor", () => { endpoint.close(); }); + test("a prepare delayed by send backpressure never grants a timed-out mutation", async () => { + const state = fixture(); + const account = generateRemoteControlIdentityKeyPair(); + const device = generateRemoteControlIdentityKeyPair(); + const deviceId = randomUUID(); + const sessionId = randomUUID(); + const handshake = RemoteControlClientHandshake.create({ + sessionId, deviceId, commandProfile: "codex", capabilities: ["workspace.write"], + accountPrivateKey: account.privateKey, + }); + const accepted = acceptRemoteControlClientHello(handshake.hello, { + expectedSessionId: sessionId, expectedDeviceId: deviceId, + accountPublicKey: account.publicKey, devicePrivateKey: device.privateKey, + allowedCapabilities: ["workspace.write"], + }); + let releaseSend!: () => void; + const backpressure = new Promise(resolve => { releaseSend = resolve; }); + let markDrained!: () => void; + const drained = new Promise(resolve => { markDrained = resolve; }); + let endpoint!: EncryptedRemoteWorkspaceExecutorEndpoint; + let sent = 0; + const client = new EncryptedRemoteWorkspaceTransport({ + executorDeviceId: deviceId, cipher: handshake.complete(accepted.hello, device.publicKey), + timeoutMs: 50, + async sendCiphertext(value) { + sent++; + if (sent === 1) await backpressure; + await endpoint.receiveCiphertext(value); + if (sent === 2) markDrained(); + }, + }); + let invocations = 0; + const executor = new RemoteWorkspaceExecutor({ + deviceId, roots: [{ id: "project-root", path: state.executorRoot }], + }); + endpoint = new EncryptedRemoteWorkspaceExecutorEndpoint({ + executorDeviceId: deviceId, sessionId, rootId: "project-root", + capabilities: ["workspace.write"], cipher: accepted.cipher, + executor: { invoke(request, signal) { invocations++; return executor.invoke(request, signal); } }, + sendCiphertext: value => client.receiveCiphertext(value), + }); + const target = join(state.executorRoot, "project", "delayed-prepare.txt"); + try { + await expect(client.invoke({ + requestId: randomUUID(), sessionId, executorDeviceId: deviceId, rootId: "project-root", + tool: "write_file", + arguments: { path: "project/delayed-prepare.txt", content: "must-not-run", expectedSha256: null }, + })).rejects.toThrow("cancellation was requested"); + expect(sent).toBe(1); + releaseSend(); + await drained; + // Let the queued grant admission run after the cancellation send has drained. + await new Promise(resolve => setTimeout(resolve, 0)); + expect(sent).toBe(2); + expect(invocations).toBe(0); + expect(existsSync(target)).toBe(false); + } finally { + releaseSend(); + client.close(); + endpoint.close(); + } + }); + test("a timed-out mutation queued behind a command is cancelled before dequeue", async () => { const state = fixture(); const account = generateRemoteControlIdentityKeyPair(); From f19ef3dbfe4f90ddf51d98baf82155a5c9ec1650 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Fri, 25 Sep 2026 11:23:34 +0000 Subject: [PATCH 3/3] docs(remote): describe RPC v2 timeout contract --- docs-site/src/content/docs/guides/remote-workspace.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs-site/src/content/docs/guides/remote-workspace.md b/docs-site/src/content/docs/guides/remote-workspace.md index 33db5f4eace..84ab94e12b0 100644 --- a/docs-site/src/content/docs/guides/remote-workspace.md +++ b/docs-site/src/content/docs/guides/remote-workspace.md @@ -27,6 +27,16 @@ lifecycle owner can retain cleanup authority through cancellation. Missing comma falls back to executing on the Hub. ::: +## RPC compatibility and timeouts + +Remote Workspace uses encrypted RPC v2. The Hub and every Executor must support v2; RPC v1 peers +fail closed instead of falling back to immediate execution, so upgrade the Hub and Executors +together. + +A timeout requests executor cancellation but does not confirm it. A grant may already be in transit, +or its operation may already be running. The default RPC timeout is 65 seconds, and `timeoutMs` +accepts inclusive values from 1 through 120,000 milliseconds. + ## Set up the Hub Computer 1 owns every coding-agent login and model session. Install and log in to whichever agents