mirror of
https://github.com/comfyanonymous/ComfyUI.git
synced 2026-07-04 05:31:03 +08:00
Some checks are pending
Detect Unreviewed Merge / detect (push) Waiting to run
Python Linting / Run Ruff (push) Waiting to run
Python Linting / Run Pylint (push) Waiting to run
Full Comfy CI Workflow Runs / test-stable (12.1, , linux, 3.10, [self-hosted Linux], stable) (push) Waiting to run
Full Comfy CI Workflow Runs / test-stable (12.1, , linux, 3.11, [self-hosted Linux], stable) (push) Waiting to run
Full Comfy CI Workflow Runs / test-stable (12.1, , linux, 3.12, [self-hosted Linux], stable) (push) Waiting to run
Full Comfy CI Workflow Runs / test-unix-nightly (12.1, , linux, 3.11, [self-hosted Linux], nightly) (push) Waiting to run
Execution Tests / test (macos-latest) (push) Waiting to run
Execution Tests / test (ubuntu-latest) (push) Waiting to run
Execution Tests / test (windows-latest) (push) Waiting to run
Test server launches without errors / test (push) Waiting to run
Unit Tests / test (macos-latest) (push) Waiting to run
Unit Tests / test (ubuntu-latest) (push) Waiting to run
Unit Tests / test (windows-2022) (push) Waiting to run
* security: fix five vulnerabilities (GHSA-779p-m5rp-r4h4)
- CVE-2026-56670: force download of SVG/XML responses on /view to prevent stored XSS
- CVE-2026-56671: contain /experiment/models/preview reads within the model folder
- CVE-2026-56672: stop inline rendering of uploaded /userdata/{file} content
- CVE-2026-56673: prevent path traversal in get_annotated_filepath (LoadImage /prompt input)
- CVE-2026-56674: reject opaque/null Origin to close the CSRF middleware bypass
Adds regression tests under tests-unit/security_test/ covering all five.
* security: address review feedback on GHSA-779p fixes
- Fix Windows CI failure in test_get_annotated_filepath: compare against
os.path.abspath(...) to match the intentional abspath normalization added
by the traversal hardening (abspath prepends the drive letter on Windows).
- origin_check: narrow the bare `except:` in is_loopback() to ValueError so
genuine interrupts aren't swallowed (review nit).
- origin_check: guard .port access in is_cross_origin_forbidden() so a
malformed/out-of-range port (e.g. Origin: http://127.0.0.1:99999) fails
closed with a 403 instead of surfacing an uncaught 500 in the middleware.
- server /view: escape backslash/quote in the Content-Disposition filename
(RFC 6266 quoted-string) so a filename containing a double quote can't
malform the response header.
* security: address CodeRabbit review feedback on GHSA-779p tests
- test #3: guard the symlink-escape test with a try/except skip so it no
longer errors on Windows CI where os.symlink needs elevated privileges /
Developer Mode (mirrors the guard in the sibling test #2).
- test #5: refresh the stale module docstring to describe the actual /view
gating (view_image closure calling folder_paths.is_dangerous_content_type,
the normalising check) instead of the bypassable raw set-membership test.
* revert(security): drop CVE-2026-56674 Origin: null CSRF change
Per maintainer review, the reported CSRF is already mitigated by the pre-existing
Sec-Fetch-Site: cross-site check for current browsers, and the null-origin
rejection risked breaking legitimate sandboxed-iframe embeds. Restores
origin_only_middleware and is_loopback in server.py to their prior state
(the Sec-Fetch-Site check is retained) and removes utils/origin_check.py and its
regression test. The other four GHSA-779p fixes are unaffected.
148 lines
5.3 KiB
Python
148 lines
5.3 KiB
Python
"""
|
|
CI unit tests for FIX #4 of GHSA-779p-m5rp-r4h4.
|
|
|
|
Stored-XSS hardening on GET /userdata/{file} in app/user_manager.py.
|
|
|
|
User data files are arbitrary user-supplied content and must never render
|
|
inline in the app origin. The getuserdata handler:
|
|
- forces Content-Type to application/octet-stream for any type in
|
|
folder_paths.DANGEROUS_CONTENT_TYPES (text/html, image/svg+xml,
|
|
text/javascript, ...),
|
|
- sets X-Content-Type-Options: nosniff,
|
|
- sets Content-Disposition: attachment.
|
|
|
|
These tests pre-create files in tmp_path and GET them back, asserting the
|
|
secure response headers. They mirror the aiohttp_client pattern in
|
|
tests-unit/prompt_server_test/user_manager_test.py.
|
|
"""
|
|
|
|
import pytest
|
|
import os
|
|
from aiohttp import web
|
|
from app.user_manager import UserManager
|
|
|
|
pytestmark = (
|
|
pytest.mark.asyncio
|
|
) # This applies the asyncio mark to all test functions in the module
|
|
|
|
|
|
@pytest.fixture
|
|
def user_manager(tmp_path):
|
|
um = UserManager()
|
|
um.get_request_user_filepath = lambda req, file, **kwargs: os.path.join(
|
|
tmp_path, file
|
|
) if file else tmp_path
|
|
return um
|
|
|
|
|
|
@pytest.fixture
|
|
def app(user_manager):
|
|
app = web.Application()
|
|
routes = web.RouteTableDef()
|
|
user_manager.add_routes(routes)
|
|
app.add_routes(routes)
|
|
return app
|
|
|
|
|
|
async def test_html_served_as_octet_stream(aiohttp_client, app, tmp_path):
|
|
(tmp_path / "evil.html").write_text(
|
|
"<script>console.log('xss-marker-ghsa-779p')</script>"
|
|
)
|
|
|
|
client = await aiohttp_client(app)
|
|
resp = await client.get("/userdata/evil.html")
|
|
|
|
assert resp.status == 200
|
|
ct = resp.headers.get("Content-Type", "")
|
|
# The load-bearing assertion: a .html file must NOT be served as text/html.
|
|
assert "text/html" not in ct.lower(), (
|
|
f"Content-Type {ct!r} would let a browser render/execute the file (stored XSS)."
|
|
)
|
|
assert ct == "application/octet-stream"
|
|
assert resp.headers.get("X-Content-Type-Options") == "nosniff"
|
|
assert "attachment" in resp.headers.get("Content-Disposition", "")
|
|
|
|
|
|
async def test_svg_served_as_octet_stream(aiohttp_client, app, tmp_path):
|
|
(tmp_path / "evil.svg").write_text(
|
|
'<?xml version="1.0"?>'
|
|
'<svg xmlns="http://www.w3.org/2000/svg">'
|
|
'<script>console.log("xss-marker-ghsa-779p")</script>'
|
|
"</svg>"
|
|
)
|
|
|
|
client = await aiohttp_client(app)
|
|
resp = await client.get("/userdata/evil.svg")
|
|
|
|
assert resp.status == 200
|
|
ct = resp.headers.get("Content-Type", "")
|
|
# SVG can carry inline <script>; it must not be served as image/svg+xml.
|
|
assert "svg" not in ct.lower(), (
|
|
f"Content-Type {ct!r} would let a browser render the SVG and execute embedded scripts."
|
|
)
|
|
assert ct == "application/octet-stream"
|
|
assert resp.headers.get("X-Content-Type-Options") == "nosniff"
|
|
assert "attachment" in resp.headers.get("Content-Disposition", "")
|
|
|
|
|
|
async def test_js_served_as_octet_stream(aiohttp_client, app, tmp_path):
|
|
(tmp_path / "evil.js").write_text("alert('xss-marker-ghsa-779p')")
|
|
|
|
client = await aiohttp_client(app)
|
|
resp = await client.get("/userdata/evil.js")
|
|
|
|
assert resp.status == 200
|
|
ct = resp.headers.get("Content-Type", "").lower()
|
|
# Must not be served as any executable JavaScript content type.
|
|
assert "javascript" not in ct, (
|
|
f"Content-Type {ct!r} is an executable JS type."
|
|
)
|
|
assert "ecmascript" not in ct, (
|
|
f"Content-Type {ct!r} is an executable JS type."
|
|
)
|
|
assert ct == "application/octet-stream"
|
|
assert resp.headers.get("X-Content-Type-Options") == "nosniff"
|
|
assert "attachment" in resp.headers.get("Content-Disposition", "")
|
|
|
|
|
|
async def test_xml_dialect_served_as_octet_stream(aiohttp_client, app, tmp_path):
|
|
"""An XML dialect outside the original blocklist (.xslt -> application/xslt+xml)
|
|
must still be forced to download. This pins the normalised *+xml family rule
|
|
in folder_paths.is_dangerous_content_type(); a plain set-membership test would
|
|
have served this inline."""
|
|
(tmp_path / "evil.xslt").write_text(
|
|
'<?xml version="1.0"?>'
|
|
'<xsl:stylesheet version="1.0" '
|
|
'xmlns:xsl="http://www.w3.org/1999/XSL/Transform">'
|
|
"<!-- xss-marker-ghsa-779p -->"
|
|
"</xsl:stylesheet>"
|
|
)
|
|
|
|
client = await aiohttp_client(app)
|
|
resp = await client.get("/userdata/evil.xslt")
|
|
|
|
assert resp.status == 200
|
|
ct = resp.headers.get("Content-Type", "")
|
|
assert ct == "application/octet-stream", (
|
|
f"Content-Type {ct!r}: an *+xml dialect must be forced to octet-stream "
|
|
f"(it can carry inline script via stylesheet/entity tricks)."
|
|
)
|
|
assert resp.headers.get("X-Content-Type-Options") == "nosniff"
|
|
assert "attachment" in resp.headers.get("Content-Disposition", "")
|
|
|
|
|
|
async def test_benign_txt_still_served(aiohttp_client, app, tmp_path):
|
|
(tmp_path / "note.txt").write_text("just a harmless note")
|
|
|
|
client = await aiohttp_client(app)
|
|
resp = await client.get("/userdata/note.txt")
|
|
|
|
assert resp.status == 200
|
|
assert await resp.text() == "just a harmless note"
|
|
ct = resp.headers.get("Content-Type", "")
|
|
# text/plain is not in the dangerous set, so it is acceptable here. The
|
|
# defence-in-depth headers must still be present regardless.
|
|
assert "text/plain" in ct.lower()
|
|
assert resp.headers.get("X-Content-Type-Options") == "nosniff"
|
|
assert "attachment" in resp.headers.get("Content-Disposition", "")
|