From ffda536612182cb15497f4210e3b2155fef57061 Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Fri, 30 Oct 2026 22:04:29 +0000 Subject: [PATCH 1/2] fix(sdk/ts): revoke forward tokens on close Signed-off-by: Eric Curtin --- sdk/typescript/src/client.test.ts | 37 +++++++++++++++++++++++++++++++ sdk/typescript/src/client.ts | 9 +++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/sdk/typescript/src/client.test.ts b/sdk/typescript/src/client.test.ts index 86fdfa3972..3c1d189477 100644 --- a/sdk/typescript/src/client.test.ts +++ b/sdk/typescript/src/client.test.ts @@ -2236,6 +2236,43 @@ describe('forward', () => { expect(revokedToken).toBe('fwd-tok'); }); + it('revokes the token of a connection still open at close()', async () => { + const revoked: string[] = []; + const sandbox = client({ + getSandbox: () => readySandbox('sb', 'sb-id-open'), + createSshSession: () => ({ + sandboxId: 'sb-id-open', + token: 'open-tok', + gatewayHost: 'gw', + gatewayPort: 443, + gatewayScheme: 'https', + hostKeyFingerprint: '', + expirationTime: undefined, + }), + // A real transport never delivers a call whose signal is already aborted. + revokeSshSession: (req, ctx) => { + if (!ctx.signal.aborted) revoked.push(req.token); + return { outcome: 1 }; + }, + forwardTcp: async function* (requests) { + for await (const frame of requests) { + if (frame.payload.case === 'data') yield { payload: { case: 'data', value: frame.payload.value } }; + } + }, + }); + + const handle = await sandbox.forward('sb', { targetPort: 9000 }); + const socket = net.connect(handle.localPort, handle.localHost); + socket.on('error', () => {}); + await new Promise((resolve) => { + socket.once('data', () => resolve()); + socket.write('ping'); + }); + + await handle.close(); + expect(revoked).toEqual(['open-tok']); + }); + it('rejects when the sandbox is not ready', async () => { const sandbox = client({ getSandbox: () => ({ diff --git a/sdk/typescript/src/client.ts b/sdk/typescript/src/client.ts index f82242d4a7..e6e2acc255 100644 --- a/sdk/typescript/src/client.ts +++ b/sdk/typescript/src/client.ts @@ -723,6 +723,9 @@ function versionPin(value: string | undefined): bigint { const FORWARD_CHUNK = 64 * 1024; +// Bound for revoking a forward's SSH session token during cleanup. +const SSH_CLEANUP_TIMEOUT_MS = 5_000; + function workspaceName(options?: SandboxWorkspaceOptions | null): string { const workspace = options?.workspace ?? 'default'; if (workspace.trim() === '') throw new SdkError('invalid_config', 'workspace must be non-empty'); @@ -1894,7 +1897,11 @@ export class SandboxClient { input.end(); if (token !== undefined) { try { - await this.grpc.revokeSshSession({ token, allowMissing: true }, { signal }); + // Not the connection signal: close() aborts it before this runs. + await this.grpc.revokeSshSession( + { token, allowMissing: true }, + { signal: AbortSignal.timeout(SSH_CLEANUP_TIMEOUT_MS) }, + ); } catch { // Best-effort revoke; the token expires on its own regardless. } From 09b24e9d8d159664f0b8e3b41bc0f151d46c559a Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Mon, 2 Nov 2026 21:33:44 +0000 Subject: [PATCH 2/2] test(sdk/ts): cover forward close with stalled revoke Signed-off-by: Eric Curtin --- sdk/typescript/src/client.test.ts | 39 +++++++++++++++++++++++++++++++ sdk/typescript/src/client.ts | 11 +++++++-- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/sdk/typescript/src/client.test.ts b/sdk/typescript/src/client.test.ts index 3c1d189477..756ff6e44d 100644 --- a/sdk/typescript/src/client.test.ts +++ b/sdk/typescript/src/client.test.ts @@ -12,6 +12,7 @@ import type { MessageInitShape } from '@bufbuild/protobuf'; import { Code, ConnectError, createRouterTransport, type ServiceImpl, type Transport } from '@connectrpc/connect'; import { afterEach, describe, expect, it } from 'vitest'; import { + __setSshCleanupTimeoutMsForTests, __setWatchLogsBackoffSleepForTests, errorCode, Pager, @@ -2273,6 +2274,44 @@ describe('forward', () => { expect(revoked).toEqual(['open-tok']); }); + it('close() resolves within the cleanup bound when revoke never resolves', async () => { + __setSshCleanupTimeoutMsForTests(100); + try { + const sandbox = client({ + getSandbox: () => readySandbox('sb', 'sb-id-stall'), + createSshSession: () => ({ + sandboxId: 'sb-id-stall', + token: 'stall-tok', + gatewayHost: 'gw', + gatewayPort: 443, + gatewayScheme: 'https', + hostKeyFingerprint: '', + expirationTime: undefined, + }), + revokeSshSession: () => new Promise(() => {}), + forwardTcp: async function* (requests) { + for await (const frame of requests) { + if (frame.payload.case === 'data') yield { payload: { case: 'data', value: frame.payload.value } }; + } + }, + }); + + const handle = await sandbox.forward('sb', { targetPort: 9000 }); + const socket = net.connect(handle.localPort, handle.localHost); + socket.on('error', () => {}); + await new Promise((resolve) => { + socket.once('data', () => resolve()); + socket.write('ping'); + }); + + const start = Date.now(); + await handle.close(); + expect(Date.now() - start).toBeLessThan(2_000); + } finally { + __setSshCleanupTimeoutMsForTests(); + } + }); + it('rejects when the sandbox is not ready', async () => { const sandbox = client({ getSandbox: () => ({ diff --git a/sdk/typescript/src/client.ts b/sdk/typescript/src/client.ts index e6e2acc255..1a92768e5b 100644 --- a/sdk/typescript/src/client.ts +++ b/sdk/typescript/src/client.ts @@ -485,7 +485,8 @@ export interface ForwardOptions extends SandboxWorkspaceOptions { // A process-lifetime local listener that tunnels each accepted connection into // the sandbox. Call `close()` on teardown; `closed` resolves once the listener -// is fully torn down. An in-process forward cannot outlive the Node process. +// is fully torn down. `close()` may wait up to 5s for best-effort revokes of +// open connections' tokens. An in-process forward cannot outlive the Node process. export interface ForwardHandle { localHost: string; localPort: number; @@ -725,6 +726,12 @@ const FORWARD_CHUNK = 64 * 1024; // Bound for revoking a forward's SSH session token during cleanup. const SSH_CLEANUP_TIMEOUT_MS = 5_000; +let sshCleanupTimeoutMs = SSH_CLEANUP_TIMEOUT_MS; + +// Test hook; not re-exported from index.ts. No argument restores the default. +export function __setSshCleanupTimeoutMsForTests(ms?: number): void { + sshCleanupTimeoutMs = ms ?? SSH_CLEANUP_TIMEOUT_MS; +} function workspaceName(options?: SandboxWorkspaceOptions | null): string { const workspace = options?.workspace ?? 'default'; @@ -1900,7 +1907,7 @@ export class SandboxClient { // Not the connection signal: close() aborts it before this runs. await this.grpc.revokeSshSession( { token, allowMissing: true }, - { signal: AbortSignal.timeout(SSH_CLEANUP_TIMEOUT_MS) }, + { signal: AbortSignal.timeout(sshCleanupTimeoutMs) }, ); } catch { // Best-effort revoke; the token expires on its own regardless.