preserve(security): bounded diagnostic findings for canonical #1055

.jules/sentinel.md

@@ -6,7 +6,7 @@
 ## 2024-07-07 - Unsanitized Directory Input Paths API Validation
 **Vulnerability:** The API logic allowed user-controlled local data directory paths (`cacheRoot` and `tempRoot`) to be directly used without mitigating cross-platform path traversal vulnerabilities.
 **Learning:** Checking for '..' sequences in untrusted paths fails to parse cross-platform separators reliably for untrusted inputs (e.g., Windows backslashes on POSIX). Relying solely on `os.sep` or `os.altsep` is inadequate because absolute paths can bypass restrictions if not resolved correctly, or if `os.altsep` is None.
-**Prevention:** Manually replace backslashes with forward slashes and split by forward slash (e.g., `if '..' in path.replace('\\', '/').split('/')`) to enforce path traversal protections explicitly for restricted directory inputs provided via the API. Do not block `~` for user-selected input files.
+**Prevention:** Manually replace backslashes with forward slashes and split by forward slash (e.g. `if '..' in path.replace('\\', '/').split('/')`) to enforce path traversal protections explicitly for restricted directory inputs provided via the API. Do not block `~` for user-selected input files.
 
 ## 2024-05-20 - Python Path Traversal Mitigation bypass
 **Vulnerability:** Path traversal detection in Python backend APIs relied solely on checking the input path string or basic parsed parts which might not adequately catch sequences like `..` when intermixed with different path separators.
@@ -28,3 +28,8 @@
 **Vulnerability:** The Rust backend (`apps/desktop/src-tauri/src/main.rs`) did not enforce a maximum URL length limit when processing YouTube URLs via `import_youtube_url`. While the frontend enforced `MAX_YOUTUBE_URL_LENGTH = 2000` via the input element, this could be bypassed by an attacker sending requests directly to the Tauri backend API, potentially causing a Denial of Service (DoS) due to unbounded URL parsing and regex matching.
 **Learning:** Input validation must occur at the entry point of untrusted data on the backend, even if it is also validated on the frontend. Relying solely on frontend validation for constraints like string length can expose the backend to resource exhaustion vulnerabilities.
 **Prevention:** Always enforce constraints like maximum length, format validation, and sanitization at the earliest possible point on the backend, typically at the API boundary, regardless of frontend safeguards.
+
+## 2026-09-21 - Prevent log forging and excessive diagnostic output
+**Vulnerability:** Untrusted file names, paths, or decoder exception text could carry carriage returns, line separators, terminal controls, or oversized payloads into line-oriented logs or the analyzer's wrapped failure text. Invoking an arbitrary dependency exception's `__repr__` or `__str__` while handling a failure also executes dependency-controlled code on an error path and can replace the original decoder failure or allocate an unbounded representation before sanitization.
+**Learning:** Deferred logging interpolation avoids eager template construction but does not neutralize controls or bound diagnostic size. Sanitizing the result of `repr()` or `str()` is too late to make arbitrary dependency representation code a trusted boundary. Diagnostics should consume bounded plain-text fields and code-owned exception metadata without invoking dependency `str`/`repr`; control escaping and size limits belong before both log formatting and error-envelope construction.
+**Prevention:** Keep log templates parameterized. Route buyer/dependency text through the shared bounded single-line renderer, which stops consuming input when the rendered budget is exhausted and marks truncation. For arbitrary exceptions, read only the runtime type name and an exact-string base exception argument; do not call dependency `__repr__` or `__str__` for the log record or the analyzer's wrapped `ValueError`. Regression tests cover LF/CRLF, tab, ANSI ESC, NUL/C0, Unicode line separators, printable Unicode, hostile/broken `__repr__`, broken `__str__`, oversized exception messages, and oversized buyer-controlled file names. One logical event must remain one bounded physical log record, and diagnostic handling must not mask the original decoder failure.

services/analysis-engine/src/bandscope_analysis/cli.py

@@ -8,6 +8,7 @@
 from datetime import UTC, datetime
 
 from bandscope_analysis.api import get_analysis_status, run_analysis_job, run_analysis_job_updates
+from bandscope_analysis.logging_safety import safe_log_value
 from bandscope_analysis.temporal import TemporalAnalyzer
 
 logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s")
@@ -86,15 +87,15 @@ def main() -> int:
         audio_path = local_source.get("sourcePath")
         file_name = local_source.get("fileName", "selected audio")
         if audio_path:
-            logging.info("Extracting temporal features from %s...", file_name)
+            logging.info("Extracting temporal features from %s...", safe_log_value(file_name))
             try:
                 temporal_analyzer = TemporalAnalyzer()
                 features = temporal_analyzer.analyze(audio_path)
-                logging.info(f"Extracted BPM: {features['bpm']}")
+                logging.info("Extracted BPM: %s", features["bpm"])
             except Exception:
                 logging.warning(
                     "Temporal analysis failed for %s; continuing with safe fallback.",
-                    file_name,
+                    safe_log_value(file_name),
                 )
 
     requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z")

services/analysis-engine/src/bandscope_analysis/logging_safety.py

@@ -0,0 +1,89 @@
+"""Bounded rendering helpers for untrusted analysis diagnostics."""
+
+from __future__ import annotations
+
+MAX_LOG_DIAGNOSTIC_CHARS = 1024
+MAX_LOG_TYPE_CHARS = 128
+_LOG_TRUNCATION_SUFFIX = "...<truncated>"
+
+
+def _safe_type_name(value: object) -> str:
+    """Read a bounded runtime type name without invoking instance or metaclass hooks."""
+    value_type = object.__getattribute__(value, "__class__")
+    type_name = type.__getattribute__(value_type, "__name__")
+    if type(type_name) is not str:
+        return "unknown"
+    return single_line_log_text(type_name, max_chars=MAX_LOG_TYPE_CHARS)
+
+
+def single_line_log_text(
+    value: str,
+    *,
+    max_chars: int = MAX_LOG_DIAGNOSTIC_CHARS,
+) -> str:
+    """Escape controls and cap one untrusted textual diagnostic field.
+
+    The function stops reading input once the rendered budget is exhausted, so
+    oversized buyer/dependency text cannot force a full-size escaped copy solely
+    for diagnostics.
+    """
+    if max_chars < len(_LOG_TRUNCATION_SUFFIX):
+        raise ValueError("max_chars is too small for the truncation marker")
+
+    chunks: list[str] = []
+    rendered_chars = 0
+    truncated = False
+    for character in value:
+        safe_character = (
+            character
+            if character.isprintable()
+            else character.encode("unicode_escape").decode("ascii")
+        )
+        if rendered_chars + len(safe_character) > max_chars:
+            truncated = True
+            break
+        chunks.append(safe_character)
+        rendered_chars += len(safe_character)
+
+    if truncated:
+        while chunks and rendered_chars + len(_LOG_TRUNCATION_SUFFIX) > max_chars:
+            rendered_chars -= len(chunks.pop())
+        chunks.append(_LOG_TRUNCATION_SUFFIX)
+
+    return "".join(chunks)
+
+
+def safe_log_value(value: object) -> str:
+    """Render an untrusted log value without executing arbitrary representation code."""
+    if type(value) is str:
+        return single_line_log_text(value)
+    return f"<{_safe_type_name(value)}>"
+
+
+def safe_exception_message(error: BaseException) -> str:
+    """Return bounded exception text without calling dependency ``str``/``repr``."""
+    try:
+        args = BaseException.args.__get__(error, type(error))
+    except Exception:
+        return _safe_type_name(error)
+
+    if not args or type(args[0]) is not str or not args[0]:
+        return _safe_type_name(error)
+    return single_line_log_text(args[0])
+
+
+def safe_exception_summary(error: BaseException) -> str:
+    """Render a bounded typed exception summary without dependency ``str``/``repr``."""
+    error_type = _safe_type_name(error)
+    try:
+        args = BaseException.args.__get__(error, type(error))
+    except Exception:
+        return error_type
+
+    if not args or type(args[0]) is not str or not args[0]:
+        return error_type
+
+    message_budget = MAX_LOG_DIAGNOSTIC_CHARS - len(error_type) - 2
+    if message_budget < len(_LOG_TRUNCATION_SUFFIX):
+        return error_type
+    return f"{error_type}: {single_line_log_text(args[0], max_chars=message_budget)}"

services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py

@@ -12,6 +12,7 @@
 import numpy as np
 from numpy.typing import NDArray
 
+from ..logging_safety import safe_exception_message, safe_exception_summary, safe_log_value
 from .model import TemporalFeatures
 
 logger = logging.getLogger(__name__)
@@ -73,7 +74,7 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures:
         if not path.exists() or not path.is_file():
             raise FileNotFoundError(f"Audio file not found: {path_str}")
 
-        logger.info(f"Loading and decoding audio: {path_str}")
+        logger.info("Loading and decoding audio: %s", safe_log_value(path_str))
 
         try:
             with path.open("rb") as fileobj:
@@ -128,7 +129,7 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures:
 
             bpm_val = float(tempo[0]) if isinstance(tempo, np.ndarray) else float(tempo)
 
-            logger.info(f"Analysis complete: {bpm_val:.1f} BPM, {len(beat_times)} beats detected.")
+            logger.info("Analysis complete: %.1f BPM, %d beats detected.", bpm_val, len(beat_times))
 
             return {
                 "bpm": bpm_val,
@@ -140,5 +141,9 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures:
             }
 
         except Exception as e:
-            logger.error(f"Failed to analyze audio {path_str}: {e}")
-            raise ValueError(f"Temporal analysis failed: {e}") from e
+            logger.error(
+                "Failed to analyze audio %s: %s",
+                safe_log_value(path_str),
+                safe_exception_summary(e),
+            )
+            raise ValueError(f"Temporal analysis failed: {safe_exception_message(e)}") from e