Setup Node refactor (the secret-bearing fields no longer exist on the
node at all):
• Removed app_key / app_secret / auth_code from INPUT_TYPES. The
Setup Node now exposes only storage_method, encryption_key_method,
and reconnect — none of which are credentials.
• Added a "Set credentials..." button on the node (LiteGraph button
widget installed via beforeRegisterNodeDef + onNodeCreated
prototype hook so it survives reload, drag-and-drop workflow
JSON, and PNG metadata restore).
• Click opens a browser-only modal with three <input type=password>
fields. Browser-native dot masking the whole time the user types
or pastes — no canvas-rendered plaintext, no shoulder-surf.
• Modal Save POSTs values to a new same-origin route
/dropsend/setup/stash and closes. setup() consumes the stash
entry on the next Queue (one-shot, keyed by client_id, 60-second
TTL, 32-entry cap with oldest-eviction).
• Architectural consequence: secrets never become widget values, so
they cannot enter saved workflow JSON, ComfyUI's localStorage
auto-save, PNG metadata, copy-pasted nodes, or PromptServer.history
(and therefore the unauthenticated /history HTTP endpoint).
Auto-curl auth flow (one-click second paste):
• After Run 1 prints the OAuth URL banner, setup() sends a
sid-targeted dropsend_credentials_needed WebSocket event.
• JS handler auto-opens the entry modal in the originating browser
so the user can paste auth_code as soon as Dropbox shows it,
without clicking the button between runs.
• Defense in depth: client_id is echoed in the WS payload and the
JS handler verifies it matches api.clientId before opening.
Security hardening on /dropsend/setup/stash:
• Same-origin enforcement: Origin header must match request.host.
Closes cross-origin CSRF from another browser tab even when an
attacker has sniffed a victim's client_id.
• Content-Type lockdown: rejects anything other than
application/json with HTTP 415. Closes the confused-deputy class
where form-encoded bodies could be parsed as JSON.
• Live-session check: client_id must correspond to an active
PromptServer.instance.sockets entry. Returns 403 otherwise.
Closes the stash-poisoning vector where an attacker who guesses
a victim's client_id could pre-populate credentials.
• Per-IP rate limit: 30 POSTs / 10-second sliding window, returns
429 over budget. Defense against stash-spam DoS attempting to
evict a legitimate user's entry.
• Body cap (32 KB), client_id regex validation, JSON parse errors
return 400, empty payloads dropped.
• Both rate-limit and live-session checks empirically verified via
curl smoke tests.
Reconnect now revokes the Dropbox refresh token at the source:
• reconnect=True populates DropboxAuthManager from os.environ and
calls reset(revoke_token=True).
• POST /2/auth/token/revoke with an 8-second timeout so a Dropbox
outage cannot hang the operator mid-reconnect. Local cleanup
proceeds regardless of revocation outcome.
• Revocation outcome is now logged at logger.error level (not just
print) and the failure message tells the operator to manually
disconnect at https://www.dropbox.com/account/connected_apps so
a silent revocation failure cannot leave a still-valid token at
Dropbox.
Logging:
• Single FileHandler attached to the root logger from __init__.py,
pointed at a plugin-relative dropsend.log. Replaces the four
duplicated logging.basicConfig() blocks in monitor_output.py /
encrypt_file.py / dropbox_upload.py / dropsend_uploader_node.py
that were dead code (basicConfig is a no-op once ComfyUI has
already configured root). Log records from every DropSend module
now actually persist to disk.
• Race-free creation: pre-create with os.open(O_CREAT | O_WRONLY |
O_APPEND, 0o600) before FileHandler attaches, follow-up chmod as
belt-and-suspenders. Closes the brief default-umask window
between FileHandler creating the file and post-hoc chmod.
• Rotation via RotatingFileHandler (5 MB per file, 3 backups, ~20
MB max). Bounds disk use on long-lived hosts.
• dropbox_auth_manager.py now has its own logger so revocation
successes/failures land in dropsend.log alongside stdout.
Tests (tests/ directory, unittest, no extra deps):
• test_safe_paths.py — 23 tests covering symlink rejection, path-
traversal rejection, allowlist behaviour, dropbox_dest validation
(NUL bytes, missing leading slash, parent-traversal segments),
and resolve_safe_watch_folder edge cases. safe_paths is the only
barrier between remote workflow inputs and arbitrary-file-read
on the host; this is regression coverage for that barrier.
• test_setup_node_security.py — static AST analysis of
dropsend_setup_node.py asserting that every send_sync call for a
credential event passes a non-None sid AND sits inside an
"if sid:" guard. Catches future regressions of the original
WebSocket-broadcast vulnerability deterministically. Also asserts
INPUT_TYPES never re-introduces app_key/app_secret/auth_code.
WebSocket-broadcast guard maintained on all three event sites
(dropsend_credentials_ready, dropbox_reconnect_complete,
dropsend_credentials_needed) — refuse to send when client_id is None
to prevent send_sync(..., None) from broadcasting credentials to
every connected browser tab.
README:
• New Changelog section documenting the v1.1.0 security advisory
(workflow-persistence + WS-broadcast leaks in earlier versions)
and the fixes shipped in this release.
• New "Bind ComfyUI to localhost or front it with auth" Best
Practice item (item 4) explicitly calling out that ComfyUI's
/prompt endpoint is unauthenticated.
• New "Encryption key rotation" subsection covering symmetric-key
implications, leaked-key recovery, and how to generate a new key.
• New "Uninstalling / disconnecting" subsection with step-by-step
instructions for revoking Dropbox app, resetting app secret,
removing platform secrets, and handling existing .enc files.
• Threat model reflects same-origin/Content-Type checks, log
rotation, plugin-directory metadata-leak note for multi-tenant
hosts, reconnect-revocation silent-failure note, and Dropbox
SDK error-message log review.
.gitignore:
• Expanded to cover *.bak, *.enc, *.key/.pem/.p12/.pfx, .idea/,
.pytest_cache/, build artifacts, Thumbs.db/Desktop.ini.
Bundled in this minor release:
• Single-Observer fix for the macOS _fsevents conflict that
previously prevented .enc files from being picked up by the
upload watcher.
• folder_format dropdown for AutoUploader (project_name default
plus five worldwide date formats), with two-base persistence so
switching between project-name and dated modes preserves both
patterns.
• RUNNING banner with per-setting plain-English explanations.
• Branded watching/encrypted/sent log emojis.
• Per-shell copy-paste commands and echo verification for the
COMFYUI_DROPSEND_ALLOW_SETUP gate.
145 lines
4.9 KiB
Python
145 lines
4.9 KiB
Python
# safe_paths.py
|
|
#
|
|
# Path validation helpers used by DropSend nodes to constrain user-supplied
|
|
# paths to safe locations.
|
|
#
|
|
# Threat model: a ComfyUI instance may be reachable over the network. Any
|
|
# workflow author can therefore set node inputs. Without restriction, the
|
|
# DropSend uploader's watch_folder input becomes an arbitrary-file-read
|
|
# primitive — the watcher recursively uploads everything under the path to
|
|
# the operator's Dropbox account. We constrain watch_folder to the ComfyUI
|
|
# output directory tree, with an operator-controlled allowlist for advanced
|
|
# setups.
|
|
|
|
import os
|
|
import logging
|
|
|
|
logger = logging.getLogger(__name__)
|
|
|
|
|
|
def get_output_root():
|
|
"""Return the absolute, real path of ComfyUI's output directory.
|
|
|
|
Falls back to <cwd>/output if folder_paths is unavailable (e.g. running
|
|
unit tests outside a ComfyUI process).
|
|
"""
|
|
try:
|
|
import folder_paths # type: ignore
|
|
root = folder_paths.get_output_directory()
|
|
except Exception:
|
|
root = os.path.join(os.getcwd(), "output")
|
|
return os.path.realpath(root)
|
|
|
|
|
|
def get_allowed_roots():
|
|
"""Return the list of directory roots a watch_folder is allowed to be in.
|
|
|
|
Always includes the ComfyUI output directory. Operators who need to
|
|
monitor additional locations (e.g. a custom output mount) can opt in by
|
|
setting COMFYUI_DROPSEND_ALLOWED_WATCH_PATHS to an os.pathsep-separated
|
|
list of absolute paths. Workflow inputs cannot expand this list.
|
|
"""
|
|
roots = [get_output_root()]
|
|
extra = os.environ.get("COMFYUI_DROPSEND_ALLOWED_WATCH_PATHS", "")
|
|
if extra:
|
|
for raw in extra.split(os.pathsep):
|
|
raw = raw.strip()
|
|
if raw:
|
|
roots.append(os.path.realpath(raw))
|
|
return roots
|
|
|
|
|
|
def _is_within(child, parent):
|
|
"""True iff `child` is `parent` or a descendant of it (after realpath)."""
|
|
try:
|
|
return os.path.commonpath([child, parent]) == parent
|
|
except ValueError:
|
|
# Different drives on Windows, etc.
|
|
return False
|
|
|
|
|
|
def resolve_safe_watch_folder(user_input):
|
|
"""Resolve a user-supplied watch_folder to a safe absolute path.
|
|
|
|
- Empty input → the ComfyUI output directory.
|
|
- Relative input → joined under the output directory.
|
|
- Absolute input → must resolve under one of the allowed roots.
|
|
|
|
Raises ValueError with a clear message if the input escapes the allowed
|
|
set (path traversal, symlinks pointing outside, unrelated absolute path).
|
|
"""
|
|
output_root = get_output_root()
|
|
allowed_roots = get_allowed_roots()
|
|
|
|
if user_input is None or str(user_input).strip() == "":
|
|
return output_root
|
|
|
|
candidate = str(user_input).strip()
|
|
|
|
if not os.path.isabs(candidate):
|
|
candidate = os.path.join(output_root, candidate)
|
|
|
|
resolved = os.path.realpath(candidate)
|
|
|
|
for root in allowed_roots:
|
|
if _is_within(resolved, root):
|
|
return resolved
|
|
|
|
raise ValueError(
|
|
"watch_folder must be inside the ComfyUI output directory "
|
|
f"({output_root}). Got: {user_input!r} (resolved: {resolved}). "
|
|
"If you need to monitor a different location, set the "
|
|
"COMFYUI_DROPSEND_ALLOWED_WATCH_PATHS environment variable on the "
|
|
"host before starting ComfyUI."
|
|
)
|
|
|
|
|
|
def is_safe_event_path(path):
|
|
"""True iff a Watchdog file event path is safe to process.
|
|
|
|
Rejects (a) symlinks and (b) paths whose realpath escapes the allowed
|
|
roots. Defends against symlinks placed inside the watched directory
|
|
that point at sensitive files outside it (e.g. an output/leak.png
|
|
-> ~/.ssh/id_rsa would otherwise be read, encrypted, and uploaded by
|
|
the watchers).
|
|
"""
|
|
try:
|
|
if os.path.islink(path):
|
|
return False
|
|
resolved = os.path.realpath(path)
|
|
except OSError:
|
|
return False
|
|
|
|
for root in get_allowed_roots():
|
|
if _is_within(resolved, root):
|
|
return True
|
|
return False
|
|
|
|
|
|
def validate_dropbox_dest(dest):
|
|
"""Validate a Dropbox destination folder string.
|
|
|
|
Dropbox paths are POSIX-style absolute paths. We reject empty values,
|
|
paths missing the leading slash, parent-traversal segments, and NUL
|
|
bytes. This is defense-in-depth — Dropbox itself does not interpret
|
|
`..` — but it keeps malformed inputs from being silently accepted.
|
|
"""
|
|
if dest is None or str(dest).strip() == "":
|
|
raise ValueError("dropbox_dest_folder must not be empty.")
|
|
|
|
value = str(dest).strip()
|
|
|
|
if "\x00" in value:
|
|
raise ValueError("dropbox_dest_folder contains a NUL byte.")
|
|
if not value.startswith("/"):
|
|
raise ValueError(
|
|
f"dropbox_dest_folder must start with '/'. Got: {value!r}"
|
|
)
|
|
parts = [p for p in value.split("/") if p]
|
|
if any(p == ".." for p in parts):
|
|
raise ValueError(
|
|
f"dropbox_dest_folder must not contain '..' segments. Got: {value!r}"
|
|
)
|
|
|
|
return "/" + "/".join(parts) if parts else "/"
|