fix: return 202 for stale-session notifications, warn on updateTable columns (#654) (#655)

This commit is contained in:
Romuald Członkowski
2026-03-22 19:59:57 +01:00
committed by GitHub
parent 6f6668acc4
commit ec19c9dade
7 changed files with 200 additions and 10 deletions

View File

@@ -253,6 +253,22 @@ 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).
* Per JSON-RPC 2.0 §4.1, a notification is a request without an "id" member.
* Note: `!('id' in msg)` is strict — messages with `id: null` are treated as
* requests, not notifications. This is spec-compliant.
*/
private isJsonRpcNotification(body: unknown): boolean {
if (!body || typeof body !== 'object') return false;
const isSingleNotification = (msg: any): boolean =>
msg && typeof msg.method === 'string' && !('id' in msg);
if (Array.isArray(body)) {
return body.length > 0 && body.every(isSingleNotification);
}
return isSingleNotification(body);
}
/**
* Sanitize error information for client responses
@@ -614,6 +630,22 @@ 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 above and here
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 +659,33 @@ export class SingleSessionHTTPServer {
this.updateSessionAccess(sessionId);
} else {
// Invalid request - no session ID and not an initialize request
// Notifications are fire-and-forget; returning 400 triggers 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: {

View File

@@ -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);

View File

@@ -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: {