diff --git a/py/api/images.py b/py/api/images.py index 3a46d5f..d12af2b 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -20,7 +20,13 @@ from PIL import Image, UnidentifiedImageError from .prompts import bad_request, parse_page_params, publish_image_paths from .prompts import thumbnail_url_for from .prompts import safe_error_message as _safe_error -from .inspection_service import CurrentFileChanged, inspect_current_file +from .inspection_service import ( + CurrentFileChanged, + IMAGE_EXTENSIONS as _IMAGE_EXTENSIONS, + SERVABLE_EXTENSIONS, + VIDEO_EXTENSIONS, + inspect_current_file, +) try: from ...utils.parallel import map_parallel @@ -29,23 +35,8 @@ except ImportError: from utils.parallel import map_parallel from utils.inspection_response import encode_inspection_response -# Only these file types are ever served by the image routes, regardless of -# what sits inside an allowed directory (defence in depth for the gallery). -IMAGE_EXTENSIONS = frozenset( - { - ".png", - ".jpg", - ".jpeg", - ".webp", - ".gif", - ".bmp", - ".tiff", - ".tif", - ".mp4", - ".webm", - ".mov", - } -) + +IMAGE_EXTENSIONS = _IMAGE_EXTENSIONS def _public_path(path): @@ -97,11 +88,7 @@ def _media_access_error(path, allowed_dirs): # Media types the gallery scans and thumbnails (a superset is never served). GALLERY_IMAGE_EXTENSIONS = frozenset({".png", ".jpg", ".jpeg", ".webp", ".gif"}) -VIDEO_EXTENSIONS = frozenset({".mp4", ".webm", ".avi", ".mov", ".mkv", ".m4v", ".wmv"}) MEDIA_EXTENSIONS = GALLERY_IMAGE_EXTENSIONS | VIDEO_EXTENSIONS -# What the serve routes hand out: every image type above plus every video type -# the gallery lists, so a card never points at a 403. -SERVABLE_EXTENSIONS = IMAGE_EXTENSIONS | VIDEO_EXTENSIONS # Caps on request-triggered filesystem work. MAX_SCAN_DEPTH = 12 diff --git a/py/api/inspection_service.py b/py/api/inspection_service.py index 47ca736..5f4e0d7 100644 --- a/py/api/inspection_service.py +++ b/py/api/inspection_service.py @@ -1,5 +1,7 @@ """One bounded current-file inspection worker shared by physical identities.""" +import os +import stat from pathlib import Path try: @@ -18,23 +20,56 @@ except ImportError: from utils.video_metadata import is_video_path +# Only these file types are served or inspected within an authorized directory. +IMAGE_EXTENSIONS = frozenset( + { + ".png", + ".jpg", + ".jpeg", + ".webp", + ".gif", + ".bmp", + ".tiff", + ".tif", + ".mp4", + ".webm", + ".mov", + } +) +VIDEO_EXTENSIONS = frozenset({".mp4", ".webm", ".avi", ".mov", ".mkv", ".m4v", ".wmv"}) +SERVABLE_EXTENSIONS = IMAGE_EXTENSIONS | VIDEO_EXTENSIONS + + class CurrentFileChanged(Exception): - """File facts changed across the bounded metadata observation.""" + """File facts are invalid or changed across the bounded metadata observation.""" + + +def _current_file_facts(path: Path) -> os.stat_result: + if path.suffix.lower() not in SERVABLE_EXTENSIONS: + raise CurrentFileChanged + try: + observed = path.stat() + except (FileNotFoundError, NotADirectoryError, PermissionError): + raise CurrentFileChanged from None + if ( + not stat.S_ISREG(observed.st_mode) + or not observed.st_mode & 0o444 + or not os.access(path, os.R_OK) + ): + raise CurrentFileChanged + return observed def inspect_current_file( path: Path, *, source_label: str, saved: dict | None, library: dict ) -> dict: - before = path.stat() + before = _current_file_facts(path) try: inspection = read_inspection_file(path) except MetadataReadError: inspection = None - try: - after = path.stat() - except FileNotFoundError: - raise CurrentFileChanged from None - fields = ("st_dev", "st_ino", "st_size", "st_mtime_ns", "st_ctime_ns") + after = _current_file_facts(path) + fields = ("st_dev", "st_ino", "st_mode", "st_size", "st_mtime_ns", "st_ctime_ns") if any(getattr(before, key) != getattr(after, key) for key in fields): raise CurrentFileChanged if inspection is None: diff --git a/tests/test_api_history_outputs.py b/tests/test_api_history_outputs.py index 47a6597..3f08e00 100644 --- a/tests/test_api_history_outputs.py +++ b/tests/test_api_history_outputs.py @@ -4,11 +4,13 @@ import asyncio import errno import importlib import json +import os import sqlite3 import threading from contextlib import ExitStack from pathlib import Path from types import SimpleNamespace +from unittest import skipUnless from unittest.mock import patch from aiohttp import web @@ -1088,3 +1090,150 @@ class HistoryOutputAPITests(ImageAPITestCase): body = await response.json() self.assert_error(response, body, 503, "storage-unavailable") self.assertEqual(body["recovery"], {"action": "retry-read"}) + + async def test_inspection_rejects_directory_replacement_before_parser(self): + path = png(self.output_dir / "image.png", prompt=graph("Current portrait")) + start = self.capture([self.ref()]) + descriptor = await self.media(start) + response = await self.current(descriptor["inspection_url"]) + body = await response.json() + self.assertEqual(response.status, 200, body) + self.assertEqual(body["prompts"]["positive"]["text"], "Current portrait") + service = importlib.import_module("py.api.inspection_service") + inspect = service.inspect_current_file + read = service.read_inspection_file + reads = [] + + def replaced(target, **kwargs): + path.unlink() + path.mkdir() + return inspect(target, **kwargs) + + def traced(target): + reads.append(target) + return read(target) + + with patch("py.api.history.inspect_current_file", side_effect=replaced): + with patch.object(service, "read_inspection_file", side_effect=traced): + response = await self.current(descriptor["inspection_url"]) + body = await response.json() + self.assert_error(response, body, 409, "candidate-stale") + self.assertEqual(reads, [], "Nonregular targets must not reach the parser") + self.assertTrue(path.is_dir()) + self.assertNotIn("Current portrait", json.dumps(body)) + + @skipUnless(os.name == "posix", "FIFO replacement is a POSIX filesystem case") + async def test_inspection_rejects_fifo_replacement_before_parser(self): + path = make_png(self.output_dir / "image.png") + start = self.capture([self.ref()]) + descriptor = await self.media(start) + response = await self.current(descriptor["inspection_url"]) + self.assertEqual(response.status, 200, await response.json()) + service = importlib.import_module("py.api.inspection_service") + inspect = service.inspect_current_file + reads = [] + + def replaced(target, **kwargs): + path.unlink() + os.mkfifo(path) + return inspect(target, **kwargs) + + def forbidden(target): + reads.append(target) + raise AssertionError("FIFO must be rejected before a metadata open") + + with patch("py.api.history.inspect_current_file", side_effect=replaced): + with patch.object(service, "read_inspection_file", side_effect=forbidden): + response = await self.current(descriptor["inspection_url"]) + body = await response.json() + self.assert_error(response, body, 409, "candidate-stale") + self.assertEqual(reads, [], "A FIFO must never reach the blocking parser") + + async def test_inspection_rejects_unreadable_file_before_parser(self): + path = png(self.output_dir / "image.png", prompt=graph("Readable portrait")) + start = self.capture([self.ref()]) + descriptor = await self.media(start) + service = importlib.import_module("py.api.inspection_service") + inspect = service.inspect_current_file + read = service.read_inspection_file + access = os.access + revoked = False + reads = [] + + def unreadable(target, **kwargs): + nonlocal revoked + revoked = True + return inspect(target, **kwargs) + + def current_access(target, mode, **kwargs): + if revoked and Path(target) == path: + return False + return access(target, mode, **kwargs) + + def traced(target): + reads.append(target) + return read(target) + + with patch("py.api.history.inspect_current_file", side_effect=unreadable): + with patch("os.access", side_effect=current_access): + with patch.object(service, "read_inspection_file", side_effect=traced): + response = await self.current(descriptor["inspection_url"]) + body = await response.json() + self.assert_error(response, body, 409, "candidate-stale") + self.assertEqual(reads, [], "Unreadable targets must not reach the parser") + response = await self.current(descriptor["inspection_url"]) + body = await response.json() + self.assertEqual(response.status, 200, body) + self.assertEqual(body["prompts"]["positive"]["text"], "Readable portrait") + + async def test_inspection_rejects_directory_replacement_after_parser(self): + path = png(self.output_dir / "image.png", prompt=graph("Current portrait")) + start = self.capture([self.ref()]) + descriptor = await self.media(start) + service = importlib.import_module("py.api.inspection_service") + read = service.read_inspection_file + + def replaced(target): + result = read(target) + path.unlink() + path.mkdir() + return result + + with patch.object(service, "read_inspection_file", side_effect=replaced): + response = await self.current(descriptor["inspection_url"]) + body = await response.json() + self.assert_error(response, body, 409, "candidate-stale") + self.assertNotIn("Current portrait", json.dumps(body)) + + async def test_inspection_rejects_readability_revoked_after_parser(self): + path = png(self.output_dir / "image.png", prompt=graph("Readable portrait")) + before = path.read_bytes() + start = self.capture([self.ref()]) + descriptor = await self.media(start) + service = importlib.import_module("py.api.inspection_service") + read = service.read_inspection_file + access = os.access + revoked = False + + def changed(target): + nonlocal revoked + result = read(target) + revoked = True + return result + + def current_access(target, mode, **kwargs): + if revoked and Path(target) == path: + return False + return access(target, mode, **kwargs) + + with patch.object(service, "read_inspection_file", side_effect=changed): + with patch("os.access", side_effect=current_access): + response = await self.current(descriptor["inspection_url"]) + body = await response.json() + self.assert_error(response, body, 409, "candidate-stale") + self.assertNotIn("Readable portrait", json.dumps(body)) + self.assertEqual(path.read_bytes(), before) + response = await self.current(descriptor["inspection_url"]) + body = await response.json() + self.assertEqual(response.status, 200, body) + self.assertEqual(body["prompts"]["positive"]["text"], "Readable portrait") diff --git a/tests/test_history_current.py b/tests/test_history_current.py index dfc62a8..f65670d 100644 --- a/tests/test_history_current.py +++ b/tests/test_history_current.py @@ -408,6 +408,33 @@ class CurrentLocationTests(unittest.TestCase): self.assertIsNone(result["library"].get("pair_id")) self.assertIsNone(result["saved"].get("pair")) + def test_inspection_rejects_unsupported_suffix_before_parser(self): + service = self.module("py.api.inspection_service") + path = make_png(self.output / "image.png") + observed = service.inspect_current_file( + path, source_label="output", saved=None, library={"status": "none"} + ) + self.assertEqual(observed["file"]["media_type"], "image") + self.assertEqual(observed["inspection"].format, "PNG") + unsupported = self.output / "image.txt" + unsupported.write_bytes(path.read_bytes()) + read = service.read_inspection_file + reads = [] + + def traced(target): + reads.append(target) + return read(target) + + with patch.object(service, "read_inspection_file", side_effect=traced): + with self.assertRaises(service.CurrentFileChanged): + service.inspect_current_file( + unsupported, + source_label="output", + saved=None, + library={"status": "none"}, + ) + self.assertEqual(reads, [], "Unsupported media must not reach the parser") + class HistoryImportTests(unittest.TestCase): def test_api_import_does_not_require_a_running_host_server(self):