From d17ba48dea019697e17e39e2b9c613dfd5741176 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 19:44:28 +0000 Subject: [PATCH 01/48] Make the clip folder reveal tests independent of xdg-open The route refuses before validating the folder when no file manager is installed, so on a headless runner one test failed and the other passed for the wrong reason. Both now stub the file manager. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE --- tests/test_take_files.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_take_files.py b/tests/test_take_files.py index e7814c2..3e2042f 100644 --- a/tests/test_take_files.py +++ b/tests/test_take_files.py @@ -336,7 +336,8 @@ async def test_open_clip_folder_uses_stable_identity_and_supports_empty_shots(se saved = self.store.save(self.project["id"], self.project["revision"], document) empty_folder = self.store.root / saved["document"]["clips"][1]["outputFolder"] empty_folder.rmdir() - with patch("spool.system.show") as launch: + # The route asks for a file manager first; a headless runner has none. + with patch("spool.system.file_manager", return_value="file manager"), patch("spool.system.show") as launch: for clip_id, folder in (("shot", self.store.root / original_folder), ("empty", empty_folder)): result = await self.client.post( "/spool/api/projects/" + self.project["id"] + "/clip-folder/" + clip_id + "/reveal", json={} @@ -359,7 +360,8 @@ async def test_open_clip_folder_uses_stable_identity_and_supports_empty_shots(se async def test_open_clip_folder_rejects_unknown_clip_cross_origin_and_invalid_folder(self): path = "/spool/api/projects/" + self.project["id"] + "/clip-folder/shot/reveal" - with patch("spool.system.show") as launch: + # With a file manager present, each 400 below comes from the folder checks. + with patch("spool.system.file_manager", return_value="file manager"), patch("spool.system.show") as launch: self.assertEqual((await self.client.post(path.replace("/shot/", "/unknown/"), json={})).status, 400) self.assertEqual( (await self.client.post(path, headers={"Origin": "https://elsewhere.invalid"}, json={})).status, 403 From 4a3120a1fa1b970a88df49cc4a49152a8de26146 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 19:45:30 +0000 Subject: [PATCH 02/48] Keep comparisons and cut detection of outside footage on this computer The comparison sheet and the cut list are made from a take's original, but neither route applied the rule that downloading that original does. A client on another machine (or a DNS-rebound page) could set a take's original to any image or video on the Spool computer and read a frame of it back from the comparison sheet. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE --- CHANGELOG.md | 9 +++++++++ docs/guide/security.md | 3 ++- spool/routes.py | 22 +++++++++++++++++++--- tests/test_security.py | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0bb67e..3965606 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,15 @@ All notable changes to Spool are recorded here. Versions follow [semantic versioning](https://semver.org/). +## Unreleased + +### Fixes + +- Comparing takes and detecting cuts in a take whose original lies outside the + project and Comfy folders now answer only this computer, as downloading that + original already did. Another machine could otherwise read frames of any image + or video on the Spool computer. + ## 0.1.1 (2026-11-03) ### Directing tools diff --git a/docs/guide/security.md b/docs/guide/security.md index 64fc5a4..d0c3908 100644 --- a/docs/guide/security.md +++ b/docs/guide/security.md @@ -19,7 +19,8 @@ does not apply. - revealing a take or clip folder in the file manager - Comfy connection settings and the Cloud API key - starting ComfyUI -- serving original footage, or any footage stored outside the project and Comfy folders +- serving original footage, or any footage stored outside the project and Comfy folders, + and comparing takes or detecting cuts in that footage - preparing playback and exporting frames A request to them must come from a loopback address. It must also use a loopback diff --git a/spool/routes.py b/spool/routes.py index 9b0bac9..919fd6a 100644 --- a/spool/routes.py +++ b/spool/routes.py @@ -421,6 +421,10 @@ async def comparison(request): store, media = await asyncio.to_thread(services, request) data = await request.json() clip = await asyncio.to_thread(store.clip, request.match_info["project"], data["clipId"]) + chosen = data["takeIds"] if isinstance(data["takeIds"], list) else [] + # The sheet is served to the caller, so it reads only footage the caller may download. + records = [t["media"] for t in clip["takes"] if t["id"] in chosen and not t.get("placeholder")] + await asyncio.to_thread(guard_originals, request, store, media, records) fps = await asyncio.to_thread(store.frame_rate, request.match_info["project"]) return response(await asyncio.to_thread(media.comparison, clip, data["takeIds"], fps)) @@ -569,6 +573,19 @@ async def take_setup(request): ) return response(await prepare_inputs(capture, request, True) if capture else None) + def outside_footage(request, selected, project_media, path): + """Footage outside the project and Comfy folders is read only for this computer.""" + roots = [selected.root, *project_media.host_roots.values()] + if not any(inside(path, root) for root in roots): + local_control(request, "Use footage outside this project from the computer running Spool") + + def guard_originals(request, selected, project_media, records): + """Refuse a remote caller a result made from footage it could not download.""" + for metadata in records: + original = metadata.get("originalFile", metadata.get("file", "")) + if isinstance(original, str) and Path(original).is_absolute(): + outside_footage(request, selected, project_media, original) + def take_source(request): selected, project_media = services(request) clip = selected.clip(request.match_info["project"], request.match_info["clip"]) @@ -578,9 +595,7 @@ def take_source(request): if not take: raise NotFound("Take not found") path = project_media.original_source(take["media"])["path"] - roots = [selected.root, *project_media.host_roots.values()] - if not any(inside(path, root) for root in roots): - local_control(request, "Use footage outside this project from the computer running Spool") + outside_footage(request, selected, project_media, path) return path @routes.get("/spool/api/projects/{project}/take-file/{clip}/{take}") @@ -944,6 +959,7 @@ async def detect_clip_cuts(request): take = next((t for t in clip["takes"] if t["id"] == data["takeId"]), None) if not take or take.get("placeholder"): raise ValueError("Select a video take in this project") + await asyncio.to_thread(guard_originals, request, store, media, [take["media"]]) fps = await asyncio.to_thread(store.frame_rate, request.match_info["project"]) document = await asyncio.to_thread(store.document, request.match_info["project"]) size = document.get("resolution") or {"width": 960, "height": 540} diff --git a/tests/test_security.py b/tests/test_security.py index 85a2f8b..7a93dfb 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -131,6 +131,38 @@ async def test_take_downloads_outside_the_project_stay_on_this_computer(self): self.assertEqual(response.status, 200, await response.text()) self.assertEqual(await response.read(), b"project footage") + async def test_frames_of_footage_outside_the_project_stay_on_this_computer(self): + # A comparison sheet or cut list is made from the original, so it is read + # under the same rule as downloading that original. + with patch.object(Media, "comparison", return_value={"url": "", "takes": []}) as compare: + for take_ids in (["outside"], ["inside", "outside"]): + response = await self.client.post( + self.base + "/comparison", json={"clipId": "shot", "takeIds": take_ids}, headers=REBOUND + ) + self.assertEqual(response.status, 403, await response.text()) + compare.assert_not_called() + response = await self.client.post( + self.base + "/comparison", json={"clipId": "shot", "takeIds": ["inside"]}, headers=REBOUND + ) + self.assertEqual(response.status, 200, await response.text()) + response = await self.client.post( + self.base + "/comparison", json={"clipId": "shot", "takeIds": ["outside"]} + ) + self.assertEqual(response.status, 200, await response.text()) + with patch.object(Media, "find_cuts", return_value=[]) as cuts: + response = await self.client.post( + self.base + "/detect-cuts", json={"clipId": "shot", "takeId": "outside"}, headers=REBOUND + ) + self.assertEqual(response.status, 403, await response.text()) + cuts.assert_not_called() + for headers in (REBOUND, {}): + response = await self.client.post( + self.base + "/detect-cuts", + json={"clipId": "shot", "takeId": "inside" if headers else "outside"}, + headers=headers, + ) + self.assertEqual(response.status, 200, await response.text()) + async def test_reads_and_listed_comfy_address_remain_available(self): response = await self.client.get("/spool/api/projects", headers=ELSEWHERE) self.assertEqual(response.status, 200) From 3aeea821c3a8d97216c4995c603002d62de5e9ef Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 19:47:22 +0000 Subject: [PATCH 03/48] Send the CLI's requests to this computer directly when a proxy is set urllib routes 127.0.0.1 through http_proxy unless no_proxy names it, so agent commands failed on machines with a proxy for other traffic, or reached Spool from the proxy and lost local-only access. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE --- CHANGELOG.md | 2 ++ spool/cli.py | 14 +++++++++++++- tests/test_cli.py | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3965606..9e99eb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ All notable changes to Spool are recorded here. Versions follow project and Comfy folders now answer only this computer, as downloading that original already did. Another machine could otherwise read frames of any image or video on the Spool computer. +- `python -m spool` agent commands reach a Spool on this computer directly when + `http_proxy` is set for other traffic, instead of failing through the proxy. ## 0.1.1 (2026-11-03) diff --git a/spool/cli.py b/spool/cli.py index 97f7615..21ee453 100644 --- a/spool/cli.py +++ b/spool/cli.py @@ -11,6 +11,18 @@ from .wire import wire, unwire ACTIONS = {"status", "shot", "events", "focus", "command", "compare"} +LOOPBACK = {"localhost", "127.0.0.1", "::1"} + + +def urlopen(request, timeout): + """Reach a Spool on this computer directly, even when a proxy is set for other traffic. + + urllib sends loopback requests to `http_proxy` unless `no_proxy` names them, and a + proxy cannot reach this computer's Spool or would make the request look remote. + """ + if (urllib.parse.urlsplit(request.full_url).hostname or "").lower() in LOOPBACK: + return urllib.request.build_opener(urllib.request.ProxyHandler({})).open(request, timeout=timeout) + return urllib.request.urlopen(request, timeout=timeout) def run(): @@ -56,7 +68,7 @@ def run(): headers={"Content-Type": "application/json"}, ) try: - with urllib.request.urlopen(request, timeout=120) as response: + with urlopen(request, timeout=120) as response: value = json.load(response) if not args.wire_json: value = unwire(value) diff --git a/tests/test_cli.py b/tests/test_cli.py index 5b640c4..b32274a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,6 +1,9 @@ import contextlib +import http.server import io import json +import os +import threading import unittest import urllib.error from unittest.mock import patch @@ -13,7 +16,7 @@ def invoke(self, reply, *args): out, err = io.StringIO(), io.StringIO() with ( patch("sys.argv", ["spool", "status", "project", *args]), - patch("urllib.request.urlopen") as send, + patch("spool.cli.urlopen") as send, contextlib.redirect_stdout(out), contextlib.redirect_stderr(err), ): @@ -49,3 +52,34 @@ def test_http_network_and_non_json_failures(self): ): code, out, err = self.invoke(failure) self.assertEqual((code, out, json.loads(err)["code"]), (1, "", expected)) + + +class LoopbackTests(unittest.TestCase): + def test_a_proxy_for_other_traffic_does_not_capture_this_computer(self): + class Spool(http.server.BaseHTTPRequestHandler): + def do_GET(self): + body = b'{"ok":true}' + self.send_response(200) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args): + pass + + server = http.server.HTTPServer(("127.0.0.1", 0), Spool) + threading.Thread(target=server.serve_forever, daemon=True).start() + self.addCleanup(server.server_close) + self.addCleanup(server.shutdown) + url = "http://127.0.0.1:%d" % server.server_port + # Port 9 on loopback refuses: a request sent to this proxy would fail. + environment = {key: value for key, value in os.environ.items() if "proxy" not in key.lower()} + out = io.StringIO() + with ( + patch.dict(os.environ, {**environment, "http_proxy": "http://127.0.0.1:9"}, clear=True), + patch("sys.argv", ["spool", "status", "project", "--url", url]), + contextlib.redirect_stdout(out), + contextlib.redirect_stderr(io.StringIO()), + ): + self.assertEqual(run(), 0) + self.assertEqual(json.loads(out.getvalue()), {"ok": True}) From 29153b42019736ceb5240115c694ed8ad2d63d3b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 19:51:59 +0000 Subject: [PATCH 04/48] Make Find existing Deadline job work recover_submission read attempt and submission inside the command transaction, where apply()'s cancel branch makes both names local, so every call raised UnboundLocalError and an uncertain Deadline submission could not be resolved. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE --- CHANGELOG.md | 2 ++ spool/commands.py | 14 ++++++++------ tests/test_farm_protocol.py | 25 +++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e99eb0..6393a06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ All notable changes to Spool are recorded here. Versions follow or video on the Spool computer. - `python -m spool` agent commands reach a Spool on this computer directly when `http_proxy` is set for other traffic, instead of failing through the proxy. +- **Find existing Deadline job** works again. It failed on every call, so a + Deadline render whose submission was uncertain could not be resolved. ## 0.1.1 (2026-11-03) diff --git a/spool/commands.py b/spool/commands.py index 323e20a..f7b5cac 100644 --- a/spool/commands.py +++ b/spool/commands.py @@ -405,11 +405,13 @@ def execute(store, media, project_id, data): adapter.cancel(attempt, {"stage": "check", "submission": submission, "done": done}) ).get("delivery") if action == "recover_submission": - attempt = store.attempt(arguments["attemptId"]) - if attempt["project_id"] != project_id or attempt["clip_id"] not in scope.get("clipIds", []): + # Named apart from apply()'s cancel locals: a name assigned anywhere in + # apply() is local to all of it, so reading `attempt` there would fail. + recovering = store.attempt(arguments["attemptId"]) + if recovering["project_id"] != project_id or recovering["clip_id"] not in scope.get("clipIds", []): raise ValueError("Submission is outside recovery scope") - submission = store.submission_for(attempt["id"]) - adopted = adapter_for(attempt["request"], submission).recover(attempt, {"submission": submission})["jobId"] + recovered = store.submission_for(recovering["id"]) + adopted = adapter_for(recovering["request"], recovered).recover(recovering, {"submission": recovered})["jobId"] def apply(db, document): clips = [*document["clips"], *document.get("audioClips", [])] if action in note_actions else document["clips"] @@ -524,9 +526,9 @@ def apply(db, document): return plan["result"], False elif action == "recover_submission": if adopted: - store.adopt_submission_job(db, submission["id"], adopted) + store.adopt_submission_job(db, recovered["id"], adopted) return { - "attemptId": attempt["id"], + "attemptId": recovering["id"], "jobId": adopted, "status": "adopted" if adopted else "submission_uncertain", "note": "No automatic resubmission", diff --git a/tests/test_farm_protocol.py b/tests/test_farm_protocol.py index 9bb988d..0095c16 100644 --- a/tests/test_farm_protocol.py +++ b/tests/test_farm_protocol.py @@ -319,6 +319,31 @@ def test_unavailable_progress_does_not_invent_a_percentage_or_fail_the_job(self) self.assertIsNone(job["farmProgress"]["percent"]) self.assertEqual(job["status"], "farm_running") + def test_find_existing_deadline_job_adopts_it_or_reports_still_uncertain(self): + from spool import commands + + attempt = self.farm_attempts[0] + package = self.package(attempt) + project = self.store.get(self.project["id"]) + envelope = { + "revision": project["revision"], + "actor": "agent:test", + "scope": {"projectId": project["id"], "clipIds": ["shot"], "capabilities": ["recover"]}, + "action": "recover_submission", + "arguments": {"attemptId": attempt["id"]}, + } + values = {"SpoolSubmissionId": package["submissionId"], "SpoolAttemptId": attempt["id"]} + for jobs, status in (([], "submission_uncertain"), (["a" * 24], "adopted")): + with ( + patch("spool.farm.submission_jobs", return_value=jobs), + patch("spool.farm.job_value", side_effect=lambda job, key: values[key]), + ): + result = commands.execute( + self.store, self.media, project["id"], {**envelope, "operationId": "recover-" + status} + ) + self.assertEqual((result["attemptId"], result["status"]), (attempt["id"], status)) + self.assertEqual(self.store.submission_for(attempt["id"])["external_id"], "a" * 24) + if __name__ == "__main__": unittest.main() From d452db39c1512268e2f84295fbe7303ae832daef Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 19:53:06 +0000 Subject: [PATCH 05/48] Reject Deadline receipts whose IDs could name paths outside the attempt Ingestion builds destination file names from a receipt's artifactId and executionId, but verify only checked that artifact IDs were unique. Anyone able to write the farm share could make collection copy a checksummed file of their choosing anywhere Spool may write. Both IDs must now be the uuid4 hex the worker always writes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE --- CHANGELOG.md | 3 +++ spool/farm_protocol.py | 11 ++++++++++- tests/test_farm_protocol.py | 17 +++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6393a06..b3545c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ All notable changes to Spool are recorded here. Versions follow `http_proxy` is set for other traffic, instead of failing through the proxy. - **Find existing Deadline job** works again. It failed on every call, so a Deadline render whose submission was uncertain could not be resolved. +- A Deadline receipt whose artifact or execution ID is not the ID Spool's worker + writes is rejected. A forged ID could make collection copy a file outside the + project. ## 0.1.1 (2026-11-03) diff --git a/spool/farm_protocol.py b/spool/farm_protocol.py index ae5e8f9..382898c 100644 --- a/spool/farm_protocol.py +++ b/spool/farm_protocol.py @@ -3,6 +3,7 @@ import copy import hashlib import json +import re import shutil import uuid from pathlib import Path @@ -18,6 +19,8 @@ atomic_json = _durable.atomic_json SCHEMA = 2 +# Execution and artifact IDs are uuid4().hex; they name the files Spool writes on ingestion. +IDENTITY = re.compile(r"[0-9a-f]{32}") SAVERS = { "SaveImage": {"images"}, "PreviewImage": {"images"}, @@ -152,13 +155,17 @@ def finalize(package, execution, prompt_id, history, roots, worker, job_id): return receipt +def identity(value): + return isinstance(value, str) and IDENTITY.fullmatch(value) is not None + + def verify(package, receipt, result_root): if receipt.get("schemaVersion") != SCHEMA: raise ValueError("Unsupported farm receipt version") for key in ("projectId", "clipId", "takeId", "attemptId", "fingerprint", "submissionId", "variationIndex"): if receipt.get(key) != package[key]: raise ValueError("Farm receipt belongs to another request: " + key) - if receipt.get("outcome") != "success" or not receipt.get("promptId") or not receipt.get("executionId"): + if receipt.get("outcome") != "success" or not receipt.get("promptId") or not identity(receipt.get("executionId")): raise ValueError("Farm receipt lacks execution identity or success") history = receipt.get("history", {}) if history.get("status", {}).get("status_str") != "success" or not history.get("status", {}).get("completed"): @@ -174,6 +181,8 @@ def verify(package, receipt, result_root): ): raise ValueError("Farm observed prompt differs from the permitted execution copy") artifacts = receipt["artifacts"] + if not all(identity(a["artifactId"]) for a in artifacts): + raise ValueError("Farm receipt has an invalid artifact identity") if len({a["artifactId"] for a in artifacts}) != len(artifacts): raise ValueError("Duplicate artifact identity") reported = { diff --git a/tests/test_farm_protocol.py b/tests/test_farm_protocol.py index 0095c16..32d6c42 100644 --- a/tests/test_farm_protocol.py +++ b/tests/test_farm_protocol.py @@ -91,6 +91,23 @@ def test_invalid_receipt_does_not_block_a_later_valid_execution(self): events = self.store.events(self.project["id"])["events"] self.assertEqual(sum(e["kind"] == "receipt.rejected" for e in events), 1) + def test_receipt_identities_cannot_name_files_outside_the_attempt(self): + for field, forged in (("artifactId", "../../../../escaped"), ("executionId", "../../../../escaped")): + with self.subTest(field=field): + attempt = self.farm_attempts[0 if field == "artifactId" else 1] + package, receipt = self.finish(attempt) + if field == "artifactId": + receipt["artifacts"][0]["artifactId"] = receipt["primaryArtifactId"] = forged + else: + receipt["executionId"] = forged + with self.assertRaises(ValueError): + protocol.verify(package, receipt, package["resultRoot"]) + folder = self.root / "farm" / "spool-receipts" / attempt["id"] + protocol.atomic_json(next(folder.glob("*/receipt.json")), receipt) + Collector(self.store, self.media).scan() + self.assertNotEqual(self.store.attempt(attempt["id"])["status"], "complete") + self.assertEqual(list(self.root.rglob("escaped*")), []) + def test_cancel_during_farm_observation_cannot_be_overwritten(self): attempt = self.farm_attempts[0] self.package(attempt) From 34e90ec69cfa555898d9bbaeb47fa164f932793a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 19:53:45 +0000 Subject: [PATCH 06/48] Record wrong-shape Deadline receipts as invalid instead of stopping scans A receipt or submission file that parsed as JSON but had the wrong shape raised TypeError or AttributeError past the per-receipt handler, so every scan aborted there: later valid receipts and later projects were never collected. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE --- CHANGELOG.md | 2 ++ spool/collection.py | 12 +++++++++++- spool/farm.py | 3 ++- tests/test_farm_protocol.py | 15 +++++++++++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3545c8..a090c0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ All notable changes to Spool are recorded here. Versions follow - A Deadline receipt whose artifact or execution ID is not the ID Spool's worker writes is rejected. A forged ID could make collection copy a file outside the project. +- A Deadline receipt that is valid JSON of the wrong shape is recorded as invalid + instead of stopping collection of every later receipt and project. ## 0.1.1 (2026-11-03) diff --git a/spool/collection.py b/spool/collection.py index 641ab55..4044290 100644 --- a/spool/collection.py +++ b/spool/collection.py @@ -73,7 +73,17 @@ def scan_project(self, store): if attempts.target_of(attempt["request"]) == attempts.DEADLINE: try: farm.collect(attempt) - except (ValueError, KeyError, OSError, RuntimeError, subprocess.SubprocessError) as error: + # Submission files on the farm share may hold valid JSON of the wrong shape. + except ( + ValueError, + KeyError, + OSError, + RuntimeError, + subprocess.SubprocessError, + TypeError, + AttributeError, + IndexError, + ) as error: store.transition( attempt["id"], attempts.COLLECTION_PENDING, error=str(error), expected=attempt["status"] ) diff --git a/spool/farm.py b/spool/farm.py index 0e7d254..77dea69 100644 --- a/spool/farm.py +++ b/spool/farm.py @@ -126,7 +126,8 @@ def transition(status, **kwargs): ): self.verified_receipts.pop(path, None) farm_protocol.verify(package, receipt, root) - except (ValueError, KeyError, OSError) as error: + # Valid JSON of the wrong shape fails like invalid JSON: the farm share is not trusted. + except (ValueError, KeyError, OSError, TypeError, AttributeError, IndexError) as error: self.verified_receipts.pop(path, None) self.store.reject_receipt(attempt["id"], str(path), str(error)) continue diff --git a/tests/test_farm_protocol.py b/tests/test_farm_protocol.py index 32d6c42..2d6cb1f 100644 --- a/tests/test_farm_protocol.py +++ b/tests/test_farm_protocol.py @@ -108,6 +108,21 @@ def test_receipt_identities_cannot_name_files_outside_the_attempt(self): self.assertNotEqual(self.store.attempt(attempt["id"])["status"], "complete") self.assertEqual(list(self.root.rglob("escaped*")), []) + def test_receipt_of_the_wrong_shape_does_not_block_a_later_valid_execution(self): + attempt = self.farm_attempts[0] + self.finish(attempt) + folder = self.root / "farm" / "spool-receipts" / attempt["id"] + for index, value in enumerate(([], {"artifacts": ["x"]}, {"schemaVersion": 2, "history": []})): + path = folder / ("000-shape-%d" % index) / "receipt.json" + path.parent.mkdir(parents=True) + protocol.atomic_json(path, value) + collector = Collector(self.store, self.media) + collector.scan() + collector.scan() + self.assertEqual(self.store.attempt(attempt["id"])["status"], "complete") + events = self.store.events(self.project["id"])["events"] + self.assertEqual(sum(e["kind"] == "receipt.rejected" for e in events), 3) + def test_cancel_during_farm_observation_cannot_be_overwritten(self): attempt = self.farm_attempts[0] self.package(attempt) From 541377dbbe7391b232bbbc4a8d2c76d324e18fd2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 19:55:03 +0000 Subject: [PATCH 07/48] Discard a crashed save's rollback journal with the old working copy Refreshing the local working copy deleted the database and its -wal and -shm files but not -journal. After a crash mid-transaction SQLite rolled that stale journal into the freshly copied snapshot, so the next save could publish a rolled-back project over another computer's work. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE --- CHANGELOG.md | 3 +++ spool/projects.py | 3 ++- tests/test_project_folders.py | 42 +++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a090c0a..73b9555 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,9 @@ All notable changes to Spool are recorded here. Versions follow project. - A Deadline receipt that is valid JSON of the wrong shape is recorded as invalid instead of stopping collection of every later receipt and project. +- Reopening a project after Spool stopped in the middle of a save restores the + last published snapshot. The interrupted save could be rolled into the fresh + copy and, on the next save, undo work published from another computer. ## 0.1.1 (2026-11-03) diff --git a/spool/projects.py b/spool/projects.py index a1ef946..339315f 100644 --- a/spool/projects.py +++ b/spool/projects.py @@ -112,7 +112,8 @@ def connect(self): except json.JSONDecodeError: saved = None if signature is None or not self.runtime.exists() or saved != signature: - for suffix in ("", "-wal", "-shm"): + # A crash leaves "-journal" beside the old copy; SQLite would roll it into the new one. + for suffix in ("", "-journal", "-wal", "-shm"): self.runtime.with_name(self.runtime.name + suffix).unlink(missing_ok=True) if signature is not None: shutil.copyfile(self.snapshot, self.runtime) diff --git a/tests/test_project_folders.py b/tests/test_project_folders.py index b6b73ec..cf3b358 100644 --- a/tests/test_project_folders.py +++ b/tests/test_project_folders.py @@ -7,6 +7,7 @@ import os import shutil import subprocess +import sys import tempfile import time import unittest @@ -31,6 +32,9 @@ from spool.store import Conflict, Store +ROOT = Path(__file__).resolve().parent.parent + + class ProjectFolderTests(unittest.TestCase): def setUp(self): self.temp = tempfile.TemporaryDirectory() @@ -1642,5 +1646,43 @@ def slow_storage(*args, **kwargs): asyncio.run(check()) +class CrashRecoveryTests(unittest.TestCase): + def test_a_crashed_save_cannot_roll_back_another_computers_publications(self): + temp = tempfile.TemporaryDirectory() + self.addCleanup(temp.cleanup) + root = Path(temp.name) + folder = root / "projects" / "Film" + here = Workspace(root / "here") + project_id = here.create({"project": "initial", "clips": [], "media": []}, folder)["id"] + # A writer on this computer dies inside a transaction, leaving a hot rollback journal. + crash = ( + "import os, sys\n" + "from spool.projects import Workspace\n" + "store = Workspace(sys.argv[1]).for_project(sys.argv[2])\n" + "with store.editing(sys.argv[2], 0) as edit:\n" + " edit.db.execute('PRAGMA cache_size=1')\n" + " edit.save({'project': 'uncommitted', 'clips': [], 'media': []})\n" + " edit.db.execute('CREATE TABLE junk(x)')\n" + " edit.db.executemany('INSERT INTO junk VALUES(?)', ((os.urandom(2000),) for _ in range(50)))\n" + " os._exit(9)\n" + ) + subprocess.run([sys.executable, "-c", crash, str(here.root), project_id], cwd=ROOT, check=False) + journal = here.for_project(project_id).runtime.with_name("working.sqlite3-journal") + self.assertTrue(journal.exists()) + # Another computer publishes newer work to the shared folder meanwhile. + there = Workspace(root / "there").open_folder(folder) + other = Workspace(root / "there").for_project(project_id) + revision = there["revision"] + for index in range(3): + revision = other.save(project_id, revision, {"project": "there %d" % index, "clips": [], "media": []})[ + "revision" + ] + restarted = Workspace(root / "here").for_project(project_id) + self.assertEqual(restarted.get(project_id)["document"]["project"], "there 2") + saved = restarted.save(project_id, revision, {"project": "here", "clips": [], "media": []}) + self.assertEqual(saved["revision"], revision + 1) + self.assertEqual(Workspace(root / "third").open_folder(folder)["document"]["project"], "here") + + if __name__ == "__main__": unittest.main() From c7fd10a0892997c797033035250977a43592932d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 19:56:19 +0000 Subject: [PATCH 08/48] Unpack the stored document before a capture saves it Store.capture read the packed document with json.loads and saved it, so each Apply workflow or Use take wrapped every blob reference in one more blob. Without an ordinary save in between, reads eventually hit RecursionError and the project could no longer be edited. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE --- CHANGELOG.md | 3 +++ spool/store.py | 3 ++- tests/test_store_interface.py | 36 +++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73b9555..9511851 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ All notable changes to Spool are recorded here. Versions follow - Reopening a project after Spool stopped in the middle of a save restores the last published snapshot. The interrupted save could be rolled into the fresh copy and, on the next save, undo work published from another computer. +- Applying a workflow or using a take no longer stores another copy of the + project's workflows each time. Repeated without a save in between, it grew the + project until it could not be opened. ## 0.1.1 (2026-11-03) diff --git a/spool/store.py b/spool/store.py index ca53bdc..388617c 100644 --- a/spool/store.py +++ b/spool/store.py @@ -1229,7 +1229,8 @@ def capture( row = db.execute("SELECT document FROM projects WHERE id=?", (project_id,)).fetchone() if not row: raise NotFound("Project not found") - document = json.loads(row[0]) + # Unpacked as editing() reads it: this document is saved, and saving packs it again. + document = self._load(row[0], db) generated = ( db.execute( "SELECT clip_id,take_id,result FROM attempts WHERE project_id=? AND take_id=? AND status='complete'", diff --git a/tests/test_store_interface.py b/tests/test_store_interface.py index ed799a3..d138bda 100644 --- a/tests/test_store_interface.py +++ b/tests/test_store_interface.py @@ -62,5 +62,41 @@ def test_project_tables_stay_inside_the_storage_modules(self): self.assertIsNone(re.search(r"\._(write_document|event)\(", source), path.name) +class RepeatedCaptureTests(unittest.TestCase): + def test_repeated_workflow_assignment_does_not_wrap_stored_values_again(self): + temp = tempfile.TemporaryDirectory() + self.addCleanup(temp.cleanup) + store = Store(Path(temp.name)) + graph = {"nodes": [{"id": 1, "type": "LoadImage"}], "links": []} + project = store.create( + { + "project": "Captures", + "clips": [{"id": "shot", "name": "Shot", "in": 0, "out": 24, "takes": []}], + "media": [], + "importedWorkflows": [{"id": "imported", "name": "Imported", "workflow": graph}], + } + ) + capture = { + "workflow": {"nodes": []}, + "prompt": { + "save": {"class_type": "SaveImage", "inputs": {"images": ["image", 0], "filename_prefix": "p"}}, + "image": {"class_type": "EmptyImage", "inputs": {"width": 64}}, + }, + "bindings": [], + "result": {"nodeId": "save", "group": "images", "index": 0}, + } + + def blobs(): + with store.reading() as db: + return db.execute("SELECT count(*) FROM blobs").fetchone()[0] + + revision = store.capture(project["id"], project["revision"], "shot", capture)["revision"] + before = blobs() + for _ in range(5): + revision = store.capture(project["id"], revision, "shot", capture)["revision"] + self.assertEqual(blobs(), before) + self.assertEqual(store.get(project["id"])["document"]["importedWorkflows"][0]["workflow"], graph) + + if __name__ == "__main__": unittest.main() From 97d0d1e744ad25773ca99080349637473c463eb2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 19:57:05 +0000 Subject: [PATCH 09/48] Keep referenced previews whose names hold spaces or accents when pruning The reference scan split stored JSON into ASCII path tokens, so a preview named after footage with a space or a non-ASCII letter (which JSON stores escaped) never matched and prune --confirm deleted it while clips still used it. Whole decoded string values are now matched as well. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE --- CHANGELOG.md | 2 ++ spool/store.py | 24 +++++++++++++++++++++++- tests/test_native.py | 17 +++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9511851..8d86576 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ All notable changes to Spool are recorded here. Versions follow - Applying a workflow or using a take no longer stores another copy of the project's workflows each time. Repeated without a save in between, it grew the project until it could not be opened. +- `python -m spool prune --confirm` no longer removes previews the edit uses when + their file names contain spaces or accented letters. ## 0.1.1 (2026-11-03) diff --git a/spool/store.py b/spool/store.py index 388617c..f43224c 100644 --- a/spool/store.py +++ b/spool/store.py @@ -175,6 +175,21 @@ def forget_media_evidence(self, media_ids): ) +def json_strings(value): + """Every string in a decoded JSON value, keys included.""" + found, pending = [], [value] + while pending: + item = pending.pop() + if isinstance(item, str): + found.append(item) + elif isinstance(item, dict): + found.extend(k for k in item if isinstance(k, str)) + pending.extend(item.values()) + elif isinstance(item, list): + pending.extend(item) + return found + + class Store: def __init__(self, root, migrate=True, filename="spool.sqlite3"): self.focus_store = self @@ -654,7 +669,14 @@ def referenced_names(self): except sqlite3.OperationalError: continue for row in rows: - for token in re.findall(r"[A-Za-z0-9_\-./\\]+", row[0]): + tokens = re.findall(r"[A-Za-z0-9_\-./\\]+", row[0]) + try: + # Whole string values too: file names may hold spaces, and JSON + # escapes non-ASCII characters, which the tokens alone would miss. + tokens += json_strings(json.loads(row[0])) + except (TypeError, ValueError): + pass + for token in tokens: names.add(token) if "/" in token or "\\" in token: names.add(token.replace("\\", "/").rsplit("/", 1)[-1]) diff --git a/tests/test_native.py b/tests/test_native.py index e55c0e6..4f3417b 100644 --- a/tests/test_native.py +++ b/tests/test_native.py @@ -346,6 +346,23 @@ def test_prune_reports_and_only_evicts_unreferenced_previews(self): self.assertEqual(self.store.prune(confirm=True, budget=1, grace=600)["evicted"], []) self.assertTrue(fresh.is_file()) + def test_prune_keeps_referenced_previews_named_with_spaces_or_accents(self): + previews = self.store.root / "previews" + previews.mkdir(exist_ok=True) + kept = [previews / "Desert walk-abc.mp4", previews / "café-abc.mp4"] + orphan = previews / "Desert walk-orphan.mp4" + for path in (*kept, orphan): + path.write_bytes(b"preview" * 100) + document = store_module.empty_project() + document["media"] = [ + {"id": "media-%d" % i, "kind": "video", "name": p.name, "file": "previews/" + p.name, "frames": 24} + for i, p in enumerate(kept) + ] + self.store.create(document) + evicted = self.store.prune(confirm=True, budget=1, grace=0) + self.assertEqual(evicted["evicted"], ["previews/Desert walk-orphan.mp4"]) + self.assertTrue(all(path.is_file() for path in kept)) + def test_prune_evicts_disposable_cut_scores_but_preserves_source(self): scores = self.store.root / "previews" / "cut-scores" / "scan.json" scores.parent.mkdir(parents=True) From 0899184cf6dd4955a2c08173a54265966d83ebb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 20:00:33 +0000 Subject: [PATCH 10/48] Escape project IDs and numbers where the studio builds HTML Clip, take and media IDs, workflow keys, take numbers and track numbers were joined into innerHTML raw while names beside them were escaped. The backend keeps them as written, so an imported project or a shared project folder ran script on open, with the studio's (and inside ComfyUI, Comfy's) origin. A new gate check fails when one of these fields reaches markup without esc(). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE --- CHANGELOG.md | 3 ++ tests/html_escape_checks.mjs | 89 ++++++++++++++++++++++++++++++++++++ ui/action-menu.js | 2 +- ui/audio-editor.js | 14 +++--- ui/library-renderer.js | 6 +-- ui/take-renderer.js | 8 ++-- ui/timeline-renderer.js | 28 ++++++------ ui/viewer.js | 6 +-- ui/workflow-coordinator.js | 7 ++- 9 files changed, 130 insertions(+), 33 deletions(-) create mode 100644 tests/html_escape_checks.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d86576..877e8e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,9 @@ All notable changes to Spool are recorded here. Versions follow project until it could not be opened. - `python -m spool prune --confirm` no longer removes previews the edit uses when their file names contain spaces or accented letters. +- Opening a project no longer runs script hidden in its clip, take or media IDs, + take numbers or track numbers. The studio now escapes them like names, so an + imported project or shared project folder cannot act as the studio. ## 0.1.1 (2026-11-03) diff --git a/tests/html_escape_checks.mjs b/tests/html_escape_checks.mjs new file mode 100644 index 0000000..0cfe3f1 --- /dev/null +++ b/tests/html_escape_checks.mjs @@ -0,0 +1,89 @@ +// Identity and number fields from a project reach HTML only through esc(). +// +// A project is data from whoever made it: an imported export or a shared project +// folder keeps clip, take and media IDs, take numbers and track numbers exactly as +// written. Joined raw into markup they ran script when the project was opened. +import assert from 'node:assert/strict'; +import { readdirSync, readFileSync } from 'node:fs'; +import { Linter } from 'eslint'; + +const FIELDS = new Set(['id', 'number', 'workflow', 'videoTrack', 'sourceLane']); +const CALLS = new Set(['videoTrack', 'sourceLane', 'takeName']); +const markup = node => + (node.type === 'Literal' && typeof node.value === 'string' && /[<>]|="/.test(node.value)) || + (node.type === 'TemplateLiteral' && node.quasis.some(q => /[<>]|="/.test(q.value.raw))); +const parts = (node, out = []) => { + if (node.type === 'BinaryExpression' && node.operator === '+') { + parts(node.left, out); + parts(node.right, out); + } else out.push(node); + return out; +}; +const name = node => + node.type === 'Identifier' ? node.name : node.type === 'MemberExpression' ? node.property.name : ''; +function raw(node) { + switch (node.type) { + case 'MemberExpression': + return !node.computed && FIELDS.has(node.property.name); + case 'CallExpression': { + const callee = name(node.callee); + if (CALLS.has(callee)) return true; + if (callee === 'String') return node.arguments.some(raw); + if (node.callee.type === 'MemberExpression' && ['padStart', 'toString'].includes(callee)) + return raw(node.callee.object); + return false; + } + case 'ConditionalExpression': + return raw(node.consequent) || raw(node.alternate); + case 'LogicalExpression': + return raw(node.left) || raw(node.right); + case 'BinaryExpression': + return node.operator === '+' && parts(node).some(raw); + case 'TemplateLiteral': + return node.expressions.some(raw); + default: + return false; + } +} +const rule = { + create(context) { + return { + BinaryExpression(node) { + if (node.operator !== '+' || (node.parent.type === 'BinaryExpression' && node.parent.operator === '+')) return; + const all = parts(node); + if (!all.some(markup)) return; + for (const part of all) if (raw(part)) context.report({ node: part, message: 'Escape with esc()' }); + }, + TemplateLiteral(node) { + if (!markup(node)) return; + for (const part of node.expressions) + if (raw(part)) context.report({ node: part, message: 'Escape with esc()' }); + }, + }; + }, +}; + +const linter = new Linter(); +const config = { + languageOptions: { ecmaVersion: 'latest', sourceType: 'module' }, + plugins: { spool: { rules: { 'escape-fields': rule } } }, + rules: { 'spool/escape-fields': 'error' }, +}; +const check = source => linter.verify(source, config).map(m => m.line + ':' + m.column + ' ' + m.message); + +// The check itself finds the shapes that were unescaped. +assert.equal(check(`h.innerHTML = '
';`).length, 1); +assert.equal(check(`h.innerHTML = '' + 'T' + String(t.number).padStart(2, '0') + '';`).length, 1); +assert.equal(check(`h.innerHTML = '';`).length, 1); +assert.equal(check(`h.innerHTML = '

' + (x ? takeName(t) : 'None') + '

';`).length, 1); +assert.equal(check('h.innerHTML = `

${m.id}

`;').length, 1); +assert.deepEqual(check(`h.innerHTML = '
' + (a.id === b.id ? ' on' : '');`), []); +assert.deepEqual(check(`label.textContent = 'Take ' + t.number;`), []); + +const problems = []; +for (const folder of ['ui', 'web']) + for (const file of readdirSync(folder).filter(f => f.endsWith('.js'))) + for (const problem of check(readFileSync(folder + '/' + file, 'utf8'))) + problems.push(folder + '/' + file + ':' + problem); +assert.deepEqual(problems, [], 'Unescaped project fields in HTML:\n' + problems.join('\n')); +console.log('html escape checks passed'); diff --git a/ui/action-menu.js b/ui/action-menu.js index 60ff911..1fe6002 100644 --- a/ui/action-menu.js +++ b/ui/action-menu.js @@ -14,7 +14,7 @@ export function createActionMenu({ $, esc, menuParent }) { if (menuOrigin?.hasAttribute('aria-expanded')) menuOrigin.setAttribute('aria-expanded', 'false'); const origin = menuOrigin?.isConnected ? menuOrigin - : $('clipLane').querySelector('[data-clip="' + menuClipId + '"]'); + : $('clipLane').querySelector('[data-clip="' + CSS.escape(menuClipId) + '"]'); menuClipId = null; menuOrigin = null; if (returnFocus) origin?.focus({ preventScroll: true }); diff --git a/ui/audio-editor.js b/ui/audio-editor.js index 95827d1..958c027 100644 --- a/ui/audio-editor.js +++ b/ui/audio-editor.js @@ -103,11 +103,11 @@ export function createAudioEditor({ '
' + seconds(c.out - c.in) + ' · ' + - (bed ? 'at ' + tc(c.start) : takeName(take(c))) + + (bed ? 'at ' + tc(c.start) : esc(takeName(take(c)))) + '

' + (m.audio ? '
'; return ( '
' + - (trial - ? 'T' + String(trial.number).padStart(2, '0') + ' trial' - : t.placeholder - ? 'No take' - : 'T' + String(t.number).padStart(2, '0')) + + esc( + trial + ? 'T' + String(trial.number).padStart(2, '0') + ' trial' + : t.placeholder + ? 'No take' + : 'T' + String(t.number).padStart(2, '0'), + ) + '' + @@ -387,9 +389,9 @@ export function createTimelineRenderer({ '" style="left:' + row.end * getPixelScale() + 'px;top:calc(var(--video-top-' + - T.videoTrack(parent) + + esc(T.videoTrack(parent)) + ') + var(--video-height-' + - T.videoTrack(parent) + + esc(T.videoTrack(parent)) + ') / 2)" title="' + esc(getState().clips.find(c => c.id === id)?.name + ' extends ' + parent.name) + '" aria-hidden="true">' + diff --git a/ui/viewer.js b/ui/viewer.js index f13ff49..75e2eb9 100644 --- a/ui/viewer.js +++ b/ui/viewer.js @@ -151,7 +151,7 @@ export function createViewer({ ? '' + esc(c.name) + ' · ' + - takeName(t) + + esc(takeName(t)) + ' · ' + (t.id === c.takeId ? 'in edit' @@ -159,7 +159,7 @@ export function createViewer({ : 'Media · ' + esc(review.source?.media.name || '') : trial ? 'Trying ' + - takeName(trial) + + esc(takeName(trial)) + ' in ' + esc(tried.name) + ' · not saved until you use it' @@ -234,7 +234,7 @@ export function createViewer({ '' + - (d.take.placeholder ? 'No take in edit' : takeName(d.take)) + + (d.take.placeholder ? 'No take in edit' : esc(takeName(d.take))) + (trial ? ' · trial' : '') + '', ); diff --git a/ui/workflow-coordinator.js b/ui/workflow-coordinator.js index 8046c1c..0ead7a6 100644 --- a/ui/workflow-coordinator.js +++ b/ui/workflow-coordinator.js @@ -196,7 +196,10 @@ export function createWorkflowCoordinator({ 'Add shot', '

Workflows

' + Object.entries(workflows) - .map(([id, w]) => '') + .map( + ([id, w]) => + '', + ) .join('') + '