Comfy-Org/desktop

Fork 0

mirror of https://github.com/Comfy-Org/desktop.git synced 2026-11-04 23:47:43 +00:00

OneDrive) #402

New issue

Closed

opened 2026-10-31 22:12:32 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 22:12:32 +00:00

Owner

Originally created by @benceruleanlu on GitHub (Nov 19, 2025).
Original GitHub issue: https://github.com/Comfy-Org/desktop/issues/1406

Originally assigned to: @benceruleanlu on GitHub.

Summary

We’ve added validation in PathHandlers.ts so new installs cannot choose paths inside the app bundle, updater cache, or OneDrive. However, existing installs may still have their basePath set to locations we now consider unsafe (e.g. D:\Apps\ComfyUI\config under the app install root), which can lead to data loss on update (see #1395).

Right now, the startup/validation path only checks that basePath is accessible, not that it’s safe according to the new rules.

Current behavior

New installs go through the Install screen and VALIDATE_INSTALL_PATH, which uses evaluatePathRestrictions() in src/handlers/pathHandlers.ts and blocks:
- paths inside path.dirname(app.getPath('exe')) (the app install root)
- paths under process.resourcesPath
- legacy LOCALAPPDATA/Programs/comfyui-electron
- updater caches (LOCALAPPDATA/comfyui-electron-updater and LOCALAPPDATA/@comfyorgcomfyui-electron-updater)
- OneDrive root (via process.env.OneDrive)

Existing installs load their base path from desktop config and only check for accessibility:

src/main-process/comfyInstallation.ts:

static async fromConfig(): Promise<ComfyInstallation | undefined> {
  const config = useDesktopConfig();
  const state = config.get('installState');
  const basePath = config.get('basePath');
  if (state && basePath) {
    await ComfySettings.load(basePath);
    return new ComfyInstallation(state, basePath, getTelemetry());
  }
}

async validate(): Promise<DesktopInstallState> {
  // ...
  const basePath = useDesktopConfig().get('basePath');
  if (basePath && (await pathAccessible(basePath))) {
    await this.updateBasePathAndVenv(basePath);
    validation.basePath = 'OK';
    // then validate venv, python, uv, packages...
  } else {
    log.error('"base_path" is inaccessible or undefined.');
    validation.basePath = 'error';
  }
}

There is no check that basePath itself passes the new restriction rules. If an older install chose D:\Apps\ComfyUI\config as the base path, we still happily run from there.

Problem

For users who originally installed ComfyUI Desktop with a base path under the app install root (e.g. D:\Apps\ComfyUI\config), updates can delete or partially wipe their user data.
#1395 is likely this kind of setup: user data was co-located with the executable / app bundle and got nuked on update.
New installs are safe, but existing installs remain vulnerable until the base path is moved.

Desired behavior

On startup/validation, if the stored basePath is in an unsafe location (inside app bundle, updater cache, or OneDrive), we should:
- Detect this explicitly, and
- Guide the user to migrate to a safe location rather than silently continuing and risking data loss.

Proposed solution

Reuse the restriction logic for existing installs
- Shared logic: factor evaluatePathRestrictions() or equivalent into a reusable helper accessible from the main process (or add a main-process wrapper that calls the same logic as VALIDATE_INSTALL_PATH).
- In ComfyInstallation.validate():
  - After confirming basePath is accessible, call the restriction checker on basePath.
  - If isInsideAppInstallDir or isInsideUpdaterCache or isOneDrive is true, treat this as an unsafe configuration.
Surface an explicit validation error/state for unsafe base paths
- Extend InstallValidation.basePath semantics to distinguish:
  - "OK" – accessible and safe
  - "error" – inaccessible (current behavior)
  - Possibly a new state like "warning" or a separate flag (e.g. unsafeBasePath) for “accessible but unsafe location”.
- The frontend maintenance view can then:
  - Show a clear message: e.g. “Your install location is inside the application bundle / updater cache / OneDrive and may be deleted during updates.”
  - Explain that the app needs to move to a new base path.
Offer a guided migration to a safe location
- On maintenance screen when an unsafe base path is detected:
  - Provide a suggested default new base path (e.g. from GET_SYSTEM_PATHS.defaultInstallPath).
  - Provide a “Choose new location” button that reuses the existing Install Location Picker (InstallLocationPicker.vue) and validateInstallPath flow.
- When the user confirms a new location:
  - Use existing APIs (ComfyConfigManager.createComfyDirectories, ComfyServerConfig.setBasePathInDefaultConfig, etc.) to prepare the new base path.
  - Optionally migrate relevant subdirectories from old base path to new base path:
    - At minimum: user, input, output.
    - Possibly custom_nodes and any other user data directories.
  - Update desktop config basePath and re-create the ComfyInstallation with the new base path.
Hard-stop running on unsafe base paths going forward
- Once this detection + migration path exists, consider treating unsafe base paths as a blocking condition:
  - Don’t start the server until the base path is moved.
  - This prevents future updates from repeatedly risking user data.

Edge cases / considerations

Already damaged installs: If a user is already in a bad state (e.g. venv missing, packages broken), the maintenance screen may need to show both “basePath unsafe” and other issues. We should decide priority / messaging.
Power users: Some advanced users may intentionally colocate data, but given the risk of updater cleanup, it’s probably safer to insist on a separate base path.

Tasks

Add a shared helper to evaluate whether a base path is inside restricted locations (app bundle, updater cache, OneDrive), reusing the same rules as VALIDATE_INSTALL_PATH.
Update ComfyInstallation.validate() to call this helper on the stored basePath.
Extend InstallValidation / maintenance UI to represent and display an "unsafe base path" condition.
Implement a flow to pick a new base path using the existing Install Location Picker.
(Optional but recommended) Implement data migration of key subdirectories from the old base path to the new one, or at least provide clear instructions for manual migration.

Related: #1395 (config deleted when updating due to basePath inside app install root).

┆Issue is synchronized with this Notion page by Unito

Originally created by @benceruleanlu on GitHub (Nov 19, 2025). Original GitHub issue: https://github.com/Comfy-Org/desktop/issues/1406 Originally assigned to: @benceruleanlu on GitHub. ### Summary We’ve added validation in `PathHandlers.ts` so new installs cannot choose paths inside the app bundle, updater cache, or OneDrive. However, existing installs may still have their `basePath` set to locations we now consider unsafe (e.g. `D:\Apps\ComfyUI\config` under the app install root), which can lead to data loss on update (see #1395). Right now, the startup/validation path only checks that `basePath` is accessible, not that it’s *safe* according to the new rules. ### Current behavior - New installs go through the Install screen and `VALIDATE_INSTALL_PATH`, which uses `evaluatePathRestrictions()` in `src/handlers/pathHandlers.ts` and blocks: - paths inside `path.dirname(app.getPath('exe'))` (the app install root) - paths under `process.resourcesPath` - legacy `LOCALAPPDATA/Programs/comfyui-electron` - updater caches (`LOCALAPPDATA/comfyui-electron-updater` and `LOCALAPPDATA/@comfyorgcomfyui-electron-updater`) - OneDrive root (via `process.env.OneDrive`) - Existing installs load their base path from desktop config and only check for accessibility: - `src/main-process/comfyInstallation.ts`: ```ts static async fromConfig(): Promise<ComfyInstallation | undefined> { const config = useDesktopConfig(); const state = config.get('installState'); const basePath = config.get('basePath'); if (state && basePath) { await ComfySettings.load(basePath); return new ComfyInstallation(state, basePath, getTelemetry()); } } async validate(): Promise<DesktopInstallState> { // ... const basePath = useDesktopConfig().get('basePath'); if (basePath && (await pathAccessible(basePath))) { await this.updateBasePathAndVenv(basePath); validation.basePath = 'OK'; // then validate venv, python, uv, packages... } else { log.error('"base_path" is inaccessible or undefined.'); validation.basePath = 'error'; } } ``` - There is no check that `basePath` itself passes the new restriction rules. If an older install chose `D:\Apps\ComfyUI\config` as the base path, we still happily run from there. ### Problem - For users who originally installed ComfyUI Desktop with a base path under the app install root (e.g. `D:\Apps\ComfyUI\config`), updates can delete or partially wipe their user data. - #1395 is likely this kind of setup: user data was co-located with the executable / app bundle and got nuked on update. - New installs are safe, but existing installs remain vulnerable until the base path is moved. ### Desired behavior - On startup/validation, if the stored `basePath` is in an unsafe location (inside app bundle, updater cache, or OneDrive), we should: - Detect this explicitly, and - Guide the user to migrate to a safe location rather than silently continuing and risking data loss. ### Proposed solution 1. **Reuse the restriction logic for existing installs** - Shared logic: factor `evaluatePathRestrictions()` or equivalent into a reusable helper accessible from the main process (or add a main-process wrapper that calls the same logic as `VALIDATE_INSTALL_PATH`). - In `ComfyInstallation.validate()`: - After confirming `basePath` is accessible, call the restriction checker on `basePath`. - If `isInsideAppInstallDir` or `isInsideUpdaterCache` or `isOneDrive` is true, treat this as an unsafe configuration. 2. **Surface an explicit validation error/state for unsafe base paths** - Extend `InstallValidation.basePath` semantics to distinguish: - `"OK"` – accessible and safe - `"error"` – inaccessible (current behavior) - Possibly a new state like `"warning"` or a separate flag (e.g. `unsafeBasePath`) for “accessible but unsafe location”. - The frontend maintenance view can then: - Show a clear message: e.g. “Your install location is inside the application bundle / updater cache / OneDrive and may be deleted during updates.” - Explain that the app needs to move to a new base path. 3. **Offer a guided migration to a safe location** - On maintenance screen when an unsafe base path is detected: - Provide a suggested default new base path (e.g. from `GET_SYSTEM_PATHS.defaultInstallPath`). - Provide a “Choose new location” button that reuses the existing Install Location Picker (`InstallLocationPicker.vue`) and `validateInstallPath` flow. - When the user confirms a new location: - Use existing APIs (`ComfyConfigManager.createComfyDirectories`, `ComfyServerConfig.setBasePathInDefaultConfig`, etc.) to prepare the new base path. - Optionally migrate relevant subdirectories from old base path to new base path: - At minimum: `user`, `input`, `output`. - Possibly `custom_nodes` and any other user data directories. - Update desktop config `basePath` and re-create the `ComfyInstallation` with the new base path. 4. **Hard-stop running on unsafe base paths going forward** - Once this detection + migration path exists, consider treating unsafe base paths as a blocking condition: - Don’t start the server until the base path is moved. - This prevents future updates from repeatedly risking user data. ### Edge cases / considerations - **Already damaged installs**: If a user is already in a bad state (e.g. venv missing, packages broken), the maintenance screen may need to show both “basePath unsafe” and other issues. We should decide priority / messaging. - **Power users**: Some advanced users may intentionally colocate data, but given the risk of updater cleanup, it’s probably safer to insist on a separate base path. ### Tasks - [ ] Add a shared helper to evaluate whether a base path is inside restricted locations (app bundle, updater cache, OneDrive), reusing the same rules as `VALIDATE_INSTALL_PATH`. - [ ] Update `ComfyInstallation.validate()` to call this helper on the stored `basePath`. - [ ] Extend `InstallValidation` / maintenance UI to represent and display an "unsafe base path" condition. - [ ] Implement a flow to pick a new base path using the existing Install Location Picker. - [ ] (Optional but recommended) Implement data migration of key subdirectories from the old base path to the new one, or at least provide clear instructions for manual migration. Related: #1395 (config deleted when updating due to basePath inside app install root). ┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-1406-Detect-and-migrate-unsafe-existing-basePath-locations-inside-app-bundle-updater-cac-2b06d73d36508131989dcf4f9b54af95) by [Unito](https://www.unito.io)

bsakaguchi

2026-10-31 22:12:32 +00:00

closed this issue
added the
enhancement
label

bsakaguchi commented

2026-10-31 22:12:33 +00:00

Author

Owner

@dosubot[bot] commented on GitHub (Nov 19, 2025):

To detect and migrate unsafe basePath locations for existing installs, you’ll want to refactor the path restriction logic currently used in the VALIDATE_INSTALL_PATH IPC handler in src/handlers/pathHandlers.ts into a shared helper function. This logic already checks for unsafe locations like the app bundle, updater cache, and OneDrive, but is only accessible via IPC and not directly from the main process. Refactoring it will let you call it from ComfyInstallation.validate() after confirming basePath accessibility, so you can explicitly detect and flag unsafe configurations for existing installs as well see code.

Once you have the shared helper, update the validation flow in ComfyInstallation.validate() (in src/main-process/comfyInstallation.ts) to call this helper after checking accessibility. If the basePath is unsafe, set a new validation state (like "warning" or an unsafeBasePath flag) instead of "OK", and surface this to the frontend so the maintenance UI can prompt the user to migrate their data see code.

This approach will let you block startup on unsafe basePaths, guide users to a safe location using the existing Install Location Picker, and (optionally) migrate key subdirectories like user, input, output, and custom_nodes to the new basePath. This matches the planned fixes and recommendations discussed in #1395 and related issues.

To reply, just mention @dosu.

Docs are dead. Just use Dosu.

@dosubot[bot] commented on GitHub (Nov 19, 2025):  To detect and migrate unsafe basePath locations for existing installs, you’ll want to refactor the path restriction logic currently used in the `VALIDATE_INSTALL_PATH` IPC handler in `src/handlers/pathHandlers.ts` into a shared helper function. This logic already checks for unsafe locations like the app bundle, updater cache, and OneDrive, but is only accessible via IPC and not directly from the main process. Refactoring it will let you call it from `ComfyInstallation.validate()` after confirming basePath accessibility, so you can explicitly detect and flag unsafe configurations for existing installs as well [see code](https://github.com/Comfy-Org/desktop/blob/35d386c3dd74c79951c79a5105df07a5f28b0c91/src/handlers/pathHandlers.ts#L14-L164). Once you have the shared helper, update the validation flow in `ComfyInstallation.validate()` (in `src/main-process/comfyInstallation.ts`) to call this helper after checking accessibility. If the basePath is unsafe, set a new validation state (like `"warning"` or an `unsafeBasePath` flag) instead of `"OK"`, and surface this to the frontend so the maintenance UI can prompt the user to migrate their data [see code](https://github.com/Comfy-Org/desktop/blob/35d386c3dd74c79951c79a5105df07a5f28b0c91/src/main-process/comfyInstallation.ts#L29-L185). This approach will let you block startup on unsafe basePaths, guide users to a safe location using the existing Install Location Picker, and (optionally) migrate key subdirectories like `user`, `input`, `output`, and `custom_nodes` to the new basePath. This matches the planned fixes and recommendations discussed in #1395 and related issues.  *To reply, just mention [@dosu](https://go.dosu.dev/dosubot?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=reply-with-mention&utm_term=Comfy-Org%2Fdesktop).* --- Docs are dead. Just use [Dosu](https://dosu.dev?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=static-docs-tagline&utm_term=Comfy-Org%2Fdesktop). [![Leave Feedback](https://img.shields.io/badge/Leave%20Feedback-555555?style=flat)](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=static-docs-feedback&utm_term=Comfy-Org%2Fdesktop#message_id=7320f85e-a62e-42bf-bea1-c1d4786cab7d) [![Ask Dosu about desktop](https://img.shields.io/badge/Ask%20Dosu%20about%20desktop-2f7b3f?style=flat&logo=data%3Aimage%2Fsvg%2Bxml%3Bbase64%2CPHN2ZyB3aWR0aD0iODYiIGhlaWdodD0iODkiIHZpZXdCb3g9IjAgMCA4NiA4OSIgZmlsbD0ibm9uZSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48cGF0aCBkPSJNNS4yOTIzNiAxMi43OTI4TDE3Ljc1OTMgNi42ODE4OFY3Mi41NjY3TDUuMjkyMzYgODQuMDYxOFYxMi43OTI4WiIgZmlsbD0iI0I0QkI5MSIvPjxwYXRoIGQ9Ik0xOC4yNTc1IDczLjExOTZMNTkuMTMyOSA3Mi43NDhMNTEuNzAxMSA4Mi40MDk1TDI5LjAzMzggODYuMjkxTDYuMjM5NjIgODUuMTU1NEwxOC4yNTc1IDczLjExOTZaIiBmaWxsPSIjNzc4NTYxIi8%2BPHBhdGggZD0iTTE3LjQ5MTYgMy43MzYzM0wzLjU4NTU3IDEyLjcwOTlWODMuNTc5MkMzLjU4NTU3IDg0Ljc1NDIgNC45ODU2MyA4NS4zNjUyIDUuODQ3MDUgODQuNTY2TDE5LjYyOTYgNzEuNzgwMSIgc3Ryb2tlPSJibGFjayIgc3Ryb2tlLXdpZHRoPSI2LjQyODQ0IiBzdHJva2UtbGluZWNhcD0icm91bmQiLz48bWFzayBpZD0iZG9zdS1kLWN1dG91dCIgZmlsbD0id2hpdGUiPjxwYXRoIGZpbGwtcnVsZT0iZXZlbm9kZCIgY2xpcC1ydWxlPSJldmVub2RkIiBkPSJNNDAuNzA0IDAuNTE4MDY2SDE3LjA0MzlWNzYuMjIyMUg0MC43MDRINDIuNTgwNUg0Ny44MDEzQzY4LjcwNjQgNzYuMjIyMSA4NS42NTMzIDU5LjI3NTIgODUuNjUzMyAzOC4zNzAxQzg1LjY1MzMgMTcuNDY1IDY4LjcwNjMgMC41MTgwNjYgNDcuODAxMyAwLjUxODA2Nkg0Mi41ODA1SDQwLjcwNFoiLz48L21hc2s%2BPHBhdGggZmlsbC1ydWxlPSJldmVub2RkIiBjbGlwLXJ1bGU9ImV2ZW5vZGQiIGQ9Ik00MC43MDQgMC41MTgwNjZIMTcuMDQzOVY3Ni4yMjIxSDQwLjcwNEg0Mi41ODA1SDQ3LjgwMTNDNjguNzA2NCA3Ni4yMjIxIDg1LjY1MzMgNTkuMjc1MiA4NS42NTMzIDM4LjM3MDFDODUuNjUzMyAxNy40NjUgNjguNzA2MyAwLjUxODA2NiA0Ny44MDEzIDAuNTE4MDY2SDQyLjU4MDVINDAuNzA0WiIgZmlsbD0iI0YzRjZGMSIvPjxwYXRoIGQ9Ik0xNy4wNDM5IDAuNTE4MDY2Vi02LjU3OTE5SDkuOTQ2NjlWMC41MTgwNjZIMTcuMDQzOVpNMTcuMDQzOSA3Ni4yMjIxSDkuOTQ2NjlWODMuMzE5NEgxNy4wNDM5Vjc2LjIyMjFaTTE3LjA0MzkgNy42MTUzMkg0MC43MDRWLTYuNTc5MTlIMTcuMDQzOVY3LjYxNTMyWk0yNC4xNDEyIDc2LjIyMjFWMC41MTgwNjZIOS45NDY2OVY3Ni4yMjIxSDI0LjE0MTJaTTQwLjcwNCA2OS4xMjQ5SDE3LjA0MzlWODMuMzE5NEg0MC43MDRWNjkuMTI0OVpNNDIuNTgwNSA2OS4xMjQ5SDQwLjcwNFY4My4zMTk0SDQyLjU4MDVWNjkuMTI0OVpNNDcuODAxMyA2OS4xMjQ5SDQyLjU4MDVWODMuMzE5NEg0Ny44MDEzVjY5LjEyNDlaTTc4LjU1NiAzOC4zNzAxQzc4LjU1NiA1NS4zNTU1IDY0Ljc4NjcgNjkuMTI0OSA0Ny44MDEzIDY5LjEyNDlWODMuMzE5NEM3Mi42MjYxIDgzLjMxOTQgOTIuNzUwNSA2My4xOTQ5IDkyLjc1MDUgMzguMzcwMUg3OC41NTZaTTQ3LjgwMTMgNy42MTUzMkM2NC43ODY2IDcuNjE1MzIgNzguNTU2IDIxLjM4NDcgNzguNTU2IDM4LjM3MDFIOTIuNzUwNUM5Mi43NTA1IDEzLjU0NTMgNzIuNjI2IC02LjU3OTE5IDQ3LjgwMTMgLTYuNTc5MTlWNy42MTUzMlpNNDIuNTgwNSA3LjYxNTMySDQ3LjgwMTNWLTYuNTc5MTlINDIuNTgwNVY3LjYxNTMyWk00MC43MDQgNy42MTUzMkg0Mi41ODA1Vi02LjU3OTE5SDQwLjcwNFY3LjYxNTMyWiIgZmlsbD0iYmxhY2siIG1hc2s9InVybCgjZG9zdS1kLWN1dG91dCkiLz48cGF0aCBkPSJNNjguOTIxNSAzNi4wMTM1QzY4LjkyMTUgMzYuMDEzNSA2NS43MzY5IDQ5LjQ3MzggNTEuNDIzMSA0OS40NzM4QzM3LjEwOTMgNDkuNDczOCAzMi41Nzg3IDM3LjM1OTYgMzIuNTc4NyAzNi4wMTM1IiBzdHJva2U9ImJsYWNrIiBzdHJva2Utd2lkdGg9IjcuNjkxNjEiIHN0cm9rZS1saW5lY2FwPSJyb3VuZCIgc3Ryb2tlLWxpbmVqb2luPSJyb3VuZCIvPjxwYXRoIGQ9Ik0wLjM0ODYzMyA4NS40OTQ2QzAuMzQ4NjMzIDg1LjQ5NDYgMjkuNDg1NiA4NS44MzA5IDM0LjgwOSA4NS42OThDNDQuODMzNyA4NS40NDc3IDUxLjI4NzIgODQuNDAyIDU3LjUyNjkgNzguOTcyNEM2Mi44MTI5IDc0LjM3MjcgNzUuMTM0MiA1OS42ODM2IDc1LjEzNDIgNTkuNjgzNiIgc3Ryb2tlPSJibGFjayIgc3Ryb2tlLXdpZHRoPSI2LjE2NDgyIi8%2BPC9zdmc%2B)](https://github.dosu.com/Comfy-Org/desktop?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=static-docs-ask-repo&utm_term=Comfy-Org%2Fdesktop) [![Share Dosu with your team](https://img.shields.io/badge/Share%20Dosu%20with%20your%20team-1f6feb?style=flat)](https://app.dosu.dev/signup?referrer=openSource&source=github-footer&utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=static-docs-share-team&utm_term=Comfy-Org%2Fdesktop)

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/desktop#402

No description provided.

Rows
Columns