RomanNum3ral
/
ComfyUI
mirror of https://github.com/Comfy-Org/ComfyUI.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'master' into matt/be-1899-listassets-contract-fields

This commit is contained in:
Matt Miller 2026-07-30 16:32:06 -07:00 committed by GitHub
parent eb5dd9b54b 9b3aa08967
commit 9bab190e6b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 381 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
app/assets/api/routes.py
View File

@ -316,15 +316,29 @@ async def download_asset_content(request: web.Request) -> web.Response:
404, "FILE_NOT_FOUND", "Underlying file not found on disk."
)
# User-controlled asset content must never render inline in the app origin
# User-controlled asset content must not render inline in the app origin
# (stored XSS via SVG/HTML/XML). Force dangerous types to download and
# override any requested inline disposition. Centralised through
# folder_paths.is_dangerous_content_type so this can't drift from /view and
# /userdata (the previous inline set here omitted image/svg+xml and missed
# the charset/casing/+xml-dialect bypasses).
# override any requested inline disposition; SVG loaded into an <img> is
# exempt, see renders_safely_as_image. Centralised through folder_paths so
# this can't drift from /view and /userdata (the previous inline set here
# omitted image/svg+xml and missed the charset/casing/+xml-dialect bypasses).
extra_headers = {}
sec_fetch_dest = request.headers.get("Sec-Fetch-Dest")
if folder_paths.is_dangerous_content_type(content_type):
content_type = "application/octet-stream"
disposition = "attachment"
# This response now depends on a request header, so it must not be
# reused across destinations by a browser or intermediary cache: an
# inline SVG primed by an <img> fetch and replayed to a document
# navigation of the same URL would re-enable the stored XSS.
extra_headers["Vary"] = "Sec-Fetch-Dest"
extra_headers["Cache-Control"] = "no-store"
if not folder_paths.renders_safely_as_image(content_type, sec_fetch_dest):
content_type = "application/octet-stream"
disposition = "attachment"
# mime_type is uploader-supplied and unvalidated, so it can carry
# parameters. aiohttp rejects a charset in the content_type argument with
# ValueError, which would turn a valid inline SVG into a 500.
content_type = content_type.split(";", 1)[0].strip() or "application/octet-stream"
safe_name = (filename or "").replace("\r", "").replace("\n", "")
encoded = urllib.parse.quote(safe_name)
@ -357,6 +371,7 @@ async def download_asset_content(request: web.Request) -> web.Response:
"Content-Disposition": cd,
"Content-Length": str(file_size),
"X-Content-Type-Options": "nosniff",
**extra_headers,
},
)

2
app/logger.py
View File

@ -113,7 +113,7 @@ def setup_logger(log_level: str = 'INFO', file_outputs=None, capacity: int = 300
logger = logging.getLogger()
console_level = get_log_level(log_level)
file_levels = [get_log_level(level) for level, _ in file_outputs]
logger.setLevel(min(console_level, *file_levels))
logger.setLevel(min([console_level, *file_levels]))
formatter = ColoredFormatter("%(message)s")

15
app/user_manager.py
View File

@ -343,13 +343,22 @@ class UserManager():
# XSS). Content-Disposition: attachment is the load-bearing guard;
# the content-type override and nosniff are defence in depth.
content_type = mimetypes.guess_type(path)[0] or 'application/octet-stream'
if folder_paths.is_dangerous_content_type(content_type):
content_type = 'application/octet-stream'
user_root = self.get_request_user_filepath(request, None, create_dir=False)
is_user_css = path == os.path.abspath(os.path.join(user_root, "user.css"))
if is_user_css:
content_type = "text/css"
disposition = "inline"
else:
if folder_paths.is_dangerous_content_type(content_type):
content_type = 'application/octet-stream'
disposition = "attachment"
return web.FileResponse(path, headers={
"Content-Type": content_type,
"X-Content-Type-Options": "nosniff",
"Content-Disposition": "attachment",
"Content-Disposition": disposition,
})
@routes.post("/userdata/{file}")

7
comfy/ops.py
View File

@ -41,7 +41,7 @@ def scaled_dot_product_attention(q, k, v, *args, **kwargs):
try:
if torch.cuda.is_available() and comfy.model_management.WINDOWS:
if torch.cuda.is_available():
from torch.nn.attention import SDPBackend, sdpa_kernel
import inspect
if "set_priority" in inspect.signature(sdpa_kernel).parameters:
@ -51,7 +51,10 @@ try:
SDPBackend.MATH,
]
SDPA_BACKEND_PRIORITY.insert(0, SDPBackend.CUDNN_ATTENTION)
if comfy.model_management.WINDOWS:
SDPA_BACKEND_PRIORITY.insert(0, SDPBackend.CUDNN_ATTENTION)
else:
SDPA_BACKEND_PRIORITY.insert(1, SDPBackend.CUDNN_ATTENTION)
def scaled_dot_product_attention(q, k, v, *args, **kwargs):
if q.nelement() < 1024 * 128: # arbitrary number, for small inputs cudnn attention seems slower

35
comfy_execution/jobs.py
View File

@ -170,6 +170,19 @@ def is_previewable(media_type: str, item: dict) -> bool:
return False
def is_text_preview(media_type: str, item: dict) -> bool:
"""
Check if a previewable output item is textual rather than visual media.
Saved text files (SaveText's .txt/.md/.json) are real outputs but must not
outrank visual media when picking the job preview.
"""
if media_type == 'text':
return True
filename = item.get('filename', '').lower()
return any(filename.endswith(ext) for ext in TEXT_EXTENSIONS)
def normalize_queue_item(item: tuple, status: str) -> dict:
"""Convert queue item tuple to unified job dict.
@ -259,8 +272,13 @@ def get_outputs_summary(outputs: dict) -> tuple[int, Optional[dict]]:
Returns (outputs_count, preview_output).
Preview priority (matching frontend):
1. type="output" with previewable media
2. Any previewable media
1. type="output" visual media (saved images/video/audio/3d)
2. any other previewable visual media (e.g. temp/preview images)
3. saved text file (e.g. SaveText's .txt/.md/.json)
4. raw text (only when the job produced nothing else previewable)
Text is kept in its own slots so node/execution order can't let a text
output mask a visual one (e.g. a text node that runs before an image).
Text content entries (strings under 'text') are preview-only metadata,
matching the frontend's METADATA_KEYS: they can serve as the fallback
@ -269,6 +287,8 @@ def get_outputs_summary(outputs: dict) -> tuple[int, Optional[dict]]:
count = 0
preview_output = None
fallback_preview = None
text_file_fallback = None
text_fallback = None
for node_id, node_outputs in outputs.items():
if not isinstance(node_outputs, dict):
@ -296,8 +316,8 @@ def get_outputs_summary(outputs: dict) -> tuple[int, Optional[dict]]:
'nodeId': node_id,
'mediaType': media_type
}
if fallback_preview is None:
fallback_preview = enriched
if text_fallback is None:
text_fallback = enriched
continue
# normalize_output_item returned a dict (e.g. 3D file)
item = normalized
@ -314,12 +334,15 @@ def get_outputs_summary(outputs: dict) -> tuple[int, Optional[dict]]:
}
if 'mediaType' not in item:
enriched['mediaType'] = media_type
if item.get('type') == 'output':
if is_text_preview(media_type, item):
if text_file_fallback is None:
text_file_fallback = enriched
elif item.get('type') == 'output':
preview_output = enriched
elif fallback_preview is None:
fallback_preview = enriched
return count, preview_output or fallback_preview
return count, preview_output or fallback_preview or text_file_fallback or text_fallback
def apply_sorting(jobs: list[dict], sort_by: str, sort_order: str) -> list[dict]:

17
folder_paths.py
View File

@ -306,6 +306,23 @@ def is_dangerous_content_type(content_type: str | None) -> bool:
return normalized.endswith('+xml') or normalized.endswith('/xml')
def renders_safely_as_image(content_type: str | None, sec_fetch_dest: str | None) -> bool:
"""Return True if a dangerous `content_type` is safe to serve inline anyway.
An SVG referenced by an ``<img>`` is loaded in secure static mode: scripts
and external references are disabled, so the stored XSS that
``is_dangerous_content_type`` guards against cannot fire. The attack needs
the SVG to become a document, which is a separate ``Sec-Fetch-Dest``.
Browsers set that header themselves and script cannot override it (the
``Sec-`` prefix makes it a forbidden header name), so it is trustworthy for
this decision. Anything else, including a missing header from a non-browser
client or a proxy that strips it, fails closed.
"""
if sec_fetch_dest != 'image':
return False
return (content_type or '').split(';', 1)[0].strip().lower() == 'image/svg+xml'
def is_within_directory(directory: str, target: str) -> bool:
"""Return True if `target` resolves to a path inside `directory`.

2
main.py
View File

@ -19,7 +19,7 @@ import time
from comfy.cli_args import enables_dynamic_vram
from app.logger import setup_logger
console_log_level = get_console_log_level(args.verbose)
file_log_outputs = [('DETAIL', 'comfyui_detail.log'), *get_file_log_outputs(args.verbose)]
file_log_outputs = get_file_log_outputs(args.verbose)
setup_logger(log_level=console_log_level, file_outputs=file_log_outputs, use_stdout=args.log_stdout)
from app.assets.seeder import asset_seeder

2
requirements.txt
View File

@ -22,7 +22,7 @@ alembic
SQLAlchemy>=2.0.0
filelock
av>=16.0.0
comfy-kitchen==0.2.23
comfy-kitchen==0.2.24
comfy-aimdo==0.4.10
requests
simpleeval>=1.0.0

34
server.py
View File

@ -624,8 +624,9 @@ class PromptServer():
# For security, force renderable/active types (HTML, JS,
# CSS, SVG, XML — anything that can carry inline <script>
# and execute in the page origin) to download instead of
# displaying inline, preventing stored XSS. The
# attachment disposition is the load-bearing guard: a
# displaying inline, preventing stored XSS. SVG loaded
# into an <img> is exempt, see renders_safely_as_image.
# The attachment disposition is the load-bearing guard: a
# bare filename= hint does not force a download per
# RFC 6266, so we only attach it on the dangerous branch
# to avoid breaking inline display of legitimate images.
@ -635,18 +636,27 @@ class PromptServer():
# header's quoted-string and malform the disposition.
safe_filename = filename.replace("\\", "\\\\").replace('"', '\\"')
disposition = f"filename=\"{safe_filename}\""
headers = {"X-Content-Type-Options": "nosniff"}
sec_fetch_dest = request.headers.get('Sec-Fetch-Dest')
if folder_paths.is_dangerous_content_type(content_type):
content_type = 'application/octet-stream'
disposition = f"attachment; filename=\"{safe_filename}\""
# This response now depends on a request header, so
# it must not be reused across destinations.
# FileResponse emits Last-Modified/ETag and nothing
# sets Cache-Control on /view, which makes it
# heuristically cacheable: without these headers a
# cache could replay the inline SVG served to an
# <img> to a later document navigation of the same
# URL and re-enable the stored XSS, or replay the
# attachment to an <img> and re-break the preview.
headers["Vary"] = "Sec-Fetch-Dest"
headers["Cache-Control"] = "no-store"
if not folder_paths.renders_safely_as_image(content_type, sec_fetch_dest):
content_type = 'application/octet-stream'
disposition = f"attachment; filename=\"{safe_filename}\""
return web.FileResponse(
file,
headers={
"Content-Disposition": disposition,
"Content-Type": content_type,
"X-Content-Type-Options": "nosniff"
}
)
headers["Content-Disposition"] = disposition
headers["Content-Type"] = content_type
return web.FileResponse(file, headers=headers)
return web.Response(status=404)

191
tests-unit/security_test/test_ghsa_779p_06_inline_svg_image_dest.py Normal file
View File

@ -0,0 +1,191 @@
"""CI unit guard for the Sec-Fetch-Dest exemption to FIX #5 of GHSA-779p-m5rp-r4h4.
FIX #5 forced every SVG served by /view and the assets download route to
application/octet-stream + Content-Disposition: attachment. That blocked the
stored XSS, but it also stopped SVG node outputs and Media Assets thumbnails
from rendering, because the frontend requests them with a plain <img>.
An SVG referenced by an <img> is loaded in secure static mode: scripting and
external references are disabled, so the payload from vuln #5 cannot execute.
The attack needs the SVG to load as a document, which arrives with a different
Sec-Fetch-Dest. Browsers set that header themselves and page script cannot
override it, so renders_safely_as_image() uses it to re-allow only the <img>
case. Everything else, including a missing header, keeps the forced download.
Because the decision reads a request header, the response must also carry
Vary: Sec-Fetch-Dest and Cache-Control: no-store. Without them a cache keyed on
the URL alone can replay the inline SVG served to an <img> to a later document
navigation of the same URL, which re-enables the very XSS the exemption is
built around. The route tests at the bottom pin those headers.
server.py cannot be imported in a unit test (importing it spins up the full
PromptServer/aiohttp app and its global side effects), so the /view side is
covered by pinning the helper its closure calls.
"""
import pytest
from aiohttp import web
import folder_paths
from app.assets.api import routes as asset_routes
from app.assets.services.schemas import DownloadResolutionResult
# Every Sec-Fetch-Dest that must keep the forced download. 'document' is the
# vuln #5 attack itself; the rest either cannot execute an SVG or have no
# reason to receive one inline. None covers curl and proxies that strip the
# header.
UNSAFE_DESTS = [
'document',
'iframe',
'object',
'embed',
'frame',
'script',
'style',
'empty',
'',
None,
]
def test_svg_renders_inline_for_image_dest():
assert folder_paths.renders_safely_as_image('image/svg+xml', 'image')
def test_svg_forced_to_download_for_every_other_dest():
for dest in UNSAFE_DESTS:
assert not folder_paths.renders_safely_as_image('image/svg+xml', dest), (
f"SVG must not be served inline for Sec-Fetch-Dest={dest!r}. Only "
"an <img> load is safe, everything else can reach document context"
)
def test_missing_header_fails_closed():
assert not folder_paths.renders_safely_as_image('image/svg+xml', None)
def test_exemption_normalises_parameters_and_casing():
for content_type in ('IMAGE/SVG+XML', 'image/svg+xml; charset=utf-8', ' image/svg+xml '):
assert folder_paths.renders_safely_as_image(content_type, 'image'), (
f"{content_type!r} is an SVG and must not be excluded from the "
"exemption by casing or a charset parameter"
)
def test_other_dangerous_types_are_never_exempt():
# Only SVG is safe as an <img>. Nothing else in the blocklist may ride the
# image dest back into an inline response.
for content_type in ('text/html', 'text/javascript', 'text/css',
'application/xml', 'text/xml', 'application/xhtml+xml',
'image/svg+xml.html', 'application/rss+xml'):
assert not folder_paths.renders_safely_as_image(content_type, 'image'), (
f"{content_type!r} must stay forced-download regardless of Sec-Fetch-Dest"
)
def test_exemption_does_not_widen_the_blocklist():
# The exemption is a call-site gate, not a change to what counts as
# dangerous. is_dangerous_content_type must still flag SVG on its own.
assert folder_paths.is_dangerous_content_type('image/svg+xml')
# --- Response-level guards on the assets content route -----------------------
#
# The helper above decides correctly, but the exemption is only safe if the
# response cannot be reused across Sec-Fetch-Dest values. These mount the real
# route and assert on the headers that actually ship.
SVG_PAYLOAD = b'<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>'
ASSET_ID = "00000000-0000-4000-8000-000000000001"
CONTENT_URL = f"/api/assets/{ASSET_ID}/content"
class _StubUserManager:
def get_request_user_id(self, request):
return "test-user"
@pytest.fixture
def asset_app(monkeypatch, tmp_path):
"""Mount the real /api/assets/{id}/content route over a stored SVG."""
def _factory(stored_mime_type):
svg = tmp_path / "thumb.svg"
svg.write_bytes(SVG_PAYLOAD)
monkeypatch.setattr(asset_routes, "_ASSETS_ENABLED", True)
monkeypatch.setattr(asset_routes, "USER_MANAGER", _StubUserManager())
monkeypatch.setattr(
asset_routes,
"resolve_asset_for_download",
lambda reference_id, owner_id: DownloadResolutionResult(
abs_path=str(svg),
content_type=stored_mime_type,
download_name="thumb.svg",
),
)
app = web.Application()
app.add_routes(asset_routes.ROUTES)
return app
return _factory
@pytest.mark.asyncio
async def test_inline_svg_response_is_not_cacheable_across_destinations(
aiohttp_client, asset_app
):
client = await aiohttp_client(asset_app("image/svg+xml"))
resp = await client.get(
CONTENT_URL, params={"disposition": "inline"}, headers={"Sec-Fetch-Dest": "image"}
)
assert resp.status == 200
assert "image/svg+xml" in resp.headers.get("Content-Type", "").lower()
# The load-bearing assertion: a cache must not be able to hand this inline
# SVG to a later document navigation of the same URL.
assert "sec-fetch-dest" in resp.headers.get("Vary", "").lower(), (
"The response varies on Sec-Fetch-Dest but does not say so, so a cache "
"keyed on the URL alone can replay the inline SVG into document context."
)
assert "no-store" in resp.headers.get("Cache-Control", "").lower()
@pytest.mark.asyncio
async def test_forced_download_response_also_declares_the_variance(
aiohttp_client, asset_app
):
# The attachment branch needs the same headers, in both directions: a
# cached octet-stream replayed to an <img> re-breaks the preview this fix
# exists to restore.
client = await aiohttp_client(asset_app("image/svg+xml"))
resp = await client.get(
CONTENT_URL,
params={"disposition": "inline"},
headers={"Sec-Fetch-Dest": "document"},
)
assert resp.status == 200
assert "application/octet-stream" in resp.headers.get("Content-Type", "").lower()
assert "attachment" in resp.headers.get("Content-Disposition", "").lower()
assert "sec-fetch-dest" in resp.headers.get("Vary", "").lower()
assert "no-store" in resp.headers.get("Cache-Control", "").lower()
@pytest.mark.asyncio
async def test_parameterised_svg_mime_type_does_not_500(aiohttp_client, asset_app):
# mime_type is uploader-supplied and unvalidated. aiohttp rejects a charset
# in the content_type argument with ValueError, so the exempt branch must
# strip parameters before building the response.
client = await aiohttp_client(asset_app("image/svg+xml; charset=utf-8"))
resp = await client.get(
CONTENT_URL, params={"disposition": "inline"}, headers={"Sec-Fetch-Dest": "image"}
)
assert resp.status == 200, (
"A charset parameter on the stored mime type must not turn a valid "
"inline SVG request into a 500."
)
assert "image/svg+xml" in resp.headers.get("Content-Type", "").lower()

80
tests/execution/test_jobs.py
View File

@ -280,6 +280,86 @@ class TestGetOutputsSummary:
assert preview['filename'] == 'model.glb'
assert preview['mediaType'] == '3d'
def test_media_preview_preferred_over_text(self):
"""A visual output wins the preview even when a text node is iterated
first (regression: text could mask a later temp/preview image)."""
outputs = {
'text_node': {'text': ['a caption']},
'image_node': {'images': [{'filename': 'preview.png', 'type': 'temp'}]},
}
count, preview = get_outputs_summary(outputs)
# Text is preview-only metadata and not counted; only the image counts.
assert count == 1
assert preview['filename'] == 'preview.png'
assert preview['mediaType'] == 'images'
def test_text_used_as_preview_when_no_media(self):
"""Text is the preview only when the job produced no media output."""
outputs = {
'text_node': {'text': ['hello world']},
}
count, preview = get_outputs_summary(outputs)
assert count == 0 # text entries are not counted as outputs
assert preview['mediaType'] == 'text'
assert preview['content'] == 'hello world'
def test_media_preview_preferred_over_saved_text_file(self):
"""A visual output wins the preview over a saved text file (SaveText),
even a temp/preview image iterated after the text node."""
outputs = {
'save_text': {
'text': ['the text'],
'files': [{'filename': 'ComfyUI_00001.txt', 'subfolder': '', 'type': 'output'}],
},
'preview_image': {'images': [{'filename': 'preview.png', 'type': 'temp'}]},
}
count, preview = get_outputs_summary(outputs)
assert count == 2 # the .txt file and the image; raw text is metadata
assert preview['filename'] == 'preview.png'
assert preview['mediaType'] == 'images'
def test_saved_media_preferred_over_saved_text_file(self):
outputs = {
'save_text': {
'text': ['the text'],
'files': [{'filename': 'ComfyUI_00001.txt', 'subfolder': '', 'type': 'output'}],
},
'save_image': {'images': [{'filename': 'result.png', 'type': 'output'}]},
}
count, preview = get_outputs_summary(outputs)
assert count == 2
assert preview['filename'] == 'result.png'
def test_mime_format_file_preferred_over_saved_text_file(self):
"""Custom-node outputs previewable via MIME format (e.g. VHS videos
under arbitrary keys) rank as visual media, above saved text files."""
outputs = {
'save_text': {
'files': [{'filename': 'notes.md', 'subfolder': '', 'type': 'output'}],
},
'video_node': {
'files': [{'filename': 'clip.webm', 'format': 'video/webm', 'type': 'output'}],
},
}
count, preview = get_outputs_summary(outputs)
assert count == 2
assert preview['filename'] == 'clip.webm'
def test_saved_text_file_preferred_over_raw_text(self):
"""With no media in the job, the saved text file (a real, counted
output) is the preview rather than the raw text metadata."""
outputs = {
'save_text': {
'text': ['the text'],
'files': [{'filename': 'ComfyUI_00001.txt', 'subfolder': '', 'type': 'output'}],
},
}
count, preview = get_outputs_summary(outputs)
assert count == 1
assert preview['filename'] == 'ComfyUI_00001.txt'
assert preview['mediaType'] == 'files'
class TestHas3DExtension:
"""Unit tests for has_3d_extension()"""