diff --git a/movie_editor/server.py b/movie_editor/server.py index 9423ad23..e146c7d4 100644 --- a/movie_editor/server.py +++ b/movie_editor/server.py @@ -424,7 +424,7 @@ def _extract_prev_scene_frame(prev_media: Optional[dict], scene_id: str) -> Opti import os import shutil import subprocess - if not prev_media or not prev_media.get("filename"): + if not isinstance(prev_media, dict) or not prev_media.get("filename"): return None if shutil.which("ffmpeg") is None: return None @@ -438,8 +438,8 @@ def _extract_prev_scene_frame(prev_media: Optional[dict], scene_id: str) -> Opti # is then re-checked against tempdir in case a crafted subfolder still escaped it. fn_in = os.path.basename(str(prev_media["filename"])) sub_in = str(prev_media.get("subfolder") or "").strip("/\\") - src = os.path.normpath(os.path.join(tempdir, sub_in, fn_in)) - if os.path.commonpath([src, tempdir]) != tempdir or not os.path.isfile(src): + src = os.path.join(tempdir, sub_in, fn_in) + if not folder_paths.is_within_directory(tempdir, src) or not os.path.isfile(src): return None out_fn = f"funpack_prevframe_{scene_id}.png" out_path = os.path.join(tempdir, out_fn) @@ -1848,8 +1848,13 @@ if web is not None and PromptServer is not None: sampler_inputs, _settings = pipeline_caps.apply_simple_mode(sampler_inputs, _settings) studio_inputs["studio_settings"] = json.dumps(_settings) prev_scene_media = body.get("prev_scene_media") + # `only_scene` (not target.scenes[0]) is the requesting scene: _solo() expands + # `target` to every scene sharing a generative unit, so two concurrent solo + # requests for different scenes in the same unit would otherwise scope their + # extraction to the same first-scene id and clobber each other's frame. + scene_scope = body.get("only_scene") or (target.scenes[0].id if target.scenes else "run") prev_scene_frame = ( - _extract_prev_scene_frame(prev_scene_media, target.scenes[0].id if target.scenes else "run") + _extract_prev_scene_frame(prev_scene_media, scene_scope) if prev_scene_media else None ) graph, report = builder.build(oi, models_cfg, { diff --git a/movie_editor/tests/test_server.py b/movie_editor/tests/test_server.py index a92bcd19..c7911bf0 100644 --- a/movie_editor/tests/test_server.py +++ b/movie_editor/tests/test_server.py @@ -14,6 +14,7 @@ from movie_editor.server import ( _build_render_filter, _clip_bytes_for_media, _clip_needs_trim, + _extract_prev_scene_frame, _has_graphics_export_content, _parse_has_scenes, _parse_prompt_variants, @@ -60,6 +61,66 @@ def test_list_temp_media_lists_media_newest_first(tmp_path, monkeypatch): clip = next(f for f in files if f["filename"] == "clip.webm") assert clip["subfolder"] == "previews" assert clip["kind"] == "video" +# ── _extract_prev_scene_frame: prevframe extraction, never a media-bin asset ────── + + +def _fake_folder_paths(tempdir, monkeypatch): + fake = type("FP", (), { + "get_temp_directory": staticmethod(lambda: str(tempdir)), + "is_within_directory": staticmethod( + lambda d, t: __import__("os").path.commonpath( + [__import__("os").path.realpath(d), __import__("os").path.realpath(t)] + ) == __import__("os").path.realpath(d)), + }) + monkeypatch.setitem(__import__("sys").modules, "folder_paths", fake) + + +def test_prev_scene_frame_rejects_a_non_dict_payload(tmp_path, monkeypatch): + """A malformed prev_scene_media must degrade to 'nothing to extract', not blow up the + whole generate request — it round-trips through client JSON and is never trusted.""" + _fake_folder_paths(tmp_path, monkeypatch) + assert _extract_prev_scene_frame("not-a-dict", "scene1") is None + assert _extract_prev_scene_frame(["also", "not"], "scene1") is None + assert _extract_prev_scene_frame({}, "scene1") is None + + +def test_prev_scene_frame_blocks_subfolder_traversal(tmp_path, monkeypatch): + (tmp_path / "real.mp4").write_bytes(b"x") + secret_dir = tmp_path.parent / "funpack_traversal_secret" + secret_dir.mkdir(exist_ok=True) + (secret_dir / "real.mp4").write_bytes(b"secret") + _fake_folder_paths(tmp_path, monkeypatch) + escaping = {"filename": "real.mp4", "subfolder": f"../{secret_dir.name}"} + assert _extract_prev_scene_frame(escaping, "scene1") is None + + +def test_prev_scene_frame_returns_none_without_ffmpeg(tmp_path, monkeypatch): + (tmp_path / "clip.mp4").write_bytes(b"x") + _fake_folder_paths(tmp_path, monkeypatch) + monkeypatch.setattr(__import__("shutil"), "which", lambda name: None) + assert _extract_prev_scene_frame({"filename": "clip.mp4"}, "scene1") is None + + +def test_prev_scene_frame_scopes_output_name_to_the_requesting_scene(tmp_path, monkeypatch): + """Regression: the output filename must key off the scene actually asking (`only_scene`), + not the first scene of whatever generative unit the server expanded it into — otherwise + two different scenes in one unit clobber each other's extracted frame.""" + (tmp_path / "clip.mp4").write_bytes(b"x") + _fake_folder_paths(tmp_path, monkeypatch) + monkeypatch.setattr(__import__("shutil"), "which", lambda name: "/usr/bin/ffmpeg") + + def fake_run(cmd, **kw): + out_path = cmd[-1] + with open(out_path, "wb") as f: + f.write(b"png") + return type("R", (), {"returncode": 0})() + monkeypatch.setattr(__import__("subprocess"), "run", fake_run) + + fn = _extract_prev_scene_frame({"filename": "clip.mp4"}, "scene_b") + assert fn == "funpack_prevframe_scene_b.png" + assert (tmp_path / fn).is_file() + + FRESH = "__funpack_fresh_prompt__"