Remove all routes and process launches, approve folders via ComfyUI settings (1.1.5)

The registry scan report on 1.1.4 listed exactly three findings, all of them
the folder-dialog process launches in __init__.py (yara rule
python_command_injection_risk, severity "info"), and no path-traversal
findings at all: the 1.1.4 containment guard did its job. An admin escalated
1.1.2, 1.1.3 and 1.1.4 from Flagged to Banned on those three hits anyway.

The pattern itself is not against the rules (Pixaroma ships an identical
PowerShell/osascript picker and stays listed), but arguing that point does not
make the package installable, so the surface is gone instead:

- __init__.py registers no HTTP route and launches no process. The Browse
  button and POST /craftkit/browse_folder are removed.
- Folder approval moved to ComfyUI's own settings store under
  CraftKit.AllowedFolders. The node button writes it through the frontend
  settings API; Python only ever reads the file.
- craftkit_folder_guard.py drops remember_folder/dialog_available and the
  allow_any flag, so containment can no longer be disabled at all. The legacy
  allowed_folders.json is still read so folders approved with the old dialog
  keep working.
- Comments avoid naming the module and call form the scanner matches, since
  the scan reads source text.

Verified against a simulated install (10/10 cases) and end to end in ComfyUI:
button approves, Python accepts the folder and its subfolders, pre-update
approvals still work, system folders and .. traversal still refused.
This commit is contained in:
rubz
2026-09-07 21:22:14 +02:00
parent 0c15d52256
commit bad9f72000
7 changed files with 213 additions and 244 deletions
+1
View File
@@ -7,6 +7,7 @@ opus-review-craftkit-2026-07-02.md
# Local dev-only tooling, not part of the published node package
check_registry_status.bat
check_registry_status_readable.ps1
registry_status_last.txt
# Internal planning/implementation docs — never meant for the public repo
docs/superpowers/
+4 -2
View File
@@ -115,11 +115,13 @@ Load **all images from a folder**, resize each one by longest side, and save int
Use this for **bulk preprocessing** - e.g. preparing a LoRA dataset from a folder of high-res images, or a batch of frames/renders for After Effects where transparency needs to survive the resize. It works as a **standalone node**: it needs no upstream input and no downstream connection to do its job - drop it on the canvas, point it at a folder, hit **Run Batch**, and it reads, resizes, and saves everything itself. Connecting the `images`/`count` outputs is entirely optional, e.g. for previewing results.
Includes a **Browse folder** button to pick the input folder directly from the node, and quick presets (512 / 768 / 1024 / 1536) for the longest side - or type any custom value directly into the field. Widgets are grouped into labeled sections (Resize, Filename, Output Location, Output Format, Options) to keep the node scannable despite the number of settings.
Includes quick presets (512 / 768 / 1024 / 1536) for the longest side - or type any custom value directly into the field. Widgets are grouped into labeled sections (Resize, Filename, Output Location, Output Format, Options) to keep the node scannable despite the number of settings.
**Approving folders.** This node reads and writes real folders on your disk, so it only works inside folders you approved yourself. ComfyUI's own `input`, `output` and `temp` folders always work. For anything else - your dataset folder on another drive, say - paste the path into `input_folder` and click **Approve folder** once. That folder and everything under it stays approved from then on. The list lives under *Settings > CraftKit > Approved folders*, where you can also edit or remove entries by hand.
| Input | Type | Default | Description |
|---|---|---|---|
| Input folder | STRING | - | Source folder path (use Browse button or paste manually) |
| Input folder | STRING | - | Source folder path. Paste it, then click **Approve folder** once (see above) |
| Longest side (px) | INT | 1024 | Target size for longest side. Quick presets: 512 / 768 / 1024 / 1536 - or type any custom value directly into the field |
| Round to multiple of | INT | 8 | Snap dimensions to this multiple |
| Interpolation method | ENUM | lanczos | lanczos / bicubic / bilinear / nearest |
+23 -162
View File
@@ -11,165 +11,26 @@ NODE_DISPLAY_NAME_MAPPINGS = {**A_NAMES, **B_NAMES, **C_NAMES, **D_NAMES, **E_NA
__all__ = ["NODE_CLASS_MAPPINGS", "NODE_DISPLAY_NAME_MAPPINGS"]
try:
import asyncio
import concurrent.futures
import os
import threading
from aiohttp import web
from server import PromptServer
_dialog_lock = threading.Lock()
# Must match the literal 'CRAFTKIT_DIALOG_ERROR:' prefix written by the
# PowerShell catch block below (kept literal there to avoid brace-escaping
# a dense here-string).
_DIALOG_ERROR_PREFIX = "CRAFTKIT_DIALOG_ERROR:"
def _open_dialog_macos():
import subprocess
script = 'POSIX path of (choose folder with prompt "Select input folder")'
try:
result = subprocess.run(
["osascript", "-e", script],
capture_output=True, text=True, timeout=300
)
except (subprocess.TimeoutExpired, FileNotFoundError):
return ""
if result.returncode != 0:
return "" # cancelled, or osascript unavailable
return result.stdout.strip()
_LINUX_PICKER_COMMANDS = [
["zenity", "--file-selection", "--directory", "--title=Select input folder"],
["kdialog", "--getexistingdirectory", os.path.expanduser("~"), "--title", "Select input folder"],
]
def _open_dialog_linux():
import subprocess
for cmd in _LINUX_PICKER_COMMANDS:
try:
result = subprocess.run(cmd, capture_output=True, text=True, timeout=300)
except FileNotFoundError:
continue
except subprocess.TimeoutExpired:
return ""
if result.returncode != 0:
return "" # cancelled
return result.stdout.strip()
print("[CraftKit] No folder picker found (tried zenity, kdialog); paste input folder paths manually.")
return ""
def _open_dialog_windows():
# Deliberately uses the plain .NET FolderBrowserDialog instead of the
# earlier IFileDialog/COM-interop approach (which used Add-Type to
# compile inline C#, ComImport, and raw Marshal/IntPtr calls). That
# pattern reads as COM-hijack-style shellcode to automated registry
# scanners even though it was benign; this trades the modern Explorer
# look for a plain tree-view dialog, in exchange for a much smaller,
# ordinary-looking script with no dynamic code compilation.
#
# Reverting to the modern look: the full IFileDialog/COM version
# (modern Explorer-style picker, works on Windows PowerShell 5.1 too)
# lived here before commit that introduced this comment — `git log -p
# -- __init__.py` to dig it out — and can be restored if the registry
# confirms that code pattern wasn't actually the scan trigger, or once
# pwsh is common enough that this fallback rarely matters. See
# [[comfyui-browse-folder-dialog]] memory for the full history/tradeoffs.
import subprocess
ps = r"""
Add-Type -AssemblyName System.Windows.Forms
$r = ''
$o = New-Object System.Windows.Forms.Form
$o.TopMost = $true
$o.ShowInTaskbar = $false
$o.FormBorderStyle = 'None'
$o.Width = 1; $o.Height = 1; $o.Opacity = 0
$o.StartPosition = 'CenterScreen'
$o.Add_Shown({
$o.Activate()
try {
$dlg = New-Object System.Windows.Forms.FolderBrowserDialog
$dlg.Description = 'Select input folder'
$dlg.ShowNewFolderButton = $true
if ($dlg.ShowDialog($o) -eq [System.Windows.Forms.DialogResult]::OK) {
$script:r = $dlg.SelectedPath
}
} catch {
# Must match _DIALOG_ERROR_PREFIX in __init__.py.
$script:r = 'CRAFTKIT_DIALOG_ERROR:' + $_.Exception.Message
}
$o.Close()
})
$o.ShowDialog() | Out-Null
$r
"""
# PowerShell 7+ (pwsh, .NET 5+) renders FolderBrowserDialog with the
# modern Explorer-style picker automatically; Windows PowerShell 5.1
# (.NET Framework) only has the classic tree-view. Prefer pwsh when
# present and fall back to the always-available powershell.exe.
for exe in ("pwsh", "powershell"):
try:
result = subprocess.run(
[exe, "-NoProfile", "-Command", ps],
capture_output=True, text=True, timeout=300,
# Without this, launching powershell.exe pops up its own
# visible console window behind the folder dialog (ComfyUI
# itself doesn't reuse it) — the dialog is the only UI this
# is meant to show.
creationflags=subprocess.CREATE_NO_WINDOW,
)
except FileNotFoundError:
continue
except subprocess.TimeoutExpired:
return ""
return result.stdout.strip()
return ""
def _open_dialog():
import sys
if sys.platform == "win32":
return _open_dialog_windows()
elif sys.platform == "darwin":
return _open_dialog_macos()
else:
return _open_dialog_linux()
async def browse_folder(request):
if request.remote not in ("127.0.0.1", "::1"):
return web.json_response({"ok": False, "error": "forbidden"}, status=403)
origin = request.headers.get("Origin")
if origin is not None:
host = request.headers.get("Host", "")
origin_host = origin.split("://", 1)[-1]
if origin_host != host:
return web.json_response({"ok": False, "error": "forbidden"}, status=403)
if not _dialog_lock.acquire(blocking=False):
return web.json_response({"ok": False, "error": "A folder dialog is already open."}, status=409)
try:
loop = asyncio.get_running_loop()
with concurrent.futures.ThreadPoolExecutor() as pool:
folder = await loop.run_in_executor(pool, _open_dialog)
finally:
_dialog_lock.release()
if folder.startswith(_DIALOG_ERROR_PREFIX):
return web.json_response({"ok": False, "error": folder.removeprefix(_DIALOG_ERROR_PREFIX)}, status=500)
if folder and os.path.isdir(folder):
# The dialog's click-OK just happened in the operating system, not
# in this request, so this is the one place a folder can be
# authorized for SmartBatchResize's input_folder — see
# craftkit_folder_guard.py.
try:
from .craftkit_folder_guard import remember_folder
remember_folder(folder)
except Exception as e:
print(f"[CraftKit] Could not remember approved folder ({e}); it will need re-approving.")
return web.json_response({"ok": True, "path": folder})
return web.json_response({"ok": False, "cancelled": True})
PromptServer.instance.routes.post("/craftkit/browse_folder")(browse_folder)
except Exception as e:
print(f"[CraftKit] Browse folder endpoint unavailable ({e}); paste input folder paths manually.")
# This package deliberately registers no HTTP routes and launches no external
# processes.
#
# Up to 1.1.4 there was a "Browse folder" button here, backed by a POST
# /craftkit/browse_folder route that opened the native OS folder dialog by
# launching a helper process: PowerShell's FolderBrowserDialog on Windows,
# osascript on macOS, zenity/kdialog on Linux. The commands were fixed
# constants with nothing interpolated into them, and the route was
# loopback-only with an Origin/Host check.
#
# The Comfy Registry's yara scanner matches every process launch under rule
# `python_command_injection_risk` regardless of whether attacker input can
# reach it. On 1.1.2/1.1.3/1.1.4 that produced three severity-"info" hits, all
# of them these dialog calls and nothing else, after which a registry admin
# escalated each version from Flagged to Banned. Removing the calls removes the
# only finding the scanner has against this package.
#
# The scanner reads source text, so this note deliberately avoids naming the
# module involved: even a mention inside a comment can re-trigger the rule.
#
# Do not reintroduce a process launch or a PromptServer route here without
# checking the registry's current policy first. Folder approval moved to
# ComfyUI's own settings store instead — see craftkit_folder_guard.py.
+102 -69
View File
@@ -13,15 +13,32 @@ see smart_batch_resize.py:246-263 in the pre-fix version).
THE FIX: a folder is only usable if one of these holds:
a) it's inside ComfyUI's own input/output/temp directories, or
b) the user picked it in the native OS folder dialog at some point (the
click-OK happens in the operating system, outside anything a request
can reach — that's the one signal we have that a human authorized it), or
c) "allow_any": true was hand-edited into the config file (never settable
via any route — a route that could flip it would be a one-call bypass).
b) the user approved it, either via the "Approve folder" button on the node
or by editing Settings → CraftKit → Approved folders (both write the
`CraftKit.AllowedFolders` setting through ComfyUI's own settings store),
or
c) it's listed in the legacy allowed_folders.json.
Config lives at <ComfyUI user dir>/craftkit/allowed_folders.json, not inside
this plugin's folder (which is a git working tree — a stray `git add -A`
would publish the user's personal folder list).
There is deliberately no "allow everything" escape hatch. An earlier version of
this file honoured an "allow_any": true flag in the config; it was removed in
1.1.5 because a registry reviewer judges what the code makes possible, not what
the default is, and a flag that disables containment outright is a finding on
its own. Anyone who needs a wide scope approves a wide folder instead.
Up to 1.1.4, (b) was a native OS folder dialog: the click-OK happened in the
operating system, outside anything an HTTP request could reach. That was the
stronger signal, but it meant launching a helper process, which the registry
scanner flags as command injection regardless of context — see the note in
__init__.py. Approval is now a click in the ComfyUI frontend instead, which
is weaker on paper (ComfyUI's own settings route is unauthenticated too) but
still confines every path to a list the user built deliberately, rather than
accepting whatever a prompt payload contains.
Legacy config lives at <ComfyUI user dir>/craftkit/allowed_folders.json, not
inside this plugin's folder (which is a git working tree — a stray
`git add -A` would publish the user's personal folder list). It is still read
so folders approved with the old dialog keep working after an update; nothing
writes it any more.
Pattern adapted from Pixaroma's ComfyUI-Pixaroma/nodes/_path_guard.py, which
solves the identical problem for its own arbitrary-folder features.
@@ -30,12 +47,13 @@ from __future__ import annotations
import json
import os
import sys
import threading
_LOCK = threading.Lock()
_CONFIG_NAME = "allowed_folders.json"
# Setting id under which approved folders are stored in ComfyUI's own settings.
# Must stay identical to the id registered in js/smart_batch_resize.js.
SETTING_KEY = "CraftKit.AllowedFolders"
def is_path_under(child: str, *parents: str) -> bool:
"""True iff `child` resolves inside ANY of `parents`.
@@ -167,10 +185,10 @@ def _config_path() -> str:
def _read_config() -> dict:
"""Never raises — a missing or damaged file means 'nothing extra is
allowed', never an exception mid-run. `damaged` lets remember_folder
refuse to write, rather than silently overwriting a corrupt-but-recoverable
config and wiping every previously approved folder."""
out = {"folders": [], "allow_any": False, "damaged": False}
allowed', never an exception mid-run. `damaged` is kept from when this file
was still written by the native-dialog approval flow; nothing writes it any
more, so today it only downgrades a corrupt file to 'nothing extra'."""
out = {"folders": [], "damaged": False}
path = _config_path()
if not os.path.exists(path):
return out
@@ -192,60 +210,84 @@ def _read_config() -> dict:
out["folders"] = [x for x in folders if isinstance(x, str) and x.strip()]
elif folders is not None:
out["damaged"] = True
out["allow_any"] = obj.get("allow_any") is True
return out
def dialog_available() -> bool:
if os.name == "nt" or sys.platform == "darwin":
return True
return bool(os.environ.get("DISPLAY") or os.environ.get("WAYLAND_DISPLAY"))
def _settings_file() -> str:
"""ComfyUI's own settings JSON, where the Approve button stores folders.
def remember_folder(path: str) -> bool:
"""Add a folder the user picked IN THE NATIVE OS DIALOG to the allowlist.
Call this ONLY from the native-picker route (__init__.py's browse_folder),
never from anywhere an ordinary request body can reach — the dialog's
click-OK is the one signal that a human, not a request, chose this path.
Written by ComfyUI itself (the frontend POSTs to its /settings route); this
module only ever reads it. Nothing here registers a route or writes the
file, which is what keeps the package free of the patterns that got
1.1.2-1.1.4 banned — see __init__.py.
"""
if not path or not isinstance(path, str):
return False
base = None
try:
real = os.path.realpath(path)
except (OSError, ValueError, TypeError):
return False
if not os.path.isdir(real):
return False
with _LOCK:
cfg = _read_config()
if cfg["damaged"]:
return False
if is_path_under(real, *cfg["folders"]):
return True # already covered by an approved parent
kept = [f for f in cfg["folders"] if not is_path_under(f, real)]
kept.append(real)
path = _config_path()
try:
os.makedirs(os.path.dirname(path), exist_ok=True)
tmp = path + ".tmp"
with open(tmp, "w", encoding="utf-8") as f:
json.dump({"folders": kept, "allow_any": cfg["allow_any"]}, f, indent=2)
os.replace(tmp, path)
except Exception:
return False
return True
import folder_paths
base = folder_paths.get_user_directory()
except Exception:
base = None
if not base:
return ""
default = os.path.join(base, "default", "comfy.settings.json")
if os.path.exists(default):
return default
# Older layouts kept the file directly under the user directory.
flat = os.path.join(base, "comfy.settings.json")
return flat if os.path.exists(flat) else ""
def _split_folders(value) -> list:
"""Accept either a list or a ';'/newline-separated string.
The setting is a single-line text field in ComfyUI's settings UI, so ';'
is the practical separator (no Windows or POSIX path contains one).
Newlines are accepted too for anyone who pastes a list by hand.
"""
if isinstance(value, list):
raw = [x for x in value if isinstance(x, str)]
elif isinstance(value, str):
raw = value.replace("\r", "\n").replace(";", "\n").split("\n")
else:
return []
return [p for p in (_norm_raw(x) for x in raw) if p]
def settings_folders() -> list:
"""Folders the user approved via the node button or the settings UI.
Never raises: an unreadable or malformed settings file means "nothing extra
is approved", never an exception mid-run.
"""
path = _settings_file()
if not path:
return []
try:
with open(path, "r", encoding="utf-8-sig") as f:
obj = json.load(f)
except Exception:
return []
if not isinstance(obj, dict):
return []
return _split_folders(obj.get(SETTING_KEY))
def approved_folders() -> list:
"""Every folder the user approved, from both stores.
settings first (the live one, written by the Approve button), then the
legacy JSON so folders approved with the pre-1.1.5 native dialog keep
working across the update without the user re-approving them.
"""
return settings_folders() + _read_config()["folders"]
def prescreen(raw) -> bool:
"""Cheap no-filesystem screen to run BEFORE anything calls realpath/isdir
on an untrusted folder string. False = refuse outright without ever
touching the filesystem (see unc_like's docstring for why that matters)."""
cfg = _read_config()
if cfg["allow_any"]:
return True
if unc_like(raw):
return _lexically_under_any(raw, cfg["folders"])
return _lexically_under_any(raw, approved_folders())
return True
@@ -255,13 +297,10 @@ def folder_allowed(path: str) -> bool:
return False
if not prescreen(path):
return False
cfg = _read_config()
if cfg["allow_any"]:
return True
roots = comfy_roots()
if roots and is_path_under(path, *roots):
return True
folders = cfg["folders"]
folders = approved_folders()
if not folders:
return False
if is_path_under(path, *folders):
@@ -271,16 +310,10 @@ def folder_allowed(path: str) -> bool:
def denied_message(path: str) -> str:
browse = "click the Browse button on the node and pick this folder once — that approves it permanently."
manual = (
"add it to \"folders\" (or set \"allow_any\": true) in\n {config}"
).format(config=_config_path())
if dialog_available():
how = f"To approve it, {browse}\nNo folder dialog available (headless install)? Instead {manual}"
else:
how = f"No folder dialog on this machine (headless install), so approve it by hand: {manual}"
return (
f"[SmartBatchResize] Folder not approved: {path}\n"
f"{how}\n"
"To approve it: put this path in the node's input_folder field and click "
"'Approve folder'. That approves it, and everything under it, permanently.\n"
"The full list is editable under Settings > CraftKit > Approved folders.\n"
"ComfyUI's own input/output/temp folders always work without approval."
)
+81 -9
View File
@@ -3,31 +3,103 @@ import { createDividerWidget, createStatusWidget } from "./shared/canvas_widgets
import { createPresetPickerWidget } from "./shared/preset_picker_widget.mjs?v=6";
import { setWidgetVisible } from "./shared/widget_visibility.mjs?v=2";
// Approved folders live in ComfyUI's own settings store, so approving one is a
// plain settings write from the frontend: no CraftKit route, no helper process.
// The id must stay identical to SETTING_KEY in craftkit_folder_guard.py.
const SETTING_ID = "CraftKit.AllowedFolders";
function getSetting(id) {
try {
if (app.extensionManager?.setting?.get) return app.extensionManager.setting.get(id);
} catch (e) { /* older frontend, fall through */ }
try {
return app.ui?.settings?.getSettingValue?.(id);
} catch (e) {
return undefined;
}
}
async function setSetting(id, value) {
if (app.extensionManager?.setting?.set) return await app.extensionManager.setting.set(id, value);
return await app.ui.settings.setSettingValue(id, value);
}
// ';' is the separator (no path contains one); newlines are accepted for
// anyone who pastes a list into the settings field by hand. Mirrors
// _split_folders() in craftkit_folder_guard.py.
function splitFolders(value) {
const parts = Array.isArray(value)
? value
: String(value ?? "").replace(/\r/g, "\n").replace(/;/g, "\n").split("\n");
return parts.map(cleanPath).filter(Boolean);
}
function cleanPath(p) {
let s = String(p ?? "").trim().replace(/^["']+|["']+$/g, "").trim();
// Strip a trailing separator, but never turn "D:/" into "D:".
while (s.length > 3 && (s.endsWith("/") || s.endsWith("\\"))) s = s.slice(0, -1);
return s;
}
app.registerExtension({
name: "Craftopia.SmartBatchResize",
settings: [
{
id: SETTING_ID,
category: ["CraftKit", "Folders", "Approved folders"],
name: "Approved folders",
tooltip:
"Folders Smart Batch Resize is allowed to read and write, separated by ';'. " +
"Subfolders are included. Use the 'Approve folder' button on the node to add one. " +
"ComfyUI's own input/output/temp folders always work without being listed here.",
type: "text",
defaultValue: "",
},
],
async nodeCreated(node) {
if (node.comfyClass !== "SmartBatchResize") return;
const folderWidget = node.widgets?.find(w => w.name === "input_folder");
if (!folderWidget) return;
const btn = node.addWidget("button", "📁 Browse folder", null, async () => {
try {
const res = await fetch("/craftkit/browse_folder", { method: "POST" });
const data = await res.json();
if (data.ok && data.path) {
folderWidget.value = data.path;
const APPROVE_LABEL = "✅ Approve folder";
const btn = node.addWidget("button", APPROVE_LABEL, null, async () => {
const flash = (label) => {
btn.label = label;
btn.name = label;
node.setDirtyCanvas(true);
setTimeout(() => {
btn.label = APPROVE_LABEL;
btn.name = APPROVE_LABEL;
node.setDirtyCanvas(true);
}, 1800);
};
const folder = cleanPath(folderWidget.value);
if (!folder) {
flash("⚠ Paste a path first");
return;
}
try {
const existing = splitFolders(getSetting(SETTING_ID));
const known = existing.some(f => f.toLowerCase() === folder.toLowerCase());
if (known) {
flash("✔ Already approved");
return;
}
await setSetting(SETTING_ID, [...existing, folder].join(";"));
flash("✔ Folder approved");
} catch (e) {
console.error("[SmartBatchResize] Browse failed:", e);
console.error("[SmartBatchResize] Could not approve folder:", e);
flash("⚠ Approve failed (see console)");
}
}, { serialize: false });
btn.serialize = false;
// Move Browse button to right after input_folder (index 1)
// Move Approve button to right after input_folder (index 1)
const folderIdx = node.widgets.indexOf(folderWidget);
const btnIdx = node.widgets.indexOf(btn);
if (btnIdx !== folderIdx + 1) {
@@ -68,7 +140,7 @@ app.registerExtension({
addSectionDivider("skip_if_exists", "OPTIONS");
addSectionDivider("output_format", "OUTPUT FORMAT");
// Override serialize so non-serializable JS widgets (Browse, presets, status,
// Override serialize so non-serializable JS widgets (Approve, presets, status,
// run batch) are excluded from widgets_values in the saved workflow JSON.
// Without this, LiteGraph saves null slots for these widgets which then shift
// all Python widget values on load (configure runs before nodeCreated).
+1 -1
View File
@@ -1,7 +1,7 @@
[project]
name = "comfyui-craftkit"
description = "Smart resize, batch resize, resolution multiplier and prompt controller nodes for ComfyUI"
version = "1.1.4"
version = "1.1.5"
readme = "README.md"
requires-python = ">=3.10"
license = { file = "LICENSE" }
+1 -1
View File
@@ -247,7 +247,7 @@ class SmartBatchResize:
input_folder = input_folder.strip()
if not input_folder:
raise ValueError("[SmartBatchResize] No folder selected. Enter a path or use the Browse button.")
raise ValueError("[SmartBatchResize] No folder selected. Paste a folder path into input_folder.")
if not folder_allowed(input_folder):
raise ValueError(denied_message(input_folder))
if not os.path.isdir(input_folder):