Comfy-Org / Comfy-Org/ComfyUI

Unhandled ValueError on cross-drive path check in UserManager on Windows

Open Beginner friendly
#15,820 0 comments 0 reactions 0 assignees View on GitHub
Potential Bug
Dominant language
Python
Stars
133k
Forks
15.7k
Avg merge
1d 7h
Merged PRs (30d)
158

Description

### Custom Node Testing

- [x] I have tried disabling custom nodes and the issue persists (see [how to disable custom nodes](https://docs.comfy.org/troubleshooting/custom-node-issues#step-1%3A-test-with-all-custom-nodes-disabled) if you need help)

### Expected Behavior

When requesting a file path referencing a different drive (e.g. `C:\...` while ComfyUI is installed on `D:\`), `UserManager.get_request_user_filepath()` should gracefully return `None` (resulting in a 403/404 response) without throwing an uncaught exception.

### Actual Behavior

The server throws an unhandled exception:
`ValueError: Paths don't have the same drive`
at `app/user_manager.py:95` in `os.path.commonpath((user_root, path))`, which crashes the request handler with an HTTP 500 Internal Server Error.

### Steps to Reproduce

1. Run ComfyUI on Windows installed on drive `D:\` (or any drive other than `C:\`).
2. Make an HTTP request to `/userdata/C:%5CWindows%5Ctemp%5Ctest.txt`.
3. Notice that `os.path.join(user_root, file)` returns `C:\Windows\temp\test.txt`.
4. `os.path.commonpath` compares paths on two different drive letters (`D:` vs `C:`) and raises `ValueError: Paths don't have the same drive`.

### Debug Logs

```powershell
[INFO] ComfyUI version: 0.33.0
[INFO] Python version: 3.11.9 (tags/v3.11.9:de542f0, Apr 2 2024, 10:12:12) [MSC v.1938 64 bit (AMD64)]
[INFO] Starting server
[INFO] To see the GUI go to: http://127.0.0.1:8188

[ERROR] aiohttp.server: Error handling request
Traceback (most recent call last):
File "aiohttp\web_protocol.py", line 452, in _handle_request
resp = await request_handler(request)
File "aiohttp\web_app.py", line 543, in _handle
resp = await handler(request)
File "app\user_manager.py", line 336, in getuserdata
path = get_user_data_path(request, check_exists=True)
File "app\user_manager.py", line 324, in get_user_data_path
path = self.get_request_user_filepath(request, file)
File "app\user_manager.py", line 95, in get_request_user_filepath
if os.path.commonpath((user_root, path)) != user_root:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "", line 854, in commonpath
ValueError: Paths don't have the same drive
```

### Other

**Proposed Solution**
In `app/user_manager.py` (lines 94-96), replace the raw `os.path.commonpath` check with `folder_paths.is_within_directory()`:

```diff
--- a/app/user_manager.py
+++ b/app/user_manager.py
@@ -94,8 +94,8 @@ class UserManager:
# prevent leaving /{type}/{user}
path = os.path.abspath(os.path.join(user_root, file))
- if os.path.commonpath((user_root, path)) != user_root:
+ if not folder_paths.is_within_directory(user_root, path):
return None

Contributor guide

Open the contributing guide

Research direction

Start in app/user_manager.py at UserManager.get_request_user_filepath(), then inspect folder_paths.is_within_directory(). Reproduce the cross-drive /userdata request on Windows and verify that the method returns None, producing a 403/404 response without an unhandled ValueError or HTTP 500.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.