Security
--------
ComfyUI's PromptServer.add_routes() mirrors every registered route under a
second "/api" prefix, so /mjr/am/<mutation> is also served at
/api/mjr/am/<mutation>. The auth middleware matched only the canonical
prefix, so the mirrored path reached the same handler with the
authentication check skipped entirely, and without the hardening headers:
POST /mjr/am/assets/delete -> requires_auth = True
POST /api/mjr/am/assets/delete -> requires_auth = False
Replaced the prefix test with a shared _is_majoor_path() boundary covering
the canonical prefixes, ComfyUI's /api mirror, and the /api/v2/assets
compat surface. Verified it does not capture ComfyUI's own routes. CSRF,
rate limiting and safe-mode gates are per-handler and were unaffected.
Licensing
---------
PrimeVue 5 is no longer MIT: PrimeTek moved it to the commercial "PrimeUI"
license, which requires a key, shows a license notice without one, and
forbids redistributing it "as a component library or development tool"
without a separate OEM License. Dependabot had auto-merged 4.5.5 -> 5.0.1.
This project is GPL-3.0 and commits dist/ to the repo, so shipping it would
nag every user and conflict with the GPL's redistribution requirement.
Pinned back to 4.5.5 (last MIT release). The 5.x bump had touched only
package.json/package-lock.json, so no source depended on v5 APIs. The
revert drops @primeui/license-manager and its @noble/ed25519 key-signature
stack; the primevue chunk shrinks 555.70 -> 520.61 kB. Dependabot is now
configured to ignore primevue major bumps.
Correctness
-----------
- api_v2_assets: stop returning the DB's absolute filepath on this
network-reachable surface; subfolder + root_id already convey location
without disclosing local disk layout.
- watcher: enforce _RECENT_GENERATED_HARD_CAP after inserting a batch;
pruning only beforehand let a large batch overshoot the cap.
- routes/registry: track registered route groups individually so a group
failing part-way through does not re-register earlier groups on retry.
- fs_walker/scanner: apply the viewer's root check to symlinked files, so
the scanner stops indexing symlinks whose target escapes every allowed
root ("ghost assets" the viewer then refuses to open). Consulted only for
actual symlinks, so ordinary scans pay no extra I/O.
- routes/registry_app: skip the redundant app.add_routes() on the live
PromptServer app; ComfyUI adds the shared table itself after node
loading, so every core and Majoor route was registered twice at startup.
- Result: unwrap()/unwrap_or()/map() gated on `data is not None`, treating
a legitimate Ok(None) as failure; gate on `ok` instead.
- custom_roots: replace a leftover debug print() that wrote user file paths
to stderr on every move request with logger.debug.
- schema: drop the redundant UNIQUE(filepath, source, root_id) index
(migration m022). assets.filepath is already UNIQUE, so it could never
reject a row the column constraint does not, nor narrow a lookup - it
only cost a second B-tree write per asset on the scan hot path.
Frontend
--------
- entry.ts: app.registerExtension() throws on a duplicate name and ComfyUI
exposes no unregisterExtension, so a live re-import threw before setup()
ran and the reload-safe teardown never executed. Catch it and re-run
setup() directly.
- Rebuilt dist/ so both the reload fix and the PrimeVue revert ship.
CI / tooling
------------
- frontend-compat-nightly: the types step died with a bare "cp: cannot
stat" (upstream's build has not emitted dist/index.d.ts since Aug 26).
Check both candidate paths and dump dist/ with an actionable message.
- package.json: lenient checks that swallowed their exit code are now
suffixed :advisory, the plain names are strict, and quality:strict runs
the full set.
Verified: 1841 Python tests pass (5 skipped, symlink privileges), ruff
clean; tsc --noEmit clean, 831 JS tests pass, eslint clean, build succeeds.
333 lines
14 KiB
Python
333 lines
14 KiB
Python
"""
|
|
Tests for degraded service states — Phase 2 chaos coverage.
|
|
|
|
Verifies that the system survives and reports correctly when:
|
|
- DB cannot be opened (path unwritable / locked)
|
|
- exiftool is absent from PATH
|
|
- watcher crashes on start
|
|
- services are built twice (idempotency)
|
|
- DB migration fails
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Helpers
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def _ok_health_result():
|
|
from mjr_am_backend.shared import Result
|
|
return Result.Ok({
|
|
"overall": "healthy",
|
|
"db": {"locked": False},
|
|
"index": {"enrichment_active": False},
|
|
})
|
|
|
|
|
|
def _err_result(code: str, msg: str):
|
|
from mjr_am_backend.shared import Result
|
|
return Result.Err(code, msg)
|
|
|
|
|
|
def _ok_bool_result():
|
|
from mjr_am_backend.shared import Result
|
|
return Result.Ok(True)
|
|
|
|
|
|
def _ok_db_result(tmp_path: Path):
|
|
"""Return a Result.Ok wrapping a minimal mock Sqlite-like object."""
|
|
from mjr_am_backend.shared import Result
|
|
|
|
db = MagicMock()
|
|
db.aclose = AsyncMock()
|
|
db.get_runtime_status = MagicMock(return_value={})
|
|
return Result.Ok(db)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 1. DB cannot be opened — build_services returns Err
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestDBUnavailable:
|
|
@pytest.mark.asyncio
|
|
async def test_build_services_returns_err_on_db_failure(self, tmp_path):
|
|
"""Simulate SQLite refusing to open (e.g. path in a read-only dir)."""
|
|
from mjr_am_backend import deps
|
|
|
|
unwritable = tmp_path / "locked_dir" / "index.db"
|
|
with patch.object(deps, "_init_db_or_error", return_value=_err_result("DB_ERROR", "DB locked")):
|
|
result = await deps.build_services(str(unwritable))
|
|
assert not result.ok
|
|
assert result.code == "DB_ERROR"
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_require_services_returns_err_when_services_none(self):
|
|
"""_require_services yields Err when services are None (uninitialized/failed)."""
|
|
from mjr_am_backend.routes.core import services as svc_mod
|
|
|
|
with patch.object(svc_mod, "_services", None):
|
|
with patch.object(svc_mod, "_services_error", "DB_ERROR: DB locked"):
|
|
with patch.object(svc_mod, "_build_services", AsyncMock(return_value=None)):
|
|
svc, err = await svc_mod._require_services()
|
|
assert svc is None
|
|
assert err is not None
|
|
assert not err.ok
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 2. ExifTool absent — metadata degrades gracefully
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestExiftoolAbsent:
|
|
def test_tool_detect_returns_false_when_binary_missing(self):
|
|
"""Simulate exiftool absent by patching the probe function and clearing cache."""
|
|
from mjr_am_backend import tool_detect
|
|
|
|
# Reset cache so the function actually checks
|
|
original_cache = dict(tool_detect._TOOL_CACHE)
|
|
tool_detect._TOOL_CACHE["exiftool"] = None
|
|
try:
|
|
with patch.object(tool_detect, "_probe_exiftool_candidates", return_value=(False, None, "")):
|
|
assert not tool_detect.has_exiftool()
|
|
finally:
|
|
tool_detect._TOOL_CACHE.update(original_cache)
|
|
|
|
def test_probe_router_returns_empty_when_no_tools(self):
|
|
"""probe_router imports has_exiftool/has_ffprobe directly — patch there."""
|
|
from mjr_am_backend import probe_router
|
|
|
|
with patch.object(probe_router, "has_exiftool", return_value=False):
|
|
with patch.object(probe_router, "has_ffprobe", return_value=False):
|
|
result = probe_router.pick_probe_backend("test.png")
|
|
assert result == [], "should return empty list, not raise"
|
|
|
|
def test_probe_router_falls_back_to_ffprobe_when_exiftool_missing(self):
|
|
from mjr_am_backend import probe_router
|
|
|
|
with patch.object(probe_router, "has_exiftool", return_value=False):
|
|
with patch.object(probe_router, "has_ffprobe", return_value=True):
|
|
result = probe_router.pick_probe_backend("video.mp4")
|
|
assert "ffprobe" in result
|
|
|
|
def test_probe_router_single_mode_falls_back_to_available(self):
|
|
"""When exiftool is requested but missing, fall back to ffprobe."""
|
|
from mjr_am_backend import probe_router
|
|
|
|
with patch.object(probe_router, "has_exiftool", return_value=False):
|
|
with patch.object(probe_router, "has_ffprobe", return_value=True):
|
|
result = probe_router.pick_probe_backend("test.png", settings_override="exiftool")
|
|
assert result == ["ffprobe"]
|
|
|
|
def test_probe_router_both_mode_empty_when_nothing_available(self):
|
|
from mjr_am_backend import probe_router
|
|
|
|
with patch.object(probe_router, "has_exiftool", return_value=False):
|
|
with patch.object(probe_router, "has_ffprobe", return_value=False):
|
|
result = probe_router.pick_probe_backend("test.png", settings_override="both")
|
|
assert result == []
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 3. Watcher safe helpers — must never raise
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestWatcherHelpers:
|
|
def test_safe_watcher_is_running_false_for_stopped_watcher(self):
|
|
from mjr_am_backend.routes.handlers.health import _safe_watcher_is_running
|
|
|
|
watcher = MagicMock()
|
|
watcher.is_running = False
|
|
assert _safe_watcher_is_running(watcher) is False
|
|
|
|
def test_safe_watcher_is_running_callable_returns_false(self):
|
|
from mjr_am_backend.routes.handlers.health import _safe_watcher_is_running
|
|
|
|
watcher = MagicMock()
|
|
watcher.is_running = lambda: False
|
|
assert _safe_watcher_is_running(watcher) is False
|
|
|
|
def test_safe_watcher_is_running_crash_returns_false(self):
|
|
"""Even if watcher.is_running raises, the helper must return False."""
|
|
from mjr_am_backend.routes.handlers.health import _safe_watcher_is_running
|
|
|
|
broken = MagicMock()
|
|
broken.is_running = MagicMock(side_effect=RuntimeError("crash"))
|
|
assert _safe_watcher_is_running(broken) is False
|
|
|
|
def test_safe_watcher_pending_count_crash_returns_zero(self):
|
|
from mjr_am_backend.routes.handlers.health import _safe_watcher_pending_count
|
|
|
|
broken = MagicMock()
|
|
broken.get_pending_count = MagicMock(side_effect=RuntimeError("crash"))
|
|
assert _safe_watcher_pending_count(broken) == 0
|
|
|
|
def test_safe_watcher_directories_crash_returns_empty(self):
|
|
from mjr_am_backend.routes.handlers.health import _safe_watcher_directories
|
|
|
|
broken = MagicMock()
|
|
broken.watched_directories = MagicMock(side_effect=RuntimeError("crash"))
|
|
result = _safe_watcher_directories(broken)
|
|
assert result == []
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 4. Services built twice — idempotency of register_routes
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestRoutesIdempotency:
|
|
@pytest.mark.asyncio
|
|
async def test_register_routes_twice_same_app_no_raise(self):
|
|
"""
|
|
Calling register_routes(app) twice for the same app must not raise and
|
|
must not double-register (the AppKey guard prevents it).
|
|
"""
|
|
from aiohttp import web
|
|
from mjr_am_backend.routes import registry
|
|
|
|
app = web.Application()
|
|
call_count = {"n": 0}
|
|
|
|
def _mock_prepare(a, um):
|
|
call_count["n"] += 1
|
|
return False # simulate "already registered"
|
|
|
|
with patch.object(registry, "_prepare_route_table", side_effect=_mock_prepare):
|
|
registry.register_routes(app)
|
|
registry.register_routes(app)
|
|
|
|
assert call_count["n"] == 2 # called twice but both returned False → no double-register
|
|
|
|
def test_registry_routes_registered_flag_is_idempotent(self):
|
|
"""
|
|
register_all_routes() must skip silently if called a second time.
|
|
"""
|
|
from mjr_am_backend.routes import registry
|
|
|
|
original_flag = registry._ROUTES_REGISTERED
|
|
try:
|
|
registry._ROUTES_REGISTERED = True
|
|
# Should return without raising and without doing any work
|
|
with patch.object(registry, "_get_prompt_server") as mock_ps:
|
|
mock_routes = MagicMock()
|
|
mock_ps.return_value.instance.routes = mock_routes
|
|
registry.register_all_routes()
|
|
# register_fn on any route group must NOT have been called
|
|
mock_routes.assert_not_called()
|
|
finally:
|
|
registry._ROUTES_REGISTERED = original_flag
|
|
|
|
def test_core_route_group_failure_does_not_duplicate_on_retry(self):
|
|
"""A core group raising part-way must not cause the groups registered
|
|
before it to be added a second time when registration is retried.
|
|
|
|
register_fn() mutates the shared RouteTableDef in place, so re-running
|
|
an already-applied group would register duplicate routes on the app.
|
|
"""
|
|
from mjr_am_backend.routes import registry
|
|
from mjr_am_backend.routes.route_catalog import RouteRegistration
|
|
|
|
applied: list[str] = []
|
|
should_fail = {"value": True}
|
|
|
|
def _ok_a(_routes):
|
|
applied.append("group-a")
|
|
|
|
def _ok_b(_routes):
|
|
applied.append("group-b")
|
|
|
|
def _flaky(_routes):
|
|
applied.append("group-flaky")
|
|
if should_fail["value"]:
|
|
raise RuntimeError("boom")
|
|
|
|
fake_core = (
|
|
RouteRegistration("group-a", _ok_a),
|
|
RouteRegistration("group-flaky", _flaky),
|
|
RouteRegistration("group-b", _ok_b),
|
|
)
|
|
|
|
original_flag = registry._ROUTES_REGISTERED
|
|
original_groups = set(registry._REGISTERED_ROUTE_GROUPS)
|
|
try:
|
|
registry._ROUTES_REGISTERED = False
|
|
registry._REGISTERED_ROUTE_GROUPS.clear()
|
|
with (
|
|
patch.object(registry, "CORE_ROUTE_REGISTRATIONS", fake_core),
|
|
patch.object(registry, "OPTIONAL_ROUTE_REGISTRATIONS", ()),
|
|
patch.object(registry, "_get_prompt_server") as mock_ps,
|
|
):
|
|
mock_ps.return_value.instance.routes = MagicMock()
|
|
|
|
with pytest.raises(RuntimeError):
|
|
registry.register_all_routes()
|
|
assert applied == ["group-a", "group-flaky"]
|
|
|
|
# Retry once the transient failure clears.
|
|
should_fail["value"] = False
|
|
registry.register_all_routes()
|
|
|
|
# group-a must NOT have been re-applied; the retry resumes at the
|
|
# failed group and continues past it.
|
|
assert applied == ["group-a", "group-flaky", "group-flaky", "group-b"]
|
|
finally:
|
|
registry._ROUTES_REGISTERED = original_flag
|
|
registry._REGISTERED_ROUTE_GROUPS.clear()
|
|
registry._REGISTERED_ROUTE_GROUPS.update(original_groups)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 5. DB migration failure — structured Err, not raw exception
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestDBMigrationFailure:
|
|
@pytest.mark.asyncio
|
|
async def test_build_services_returns_err_on_migration_failure(self, tmp_path):
|
|
"""Migration failure must produce Result.Err, not an uncaught exception."""
|
|
from mjr_am_backend import deps
|
|
|
|
db_path = str(tmp_path / "index.db")
|
|
|
|
with patch.object(deps, "_migrate_db_or_error", AsyncMock(
|
|
return_value=_err_result("DB_ERROR", "migration: no such table")
|
|
)):
|
|
result = await deps.build_services(db_path)
|
|
assert not result.ok
|
|
assert result.code == "DB_ERROR"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 6. Vector service absent — health endpoint stays ok/degraded, not fatal
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestVectorServiceAbsent:
|
|
def test_vector_runtime_diagnostics_no_service_returns_disabled(self):
|
|
from mjr_am_backend.routes.handlers.health import _vector_runtime_diagnostics
|
|
|
|
diag = _vector_runtime_diagnostics({"vector_service": None})
|
|
assert diag["enabled"] is False
|
|
assert diag["degraded"] is False # absent ≠ broken
|
|
|
|
def test_vector_runtime_diagnostics_with_degraded_service(self):
|
|
from mjr_am_backend.routes.handlers.health import _vector_runtime_diagnostics
|
|
|
|
svc = MagicMock()
|
|
svc.get_runtime_status = MagicMock(return_value={"degraded": True, "last_error": "OOM"})
|
|
diag = _vector_runtime_diagnostics({"vector_service": svc})
|
|
assert diag["degraded"] is True
|
|
assert diag["last_error"] == "OOM"
|
|
|
|
def test_vector_runtime_diagnostics_crash_in_service_returns_fallback(self):
|
|
from mjr_am_backend.routes.handlers.health import _vector_runtime_diagnostics
|
|
|
|
svc = MagicMock()
|
|
svc.get_runtime_status = MagicMock(side_effect=RuntimeError("crash"))
|
|
diag = _vector_runtime_diagnostics({"vector_service": svc})
|
|
# Must return a safe dict, not raise
|
|
assert isinstance(diag, dict)
|
|
assert "enabled" in diag
|