You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Validates custom node pack names before using them in filesystem paths during install and uninstall operations.
Security impact
The custom node installer derived pack_name directly from the submitted git source and then used it for clone targets, module loading, tags, and cleanup paths. This change rejects empty names, dot segments, path separators, and unsafe characters before any filesystem operation is attempted.
This is especially important on Windows, where backslashes in a URL segment can be interpreted as path separators when building Path targets.
Changes
Add a shared pack-name validator.
Use it when deriving names from install sources.
Use it before uninstall path construction.
Add regression coverage for valid git URLs, empty names, dot segments, and Windows separator traversal.
Medium — invokeai/app/api/routers/custom_nodes.py:288-300: the directly-dangerous sink (uninstall rmtree) gained a guard but no test covers it.
The traversal-to-shutil.rmtree(target_dir) path in uninstall_custom_node_pack is exactly what makes this a security PR: target_dir = custom_nodes_path / pack_name followed by shutil.rmtree(target_dir) at line 318. The new _validate_pack_name(pack_name) call at line 298 is the guard. But the added tests (TestPackNameValidation) only exercise _extract_pack_name_from_source (the install/derivation side). Nothing asserts that the uninstall route, or _validate_pack_name itself, rejects .., ., foo/bar, or ..\x before reaching rmtree. A future refactor could drop the line-298 guard and every test would still pass while the parent-directory-deletion vector silently reopens.
Evidence chain: uninstall accepts pack_name path param directly (not through _extract_pack_name_from_source) -> builds custom_nodes_path / pack_name -> shutil.rmtree. Guard is _validate_pack_name, verified to reject .././foo/bar/..\x/foo\n and accept good_pack. The guard is correct; the coverage is not.
To expose this issue, add a test that calls _validate_pack_name("..") (and "foo/bar", ".") and asserts ValueError, plus a route-level test that DELETE of an invalid pack_name returns success=False and never invokes shutil.rmtree/workflow_records.delete.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validates custom node pack names before using them in filesystem paths during install and uninstall operations.
Security impact
The custom node installer derived
pack_namedirectly from the submitted git source and then used it for clone targets, module loading, tags, and cleanup paths. This change rejects empty names, dot segments, path separators, and unsafe characters before any filesystem operation is attempted.This is especially important on Windows, where backslashes in a URL segment can be interpreted as path separators when building
Pathtargets.Changes
Verification
python -m py_compile invokeai/app/api/routers/custom_nodes.py tests/app/routers/test_custom_nodes.pyI could not run the focused pytest module locally because the checkout is missing the
blake3dependency before test collection reaches this module.