fix(system): no existence oracle in source validation
A gallery root's containment in the allowed ComfyUI folders is now decided before its existence and type, so a path outside them answers "outside-allowed" whether or not it exists. validate-source also folds the filesystem-root and home-directory codes into "outside-allowed", with one reason, and logs the detail server-side only. Paths inside the allowed folders still say "not-found" or "not-directory". The legacy settings save shares the same check.
This commit is contained in:
@@ -17,10 +17,13 @@ process has no monitor; else ``restart_pending`` (a source added since the
|
||||
monitor started, monitoring switched on, or an observer that stopped).
|
||||
"""
|
||||
|
||||
import logging
|
||||
import ntpath
|
||||
import os
|
||||
import re
|
||||
|
||||
logger = logging.getLogger("prompt_manager.api")
|
||||
|
||||
MAX_SOURCE_PATH = 4096 # characters
|
||||
FORM_REASONS = {
|
||||
"empty": "Gallery root must be a non-empty path",
|
||||
@@ -38,6 +41,12 @@ _COMFYUI_LABELS = (
|
||||
("get_temp_directory", "ComfyUI temp"),
|
||||
("get_user_directory", "ComfyUI user"),
|
||||
)
|
||||
# Codes validate-source folds into "outside-allowed" (see validate_source).
|
||||
_OUTSIDE_CODES = frozenset({"filesystem-root", "home-directory", "outside-allowed"})
|
||||
OUTSIDE_REASON = (
|
||||
"Gallery root must be inside a ComfyUI directory (output, input, temp, user) "
|
||||
"or one listed in PROMPT_MANAGER_EXTRA_GALLERY_ROOTS"
|
||||
)
|
||||
WATCH_STATES = ("watching", "manual", "restart_pending")
|
||||
|
||||
|
||||
@@ -217,6 +226,12 @@ def validate_source(text):
|
||||
reason = FORM_REASONS.get(code)
|
||||
if code is None:
|
||||
code, reason = GalleryConfig.gallery_root_problem(text)
|
||||
if code in _OUTSIDE_CODES:
|
||||
# One answer for every path outside the allowed folders: naming
|
||||
# the filesystem root or the home directory would let a caller
|
||||
# probe the server's layout. The detail goes to the log only.
|
||||
logger.info(f"Source refused: {code}")
|
||||
code, reason = "outside-allowed", OUTSIDE_REASON
|
||||
if code is not None:
|
||||
return {
|
||||
"valid": False,
|
||||
|
||||
+17
-13
@@ -412,8 +412,10 @@ class GalleryConfig:
|
||||
"""``(code, reason)`` why ``path`` may not be a gallery root, else (None, "").
|
||||
|
||||
Codes: ``empty``, ``unresolvable``, ``filesystem-root``,
|
||||
``home-directory``, ``not-found``, ``not-directory`` and
|
||||
``outside-allowed``. No reason names the path.
|
||||
``home-directory``, ``outside-allowed``, ``not-found`` and
|
||||
``not-directory``. No reason names the path. Containment is decided
|
||||
before existence, so a path outside the allowed folders answers
|
||||
``outside-allowed`` whether or not it exists (no existence oracle).
|
||||
"""
|
||||
if not isinstance(path, str) or not path.strip():
|
||||
return "empty", "Gallery root must be a non-empty path"
|
||||
@@ -427,21 +429,23 @@ class GalleryConfig:
|
||||
return "filesystem-root", "Gallery root cannot be a filesystem root"
|
||||
if canonical == _canonical_path(os.path.expanduser("~")):
|
||||
return "home-directory", "Gallery root cannot be the home directory"
|
||||
|
||||
candidate = Path(canonical)
|
||||
if not any(
|
||||
candidate.is_relative_to(Path(parent))
|
||||
for parent in cls.allowed_gallery_parents()
|
||||
):
|
||||
return (
|
||||
"outside-allowed",
|
||||
"Gallery root must be inside a ComfyUI directory "
|
||||
f"(output, input, temp, user) or one listed in "
|
||||
f"{EXTRA_GALLERY_ROOTS_ENV}",
|
||||
)
|
||||
if not os.path.exists(canonical):
|
||||
return "not-found", "Gallery root does not exist"
|
||||
if not os.path.isdir(canonical):
|
||||
return "not-directory", "Gallery root is not a directory"
|
||||
|
||||
candidate = Path(canonical)
|
||||
for parent in cls.allowed_gallery_parents():
|
||||
if candidate.is_relative_to(Path(parent)):
|
||||
return None, ""
|
||||
|
||||
return (
|
||||
"outside-allowed",
|
||||
"Gallery root must be inside a ComfyUI directory "
|
||||
f"(output, input, temp, user) or one listed in {EXTRA_GALLERY_ROOTS_ENV}",
|
||||
)
|
||||
return None, ""
|
||||
|
||||
|
||||
class IntegrationConfig:
|
||||
|
||||
@@ -416,8 +416,8 @@ class TestValidateSource(SourcesTestCase):
|
||||
str(self.output_dir / "missing"): "not-found",
|
||||
str(a_file): "not-directory",
|
||||
str(self.outside_dir): "outside-allowed",
|
||||
os.path.abspath(os.sep): "filesystem-root",
|
||||
os.path.expanduser("~"): "home-directory",
|
||||
os.path.abspath(os.sep): "outside-allowed",
|
||||
os.path.expanduser("~"): "outside-allowed",
|
||||
os.path.join(str(self.output_dir), "b", "..", "a"): "traversal",
|
||||
"output/../output/a": "traversal",
|
||||
str(self.a) + "\x00": "invalid-form",
|
||||
@@ -437,6 +437,47 @@ class TestValidateSource(SourcesTestCase):
|
||||
self.assertIsNone(data["path"])
|
||||
self.assertNotIn(os.path.expanduser("~"), json.dumps(data))
|
||||
|
||||
async def test_outside_paths_answer_alike_whether_they_exist(self):
|
||||
"""No existence oracle: outside the allowed folders every path looks
|
||||
the same, existing or not, file or folder."""
|
||||
a_file = self.outside_dir / "file.txt"
|
||||
a_file.write_bytes(b"x")
|
||||
answers = []
|
||||
for path in (
|
||||
self.outside_dir,
|
||||
self.outside_dir / "missing",
|
||||
a_file,
|
||||
os.path.join(self.tmpdir, "no", "such", "tree"),
|
||||
os.path.abspath(os.sep),
|
||||
os.path.expanduser("~"),
|
||||
):
|
||||
status, data = await self._post(VALIDATE, {"path": str(path)})
|
||||
answers.append((status, data))
|
||||
self.assertEqual(answers[0][1]["code"], "outside-allowed")
|
||||
self.assertEqual([answer for answer in answers], [answers[0]] * len(answers))
|
||||
|
||||
async def test_settings_save_refuses_outside_paths_alike(self):
|
||||
(self.outside_dir / "x").mkdir()
|
||||
answers = []
|
||||
for path in (self.outside_dir / "x", self.tmpdir + "/nothing/x"):
|
||||
status, data = await self._post(
|
||||
"/prompt_manager/settings", {"gallery_root_paths": [str(path)]}
|
||||
)
|
||||
answers.append((status, data))
|
||||
self.assertEqual(answers[0][0], 400)
|
||||
self.assertEqual(answers[0], answers[1])
|
||||
|
||||
async def test_inside_paths_say_why_they_are_not_usable(self):
|
||||
a_file = self.output_dir / "file.png"
|
||||
a_file.write_bytes(b"x")
|
||||
status, missing = await self._post(
|
||||
VALIDATE, {"path": str(self.output_dir / "missing")}
|
||||
)
|
||||
status, not_dir = await self._post(VALIDATE, {"path": str(a_file)})
|
||||
self.assertEqual(
|
||||
(missing["code"], not_dir["code"]), ("not-found", "not-directory")
|
||||
)
|
||||
|
||||
async def test_malformed_request_is_400(self):
|
||||
for body in ({}, {"path": 42}, {"path": None}, ["x"]):
|
||||
with self.subTest(body=body):
|
||||
|
||||
Reference in New Issue
Block a user