Security hardening (maziggy/bambuddy-security #N)

Subprocess output and user-supplied URLs are scrubbed of credentials
before they reach the application log. Adds a shared redaction helper in
core/logging_filters and routes the existing support-bundle sanitizer
through the same pattern.

backend/app/api/routes/camera.py

@@ -19,6 +19,7 @@ from backend.app.core.auth import (
     create_camera_stream_token,
 )
 from backend.app.core.database import get_db
+from backend.app.core.logging_filters import redact_url_credentials
 from backend.app.core.permissions import Permission
 from backend.app.models.printer import Printer
 from backend.app.models.user import User
@@ -276,9 +277,15 @@ def _summarize_ffmpeg_stderr(text: str | None) -> str:
     any actual error message. Logging the full banner on every retry floods
     the log (hundreds of lines per failed stream). This filter drops the
     banner and caps output at the last 10 meaningful lines.
+
+    Credentials are masked here rather than at each ``logger`` call because
+    this is the one funnel every stderr log in this module passes through.
+    ffmpeg echoes the RTSP input URL back in its ``Input #0`` line, which
+    carries the printer access code.
     """
     if not text:
         return ""
+    text = redact_url_credentials(text) or ""
     banner_prefixes = (
         "ffmpeg version ",
         "  built with ",

backend/app/core/logging_filters.py

@@ -1,4 +1,4 @@
-"""Logging filters for the Bambuddy log pipeline.
+"""Logging filters and redaction helpers for the Bambuddy log pipeline.
 
 Holds two filters: ``WriteRequestsOnlyFilter`` keeps the file-side
 uvicorn access log focused on state-changing HTTP methods, and
@@ -6,12 +6,45 @@ uvicorn access log focused on state-changing HTTP methods, and
 caused by Starlette's ``BaseHTTPMiddleware`` cancellation propagation
 (see the filter's docstring for details). Both live here so tests can
 import them without pulling in ``backend.app.main``'s startup graph.
+
+Also holds :data:`URL_CREDENTIALS_PATTERN` and
+:func:`redact_url_credentials`, the single place where the shape of a
+credentialed URL is defined for the whole backend.
 """
 
 from __future__ import annotations
 
 import asyncio
 import logging
+import re
+
+# ``scheme://user:secret@host`` — the only URL shape that carries a secret.
+# Both userinfo parts exclude ``/`` so the match can never run past the
+# authority into the path, and exclude whitespace so a wrapped log line can't
+# glue two URLs together. ``secret`` is otherwise unrestricted and greedy so
+# it reaches the *last* ``@`` before the path, which is where RFC 3986 ends
+# the userinfo — that keeps an unescaped ``@`` inside a password (legal in an
+# external camera URL) from leaving its tail in the log. Named groups let
+# callers choose how much to mask: the log pipeline keeps the username, the
+# support-bundle sanitizer drops it (see ``log_reader.sanitize_log_content``).
+URL_CREDENTIALS_PATTERN = re.compile(r"(?P<scheme>[a-zA-Z][a-zA-Z0-9+.\-]*://)(?P<user>[^/:@\s]+):(?P<secret>[^/\s]+)@")
+
+
+def redact_url_credentials(text: str | None) -> str | None:
+    """Mask the password in every ``scheme://user:secret@host`` URL in *text*.
+
+    Subprocesses echo their input URL back at us — ffmpeg prints the RTSP
+    input in its ``Input #0`` line, so logging its stderr verbatim publishes
+    the printer access code (or an external camera's password) into
+    ``bambuddy.log``, which users routinely attach to public issues.
+
+    The username, host, port and path survive so the line stays useful for
+    diagnosis; only the secret is replaced. Returns *text* unchanged when
+    there is nothing to mask, including ``None``/``""``.
+    """
+    if not text or "://" not in text or "@" not in text:
+        return text
+    return URL_CREDENTIALS_PATTERN.sub(r"\g<scheme>\g<user>:[REDACTED]@", text)
 
 
 class WriteRequestsOnlyFilter(logging.Filter):

backend/app/services/camera.py

@@ -16,6 +16,8 @@ import uuid
 from datetime import datetime
 from pathlib import Path
 
+from backend.app.core.logging_filters import redact_url_credentials
+
 logger = logging.getLogger(__name__)
 
 # JPEG markers
@@ -608,7 +610,8 @@ async def capture_camera_frame_bytes(
             logger.info("Successfully captured camera frame bytes: %s bytes", len(stdout))
             return stdout
         else:
-            stderr_text = stderr.decode() if stderr else "Unknown error"
+            # ffmpeg echoes the RTSP input URL, which carries the access code.
+            stderr_text = redact_url_credentials(stderr.decode()) if stderr else "Unknown error"
             logger.error("ffmpeg frame bytes capture failed (code %s): %s", process.returncode, stderr_text[:200])
             return None
 

backend/app/services/external_camera.py

@@ -17,6 +17,8 @@ from urllib.parse import urlparse
 
 import aiohttp
 
+from backend.app.core.logging_filters import redact_url_credentials
+
 logger = logging.getLogger(__name__)
 
 
@@ -195,9 +197,15 @@ async def capture_frame(
         JPEG bytes or None on failure
     """
     if snapshot_url:
-        logger.debug("capture_frame using snapshot override url=%s...", snapshot_url[:50])
+        # Redact before truncating — slicing first can cut the URL short of the
+        # ``@`` the pattern anchors on and leave the password in the log.
+        logger.debug("capture_frame using snapshot override url=%s...", redact_url_credentials(snapshot_url)[:50])
         return await _capture_snapshot(snapshot_url, timeout)
-    logger.debug("capture_frame called: type=%s, url=%s...", camera_type, url[:50] if url else "None")
+    logger.debug(
+        "capture_frame called: type=%s, url=%s...",
+        camera_type,
+        redact_url_credentials(url)[:50] if url else "None",
+    )
     if camera_type == "mjpeg":
         return await _capture_mjpeg_frame(url, timeout)
     elif camera_type == "rtsp":
@@ -311,7 +319,7 @@ async def _capture_mjpeg_frame(url: str, timeout: int) -> bytes | None:
     """
     safe_url = _sanitize_camera_url(url, ("http", "https"))
     if not safe_url:
-        logger.error("Invalid MJPEG URL format: %s...", url[:50])
+        logger.error("Invalid MJPEG URL format: %s...", redact_url_credentials(url)[:50])
         return None
 
     jpeg_start = b"\xff\xd8"
@@ -438,7 +446,8 @@ async def _capture_rtsp_frame(url: str, timeout: int) -> bytes | None:
         )
 
         if process.returncode != 0:
-            logger.error("ffmpeg RTSP capture failed: %s", stderr.decode()[:200])
+            # ffmpeg echoes the RTSP input URL, which carries the camera password.
+            logger.error("ffmpeg RTSP capture failed: %s", redact_url_credentials(stderr.decode())[:200])
             return None
 
         if not stdout or len(stdout) < 100:
@@ -504,7 +513,7 @@ async def _capture_snapshot(url: str, timeout: int) -> bytes | None:
     # Sanitize URL - returns reconstructed URL from validated components
     safe_url = _sanitize_camera_url(url, ("http", "https"))
     if not safe_url:
-        logger.error("Invalid snapshot URL format: %s...", url[:50])
+        logger.error("Invalid snapshot URL format: %s...", redact_url_credentials(url)[:50])
         return None
 
     try:
@@ -559,7 +568,7 @@ async def test_connection(url: str, camera_type: str) -> dict:
     Returns:
         Dict with {success: bool, error?: str, resolution?: str}
     """
-    logger.info("Testing camera connection: type=%s, url=%s...", camera_type, url[:50])
+    logger.info("Testing camera connection: type=%s, url=%s...", camera_type, redact_url_credentials(url)[:50])
     try:
         frame = await capture_frame(url, camera_type, timeout=10)
         logger.info("Capture result: %s bytes", len(frame) if frame else 0)
@@ -700,7 +709,7 @@ async def _stream_mjpeg(url: str) -> AsyncGenerator[bytes, None]:
     # Sanitize URL - returns reconstructed URL from validated components
     safe_url = _sanitize_camera_url(url, ("http", "https"))
     if not safe_url:
-        logger.error("Invalid MJPEG stream URL: %s...", url[:50])
+        logger.error("Invalid MJPEG stream URL: %s...", redact_url_credentials(url)[:50])
         return
 
     try:
@@ -837,7 +846,8 @@ async def _stream_rtsp(
         await asyncio.sleep(0.1)
         if process.returncode is not None:
             stderr = await process.stderr.read()
-            logger.error("ffmpeg RTSP stream failed immediately: %s", stderr.decode()[:300])
+            # ffmpeg echoes the RTSP input URL, which carries the camera password.
+            logger.error("ffmpeg RTSP stream failed immediately: %s", redact_url_credentials(stderr.decode())[:300])
             return
 
         buffer = b""

backend/app/services/log_reader.py

@@ -14,6 +14,7 @@ from sqlalchemy import select
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from backend.app.core.config import settings
+from backend.app.core.logging_filters import URL_CREDENTIALS_PATTERN
 from backend.app.models.printer import Printer
 from backend.app.models.settings import Settings
 from backend.app.models.user import User
@@ -168,8 +169,11 @@ def sanitize_log_content(content: str, sensitive_strings: dict[str, str] | None
                 continue  # Skip very short strings to prevent over-redaction
             content = re.sub(re.escape(value), label, content)
 
-    # Replace credentials in URLs (e.g. http://user:pass@host, rtsps://bblp:code@host)
-    content = re.sub(r"((?:https?|rtsps?)://)[^/:@\s]+:[^/@\s]+@", r"\1[CREDENTIALS]@", content)
+    # Replace credentials in URLs (e.g. http://user:pass@host, rtsps://bblp:code@host).
+    # Shares its pattern with the log-pipeline redaction in ``core.logging_filters`` so
+    # the two can't drift; the bundle drops the username too, where the live log keeps
+    # it for diagnosis.
+    content = URL_CREDENTIALS_PATTERN.sub(r"\g<scheme>[CREDENTIALS]@", content)
 
     # Replace email addresses
     content = re.sub(r"\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b", "[EMAIL]", content)

backend/tests/unit/test_log_credential_redaction.py

@@ -0,0 +1,112 @@
+"""Credentials must never reach bambuddy.log.
+
+Subprocesses echo their input URL back at us: ffmpeg prints the RTSP input in
+its ``Input #0`` line, so logging its stderr verbatim published the printer
+access code (or an external camera's password) into the log file — which users
+routinely attach to public GitHub issues.
+
+These cover the shared helper plus the two funnels that carry subprocess output
+into the log.
+"""
+
+import asyncio
+
+from backend.app.api.routes.camera import _read_ffmpeg_stderr, _summarize_ffmpeg_stderr
+from backend.app.core.logging_filters import redact_url_credentials
+from backend.app.services.log_reader import sanitize_log_content
+
+# What ffmpeg actually prints for the camera's local TLS-proxy input. The
+# access code sits in the userinfo of the URL it quotes back.
+FFMPEG_INPUT_LINE = "Input #0, rtsp, from 'rtsp://bblp:38A4KQ2P@127.0.0.1:48521/streaming/live/1':"
+
+
+class TestRedactUrlCredentials:
+    def test_masks_the_printer_access_code(self):
+        result = redact_url_credentials(FFMPEG_INPUT_LINE)
+        assert "38A4KQ2P" not in result
+        assert result == "Input #0, rtsp, from 'rtsp://bblp:[REDACTED]@127.0.0.1:48521/streaming/live/1':"
+
+    def test_keeps_everything_that_is_not_the_secret(self):
+        """Host, port, path and username stay — the line has to remain diagnosable."""
+        result = redact_url_credentials("rtsp://admin:hunter2@192.168.1.50:554/stream1")
+        assert result == "rtsp://admin:[REDACTED]@192.168.1.50:554/stream1"
+
+    def test_masks_every_scheme_not_just_the_ones_we_use_today(self):
+        for url, expected in (
+            ("http://user:pw@cam.local/snapshot", "http://user:[REDACTED]@cam.local/snapshot"),
+            ("https://user:pw@cam.local/snapshot", "https://user:[REDACTED]@cam.local/snapshot"),
+            ("rtsps://bblp:code@printer:322/streaming/live/1", "rtsps://bblp:[REDACTED]@printer:322/streaming/live/1"),
+            ("ftp://bblp:code@printer:990/", "ftp://bblp:[REDACTED]@printer:990/"),
+        ):
+            assert redact_url_credentials(url) == expected
+
+    def test_masks_a_password_containing_an_at_sign(self):
+        """The userinfo ends at the LAST @ before the path — no tail may survive."""
+        result = redact_url_credentials("rtsp://admin:p@ssw0rd@192.168.1.50/stream")
+        assert result == "rtsp://admin:[REDACTED]@192.168.1.50/stream"
+        assert "ssw0rd" not in result
+
+    def test_masks_several_urls_in_one_blob(self):
+        text = "first rtsp://bblp:AAAAAAAA@10.0.0.1/live then rtsp://bblp:BBBBBBBB@10.0.0.2/live"
+        result = redact_url_credentials(text)
+        assert "AAAAAAAA" not in result
+        assert "BBBBBBBB" not in result
+        assert result.count("[REDACTED]") == 2
+
+    def test_never_runs_past_the_authority_into_the_path(self):
+        """A later @ in the path must not drag the host into the mask."""
+        result = redact_url_credentials("rtsp://bblp:code@10.0.0.1/live/user@example")
+        assert result == "rtsp://bblp:[REDACTED]@10.0.0.1/live/user@example"
+
+    def test_leaves_credential_free_text_alone(self):
+        for untouched in (
+            "Connection refused",
+            "rtsp://10.0.0.1:554/stream1",
+            "mailto and user@example.com in prose",
+            "Starting USB camera stream from /dev/video0 at 10 fps",
+        ):
+            assert redact_url_credentials(untouched) == untouched
+
+    def test_tolerates_empty_and_none(self):
+        assert redact_url_credentials("") == ""
+        assert redact_url_credentials(None) is None
+
+
+class TestFfmpegStderrFunnel:
+    """`_summarize_ffmpeg_stderr` is the one funnel every stderr log in the
+    camera route passes through, so redaction lands there."""
+
+    def test_summary_strips_the_access_code(self):
+        stderr = f"{FFMPEG_INPUT_LINE}\n[rtsp @ 0x5] Could not find codec parameters\n"
+        result = _summarize_ffmpeg_stderr(stderr)
+        assert "38A4KQ2P" not in result
+        assert "[REDACTED]" in result
+        # The actionable error is untouched.
+        assert "Could not find codec parameters" in result
+
+    def test_incremental_reader_strips_the_access_code(self):
+        async def run():
+            reader = asyncio.StreamReader()
+            reader.feed_data(f"{FFMPEG_INPUT_LINE}\nError opening input: Connection refused\n".encode())
+            reader.feed_eof()
+
+            class _FakeProcess:
+                stderr = reader
+
+            return await _read_ffmpeg_stderr(_FakeProcess())
+
+        result = asyncio.run(run())
+        assert result is not None
+        assert "38A4KQ2P" not in result
+        assert "Connection refused" in result
+
+
+class TestSupportBundleSanitizerUnchanged:
+    """The bundle sanitizer shares the pattern but keeps its own, stricter
+    replacement — it drops the username too. Guard against drift."""
+
+    def test_bundle_still_drops_the_whole_userinfo(self):
+        result = sanitize_log_content("rtsp://bblp:38A4KQ2P@10.0.0.1/live")
+        assert "38A4KQ2P" not in result
+        assert "bblp" not in result
+        assert "[CREDENTIALS]@" in result