-
Notifications
You must be signed in to change notification settings - Fork 406
sync devtools nodes with test ComfyUI dir in test global setup and add dev nodes for all new widget types #6623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
fcdb5dd
9985101
4699859
6596aa8
5b33d3f
8fc5e87
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import fs from 'fs-extra' | ||
| import path from 'path' | ||
| import { fileURLToPath } from 'url' | ||
|
|
||
| export function syncDevtools(targetComfyDir: string): boolean { | ||
| if (!targetComfyDir) { | ||
| console.warn('syncDevtools skipped: TEST_COMFYUI_DIR not set') | ||
| return false | ||
| } | ||
|
|
||
| // Validate and sanitize the target directory path | ||
| const resolvedTargetDir = path.resolve(targetComfyDir) | ||
|
|
||
| // Basic path validation to prevent directory traversal | ||
| if (resolvedTargetDir.includes('..') || !path.isAbsolute(resolvedTargetDir)) { | ||
| console.error('syncDevtools failed: Invalid target directory path') | ||
| return false | ||
| } | ||
|
|
||
| const moduleDir = | ||
| typeof __dirname !== 'undefined' | ||
| ? __dirname | ||
| : path.dirname(fileURLToPath(import.meta.url)) | ||
|
|
||
| const devtoolsSrc = path.resolve(moduleDir, '..', '..', 'tools', 'devtools') | ||
|
|
||
| if (!fs.pathExistsSync(devtoolsSrc)) { | ||
| console.warn( | ||
| `syncDevtools skipped: source directory not found at ${devtoolsSrc}` | ||
| ) | ||
| return false | ||
| } | ||
|
|
||
| const devtoolsDest = path.resolve( | ||
| resolvedTargetDir, | ||
| 'custom_nodes', | ||
| 'ComfyUI_devtools' | ||
| ) | ||
|
|
||
| console.warn(`syncDevtools: copying ${devtoolsSrc} -> ${devtoolsDest}`) | ||
|
|
||
| try { | ||
| fs.removeSync(devtoolsDest) | ||
| fs.ensureDirSync(devtoolsDest) | ||
| fs.copySync(devtoolsSrc, devtoolsDest, { overwrite: true }) | ||
| console.warn('syncDevtools: copy complete') | ||
| return true | ||
| } catch (error) { | ||
| console.error(`Failed to sync DevTools to ${devtoolsDest}:`, error) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [quality] medium Priority Issue: Error logging uses console.error but function continues execution after failure |
||
| return false | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[security] high Priority
Issue: Directory traversal vulnerability - fs.copySync without path validation
Context: The devtoolsDest path is constructed from user input without validation, potentially allowing directory traversal attacks
Suggestion: Validate and sanitize the targetComfyDir path before using it in file operations. Use path.resolve() and check that the resolved path stays within expected boundaries