feat(core): Harden n8n sandbox service adapter (no-changelog) (#30813)

packages/@n8n/instance-ai/src/workspace/__tests__/n8n-sandbox-sandbox.test.ts

@@ -0,0 +1,264 @@
+import { SandboxServiceError } from '@n8n/sandbox-client';
+
+import { N8nSandboxServiceSandbox } from '../n8n-sandbox-sandbox';
+
+// ---------------------------------------------------------------------------
+// Mocks
+// ---------------------------------------------------------------------------
+
+const mockCreateSandbox = jest.fn();
+const mockGetSandbox = jest.fn();
+const mockDeleteSandbox = jest.fn();
+const mockExec = jest.fn();
+
+jest.mock('@n8n/sandbox-client', () => {
+	class MockSandboxServiceError extends Error {
+		readonly status: number;
+
+		constructor(message: string, status: number) {
+			super(message);
+			this.name = 'SandboxServiceError';
+			this.status = status;
+		}
+	}
+
+	return {
+		SandboxServiceError: MockSandboxServiceError,
+		SandboxClient: class {
+			createSandbox = mockCreateSandbox;
+			getSandbox = mockGetSandbox;
+			deleteSandbox = mockDeleteSandbox;
+			exec = mockExec;
+		},
+	};
+});
+
+// ---------------------------------------------------------------------------
+// Helpers
+// ---------------------------------------------------------------------------
+
+function makeDefaultOptions() {
+	return { apiKey: 'key', serviceUrl: 'https://sandbox.test' };
+}
+
+function makeSandboxRecord(overrides: Record<string, unknown> = {}) {
+	return {
+		id: 'sb-123',
+		status: 'running',
+		createdAt: 1700000000,
+		lastActiveAt: 1700000100,
+		...overrides,
+	};
+}
+
+function makeExecResult(overrides: Record<string, unknown> = {}) {
+	return {
+		exitCode: 0,
+		stdout: '',
+		stderr: '',
+		executionTimeMs: 42,
+		timedOut: false,
+		killed: false,
+		success: true,
+		...overrides,
+	};
+}
+
+// ---------------------------------------------------------------------------
+// Tests
+// ---------------------------------------------------------------------------
+
+beforeEach(() => {
+	jest.clearAllMocks();
+	mockCreateSandbox.mockResolvedValue(makeSandboxRecord());
+	mockGetSandbox.mockResolvedValue(makeSandboxRecord());
+	mockDeleteSandbox.mockResolvedValue(undefined);
+	mockExec.mockResolvedValue(makeExecResult({ stdout: '/home/user\n' }));
+});
+
+describe('destroy()', () => {
+	it('calls deleteSandbox when sandbox exists', async () => {
+		const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions());
+		await sandbox.start();
+		await sandbox.destroy();
+
+		expect(mockDeleteSandbox).toHaveBeenCalledWith('sb-123');
+	});
+
+	it('swallows 404 (sandbox already gone)', async () => {
+		mockDeleteSandbox.mockRejectedValue(new SandboxServiceError('not found', 404));
+
+		const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions());
+		await sandbox.start();
+
+		await expect(sandbox.destroy()).resolves.toBeUndefined();
+	});
+
+	it('re-throws non-404 errors', async () => {
+		mockDeleteSandbox.mockRejectedValue(new SandboxServiceError('server error', 500));
+
+		const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions());
+		await sandbox.start();
+
+		await expect(sandbox.destroy()).rejects.toThrow('server error');
+	});
+
+	it('is a no-op when no sandboxId is set', async () => {
+		const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions());
+		await sandbox.destroy();
+
+		expect(mockDeleteSandbox).not.toHaveBeenCalled();
+	});
+});
+
+describe('start()', () => {
+	describe('fresh creation (no existing ID)', () => {
+		it('creates a new sandbox', async () => {
+			const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions());
+			await sandbox.start();
+
+			expect(mockCreateSandbox).toHaveBeenCalledTimes(1);
+			expect(sandbox.id).toBe('sb-123');
+		});
+	});
+
+	describe('reconnect to existing ID', () => {
+		it('reconnects when sandbox exists', async () => {
+			const sandbox = new N8nSandboxServiceSandbox({
+				...makeDefaultOptions(),
+				id: 'existing-sb',
+			});
+			mockGetSandbox.mockResolvedValue(makeSandboxRecord({ id: 'existing-sb' }));
+
+			await sandbox.start();
+
+			expect(mockGetSandbox).toHaveBeenCalledWith('existing-sb');
+			expect(mockCreateSandbox).not.toHaveBeenCalled();
+			expect(sandbox.id).toBe('existing-sb');
+		});
+
+		it('creates new when getSandbox returns 404', async () => {
+			mockGetSandbox.mockRejectedValue(new SandboxServiceError('not found', 404));
+			mockCreateSandbox.mockResolvedValue(makeSandboxRecord({ id: 'new-sb' }));
+
+			const sandbox = new N8nSandboxServiceSandbox({
+				...makeDefaultOptions(),
+				id: 'gone-sb',
+			});
+			await sandbox.start();
+
+			expect(mockCreateSandbox).toHaveBeenCalledTimes(1);
+			expect(sandbox.id).toBe('new-sb');
+		});
+
+		it('re-throws non-404 errors from getSandbox', async () => {
+			mockGetSandbox.mockRejectedValue(new SandboxServiceError('forbidden', 403));
+
+			const sandbox = new N8nSandboxServiceSandbox({
+				...makeDefaultOptions(),
+				id: 'existing-sb',
+			});
+
+			await expect(sandbox.start()).rejects.toThrow('forbidden');
+		});
+	});
+});
+
+describe('getInstructions()', () => {
+	it('includes runtime description, working directory, and timeout', async () => {
+		const sandbox = new N8nSandboxServiceSandbox({
+			...makeDefaultOptions(),
+			timeout: 60_000,
+		});
+		await sandbox.start();
+
+		const instructions = sandbox.getInstructions();
+		expect(instructions).toContain('Cloud sandbox');
+		expect(instructions).toContain('TypeScript');
+		expect(instructions).toContain('Default working directory: /home/user/workspace');
+		expect(instructions).toContain('60s');
+	});
+});
+
+describe('executeCommand() env merging', () => {
+	it('merges constructor env with per-command env', async () => {
+		const sandbox = new N8nSandboxServiceSandbox({
+			...makeDefaultOptions(),
+			env: { BASE_KEY: 'base' },
+		});
+		await sandbox.start();
+		mockExec.mockResolvedValue(makeExecResult());
+
+		await sandbox.executeCommand('ls', [], { env: { CMD_KEY: 'cmd' } });
+
+		expect(mockExec).toHaveBeenLastCalledWith(
+			'sb-123',
+			expect.objectContaining({
+				env: { BASE_KEY: 'base', CMD_KEY: 'cmd' },
+			}),
+		);
+	});
+
+	it('per-command env overrides constructor env for same key', async () => {
+		const sandbox = new N8nSandboxServiceSandbox({
+			...makeDefaultOptions(),
+			env: { KEY: 'base' },
+		});
+		await sandbox.start();
+		mockExec.mockResolvedValue(makeExecResult());
+
+		await sandbox.executeCommand('ls', [], { env: { KEY: 'override' } });
+
+		expect(mockExec).toHaveBeenLastCalledWith(
+			'sb-123',
+			expect.objectContaining({
+				env: { KEY: 'override' },
+			}),
+		);
+	});
+
+	it('filters out undefined values from env', async () => {
+		const sandbox = new N8nSandboxServiceSandbox({
+			...makeDefaultOptions(),
+			env: { KEEP: 'yes' },
+		});
+		await sandbox.start();
+		mockExec.mockResolvedValue(makeExecResult());
+
+		await sandbox.executeCommand('ls', [], { env: { DROP: undefined } as NodeJS.ProcessEnv });
+
+		expect(mockExec).toHaveBeenLastCalledWith(
+			'sb-123',
+			expect.objectContaining({
+				env: { KEEP: 'yes' },
+			}),
+		);
+	});
+
+	it('passes undefined env when no env is configured', async () => {
+		const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions());
+		await sandbox.start();
+		mockExec.mockResolvedValue(makeExecResult());
+
+		await sandbox.executeCommand('ls');
+
+		expect(mockExec).toHaveBeenLastCalledWith(
+			'sb-123',
+			expect.objectContaining({ env: undefined }),
+		);
+	});
+});
+
+describe('getInfo()', () => {
+	it('includes locally tracked createdAt and workingDirectory', async () => {
+		const sandbox = new N8nSandboxServiceSandbox(makeDefaultOptions());
+		await sandbox.start();
+
+		const info = await sandbox.getInfo();
+		expect(info.createdAt).toBeInstanceOf(Date);
+		expect(info.metadata).toEqual(
+			expect.objectContaining({ workingDirectory: '/home/user/workspace' }),
+		);
+		expect(info.provider).toBe('n8n-sandbox');
+	});
+});

packages/@n8n/instance-ai/src/workspace/builder-sandbox-factory.ts

@@ -367,7 +367,7 @@ export class BuilderSandboxFactory {
 	}
 
 	private async createN8nSandbox(
-		_builderId: string,
+		builderId: string,
 		context: InstanceAiContext,
 	): Promise<BuilderWorkspace> {
 		const config = this.assertIsN8nSandbox();
@@ -419,6 +419,13 @@ export class BuilderSandboxFactory {
 		} catch (error) {
 			// If any step after sandbox creation throws (workspace init, catalog
 			// write, SDK link), destroy the remote sandbox so it isn't orphaned.
+			this.errorReporter?.error(error, {
+				tags: {
+					component: 'builder-sandbox-factory',
+					provider: 'n8n-sandbox',
+				},
+				extra: { builderId },
+			});
 			await destroySandbox();
 			throw error;
 		}

packages/@n8n/instance-ai/src/workspace/n8n-sandbox-sandbox.ts

@@ -5,7 +5,7 @@ import type {
 	SandboxInfo,
 } from '@n8n/agents';
 import { BaseSandbox } from '@n8n/agents';
-import { SandboxClient } from '@n8n/sandbox-client';
+import { SandboxClient, SandboxServiceError, type SandboxRecord } from '@n8n/sandbox-client';
 import assert from 'node:assert/strict';
 import { randomUUID } from 'node:crypto';
 
@@ -14,6 +14,7 @@ export interface N8nSandboxServiceSandboxOptions {
 	apiKey?: string;
 	serviceUrl?: string;
 	timeout?: number;
+	env?: Record<string, string>;
 }
 
 function shellEscape(value: string): string {
@@ -37,8 +38,16 @@ export class N8nSandboxServiceSandbox extends BaseSandbox {
 
 	private readonly client: SandboxClient;
 
+	private readonly timeout: number;
+
+	private static readonly HOME_DIR = '/home/user';
+
+	private static readonly WORKSPACE_DIR = `${N8nSandboxServiceSandbox.HOME_DIR}/workspace`;
+
 	private sandboxId?: string;
 
+	private createdAt?: Date;
+
 	constructor(private readonly options: N8nSandboxServiceSandboxOptions) {
 		super();
 		this.client = new SandboxClient({
@@ -46,6 +55,7 @@ export class N8nSandboxServiceSandbox extends BaseSandbox {
 			baseUrl: options.serviceUrl,
 		});
 		this.sandboxId = options.id;
+		this.timeout = options.timeout ?? 300_000;
 	}
 
 	get id(): string {
@@ -54,17 +64,26 @@ export class N8nSandboxServiceSandbox extends BaseSandbox {
 
 	override async start(): Promise<void> {
 		if (this.sandboxId) {
-			await this.client.getSandbox(this.sandboxId);
-			return;
+			const existing = await this.tryGetExistingSandbox(this.sandboxId);
+			if (existing) {
+				this.createdAt = new Date(existing.createdAt * 1000);
+				return;
+			}
 		}
 
 		const sandbox = await this.client.createSandbox();
 		this.sandboxId = sandbox.id;
+		this.createdAt = new Date();
 	}
 
 	override async destroy(): Promise<void> {
 		if (!this.sandboxId) return;
-		await this.client.deleteSandbox(this.sandboxId);
+		try {
+			await this.client.deleteSandbox(this.sandboxId);
+		} catch (error) {
+			if (error instanceof SandboxServiceError && error.status === 404) return;
+			throw error;
+		}
 	}
 
 	override async stop(): Promise<void> {
@@ -80,14 +99,23 @@ export class N8nSandboxServiceSandbox extends BaseSandbox {
 			name: this.name,
 			provider: this.provider,
 			status: this.status,
-			createdAt: new Date(sandbox.createdAt * 1000),
+			createdAt: this.createdAt ?? new Date(sandbox.createdAt * 1000),
 			metadata: {
 				lastActiveAt: new Date(sandbox.lastActiveAt * 1000).toISOString(),
 				remoteStatus: sandbox.status,
+				workingDirectory: N8nSandboxServiceSandbox.WORKSPACE_DIR,
 			},
 		};
 	}
 
+	override getInstructions(): string {
+		return [
+			'Cloud sandbox with isolated execution (TypeScript runtime).',
+			`Default working directory: ${N8nSandboxServiceSandbox.WORKSPACE_DIR}.`,
+			`Command timeout: ${Math.ceil(this.timeout / 1000)}s.`,
+		].join(' ');
+	}
+
 	override async executeCommand(
 		command: string,
 		args: string[] = [],
@@ -96,9 +124,9 @@ export class N8nSandboxServiceSandbox extends BaseSandbox {
 		await this.ensureRunning();
 		const result = await this.client.exec(this.requireSandboxId(), {
 			command: toShellCommand(command, args),
-			env: options?.env,
+			env: this.compactEnv(options?.env),
 			workdir: options?.cwd,
-			timeoutMs: options?.timeout ?? this.options.timeout,
+			timeoutMs: options?.timeout ?? this.timeout,
 			abortSignal: options?.abortSignal,
 			onStdout: options?.onStdout,
 			onStderr: options?.onStderr,
@@ -121,6 +149,28 @@ export class N8nSandboxServiceSandbox extends BaseSandbox {
 		return this.client;
 	}
 
+	/** Returns the remote sandbox record, or `null` if it no longer exists (404). */
+	private async tryGetExistingSandbox(sandboxId: string): Promise<SandboxRecord | null> {
+		try {
+			return await this.client.getSandbox(sandboxId);
+		} catch (error) {
+			if (error instanceof SandboxServiceError && error.status === 404) return null;
+			throw error;
+		}
+	}
+
+	/** Merges constructor-level env with per-command env, filtering out undefined values. */
+	private compactEnv(env: NodeJS.ProcessEnv | undefined): Record<string, string> | undefined {
+		const merged = {
+			...this.options.env,
+			...env,
+		};
+		const entries = Object.entries(merged).filter(
+			(entry): entry is [string, string] => typeof entry[1] === 'string',
+		);
+		return entries.length > 0 ? Object.fromEntries(entries) : undefined;
+	}
+
 	private requireSandboxId(): string {
 		assert(this.sandboxId, 'Sandbox has not been created yet');
 		return this.sandboxId;