fix(variables): added block/variable name validation to prevent invalid characters, fixed block renaming issue with drag handler (#448)

* added block name and variable name validation to prevent invalid characters, fixed block renaming issue with drag handler

* also collapse multiple whitespaces into one
This commit is contained in:
Waleed Latif
2025-06-01 23:51:07 -07:00
committed by GitHub
parent de2ce6fcb7
commit 7d640823c3
4 changed files with 296 additions and 28 deletions

View File

@@ -18,6 +18,7 @@ import {
import { Input } from '@/components/ui/input'
import { ScrollArea } from '@/components/ui/scroll-area'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
import { validateName } from '@/lib/utils'
import { useVariablesStore } from '@/stores/panel/variables/store'
import type { Variable, VariableType } from '@/stores/panel/variables/types'
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
@@ -54,6 +55,12 @@ export function Variables({ panelWidth }: VariablesProps) {
// Track which variables are currently being edited
const [_activeEditors, setActiveEditors] = useState<Record<string, boolean>>({})
// Handle variable name change with validation
const handleVariableNameChange = (variableId: string, newName: string) => {
const validatedName = validateName(newName)
updateVariable(variableId, { name: validatedName })
}
// Auto-save when variables are added/edited
const handleAddVariable = () => {
if (!activeWorkflowId) return
@@ -257,7 +264,7 @@ export function Variables({ panelWidth }: VariablesProps) {
className='!text-md h-9 max-w-40 border-input bg-background focus-visible:ring-1 focus-visible:ring-ring'
placeholder='Variable name'
value={variable.name}
onChange={(e) => updateVariable(variable.id, { name: e.target.value })}
onChange={(e) => handleVariableNameChange(variable.id, e.target.value)}
/>
<DropdownMenu>

View File

@@ -6,7 +6,7 @@ import { Button } from '@/components/ui/button'
import { Card } from '@/components/ui/card'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
import { parseCronToHumanReadable } from '@/lib/schedules/utils'
import { cn, formatDateTime } from '@/lib/utils'
import { cn, formatDateTime, validateName } from '@/lib/utils'
import type { BlockConfig, SubBlockConfig } from '@/blocks/types'
import { useExecutionStore } from '@/stores/execution/store'
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
@@ -50,6 +50,7 @@ export function WorkflowBlock({ id, data }: NodeProps<WorkflowBlockProps>) {
// Refs
const blockRef = useRef<HTMLDivElement>(null)
const contentRef = useRef<HTMLDivElement>(null)
const nameInputRef = useRef<HTMLInputElement>(null)
const updateNodeInternals = useUpdateNodeInternals()
// Workflow store selectors
@@ -329,11 +330,25 @@ export function WorkflowBlock({ id, data }: NodeProps<WorkflowBlockProps>) {
const subBlockRows = groupSubBlocks(config.subBlocks, id)
// Name editing handlers
const handleNameClick = () => {
const handleNameClick = (e: React.MouseEvent) => {
e.stopPropagation() // Prevent drag handler from interfering
setEditedName(name)
setIsEditing(true)
}
// Auto-focus the input when edit mode is activated
useEffect(() => {
if (isEditing && nameInputRef.current) {
nameInputRef.current.focus()
}
}, [isEditing])
// Handle node name change with validation
const handleNodeNameChange = (newName: string) => {
const validatedName = validateName(newName)
setEditedName(validatedName.slice(0, 18))
}
const handleNameSubmit = () => {
const trimmedName = editedName.trim().slice(0, 18)
if (trimmedName && trimmedName !== name) {
@@ -432,37 +447,43 @@ export function WorkflowBlock({ id, data }: NodeProps<WorkflowBlockProps>) {
e.stopPropagation()
}}
>
<div className='flex items-center gap-3'>
<div className='flex min-w-0 flex-1 items-center gap-3'>
<div
className='flex h-7 w-7 items-center justify-center rounded'
className='flex h-7 w-7 flex-shrink-0 items-center justify-center rounded'
style={{ backgroundColor: isEnabled ? config.bgColor : 'gray' }}
>
<config.icon className='h-5 w-5 text-white' />
</div>
{isEditing ? (
<input
type='text'
value={editedName}
onChange={(e) => setEditedName(e.target.value.slice(0, 18))}
onBlur={handleNameSubmit}
onKeyDown={handleNameKeyDown}
className='w-[180px] border-none bg-transparent p-0 font-medium text-md outline-none'
maxLength={18}
/>
) : (
<span
className={cn(
'cursor-text truncate font-medium text-md hover:text-muted-foreground',
!isEnabled ? (isWide ? 'max-w-[200px]' : 'max-w-[100px]') : 'max-w-[180px]'
)}
onClick={handleNameClick}
title={name}
>
{name}
</span>
)}
<div className='min-w-0'>
{isEditing ? (
<input
ref={nameInputRef}
type='text'
value={editedName}
onChange={(e) => handleNodeNameChange(e.target.value)}
onBlur={handleNameSubmit}
onKeyDown={handleNameKeyDown}
className='border-none bg-transparent p-0 font-medium text-md outline-none'
maxLength={18}
/>
) : (
<span
className={cn(
'inline-block cursor-text font-medium text-md hover:text-muted-foreground',
!isEnabled && 'text-muted-foreground'
)}
onClick={handleNameClick}
title={name}
style={{
maxWidth: !isEnabled ? (isWide ? '200px' : '140px') : '180px',
}}
>
{name}
</span>
)}
</div>
</div>
<div className='flex items-center gap-2'>
<div className='flex flex-shrink-0 items-center gap-2'>
{!isEnabled && (
<Badge variant='secondary' className='bg-gray-100 text-gray-500 hover:bg-gray-100'>
Disabled

View File

@@ -9,6 +9,11 @@ import {
formatDuration,
formatTime,
generateApiKey,
getInvalidCharacters,
getTimezoneAbbreviation,
isValidName,
redactApiKeys,
validateName,
} from './utils'
// Mock crypto module for encryption/decryption tests
@@ -197,3 +202,204 @@ describe('formatDuration', () => {
expect(result).toBe('1h 2m')
})
})
describe('getTimezoneAbbreviation', () => {
it('should return UTC for UTC timezone', () => {
const result = getTimezoneAbbreviation('UTC')
expect(result).toBe('UTC')
})
it('should return PST/PDT for Los Angeles timezone', () => {
const winterDate = new Date('2023-01-15') // Standard time
const summerDate = new Date('2023-07-15') // Daylight time
const winterResult = getTimezoneAbbreviation('America/Los_Angeles', winterDate)
const summerResult = getTimezoneAbbreviation('America/Los_Angeles', summerDate)
expect(['PST', 'PDT']).toContain(winterResult)
expect(['PST', 'PDT']).toContain(summerResult)
})
it('should return JST for Tokyo timezone (no DST)', () => {
const winterDate = new Date('2023-01-15')
const summerDate = new Date('2023-07-15')
const winterResult = getTimezoneAbbreviation('Asia/Tokyo', winterDate)
const summerResult = getTimezoneAbbreviation('Asia/Tokyo', summerDate)
expect(winterResult).toBe('JST')
expect(summerResult).toBe('JST')
})
it('should return full timezone name for unknown timezones', () => {
const result = getTimezoneAbbreviation('Unknown/Timezone')
expect(result).toBe('Unknown/Timezone')
})
})
describe('redactApiKeys', () => {
it('should redact API keys in objects', () => {
const obj = {
apiKey: 'secret-key',
api_key: 'another-secret',
access_token: 'token-value',
secret: 'secret-value',
password: 'password-value',
normalField: 'normal-value',
}
const result = redactApiKeys(obj)
expect(result.apiKey).toBe('***REDACTED***')
expect(result.api_key).toBe('***REDACTED***')
expect(result.access_token).toBe('***REDACTED***')
expect(result.secret).toBe('***REDACTED***')
expect(result.password).toBe('***REDACTED***')
expect(result.normalField).toBe('normal-value')
})
it('should redact API keys in nested objects', () => {
const obj = {
config: {
apiKey: 'secret-key',
normalField: 'normal-value',
},
}
const result = redactApiKeys(obj)
expect(result.config.apiKey).toBe('***REDACTED***')
expect(result.config.normalField).toBe('normal-value')
})
it('should redact API keys in arrays', () => {
const arr = [{ apiKey: 'secret-key-1' }, { apiKey: 'secret-key-2' }]
const result = redactApiKeys(arr)
expect(result[0].apiKey).toBe('***REDACTED***')
expect(result[1].apiKey).toBe('***REDACTED***')
})
it('should handle primitive values', () => {
expect(redactApiKeys('string')).toBe('string')
expect(redactApiKeys(123)).toBe(123)
expect(redactApiKeys(null)).toBe(null)
expect(redactApiKeys(undefined)).toBe(undefined)
})
it('should handle complex nested structures', () => {
const obj = {
users: [
{
name: 'John',
credentials: {
apiKey: 'secret-key',
username: 'john_doe',
},
},
],
config: {
database: {
password: 'db-password',
host: 'localhost',
},
},
}
const result = redactApiKeys(obj)
expect(result.users[0].name).toBe('John')
expect(result.users[0].credentials.apiKey).toBe('***REDACTED***')
expect(result.users[0].credentials.username).toBe('john_doe')
expect(result.config.database.password).toBe('***REDACTED***')
expect(result.config.database.host).toBe('localhost')
})
})
describe('validateName', () => {
it('should remove invalid characters', () => {
const result = validateName('test@#$%name')
expect(result).toBe('testname')
})
it('should keep valid characters', () => {
const result = validateName('test_name_123')
expect(result).toBe('test_name_123')
})
it('should keep spaces', () => {
const result = validateName('test name')
expect(result).toBe('test name')
})
it('should handle empty string', () => {
const result = validateName('')
expect(result).toBe('')
})
it('should handle string with only invalid characters', () => {
const result = validateName('@#$%')
expect(result).toBe('')
})
it('should handle mixed valid and invalid characters', () => {
const result = validateName('my-workflow@2023!')
expect(result).toBe('myworkflow2023')
})
it('should collapse multiple spaces into single spaces', () => {
const result = validateName('test multiple spaces')
expect(result).toBe('test multiple spaces')
})
it('should handle mixed whitespace and invalid characters', () => {
const result = validateName('test@#$ name')
expect(result).toBe('test name')
})
})
describe('isValidName', () => {
it('should return true for valid names', () => {
expect(isValidName('test_name')).toBe(true)
expect(isValidName('test123')).toBe(true)
expect(isValidName('test name')).toBe(true)
expect(isValidName('TestName')).toBe(true)
expect(isValidName('')).toBe(true)
})
it('should return false for invalid names', () => {
expect(isValidName('test@name')).toBe(false)
expect(isValidName('test-name')).toBe(false)
expect(isValidName('test#name')).toBe(false)
expect(isValidName('test$name')).toBe(false)
expect(isValidName('test%name')).toBe(false)
})
})
describe('getInvalidCharacters', () => {
it('should return empty array for valid names', () => {
const result = getInvalidCharacters('test_name_123')
expect(result).toEqual([])
})
it('should return invalid characters', () => {
const result = getInvalidCharacters('test@#$name')
expect(result).toEqual(['@', '#', '$'])
})
it('should return unique invalid characters', () => {
const result = getInvalidCharacters('test@@##name')
expect(result).toEqual(['@', '#'])
})
it('should handle empty string', () => {
const result = getInvalidCharacters('')
expect(result).toEqual([])
})
it('should handle string with only invalid characters', () => {
const result = getInvalidCharacters('@#$%')
expect(result).toEqual(['@', '#', '$', '%'])
})
})

View File

@@ -344,3 +344,37 @@ export const redactApiKeys = (obj: any): any => {
return result
}
/**
* Validates a name by removing any characters that could cause issues
* with variable references or node naming.
*
* @param name - The name to validate
* @returns The validated name with invalid characters removed, trimmed, and collapsed whitespace
*/
export function validateName(name: string): string {
return name
.replace(/[^a-zA-Z0-9_\s]/g, '') // Remove invalid characters
.replace(/\s+/g, ' ') // Collapse multiple spaces into single spaces
}
/**
* Checks if a name contains invalid characters
*
* @param name - The name to check
* @returns True if the name is valid, false otherwise
*/
export function isValidName(name: string): boolean {
return /^[a-zA-Z0-9_\s]*$/.test(name)
}
/**
* Gets a list of invalid characters in a name
*
* @param name - The name to check
* @returns Array of invalid characters found
*/
export function getInvalidCharacters(name: string): string[] {
const invalidChars = name.match(/[^a-zA-Z0-9_\s]/g)
return invalidChars ? [...new Set(invalidChars)] : []
}