fix(frontend): clean up @latest frontend downloads (CORE-285)

[DrJKL]

PR Created by the Glary-Bot Agent

---

Summary

Fixes CORE-285: old ComfyUI_frontend releases pulled in via --front-end-version <repo>@latest accumulate in web_custom_versions/ and never get cleaned up. One user reported 5.11 GB of stale downloads.

Approach

Per Austin's suggestion in the thread: distinguish between auto-resolving specifiers (@latest, @prerelease) and explicit pins (@v1.46.0, @1.46.0). Auto-cleanup applies only to the former. No new CLI flag — design matches the thread consensus.

A small hidden metadata file .auto_managed.json is written next to the per-provider release folders (e.g. web_custom_versions/Comfy-Org_ComfyUI_frontend/.auto_managed.json). It records which concrete versions were materialized via an auto-resolving specifier. Whenever the user re-runs with @latest / @prerelease, any tracked-but-no-longer-current folder is shutil.rmtree'd before the metadata is rewritten.

Explicitly pinned versions are never touched. As a bonus, if a user later pins a version that had previously been downloaded under @latest, it is promoted out of the auto-managed set so it survives the next cleanup pass — useful for the bisecting workflow mentioned in the thread.

Changes

• app/frontend_management.py
  • AUTO_MANAGED_VERSION_SPECIFIERS = ("latest", "prerelease") + metadata filename constant.
  • Helpers: _read_auto_managed_versions, _write_auto_managed_versions (atomic temp file + os.replace), _prune_auto_managed_versions, _untrack_auto_managed_version, _is_safe_version_dirname.
  • In init_frontend_unsafe, after a successful download or hit-on-disk:
    • Auto-managed specifier → prune previously-tracked versions, record current one.
    • Explicit specifier → promote it out of the auto-managed set if present.
• tests-unit/app_test/frontend_manager_test.py
  • 15 new tests covering metadata roundtrip, missing / corrupt / non-dict / non-list metadata, unsafe-name filtering at both read and write, pruning, pinned-version-survives-cleanup, the "explicit pin promotes" path for both @v1.0.0 and @1.0.0 forms, and a path-containment defense-in-depth check.

Robustness (post-review hardening)

Oracle review surfaced two issues — both addressed:

1. Metadata shape: json.load() can return a non-dict root ([], null, "oops", 42). .get(...) would have crashed. Now we isinstance(data, dict) check before reading and ignore anything else.
2. Path-traversal hardening: a tampered / hand-edited .auto_managed.json could contain "../escape". Defense in depth:
   • Read time: every entry is run through _is_safe_version_dirname (allowlist regex, rejects .., /, \, NUL, etc.) before being returned.
   • Write time: same filter is applied before serializing, so the on-disk file never persists hostile names.
   • Cleanup time: _prune_auto_managed_versions resolves both the provider directory and each candidate path, then refuses to rmtree anything that isn't strictly under the resolved provider directory. Symlinks and path tricks can't escape.

Verification

• Unit tests: 33/33 pass in frontend_manager_test.py (18 pre-existing + 15 new). Pre-existing failures elsewhere in tests-unit/ are unrelated (user_manager, model_management, etc. — same count on master).
• End-to-end simulation: drove the real init_frontend_unsafe through a 5-step scenario (1st @latest → 2nd @latest → explicit pin → 3rd @latest → identical 4th @latest). Disk state after each step matched the design: stale auto-managed folders pruned, pinned folder preserved through subsequent cleanups.
• Static analysis (lsp_diagnostics): zero new errors introduced; pre-existing diagnostics unchanged.

Backward compatibility / migration

• Forward-only cleanup by design: users whose web_custom_versions/ is already cluttered won't have those existing folders retroactively cleaned (no metadata exists for them yet). The next @latest invocation starts tracking from that point forward and cleans up on subsequent runs of new @latest versions. Old cruft can be manually deleted once. A retroactive sweep would require guessing which existing folders were @latest vs explicit pins, which we can't reconstruct.
• Default behavior (no --front-end-version flag, using the pip-shipped frontend) is unchanged.

[DrJKL]

Way too many long comments.

[glary-bot]

Fair — trimmed in 0a2b9f8. Net -76 lines of prose across the two files:

• Killed all narrative/scenario comments inside test bodies and helper functions (they restated the test setup).
• Removed the multi-line block comments on AUTO_MANAGED_VERSION_SPECIFIERS and the regex constant — kept tight one-liners.
• Collapsed the init_frontend_unsafe "promote/prune" comments into nothing — the method names already say it.
• Shortened docstrings on _read/_prune/_untrack_auto_managed_versions to one line each.

Kept just five short comments that aren't restating code:

• AUTO_MANAGED_VERSION_SPECIFIERS constant (says why this tuple exists vs explicit pins)
• The regex (security: explains the allowlist purpose)
• The atomic-write step (non-obvious tmp+rename pattern)
• The defense-in-depth _is_safe_version_dirname re-check in _prune_auto_managed_versions (would otherwise look redundant and get "simplified" away)

33/33 tests still pass.

[glary-bot]

Cut further in 273314a — only one comment remains in the new code:

# Re-check even though read already filters: keeps rmtree provably
# bounded under provider_dir regardless of caller.
if not cls._is_safe_version_dirname(stale_version):
    ...

This one I'd push back on removing: the _is_safe_version_dirname re-check inside _prune_auto_managed_versions looks redundant (the read path already filters), and without a comment a future maintainer will simplify it away. That would re-open the path-traversal vector Oracle flagged in the first review pass. The test test_prune_refuses_to_delete_outside_provider_dir would still pass without the re-check (because it relies on the resolve-and-check below it), so the comment is the only thing protecting the layered defense.

Everything else — class-constant explanations, regex purpose, atomic-write idiom, all docstrings on the new private helpers — is gone. Happy to delete this last one too if you'd rather rename the helper or merge the check into _is_safe_version_dirname's docstring instead.

33/33 tests still pass.