diff --git a/__init__.py b/__init__.py index 8ed1def..29c60c6 100644 --- a/__init__.py +++ b/__init__.py @@ -22,22 +22,18 @@ Features: - Comprehensive logging and diagnostics """ -import re from pathlib import Path def get_version(): - """Parse version from pyproject.toml""" + """Parse version from pyproject.toml (``utils/version.py``).""" + from .utils.version import project_version + try: - pyproject_path = Path(__file__).parent / "pyproject.toml" - if pyproject_path.exists(): - content = pyproject_path.read_text() - match = re.search(r'version\s*=\s*["\']([^"\']+)["\']', content) - if match: - return match.group(1) + root = Path(__file__).parent except Exception: - pass - return "unknown" + return "unknown" + return project_version(root) from .prompt_manager import ( # noqa: E402 diff --git a/py/api/__init__.py b/py/api/__init__.py index b6466c1..ac839fc 100644 --- a/py/api/__init__.py +++ b/py/api/__init__.py @@ -6,6 +6,7 @@ Split into domain-specific mixins for maintainability: - AdminRoutesMixin: duplicates, stats, settings, diagnostics, maintenance, backup, scan - LoggingRoutesMixin: log management endpoints - LogCleanupMixin: the reviewed rotated-log cleanup (a maintenance plan) +- DiagnosticsReportMixin: the shareable diagnostic report, without secrets or paths - AutotagRoutesMixin: auto-tagging model management and tagging - JobRoutesMixin: the session job registry behind Activity (py/api/jobs.py) - ImageTagJobsMixin: bulk Add tags / Auto-tag as image-tags jobs @@ -67,6 +68,7 @@ from .maintenance_plans import PlanStore from .jobs import JobRegistry from .logging_routes import LoggingRoutesMixin from .log_cleanup import LogCleanupMixin +from .diagnostics_report import DiagnosticsReportMixin from .autotag_routes import AutotagRoutesMixin from .lora_integration import LoraIntegrationMixin from .lora_import_routes import LoraImportRoutesMixin @@ -326,6 +328,7 @@ class PromptManagerAPI( DuplicateFilesMixin, LoggingRoutesMixin, LogCleanupMixin, + DiagnosticsReportMixin, AutotagRoutesMixin, LoraIntegrationMixin, LoraImportRoutesMixin, @@ -681,6 +684,7 @@ class PromptManagerAPI( self._register_settings_routes(routes) self._register_job_routes(routes) self._register_system_routes(routes) + self._register_diagnostics_report_routes(routes) self._register_library_location_routes(routes) self._register_maintenance_routes(routes) self._register_backup_restore_routes(routes) diff --git a/py/api/admin.py b/py/api/admin.py index 0405822..4ebf8ee 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -489,9 +489,29 @@ def _diagnose_dependencies(): } -def _diagnose_output_dirs(configured_dirs): +def legacy_checks(db_path, public_path=None): + """The four checks of ``GET /diagnostics`` (blocking). + + Shared with the diagnostic report (``py/api/diagnostics_report.py``), + which passes its own ``public_path`` so a folder outside ComfyUI is never + named. + """ + from ..config import GalleryConfig + + return { + "database": _diagnose_database(db_path), + "dependencies": _diagnose_dependencies(), + "comfyui_output": _diagnose_output_dirs( + GalleryConfig.MONITORING_DIRECTORIES, public_path + ), + "image_monitor": _diagnose_image_monitor(public_path), + } + + +def _diagnose_output_dirs(configured_dirs, public_path=None): """Configured gallery roots, ComfyUI's output dir, or relative fallbacks.""" _public_path, _ = _public_helpers() + _public_path = public_path or _public_path candidates = list(configured_dirs) try: import folder_paths @@ -525,9 +545,10 @@ def _diagnose_output_dirs(configured_dirs): } -def _diagnose_image_monitor(): +def _diagnose_image_monitor(public_path=None): """Status of the running image monitor, with public directory names.""" _public_path, _public_error = _public_helpers() + _public_path = public_path or _public_path try: monitor = _current_image_monitor() if monitor is None: @@ -1055,16 +1076,7 @@ class AdminRoutesMixin: async def run_diagnostics(self, request): """Run comprehensive system diagnostics and health checks.""" try: - from ..config import GalleryConfig - - results = { - "database": _diagnose_database(self.db.model.db_path), - "dependencies": _diagnose_dependencies(), - "comfyui_output": _diagnose_output_dirs( - GalleryConfig.MONITORING_DIRECTORIES - ), - "image_monitor": _diagnose_image_monitor(), - } + results = legacy_checks(self.db.model.db_path) return web.json_response({"success": True, "diagnostics": results}) except Exception as e: diff --git a/py/api/diagnostics_report.py b/py/api/diagnostics_report.py new file mode 100644 index 0000000..2866d80 --- /dev/null +++ b/py/api/diagnostics_report.py @@ -0,0 +1,229 @@ +"""The shareable diagnostic report (#329 System S12). + +``GET /prompt_manager/system/diagnostics`` answers a report built on the +server from an explicit list of fields: the project and Viewer.js versions, +Python and the platform, the four checks of the legacy ``GET /diagnostics`` +(one helper, ``admin.legacy_checks``), the database's public info, source +counts, the log configuration and statistics, the last health summary and +the configuration keys that only ``config.json`` holds. It never copies +``PromptManagerConfig.get_config()``: no path leaf is listed, and the CivitAI +key is reported only as ``has_civitai_api_key``. + +Every path in the report goes through ``report_path``: relative to ComfyUI +(or another public anchor), else ``"outside ComfyUI"``, never a basename +that could be a user name. As a backstop, ``scrub`` replaces any string that +still looks like an absolute path, holds the saved key or the home folder, +and counts each replacement in ``redacted`` (0 for a correct builder). + +The report opens the database, so it takes a registry lease: it is refused +(409 ``busy``) while an ``exclusive`` job (a restore) runs, and a restore +waits for it. It is built off the event loop. +""" + +import datetime +import json +import ntpath +import os +import platform +import posixpath +import re +from pathlib import Path + +from aiohttp import web + +from . import admin as _admin +from .job_routes import busy_response +from .jobs import JobBusy +from .system import health_summary + +try: + from ...utils.version import project_version +except ImportError: # loaded as the top-level ``py`` package (tests) + from utils.version import project_version + +DIAGNOSTICS_LEASE = "Diagnostics" +_ROOT = Path(__file__).resolve().parents[2] +VIEWERJS_PROVENANCE = str(_ROOT / "web" / "lib" / "viewerjs" / "provenance.json") +OUTSIDE = "outside ComfyUI" +REMOVED = "[removed]" +# The CivitAI key is searched for only when it is long enough to be a key. +MIN_KEY_LENGTH = 8 +_WINDOWS_ABSOLUTE = re.compile(r"^[A-Za-z]:[\\/]") + + +def report_path(path): + """``path`` relative to a public anchor (POSIX style), else ``outside ComfyUI``.""" + if not path: + return "" + from ..config import GalleryConfig + + try: + canonical = Path(os.path.normcase(os.path.realpath(str(path)))) + except (OSError, ValueError): + return OUTSIDE + for anchor in GalleryConfig.path_anchors(): + if canonical.is_relative_to(Path(anchor)): + return canonical.relative_to(Path(anchor)).as_posix() or "." + return OUTSIDE + + +def _looks_absolute(text): + return ( + posixpath.isabs(text) + or ntpath.isabs(text) + or _WINDOWS_ABSOLUTE.match(text) is not None + or text.startswith("\\\\") + ) + + +def _home_spellings(): + """The home folder as it may be spelled in a string (never ``/`` itself).""" + home = os.path.expanduser("~") + if not home or len(home) < 4 or home in ("/", "\\"): + return () + return tuple({home, home.replace("\\", "/")}) + + +def scrub(report, key): + """``report`` with leaked paths and the key replaced, counted in ``redacted``.""" + key = key if isinstance(key, str) and len(key) >= MIN_KEY_LENGTH else None + homes = _home_spellings() + count = 0 + + def clean(value): + nonlocal count + if isinstance(value, str): + if ( + _looks_absolute(value) + or (key and key in value) + or any(home in value for home in homes) + ): + count += 1 + return REMOVED + return value + if isinstance(value, dict): + return {name: clean(item) for name, item in value.items()} + if isinstance(value, (list, tuple)): + return [clean(item) for item in value] + return value + + cleaned = clean(report) + cleaned["redacted"] = int(cleaned.get("redacted") or 0) + count + return cleaned + + +def viewerjs_version(path=None): + """The pinned Viewer.js version from its provenance file, or ``unknown``.""" + try: + with open(path or VIEWERJS_PROVENANCE, encoding="utf-8") as handle: + version = json.load(handle).get("version") + except (OSError, ValueError, AttributeError): + return "unknown" + return version if isinstance(version, str) and version else "unknown" + + +def public_config(): + """The configuration keys only ``config.json`` holds, from an explicit list.""" + from ..config import GalleryConfig, IntegrationConfig, PromptManagerConfig + + return { + "database": { + "enable_duplicate_detection": ( + PromptManagerConfig.ENABLE_DUPLICATE_DETECTION + ), + "enable_auto_save": PromptManagerConfig.ENABLE_AUTO_SAVE, + }, + "web_ui": { + "result_timeout": PromptManagerConfig.RESULT_TIMEOUT, + "show_test_button": PromptManagerConfig.SHOW_TEST_BUTTON, + "webui_display_mode": PromptManagerConfig.WEBUI_DISPLAY_MODE, + "infinite_scroll": PromptManagerConfig.INFINITE_SCROLL, + }, + "performance": { + "max_search_results": PromptManagerConfig.MAX_SEARCH_RESULTS, + "enable_fuzzy_search": PromptManagerConfig.ENABLE_FUZZY_SEARCH, + "auto_backup_interval": PromptManagerConfig.AUTO_BACKUP_INTERVAL, + "worker_threads": PromptManagerConfig.WORKER_THREADS, + }, + "gallery": { + "processing_delay": GalleryConfig.PROCESSING_DELAY, + "prompt_timeout": GalleryConfig.PROMPT_TIMEOUT, + "cleanup_interval": GalleryConfig.CLEANUP_INTERVAL, + "auto_cleanup_missing_files": GalleryConfig.AUTO_CLEANUP_MISSING_FILES, + "max_image_age_days": GalleryConfig.MAX_IMAGE_AGE_DAYS, + "max_concurrent_processing": GalleryConfig.MAX_CONCURRENT_PROCESSING, + "metadata_extraction_timeout": GalleryConfig.METADATA_EXTRACTION_TIMEOUT, + }, + "integrations": { + "lora_manager_enabled": bool(IntegrationConfig.LORA_MANAGER_ENABLED), + "has_civitai_api_key": bool(IntegrationConfig.CIVITAI_API_KEY), + }, + } + + +def _source_counts(sources): + configured = sources.get("configured") or [] + effective = sources.get("effective") or [] + return { + "configured": len(configured), + "effective": len(effective), + "watching": sum(1 for row in effective if row.get("watch") == "watching"), + "unavailable": sum(1 for row in configured if row.get("valid") is False), + } + + +class DiagnosticsReportMixin: + """Mixin providing ``GET /prompt_manager/system/diagnostics``.""" + + def _register_diagnostics_report_routes(self, routes): + @routes.get("/prompt_manager/system/diagnostics") + async def diagnostics_report_route(request): + return await self.get_diagnostics_report(request) + + async def get_diagnostics_report(self, request): + """The report, built off the loop under a lease (409 ``busy`` in a restore).""" + from ..config import IntegrationConfig + + try: + async with self._jobs.lease(DIAGNOSTICS_LEASE): + report = await self._run_in_executor(self.build_report) + except JobBusy as busy: + return busy_response(busy) + except Exception: + self.logger.exception("The diagnostic report could not be built") + return web.json_response( + { + "success": False, + "code": None, + "error": "The diagnostic report could not be built.", + }, + status=500, + ) + report = scrub(report, IntegrationConfig.CIVITAI_API_KEY) + return web.json_response(report, headers={"Cache-Control": "no-store"}) + + def build_report(self): + """The report's fields (blocking: reads the database and the log folder).""" + db = self.db + manager = self._get_logger_manager() + stats = dict(manager.get_log_stats()) + stats["log_directory"] = report_path(stats.get("log_directory")) + info = dict(db.model.get_database_info() or {}) + if "database_path" in info: + info["database_path"] = report_path(info["database_path"]) + sources, _monitor = self._source_status() + return { + "success": True, + "generated_at": datetime.datetime.now().replace(microsecond=0).isoformat(), + "version": project_version(_ROOT), + "python": platform.python_version(), + "platform": {"system": platform.system(), "release": platform.release()}, + "viewerjs": viewerjs_version(VIEWERJS_PROVENANCE), + "checks": _admin.legacy_checks(db.model.db_path, report_path), + "database_info": info, + "sources": _source_counts(sources), + "logging": {"config": dict(manager.get_config()), "stats": stats}, + "health": health_summary(self._current_health_report()), + "config": public_config(), + "redacted": 0, + } diff --git a/tests/test_api_diagnostics_report.py b/tests/test_api_diagnostics_report.py new file mode 100644 index 0000000..ba8f595 --- /dev/null +++ b/tests/test_api_diagnostics_report.py @@ -0,0 +1,302 @@ +"""The shareable diagnostic report (#329, py/api/diagnostics_report.py). + +``GET /prompt_manager/system/diagnostics`` builds the report on the server +from an explicit list of fields: the project version, the four legacy checks, +public paths only and no secret. A scrub replaces anything that still looks +like an absolute path or holds the CivitAI key and counts it in ``redacted``; +these tests require ``redacted == 0`` so a builder mistake fails instead of +being hidden. +""" + +import json +import ntpath +import os +import posixpath +import re +import sqlite3 +import sys +import unittest +from unittest import mock +from unittest.mock import MagicMock + +_mock_server = MagicMock() +_mock_server.PromptServer.instance.routes = MagicMock() +sys.modules.setdefault("server", _mock_server) + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from py.api import admin, diagnostics_report # noqa: E402 +from py.api.diagnostics_report import report_path, scrub # noqa: E402 +from py.config import ( + GalleryConfig, + IntegrationConfig, + PromptManagerConfig, +) # noqa: E402 + +try: + from tests.test_api_admin import AdminAPITestCase # noqa: E402 +except ImportError: # discovered with tests/ as the top-level directory + from test_api_admin import AdminAPITestCase # noqa: E402 + +REPORT = "/prompt_manager/system/diagnostics" +KEY = "civitai-secret-0123456789abcdef" +SEEDED_PATHS = ( + "/home/someone/private/LoraManager", + "/srv/someone/renders", + "C:\\Users\\someone\\Documents\\renders", + "D:/Users/someone/prompts.db", + "\\\\nas\\share\\someone\\renders", +) +_WINDOWS_ABSOLUTE = re.compile(r"^[A-Za-z]:[\\/]") +TOP_KEYS = { + "success", + "generated_at", + "version", + "python", + "platform", + "viewerjs", + "checks", + "database_info", + "sources", + "logging", + "health", + "config", + "redacted", +} +CONFIG = { + "database": {"enable_duplicate_detection", "enable_auto_save"}, + "web_ui": { + "result_timeout", + "show_test_button", + "webui_display_mode", + "infinite_scroll", + }, + "performance": { + "max_search_results", + "enable_fuzzy_search", + "auto_backup_interval", + "worker_threads", + }, + "gallery": { + "processing_delay", + "prompt_timeout", + "cleanup_interval", + "auto_cleanup_missing_files", + "max_image_age_days", + "max_concurrent_processing", + "metadata_extraction_timeout", + }, + "integrations": {"lora_manager_enabled", "has_civitai_api_key"}, +} + + +def _strings(value): + if isinstance(value, str): + yield value + elif isinstance(value, dict): + for key, item in value.items(): + yield str(key) + yield from _strings(item) + elif isinstance(value, (list, tuple)): + for item in value: + yield from _strings(item) + + +class DiagnosticsReportTestCase(AdminAPITestCase): + async def get_application(self): + app = await super().get_application() + saved = IntegrationConfig.get_config() + self.addCleanup(IntegrationConfig.update_config, saved) + return app + + async def report(self, status=200): + response = await self.client.request("GET", REPORT) + text = await response.text() + self.assertEqual(response.status, status, text) + return json.loads(text), text + + def seed_secrets(self): + IntegrationConfig.update_config( + { + "lora_manager": { + "enabled": True, + "path": SEEDED_PATHS[0], + "civitai_api_key": KEY, + } + } + ) + GalleryConfig.MONITORING_DIRECTORIES = list(SEEDED_PATHS[1:3]) + [ + SEEDED_PATHS[4] + ] + PromptManagerConfig.update_config( + {"database": {"default_path": SEEDED_PATHS[3]}} + ) + + def assert_no_paths_or_key(self, report, text): + self.assertNotIn(KEY, text) + self.assertNotIn(self.tmpdir, text) + self.assertNotIn(self.tmpdir.replace("\\", "/"), text) + for seeded in SEEDED_PATHS: + self.assertNotIn(seeded, text) + self.assertNotIn(json.dumps(seeded)[1:-1], text) + self.assertNotIn("someone", text) + for value in _strings(report): + self.assertFalse(posixpath.isabs(value), value) + self.assertFalse(ntpath.isabs(value), value) + self.assertIsNone(_WINDOWS_ABSOLUTE.match(value), value) + self.assertFalse(value.startswith("\\\\"), value) + + +class TestReport(DiagnosticsReportTestCase): + async def test_report_has_version_viewerjs_and_the_four_checks(self): + report, _text = await self.report() + self.assertEqual(set(report), TOP_KEYS) + self.assertTrue(report["success"]) + with open( + os.path.join(os.path.dirname(os.path.dirname(__file__)), "pyproject.toml"), + encoding="utf-8", + ) as handle: + version = re.search(r'version\s*=\s*["\']([^"\']+)["\']', handle.read()) + self.assertEqual(report["version"], version.group(1)) + self.assertRegex(report["viewerjs"], r"^\d+\.\d+\.\d+$") + self.assertRegex(report["python"], r"^3\.\d+\.\d+") + self.assertEqual(set(report["platform"]), {"system", "release"}) + self.assertEqual( + set(report["checks"]), + {"database", "dependencies", "comfyui_output", "image_monitor"}, + ) + self.assertEqual(report["checks"]["database"]["status"], "ok") + self.assertEqual( + set(report["sources"]), + {"configured", "effective", "watching", "unavailable"}, + ) + self.assertEqual(set(report["logging"]), {"config", "stats"}) + self.assertIsNone(report["health"]) + self.assertEqual(report["redacted"], 0) + self.assertRegex(report["generated_at"], r"^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d$") + + async def test_report_has_no_secret_or_absolute_path(self): + self.seed_secrets() + with mock.patch.object( + admin.sqlite3, + "connect", + side_effect=sqlite3.DatabaseError("file is not a database"), + ): + report, text = await self.report() + self.assertEqual(report["checks"]["database"]["status"], "error") + self.assertTrue(report["config"]["integrations"]["has_civitai_api_key"]) + self.assertTrue(report["config"]["integrations"]["lora_manager_enabled"]) + self.assertEqual(report["sources"]["configured"], 3) + self.assert_no_paths_or_key(report, text) + self.assertEqual(report["redacted"], 0) + + async def test_report_leaves_get_config_unchanged(self): + self.seed_secrets() + before = json.dumps(PromptManagerConfig.get_config(), sort_keys=True) + directories = GalleryConfig.MONITORING_DIRECTORIES + await self.report() + self.assertEqual( + json.dumps(PromptManagerConfig.get_config(), sort_keys=True), before + ) + self.assertIs(GalleryConfig.MONITORING_DIRECTORIES, directories) + self.assertEqual(IntegrationConfig.CIVITAI_API_KEY, KEY) + + def test_outside_path_reads_outside_comfyui(self): + inside = os.path.join(str(self.comfy_dir), "user", "__promptmanager", "logs") + self.assertEqual(report_path(inside), "user/__promptmanager/logs") + self.assertEqual( + report_path(os.path.join(str(self.outside_dir), "vito")), "outside ComfyUI" + ) + self.assertEqual(report_path(""), "") + self.assertEqual(report_path(None), "") + + def test_scrub_removes_a_leaked_path_and_counts_it(self): + leaked = { + "a": "/home/someone/x", + "b": ["C:\\Users\\someone", "D:/data", "\\\\nas\\share", "ok"], + "c": {"d": f"token {KEY} here", "e": 3, "f": None, "g": "relative/path"}, + "redacted": 0, + } + cleaned = scrub(leaked, KEY) + self.assertEqual(cleaned["redacted"], 5) + self.assertEqual(cleaned["a"], "[removed]") + self.assertEqual(cleaned["b"], ["[removed]"] * 3 + ["ok"]) + self.assertEqual( + cleaned["c"], {"d": "[removed]", "e": 3, "f": None, "g": "relative/path"} + ) + # A short key is not searched for (it would match ordinary words). + self.assertEqual(scrub({"x": "abc", "redacted": 0}, "abc")["redacted"], 0) + + async def test_report_refused_while_exclusive_job_runs(self): + async with self.api._jobs.hold( + "restore", "Database restore", admission="exclusive" + ) as restore: + report, _text = await self.report(status=409) + self.assertEqual(report["code"], "busy") + self.assertEqual( + report["running"], {"id": restore.id, "title": "Database restore"} + ) + + async def test_report_lists_config_only_keys(self): + self.seed_secrets() + report, _text = await self.report() + self.assertEqual({k: set(v) for k, v in report["config"].items()}, CONFIG) + self.assertNotIn("default_path", json.dumps(report["config"])) + self.assertNotIn("directories", json.dumps(report["config"])) + self.assertNotIn( + "civitai_api_key", + json.dumps(report["config"]).replace("has_civitai_api_key", ""), + ) + + async def test_report_logging_has_public_directory(self): + report, _text = await self.report() + stats = report["logging"]["stats"] + self.assertFalse(posixpath.isabs(stats["log_directory"])) + self.assertIn("level", report["logging"]["config"]) + + async def test_report_survives_an_unreadable_viewerjs_provenance(self): + with mock.patch.object( + diagnostics_report, + "VIEWERJS_PROVENANCE", + os.path.join(self.tmpdir, "nope.json"), + ): + report, _text = await self.report() + self.assertEqual(report["viewerjs"], "unknown") + + async def test_a_build_failure_is_coded_without_details(self): + with mock.patch.object( + self.api, "build_report", side_effect=RuntimeError(self.tmpdir) + ): + with self.assertLogs("prompt_manager", "ERROR"): + report, text = await self.report(status=500) + self.assertNotIn(self.tmpdir, text) + self.assertFalse(report["success"]) + + +class TestLegacyDiagnostics(DiagnosticsReportTestCase): + async def test_legacy_diagnostics_keeps_its_four_checks(self): + response = await self.client.request("GET", "/prompt_manager/diagnostics") + data = await response.json() + self.assertEqual(response.status, 200) + self.assertEqual(set(data), {"success", "diagnostics"}) + self.assertEqual( + set(data["diagnostics"]), + {"database", "dependencies", "comfyui_output", "image_monitor"}, + ) + report, _text = await self.report() + self.assertEqual(report["checks"]["database"], data["diagnostics"]["database"]) + self.assertEqual( + report["checks"]["dependencies"], data["diagnostics"]["dependencies"] + ) + + async def test_both_routes_build_their_checks_with_one_helper(self): + with mock.patch.object( + admin, "legacy_checks", wraps=admin.legacy_checks + ) as spy: + await self.client.request("GET", "/prompt_manager/diagnostics") + await self.report() + self.assertEqual(spy.call_count, 2) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/version.py b/utils/version.py new file mode 100644 index 0000000..d78c944 --- /dev/null +++ b/utils/version.py @@ -0,0 +1,18 @@ +"""The project version, read from ``pyproject.toml`` (#329).""" + +import re +from pathlib import Path + + +def project_version(root): + """The version in ``root``'s ``pyproject.toml``; ``"unknown"`` on failure.""" + try: + pyproject_path = Path(root) / "pyproject.toml" + if pyproject_path.exists(): + content = pyproject_path.read_text() + match = re.search(r'version\s*=\s*["\']([^"\']+)["\']', content) + if match: + return match.group(1) + except Exception: # nosec B110 - an unreadable file reads as unknown + pass + return "unknown"