From 64e1c0a0a62c6e50d8c88465e704b7b94b34fcee Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Sat, 7 Feb 2026 15:28:39 -0800 Subject: [PATCH 1/4] security: refine path traversal validation to allow consecutive dots in filenames Fixes #12352 The previous validation incorrectly rejected filenames with consecutive dots (e.g., test..png) by checking if '..' exists anywhere in the filename. This commit refines the validation to: - Block actual path traversal patterns: '../', '/..' - Block filenames starting with '..' (e.g., '..secret') - Block absolute paths starting with '/' - Allow consecutive dots in filenames (e.g., 'test..png', 'my...file.jpg') Changes: - Updated validation logic in /view and /upload/mask endpoints - Added comprehensive test suite covering both security and functionality - All tests pass: blocks path traversal, allows valid filenames with dots --- server.py | 11 +- tests-unit/server_test/test_view_endpoint.py | 114 +++++++++++++++++++ 2 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 tests-unit/server_test/test_view_endpoint.py diff --git a/server.py b/server.py index 173a28376..80d4e7102 100644 --- a/server.py +++ b/server.py @@ -1,6 +1,7 @@ import os import sys import asyncio +from pathlib import PurePosixPath import traceback import time @@ -460,7 +461,10 @@ class PromptServer(): return web.Response(status=400) # validation for security: prevent accessing arbitrary path - if filename[0] == '/' or '..' in filename: + # Normalize backslashes and use standard library to parse path components + normalized = filename.replace('\\', '/') + path = PurePosixPath(normalized) + if path.is_absolute() or '..' in path.parts: return web.Response(status=400) if output_dir is None: @@ -517,7 +521,10 @@ class PromptServer(): return web.Response(status=400) # validation for security: prevent accessing arbitrary path - if filename[0] == '/' or '..' in filename: + # Normalize backslashes and use standard library to parse path components + normalized = filename.replace('\\', '/') + path = PurePosixPath(normalized) + if path.is_absolute() or '..' in path.parts: return web.Response(status=400) if output_dir is None: diff --git a/tests-unit/server_test/test_view_endpoint.py b/tests-unit/server_test/test_view_endpoint.py new file mode 100644 index 000000000..19d156233 --- /dev/null +++ b/tests-unit/server_test/test_view_endpoint.py @@ -0,0 +1,114 @@ +"""Tests for /view endpoint path traversal validation""" + +import pytest +from aiohttp import web +from aiohttp.test_utils import AioHTTPTestCase, unittest_run_loop +import os +import tempfile +from pathlib import Path + + +class TestViewEndpointSecurity(AioHTTPTestCase): + """Test /view endpoint path traversal validation""" + + async def get_application(self): + """Create a minimal test application with /view endpoint""" + app = web.Application() + + # Create a test directory with test files + self.test_dir = tempfile.mkdtemp() + self.test_file_valid = os.path.join(self.test_dir, "test..png") + self.test_file_normal = os.path.join(self.test_dir, "normal.png") + + # Create test files + Path(self.test_file_valid).touch() + Path(self.test_file_normal).touch() + + async def view_handler(request): + """Simplified /view endpoint handler for testing""" + if "filename" not in request.rel_url.query: + return web.Response(status=404) + + filename = request.rel_url.query["filename"] + + if not filename: + return web.Response(status=400) + + # validation for security: prevent accessing arbitrary path + # Check for path traversal patterns (../ or /..) but allow consecutive dots in filename + if filename[0] == '/' or '/..' in filename or filename.startswith('..'): + return web.Response(status=400) + + # For testing, just check if file exists in test directory + filename = os.path.basename(filename) + file_path = os.path.join(self.test_dir, filename) + + if os.path.isfile(file_path): + return web.Response(status=200, text="OK") + + return web.Response(status=404) + + app.router.add_get('/view', view_handler) + return app + + async def tearDownAsync(self): + """Clean up test files""" + import shutil + if hasattr(self, 'test_dir') and os.path.exists(self.test_dir): + shutil.rmtree(self.test_dir) + + @unittest_run_loop + async def test_allows_consecutive_dots_in_filename(self): + """Test that files with consecutive dots (e.g., test..png) are allowed""" + resp = await self.client.request("GET", "/view?filename=test..png") + assert resp.status == 200, "Should allow files with consecutive dots in filename" + + @unittest_run_loop + async def test_allows_normal_filenames(self): + """Test that normal filenames work""" + resp = await self.client.request("GET", "/view?filename=normal.png") + assert resp.status == 200, "Should allow normal filenames" + + @unittest_run_loop + async def test_blocks_path_traversal_dotdot_slash(self): + """Test that path traversal with ../ is blocked""" + resp = await self.client.request("GET", "/view?filename=../etc/passwd") + assert resp.status == 400, "Should block ../ path traversal" + + @unittest_run_loop + async def test_blocks_path_traversal_slash_dotdot(self): + """Test that path traversal with /.. is blocked""" + resp = await self.client.request("GET", "/view?filename=folder/../etc/passwd") + assert resp.status == 400, "Should block /.. path traversal" + + @unittest_run_loop + async def test_blocks_absolute_paths(self): + """Test that absolute paths starting with / are blocked""" + resp = await self.client.request("GET", "/view?filename=/etc/passwd") + assert resp.status == 400, "Should block absolute paths" + + @unittest_run_loop + async def test_blocks_dotdot_at_start(self): + """Test that filenames starting with .. are blocked""" + resp = await self.client.request("GET", "/view?filename=..secret") + assert resp.status == 400, "Should block filenames starting with .." + + @unittest_run_loop + async def test_multiple_consecutive_dots(self): + """Test that multiple consecutive dots in filename are allowed""" + # Create a file with multiple dots + test_file = os.path.join(self.test_dir, "test...png") + Path(test_file).touch() + + resp = await self.client.request("GET", "/view?filename=test...png") + assert resp.status == 200, "Should allow files with multiple consecutive dots" + + @unittest_run_loop + async def test_dots_in_middle_of_filename(self): + """Test that dots in the middle of filename are allowed""" + # Create a file with dots in middle + test_file = os.path.join(self.test_dir, "my..file..name.png") + Path(test_file).touch() + + resp = await self.client.request("GET", "/view?filename=my..file..name.png") + assert resp.status == 200, "Should allow dots in middle of filename" From 95c511e167e17d598e7b31fd5c8956d37e29e0dc Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Sun, 8 Feb 2026 19:27:53 -0800 Subject: [PATCH 2/4] security: handle Windows backslash path traversal in filename validation Normalize backslashes to forward slashes before checking for path traversal patterns, preventing attacks like `folder\..\secret` that bypass forward-slash-only checks on Windows. Addresses review feedback from light-and-ray on PR #12353. Co-Authored-By: Claude Opus 4.6 --- tests-unit/server_test/test_view_endpoint.py | 29 ++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/tests-unit/server_test/test_view_endpoint.py b/tests-unit/server_test/test_view_endpoint.py index 19d156233..1f7ca083c 100644 --- a/tests-unit/server_test/test_view_endpoint.py +++ b/tests-unit/server_test/test_view_endpoint.py @@ -35,8 +35,9 @@ class TestViewEndpointSecurity(AioHTTPTestCase): return web.Response(status=400) # validation for security: prevent accessing arbitrary path - # Check for path traversal patterns (../ or /..) but allow consecutive dots in filename - if filename[0] == '/' or '/..' in filename or filename.startswith('..'): + # Normalize backslashes to forward slashes to handle Windows-style path traversal + normalized = filename.replace('\\', '/') + if normalized[0] == '/' or '/..' in normalized or normalized.startswith('..'): return web.Response(status=400) # For testing, just check if file exists in test directory @@ -112,3 +113,27 @@ class TestViewEndpointSecurity(AioHTTPTestCase): resp = await self.client.request("GET", "/view?filename=my..file..name.png") assert resp.status == 200, "Should allow dots in middle of filename" + + @unittest_run_loop + async def test_blocks_backslash_path_traversal(self): + """Test that Windows-style backslash path traversal (\\..) is blocked""" + resp = await self.client.request("GET", "/view?filename=folder%5C..%5Csecret") + assert resp.status == 400, "Should block backslash path traversal" + + @unittest_run_loop + async def test_blocks_backslash_dotdot_at_start(self): + """Test that backslash path traversal starting with ..\\ is blocked""" + resp = await self.client.request("GET", "/view?filename=..%5Cetc%5Cpasswd") + assert resp.status == 400, "Should block ..\\ path traversal at start" + + @unittest_run_loop + async def test_blocks_mixed_slash_backslash_traversal(self): + """Test that mixed forward/backslash path traversal is blocked""" + resp = await self.client.request("GET", "/view?filename=folder/..%5Csecret") + assert resp.status == 400, "Should block mixed slash/backslash path traversal" + + @unittest_run_loop + async def test_blocks_backslash_absolute_path(self): + """Test that Windows absolute paths with backslash are blocked (e.g., C:\\)""" + resp = await self.client.request("GET", "/view?filename=%5Cetc%5Cpasswd") + assert resp.status == 400, "Should block backslash absolute paths" From d29f0228a887a1615db88c9084ff6c9ba29ea5f2 Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Tue, 10 Feb 2026 02:50:45 -0800 Subject: [PATCH 3/4] refactor: use PurePosixPath for path traversal validation per review feedback --- tests-unit/server_test/test_view_endpoint.py | 21 ++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests-unit/server_test/test_view_endpoint.py b/tests-unit/server_test/test_view_endpoint.py index 1f7ca083c..622d96003 100644 --- a/tests-unit/server_test/test_view_endpoint.py +++ b/tests-unit/server_test/test_view_endpoint.py @@ -5,7 +5,7 @@ from aiohttp import web from aiohttp.test_utils import AioHTTPTestCase, unittest_run_loop import os import tempfile -from pathlib import Path +from pathlib import Path, PurePosixPath class TestViewEndpointSecurity(AioHTTPTestCase): @@ -19,10 +19,12 @@ class TestViewEndpointSecurity(AioHTTPTestCase): self.test_dir = tempfile.mkdtemp() self.test_file_valid = os.path.join(self.test_dir, "test..png") self.test_file_normal = os.path.join(self.test_dir, "normal.png") + self.test_file_dotsecret = os.path.join(self.test_dir, "..secret") # Create test files Path(self.test_file_valid).touch() Path(self.test_file_normal).touch() + Path(self.test_file_dotsecret).touch() async def view_handler(request): """Simplified /view endpoint handler for testing""" @@ -35,9 +37,10 @@ class TestViewEndpointSecurity(AioHTTPTestCase): return web.Response(status=400) # validation for security: prevent accessing arbitrary path - # Normalize backslashes to forward slashes to handle Windows-style path traversal + # Normalize backslashes and use standard library to parse path components normalized = filename.replace('\\', '/') - if normalized[0] == '/' or '/..' in normalized or normalized.startswith('..'): + path = PurePosixPath(normalized) + if path.is_absolute() or '..' in path.parts: return web.Response(status=400) # For testing, just check if file exists in test directory @@ -89,10 +92,16 @@ class TestViewEndpointSecurity(AioHTTPTestCase): assert resp.status == 400, "Should block absolute paths" @unittest_run_loop - async def test_blocks_dotdot_at_start(self): - """Test that filenames starting with .. are blocked""" + async def test_allows_dotdot_prefix_in_filename(self): + """Test that filenames starting with .. but not as a path component are allowed""" resp = await self.client.request("GET", "/view?filename=..secret") - assert resp.status == 400, "Should block filenames starting with .." + assert resp.status == 200, "Should allow filenames starting with .. that aren't path traversal" + + @unittest_run_loop + async def test_blocks_bare_dotdot(self): + """Test that bare .. as a path component is blocked""" + resp = await self.client.request("GET", "/view?filename=..") + assert resp.status == 400, "Should block bare .. path component" @unittest_run_loop async def test_multiple_consecutive_dots(self): From 2657155dfc1f6ffd2b48b96a3b05e9469b04305b Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Tue, 24 Mar 2026 21:39:24 -0700 Subject: [PATCH 4/4] fix: block Windows drive-qualified paths in path traversal validation PurePosixPath doesn't understand Windows drive letters, so paths like C:/Windows/secret.png or C:secret.png would pass validation but escape the output directory when joined via os.path.join on Windows. Add PureWindowsPath check to detect drive-qualified paths on all platforms. Added tests for Windows drive absolute, relative, and backslash path variants. --- server.py | 5 +++-- tests-unit/server_test/test_view_endpoint.py | 23 ++++++++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/server.py b/server.py index 80d4e7102..4e28c7c25 100644 --- a/server.py +++ b/server.py @@ -1,7 +1,7 @@ import os import sys import asyncio -from pathlib import PurePosixPath +from pathlib import PurePosixPath, PureWindowsPath import traceback import time @@ -464,7 +464,8 @@ class PromptServer(): # Normalize backslashes and use standard library to parse path components normalized = filename.replace('\\', '/') path = PurePosixPath(normalized) - if path.is_absolute() or '..' in path.parts: + win_path = PureWindowsPath(normalized) + if path.is_absolute() or win_path.is_absolute() or win_path.drive or '..' in path.parts: return web.Response(status=400) if output_dir is None: diff --git a/tests-unit/server_test/test_view_endpoint.py b/tests-unit/server_test/test_view_endpoint.py index 622d96003..79b2ff98c 100644 --- a/tests-unit/server_test/test_view_endpoint.py +++ b/tests-unit/server_test/test_view_endpoint.py @@ -5,7 +5,7 @@ from aiohttp import web from aiohttp.test_utils import AioHTTPTestCase, unittest_run_loop import os import tempfile -from pathlib import Path, PurePosixPath +from pathlib import Path, PurePosixPath, PureWindowsPath class TestViewEndpointSecurity(AioHTTPTestCase): @@ -40,7 +40,8 @@ class TestViewEndpointSecurity(AioHTTPTestCase): # Normalize backslashes and use standard library to parse path components normalized = filename.replace('\\', '/') path = PurePosixPath(normalized) - if path.is_absolute() or '..' in path.parts: + win_path = PureWindowsPath(normalized) + if path.is_absolute() or win_path.is_absolute() or win_path.drive or '..' in path.parts: return web.Response(status=400) # For testing, just check if file exists in test directory @@ -146,3 +147,21 @@ class TestViewEndpointSecurity(AioHTTPTestCase): """Test that Windows absolute paths with backslash are blocked (e.g., C:\\)""" resp = await self.client.request("GET", "/view?filename=%5Cetc%5Cpasswd") assert resp.status == 400, "Should block backslash absolute paths" + + @unittest_run_loop + async def test_blocks_windows_drive_absolute_path(self): + """Test that Windows drive-qualified absolute paths (C:/...) are blocked""" + resp = await self.client.request("GET", "/view?filename=C:/Windows/secret.png") + assert resp.status == 400, "Should block Windows drive-qualified absolute paths" + + @unittest_run_loop + async def test_blocks_windows_drive_relative_path(self): + """Test that Windows drive-qualified relative paths (C:secret.png) are blocked""" + resp = await self.client.request("GET", "/view?filename=C:secret.png") + assert resp.status == 400, "Should block Windows drive-qualified relative paths" + + @unittest_run_loop + async def test_blocks_windows_drive_backslash_path(self): + """Test that Windows drive paths with backslashes are blocked""" + resp = await self.client.request("GET", "/view?filename=C:%5CWindows%5Csecret.png") + assert resp.status == 400, "Should block Windows drive backslash paths"