mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-03-22 18:33:08 +00:00
Compare commits
1 Commits
v2.40.2
...
fix/notifi
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e52a04a291 |
11
CHANGELOG.md
11
CHANGELOG.md
@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [2.40.3] - 2026-03-22
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Notification 400 disconnect storms (#654)**: `handleRequest()` now returns 202 Accepted for JSON-RPC notifications with stale/expired session IDs instead of 400. Per JSON-RPC 2.0 spec, notifications don't expect responses — returning 400 caused Claude's proxy to trigger reconnection storms (930 errors/day, 216 users affected)
|
||||
- **TOCTOU race in session lookup**: Added null guard after transport assignment to handle sessions removed between the existence check and use
|
||||
- **`updateTable` silently ignoring `columns` parameter**: Now returns a warning message when `columns` is passed to `updateTable`, clarifying that table schema is immutable after creation via the public API
|
||||
- **Tool schema descriptions clarified**: `name` and `columns` parameter descriptions now explicitly document that `updateTable` is rename-only and columns are for `createTable` only
|
||||
|
||||
Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en
|
||||
|
||||
## [2.40.2] - 2026-03-22
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "n8n-mcp",
|
||||
"version": "2.40.2",
|
||||
"version": "2.40.3",
|
||||
"description": "Integration between n8n workflow automation and Model Context Protocol (MCP)",
|
||||
"main": "dist/index.js",
|
||||
"types": "dist/index.d.ts",
|
||||
|
||||
@@ -253,6 +253,21 @@ export class SingleSessionHTTPServer {
|
||||
// This ensures compatibility with all MCP clients and proxies
|
||||
return Boolean(sessionId && sessionId.length > 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a request body is a JSON-RPC notification (or batch of only notifications).
|
||||
* Notifications have a `method` field but no `id` field per JSON-RPC 2.0 spec.
|
||||
*/
|
||||
private isJsonRpcNotification(body: unknown): boolean {
|
||||
if (!body || typeof body !== 'object') return false;
|
||||
if (Array.isArray(body)) {
|
||||
return body.length > 0 && body.every(
|
||||
(msg: any) => msg && typeof msg.method === 'string' && !('id' in msg)
|
||||
);
|
||||
}
|
||||
const msg = body as Record<string, unknown>;
|
||||
return typeof msg.method === 'string' && !('id' in msg);
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize error information for client responses
|
||||
@@ -614,6 +629,24 @@ export class SingleSessionHTTPServer {
|
||||
logger.info('handleRequest: Reusing existing transport for session', { sessionId });
|
||||
transport = this.transports[sessionId];
|
||||
|
||||
// TOCTOU guard: session may have been removed between the check and this line
|
||||
if (!transport) {
|
||||
if (this.isJsonRpcNotification(req.body)) {
|
||||
logger.info('handleRequest: Session removed during lookup, accepting notification', {
|
||||
sessionId,
|
||||
});
|
||||
res.status(202).end();
|
||||
return;
|
||||
}
|
||||
logger.warn('handleRequest: Session removed between check and use (TOCTOU)', { sessionId });
|
||||
res.status(400).json({
|
||||
jsonrpc: '2.0',
|
||||
error: { code: -32000, message: 'Bad Request: Session not found or expired' },
|
||||
id: req.body?.id || null,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// In multi-tenant shared mode, update instance context if provided
|
||||
const isMultiTenantEnabled = process.env.ENABLE_MULTI_TENANT === 'true';
|
||||
const sessionStrategy = process.env.MULTI_TENANT_SESSION_STRATEGY || 'instance';
|
||||
@@ -627,23 +660,36 @@ export class SingleSessionHTTPServer {
|
||||
this.updateSessionAccess(sessionId);
|
||||
|
||||
} else {
|
||||
// Invalid request - no session ID and not an initialize request
|
||||
// Check if this is a JSON-RPC notification (no "id" field = fire-and-forget)
|
||||
// Per JSON-RPC 2.0 spec, notifications don't expect responses.
|
||||
// Returning 400 for stale-session notifications causes Claude's proxy to
|
||||
// interpret the connection as broken, triggering reconnection storms (#654).
|
||||
if (this.isJsonRpcNotification(req.body)) {
|
||||
logger.info('handleRequest: Accepting notification for stale/missing session', {
|
||||
method: req.body?.method,
|
||||
sessionId: sessionId || 'none',
|
||||
});
|
||||
res.status(202).end();
|
||||
return;
|
||||
}
|
||||
|
||||
// Only return 400 for actual requests that need a valid session
|
||||
const errorDetails = {
|
||||
hasSessionId: !!sessionId,
|
||||
isInitialize: isInitialize,
|
||||
sessionIdValid: sessionId ? this.isValidSessionId(sessionId) : false,
|
||||
sessionExists: sessionId ? !!this.transports[sessionId] : false
|
||||
};
|
||||
|
||||
|
||||
logger.warn('handleRequest: Invalid request - no session ID and not initialize', errorDetails);
|
||||
|
||||
|
||||
let errorMessage = 'Bad Request: No valid session ID provided and not an initialize request';
|
||||
if (sessionId && !this.isValidSessionId(sessionId)) {
|
||||
errorMessage = 'Bad Request: Invalid session ID format';
|
||||
} else if (sessionId && !this.transports[sessionId]) {
|
||||
errorMessage = 'Bad Request: Session not found or expired';
|
||||
}
|
||||
|
||||
|
||||
res.status(400).json({
|
||||
jsonrpc: '2.0',
|
||||
error: {
|
||||
|
||||
@@ -2834,10 +2834,13 @@ export async function handleUpdateTable(args: unknown, context?: InstanceContext
|
||||
const client = ensureApiConfigured(context);
|
||||
const { tableId, name } = updateTableSchema.parse(args);
|
||||
const dataTable = await client.updateDataTable(tableId, { name });
|
||||
const rawArgs = args as Record<string, unknown>;
|
||||
const hasColumns = rawArgs && typeof rawArgs === 'object' && 'columns' in rawArgs;
|
||||
return {
|
||||
success: true,
|
||||
data: dataTable,
|
||||
message: `Data table renamed to "${dataTable.name}"`,
|
||||
message: `Data table renamed to "${dataTable.name}"` +
|
||||
(hasColumns ? '. Note: columns parameter was ignored — table schema is immutable after creation via the public API' : ''),
|
||||
};
|
||||
} catch (error) {
|
||||
return handleDataTableError(error);
|
||||
|
||||
@@ -619,10 +619,10 @@ export const n8nManagementTools: ToolDefinition[] = [
|
||||
description: 'Operation to perform',
|
||||
},
|
||||
tableId: { type: 'string', description: 'Data table ID (required for all actions except createTable and listTables)' },
|
||||
name: { type: 'string', description: 'For createTable/updateTable: table name' },
|
||||
name: { type: 'string', description: 'For createTable: table name. For updateTable: new name (rename only — schema is immutable after creation)' },
|
||||
columns: {
|
||||
type: 'array',
|
||||
description: 'For createTable: column definitions',
|
||||
description: 'For createTable only: column definitions (schema is immutable after creation via public API)',
|
||||
items: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
|
||||
@@ -219,6 +219,7 @@ describe('HTTP Server Session Management', () => {
|
||||
status: vi.fn().mockReturnThis(),
|
||||
json: vi.fn().mockReturnThis(),
|
||||
send: vi.fn().mockReturnThis(),
|
||||
end: vi.fn().mockReturnThis(),
|
||||
setHeader: vi.fn((key: string, value: string) => {
|
||||
headers[key.toLowerCase()] = value;
|
||||
}),
|
||||
@@ -1170,7 +1171,7 @@ describe('HTTP Server Session Management', () => {
|
||||
|
||||
it('should show legacy SSE session when present', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
|
||||
// Mock legacy session
|
||||
const mockSession = {
|
||||
sessionId: 'sse-session-123',
|
||||
@@ -1180,10 +1181,111 @@ describe('HTTP Server Session Management', () => {
|
||||
(server as any).session = mockSession;
|
||||
|
||||
const sessionInfo = server.getSessionInfo();
|
||||
|
||||
|
||||
expect(sessionInfo.active).toBe(true);
|
||||
expect(sessionInfo.sessionId).toBe('sse-session-123');
|
||||
expect(sessionInfo.age).toBeGreaterThanOrEqual(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Notification handling for stale sessions (#654)', () => {
|
||||
beforeEach(() => {
|
||||
// Re-apply mockImplementation after vi.clearAllMocks() resets it
|
||||
mockConsoleManager.wrapOperation.mockImplementation(async (fn: () => Promise<any>) => {
|
||||
return await fn();
|
||||
});
|
||||
});
|
||||
|
||||
it('should return 202 for notification with stale session ID', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
const { req, res } = createMockReqRes();
|
||||
|
||||
req.headers = { 'mcp-session-id': 'stale-session-that-does-not-exist' };
|
||||
req.method = 'POST';
|
||||
req.body = {
|
||||
jsonrpc: '2.0',
|
||||
method: 'notifications/initialized',
|
||||
};
|
||||
|
||||
await server.handleRequest(req as any, res as any);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(202);
|
||||
expect(res.end).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return 202 for notification batch with stale session ID', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
const { req, res } = createMockReqRes();
|
||||
|
||||
req.headers = { 'mcp-session-id': 'stale-session-that-does-not-exist' };
|
||||
req.method = 'POST';
|
||||
req.body = [
|
||||
{ jsonrpc: '2.0', method: 'notifications/initialized' },
|
||||
{ jsonrpc: '2.0', method: 'notifications/cancelled' },
|
||||
];
|
||||
|
||||
await server.handleRequest(req as any, res as any);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(202);
|
||||
expect(res.end).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return 400 for request (with id) with stale session ID', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
const { req, res } = createMockReqRes();
|
||||
req.headers = { 'mcp-session-id': 'stale-session-that-does-not-exist' };
|
||||
req.method = 'POST';
|
||||
req.body = {
|
||||
jsonrpc: '2.0',
|
||||
method: 'tools/call',
|
||||
params: { name: 'search_nodes', arguments: { query: 'http' } },
|
||||
id: 42,
|
||||
};
|
||||
|
||||
await server.handleRequest(req as any, res as any);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(400);
|
||||
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
|
||||
error: expect.objectContaining({
|
||||
message: 'Bad Request: Session not found or expired',
|
||||
}),
|
||||
}));
|
||||
});
|
||||
|
||||
it('should return 202 for notification with no session ID', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
const { req, res } = createMockReqRes();
|
||||
|
||||
req.method = 'POST';
|
||||
req.body = {
|
||||
jsonrpc: '2.0',
|
||||
method: 'notifications/cancelled',
|
||||
};
|
||||
|
||||
await server.handleRequest(req as any, res as any);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(202);
|
||||
expect(res.end).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return 400 for request with no session ID and not initialize', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
const { req, res } = createMockReqRes();
|
||||
req.method = 'POST';
|
||||
req.body = {
|
||||
jsonrpc: '2.0',
|
||||
method: 'tools/list',
|
||||
id: 1,
|
||||
};
|
||||
|
||||
await server.handleRequest(req as any, res as any);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(400);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -363,6 +363,22 @@ describe('Data Table Handlers (n8n_manage_datatable)', () => {
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toBe('Update failed');
|
||||
});
|
||||
|
||||
it('should warn when columns parameter is passed', async () => {
|
||||
const updatedTable = { id: 'dt-1', name: 'Renamed' };
|
||||
mockApiClient.updateDataTable.mockResolvedValue(updatedTable);
|
||||
|
||||
const result = await handlers.handleUpdateTable({
|
||||
tableId: 'dt-1',
|
||||
name: 'Renamed',
|
||||
columns: [{ name: 'phone', type: 'string' }],
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.message).toContain('columns parameter was ignored');
|
||||
expect(result.message).toContain('immutable after creation');
|
||||
expect(mockApiClient.updateDataTable).toHaveBeenCalledWith('dt-1', { name: 'Renamed' });
|
||||
});
|
||||
});
|
||||
|
||||
// ========================================================================
|
||||
|
||||
Reference in New Issue
Block a user