Files
MachinePainting 09362f42c7 v1.1.0: refactor Setup Node, security hardening, auto-curl auth flow
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.
2026-05-09 05:44:47 -07:00

190 lines
8.1 KiB
Python

# dropbox_auth_manager.py
import os
import logging
import requests
import json
logger = logging.getLogger(__name__)
class DropboxAuthManager:
def __init__(self, app_key=None, app_secret=None, refresh_token=None):
self.app_key = app_key
self.app_secret = app_secret
self.refresh_token = refresh_token
def _get_credentials_file_path(self):
"""Path to a legacy JSON credential file from older versions.
The current code never writes this file — credentials live in
`.env` (or are delivered to the browser via WebSocket and never
touch disk). The path is kept only so reset() can clean up a
stale file left over from earlier installs.
"""
plugin_dir = os.path.dirname(os.path.abspath(__file__))
return os.path.join(plugin_dir, ".dropbox_credentials.json")
def is_connected(self):
return bool(self.app_key and self.app_secret and self.refresh_token)
def exchange_auth_code_raw(self, auth_code, redirect_uri=None):
"""Exchange auth code for tokens without storing them"""
if not (self.app_key and self.app_secret):
raise ValueError("App key and secret must be set before exchanging auth code.")
data = {
"code": auth_code,
"grant_type": "authorization_code",
"client_id": self.app_key,
"client_secret": self.app_secret
}
# Include redirect_uri if it was used in the authorization request
if redirect_uri:
data["redirect_uri"] = redirect_uri
headers = {"Content-Type": "application/x-www-form-urlencoded"}
# Do not log `data` or `response.text` here — both contain secrets
# (client_secret, refresh_token, access_token). On platforms that
# capture stdout (RunPod, Docker, CI) those would land in shared logs.
response = requests.post("https://api.dropbox.com/oauth2/token", headers=headers, data=data)
print(f"[DropboxAuthManager] Token exchange response status: {response.status_code}")
# If 400 error, try without redirect_uri. This handles cases where
# the auth code was obtained without a callback URL.
if response.status_code == 400 and redirect_uri:
print("[DropboxAuthManager] 400 with redirect_uri, retrying without it")
data_without_redirect = {
"code": auth_code,
"grant_type": "authorization_code",
"client_id": self.app_key,
"client_secret": self.app_secret
}
response = requests.post("https://api.dropbox.com/oauth2/token", headers=headers, data=data_without_redirect)
print(f"[DropboxAuthManager] Retry response status: {response.status_code}")
response.raise_for_status()
return response.json()
def get_access_token(self):
if not self.is_connected():
raise RuntimeError("Dropbox not connected. Please run authentication first.")
data = {
"refresh_token": self.refresh_token,
"grant_type": "refresh_token",
"client_id": self.app_key,
"client_secret": self.app_secret
}
headers = {"Content-Type": "application/x-www-form-urlencoded"}
response = requests.post("https://api.dropbox.com/oauth2/token", headers=headers, data=data)
response.raise_for_status()
access_token = response.json().get("access_token")
if not access_token:
raise RuntimeError("Failed to retrieve access token.")
return access_token
def reset(self, revoke_token=True):
"""Clear stored tokens and optionally revoke authorization with Dropbox"""
if revoke_token:
# Revoke the token with Dropbox if we have it in memory.
# The legacy auto-load from .dropbox_credentials.json is gone;
# callers that want to revoke must construct this manager with
# creds (or set them) before calling reset(revoke_token=True).
if self.refresh_token and self.app_key and self.app_secret:
try:
print("[DropboxAuthManager] Revoking token with Dropbox...")
# Get current access token to revoke
access_token = self.get_access_token()
# Revoke the token with Dropbox
revoke_data = {
"token": access_token
}
headers = {
"Authorization": f"Bearer {access_token}",
"Content-Type": "application/json"
}
response = requests.post(
"https://api.dropboxapi.com/2/auth/token/revoke",
headers=headers,
json=revoke_data,
# Bound the wait so a Dropbox outage or DNS
# failure doesn't hang the user mid-reconnect.
# Local-cleanup is the priority here; the
# token-revoke is best-effort (and Dropbox
# tokens expire on their own anyway).
timeout=8,
)
if response.status_code == 200:
msg = "[DropboxAuthManager] Token successfully revoked with Dropbox"
print(msg)
logger.info(msg)
else:
# Don't log response.text — it can echo the access token on some failures.
msg = (
f"[DropboxAuthManager] Token revocation FAILED at Dropbox "
f"(HTTP {response.status_code}). The refresh token may still "
f"be valid. Manually disconnect at "
f"https://www.dropbox.com/account/connected_apps to be sure."
)
print(msg)
logger.error(msg)
except Exception as e:
# Common case: Dropbox unreachable, refresh token already
# invalidated, or get_access_token() couldn't mint one.
# Local cleanup proceeds, but the operator needs to know
# the token may still be live at Dropbox.
msg = (
f"[DropboxAuthManager] Token revocation FAILED locally: {e}. "
f"The refresh token may still be valid at Dropbox. Manually "
f"disconnect at https://www.dropbox.com/account/connected_apps."
)
print(msg)
logger.error(msg)
else:
print("[DropboxAuthManager] Skipping token revocation (revoke_token=False)")
# Clear credentials file
try:
credentials_path = self._get_credentials_file_path()
if os.path.exists(credentials_path):
os.remove(credentials_path)
print(f"[DropboxAuthManager] Removed credentials file: {credentials_path}")
except Exception as e:
print(f"[DropboxAuthManager] Could not remove credentials file: {e}")
# Clear instance variables
self.app_key = None
self.app_secret = None
self.refresh_token = None
def get_oauth_url(self, redirect_uri=None, state=None, force_reapprove=False):
"""Generate OAuth URL for initial authorization"""
if not self.app_key:
raise ValueError("App key must be set to generate OAuth URL.")
url = f"https://www.dropbox.com/oauth2/authorize?response_type=code&client_id={self.app_key}&token_access_type=offline"
if redirect_uri:
url += f"&redirect_uri={redirect_uri}"
if state:
url += f"&state={state}"
# Force reapproval to ensure user sees authorization screen
if force_reapprove:
url += "&force_reapprove=true"
return url