This commit is contained in:
adobex 2026-08-15 20:51:43 +02:00 committed by GitHub
commit 9fe4f8de37
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 1303 additions and 7 deletions

View File

@ -9,6 +9,11 @@ ComfyUI is designed to run locally. By default, the server binds to `127.0.0.1`,
- Anyone with access to the ComfyUI URL is trusted (a direct consequence of the localhost-only default). - Anyone with access to the ComfyUI URL is trusted (a direct consequence of the localhost-only default).
- PyTorch and other dependencies are at the versions we ship or recommend in the README. - PyTorch and other dependencies are at the versions we ship or recommend in the README.
The asset `open-location` action trusts the direct transport peer's loopback
address and opens the host file manager. Do not forward that endpoint through a
same-host reverse proxy unless an additional access control restricts it to the
host operator.
A report is in scope only if it affects a user operating within this threat model. A report is in scope only if it affects a user operating within this threat model.
## What We Consider a Vulnerability ## What We Consider a Vulnerability

View File

@ -25,12 +25,14 @@ from app.assets.api.upload import (
) )
from app.assets.seeder import ScanInProgressError, asset_seeder from app.assets.seeder import ScanInProgressError, asset_seeder
from app.assets.services import ( from app.assets.services import (
AssetFileDeleteForbiddenError,
DependencyMissingError, DependencyMissingError,
HashMismatchError, HashMismatchError,
apply_tags, apply_tags,
asset_exists, asset_exists,
create_from_hash, create_from_hash,
delete_asset_reference, delete_asset_reference,
delete_asset_reference_with_file,
get_asset_detail, get_asset_detail,
list_assets_page, list_assets_page,
list_tags, list_tags,
@ -40,6 +42,10 @@ from app.assets.services import (
upload_from_temp_path, upload_from_temp_path,
) )
from app.assets.services.cursor import InvalidCursorError from app.assets.services.cursor import InvalidCursorError
from app.assets.services.file_location import (
is_loopback_address,
reveal_file_in_file_manager,
)
from app.assets.services.path_utils import compute_display_name from app.assets.services.path_utils import compute_display_name
from app.assets.services.tagging import list_tag_histogram from app.assets.services.tagging import list_tag_histogram
@ -636,20 +642,62 @@ async def update_asset_route(request: web.Request) -> web.Response:
@_require_assets_feature_enabled @_require_assets_feature_enabled
async def delete_asset_route(request: web.Request) -> web.Response: async def delete_asset_route(request: web.Request) -> web.Response:
reference_id = str(uuid.UUID(request.match_info["id"])) reference_id = str(uuid.UUID(request.match_info["id"]))
delete_content = request.query.get("delete_content") == "true"
owner_id = None
try: try:
# Deleting an asset is a soft delete of the reference; the underlying owner_id = USER_MANAGER.get_request_user_id(request)
# content is preserved (it may be shared with other references). if delete_content:
deleted = delete_asset_reference( detail = get_asset_detail(reference_id=reference_id, owner_id=owner_id)
reference_id=reference_id, if detail is None:
owner_id=USER_MANAGER.get_request_user_id(request), return _build_error_response(
delete_content_if_orphan=False, 404,
"ASSET_NOT_FOUND",
f"AssetReference {reference_id} not found.",
)
allowed_roots = []
if "input" in detail.tags:
allowed_roots.append(folder_paths.get_input_directory())
if "output" in detail.tags:
allowed_roots.append(folder_paths.get_output_directory())
if not detail.ref.file_path or not any(
folder_paths.is_within_directory(root, detail.ref.file_path)
for root in allowed_roots
):
return _build_error_response(
403,
"ASSET_DELETE_FORBIDDEN",
"Only tagged files inside the input or output directory can be deleted with their content.",
)
deleted = delete_asset_reference_with_file(
reference_id=reference_id,
owner_id=owner_id,
staging_directory=folder_paths.get_temp_directory(),
expected_file_path=detail.ref.file_path,
allowed_directories=allowed_roots,
allow_ownerless=not user_manager.args.multi_user,
)
else:
deleted = delete_asset_reference(
reference_id=reference_id,
owner_id=owner_id,
delete_content_if_orphan=False,
)
except AssetFileDeleteForbiddenError as error:
return _build_error_response(
403,
"ASSET_DELETE_FORBIDDEN",
str(error),
{"id": reference_id},
) )
except Exception: except Exception:
logging.exception( logging.exception(
"delete_asset_reference failed for reference_id=%s, owner_id=%s", "delete_asset_reference failed for reference_id=%s, owner_id=%s",
reference_id, reference_id,
USER_MANAGER.get_request_user_id(request), owner_id,
) )
return _build_error_response(500, "INTERNAL", "Unexpected server error.") return _build_error_response(500, "INTERNAL", "Unexpected server error.")
@ -660,6 +708,82 @@ async def delete_asset_route(request: web.Request) -> web.Response:
return web.Response(status=204) return web.Response(status=204)
@ROUTES.post(f"/api/assets/{{id:{UUID_RE}}}/open-location")
@_require_assets_feature_enabled
async def open_asset_location_route(request: web.Request) -> web.Response:
"""Open an output in the host file manager for a direct local client.
The transport peer is the trust boundary. Operators must not forward this
endpoint through a same-host reverse proxy without an additional access
control that restricts it to the host operator.
"""
reference_id = str(uuid.UUID(request.match_info["id"]))
if not is_loopback_address(request.remote):
return _build_error_response(
403,
"LOCAL_ACCESS_REQUIRED",
"Opening a file location is only available from the ComfyUI host machine.",
)
if request.headers.get("Sec-Fetch-Site") not in (None, "same-origin"):
return _build_error_response(
403,
"CROSS_SITE_REQUEST_FORBIDDEN",
"Cross-site requests cannot open local file locations.",
)
owner_id = None
try:
owner_id = USER_MANAGER.get_request_user_id(request)
detail = get_asset_detail(reference_id=reference_id, owner_id=owner_id)
except Exception:
logging.exception(
"get_asset_detail failed for reference_id=%s, owner_id=%s",
reference_id,
owner_id,
)
return _build_error_response(500, "INTERNAL", "Unexpected server error.")
if detail is None:
return _build_error_response(
404,
"ASSET_NOT_FOUND",
f"AssetReference {reference_id} not found.",
)
file_path = detail.ref.file_path
output_root = folder_paths.get_output_directory()
if (
"output" not in detail.tags
or not file_path
or not folder_paths.is_within_directory(output_root, file_path)
):
return _build_error_response(
403,
"ASSET_LOCATION_FORBIDDEN",
"Only generated files inside the output directory can be opened.",
)
try:
await asyncio.to_thread(reveal_file_in_file_manager, file_path)
except FileNotFoundError:
return _build_error_response(
404,
"ASSET_FILE_NOT_FOUND",
"The generated source file no longer exists on disk.",
)
except OSError:
logging.exception(
"Failed to reveal asset file for reference_id=%s", reference_id
)
return _build_error_response(
500,
"FILE_MANAGER_ERROR",
"Unable to open the system file manager.",
)
return web.Response(status=204)
@ROUTES.get("/api/tags") @ROUTES.get("/api/tags")
@_require_assets_feature_enabled @_require_assets_feature_enabled
async def get_tags(request: web.Request) -> web.Response: async def get_tags(request: web.Request) -> web.Response:

View File

@ -11,6 +11,7 @@ from app.assets.database.queries.asset import (
from app.assets.database.queries.asset_reference import ( from app.assets.database.queries.asset_reference import (
CacheStateRow, CacheStateRow,
UnenrichedReferenceRow, UnenrichedReferenceRow,
any_reference_exists_for_asset_id,
bulk_insert_references_ignore_conflicts, bulk_insert_references_ignore_conflicts,
bulk_update_enrichment_level, bulk_update_enrichment_level,
count_active_siblings, count_active_siblings,
@ -75,6 +76,7 @@ __all__ = [
"RemoveTagsResult", "RemoveTagsResult",
"SetTagsResult", "SetTagsResult",
"UnenrichedReferenceRow", "UnenrichedReferenceRow",
"any_reference_exists_for_asset_id",
"add_missing_tag_for_asset_id", "add_missing_tag_for_asset_id",
"add_tags_to_reference", "add_tags_to_reference",
"asset_exists_by_hash", "asset_exists_by_hash",

View File

@ -145,6 +145,19 @@ def reference_exists_for_asset_id(
return session.execute(q).first() is not None return session.execute(q).first() is not None
def any_reference_exists_for_asset_id(
session: Session,
asset_id: str,
) -> bool:
q = (
select(sa.literal(True))
.select_from(AssetReference)
.where(AssetReference.asset_id == asset_id)
.limit(1)
)
return session.execute(q).first() is not None
def reference_exists( def reference_exists(
session: Session, session: Session,
reference_id: str, reference_id: str,

View File

@ -1,6 +1,8 @@
from app.assets.services.asset_management import ( from app.assets.services.asset_management import (
AssetFileDeleteForbiddenError,
asset_exists, asset_exists,
delete_asset_reference, delete_asset_reference,
delete_asset_reference_with_file,
get_asset_by_hash, get_asset_by_hash,
get_asset_detail, get_asset_detail,
list_assets_page, list_assets_page,
@ -52,6 +54,7 @@ from app.assets.services.tagging import (
__all__ = [ __all__ = [
"AddTagsResult", "AddTagsResult",
"AssetFileDeleteForbiddenError",
"AssetData", "AssetData",
"AssetDetailResult", "AssetDetailResult",
"AssetSummaryData", "AssetSummaryData",
@ -72,6 +75,7 @@ __all__ = [
"batch_insert_seed_assets", "batch_insert_seed_assets",
"create_from_hash", "create_from_hash",
"delete_asset_reference", "delete_asset_reference",
"delete_asset_reference_with_file",
"get_asset_by_hash", "get_asset_by_hash",
"get_asset_detail", "get_asset_detail",
"ingest_existing_file", "ingest_existing_file",

View File

@ -1,9 +1,15 @@
import contextlib import contextlib
import errno
import logging
import mimetypes import mimetypes
import os import os
import shutil
import uuid
from datetime import timezone from datetime import timezone
from typing import Sequence from typing import Sequence
import folder_paths
from app.assets.services.cursor import ( from app.assets.services.cursor import (
CursorPayload, CursorPayload,
InvalidCursorError, InvalidCursorError,
@ -17,6 +23,7 @@ from app.assets.services.cursor import (
from app.assets.database.models import Asset from app.assets.database.models import Asset
from app.assets.database.queries import ( from app.assets.database.queries import (
any_reference_exists_for_asset_id,
asset_exists_by_hash, asset_exists_by_hash,
reference_exists_for_asset_id, reference_exists_for_asset_id,
delete_reference_by_id, delete_reference_by_id,
@ -52,6 +59,45 @@ from app.assets.services.schemas import (
from app.database.db import create_session from app.database.db import create_session
class AssetFileDeleteForbiddenError(PermissionError):
pass
def _copy_file_with_fsync(source_path: str, destination_path: str) -> None:
with open(source_path, "rb") as source_file, open(
destination_path, "wb"
) as destination_file:
shutil.copyfileobj(source_file, destination_file)
destination_file.flush()
os.fsync(destination_file.fileno())
shutil.copystat(source_path, destination_path)
def _stage_file_across_devices(source_path: str, staged_path: str) -> None:
try:
_copy_file_with_fsync(source_path, staged_path)
os.remove(source_path)
except OSError:
with contextlib.suppress(OSError):
os.remove(staged_path)
raise
def _restore_file_across_devices(staged_path: str, original_path: str) -> None:
restore_path = os.path.join(
os.path.dirname(original_path), f".comfy-restore-{uuid.uuid4().hex}.tmp"
)
try:
_copy_file_with_fsync(staged_path, restore_path)
os.replace(restore_path, original_path)
except OSError:
with contextlib.suppress(OSError):
os.remove(restore_path)
raise
with contextlib.suppress(OSError):
os.remove(staged_path)
def get_asset_detail( def get_asset_detail(
reference_id: str, reference_id: str,
owner_id: str = "", owner_id: str = "",
@ -221,6 +267,131 @@ def delete_asset_reference(
return True return True
def delete_asset_reference_with_file(
reference_id: str,
owner_id: str,
staging_directory: str,
expected_file_path: str,
allowed_directories: Sequence[str],
allow_ownerless: bool = False,
) -> bool:
"""Delete a reference and permanently remove its source file.
``expected_file_path`` must still match the stored path, and the caller must
restrict ``allowed_directories`` to managed roots. The source is staged in
``staging_directory`` before the database commit and restored if the commit
fails. ``allow_ownerless`` permits deleting legacy ownerless references.
The asset row is removed only when no references of any state remain.
The filesystem move and database commit are failure-compensated but are not
a single atomic transaction; abrupt process termination between them is not
recoverable by this function.
Raises ``AssetFileDeleteForbiddenError`` when ownership, path identity, or
containment checks fail.
"""
with create_session() as session:
ref = get_reference_by_id(session, reference_id=reference_id)
if ref is None or ref.deleted_at is not None:
return False
caller_owner_id = (owner_id or "").strip()
owns_reference = ref.owner_id == caller_owner_id
may_delete_ownerless = allow_ownerless and ref.owner_id == ""
if not (owns_reference or may_delete_ownerless):
raise AssetFileDeleteForbiddenError(
"Only the owning user can delete an asset's source file."
)
asset_id = ref.asset_id
file_path = ref.file_path
if not file_path or os.path.realpath(file_path) != os.path.realpath(
expected_file_path
):
raise AssetFileDeleteForbiddenError(
"The asset source path changed before it could be deleted."
)
if not any(
folder_paths.is_within_directory(directory, file_path)
for directory in allowed_directories
):
raise AssetFileDeleteForbiddenError(
"The asset source file is outside the directories allowed for deletion."
)
deleted = delete_reference_by_id(
session, reference_id=reference_id, owner_id=caller_owner_id
)
if not deleted:
session.rollback()
return False
staged_file_path: str | None = None
cross_device_staging = False
if file_path and os.path.isfile(file_path):
os.makedirs(staging_directory, exist_ok=True)
staged_file_path = os.path.join(
staging_directory, f".comfy-delete-{uuid.uuid4().hex}.tmp"
)
if not any(
folder_paths.is_within_directory(directory, file_path)
for directory in allowed_directories
):
raise AssetFileDeleteForbiddenError(
"The asset source file moved outside the directories allowed for deletion."
)
try:
os.replace(file_path, staged_file_path)
except OSError as staging_error:
if staging_error.errno != errno.EXDEV:
raise
if not any(
folder_paths.is_within_directory(directory, file_path)
for directory in allowed_directories
):
raise AssetFileDeleteForbiddenError(
"The asset source file moved outside the directories allowed for deletion."
)
_stage_file_across_devices(file_path, staged_file_path)
cross_device_staging = True
try:
session.flush()
if not any_reference_exists_for_asset_id(session, asset_id=asset_id):
asset = session.get(Asset, asset_id)
if asset is not None:
session.delete(asset)
session.commit()
except Exception:
session.rollback()
if (
staged_file_path
and file_path
and os.path.isfile(staged_file_path)
and not os.path.exists(file_path)
):
try:
if cross_device_staging:
_restore_file_across_devices(staged_file_path, file_path)
else:
os.replace(staged_file_path, file_path)
except OSError:
logging.exception(
"Failed to restore staged asset file after rollback: %s",
file_path,
)
raise
if staged_file_path:
try:
os.remove(staged_file_path)
except OSError:
logging.exception(
"Failed to remove staged asset file after commit: %s",
staged_file_path,
)
return True
def set_asset_preview( def set_asset_preview(
reference_id: str, reference_id: str,
preview_reference_id: str | None = None, preview_reference_id: str | None = None,

View File

@ -0,0 +1,48 @@
import ipaddress
import os
import subprocess
import sys
import threading
def is_loopback_address(address: str | None) -> bool:
if not address:
return False
try:
parsed = ipaddress.ip_address(address.split("%", 1)[0])
except ValueError:
return False
if parsed.is_loopback:
return True
return bool(
parsed.version == 6 and parsed.ipv4_mapped and parsed.ipv4_mapped.is_loopback
)
def build_file_manager_command(
file_path: str, platform: str | None = None
) -> list[str]:
normalized_path = os.path.abspath(file_path)
current_platform = platform or sys.platform
if current_platform == "win32":
return ["explorer.exe", f"/select,{normalized_path}"]
if current_platform == "darwin":
return ["open", "-R", normalized_path]
return ["xdg-open", os.path.dirname(normalized_path)]
def reveal_file_in_file_manager(file_path: str) -> None:
normalized_path = os.path.abspath(file_path)
if not os.path.isfile(normalized_path):
raise FileNotFoundError(normalized_path)
process = subprocess.Popen(
build_file_manager_command(normalized_path),
close_fds=True,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
threading.Thread(target=process.wait, daemon=True).start()

View File

@ -1,4 +1,8 @@
"""Tests for asset_management services.""" """Tests for asset_management services."""
import errno
import os
import folder_paths
import pytest import pytest
from sqlalchemy.orm import Session from sqlalchemy.orm import Session
@ -6,11 +10,14 @@ from app.assets.database.models import Asset, AssetReference
from app.assets.database.queries import ensure_tags_exist, add_tags_to_reference from app.assets.database.queries import ensure_tags_exist, add_tags_to_reference
from app.assets.helpers import get_utc_now from app.assets.helpers import get_utc_now
from app.assets.services import ( from app.assets.services import (
AssetFileDeleteForbiddenError,
delete_asset_reference_with_file,
get_asset_detail, get_asset_detail,
update_asset_metadata, update_asset_metadata,
delete_asset_reference, delete_asset_reference,
set_asset_preview, set_asset_preview,
) )
from app.assets.services import asset_management
from app.assets.services.asset_management import resolve_hash_to_path from app.assets.services.asset_management import resolve_hash_to_path
@ -215,6 +222,557 @@ class TestDeleteAssetReference:
assert session.get(Asset, asset_id) is None assert session.get(Asset, asset_id) is None
class TestDeleteAssetReferenceWithFile:
def test_deletes_only_selected_reference_file(
self, mock_create_session, session: Session, temp_dir
):
selected_file = temp_dir / "selected.bin"
retained_file = temp_dir / "retained.bin"
selected_file.write_bytes(b"same-content")
retained_file.write_bytes(b"same-content")
asset = _make_asset(session)
selected_ref = _make_reference(session, asset, name=selected_file.name)
selected_ref.file_path = str(selected_file)
retained_ref = _make_reference(session, asset, name=retained_file.name)
retained_ref.file_path = str(retained_file)
asset_id = asset.id
session.commit()
result = delete_asset_reference_with_file(
reference_id=selected_ref.id,
owner_id="",
staging_directory=str(temp_dir / "staging"),
expected_file_path=str(selected_file),
allowed_directories=[str(temp_dir)],
)
assert result is True
assert not selected_file.exists()
assert retained_file.exists()
assert session.get(Asset, asset_id) is not None
assert session.get(AssetReference, retained_ref.id) is not None
def test_rejects_ownerless_reference_without_explicit_permission(
self, mock_create_session, session: Session, temp_dir
):
selected_file = temp_dir / "selected.bin"
selected_file.write_bytes(b"content")
asset = _make_asset(session)
selected_ref = _make_reference(session, asset, owner_id="")
selected_ref.file_path = str(selected_file)
selected_ref_id = selected_ref.id
session.commit()
with pytest.raises(AssetFileDeleteForbiddenError, match="owning user"):
delete_asset_reference_with_file(
reference_id=selected_ref_id,
owner_id="another-user",
staging_directory=str(temp_dir / "staging"),
expected_file_path=str(selected_file),
allowed_directories=[str(temp_dir)],
)
assert selected_file.exists()
assert session.get(AssetReference, selected_ref_id) is not None
def test_allows_ownerless_reference_in_single_user_mode(
self, mock_create_session, session: Session, temp_dir
):
selected_file = temp_dir / "selected.bin"
selected_file.write_bytes(b"content")
asset = _make_asset(session)
selected_ref = _make_reference(session, asset, owner_id="")
selected_ref.file_path = str(selected_file)
selected_ref_id = selected_ref.id
session.commit()
result = delete_asset_reference_with_file(
reference_id=selected_ref_id,
owner_id="default",
staging_directory=str(temp_dir / "staging"),
expected_file_path=str(selected_file),
allowed_directories=[str(temp_dir)],
allow_ownerless=True,
)
assert result is True
assert not selected_file.exists()
assert session.get(AssetReference, selected_ref_id) is None
def test_uses_normalized_owner_for_deletion(
self, mock_create_session, session: Session, temp_dir, monkeypatch
):
selected_file = temp_dir / "selected.bin"
selected_file.write_bytes(b"content")
asset = _make_asset(session)
selected_ref = _make_reference(session, asset, owner_id="user1")
selected_ref.file_path = str(selected_file)
selected_ref_id = selected_ref.id
session.commit()
owner_ids = []
real_delete = asset_management.delete_reference_by_id
def capture_owner_id(session, reference_id, owner_id):
owner_ids.append(owner_id)
return real_delete(session, reference_id, owner_id)
monkeypatch.setattr(
asset_management, "delete_reference_by_id", capture_owner_id
)
result = delete_asset_reference_with_file(
reference_id=selected_ref_id,
owner_id=" user1 ",
staging_directory=str(temp_dir / "staging"),
expected_file_path=str(selected_file),
allowed_directories=[str(temp_dir)],
)
assert result is True
assert owner_ids == ["user1"]
def test_preserves_soft_deleted_shared_reference(
self, mock_create_session, session: Session, temp_dir
):
selected_file = temp_dir / "selected.bin"
retained_file = temp_dir / "retained.bin"
selected_file.write_bytes(b"same-content")
retained_file.write_bytes(b"same-content")
asset = _make_asset(session)
selected_ref = _make_reference(session, asset, name=selected_file.name)
selected_ref.file_path = str(selected_file)
retained_ref = _make_reference(session, asset, name=retained_file.name)
retained_ref.file_path = str(retained_file)
retained_ref.deleted_at = get_utc_now()
asset_id = asset.id
retained_ref_id = retained_ref.id
session.commit()
result = delete_asset_reference_with_file(
reference_id=selected_ref.id,
owner_id="",
staging_directory=str(temp_dir / "staging"),
expected_file_path=str(selected_file),
allowed_directories=[str(temp_dir)],
)
assert result is True
assert not selected_file.exists()
assert retained_file.exists()
assert session.get(Asset, asset_id) is not None
assert session.get(AssetReference, retained_ref_id) is not None
def test_restores_file_when_commit_fails(
self, mock_create_session, session: Session, temp_dir, monkeypatch
):
selected_file = temp_dir / "selected.bin"
selected_file.write_bytes(b"content")
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_commit(_session):
raise RuntimeError("commit failed")
monkeypatch.setattr(Session, "commit", fail_commit)
with pytest.raises(RuntimeError, match="commit failed"):
delete_asset_reference_with_file(
reference_id=selected_ref_id,
owner_id="",
staging_directory=str(temp_dir / "staging"),
expected_file_path=str(selected_file),
allowed_directories=[str(temp_dir)],
)
session.expire_all()
assert selected_file.read_bytes() == b"content"
assert session.get(AssetReference, selected_ref_id) is not None
def test_retains_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_stages_cross_device_file_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()
copy_calls = []
real_copy = asset_management._copy_file_with_fsync
def fail_cross_device_replace(_source, _destination):
raise OSError(errno.EXDEV, "Cross-device link")
def capture_copy(source, destination):
copy_calls.append((os.fspath(source), os.fspath(destination)))
return real_copy(source, destination)
monkeypatch.setattr(
asset_management.os, "replace", fail_cross_device_replace
)
monkeypatch.setattr(
asset_management, "_copy_file_with_fsync", capture_copy
)
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 len(copy_calls) == 1
assert copy_calls[0][0] == str(selected_file)
assert os.path.dirname(copy_calls[0][1]) == str(staging_directory)
assert not selected_file.exists()
assert list(staging_directory.glob(".comfy-delete-*.tmp")) == []
assert session.get(AssetReference, selected_ref_id) is None
def test_retains_cross_device_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()
real_copy = asset_management._copy_file_with_fsync
real_remove = os.remove
copy_calls = []
def fail_cross_device_replace(_source, _destination):
raise OSError(errno.EXDEV, "Cross-device link")
def capture_copy(source, destination):
copy_calls.append((os.fspath(source), os.fspath(destination)))
return real_copy(source, destination)
def fail_final_cleanup(path):
if os.fspath(path) == os.fspath(selected_file):
return real_remove(path)
raise PermissionError("file is busy")
monkeypatch.setattr(
asset_management.os, "replace", fail_cross_device_replace
)
monkeypatch.setattr(
asset_management, "_copy_file_with_fsync", capture_copy
)
monkeypatch.setattr(asset_management.os, "remove", fail_final_cleanup)
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()
queued_files = list(staging_directory.glob(".comfy-delete-*.tmp"))
assert result is True
assert len(copy_calls) == 1
assert not selected_file.exists()
assert len(queued_files) == 1
assert queued_files[0].read_bytes() == b"content"
assert session.get(AssetReference, selected_ref_id) is None
def test_cleans_cross_device_copy_when_source_unlink_fails(
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()
real_remove = os.remove
def fail_cross_device_replace(_source, _destination):
raise OSError(errno.EXDEV, "Cross-device link")
def fail_source_unlink(path):
if os.fspath(path) == os.fspath(selected_file):
raise PermissionError(errno.EACCES, "Permission denied")
return real_remove(path)
monkeypatch.setattr(
asset_management.os, "replace", fail_cross_device_replace
)
monkeypatch.setattr(asset_management.os, "remove", fail_source_unlink)
with pytest.raises(PermissionError, match="Permission denied"):
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 selected_file.read_bytes() == b"content"
assert list(staging_directory.glob(".comfy-delete-*.tmp")) == []
assert session.get(AssetReference, selected_ref_id) is not None
def test_restores_cross_device_stage_when_commit_fails(
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()
real_replace = os.replace
real_copy = asset_management._copy_file_with_fsync
replace_calls = 0
copy_calls = []
def fail_first_replace(source, destination):
nonlocal replace_calls
replace_calls += 1
if replace_calls == 1:
raise OSError(errno.EXDEV, "Cross-device link")
return real_replace(source, destination)
def capture_copy(source, destination):
copy_calls.append((os.fspath(source), os.fspath(destination)))
return real_copy(source, destination)
def fail_commit(_session):
raise RuntimeError("commit failed")
monkeypatch.setattr(asset_management.os, "replace", fail_first_replace)
monkeypatch.setattr(
asset_management, "_copy_file_with_fsync", capture_copy
)
monkeypatch.setattr(Session, "commit", fail_commit)
with pytest.raises(RuntimeError, match="commit failed"):
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 len(copy_calls) == 2
assert copy_calls[0][0] == str(selected_file)
assert copy_calls[1][1].startswith(
str(selected_file.parent / ".comfy-restore-")
)
assert selected_file.read_bytes() == b"content"
assert list(staging_directory.glob(".comfy-delete-*.tmp")) == []
assert session.get(AssetReference, selected_ref_id) is not None
def test_does_not_fallback_for_non_cross_device_staging_error(
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")
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()
copy_calls = []
def fail_replace(_source, _destination):
raise PermissionError(errno.EACCES, "Permission denied")
monkeypatch.setattr(asset_management.os, "replace", fail_replace)
monkeypatch.setattr(
asset_management,
"_copy_file_with_fsync",
lambda *args: copy_calls.append(args),
)
with pytest.raises(PermissionError, match="Permission denied"):
delete_asset_reference_with_file(
reference_id=selected_ref_id,
owner_id="",
staging_directory=str(temp_dir / "staging"),
expected_file_path=str(selected_file),
allowed_directories=[str(selected_file.parent)],
)
session.expire_all()
assert copy_calls == []
assert selected_file.read_bytes() == b"content"
assert session.get(AssetReference, selected_ref_id) is not None
def test_rechecks_containment_before_cross_device_fallback(
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")
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()
containment_results = iter([True, True, False])
copy_calls = []
def fail_cross_device_replace(_source, _destination):
raise OSError(errno.EXDEV, "Cross-device link")
def capture_copy(*args):
copy_calls.append(args)
monkeypatch.setattr(
folder_paths,
"is_within_directory",
lambda *_args: next(containment_results),
)
monkeypatch.setattr(
asset_management.os,
"replace",
fail_cross_device_replace,
)
monkeypatch.setattr(
asset_management,
"_copy_file_with_fsync",
capture_copy,
)
with pytest.raises(AssetFileDeleteForbiddenError, match="moved outside"):
delete_asset_reference_with_file(
reference_id=selected_ref_id,
owner_id="",
staging_directory=str(temp_dir / "staging"),
expected_file_path=str(selected_file),
allowed_directories=[str(selected_file.parent)],
)
session.expire_all()
assert copy_calls == []
assert selected_file.read_bytes() == b"content"
assert session.get(AssetReference, selected_ref_id) is not None
def test_rejects_a_source_path_changed_after_authorization(
self, mock_create_session, session: Session, temp_dir
):
selected_file = temp_dir / "selected.bin"
replacement_file = temp_dir / "replacement.bin"
selected_file.write_bytes(b"selected")
replacement_file.write_bytes(b"replacement")
asset = _make_asset(session)
selected_ref = _make_reference(session, asset, name=selected_file.name)
selected_ref.file_path = str(replacement_file)
selected_ref_id = selected_ref.id
session.commit()
with pytest.raises(PermissionError, match="source path changed"):
delete_asset_reference_with_file(
reference_id=selected_ref_id,
owner_id="",
staging_directory=str(temp_dir / "staging"),
expected_file_path=str(selected_file),
allowed_directories=[str(temp_dir)],
)
assert selected_file.exists()
assert replacement_file.exists()
assert session.get(AssetReference, selected_ref_id) is not None
def test_rechecks_containment_immediately_before_staging(
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")
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()
containment_results = iter([True, False])
monkeypatch.setattr(
folder_paths,
"is_within_directory",
lambda *_args: next(containment_results),
)
with pytest.raises(PermissionError, match="moved outside"):
delete_asset_reference_with_file(
reference_id=selected_ref_id,
owner_id="",
staging_directory=str(temp_dir / "staging"),
expected_file_path=str(selected_file),
allowed_directories=[str(temp_dir / "managed")],
)
session.expire_all()
assert selected_file.read_bytes() == b"content"
assert session.get(AssetReference, selected_ref_id) is not None
class TestSetAssetPreview: class TestSetAssetPreview:
def test_sets_preview(self, mock_create_session, session: Session): def test_sets_preview(self, mock_create_session, session: Session):
asset = _make_asset(session, hash_val="blake3:main") asset = _make_asset(session, hash_val="blake3:main")

View File

@ -0,0 +1,83 @@
import subprocess
import threading
from unittest.mock import Mock
import pytest
from app.assets.services.file_location import (
build_file_manager_command,
is_loopback_address,
reveal_file_in_file_manager,
)
@pytest.mark.parametrize(
"address",
["127.0.0.1", "127.42.0.9", "::1", "::ffff:127.0.0.1"],
)
def test_accepts_loopback_addresses(address):
assert is_loopback_address(address)
@pytest.mark.parametrize(
"address",
[
None,
"",
"localhost",
"0.0.0.0",
"192.168.1.20",
"::",
"fe80::1",
"::ffff:192.168.1.20",
"fe80::1%eth0",
],
)
def test_rejects_non_loopback_addresses(address):
assert not is_loopback_address(address)
def test_builds_platform_commands(tmp_path):
source = tmp_path / "folder with spaces" / "render.mp4"
source.parent.mkdir()
source.write_bytes(b"video")
assert build_file_manager_command(str(source), "win32") == [
"explorer.exe",
f"/select,{source}",
]
assert build_file_manager_command(str(source), "darwin") == [
"open",
"-R",
str(source),
]
assert build_file_manager_command(str(source), "linux") == [
"xdg-open",
str(source.parent),
]
def test_reveal_launches_without_a_shell(tmp_path, monkeypatch):
source = tmp_path / "render.png"
source.write_bytes(b"image")
process = Mock()
popen = Mock(return_value=process)
reaper = Mock()
thread = Mock(return_value=reaper)
monkeypatch.setattr(subprocess, "Popen", popen)
monkeypatch.setattr(threading, "Thread", thread)
reveal_file_in_file_manager(str(source))
popen.assert_called_once()
args, kwargs = popen.call_args
assert args[0]
assert kwargs["close_fds"] is True
assert "shell" not in kwargs
thread.assert_called_once_with(target=process.wait, daemon=True)
reaper.start.assert_called_once_with()
def test_reveal_rejects_missing_files(tmp_path):
with pytest.raises(FileNotFoundError):
reveal_file_in_file_manager(str(tmp_path / "missing.mp4"))

View File

@ -0,0 +1,288 @@
import asyncio
import json
import uuid
from types import SimpleNamespace
from unittest.mock import Mock
from app.assets.api import routes
def make_request(remote="127.0.0.1", headers=None, query=None):
return SimpleNamespace(
match_info={"id": str(uuid.uuid4())},
remote=remote,
headers=headers or {},
query=query or {},
)
def run_route(request):
return asyncio.run(routes.open_asset_location_route.__wrapped__(request))
def run_delete_route(request):
return asyncio.run(routes.delete_asset_route.__wrapped__(request))
def test_open_location_rejects_lan_clients(monkeypatch):
get_detail = Mock()
monkeypatch.setattr(routes, "get_asset_detail", get_detail)
response = run_route(make_request(remote="192.168.1.50"))
assert response.status == 403
assert json.loads(response.text)["error"]["code"] == "LOCAL_ACCESS_REQUIRED"
get_detail.assert_not_called()
def test_open_location_rejects_cross_site_requests(monkeypatch):
get_detail = Mock()
monkeypatch.setattr(routes, "get_asset_detail", get_detail)
response = run_route(make_request(headers={"Sec-Fetch-Site": "cross-site"}))
assert response.status == 403
assert json.loads(response.text)["error"]["code"] == "CROSS_SITE_REQUEST_FORBIDDEN"
get_detail.assert_not_called()
def test_open_location_reveals_managed_generated_file(tmp_path, monkeypatch):
source = tmp_path / "output" / "video" / "render.mp4"
source.parent.mkdir(parents=True)
source.write_bytes(b"video")
reveal = Mock()
monkeypatch.setattr(
routes,
"USER_MANAGER",
SimpleNamespace(get_request_user_id=lambda _request: "default"),
)
monkeypatch.setattr(
routes,
"get_asset_detail",
lambda **_kwargs: SimpleNamespace(
tags=["output"], ref=SimpleNamespace(file_path=str(source))
),
)
monkeypatch.setattr(
routes.folder_paths,
"get_output_directory",
lambda: str(tmp_path / "output"),
)
monkeypatch.setattr(routes, "reveal_file_in_file_manager", reveal)
response = run_route(make_request())
assert response.status == 204
reveal.assert_called_once_with(str(source))
def test_open_location_returns_structured_error_for_unknown_user(monkeypatch):
monkeypatch.setattr(
routes,
"USER_MANAGER",
SimpleNamespace(get_request_user_id=Mock(side_effect=KeyError("unknown"))),
)
response = run_route(make_request())
assert response.status == 500
assert json.loads(response.text)["error"]["code"] == "INTERNAL"
def test_open_location_rejects_non_output_assets(tmp_path, monkeypatch):
source = tmp_path / "output" / "image.png"
source.parent.mkdir()
source.write_bytes(b"image")
monkeypatch.setattr(
routes,
"USER_MANAGER",
SimpleNamespace(get_request_user_id=lambda _request: "default"),
)
monkeypatch.setattr(
routes,
"get_asset_detail",
lambda **_kwargs: SimpleNamespace(
tags=["input"], ref=SimpleNamespace(file_path=str(source))
),
)
monkeypatch.setattr(
routes.folder_paths,
"get_output_directory",
lambda: str(tmp_path / "output"),
)
reveal = Mock()
monkeypatch.setattr(routes, "reveal_file_in_file_manager", reveal)
response = run_route(make_request())
assert response.status == 403
assert json.loads(response.text)["error"]["code"] == "ASSET_LOCATION_FORBIDDEN"
reveal.assert_not_called()
def test_open_location_rejects_output_asset_outside_output_root(
tmp_path, monkeypatch
):
source = tmp_path / "outside" / "image.png"
source.parent.mkdir()
source.write_bytes(b"image")
monkeypatch.setattr(
routes,
"USER_MANAGER",
SimpleNamespace(get_request_user_id=lambda _request: "default"),
)
monkeypatch.setattr(
routes,
"get_asset_detail",
lambda **_kwargs: SimpleNamespace(
tags=["output"], ref=SimpleNamespace(file_path=str(source))
),
)
monkeypatch.setattr(
routes.folder_paths,
"get_output_directory",
lambda: str(tmp_path / "output"),
)
reveal = Mock()
monkeypatch.setattr(routes, "reveal_file_in_file_manager", reveal)
response = run_route(make_request())
assert response.status == 403
assert json.loads(response.text)["error"]["code"] == "ASSET_LOCATION_FORBIDDEN"
reveal.assert_not_called()
def test_delete_content_rejects_paths_outside_managed_root(tmp_path, monkeypatch):
source = tmp_path / "outside" / "render.png"
source.parent.mkdir()
source.write_bytes(b"image")
delete_with_file = Mock()
monkeypatch.setattr(
routes,
"USER_MANAGER",
SimpleNamespace(get_request_user_id=lambda _request: "default"),
)
monkeypatch.setattr(
routes,
"get_asset_detail",
lambda **_kwargs: SimpleNamespace(
tags=["output"], ref=SimpleNamespace(file_path=str(source))
),
)
monkeypatch.setattr(
routes.folder_paths,
"get_output_directory",
lambda: str(tmp_path / "output"),
)
monkeypatch.setattr(routes, "delete_asset_reference_with_file", delete_with_file)
response = run_delete_route(make_request(query={"delete_content": "true"}))
assert response.status == 403
assert json.loads(response.text)["error"]["code"] == "ASSET_DELETE_FORBIDDEN"
delete_with_file.assert_not_called()
def test_delete_content_uses_guarded_file_service(tmp_path, monkeypatch):
source = tmp_path / "output" / "render.png"
source.parent.mkdir()
source.write_bytes(b"image")
delete_with_file = Mock(return_value=True)
monkeypatch.setattr(
routes,
"USER_MANAGER",
SimpleNamespace(get_request_user_id=lambda _request: "default"),
)
monkeypatch.setattr(
routes,
"get_asset_detail",
lambda **_kwargs: SimpleNamespace(
tags=["output"], ref=SimpleNamespace(file_path=str(source))
),
)
monkeypatch.setattr(
routes.folder_paths,
"get_output_directory",
lambda: str(tmp_path / "output"),
)
monkeypatch.setattr(
routes.folder_paths,
"get_temp_directory",
lambda: str(tmp_path / "temp"),
)
monkeypatch.setattr(routes.user_manager.args, "multi_user", False)
monkeypatch.setattr(routes, "delete_asset_reference_with_file", delete_with_file)
request = make_request(query={"delete_content": "true"})
response = run_delete_route(request)
assert response.status == 204
delete_with_file.assert_called_once_with(
reference_id=request.match_info["id"],
owner_id="default",
staging_directory=str(tmp_path / "temp"),
expected_file_path=str(source),
allowed_directories=[str(tmp_path / "output")],
allow_ownerless=True,
)
def test_delete_returns_structured_error_for_unknown_user(monkeypatch):
delete_reference = Mock()
monkeypatch.setattr(
routes,
"USER_MANAGER",
SimpleNamespace(get_request_user_id=Mock(side_effect=KeyError("unknown"))),
)
monkeypatch.setattr(routes, "delete_asset_reference", delete_reference)
response = run_delete_route(make_request())
assert response.status == 500
assert json.loads(response.text)["error"]["code"] == "INTERNAL"
delete_reference.assert_not_called()
def test_delete_does_not_expose_os_permission_error_path(tmp_path, monkeypatch):
source = tmp_path / "output" / "private" / "render.png"
source.parent.mkdir(parents=True)
source.write_bytes(b"image")
delete_with_file = Mock(
side_effect=PermissionError(13, "Permission denied", str(source))
)
monkeypatch.setattr(
routes,
"USER_MANAGER",
SimpleNamespace(get_request_user_id=lambda _request: "default"),
)
monkeypatch.setattr(
routes,
"get_asset_detail",
lambda **_kwargs: SimpleNamespace(
tags=["output"], ref=SimpleNamespace(file_path=str(source))
),
)
monkeypatch.setattr(
routes.folder_paths,
"get_output_directory",
lambda: str(tmp_path / "output"),
)
monkeypatch.setattr(
routes.folder_paths,
"get_temp_directory",
lambda: str(tmp_path / "temp"),
)
monkeypatch.setattr(routes.user_manager.args, "multi_user", False)
monkeypatch.setattr(routes, "delete_asset_reference_with_file", delete_with_file)
response = run_delete_route(make_request(query={"delete_content": "true"}))
assert response.status == 500
assert json.loads(response.text)["error"]["code"] == "INTERNAL"
assert str(source) not in response.text