From 7103b8cc7522eae1f3ae362a9204dd99cbcdb8d1 Mon Sep 17 00:00:00 2001 From: adobeluo Date: Mon, 10 Aug 2026 14:07:03 +0800 Subject: [PATCH] fix(assets): queue failed source cleanup safely --- app/assets/services/asset_management.py | 15 ++++++++- .../services/test_asset_management.py | 33 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/app/assets/services/asset_management.py b/app/assets/services/asset_management.py index d508a4448..93cf6ba87 100644 --- a/app/assets/services/asset_management.py +++ b/app/assets/services/asset_management.py @@ -326,11 +326,24 @@ def delete_asset_reference_with_file( if staged_file_path: try: os.remove(staged_file_path) - except OSError: + except OSError as cleanup_error: logging.exception( "Failed to remove staged asset file after commit: %s", staged_file_path, ) + if not folder_paths.is_within_directory( + staging_directory, staged_file_path + ): + retry_path = os.path.join( + staging_directory, f".comfy-delete-{uuid.uuid4().hex}.tmp" + ) + try: + os.replace(staged_file_path, retry_path) + except OSError: + os.replace(staged_file_path, file_path) + raise RuntimeError( + "Cleanup could not be queued; the source file was restored." + ) from cleanup_error return True diff --git a/tests-unit/assets_test/services/test_asset_management.py b/tests-unit/assets_test/services/test_asset_management.py index 50dca936b..e2e64b3bf 100644 --- a/tests-unit/assets_test/services/test_asset_management.py +++ b/tests-unit/assets_test/services/test_asset_management.py @@ -359,6 +359,39 @@ class TestDeleteAssetReferenceWithFile: assert selected_file.read_bytes() == b"content" assert session.get(AssetReference, selected_ref_id) is not None + def test_queues_final_cleanup_failure_in_managed_temp( + self, mock_create_session, session: Session, temp_dir, monkeypatch + ): + selected_file = temp_dir / "managed" / "selected.bin" + selected_file.parent.mkdir() + selected_file.write_bytes(b"content") + staging_directory = temp_dir / "staging" + asset = _make_asset(session) + selected_ref = _make_reference(session, asset, name=selected_file.name) + selected_ref.file_path = str(selected_file) + selected_ref_id = selected_ref.id + session.commit() + + def fail_remove(_path): + raise PermissionError("file is busy") + + monkeypatch.setattr("app.assets.services.asset_management.os.remove", fail_remove) + result = delete_asset_reference_with_file( + reference_id=selected_ref_id, + owner_id="", + staging_directory=str(staging_directory), + expected_file_path=str(selected_file), + allowed_directories=[str(selected_file.parent)], + ) + + session.expire_all() + assert result is True + assert not selected_file.exists() + queued_files = list(staging_directory.glob(".comfy-delete-*.tmp")) + assert len(queued_files) == 1 + assert queued_files[0].read_bytes() == b"content" + assert session.get(AssetReference, selected_ref_id) is None + def test_rejects_a_source_path_changed_after_authorization( self, mock_create_session, session: Session, temp_dir ):