diff --git a/py/api/admin.py b/py/api/admin.py index 97765b52..bd93bcde 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -36,10 +36,10 @@ except ImportError: try: from ...utils import exit_signal - from ...utils.media_folders import is_skipped_folder + from ...utils.media_folders import in_trash_below_roots, is_skipped_folder except ImportError: from utils import exit_signal - from utils.media_folders import is_skipped_folder + from utils.media_folders import in_trash_below_roots, is_skipped_folder try: from ...database.image_removal import mark_hides @@ -1106,6 +1106,8 @@ class AdminRoutesMixin: return "not a media file" if "thumbnails" in rel_path.parts: return "thumbnails are managed by the gallery" + if in_trash_below_roots(file_path, [output_path]): + return "it is in a trash folder" # never unlinked from a trash (#543) if not file_path.is_file(): return "file not found" diff --git a/py/api/history_current.py b/py/api/history_current.py index 4842db5b..c3128f34 100644 --- a/py/api/history_current.py +++ b/py/api/history_current.py @@ -199,6 +199,9 @@ def resolve_current_candidate( if namespace is None or not namespace.is_dir(): return _unavailable("root_unavailable") target = _canonical(lexical) + if in_trash_below_roots(target, [namespace]): + # Every kind and the outputs listing come through here (#543). + return _unavailable("missing") if not _authorized(target, namespace, scope): return _unavailable("not_authorized") if target.suffix.lower() not in SERVABLE_EXTENSIONS: @@ -249,9 +252,6 @@ def _open_current_target( A *preview* (a file under ``thumbnails/``) may also be JPEG XL (#317).""" if _canonical(target) != target: raise CurrentLocationUnavailable("not_authorized", stale=True) - if in_trash_below_roots(target, [namespace]): - # An output recorded before #501 skipped trash folders (#543). - raise CurrentLocationUnavailable("missing") if not _authorized(target, namespace, scope): raise CurrentLocationUnavailable("not_authorized") suffix = target.suffix.lower() diff --git a/tests/test_api_history_outputs.py b/tests/test_api_history_outputs.py index b3832030..06dcbc13 100644 --- a/tests/test_api_history_outputs.py +++ b/tests/test_api_history_outputs.py @@ -212,6 +212,31 @@ class HistoryOutputAPITests(ImageAPITestCase): self.assertNotIn("root=", candidate["media"][field]) self.assertNotIn(str(self.comfy_dir), candidate["media"][field]) + async def test_an_output_in_a_trash_folder_is_read_by_no_kind(self): + # #543: inspection, media, thumbnail and the listing share one check. + path = make_png(self.output_dir / "image.png") + start = self.capture([self.ref()]) + media = await self.media(start) + trashed = self.output_dir / ".Trash-1000" / "files" + trashed.mkdir(parents=True) + os.rename(path, trashed / "image.png") + try: + os.symlink(trashed / "image.png", path) + except OSError: + if os.environ.get("GITHUB_ACTIONS"): + raise + self.skipTest("this machine cannot make symlinks") + for field in ("inspection_url", "url", "thumbnail_url"): + with self.subTest(field=field): + response = await self.current(media[field]) + self.assertIn(response.status, (404, 409, 410)) + await response.read() + response, page = await self.outputs(start) + self.assertEqual(response.status, 200, page) + candidate = page["items"][0]["current_candidate"] + self.assertEqual(candidate["status"], "unavailable") + self.assertEqual(candidate["reason"], "missing") + async def test_complete_zero_incomplete_unavailable_and_not_captured_are_distinct( self, ): diff --git a/tests/test_trash_gate_routes.py b/tests/test_trash_gate_routes.py index ede6edee..22a05853 100644 --- a/tests/test_trash_gate_routes.py +++ b/tests/test_trash_gate_routes.py @@ -14,14 +14,19 @@ from pathlib import Path from unittest.mock import AsyncMock, patch try: + from tests.trash_route_audit import ( + FILE_READER_CALLS, + KNOWN_FILE_READERS, + ROUTE_AUDIT, + ) from tests.test_api_admin import _write_png from tests.test_image_delete_routes import ImageDeleteTestCase except ImportError: # discovered with tests/ as the top-level directory + from trash_route_audit import FILE_READER_CALLS, KNOWN_FILE_READERS, ROUTE_AUDIT from test_api_admin import _write_png from test_image_delete_routes import ImageDeleteTestCase from py.api.images import media_thumbnail_rel -from py.api.history_current import CurrentLocationUnavailable, _open_current_target from py.api.thumbnail_auto import OVER, _quick_source, _thumbnail_source from utils.media_folders import in_trash_below_roots, in_trash_folder, is_trash_folder @@ -221,12 +226,13 @@ class TestTheGate(_GateCase): ) ) - async def test_a_history_output_in_a_trash_folder_does_not_open(self): + async def test_the_legacy_duplicate_delete_never_unlinks_from_a_trash(self): self.links() - target = self.root / ".Trash-1000" / "files" / "x.png" - with self.assertRaises(CurrentLocationUnavailable) as caught: - _open_current_target(Path(os.path.realpath(target)), self.root, None) - self.assertEqual(caught.exception.reason, "missing") + for relative in (*NAMED, *LINKED): + with self.subTest(relative=relative): + reason = self.api._delete_one_duplicate(relative, self.root) + self.assertEqual(reason, "it is in a trash folder") + self.assertTrue((self.root / relative).exists()) async def test_a_source_named_like_a_trash_folder_still_reads(self): source = self.root / ".Trash-77" @@ -290,3 +296,31 @@ class TestWindowsSpellings(_GateCase): self.assertTrue(in_trash_below_roots(inside, [str(self.root).upper()])) kept = self.root / "kept.png" self.assertFalse(in_trash_below_roots(kept, [str(self.root).upper()])) + + +class TestEveryRouteIsClassified(_GateCase): + async def test_a_new_route_must_say_how_it_treats_media_files(self): + registered = { + f"{route.method} {route.resource.canonical}" + for route in self.app.router.routes() + if route.method != "HEAD" + and route.resource is not None + and route.resource.canonical.startswith("/prompt_manager") + } + audited = {route for routes in ROUTE_AUDIT.values() for route in routes} + self.assertEqual( + registered ^ audited, + set(), + "add the route to tests/trash_route_audit.py with its media check", + ) + + def test_a_file_reader_in_the_api_layer_is_a_known_one(self): + # A new way to open a media file must be looked at for the same gate. + api = Path(__file__).resolve().parent.parent / "py" / "api" + found = set() + for module in api.glob("*.py"): + text = module.read_text(encoding="utf-8") + found.update( + f"{module.name}:{call}" for call in FILE_READER_CALLS if call in text + ) + self.assertEqual(found ^ KNOWN_FILE_READERS, set()) diff --git a/tests/trash_route_audit.py b/tests/trash_route_audit.py new file mode 100644 index 00000000..a18e2387 --- /dev/null +++ b/tests/trash_route_audit.py @@ -0,0 +1,211 @@ +"""Every /prompt_manager route and how it treats a file in a trash folder (#543). + +``tests/test_trash_gate_routes.py`` fails when a route is missing here, so a new +route has to choose a group. ``FILE_READER_CALLS`` is the same idea for code that +opens files in ``py/api``. +""" + +FILE_READER_CALLS = ("FileResponse(", "Image.open(", "os.open(", "read_bytes(") + +KNOWN_FILE_READERS = frozenset( + { + "history_current.py:os.open(", + "images.py:FileResponse(", + "images.py:Image.open(", + "images.py:os.open(", + "__init__.py:Image.open(", + "logging_routes.py:os.open(", + "thumbnail_auto.py:FileResponse(", + "thumbnail_encode.py:Image.open(", + } +) + +ROUTE_AUDIT = { + "gate: _media_access_error, resolved": ( + "GET /prompt_manager/images/{image_id}/file", + "GET /prompt_manager/images/inspect/{image_path}", + "POST /prompt_manager/images/reveal", + "POST /prompt_manager/images/link", + "POST /prompt_manager/images/rescope", + "GET /prompt_manager/library/{positive_id}/images", + "GET /prompt_manager/library/{positive_id}", + "GET /prompt_manager/library", + ), + "in_trash_below_roots, resolved": ( + "GET /prompt_manager/images/serve/{filepath}", + "POST /prompt_manager/autotag/single", + "POST /prompt_manager/autotag/apply", + "GET /prompt_manager/history/runs/{run_id}/outputs", + "GET /prompt_manager/history/runs/{run_id}/output/current/media", + "GET /prompt_manager/history/runs/{run_id}/output/current/thumbnail", + "GET /prompt_manager/history/runs/{run_id}/output/current/inspection", + "POST /prompt_manager/delete_duplicate_images", + ), + "media_target by name, then the capture alias check or listed files": ( + "POST /prompt_manager/image-annotations/observe", + "POST /prompt_manager/image-annotations/suggest", + "PUT /prompt_manager/image-annotations/tags", + "GET /prompt_manager/image-annotations/capture/{token}", + "DELETE /prompt_manager/image-annotations/capture/{token}", + "POST /prompt_manager/jobs/image-tags", + "POST /prompt_manager/collections/items", + "DELETE /prompt_manager/collections/items", + "GET /prompt_manager/collections/membership", + "PUT /prompt_manager/images/category", + "DELETE /prompt_manager/images/category", + ), + "walker: utils/media_folders (listed files only)": ( + "GET /prompt_manager/images/output", + "POST /prompt_manager/gallery/rescan", + "GET /prompt_manager/gallery/subfolders", + "PUT /prompt_manager/images/favorite", + "DELETE /prompt_manager/images/favorite", + "GET /prompt_manager/images/categories", + "POST /prompt_manager/images/delete/preview", + "POST /prompt_manager/images/removed/return", + "POST /prompt_manager/scan", + "POST /prompt_manager/jobs/scan", + "GET /prompt_manager/scan_duplicates", + "POST /prompt_manager/scan_duplicates/stream", + "POST /prompt_manager/jobs/duplicates", + "GET /prompt_manager/maintenance/duplicates/{job_id}", + "GET /prompt_manager/scan_output_dir", + "POST /prompt_manager/autotag/start", + "POST /prompt_manager/autotag/batch/preview", + "POST /prompt_manager/jobs/autotag", + "POST /prompt_manager/jobs/thumbnails", + "GET /prompt_manager/images/thumbnails/plan", + "GET /prompt_manager/images/thumbnails/status", + "POST /prompt_manager/images/generate-thumbnails", + "POST /prompt_manager/images/generate-thumbnails/progress", + "GET /prompt_manager/diagnostics", + ), + "record text only, reads no file; its media is served through the gate": ( + "GET /prompt_manager/images/all", + "GET /prompt_manager/images/recent", + "GET /prompt_manager/images/search", + "GET /prompt_manager/images/prompt/{image_path}", + "GET /prompt_manager/prompts/{prompt_id}/images", + "DELETE /prompt_manager/images/{image_id}", + "GET /prompt_manager/recent", + "GET /prompt_manager/search", + "GET /prompt_manager/export", + "GET /prompt_manager/library/export", + "GET /prompt_manager/history/runs/{run_id}", + "GET /prompt_manager/review/items", + "GET /prompt_manager/review/items/{item_id}", + "POST /prompt_manager/review/items/{item_id}/decision", + "POST /prompt_manager/maintenance/preview", + "POST /prompt_manager/maintenance/apply", + ), + "no media file": ( + "DELETE /prompt_manager/collections/{collection_id}", + "DELETE /prompt_manager/delete/{prompt_id}", + "DELETE /prompt_manager/pairs/{pair_id}", + "DELETE /prompt_manager/prompts/{prompt_id}/tags", + "DELETE /prompt_manager/restore/preview/{token}", + "DELETE /prompt_manager/saved-views/{view_id}", + "DELETE /prompt_manager/tags/{tag_name}", + "GET /prompt_manager/admin", + "GET /prompt_manager/autotag/models", + "GET /prompt_manager/backup", + "GET /prompt_manager/backup/status", + "GET /prompt_manager/categories", + "GET /prompt_manager/collections", + "GET /prompt_manager/gallery", + "GET /prompt_manager/gallery.html", + "GET /prompt_manager/jobs", + "GET /prompt_manager/jobs/{job_id}", + "GET /prompt_manager/jobs/{job_id}/events", + "GET /prompt_manager/jobs/{job_id}/report", + "GET /prompt_manager/js/{filepath}", + "GET /prompt_manager/lib/{filepath}", + "GET /prompt_manager/library/options", + "GET /prompt_manager/library/{positive_id}/history", + "GET /prompt_manager/library/{positive_id}/history/pairs/{pair_id}/newest", + "GET /prompt_manager/logs", + "GET /prompt_manager/logs/config", + "GET /prompt_manager/logs/download/{filename}", + "GET /prompt_manager/logs/files", + "GET /prompt_manager/logs/stats", + "GET /prompt_manager/lora/detect", + "GET /prompt_manager/lora/status", + "GET /prompt_manager/lora/trigger-words", + "GET /prompt_manager/pairs", + "GET /prompt_manager/pairs/export", + "GET /prompt_manager/pairs/{pair_id}", + "GET /prompt_manager/review/events", + "GET /prompt_manager/saved-views", + "GET /prompt_manager/scan/status", + "GET /prompt_manager/scan_duplicates/status", + "GET /prompt_manager/settings", + "GET /prompt_manager/stats", + "GET /prompt_manager/subfolders", + "GET /prompt_manager/system/diagnostics", + "GET /prompt_manager/system/health", + "GET /prompt_manager/system/library", + "GET /prompt_manager/system/overview", + "GET /prompt_manager/system/statistics", + "GET /prompt_manager/tags", + "GET /prompt_manager/tags/filter", + "GET /prompt_manager/tags/stats", + "GET /prompt_manager/tags/suggest", + "GET /prompt_manager/tags/{tag_name}/prompts", + "GET /prompt_manager/test", + "GET /prompt_manager/vocabulary/categories", + "GET /prompt_manager/vocabulary/tags", + "GET /prompt_manager/vocabulary/tags/{tag_id}/impact", + "GET /prompt_manager/web", + "GET /prompt_manager/workspace", + "PATCH /prompt_manager/collections/{collection_id}", + "PATCH /prompt_manager/library/{positive_id}", + "PATCH /prompt_manager/pairs/{pair_id}", + "PATCH /prompt_manager/saved-views/{view_id}", + "PATCH /prompt_manager/vocabulary/tags/{tag_id}", + "POST /prompt_manager/autotag/download/{model_type}", + "POST /prompt_manager/autotag/unload", + "POST /prompt_manager/bulk/category", + "POST /prompt_manager/bulk/delete", + "POST /prompt_manager/bulk/tags", + "POST /prompt_manager/cleanup", + "POST /prompt_manager/diagnostics/test-link", + "POST /prompt_manager/images/clear-thumbnails", + "POST /prompt_manager/jobs/model-download", + "POST /prompt_manager/jobs/{job_id}/cancel", + "POST /prompt_manager/library", + "POST /prompt_manager/library/delete", + "POST /prompt_manager/library/delete/preview", + "POST /prompt_manager/library/restore", + "POST /prompt_manager/library/{positive_id}/variants", + "POST /prompt_manager/logs/config", + "POST /prompt_manager/logs/truncate", + "POST /prompt_manager/lora/enable", + "POST /prompt_manager/lora/import/apply", + "POST /prompt_manager/lora/import/preview", + "POST /prompt_manager/lora/refresh-cache", + "POST /prompt_manager/lora/scan", + "POST /prompt_manager/maintenance", + "POST /prompt_manager/pairs", + "POST /prompt_manager/prompts/tags", + "POST /prompt_manager/prompts/{prompt_id}/tags", + "POST /prompt_manager/restore", + "POST /prompt_manager/restore/apply", + "POST /prompt_manager/restore/preview", + "POST /prompt_manager/save", + "POST /prompt_manager/saved-views", + "POST /prompt_manager/scan/cancel", + "POST /prompt_manager/settings", + "POST /prompt_manager/settings/validate-source", + "POST /prompt_manager/system/health", + "POST /prompt_manager/system/library/keep", + "POST /prompt_manager/system/library/move", + "POST /prompt_manager/tags/merge", + "POST /prompt_manager/vocabulary/categories/apply", + "POST /prompt_manager/vocabulary/categories/preview", + "POST /prompt_manager/vocabulary/tags", + "POST /prompt_manager/vocabulary/tags/{tag_id}/delete", + "PUT /prompt_manager/prompts/{prompt_id}", + "PUT /prompt_manager/prompts/{prompt_id}/rating", + "PUT /prompt_manager/tags/{tag_name}", + ), +}