Skip to content

Commit 7d64082

Browse files
authored
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
1 parent de2ce6f commit 7d64082

File tree

4 files changed

+296
-28
lines changed

4 files changed

+296
-28
lines changed

apps/sim/app/w/[id]/components/panel/components/variables/variables.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
import { Input } from '@/components/ui/input'
1919
import { ScrollArea } from '@/components/ui/scroll-area'
2020
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
21+
import { validateName } from '@/lib/utils'
2122
import { useVariablesStore } from '@/stores/panel/variables/store'
2223
import type { Variable, VariableType } from '@/stores/panel/variables/types'
2324
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
@@ -54,6 +55,12 @@ export function Variables({ panelWidth }: VariablesProps) {
5455
// Track which variables are currently being edited
5556
const [_activeEditors, setActiveEditors] = useState<Record<string, boolean>>({})
5657

58+
// Handle variable name change with validation
59+
const handleVariableNameChange = (variableId: string, newName: string) => {
60+
const validatedName = validateName(newName)
61+
updateVariable(variableId, { name: validatedName })
62+
}
63+
5764
// Auto-save when variables are added/edited
5865
const handleAddVariable = () => {
5966
if (!activeWorkflowId) return
@@ -257,7 +264,7 @@ export function Variables({ panelWidth }: VariablesProps) {
257264
className='!text-md h-9 max-w-40 border-input bg-background focus-visible:ring-1 focus-visible:ring-ring'
258265
placeholder='Variable name'
259266
value={variable.name}
260-
onChange={(e) => updateVariable(variable.id, { name: e.target.value })}
267+
onChange={(e) => handleVariableNameChange(variable.id, e.target.value)}
261268
/>
262269

263270
<DropdownMenu>

apps/sim/app/w/[id]/components/workflow-block/workflow-block.tsx

Lines changed: 48 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Button } from '@/components/ui/button'
66
import { Card } from '@/components/ui/card'
77
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
88
import { parseCronToHumanReadable } from '@/lib/schedules/utils'
9-
import { cn, formatDateTime } from '@/lib/utils'
9+
import { cn, formatDateTime, validateName } from '@/lib/utils'
1010
import type { BlockConfig, SubBlockConfig } from '@/blocks/types'
1111
import { useExecutionStore } from '@/stores/execution/store'
1212
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
@@ -50,6 +50,7 @@ export function WorkflowBlock({ id, data }: NodeProps<WorkflowBlockProps>) {
5050
// Refs
5151
const blockRef = useRef<HTMLDivElement>(null)
5252
const contentRef = useRef<HTMLDivElement>(null)
53+
const nameInputRef = useRef<HTMLInputElement>(null)
5354
const updateNodeInternals = useUpdateNodeInternals()
5455

5556
// Workflow store selectors
@@ -329,11 +330,25 @@ export function WorkflowBlock({ id, data }: NodeProps<WorkflowBlockProps>) {
329330
const subBlockRows = groupSubBlocks(config.subBlocks, id)
330331

331332
// Name editing handlers
332-
const handleNameClick = () => {
333+
const handleNameClick = (e: React.MouseEvent) => {
334+
e.stopPropagation() // Prevent drag handler from interfering
333335
setEditedName(name)
334336
setIsEditing(true)
335337
}
336338

339+
// Auto-focus the input when edit mode is activated
340+
useEffect(() => {
341+
if (isEditing && nameInputRef.current) {
342+
nameInputRef.current.focus()
343+
}
344+
}, [isEditing])
345+
346+
// Handle node name change with validation
347+
const handleNodeNameChange = (newName: string) => {
348+
const validatedName = validateName(newName)
349+
setEditedName(validatedName.slice(0, 18))
350+
}
351+
337352
const handleNameSubmit = () => {
338353
const trimmedName = editedName.trim().slice(0, 18)
339354
if (trimmedName && trimmedName !== name) {
@@ -432,37 +447,43 @@ export function WorkflowBlock({ id, data }: NodeProps<WorkflowBlockProps>) {
432447
e.stopPropagation()
433448
}}
434449
>
435-
<div className='flex items-center gap-3'>
450+
<div className='flex min-w-0 flex-1 items-center gap-3'>
436451
<div
437-
className='flex h-7 w-7 items-center justify-center rounded'
452+
className='flex h-7 w-7 flex-shrink-0 items-center justify-center rounded'
438453
style={{ backgroundColor: isEnabled ? config.bgColor : 'gray' }}
439454
>
440455
<config.icon className='h-5 w-5 text-white' />
441456
</div>
442-
{isEditing ? (
443-
<input
444-
type='text'
445-
value={editedName}
446-
onChange={(e) => setEditedName(e.target.value.slice(0, 18))}
447-
onBlur={handleNameSubmit}
448-
onKeyDown={handleNameKeyDown}
449-
className='w-[180px] border-none bg-transparent p-0 font-medium text-md outline-none'
450-
maxLength={18}
451-
/>
452-
) : (
453-
<span
454-
className={cn(
455-
'cursor-text truncate font-medium text-md hover:text-muted-foreground',
456-
!isEnabled ? (isWide ? 'max-w-[200px]' : 'max-w-[100px]') : 'max-w-[180px]'
457-
)}
458-
onClick={handleNameClick}
459-
title={name}
460-
>
461-
{name}
462-
</span>
463-
)}
457+
<div className='min-w-0'>
458+
{isEditing ? (
459+
<input
460+
ref={nameInputRef}
461+
type='text'
462+
value={editedName}
463+
onChange={(e) => handleNodeNameChange(e.target.value)}
464+
onBlur={handleNameSubmit}
465+
onKeyDown={handleNameKeyDown}
466+
className='border-none bg-transparent p-0 font-medium text-md outline-none'
467+
maxLength={18}
468+
/>
469+
) : (
470+
<span
471+
className={cn(
472+
'inline-block cursor-text font-medium text-md hover:text-muted-foreground',
473+
!isEnabled && 'text-muted-foreground'
474+
)}
475+
onClick={handleNameClick}
476+
title={name}
477+
style={{
478+
maxWidth: !isEnabled ? (isWide ? '200px' : '140px') : '180px',
479+
}}
480+
>
481+
{name}
482+
</span>
483+
)}
484+
</div>
464485
</div>
465-
<div className='flex items-center gap-2'>
486+
<div className='flex flex-shrink-0 items-center gap-2'>
466487
{!isEnabled && (
467488
<Badge variant='secondary' className='bg-gray-100 text-gray-500 hover:bg-gray-100'>
468489
Disabled

apps/sim/lib/utils.test.ts

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ import {
99
formatDuration,
1010
formatTime,
1111
generateApiKey,
12+
getInvalidCharacters,
13+
getTimezoneAbbreviation,
14+
isValidName,
15+
redactApiKeys,
16+
validateName,
1217
} from './utils'
1318

1419
// Mock crypto module for encryption/decryption tests
@@ -197,3 +202,204 @@ describe('formatDuration', () => {
197202
expect(result).toBe('1h 2m')
198203
})
199204
})
205+
206+
describe('getTimezoneAbbreviation', () => {
207+
it('should return UTC for UTC timezone', () => {
208+
const result = getTimezoneAbbreviation('UTC')
209+
expect(result).toBe('UTC')
210+
})
211+
212+
it('should return PST/PDT for Los Angeles timezone', () => {
213+
const winterDate = new Date('2023-01-15') // Standard time
214+
const summerDate = new Date('2023-07-15') // Daylight time
215+
216+
const winterResult = getTimezoneAbbreviation('America/Los_Angeles', winterDate)
217+
const summerResult = getTimezoneAbbreviation('America/Los_Angeles', summerDate)
218+
219+
expect(['PST', 'PDT']).toContain(winterResult)
220+
expect(['PST', 'PDT']).toContain(summerResult)
221+
})
222+
223+
it('should return JST for Tokyo timezone (no DST)', () => {
224+
const winterDate = new Date('2023-01-15')
225+
const summerDate = new Date('2023-07-15')
226+
227+
const winterResult = getTimezoneAbbreviation('Asia/Tokyo', winterDate)
228+
const summerResult = getTimezoneAbbreviation('Asia/Tokyo', summerDate)
229+
230+
expect(winterResult).toBe('JST')
231+
expect(summerResult).toBe('JST')
232+
})
233+
234+
it('should return full timezone name for unknown timezones', () => {
235+
const result = getTimezoneAbbreviation('Unknown/Timezone')
236+
expect(result).toBe('Unknown/Timezone')
237+
})
238+
})
239+
240+
describe('redactApiKeys', () => {
241+
it('should redact API keys in objects', () => {
242+
const obj = {
243+
apiKey: 'secret-key',
244+
api_key: 'another-secret',
245+
access_token: 'token-value',
246+
secret: 'secret-value',
247+
password: 'password-value',
248+
normalField: 'normal-value',
249+
}
250+
251+
const result = redactApiKeys(obj)
252+
253+
expect(result.apiKey).toBe('***REDACTED***')
254+
expect(result.api_key).toBe('***REDACTED***')
255+
expect(result.access_token).toBe('***REDACTED***')
256+
expect(result.secret).toBe('***REDACTED***')
257+
expect(result.password).toBe('***REDACTED***')
258+
expect(result.normalField).toBe('normal-value')
259+
})
260+
261+
it('should redact API keys in nested objects', () => {
262+
const obj = {
263+
config: {
264+
apiKey: 'secret-key',
265+
normalField: 'normal-value',
266+
},
267+
}
268+
269+
const result = redactApiKeys(obj)
270+
271+
expect(result.config.apiKey).toBe('***REDACTED***')
272+
expect(result.config.normalField).toBe('normal-value')
273+
})
274+
275+
it('should redact API keys in arrays', () => {
276+
const arr = [{ apiKey: 'secret-key-1' }, { apiKey: 'secret-key-2' }]
277+
278+
const result = redactApiKeys(arr)
279+
280+
expect(result[0].apiKey).toBe('***REDACTED***')
281+
expect(result[1].apiKey).toBe('***REDACTED***')
282+
})
283+
284+
it('should handle primitive values', () => {
285+
expect(redactApiKeys('string')).toBe('string')
286+
expect(redactApiKeys(123)).toBe(123)
287+
expect(redactApiKeys(null)).toBe(null)
288+
expect(redactApiKeys(undefined)).toBe(undefined)
289+
})
290+
291+
it('should handle complex nested structures', () => {
292+
const obj = {
293+
users: [
294+
{
295+
name: 'John',
296+
credentials: {
297+
apiKey: 'secret-key',
298+
username: 'john_doe',
299+
},
300+
},
301+
],
302+
config: {
303+
database: {
304+
password: 'db-password',
305+
host: 'localhost',
306+
},
307+
},
308+
}
309+
310+
const result = redactApiKeys(obj)
311+
312+
expect(result.users[0].name).toBe('John')
313+
expect(result.users[0].credentials.apiKey).toBe('***REDACTED***')
314+
expect(result.users[0].credentials.username).toBe('john_doe')
315+
expect(result.config.database.password).toBe('***REDACTED***')
316+
expect(result.config.database.host).toBe('localhost')
317+
})
318+
})
319+
320+
describe('validateName', () => {
321+
it('should remove invalid characters', () => {
322+
const result = validateName('test@#$%name')
323+
expect(result).toBe('testname')
324+
})
325+
326+
it('should keep valid characters', () => {
327+
const result = validateName('test_name_123')
328+
expect(result).toBe('test_name_123')
329+
})
330+
331+
it('should keep spaces', () => {
332+
const result = validateName('test name')
333+
expect(result).toBe('test name')
334+
})
335+
336+
it('should handle empty string', () => {
337+
const result = validateName('')
338+
expect(result).toBe('')
339+
})
340+
341+
it('should handle string with only invalid characters', () => {
342+
const result = validateName('@#$%')
343+
expect(result).toBe('')
344+
})
345+
346+
it('should handle mixed valid and invalid characters', () => {
347+
const result = validateName('my-workflow@2023!')
348+
expect(result).toBe('myworkflow2023')
349+
})
350+
351+
it('should collapse multiple spaces into single spaces', () => {
352+
const result = validateName('test multiple spaces')
353+
expect(result).toBe('test multiple spaces')
354+
})
355+
356+
it('should handle mixed whitespace and invalid characters', () => {
357+
const result = validateName('test@#$ name')
358+
expect(result).toBe('test name')
359+
})
360+
})
361+
362+
describe('isValidName', () => {
363+
it('should return true for valid names', () => {
364+
expect(isValidName('test_name')).toBe(true)
365+
expect(isValidName('test123')).toBe(true)
366+
expect(isValidName('test name')).toBe(true)
367+
expect(isValidName('TestName')).toBe(true)
368+
expect(isValidName('')).toBe(true)
369+
})
370+
371+
it('should return false for invalid names', () => {
372+
expect(isValidName('test@name')).toBe(false)
373+
expect(isValidName('test-name')).toBe(false)
374+
expect(isValidName('test#name')).toBe(false)
375+
expect(isValidName('test$name')).toBe(false)
376+
expect(isValidName('test%name')).toBe(false)
377+
})
378+
})
379+
380+
describe('getInvalidCharacters', () => {
381+
it('should return empty array for valid names', () => {
382+
const result = getInvalidCharacters('test_name_123')
383+
expect(result).toEqual([])
384+
})
385+
386+
it('should return invalid characters', () => {
387+
const result = getInvalidCharacters('test@#$name')
388+
expect(result).toEqual(['@', '#', '$'])
389+
})
390+
391+
it('should return unique invalid characters', () => {
392+
const result = getInvalidCharacters('test@@##name')
393+
expect(result).toEqual(['@', '#'])
394+
})
395+
396+
it('should handle empty string', () => {
397+
const result = getInvalidCharacters('')
398+
expect(result).toEqual([])
399+
})
400+
401+
it('should handle string with only invalid characters', () => {
402+
const result = getInvalidCharacters('@#$%')
403+
expect(result).toEqual(['@', '#', '$', '%'])
404+
})
405+
})

apps/sim/lib/utils.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,3 +344,37 @@ export const redactApiKeys = (obj: any): any => {
344344

345345
return result
346346
}
347+
348+
/**
349+
* Validates a name by removing any characters that could cause issues
350+
* with variable references or node naming.
351+
*
352+
* @param name - The name to validate
353+
* @returns The validated name with invalid characters removed, trimmed, and collapsed whitespace
354+
*/
355+
export function validateName(name: string): string {
356+
return name
357+
.replace(/[^a-zA-Z0-9_\s]/g, '') // Remove invalid characters
358+
.replace(/\s+/g, ' ') // Collapse multiple spaces into single spaces
359+
}
360+
361+
/**
362+
* Checks if a name contains invalid characters
363+
*
364+
* @param name - The name to check
365+
* @returns True if the name is valid, false otherwise
366+
*/
367+
export function isValidName(name: string): boolean {
368+
return /^[a-zA-Z0-9_\s]*$/.test(name)
369+
}
370+
371+
/**
372+
* Gets a list of invalid characters in a name
373+
*
374+
* @param name - The name to check
375+
* @returns Array of invalid characters found
376+
*/
377+
export function getInvalidCharacters(name: string): string[] {
378+
const invalidChars = name.match(/[^a-zA-Z0-9_\s]/g)
379+
return invalidChars ? [...new Set(invalidChars)] : []
380+
}

0 commit comments

Comments
 (0)