Skip to content

Commit 5611d1c

Browse files
committed
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.
1 parent e4eb7f2 commit 5611d1c

2 files changed

Lines changed: 19 additions & 8 deletions

File tree

tests-unit/security_test/test_ghsa_779p_03_annotated_traversal.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,14 @@ def test_is_within_directory_symlink_escape(sandbox):
8787
f.write("top secret")
8888

8989
# Plant a symlink inside base that points at the outside directory.
90+
# symlink creation can require elevated privileges / Developer Mode on
91+
# Windows, so skip cleanly where it isn't available (same guard as the
92+
# sibling test in test_ghsa_779p_02_preview_traversal.py).
9093
link = os.path.join(base, "escape_link")
91-
os.symlink(outside, link)
94+
try:
95+
os.symlink(outside, link)
96+
except (OSError, NotImplementedError):
97+
pytest.skip("symlinks not supported on this platform/filesystem")
9298

9399
# Accessing the secret "through" the in-base symlink must be rejected.
94100
target_via_link = os.path.join(link, "secret.txt")

tests-unit/security_test/test_ghsa_779p_05_dangerous_content_types.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,18 @@
55
image/svg+xml, so an uploaded SVG carrying an inline <script> was served as
66
image/svg+xml and executed in the page origin when rendered.
77
8-
The /view forced-download decision lives in a closure inside
9-
server.PromptServer.add_routes (server.py ~line 624), which checks
10-
`content_type in folder_paths.DANGEROUS_CONTENT_TYPES` and, on a match, rewrites
11-
the response to application/octet-stream with a Content-Disposition: attachment
12-
header. server.py cannot be imported in a unit test (importing it spins up the
13-
full PromptServer/aiohttp app and its global side effects), so these tests pin
14-
the *data* that drives the closure: folder_paths.DANGEROUS_CONTENT_TYPES.
8+
The /view forced-download decision lives in the view_image closure registered by
9+
server.PromptServer.add_routes (server.py ~line 596), which calls
10+
`folder_paths.is_dangerous_content_type(content_type)` — a normalising check that
11+
strips charset/boundary parameters and casing and folds in the whole */xml and
12+
*+xml dialect family — rather than a bypassable raw
13+
`content_type in folder_paths.DANGEROUS_CONTENT_TYPES` membership test. On a match
14+
it rewrites the response to application/octet-stream with a
15+
Content-Disposition: attachment header. server.py cannot be imported in a unit
16+
test (importing it spins up the full PromptServer/aiohttp app and its global side
17+
effects), so these tests pin the underlying dangerous-content data
18+
(folder_paths.DANGEROUS_CONTENT_TYPES) and the normalising is_dangerous_content_type()
19+
helper that the closure actually calls.
1520
1621
The end-to-end /view assertion (upload an SVG, GET /view, confirm the response
1722
is not served as image/svg+xml) lives in the live POC at

0 commit comments

Comments
 (0)