fix(security): decide source containment as text before any filesystem access
Every caller (validate-source, POST /settings, config.json at load) now refuses a source through one check, sources.source_problem: the form check first, then GalleryConfig.gallery_root_problem, which compares the normalised spelling with the allowed folders before anything resolves, stats or opens the path. A UNC share, the \\?\ and \\.\ namespaces, a device or any outside folder is refused with no SMB request, no executor stall and no existence or timing oracle; only then is the path resolved and contained again, which still catches a symlink out. The filesystem root and the home directory answer outside-allowed with the same reason as any outside path, so no caller learns the home path. Also: - POST /settings runs the form checks and parses the roots off the loop; an unusable source is a coded 400 invalid-source naming its public path (as text) and its index. - ComfyUI folders outside the base directory are path anchors, so their sources round-trip and never collide by folder name. - A per-source scan sends each index with its public path; a stale or shifted index is 409 sources-changed and starts nothing. - A source still watched after monitoring was switched off is "stopping". - Roots are compared as folders, not spellings, so an unchanged legacy resend takes no source-change hold. - Tests: no realpath/stat/lstat of an outside path (native and Windows forms with ntpath fakes), the alike answers with /, ~ and a link to ~, an event instead of a timer wait, and a teardown that drains jobs before the temporary database goes.
This commit is contained in:
Vito Sansevero
+42
-13
@@ -21,8 +21,10 @@ def _not_found():
|
||||
MAX_SCAN_ROOTS = 64
|
||||
_SCOPE_ERROR = 'The scan scope must be "new" or "all"'
|
||||
_ROOTS_ERROR = (
|
||||
f"roots must be a non-empty list of up to {MAX_SCAN_ROOTS} source indexes"
|
||||
f"roots must be a non-empty list of up to {MAX_SCAN_ROOTS} source indexes, "
|
||||
"with paths naming the public path of each"
|
||||
)
|
||||
SOURCES_CHANGED_ERROR = "The sources changed since they were loaded. Reload them."
|
||||
|
||||
|
||||
def _bad_request(message, code=None):
|
||||
@@ -33,20 +35,29 @@ def _bad_request(message, code=None):
|
||||
|
||||
|
||||
def _scan_roots(body):
|
||||
"""``(roots, error)``: the sorted, deduplicated source indexes, or None.
|
||||
"""``(roots, error)``: ``{index: public path}`` of the selected sources, or None.
|
||||
|
||||
Absent ``roots`` means every source. When present it is a non-empty list
|
||||
of at most MAX_SCAN_ROOTS non-negative ints (never bools or floats).
|
||||
of at most MAX_SCAN_ROOTS non-negative ints (never bools or floats), and
|
||||
``paths`` the same-length list of the public path the caller read for
|
||||
each (GET /settings), so a stale index never scans another folder.
|
||||
"""
|
||||
if "roots" not in body:
|
||||
return None, None
|
||||
raw = body["roots"]
|
||||
paths = body.get("paths")
|
||||
if not isinstance(raw, list) or not raw or len(raw) > MAX_SCAN_ROOTS:
|
||||
return None, _ROOTS_ERROR
|
||||
for value in raw:
|
||||
if not isinstance(paths, list) or len(paths) != len(raw):
|
||||
return None, _ROOTS_ERROR
|
||||
selected = {}
|
||||
for value, path in zip(raw, paths):
|
||||
if isinstance(value, bool) or not isinstance(value, int) or value < 0:
|
||||
return None, _ROOTS_ERROR
|
||||
return sorted(set(raw)), None
|
||||
if not isinstance(path, str) or selected.get(value, path) != path:
|
||||
return None, _ROOTS_ERROR
|
||||
selected[value] = path
|
||||
return dict(sorted(selected.items())), None
|
||||
|
||||
|
||||
async def _scan_request(request):
|
||||
@@ -98,6 +109,13 @@ def _summary_event(job):
|
||||
class JobRoutesMixin:
|
||||
"""Mixin providing the /prompt_manager/jobs endpoints."""
|
||||
|
||||
def _roots_still_listed(self, selected, output_dirs):
|
||||
"""Whether each ``{index: public path}`` still names that source."""
|
||||
return all(
|
||||
index < len(output_dirs) and self._public_path(output_dirs[index]) == path
|
||||
for index, path in selected.items()
|
||||
)
|
||||
|
||||
def _register_job_routes(self, routes):
|
||||
@routes.get("/prompt_manager/jobs")
|
||||
async def list_jobs_route(request):
|
||||
@@ -167,24 +185,35 @@ class JobRoutesMixin:
|
||||
|
||||
The body's ``scope`` is "new" (the default: new and unresolved files)
|
||||
or "all". ``roots``, when given, selects sources by their index in
|
||||
the effective sources (``_get_all_output_dirs``); an index past them
|
||||
is a 400 ``unknown-source`` and starts nothing. The job walks the
|
||||
sources resolved here, so a later source change cannot move it.
|
||||
the effective sources (``_get_all_output_dirs``), and ``paths`` names
|
||||
the public path read for each. An index past them, or one whose
|
||||
source now has another public path (the list changed since it was
|
||||
read), is a 409 ``sources-changed`` and starts nothing. The job walks
|
||||
the sources resolved here, so a later source change cannot move it.
|
||||
"""
|
||||
scan, error = await _scan_request(request)
|
||||
if error:
|
||||
return _bad_request(error)
|
||||
output_dirs = None
|
||||
roots = None
|
||||
if scan["roots"] is not None:
|
||||
output_dirs = await self._run_in_executor(self._get_all_output_dirs)
|
||||
if scan["roots"][-1] >= len(output_dirs):
|
||||
return _bad_request(
|
||||
"That source is no longer configured. Reload the sources.",
|
||||
code="unknown-source",
|
||||
listed = await self._run_in_executor(
|
||||
self._roots_still_listed, scan["roots"], output_dirs
|
||||
)
|
||||
if not listed:
|
||||
return web.json_response(
|
||||
{
|
||||
"success": False,
|
||||
"code": "sources-changed",
|
||||
"error": SOURCES_CHANGED_ERROR,
|
||||
},
|
||||
status=409,
|
||||
)
|
||||
roots = list(scan["roots"])
|
||||
try:
|
||||
job = self._start_scan_job(
|
||||
scan["files"], roots=scan["roots"], output_dirs=output_dirs
|
||||
scan["files"], roots=roots, output_dirs=output_dirs
|
||||
)
|
||||
except JobBusy as busy:
|
||||
return busy_response(busy)
|
||||
|
||||
+81
-19
@@ -65,6 +65,45 @@ def _stat_identity(path):
|
||||
return (stat.st_dev, stat.st_ino) if stat.st_ino else None
|
||||
|
||||
|
||||
# Refusals that a reconnected drive or folder can lift.
|
||||
_RECONNECTABLE = frozenset({"not-found", "not-directory"})
|
||||
|
||||
|
||||
def _root_key(root):
|
||||
"""The directory a configured root names (resolved), else its spelling."""
|
||||
from ..config import GalleryConfig
|
||||
|
||||
try:
|
||||
return GalleryConfig.resolve_gallery_root(root)
|
||||
except (OSError, TypeError, ValueError, AttributeError):
|
||||
return root
|
||||
|
||||
|
||||
def _request_error(message):
|
||||
return {"code": "invalid-request", "error": message}
|
||||
|
||||
|
||||
def _invalid_source(index, entry, code, reason):
|
||||
"""The 400 body for the source at ``index`` that cannot be used.
|
||||
|
||||
Names it by its public spelling (computed as text: a refused path is
|
||||
never resolved), so the page can mark that row and say why.
|
||||
"""
|
||||
source = sources.public_spelling(entry)
|
||||
advice = (
|
||||
"Remove it or reconnect it, then save."
|
||||
if code in _RECONNECTABLE
|
||||
else "Remove it, then save."
|
||||
)
|
||||
return {
|
||||
"code": "invalid-source",
|
||||
"source": source,
|
||||
"index": index,
|
||||
"reason": reason,
|
||||
"error": f"{source} can't be used: {reason}. {advice}",
|
||||
}
|
||||
|
||||
|
||||
def _section(config, name):
|
||||
"""``config[name]`` as a fresh dict (a non-object section starts empty)."""
|
||||
value = config.get(name)
|
||||
@@ -255,9 +294,15 @@ class SettingsRoutesMixin:
|
||||
{"success": False, "error": str(ve)}, status=400
|
||||
)
|
||||
|
||||
new_roots, error = self._parse_gallery_roots(data)
|
||||
# Off the loop: an accepted root is resolved (filesystem access).
|
||||
new_roots, error = await self._run_in_executor(
|
||||
self._parse_gallery_roots, data
|
||||
)
|
||||
if error:
|
||||
return web.json_response({"success": False, "error": error}, status=400)
|
||||
return web.json_response({"success": False, **error}, status=400)
|
||||
roots_change = new_roots is not None and await self._run_in_executor(
|
||||
self._roots_change, new_roots
|
||||
)
|
||||
|
||||
published = {}
|
||||
|
||||
@@ -268,7 +313,7 @@ class SettingsRoutesMixin:
|
||||
|
||||
async with contextlib.AsyncExitStack() as stack:
|
||||
hold = None
|
||||
if self._roots_change(new_roots):
|
||||
if roots_change:
|
||||
try:
|
||||
hold = await stack.enter_async_context(
|
||||
self._jobs.hold(
|
||||
@@ -344,12 +389,19 @@ class SettingsRoutesMixin:
|
||||
|
||||
@staticmethod
|
||||
def _roots_change(new_roots):
|
||||
"""Whether a save changes the sources (the legacy page resends them)."""
|
||||
"""Whether a save changes the sources (blocking: it resolves them).
|
||||
|
||||
Compared as directories, not spellings: the legacy page resends the
|
||||
roots resolved, while config.json may hold them as hand-written
|
||||
(relative, another case), and an unchanged resend is no change.
|
||||
"""
|
||||
from ..config import GalleryConfig
|
||||
|
||||
return new_roots is not None and new_roots != list(
|
||||
GalleryConfig.MONITORING_DIRECTORIES
|
||||
)
|
||||
if new_roots is None:
|
||||
return False
|
||||
return [_root_key(root) for root in new_roots] != [
|
||||
_root_key(root) for root in GalleryConfig.MONITORING_DIRECTORIES
|
||||
]
|
||||
|
||||
@staticmethod
|
||||
def _observer_alive():
|
||||
@@ -441,15 +493,21 @@ class SettingsRoutesMixin:
|
||||
Accepts ``gallery_root_paths`` (list, preferred) or the legacy single
|
||||
``gallery_root_path`` string. Blank entries are dropped.
|
||||
|
||||
Each entry passes the same checks as validate-source
|
||||
(``sources.source_problem``): the form, then containment as text,
|
||||
before anything is resolved. Blocking (an accepted root is resolved).
|
||||
|
||||
Returns:
|
||||
(roots, error): ``roots`` is the validated list, or ``None`` when
|
||||
the payload carries no gallery root key; ``error`` is a message
|
||||
when validation failed (and ``roots`` is then ``None``).
|
||||
the payload carries no gallery root key; ``error`` is the coded
|
||||
body of the 400 when validation failed (``roots`` is then None).
|
||||
An unusable source is ``invalid-source`` naming its public path
|
||||
and its position in the list.
|
||||
"""
|
||||
if "gallery_root_paths" in data:
|
||||
raw = data["gallery_root_paths"]
|
||||
if not isinstance(raw, list):
|
||||
return None, "gallery_root_paths must be a list"
|
||||
return None, _request_error("gallery_root_paths must be a list")
|
||||
elif "gallery_root_path" in data:
|
||||
raw = [data["gallery_root_path"]]
|
||||
else:
|
||||
@@ -459,18 +517,15 @@ class SettingsRoutesMixin:
|
||||
|
||||
roots = []
|
||||
seen = set()
|
||||
for entry in raw:
|
||||
for index, entry in enumerate(raw):
|
||||
if not isinstance(entry, str):
|
||||
return None, "Gallery root paths must be strings"
|
||||
return None, _request_error("Gallery root paths must be strings")
|
||||
entry = entry.strip()
|
||||
if not entry:
|
||||
continue
|
||||
ok, reason = GalleryConfig.validate_gallery_root(entry)
|
||||
if not ok:
|
||||
return (
|
||||
None,
|
||||
f"Invalid gallery root '{os.path.basename(entry)}': {reason}",
|
||||
)
|
||||
code, reason = sources.source_problem(entry)
|
||||
if code is not None:
|
||||
return None, _invalid_source(index, entry, code, reason)
|
||||
root = GalleryConfig.resolve_gallery_root(entry)
|
||||
keys = {("path", _source_identity(root)), ("file", _stat_identity(root))}
|
||||
keys.discard(("file", None))
|
||||
@@ -481,11 +536,18 @@ class SettingsRoutesMixin:
|
||||
return roots, None
|
||||
|
||||
def _apply_gallery_roots(self, roots):
|
||||
"""Install validated gallery roots; returns True when they changed."""
|
||||
"""Install validated gallery roots; returns True when they changed.
|
||||
|
||||
Another spelling of the same directories installs the resolved
|
||||
spelling and is no change (nothing to restart for).
|
||||
"""
|
||||
from ..config import GalleryConfig
|
||||
|
||||
if roots == list(GalleryConfig.MONITORING_DIRECTORIES):
|
||||
return False
|
||||
if not self._roots_change(roots):
|
||||
GalleryConfig.MONITORING_DIRECTORIES = roots
|
||||
return False
|
||||
GalleryConfig.MONITORING_DIRECTORIES = roots
|
||||
self._cached_output_dir = None
|
||||
self.reset_gallery_roots_cache()
|
||||
|
||||
+53
-23
@@ -6,15 +6,21 @@
|
||||
``\\\\?\\`` and ``\\\\.\\`` namespaces, a drive-relative path, an alternate
|
||||
data stream, a device name or a part Windows would silently trim; on POSIX
|
||||
no Windows drive or backslash form).
|
||||
- ``source_problem`` is the one refusal every caller answers (validate-source
|
||||
and the settings save): the form check, then
|
||||
``GalleryConfig.gallery_root_problem``, which decides containment as text
|
||||
before it touches the filesystem.
|
||||
- ``describe_sources`` (blocking: it resolves paths) reports the configured
|
||||
sources with their validity, the effective sources with their listing
|
||||
index and watch state, and the image monitor's state. Every path in it is
|
||||
a public path (``_public_path``), never an absolute one.
|
||||
|
||||
A source's watch state is ``watching`` when the monitor's observer is alive
|
||||
and scheduled that directory; ``manual`` when monitoring is off or this
|
||||
process has no monitor; else ``restart_pending`` (a source added since the
|
||||
monitor started, monitoring switched on, or an observer that stopped).
|
||||
A source's watch state is ``watching`` when the monitor's observer is alive,
|
||||
scheduled that directory and monitoring is on; ``stopping`` when it still
|
||||
watches it but monitoring was switched off (it stops at the next restart);
|
||||
``manual`` when monitoring is off or this process has no monitor; else
|
||||
``restart_pending`` (a source added since the monitor started, monitoring
|
||||
switched on, or an observer that stopped).
|
||||
"""
|
||||
|
||||
import logging
|
||||
@@ -41,13 +47,7 @@ _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")
|
||||
WATCH_STATES = ("watching", "stopping", "manual", "restart_pending")
|
||||
|
||||
|
||||
def source_form_error(text, *, windows=None):
|
||||
@@ -100,6 +100,45 @@ def _windows_form_error(text):
|
||||
return None
|
||||
|
||||
|
||||
def source_problem(text):
|
||||
"""``(code, reason)`` that refuses ``text`` as a source, else ``(None, "")``.
|
||||
|
||||
The form check runs first and touches nothing; then containment is
|
||||
decided as text before any filesystem access (see
|
||||
``GalleryConfig.gallery_root_problem``). Every caller answers alike.
|
||||
"""
|
||||
from ..config import GalleryConfig
|
||||
|
||||
code = source_form_error(text)
|
||||
if code is not None:
|
||||
return code, FORM_REASONS[code]
|
||||
return GalleryConfig.gallery_root_problem(text)
|
||||
|
||||
|
||||
def public_spelling(text):
|
||||
"""How a refused source is named in a response, computed as text only.
|
||||
|
||||
Relative to the path anchor it lies under (its public path), else its
|
||||
last part: the caller sent the path, so naming it reveals nothing, and a
|
||||
refused path is never resolved to name it.
|
||||
"""
|
||||
from ..config import GalleryConfig, _lexical_path, _lexically_inside
|
||||
|
||||
text = str(text).strip()
|
||||
try:
|
||||
lexical = os.path.normpath(text)
|
||||
key = _lexical_path(text) # the same length as ``lexical``
|
||||
if not os.path.isabs(text):
|
||||
return lexical.replace("\\", "/")
|
||||
for anchor in GalleryConfig.path_anchors():
|
||||
if _lexically_inside(key, [anchor]):
|
||||
rest = lexical[len(anchor.rstrip(os.sep)) :].lstrip(os.sep)
|
||||
return rest.replace(os.sep, "/") or "."
|
||||
return os.path.basename(lexical) or lexical
|
||||
except (TypeError, ValueError):
|
||||
return ""
|
||||
|
||||
|
||||
def identity(path):
|
||||
"""The comparison key of a directory: resolved and case-folded (Windows)."""
|
||||
return os.path.normcase(os.path.realpath(str(path)))
|
||||
@@ -162,9 +201,9 @@ def monitor_state(monitor):
|
||||
|
||||
|
||||
def watch_state(path, *, present, alive, scheduled_keys, enabled):
|
||||
"""``watching``, ``manual`` or ``restart_pending`` (see the module docstring)."""
|
||||
"""One of ``WATCH_STATES`` (see the module docstring)."""
|
||||
if alive and identity(path) in scheduled_keys:
|
||||
return "watching"
|
||||
return "watching" if enabled else "stopping"
|
||||
if not present or not enabled:
|
||||
return "manual"
|
||||
return "restart_pending"
|
||||
@@ -222,16 +261,7 @@ def validate_source(text):
|
||||
from ..config import GalleryConfig
|
||||
from . import _public_path
|
||||
|
||||
code = source_form_error(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
|
||||
code, reason = source_problem(text)
|
||||
if code is not None:
|
||||
return {
|
||||
"valid": False,
|
||||
|
||||
+104
-28
@@ -31,7 +31,6 @@ routes = server_instance.routes
|
||||
extension_uri = None # Will be set in __init__.py
|
||||
|
||||
import os # noqa: E402
|
||||
from pathlib import Path # noqa: E402
|
||||
from typing import Any, Dict, List, Optional, Tuple # noqa: E402
|
||||
|
||||
# Import logging system
|
||||
@@ -51,6 +50,14 @@ config_logger = get_logger("prompt_manager.config")
|
||||
# may be used as gallery roots in addition to ComfyUI's own directories.
|
||||
EXTRA_GALLERY_ROOTS_ENV = "PROMPT_MANAGER_EXTRA_GALLERY_ROOTS"
|
||||
|
||||
# The one answer for every path outside the allowed folders. The filesystem
|
||||
# root and the home directory answer it too, so no caller learns the server's
|
||||
# layout (home path, user name) from a refusal (#249 S4).
|
||||
OUTSIDE_ALLOWED_REASON = (
|
||||
"Gallery root must be inside a ComfyUI directory (output, input, temp, user) "
|
||||
f"or one listed in {EXTRA_GALLERY_ROOTS_ENV}"
|
||||
)
|
||||
|
||||
# Environment variable overriding where config.json is read from and saved to.
|
||||
CONFIG_PATH_ENV = "PROMPT_MANAGER_CONFIG_PATH"
|
||||
|
||||
@@ -89,6 +96,29 @@ def write_private_json(path: str, data: Any) -> None:
|
||||
os.chmod(path, 0o600)
|
||||
|
||||
|
||||
def _lexical_path(path: str) -> str:
|
||||
"""``path`` normalised for comparison without touching the filesystem.
|
||||
|
||||
normpath collapses ``.`` and ``..`` as text and normcase folds case on
|
||||
Windows; neither opens, stats nor resolves anything (no SMB request for a
|
||||
UNC path). A symlink is only seen by :func:`_canonical_path` later.
|
||||
"""
|
||||
return os.path.normcase(os.path.normpath(path))
|
||||
|
||||
|
||||
def _lexically_inside(path: str, parents: List[str]) -> bool:
|
||||
"""Whether the normalised ``path`` is one of ``parents`` or below one.
|
||||
|
||||
Pure text on normalised spellings (:func:`_lexical_path` or
|
||||
:func:`_canonical_path`): no filesystem access.
|
||||
"""
|
||||
sep = os.path.sep
|
||||
return any(
|
||||
path == parent or path.startswith(parent.rstrip(sep) + sep)
|
||||
for parent in parents
|
||||
)
|
||||
|
||||
|
||||
def _is_filesystem_root(path: str) -> bool:
|
||||
"""True for '/' on POSIX and drive roots such as 'C:\\' on Windows."""
|
||||
return os.path.dirname(path) == path
|
||||
@@ -342,31 +372,56 @@ class GalleryConfig:
|
||||
Filesystem roots are never allowed as parents, even when listed in
|
||||
the environment, because that would re-open the whole disk.
|
||||
"""
|
||||
parents = []
|
||||
return [canonical for canonical, _spelling in cls._allowed_parent_pairs()]
|
||||
|
||||
@classmethod
|
||||
def _allowed_parent_pairs(cls) -> List[Tuple[str, str]]:
|
||||
"""``(canonical, lexical spelling)`` of each allowed parent.
|
||||
|
||||
Only trusted directories (ComfyUI's own and the environment's) are
|
||||
resolved here; the spelling is how ComfyUI or the environment wrote
|
||||
the directory, so a submitted path can be compared without resolving it.
|
||||
"""
|
||||
pairs = []
|
||||
seen = set()
|
||||
for candidate in _comfyui_directories() + _extra_gallery_roots():
|
||||
try:
|
||||
canonical = _canonical_path(candidate)
|
||||
spelling = _lexical_path(os.path.abspath(candidate))
|
||||
except (OSError, ValueError):
|
||||
continue
|
||||
if _is_filesystem_root(canonical) or not os.path.isdir(canonical):
|
||||
continue
|
||||
if canonical not in parents:
|
||||
parents.append(canonical)
|
||||
return parents
|
||||
if canonical not in seen:
|
||||
seen.add(canonical)
|
||||
pairs.append((canonical, spelling))
|
||||
return pairs
|
||||
|
||||
@classmethod
|
||||
def path_anchors(cls) -> List[str]:
|
||||
"""Canonical directories that relative gallery paths are resolved against.
|
||||
|
||||
The ComfyUI base directory comes first, followed by the parent of each
|
||||
directory listed in PROMPT_MANAGER_EXTRA_GALLERY_ROOTS, so an extra
|
||||
root is addressed by its own name (``gallery/sub``) rather than by an
|
||||
absolute path. The same anchors drive the public (relative) form of
|
||||
paths in API responses.
|
||||
ComfyUI directory outside it (``--output-directory D:\\Renders``) and of
|
||||
each directory listed in PROMPT_MANAGER_EXTRA_GALLERY_ROOTS, so such a
|
||||
directory is addressed by its own name (``Renders/sub``) rather than by
|
||||
an absolute path, and two folders of one name stay apart. The same
|
||||
anchors drive the public (relative) form of paths in API responses.
|
||||
"""
|
||||
anchors = []
|
||||
base = _comfyui_base_directory()
|
||||
candidates = [base] if base else []
|
||||
try:
|
||||
base_key = _canonical_path(base) if base else None
|
||||
except (OSError, ValueError):
|
||||
base_key = None
|
||||
for directory in _comfyui_directories():
|
||||
try:
|
||||
canonical = _canonical_path(directory)
|
||||
except (OSError, ValueError):
|
||||
continue
|
||||
if base_key is None or not _lexically_inside(canonical, [base_key]):
|
||||
candidates.append(os.path.dirname(canonical))
|
||||
candidates.extend(os.path.dirname(root) for root in _extra_gallery_roots())
|
||||
for candidate in candidates:
|
||||
try:
|
||||
@@ -411,42 +466,63 @@ class GalleryConfig:
|
||||
def gallery_root_problem(cls, path: Any) -> Tuple[Optional[str], str]:
|
||||
"""``(code, reason)`` why ``path`` may not be a gallery root, else (None, "").
|
||||
|
||||
Codes: ``empty``, ``unresolvable``, ``filesystem-root``,
|
||||
``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).
|
||||
Codes: ``empty``, ``unresolvable``, ``outside-allowed``, ``not-found``
|
||||
and ``not-directory``. No reason names the path.
|
||||
|
||||
Containment is decided twice. First lexically, with no filesystem
|
||||
access at all: a path (UNC, ``\\\\?\\``, a device, anything) whose
|
||||
normalised spelling lies outside every allowed parent answers
|
||||
``outside-allowed`` before it is resolved, so the server never opens
|
||||
or stats it (no SMB connection, no existence or timing oracle). Only
|
||||
then is it resolved and checked again, which catches a symlink out.
|
||||
The filesystem root and the home directory answer ``outside-allowed``
|
||||
too: one answer for every caller (validate-source, the settings save,
|
||||
config.json), and the detail goes to the log only.
|
||||
"""
|
||||
if not isinstance(path, str) or not path.strip():
|
||||
return "empty", "Gallery root must be a non-empty path"
|
||||
|
||||
pairs = cls._allowed_parent_pairs()
|
||||
spellings = [spelling for pair in pairs for spelling in pair]
|
||||
try:
|
||||
lexical = cls._lexical_candidates(path)
|
||||
except (TypeError, ValueError):
|
||||
return "unresolvable", "Gallery root could not be resolved"
|
||||
if not any(_lexically_inside(candidate, spellings) for candidate in lexical):
|
||||
return "outside-allowed", OUTSIDE_ALLOWED_REASON
|
||||
|
||||
try:
|
||||
canonical = cls.resolve_gallery_root(path)
|
||||
except (OSError, ValueError):
|
||||
return "unresolvable", "Gallery root could not be resolved"
|
||||
|
||||
if _is_filesystem_root(canonical):
|
||||
return "filesystem-root", "Gallery root cannot be a filesystem root"
|
||||
config_logger.info("Gallery root refused: a filesystem root")
|
||||
return "outside-allowed", OUTSIDE_ALLOWED_REASON
|
||||
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}",
|
||||
)
|
||||
config_logger.info("Gallery root refused: the home directory")
|
||||
return "outside-allowed", OUTSIDE_ALLOWED_REASON
|
||||
if not _lexically_inside(canonical, [canonical for canonical, _ in pairs]):
|
||||
return "outside-allowed", OUTSIDE_ALLOWED_REASON
|
||||
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"
|
||||
return None, ""
|
||||
|
||||
@classmethod
|
||||
def _lexical_candidates(cls, path: str) -> List[str]:
|
||||
"""Every spelling ``path`` may resolve to, normalised as text (no IO).
|
||||
|
||||
An absolute path is itself; a relative one is joined to each
|
||||
:meth:`path_anchors` entry, as :meth:`resolve_gallery_root` tries them.
|
||||
"""
|
||||
path = path.strip()
|
||||
if os.path.isabs(path):
|
||||
return [_lexical_path(path)]
|
||||
anchors = cls.path_anchors()
|
||||
return [_lexical_path(os.path.join(anchor, path)) for anchor in anchors]
|
||||
|
||||
|
||||
class IntegrationConfig:
|
||||
"""Configuration for third-party extension integrations.
|
||||
|
||||
@@ -52,6 +52,7 @@ class AdminAPITestCase(AioHTTPTestCase):
|
||||
self.outside_dir = Path(self.tmpdir) / "outside"
|
||||
self.outside_dir.mkdir()
|
||||
|
||||
self.addAsyncCleanup(self._drain_and_remove)
|
||||
self.config_path = os.path.join(self.tmpdir, "config.json")
|
||||
self._orig_env = {
|
||||
key: os.environ.pop(key, None) for key in (CONFIG_PATH_ENV, EXTRA_ROOTS_ENV)
|
||||
@@ -89,6 +90,23 @@ class AdminAPITestCase(AioHTTPTestCase):
|
||||
sys.modules.pop("folder_paths", None)
|
||||
else:
|
||||
sys.modules["folder_paths"] = self._orig_folder_paths
|
||||
|
||||
async def _drain_and_remove(self):
|
||||
"""The last cleanup: no job outlives the temporary database.
|
||||
|
||||
Registered first, so it runs after every cleanup a test added (which
|
||||
may release its own gated jobs). A job still running then, such as a
|
||||
scan a failed assertion never awaited, is cancelled and awaited
|
||||
before the database and the tree are removed.
|
||||
"""
|
||||
api = getattr(self, "api", None)
|
||||
jobs = api._jobs.list() if api is not None else []
|
||||
tasks = [
|
||||
job.task for job in jobs if job.task is not None and not job.task.done()
|
||||
]
|
||||
for task in tasks:
|
||||
task.cancel()
|
||||
await asyncio.gather(*tasks, return_exceptions=True)
|
||||
PromptModel.close_all_instances() # restore may have swapped api.db
|
||||
assert_closed(self, self.tmpdir)
|
||||
shutil.rmtree(self.tmpdir, ignore_errors=True)
|
||||
|
||||
@@ -23,7 +23,11 @@ sys.modules.setdefault("server", _mock_server)
|
||||
|
||||
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||
|
||||
from py.config import GalleryConfig, PromptManagerConfig # noqa: E402
|
||||
from py.config import ( # noqa: E402
|
||||
OUTSIDE_ALLOWED_REASON,
|
||||
GalleryConfig,
|
||||
PromptManagerConfig,
|
||||
)
|
||||
|
||||
EXTRA_ROOTS_ENV = "PROMPT_MANAGER_EXTRA_GALLERY_ROOTS"
|
||||
|
||||
@@ -158,9 +162,10 @@ class GalleryRootTestCase(unittest.TestCase):
|
||||
class TestValidateGalleryRootRejects(GalleryRootTestCase):
|
||||
|
||||
def test_filesystem_root_rejected(self):
|
||||
"""Refused as outside the allowed folders: no reason names the layout."""
|
||||
ok, reason = GalleryConfig.validate_gallery_root(os.path.abspath(os.sep))
|
||||
self.assertFalse(ok)
|
||||
self.assertIn("root", reason.lower())
|
||||
self.assertEqual(reason, OUTSIDE_ALLOWED_REASON)
|
||||
|
||||
@unittest.skipUnless(os.name == "nt", "drive roots only exist on Windows")
|
||||
def test_windows_drive_root_rejected(self):
|
||||
@@ -170,7 +175,17 @@ class TestValidateGalleryRootRejects(GalleryRootTestCase):
|
||||
def test_home_directory_rejected(self):
|
||||
ok, reason = GalleryConfig.validate_gallery_root(os.path.expanduser("~"))
|
||||
self.assertFalse(ok)
|
||||
self.assertIn("home", reason.lower())
|
||||
self.assertEqual(reason, OUTSIDE_ALLOWED_REASON)
|
||||
|
||||
def test_home_directory_inside_an_allowed_parent_is_still_refused(self):
|
||||
"""Even when the environment opens the home's parent, home answers
|
||||
like any outside path."""
|
||||
home = os.path.expanduser("~")
|
||||
os.environ[EXTRA_ROOTS_ENV] = os.path.dirname(home)
|
||||
self.assertEqual(
|
||||
GalleryConfig.gallery_root_problem(home),
|
||||
("outside-allowed", OUTSIDE_ALLOWED_REASON),
|
||||
)
|
||||
|
||||
def test_directory_outside_comfyui_rejected(self):
|
||||
outside = Path(self.tmpdir) / "elsewhere"
|
||||
|
||||
+68
-15
@@ -275,8 +275,13 @@ class TestScanScope(ScanScopeTestCase):
|
||||
self.assertEqual(await _scan_request(Request("")), default)
|
||||
self.assertEqual(await _scan_request(Request(" \n")), default)
|
||||
self.assertEqual(
|
||||
await _scan_request(Request('{"scope": "all", "roots": [2, 0, 2]}')),
|
||||
({"files": "all", "roots": [0, 2]}, None),
|
||||
await _scan_request(
|
||||
Request(
|
||||
'{"scope": "all", "roots": [2, 0, 2],'
|
||||
' "paths": ["output/c", "output/a", "output/c"]}'
|
||||
)
|
||||
),
|
||||
({"files": "all", "roots": {0: "output/a", 2: "output/c"}}, None),
|
||||
)
|
||||
request, error = await _scan_request(Request("{"))
|
||||
self.assertIsNone(request)
|
||||
@@ -332,7 +337,7 @@ class TestPerSourceScope(ScanScopeTestCase):
|
||||
self.api._collect_output_media_sync = spy
|
||||
reads = self._spy_reads()
|
||||
|
||||
job = await self._scan({"scope": "all", "roots": [1]})
|
||||
job = await self._scan({"scope": "all", "roots": [1], "paths": ["output/b"]})
|
||||
|
||||
self.assertEqual(walked, [["b"]])
|
||||
self.assertEqual(reads, ["in_b.png"])
|
||||
@@ -354,13 +359,15 @@ class TestPerSourceScope(ScanScopeTestCase):
|
||||
self.assertEqual(self.api._scan_job.last_event["directories"], ["output/b"])
|
||||
|
||||
async def test_duplicate_roots_are_one_selection(self):
|
||||
job = await self._scan({"roots": [1, 0, 1]})
|
||||
job = await self._scan(
|
||||
{"roots": [1, 0, 1], "paths": ["output/b", "output/a", "output/b"]}
|
||||
)
|
||||
self.assertEqual(job["scope"]["roots"], [0, 1])
|
||||
self.assertEqual(job["scope"]["output_folders"], 2)
|
||||
self.assertEqual(job["counters"]["added"], 2)
|
||||
|
||||
async def test_scope_line_counts_selected_folders(self):
|
||||
job = await self._scan({"roots": [0]})
|
||||
job = await self._scan({"roots": [0], "paths": ["output/a"]})
|
||||
self.assertTrue(
|
||||
job["log"][0].endswith(
|
||||
"Scan started · 1 output folder · new and unresolved files"
|
||||
@@ -375,11 +382,43 @@ class TestPerSourceScope(ScanScopeTestCase):
|
||||
)
|
||||
self.assertIn("Scan started · 2 output folders · all files", unscoped["log"][0])
|
||||
|
||||
async def test_invalid_root_index_is_400_without_job(self):
|
||||
data = await self._refused({"roots": [2]})
|
||||
self.assertEqual(data["code"], "unknown-source")
|
||||
data = await self._refused({"roots": [0, 64]})
|
||||
self.assertEqual(data["code"], "unknown-source")
|
||||
async def _stale(self, body):
|
||||
resp = await self.client.request("POST", "/prompt_manager/jobs/scan", json=body)
|
||||
data = await resp.json()
|
||||
self.assertEqual(resp.status, 409, data)
|
||||
self.assertEqual(data["code"], "sources-changed")
|
||||
self.assertFalse(data["success"])
|
||||
self.assertNotIn(self.tmpdir, json.dumps(data))
|
||||
self.assertIsNone(self.api._scan_job)
|
||||
self.assertEqual(self.api._jobs.list(), [])
|
||||
return data
|
||||
|
||||
async def test_index_past_the_sources_is_409_without_job(self):
|
||||
await self._stale({"roots": [2], "paths": ["output/c"]})
|
||||
await self._stale({"roots": [0, 64], "paths": ["output/a", "output/x"]})
|
||||
|
||||
async def test_stale_index_never_scans_another_source(self):
|
||||
"""The list changed after the page read it: source 0 was removed, so
|
||||
old index 1 (b) is past the end and old index 0 now names b."""
|
||||
GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.b)]
|
||||
await self._stale({"roots": [1], "paths": ["output/b"]})
|
||||
await self._stale({"roots": [0], "paths": ["output/a"]})
|
||||
|
||||
job = await self._scan({"roots": [0], "paths": ["output/b"]})
|
||||
self.assertEqual(job["outcomes"]["added"][0]["root"], "output/b")
|
||||
|
||||
async def test_roots_need_their_paths(self):
|
||||
for body in (
|
||||
{"roots": [0]},
|
||||
{"roots": [0], "paths": None},
|
||||
{"roots": [0], "paths": "output/a"},
|
||||
{"roots": [0], "paths": []},
|
||||
{"roots": [0, 1], "paths": ["output/a"]},
|
||||
{"roots": [0], "paths": [0]},
|
||||
{"roots": [0, 0], "paths": ["output/a", "output/b"]},
|
||||
):
|
||||
with self.subTest(body=body):
|
||||
await self._refused(body)
|
||||
|
||||
async def test_bool_and_float_roots_rejected(self):
|
||||
for roots in (
|
||||
@@ -399,7 +438,8 @@ class TestPerSourceScope(ScanScopeTestCase):
|
||||
list(range(65)),
|
||||
):
|
||||
with self.subTest(roots=roots):
|
||||
await self._refused({"roots": roots})
|
||||
paths = ["output/a"] * len(roots) if isinstance(roots, list) else []
|
||||
await self._refused({"roots": roots, "paths": paths})
|
||||
|
||||
async def test_legacy_scan_unscoped_unchanged(self):
|
||||
from py.api.admin import SCAN_COUNT_KEYS
|
||||
@@ -429,16 +469,29 @@ class TestPerSourceScope(ScanScopeTestCase):
|
||||
return original(output_dirs)
|
||||
|
||||
self.api._collect_output_media_sync = gated
|
||||
# The legacy route has decided to attach once it streams the job.
|
||||
attached = asyncio.Event()
|
||||
stream = self.api._stream_scan_job
|
||||
|
||||
async def attaching(request, job):
|
||||
attached.set()
|
||||
return await stream(request, job)
|
||||
|
||||
self.api._stream_scan_job = attaching
|
||||
resp = await self.client.request(
|
||||
"POST", "/prompt_manager/jobs/scan", json={"roots": [0]}
|
||||
"POST",
|
||||
"/prompt_manager/jobs/scan",
|
||||
json={"roots": [0], "paths": ["output/a"]},
|
||||
)
|
||||
self.assertEqual(resp.status, 202)
|
||||
started = (await resp.json())["job"]
|
||||
legacy = asyncio.ensure_future(
|
||||
self.client.request("POST", "/prompt_manager/scan", data="{}")
|
||||
)
|
||||
await asyncio.sleep(0.05)
|
||||
gate.set()
|
||||
try:
|
||||
await asyncio.wait_for(attached.wait(), 10)
|
||||
finally:
|
||||
gate.set()
|
||||
payloads = _sse_payloads(await (await legacy).text())
|
||||
await asyncio.wait_for(self.api._scan_job.task, 10)
|
||||
|
||||
@@ -459,7 +512,7 @@ class TestPerSourceScope(ScanScopeTestCase):
|
||||
return original()
|
||||
|
||||
self.api._get_all_output_dirs = counting
|
||||
job = await self._scan({"roots": [1]})
|
||||
job = await self._scan({"roots": [1], "paths": ["output/b"]})
|
||||
self.assertEqual(len(resolved), 1) # the request's own resolution only
|
||||
self.assertEqual(job["outcomes"]["added"][0]["root_index"], 1)
|
||||
|
||||
|
||||
+379
-11
@@ -8,10 +8,14 @@ checks one path and writes nothing. No response carries an absolute path.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import functools
|
||||
import json
|
||||
import ntpath
|
||||
import os
|
||||
import sys
|
||||
import types
|
||||
import unittest
|
||||
from contextlib import contextmanager
|
||||
from unittest.mock import patch
|
||||
|
||||
try:
|
||||
@@ -19,10 +23,40 @@ try:
|
||||
except ImportError: # discovered with tests/ as the top-level directory
|
||||
from test_api_admin import AdminAPITestCase
|
||||
|
||||
from py import config as config_module # noqa: E402
|
||||
from py.api import settings_routes, sources # noqa: E402
|
||||
from py.config import GalleryConfig # noqa: E402
|
||||
|
||||
VALIDATE = "/prompt_manager/settings/validate-source"
|
||||
SETTINGS = "/prompt_manager/settings"
|
||||
|
||||
|
||||
@contextmanager
|
||||
def no_filesystem_access_for(marker, test):
|
||||
"""Fail ``test`` if anything resolves, stats or opens a path holding ``marker``.
|
||||
|
||||
Trusted directories (ComfyUI's, the anchors) may still be resolved.
|
||||
"""
|
||||
real = {
|
||||
"realpath": os.path.realpath,
|
||||
"stat": os.stat,
|
||||
"lstat": os.lstat,
|
||||
}
|
||||
|
||||
def guard(name):
|
||||
def call(path, *args, **kwargs):
|
||||
if isinstance(path, (str, bytes)) and marker in os.fsdecode(path):
|
||||
test.fail(f"{name}({path!r}) touched an outside path")
|
||||
return real[name](path, *args, **kwargs)
|
||||
|
||||
return call
|
||||
|
||||
with (
|
||||
patch("os.path.realpath", guard("realpath")),
|
||||
patch("os.stat", guard("stat")),
|
||||
patch("os.lstat", guard("lstat")),
|
||||
):
|
||||
yield
|
||||
|
||||
|
||||
class FakeMonitor:
|
||||
@@ -188,12 +222,12 @@ class TestSourceStatus(SourcesTestCase):
|
||||
)
|
||||
|
||||
# Monitoring switched off: the live observer still watches until the
|
||||
# restart, and nothing else is pending.
|
||||
# restart ("stopping"), and nothing else is pending.
|
||||
GalleryConfig.MONITORING_ENABLED = False
|
||||
data = await self._get()
|
||||
self.assertFalse(data["settings"]["monitoring_enabled"])
|
||||
self.assertEqual(
|
||||
self._states(data), {"output/a": "watching", "output/b": "manual"}
|
||||
self._states(data), {"output/a": "stopping", "output/b": "manual"}
|
||||
)
|
||||
|
||||
async def test_scheduled_root_matches_through_its_spelling(self):
|
||||
@@ -343,9 +377,7 @@ class TestSourceChangeHold(SourcesTestCase):
|
||||
async def test_mixed_case_windows_paths_dedupe(self):
|
||||
"""Two spellings of one Windows folder are saved once (normcase)."""
|
||||
with (
|
||||
patch.object(
|
||||
GalleryConfig, "validate_gallery_root", return_value=(True, "")
|
||||
),
|
||||
patch.object(sources, "source_problem", return_value=(None, "")),
|
||||
patch.object(
|
||||
GalleryConfig, "resolve_gallery_root", side_effect=lambda path: path
|
||||
),
|
||||
@@ -457,15 +489,139 @@ class TestValidateSource(SourcesTestCase):
|
||||
self.assertEqual([answer for answer in answers], [answers[0]] * len(answers))
|
||||
|
||||
async def test_settings_save_refuses_outside_paths_alike(self):
|
||||
"""The legacy save answers every outside path alike too: the
|
||||
filesystem root, the home directory and a link to it included."""
|
||||
(self.outside_dir / "x").mkdir()
|
||||
link = os.path.join(self.tmpdir, "link-home")
|
||||
try:
|
||||
os.symlink(os.path.expanduser("~"), link, target_is_directory=True)
|
||||
except (OSError, NotImplementedError):
|
||||
link = None
|
||||
paths = [
|
||||
str(self.outside_dir / "x"),
|
||||
self.tmpdir + "/nothing/x",
|
||||
os.path.abspath(os.sep),
|
||||
os.path.expanduser("~"),
|
||||
]
|
||||
if link:
|
||||
paths.append(link)
|
||||
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)]}
|
||||
for path in paths:
|
||||
resp = await self.client.request(
|
||||
"POST", SETTINGS, json={"gallery_root_paths": [path]}
|
||||
)
|
||||
answers.append((status, data))
|
||||
self.assertEqual(answers[0][0], 400)
|
||||
self.assertEqual(answers[0], answers[1])
|
||||
body = await resp.text()
|
||||
self.assertNotIn(os.path.expanduser("~"), body)
|
||||
data = json.loads(body)
|
||||
# Only the name the caller sent may differ.
|
||||
answers.append(
|
||||
(
|
||||
resp.status,
|
||||
{k: v for k, v in data.items() if k not in ("source", "error")},
|
||||
)
|
||||
)
|
||||
self.assertEqual(
|
||||
data["error"],
|
||||
f"{data['source']} can't be used: "
|
||||
f"{config_module.OUTSIDE_ALLOWED_REASON}. Remove it, then save.",
|
||||
)
|
||||
self.assertEqual(
|
||||
answers[0],
|
||||
(
|
||||
400,
|
||||
{
|
||||
"success": False,
|
||||
"code": "invalid-source",
|
||||
"index": 0,
|
||||
"reason": config_module.OUTSIDE_ALLOWED_REASON,
|
||||
},
|
||||
),
|
||||
)
|
||||
self.assertEqual(answers, [answers[0]] * len(answers))
|
||||
self.assertEqual(GalleryConfig.MONITORING_DIRECTORIES, [])
|
||||
|
||||
def test_every_caller_folds_root_and_home_alike(self):
|
||||
for path in (os.path.abspath(os.sep), os.path.expanduser("~")):
|
||||
with self.subTest(path=path):
|
||||
self.assertEqual(
|
||||
GalleryConfig.gallery_root_problem(path),
|
||||
("outside-allowed", config_module.OUTSIDE_ALLOWED_REASON),
|
||||
)
|
||||
self.assertEqual(
|
||||
GalleryConfig.validate_gallery_root(path),
|
||||
(False, config_module.OUTSIDE_ALLOWED_REASON),
|
||||
)
|
||||
self.assertEqual(
|
||||
sources.validate_source(path)["code"], "outside-allowed"
|
||||
)
|
||||
|
||||
async def test_outside_path_is_never_resolved_or_stat(self):
|
||||
"""Containment is decided as text first: no realpath, stat or lstat of
|
||||
an outside path, existing or not, by any caller (I2)."""
|
||||
probe = self.outside_dir / "probe-x"
|
||||
probe.mkdir()
|
||||
(probe / "deep").mkdir()
|
||||
cases = [str(probe), str(probe / "deep"), str(probe / "missing")]
|
||||
with no_filesystem_access_for("probe-x", self):
|
||||
for path in cases:
|
||||
with self.subTest(path=path):
|
||||
self.assertEqual(
|
||||
GalleryConfig.gallery_root_problem(path)[0], "outside-allowed"
|
||||
)
|
||||
self.assertEqual(
|
||||
sources.validate_source(path)["code"], "outside-allowed"
|
||||
)
|
||||
_roots, error = self.api._parse_gallery_roots(
|
||||
{"gallery_root_paths": [path]}
|
||||
)
|
||||
self.assertEqual(error["code"], "invalid-source")
|
||||
status, data = await self._post(VALIDATE, {"path": path})
|
||||
self.assertEqual(data["code"], "outside-allowed")
|
||||
status, data = await self._post(
|
||||
SETTINGS, {"gallery_root_paths": [path]}
|
||||
)
|
||||
self.assertEqual((status, data["code"]), (400, "invalid-source"))
|
||||
# A traversal that normalises outside is refused as text too.
|
||||
sneaky = os.path.join(
|
||||
str(self.output_dir), "..", "..", "outside", "probe-x"
|
||||
)
|
||||
self.assertEqual(
|
||||
GalleryConfig.gallery_root_problem(sneaky)[0], "outside-allowed"
|
||||
)
|
||||
|
||||
async def test_settings_save_applies_the_form_checks(self):
|
||||
"""POST /settings refuses what validate-source refuses, by its form."""
|
||||
for path, code in {
|
||||
str(self.a) + "\x00": "invalid-form",
|
||||
os.path.join(str(self.output_dir), "b", "..", "a"): "traversal",
|
||||
}.items():
|
||||
with self.subTest(code=code):
|
||||
status, data = await self._post(
|
||||
SETTINGS, {"gallery_root_paths": [path]}
|
||||
)
|
||||
self.assertEqual((status, data["code"]), (400, "invalid-source"))
|
||||
self.assertEqual(data["reason"], sources.FORM_REASONS[code])
|
||||
windows = functools.partial(sources.source_form_error, windows=True)
|
||||
with (
|
||||
patch.object(sources, "source_form_error", windows),
|
||||
no_filesystem_access_for("C:", self),
|
||||
):
|
||||
for path in (
|
||||
"\\\\?\\C:\\ComfyUI\\output",
|
||||
"\\\\.\\pipe\\x",
|
||||
"C:\\ComfyUI\\output\\CON",
|
||||
"C:\\ComfyUI\\output\\a:stream",
|
||||
"C:ComfyUI",
|
||||
):
|
||||
with self.subTest(path=path):
|
||||
_roots, error = self.api._parse_gallery_roots(
|
||||
{"gallery_root_paths": ["output/a", path]}
|
||||
)
|
||||
self.assertEqual(error["code"], "invalid-source")
|
||||
self.assertEqual(error["index"], 1)
|
||||
self.assertEqual(
|
||||
error["reason"], sources.FORM_REASONS["invalid-form"]
|
||||
)
|
||||
|
||||
async def test_inside_paths_say_why_they_are_not_usable(self):
|
||||
a_file = self.output_dir / "file.png"
|
||||
@@ -535,6 +691,218 @@ class TestSourceEdges(SourcesTestCase):
|
||||
self.assertEqual(sources.source_label(self.output_dir), "output")
|
||||
|
||||
|
||||
class TestOutsideBaseDirectories(SourcesTestCase):
|
||||
"""ComfyUI folders outside the base directory (``--output-directory``):
|
||||
their sources round-trip and two folders of one name stay apart (I3)."""
|
||||
|
||||
async def asyncSetUp(self):
|
||||
await super().asyncSetUp()
|
||||
self.renders = self.outside_dir / "renders"
|
||||
self.input_dir = self.comfy_dir / "input"
|
||||
(self.renders / "portraits").mkdir(parents=True)
|
||||
(self.input_dir / "portraits").mkdir(parents=True)
|
||||
sys.modules["folder_paths"] = types.SimpleNamespace(
|
||||
base_path=str(self.comfy_dir),
|
||||
get_output_directory=lambda: str(self.renders),
|
||||
get_input_directory=lambda: str(self.input_dir),
|
||||
)
|
||||
|
||||
async def test_source_outside_base_round_trips(self):
|
||||
status, rendered = await self._post(
|
||||
VALIDATE, {"path": str(self.renders / "portraits")}
|
||||
)
|
||||
self.assertEqual(
|
||||
(rendered["valid"], rendered["path"]), (True, "renders/portraits")
|
||||
)
|
||||
status, data = await self._post(
|
||||
SETTINGS, {"gallery_root_paths": [rendered["path"]]}
|
||||
)
|
||||
self.assertEqual(status, 200, data)
|
||||
self.assertEqual(
|
||||
GalleryConfig.MONITORING_DIRECTORIES,
|
||||
[self._root(self.renders / "portraits")],
|
||||
)
|
||||
configured = (await self._get())["sources"]["configured"]
|
||||
self.assertEqual(
|
||||
[(row["path"], row["valid"]) for row in configured],
|
||||
[("renders/portraits", True)],
|
||||
)
|
||||
# The page resends what it read: still the same folder, no change.
|
||||
status, data = await self._post(
|
||||
SETTINGS, {"gallery_root_paths": ["renders/portraits"]}
|
||||
)
|
||||
self.assertEqual((status, data["restart_required_fields"]), (200, []))
|
||||
|
||||
async def test_same_name_folders_stay_apart(self):
|
||||
answers = []
|
||||
for path in (self.renders / "portraits", self.input_dir / "portraits"):
|
||||
status, data = await self._post(VALIDATE, {"path": str(path)})
|
||||
answers.append(data["path"])
|
||||
self.assertEqual(answers, ["renders/portraits", "input/portraits"])
|
||||
status, data = await self._post(SETTINGS, {"gallery_root_paths": answers})
|
||||
self.assertEqual(status, 200, data)
|
||||
sources_ = (await self._get())["sources"]
|
||||
self.assertEqual([row["path"] for row in sources_["configured"]], answers)
|
||||
self.assertEqual([row["path"] for row in sources_["effective"]], answers)
|
||||
|
||||
|
||||
class TestUnusableSourceSave(SourcesTestCase):
|
||||
async def test_unusable_source_is_named_and_saves_nothing(self):
|
||||
"""A configured source that went missing blocks the save and is named."""
|
||||
gone = self.output_dir / "gone"
|
||||
gone.mkdir()
|
||||
GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.a), self._root(gone)]
|
||||
self._write_config({"custom": 1})
|
||||
before = self._config_bytes()
|
||||
gone.rmdir() # an unplugged drive
|
||||
|
||||
status, data = await self._post(
|
||||
SETTINGS,
|
||||
{
|
||||
"gallery_root_paths": ["output/a", "output/gone", "output/b"],
|
||||
"worker_threads": 2,
|
||||
},
|
||||
)
|
||||
|
||||
self.assertEqual(status, 400, data)
|
||||
self.assertEqual(
|
||||
data,
|
||||
{
|
||||
"success": False,
|
||||
"code": "invalid-source",
|
||||
"source": "output/gone",
|
||||
"index": 1,
|
||||
"reason": "Gallery root does not exist",
|
||||
"error": "output/gone can't be used: Gallery root does not exist. "
|
||||
"Remove it or reconnect it, then save.",
|
||||
},
|
||||
)
|
||||
self.assertEqual(self._config_bytes(), before)
|
||||
|
||||
async def test_malformed_roots_are_coded_invalid_request(self):
|
||||
for roots in ("output/a", [1]):
|
||||
with self.subTest(roots=roots):
|
||||
status, data = await self._post(SETTINGS, {"gallery_root_paths": roots})
|
||||
self.assertEqual((status, data["code"]), (400, "invalid-request"))
|
||||
|
||||
|
||||
class TestUnchangedRootsTakeNoHold(SourcesTestCase):
|
||||
async def test_resent_spelling_of_configured_roots_is_no_change(self):
|
||||
"""config.json holds a hand-written spelling; the legacy page resends
|
||||
it resolved: no source change, so no hold and no 409 during a scan (M1)."""
|
||||
GalleryConfig.MONITORING_DIRECTORIES = ["output/a"]
|
||||
gate = asyncio.Event()
|
||||
|
||||
async def runner(job):
|
||||
await gate.wait()
|
||||
job.finish("finished")
|
||||
|
||||
self.api._jobs.start(
|
||||
"scan", "Metadata scan", record_changing=True, runner=runner
|
||||
)
|
||||
try:
|
||||
status, data = await self._post(
|
||||
SETTINGS, {"gallery_root_paths": [str(self.a)], "result_timeout": 30}
|
||||
)
|
||||
self.assertEqual(status, 200, data)
|
||||
self.assertEqual(data["restart_required_fields"], [])
|
||||
self.assertEqual([j.kind for j in self.api._jobs.list()], ["scan"])
|
||||
status, data = await self._post(
|
||||
SETTINGS, {"gallery_root_paths": [str(self.b)]}
|
||||
)
|
||||
self.assertEqual((status, data["code"]), (409, "busy"))
|
||||
finally:
|
||||
gate.set()
|
||||
|
||||
|
||||
class TestWindowsContainment(unittest.TestCase):
|
||||
"""Windows path forms, with ntpath fakes: anything outside the allowed
|
||||
folders (a UNC share, the verbatim and device namespaces, another drive)
|
||||
is refused as text, before any path is resolved or opened (I2)."""
|
||||
|
||||
ALLOWED = "c:\\comfyui\\output"
|
||||
NAS = "\\\\nas\\share\\renders"
|
||||
|
||||
def setUp(self):
|
||||
self.touched = []
|
||||
|
||||
def realpath(path, *args, **kwargs):
|
||||
self.touched.append(path)
|
||||
return ntpath.normcase(ntpath.normpath(path))
|
||||
|
||||
def probe(path):
|
||||
self.touched.append(path)
|
||||
return True
|
||||
|
||||
names = ("normcase", "normpath", "isabs", "join", "dirname", "basename", "sep")
|
||||
fake_path = types.SimpleNamespace(
|
||||
**{name: getattr(ntpath, name) for name in names},
|
||||
realpath=realpath,
|
||||
exists=probe,
|
||||
isdir=probe,
|
||||
expanduser=lambda path: "c:\\users\\someone",
|
||||
)
|
||||
fake_os = types.SimpleNamespace(
|
||||
path=fake_path, environ={}, pathsep=";", name="nt"
|
||||
)
|
||||
for target, value in (
|
||||
(patch.object(config_module, "os", fake_os), None),
|
||||
(
|
||||
patch.object(
|
||||
GalleryConfig,
|
||||
"_allowed_parent_pairs",
|
||||
classmethod(
|
||||
lambda cls: [(self.ALLOWED, self.ALLOWED), (self.NAS, self.NAS)]
|
||||
),
|
||||
),
|
||||
None,
|
||||
),
|
||||
(
|
||||
patch.object(
|
||||
GalleryConfig,
|
||||
"path_anchors",
|
||||
classmethod(lambda cls: ["c:\\comfyui", "\\\\nas\\share"]),
|
||||
),
|
||||
None,
|
||||
),
|
||||
):
|
||||
target.start()
|
||||
self.addCleanup(target.stop)
|
||||
|
||||
def test_outside_forms_are_refused_untouched(self):
|
||||
for path in (
|
||||
"\\\\attacker\\share\\x",
|
||||
"//attacker/share/x",
|
||||
"\\\\?\\C:\\ComfyUI\\output\\a",
|
||||
"\\\\?\\UNC\\attacker\\share",
|
||||
"\\\\.\\pipe\\x",
|
||||
"\\\\.\\C:\\ComfyUI\\output",
|
||||
"D:\\Renders",
|
||||
"C:\\Windows",
|
||||
"C:\\ComfyUI\\output\\..\\..\\Windows",
|
||||
"C:\\ComfyUI\\output2",
|
||||
"\\\\nas\\share\\other",
|
||||
"..\\..\\Windows",
|
||||
):
|
||||
with self.subTest(path=path):
|
||||
self.touched.clear()
|
||||
self.assertEqual(
|
||||
GalleryConfig.gallery_root_problem(path),
|
||||
("outside-allowed", config_module.OUTSIDE_ALLOWED_REASON),
|
||||
)
|
||||
self.assertEqual(self.touched, [])
|
||||
|
||||
def test_allowed_spellings_are_accepted(self):
|
||||
for path in (
|
||||
"C:\\ComfyUI\\output\\a",
|
||||
"c:/comfyui/OUTPUT/a",
|
||||
"output\\a",
|
||||
"\\\\NAS\\share\\renders\\a", # a UNC spelling of an allowed extra root
|
||||
):
|
||||
with self.subTest(path=path):
|
||||
self.assertEqual(GalleryConfig.gallery_root_problem(path), (None, ""))
|
||||
|
||||
|
||||
class TestSourceFormError(unittest.TestCase):
|
||||
"""The lexical check that runs before any filesystem access."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user