Skip to content

Full audit fixes (2026-11-04) - #35

Draft
doubletwisted wants to merge 48 commits into
mainfrom
claude/full-audit-2026-11-04-hta0pw
Draft

doubletwisted wants to merge 48 commits into
mainfrom
claude/full-audit-2026-11-04-hta0pw

Conversation

@doubletwisted

@doubletwisted doubletwisted commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

The audit found host-file exposure, unsafe farm receipts, stale snapshots and media/editing errors. This PR corrects those paths while preserving existing project records, source files, success replies and manifest shapes.

  • Local-control checks protect host-file operations. Farm receipts enforce identity/path rules; studio markup escapes IDs, take numbers and transition tracks. Authored blob-marker impersonation is rejected.
  • Snapshot publication carries an atomic per-publication UUID, detects coarse/same-size replacement, fails on unreadable identities and coordinates marker handles with Windows replacement. Registration is atomic. Maintenance scans use private read generations; project backup reporting uses .spool/backups.
  • Copied media remains portable. Orientation, finite source rates, output-folder names, joined-file audio clocks and loader refresh/upload races are corrected. Existing media records remain readable.
  • Cloud observation batches settle before reporting failures; cancellation drains storage work. Reconciliation/collection share per-project locks within one collector. Local, farm and Cloud copies own unique partial files and preserve prior outputs on failure. Deadline grace excludes queue/preparation time; recovery lookup failures return retryable 503 without resubmission.
  • Save/capture preflight rejects covered malformed clip/take/workflow/recovery containers with 400 before services lookup or staging, including a later malformed recipe. Demo save/value errors match. Valid stale-revision behavior and genuine internal-error handling are preserved. Save-error wording also applies to permanent deletion.
  • Imports with many cuts use bounded exact-frame batches and balanced keyframe expressions, avoiding Windows command length and FFmpeg expression-depth failures. Short six-frame GOPs retain seeking behavior. Invalid cuts are rejected before encoding without sorting/deduplication. Requirements document the existing FFmpeg/ffprobe 5.1 minimum.
  • Launcher storage, globbing and process creation run off-loop with cancellation draining and protected startup reservations. Failed imports clean only owned destinations/new caches, including early path failures; uncertain/pre-existing caches are retained.
  • OTIO publication retries transient Windows sharing denials and owns its exclusive pending file. Comparison frames live in private temporary directories; returned sheets and explicit frame exports remain durable.
  • Cancellation preflight uses the read generation, avoiding writer-lock waits and checkpoint rewrites. Plan/delivery transactions retain their supplied connections. FFmpeg audio/export/cut diagnostics decode UTF-8 and preserve Unicode filenames under Windows legacy locales.
  • Effective credential status handles empty saved/environment values. Credential creation is safe, the registry action is commit-pinned, permanent deletion flushes drafts, encoded media URLs play/seek, and repeated OTIO exports revoke replaced blobs. Local demo serving returns complete 416 responses.

Refs #3, #36, #37, #38, #39, #40, #41, #42, #43, #44, #45. Head cf37734368c3de9a6d576a7222f26aa345adc107.

Validation:

Evidence Qualified source Result
Full Windows gate Current commit tree 109 commands / 658 Python tests enumerated, 8 skips; JS/format/lint/type checks pass
Real isolated ComfyUI CPU Current commit tree 19 image/video, collection, restart, import/export and cut checks
Real Chrome deletion replay e70d2fd 33 F-12 assertions, including save failure/retry and retained file bytes
Real Chrome deletion/download/lifecycle Current commit tree 151 assertions; three actual completed OTIO link-click downloads
Real Chrome studio/link/security/media 73d084e 187 assertions: 128 studio + 28 linked workflow + 31 dedicated checks
Demo API Current commit tree 109 Node checks with simulated replies

The current 151 Chrome checks rerun deletion, three actual OTIO downloads, failure handling, blob lifecycle and take cleanup after the export changes. Seven lifecycle checks overlap the historical 187 run; the separate historical 33-case F-12 replay also overlaps the current suite. Counts are not additive. Those browser results remain identified by their actual commits.

Nine OTIO publication tests include a real Windows reader denying rename; they also pass on embedded Python 3.12. Seven comparison tests verify actual FFmpeg/Pillow pixels, repeated/concurrent sheets and failure cleanup. Independent source review approves both. Denied cleanup may retain an owned temporary file.

Five real-process diagnostic tests and six cancellation tests pass. Independent reviews approve both. Native cp1252 HTTP reproduction changes from 500 to readable 400, with normal control 200; embedded Python 3.12 also preserves the error without reader-thread failures. This HTTP harness is separate from the 19 real ComfyUI CPU checks.

The 11 many-cut tests run real fractional-rate imports across batch boundaries, verify frames/colors/source hashes/GOP spacing and cleanup, and execute/decode the largest planned batch against a real 28,020-frame source. Structural Unicode-path cases plan 1,800 and 20,000 cuts with every command at or below 24,000 UTF-16 units including NUL; full imports with all posters at those counts were not run. A representative 10,000-frame shot retains 1,667 keyframes and prior seek performance. Actual qualification used FFmpeg 9.0.2; 5.1 runtime is not claimed.

Other focused checks include 12 isolated HTTP tests/52 malformed requests with project/file-hash preservation, 22 launcher tests (one POSIX skip), and 11 import-cleanup tests with native Windows junctions. Independent source/evidence reviews cover publication, collection, timing, request/demo validation, launcher/import cleanup and the many-cut corrections. External Deadline/Cloud behavior uses doubles, separately from the real ComfyUI CPU evidence. Portable embedded Python loads run_spool.bat --help; this is module resolution, not a full launcher run.

Remaining acceptance: real Deadline, paid Cloud, Mac/Desktop/portable startup, bare UNC share-root containment, native viewport/timeline cases beyond the named fixtures, and original unsafe-source before/fixed comparisons for F-12/F-23. #43 still covers valid-looking stale media staging and acceptance/freeze I/O failures. Other nested fields/routes are outside these structural guards. Mixed-version snapshot detection relies on replacement identity when old writers preserve the UUID. Collector locks do not establish cross-process receipt/ledger idempotence. Joined-file audio is retimed; studio playback/export still uses plain takes. CI on this head is not claimed; broader acceptance keeps this PR draft.

Initial audit/implementation: Claude Code. Follow-up implementation: GPT-6.1 Sol through Codex and DeepSeek V4.1 Flash through OpenCode and Pi in T3 Code.

claude added 30 commits November 4, 2026 19:44
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
…cans

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
python_embeded ships a ._pth file, which makes Python leave the current
folder off sys.path, so 'python -m spool' from the Spool folder failed
with 'No module named spool'. The script now adds the folder and runs the
package through runpy. Checked on Linux with python -I, which has the
same effect; not run on Windows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
describe() encoded straight to the cache name and reuses any file found
there, so a timeout, a stopped host or a full disk left a truncated proxy
that made every later import of that source fail. Encodes now write a
unique pending file and rename it into place.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
The list named each rendered segment as file '<path>' without escaping,
so a temporary folder such as C:/Users/O'Brien/... made ffmpeg open a
different path and every MP4 export failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
Delete from disk called projectEdit.change() without flushing, unlike
the other authored actions. change() accepts the reply when the draft did
not change during the request, so an edit left unsaved (for example after
a failed save) was replaced and lost, and an edit still saving made the
delete fail with a revision conflict.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
Collection copies every execution's primary into the clip folder, but the
folder inventory knew only the winning attempt's artifacts, so once the
attempt settled the folder scan imported the later copy as an external
take, against the documented rule. Every recorded execution's artifacts
are now known.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
Workspace.list, project_stores and lookup raised on the first folder that
could not be opened (a newer format, a damaged manifest), and the
collector, dispatcher and Cloud loop all start from project_stores, so
rendering and the project list stopped for every project. Such a folder
is now listed as unavailable and skipped. Its retries back off from 5 s
to an hour, because each attempt at a failing upgrade first wrote a full
backup of the snapshot.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
prune --confirm and reclaim --confirm kept their working copy at
<tmp>/spool-maintenance/<sha256(folder)> and trusted it whenever its
checkpoint matched the snapshot, so another user of the computer could
plant a database there for the run to publish into the project. Each
writable run now uses its own mkdtemp folder and removes it afterwards.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
create, open_folder and import_project read the project_folders setting
and wrote it back separately from worker threads, so concurrent requests
dropped each other's entries: the folder existed but its project was no
longer listed or reachable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
Registration no longer goes through set_setting, so the two tests that
used it as their seam stopped exercising it. They now patch
register_folder and keep checking the same things: a failed registration
removes the import, and only a durable, complete project is registered.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
contained() unquoted every relative path, although aiohttp has already
decoded route paths and stored names are plain. A file named a%41.mp4
resolved to aA.mp4 (folder takes could not play, Comfy outputs failed
collection), and a doubly encoded upload name could turn into a path,
putting an upload into a clip's output folder.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
url() joined a project path onto /spool/media/ unencoded, so a name with
'#', '?' or '%' (an imported 'Take #3.mp4' keeps its name) was cut at
that character and its poster, proxy and playback failed to load.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
Freezing accepted SaveImage and PreviewImage results for the Cloud
target, but Cloud collection accepts only video files, so such a render
was paid for, then retried as collection_pending every two seconds and
never arrived. It is now refused when queued.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
ffprobe failures escaped _probe as CalledProcessError, so importing a
damaged or non-media file answered 500 internal_error and left the
uploaded bytes in the project's input folder. They are now an
invalid_request (still a SubprocessError for existing handlers) and the
upload is deleted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
contained() and inside() compared against root + os.sep, which never
matches a root that already ends in one: a project folder at E:\ or
\\nas\projects\ refused every media path as outside it. Reasoned for
Windows; the same comparison is tested here with /.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
A missing --file or invalid JSON escaped as a traceback, while the
automation guide promises JSON errors on stderr and exit code 1 for
agents to read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
Each timeline export created a blob URL for its download link and never
revoked it, so every export kept its timeline JSON in memory until the
page closed. The link stays valid while shown and is released when a
later export replaces it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
The waveform PNG was named from the path alone, so audio replaced at the
same path (folder takes do this) kept showing the old waveform. The key
now matches the review proxy's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
__main__ and maintenance indexed parents[3] and the launcher parents[4]
to find ComfyUI and the portable build, so a standalone checkout at
C:/Spool failed to import and one at /opt/spool answered the Comfy
status route with IndexError. A missing parent now means not installed
under ComfyUI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
save_key wrote the key with the umask's permissions (usually 0644) and
only then narrowed the file to 0600, so another user of the computer
could open it in between.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
- deadline.md: in a project folder, submissions and receipts live under
  the project's .spool/deadline folder, not the project output.
- durable-names.md: the project.json manifest exists; it is no longer
  planned.
- architecture.md: a project folder publishes an upgrade only after every
  migration step succeeds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
Findings, reproduction, what was fixed, what needs an owner decision and
the final gate run. The audit folder stays out of the Comfy Registry
package with the other development material.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RPzGzyA5uQiMcYoZxdUdaE
@doubletwisted

Copy link
Copy Markdown
Owner Author

Tracked leftovers from the audit report:

Existing issues referenced: #17 (Mac Desktop and launcher acceptance), #11 (path import), #5 (project folders, snapshot write amplification).

The remaining needs-decision items (N-01, N-02, N-09) and unconfirmed U-03 are security-related and are not filed as public issues.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants