Alexopus
bc798df250
`path` and `filename` are free-form strings that can come from a workflow's JSON, not just a person typing into the local UI. Only the final path segment was ever sanitized (make_pathname/sanitize_filename), so a value like `../../../etc` or an absolute path could make ImageSaver.save_images() create directories and write the image/JSON sidecar outside ComfyUI's output directory (CWE-22/CWE-73). This is what caused v1.24.0 to be banned on the Comfy Registry (Comfy-Org/registry-backend#206). Add utils.resolve_within_output(), which joins path pieces onto folder_paths.output_directory, resolves symlinks, and rejects the result if it escapes that directory. Apply it everywhere ImageSaver.save_images() turns `path`/`filename` into a filesystem path: the output directory itself, the final image target, and the optional workflow-JSON sidecar. Nested relative subfolders (e.g. `portraits/2026-08-27`) still work; `..`, absolute paths, and symlink escapes are now rejected before anything is created. Covered by a new tests/ suite (tests/test_paths.py), since exercising nodes.py requires stubbing ComfyUI's runtime (see tests/conftest.py). Fixes alexopus/ComfyUI-Image-Saver#138
114 lines
3.5 KiB
Python
114 lines
3.5 KiB
Python
"""
|
|
Regression tests for the output-path/filename containment fix in nodes.py
|
|
(ImageSaver.save_images) and its underlying primitive, utils.resolve_within_output.
|
|
|
|
See conftest.py for how nodes.py/utils.py get imported without a full ComfyUI
|
|
install.
|
|
"""
|
|
import numpy as np
|
|
import pytest
|
|
|
|
import folder_paths
|
|
from image_saver_under_test.nodes import ImageSaver, Metadata
|
|
from image_saver_under_test.utils import resolve_within_output
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def output_dir(tmp_path, monkeypatch):
|
|
monkeypatch.setattr(folder_paths, "output_directory", str(tmp_path))
|
|
return tmp_path
|
|
|
|
|
|
class FakeTensor:
|
|
"""Duck-types the .cpu().numpy() calls ImageSaver.save_images makes on IMAGE tensors."""
|
|
|
|
def __init__(self, array):
|
|
self._array = array
|
|
|
|
def cpu(self):
|
|
return self
|
|
|
|
def numpy(self):
|
|
return self._array
|
|
|
|
|
|
def make_image(size=8):
|
|
return FakeTensor(np.random.rand(size, size, 3))
|
|
|
|
|
|
def make_metadata(**overrides):
|
|
defaults = dict(
|
|
modelname="", positive="", negative="", width=8, height=8, seed=0,
|
|
steps=1, cfg=1.0, sampler_name="euler", scheduler_name="normal",
|
|
denoise=1.0, clip_skip=0, custom="", additional_hashes="",
|
|
ckpt_path="", a111_params="test params", final_hashes="",
|
|
)
|
|
defaults.update(overrides)
|
|
return Metadata(**defaults)
|
|
|
|
|
|
def call_save_images(path, filename="test", extension="png", save_workflow_as_json=False):
|
|
return ImageSaver.save_images(
|
|
images=[make_image()],
|
|
filename_pattern=filename,
|
|
extension=extension,
|
|
path=path,
|
|
quality_jpeg_or_webp=100,
|
|
lossless_webp=True,
|
|
optimize_png=False,
|
|
prompt=None,
|
|
extra_pnginfo={"workflow": {"nodes": []}},
|
|
save_workflow_as_json=save_workflow_as_json,
|
|
embed_workflow=False,
|
|
counter=0,
|
|
time_format="%Y-%m-%d-%H%M%S",
|
|
metadata=make_metadata(),
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize("path", ["", "portraits", "portraits/2026-08-27", "a/b/c"])
|
|
def test_accepts_nested_paths_within_output_dir(path, output_dir):
|
|
call_save_images(path)
|
|
written = list(output_dir.rglob("*.png"))
|
|
assert len(written) == 1
|
|
assert output_dir in written[0].parents
|
|
|
|
|
|
@pytest.mark.parametrize("path", ["../escape", "../../escape", "/etc/escape"])
|
|
def test_rejects_traversal_via_path(path, output_dir):
|
|
with pytest.raises(ValueError):
|
|
call_save_images(path)
|
|
assert list(output_dir.rglob("*.png")) == []
|
|
|
|
|
|
def test_rejects_traversal_via_filename(output_dir):
|
|
with pytest.raises(ValueError):
|
|
call_save_images(path="safe", filename="../../../escape")
|
|
assert list(output_dir.rglob("*.png")) == []
|
|
|
|
|
|
def test_rejects_traversal_via_json_sidecar(output_dir):
|
|
with pytest.raises(ValueError):
|
|
call_save_images(path="safe", filename="../../../escape", save_workflow_as_json=True)
|
|
assert list(output_dir.rglob("*.json")) == []
|
|
|
|
|
|
def test_symlink_escape_is_rejected(output_dir):
|
|
outside = output_dir.parent / "outside"
|
|
outside.mkdir()
|
|
(output_dir / "linked").symlink_to(outside)
|
|
with pytest.raises(ValueError):
|
|
call_save_images(path="linked")
|
|
assert list(outside.iterdir()) == []
|
|
|
|
|
|
def test_resolve_within_output_accepts_nested_relative(output_dir):
|
|
resolved = resolve_within_output("a/b")
|
|
assert resolved == str((output_dir / "a" / "b").resolve())
|
|
|
|
|
|
@pytest.mark.parametrize("bad", ["../escape", "/etc/escape"])
|
|
def test_resolve_within_output_rejects_escape(bad, output_dir):
|
|
with pytest.raises(ValueError):
|
|
resolve_within_output(bad)
|