[GH-ISSUE #3283] RMR: WAS Node Suite v3.2.2 #1137

Open
opened 2026-10-27 21:26:07 +00:00 by bsakaguchi · 7 comments
Owner

Originally created by @WASasquatch on GitHub (Sep 14, 2026).
Original GitHub issue: https://github.com/Comfy-Org/ComfyUI-Manager/issues/3283

Since manual review process closed I was able to get the full report, and where applicable, have made sweeping revisions to pack. After polling workflows in the wild I decided to retire a few nodes as apposed to deprecation. I've also implemented a few systems into the build process that verifies upstream vendor code, that being said, vendor code is vendor code, and out-of-bounds to modify extensively and chain-of-trust should be considered.

To help the manual review process I have a report written for the security angle of the pack, which also points out lockdowns, and non-active detections.

https://github.com/WASasquatch/was-node-suite-comfyui/blob/main/SECURITY.md (the SECURITY.md itself was flagged, as a non-executable file hehe).

Originally created by @WASasquatch on GitHub (Sep 14, 2026). Original GitHub issue: https://github.com/Comfy-Org/ComfyUI-Manager/issues/3283 Since manual review process closed I was able to get the full report, and where applicable, have made sweeping revisions to pack. After polling workflows in the wild I decided to retire a few nodes as apposed to deprecation. I've also implemented a few systems into the build process that verifies upstream vendor code, that being said, vendor code is vendor code, and out-of-bounds to modify extensively and chain-of-trust should be considered. To help the manual review process I have a report written for the security angle of the pack, which also points out lockdowns, and non-active detections. https://github.com/WASasquatch/was-node-suite-comfyui/blob/main/SECURITY.md (the SECURITY.md itself was flagged, as a non-executable file hehe).
Author
Owner

@WASasquatch commented on GitHub (Oct 2, 2026):

@ltdrdata Can I get an update here? It's been 3 weeks.

Is it possible the level of noise security scans are creating, it's not even conceivable for you to properly manually review serious node packs of scale? How is this going to work?

<!-- gh-comment-id:5955230796 --> @WASasquatch commented on GitHub (Oct 2, 2026): @ltdrdata Can I get an update here? It's been 3 weeks. Is it possible the level of noise security scans are creating, it's not even conceivable for you to properly manually review serious node packs of scale? How is this going to work?
Author
Owner

@ltdrdata commented on GitHub (Oct 3, 2026):

Sorry for the wait, and thanks for the detailed SECURITY.md.

The registry scanner is meant to be a wide net, so on a pack this size much of what it reports is noise. It will be tightened over time, but giving authors proper feedback on CNR was the more urgent gap, so I worked on that first. Your Static analysis notes match what I see.

That aside, reviewing 3.2.10 turned up one issue in the pack itself, and it is why the versions are still flagged. SECURITY.md says a path naming another machine is refused before it is resolved, and most of the pack does that through sandbox.names_another_host. The glob patterns in three nodes skip it:

  • Load Image Batch / Load Image Sequence — scan() (nodes/io/load_image_batch.py:46-57) refuses only .. in pattern.
  • Create Morph Image from Path — image_paths() (nodes/animation/create_morph_image_from_path.py:54-74) does not check the pattern at all.

An absolute pattern makes os.path.join drop the chosen folder, so glob lists whatever location the pattern names. The pattern is saved in the workflow, so whoever shares a workflow decides where that goes. When the pattern names another machine, I confirmed on Windows that the host opens a connection to it, which is enough to hand that machine the user's sign-in credentials; an ordinary same-folder pattern stayed inside the folder as expected.

Please refuse absolute patterns and patterns naming another machine before calling glob, the same way the rest of the pack already does, so matches stay inside the chosen folder.

I'll re-evaluate once this is fixed. Please publish it as a new version and mention it here.

<!-- gh-comment-id:5965784351 --> @ltdrdata commented on GitHub (Oct 3, 2026): Sorry for the wait, and thanks for the detailed SECURITY.md. The registry scanner is meant to be a wide net, so on a pack this size much of what it reports is noise. It will be tightened over time, but giving authors proper feedback on CNR was the more urgent gap, so I worked on that first. Your Static analysis notes match what I see. That aside, reviewing 3.2.10 turned up one issue in the pack itself, and it is why the versions are still flagged. SECURITY.md says a path naming another machine is refused before it is resolved, and most of the pack does that through `sandbox.names_another_host`. The glob patterns in three nodes skip it: - **Load Image Batch / Load Image Sequence** — `scan()` (`nodes/io/load_image_batch.py:46-57`) refuses only `..` in `pattern`. - **Create Morph Image from Path** — `image_paths()` (`nodes/animation/create_morph_image_from_path.py:54-74`) does not check the pattern at all. An absolute pattern makes `os.path.join` drop the chosen folder, so `glob` lists whatever location the pattern names. The pattern is saved in the workflow, so whoever shares a workflow decides where that goes. When the pattern names another machine, I confirmed on Windows that the host opens a connection to it, which is enough to hand that machine the user's sign-in credentials; an ordinary same-folder pattern stayed inside the folder as expected. Please refuse absolute patterns and patterns naming another machine before calling `glob`, the same way the rest of the pack already does, so matches stay inside the chosen folder. I'll re-evaluate once this is fixed. Please publish it as a new version and mention it here.
Author
Owner

@WASasquatch commented on GitHub (Oct 3, 2026):

Sorry for the wait, and thanks for the detailed SECURITY.md.

The registry scanner is meant to be a wide net, so on a pack this size much of what it reports is noise. It will be tightened over time, but giving authors proper feedback on CNR was the more urgent gap, so I worked on that first. Your Static analysis notes match what I see.

That aside, reviewing 3.2.10 turned up one issue in the pack itself, and it is why the versions are still flagged. SECURITY.md says a path naming another machine is refused before it is resolved, and most of the pack does that through sandbox.names_another_host. The glob patterns in three nodes skip it:

* **Load Image Batch / Load Image Sequence** — `scan()` (`nodes/io/load_image_batch.py:46-57`) refuses only `..` in `pattern`.

* **Create Morph Image from Path** — `image_paths()` (`nodes/animation/create_morph_image_from_path.py:54-74`) does not check the pattern at all.

An absolute pattern makes os.path.join drop the chosen folder, so glob lists whatever location the pattern names. The pattern is saved in the workflow, so whoever shares a workflow decides where that goes. When the pattern names another machine, I confirmed on Windows that the host opens a connection to it, which is enough to hand that machine the user's sign-in credentials; an ordinary same-folder pattern stayed inside the folder as expected.

Please refuse absolute patterns and patterns naming another machine before calling glob, the same way the rest of the pack already does, so matches stay inside the chosen folder.

I'll re-evaluate once this is fixed. Please publish it as a new version and mention it here.

Hey Data,

Thanks for the report corrections. Those are definitely gaps in my vetting. There is a lot of coverage I took on rewriting this pack so it makes sense some fell through. I'll address those.

<!-- gh-comment-id:5972114152 --> @WASasquatch commented on GitHub (Oct 3, 2026): > Sorry for the wait, and thanks for the detailed SECURITY.md. > > The registry scanner is meant to be a wide net, so on a pack this size much of what it reports is noise. It will be tightened over time, but giving authors proper feedback on CNR was the more urgent gap, so I worked on that first. Your Static analysis notes match what I see. > > That aside, reviewing 3.2.10 turned up one issue in the pack itself, and it is why the versions are still flagged. SECURITY.md says a path naming another machine is refused before it is resolved, and most of the pack does that through `sandbox.names_another_host`. The glob patterns in three nodes skip it: > > * **Load Image Batch / Load Image Sequence** — `scan()` (`nodes/io/load_image_batch.py:46-57`) refuses only `..` in `pattern`. > > * **Create Morph Image from Path** — `image_paths()` (`nodes/animation/create_morph_image_from_path.py:54-74`) does not check the pattern at all. > > > An absolute pattern makes `os.path.join` drop the chosen folder, so `glob` lists whatever location the pattern names. The pattern is saved in the workflow, so whoever shares a workflow decides where that goes. When the pattern names another machine, I confirmed on Windows that the host opens a connection to it, which is enough to hand that machine the user's sign-in credentials; an ordinary same-folder pattern stayed inside the folder as expected. > > Please refuse absolute patterns and patterns naming another machine before calling `glob`, the same way the rest of the pack already does, so matches stay inside the chosen folder. > > I'll re-evaluate once this is fixed. Please publish it as a new version and mention it here. Hey Data, Thanks for the report corrections. Those are definitely gaps in my vetting. There is a lot of coverage I took on rewriting this pack so it makes sense some fell through. I'll address those.
Author
Owner

@WASasquatch commented on GitHub (Oct 3, 2026):

Thanks again @ltdrdata , fixed in 3.2.12.

Glob patterns

Load Image Batch / Load Image Sequence and Create Morph Image from Path, plus Directory Listing and Create Grid Image, now match through one function, sandbox.glob_read.

Before anything is listed, it refuses a pattern that names another machine, carries a drive, starts at a root, or holds a dots-only segment such as .., and it keeps only matches inside the chosen folder.

Further auditing found a few more issues that are now fixed:

Your findings made me suspect more holes so I hardened auditing and re-ran scans.

  • app_exposure route / App Workflow resolved a workflow name before checking it.
  • The Image Save delimiter reached os.path.exists before it was refused.
  • EXR Load / Load Image validation and the video loaders' change checks called folder_paths.exists_annotated_filepath on the raw value.
  • EXR Save / DNG Save passed the raw prefix to get_save_image_path.
  • Diffusers Hub joined repo_id onto the model folder before validating it.
  • \??\ slipped past the host check, and ~ was expanded after it.
  • A symlink to a share was followed by resolve().
  • Typed file names in Create Morph/Video from Path, Export API and Cache were joined before containment.
  • The pack now calls ComfyUI's path helpers only through wrappers that refuse first, and no node can write its config.yaml or viewer-extensions/.

Also hardened in the same pass:

  • The Content Viewer acts only on messages from a node's own frame.
  • Remote texture and model addresses need features.network.
  • Three Compile refuses scripts.
  • Render jobs need a session, and frame size is checked before the body is read.
  • Held runs release only for their own session.
  • The PDF export no longer fetches.
  • The EXR/PSD/TIFF/PNG readers have bounded allocations.
  • load_torch_file passes safe_load=True.
  • SECURITY.md now matches the code and lists every HTTP route with what it takes.

I hope this resolves the issues.

<!-- gh-comment-id:5973259762 --> @WASasquatch commented on GitHub (Oct 3, 2026): Thanks again @ltdrdata , fixed in **3.2.12**. ### Glob patterns Load Image Batch / Load Image Sequence and Create Morph Image from Path, plus Directory Listing and Create Grid Image, now match through one function, `sandbox.glob_read`. Before anything is listed, it refuses a pattern that names another machine, carries a drive, starts at a root, or holds a dots-only segment such as `..`, and it keeps only matches inside the chosen folder. ### Further auditing found a few more issues that are now fixed: Your findings made me suspect more holes so I hardened auditing and re-ran scans. - app_exposure route / App Workflow resolved a workflow name before checking it. - The Image Save delimiter reached `os.path.exists` before it was refused. - EXR Load / Load Image validation and the video loaders' change checks called `folder_paths.exists_annotated_filepath` on the raw value. - EXR Save / DNG Save passed the raw prefix to `get_save_image_path`. - Diffusers Hub joined `repo_id` onto the model folder before validating it. - `\??\` slipped past the host check, and `~` was expanded after it. - A symlink to a share was followed by `resolve()`. - Typed file names in Create Morph/Video from Path, Export API and Cache were joined before containment. - The pack now calls ComfyUI's path helpers only through wrappers that refuse first, and no node can write its `config.yaml` or `viewer-extensions/`. ### Also hardened in the same pass: - The Content Viewer acts only on messages from a node's own frame. - Remote texture and model addresses need `features.network`. - Three Compile refuses scripts. - Render jobs need a session, and frame size is checked before the body is read. - Held runs release only for their own session. - The PDF export no longer fetches. - The EXR/PSD/TIFF/PNG readers have bounded allocations. - `load_torch_file` passes `safe_load=True`. - `SECURITY.md` now matches the code and lists every HTTP route with what it takes. I hope this resolves the issues.
Author
Owner

@ltdrdata commented on GitHub (Oct 11, 2026):

Glob fix holds (sandbox.glob_read, unchanged in 3.4.1).

Two frame-cache issues in 3.4.1 still block approval — both reproduced on real Windows:

1. A drive in name escapes the write root on Windows. Video Cache / H3 Decode / H3 Upscale pass name's last segment to FrameCache.create with only .strip(), joined by os.path.join (video_cache.py:134-137, frame_cache.py:72). X:clip drops the chosen folder → cache lands at X:\clip_00001, outside every write root. Fix: check it with sandbox.leaves_folder, like resolve_write_file does.

2. Load Video Cache can rmtree a root folder. execute() checks the cache only with resolve_read, then with delete_after_save on deletes os.path.dirname(manifest) (load_video_cache.py:114, frame_cache.py:353). A cache.wasframes uploaded to the top of input/ or output/ is accepted, and its parent is the root — so the whole root is removed. Fix: reject a cache folder that is a root, or delete only the cache's own files.

Publish a new version and I'll review it.

<!-- gh-comment-id:6106502566 --> @ltdrdata commented on GitHub (Oct 11, 2026): Glob fix holds (`sandbox.glob_read`, unchanged in 3.4.1). Two frame-cache issues in 3.4.1 still block approval — both reproduced on real Windows: **1. A drive in `name` escapes the write root on Windows.** Video Cache / H3 Decode / H3 Upscale pass `name`'s last segment to `FrameCache.create` with only `.strip()`, joined by `os.path.join` (`video_cache.py:134-137`, `frame_cache.py:72`). `X:clip` drops the chosen folder → cache lands at `X:\clip_00001`, outside every write root. Fix: check it with `sandbox.leaves_folder`, like `resolve_write_file` does. **2. Load Video Cache can `rmtree` a root folder.** `execute()` checks the cache only with `resolve_read`, then with `delete_after_save` on deletes `os.path.dirname(manifest)` (`load_video_cache.py:114`, `frame_cache.py:353`). A `cache.wasframes` uploaded to the top of input/ or output/ is accepted, and its parent is the root — so the whole root is removed. Fix: reject a cache folder that is a root, or delete only the cache's own files. Publish a new version and I'll review it.
Author
Owner

@WASasquatch commented on GitHub (Oct 16, 2026):

Hey @ltdrdata Take a look at v3.5.3(+)

<!-- gh-comment-id:6207416152 --> @WASasquatch commented on GitHub (Oct 16, 2026): Hey @ltdrdata Take a look at v3.5.3(+)
Author
Owner

@WASasquatch commented on GitHub (Oct 28, 2026):

Hey @ltdrdata how is it going? We are now on ~v3.8.0... Really seems like Comfy Registry will not be able to provide users adequately if versions need manual review all the time for a lack of a confident system.

Also, I have removed my old WAS-NS versions from my registery account, but they persist on Manager. Users are still silently downgraded and their entire installs wrecked by ComfyUI Manager. If I remove versions, they should not be public anymore, immediately. Additionally, Comfy Registry needs to retroactively scan all it's ComfyUI packages to be compliant with itself.

Are things going to improve?

<!-- gh-comment-id:6474046817 --> @WASasquatch commented on GitHub (Oct 28, 2026): Hey @ltdrdata how is it going? We are now on ~v3.8.0... Really seems like Comfy Registry will not be able to provide users adequately if versions need manual review all the time for a lack of a confident system. Also, I have removed my old WAS-NS versions from my registery account, but they persist on Manager. Users are still silently downgraded and their entire installs wrecked by ComfyUI Manager. If I remove versions, they should not be public anymore, immediately. Additionally, Comfy Registry needs to retroactively scan all it's ComfyUI packages to be compliant with itself. Are things going to improve?
Sign in to join this conversation.
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/ComfyUI-Manager#1137
No description provided.