release: promote release/0.5.x to main for v0.5.8

CHANGELOG.md

@@ -5,6 +5,19 @@ All notable changes to the Fovea project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.5.8] - 2026-06-30
+
+The 0.5.8 patch is the third and final audit-driven hardening release — the model-service slice — fixing resource and concurrency defects in the Python inference service ([#192](https://github.com/parafovea/fovea/pull/192)). Nothing is breaking.
+
+### Fixed
+
+- The `/detection/detect` and `/tracking/track` routes leaked an OpenCV `VideoCapture` handle (and decoder memory) whenever a frame read or mask decode failed, since the capture was released only on the success path. The capture is now released in a `try/finally` on every exit path, and an open failure raises a clean error (`model-service/.../routes/detection.py`, `tracking.py`).
+- Synchronous VLM, transcription, and diarization inference ran on the asyncio event loop, so a single request froze all concurrent requests, the `/health` probe, and telemetry export until it finished. The blocking calls are now offloaded with `asyncio.to_thread` (`model-service/.../use_cases/summarize_video.py`, `routes/transcribe.py`, `routes/diarize.py`).
+- ffprobe/ffmpeg subprocesses were awaited with no timeout, so a malformed or slow media file could wedge a request indefinitely and leak the child process. These calls now go through a shared helper that bounds the wait and kills and reaps the process on timeout (`model-service/.../services/audio_processing.py`).
+- `ModelManager` had no concurrency guard, so two requests loading the same not-yet-loaded model could both pass the "already loaded?" check and double-load (risking OOM). Load, unload, and eviction are now serialized by a reentrant model lock with a post-acquire re-check (`model-service/.../services/model_management.py`).
+- Id-based video resolution hardcoded `/videos`, returning 404 on any deployment whose video volume is mounted elsewhere; it now reads the configured `video_data_root` (`model-service/.../use_cases/summarize_video.py`).
+- The admin reconfigure token is now compared with `hmac.compare_digest` (constant-time) instead of `!=` (`model-service/.../routes/admin.py`).
+
 ## [0.5.7] - 2026-06-30
 
 The 0.5.7 patch is the second of three audit-driven hardening releases — the frontend slice — fixing data-loss and stale-cache defects in the annotation UI ([#190](https://github.com/parafovea/fovea/pull/190)). Nothing is breaking.

annotation-tool/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@fovea/annotation-tool",
   "private": true,
-  "version": "0.5.7",
+  "version": "0.5.8",
   "type": "module",
   "scripts": {
     "dev": "vite",

docs/docs/project/changelog.md

@@ -11,6 +11,19 @@ All notable changes to the Fovea project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.5.8] - 2026-06-30
+
+The 0.5.8 patch is the third and final audit-driven hardening release — the model-service slice — fixing resource and concurrency defects in the Python inference service ([#192](https://github.com/parafovea/fovea/pull/192)). Nothing is breaking.
+
+### Fixed
+
+- The `/detection/detect` and `/tracking/track` routes leaked an OpenCV `VideoCapture` handle (and decoder memory) whenever a frame read or mask decode failed, since the capture was released only on the success path. The capture is now released in a `try/finally` on every exit path, and an open failure raises a clean error (`model-service/.../routes/detection.py`, `tracking.py`).
+- Synchronous VLM, transcription, and diarization inference ran on the asyncio event loop, so a single request froze all concurrent requests, the `/health` probe, and telemetry export until it finished. The blocking calls are now offloaded with `asyncio.to_thread` (`model-service/.../use_cases/summarize_video.py`, `routes/transcribe.py`, `routes/diarize.py`).
+- ffprobe/ffmpeg subprocesses were awaited with no timeout, so a malformed or slow media file could wedge a request indefinitely and leak the child process. These calls now go through a shared helper that bounds the wait and kills and reaps the process on timeout (`model-service/.../services/audio_processing.py`).
+- `ModelManager` had no concurrency guard, so two requests loading the same not-yet-loaded model could both pass the "already loaded?" check and double-load (risking OOM). Load, unload, and eviction are now serialized by a reentrant model lock with a post-acquire re-check (`model-service/.../services/model_management.py`).
+- Id-based video resolution hardcoded `/videos`, returning 404 on any deployment whose video volume is mounted elsewhere; it now reads the configured `video_data_root` (`model-service/.../use_cases/summarize_video.py`).
+- The admin reconfigure token is now compared with `hmac.compare_digest` (constant-time) instead of `!=` (`model-service/.../routes/admin.py`).
+
 ## [0.5.7] - 2026-06-30
 
 The 0.5.7 patch is the second of three audit-driven hardening releases — the frontend slice — fixing data-loss and stale-cache defects in the annotation UI ([#190](https://github.com/parafovea/fovea/pull/190)). Nothing is breaking.

docs/package.json

@@ -1,6 +1,6 @@
 {
   "name": "docs",
-  "version": "0.5.7",
+  "version": "0.5.8",
   "private": true,
   "scripts": {
     "docusaurus": "docusaurus",

model-service/package.json

@@ -1,6 +1,6 @@
 {
   "name": "fovea-model-service",
-  "version": "0.5.7",
+  "version": "0.5.8",
   "description": "AI model inference service for video annotation",
   "scripts": {
     "dev": "source venv/bin/activate && uvicorn src.main:app --reload --host 0.0.0.0 --port 8000",

model-service/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "fovea-model-service"
-version = "0.5.7"
+version = "0.5.8"
 description = "Model service for fovea video annotation tool"
 requires-python = ">=3.12"
 dependencies = [

model-service/src/application/services/audio_processing.py

@@ -25,6 +25,51 @@ class AudioProcessingError(Exception):
     """Raised when audio processing operations fail."""
 
 
+#: Bounded wait for an ffprobe metadata probe. Probes read only a few
+#: stream headers and should return in well under a second; the ceiling
+#: keeps a wedged ffprobe from hanging a request forever.
+_FFPROBE_TIMEOUT_SECONDS = 30
+
+#: Bounded wait for an ffmpeg decode/transcode that streams full audio.
+#: Larger than the probe ceiling because decoding a long file legitimately
+#: takes longer.
+_FFMPEG_DECODE_TIMEOUT_SECONDS = 300
+
+
+async def _communicate_with_timeout(
+    process: asyncio.subprocess.Process, timeout: float
+) -> tuple[bytes, bytes]:
+    """Await ``process.communicate()`` with a bounded timeout.
+
+    On timeout the process is killed and reaped before the error
+    propagates, so a wedged ffprobe/ffmpeg cannot leak a child process or
+    hang the request indefinitely.
+
+    Parameters
+    ----------
+    process : asyncio.subprocess.Process
+        The running subprocess.
+    timeout : float
+        Maximum seconds to wait for completion.
+
+    Returns
+    -------
+    tuple[bytes, bytes]
+        The process stdout and stderr.
+
+    Raises
+    ------
+    AudioProcessingError
+        If the process does not finish within ``timeout``.
+    """
+    try:
+        return await asyncio.wait_for(process.communicate(), timeout=timeout)
+    except TimeoutError as exc:
+        process.kill()
+        await process.wait()
+        raise AudioProcessingError(f"Audio subprocess timed out after {timeout:g}s") from exc
+
+
 async def has_audio_stream(video_path: str) -> bool:
     """Check if video file contains audio streams.
 
@@ -56,7 +101,7 @@ async def has_audio_stream(video_path: str) -> bool:
                 stdout=asyncio.subprocess.PIPE,
                 stderr=asyncio.subprocess.PIPE,
             )
-            stdout, _ = await process.communicate()
+            stdout, _ = await _communicate_with_timeout(process, _FFPROBE_TIMEOUT_SECONDS)
 
             has_audio = stdout.decode().strip() == "audio"
             span.set_attribute("audio.has_stream", has_audio)
@@ -109,7 +154,7 @@ async def get_audio_info(video_path: str) -> dict[str, str | int | float]:
                 stderr=asyncio.subprocess.PIPE,
             )
 
-            stdout, stderr = await process.communicate()
+            stdout, stderr = await _communicate_with_timeout(process, _FFPROBE_TIMEOUT_SECONDS)
 
             if process.returncode != 0:
                 error_msg = stderr.decode() if stderr else "Unknown error"
@@ -277,7 +322,7 @@ async def chunk_audio_file(
                 stderr=asyncio.subprocess.PIPE,
             )
 
-            stdout, stderr = await process.communicate()
+            stdout, stderr = await _communicate_with_timeout(process, _FFPROBE_TIMEOUT_SECONDS)
 
             if process.returncode != 0:
                 error_msg = stderr.decode() if stderr else "Unknown error"
@@ -351,7 +396,9 @@ async def load_audio_array(
                 stderr=asyncio.subprocess.PIPE,
             )
 
-            stdout, stderr = await process.communicate()
+            stdout, stderr = await _communicate_with_timeout(
+                process, _FFMPEG_DECODE_TIMEOUT_SECONDS
+            )
 
             if process.returncode != 0:
                 error_msg = stderr.decode() if stderr else "Unknown error"
@@ -403,7 +450,7 @@ async def get_audio_duration(audio_path: str) -> float:
                 stderr=asyncio.subprocess.PIPE,
             )
 
-            stdout, stderr = await process.communicate()
+            stdout, stderr = await _communicate_with_timeout(process, _FFPROBE_TIMEOUT_SECONDS)
 
             if process.returncode != 0:
                 error_msg = stderr.decode() if stderr else "Unknown error"

model-service/src/application/services/model_management.py

@@ -12,10 +12,12 @@
 
 from __future__ import annotations
 
+import asyncio
 import logging
 import time
 from collections import OrderedDict
-from collections.abc import Callable
+from collections.abc import AsyncIterator, Callable
+from contextlib import asynccontextmanager
 from pathlib import Path
 from typing import TYPE_CHECKING, Any
 
@@ -166,6 +168,14 @@ def __init__(
         self.model_load_times: dict[str, float] = {}
         self.model_memory_usage: dict[str, int] = {}
 
+        # Serializes load/unload/evict so two concurrent loads of the same
+        # not-yet-loaded task cannot both pass the "already loaded?" check
+        # (the eviction await inside load_model is a yield point). The lock
+        # is reentrant within a single task via _mutate_models so that
+        # load_model can drive eviction/unload while holding it.
+        self._model_lock = asyncio.Lock()
+        self._lock_holder: asyncio.Task[Any] | None = None
+
         self.device = self._detect_device()
         self.cpu_only_mode = self.device == "cpu"
 
@@ -265,39 +275,73 @@ def get_lru_model(self) -> str | None:
 
     # --- Loading / unloading ------------------------------------------------------
 
+    @asynccontextmanager
+    async def _mutate_models(self) -> AsyncIterator[None]:
+        """Hold the model lock for the duration of the block.
+
+        Reentrant within a single asyncio task: ``load_model`` acquires the
+        lock and then calls ``evict_lru_model``/``unload_model``, which use
+        this same guard. The nested calls run in the same task that already
+        holds the lock, so they pass through without re-acquiring (an
+        ``asyncio.Lock`` is not reentrant and would otherwise deadlock).
+        """
+        current = asyncio.current_task()
+        if self._lock_holder is current:
+            yield
+            return
+
+        async with self._model_lock:
+            self._lock_holder = current
+            try:
+                yield
+            finally:
+                self._lock_holder = None
+
     @tracer.start_as_current_span("evict_lru_model")
     async def evict_lru_model(self) -> str | None:
         """Evict the least recently used model from memory."""
-        lru_task = self.get_lru_model()
-        if lru_task is None:
-            logger.warning("No models to evict")
-            return None
+        async with self._mutate_models():
+            lru_task = self.get_lru_model()
+            if lru_task is None:
+                logger.warning("No models to evict")
+                return None
 
-        logger.info(f"Evicting LRU model: {lru_task}")
-        await self.unload_model(lru_task)
-        return lru_task
+            logger.info(f"Evicting LRU model: {lru_task}")
+            await self.unload_model(lru_task)
+            return lru_task
 
     @tracer.start_as_current_span("unload_model")
     async def unload_model(self, task_type: str) -> None:
         """Unload a model from memory."""
-        if task_type not in self.loaded_models:
-            logger.warning(f"Model {task_type} not loaded")
-            return
+        async with self._mutate_models():
+            if task_type not in self.loaded_models:
+                logger.warning(f"Model {task_type} not loaded")
+                return
 
-        logger.info(f"Unloading model: {task_type}")
-        del self.loaded_models[task_type]
-        del self.model_load_times[task_type]
-        del self.model_memory_usage[task_type]
+            logger.info(f"Unloading model: {task_type}")
+            del self.loaded_models[task_type]
+            del self.model_load_times[task_type]
+            del self.model_memory_usage[task_type]
 
-        self._probe().empty_cache()
-        logger.info(f"Model {task_type} unloaded successfully")
+            self._probe().empty_cache()
+            logger.info(f"Model {task_type} unloaded successfully")
 
     @tracer.start_as_current_span("load_model")
     async def load_model(self, task_type: str) -> Any:
         """Load a model for the specified task type."""
         if task_type not in self.tasks:
             raise ValueError(f"Invalid task type: {task_type}")
 
+        async with self._mutate_models():
+            return await self._load_model_locked(task_type)
+
+    async def _load_model_locked(self, task_type: str) -> Any:
+        """Load a model with the model lock already held.
+
+        Re-checks ``loaded_models`` after the lock is acquired so a
+        coroutine that lost the race returns the instance loaded by the
+        winner instead of double-loading and risking OOM.
+        """
         if task_type in self.loaded_models:
             self.loaded_models.move_to_end(task_type)
             logger.info(f"Model {task_type} already loaded, moved to end")

model-service/src/application/use_cases/summarize_video.py

@@ -8,6 +8,7 @@
 
 from __future__ import annotations
 
+import asyncio
 import io
 import logging
 import os
@@ -287,14 +288,18 @@ async def execute_with_vlm(
                 ) = await self._maybe_transcribe(request, video_path, span)
 
                 logger.info(f"Loading VLM model: {model_name}")
-                self._vlm.load()
+                # VLM load/generate/unload are blocking CPU/GPU calls; run
+                # them off the event loop so concurrent requests, /health,
+                # and OTLP export are not starved.
+                await asyncio.to_thread(self._vlm.load)
 
                 try:
                     prompt = get_persona_prompt(persona_role, information_need)
                     logger.info(f"Generating summary with {len(images)} frames")
                     visual_start_time = time.time()
                     overrides = request.generation_overrides
-                    reasoned = self._vlm.generate_reasoned_from_images(
+                    reasoned = await asyncio.to_thread(
+                        self._vlm.generate_reasoned_from_images,
                         images,
                         prompt,
                         max_tokens=(
@@ -361,7 +366,7 @@ async def execute_with_vlm(
                         reasoning_trace=reasoned.thinking,
                     )
                 finally:
-                    self._vlm.unload()
+                    await asyncio.to_thread(self._vlm.unload)
                     logger.info("VLM model unloaded")
 
             except SummarizationError:
@@ -700,21 +705,28 @@ def _safe(value: str) -> str:
     return str(value).replace("\r", "").replace("\n", "")
 
 
-def get_video_path_for_id(video_id: str, data_dir: str = "/videos") -> str | None:
+def get_video_path_for_id(video_id: str, data_dir: str | None = None) -> str | None:
     """Resolve video ID to file path.
 
     Parameters
     ----------
     video_id : str
         Video identifier from request.
-    data_dir : str
-        Base directory containing video files.
+    data_dir : str | None
+        Base directory containing video files. When None, the configured
+        ``video_data_root`` setting is used so deployments whose videos do
+        not live at ``/videos`` resolve correctly.
 
     Returns
     -------
     str | None
         Full path to video file, or None if not found.
     """
+    if data_dir is None:
+        from src.infrastructure.config.settings import get_settings
+
+        data_dir = str(get_settings().video_data_root)
+
     # CodeQL sanitizer: ``re.fullmatch`` with a constant pattern restricts
     # ``video_id`` to a safe character set. The matched branch clears the
     # taint tracker's path-injection flag.

model-service/src/infrastructure/adapters/inbound/fastapi/routes/admin.py

@@ -19,6 +19,7 @@
 
 from __future__ import annotations
 
+import hmac
 import logging
 from typing import Literal
 
@@ -113,7 +114,7 @@ def _require_admin_token(x_admin_token: str | None) -> None:
             status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
             detail="Admin reconfigure is not enabled on this service",
         )
-    if not x_admin_token or x_admin_token != expected:
+    if not x_admin_token or not hmac.compare_digest(x_admin_token or "", expected):
         raise HTTPException(
             status_code=status.HTTP_401_UNAUTHORIZED,
             detail="Invalid or missing admin token",