fix: address critical code review issues

Fix security and reliability issues identified in code review:

1. Security: Remove non-null assertions in credentials.ts
   - Add proper validation before returning credentials
   - Throw early with clear error messages showing which vars are missing
   - Prevents runtime failures with cryptic undefined errors

2. Reliability: Add pagination safety limits
   - Add MAX_PAGES limit (1000) to all pagination loops
   - Prevents infinite loops if API returns same cursor repeatedly
   - Applies to: cleanupOrphanedWorkflows, cleanupOldExecutions, cleanupExecutionsByWorkflow

Changes ensure safer credential handling and prevent potential infinite loops
in cleanup operations.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
czlonkowski
2025-10-03 13:22:10 +02:00
parent 2305aaab9e
commit 1728495146
4 changed files with 59 additions and 11 deletions

Binary file not shown.

View File

@@ -1,6 +1,6 @@
{
"name": "n8n-mcp-runtime",
"version": "2.15.0",
"version": "2.15.1",
"description": "n8n MCP Server Runtime Dependencies Only",
"private": true,
"dependencies": {

View File

@@ -32,11 +32,18 @@ export async function cleanupOrphanedWorkflows(): Promise<string[]> {
let allWorkflows: any[] = [];
let cursor: string | undefined;
let pageCount = 0;
const MAX_PAGES = 1000; // Safety limit to prevent infinite loops
// Fetch all workflows with pagination
try {
do {
pageCount++;
if (pageCount > MAX_PAGES) {
logger.error(`Exceeded maximum pages (${MAX_PAGES}). Possible infinite loop or API issue.`);
throw new Error('Pagination safety limit exceeded while fetching workflows');
}
logger.debug(`Fetching workflows page ${pageCount}...`);
const response = await client.listWorkflows({
@@ -101,11 +108,18 @@ export async function cleanupOldExecutions(
let allExecutions: any[] = [];
let cursor: string | undefined;
let pageCount = 0;
const MAX_PAGES = 1000; // Safety limit to prevent infinite loops
// Fetch all executions
try {
do {
pageCount++;
if (pageCount > MAX_PAGES) {
logger.error(`Exceeded maximum pages (${MAX_PAGES}). Possible infinite loop or API issue.`);
throw new Error('Pagination safety limit exceeded while fetching executions');
}
logger.debug(`Fetching executions page ${pageCount}...`);
const response = await client.listExecutions({
@@ -239,9 +253,18 @@ export async function cleanupExecutionsByWorkflow(
let cursor: string | undefined;
let totalCount = 0;
let pageCount = 0;
const MAX_PAGES = 1000; // Safety limit to prevent infinite loops
try {
do {
pageCount++;
if (pageCount > MAX_PAGES) {
logger.error(`Exceeded maximum pages (${MAX_PAGES}). Possible infinite loop or API issue.`);
throw new Error(`Pagination safety limit exceeded while fetching executions for workflow ${workflowId}`);
}
const response = await client.listExecutions({
workflowId,
cursor,

View File

@@ -39,15 +39,27 @@ export interface N8nTestCredentials {
*/
export function getN8nCredentials(): N8nTestCredentials {
if (process.env.CI) {
// CI: Use GitHub secrets
// CI: Use GitHub secrets - validate required variables first
const url = process.env.N8N_URL;
const apiKey = process.env.N8N_API_KEY;
if (!url || !apiKey) {
throw new Error(
'Missing required CI credentials:\n' +
` N8N_URL: ${url ? 'set' : 'MISSING'}\n` +
` N8N_API_KEY: ${apiKey ? 'set' : 'MISSING'}\n` +
'Please configure GitHub secrets for integration tests.'
);
}
return {
url: process.env.N8N_URL!,
apiKey: process.env.N8N_API_KEY!,
url,
apiKey,
webhookWorkflows: {
get: process.env.N8N_TEST_WEBHOOK_GET_ID!,
post: process.env.N8N_TEST_WEBHOOK_POST_ID!,
put: process.env.N8N_TEST_WEBHOOK_PUT_ID!,
delete: process.env.N8N_TEST_WEBHOOK_DELETE_ID!
get: process.env.N8N_TEST_WEBHOOK_GET_ID || '',
post: process.env.N8N_TEST_WEBHOOK_POST_ID || '',
put: process.env.N8N_TEST_WEBHOOK_PUT_ID || '',
delete: process.env.N8N_TEST_WEBHOOK_DELETE_ID || ''
},
cleanup: {
enabled: true,
@@ -56,10 +68,23 @@ export function getN8nCredentials(): N8nTestCredentials {
}
};
} else {
// Local: Use .env file
// Local: Use .env file - validate required variables first
const url = process.env.N8N_API_URL;
const apiKey = process.env.N8N_API_KEY;
if (!url || !apiKey) {
throw new Error(
'Missing required credentials in .env:\n' +
` N8N_API_URL: ${url ? 'set' : 'MISSING'}\n` +
` N8N_API_KEY: ${apiKey ? 'set' : 'MISSING'}\n\n` +
'Please add these to your .env file.\n' +
'See .env.example for configuration details.'
);
}
return {
url: process.env.N8N_API_URL!,
apiKey: process.env.N8N_API_KEY!,
url,
apiKey,
webhookWorkflows: {
get: process.env.N8N_TEST_WEBHOOK_GET_ID || '',
post: process.env.N8N_TEST_WEBHOOK_POST_ID || '',