diff --git a/AGENTS.md b/AGENTS.md index 399a876..9d1c4b3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -108,6 +108,12 @@ example_workflows/ # example workflows (regular workflow JSON, not API JSON) even for one output: `(value,)`. - Console output at import time must stay ASCII — ComfyUI on Windows often runs a cp1252 console, and a UnicodeEncodeError there kills the whole pack. +- Widget values and route parameters are attacker-controlled: ComfyUI's + `/prompt` and the pack's routes need no login. A node never contacts a + host taken from one, never hands one to a subprocess, and reads or writes + only inside ComfyUI's input, output and temp folders - no opt-in + switches, no "any folder" settings, no folder pickers. The Registry bans + versions for exactly this. - No new pip dependencies without an explicit decision; if truly optional, use `[project.optional-dependencies]` and fail soft at runtime. - Frontend JS never assigns prototype callbacks directly — use diff --git a/CHANGELOG.md b/CHANGELOG.md index aa9b3ec..f2b3050 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,26 @@ All notable changes to ComfyUI-AusBoss are documented here. +## Unreleased + +- **Save Image writes only inside ComfyUI's output folder.** `output_dir` + used to take any absolute path, so any client that can reach the + unauthenticated `/prompt` route could make the server write an image and a + caption `.txt` of its choosing into any folder ComfyUI can write, + overwriting by default - one step from code execution through a custom + node's `requirements.txt`. `output_dir` now takes a subfolder name inside + the output folder, like ComfyUI's own Save Image; absolute paths, drive + letters, `~` and `..` are refused before the run, and the card's path + preview says so as you type. Workflows that saved into an absolute folder + need a subfolder name instead. +- **Local path mode reads only ComfyUI's input, output and temp folders.** + The 2.0.1 `AUSBOSS_TRANSFORM_LOCAL_PREVIEW` switch that opened the rest of + the disk is gone; a refused path says where the video has to be. +- Save Image checks the complete image and caption destinations, and Save + Video checks both the prefix and final file, rejecting symlinks that lead + outside the output folder. Windows drive and alternate-stream syntax is + rejected before filesystem access. + ## 2.0.1 - 2026-09-09 - **Local path mode is opt-in beyond ComfyUI's folders.** The video diff --git a/README.md b/README.md index 3a259dd..4617b34 100644 --- a/README.md +++ b/README.md @@ -291,7 +291,7 @@ The fourteen other examples use core nodes plus this pack. Model-free examples a Under **Chrome**, queue status can appear in the browser title/favicon, live badges report node progress, and optional runtime badges display per-node execution time. LoRA Loader and Resolution Master have their own gear menus. -The video transform nodes' local path mode reads only ComfyUI's input, output and temp folders unless ComfyUI is started with the explicit option `AUSBOSS_TRANSFORM_LOCAL_PREVIEW=1`, for queued runs and live previews alike; see [the video transform help](js/docs/AUSBOSS_NODES_VideoCropRotatePad.md). +The video transform nodes' local path mode reads only videos inside ComfyUI's input, output and temp folders, and Save Image writes only inside the output folder: no widget can point the pack anywhere else on the disk; see [the video transform help](js/docs/AUSBOSS_NODES_VideoCropRotatePad.md). ## Optional extras diff --git a/js/docs/AUSBOSS_NODES_SaveImage.md b/js/docs/AUSBOSS_NODES_SaveImage.md index 80f4485..3812c26 100644 --- a/js/docs/AUSBOSS_NODES_SaveImage.md +++ b/js/docs/AUSBOSS_NODES_SaveImage.md @@ -8,9 +8,11 @@ file's name. A caption on the `caption_text` input writes the paired `.txt`. ## The card -- **Folder**: empty saves to ComfyUI's output folder; a relative path is a - subfolder of it; an absolute path saves anywhere you can write. **Browse** - walks the subfolders of the output folder. +- **Folder**: empty saves to ComfyUI's output folder; a name such as + `sets/portraits` saves to that subfolder, created if missing. Save Image + never writes outside the output folder: absolute paths, drive letters, `~` + and `..` are refused before the run, and the path preview says so as you + type. **Browse** walks the subfolders of the output folder. - **Filename**: the local name, subfolders allowed (`sets/shot`). While the `filename` input is linked the field reads `{{filename}}` and the tags fold away - an exact name is never decorated. A workflow saved with the old diff --git a/js/docs/AUSBOSS_NODES_VideoCropRotatePad.md b/js/docs/AUSBOSS_NODES_VideoCropRotatePad.md index dd34e29..7ec872a 100644 --- a/js/docs/AUSBOSS_NODES_VideoCropRotatePad.md +++ b/js/docs/AUSBOSS_NODES_VideoCropRotatePad.md @@ -14,10 +14,9 @@ Loads one exact video frame and applies the same **rotate → crop → pad** tra - **Canvas row** under the format chips: fill swatch, feather amount and resize budget on the node face. - **source_mode**: `input folder` or `local path`. Local path mode avoids copying large files. -- **local_path**: Absolute path used only in local path mode. By default it reaches - only ComfyUI's own input, output and temp folders, for queued runs and the editor's - live preview alike; starting ComfyUI with `AUSBOSS_TRANSFORM_LOCAL_PREVIEW=1` opens - the rest of the disk. +- **local_path**: Absolute path used only in local path mode, read in place without an + upload copy. It must point inside ComfyUI's input, output or temp folder, for queued + runs and the editor's live preview alike; paths anywhere else are refused. - **Playhead**: The rail under the node's preview is the timeline: press or drag it to scrub, or type a frame in the **Frame** box; the frame on the stage is the frame the node outputs (`frame_index`, with `frame_time` kept in step). diff --git a/js/docs/AUSBOSS_NODES_VideoCropRotatePadClip.md b/js/docs/AUSBOSS_NODES_VideoCropRotatePadClip.md index 6fcb23c..ea172de 100644 --- a/js/docs/AUSBOSS_NODES_VideoCropRotatePadClip.md +++ b/js/docs/AUSBOSS_NODES_VideoCropRotatePadClip.md @@ -22,10 +22,9 @@ model that paints black regions. sizes rounded to 32 - so a wrong value shows here before a render is wasted. A fresh clip node starts with that pair (black, feather 0) and the editor's **Reset all** returns to it; the image nodes keep their soft grey canvas. -- **Local path**: Read a file on the ComfyUI server in place without copying it. By - default only ComfyUI's own input, output and temp folders are readable, for queued - runs and the editor's live preview alike; starting ComfyUI with - `AUSBOSS_TRANSFORM_LOCAL_PREVIEW=1` opens the rest of the disk. +- **Local path**: Read a video on the ComfyUI server in place without an upload copy. + It must sit inside ComfyUI's input, output or temp folder, for queued runs and the + editor's live preview alike; paths anywhere else are refused. - **Timeline**: The rail on the node and in the editor is one timeline. Press or drag anywhere on it to scrub the **playhead**; the stage shows that frame. Drag the **IN** or **OUT** handle to trim: the playhead rides on the handle, so what you see diff --git a/js/save_image/index.js b/js/save_image/index.js index 08f2622..f9ab66e 100644 --- a/js/save_image/index.js +++ b/js/save_image/index.js @@ -201,7 +201,7 @@ function buildCard(node) { // Folder root.append(el("div", "ausboss-save-cap", "Folder")); const folderRow = el("div", "ausboss-save-row"); - const folder = textField(state, "output_dir", "ComfyUI/output", "Where to save. Empty is ComfyUI's output folder; a relative path is a subfolder of it; an absolute path saves anywhere you can write."); + const folder = textField(state, "output_dir", "ComfyUI/output", "Subfolder of ComfyUI's output folder to save into, such as sets/portraits. Empty saves to the output folder itself; Save Image never writes outside it."); const browse = el("button", "ausboss-card-btn", "Browse"); browse.title = "Pick a subfolder of ComfyUI's output folder"; browse.addEventListener("click", () => openBrowse(state, browse)); diff --git a/js/shared/save_image_naming.mjs b/js/shared/save_image_naming.mjs index 3074466..5dc3ce5 100644 --- a/js/shared/save_image_naming.mjs +++ b/js/shared/save_image_naming.mjs @@ -68,8 +68,12 @@ export function previewFolder(values, outputLabel = "ComfyUI/output") { const prefix = String(values.filename_prefix ?? "").trim().replace(/\\/g, "/"); const mode = namingMode({ filenameLinked: values.filenameLinked, exactName: values.exact_name }); const prefixFolder = mode === "local" ? prefix.split("/").filter((part) => part && part !== ".").slice(0, -1).join("/") : ""; - const absolute = /^([A-Za-z]:[\\/]|\/|~)/.test(folder); - const segments = absolute ? [folder.replace(/\/+$/, "")] : [outputLabel, folder.replace(/^\/+|\/+$/g, "")]; + // Save Image writes only inside the output folder; say so where the path + // would be, before a run refuses it. + if (/^(\/|~)/.test(folder) || folder.includes(":") || folder.split("/").includes("..")) { + return `Not saved: use a subfolder of ${outputLabel}`; + } + const segments = [outputLabel, folder.replace(/^\/+|\/+$/g, "")]; if (prefixFolder) segments.push(prefixFolder); return segments.filter(Boolean).join("/") + "/"; } diff --git a/js/shared/transform_editor.mjs b/js/shared/transform_editor.mjs index a40d566..b68bca4 100644 --- a/js/shared/transform_editor.mjs +++ b/js/shared/transform_editor.mjs @@ -248,7 +248,7 @@ function buildMediaSourceCard(state) { const localMode = createElement("button", "", "Local path"); uploadsMode.type = localMode.type = "button"; uploadsMode.title = "Choose a video already in ComfyUI's input folder or upload another."; - localMode.title = "Read a video directly from an absolute path on this server without copying it."; + localMode.title = "Read a video inside ComfyUI's input, output or temp folder in place, without copying it."; modes.append(uploadsMode, localMode); const field = createElement("div", "ausboss-transform-source-field"); @@ -303,7 +303,7 @@ function buildMediaSourceCard(state) { else field.append(selection, upload); hint.textContent = source.hint; hint.title = source.mode === LOCAL_PATH_MODE - ? `${source.hint} Editor previews also require local preview access to be enabled by the server owner.` + ? `${source.hint} Only videos inside ComfyUI's input, output or temp folder can be read.` : source.hint; }; const chooseMode = (mode) => { diff --git a/nodes/_image_save_helpers.py b/nodes/_image_save_helpers.py index c94d600..16632d8 100644 --- a/nodes/_image_save_helpers.py +++ b/nodes/_image_save_helpers.py @@ -78,7 +78,7 @@ def sanitize_exact_name(name: str) -> str: text = str(name or "").strip().replace("\\", "/") if not text: return "" - if PureWindowsPath(text).drive or text.startswith("/"): + if ":" in text or PureWindowsPath(text).drive or text.startswith("/"): raise ValueError("Save Image: exact_name must be a relative name, not a rooted path.") parts = [part for part in text.split("/") if part not in ("", ".")] if any(part == ".." for part in parts): @@ -88,21 +88,48 @@ def sanitize_exact_name(name: str) -> str: return "/".join(parts) +OUTPUT_DIR_RULE = ( + "Save Image saves only inside ComfyUI's output folder: output_dir takes a " + "subfolder name such as sets/portraits, or stays empty for the output folder itself." +) + + def resolve_output_root(output_dir: str, default_root: str | Path) -> Path: - """'' -> the default root; relative -> under it; absolute -> as given.""" + """'' -> the output folder; otherwise a subfolder inside it. + + output_dir is a widget, and widget values arrive through ComfyUI's + unauthenticated /prompt route, so Save Image writes only inside the + output folder, like ComfyUI's own Save Image. Rooted paths, drive + letters, network paths, '~' and '..' are refused from the text alone, + before anything touches the disk; a subfolder that a link would carry + outside the output folder is refused as well. + """ text = str(output_dir or "").strip() default = Path(default_root) if not text: return default - candidate = Path(text).expanduser() - if candidate.is_absolute(): - return candidate - resolved = (default / candidate).resolve() - if default.resolve() not in resolved.parents and resolved != default.resolve(): - raise ValueError("Save Image: a relative output_dir may not escape the output folder.") + written = text.replace("\\", "/") + parts = [part for part in written.split("/") if part not in ("", ".")] + if written.startswith(("/", "~")) or ":" in written or PureWindowsPath(text).drive or ".." in parts: + raise ValueError(OUTPUT_DIR_RULE) + if not parts: + return default + root = default.resolve() + resolved = (root / "/".join(parts)).resolve() + if resolved != root and root not in resolved.parents: + raise ValueError(OUTPUT_DIR_RULE) return resolved +def require_output_path(path: Path, output_root: Path) -> Path: + """Check the final destination too: a filename or caption can be a symlink.""" + root = output_root.resolve() + resolved = path.resolve() + if root not in resolved.parents and resolved != root: + raise ValueError("Save destination must stay inside ComfyUI's output folder.") + return path + + def plan_exact_names(base: str, extension: str, count: int) -> list[str]: """One exact filename, or numbered siblings for a batch. @@ -279,6 +306,7 @@ __all__ = [ "IMAGE_EXTENSIONS", "IMAGE_FORMATS", "NAME_MODIFIERS", + "OUTPUT_DIR_RULE", "counter_pattern", "encode_image", "existing_action", @@ -289,6 +317,7 @@ __all__ = [ "next_free_counter", "plan_exact_names", "plan_local_names", + "require_output_path", "resolve_output_root", "sanitize_exact_name", "sidecar_path", diff --git a/nodes/_media_helpers.py b/nodes/_media_helpers.py index 9b9eb01..40ac9a7 100644 --- a/nodes/_media_helpers.py +++ b/nodes/_media_helpers.py @@ -22,13 +22,14 @@ except ImportError: # Offline tests import this module without ComfyUI. folder_paths = None -# Reading a video from an arbitrary local path is opt-in, for queued runs -# and the preview routes alike: a widget value arrives through the -# unauthenticated /prompt route and the previews answer any HTTP client -# that can reach the server, so by default local path mode only reaches -# ComfyUI's own input, output and temp folders. The server operator turns -# the rest of the disk on with this environment variable. -LOCAL_PREVIEW_ENV = "AUSBOSS_TRANSFORM_LOCAL_PREVIEW" +# Local path mode reads only from ComfyUI's input, output and temp folders, +# for queued runs and the preview routes alike: a widget value arrives +# through the unauthenticated /prompt route, and the previews answer any +# HTTP client that can reach the server. +LOCAL_PATH_RULE = ( + "Local path mode reads only videos inside ComfyUI's input, output or temp " + "folder. Move the video there, or use Upload." +) VIDEO_EXTENSIONS = { ".avi", @@ -76,6 +77,8 @@ def list_input_videos() -> list[str]: def resolve_input_path(selection: str) -> Path: if not selection: raise ValueError("Select or upload a source file first.") + if str(selection).replace("\\", "/").startswith("//"): + raise ValueError("Network source paths are not allowed.") if folder_paths is None: path = Path(selection).expanduser().resolve() else: @@ -96,10 +99,7 @@ def resolve_video_path(source_mode: str, video: str, local_path: str) -> Path: if not text: raise ValueError("Local path mode requires a video path.") if not local_preview_allowed(text): - raise ValueError( - "Local path mode reads only ComfyUI's input, output and temp folders " - f"unless ComfyUI is started with {LOCAL_PREVIEW_ENV}=1." - ) + raise ValueError(LOCAL_PATH_RULE) path = Path(text).expanduser().resolve() if not path.is_file(): raise ValueError("The local video file does not exist.") @@ -125,13 +125,14 @@ def _comfy_managed_roots() -> list[Path]: def local_preview_allowed(candidate: str) -> bool: - """A local path may be read when the operator opted in via the environment - flag, or when the path is already inside a ComfyUI-managed folder - (input/output/temp) that core routes serve anyway.""" - if os.environ.get(LOCAL_PREVIEW_ENV, "").strip().lower() in {"1", "true", "yes", "on"}: - return True + """A local path may be read only inside ComfyUI's input, output and temp + folders, the folders core routes serve anyway. A network path is refused + from its text alone, before anything could contact its host.""" + text = str(candidate or "").strip().strip('"') + if not text or text.replace("\\", "/").startswith("//"): + return False try: - resolved = Path(str(candidate or "").strip().strip('"')).expanduser().resolve() + resolved = Path(text).expanduser().resolve() except (OSError, ValueError): return False return any(resolved == root or root in resolved.parents for root in _comfy_managed_roots()) @@ -625,10 +626,7 @@ def register_video_routes() -> None: # Gate before touching the filesystem so a denied request can not be # used to probe which paths exist. if source_mode == "local path" and not local_preview_allowed(local_path): - raise ValueError( - "Local path mode reads only ComfyUI's input, output and temp folders " - f"unless ComfyUI is started with {LOCAL_PREVIEW_ENV}=1." - ) + raise ValueError(LOCAL_PATH_RULE) return resolve_video_path(source_mode, request.query.get("video", ""), local_path) # Decodes run in worker threads: a blocking decode inside these async diff --git a/nodes/node_save_image.py b/nodes/node_save_image.py index 2aae933..f9fe27c 100644 --- a/nodes/node_save_image.py +++ b/nodes/node_save_image.py @@ -17,6 +17,7 @@ from ._image_save_helpers import ( plan_exact_names, plan_local_names, resolve_output_root, + require_output_path, sanitize_exact_name, sidecar_path, split_prefix, @@ -49,7 +50,7 @@ def _register_folder_route() -> None: root = Path(folder_paths.get_output_directory()).resolve() relative = str(request.rel_url.query.get("path", "")).strip().replace("\\", "/") try: - target = (root / relative).resolve() if relative else root + target = resolve_output_root(relative, root) except Exception: return web.json_response({"error": "bad path"}, status=400) if target != root and root not in target.parents: @@ -58,7 +59,7 @@ def _register_folder_route() -> None: return web.json_response({"error": "not a folder"}, status=404) folders = sorted( entry.name for entry in target.iterdir() - if entry.is_dir() and not entry.name.startswith(".") + if not entry.name.startswith(".") and not entry.is_symlink() and entry.is_dir() ) shown = "" if target == root else target.relative_to(root).as_posix() return web.json_response({"path": shown, "folders": folders}) @@ -166,10 +167,10 @@ class AusBossSaveImage: { "default": "", "tooltip": ( - "Where to save. Empty is ComfyUI's output folder; " - "a relative path is a subfolder of it; an absolute " - "path saves anywhere you can write. The node " - "preview only shows files inside the output folder." + "Subfolder of ComfyUI's output folder to save into, " + "such as sets/portraits; created if missing. Empty " + "saves to the output folder itself. Save Image never " + "writes outside the output folder." ), }, ), @@ -315,6 +316,7 @@ class AusBossSaveImage: else: subfolder, base = split_prefix(filename_prefix) folder = root / subfolder if subfolder else root + require_output_path(folder, output_root) stem = name_stem( base, {"date": bool(name_date), "time": bool(name_time), "size": bool(name_size)}, @@ -333,6 +335,12 @@ class AusBossSaveImage: policy = "overwrite" if counter else on_existing planned.extend((folder / name, policy) for name in names) + # Validate the whole batch before any write, including caption targets. + for path, _policy in planned: + require_output_path(path, output_root) + if caption_value.strip(): + require_output_path(sidecar_path(path), output_root) + saved: list[Path] = [] for (path, policy), frame in zip(planned, images): if existing_action(path.exists(), policy) == "skip": @@ -381,6 +389,14 @@ class AusBossSaveImage: sanitize_exact_name(value) except ValueError as exc: return str(exc).replace("exact_name", label) + # **_values carries every input, so core skips its own checks here: + # a folder outside the output folder is refused before the run. + output_dir = _values.get("output_dir") + if isinstance(output_dir, str) and output_dir.strip() and folder_paths is not None: + try: + resolve_output_root(output_dir, folder_paths.get_output_directory()) + except ValueError as exc: + return str(exc) return True diff --git a/nodes/node_save_video.py b/nodes/node_save_video.py index 8a9944c..f9ad0b4 100644 --- a/nodes/node_save_video.py +++ b/nodes/node_save_video.py @@ -4,6 +4,7 @@ from __future__ import annotations import asyncio from pathlib import Path +from ._image_save_helpers import require_output_path, sanitize_exact_name from ._video_save_helpers import ( VIDEO_FORMATS, @@ -215,6 +216,9 @@ class AusBossSaveVideo: # clip, not a playback trick, so every frame count downstream counts it. if pingpong: frames = pingpong_frames(frames) + filename_prefix = sanitize_exact_name(filename_prefix) + output_root = Path(folder_paths.get_output_directory()) + require_output_path(output_root / filename_prefix, output_root) full_output_folder, filename, counter, subfolder, filename_prefix = ( folder_paths.get_save_image_path( filename_prefix, @@ -226,6 +230,7 @@ class AusBossSaveVideo: extension = VIDEO_FORMATS[format][0] if format in VIDEO_FORMATS else "mp4" file = f"{filename}_{counter:05}_.{extension}" output_path = Path(full_output_folder) / file + require_output_path(output_path, output_root) width, height, frame_count = await asyncio.to_thread( encode_video, output_path, diff --git a/nodes/node_video_crop_rotate_pad.py b/nodes/node_video_crop_rotate_pad.py index 6f2fe86..7590090 100644 --- a/nodes/node_video_crop_rotate_pad.py +++ b/nodes/node_video_crop_rotate_pad.py @@ -40,16 +40,16 @@ class AusBossVideoCropRotatePad: ), "source_mode": ( ["input folder", "local path"], - {"default": "input folder", "tooltip": "Local path mode reads a file in place instead of copying it into the input folder."}, + {"default": "input folder", "tooltip": "Local path mode reads a video already inside ComfyUI's input, output or temp folder in place, without an upload copy."}, ), "local_path": ( "STRING", { "default": "", "tooltip": ( - "Absolute video path used only in local path mode. By default only " - "ComfyUI's input, output and temp folders are readable; start ComfyUI " - "with AUSBOSS_TRANSFORM_LOCAL_PREVIEW=1 to allow any path." + "Absolute path to a video inside ComfyUI's input, output or temp " + "folder, used only in local path mode and read in place without a " + "copy. Paths anywhere else are refused." ), }, ), diff --git a/nodes/node_video_crop_rotate_pad_clip.py b/nodes/node_video_crop_rotate_pad_clip.py index 1f1f095..177f476 100644 --- a/nodes/node_video_crop_rotate_pad_clip.py +++ b/nodes/node_video_crop_rotate_pad_clip.py @@ -93,16 +93,16 @@ class AusBossVideoCropRotatePadClip: ), "source_mode": ( ["input folder", "local path"], - {"default": "input folder", "tooltip": "Local path mode reads a file in place instead of copying it into the input folder."}, + {"default": "input folder", "tooltip": "Local path mode reads a video already inside ComfyUI's input, output or temp folder in place, without an upload copy."}, ), "local_path": ( "STRING", { "default": "", "tooltip": ( - "Absolute video path used only in local path mode. By default only " - "ComfyUI's input, output and temp folders are readable; start ComfyUI " - "with AUSBOSS_TRANSFORM_LOCAL_PREVIEW=1 to allow any path." + "Absolute path to a video inside ComfyUI's input, output or temp " + "folder, used only in local path mode and read in place without a " + "copy. Paths anywhere else are refused." ), }, ), diff --git a/tests/save_image_naming.test.mjs b/tests/save_image_naming.test.mjs index 6f8c1db..5bac633 100644 --- a/tests/save_image_naming.test.mjs +++ b/tests/save_image_naming.test.mjs @@ -30,7 +30,9 @@ test("previewName in exact and linked modes", () => { test("previewFolder joins the output folder, the folder field and the prefix's subfolders", () => { assert.equal(previewFolder({ output_dir: "", filename_prefix: "shot" }), "ComfyUI/output/"); assert.equal(previewFolder({ output_dir: "datasets/portraits", filename_prefix: "sets/shot" }), "ComfyUI/output/datasets/portraits/sets/"); - assert.equal(previewFolder({ output_dir: "/mnt/data/", filename_prefix: "shot" }), "/mnt/data/"); + for (const outside of ["/mnt/data/", "C:\\Pictures", "~/Pictures", "sets/../..", "\\\\host\\share"]) { + assert.equal(previewFolder({ output_dir: outside, filename_prefix: "shot" }), "Not saved: use a subfolder of ComfyUI/output"); + } assert.equal(previewFolder({ output_dir: "", filename_prefix: "sets/shot", filenameLinked: true }), "ComfyUI/output/"); }); diff --git a/tests/test_image_save_helpers.py b/tests/test_image_save_helpers.py index f1e1047..91f6020 100644 --- a/tests/test_image_save_helpers.py +++ b/tests/test_image_save_helpers.py @@ -52,6 +52,9 @@ class NamingTests(unittest.TestCase): sanitize_exact_name("/rooted/photo") with self.assertRaises(ValueError): sanitize_exact_name("C:\\rooted\\photo") + for name in ("sets/C:photo", "photo:stream", "sets/photo:stream"): + with self.subTest(name=name), self.assertRaises(ValueError): + sanitize_exact_name(name) def test_single_image_gets_exactly_the_name(self): self.assertEqual(plan_exact_names("photo123", "png", 1), ["photo123.png"]) @@ -74,10 +77,32 @@ class NamingTests(unittest.TestCase): default = Path(tmp) self.assertEqual(resolve_output_root("", default), default) self.assertEqual(resolve_output_root("sub/dir", default), (default / "sub/dir").resolve()) - absolute = Path(tmp) / "elsewhere" - self.assertEqual(resolve_output_root(str(absolute), default), absolute) - with self.assertRaises(ValueError): - resolve_output_root("../outside", default) + self.assertEqual(resolve_output_root("sub\\dir/", default), (default / "sub/dir").resolve()) + self.assertEqual(resolve_output_root("./", default), default) + + def test_output_dir_never_leaves_the_output_folder(self): + # output_dir is a widget, so it may only name a subfolder: anything + # rooted is refused from its text, before the disk is touched. + with tempfile.TemporaryDirectory() as tmp, tempfile.TemporaryDirectory() as elsewhere: + default = Path(tmp) + for refused in ( + elsewhere, + str(Path(tmp) / "inside_but_absolute"), + "/", + "~/Pictures", + r"C:\Users\me", + "C:relative", + r"\\host\share", + "//host/share", + "../outside", + "sets/../../outside", + ): + with self.subTest(output_dir=refused): + with self.assertRaisesRegex(ValueError, "only inside ComfyUI's output folder"): + resolve_output_root(refused, default) + (Path(tmp) / "link").symlink_to(elsewhere, target_is_directory=True) + with self.assertRaisesRegex(ValueError, "only inside ComfyUI's output folder"): + resolve_output_root("link/sets", default) def test_sidecar_shares_the_basename(self): self.assertEqual(sidecar_path(Path("/x/photo123.png")), Path("/x/photo123.txt")) @@ -181,11 +206,49 @@ class SaveImageNodeTests(unittest.TestCase): self.assertTrue((Path(tmp) / "clip_001.png").exists()) self.assertTrue((Path(tmp) / "clip_002.png").exists()) - def test_absolute_output_dir_saves_outside_and_skips_preview(self): + def test_an_absolute_output_dir_is_refused_and_nothing_is_written(self): with tempfile.TemporaryDirectory() as tmp, tempfile.TemporaryDirectory() as elsewhere: - result = self.run_node(tmp, exact_name="photo", output_dir=elsewhere) - self.assertEqual(Path(result["result"][0]).parent, Path(elsewhere)) - self.assertEqual(result["ui"]["images"], []) + with self.assertRaisesRegex(ValueError, "only inside ComfyUI's output folder"): + self.run_node(tmp, exact_name="photo", caption="x", output_dir=elsewhere) + self.assertEqual(list(Path(elsewhere).iterdir()), []) + + def test_a_subfolder_output_dir_saves_there_with_a_preview(self): + with tempfile.TemporaryDirectory() as tmp: + result = self.run_node(tmp, exact_name="photo", output_dir="sets/portraits") + self.assertEqual(Path(result["result"][0]).parent, (Path(tmp) / "sets" / "portraits").resolve()) + self.assertEqual(result["ui"]["images"][0]["subfolder"], "sets/portraits") + + def test_filename_and_caption_symlinks_cannot_escape(self): + for mode in ("exact_name", "filename", "filename_prefix", "image", "caption"): + with self.subTest(mode=mode), tempfile.TemporaryDirectory() as tmp, tempfile.TemporaryDirectory() as outside: + root, other = Path(tmp), Path(outside) + sentinel = other / "requirements.txt" + sentinel.write_text("unchanged") + values = {"caption": "replacement", "name_counter": False} + if mode in ("image", "caption"): + (root / ("photo.png" if mode == "image" else "photo.txt")).symlink_to(sentinel) + values["exact_name"] = "photo" + else: + (root / "link").symlink_to(other, target_is_directory=True) + values[mode] = "link/photo" + with self.assertRaisesRegex(ValueError, "inside ComfyUI's output folder"): + self.run_node(tmp, **values) + self.assertEqual(sentinel.read_text(), "unchanged") + self.assertEqual(sorted(p.name for p in other.iterdir()), ["requirements.txt"]) + self.assertFalse((root / "photo.png").is_file() and mode == "caption") + + def test_validation_refuses_an_absolute_output_dir_early(self): + class FakeFolderPaths: + @staticmethod + def get_output_directory(): + return "/nonexistent/comfy/output" + + node = node_save_image.AusBossSaveImage + with patch.object(node_save_image, "folder_paths", FakeFolderPaths): + message = node.VALIDATE_INPUTS(output_dir="/etc") + self.assertIsInstance(message, str) + self.assertIn("only inside ComfyUI's output folder", message) + self.assertIs(node.VALIDATE_INPUTS(output_dir="datasets/portraits"), True) def test_classic_mode_counters_and_never_overwrites(self): with tempfile.TemporaryDirectory() as tmp: diff --git a/tests/test_transform_engine.py b/tests/test_transform_engine.py index 4851c75..e50f87c 100644 --- a/tests/test_transform_engine.py +++ b/tests/test_transform_engine.py @@ -373,35 +373,42 @@ class VideoDecodeTests(unittest.TestCase): class LocalPreviewGateTests(unittest.TestCase): - def test_disabled_by_default_outside_managed_folders(self): + def test_nothing_outside_comfyuis_folders_is_readable(self): from nodes._media_helpers import local_preview_allowed - with unittest.mock.patch.dict(os.environ, {}, clear=False): - os.environ.pop("AUSBOSS_TRANSFORM_LOCAL_PREVIEW", None) - self.assertFalse(local_preview_allowed(str(Path.home() / "video.mp4"))) - self.assertFalse(local_preview_allowed("")) + self.assertFalse(local_preview_allowed(str(Path.home() / "video.mp4"))) + self.assertFalse(local_preview_allowed("")) + self.assertFalse(local_preview_allowed(r"\\host\share\video.mp4")) + self.assertFalse(local_preview_allowed("//host/share/video.mp4")) - def test_environment_flag_enables_previews(self): - from nodes._media_helpers import local_preview_allowed - - with unittest.mock.patch.dict( - os.environ, {"AUSBOSS_TRANSFORM_LOCAL_PREVIEW": "1"} - ): - self.assertTrue(local_preview_allowed(str(Path.home() / "video.mp4"))) - - def test_queued_local_path_reads_follow_the_same_gate(self): - from nodes._media_helpers import resolve_video_path + def test_comfyuis_folders_are_readable(self): + from nodes import _media_helpers with tempfile.TemporaryDirectory() as folder: - outside = Path(folder) / "clip.mp4" - outside.write_bytes(b"") - with unittest.mock.patch.dict(os.environ, {}, clear=False): - os.environ.pop("AUSBOSS_TRANSFORM_LOCAL_PREVIEW", None) - with self.assertRaisesRegex(ValueError, "AUSBOSS_TRANSFORM_LOCAL_PREVIEW"): - resolve_video_path("local path", "", str(outside)) - with unittest.mock.patch.dict(os.environ, {"AUSBOSS_TRANSFORM_LOCAL_PREVIEW": "1"}): - self.assertEqual(resolve_video_path("local path", "", str(outside)), outside.resolve()) + root = Path(folder).resolve() + with unittest.mock.patch.object(_media_helpers, "_comfy_managed_roots", lambda: [root]): + self.assertTrue(_media_helpers.local_preview_allowed(str(root / "sub" / "clip.mp4"))) + self.assertFalse(_media_helpers.local_preview_allowed(str(root.parent / "clip.mp4"))) + def test_the_retired_environment_switch_opens_nothing(self): + from nodes._media_helpers import local_preview_allowed + + with tempfile.TemporaryDirectory() as folder, unittest.mock.patch.dict( + os.environ, {"AUSBOSS_TRANSFORM_LOCAL_PREVIEW": "1"} + ): + self.assertFalse(local_preview_allowed(str(Path(folder) / "video.mp4"))) + + def test_queued_runs_and_previews_explain_the_rule(self): + from nodes import _media_helpers + + with tempfile.TemporaryDirectory() as folder: + clip = Path(folder) / "clip.mp4" + clip.write_bytes(b"") + with self.assertRaises(ValueError) as caught: + _media_helpers.resolve_video_path("local path", "", str(clip)) + self.assertIn("input, output or temp", str(caught.exception)) + # The preview routes pass it through: it names no server path. + self.assertEqual(_media_helpers._safe_route_error(caught.exception), str(caught.exception)) diff --git a/tests/test_video_clip_transform.py b/tests/test_video_clip_transform.py index 1365c1f..ef97b79 100644 --- a/tests/test_video_clip_transform.py +++ b/tests/test_video_clip_transform.py @@ -66,9 +66,12 @@ class VideoClipNodeTests(unittest.TestCase): cls._tmp = tempfile.TemporaryDirectory() cls.video = Path(cls._tmp.name) / "clip.mp4" write_test_video(cls.video, with_audio=True) - # The fixture lives outside ComfyUI's folders; local path mode only - # reaches it with the operator's opt-in. - cls._env = unittest.mock.patch.dict(os.environ, {"AUSBOSS_TRANSFORM_LOCAL_PREVIEW": "1"}) + # Local path mode reads only ComfyUI's input/output/temp folders; + # the fixture's temporary folder stands in for one. + from nodes import _media_helpers + + root = Path(cls._tmp.name).resolve() + cls._env = unittest.mock.patch.object(_media_helpers, "_comfy_managed_roots", lambda: [root]) cls._env.start() @classmethod diff --git a/tests/test_video_save_helpers.py b/tests/test_video_save_helpers.py index 699f2fc..b4a7246 100644 --- a/tests/test_video_save_helpers.py +++ b/tests/test_video_save_helpers.py @@ -299,6 +299,23 @@ class SaveVideoHelperTests(unittest.TestCase): self.assertEqual(result["result"], ("/tmp/video_00003_.mp4",)) self.assertTrue(Path(result["result"][0]).is_absolute()) + def test_symlinked_prefix_is_refused_before_core_creates_folders(self): + with tempfile.TemporaryDirectory() as tmp, tempfile.TemporaryDirectory() as outside: + (Path(tmp) / "link").symlink_to(outside, target_is_directory=True) + with ( + patch.object(node_save_video, "folder_paths") as folders, + patch.object(node_save_video, "encode_video") as encode, + ): + folders.get_output_directory.return_value = tmp + with self.assertRaisesRegex(ValueError, "inside ComfyUI's output folder"): + run_node(node_save_video.AusBossSaveVideo().save( + frames=gradient_batch(1, 16, 16), fps=8.0, + filename_prefix="link/new/video", crf=19, + )) + folders.get_save_image_path.assert_not_called() + encode.assert_not_called() + self.assertEqual(list(Path(outside).iterdir()), []) + def test_the_file_path_output_is_appended_and_the_node_stays_an_output(self): node = node_save_video.AusBossSaveVideo self.assertEqual(node.RETURN_TYPES, ("STRING",))