feat: implement Phase 2 validation improvements
Phase 2 Professional Validation Features: 1. Validation Profiles: - minimal: Only required fields - runtime: Critical errors + security warnings - ai-friendly: Balanced (default) - strict: All checks + best practices 2. New Node Validators: - Webhook: Path validation, response modes, auth warnings - PostgreSQL: SQL injection detection, query safety - MySQL: Similar to Postgres with MySQL-specific checks 3. New Tools: - validate_node_minimal: Lightning-fast required field checking - Updated validate_node_operation with profile support 4. SQL Safety Features: - Detects template expressions vulnerable to injection - Warns about DELETE/UPDATE without WHERE - Catches dangerous operations (DROP, TRUNCATE) - Suggests parameterized queries 5. Enhanced Coverage: - Now supports 7+ major nodes with specific validators - Flexible validation based on use case - Professional-grade safety checks This completes the major validation system overhaul from the original plan. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
czlonkowski
@@ -304,28 +304,7 @@ export class ConfigValidator {
|
||||
warnings: ValidationWarning[],
|
||||
suggestions: string[]
|
||||
): void {
|
||||
// Path validation
|
||||
if (config.path) {
|
||||
if (config.path.startsWith('/')) {
|
||||
warnings.push({
|
||||
type: 'inefficient',
|
||||
property: 'path',
|
||||
message: 'Webhook path should not start with /',
|
||||
suggestion: 'Remove the leading / from the path'
|
||||
});
|
||||
}
|
||||
|
||||
if (config.path.includes(' ')) {
|
||||
warnings.push({
|
||||
type: 'inefficient',
|
||||
property: 'path',
|
||||
message: 'Webhook path contains spaces',
|
||||
suggestion: 'Use hyphens or underscores instead of spaces'
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Response mode suggestions
|
||||
// Basic webhook validation - moved detailed validation to NodeSpecificValidators
|
||||
if (config.responseMode === 'responseNode' && !config.responseData) {
|
||||
suggestions.push('When using responseMode=responseNode, add a "Respond to Webhook" node to send custom responses');
|
||||
}
|
||||
|
||||
@@ -10,9 +10,11 @@ import { NodeSpecificValidators, NodeValidationContext } from './node-specific-v
|
||||
import { ExampleGenerator } from './example-generator';
|
||||
|
||||
export type ValidationMode = 'full' | 'operation' | 'minimal';
|
||||
export type ValidationProfile = 'strict' | 'runtime' | 'ai-friendly' | 'minimal';
|
||||
|
||||
export interface EnhancedValidationResult extends ValidationResult {
|
||||
mode: ValidationMode;
|
||||
profile?: ValidationProfile;
|
||||
operation?: {
|
||||
resource?: string;
|
||||
operation?: string;
|
||||
@@ -40,7 +42,8 @@ export class EnhancedConfigValidator extends ConfigValidator {
|
||||
nodeType: string,
|
||||
config: Record<string, any>,
|
||||
properties: any[],
|
||||
mode: ValidationMode = 'operation'
|
||||
mode: ValidationMode = 'operation',
|
||||
profile: ValidationProfile = 'ai-friendly'
|
||||
): EnhancedValidationResult {
|
||||
// Extract operation context from config
|
||||
const operationContext = this.extractOperationContext(config);
|
||||
@@ -60,11 +63,15 @@ export class EnhancedConfigValidator extends ConfigValidator {
|
||||
const enhancedResult: EnhancedValidationResult = {
|
||||
...baseResult,
|
||||
mode,
|
||||
profile,
|
||||
operation: operationContext,
|
||||
examples: [],
|
||||
nextSteps: []
|
||||
};
|
||||
|
||||
// Apply profile-based filtering
|
||||
this.applyProfileFilters(enhancedResult, profile);
|
||||
|
||||
// Add operation-specific enhancements
|
||||
this.addOperationSpecificEnhancements(nodeType, config, enhancedResult);
|
||||
|
||||
@@ -216,6 +223,18 @@ export class EnhancedConfigValidator extends ConfigValidator {
|
||||
case 'nodes-base.mongoDb':
|
||||
NodeSpecificValidators.validateMongoDB(context);
|
||||
break;
|
||||
|
||||
case 'nodes-base.webhook':
|
||||
NodeSpecificValidators.validateWebhook(context);
|
||||
break;
|
||||
|
||||
case 'nodes-base.postgres':
|
||||
NodeSpecificValidators.validatePostgres(context);
|
||||
break;
|
||||
|
||||
case 'nodes-base.mysql':
|
||||
NodeSpecificValidators.validateMySQL(context);
|
||||
break;
|
||||
}
|
||||
|
||||
// Update autofix if changes were made
|
||||
@@ -441,4 +460,50 @@ export class EnhancedConfigValidator extends ConfigValidator {
|
||||
|
||||
return Array.from(seen.values());
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply profile-based filtering to validation results
|
||||
*/
|
||||
private static applyProfileFilters(
|
||||
result: EnhancedValidationResult,
|
||||
profile: ValidationProfile
|
||||
): void {
|
||||
switch (profile) {
|
||||
case 'minimal':
|
||||
// Only keep missing required errors
|
||||
result.errors = result.errors.filter(e => e.type === 'missing_required');
|
||||
result.warnings = [];
|
||||
result.suggestions = [];
|
||||
break;
|
||||
|
||||
case 'runtime':
|
||||
// Keep critical runtime errors only
|
||||
result.errors = result.errors.filter(e =>
|
||||
e.type === 'missing_required' ||
|
||||
e.type === 'invalid_value' ||
|
||||
(e.type === 'invalid_type' && e.message.includes('undefined'))
|
||||
);
|
||||
// Keep only security warnings
|
||||
result.warnings = result.warnings.filter(w => w.type === 'security');
|
||||
result.suggestions = [];
|
||||
break;
|
||||
|
||||
case 'strict':
|
||||
// Keep everything, add more suggestions
|
||||
if (result.warnings.length === 0 && result.errors.length === 0) {
|
||||
result.suggestions.push('Consider adding error handling and timeout configuration');
|
||||
result.suggestions.push('Add authentication if connecting to external services');
|
||||
}
|
||||
break;
|
||||
|
||||
case 'ai-friendly':
|
||||
default:
|
||||
// Current behavior - balanced for AI agents
|
||||
// Filter out noise but keep helpful warnings
|
||||
result.warnings = result.warnings.filter(w =>
|
||||
w.type !== 'inefficient' || !w.property?.startsWith('_')
|
||||
);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -502,4 +502,336 @@ export class NodeSpecificValidators {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate Webhook node configuration
|
||||
*/
|
||||
static validateWebhook(context: NodeValidationContext): void {
|
||||
const { config, errors, warnings, suggestions } = context;
|
||||
|
||||
// Path validation
|
||||
if (!config.path) {
|
||||
errors.push({
|
||||
type: 'missing_required',
|
||||
property: 'path',
|
||||
message: 'Webhook path is required',
|
||||
fix: 'Set a unique path like "my-webhook" (no leading slash)'
|
||||
});
|
||||
} else {
|
||||
const path = config.path;
|
||||
|
||||
// Check for leading slash
|
||||
if (path.startsWith('/')) {
|
||||
warnings.push({
|
||||
type: 'inefficient',
|
||||
property: 'path',
|
||||
message: 'Webhook path should not start with /',
|
||||
suggestion: 'Remove the leading slash: use "my-webhook" instead of "/my-webhook"'
|
||||
});
|
||||
}
|
||||
|
||||
// Check for spaces
|
||||
if (path.includes(' ')) {
|
||||
errors.push({
|
||||
type: 'invalid_value',
|
||||
property: 'path',
|
||||
message: 'Webhook path cannot contain spaces',
|
||||
fix: 'Replace spaces with hyphens or underscores'
|
||||
});
|
||||
}
|
||||
|
||||
// Check for special characters
|
||||
if (!/^[a-zA-Z0-9\-_\/]+$/.test(path.replace(/^\//, ''))) {
|
||||
warnings.push({
|
||||
type: 'inefficient',
|
||||
property: 'path',
|
||||
message: 'Webhook path contains special characters',
|
||||
suggestion: 'Use only letters, numbers, hyphens, and underscores'
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Response mode validation
|
||||
if (config.responseMode === 'responseNode') {
|
||||
suggestions.push('Add a "Respond to Webhook" node to send custom responses');
|
||||
|
||||
if (!config.responseData) {
|
||||
warnings.push({
|
||||
type: 'missing_common',
|
||||
property: 'responseData',
|
||||
message: 'Response data not configured for responseNode mode',
|
||||
suggestion: 'Add a "Respond to Webhook" node or change responseMode'
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// HTTP method validation
|
||||
if (config.httpMethod && Array.isArray(config.httpMethod)) {
|
||||
if (config.httpMethod.length === 0) {
|
||||
errors.push({
|
||||
type: 'invalid_value',
|
||||
property: 'httpMethod',
|
||||
message: 'At least one HTTP method must be selected',
|
||||
fix: 'Select GET, POST, or other methods your webhook should accept'
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Authentication warnings
|
||||
if (!config.authentication || config.authentication === 'none') {
|
||||
warnings.push({
|
||||
type: 'security',
|
||||
message: 'Webhook has no authentication',
|
||||
suggestion: 'Consider adding authentication to prevent unauthorized access'
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate Postgres node configuration
|
||||
*/
|
||||
static validatePostgres(context: NodeValidationContext): void {
|
||||
const { config, errors, warnings, suggestions, autofix } = context;
|
||||
const { operation } = config;
|
||||
|
||||
// Common query validation
|
||||
if (['execute', 'select', 'insert', 'update', 'delete'].includes(operation)) {
|
||||
this.validateSQLQuery(context, 'postgres');
|
||||
}
|
||||
|
||||
// Operation-specific validation
|
||||
switch (operation) {
|
||||
case 'insert':
|
||||
if (!config.table) {
|
||||
errors.push({
|
||||
type: 'missing_required',
|
||||
property: 'table',
|
||||
message: 'Table name is required for insert operation',
|
||||
fix: 'Specify the table to insert data into'
|
||||
});
|
||||
}
|
||||
|
||||
if (!config.columns && !config.dataMode) {
|
||||
warnings.push({
|
||||
type: 'missing_common',
|
||||
property: 'columns',
|
||||
message: 'No columns specified for insert',
|
||||
suggestion: 'Define which columns to insert data into'
|
||||
});
|
||||
}
|
||||
break;
|
||||
|
||||
case 'update':
|
||||
if (!config.table) {
|
||||
errors.push({
|
||||
type: 'missing_required',
|
||||
property: 'table',
|
||||
message: 'Table name is required for update operation',
|
||||
fix: 'Specify the table to update'
|
||||
});
|
||||
}
|
||||
|
||||
if (!config.updateKey) {
|
||||
warnings.push({
|
||||
type: 'missing_common',
|
||||
property: 'updateKey',
|
||||
message: 'No update key specified',
|
||||
suggestion: 'Set updateKey to identify which rows to update (e.g., "id")'
|
||||
});
|
||||
}
|
||||
break;
|
||||
|
||||
case 'delete':
|
||||
if (!config.table) {
|
||||
errors.push({
|
||||
type: 'missing_required',
|
||||
property: 'table',
|
||||
message: 'Table name is required for delete operation',
|
||||
fix: 'Specify the table to delete from'
|
||||
});
|
||||
}
|
||||
|
||||
if (!config.deleteKey) {
|
||||
errors.push({
|
||||
type: 'missing_required',
|
||||
property: 'deleteKey',
|
||||
message: 'Delete key is required to identify rows',
|
||||
fix: 'Set deleteKey (e.g., "id") to specify which rows to delete'
|
||||
});
|
||||
}
|
||||
break;
|
||||
|
||||
case 'execute':
|
||||
if (!config.query) {
|
||||
errors.push({
|
||||
type: 'missing_required',
|
||||
property: 'query',
|
||||
message: 'SQL query is required',
|
||||
fix: 'Provide the SQL query to execute'
|
||||
});
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
// Connection pool suggestions
|
||||
if (config.connectionTimeout === undefined) {
|
||||
suggestions.push('Consider setting connectionTimeout to handle slow connections');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate MySQL node configuration
|
||||
*/
|
||||
static validateMySQL(context: NodeValidationContext): void {
|
||||
const { config, errors, warnings, suggestions } = context;
|
||||
const { operation } = config;
|
||||
|
||||
// MySQL uses similar validation to Postgres
|
||||
if (['execute', 'insert', 'update', 'delete'].includes(operation)) {
|
||||
this.validateSQLQuery(context, 'mysql');
|
||||
}
|
||||
|
||||
// Operation-specific validation (similar to Postgres)
|
||||
switch (operation) {
|
||||
case 'insert':
|
||||
if (!config.table) {
|
||||
errors.push({
|
||||
type: 'missing_required',
|
||||
property: 'table',
|
||||
message: 'Table name is required for insert operation',
|
||||
fix: 'Specify the table to insert data into'
|
||||
});
|
||||
}
|
||||
break;
|
||||
|
||||
case 'update':
|
||||
if (!config.table) {
|
||||
errors.push({
|
||||
type: 'missing_required',
|
||||
property: 'table',
|
||||
message: 'Table name is required for update operation',
|
||||
fix: 'Specify the table to update'
|
||||
});
|
||||
}
|
||||
|
||||
if (!config.updateKey) {
|
||||
warnings.push({
|
||||
type: 'missing_common',
|
||||
property: 'updateKey',
|
||||
message: 'No update key specified',
|
||||
suggestion: 'Set updateKey to identify which rows to update'
|
||||
});
|
||||
}
|
||||
break;
|
||||
|
||||
case 'delete':
|
||||
if (!config.table) {
|
||||
errors.push({
|
||||
type: 'missing_required',
|
||||
property: 'table',
|
||||
message: 'Table name is required for delete operation',
|
||||
fix: 'Specify the table to delete from'
|
||||
});
|
||||
}
|
||||
break;
|
||||
|
||||
case 'execute':
|
||||
if (!config.query) {
|
||||
errors.push({
|
||||
type: 'missing_required',
|
||||
property: 'query',
|
||||
message: 'SQL query is required',
|
||||
fix: 'Provide the SQL query to execute'
|
||||
});
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
// MySQL-specific warnings
|
||||
if (config.timezone === undefined) {
|
||||
suggestions.push('Consider setting timezone to ensure consistent date/time handling');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate SQL queries for injection risks and common issues
|
||||
*/
|
||||
private static validateSQLQuery(
|
||||
context: NodeValidationContext,
|
||||
dbType: 'postgres' | 'mysql' | 'generic' = 'generic'
|
||||
): void {
|
||||
const { config, errors, warnings, suggestions } = context;
|
||||
const query = config.query || config.deleteQuery || config.updateQuery || '';
|
||||
|
||||
if (!query) return;
|
||||
|
||||
const lowerQuery = query.toLowerCase();
|
||||
|
||||
// SQL injection checks
|
||||
if (query.includes('${') || query.includes('{{')) {
|
||||
warnings.push({
|
||||
type: 'security',
|
||||
message: 'Query contains template expressions that might be vulnerable to SQL injection',
|
||||
suggestion: 'Use parameterized queries with query parameters instead of string interpolation'
|
||||
});
|
||||
|
||||
suggestions.push('Example: Use "SELECT * FROM users WHERE id = $1" with queryParams: [userId]');
|
||||
}
|
||||
|
||||
// DELETE without WHERE
|
||||
if (lowerQuery.includes('delete') && !lowerQuery.includes('where')) {
|
||||
errors.push({
|
||||
type: 'invalid_value',
|
||||
property: 'query',
|
||||
message: 'DELETE query without WHERE clause will delete all records',
|
||||
fix: 'Add a WHERE clause to specify which records to delete'
|
||||
});
|
||||
}
|
||||
|
||||
// UPDATE without WHERE
|
||||
if (lowerQuery.includes('update') && !lowerQuery.includes('where')) {
|
||||
warnings.push({
|
||||
type: 'security',
|
||||
message: 'UPDATE query without WHERE clause will update all records',
|
||||
suggestion: 'Add a WHERE clause to specify which records to update'
|
||||
});
|
||||
}
|
||||
|
||||
// TRUNCATE warning
|
||||
if (lowerQuery.includes('truncate')) {
|
||||
warnings.push({
|
||||
type: 'security',
|
||||
message: 'TRUNCATE will remove all data from the table',
|
||||
suggestion: 'Consider using DELETE with WHERE clause if you need to keep some data'
|
||||
});
|
||||
}
|
||||
|
||||
// DROP warning
|
||||
if (lowerQuery.includes('drop')) {
|
||||
errors.push({
|
||||
type: 'invalid_value',
|
||||
property: 'query',
|
||||
message: 'DROP operations are extremely dangerous and will permanently delete database objects',
|
||||
fix: 'Use this only if you really intend to delete tables/databases permanently'
|
||||
});
|
||||
}
|
||||
|
||||
// Performance suggestions
|
||||
if (lowerQuery.includes('select *')) {
|
||||
suggestions.push('Consider selecting specific columns instead of * for better performance');
|
||||
}
|
||||
|
||||
// Database-specific checks
|
||||
if (dbType === 'postgres') {
|
||||
// PostgreSQL specific validations
|
||||
if (query.includes('$$')) {
|
||||
suggestions.push('Dollar-quoted strings detected - ensure they are properly closed');
|
||||
}
|
||||
} else if (dbType === 'mysql') {
|
||||
// MySQL specific validations
|
||||
if (query.includes('`')) {
|
||||
suggestions.push('Using backticks for identifiers - ensure they are properly paired');
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user