diff --git a/app/assets/api/routes.py b/app/assets/api/routes.py index 13ff6a38c..ede00008c 100644 --- a/app/assets/api/routes.py +++ b/app/assets/api/routes.py @@ -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 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 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, }, ) diff --git a/app/logger.py b/app/logger.py index 1aed54e37..2e6116813 100644 --- a/app/logger.py +++ b/app/logger.py @@ -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") diff --git a/app/user_manager.py b/app/user_manager.py index de261ad39..55e7e81e3 100644 --- a/app/user_manager.py +++ b/app/user_manager.py @@ -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}") diff --git a/comfy/ops.py b/comfy/ops.py index 5e1cce333..9d692dcc7 100644 --- a/comfy/ops.py +++ b/comfy/ops.py @@ -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 diff --git a/comfy_execution/jobs.py b/comfy_execution/jobs.py index f0ad59f86..34c06363b 100644 --- a/comfy_execution/jobs.py +++ b/comfy_execution/jobs.py @@ -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]: diff --git a/folder_paths.py b/folder_paths.py index bd3f25095..df53542dc 100644 --- a/folder_paths.py +++ b/folder_paths.py @@ -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 ```` 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`. diff --git a/main.py b/main.py index c33e75f62..9c318fafe 100644 --- a/main.py +++ b/main.py @@ -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 diff --git a/requirements.txt b/requirements.txt index 3a8203aff..ef30cd7fa 100644 --- a/requirements.txt +++ b/requirements.txt @@ -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 diff --git a/server.py b/server.py index e28fe2d22..c9ffcaa0d 100644 --- a/server.py +++ b/server.py @@ -624,8 +624,9 @@ class PromptServer(): # For security, force renderable/active types (HTML, JS, # CSS, SVG, XML — anything that can carry inline ' +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 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() diff --git a/tests/execution/test_jobs.py b/tests/execution/test_jobs.py index f7cb612e4..cef2b41cb 100644 --- a/tests/execution/test_jobs.py +++ b/tests/execution/test_jobs.py @@ -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()"""