diff --git a/CHANGELOG.md b/CHANGELOG.md index b837687c..59fd43d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,18 @@ 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.11] - 2026-07-02 + +The 0.5.11 patch is the third and final release remediating the second swarm audit — the model-service slice — fixing event-loop starvation, ffmpeg subprocess leaks, a shared-model inference race, and a DNS-rebinding SSRF gap in the Python inference service ([#198](https://github.com/parafovea/fovea/pull/198)). Nothing is breaking. This completes the second-audit three-patch series (0.5.9 backend, 0.5.10 frontend, 0.5.11 model-service). + +### Fixed + +- The summarize-with-audio pipeline ran Whisper transcription and pyannote diarization directly on the asyncio event loop — the 0.5.8 offload reached the standalone `/transcribe` and `/diarize` routes and the VLM path but missed the summarizer's own audio path — so a `POST /summarize` with audio froze the `/health` probe, metric export, and every concurrent request for the tens of seconds to minutes the inference took. The load/transcribe/diarize/unload sequence now runs on a worker thread via `asyncio.to_thread` (`model-service/src/infrastructure/adapters/outbound/transcriber_whisper.py`). +- Two ffmpeg extraction helpers on the transcription hot path (`extract_audio_track`, `extract_audio_segment`) awaited a timeout that cancelled only the coroutine, leaving the ffmpeg child running as an orphan; they now go through the shared kill-and-reap helper so the timeout path terminates and reaps the subprocess (`model-service/src/application/services/audio_processing.py`). +- The video processor's `extract_audio` and `extract_thumbnail` awaited ffmpeg with no timeout at all, so a malformed or truncated video could wedge the request forever and leak the child; both now use a bounded timeout that kills and reaps on expiry (`model-service/src/infrastructure/adapters/outbound/video/processor.py`). +- The standalone transcribe and diarize routes fetched the single cached model instance and ran its native inference in a worker thread with no serialization, so two concurrent requests called the same non-thread-safe object (faster-whisper / pyannote) at once, risking garbled output or a crash inside the native code. Inference is now serialized per task type by an `asyncio.Lock`, while distinct tasks still run in parallel (`model-service/src/infrastructure/adapters/inbound/fastapi/routes/transcribe.py`, `diarize.py`, `inference_locks.py`). +- The video downloader's SSRF allow-list resolved the host and required a public IP but then fetched by hostname, so DNS could be rebound between the check and the connect (a TOCTOU). The connection is now pinned to the address vetted during preflight while still presenting the hostname for TLS, so a rebind cannot redirect it to a private or internal host (`model-service/src/infrastructure/adapters/outbound/video/downloader.py`). + ## [0.5.10] - 2026-07-01 The 0.5.10 patch is the second of three releases remediating the second swarm audit — the frontend slice — fixing auto-save data-loss and stale-cache defects in the annotation UI ([#196](https://github.com/parafovea/fovea/pull/196)). Several were gaps in the first audit's own frontend fixes, which closed one instance of a defect class without closing every instance. Nothing is breaking. diff --git a/annotation-tool/package.json b/annotation-tool/package.json index ea662a89..ab589e65 100644 --- a/annotation-tool/package.json +++ b/annotation-tool/package.json @@ -1,7 +1,7 @@ { "name": "@fovea/annotation-tool", "private": true, - "version": "0.5.10", + "version": "0.5.11", "type": "module", "scripts": { "dev": "vite", diff --git a/docs/docs/project/changelog.md b/docs/docs/project/changelog.md index fedbcb60..65c400f7 100644 --- a/docs/docs/project/changelog.md +++ b/docs/docs/project/changelog.md @@ -11,6 +11,18 @@ 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.11] - 2026-07-02 + +The 0.5.11 patch is the third and final release remediating the second swarm audit — the model-service slice — fixing event-loop starvation, ffmpeg subprocess leaks, a shared-model inference race, and a DNS-rebinding SSRF gap in the Python inference service ([#198](https://github.com/parafovea/fovea/pull/198)). Nothing is breaking. This completes the second-audit three-patch series (0.5.9 backend, 0.5.10 frontend, 0.5.11 model-service). + +### Fixed + +- The summarize-with-audio pipeline ran Whisper transcription and pyannote diarization directly on the asyncio event loop — the 0.5.8 offload reached the standalone `/transcribe` and `/diarize` routes and the VLM path but missed the summarizer's own audio path — so a `POST /summarize` with audio froze the `/health` probe, metric export, and every concurrent request for the tens of seconds to minutes the inference took. The load/transcribe/diarize/unload sequence now runs on a worker thread via `asyncio.to_thread` (`model-service/src/infrastructure/adapters/outbound/transcriber_whisper.py`). +- Two ffmpeg extraction helpers on the transcription hot path (`extract_audio_track`, `extract_audio_segment`) awaited a timeout that cancelled only the coroutine, leaving the ffmpeg child running as an orphan; they now go through the shared kill-and-reap helper so the timeout path terminates and reaps the subprocess (`model-service/src/application/services/audio_processing.py`). +- The video processor's `extract_audio` and `extract_thumbnail` awaited ffmpeg with no timeout at all, so a malformed or truncated video could wedge the request forever and leak the child; both now use a bounded timeout that kills and reaps on expiry (`model-service/src/infrastructure/adapters/outbound/video/processor.py`). +- The standalone transcribe and diarize routes fetched the single cached model instance and ran its native inference in a worker thread with no serialization, so two concurrent requests called the same non-thread-safe object (faster-whisper / pyannote) at once, risking garbled output or a crash inside the native code. Inference is now serialized per task type by an `asyncio.Lock`, while distinct tasks still run in parallel (`model-service/src/infrastructure/adapters/inbound/fastapi/routes/transcribe.py`, `diarize.py`, `inference_locks.py`). +- The video downloader's SSRF allow-list resolved the host and required a public IP but then fetched by hostname, so DNS could be rebound between the check and the connect (a TOCTOU). The connection is now pinned to the address vetted during preflight while still presenting the hostname for TLS, so a rebind cannot redirect it to a private or internal host (`model-service/src/infrastructure/adapters/outbound/video/downloader.py`). + ## [0.5.10] - 2026-07-01 The 0.5.10 patch is the second of three releases remediating the second swarm audit — the frontend slice — fixing auto-save data-loss and stale-cache defects in the annotation UI ([#196](https://github.com/parafovea/fovea/pull/196)). Several were gaps in the first audit's own frontend fixes, which closed one instance of a defect class without closing every instance. Nothing is breaking. diff --git a/model-service/pyproject.toml b/model-service/pyproject.toml index ac013fb8..8437f11c 100644 --- a/model-service/pyproject.toml +++ b/model-service/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "fovea-model-service" -version = "0.5.10" +version = "0.5.11" description = "Model service for fovea video annotation tool" requires-python = ">=3.12" dependencies = [ diff --git a/model-service/src/application/services/audio_processing.py b/model-service/src/application/services/audio_processing.py index 1ed6c8d4..60df9d01 100644 --- a/model-service/src/application/services/audio_processing.py +++ b/model-service/src/application/services/audio_processing.py @@ -257,7 +257,7 @@ async def extract_audio_segment( stderr=asyncio.subprocess.PIPE, ) - _stdout, stderr = await asyncio.wait_for(process.communicate(), timeout=60) + _stdout, stderr = await _communicate_with_timeout(process, 60) if process.returncode != 0: error_msg = stderr.decode() if stderr else "Unknown error" @@ -273,8 +273,8 @@ async def extract_audio_segment( return output_path - except TimeoutError as e: - raise AudioProcessingError("Segment extraction timed out after 60s") from e + except AudioProcessingError: + raise except Exception as e: raise AudioProcessingError(f"Segment extraction failed: {e}") from e @@ -547,7 +547,7 @@ async def extract_audio_track( stderr=asyncio.subprocess.PIPE, ) - _stdout, stderr = await asyncio.wait_for(process.communicate(), timeout=300) + _stdout, stderr = await _communicate_with_timeout(process, 300) if process.returncode != 0: error_msg = stderr.decode() if stderr else "Unknown error" @@ -562,8 +562,8 @@ async def extract_audio_track( return output_path - except TimeoutError as e: - raise AudioProcessingError("Audio extraction timed out after 300s") from e + except AudioProcessingError: + raise except Exception as e: raise AudioProcessingError(f"Audio extraction failed: {e}") from e diff --git a/model-service/src/infrastructure/adapters/inbound/fastapi/routes/diarize.py b/model-service/src/infrastructure/adapters/inbound/fastapi/routes/diarize.py index 153d2174..d47e5fda 100644 --- a/model-service/src/infrastructure/adapters/inbound/fastapi/routes/diarize.py +++ b/model-service/src/infrastructure/adapters/inbound/fastapi/routes/diarize.py @@ -31,6 +31,7 @@ from pydantic import BaseModel, Field from src.infrastructure.adapters.inbound.fastapi.dependencies import ModelManagerDep # noqa: TC001 +from src.infrastructure.adapters.inbound.fastapi.routes.inference_locks import inference_lock from src.infrastructure.config.settings import get_settings if TYPE_CHECKING: @@ -146,8 +147,11 @@ async def diarize( start = time.time() try: # Diarization is a blocking CPU/GPU call; offload to a worker - # thread so it does not stall the event loop. - result: DiarizationResult = await asyncio.to_thread(model.diarize, audio_path_real) + # thread so it does not stall the event loop. Serialize inference on + # the shared cached pyannote pipeline: two concurrent diarizations + # would otherwise call the same non-thread-safe object at once. + async with inference_lock("speaker_diarization"): + result: DiarizationResult = await asyncio.to_thread(model.diarize, audio_path_real) except Exception as exc: logger.exception("Diarization failed") raise HTTPException(status_code=500, detail=f"Diarization failed: {exc}") from exc diff --git a/model-service/src/infrastructure/adapters/inbound/fastapi/routes/inference_locks.py b/model-service/src/infrastructure/adapters/inbound/fastapi/routes/inference_locks.py new file mode 100644 index 00000000..e4ea70c6 --- /dev/null +++ b/model-service/src/infrastructure/adapters/inbound/fastapi/routes/inference_locks.py @@ -0,0 +1,28 @@ +"""Per-task inference serialization locks. + +The transcribe and diarize routes fetch the single model instance cached in the +`ModelManager` and run its native inference (faster-whisper / pyannote / torch) +inside ``asyncio.to_thread``. Offloading to a worker thread removed the implicit +serialization that running synchronously on the event loop provided, so two +concurrent requests for the same task would now call the SAME model object's +inference method from two threads at once — which those native objects are not +safe for (garbled/interleaved output, or a crash inside CTranslate2/torch). + +These locks serialize inference PER TASK TYPE: only one inference runs at a time +on a given shared model, while different tasks (transcription vs diarization, +which hold distinct model instances) still run in parallel. The service runs on +a single event loop, so a plain per-key :class:`asyncio.Lock` is sufficient; the +locks are created lazily on first use within a running loop. +""" + +from __future__ import annotations + +import asyncio +from collections import defaultdict + +_inference_locks: defaultdict[str, asyncio.Lock] = defaultdict(asyncio.Lock) + + +def inference_lock(task_type: str) -> asyncio.Lock: + """Return the process-wide inference lock for ``task_type``.""" + return _inference_locks[task_type] diff --git a/model-service/src/infrastructure/adapters/inbound/fastapi/routes/transcribe.py b/model-service/src/infrastructure/adapters/inbound/fastapi/routes/transcribe.py index 06d08a75..30d39c5a 100644 --- a/model-service/src/infrastructure/adapters/inbound/fastapi/routes/transcribe.py +++ b/model-service/src/infrastructure/adapters/inbound/fastapi/routes/transcribe.py @@ -23,6 +23,7 @@ from pydantic import BaseModel, Field from src.infrastructure.adapters.inbound.fastapi.dependencies import ModelManagerDep # noqa: TC001 +from src.infrastructure.adapters.inbound.fastapi.routes.inference_locks import inference_lock from src.infrastructure.adapters.outbound.models.audio.base import ( AudioTranscriptionLoader, TranscriptionResult, @@ -130,9 +131,13 @@ async def transcribe( # requests, /health, and OTLP export). Pass language=None when the # caller did not specify one; some loaders (faster-whisper) treat # an empty string as a hard lookup miss rather than "auto-detect". - result: TranscriptionResult = await asyncio.to_thread( - model.transcribe, audio_path_real, language=request.language or None - ) + # Serialize inference on the shared cached model: two concurrent + # transcriptions would otherwise call the same non-thread-safe object + # from two worker threads at once. + async with inference_lock("audio_transcription"): + result: TranscriptionResult = await asyncio.to_thread( + model.transcribe, audio_path_real, language=request.language or None + ) except Exception as exc: logger.exception("Transcription failed") raise HTTPException(status_code=500, detail=f"Transcription failed: {exc}") from exc diff --git a/model-service/src/infrastructure/adapters/outbound/transcriber_whisper.py b/model-service/src/infrastructure/adapters/outbound/transcriber_whisper.py index 17d15baa..a1a0ed68 100644 --- a/model-service/src/infrastructure/adapters/outbound/transcriber_whisper.py +++ b/model-service/src/infrastructure/adapters/outbound/transcriber_whisper.py @@ -7,10 +7,12 @@ from __future__ import annotations +import asyncio import logging import os import tempfile import time +from typing import TYPE_CHECKING import torch @@ -22,6 +24,9 @@ from src.application.services.audio_processing import extract_audio_track, has_audio_stream from src.infrastructure.observability.telemetry import record_inference +if TYPE_CHECKING: + from src.infrastructure.adapters.outbound.models.audio.loader import TranscriptionConfig + logger = logging.getLogger(__name__) @@ -40,11 +45,9 @@ async def transcribe_video( enable_diarization: bool = False, ) -> TranscriptionResultDTO: """Transcribe audio from a video file.""" - from src.domain.entities.architectures import Whisper # noqa: PLC0415 from src.infrastructure.adapters.outbound.models.audio.loader import ( # noqa: PLC0415 AudioFramework, TranscriptionConfig, - WhisperLoader, ) start = time.time() @@ -67,40 +70,75 @@ async def transcribe_video( language=language, device=device, ) - loader = WhisperLoader(Whisper(), config) - loader.load() - try: - with record_inference(task="transcribe", model_id=self._model_id): - result = loader.transcribe(audio_path) - segments = [ - TranscriptSegmentDTO( - start=float(seg.start), - end=float(seg.end), - text=str(seg.text), - confidence=float(getattr(seg, "confidence", 0.0) or 0.0), - ) - for seg in result.segments - ] - - speaker_count: int | None = None - if enable_diarization: - speaker_count = _apply_diarization(audio_path, segments, device) - - return TranscriptionResultDTO( - text=str(result.text), - segments=segments, - language=getattr(result, "language", None), - speaker_count=speaker_count, - processing_time=time.time() - start, - ) - finally: - loader.unload() + # Whisper load/transcribe, pyannote diarization, and unload are heavy + # blocking (CPU/GPU) calls that would run for tens of seconds to + # minutes. Offload the whole sequence to a worker thread so the event + # loop — /health probes, OTLP metric export, other concurrent + # requests — is not frozen for the duration. + text, segments, language_out, speaker_count = await asyncio.to_thread( + _run_transcription, + config, + audio_path, + self._model_id, + device, + enable_diarization, + ) + + return TranscriptionResultDTO( + text=text, + segments=segments, + language=language_out, + speaker_count=speaker_count, + processing_time=time.time() - start, + ) finally: if os.path.exists(audio_path): os.remove(audio_path) +def _run_transcription( + config: TranscriptionConfig, + audio_path: str, + model_id: str, + device: str, + enable_diarization: bool, +) -> tuple[str, list[TranscriptSegmentDTO], str | None, int | None]: + """Load Whisper, transcribe, optionally diarize, and unload — all blocking. + + Runs on a worker thread (via ``asyncio.to_thread``) so the event loop is not + frozen for the duration of inference. Returns the transcript text, segments, + detected language, and speaker count. + """ + from src.domain.entities.architectures import Whisper # noqa: PLC0415 + from src.infrastructure.adapters.outbound.models.audio.loader import ( # noqa: PLC0415 + WhisperLoader, + ) + + loader = WhisperLoader(Whisper(), config) + loader.load() + try: + with record_inference(task="transcribe", model_id=model_id): + result = loader.transcribe(audio_path) + segments = [ + TranscriptSegmentDTO( + start=float(seg.start), + end=float(seg.end), + text=str(seg.text), + confidence=float(getattr(seg, "confidence", 0.0) or 0.0), + ) + for seg in result.segments + ] + + speaker_count: int | None = None + if enable_diarization: + speaker_count = _apply_diarization(audio_path, segments, device) + + return str(result.text), segments, getattr(result, "language", None), speaker_count + finally: + loader.unload() + + def _apply_diarization( audio_path: str, segments: list[TranscriptSegmentDTO], device: str ) -> int | None: diff --git a/model-service/src/infrastructure/adapters/outbound/video/downloader.py b/model-service/src/infrastructure/adapters/outbound/video/downloader.py index 9fd82752..2127bda6 100644 --- a/model-service/src/infrastructure/adapters/outbound/video/downloader.py +++ b/model-service/src/infrastructure/adapters/outbound/video/downloader.py @@ -30,6 +30,7 @@ import aiofiles import aiohttp +from aiohttp.abc import AbstractResolver, ResolveResult logger = logging.getLogger(__name__) @@ -118,12 +119,56 @@ def _resolve_host_addresses(host: str) -> list[str]: return addresses -def _preflight_url(url: str) -> str: +class _PinnedResolver(AbstractResolver): + """aiohttp resolver that pins a single host to a pre-vetted public IP. + + ``_preflight_url`` resolves the host and verifies every address is public, + but the URL it returns still carries the hostname, so a plain + ``session.get`` re-resolves DNS at connect time — a window in which an + allow-listed host can be rebound to a private/internal address (a + DNS-rebinding TOCTOU). Pinning the connection to the address vetted during + preflight closes that window while still presenting the hostname for TLS + SNI and certificate validation. Any host other than the pinned one is + refused. + """ + + def __init__(self, host: str, ip: str) -> None: + self._host = host.lower() + self._ip = ip + self._family = socket.AF_INET6 if ipaddress.ip_address(ip).version == 6 else socket.AF_INET + + async def resolve( + self, host: str, port: int = 0, family: socket.AddressFamily = socket.AF_INET + ) -> list[ResolveResult]: + """Return the pinned address for the pinned host; refuse any other.""" + if host.lower() != self._host: + raise OSError(f"host {host!r} is not the pinned host") + return [ + ResolveResult( + hostname=host, + host=self._ip, + port=port, + family=self._family, + proto=socket.IPPROTO_TCP, + flags=0, + ) + ] + + async def close(self) -> None: + """No resources to release.""" + + +def _preflight_url(url: str) -> tuple[str, str | None]: """Run defense-in-depth URL checks (scheme, host allow-list, DNS, IP). This is *not* the CodeQL-recognised sanitizer — the inline regex fullmatch at the sink is. This helper runs first so bad URLs fail fast with a descriptive error before any network I/O. + + Returns the sanitized URL and, for non-localhost hosts, the single vetted + public IP the connection must be pinned to (defeating a DNS rebind between + this check and the fetch); ``None`` for localhost/127.0.0.1, where no DNS + resolution occurs. """ parsed = urlparse(url) scheme = (parsed.scheme or "").lower() @@ -145,17 +190,21 @@ def _preflight_url(url: str) -> str: if port is not None and port not in _ALLOWED_PORTS.get(scheme, frozenset()): raise ValueError(f"URL not allowed: port {port} is not permitted") + pinned_ip: str | None = None if host not in {"localhost", "127.0.0.1"}: addresses = _resolve_host_addresses(host) if not addresses: raise ValueError("URL not allowed: host did not resolve") if not all(_is_public_ip(addr) for addr in addresses): raise ValueError("URL not allowed: host resolves to a private or reserved IP") + # Pin to the first vetted address; the fetch connects here rather than + # re-resolving, so a rebind cannot redirect it to a private host. + pinned_ip = addresses[0] port_str = f":{port}" if port is not None else "" query = f"?{parsed.query}" if parsed.query else "" path = parsed.path or "" - return f"{scheme}://{host}{port_str}{path}{query}" + return f"{scheme}://{host}{port_str}{path}{query}", pinned_ip async def download_video_if_needed(video_path: str) -> tuple[str, bool]: @@ -184,7 +233,7 @@ async def download_video_if_needed(video_path: str) -> tuple[str, bool]: return video_path, False try: - candidate_url = _preflight_url(video_path) + candidate_url, pinned_ip = _preflight_url(video_path) except ValueError: parsed_attempt = urlparse(video_path) # CodeQL log-injection sanitizer: ``.replace("\r","").replace("\n","")`` @@ -241,8 +290,19 @@ async def download_video_if_needed(video_path: str) -> tuple[str, bool]: if not temp_real.startswith(_TEMP_DIR_PREFIX): raise RuntimeError("tempfile returned a path outside the temp directory") + # Pin the connection to the address vetted in preflight so a DNS rebind + # between the check and the fetch cannot redirect it to a private host. + # Localhost has no DNS to rebind, so it uses the default resolver. + connector = ( + aiohttp.TCPConnector( + resolver=_PinnedResolver(urlparse(candidate_url).hostname or "", pinned_ip) + ) + if pinned_ip is not None + else None + ) + try: - async with aiohttp.ClientSession() as session: + async with aiohttp.ClientSession(connector=connector) as session: async with session.get(candidate_url) as response: response.raise_for_status() diff --git a/model-service/src/infrastructure/adapters/outbound/video/processor.py b/model-service/src/infrastructure/adapters/outbound/video/processor.py index 282b1354..79075f97 100644 --- a/model-service/src/infrastructure/adapters/outbound/video/processor.py +++ b/model-service/src/infrastructure/adapters/outbound/video/processor.py @@ -43,6 +43,40 @@ class VideoProcessingError(Exception): """Raised when video processing operations fail.""" +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 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 + ------ + VideoProcessingError + 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 VideoProcessingError(f"Video subprocess timed out after {timeout:g}s") from exc + + # Roots constrain where videos may be read from and where outputs may be # written. Sourced from the typed settings (which own every environment # read) and resolved here so later validation is symlink-safe. Override via @@ -452,7 +486,7 @@ async def extract_audio( stderr=asyncio.subprocess.PIPE, ) - _stdout, stderr = await process.communicate() + _stdout, stderr = await _communicate_with_timeout(process, 300) if process.returncode != 0: error_msg = stderr.decode() if stderr else "Unknown error" @@ -470,6 +504,8 @@ async def extract_audio( raise VideoProcessingError( "FFmpeg not found. Please install FFmpeg and ensure it's in your PATH." ) from e + except VideoProcessingError: + raise except Exception as e: raise VideoProcessingError(f"Audio extraction failed: {e!s}") from e @@ -565,7 +601,7 @@ async def extract_thumbnail( stderr=asyncio.subprocess.PIPE, ) - _stdout, stderr = await process.communicate() + _stdout, stderr = await _communicate_with_timeout(process, 60) if process.returncode != 0: error_msg = stderr.decode() if stderr else "Unknown error" @@ -583,5 +619,7 @@ async def extract_thumbnail( raise VideoProcessingError( "FFmpeg not found. Please install FFmpeg and ensure it's in your PATH." ) from e + except VideoProcessingError: + raise except Exception as e: raise VideoProcessingError(f"Thumbnail extraction failed: {e!s}") from e diff --git a/model-service/test/infrastructure/adapters/inbound/fastapi/routes/test_inference_locks.py b/model-service/test/infrastructure/adapters/inbound/fastapi/routes/test_inference_locks.py new file mode 100644 index 00000000..c43c5f19 --- /dev/null +++ b/model-service/test/infrastructure/adapters/inbound/fastapi/routes/test_inference_locks.py @@ -0,0 +1,41 @@ +"""Tests for the per-task inference serialization locks.""" + +import asyncio + +import pytest + +from src.infrastructure.adapters.inbound.fastapi.routes.inference_locks import inference_lock + + +def test_same_task_shares_one_lock() -> None: + """Repeated calls for a task return the same lock instance.""" + assert inference_lock("audio_transcription") is inference_lock("audio_transcription") + + +def test_different_tasks_have_distinct_locks() -> None: + """Different task types get distinct locks so they run in parallel.""" + assert inference_lock("audio_transcription") is not inference_lock("speaker_diarization") + + +@pytest.mark.asyncio +async def test_inference_is_serialized_per_task() -> None: + """Two coroutines holding the same task lock never overlap. + + Without the lock the interleaving would be a-start, b-start, a-end, b-end; + the lock forces each critical section to complete before the next begins. + """ + order: list[str] = [] + lock = inference_lock("test_task_serialization") + + async def worker(name: str) -> None: + async with lock: + order.append(f"{name}-start") + await asyncio.sleep(0.01) + order.append(f"{name}-end") + + await asyncio.gather(worker("a"), worker("b")) + + assert order in ( + ["a-start", "a-end", "b-start", "b-end"], + ["b-start", "b-end", "a-start", "a-end"], + ) diff --git a/model-service/test/test_video_downloader.py b/model-service/test/test_video_downloader.py index 1e7a532a..2e03adc5 100644 --- a/model-service/test/test_video_downloader.py +++ b/model-service/test/test_video_downloader.py @@ -8,6 +8,8 @@ import pytest from src.infrastructure.adapters.outbound.video.downloader import ( + _PinnedResolver, + _preflight_url, cleanup_temp_video, download_video_if_needed, ) @@ -353,3 +355,48 @@ def test_cleanup_with_permission_error(self): # Cleanup for real if os.path.exists(temp_path): os.unlink(temp_path) + + +_RESOLVE = "src.infrastructure.adapters.outbound.video.downloader._resolve_host_addresses" + + +class TestPreflightPinning: + """Preflight returns the vetted address the fetch pins to (DNS-rebind defense).""" + + def test_preflight_returns_pinned_public_ip(self): + """A public host resolves to a vetted IP the fetch will pin to.""" + with patch(_RESOLVE, return_value=["8.8.8.8"]): + url, pinned = _preflight_url(TEST_S3_URL_HTTPS) + assert pinned == "8.8.8.8" + assert url == TEST_S3_URL_HTTPS + + def test_preflight_localhost_is_not_pinned(self): + """Localhost has no DNS to rebind, so no pin is returned.""" + _url, pinned = _preflight_url(TEST_LOCALHOST_URL) + assert pinned is None + + def test_preflight_rejects_private_resolution(self): + """A host that resolves to a private IP is refused.""" + with patch(_RESOLVE, return_value=["10.0.0.5"]): + with pytest.raises(ValueError, match="private or reserved"): + _preflight_url(TEST_S3_URL_HTTPS) + + +class TestPinnedResolver: + """The connection resolver pins to the vetted IP and refuses a rebound host.""" + + @pytest.mark.asyncio + async def test_resolves_pinned_host_to_vetted_ip(self): + resolver = _PinnedResolver("bucket.s3.us-east-1.amazonaws.com", "8.8.8.8") + results = await resolver.resolve("bucket.s3.us-east-1.amazonaws.com", 443) + assert results[0]["host"] == "8.8.8.8" + assert results[0]["port"] == 443 + await resolver.close() + + @pytest.mark.asyncio + async def test_refuses_a_rebound_host(self): + """A different host at connect time (a rebind) is refused.""" + resolver = _PinnedResolver("bucket.s3.us-east-1.amazonaws.com", "8.8.8.8") + with pytest.raises(OSError, match="not the pinned host"): + await resolver.resolve("evil.internal.example", 443) + await resolver.close() diff --git a/package.json b/package.json index 18514955..4b71e743 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "fovea", "private": true, - "version": "0.5.10", + "version": "0.5.11", "description": "FOVEA - Flexible Ontology Visual Event Analyzer", "packageManager": "pnpm@10.15.0", "engines": { diff --git a/server/package.json b/server/package.json index e9002c1a..82889fad 100644 --- a/server/package.json +++ b/server/package.json @@ -1,6 +1,6 @@ { "name": "@fovea/server", - "version": "0.5.10", + "version": "0.5.11", "private": true, "type": "module", "scripts": {