fix(tools): validate Tool Icon Source URL and default icon fallbacks (#6369)#6376
fix(tools): validate Tool Icon Source URL and default icon fallbacks (#6369)#6376ExileK1G wants to merge 1 commit into
Conversation
) - Require http(s) URLs when set; optional empty with server-side normalization - Tool dialog validation, save guards, and iconSrc null safe state - Item card / tools table fallbacks for invalid or missing icon URLs - formatDataGridRows: guard non-array parsed schema Co-authored-by: Cursor <cursoragent@cursor.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces server and client-side validation for tool icon URLs, ensuring they use valid http/https protocols. Key updates include new normalization utilities, enhanced icon rendering with fallbacks in UI components, improved error handling in the ToolDialog, and defensive array checks in the data grid helper. Feedback highlights a discrepancy between UI validation and rendering support for relative or data URLs, suggests more defensive string handling in the UI, and notes the strict enforcement of absolute URLs in the backend.
| const trimmed = iconSrc.trim() | ||
| if (!trimmed) return null | ||
| try { | ||
| const u = new URL(trimmed) |
There was a problem hiding this comment.
In Node.js, new URL(trimmed) will throw a TypeError if the input is not an absolute URL (e.g., just a domain like example.com). While the catch block correctly handles this by throwing a BAD_REQUEST error, it's worth noting that this implementation strictly enforces absolute URLs. This is consistent with the goal of validating the icon source and promotes fail-fast behavior for invalid external data types.
References
- When handling potentially invalid data from external sources (like an API response), prefer throwing an error for invalid input types rather than silently returning a default or empty value. This promotes fail-fast behavior.
| export function getItemCardIconBackgroundUrl(iconSrc) { | ||
| if (iconSrc == null) return null | ||
| const t = String(iconSrc).trim() | ||
| if (!t) return null | ||
| const http = getValidHttpOrHttpsToolIconUrl(t) | ||
| if (http) return http | ||
| if (t.startsWith('/') || t.startsWith('./')) return t | ||
| if (t.toLowerCase().startsWith('data:')) return t | ||
| return null | ||
| } |
There was a problem hiding this comment.
There is a slight discrepancy between the UI rendering logic and the validation logic. getItemCardIconBackgroundUrl allows relative paths (/, ./) and data: URLs for display, but the validation in ToolDialog.jsx strictly requires http or https. To ensure a safe fallback mechanism, the validation logic should be updated to match the supported rendering formats, ensuring that validly rendered icons can also be saved.
References
- When using a heuristic for detection (e.g., for content type), ensure a safe fallback mechanism is in place to correctly handle cases where the heuristic fails.
1 participant
Fixes #6369
Summary
iconSrcisnullfrom API.formatDataGridRowsreturns[]when parsed schema is not an array.How to test
pnpm build(or at leastpnpm --filter flowise-ui build) thenpnpm start.abc123) → error + cannot save.https://…icon → saves and displays.Notes
pnpm lintreports 0 errors; existing warnings in unrelated test files remain unchanged.